[codex] Unify SST service Docker path roots#780
Conversation
|
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:
📝 WalkthroughWalkthroughShifts the Docker build ChangesDocker build path normalization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29709874f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8da2f75 to
432c754
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/runner/Dockerfile (1)
71-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDrop root privileges in the runtime stage.
From Line 71 onward, the final image never sets
USER, soboxlite-runnerruns as root (matching the Trivy DS-0002 finding). Please run the binary as a dedicated non-root user.Suggested patch
FROM debian:bookworm-slim AS runner RUN apt-get update && apt-get install -y --no-install-recommends \ ca-certificates \ curl \ libx11-6 \ libxtst6 \ libxinerama1 \ + passwd \ && rm -rf /var/lib/apt/lists/* +RUN groupadd --system boxlite && useradd --system --gid boxlite --home /nonexistent --shell /usr/sbin/nologin boxlite + WORKDIR /usr/local/bin COPY --from=build /boxlite/apps/dist/apps/runner boxlite-runner -RUN chmod +x boxlite-runner +RUN chmod +x boxlite-runner && chown boxlite:boxlite boxlite-runner + +USER boxlite HEALTHCHECK CMD [ "curl", "-f", "http://localhost:3003/" ] ENTRYPOINT ["boxlite-runner"]🤖 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 `@apps/runner/Dockerfile` around lines 71 - 89, The runner stage executes the boxlite-runner binary as root, which is a security vulnerability. Create a dedicated non-root user in the runner stage (for example, using a RUN command to add a system user or leverage an existing unprivileged user) and add a USER directive to switch to that user before the ENTRYPOINT instruction. This ensures the container runs with reduced privileges rather than as root.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@apps/runner/Dockerfile`:
- Around line 71-89: The runner stage executes the boxlite-runner binary as
root, which is a security vulnerability. Create a dedicated non-root user in the
runner stage (for example, using a RUN command to add a system user or leverage
an existing unprivileged user) and add a USER directive to switch to that user
before the ENTRYPOINT instruction. This ensures the container runs with reduced
privileges rather than as root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 650f5b94-8812-4549-b4e6-201915dff886
📒 Files selected for processing (9)
apps/otel-collector/Dockerfileapps/otel-collector/builder-config.dev.yamlapps/otel-collector/project.jsonapps/proxy/Dockerfileapps/proxy/project.jsonapps/runner/Dockerfileapps/runner/project.jsonapps/ssh-gateway/Dockerfileapps/ssh-gateway/project.json
💤 Files with no reviewable changes (1)
- apps/otel-collector/builder-config.dev.yaml
✅ Files skipped from review due to trivial changes (1)
- apps/ssh-gateway/project.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/proxy/project.json
- apps/runner/project.json
- apps/otel-collector/Dockerfile
- apps/ssh-gateway/Dockerfile
Run SST-built Go service images from the apps workspace root inside Docker, matching the API image and local apps workspace layout. This lets the OtelCollector dev and production builds share one builder config instead of maintaining separate root-specific paths.
432c754 to
58e6c03
Compare
Summary
/boxlite/apps) while keeping Docker build context repo-root-relative.builder-config.dev.yaml; dev and production now sharebuilder-config.yaml.context: ".."andfile: "apps/<service>/Dockerfile"for OtelCollector, Proxy, and SshGateway.Why
Latest
origin/mainfailed duringsst deploy --stage devwhile buildingOtelCollectorImageOtelCollectorbecause the Docker build ran from/boxlitebut the Otel builder config expected an apps-workspace root. A single YAML path tweak would leave the same root split in place.The scoped rule for SST service containers is now:
COPYsources are repo-root-relative because SST uses repo root as build context./boxlite/apps, the actual Nx workspace root./boxlite/apps/go.work.Runner is intentionally out of scope for this PR: production Runner is an EC2 instance that downloads a GitHub Release binary via SST user-data, not an SST-built service container.
Verification
Passed on remote dev machine at commit
58e6c03e:git diff --checkorigin/main.cd apps && yarn nx build otel-collector --nxBail=truedocker build -f apps/otel-collector/Dockerfile --target otel-collector .docker build -f apps/proxy/Dockerfile --target proxy .docker build -f apps/ssh-gateway/Dockerfile --target ssh-gateway .These are the affected Docker images directly built by
apps/infra/sst.config.tsfor SST services.