Skip to content

Commit ecceb38

Browse files
sliamh11claude
andauthored
fix(wardens): distinguish OPA ledger staleness from a real review gap (LIA-535) (#1161)
## Summary - `guardrails.rego`'s file-level `default decision` message ("guardrails policy produced no valid decision") fired identically for a genuine missing-review-attestation denial and for a transient OPA ledger generation mismatch (disk/OPA desync, self-heals within 5 min via LIA-533's periodic job) — no hint to a developer that the cause might be infra staleness, not "you need a SHIP." - New `decision` rule fires a distinct, actionable message specifically for the generation-mismatch case, scoped to `git.commit`/`file.write` (the two operations whose decision bodies all require `supported`). `attestation.verify` is explicitly excluded — it already has its own composite deny message, and including it would create a real Rego `eval_conflict_error` (verified by mutation testing). - Folds in a one-line fix to an adjacent, already-stale ticket citation (was `LIA-534`, corrected to `LIA-539` — `LIA-534`/`LIA-531` are both Done/merged; `LIA-539` is the actual open ticket gating `main-attestation-backstop`'s activation, independently re-verified against live Linear state rather than the ADR, which itself still lags on this point). ## Test plan - [x] `opa test -v --ignore="*.schema.json" scripts/warden_policy/policy` → 68/68 pass (65 existing + 3 new) - [x] `opa check --strict scripts/warden_policy/policy` → clean - [x] `python3 -m pytest scripts/warden_policy/tests` → 303 passed, 1 skipped, 65 subtests, zero regressions - [x] Mutation-tested the exclusion guard: dropping it entirely reproduces a real `eval_conflict_error`; the new test catches it - [x] plan-reviewer: 3 rounds (round 1 REVISE — guard was too broad, `!= "attestation.verify"` would misclassify unrecognized operations as staleness, caught via a live failing-test reproduction; round 2 REVISE — wrong ticket citation; round 3 SHIP after independent live-Linear + doc-timestamp re-verification), Claude + GPT co-gate - [x] code-reviewer SHIP (Claude + GPT; GLM `COULD_NOT_RUN` on quota exhaustion, fails open per established precedent) - [x] ai-eng-warden SHIP (gate-spec file; confirmed reason-string-only change, no allow/deny regression, no attacker-controllable input, accurate remediation hint) - [x] verification-gate: 4 rounds — caught 2 real comment-accuracy defects (a misleading counterfactual claim in a test comment, a stale-doc pointer that would lead a reader back into the exact misread `LIA-539` was filed to prevent, plus one wrong-direction cross-reference), all fixed and re-verified; final SHIP 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent ccc5ff5 commit ecceb38

2 files changed

Lines changed: 69 additions & 4 deletions

File tree

scripts/warden_policy/policy/guardrails.rego

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,41 @@ default decision := {"allow": false, "reason": "guardrails policy produced no va
1313
# `supported` gates every non-default decision below. Found missing by adversarial plan-review:
1414
# an unrecognized root schema_version, an unrecognized input contract_version, or OPA serving a
1515
# stale ledger snapshot (generation mismatch, e.g. after a failed/ambiguous PUT even though OPA
16-
# itself is reachable) must all fall through to the default deny, not be silently accepted by a
17-
# rule that only checked the individual attestation record's own schema_version.
16+
# itself is reachable) must all fall through to a deny, not be silently accepted by a rule that
17+
# only checked the individual attestation record's own schema_version. For `git.commit`/
18+
# `file.write` operations specifically, a generation mismatch gets its own dedicated message
19+
# (see the decision rule right below `supported`, LIA-535) rather than the fully generic default
20+
# at the top of this file -- every other `not supported` cause, and every other/unrecognized
21+
# operation, still falls through to that generic default unchanged.
1822
supported if {
1923
input.contract_version == 1
2024
data.warden_attestations.schema_version == 1
2125
data.warden_attestations.generation == input.expected_generation
2226
}
2327

28+
# Distinguishes ledger staleness (self-healing infra desync, LIA-533) from every other
29+
# `supported`-gated denial and from the fully generic default above (LIA-535). Scoped to exactly
30+
# the two operations whose decision bodies all require `supported` (git.commit, file.write) --
31+
# NOT `input.operation != "attestation.verify"`, which would also swallow any unrecognized/future
32+
# operation value and misreport it as ledger staleness (caught by plan-review round 1). Excludes
33+
# attestation.verify specifically because that operation already has its own dedicated composite
34+
# deny message (below) covering "no SHIP found ... or OPA snapshot stale/unsupported" for BOTH
35+
# ledgers -- letting this rule also match attestation.verify inputs would create a genuine
36+
# multi-value `decision` conflict (OPA eval_conflict_error) whenever both conditions hold
37+
# simultaneously, which they can.
38+
decision := {
39+
"allow": false,
40+
"reason": sprintf(
41+
"OPA ledger generation stale (expected %d, got %d) -- run `python3 scripts/warden_attest.py sync` or wait for the next self-heal tick (LIA-533)",
42+
[input.expected_generation, data.warden_attestations.generation],
43+
),
44+
} if {
45+
input.operation in {"git.commit", "file.write"}
46+
input.contract_version == 1
47+
data.warden_attestations.schema_version == 1
48+
data.warden_attestations.generation != input.expected_generation
49+
}
50+
2451
enrolled if data.warden_attestations.config.enforced_repos[input.repo_id].enabled
2552

2653
# `latest["code-review"]` is a pointer, not a trusted authority on its own -- every field the
@@ -234,8 +261,11 @@ decision := {
234261
# gpt/glm co-gate backends -- permanent, by-design limitation, disclosed here and in the allow
235262
# reason string. No signing, no runner isolation -- same-host trust, same accepted-risk framing
236263
# as git-level-hard-backstop-design.md §3.3. A DENY is authoritative-to-block; an ALLOW means
237-
# "evidence found," never "fully reviewed." **DOES NOT ACTIVATE UNTIL LIA-534's gate-wiring lands
238-
# too -- see git-level-hard-backstop-design.md §3.6's corrected three-part precondition.**
264+
# "evidence found," never "fully reviewed." **DOES NOT ACTIVATE UNTIL LIA-539 (the credential-
265+
# separation implementation) lands -- LIA-531 merged as DESIGN ONLY and does not clear this gate
266+
# on its own; confirmed live via LIA-539's own Linear description, not via
267+
# git-level-hard-backstop-design.md §5, which is stale on this specific point as of this writing
268+
# (still cites LIA-531 alone -- needs its own follow-up, out of scope here).**
239269

240270
cc_supported if {
241271
input.contract_version == 1

scripts/warden_policy/policy/guardrails_test.rego

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,24 @@ test_deny_stale_opa_generation_mismatch if {
167167
not decision.allow with input as inp with data.warden_attestations as base_attestations
168168
}
169169

170+
# LIA-535: git.commit/file.write generation mismatches get a distinct, diagnostic reason instead
171+
# of the fully generic default -- proves the new decision body actually fires with the right text,
172+
# not just that allow is false (test_deny_stale_opa_generation_mismatch above only checks that).
173+
test_deny_stale_opa_generation_mismatch_has_distinct_reason if {
174+
inp := object.union(base_input(subject_reviewed), {"expected_generation": 4})
175+
decision.reason == "OPA ledger generation stale (expected 4, got 5) -- run `python3 scripts/warden_attest.py sync` or wait for the next self-heal tick (LIA-533)" with input as inp with data.warden_attestations as base_attestations
176+
}
177+
178+
# LIA-535 round-1 regression test: an unrecognized operation with a stale expected_generation must
179+
# still fall through to the fully generic default, not get misclassified as ledger staleness. This
180+
# is the exact scenario plan-review round 1 caught against the original, too-broad
181+
# `input.operation != "attestation.verify"` guard.
182+
test_deny_unrecognized_operation_with_stale_generation_still_hits_generic_default if {
183+
inp := object.union(base_input(subject_reviewed), {"operation": "something.else", "expected_generation": 4})
184+
not decision.allow with input as inp with data.warden_attestations as base_attestations
185+
decision.reason == "guardrails policy produced no valid decision" with input as inp with data.warden_attestations as base_attestations
186+
}
187+
170188
test_deny_malformed_store_missing_records_key if {
171189
att := {
172190
"schema_version": 1, "generation": 5,
@@ -614,6 +632,23 @@ test_attestation_verify_deny_stale_hermes_generation_with_real_ship_present if {
614632
not decision.allow with input as inp with data.warden_attestations as base_attestations with data.warden_cc_attestations as base_cc_attestations
615633
}
616634

635+
# LIA-535 exclusion-proof: attestation.verify must NOT match the new git.commit/file.write-scoped
636+
# generation-mismatch rule -- it keeps its own composite deny message. This also proves there's no
637+
# Rego eval_conflict_error: if the new rule's `input.operation in {...}` guard were dropped
638+
# entirely, this exact input would satisfy BOTH the new rule's condition and the existing
639+
# attestation.verify fallback's `not hermes_path_ok, not cc_path_ok` condition simultaneously, and
640+
# `opa test` would report eval_conflict_error for this test rather than PASS/FAIL. (The round-1-
641+
# rejected `!= "attestation.verify"` guard does NOT trigger this specific conflict -- it still
642+
# excludes attestation.verify correctly; its bug was misclassifying unrecognized operations
643+
# instead, covered by test_deny_unrecognized_operation_with_stale_generation_still_hits_generic_default above.)
644+
test_attestation_verify_stale_hermes_generation_keeps_composite_message if {
645+
inp := object.union(attestation_verify_input(subject_cc_mirrored), {"expected_generation": 4})
646+
decision.reason == sprintf(
647+
"no SHIP found for %s (Hermes-native or Claude-Code-mirrored; or an explicit non-SHIP Hermes verdict exists; or OPA snapshot stale/unsupported)",
648+
[subject_cc_mirrored],
649+
) with input as inp with data.warden_attestations as base_attestations with data.warden_cc_attestations as base_cc_attestations
650+
}
651+
617652
test_attestation_verify_allow_hermes_native_precedence_when_cc_evidence_also_exists if {
618653
# Code-reviewer finding: no fixture previously had BOTH a fresh Hermes SHIP and a valid
619654
# CC-mirrored SHIP for the SAME subject -- the normal production state once LIA-534 lands (a

0 commit comments

Comments
 (0)