test(contracts): pin each contract's grant kind so a privilege change cannot pass review quietly - #517
Merged
Conversation
… cannot pass review quietly #496 fixed the runtime consequence: consented grant kinds are frozen at install, so widening a shipped contract's `providerGrant` no longer reaches installs that consented before the change. This is the other half — it puts the mistake in front of a reviewer instead of letting it arrive as a warning on somebody's host weeks later, which keeps #496's runtime mismatch path near-unreachable rather than merely survivable. A pinned ref -> grant-kind map, asserted against the live table as one equality so a changed kind, an unpinned new contract and a removed one all show up with the whole map in the diff. Every change to the table fails this test, including legitimate ones, and that is the design: the map has to be edited by hand in the same commit, which is the moment someone reads what they are about to change. The three cases were measured rather than assumed: - editing `providerGrant.kind` on a shipped ref, or adding a grant to a ref that had none, fails the equality — the hazard itself; - adding a new contract fails with the new ref in the diff, which is legitimate and just needs pinning; - bumping a version fails twice, and the second failure is the valuable one: it reports that the old ref is pinned but gone, because every install that consented to it now holds a recorded privilege that no longer resolves and silently loses the access. That is precisely the consequence a version bump should make someone confront. The header documents that measured behaviour rather than my first draft's claim that version bumps and new contracts pass cleanly — they do not, and saying so is the difference between a guard someone trusts and one they route around. Also asserts that any contract carrying a grant has a non-empty operator-facing label and risk. The pinning protects the kind; those two strings are what an operator actually reads when consenting, so an empty one makes the consent row meaningless even with the kind unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vck5KSX2CLxhohx14nb5Sh
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
1 participant
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.
Follow-up to #516 (#496), kept as its own reviewed commit rather than riding on that PR.
What it guards
#516 fixed the runtime consequence: an install's consented grant kinds are frozen at consent time, so widening a shipped contract's
providerGrantno longer reaches installs that consented before the change.This is the review-time half. A pinned
ref -> grant kindmap, asserted against the liveCONTRACTStable as a single equality, so a changed kind, an unpinned new contract and a removed one all surface with the whole map in the diff.Together they mean the runtime mismatch path in #516 should be near-unreachable rather than merely survivable — the warning it logs becomes a backstop, not the primary defence.
Every table change fails this test, on purpose
The pinned map must be edited by hand in the same commit. That edit is the moment someone reads what they are about to change. All three cases were measured, not assumed:
providerGrant.kindchanged on a shipped refPINNED; bump the version+ "demo@1": nullin the diff. Legitimate; pin itbackup@1→backup@2)That second failure on a version bump is worth spelling out: it reports that
backup@1is pinned but gone, because every install that consented tobackup@1now holds a recorded privilege that no longer resolves — they silently lose the access. That is exactly the consequence a version bump ought to make someone confront, and it only became visible because #516 started recording privileges per install.I got the header wrong first. My initial draft claimed version bumps and new contracts pass cleanly. They don't, as the table above shows. I corrected the comment to the measured behaviour, because a guard whose documentation overpromises is one people route around the first time it surprises them.
Third assertion
Any contract carrying a grant must have a non-empty operator-facing
labelandrisk. The pinning protects the kind; those two strings are what an operator actually reads when consenting, so an empty one makes the consent row meaningless even with the kind unchanged.Verification
Mutation-tested against both hazards and both legitimate edits — results in the table above, each run against the real table with the change applied and reverted. Full gate green: 1392 server / 377 web / 286 CLI, typecheck ×2, lint, build. New file, so no conflict with #516's edits to
contracts.test.ts.🤖 Generated with Claude Code
https://claude.ai/code/session_01Vck5KSX2CLxhohx14nb5Sh