Single quotes - #6
Merged
Merged
Conversation
eldrig0
approved these changes
Dec 8, 2023
grafele
added a commit
that referenced
this pull request
Aug 5, 2026
The review verdict was: a dependable assessment core, and three provable critical defects in
exactly the layer with no tests — the seam to the outside world. All fixed, each with a
regression test, 154 passing.
Critical:
resolve-deployed.sh passed --arg through `gh api --jq`, which takes one expression and
forwards nothing. gh refused the call, `|| echo ""` ate the refusal, and every deployed tag
reported "no SBOM asset" — the daily monitor could never have fetched a real inventory. The
comment at the top of the same file warns about exactly this flag. The lookup now pipes to
jq, and a fake `gh` in the test suite makes the whole resolution path testable at all.
monitor-kev.sh filtered targets on /prod/ while the backstop counts prod|study. A
Study-only product fell out of the targets AND out of the unscannable list: the record said
all_clear with nothing scanned. The filter now matches the backstop, and all_clear
additionally requires that at least one target was actually scanned.
soup-sbom.yml (reusable) never passed release-tag to the action, so the entire
workflow_run path — the normal trigger — produced branch-tier bundles that were never
attached to a release. One line, and the one the review almost had to find twice.
High:
check-currency.py validated the patch limit and never measured it: the comparison looked at
major and minor only, so the TR-03161 O.TrdP_2 tightening existed in config validation and
nowhere else. All three levels now compare, with `unlimited` handled safely.
scan-vulns.sh exited 0 with an incomplete list when an OSV chunk failed, and dropped a
finding entirely when its advisory could not be fetched. Now: retries on every OSV call,
querybatch pagination for >1000-vuln packages (kernel RPMs are real), an unfetchable
advisory is carried unscored (rule 9) instead of dropped, and incompleteness is exit 1 —
the monitor turns that into an `incomplete` verdict, which is the honest answer.
monitor-kev.sh kept one state file for all targets, so a product with an app and a backend
restarted the second target's clocks on every run. State, lifecycle and escalation are now
per target, merged into one product view for the record and the alert.
merge-assessment.sh applied a VEX statement keyed on the CVE alone, so a not_affected
recorded for package A suppressed the same CVE on package B. The analysis is now applied
only when every affected component is covered by its own record; partial coverage becomes a
named property instead of a suppression. Justification codes are validated against the
CycloneDX vocabulary here and in the fix-or-VEX gate — an invalid code was a working mute
button.
resolve-deployed.sh read the first 100 deployment records client-side; a rarely-deploying
environment fell off the page and vanished without a trace. Environments are now enumerated
server-side and queried per environment, the same access pattern the backstop already uses.
Medium, in brief: group-remediation compared the kev field `is True` against data that is
always a string, so units never listed their KEV members (the fixture had encoded the bug —
it used a boolean the production data never has); a CVSS-4-only advisory fell to rule 9
regardless of severity and now routes through the database severity band; `onboarded`
compared as an instant, so a scan on the onboarding day missed the baseline for the whole
backlog — dates compare now; the alert hardcoded "remediation 21 d" from a superseded policy
revision and referenced record fields nothing wrote; discover.sh ids collided across equal
basenames and the second scan source silently vanished — ids are path-based now, and the six
per-product scope drafts were re-verified against their live repositories (five needed only
renames; apellis surfaced one genuinely new candidate, the digest-pinned GPU inference image,
now classified); validate-policy refuses unknown keys, because a typo in an override
otherwise does nothing silently.
Hardening: action inputs reach scripts through env instead of ${{ }} interpolation in run
blocks; a failed Slack post fails the step instead of leaving an annotation nobody reads;
reportlab installs on PEP-668 runners; docker and python3 join the prerequisite check; the
yq variant is verified once in validate-policy (everything here needs mikefarah v4); scope
includes need reasons like excludes always did.
One decision surfaced to the document rather than the code: WI §1 claimed all timeframes are
calendar time while §7 #6 says five working days. The code counts working days, with reasons;
the WI now states the exception explicitly (v0.4).
154 tests, 5 against live feeds. Every fix above has one.
Co-Authored-By: Claude Opus 5 (1M context) <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
2 participants
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.
No description provided.