Mail/562 fixes and inbound foundation - #586
Merged
Merged
Conversation
Importing a foreign nginx carried over its SHIPPED default vhost (`server_name localhost; root html;`) as if it were a site. That reached the vhost writer and died on "must be an absolute path" — an accurate sentence about config that is perfectly valid nginx — and surfaced to the operator as "1 site not served" for a placeholder page they never hosted. Two fixes at the importer: - Filter loopback `server_name`s where `_` and regex names were already being filtered. A loopback name only matches a request that ARRIVED with `Host: localhost`, so it can never be served for anybody through a public edge. A block left with no names falls into the existing no-usable-name skip, which already treats that as expected rather than a lost site. A vhost carrying both a loopback and a real name keeps the real one. - Resolve prefix-relative roots against nginx's compiled `--prefix`, read from `nginx -V`. `-T` inlines includes but does not rewrite directive VALUES, so a bare `root html` survives the dump verbatim and only the prefix says where it points. When no prefix is reported we skip with that as the stated reason rather than guessing the compiled default, because a wrong guess publishes the wrong directory. The `-V` call is lazy: only paid when a relative root is actually present. Also fixes the wizard's report of a PARTIAL import. `importMigratedSites` returns ok:false when even one site missed, so keying the warning off `ok` and printing the full count claimed every site was dark one line after the import itself said "Migrated 3/4". It now reports only the real shortfall, and leaves retry advice to the import — the only layer that knows whether the cause was transient or a config it will reject identically forever.
Creating a mailbox returned 500 on every containerized install, with no
actionable error anywhere in the log.
`doveadm` and the `vmail` user live wherever Dovecot does, which on a
container-flavor box is INSIDE `openship-mail` and emphatically not on the
host. Both helpers ran bare against the raw host executor:
- `hashPassword` ran `doveadm pw -s SSHA512`, got empty output from a host
that has no doveadm, and threw when the SSHA512 gate rejected it.
- `createMaildirOnDisk` ran `chown -R vmail:vmail` against a host with no
`vmail` user, failing the whole `&&` chain after the DB rows were inserted.
The reporter's own evidence confirms the mismatch: they verified doveadm was
available IN THE CONTAINER while the code was running it on the host.
Both now go through `runMailCommand(target, flavor => mailEngineCommand(...))`,
which also buys the wrong-flavor reclassification a hand-rolled exec skips.
Two traps that came with it:
- `mailEngineCommand` prefixes `docker exec <container>`, so a top-level
`a && b` would run `a` in the engine and `b` ON THE HOST — the exact bug
class being fixed. The maildir chain is handed to one `sh -c`, matching the
precedent at mail.service.ts:91.
- The maildir tree was one level too shallow. `mail_location` is
`maildir:%Lh/Maildir/` with `home = <base>/<node>/<maildir>`, so the tree
Dovecot opens is `<home>/Maildir/{cur,new,tmp}`, not `<home>/{cur,new,tmp}`.
Harmless only because the LDA autocreates the real one, which is also why
nothing caught it.
`mail-credentials.service.ts` carried a PRIVATE COPY of the hasher plus a
hardcoded `sudo -u postgres psql`, i.e. the same defect twice. Both are
deleted in favour of the shared helpers, so the transport is decided in one
place.
Separately, a 5xx `AppError` was logged NOWHERE — and `AppError`'s statusCode
defaults to 500, so `new AppError(msg)` answered 500 in silence. That is the
"500 with no detailed error in the logs" half of the report. 5xx now logs with
the request method and path; 4xx stays quiet, since those are client outcomes.
Tests assert the COMMAND STRING per flavor. Every existing mail test hands
these helpers a vi.fn() executor and asserts on the resulting rows, which is
precisely how a command that could not run anywhere stayed green.
) On first deploy the mail engine came up with no schema: dovecot, iredapd and amavis crash-looped against an empty database while the boot log claimed success, and the only recovery was running the bootstrap by hand. Three compounding defects, and the third is why it was silent: 1. The wait was `nc -z` in entrypoint.sh. A TCP probe succeeds as soon as postgres BINDS its port, which is before it will serve a query — during initdb or crash recovery it accepts the connection and then refuses with "the database system is starting up". The loop also fell through after 60 tries WITHOUT checking, so an absent sidecar proceeded anyway. 2. `bash db-bootstrap.sh || log "ERROR: ..."` downgraded a hard failure to a log line and then started every daemon against a schemaless DB. 3. db-bootstrap.sh ran `set -uo pipefail` with NO `-e`, so it never returned non-zero for (2) to catch. Every psql failed in turn and it still printed "── DB bootstrap complete ──" and exited 0. Verified against the pre-change script: exit 0, "complete" printed, and the postmaster password column literally empty. Now: `set -euo pipefail`, a `SELECT 1` readiness poll that lives INSIDE the script (so the documented manual re-run is self-sufficient), and the closing four psql prints turned into ASSERTIONS — including one that refuses to report success when the postmaster row has an empty password. `PGCONNECT_TIMEOUT` is not optional: against a host that drops packets libpq blocks for the OS default, so the wait budget never gets a second iteration and the boot hangs with no output. The e2e caught that. Two `set -e` hazards the e2e also caught: `[ -f x ] && cmd` returns non-zero for a missing optional file, and a bare `ls` of an absent directory exits 2 — on the WARN path, so it aborted the bootstrap on its way to printing a warning. `doveadm` joins the Dockerfile smoke gate, and the gate now runs `doveadm pw` to prove it works. It only ever arrived as a transitive dependency — iRedMail's package list names dovecot-imapd/pop3d/lmtpd/managesieved/sieve/pgsql, never dovecot-core — so nothing guaranteed the binary three separate paths depend on. The e2e runs the REAL script against a real postgres sidecar with the repo's real engine/samples, stubbing only doveadm so its failure mode can be driven. Booting openship-mail itself is not viable in a test: unpublished image, full iRedMail install to build. Files travel by `docker cp` rather than a bind mount so the gate does not evaporate on a macOS/Colima daemon.
…nly mail group Wiring only, no producer yet. It lands first because an unmapped eventType is dropped by `notification.emit` with NO log and NO delivery row — so without this the inbound-rules UI would look like it works while nothing was ever sent. - CATEGORY_GROUPS gains `mail`, placed before `billing` so the cloud-only group stays last in the Settings tab strip. The group id union is derived from this array (`as const satisfies`), so the category below would not compile without it. - `mail.inbound_received` category, with defaultEnabled FALSE and deliberately so: the dispatcher's fallback fans a default-enabled category to every org member's verified email channel with no opt-in, and for a per-message event that is both a flood and a mail loop — the notification would land on the same engine, inside a watched domain, and capture itself. - EVENT_TYPE_TO_CATEGORY mapping, without which the emit is silent. - No EVENT_HEADLINES entry: the registry test only permits an override on a category carrying more than one mapped eventType, so adding one there fails CI. The wording lives in the category label/description, which renderMessage takes verbatim as the alert's title and first body line. `listCategories` becomes symmetric instead of billing-specific: each group is dropped in the mode that can never produce it — billing outside CLOUD_MODE, mail inside it, since the whole mail module is absent from the cloud runtime. The filter stays in the controller rather than in CATEGORIES, because findCategory supplies the title and body of every delivered alert and the registry has to stay complete or a stored row degrades to a raw id. The audit taxonomy entry is required even though this emit never writes an audit_event row: the taxonomy scan greps apps/api/src for `eventType:` literals and is over-inclusive by design. Tests pin the direction of the new gate (mail hidden on cloud, present self-hosted, registry still complete either way) and that inbound mail can never default to enabled. The pinned category-id array is an ordered compare, so the new id is inserted at its real position.
…e matching The decision layer for inbound-mail rules, kept PURE so none of its failure modes need a mail server to test. All three are silent in production: LOOPS. A notification about mail is itself mail. Four guards, none of them tuning knobs: a null envelope sender (a bounce — and notifying on bounces is how a bounce storm becomes an alert storm), `Auto-Submitted` other than `no`, bulk/junk `Precedence`, `List-Id`, plus this instance's own outbound sender addresses. Miss any one and a single alert delivered to an address in a watched domain re-captures itself until somebody notices. SPAM. There is no upstream filter to rely on: the shipped amavis policy sets spam_lover='Y' AND bad_header_lover='Y' on the catch-all '@.' policy with empty quarantine targets, so the global $final_spam_destiny = D_DISCARD never applies to any recipient. Spam is delivered and therefore captured. Bad-header mail is delivered too and carries no X-Spam-Flag at all, which is why the score is checked independently of the flag. With no threshold set, a positive flag alone drops. FAIL CLOSED. There is no CHECK constraint tying `scope` to `target` (this schema has none anywhere), so a mailbox/domain rule with a null target is representable. Treated as "no constraint" it would silently widen to every message on the server — an operator who mistyped a rule quietly shipping a whole domain's mail metadata to Slack. It matches nothing instead, as does a scope this build does not recognise. Two smaller decisions worth the comment they carry: - Header parsing unfolds continuation lines. Long To/Subject values ARE folded in real mail, and a naive line split would truncate a subject and lose half a recipient list — which would make a mailbox rule miss its own target. - Operator patterns are case-insensitive substring with `*` as the ONLY wildcard, never a regex. Every metacharacter is escaped: these come from a text box and run on the mail path, so a pasted `(a+)+$` must not stall it. Known limit, documented in the module header because it is a property of Postfix's config and not something the control plane can fix: `enable_original_recipient = no` means the BCC copy carries no X-Original-To, so a mailbox-scope rule can only attribute a message via To/Cc and will miss anything Bcc'd or alias-expanded. A domain-scope rule has no such gap, since capture itself is domain-keyed.
…onnection Boot-tested the whole catalog against real containers. Several apps had never run at all, and the failures were structural rather than cosmetic. Dead on arrival, now fixed: - MinIO crash-looped on every boot since its `command` was added. Openship wraps a template `command` as ["sh","-c",cmd], and MinIO's entrypoint prepends `minio` unless argv[0] already is it, so the container ran `minio sh -c "server /data"` and exited with "'sh' is not a minio sub-command". No command string can fix that, so add `commandArgv` to the template schema: exact argv, no shell wrap. The DB column and runtime already supported it (#332); only the template could not ask. - PostHog could never have served a request. `./bin/plugin-server` was deleted upstream on 2025-12-30 and moved to posthog/posthog-node (exec: not found); `./bin/docker-server` calls `bin/migrate-check` under `set -e`, which exits 1 on an empty database, so web never bound :8000; and ClickHouse mounted no config, so migrate_clickhouse could not create a single ReplicatedMergeTree table. Rebuilt as the full 21-service topology with a Caddy path router, since Django no longer serves /e, /capture or /batch — ingestion moved to the Rust capture services, and the old 9-service shape could not ingest an event. - Directus created no admin at all: `create-admin.js` bare-returns unless ADMIN_EMAIL and ADMIN_PASSWORD are set, and Directus has no first-visit signup, so nothing could ever sign in. The description claimed otherwise. - code-server crash-looped with EACCES: none of its three volume paths exist in the image while it runs as uid 1000, so each named volume mounted root:root. One volume at /home/coder inherits ownership and persists everything. - Redis ran `valkey-server /etc/valkey/valkey.conf` from files[], which is skipped on the cloud runtime; and if the bind source was not a file Docker mounted an empty directory, so valkey started WITH NO PASSWORD and the old healthcheck still reported healthy. Now VALKEY_EXTRA_FLAGS, and the healthcheck asserts auth is actually required. The `sh -c` wrap was doing quiet damage beyond MinIO: redis/valkey dropped privileges only when argv[0] is literally `valkey-server`, so they ran as root; and on Debian images PID 1 stayed as dash, which never forwards SIGTERM, so PostHog's Celery drain trap was dead code. Those services use commandArgv now. Ports: freshrss, it-tools and vaultwarden claimed :80 (the edge owns it) and uptime-kuma claimed :3001 (Openship's own dashboard owns it) — all four failed to start with real "port is already allocated" errors. Host ports are now unique catalog-wide and avoid 80/443/3001. They are kept rather than removed because {{publicUrl:svc}} resolves from the published host port on a port-only install; without one, PUBLIC_URL/DOMAIN/ROOT_URL are omitted entirely. Security defaults that were wrong: Gitea shipped an unauthenticated install wizard anyone could claim, Vaultwarden allowed open registration on a password manager, and Stirling-PDF silently created the publicly-known admin/stirling. New: ClickHouse with the CH-UI console (verified server-side, so the database never needs to be public), and Neon rebuilt on the neond control plane — Neon's own console is proprietary and the upstream repo ships no web UI, so this is the only way to get self-hosted Neon with a dashboard. Also fixes multi-route collapse: only proxyRoutes[0] got a pinned host port while resolveTargetUrl handed that one port to every route, so MinIO's `s3` subdomain served the console and Convex's `http` subdomain served the 3210 API. Now one host port per routed port, with regression tests. Adds stopGracePeriod to the schema (the engine always honoured it) so an app whose clean shutdown does real work is not SIGKILLed at Docker's 10s default. 27 of 29 apps now install with a UI link and a connection path. PostHog and Neon remain verified:false — neither has been booted end to end.
Booted the pinned neond image and drove it end to end. The console is real — a 1.2 MB Vue bundle embedded in the Rust binary, so no HTML exists on disk — and the connection string reached PostgreSQL 17.5 over TLS. But the bootstrap step had two defects that only show up against a real container: - It fired too early. The management API answers /api/auth/setup before the storage layer can accept a tenant, and Openship runs prepare[] the moment readiness passes, so creating the project returned nothing and the step gave up with pg_port=no-project. Creates are retried now. - It could not recover. Re-running against an already-bootstrapped instance printed a sentinel that cannot match capturePattern, so if the first capture ever missed, NEOND_PG_PORT never persisted and the Database URL stayed blank forever. It now signs in, finds the existing org/project/branch, and reads the live port out of the branch's connection_string — every run reports a real port. (The branches payload carries the port inside connection_string, not as a "port" field, which is why the first attempt at this still read nothing.) Verified on a fresh volume with prepare fired at the earliest possible moment (pg_port=55434, psql -> PostgreSQL 17.5), and on a re-run against an existing install (same port, idempotent). Marks the app verified, and documents the two upstream hazards that cannot be fixed from a template: a hard kill leaves a boot lockfile that must be removed by hand, and a container restart can leave a stale compute lock while the API still reports the endpoint running. An ordinary redeploy is safe — that is what stopGracePeriod buys, and it was tested.
This was referenced Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Motivation
Related issue
Changes
Verification
Screenshots
Checklist
bun run test,bun run --cwd <workspace> lint, andbun formatall pass locally