Skip to content

fix(security): stop reporting the platform bucket as a "pre-fix" leftover with a remedy that recreates it - #11629

Merged
os-sam merged 1 commit into
mainfrom
claude/issue-11532-walled-orgless-permission-sets
Aug 24, 2026
Merged

fix(security): stop reporting the platform bucket as a "pre-fix" leftover with a remedy that recreates it#11629
os-sam merged 1 commit into
mainfrom
claude/issue-11532-walled-orgless-permission-sets

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Part of #11532

Implements the diagnostic half of the card. The minting half is deliberately
untouched and is explained under "What this PR does NOT do", so the card stays
open after this merges — hence Part of, not a closing keyword.

Premise re-measured on current main before any code was written

The card was dispatched with an explicit precondition: check current origin/main
for the symptom, not the issue number. Measured at 2cf5a96cc7, against a real
ObjectQL + SqlDriver (better-sqlite3), driving the SHIPPED seeders in the boot
order security-plugin.ts uses (bootstrapPlatformAdmin, then one
bootstrapDeclaredPermissions pass per organization), OS_TENANCY_POSTURE=isolated,
three organizations:

whatmeasured
organization-less sys_permission_set rows after a fresh walled boot8
their namesadmin_full_access, mcp_agent_data_read, mcp_agent_data_write, mcp_agent_restricted, member_default, organization_admin, organization_admin_no_bypass, viewer_readonly
their provenancemanaged_by = 'platform', package_id = NULL (every row)
each organization's own copies9 rows, managed_by = 'package', package_id = com.objectstack.plugin-securitydifferent row ids
what the per-organization sweep does to the 8nothing: the org-less row set is byte-identical before and after (JSON snapshot of every stored column)
what the pass then said, once per organizationpre-fix organization-less sys_permission_set rows are still present … Remedy: re-initialize the deployment, or adopt each row by hand

The symptom is live on current main. Pinned by identity, not count — the
name set and the row ids are asserted, because two offsetting errors can hold a
count constant while the membership inverts.

Nothing here reads the pass's own verdict as evidence. That verdict is the thing
under test and it is self-falsifying, so every assertion reads either rows straight
off knex (past every engine-side projection) or the recorded logger calls.

The two things that did not hold up

  1. Nothing was pre-fix. The 8 rows were minted by bootstrapPlatformAdmin
    the fifth seeder, outside the four fix(security,sharing): materialize the RBAC catalog per organization #11121 converted — before the deployment's
    first organization existed. An operator on a deployment hours old was told they
    were carrying legacy state they never had.
  2. The first remedy did not terminate. Re-initializing a fresh walled deployment
    mints exactly those 8 rows again on the next boot, so only the hand-adoption
    branch ends — and that one hands a platform-wide bucket to a single tenant.

Related, and repaired here too: per-organization-catalog.ts's own header asserted
that "a fresh walled deployment never mints an organization-less catalog row once
these seeders run per organization"
. On the measured boot it mints 8, from a seeder
outside those four.

What changed

  • per-organization-catalog.tswarnPreFixOrganizationLessRows becomes
    warnOrganizationLessRows and splits its input into the two classes it was
    conflating, emitting one line each with the remedy that fits, and carrying a
    machine-readable origin ('platform-bucket' / 'pre-fix-residue') beside the
    named rows. The pre-fix-residue message and remedy are unchanged, verbatim.
    The module header's false claim is corrected and now names the ruling it depends on.
  • bootstrap-declared-permissions.ts — new optional platformBucketNames,
    defaulting to the shipped defaultPermissionSets. The default is what makes this
    correct for every shipped composition even if the option is never threaded; the
    option exists for a host that overrode SecurityPluginOptions.defaultPermissionSets.
  • security-plugin.ts — threads that host's actual array, computed from the same
    bootstrapPermissionSets that goes to the platform bootstrap and onto the manifest,
    so the two can never disagree.
  • bootstrap-platform-admin.ts — under a walled posture the producer now says what
    it wrote: the defaults were seeded without an organization, and each organization's
    copies come from the catalog pass. The rig's boot line read {"seeded":8} with
    nothing to indicate the rows carried no organization at all, so the operator's first
    sight of them was the warning above. The seeding itself is byte-identical.
  • bootstrap-declared-positions.ts / bootstrap-builtin-positions.ts — import
    rename only. They declare no platform bucket, because nothing mints an
    organization-less sys_position row any more, so every leftover there really is
    pre-fix residue. Behaviour unchanged.

Membership is decided by name, not by managed_by, because the question the remedy
turns on is "will a re-initialized deployment have this row again?" — true for these
names whatever provenance the current row carries (a pre-#8692 install stores 'admin'
on the very same names).

What this PR does NOT do — and why that is a ruling, not an omission

⛔ It does not stop the minting. #10103's maintainer ruling of 2026-08-20
(Q1/Q2, live decision-inbox session) closes that, verbatim:

Q1's platform-defaults residue (bootstrapPlatformAdmin's three sets, the env-door
projection) stays outside this card, unreaped and loudly warned about under walled
posture; whether customers ever need those visible is a future small decision card,
filed only when an onboarding flow actually wants it.

bootstrap-declared-permissions.ts's own header already records the same boundary.
The rows are load-bearing besides: PLATFORM_ADMIN is derived from an unscoped
sys_user_permission_set grant pointing at the organization-less admin_full_access
row by row id (resolve-authz-context.ts §6b), which is why the ruled Option C
refused a #8617-breadth reap in the first place.

So the loop this PR breaks is the one that was actually breakable: it lived in the
remedy text. Routing the platform bucket through the per-organization pass remains
the maintainer's open call, and it is raised in the dev report rather than filed as a
card, because the ruling says that card is filed only when an onboarding flow wants it.

Verification

All at final commit c83945d19c unless stated.

  • Reverse verification (the strongest evidence here). The new suite was written and
    run before any source edit, on a tree whose only difference from origin/main was
    the untracked test file (git status showed exactly that one entry). Predicted in
    writing first, then run: cases 1/2/5 green (the premise), cases 3/4 red. Observed
    exactly that — Tests 2 failed | 3 passed (5), failing on
    AssertionError: expected undefined to be 'platform-bucket' and
    AssertionError: expected undefined to be 'pre-fix-residue'. The pre-fix code is
    the ablation; no mutation was needed, so no restore could silently fail.
  • pnpm --filter @objectstack/plugin-security typecheck> tsc --noEmit, exit 0.
  • pnpm --filter @objectstack/plugin-security test -- --maxWorkers=2
    Test Files 77 passed (77) / Tests 1470 passed (1470), including
    per-organization-catalog.test.ts case 3, which pins the unchanged pre-fix wording
    and its re-initialize remedy.
  • pnpm lint (repo-wide eslint . --no-inline-config) →
    os-verify-lock: VERDICT command-exit 0 · held the lock 87s. Not narrowed.
  • Gate families derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
    (no hand-written path list), all green at this commit: check:nul-bytes,
    check:engine-double-contract, check:where-matcher, check:slot-lookup,
    check:query-options-erasure, check:type-check-coverage,
    check:cross-package-test-inputs, check:test-source-alias,
    check:type-source-resolution, check:published-files, check:i18n (CLI built
    first — it refuses as PREREQUISITE NOT MET otherwise, which is not measured, not
    green), check:empty-changeset, check:adr-0087-registration,
    check:changeset-gate-self-tests, check:objectui-changeset,
    check:ci-filter-parity, check:plugin-teardown-shape,
    scripts/docs-audit/check-affected-docs.mjs, scripts/pm/release-rehearsal-clone.mjs --self-test.
  • scripts/engine-double-contract.pinned.json gains one row. The new suite's
    delegating seam routes update() through the producer's own
    assertEngineUpdateDispatch, so this is the pinned ledger growing by one — not
    the shrink-only baseline. Written with the gate's own
    node scripts/check-engine-double-contract.mjs --write
    (1 added or grown, 0 lost), never by hand.
  • check:type-check-debt: the whole-ledger --re-measure needs the entire
    workspace built and is CI's run. What was measured here instead is the exact quantity
    the ratchet compares for the one package this PR touches: the gate's own
    remeasureProject shape was reproduced (package tsconfig with the **/*.test.ts
    exclusion dropped) and tsc --noEmit reported 11 errors — equal to the recorded
    @objectstack/plugin-security: { errors: 11 }, with 0 of them in the new test file
    .
    The ratchet cannot drift upward from this change. The other ledger entries were not
    re-measured here; no path in this diff reaches them.

Heavy steps were serialized through scripts/pm/os-verify-lock.sh; every verdict above
is read from the gate's or the lock's own printed line, never from $? after a pipe.

Generated by Claude Code


Generated by Claude Code

…over with a remedy that recreates it
A fresh walled deployment mints 8 organization-less `sys_permission_set` rows
from `bootstrapPlatformAdmin` — the fifth seeder, kept outside the
per-organization conversion by the 2026-08-20 ruling on #10103. The
per-organization pass then warned, once per organization, that "pre-fix
organization-less rows are still present" and offered "re-initialize the
deployment, or adopt each row by hand". On a deployment hours old nothing was
pre-fix, and re-initializing mints exactly those rows again, so the first
remedy branch was a loop.
The pass now separates the two classes it was conflating and gives each the
remedy that fits, carrying a machine-readable `origin` beside the named rows.
The seeding itself is byte-identical: the rows stay, unreaped, because
PLATFORM_ADMIN is derived from an unscoped grant pointing at the
`admin_full_access` row by row id.
Part of #11532
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 12 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx(via sys_permission_set (literal))
  • content/docs/permissions/authorization.mdx(via bootstrapDeclaredPermissions (symbol), bootstrapDeclaredPositions (symbol), sys_permission_set (literal), sys_position (literal))
  • content/docs/permissions/delegated-administration.mdx(via sys_permission_set (literal), sys_position (literal))
  • content/docs/permissions/permission-sets.mdx(via sys_permission_set (literal))
  • content/docs/permissions/positions.mdx(via sys_position (literal))

5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v12.mdx(via bootstrapDeclaredPermissions (symbol), sys_permission_set (literal))
  • content/docs/releases/v13.mdx(via sys_permission_set (literal), sys_position (literal))
  • content/docs/releases/v14.mdx(via sys_position (literal))
  • content/docs/releases/v15.mdx(via sys_permission_set (literal), sys_position (literal))
  • content/docs/releases/v17.mdx(via sys_permission_set (literal), sys_position (literal))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 3637731e2811d375fe98187bc67f3e8a0b4585capackageMentionDocs.

Which tree this was computed on

This run read content/docs from 0ccc27e7b1a8c329cca71528e81c496752411365 — the merge of head c83945d19c60b1e1f39aafa6b2b8fe688286229c into base 3637731e2811d375fe98187bc67f3e8a0b4585ca, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0ccc27e7b1a8c329cca71528e81c496752411365 && git checkout 0ccc27e7b1a8c329cca71528e81c496752411365
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3637731e2811d375fe98187bc67f3e8a0b4585ca c83945d19c60b1e1f39aafa6b2b8fe688286229c && git checkout -B drift-repro 3637731e2811d375fe98187bc67f3e8a0b4585ca && git merge --no-ff c83945d19c60b1e1f39aafa6b2b8fe688286229c
node scripts/docs-audit/affected-docs.mjs --json 3637731e2811d375fe98187bc67f3e8a0b4585ca

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 3637731e2811d375fe98187bc67f3e8a0b4585ca → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@os-sam
os-sam marked this pull request as ready for review August 24, 2026 10:07
@os-sam
os-sam added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit e3f056fAug 24, 2026
32 checks passed
@os-sam
os-sam deleted the claude/issue-11532-walled-orgless-permission-sets branch August 24, 2026 10:22
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-sam@claude