feat(plugin-security): a rank-and-file member may edit their OWN sys_user row - #15108
Conversation
📓 Docs Drift CheckThis PR changes 2 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 20 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 e43ce297541f01a7e4fbefc699a70b5196988aa3 && git checkout e43ce297541f01a7e4fbefc699a70b5196988aa3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4428dd5756750939e2fd503e431c34c1cb83a19c d845d564ec7bc7c8184ae8e4bec1459643a655c9 && git checkout -B drift-repro 4428dd5756750939e2fd503e431c34c1cb83a19c && git merge --no-ff d845d564ec7bc7c8184ae8e4bec1459643a655c9
node scripts/docs-audit/affected-docs.mjs --json 4428dd5756750939e2fd503e431c34c1cb83a19c
|
Fixes #14959
⛔ Do not arm this PR for merge before the ADR PR lands. The decision this implements amends ADR-0092 D5,
docs/adr/**is governed and the maintainer merges it by hand, and the amendment ships as its own PR: #15109. This PR is draft and unarmed; the seat lands them in that order.needs:contract-reviewcarrier label applied (clause ②: permission-set defaults move). ⛔ Not to be cleared here — the seat clears it after review.The ruling
Maintainer ruling 2026-09-03, decision batch #22, verbatim and untranslated as adopted:
A rank-and-file member may edit their own
sys_userrow on the generic data path, bounded on two axes that already exist.The gap this closes, restated as measured
The 2026-09-03 ruling on #14787 (landed as PR #14958) admitted
localeto the ADR-0092 D2 column whitelist — it opened which columns a permitted actor may touch. It did not open who, and ADR-0092 D5 kept that with the permission layer, wheremember_defaultdeniedallowEditonsys_user. So a member'sPATCH /api/v1/data/sys_user/selfwas refused by the object gate, before the column guard was ever consulted, andsys_user.localeshipped as a user-stated preference only a platform administrator could set.Verified at the tree rather than assumed, per the dispatch:
SYS_USER_PROFILE_EDIT_FIELDSonorigin/mainalready holds{name, image, locale}andsys_user.localealready carries noreadonly. The column half is in place; only the route was missing.What changed
packages/plugins/plugin-security/src/objects/default-permission-sets.ts— two lines of behaviour, in the shapesys_api_keyhas shipped since #8053:member_defaultgains an explicitsys_userentry:allowRead/allowEdittrue,allowCreate/allowDeletefalse. Being explicit is also what makes it survivekernel:ready—applyManagedWriteDeniesinjects its deny only for managed objects a set does not already name.sys_user_selfRLS carve-out (id == current_user.id) widens fromselecttoall, so it reaches the by-id write pre-image check.sys_user_org_members— the org-peer visibility policy — deliberately staysselect-only. RLS policies OR-combine, so widening it would have composed "my id OR every user id in my organization" and handed every member their colleagues' profile rows. That is pinned in three separate places, because the two lines are 100+ apart in the source.packages/plugins/plugin-auth/src/sys-user-writable-fields.ts— comment only. Its doc block asserted thatmember_defaultstill deniesallowEditonsys_user; this change makes that false, so it is corrected in the same diff.The pins, and why each names a layer
Three layers can refuse this write, in order: the CRUD object gate, the row scope (the by-id write pre-image check), and ADR-0092 D2's identity write guard. Layer 1 shadows the other two — before this change all four of the ruling's cases were refused by the object gate, so "another member's row is refused" and "a non-whitelisted column is refused by the guard" were both green while neither mechanism had run.
packages/plugins/plugin-auth/src/sys-user-self-service-route.test.tstherefore reports which layer answered, established mechanically rather than inferred: the middleware throwing withql.findOnenever called dates the refusal to the object gate (the pre-image re-read is the first engine call past the CRUD check); throwing withfindOnecalled is the row scope; passing and then having the guard hook throw is the guard. It drives the realSecurityPluginmiddleware over the real shipped permission sets and the realSysUserschema, and answersfindOneby evaluating the filter the middleware actually composed against a two-row fixture, so the row scope is a measurement and not a stub.localeupdate is admitted end to end, and the value survives to the payload (a "success" that stripped the column would be a silent no-op at the driver).refusedBy === 'row-scope', and the composed pre-image filter is asserted verbatim as the caller's id.name/imageare admitted, and the ADR-0092 D6 session refresh is observed: better-auth's cached{session, user}snapshot is re-written at the same key with the new value, and the session survives (rewrite, not delete). A companion pin records thatlocalecorrectly does not touch the snapshot — better-auth carries no such field, so mirroring it would manufacture an incoherence rather than repair one.emailgets past the object gate and past the row scope (the pre-image read ran and succeeded) and is stopped by the guard:refusedBy === 'identity-guard',code: 'PERMISSION_DENIED',status: 403, message naming the field and the editable set.Plus: the composed write filter asserted verbatim; the org-peer scope proved absent from it;
insert/deletestill refused at the object gate (which is also the file's positive control for that verdict, so PIN 2 and PIN 4 can fail for the reason they are written to catch).Ablation — predicted before measuring, two legs
Predictions were written down before either leg ran. No rebuild was needed and none happened: plugin-auth's vitest config aliases the plugin-security specifier to
src/index.ts, anddist/index.jswas hash-compared before and after each leg and was byte-identical, so the redness came from source.Leg A — revert the whole permission-set file to
origin/main. Predicted 9 red / 3 green in the route suite and 6 red across the three shipped-set suites. Measured exactly that. The two discriminating failures are the point:The 3 that stay green are correct: the whitelist-property pin is a pure column-half assertion, and the two object-gate cases were already object-gate refusals.
Leg B — revert only the RLS widening, keeping the object entry. Predicted 9 red / 3 green in the route suite and 3 red in the shipped-set suites. Measured exactly that, and PIN 4 now reads:
which is the independent evidence that the which-rows half is load-bearing for the guard even being reached. Each leg proved its mutation on disk before measuring (blob hash differs from the HEAD blob, plus marker counts), restored with
git checkout HEAD -- ABSOLUTE_PATHunder anEXIT/INT/TERMtrap, and proved the restore by an emptygit diff HEADand a blob hash equal to the HEAD blob.Shipped-set pins updated (and why each moved)
default-permission-sets.test.ts—EDIT_EXCEPTIONSgrows from one pair to two. The "exactly one pair" assertion becomes "exactly these two", and now checks that each exception rides a WRITE-class row scope, plus that the org-peer scope staysselect.member-default-explicit-allow.test.ts— the managed-object update axis grows a second permitted table;sys_userinsert/delete are asserted still shut, and a new case pins the write-class row scope and the read-only org-peer scope.authz-matrix-gate.test.ts— the threebetter_authwrite cells move fromCRUD_DENYto the caller's own row. This is the most informative reading in the diff:org_admingetsoadminand not the organization, andno_org_membergets its own id even with no active organization (sys_useris non-tenant, so Layer 0 is inert). Exactly one row per principal, nobody reaches anybody else's identity row.Verification
Run at the pushed head d845d56.
pnpm --filter @objectstack/plugin-security exec vitest run— 96 files / 1801 tests, all passing (exit 0).pnpm --filter @objectstack/plugin-auth exec vitest run— 94 files / 1951 tests, all passing (exit 0).pnpm --filter @objectstack/plugin-security run typecheck— exit 0, and its test layer is measured:check:test-typecheck: OK — 0 file(s) / 0 error(s).pnpm --filter @objectstack/plugin-auth run typecheck— exit 0.tsconfig.jsonexcludes**/*.test.ts:tsc --noEmitsays nothing about the new test file. The leg that does ischeck:test-typecheckagainsttsconfig.test.json, and it was confirmed to actually see the file —tsc --listFiles -p tsconfig.test.jsonnamessys-user-self-service-route.test.ts(1 hit), and the file appears in no entry of the shrink-onlytest-typecheck-debt.json, so it compiles with zero errors under the strict test config rather than being ledgered.Derived gate family —
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsderived 52 commands from the merge-base change set (not a hand-written diff). All 52 were run, exit code captured by redirect before any pipe. 48 exit 0. The other four answered exit 3 = NOT MEASURED, which is neither a pass nor a finding, and none of them is movable by this diff:check-test-completeness— grades a savedturbo run testlog that only CI produces.check:dual-build-cjs-loads— reads built output for every workspace package; most have nodist/in this worktree.check:i18n— runs the built CLI (packages/cli/dist/commands/i18n/extract.js), which is not built here. This diff adds no translatable strings.check:type-check-debt— refuses to re-measure without the whole workspace closure built, deliberately, because an unresolved import invents TS2307/TS7006 and erases real debt. Its siblingcheck:type-check-coverageran green.Gates worth naming individually because they are the ones this diff could plausibly move, all green:
check:engine-double-contract,check:test-source-alias,check:cross-package-test-inputs(both spellings),check:nul-bytes,check:empty-changeset,check:changeset-no-major,check:published-files,check:type-check-coverage,check-undeclared-dep-imports,check:pm-half-states.Control bytes:
check:nul-bytesgreen, plus a direct scan of all seven changed files withgrep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'— no matches.Out of scope, named rather than fixed
ADR-0092 D1's tier table still lists two Tier-1 members while the enforced whitelist constant holds three. Already filed as #14951 and left alone here.
Cross-links
sys_user.locale? Ruling B opened the COLUMN; ADR-0092 D5 still answers no to the ROUTE, so today only admins can set it #14959 (ruling of record: itsissuecomment-5528362189)sys_api_key) · column half: [Decision] May a user set their ownsys_user.locale? — the ADR-0092 D2 self-service whitelist stays{name, image}after #13881 (column landsreadonly, system-context writes only) #14787 / PR feat(platform-objects,plugin-auth): makesys_user.localeuser-writable, with a loud BCP-47 refusal (#14787) #14958🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code