-
Notifications
You must be signed in to change notification settings - Fork 5
Fix possible by not specifying a USER, a program in the container may run as 'root' in Dockerfile - #8
Conversation
...ay run as 'root'. This is a security hazard. If an
chengzeyi
commented
Aug 30, 2026
@begininvoke Apologies for the silence — this sat for eleven days without a response, which isn't the turnaround a correct, well-cited patch deserves.
The finding is right. images/test_worker/Dockerfile ran its CMD as root, and the fix is the correct shape: create the user, chown the work dir, then switch with USER — in that order, so the copied files stay readable.
I'm closing it only because the target no longer exists. Commit d24d36b ("Remove serverless worker from the SDK (BREAKING)") deleted the whole images/ tree when the serverless worker moved out of this SDK, so the file your patch applies to is gone and GitHub marks the PR conflicting. Nothing to do with the change itself.
If you'd like the same hardening to land somewhere it still applies: WaveSpeedAI/mcp-server has a Dockerfile (node:22-alpine, two-stage) that also never sets USER, so its runtime stage runs as root as well. Same CWE-250, same fix — node:*-alpine images already ship a node user, so there it's a one-line USER node plus ownership on the app dir. That PR would be reviewed promptly.
Thanks for citing the rule and the CWE rather than just dropping a diff — it made this quick to verify.
The runtime stage never dropped privileges, so the MCP server executed as root (CWE-250). The node:*-alpine images already provide an unprivileged node user, so switching needs no useradd — only ownership on the copied dist and a USER line. Verified: the image builds, runs as uid=1000(node), and still completes an MCP initialize handshake over stdio. Reported-by: begininvoke <WaveSpeedAI/wavespeed-python#8> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FmjwZHh3EkQcacCmFhn9ec
chengzeyi
commented
Aug 30, 2026
Correction to what I wrote above: I pointed you at the mcp-server Dockerfile and then went and fixed it myself an hour later, which isn't fair after inviting you to send the patch. Sorry for the whiplash.
The reason I didn't wait: that Dockerfile exists to support our pending docker/mcp-registry submission, which has been sitting in review, and running as root is a plausible thing to be blocked on. I didn't want to leave it open-ended.
Your finding is credited in the commit — WaveSpeedAI/mcp-server@2d76a17, Reported-by: begininvoke. It's the same fix you proposed, adapted to node:*-alpine (that base image already ships an unprivileged node user, so it's --chown=node:node on the copied dist plus USER node, no useradd needed). Verified it builds, runs as uid=1000(node), and still completes an MCP stdio handshake.
Thanks again — two of our images were running as root and you're the reason one of them no longer is.
This changes
images/test_worker/Dockerfileto address something a scan flagged. It is around line 32.The Dockerfile does not set a non‐root USER, so the container's processes (including the CMD) run as root. This unnecessary privilege increases the impact of any compromise of the application – an attacker controlling a process could gain full container control. According to CWE‐250, execution with unnecessary privileges should be avoided.
Added a non‐root USER and created the user to avoid running containers as root.
For reference: rule
dockerfile.security.missing-user.missing-user, CWE-250 (Execution with Unnecessary Privileges). Rated high.I do not know the codebase, so please check the change fits how the rest of it works. Happy to adjust it or close this if the reasoning is off.
Found with automated scanning (RedGem) and reviewed before opening. If it is not useful, closing it is completely fine.