From 3c09c23e85fa1d7703668ab73c1b72acc1acf588 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Thu, 27 Aug 2026 00:19:56 +0000 Subject: [PATCH 01/13] feat: create credential providers before synthesizing a deploy The synthesized CDK app reads credential provider ARNs out of deployed-state.json and fails to synth a project that declares credentials until they exist. Provision them between the account preflight and the build, then record their ARNs via updateTargetState so the assembly is synthesized against a state file that already describes them. Providers are created when absent and reused when present, never updated, so a redeploy neither mints a new secret version nor overwrites one rotated outside the CLI. Payment credentials are rejected up front (agentcore.json can't express the vendor config they need). Secrets come from the same place 'project add credentials' writes them, so the env-var name is now derived from one function in envLocal.ts that both sides share. --- src/core/project/backends/cdk.test.ts | 41 +++ src/core/project/backends/cdk.ts | 17 + .../project/backends/cdk/credentials.test.ts | 298 ++++++++++++++++ src/core/project/backends/cdk/credentials.ts | 337 ++++++++++++++++++ src/core/project/envLocal.test.ts | 22 +- src/core/project/envLocal.ts | 27 ++ .../project/add/credentials/oauth/index.ts | 9 +- .../project/add/credentials/shared.ts | 8 +- 8 files changed, 752 insertions(+), 7 deletions(-) create mode 100644 src/core/project/backends/cdk/credentials.test.ts create mode 100644 src/core/project/backends/cdk/credentials.ts diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 288bb2199..ba992de44 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -7,6 +7,7 @@ import type { DeployResult, Project, ProjectEvent } from "../../../handlers/proj import { ProjectSpecSchema } from "../../../projectSchemas/project"; import { createSilentLogger } from "../../../testing"; import { CdkBackend } from "./cdk"; +import type { CredentialProvisioner } from "./cdk/credentials"; import { DEPLOYED_STATE_RELATIVE_PATH } from "./cdk/deployedState"; import type { BootstrapState } from "./cdk/environment"; import type { CdkCredentialProvider, CdkOperation, CdkOutputs, CdkRunOptions } from "./cdk/toolkit"; @@ -85,6 +86,7 @@ type HarnessOptions = { template?: boolean; failOperation?: CdkOperation["kind"]; bootstrapError?: Error; + provisionCredentials?: CredentialProvisioner; }; function harness(options: HarnessOptions = {}) { @@ -152,6 +154,7 @@ function harness(options: HarnessOptions = {}) { }, }; }, + ...(options.provisionCredentials && { provisionCredentials: options.provisionCredentials }), }); return { @@ -276,6 +279,44 @@ describe("CdkBackend.deploy", () => { }); }); + 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, { target: TARGET })); + + // 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 () => { const input = await project(); await writeAssembly(input, [TARGET.name]); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index ee160263e..063c3a8e4 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -12,6 +12,7 @@ import { import type { Logger } from "../../../logging"; import type { DeployBackendInput, ProjectBackend } from "./types"; import { stackArtifactIdForTarget } from "./cdk/assembly"; +import { createCredentialProvisioner, type CredentialProvisioner } from "./cdk/credentials"; import { readDeployedState, updateTargetState } from "./cdk/deployedState"; import { probeBootstrap, @@ -38,6 +39,7 @@ export type CdkBackendConfig = { bootstrap?: BootstrapProbe; resolveAccount?: AccountResolver; loadBootstrapTemplate?: BootstrapTemplateLoader; + provisionCredentials?: CredentialProvisioner; }; /** Builds and deploys projects through the scaffolded CDK app. */ @@ -51,6 +53,7 @@ export class CdkBackend implements ProjectBackend { private readonly bootstrap: BootstrapProbe; private readonly resolveAccount: AccountResolver; private readonly loadBootstrapTemplate: BootstrapTemplateLoader; + private readonly provisionCredentials: CredentialProvisioner; constructor(config: CdkBackendConfig) { this.logger = config.logger; @@ -63,6 +66,7 @@ export class CdkBackend implements ProjectBackend { this.bootstrap = config.bootstrap ?? probeBootstrap; this.resolveAccount = config.resolveAccount ?? resolveAwsAccount; this.loadBootstrapTemplate = config.loadBootstrapTemplate ?? loadBootstrapTemplate; + this.provisionCredentials = config.provisionCredentials ?? createCredentialProvisioner(); } public async *build(project: Project): AsyncGenerator { @@ -106,6 +110,19 @@ export class CdkBackend implements ProjectBackend { // with the new stack ARN unrecorded because the post-deploy write can't parse it. await readDeployedState(this.json, project.rootPath); + // Credential providers exist before synthesis, not as part of the stack: the + // synthesized app reads their ARNs out of deployed-state.json, so a project + // declaring credentials cannot synthesize until they have been recorded. + const provisioned = yield* this.provisionCredentials(project, { + credentials, + region: target.region, + }); + if (Object.keys(provisioned).length > 0) { + await updateTargetState(this.json, project.rootPath, target.name, { + resources: { credentials: provisioned }, + }); + } + yield* this.build(project); const assemblyDirectory = this.assemblyDirectory(project); const stackArtifactId = await stackArtifactIdForTarget( 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..6500fc6a1 --- /dev/null +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -0,0 +1,298 @@ +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 type { Project, ProjectEvent } from "../../../../handlers/project/types"; +import { ProjectSpecSchema } from "../../../../projectSchemas/project"; +import { EnvLocalFile } from "../../envLocal"; +import { + createCredentialProvisioner, + type CredentialProvisioner, + type DeployedCredential, + type DeployedCredentials, + type IdentityProviderClient, +} from "./credentials"; +import type { CdkCredentialProvider } from "./toolkit"; + +const REGION = "us-east-1"; +const CREDENTIALS: CdkCredentialProvider = async () => ({ + accessKeyId: "access-key", + secretAccessKey: "secret-key", +}); + +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 }; + +function identity(existing: DeployedCredentials = {}) { + const calls: Call[] = []; + const factoryArgs: { region: string; credentials: CdkCredentialProvider }[] = []; + + const created = (name: string, prefix: string): DeployedCredential => ({ + credentialProviderArn: `arn:${prefix}:${name}`, + clientSecretArn: `arn:secret:${name}`, + }); + + const client: IdentityProviderClient = { + async getApiKeyProvider(name) { + calls.push({ kind: "getApiKey", input: name }); + return existing[name]; + }, + async createApiKeyProvider(input) { + calls.push({ kind: "createApiKey", input }); + return created(input.name, "apikey"); + }, + async getOauth2Provider(name) { + calls.push({ kind: "getOauth2", input: name }); + return existing[name]; + }, + async createOauth2Provider(input) { + calls.push({ kind: "createOauth2", input }); + return created(input.name, "oauth"); + }, + }; + + return { + calls, + factoryArgs, + provision: createCredentialProvisioner(async (region, credentials) => { + factoryArgs.push({ region, credentials }); + return client; + }), + }; +} + +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("does not build a client 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.factoryArgs).toEqual([]); + }); + + test("builds the client 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.factoryArgs).toEqual([{ region: REGION, credentials: CREDENTIALS }]); + }); + + 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).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).toEqual([ + { kind: "getApiKey", input: "openai-key" }, + { kind: "createApiKey", input: { name: "openai-key", secretRef } }, + ]); + }); + + 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("reuses a provider that already exists instead of recreating it", 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-live'\n"); + + const { result } = await run(subject.provision, input); + + expect(subject.calls).toEqual([{ kind: "getApiKey", input: "openai-key" }]); + expect(result).toEqual({ "openai-key": existing }); + }); + + 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).toEqual([ + { kind: "getOauth2", input: "my-oauth" }, + { + kind: "createOauth2", + input: { + name: "my-oauth", + vendor: "CustomOauth2", + config: { + 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("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]).toEqual({ + kind: "createOauth2", + input: { + name: "vendored", + vendor: "GoogleOauth2", + config: { + 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]).toEqual({ + kind: "createOauth2", + input: { + name: "my-oauth", + vendor: "CustomOauth2", + config: { + 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("rejects a payment credential before creating any provider", async () => { + const subject = identity(); + const input = await project( + [ + API_KEY, + { authorizerType: "PaymentCredentialProvider", name: "pay-1", provider: "StripePrivy" }, + ], + "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n", + ); + + await expect(run(subject.provision, input)).rejects.toThrow( + /PaymentCredentialProvider, which 'agentcore project deploy' cannot create/, + ); + expect(subject.calls).toEqual([]); + }); +}); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts new file mode 100644 index 000000000..a0739930a --- /dev/null +++ b/src/core/project/backends/cdk/credentials.ts @@ -0,0 +1,337 @@ +import { join } from "node:path"; +import type { Oauth2ProviderConfigInput } from "@aws-sdk/client-bedrock-agentcore-control"; +import { MalformedServiceResponseError, ProjectStateError } from "../../../../errors/errors"; +import type { Project, ProjectEvent } from "../../../../handlers/project/types"; +import type { + ApiKeyCredential, + Credential, + OAuthCredential, + SecretReference, +} from "../../../../projectSchemas/credential"; +import { + CLIENT_SECRET_SUFFIX, + credentialEnvVarName, + 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; + +export type ApiKeyProviderInput = { + name: string; + /** Inline key material; mutually exclusive with `secretRef`. */ + apiKey?: string; + /** An existing Secrets Manager secret the customer manages themselves. */ + secretRef?: SecretReference; +}; + +export type Oauth2ProviderInput = { + name: string; + vendor: string; + config: Oauth2ProviderConfigInput; +}; + +/** + * The Identity calls credential provisioning needs. Narrowed to four methods so + * tests can substitute a recorder without standing up the SDK client, following + * the seam style of the other backend collaborators in this directory. + */ +export type IdentityProviderClient = { + getApiKeyProvider(name: string): Promise; + createApiKeyProvider(input: ApiKeyProviderInput): Promise; + getOauth2Provider(name: string): Promise; + createOauth2Provider(input: Oauth2ProviderInput): Promise; +}; + +export type IdentityProviderClientFactory = ( + region: string, + credentials: CdkCredentialProvider, +) => Promise; + +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; + +/** + * Builds an Identity client against the deployment target's own credentials. + * The SDK is imported lazily so projects without credentials never pay for + * loading it, matching how the CloudFormation and STS clients are built. + */ +export const createIdentityProviderClient: IdentityProviderClientFactory = async ( + region, + credentials, +) => { + const { + BedrockAgentCoreControlClient, + CreateApiKeyCredentialProviderCommand, + CreateOauth2CredentialProviderCommand, + GetApiKeyCredentialProviderCommand, + GetOauth2CredentialProviderCommand, + ResourceNotFoundException, + } = await import("@aws-sdk/client-bedrock-agentcore-control"); + const client = new BedrockAgentCoreControlClient({ credentials, region }); + + // A missing provider is the normal first-deploy case, not a failure. + const undefinedWhenAbsent = async (send: () => Promise): Promise => { + try { + return await send(); + } catch (error) { + if (error instanceof ResourceNotFoundException) return undefined; + throw error; + } + }; + + return { + async getApiKeyProvider(name) { + const response = await undefinedWhenAbsent(() => + client.send(new GetApiKeyCredentialProviderCommand({ name })), + ); + if (!response) return undefined; + return { + credentialProviderArn: requireArn(response.credentialProviderArn, name), + ...(response.apiKeySecretArn?.secretArn && { + clientSecretArn: response.apiKeySecretArn.secretArn, + }), + }; + }, + async createApiKeyProvider({ name, apiKey, secretRef }) { + const response = await client.send( + new CreateApiKeyCredentialProviderCommand({ + name, + ...(apiKey !== undefined && { apiKey }), + ...(secretRef && { apiKeySecretConfig: secretRef, apiKeySecretSource: "EXTERNAL" }), + }), + ); + return { + credentialProviderArn: requireArn(response.credentialProviderArn, name), + ...(response.apiKeySecretArn?.secretArn && { + clientSecretArn: response.apiKeySecretArn.secretArn, + }), + }; + }, + async getOauth2Provider(name) { + const response = await undefinedWhenAbsent(() => + client.send(new GetOauth2CredentialProviderCommand({ name })), + ); + if (!response) return undefined; + return { + credentialProviderArn: requireArn(response.credentialProviderArn, name), + ...(response.clientSecretArn?.secretArn && { + clientSecretArn: response.clientSecretArn.secretArn, + }), + }; + }, + async createOauth2Provider({ name, vendor, config }) { + const response = await client.send( + new CreateOauth2CredentialProviderCommand({ + 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: vendor as never, + oauth2ProviderConfigInput: config, + }), + ); + return { + credentialProviderArn: requireArn(response.credentialProviderArn, name), + ...(response.clientSecretArn?.secretArn && { + clientSecretArn: response.clientSecretArn.secretArn, + }), + }; + }, + }; +}; + +/** + * Creates the credential providers a project declares, before its CloudFormation + * templates are synthesized: the synthesized app reads the resulting ARNs out of + * `deployed-state.json` and cannot synthesize a project with credentials until + * they exist. + * + * Providers are created when absent and reused when already present, never + * updated. Reuse keeps a deploy from minting a new secret version each run and + * from overwriting a secret rotated outside the CLI; reconciling a provider + * whose declaration has since changed is deliberately left to a later change. + */ +export function createCredentialProvisioner( + createClient: IdentityProviderClientFactory = createIdentityProviderClient, +): CredentialProvisioner { + return async function* provisionCredentials(project, { region, credentials }) { + const declared = project.spec.credentials; + if (declared.length === 0) return {}; + + // Rejected up front so a project with an unsupported credential fails before + // any provider is created, rather than part-way through the list. + const payment = declared.find((c) => c.authorizerType === "PaymentCredentialProvider"); + if (payment) throw paymentUnsupported(payment.name); + + const env = await new EnvLocalFile(project.rootPath).read(); + const client = await createClient(region, credentials); + + const provisioned: DeployedCredentials = {}; + for (const credential of declared) { + // Provider names are account-global, so a name already taken by another + // project in this account is adopted rather than recreated. + yield { message: `Preparing credential provider '${credential.name}'` }; + provisioned[credential.name] = await provisionOne(client, credential, env, project.rootPath); + } + return provisioned; + }; +} + +async function provisionOne( + client: IdentityProviderClient, + credential: Credential, + env: Record, + rootPath: string, +): Promise { + switch (credential.authorizerType) { + case "ApiKeyCredentialProvider": + return provisionApiKey(client, credential, env, rootPath); + case "OAuthCredentialProvider": + return provisionOauth2(client, credential, env, rootPath); + case "PaymentCredentialProvider": + // Unreachable: rejected before provisioning starts. + throw paymentUnsupported(credential.name); + } +} + +async function provisionApiKey( + client: IdentityProviderClient, + credential: ApiKeyCredential, + env: Record, + rootPath: string, +): Promise { + const existing = await client.getApiKeyProvider(credential.name); + if (existing) return existing; + + if (credential.secretRef) { + return client.createApiKeyProvider({ name: credential.name, secretRef: credential.secretRef }); + } + + const envKey = credentialEnvVarName(credential.name); + const apiKey = env[envKey]; + if (!apiKey) throw missingSecret(credential.name, envKey, "secretRef", rootPath); + return client.createApiKeyProvider({ name: credential.name, apiKey }); +} + +async function provisionOauth2( + client: IdentityProviderClient, + credential: OAuthCredential, + env: Record, + rootPath: string, +): Promise { + const existing = await client.getOauth2Provider(credential.name); + if (existing) return existing; + + let secret: Record; + if (credential.clientSecretRef) { + secret = { clientSecretConfig: credential.clientSecretRef, clientSecretSource: "EXTERNAL" }; + } else { + const envKey = credentialEnvVarName(credential.name, CLIENT_SECRET_SUFFIX); + const clientSecret = env[envKey]; + if (!clientSecret) throw missingSecret(credential.name, envKey, "clientSecretRef", rootPath); + secret = { clientSecret }; + } + + return client.createOauth2Provider({ + name: credential.name, + vendor: credential.vendor, + config: credential.providerConfig + ? vendorConfigWithSecret(credential.name, credential.providerConfig, secret) + : guidedCustomConfig(credential, secret), + }); +} + +/** + * 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, + 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 }, + ...(credential.clientId !== undefined && { clientId: credential.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 give the credential a '${refField}' in ` + + `agentcore.json pointing at a secret you keep in AWS Secrets Manager.`, + ); +} + +function paymentUnsupported(name: string): ProjectStateError { + return new ProjectStateError( + `Credential '${name}' is a PaymentCredentialProvider, which 'agentcore project deploy' ` + + `cannot create: a payment provider needs vendor configuration (API key, wallet and ` + + `authorization secrets) that agentcore.json has no fields for. Remove it from the project ` + + `spec to deploy the rest of the project.`, + ); +} + +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 44834806d..4da8e4a13 100644 --- a/src/core/project/envLocal.test.ts +++ b/src/core/project/envLocal.test.ts @@ -4,7 +4,7 @@ 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"; +import { CLIENT_SECRET_SUFFIX, credentialEnvVarName, EnvLocalFile } from "./envLocal"; const roots: string[] = []; afterEach(async () => { @@ -75,3 +75,23 @@ test("rejects a value that contains a single quote", async () => { /single quote/, ); }); + +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("credentialEnvVarName upcases, replaces hyphens, and appends the suffix", () => { + expect(credentialEnvVarName("openai-key")).toBe("AGENTCORE_CREDENTIAL_OPENAI_KEY"); + expect(credentialEnvVarName("my-oauth", CLIENT_SECRET_SUFFIX)).toBe( + "AGENTCORE_CREDENTIAL_MY_OAUTH_CLIENT_SECRET", + ); +}); diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts index 5774dde36..5581d55c0 100644 --- a/src/core/project/envLocal.ts +++ b/src/core/project/envLocal.ts @@ -1,5 +1,6 @@ import { 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"; @@ -9,6 +10,19 @@ export const ENV_LOCAL_RELATIVE_PATH = join("agentcore", ".env.local"); const KEY_LINE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/; +/** Suffix distinguishing an OAuth credential's client secret from an API key. */ +export const CLIENT_SECRET_SUFFIX = "_CLIENT_SECRET"; + +/** + * Derives the variable name a credential's secret is stored under. This is the + * only contract between `project add credentials` (which writes the entry) and + * `project deploy` (which reads it back to create the provider), so both sides + * derive the name here rather than formatting it themselves. + */ +export function credentialEnvVarName(credentialName: string, suffix = ""): string { + return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; +} + /** * The project's `.env.local` secrets file, edited transactionally. `insertIfNew` * appends entries (never overwriting an existing key) and snapshots the prior @@ -63,6 +77,19 @@ 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 {}; + // parseEnv types values as string | undefined for repeated keys; the last + // assignment wins and only string values are ever produced. + return parseEnv(content) as Record; + } + /** Restores the file to its pre-write state; a no-op when nothing was written. */ async rollback(): Promise { if (this.snapshot === undefined) return; diff --git a/src/handlers/project/add/credentials/oauth/index.ts b/src/handlers/project/add/credentials/oauth/index.ts index 96c71fe58..818f10699 100644 --- a/src/handlers/project/add/credentials/oauth/index.ts +++ b/src/handlers/project/add/credentials/oauth/index.ts @@ -9,7 +9,12 @@ import { } from "../../../../identity/oauth2-credential-provider/config"; import type { AddProjectResourceConfig } from "../../types"; import type { EnvLocalEntry } from "../../../types"; -import { addCredentialToProject, credentialEnvVarName, parseExclusiveSecretRef } from "../shared"; +import { + addCredentialToProject, + CLIENT_SECRET_SUFFIX, + credentialEnvVarName, + parseExclusiveSecretRef, +} from "../shared"; export const createAddOauthCredentialHandler = (config: AddProjectResourceConfig) => createHandler({ @@ -95,7 +100,7 @@ export const createAddOauthCredentialHandler = (config: AddProjectResourceConfig ? [] : [ { - key: credentialEnvVarName(flags.name, "_CLIENT_SECRET"), + key: credentialEnvVarName(flags.name, CLIENT_SECRET_SUFFIX), value: clientSecret, comment: `OAuth client secret for credential provider '${flags.name}' (set before deploy)`, }, diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts index a9a1445c9..b4f05c554 100644 --- a/src/handlers/project/add/credentials/shared.ts +++ b/src/handlers/project/add/credentials/shared.ts @@ -1,13 +1,13 @@ import { ProjectKey, type Context } from "../../../../router"; import { InputValidationError } from "../../../../errors"; +import { CLIENT_SECRET_SUFFIX, credentialEnvVarName } from "../../../../core/project/envLocal"; 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}`; -} +// Re-exported so the add handlers and `project deploy` derive secret variable +// names from one definition: deploy reads back exactly what add writes. +export { CLIENT_SECRET_SUFFIX, credentialEnvVarName }; /** Parses a secret-reference flag, rejecting a directly supplied secret alongside it. */ export function parseExclusiveSecretRef( From a5cd14b3241c375bf6004b784a47058a067e1d5c Mon Sep 17 00:00:00 2001 From: gitikavj Date: Thu, 27 Aug 2026 16:50:27 +0000 Subject: [PATCH 02/13] test+fix: cover identity client, clear dropped credentials, honest env type - Add SDK-mocked coverage for createIdentityProviderClient (the real Identity factory the provisioner tests bypass): ~55% -> ~95% on credentials.ts. - Always record the provisioned credential set, so removing the last credential from the spec clears the stale entry instead of leaving it advertised. - EnvLocalFile.read returns Record (parseEnv's real type) rather than casting it away. - Tighten a few verbose comments. --- src/core/project/backends/cdk.test.ts | 11 +- src/core/project/backends/cdk.ts | 15 +- .../backends/cdk/credentials.client.test.ts | 146 ++++++++++++++++++ src/core/project/backends/cdk/credentials.ts | 31 ++-- src/core/project/envLocal.ts | 13 +- 5 files changed, 178 insertions(+), 38 deletions(-) create mode 100644 src/core/project/backends/cdk/credentials.client.test.ts diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index ba992de44..e8147809e 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"; @@ -272,6 +271,7 @@ describe("CdkBackend.deploy", () => { 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", }, @@ -317,7 +317,7 @@ describe("CdkBackend.deploy", () => { }); }); - 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 }); @@ -325,7 +325,12 @@ describe("CdkBackend.deploy", () => { await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).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("fails before touching AWS when the existing state file is malformed", async () => { diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index 063c3a8e4..40376c4b6 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -110,18 +110,17 @@ export class CdkBackend implements ProjectBackend { // with the new stack ARN unrecorded because the post-deploy write can't parse it. await readDeployedState(this.json, project.rootPath); - // Credential providers exist before synthesis, not as part of the stack: the - // synthesized app reads their ARNs out of deployed-state.json, so a project - // declaring credentials cannot synthesize until they have been recorded. + // 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, }); - if (Object.keys(provisioned).length > 0) { - await updateTargetState(this.json, project.rootPath, target.name, { - resources: { credentials: provisioned }, - }); - } + // 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); diff --git a/src/core/project/backends/cdk/credentials.client.test.ts b/src/core/project/backends/cdk/credentials.client.test.ts new file mode 100644 index 000000000..4b2d6cda1 --- /dev/null +++ b/src/core/project/backends/cdk/credentials.client.test.ts @@ -0,0 +1,146 @@ +import { afterEach, describe, expect, mock, test } from "bun:test"; + +// credentials.test.ts drives the provisioner with a fake client; this covers the +// real factory by mocking the AWS SDK it lazily imports. + +class ResourceNotFoundException extends Error { + constructor() { + super("not found"); + this.name = "ResourceNotFoundException"; + } +} +class GetApiKeyCredentialProviderCommand { + constructor(readonly input: unknown) {} +} +class CreateApiKeyCredentialProviderCommand { + constructor(readonly input: unknown) {} +} +class GetOauth2CredentialProviderCommand { + constructor(readonly input: unknown) {} +} +class CreateOauth2CredentialProviderCommand { + constructor(readonly input: unknown) {} +} + +const sent: unknown[] = []; +let send: (command: unknown) => Promise; + +class BedrockAgentCoreControlClient { + constructor(readonly config: unknown) {} + send(command: unknown) { + sent.push(command); + return send(command); + } +} + +mock.module("@aws-sdk/client-bedrock-agentcore-control", () => ({ + BedrockAgentCoreControlClient, + GetApiKeyCredentialProviderCommand, + CreateApiKeyCredentialProviderCommand, + GetOauth2CredentialProviderCommand, + CreateOauth2CredentialProviderCommand, + ResourceNotFoundException, +})); + +const { createIdentityProviderClient } = await import("./credentials"); +const credentials = async () => ({ accessKeyId: "a", secretAccessKey: "b" }); + +afterEach(() => { + sent.length = 0; +}); + +describe("createIdentityProviderClient", () => { + test("passes region and credentials to the SDK client", async () => { + send = async () => ({ credentialProviderArn: "arn:cp" }); + const client = await createIdentityProviderClient("eu-west-1", credentials); + await client.getApiKeyProvider("k"); + + expect((sent[0] as GetApiKeyCredentialProviderCommand).input).toEqual({ name: "k" }); + }); + + test("maps an API key provider, including its secret ARN", async () => { + send = async () => ({ + credentialProviderArn: "arn:cp", + apiKeySecretArn: { secretArn: "arn:secret" }, + }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + expect(await client.getApiKeyProvider("k")).toEqual({ + credentialProviderArn: "arn:cp", + clientSecretArn: "arn:secret", + }); + }); + + test("returns undefined when the provider does not exist", async () => { + send = async () => { + throw new ResourceNotFoundException(); + }; + const client = await createIdentityProviderClient("us-east-1", credentials); + + expect(await client.getApiKeyProvider("missing")).toBeUndefined(); + expect(await client.getOauth2Provider("missing")).toBeUndefined(); + }); + + test("propagates errors other than not-found", async () => { + const failure = Object.assign(new Error("denied"), { name: "AccessDeniedException" }); + send = async () => { + throw failure; + }; + const client = await createIdentityProviderClient("us-east-1", credentials); + + await expect(client.getApiKeyProvider("k")).rejects.toBe(failure); + }); + + test("throws when Identity returns no provider ARN", async () => { + send = async () => ({}); + const client = await createIdentityProviderClient("us-east-1", credentials); + + await expect(client.createApiKeyProvider({ name: "k", apiKey: "sk" })).rejects.toThrow( + /no credentialProviderArn/, + ); + }); + + test("creates an API key provider from an inline key", async () => { + send = async () => ({ credentialProviderArn: "arn:cp" }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + await client.createApiKeyProvider({ name: "k", apiKey: "sk-live" }); + + expect((sent[0] as CreateApiKeyCredentialProviderCommand).input).toEqual({ + name: "k", + apiKey: "sk-live", + }); + }); + + test("creates an API key provider from an external secret reference", async () => { + send = async () => ({ credentialProviderArn: "arn:cp" }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + const secretRef = { secretId: "s", jsonKey: "apiKey" }; + await client.createApiKeyProvider({ name: "k", secretRef }); + + expect((sent[0] as CreateApiKeyCredentialProviderCommand).input).toEqual({ + name: "k", + apiKeySecretConfig: secretRef, + apiKeySecretSource: "EXTERNAL", + }); + }); + + test("creates an OAuth2 provider with its vendor and config", async () => { + send = async () => ({ + credentialProviderArn: "arn:cp", + clientSecretArn: { secretArn: "arn:secret" }, + }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + const config = { customOauth2ProviderConfig: { oauthDiscovery: { discoveryUrl: "u" } } }; + const result = await client.createOauth2Provider({ name: "o", vendor: "CustomOauth2", config }); + + expect((sent[0] as CreateOauth2CredentialProviderCommand).input).toEqual({ + name: "o", + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: config, + }); + expect(result).toEqual({ credentialProviderArn: "arn:cp", clientSecretArn: "arn:secret" }); + }); +}); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index a0739930a..aa5a95c4e 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -37,11 +37,7 @@ export type Oauth2ProviderInput = { config: Oauth2ProviderConfigInput; }; -/** - * The Identity calls credential provisioning needs. Narrowed to four methods so - * tests can substitute a recorder without standing up the SDK client, following - * the seam style of the other backend collaborators in this directory. - */ +/** The Identity calls provisioning needs — four methods, so tests inject a fake instead of the SDK client. */ export type IdentityProviderClient = { getApiKeyProvider(name: string): Promise; createApiKeyProvider(input: ApiKeyProviderInput): Promise; @@ -66,9 +62,8 @@ export type CredentialProvisioner = ( ) => AsyncGenerator; /** - * Builds an Identity client against the deployment target's own credentials. - * The SDK is imported lazily so projects without credentials never pay for - * loading it, matching how the CloudFormation and STS clients are built. + * Builds an Identity client for the target's credentials. The SDK is imported + * lazily so projects without credentials never pay to load it. */ export const createIdentityProviderClient: IdentityProviderClientFactory = async ( region, @@ -155,15 +150,13 @@ export const createIdentityProviderClient: IdentityProviderClientFactory = async }; /** - * Creates the credential providers a project declares, before its CloudFormation - * templates are synthesized: the synthesized app reads the resulting ARNs out of - * `deployed-state.json` and cannot synthesize a project with credentials until - * they exist. + * 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. * - * Providers are created when absent and reused when already present, never - * updated. Reuse keeps a deploy from minting a new secret version each run and - * from overwriting a secret rotated outside the CLI; reconciling a provider - * whose declaration has since changed is deliberately left to a later change. + * Created when absent, reused when present, never updated — so a redeploy neither + * mints a new secret version nor overwrites one rotated outside the CLI. + * Reconciling a changed declaration is left to a later change. */ export function createCredentialProvisioner( createClient: IdentityProviderClientFactory = createIdentityProviderClient, @@ -194,7 +187,7 @@ export function createCredentialProvisioner( async function provisionOne( client: IdentityProviderClient, credential: Credential, - env: Record, + env: Record, rootPath: string, ): Promise { switch (credential.authorizerType) { @@ -211,7 +204,7 @@ async function provisionOne( async function provisionApiKey( client: IdentityProviderClient, credential: ApiKeyCredential, - env: Record, + env: Record, rootPath: string, ): Promise { const existing = await client.getApiKeyProvider(credential.name); @@ -230,7 +223,7 @@ async function provisionApiKey( async function provisionOauth2( client: IdentityProviderClient, credential: OAuthCredential, - env: Record, + env: Record, rootPath: string, ): Promise { const existing = await client.getOauth2Provider(credential.name); diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts index 5581d55c0..83847e508 100644 --- a/src/core/project/envLocal.ts +++ b/src/core/project/envLocal.ts @@ -14,10 +14,9 @@ const KEY_LINE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/; export const CLIENT_SECRET_SUFFIX = "_CLIENT_SECRET"; /** - * Derives the variable name a credential's secret is stored under. This is the - * only contract between `project add credentials` (which writes the entry) and - * `project deploy` (which reads it back to create the provider), so both sides - * derive the name here rather than formatting it themselves. + * The `.env.local` variable name a credential's secret is stored under — the one + * contract between `add credentials` (writes it) and `deploy` (reads it), so both + * derive it here rather than formatting their own. */ export function credentialEnvVarName(credentialName: string, suffix = ""): string { return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; @@ -82,12 +81,10 @@ export class EnvLocalFile { * 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> { + async read(): Promise> { const content = await this.readOrNull(); if (content === null) return {}; - // parseEnv types values as string | undefined for repeated keys; the last - // assignment wins and only string values are ever produced. - return parseEnv(content) as Record; + return parseEnv(content); } /** Restores the file to its pre-write state; a no-op when nothing was written. */ From 2c4726667a8bfaeb48910c4280d085efbe28b3aa Mon Sep 17 00:00:00 2001 From: gitikavj Date: Thu, 27 Aug 2026 16:57:56 +0000 Subject: [PATCH 03/13] test: cover the identity factory's found + secret-ARN mapping branches --- .../backends/cdk/credentials.client.test.ts | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/core/project/backends/cdk/credentials.client.test.ts b/src/core/project/backends/cdk/credentials.client.test.ts index 4b2d6cda1..6a808c3fb 100644 --- a/src/core/project/backends/cdk/credentials.client.test.ts +++ b/src/core/project/backends/cdk/credentials.client.test.ts @@ -71,6 +71,26 @@ describe("createIdentityProviderClient", () => { }); }); + test("maps an OAuth2 provider it finds, including its secret ARN", async () => { + send = async () => ({ + credentialProviderArn: "arn:cp", + clientSecretArn: { secretArn: "arn:secret" }, + }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + expect(await client.getOauth2Provider("o")).toEqual({ + credentialProviderArn: "arn:cp", + clientSecretArn: "arn:secret", + }); + }); + + test("omits the secret ARN when Identity returns none", async () => { + send = async () => ({ credentialProviderArn: "arn:cp" }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + expect(await client.getApiKeyProvider("k")).toEqual({ credentialProviderArn: "arn:cp" }); + }); + test("returns undefined when the provider does not exist", async () => { send = async () => { throw new ResourceNotFoundException(); @@ -126,6 +146,28 @@ describe("createIdentityProviderClient", () => { }); }); + test("returns the created API key provider's secret ARN", async () => { + send = async () => ({ + credentialProviderArn: "arn:cp", + apiKeySecretArn: { secretArn: "arn:secret" }, + }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + expect(await client.createApiKeyProvider({ name: "k", apiKey: "sk" })).toEqual({ + credentialProviderArn: "arn:cp", + clientSecretArn: "arn:secret", + }); + }); + + test("creates an OAuth2 provider without a returned secret ARN", async () => { + send = async () => ({ credentialProviderArn: "arn:cp" }); + const client = await createIdentityProviderClient("us-east-1", credentials); + + expect( + await client.createOauth2Provider({ name: "o", vendor: "CustomOauth2", config: {} }), + ).toEqual({ credentialProviderArn: "arn:cp" }); + }); + test("creates an OAuth2 provider with its vendor and config", async () => { send = async () => ({ credentialProviderArn: "arn:cp", From 0e76e9fd6ccca3ce3252408ce53ca3fa4ac228c2 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Thu, 27 Aug 2026 19:03:12 +0000 Subject: [PATCH 04/13] fix: use a valid Oauth2ProviderConfigInput in the no-secret-ARN test --- src/core/project/backends/cdk/credentials.client.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/project/backends/cdk/credentials.client.test.ts b/src/core/project/backends/cdk/credentials.client.test.ts index 6a808c3fb..8929d00da 100644 --- a/src/core/project/backends/cdk/credentials.client.test.ts +++ b/src/core/project/backends/cdk/credentials.client.test.ts @@ -164,7 +164,11 @@ describe("createIdentityProviderClient", () => { const client = await createIdentityProviderClient("us-east-1", credentials); expect( - await client.createOauth2Provider({ name: "o", vendor: "CustomOauth2", config: {} }), + await client.createOauth2Provider({ + name: "o", + vendor: "CustomOauth2", + config: { customOauth2ProviderConfig: { oauthDiscovery: { discoveryUrl: "u" } } }, + }), ).toEqual({ credentialProviderArn: "arn:cp" }); }); From 10672bee79f410cecb305e43fb8bad3ce013e19c Mon Sep 17 00:00:00 2001 From: gitikavj Date: Thu, 27 Aug 2026 21:13:14 +0000 Subject: [PATCH 05/13] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20env?= =?UTF-8?q?-key=20collisions,=20prereq=20order,=20no=20partial=20provision?= =?UTF-8?q?ing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Collision detection now compares each credential's actual per-type .env.local variable (API key uses the base name, OAuth appends _CLIENT_SECRET), so cross-type clashes like api-key 'foo_client_secret' vs oauth 'foo' are rejected before writing the spec. - Check local CDK prerequisites (npm + node_modules) before provisioning credentials, so a local setup error no longer mutates AWS. - Resolve every credential (look up existing, validate the secret) before creating any, so a missing secret fails before the first provider is created rather than leaving a half-provisioned state. --- src/core/project/backends/cdk.test.ts | 15 +++ src/core/project/backends/cdk.ts | 18 ++- .../project/backends/cdk/credentials.test.ts | 14 +++ src/core/project/backends/cdk/credentials.ts | 108 +++++++++++------- .../project/add/credentials/shared.test.ts | 60 ++++++++++ .../project/add/credentials/shared.ts | 36 ++++-- 6 files changed, 195 insertions(+), 56 deletions(-) create mode 100644 src/handlers/project/add/credentials/shared.test.ts diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index e8147809e..18e150d5b 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -333,6 +333,21 @@ describe("CdkBackend.deploy", () => { 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; + const provisionCredentials: CredentialProvisioner = async function* () { + provisioned = true; + return {}; + }; + const subject = harness({ provisionCredentials }); + + await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + /npm install/, + ); + expect(provisioned).toBe(false); + }); + test("fails before touching AWS when the existing state file is malformed", async () => { const input = await project(); const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index 40376c4b6..65ead2a19 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -69,9 +69,10 @@ export class CdkBackend implements ProjectBackend { this.provisionCredentials = config.provisionCredentials ?? createCredentialProvisioner(); } - 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}'. ` + @@ -79,12 +80,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), }, ); @@ -105,9 +110,10 @@ export class CdkBackend implements ProjectBackend { ); } - // 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 diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index 6500fc6a1..1e19cafb1 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -295,4 +295,18 @@ describe("createCredentialProvisioner", () => { ); expect(subject.calls).toEqual([]); }); + + 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 index aa5a95c4e..a426b8ce3 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -165,87 +165,113 @@ export function createCredentialProvisioner( const declared = project.spec.credentials; if (declared.length === 0) return {}; - // Rejected up front so a project with an unsupported credential fails before - // any provider is created, rather than part-way through the list. + // Rejected up front so an unsupported credential fails before any AWS call. const payment = declared.find((c) => c.authorizerType === "PaymentCredentialProvider"); if (payment) throw paymentUnsupported(payment.name); const env = await new EnvLocalFile(project.rootPath).read(); const client = await createClient(region, credentials); - const provisioned: DeployedCredentials = {}; + // Resolve every credential before creating any: look up existing providers + // (reused as-is) and validate the secret for the rest. A missing secret then + // fails before the first provider is created, not partway through the list. + const plans: { name: string; provision: Provision }[] = []; for (const credential of declared) { - // Provider names are account-global, so a name already taken by another - // project in this account is adopted rather than recreated. - yield { message: `Preparing credential provider '${credential.name}'` }; - provisioned[credential.name] = await provisionOne(client, credential, env, project.rootPath); + plans.push({ + name: credential.name, + provision: await resolveCredential(client, credential, env, project.rootPath), + }); + } + + const provisioned: DeployedCredentials = {}; + for (const { name, provision } of plans) { + yield { message: `Preparing credential provider '${name}'` }; + provisioned[name] = "reuse" in provision ? provision.reuse : await provision.create(); } return provisioned; }; } -async function provisionOne( +/** An existing provider to reuse, or a creation deferred until every secret is validated. */ +type Provision = { reuse: DeployedCredential } | { create: () => Promise }; + +function resolveCredential( client: IdentityProviderClient, credential: Credential, env: Record, rootPath: string, -): Promise { +): Promise { switch (credential.authorizerType) { case "ApiKeyCredentialProvider": - return provisionApiKey(client, credential, env, rootPath); + return resolveApiKey(client, credential, env, rootPath); case "OAuthCredentialProvider": - return provisionOauth2(client, credential, env, rootPath); + return resolveOauth2(client, credential, env, rootPath); case "PaymentCredentialProvider": // Unreachable: rejected before provisioning starts. throw paymentUnsupported(credential.name); } } -async function provisionApiKey( +async function resolveApiKey( client: IdentityProviderClient, credential: ApiKeyCredential, env: Record, rootPath: string, -): Promise { +): Promise { + // Provider names are account-global, so one already in this account is reused. const existing = await client.getApiKeyProvider(credential.name); - if (existing) return existing; + if (existing) return { reuse: existing }; - if (credential.secretRef) { - return client.createApiKeyProvider({ name: credential.name, secretRef: credential.secretRef }); - } - - const envKey = credentialEnvVarName(credential.name); - const apiKey = env[envKey]; - if (!apiKey) throw missingSecret(credential.name, envKey, "secretRef", rootPath); - return client.createApiKeyProvider({ name: credential.name, apiKey }); + const input: ApiKeyProviderInput = credential.secretRef + ? { name: credential.name, secretRef: credential.secretRef } + : { + name: credential.name, + apiKey: requireEnvSecret(credential.name, env, rootPath, "secretRef"), + }; + return { create: () => client.createApiKeyProvider(input) }; } -async function provisionOauth2( +async function resolveOauth2( client: IdentityProviderClient, credential: OAuthCredential, env: Record, rootPath: string, -): Promise { +): Promise { const existing = await client.getOauth2Provider(credential.name); - if (existing) return existing; + if (existing) return { reuse: existing }; - let secret: Record; - if (credential.clientSecretRef) { - secret = { clientSecretConfig: credential.clientSecretRef, clientSecretSource: "EXTERNAL" }; - } else { - const envKey = credentialEnvVarName(credential.name, CLIENT_SECRET_SUFFIX); - const clientSecret = env[envKey]; - if (!clientSecret) throw missingSecret(credential.name, envKey, "clientSecretRef", rootPath); - secret = { clientSecret }; - } + const secret: Record = credential.clientSecretRef + ? { clientSecretConfig: credential.clientSecretRef, clientSecretSource: "EXTERNAL" } + : { + clientSecret: requireEnvSecret( + credential.name, + env, + rootPath, + "clientSecretRef", + CLIENT_SECRET_SUFFIX, + ), + }; + const config = credential.providerConfig + ? vendorConfigWithSecret(credential.name, credential.providerConfig, secret) + : guidedCustomConfig(credential, secret); + return { + create: () => + client.createOauth2Provider({ name: credential.name, vendor: credential.vendor, config }), + }; +} - return client.createOauth2Provider({ - name: credential.name, - vendor: credential.vendor, - config: credential.providerConfig - ? vendorConfigWithSecret(credential.name, credential.providerConfig, secret) - : guidedCustomConfig(credential, secret), - }); +/** Reads a credential's secret from `.env.local`, throwing an actionable error when absent. */ +function requireEnvSecret( + name: string, + env: Record, + rootPath: string, + refField: "secretRef" | "clientSecretRef", + suffix = "", +): string { + const envKey = credentialEnvVarName(name, suffix); + const secret = env[envKey]; + if (!secret) throw missingSecret(name, envKey, refField, rootPath); + return secret; } /** diff --git a/src/handlers/project/add/credentials/shared.test.ts b/src/handlers/project/add/credentials/shared.test.ts new file mode 100644 index 000000000..761c213f6 --- /dev/null +++ b/src/handlers/project/add/credentials/shared.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test } from "bun:test"; +import { credentialSecretEnvKey } from "./shared"; + +describe("credentialSecretEnvKey", () => { + test("API keys use the base variable; OAuth appends the client-secret suffix", () => { + expect( + credentialSecretEnvKey({ authorizerType: "ApiKeyCredentialProvider", name: "openai" }), + ).toBe("AGENTCORE_CREDENTIAL_OPENAI"); + expect( + credentialSecretEnvKey({ + authorizerType: "OAuthCredentialProvider", + name: "openai", + vendor: "CustomOauth2", + }), + ).toBe("AGENTCORE_CREDENTIAL_OPENAI_CLIENT_SECRET"); + }); + + test("an OAuth name collides with an API key named like it + client_secret", () => { + // The bug this guards: both resolve to the same .env.local variable. + const oauth = credentialSecretEnvKey({ + authorizerType: "OAuthCredentialProvider", + name: "foo", + vendor: "CustomOauth2", + }); + const apiKey = credentialSecretEnvKey({ + authorizerType: "ApiKeyCredentialProvider", + name: "foo_client_secret", + }); + expect(oauth).toBe("AGENTCORE_CREDENTIAL_FOO_CLIENT_SECRET"); + expect(apiKey).toBe(oauth); + }); + + test("credentials backed by an external secret reference have no .env.local variable", () => { + expect( + credentialSecretEnvKey({ + authorizerType: "ApiKeyCredentialProvider", + name: "openai", + secretRef: { secretId: "s", jsonKey: "k" }, + }), + ).toBeUndefined(); + expect( + credentialSecretEnvKey({ + authorizerType: "OAuthCredentialProvider", + name: "openai", + vendor: "CustomOauth2", + clientSecretRef: { secretId: "s", jsonKey: "k" }, + }), + ).toBeUndefined(); + }); + + test("payment credentials have no .env.local variable", () => { + expect( + credentialSecretEnvKey({ + authorizerType: "PaymentCredentialProvider", + name: "pay", + provider: "StripePrivy", + }), + ).toBeUndefined(); + }); +}); diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts index b4f05c554..5ad42b177 100644 --- a/src/handlers/project/add/credentials/shared.ts +++ b/src/handlers/project/add/credentials/shared.ts @@ -1,6 +1,7 @@ import { ProjectKey, type Context } from "../../../../router"; import { InputValidationError } from "../../../../errors"; import { CLIENT_SECRET_SUFFIX, credentialEnvVarName } from "../../../../core/project/envLocal"; +import type { Credential } from "../../../../projectSchemas/credential"; import { parseSecretReference } from "../../../identity/parser"; import type { AddProjectResourceConfig } from "../types"; import type { AddResourceInput } from "../../types"; @@ -31,18 +32,21 @@ export async function addCredentialToProject( ): 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. + // A credential's secret goes into a per-type .env.local variable (API keys use + // the base name; OAuth appends _CLIENT_SECRET), so two credentials of different + // types or hyphen/underscore spellings can collide on one variable and silently + // share a secret. Reject that on the final key before writing the spec. const newName = input.resourceConfig.name; - const clash = project.spec.credentials.find( - (existing) => - existing.name !== newName && - credentialEnvVarName(existing.name) === credentialEnvVarName(newName), - ); + const newKeys = new Set((input.envEntries ?? []).map((entry) => entry.key)); + const clash = project.spec.credentials.find((existing) => { + if (existing.name === newName) return false; + const key = credentialSecretEnvKey(existing); + return key !== undefined && newKeys.has(key); + }); 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 '_'", + `credential '${newName}' would store its secret in the same .env.local variable as ` + + `'${clash.name}'; choose a name that does not collide.`, ); } @@ -58,3 +62,17 @@ export async function addCredentialToProject( config.io.stderr.write(`Set ${entry.key} in agentcore/.env.local before you deploy.\n`); } } + +/** The .env.local variable a credential's secret is written to, or undefined when it lives elsewhere (an external ref, or no secret). */ +export function credentialSecretEnvKey(credential: Credential): string | undefined { + switch (credential.authorizerType) { + case "ApiKeyCredentialProvider": + return credential.secretRef ? undefined : credentialEnvVarName(credential.name); + case "OAuthCredentialProvider": + return credential.clientSecretRef + ? undefined + : credentialEnvVarName(credential.name, CLIENT_SECRET_SUFFIX); + case "PaymentCredentialProvider": + return undefined; + } +} From 903abbc8adea9e325e9d3064dc1575c5f4dc4fc2 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Thu, 27 Aug 2026 21:32:50 +0000 Subject: [PATCH 06/13] test+fix: keep collision message wording, cover cross-type collision, lint - Keep the 'same environment variable' wording so the existing add-credentials collision test still asserts it, and add an integration test for the cross-type case (oauth 'foo' vs api-key 'foo_client_secret'). - Silence require-yield on a deploy-prereq spy generator that never runs. --- src/core/project/backends/cdk.test.ts | 1 + src/handlers/project/add/credentials/shared.ts | 2 +- src/handlers/project/project.test.ts | 9 +++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 18e150d5b..21a9884f1 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -336,6 +336,7 @@ describe("CdkBackend.deploy", () => { 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 {}; diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts index 5ad42b177..7458ef48f 100644 --- a/src/handlers/project/add/credentials/shared.ts +++ b/src/handlers/project/add/credentials/shared.ts @@ -45,7 +45,7 @@ export async function addCredentialToProject( }); if (clash) { throw new InputValidationError( - `credential '${newName}' would store its secret in the same .env.local variable as ` + + `credential '${newName}' would use the same environment variable for its secret as ` + `'${clash.name}'; choose a name that does not collide.`, ); } diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index a103c59ea..787bd5fc6 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -589,6 +589,15 @@ 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.each<[string, string[], RegExp]>([ [ "api-key: an inline secret value", From 30fea4b9d946a22f23152cdb5d7a02a26816a742 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Fri, 28 Aug 2026 19:34:56 +0000 Subject: [PATCH 07/13] fix: fall back to legacy _CLIENT_ID env var for OAuth client id Older CLIs stored an OAuth credential's client id in AGENTCORE_CREDENTIAL__CLIENT_ID rather than agentcore.json. When recreating such a provider, prefer credential.clientId and fall back to that legacy variable so an upgraded project keeps its client id. Adds an upgrade test. --- .../project/backends/cdk/credentials.test.ts | 28 +++++++++++++++++++ src/core/project/backends/cdk/credentials.ts | 10 +++++-- src/core/project/envLocal.ts | 3 ++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index 1e19cafb1..e6b4c938a 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -206,6 +206,34 @@ describe("createCredentialProvisioner", () => { }); }); + 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]).toEqual({ + kind: "createOauth2", + input: { + name: "my-oauth", + vendor: "CustomOauth2", + config: { + 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( diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index a426b8ce3..c17f61077 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -9,6 +9,7 @@ import type { SecretReference, } from "../../../../projectSchemas/credential"; import { + CLIENT_ID_SUFFIX, CLIENT_SECRET_SUFFIX, credentialEnvVarName, ENV_LOCAL_RELATIVE_PATH, @@ -251,9 +252,13 @@ async function resolveOauth2( CLIENT_SECRET_SUFFIX, ), }; + // 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_SUFFIX)]; const config = credential.providerConfig ? vendorConfigWithSecret(credential.name, credential.providerConfig, secret) - : guidedCustomConfig(credential, secret); + : guidedCustomConfig(credential, clientId, secret); return { create: () => client.createOauth2Provider({ name: credential.name, vendor: credential.vendor, config }), @@ -303,6 +308,7 @@ function vendorConfigWithSecret( function guidedCustomConfig( credential: OAuthCredential, + clientId: string | undefined, secret: Record, ): Oauth2ProviderConfigInput { // The spec's schema requires discoveryUrl for a guided credential; this guards @@ -318,7 +324,7 @@ function guidedCustomConfig( return { customOauth2ProviderConfig: { oauthDiscovery: { discoveryUrl: credential.discoveryUrl }, - ...(credential.clientId !== undefined && { clientId: credential.clientId }), + ...(clientId !== undefined && { clientId }), ...secret, }, }; diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts index 83847e508..64b0ba40b 100644 --- a/src/core/project/envLocal.ts +++ b/src/core/project/envLocal.ts @@ -13,6 +13,9 @@ const KEY_LINE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/; /** Suffix distinguishing an OAuth credential's client secret from an API key. */ export const CLIENT_SECRET_SUFFIX = "_CLIENT_SECRET"; +/** Legacy suffix older CLIs stored an OAuth credential's client id under (now in agentcore.json). */ +export const CLIENT_ID_SUFFIX = "_CLIENT_ID"; + /** * The `.env.local` variable name a credential's secret is stored under — the one * contract between `add credentials` (writes it) and `deploy` (reads it), so both From 96a58875e6e5ead5492ce5c7b79f32a5fb8cb2c6 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Mon, 31 Aug 2026 21:49:55 +0000 Subject: [PATCH 08/13] refactor: provision credentials through the core Identity client Deploy's credential provisioning built its own BedrockAgentCoreControlClient and sent Get/Create commands itself, duplicating the IdentityClient in src/core/identity.tsx that already backs the `agentcore identity` commands. It existed only because the credentials had nowhere to travel: control-plane clients were built from a ClientConfig that carried no credentials, while provisioning must run against the deployment target's own. - CoreOptions and ClientConfig now carry optional credentials, forwarded by toClientConfig, mirroring the CredentialedClientConfig the CloudFormation factory already uses. - CoreClient hands its IdentityClient to FsProjectManager, which passes it to CdkBackend; the provisioner takes CredentialProviderCalls, a four-method Pick of CoreIdentityClient, so tests fake four calls instead of ten. - cacheKey no longer keys clients by JSON.stringify alone: credentials are a provider function, which JSON.stringify drops, so two callers with different credentials in one region would have shared a cached client. They are keyed by object identity instead. - IdentityClient's dependency narrows to Pick, letting a project manager built outside CoreClient construct one from the existing createControlClient factory. The lazily imported SDK is gone with the duplicate client; the claim that it saved startup cost was already untrue, since core/factories.tsx imports the same client statically on every run. credentials.client.test.ts existed only to mock that import and is deleted; its response-mapping and not-found cases move into credentials.test.ts. --- src/core/identity.tsx | 10 +- src/core/index.tsx | 26 +- src/core/project/backends/cdk.test.ts | 21 +- src/core/project/backends/cdk.ts | 11 +- .../backends/cdk/credentials.client.test.ts | 192 -------------- .../project/backends/cdk/credentials.test.ts | 199 ++++++++------ src/core/project/backends/cdk/credentials.ts | 246 ++++++++---------- src/core/project/manager.tsx | 4 + src/core/types.tsx | 12 +- src/core/utils.tsx | 8 +- 10 files changed, 316 insertions(+), 413 deletions(-) delete mode 100644 src/core/project/backends/cdk/credentials.client.test.ts diff --git a/src/core/identity.tsx b/src/core/identity.tsx index c52f1c3d3..f1a08aee5 100644 --- a/src/core/identity.tsx +++ b/src/core/identity.tsx @@ -27,11 +27,19 @@ import type { UpdateApiKeyCredentialProviderInput, UpdateOauth2CredentialProviderInput, } 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, diff --git a/src/core/index.tsx b/src/core/index.tsx index 87948c358..1bbaedcf2 100644 --- a/src/core/index.tsx +++ b/src/core/index.tsx @@ -92,6 +92,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, }); } @@ -146,6 +149,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 d12bf39d8..8a683ac8d 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -6,7 +6,7 @@ import type { DeployResult, Project, ProjectEvent } from "../../../handlers/proj import { ProjectSpecSchema } from "../../../projectSchemas/project"; import { createSilentLogger } from "../../../testing"; import { CdkBackend } from "./cdk"; -import type { CredentialProvisioner } from "./cdk/credentials"; +import type { CredentialProviderCalls, CredentialProvisioner } from "./cdk/credentials"; import { DEPLOYED_STATE_RELATIVE_PATH } from "./cdk/deployedState"; import type { DeployBackendInput } from "./types"; import type { BootstrapState } from "./cdk/environment"; @@ -21,6 +21,23 @@ const TARGET = { /** 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"), + getOauth2CredentialProvider: unexpected("getOauth2CredentialProvider"), + createOauth2CredentialProvider: unexpected("createOauth2CredentialProvider"), + }; +} + function deployInput(overrides: Partial = {}): DeployBackendInput { return { target: TARGET, confirmTeardown: async () => false, ...overrides }; } @@ -143,6 +160,7 @@ function harness(options: HarnessOptions = {}) { const backend = new CdkBackend({ logger: createSilentLogger(), + identity: unusedIdentity(), runner: async (command, { cwd }) => { commands.push({ command, cwd }); }, @@ -254,6 +272,7 @@ describe("CdkBackend.build", () => { const input = await project(); const subject = new CdkBackend({ logger: createSilentLogger(), + identity: unusedIdentity(), runner: async () => { throw new Error("cdk synth exploded"); }, diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index 79f50a440..1eb9c8e01 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -13,7 +13,11 @@ import type { Logger } from "../../../logging"; import type { DeployBackendInput, ProjectBackend } from "./types"; import { createCloudFormationClient } from "../../factories"; import type { CreateCloudFormationClient } from "../../types"; -import { createCredentialProvisioner, type CredentialProvisioner } from "./cdk/credentials"; +import { + createCredentialProvisioner, + type CredentialProviderCalls, + type CredentialProvisioner, +} from "./cdk/credentials"; import { countDeployableResources, stackArtifactForTarget, @@ -46,6 +50,8 @@ 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; @@ -89,7 +95,8 @@ export class CdkBackend implements ProjectBackend { ((stackName, region, credentials) => probeStack(stackName, region, credentials, readStack)); this.resolveAccount = config.resolveAccount ?? resolveAwsAccount; this.loadBootstrapTemplate = config.loadBootstrapTemplate ?? loadBootstrapTemplate; - this.provisionCredentials = config.provisionCredentials ?? createCredentialProvisioner(); + this.provisionCredentials = + config.provisionCredentials ?? createCredentialProvisioner(config.identity); } // Local prerequisites for synth. Checked before any AWS mutation so a missing diff --git a/src/core/project/backends/cdk/credentials.client.test.ts b/src/core/project/backends/cdk/credentials.client.test.ts deleted file mode 100644 index 8929d00da..000000000 --- a/src/core/project/backends/cdk/credentials.client.test.ts +++ /dev/null @@ -1,192 +0,0 @@ -import { afterEach, describe, expect, mock, test } from "bun:test"; - -// credentials.test.ts drives the provisioner with a fake client; this covers the -// real factory by mocking the AWS SDK it lazily imports. - -class ResourceNotFoundException extends Error { - constructor() { - super("not found"); - this.name = "ResourceNotFoundException"; - } -} -class GetApiKeyCredentialProviderCommand { - constructor(readonly input: unknown) {} -} -class CreateApiKeyCredentialProviderCommand { - constructor(readonly input: unknown) {} -} -class GetOauth2CredentialProviderCommand { - constructor(readonly input: unknown) {} -} -class CreateOauth2CredentialProviderCommand { - constructor(readonly input: unknown) {} -} - -const sent: unknown[] = []; -let send: (command: unknown) => Promise; - -class BedrockAgentCoreControlClient { - constructor(readonly config: unknown) {} - send(command: unknown) { - sent.push(command); - return send(command); - } -} - -mock.module("@aws-sdk/client-bedrock-agentcore-control", () => ({ - BedrockAgentCoreControlClient, - GetApiKeyCredentialProviderCommand, - CreateApiKeyCredentialProviderCommand, - GetOauth2CredentialProviderCommand, - CreateOauth2CredentialProviderCommand, - ResourceNotFoundException, -})); - -const { createIdentityProviderClient } = await import("./credentials"); -const credentials = async () => ({ accessKeyId: "a", secretAccessKey: "b" }); - -afterEach(() => { - sent.length = 0; -}); - -describe("createIdentityProviderClient", () => { - test("passes region and credentials to the SDK client", async () => { - send = async () => ({ credentialProviderArn: "arn:cp" }); - const client = await createIdentityProviderClient("eu-west-1", credentials); - await client.getApiKeyProvider("k"); - - expect((sent[0] as GetApiKeyCredentialProviderCommand).input).toEqual({ name: "k" }); - }); - - test("maps an API key provider, including its secret ARN", async () => { - send = async () => ({ - credentialProviderArn: "arn:cp", - apiKeySecretArn: { secretArn: "arn:secret" }, - }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - expect(await client.getApiKeyProvider("k")).toEqual({ - credentialProviderArn: "arn:cp", - clientSecretArn: "arn:secret", - }); - }); - - test("maps an OAuth2 provider it finds, including its secret ARN", async () => { - send = async () => ({ - credentialProviderArn: "arn:cp", - clientSecretArn: { secretArn: "arn:secret" }, - }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - expect(await client.getOauth2Provider("o")).toEqual({ - credentialProviderArn: "arn:cp", - clientSecretArn: "arn:secret", - }); - }); - - test("omits the secret ARN when Identity returns none", async () => { - send = async () => ({ credentialProviderArn: "arn:cp" }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - expect(await client.getApiKeyProvider("k")).toEqual({ credentialProviderArn: "arn:cp" }); - }); - - test("returns undefined when the provider does not exist", async () => { - send = async () => { - throw new ResourceNotFoundException(); - }; - const client = await createIdentityProviderClient("us-east-1", credentials); - - expect(await client.getApiKeyProvider("missing")).toBeUndefined(); - expect(await client.getOauth2Provider("missing")).toBeUndefined(); - }); - - test("propagates errors other than not-found", async () => { - const failure = Object.assign(new Error("denied"), { name: "AccessDeniedException" }); - send = async () => { - throw failure; - }; - const client = await createIdentityProviderClient("us-east-1", credentials); - - await expect(client.getApiKeyProvider("k")).rejects.toBe(failure); - }); - - test("throws when Identity returns no provider ARN", async () => { - send = async () => ({}); - const client = await createIdentityProviderClient("us-east-1", credentials); - - await expect(client.createApiKeyProvider({ name: "k", apiKey: "sk" })).rejects.toThrow( - /no credentialProviderArn/, - ); - }); - - test("creates an API key provider from an inline key", async () => { - send = async () => ({ credentialProviderArn: "arn:cp" }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - await client.createApiKeyProvider({ name: "k", apiKey: "sk-live" }); - - expect((sent[0] as CreateApiKeyCredentialProviderCommand).input).toEqual({ - name: "k", - apiKey: "sk-live", - }); - }); - - test("creates an API key provider from an external secret reference", async () => { - send = async () => ({ credentialProviderArn: "arn:cp" }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - const secretRef = { secretId: "s", jsonKey: "apiKey" }; - await client.createApiKeyProvider({ name: "k", secretRef }); - - expect((sent[0] as CreateApiKeyCredentialProviderCommand).input).toEqual({ - name: "k", - apiKeySecretConfig: secretRef, - apiKeySecretSource: "EXTERNAL", - }); - }); - - test("returns the created API key provider's secret ARN", async () => { - send = async () => ({ - credentialProviderArn: "arn:cp", - apiKeySecretArn: { secretArn: "arn:secret" }, - }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - expect(await client.createApiKeyProvider({ name: "k", apiKey: "sk" })).toEqual({ - credentialProviderArn: "arn:cp", - clientSecretArn: "arn:secret", - }); - }); - - test("creates an OAuth2 provider without a returned secret ARN", async () => { - send = async () => ({ credentialProviderArn: "arn:cp" }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - expect( - await client.createOauth2Provider({ - name: "o", - vendor: "CustomOauth2", - config: { customOauth2ProviderConfig: { oauthDiscovery: { discoveryUrl: "u" } } }, - }), - ).toEqual({ credentialProviderArn: "arn:cp" }); - }); - - test("creates an OAuth2 provider with its vendor and config", async () => { - send = async () => ({ - credentialProviderArn: "arn:cp", - clientSecretArn: { secretArn: "arn:secret" }, - }); - const client = await createIdentityProviderClient("us-east-1", credentials); - - const config = { customOauth2ProviderConfig: { oauthDiscovery: { discoveryUrl: "u" } } }; - const result = await client.createOauth2Provider({ name: "o", vendor: "CustomOauth2", config }); - - expect((sent[0] as CreateOauth2CredentialProviderCommand).input).toEqual({ - name: "o", - credentialProviderVendor: "CustomOauth2", - oauth2ProviderConfigInput: config, - }); - expect(result).toEqual({ credentialProviderArn: "arn:cp", clientSecretArn: "arn:secret" }); - }); -}); diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index e6b4c938a..33ac039e8 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -2,15 +2,23 @@ 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, +} 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, + type CredentialProviderCalls, type CredentialProvisioner, type DeployedCredential, type DeployedCredentials, - type IdentityProviderClient, } from "./credentials"; import type { CdkCredentialProvider } from "./toolkit"; @@ -19,6 +27,7 @@ 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"; @@ -50,44 +59,70 @@ async function project(credentials: unknown[], envLocal?: string): Promise + 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; + +const oauth2Response = (provider: DeployedCredential | undefined) => + ({ + credentialProviderArn: provider?.credentialProviderArn, + ...(provider?.clientSecretArn && { clientSecretArn: { secretArn: provider.clientSecretArn } }), + }) as GetOauth2CredentialProviderResponse & CreateOauth2CredentialProviderResponse; + +/** 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; +}; + +function identity(existing: DeployedCredentials = {}, behavior: Behavior = {}) { const calls: Call[] = []; - const factoryArgs: { region: string; credentials: CdkCredentialProvider }[] = []; - const created = (name: string, prefix: string): DeployedCredential => ({ - credentialProviderArn: `arn:${prefix}:${name}`, - clientSecretArn: `arn:secret:${name}`, - }); + 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: IdentityProviderClient = { - async getApiKeyProvider(name) { - calls.push({ kind: "getApiKey", input: name }); - return existing[name]; + const client: CredentialProviderCalls = { + async getApiKeyCredentialProvider(name, options) { + calls.push({ kind: "getApiKey", input: name, options }); + return apiKeyResponse(lookup(name)); }, - async createApiKeyProvider(input) { - calls.push({ kind: "createApiKey", input }); - return created(input.name, "apikey"); + async createApiKeyCredentialProvider(input, options) { + calls.push({ kind: "createApiKey", input, options }); + return apiKeyResponse(created(input.name ?? "", "apikey")); }, - async getOauth2Provider(name) { - calls.push({ kind: "getOauth2", input: name }); - return existing[name]; + async getOauth2CredentialProvider(name, options) { + calls.push({ kind: "getOauth2", input: name, options }); + return oauth2Response(lookup(name)); }, - async createOauth2Provider(input) { - calls.push({ kind: "createOauth2", input }); - return created(input.name, "oauth"); + async createOauth2CredentialProvider(input, options) { + calls.push({ kind: "createOauth2", input, options }); + return oauth2Response(created(input.name ?? "", "oauth")); }, }; - return { - calls, - factoryArgs, - provision: createCredentialProvisioner(async (region, credentials) => { - factoryArgs.push({ region, credentials }); - return client; - }), - }; + return { calls, provision: createCredentialProvisioner(client) }; } async function run( @@ -104,23 +139,23 @@ async function run( } describe("createCredentialProvisioner", () => { - test("does not build a client for a project without credentials", async () => { + 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.factoryArgs).toEqual([]); + expect(subject.calls).toEqual([]); }); - test("builds the client against the target's own region and credentials", async () => { + 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.factoryArgs).toEqual([{ region: REGION, credentials: CREDENTIALS }]); + expect(subject.calls.map((call) => call.options)).toEqual([OPTIONS, OPTIONS]); }); test("creates an API key provider from the secret in .env.local", async () => { @@ -130,7 +165,7 @@ describe("createCredentialProvisioner", () => { const { events, result } = await run(subject.provision, input); expect(events).toEqual([{ message: "Preparing credential provider 'openai-key'" }]); - expect(subject.calls).toEqual([ + 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" } }, ]); @@ -149,10 +184,11 @@ describe("createCredentialProvisioner", () => { await run(subject.provision, input); - expect(subject.calls).toEqual([ - { kind: "getApiKey", input: "openai-key" }, - { kind: "createApiKey", input: { name: "openai-key", secretRef } }, - ]); + expect(subject.calls[1]?.input).toEqual({ + name: "openai-key", + apiKeySecretConfig: secretRef, + apiKeySecretSource: "EXTERNAL", + }); }); test("names the variable and file to fix when an API key secret is missing", async () => { @@ -173,24 +209,48 @@ describe("createCredentialProvisioner", () => { const { result } = await run(subject.provision, input); - expect(subject.calls).toEqual([{ kind: "getApiKey", input: "openai-key" }]); + expect(subject.calls.map((call) => call.kind)).toEqual(["getApiKey"]); expect(result).toEqual({ "openai-key": existing }); }); + 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).toEqual([ + expect(subject.calls.map(({ kind, input: called }) => ({ kind, input: called }))).toEqual([ { kind: "getOauth2", input: "my-oauth" }, { kind: "createOauth2", input: { name: "my-oauth", - vendor: "CustomOauth2", - config: { + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: { customOauth2ProviderConfig: { oauthDiscovery: { discoveryUrl: DISCOVERY }, clientId: "client-1", @@ -218,17 +278,14 @@ describe("createCredentialProvisioner", () => { await run(subject.provision, input); - expect(subject.calls[1]).toEqual({ - kind: "createOauth2", - input: { - name: "my-oauth", - vendor: "CustomOauth2", - config: { - customOauth2ProviderConfig: { - oauthDiscovery: { discoveryUrl: DISCOVERY }, - clientId: "legacy-client", - clientSecret: "shh", - }, + expect(subject.calls[1]?.input).toEqual({ + name: "my-oauth", + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: { + customOauth2ProviderConfig: { + oauthDiscovery: { discoveryUrl: DISCOVERY }, + clientId: "legacy-client", + clientSecret: "shh", }, }, }); @@ -252,14 +309,11 @@ describe("createCredentialProvisioner", () => { await run(subject.provision, input); - expect(subject.calls[1]).toEqual({ - kind: "createOauth2", - input: { - name: "vendored", - vendor: "GoogleOauth2", - config: { - googleOauth2ProviderConfig: { clientId: "google-client", clientSecret: "g-secret" }, - }, + expect(subject.calls[1]?.input).toEqual({ + name: "vendored", + credentialProviderVendor: "GoogleOauth2", + oauth2ProviderConfigInput: { + googleOauth2ProviderConfig: { clientId: "google-client", clientSecret: "g-secret" }, }, }); }); @@ -271,18 +325,15 @@ describe("createCredentialProvisioner", () => { await run(subject.provision, input); - expect(subject.calls[1]).toEqual({ - kind: "createOauth2", - input: { - name: "my-oauth", - vendor: "CustomOauth2", - config: { - customOauth2ProviderConfig: { - oauthDiscovery: { discoveryUrl: DISCOVERY }, - clientId: "client-1", - clientSecretConfig: clientSecretRef, - clientSecretSource: "EXTERNAL", - }, + expect(subject.calls[1]?.input).toEqual({ + name: "my-oauth", + credentialProviderVendor: "CustomOauth2", + oauth2ProviderConfigInput: { + customOauth2ProviderConfig: { + oauthDiscovery: { discoveryUrl: DISCOVERY }, + clientId: "client-1", + clientSecretConfig: clientSecretRef, + clientSecretSource: "EXTERNAL", }, }, }); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index 27c2ca2bf..199c1db9f 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -1,14 +1,18 @@ import { join } from "node:path"; -import type { Oauth2ProviderConfigInput } from "@aws-sdk/client-bedrock-agentcore-control"; +import { + ResourceNotFoundException, + type Oauth2ProviderConfigInput, +} 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, - SecretReference, } from "../../../../projectSchemas/credential"; import { credentialEnvVarName } from "../../../../projectSchemas/credential"; +import type { CoreOptions } from "../../../types"; import { ENV_LOCAL_RELATIVE_PATH, EnvLocalFile } from "../../envLocal"; import type { CdkCredentialProvider } from "./toolkit"; @@ -19,32 +23,18 @@ export type DeployedCredential = { }; export type DeployedCredentials = Record; -export type ApiKeyProviderInput = { - name: string; - /** Inline key material; mutually exclusive with `secretRef`. */ - apiKey?: string; - /** An existing Secrets Manager secret the customer manages themselves. */ - secretRef?: SecretReference; -}; - -export type Oauth2ProviderInput = { - name: string; - vendor: string; - config: Oauth2ProviderConfigInput; -}; - -/** The Identity calls provisioning needs — four methods, so tests inject a fake instead of the SDK client. */ -export type IdentityProviderClient = { - getApiKeyProvider(name: string): Promise; - createApiKeyProvider(input: ApiKeyProviderInput): Promise; - getOauth2Provider(name: string): Promise; - createOauth2Provider(input: Oauth2ProviderInput): Promise; -}; - -export type IdentityProviderClientFactory = ( - region: string, - credentials: CdkCredentialProvider, -) => Promise; +/** + * The Identity operations provisioning uses, narrowed from the Core client that + * backs the `agentcore identity` commands. Narrowed rather than taken whole so + * tests fake four calls instead of ten. + */ +export type CredentialProviderCalls = Pick< + CoreIdentityClient, + | "getApiKeyCredentialProvider" + | "createApiKeyCredentialProvider" + | "getOauth2CredentialProvider" + | "createOauth2CredentialProvider" +>; export type CredentialProvisionInput = { region: string; @@ -57,94 +47,6 @@ export type CredentialProvisioner = ( input: CredentialProvisionInput, ) => AsyncGenerator; -/** - * Builds an Identity client for the target's credentials. The SDK is imported - * lazily so projects without credentials never pay to load it. - */ -export const createIdentityProviderClient: IdentityProviderClientFactory = async ( - region, - credentials, -) => { - const { - BedrockAgentCoreControlClient, - CreateApiKeyCredentialProviderCommand, - CreateOauth2CredentialProviderCommand, - GetApiKeyCredentialProviderCommand, - GetOauth2CredentialProviderCommand, - ResourceNotFoundException, - } = await import("@aws-sdk/client-bedrock-agentcore-control"); - const client = new BedrockAgentCoreControlClient({ credentials, region }); - - // A missing provider is the normal first-deploy case, not a failure. - const undefinedWhenAbsent = async (send: () => Promise): Promise => { - try { - return await send(); - } catch (error) { - if (error instanceof ResourceNotFoundException) return undefined; - throw error; - } - }; - - return { - async getApiKeyProvider(name) { - const response = await undefinedWhenAbsent(() => - client.send(new GetApiKeyCredentialProviderCommand({ name })), - ); - if (!response) return undefined; - return { - credentialProviderArn: requireArn(response.credentialProviderArn, name), - ...(response.apiKeySecretArn?.secretArn && { - clientSecretArn: response.apiKeySecretArn.secretArn, - }), - }; - }, - async createApiKeyProvider({ name, apiKey, secretRef }) { - const response = await client.send( - new CreateApiKeyCredentialProviderCommand({ - name, - ...(apiKey !== undefined && { apiKey }), - ...(secretRef && { apiKeySecretConfig: secretRef, apiKeySecretSource: "EXTERNAL" }), - }), - ); - return { - credentialProviderArn: requireArn(response.credentialProviderArn, name), - ...(response.apiKeySecretArn?.secretArn && { - clientSecretArn: response.apiKeySecretArn.secretArn, - }), - }; - }, - async getOauth2Provider(name) { - const response = await undefinedWhenAbsent(() => - client.send(new GetOauth2CredentialProviderCommand({ name })), - ); - if (!response) return undefined; - return { - credentialProviderArn: requireArn(response.credentialProviderArn, name), - ...(response.clientSecretArn?.secretArn && { - clientSecretArn: response.clientSecretArn.secretArn, - }), - }; - }, - async createOauth2Provider({ name, vendor, config }) { - const response = await client.send( - new CreateOauth2CredentialProviderCommand({ - 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: vendor as never, - oauth2ProviderConfigInput: config, - }), - ); - return { - credentialProviderArn: requireArn(response.credentialProviderArn, name), - ...(response.clientSecretArn?.secretArn && { - clientSecretArn: response.clientSecretArn.secretArn, - }), - }; - }, - }; -}; - /** * Provisions the credential providers a project declares, before synthesis: the * synthesized app reads their ARNs from `deployed-state.json`, so a project with @@ -155,7 +57,7 @@ export const createIdentityProviderClient: IdentityProviderClientFactory = async * Reconciling a changed declaration is left to a later change. */ export function createCredentialProvisioner( - createClient: IdentityProviderClientFactory = createIdentityProviderClient, + identity: CredentialProviderCalls, ): CredentialProvisioner { return async function* provisionCredentials(project, { region, credentials }) { const declared = project.spec.credentials; @@ -166,7 +68,9 @@ export function createCredentialProvisioner( if (payment) throw paymentUnsupported(payment.name); const env = await new EnvLocalFile(project.rootPath).read(); - const client = await createClient(region, credentials); + // 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 creating any: look up existing providers // (reused as-is) and validate the secret for the rest. A missing secret then @@ -175,7 +79,7 @@ export function createCredentialProvisioner( for (const credential of declared) { plans.push({ name: credential.name, - provision: await resolveCredential(client, credential, env, project.rootPath), + provision: await resolveCredential(identity, credential, options, env, project.rootPath), }); } @@ -192,16 +96,17 @@ export function createCredentialProvisioner( type Provision = { reuse: DeployedCredential } | { create: () => Promise }; function resolveCredential( - client: IdentityProviderClient, + identity: CredentialProviderCalls, credential: Credential, + options: CoreOptions, env: Record, rootPath: string, ): Promise { switch (credential.authorizerType) { case "ApiKeyCredentialProvider": - return resolveApiKey(client, credential, env, rootPath); + return resolveApiKey(identity, credential, options, env, rootPath); case "OAuthCredentialProvider": - return resolveOauth2(client, credential, env, rootPath); + return resolveOauth2(identity, credential, options, env, rootPath); case "PaymentCredentialProvider": // Unreachable: rejected before provisioning starts. throw paymentUnsupported(credential.name); @@ -209,32 +114,39 @@ function resolveCredential( } async function resolveApiKey( - client: IdentityProviderClient, + 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 reused. - const existing = await client.getApiKeyProvider(credential.name); - if (existing) return { reuse: existing }; + const existing = await undefinedWhenAbsent(() => + identity.getApiKeyCredentialProvider(name, options), + ); + if (existing) return { reuse: apiKeyProvision(name, existing) }; - const input: ApiKeyProviderInput = credential.secretRef - ? { name: credential.name, secretRef: credential.secretRef } - : { - name: credential.name, - apiKey: requireEnvSecret(credential.name, env, rootPath, "secretRef"), - }; - return { create: () => client.createApiKeyProvider(input) }; + const input = credential.secretRef + ? { name, apiKeySecretConfig: credential.secretRef, apiKeySecretSource: "EXTERNAL" as const } + : { name, apiKey: requireEnvSecret(name, env, rootPath, "secretRef") }; + return { + create: async () => + apiKeyProvision(name, await identity.createApiKeyCredentialProvider(input, options)), + }; } async function resolveOauth2( - client: IdentityProviderClient, + identity: CredentialProviderCalls, credential: OAuthCredential, + options: CoreOptions, env: Record, rootPath: string, ): Promise { - const existing = await client.getOauth2Provider(credential.name); - if (existing) return { reuse: existing }; + const existing = await undefinedWhenAbsent(() => + identity.getOauth2CredentialProvider(credential.name, options), + ); + if (existing) return { reuse: oauth2Provision(credential.name, existing) }; const secret: Record = credential.clientSecretRef ? { clientSecretConfig: credential.clientSecretRef, clientSecretSource: "EXTERNAL" } @@ -254,8 +166,68 @@ async function resolveOauth2( ? vendorConfigWithSecret(credential.name, credential.providerConfig, secret) : guidedCustomConfig(credential, clientId, secret); return { - create: () => - client.createOauth2Provider({ name: credential.name, vendor: credential.vendor, config }), + create: async () => + oauth2Provision( + credential.name, + await identity.createOauth2CredentialProvider( + { + 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, + }, + options, + ), + ), + }; +} + +/** + * 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, + ); +} + +function deployedCredential( + name: string, + credentialProviderArn: string | undefined, + secretArn: string | undefined, +): DeployedCredential { + return { + credentialProviderArn: requireArn(credentialProviderArn, name), + ...(secretArn && { clientSecretArn: secretArn }), }; } diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index fae997d59..fd809a07c 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -49,12 +49,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; @@ -85,6 +88,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 } : {}), }; } From 6ccd84e8b2cf137c00d3050200a6cd8240dab2b3 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Mon, 31 Aug 2026 21:52:53 +0000 Subject: [PATCH 09/13] fix(project): refuse credential names that shadow a credential field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing collision check compares the .env.local variables the credentials in the spec write today, so it cannot catch a clash with a field the CLI no longer writes but still reads — an OAuth client id, which pre-0.29 projects keep in AGENTCORE_CREDENTIAL__CLIENT_ID — or with one a later credential type adds. An api-key credential named 'svc-client-id' therefore added cleanly and its key was then read as the client id of an OAuth credential named 'svc'. Names whose derived variable ends in a field suffix are now refused at add time, the way main's validateCredentialNameEncryptable refuses them. The check stays in the add flow rather than the schema so that a project already holding such a name keeps loading and can be repaired. --- .../project/add/credentials/shared.ts | 16 +++++++++ src/handlers/project/project.test.ts | 9 +++++ src/projectSchemas/credential.test.ts | 17 +++++++++- src/projectSchemas/credential.ts | 34 +++++++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) 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 e83062480..dfc834bbb 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -778,6 +778,15 @@ describe("project add credentials", () => { ).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..543ab06ed 100644 --- a/src/projectSchemas/credential.ts +++ b/src/projectSchemas/credential.ts @@ -119,6 +119,40 @@ export function credentialEnvVarName(credentialName: string, suffix = ""): strin return `AGENTCORE_CREDENTIAL_${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. */ export function credentialEnvironmentVariableNames(credential: Credential): string[] { switch (credential.authorizerType) { From 1d49629f05584b05a9f977e383e852cd313b644a Mon Sep 17 00:00:00 2001 From: gitikavj Date: Mon, 31 Aug 2026 21:56:31 +0000 Subject: [PATCH 10/13] feat(project): push the current secret to an existing credential provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deploy reused an existing provider untouched, so editing a secret in .env.local and redeploying had no effect on AWS: the provider kept the value it was created with, and nothing said so. main's deploy updates instead — `// Always update to ensure provider has current credentials` — and losing that on the rewrite would be a silent regression for anyone rotating a key. A credential whose secret the CLI can see is now written on every deploy: created when the provider is absent, updated when it is present. A credential with no secret to offer — nothing in .env.local and no external reference — leaves an existing provider exactly as it is, so a project that provisioned once and no longer keeps the secret on disk still deploys; only an absent provider fails, as before. --- src/core/project/backends/cdk.test.ts | 2 + .../project/backends/cdk/credentials.test.ts | 76 +++++++++- src/core/project/backends/cdk/credentials.ts | 139 +++++++++++------- 3 files changed, 161 insertions(+), 56 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 8a683ac8d..827cca1cd 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -33,8 +33,10 @@ function unusedIdentity(): CredentialProviderCalls { return { getApiKeyCredentialProvider: unexpected("getApiKeyCredentialProvider"), createApiKeyCredentialProvider: unexpected("createApiKeyCredentialProvider"), + updateApiKeyCredentialProvider: unexpected("updateApiKeyCredentialProvider"), getOauth2CredentialProvider: unexpected("getOauth2CredentialProvider"), createOauth2CredentialProvider: unexpected("createOauth2CredentialProvider"), + updateOauth2CredentialProvider: unexpected("updateOauth2CredentialProvider"), }; } diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index 33ac039e8..795ee153b 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -8,6 +8,8 @@ import { type CreateOauth2CredentialProviderResponse, type GetApiKeyCredentialProviderResponse, type GetOauth2CredentialProviderResponse, + type UpdateApiKeyCredentialProviderResponse, + type UpdateOauth2CredentialProviderResponse, } from "@aws-sdk/client-bedrock-agentcore-control"; import type { Project, ProjectEvent } from "../../../../handlers/project/types"; import { ProjectSpecSchema } from "../../../../projectSchemas/project"; @@ -71,13 +73,17 @@ const apiKeyResponse = (provider: DeployedCredential | undefined) => ({ credentialProviderArn: provider?.credentialProviderArn, ...(provider?.clientSecretArn && { apiKeySecretArn: { secretArn: provider.clientSecretArn } }), - }) as GetApiKeyCredentialProviderResponse & CreateApiKeyCredentialProviderResponse; + }) as GetApiKeyCredentialProviderResponse & + CreateApiKeyCredentialProviderResponse & + UpdateApiKeyCredentialProviderResponse; const oauth2Response = (provider: DeployedCredential | undefined) => ({ credentialProviderArn: provider?.credentialProviderArn, ...(provider?.clientSecretArn && { clientSecretArn: { secretArn: provider.clientSecretArn } }), - }) as GetOauth2CredentialProviderResponse & CreateOauth2CredentialProviderResponse; + }) as GetOauth2CredentialProviderResponse & + CreateOauth2CredentialProviderResponse & + UpdateOauth2CredentialProviderResponse; /** Overrides for the paths that only a failing or incomplete Identity produces. */ type Behavior = { @@ -112,6 +118,10 @@ function identity(existing: DeployedCredentials = {}, behavior: Behavior = {}) { calls.push({ kind: "createApiKey", input, options }); return apiKeyResponse(created(input.name ?? "", "apikey")); }, + async updateApiKeyCredentialProvider(input, options) { + calls.push({ kind: "updateApiKey", input, options }); + return apiKeyResponse(created(input.name ?? "", "apikey")); + }, async getOauth2CredentialProvider(name, options) { calls.push({ kind: "getOauth2", input: name, options }); return oauth2Response(lookup(name)); @@ -120,6 +130,10 @@ function identity(existing: DeployedCredentials = {}, behavior: Behavior = {}) { 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")); + }, }; return { calls, provision: createCredentialProvisioner(client) }; @@ -202,10 +216,49 @@ describe("createCredentialProvisioner", () => { ); }); - test("reuses a provider that already exists instead of recreating it", async () => { + 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-live'\n"); + 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); @@ -213,6 +266,21 @@ describe("createCredentialProvisioner", () => { 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]); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index 199c1db9f..2d1d6a4c6 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -26,14 +26,16 @@ 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 four calls instead of ten. + * tests fake six calls instead of ten. */ export type CredentialProviderCalls = Pick< CoreIdentityClient, | "getApiKeyCredentialProvider" | "createApiKeyCredentialProvider" + | "updateApiKeyCredentialProvider" | "getOauth2CredentialProvider" | "createOauth2CredentialProvider" + | "updateOauth2CredentialProvider" >; export type CredentialProvisionInput = { @@ -52,9 +54,10 @@ export type CredentialProvisioner = ( * synthesized app reads their ARNs from `deployed-state.json`, so a project with * credentials can't synthesize until they exist. * - * Created when absent, reused when present, never updated — so a redeploy neither - * mints a new secret version nor overwrites one rotated outside the CLI. - * Reconciling a changed declaration is left to a later change. + * 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, @@ -72,9 +75,9 @@ export function createCredentialProvisioner( // rather than the default chain, in the region the target deploys to. const options: CoreOptions = { region, credentials }; - // Resolve every credential before creating any: look up existing providers - // (reused as-is) and validate the secret for the rest. A missing secret then - // fails before the first provider is created, not partway through the list. + // 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: { name: string; provision: Provision }[] = []; for (const credential of declared) { plans.push({ @@ -86,14 +89,20 @@ export function createCredentialProvisioner( const provisioned: DeployedCredentials = {}; for (const { name, provision } of plans) { yield { message: `Preparing credential provider '${name}'` }; - provisioned[name] = "reuse" in provision ? provision.reuse : await provision.create(); + provisioned[name] = "reuse" in provision ? provision.reuse : await provision.write(); } return provisioned; }; } -/** An existing provider to reuse, or a creation deferred until every secret is validated. */ -type Provision = { reuse: DeployedCredential } | { create: () => Promise }; +/** + * 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, @@ -121,17 +130,33 @@ async function resolveApiKey( rootPath: string, ): Promise { const { name } = credential; - // Provider names are account-global, so one already in this account is reused. + // 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), ); - if (existing) return { reuse: apiKeyProvision(name, existing) }; - const input = credential.secretRef - ? { name, apiKeySecretConfig: credential.secretRef, apiKeySecretSource: "EXTERNAL" as const } - : { name, apiKey: requireEnvSecret(name, env, rootPath, "secretRef") }; + 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 { - create: async () => + kind: "create", + write: async () => apiKeyProvision(name, await identity.createApiKeyCredentialProvider(input, options)), }; } @@ -146,43 +171,67 @@ async function resolveOauth2( const existing = await undefinedWhenAbsent(() => identity.getOauth2CredentialProvider(credential.name, options), ); - if (existing) return { reuse: oauth2Provision(credential.name, existing) }; - const secret: Record = credential.clientSecretRef + const secret: Record | undefined = credential.clientSecretRef ? { clientSecretConfig: credential.clientSecretRef, clientSecretSource: "EXTERNAL" } - : { - clientSecret: requireEnvSecret( - credential.name, - env, - rootPath, - "clientSecretRef", - "_CLIENT_SECRET", - ), - }; + : 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 { - create: async () => + kind: "create", + write: async () => oauth2Provision( credential.name, - await identity.createOauth2CredentialProvider( - { - 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, - }, - options, - ), + await identity.createOauth2CredentialProvider(input, options), ), }; } +/** + * 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. @@ -231,20 +280,6 @@ function deployedCredential( }; } -/** Reads a credential's secret from `.env.local`, throwing an actionable error when absent. */ -function requireEnvSecret( - name: string, - env: Record, - rootPath: string, - refField: "secretRef" | "clientSecretRef", - suffix = "", -): string { - const envKey = credentialEnvVarName(name, suffix); - const secret = env[envKey]; - if (!secret) throw missingSecret(name, envKey, refField, rootPath); - return secret; -} - /** * 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 From 792e9f759a85e3b737deaedc69972452591e5217 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Mon, 31 Aug 2026 21:58:13 +0000 Subject: [PATCH 11/13] feat(project): read credential secrets from the deploy environment Provisioning read agentcore/.env.local and nothing else, so a deploy from CI had to write its secrets to disk first. main reads every credential variable from process.env and merges it over the file, which is what makes a non-interactive deploy possible without persisting secrets. Variables carrying the credential prefix now override the file. The filter keeps the deploy from reading anything else out of the environment, and the provisioner takes the environment as an argument so tests do not mutate the process's own. --- .../project/backends/cdk/credentials.test.ts | 30 +++++++++++++++++-- src/core/project/backends/cdk/credentials.ts | 27 ++++++++++++++--- src/projectSchemas/credential.ts | 5 +++- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index 795ee153b..07fc18465 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -93,7 +93,11 @@ type Behavior = { createReturns?: DeployedCredential; }; -function identity(existing: DeployedCredentials = {}, behavior: Behavior = {}) { +function identity( + existing: DeployedCredentials = {}, + behavior: Behavior = {}, + processEnv: Record = {}, +) { const calls: Call[] = []; const created = (name: string, prefix: string): DeployedCredential => @@ -136,7 +140,7 @@ function identity(existing: DeployedCredentials = {}, behavior: Behavior = {}) { }, }; - return { calls, provision: createCredentialProvisioner(client) }; + return { calls, provision: createCredentialProvisioner(client, processEnv) }; } async function run( @@ -205,6 +209,28 @@ describe("createCredentialProvisioner", () => { }); }); + 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]); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index 2d1d6a4c6..3eaf614a1 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -11,7 +11,7 @@ import type { Credential, OAuthCredential, } from "../../../../projectSchemas/credential"; -import { credentialEnvVarName } from "../../../../projectSchemas/credential"; +import { CREDENTIAL_ENV_PREFIX, credentialEnvVarName } from "../../../../projectSchemas/credential"; import type { CoreOptions } from "../../../types"; import { ENV_LOCAL_RELATIVE_PATH, EnvLocalFile } from "../../envLocal"; import type { CdkCredentialProvider } from "./toolkit"; @@ -61,6 +61,7 @@ export type CredentialProvisioner = ( */ export function createCredentialProvisioner( identity: CredentialProviderCalls, + processEnv: Record = process.env, ): CredentialProvisioner { return async function* provisionCredentials(project, { region, credentials }) { const declared = project.spec.credentials; @@ -70,7 +71,12 @@ export function createCredentialProvisioner( const payment = declared.find((c) => c.authorizerType === "PaymentCredentialProvider"); if (payment) throw paymentUnsupported(payment.name); - const env = await new EnvLocalFile(project.rootPath).read(); + // 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 }; @@ -218,6 +224,18 @@ async function resolveOauth2( }; } +/** + * 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)), + ); +} + /** * Reads a credential's secret from `.env.local`, shaped into the request field it * fills, or undefined when the variable is unset. @@ -339,8 +357,9 @@ function missingSecret( ): ProjectStateError { return new ProjectStateError( `Credential '${name}' has no secret to create its provider with. Set ${envKey} in ` + - `${join(rootPath, ENV_LOCAL_RELATIVE_PATH)}, or give the credential a '${refField}' in ` + - `agentcore.json pointing at a secret you keep in AWS Secrets Manager.`, + `${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.`, ); } diff --git a/src/projectSchemas/credential.ts b/src/projectSchemas/credential.ts index 543ab06ed..afb162259 100644 --- a/src/projectSchemas/credential.ts +++ b/src/projectSchemas/credential.ts @@ -114,9 +114,12 @@ 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}`; } /** From b4bf5f109759c3b5c53386ffb86bb5f97189c54a Mon Sep 17 00:00:00 2001 From: gitikavj Date: Mon, 31 Aug 2026 22:10:26 +0000 Subject: [PATCH 12/13] feat(project): provision payment credential providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `project add credentials payment` and `add payment-connector` write a payment credential to the spec, but deploy refused to provision one — it threw and told the user to remove it — so a project could be assembled that added cleanly and then could not deploy at all. main provisions them, and the CDK construct that wires payment connectors already reads their ARNs out of the credentials map in deployed-state.json. - Core's Identity client gains the payment provider operations. The pinned SDK carries them, so unlike main there is no hand-signed HTTP request. - A payment credential is created when absent and updated when present, from the vendor's variables: CoinbaseCDP's api key id, api key secret and wallet secret, or StripePrivy's app id, app secret, authorization private key and authorization id. Every variable that is unset is named in one error rather than one per attempt, and an existing provider is left alone when they are all absent. - Teardown deletes the payment providers the project declares, as main's cleanupPaymentCredentialProviders does, after the stack rather than before, since a resource in it may still be using one. Only payment providers: an api-key or OAuth provider is named account-globally and may be shared with another project. The payment collision test now asserts what actually guards that case: a name ending in a payment field suffix is refused on its own, so a credential can no longer be created that would collide with a payment credential's variables. --- src/core/identity.tsx | 46 +++++ src/core/project/backends/cdk.test.ts | 36 +++- src/core/project/backends/cdk.ts | 11 ++ .../project/backends/cdk/credentials.test.ts | 163 +++++++++++++++++- src/core/project/backends/cdk/credentials.ts | 161 +++++++++++++++-- src/handlers/identity/types.tsx | 28 +++ .../add/credentials/payment/index.test.ts | 14 +- src/testing/TestCoreClient.tsx | 52 ++++++ 8 files changed, 483 insertions(+), 28 deletions(-) diff --git a/src/core/identity.tsx b/src/core/identity.tsx index f1a08aee5..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,13 +23,19 @@ 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"; @@ -132,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/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 827cca1cd..1a38c8f2e 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -6,7 +6,11 @@ import type { DeployResult, Project, ProjectEvent } from "../../../handlers/proj import { ProjectSpecSchema } from "../../../projectSchemas/project"; import { createSilentLogger } from "../../../testing"; import { CdkBackend } from "./cdk"; -import type { CredentialProviderCalls, CredentialProvisioner } from "./cdk/credentials"; +import type { + CredentialProviderCalls, + CredentialProvisioner, + PaymentCredentialRemover, +} from "./cdk/credentials"; import { DEPLOYED_STATE_RELATIVE_PATH } from "./cdk/deployedState"; import type { DeployBackendInput } from "./types"; import type { BootstrapState } from "./cdk/environment"; @@ -37,6 +41,10 @@ function unusedIdentity(): CredentialProviderCalls { getOauth2CredentialProvider: unexpected("getOauth2CredentialProvider"), createOauth2CredentialProvider: unexpected("createOauth2CredentialProvider"), updateOauth2CredentialProvider: unexpected("updateOauth2CredentialProvider"), + getPaymentCredentialProvider: unexpected("getPaymentCredentialProvider"), + createPaymentCredentialProvider: unexpected("createPaymentCredentialProvider"), + updatePaymentCredentialProvider: unexpected("updatePaymentCredentialProvider"), + deletePaymentCredentialProvider: unexpected("deletePaymentCredentialProvider"), }; } @@ -140,6 +148,7 @@ type HarnessOptions = { failOperation?: CdkOperation["kind"]; bootstrapError?: Error; provisionCredentials?: CredentialProvisioner; + removePaymentCredentials?: PaymentCredentialRemover; /** Whether CloudFormation still holds the target's stack. Defaults to present. */ stackExists?: boolean; }; @@ -216,6 +225,9 @@ function harness(options: HarnessOptions = {}) { }; }, ...(options.provisionCredentials && { provisionCredentials: options.provisionCredentials }), + ...(options.removePaymentCredentials && { + removePaymentCredentials: options.removePaymentCredentials, + }), }); return { @@ -523,6 +535,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 1eb9c8e01..96277ace5 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -15,8 +15,10 @@ import { createCloudFormationClient } from "../../factories"; import type { CreateCloudFormationClient } from "../../types"; import { createCredentialProvisioner, + createPaymentCredentialRemover, type CredentialProviderCalls, type CredentialProvisioner, + type PaymentCredentialRemover, } from "./cdk/credentials"; import { countDeployableResources, @@ -59,6 +61,7 @@ export type CdkBackendConfig = { resolveAccount?: AccountResolver; loadBootstrapTemplate?: BootstrapTemplateLoader; provisionCredentials?: CredentialProvisioner; + removePaymentCredentials?: PaymentCredentialRemover; }; /** Builds and deploys projects through the scaffolded CDK app. */ @@ -74,6 +77,7 @@ export class CdkBackend implements ProjectBackend { private readonly resolveAccount: AccountResolver; private readonly loadBootstrapTemplate: BootstrapTemplateLoader; private readonly provisionCredentials: CredentialProvisioner; + private readonly removePaymentCredentials: PaymentCredentialRemover; constructor(config: CdkBackendConfig) { this.logger = config.logger; @@ -97,6 +101,8 @@ export class CdkBackend implements ProjectBackend { this.loadBootstrapTemplate = config.loadBootstrapTemplate ?? loadBootstrapTemplate; this.provisionCredentials = config.provisionCredentials ?? createCredentialProvisioner(config.identity); + this.removePaymentCredentials = + config.removePaymentCredentials ?? createPaymentCredentialRemover(config.identity); } // Local prerequisites for synth. Checked before any AWS mutation so a missing @@ -266,6 +272,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 index 07fc18465..9daa62702 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -10,6 +10,10 @@ import { type GetOauth2CredentialProviderResponse, type UpdateApiKeyCredentialProviderResponse, type UpdateOauth2CredentialProviderResponse, + type CreatePaymentCredentialProviderResponse, + 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"; @@ -17,6 +21,7 @@ import type { CoreOptions } from "../../../types"; import { EnvLocalFile } from "../../envLocal"; import { createCredentialProvisioner, + createPaymentCredentialRemover, type CredentialProviderCalls, type CredentialProvisioner, type DeployedCredential, @@ -77,6 +82,13 @@ const apiKeyResponse = (provider: DeployedCredential | undefined) => CreateApiKeyCredentialProviderResponse & UpdateApiKeyCredentialProviderResponse; +const paymentResponse = (provider: DeployedCredential | undefined) => + ({ + credentialProviderArn: provider?.credentialProviderArn, + }) as GetPaymentCredentialProviderResponse & + CreatePaymentCredentialProviderResponse & + UpdatePaymentCredentialProviderResponse; + const oauth2Response = (provider: DeployedCredential | undefined) => ({ credentialProviderArn: provider?.credentialProviderArn, @@ -91,6 +103,8 @@ type Behavior = { getFails?: Error; /** Returned by every create, in place of a fully populated provider. */ createReturns?: DeployedCredential; + /** Thrown by every payment-provider deletion. */ + deleteFails?: Error; }; function identity( @@ -138,9 +152,37 @@ function identity( calls.push({ kind: "updateOauth2", input, options }); return oauth2Response(created(input.name ?? "", "oauth")); }, + 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, provision: createCredentialProvisioner(client, processEnv) }; + 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( @@ -453,20 +495,123 @@ describe("createCredentialProvisioner", () => { await expect(run(subject.provision, input)).rejects.toThrow(/exactly one vendor config object/); }); - test("rejects a payment credential before creating any provider", async () => { + test("creates a Coinbase payment provider from the vendor's variables", async () => { const subject = identity(); const input = await project( - [ - API_KEY, - { authorizerType: "PaymentCredentialProvider", name: "pay-1", provider: "StripePrivy" }, - ], - "AGENTCORE_CREDENTIAL_OPENAI_KEY='sk-live'\n", + [{ 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( - /PaymentCredentialProvider, which 'agentcore project deploy' cannot create/, + /AGENTCORE_CREDENTIAL_WALLET_API_KEY_SECRET, AGENTCORE_CREDENTIAL_WALLET_WALLET_SECRET/, ); - expect(subject.calls).toEqual([]); + }); + + 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("creates nothing when a later credential's secret is missing", async () => { diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index 3eaf614a1..709b256d4 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -2,6 +2,7 @@ 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"; @@ -10,8 +11,13 @@ import type { ApiKeyCredential, Credential, OAuthCredential, + PaymentCredential, +} from "../../../../projectSchemas/credential"; +import { + CREDENTIAL_ENV_PREFIX, + credentialEnvironmentVariableNames, + credentialEnvVarName, } from "../../../../projectSchemas/credential"; -import { CREDENTIAL_ENV_PREFIX, credentialEnvVarName } from "../../../../projectSchemas/credential"; import type { CoreOptions } from "../../../types"; import { ENV_LOCAL_RELATIVE_PATH, EnvLocalFile } from "../../envLocal"; import type { CdkCredentialProvider } from "./toolkit"; @@ -36,6 +42,10 @@ export type CredentialProviderCalls = Pick< | "getOauth2CredentialProvider" | "createOauth2CredentialProvider" | "updateOauth2CredentialProvider" + | "getPaymentCredentialProvider" + | "createPaymentCredentialProvider" + | "updatePaymentCredentialProvider" + | "deletePaymentCredentialProvider" >; export type CredentialProvisionInput = { @@ -49,6 +59,49 @@ export type CredentialProvisioner = ( 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 @@ -67,10 +120,6 @@ export function createCredentialProvisioner( const declared = project.spec.credentials; if (declared.length === 0) return {}; - // Rejected up front so an unsupported credential fails before any AWS call. - const payment = declared.find((c) => c.authorizerType === "PaymentCredentialProvider"); - if (payment) throw paymentUnsupported(payment.name); - // 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 = { @@ -123,8 +172,7 @@ function resolveCredential( case "OAuthCredentialProvider": return resolveOauth2(identity, credential, options, env, rootPath); case "PaymentCredentialProvider": - // Unreachable: rejected before provisioning starts. - throw paymentUnsupported(credential.name); + return resolvePayment(identity, credential, options, env, rootPath); } } @@ -236,6 +284,83 @@ function credentialEnvironment( ); } +/** + * 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. @@ -287,6 +412,15 @@ function oauth2Provision( ); } +// 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, @@ -363,12 +497,15 @@ function missingSecret( ); } -function paymentUnsupported(name: string): ProjectStateError { +function missingPaymentSecrets( + name: string, + missing: string[], + rootPath: string, +): ProjectStateError { return new ProjectStateError( - `Credential '${name}' is a PaymentCredentialProvider, which 'agentcore project deploy' ` + - `cannot create: a payment provider needs vendor configuration (API key, wallet and ` + - `authorization secrets) that agentcore.json has no fields for. Remove it from the project ` + - `spec to deploy the rest of the project.`, + `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.`, ); } 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/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index 8fcce7a46..0f233ed76 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, @@ -121,6 +125,8 @@ import type { import type { CoreIdentityClient, CreateApiKeyCredentialProviderInput, + CreatePaymentCredentialProviderInput, + UpdatePaymentCredentialProviderInput, CreateOauth2CredentialProviderInput, UpdateApiKeyCredentialProviderInput, UpdateOauth2CredentialProviderInput, @@ -211,6 +217,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 }; @@ -1226,8 +1236,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; @@ -1369,6 +1385,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. From 0ce21b5b2299977d1e1a97e61ee1dec09e6946b6 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Mon, 31 Aug 2026 22:13:34 +0000 Subject: [PATCH 13/13] fix(project): undo credential providers a failed deploy created MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolving every credential before writing any removes the likeliest cause of a half-provisioned deploy — a missing secret — but not the rest: a create that fails on throttling or permissions after an earlier one succeeded left a provider in AWS that deployed-state.json never recorded. Retrying adopted it by name, but abandoning the deploy or dropping the credential orphaned it. A failure during the write loop now deletes the providers that same run created, newest first, and rethrows the original error. A provider that already existed is not deleted: this deploy only updated its secret, and undoing that would need the value it held before, which the CLI never had. A deletion that fails is reported — naming the provider and saying the next deploy will adopt it — rather than replacing the error that stopped the deploy. --- src/core/project/backends/cdk.test.ts | 2 + .../project/backends/cdk/credentials.test.ts | 81 +++++++++++++++++++ src/core/project/backends/cdk/credentials.ts | 70 ++++++++++++++-- 3 files changed, 148 insertions(+), 5 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 1a38c8f2e..bf1c9266b 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -44,6 +44,8 @@ function unusedIdentity(): CredentialProviderCalls { getPaymentCredentialProvider: unexpected("getPaymentCredentialProvider"), createPaymentCredentialProvider: unexpected("createPaymentCredentialProvider"), updatePaymentCredentialProvider: unexpected("updatePaymentCredentialProvider"), + deleteApiKeyCredentialProvider: unexpected("deleteApiKeyCredentialProvider"), + deleteOauth2CredentialProvider: unexpected("deleteOauth2CredentialProvider"), deletePaymentCredentialProvider: unexpected("deletePaymentCredentialProvider"), }; } diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index 9daa62702..2b2f612e4 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -11,6 +11,8 @@ import { type UpdateApiKeyCredentialProviderResponse, type UpdateOauth2CredentialProviderResponse, type CreatePaymentCredentialProviderResponse, + type DeleteApiKeyCredentialProviderResponse, + type DeleteOauth2CredentialProviderResponse, type DeletePaymentCredentialProviderResponse, type GetPaymentCredentialProviderResponse, type UpdatePaymentCredentialProviderResponse, @@ -105,6 +107,8 @@ type Behavior = { 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( @@ -134,12 +138,18 @@ function identity( }, 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)); @@ -152,6 +162,11 @@ function identity( 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)); @@ -614,6 +629,72 @@ describe("createCredentialProvisioner", () => { 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. diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index 709b256d4..58ba2c968 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -45,6 +45,10 @@ export type CredentialProviderCalls = Pick< | "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" >; @@ -133,23 +137,79 @@ export function createCredentialProvisioner( // 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: { name: string; provision: Provision }[] = []; + const plans: { credential: Credential; provision: Provision }[] = []; for (const credential of declared) { plans.push({ - name: credential.name, + credential, provision: await resolveCredential(identity, credential, options, env, project.rootPath), }); } const provisioned: DeployedCredentials = {}; - for (const { name, provision } of plans) { - yield { message: `Preparing credential provider '${name}'` }; - provisioned[name] = "reuse" in provision ? provision.reuse : await provision.write(); + // 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 —