-
Notifications
You must be signed in to change notification settings - Fork 193
feat: make the studio image rootless #2211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe Dockerfile for the studio image was updated to run as a non-root user, add bash, set a working directory, and adjust COPY operations with ownership to node:node. Entrypoint permissions are ensured, and environment variables remain unchanged. The final stage uses USER node with owner-aware artifacts. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks✅ Passed checks (5 passed)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
studio/Dockerfile (2)
52-55: Create and own /app before dropping privileges (defensive hardening).While this often works as-is, explicitly creating/chowning the workdir under root avoids edge cases with directory ownership across layers.
-USER node -WORKDIR /app +RUN mkdir -p /app && chown -R node:node /app +USER node +WORKDIR /app
50-51: Drop --upgrade; keep bash (entrypoint uses bash-only syntax)entrypoint.sh begins with #!/bin/bash and uses bash-specific [[ tests (lines 8, 14, 17, 30) — keep installing bash but remove --upgrade to avoid image bloat/drift.
File: studio/Dockerfile (lines 50-51)
-RUN apk add --no-cache --upgrade bash +RUN apk add --no-cache bash
| FROM --platform=${BUILDPLATFORM} node:lts-alpine | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix platform selection for the final image stage.
Using BUILDPLATFORM for the final stage will produce images for the builder arch in multi-arch builds. Use TARGETPLATFORM or drop the override.
-FROM --platform=${BUILDPLATFORM} node:lts-alpine
+FROM --platform=${TARGETPLATFORM} node:lts-alpine📝 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.
| FROM --platform=${BUILDPLATFORM} node:lts-alpine | |
| FROM --platform=${TARGETPLATFORM} node:lts-alpine |
🤖 Prompt for AI Agents
In studio/Dockerfile around lines 48-49, the final image stage uses
--platform=${BUILDPLATFORM}, which forces the builder architecture into the
produced image; change the platform specifier to --platform=${TARGETPLATFORM} or
remove the --platform override entirely so the final image is built for the
intended target platform in multi-arch builds.
| COPY --from=builder --chown=node:node /app/studio/.next/standalone ./ | ||
| COPY --from=builder --chown=node:node /app/studio/public/ ./studio/public | ||
| COPY --from=builder --chown=node:node /app/studio/.next/static ./studio/.next/static | ||
| COPY --from=builder --chown=node:node /app/studio/entrypoint.sh . | ||
| COPY --from=builder --chown=node:node /app/studio/.env.production . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Ensure .next root (and BUILD_ID) are present and writable for non-root sed operations.
The original failure referenced sed temp files in ./studio/.next/BUILD_IDXXXXXX. You chown static/ but not the file itself if it’s used. Copy it (and ensure parent exists) so entrypoint edits won’t fail.
COPY --from=builder --chown=node:node /app/studio/.next/standalone ./
COPY --from=builder --chown=node:node /app/studio/public/ ./studio/public
COPY --from=builder --chown=node:node /app/studio/.next/static ./studio/.next/static
+COPY --from=builder --chown=node:node /app/studio/.next/BUILD_ID ./studio/.next/BUILD_ID
COPY --from=builder --chown=node:node /app/studio/entrypoint.sh .
COPY --from=builder --chown=node:node /app/studio/.env.production .Quick check:
🏁 Script executed:
#!/bin/bash
# Does entrypoint mutate BUILD_ID or anything under .next?
rg -n 'sed.*\.next/BUILD_ID|\.next/.* -i' studio/entrypoint.shLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "---- studio dir listing ----"
ls -la studio || true
echo
echo "---- studio/Dockerfile (first 240 lines) ----"
if [ -f studio/Dockerfile ]; then
sed -n '1,240p' studio/Dockerfile
else
echo "NO studio/Dockerfile"
fi
echo
echo "---- studio/entrypoint.sh (first 240 lines) ----"
if [ -f studio/entrypoint.sh ]; then
sed -n '1,240p' studio/entrypoint.sh
else
echo "NO studio/entrypoint.sh"
fi
echo
echo "---- Search repo for BUILD_ID references ----"
rg -n --hidden -S 'BUILD_ID|\\.next/BUILD_ID' || true
echo
echo "---- Search repo for sed editing files under .next or using -i ----"
rg -n --hidden -S 'sed.*\\.next|sed\\s+-i' || true
echo
echo "---- Search Dockerfiles for copying .next artifacts ----"
rg -n --hidden -S 'COPY .*\\.next' || trueLength of output: 5091
Copy .next/BUILD_ID into the image and ensure .next is writable
entrypoint.sh runs sed -i across ./studio/.next files; Dockerfile copies .next/static but not .next/BUILD_ID — add BUILD_ID so sed/temp-file creation won't fail at runtime.
COPY --from=builder --chown=node:node /app/studio/.next/standalone ./
COPY --from=builder --chown=node:node /app/studio/public/ ./studio/public
COPY --from=builder --chown=node:node /app/studio/.next/static ./studio/.next/static
+COPY --from=builder --chown=node:node /app/studio/.next/BUILD_ID ./studio/.next/BUILD_ID
COPY --from=builder --chown=node:node /app/studio/entrypoint.sh .
COPY --from=builder --chown=node:node /app/studio/.env.production .📝 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.
| COPY --from=builder --chown=node:node /app/studio/.next/standalone ./ | |
| COPY --from=builder --chown=node:node /app/studio/public/ ./studio/public | |
| COPY --from=builder --chown=node:node /app/studio/.next/static ./studio/.next/static | |
| COPY --from=builder --chown=node:node /app/studio/entrypoint.sh . | |
| COPY --from=builder --chown=node:node /app/studio/.env.production . | |
| COPY --from=builder --chown=node:node /app/studio/.next/standalone ./ | |
| COPY --from=builder --chown=node:node /app/studio/public/ ./studio/public | |
| COPY --from=builder --chown=node:node /app/studio/.next/static ./studio/.next/static | |
| COPY --from=builder --chown=node:node /app/studio/.next/BUILD_ID ./studio/.next/BUILD_ID | |
| COPY --from=builder --chown=node:node /app/studio/entrypoint.sh . | |
| COPY --from=builder --chown=node:node /app/studio/.env.production . |
🤖 Prompt for AI Agents
In studio/Dockerfile around lines 60 to 64, the Docker image currently copies
.next/static but omits .next/BUILD_ID and does not ensure the .next tree is
writable, causing sed -i in entrypoint.sh to fail; add a COPY --from=builder
--chown=node:node /app/studio/.next/BUILD_ID ./studio/.next/BUILD_ID (or copy
the entire .next directory) and ensure the directory permissions allow write by
the node user (e.g., run a chmod -R u+w ./studio/.next or set correct ownership)
so sed/temp-file creation succeeds at runtime.
|
@StarpTech Any chance you could have a look at this one ? Sorry for tagging you directly, just want to avoid the PR getting stale and closed. The ability to run cosmo stack while being rootless is of great importance in heavily locked down production environments. |
|
Hello, any chance this will get merged ? It's blocking us to use Cosmo in our clusters as we can't run containers as root |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
@miklosbarabas are there any updates on this? (Just trying to keep the pr active as otherwise it'll be closed) |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Bump |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
@coderabbitai summary
Checklist
I tested this locally by altering the docker-compose.full.yml to build the studio by copying the build instructions for the studio from the docker-compose.cosmo.yml.
Closes #2206