Skip to content

chore: add Trivy security scanning and fix non-root container users#3082

Open
tac0turtle wants to merge 6 commits intomainfrom
marko/add-trivy-scan
Open

chore: add Trivy security scanning and fix non-root container users#3082
tac0turtle wants to merge 6 commits intomainfrom
marko/add-trivy-scan

Conversation

@tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Feb 18, 2026

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.

Overview

add container users

Summary by CodeRabbit

  • Breaking Changes

    • Containers (evm, testapp, local-da) now run as non-root (uid 1000); existing bind mounts or volumes with root-owned files may need permission changes—see migration guide.
    • Volume names/mappings for the evm stack were updated and may require adjustments.
  • New Features

    • Automated Trivy security scans added for repo filesystem and configured images with progress output.
  • Documentation

    • Migration guide added for running Docker containers as non-root (incl. Docker Compose & Kubernetes).

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 <noreply@anthropic.com>
@tac0turtle tac0turtle marked this pull request as ready for review February 18, 2026 14:43
@tac0turtle tac0turtle requested a review from chatton February 18, 2026 14:43
@tac0turtle tac0turtle requested a review from auricom February 18, 2026 14:43
@github-actions
Copy link
Contributor

github-actions bot commented Feb 18, 2026

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedMar 5, 2026, 5:26 PM

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @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

  • Security Scanning: Integrated Trivy security scanning into the build process, allowing for both filesystem and Docker image vulnerability, misconfiguration, and secret scanning via new make trivy-scan targets.
  • Container Security Hardening: Addressed the DS-0002 (HIGH) vulnerability by configuring the evm, testapp, and local-da Docker containers to run as a dedicated non-root ev-node user, enhancing security posture.

🧠 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
  • Makefile
    • Included the new scripts/security.mk makefile.
  • apps/evm/Dockerfile
    • Created a new ev-node user and group with UID/GID 1000.
    • Updated the working directory from /root to /home/ev-node.
    • Modified the chmod command to also chown the /home/ev-node directory to ev-node.
    • Set the container to run as the ev-node user.
  • apps/testapp/Dockerfile
    • Created a new ev-node user and group with UID/GID 1000.
    • Updated the working directory from /apps to /home/ev-node.
    • Changed ownership of /home/ev-node to ev-node.
    • Set the container to run as the ev-node user.
  • scripts/security.mk
    • Added a new makefile for Trivy security scanning.
    • Defined variables for Trivy image, severity, cache volume, and images to scan.
    • Implemented trivy-scan target to execute both filesystem and image scans.
    • Created trivy-scan-fs target for scanning the repository filesystem for vulnerabilities, misconfigurations, and secrets.
    • Created trivy-scan-image target for scanning built Docker images.
  • tools/local-da/Dockerfile
    • Created a new ev-node user and group with UID/GID 1000.
    • Updated the working directory from /root to /home/ev-node.
    • Changed ownership of /home/ev-node to ev-node.
    • Set the container to run as the ev-node user.
Activity
  • No human activity has been recorded on this pull request yet.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@claude
Copy link
Contributor

claude bot commented Feb 18, 2026

Claude finished @auricom's task in 1m 32s —— View job


Code Review

  • Gather context and understand the PR
  • Read changed files
  • Review security scanning setup (.just/security.just)
  • Review Dockerfile changes (evm, testapp, local-da)
  • Review docker-compose changes
  • Review documentation
  • Post comprehensive review

Good security work overall. The non-root container changes are well-structured, consistent with the existing grpc Dockerfile pattern, and the migration guide is thorough. A few issues need attention before merging.

Critical: Trivy scans don't fail on findings

.just/security.just:16-21 and :27-34

By default, Trivy exits with code 0 even when it finds vulnerabilities. Without --exit-code 1, these targets will always succeed, making them useless as a CI gate. The security scan will print findings but never block a build.

+trivy_exit_code := env("TRIVY_EXIT_CODE", "1")

 # trivy-scan-fs
         fs --scanners vuln,misconfig,secret \
+        --exit-code {{trivy_exit_code}} \
         --severity {{trivy_severity}} \
         /workspace

 # trivy-scan-image
             image --severity {{trivy_severity}} \
+            --exit-code {{trivy_exit_code}} \
             $img; \

Using a variable allows callers to opt out (TRIVY_EXIT_CODE=0 just trivy-scan) for informational-only runs while keeping the default behavior safe. Fix this →

Important: trivy_image pinned to latest — non-reproducible scans

.just/security.just:3

-trivy_image := "aquasec/trivy:latest"
+trivy_image := env("TRIVY_IMAGE", "aquasec/trivy:0.60.0")

latest floats, meaning the same code can produce different scan results over time. The project already caches the vulnerability DB via trivy_cache_volume, which helps with DB reproducibility, but the tool binary itself should also be pinned. Fix this →

Important: Migration guide step 3 will silently use the old image

docs/guides/migrate-docker-nonroot.md:39-44

# Current (WRONG):
docker pull <image>
docker start <container-name>   # ← starts existing container, ignores new image

docker start resumes an existing container, which retains a reference to the image ID it was created from. Pulling a new image does not automatically update running or stopped containers. The user must recreate the container:

# Correct:
docker pull <image>
docker rm <container-name>
docker run [original flags] <image>

# Or with docker-compose:
docker-compose up -d --force-recreate

Fix this →

Minor: Volume name inconsistency across compose files

apps/evm/docker-compose.yml:81-83 renames volumes to ev-node-evm-data and ev-reth-data, but execution/evm/docker/docker-compose.yml:67 still uses reth. These appear to be parallel compose configurations for the same stack — if users switch between them they'll lose their data or have duplicate orphaned volumes. Worth aligning the naming, or at least documenting that these are separate environments.

Minor: ev-reth bind mounts still target /root/ paths

apps/evm/docker-compose.yml:15-16

volumes:
  - ./chain:/root/chain:ro
  - ./jwttoken:/root/jwt:ro

These mount into ev-reth (the external ghcr.io/evstack/ev-reth image) — not an image owned by this repo. Just worth noting: if ev-reth is also migrated to non-root in the future these paths will need updating. No action needed now.

What's good

  • ENV HOME=/home/ev-node is set before USER ev-node in apps/evm/Dockerfile — this correctly fixes the entrypoint.sh which defaults $HOME/.evm as the config dir. The concern raised by @alpe about /root/.evm-single permission errors should be resolved by this.
  • -D flag is correctly present in all Alpine adduser calls (evm, local-da, grpc), preventing interactive password prompts.
  • Consistent UID/GID 1000 across all images matches the grpc Dockerfile pattern.
  • Migration guide is comprehensive — the init container and fsGroup Kubernetes examples are well thought-out.
  • Trivy cache volume reuse between scans avoids redundant DB downloads.
  • The CHANGELOG.md breaking change entry is appropriately prominent.

@codecov
Copy link

codecov bot commented Feb 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.94%. Comparing base (b1e3010) to head (517f2f4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3082      +/-   ##
==========================================
- Coverage   61.00%   60.94%   -0.06%     
==========================================
  Files         113      113              
  Lines       11700    11700              
==========================================
- Hits         7137     7131       -6     
- Misses       3753     3759       +6     
  Partials      810      810              
Flag Coverage Δ
combined 60.94% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


WORKDIR /root
RUN addgroup -g 1000 ev-node && \
adduser -u 1000 -G ev-node -s /bin/sh -D ev-node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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

TRIVY_CACHE_VOLUME := trivy-cache

# Docker images to scan (space-separated, override or extend as needed)
SCAN_IMAGES ?= evstack:local-dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

tac0turtle and others added 2 commits February 18, 2026 15:55
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Covers bind mounts, named volumes, Kubernetes init containers,
fsGroup, and docker-compose. Links from the changelog entry.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@alpe
Copy link
Contributor

alpe commented Feb 19, 2026

Good call to improve the Dockerfiles. The test fail due to RO volumes now.
init container: init evm-single: exit code 1: Error: error validating config: could not create directory "/root/.evm-single/config": mkdir /root/.evm-single: permission denied

# Conflicts:
#	CHANGELOG.md
#	Makefile
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7885e702-eb62-4579-a718-1fff959306b5

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb45b0 and 517f2f4.

📒 Files selected for processing (1)
  • CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

📝 Walkthrough

Walkthrough

Adds a Justfile-based Trivy security workflow and converts runtime container users to non-root ev-node (UID/GID 1000); updates docker-compose volume names and mount paths, adds a migration guide and sidebar entry, and records a BREAKING note in CHANGELOG.

Changes

Cohort / File(s) Summary
Security Workflow
.just/security.just, justfile
Add Trivy-based scan recipes and configurable variables (trivy_image, trivy_severity, trivy_cache_volume, scan_images); introduce trivy-scan, trivy-scan-fs, trivy-scan-image and import security.just into top-level justfile.
Runtime Dockerfiles
apps/evm/Dockerfile, apps/testapp/Dockerfile, tools/local-da/Dockerfile
Final images now create ev-node (UID/GID 1000), set HOME to /home/ev-node, adjust WORKDIR, chown relevant dirs, and switch runtime USER to ev-node.
Compose / Volumes
apps/evm/docker-compose.yml
Rename/replace persistent volumes and service mounts to non-root paths (evm-single-dataev-node-evm-data, rethev-reth-data) and change mount targets to /home/ev-node/... where applicable.
Docs & Changelog
docs/guides/migrate-docker-nonroot.md, docs/.vitepress/config.ts, CHANGELOG.md
Add migration guide for moving containers to non-root, add sidebar link, and add BREAKING note that specified containers now run as non-root ev-node (UID 1000).

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer
    participant Just as Just runner
    participant Trivy as Trivy (Docker)
    participant Docker as Docker daemon
    participant FS as Repo FS
    participant Registry as Image registry

    Dev->>Just: run `just trivy-scan`
    Just->>Just: run `trivy-scan-fs` and `trivy-scan-image`
    Just->>Trivy: start Trivy for FS scan (via Docker image)
    Trivy->>FS: scan filesystem (vuln, misconfig, secret)
    Trivy-->>Just: FS scan results
    Just->>Docker: enumerate images from `scan_images`
    loop per image
      Just->>Trivy: start Trivy for image (with cache volume)
      Trivy->>Docker: analyze image layers
      Trivy->>Registry: fetch layers if needed
      Trivy-->>Just: image scan results
    end
    Just-->>Dev: print progress and completion messages
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hop through lines with whiskered care,
Trivy at paw, I sniff each layer.
ev-node nibbles root away,
Volumes shifted, night and day.
A tidy yard—safe code to share.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description mentions the main objectives but lacks detail about implementation approach, testing, migration impact, and only provides a minimal 'Overview' section. Expand the Overview section with more details: explain the security rationale, mention the migration guide for existing deployments, and clarify the current test failures and planned resolution.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main changes: adding Trivy security scanning and fixing non-root container users for three services.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch marko/add-trivy-scan

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2c75e9e and 62e68bd.

📒 Files selected for processing (8)
  • .just/security.just
  • CHANGELOG.md
  • apps/evm/Dockerfile
  • apps/testapp/Dockerfile
  • docs/.vitepress/config.ts
  • docs/guides/migrate-docker-nonroot.md
  • justfile
  • tools/local-da/Dockerfile

@@ -0,0 +1,35 @@
# Security scanning with Trivy (https://trivy.dev)

trivy_image := "aquasec/trivy:latest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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

  1. 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: latest or version: vX.Y.Z; don’t use latest for reproducibility). [2]
    • Avoid “canary” images/builds for production pipelines. [3]
  2. 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-update to prevent pulling a newer DB during the scan. [4]
  3. 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]
  4. 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.

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.just

Repository: 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-update in 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.

Suggested change
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.

Comment on lines +11 to +34
trivy-scan: trivy-scan-fs trivy-scan-image

# Scan repo for dependency vulnerabilities, misconfigs, and secrets
trivy-scan-fs:
@echo "--> Scanning repository filesystem with Trivy"
@{{trivy_run}} \
-v {{justfile_directory()}}:/workspace \
{{trivy_image}} \
fs --scanners vuln,misconfig,secret \
--severity {{trivy_severity}} \
/workspace
@echo "--> Filesystem scan complete"

# Scan built Docker images for vulnerabilities
trivy-scan-image:
@echo "--> Scanning Docker images with Trivy"
@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}} \
$img; \
done
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Trivy exit code vulnerabilities found documentation

💡 Result:

Trivy’s default behavior is to exit with code 0 even if vulnerabilities (or other security issues) are found. To make a CI job fail when findings exist, set a non‑zero exit code via --exit-code (or the config/action equivalent). (trivy.dev)

CLI (Trivy)

  • --exit-code <n>: “Exit code when vulnerabilities were found (default: 0).” (trivy.dev)
  • Common pattern (fail build if matching findings are present):
    • trivy image --severity HIGH,CRITICAL --exit-code 1 <image>
    • trivy fs --severity HIGH,CRITICAL --exit-code 1 <path> (trivy.dev)

GitHub Action (aquasecurity/trivy-action)

  • exit-code input: “Exit code when specified vulnerabilities are found” (default 0). (github.com)

Citations:


Make scans fail on findings.

The current implementation lacks explicit failure behavior. By default, Trivy exits with code 0 even when vulnerabilities are found. Add --exit-code 1 (or a configurable equivalent) to both trivy-scan-fs and trivy-scan-image so that CI jobs fail when security issues are detected.

🔧 Suggested fix
 trivy_severity := env("TRIVY_SEVERITY", "CRITICAL,HIGH")
+trivy_exit_code := env("TRIVY_EXIT_CODE", "1")
@@
         fs --scanners vuln,misconfig,secret \
+        --exit-code {{trivy_exit_code}} \
         --severity {{trivy_severity}} \
         /workspace
@@
             {{trivy_image}} \
             image --severity {{trivy_severity}} \
+            --exit-code {{trivy_exit_code}} \
             $img; \
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.just/security.just around lines 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.

Comment on lines +39 to +44
### 3. Pull the new image and restart

```bash
docker pull <image>
docker start <container-name>
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

docker pull image docker start container does it use new image automatically

💡 Result:

No.

  • docker pull repo:tag only updates your local image (it may move the repo:tag label 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:


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 -->

@auricom auricom self-assigned this Mar 5, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.yml from reth to ev-reth-data and ev-node-evm-data, but execution/evm/docker/docker-compose.yml and execution/evm/docker/docker-compose-full-node.yml still use the older reth naming. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 62e68bd and 86a4655.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • apps/evm/Dockerfile
  • apps/evm/docker-compose.yml

Comment on lines +22 to +23
RUN addgroup -g 1000 ev-node && \
adduser -u 1000 -G ev-node -s /bin/sh ev-node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Search for adduser usage in Dockerfiles across the repo
rg 'adduser' --type dockerfile -n -B2 -A2

Repository: evstack/ev-node

Length of output: 93


🏁 Script executed:

# Also check for addgroup and adduser patterns
fd -name 'Dockerfile*' -type f | head -20 | xargs -I {} sh -c 'echo "=== {} ===" && grep -n "add\(user\|group\)" {} 2>/dev/null || true'

Repository: evstack/ev-node

Length of output: 289


🌐 Web query:

Alpine Linux adduser -D flag documentation required non-interactive

💡 Result:

In Alpine, adduser is the BusyBox applet. Its -D flag is documented as:

  • -D — “Do not assign a password” (so it won’t prompt, i.e., suitable for non-interactive user creation). [1]

However, in practice BusyBox adduser -D has been reported to create the account with a locked password field in /etc/shadow (e.g., !), not a usable passwordless login. [1] Alpine’s own docs therefore commonly suggest setting a password afterward with passwd <user> if one wasn’t set during creation. [2]

Where to get the local documentation/manpages on Alpine:

  • Install BusyBox’s documentation package: apk add busybox-doc (package description: BusyBox documentation). [3]
  • Or just view the built-in help: adduser --help / busybox adduser --help (the same option text is shown/quoted in BusyBox’s bug report). [1]

Sources

  1. BusyBox bug report quoting adduser option doc and showing -D result in /etc/shadow. https://lists.busybox.net/pipermail/busybox-cvs/2018-April/038077.html
  2. Alpine User Handbook (post-install): set a password with passwd if the tool didn’t ask. https://docs.alpinelinux.org/user-handbook/0.1a/Working/post-install.html
  3. Alpine package index: busybox-doc package details. https://pkgs.alpinelinux.org/package/edge/main/x86/busybox-doc

🏁 Script executed:

# Find all Dockerfile files
find . -name 'Dockerfile*' -type f

Repository: evstack/ev-node

Length of output: 157


🏁 Script executed:

# Check the specific file mentioned in the review
cat -n apps/evm/Dockerfile | sed -n '15,35p'

Repository: evstack/ev-node

Length of output: 660


🏁 Script executed:

# Search for adduser in any Dockerfiles
find . -name 'Dockerfile*' -type f -exec grep -l 'adduser' {} \;

Repository: evstack/ev-node

Length of output: 131


🏁 Script executed:

# Check how other Dockerfiles in the repo create users
grep -n 'adduser' ./apps/grpc/Dockerfile ./tools/local-da/Dockerfile -A 1 -B 1

Repository: evstack/ev-node

Length of output: 408


🏁 Script executed:

# Also check apps/testapp since it was in the list
grep -n 'adduser' ./apps/testapp/Dockerfile -A 1 -B 1

Repository: evstack/ev-node

Length of output: 41


Missing -D flag in adduser command will cause Docker build to hang.

Alpine's adduser requires the -D flag to skip the interactive password prompt. Without it, the Docker build will hang indefinitely during the password prompt phase. This is inconsistent with other Dockerfiles in the repository (e.g., apps/grpc/Dockerfile, tools/local-da/Dockerfile), which correctly use the -D flag.

🔧 Proposed fix
RUN addgroup -g 1000 ev-node && \
-    adduser -u 1000 -G ev-node -s /bin/sh ev-node
+    adduser -D -u 1000 -G ev-node -s /bin/sh ev-node
📝 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.

Suggested change
RUN addgroup -g 1000 ev-node && \
adduser -u 1000 -G ev-node -s /bin/sh ev-node
RUN addgroup -g 1000 ev-node && \
adduser -D -u 1000 -G ev-node -s /bin/sh ev-node
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/evm/Dockerfile` around lines 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.

@auricom auricom force-pushed the marko/add-trivy-scan branch from 86a4655 to 8cb45b0 Compare March 5, 2026 16:44
@auricom
Copy link
Contributor

auricom commented Mar 6, 2026

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants