Repository navigation
fix(plugin-security)!: security explain refuses a userId that no sys_user record has with 404 RECORD_NOT_FOUND - #26801
Conversation
…ow has with 404 RECORD_NOT_FOUND Explaining another user built the principal through the authorization resolver, which synthesises the baseline anchor and sets for any id, so an unknown id was answered 200 allowed: true. After the caller is admitted to explain others, and before the context is built, the explained id is now read through the delegator path's own sys_user existence read (readSysUserRow, shared with resolveDelegatorContext) and refused 404 RECORD_NOT_FOUND naming it. An anonymous caller still gets 401 and a caller who may not explain others still gets their 403 before any existence read. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FEkQ5kdwdgQcbyZc22QQYf
…hangeset Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FEkQ5kdwdgQcbyZc22QQYf
…the same three plugin-security files) Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FEkQ5kdwdgQcbyZc22QQYf
📓 Docs Drift CheckThis PR changes 1 package(s): 41 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 7 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1bbc835ec952737ea4d23dfdcc5e731b845e37f5 && git checkout 1bbc835ec952737ea4d23dfdcc5e731b845e37f5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8b276385e6f1e2b94a4061160c148cfa56f81588 6c7cf979dacf820b0dbd0b5385f71669099e9af7 && git checkout -B drift-repro 8b276385e6f1e2b94a4061160c148cfa56f81588 && git merge --no-ff 6c7cf979dacf820b0dbd0b5385f71669099e9af7
node scripts/docs-audit/affected-docs.mjs --json 8b276385e6f1e2b94a4061160c148cfa56f81588
|
席位核对ACCEPT · The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.
Generated by Claude Code |
Requested by Leehom · project thread
Fixes #26641
Clause-②: no (narrowing) —
POST/GET /security/explainand the in-processexplain()refuse a request naming another user whose id nosys_userrecord has, which they answered with a verdict before. Nothing is widened.Metadata-protocol: none
Skills-docs: updated here —
content/docs/permissions/explain.mdx(the refusal and its place after the 403); noskills/**file teaches the explain door'suserId维护者速读
RECORD_NOT_FOUND并点名该 id,不再回 200allowed: true。security-plugin-explain-access.ts(新增的存在性检查及其位置)、explain-engine.ts(与委托人路径共用的readSysUserRow)、errors.ts(ExplainUserNotFoundError)。pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 src/security-plugin.test.ts -t 26641,预期 3 条全绿。What changes
explainAccessForCaller(security-plugin-explain-access.ts) now reads the explained id throughreadSysUserRowafter the caller is admitted to explain someone else and beforebuildContextForUser. No row ⇒ExplainUserNotFoundError:code: 'RECORD_NOT_FOUND',status/statusCode404,userId, and a message naming the id that says no principal was evaluated.readSysUserRow(explain-engine.ts) is thesys_userlookupresolveDelegatorContextalready made, lifted out unchanged (system context,findOneby id). The delegator path calls it inside its existingtryand still reads a failed read asmissing. The explain path does not catch: a store fault is not absence, so it propagates as a fault instead of being answered "this user does not exist".RECORD_NOT_FOUND/ 404 is the standard catalog's pair ("Record with given ID not found",errors.zod.ts), the same pairDetailRecordNotFoundErrorin this package uses andGET /data/sys_user/IDanswers. No ledger row, no spec change, no new code. The REST door serves it through its existing classified-refusal arm (rest:POST /api/v1/security/explainanswers a service refusal carryingINVALID_FILTER/ 400 as500 EXPLAIN_FAILED— the route's catch maps only PERMISSION_DENIED and OBJECT_NOT_FOUND #20603): a declared 4xxcode+statuskeeps its own answer. No REST source changes.manage_users, organization administration of the user's organization, or a covering delegated scope ⇒ the same 403PERMISSION_DENIEDfor an unknown id as for an existing user, and no existence read is made. An organization administrator or delegated administrator naming an unknown id therefore gets the 403 (no membership or scope covers nobody); amanage_usersholder or a system caller gets the 404.userId, or your own id) and explaining a user who exists.org-admin-assignment-boundary.posture.test.tsexplainedu_target, a member with nosys_userrow (the boot leavessys_userunprovisioned for its write cases), so the read hit "no such table" and the explanation now propagates that fault. Disposition: add the declaration: that one case provisionssys_userand insertsu_targetbefore it explains; its assertions are untouched.content/docs/permissions/explain.mdxnames the refusal under "Another user".Why
The QA run #26639 (observation 4) measured it twice on fresh showcase boots: the seed admin's
POST /api/v1/security/explainwith{"object":"showcase_task","operation":"read","userId":"no_such_user_qa_123"}answered200 allowed: true, principalpositions: ["everyone"],permissionSets: ["showcase_member_default", "member_default"], whileGET /data/sys_user/no_such_user_qa_123answered 404. Triage6441402728ruled: answer 404 for an unknown user, and pin it. The delegator path already states the rule this applies (resolveDelegatorContext: "Thesys_userexistence check is the only correct fail-closed point").Tests
packages/qa/dogfood/test/explain-unknown-user.dogfood.test.ts, new, 5 cases):[control]the data door's 404 for the same id;[unknown]the seed admin is refused 404RECORD_NOT_FOUNDnaming the id, forreadandcreate, with no verdict in the body;[anonymous]401;[unprivileged]a plain member gets the same 403PERMISSION_DENIEDfor the unknown id as for the admin who exists;[existing]the admin explaining the member, and the member explaining themselves (nouserId, and their own), answer 200 with that principal.security-plugin.test.ts, 3 cases in the ADR-0090 D6/D12 harness): themanage_userscaller's refusal envelope (code,status,statusCode,userId, the id in the message) forreadandcreate; a plain caller and a delegated admin both getPermissionDeniedErrorandsys_useris never asked about the unknown id; an existing user and a self-explanation by an id with no row still answer.security-explain-envelope.test.ts, 1 case): a thrownRECORD_NOT_FOUND/ 404 leaves both transports as the nested 404RECORD_NOT_FOUNDenvelope, not the 500.Acceptance notes
AccessExplainPanel.tsx(ui/packages/app-shell) is the onlyui/**caller that sendsuserId; the grid and detail verdict hooks (useRecordCrudVerdicts,useRecordEditable) explain the signed-in user only and never name another id. The panel picks the user from a record picker, so an unknown id reaches it only for a user deleted after it was picked. Its non-403 branch readsdata.message || data.error; on the server's nested envelopeerroris an object, and rendering it unmounts the panel. Measured in a throwaway render probe over the panel's own test harness (deleted, not committed): a 404 nested envelope ⇒document.bodyis an emptydiv, no alert; the control, a flat{ message }404 body ⇒ the alert "Explain failed — Unknown object x". This is NOT new with this PR: every non-403 refusal the door already sends (400VALIDATION_FAILED, 404OBJECT_NOT_FOUNDfor a mistyped object in the free-text input, 400INVALID_FILTER, 500) takes the same branch. Reported to the seat as a finding; not fixed here.errors.tsadds a class afterExplainObjectNotFoundError(this PR adds one before it),explain-engine.tsat the deps interface and record attribution (this PR atresolveDelegatorContext),security-plugin-explain-access.tsat the imports, host interface and the sharing block (this PR inside the explain-another-user branch). security: the security plugin does not pick up some runtime metadata changes until a restart (detail withheld pending maintainer) #26698 (same package, runtime metadata) has no PR yet.Verification
Head
6c7cf979da(the fix, thenorigin/mainmerged in: #26337 landed in the same three source files mid-run, merge clean). Every reading below was taken at this head unless it says otherwise; the first head39a455551f(before the merge) was measured the same way and was green throughout (full dogfood 303 files / 2510 tests).39a455551f; the fix committed first;scripts/ablation-replace.mjsturned the check off behind a never-set env marker; plugin-security rebuilt;ablation-dist-preflight.mjssaw the marker indist/index.jsanddist/index.mjs): the dogfood[unknown]case red with exactly the card's answer,expected 200 to be 404, body"allowed":true … "positions":["everyone"],"permissionSets":["showcase_member_default","member_default"]; the service case red (expected null to match object). The other 4 dogfood cases and 2 service cases stayed green. Restored: blob equals HEAD,git diff HEADempty, rebuilt, preflight--absent✓, tree clean.pnpm --filter @objectstack/plugin-security test: 231 files, 5032 passed, 45 skipped, exit 0.pnpm --filter @objectstack/rest test: 326 files, 6499 passed, 326 skipped, exit 0. Before the fixture triage, the plugin-security suite was red in exactly the 3 posture cases named above (and nowhere else).dispatch-gates.mjs --commandson this diff derived 95 families; all 95 ran and exit 0 at this head (--ranreconciliation: 95 derived, 95 run, 0 NOT-MEASURED, 0 UNRUN). At the first head, three first exited non-zero for reasons outside the diff and were re-run green:check-comment-mask-corpus(exit 1: my throwaway UI probe file was deleted mid-sweep),check:skill-examplesandcheck:dual-build-cjs-loads(exit 3, PREREQUISITE NOT MET: spec declarations and seven unrelated packages unbuilt; built, then green). Also green:eslint --no-inline-configon the 7 touched code files,typecheckandcheck:test-typecheckfor plugin-security and rest, dogfoodtypecheck,check-platform-checklist --changed-from origin/main,check-nul-bytes.(narrowing)arm):pnpm --filter @objectstack/dogfood testunder the verify lock, afterpnpm --filter '@objectstack/dogfood^...' build: VERDICT command-exit 0, Test Files 303 passed / 3 skipped (306), Tests 2512 passed / 20 skipped (2532), no failure.Human Minimum
packages/specholds no runtime logic. (spec untouched)pnpm lintpasses (eslint on the touched files).main, one pull request.packages/spectouched: n/a.content/docs/references/,packages/*/CHANGELOG.mdorcontent/docs/releases/.Generated by Claude Code