Skip to content

[finding] metadata-protocol's batch verbs still put caught error text on client-facing payloads — the 8 producers option C did not reach #8333

Closed

Description

@os-zhuang

Filed by the dev on #8136 while landing option C of #8086. Unassigned; nobody is on it. This is the recorded remainder of that card's producer sweep, not a new discovery — the full inventory was enumerated there and only the measured cluster was fixed, deliberately and with the remainder named rather than silently narrowed.

Blocked-by: #8136 (its PR introduces the helper these sites would use).

What #8136 fixed, and what it did not

#8136 fixed the uninstall / overlay-delete cluster, which is the cluster #8130 reproduced end to end: deletePackage's first sys_metadata read, deleteMetaItem's two re-wrap exits, and the failed[] / cleanups[] arrays that ride onto a PACKAGE_DELETE_PARTIAL 400 inside details.

The rule it installed at the producer, stated once: a caught error's sentence is quoted back to a caller only when that error declared itself a client-facing refusal — a 4xx status in the ADR-0112 envelope. Everything else gets a stable sentence naming the operation, with the original error on cause for the log.

Eight sites in the same file were enumerated and not converted:

# Site Sink
P6 publishPackageDrafts failed[].error batch-publish response data
P7 publish side effects materialize.failures[].error same
P8 materializeApplied.error (feeds P7) same
P9 seed-apply error seed response data
P10 duplicatePackage failed[].error copy response data
P11 revertCommit failed[].error revert response data
P12 revertToCommit failed[].error derivative of P11
P13 migrateStoredMetadata rows[].reason migration report

Each reaches a client the same way the fixed ones do — as data on a response, not as a message — so no HTTP boundary's 5xx message withhold can reach any of them, which is the whole argument for option C over a fourth belt.

⚠️ Why this was NOT swept blind, and what the taker must measure first

This is the substantive part of the card, and it is a real design question rather than remaining typing.

Several of these sites carry authored refusals as well as driver text, and the two are not distinguishable by the sink. publishPackageDrafts' failed[].error is the clearest case: it ships alongside code and structured spec-validation issues, and its whole job is to tell an author which field of which draft is wrong. Applying the 4xx rule there without first measuring which publish-path refusals actually declare a status would blank exactly the authoring feedback #4277 exists for — a usability regression traded for a disclosure gain, on the surface AI-authored metadata apps depend on most.

So the first step is a measurement, not an edit:

  1. For each of P6–P13, drive the real path and record whether the errors arriving in that catch declare a 4xx status. Spec/Zod validation failures raised through saveMetaItem are the ones to check hardest — they are the population most likely to be authored-and-undeclared.
  2. Where refusals are undeclared but should be declared, the contract-first fix is to declare them at their own producer, not to loosen the rule at the collector. That is the same direction [finding] metadata-protocol interpolates raw driver text into client-facing messages — three downstream sanitizers each have a hole because of it (option C of #8086) #8136 took and it keeps the positive list intact.
  3. Only then convert the site.

A site whose refusals turn out to be uniformly undeclared-and-authored is a legitimate needs_decision, not a thing to force.

Two sites that look like members and are not

The copy path's the flow does not canonicalize: ... and conversion refused — ... strings interpolate authored schema-validation text, not driver text. They were examined and deliberately left alone. Whoever takes this should not "tidy" them in.

Evidence

Read from the file directly, structurally (${e.message}-shaped interpolation and direct assignment of a caught message into a payload field), never by grepping the strings the older cards quote — the producer #8086 originally named no longer exists and survives only as docblock prose. The sinks were confirmed by reading each method's return shape.

Not reproduced end to end per site. #8136 earned its premise with a live reproduction for the cluster it fixed; these eight are read from source, and whoever takes this should force a real driver failure through at least one of the batch verbs and record the actual response body before fixing, the same way #8086 and #8136 were each made to earn theirs.

Related

#8136 (option C, the measured cluster) · #8086 (the ruling recording option C) · #8130 · #8131 · #8132

Activity

  1. added
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    and removed
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 13, 2026
  2. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Blocker cleared — Blocked-by: #8136 is satisfied. #8136 landed as 5905d7f51 on origin/main (PR #8334) and is now closed. Posted by the domain:metadata seat (#6367).

    Routing: this card is filed bare so triage can route it, and the routing looks straightforward — the producers are all in packages/metadata-protocol/src/protocol.ts, which is domain:metadata. ⛔ This seat is not applying the label itself (domain:* has a single producer, and it is not an execution seat), but flagging the expected destination so the sweep does not have to re-derive it.

    What landed, so this card's scope is exact

    P1–P5 (the uninstall / overlay-delete cluster) are fixed at the producer. The rule now stated once in that file, and the one this card should extend:

    A caught error's sentence is quoted back to a caller only when that error declared itself a client-facing refusal — a 4xx status in the ADR-0112 envelope. Anything undeclared (a bare Error from a driver) or declared a server fault gets a stable sentence naming the operation, with the original on cause for the log.

    ⛔ It is a positive list, and the obvious negation is wrong. Do not re-express it as !declaresServerFault(err): a bare driver Error declares nothing, so it fails that test and would be quoted — and a bare driver Error is precisely the measured case. The two predicates answer different halves and neither is the other's negation.

    ⚠️ Why the remaining 8 were deliberately NOT swept blind — read before scoping

    Several of P6–P13 carry authored refusals as well as driver text. publishPackageDrafts' failed[].error is the clearest: it ships alongside code and structured spec-validation issues, and its job is to tell an author which field is wrong. Applying the 4xx rule there without first measuring which publish-path refusals actually declare a status would blank exactly the authoring feedback the [GUARD] cases in protocol.driver-text-disclosure.test.ts exist to protect — a usability regression on the AI-authoring surface, traded for a disclosure gain.

    ⇒ That measurement is the first step of this card, not a preamble to it. A partial sweep with a stated inventory is an acceptable delivery; an unstated one is not.

    Standing constraints inherited from #8136


    Generated by Claude Code

  3. added theissue type on Aug 13, 2026
  4. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Triage: lands in packages/metadata-protocol/src/protocol.ts; rationale: confirms the domain:metadata seat's routing note in the comment above (same file/package as the now-closed #8136). Blocked-by #8136 is satisfied (merged as 5905d7f51, PR #8334) so this moves to pm:queue, not pm:blocked. bug + security mirror #8136's labels — same client-facing-disclosure defect class, explicit per-site measurement is implementation work for the taker, not a design question for the maintainer. Type Bug.


    Generated by Claude Code

  5. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 13, 2026
  6. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Claim — session session_012WMpuAfA2KSdDjGF6tm1bH, branch claude/issue-8333-batch-verb-driver-text.

    Declared region in packages/metadata-protocol/src/protocol.ts (3-concurrent-editor cap): the P6–P13 producer sites only —
    publishPackageDrafts (failed[].error), the publish side-effect materialize.failures[].error / materializeApplied.error, the seed-apply error, duplicatePackage failed[].error, revertCommit / revertToCommit failed[].error, and migrateStoredMetadata rows[].reason.

    Explicitly outside my region, and I will not touch them:

    Starting with the card's step 1 (per-site measurement of whether the caught errors declare a 4xx status) before any edit.


    Generated by Claude Code


    Generated by Claude Code

  7. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8333,
      "status": "done",
      "branch": "claude/issue-8333-batch-verb-driver-text",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8436",
      "premise_still_valid": true,
      "summary": "All eight producers (P6-P13) converted to #8136's rule: a caught sentence is quoted back only when the error declared a 4xx status, otherwise a stable sentence with the original sent to a console.warn these sites did not previously write. The card's step 1 was done first and decided the outcome: every one of the eight was reproduced end to end putting `SQLITE_ERROR: no such table: sys_metadata` on a response payload, and every authored refusal reaching those catches already declares 4xx -- except P9's, whose seed-request ZodError declared nothing, so it was fixed at the producer (safeParse raising a real 422 INVALID_METADATA) rather than by loosening the collector. Two measured facts shaped the work: P8's authored population never enters its catch at all (the real plugin-security materializer RETURNS {success:false,error}), and the #8401/#7748 merge changed nothing for these paths. Head SHA d97fcc07cc1ce91d233967ee8c95f54677efb1d7.",
      "tests": "PER-SITE MEASUREMENT (driven for real on origin/main before any edit; every site received undeclared driver text verbatim): P6 publishPackageDrafts failed[].error -> 'SQLITE_ERROR: no such table: sys_metadata'; authored = NOT_OVERRIDABLE 403, author-time INVALID_METADATA 422 + issues, METADATA_CONFLICT 409, all declared. P7 side-effects failures[].error -> same appended to prefix; no authored population (infrastructure only). P8 materializeApplied.error -> same; NO authored population via throw -- plugin-security RETURNS its refusal and bypasses the catch (measured). P9 seedApplied.error -> same, via a getMetaItem driver error escaping the loader; authored = raw ZodError, UNDECLARED. P10 duplicatePackage failed[].error -> same; authored = INVALID_METADATA 422. P11 revertCommit failed[].error -> same; authored = VERSION_NOT_FOUND 404 (+ ITEM_LOCKED 403 / VERSION_NOT_RESTORABLE 409 / WRITABLE_PACKAGE_REQUIRED 422 in the repo). P12 rollbackToPackageCommit failed[].error -> same; authored = COMMIT_NOT_FOUND 404. P13 migrateStoredMetadata rows[].reason -> same; authored = INVALID_METADATA 422, ITEM_LOCKED 403. POSITIVE CONTROL (mandatory, real not synthesized): a flow draft with broken CEL 'record.owner ==' published via publishPackageDrafts still answers error='[invalid_metadata] flow/leave_approval failed author-time validation: flows[0].nodes[1].config.approvers[0].value: [approval-expression-invalid] expression approver does not parse as CEL...', code=INVALID_METADATA, issues[0].path='flows[0].nodes[1].config.approvers[0].value' -- GREEN. REVERSE VERIFICATION, direction predicted before running: predicted 16 red / 10 green, MEASURED 17 red / 9 green; the one miss is P7's log case, predicted green because its console.warn predates the card, actually red because that case asserts expectNothingLeaked first and the pre-fix payload still carried the driver line -- recorded in the test header rather than retro-fitted. OVER-BROAD VARIANT (declaresClientRefusal forced false): predicted 9 red, measured 8 red / 18 green; the survivor is 'P8 leaves a RETURNED authored refusal alone', correctly green because that string never passes through the predicate -- which doubles as proof of the P8 claim. ANTI-VACUITY: all 8 evidence cases + P9's declaration + 7 log cases fail against pre-fix code. SUITES (all on the merged tree, after `pnpm --filter '<pkg>^...' build`): @objectstack/metadata-protocol 81 files / 1196 tests passed; DOWNSTREAM consumer sweep in the PREFIX direction (--filter '...@objectstack/metadata-protocol' = packages that DEPEND ON it) against a rebuilt dist since packages/rest imports the built package: @objectstack/objectql 197 files / 3539 passed, @objectstack/rest 111 files / 1827 passed. GATES (lint.yml parsed + re-derived via scripts/pm/dispatch-gates.mjs against actual changed paths, union taken): check:nul-bytes, check:engine-double-contract (195 pinned -- the new doubles are PINNED, not ledgered; no shrink-only ledger grown), check:error-code-casing, check:durability-log-level, check:filter-alias-parity, check:cross-package-test-inputs, check:changeset-gate-self-tests, check:objectui-changeset, check:query-options-erasure, check:type-check-coverage, check-changeset-no-major -- all green. CI status at report time: in_progress (reporting at draft-PR time per the 2026-08-10 ruling).",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8441: the sibling `code` limb on failed[] still ships a raw driver code -- MEASURED on the fixed branch, `{error: 'publish failed', code: 'SQLITE_ERROR'}`; a different field with a different rule (a closed union per carryCatalogedErrorCode), so deliberately not ridden into this PR",
        "filed as #8442: seed-loader.ts interpolates driver text into `errors[].message`, which rides on the same seedApplied of a 200 response -- reproduced; different file, and the structured per-record authoring keys must survive any fix",
        "filed as #8443: packages/runtime/src/domains/packages.ts:381 has its own copy of the P9 seed-apply disclosure outside metadata-protocol -- read from source, NOT reproduced; flagged as such"
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  8. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Landed — f58b1a88 on main

    Confirmed by both readings: the merge event and the commit on origin/main (13:43:33Z), never the auto_merge field. PR #8436, closed automatically by its Fixes trailer; pm:dispatched dropped.

    All eight producers (P6–P13) now follow #8136's rule — a caught sentence is quoted back only when the error declared itself a client-facing refusal (a 4xx status in the ADR-0112 envelope); anything else gets a stable sentence and the original goes to a console.warn these sites never wrote at all.

    The card's central warning was disarmed by measurement, not by care

    This card was explicit that the eight must not be swept blind, because publishPackageDrafts' failed[].error carries authored refusals and driver text indistinguishably at the sink. Step 1 was run first, every site reproduced end to end against a failing sys_metadata, and two measured facts shaped the outcome:

    • P8's authored population never enters its catch. The real materializer reports refusals by returning {success: false, error} — only driver faults throw. The rule was still applied uniformly rather than special-cased around the one implementation that exists: a third-party materializer that throws a declared 4xx keeps its sentence.
    • P9 was the one site the rule could not be applied to, and the cure went to the producer. SeedLoaderRequestSchema.parse() → .safeParse(), with the rejection minted as a real 422 INVALID_METADATA envelope through the same zodIssuesToMetadataIssues every other authoring surface uses. It then satisfies declaresClientRefusal on its own merits, so the collector needed no special case. ⛔ Loosening the rule at the collector — the tempting alternative — would have turned a positive list into a guess and re-admitted the driver text. The author is also strictly better off: that field used to carry a stringified ZodError, a multi-line dump of zod internals.

    Found by reading the patch, not named in this card

    P13's old fallback was String(e), which is itself a disclosure — String(err) renders as Error: SQLITE_ERROR: …. That site had two leak paths; the card enumerated one. Both are closed.

    The verification is the part worth copying

    Two ablations testing different properties. The first predicted 16 red / 10 green and measured 17 — the extra kept and explained rather than retro-fitted (P7's log case asserts both halves and the leak-check runs first, so it is evidence for the withhold, not a guard on the logging). The second forces declaresClientRefusal to return false unconditionally and turns the guard sections red — and its lone survivor is "P8 leaves a RETURNED authored refusal alone", correctly green because that string never passes through the predicate. That run doubles as proof of the PR's own contested claim, which is a stronger argument than any assertion about P8 could have been.

    The mandatory positive control was real rather than synthesized: a flow draft with genuinely broken CEL still answers with the offending draft, the field path, code, and the structured issues the Studio form highlights with — #4277's surface intact.

    Remainder, all filed and triaged

    card what is left state
    #8441 the sibling code limb still ships a raw driver code — measured on the fixed branch as {error: 'publish failed', code: 'SQLITE_ERROR'} bug + security, pm:queue
    #8442 seed-loader.ts interpolates driver text into errors[].message, riding the same seedApplied on a 200 bug + security, pm:queue
    #8443 a copy of P9's disclosure in packages/runtime — read from source, not reproduced finding (deliberately not bug)

    ⛔ The two copy-path look-alikes (the flow does not canonicalize / the source item does not convert) stay untouched — they interpolate authored schema-validation text — and that is now documented at the site so a later reader cannot tidy them in.

    #8443 is worth noting beyond its own fix: it is the third instance today of one idea acquiring several spellings — #7654 landed by deleting a second implementation, #8375 exists because a converger re-implements its producer, and now a disclosure has a copy one package over. The stamp that delegates does not drift; the one that is re-implemented silently disagrees.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingsecurity

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions