[SVLS-8660] ci: add gitleaks secrets scanning#1134
[SVLS-8660] ci: add gitleaks secrets scanning#1134litianningdatadog wants to merge 2 commits intotianning.li/SVLS-8660-ci-checksfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds automated secrets scanning to the repo’s CI to prevent credentials from reaching main, alongside configuration for suppressing known false positives.
Changes:
- Add a new GitHub Actions workflow to run
gitleakson PRs and pushes tomain - Add a
.gitleaks.tomlallowlist configuration with maintenance guidance - Add
.github/copilot-instructions.mdwith security-focused code review guidance
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.github/workflows/secrets-scan.yml |
Introduces the gitleaks secrets scanning CI job on PR/push events. |
.gitleaks.toml |
Defines allowlist structure (paths/regexes/commits) and contributor guidance for handling findings. |
.github/copilot-instructions.md |
Adds Copilot review instructions focused on PII/secrets logging, unsafe Rust, and error handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Copilot Code Review Instructions | ||
|
|
||
| ## Security — PII and Secrets | ||
|
|
There was a problem hiding this comment.
The PR description focuses on adding gitleaks scanning and .gitleaks.toml, but this PR also adds .github/copilot-instructions.md and references a strategy doc at .github/docs/ci-security-scanning-strategy.md that doesn’t appear to exist in the repo. Please confirm whether this file is intended to be part of this PR (vs the companion PR #1133) and update the PR description (or split the changes) to avoid confusion.
| ## Security — Unsafe Rust | ||
|
|
||
| Flag new `unsafe` blocks and explain what invariant the author must uphold to make the | ||
| block safe. If there is a safe alternative, suggest it. | ||
|
|
||
| ## Security — Error Handling | ||
|
|
||
| Flag cases where errors are silently swallowed (empty `catch`, `.ok()` without | ||
| handling, `let _ = result`) or where operations like `.unwrap()`/`.expect()` may panic, | ||
| in code paths that handle external input or network responses. |
There was a problem hiding this comment.
Why add this?
Is this from Claude/another agent? Did you just ask "What are important issues to check for in Rust PRs"?
There was a problem hiding this comment.
Not exactly, these were in the implementation plan as intentional Copilot guidance for Rust-specific security patterns, not random AI additions. They cover different layers:
- PII/Secrets — what not to log
- Unsafe Rust — flag unsafe blocks and explain required invariants
- Error Handling — catch silent failures in network/input paths
|
see if this pattern is used somewhere, we need a key to run this thing |
litianningdatadog
changed the base branch from
main
to
tianning.li/SVLS-8660-ci-checks
litianningdatadog
force-pushed
the
tianning.li/SVLS-8660-ci-checks
branch
2 times, most recently
from
530325e to
faba926
Compare
- Add secrets-scan workflow triggered on PR and push to main - Pin gitleaks-action to SHA (v2.3.9) - Add .gitleaks.toml with documented allowlist structure for paths, regexes, and commits with maintenance guidance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
litianningdatadog
force-pushed
the
tianning.li/SVLS-8660-gitleak
branch
from
0a2ea9c to
72adf5b
Compare
- Fix .gitleaks.toml comment to reference correct `commits` key under [allowlist] - Add `permissions: contents: read` to secrets-scan workflow job - Enable `--redact` flag to prevent secret values appearing in CI logs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
litianningdatadog
force-pushed
the
tianning.li/SVLS-8660-gitleak
branch
from
72adf5b to
efbe64d
Compare
Summary
Why
Secrets accidentally committed to a public repo (API keys, tokens, credentials) are the highest-severity risk for a public repository. Neither `cargo audit` nor Copilot catch this category. `gitleaks` uses pattern matching to block merges before a credential lands in `main`.
Test plan
Related
🤖 Generated with Claude Code