fix: fail closed on invalid self-hosted API responses

[andrei-hasna]

Summary

• validate every declared self-hosted JSON success envelope before repositories, CLI/TUI/MCP projections, and the generated SDK consume it
• validate routine error statuses, generic 404 absence paths, and root health probes without forwarding credentials or exposing raw response bodies
• align OpenAPI, generated SDK/contracts, attachment index semantics, replay/reconciliation behavior, fixtures, and release metadata for 1.3.2
• isolate CI test files by process so process-wide mode/credential mutations cannot route local tests through another file’s self-hosted fixture

Verification

• merged local full suite: 2489 passed, 138 skipped, 0 failed, 12525 assertions across 2627 tests
• merged PostgreSQL 18.1 full suite: 2625 passed, 2 skipped, 0 failed, 13772 assertions
• merged OpenAPI/webhook suite: 61 passed, 0 failed
• merged V1 stub/paging suite: 78 passed, 0 failed
• TypeScript and generated SDK signature checks: passed
• deterministic generation: selfhost 3c7e10d8e7cceb6991d1b253edd272201c6eb47e742d4123c022e00c35cac5fa; response contracts 750487b69c23d49cde4714134aedd9eb26b805e02a8d1d147c97abe42f5b6057
• build, no-cloud source/packed-artifact scans, and package identity: passed
• npm dry run: @hasna/emails 1.3.2, 753 files
• staged and commit-range gitleaks scans: 0 findings
• fresh independent terminal reviewers: PASS after all P1/P2 and merge-conflict reconciliation
• GitHub head 4330ff2: verify, selfhost-postgres, container-runtime, and Terraform validate all passed; PR is CLEAN

This PR does not publish the package or deploy a service.

[andrei-hasna]

Triaged against main (5bb126e). The core of this PR is right and I want it landed — but not in its current shape. Findings below, with the evidence for each.

The fail-closed core is good and complements what landed

Validating every declared /v1 success envelope before repositories, CLI/TUI/MCP projections and the generated SDK consume it is the same discipline as today's bounded-read honesty fix (#81) and the fail-closed refusal contract. It complements rather than duplicates them: #81 made a bounded read refuse when it cannot prove completeness; this makes a malformed response refuse instead of being coerced. Different failure, same rule. It also adds no credential storage — no schema changes, no migrations, no provider-secret columns — so the encryption-at-rest concern does not apply here.

1. ci: isolate test files by process (4330ff2) should be dropped

It replaces one bun test with a serial loop running one Bun process per test file at --max-concurrency 1 (225 invocations).

It is not needed. I checked out this PR's head and ran its own suite with plain single-process bun test in a clean environment:

2489 pass, 0 fail, 138 skip — Ran 2627 tests across 225 files

Green. The 27 failures on the earlier head (f91165a) were fixed by the env commit 5513c03, not by process isolation. Isolation is masking nothing that is still broken — it is just cost:

	verify duration
this PR (isolated processes)	3m35s
main	~2m00s
#84	2m08s

It also costs signal. bun test produced one aggregate summary; 225 invocations under bash -euo pipefail abort at the first failing file, so a red run now shows one file instead of the full failure set. And the only aggregate assertion left is test "${#test_files[@]}" -gt 0 — the file list is non-empty, but nothing asserts the expected test count, so a file that silently runs zero tests passes.

If there is a cross-file pollution case I did not reproduce, it is worth naming and fixing directly — isolation hides that class permanently.

2. The env fix overlaps #84 — and we found it independently

5513c03 ci: clear self-hosted env in local tests adds -u EMAILS_CLIENT_ENV_SECRET (plus EMAILS_SESSION_TOKEN, DATABASE_URL, …). I hit the same thing from the other direction in #84: the mere presence of EMAILS_CLIENT_ENV_SECRET routes the store to self-hosted mode (src/db/self-hosted-store.ts:83), and ~126 local-mode tests fail with HTTP 401. Two sessions finding this independently is a good argument it belongs on main on its own.

The two PRs do auto-merge cleanly (I test-merged them), though the result carries -u EMAILS_CLIENT_ENV_SECRET twice — harmless, worth tidying in whichever lands second. Your list is the more complete one.

3. The version bump does not belong here

package.json goes 1.3.1 → 1.3.2. A release decision shouldn't ride along inside a fail-closed correctness fix — please split it out.

4. This deepens the structures that are about to be deleted

The EMAILS_MODE axis is slated for removal and the .local/.remote split for collapse behind a store seam. Against that:

destination	added	removed
src/self-hosted-wire-regression.test.ts	+1124	0
src/lib/self-hosted-wire.ts	+567	0
src/lib/self-hosted-mail-data-source{,.test}.ts	+249	-116
src/db/self-hosted-store{,.test}.ts	+300	-49
src/cli/tui/data.remote.ts, src/db/provisioning.remote.ts	+42	-14
total in doomed files	+2309	-179

Plus 11 new EMAILS_MODE references, and 36,774 of the 44,232 insertions are one generated artifact (self-hosted-response-contracts.generated.ts).

Landing this as-is means writing ~2,300 lines of it twice. If the validation can be expressed once at the store seam, the refactor inherits it instead of having to port it.

Suggested split

Land now, independently — small, valuable, no interaction with the refactor:

1. 5513c03 env scrubbing (or take fix(db): batch cold database initialisation behind an immediate transaction #84's and add your extra names)
2. extending the generated-SDK drift check to src/lib/self-hosted-response-contracts.generated.ts — a real gap
3. the test:selfhost-sdk-types step

Drop: 4330ff2 process isolation (evidence above), and the 1.3.2 bump.

Hold / restructure against the store seam: the wire-validation body and its generated contracts.

Not merging, and why

Green CI is necessary but not sufficient here: the PR bundles a correctness fix with a release bump and a CI restructuring that measurably costs time and signal without being needed. I am not merging it — the split above is a judgement call for whoever owns the refactor sequencing, and it is cheap to do before the seam lands rather than after. Happy to be wrong about the isolation commit if there is a pollution case I missed.

For reference: #84 fixes the timeout flakes at their cause (cold SQLite init was doing ~200 individually-fsynced DDL statements; 940 ms → 50 ms) with no test changes and no raised timeouts, which removes the pressure that a --max-concurrency 1 workaround would otherwise be relieving.

[andrei-hasna]

Correction and addendum to my previous comment

I need to correct something I said above, and the corrected version is more useful.

What I got wrong. I said the process isolation "is not needed" because this PR's tree is green under a single-process bun test. The run was real (2489 pass / 0 fail / 138 skip), but my conclusion was too strong: it is green in bun test's default sorted order. Run the same 225 files in one process in a shuffled order and it is not green:

2210 pass / 17 skip / 21 fail / 21 errors — Ran 2248 tests across 225 files

Note the totals move: 2627 → 2248 tests actually run, 138 → 17 skips. So the suite genuinely is order-fragile, and 4330ff2 does neutralise something real. I was wrong to imply otherwise.

What that something is — and it originates in this PR. The 27 CI failures were caused by EMAILS_MODE going missing from the shared test process, after which resolveEmailsModeSelection() (src/lib/mode.ts:120-158) falls through to EMAILS_CLIENT_ENV_SECRET → on-disk config → default. Injecting EMAILS_MODE=self_hosted reproduces all 27 failures with the exact CI test names and the exact per-group counts (3 / 23 / 1), including why only one resend inbound webhook test fails (that file's own afterEach clears the leak for the remaining six). The chain is createProvider → selfHostedResource(...) → assertNoProviderCredentials throwing (src/db/provider-credentials.ts:42).

None of src/server/routes/{rest-parity,core-redaction,resend-webhook}.test.ts, core.ts, or api-routes.ts differ between origin/main and this PR — identical blobs. So this was never a conflict with the store/paging or webhook work that landed. I withdraw that part of my triage too.

Two files delete EMAILS_MODE instead of restoring it, and both are added or changed by this PR:

1. src/self-hosted-wire-regression.test.ts — new here, 1124 lines. clearSelfHostedEnv() at lines 224-231 does delete process.env["EMAILS_MODE"] and is wired to afterEach (line 253). CI sets EMAILS_MODE=local once, for one shared process, so after this file runs the variable is simply gone for every file that sorts after it.
2. src/mcp/inbox-tools.test.ts — +118 here. The test at line 353 deletes MAILERY_MODE, HASNA_MAILERY_MODE, EMAILS_MODE, HASNA_EMAILS_MODE (lines 359-361); the finally (373-383) restores HOME, EMAILS_DB_PATH, EMAILS_CLIENT_ENV_SECRET and EMAILS_SESSION_TOKEN but not those four, and leaves EMAILS_SELF_HOSTED_URL / EMAILS_SELF_HOSTED_API_KEY set.

So the shape is: this PR introduces a process.env leak in its own new tests, and then changes CI to run 225 isolated processes so the leak cannot propagate. The fix is save-and-restore in those two blocks — the same pattern inbox-tools.test.ts already uses correctly for the other four variables it touches. Fix those and the isolation commit, the -u additions, and the order-pinning all stop being load-bearing. My recommendation to drop 4330ff2 stands, but only together with that fix, not on its own.

Two smaller notes on that commit, still worth acting on:

• --max-concurrency 1 is a no-op — nothing in the suite is declared .concurrent, so 4627409 ("serialize hermetic tests") cannot have fixed anything.
• The find predicate matches only *.test.ts / *.test.tsx, while bun test also auto-discovers *.spec.*, *_test.*, and .js/.jsx. The sets are identical today (225 either way), so nothing is masked now — but a .spec.ts added later would be silently skipped by CI, which is a discovery guard quietly narrowing.

One defect neither of us flagged: the CHANGELOG attribution

The new ## 1.3.2 (2026-07-26) heading is inserted immediately after ## [Unreleased] and above the pre-existing unreleased bullets. On main the fix(status) / test(status) / BREAKING fix(status) / fix(mcp) / fix(cli) entries sit under [Unreleased]; here they render under 1.3.2, and [Unreleased] is left empty. That silently ships ~10 unrelated entries — including a BREAKING one — as part of this release. Worth fixing regardless of what happens to the version bump.

Credit where it is due

Two changes in here actively reduce secret exposure and I would not want them lost in a split:

// SelfHostedHttpError no longer captures or interpolates the response body
-    readonly bodyText: string,
-    super(`Self-hosted ${method} ${path} failed: HTTP ${status}${bodyText ? ` — ${bodyText.slice(0,300)}` : ""}`);
+    super(`Self-hosted ${method} ${path} failed: HTTP ${status}`);

// requireCredential:false is now genuinely credentialless
-    const credential = process.env[EMAILS_SESSION_TOKEN_ENV]?.trim() || process.env["EMAILS_SELF_HOSTED_API_KEY"]?.trim() || "";
+    config = { baseUrl, credential: "" };

Plus a response-size ceiling (EMAILS_SELF_HOSTED_HTTP_MAX_RESPONSE_BYTES, default 8 MiB). And confirmed: no credential storage anywhere — src/db/database.ts, src/server/self-hosted/migrations.ts and store.ts are untouched, the whole diff contains no product DDL, and the 36,774-line generated contracts file declares no access_key / secret_key / oauth_* field. The encryption-at-rest concern does not apply to this PR.

One thing for a reviewer's eye: the generated per-resource item schemas in src/server/self-hosted/openapi.ts now carry an explicit required list and additionalProperties: true, justified in-code by the runtime still doing SELECT * over possibly-drifted operator tables. The published contract therefore permits undeclared columns in responses, and the redaction guarantee rests entirely on redactColumns rather than on the schema. That is a defensible trade for a fail-closed PR to state explicitly.

Net: my split recommendation is unchanged, with the env-restore fix added to the "land now" list and promoted above the CI change.