chore: add Trivy security scanning and fix non-root container users#3082
chore: add Trivy security scanning and fix non-root container users#3082tac0turtle wants to merge 11 commits intomainfrom
Conversation
Add `make trivy-scan` (filesystem + image) via dockerized Trivy. Fix DS-0002 (HIGH): run evm, testapp, and local-da containers as non-root `ev-node` user, consistent with the grpc Dockerfile. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
Summary of ChangesHello @tac0turtle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the project's security by introducing automated vulnerability scanning with Trivy and hardening several Docker containers to run with reduced privileges. These changes aim to proactively identify and mitigate security risks, ensuring a more robust and secure application environment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Claude finished @alpe's task in 2m 10s —— View job Code Review
Good security improvement overall — non-root containers and integrated Trivy scanning are both solid additions. Several issues need attention before merging. Critical Issues1. Bare binary names in ENTRYPOINT won't work in scratch images
# testapp/Dockerfile
ENTRYPOINT ["testapp"]
# grpc/Dockerfile
ENTRYPOINT ["evgrpc"]Scratch images have no shell and no 2. UID mismatch between Dockerfiles and documentation All Dockerfiles now use The changelog entry in the PR's own base changelog ( Major Issues3. Trivy image pinned to trivy_image := "aquasec/trivy:latest"Trivy's vulnerability DB updates every ~6 hours. Using trivy_image := env("TRIVY_IMAGE", "aquasec/trivy:0.62.0")The CI workflow already does this correctly — the GitHub Actions step pins 4. Migration guide tells users to docker pull <image>
docker start <container-name> # BUG: starts old container, not new image
docker pull <image>
docker rm <container-name>
docker run ... <image> ...Or for compose users: 5.
Minor Issues6. Missing
7. Redundant trivy_run := "docker run --rm -v " + trivy_cache_volume + ":/root/.cache/ -e TRIVY_SEVERITY=" + trivy_severityBoth scan targets already pass 8. Volume name inconsistency across compose files
Positive Highlights
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3082 +/- ##
=======================================
Coverage 60.20% 60.20%
=======================================
Files 115 115
Lines 11902 11902
=======================================
Hits 7166 7166
Misses 3925 3925
Partials 811 811
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces Trivy security scanning capabilities and enhances container security by configuring evm, testapp, and local-da containers to run as a non-root ev-node user. The addition of scripts/security.mk provides a convenient way to perform filesystem and image scans. The changes to the Dockerfiles for evm, testapp, and local-da correctly implement the non-root user principle, which is a significant security improvement. There are minor opportunities for consistency and clarity in the Dockerfile user creation and the default image scanning configuration.
apps/evm/Dockerfile
Outdated
|
|
||
| WORKDIR /root | ||
| RUN addgroup -g 1000 ev-node && \ | ||
| adduser -u 1000 -G ev-node -s /bin/sh -D ev-node |
There was a problem hiding this comment.
The adduser command uses the -D flag, which prevents the creation of a home directory. However, the subsequent WORKDIR /home/ev-node implies that /home/ev-node is intended to be the user's home directory. For consistency with apps/testapp/Dockerfile and clearer intent, it's better to allow adduser to create the home directory by removing the -D flag, or explicitly create it if -D is strictly necessary for other reasons. Removing -D is the most straightforward approach to align with the WORKDIR and chown commands.
adduser -u 1000 -G ev-node -s /bin/sh ev-node
scripts/security.mk
Outdated
| TRIVY_CACHE_VOLUME := trivy-cache | ||
|
|
||
| # Docker images to scan (space-separated, override or extend as needed) | ||
| SCAN_IMAGES ?= evstack:local-dev |
There was a problem hiding this comment.
The SCAN_IMAGES variable defaults to evstack:local-dev. While the comment indicates it can be overridden, having a single specific image as the default might lead to other relevant images being missed during scans if the user doesn't explicitly configure this variable. Consider making this variable empty by default or providing a more generic placeholder, encouraging users to define the images they intend to scan, or adding a clear example of how to extend it for multiple images.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Covers bind mounts, named volumes, Kubernetes init containers, fsGroup, and docker-compose. Links from the changelog entry. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Good call to improve the Dockerfiles. The test fail due to RO volumes now. |
# Conflicts: # CHANGELOG.md # Makefile
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Justfile-based Trivy security workflow and filesystem/image scan targets; converts multiple runtime images and compose services to run as non-root (adjusting volumes/mounts and workdirs); adds a migration guide and sidebar entry; records a BREAKING note in CHANGELOG. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Just as Just runner
participant Docker as Docker daemon
participant Trivy as Trivy (Docker)
participant FS as Repo FS
participant Registry as Image registry
Dev->>Just: run `just trivy-scan`
Just->>Just: invoke `trivy-scan-fs` and `trivy-scan-image`
Just->>Trivy: start FS scan (docker run using `trivy_image`)
Trivy->>FS: scan codebase (vuln, misconfig, secret)
Trivy-->>Just: return FS scan results
Just->>Docker: list images from `scan_images`
loop per image
Just->>Trivy: run image scan (with `trivy_cache_volume`)
Trivy->>Docker: analyze image layers
Trivy->>Registry: fetch layers if needed
Trivy-->>Just: return image scan results
end
Just-->>Dev: print progress and completion messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.just/security.just:
- Around line 11-34: The Trivy targets trivy-scan-fs and trivy-scan-image
currently do not fail CI when findings exist; update both command invocations
(the fs command inside target trivy-scan-fs and the image command inside target
trivy-scan-image) to include an exit code flag (e.g., add --exit-code 1) so
Trivy returns non-zero on findings; optionally expose a variable (e.g.,
trivy_exit_code with default 1) and use {{trivy_exit_code}} in both invocations
so the behavior is configurable while ensuring CI fails on detected issues.
- Line 3: The Trivy image is pinned to "latest" via the trivy_image variable
which allows non-reproducible scans; change trivy_image to a specific release
tag or digest (e.g., aquasec/trivy:0.48.0 or the image@sha256:...) to lock the
tool version, and update any scan invocations (where Trivy is run) to optionally
include --skip-db-update for PR/gate scans while reserving DB updates for
scheduled jobs; update the trivy_image variable and adjust the scan invocation
logic that references trivy_image to implement these changes.
In `@apps/evm/Dockerfile`:
- Around line 23-30: The Dockerfile switches to the non-root user (USER ev-node)
but never sets HOME, so entrypoint.sh (which defaults to $HOME/.evm) may still
resolve to /root; add an ENV HOME=/home/ev-node before the USER ev-node
instruction so $HOME points to /home/ev-node at runtime, and keep the existing
WORKDIR /home/ev-node and chown/chmod steps to ensure ownership/permissions for
entrypoint.sh and the home directory.
In `@CHANGELOG.md`:
- Line 27: Update the changelog entry that starts with "**BREAKING:** Docker
images for `evm`, `testapp`, and `local-da` now run as non-root user `ev-node`
(uid 1000)..." to append the PR reference (e.g., "(`#1234`)") consistent with
neighboring entries; edit the line to include the PR link or number so the entry
matches the format used elsewhere for traceability.
In `@docs/guides/migrate-docker-nonroot.md`:
- Around line 39-44: The step currently pulls the new image but incorrectly uses
docker start on the existing container (so the old image stays in use); replace
the docker start <container-name> instruction with commands that remove and
recreate the container so it runs the new image (e.g., docker rm
<container-name> followed by docker run ... <image> ...), or instruct users to
use docker-compose recreate / docker-compose up -d --force-recreate when using
compose; ensure the docs explicitly state that the container must be recreated
to pick up the new image.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 153f9ac0-dce3-4e35-8776-18168486bb43
📒 Files selected for processing (8)
.just/security.justCHANGELOG.mdapps/evm/Dockerfileapps/testapp/Dockerfiledocs/.vitepress/config.tsdocs/guides/migrate-docker-nonroot.mdjustfiletools/local-da/Dockerfile
| @@ -0,0 +1,35 @@ | |||
| # Security scanning with Trivy (https://trivy.dev) | |||
|
|
|||
| trivy_image := "aquasec/trivy:latest" | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Trivy container image latest tag best practices CI/CD reproducibility
💡 Result:
Recommendation: don’t use :latest for Trivy in CI/CD if you care about reproducibility
latest (and other floating tags) can be retagged to different image contents over time, which breaks determinism. Docker explicitly recommends pinning by digest when you need fully repeatable builds/scans. [1]
Best practices for reproducible Trivy container-image scanning
-
Pin the Trivy tool version (and ideally the image digest)
- Prefer an explicit Trivy release version over
latest. - If you use GitHub Actions, pin the action and the Trivy version it installs (the action supports
version: latestorversion: vX.Y.Z; don’t uselatestfor reproducibility). [2] - Avoid “canary” images/builds for production pipelines. [3]
- Prefer an explicit Trivy release version over
-
Control the vulnerability database lifecycle (this is a major source of “same image, different results”)
- Trivy’s vulnerability DB is updated frequently (published every ~6 hours), so scan results can change even when the target image is unchanged. [4]
- For reproducible runs, cache the DB between CI jobs and/or runs (Trivy explicitly calls out CI caching benefits). [4]
- For “frozen” results (e.g., audits), run with
--skip-db-updateto prevent pulling a newer DB during the scan. [4]
-
If you need strict repeatability across networks/regions, self-host or mirror the Trivy DB
- Trivy distributes DBs as OCI artifacts (notably via GHCR), and its “air-gapped/offline” guidance is designed around controlling that dependency. [5]
- In locked-down enterprise CI, mirror/self-host the DB and point Trivy at your controlled source (so the exact DB used is explicit and reviewable). [5]
-
Separate “reproducible security gate” from “continuously updated security signal”
- Common pattern:
- PR gate: pinned Trivy version + cached/frozen DB (stable pass/fail).
- Scheduled job (daily): same Trivy version (or intentionally updated), DB updates enabled to detect newly disclosed CVEs.
- Common pattern:
Sources: Docker pinning/digest guidance for determinism [1]; Trivy Action supports explicit Trivy version pinning [2]; Trivy warns about canary builds [3]; Trivy DB update frequency + skip-update + caching [4]; Trivy DB distribution via GHCR / offline control concepts [5].
🏁 Script executed:
# Examine the .just/security.just file to understand context
cat -n .just/security.justRepository: evstack/ev-node
Length of output: 1566
🏁 Script executed:
# Check if there are other references to trivy_image or container images in the project
rg "trivy_image|docker|container|image" --type-list | head -20
fd -t f "*.just" -exec rg "trivy_image" {} +Repository: evstack/ev-node
Length of output: 693
Pin Trivy version and consider database update strategy.
Line 3 uses :latest, which allows the Trivy vulnerability database to update automatically (~every 6 hours), causing identical image scans to produce different results. Pin an explicit Trivy version (and optionally the image digest) for reproducible security scans.
The project already caches the Trivy database via trivy_cache_volume, which helps. For further reproducibility, consider:
- Pinning to a specific Trivy release (e.g.,
aquasec/trivy:0.48.0) or digest - Using
--skip-db-updatein PR/gate scans to freeze results; enable DB updates only in scheduled/continuous jobs
🔧 Suggested fix
-trivy_image := "aquasec/trivy:latest"
+trivy_image := env("TRIVY_IMAGE", "aquasec/trivy:0.48.0")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| trivy_image := "aquasec/trivy:latest" | |
| trivy_image := env("TRIVY_IMAGE", "aquasec/trivy:0.48.0") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.just/security.just at line 3, The Trivy image is pinned to "latest" via the
trivy_image variable which allows non-reproducible scans; change trivy_image to
a specific release tag or digest (e.g., aquasec/trivy:0.48.0 or the
image@sha256:...) to lock the tool version, and update any scan invocations
(where Trivy is run) to optionally include --skip-db-update for PR/gate scans
while reserving DB updates for scheduled jobs; update the trivy_image variable
and adjust the scan invocation logic that references trivy_image to implement
these changes.
| ### 3. Pull the new image and restart | ||
|
|
||
| ```bash | ||
| docker pull <image> | ||
| docker start <container-name> | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
docker pull image docker start container does it use new image automatically
💡 Result:
No.
docker pull repo:tagonly updates your local image (it may move therepo:taglabel to a new image ID). Existing containers keep pointing to the image ID they were created from, and that never changes. (forums.docker.com)docker start <container>starts an already-created container, so it cannot switch to a newer image. (stackoverflow.com)
To use the newly pulled image, you must recreate the container (e.g., docker rm <container> then docker run ... repo:tag), or force pulling when creating a new container with docker run --pull=always .... (docs.docker.com)
Citations:
- 1: https://forums.docker.com/t/is-it-safe-to-perform-a-docker-pull-while-the-image-is-being-run/10946?utm_source=openai
- 2: https://stackoverflow.com/questions/68585895/does-docker-automatically-update-the-image-when-i-start-my-docker-container?utm_source=openai
- 3: https://docs.docker.com/reference/cli/docker/container/run?utm_source=openai
Step 3 must recreate the container to use the newly pulled image.
The current step pulls a new image but then runs docker start on the existing container, which continues to use the old image. Containers retain references to the specific image ID they were created with; pulling only updates the local tag label. To apply the new image, the container must be recreated with docker rm <container-name> followed by docker run (or equivalent docker-compose recreate), otherwise the migration appears successful but the old image remains active.
🔧 Suggested fix
### 3. Pull the new image and restart
```bash
docker pull <image>
-docker start <container-name>
+# Recreate container to use the newly pulled image
+docker rm <container-name>
+docker run ... <image> ...</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/guides/migrate-docker-nonroot.md around lines 39 - 44, The step
currently pulls the new image but incorrectly uses docker start on the existing
container (so the old image stays in use); replace the docker start
instruction with commands that remove and recreate the
container so it runs the new image (e.g., docker rm followed by
docker run ... ...), or instruct users to use docker-compose recreate /
docker-compose up -d --force-recreate when using compose; ensure the docs
explicitly state that the container must be recreated to pick up the new image.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/evm/docker-compose.yml (1)
81-83: Align volume names across docker-compose files for consistency.The volume naming convention changed in
apps/evm/docker-compose.ymlfromrethtoev-reth-dataandev-node-evm-data, butexecution/evm/docker/docker-compose.ymlandexecution/evm/docker/docker-compose-full-node.ymlstill use the olderrethnaming. This inconsistency may cause confusion when switching between configurations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/docker-compose.yml` around lines 81 - 83, The volumes in this compose file use non-canonical names (ev-node-evm-data, ev-reth-data); change them to the canonical volume names used in the other EVM compose files (replace ev-reth-data and ev-node-evm-data with the exact volume identifiers used in execution/evm/docker/docker-compose.yml and execution/evm/docker/docker-compose-full-node.yml), and update any service volume mounts in this file that reference ev-reth-data or ev-node-evm-data to use those canonical names so all compose files are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/evm/Dockerfile`:
- Around line 22-23: The Dockerfile ADDUSER invocation is missing the Alpine
non-interactive flag; update the adduser command (the line calling adduser -u
1000 -G ev-node -s /bin/sh ev-node) to include the -D flag so the user is
created without an interactive password prompt (matching other Dockerfiles like
apps/grpc/Dockerfile and tools/local-da/Dockerfile) to prevent the Docker build
from hanging.
---
Nitpick comments:
In `@apps/evm/docker-compose.yml`:
- Around line 81-83: The volumes in this compose file use non-canonical names
(ev-node-evm-data, ev-reth-data); change them to the canonical volume names used
in the other EVM compose files (replace ev-reth-data and ev-node-evm-data with
the exact volume identifiers used in execution/evm/docker/docker-compose.yml and
execution/evm/docker/docker-compose-full-node.yml), and update any service
volume mounts in this file that reference ev-reth-data or ev-node-evm-data to
use those canonical names so all compose files are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e52ddfe0-630d-41da-bfb7-e2f1fb4e0c0a
📒 Files selected for processing (3)
CHANGELOG.mdapps/evm/Dockerfileapps/evm/docker-compose.yml
86a4655 to
8cb45b0
Compare
|
it seems tastora is hardcoding /var/evstack as home folder for test @chatton can we make it optional or rely on default $HOME value now that we have non root image |
* main: fix(syncer): include in-flight and pending-cache work in PendingCount (#3162) chore: re-add replaces for development (#3161) build(deps): Bump quinn-proto from 0.11.13 to 0.11.14 in the cargo group across 1 directory (#3155) build(deps): bump execution/evm (#3160) build(deps): bump ev-node (#3159) chore: remove replaces from ev-node (#3158) chore: fix some function names in comment (#3156) fix: persist snapshot once for avoiding badger vlog (#3153) build(deps): Bump docker/setup-buildx-action from 3 to 4 (#3148) build(deps): Bump the all-go group across 5 directories with 8 updates (#3147) build(deps): Bump docker/login-action from 3 to 4 (#3149) build(deps): Bump docker/build-push-action from 6 to 7 (#3151) build(deps): Bump actions/setup-go from 6.2.0 to 6.3.0 (#3150) chore: prep evm rc.5 (#3145) build(deps): bump ev-node (#3144) feat(block): Event-Driven DA Follower with WebSocket Subscriptions (#3131) build(deps): Bump dompurify from 3.2.6 to 3.3.2 in /docs in the npm_and_yarn group across 1 directory (#3140) feat(benchmarking): adding gas burner test (#3115) chore: minor deduplication (#3139) refactor(store,cache)!: optimize cache restore as O(1) (#3134)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
.just/security.just (2)
8-8: Remove redundantTRIVY_SEVERITYenvironment variable.The
trivy_runcommand setsTRIVY_SEVERITYvia-e, but both scan targets also pass--severity {{trivy_severity}}explicitly. Since CLI flags take precedence over environment variables in Trivy, the env var is ineffective and adds noise.♻️ Suggested simplification
-trivy_run := "docker run --rm -v " + trivy_cache_volume + ":/root/.cache/ -e TRIVY_SEVERITY=" + trivy_severity +trivy_run := "docker run --rm -v " + trivy_cache_volume + ":/root/.cache/"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.just/security.just at line 8, The trivy_run string sets the TRIVY_SEVERITY environment variable which is redundant because the scan commands already pass --severity {{trivy_severity}}; remove the `-e TRIVY_SEVERITY=` portion from the trivy_run construction to avoid a no-op env var and reduce noise (update the trivy_run variable that currently concatenates trivy_cache_volume and TRIVY_SEVERITY, leaving the rest of the command and the existing --severity flags in the scan targets unchanged).
28-36: Consider scanning all images before failing.With Just's default shell settings (
set -e), the loop exits on the first image that has vulnerabilities, leaving remaining images unscanned. If comprehensive reporting is desired, consider capturing exit codes and failing at the end.♻️ Optional: Scan all images, then fail
trivy-scan-image: `@echo` "--> Scanning Docker images with Trivy" - `@for` img in {{scan_images}}; do \ + `@exit_code`=0; \ + for img in {{scan_images}}; do \ echo "--> Scanning image: $img"; \ {{trivy_run}} \ -v /var/run/docker.sock:/var/run/docker.sock \ {{trivy_image}} \ image --severity {{trivy_severity}} \ --exit-code 1 \ - $img; \ - done + $img || exit_code=1; \ + done; \ + exit $exit_code `@echo` "--> Image scan complete"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.just/security.just around lines 28 - 36, The loop over scan_images currently aborts on the first failing trivy run due to Just's set -e; update the loop that iterates over {{scan_images}} to run each trivy invocation but capture its exit code (e.g., store $? in a variable or append to a list) and continue scanning remaining images, then after the for loop check any recorded non-zero exit codes and call exit 1 if any failures occurred; refer to the existing loop and the variables {{trivy_run}}, {{trivy_image}}, and {{trivy_severity}} when making this change.apps/testapp/Dockerfile (1)
16-24: Consider addingENV HOME=/home/ev-nodefor consistency.Other Dockerfiles in this PR (e.g.,
apps/evm/Dockerfileline 22) explicitly setENV HOME=/home/ev-node. Without this,$HOMEmay be undefined or default to/in a scratch image, which could cause issues iftestappor any library it uses references$HOME.Suggested fix
FROM scratch +ENV HOME=/home/ev-node + COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ COPY --from=builder --chown=10001:10001 /home/ev-node /home/ev-node COPY --from=builder /out/testapp /usr/bin/testapp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/testapp/Dockerfile` around lines 16 - 24, The Dockerfile for testapp is missing an explicit HOME environment, which can be undefined in a scratch base and break code that reads $HOME; add an ENV HOME=/home/ev-node declaration (for example directly after WORKDIR /home/ev-node or before USER 10001:10001) so the runtime process and any libraries see the correct home directory for the unprivileged user.apps/grpc/Dockerfile (1)
19-26: Consider addingENV HOME=/home/ev-nodefor consistency.The
apps/evm/DockerfilesetsENV HOME=/home/ev-node, but this file doesn't. Ifevgrpcor any library references$HOME, it may behave unexpectedly.Suggested fix
FROM scratch +ENV HOME=/home/ev-node + COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/grpc/Dockerfile` around lines 19 - 26, Add an explicit HOME environment variable to the Docker image so runtimes or libraries that reference $HOME behave consistently: set ENV HOME=/home/ev-node in the Dockerfile (near WORKDIR /home/ev-node or before USER 10001:10001) so the evgrpc binary and any libraries see the same home directory as in apps/evm/Dockerfile and when running as USER 10001:10001.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/evm/Dockerfile`:
- Around line 24-28: The Dockerfile copies /out/home-ev-node from the build
stage ("build-env") but that path is never created, causing the build to fail;
fix by ensuring the build stage creates and populates /out/home-ev-node (or at
minimum creates the directory and sets ownership) before the final-stage COPY
--from=build-env /out/home-ev-node /home/ev-node is executed — update the
build-env stage to mkdir -p /out/home-ev-node and place the intended files (and
chown to 10001:10001) or remove/adjust the COPY referencing /out/home-ev-node if
that artifact is not needed.
In `@apps/grpc/docker-compose.yml`:
- Around line 39-41: The change replaced the depends_on condition for local-da
from service_healthy to service_started, which can cause evolve-grpc to start
before local-da is accepting connections on port 7980; restore a proper
readiness check by adding a lightweight Docker healthcheck to the local-da
service that probes the DA port (e.g., curl/tcp check on 7980) and then revert
the depends_on entry for evolve-grpc back to condition: service_healthy so
Docker waits for readiness, or alternatively implement retry/backoff connection
logic in evolve-grpc's startup (where it connects to the DA service) to retry
until the DA responds.
In `@apps/grpc/Dockerfile`:
- Around line 32-33: The Dockerfile uses ENTRYPOINT ["evgrpc"] which relies on
PATH resolution that doesn't exist in scratch images; update the ENTRYPOINT to
the binary's absolute path (e.g., ENTRYPOINT ["/usr/local/bin/evgrpc"]) so the
container can locate and execute evgrpc reliably, and keep CMD ["start"] as-is;
locate the ENTRYPOINT line in the Dockerfile and replace the bare "evgrpc" with
the full filesystem path to the built binary.
In `@CHANGELOG.md`:
- Line 46: Update the CHANGELOG.md entry to correct the UID and wording: change
the uid from 1000 to 10001, remove the reference to a named user "ev-node"
(since the Dockerfiles only set USER 10001:10001) and explicitly state that
containers run as numeric UID 10001 (GID 10001) so users will chown volumes to
10001:10001; ensure this matches the Dockerfiles that use the USER 10001:10001
directive.
---
Nitpick comments:
In @.just/security.just:
- Line 8: The trivy_run string sets the TRIVY_SEVERITY environment variable
which is redundant because the scan commands already pass --severity
{{trivy_severity}}; remove the `-e TRIVY_SEVERITY=` portion from the trivy_run
construction to avoid a no-op env var and reduce noise (update the trivy_run
variable that currently concatenates trivy_cache_volume and TRIVY_SEVERITY,
leaving the rest of the command and the existing --severity flags in the scan
targets unchanged).
- Around line 28-36: The loop over scan_images currently aborts on the first
failing trivy run due to Just's set -e; update the loop that iterates over
{{scan_images}} to run each trivy invocation but capture its exit code (e.g.,
store $? in a variable or append to a list) and continue scanning remaining
images, then after the for loop check any recorded non-zero exit codes and call
exit 1 if any failures occurred; refer to the existing loop and the variables
{{trivy_run}}, {{trivy_image}}, and {{trivy_severity}} when making this change.
In `@apps/grpc/Dockerfile`:
- Around line 19-26: Add an explicit HOME environment variable to the Docker
image so runtimes or libraries that reference $HOME behave consistently: set ENV
HOME=/home/ev-node in the Dockerfile (near WORKDIR /home/ev-node or before USER
10001:10001) so the evgrpc binary and any libraries see the same home directory
as in apps/evm/Dockerfile and when running as USER 10001:10001.
In `@apps/testapp/Dockerfile`:
- Around line 16-24: The Dockerfile for testapp is missing an explicit HOME
environment, which can be undefined in a scratch base and break code that reads
$HOME; add an ENV HOME=/home/ev-node declaration (for example directly after
WORKDIR /home/ev-node or before USER 10001:10001) so the runtime process and any
libraries see the correct home directory for the unprivileged user.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01b53a0b-c034-452f-b4e4-bb715c0f8d39
📒 Files selected for processing (10)
.just/security.justCHANGELOG.mdapps/evm/Dockerfileapps/evm/docker-compose.ymlapps/grpc/Dockerfileapps/grpc/docker-compose.ymlapps/testapp/Dockerfileexecution/evm/docker/docker-compose-full-node.ymlexecution/evm/docker/docker-compose.ymltools/local-da/Dockerfile
| depends_on: | ||
| local-da: | ||
| condition: service_healthy | ||
| condition: service_started |
There was a problem hiding this comment.
Removing healthcheck may cause startup race conditions.
Changing from service_healthy to service_started means evolve-grpc will start as soon as the local-da container starts, not when it's actually ready to accept connections on port 7980. This could cause transient connection failures on startup.
Consider adding a lightweight healthcheck back to local-da, or ensure evolve-grpc has retry logic for connecting to the DA service.
💡 Suggested healthcheck for local-da
tmpfs:
- /tmp
+ healthcheck:
+ test: ["CMD-SHELL", "nc -z localhost 7980 || exit 1"]
+ interval: 5s
+ timeout: 3s
+ retries: 5Then revert the depends_on condition:
depends_on:
local-da:
- condition: service_started
+ condition: service_healthy🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/grpc/docker-compose.yml` around lines 39 - 41, The change replaced the
depends_on condition for local-da from service_healthy to service_started, which
can cause evolve-grpc to start before local-da is accepting connections on port
7980; restore a proper readiness check by adding a lightweight Docker
healthcheck to the local-da service that probes the DA port (e.g., curl/tcp
check on 7980) and then revert the depends_on entry for evolve-grpc back to
condition: service_healthy so Docker waits for readiness, or alternatively
implement retry/backoff connection logic in evolve-grpc's startup (where it
connects to the DA service) to retry until the DA responds.
| ENTRYPOINT ["evgrpc"] | ||
|
|
||
| # Default command | ||
| CMD ["start"] |
There was a problem hiding this comment.
Use full path in ENTRYPOINT for scratch images.
In a scratch image, there's no shell and no default PATH. The entrypoint ["evgrpc"] relies on Docker's search behavior, which may not find the binary at /usr/local/bin/evgrpc. Use the full path for reliability.
Suggested fix
-ENTRYPOINT ["evgrpc"]
+ENTRYPOINT ["/usr/local/bin/evgrpc"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ENTRYPOINT ["evgrpc"] | |
| # Default command | |
| CMD ["start"] | |
| ENTRYPOINT ["/usr/local/bin/evgrpc"] | |
| CMD ["start"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/grpc/Dockerfile` around lines 32 - 33, The Dockerfile uses ENTRYPOINT
["evgrpc"] which relies on PATH resolution that doesn't exist in scratch images;
update the ENTRYPOINT to the binary's absolute path (e.g., ENTRYPOINT
["/usr/local/bin/evgrpc"]) so the container can locate and execute evgrpc
reliably, and keep CMD ["start"] as-is; locate the ENTRYPOINT line in the
Dockerfile and replace the bare "evgrpc" with the full filesystem path to the
built binary.
| tags: ghcr.io/${{ github.repository_owner }}/${{ matrix.app.name }}:${{ inputs.image-tag }} | ||
|
|
||
| - name: Scan pushed image with Trivy | ||
| uses: aquasecurity/[email protected] |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
Add
make trivy-scan(filesystem + image) via dockerized Trivy. Fix DS-0002 (HIGH): run evm, testapp, and local-da containers as non-rootev-nodeuser, consistent with the grpc Dockerfile.Overview
add container users
Summary by CodeRabbit
Breaking Changes
New Features
Documentation