diff --git a/src/core/identity.tsx b/src/core/identity.tsx index c52f1c3d3..0ab2448d8 100644 --- a/src/core/identity.tsx +++ b/src/core/identity.tsx @@ -1,14 +1,18 @@ import { CreateApiKeyCredentialProviderCommand, CreateOauth2CredentialProviderCommand, + CreatePaymentCredentialProviderCommand, DeleteApiKeyCredentialProviderCommand, DeleteOauth2CredentialProviderCommand, + DeletePaymentCredentialProviderCommand, GetApiKeyCredentialProviderCommand, GetOauth2CredentialProviderCommand, + GetPaymentCredentialProviderCommand, ListApiKeyCredentialProvidersCommand, ListOauth2CredentialProvidersCommand, UpdateApiKeyCredentialProviderCommand, UpdateOauth2CredentialProviderCommand, + UpdatePaymentCredentialProviderCommand, type CreateApiKeyCredentialProviderResponse, type CreateOauth2CredentialProviderResponse, type DeleteApiKeyCredentialProviderResponse, @@ -19,19 +23,33 @@ import { type ListOauth2CredentialProvidersResponse, type UpdateApiKeyCredentialProviderResponse, type UpdateOauth2CredentialProviderResponse, + type CreatePaymentCredentialProviderResponse, + type DeletePaymentCredentialProviderResponse, + type GetPaymentCredentialProviderResponse, + type UpdatePaymentCredentialProviderResponse, } from "@aws-sdk/client-bedrock-agentcore-control"; import type { CoreIdentityClient, CreateApiKeyCredentialProviderInput, CreateOauth2CredentialProviderInput, + CreatePaymentCredentialProviderInput, UpdateApiKeyCredentialProviderInput, UpdateOauth2CredentialProviderInput, + UpdatePaymentCredentialProviderInput, } from "../handlers/identity/types"; +import { createControlClient } from "./factories"; import type { AwsClients, CoreOptions } from "./types"; import { toClientConfig } from "./utils"; +// createIdentityClient builds an IdentityClient that owns its control-plane client, +// for callers constructed outside CoreClient (which hands out its cached ones). +export const createIdentityClient = (): IdentityClient => + new IdentityClient({ control: createControlClient }); + export class IdentityClient implements CoreIdentityClient { - constructor(private readonly clients: AwsClients) {} + // Only the control plane is used, so the dependency is narrowed to it: CoreClient + // still satisfies this by passing itself. + constructor(private readonly clients: Pick) {} async createApiKeyCredentialProvider( input: CreateApiKeyCredentialProviderInput, @@ -124,4 +142,40 @@ export class IdentityClient implements CoreIdentityClient { .control(toClientConfig(options)) .send(new DeleteOauth2CredentialProviderCommand({ name })); } + + async createPaymentCredentialProvider( + input: CreatePaymentCredentialProviderInput, + options: CoreOptions, + ): Promise { + return this.clients + .control(toClientConfig(options)) + .send(new CreatePaymentCredentialProviderCommand(input)); + } + + async getPaymentCredentialProvider( + name: string, + options: CoreOptions, + ): Promise { + return this.clients + .control(toClientConfig(options)) + .send(new GetPaymentCredentialProviderCommand({ name })); + } + + async updatePaymentCredentialProvider( + input: UpdatePaymentCredentialProviderInput, + options: CoreOptions, + ): Promise { + return this.clients + .control(toClientConfig(options)) + .send(new UpdatePaymentCredentialProviderCommand(input)); + } + + async deletePaymentCredentialProvider( + name: string, + options: CoreOptions, + ): Promise { + return this.clients + .control(toClientConfig(options)) + .send(new DeletePaymentCredentialProviderCommand({ name })); + } } diff --git a/src/core/index.tsx b/src/core/index.tsx index 5a6bab9fb..01d4f5989 100644 --- a/src/core/index.tsx +++ b/src/core/index.tsx @@ -109,6 +109,9 @@ export class CoreClient implements AwsClients { this.projectManager = new FsProjectManager({ logger: this.logger.child({ module: "projectManager" }), createCloudFormationClient: config.createCloudFormationClient, + // A project deploy provisions credential providers through the same Identity + // client the `agentcore identity` commands use, against its target's credentials. + identity: this.identity, }); this.describeBedrockAgent = config.describeBedrockAgent ?? describeBedrockAgent; } @@ -164,6 +167,27 @@ export class CoreClient implements AwsClients { // cacheKey derives a stable cache key from a ClientConfig so that distinct // configurations (region, endpoint, ...) map to distinct cached clients. +// +// `credentials` is a provider function or an object of resolved credentials, so it +// cannot be serialized — JSON.stringify drops functions silently, which would map two +// callers with different credentials in the same region onto one cached client. It is +// keyed by identity instead. function cacheKey(config: ClientConfig): string { - return JSON.stringify(config); + const { credentials, ...serializable } = config; + const suffix = credentials ? `|credentials:${credentialsId(credentials)}` : ""; + return JSON.stringify(serializable) + suffix; +} + +const credentialsIds = new WeakMap(); +let nextCredentialsId = 0; + +// credentialsId assigns each credential source a stable id for the lifetime of the +// object, so the same source reuses its client and a different one gets its own. +function credentialsId(credentials: NonNullable): number { + let id = credentialsIds.get(credentials); + if (id === undefined) { + id = nextCredentialsId++; + credentialsIds.set(credentials, id); + } + return id; } diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 33f509877..1c732371d 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -1,5 +1,4 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { existsSync } from "node:fs"; import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; @@ -9,6 +8,11 @@ import { FsReadWriteJson } from "../../../io"; import { ProjectSpecSchema } from "../../../projectSchemas/project"; import { createSilentLogger } from "../../../testing"; import { CdkBackend } from "./cdk"; +import type { + CredentialProviderCalls, + CredentialProvisioner, + PaymentCredentialRemover, +} from "./cdk/credentials"; import { DEPLOYED_STATE_RELATIVE_PATH, updateTargetState } from "./cdk/deployedState"; import type { DeployBackendInput } from "./types"; import type { BootstrapState } from "./cdk/environment"; @@ -26,6 +30,31 @@ const json = new FsReadWriteJson({ logger: createSilentLogger() }); /** A template holding only what CDK adds itself, as an empty project synthesizes. */ const METADATA_ONLY = { CDKMetadata: { Type: "AWS::CDK::Metadata" } }; +/** + * Identity for backends whose provisioning is not under test: these projects declare + * no credentials, and the tests that do exercise provisioning inject their own + * CredentialProvisioner. Any call here is a test that stopped meaning what it says. + */ +function unusedIdentity(): CredentialProviderCalls { + const unexpected = (call: string) => async (): Promise => { + throw new Error(`unexpected Identity call: ${call}`); + }; + return { + getApiKeyCredentialProvider: unexpected("getApiKeyCredentialProvider"), + createApiKeyCredentialProvider: unexpected("createApiKeyCredentialProvider"), + updateApiKeyCredentialProvider: unexpected("updateApiKeyCredentialProvider"), + getOauth2CredentialProvider: unexpected("getOauth2CredentialProvider"), + createOauth2CredentialProvider: unexpected("createOauth2CredentialProvider"), + updateOauth2CredentialProvider: unexpected("updateOauth2CredentialProvider"), + getPaymentCredentialProvider: unexpected("getPaymentCredentialProvider"), + createPaymentCredentialProvider: unexpected("createPaymentCredentialProvider"), + updatePaymentCredentialProvider: unexpected("updatePaymentCredentialProvider"), + deleteApiKeyCredentialProvider: unexpected("deleteApiKeyCredentialProvider"), + deleteOauth2CredentialProvider: unexpected("deleteOauth2CredentialProvider"), + deletePaymentCredentialProvider: unexpected("deletePaymentCredentialProvider"), + }; +} + function deployInput(overrides: Partial = {}): DeployBackendInput { return { target: TARGET, confirmTeardown: async () => false, ...overrides }; } @@ -125,6 +154,8 @@ type HarnessOptions = { template?: boolean; failOperation?: CdkOperation["kind"]; bootstrapError?: Error; + provisionCredentials?: CredentialProvisioner; + removePaymentCredentials?: PaymentCredentialRemover; /** Stack returned by CloudFormation. Defaults to a present stack; null means absent. */ describedStack?: Stack | null; }; @@ -148,6 +179,7 @@ function harness(options: HarnessOptions = {}) { const backend = new CdkBackend({ logger: createSilentLogger(), + identity: unusedIdentity(), runner: async (command, { cwd }) => { commands.push({ command, cwd }); }, @@ -196,6 +228,10 @@ function harness(options: HarnessOptions = {}) { }, }; }, + ...(options.provisionCredentials && { provisionCredentials: options.provisionCredentials }), + ...(options.removePaymentCredentials && { + removePaymentCredentials: options.removePaymentCredentials, + }), describeStack: async (region, provider, stackName) => { stackReads.push({ stackName, region, credentials: provider }); if (options.describedStack === null) return undefined; @@ -265,6 +301,7 @@ describe("CdkBackend.build", () => { const input = await project(); const subject = new CdkBackend({ logger: createSilentLogger(), + identity: unusedIdentity(), runner: async () => { throw new Error("cdk synth exploded"); }, @@ -321,18 +358,57 @@ describe("CdkBackend.deploy", () => { await collectDeploy(subject.backend.deploy(input, deployInput())); + const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); + expect(JSON.parse(await Bun.file(statePath).text())).toEqual({ + targets: { + default: { + resources: { credentials: {} }, + stackArn: + "arn:aws:cloudformation:us-east-1:111122223333:stack/AgentCore-example-default/abc", + }, + }, + }); + }); + + test("provisions credentials before synth and records them under the target", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name]); + const provisionCredentials: CredentialProvisioner = async function* () { + yield { message: "Preparing credential provider 'openai-key'" }; + return { "openai-key": { credentialProviderArn: "arn:apikey:openai-key" } }; + }; + const subject = harness({ + outputs: { RuntimeArn: "arn:runtime" }, + stackArn: "arn:aws:cloudformation:us-east-1:111122223333:stack/AgentCore-example-default/abc", + provisionCredentials, + }); + + const deployed = await collectDeploy(subject.backend.deploy(input, deployInput())); + + // The credential step runs (and its ARNs are recorded) before synthesis, so + // the assembly is synthesized against a state file that already describes them. + const messages = deployed.events.map((event) => event.message); + expect(messages.indexOf("Preparing credential provider 'openai-key'")).toBeLessThan( + messages.indexOf("Synthesizing CloudFormation templates"), + ); + + // The pre-synth credentials write and the post-deploy stack-ARN write merge + // into one target entry rather than clobbering each other. const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); expect(JSON.parse(await Bun.file(statePath).text())).toEqual({ targets: { default: { stackArn: "arn:aws:cloudformation:us-east-1:111122223333:stack/AgentCore-example-default/abc", + resources: { + credentials: { "openai-key": { credentialProviderArn: "arn:apikey:openai-key" } }, + }, }, }, }); }); - test("fails a deploy whose result carries no stack ARN, recording nothing", async () => { + test("fails a deploy whose result carries no stack ARN, recording no binding", async () => { const input = await project(); await writeAssembly(input, [TARGET.name]); const subject = harness({ outputs: { RuntimeArn: "arn:runtime" }, omitStackArn: true }); @@ -340,7 +416,28 @@ describe("CdkBackend.deploy", () => { await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( /without a stack ARN/, ); - expect(existsSync(join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH))).toBe(false); + // The pre-synth credentials write may have created the file, but the failed + // deploy must not have recorded a stack binding. + const state = JSON.parse( + await Bun.file(join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH)).text(), + ); + expect(state.targets.default?.stackArn).toBeUndefined(); + }); + + test("checks local CDK prerequisites before provisioning credentials", async () => { + const input = await project(false); // no agentcore/cdk/node_modules + let provisioned = false; + // eslint-disable-next-line require-yield -- a spy that should never run (deploy fails first) + const provisionCredentials: CredentialProvisioner = async function* () { + provisioned = true; + return {}; + }; + const subject = harness({ provisionCredentials }); + + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( + /npm install/, + ); + expect(provisioned).toBe(false); }); test("fails before touching AWS when the existing state file is malformed", async () => { @@ -459,6 +556,28 @@ describe("CdkBackend.deploy", () => { }); }); + test("removes the project's payment credential providers after its stack", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY }); + const removals: string[] = []; + const removePaymentCredentials: PaymentCredentialRemover = async function* (project) { + removals.push(project.name); + yield { message: "Removing credential provider 'wallet'" }; + }; + const subject = harness({ removePaymentCredentials }); + + const deployed = await collectDeploy( + subject.backend.deploy(input, deployInput({ confirmTeardown: async () => true })), + ); + + expect(removals).toEqual(["example"]); + // After the destroy, since a resource in the stack may still be using it. + const messages = deployed.events.map((event) => event.message); + expect(messages.indexOf("Removing stack AgentCore-example-default-0")).toBeLessThan( + messages.indexOf("Removing credential provider 'wallet'"), + ); + }); + test("says to add a resource when there is no stack to remove either", async () => { const input = await project(); await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY }); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index 78b5cac8d..8ba12fc46 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -24,6 +24,13 @@ import type { } from "./types"; import { createCloudFormationClient } from "../../factories"; import type { CreateCloudFormationClient } from "../../types"; +import { + createCredentialProvisioner, + createPaymentCredentialRemover, + type CredentialProviderCalls, + type CredentialProvisioner, + type PaymentCredentialRemover, +} from "./cdk/credentials"; import { countDeployableResources, stackArtifactForTarget, @@ -70,11 +77,15 @@ export type CdkBackendConfig = { checkTool?: typeof requireTool; json?: ReadWriteJson; createCloudFormationClient?: CreateCloudFormationClient; + /** Identity client used to provision the project's credential providers. */ + identity: CredentialProviderCalls; cdk?: CdkRunner; resolveCredentials?: CdkCredentialResolver; bootstrap?: BootstrapProbe; resolveAccount?: AccountResolver; loadBootstrapTemplate?: BootstrapTemplateLoader; + provisionCredentials?: CredentialProvisioner; + removePaymentCredentials?: PaymentCredentialRemover; describeStack?: StackDescriber; }; @@ -89,6 +100,8 @@ export class CdkBackend implements ProjectBackend { private readonly bootstrap: BootstrapProbe; private readonly resolveAccount: AccountResolver; private readonly loadBootstrapTemplate: BootstrapTemplateLoader; + private readonly provisionCredentials: CredentialProvisioner; + private readonly removePaymentCredentials: PaymentCredentialRemover; private readonly describeStack: StackDescriber; constructor(config: CdkBackendConfig) { @@ -108,6 +121,10 @@ export class CdkBackend implements ProjectBackend { ((region, credentials) => probeBootstrap(region, credentials, readBootstrapStack)); this.resolveAccount = config.resolveAccount ?? resolveAwsAccount; this.loadBootstrapTemplate = config.loadBootstrapTemplate ?? loadBootstrapTemplate; + this.provisionCredentials = + config.provisionCredentials ?? createCredentialProvisioner(config.identity); + this.removePaymentCredentials = + config.removePaymentCredentials ?? createPaymentCredentialRemover(config.identity); this.describeStack = config.describeStack ?? ((region, credentials, stackName) => @@ -116,9 +133,10 @@ export class CdkBackend implements ProjectBackend { )); } - public async *build(project: Project): AsyncGenerator { + // Local prerequisites for synth. Checked before any AWS mutation so a missing + // toolchain or dependencies fails without having provisioned credentials. + private async ensureCdkDependencies(project: Project): Promise { const cdkDir = this.cdkDirectory(project); - if (!existsSync(join(cdkDir, "node_modules"))) { throw new ProjectStateError( `CDK dependencies are missing for project '${project.name}'. ` + @@ -126,12 +144,16 @@ export class CdkBackend implements ProjectBackend { ); } await this.checkTool("npm", "Install Node.js: https://nodejs.org/"); + } + + public async *build(project: Project): AsyncGenerator { + await this.ensureCdkDependencies(project); yield { message: "Synthesizing CloudFormation templates" }; await this.runner( ["npm", "run", "cdk", "--", "synth", "--quiet", "--output", this.assemblyDirectory(project)], { - cwd: cdkDir, + cwd: this.cdkDirectory(project), onOutput: (chunk) => this.logger.debug(chunk), }, ); @@ -145,11 +167,24 @@ export class CdkBackend implements ProjectBackend { yield { message: `Verifying AWS account ${target.account}` }; const credentials = await this.credentialsForTarget(target); - // Validate any existing deployed state before mutating AWS. A malformed file - // must fail here — not after bootstrap/deploy — so we never leave AWS changed - // with the new stack ARN unrecorded because the post-deploy write can't parse it. + // Fail on local setup errors (missing toolchain/deps) and malformed state + // before any AWS mutation, so a local problem never leaves credentials + // provisioned or the stack ARN unrecorded. + await this.ensureCdkDependencies(project); await readDeployedState(this.json, project.rootPath); + // Credential providers aren't stack resources; the synthesized app reads their + // ARNs from deployed-state.json, so they must exist and be recorded before synth. + const provisioned = yield* this.provisionCredentials(project, { + credentials, + region: target.region, + }); + // Recorded every deploy (even when empty) so dropping the last credential + // from the spec clears the stale entry instead of leaving it advertised. + await updateTargetState(this.json, project.rootPath, target.name, { + resources: { credentials: provisioned }, + }); + yield* this.build(project); const assemblyDirectory = this.assemblyDirectory(project); const artifact = await stackArtifactForTarget(this.json, assemblyDirectory, target.name); @@ -258,6 +293,11 @@ export class CdkBackend implements ProjectBackend { yield { message: `Removing stack ${artifact.stackName}` }; await this.cdk({ kind: "destroy", stackArtifactId: artifact.id }, options); + // After the stack, since a resource in it may still be using the provider. + yield* this.removePaymentCredentials(project, { + credentials: options.credentials, + region: target.region, + }); await removeTargetState(this.json, project.rootPath, target.name); return { outputs: {}, tornDown: true }; } diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts new file mode 100644 index 000000000..2b2f612e4 --- /dev/null +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -0,0 +1,711 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { tmpdir } from "node:os"; +import { + ResourceNotFoundException, + type CreateApiKeyCredentialProviderResponse, + type CreateOauth2CredentialProviderResponse, + type GetApiKeyCredentialProviderResponse, + type GetOauth2CredentialProviderResponse, + type UpdateApiKeyCredentialProviderResponse, + type UpdateOauth2CredentialProviderResponse, + type CreatePaymentCredentialProviderResponse, + type DeleteApiKeyCredentialProviderResponse, + type DeleteOauth2CredentialProviderResponse, + type DeletePaymentCredentialProviderResponse, + type GetPaymentCredentialProviderResponse, + type UpdatePaymentCredentialProviderResponse, +} from "@aws-sdk/client-bedrock-agentcore-control"; +import type { Project, ProjectEvent } from "../../../../handlers/project/types"; +import { ProjectSpecSchema } from "../../../../projectSchemas/project"; +import type { CoreOptions } from "../../../types"; +import { EnvLocalFile } from "../../envLocal"; +import { + createCredentialProvisioner, + createPaymentCredentialRemover, + type CredentialProviderCalls, + type CredentialProvisioner, + type DeployedCredential, + type DeployedCredentials, +} from "./credentials"; +import type { CdkCredentialProvider } from "./toolkit"; + +const REGION = "us-east-1"; +const CREDENTIALS: CdkCredentialProvider = async () => ({ + accessKeyId: "access-key", + secretAccessKey: "secret-key", +}); +const OPTIONS: CoreOptions = { region: REGION, credentials: CREDENTIALS }; + +const API_KEY = { authorizerType: "ApiKeyCredentialProvider", name: "openai-key" } as const; +const DISCOVERY = "https://example.com/.well-known/openid-configuration"; +const OAUTH = { + authorizerType: "OAuthCredentialProvider", + name: "my-oauth", + clientId: "client-1", + discoveryUrl: DISCOVERY, + scopes: ["read"], +} as const; + +const tempDirectories: string[] = []; +afterEach(async () => { + await Promise.all( + tempDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true })), + ); +}); + +async function project(credentials: unknown[], envLocal?: string): Promise { + const rootPath = await mkdtemp(join(tmpdir(), "agentcore-credentials-")); + tempDirectories.push(rootPath); + const file = new EnvLocalFile(rootPath); + await mkdir(dirname(file.path), { recursive: true }); + if (envLocal !== undefined) await writeFile(file.path, envLocal); + return { + name: "example", + rootPath, + spec: ProjectSpecSchema.parse({ name: "example", version: 1, credentials }), + }; +} + +type Call = { kind: string; input: unknown; options: CoreOptions }; + +// Identity reports a provider that does not exist by throwing, which is the normal +// first-deploy case rather than a failure. +const notFound = () => + new ResourceNotFoundException({ $metadata: {}, message: "provider not found" }); + +// The two provider families carry their secret ARN under different response fields. +const apiKeyResponse = (provider: DeployedCredential | undefined) => + ({ + credentialProviderArn: provider?.credentialProviderArn, + ...(provider?.clientSecretArn && { apiKeySecretArn: { secretArn: provider.clientSecretArn } }), + }) as GetApiKeyCredentialProviderResponse & + CreateApiKeyCredentialProviderResponse & + UpdateApiKeyCredentialProviderResponse; + +const paymentResponse = (provider: DeployedCredential | undefined) => + ({ + credentialProviderArn: provider?.credentialProviderArn, + }) as GetPaymentCredentialProviderResponse & + CreatePaymentCredentialProviderResponse & + UpdatePaymentCredentialProviderResponse; + +const oauth2Response = (provider: DeployedCredential | undefined) => + ({ + credentialProviderArn: provider?.credentialProviderArn, + ...(provider?.clientSecretArn && { clientSecretArn: { secretArn: provider.clientSecretArn } }), + }) as GetOauth2CredentialProviderResponse & + CreateOauth2CredentialProviderResponse & + UpdateOauth2CredentialProviderResponse; + +/** Overrides for the paths that only a failing or incomplete Identity produces. */ +type Behavior = { + /** Thrown by every lookup, in place of the not-found default. */ + getFails?: Error; + /** Returned by every create, in place of a fully populated provider. */ + createReturns?: DeployedCredential; + /** Thrown by every payment-provider deletion. */ + deleteFails?: Error; + /** Thrown by the creation of the named provider, in place of a created one. */ + createFailsFor?: string; +}; + +function identity( + existing: DeployedCredentials = {}, + behavior: Behavior = {}, + processEnv: Record = {}, +) { + const calls: Call[] = []; + + const created = (name: string, prefix: string): DeployedCredential => + behavior.createReturns ?? { + credentialProviderArn: `arn:${prefix}:${name}`, + clientSecretArn: `arn:secret:${name}`, + }; + + const lookup = (name: string): DeployedCredential => { + if (behavior.getFails) throw behavior.getFails; + const found = existing[name]; + if (!found) throw notFound(); + return found; + }; + + const client: CredentialProviderCalls = { + async getApiKeyCredentialProvider(name, options) { + calls.push({ kind: "getApiKey", input: name, options }); + return apiKeyResponse(lookup(name)); + }, + async createApiKeyCredentialProvider(input, options) { + calls.push({ kind: "createApiKey", input, options }); + if (behavior.createFailsFor === input.name) throw new Error(`create ${input.name} failed`); + return apiKeyResponse(created(input.name ?? "", "apikey")); + }, + async updateApiKeyCredentialProvider(input, options) { + calls.push({ kind: "updateApiKey", input, options }); + return apiKeyResponse(created(input.name ?? "", "apikey")); + }, + async deleteApiKeyCredentialProvider(name, options) { + calls.push({ kind: "deleteApiKey", input: name, options }); + if (behavior.deleteFails) throw behavior.deleteFails; + return {} as DeleteApiKeyCredentialProviderResponse; + }, + async getOauth2CredentialProvider(name, options) { + calls.push({ kind: "getOauth2", input: name, options }); + return oauth2Response(lookup(name)); + }, + async createOauth2CredentialProvider(input, options) { + calls.push({ kind: "createOauth2", input, options }); + return oauth2Response(created(input.name ?? "", "oauth")); + }, + async updateOauth2CredentialProvider(input, options) { + calls.push({ kind: "updateOauth2", input, options }); + return oauth2Response(created(input.name ?? "", "oauth")); + }, + async deleteOauth2CredentialProvider(name, options) { + calls.push({ kind: "deleteOauth2", input: name, options }); + if (behavior.deleteFails) throw behavior.deleteFails; + return {} as DeleteOauth2CredentialProviderResponse; + }, + async getPaymentCredentialProvider(name, options) { + calls.push({ kind: "getPayment", input: name, options }); + return paymentResponse(lookup(name)); + }, + async createPaymentCredentialProvider(input, options) { + calls.push({ kind: "createPayment", input, options }); + return paymentResponse(created(input.name ?? "", "payment")); + }, + async updatePaymentCredentialProvider(input, options) { + calls.push({ kind: "updatePayment", input, options }); + return paymentResponse(created(input.name ?? "", "payment")); + }, + async deletePaymentCredentialProvider(name, options) { + calls.push({ kind: "deletePayment", input: name, options }); + if (behavior.deleteFails) throw behavior.deleteFails; + return {} as DeletePaymentCredentialProviderResponse; + }, + }; + + return { calls, client, provision: createCredentialProvisioner(client, processEnv) }; +} + +async function collect( + generator: AsyncGenerator, +): Promise<{ events: ProjectEvent[] }> { + const events: ProjectEvent[] = []; + while (true) { + const next = await generator.next(); + if (next.done) return { events }; + events.push(next.value); + } +} + +async function run( + provision: CredentialProvisioner, + input: Project, +): Promise<{ events: ProjectEvent[]; result: DeployedCredentials }> { + const generator = provision(input, { credentials: CREDENTIALS, region: REGION }); + const events: ProjectEvent[] = []; + while (true) { + const next = await generator.next(); + if (next.done) return { events, result: next.value }; + events.push(next.value as ProjectEvent); + } +} + +describe("createCredentialProvisioner", () => { + test("calls Identity not at all for a project without credentials", async () => { + const subject = identity(); + + const { events, result } = await run(subject.provision, await project([])); + + expect(result).toEqual({}); + expect(events).toEqual([]); + expect(subject.calls).toEqual([]); + }); + + test("runs every call against the target's own region and credentials", async () => { + const subject = identity(); + const input = await project([API_KEY], "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n"); + + await run(subject.provision, input); + + expect(subject.calls.map((call) => call.options)).toEqual([OPTIONS, OPTIONS]); + }); + + test("creates an API key provider from the secret in .env.local", async () => { + const subject = identity(); + const input = await project([API_KEY], "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n"); + + const { events, result } = await run(subject.provision, input); + + expect(events).toEqual([{ message: "Preparing credential provider 'openai-key'" }]); + expect(subject.calls.map(({ kind, input: called }) => ({ kind, input: called }))).toEqual([ + { kind: "getApiKey", input: "openai-key" }, + { kind: "createApiKey", input: { name: "openai-key", apiKey: "sk-live" } }, + ]); + expect(result).toEqual({ + "openai-key": { + credentialProviderArn: "arn:apikey:openai-key", + clientSecretArn: "arn:secret:openai-key", + }, + }); + }); + + test("creates an API key provider from a Secrets Manager reference", async () => { + const secretRef = { secretId: "prod/openai", jsonKey: "apiKey" }; + const subject = identity(); + const input = await project([{ ...API_KEY, secretRef }]); + + await run(subject.provision, input); + + expect(subject.calls[1]?.input).toEqual({ + name: "openai-key", + apiKeySecretConfig: secretRef, + apiKeySecretSource: "EXTERNAL", + }); + }); + + test("takes a secret from the environment when .env.local has none", async () => { + const subject = identity({}, {}, { AGENTCORE_CREDENTIAL_OPENAI_KEY: "sk-from-env" }); + const input = await project([API_KEY]); + + await run(subject.provision, input); + + expect(subject.calls[1]?.input).toEqual({ name: "openai-key", apiKey: "sk-from-env" }); + }); + + test("prefers the environment over .env.local, ignoring unrelated variables", async () => { + const subject = identity( + {}, + {}, + { AGENTCORE_CREDENTIAL_OPENAI_KEY: "sk-from-env", HOME: "/should/not/matter" }, + ); + const input = await project([API_KEY], "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-from-file'\n"); + + await run(subject.provision, input); + + expect(subject.calls[1]?.input).toEqual({ name: "openai-key", apiKey: "sk-from-env" }); + }); + + test("names the variable and file to fix when an API key secret is missing", async () => { + const subject = identity(); + const input = await project([API_KEY]); + + await expect(run(subject.provision, input)).rejects.toThrow( + new RegExp( + `AGENTCORE_CREDENTIAL_OPENAI_KEY[\\s\\S]*${join(input.rootPath, "agentcore", ".env.local").replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}[\\s\\S]*secretRef`, + ), + ); + }); + + test("updates a provider that already exists with the current secret", async () => { + const existing = { credentialProviderArn: "arn:existing", clientSecretArn: "arn:existing/s" }; + const subject = identity({ "openai-key": existing }); + const input = await project([API_KEY], "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-rotated'\n"); + + const { result } = await run(subject.provision, input); + + expect(subject.calls.map((call) => call.kind)).toEqual(["getApiKey", "updateApiKey"]); + expect(subject.calls[1]?.input).toEqual({ name: "openai-key", apiKey: "sk-rotated" }); + expect(result).toEqual({ + "openai-key": { + credentialProviderArn: "arn:apikey:openai-key", + clientSecretArn: "arn:secret:openai-key", + }, + }); + }); + + test("updates an existing OAuth provider with the current client secret", async () => { + const subject = identity({ "my-oauth": { credentialProviderArn: "arn:existing" } }); + const input = await project([OAUTH], "AGENTCORE_CREDENTIAL_MY_OAUTH_CLIENT_SECRET='rotated'\n"); + + await run(subject.provision, input); + + expect(subject.calls.map((call) => call.kind)).toEqual(["getOauth2", "updateOauth2"]); + expect(subject.calls[1]?.input).toEqual({ + name: "my-oauth", + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: { + customOauth2ProviderConfig: { + oauthDiscovery: { discoveryUrl: DISCOVERY }, + clientId: "client-1", + clientSecret: "rotated", + }, + }, + }); + }); + + test("leaves an existing provider alone when no secret is available locally", async () => { + const existing = { credentialProviderArn: "arn:existing", clientSecretArn: "arn:existing/s" }; + const subject = identity({ "openai-key": existing }); + // No .env.local entry and no secretRef: there is nothing to push, so the + // provider keeps whatever secret it holds rather than failing the deploy. + const input = await project([API_KEY]); + + const { result } = await run(subject.provision, input); + + expect(subject.calls.map((call) => call.kind)).toEqual(["getApiKey"]); + expect(result).toEqual({ "openai-key": existing }); + }); + + test("updates a provider backed by an external secret reference", async () => { + const secretRef = { secretId: "prod/openai", jsonKey: "apiKey" }; + const subject = identity({ "openai-key": { credentialProviderArn: "arn:existing" } }); + const input = await project([{ ...API_KEY, secretRef }]); + + await run(subject.provision, input); + + expect(subject.calls.map((call) => call.kind)).toEqual(["getApiKey", "updateApiKey"]); + expect(subject.calls[1]?.input).toEqual({ + name: "openai-key", + apiKeySecretConfig: secretRef, + apiKeySecretSource: "EXTERNAL", + }); + }); + + test("records a provider that has no secret ARN without one", async () => { + const subject = identity({ "openai-key": { credentialProviderArn: "arn:existing" } }); + const input = await project([API_KEY]); + + const { result } = await run(subject.provision, input); + + expect(result).toEqual({ "openai-key": { credentialProviderArn: "arn:existing" } }); + }); + + test("fails when Identity returns a provider without an ARN", async () => { + const subject = identity({}, { createReturns: {} as DeployedCredential }); + const input = await project([API_KEY], "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n"); + + await expect(run(subject.provision, input)).rejects.toThrow(/no credentialProviderArn/); + }); + + test("propagates a lookup failure that is not a missing provider", async () => { + const denied = Object.assign(new Error("denied"), { name: "AccessDeniedException" }); + const subject = identity({}, { getFails: denied }); + const input = await project([API_KEY], "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n"); + + await expect(run(subject.provision, input)).rejects.toBe(denied); + }); + + test("creates a guided OAuth2 provider without forwarding scopes", async () => { + const subject = identity(); + const input = await project([OAUTH], "AGENTCORE_CREDENTIAL_MY_OAUTH_CLIENT_SECRET='shh'\n"); + + const { result } = await run(subject.provision, input); + + expect(subject.calls.map(({ kind, input: called }) => ({ kind, input: called }))).toEqual([ + { kind: "getOauth2", input: "my-oauth" }, + { + kind: "createOauth2", + input: { + name: "my-oauth", + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: { + customOauth2ProviderConfig: { + oauthDiscovery: { discoveryUrl: DISCOVERY }, + clientId: "client-1", + clientSecret: "shh", + }, + }, + }, + }, + ]); + expect(result["my-oauth"]).toEqual({ + credentialProviderArn: "arn:oauth:my-oauth", + clientSecretArn: "arn:secret:my-oauth", + }); + }); + + test("falls back to the legacy _CLIENT_ID variable when the spec has no clientId", async () => { + const subject = identity(); + // An older CLI kept the client id in .env.local, not agentcore.json. + const { clientId: _dropped, ...withoutClientId } = OAUTH; + const input = await project( + [withoutClientId], + "AGENTCORE_CREDENTIAL_MY_OAUTH_CLIENT_SECRET='shh'\n" + + "AGENTCORE_CREDENTIAL_MY_OAUTH_CLIENT_ID='legacy-client'\n", + ); + + await run(subject.provision, input); + + expect(subject.calls[1]?.input).toEqual({ + name: "my-oauth", + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: { + customOauth2ProviderConfig: { + oauthDiscovery: { discoveryUrl: DISCOVERY }, + clientId: "legacy-client", + clientSecret: "shh", + }, + }, + }); + }); + + test("injects the secret into a spec-supplied provider config", async () => { + const subject = identity(); + const input = await project( + [ + { + authorizerType: "OAuthCredentialProvider", + name: "vendored", + vendor: "GoogleOauth2", + providerConfig: { + googleOauth2ProviderConfig: { clientId: "google-client" }, + }, + }, + ], + "AGENTCORE_CREDENTIAL_VENDORED_CLIENT_SECRET='g-secret'\n", + ); + + await run(subject.provision, input); + + expect(subject.calls[1]?.input).toEqual({ + name: "vendored", + credentialProviderVendor: "GoogleOauth2", + oauth2ProviderConfigInput: { + googleOauth2ProviderConfig: { clientId: "google-client", clientSecret: "g-secret" }, + }, + }); + }); + + test("passes an OAuth secret reference through as an external secret", async () => { + const clientSecretRef = { secretId: "prod/oauth", jsonKey: "clientSecret" }; + const subject = identity(); + const input = await project([{ ...OAUTH, clientSecretRef }]); + + await run(subject.provision, input); + + expect(subject.calls[1]?.input).toEqual({ + name: "my-oauth", + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: { + customOauth2ProviderConfig: { + oauthDiscovery: { discoveryUrl: DISCOVERY }, + clientId: "client-1", + clientSecretConfig: clientSecretRef, + clientSecretSource: "EXTERNAL", + }, + }, + }); + }); + + test("rejects a provider config that is not a single vendor object", async () => { + const subject = identity(); + const input = await project( + [ + { + authorizerType: "OAuthCredentialProvider", + name: "two-vendors", + vendor: "GoogleOauth2", + providerConfig: { + googleOauth2ProviderConfig: { clientId: "a" }, + githubOauth2ProviderConfig: { clientId: "b" }, + }, + }, + ], + "AGENTCORE_CREDENTIAL_TWO_VENDORS_CLIENT_SECRET='s'\n", + ); + + await expect(run(subject.provision, input)).rejects.toThrow(/exactly one vendor config object/); + }); + + test("creates a Coinbase payment provider from the vendor's variables", async () => { + const subject = identity(); + const input = await project( + [{ authorizerType: "PaymentCredentialProvider", name: "wallet", provider: "CoinbaseCDP" }], + "AGENTCORE_CREDENTIAL_WALLET_API_KEY_ID='key-1'\n" + + "AGENTCORE_CREDENTIAL_WALLET_API_KEY_SECRET='key-secret'\n" + + "AGENTCORE_CREDENTIAL_WALLET_WALLET_SECRET='wallet-secret'\n", + ); + + const { result } = await run(subject.provision, input); + + expect(subject.calls.map((call) => call.kind)).toEqual(["getPayment", "createPayment"]); + expect(subject.calls[1]?.input).toEqual({ + name: "wallet", + credentialProviderVendor: "CoinbaseCDP", + providerConfigurationInput: { + coinbaseCdpConfiguration: { + apiKeyId: "key-1", + apiKeySecret: "key-secret", + walletSecret: "wallet-secret", + }, + }, + }); + // A payment provider holds several secrets, each under its own vendor field, so + // only the provider ARN is recorded. + expect(result).toEqual({ wallet: { credentialProviderArn: "arn:payment:wallet" } }); + }); + + test("updates an existing StripePrivy payment provider", async () => { + const subject = identity({ pay: { credentialProviderArn: "arn:existing" } }); + const input = await project( + [{ authorizerType: "PaymentCredentialProvider", name: "pay", provider: "StripePrivy" }], + "AGENTCORE_CREDENTIAL_PAY_APP_ID='app-1'\n" + + "AGENTCORE_CREDENTIAL_PAY_APP_SECRET='app-secret'\n" + + "AGENTCORE_CREDENTIAL_PAY_AUTHORIZATION_PRIVATE_KEY='priv-key'\n" + + "AGENTCORE_CREDENTIAL_PAY_AUTHORIZATION_ID='auth-1'\n", + ); + + await run(subject.provision, input); + + expect(subject.calls.map((call) => call.kind)).toEqual(["getPayment", "updatePayment"]); + expect(subject.calls[1]?.input).toEqual({ + name: "pay", + credentialProviderVendor: "StripePrivy", + providerConfigurationInput: { + stripePrivyConfiguration: { + appId: "app-1", + appSecret: "app-secret", + authorizationPrivateKey: "priv-key", + authorizationId: "auth-1", + }, + }, + }); + }); + + test("names every payment variable that is unset", async () => { + const subject = identity(); + const input = await project( + [{ authorizerType: "PaymentCredentialProvider", name: "wallet", provider: "CoinbaseCDP" }], + "AGENTCORE_CREDENTIAL_WALLET_API_KEY_ID='key-1'\n", + ); + + await expect(run(subject.provision, input)).rejects.toThrow( + /AGENTCORE_CREDENTIAL_WALLET_API_KEY_SECRET, AGENTCORE_CREDENTIAL_WALLET_WALLET_SECRET/, + ); + }); + + test("leaves an existing payment provider alone when its variables are unset", async () => { + const existing = { credentialProviderArn: "arn:existing" }; + const subject = identity({ wallet: existing }); + const input = await project([ + { authorizerType: "PaymentCredentialProvider", name: "wallet", provider: "CoinbaseCDP" }, + ]); + + const { result } = await run(subject.provision, input); + + expect(subject.calls.map((call) => call.kind)).toEqual(["getPayment"]); + expect(result).toEqual({ wallet: existing }); + }); + + test("deletes only the payment providers a torn-down project declares", async () => { + const subject = identity(); + const input = await project([ + API_KEY, + { authorizerType: "PaymentCredentialProvider", name: "wallet", provider: "CoinbaseCDP" }, + ]); + + const remove = createPaymentCredentialRemover(subject.client); + const { events } = await collect(remove(input, { credentials: CREDENTIALS, region: REGION })); + + // The api-key provider is named account-globally and may be shared, so it stays. + expect(subject.calls).toEqual([{ kind: "deletePayment", input: "wallet", options: OPTIONS }]); + expect(events).toEqual([{ message: "Removing credential provider 'wallet'" }]); + }); + + test("treats a payment provider that is already gone as removed", async () => { + const subject = identity({}, { deleteFails: notFound() }); + const input = await project([ + { authorizerType: "PaymentCredentialProvider", name: "wallet", provider: "CoinbaseCDP" }, + ]); + + const remove = createPaymentCredentialRemover(subject.client); + const { events } = await collect(remove(input, { credentials: CREDENTIALS, region: REGION })); + + expect(events).toEqual([{ message: "Removing credential provider 'wallet'" }]); + }); + + test("reports a payment provider it could not delete rather than failing", async () => { + const subject = identity({}, { deleteFails: new Error("still in use") }); + const input = await project([ + { authorizerType: "PaymentCredentialProvider", name: "wallet", provider: "CoinbaseCDP" }, + ]); + + const remove = createPaymentCredentialRemover(subject.client); + const { events } = await collect(remove(input, { credentials: CREDENTIALS, region: REGION })); + + expect(events[1]?.message).toMatch(/Could not remove credential provider 'wallet'.*in use/); + }); + + test("deletes what it created when a later provider fails", async () => { + const subject = identity({}, { createFailsFor: "other-key" }); + const input = await project( + [API_KEY, { authorizerType: "ApiKeyCredentialProvider", name: "other-key" }], + "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n" + "AGENTCORE_CREDENTIAL_OTHER_KEY='sk-other'\n", + ); + + // The failure the user needs to see is the one that stopped the deploy. + await expect(run(subject.provision, input)).rejects.toThrow(/create other-key failed/); + expect(subject.calls.map((call) => call.kind)).toEqual([ + "getApiKey", + "getApiKey", + "createApiKey", + "createApiKey", + // Only the one this run brought into existence. + "deleteApiKey", + ]); + expect(subject.calls[4]?.input).toBe("openai-key"); + }); + + test("does not delete a provider it only updated", async () => { + const subject = identity( + { "openai-key": { credentialProviderArn: "arn:existing" } }, + { createFailsFor: "other-key" }, + ); + const input = await project( + [API_KEY, { authorizerType: "ApiKeyCredentialProvider", name: "other-key" }], + "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n" + "AGENTCORE_CREDENTIAL_OTHER_KEY='sk-other'\n", + ); + + await expect(run(subject.provision, input)).rejects.toThrow(/create other-key failed/); + // Undoing an update would need the secret the provider held before, which the + // CLI never had, so the pre-existing provider is left as this deploy set it. + expect(subject.calls.map((call) => call.kind)).not.toContain("deleteApiKey"); + }); + + test("reports a provider it created but could not delete", async () => { + const subject = identity( + {}, + { createFailsFor: "other-key", deleteFails: new Error("access denied") }, + ); + const input = await project( + [API_KEY, { authorizerType: "ApiKeyCredentialProvider", name: "other-key" }], + "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n" + "AGENTCORE_CREDENTIAL_OTHER_KEY='sk-other'\n", + ); + + const generator = subject.provision(input, { credentials: CREDENTIALS, region: REGION }); + const events: ProjectEvent[] = []; + await expect( + (async () => { + while (true) { + const next = await generator.next(); + if (next.done) return; + events.push(next.value); + } + })(), + ).rejects.toThrow(/create other-key failed/); + + expect(events.map((event) => event.message)).toContain( + "Removing credential provider 'openai-key' this deploy created", + ); + expect(events[events.length - 1]?.message).toMatch( + /Could not remove credential provider 'openai-key'.*access denied.*next deploy/s, + ); + }); + + test("creates nothing when a later credential's secret is missing", async () => { + const subject = identity(); + // First credential's secret is present; the second's is not. + const input = await project( + [API_KEY, { authorizerType: "ApiKeyCredentialProvider", name: "other-key" }], + "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n", + ); + + await expect(run(subject.provision, input)).rejects.toThrow(/AGENTCORE_CREDENTIAL_OTHER_KEY/); + // Both looked up, but no provider was created — the missing secret is caught + // before the first create, so there is no half-provisioned AWS state. + expect(subject.calls.map((c) => c.kind)).toEqual(["getApiKey", "getApiKey"]); + }); +}); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts new file mode 100644 index 000000000..58ba2c968 --- /dev/null +++ b/src/core/project/backends/cdk/credentials.ts @@ -0,0 +1,579 @@ +import { join } from "node:path"; +import { + ResourceNotFoundException, + type Oauth2ProviderConfigInput, + type PaymentProviderConfigurationInput, +} from "@aws-sdk/client-bedrock-agentcore-control"; +import { MalformedServiceResponseError, ProjectStateError } from "../../../../errors/errors"; +import type { CoreIdentityClient } from "../../../../handlers/identity/types"; +import type { Project, ProjectEvent } from "../../../../handlers/project/types"; +import type { + ApiKeyCredential, + Credential, + OAuthCredential, + PaymentCredential, +} from "../../../../projectSchemas/credential"; +import { + CREDENTIAL_ENV_PREFIX, + credentialEnvironmentVariableNames, + credentialEnvVarName, +} from "../../../../projectSchemas/credential"; +import type { CoreOptions } from "../../../types"; +import { ENV_LOCAL_RELATIVE_PATH, EnvLocalFile } from "../../envLocal"; +import type { CdkCredentialProvider } from "./toolkit"; + +/** A provisioned provider, in the shape the synthesized CDK app reads back. */ +export type DeployedCredential = { + credentialProviderArn: string; + clientSecretArn?: string; +}; +export type DeployedCredentials = Record; + +/** + * The Identity operations provisioning uses, narrowed from the Core client that + * backs the `agentcore identity` commands. Narrowed rather than taken whole so + * tests fake six calls instead of ten. + */ +export type CredentialProviderCalls = Pick< + CoreIdentityClient, + | "getApiKeyCredentialProvider" + | "createApiKeyCredentialProvider" + | "updateApiKeyCredentialProvider" + | "getOauth2CredentialProvider" + | "createOauth2CredentialProvider" + | "updateOauth2CredentialProvider" + | "getPaymentCredentialProvider" + | "createPaymentCredentialProvider" + | "updatePaymentCredentialProvider" + // Deletes undo what one deploy created; a provider that already existed is never + // deleted by a deploy, and only payment providers are deleted by a teardown. + | "deleteApiKeyCredentialProvider" + | "deleteOauth2CredentialProvider" + | "deletePaymentCredentialProvider" +>; + +export type CredentialProvisionInput = { + region: string; + /** Credential provider shared with the rest of the deployment preflight. */ + credentials: CdkCredentialProvider; +}; + +export type CredentialProvisioner = ( + project: Project, + input: CredentialProvisionInput, +) => AsyncGenerator; + +export type PaymentCredentialRemover = ( + project: Project, + input: CredentialProvisionInput, +) => AsyncGenerator; + +/** + * Deletes the payment credential providers a project declares, for a teardown that + * has already removed its stack. + * + * Only payment providers: they hold a payment vendor's own API key, wallet and + * authorization secrets, provisioned for this project alone. An API-key or OAuth + * provider is named account-globally and may be shared with another project or with + * work done outside the CLI, so tearing down one project never removes it. + * + * A provider that is already gone is not an error, and a provider that cannot be + * deleted is reported rather than failing a teardown whose stack is already gone. + */ +export function createPaymentCredentialRemover( + identity: Pick, +): PaymentCredentialRemover { + return async function* removePaymentCredentials(project, { region, credentials }) { + const payments = project.spec.credentials.filter( + (credential) => credential.authorizerType === "PaymentCredentialProvider", + ); + if (payments.length === 0) return; + + const options: CoreOptions = { region, credentials }; + for (const { name } of payments) { + yield { message: `Removing credential provider '${name}'` }; + try { + await identity.deletePaymentCredentialProvider(name, options); + } catch (error) { + if (error instanceof ResourceNotFoundException) continue; + yield { + message: + `Could not remove credential provider '${name}': ${(error as Error).message}. ` + + `Delete it with 'aws bedrock-agentcore-control delete-payment-credential-provider'.`, + }; + } + } + }; +} + +/** + * Provisions the credential providers a project declares, before synthesis: the + * synthesized app reads their ARNs from `deployed-state.json`, so a project with + * credentials can't synthesize until they exist. + * + * A provider is created when absent and updated when present, so editing a secret + * in `.env.local` and redeploying pushes the new value. A provider whose secret the + * CLI cannot see — nothing in `.env.local` and no external reference — is left + * exactly as it is rather than failing the deploy. + */ +export function createCredentialProvisioner( + identity: CredentialProviderCalls, + processEnv: Record = process.env, +): CredentialProvisioner { + return async function* provisionCredentials(project, { region, credentials }) { + const declared = project.spec.credentials; + if (declared.length === 0) return {}; + + // Credential variables set in the process environment win over the file, so a + // deploy can be handed its secrets without writing them to disk first. + const env = { + ...(await new EnvLocalFile(project.rootPath).read()), + ...credentialEnvironment(processEnv), + }; + // Every Identity call runs against the deployment target's own credentials + // rather than the default chain, in the region the target deploys to. + const options: CoreOptions = { region, credentials }; + + // Resolve every credential before writing any: look up existing providers and + // validate the secret each one needs. A missing secret then fails before the + // first provider is written, not partway through the list. + const plans: { credential: Credential; provision: Provision }[] = []; + for (const credential of declared) { + plans.push({ + credential, + provision: await resolveCredential(identity, credential, options, env, project.rootPath), + }); + } + + const provisioned: DeployedCredentials = {}; + // Providers this deploy brought into existence, so a later failure can undo them + // rather than leaving one behind that nothing records. + const created: Credential[] = []; + try { + for (const { credential, provision } of plans) { + yield { message: `Preparing credential provider '${credential.name}'` }; + if ("reuse" in provision) { + provisioned[credential.name] = provision.reuse; + continue; + } + provisioned[credential.name] = await provision.write(); + if (provision.kind === "create") created.push(credential); + } + } catch (error) { + yield* rollback(identity, created, options); + throw error; + } + return provisioned; + }; +} + +/** + * Deletes the providers a failed deploy created, newest first. A provider that + * already existed is left alone — this deploy only updated its secret, and undoing + * that would need the value it held before. + * + * A deletion that fails is reported rather than thrown: the error that started the + * rollback is the one the user needs to see. + */ +async function* rollback( + identity: CredentialProviderCalls, + created: Credential[], + options: CoreOptions, +): AsyncGenerator { + for (const credential of [...created].reverse()) { + yield { message: `Removing credential provider '${credential.name}' this deploy created` }; + try { + await deleteCredential(identity, credential, options); + } catch (error) { + yield { + message: + `Could not remove credential provider '${credential.name}': ` + + `${(error as Error).message}. It exists in AWS but is not recorded; the next deploy ` + + `of this project will adopt it.`, + }; + } + } +} + +function deleteCredential( + identity: CredentialProviderCalls, + credential: Credential, + options: CoreOptions, +): Promise { + switch (credential.authorizerType) { + case "ApiKeyCredentialProvider": + return identity.deleteApiKeyCredentialProvider(credential.name, options); + case "OAuthCredentialProvider": + return identity.deleteOauth2CredentialProvider(credential.name, options); + case "PaymentCredentialProvider": + return identity.deletePaymentCredentialProvider(credential.name, options); + } +} + +/** + * What a credential needs: an existing provider to leave alone, or a write — + * `create` for a provider that does not exist yet, `update` for one that does — + * deferred until every credential has been resolved. + */ +type Provision = + | { reuse: DeployedCredential } + | { kind: "create" | "update"; write: () => Promise }; + +function resolveCredential( + identity: CredentialProviderCalls, + credential: Credential, + options: CoreOptions, + env: Record, + rootPath: string, +): Promise { + switch (credential.authorizerType) { + case "ApiKeyCredentialProvider": + return resolveApiKey(identity, credential, options, env, rootPath); + case "OAuthCredentialProvider": + return resolveOauth2(identity, credential, options, env, rootPath); + case "PaymentCredentialProvider": + return resolvePayment(identity, credential, options, env, rootPath); + } +} + +async function resolveApiKey( + identity: CredentialProviderCalls, + credential: ApiKeyCredential, + options: CoreOptions, + env: Record, + rootPath: string, +): Promise { + const { name } = credential; + // Provider names are account-global, so one already in this account is the one + // this project's credential resolves to. + const existing = await undefinedWhenAbsent(() => + identity.getApiKeyCredentialProvider(name, options), + ); + + const secret = credential.secretRef + ? { apiKeySecretConfig: credential.secretRef, apiKeySecretSource: "EXTERNAL" as const } + : secretFromEnv(env, name, (apiKey) => ({ apiKey })); + if (!secret) { + // Nothing to write. An existing provider keeps whatever secret it holds; an + // absent one cannot be created at all. + if (existing) return { reuse: apiKeyProvision(name, existing) }; + throw missingSecret(name, credentialEnvVarName(name), "secretRef", rootPath); + } + + const input = { name, ...secret }; + if (existing) { + return { + kind: "update", + write: async () => + apiKeyProvision(name, await identity.updateApiKeyCredentialProvider(input, options)), + }; + } + return { + kind: "create", + write: async () => + apiKeyProvision(name, await identity.createApiKeyCredentialProvider(input, options)), + }; +} + +async function resolveOauth2( + identity: CredentialProviderCalls, + credential: OAuthCredential, + options: CoreOptions, + env: Record, + rootPath: string, +): Promise { + const existing = await undefinedWhenAbsent(() => + identity.getOauth2CredentialProvider(credential.name, options), + ); + + const secret: Record | undefined = credential.clientSecretRef + ? { clientSecretConfig: credential.clientSecretRef, clientSecretSource: "EXTERNAL" } + : secretFromEnv(env, credential.name, (clientSecret) => ({ clientSecret }), "_CLIENT_SECRET"); + if (!secret) { + if (existing) return { reuse: oauth2Provision(credential.name, existing) }; + throw missingSecret( + credential.name, + credentialEnvVarName(credential.name, "_CLIENT_SECRET"), + "clientSecretRef", + rootPath, + ); + } + + // Projects created by older CLIs kept the client id in .env.local rather than + // agentcore.json, so fall back to that legacy variable when the spec has none. + const clientId = credential.clientId ?? env[credentialEnvVarName(credential.name, "_CLIENT_ID")]; + const config = credential.providerConfig + ? vendorConfigWithSecret(credential.name, credential.providerConfig, secret) + : guidedCustomConfig(credential, clientId, secret); + const input = { + name: credential.name, + // The spec's vendor is free-form so a new service vendor works without + // a CLI release; the service rejects values it does not know. + credentialProviderVendor: credential.vendor as never, + oauth2ProviderConfigInput: config, + }; + if (existing) { + return { + kind: "update", + write: async () => + oauth2Provision( + credential.name, + await identity.updateOauth2CredentialProvider(input, options), + ), + }; + } + return { + kind: "create", + write: async () => + oauth2Provision( + credential.name, + await identity.createOauth2CredentialProvider(input, options), + ), + }; +} + +/** + * The credential variables an environment carries. Filtered to the credential prefix + * so a deploy reads the secrets it was handed and nothing else from the environment. + */ +function credentialEnvironment( + processEnv: Record, +): Record { + return Object.fromEntries( + Object.entries(processEnv).filter(([key]) => key.startsWith(CREDENTIAL_ENV_PREFIX)), + ); +} + +/** + * A payment provider's fields all come from the environment — the vendor's own + * identifiers as well as its secrets — so the credential is written only when every + * one of them is present, and an existing provider is otherwise left alone. + */ +async function resolvePayment( + identity: CredentialProviderCalls, + credential: PaymentCredential, + options: CoreOptions, + env: Record, + rootPath: string, +): Promise { + const { name } = credential; + const existing = await undefinedWhenAbsent(() => + identity.getPaymentCredentialProvider(name, options), + ); + + const fields = paymentFields(credential, env); + if ("missing" in fields) { + if (existing) return { reuse: paymentProvision(name, existing) }; + throw missingPaymentSecrets(name, fields.missing, rootPath); + } + + const input = { + name, + credentialProviderVendor: credential.provider as never, + providerConfigurationInput: fields.configuration, + }; + if (existing) { + return { + kind: "update", + write: async () => + paymentProvision(name, await identity.updatePaymentCredentialProvider(input, options)), + }; + } + return { + kind: "create", + write: async () => + paymentProvision(name, await identity.createPaymentCredentialProvider(input, options)), + }; +} + +/** + * Collects a payment vendor's configuration from the environment, or reports every + * variable that is unset so the user can fill them in one pass. + */ +function paymentFields( + credential: PaymentCredential, + env: Record, +): { configuration: PaymentProviderConfigurationInput } | { missing: string[] } { + const read = (suffix: string) => env[credentialEnvVarName(credential.name, suffix)]; + const missing = credentialEnvironmentVariableNames(credential).filter((key) => !env[key]); + if (missing.length > 0) return { missing }; + + if (credential.provider === "CoinbaseCDP") { + return { + configuration: { + coinbaseCdpConfiguration: { + apiKeyId: read("_API_KEY_ID")!, + apiKeySecret: read("_API_KEY_SECRET")!, + walletSecret: read("_WALLET_SECRET")!, + }, + }, + }; + } + return { + configuration: { + stripePrivyConfiguration: { + appId: read("_APP_ID")!, + appSecret: read("_APP_SECRET")!, + authorizationPrivateKey: read("_AUTHORIZATION_PRIVATE_KEY")!, + authorizationId: read("_AUTHORIZATION_ID")!, + }, + }, + }; +} + +/** + * Reads a credential's secret from `.env.local`, shaped into the request field it + * fills, or undefined when the variable is unset. + */ +function secretFromEnv( + env: Record, + name: string, + field: (secret: string) => T, + suffix = "", +): T | undefined { + const secret = env[credentialEnvVarName(name, suffix)]; + return secret ? field(secret) : undefined; +} + +/** + * A provider lookup that treats "not found" as absent. Identity throws for a + * provider that does not exist yet, which is the normal first-deploy case. + */ +async function undefinedWhenAbsent(send: () => Promise): Promise { + try { + return await send(); + } catch (error) { + if (error instanceof ResourceNotFoundException) return undefined; + throw error; + } +} + +// The two provider families report their secret under different response fields, +// so each maps its own; both record the same shape in deployed-state.json. +function apiKeyProvision( + name: string, + response: { credentialProviderArn?: string; apiKeySecretArn?: { secretArn?: string } }, +): DeployedCredential { + return deployedCredential( + name, + response.credentialProviderArn, + response.apiKeySecretArn?.secretArn, + ); +} + +function oauth2Provision( + name: string, + response: { credentialProviderArn?: string; clientSecretArn?: { secretArn?: string } }, +): DeployedCredential { + return deployedCredential( + name, + response.credentialProviderArn, + response.clientSecretArn?.secretArn, + ); +} + +// A payment provider holds several secrets rather than one, each reported under its +// vendor's own field, so only the provider ARN is recorded. +function paymentProvision( + name: string, + response: { credentialProviderArn?: string }, +): DeployedCredential { + return deployedCredential(name, response.credentialProviderArn, undefined); +} + +function deployedCredential( + name: string, + credentialProviderArn: string | undefined, + secretArn: string | undefined, +): DeployedCredential { + return { + credentialProviderArn: requireArn(credentialProviderArn, name), + ...(secretArn && { clientSecretArn: secretArn }), + }; +} + +/** + * Injects the secret into a complete, spec-supplied vendor config. The spec + * keeps provider configs secret-free, so the one vendor key it carries is the + * only place the secret can go. + */ +function vendorConfigWithSecret( + name: string, + providerConfig: Record, + secret: Record, +): Oauth2ProviderConfigInput { + const entries = Object.entries(providerConfig); + const [configKey, vendorConfig] = entries[0] ?? []; + if ( + entries.length !== 1 || + !configKey || + typeof vendorConfig !== "object" || + vendorConfig === null || + Array.isArray(vendorConfig) + ) { + throw new ProjectStateError( + `Credential '${name}' has a providerConfig with ${entries.length} entries; it must hold ` + + `exactly one vendor config object (for example { "customOauth2ProviderConfig": { ... } }).`, + ); + } + return { [configKey]: { ...vendorConfig, ...secret } } as unknown as Oauth2ProviderConfigInput; +} + +function guidedCustomConfig( + credential: OAuthCredential, + clientId: string | undefined, + secret: Record, +): Oauth2ProviderConfigInput { + // The spec's schema requires discoveryUrl for a guided credential; this guards + // a spec written before that rule rather than a reachable state. + if (!credential.discoveryUrl) { + throw new ProjectStateError( + `Credential '${credential.name}' needs either a discoveryUrl or a providerConfig ` + + `to create its OAuth2 provider.`, + ); + } + // `scopes` is deliberately not forwarded: provider creation has no scopes + // field, and the spec's scopes are consumed where the credential is used. + return { + customOauth2ProviderConfig: { + oauthDiscovery: { discoveryUrl: credential.discoveryUrl }, + ...(clientId !== undefined && { clientId }), + ...secret, + }, + }; +} + +function missingSecret( + name: string, + envKey: string, + refField: "secretRef" | "clientSecretRef", + rootPath: string, +): ProjectStateError { + return new ProjectStateError( + `Credential '${name}' has no secret to create its provider with. Set ${envKey} in ` + + `${join(rootPath, ENV_LOCAL_RELATIVE_PATH)} or in the environment you deploy from, or ` + + `give the credential a '${refField}' in agentcore.json pointing at a secret you keep in ` + + `AWS Secrets Manager.`, + ); +} + +function missingPaymentSecrets( + name: string, + missing: string[], + rootPath: string, +): ProjectStateError { + return new ProjectStateError( + `Credential '${name}' is missing the values its payment provider needs: ` + + `${missing.join(", ")}. Set them in ${join(rootPath, ENV_LOCAL_RELATIVE_PATH)} or in the ` + + `environment you deploy from.`, + ); +} + +function requireArn(arn: string | undefined, name: string): string { + if (!arn) { + throw new MalformedServiceResponseError( + `Identity returned no credentialProviderArn for credential provider '${name}'`, + ); + } + return arn; +} diff --git a/src/core/project/envLocal.test.ts b/src/core/project/envLocal.test.ts index cb808c5d8..8fd87d96e 100644 --- a/src/core/project/envLocal.test.ts +++ b/src/core/project/envLocal.test.ts @@ -96,6 +96,19 @@ test("rejects a value that contains a single quote", async () => { ); }); +test("read returns {} when the file does not exist", async () => { + const root = await tempRoot(); + expect(await new EnvLocalFile(root).read()).toEqual({}); +}); + +test("read parses back the entries insertIfNew wrote", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await file.insertIfNew([{ key: "SECRET", value: "s k", comment: "c" }]); + + expect(await file.read()).toEqual({ SECRET: "s k" }); +}); + test("removeKeys deletes an entry and its comment while leaving neighbors", async () => { const root = await tempRoot(); const file = new EnvLocalFile(root); diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts index 720f3a5af..e81c96b29 100644 --- a/src/core/project/envLocal.ts +++ b/src/core/project/envLocal.ts @@ -1,5 +1,6 @@ import { chmod, rm } from "node:fs/promises"; import { join } from "node:path"; +import { parseEnv } from "node:util"; import { atomicWrite, readTextFile } from "../../io"; import { InputValidationError } from "../../errors"; import type { EnvLocalEntry } from "../../handlers/project/types"; @@ -66,6 +67,17 @@ export class EnvLocalFile { return { written, skipped }; } + /** + * Reads the file's entries as a key/value map, returning {} when the file + * does not exist. Values are read back with the same parser `agentcore dev` + * uses, so quoting written by {@link insertIfNew} round-trips. + */ + async read(): Promise> { + const content = await this.readOrNull(); + if (content === null) return {}; + return parseEnv(content); + } + /** * Deletes entries by key, along with the comment line `insertIfNew` wrote * directly above each one. Keys that are absent (or a file that does not diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 45b99d4d6..14c5faaac 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -78,12 +78,15 @@ import type { RuntimeResourceConfig } from "../../handlers/project/add/runtime/t import type { TemplateRenderer } from "./templates/types"; import { HandlebarsTemplateRenderer } from "./templates/renderer"; import type { CreateCloudFormationClient } from "../types"; +import { createIdentityClient } from "../identity"; +import type { CoreIdentityClient } from "../../handlers/identity/types"; const TARGETS_EXAMPLE = '[{ "name": "default", "account": "111122223333", "region": "us-east-1" }]'; type ProjectManagerConfig = { logger: Logger; createCloudFormationClient?: CreateCloudFormationClient; + identity?: CoreIdentityClient; source?: AssetSource; runner?: ProcessRunner; checkTool?: typeof requireTool; @@ -121,6 +124,7 @@ export class FsProjectManager implements ProjectManager { CDK: new CdkBackend({ logger: config.logger, createCloudFormationClient: config.createCloudFormationClient, + identity: config.identity ?? createIdentityClient(), runner: config.runner, checkTool: config.checkTool, json: config.json, diff --git a/src/core/types.tsx b/src/core/types.tsx index 9e36a7c17..f0ea899a5 100644 --- a/src/core/types.tsx +++ b/src/core/types.tsx @@ -7,13 +7,20 @@ import type { CloudFormationClientConfig, } from "@aws-sdk/client-cloudformation"; +// AwsCredentials is an explicit credential source for a call: either resolved +// credentials or a provider that resolves them. Callers that rely on the SDK's own +// default credential chain leave it unset. +export type AwsCredentials = NonNullable; + // CoreOptions is the standard trailing argument for Core operations. It carries // the per-call settings a handler resolves from context (the AWS region and an // optional endpoint URL override) and is translated into a ClientConfig by the -// sub-clients. +// sub-clients. `credentials` is for callers that must not use the default chain — +// a project deploy runs against its target's credentials. export interface CoreOptions { region: string; endpointUrl?: string; + credentials?: AwsCredentials; } // ClientConfig is the per-request configuration handed to the client factories. It @@ -22,11 +29,12 @@ export interface CoreOptions { export interface ClientConfig { region: string; endpoint?: string; + credentials?: AwsCredentials; } export type CredentialedClientConfig = { region: string; - credentials: NonNullable; + credentials: AwsCredentials; }; // Factories construct an SDK client from a ClientConfig. Injecting these (rather diff --git a/src/core/utils.tsx b/src/core/utils.tsx index 781ebdb36..51aa7eb2b 100644 --- a/src/core/utils.tsx +++ b/src/core/utils.tsx @@ -1,12 +1,14 @@ import type { ClientConfig, CoreOptions } from "./types"; // toClientConfig translates the caller-facing CoreOptions into the ClientConfig the -// SDK client factories expect. `endpoint` is only set when an override is provided -// so the SDK falls back to its default endpoint resolution otherwise. Shared by all -// Core sub-clients so they translate options consistently. +// SDK client factories expect. `endpoint` and `credentials` are only set when the +// caller provides them, so the SDK falls back to its default endpoint resolution and +// credential chain otherwise. Shared by all Core sub-clients so they translate +// options consistently. export function toClientConfig(options: CoreOptions): ClientConfig { return { region: options.region, ...(options.endpointUrl ? { endpoint: options.endpointUrl } : {}), + ...(options.credentials ? { credentials: options.credentials } : {}), }; } diff --git a/src/handlers/identity/types.tsx b/src/handlers/identity/types.tsx index 6bf0ce8a0..575ed40a3 100644 --- a/src/handlers/identity/types.tsx +++ b/src/handlers/identity/types.tsx @@ -13,6 +13,12 @@ import type { UpdateApiKeyCredentialProviderResponse, UpdateOauth2CredentialProviderRequest, UpdateOauth2CredentialProviderResponse, + CreatePaymentCredentialProviderRequest, + CreatePaymentCredentialProviderResponse, + DeletePaymentCredentialProviderResponse, + GetPaymentCredentialProviderResponse, + UpdatePaymentCredentialProviderRequest, + UpdatePaymentCredentialProviderResponse, } from "@aws-sdk/client-bedrock-agentcore-control"; import type { CoreOptions } from "../../core/types"; @@ -20,6 +26,8 @@ export type CreateApiKeyCredentialProviderInput = CreateApiKeyCredentialProvider export type UpdateApiKeyCredentialProviderInput = UpdateApiKeyCredentialProviderRequest; export type CreateOauth2CredentialProviderInput = CreateOauth2CredentialProviderRequest; export type UpdateOauth2CredentialProviderInput = UpdateOauth2CredentialProviderRequest; +export type CreatePaymentCredentialProviderInput = CreatePaymentCredentialProviderRequest; +export type UpdatePaymentCredentialProviderInput = UpdatePaymentCredentialProviderRequest; export interface CoreIdentityClient { createApiKeyCredentialProvider( @@ -65,4 +73,24 @@ export interface CoreIdentityClient { name: string, options: CoreOptions, ): Promise; + + // Payment credential providers hold a payment vendor's own credentials (a Coinbase + // CDP API key and wallet secret, or Privy app and authorization secrets). They are + // provisioned by `project deploy` rather than an `agentcore identity` subcommand. + createPaymentCredentialProvider( + input: CreatePaymentCredentialProviderInput, + options: CoreOptions, + ): Promise; + getPaymentCredentialProvider( + name: string, + options: CoreOptions, + ): Promise; + updatePaymentCredentialProvider( + input: UpdatePaymentCredentialProviderInput, + options: CoreOptions, + ): Promise; + deletePaymentCredentialProvider( + name: string, + options: CoreOptions, + ): Promise; } diff --git a/src/handlers/project/add/credentials/payment/index.test.ts b/src/handlers/project/add/credentials/payment/index.test.ts index 13bfe7322..a0e6cbc6f 100644 --- a/src/handlers/project/add/credentials/payment/index.test.ts +++ b/src/handlers/project/add/credentials/payment/index.test.ts @@ -213,14 +213,16 @@ describe("project add credentials payment", () => { expect((await projectSpec(projectRoot)).credentials).toHaveLength(1); }); - test("rejects credentials that generate overlapping environment variables", async () => { + test("rejects a credential that would overlap a payment credential's variables", async () => { const projectRoot = await inProject(); - await run(["add", "credentials", "api-key", "--name", "stripe_app_id"]); - await expect( - run(["add", "credentials", "payment", "--name", "stripe", "--provider", "StripePrivy"]), - ).rejects.toThrow("environment variable"); + // The only way to collide with a payment credential 'stripe' is to be named for + // one of its fields, so such a name is refused on its own — before a payment + // credential exists to collide with, and whether or not one ever does. + await expect(run(["add", "credentials", "api-key", "--name", "stripe_app_id"])).rejects.toThrow( + /_APP_ID/, + ); - expect((await projectSpec(projectRoot)).credentials).toHaveLength(1); + expect((await projectSpec(projectRoot)).credentials ?? []).toHaveLength(0); }); }); diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts index a36abd506..9c69ba293 100644 --- a/src/handlers/project/add/credentials/shared.ts +++ b/src/handlers/project/add/credentials/shared.ts @@ -6,6 +6,7 @@ import type { AddResourceInput } from "../../types"; import { credentialEnvironmentVariableNames, credentialEnvVarName, + credentialNameFieldSuffix, } from "../../../../projectSchemas/credential"; export { credentialEnvVarName }; @@ -50,6 +51,21 @@ export async function addCredentialToProject( ); } + // The collision check above only compares the fields credentials write today. A name + // ending in a field suffix would also shadow a field the CLI no longer writes but + // still reads (an OAuth client id in a pre-0.29 project) or one a later credential + // adds, so such a name is refused outright rather than when something clashes. + const fieldSuffix = credentialNameFieldSuffix(newName); + if (fieldSuffix) { + throw new InputValidationError( + `credential '${newName}' derives the environment variable ` + + `${credentialEnvVarName(newName)}, which ends in '${fieldSuffix}' — the suffix the CLI ` + + `appends to name one of a credential's own fields, so the variable would be ` + + `indistinguishable from that field of another credential. Choose a name that does not ` + + `end in '${fieldSuffix}' (hyphens count as underscores).`, + ); + } + for await (const event of config.projectManager.addResource(project, { resourceType: "credential", ...input, diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 328646ccb..d17f51c59 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -1095,6 +1095,24 @@ describe("project add credentials", () => { ); }); + test("rejects different credential types that collide on one secret variable", async () => { + await inProject(); + // OAuth 'foo' → AGENTCORE_CREDENTIAL_FOO_CLIENT_SECRET; api-key 'foo_client_secret' → the same. + await run(["add", "credentials", "oauth", "--name", "foo", "--discovery-url", discoveryUrl]); + await expect( + run(["add", "credentials", "api-key", "--name", "foo_client_secret"]), + ).rejects.toThrow(/same environment variable/); + }); + + test("rejects a name ending in a field suffix even with nothing to collide with", async () => { + await inProject(); + // Nothing in the spec derives AGENTCORE_CREDENTIAL_SVC_CLIENT_ID, but a pre-0.29 + // OAuth credential named 'svc' would read it as its client id. + await expect(run(["add", "credentials", "api-key", "--name", "svc-client-id"])).rejects.toThrow( + /_CLIENT_ID/, + ); + }); + test.each<[string, string[], RegExp]>([ [ "api-key: an inline secret value", diff --git a/src/projectSchemas/credential.test.ts b/src/projectSchemas/credential.test.ts index f27b4faaf..c1f22b306 100644 --- a/src/projectSchemas/credential.test.ts +++ b/src/projectSchemas/credential.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "bun:test"; -import { CredentialSchema, credentialEnvironmentVariableNames } from "./credential"; +import { + CredentialSchema, + credentialEnvironmentVariableNames, + credentialNameFieldSuffix, +} from "./credential"; const DISCOVERY_URL = "https://idp.example.com/.well-known/openid-configuration"; const SECRET_REF = { @@ -83,6 +87,17 @@ describe("credential schema", () => { expect(result.data).toMatchObject({ vendor: "CustomOauth2" }); }); + it("reports the field suffix a credential name would shadow", () => { + expect(credentialNameFieldSuffix("service-key")).toBeUndefined(); + expect(credentialNameFieldSuffix("svc-client-id")).toBe("_CLIENT_ID"); + expect(credentialNameFieldSuffix("svc_client_secret")).toBe("_CLIENT_SECRET"); + expect(credentialNameFieldSuffix("wallet-authorization-private-key")).toBe( + "_AUTHORIZATION_PRIVATE_KEY", + ); + // The suffix must terminate the name; carrying it in the middle is fine. + expect(credentialNameFieldSuffix("client-id-service")).toBeUndefined(); + }); + it("derives the environment variables used by each credential type", () => { const environmentNames = (value: Record) => credentialEnvironmentVariableNames(CredentialSchema.parse(value)); diff --git a/src/projectSchemas/credential.ts b/src/projectSchemas/credential.ts index 5fe17526b..afb162259 100644 --- a/src/projectSchemas/credential.ts +++ b/src/projectSchemas/credential.ts @@ -114,9 +114,46 @@ export const CredentialSchema = z.discriminatedUnion("authorizerType", [ ]); export type Credential = z.infer; +/** The prefix every variable carrying credential material shares. */ +export const CREDENTIAL_ENV_PREFIX = "AGENTCORE_CREDENTIAL_"; + /** Derives the .env.local variable name used for credential material. */ export function credentialEnvVarName(credentialName: string, suffix = ""): string { - return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; + return `${CREDENTIAL_ENV_PREFIX}${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; +} + +/** + * The suffixes the CLI appends to a credential's base variable to name one of its + * fields — both the secret-bearing ones and the readable identifiers. + */ +const CREDENTIAL_FIELD_SUFFIXES = [ + "_CLIENT_SECRET", + "_API_KEY_SECRET", + "_APP_SECRET", + "_WALLET_SECRET", + "_AUTHORIZATION_PRIVATE_KEY", + // Identifiers rather than secrets. `_CLIENT_ID` is no longer written — an OAuth + // client id lives in agentcore.json — but deploy still reads it for projects + // created before that move, so a name that produces it is still a hazard. + "_CLIENT_ID", + "_API_KEY_ID", + "_APP_ID", + "_AUTHORIZATION_ID", +] as const; + +/** + * Reports the field suffix a credential name ends in, if any. + * + * A credential named `svc_client_id` derives `AGENTCORE_CREDENTIAL_SVC_CLIENT_ID`, + * which is indistinguishable from the client id of an OAuth credential named `svc`. + * Rejecting such a name at creation fails closed: the collision check in + * {@link credentialEnvironmentVariableNames} only sees fields a credential currently + * writes, so it cannot catch a clash with a field written by an older CLI or added + * by a later one. + */ +export function credentialNameFieldSuffix(credentialName: string): string | undefined { + const normalized = credentialName.replace(/-/g, "_").toUpperCase(); + return CREDENTIAL_FIELD_SUFFIXES.find((suffix) => normalized.endsWith(suffix)); } /** Returns every .env.local key a credential reserves when it does not use an external secret. */ diff --git a/src/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index 6641c7159..01050ee82 100644 --- a/src/testing/TestCoreClient.tsx +++ b/src/testing/TestCoreClient.tsx @@ -63,6 +63,10 @@ import type { UpdateEvaluatorResponse, UpdateOnlineEvaluationConfigResponse, UpdateApiKeyCredentialProviderResponse, + CreatePaymentCredentialProviderResponse, + DeletePaymentCredentialProviderResponse, + GetPaymentCredentialProviderResponse, + UpdatePaymentCredentialProviderResponse, UpdateOauth2CredentialProviderResponse, UpdateGatewayResponse, UpdateGatewayRuleResponse, @@ -122,6 +126,8 @@ import type { import type { CoreIdentityClient, CreateApiKeyCredentialProviderInput, + CreatePaymentCredentialProviderInput, + UpdatePaymentCredentialProviderInput, CreateOauth2CredentialProviderInput, UpdateApiKeyCredentialProviderInput, UpdateOauth2CredentialProviderInput, @@ -228,6 +234,10 @@ const DEFAULT_LIST_API_KEYS_RESPONSE: ListApiKeyCredentialProvidersResponse = { }; const DEFAULT_UPDATE_API_KEY_RESPONSE = {} as UpdateApiKeyCredentialProviderResponse; const DEFAULT_DELETE_API_KEY_RESPONSE = {} as DeleteApiKeyCredentialProviderResponse; +const DEFAULT_CREATE_PAYMENT_RESPONSE = {} as CreatePaymentCredentialProviderResponse; +const DEFAULT_GET_PAYMENT_RESPONSE = {} as GetPaymentCredentialProviderResponse; +const DEFAULT_UPDATE_PAYMENT_RESPONSE = {} as UpdatePaymentCredentialProviderResponse; +const DEFAULT_DELETE_PAYMENT_RESPONSE = {} as DeletePaymentCredentialProviderResponse; const DEFAULT_GET_MEMORY_RESPONSE = {} as GetMemoryOutput; const DEFAULT_LIST_MEMORIES_RESPONSE: ListMemoriesOutput = { memories: [] }; const DEFAULT_GET_EVENT_RESPONSE: GetEventOutput = { event: undefined }; @@ -1245,8 +1255,14 @@ export class TestIdentityClient implements CoreIdentityClient { >(); private updateOauth2Response: UpdateOauth2CredentialProviderResponse = DEFAULT_UPDATE_OAUTH2_RESPONSE; + private getPaymentResponse: GetPaymentCredentialProviderResponse = DEFAULT_GET_PAYMENT_RESPONSE; private error?: Error; + setGetPaymentResponse(response: GetPaymentCredentialProviderResponse): this { + this.getPaymentResponse = response; + return this; + } + setGetApiKeyResponse(response: GetApiKeyCredentialProviderResponse): this { this.getApiKeyResponse = response; return this; @@ -1388,6 +1404,42 @@ export class TestIdentityClient implements CoreIdentityClient { if (this.error) throw this.error; return DEFAULT_DELETE_OAUTH2_RESPONSE; } + + async createPaymentCredentialProvider( + input: CreatePaymentCredentialProviderInput, + options: CoreOptions, + ): Promise { + this.calls.push({ method: "createPaymentCredentialProvider", args: [input, options] }); + if (this.error) throw this.error; + return DEFAULT_CREATE_PAYMENT_RESPONSE; + } + + async getPaymentCredentialProvider( + name: string, + options: CoreOptions, + ): Promise { + this.calls.push({ method: "getPaymentCredentialProvider", args: [name, options] }); + if (this.error) throw this.error; + return this.getPaymentResponse; + } + + async updatePaymentCredentialProvider( + input: UpdatePaymentCredentialProviderInput, + options: CoreOptions, + ): Promise { + this.calls.push({ method: "updatePaymentCredentialProvider", args: [input, options] }); + if (this.error) throw this.error; + return DEFAULT_UPDATE_PAYMENT_RESPONSE; + } + + async deletePaymentCredentialProvider( + name: string, + options: CoreOptions, + ): Promise { + this.calls.push({ method: "deletePaymentCredentialProvider", args: [name, options] }); + if (this.error) throw this.error; + return DEFAULT_DELETE_PAYMENT_RESPONSE; + } } // TestEvalClient is the eval sub-client of TestCoreClient.