Close the mirror gate's last hole, add reference-integrity and oval_pattern guards

[stevebeattie]

Follow-ups from the review of #172 (merged; the review found zero in-scope findings). Each has been verified against main at 1e979cb, with notes below where verification changed the picture.

1. report.Unmirrored is logged, not failed

tests/oscap-offline/internal/mirrors/mirrors_test.go fails on report.Functional and report.Descriptive but only t.Logfs report.Unmirrored. The stub was that list's one permanent entry, so this is the last hole in the mirror gate: oscap-playground's has_stub_oval metadata flag could reintroduce the component on a regeneration and nothing would fail.

Verified: the list is empty on main — TestRepositoryMirrorsMatch reports compared 8 standalone OVAL file(s) with no "present only in the datastream" lines. So flipping it to t.Errorf is a no-op today and a guard from here on.

Worth deciding deliberately: it would also mean any future datastream-only definition has to gain a standalone counterpart before it can land. Given #163 and #164 moved deliberately toward full mirroring, that seems like the intent rather than a side effect.

2. Reference-integrity check over check-content-ref/@href

Nothing currently resolves every check-content-ref/@href through cat:catalog and fails on an unresolved one. mirrors.Check only covers the standalone-file-without-datastream-block direction, and the full-datastream validation in .github/workflows/create-release.yaml is commented out. tests/e2e/fixtures/*/expected.txt asserts 8 lines across 4 rule ids out of 198 rules, so a deletion touching any of the ~180 unasserted, unmirrored rules would go unnoticed — which is what makes this worth having.

Correction to the review's stated payoff. It held that such a check "would immediately surface the pre-existing, unrelated ./AslrCheck.sh mismatch (ssg-chainguard-gpos-ds.xml:6079, :6098 — no matching cat:uri)". That reference is not broken. There is a catalog entry:

AslrCheck.sh -> #scap_org.open-scap_cref_AslrCheck

The href reads ./AslrCheck.sh and the cat:uri name is AslrCheck.sh, so the strings differ, but OpenSCAP normalizes the prefix and resolves it. Confirmed by evaluating the rule: xccdf_mil.disa.stig_rule_SV-203754r958928_rule returns pass, i.e. the script was found and run.

./AslrCheck.sh is also the only href in the datastream carrying a ./ prefix; the other eight are bare and match their catalog names exactly.

So the requirement for this check is that it normalize ./ prefixes before comparing, or it will report a false positive on a working reference on its first run. A naive exact-match version was written while verifying this and did exactly that.

3. oval_pattern has no test coverage, and fails closed only via set -e

tests/stamps/run.sh:79-93's oval_pattern exits non-zero for an unknown object id, and run_test.sh has no coverage of it at all.

The margin is thinner than the review suggested. pattern="$(oval_pattern "${obj_id}")" feeds grep -Pq -- "${pattern}" at :177-178, and grep -Pq -- "" matches anything — so an empty pattern would pass the guard silently rather than failing it. The only thing preventing that is set -euo pipefail at :35, under which a failing command substitution in an assignment aborts the script (verified).

There is no bug today. But the fail-closed behaviour rests entirely on set -e: wrapping that call in a conditional, or appending || true, would silently turn a trust-store guard into a no-op with no test to catch it. A run_test.sh case asserting oval_pattern exits non-zero on an unknown id — and that the script aborts rather than reaching grep — is worth having for that reason specifically.

Not included

The review also raised a process point: agents citing container-local artifact paths (randomized claude-guard-diffmap-*.json mount roots) as corroborating evidence, which produced a false accusation of a fabricated path between agents. Reviews should cite repo-relative paths only. That is tooling guidance rather than a repo change, so it is recorded here but has no task attached.

[stevebeattie]

Item 2 (reference integrity over check-content-ref/@href) is now tracked as PSEC-2851, since it needs design decisions rather than a small patch — where the check lives, what counts as a failure, and the ./ normalization requirement described above.

Items 1 and 3 stay here: both are small, verified gaps on existing suites and are headed for a PR rather than a ticket.

One thing ruled out while triaging, worth recording so nobody re-treads it: the commented-out Validate full datastream step in create-release.yaml does not cover item 2. That step is oscap xccdf validate, i.e. schema/schematron validation — on edac76f it exits 2 with 592 lines, which is why it is disabled. Reference integrity is a separate concern; re-enabling that block would not answer it.

[stevebeattie]

Items 1 and 3 are now tracked as PSEC-2981 and are up for review in #183 — the mirror gate failing on datastream-only definitions, and a case pinning that an unresolvable OVAL object id aborts the stamp guard.

So the whole list is now accounted for: items 1 and 3 in PSEC-2981 / #183, item 2 in PSEC-2851 (still Todo, design decisions pending).