diff --git a/docs/configuration.md b/docs/configuration.md index bac57bc00..7f81358e6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -177,24 +177,26 @@ on the next deployment. } ``` -| Field | Required | Description | -| ------------------------- | -------- | --------------------------------------------------------------- | -| `name` | Yes | Agent name (1-48 chars, alphanumeric + underscore) | -| `build` | Yes | `"CodeZip"` or `"Container"` | -| `entrypoint` | Yes | Entry file (e.g., `main.py` or `main.py:handler`) | -| `codeLocation` | Yes | Directory containing agent code | -| `runtimeVersion` | Yes | Runtime version (see below) | -| `networkMode` | No | `"PUBLIC"` (default) or `"VPC"` | -| `networkConfig` | No | VPC configuration (subnets, security groups) | -| `protocol` | No | `"HTTP"` (default), `"MCP"`, or `"A2A"` | -| `envVars` | No | Custom environment variables | -| `instrumentation` | No | OpenTelemetry settings | -| `authorizerType` | No | `"AWS_IAM"` or `"CUSTOM_JWT"` | -| `authorizerConfiguration` | No | JWT authorizer settings (for `CUSTOM_JWT`) | -| `requestHeaderAllowlist` | No | Headers to forward to the agent | -| `lifecycleConfiguration` | No | Runtime session lifecycle settings (idle timeout, max lifetime) | -| `executionRoleArn` | No | ARN of an existing IAM execution role (skips CDK-managed role) | -| `tags` | No | Agent-level tags | +| Field | Required | Description | +| ------------------------- | -------- | ---------------------------------------------------------------------------------------- | +| `name` | Yes | Agent name (1-48 chars, alphanumeric + underscore) | +| `build` | Yes | `"CodeZip"` or `"Container"` | +| `entrypoint` | Yes | Entry file (e.g., `main.py` or `main.py:handler`) | +| `codeLocation` | Yes | Directory containing agent code (and the `Dockerfile` for Container builds) | +| `runtimeVersion` | Yes | Runtime version (see below) | +| `networkMode` | No | `"PUBLIC"` (default) or `"VPC"` | +| `networkConfig` | No | VPC configuration (subnets, security groups) | +| `protocol` | No | `"HTTP"` (default), `"MCP"`, or `"A2A"` | +| `envVars` | No | Custom environment variables | +| `instrumentation` | No | OpenTelemetry settings | +| `authorizerType` | No | `"AWS_IAM"` or `"CUSTOM_JWT"` | +| `authorizerConfiguration` | No | JWT authorizer settings (for `CUSTOM_JWT`) | +| `requestHeaderAllowlist` | No | Headers to forward to the agent | +| `lifecycleConfiguration` | No | Runtime session lifecycle settings (idle timeout, max lifetime) | +| `executionRoleArn` | No | ARN of an existing IAM execution role (skips CDK-managed role) | +| `tags` | No | Agent-level tags | +| `buildContextPath` | No | **Container only.** Docker build context directory (replaces `codeLocation` as context). | +| `customDockerBuildArgs` | No | **Container only.** Key/value pairs forwarded as `--build-arg` flags. | ### Runtime Versions diff --git a/docs/container-builds.md b/docs/container-builds.md index 1f34ea64e..d785adac3 100644 --- a/docs/container-builds.md +++ b/docs/container-builds.md @@ -91,6 +91,73 @@ All other fields work the same as CodeZip agents. > must also add a `Dockerfile` and `.dockerignore` to the agent's code directory. The easiest way is to create a > throwaway container agent with `agentcore add agent --build Container` and copy the generated files. +### Advanced: Shared Dockerfile (monorepo) + +When multiple agents share the same build logic, you can point them all at a single `Dockerfile` using two optional +fields: + +| Field | Description | +| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------ | +| `buildContextPath` | Docker build context directory. Replaces `codeLocation` as the `docker build` context and as the root the `dockerfile` path is resolved against. | +| `customDockerBuildArgs` | Key/value pairs forwarded as `--build-arg` flags, allowing a shared Dockerfile to branch per agent. Keys must be valid identifiers. | + +Both fields are honored identically by local `agentcore dev` / `agentcore package` and by `agentcore deploy` (which +builds in CodeBuild). The `dockerfile` field is resolved relative to the build context, so with `buildContextPath: "."` +the default `Dockerfile` refers to `./Dockerfile` at the project root. `dockerfile` may also be a relative subpath (e.g. +`docker/Dockerfile` or `.docker/Dockerfile`); absolute paths, `..` traversal, and directory-shaped values (a trailing +slash) are rejected. + +**Example — two agents, one shared `Dockerfile` at the project root:** + +```json +{ + "name": "agent-one", + "build": "Container", + "entrypoint": "main.py", + "codeLocation": "app/agent-one/", + "buildContextPath": ".", + "customDockerBuildArgs": { "AGENT_NAME": "agent-one" } +}, +{ + "name": "agent-two", + "build": "Container", + "entrypoint": "main.py", + "codeLocation": "app/agent-two/", + "buildContextPath": ".", + "customDockerBuildArgs": { "AGENT_NAME": "agent-two" } +} +``` + +The shared `Dockerfile` can then branch on the build arg: + +```dockerfile +ARG AGENT_NAME +COPY app/${AGENT_NAME}/ ./app/ +``` + +**When to use `buildContextPath`:** use it when your `Dockerfile` needs to `COPY` files that live outside of +`codeLocation` (e.g. shared libraries at the project root). Without it, Docker only sees the `codeLocation` directory as +its build context. + +> **Secrets and build junk are excluded from the upload — always.** On `agentcore deploy`, the build context uploaded to +> CodeBuild (whether it's `codeLocation` or a `buildContextPath`) always has a baseline of secret/junk patterns excluded +> — `.env`, `.env.*`, `.git`, `.venv`, `node_modules`, `__pycache__`, `.pytest_cache`, `.DS_Store` — at every depth +> (nested `app/agent-one/.env` and `app/agent-one/node_modules` are excluded too, not just the top level). So a stray +> `.env` is never persisted to the assets bucket or baked into the deployed image, regardless of whether you have a +> `.dockerignore`. Your own `.dockerignore` at the build-context root is honored on top of this baseline (exclude more, +> or re-include a baseline path with a `!pattern` line), and the `Dockerfile` itself is always kept even under an +> allowlist-style (`*`) `.dockerignore`. +> +> **A `.dockerignore` is also generated for local builds when you set `buildContextPath`.** The whole of that directory +> is the build context — for `buildContextPath: "."` that's your entire project — and a local `docker build` honors only +> a real `.dockerignore`. So if none exists at that root, `agentcore dev` / `agentcore package` creates one (the same +> baseline above, listed both bare and `**/`-prefixed, plus `agentcore/`) so your local image matches the deploy upload. +> It's an ordinary file — commit it, and edit it to include anything you intentionally want in the context (e.g. a +> non-secret `.env.production`, by adding a `!.env.production` line). + +**When to use `customDockerBuildArgs`:** use it to parameterise a shared `Dockerfile` so each agent produces a different +image (different entry point, bundled code, etc.) without duplicating the file. + ## Local Development ```bash diff --git a/src/cli/operations/deploy/__tests__/preflight-container.test.ts b/src/cli/operations/deploy/__tests__/preflight-container.test.ts index ff5f5c3c9..3b203816e 100644 --- a/src/cli/operations/deploy/__tests__/preflight-container.test.ts +++ b/src/cli/operations/deploy/__tests__/preflight-container.test.ts @@ -3,28 +3,38 @@ import { validateContainerAgents } from '../preflight.js'; import { existsSync, readFileSync } from 'node:fs'; import { afterEach, describe, expect, it, vi } from 'vitest'; +const { ensureMock } = vi.hoisted(() => ({ ensureMock: vi.fn() })); + vi.mock('node:fs', () => ({ existsSync: vi.fn(), readFileSync: vi.fn(), })); -vi.mock('../../../../lib', () => ({ - DOCKERFILE_NAME: 'Dockerfile', - getDockerfilePath: (codeLocation: string, dockerfile?: string) => { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const p = require('node:path') as typeof import('node:path'); - return p.join(codeLocation, dockerfile ?? 'Dockerfile'); - }, - resolveCodeLocation: vi.fn((codeLocation: string, configBaseDir: string) => { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const p = require('node:path') as typeof import('node:path'); - const repoRoot = p.dirname(configBaseDir); - return p.resolve(repoRoot, codeLocation); - }), - // Stub other exports that the module may pull in - ConfigIO: vi.fn(), - requireConfigRoot: vi.fn(), -})); +vi.mock('../../../../lib', () => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const p = require('node:path') as typeof import('node:path'); + const resolveCodeLocation = (codeLocation: string, configBaseDir: string) => + p.resolve(p.dirname(configBaseDir), codeLocation); + const getDockerfilePath = (buildContext: string, dockerfile?: string) => + p.join(buildContext, dockerfile ?? 'Dockerfile'); + return { + DOCKERFILE_NAME: 'Dockerfile', + getDockerfilePath, + resolveCodeLocation, + // Mirror the real shared resolver so existsSync/ensure are called with the same paths. + resolveBuildContext: ( + spec: { codeLocation: string; buildContextPath?: string; dockerfile?: string }, + baseDir: string + ) => { + const buildContext = resolveCodeLocation(spec.buildContextPath ?? spec.codeLocation, baseDir); + return { buildContext, dockerfilePath: getDockerfilePath(buildContext, spec.dockerfile) }; + }, + ensureBuildContextDockerignore: ensureMock, + // Stub other exports that the module may pull in + ConfigIO: vi.fn(), + requireConfigRoot: vi.fn(), + }; +}); const mockedExistsSync = vi.mocked(existsSync); const mockedReadFileSync = vi.mocked(readFileSync); @@ -135,6 +145,49 @@ describe('validateContainerAgents', () => { expect(() => validateContainerAgents(spec, CONFIG_ROOT)).toThrow(/Dockerfile\.gpu not found/); }); + it('resolves the Dockerfile against buildContextPath when set', () => { + mockedExistsSync.mockReturnValue(true); + mockValidDockerfile(); + + const spec = makeSpec([ + { name: 'mono', build: 'Container', codeLocation: dir('agents/mono'), buildContextPath: dir('.') }, + ]); + + expect(() => validateContainerAgents(spec, CONFIG_ROOT)).not.toThrow(); + // The Dockerfile is resolved from the build context (repo root), not the agent's codeLocation. + const calledPath = mockedExistsSync.mock.calls[0]?.[0] as string; + expect(calledPath).not.toContain('agents/mono'); + }); + + it('generates a build-context .dockerignore (and logs) when buildContextPath is set and none exists', () => { + mockedExistsSync.mockReturnValue(true); + mockValidDockerfile(); + // Simulate "created" — return the path so the preflight logs it. + ensureMock.mockReturnValue('/project/.dockerignore'); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const spec = makeSpec([ + { name: 'mono', build: 'Container', codeLocation: dir('agents/mono'), buildContextPath: dir('.') }, + ]); + + expect(() => validateContainerAgents(spec, CONFIG_ROOT)).not.toThrow(); + // Ensured against the resolved build context (repo root), not the agent's codeLocation. + const ensuredPath = ensureMock.mock.calls[0]?.[0] as string; + expect(ensuredPath).not.toContain('agents/mono'); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('created')); + warnSpy.mockRestore(); + }); + + it('does not touch .dockerignore when buildContextPath is unset', () => { + mockedExistsSync.mockReturnValue(true); + mockValidDockerfile(); + + const spec = makeSpec([{ name: 'plain', build: 'Container', codeLocation: dir('agents/plain') }]); + + validateContainerAgents(spec, CONFIG_ROOT); + expect(ensureMock).not.toHaveBeenCalled(); + }); + it('warns when Dockerfile uses deprecated bookworm base image', () => { mockedExistsSync.mockReturnValue(true); mockedReadFileSync.mockReturnValue( diff --git a/src/cli/operations/deploy/preflight.ts b/src/cli/operations/deploy/preflight.ts index bf307b87f..ca7281c46 100644 --- a/src/cli/operations/deploy/preflight.ts +++ b/src/cli/operations/deploy/preflight.ts @@ -1,4 +1,10 @@ -import { ConfigIO, DOCKERFILE_NAME, getDockerfilePath, requireConfigRoot, resolveCodeLocation } from '../../../lib'; +import { + ConfigIO, + DOCKERFILE_NAME, + ensureBuildContextDockerignore, + requireConfigRoot, + resolveBuildContext, +} from '../../../lib'; import { StaleCdkConstructError, ValidationError } from '../../../lib/errors/types'; import type { AgentCoreProjectSpec, AwsDeploymentTarget } from '../../../schema'; import { validateAwsCredentials } from '../../aws/account'; @@ -198,15 +204,31 @@ export function validateContainerAgents(projectSpec: AgentCoreProjectSpec, confi const errors: string[] = []; for (const agent of projectSpec.runtimes || []) { if (agent.build === 'Container') { - const codeLocation = resolveCodeLocation(agent.codeLocation, configRoot); - const dockerfilePath = getDockerfilePath(codeLocation, agent.dockerfile); + // Build context + Dockerfile via the shared resolver (identical to how ContainerSourceAsset + // uploads the context and how the CodeBuild buildspec resolves the -f path). + const { buildContext, dockerfilePath } = resolveBuildContext(agent, configRoot); if (!existsSync(dockerfilePath)) { errors.push( `Agent "${agent.name}": ${agent.dockerfile ?? DOCKERFILE_NAME} not found at ${dockerfilePath}. Container agents require a Dockerfile.` ); - } else { - warnDeprecatedBaseImage(dockerfilePath, agent.name); + } + 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.` + ); + } } } } diff --git a/src/cli/operations/dev/__tests__/config.test.ts b/src/cli/operations/dev/__tests__/config.test.ts index 6dbc25885..38b096451 100644 --- a/src/cli/operations/dev/__tests__/config.test.ts +++ b/src/cli/operations/dev/__tests__/config.test.ts @@ -484,6 +484,77 @@ describe('getDevConfig', () => { expect(config).not.toBeNull(); expect(config?.dockerfile).toBe('Dockerfile.gpu'); }); + + it('threads buildContextPath from Container agent spec to DevConfig (resolved absolute)', () => { + const project: AgentCoreProjectSpec = { + name: 'TestProject', + version: 1, + managedBy: 'CDK' as const, + runtimes: [ + { + name: 'ContainerAgent', + build: 'Container', + runtimeVersion: 'PYTHON_3_12', + entrypoint: filePath('main.py'), + codeLocation: dirPath('./agents/container'), + protocol: 'HTTP', + buildContextPath: dirPath('.'), + }, + ], + memories: [], + knowledgeBases: [], + credentials: [], + evaluators: [], + onlineEvalConfigs: [], + agentCoreGateways: [], + policyEngines: [], + configBundles: [], + abTests: [], + harnesses: [], + datasets: [], + payments: [], + }; + + const config = getDevConfig(workingDir, project, '/test/project/agentcore'); + expect(config).not.toBeNull(); + // resolveCodeDirectory('.', '/test/project/agentcore') => dirname('/test/project/agentcore') + '/' + '.' => '/test/project' + expect(config?.buildContextPath).toBe('/test/project'); + }); + + it('threads customDockerBuildArgs from Container agent spec to DevConfig', () => { + const project: AgentCoreProjectSpec = { + name: 'TestProject', + version: 1, + managedBy: 'CDK' as const, + runtimes: [ + { + name: 'ContainerAgent', + build: 'Container', + runtimeVersion: 'PYTHON_3_12', + entrypoint: filePath('main.py'), + codeLocation: dirPath('./agents/container'), + protocol: 'HTTP', + customDockerBuildArgs: { AGENT_NAME: 'myagent' }, + }, + ], + memories: [], + knowledgeBases: [], + credentials: [], + evaluators: [], + onlineEvalConfigs: [], + agentCoreGateways: [], + policyEngines: [], + configBundles: [], + abTests: [], + harnesses: [], + datasets: [], + payments: [], + }; + + const config = getDevConfig(workingDir, project, '/test/project/agentcore'); + expect(config).not.toBeNull(); + expect(config?.customDockerBuildArgs).toEqual({ AGENT_NAME: 'myagent' }); + }); }); describe('getAgentPort', () => { diff --git a/src/cli/operations/dev/__tests__/container-dev-server.test.ts b/src/cli/operations/dev/__tests__/container-dev-server.test.ts index bfd374e91..aebf552b3 100644 --- a/src/cli/operations/dev/__tests__/container-dev-server.test.ts +++ b/src/cli/operations/dev/__tests__/container-dev-server.test.ts @@ -3,6 +3,7 @@ import type { DevConfig } from '../config'; import { ContainerDevServer } from '../container-dev-server'; import type { DevServerCallbacks, DevServerOptions } from '../dev-server'; import { EventEmitter } from 'events'; +import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockSpawnSync = vi.fn(); @@ -226,6 +227,29 @@ describe('ContainerDevServer', () => { expect(buildArgs[tagIdx + 1]).toBe('agentcore-dev-testagent'); }); + it('forwards customDockerBuildArgs and uses buildContextPath as the context + Dockerfile root', async () => { + mockSuccessfulPrepare(); + + const monoConfig: DevConfig = { + ...defaultConfig, + directory: '/project/app/agent-one', + buildContextPath: '/project', + customDockerBuildArgs: { AGENT_NAME: 'one' }, + }; + const server = new ContainerDevServer(monoConfig, defaultOptions); + await server.start(); + + const buildArgs = mockSpawn.mock.calls[0]![1] as string[]; + // Build args are forwarded as --build-arg KEY=VALUE. + expect(buildArgs).toContain('--build-arg'); + expect(buildArgs).toContain('AGENT_NAME=one'); + // The positional build context (last arg) is buildContextPath, not codeLocation. + expect(buildArgs[buildArgs.length - 1]).toBe('/project'); + // The Dockerfile (-f) is resolved against the build context, not codeLocation. + const fIdx = buildArgs.indexOf('-f'); + expect(buildArgs[fIdx + 1]).toBe(join('/project', 'Dockerfile')); + }); + it('streams build output lines at system level in real-time', async () => { mockDetectContainerRuntime.mockResolvedValue({ runtime: { runtime: 'docker', binary: 'docker', version: 'Docker 24.0' }, diff --git a/src/cli/operations/dev/config.ts b/src/cli/operations/dev/config.ts index 37cd4c48d..0d5a358e7 100644 --- a/src/cli/operations/dev/config.ts +++ b/src/cli/operations/dev/config.ts @@ -11,6 +11,10 @@ export interface DevConfig { buildType: BuildType; protocol: ProtocolMode; dockerfile?: string; + /** Resolved absolute path to use as the Docker build context. Defaults to `directory`. */ + buildContextPath?: string; + /** Custom `--build-arg` key/value pairs forwarded to `docker build`. */ + customDockerBuildArgs?: Record; } interface DevSupportResult { @@ -139,6 +143,11 @@ export function getDevConfig( buildType: targetAgent.build, protocol: targetAgent.protocol ?? 'HTTP', dockerfile: targetAgent.dockerfile, + buildContextPath: + configRoot && targetAgent.buildContextPath + ? resolveCodeDirectory(targetAgent.buildContextPath, configRoot) + : undefined, + customDockerBuildArgs: targetAgent.customDockerBuildArgs, }; } diff --git a/src/cli/operations/dev/container-dev-server.ts b/src/cli/operations/dev/container-dev-server.ts index 5fb21ee29..f203c07f4 100644 --- a/src/cli/operations/dev/container-dev-server.ts +++ b/src/cli/operations/dev/container-dev-server.ts @@ -1,5 +1,6 @@ import { CONTAINER_INTERNAL_PORT, DOCKERFILE_NAME, getDockerfilePath } from '../../../lib'; -import { getUvBuildArgs } from '../../../lib/packaging/build-args'; +import { getCustomBuildArgs, getUvBuildArgs } from '../../../lib/packaging/build-args'; +import { ensureBuildContextDockerignore } from '../../../lib/packaging/build-context-dockerignore'; import { detectContainerRuntime } from '../../external-requirements/detect'; import { DevServer, type LogLevel, type SpawnConfig } from './dev-server'; import { waitForServerReady } from './utils'; @@ -65,21 +66,36 @@ export class ContainerDevServer extends DevServer { } this.runtimeBinary = runtime.binary; - // 2. Verify Dockerfile exists + // 2. Verify Dockerfile exists (resolved relative to the build context, matching deploy) + const buildContext = this.config.buildContextPath ?? this.config.directory; const dockerfileName = this.config.dockerfile ?? DOCKERFILE_NAME; - const dockerfilePath = getDockerfilePath(this.config.directory, this.config.dockerfile); + const dockerfilePath = getDockerfilePath(buildContext, this.config.dockerfile); if (!existsSync(dockerfilePath)) { onLog('error', `${dockerfileName} not found at ${dockerfilePath}. Container agents require a Dockerfile.`); return false; } + // Dockerfile validated — when buildContextPath widens the context, ensure a .dockerignore keeps + // secrets/junk out of the local image (the same file the deploy path honors). No-op if the dir is + // missing or a .dockerignore already exists. + if (this.config.buildContextPath) { + const created = ensureBuildContextDockerignore(buildContext); + if (created) { + onLog( + 'system', + `Created ${created} with default secret exclusions (buildContextPath is set); review and commit it.` + ); + } + } + // 3. Remove any stale container from a previous run (prevents "proxy already running" errors) spawnSync(this.runtimeBinary, ['rm', '-f', this.containerName], { stdio: 'ignore' }); // 4. Build the container image, streaming output in real-time onLog('system', `Building container image: ${this.imageName}...`); + const buildArgFlags = getCustomBuildArgs(this.config.customDockerBuildArgs); const exitCode = await this.streamBuild( - ['-t', this.imageName, '-f', dockerfilePath, ...getUvBuildArgs(), this.config.directory], + ['-t', this.imageName, '-f', dockerfilePath, ...getUvBuildArgs(), ...buildArgFlags, buildContext], onLog ); diff --git a/src/lib/__tests__/constants.test.ts b/src/lib/__tests__/constants.test.ts index 21b952b69..e4796ab96 100644 --- a/src/lib/__tests__/constants.test.ts +++ b/src/lib/__tests__/constants.test.ts @@ -25,23 +25,31 @@ describe('getDockerfilePath', () => { expect(getDockerfilePath('/app/code')).toBe(join('/app/code', 'Dockerfile')); }); - it('returns custom dockerfile name joined to code location', () => { + it('returns custom dockerfile name joined to the build context', () => { expect(getDockerfilePath('/app/code', 'Dockerfile.gpu')).toBe(join('/app/code', 'Dockerfile.gpu')); }); - it('rejects forward slash in dockerfile name', () => { - expect(() => getDockerfilePath('/app/code', '../Dockerfile')).toThrow(/Invalid dockerfile name/); + it('allows a relative subpath within the build context', () => { + expect(getDockerfilePath('/app/code', 'path/to/Dockerfile')).toBe(join('/app/code', 'path/to/Dockerfile')); }); - it('rejects backslash in dockerfile name', () => { - expect(() => getDockerfilePath('/app/code', 'Dockerfile\\..\\secret')).toThrow(/Invalid dockerfile name/); + it('rejects an absolute dockerfile path', () => { + expect(() => getDockerfilePath('/app/code', '/etc/Dockerfile')).toThrow(/Invalid dockerfile path/); }); - it('rejects dot-dot traversal in dockerfile name', () => { - expect(() => getDockerfilePath('/app/code', '..')).toThrow(/Invalid dockerfile name/); + it('rejects leading dot-dot traversal', () => { + expect(() => getDockerfilePath('/app/code', '../Dockerfile')).toThrow(/Invalid dockerfile path/); }); - it('rejects path/to/Dockerfile', () => { - expect(() => getDockerfilePath('/app/code', 'path/to/Dockerfile')).toThrow(/Invalid dockerfile name/); + it('rejects a dot-dot traversal segment mid-path', () => { + expect(() => getDockerfilePath('/app/code', 'a/../secret')).toThrow(/Invalid dockerfile path/); + }); + + it('rejects backslash in dockerfile path', () => { + expect(() => getDockerfilePath('/app/code', 'Dockerfile\\..\\secret')).toThrow(/Invalid dockerfile path/); + }); + + it('rejects a bare dot-dot', () => { + expect(() => getDockerfilePath('/app/code', '..')).toThrow(/Invalid dockerfile path/); }); }); diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 5ef7b5c34..d1a35d21a 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -1,3 +1,4 @@ +import { isValidDockerfilePath } from '../schema'; import { join } from 'path'; // Re-export all schema constants from schema @@ -51,14 +52,21 @@ export type ContainerRuntime = 'docker' | 'podman' | 'finch'; export const CONTAINER_RUNTIMES: ContainerRuntime[] = ['docker', 'podman', 'finch']; /** - * Get the Dockerfile path for a given code location. - * @param codeLocation - Directory containing the Dockerfile - * @param dockerfile - Custom Dockerfile name (default: 'Dockerfile') + * Resolve the Dockerfile path against the Docker build context. + * @param buildContext - The build context directory (`buildContextPath` when set, else `codeLocation`) + * @param dockerfile - Dockerfile name or relative subpath within the context (default: 'Dockerfile') + * + * `dockerfile` may be a filename ('Dockerfile') or a forward-slash relative subpath + * ('docker/Dockerfile'); absolute paths, backslashes, and `..` traversal are rejected so it can + * never escape the build context. */ -export function getDockerfilePath(codeLocation: string, dockerfile?: string): string { +export function getDockerfilePath(buildContext: string, dockerfile?: string): string { const name = dockerfile ?? DOCKERFILE_NAME; - if (name.includes('/') || name.includes('\\') || name.includes('..')) { - throw new Error(`Invalid dockerfile name: must be a filename without path separators or traversal`); + if (!isValidDockerfilePath(name)) { + throw new Error( + `Invalid dockerfile path "${name}": must be a relative path within the build context (a filename or ` + + `forward-slash subpath; no leading slash, backslash, empty segments, or ".." traversal)` + ); } - return join(codeLocation, name); + return join(buildContext, name); } diff --git a/src/lib/packaging/__tests__/build-context-dockerignore.test.ts b/src/lib/packaging/__tests__/build-context-dockerignore.test.ts new file mode 100644 index 000000000..e4df04515 --- /dev/null +++ b/src/lib/packaging/__tests__/build-context-dockerignore.test.ts @@ -0,0 +1,49 @@ +import { BUILD_CONTEXT_DOCKERIGNORE_TEMPLATE, ensureBuildContextDockerignore } from '../build-context-dockerignore.js'; +import { existsSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; + +describe('ensureBuildContextDockerignore', () => { + const created: string[] = []; + afterEach(() => { + created.length = 0; + }); + + it('creates a .dockerignore with secret exclusions when none exists', () => { + const ctx = mkdtempSync(join(tmpdir(), 'bctx-')); + const result = ensureBuildContextDockerignore(ctx); + + expect(result).toBe(join(ctx, '.dockerignore')); + expect(existsSync(join(ctx, '.dockerignore'))).toBe(true); + const contents = readFileSync(join(ctx, '.dockerignore'), 'utf-8'); + expect(contents).toBe(BUILD_CONTEXT_DOCKERIGNORE_TEMPLATE); + // Covers the real secret/junk vectors, both bare and nested (**/), plus the config dir. + for (const pattern of ['.env', '.env.*', '**/.env', '.git', '**/.git', '**/node_modules', 'agentcore/']) { + expect(contents).toContain(pattern); + } + }); + + it('never overwrites an existing .dockerignore (returns null)', () => { + const ctx = mkdtempSync(join(tmpdir(), 'bctx-')); + writeFileSync(join(ctx, '.dockerignore'), '# user-owned\ncustom-pattern\n'); + + const result = ensureBuildContextDockerignore(ctx); + + expect(result).toBeNull(); + // The user's content is preserved untouched. + expect(readFileSync(join(ctx, '.dockerignore'), 'utf-8')).toBe('# user-owned\ncustom-pattern\n'); + }); + + it('returns null without throwing when the build-context directory does not exist', () => { + // A missing/typo'd buildContextPath must be a no-op, not a raw ENOENT from writeFileSync — that + // would crash `dev`, bypass PackagingError in `package`, and mask preflight's "Dockerfile not found". + const missing = join(tmpdir(), 'bctx-does-not-exist-9f3a2b1c', 'nested'); + + expect(existsSync(missing)).toBe(false); + expect(() => ensureBuildContextDockerignore(missing)).not.toThrow(); + expect(ensureBuildContextDockerignore(missing)).toBeNull(); + // No .dockerignore was written into the (still-absent) directory. + expect(existsSync(join(missing, '.dockerignore'))).toBe(false); + }); +}); diff --git a/src/lib/packaging/__tests__/build-context.test.ts b/src/lib/packaging/__tests__/build-context.test.ts new file mode 100644 index 000000000..76b489ceb --- /dev/null +++ b/src/lib/packaging/__tests__/build-context.test.ts @@ -0,0 +1,45 @@ +import type { AgentEnvSpec } from '../../../schema'; +import { resolveBuildContext } from '../build-context.js'; +import { describe, expect, it } from 'vitest'; + +// resolveCodeLocation resolves relative paths against the repo root (dirname of the config dir). +const CONFIG_ROOT = '/project/agentcore'; // repo root => /project + +type ContextSpec = Pick; +const spec = (o: Partial = {}): ContextSpec => ({ codeLocation: './agents/one', ...o }) as ContextSpec; + +describe('resolveBuildContext', () => { + it('uses codeLocation as the build context when buildContextPath is unset', () => { + const { buildContext, dockerfilePath } = resolveBuildContext(spec(), CONFIG_ROOT); + expect(buildContext).toBe('/project/agents/one'); + expect(dockerfilePath).toBe('/project/agents/one/Dockerfile'); + }); + + it('uses buildContextPath as the build context when set, resolving the Dockerfile against it', () => { + const { buildContext, dockerfilePath } = resolveBuildContext( + spec({ buildContextPath: '.' as ContextSpec['buildContextPath'] }), + CONFIG_ROOT + ); + // The monorepo case: context is the repo root, and a shared root Dockerfile is resolved there — + // NOT under the agent's own codeLocation (this is what keeps local dev/package matching deploy). + expect(buildContext).toBe('/project'); + expect(dockerfilePath).toBe('/project/Dockerfile'); + }); + + it('resolves a custom dockerfile subpath against the build context', () => { + const { dockerfilePath } = resolveBuildContext( + spec({ + buildContextPath: '.' as ContextSpec['buildContextPath'], + dockerfile: 'docker/Dockerfile.gpu' as ContextSpec['dockerfile'], + }), + CONFIG_ROOT + ); + expect(dockerfilePath).toBe('/project/docker/Dockerfile.gpu'); + }); + + it('throws for an unsafe dockerfile path (traversal escaping the context)', () => { + expect(() => + resolveBuildContext(spec({ dockerfile: '../evil' as ContextSpec['dockerfile'] }), CONFIG_ROOT) + ).toThrow(/Invalid dockerfile path/); + }); +}); diff --git a/src/lib/packaging/__tests__/container.test.ts b/src/lib/packaging/__tests__/container.test.ts index cb5685dbf..015137226 100644 --- a/src/lib/packaging/__tests__/container.test.ts +++ b/src/lib/packaging/__tests__/container.test.ts @@ -121,6 +121,28 @@ describe('ContainerPackager', () => { expect(result.artifactPath).toBe('docker://agentcore-package-custom-agent'); }); + it('lower-cases the image tag for a mixed-case agent name (docker tags must be lowercase)', async () => { + mockResolveCodeLocation.mockReturnValue('/resolved/src'); + mockExistsSync.mockReturnValue(true); + mockSpawnSync.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'which' && args[0] === 'docker') return { status: 0 }; + if (cmd === 'docker' && args[0] === '--version') return { status: 0 }; + if (cmd === 'docker' && args[0] === 'build') return { status: 0 }; + if (cmd === 'docker' && args[0] === 'image') return { status: 0, stdout: Buffer.from('1000') }; + return { status: 1 }; + }); + + const result = await packager.pack({ ...baseSpec, name: 'AgentOne' } as any); + + // Docker rejects uppercase in image tags; the default agent name is PascalCase, so it must be lowered. + expect(result.artifactPath).toBe('docker://agentcore-package-agentone'); + const buildCall = mockSpawnSync.mock.calls.find( + (c: unknown[]) => c[0] === 'docker' && (c[1] as string[])[0] === 'build' + ); + const buildArgs = buildCall![1] as string[]; + expect(buildArgs[buildArgs.indexOf('-t') + 1]).toBe('agentcore-package-agentone'); + }); + it('uses options.artifactDir as configBaseDir', async () => { mockResolveCodeLocation.mockReturnValue('/artifact/dir/src'); mockExistsSync.mockReturnValue(true); @@ -210,6 +232,61 @@ describe('ContainerPackager', () => { await expect(packager.pack(specWithDockerfile as any)).rejects.toThrow('Dockerfile.custom not found'); }); + it('uses buildContextPath as docker build context instead of codeLocation', async () => { + mockResolveCodeLocation.mockImplementation((path: string) => { + if (path === './src') return '/resolved/src'; + if (path === './context') return '/resolved/context'; + return path; + }); + mockExistsSync.mockReturnValue(true); + mockSpawnSync.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'which' && args[0] === 'docker') return { status: 0 }; + if (cmd === 'docker' && args[0] === '--version') return { status: 0 }; + if (cmd === 'docker' && args[0] === 'build') return { status: 0 }; + if (cmd === 'docker' && args[0] === 'image') return { status: 0, stdout: Buffer.from('1000') }; + return { status: 1 }; + }); + + const specWithContext = { ...baseSpec, buildContextPath: './context' }; + await packager.pack(specWithContext as any); + + const buildCall = mockSpawnSync.mock.calls.find( + (c: unknown[]) => c[0] === 'docker' && (c[1] as string[])[0] === 'build' + ); + expect(buildCall).toBeDefined(); + const buildArgs = buildCall![1] as string[]; + // Last arg should be the buildContextPath, not codeLocation + expect(buildArgs[buildArgs.length - 1]).toBe('/resolved/context'); + // The Dockerfile (-f) must be resolved against the build context, not codeLocation — otherwise the + // local build diverges from the deploy build (which resolves it against the uploaded context root). + const fIdx = buildArgs.indexOf('-f'); + expect(buildArgs[fIdx + 1]).toBe('/resolved/context/Dockerfile'); + }); + + it('passes customDockerBuildArgs as --build-arg flags', async () => { + mockResolveCodeLocation.mockReturnValue('/resolved/src'); + mockExistsSync.mockReturnValue(true); + mockSpawnSync.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'which' && args[0] === 'docker') return { status: 0 }; + if (cmd === 'docker' && args[0] === '--version') return { status: 0 }; + if (cmd === 'docker' && args[0] === 'build') return { status: 0 }; + if (cmd === 'docker' && args[0] === 'image') return { status: 0, stdout: Buffer.from('1000') }; + return { status: 1 }; + }); + + const specWithBuildArgs = { ...baseSpec, customDockerBuildArgs: { AGENT_NAME: 'dummyagent', BUILD_ENV: 'prod' } }; + await packager.pack(specWithBuildArgs as any); + + const buildCall = mockSpawnSync.mock.calls.find( + (c: unknown[]) => c[0] === 'docker' && (c[1] as string[])[0] === 'build' + ); + expect(buildCall).toBeDefined(); + const buildArgs = buildCall![1] as string[]; + expect(buildArgs).toContain('--build-arg'); + expect(buildArgs).toContain('AGENT_NAME=dummyagent'); + expect(buildArgs).toContain('BUILD_ENV=prod'); + }); + it('detects podman runtime last', async () => { mockResolveCodeLocation.mockReturnValue('/resolved/src'); mockExistsSync.mockReturnValue(true); diff --git a/src/lib/packaging/build-args.ts b/src/lib/packaging/build-args.ts index eadc35875..5bee4c237 100644 --- a/src/lib/packaging/build-args.ts +++ b/src/lib/packaging/build-args.ts @@ -11,3 +11,12 @@ export function getUvBuildArgs(): string[] { if (config.uvIndex) args.push('--build-arg', `UV_INDEX=${config.uvIndex}`); return args; } + +/** + * Return Docker `--build-arg KEY=VALUE` flags for an agent's `customDockerBuildArgs`. + * Returns an empty array when there are none. Shared by the local `package` and `dev` build paths + * so the flag encoding stays in one place. + */ +export function getCustomBuildArgs(customDockerBuildArgs?: Record): string[] { + return Object.entries(customDockerBuildArgs ?? {}).flatMap(([key, value]) => ['--build-arg', `${key}=${value}`]); +} diff --git a/src/lib/packaging/build-context-dockerignore.ts b/src/lib/packaging/build-context-dockerignore.ts new file mode 100644 index 000000000..ac2ae6fdf --- /dev/null +++ b/src/lib/packaging/build-context-dockerignore.ts @@ -0,0 +1,60 @@ +import { existsSync, writeFileSync } from 'fs'; +import { join } from 'path'; + +/** + * Default `.dockerignore` written at a Container agent's build-context root when `buildContextPath` + * is set and none exists. Keeps secrets and build junk out of the image and out of the CodeBuild + * upload. Both local `agentcore dev`/`package` and `agentcore deploy` honor this same file, so what + * is excluded locally is exactly what is excluded on deploy. + */ +export const BUILD_CONTEXT_DOCKERIGNORE_TEMPLATE = `# Generated by agentcore because this agent sets buildContextPath — the whole of this directory is the +# Docker build context, so secrets/junk here would otherwise be baked into the image and uploaded to +# CodeBuild. Patterns are listed both bare and '**/'-prefixed so nested copies (e.g. +# app/agent-one/node_modules, app/agent-one/.env) are excluded too, not just the top level — Docker +# matches .dockerignore patterns root-relative. Both 'agentcore dev'/'package' and 'agentcore deploy' +# honor this file. Edit freely; to intentionally include a file listed here (e.g. a non-secret +# .env.production), add a '!'-prefixed line for it below. +.env +.env.* +**/.env +**/.env.* +.git +**/.git +.venv +**/.venv +node_modules +**/node_modules +__pycache__ +**/__pycache__ +.pytest_cache +**/.pytest_cache +.DS_Store +**/.DS_Store +agentcore/ +`; + +/** + * Ensure a `.dockerignore` exists at the resolved build-context root. No-op when the directory does + * not exist or a `.dockerignore` is already present (the user's file is never overwritten). Returns + * the path when it creates one, otherwise null. + * + * Guards the DIRECTORY, not just the file: a missing/typo'd `buildContextPath` would otherwise make + * `writeFileSync` throw a raw ENOENT — crashing `agentcore dev`, bypassing `PackagingError` in + * `package`, and masking preflight's friendly "Dockerfile not found". Callers invoke this only AFTER + * validating that the Dockerfile exists, so on a failing build/deploy no stray `.dockerignore` is + * left behind. + * + * Called only when `buildContextPath` is set — that is when the context is widened beyond the agent's + * own `codeLocation` (e.g. to a monorepo root) and a stray root-level `.env`/`.git` would be swept in. + */ +export function ensureBuildContextDockerignore(resolvedBuildContext: string): string | null { + if (!existsSync(resolvedBuildContext)) { + return null; + } + const dockerignorePath = join(resolvedBuildContext, '.dockerignore'); + if (existsSync(dockerignorePath)) { + return null; + } + writeFileSync(dockerignorePath, BUILD_CONTEXT_DOCKERIGNORE_TEMPLATE); + return dockerignorePath; +} diff --git a/src/lib/packaging/build-context.ts b/src/lib/packaging/build-context.ts new file mode 100644 index 000000000..dd895a7d9 --- /dev/null +++ b/src/lib/packaging/build-context.ts @@ -0,0 +1,30 @@ +import type { AgentEnvSpec } from '../../schema'; +import { getDockerfilePath } from '../constants'; +import { resolveCodeLocation } from './helpers'; + +/** Resolved Docker build context and Dockerfile path for a Container agent. */ +export interface ResolvedBuildContext { + /** Absolute path to the Docker build context (`buildContextPath` when set, else `codeLocation`). */ + buildContext: string; + /** Absolute path to the Dockerfile, resolved against the build context. */ + dockerfilePath: string; +} + +/** + * Resolve the Docker build context + Dockerfile path for a Container agent — the single source of + * truth shared by `package` (ContainerPackager), `deploy` preflight, and `dev`. + * + * The context is `buildContextPath` when set, otherwise `codeLocation`, resolved via + * `resolveCodeLocation` (the same resolver the deploy-side `ContainerSourceAsset` uses to upload it), + * and the Dockerfile is resolved against that context (matching the CodeBuild buildspec's `-f`). + * Collapsing this into one function keeps local `dev`/`package` and `deploy` from ever resolving the + * context differently — a divergence would otherwise let a config build from one directory locally and + * a different one on deploy. + */ +export function resolveBuildContext( + spec: Pick, + baseDir: string +): ResolvedBuildContext { + const buildContext = resolveCodeLocation(spec.buildContextPath ?? spec.codeLocation, baseDir); + return { buildContext, dockerfilePath: getDockerfilePath(buildContext, spec.dockerfile) }; +} diff --git a/src/lib/packaging/container.ts b/src/lib/packaging/container.ts index fc9400a86..b70eab568 100644 --- a/src/lib/packaging/container.ts +++ b/src/lib/packaging/container.ts @@ -1,7 +1,9 @@ import type { AgentEnvSpec } from '../../schema'; -import { CONTAINER_RUNTIMES, DOCKERFILE_NAME, ONE_GB, getDockerfilePath } from '../constants'; +import { CONTAINER_RUNTIMES, DOCKERFILE_NAME, ONE_GB } from '../constants'; import { PackagingError } from '../errors/types'; -import { getUvBuildArgs } from './build-args'; +import { getCustomBuildArgs, getUvBuildArgs } from './build-args'; +import { resolveBuildContext } from './build-context'; +import { ensureBuildContextDockerignore } from './build-context-dockerignore'; import { resolveCodeLocation } from './helpers'; import type { ArtifactResult, PackageOptions, RuntimePackager } from './types/packaging'; import { spawnSync } from 'child_process'; @@ -35,7 +37,8 @@ export class ContainerPackager implements RuntimePackager { const agentName = options.agentName ?? spec.name; const configBaseDir = options.artifactDir ?? options.projectRoot ?? process.cwd(); const codeLocation = resolveCodeLocation(spec.codeLocation, configBaseDir); - const dockerfilePath = getDockerfilePath(codeLocation, spec.dockerfile); + // Build context + Dockerfile via the shared resolver (identical to the deploy/CodeBuild path). + const { buildContext, dockerfilePath } = resolveBuildContext(spec, configBaseDir); // Preflight: Dockerfile must exist if (!existsSync(dockerfilePath)) { @@ -46,6 +49,13 @@ export class ContainerPackager implements RuntimePackager { ); } + // Dockerfile validated — when buildContextPath widens the context, ensure a .dockerignore keeps + // secrets/junk out of the local image (the same file the deploy path honors). No-op if the dir is + // missing or a .dockerignore already exists, so a failing build never leaves a stray file. + if (spec.buildContextPath) { + ensureBuildContextDockerignore(buildContext); + } + // Detect container runtime const runtime = detectContainerRuntimeSync(); if (!runtime) { @@ -57,11 +67,13 @@ export class ContainerPackager implements RuntimePackager { }); } - // Build locally - const imageName = `agentcore-package-${agentName}`; + // Build locally. Docker image tags must be lowercase, but agent names allow uppercase + // (e.g. the default "AgentOne"), so lower-case the tag — matching the dev server. + const imageName = `agentcore-package-${agentName}`.toLowerCase(); + const buildArgFlags = getCustomBuildArgs(spec.customDockerBuildArgs); const buildResult = spawnSync( runtime, - ['build', '-t', imageName, '-f', dockerfilePath, ...getUvBuildArgs(), codeLocation], + ['build', '-t', imageName, '-f', dockerfilePath, ...getUvBuildArgs(), ...buildArgFlags, buildContext], { stdio: 'pipe', } diff --git a/src/lib/packaging/index.ts b/src/lib/packaging/index.ts index 8c3067ca3..d97754cab 100644 --- a/src/lib/packaging/index.ts +++ b/src/lib/packaging/index.ts @@ -93,3 +93,5 @@ export type { } from './types/packaging'; export { resolveCodeLocation } from './helpers'; +export { ensureBuildContextDockerignore } from './build-context-dockerignore'; +export { resolveBuildContext, type ResolvedBuildContext } from './build-context'; diff --git a/src/schema/llm-compacted/agentcore.ts b/src/schema/llm-compacted/agentcore.ts index 13e2ba719..2ab5290cf 100644 --- a/src/schema/llm-compacted/agentcore.ts +++ b/src/schema/llm-compacted/agentcore.ts @@ -102,7 +102,9 @@ interface AgentEnvSpec { build: BuildType; entrypoint: string; // @regex ^[a-zA-Z0-9_][a-zA-Z0-9_/.-]*\.(py|ts|js)(:[a-zA-Z_][a-zA-Z0-9_]*)?$ e.g. "main.py:handler" or "index.ts" codeLocation: string; // Directory path - dockerfile?: string; // Custom Dockerfile name for Container builds (default: 'Dockerfile'). Must be a filename, not a path. + dockerfile?: string; // @regex ^[A-Za-z0-9._/-]+$ @max 255 Dockerfile for Container builds, resolved relative to the build context (buildContextPath ?? codeLocation). Filename or relative subpath; no leading slash, trailing/double slash, or '..' traversal. Default: 'Dockerfile'. + buildContextPath?: string; // Container only. Docker build context directory; replaces codeLocation as the `docker build` context (e.g. a monorepo root shared across agents). + customDockerBuildArgs?: Record; // Container only. --build-arg flags. Keys are env-var names @regex ^[A-Za-z_][A-Za-z0-9_]*$ @max 255 (reserved build-env names rejected); values @max 4096, no control chars. runtimeVersion?: RuntimeVersion; envVars?: EnvVar[]; networkMode?: NetworkMode; // default 'PUBLIC' diff --git a/src/schema/schemas/__tests__/agent-env.test.ts b/src/schema/schemas/__tests__/agent-env.test.ts index 57fc01657..750f1a0d0 100644 --- a/src/schema/schemas/__tests__/agent-env.test.ts +++ b/src/schema/schemas/__tests__/agent-env.test.ts @@ -506,6 +506,12 @@ describe('AgentEnvSpecSchema - dockerfile', () => { ); }); + it('accepts a relative subpath (resolved against the build context)', () => { + expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: 'docker/Dockerfile' }).success).toBe( + true + ); + }); + it('rejects dockerfile on CodeZip builds', () => { const result = AgentEnvSpecSchema.safeParse({ ...validCodeZipAgent, dockerfile: 'Dockerfile.custom' }); expect(result.success).toBe(false); @@ -514,11 +520,10 @@ describe('AgentEnvSpecSchema - dockerfile', () => { } }); - it('rejects path traversal or path separator in dockerfile', () => { + it('rejects path traversal and absolute paths in dockerfile', () => { expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: '../Dockerfile' }).success).toBe(false); - expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: 'path/to/Dockerfile' }).success).toBe( - false - ); + expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: 'a/../secret' }).success).toBe(false); + expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: '/etc/Dockerfile' }).success).toBe(false); }); it('rejects empty string dockerfile', () => { @@ -547,6 +552,171 @@ describe('AgentEnvSpecSchema - dockerfile', () => { ); expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: '..\\Dockerfile' }).success).toBe(false); }); + + it('rejects a trailing slash (a directory-shaped value would make `docker build -f docker/` fail)', () => { + expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: 'docker/' }).success).toBe(false); + }); + + it('rejects an empty path segment (double slash)', () => { + expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: 'a//Dockerfile' }).success).toBe(false); + }); + + it('accepts a leading-dot directory (e.g. .docker/Dockerfile)', () => { + expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: '.docker/Dockerfile' }).success).toBe( + true + ); + }); +}); + +describe('AgentEnvSpecSchema - buildContextPath', () => { + const validContainerAgent = { + name: 'ContainerAgent', + build: 'Container', + entrypoint: 'main.py', + codeLocation: './agents/container', + }; + + const validCodeZipAgent = { + name: 'CodeZipAgent', + build: 'CodeZip', + entrypoint: 'main.py:handler', + codeLocation: './agents/test', + runtimeVersion: 'PYTHON_3_12', + }; + + it('accepts Container agent with buildContextPath', () => { + const result = AgentEnvSpecSchema.safeParse({ ...validContainerAgent, buildContextPath: '.' }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.buildContextPath).toBe('.'); + } + }); + + it('accepts Container agent without buildContextPath (optional)', () => { + const result = AgentEnvSpecSchema.safeParse(validContainerAgent); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.buildContextPath).toBeUndefined(); + } + }); + + it('rejects buildContextPath on CodeZip builds', () => { + const result = AgentEnvSpecSchema.safeParse({ ...validCodeZipAgent, buildContextPath: '.' }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.some(i => i.message.includes('only allowed for Container'))).toBe(true); + } + }); +}); + +describe('AgentEnvSpecSchema - customDockerBuildArgs', () => { + const validContainerAgent = { + name: 'ContainerAgent', + build: 'Container', + entrypoint: 'main.py', + codeLocation: './agents/container', + }; + + const validCodeZipAgent = { + name: 'CodeZipAgent', + build: 'CodeZip', + entrypoint: 'main.py:handler', + codeLocation: './agents/test', + runtimeVersion: 'PYTHON_3_12', + }; + + it('accepts Container agent with customDockerBuildArgs', () => { + const result = AgentEnvSpecSchema.safeParse({ + ...validContainerAgent, + customDockerBuildArgs: { AGENT_NAME: 'dummyagent', BUILD_ENV: 'prod' }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.customDockerBuildArgs).toEqual({ AGENT_NAME: 'dummyagent', BUILD_ENV: 'prod' }); + } + }); + + it('accepts Container agent without customDockerBuildArgs (optional)', () => { + const result = AgentEnvSpecSchema.safeParse(validContainerAgent); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.customDockerBuildArgs).toBeUndefined(); + } + }); + + it('rejects customDockerBuildArgs on CodeZip builds', () => { + const result = AgentEnvSpecSchema.safeParse({ ...validCodeZipAgent, customDockerBuildArgs: { KEY: 'val' } }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.some(i => i.message.includes('only allowed for Container'))).toBe(true); + } + }); + + it('accepts empty customDockerBuildArgs object', () => { + const result = AgentEnvSpecSchema.safeParse({ ...validContainerAgent, customDockerBuildArgs: {} }); + expect(result.success).toBe(true); + }); + + it('rejects a key that is not a valid identifier', () => { + expect( + AgentEnvSpecSchema.safeParse({ ...validContainerAgent, customDockerBuildArgs: { 'has-dash': 'v' } }).success + ).toBe(false); + }); + + it.each([ + 'IMAGE_URI', + 'ECR_REGISTRY', + 'DOCKERFILE_PATH', + 'BUILD_ARG_FLAGS', + 'AWS_DEFAULT_REGION', + 'CODEBUILD_FOO', + 'PATH', + 'HOME', + 'IFS', + 'LD_PRELOAD', + 'LD_LIBRARY_PATH', + 'DOCKER_HOST', + 'DOCKER_CONFIG', + ])('rejects the build-environment-reserved key %s (would break only on deploy)', key => { + const result = AgentEnvSpecSchema.safeParse({ ...validContainerAgent, customDockerBuildArgs: { [key]: 'v' } }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.some(i => i.message.includes('reserved by the build environment'))).toBe(true); + } + }); + + it.each(['USER', 'LANG', 'SHELL', 'TERM'])('accepts the common non-dangerous build-arg key %s', key => { + // Legitimate Dockerfile ARGs (e.g. `ARG USER` -> `useradd $USER`) must not be over-rejected. + const result = AgentEnvSpecSchema.safeParse({ ...validContainerAgent, customDockerBuildArgs: { [key]: 'v' } }); + expect(result.success).toBe(true); + }); + + it('accepts a normal value with spaces and symbols', () => { + const result = AgentEnvSpecSchema.safeParse({ + ...validContainerAgent, + customDockerBuildArgs: { GREETING: 'hello world = ok!' }, + }); + expect(result.success).toBe(true); + }); + + it('rejects a value containing a newline or control character', () => { + const result = AgentEnvSpecSchema.safeParse({ + ...validContainerAgent, + customDockerBuildArgs: { BAD: 'line1\nline2' }, + }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.some(i => i.message.includes('control characters'))).toBe(true); + } + }); + + it('rejects a value longer than 4096 characters', () => { + const result = AgentEnvSpecSchema.safeParse({ + ...validContainerAgent, + customDockerBuildArgs: { BIG: 'x'.repeat(4097) }, + }); + expect(result.success).toBe(false); + }); }); describe('AgentEnvSpecSchema - lifecycleConfiguration', () => { diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index e5cd74395..dc286134a 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -94,6 +94,88 @@ export const EntrypointSchema = z const DirectoryPathSchema = z.string().min(1) as unknown as z.ZodType; +// Char-safe set only (no backslash, spaces, or shell metacharacters): on deploy the buildspec +// interpolates this unquoted as `docker build -f $DOCKERFILE_PATH`. A leading '.' is allowed so +// conventional paths like '.docker/Dockerfile' pass. +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. + * + * 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' + * while the other rejects it, or one accepting a trailing-slash directory value like 'docker/'. + */ +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 === '..'); +} + +/** + * Dockerfile location for Container builds, resolved relative to the Docker build context + * (`buildContextPath` when set, otherwise `codeLocation`). Accepts a filename ('Dockerfile', + * 'Dockerfile.gpu') or a forward-slash relative subpath ('docker/Dockerfile'). Absolute paths, + * trailing/double slashes, and '..' traversal are rejected so the Dockerfile can never escape or + * name the build context directory itself. + */ +const DockerfilePathSchema = z + .string() + .min(1) + .max(255) + .refine( + isValidDockerfilePath, + 'Must be a relative path within the build context: a filename or forward-slash subpath using only letters, digits, dot, dash, underscore, and slash; no leading slash, no ".." traversal, and no empty segments (no trailing or double slash)' + ); + +/** + * Build-arg names reserved by the CodeBuild build environment. On deploy each build arg becomes a + * CodeBuild environment-variable override alongside the buildspec's own control vars, so a colliding + * name would break the deploy build — a wrong image tag / ECR region, a rejected StartBuild, or a + * clobbered build-container shell. We reject the buildspec control vars, the process-critical shell + * vars, and CodeBuild's own `AWS_*` / `CODEBUILD_*` namespaces. This is a best-effort denylist (the + * shell has other sensitive vars), but it covers the realistic footguns and keeps a config that + * builds locally from failing only on deploy. + */ +export const RESERVED_BUILD_ARG_KEYS = [ + // Buildspec control vars (mirrored in @aws/agentcore-cdk's ContainerBuildProject / build handler). + 'ECR_REGISTRY', + 'IMAGE_URI', + 'DOCKERFILE_PATH', + 'BUILD_ARG_FLAGS', + // Overriding these in the build shell would break the buildspec's own commands (command lookup, + // word-splitting, dynamic linking) or redirect the docker client off the build daemon. Kept + // deliberately narrow to the genuinely-dangerous names so common ARGs (e.g. USER, LANG) stay usable. + 'PATH', + 'HOME', + 'IFS', + 'LD_PRELOAD', + 'LD_LIBRARY_PATH', + 'DOCKER_HOST', + 'DOCKER_CONFIG', + 'DOCKER_TLS_VERIFY', + 'DOCKER_CERT_PATH', +]; + +export function isReservedBuildArgKey(key: string): boolean { + return RESERVED_BUILD_ARG_KEYS.includes(key) || key.startsWith('CODEBUILD_') || key.startsWith('AWS_'); +} + +/** + * Build-arg value. Bounded and control-char-free because on deploy each value becomes a CodeBuild + * `environmentVariablesOverride` PLAINTEXT entry; newlines/control chars or an over-long value can be + * rejected by StartBuild, so we reject them at schema time rather than let a local-only-valid config + * fail on deploy. + */ +const BuildArgValueSchema = z + .string() + .max(4096, 'Build arg values must be at most 4096 characters') + .refine( + v => ![...v].some(ch => ch.charCodeAt(0) < 0x20 || ch.charCodeAt(0) === 0x7f), + 'Build arg values must not contain control characters (including newlines)' + ); + export const EnvVarSchema = z.object({ name: EnvVarNameSchema, value: z.string(), @@ -319,13 +401,29 @@ export const AgentEnvSpecSchema = z build: BuildTypeSchema, entrypoint: EntrypointSchema, codeLocation: DirectoryPathSchema, - /** Custom Dockerfile name for Container builds. Must be a filename, not a path. Default: 'Dockerfile' */ - dockerfile: z - .string() - .min(1) - .max(255) - .regex(/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/, 'Must be a filename (no path separators or traversal)') - .optional(), + /** + * Dockerfile for Container builds, resolved relative to the build context (`buildContextPath` + * when set, otherwise `codeLocation`). A filename or a relative subpath; no traversal. Default: 'Dockerfile'. + */ + dockerfile: DockerfilePathSchema.optional(), + /** + * Docker build context directory for Container builds. When set, this directory (instead of + * `codeLocation`) is used as the `docker build` context and as the root the Dockerfile path is + * resolved against, so a single Dockerfile can be shared across multiple agents in a monorepo + * (e.g. so it can COPY files that live outside `codeLocation`). Container builds only. + * + * Like `codeLocation`, this is intentionally not containment-checked: the config author may point + * it at any directory (including a parent via `..` for a monorepo root). The unconditional + * secret-exclusion baseline on the deploy upload still applies to whatever directory is chosen. + */ + buildContextPath: DirectoryPathSchema.optional(), + /** + * Custom build arguments passed to `docker build` as `--build-arg` flags. Keys must be valid + * environment-variable names (each arg is forwarded to the build as an env var), reusing + * `EnvVarNameSchema`. Useful for parameterising a shared Dockerfile per agent + * (e.g. `{ "AGENT_NAME": "myagent" }`). Container builds only. + */ + customDockerBuildArgs: z.record(EnvVarNameSchema, BuildArgValueSchema).optional(), runtimeVersion: RuntimeVersionSchemaFromConstants.optional(), /** Environment variables to set on the runtime */ envVars: z.array(EnvVarSchema).optional(), @@ -410,13 +508,25 @@ export const AgentEnvSpecSchema = z path: ['authorizerConfiguration'], }); } - // If adding more Container-specific fields, consider consolidating into a containerConfig object (see networkConfig pattern) - if (data.build !== 'Container' && data.dockerfile) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: 'dockerfile is only allowed for Container builds', - path: ['dockerfile'], - }); + // Container-only fields. If adding more, extend this list (and consider consolidating into a + // containerConfig object — see the networkConfig pattern). + for (const field of ['dockerfile', 'buildContextPath', 'customDockerBuildArgs'] as const) { + if (data.build !== 'Container' && data[field]) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `${field} is only allowed for Container builds`, + path: [field], + }); + } + } + for (const key of Object.keys(data.customDockerBuildArgs ?? {})) { + if (isReservedBuildArgKey(key)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `customDockerBuildArgs key "${key}" is reserved by the build environment (must not be ${RESERVED_BUILD_ARG_KEYS.join(', ')}, or start with CODEBUILD_ or AWS_)`, + path: ['customDockerBuildArgs', key], + }); + } } const fcs = data.filesystemConfigurations ?? []; if (fcs.length > 0) {