Skip to content

fix(ci): supply-chain-audit two-dot diff causes false positives on stale-branch PRs - #30621

Merged
teknium1 merged 1 commit into
mainfrom
fix/supply-chain-audit-diff-range
May 22, 2026
Merged

fix(ci): supply-chain-audit two-dot diff causes false positives on stale-branch PRs#30621
teknium1 merged 1 commit into
mainfrom
fix/supply-chain-audit-diff-range

Conversation

@ethernet8023

@ethernet8023 ethernet8023 commented May 22, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

The supply-chain-audit workflow uses git diff "$BASE".."$HEAD" (two-dot), which compares the tip-of-main tree directly against the PR tip. When files land on main after a PR branched off, they appear in the diff even though the PR never touched them — triggering false-positive CRITICAL findings.

Example: PR #30609 was flagged for hermes_cli/setup.py, a file added to main by an unrelated commit after that PR branched. The PR itself never touched setup.py.

Switches all four diff commands to three-dot syntax ("$BASE"..."$HEAD"), which diffs from the merge base to the PR tip — only changes actually introduced by the PR are included.

Related Issue

N/A — discovered via a real false positive on PR #30609.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • .github/workflows/supply-chain-audit.yml line 50: "$BASE".."$HEAD""$BASE"..."$HEAD" (content diff in scan job)
  • .github/workflows/supply-chain-audit.yml line 57: same fix (.pth file-name check)
  • .github/workflows/supply-chain-audit.yml line 100: same fix (install-hook file-name check)
  • .github/workflows/supply-chain-audit.yml line 161: same fix (dep-bounds job pyproject.toml diff)
  • Added inline comment explaining why three-dot is used

How to Test

  1. Open a PR from a branch that diverged from main before hermes_cli/setup.py was added
  2. Observe that the supply-chain-audit scan job no longer flags setup.py as an install-hook finding
  3. Verify that a PR that actually adds a .pth file or base64+exec combo still correctly triggers the scanner

Alternatively: git diff A..B vs git diff A...B on any branch pair where main has moved past the merge base — the two-dot diff will show spurious changes from main, the three-dot diff will not.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've added tests for my changes — N/A (workflow-only change, no test surface)
  • I've tested on my platform: NixOS

Documentation & Housekeeping

  • I've updated relevant documentation — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

closes #23592
closes #13411

both did the same fix, but didn't catch the pyproject.toml one.

Infographic

supply-chain-audit-diff-fix

…s on stale-branch PRs

The workflow diffs base.sha..head.sha (two-dot), which compares the
tip-of-main tree directly against the PR tip. When files land on main
after a PR branched off, they appear in the diff even though the PR
never touched them — triggering false-positive findings.

Example: PR #30609 was flagged for hermes_cli/setup.py, a file added
to main by an unrelated commit after the PR branched.

Switch to three-dot diff (base.sha...head.sha), which diffs from the
merge base to the PR tip — only changes introduced by this PR are
included. Applied to all four diff commands in both jobs (scan and
dep-bounds).
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/supply-chain-audit-diff-range vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8991 on HEAD, 8991 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4770 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists labels May 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #13411 — same fix (two-dot → three-dot git diff in supply-chain-audit.yml). #23592 was also already flagged as a duplicate of #13411.

@teknium1
teknium1 merged commit 2f320cb into main May 22, 2026
13 checks passed
@teknium1
teknium1 deleted the fix/supply-chain-audit-diff-range branch May 22, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants