fix(deploy): delete orphaned credential providers on teardown and removal

e2e-tests/harness-custom-jwt.test.ts

@@ -12,8 +12,14 @@
  *
  * Requires: AWS credentials, npm, git.
  */
-import { hasAwsCredentials, parseJsonOutput, prereqs, runCLI, stripAnsi } from '../src/test-utils/index.js';
+import { computeManagedOAuthCredentialName } from '../src/cli/primitives/credential-utils.js';
+import { hasAwsCredentials, parseJsonOutput, prereqs, retry, runCLI, stripAnsi } from '../src/test-utils/index.js';
 import { installCdkTarball, writeAwsTargets } from './e2e-helper.js';
+import {
+  BedrockAgentCoreControlClient,
+  GetOauth2CredentialProviderCommand,
+  ResourceNotFoundException,
+} from '@aws-sdk/client-bedrock-agentcore-control';
 import { CloudFormationClient, GetTemplateCommand } from '@aws-sdk/client-cloudformation';
 import {
   CognitoIdentityProviderClient,
@@ -163,6 +169,9 @@ describe.sequential('e2e: harness with CUSTOM_JWT auth', () => {
     if (!canRun) return;
 
     // ── Tear down deployed stack ──
+    // The final test ('teardown deletes the managed OAuth2 credential provider') normally tears the
+    // project down. This is a best-effort safety net for when an earlier test failed before reaching
+    // it — it re-runs the teardown (idempotent; no-ops if already torn down).
     if (projectPath) {
       try {
         await runCLI(['remove', 'all', '--json'], projectPath, { skipInstall: false });
@@ -328,4 +337,49 @@ describe.sequential('e2e: harness with CUSTOM_JWT auth', () => {
     },
     120000
   );
+
+  it.skipIf(!canRun)(
+    'teardown deletes the managed OAuth2 credential provider',
+    async () => {
+      // Regression test for the orphaned-provider leak: `add harness --client-id/--client-secret`
+      // registers a managed OAuth2 credential provider that deploy creates imperatively, outside the
+      // CloudFormation stack. Tearing the project down must delete it — otherwise it leaks and counts
+      // against the account's 50-provider OAuth2 quota (which previously broke the e2e suite).
+      const controlClient = new BedrockAgentCoreControlClient({ region });
+      const providerName = computeManagedOAuthCredentialName(harnessName);
+
+      // Precondition: the provider exists while the harness is deployed (proves the test is meaningful).
+      await expect(
+        controlClient.send(new GetOauth2CredentialProviderCommand({ name: providerName })),
+        `Managed OAuth2 provider "${providerName}" should exist before teardown`
+      ).resolves.toBeTruthy();
+
+      // Tear the project down: empty the spec, then deploy the empty spec (teardown deploy).
+      const removeResult = await runCLI(['remove', 'all', '--json'], projectPath, { skipInstall: false });
+      expect(removeResult.exitCode, `remove all failed: ${removeResult.stderr}`).toBe(0);
+      const teardownResult = await runCLI(['deploy', '--yes', '--json'], projectPath, { skipInstall: false });
+      expect(teardownResult.exitCode, `teardown deploy failed: ${teardownResult.stderr}`).toBe(0);
+
+      // The provider must be gone after teardown. Retry to tolerate read-after-delete
+      // eventual consistency — the delete is issued during teardown, but the GET may briefly
+      // still see it. Resolves as soon as the GET throws ResourceNotFoundException.
+      await retry(
+        async () => {
+          let stillExists = false;
+          try {
+            await controlClient.send(new GetOauth2CredentialProviderCommand({ name: providerName }));
+            stillExists = true;
+          } catch (error) {
+            if (!(error instanceof ResourceNotFoundException)) throw error;
+          }
+          expect(stillExists, `Managed OAuth2 provider "${providerName}" should be deleted after teardown`).toBe(false);
+        },
+        5,
+        3000
+      );
+
+      controlClient.destroy();
+    },
+    600000
+  );
 });

src/cli/commands/deploy/actions.ts

@@ -51,6 +51,7 @@ import { enableOnlineEvalConfigs } from '../../operations/deploy/post-deploy-onl
 import {
   cleanupPaymentCredentialProviders,
   hasPaymentCredentialProviders,
+  reconcileCredentialProviders,
   setupPaymentCredentialProviders,
 } from '../../operations/deploy/pre-deploy-identity';
 import { findOrphanHarnesses } from '../../operations/harness/orphan';
@@ -289,6 +290,13 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
       { credentialProviderArn: string; clientSecretArn?: string; callbackUrl?: string }
     > = {};
 
+    // Snapshot the credential providers recorded on the prior deploy BEFORE any state
+    // mutation below overwrites them. These are CLI-created OAuth2/ApiKey providers; after
+    // a successful deploy we reconcile this against the current spec to delete the ones no
+    // longer referenced (they live outside the CFN stack, so teardown wouldn't reap them).
+    const priorRecordedCredentials =
+      (await configIO.readDeployedState().catch(() => undefined))?.targets?.[target.name]?.resources?.credentials ?? {};
+
     if (hasIdentityApiProviders(context.projectSpec)) {
       startStep('Creating credentials...');
 
@@ -533,6 +541,39 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
 
     endStep('success');
 
+    // Reap OAuth2/ApiKey credential providers the CLI created on a prior deploy that the
+    // current spec no longer references. These are created imperatively outside the CFN
+    // stack, so stack teardown never removes them — without this they leak against the
+    // account's 50-provider quota (teardown deletes all; a single remove deletes just that
+    // provider). Only providers recorded in deployed state are touched, so imported/
+    // pre-existing providers (never recorded) are never deleted.
+    //
+    // On a teardown deploy this MUST run after performStackTeardown (below): the harness
+    // runtime in the stack references the provider, so deleting it before the stack is gone
+    // can fail. On a normal deploy the removed resource is already detached by the deploy
+    // above, so reconcile runs here.
+    const reconcileUnusedCredentials = async (): Promise<void> => {
+      if (Object.keys(priorRecordedCredentials).length === 0) return;
+      const retainedCredentialNames = new Set(context.projectSpec.credentials.map(c => c.name));
+      const hasOrphans = Object.keys(priorRecordedCredentials).some(name => !retainedCredentialNames.has(name));
+      if (!hasOrphans) return;
+
+      startStep('Clean up unused credentials');
+      const reconcileResult = await reconcileCredentialProviders({
+        region: target.region,
+        priorCredentials: priorRecordedCredentials,
+        retainedCredentialNames,
+      });
+      for (const { providerName, error } of reconcileResult.errors) {
+        logger.log(`Failed to delete unused credential provider '${providerName}': ${error.message}`, 'warn');
+      }
+      endStep(reconcileResult.errors.length > 0 ? 'error' : 'success');
+    };
+
+    if (!context.isTeardownDeploy) {
+      await reconcileUnusedCredentials();
+    }
+
     if (context.isTeardownDeploy) {
       // Clean up imperative payment credential providers (CFN stack delete handles manager/connector/roles).
       // Harnesses are part of the CloudFormation stack, so stack destroy handles them.
@@ -564,6 +605,10 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
       }
       endStep('success');
 
+      // Now that the stack (and the harness/agent referencing them) is gone, reap the
+      // imperative OAuth2/ApiKey providers it left behind.
+      await reconcileUnusedCredentials();
+
       logger.finalize(true);
 
       return {

src/cli/operations/deploy/__tests__/pre-deploy-identity.test.ts

@@ -1,6 +1,7 @@
 import {
   getAllCredentials,
   hasIdentityOAuthProviders,
+  reconcileCredentialProviders,
   setupApiKeyProviders,
   setupOAuth2Providers,
 } from '../pre-deploy-identity.js';
@@ -15,6 +16,8 @@ const {
   mockOAuth2ProviderExists,
   mockCreateOAuth2Provider,
   mockUpdateOAuth2Provider,
+  mockDeleteOAuth2Provider,
+  mockDeleteApiKeyProvider,
 } = vi.hoisted(() => ({
   mockKmsSend: vi.fn(),
   mockControlSend: vi.fn(),
@@ -24,6 +27,8 @@ const {
   mockOAuth2ProviderExists: vi.fn(),
   mockCreateOAuth2Provider: vi.fn(),
   mockUpdateOAuth2Provider: vi.fn(),
+  mockDeleteOAuth2Provider: vi.fn(),
+  mockDeleteApiKeyProvider: vi.fn(),
 }));
 
 vi.mock('@aws-sdk/client-kms', () => ({
@@ -47,10 +52,12 @@ vi.mock('@aws-sdk/client-bedrock-agentcore-control', () => ({
 vi.mock('../../identity/index.js', () => ({
   apiKeyProviderExists: vi.fn(),
   createApiKeyProvider: vi.fn(),
+  deleteApiKeyProvider: mockDeleteApiKeyProvider,
   setTokenVaultKmsKey: mockSetTokenVaultKmsKey,
   updateApiKeyProvider: vi.fn(),
   oAuth2ProviderExists: mockOAuth2ProviderExists,
   createOAuth2Provider: mockCreateOAuth2Provider,
+  deleteOAuth2Provider: mockDeleteOAuth2Provider,
   updateOAuth2Provider: mockUpdateOAuth2Provider,
 }));
 
@@ -399,3 +406,102 @@ describe('setupOAuth2Providers', () => {
     expect(result.results[0]!.error).toBe('Creation failed');
   });
 });
+
+describe('reconcileCredentialProviders', () => {
+  const ARN_BASE = 'arn:aws:bedrock-agentcore:us-east-1:123456789012:token-vault/default';
+  const oauth2Arn = (name: string) => `${ARN_BASE}/oauth2credentialprovider/${name}`;
+  const apiKeyArn = (name: string) => `${ARN_BASE}/apikeycredentialprovider/${name}`;
+  const paymentArn = (name: string) => `${ARN_BASE}/paymentcredentialprovider/${name}`;
+
+  beforeEach(() => {
+    mockGetCredentialProvider.mockReturnValue({});
+    mockDeleteOAuth2Provider.mockResolvedValue({ success: true });
+    mockDeleteApiKeyProvider.mockResolvedValue({ success: true });
+  });
+
+  afterEach(() => vi.clearAllMocks());
+
+  it('deletes an OAuth2 provider no longer in the spec, routing by ARN', async () => {
+    const result = await reconcileCredentialProviders({
+      region: 'us-east-1',
+      priorCredentials: { 'my-harness-oauth': { credentialProviderArn: oauth2Arn('my-harness-oauth') } },
+      retainedCredentialNames: new Set(),
+    });
+
+    expect(mockDeleteOAuth2Provider).toHaveBeenCalledWith(expect.anything(), 'my-harness-oauth');
+    expect(mockDeleteApiKeyProvider).not.toHaveBeenCalled();
+    expect(result.deleted).toEqual(['my-harness-oauth']);
+    expect(result.errors).toEqual([]);
+  });
+
+  it('deletes an ApiKey provider no longer in the spec, routing by ARN', async () => {
+    const result = await reconcileCredentialProviders({
+      region: 'us-east-1',
+      priorCredentials: { 'gemini-key': { credentialProviderArn: apiKeyArn('gemini-key') } },
+      retainedCredentialNames: new Set(),
+    });
+
+    expect(mockDeleteApiKeyProvider).toHaveBeenCalledWith(expect.anything(), 'gemini-key');
+    expect(mockDeleteOAuth2Provider).not.toHaveBeenCalled();
+    expect(result.deleted).toEqual(['gemini-key']);
+  });
+
+  it('keeps providers still referenced by the spec', async () => {
+    const result = await reconcileCredentialProviders({
+      region: 'us-east-1',
+      priorCredentials: {
+        'kept-oauth': { credentialProviderArn: oauth2Arn('kept-oauth') },
+        'gone-oauth': { credentialProviderArn: oauth2Arn('gone-oauth') },
+      },
+      retainedCredentialNames: new Set(['kept-oauth']),
+    });
+
+    expect(mockDeleteOAuth2Provider).toHaveBeenCalledTimes(1);
+    expect(mockDeleteOAuth2Provider).toHaveBeenCalledWith(expect.anything(), 'gone-oauth');
+    expect(result.deleted).toEqual(['gone-oauth']);
+  });
+
+  it('never touches payment provider ARNs (handled separately)', async () => {
+    const result = await reconcileCredentialProviders({
+      region: 'us-east-1',
+      priorCredentials: { 'pay-cred': { credentialProviderArn: paymentArn('pay-cred') } },
+      retainedCredentialNames: new Set(),
+    });
+
+    expect(mockDeleteOAuth2Provider).not.toHaveBeenCalled();
+    expect(mockDeleteApiKeyProvider).not.toHaveBeenCalled();
+    expect(result.deleted).toEqual([]);
+    expect(result.errors).toEqual([]);
+  });
+
+  it('collects delete failures without throwing', async () => {
+    mockDeleteOAuth2Provider.mockResolvedValue({ success: false, error: new Error('quota throttled') });
+
+    const result = await reconcileCredentialProviders({
+      region: 'us-east-1',
+      priorCredentials: { 'bad-oauth': { credentialProviderArn: oauth2Arn('bad-oauth') } },
+      retainedCredentialNames: new Set(),
+    });
+
+    expect(result.deleted).toEqual([]);
+    expect(result.errors).toHaveLength(1);
+    expect(result.errors[0]!.providerName).toBe('bad-oauth');
+    expect(result.errors[0]!.error.message).toBe('quota throttled');
+  });
+
+  it('deletes all recorded providers on teardown (empty retained set)', async () => {
+    const result = await reconcileCredentialProviders({
+      region: 'us-east-1',
+      priorCredentials: {
+        'h-oauth': { credentialProviderArn: oauth2Arn('h-oauth') },
+        'k-key': { credentialProviderArn: apiKeyArn('k-key') },
+      },
+      retainedCredentialNames: new Set(),
+    });
+
+    expect(mockDeleteOAuth2Provider).toHaveBeenCalledWith(expect.anything(), 'h-oauth');
+    expect(mockDeleteApiKeyProvider).toHaveBeenCalledWith(expect.anything(), 'k-key');
+    expect(result.deleted).toEqual(expect.arrayContaining(['h-oauth', 'k-key']));
+    expect(result.deleted).toHaveLength(2);
+  });
+});

src/cli/operations/deploy/index.ts

@@ -47,9 +47,11 @@ export {
   setupPaymentCredentialProviders,
   hasPaymentCredentialProviders,
   cleanupPaymentCredentialProviders,
+  reconcileCredentialProviders,
   type SetupPaymentCredentialProvidersOptions,
   type PaymentCredentialProvidersResult,
   type PaymentCredentialProviderResult,
+  type ReconcileCredentialProvidersResult,
 } from './pre-deploy-identity';
 
 // Post-deploy observability setup

src/cli/operations/deploy/pre-deploy-identity.ts

@@ -7,7 +7,7 @@ import {
   readEnvFile,
   toError,
 } from '../../../lib';
-import type { AgentCoreProjectSpec, Credential } from '../../../schema';
+import type { AgentCoreProjectSpec, Credential, CredentialDeployedState } from '../../../schema';
 import { getCredentialProvider } from '../../aws';
 import {
   createPaymentCredentialProvider,
@@ -26,6 +26,8 @@ import {
   apiKeyProviderExists,
   createApiKeyProvider,
   createOAuth2Provider,
+  deleteApiKeyProvider,
+  deleteOAuth2Provider,
   oAuth2ProviderExists,
   setTokenVaultKmsKey,
   updateApiKeyProvider,
@@ -596,6 +598,74 @@ export async function cleanupPaymentCredentialProviders(options: {
   }
 }
 
+// ─────────────────────────────────────────────────────────────────────────────
+// Orphaned Credential Provider Reconciliation
+// ─────────────────────────────────────────────────────────────────────────────
+
+/** ARN resource segment that identifies an OAuth2 credential provider. */
+const OAUTH2_PROVIDER_ARN_SEGMENT = '/oauth2credentialprovider/';
+/** ARN resource segment that identifies an API key credential provider. */
+const API_KEY_PROVIDER_ARN_SEGMENT = '/apikeycredentialprovider/';
+
+export interface ReconcileCredentialProvidersResult {
+  deleted: string[];
+  errors: { providerName: string; error: Error }[];
+}
+
+/**
+ * Delete OAuth2 and API key credential providers the CLI created on a prior deploy
+ * but that are no longer referenced by the project spec.
+ *
+ * OAuth2 and API key providers are created imperatively pre-deploy, outside the
+ * CloudFormation stack (see {@link setupOAuth2Providers} / {@link setupApiKeyProviders}),
+ * so stack teardown never removes them and they accumulate against the account's
+ * 50-provider quota. This reconciles AWS with the spec on every deploy: tearing the
+ * project down (empty spec) deletes all recorded providers; removing a single
+ * credential deletes just that one.
+ *
+ * Safety:
+ * - Operates ONLY on providers recorded in deployed state — i.e. providers the CLI
+ *   created. Imported/pre-existing providers are never recorded (they are skipped at
+ *   creation time for lacking a discoveryUrl), so they are never deleted here.
+ * - Routes by ARN resource segment, so payment provider ARNs recorded in the same map
+ *   are left untouched (payments have their own {@link cleanupPaymentCredentialProviders}).
+ * - Best-effort: a delete failure is collected, not thrown, so it never aborts teardown.
+ */
+export async function reconcileCredentialProviders(options: {
+  region: string;
+  /** Providers recorded in deployed state on the prior deploy (CLI-created only). */
+  priorCredentials: Record<string, CredentialDeployedState>;
+  /** Credential names still present in the project spec (these are kept). */
+  retainedCredentialNames: Set<string>;
+}): Promise<ReconcileCredentialProvidersResult> {
+  const { region, priorCredentials, retainedCredentialNames } = options;
+  const result: ReconcileCredentialProvidersResult = { deleted: [], errors: [] };
+
+  // maxAttempts > 1 so a throttled/transient delete is retried by the SDK rather than
+  // silently left as an orphan (this runs alongside other e2e shards hammering the same APIs).
+  const client = new BedrockAgentCoreControlClient({ region, credentials: getCredentialProvider(), maxAttempts: 5 });
+
+  for (const [providerName, state] of Object.entries(priorCredentials)) {
+    if (retainedCredentialNames.has(providerName)) continue;
+
+    const arn = state.credentialProviderArn;
+    const deleteResult = arn.includes(OAUTH2_PROVIDER_ARN_SEGMENT)
+      ? await deleteOAuth2Provider(client, providerName)
+      : arn.includes(API_KEY_PROVIDER_ARN_SEGMENT)
+        ? await deleteApiKeyProvider(client, providerName)
+        : undefined; // payment or unknown ARN — not ours to reap here
+
+    if (deleteResult === undefined) continue;
+    if (deleteResult.success) {
+      result.deleted.push(providerName);
+    } else {
+      result.errors.push({ providerName, error: deleteResult.error });
+    }
+  }
+
+  return result;
+}
+
 // ── Payment Credential Provider Helper ────────────────────────────────────
 
 interface CreateOrUpdatePaymentCredentialProviderOptions {