Skip to content

feat(ci/diff-guard): wire per-target change rate threshold overrides#146

Draft
shino wants to merge 5 commits into
mainfrom
shino/diff-per-target-threshold
Draft

feat(ci/diff-guard): wire per-target change rate threshold overrides#146
shino wants to merge 5 commits into
mainfrom
shino/diff-per-target-threshold

Conversation

@shino
Copy link
Copy Markdown
Contributor

@shino shino commented May 8, 2026

Summary

  • Adds two optional inputs to the diff-guard composite action — db_change_rate_threshold_overrides and detection_change_rate_threshold_overrides — each a multi-line "=" string. Blank/whitespace-only lines are dropped; inline # ... comments are NOT stripped (they would be forwarded as part of the value and rejected by vuls2). Each diff step joins the remaining entries with commas and passes them as a single --change-rate-threshold-override flag (omitting the flag entirely when the result is empty).
  • Adds matching workflow_dispatch inputs to db-nightly.yml with empty defaults. The workflow becomes the single source of truth for which targets are persistently relaxed and why.
  • Depends on MaineK00n/vuls2#371 — that PR ships the new --change-rate-threshold-override CLI flag. Draft until it lands and the next vuls2 nightly is published, since the composite action installs vuls@nightly.

Why

The current single threshold (5% detection / 10% db) forces a one-size-fits-all bar across every ecosystem and scan-result file. Recent operational pain has been driven by individual targets tripping the guard for upstream-driven, non-regression reasons:

  • ubuntu:26.04 — 19.8% DB drift from new-distro structural churn.
  • debian_13 — 6.0% detection drift after 127 brand-new CVEs landed.

Both required manual promote-tag overrides even though nothing was wrong with the build. Per-target overrides let the strict default keep guarding mature distros while letting individually-justified targets pass automatically.

Operational model

  • Persistent overrides: edit the multi-line default in db-nightly.yml via PR. Rationale goes in the PR description so the "why we relaxed it" trail lives in git history.
  • Emergency dispatch override: paste a full multi-line value into the workflow_dispatch input (GitHub Actions replaces the default wholesale rather than merging — copy-edit-paste is acceptable for rare cases).
  • Stale entry hygiene: vuls2 logs a warning when an override key matches no ecosystem, surfacing typos and dead entries in run logs.

Test plan

  • Wait for MaineK00n/vuls2#371 merge + new vuls2 nightly publish
  • Take this PR out of draft
  • Manual workflow_dispatch with empty overrides → identical to current production behavior (both _overrides paths are no-ops)
  • Manual dispatch with ubuntu:26.04=25 for db and debian_13=8 for detection on a baseline that historically fails on those — confirm step summary shows * markers and footnote, and exit code is 0
  • Confirm an unmatched override key (e.g. unknown:99=50) emits a warning in the action log without affecting pass/fail

🤖 Generated with Claude Code

vuls2 now accepts `--change-rate-threshold-override <key>=<rate>` on
both `vuls diff db` and `vuls diff detection`. Surface that surface
through the diff-guard composite action and the db-nightly workflow so
specific ecosystems / scan-result files can be relaxed without
weakening the default threshold for the rest.

The composite action gains two new optional inputs
(`db_change_rate_threshold_overrides`,
`detection_change_rate_threshold_overrides`). Each is a multi-line
string of "<key>=<rate>" entries; trailing `# ...` comments and blank
lines are stripped at parse time. Each diff step expands the input
into repeated `--change-rate-threshold-override` flags via a small
bash loop. Empty input yields zero added args, so existing callers
keep their bit-for-bit current behavior.

The db-nightly workflow gets matching workflow_dispatch inputs with
empty defaults; persistent overrides are intended to be added to
those defaults via PR (with rationale in the PR description) rather
than at dispatch time, so the workflow file itself becomes the
single source of truth for which targets are relaxed and why.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 8, 2026 03:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds per-target change rate threshold overrides to the diff-guard composite action and wires them through db-nightly.yml as optional workflow_dispatch inputs, allowing specific ecosystems/scan-result files to use relaxed thresholds while keeping strict defaults.

Changes:

  • Add multi-line override inputs for DB and detection thresholds (composite action + workflow inputs).
  • Parse override lists in the composite action and expand them into repeated --change-rate-threshold-override CLI flags.
  • Update workflow wiring so scheduled runs behave the same (empty overrides) and dispatch runs can supply overrides.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/db-nightly.yml Adds workflow_dispatch inputs and passes override strings into the composite action with safe defaults.
.github/actions/diff-guard/action.yml Adds new composite-action inputs and expands multi-line override strings into CLI args for vuls diff.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/actions/diff-guard/action.yml
Comment thread .github/actions/diff-guard/action.yml Outdated
Comment thread .github/actions/diff-guard/action.yml Outdated
…ript

The override-expansion loop was duplicated across the three diff steps
(detection master, detection old, db). Two issues with that:

1. The inline loop stripped trailing "# ..." comments but did not trim
   leading or trailing whitespace from the remaining entry, so an
   indented or trailing-space line could produce a key with a literal
   space attached, making the override silently miss its target.
2. Three copies of the same shell logic invite drift if the parsing
   rules change.

Move the parsing into ./expand-overrides.sh and have each diff step
mapfile-read its output into the override-args array. The helper
strips comments, normalizes whitespace via `awk '{$1=$1};1'`, skips
blank entries, and emits one `--change-rate-threshold-override <entry>`
token pair per surviving line. Empty / unset input yields no tokens,
so an array expansion with zero elements still produces the same
command line as before for callers that don't supply overrides.

Reported by copilot-pull-request-reviewer on #146.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

…cript

`expand-overrides.sh` existed to strip `# ...` inline comments and
normalize whitespace before emitting paired
`--change-rate-threshold-override <entry>` tokens for `mapfile` to
slurp into an array. In the actual end-to-end pipeline the script was
doing too much: vuls2's `override.Parse` already calls
`strings.TrimSpace` around the `=`, and inline `# ...` comments inside
the workflow input string are an unsupported quirk that produced
parse-time errors anyway.

Drop the script and the inline-comment-stripping promise it made, then
collapse the expansion to three bash lines per step:

  overrides_csv=$(printf '%s' "$INPUT" | awk 'NF' | paste -sd, -)
  overrides_args=()
  [ -n "$overrides_csv" ] && overrides_args+=(--change-rate-threshold-override "$overrides_csv")

`awk 'NF'` drops blank/whitespace-only lines (NF = 0 for those under
awk's default field separator); `paste -sd, -` joins survivors with
",". cobra's `StringSliceVar` already splits on commas, so passing a
single comma-joined value is exactly equivalent to passing the flag
N times — and an empty input naturally produces zero args, matching
the no-overrides default exactly.

Updated the input descriptions on both action.yml and db-nightly.yml
to make the new contract explicit: blank lines dropped, but inline
`# ...` comments NOT supported (rationale belongs in PR descriptions
or YAML comments outside the default string).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

.github/actions/diff-guard/action.yml:271

  • The overrides parsing logic is duplicated across the detection-master, detection-old, and db diff steps (same awk 'NF' | paste -sd, - pipeline + flag assembly). This creates drift risk if the parsing rules need to change (e.g., whitespace normalization, comment handling, validation). Consider centralizing this in a small script within the action (or a single step output reused by the later steps) so there’s one source of truth.
        # See diff_detection_master for the parsing-rule rationale.
        overrides_csv=$(printf '%s' "$DETECTION_CHANGE_RATE_THRESHOLD_OVERRIDES" | awk 'NF' | paste -sd, -)
        overrides_args=()
        [ -n "$overrides_csv" ] && overrides_args+=(--change-rate-threshold-override "$overrides_csv")

.github/workflows/db-nightly.yml:115

  • This comment says the composite action has “whitespace/comment-only handling” for the _overrides inputs, but the action’s parsing currently only drops blank/whitespace-only lines (it does not strip # ... comments). As written, a comment-only line like # note would be forwarded into the vuls2 flag and likely fail parsing. Please adjust this comment (or the action behavior) so operators aren’t misled when using the multi-line override inputs.
        # The `_overrides` inputs default to an empty string when unset so
        # the composite action's whitespace/comment-only handling treats them
        # as "no overrides".

Comment thread .github/actions/diff-guard/action.yml Outdated
Comment thread .github/workflows/db-nightly.yml
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

.github/actions/diff-guard/action.yml:272

  • Same override parsing logic is duplicated here as well; centralizing the multiline-to-args expansion would avoid having to keep multiple steps in sync.
        # See diff_detection_master for the parsing-rule rationale.
        overrides_csv=$(printf '%s' "$DETECTION_CHANGE_RATE_THRESHOLD_OVERRIDES" | awk 'NF' | paste -sd, -)
        overrides_args=()
        [ -n "$overrides_csv" ] && overrides_args+=(--change-rate-threshold-override "$overrides_csv")

.github/actions/diff-guard/action.yml:310

  • Same override parsing logic is duplicated again in the DB diff step. Consider factoring the override expansion into a single script/function shared by all diff steps.
        # See diff_detection_master for the parsing-rule rationale.
        overrides_csv=$(printf '%s' "$DB_CHANGE_RATE_THRESHOLD_OVERRIDES" | awk 'NF' | paste -sd, -)
        overrides_args=()
        [ -n "$overrides_csv" ] && overrides_args+=(--change-rate-threshold-override "$overrides_csv")

Comment thread .github/actions/diff-guard/action.yml Outdated
Comment thread .github/workflows/db-nightly.yml Outdated
Comment thread .github/actions/diff-guard/action.yml
The `Run diff guard` step's comment claimed the composite action has
"whitespace/comment-only handling" for the `_overrides` inputs, but the
action only drops blank/whitespace-only lines — it does not strip
inline `# ...` comments. Rewrite the comment to describe the actual
behavior (drop blanks, omit the flag when the result is empty) so
operators reading the workflow are not misled.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@shino shino requested a review from Copilot May 19, 2026 01:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread .github/actions/diff-guard/action.yml Outdated
Comment thread .github/actions/diff-guard/action.yml
Move the multi-line `_overrides` input parsing out of the three diff
steps (diff_detection_master, diff_detection_old, diff_db) into a single
`Parse threshold overrides` step that emits `detection_csv` and `db_csv`
as step outputs. Each diff step now reads the relevant output via
`steps.parse_overrides.outputs.<which>_csv` and only retains the local
flag-omission guard (`[ -n "$OVERRIDES_CSV" ] && ...`), which is
genuinely per-step (the consuming command differs).

Eliminates the drift risk Copilot flagged: a future change to the
parsing rule (whitespace handling, trimming, validation) lives in one
place instead of being threaded through three near-identical bash
blocks.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants