Preserve explicit API error responses

changelog.d/record-hook-client-errors.md

@@ -0,0 +1,3 @@
+### Fixes
+- Record before-create, before-update, and before-delete hooks preserve explicit `ctx.fail` 4xx and 5xx statuses supported by the HTTP transport and their messages, and `ctx.invalid` field details. Stack-backed error messages and fields are copied before hooks return; recovered failures cannot leak into later handlers. A supported status without a dedicated code, such as 422, uses `bad_request`. Other hook failures, including unnamed 4xx statuses such as 432, retain the generic 400 response, and rejected writes still roll back atomically. Hook 5xx responses invoke `onError` and the reporter backstop with phase `before_hook`, including allocation-failure fallback responses; each report's message names the collection, status, and hook message.
+- Unmapped 4xx statuses use the generic `bad_request` code consistently in typed routes, route `ctx.fail`, and auth-method failures. Auth-method failure messages with a named 4xx status are deliberately client-facing, including 422; these previously became a generic 500 for statuses without dedicated codes. Auth-method 5xx and unnamed-status (such as 432) messages remain operator-only: logged and redacted from client responses.

docs/framework.md

@@ -304,6 +304,32 @@ The six valid phase fields are `beforeCreate`, `afterCreate`, `beforeUpdate`,
 `any` group runs first, then the collection-specific group; only the field
 matching the current phase runs.
 
+On HTTP record create, update, and delete, a before-hook can return
+`ctx.fail(status, message)` to reject the write with a **supported error**
+status and message in the standard error envelope. Supported statuses are
+the transport’s named 4xx and 5xx statuses (`http.isErrorStatus`). The machine `code` follows the same
+status mapping as other `ctx.fail` callers (`409` → `conflict`; a supported status
+without a specific code uses `bad_request` for 4xx and `internal` for 5xx). `ctx.invalid(fields)` returns a 400 validation envelope
+with the supplied field details. The primary write and the hook's database
+side-writes roll back together. After rollback and writer release, rendered 5xx
+failures invoke `onError` and its reporter backstop with phase `before_hook`;
+deliberate 4xx failures do not. The event's `message` is
+`<collection>: <status> <hook message>`, so reporter dedup keeps distinct failures
+apart. Its `err` is `error.Handled`, or `error.OutOfMemory` when the response is the
+allocation-failure fallback.
+For example:
+
+```zig
+return ctx.fail(409, "That item is already queued.");
+```
+
+Any other returned error, `error.Handled` without a stashed error, or an
+unsupported `ctx.fail` status (including unnamed 4xx values such as 432 or 499)
+retains the generic 400 response
+(`bad_request`, `Request rejected by a hook.`). Internal error names are never
+returned to the client. After-hook errors keep their existing error-reporting
+behavior and do not change the committed write's response.
+
 Every hook has the signature:
 
 ```zig
@@ -332,7 +358,8 @@ Need the app itself (e.g. `app.allocator`, `app.io`)? Use the hook's `ctx.app`
 ### Semantics
 
 - **`before*` hooks** may MUTATE `ev.record` and may return an error to REJECT
-  the write (the request fails with `400`). Early locked-rule and tenant gates
+  the write (explicit supported `ctx.fail` statuses and `ctx.invalid` details
+  are preserved; other errors return generic `400`). Early locked-rule and tenant gates
   run first; conditional create/update predicates are checked in the write
   transaction after the hook. A later denial rolls back its database side-writes.
   A before-hook is not proof of final authorization: keep external side effects
@@ -1732,7 +1759,10 @@ fn confirmBooking(ctx: *zigbase.Ctx) anyerror!zigbase.http.Response {
 }
 ```
 
-`ctx.fail(status, message)` stashes the error and returns `error.Handled`.
+`ctx.fail(status, message)` copies the error and returns `error.Handled`.
+`ctx.fail` and `ctx.invalid` copy all supplied strings and field descriptors
+before returning, so stack buffers are safe. The `Ctx` owns these copies until
+`deinit` or the next handler invocation; a rendered response owns its separate body.
 `ctx.errorResponse(err)` maps any error to a `{code, message, data}` JSON response:
 
 | `anyerror` | HTTP status |
@@ -2648,6 +2678,14 @@ ZigBase ships with a **pluggable auth-method system** that lets every auth colle
 1. **Initiate** — challenge delivery (email a link/code, return WebAuthn options, or a no-op for API-key flows).
 2. **Complete** — proof verification. On success the method returns a `Resolution.record(record_id)` and the framework mints the session via the one seam (`issueSession`+`emitAuth`), firing `onAuth`. The method never mints sessions itself.
 
+Auth-method `Resolution.fail` messages for the transport's named 4xx statuses
+(`http.isErrorStatus`) are intentionally public and returned to the anonymous
+caller. A status without a dedicated machine code (such as 422) uses
+`bad_request`. Write these as client explanations, without operator secrets.
+Server-error and unnamed-status (such as 432) failure messages remain
+operator-only: they are logged and replaced with the generic 500 `internal`
+response.
+
 There are three tiers:
 
 | Tier | How | When |

docs/recipes.md

@@ -449,8 +449,11 @@ to control built-in record operations. A route guard does not replace record rul
 A `beforeCreate` hook runs after early locked-rule/tenant gates and **before** the
 write. A conditional create rule is checked afterward, inside the same transaction;
 a denial rolls back the hook's database side-writes. Put external side effects in
-after-hooks. The hook may mutate `ev.record` and may **return an error to reject** the write (the request
-fails with `400`). This example, on `bookings`, (a) rejects a booking that overlaps
+after-hooks. The hook may mutate `ev.record` and may **return an error to reject** the write.
+Use `ctx.fail(status, message)` with a [supported error status](framework.md#4-record-hooks-hooks)
+(for example, 409) for a specific status and explanation, or
+`ctx.invalid(fields)` for field validation details; other errors return a generic
+`400`. This example, on `bookings`, (a) rejects a booking that overlaps
 an existing confirmed booking for the same listing, and (b) computes a derived
 `duration_minutes` field.
 

scripts/allocator-allowlist.txt

@@ -2,7 +2,7 @@
 # A test may wrap testing.allocator in an arena ONLY if the code under test takes
 # RequestArena (contract 4). Every line here is a promise to remove it or justify it.
 src/api/auth.zig	41	3 non-handler outliers un-masked: a dead unused arena in the gcExpiredSessions test; the backlog test whose arena only held transient session-id strings; and the "findByIdentity nocase" test (now that records.zig's create/error-path partial-record leak is fixed, #334, it frees col via Collection.deinit, the applyCreate graphs via freeProvisioned, the record via freeRecord, and each findByIdentity hit — a duped id string — after its compare). The rehash-on-login tests un-mask two more: "verifyPasswordUpgrading verifies a legacy hash…" and "…leaves an argon2 hash untouched" call verifyPasswordUpgrading directly (no RequestArena/Response graph), so they run on raw std.testing.allocator via the seedRawHash helper, which owns and frees every intermediate (fetched Collection via Collection.deinit, the applyCreate/records.create graphs, the UPDATE SQL, the returned record id). 41 remain: genuine handler tests holding a RequestArena/Response graph (contract-4), including the require_verified login test (email_not_verified) and "#80/legacy-upgrade: a hook-free epoch login over a legacy hash acquires the writer EXACTLY once", both of which drive authWithPassword through a real RequestArena — the exact production shape.
-src/api/auth_methods.zig	11	contract-4: all 11 tests drive public auth-method dispatch through env.ctx with a real RequestArena, including the two-factor gate for every primary-method resolution. Response bodies and cookies are reclaimed at request end.
+src/api/auth_methods.zig	12	contract-4: all 12 tests drive public auth-method dispatch through env.ctx with a real RequestArena, including the two-factor gate for every primary-method resolution and normalized client-failure envelopes. Response bodies and cookies are reclaimed at request end.
 src/api/two_factor.zig	1	contract-4: the integration test drives RequestCtx handlers through password login, factor completion, and refresh; response bodies, cookies, and policy graphs use the RequestArena lifetime required by those production APIs. Primitive storage and cryptography tests use the raw testing allocator.
 src/api/collections.zig	9	contract-4: all 9 tests drive the collection create/get/list/update/delete handlers through ctxFor(env, RequestArena.from(&arena), ...) — a real http.RequestCtx whose .allocator is a RequestArena, the exact production shape; the handlers allocate their whole http.Response graph (serialized collection JSON, field-error bodies) on ctx.allocator.a, reclaimed wholesale at request end.
 src/api/features.zig	4	contract-4: all 4 tests drive the GET /api/features handler through ctxFor(env, RequestArena.from(&arena), ...) — a real http.RequestCtx whose .allocator is a RequestArena; the response graph (declared flags/experiments JSON incl. override values) is allocated on ctx.allocator.a, the exact production shape.
@@ -11,7 +11,7 @@ src/api/health.zig	1	contract-4: the single test drives the health handler throu
 src/api/magic_link_consume.zig	7	contract-4: 6 tests drive the public consume GET handler through consumeCtx(...) — a real http.RequestCtx whose .allocator is a RequestArena; the handler's 302/400/404 Response graphs (Location header, session Set-Cookies, confirm bodies) live on ctx.allocator.a, the exact production shape. The 7th is setupConsumeEnv's setup-only arena around collections.create provisioning scaffolding (never touched by the SUT).
 src/api/mail_unsubscribe.zig	6	contract-4: all 6 tests drive the public post()/get() handlers through a real http.RequestCtx whose .allocator is a RequestArena (RequestArena.from(&arena)) — the exact production shape. The handler allocates its whole response graph on ctx.allocator.a (the badRequest/notFound/429 responses, the confirm-page HTML, the one_click:<list> source string, and unsubscribe.verify's owned Parts) and never individually frees it, exactly as a request-arena consumer should. Reclaimed wholesale at request end.
 src/api/oauth.zig	6	The "deleting an auth record removes its external-auth links" outlier is un-masked (seedOAuthCollection now self-frees its encrypted-secret blob + created collection; the test frees the fetched collection via Collection.deinit) and runs under std.testing.allocator. 6 remain: genuine handler tests holding a RequestArena/Response graph (contract-4), including the independently named cross-collection OAuth regression using the resolver through a real RequestCtx.
-src/api/records.zig	38	contract-4: 37 tests drive the records create/list/view/update/delete + multipart handlers through ctxFor/formCtx — a real http.RequestCtx whose .allocator is a RequestArena; each handler's http.Response graph is allocated on ctx.allocator.a and reclaimed wholesale at request end. Includes upload and durable cleanup job Ctx coverage, including renamed physical namespaces. The 38th is TestEnv.init's setup-only arena around collections.create provisioning scaffolding.
+src/api/records.zig	39	contract-4: 37 tests drive the records create/list/view/update/delete + multipart handlers through ctxFor/formCtx — a real http.RequestCtx whose .allocator is a RequestArena; each handler's http.Response graph is allocated on ctx.allocator.a and reclaimed wholesale at request end. Includes upload and durable cleanup job Ctx coverage, including renamed physical namespaces. The 38th is TestEnv.init's setup-only arena around collections.create provisioning scaffolding. The onError-after-rollback test drives real create/update/delete handlers with a RequestArena response graph.
 src/api/sessions.zig	6	contract-4: all 6 tests drive the sessions REST handlers (list/get/revoke/revoke-all, plus login via api/auth to mint tokens) through bearerCtx/login/env.ctx — real http.RequestCtx whose .allocator is a RequestArena; the {items} pages, minted tokens and cookie-bearing Response graphs live on ctx.allocator.a, the exact production shape.
 src/api/settings.zig	2	contract-4: both tests drive the settings GET/PUT/DELETE handlers through ctxFor(env, RequestArena.from(&arena), ...) — a real http.RequestCtx whose .allocator is a RequestArena; response bodies live on ctx.allocator.a, the exact production shape.
 src/api/state.zig	5	contract-4: all 5 tests drive the public GET /api/state handler through a real http.RequestCtx whose .allocator is a RequestArena (incl. the #230 cache-probe helper that takes a RequestArena directly); the resolved flags/experiments JSON is allocated on ctx.allocator.a, the exact production shape.

src/api/auth_methods.zig

@@ -154,12 +154,18 @@ fn dispatch(ctx: *http.RequestCtx, phase: DispatchPhase) anyerror!http.Response
                     // code of its own — derive one from the status, same as route_types.zig's
                     // jsonError does for typed routes.
                     const derived = codes.forStatus(f.status);
-                    // `internal` promises in the frozen registry that no detail is
-                    // leaked, so a 5xx must NOT forward the method's own message —
-                    // that text is written for operators, not for an anonymous caller.
-                    // Log it (the operator still needs it) and answer generically.
-                    if (derived == .internal) {
-                        std.log.err("auth method failed with {d}: {s}", .{ f.status, f.message });
+                    // A failure message with a named 4xx status is intentionally
+                    // client-facing, including statuses without a dedicated code
+                    // (422 -> bad_request). `internal` promises no detail is leaked:
+                    // messages attached to 5xx, non-error, or transport-unnamed statuses
+                    // (e.g. 432, which response normalization would replace with a 500)
+                    // are operator-only. Log and redact them.
+                    if (derived == .internal or !http.isErrorStatus(f.status)) {
+                        // warn under test: the test runner fails the build on any logged error.
+                        if (@import("builtin").is_test)
+                            std.log.warn("auth method failed with {d}: {s}", .{ f.status, f.message })
+                        else
+                            std.log.err("auth method failed with {d}: {s}", .{ f.status, f.message });
                         return ApiError.internal().toResponse(ctx.allocator.a);
                     }
                     return (ApiError{ .status = f.status, .code = codes.s(derived), .message = f.message }).toResponse(ctx.allocator.a);
@@ -288,6 +294,52 @@ test "every primary method resolution enters the second-factor gate" {
 // Tests
 // ---------------------------------------------------------------------------
 
+test "auth-method client failures preserve their public message without a dedicated status code" {
+    const env = try auth.TestEnv.initAuth("users");
+    defer env.deinit();
+    var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
+    defer arena.deinit();
+    const Failure = struct {
+        status: u16,
+        fn initiate(_: *anyopaque, _: *method_mod.AuthCtx) anyerror!method_mod.InitiateResult {
+            return .{};
+        }
+        fn complete(ptr: *anyopaque, _: *method_mod.AuthCtx) anyerror!method_mod.Resolution {
+            const self: *@This() = @ptrCast(@alignCast(ptr));
+            return .{ .fail = .{ .status = self.status, .message = "Choose an allowed account option." } };
+        }
+        const vt = method_mod.AuthMethod.VTable{ .initiate = @This().initiate, .complete = @This().complete };
+    };
+    var failure = Failure{ .status = 422 };
+    const methods = [_]method_mod.AuthMethod{.{ .slug = "password", .ctx = &failure, .vtable = &Failure.vt }};
+    var registry = registry_mod.Registry{ .methods = &methods };
+    env.app.auth_methods = @ptrCast(&registry);
+    const params = [_]http.Param{ .{ .key = "col", .value = "users" }, .{ .key = "method", .value = "password" } };
+    for ([_]u16{ 401, 422, 451 }) |status| {
+        failure.status = status;
+        var ctx = env.ctx(RequestArena.from(&arena), .POST, "{}", &params);
+        const response = try complete(&ctx);
+        const normalized = try @import("../server.zig").normalizeResponse(&ctx, response);
+        try std.testing.expectEqual(status, normalized.status);
+        const envelope = (try std.json.parseFromSlice(std.json.Value, arena.allocator(), normalized.body, .{})).value.object;
+        try std.testing.expectEqualStrings(if (status == 401) "unauthorized" else "bad_request", envelope.get("code").?.string);
+        try std.testing.expectEqualStrings("Choose an allowed account option.", envelope.get("message").?.string);
+        try std.testing.expectEqual(@as(i64, status), envelope.get("status").?.integer);
+        try std.testing.expectEqual(@as(usize, 0), response.cookies.len);
+    }
+    // 432 has no transport name; its message must be redacted like a 5xx, not lost to normalization.
+    for ([_]u16{ 432, 500, 503, 200 }) |status| {
+        failure.status = status;
+        var ctx = env.ctx(RequestArena.from(&arena), .POST, "{}", &params);
+        const response = try complete(&ctx);
+        const normalized = try @import("../server.zig").normalizeResponse(&ctx, response);
+        try std.testing.expectEqual(@as(u16, 500), normalized.status);
+        const envelope = (try std.json.parseFromSlice(std.json.Value, arena.allocator(), normalized.body, .{})).value.object;
+        try std.testing.expectEqualStrings("internal", envelope.get("code").?.string);
+        try std.testing.expect(std.mem.indexOf(u8, normalized.body, "Choose an allowed") == null);
+    }
+}
+
 test "auth-method dispatch: password complete succeeds + 2 cookies + onAuth fires" {
     const api_auth = @import("auth.zig");
 

src/api/error.zig

@@ -184,14 +184,15 @@ fn parseLiteralStatus(span: []const u8) ?u16 {
 // Regression guard (SP-1 Task 2 follow-up): walks every `src/**/*.zig` file at test
 // time and flags a bare `ApiError{ .status = N, .message = … }` struct-literal
 // construction that omits `.code` while `N` is a status `error_codes.forStatus` maps
-// to something OTHER than `.internal` (400/401/403/404/409/410/429 today, read live
-// off `forStatus` so this guard tracks it automatically). Such a literal silently
+// to something OTHER than `.internal` (every 4xx today: the dedicated codes for
+// 400/401/403/404/409/410/413/429 and `bad_request` for the rest, read live off
+// `forStatus` so this guard tracks it automatically). Such a literal silently
 // falls back to `ApiError`'s default `code: []const u8 = codes.s(.internal)` even
 // though a specific registered code exists for its status — the exact defect this
 // task fixed at ~30 call sites (see task-2-report.md, "Concern 2").
 //
-// Literals at 500, or any other status `forStatus` does not specifically map, are
-// NOT flagged: `internal` is the documented, correct default there.
+// Literals at 5xx or other non-4xx statuses, which `forStatus` maps to `internal`,
+// are NOT flagged: `internal` is the documented, correct default there.
 //
 // Limits (stated, not hidden): this is a textual brace-balancing scan, not a Zig
 // parser. It only understands a single `ApiError{ ... }` literal per match (nested