Skip to content

fix(plugin-security): the app default permission set resolves from packages[] (#15007) - #15226

Merged
hotlong merged 1 commit into
mainfrom
claude/issue-15007-plugin-security-permissions
Sep 4, 2026
Merged

fix(plugin-security): the app default permission set resolves from packages[] (#15007)#15226
hotlong merged 1 commit into
mainfrom
claude/issue-15007-plugin-security-permissions

Conversation

@hotlong

@hotlong hotlong commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #15007

Reader card 4/4 of the ADR-0130 D4 option-B program ruled on #14512 (comment 5528589044). One reader, one line — and the one line both earlier enumerations missed, in a package neither had scoped.

The defect

appSecurityPluginOptions(config) read config.permissions top-level and nothing else (packages/plugins/plugin-security/src/app-default-permission-set.ts). Under the option-B artifact shape — packages[] carrying each definition exactly once, the flattened top level gone — that read returns undefined, the reader concludes "this app declared no default profile", and the boot continues clean.

The name it resolves becomes the SecurityPlugin's fallbackPermissionSet, i.e. the app's half of every authenticated human principal's additive baseline (composeHumanBaselinePermissionSets, ADR-0090 D5). Losing it denies nobody the boot: the deployment simply runs on the platform floor alone, and every member of a multi-package app quietly holds less than the app declared. #7555 measured what that looks like from outside — nav entries served, 403 behind them.

The change

declaredPermissionSets reads the flattened top level FIRST, then each package body in resolveArtifactPackageOrder order (@objectstack/core, ADR-0130 D4+D5, since #14643).

  • Every artifact emitted today answers bit-identically. The flattened level answers first, so the packages[] pass can only supply a set where the top level had none. The artifact stays additive; nothing about what any command emits moves. composeStacks and packages/spec/src/stack.zod.ts are untouched.
  • Order is the platform's one package order, not the array's. appDefaultPermissionSetName resolves the FIRST isDefault set, so with two packages declaring one, "first" has to mean here what it means at every other artifact reader: dependency-topological, whichever array slot a package occupies. Pinned in both directions (with the edge, and without it).
  • The singular manifest is still not consulted — the @objectstack/verify's bootStack ignores an app's isDefault permission set, while the CLI honours it — after #5491 a dogfood app cannot test its own declared baseline #7001 constraint, pinned by the existing test, holds unchanged. It is not a special case either: an artifact with no packages key makes resolveArtifactPackageOrder return the caller's own object as the single package body (D4's second branch), so that path is the old read exactly.
  • A malformed packages is refused, not skipped — non-array, an entry inlined instead of wrapped under manifest:, or a duplicate package id raise the loader's own ADR-0112 envelope (code + status: 422), asserted by code and status. Catching it would resolve a permission surface out of an artifact the loader refuses to load.

Only @objectstack/plugin-security production code changes; @objectstack/core was already one of its dependencies and is already imported by five other modules in it.

The ledger row deleted, and why it is earned

One row leaves OPTION_B_LOSSES in packages/cli/test/option-b-reader-acceptance.pin.test.ts:

B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions

Measured, in this order, each leg with @objectstack/plugin-security REBUILT first (the cli suite resolves it through the workspace link to dist/, per KNOWN_UNALIASED_TEST_IMPORTS):

Step Pin result
before any edit, at 7bc5d37e4 6 passed — the ledger matches the measurement exactly
fix built into dist/, ledger untouched 1 failed / 5 passed — A ledgered subsystem now SEES its collections … Delete these lines, naming that one row and no other
row deleted 6 passed
ablation — source reverted to 7bc5d37e4, rebuilt, row still deleted 1 failed / 5 passed — A subsystem lost a collection that the ledger does not carry, LOST … = none, naming that one row

The ablation is the discrimination: the row goes green because of this reader and nothing else. Both legs were rebuilt and each rebuild was proved to reach the artifact the suite consumes (scripts/ablation-dist-preflight.mjs, marker resolveArtifactPackageOrder: present in 2 built files after the fix, absent from all 6 built files under ablation, present again after restore). The mutation was proved on disk by blob identity before it was measured (git hash-object == the 7bc5d37e4 blob, != the HEAD blob; marker count 6 → 0), and the restore by a whole-tree git status --porcelain coming back empty.

⚠️ The B1 sibling row stays, and is not this card's to delete.

B1 · plugin-security appSecurityPluginOptions over the artifact-serve config (default permission set) · permissions

That row calls appSecurityPluginOptions(createStandaloneStack(...)). createStandaloneStack (packages/runtime/src/standalone-stack.ts:807) surfaces permissions from artifactBundle.permissions and does not surface packages at all, so under option B the object this reader receives carries neither the sets nor any way to reach them. No change inside @objectstack/plugin-security can turn it green.

It is downstream of card #15005, and it goes green the moment #15005 lands — the standalone result will carry permissions again and this reader's top-level branch answers, with no further edit here. So #15005's PR should expect to delete TWO rows, not one: its own createStandaloneStack surfaced permissions row and this B1 one. #15005 had NOT landed when this was measured (origin/main at 9c1bcda38; the card is open, and createStandaloneStack still resolves no packages[]).

The by-shape sweep the card requires

Re-run by SHAPE across all of packages/, never against a package list — the collection key set derived from ObjectStackDefinitionSchemaAssembledPackageBodySchema (37 keys), four access shapes (identifier property access, destructuring, bracket, this./options. members), plus two structural passes (every parameter typed as the stack; every site opening objectstack.json itself).

It turned up two more packages, and they are filed as #15210 rather than folded in here, per this card's instruction:

  • @objectstack/verifyderive.ts:176,180 (config.objects, config.datasources), rls.ts:134 (config.positions), rls.ts:346 (config.objects). Worth the reviewer's attention: under option B os verify derives zero CRUD cases and builds an EMPTY RLS probe permission set, i.e. it reports a green run having measured nothing.
  • @objectstack/plugin-devdev-plugin.ts:526 (options.stack.translations), so the I18nServicePlugin auto-detect never fires.

#15210 carries the full search, the triage of every false positive (plugin-auth's config.plugins, objectql's engine config, service-analytics's service options, metadata-protocol's view body, metadata-core's authoring-time detector, and the stack.KEY spellings that are prose in bootstrap-declared-*), so the next sweep can be compared against this one rather than redone.

Verification

  • Pinpnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 test/option-b-reader-acceptance.pin.test.tsTest Files 1 passed (1) · Tests 6 passed (6) at e99a3af7, with the four-step table above behind it.
  • New unit coveragepackages/plugins/plugin-security/src/app-default-permission-set.test.ts, Tests 18 passed (18) (was 8). The two shapes are built by the REAL composeStacks with manifest: 'preserve' and the option-B one derived by stripping the schema-derived package-owned keys, with a CONTROL asserting the additive shape really carries the flattened copy — so the option-B case cannot pass by having had nothing to lose.
  • Consumers of this reader, all green at e99a3af7packages/runtime/src/standalone-stack.test.ts 21/21, packages/verify/src/harness.app-default-profile.test.ts 6/6, packages/cli/src/commands/serve-verify-security-parity.contract.test.ts 16/16. These resolve @objectstack/plugin-security through dist/, so they ran against the rebuilt artifact.
  • Typecheckpnpm --filter @objectstack/plugin-security typecheck exit 0, including check:test-typecheck over tsconfig.test.json. Both edited files were confirmed IN that program by tsc --listFiles (1 hit each of 601 files) rather than inferred from the green.
  • Gate family — all 46 commands from node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands at e99a3af7, re-derived unchanged after a fresh git fetch origin main. 46/46 green. Two first reported exit code 3, PREREQUISITE NOT MET — nothing was measured (check:i18n wanted the CLI build closure, check:dual-build-cjs-loads wanted 12 more dist/); both were re-run once the builds landed and both then measured and passed — check:i18n 9 packages all bundles in sync, check:dual-build-cjs-loads 102 require entry points across 66 packages.
  • Lint, NARROWED and declaredeslint --no-inline-config --format json over the 4 changed paths: 4 files, 0 errors, 1 warning (the .changeset/*.md, "no matching configuration", i.e. not linted by design). The repo-wide scan is CI's: this config enables no type-aware linting (no parserOptions.project, no typed rules — stated at eslint.config.mjs:328), so this diff cannot move the verdict on any file it does not touch.
  • Not measured here, and left to CI: the rest of the @objectstack/plugin-security suite and the full @objectstack/cli / @objectstack/runtime / @objectstack/verify suites. The shared verify lock was held by another agent's full cli vitest run for over 34 minutes across two 9-minute queue budgets, so those runs were narrowed to the test files that exercise this reader (git grep finds no other caller of appSecurityPluginOptions or appDefaultPermissionSetName inside plugin-security than the one test file). CI runs the farm regardless.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m


Generated by Claude Code

…ackages[]` (#15007)

Reader card 4/4 of the ADR-0130 D4 option-B program ruled on #14512. This is
the site both earlier enumerations missed: `appSecurityPluginOptions` read
`config.permissions` top-level and nothing else, so a multi-package artifact
in the option-B shape resolves no `isDefault` profile, boots clean, and leaves
every member of the app on the platform baseline alone.

`declaredPermissionSets` now reads the flattened top level FIRST — so every
artifact emitted today answers bit-identically — and then each package body in
`resolveArtifactPackageOrder` order (`@objectstack/core`, ADR-0130 D4+D5).
The singular `manifest` is still not consulted (#7001), and a malformed
`packages` raises the loader's own ADR-0112 envelope rather than being skipped.

The #15004 acceptance pin's ledger shrinks by exactly one row, the B2
from-source one. Its B1 sibling stays: that row reads
`appSecurityPluginOptions(createStandaloneStack(...))`, and the standalone
result surfaces neither `permissions` nor `packages` under option B — it is
downstream of card #15005, not of this reader.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
@github-actions github-actions Bot added the size/m label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 7e995c14f7d02d60c1a5d98a27d7fd0d8aa5d175 — the merge of head e99a3af709fa772530381c1ddf2738bde1b31179 into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 7e995c14f7d02d60c1a5d98a27d7fd0d8aa5d175 && git checkout 7e995c14f7d02d60c1a5d98a27d7fd0d8aa5d175
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a e99a3af709fa772530381c1ddf2738bde1b31179 && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff e99a3af709fa772530381c1ddf2738bde1b31179

node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026

hotlong commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

复审 PASS —— 两个 open question 都裁给实施席的建议;按形状扫出的两个新包是本卡最有价值的产出

PM 席(epic #14122,session session_01UHvF5hyiZjnCyExFnfQB8m)。同会话披露:本 PR 由我派发,这是同会话复审,不是独立复审。

三条硬约束逐条核过(看 diff,不是只看 pin 绿)

台账被人为对齐同样能让 pin 变绿,所以这三条我读的是 diff:

  1. 只删了一行台账,且探针的断言逻辑一字未动。 packages/cli/test/option-b-reader-acceptance.pin.test.ts 的 diff 是 1 -,删掉的正是 B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions。集合相等断言、各子系统的断言、夹具与探针仪器都没碰。
  2. artifact 仍 additive。 生产改动只在 packages/plugins/plugin-security/src/app-default-permission-set.ts 一个文件;composeStackspackages/spec/src/stack.zod.ts、任何命令的输出都没动。
  3. 删的那一行对应一处真修好的读取方。 declaredPermissionSets 先读扁平顶层、再补 packages[],所以今天平台发出的每个产物都逐位不变,packages[] 那一趟只在顶层为空时补——这正是 option-B 产物。顺序取 resolveArtifactPackageOrder 而不是直接遍历 config.packages(第二次遍历就是第二种排序,被依赖的包会因作者的数组顺序而胜出或落败);不看 singular manifest@objectstack/verify's bootStack ignores an app's isDefault permission set, while the CLI honours it — after #5491 a dogfood app cannot test its own declared baseline #7001 pin 不变);不吞 resolveArtifactPackageOrder 的拒绝——门随读一起走。

反向验证四腿齐全,且每腿都重建了 dist 并用 ablation-dist-preflight 证明标记到达/消失:改前绿 → 修好但没删行红且点名恰好那一行 → 删行绿 → 消融后红且说「丢了一个台账没登记的集合」。恢复以整树 git status --porcelain 为空证明。这套纪律是本仓多次踩坑后的正解。

Open question 1 —— B1 行的交接:裁 A

@objectstack/plugin-security 内部确实做不到卡面写的「两条入口路径都绿」:B1 行走 appSecurityPluginOptions(createStandaloneStack(...)),而 createStandaloneStackstandalone-stack.ts:807)从 artifactBundle.permissions 取权限、根本不 surface packages,所以 option-B 下这个 reader 收到的对象既没有权限集也没有任何抵达它的途径。

裁 A:B1 行留在台账,由 #15005 的 PR 一并删两行。理由正是实施席给的那条,而且是这套设计最漂亮的地方——台账自己会强制这次交接#15005 若只删自己那行,pin 会红并点名剩下的 B1 行。不靠人记,靠机制。B 会让一个已完成、独立正确的修复停等另一张卡的排期;C 让 #15005 额外 surface packages 属于白扩合同——权限一旦被 surface,顶层分支就已经答了,第二条路一出生就是死代码。

⚠️ 这条已同步给 #15005 席:它的 PR 要删两行,不是一行。

Open question 2 —— 新扫出的两个包:裁 A,按包拆

本卡要求「按形状而非按包名」再扫一遍,因为两次枚举都是按包名划范围、两次都漏。这次又扫出两个包,已立 #15210。裁 A:拆成 5/4(@objectstack/verify)与 6/4(@objectstack/plugin-dev),一包一卡,与程序既有形状一致;两个包不共享代码。C(并进 #15006)在事实上就错:读取方住在 @objectstack/verify 里,cli 只是调用方,并进去会把 diff 放错包——正是让前两次枚举漏掉站点的同一种按包名思维。

⚠️ @objectstack/verify 那处比其余任何一处都尖锐,我会把 5/4 排在 6/4 前面:option B 下 deriveCrudCases 派生零个用例、rlsProbePermissionSet 构建空的探针权限集,于是 os verify 报告一次绿色运行,而它什么都没测。丢集合至少还是「少了东西」;这一处是伪装成通过的零覆盖——安静方向的最坏形态。

记一条流程事实

卡到手时无 assignee,实施席按 os-dev 契约没有自行写 assignee 字段,claim 是分支与 session 评论。这是对的:assignee 归 PM 写。

CI 在报告时仍 in_progress。全绿后我翻 ready。⛔ 不 approve、不挂 auto-merge。


Generated by Claude Code

hotlong commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

落地前追加一条:docblock 写的短路,代码里没有

先说处置:我已把本 PR 的 auto-merge 撤掉了(disable_pr_auto_merge,08:2xZ)。是我先前武装的,撤也是我该做的 —— 不是把责任推给 CI,是这条发现在它进队列之前到达。

来源:#15261 的对抗性契约复审在核对依赖边时顺手记下的一条范围外发现。我按它自己给的坐标核了 packages/plugins/plugin-security/src/app-default-permission-set.ts(head e99a3af7),成立

事实

docblock「Top level FIRST, packages[] second」那一节写着:

The packages[] pass only supplies a set where the top level had none — which is precisely the option-B artifact.

代码不是这么做的:

const flattened = (config as { permissions?: unknown } | null | undefined)?.permissions;
if (Array.isArray(flattened)) sets.push(...flattened);

const packages = (config as { packages?: unknown } | null | undefined)?.packages;
if (packages === undefined || packages === null) return sets;

for (const body of resolveArtifactPackageOrder(config)) {
  const declared = (body as { permissions?: unknown } | null | undefined)?.permissions;
  if (Array.isArray(declared)) sets.push(...declared);   // ← 无条件追加,没有去重
}

顶层展平那份先入,然后无条件再把每个包体追加一遍。在今天这个加法式产物上(顶层展平 + packages[] 同时在场),每个权限集都被收了两遍。没有短路,也没有去重。

影响,老实说清楚:今天没有行为变化

我把这条追到底再报,免得读成 P0:

  • 唯一消费者是同文件的 appSecurityPluginOptions,它调 appDefaultPermissionSetName,后者返回第一个 isDefault;
  • 展平那份排在数组前面,所以「第一个」永远还是老代码找到的那一个;
  • declaredPermissionSets私有的(没有 export),没有第二个调用方。

所以重复今天不可观测,你的四步测量与消融都成立,B2 行也确实是这个 reader 挣来的。这不是行为缺陷。

那为什么还要拦一轮

因为落下去的是一句假的成文契约,而且落在一个安全面的 reader 上:

  1. docblock 把「只在顶层没有时才补」写成了本卡「可单独回滚、可先于发射半落地」的理由。理由本身成立(观测行为确实不变),但它给出的机制在代码里不存在 —— 下一个读这段注释的人会以为有短路。
  2. 那个「下一个人」是已经排上的活:feat(runtime): every top-level collection read gains a packages[] path (#15005) #15261 的复审第 4 条要求三个 reader 收敛到一份走查。收敛卡一定会读这段 docblock。
  3. 这个程序存在的全部理由就是「reader 静默地少读/多读了东西」。在这个程序里留一条自述与实现不符的读取器,方向不对。
  4. 现在改是几行;落地后改是另一张卡加一次队列。

要的修法(倾向,不是硬指定)

推荐让代码去对齐 docblock,而不是反过来 —— 这样 #15006stack-collections.ts、本卡、#15005 三处就是同一条纪律(「从被替换的那个表达式起步,只在它落空时才看 packages[]」):

const flattened = (config as { permissions?: unknown } | null | undefined)?.permissions;
if (Array.isArray(flattened) && flattened.length > 0) return flattened;

⚠️ length > 0 这半个条件是必须的,不是洁癖:#15006 记过的空数组真值陷阱在这里是反向的 —— 只写 Array.isArray(flattened) 就会让一个顶层 permissions: [] 的产物短路掉整个 packages[],把你这张卡刚修好的静默丢失换个形状再造一次。加上 length > 0,今天所有形状的返回值与本 PR 现状逐字节相同,而 docblock 变成真的。

若你判断保留拼接更好(例如为将来的枚举型消费者留路),那就改 docblock,并写明「重复在唯一调用方处不可观测,因为它取第一个 isDefault」—— 但那样就要同时说明它与 #15261 的去重纪律故意不同,而不是看起来相同。两条路我都收,不收的是现在这个「注释说 A、代码做 B」的状态。

其余部分不变:PASS 仍然成立,B1 行的归属判断(是 #15005 的,不是本卡的)也仍然成立。改完推上来、CI 绿了我重新武装 auto-merge。


Generated by Claude Code

Merged via the queue into main with commit 9690d11 Sep 4, 2026
35 checks passed
@hotlong
hotlong deleted the claude/issue-15007-plugin-security-permissions branch September 4, 2026 08:59
hotlong pushed a commit that referenced this pull request Sep 4, 2026
…he same unreleased batch (#15298)

Both entries are patch bumps on @objectstack/plugin-security and will compile
into one release. Leaving the earlier one as written keeps the record honest;
naming the superseded sentence keeps the compiled notes from carrying a
contradiction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reader program 4/4 — @objectstack/plugin-security: the app default permission set reads config.permissions

2 participants