ci(security): TruffleHog secret gate and Trivy SCA - #2002
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds GitHub Actions workflows for Trivy filesystem and container scanning, TruffleHog pull-request and history scanning, SARIF publication, job-summary reporting, and local exclusion of generated scanner artifacts. ChangesSecurity scanning
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/trivy.yml (1)
86-108: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid scanning twice just to get two output formats.
Both the filesystem report job and the per-image scan step run the full Trivy scan twice back-to-back — once with
--format jsonand once with--format sarif— against the identical target and severity filter. Trivy'sconvertsubcommand exists to turn a saved JSON report into SARIF (or any other format) without rescanning, which halves scan time/DB-lookup cost in both places.
.github/workflows/trivy.yml#L86-L108: generatetrivy-fs.jsononce, then run"$TRIVY_IMAGE" convert --format sarif --output trivy-fs.sarif trivy-fs.jsoninstead of a second fullfsscan..github/workflows/trivy-images.yml#L90-L116: generatetrivy-image.jsononce per matrix image, then run"$TRIVY_IMAGE" convert --format sarif --output trivy-image.sarif trivy-image.jsoninstead of a second fullimagescan.♻️ Example for trivy.yml
docker run --rm \ -v "${{ github.workspace }}:/src" \ -v /tmp/trivy-cache:/root/.cache \ "$TRIVY_IMAGE" fs \ --scanners vuln,misconfig \ --severity HIGH,MEDIUM,LOW \ --exit-code 0 \ --no-progress \ --format json --output /src/trivy-fs.json \ /src - docker run --rm \ - -v "${{ github.workspace }}:/src" \ - -v /tmp/trivy-cache:/root/.cache \ - "$TRIVY_IMAGE" fs \ - --scanners vuln,misconfig \ - --severity HIGH,MEDIUM,LOW \ - --exit-code 0 \ - --no-progress \ - --format sarif --output /src/trivy-fs.sarif \ - /src + docker run --rm \ + -v "${{ github.workspace }}:/src" \ + "$TRIVY_IMAGE" convert \ + --format sarif --output /src/trivy-fs.sarif \ + /src/trivy-fs.json🤖 Prompt for 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. In @.github/workflows/trivy.yml around lines 86 - 108, Update the filesystem scan steps to run Trivy once in JSON format, then use the Trivy convert subcommand to generate the SARIF report from that JSON without rescanning. Apply this in .github/workflows/trivy.yml lines 86-108 and .github/workflows/trivy-images.yml lines 90-116, preserving each workflow’s existing targets, severity filters, output filenames, and artifact paths.
🤖 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/trivy.yml:
- Around line 164-170: Add an existence guard using hashFiles(...) to both SARIF
upload conditions: .github/workflows/trivy.yml lines 164-170 must require
trivy-fs.sarif, while .github/workflows/trivy-images.yml lines 158-165 must
require trivy-image.sarif; preserve always() and the existing fork-PR repository
condition in each upload step.
- Around line 58-72: Update the Trivy invocation in the “Trivy fs scan —
CRITICAL only (blocking)” workflow step to explicitly pass the mounted
repository-root ignore file via --ignorefile /src/.trivyignore. Preserve the
existing read-only /src mount and all scan options.
---
Nitpick comments:
In @.github/workflows/trivy.yml:
- Around line 86-108: Update the filesystem scan steps to run Trivy once in JSON
format, then use the Trivy convert subcommand to generate the SARIF report from
that JSON without rescanning. Apply this in .github/workflows/trivy.yml lines
86-108 and .github/workflows/trivy-images.yml lines 90-116, preserving each
workflow’s existing targets, severity filters, output filenames, and artifact
paths.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e50396d3-b209-4f4e-97d1-9d67f80e35bf
📥 Commits
Reviewing files that changed from the base of the PR and between d284207 and ec982abf8b74c708956f3f36031f077cbaf62876.
📒 Files selected for processing (4)
.github/workflows/trivy-images.yml.github/workflows/trivy.yml.github/workflows/trufflehog.yml.gitignore
ec982ab to
eabed72
Compare
Completes the scanning set semgrep.yml started (#1478): secret detection and software-composition analysis, over both the repository and the published images. Rollout state per gate follows the measured baseline rather than a uniform policy. TruffleHog splits in two, because a full-history scan returns 253 findings — all unverified, all triaged as detector false positives: synthetic e2e metric fixtures (178), the loose GitLab token regex firing on prose in docs/ and on identifiers in dbt models, zero-filled UUIDs in test data, CHANGE_ME templates in .env.local.example. - secrets-diff: pull requests only, BLOCKING. Scans just the commits the PR adds, so the baseline is out of range and the gate starts green. - secrets-history: nightly, report-only, every ref, no exclusions, so the 253 stay visible and any rise above them is noticeable. Both pass --results=verified,unknown,unverified,filtered_unverified. The default set hides findings whose live verification fails, so a revoked credential in history yields a green run — on a public repository that is still a leak. --filter-entropy was evaluated as a way to suppress the fixture noise and rejected: at the threshold that clears the baseline (4.0, cutting 253 to 3) a planted AWS access key was suppressed too, since a 20-character key ID sits at that entropy boundary. One matrix entry in trivy-images.yml carries a per-line trufflehog:ignore: the GitLab detector reads that image name as a token. The gate flagged it on this very branch, which is the intended escape hatch — waive the line, keep the detector. Trivy runs three passes. The repository gate blocks on CRITICAL (baseline 0); the repository report covers HIGH/MEDIUM/LOW (baseline 47 misconfigurations, 0 vulnerabilities) without blocking. trivy-images.yml is the one that matters most here: `trivy fs` resolves dependency versions from lock files, and this repository has only Cargo.lock — uv.lock is gitignored and no packages.lock.json exists — so .NET and Python dependencies are invisible to it. Inside the images they are installed and resolvable. First scans found what the repository pass cannot see: 11 CRITICAL and 88 HIGH fixable in one connector image (debian 12.6 base), 2 CRITICAL in the gateway image, 1 CRITICAL in toolbox. Report-only for that reason — blocking on CRITICAL would fail every build today. Raw secret values never reach a log, job summary or artifact: this repository is public, so those are world-readable. Summaries carry detector, commit and path. Scanner images are digest-pinned and actions are SHA-pinned, matching semgrep.yml, so these files add no findings to the rules they run. Signed-off-by: Grigoriy Gogin <Grigoriy.Gogin@constructor.tech>
eabed72 to
d9aeae3
Compare
…once Three findings from review on this PR. The repo-root `.trivyignore` documented on the gate job was never read. Trivy resolves the default ignore file relative to the working directory, and the container had no `-w`, so waivers were looked for in the image's own cwd. Adding `-w` to the mount fixes it. Verified: with a waiver present the gate now passes where it previously ignored the file, and with no `.trivyignore` at all — the current state of the repository — the gate is unaffected. `--ignorefile` is deliberately not used: Trivy exits FATAL when the named file is absent, which would break every run until someone adds a waiver. SARIF upload steps ran under `always()` against files a failed scan never wrote, producing a second error that pointed at Code Scanning instead of the scan that actually broke. A `hashFiles` guard drops that. It matters most in the image matrix, where one failed registry pull reported twice. Both report passes scanned the same target twice to produce two formats. Now the scan runs once to JSON and `trivy convert` derives the SARIF from it — identical output (47 results, unchanged), and for the 14-image matrix it removes 14 redundant pulls and layer analyses. Signed-off-by: Grigoriy Gogin <Grigoriy.Gogin@constructor.tech>
…once Ports three review findings from constructorfabric#2002 to this repo, so the two scanning setups do not drift. The repo-root `.trivyignore` documented on the gate job was never read. Trivy resolves the default ignore file relative to the working directory, and the containers had no `-w`, so waivers were looked for in the image's own cwd. Verified: with a waiver present the gate now honors it where it previously did not, and with no `.trivyignore` at all — the current state of this repo — the gate is unaffected. `--ignorefile` is deliberately not used: Trivy exits FATAL when the named file is absent, which would break every run until someone adds a waiver. SARIF upload steps ran under `always()` against files a failed scan never wrote, producing a second error pointing at Code Scanning instead of the scan that actually broke. A `hashFiles` guard drops that. Both report passes scanned the same target twice to produce two formats. The scan now runs once to JSON and `trivy convert` derives the rest — SARIF for the repository pass, and both the log table and SARIF for the image scan. Output is identical: 31 results for the repository pass and 37 for the image, matching what main currently reports. Signed-off-by: Grigoriy Gogin <Grigoriy.Gogin@constructor.tech>
Completes the scanning set
semgrep.ymlstarted (umbrella #1478, which also referenced a TruffleHog gate that was never added): secret detection and software-composition analysis, over the repository and the published images.Rollout state per gate follows the measured baseline rather than a uniform policy.
secrets (diff)secrets (full history)gate (CRITICAL)— reporeport (HIGH/MEDIUM/LOW)— repoTrivy Images(nightly, 14 images)Why TruffleHog is split instead of blocking on history
A full-history scan returns 253 findings. Every one is unverified, and each was triaged: 178 are synthetic e2e metric fixtures (JiraToken and Lob matching 24- and 40-character fixture strings), the rest are the loose GitLab token regex firing on prose in
docs/and on identifiers in dbt models, zero-filled UUIDs in test data, andCHANGE_MEtemplates in.env.local.example. Blocking on that would be permanently red, so the blocking gate scans the PR range only (--since-commit) and the history sweep reports nightly without blocking.Known gap, stated in the workflow: direct pushes to
mainare covered by the nightly report, not by the blocking gate, because a push has no reliable range to diff against on a new or force-pushed ref. Closing it requires the history baseline to be clean.Two flags worth reviewing
--results=verified,unknown,unverified,filtered_unverified. The default (verified,unknown) hides findings whose live verification fails, so an already-revoked or inactive credential in history produces a green run — on a public repository that is still a leak requiring rotation. Verified against a control repository with a planted AWS key: under the default flags TruffleHog found it and reported nothing.--filter-entropywas evaluated and rejected. It looked like the clean way to suppress the fixture noise — threshold 4.0 cuts 253 findings to 3. On the same control repository that threshold also suppressed the planted AWS key, because a 20-character AWS key ID sits right at that entropy boundary. The flag trades a known false-negative class for cosmetic quiet.Why
trivy-images.ymlcarries most of the valuetrivy fsresolves dependency versions from lock files, and this repository has exactly one:Cargo.lock.uv.lockis gitignored and nopackages.lock.jsonexists, so the five.csprojand elevenpyproject.tomlprojects are skipped silently — the repository pass covers Rust only. A green result there says much less than it appears to.Inside the published images those dependencies are installed and resolvable. First scans:
source-gitlab-insightnltk×4)insight-gatewayinsight-toolboxtar@6.2.1×8, gox/net×4)Report-only for exactly that reason: blocking on CRITICAL would fail every build today. Barring a vulnerable image from promotion means adding the scan to
build-images.ymlwith--exit-code 1— a change to a much larger workflow, and a separate step. Committing lock files (dotnet restore --use-lock-file, un-ignoringuv.lock) would move Python/.NET coverage earlier, into the repository pass.Notes on what the repository pass found
47 misconfigurations, no vulnerabilities. Two things stand out:
src/frontend/helm/templates/deployment.yamlaccounts for 15 of them (KSV-0014 root filesystem writable, KSV-0118 default security context, KSV-0001 privilege escalation, KSV-0012 runs as root, KSV-0104 seccomp disabled, KSV-0117 privileged ports). The chart declares nosecurityContextat all. For comparison the gateway chart has 6, all LOW/MEDIUM.USER: of 19 Dockerfiles only five run as root, and all five are dev or tooling images (rust-watch,seed, the e2e runner,declarative-connector,toolbox).Neither is touched by this PR.
Test plan
Run locally against this branch.
actionlintover the three new files — exit 0, no findingssecrets-diffsummary script against a control repository with planted AWS and GitLab tokens — exits 1, prints detector/commit/path, prints no secret valuesecrets-historysummary script against the real 253 findings — exits 0, baseline logic renders--since-commitdiff mode — 43 chunks over 5 commits, confirming the range is honoredtrivy-fscategory recorded 47 results againstrefs/pull/2002/merge, matching the local count exactly (9 HIGH / 8 MEDIUM / 30 LOW)workflow_dispatchSummary by CodeRabbit
The gate flagged this PR, and that was the point
secrets (diff)failed on the first push: TruffleHog's GitLab detector read the matrix entrysource-hubspot-insightas a token — a 21-character string next to the keyword. Exactly the false-positive class documented above, arriving in the PR that introduces the gate. Two things surfaced while fixing it, both worth knowing before anyone hits them:trufflehog:ignoreonly suppresses a finding when the marker sits on the same line. A comment above the line does nothing.Because the scan covers every commit a PR adds, a follow-up commit cannot clear it — the original commit without the waiver is still in range. The branch was therefore squashed so the waiver is present from the first commit. Verified on a clean clone of the squashed branch: 0 findings over the same range the gate uses.
Refs #2020