diff --git a/CLAUDE.md b/CLAUDE.md index 5ebfffe..3c82131 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,7 @@ Wire: `browser → web-ui → brain`, and the brain fans out to `docker compose` ## Where the important files are - **`cmd/brain/main.go`** — ~100 lines; names every package and how they wire. Best single starting point for the code. -- **`internal/`** — the brain's packages (`api`, `lifecycle`, `store`, `catalog`, `manifest`, `admission`, `caddy`, `profile`, `hostclient`, `protocol`, `auth`, `assertion`, `audit`, `events`, `version`, plus the health/observability set — `health`, `notify`, `applog`, `systemlive`, `storageverify` — and `internal/hostagent/…`, the host-side implementation packages). What each owns and the import rules are in `docs/architecture.md` # Inside the brain. +- **`internal/`** — the brain's packages (`api`, `lifecycle`, `store`, `catalog`, `manifest`, `admission`, `caddy`, `profile`, `hostclient`, `protocol`, `auth`, `assertion`, `audit`, `events`, `mailpreset`, `version`, plus the health/observability set — `health`, `notify`, `applog`, `systemlive`, `storageverify` — and `internal/hostagent/…`, the host-side implementation packages). What each owns and the import rules are in `docs/architecture.md` # Inside the brain. - **`cmd/`** — entry points: `brain`, `host-agent` (fake), `host-agent-real`, plus small tools (`malmo`, `malmo-storage-verify`, `malmo-network-verify`, `openapi-gen`). - **`web-ui/`** — the dashboard. Internal code architecture in `docs/dev/web-ui.md`. - **Catalog apps** — not in this repo, and they don't get added here. The artifacts (`manifest.yml`, `compose.yml`, icons, screenshots) live in `malmoos/store` under `apps//`, the control plane publishes them as one snapshot, and a box pulls that snapshot from a malmo endpoint at runtime — it keeps no copy on disk (cloud #62, `DECISIONS.md` 2026-07-02 and 2026-08-17). **Adding an app starts with a Catalog app issue in `malmoos/store`**, not here; authoring works from that issue per `docs/dev/authoring-apps-with-an-agent.md`, which keeps the schema (`internal/manifest`), the admission policy, the `malmo manifest` CLI, and the gap ledger on this side. Catalog test fixtures in this repo are **synthetic** — fake apps in the published wire shape, never a copy of a snapshot the endpoint serves (`internal/catalog/testdata/snapshot.json`). @@ -122,7 +122,7 @@ Small set of rules. Codified now so we don't have to back them out later. ## Working style -- **Always work in a git worktree for local implementations.** Use the `isolation: "worktree"` option when spawning agents, or manually create a worktree (`git worktree add`) before making changes. Never implement directly on the checked-out branch. +- **Work in the checked-out folder, not a worktree — unless the folder has uncommitted work.** Branch off latest `dev` and implement in place. A worktree is only for the case it exists to solve: uncommitted changes in the working tree that a checkout would disturb. Otherwise it just leaves a second copy of the repo to find, test in and clean up, and the finished branch has to be pulled back into this folder anyway. When you do need one, put it outside the repo folder (never `.worktrees/` at the repo root) or pass `isolation: "worktree"` when spawning an agent, and remove it once the branch is pushed. - **When reviewing a PR** (your own or someone else's), read `docs/dev/code-review.md` end-to-end before looking at any diff — it defines the lenses, severity levels, and what "reviewed" means on this project. Use a sonnet agent for code review (`model: "sonnet"`). - **Write in plain English — CEFR B1.** This applies to everything you produce: chat replies, docs, progress entries, PR bodies, issue text, commit messages, code comments, and UI copy. Short sentences, common words, one idea per sentence. Prefer "use" over "utilise", "let" over "facilitate", "so" over "consequently". Cut clause-stacking and em-dash pile-ups; split into two sentences instead. Technical terms the project owns (`manifest`, `reconciler`, `bind mount`, `PGDATA`) stay — B1 is about the sentence around the term, not about dumbing down the domain. Precision wins if the two ever conflict: say the exact thing in simple words rather than a vague thing in simple words. - Read the relevant `docs/specs/` doc(s) end-to-end before proposing changes — they cross-reference each other heavily and decisions in one constrain the others. Use `docs/README.md` to find the right one. diff --git a/Makefile b/Makefile index ced6cc6..6bee533 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ help: @echo "make check-web - pre-PR gate for frontend changes: web-ui typecheck + build" @echo "make clean - stop apps, remove dev state" @echo "make control-plane-images - build malmo-brain + malmo-ui images and docker-save the control-plane bundle to .dev/" - @echo "make dev - all three foreground procs in one terminal (recommended)" + @echo "make dev - all three foreground procs in one terminal (recommended); Go edits rebuild + restart the brain" @echo "make dev-app APP= [STORE=../store] - boot ONE store app under curation: seed its catalog snapshot, then make dev with an inert catalog URL" @echo "make seed-catalog APPS=\" ...\" [HOMEFILE=] - seed several store apps (+ optionally the curated landing) into a local snapshot file, without starting dev" @echo "make fmt - rewrite Go sources into gofmt-canonical form (autofix)" @@ -317,8 +317,7 @@ dev: check-state-owner build caddy @mkdir -p $(STATE_DIR) @cd web-ui && [ -d node_modules ] || npm install @trap 'kill 0' INT TERM EXIT; \ - (MALMO_DEV_AVAHI=1 $(DEV_DIR)/host-agent 2>&1 | sed -u 's/^/[agent] /') & \ - ($(DEV_DIR)/brain 2>&1 | sed -u 's/^/[brain] /') & \ + (GO="$(GO)" DEV_DIR="$(DEV_DIR)" LDFLAGS="$(LDFLAGS)" ./dev/dev-go.sh) & \ (cd web-ui && npm run dev 2>&1 | sed -u 's/^/[ui] /') & \ wait @@ -431,4 +430,4 @@ clean: stop caddy-down @# where the privileged uninstall path removes it. A plain `rm` as the dev @# user can't, so reclaim it via a throwaway root container first. No sudo. -@docker run --rm -v $(abspath $(DEV_DIR)/state):/state alpine:3 rm -rf /state 2>/dev/null || true - rm -rf $(DEV_DIR)/state + rm -rf $(DEV_DIR)/state $(DEV_DIR)/next diff --git a/api/openapi.json b/api/openapi.json index 64b9661..1c274e6 100644 --- a/api/openapi.json +++ b/api/openapi.json @@ -1625,6 +1625,33 @@ ], "type": "object" }, + "List-mail-presetsResponse": { + "additionalProperties": false, + "properties": { + "$schema": { + "description": "A URL to the JSON Schema for this object.", + "examples": [ + "https://example.com/schemas/List-mail-presetsResponse.json" + ], + "format": "uri", + "readOnly": true, + "type": "string" + }, + "presets": { + "items": { + "$ref": "#/components/schemas/MailPresetDTO" + }, + "type": [ + "array", + "null" + ] + } + }, + "required": [ + "presets" + ], + "type": "object" + }, "List-mail-provider-optionsResponse": { "additionalProperties": false, "properties": { @@ -1822,6 +1849,118 @@ ], "type": "object" }, + "MailPresetDTO": { + "additionalProperties": false, + "properties": { + "credential_label": { + "type": "string" + }, + "docs_url": { + "type": "string" + }, + "encryption": { + "enum": [ + "none", + "starttls", + "tls" + ], + "type": "string" + }, + "help": { + "type": "string" + }, + "host": { + "type": "string" + }, + "id": { + "type": "string" + }, + "label": { + "type": "string" + }, + "port": { + "format": "int64", + "type": "integer" + }, + "region": { + "$ref": "#/components/schemas/MailPresetRegionDTO" + }, + "username_fixed": { + "type": "string" + }, + "username_mode": { + "enum": [ + "user", + "fixed", + "same_as_password" + ], + "type": "string" + }, + "username_prefill": { + "type": "string" + } + }, + "required": [ + "id", + "label", + "host", + "port", + "encryption", + "username_mode", + "username_fixed", + "username_prefill", + "credential_label", + "help", + "docs_url" + ], + "type": "object" + }, + "MailPresetRegionDTO": { + "additionalProperties": false, + "properties": { + "default": { + "type": "string" + }, + "label": { + "type": "string" + }, + "options": { + "items": { + "$ref": "#/components/schemas/MailPresetRegionOptionDTO" + }, + "type": [ + "array", + "null" + ] + } + }, + "required": [ + "label", + "default", + "options" + ], + "type": "object" + }, + "MailPresetRegionOptionDTO": { + "additionalProperties": false, + "properties": { + "host": { + "type": "string" + }, + "label": { + "type": "string" + }, + "value": { + "type": "string" + } + }, + "required": [ + "value", + "label", + "host" + ], + "type": "object" + }, "MailProviderBody": { "additionalProperties": false, "properties": { @@ -1858,6 +1997,9 @@ "format": "int64", "type": "integer" }, + "provider_type": { + "type": "string" + }, "username": { "type": "string" } @@ -1911,6 +2053,12 @@ "format": "int64", "type": "integer" }, + "provider_label": { + "type": "string" + }, + "provider_type": { + "type": "string" + }, "username": { "type": "string" } @@ -1923,6 +2071,8 @@ "username", "from_address", "encryption", + "provider_type", + "provider_label", "created_at" ], "type": "object" @@ -1935,11 +2085,15 @@ }, "label": { "type": "string" + }, + "provider_type": { + "type": "string" } }, "required": [ "id", - "label" + "label", + "provider_type" ], "type": "object" }, @@ -3723,6 +3877,34 @@ "summary": "Revoke the current session" } }, + "/api/v1/mail-presets": { + "get": { + "operationId": "list-mail-presets", + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/List-mail-presetsResponse" + } + } + }, + "description": "OK" + }, + "default": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorModel" + } + } + }, + "description": "Error" + } + }, + "summary": "List built-in outgoing-mail provider presets (admin only)" + } + }, "/api/v1/mail-providers": { "get": { "operationId": "list-mail-providers", @@ -3812,7 +3994,38 @@ "description": "Error" } }, - "summary": "List provider picker options — id and label only (any authenticated user)" + "summary": "List provider picker options: id, label and provider type (any authenticated user)" + } + }, + "/api/v1/mail-providers/verify": { + "post": { + "operationId": "verify-mail-provider-config", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/MailProviderBody" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "No Content" + }, + "default": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorModel" + } + } + }, + "description": "Error" + } + }, + "summary": "Check an unsaved provider config by connecting and authenticating (admin only)" } }, "/api/v1/mail-providers/{id}": { diff --git a/api/openapi.yaml b/api/openapi.yaml index 6163ecc..551de1a 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -1147,6 +1147,25 @@ components: required: - apps type: object + List-mail-presetsResponse: + additionalProperties: false + properties: + $schema: + description: A URL to the JSON Schema for this object. + examples: + - https://example.com/schemas/List-mail-presetsResponse.json + format: uri + readOnly: true + type: string + presets: + items: + $ref: "#/components/schemas/MailPresetDTO" + type: + - array + - "null" + required: + - presets + type: object List-mail-provider-optionsResponse: additionalProperties: false properties: @@ -1286,6 +1305,87 @@ components: required: - user type: object + MailPresetDTO: + additionalProperties: false + properties: + credential_label: + type: string + docs_url: + type: string + encryption: + enum: + - none + - starttls + - tls + type: string + help: + type: string + host: + type: string + id: + type: string + label: + type: string + port: + format: int64 + type: integer + region: + $ref: "#/components/schemas/MailPresetRegionDTO" + username_fixed: + type: string + username_mode: + enum: + - user + - fixed + - same_as_password + type: string + username_prefill: + type: string + required: + - id + - label + - host + - port + - encryption + - username_mode + - username_fixed + - username_prefill + - credential_label + - help + - docs_url + type: object + MailPresetRegionDTO: + additionalProperties: false + properties: + default: + type: string + label: + type: string + options: + items: + $ref: "#/components/schemas/MailPresetRegionOptionDTO" + type: + - array + - "null" + required: + - label + - default + - options + type: object + MailPresetRegionOptionDTO: + additionalProperties: false + properties: + host: + type: string + label: + type: string + value: + type: string + required: + - value + - label + - host + type: object MailProviderBody: additionalProperties: false properties: @@ -1313,6 +1413,8 @@ components: port: format: int64 type: integer + provider_type: + type: string username: type: string required: @@ -1352,6 +1454,10 @@ components: port: format: int64 type: integer + provider_label: + type: string + provider_type: + type: string username: type: string required: @@ -1362,6 +1468,8 @@ components: - username - from_address - encryption + - provider_type + - provider_label - created_at type: object MailProviderOption: @@ -1371,9 +1479,12 @@ components: type: string label: type: string + provider_type: + type: string required: - id - label + - provider_type type: object NotificationDTO: additionalProperties: false @@ -2530,6 +2641,23 @@ paths: $ref: "#/components/schemas/ErrorModel" description: Error summary: Revoke the current session + /api/v1/mail-presets: + get: + operationId: list-mail-presets + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/List-mail-presetsResponse" + description: OK + default: + content: + application/problem+json: + schema: + $ref: "#/components/schemas/ErrorModel" + description: Error + summary: List built-in outgoing-mail provider presets (admin only) /api/v1/mail-providers: get: operationId: list-mail-providers @@ -2585,7 +2713,26 @@ paths: schema: $ref: "#/components/schemas/ErrorModel" description: Error - summary: List provider picker options — id and label only (any authenticated user) + summary: "List provider picker options: id, label and provider type (any authenticated user)" + /api/v1/mail-providers/verify: + post: + operationId: verify-mail-provider-config + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/MailProviderBody" + required: true + responses: + "204": + description: No Content + default: + content: + application/problem+json: + schema: + $ref: "#/components/schemas/ErrorModel" + description: Error + summary: Check an unsaved provider config by connecting and authenticating (admin only) /api/v1/mail-providers/{id}: delete: operationId: delete-mail-provider diff --git a/dev/dev-go.sh b/dev/dev-go.sh new file mode 100755 index 0000000..d092bd1 --- /dev/null +++ b/dev/dev-go.sh @@ -0,0 +1,117 @@ +#!/usr/bin/env bash +# The Go half of `make dev`: run the fake host-agent and the brain, and rebuild +# both when a .go file changes. +# +# Why this exists: Vite watches the UI and reloads it, but nothing watched the +# Go side, so a brain edit only reached the running process on the next +# `make dev`. The symptom is never obvious — a route added in this session +# answers 405, a new response field silently arrives undefined — and it costs +# more time to diagnose than the rebuild costs to run. +# +# Two choices worth knowing: +# +# * The debounce is long (10s by default) because most edits here arrive from +# a coding agent, which writes a burst of files over several seconds. A +# short debounce would rebuild and restart the brain in the middle of that +# burst, repeatedly. Set MALMO_DEV_DEBOUNCE=2 when editing by hand. +# * The build runs BEFORE the running processes are stopped, into a staging +# path. A build that fails leaves the old brain up and serving, so a typo +# never costs you the stack. (Staging is also what avoids "text file busy": +# Linux refuses to write over a running executable.) +# +# A restart drops open SSE streams — the dashboard reconnects on its own — and +# kills any install job in flight. +set -uo pipefail + +GO=${GO:-go} +DEV_DIR=${DEV_DIR:-.dev} +LDFLAGS=${LDFLAGS:-} +WATCH_DIRS=${MALMO_DEV_WATCH_DIRS:-cmd internal} +DEBOUNCE=${MALMO_DEV_DEBOUNCE:-10} +POLL=${MALMO_DEV_POLL:-2} + +STAGE="$DEV_DIR/next" +STAMP="$DEV_DIR/.dev-go-stamp" +agent_pid="" +brain_pid="" + +log() { printf '[watch] %s\n' "$*"; } + +start_procs() { + MALMO_DEV_AVAHI=1 "$DEV_DIR/host-agent" > >(sed -u 's/^/[agent] /') 2>&1 & + agent_pid=$! + "$DEV_DIR/brain" > >(sed -u 's/^/[brain] /') 2>&1 & + brain_pid=$! +} + +# SIGTERM, then SIGKILL after a few seconds. The wait is bounded on purpose: a +# child that ignores TERM (a wedged brain, a slow flush) would otherwise hang +# the Ctrl-C that got you here. +stop_procs() { + local pid alive waited=0 + # Unquoted on purpose: an empty pid drops out by word splitting. + for pid in $brain_pid $agent_pid; do kill "$pid" 2>/dev/null; done + while [ "$waited" -lt 5 ]; do + alive=0 + for pid in $brain_pid $agent_pid; do kill -0 "$pid" 2>/dev/null && alive=1; done + [ "$alive" = 0 ] && break + sleep 1 + waited=$((waited + 1)) + done + for pid in $brain_pid $agent_pid; do + kill -0 "$pid" 2>/dev/null && kill -KILL "$pid" 2>/dev/null + wait "$pid" 2>/dev/null + done + brain_pid="" + agent_pid="" +} + +trap 'stop_procs; exit 0' INT TERM EXIT + +# Anything modified since the last build started. +changed() { + [ -n "$(find $WATCH_DIRS -name '*.go' -newer "$STAMP" -print -quit 2>/dev/null)" ] +} + +# Anything modified inside the debounce window, i.e. the burst is still running. +settling() { + [ -n "$(find $WATCH_DIRS -name '*.go' -newermt "-$DEBOUNCE seconds" -print -quit 2>/dev/null)" ] +} + +# Build into the staging path. Both binaries, because a change in internal/ can +# belong to either one and working out which is not worth the seconds it saves. +build_go() { + mkdir -p "$STAGE" + touch "$STAMP" + $GO build -ldflags "$LDFLAGS" -o "$STAGE/host-agent" ./cmd/host-agent || return 1 + $GO build -ldflags "$LDFLAGS" -o "$STAGE/brain" ./cmd/brain || return 1 +} + +# -newermt with a relative time is a GNU find extension. Without it the loop +# still runs, it just does not watch — better than refusing to start. +if ! find . -maxdepth 0 -newermt '-1 seconds' >/dev/null 2>&1; then + log "this find has no -newermt, so Go changes will not rebuild; restart make dev after editing Go" + start_procs + wait + exit 0 +fi + +touch "$STAMP" +start_procs + +while true; do + sleep "$POLL" + changed || continue + log "Go files changed, waiting ${DEBOUNCE}s for the edits to settle" + while settling; do sleep "$POLL"; done + log "rebuilding" + if build_go; then + stop_procs + mv -f "$STAGE/host-agent" "$DEV_DIR/host-agent" + mv -f "$STAGE/brain" "$DEV_DIR/brain" + start_procs + log "host-agent and brain restarted" + else + log "build failed, so the running brain was left alone; fix and save again" + fi +done diff --git a/docs/architecture.md b/docs/architecture.md index ac64ef7..f27bcdd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -95,6 +95,7 @@ the `host-agent-real` row in # Components and by # What is not built yet. | `admission` | The single compose admission policy applied to both doors (image pinning rules, forbidden constructs, etc.). | `lifecycle` | | `caddy` | Client for Caddy's admin API. Site-block JSON generation lives here (per-app route via `AddRoute(RouteConfig)` — optional hosted `forward_auth` gate, a strip of the single `RouteConfig.StripCookieName` cookie from the `Cookie` header, never the whole header, #306/#335, an unconditional `RouteConfig.ScrubHeaders` delete, and a `subroute` that carves `RouteConfig.PublicPaths` out of the gate, #415), plus the hosted wildcard-TLS automation policy (`EnsureWildcardTLS`: ACME DNS-01 via the `acmedns` provider for `*..malmo.network`). Profile-agnostic: the strip/gate policy is resolved by the caller. | `lifecycle`, `cmd/brain` | | `profile` | The environment-profile marker (`appliance`\|`hosted`) + the first-boot seed reader, and the hosted URL-shape helpers (`HostedAppHost`/`HostedAppURL`/`HostedDashboardHost`/`CertSubjects` — the single place `..malmo.network` is named). Leaf package. | `api`, `lifecycle`, `cmd/brain` | +| `mailpreset` | The built-in outgoing-mail provider presets (host, port, encryption and the username rule per provider) plus `List`/`Get`/`Valid`/`LabelFor`. Hardcoded rather than catalog-served: the catalog is an app-distribution channel, these constants change roughly never, and the credential broker's per-provider logic has to be Go (`DECISIONS.md` 2026-08-27). Leaf package, no malmo imports. | `api`, `store` | | `hostclient` | Brain-side client for `host-agent`. Mirrors the routes in `protocol`. | `lifecycle`, `api`, `auth`, `cmd/brain` | | `protocol` | Wire types shared with `cmd/host-agent`. Source of truth for the host protocol. | `hostclient`, `cmd/host-agent` | | `auth` | First-admin bootstrap, password verification (delegates to host-agent), opaque cookie sessions, plus the hosted per-app forward-auth credential (a second, lower-privilege `Domain`-scoped cookie on the session row that the box Caddy's `forward_auth` verifies against the brain, #305). Owns `ForwardAuthCookieName`, the one source of truth for the cookie name the route builder strips (#335). No password hashes on the brain side. | `api`, `lifecycle`, `cmd/brain` | diff --git a/docs/dev/running-locally.md b/docs/dev/running-locally.md index c56154e..9c14d27 100644 --- a/docs/dev/running-locally.md +++ b/docs/dev/running-locally.md @@ -139,6 +139,20 @@ make dev # starts Caddy detached, then backgrounds agent + brain + ui No extra tools — pure bash supervisor with a `trap` that kills the process group on signal. +### Go changes rebuild themselves + +Vite watches the UI, and since `dev/dev-go.sh` the Go side is watched too: save a file under `cmd/` or `internal/` and `make dev` rebuilds both binaries and restarts them, printing `[watch]` lines as it goes. Before this, a brain edit only reached the running process on the next `make dev`, and the symptom was never obvious — a route added minutes ago answering 405, a new response field arriving `undefined`. + +Three things about how it behaves: + +- **The debounce is long: 10 seconds of no further edits before it builds.** Most edits here arrive from a coding agent, which writes a burst of files over several seconds; a short debounce would restart the brain in the middle of one, again and again. Editing by hand? `MALMO_DEV_DEBOUNCE=2 make dev`. `MALMO_DEV_POLL` (default 2s) is how often it looks. +- **A failed build leaves the running brain alone.** The build goes to `.dev/next/` first, and only a build that succeeded stops anything. A typo costs you a `[watch] build failed` line, not your stack. +- **A restart drops open SSE streams and kills any install job in flight.** The dashboard reconnects on its own; an install that was running does not resume. + +A rebuild is about 7 seconds for a change in `internal/api` (recompile plus linking a 22MB binary), 1–3 seconds for a leaf package, and nothing at all while you are only editing the UI. + +The watcher needs GNU `find` (`-newermt`), so it is Linux and WSL. Elsewhere `make dev` prints one line saying Go changes will not rebuild and runs exactly as it did before. + **Four terminals (no extra tools):** ```bash diff --git a/docs/dev/web-ui.md b/docs/dev/web-ui.md index 3a62a20..eceab8c 100644 --- a/docs/dev/web-ui.md +++ b/docs/dev/web-ui.md @@ -53,6 +53,8 @@ web-ui/ ├── lib/ │ └── utils.ts # cn() class-merge helper (shadcn convention) │ + ├── mailProviderForm.ts # outgoing-mail form shape + preset rules, shared by + │ # the add flow and the inline edit form ├── useInstall.ts # catalog-app install flow (plan fetch, consent dialog, │ # duplicate/job errors, per-app button state) — shared │ # by AppDetailView; see "Install flow" below @@ -70,6 +72,8 @@ web-ui/ │ ├── InstalledAppsSection.vue # manage/uninstall/logs list │ ├── ActivitySection.vue # audit-log browser (all users) │ ├── UsersSection.vue # admin-only user management + │ ├── OutgoingEmailSection.vue # admin-only SMTP account list + │ ├── OutgoingEmailAddSection.vue # /mail/add + /mail/add/:preset │ └── AboutSection.vue # product identity │ └── components/ # reusable chrome + dialogs @@ -78,6 +82,8 @@ web-ui/ ├── AppTile.vue # dashboard launcher tile (opens the app) ├── StoreAppCard.vue # store browse card (links to the detail page) ├── AppGlyph.vue # icon-less fallback: manifest icon_glyph → Lucide icon, else AppWindow + ├── MailProviderLogo.vue # provider mark from assets/mail-providers/, by preset id + │ # (that folder's README is the how-to for adding one) ├── SplitButton.vue ├── InstallDialog.vue, ElevateDialog.vue └── ToastHost.vue diff --git a/docs/progress/README.md b/docs/progress/README.md index aafd05e..233c7ea 100644 --- a/docs/progress/README.md +++ b/docs/progress/README.md @@ -245,3 +245,4 @@ Oldest first; append new entries to the bottom. | [path-scoped-app-exposure.md](path-scoped-app-exposure.md) — **Closes #415.** Hosted exposure was whole-app and binary, which has no answer for an app that pairs a **token-authed API** with a **session-authed UI**: letting an external SDK reach the API meant making the app public, which dropped the box login in front of the UI too. Two catalog apps already hit that wall, and one of them (Laminar) signs in **any email with no password** on its self-hosted UI, so "go public to ingest" read as "anyone can sign in as anyone". A manifest may now declare `access.public_paths`, and a `restricted` app's route becomes **one** Caddy `subroute` — declared paths straight to the app, everything else gated exactly as before — so `upsertRoute`'s insert-at-0 and the catch-all order are untouched, and the proxy handler is built once and used on both branches so the #335 per-cookie strip cannot land on one and not the other. **The scrub grew past the issue's shape and that is the load-bearing part:** the identity headers are now deleted at the head of **every** hosted app route, not just where the gate runs, because the gate does not run on a public path — the app would otherwise get a brain-vouched `X-Malmo-User` on one path and a client-forged one on another with no way to tell them apart — and the same hole already existed for a fully public app, where an app that learned to trust the header while restricted kept trusting it the moment the owner flipped the toggle. Validation refuses `/`, `/*`, `/**` (a manifest must not void the owner's toggle), a bare-suffix wildcard (`/v1*` also matches `/v1admin`), and `%`/`?`/`#`/`\`/`//`/`..` (the proxy matches a cleaned decoded path while the app sees the original URI, so those mean two different things on the two sides) — but the spec now says plainly that **catalog review is the trust boundary**, not this validator. The dashboard names the open paths instead of claiming a bare "Only me", reading them from the **instance's** manifest copy so the label cannot drift from the route, and carrying them on **both** responses that report an app's exposure (the detail `GET` and the toggle's echo) rather than only the first — the pair is what the label is built from. The hosted lane's `access` boot carries the proof through real Caddy, including a bypass table (prefix footgun, traversal, encoded traversal, double slash, case variant) that must stay gated. **The first CI run paid for itself twice:** `//v1/` never reaches the gate — Caddy collapses the duplicate slash and 301's to the normalized path before matching, so the probe was safe and the expectation was wrong (the table now asserts the claim that matters: an undeclared path never reaches the app upstream anonymously) — and that failure came back **green**, because the lane matched its verdict with a `*PASS*` glob and the reason read "PATH GATE **BYPASS**". The harness now anchors on the exact verdict string; any failure text containing *bypass*/*password* would have silently greened any boot in this lane, so that one was latent well beyond this PR. **Gaps:** no catalog manifest declares the field yet, so it ships inert; the `UPDATES.md` clause making a widened `public_paths` prompt the owner is specified, not built; anonymous public-path traffic is invisible to Activity and has no rate limit in front of it | done | | [catalog-no-box-side-copy.md](catalog-no-box-side-copy.md) — The box no longer keeps the catalog on disk. `loadCache` / the write-through in `syncOnce` are gone: the brain holds one snapshot in memory, re-fetches it, and shows an **empty store** when it cannot reach the endpoint, rather than browsing a last-good copy it could not install from anyway (a pinned old snapshot can also offer a manifest the store no longer publishes — `DECISIONS.md` 2026-08-17). `RemoteOptions.CacheDir` became `AssetCacheDir` and now means what it says. The three dev/test lanes that booted from a pre-seeded cache (`make dev-app`, both QEMU bootstraps) pass the snapshot explicitly as the new `MALMO_CATALOG_FILE` — read once, never written, unset on a real box. Icons and screenshots keep their disk cache but gained a **24-hour TTL**: filenames are stable per app, so without one the first icon a box fetched was the icon it served forever; an expired asset whose refetch fails is served stale rather than broken. The pinned wire fixture is now **synthetic** — three hand-written fake apps covering the full key surface, replacing a copy of the published catalog — with a `-update` flag that re-stamps its digest. `TestNoUnmodeledFields` is honest about what that costs it: it can no longer detect that the published shape moved, which becomes a publish-side check. `make test-nopam`, `fmt-check`, `vet`, `openapi-check` green | done | | [store-detail-costs-and-screenshots.md](store-detail-costs-and-screenshots.md) — The store detail page now shows **what a third party charges** and **what the app looks like**. `external_costs` reached the box API in [catalog-wire-external-costs.md](catalog-wire-external-costs.md) and nothing rendered it, so a person learned about listmonk's mail bill after installing; a pricing panel above Information now shows a `Price` row plus each cost as a disclosure, **required ones open** (the one you must read to decide) and optional ones collapsed, with the rate estimate and the manifest prose. The "you pay the provider, not malmo" note is a **CSS-only tooltip** (`group-hover` / `group-focus-within`), so it works for pointer, keyboard and tap with no script. Price stays a **constant in the view**, not a wire field — every app is free today and what malmo charges is authored in the curation source — so there is one line to change when it does reach the wire. The screenshot strip, which was inert, now opens each shot **fit to the viewport** with arrow keys, prev/next and a live counter, using a **native ``** rather than the fixed-overlay idiom `AppMenuDialog` hand-writes: the browser supplies the top layer, backdrop, Esc, focus trap and focus restore, leaving an index and a `src` swap. Both halves are ported from the marketing store's app page in `../cloud`, so the two surfaces show one catalog the same way — minus its progressive-enhancement half, which a Vue view cannot use. **Gaps:** typechecked and built but not yet clicked in a browser; `web-ui` has no test runner; the install dialog still does not repeat a required cost | done | +| [mail-provider-presets.md](mail-provider-presets.md) — **Closes #426.** Adding an email account meant typing **seven fields** the admin had to look up in their provider's docs, four of which are constants for any provider worth presetting. Settings → Outgoing email is now a two-step add: pick the provider, then supply only the credential, the from address, a username where the provider does not fix one, and a region for the two providers whose region changes the host — with host / port / encryption prefilled behind an **Advanced settings** disclosure, so a non-standard endpoint is never trapped, and `custom` rendering as that disclosure open with nothing prefilled (the old form). Nine presets in a new leaf package `internal/mailpreset`, served admin-only at `GET /api/v1/mail-presets`, **every one STARTTLS on a port a hosted box can reach** (587, except SMTP2GO's 2525) — hosted blocks 25 and 465, so no preset may default to implicit TLS. **Two of the issue's constants were wrong and one changed the design:** Mailgun's EU host is `smtp.eu.mailgun.org`, a *prefix* rather than a region code, so the planned single `{region}` host template could not serve both it and SES — each region option now names the host it resolves to, one mechanism with no substitution; and Brevo's username is the SMTP login `xxx@smtp-brevo.com`, not the account email, which is the kind of detail that turns into a failed test-send. `mail_providers.provider_type` persists **which** preset was picked (`DEFAULT 'custom'` via the idempotent ALTER path, no `CHECK` — one cannot ride an ALTER, so it is validated in Go like `scope` and `exposure`); pre-preset rows load as `custom`, which is right, they were typed by hand. It is kept **for the credential broker**, not for the form: without it every provider registered before the broker ships is an untyped row someone must classify by pattern-matching hostnames. The server **stores what the client sends and never re-derives** host / port / encryption from the preset, because re-deriving would silently undo the advanced override — so `provider_type` records what the admin picked, not a guarantee the values still match. The test-send now **names the blocked port** on hosted (a live bug for anyone who already registered a 465 provider there) and only on a *connect* failure, since an auth rejection means the port was reachable. `internal/lifecycle/mail.go` is untouched on purpose — `mailEnvLines` is the seam the broker replaces, and keeping it out is what makes the two features independent. **Gaps:** **no live test-send was performed against any provider** — that is the issue's real acceptance gate and needs a provisioned hosted box plus a real account at each of eight providers; the constants are verified against current vendor docs and nothing more. The SES region list is a common subset (8 of ~19); edit does not restore the region select (only the resolved host is stored); `web-ui` has no test runner; `make check` could not run end to end on this machine (`msteinert/pam` fails to build identically on unmodified `dev`) | done | diff --git a/docs/progress/mail-provider-presets.md b/docs/progress/mail-provider-presets.md new file mode 100644 index 0000000..e8d24f6 --- /dev/null +++ b/docs/progress/mail-provider-presets.md @@ -0,0 +1,110 @@ +# Outgoing email: pick a provider instead of typing seven fields + +**Issue:** #426. Builds on [byo-outgoing-mail.md](byo-outgoing-mail.md) (#122) and on the port measurement recorded in `SERVICE_PROVISIONING.md` # BYO outgoing mail (#425). + +## What was done + +Adding an email account meant typing seven fields — host, port, encryption, username, password, from address, label — each of which the admin had to look up in their provider's docs. For every provider worth presetting, four of those are constants. So the add flow is now two steps: pick the provider, then fill in only the credential, the from address, a username when the provider does not fix one, and a region for the two providers whose region changes the host. + +**`internal/mailpreset`** (new leaf package, no malmo dependencies) holds the table plus `List`, `Get`, `Valid` and `LabelFor`. Nine entries: `ses`, `sendgrid`, `mailgun`, `postmark`, `brevo`, `resend`, `smtp2go`, `google_workspace`, `custom`. Every one is STARTTLS on a port a hosted box can reach — 587, except SMTP2GO's 2525, the port it recommends as open in the most places. Hosts, ports and username rules were each checked against that provider's own docs on 2026-08-27; the package comment says so and asks the next person to re-check. + +Two of the issue's constants were wrong, and both changed the design: + +- **Mailgun's EU host is `smtp.eu.mailgun.org`** — a prefix, not a region code — so the issue's single `{region}` host template could not serve both it and SES. Each region option now names the host it resolves to. One mechanism, no substitution, and adding a provider with an odd regional host later costs nothing. +- **Brevo's username is the SMTP login `xxx@smtp-brevo.com`**, not the account email. That is help text, not structure, but it is exactly the kind of thing that turns into a failed test-send and a support message. + +**Store.** `mail_providers.provider_type` via the existing idempotent ALTER path, `DEFAULT 'custom'`. No `CHECK` — one cannot ride an ALTER, so it would apply only to fresh DBs; validated in Go instead, the way `scope` and `exposure` are, on both write paths. Rows that predate this change load as `custom`, which is right: they were typed by hand. + +**API.** `GET /api/v1/mail-presets`, admin-only, matching `listMailProviders` — only an admin can register a provider. `MailProviderDTO` and `MailProviderBody` gain `provider_type`; an unknown value is a 422, an empty one normalizes to `custom`. The DTO also carries a derived `provider_label`, so a row can render "Amazon SES" without the UI re-deriving it and without breaking if a preset is ever withdrawn. The test-send now **names the blocked port**: a hosted box that fails to *connect* on 25 or 465 gets "this box cannot reach port 465, use 587 with STARTTLS" appended instead of a bare timeout. Only a connect failure triggers it — an auth rejection means the port was reachable, and naming it would mislead. + +**Web UI.** `OutgoingEmailSection.vue`: a provider grid, then a short form with the credential labelled per provider ("Server API token", not "Password"), the provider's one-line "what you need and where to get it" plus a docs link, and an **Advanced settings** disclosure holding host, port and encryption — prefilled and editable. `custom` opens that disclosure with nothing prefilled, which is the old form. The label defaults to the provider name so it stops being a field the admin must invent. A hosted admin who selects TLS/465 sees the warning before saving. Rows show the provider name rather than the raw host; a hand-typed row still shows the host, because it has no name to show. + +**Unchanged on purpose:** `internal/lifecycle/mail.go`, the install plan, `PUT /apps/{id}/mail-binding`, every audit action. `mailEnvLines` is the seam the credential broker replaces later, and keeping it out is what makes the two features independent (`DECISIONS.md` 2026-08-27, D4). + +## Decisions + +`DECISIONS.md` 2026-08-27 records D1 (presets in the brain, not the catalog), D2 (`provider_type` is persisted, because the broker needs the identity), D3 (the server stores what the client sends and never re-derives host/port from the preset, so the advanced override survives) and D4 (no change to env injection). + +The one deviation from the issue text is the region mechanism, above. The other is `provider_label` on the DTO, which the issue did not list: the alternative was for the UI to resolve the label from the preset list, which works only for admins and breaks on a withdrawn preset. + +## Verification + +**Every preset host was probed live**, using the same library the test-send uses (`net/smtp`): dial, STARTTLS with `ServerName` set, EHLO, read the AUTH extension. All sixteen hostnames — the nine presets plus all eight SES regions — resolve, connect on the port the preset carries, complete TLS with a certificate valid for that exact name, and offer `AUTH PLAIN`, which is the mechanism `sendTestMail` uses. That covers hostname, port, TLS and auth-mechanism compatibility. It does **not** cover the username rules or that mail actually leaves, and it ran from a developer machine, not a hosted box, so it says nothing about the 25/465 block. + +`make test-nopam`, `gofmt`, `go vet`, `make openapi` and the web typecheck + production build are green. New tests: the preset table's invariants (a fixed username mode with no value, a region option with no host, a port a hosted box cannot reach), an old row migrating to `custom`, an unknown `provider_type` rejected at both the store and the API, `provider_type` round-tripping without the host being re-derived, `/mail-presets` refused to a member, and the blocked-port hint firing only on a hosted connect failure. + +`make check` was not run end to end: this machine cannot build `github.com/msteinert/pam/v2` (`C.RTLD_NEXT`), which fails identically on unmodified `dev`. Everything `check` runs apart from that package was run individually. + +## Self-review + +A sonnet agent reviewed the diff against `docs/dev/code-review.md` and found one Block, now fixed: **editing a Postmark account wiped its username.** `startEdit` blanks the password field, because an empty password means "keep the stored one"; the update path then copied that empty value across to the username, which `same_as_password` keeps in step. Editing a Postmark account's label was enough to leave it with a stored password and no username, failing AUTH on its next send with nothing in the UI to say why. A blank password is no longer copied across, so the paired username is kept the same way the password is. + +Two Notes were also acted on. `blockedPortHint` matched on the string prefix `"connect: "`, a silent coupling to `sendTestMail`'s error wording that no test would have caught breaking; dial failures are now a typed `*dialError` matched with `errors.As`, and a new test drives the real `sendTestMail` against a closed port to assert the live path still produces one. (Reverting the type makes that test fail, which is the point of it.) The `:open` binding on the two `
` disclosures is correct today only because both blocks remount on change; that assumption is now written next to the binding rather than left for the next person to rediscover. + +## The form, second pass + +The first version of the picker was a grid of bordered text boxes and a two-column form of bare placeholder inputs. Three things were wrong with it and all three are fixed: + +- **The cards did not read as clickable.** They are now real buttons carrying the provider's logo above its name, with hover, focus-visible and active states. +- **The first field was confusing.** It said only "Name", and nothing said whose name. It is now labelled **Account name**, with a hint saying it is malmo's own label for the account and only the admin sees it. Every other input gained a real `