diff --git a/src/assets/cdk/package.json b/src/assets/cdk/package.json index 550a52797..0ac28f946 100644 --- a/src/assets/cdk/package.json +++ b/src/assets/cdk/package.json @@ -24,7 +24,7 @@ }, "dependencies": { "@aws/agentcore-cdk": "0.1.0-alpha.45", - "aws-cdk-lib": "~2.261.0", + "aws-cdk-lib": "~2.266.0", "constructs": "~10.7.0" } } diff --git a/src/core/project/envLocal.test.ts b/src/core/project/envLocal.test.ts new file mode 100644 index 000000000..44834806d --- /dev/null +++ b/src/core/project/envLocal.test.ts @@ -0,0 +1,77 @@ +import { afterEach, expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { tmpdir } from "node:os"; +import { parseEnv } from "node:util"; +import { EnvLocalFile } from "./envLocal"; + +const roots: string[] = []; +afterEach(async () => { + await Promise.all(roots.splice(0).map((r) => rm(r, { recursive: true, force: true }))); +}); + +async function tempRoot(): Promise { + const root = await mkdtemp(join(tmpdir(), "envlocal-")); + roots.push(root); + // Real projects always have the agentcore/ dir; the class does not create it. + await mkdir(dirname(new EnvLocalFile(root).path), { recursive: true }); + return root; +} + +const ENTRY = { key: "SECRET", value: "v", comment: "c" }; + +test("rollback deletes the file it created", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await file.insertIfNew([ENTRY]); + expect(existsSync(file.path)).toBe(true); + + await file.rollback(); + expect(existsSync(file.path)).toBe(false); +}); + +test("rollback restores the prior content of an existing file", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await Bun.write(file.path, "EXISTING=1\n"); + + await file.insertIfNew([ENTRY]); + expect(await Bun.file(file.path).text()).toContain("SECRET='v'"); + + await file.rollback(); + expect(await Bun.file(file.path).text()).toBe("EXISTING=1\n"); +}); + +test("rollback is a no-op when insertIfNew wrote nothing", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await Bun.write(file.path, "SECRET=kept\n"); + + await file.insertIfNew([ENTRY]); // key already present, so nothing is written + await file.rollback(); + expect(await Bun.file(file.path).text()).toBe("SECRET=kept\n"); +}); + +test.each([ + ["left#right", "left#right"], + [" padded ", " padded "], + ['has"double', 'has"double'], + ["back\\slash", "back\\slash"], + ["dollar$sign", "dollar$sign"], +])("a value with %p round-trips through parseEnv", async (value, expected) => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await file.insertIfNew([{ key: "SECRET", value, comment: "c" }]); + + const parsed = parseEnv(await Bun.file(file.path).text()) as Record; + expect(parsed.SECRET).toBe(expected); +}); + +test("rejects a value that contains a single quote", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await expect(file.insertIfNew([{ key: "SECRET", value: "a'b", comment: "c" }])).rejects.toThrow( + /single quote/, + ); +}); diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts new file mode 100644 index 000000000..5774dde36 --- /dev/null +++ b/src/core/project/envLocal.ts @@ -0,0 +1,97 @@ +import { rm } from "node:fs/promises"; +import { join } from "node:path"; +import { atomicWrite, readTextFile } from "../../io"; +import { InputValidationError } from "../../errors"; +import type { EnvLocalEntry } from "../../handlers/project/types"; + +/** The project-relative path of the local secrets file (read by `agentcore dev`). */ +export const ENV_LOCAL_RELATIVE_PATH = join("agentcore", ".env.local"); + +const KEY_LINE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/; + +/** + * The project's `.env.local` secrets file, edited transactionally. `insertIfNew` + * appends entries (never overwriting an existing key) and snapshots the prior + * state so `rollback` can undo the write if a later step in the same operation + * fails. Mirrors the class shape of {@link SourceResolver} so callers hold one + * object and reverse its effect, rather than tracking loose paths. + */ +export class EnvLocalFile { + // undefined: no write yet; null: file did not exist before the write; + // string: the file's content before the write. + private snapshot?: string | null; + + constructor(private readonly rootPath: string) {} + + /** The absolute path of the secrets file. */ + get path(): string { + return join(this.rootPath, ENV_LOCAL_RELATIVE_PATH); + } + + /** + * Appends entries, creating the file when missing. Keys that already exist + * are left unchanged so user-managed values survive re-runs. Returns the keys + * written and those skipped. + */ + async insertIfNew(entries: EnvLocalEntry[]): Promise<{ written: string[]; skipped: string[] }> { + const existing = await this.readOrNull(); + const existingKeys = new Set( + (existing ?? "") + .split("\n") + .map((line) => KEY_LINE.exec(line)?.[1]) + .filter((key) => key !== undefined), + ); + + const written: string[] = []; + const skipped: string[] = []; + let content = existing ?? ""; + for (const entry of entries) { + if (existingKeys.has(entry.key)) { + skipped.push(entry.key); + continue; + } + const separator = content === "" || content.endsWith("\n") ? "" : "\n"; + // Each entry is two lines: # \n= + content += `${separator}# ${entry.comment}\n${entry.key}=${formatValue(entry.value)}\n`; + written.push(entry.key); + } + + if (written.length > 0) { + this.snapshot = existing; + await atomicWrite(this.path, content); + } + return { written, skipped }; + } + + /** Restores the file to its pre-write state; a no-op when nothing was written. */ + async rollback(): Promise { + if (this.snapshot === undefined) return; + if (this.snapshot === null) await rm(this.path, { force: true }); + else await atomicWrite(this.path, this.snapshot); + } + + private async readOrNull(): Promise { + try { + return await readTextFile(this.path); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + } + } +} + +/** + * Single-quotes a value so `node:util`'s `parseEnv` reads it back byte-for-byte. + * Single quotes are literal in that parser, so no character needs escaping, + * except a single quote itself, which the format cannot represent. + */ +function formatValue(value?: string): string { + if (!value) return ""; + if (value.includes("'")) { + throw new InputValidationError( + "a secret value that contains a single quote (') cannot be written to " + + ".env.local; supply it with a Secrets Manager reference instead", + ); + } + return `'${value}'`; +} diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 3c3799c54..baea4c650 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -20,6 +20,7 @@ import { type ReadWriteJson, } from "../../io"; import { defaultSource, type AssetSource } from "./source"; +import { ENV_LOCAL_RELATIVE_PATH, EnvLocalFile } from "./envLocal"; import { createHarnessTreeFromSpec, createProjectTreeFromTemplate, TEMPLATES } from "./templates"; import { ProjectSpecSchema, type ManagedBy } from "../../projectSchemas/project"; import { enclosingProjectRoot } from "./fsUtils"; @@ -151,8 +152,11 @@ export class FsProjectManager implements ProjectManager { `a ${resourceType} with name '${resourceConfig.name}' already exists`, ); + // Widened: arms push their own shapes; the whole-spec safeParse below validates. const newResources: unknown[] = [...existingResources]; const scaffoldedPaths: string[] = []; + // Non-file work that a failed spec write must also reverse. + let envFile: EnvLocalFile | undefined; switch (resourceType) { case "harness": { @@ -172,6 +176,22 @@ export class FsProjectManager implements ProjectManager { "runtime case not yet implemented in FsProjectManager.addResource", ); } + case "credential": { + // No file scaffolding; the secret placeholder is staged into .env.local + // and reversed with the spec write if that commit fails. + newResources.push(input.resourceConfig); + if (input.envEntries?.length) { + envFile = new EnvLocalFile(project.rootPath); + yield { message: `Updating secrets file at '${envFile.path}'` }; + const { skipped } = await envFile.insertIfNew(input.envEntries); + for (const key of skipped) { + yield { + message: `'${key}' already exists in ${ENV_LOCAL_RELATIVE_PATH}; left unchanged`, + }; + } + } + break; + } case "config-bundle": case "online-eval": case "online-insight": @@ -186,27 +206,24 @@ export class FsProjectManager implements ProjectManager { yield { message: `Updating project spec file at '${agentCoreSpecPath}'` }; - // rollback scaffolding changes on failed config writes to prevent bad state. + const newSpec = { ...existingProjectSpec, [projectSpecKey]: newResources }; + + // Validate and write inside the same boundary so a rejected spec rolls back + // staged side effects (.env.local, scaffolded files) rather than leaving them. + let newProjectSpec: z.infer; try { - const newSpec = { ...existingProjectSpec, [projectSpecKey]: newResources }; const newSpecParseResult = ProjectSpecSchema.safeParse(newSpec); - if (!newSpecParseResult.success) throw new InputValidationError(z.prettifyError(newSpecParseResult.error), { cause: newSpecParseResult.error, }); - const newProjectSpec = await this.json.write(agentCoreSpecPath, newSpecParseResult.data); - - return { - ...project, - spec: newProjectSpec, - }; + newProjectSpec = await this.json.write(agentCoreSpecPath, newSpecParseResult.data); } catch (err) { this.logger.warn( - `failed to update ${agentCoreSpecPath}; attempting best-effort cleanup of scaffolded files`, + `could not commit the spec update to ${agentCoreSpecPath}; attempting best-effort cleanup of staged changes`, ); - await Promise.all( - scaffoldedPaths.map((p) => + await Promise.all([ + ...scaffoldedPaths.map((p) => rm(p, { recursive: true, force: true }).catch((e) => { const error = AgentCoreCLIError.fromError(e); this.logger @@ -214,9 +231,20 @@ export class FsProjectManager implements ProjectManager { .warn(`failed to clean up ${p}`); }), ), - ); + envFile?.rollback().catch((e) => { + const error = AgentCoreCLIError.fromError(e); + this.logger + .child({ errorName: error.name, errorMessage: error.message }) + .warn(`failed to roll back ${ENV_LOCAL_RELATIVE_PATH}`); + }), + ]); throw err; } + + return { + ...project, + spec: newProjectSpec, + }; } private getProjectSpecPath(project: Project): string { @@ -306,6 +334,8 @@ function toProjectSpecKey(resourceType: ProjectResource) { return "harnesses"; case "runtime": return "runtimes"; + case "credential": + return "credentials"; case "config-bundle": return "configBundles"; case "online-eval": diff --git a/src/handlers/project/add/credentials/api-key/index.ts b/src/handlers/project/add/credentials/api-key/index.ts new file mode 100644 index 000000000..a1f0ae0a6 --- /dev/null +++ b/src/handlers/project/add/credentials/api-key/index.ts @@ -0,0 +1,56 @@ +import z from "zod"; +import { createHandler, flag } from "../../../../../router"; +import { InputValidationError } from "../../../../../errors"; +import { SourceResolver } from "../../../../../io"; +import type { AddProjectResourceConfig } from "../../types"; +import type { EnvLocalEntry } from "../../../types"; +import { addCredentialToProject, credentialEnvVarName, parseExclusiveSecretRef } from "../shared"; + +export const createAddApiKeyCredentialHandler = (config: AddProjectResourceConfig) => + createHandler({ + name: "api-key", + description: "add an API key credential provider to the current project", + flags: [ + flag("name", "the name of the credential provider", z.string().optional()), + flag( + "api-key", + "the API key (file://path or - for stdin; inline values are rejected)", + z.string().optional(), + { sensitive: true }, + ), + flag( + "api-key-secret-reference", + 'external secret reference JSON: {"secretId":"","jsonKey":""}', + z.string().optional(), + ), + ], + handle: async (ctx, flags) => { + if (!flags.name) + throw new InputValidationError("required option '--name ' not specified"); + + const secretRef = parseExclusiveSecretRef( + "api-key-secret-reference", + flags["api-key-secret-reference"], + "api-key", + flags["api-key"], + ); + + const resolver = new SourceResolver({ stdin: config.io.stdin }); + const apiKey = await resolver.resolveSecret("api-key", flags["api-key"]); + + const envEntries: EnvLocalEntry[] = secretRef + ? [] + : [ + { + key: credentialEnvVarName(flags.name), + value: apiKey, + comment: `API key for credential provider '${flags.name}' (set before deploy)`, + }, + ]; + + await addCredentialToProject(ctx, config, { + resourceConfig: { authorizerType: "ApiKeyCredentialProvider", name: flags.name, secretRef }, + envEntries, + }); + }, + }); diff --git a/src/handlers/project/add/credentials/index.ts b/src/handlers/project/add/credentials/index.ts new file mode 100644 index 000000000..179ee66c3 --- /dev/null +++ b/src/handlers/project/add/credentials/index.ts @@ -0,0 +1,14 @@ +import { Router } from "../../../../router"; +import type { AddProjectResourceConfig } from "../types"; +import { createAddApiKeyCredentialHandler } from "./api-key"; +import { createAddOauthCredentialHandler } from "./oauth"; + +export function createAddCredentialsHandler(config: AddProjectResourceConfig): Router { + const credentials = new Router( + "credentials", + "add AgentCore Identity credential providers to the current project", + ); + credentials.handler(createAddApiKeyCredentialHandler(config)); + credentials.handler(createAddOauthCredentialHandler(config)); + return credentials; +} diff --git a/src/handlers/project/add/credentials/oauth/index.ts b/src/handlers/project/add/credentials/oauth/index.ts new file mode 100644 index 000000000..96c71fe58 --- /dev/null +++ b/src/handlers/project/add/credentials/oauth/index.ts @@ -0,0 +1,106 @@ +import z from "zod"; +import type { CredentialProviderVendorType } from "@aws-sdk/client-bedrock-agentcore-control"; +import { createHandler, flag } from "../../../../../router"; +import { InputValidationError } from "../../../../../errors"; +import { SourceResolver } from "../../../../../io"; +import { + parseProviderConfigFlags, + validateProviderConfigMode, +} from "../../../../identity/oauth2-credential-provider/config"; +import type { AddProjectResourceConfig } from "../../types"; +import type { EnvLocalEntry } from "../../../types"; +import { addCredentialToProject, credentialEnvVarName, parseExclusiveSecretRef } from "../shared"; + +export const createAddOauthCredentialHandler = (config: AddProjectResourceConfig) => + createHandler({ + name: "oauth", + description: "add an OAuth2 credential provider to the current project", + flags: [ + flag("name", "the name of the credential provider", z.string().optional()), + flag( + "vendor", + "the OAuth2 vendor (e.g. GithubOauth2); custom providers use the guided flags instead", + z.string().default("CustomOauth2"), + ), + flag("client-id", "OAuth2 client ID (guided custom OAuth2)", z.string().optional()), + flag("discovery-url", "OAuth2 discovery URL (guided custom OAuth2)", z.string().optional()), + flag( + "scopes", + "OAuth2 scopes the provider grants (guided custom OAuth2)", + z.array(z.string()).optional(), + ), + flag( + "provider-configuration", + "complete secret-free Oauth2ProviderConfigInput JSON (required for vendored providers)", + z.string().optional(), + ), + flag( + "client-secret", + "the client secret (file://path or - for stdin; inline values are rejected)", + z.string().optional(), + { sensitive: true }, + ), + flag( + "client-secret-reference", + 'external secret reference JSON: {"secretId":"","jsonKey":""}', + z.string().optional(), + ), + ], + handle: async (ctx, flags) => { + if (!flags.name) + throw new InputValidationError("required option '--name ' not specified"); + + const secretRef = parseExclusiveSecretRef( + "client-secret-reference", + flags["client-secret-reference"], + "client-secret", + flags["client-secret"], + ); + + const mode = parseProviderConfigFlags({ + clientId: flags["client-id"], + discoveryUrl: flags["discovery-url"], + providerConfiguration: flags["provider-configuration"], + }); + validateProviderConfigMode(mode, flags.vendor as CredentialProviderVendorType); + if (mode.kind === "complete" && flags.scopes !== undefined) { + throw new InputValidationError( + "--provider-configuration and --scopes are mutually exclusive", + ); + } + + const resolver = new SourceResolver({ stdin: config.io.stdin }); + const clientSecret = await resolver.resolveSecret("client-secret", flags["client-secret"]); + + const resourceConfig = + mode.kind === "complete" + ? { + authorizerType: "OAuthCredentialProvider" as const, + name: flags.name, + vendor: flags.vendor, + providerConfig: mode.config, + clientSecretRef: secretRef, + } + : { + authorizerType: "OAuthCredentialProvider" as const, + name: flags.name, + vendor: flags.vendor, + clientId: flags["client-id"], + discoveryUrl: flags["discovery-url"], + scopes: flags.scopes, + clientSecretRef: secretRef, + }; + + const envEntries: EnvLocalEntry[] = secretRef + ? [] + : [ + { + key: credentialEnvVarName(flags.name, "_CLIENT_SECRET"), + value: clientSecret, + comment: `OAuth client secret for credential provider '${flags.name}' (set before deploy)`, + }, + ]; + + await addCredentialToProject(ctx, config, { resourceConfig, envEntries }); + }, + }); diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts new file mode 100644 index 000000000..a9a1445c9 --- /dev/null +++ b/src/handlers/project/add/credentials/shared.ts @@ -0,0 +1,60 @@ +import { ProjectKey, type Context } from "../../../../router"; +import { InputValidationError } from "../../../../errors"; +import { parseSecretReference } from "../../../identity/parser"; +import type { AddProjectResourceConfig } from "../types"; +import type { AddResourceInput } from "../../types"; + +/** Derives the .env.local variable name a credential's secret is stored under. */ +export function credentialEnvVarName(credentialName: string, suffix = ""): string { + return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; +} + +/** Parses a secret-reference flag, rejecting a directly supplied secret alongside it. */ +export function parseExclusiveSecretRef( + refFlag: string, + refValue: string | undefined, + secretFlag: string, + secretValue: string | undefined, +) { + if (!refValue) return undefined; + if (secretValue !== undefined) { + throw new InputValidationError(`--${secretFlag} and --${refFlag} are mutually exclusive`); + } + return parseSecretReference(refFlag, refValue); +} + +/** Runs the shared add flow: spec update, env entries, progress, and fill-before-deploy notice. */ +export async function addCredentialToProject( + ctx: Context, + config: AddProjectResourceConfig, + input: Omit, "resourceType">, +): Promise { + const project = ctx.require(ProjectKey); + + // Two names that differ only by '-' vs '_' derive the same environment + // variable, which would silently reuse one secret for both providers. + const newName = input.resourceConfig.name; + const clash = project.spec.credentials.find( + (existing) => + existing.name !== newName && + credentialEnvVarName(existing.name) === credentialEnvVarName(newName), + ); + if (clash) { + throw new InputValidationError( + `credential '${newName}' and '${clash.name}' derive the same environment variable name; ` + + "choose a name that differs by more than '-' and '_'", + ); + } + + for await (const event of config.projectManager.addResource(project, { + resourceType: "credential", + ...input, + })) { + config.io.stderr.write(`${event.message}\n`); + } + + config.io.stderr.write(`added credential '${input.resourceConfig.name}' to '${project.name}'\n`); + for (const entry of (input.envEntries ?? []).filter((e) => e.value === undefined)) { + config.io.stderr.write(`Set ${entry.key} in agentcore/.env.local before you deploy.\n`); + } +} diff --git a/src/handlers/project/add/index.ts b/src/handlers/project/add/index.ts index d934735a9..42f6de230 100644 --- a/src/handlers/project/add/index.ts +++ b/src/handlers/project/add/index.ts @@ -1,6 +1,7 @@ import { withProject } from "../../../middleware/"; import { Router } from "../../../router"; import { createAddConfigBundleHandler } from "./config-bundle"; +import { createAddCredentialsHandler } from "./credentials"; import { createAddHarnessHandler } from "./harness"; import { createAddRuntimeHandler } from "./runtime"; import { createAddOnlineEvalHandler } from "./online-eval"; @@ -15,5 +16,6 @@ export function createAddProjectResourceHandler(config: AddProjectResourceConfig projectAdd.handler(createAddRuntimeHandler(config)); projectAdd.handler(createAddOnlineEvalHandler(config)); projectAdd.handler(createAddOnlineInsightHandler(config)); + projectAdd.handler(createAddCredentialsHandler(config)); return projectAdd; } diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index ced2d9277..dd0ae3dc7 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -12,8 +12,8 @@ import { } from "../../testing"; import { InputValidationError } from "../../errors"; -async function run(args: string[], opts?: { core?: TestCoreClient }) { - const io = testIO(); +async function run(args: string[], opts?: { core?: TestCoreClient; stdin?: string }) { + const io = testIO({ stdin: opts?.stdin }); const core = opts?.core ?? new TestCoreClient(); const root = createRootHandler(core, { io: io.io, @@ -301,6 +301,273 @@ describe("project add config-bundle", () => { }); }); +describe("project add credentials", () => { + test("api-key with a file:// secret records the spec entry and stores the trailing-newline-stripped key in .env.local", async () => { + const projectRoot = await inProject(); + const keyPath = join(projectRoot, "key.txt"); + // The trailing newline mirrors `echo` and editor output; it must not reach the value. + await Bun.write(keyPath, "sk-123\n"); + + await run([ + "add", + "credentials", + "api-key", + "--name", + "svc-key", + "--api-key", + `file://${keyPath}`, + ]); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { authorizerType: "ApiKeyCredentialProvider", name: "svc-key" }, + ]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY='sk-123'\n"); + expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY='sk-123'\n\n"); + }); + + test("api-key without a secret writes a commented placeholder and tells the user to fill it", async () => { + const projectRoot = await inProject(); + const { io } = await run(["add", "credentials", "api-key", "--name", "svc-key"]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("# API key for credential provider 'svc-key' (set before deploy)"); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=\n"); + expect(io.stderr()).toContain( + "Set AGENTCORE_CREDENTIAL_SVC_KEY in agentcore/.env.local before you deploy", + ); + }); + + test("api-key with an external secret reference records it in the spec and skips .env.local", async () => { + const projectRoot = await inProject(); + const secretRef = { + secretId: "arn:aws:secretsmanager:us-west-2:123456789012:secret:s", + jsonKey: "apiKey", + }; + + await run([ + "add", + "credentials", + "api-key", + "--name", + "svc-key", + "--api-key-secret-reference", + JSON.stringify(secretRef), + ]); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { authorizerType: "ApiKeyCredentialProvider", name: "svc-key", secretRef }, + ]); + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY"); + }); + + const discoveryUrl = "https://idp.example.com/.well-known/openid-configuration"; + + test("oauth custom with guided flags and a stdin secret records the spec entry and the secret", async () => { + const projectRoot = await inProject(); + + await run( + [ + "add", + "credentials", + "oauth", + "--name", + "idp", + "--discovery-url", + discoveryUrl, + "--client-id", + "client-1", + "--scopes", + "openid", + "email", + "--client-secret", + "-", + ], + { stdin: "sssh" }, + ); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + vendor: "CustomOauth2", + clientId: "client-1", + discoveryUrl, + scopes: ["openid", "email"], + }, + ]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_IDP_CLIENT_SECRET='sssh'"); + }); + + test("oauth vendored with --provider-configuration records the config and a secret placeholder", async () => { + const projectRoot = await inProject(); + + const { io } = await run([ + "add", + "credentials", + "oauth", + "--name", + "github", + "--vendor", + "GithubOauth2", + "--provider-configuration", + '{"githubOauth2ProviderConfig":{"clientId":"client-1"}}', + ]); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { githubOauth2ProviderConfig: { clientId: "client-1" } }, + }, + ]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_GITHUB_CLIENT_SECRET=\n"); + expect(io.stderr()).toContain( + "Set AGENTCORE_CREDENTIAL_GITHUB_CLIENT_SECRET in agentcore/.env.local before you deploy", + ); + }); + + test("preserves existing .env.local content and never overwrites an existing key", async () => { + const projectRoot = await inProject(); + const envPath = join(projectRoot, "agentcore", ".env.local"); + const original = await Bun.file(envPath).text(); + await Bun.write(envPath, `${original}AGENTCORE_CREDENTIAL_SVC_KEY=user-managed\n`); + + const { io } = await run(["add", "credentials", "api-key", "--name", "svc-key"]); + + const env = await Bun.file(envPath).text(); + expect(env).toStartWith(original); + expect(env.match(/AGENTCORE_CREDENTIAL_SVC_KEY=/g)).toHaveLength(1); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=user-managed"); + expect(io.stderr()).toContain("already exists"); + }); + + test("creates .env.local when the project lacks one", async () => { + const projectRoot = await inProject(); + const envPath = join(projectRoot, "agentcore", ".env.local"); + await rm(envPath); + + await run(["add", "credentials", "api-key", "--name", "svc-key"]); + + const env = await Bun.file(envPath).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=\n"); + }); + + test("rejects a duplicate credential name across credential types", async () => { + await inProject(); + await run(["add", "credentials", "api-key", "--name", "dup"]); + await expect( + run(["add", "credentials", "oauth", "--name", "dup", "--discovery-url", discoveryUrl]), + ).rejects.toThrow(/already exists/); + }); + + test("rejects two names that derive the same environment variable", async () => { + await inProject(); + await run(["add", "credentials", "api-key", "--name", "svc-key"]); + await expect(run(["add", "credentials", "api-key", "--name", "svc_key"])).rejects.toThrow( + /same environment variable/, + ); + }); + + test.each<[string, string[], RegExp]>([ + [ + "api-key: an inline secret value", + ["api-key", "--name", "x", "--api-key", "sk-inline"], + /file:\/\//, + ], + ["api-key: a multi-line secret", ["api-key", "--name", "x", "--api-key", "-"], /single-line/], + [ + "oauth: an inline secret value", + ["oauth", "--name", "x", "--discovery-url", discoveryUrl, "--client-secret", "sssh"], + /file:\/\//, + ], + [ + "api-key: a secret combined with a secret reference", + [ + "api-key", + "--name", + "x", + "--api-key", + "-", + "--api-key-secret-reference", + '{"secretId":"arn:aws:secretsmanager:us-west-2:123:secret:s","jsonKey":"apiKey"}', + ], + /mutually exclusive/, + ], + [ + "oauth: a secret combined with a secret reference", + [ + "oauth", + "--name", + "x", + "--discovery-url", + discoveryUrl, + "--client-secret", + "-", + "--client-secret-reference", + '{"secretId":"arn:aws:secretsmanager:us-west-2:123:secret:s","jsonKey":"clientSecret"}', + ], + /mutually exclusive/, + ], + ["api-key: a missing --name", ["api-key"], /--name/], + ["oauth: a missing --name", ["oauth"], /--name/], + [ + "oauth: a vendored provider without --provider-configuration", + ["oauth", "--name", "x", "--vendor", "GithubOauth2"], + /--provider-configuration/, + ], + [ + "oauth: a guided custom provider without --discovery-url", + ["oauth", "--name", "x", "--client-id", "c"], + /--discovery-url/, + ], + [ + "oauth: --provider-configuration combined with --scopes", + [ + "oauth", + "--name", + "x", + "--vendor", + "GithubOauth2", + "--provider-configuration", + '{"githubOauth2ProviderConfig":{"clientId":"c"}}', + "--scopes", + "repo", + ], + /mutually exclusive/, + ], + [ + "oauth: secret material inside --provider-configuration", + [ + "oauth", + "--name", + "x", + "--vendor", + "GithubOauth2", + "--provider-configuration", + '{"githubOauth2ProviderConfig":{"clientId":"c","clientSecret":"sssh"}}', + ], + /secret material/, + ], + ])("rejects %s", async (_label, args, message) => { + await inProject(); + await expect(run(["add", "credentials", ...args], { stdin: "line1\nline2" })).rejects.toThrow( + message, + ); + }); +}); + describe("project build", () => { async function inBuildableProject(): Promise { const projectRoot = await inProject("MyAgent"); diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index a62e11e1f..625b9ec69 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -1,4 +1,5 @@ import { HarnessSpecSchema } from "../../projectSchemas/harness"; +import type { CredentialSchema } from "../../projectSchemas/credential"; import type { ConfigBundleSchema } from "../../projectSchemas/config-bundle"; import type { ProjectSpecSchema } from "../../projectSchemas/project"; import z from "zod"; @@ -49,6 +50,14 @@ export type Project = { spec: z.infer; }; +/** A line to add to agentcore/.env.local. Secret values travel here, never in the spec. */ +export type EnvLocalEntry = { + key: string; + /** An omitted value writes an empty placeholder the user fills before deploy. */ + value?: string; + comment: string; +}; + /** Discriminated union input for {@link ProjectManager.addResource}. */ export type AddResourceInput = | { @@ -59,6 +68,11 @@ export type AddResourceInput = resourceType: "runtime"; resourceConfig: RuntimeResourceConfig; } + | { + resourceType: "credential"; + resourceConfig: z.input; + envEntries?: EnvLocalEntry[]; + } | { resourceType: "config-bundle"; resourceConfig: z.input; diff --git a/src/io/fileRead.ts b/src/io/fileRead.ts index 804532cea..8d4a14f4e 100644 --- a/src/io/fileRead.ts +++ b/src/io/fileRead.ts @@ -1,3 +1,5 @@ +import { readFile } from "node:fs/promises"; + export type ReadTextFileOptions = { signal?: AbortSignal; }; @@ -6,8 +8,5 @@ export async function readTextFile( path: string, options: ReadTextFileOptions = {}, ): Promise { - options.signal?.throwIfAborted(); - const text = await Bun.file(path).text(); - options.signal?.throwIfAborted(); - return text; + return readFile(path, { encoding: "utf8", signal: options.signal }); } diff --git a/src/io/source.ts b/src/io/source.ts index 60ad1e952..69bae8401 100644 --- a/src/io/source.ts +++ b/src/io/source.ts @@ -45,6 +45,28 @@ export class SourceResolver { } } + /** + * Resolves a secret-bearing flag while refusing inline values: an inline + * secret leaks into shell history and process listings, so only stdin and + * files are allowed. A single trailing newline is stripped (echo and editors + * add one) and an embedded newline is rejected. + */ + async resolveSecret(name: string, source: string | undefined): Promise { + if (source !== undefined && source !== STDIN && !source.startsWith(FILE_PREFIX)) { + throw new SourceResolutionError( + `--${name} must come from stdin ('-') or a file ('file://'); ` + + "inline secret values are not accepted", + ); + } + const value = await this.resolveText(name, source); + if (value === undefined) return undefined; + const normalized = value.replace(/\r?\n$/, ""); + if (normalized.includes("\n")) { + throw new SourceResolutionError(`--${name} must be a single-line value`); + } + return normalized; + } + private async readStdin(name: string): Promise { if (this.stdinClaimedBy !== undefined) { throw new SourceResolutionError( diff --git a/src/projectSchemas/credential.test.ts b/src/projectSchemas/credential.test.ts new file mode 100644 index 000000000..c176ecb4e --- /dev/null +++ b/src/projectSchemas/credential.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, it } from "bun:test"; +import { CredentialSchema } from "./credential"; + +const DISCOVERY_URL = "https://idp.example.com/.well-known/openid-configuration"; +const SECRET_REF = { + secretId: "arn:aws:secretsmanager:us-west-2:123456789012:secret:s", + jsonKey: "value", +}; + +describe("credential schema", () => { + it.each<[string, Record]>([ + ["an api-key credential", { authorizerType: "ApiKeyCredentialProvider", name: "svc-key" }], + [ + "an api-key credential with an external secret reference", + { authorizerType: "ApiKeyCredentialProvider", name: "svc-key", secretRef: SECRET_REF }, + ], + [ + "a guided custom OAuth credential", + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + vendor: "CustomOauth2", + clientId: "client-1", + discoveryUrl: DISCOVERY_URL, + scopes: ["openid"], + managed: true, + }, + ], + [ + "a custom OAuth credential with a complete provider config", + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + vendor: "CustomOauth2", + providerConfig: { + customOauth2ProviderConfig: { + clientId: "client-1", + oauthDiscovery: { discoveryUrl: DISCOVERY_URL }, + }, + }, + }, + ], + [ + "a vendored OAuth credential with an external client secret reference", + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { githubOauth2ProviderConfig: { clientId: "client-1" } }, + clientSecretRef: SECRET_REF, + }, + ], + ])("accepts %s and retains its fields", (_label, value) => { + const result = CredentialSchema.safeParse(value); + expect(result.success).toBe(true); + expect(result.data).toMatchObject(value); + }); + + it("defaults the OAuth vendor to CustomOauth2", () => { + const result = CredentialSchema.safeParse({ + authorizerType: "OAuthCredentialProvider", + name: "idp", + clientId: "client-1", + discoveryUrl: DISCOVERY_URL, + }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ vendor: "CustomOauth2" }); + }); + + it.each<[string, Record, RegExp]>([ + [ + "a vendored OAuth credential without a provider config", + { authorizerType: "OAuthCredentialProvider", name: "github", vendor: "GithubOauth2" }, + /providerConfig/, + ], + [ + "a guided custom OAuth credential without a discovery URL", + { authorizerType: "OAuthCredentialProvider", name: "idp", clientId: "client-1" }, + /discoveryUrl/, + ], + [ + "a provider config combined with guided fields", + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + discoveryUrl: DISCOVERY_URL, + providerConfig: { customOauth2ProviderConfig: {} }, + }, + /mutually exclusive/, + ], + [ + "a provider config with a nested clientSecret", + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { + githubOauth2ProviderConfig: { clientId: "client-1", clientSecret: "sssh" }, + }, + }, + /secret material.*clientSecret/, + ], + [ + "a provider config with a nested apiKey", + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { githubOauth2ProviderConfig: { nested: { apiKey: "sssh" } } }, + }, + /secret material.*apiKey/, + ], + [ + "a secret reference with unexpected fields", + { + authorizerType: "ApiKeyCredentialProvider", + name: "svc-key", + secretRef: { ...SECRET_REF, extra: "bad" }, + }, + /secretRef/, + ], + [ + "an invalid credential name", + { authorizerType: "ApiKeyCredentialProvider", name: "bad name!" }, + /alphanumeric/, + ], + [ + "a credential name shorter than 3 characters", + { authorizerType: "ApiKeyCredentialProvider", name: "ab" }, + /3 characters/, + ], + ])("rejects %s", (_label, value, message) => { + const result = CredentialSchema.safeParse(value); + expect(result.success).toBe(false); + expect(JSON.stringify(result.error?.issues)).toMatch(message); + }); +}); diff --git a/src/projectSchemas/credential.ts b/src/projectSchemas/credential.ts index d4bce69a3..49db8a4d6 100644 --- a/src/projectSchemas/credential.ts +++ b/src/projectSchemas/credential.ts @@ -1,8 +1,12 @@ import z from "zod"; import { PaymentProviderSchema } from "./payment"; +// Max length and character set are the Identity service's own limits. The min +// is temporarily raised above 1 so `add` never produces a name that the pinned +// @aws/agentcore-cdk rejects at `build`; drop it back to 1 once a release that +// aligns the two ships. See credentials-add-followups. export const CredentialNameSchema = z .string() - .min(1, "Credential name is required") + .min(3, "Credential name must be at least 3 characters") .max(128, "Credential name must be 128 characters or less") .regex( /^[a-zA-Z0-9\-_]+$/, @@ -14,20 +18,88 @@ export const CredentialTypeSchema = z.enum([ "PaymentCredentialProvider", ]); export type CredentialType = z.infer; +/** A reference to a secret the customer already keeps in AWS Secrets Manager. */ +export const SecretReferenceSchema = z + .object({ + secretId: z.string().min(1), + jsonKey: z.string().min(1), + }) + .strict(); +export type SecretReference = z.infer; export const ApiKeyCredentialSchema = z.object({ authorizerType: z.literal("ApiKeyCredentialProvider"), name: CredentialNameSchema, + /** External Secrets Manager reference; when absent the key comes from .env.local. */ + secretRef: SecretReferenceSchema.optional(), }); export type ApiKeyCredential = z.infer; -export const OAuthCredentialSchema = z.object({ - authorizerType: z.literal("OAuthCredentialProvider"), - name: CredentialNameSchema, - discoveryUrl: z.string().url().optional(), - scopes: z.array(z.string()).optional(), - vendor: z.string().default("CustomOauth2"), - managed: z.boolean().optional(), - usage: z.enum(["inbound", "outbound"]).optional(), -}); +const CUSTOM_OAUTH_VENDOR = "CustomOauth2"; +// Secret values never belong in agentcore.json; they travel via .env.local or +// Secrets Manager references. These key names match the SDK's inline-secret fields. +const SECRET_MATERIAL_KEYS = new Set(["clientSecret", "apiKey"]); +function findSecretMaterialKey(value: unknown): string | undefined { + if (typeof value !== "object" || value === null) return undefined; + for (const [key, nested] of Object.entries(value)) { + if (SECRET_MATERIAL_KEYS.has(key)) return key; + const found = findSecretMaterialKey(nested); + if (found) return found; + } + return undefined; +} +export const OAuthCredentialSchema = z + .object({ + authorizerType: z.literal("OAuthCredentialProvider"), + name: CredentialNameSchema, + /** Credential provider vendor (free-form to track the service without CLI releases). */ + vendor: z.string().default(CUSTOM_OAUTH_VENDOR), + /** Guided custom OAuth fields. */ + clientId: z.string().optional(), + discoveryUrl: z.string().url().optional(), + scopes: z.array(z.string()).optional(), + /** Complete Oauth2ProviderConfigInput (secret-free) for vendored providers. */ + providerConfig: z.record(z.string(), z.unknown()).optional(), + /** External Secrets Manager reference; when absent the secret comes from .env.local. */ + clientSecretRef: SecretReferenceSchema.optional(), + /** Whether this credential was auto-created by the CLI (e.g. for CUSTOM_JWT inbound auth). */ + managed: z.boolean().optional(), + }) + .superRefine((credential, ctx) => { + const hasGuidedFields = + credential.clientId !== undefined || + credential.discoveryUrl !== undefined || + credential.scopes !== undefined; + if (credential.providerConfig !== undefined && hasGuidedFields) { + ctx.addIssue({ + code: "custom", + message: + "providerConfig and the guided fields (clientId, discoveryUrl, scopes) are mutually exclusive", + }); + return; + } + if (credential.providerConfig === undefined) { + if (credential.vendor !== CUSTOM_OAUTH_VENDOR) { + ctx.addIssue({ + code: "custom", + message: `vendor "${credential.vendor}" requires providerConfig; guided fields only support ${CUSTOM_OAUTH_VENDOR}`, + }); + } else if (credential.discoveryUrl === undefined) { + ctx.addIssue({ + code: "custom", + message: `guided ${CUSTOM_OAUTH_VENDOR} requires discoveryUrl`, + }); + } + return; + } + const secretKey = findSecretMaterialKey(credential.providerConfig); + if (secretKey) { + ctx.addIssue({ + code: "custom", + message: + `providerConfig must not contain secret material (found "${secretKey}"). ` + + "Provide secrets via --client-secret, a secret reference, or .env.local.", + }); + } + }); export type OAuthCredential = z.infer; export const PaymentCredentialSchema = z.object({ authorizerType: z.literal("PaymentCredentialProvider"), diff --git a/src/testing/testIO.tsx b/src/testing/testIO.tsx index 48b799fee..527c32a57 100644 --- a/src/testing/testIO.tsx +++ b/src/testing/testIO.tsx @@ -16,6 +16,9 @@ export interface TestIO { export interface TestIOOptions { isTTY?: boolean; + // stdin pre-fills the input stream with piped content, then ends it. Use this + // to test handlers that read a secret or prompt from stdin ('-'). + stdin?: string; } // collect wraps a PassThrough, accumulating everything written to it as a string. @@ -33,10 +36,12 @@ function collect(): { stream: NodeJS.WriteStream; read: () => string } { // testIO builds a fresh in-memory TestIO for a single test. stdin is an idle // PassThrough (no input) so screens that read input simply see nothing. -export function testIO({ isTTY = false }: TestIOOptions = {}): TestIO { +export function testIO({ isTTY = false, stdin: stdinContent }: TestIOOptions = {}): TestIO { const out = collect(); const err = collect(); - const stdin = new PassThrough() as unknown as NodeJS.ReadStream; + const stdinStream = new PassThrough(); + if (stdinContent !== undefined) stdinStream.end(stdinContent); + const stdin = stdinStream as unknown as NodeJS.ReadStream; for (const stream of [stdin, out.stream, err.stream]) { Object.defineProperty(stream, "isTTY", { configurable: true, value: isTTY });