fix(spec,runtime): 槽契约账本越过 CoreServiceName，/security 不再把未校验输入交给安全服务 (#4127) - #4202
Merged
os-zhuang merged 2 commits intoJul 30, 2026
Merged
Conversation
…eName`, and `/security` stops passing unvalidated input to the security service (#4127) Batch 3 of the #4127 gate, after #4168 (`getService`) and #4176 (`resolveService`). Three slots — `security`, `shareLinks`, `objectql` — each had a written contract, a provider registering them, and call sites already inside the contract. The only missing link was that the slot name was not a `CoreServiceName` member, so nothing could connect them and all three sat behind `as any`. The ledger extends past the enum rather than the enum growing. The two answer different questions, and conflating them is what left these untyped: `CoreServiceName` answers "what happens at boot when this slot is empty?" — it sits beside `ServiceCriticality` and drives startup orchestration and discovery, so adding a member changes runtime behaviour and is effectively permanent. The ledger answers "what shape occupies this slot?" — pure type information. These three need only the second, so `ServiceSlotContracts extends CoreServiceContracts` adds them there and `resolveService` keys on `keyof ServiceSlotContracts`. Zero runtime effect. If one is later promoted to a genuine core service, its entry moves up and nothing else changes. Evidence before an entry, as always: `plugin-security` registers `security` and `ISecurityService`'s own doc names that registration; `plugin-sharing` registers `ShareLinkService`, which declares `implements IShareLinkService`; and `objectql` is an ALIAS of `data` — `packages/objectql`'s plugin registers the same instance under both names two lines apart, so one object was resolving as `IDataEngine` through one name and `any` through the other. `protocol` (22 call sites) and `mcp` have no written contract and stay unmapped. Turning it on found four things, all on the `/security` admin surface: 1. Request input reached the security service unvalidated. `?status=` was `String(query.status)` — any string — handed to a method whose contract declares exactly three values, and from there into the query's `where` clause. Not an injection (the `where` is structured, never interpolated), but `?status=garbage` matched no row and returned an empty list, which reads as "there are no suggestions" rather than "that is not a status". Now a 400. 2. A test pinned that bug as expected behaviour. The existing case asserted `status: 'open'` — not one of the three declared values — reached the service and returned 200. It proved the delegate carried A FILTER and nothing about that filter being a status. Same shape as batch 1's `auth.handler` mocks: coverage in appearance, a wrong contract in substance. 3. and 4. Two writes could not prove they had a caller. `confirmAudienceBindingSuggestion`/`dismissAudienceBindingSuggestion` declare `callerContext: SecurityContext` non-optionally — deliberately, since the read beside them declares it optional — and the domain passed a possibly-`undefined` execution context. This was NOT a live hole, and the distinction matters: with no execution context `shouldDenyAnonymous` already denied, because it sees no `userId`/`isSystem` and its allowlist arm needs a non-empty `path` this seam never passes, so it fell through to `return true`. What it never did was narrow `ec` itself — it only read `ec?.userId`. Checking `ec` directly is behaviour-preserving and makes the invariant legible to the compiler and the next reader. The `?status=` rejection is the one BEHAVIOUR CHANGE: an unknown status was a silent empty list and is now a 400 naming the accepted values. The accepted set is a `Record` keyed on the contract type, so adding a status to the contract leaves a key missing and renaming one leaves a key excess — either fails to compile, where a plain array would have drifted silently. Refs #4127 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 113 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…ecurity/suggested-bindings (#4127) The 400 introduced alongside the slot-contract ledger is observable API behaviour, and this page is where the endpoint is documented. Both existing examples already used valid statuses, so nothing here was invalidated — what was missing is that the filter has a fixed set at all, and what an unknown value now does instead of quietly returning an empty list. Refs #4127 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw
os-zhuang
marked this pull request as ready for review
July 30, 2026 15:24
os-zhuang
deleted the
claude/dispatcher-storage-upload-typeerror-0exbcb-b3
branch
July 30, 2026 15:25
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
… which found the project-membership gate not gating (#4127) (#4214) Batch 4 of the #4127 gate. #4168/#4176/#4202 made a slot lookup return the slot's contract. Nothing protected that: an `any` annotation on the RESULT switches the checking back off for that call site, silently, with no test failing and no visual difference from code that has it. Three such sites already existed and were found by grep — the same unrepeatable sweep this work replaced. The rule bans `: any` / `as any` on a `resolveService` / `getService` / `getRequestKernelService` result. Slots with no written contract (`protocol`, `mcp`, `kernel-resolver`, `scope-manager`) are exempted BY NAME, CENTRALLY, in eslint.config.mjs — not by inline disables, because `pnpm lint` runs `--no-inline-config` and ignores those on purpose. The effect is the one worth having: a deliberate gap is a reviewed line in one file, a careless one is a build failure, and they stop looking identical in the code. ITS FIRST RUN FOUND A LIVE FAIL-OPEN. `enforceProjectMembership` read the session as `authService?.api?.getSession?.(...)` with no `getApi()` fallback — the only one of the codebase's three `.api` readers without it. `plugin-auth` registers `AuthManager`, which has NO `.api` member at all. So the read yielded `undefined`, `userId` stayed unset, and the function returned at its "anonymous — upstream auth will decide" line BEFORE ever querying `sys_environment_member`. A signed-in non-member passed the gate, on every deployment with project scoping on — which is where the flag defaults to true. Anonymous callers were still denied elsewhere (#2567/#3963), so this was specifically the signed-in non-member case. The existing test for that gate mocked auth as `{ api: { getSession } }` — the legacy shape the shipped provider does not have — so it was green throughout. That is the FOURTH test in this work line found encoding a contract nobody implements, after batch 1's three `auth.handler` mocks and batch 3's `status: 'open'`. The new test uses the `getApi()` shape and fails against the pre-fix code. Also found by the rule, all the same #4127 shape (implemented, called, undeclared) and all now declared: IAuthService gains `api`, `getApi`, `isAuthGateActive` and `verifyMcpAccessToken`; IMetadataService gains `load` and `loadDiagnosed`. `getApi`'s return type is the EVIDENCED SUBSET — `getSession({headers})` and the three fields callers read — not a re-declaration of better-auth's handle, which belongs to that library. And the pattern's real root: the lookup facade returning `any` was re-declared in THREE places. Batches 1-3 typed `DomainHandlerDeps` and left `ActionExecutionDeps` and resolve-execution-context's `ResolveOptions` still saying `any` — so the copy that stayed untyped was the way around all the others, and it is where the auth reads lived. All three are typed now. Completing the interface: `getRequestKernelService` gets the same overload split (its one caller resolves the same `objectql` slot the `resolveService` fallback beside it does, so the two arms of one expression had different types), and share-links' `getEngine` loses a `Promise<any>` return annotation — a THIRD erasure syntax after `: any` and `as any`, and one this AST rule cannot see. That residual is documented in the config. `getObjectQL` STAYS `any`, deliberately, with the reason recorded: it exists to reach ObjectQL's surface beyond IDataEngine (`registry`, `executeAction`), which has no contract. Typing it IDataEngine would be the comfortable-looking lie. The auth and metadata contract doc pages are brought back in step with the interfaces — the auth page still called itself "intentionally minimal" while missing six members, two of them from batch 2. Refs #4127
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
2 participants
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.
Refs #4127。#4127 gate 的第三批（前两批 #4168
getService、#4176resolveService）。security、shareLinks、objectql三个槽，契约写了、提供方注册了、调用点也在契约内 —— 唯一断掉的环节是槽名不是CoreServiceName成员，于是没有任何东西能把它们连起来，三处全部躺在as any后面。1. 让账本越过枚举，而不是让枚举变大
这是本 PR 的核心决定。两者回答的是不同问题，混为一谈正是这些槽没类型的原因：
CoreServiceNameServiceCriticality，驱动启动编排与 discovery这三个槽只需要第二种。所以
ServiceSlotContracts extends CoreServiceContracts在账本侧追加，resolveService改用keyof ServiceSlotContracts作键。枚举一字未动。这不是永久分裂：将来若真要把其中之一升为核心服务（连同 criticality 语义），条目上移进
CoreServiceContracts即可，其余不变 —— 测试里有一条断言专门守着这个迁移点。每条条目仍然只在有证据处写：
security——plugin-security注册，且ISecurityService自己的注释就写着ctx.registerService('security', …)shareLinks——plugin-sharing注册ShareLinkService，后者显式implements IShareLinkServiceobjectql—— 是data的别名：packages/objectql的 plugin 把同一个实例注册在两个名字下，相隔两行。同一个对象，经data解析为IDataEngine，经objectql解析为anyprotocol（22 处）和mcp没有写过契约，保持未映射。2. 开机后抓出四处，全在
/security管理面① 请求输入未经校验进入安全服务
?status=是String(query.status)—— 任意字符串 —— 交给一个契约声明恰好三个值的方法，再从那里进入查询的where子句。不是注入（
where是结构化的，从不拼接字符串），但?status=garbage匹配不到任何行、返回空列表 —— 这读起来是「没有建议」，而不是「你这个过滤器不是一个状态」。现在返回 400。② 一个测试把这个 bug 钉成了预期行为
现有用例断言
status: 'open'（不在三个合法值里）会抵达服务并返回 200：它证明的是 delegate 传递了某个 filter，对「这个 filter 是不是一个 status」一无所证。和 batch 1 的
auth.handlermock 同一形状 —— 外观是覆盖，实质是错误契约。这处更糟一点：它锁定的是「未校验输入进入安全服务」。③④ 两个写操作无法证明自己有调用者
confirmAudienceBindingSuggestion/dismissAudienceBindingSuggestion的callerContext: SecurityContext是非可选的 —— 这是刻意的，旁边那个读操作声明的就是可选 —— 而 domain 传的是可能为undefined的 execution context。改成直接检查
ec是行为等价的，只是让这个不变量对编译器和下一个读者都可见。?status=的拒绝：未知状态从「静默空列表」变成「400 并列出接受的值」。接受集合是一个以契约类型为键的
Record，所以契约新增状态会导致这里缺键、重命名会导致多键 —— 两种情况都编译失败。换成普通数组就会静默漂移，而那正是这条工作线要消灭的失效模式。验证
@objectstack/runtime@objectstack/spectsc --noEmitpnpm lintcheck:*门禁下一步
只剩真正没有契约的槽：
protocol（22 处）、mcp、kernel-resolver、scope-manager。给它们写契约是实打实的设计工作，不适合顺手做 —— 建议单独立项，逐个按「实现是什么形状」而非「调用点碰巧调了什么」来写。🤖 Generated with Claude Code
https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw
Generated by Claude Code