Skip to content

fix(rest): security/explain refuses an empty object with 400 VALIDATION_FAILED instead of 500 - #27892

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-kwainq
Oct 30, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-kwainq

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Elon · project thread
Fixes #27719

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:管理员调用访问解释接口(POST/GET /api/v1/security/explain)时,object 传空字符串现在直接回 400 VALIDATION_FAILED,不再回 500 EXPLAIN_FAILED。
  • 为什么改:这是调用方能自己改正的错误请求,却被当成服务器故障回给客户端、也按故障记日志;与上一张卡(空 userId 回 400)同一道门、同一种处理。
  • 风险与回滚:风险低——本来就失败的请求只是从 500 变成 400,合法请求一字节不变;回滚即 revert 本 PR。
  • 先看:packages/rest/src/security-routes.ts(EMPTY_KEY_REMEDIES 表与 handler 里那一个循环)、packages/rest/src/security-routes.test.ts([#27719] 一组)。
  • 两分钟验证:pnpm --filter @objectstack/rest exec vitest run --project local src/security-routes.test.ts,预期 22 个用例全部通过。
  • 需要你做的:无,席位落地。

What changes

ExplainRequestSchema.object is z.string(), so an empty object passed the schema, reached the explain engine, and the engine's plain Error('[Security] explain: request.object is required') fell to the route's catch-all 500 EXPLAIN_FAILED arm.

The route now refuses it 400 VALIDATION_FAILED, in its own envelope and before any evaluation, through the check that already refused an empty userId. That check is now one table, EMPTY_KEY_REMEDIES (module-private, no export changes), with one row per key and the remedy its refusal names. The handler loops over it once, so the userId refusal is not copied. The userId message is byte-identical to before.

Per the triage ruling, the refusal is graded at the route. packages/spec is not touched. Moving both keys into ExplainRequestSchema (min(1)) is a separate spec narrowing that this PR does not grade.

Files: packages/rest/src/security-routes.ts, packages/rest/src/security-routes.test.ts, .changeset/27719-explain-empty-object.md (@objectstack/rest patch).

Tests

New [#27719] block in packages/rest/src/security-routes.test.ts. Its fake service throws the engine's own plain error for an empty object:

  • POST with object: "" gets 400, success: false, error.code VALIDATION_FAILED, and no verdict. The engine is never called.
  • GET ?object= is refused the same way.
  • A request that names an object is still explained, and the engine gets exactly the request it was sent (POST with update, and GET defaulting to read).

The [#27442] empty-userId block runs unchanged and passes.

Run on head 4f5150d8 (objectstack-ai/objectstack):

Red first, on the unfixed route: 2 failed | 20 passed
pnpm --filter @objectstack/rest exec vitest run --project local --maxWorkers=2 src/security-routes.test.ts
     × POST object "" is refused 400 VALIDATION_FAILED in the route envelope, and the engine is never called
     × GET ?object= is refused the same way
AssertionError: expected 500 to be 400 // Object.is equality
      Tests  2 failed | 20 passed (22)
Green after the fix: 22/22; the whole package 347 files / 6737 tests; typecheck exit 0
src/security-routes.test.ts      Tests  22 passed (22)
pnpm --filter @objectstack/rest test        Test Files 347 passed (347) · Tests 6737 passed | 332 skipped (7069) · VERDICT command-exit 0
pnpm --filter @objectstack/rest typecheck   VERDICT command-exit 0 (check:test-typecheck: 0 files / 0 errors)
Ablation: delete the object row from EMPTY_KEY_REMEDIES, and both new refusal pins go red; restored byte-identical
ablation-replace: ok mutation landed: anchor 1 -> 0, blob 73f00e095d32 -> 88505ad43e41
     × POST object "" is refused 400 VALIDATION_FAILED in the route envelope, and the engine is never called
     × GET ?object= is refused the same way
      Tests  2 failed | 20 passed (22)
ablation-replace: ok restored: blob == HEAD (73f00e095d32) and `git diff HEAD` is empty

The route test imports ./rest-server relatively, so it runs against src/ and no dist rebuild was needed for this leg.

End to end: real showcase boot as the seed admin, through a throwaway dogfood probe that was not committed
main 9ae6384e:
POST object="" operation=read   status=500 {"success":false,"error":{"code":"EXPLAIN_FAILED","message":"[Security] explain: request.object is required"}}
GET ?object=&operation=read     status=500 (same body)
this branch 4f5150d8 (rest rebuilt):
POST object="" operation=read   status=400 {"success":false,"error":{"code":"VALIDATION_FAILED","message":"Invalid explain request — object is empty. Give the name of the object whose access to explain."}}
GET ?object=&operation=read     status=400 (same body)
controls, unchanged on both: POST object=showcase_task -> 200 allowed:true · POST userId="" -> 400 VALIDATION_FAILED (the userId message is byte-identical)

Acceptance notes

  • The empty object refusal names its remedy ("Give the name of the object whose access to explain."). The test does not pin that wording: it asserts the code, the status, and that the engine was never called.
  • A missing object (the key absent) is still refused by the request schema's own 400. This PR leaves it alone.

Verification

Local subset on head 4f5150d8: the gates derived by node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack from 3 changed paths (+93 / -10), run one after another with each exit code captured before any pipe.

60 derived, 60 run, 60 exit 0; --ran reconciliation: 0 NOT-MEASURED, 0 UNRUN
  • pnpm check:dual-build-cjs-loads first exited 3 (PREREQUISITE NOT MET: 8 unrelated packages had no dist/). After building them it exited 0: 113 entry points across 70 packages load. The reconciliation records that rerun.
  • --ran with :: exit N per line: "60 derived famil(ies) accounted for — 60 run, 0 NOT-MEASURED (a DERIVED zero — all 60 recorded an exit code and none of them is 3)".
  • npx eslint --no-inline-config on both touched source files: exit 0.
  • Not run locally, and left to CI: test:repo, the full-tree pnpm lint, and the type-check lanes and path-scheduled jobs the derivation lists outside its total. No dogfood battery: the title has no ! and the clause has no (narrowing) arm.

🤖 Generated with Claude Code

https://claude.ai/code/session_016WV4d1Np3hU7RQnA13YZcL

…ON_FAILED

An empty `object` passed `ExplainRequestSchema` (`z.string()`), reached the
explain engine, and its plain Error fell to the route's 500 EXPLAIN_FAILED
arm. The door now refuses it 400 VALIDATION_FAILED in its own envelope
before any evaluation, through the same check that refuses an empty
`userId`: one table of keys, each with its remedy.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4d1Np3hU7RQnA13YZcL
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 3 documentable anchor(s).

23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 64e5efde9e86f3f823b861da289b547310b041ab.

⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: VALIDATION_FAILED (literal, 37 pages)
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 19 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 64e5efde9e86f3f823b861da289b547310b041ab → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2522f5d6e1c4ba2ba2a53fb6cddc7acea7c65675 — the merge of head 4f5150d8d1f7d5a2230cab4ae7a1460cc81ff5dd into base 64e5efde9e86f3f823b861da289b547310b041ab, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2522f5d6e1c4ba2ba2a53fb6cddc7acea7c65675 && git checkout 2522f5d6e1c4ba2ba2a53fb6cddc7acea7c65675
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 64e5efde9e86f3f823b861da289b547310b041ab 4f5150d8d1f7d5a2230cab4ae7a1460cc81ff5dd && git checkout -B drift-repro 64e5efde9e86f3f823b861da289b547310b041ab && git merge --no-ff 4f5150d8d1f7d5a2230cab4ae7a1460cc81ff5dd

node scripts/docs-audit/affected-docs.mjs --json 64e5efde9e86f3f823b861da289b547310b041ab

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 64e5efde9e86f3f823b861da289b547310b041ab → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-elon-musk · session_016WV4d1Np3hU7RQnA13YZcL

维护者速读

席位意见:ACCEPT。

席位核对

Head-sha: 4f5150d8d1f7d5a2230cab4ae7a1460cc81ff5dd

  • Shape: one module-private table of empty-key remedies, looped once in the handler before ExplainRequestSchema parses; an explicit "" for object or userId (POST body or ?key= on GET) answers 400 VALIDATION_FAILED in the route's own envelope before the engine runs. Nothing exported; the userId sentence is byte-identical. Changeset patch @objectstack/rest, Clause-②: no (a request that already failed now fails with the right status; nothing accepted today is refused), Metadata-protocol: none. check-governed-merges --pr 27892: NOT governed, 103 changed lines.
  • Premise measured: on main, POST and GET with object: "" answered 500 EXPLAIN_FAILED on a real showcase boot (throwaway probe, never committed); after the fix both answer 400, controls unchanged. Red first 2 failed (expected 500 to be 400); ablating the object row turns the same 2 red, restore verified; @objectstack/rest 347 files / 6737 tests (seat report 6513745303 on security: POST /api/v1/security/explain with an empty object answers 500 EXPLAIN_FAILED, a malformed request answered as a server fault #27719).
  • Gate check-runs on the head: 48 runs, 35 success, 13 skipped, 0 failure/pending; mergeable_state clean.
  • Contract review: not owed (no contract face: no packages/spec, no narrowing, no new public surface).
  • Accepted deviations: none. Read from code, not filed: a whitespace-only object / userId still reaches the engine; the ExplainRequestSchema min(1) spec narrowing stays ungraded per triage 6508146400.
  • Landing: under the maintainer's landing grant for this seat term (seat 4 thread, 2026-10-29 22:04Z): pr_ready + automerge_enable through the relay.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 30, 2026 03:10
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit 92493e3 Oct 30, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-kwainq branch October 30, 2026 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: POST /api/v1/security/explain with an empty object answers 500 EXPLAIN_FAILED, a malformed request answered as a server fault

1 participant