From 3c09c23e85fa1d7703668ab73c1b72acc1acf588 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Thu, 27 Aug 2026 00:19:56 +0000 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20env-k?= =?UTF-8?q?ey=20collisions,=20prereq=20order,=20no=20partial=20provisionin?= =?UTF-8?q?g?= 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 6/7] 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 7/7] 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