fix(showcase): grant the seven navigable objects and the three master-detail children - #16069
fix(showcase): grant the seven navigable objects and the three master-detail children#16069os-warren wants to merge 3 commits into
Conversation
…-detail children The showcase shipped seven `type: 'object'` navigation entries whose targets no permission set granted read on, and three master-detail children with no object-level CRUD grant anywhere. Both rules are advisory, so `objectstack build` stayed green and warned on every run — 33 author-time warnings, 10 of them these. Measured on origin/main (53cbad9): `nav-object-ungranted` x7 (showcase_preference, showcase_expense_report, showcase_team, showcase_category, showcase_business_unit, showcase_field_zoo, showcase_cascade) and `security-master-detail-ungranted` x3 (showcase_expense_line, showcase_field_zoo, showcase_project_membership). After: both at 0, total 33 -> 23, no new warning class. The grants are distributed by persona rather than pooled: read on the `everyone` baseline for every object the shared navigation exposes (navigation is an access claim), reference-data curation on showcase_ops, expense filing and the project-detail children on showcase_contributor. access-matrix.json gains 14 rows and loses none. Two object doc comments claimed a wide `sharingModel` let any seeded persona use the object "without a bespoke permission set". That is the misconception the rule exists to catch — what actually carried the seeded admin was the platform's wildcard set — so both are rewritten to separate gate (1) object CRUD from gate (2) the OWD. Pin: examples/app-showcase/test/nav-and-detail-grants.test.ts runs the shipped rules against the real stack, with two controls that drop a real grant and demand the rules name that object back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…e pin controls `pnpm check:type-source-resolution` went RED on the previous commit: the new `@objectstack/lint` import in the showcase resolved its TYPES through the workspace link to `packages/lint/dist/*.d.ts`. The gate names the remedy and refuses the alternative -- "Add the `paths` rules to the package's tsconfig.json -- widening the registry entry is not the fix" -- so this adds the third `paths` entry beside the two already there, bare-keyed for exact match. It is the type half of the vitest alias in the same PR; the runtime half was already there. The reason bites harder for these two rules than for the existing two entries: both are advisory and return findings rather than throwing, so a `dist` merely behind the checkout raises nothing -- it answers with the OLD rule and the pin passes. Pin controls now match by CONTENT rather than by array position, so each stays a statement about the object it names even on a tree where several grants are missing at once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
… object comments Self-review catch on this PR's own prose, not on the code. The rewritten comments asserted two things this change never measured: that the objectui live e2e "no longer depends on being admin" (that suite was not run here), and that "every non-admin who clicked the entry got a 403" (no app was booted; the 403 is what `nav-object-ungranted` REPORTS, not something observed). Both are restated as what they are -- the lint rule's finding -- so the comments claim exactly the evidence this PR carries and no more. A showcase comment is read as documentation, so an overclaim there costs the same as an overclaim in the docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
Contract review (clause ②) — no gate · content PASS · NOT LANDABLE (red CI, real) — PR #16069 at head
|
Fixes #14453
Reproduction, before anything was edited
pnpm --filter @objectstack/example-showcase buildonorigin/mainat53cbad9f7,exit 0. The build's own verdict line:
Histogram of those 33 by the rule id each finding prints (sums to 33, i.e. the
histogram is the whole population, not a sample):
approval-approvers-may-resolve-emptynav-object-ungrantedreact-prop-deprecatedsecurity-master-detail-ungrantedsecurity-private-no-readscopeliveness-planned-propertyview/row-color-without-colorstitle-unresolvableflow-loop-body-uncontainedflow-draft-status-ambiguousThe card's "seven" is exact. Its master-detail claim is low by two. The card
names
showcase_field_zooas the detail object with no CRUD grant; the buildnames three.
The seven, verbatim from the build (message body elided after the first clause,
which is identical in all seven):
showcase_preferencenav_settingsapps[0].navigation[2].children[3].objectNameshowcase_expense_reportnav_expense_reportsapps[0].navigation[3].children[5].objectNameshowcase_teamnav_teamsapps[0].navigation[3].children[7].objectNameshowcase_categorynav_categoriesapps[0].navigation[3].children[8].objectNameshowcase_business_unitnav_business_unitsapps[0].navigation[3].children[9].objectNameshowcase_field_zoonav_field_zooapps[0].navigation[3].children[10].objectNameshowcase_cascadenav_cascadeapps[0].navigation[3].children[11].objectNameAnd the three, not one:
So the real population is 10 findings over 9 distinct objects —
showcase_field_zooappears in both classes.One measurement that changed the shape of the fix
The
nav-object-ungrantedhint offers three remedies: grant read, gate the entrywith
requiredPermissions/visible, or drop it.validate-nav-access.tsneverreads either gating key —
collectNavExposureswalkstype,objectName,id,childrenand nothing else. Measured against the shipped rule, with both controls:Only grant and drop actually clear the finding. Dropping seven working demo
surfaces out of the navigation is the silencing failure mode, so every one of the
seven is granted. (The ineffective hint is a defect in the rule, not in this app —
filed separately, not touched here.)
What an app author is supposed to learn from the result
A wide
sharingModeldoes not make an object reachable. All seven navigableobjects were already
public_read_write— the widest RECORD baseline the modeloffers — and all seven were unreachable anyway. Object-level CRUD (gate ①,
checkObjectPermission, granted only by a permission set) and the OWD (gate ②) areindependent, and gate ① runs first. The only principal it admits with no grant is
the platform's built-in wildcard admin set — which is exactly who an author browses
as while building, which is why this class survives to production.
The showcase was not merely silent about this; it taught the opposite. Two object
doc comments said the wide OWD let personas use the object "without a bespoke
permission set", and named the absence from
access-matrix.jsonas intentional.Both are rewritten.
controlled_by_parentderives RECORD access, never OBJECT CRUD. That is thewhole content of the three master-detail findings:
showcase_expense_lineis acontrolled_by_parentchild of a granted master and still needed its own grant —the "can't submit the subtable" 403 the rule exists for (ADR-0055).
Navigation is an access claim. Putting an object in the shared navigation
asserts that every member may open it, so the
everyonebaseline is where that getspaid for. Grants are therefore distributed by persona rather than pooled:
showcase_member_default(isDefault, theeveryonebaseline)showcase_preferenceandshowcase_cascade, the two rows a member genuinely ownsshowcase_contributorshowcase_expense_report,showcase_expense_line,showcase_field_zoocontrolled_by_parentline), andshowcase_field_zoo, a child of theshowcase_projectthis set already grantsshowcase_opsshowcase_team,showcase_category,showcase_business_unit,showcase_project_membershipallowDeleteonly on the junctionTwo narrowing decisions worth naming, because they are the difference between this
and a blanket grant:
Read on the baseline, write on the persona. Reference data (
showcase_team,showcase_category,showcase_business_unit) is readable by every member andauthored only by ops.
allowDeletestays off the baseline. Un-staffing a team isshowcase_project_membershipdelete, which lives onshowcase_ops. The D7 anchor gaterefuses delete on an
everyone-suggested set, and that refusal is the reason for theplacement rather than a constraint routed around. Measured against
describeAnchorForbiddenBits, with the control that the bits actually granted on thebaseline are the permitted ones:
showcase_cascadealso shows where narrowing genuinely belongs in that app: not onthe object, but on one option (
tier: 'restricted', admin-only, re-checkedserver-side by the objectql rule validator).
showcase_field_zoo— dispositionThe card flagged it as possibly a separate case: "a detail object may legitimately
be reached only through its master." Measured false for this object. It sits in
the shared navigation as
nav_field_zoounder Data Model, so it must be directlyreachable, and it is the one object that draws both warnings. Its
f_master_detail → showcase_projectfield exists to exhibit themaster_detailfield type in a field-type catalogue, not to make it a subtable. Both findings
collapse into one disposition: grant it — read on the baseline (it is navigable) and
read/create/edit on
showcase_contributor(which already grants its master).The card's framing is right about a different object:
showcase_expense_lineiscontrolled_by_parent, is not in navigation, and is reached only through itsmaster — and per ADR-0055 it needed the grant anyway. That is the separate case.
After
nav-object-ungranted7 → 0,security-master-detail-ungranted3 → 0, every otherclass byte-identical in count (10/5/2/2/1/1/1/1). 33 − 10 = 23 — the count dropped by
exactly the number of entries fixed, and no new class appeared. In particular
security-private-no-readscopestayed at 2. That is measured, not reasoned — read backout of the built artifact, with a control proving the field is being read at all:
The rule fires only on
private/unset OWD, and none of the nine is either — so addingread to them could not produce a new finding of that class, and did not.
access-matrix.json(the ADR-0090 D6 snapshot gate, which errors the build on drift)went 33 → 47 entries. The whole capability delta, diffed by
(set, object)key soadditions, removals and in-place widenings are counted separately:
No existing row moved, no
viewAllRecords/modifyAllRecordsbit appears anywhere inthe delta, and the single
deleteis the ops-held junction. The build named this driftas an error before the snapshot was regenerated; the listing above is the review artifact
the ADR asks for.
The pin, and its population
examples/app-showcase/test/nav-and-detail-grants.test.tsruns the shippedvalidateNavAccessandvalidateSecurityPostureagainst the real stack.It deliberately does not assert "the build exits 0" — the build exited 0 on the
broken tree too, for all 33 warnings. Both rules are advisory; only the findings
carry the verdict.
vitest.config.tsaliases@objectstack/lintto itssrc/, joining the two aliasesalready there. The reason is sharper for an advisory rule than for the existing two:
a
distbehind the checkout does not error, it answers with the old rule and thepin passes.
Covers, for this app's stack only:
type: 'object'nav entry inshowcase_appat any depth (top level, groups,areas, nested children) whose target is an object this stack defines — each must be
readable via some declared set (
allowRead/viewAllRecords/modifyAllRecords, asbuildAccessMatrixfolds them);master_detailfield — each musthold an object-level CRUD grant (any of the six bits) in some declared set.
Does NOT cover:
app-crm,app-todo,app-multi-package) — each needs itsown pin;
says nothing about the total;
the object. Pooling every grant onto one set, or widening one to a
'*'wildcard,satisfies both rules and this pin.
access-matrix.jsonis what makes that visible:any such change is snapshot drift and errors the build until reviewed and committed;
nothing and issues no request.
Two of the four tests are controls: each strips one real grant from every set and
demands the rule name that object back, matched by content rather than by position.
Without them the two positive assertions could go green because the rules stopped
finding anything at all.
Mutation proof
Fix committed first (
774832baa), then mutated — the restore leg needsHEADtoalready carry the implementation.
RED, quoting the assertion text:
The RED population is the same 7 + 3 the build warned about — the pin covers the whole
reproduction, not a sample of it.
The
objects[N]indices differ between the two readings (objects[6]/[11]/[12]here vsobjects[17]/[7]/[9]in the build) and that is expected, not a discrepancy: the pin readsthe config's own export while the build reads the normalized stack, which orders the
objects array differently. The rules address objects by NAME, and the three names match
exactly. The nav paths, which come from a structure normalization does not reorder, are
identical in both.
Restore ran from
trap '…' EXIT INT TERMwith an absolute path(
REPO_ROOT="$(git rev-parse --show-toplevel)"), spelledgit checkout HEAD -- ABSOLUTE_PATHso it reads the HEAD blob rather than a mutated index:
Neither hash is empty; blob equality and the empty
git diff HEADare both requiredand both hold.
Changeset
None, deliberately. Every source path here is under
examples/app-showcase, whosepackage.jsonis"private": true— nothing published moves.packages/lintisadded only as a
workspace:*devDependency of that private example (with the matchingpnpm-lock.yamlrow); no published package's manifest or output changes. Carrying theskip-changesetlabel accordingly.Out-of-scope finding, filed not fixed
#16065 —
nav-object-ungranted's hint prescribesrequiredPermissions/visibleas aremedy, and
validateNavAccessreads neither. Different package, different defect class,so it is filed rather than ridden along. It is the measurement quoted above under "One
measurement that changed the shape of the fix".
Verification
All exit codes captured bare (redirect, then read) — never through a pipe.
pnpm --filter @objectstack/example-showcase build(onorigin/main53cbad9f7)⚠ 33 author-time warning(s) — see abovepnpm --filter @objectstack/example-showcase build(at head, before snapshot regen)✗ Access matrix drift (14 changes) — capability changes must be reviewedobjectstack build --update-access-matrix→ Access matrix snapshot written to access-matrix.json (ADR-0090 D6) — review the diff.pnpm --filter @objectstack/example-showcase build(at00c12c3b6)⚠ 23 author-time warning(s) — see above;nav-object-ungranted0,security-master-detail-ungranted0, control rulereact-prop-deprecatedstill 5vitest run test/nav-and-detail-grants.test.ts(at00c12c3b6)Test Files 1 passed (1) / Tests 4 passed (4)— 2 assertions + 2 controlspnpm --filter @objectstack/example-showcase typechecktsc --noEmit— a real run, not a zero-package matchpnpm --filter @objectstack/example-showcase testTest Files 28 passed (28) / Tests 379 passed (379)pnpm lint(repo-wide,eslint . --no-inline-config)pnpm check:type-source-resolutioncheck-type-source-resolution OK — 125 tsc program(s) across 78 packages scannedpnpm check:dual-build-cjs-loadsPREREQUISITE NOT MET … ⛔ This is NOT a pass: nothing was measured; after building the 7 packages it named,✓ check:dual-build-cjs-loads — 103 published require entry point(s) across 66 package(s) loadnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackreconciles at 44families (its Reconciliation line, not the local-gate block, which is 32 of the 44).
All 44 ran on the first commit — that is how
check:type-source-resolutionwas caught —and all 44 ran again as a union on the final commit
00c12c3b6, on a clean tree(
git status --porcelainempty), with the family set re-derived at that head andbyte-identical to the first derivation. That union is 44/44 exit 0, no failure
output produced.
One union run was discarded rather than reported: it was started at
5a31720eaandtwo comment files were edited under it mid-sweep, so roughly the first three quarters of
its gates had graded a tree the last quarter had not. A partly-stale union reads exactly
like a clean one, so it was stopped and re-run from scratch on the committed tree instead.
The typecheck's coverage was itself checked rather than assumed:
tsc --noEmit --listFilesputs the new pin and all three edited source files in the program, and resolves
@objectstack/linttopackages/lint/src/*rather thandist— so the green is about thesource in this checkout.
NOT MEASURED
actually open
nav_cascadefollows from the model, not from an observation made here.app-crm,app-todoandapp-multi-packagewere not built orexamined; whether they carry the same class is unknown, not answered.
reading is the PR's own jobs.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
Generated by Claude Code