refactor(ci): standardize security scans via Makefile targets#3678
Conversation
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Summary by CodeRabbit
WalkthroughReplaces explicit Trivy GitHub Action steps with Makefile-driven Trivy scans across CI; adds/updates Makefile targets for code and image scanning, pins Trivy image, updates Trivy config/ignore, and tweaks several docker bind-mounts and a frontend build arg. (50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Makefile`:
- Around line 116-119: The Makefile Trivy command is missing a line
continuation: add a trailing backslash to the line containing "--config
trivy.yaml" so the final "." token is passed as the scan target to the "fs"
subcommand; update the Makefile rule that builds the Trivy invocation (the lines
using "grep -E '^FROM aquasec/trivy:' ...", "fs", and "--config trivy.yaml") to
include the backslash so "." becomes the target path for the scan.
🧹 Nitpick comments (2)
backend/Makefile (1)
160-162: Typo in comment.Minor: "defalts" should be "defaults" to match the frontend/Makefile comment.
✏️ Proposed fix
-# vars (defalts for Local dev) +# Defaults for Local Development BACKEND_IMAGE_NAME ?= nest-backend-local TRIVY_EXIT_CODE ?= 0docker/trivy/Dockerfile (1)
1-1: Good approach for Dependabot tracking, but version is outdated.Using a dedicated Dockerfile to pin the Trivy version enables Dependabot to track and propose updates automatically. However, version 0.58.0 is significantly behind the latest release (0.68.2 as of December 2025). Consider updating to a more recent version or await Dependabot's automated update proposal.
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
…hub.com/hassaansaleem28/Nest into refactor/ci-use-makefile-security-scans
|
@SpruhaCK I’ve updated the PR with the Makefile fixes. CI checks pass (the SonarCloud failure is a known false positive requiring admin approval). I would love your feedback! If any changes are required, please just tag me. |
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3678 +/- ##
=======================================
Coverage 90.26% 90.26%
=======================================
Files 463 463
Lines 14420 14420
Branches 1934 1934
=======================================
Hits 13016 13016
Misses 987 987
Partials 417 417
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
A couple things I noticed:
- CI/CD still uses action for code scanning
- there is no way to disable secret scanning locally while keeping it enabled on CI/CD
|
Hi @hassaansaleem28,
|
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
.github/workflows/run-ci-cd.yaml
Outdated
| make backend-security-scan-secrets TRIVY_EXIT_CODE=1 | ||
| make frontend-security-scan-secrets TRIVY_EXIT_CODE=1 |
There was a problem hiding this comment.
I don't think we need a separate target for that if we could control it via options similar to TRIVY_EXIT_CODE -- --scanners vuln depending on the env.
backend/Makefile
Outdated
| fs \ | ||
| --scanners vuln,config \ | ||
| --exit-code $(TRIVY_EXIT_CODE) \ | ||
| --severity CRITICAL,HIGH \ |
There was a problem hiding this comment.
It looks like you deleted the configuration I pushed. Please revert.
Where possible params must be taken from trivy configuration file.
docker/frontend/Dockerfile
Outdated
| rm -rf package tar-7.5.7.tgz && \ | ||
| grep -q 'version.*7.5.7' "${TAR_DIR}/package.json" | ||
|
|
||
| # Fix CVE-2025-64756: Update npm's bundled glob to 11.1.0 in runner stage |
There was a problem hiding this comment.
Is this related to the PR?
There was a problem hiding this comment.
while not part of the makefile refactor itself, this change is necessary because this pr enforces the trivy security scans.
My local frontend security scan was failing because trivy flags the current glob package (v10.4.5) as a High vulnerability. Since the goal of this PR is to enforce these checks, the pipeline effectively blocks itself without this fix.
should i remove this and do it in a separate pr depends on if you prefer, but it is currently a blocker for this one passing.
frontend/next.config.ts
Outdated
| serverExternalPackages: ['import-in-the-middle', 'require-in-the-middle'], | ||
| transpilePackages: ['@react-leaflet/core', 'leaflet', 'react-leaflet', 'react-leaflet-cluster'], | ||
| ...(isLocal ? {} : { output: 'standalone' }), | ||
| output: 'standalone' , |
There was a problem hiding this comment.
I believe this is not what we want for dev by default. You probably should set some flag during image local build process.
There was a problem hiding this comment.
right
I added a flag for that.
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/frontend/Dockerfile (1)
55-65:⚠️ Potential issue | 🟡 MinorThe Dockerfile comment needs updating to reflect the actual version and scope.
Verification confirms tar 7.5.7 correctly addresses all three CVEs:
- CVE-2026-23745: fixed in 7.5.3
- CVE-2026-23950: fixed in 7.5.4
- CVE-2026-24842: fixed in 7.5.7
However, the comment at line 55 states "Fix CVE-2026-23745: Update npm's bundled tar to 7.5.3" while the code uses 7.5.7. Update the comment to accurately reflect that 7.5.7 is required to address all three vulnerabilities.
The atomic operation concern is acceptable since the
rm -rfandcp -rsequence is within a singleRUNinstruction—Docker discards the entire layer on failure.
🤖 Fix all issues with AI agents
In @.github/workflows/run-ci-cd.yaml:
- Around line 147-151: Update the comment text containing the typo "shouldnot"
to "should not" so the inline comment above the "Run Trivy security scan" step
reads correctly; locate the comment string "shouldnot run again" and change it
to "should not run again" (no other behavioral changes required).
🧹 Nitpick comments (2)
trivyignore.yaml (1)
4-7: Add TODO comments with remediation timeline for new CVE suppressions.The existing CVE-2025-64756 entry includes a TODO comment indicating when it should be removed. The new suppressions for High-severity vulnerabilities should follow the same pattern to ensure they're revisited and not permanently ignored.
Consider adding version-based or date-based TODO comments similar to line 2, e.g.:
# TODO(author): Remove when <package>@<fixed_version> is adopted.Also, note that CVE-2026-23745, CVE-2026-23950, and CVE-2026-24842 appear to be addressed by the tar 7.5.7 patch in
docker/frontend/Dockerfile. Once that patch is confirmed effective across all scanned images, these suppressions may become unnecessary.backend/Makefile (1)
160-207: LGTM! Minor inconsistency with frontend config path.The backend security scan targets mirror the frontend implementation correctly, with appropriate separation between local and CI modes.
One minor inconsistency: the backend uses absolute path
/trivy.yaml(lines 178, 193) while the frontend uses relative pathtrivy.yaml(lines 77, 92 in frontend/Makefile). Both work since the file is mounted at/trivy.yaml, but consider aligning them for consistency.📝 Optional: align with frontend's relative path
backend-security-scan-image: ... - --config /trivy.yaml \ + --config trivy.yaml \ ... backend-security-scan-code: ... - --config /trivy.yaml \ + --config trivy.yaml \ ...
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 8 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/Makefile">
<violation number="1" location="frontend/Makefile:77">
P2: The `trivy.yaml` configuration uses `vulnerability.security-checks`, which is deprecated and likely ignored in Trivy v0.58.0 (which uses `scanners`). If ignored, Trivy will fall back to default scanners (excluding `misconfig`), causing the "config" check mentioned in the echo command to be skipped.
(Based on your team's feedback about keeping CI and security scanner configuration centralized.) [FEEDBACK_USED]</violation>
</file>
<file name="trivyignore.yaml">
<violation number="1" location="trivyignore.yaml:4">
P2: High-severity CVEs are being permanently ignored without any justification or removal criteria. This can hide real vulnerabilities indefinitely. Add a TODO/issue reference and removal plan (or avoid ignoring these CVEs altogether).</violation>
</file>
<file name="backend/Makefile">
<violation number="1" location="backend/Makefile:201">
P3: The PR description states that the local scan "Warns on secrets/vulns", but `TRIVY_SCANNERS` is explicitly set to `vuln,misconfig`, which excludes the `secret` scanner.
If the intention is to scan for secrets locally, add `secret` to the list. If the intention is to skip secrets for performance, please update the PR description to match the code.</violation>
</file>
<file name=".github/workflows/run-ci-cd.yaml">
<violation number="1" location=".github/workflows/run-ci-cd.yaml:147">
P3: Typo in comment: "shouldnot" should be "should not".</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
|
@arkid15r Thanks for the detailed feedback. I've pushed updates to address all your points:
Happy to make the changes if needed again. Learning alot in this. |
|
There was a problem hiding this comment.
3 issues found across 10 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="Makefile">
<violation number="1" location="Makefile:31">
P2: The `clean-trivy-cache` target will likely fail on Linux systems because the `.trivy-cache` directory is populated by the Docker container running as root.
When `security-scan-code-trivy` runs, Docker creates root-owned files in `.trivy-cache`. The subsequent `rm -rf` command running on the host as a standard user will fail with "Permission denied".
To fix this, perform the cleanup using a Docker container that mounts the directory.</violation>
</file>
<file name=".github/workflows/run-ci-cd.yaml">
<violation number="1" location=".github/workflows/run-ci-cd.yaml:148">
P2: The `Makefile` target `security-scan-code-trivy` does not pass the `TRIVY_EXIT_CODE` environment variable to the Docker container.
This prevents the "Local Mode" (exit code 0) described in the PR from working, as Trivy will fallback to the default `exit-code: 1` defined in `trivy.yaml` when issues are found.
(Based on your team's feedback about verifying changes actually fix the reported issue.) [FEEDBACK_USED]</violation>
<violation number="2" location=".github/workflows/run-ci-cd.yaml:553">
P2: The `Makefile` targets for image scanning also fail to pass `TRIVY_EXIT_CODE` to the Docker container, making the "Strict Mode" and "Local Mode" configuration dependent solely on `trivy.yaml` rather than the environment variable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
trivyignore.yaml
Outdated
|
|
||
| # node-tar: Arbitrary file creation via path traversal (High) | ||
| # https://avd.aquasec.com/nvd/cve-2026-24842 | ||
| - id: CVE-2026-24842 No newline at end of file |
There was a problem hiding this comment.
You didn't run make check
There was a problem hiding this comment.
right, I missed that step this time. I've made a note to strictly run make check before every push from now on. Thanks for headsup.
Thanks for merging and for the follow up commit! I just reviewed I'll ensure the future PRs meets this standard. |
3 participants
Proposed change
Resolves #3615
Description
This PR refactors the CI pipeline to use
maketargets for security scanning instead of inline configuration in the YAML workflow. This aligns our local development environment with CI/CD and solves the hardcoded configuration issues identified in #3615.Key Changes
docker/trivy/Dockerfileto allow Dependabot to track and update the Trivy version automatically.trivy-cache) in the Makefile targets to prevent re-downloading the vulnerability DB on every run.backend/Makefileandfrontend/Makefileto support:TRIVY_EXIT_CODE=0(Runs only misconfigs+vulns and not secrets).TRIVY_EXIT_CODE=1(Fails the build on Critical/High vulnerabilities, runs misconfigs + secrets + vulns).Testing
make security-scan-imagesruns successfully.make security-scan-backend-image TRIVY_EXIT_CODE=1(confirmed 0 critical vulnerabilities).Checklist
make check-testlocally: all warnings addressed, tests passed