test(plugin-auth): register sys_position / sys_user_position in the sso-register harness - #16305
Merged
Merged
Conversation
…so-register harness `bootEngine()` in `sso-register-platform-admin-gate.test.ts` registered the auth identity objects plus hand-declared `sys_permission_set` and `sys_user_permission_set`, but never the position pair the platform-admin standing resolver also reads. `resolveAuthzContext` reads `sys_user_position` on every resolution, and `sys_position` whenever the principal holds any position — always, since every authenticated member implicitly holds the ADR-0090 D5 `everyone` anchor. With no such tables, both reads were refused by the driver and `tryFind`'s loud-failure arm logged them, while the resolver went on treating each refusal as "this principal holds no positions". The suite reported `Tests 4 passed (4)` over eight `[sql-driver] DATABASE_ERROR ... no such table` lines, four per table, and no assertion could notice. That is a latent false green on a security-adjacent resolver: a change making platform-admin standing genuinely depend on a position row would have been measured against an engine that can never return one, and this file would have stayed green. Same shape #14756 exists to remove — a harness whose registered object set is narrower than the code path it drives. Registers both objects the way the file already hand-declares the permission-set pair, with only the columns the judge reads: `active` (the ADR-0049 predicate `isRowActive` applies in §6a) and `organization_id` (what the driver's `applyTenantScope` filters the organization-scoped `sys_position` read on, and what §4 tests to place a `sys_user_position` row). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-warren
marked this pull request as ready for review
September 6, 2026 13:47
os-warren
enabled auto-merge
September 6, 2026 13:47
os-warren
deleted the
claude/issue-14846-sso-register-harness-positions
branch
September 6, 2026 14:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14846
What this changes
bootEngine()inpackages/plugins/plugin-auth/src/sso-register-platform-admin-gate.test.tsregistered the auth identity objects plus hand-declared
sys_permission_setandsys_user_permission_setfixtures, but never the position pair the platform-admin standingresolver also reads. This registers
sys_positionandsys_user_positionthe samehand-declared way, with only the columns the judge reads. Test-only; no source, contract or
export surface is touched.
The defect, and why the suite could not see it
resolveAuthzContextreadssys_user_positionon every resolution, andsys_positionwhenever the principal holds any position — which is always here, because every
authenticated member implicitly holds the ADR-0090 D5
everyoneanchor. With neither tableregistered, both reads were refused by the driver and
tryFind's loud-failure arm loggedthem, while the resolver went on treating each refusal as "this principal holds no
positions".
So the file reported
Tests 4 passed (4)sitting on top of eight[sql-driver] DATABASE_ERROR ... no such tablelines, four per table, and not one assertioncould notice.
That is a latent false green on a security-adjacent resolver, not untidy log output:
a future change that made platform-admin standing genuinely depend on a position row would
have been measured against an engine that can never return one, and this file would have
stayed green. It is the same shape #14756 exists to remove — a harness whose registered
object set is narrower than the code path it drives — kept invisible because the errors are
logged rather than thrown.
Measurement
The acceptance instrument is one
vitest runof that single file, counting[sql-driver] DATABASE_ERROR ... no such tablelines. Re-derived on this branch's own headrather than quoting the card's two-day-old tally. The before-leg was measured by restoring
the file to
origin/mainon the same tree; the mutation and the restore were both proven byblob hash (
git hash-objectagainst theHEADblob), and the tree was verified cleanafterwards.
sys_positionsys_user_positionTests 4 passed (4)Tests 4 passed (4)The six paging warnings fall out for the same root cause and are not a second fix: the driver
emitted them because it had not created those tables and therefore could not name a unique
column to order by. Its own remedy line says "declare the object so this driver manages its
table", which is exactly what registering them does.
No test turned red.
Which columns, and why
Both fixtures follow the permission-set pair already in the file — a plain object literal
with the columns the resolver actually touches, so a fixture adds no dependency edge from
plugin-auth to plugin-security.
sys_position:id,name,active,organization_id.activeis the ADR-0049predicate the resolver applies through
isRowActivein section 6a;organization_idiswhat the driver's
applyTenantScopefilters the organization-scopedsys_positionreadon, so omitting it would have quietly dropped that wall from the harness.
sys_user_position:id,user_id,position,organization_id— the where key plusthe three columns section 4 reads to place a row as global or another tenant's.
Scope
The card offered a second direction — deciding the resolver's absent-table read is a benign
discriminated case and no longer logging it. That one is not taken here: it overlaps the
fenced arm of #14615 and belongs to that card, per triage. Also untouched, deliberately:
packages/core/src/security/admin-standing-surface.ts, the two plugin-security objectdefinitions, and the other four plugin-auth harnesses.
Verification
Run at
519a76915, the head this PR carries.pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/sso-register-platform-admin-gate.test.ts—
Test Files 1 passed (1)/Tests 4 passed (4), zeroDATABASE_ERRORlines.pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2—
Test Files 102 passed (102)/Tests 2158 passed (2158).pnpm --filter @objectstack/plugin-auth typecheck— green, including thecheck:test-typecheckleg that covers the test layer (the package's owntsconfig.jsonexcludes
**/*.test.ts, so that third leg is the one that measures this file). Its debtledger is unmoved: 10 files / 94 errors / 23 pinned signatures held.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus thefour artifact rosters the tool flags as sitting in a directory this diff's path is in
(
check:auth-mount-ledger,check:authz-resolver,check:error-code-casing,check:filter-alias-parity). All 50 commands run, all exit 0, each exit code capturedimmediately after a single redirect and never through a pipe. Two of them
(
check:dual-build-cjs-loads,check:type-check-debt) first answered exit 3PREREQUISITE NOT MET; the closure was built with
pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'and both werere-run to a real green rather than recorded as passes.
Changeset
Carries
skip-changeset, judged rather than assumed: the diff is one*.test.tsfile, andplugin-auth publishes
files: ["dist", "README.md", "CHANGELOG.md"]with two tsup entries(
src/index.ts,src/rate-limit-storage.ts). No test file reachesdist, so this PRreleases nothing from any package and there is no user-visible change to describe.
Generated by Claude Code