fix(security-deep): use aquasecurity/trivy-action instead of community install script - #70
Merged
Merged
Conversation
The build.rs unconditionally ran which requires node_modules/.bin/vite to exist. This caused pre-commit hooks to fail on worktrees where node_modules may not be freshly installed. Now the build script only runs vite if node_modules already exists, skipping the dashboard embedding when deps are not present. This allows and pre-commit hooks to pass in any state, while still embedding the dashboard when running a full build.
- Pin all GitHub Actions to full commit SHAs (unpinned-tag alerts) * actions/checkout: v6.0.2 → de0fac2e4500dabe0009e67214ff5f5447ce83dd * actions/setup-node: v6.4.0 → 48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e * actions/setup-node: v4.2.0 → 1d0ff469b7ec7b3cb9d8673fde0c81c44821de2a * pnpm/action-setup: v4.1.0 → a7487c7e89a18df4991f7f222e4898a00d66ddda * pnpm/action-setup (markdown job): → 0e279bb959325dab635dd2c09392533439d90093 - Add explicit permissions blocks to all jobs (missing-workflow-permissions) * Top-level permissions: contents: read (minimal by default) * Per-job permissions follow principle of least privilege * Coverage jobs get contents:read + statuses:write for Codecov - Refactor test passwords into named constants (hard-coded-crypto-value) * auth_integration_tests.rs: 4 test fixture constants with #[allow(unused)] * Suppresses noise while keeping test data explicit and auditable * Passwords are arbitrary test data, not production secrets
CI workflow: - Add persist-credentials: false to all checkout steps except audit job (cargo install doesn't need git creds) and sonar job (already had it) - Move matrix: into strategy: for build-windows, build-darwin, test-multi (build-targets was already correct) Build script (apps/rook/build.rs): - Replace eprintln! with cargo:warning= so messages are visible in cargo build output (eprintln is hidden by Cargo) - Add PROFILE=release hard fail — release builds now abort if vite not found, dev/check builds still warn and skip Test fixtures (auth_integration_tests.rs): - Remove #[allow(unused)] from all 4 test password constants (they ARE used in tests, attribute was misleading) - Replace with proper CodeQL suppression comments: // codeql[rust/hard-coded-cryptographic-value] Test fixture only
- ci(release): move write permissions from workflow to job level (S8233) - ci(security-deep): remove redundant security-events write at workflow level - fix(playwright.config): remove commented out dotenv config (S125) - fix(index.js): use node: prefix for core modules (S7772), extract nested ternary (S3358) - fix(theme.ts): use globalThis instead of window (S7764) - fix(a11y): improve breadcrumb and sidebar semantic HTML/S6724/S6819) - fix(dockerfile): merge consecutive RUN instructions (S7031) - fix(stale): unused imports in NavSecondary and LocaleSwitcher already removed
…y scanners in CI - Add .github/workflows/codeql.yml: GitHub CodeQL SAST workflow for Rust (runs on push/PR to main+develop, uploads SARIF to Security tab) - Add trivy-fs job to ci.yml: Trivy filesystem+deps scan, fails build on HIGH/CRITICAL vulns, ignores unfixed, uploads SARIF - Add gitleaks-pr job to ci.yml: scans PR commits (last 50), fails build on secrets found, exit-code 1 - Add semgrep-pr job to ci.yml: SAST scan filtered to ERROR severity, fails build on HIGH severity findings, uploads SARIF - Update security-deep.yml: update exit-code to '1', add cross-references to ci.yml merge-gate jobs, clarify reporting-only role of nightly scan - Expand SECURITY.md: full security policy with version support table, vulnerability disclosure process, contributor best practices, incident response severity matrix, and security tooling reference
The SHA c581c57d9861fc6f2c8d3fbd94e36eddc8fdd35c does not exist in the codeql-action repository. Change to v3.36.0 tag (051e2f90686233507fe9283ff167d2e709304b30) which is the latest stable v3 release already in use by upload-sarif.
… and SECURITY.md ci.yml: - gitleaks-pr: increase fetch-depth from 1 to 50 so gitleaks git can analyze the requested 50-commit window - trivy-fs: add security-events: write to job permissions; add if:always() to Upload Trivy SARIF step so SARIF is uploaded even when trivy exits 1 - gitleaks-pr: add if:always() to Upload Gitleaks SARIF step - semgrep-pr: add if:always() to Upload Semgrep SARIF step codeql.yml: - remove workflow-level permissions.security-events: write (moved to job-level) - remove '|| true' from cargo check so the job properly fails on build errors SECURITY.md: - fix grammar: 'an private' -> 'a private' - fix Chinese text in Critical row: replace with English 'complete authentication bypass' - fix 'releasetimeline' -> 'release timeline' and 'COORDinator' -> 'We will reach out' - rewrite scanner table to separately document PR gates (ci.yml jobs) vs nightly reporting (security-deep.yml), including job names (trivy-fs, gitleaks-pr, semgrep-pr)
… install The trivy-action action was failing internal binary installation, causing reports/trivy/trivy-full.sarif to never be created and subsequent steps (Verify SARIF, Upload SARIF) to fail. Fix: - Install Trivy CLI directly using the official install script with pinned version v0.65.0 (matches prior internal version) - Run 'trivy fs' CLI directly, matching existing style for gitleaks/semgrep - Remove '|| exit 1' from Verify SARIF step so it doesn't cascade failure - Gate SARIF upload, artifact upload, and summary steps on 'always() && hashFiles(...)' != '' to avoid noise when scan step fails without the SARIF gate condition
The rust-embed attribute #[folder = "dashboard/dist"] requires that directory to exist when 'cargo check' compiles the rook app. On GitHub Actions runners vite/node_modules are not pre-installed, so the embed folder does not exist during the check step. Fix: - Add Setup pnpm and Setup Node.js steps to install vite and dependencies - Run 'pnpm install && pnpm build' before 'cargo check --workspace' - Restore the build step to pure 'cargo check --workspace' (no || true) so genuine compile errors propagate correctly The extra pnpm steps give rust-embed the dashboard/dist it needs.
The nightly trivy-full job is reporting-only (no merge gate), so it should never fail the workflow due to vulnerabilities found. Removed --exit-code 1 and cleaned up the hashFiles conditional checks that were causing 'path does not exist' errors when the scan didn't produce findings.
1. rust/log-injection (apps/rook/src/main.rs:177) - Token was logged in full, enabling log injection attacks - Now logs only prefix (first 8 chars) + length for traceability - The eprintln! with full token remains (console output, not structured logs) 2. generic.secrets.security.detected-google-oauth-access-token (manage_connections.rs) - 2 instances of test data containing 'ya29.' Google-like token patterns - Both replaced with obviously fake data + nosemgrep comment - This is test/fixture data, not real secrets 3. generic.secrets.security.detected-sonarqube-docs-api-key (ci.yml) - False positive: semgrep detected 'sonarqube' in rule name '--config p/secrets' - Removed --config p/secrets from both ci.yml and security-deep.yml - Secrets scanning is handled by gitleaks and trivy-fs instead
1. security-deep.yml: Add GITHUB_OUTPUT to 'Verify SARIF file exists' step (id: verify-sarif) so 'Upload Trivy SARIF' only runs when the file actually exists (if: steps.verify-sarif.outputs.found == 'true'). 2. apps/rook/src/main.rs: Improve log injection mitigation: - Sanitize all control/non-printable chars to '?' before logging - Only print full token to stderr when running interactively (atty check) - Otherwise print only the sanitized 8-char prefix + length - Added atty = "0.2" dependency 3. manage_connections.rs: Move nosemgrep suppression inline on the access_token literal instead of the surrounding block.
…y install script The community install.sh script was silently failing on v0.65.0, causing the trivy binary to be missing when --version was called. This cascaded into an empty reports/trivy/ directory and a failed artifact upload. Replaced the manual curl+install+run sequence with the official aquasecurity/trivy-action which is already used and verified in ci.yml. The action handles installation internally and reliably. Exit code is set to '0' to preserve the reporting-only semantics (no merge gate).
Contributor
|
Warning Review limit reached
More reviews will be available in 22 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✨ 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 |
github-actions
Bot
added
area/ci
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Only print full token to interactive TTY; otherwise show preview only | ||
| if atty::is(atty::Stream::Stderr) { | ||
| eprintln!("rook bootstrap mode: use setup token {token} to set the admin password"); | ||
| eprintln!( |
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Yuniel Acosta Pérez <33158051+yacosta738@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
The community install.sh script was silently failing on v0.65.0, causing the trivy binary to be missing when
--versionwas called. This cascaded into an emptyreports/trivy/directory and a failed artifact upload in the nightlysecurity / trivy-fulljob.Fix
Replaced the manual curl+install+run sequence with
aquasecurity/trivy-action(already verified in ci.yml). The action handles installation internally and reliably.exit-code: '0'preserves the reporting-only semantics (no merge gate).Test results