fix(core): enforce universal-policy skip legitimacy, record it on receipts - #61
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed5aa84381
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
wonkwonlee
added a commit
that referenced
this pull request
Aug 7, 2026
…parseable Addresses PR #61 review feedback: - policyCoverage is now optional on ChangeReceiptSchema. Making it required broke `changesafe verify` and ledger reads for every receipt issued before this field existed (v0.4.1 and earlier) with a schema-parse error before the hash/signature check ever ran. `createReceipt` still always sets it, and canonicalize drops undefined properties, so a legacy receipt's hash is unaffected by the field's absence. - SkipReplacement drops the "out-of-band" variant entirely. A replacement that names a non-mechanical process produces no finding, so the gate would pass with a genuine verdict gap rather than merely an honestly-labeled one — every skip must now name a real domain policy that actually ran. - validateSkips rejects a replacement whose policyId collides with a universal policy id: policyOrder's filter removes every occurrence of a skipped id, so a same-named domain policy would vanish from policyCoverage despite having actually evaluated and produced a finding. Full gate green: lint, typecheck, build, build:cli, 1174 tests, scenario check, gallery, and `changesafe verify` against the published v0.1.0 receipt. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…eipts An adapter could declare skippedUniversalPolicies for any of the five universal policies, not just the two whose shape assumes a simulated-state domain — and the only check was in the app's runtime registration, not published core, so `changesafe gate` against a third-party adapter got no enforcement at all. replacedBy was also a bare string, so a skip's stated replacement was never verified to exist. evaluatePolicies and policyOrder now both validate every skip: only ROLLBACK_COMPLETE and VERIFICATION_REQUIRED may ever be skipped, never twice each, and a "domain-policy" replacement must name a policy the adapter actually declares. SkipReplacement is a typed union instead of prose. Every receipt now carries policyCoverage (computePolicyCoverage), so a verifier reads what ran and what was skipped from the signed payload rather than trusting the adapter's source. Terraform's VERIFICATION_REQUIRED skip now names a real domain policy (PLAN_CONTEXT_REQUIRED — WARN when a destructive change carries no PR or commit context) instead of "the pull request review" as prose. CORE_POLICY_VERSION and TERRAFORM_POLICY_VERSION both bump accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…parseable Addresses PR #61 review feedback: - policyCoverage is now optional on ChangeReceiptSchema. Making it required broke `changesafe verify` and ledger reads for every receipt issued before this field existed (v0.4.1 and earlier) with a schema-parse error before the hash/signature check ever ran. `createReceipt` still always sets it, and canonicalize drops undefined properties, so a legacy receipt's hash is unaffected by the field's absence. - SkipReplacement drops the "out-of-band" variant entirely. A replacement that names a non-mechanical process produces no finding, so the gate would pass with a genuine verdict gap rather than merely an honestly-labeled one — every skip must now name a real domain policy that actually ran. - validateSkips rejects a replacement whose policyId collides with a universal policy id: policyOrder's filter removes every occurrence of a skipped id, so a same-named domain policy would vanish from policyCoverage despite having actually evaluated and produced a finding. Full gate green: lint, typecheck, build, build:cli, 1174 tests, scenario check, gallery, and `changesafe verify` against the published v0.1.0 receipt. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
wonkwonlee
force-pushed
the
wonkwonlee/next-phase-planning
branch
from
August 7, 2026 06:33
9189e0c to
22af73a
Compare
9 tasks
wonkwonlee
added a commit
that referenced
this pull request
Aug 7, 2026
* chore(release): prepare v0.5.0 Bumps all nine workspace packages and every internal @changesafe/* range from 0.4.1 to 0.5.0. The currently-published 0.4.1 core carries a real skip-legitimacy gap (PR #61) and a broken Kubernetes eval path (PR #60); this is not a routine version bump but the release that gets those fixes into published hands. No breaking changes: policyCoverage is an additive optional receipt field, Terraform's new PLAN_CONTEXT_REQUIRED policy replaces a skip that already existed, and Kubernetes eval support only adds a domain --domain kubernetes could not previously use. CORE_POLICY_VERSION and TERRAFORM_POLICY_VERSION move independently of the package semver, as always. This prepares the release; it does not publish. Publishing still requires an explicit owner-authorized tag and GitHub Release. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(release): keep v0.5.0 pinned references at v0.4.1 until actually published Fixes review feedback on the v0.5.0 prep PR: the example GitHub Action workflow, README, and docs/LAUNCH.md all pointed wonkwonlee/ChangeSafe@v0.5.0 before that tag exists, so a workflow copied from the example would fail to resolve the action rather than run the gate. Reverted those to v0.4.1 (still published and valid) and reworded "canonical" claims to say v0.5.0 is prepared but not yet published, matching what MEMORY.md's release-status section already said correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
Code review found that
skippedUniversalPolicies(packages/core/src/domain.ts) let a domain adapter skip any of the five universal policies, and the only legitimacy check lived in the app's runtime registration (features/domains/runtime.ts), not published core — sochangesafe gateagainst a hand-written or third-party adapter got zero enforcement. A probe adapter that skipped all five universal policies passed throughevaluatePoliciescleanly with 0 findings andLOWrisk, despite the proposal containing an injection attempt, invented evidence, no rollback, and no verification.replacedBywas also a bare string, so nothing verified a skip's claimed replacement actually existed — Terraform'sVERIFICATION_REQUIREDskip named"the pull request review", which is prose, not a policy.SKIPPABLE_UNIVERSAL_POLICY_IDS— onlyROLLBACK_COMPLETEandVERIFICATION_REQUIREDmay ever be skipped;PATCH_SCHEMA,BLAST_RADIUS,UNTRUSTED_INSTRUCTIONare structurally answerable by every domain and no adapter may skip them.evaluatePoliciesandpolicyOrderboth call a sharedvalidateSkipsand throw for an illegitimatepolicyId, a duplicate skip, or a"domain-policy"replacedBythe adapter doesn't actually declare — enforced in core itself, so a third-party adapter reaching the gate directly gets the identical guarantee the app's registration path gets.SkipReplacementis a typed discriminated union ({kind:"domain-policy",policyId}|{kind:"out-of-band",process}) instead of a bare string.policyCoverage(computePolicyCoverage): the exact ordered policy ids that ran plus what was skipped and why, so a verifier reads this from the signed payload instead of trusting the adapter's source.VERIFICATION_REQUIREDskip now names a real domain policy,PLAN_CONTEXT_REQUIRED(WARNs when a destructive change carries no PR/commit context), replacing the "pull request review" prose.CORE_POLICY_VERSION→core-v0.2.0,TERRAFORM_POLICY_VERSION→terraform-v0.2.0.expectations.jsonupdated for the new policy;n/o/tgot realistic PR context added to their fixtures to preserve their original MEDIUM/HIGH-risk narratives;docs/SCENARIOS.mdregenerated.Test plan
npm run lintnpm run typechecknpm run buildnpm run build:clinpm test— 1173 passed, 4 skippednode packages/cli/dist/changesafe.js scenario checknode packages/cli/dist/changesafe.js scenario gallery --checktests/unit/policies.test.ts),PLAN_CONTEXT_REQUIREDPASS/WARN behavior (packages/domain-terraform/tests/terraform.test.ts)🤖 Generated with Claude Code