Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
|
Need the big picture first? Review this PR in Change Stack to see what changed before going file by file. 📝 WalkthroughWalkthroughFive Dockerfiles add the Dockerfile frontend syntax, pin and upgrade docker-compose-wait to 2.12.1 in publish stages, and change final-stage startup to use ChangesDocker startup and wait mechanism upgrade
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Web/Resgrid.Web.Eventing/Dockerfile`:
- Around line 29-30: The ADD step that downloads the
ufoscout/docker-compose-wait binary needs integrity verification: update the
Dockerfile where the ADD downloads to /app/publish/wait (the ADD https.../wait
line) to verify the SHA256
2241be671073520e028b2f12df1e9ef0419014cffb5670b7a80b2080804be17d either by using
BuildKit's ADD --checksum=sha256:<hash> (ensure a `#syntax`=... frontend >=1.6
directive is present) or replace the ADD with an explicit download step that
saves the file, computes and checks the sha256sum against that value, and fails
the build if it doesn't match before running chmod +x /app/publish/wait.
In `@Web/Resgrid.Web.Mcp/Dockerfile`:
- Around line 40-41: The Dockerfile currently ADDs the wait binary directly (ADD
https://github.com/ufoscout/docker-compose-wait/releases/download/2.12.1/wait ->
/app/publish/wait) and then chmods it; change this to download and verify the
SHA256 checksum for the 2.12.1 release before making it executable: fetch the
official checksum (or embed the known SHA256 for version 2.12.1), compute the
downloaded file's SHA256 (e.g., via sha256sum) and compare it, fail the build if
it doesn’t match, and only then run chmod +x on /app/publish/wait so the image
only includes a verified binary.
In `@Web/Resgrid.Web.Services/Dockerfile`:
- Around line 40-41: Replace the unconditional ADD of the external binary with a
download + checksum verification flow: instead of "ADD https://.../wait
/app/publish/wait" and "RUN chmod +x /app/publish/wait", download the file to a
temporary path (e.g., /tmp/wait) using curl/wget, verify its SHA256 (compare
against a hardcoded expected checksum string) with sha256sum (or printf ... |
sha256sum -c), fail the build if verification fails, then mv the verified file
to /app/publish/wait and chmod +x it; update the Dockerfile to use this sequence
so the binary at /app/publish/wait is only installed after checksum
verification.
In `@Web/Resgrid.Web/Dockerfile`:
- Around line 58-59: The Dockerfile currently adds the docker-compose-wait
binary via ADD without verifying integrity; replace the ADD of the binary
(/app/publish/wait) with a secure download and checksum verification: fetch the
release artifact and its published SHA256 (or a vendor-provided checksum),
compute the downloaded file's SHA256 and compare it before setting executable
bit, and fail the build if the checksum does not match; update the Dockerfile
steps that reference the wait binary (the current ADD and subsequent chmod +x
/app/publish/wait) to use the verified-download-and-verify sequence so the build
only proceeds with a checksum-validated /app/publish/wait.
In `@Workers/Resgrid.Workers.Console/Dockerfile`:
- Around line 40-41: Replace the unconditional ADD of the docker-compose-wait
binary with a verified download flow: introduce a build ARG (e.g., WAIT_SHA256)
containing the expected sha256, download the binary to /app/publish/wait (or
temp file) and compute its sha256sum inside the Dockerfile, compare it to
WAIT_SHA256, and fail the build if they differ; only after verification run
chmod +x /app/publish/wait and remove any temporary files. Reference the
existing ADD line and the chmod +x /app/publish/wait step to locate where to
implement the checksum check and ARG injection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bada4d05-b3d1-47c6-bc9f-c541fb2bb030
📒 Files selected for processing (5)
Web/Resgrid.Web.Eventing/DockerfileWeb/Resgrid.Web.Mcp/DockerfileWeb/Resgrid.Web.Services/DockerfileWeb/Resgrid.Web/DockerfileWorkers/Resgrid.Workers.Console/Dockerfile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Web/Resgrid.Web.Eventing/Dockerfile (1)
1-1: ⚖️ Poor tradeoffConsider adding a non-root USER directive.
Trivy flags that all five Dockerfiles run containers as root. For defense-in-depth, consider adding a
USERdirective in the final stage to run the application with a non-root user, reducing the impact of potential container escapes.This would require ensuring the application and wait binary have appropriate permissions and that the base image includes a suitable non-root user.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Eventing/Dockerfile` at line 1, Add a non-root runtime user in the Dockerfile's final stage, create a dedicated UID/GID (e.g., appuser), chown the application files and the wait binary to that user and set USER to it; ensure any folders the app writes to (logs, tmp) are owned or writable by that user and that the base image provides required user utilities. Locate the final stage in the Dockerfile (the stage that copies the app and wait binary into the image), add commands to create the user/group, run chown on the application directory and the wait binary, and then add a USER directive so the container runs as the non-root account.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Web/Resgrid.Web.Eventing/Dockerfile`:
- Line 1: Add a non-root runtime user in the Dockerfile's final stage, create a
dedicated UID/GID (e.g., appuser), chown the application files and the wait
binary to that user and set USER to it; ensure any folders the app writes to
(logs, tmp) are owned or writable by that user and that the base image provides
required user utilities. Locate the final stage in the Dockerfile (the stage
that copies the app and wait binary into the image), add commands to create the
user/group, run chown on the application directory and the wait binary, and then
add a USER directive so the container runs as the non-root account.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c51037c3-5228-4501-ade1-d0e11dfcd883
📒 Files selected for processing (5)
Web/Resgrid.Web.Eventing/DockerfileWeb/Resgrid.Web.Mcp/DockerfileWeb/Resgrid.Web.Services/DockerfileWeb/Resgrid.Web/DockerfileWorkers/Resgrid.Workers.Console/Dockerfile
|
Approve |
Summary by CodeRabbit