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
13 changes: 13 additions & 0 deletions src/cli/operations/deploy/__tests__/preflight-container.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,19 @@ describe('validateContainerAgents', () => {
expect(ensureMock).not.toHaveBeenCalled();
});

it('does NOT ensure .dockerignore when buildContextPath is set but the Dockerfile is missing (ordering guard)', () => {
// Dockerfile missing — ensure must be SKIPPED (runs only after validation), so a failing deploy
// leaves no stray .dockerignore and the friendly "Dockerfile not found" error is not masked.
mockedExistsSync.mockReturnValue(false);

const spec = makeSpec([
{ name: 'mono', build: 'Container', codeLocation: dir('agents/mono'), buildContextPath: dir('.') },
]);

expect(() => validateContainerAgents(spec, CONFIG_ROOT)).toThrow(/Dockerfile not found/);
expect(ensureMock).not.toHaveBeenCalled();
});

it('warns when Dockerfile uses deprecated bookworm base image', () => {
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(
Expand Down
33 changes: 17 additions & 16 deletions src/cli/operations/deploy/preflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,22 +212,23 @@ export function validateContainerAgents(projectSpec: AgentCoreProjectSpec, confi
errors.push(
`Agent "${agent.name}": ${agent.dockerfile ?? DOCKERFILE_NAME} not found at ${dockerfilePath}. Container agents require a Dockerfile.`
);
}
warnDeprecatedBaseImage(dockerfilePath, agent.name);

// Dockerfile validated. buildContextPath widens the docker build context (e.g. the whole repo);
// ensure a .dockerignore at that root so secrets/junk (.env, .git, agentcore/) are never baked
// into the local image. Both local and deploy honor this file; it is created only when absent
// (never overwrites the user's) and only AFTER validation — so a failing deploy leaves no stray
// file and this never masks the friendly "Dockerfile not found" error above.
if (agent.buildContextPath) {
const created = ensureBuildContextDockerignore(buildContext);
if (created) {
console.warn(
`Agent "${agent.name}": created ${created} with default secret exclusions because buildContextPath is ` +
`set (the entire context is sent to Docker/CodeBuild). Review and commit it; edit to include any ` +
`intentionally-bundled files.`
);
} else {
warnDeprecatedBaseImage(dockerfilePath, agent.name);

// Dockerfile validated. buildContextPath widens the docker build context (e.g. the whole repo);
// ensure a .dockerignore at that root so secrets/junk (.env, .git, agentcore/) are never baked
// into the local image. Both local and deploy honor this file; it is created only when absent
// (never overwrites the user's) and only AFTER validation — so a failing deploy leaves no stray
// file and this never masks the friendly "Dockerfile not found" error above.
if (agent.buildContextPath) {
const created = ensureBuildContextDockerignore(buildContext);
if (created) {
console.warn(
`Agent "${agent.name}": created ${created} with default secret exclusions because buildContextPath is ` +
`set (the entire context is sent to Docker/CodeBuild). Review and commit it; edit to include any ` +
`intentionally-bundled files.`
);
}
}
}
}
Expand Down
15 changes: 15 additions & 0 deletions src/schema/schemas/__tests__/agent-env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,14 @@ describe('AgentEnvSpecSchema - dockerfile', () => {
true
);
});

it('rejects "." (names the build-context directory itself → broken `docker build -f .`)', () => {
expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: '.' }).success).toBe(false);
});

it('accepts a "./"-prefixed filename (normalizes to the file)', () => {
expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: './Dockerfile' }).success).toBe(true);
});
});

describe('AgentEnvSpecSchema - buildContextPath', () => {
Expand Down Expand Up @@ -663,6 +671,13 @@ describe('AgentEnvSpecSchema - customDockerBuildArgs', () => {
).toBe(false);
});

it('rejects a build-arg key over 255 characters (env-var-name length limit via EnvVarNameSchema)', () => {
const longKey = 'A' + 'B'.repeat(255); // 256 chars
expect(
AgentEnvSpecSchema.safeParse({ ...validContainerAgent, customDockerBuildArgs: { [longKey]: 'v' } }).success
).toBe(false);
});

it.each([
'IMAGE_URI',
'ECR_REGISTRY',
Expand Down
9 changes: 7 additions & 2 deletions src/schema/schemas/agent-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ const DOCKERFILE_PATH_ALLOWED_CHARS = /^[A-Za-z0-9._/-]+$/;

/**
* True when `p` is a safe relative Dockerfile path within the build context: allowed chars only, no
* leading slash (absolute), and no empty ('' from a leading/trailing/double slash) or '..' segments.
* leading slash (absolute), no empty ('' from a leading/trailing/double slash) or '..' segments, and
* it must name a file — not the build-context directory itself (a value like '.' or './' resolves to
* the context dir, producing a broken `docker build -f <dir>`).
*
* The single source of truth shared by `DockerfilePathSchema` (config-validation time) and the runtime
* `getDockerfilePath` guard, so the two can never diverge — e.g. one accepting '.docker/Dockerfile'
Expand All @@ -110,7 +112,10 @@ const DOCKERFILE_PATH_ALLOWED_CHARS = /^[A-Za-z0-9._/-]+$/;
export function isValidDockerfilePath(p: string): boolean {
if (!DOCKERFILE_PATH_ALLOWED_CHARS.test(p)) return false;
if (p.startsWith('/')) return false;
return !p.split('/').some(segment => segment === '' || segment === '..');
const segments = p.split('/');
if (segments.some(segment => segment === '' || segment === '..')) return false;
// Must resolve to a file inside the context, not the context dir: reject '.', './', '././', etc.
return segments.some(segment => segment !== '.');
}

/**
Expand Down
Loading