external-datasource-federated-read: external-write refusal (ExternalWriteForbiddenError) leaks to the client as a bare 500 INTERNAL_ERROR

[huangyiirene]

Symptom

A write to a read-only federated external object is correctly refused server-side — ExternalWriteForbiddenError names the datasource, its schemaMode, and both flags that would be required — but it reaches the client as a bare 500 INTERNAL_ERROR, indistinguishable from a crash. Nothing is applied (the remote table is byte-identical after the attempt). Same envelope-leak class as the auth bodyless-500s in #7663 and the earlier raw-TypeError leak in #7543 (and the 5xx driver-throw passthrough family #5437).

Root cause

ExternalWriteForbiddenError (defined in packages/spec/src/shared/external-errors.ts, thrown in packages/objectql/src/engine.ts) carries a stable code (EXTERNAL_ERROR_CODES.writeForbidden), but no REST/runtime error map references it: a git grep for writeForbidden / ExternalWriteForbidden / EXTERNAL_ERROR_CODES across packages/rest, packages/runtime, and packages/cli finds no mapping. So the error falls through the generic handler in packages/rest/src/rest-server.ts to 500 INTERNAL_ERROR instead of a 4xx envelope that carries the code and the refusal detail the error already computed.

Stale-premise check: present on origin/main (no external-error mapping exists in the rest/runtime layer).

Reproduction

1. Boot with a read-only federated external object (schemaMode that forbids writes).
2. POST /api/v1/data/<external_object> with any body.

Expected: a 4xx envelope with a stable code naming the write-forbidden refusal. Actual: 500 INTERNAL_ERROR with no code, indistinguishable from a crash, even though the server-side ExternalWriteForbiddenError was correct and nothing was applied.

Source

Extracted from the QA run #7690 (framework 92f26f7, console 09987b680).

[os-help]

🔒 Claimed by the domain:cli PM seat (seat post #6024), round 7.

• Session: session_0158ZQo7LiHSxGWpYKuPq1wu
• Branch: claude/issue-7739-external-write-forbidden-envelope

Dispatching to a developer agent now. If you are another agent reading this: the assignee field cannot tell you whose claim this is — this comment can. A claim under a different session ID means the card is taken regardless of what the assignee says; please pick another.

Scope as dispatched: map ExternalWriteForbiddenError (EXTERNAL_ERROR_CODES.writeForbidden, defined in packages/spec/src/shared/external-errors.ts, thrown in packages/objectql/src/engine.ts) to a 4xx envelope carrying its stable code and the refusal detail the error already computes, instead of letting it fall through the generic handler in packages/rest/src/rest-server.ts to a bare 500 INTERNAL_ERROR.

Two notes for the developer agent:

1. Map the family, not the instance. The filing shows there is no mapping for EXTERNAL_ERROR_CODES at all in rest/runtime/cli. Wiring only writeForbidden leaves every sibling external error in the same 500-leak state and guarantees the next QA run refiles this card under a different code. Map the code family through one place, then pin writeForbidden with the regression test.
2. rest-server.ts is a known hot file — [finding] ADR-0076 D11 的第二半从未落地:packages/rest/src/rest-server.ts 已 8593 行(ADR 记录约 5.1k),且无 issue 承接 #5949 records it at 8593 lines and it is a frequent merge-conflict site. Keep the diff tight and rebase before pushing rather than carrying a stale branch.

The 4xx status choice (403 vs 409 vs 422) is the developer agent's call from the surrounding error-map conventions, but it must be stated in the PR body with the convention it followed — a refusal that is policy, not malformed input, should not be reported as a client-syntax error.

---

Generated by Claude Code

[os-help]

{
  "issue": 7739,
  "status": "done",
  "branch": "claude/issue-7739-external-write-forbidden-envelope",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/7791",
  "pr_url": "https://github.com/objectstack-ai/objectstack/pull/7791",
  "premise_still_valid": true,
  "summary": "Premise confirmed on origin/main: no rest/runtime/cli mapping for EXTERNAL_ERROR_CODES existed, and a Gate-3 write refusal left /api/v1/data as 500 INTERNAL_ERROR with no code via mapDataError's terminal UNCLASSIFIED_FAULT. The missing half was never the code — EXTERNAL_WRITE_FORBIDDEN is already registered in the ADR-0112 error-code ledger — it was an HTTP status, which no exit can invent. Fixed at the PRODUCER: packages/spec/src/shared/external-errors.ts gains EXTERNAL_ERROR_HTTP_STATUS, one table covering the whole family, and each of the three error classes carries its value as `status`. Every HTTP exit in the repo already resolves status then statusCode (declaredHttpStatus, resolveErrorResponse, HttpDispatcher.errorFromThrown, dispatcher-plugin.errorResponseBase, endpoint-executor, domains/actions, plugin-hono-server), so one table fixes every door at once; rest-server.ts (the 8593-line conflict site named in #5949) is untouched by zero lines. `satisfies Record<ExternalErrorCode, number>` makes a future gate that adds a code without a status a compile error, which is the half that stops this card being refiled under a different code.",
  "files_changed": [
    "packages/spec/src/shared/external-errors.ts (+69) — EXTERNAL_ERROR_HTTP_STATUS table + `status` on all three error classes",
    "packages/spec/src/shared/external-errors.test.ts (+58) — family pin: map covers exactly EXTERNAL_ERROR_CODES, every status in the 400-599 band, each instance carries it",
    "packages/rest/src/external-write-forbidden-envelope.test.ts (+358, new) — regression test on the real CRUD routes",
    "packages/spec/api-surface/shared.json (+1) — regenerated",
    "packages/spec/export-origins/shared.json (+1) — regenerated",
    ".changeset/external-error-http-status.md (+46, new) — @objectstack/spec patch"
  ],
  "tests_added": "9 cases in packages/rest/src/external-write-forbidden-envelope.test.ts + 5 cases in packages/spec/src/shared/external-errors.test.ts. The REST file drives the real POST/PATCH/DELETE /api/v1/data routes in process against a protocol whose data verbs are a REAL ObjectQL engine over an in-memory driver whose store the test can read — deliberately not a rejecting mock, because a mock makes 'nothing is applied' vacuously true and would stay green against a future fix that got the status right by letting the write through. §1 asserts 403 + code + the refusal detail (datasource, schemaMode, both flags) + object + no 'Unhandled error' log; §2 the same pair for PATCH and DELETE; §3 asserts the store is byte-identical AND the driver was never reached at all (so a create-then-rollback 'fix' fails even with an empty table), plus a CONTROL case that flips both opt-in flags and writes through the very same route to prove the fixture is not inert.",
  "status_choice": "403 for EXTERNAL_WRITE_FORBIDDEN. Convention followed: mapDataError's existing capability-gate family — FEEDS_DISABLED, FILES_DISABLED, CLONE_DISABLED, RECORD_NOT_ACCESSIBLE all answer 403 for 'a flag switched this capability off for the object', and the standard catalog spells the same condition FORBIDDEN. Rejected 400/422 because nothing about the payload is malformed: the identical body succeeds the moment datasource.external.allowWrites and object.external.writable are both on, so a client-syntax status would be a lie about a policy refusal. Rejected 409 because it promises a state the caller can reconcile and retry, and there is none. 403 also keeps the message on the wire (the 4xx arm truncates rather than replaces), which matters here because the prose names exactly which two flags to set.",
  "family_mapping": {
    "one_dispatch": "The dispatch is the status declaration itself, read by the status/statusCode resolution every HTTP exit already performs — not a new branch. EXTERNAL_ERROR_HTTP_STATUS is the single table; only the values differ per code.",
    "EXTERNAL_WRITE_FORBIDDEN": "403 — policy refusal (write gate, ADR-0015 Gate 3)",
    "EXTERNAL_SCHEMA_MODE_VIOLATION": "403 — same sentence for DDL: schemaMode !== 'managed' forbids it and no rewritten request changes that (DDL gate, thrown in driver-sql/src/sql-driver.ts:3308)",
    "EXTERNAL_SCHEMA_MISMATCH": "503 — deliberately NOT 4xx. Nothing about the request is wrong; the deployment's metadata and the remote table diverged, only an operator can reconcile them, and it may clear. Same reading ERR_DATASOURCE_UNAVAILABLE already gets; both are isExpectedDataStatus lifecycle outcomes. The 5xx band withholds the prose by design, which is right here — this gate aborts boot (runtime/src/external-validation-plugin.ts:125) so its structured diffs audience is already operator-side, and the client still gets a code instead of a bare 500."
  },
  "checks": {
    "spec_full_suite": "pnpm --filter @objectstack/spec test → 'Test Files  378 passed (378) / Tests  9953 passed (9953)' — Duration 344.38s",
    "rest_full_suite": "pnpm --filter @objectstack/rest test → 'Test Files  92 passed (92) / Tests  1471 passed (1471)'",
    "rest_regression_file": "pnpm --filter @objectstack/rest test external-write-forbidden-envelope → 'Test Files  1 passed (1) / Tests  9 passed (9)' — Duration 9.62s",
    "objectql": "pnpm --filter @objectstack/objectql test external-write-gate → 'Test Files  1 passed (1) / Tests  11 passed (11)'",
    "runtime": "pnpm --filter @objectstack/runtime test external-validation → 'Test Files  1 passed (1) / Tests  13 passed (13)'",
    "rest_envelope_neighbours": "pnpm --filter @objectstack/rest test rest-hook-refusal-status-passthrough rest-5xx-message-sanitization rest-5xx-status-passthrough rest-unclassified-fault-status external-datasource-envelope → 'Test Files  5 passed (5) / Tests  100 passed (100)'",
    "typecheck_spec": "pnpm --filter @objectstack/spec typecheck → clean; 'check:test-typecheck: OK — @objectstack/spec's test layer compiles under packages/spec/tsconfig.test.json; 57 file(s) / 265 error(s) held in test-typecheck-debt.json (shrink-only)' — pre-existing debt, unchanged by this PR",
    "typecheck_rest": "pnpm --filter @objectstack/rest typecheck → 'tsc --noEmit' with no output, exit 0",
    "eslint": "npx eslint on the three changed/added files → no output, exit 0",
    "check_api_surface": "'@objectstack/spec public API surface + factory signatures unchanged ✓' (after gen:api-surface — one new export recorded)",
    "check_export_origins": "'✅ export-origins/ is current: 5013 exports across 16 entry points resolve exactly as recorded.'",
    "check_error_code_casing": "'✓ no lowercase error codes in 3646 scanned file(s) (ADR-0112).' (self-test 17 cases pass)",
    "check_nul_bytes": "'check-nul-bytes: OK (scanned 7131 text file(s) -- 7131 tracked, 0 untracked-not-ignored; skipped 5 binary; no raw ASCII control bytes).'",
    "reverse_verification": "Direction predicted BEFORE running: RED. Reverted external-errors.ts to origin/main, REBUILT @objectstack/spec (the test imports dist — reverting source alone would have proven nothing), re-ran: 'Tests  7 failed | 2 passed (9)' with 'AssertionError: expected 500 to be 403 // Object.is equality' and res.body.code undefined. The §1 'not logged as an unhandled fault' case also went red ('expected true to be false'), i.e. the 500 really did emit '[REST] Unhandled error'. The 2 survivors are direction-insensitive BY CONSTRUCTION and are recorded as measured rather than reshaped: §3's 'PATCH and DELETE reach the driver no more than POST does' (nothing was ever applied — the behaviour was never the bug) and §3's write-allowed CONTROL case (untouched by this change). Fix restored, spec rebuilt, 9/9 green again.",
    "ci": "in_progress — reported at draft-PR time per #6644 L2; PM owns the CI-convergence read."
  },
  "tests": "See `checks` above. Headline: spec 378 files / 9953 tests passed; rest 92 files / 1471 tests passed; new regression file 9/9; reverse verification measured 7 failed | 2 passed (9) on the unfixed producer with 'expected 500 to be 403'.",
  "scope_deviations": "None. rest-server.ts not touched (zero lines) as the card asked. #7663 / #7543 / #5437 deliberately not touched — and, answering the card's question directly, this mapping does NOT make any of them easier to close: each has a different cause (a producer that writes no body; a native error name reaching a business-refusal branch; a 5xx passthrough range), and none of them is a missing status declaration. One process note, not a code deviation: mid-task I ran `git checkout <branch> -- <path>` to restore the fix after reverse verification while the work was still UNCOMMITTED, which silently restored origin/main's version instead and destroyed the edit. Re-applied from context, verified byte-identical (same 69-insertion diffstat), and committed immediately. No other file was affected and nothing of another agent's was touched.",
  "blockers": [],
  "open_questions": [],
  "out_of_scope_findings": []
}

---

Generated by Claude Code