fix(security): build-arg code injection in Dockerfile - #610
Merged
ericksoa merged 2 commits intoMar 22, 2026
Conversation
…MOCLAW_MODEL) Docker build-args CHAT_UI_URL and NEMOCLAW_MODEL were interpolated directly into a `python3 -c "..."` source string via ARG substitution. A single-quote in the value closes the Python string literal and allows arbitrary code execution at image build time. Fix: promote build-args to ENV, read via os.environ in Python. 11 PoC tests proving the injection, verifying the fix, and guarding against regression by scanning the Dockerfile source. Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
Contributor
📝 WalkthroughWalkthroughDockerfile build args Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Node Test Runner
participant Docker as Docker Build
participant Python as python3 -c (RUN)
participant Env as OS Environment
participant FS as Filesystem
Tester->>Docker: trigger builds / run python variants
Docker->>Python: execute RUN python3 -c (vulnerable or fixed)
Python->>Env: read CHAT_UI_URL / NEMOCLAW_MODEL (fixed) or use interpolated literal (vulnerable)
alt Vulnerable (interpolated)
Python->>FS: execute injected code -> write canary
FS-->>Tester: canary file exists
else Fixed (os.environ)
Python->>FS: write intended config only
FS-->>Tester: no canary
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment Tip You can disable poems in the walkthrough.Disable the |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a Docker image build-time code injection risk where Docker build-args were interpolated directly into a python3 -c source string, and adds regression tests to prevent reintroduction of the pattern.
Changes:
- Promote
NEMOCLAW_MODELandCHAT_UI_URLbuild-args toENVand read them fromos.environin the Dockerfile’s Python snippet. - Add a security regression test suite covering the PoC, fix behavior, and Dockerfile pattern guards.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
Dockerfile |
Stops embedding build-arg values into Python source; reads values from environment variables instead. |
test/security-c2-dockerfile-injection.test.js |
Adds regression tests for the vulnerable pattern, fixed behavior, and Dockerfile guardrails. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+173
to
+177
| // ENV may span multiple lines via backslash continuation | ||
| assert.ok( | ||
| /^ENV\s[\s\S]*?CHAT_UI_URL/m.test(src), | ||
| "Dockerfile must have ENV instruction that promotes CHAT_UI_URL from ARG to env var", | ||
| ); |
There was a problem hiding this comment.
The ENV-promotion regression guard is currently too permissive: /^ENV\s[\s\S]*?CHAT_UI_URL/m can match the earlier ENV DEBIAN_FRONTEND=... line and then any later occurrence of CHAT_UI_URL (e.g., the ARG line or Python snippet). This means the test could pass even if CHAT_UI_URL is not actually set via an ENV ... CHAT_UI_URL=... instruction. Tighten the check to assert there is an ENV instruction line (or continued block) that explicitly assigns CHAT_UI_URL= (and ideally ensure it appears before the RUN python3 -c layer).
Suggested change
| // ENV may span multiple lines via backslash continuation | |
| assert.ok( | |
| /^ENV\s[\s\S]*?CHAT_UI_URL/m.test(src), | |
| "Dockerfile must have ENV instruction that promotes CHAT_UI_URL from ARG to env var", | |
| ); | |
| const lines = src.split("\n"); | |
| // Find the RUN layer that invokes python3 -c | |
| const runIndex = lines.findIndex((line) => | |
| /\bRUN\b.+\bpython3\b\s+-c\b/.test(line), | |
| ); | |
| assert.ok( | |
| runIndex !== -1, | |
| "Dockerfile must have a RUN layer that invokes `python3 -c` for this test to be meaningful", | |
| ); | |
| // Look for an ENV instruction (possibly multi-line via backslash continuation) | |
| // that explicitly assigns CHAT_UI_URL= and appears before the RUN python3 -c layer. | |
| let hasEnvPromotion = false; | |
| for (let i = 0; i < lines.length; i++) { | |
| const line = lines[i]; | |
| if (!/^\s*ENV\b/.test(line)) { | |
| continue; | |
| } | |
| // Collect the ENV block including any continued lines | |
| let blockLines = [line]; | |
| let j = i; | |
| while ( | |
| blockLines[blockLines.length - 1].trimEnd().endsWith("\\") && | |
| j + 1 < lines.length | |
| ) { | |
| j += 1; | |
| blockLines.push(lines[j]); | |
| } | |
| const blockText = blockLines.join("\n"); | |
| // Check for explicit assignment of CHAT_UI_URL= within this ENV block | |
| if (/\bCHAT_UI_URL\s*=/.test(blockText)) { | |
| // Ensure the ENV block starts before the python3 -c RUN layer | |
| if (i < runIndex) { | |
| hasEnvPromotion = true; | |
| break; | |
| } | |
| } | |
| } | |
| assert.ok( | |
| hasEnvPromotion, | |
| "Dockerfile must have an ENV instruction (before the python3 -c RUN layer) that promotes CHAT_UI_URL from ARG to an environment variable", | |
| ); |
Comment on lines
+141
to
+144
| it("Dockerfile does not interpolate CHAT_UI_URL into a Python string literal", () => { | ||
| const src = fs.readFileSync(DOCKERFILE, "utf-8"); | ||
| const vulnerablePattern = /'\$\{CHAT_UI_URL\}'/; | ||
| const lines = src.split("\n"); |
There was a problem hiding this comment.
This regression guard only flags the exact substring '${CHAT_UI_URL}'. The injection risk exists for any Docker ARG interpolation into the Python source (e.g., "${CHAT_UI_URL}", chat_ui_url='${CHAT_UI_URL}' with different spacing, or other Python string forms). Consider broadening the check to detect ${CHAT_UI_URL} usage inside the RUN python3 -c snippet regardless of quote style/spacing, so a trivial refactor can’t silently reintroduce the vulnerability.
Comment on lines
+158
to
+167
| const vulnerablePattern = /'\$\{NEMOCLAW_MODEL\}'/; | ||
| const lines = src.split("\n"); | ||
| for (let i = 0; i < lines.length; i++) { | ||
| if (vulnerablePattern.test(lines[i])) { | ||
| assert.fail( | ||
| `Dockerfile:${i + 1} interpolates NEMOCLAW_MODEL into a Python string literal.\n` + | ||
| ` Line: ${lines[i].trim()}\n` + | ||
| ` Fix: use os.environ['NEMOCLAW_MODEL'] instead.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Same as the CHAT_UI_URL guard: matching only '${NEMOCLAW_MODEL}' can miss equivalent vulnerable patterns that use double quotes or different formatting. If the goal is to prevent ARG interpolation into the Python source, consider scanning the Python RUN ... python3 -c block for ${NEMOCLAW_MODEL} regardless of quote style, rather than only this single-quote form.
Suggested change
| const vulnerablePattern = /'\$\{NEMOCLAW_MODEL\}'/; | |
| const lines = src.split("\n"); | |
| for (let i = 0; i < lines.length; i++) { | |
| if (vulnerablePattern.test(lines[i])) { | |
| assert.fail( | |
| `Dockerfile:${i + 1} interpolates NEMOCLAW_MODEL into a Python string literal.\n` + | |
| ` Line: ${lines[i].trim()}\n` + | |
| ` Fix: use os.environ['NEMOCLAW_MODEL'] instead.`, | |
| ); | |
| } | |
| const vulnerablePattern = /\$\{NEMOCLAW_MODEL\}/; | |
| const lines = src.split("\n"); | |
| let inPythonRunBlock = false; | |
| for (let i = 0; i < lines.length; i++) { | |
| const line = lines[i]; | |
| if (/^\s*RUN\b.*python3\s+-c\b/.test(line)) { | |
| inPythonRunBlock = true; | |
| } | |
| if (inPythonRunBlock && vulnerablePattern.test(line)) { | |
| assert.fail( | |
| `Dockerfile:${i + 1} interpolates NEMOCLAW_MODEL into a Python string literal.\n` + | |
| ` Line: ${line.trim()}\n` + | |
| ` Fix: use os.environ['NEMOCLAW_MODEL'] instead.`, | |
| ); | |
| } | |
| if (inPythonRunBlock && !/\\\s*$/.test(line)) { | |
| inPythonRunBlock = false; | |
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/security-c2-dockerfile-injection.test.js (1)
140-188: Broaden the Dockerfile guard beyond the exact original string form.These checks only catch the exact
'\${...}'form and only proveCHAT_UI_URLappears in someENVsomewhere. A future regression using different quoting, or dropping/movingNEMOCLAW_MODELpromotion before theRUN, could still leave this suite green. Prefer asserting that the config-generationRUNcontains no${CHAT_UI_URL}/${NEMOCLAW_MODEL}at all and that both vars are exported before that layer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/security-c2-dockerfile-injection.test.js` around lines 140 - 188, The tests currently only detect the exact quoted form "'${VAR}'" and only check presence of ENV anywhere; update the tests (the cases under "Dockerfile does not interpolate ..." and the ENV/RUN checks) to (1) search for any interpolation pattern /\$\{CHAT_UI_URL\}/ and /\$\{NEMOCLAW_MODEL\}/ (not just the quoted variant) within the config-generation RUN layer, failing if found, and (2) assert that ENV promotion for both CHAT_UI_URL and NEMOCLAW_MODEL occurs before that RUN (e.g., locate the first RUN that generates config and ensure preceding text contains ENV ... CHAT_UI_URL and ENV ... NEMOCLAW_MODEL); modify the variables/patterns used (replace vulnerablePattern and the per-test src checks) so they scan the RUN body and verify ENV promotions precede it rather than just checking for any ENV occurrence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/security-c2-dockerfile-injection.test.js`:
- Around line 140-188: The tests currently only detect the exact quoted form
"'${VAR}'" and only check presence of ENV anywhere; update the tests (the cases
under "Dockerfile does not interpolate ..." and the ENV/RUN checks) to (1)
search for any interpolation pattern /\$\{CHAT_UI_URL\}/ and
/\$\{NEMOCLAW_MODEL\}/ (not just the quoted variant) within the
config-generation RUN layer, failing if found, and (2) assert that ENV promotion
for both CHAT_UI_URL and NEMOCLAW_MODEL occurs before that RUN (e.g., locate the
first RUN that generates config and ensure preceding text contains ENV ...
CHAT_UI_URL and ENV ... NEMOCLAW_MODEL); modify the variables/patterns used
(replace vulnerablePattern and the per-test src checks) so they scan the RUN
body and verify ENV promotions precede it rather than just checking for any ENV
occurrence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35e068d4-ccef-4e0a-9afc-2e8a2728dd80
📒 Files selected for processing (2)
Dockerfiletest/security-c2-dockerfile-injection.test.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/security-c2-dockerfile-injection.test.js (2)
221-228:os.environpresence check is too global.This assertion passes if any Dockerfile line mentions
os.environ[...](including unrelated commands/comments). Scope it to the specific PythonRUN ... -ccontent being protected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/security-c2-dockerfile-injection.test.js` around lines 221 - 228, The current test reads the whole DOCKERFILE for any os.environ mention which is too broad; change the check to target only the Python one-liner in the Dockerfile RUN that uses python -c (locate the RUN line(s) containing "python -c" from DOCKERFILE variable), extract the quoted command portion, and then assert that that extracted command contains os.environ['CHAT_UI_URL'] or os.environ.get(...) (handle both single and double quotes); update the hasEnvRead logic to perform this scoped check rather than searching the entire file.
187-218: ENV-promotion assertion can false-pass across unrelated stages/keys.
/CHAT_UI_URL[=\s]/can match substrings in other variable names, and the check isn’t scoped to the same build stage as the validatedRUN ... python3 -c.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/security-c2-dockerfile-injection.test.js` around lines 187 - 218, The ENV-promotion check can false-positive and cross stages; tighten it by matching the exact variable name and scoping to the same build stage: change the ENV variable detection to use a word-boundary/anchor like /\bCHAT_UI_URL\b/ or /^CHAT_UI_URL[=\s]/ when scanning ENV continuation lines (referencing the variables chatUiUrlPromoted, inEnvBlock and the ENV regex), and reset chatUiUrlPromoted and inEnvBlock when a new build stage starts by detecting a FROM instruction (e.g. /^\s*FROM\b/) so the assertion for the RUN line only considers ENV entries in the current stage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/security-c2-dockerfile-injection.test.js`:
- Around line 143-144: The current vulnerablePattern only matches the ${VAR}
form and misses unbraced $VAR usages (e.g., $CHAT_UI_URL and $NEMOCLAW_MODEL);
update the regex used in vulnerablePattern (and the analogous pattern around
lines 166-167) to detect both braced and unbraced forms—e.g., change the pattern
to allow an optional brace around the variable name and include both CHAT_UI_URL
and NEMOCLAW_MODEL (use a grouped alternation like (CHAT_UI_URL|NEMOCLAW_MODEL)
with optional surrounding braces) so both $VAR and ${VAR} are caught.
- Around line 148-160: The RUN-block detection is brittle because it only looks
for "RUN ... python3 -c" on a single line; change the logic that uses
inPythonRunBlock and the /^\s*RUN\b.*python3\s+-c\b/ test to instead collect the
full Docker RUN instruction across continuation lines (join lines while previous
line ends with backslash), then run the python3 -c presence check and
vulnerablePattern test against that complete instruction string; update the same
pattern used in the other occurrences (the blocks handling lines ~171-183 and
~207-213) so all checks operate on the assembled RUN instruction rather than
single lines.
---
Nitpick comments:
In `@test/security-c2-dockerfile-injection.test.js`:
- Around line 221-228: The current test reads the whole DOCKERFILE for any
os.environ mention which is too broad; change the check to target only the
Python one-liner in the Dockerfile RUN that uses python -c (locate the RUN
line(s) containing "python -c" from DOCKERFILE variable), extract the quoted
command portion, and then assert that that extracted command contains
os.environ['CHAT_UI_URL'] or os.environ.get(...) (handle both single and double
quotes); update the hasEnvRead logic to perform this scoped check rather than
searching the entire file.
- Around line 187-218: The ENV-promotion check can false-positive and cross
stages; tighten it by matching the exact variable name and scoping to the same
build stage: change the ENV variable detection to use a word-boundary/anchor
like /\bCHAT_UI_URL\b/ or /^CHAT_UI_URL[=\s]/ when scanning ENV continuation
lines (referencing the variables chatUiUrlPromoted, inEnvBlock and the ENV
regex), and reset chatUiUrlPromoted and inEnvBlock when a new build stage starts
by detecting a FROM instruction (e.g. /^\s*FROM\b/) so the assertion for the RUN
line only considers ENV entries in the current stage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 996f2d8c-0c79-438a-b6de-a5f86ac94c10
📒 Files selected for processing (1)
test/security-c2-dockerfile-injection.test.js
Comment on lines
+143
to
+144
| const vulnerablePattern = /\$\{CHAT_UI_URL\}/; | ||
| const lines = src.split("\n"); |
Contributor
There was a problem hiding this comment.
Interpolation guard misses $CHAT_UI_URL / $NEMOCLAW_MODEL forms.
Current regexes only detect ${VAR}. A reintroduced vulnerable Dockerfile using $CHAT_UI_URL (without braces) would bypass this guard.
Suggested hardening
- const vulnerablePattern = /\$\{CHAT_UI_URL\}/;
+ const vulnerablePattern = /\$(?:\{CHAT_UI_URL(?:[:?][^}]*)?\}|CHAT_UI_URL\b)/;
...
- const vulnerablePattern = /\$\{NEMOCLAW_MODEL\}/;
+ const vulnerablePattern = /\$(?:\{NEMOCLAW_MODEL(?:[:?][^}]*)?\}|NEMOCLAW_MODEL\b)/;Also applies to: 166-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/security-c2-dockerfile-injection.test.js` around lines 143 - 144, The
current vulnerablePattern only matches the ${VAR} form and misses unbraced $VAR
usages (e.g., $CHAT_UI_URL and $NEMOCLAW_MODEL); update the regex used in
vulnerablePattern (and the analogous pattern around lines 166-167) to detect
both braced and unbraced forms—e.g., change the pattern to allow an optional
brace around the variable name and include both CHAT_UI_URL and NEMOCLAW_MODEL
(use a grouped alternation like (CHAT_UI_URL|NEMOCLAW_MODEL) with optional
surrounding braces) so both $VAR and ${VAR} are caught.
… guards
- Broaden ARG interpolation checks to detect ${VAR} regardless of quote style
- Parse ENV blocks properly to verify promotion before RUN layer
- Scan python3 -c block line by line for vulnerable patterns
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
gn00295120
force-pushed
the
security/fix-dockerfile-code-injection
branch
2 times, most recently
from
March 22, 2026 01:19
efa1fab to
318409c
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/security-c2-dockerfile-injection.test.js (1)
141-167:⚠️ Potential issue | 🟡 MinorInterpolation guards miss
$CHAT_UI_URL/$NEMOCLAW_MODEL(unbraced) forms.The current regexes at lines 143 and 166 only detect
${VAR}. A Dockerfile using$CHAT_UI_URL(without braces) would bypass this guard.Consider hardening the patterns:
- const vulnerablePattern = /\$\{CHAT_UI_URL\}/; + const vulnerablePattern = /\$(?:\{CHAT_UI_URL\}|CHAT_UI_URL\b)/;- const vulnerablePattern = /\$\{NEMOCLAW_MODEL\}/; + const vulnerablePattern = /\$(?:\{NEMOCLAW_MODEL\}|NEMOCLAW_MODEL\b)/;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/security-c2-dockerfile-injection.test.js` around lines 141 - 167, Update the vulnerablePattern regexes in the two tests ("Dockerfile does not interpolate CHAT_UI_URL into a Python string literal" and "Dockerfile does not interpolate NEMOCLAW_MODEL into a Python string literal") to also match unbraced environment variables (e.g. $CHAT_UI_URL and $NEMOCLAW_MODEL) in addition to the current /\$\{VAR\}/ form; modify the vulnerablePattern definitions (the variables named vulnerablePattern in those tests) to something like a single regex that matches either /\$\{VAR\}/ or /\$VAR\b/ (use the actual variable name in each test) so both braced and unbraced interpolations are caught.
🧹 Nitpick comments (1)
test/security-c2-dockerfile-injection.test.js (1)
187-229: Missing regression guards forNEMOCLAW_MODELENV promotion andos.environusage.The PR objectives state that both
CHAT_UI_URLandNEMOCLAW_MODELare promoted to ENV and read viaos.environ. However, the regression tests at lines 187-229 only verifyCHAT_UI_URL. Consider adding parallel tests forNEMOCLAW_MODELto prevent regression where only one variable is properly secured.Suggested additions
it("Dockerfile promotes NEMOCLAW_MODEL to ENV before the RUN layer", () => { const src = fs.readFileSync(DOCKERFILE, "utf-8"); const lines = src.split("\n"); let nemoclawModelPromoted = false; let inEnvBlock = false; for (let i = 0; i < lines.length; i++) { const line = lines[i]; if (/^\s*ENV\b/.test(line)) { inEnvBlock = true; } if (inEnvBlock && /NEMOCLAW_MODEL[=\s]/.test(line)) { nemoclawModelPromoted = true; } if (inEnvBlock && !/\\\s*$/.test(line)) { inEnvBlock = false; } if (/^\s*RUN\b.*python3\s+-c\b/.test(line)) { assert.ok( nemoclawModelPromoted, `Dockerfile:${i + 1} has a python3 -c RUN layer but NEMOCLAW_MODEL was not promoted via ENV before it`, ); return; } } assert.ok( nemoclawModelPromoted, "Dockerfile must have ENV instruction that promotes NEMOCLAW_MODEL from ARG to env var before the python3 -c RUN layer", ); }); it("Python script uses os.environ to read NEMOCLAW_MODEL", () => { const src = fs.readFileSync(DOCKERFILE, "utf-8"); const hasEnvRead = src.includes("os.environ['NEMOCLAW_MODEL']") || src.includes('os.environ["NEMOCLAW_MODEL"]') || src.includes("os.environ.get('NEMOCLAW_MODEL'") || src.includes('os.environ.get("NEMOCLAW_MODEL"'); assert.ok(hasEnvRead, "Python script must read NEMOCLAW_MODEL via os.environ"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/security-c2-dockerfile-injection.test.js` around lines 187 - 229, The tests currently only validate promotion and usage of CHAT_UI_URL; add two parallel tests that do the same checks for NEMOCLAW_MODEL: duplicate the "Dockerfile promotes CHAT_UI_URL to ENV before the RUN layer" test logic but look for NEMOCLAW_MODEL (track a nemoclawModelPromoted flag, detect ENV blocks, ensure promotion before the python3 -c RUN layer and assert with a clear message), and duplicate the "Python script uses os.environ to read CHAT_UI_URL" test but check for os.environ['NEMOCLAW_MODEL'] / os.environ.get("NEMOCLAW_MODEL") variants and assert that the Python code reads NEMOCLAW_MODEL via os.environ.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/security-c2-dockerfile-injection.test.js`:
- Around line 141-167: Update the vulnerablePattern regexes in the two tests
("Dockerfile does not interpolate CHAT_UI_URL into a Python string literal" and
"Dockerfile does not interpolate NEMOCLAW_MODEL into a Python string literal")
to also match unbraced environment variables (e.g. $CHAT_UI_URL and
$NEMOCLAW_MODEL) in addition to the current /\$\{VAR\}/ form; modify the
vulnerablePattern definitions (the variables named vulnerablePattern in those
tests) to something like a single regex that matches either /\$\{VAR\}/ or
/\$VAR\b/ (use the actual variable name in each test) so both braced and
unbraced interpolations are caught.
---
Nitpick comments:
In `@test/security-c2-dockerfile-injection.test.js`:
- Around line 187-229: The tests currently only validate promotion and usage of
CHAT_UI_URL; add two parallel tests that do the same checks for NEMOCLAW_MODEL:
duplicate the "Dockerfile promotes CHAT_UI_URL to ENV before the RUN layer" test
logic but look for NEMOCLAW_MODEL (track a nemoclawModelPromoted flag, detect
ENV blocks, ensure promotion before the python3 -c RUN layer and assert with a
clear message), and duplicate the "Python script uses os.environ to read
CHAT_UI_URL" test but check for os.environ['NEMOCLAW_MODEL'] /
os.environ.get("NEMOCLAW_MODEL") variants and assert that the Python code reads
NEMOCLAW_MODEL via os.environ.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4fab1b1f-a4ce-440c-b35f-595365c14075
📒 Files selected for processing (2)
Dockerfiletest/security-c2-dockerfile-injection.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile
ericksoa
approved these changes
Mar 22, 2026
Contributor
|
Thanks for the find and the clean fix @gn00295120 — this is a real vulnerability and the We've filed #643 to tighten the regression test guards (unbraced |
ericksoa
added a commit
that referenced
this pull request
Mar 22, 2026
- Broaden regex to catch unbraced $VAR in addition to ${VAR}
- Reset ENV tracking on FROM instructions to scope to build stage
- Scope os.environ check to the python3 -c RUN block only
- Add parallel regression guards for NEMOCLAW_MODEL
Closes #643
2 tasks
kjw3
pushed a commit
that referenced
this pull request
Mar 22, 2026
) - Broaden regex to catch unbraced $VAR in addition to ${VAR} - Reset ENV tracking on FROM instructions to scope to build stage - Scope os.environ check to the python3 -c RUN block only - Add parallel regression guards for NEMOCLAW_MODEL Closes #643
craigamcw
pushed a commit
to craigamcw/NemoClaw
that referenced
this pull request
Mar 22, 2026
* security: fix build-arg code injection in Dockerfile (CHAT_UI_URL, NEMOCLAW_MODEL)
Docker build-args CHAT_UI_URL and NEMOCLAW_MODEL were interpolated
directly into a `python3 -c "..."` source string via ARG substitution.
A single-quote in the value closes the Python string literal and
allows arbitrary code execution at image build time.
Fix: promote build-args to ENV, read via os.environ in Python.
11 PoC tests proving the injection, verifying the fix, and guarding
against regression by scanning the Dockerfile source.
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
* fix: address Copilot review - tighten Dockerfile injection regression guards
- Broaden ARG interpolation checks to detect ${VAR} regardless of quote style
- Parse ENV blocks properly to verify promotion before RUN layer
- Scan python3 -c block line by line for vulnerable patterns
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
---------
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
craigamcw
pushed a commit
to craigamcw/NemoClaw
that referenced
this pull request
Mar 22, 2026
…) (NVIDIA#649) - Broaden regex to catch unbraced $VAR in addition to ${VAR} - Reset ENV tracking on FROM instructions to scope to build stage - Scope os.environ check to the python3 -c RUN block only - Add parallel regression guards for NEMOCLAW_MODEL Closes NVIDIA#643
Ryuketsukami
pushed a commit
to Ryuketsukami/NemoClaw
that referenced
this pull request
Mar 24, 2026
* security: fix build-arg code injection in Dockerfile (CHAT_UI_URL, NEMOCLAW_MODEL)
Docker build-args CHAT_UI_URL and NEMOCLAW_MODEL were interpolated
directly into a `python3 -c "..."` source string via ARG substitution.
A single-quote in the value closes the Python string literal and
allows arbitrary code execution at image build time.
Fix: promote build-args to ENV, read via os.environ in Python.
11 PoC tests proving the injection, verifying the fix, and guarding
against regression by scanning the Dockerfile source.
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
* fix: address Copilot review - tighten Dockerfile injection regression guards
- Broaden ARG interpolation checks to detect ${VAR} regardless of quote style
- Parse ENV blocks properly to verify promotion before RUN layer
- Scan python3 -c block line by line for vulnerable patterns
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
---------
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
Ryuketsukami
pushed a commit
to Ryuketsukami/NemoClaw
that referenced
this pull request
Mar 24, 2026
…) (NVIDIA#649) - Broaden regex to catch unbraced $VAR in addition to ${VAR} - Reset ENV tracking on FROM instructions to scope to build stage - Scope os.environ check to the python3 -c RUN block only - Add parallel regression guards for NEMOCLAW_MODEL Closes NVIDIA#643
jessesanford
pushed a commit
to jessesanford/NemoClaw
that referenced
this pull request
Mar 24, 2026
* security: fix build-arg code injection in Dockerfile (CHAT_UI_URL, NEMOCLAW_MODEL)
Docker build-args CHAT_UI_URL and NEMOCLAW_MODEL were interpolated
directly into a `python3 -c "..."` source string via ARG substitution.
A single-quote in the value closes the Python string literal and
allows arbitrary code execution at image build time.
Fix: promote build-args to ENV, read via os.environ in Python.
11 PoC tests proving the injection, verifying the fix, and guarding
against regression by scanning the Dockerfile source.
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
* fix: address Copilot review - tighten Dockerfile injection regression guards
- Broaden ARG interpolation checks to detect ${VAR} regardless of quote style
- Parse ENV blocks properly to verify promotion before RUN layer
- Scan python3 -c block line by line for vulnerable patterns
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
---------
Signed-off-by: Lucas Wang <lucas_wang@lucas-futures.com>
jessesanford
pushed a commit
to jessesanford/NemoClaw
that referenced
this pull request
Mar 24, 2026
…) (NVIDIA#649) - Broaden regex to catch unbraced $VAR in addition to ${VAR} - Reset ENV tracking on FROM instructions to scope to build stage - Scope os.environ check to the python3 -c RUN block only - Add parallel regression guards for NEMOCLAW_MODEL Closes NVIDIA#643
ColinM-sys
added a commit
to ColinM-sys/NemoClaw
that referenced
this pull request
Apr 7, 2026
…rfile (C-2 followup) The original C-2 fix in NVIDIA#610 protected the python3 -c RUN layer (the SECOND-stage code injection) by reading values via os.environ instead of interpolating them into a Python string literal. It did not protect the FIRST-stage problem: patchStagedDockerfile() in bin/lib/onboard.js rewrites the staged Dockerfile's ARG lines via dockerfile.replace(/^ARG CHAT_UI_URL=.*$/m, `ARG CHAT_UI_URL=${value}`) The regex matches a single line. If the value contains a newline, the replacement string drops a literal newline into the file mid-substitution, splitting one ARG line into two lines — the second of which becomes a brand-new top-level Dockerfile directive (RUN, FROM, COPY, etc.) that 'docker build' will execute as root inside the build container. That is build-time RCE in the docker daemon's child process, triggered by anything that controls the CHAT_UI_URL or NEMOCLAW_MODEL value on the host running 'nemoclaw onboard' — a sourced .bashrc, an attacker wrapper script, a CI/CD pipeline that takes those values from less-trusted input, etc. Neither value is currently validated before substitution. Fix: - bin/lib/onboard.js: introduce assertSafeDockerArgValue(name, value) that rejects any string containing an ASCII control character (\x00-\x1f, \x7f). Call it on every value patchStagedDockerfile() feeds into an ARG-line replacement (CHAT_UI_URL, NEMOCLAW_MODEL, NEMOCLAW_PROVIDER_KEY, NEMOCLAW_PRIMARY_MODEL_REF, NEMOCLAW_INFERENCE_BASE_URL, NEMOCLAW_INFERENCE_API). The check throws hard so the user gets a clear error during onboard rather than a silently-corrupted Dockerfile that builds a poisoned image. The b64 ARG values produced internally by encodeDockerJsonArg() are already base64-clean and not reached from external input, so they don't need the guard. - test/security-c2-dockerfile-injection.test.js: add a new 'C-2 followup' describe block with 9 cases — accept ordinary URLs, reject \n, \r, null bytes, tabs/other ASCII controls, non-string types, a PoC that demonstrates the unguarded replacement injecting a new RUN directive into the file at the string level, and end-to-end checks that patchStagedDockerfile() throws (and does not write the file) on newline-injected CHAT_UI_URL and NEMOCLAW_MODEL values. Verified by stashing the fix and re-running: the 9 new tests fail as expected on main (the unguarded replacement actually does inject a new directive), and pass with the guard in place.
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.
Summary
Docker build-args
CHAT_UI_URLandNEMOCLAW_MODELwere interpolated directly into apython3 -c "..."source string. A single-quote in the value closes the Python string literal and allows arbitrary code execution at image build time.PoC:
CHAT_UI_URL="http://x'; open('/tmp/pwned','w').write('PWNED') #"writes an arbitrary file duringdocker build.Fix
Promote build-args to
ENV, read viaos.environ['CHAT_UI_URL']in Python. Values become data, never source code.Changes
DockerfileENVpromotion; replace'${CHAT_UI_URL}'withos.environ['CHAT_UI_URL']test/security-c2-dockerfile-injection.test.jsTest plan
node --test test/security-c2-dockerfile-injection.test.js)Summary by CodeRabbit
Chores
Tests