Close the mirror gate's last hole, add reference-integrity and oval_pattern guards
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 68/100
- Issue type
- Feature
- Clarity
- Mostly clear
- Activity status
- Active
- Tech stack
- shell
- Domain
- security, testing-qa
Research direction
Start with tests/oscap-offline/internal/mirrors/mirrors_test.go, then read tests/stamps/run.sh and run_test.sh for the existing guards and test flow. Run the relevant mirror and stamp tests first. Done means datastream-only entries fail, references resolve after normalizing ./, and an unknown OVAL object id is covered by a test that confirms the script aborts.
Written by the indexing model from the issue text.
Description
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.
- Dominant language
- HTML
- Stars
- 25
- Forks
- 15
- Avg merge
- 3d 8h
- Merged PRs (30d)
- 21
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
getgrav/grav-plugin-api#45 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
sisaku-security/sisakulint#647 ·
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
yunaremaia/driftcheck#246 ·
-
Docs say approvals.mode defaults to smart, but code falls back to manual when the key is absent Openarea/config comp/tools P2 sweeper:risk-compatibility type/bug
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
NousResearch/hermes-agent#117341 · 1 comment ·
-
bug good-first-issue
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
yunaremaia/taintrace#110 ·