fix(ci): verify pull request secret-scan ranges by chet · Pull Request #4788 · NVIDIA/infra-controller

chet · 2026-08-10T20:49:56Z

TLDR

This fixes how Core and REST tell TruffleHog which commits to scan. On a PR's first CI run, GitHub has no previous commit, which caused REST to scan the entire repository history. After a force-push, the next run can have the opposite problem: copy-pr-bot replaces its internal PR branch, but GitHub still identifies that branch's now-missing old commit as the starting point. TruffleHog then scans zero bytes, and the surrounding action still reports success -- so yes, we failed open. Both workflows now ignore that unreliable previous commit for PR runs, look up the PR's current base, use the commit that started the workflow as the current head, and calculate the real range from the fetched Git history before starting TruffleHog. If that range cannot be calculated, the workflow fails before the scan; this keeps the first run from scanning the whole repository and prevents this missing-commit case from going green after scanning nothing.

Expected green-run effect

No reliable steady-state speedup. The first synthetic REST scan on this PR processed 11,464 bytes instead of the roughly 446 MB full-history scan we observed on #4742 -- about 39,000x less scan input. This is still primarily a correctness fix; normal update scans should take about the same time.

What it really buys us

One latest green Core or REST secret check now proves that the repository-side range was resolved from the current PR and passed to the scanner. Rebases and force-pushes can no longer make that run depend on a stale or missing synthetic-branch commit.

Testing

bash -n .github/ci/resolve-pr-scan-range.sh .github/ci/test-resolve-pr-scan-range.sh
bash .github/ci/test-resolve-pr-scan-range.sh
python3 -B .github/ci/test_check_ci_gate.py
Core and REST final-gate inventory checks
CI token-permission checker fixtures, plus the Core and stale-workflow permission checks
Core and REST concurrency-policy checks
actionlint for both edited workflows, ignoring only the repository's known custom-runner labels
cargo make check-format-nightly
cargo make clippy
cached Carbide-lints workflow (--all-targets --all-features)
git diff --check

Hosted acceptance exercised the initial synthetic push and two rewritten heads. On the initial head, Core and REST both resolved 00ece37...b8acd55, scanned 6 chunks / 11,464 bytes, and reported no source errors. On the first rewrite, the push event contained stale before=b8acd55, but both workflows resolved 00ece37...eb5f72f and scanned the same real range. After the final simplification and rebase, the event contained stale before=eb5f72f; both workflows instead resolved 7d9b87e...d79119f, scanned 6 chunks / 9,129 bytes, and passed. Final-head Core CI completed 52 jobs with zero failures, and REST's final gate also passed.

This supports #4786

Closes #4786

coderabbitai · 2026-08-10T20:50:41Z

Summary by CodeRabbit

New Features
- Improved pull-request secret scanning with reliable validation of repository, revision, and merge-base information.
- Scans now analyze the correct base and head revisions for pull requests, including synthetic events.
- Preserved appropriate scanning behavior for regular pushes and tag events.
Tests
- Added end-to-end coverage for successful range resolution and invalid, incomplete, empty, or failed API scenarios.

Walkthrough

The PR adds a validated pull-request scan-range resolver, end-to-end coverage, and Core and REST workflow integration. Non-pull-request scans retain their existing range behavior.

Changes

Secret-scan range resolution

Layer / File(s)	Summary
Resolver validation and merge-base computation `.github/ci/resolve-pr-scan-range.sh`	The resolver validates inputs, retrieves and validates pull-request metadata, computes a non-empty merge base, and emits normalized base and head revisions.
Resolver end-to-end validation `.github/ci/test-resolve-pr-scan-range.sh`	The test verifies successful resolution and failures for malformed refs, invalid SHAs, missing or invalid bases, empty ranges, incomplete API data, and API errors.
CI secret-scan integration `.github/workflows/ci.yaml`, `.github/workflows/rest-ci.yml`	The workflows use the resolver for pull-request scans and pass its base and head outputs to TruffleHog. Other scan types retain their existing range behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Workflow
  participant Resolver
  participant GitHub_API
  participant Git
  participant TruffleHog
  Workflow->>Resolver: Provide pull-request ref and workflow SHA
  Resolver->>GitHub_API: Retrieve pull-request metadata
  GitHub_API-->>Resolver: Return base SHA
  Resolver->>Git: Compute and validate merge range
  Git-->>Resolver: Return merge base
  Resolver-->>Workflow: Set base and head outputs
  Workflow->>TruffleHog: Scan from base to head

Possibly related issues

Make Pull Request Secret Scans Use a Verified Merge Base #4786 — Directly related to the merge-base resolver and Core and REST secret-scan workflow integration.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: fixing CI verification of pull request secret-scan ranges.
Description check	✅ Passed	The description directly explains the secret-scan range issue, the workflow changes, failure behavior, and validation performed.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

chet · 2026-08-10T20:50:41Z

@coderabbitai full_review, thanks!

coderabbitai · 2026-08-10T20:50:54Z

@chet: I will perform a complete review of PR #4788.

🐇

⚠️ Action not completed

Head commit changed.

github-actions · 2026-08-10T20:51:49Z

🔐 TruffleHog Secret Scan

✅ No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

_{🕐 Last updated: 2026-08-10 20:51:49 UTC | Commit: b8acd55}

chet · 2026-08-10T20:58:21Z

@coderabbitai full_review, thanks!

coderabbitai · 2026-08-10T20:58:31Z

@chet: I will perform a complete review of PR #4788.

🐇

✅ Action performed

Full review finished.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yaml:
- Around line 1354-1375: The non-PR scan-range branch in
.github/workflows/ci.yaml must emit a head value whenever it emits base; set
head to github.sha before the TruffleHog action consumes
steps.scan-range.outputs.head. Update .github/workflows/ci.yaml lines 1354-1375
accordingly; .github/workflows/rest-ci.yml lines 126-136 requires no direct
change because both inputs remain empty there.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6af0bf0e-b21d-4f10-9446-5a900afef656

📥 Commits

Reviewing files that changed from the base of the PR and between 00ece37 and eb5f72f.

📒 Files selected for processing (4)

.github/ci/resolve-pr-scan-range.sh
.github/ci/test-resolve-pr-scan-range.sh
.github/workflows/ci.yaml
.github/workflows/rest-ci.yml

`github.event.before` works for ordinary branch pushes, but `copy-pr-bot` can replace that commit when it rewrites a synthetic PR branch. Resolve the current pull request merge-base and head for every Core and REST synthetic scan instead. - Expected green-run effect: First REST pushes stop scanning repository history; steady-state timing should be effectively unchanged. - What it really buys us: A green secret check proves the current pull request range was resolved and passed to the scanner, even after a rebase or force-push. Tests added! This supports NVIDIA#4786 Signed-off-by: Chet Nichols III <chetn@nvidia.com>

chet · 2026-08-10T22:41:50Z

@coderabbitai full_review, thanks!

coderabbitai · 2026-08-10T22:42:00Z

@chet: I will perform a complete review of PR #4788.

✅ Action performed

Full review finished.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yaml:
- Around line 1359-1366: Update the PREV_TAG lookup in the tag-push branch to
avoid the grep/head pipeline under set -euo pipefail, including upstream SIGPIPE
failures. Read the available tags with a non-failing approach, exclude
GITHUB_REF_NAME, and leave PREV_TAG empty when no different tag exists so the
existing fallback remains reachable.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6f7b0597-b374-4065-88ad-49983bf022f8

📥 Commits

Reviewing files that changed from the base of the PR and between 7d9b87e and d79119f.

📒 Files selected for processing (4)

.github/ci/resolve-pr-scan-range.sh
.github/ci/test-resolve-pr-scan-range.sh
.github/workflows/ci.yaml
.github/workflows/rest-ci.yml

thossain-nv

Thanks for this @chet. Left one note.

thossain-nv · 2026-08-11T00:25:27Z

+        if: startsWith(github.ref, 'refs/heads/pull-request/')
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: bash .github/ci/resolve-pr-scan-range.sh >> "$GITHUB_OUTPUT"


rest-ci.yml also triggers on tags (v[0-9]*.[0-9]*.[0-9]* and the release candidate pattern), but this is running only for refs/heads/pull-request/*. So a tag push leaves base empty and TruffleHog still scans the full history. Should we add the same refs/tags/* previous-tag branch here like ci.yaml?

@thossain-nv Oh yeah, 100%. I'm going to actually merge this as-is, and then do a follow-up that addresses Core and REST together. It should look pretty. Thanks for calling this out!

nv-dmendoza

LGTM with suggested changes by @thossain-nv

## TLDR This fixes how Core and REST tell TruffleHog which commits to scan. On a PR's first CI run, GitHub has no previous commit, which caused REST to scan the entire repository history. After a force-push, the next run can have the opposite problem: `copy-pr-bot` replaces its internal PR branch, but GitHub still identifies that branch's now-missing old commit as the starting point. TruffleHog then scans zero bytes, and the surrounding action still reports success -- so yes, we would fail open -- and this was caught during the CICD work that we've been doing. Both workflows now: 1. Ignore that [unreliable] previous commit for PR runs. 2. Look up the PR's current base. 3. Use the commit that started the workflow as the current head. 4. Calculate the real range from the fetched Git history before starting TruffleHog. If that range cannot be calculated, the workflow fails before the scan; this keeps the first run from scanning the whole repository and prevents this missing-commit case from going green after scanning nothing. ## Expected green-run effect No reliable steady-state speedup. The first synthetic REST scan on this PR processed 11,464 bytes instead of the roughly 446 MB full-history scan we observed on NVIDIA#4742 -- about 39,000x less scan input. This is still primarily a correctness fix; normal update scans should take about the same time. ## What it really buys us One latest green Core or REST secret check now proves that the repository-side range was resolved from the current PR and passed to the scanner. Rebases and force-pushes can no longer make that run depend on a stale or missing synthetic-branch commit. ## Testing Ran a bunch of commands to test it out: - `bash -n .github/ci/resolve-pr-scan-range.sh .github/ci/test-resolve-pr-scan-range.sh` - `bash .github/ci/test-resolve-pr-scan-range.sh` - `python3 -B .github/ci/test_check_ci_gate.py` - Core and REST final-gate inventory checks - CI token-permission checker fixtures, plus the Core and stale-workflow permission checks - Core and REST concurrency-policy checks - `actionlint` for both edited workflows, ignoring only the repository's known custom-runner labels - `cargo make check-format-nightly` - `cargo make clippy` - cached Carbide-lints workflow (`--all-targets --all-features`) - `git diff --check` Acceptance exercised the initial synthetic push and two rewritten heads. On the initial head, Core and REST both resolved `00ece37...b8acd55`, scanned 6 chunks / 11,464 bytes, and reported no source errors. On the first rewrite, the push event contained stale `before=b8acd55`, but both workflows resolved `00ece37...eb5f72f` and scanned the same real range. After the final simplification and rebase, the event contained stale `before=eb5f72f`; both workflows instead resolved `7d9b87e...d79119f`, scanned 6 chunks / 9,129 bytes, and passed. Final-head Core CI completed 52 jobs with zero failures, and REST's final gate also passed. This supports NVIDIA#4786 Signed-off-by: Chet Nichols III <chetn@nvidia.com>

This follows #4715 and #4788 by using the same shared resolver for tags (for both Core and REST), finding the closest tag behind the release commit on its first-parent history, and giving both secret scanners that exact base plus the current tag commit. Pull request behavior stays the same, and main/manual runs are unchanged. - **Expected green-run effect:** No reliable full-pipeline savings. Core avoids its observed open-ended scan, while REST intentionally does real work instead of zero. - **What it really buys us:** A green release-tag check from either workflow means both scanners were given the same known commit range. ## Related issues This supports #4806 ## Type of Change - [ ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [x] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes - [ ] **This PR contains breaking changes** ## Testing - [x] Unit tests added/updated - [ ] Integration tests added/updated - [x] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) - `bash .github/ci/test-resolve-secret-scan-range.sh` - Exercised the resolver against the existing `v2.1.0-rc.2` tag; it returned `v2.1.0-rc.1`'s commit as the base and the exact `v2.1.0-rc.2` commit as the head. - Core/REST final-gate inventories, token-permission policies, concurrency policies, `actionlint`, Bash syntax, and `git diff --check`. - `cargo make format-nightly` - `cargo make clippy` - Cached full Carbide-lints gate. ## Additional Notes This does not create a test release tag because these workflows publish release artifacts. The next natural tag is the hosted acceptance check: Core and REST should log the same non-empty base/head range, with no `No commits to scan` fallback. Managed-action source-error classification remains separate from this repository-side range fix. Signed-off-by: Chet Nichols III <chetn@nvidia.com>

> [!IMPORTANT] > This PR cherry-picks two commits into `release/v2.1`: > - c79d042 (#4534) -- discover host NICs through nv-redfish adapter ports > - e9c13b3 (#4788) -- verify pull request secret-scan ranges Site Explorer's default nv-redfish path can now find an ordinary host NIC when the System EthernetInterfaces collection has no usable MACs but the chassis NetworkAdapter Ports do. The fallback keeps System interfaces authoritative, prefers standard Port MACs before Lenovo OEM data, and fetches Port links independently so one failed member does not hide valid siblings. #4788 rides along because it fixes the secret-scan failure this branch hits. `copy-pr-bot` can rewrite its synthetic PR branch, so `github.event.before` is unusable and TruffleHog falls back to scanning the whole repository -- on this branch that surfaced 117 unrelated findings and failed `REST Secret Scan with TruffleHog` (and with it the `rest-ci-pass` rollup) on every recent `release/v2.1` PR, merged ones included. The resolver computes the current PR's merge-base and head for both Core and REST synthetic scans, so the scan is scoped to this PR's own commits. ## Related issues This supports #4469 and #4786 ## Type of Change - [x] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [ ] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes - [ ] **This PR contains breaking changes** ## Testing - [x] Unit tests added/updated - [x] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) `cargo test -p bmc-explorer` (30 passed, including the new `network_adapter_port_explore` suite) and `cargo test -p carbide-site-explorer 'redfish::tests::' --lib` (6 passed) pass on the branch. `bash .github/ci/test-resolve-pr-scan-range.sh` passes, and both workflow files still parse. ## Additional Notes **#4534 -- three conflicts, all visibility drift.** `release/v2.1` does not have #4662 (`refactor(bmc/console): adopt style guide rules for pub/module visibility`), so `bmc-explorer` here still uses `pub` where `main` uses `pub(crate)`. `chassis.rs` and `network_adapter.rs` keep this branch's `pub` and take only the new `fetch_network_adapter_ports` method and `ports` field; #4662 is style-only and is intentionally not pulled in. `bluefield3_explore.rs` conflicted because #4534's new test anchors directly after `explore_bluefield3_ignores_invalid_system_interface_mac`, which belongs to a different commit that is not on this branch -- only #4534's own `explore_bluefield3_preserves_oem_mode_and_base_mac` is added here. **#4788 -- one conflict in `.github/workflows/ci.yaml`,** because this branch already carries #4715's earlier Core-only workaround. Resolved to keep release-specific workflow behavior: #4788's resolver now backs synthetic PR scans in both Core (`ci.yaml`) and REST (`rest-ci.yml`), the existing Core tag-scan branch is preserved, and the resolver plus its focused test come along. The surrounding CI steps in that hunk (`check-ci-permissions.sh`, `check-core-ci-permissions.sh`, `check-stale-ci-permissions.sh`, `check-ci-concurrency.sh`, `test_check_ci_gate.py`, `check_ci_gate.py`) are from other commits and reference scripts that do not exist on this branch, so they are deliberately left out -- every script referenced by `ci.yaml` here resolves. **Migration is additive and safe.** `20260810143726_index_explored_endpoint_port_macs.sql` sorts after this branch's latest migration (`20260731143022`), so it appends without disturbing existing checksums. It drops and recreates `explored_endpoints_mac_addresses_idx`, and that index's pre-state is byte-identical on `release/v2.1` and `main` (both from `20260708172302_squash_snapshot.sql`), so the rebuild behaves the same here. **No dependency bump needed.** The adapter-port support this relies on is already on the branch -- `nv-redfish` is pinned at `0.14.2`, matching `main` (via #4785).

chet requested a review from a team as a code owner August 10, 2026 20:49

chet force-pushed the gh-issue-4786 branch from b8acd55 to eb5f72f Compare August 10, 2026 20:58

chet mentioned this pull request Aug 10, 2026

Make Pull Request Secret Scans Use a Verified Merge Base #4786

Closed

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread .github/workflows/ci.yaml

chet force-pushed the gh-issue-4786 branch from eb5f72f to d79119f Compare August 10, 2026 22:41

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread .github/workflows/ci.yaml

thossain-nv approved these changes Aug 11, 2026

View reviewed changes

nv-dmendoza approved these changes Aug 11, 2026

View reviewed changes

chet merged commit e9c13b3 into NVIDIA:main Aug 11, 2026
123 checks passed

This was referenced Aug 11, 2026

Limit Core Secret Scans on New Pull Requests and Tags #4805

Closed

Make Core and REST Release Tags Scan the Same Verified Range #4806

Closed

chet deleted the gh-issue-4786 branch August 11, 2026 02:17

chet mentioned this pull request Aug 11, 2026

fix(ci): verify release tag secret-scan ranges #4808

Merged

10 tasks

coderabbitai Bot mentioned this pull request Aug 11, 2026

fix(ci): stop Core main secret scans at the pushed commit #4828

Merged

10 tasks

chet mentioned this pull request Aug 11, 2026

patch(v2.1): discover host NICs through nv-redfish adapter ports #4832

Merged

10 tasks

Uh oh!

Conversation

chet commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TLDR

Expected green-run effect

What it really buys us

Testing

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

chet commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 10, 2026

🔐 TruffleHog Secret Scan

Uh oh!

chet commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chet commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thossain-nv left a comment

Choose a reason for hiding this comment

Uh oh!

thossain-nv Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

chet Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

nv-dmendoza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chet commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading