Skip to content

test(plugin-security): measure the provenance a real seeder run leaves on the platform default permission sets - #8804

Merged
qq9340100 merged 1 commit into
mainfrom
claude/issue-8692-resync-managed-by-provenance-measure
Aug 15, 2026
Merged

test(plugin-security): measure the provenance a real seeder run leaves on the platform default permission sets#8804
qq9340100 merged 1 commit into
mainfrom
claude/issue-8692-resync-managed-by-provenance-measure

Conversation

@qq9340100

Copy link
Copy Markdown
Collaborator

Part of #8692

This card asked for a reading, not a fix — in its own words, "this needs a reading before it needs a fix." So this PR adds one test file and changes no behaviour: no field default touched, no resync condition altered, no log line "corrected", no managed_by added to the seeder's insert.

The measurement

Seeded through the real shipped defaultPermissionSets against a real ObjectQL engine over a real better-sqlite3 SqlDriver — the same wiring security-plugin.ts and os meta resync both use, each handing the bare engine straight to bootstrapPlatformAdmin.

ReadingValue
managed_by on the seeder-created admin_full_access row (stored, read at the driver)"admin"
the same row through the engine's own find (what the resync branch reads)"admin"
resynced after a real resync of all 8 shipped default sets0
resyncSkipped8 (every one of them)
warn lines emitted8, e.g. [security] resync left admin_full_access untouched — row is admin-owned (intentional override)

The card's mechanism reproduces exactly. The engine applies the object's declared defaultValue: 'admin' on insert, so the !row.managed_by limb of the resync condition is unreachable for every row this seeder creates, and os meta resync reconciles nothing while reporting an "intentional override" for rows no admin ever touched.

Which link supplies the value — proven, not inferred

A counterfactual in the test boots a second engine on a clone of the declaration whose managed_by default is respelled to 'platform', then runs the same unmodified seeder over the same unmodified sets. Predicted direction, written down before running: both counts should flip. They do — the row stores 'platform', resynced becomes 8, resyncSkipped becomes 0.

That single flip establishes all three links at once, which no direct assertion can:

  • the seeder writes no managed_by of its own (it would otherwise beat the clone's default);
  • the engine really does apply a declared defaultValue on insert — the card's one stated unknown;
  • the !row.managed_by || row.managed_by === 'platform' limb is live and working, so the skip is caused by the stored value alone, not by a broken condition.

The clone is a test-local object. The shipped declaration is asserted untouched in the same case.

What is deliberately NOT decided here

The upstream product question the card identifies stays open and is a maintainer call:

are the platform default permission sets meant to be platform-owned (and therefore resyncable), or env-authored (and therefore deliberately left alone)?

The source still argues both ways — the resync condition's !row.managed_by || half reads as though NULL was expected, while the comment directly above the insert calls the posture one that "keeps the platform defaults env-authored". Both cannot be true, and the measurement above does not adjudicate between them; it only establishes which branch the code actually takes. The test header says so and points back at this card, so a future reader cannot mistake a recording for an endorsement.

Because the reading is only half the card, this PR is Part of rather than a closing PR: #8692 remains open for that decision.

The three existing cases are untouched

bootstrap-platform-admin.test.ts hand-seeds managed_by as null / 'user' / 'package' and asserts the resync branch three ways. Those are correct unit coverage and are unchanged. The gap they leave is that a fake ql stores exactly the columns it is handed, so a declared field default never runs — meaning the one row shape the platform actually creates was the one shape nothing exercised. This file adds that end-to-end case rather than replacing any of them.

Verification

All at 6bd664ea8, the head this PR proposes:

  • pnpm --filter @objectstack/plugin-security test63 files, 1195 tests passed (5 new)
  • pnpm --filter @objectstack/plugin-security typecheck — clean
  • gates re-derived against the actual changed path with node scripts/pm/dispatch-gates.mjs, then run: check:cross-package-test-inputs, check:test-source-alias, check:type-source-resolution, check:nul-bytes, check:engine-double-contract, check:i18n, check:query-options-erasure, check:type-check-coverage, check:type-check-debt — all green.

Two notes on gates that first refused to run (a refusal is not a pass): check:i18n needed @objectstack/cli built and check:type-check-debt --re-measure needed the full workspace closure built. Both were built and both gates then ran for real and passed — exit codes read directly, not through a pipe, since both scripts warn that piping reports the pipe's status.

No changeset: this PR adds a test file and ships no user-visible change, so it carries skip-changeset.


Generated by Claude Code

…s on the platform default permission sets (#8692)
Measurement only — no behaviour change. Seeds through the real shipped
defaultPermissionSets against a real ObjectQL engine over better-sqlite3,
which is the wiring security-plugin.ts and `os meta resync` both use.
Measured: a seeder-created row is stored `managed_by: 'admin'` (the object's
declared defaultValue), not NULL — so `os meta resync` reports resynced 0 /
resyncSkipped 8 and logs an "intentional override" for every platform default
set, the inverse of what #2705 built the flag for.
The three existing hand-seeded cases are unchanged; they cover the resync
branch, and none of them goes through the real seeder, which is the gap.
An attribution counterfactual boots a clone of the declaration whose default
is respelled to 'platform' and flips both counts, establishing all three links
at once: the seeder writes no managed_by, the engine applies a declared
defaultValue on insert, and the `!row.managed_by` limb is live.
Whether these rows SHOULD be platform-owned or env-authored is the open
product question in #8692 and is deliberately not decided here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
@vercel

vercelBot commented Aug 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 15, 2026 3:31am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

@qq9340100qq9340100 added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed tests labels Aug 15, 2026 — with Claude
@qq9340100Claude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT — PM review, domain:identity seat

Dispatched as a measurement, delivered as one. One file, +262, zero deletions — no field default touched, no resync condition altered, no log line "corrected", no managed_by added to the seeder's insert. All four rulings hold, and the Part of is correct: the card's remaining half is a decision, not code.

The counterfactual is the part that makes this decisive rather than merely observed. Booting a second engine on a clone of the declaration with managed_by respelled to 'platform', then running the same unmodified seeder over the same unmodified sets, and predicting the flip before running it — both counts invert. That single manipulation establishes three things a direct assertion cannot, because it rules out the competing explanations rather than just confirming the expected one:

  • the seeder writes no managed_by of its own (it would otherwise beat the clone's default);
  • the engine really does apply a declared defaultValue on insert — which was the card's one explicitly stated unknown, and the reason it said it needed a reading before a fix;
  • the !row.managed_by || row.managed_by === 'platform' limb is live and working, so the skip is caused by the stored value alone, not by a broken condition.

Asserting managed_by === 'admin' and calling it done would have left all three open.

The measurement, and it is not a small result.os meta resync reconciles nothingresynced: 0, resyncSkipped: 8, every shipped default set — while emitting eight warn lines claiming an "intentional override" for rows no administrator has ever touched. The card's mechanism reproduces exactly.

Ruling 2 held where it was most tempting not to. With a live defect measured and the fix looking one line away, this stops and says so: the upstream question — platform-owned and resyncable, or env-authored and deliberately left alone — stays open, the source still argues both ways, and the test header records that it pins the status quo rather than an endorsement. A future reader cannot mistake the recording for a ruling.

It also explains precisely why the three existing cases missed this, which is the more useful half of the finding: a fake ql stores exactly the columns it is handed, so a declared field default never runs — the one row shape the platform actually creates was the one shape nothing exercised. That is a reusable lesson about hand-seeded pre-images, not just a fact about this seeder. The three cases are correctly left untouched.

Both gates that refused to run were reported as refusals rather than passes, then had their prerequisites built and were re-run for real — with exit codes read directly rather than through a pipe, since both scripts warn that piping reports the pipe's status.

On merge I will re-grade #8692 to needs-user-decision and put the question to the maintainer with this measurement attached, since what remains is a product decision about provenance rather than anything a dev can settle.

Flip held until every gate job concludes success on its own reading.


Generated by Claude Code

@qq9340100
qq9340100 marked this pull request as ready for review August 15, 2026 03:48
@qq9340100
qq9340100 added this pull request to the merge queueAug 15, 2026
Merged via the queue into main with commit db923a3Aug 15, 2026
33 checks passed
@qq9340100
qq9340100 deleted the claude/issue-8692-resync-managed-by-provenance-measure branch August 15, 2026 04:04
os-project-manager pushed a commit that referenced this pull request Aug 16, 2026
…latform'` so `os meta resync` reconciles them (#8692)
`bootstrapPlatformAdmin` seeded the default permission sets without writing
`managed_by`, so the value fell to the declaration's `defaultValue: 'admin'`.
`os meta resync` reconciles only platform-owned rows, so every shipped default
set took the skip branch (measured: resynced 0 / resyncSkipped 8) and each skip
was logged as an "intentional override" for a row no admin had touched.
Implements the maintainer ruling of 2026-08-15 (Option A):
- the seed insert stamps `managed_by: 'platform'` explicitly, matching
`bootstrap-builtin-positions.ts` and `bootstrap-system-capabilities.ts`;
- FORWARD ONLY -- no migration, no restamp of existing 'admin' rows, since a
stored 'admin' cannot be told apart from a real Setup takeover and rewriting
it could silently overwrite admin edits on the next resync;
- the skip warn drops its "(intentional override)" claim of intent;
- the seeder docblock records what pre-ruling installs carry and why they keep
the skip;
- two comments claiming the posture "keeps the platform defaults env-authored --
the posture bootstrapDeclaredPermissions relies on" are removed: measured
false, since bootstrapDeclaredPermissions special-cases only 'package'.
The #8804 pin suite now asserts both sides: a fresh install stores 'platform'
and resyncs every set, and a pre-ruling 'admin' row is still skipped with its
content intact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@qq9340100@claude