Skip to content

chore: remove detect-secrets in favor of GitHub secret scanning - #43

Merged
ryan-lempka merged 1 commit into
mainfrom
chore/remove-detect-secrets
Jul 9, 2026
Merged

chore: remove detect-secrets in favor of GitHub secret scanning#43
ryan-lempka merged 1 commit into
mainfrom
chore/remove-detect-secrets

Conversation

@ryan-lempka

@ryan-lempka ryan-lempka commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Removes the detect-secrets pre-commit hook, its .secrets.baseline, the custom NVIDIA-key plugin, and the Secret scan CI job. Secret protection moves to GitHub's platform-level scanning: secret scanning alerts and push protection are both enabled on this repo (Settings > Advanced Security).

Why

Baseline maintenance scales with repo churn. .secrets.baseline records the line number of each allowlisted example secret (~130 entries across tests/, docs, workflows, and Rust source). Any commit that shifts lines in one of those files requires a companion baseline-refresh commit to keep the Secret scan job green; #40 needed two of them. As the repo grows, more files carry allowlisted examples and more edits require a refresh.

The entries being tracked are placeholder keys. The baseline's role today is allowlisting the fake credentials used in tests and docs, so the ongoing maintenance mostly serves examples rather than live-secret detection.

Platform scanning intervenes earlier for real tokens. GitHub secret scanning detects real token formats for its supported providers, and push protection rejects a push containing one before it reaches the repository. A CI job necessarily runs after the commit is already pushed, when rotation is required regardless of the check's result. Detections and bypass events are auditable under the repo's Security tab.

Coverage gaps, stated plainly:

  • detect-secrets' keyword and entropy heuristics can flag generic secrets (e.g. a hard-coded password matching no known token format), which platform scanning does not cover by default. GitHub's non-provider pattern detection is a follow-up option.
  • NVIDIA API keys (nvapi-) do not appear on GitHub's supported patterns list, so removing .hooks/detect_nvidia_api_key.py removes the only NVIDIA-specific detection. If we want that back, a baseline-free pattern check (a strict nvapi- regex that placeholder keys don't match) would restore it without the churn above.

Changes

  • .pre-commit-config.yaml: drop the detect-secrets hook
  • .github/workflows/ci.yml: drop the Secret scan job and its ci-success gate entry
  • Delete .secrets.baseline and .hooks/detect_nvidia_api_key.py (the plugin existed only to feed detect-secrets)

@ryan-lempka
ryan-lempka requested a review from a team as a code owner July 9, 2026 16:46
@ryan-lempka
ryan-lempka requested a review from ayushag-nv July 9, 2026 16:48
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR removes secret-scanning infrastructure: the "Secret scan" CI job and its dependency in ci-success, the detect-secrets pre-commit hook, the NvidiaApiKeyDetector script, and the .secrets.baseline file. Commitlint pre-commit config gains an explicit config-conventional dependency pin.

Changes

Secret scanning removal

Layer / File(s) Summary
CI workflow update
.github/workflows/ci.yml
Removes the Secret scan job and drops secrets from the ci-success job's needs list.
Pre-commit config and hook removal
.pre-commit-config.yaml, .hooks/detect_nvidia_api_key.py, .secrets.baseline
Removes the detect-secrets hook block, the NvidiaApiKeyDetector script, and the .secrets.baseline file; adds an additional_dependencies pin for @commitlint/config-conventional@20.4.1.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

Hop, hop, the secrets scan is gone,
No more baseline to dwell upon,
Commitlint's pinned, tidy and neat,
A leaner pipeline, quick on its feet,
Thump goes my paw in approval, complete! 🐇

🚥 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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: replacing detect-secrets with GitHub secret scanning.

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

@ayushag-nv ayushag-nv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks Good. Thanks for this. It has been causing troubles.

@ayushag-nv
ayushag-nv enabled auto-merge (squash) July 9, 2026 16:52
@ryan-lempka
ryan-lempka disabled auto-merge July 9, 2026 16:55
@ryan-lempka ryan-lempka self-assigned this Jul 9, 2026
Signed-off-by: Ryan Lempka <rlempka@nvidia.com>
@ryan-lempka
ryan-lempka force-pushed the chore/remove-detect-secrets branch from 4a070a6 to c367222 Compare July 9, 2026 17:01
@ryan-lempka
ryan-lempka merged commit 072ae3e into main Jul 9, 2026
13 checks passed
@ryan-lempka
ryan-lempka deleted the chore/remove-detect-secrets branch July 9, 2026 18:30
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