Skip to content

Commit 7534ae3

Browse files
rsliterrsliter
authored
test(sandbox): verify optimized build context sources (#9343)
<!-- markdownlint-disable MD041 --> ## Summary Add a fail-closed repository check that compares every direct local `COPY` source in the root `Dockerfile` with the actual optimized build context. Missing staged inputs now fail before Docker or E2E runs and identify the source and Dockerfile line. ## Related Issue Fixes #9342 ## Changes - Parse handled direct `COPY` forms, reject ambiguous or unsafe forms, and verify exact and wildcard sources in the staged optimized context. - Reuse the parser in the old-base E2E build-context helper and remove the redundant scripts-only integration assertion. The repository check and old-base helper need the same Dockerfile interpretation, and separate parsers previously diverged; focused parser and helper tests protect the shared contract. - Register the check under `npm run checks:repository` and select its focused test when the root `Dockerfile` changes. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Local review of commit `7c390054f` passed all nine repository security categories with no findings. The change affects repository validation and test helpers, not production sandbox behavior. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: The change adds internal repository validation, parser reuse, and focused tests. It does not change a public API, CLI behavior, configuration, supported product behavior, or contributor documentation contract. - Agent: Codex Desktop <!-- docs-review-head-sha: 7c39005 --> <!-- docs-review-agents-blob-sha: 993bdd8 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `cli`: 12 passed; `integration`: 30 passed and 2 skipped; `e2e-support`: 6 passed; test-loop scan passed; standalone optimized-context check passed; CLI type-check passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — `npm run check` passed Repository checks, then stopped on existing all-files Hadolint findings in Dockerfiles this branch does not change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Added validation to detect missing files referenced by Dockerfile `COPY` instructions in optimized build contexts. * Improved handling of Dockerfile syntax, flags, continuations, wildcards, and build-stage sources. * Updated file-change monitoring to run the appropriate validation checks for relevant Dockerfile changes. * **Tests** * Expanded coverage for Dockerfile source validation, unsupported syntax, and watch-trigger behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
1 parent a0d2a37 commit 7534ae3

9 files changed

Lines changed: 402 additions & 133 deletions

‎scripts/checks/optimized-build-context-copy-sources.mts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
/** Verifies direct COPY sources from the root Dockerfile in the optimized build context. */
5+
6+
import fs from "node:fs";
7+
import os from "node:os";
8+
import path from "node:path";
9+
import { fileURLToPath } from "node:url";
10+
11+
import {
12+
formatMissingDockerfileCopySources,
13+
missingDockerfileCopySources,
14+
} from "../lib/dockerfile-copy-sources.mts";
15+
16+
type BuildContextModule = typeof import("../../src/lib/sandbox/build-context.ts");
17+
18+
const importedBuildContext = (await import("../../src/lib/sandbox/build-context.ts")) as
19+
| BuildContextModule
20+
| { default: BuildContextModule };
21+
const buildContextModule =
22+
"default" in importedBuildContext ? importedBuildContext.default : importedBuildContext;
23+
const { stageOptimizedSandboxBuildContext } = buildContextModule;
24+
25+
const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
26+
27+
export function checkOptimizedBuildContextCopySources(
28+
rootDir: string = REPO_ROOT,
29+
temporaryRoot: string = os.tmpdir(),
30+
): void {
31+
const stagingRoot = fs.mkdtempSync(
32+
path.join(temporaryRoot, "nemoclaw-build-context-copy-check-"),
33+
);
34+
try {
35+
const staged = stageOptimizedSandboxBuildContext(rootDir, stagingRoot);
36+
const missingSources = missingDockerfileCopySources(
37+
staged.stagedDockerfile,
38+
staged.buildCtx,
39+
"Dockerfile",
40+
);
41+
if (missingSources.length > 0) {
42+
throw new Error(formatMissingDockerfileCopySources(missingSources));
43+
}
44+
} finally {
45+
fs.rmSync(stagingRoot, { recursive: true, force: true });
46+
}
47+
}
48+
49+
const currentModule = fileURLToPath(import.meta.url);
50+
if (process.argv[1] && path.resolve(process.argv[1]) === currentModule) {
51+
checkOptimizedBuildContextCopySources();
52+
}

‎scripts/checks/run.mts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ export const CHECKS: readonly CheckCommand[] = [
113113
command: TSX,
114114
args: ["scripts/checks/no-unit-blocks-in-live-e2e.mts"],
115115
},
116+
{
117+
name: "optimized-build-context-copy-sources",
118+
command: TSX,
119+
args: ["scripts/checks/optimized-build-context-copy-sources.mts"],
120+
},
116121
{
117122
name: "test-registration-boundary",
118123
command: TSX,

‎scripts/lib/dockerfile-copy-sources.mts‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
/** Parses direct Dockerfile COPY sources and rejects unhandled forms. */
5+
6+
import fs from "node:fs";
7+
import path from "node:path";
8+
9+
interface LogicalDockerfileInstruction {
10+
lineNumber: number;
11+
text: string;
12+
}
13+
14+
export interface DirectDockerfileCopySource {
15+
lineNumber: number;
16+
source: string;
17+
}
18+
19+
export interface MissingDockerfileCopySource extends DirectDockerfileCopySource {
20+
dockerfileLabel: string;
21+
}
22+
23+
function logicalDockerfileInstructions(
24+
text: string,
25+
dockerfileLabel: string,
26+
): LogicalDockerfileInstruction[] {
27+
const instructions: LogicalDockerfileInstruction[] = [];
28+
let currentParts: string[] = [];
29+
let currentLineNumber = 0;
30+
let sawInstruction = false;
31+
32+
for (const [lineIndex, rawLine] of text.split(/\r?\n/u).entries()) {
33+
const line = rawLine.trim();
34+
if (!line || line.startsWith("#")) {
35+
if (!sawInstruction) {
36+
const escapeDirective = /^#\s*escape\s*=\s*(\S+)\s*$/iu.exec(line);
37+
if (escapeDirective && escapeDirective[1] !== "\\") {
38+
throw new Error(
39+
`Unsupported ${dockerfileLabel} escape directive at line ${lineIndex + 1}: ${rawLine}`,
40+
);
41+
}
42+
}
43+
continue;
44+
}
45+
46+
sawInstruction = true;
47+
if (currentParts.length === 0) currentLineNumber = lineIndex + 1;
48+
49+
const continued = line.endsWith("\\");
50+
const part = continued ? line.slice(0, -1).trimEnd() : line;
51+
if (!part) {
52+
throw new Error(
53+
`Unsupported ${dockerfileLabel} continuation at line ${lineIndex + 1}: ${rawLine}`,
54+
);
55+
}
56+
currentParts.push(part);
57+
58+
if (!continued) {
59+
const instruction = currentParts.join(" ");
60+
if (instruction.split(/\s+/u).some((token) => token.startsWith("<<"))) {
61+
throw new Error(
62+
`Unsupported ${dockerfileLabel} heredoc instruction at line ${currentLineNumber}: ${instruction}`,
63+
);
64+
}
65+
instructions.push({ lineNumber: currentLineNumber, text: instruction });
66+
currentParts = [];
67+
currentLineNumber = 0;
68+
}
69+
}
70+
71+
if (currentParts.length > 0) {
72+
throw new Error(`Dangling ${dockerfileLabel} continuation at line ${currentLineNumber}`);
73+
}
74+
return instructions;
75+
}
76+
77+
function unsupportedDirectCopyForm(
78+
instruction: LogicalDockerfileInstruction,
79+
dockerfileLabel: string,
80+
): never {
81+
throw new Error(
82+
`Unsupported direct ${dockerfileLabel} COPY form at line ${instruction.lineNumber}: ${instruction.text}`,
83+
);
84+
}
85+
86+
function parseDirectCopyOperands(
87+
tokens: string[],
88+
instruction: LogicalDockerfileInstruction,
89+
dockerfileLabel: string,
90+
): string[] | null {
91+
let operandIndex = 0;
92+
let copiesFromStage = false;
93+
while (operandIndex < tokens.length && tokens[operandIndex]?.startsWith("--")) {
94+
const flag = tokens[operandIndex]!;
95+
if (/^--from=.+$/iu.test(flag)) {
96+
copiesFromStage = true;
97+
operandIndex += 1;
98+
continue;
99+
}
100+
const handledDirectFlag =
101+
/^--(?:chown|chmod)=.+$/iu.test(flag) ||
102+
/^--(?:link|parents)(?:=(?:true|false))?$/iu.test(flag);
103+
if (!handledDirectFlag) unsupportedDirectCopyForm(instruction, dockerfileLabel);
104+
operandIndex += 1;
105+
}
106+
107+
const operands = tokens.slice(operandIndex);
108+
if (operands[0]?.startsWith("[")) {
109+
unsupportedDirectCopyForm(instruction, dockerfileLabel);
110+
}
111+
if (operands.length < 2 || operands.some((operand) => operand.startsWith("--"))) {
112+
unsupportedDirectCopyForm(instruction, dockerfileLabel);
113+
}
114+
return copiesFromStage ? null : operands;
115+
}
116+
117+
function validateDirectCopySource(
118+
source: string,
119+
instruction: LogicalDockerfileInstruction,
120+
dockerfileLabel: string,
121+
): void {
122+
const withoutTrailingSlash = source.replace(/\/+$/u, "");
123+
const parts = withoutTrailingSlash.split("/");
124+
const invalidSource =
125+
!withoutTrailingSlash ||
126+
path.posix.isAbsolute(source) ||
127+
source.includes("\\") ||
128+
/[$"'{}]/u.test(source) ||
129+
source.includes("**") ||
130+
source.startsWith("--") ||
131+
parts.some((part) => !part || part === "." || part === "..");
132+
if (invalidSource) {
133+
throw new Error(
134+
`Unsupported direct ${dockerfileLabel} COPY source at line ${instruction.lineNumber}: ${source}`,
135+
);
136+
}
137+
}
138+
139+
export function directDockerfileCopySources(
140+
dockerfilePath: string,
141+
dockerfileLabel = path.basename(dockerfilePath),
142+
): DirectDockerfileCopySource[] {
143+
const text = fs.readFileSync(dockerfilePath, "utf8");
144+
const sources: DirectDockerfileCopySource[] = [];
145+
146+
for (const instruction of logicalDockerfileInstructions(text, dockerfileLabel)) {
147+
const instructionMatch = /^(\S+)\b([\s\S]*)$/u.exec(instruction.text);
148+
if (!instructionMatch || instructionMatch[1].toUpperCase() !== "COPY") continue;
149+
150+
const copyForm = instructionMatch[2].trim();
151+
const tokens = copyForm.split(/\s+/u).filter(Boolean);
152+
if (!copyForm) {
153+
unsupportedDirectCopyForm(instruction, dockerfileLabel);
154+
}
155+
156+
const operands = parseDirectCopyOperands(tokens, instruction, dockerfileLabel);
157+
if (operands === null) continue;
158+
159+
for (const source of operands.slice(0, -1)) {
160+
validateDirectCopySource(source, instruction, dockerfileLabel);
161+
sources.push({ lineNumber: instruction.lineNumber, source });
162+
}
163+
}
164+
165+
return sources;
166+
}
167+
168+
function sourceExistsInContext(contextRoot: string, source: string): boolean {
169+
if (/[*?[\]]/u.test(source)) {
170+
return fs.globSync(source, { cwd: contextRoot }).length > 0;
171+
}
172+
return fs.existsSync(path.join(contextRoot, ...source.split("/")));
173+
}
174+
175+
export function missingDockerfileCopySources(
176+
dockerfilePath: string,
177+
contextRoot: string,
178+
dockerfileLabel = path.basename(dockerfilePath),
179+
): MissingDockerfileCopySource[] {
180+
return directDockerfileCopySources(dockerfilePath, dockerfileLabel)
181+
.filter(({ source }) => !sourceExistsInContext(contextRoot, source))
182+
.map((source) => ({ ...source, dockerfileLabel }));
183+
}
184+
185+
export function formatMissingDockerfileCopySources(
186+
missingSources: readonly MissingDockerfileCopySource[],
187+
): string {
188+
return [
189+
"The optimized build context does not contain every direct Dockerfile COPY source.",
190+
"Review each missing source before you add it to stageOptimizedSandboxBuildContext.",
191+
"",
192+
...missingSources.map(
193+
({ dockerfileLabel, lineNumber, source }) =>
194+
`${dockerfileLabel}:${lineNumber} missing ${source}`,
195+
),
196+
].join("\n");
197+
}

‎src/lib/sandbox/optimized-build-context-copy-sources.test.ts‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import fs from "node:fs";
5+
import os from "node:os";
6+
import path from "node:path";
7+
import { afterEach, describe, expect, it } from "vitest";
8+
9+
import { testTimeout } from "../../../test/helpers/timeouts";
10+
import { checkOptimizedBuildContextCopySources } from "../../../scripts/checks/optimized-build-context-copy-sources.mts";
11+
import {
12+
directDockerfileCopySources,
13+
formatMissingDockerfileCopySources,
14+
missingDockerfileCopySources,
15+
} from "../../../scripts/lib/dockerfile-copy-sources.mts";
16+
17+
const temporaryDirectories: string[] = [];
18+
19+
function makeTemporaryDirectory(): string {
20+
const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-copy-sources-test-"));
21+
temporaryDirectories.push(directory);
22+
return directory;
23+
}
24+
25+
function writeDockerfile(directory: string, source: string): string {
26+
const dockerfilePath = path.join(directory, "Dockerfile");
27+
fs.writeFileSync(dockerfilePath, source, "utf8");
28+
return dockerfilePath;
29+
}
30+
31+
describe("optimized build-context Dockerfile sources", () => {
32+
afterEach(() => {
33+
for (const directory of temporaryDirectories.splice(0)) {
34+
fs.rmSync(directory, { recursive: true, force: true });
35+
}
36+
});
37+
38+
it(
39+
"accepts every direct COPY source from the root Dockerfile in the optimized context",
40+
() => {
41+
checkOptimizedBuildContextCopySources(path.resolve(import.meta.dirname, "../../.."));
42+
},
43+
testTimeout(30_000),
44+
);
45+
46+
it("parses flags, continuations, and multiple sources while ignoring build stages", () => {
47+
const directory = makeTemporaryDirectory();
48+
const dockerfilePath = writeDockerfile(
49+
directory,
50+
[
51+
"FROM base AS build",
52+
"COPY --chmod=0444 \\",
53+
" scripts/lib/corporate-ca-runtime.sh \\",
54+
" scripts/lib/sandbox-init.sh \\",
55+
" /usr/local/lib/nemoclaw/",
56+
"COPY --from=build /out/runtime /usr/local/lib/runtime",
57+
].join("\n"),
58+
);
59+
60+
expect(directDockerfileCopySources(dockerfilePath)).toEqual([
61+
{ lineNumber: 2, source: "scripts/lib/corporate-ca-runtime.sh" },
62+
{ lineNumber: 2, source: "scripts/lib/sandbox-init.sh" },
63+
]);
64+
});
65+
66+
it("reports an absent flagged COPY source with its Dockerfile line", () => {
67+
const directory = makeTemporaryDirectory();
68+
const dockerfilePath = writeDockerfile(
69+
directory,
70+
[
71+
"FROM base",
72+
"COPY --chmod=0444 scripts/lib/corporate-ca-runtime.sh /usr/local/lib/nemoclaw/",
73+
].join("\n"),
74+
);
75+
76+
const missing = missingDockerfileCopySources(dockerfilePath, directory);
77+
78+
expect(missing).toEqual([
79+
{
80+
dockerfileLabel: "Dockerfile",
81+
lineNumber: 2,
82+
source: "scripts/lib/corporate-ca-runtime.sh",
83+
},
84+
]);
85+
expect(formatMissingDockerfileCopySources(missing)).toContain(
86+
"Dockerfile:2 missing scripts/lib/corporate-ca-runtime.sh",
87+
);
88+
});
89+
90+
it("requires a wildcard source to match at least one staged path", () => {
91+
const directory = makeTemporaryDirectory();
92+
const dockerfilePath = writeDockerfile(
93+
directory,
94+
"COPY nemoclaw-blueprint/scripts/*.js /usr/local/lib/nemoclaw/preloads/\n",
95+
);
96+
const scriptsDirectory = path.join(directory, "nemoclaw-blueprint", "scripts");
97+
fs.mkdirSync(scriptsDirectory, { recursive: true });
98+
99+
expect(missingDockerfileCopySources(dockerfilePath, directory)).toHaveLength(1);
100+
101+
fs.writeFileSync(path.join(scriptsDirectory, "http-proxy-fix.js"), "fixture\n", "utf8");
102+
expect(missingDockerfileCopySources(dockerfilePath, directory)).toEqual([]);
103+
});
104+
105+
it.each([
106+
["JSON array", 'COPY ["scripts/lib/sandbox-init.sh", "/tmp/"]'],
107+
["build-stage JSON array", 'COPY --from=build ["out", "/target"]'],
108+
["heredoc", "COPY <<EOF /tmp/generated"],
109+
["unhandled direct flag", "COPY --exclude=*.md scripts/lib/sandbox-init.sh /tmp/"],
110+
["unhandled build-stage flag", "COPY --from=build --exclude=*.md /out/runtime /tmp/runtime"],
111+
["variable source", "COPY scripts/$SOURCE /tmp/source"],
112+
["absolute source", "COPY /etc/passwd /tmp/passwd"],
113+
["parent traversal", "COPY ../outside /tmp/outside"],
114+
])("rejects the %s COPY form instead of omitting its sources", (_form, dockerfile) => {
115+
const directory = makeTemporaryDirectory();
116+
const dockerfilePath = writeDockerfile(directory, dockerfile);
117+
118+
expect(() => directDockerfileCopySources(dockerfilePath), dockerfile).toThrow(
119+
/Unsupported (?:direct )?Dockerfile (?:COPY (?:form|source)|heredoc instruction)/u,
120+
);
121+
});
122+
});

0 commit comments

Comments
 (0)