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
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
"agentcore": "./dist/index.js"
},
"main": "./dist/index.js",
"engines": {
"node": ">=20.12.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why exactly is this needed? is there a component bun is missing that we need here?

},
"files": [
"dist"
],
Expand Down
6 changes: 3 additions & 3 deletions src/assets/templates/shared/env.local.template
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Environment variables for local development.
# `agentcore dev` loads this file into your agent's process. Values here
# override anything the CLI injects. This file is gitignored — keep secrets
# out of version control, but they are safe here.
# `agentcore project dev` loads this file into your agent's process. Values here
# override injected values except PORT, FASTMCP_PORT, and LOCAL_DEV, which the
# CLI owns. This file is gitignored — keep secrets out of version control.
#
# Example:
# MY_API_KEY=...
69 changes: 67 additions & 2 deletions src/core/dev/container.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterEach, describe, expect, test } from "bun:test";
import { createHash } from "node:crypto";
import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { InputValidationError, InvalidEnvironmentError } from "../../errors";
Expand All @@ -11,7 +11,7 @@ import {
type ProcessStreamer,
type StreamProcessOptions,
} from "../../io";
import type { ProjectRuntime } from "../project/schema";
import type { ProjectRuntime } from "../../projectSchemas/runtime";
import { ContainerDevRunner } from "./container";

type ProcessCall = {
Expand Down Expand Up @@ -61,6 +61,8 @@ function harness(
config: {
available?: (tool: string, probeArgs?: string[]) => Promise<boolean>;
stream?: StreamBehavior;
awsDirectory?: string;
processEnv?: NodeJS.ProcessEnv;
} = {},
) {
const calls: ProcessCall[] = [];
Expand All @@ -77,6 +79,11 @@ function harness(
(async (tool) => {
return tool === "docker";
}),
awsDirectory: config.awsDirectory ?? join(tmpdir(), "agentcore-container-no-aws"),
processEnv: config.processEnv ?? {
AWS_ACCESS_KEY_ID: "test-access-key",
AWS_SECRET_ACCESS_KEY: "test-secret-key",
},
}),
};
}
Expand Down Expand Up @@ -189,6 +196,10 @@ describe("ContainerDevRunner", () => {
"-p",
`127.0.0.1:3000:${containerPort}`,
"-e",
"AWS_ACCESS_KEY_ID=test-access-key",
"-e",
"AWS_SECRET_ACCESS_KEY=test-secret-key",
"-e",
"API_KEY=super-secret",
"-e",
`PORT=${containerPort}`,
Expand All @@ -202,6 +213,35 @@ describe("ContainerDevRunner", () => {
expect(run.options.redactedCommand?.join(" ")).not.toContain("super-secret");
});

test("uses a shared AWS config and rejects missing credentials", async () => {
const projectRuntime = runtime();
const root = await projectRoot(projectRuntime);
const awsDirectory = join(root, ".aws");
await mkdir(awsDirectory);
await writeFile(join(awsDirectory, "config"), "[profile sandbox]\nregion=us-east-1\n");
const { calls, runner } = harness({
awsDirectory,
processEnv: { AWS_PROFILE: "sandbox", AWS_REGION: "us-east-1" },
});

await collect(runner.run(input(root, projectRuntime)));

const run = commandCall(calls, "run");
expect(run.command).toContain(`${awsDirectory}:/aws-config:ro`);
expect(run.command).toContain("AWS_PROFILE=sandbox");
expect(run.command).toContain("AWS_CONFIG_FILE=/aws-config/config");
expect(run.options.redactedCommand?.join(" ")).not.toContain("sandbox");

const missing = harness({
awsDirectory: join(root, "missing-aws"),
processEnv: {},
});
await expect(collect(missing.runner.run(input(root, projectRuntime)))).rejects.toThrow(
"Unable to resolve AWS credentials for the container",
);
expect(missing.calls).toHaveLength(0);
});

test("preserves an existing build context .dockerignore", async () => {
const projectRuntime = runtime({ buildContextPath: "." });
const root = await projectRoot(projectRuntime);
Expand Down Expand Up @@ -409,6 +449,31 @@ describe("ContainerDevRunner", () => {
expect(calls.map(({ command }) => command[1])).toEqual(["rm"]);
});

test("rejects build contexts outside the project root, including symlinks", async () => {
const root = await mkdtemp(join(tmpdir(), "agentcore-container-"));
const outside = await mkdtemp(join(tmpdir(), "agentcore-container-outside-"));
tempDirectories.push(root, outside);
await symlink(outside, join(root, "linked"), process.platform === "win32" ? "junction" : "dir");
const probes: string[] = [];

for (const buildContextPath of ["..", "linked"]) {
const { calls, runner } = harness({
available: async (tool) => {
probes.push(tool);
return true;
},
});

await expect(collect(runner.run(input(root, runtime({ buildContextPath }))))).rejects.toThrow(
"container build context must be within the project root",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we validate the error types in these tests?

);
expect(calls).toHaveLength(0);
}

expect(probes).toHaveLength(0);
await expect(readFile(join(outside, ".dockerignore"), "utf8")).rejects.toThrow();
});

test("rejects a build context that is not a directory", async () => {
const root = await mkdtemp(join(tmpdir(), "agentcore-container-"));
tempDirectories.push(root);
Expand Down
68 changes: 56 additions & 12 deletions src/core/dev/container.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createHash } from "node:crypto";
import { existsSync, statSync, writeFileSync } from "node:fs";
import { join, resolve } from "node:path";
import { existsSync, realpathSync, statSync, writeFileSync } from "node:fs";
import { homedir } from "node:os";
import { isAbsolute, join, relative, resolve, sep } from "node:path";
import { InputValidationError, InvalidEnvironmentError } from "../../errors";
import type { DevEvent, DevRunner, DevServerInput } from "../../handlers/project/dev/types";
import {
Expand All @@ -10,8 +11,17 @@ import {
type ProcessStreamer,
type StreamProcessOptions,
} from "../../io";
import { DEV_PORTS } from "./port";

const CONTAINER_TOOLS = ["docker", "podman", "finch"] as const;
const AWS_ENV_KEYS = [
"AWS_ACCESS_KEY_ID",
"AWS_SECRET_ACCESS_KEY",
"AWS_SESSION_TOKEN",
"AWS_REGION",
"AWS_DEFAULT_REGION",
"AWS_PROFILE",
] as const;
const CLEANUP_TIMEOUT_MS = 2_000;
const DOCKERFILE_NAME = "Dockerfile";
const CONTAINER_RUNTIME_INSTALL_HINT =
Expand Down Expand Up @@ -44,15 +54,21 @@ type ToolAvailable = typeof toolAvailable;
type ContainerDevRunnerConfig = {
streamProcess?: ProcessStreamer;
toolAvailable?: ToolAvailable;
awsDirectory?: string;
processEnv?: NodeJS.ProcessEnv;
};

export class ContainerDevRunner implements DevRunner {
private readonly streamProcess: ProcessStreamer;
private readonly toolAvailable: ToolAvailable;
private readonly awsDirectory: string;
private readonly processEnv: NodeJS.ProcessEnv;

constructor(config: ContainerDevRunnerConfig = {}) {
this.streamProcess = config.streamProcess ?? streamProcess;
this.toolAvailable = config.toolAvailable ?? toolAvailable;
this.awsDirectory = config.awsDirectory ?? join(homedir(), ".aws");
this.processEnv = config.processEnv ?? process.env;
}

public async *run(input: DevServerInput): AsyncGenerator<DevEvent, void> {
Expand All @@ -65,12 +81,36 @@ export class ContainerDevRunner implements DevRunner {
throw new InputValidationError(`container build context directory not found: ${context}`);
}

const canonicalContext = realpathSync(context);
const relativeContext = relative(realpathSync(input.projectRoot), canonicalContext);
if (
relativeContext === ".." ||
relativeContext.startsWith(`..${sep}`) ||
isAbsolute(relativeContext)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in what cases does relative return an absolute path?

) {
throw new InputValidationError(
`container build context must be within the project root: ${canonicalContext}`,
);
}

const dockerfile = input.runtime.dockerfile ?? DOCKERFILE_NAME;
const dockerfilePath = join(context, dockerfile);
if (!isFile(dockerfilePath)) {
throw new InputValidationError(`container Dockerfile not found: ${dockerfilePath}`);
}

const hasAwsCredentials = Boolean(
(input.env?.AWS_ACCESS_KEY_ID ?? this.processEnv.AWS_ACCESS_KEY_ID) &&
(input.env?.AWS_SECRET_ACCESS_KEY ?? this.processEnv.AWS_SECRET_ACCESS_KEY),
);
const hasAwsConfig = existsSync(this.awsDirectory);
if (!hasAwsCredentials && !hasAwsConfig) {
throw new InputValidationError(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think it would be useful to distinguish the missing credentials case from the invalid inputs case in telemetry with a separate error type here?

"Unable to resolve AWS credentials for the container. Configure AWS credentials " +
"or set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY, then retry.",
);
}

const tool = await this.resolveContainerTool(input.signal);
input.signal.throwIfAborted();
if (input.runtime.buildContextPath) {
Expand Down Expand Up @@ -116,15 +156,23 @@ export class ContainerDevRunner implements DevRunner {
yield { type: "status", message: `Building image with ${tool}` };
yield* this.streamProcess(buildCommand, buildOptions);

const containerPort = portForProtocol(input.runtime.protocol);
const forwardedEnv: Record<string, string> = {
...input.env,
const containerPort = DEV_PORTS[input.runtime.protocol ?? "HTTP"];
const forwardedEnv: Record<string, string> = {};
for (const key of AWS_ENV_KEYS) {
if (this.processEnv[key]) forwardedEnv[key] = this.processEnv[key];
}
Object.assign(forwardedEnv, input.env, {
PORT: String(containerPort),
LOCAL_DEV: "1",
};
});
if (input.runtime.protocol === "MCP") {
forwardedEnv.FASTMCP_PORT = String(containerPort);
}
const awsMount = hasAwsConfig ? ["-v", `${this.awsDirectory}:/aws-config:ro`] : [];
if (awsMount.length) {
forwardedEnv.AWS_CONFIG_FILE = "/aws-config/config";
forwardedEnv.AWS_SHARED_CREDENTIALS_FILE = "/aws-config/credentials";
}
const envFlags = Object.entries(forwardedEnv).flatMap(([key, value]) => [
"-e",
`${key}=${value}`,
Expand All @@ -141,6 +189,7 @@ export class ContainerDevRunner implements DevRunner {
containerName,
"-p",
`127.0.0.1:${input.port}:${containerPort}`,
...awsMount,
...envFlags,
imageTag,
];
Expand All @@ -158,6 +207,7 @@ export class ContainerDevRunner implements DevRunner {
containerName,
"-p",
`127.0.0.1:${input.port}:${containerPort}`,
...awsMount,
...redactedEnvFlags,
imageTag,
],
Expand Down Expand Up @@ -204,12 +254,6 @@ export class ContainerDevRunner implements DevRunner {
}
}

function portForProtocol(protocol: DevServerInput["runtime"]["protocol"]): number {
if (protocol === "MCP") return 8000;
if (protocol === "A2A") return 9000;
return 8080;
}

function isDirectory(path: string): boolean {
try {
return statSync(path).isDirectory();
Expand Down
49 changes: 49 additions & 0 deletions src/core/dev/port.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { describe, expect, test } from "bun:test";
import type { PortChecker } from "../../io";
import { resolveDevPort } from "./port";

const signal = new AbortController().signal;

describe("resolveDevPort", () => {
test.each([
["HTTP", 8080],
["AGUI", 8080],
["MCP", 8000],
["A2A", 9000],
] as const)("uses the %s default", async (protocol, port) => {
expect(await resolveDevPort(protocol, undefined, async () => true, signal)).toEqual({
port,
requestedPort: port,
});
});

test("walks up from occupied defaults", async () => {
const checked: number[] = [];
const check: PortChecker = async (port) => {
checked.push(port);
return port === 8002;
};

expect(await resolveDevPort("MCP", undefined, check, signal)).toEqual({
port: 8002,
requestedPort: 8000,
});
expect(checked).toEqual([8000, 8001, 8002]);
});

test("accepts a free explicit port and rejects an occupied one", async () => {
expect(await resolveDevPort("A2A", 4567, async () => true, signal)).toEqual({
port: 4567,
requestedPort: 4567,
});
await expect(resolveDevPort("A2A", 4567, async () => false, signal)).rejects.toThrow(
"lsof -i :4567",
);
});

test("bounds the default search", async () => {
await expect(resolveDevPort("HTTP", undefined, async () => false, signal)).rejects.toThrow(
"No free port found in range 8080-8179",
);
});
});
45 changes: 45 additions & 0 deletions src/core/dev/port.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { InputValidationError } from "../../errors";
import type { ProjectRuntime } from "../../projectSchemas/runtime";
import type { PortChecker } from "../../io";

const MAX_PORT_ATTEMPTS = 100;
export const DEV_PORTS = { HTTP: 8080, AGUI: 8080, MCP: 8000, A2A: 9000 } as const;

export type DevPort = {
port: number;
requestedPort: number;
};

function portInUse(port: number, suffix = ""): InputValidationError {
return new InputValidationError(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be simpler to make this function its own error type?

`Port ${port} is already in use. Find the process with ` +
`'lsof -i :${port}' (macOS/Linux) or 'netstat -ano | findstr :${port}' (Windows), ` +
`then stop it${suffix}.`,
);
}

export async function resolveDevPort(
protocol: ProjectRuntime["protocol"],
explicitPort: number | undefined,
checkPort: PortChecker,
signal: AbortSignal,
): Promise<DevPort> {
const defaultPort = DEV_PORTS[protocol ?? "HTTP"];
const requestedPort = explicitPort ?? defaultPort;

if (await checkPort(requestedPort, signal)) {
return { port: requestedPort, requestedPort };
}

if (explicitPort !== undefined) {
throw portInUse(requestedPort, " or choose a different --port");
}

for (let port = requestedPort + 1; port < requestedPort + MAX_PORT_ATTEMPTS; port++) {
if (await checkPort(port, signal)) return { port, requestedPort };
}

throw new InputValidationError(
`No free port found in range ${requestedPort}-${requestedPort + MAX_PORT_ATTEMPTS - 1}.`,
);
}
2 changes: 1 addition & 1 deletion src/errors/errors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export class EmbeddedAssetNotFoundError extends AgentCoreCLIError {
}
}

export class RuntimeInvokeInterruptedError extends AgentCoreCLIError {
export class CommandInterruptedError extends AgentCoreCLIError {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is doing something similar to https://github.com/aws/agentcore-cli/pull/1986/changes (haven't fully reviewed that one yet)

readonly reported: boolean;

constructor(cause?: unknown, reported = false) {
Expand Down
Loading
Loading