fix(security): sandbox repo verification in Docker to prevent RCE (#211) - #343
Open
arpita-1111 wants to merge 1 commit into
Open
fix(security): sandbox repo verification in Docker to prevent RCE (#211)#343arpita-1111 wants to merge 1 commit into
arpita-1111 wants to merge 1 commit into
Conversation
|
🎉 Thank you @arpita-1111 for submitting a Pull Request! We're excited to review your contribution. ✅ Before Review
⚡ Want faster reviews and contributor support? Join our Discord community: 🔗 https://discord.gg/FcXuyw2Rs Maintainers and mentors are active there and can help resolve blockers quickly. Happy Contributing! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
Closes #211
What this PR does
Prevents Remote Code Execution (RCE) during repository verification by running all untrusted build/test/lint commands inside an isolated, disposable Docker container instead of directly on the host.
Previously,
verify_repo()executednpm install,npm ci,npm run <script>, andpytestdirectly on the host machine using scripts defined in the uploaded repository'spackage.json. Since these repositories are untrusted, a maliciouspackage.jsonscript could execute arbitrary commands with full access to the host system, environment variables, and network.Type of change
Stack affected
Changes
Backend
run_cmd_sandboxed()inbackend/app/utils/exec.py, which runs a command inside an ephemeral Docker container instead of on the host.--rm(self-destructs after running)--user 1000:1000(non-root)--memory 512mand--cpus 1(resource limits)--pids-limit 256(prevents fork bombs)--network noneby default (no network access)--cap-drop ALLand--security-opt no-new-privileges(minimal Linux capabilities)/workspacebackend/app/sandbox/verify.pyto userun_cmd_sandboxed()instead ofrun_cmd()for all commands that execute code from the uploaded repository (npm ci,npm install,npm run <script>,pytest).python -c "print('verify: no tests detected')") unsandboxed since it only prints a fixed string and never touches repository content.Proposed solution checklist (from issue)
--rm).Testing
How did you test this?
docker run hello-worldto confirm the Docker engine works.node:20-alpineandpython:3.11-slimbase images used by the sandbox.run_cmdis no longer called for any repository-controlled command.Checklist
console.erroror unhandled Python exceptions introducedrequirements.txt/package.jsonupdated if new dependencies added.pkl,.pt, etc.) are gitignored, not committedAnything reviewers should focus on
Please confirm the resource limits (memory/CPU/pids) are reasonable for real-world verification workloads, and whether the sandbox should be given network access in specific cases (e.g., npm registry access during
npm ci/npm install, which currently would fail with--network none). Happy to adjust based on maintainer guidance.Screenshots (if UI changed)
N/A (backend-only change)