refactor(plugin-sharing,plugin-audit): annotate enforcement on the full ExecutionContext and delete the double-casts (#7136) - #7140
Conversation
…ll ExecutionContext (#7136) Consumer half of #6523. The contracts converged onto the complete resolveAuthzContext envelope (#6206 ruling: no per-site subset contracts); the implementations behind them still named the six-field SharingExecutionContext, so nothing they could read had widened. - 27 enforcement parameters across SharingService, SharingRuleService and plugin-audit's comment-access gates now declare ExecutionContext, plus the two return types that produce the contexts feeding them. - exec-context-seam.testkit.ts returns the resolved envelope with no cast, deleting the `as unknown as` double cast on the value tests are meant to trust. - SharingRuleService's SYSTEM_CTX is typed as the envelope and passed as itself, retiring `SYSTEM_CTX as any` at all 11 call sites. - `(context as any).userId` / `.tenantId` in SharingService now read declared fields. Kept and documented in place: __readScope / __writeScope (middleware-private keys, not envelope fields) and organizationId (#5858 / check:org-identifier, held out of #7070 on purpose). No runtime behaviour changes — the values were always complete, the damage was type-side. Parameters only widen what they accept. Because a re-narrowed annotation would compile, ship and pass every test, exec-context-annotation.pin.ts pins the convergence at compile time via excess-property checks on envelope-only fields, with negatives so a parameter erased to `any` cannot pass either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 2 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
The measured count was taken after the new doc comment — which itself quotes the string `SYSTEM_CTX as any` — had been inserted, so the doc comment counted itself. On origin/main the erasure appears at exactly 10 call sites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
|
PM review — PASS. Marked ready and enqueued ( The evidence, re-measured by this seat rather than read off the reportThis card had no user-visible defect and no compiler pressure, so a green typecheck proves nothing — it was green before the branch existed. The load-bearing readings, each re-run here against
The specimen the card names — Reverse verification — the honest shape, accepted as suchThe template's before-green/after-red does not fit a type-only change, and the author did not force it. Prediction stated first: reverting should leave every runtime test green and only the new compile-time pin should move. Measured: typecheck red on exactly the six lines the pin was written to fail on (4× TS2353 on the positives, 2× TS2339 on the seam read, exit 2), and the same reverted tree still passed 410 + 137 tests. No TS2578, so all four negative cases stayed satisfied in both states. That is the card's own premise measured rather than asserted: 547 runtime tests cannot distinguish this change from its reversal. Which is precisely why The
|
2 participants
Fixes #7136
Consumer half of #6523 / PR #7068, identity lane of the #7070 split. The contract half converged 36 signatures onto the full
ExecutionContext(the #6206 ruling: enforcement adjudicates on the wholeresolveAuthzContextenvelope, never a per-site subset). The implementations behind those contracts still annotated their own parameters withSharingExecutionContext, the six-field shape the contracts used to name — so nothing they could actually read had widened, and the casts that narrowness forced were all still there.What changed
SharingExecutionContextrefs, before → afterpackages/plugins/plugin-sharing/src/sharing-service.tspackages/plugins/plugin-sharing/src/sharing-rule-service.tspackages/plugins/plugin-sharing/src/exec-context-seam.testkit.tspackages/plugins/plugin-audit/src/comment-access-hooks.ts27 enforcement parameters now declare
ExecutionContext, plus the two return types that produce the contexts feeding them. The casts that go with them:exec-context-seam.testkit.ts:106— the specimen this card names. The helper resolved a REALresolveAuthzContextenvelope and then forced it into the narrow type:return { ...authz, isSystem: false } as unknown as SharingExecutionContext;. It now returns what it resolved. That matters beyond tidiness: this seam exists ([#5852 producer 半边]resolveOwnerScopeIds按权威字段填充 hierarchy resolver context + 补「resolver 收到的 org 非空」断言 #5859) so a drift in the resolver's output breaks the tests that trust it, and a double cast on that value is the seam lying about itself.SYSTEM_CTX as anyat all 10 of its call sites insharing-rule-service.ts— the system context is typed as the envelope and passed as itself. An erasure on an enforcement input switches checking off for the whole argument, not just for the readonly-array mismatch that provoked it.(context as any)reads ofuserId/tenantId— 3 insharing-service.ts, 3 insharing-rule-service.ts— now reading declared fields.Two cast families are deliberately kept, and are now documented where they sit rather than left looking like oversights:
__readScope/__writeScopeare not fields ofExecutionContext. They are private keys plugin-security's middleware stamps onto the context it forwards (sc.__readScope = …,security-plugin.ts). Widening the annotation does not make these deletable, and the fix is not to declare them on the envelope — that would publish a middleware seam as authorable, client-supplied vocabulary.organizationIdis not on the envelope at all. That spelling has its own history ([#5852 契约半边]HierarchyScopeContext未声明organizationId/tenantId哪个权威 —— producer 只填一个、consumer 只读另一个,两边都「符合契约」 #5858 /check:org-identifier) and was held out of finding: consumer half of #6523 — three plugin implementations still annotateSharingExecutionContext, so(context as any).postureis still needed to read a field the contract now declares #7070 on purpose, so the three reads of it stay cast. Only thetenantIdhalf of those expressions lost its cast, which makes the asymmetry the visible marker of which of the two names the contract actually knows.plugin-audit'scallerContextgets the widened annotation but keeps its five-field projection — see "deliberately not done" below.Verification — and why a green typecheck is not it
This card has no user-visible defect and no compiler pressure in either direction: the values were always complete at runtime (this family's damage was type-side), and PR #7068 already measured the dependents-direction typecheck green. The typecheck passed before this branch existed, so it is a regression check here and not evidence. The evidence is the cast counts and the pin.
Measured on this branch point (
0fd855624, not the card's445a0c2— the four per-file counts re-measured identical).SharingExecutionContextreferences, before → after (grep -cover the four files): 21 → 0, 8 → 0, 3 → 0, 2 → 0.Context casts, before → after (
grep -oF | wc -lsummed over the same four files):as unknown as SharingExecutionContextSYSTEM_CTX as any(context as any)— total occurrencesThe 6 surviving code reads are exactly the two families held out on purpose:
__readScope/__writeScope(3) andorganizationId(3). The 6 deleted ones were alluserId/tenantId— fields the envelope declares.Gates (regression checks, not evidence of the change):
The mechanical guard, and whether it earns its weight
Yes — because without it nothing detects a regression. A re-narrowed annotation compiles, ships and passes every test in these packages, which is exactly why this card had to be queued deliberately rather than discovered.
packages/plugins/plugin-sharing/src/exec-context-annotation.pin.tshands each enforcement parameter a fresh object literal naming envelope-only fields (posture,accessible_org_ids,org_user_ids). TypeScript's excess-property check rejects those the moment a parameter narrows back. That is the only direction available: a@ts-expect-errorasserting the reverse would be unsatisfied and fail the build, because a narrow context IS structurally assignable to a wide parameter — the boundarySharingExecutionContext's own doc block already records.It also carries four negative cases, because a parameter erased to
anywould swallow every positive just as happily.posture: 'SUPERUSER',userId: 42,accessible_org_ids: 'org_a'andorganizationId:must all still fail.It is a
.pin.ts, not a*.test.ts, and that is load-bearing:packages/plugins/plugin-sharing/tsconfig.jsonexcludes**/*.test.ts(a measuredTEST_DEBTof 3 inscripts/check-type-check-coverage.mjs), sotsc --noEmitnever reads this package's test files. A pin written there would be a phantom check that stays green however it is broken — AGENTS.md'sPINS_CHECKEDtrap, the same hole #6212 measured on driver-mongodb. Same convention as the existingpackages/objectql/src/register-object-authored-shape.pin.ts, and imported by nothing, so tsup (entrysrc/index.ts) never bundles it.Reverse verification
Prediction, recorded before running it: this change has no runtime behaviour, so reverting it would leave the entire suite green — the honest expectation here is not a red — while the pin, and only the pin, would go red. Both halves measured, by taking the fix out with
git checkout origin/main --on the four files and keepingexec-context-annotation.pin.ts:So: 547 runtime tests cannot tell this change from its own reversal, which is the card's premise measured rather than asserted, and the reason a fabricated red would have been the wrong thing to report. The pin fails on all six lines it was written to fail on. No
@ts-expect-errorbecame unsatisfied under the revert (tsc reports no TS2578), so the negative cases keep asserting in both states rather than turning into noise.Deliberately not done
plugin-approvalsandplugin-reports— the services-lane half (Re-annotate plugin-approvals + plugin-reports onto the fullExecutionContextand delete the now-redundant casts (services half of #7070) #7135), a different seat and a disjoint file set.packages/servicesis untouched.SharingExecutionContexttype stays. Its removal rides whichever of the two split halves lands second; Re-annotate plugin-approvals + plugin-reports onto the fullExecutionContextand delete the now-redundant casts (services half of #7070) #7135 is still open andpm:queueas of this PR, and finding:SharingExecutionContext是同族第四个窄 enforcement 契约类型(sharing / approval / report 三个服务共用),#6206 裁决的「不留 per-site 子集」默认尚未覆盖它 #6523 left the type exported precisely to keep those files compiling. Deleting it here would break the sibling half mid-flight. The coordination point (including theplugin-sharing/src/index.tsre-export, which is a public-surface removal) is recorded on Re-annotate plugin-approvals + plugin-reports onto the fullExecutionContextand delete the now-redundant casts (services half of #7070) #7135 for whichever seat lands second.callerContext's five-field projection inplugin-audit— filed as plugin-audit'scallerContextrebuilds a 5-field subset of the execution envelope, droppingonBehalfOfbefore the sharing gates that are documented to fail closed on it #7141. Widening its annotation is type-side and inert; forwardingexecwhole is a RUNTIME change with a real enforcement consequence (it dropsonBehalfOfbefore a probe documented to fail closed on it, and forwarding it whole would leak one object's access depth into another object's owner-match), so it is filed rather than folded in.Commits
Two, deliberately not squashed: the change, then a correction to a number in it. The
SYSTEM_CTXcast count was first written as 11 because the measurement was taken after the new doc comment — which itself quotes the stringSYSTEM_CTX as any— had been inserted, so the comment counted itself. Onorigin/mainthe erasure appears at exactly 10 call sites.Generated by Claude Code