From 2a8babe66565dd1840817a38f0068840083a38a7 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 16:58:56 +0000 Subject: [PATCH 01/15] feat(secrets): add sensitive-key registry for at-rest encryption --- .../secrets/__tests__/sensitive-keys.test.ts | 27 ++++++++++++++ src/lib/secrets/sensitive-keys.ts | 36 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/lib/secrets/__tests__/sensitive-keys.test.ts create mode 100644 src/lib/secrets/sensitive-keys.ts diff --git a/src/lib/secrets/__tests__/sensitive-keys.test.ts b/src/lib/secrets/__tests__/sensitive-keys.test.ts new file mode 100644 index 000000000..28e167c98 --- /dev/null +++ b/src/lib/secrets/__tests__/sensitive-keys.test.ts @@ -0,0 +1,27 @@ +import { isSensitiveKey } from '../sensitive-keys'; +import { describe, expect, it } from 'vitest'; + +describe('isSensitiveKey', () => { + it.each([ + 'AGENTCORE_CREDENTIAL_MGR_CONN_API_KEY_SECRET', + 'AGENTCORE_CREDENTIAL_MGR_CONN_WALLET_SECRET', + 'AGENTCORE_CREDENTIAL_MGR_CONN_AUTHORIZATION_PRIVATE_KEY', + 'AGENTCORE_CREDENTIAL_MGR_CONN_APP_SECRET', + 'AGENTCORE_CREDENTIAL_GW_CLIENT_SECRET', + 'SOME_API_KEY', + 'AGENTCORE_CREDENTIAL_MYMODELKEY', // bare model-provider key + ])('treats %s as sensitive', key => { + expect(isSensitiveKey(key)).toBe(true); + }); + + it.each([ + 'AGENTCORE_CREDENTIAL_MGR_CONN_API_KEY_ID', + 'AGENTCORE_CREDENTIAL_MGR_CONN_APP_ID', + 'AGENTCORE_CREDENTIAL_MGR_CONN_AUTHORIZATION_ID', + 'AGENTCORE_CREDENTIAL_GW_CLIENT_ID', + 'SOME_RANDOM_CONFIG', + 'PORT', + ])('treats %s as non-sensitive', key => { + expect(isSensitiveKey(key)).toBe(false); + }); +}); diff --git a/src/lib/secrets/sensitive-keys.ts b/src/lib/secrets/sensitive-keys.ts new file mode 100644 index 000000000..530e56b44 --- /dev/null +++ b/src/lib/secrets/sensitive-keys.ts @@ -0,0 +1,36 @@ +/** + * Single source of truth for "is this env value a secret that must be encrypted + * at rest." Used by env.ts on write/read; extend with one entry when a new + * secret-bearing credential field is introduced anywhere in the CLI. + */ + +/** Suffixes whose values are secrets. */ +const SECRET_SUFFIXES = [ + '_API_KEY_SECRET', + '_WALLET_SECRET', + '_AUTHORIZATION_PRIVATE_KEY', + '_APP_SECRET', + '_CLIENT_SECRET', + '_API_KEY', +]; + +/** Reference/identifier suffixes that are NOT secrets (stay readable). */ +const REFERENCE_SUFFIXES = ['_API_KEY_ID', '_APP_ID', '_CLIENT_ID', '_AUTHORIZATION_ID']; + +export const SENSITIVE_KEY_PATTERNS: RegExp[] = SECRET_SUFFIXES.map(s => new RegExp(`${s}$`)); + +/** + * Model-provider API keys are stored as the bare `AGENTCORE_CREDENTIAL_` + * (no secret suffix). Treat such a credential var as a secret UNLESS it ends in + * a known reference suffix. + */ +function isBareModelCredential(key: string): boolean { + if (!key.startsWith('AGENTCORE_CREDENTIAL_')) return false; + return !REFERENCE_SUFFIXES.some(suffix => key.endsWith(suffix)); +} + +export function isSensitiveKey(key: string): boolean { + if (REFERENCE_SUFFIXES.some(suffix => key.endsWith(suffix))) return false; + if (SENSITIVE_KEY_PATTERNS.some(re => re.test(key))) return true; + return isBareModelCredential(key); +} From 52a95e66f7a30eb4d51065f272bfa8d8c2bdcd3b Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:01:26 +0000 Subject: [PATCH 02/15] feat(errors): add SecretEncryptionError and SecretDecryptionError --- src/cli/telemetry/schemas/common-shapes.ts | 2 ++ src/lib/errors/types.ts | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index 3a11954e4..0e08f3af7 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -125,6 +125,8 @@ export const ErrorName = z.enum([ 'PollExhaustedError', 'PollTimeoutError', 'ResourceNotFoundError', + 'SecretDecryptionError', + 'SecretEncryptionError', 'ServiceError', 'ServiceQuotaError', 'ShellKickedError', diff --git a/src/lib/errors/types.ts b/src/lib/errors/types.ts index e7e7f7c33..2993f90ea 100644 --- a/src/lib/errors/types.ts +++ b/src/lib/errors/types.ts @@ -61,6 +61,26 @@ export class AccessDeniedError extends BaseError { } } +/** + * Error thrown when a secret value cannot be encrypted before writing to disk + * (e.g. the machine encryption key could not be created/read). + */ +export class SecretEncryptionError extends BaseError { + constructor(message: string, options?: BaseErrorOptions) { + super(message, { defaultSource: 'client', ...options }); + } +} + +/** + * Error thrown when a stored secret value cannot be decrypted (missing/changed + * machine key, or corrupted ciphertext). + */ +export class SecretDecryptionError extends BaseError { + constructor(message: string, options?: BaseErrorOptions) { + super(message, { defaultSource: 'user', ...options }); + } +} + /** * Error indicating missing system dependencies required for an operation. */ From f6cb1d28ed36297448c0a01867c1e5382991c516 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:03:56 +0000 Subject: [PATCH 03/15] feat(secrets): add AES-256-GCM cipher for at-rest secret envelopes --- src/lib/secrets/__tests__/cipher.test.ts | 33 ++++++++++++++++++++ src/lib/secrets/cipher.ts | 38 ++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 src/lib/secrets/__tests__/cipher.test.ts create mode 100644 src/lib/secrets/cipher.ts diff --git a/src/lib/secrets/__tests__/cipher.test.ts b/src/lib/secrets/__tests__/cipher.test.ts new file mode 100644 index 000000000..e688b6e41 --- /dev/null +++ b/src/lib/secrets/__tests__/cipher.test.ts @@ -0,0 +1,33 @@ +import { ENC_PREFIX, decryptSecret, encryptSecret } from '../cipher'; +import { randomBytes } from 'node:crypto'; +import { describe, expect, it } from 'vitest'; + +const KEY = randomBytes(32); + +describe('cipher', () => { + it('round-trips a secret', () => { + const token = encryptSecret('super-secret-value', KEY); + expect(token.startsWith(ENC_PREFIX)).toBe(true); + expect(token).not.toContain('super-secret-value'); + expect(decryptSecret(token, KEY)).toBe('super-secret-value'); + }); + + it('produces a distinct token each call (random IV)', () => { + expect(encryptSecret('x', KEY)).not.toBe(encryptSecret('x', KEY)); + }); + + it('throws SecretDecryptionError on a tampered token', () => { + const token = encryptSecret('value', KEY); + const tampered = token.slice(0, -2) + (token.endsWith('A') ? 'B' : 'A'); + expect(() => decryptSecret(tampered, KEY)).toThrow(/decrypt/i); + }); + + it('throws SecretDecryptionError on a wrong key', () => { + const token = encryptSecret('value', KEY); + expect(() => decryptSecret(token, randomBytes(32))).toThrow(/decrypt/i); + }); + + it('throws on a non-enc token', () => { + expect(() => decryptSecret('plaintext', KEY)).toThrow(); + }); +}); diff --git a/src/lib/secrets/cipher.ts b/src/lib/secrets/cipher.ts new file mode 100644 index 000000000..884c347c8 --- /dev/null +++ b/src/lib/secrets/cipher.ts @@ -0,0 +1,38 @@ +import { SecretDecryptionError } from '../errors/types'; +import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto'; + +export const ENC_PREFIX = 'enc:v1:'; + +const ALGORITHM = 'aes-256-gcm'; +const IV_BYTES = 12; +const TAG_BYTES = 16; + +/** Encrypt a plaintext secret to an `enc:v1:` envelope: base64(IV || tag || ciphertext). */ +export function encryptSecret(plaintext: string, key: Buffer): string { + const iv = randomBytes(IV_BYTES); + const cipher = createCipheriv(ALGORITHM, key, iv); + const ciphertext = Buffer.concat([cipher.update(plaintext, 'utf-8'), cipher.final()]); + const tag = cipher.getAuthTag(); + return ENC_PREFIX + Buffer.concat([iv, tag, ciphertext]).toString('base64'); +} + +/** Decrypt an `enc:v1:` envelope produced by encryptSecret. Throws SecretDecryptionError on any failure. */ +export function decryptSecret(token: string, key: Buffer): string { + if (!token.startsWith(ENC_PREFIX)) { + throw new SecretDecryptionError('Value is not an encrypted secret envelope.'); + } + try { + const raw = Buffer.from(token.slice(ENC_PREFIX.length), 'base64'); + const iv = raw.subarray(0, IV_BYTES); + const tag = raw.subarray(IV_BYTES, IV_BYTES + TAG_BYTES); + const ciphertext = raw.subarray(IV_BYTES + TAG_BYTES); + const decipher = createDecipheriv(ALGORITHM, key, iv); + decipher.setAuthTag(tag); + return Buffer.concat([decipher.update(ciphertext), decipher.final()]).toString('utf-8'); + } catch (err) { + throw new SecretDecryptionError( + 'Could not decrypt a stored secret in agentcore/.env.local — the encryption key may be missing or changed. Re-add the credential.', + { cause: err instanceof Error ? err : undefined } + ); + } +} From dd410f003a48072a4c4921a86a46fff7c06460a7 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:09:28 +0000 Subject: [PATCH 04/15] feat(secrets): add key-provider chain (OS keychain -> keyfile fallback) --- .../secrets/__tests__/key-provider.test.ts | 40 ++++++++++ src/lib/secrets/index.ts | 3 + src/lib/secrets/key-provider.ts | 80 +++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 src/lib/secrets/__tests__/key-provider.test.ts create mode 100644 src/lib/secrets/index.ts create mode 100644 src/lib/secrets/key-provider.ts diff --git a/src/lib/secrets/__tests__/key-provider.test.ts b/src/lib/secrets/__tests__/key-provider.test.ts new file mode 100644 index 000000000..33ea7689b --- /dev/null +++ b/src/lib/secrets/__tests__/key-provider.test.ts @@ -0,0 +1,40 @@ +import { resolveEncryptionKey } from '../key-provider'; +import { existsSync, mkdtempSync, rmSync, statSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +describe('resolveEncryptionKey (keyfile fallback)', () => { + let dir: string; + const prev = { cfg: process.env.AGENTCORE_CONFIG_DIR, noKc: process.env.AGENTCORE_DISABLE_KEYCHAIN }; + + beforeEach(async () => { + dir = mkdtempSync(join(tmpdir(), 'agentcore-key-')); + process.env.AGENTCORE_CONFIG_DIR = dir; + process.env.AGENTCORE_DISABLE_KEYCHAIN = '1'; + const { __resetKeyCacheForTests } = await import('../key-provider'); + __resetKeyCacheForTests(); + }); + afterEach(() => { + process.env.AGENTCORE_CONFIG_DIR = prev.cfg; + process.env.AGENTCORE_DISABLE_KEYCHAIN = prev.noKc; + rmSync(dir, { recursive: true, force: true }); + }); + + it('creates a 32-byte 0600 keyfile and returns the key', async () => { + const key = await resolveEncryptionKey(); + expect(key).toHaveLength(32); + const keyfile = join(dir, 'secrets.key'); + expect(existsSync(keyfile)).toBe(true); + // 0600 => mode & 0o777 === 0o600 (skip exact check on Windows) + if (process.platform !== 'win32') { + expect(statSync(keyfile).mode & 0o777).toBe(0o600); + } + }); + + it('returns a stable key across calls', async () => { + const a = await resolveEncryptionKey(); + const b = await resolveEncryptionKey(); + expect(Buffer.compare(a, b)).toBe(0); + }); +}); diff --git a/src/lib/secrets/index.ts b/src/lib/secrets/index.ts new file mode 100644 index 000000000..89161ab04 --- /dev/null +++ b/src/lib/secrets/index.ts @@ -0,0 +1,3 @@ +export { isSensitiveKey, SENSITIVE_KEY_PATTERNS } from './sensitive-keys'; +export { ENC_PREFIX, encryptSecret, decryptSecret } from './cipher'; +export { resolveEncryptionKey } from './key-provider'; diff --git a/src/lib/secrets/key-provider.ts b/src/lib/secrets/key-provider.ts new file mode 100644 index 000000000..7871e198a --- /dev/null +++ b/src/lib/secrets/key-provider.ts @@ -0,0 +1,80 @@ +import { SecretEncryptionError } from '../errors/types'; +import { randomBytes } from 'node:crypto'; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +const KEY_BYTES = 32; +const KEYCHAIN_SERVICE = 'aws-agentcore'; +const KEYCHAIN_ACCOUNT = 'env-local-secret-key'; + +let cachedKey: Buffer | null = null; + +interface KeyringEntry { + getPassword(): string | null; + setPassword(password: string): void; +} + +interface KeyringModule { + Entry: new (service: string, account: string) => KeyringEntry; +} + +/** Keychain is opt-out (headless/CI) and best-effort; any failure falls through to the keyfile. */ +async function tryKeychainKey(): Promise { + if (process.env.AGENTCORE_DISABLE_KEYCHAIN === '1') return null; + try { + // Optional native dependency — dynamic import so a missing/unbuildable + // module degrades to the keyfile instead of failing the CLI. + // @ts-expect-error — @napi-rs/keyring is not declared in package.json; absent by design. + const { Entry } = (await import('@napi-rs/keyring')) as KeyringModule; + const entry = new Entry(KEYCHAIN_SERVICE, KEYCHAIN_ACCOUNT); + try { + const existing = entry.getPassword(); + if (existing) return Buffer.from(existing, 'base64'); + } catch { + // no stored password yet + } + const key = randomBytes(KEY_BYTES); + entry.setPassword(key.toString('base64')); + return key; + } catch { + return null; + } +} + +function keyfilePath(): string { + const configDir = process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore'); + return join(configDir, 'secrets.key'); +} + +function keyfileKey(): Buffer { + const path = keyfilePath(); + const configDir = process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore'); + try { + if (existsSync(path)) { + const key = readFileSync(path); + if (key.length === KEY_BYTES) return key; + } + mkdirSync(configDir, { recursive: true }); + const key = randomBytes(KEY_BYTES); + writeFileSync(path, key, { mode: 0o600 }); + return key; + } catch (err) { + throw new SecretEncryptionError(`Could not create or read the machine encryption key at ${path}.`, { + cause: err instanceof Error ? err : undefined, + }); + } +} + +/** Resolve the 32-byte machine-local encryption key: keychain first, keyfile fallback. */ +export async function resolveEncryptionKey(): Promise { + if (cachedKey) return cachedKey; + const fromKeychain = await tryKeychainKey(); + cachedKey = fromKeychain ?? keyfileKey(); + return cachedKey; +} + +/** Reset the per-process key cache. Only for use in tests. */ +export function __resetKeyCacheForTests(): void { + cachedKey = null; +} From 0df71291b1c5cb3714c7adfc16d7f368acd777ee Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:12:24 +0000 Subject: [PATCH 05/15] refactor(secrets): extract resolveConfigDir helper in key-provider --- src/lib/secrets/key-provider.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib/secrets/key-provider.ts b/src/lib/secrets/key-provider.ts index 7871e198a..099c6e94c 100644 --- a/src/lib/secrets/key-provider.ts +++ b/src/lib/secrets/key-provider.ts @@ -42,20 +42,22 @@ async function tryKeychainKey(): Promise { } } +function resolveConfigDir(): string { + return process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore'); +} + function keyfilePath(): string { - const configDir = process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore'); - return join(configDir, 'secrets.key'); + return join(resolveConfigDir(), 'secrets.key'); } function keyfileKey(): Buffer { const path = keyfilePath(); - const configDir = process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore'); try { if (existsSync(path)) { const key = readFileSync(path); if (key.length === KEY_BYTES) return key; } - mkdirSync(configDir, { recursive: true }); + mkdirSync(resolveConfigDir(), { recursive: true }); const key = randomBytes(KEY_BYTES); writeFileSync(path, key, { mode: 0o600 }); return key; From 49f79389cb3d69f892d5edfc87d38968a298cfe4 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:15:45 +0000 Subject: [PATCH 06/15] feat(secrets): encrypt sensitive .env.local values at rest in env.ts --- src/lib/utils/__tests__/env.test.ts | 81 +++++++++++++++++++++++++- src/lib/utils/env.ts | 89 +++++++++++++++++------------ 2 files changed, 132 insertions(+), 38 deletions(-) diff --git a/src/lib/utils/__tests__/env.test.ts b/src/lib/utils/__tests__/env.test.ts index b1ec0f74c..cdf1bd122 100644 --- a/src/lib/utils/__tests__/env.test.ts +++ b/src/lib/utils/__tests__/env.test.ts @@ -1,8 +1,10 @@ -import { getEnvPath, getEnvVar, readEnvFile, setEnvVar, writeEnvFile } from '../env.js'; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs'; +import { ENC_PREFIX } from '../../secrets'; +import { __resetKeyCacheForTests } from '../../secrets/key-provider'; +import { getEnvPath, getEnvVar, readEnvFile, removeEnvVars, setEnvVar, writeEnvFile } from '../env.js'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; describe('getEnvPath', () => { it('joins configRoot with .env.local', () => { @@ -186,3 +188,76 @@ describe('setEnvVar', () => { } }); }); + +describe('env.ts at-rest encryption', () => { + let root: string; // acts as configRoot (contains .env.local) + let cfgDir: string; + const prev = { cfg: process.env.AGENTCORE_CONFIG_DIR, noKc: process.env.AGENTCORE_DISABLE_KEYCHAIN }; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'agentcore-env-')); + cfgDir = mkdtempSync(join(tmpdir(), 'agentcore-cfg-')); + process.env.AGENTCORE_CONFIG_DIR = cfgDir; + process.env.AGENTCORE_DISABLE_KEYCHAIN = '1'; + __resetKeyCacheForTests(); + }); + afterEach(() => { + process.env.AGENTCORE_CONFIG_DIR = prev.cfg; + process.env.AGENTCORE_DISABLE_KEYCHAIN = prev.noKc; + __resetKeyCacheForTests(); + rmSync(root, { recursive: true, force: true }); + rmSync(cfgDir, { recursive: true, force: true }); + }); + + it('encrypts sensitive values on write, leaves reference values plaintext', async () => { + await writeEnvFile( + { + AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET: 'sek-123', + AGENTCORE_CREDENTIAL_M_C_API_KEY_ID: 'id-abc', + }, + root + ); + const onDisk = readFileSync(join(root, '.env.local'), 'utf-8'); + expect(onDisk).not.toContain('sek-123'); + expect(onDisk).toContain(`API_KEY_SECRET="${ENC_PREFIX}`); + expect(onDisk).toContain('API_KEY_ID="id-abc"'); // reference stays plaintext + }); + + it('decrypts transparently on read', async () => { + await writeEnvFile({ AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET: 'sek-123' }, root); + const env = await readEnvFile(root); + expect(env.AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET).toBe('sek-123'); + }); + + it('reads legacy plaintext secret and re-encrypts it on next write (self-migration)', async () => { + mkdirSync(root, { recursive: true }); + writeFileSync(join(root, '.env.local'), 'AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET="legacy-plain"\n'); + // read returns plaintext untouched + expect((await readEnvFile(root)).AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET).toBe('legacy-plain'); + // any write re-encrypts all sensitive values + await writeEnvFile({ OTHER: 'x' }, root); + const onDisk = readFileSync(join(root, '.env.local'), 'utf-8'); + expect(onDisk).not.toContain('legacy-plain'); + expect(onDisk).toContain(`API_KEY_SECRET="${ENC_PREFIX}`); + expect((await readEnvFile(root)).AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET).toBe('legacy-plain'); + }); + + it('does not double-encrypt an already-encrypted value', async () => { + await writeEnvFile({ AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET: 'sek-123' }, root); + await writeEnvFile({ OTHER: 'y' }, root); // merge re-writes existing values + const env = await readEnvFile(root); + expect(env.AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET).toBe('sek-123'); + }); + + it('removeEnvVars rewrites remaining secrets still encrypted', async () => { + await writeEnvFile( + { AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET: 'sek-1', AGENTCORE_CREDENTIAL_M_D_API_KEY_SECRET: 'sek-2' }, + root + ); + await removeEnvVars(['AGENTCORE_CREDENTIAL_M_C_API_KEY_SECRET'], root); + const onDisk = readFileSync(join(root, '.env.local'), 'utf-8'); + expect(onDisk).not.toContain('sek-1'); + expect(onDisk).not.toContain('sek-2'); + expect((await readEnvFile(root)).AGENTCORE_CREDENTIAL_M_D_API_KEY_SECRET).toBe('sek-2'); + }); +}); diff --git a/src/lib/utils/env.ts b/src/lib/utils/env.ts index f3884e096..bbdc3fb5a 100644 --- a/src/lib/utils/env.ts +++ b/src/lib/utils/env.ts @@ -1,5 +1,6 @@ import { ENV_FILE } from '../constants'; import { findConfigRoot } from '../schemas/io/path-resolver'; +import { ENC_PREFIX, decryptSecret, encryptSecret, isSensitiveKey, resolveEncryptionKey } from '../secrets'; import { parse } from 'dotenv'; import { existsSync } from 'fs'; import { readFile, writeFile } from 'fs/promises'; @@ -15,13 +16,35 @@ export function getEnvPath(configRoot?: string): string { return join(root, ENV_FILE); } +/** Escape and serialize an env record to dotenv `KEY="value"` lines. */ +function serializeEnv(env: Record): string { + const lines = Object.entries(env) + .map(([k, v]) => { + if (v === undefined || v === null) return ''; + if (!/^[a-z_][a-z0-9_]*$/i.test(k)) throw new Error(`Invalid env key: ${k}`); + const val = String(v).replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n').replace(/\r/g, '\\r'); + return `${k}="${val}"`; + }) + .filter(Boolean); + return lines.length > 0 ? lines.join('\n') + '\n' : ''; +} + /** - * Read agentcore/.env.local. + * Read agentcore/.env.local. Sensitive values stored as `enc:v1:` envelopes are + * decrypted transparently; legacy plaintext values are returned as-is. */ export async function readEnvFile(configRoot?: string): Promise> { const path = getEnvPath(configRoot); if (!existsSync(path)) return {}; - return parse(await readFile(path, 'utf-8')); + const parsed = parse(await readFile(path, 'utf-8')); + const entries = Object.entries(parsed); + if (!entries.some(([, v]) => v.startsWith(ENC_PREFIX))) return parsed; + const key = await resolveEncryptionKey(); + const out: Record = {}; + for (const [k, v] of entries) { + out[k] = v.startsWith(ENC_PREFIX) ? decryptSecret(v, key) : v; + } + return out; } /** @@ -31,28 +54,34 @@ export async function getEnvVar(key: string, configRoot?: string): Promise): Promise> { + const needsEncryption = Object.entries(env).some( + ([k, v]) => isSensitiveKey(k) && v !== undefined && v !== null && !String(v).startsWith(ENC_PREFIX) + ); + if (!needsEncryption) return env; + const key = await resolveEncryptionKey(); + const out: Record = {}; + for (const [k, v] of Object.entries(env)) { + out[k] = + isSensitiveKey(k) && v !== undefined && v !== null && !String(v).startsWith(ENC_PREFIX) + ? encryptSecret(String(v), key) + : v; + } + return out; +} + /** - * Write to agentcore/.env.local. Merges with existing values by default. + * Write to agentcore/.env.local. Merges with existing values by default and + * encrypts sensitive values at rest. */ export async function writeEnvFile(updates: Record, configRoot?: string, merge = true): Promise { const path = getEnvPath(configRoot); - const current = merge ? await readEnvFile(configRoot) : {}; - const env = { ...current, ...updates }; - - const content = - Object.entries(env) - .map(([k, v]) => { - if (v === undefined || v === null) return ''; - if (!/^[a-z_][a-z0-9_]*$/i.test(k)) throw new Error(`Invalid env key: ${k}`); - - const val = String(v).replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n').replace(/\r/g, '\\r'); - - return `${k}="${val}"`; - }) - .filter(Boolean) - .join('\n') + '\n'; - - await writeFile(path, content); + // Merge against the RAW on-disk values (still encrypted) so we never decrypt + // then re-encrypt unchanged secrets; encryptSensitive skips enc:v1: values. + const current = merge && existsSync(path) ? parse(await readFile(path, 'utf-8')) : {}; + const env = await encryptSensitive({ ...current, ...updates }); + await writeFile(path, serializeEnv(env)); } /** @@ -67,19 +96,9 @@ export async function setEnvVar(key: string, value: string, configRoot?: string) */ export async function removeEnvVars(keys: string[], configRoot?: string): Promise { const path = getEnvPath(configRoot); - const current = await readEnvFile(configRoot); - for (const key of keys) { - delete current[key]; - } - const entries = Object.entries(current); - const content = - entries.length > 0 - ? entries - .map( - ([k, v]) => - `${k}="${String(v).replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n').replace(/\r/g, '\\r')}"` - ) - .join('\n') + '\n' - : ''; - await writeFile(path, content); + if (!existsSync(path)) return; + const current = parse(await readFile(path, 'utf-8')); // raw (encrypted) values + for (const key of keys) delete current[key]; + const env = await encryptSensitive(current); // re-encrypts any legacy plaintext among the remainder + await writeFile(path, serializeEnv(env)); } From 9a0426ee862311e21acf36af15e23c75160d55f7 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:20:16 +0000 Subject: [PATCH 07/15] fix(payment-connector): warn when secrets are passed as literal CLI flags --- src/cli/primitives/PaymentConnectorPrimitive.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/cli/primitives/PaymentConnectorPrimitive.ts b/src/cli/primitives/PaymentConnectorPrimitive.ts index 0dc308b45..8bc81548e 100644 --- a/src/cli/primitives/PaymentConnectorPrimitive.ts +++ b/src/cli/primitives/PaymentConnectorPrimitive.ts @@ -2,6 +2,7 @@ import { findConfigRoot, removeEnvVars, setEnvVar, toError } from '../../lib'; import type { AgentCoreProjectSpec, PaymentProvider } from '../../schema'; import { PaymentConnectorNameSchema, PaymentConnectorSchema, PaymentProviderSchema } from '../../schema'; import type { RemoveResult } from '../commands/remove/types'; +import { ANSI } from '../constants'; import { getErrorMessage } from '../errors'; import type { RemovalPreview, SchemaChange } from '../operations/remove/types'; import { requireTTY } from '../tui/guards/tty'; @@ -447,6 +448,20 @@ export class PaymentConnectorPrimitive extends BasePrimitive v !== undefined); + if (usedLiteralSecretFlag && !cliOptions.json) { + process.stderr.write( + `${ANSI.yellow}Warning: passing secrets as CLI flags exposes them to shell history and the ` + + `process table. Prefer interactive mode (run \`agentcore add payment-connector\` with no ` + + `secret flags) for masked entry.${ANSI.reset}\n` + ); + } + let result: Awaited>; if (provider === 'StripePrivy') { result = await this.add({ From edb0307d2d32c138a49f01f8b0322fd85ab98f12 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:24:12 +0000 Subject: [PATCH 08/15] chore(secrets): add optional @napi-rs/keyring + document at-rest encryption Adds @napi-rs/keyring@^1.3.0 as an optionalDependency so the OS keychain path in key-provider.ts is backed by a declared package. The native module installed successfully on this machine, so the now-redundant @ts-expect-error suppression comment is removed (it was guarding against "module not found"; with the dep declared and resolved, TypeScript no longer needs it). npm ci continues to succeed even when the native build fails on a given platform because it is optional. Updates docs/payments.md with the at-rest encryption subsection and a revised credential rotation section that recommends the re-add path. --- docs/payments.md | 20 ++- npm-shrinkwrap.json | 223 ++++++++++++++++++++++++++++++++ package.json | 3 + src/lib/secrets/key-provider.ts | 1 - 4 files changed, 243 insertions(+), 4 deletions(-) diff --git a/docs/payments.md b/docs/payments.md index 3f7e844c1..90da6f4f2 100644 --- a/docs/payments.md +++ b/docs/payments.md @@ -197,14 +197,28 @@ AGENTCORE_CREDENTIAL_{CREDENTIAL_NAME}_AUTHORIZATION_ID=... `{CREDENTIAL_NAME}` is the connector's credential name uppercased with hyphens replaced by underscores. For example, a credential named `my-cdp-creds` becomes `AGENTCORE_CREDENTIAL_MY_CDP_CREDS_API_KEY_ID`. +### Secret encryption at rest + +Connector secret values in `agentcore/.env.local` (API key secrets, wallet secrets, private keys) are encrypted at rest +with a machine-local key stored outside the project directory (your OS keychain, or `~/.agentcore/secrets.key` on +machines without a keychain). Copying or syncing the project directory does **not** expose the secrets — only the holder +of the machine key can decrypt them. Reference IDs (API key ID, app ID) remain readable. + +To rotate a credential, re-run `agentcore add payment-connector` with the new values (this re-encrypts immediately). +Editing `agentcore/.env.local` by hand still works: paste the new plaintext value and run `agentcore deploy` — the CLI +re-encrypts it on the next write. + ### Credential Rotation -To rotate credentials: +The preferred rotation path is to re-run `agentcore add payment-connector` with the new values — the CLI re-encrypts the +secrets immediately and there is no plaintext window. Hand-editing `.env.local` also works: paste the new plaintext +value and run `agentcore deploy -y` — the CLI re-encrypts on the next write. -1. Update the values in `agentcore/.env.local` +1. Re-run `agentcore add payment-connector` with the new values (preferred), **or** open `agentcore/.env.local`, paste + the new plaintext secret value, and save. 2. Run `agentcore deploy -y` -Deploy automatically updates the PaymentCredentialProvider on AWS with the new secret values. +Deploy automatically updates the PaymentCredentialProvider on AWS with the new (re-encrypted) secret values. ## Deploying with Payments diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 4b2154a23..bbca23fd8 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -95,6 +95,9 @@ "engines": { "node": ">=20" }, + "optionalDependencies": { + "@napi-rs/keyring": "^1.3.0" + }, "peerDependencies": { "aws-cdk-lib": "^2.258.0", "constructs": "^10.0.0" @@ -3925,6 +3928,226 @@ } } }, + "node_modules/@napi-rs/keyring": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring/-/keyring-1.3.0.tgz", + "integrity": "sha512-WrOw/bcXm0f9qHkumlT1QlArXSTWqaY9sunsDpOk+yCCorCKMxvWT/a3xko4EYHVdeZoh00yI2TydXn6eyICDA==", + "license": "MIT", + "optional": true, + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + }, + "optionalDependencies": { + "@napi-rs/keyring-darwin-arm64": "1.3.0", + "@napi-rs/keyring-darwin-x64": "1.3.0", + "@napi-rs/keyring-freebsd-x64": "1.3.0", + "@napi-rs/keyring-linux-arm-gnueabihf": "1.3.0", + "@napi-rs/keyring-linux-arm64-gnu": "1.3.0", + "@napi-rs/keyring-linux-arm64-musl": "1.3.0", + "@napi-rs/keyring-linux-riscv64-gnu": "1.3.0", + "@napi-rs/keyring-linux-x64-gnu": "1.3.0", + "@napi-rs/keyring-linux-x64-musl": "1.3.0", + "@napi-rs/keyring-win32-arm64-msvc": "1.3.0", + "@napi-rs/keyring-win32-ia32-msvc": "1.3.0", + "@napi-rs/keyring-win32-x64-msvc": "1.3.0" + } + }, + "node_modules/@napi-rs/keyring-darwin-arm64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-arm64/-/keyring-darwin-arm64-1.3.0.tgz", + "integrity": "sha512-pl76hJvdYUBn6I24bXiOBMA9nbDapo3I5B+f3OorjDU4dUMSypXeKbOVehJe8fhgTiH24flMyTS3aAIy43xegQ==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-darwin-x64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-x64/-/keyring-darwin-x64-1.3.0.tgz", + "integrity": "sha512-YcJtEV5LA3cvA4z3BurgxH5IhTsW1JfIvcAAcqcecwk06Si9F9NqkxbZVIfDwQ8oRHgaBmT3zZJnLAotCrVahw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-freebsd-x64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-freebsd-x64/-/keyring-freebsd-x64-1.3.0.tgz", + "integrity": "sha512-vlLf31TGhfRAaxLDBhg8b89ss0HHD/lyNmL5F3UjSaz5CUXElsJmKYq9fqA/B+cZKUEUcLHHGhF0I/CqcFdaVw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm-gnueabihf": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm-gnueabihf/-/keyring-linux-arm-gnueabihf-1.3.0.tgz", + "integrity": "sha512-KiWdMMu/Inz/bHHIAGrnF7r54FZDYXuHO6UFF/rhIrshUsxbMG1Rl9lEymNtqqsVo927G0VYcb02FzWQ3iBQRQ==", + "cpu": [ + "arm" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-gnu/-/keyring-linux-arm64-gnu-1.3.0.tgz", + "integrity": "sha512-eyKGpY40lm9Jvs1aD294XRH4y7+TlJM0YVAryZeXA6TX0mb4gMkxVXwSQv7MCwgah7raeUd0dKUb4BPAYIgcMg==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-musl": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-musl/-/keyring-linux-arm64-musl-1.3.0.tgz", + "integrity": "sha512-iIK6JWHXAJqDrEyLY3TmswwloVyt2vj+04TZnew+uSJ9gnDO8EwRbp3/iw3LpWaXiDO7VomGO6y8I0Id8uBZSw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-riscv64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-riscv64-gnu/-/keyring-linux-riscv64-gnu-1.3.0.tgz", + "integrity": "sha512-/PGqrwn6EwgtK6vccASSXJRfOSP4vN1F4ASsIQ+7MdrK6hNvAJ1FZPrIuD5gGGdxezo3F++To2Wq7DbuGIeuNQ==", + "cpu": [ + "riscv64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-gnu/-/keyring-linux-x64-gnu-1.3.0.tgz", + "integrity": "sha512-2PDK1WKWTu9lBGq9VvNEkSlQD3O7YwVpmnyN2M3cy4v7NJ/8gDMd9GXv3G+FVXN13uhp4gnnPBS+ScefmEeD2A==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-musl": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-musl/-/keyring-linux-x64-musl-1.3.0.tgz", + "integrity": "sha512-oJ2HkX8YUo46QBkn0pG+HuIKQNqr523q6vBobCn+P95s4C4K6/kLBqHY/1bg5J4ap31DzsznhnFKcfBNBsjCnw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-arm64-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-arm64-msvc/-/keyring-win32-arm64-msvc-1.3.0.tgz", + "integrity": "sha512-tOd3c/uAaeoE4ycVlmAdSvygz0Zt3zdca6Y7gokBeIbaRDWpjDIUOpU3MvML59XAaqyuKGsVVu0F/DZb1lHPmw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-ia32-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-ia32-msvc/-/keyring-win32-ia32-msvc-1.3.0.tgz", + "integrity": "sha512-sPSqeAFZMGqP1R++M2JTza7GQJJ/TpCo6JU6Vcd4jnebvOaEDs9b7eipakU1PJdSvhpC2yXMCNRk9gXfrhuwHQ==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-x64-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-x64-msvc/-/keyring-win32-x64-msvc-1.3.0.tgz", + "integrity": "sha512-4DnCWXwDc0HRKwyRlG5y0VhKZW2tNRQfKKfyj6IX/KWfDNyq9hn4n+GL1auyDcOO/v8PwnhmYo2+rOOqCkvvOg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, "node_modules/@napi-rs/wasm-runtime": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.1.4.tgz", diff --git a/package.json b/package.json index 7247ae70e..5cd149f53 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,9 @@ "yaml": "^2.8.3", "zod": "^4.3.5" }, + "optionalDependencies": { + "@napi-rs/keyring": "^1.3.0" + }, "peerDependencies": { "aws-cdk-lib": "^2.258.0", "constructs": "^10.0.0" diff --git a/src/lib/secrets/key-provider.ts b/src/lib/secrets/key-provider.ts index 099c6e94c..55fb0c25b 100644 --- a/src/lib/secrets/key-provider.ts +++ b/src/lib/secrets/key-provider.ts @@ -25,7 +25,6 @@ async function tryKeychainKey(): Promise { try { // Optional native dependency — dynamic import so a missing/unbuildable // module degrades to the keyfile instead of failing the CLI. - // @ts-expect-error — @napi-rs/keyring is not declared in package.json; absent by design. const { Entry } = (await import('@napi-rs/keyring')) as KeyringModule; const entry = new Entry(KEYCHAIN_SERVICE, KEYCHAIN_ACCOUNT); try { From e9b66f318cc6c80a352bcc036efe4f227834b77f Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:30:00 +0000 Subject: [PATCH 09/15] fix(build): mark @napi-rs/keyring external so esbuild does not bundle its native binary --- esbuild.config.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esbuild.config.mjs b/esbuild.config.mjs index a76f194af..7b88a3835 100644 --- a/esbuild.config.mjs +++ b/esbuild.config.mjs @@ -56,7 +56,7 @@ await esbuild.build({ banner: { js: `import { createRequire } from 'module'; import { fileURLToPath as __ef } from 'url'; import { dirname as __ed } from 'path'; const require = createRequire(import.meta.url); const __filename = __ef(import.meta.url); const __dirname = __ed(__filename);`, }, - external: ['fsevents', '@aws-cdk/toolkit-lib'], + external: ['fsevents', '@aws-cdk/toolkit-lib', '@napi-rs/keyring'], plugins: [optionalDepsPlugin, textLoaderPlugin], }); From 5d42e0ec08096221ec131b08fd711454a4c60bba Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:37:12 +0000 Subject: [PATCH 10/15] test(secrets): assert model-provider keys are encrypted at rest in .env.local --- .../__tests__/multi-agent-credentials.test.ts | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts b/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts index 1d4e0ddba..ac21735ef 100644 --- a/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts +++ b/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts @@ -1,3 +1,4 @@ +import { readEnvFile } from '../../../../lib/utils/env'; import { runCLI } from '../../../../test-utils/index.js'; import { randomUUID } from 'node:crypto'; import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'; @@ -47,6 +48,10 @@ describe('multi-agent credential behavior', () => { } } + async function readEnvDecrypted() { + return readEnvFile(join(projectDir, 'agentcore')); + } + describe('credential reuse with same API key', () => { it('first agent creates project-scoped credential', async () => { const result = await runCLI( @@ -76,9 +81,11 @@ describe('multi-agent credential behavior', () => { expect(spec.credentials).toHaveLength(1); expect(spec.credentials[0].name).toBe(`${projectName}Gemini`); - const env = await readEnvLocal(); - expect(env).toContain('AGENTCORE_CREDENTIAL_MULTIAGENTPROJGEMINI='); - expect(env).toContain('KEY1'); + const rawEnv = await readEnvLocal(); + expect(rawEnv).toContain('AGENTCORE_CREDENTIAL_MULTIAGENTPROJGEMINI='); + expect(rawEnv).not.toContain('KEY1'); // encrypted at rest + const env = await readEnvDecrypted(); + expect(env.AGENTCORE_CREDENTIAL_MULTIAGENTPROJGEMINI).toBe('KEY1'); }); it('second agent with same key reuses credential (no duplicate)', async () => { @@ -151,12 +158,15 @@ describe('multi-agent credential behavior', () => { // Should have 3 agents expect(spec.runtimes).toHaveLength(3); - // .env.local should have both keys - const env = await readEnvLocal(); - expect(env).toContain('AGENTCORE_CREDENTIAL_MULTIAGENTPROJGEMINI='); - expect(env).toContain('KEY1'); - expect(env).toContain('AGENTCORE_CREDENTIAL_MULTIAGENTPROJAGENT3GEMINI='); - expect(env).toContain('KEY2'); + // .env.local should have both keys encrypted at rest, decryptable to original values + const rawEnv = await readEnvLocal(); + expect(rawEnv).toContain('AGENTCORE_CREDENTIAL_MULTIAGENTPROJGEMINI='); + expect(rawEnv).toContain('AGENTCORE_CREDENTIAL_MULTIAGENTPROJAGENT3GEMINI='); + expect(rawEnv).not.toContain('KEY1'); // encrypted at rest + expect(rawEnv).not.toContain('KEY2'); // encrypted at rest + const env = await readEnvDecrypted(); + expect(env.AGENTCORE_CREDENTIAL_MULTIAGENTPROJGEMINI).toBe('KEY1'); + expect(env.AGENTCORE_CREDENTIAL_MULTIAGENTPROJAGENT3GEMINI).toBe('KEY2'); // Generated code should reference correct credentials const agent1Code = await readFile(join(projectDir, 'app/Agent1/model/load.py'), 'utf-8'); From 2f53e08561538d67349aebd1f675f854af1b88ac Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:46:37 +0000 Subject: [PATCH 11/15] test(secrets): assert OAuth client secret is encrypted at rest in add-agent-auth --- integ-tests/add-agent-auth.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/integ-tests/add-agent-auth.test.ts b/integ-tests/add-agent-auth.test.ts index c0fa734ca..67224a199 100644 --- a/integ-tests/add-agent-auth.test.ts +++ b/integ-tests/add-agent-auth.test.ts @@ -1,3 +1,4 @@ +import { readEnvFile } from '../src/lib/utils/env.js'; import { createTestProject, readProjectConfig, runCLI } from '../src/test-utils/index.js'; import type { TestProject } from '../src/test-utils/index.js'; import { readFile } from 'node:fs/promises'; @@ -119,11 +120,13 @@ describe('integration: add BYO agent with CUSTOM_JWT auth', () => { expect(oauthCred!.authorizerType).toBe('OAuthCredentialProvider'); expect((oauthCred as { managed?: boolean }).managed).toBe(true); - // Verify .env.local has client secrets (namespaced per credential) + // Client ID is a reference (plaintext); the client SECRET is encrypted at rest. const envPath = join(project.projectPath, 'agentcore', '.env.local'); const envContent = await readFile(envPath, 'utf-8'); expect(envContent).toContain('my-client-id'); - expect(envContent).toContain('my-client-secret'); + expect(envContent).not.toContain('my-client-secret'); // encrypted at rest + const env = await readEnvFile(join(project.projectPath, 'agentcore')); + expect(env.AGENTCORE_CREDENTIAL_AUTHAGENT2_OAUTH_CLIENT_SECRET).toBe('my-client-secret'); }); it('adds a BYO agent with default AWS_IAM auth (no auth flags)', async () => { From 9891bb7593b93afcfb2768f5c002ae0b62ce2037 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 26 Jun 2026 20:05:37 +0000 Subject: [PATCH 12/15] fix(secrets): decrypt with all candidate keys, fix warning placement and error text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bug-bash findings on the secret-at-rest feature: - Keychain/keyfile key mismatch: a value sealed by one key source (e.g. OS keychain) failed to decrypt when the active source differed (e.g. after AGENTCORE_DISABLE_KEYCHAIN was set), aborting deploy with an opaque error. readEnvFile now resolves ALL candidate keys (keychain + keyfile) and decryptSecret tries each, so a value decrypts regardless of which source is active. resolveCandidateKeys() is read-only (never creates a key). - The connector secret-leak warning was emitted AFTER format validation, which exits on a malformed secret — so a malformed-but-already-leaked secret was never warned. Move the warning before validation (the secret hits shell history the instant it is typed, well-formed or not). - The decrypt-failure message ended in a period that collided with the validate command's own `${message}.` wrapper ("credential.. Fix"). Drop the trailing period so the wrapped sentence reads cleanly. --- .../primitives/PaymentConnectorPrimitive.ts | 29 +++++++----- src/lib/secrets/__tests__/cipher.test.ts | 29 ++++++++++++ src/lib/secrets/cipher.ts | 35 ++++++++++---- src/lib/secrets/index.ts | 2 +- src/lib/secrets/key-provider.ts | 47 ++++++++++++++++++- src/lib/utils/env.ts | 15 ++++-- 6 files changed, 130 insertions(+), 27 deletions(-) diff --git a/src/cli/primitives/PaymentConnectorPrimitive.ts b/src/cli/primitives/PaymentConnectorPrimitive.ts index 8bc81548e..492fd607f 100644 --- a/src/cli/primitives/PaymentConnectorPrimitive.ts +++ b/src/cli/primitives/PaymentConnectorPrimitive.ts @@ -436,18 +436,11 @@ export class PaymentConnectorPrimitive extends BasePrimitive>; if (provider === 'StripePrivy') { result = await this.add({ diff --git a/src/lib/secrets/__tests__/cipher.test.ts b/src/lib/secrets/__tests__/cipher.test.ts index e688b6e41..a1791baf4 100644 --- a/src/lib/secrets/__tests__/cipher.test.ts +++ b/src/lib/secrets/__tests__/cipher.test.ts @@ -30,4 +30,33 @@ describe('cipher', () => { it('throws on a non-enc token', () => { expect(() => decryptSecret('plaintext', KEY)).toThrow(); }); + + it('decrypts when the correct key is among multiple candidates (order-independent)', () => { + const token = encryptSecret('multi-key-secret', KEY); + const wrong = randomBytes(32); + expect(decryptSecret(token, [wrong, KEY])).toBe('multi-key-secret'); + expect(decryptSecret(token, [KEY, wrong])).toBe('multi-key-secret'); + }); + + it('throws when NO candidate key works', () => { + const token = encryptSecret('value', KEY); + expect(() => decryptSecret(token, [randomBytes(32), randomBytes(32)])).toThrow(/decrypt/i); + }); + + it('throws on an empty candidate list', () => { + const token = encryptSecret('value', KEY); + expect(() => decryptSecret(token, [])).toThrow(/decrypt/i); + }); + + it('decrypt-failure message has no double-period when wrapped by a caller', () => { + const token = encryptSecret('value', KEY); + try { + decryptSecret(token, randomBytes(32)); + throw new Error('should have thrown'); + } catch (err) { + // The message must not end in a period, so a wrapping `${msg}.` reads cleanly. + expect((err as Error).message).not.toMatch(/\.$/); + expect((err as Error).message).toContain('Re-add the credential'); + } + }); }); diff --git a/src/lib/secrets/cipher.ts b/src/lib/secrets/cipher.ts index 884c347c8..34eba2821 100644 --- a/src/lib/secrets/cipher.ts +++ b/src/lib/secrets/cipher.ts @@ -16,11 +16,8 @@ export function encryptSecret(plaintext: string, key: Buffer): string { return ENC_PREFIX + Buffer.concat([iv, tag, ciphertext]).toString('base64'); } -/** Decrypt an `enc:v1:` envelope produced by encryptSecret. Throws SecretDecryptionError on any failure. */ -export function decryptSecret(token: string, key: Buffer): string { - if (!token.startsWith(ENC_PREFIX)) { - throw new SecretDecryptionError('Value is not an encrypted secret envelope.'); - } +/** Attempt to decrypt with a single key. Returns the plaintext or null on any failure (does NOT throw). */ +function tryDecrypt(token: string, key: Buffer): string | null { try { const raw = Buffer.from(token.slice(ENC_PREFIX.length), 'base64'); const iv = raw.subarray(0, IV_BYTES); @@ -29,10 +26,28 @@ export function decryptSecret(token: string, key: Buffer): string { const decipher = createDecipheriv(ALGORITHM, key, iv); decipher.setAuthTag(tag); return Buffer.concat([decipher.update(ciphertext), decipher.final()]).toString('utf-8'); - } catch (err) { - throw new SecretDecryptionError( - 'Could not decrypt a stored secret in agentcore/.env.local — the encryption key may be missing or changed. Re-add the credential.', - { cause: err instanceof Error ? err : undefined } - ); + } catch { + return null; + } +} + +/** + * Decrypt an `enc:v1:` envelope, trying each candidate key in turn. A value may + * have been sealed by the OS keychain or the keyfile; trying all available keys + * means a value still decrypts even if the active key source differs from the + * one that wrote it (e.g. AGENTCORE_DISABLE_KEYCHAIN toggled between add and a + * later read). Throws SecretDecryptionError only if no key works. + */ +export function decryptSecret(token: string, keys: Buffer | Buffer[]): string { + if (!token.startsWith(ENC_PREFIX)) { + throw new SecretDecryptionError('Value is not an encrypted secret envelope.'); + } + const candidates = Array.isArray(keys) ? keys : [keys]; + for (const key of candidates) { + const plaintext = tryDecrypt(token, key); + if (plaintext !== null) return plaintext; } + throw new SecretDecryptionError( + 'Could not decrypt a stored secret in agentcore/.env.local — the encryption key may be missing or changed. Re-add the credential' + ); } diff --git a/src/lib/secrets/index.ts b/src/lib/secrets/index.ts index 89161ab04..03755c0eb 100644 --- a/src/lib/secrets/index.ts +++ b/src/lib/secrets/index.ts @@ -1,3 +1,3 @@ export { isSensitiveKey, SENSITIVE_KEY_PATTERNS } from './sensitive-keys'; export { ENC_PREFIX, encryptSecret, decryptSecret } from './cipher'; -export { resolveEncryptionKey } from './key-provider'; +export { resolveEncryptionKey, resolveCandidateKeys } from './key-provider'; diff --git a/src/lib/secrets/key-provider.ts b/src/lib/secrets/key-provider.ts index 55fb0c25b..9a2f0a238 100644 --- a/src/lib/secrets/key-provider.ts +++ b/src/lib/secrets/key-provider.ts @@ -67,7 +67,36 @@ function keyfileKey(): Buffer { } } -/** Resolve the 32-byte machine-local encryption key: keychain first, keyfile fallback. */ +/** Read the keyfile key WITHOUT creating one if absent (read-only; returns null when missing). */ +function readKeyfileKeyIfPresent(): Buffer | null { + try { + const path = keyfilePath(); + if (existsSync(path)) { + const key = readFileSync(path); + if (key.length === KEY_BYTES) return key; + } + } catch { + // unreadable keyfile — treat as absent + } + return null; +} + +/** Read the keychain key WITHOUT creating one if absent (returns null when unavailable/missing). */ +async function readKeychainKeyIfPresent(): Promise { + if (process.env.AGENTCORE_DISABLE_KEYCHAIN === '1') return null; + try { + const { Entry } = (await import('@napi-rs/keyring')) as KeyringModule; + const existing = new Entry(KEYCHAIN_SERVICE, KEYCHAIN_ACCOUNT).getPassword(); + return existing ? Buffer.from(existing, 'base64') : null; + } catch { + return null; + } +} + +/** + * Resolve the single key used to ENCRYPT new values: keychain first, keyfile + * fallback. Creates a key if none exists yet. + */ export async function resolveEncryptionKey(): Promise { if (cachedKey) return cachedKey; const fromKeychain = await tryKeychainKey(); @@ -75,6 +104,22 @@ export async function resolveEncryptionKey(): Promise { return cachedKey; } +/** + * Resolve ALL candidate keys for DECRYPTION, newest-intent first. A stored + * value may have been sealed by the keychain on one run and the keyfile on + * another (e.g. AGENTCORE_DISABLE_KEYCHAIN toggled); trying every available key + * lets a value decrypt regardless of which source is currently active. Read-only + * — never creates a key. May be empty if no key material exists at all. + */ +export async function resolveCandidateKeys(): Promise { + const keys: Buffer[] = []; + const fromKeychain = await readKeychainKeyIfPresent(); + if (fromKeychain) keys.push(fromKeychain); + const fromKeyfile = readKeyfileKeyIfPresent(); + if (fromKeyfile && !keys.some(k => k.equals(fromKeyfile))) keys.push(fromKeyfile); + return keys; +} + /** Reset the per-process key cache. Only for use in tests. */ export function __resetKeyCacheForTests(): void { cachedKey = null; diff --git a/src/lib/utils/env.ts b/src/lib/utils/env.ts index bbdc3fb5a..d213683a0 100644 --- a/src/lib/utils/env.ts +++ b/src/lib/utils/env.ts @@ -1,6 +1,13 @@ import { ENV_FILE } from '../constants'; import { findConfigRoot } from '../schemas/io/path-resolver'; -import { ENC_PREFIX, decryptSecret, encryptSecret, isSensitiveKey, resolveEncryptionKey } from '../secrets'; +import { + ENC_PREFIX, + decryptSecret, + encryptSecret, + isSensitiveKey, + resolveCandidateKeys, + resolveEncryptionKey, +} from '../secrets'; import { parse } from 'dotenv'; import { existsSync } from 'fs'; import { readFile, writeFile } from 'fs/promises'; @@ -39,10 +46,12 @@ export async function readEnvFile(configRoot?: string): Promise v.startsWith(ENC_PREFIX))) return parsed; - const key = await resolveEncryptionKey(); + // Try every available key (keychain + keyfile) so a value sealed by either + // source decrypts even if the active source differs from the one that wrote it. + const keys = await resolveCandidateKeys(); const out: Record = {}; for (const [k, v] of entries) { - out[k] = v.startsWith(ENC_PREFIX) ? decryptSecret(v, key) : v; + out[k] = v.startsWith(ENC_PREFIX) ? decryptSecret(v, keys) : v; } return out; } From 6f76853631f7f707faf551fcdc84163f9d2561a2 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 26 Jun 2026 20:11:00 +0000 Subject: [PATCH 13/15] fix(secrets): reject credential names that would store the secret unencrypted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An API-key credential stores its secret in the bare AGENTCORE_CREDENTIAL_ env var. If normalizes to a reserved reference suffix (e.g. `my-client-id` → AGENTCORE_CREDENTIAL_MY_CLIENT_ID), isSensitiveKey can't distinguish it from a non-secret reference and the value was written PLAINTEXT at rest — defeating the encryption feature. The string ambiguity is unresolvable at write time, so fail closed at creation: a shared validateCredentialNameEncryptable() rejects such names. Wired into both the CLI add path (ValidationError) and the TUI name step (customValidation → red ✗, blocks Enter until fixed). OAuth credentials append _CLIENT_SECRET so their names are unaffected. --- src/cli/primitives/CredentialPrimitive.tsx | 11 +++++++ .../screens/identity/AddIdentityScreen.tsx | 10 ++++++- .../secrets/__tests__/sensitive-keys.test.ts | 27 ++++++++++++++++- src/lib/secrets/index.ts | 2 +- src/lib/secrets/sensitive-keys.ts | 29 +++++++++++++++++++ 5 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/cli/primitives/CredentialPrimitive.tsx b/src/cli/primitives/CredentialPrimitive.tsx index 5034c1c2c..fb8824946 100644 --- a/src/cli/primitives/CredentialPrimitive.tsx +++ b/src/cli/primitives/CredentialPrimitive.tsx @@ -9,6 +9,7 @@ import { toError, } from '../../lib'; import type { Result } from '../../lib/result'; +import { validateCredentialNameEncryptable } from '../../lib/secrets'; import type { Credential, ModelProvider } from '../../schema'; import { CredentialSchema } from '../../schema'; import { validateAddCredentialOptions } from '../commands/add/validate'; @@ -84,6 +85,16 @@ export class CredentialPrimitive extends BasePrimitive> { try { + // Fail closed: an API-key credential stores its secret in the bare + // AGENTCORE_CREDENTIAL_ var. If normalizes to a reserved + // reference suffix, the value would be left unencrypted at rest. OAuth + // appends _CLIENT_SECRET, so its name is always safe. + if (options.authorizerType === 'ApiKeyCredentialProvider') { + const nameCheck = validateCredentialNameEncryptable(options.name); + if (nameCheck !== true) { + return { success: false, error: new ValidationError(nameCheck) }; + } + } const credential = await this.createCredential(options); return { success: true, credentialName: credential.name }; } catch (err) { diff --git a/src/cli/tui/screens/identity/AddIdentityScreen.tsx b/src/cli/tui/screens/identity/AddIdentityScreen.tsx index bbf34661c..9656a1221 100644 --- a/src/cli/tui/screens/identity/AddIdentityScreen.tsx +++ b/src/cli/tui/screens/identity/AddIdentityScreen.tsx @@ -1,3 +1,4 @@ +import { validateCredentialNameEncryptable } from '../../../../lib/secrets'; import type { CredentialType } from '../../../../schema'; import { CredentialNameSchema } from '../../../../schema'; import { ConfirmReview, Panel, Screen, SecretInput, StepIndicator, TextInput, WizardSelect } from '../../components'; @@ -81,7 +82,14 @@ export function AddIdentityScreen({ onComplete, onExit, existingIdentityNames, i onSubmit={wizard.setName} onCancel={() => wizard.goBack()} schema={CredentialNameSchema} - customValidation={value => !existingIdentityNames.includes(value) || 'Credential name already exists'} + customValidation={value => { + if (existingIdentityNames.includes(value)) return 'Credential name already exists'; + // API-key credentials store the secret in the bare AGENTCORE_CREDENTIAL_ + // var; reject names that would leave it unencrypted. OAuth appends + // _CLIENT_SECRET, so its name is always safe. + if (!isOAuth) return validateCredentialNameEncryptable(value); + return true; + }} /> )} diff --git a/src/lib/secrets/__tests__/sensitive-keys.test.ts b/src/lib/secrets/__tests__/sensitive-keys.test.ts index 28e167c98..1be77cbdc 100644 --- a/src/lib/secrets/__tests__/sensitive-keys.test.ts +++ b/src/lib/secrets/__tests__/sensitive-keys.test.ts @@ -1,4 +1,4 @@ -import { isSensitiveKey } from '../sensitive-keys'; +import { isSensitiveKey, validateCredentialNameEncryptable } from '../sensitive-keys'; import { describe, expect, it } from 'vitest'; describe('isSensitiveKey', () => { @@ -25,3 +25,28 @@ describe('isSensitiveKey', () => { expect(isSensitiveKey(key)).toBe(false); }); }); + +describe('validateCredentialNameEncryptable', () => { + it.each([ + 'my-client-id', // → AGENTCORE_CREDENTIAL_MY_CLIENT_ID + 'my_client_id', + 'foo-api-key-id', + 'thing-app-id', + 'x-authorization-id', + 'PROD_CLIENT_ID', + ])('rejects a name that normalizes to a reserved reference suffix: %s', name => { + const result = validateCredentialNameEncryptable(name); + expect(result).not.toBe(true); + expect(typeof result).toBe('string'); + expect(result).toMatch(/would not be encrypted at rest/i); + }); + + it.each(['my-openai-key', 'anthropic', 'gemini', 'my-secret', 'client-id-prod', 'idle', 'rapid'])( + 'accepts a safe name: %s', + name => { + // includes tricky non-collisions: 'client-id-prod' does not END in the suffix; + // 'idle'/'rapid' merely contain "id" but do not end in a reference suffix. + expect(validateCredentialNameEncryptable(name)).toBe(true); + } + ); +}); diff --git a/src/lib/secrets/index.ts b/src/lib/secrets/index.ts index 03755c0eb..82b15e15a 100644 --- a/src/lib/secrets/index.ts +++ b/src/lib/secrets/index.ts @@ -1,3 +1,3 @@ -export { isSensitiveKey, SENSITIVE_KEY_PATTERNS } from './sensitive-keys'; +export { isSensitiveKey, SENSITIVE_KEY_PATTERNS, validateCredentialNameEncryptable } from './sensitive-keys'; export { ENC_PREFIX, encryptSecret, decryptSecret } from './cipher'; export { resolveEncryptionKey, resolveCandidateKeys } from './key-provider'; diff --git a/src/lib/secrets/sensitive-keys.ts b/src/lib/secrets/sensitive-keys.ts index 530e56b44..f8b9645d2 100644 --- a/src/lib/secrets/sensitive-keys.ts +++ b/src/lib/secrets/sensitive-keys.ts @@ -34,3 +34,32 @@ export function isSensitiveKey(key: string): boolean { if (SENSITIVE_KEY_PATTERNS.some(re => re.test(key))) return true; return isBareModelCredential(key); } + +/** + * Normalize a credential name into its env-var-suffix form (matches + * computeDefaultCredentialEnvVarName: hyphens → underscores, uppercased). + */ +function normalizeCredentialNameToKey(name: string): string { + return name.replace(/-/g, '_').toUpperCase(); +} + +/** + * Validate that an API-key credential NAME won't collide with a reserved + * reference suffix. A bare API-key credential is stored as + * `AGENTCORE_CREDENTIAL_` carrying the secret value directly; if + * normalizes to end in a reference suffix (e.g. `my-client-id` → `..._CLIENT_ID`) + * the value is indistinguishable from a non-secret reference and would be left + * UNENCRYPTED at rest. Reject such names at creation (fail closed). + * + * Returns `true` if the name is safe, or an error message string if not — the + * `(value) => true | string` contract used by both the CLI and the TUI's + * SecretInput/TextInput `customValidation`. + */ +export function validateCredentialNameEncryptable(name: string): true | string { + const normalized = normalizeCredentialNameToKey(name); + const collision = REFERENCE_SUFFIXES.find(suffix => normalized.endsWith(suffix)); + if (collision) { + return `Credential name "${name}" ends in a reserved reference suffix (${collision}) and its secret would not be encrypted at rest. Choose a name that does not end in id/client-id/app-id/authorization-id.`; + } + return true; +} From 62ab702e0cc831c00348c887491574ac4bb1c0f4 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 26 Jun 2026 20:45:14 +0000 Subject: [PATCH 14/15] fix: harden machine key handling and centralize the literal-secret-flag warning Addresses review feedback on the at-rest secret encryption work: - Keyfile fallback now fails loud on a wrong-sized (corrupt) key instead of silently overwriting it, and creates the file with the wx flag so two concurrent processes cannot clobber each other's freshly minted key (the EEXIST loser re-reads the winner's key). - Keychain path re-reads the entry after setPassword so concurrent writers converge on the same persisted key rather than diverging. - Extracted the 'secret passed as a literal CLI flag' warning into a shared warnOnLiteralSecretFlag helper and wired it into both the payment-connector and credential add paths, so the nudge toward masked interactive entry is consistent and suppressed under --json. - Isolated the keychain/keyfile in the agent-auth and multi-agent-credential tests via AGENTCORE_DISABLE_KEYCHAIN + a temp AGENTCORE_CONFIG_DIR so the suite never touches the developer's real OS keychain. Adds unit tests for the corrupt-keyfile-fails-loud and keyfile-reuse paths and for the shared warning helper. --- integ-tests/add-agent-auth.test.ts | 13 ++++++ .../__tests__/multi-agent-credentials.test.ts | 10 +++++ src/cli/primitives/CredentialPrimitive.tsx | 2 + .../primitives/PaymentConnectorPrimitive.ts | 25 +++++------- .../__tests__/secret-flag-warning.test.ts | 35 ++++++++++++++++ src/cli/primitives/secret-flag-warning.ts | 30 ++++++++++++++ .../secrets/__tests__/key-provider.test.ts | 19 ++++++++- src/lib/secrets/key-provider.ts | 40 +++++++++++++++++-- 8 files changed, 155 insertions(+), 19 deletions(-) create mode 100644 src/cli/primitives/__tests__/secret-flag-warning.test.ts create mode 100644 src/cli/primitives/secret-flag-warning.ts diff --git a/integ-tests/add-agent-auth.test.ts b/integ-tests/add-agent-auth.test.ts index 67224a199..db1fe68c2 100644 --- a/integ-tests/add-agent-auth.test.ts +++ b/integ-tests/add-agent-auth.test.ts @@ -1,16 +1,26 @@ import { readEnvFile } from '../src/lib/utils/env.js'; import { createTestProject, readProjectConfig, runCLI } from '../src/test-utils/index.js'; import type { TestProject } from '../src/test-utils/index.js'; +import { mkdtempSync, rmSync } from 'node:fs'; import { readFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; describe('integration: add BYO agent with CUSTOM_JWT auth', () => { let project: TestProject; + let keyDir: string; + const prevEnv = { kc: process.env.AGENTCORE_DISABLE_KEYCHAIN, cfg: process.env.AGENTCORE_CONFIG_DIR }; const agentName = 'AuthAgent'; const discoveryUrl = 'https://cognito-idp.us-east-1.amazonaws.com/us-east-1_test123/.well-known/openid-configuration'; beforeAll(async () => { + // Isolate secret-at-rest encryption from the developer's real OS keychain and + // ~/.agentcore: force the keyfile provider into a throwaway dir. cleanSpawnEnv + // inherits process.env, so spawned CLI processes pick these up too. + keyDir = mkdtempSync(join(tmpdir(), 'agentcore-integ-keys-')); + process.env.AGENTCORE_DISABLE_KEYCHAIN = '1'; + process.env.AGENTCORE_CONFIG_DIR = keyDir; project = await createTestProject({ noAgent: true, }); @@ -18,6 +28,9 @@ describe('integration: add BYO agent with CUSTOM_JWT auth', () => { afterAll(async () => { await project.cleanup(); + process.env.AGENTCORE_DISABLE_KEYCHAIN = prevEnv.kc; + process.env.AGENTCORE_CONFIG_DIR = prevEnv.cfg; + rmSync(keyDir, { recursive: true, force: true }); }); it('adds a BYO agent with CUSTOM_JWT authorizer and audience', async () => { diff --git a/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts b/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts index ac21735ef..3f78f8115 100644 --- a/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts +++ b/src/cli/commands/add/__tests__/multi-agent-credentials.test.ts @@ -18,11 +18,19 @@ describe('multi-agent credential behavior', () => { let testDir: string; let projectDir: string; const projectName = 'MultiAgentProj'; + const prevEnv = { kc: process.env.AGENTCORE_DISABLE_KEYCHAIN, cfg: process.env.AGENTCORE_CONFIG_DIR }; beforeAll(async () => { testDir = join(tmpdir(), `agentcore-multi-agent-cred-${randomUUID()}`); await mkdir(testDir, { recursive: true }); + // Isolate secret-at-rest encryption from the developer's real OS keychain and + // ~/.agentcore: force the keyfile provider into this throwaway dir. cleanSpawnEnv + // inherits process.env, so spawned CLI processes pick these up too; the + // in-process readEnvFile() calls below read the same vars. + process.env.AGENTCORE_DISABLE_KEYCHAIN = '1'; + process.env.AGENTCORE_CONFIG_DIR = join(testDir, '.agentcore-keys'); + // Create project without agent const result = await runCLI(['create', '--name', projectName, '--no-agent'], testDir); if (result.exitCode !== 0) { @@ -32,6 +40,8 @@ describe('multi-agent credential behavior', () => { }); afterAll(async () => { + process.env.AGENTCORE_DISABLE_KEYCHAIN = prevEnv.kc; + process.env.AGENTCORE_CONFIG_DIR = prevEnv.cfg; await rm(testDir, { recursive: true, force: true }); }); diff --git a/src/cli/primitives/CredentialPrimitive.tsx b/src/cli/primitives/CredentialPrimitive.tsx index fb8824946..2296b10be 100644 --- a/src/cli/primitives/CredentialPrimitive.tsx +++ b/src/cli/primitives/CredentialPrimitive.tsx @@ -20,6 +20,7 @@ import { CredentialType, standardize } from '../telemetry/schemas/common-shapes. import { requireTTY } from '../tui/guards/tty'; import { BasePrimitive } from './BasePrimitive'; import { computeDefaultCredentialEnvVarName } from './credential-utils'; +import { warnOnLiteralSecretFlag } from './secret-flag-warning'; import type { AddResult, AddScreenComponent, RemovableResource } from './types'; import type { Command } from '@commander-js/extra-typings'; @@ -316,6 +317,7 @@ export class CredentialPrimitive extends BasePrimitive { + warnOnLiteralSecretFlag([cliOptions.apiKey, cliOptions.clientSecret], cliOptions.json, 'add credential'); const validation = validateAddCredentialOptions({ name: cliOptions.name, type: cliOptions.type as 'api-key' | 'oauth' | undefined, diff --git a/src/cli/primitives/PaymentConnectorPrimitive.ts b/src/cli/primitives/PaymentConnectorPrimitive.ts index 492fd607f..2937c3b14 100644 --- a/src/cli/primitives/PaymentConnectorPrimitive.ts +++ b/src/cli/primitives/PaymentConnectorPrimitive.ts @@ -2,7 +2,6 @@ import { findConfigRoot, removeEnvVars, setEnvVar, toError } from '../../lib'; import type { AgentCoreProjectSpec, PaymentProvider } from '../../schema'; import { PaymentConnectorNameSchema, PaymentConnectorSchema, PaymentProviderSchema } from '../../schema'; import type { RemoveResult } from '../commands/remove/types'; -import { ANSI } from '../constants'; import { getErrorMessage } from '../errors'; import type { RemovalPreview, SchemaChange } from '../operations/remove/types'; import { requireTTY } from '../tui/guards/tty'; @@ -15,6 +14,7 @@ import { validateAuthorizationPrivateKey, validateWalletSecret, } from './payment-validation'; +import { warnOnLiteralSecretFlag } from './secret-flag-warning'; import type { AddResult, AddScreenComponent, RemovableResource } from './types'; import type { Command } from '@commander-js/extra-typings'; @@ -441,19 +441,16 @@ export class PaymentConnectorPrimitive extends BasePrimitive v !== undefined); - if (usedLiteralSecretFlag && !cliOptions.json) { - process.stderr.write( - `${ANSI.yellow}Warning: passing secrets as CLI flags exposes them to shell history and the ` + - `process table. Prefer interactive mode (run \`agentcore add payment-connector\` with no ` + - `secret flags) for masked entry.${ANSI.reset}\n` - ); - } + warnOnLiteralSecretFlag( + [ + cliOptions.apiKeySecret, + cliOptions.walletSecret, + cliOptions.appSecret, + cliOptions.authorizationPrivateKey, + ], + cliOptions.json, + 'add payment-connector' + ); if (provider === 'StripePrivy') { // AWS docs ship the key with a `wallet-auth:` prefix — strip it transparently. diff --git a/src/cli/primitives/__tests__/secret-flag-warning.test.ts b/src/cli/primitives/__tests__/secret-flag-warning.test.ts new file mode 100644 index 000000000..8394347a3 --- /dev/null +++ b/src/cli/primitives/__tests__/secret-flag-warning.test.ts @@ -0,0 +1,35 @@ +import { warnOnLiteralSecretFlag } from '../secret-flag-warning'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +describe('warnOnLiteralSecretFlag', () => { + afterEach(() => vi.restoreAllMocks()); + + function capture(values: (string | undefined)[], json: boolean | undefined, command = 'add credential'): string { + let out = ''; + const spy = vi.spyOn(process.stderr, 'write').mockImplementation(chunk => { + out += String(chunk); + return true; + }); + warnOnLiteralSecretFlag(values, json, command); + spy.mockRestore(); + return out; + } + + it('warns when any secret value is provided', () => { + const out = capture(['sk-123', undefined], false); + expect(out).toMatch(/passing secrets as CLI flags/i); + expect(out).toContain('add credential'); + }); + + it('does not warn when no secret value is provided', () => { + expect(capture([undefined, undefined], false)).toBe(''); + }); + + it('is suppressed under --json', () => { + expect(capture(['sk-123'], true)).toBe(''); + }); + + it('uses the given command name in the suggestion', () => { + expect(capture(['s'], false, 'add payment-connector')).toContain('add payment-connector'); + }); +}); diff --git a/src/cli/primitives/secret-flag-warning.ts b/src/cli/primitives/secret-flag-warning.ts new file mode 100644 index 000000000..38a04ad67 --- /dev/null +++ b/src/cli/primitives/secret-flag-warning.ts @@ -0,0 +1,30 @@ +import { ANSI } from '../constants'; + +/** + * Warn (once, on stderr) when a secret was supplied as a literal CLI flag. + * + * Anything passed on the command line is captured by shell history and the + * process table the instant the command is typed — independent of how the value + * is later stored. Encrypting `.env.local` at rest does not undo that leak, so + * every command that accepts a secret as a flag surfaces this nudge toward the + * interactive (masked) flow. + * + * @param secretValues the resolved values of the command's secret flags; the + * warning fires if ANY is defined. + * @param json suppress under `--json` so machine-readable stdout stays clean. + * @param command the command to suggest running interactively (e.g. + * "add payment-connector", "add credential"). + */ +export function warnOnLiteralSecretFlag( + secretValues: (string | undefined)[], + json: boolean | undefined, + command: string +): void { + const usedLiteralSecretFlag = secretValues.some(v => v !== undefined); + if (!usedLiteralSecretFlag || json) return; + process.stderr.write( + `${ANSI.yellow}Warning: passing secrets as CLI flags exposes them to shell history and the ` + + `process table. Prefer interactive mode (run \`agentcore ${command}\` with no ` + + `secret flags) for masked entry.${ANSI.reset}\n` + ); +} diff --git a/src/lib/secrets/__tests__/key-provider.test.ts b/src/lib/secrets/__tests__/key-provider.test.ts index 33ea7689b..ee342bd11 100644 --- a/src/lib/secrets/__tests__/key-provider.test.ts +++ b/src/lib/secrets/__tests__/key-provider.test.ts @@ -1,5 +1,5 @@ import { resolveEncryptionKey } from '../key-provider'; -import { existsSync, mkdtempSync, rmSync, statSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -37,4 +37,21 @@ describe('resolveEncryptionKey (keyfile fallback)', () => { const b = await resolveEncryptionKey(); expect(Buffer.compare(a, b)).toBe(0); }); + + it('fails loud on a wrong-sized (corrupt) keyfile instead of overwriting it', async () => { + const keyfile = join(dir, 'secrets.key'); + writeFileSync(keyfile, Buffer.alloc(10, 0x41)); // 10 bytes, not 32 + const before = readFileSync(keyfile); + await expect(resolveEncryptionKey()).rejects.toThrow(/corrupt/i); + // The corrupt file must NOT have been clobbered with a fresh key. + expect(Buffer.compare(readFileSync(keyfile), before)).toBe(0); + }); + + it('reuses an existing valid keyfile rather than minting a new one', async () => { + const first = await resolveEncryptionKey(); + const { __resetKeyCacheForTests } = await import('../key-provider'); + __resetKeyCacheForTests(); + const second = await resolveEncryptionKey(); + expect(Buffer.compare(first, second)).toBe(0); + }); }); diff --git a/src/lib/secrets/key-provider.ts b/src/lib/secrets/key-provider.ts index 9a2f0a238..15cb5a152 100644 --- a/src/lib/secrets/key-provider.ts +++ b/src/lib/secrets/key-provider.ts @@ -35,6 +35,16 @@ async function tryKeychainKey(): Promise { } const key = randomBytes(KEY_BYTES); entry.setPassword(key.toString('base64')); + // Re-read after writing: if a concurrent process stored a different key + // between our getPassword() and setPassword(), the keychain now holds + // theirs — trust the persisted value, not our local one, so both processes + // converge on the same key. + try { + const persisted = entry.getPassword(); + if (persisted) return Buffer.from(persisted, 'base64'); + } catch { + // fall through to the freshly-generated key + } return key; } catch { return null; @@ -49,18 +59,40 @@ function keyfilePath(): string { return join(resolveConfigDir(), 'secrets.key'); } +/** A keyfile that exists but isn't KEY_BYTES long is corrupt — never overwrite it. */ +function assertKeyfileSize(key: Buffer, path: string): Buffer { + if (key.length === KEY_BYTES) return key; + throw new SecretEncryptionError( + `Encryption key file at ${path} is corrupt (expected ${KEY_BYTES} bytes, found ${key.length}). ` + + `Move it aside and re-add your credentials, or restore the original key file` + ); +} + function keyfileKey(): Buffer { const path = keyfilePath(); try { if (existsSync(path)) { - const key = readFileSync(path); - if (key.length === KEY_BYTES) return key; + // A wrong-sized existing file is corruption, NOT a cue to mint a new key: + // overwriting it would permanently orphan every secret sealed with the + // real key. Fail loud instead. + return assertKeyfileSize(readFileSync(path), path); } mkdirSync(resolveConfigDir(), { recursive: true }); const key = randomBytes(KEY_BYTES); - writeFileSync(path, key, { mode: 0o600 }); - return key; + try { + // `wx` fails if the file already exists, making first-time creation + // last-writer-safe: if a concurrent process created it between the + // existsSync check and here, re-read theirs instead of clobbering it. + writeFileSync(path, key, { mode: 0o600, flag: 'wx' }); + return key; + } catch (writeErr) { + if ((writeErr as NodeJS.ErrnoException).code === 'EEXIST') { + return assertKeyfileSize(readFileSync(path), path); + } + throw writeErr; + } } catch (err) { + if (err instanceof SecretEncryptionError) throw err; throw new SecretEncryptionError(`Could not create or read the machine encryption key at ${path}.`, { cause: err instanceof Error ? err : undefined, }); From 2b5c461e101dff67a086d29bb01d0a94f8b03f82 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 26 Jun 2026 20:49:04 +0000 Subject: [PATCH 15/15] test(e2e): assert payment connector secrets are encrypted at rest before deploy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The payment deploy e2e already exercises the encrypt-on-disk to deploy-decrypt round trip implicitly — if decryption broke, the deploy would fail. But it never asserted the on-disk format, so a regression that silently reverted to plaintext storage would still deploy green. Add an assertion that reads agentcore/.env.local directly and verifies (a) at least one enc:v1: envelope is present and (b) the raw CDP secret values never appear in cleartext. This pins the security invariant the encryption feature exists to guarantee, on the real deploy path, with zero added AWS cost (no extra deploy — it reads the file the prior step already wrote). --- e2e-tests/payment-strands-bedrock.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/e2e-tests/payment-strands-bedrock.test.ts b/e2e-tests/payment-strands-bedrock.test.ts index 765dc8ef7..ba1902750 100644 --- a/e2e-tests/payment-strands-bedrock.test.ts +++ b/e2e-tests/payment-strands-bedrock.test.ts @@ -125,6 +125,21 @@ describe.sequential('e2e: payments — create → add payment → deploy → sta expect(cred).toBeTruthy(); }); + it.skipIf(!canRun)('stores connector secrets encrypted at rest, never as plaintext', async () => { + // The deploy step below reads these same values back and decrypts them, so a + // passing deploy already proves the round trip works. This assertion additionally + // pins the security invariant the encryption feature exists to guarantee: the raw + // secret must never touch disk in cleartext. Without it, a regression that silently + // reverted to plaintext storage would still deploy green and go unnoticed. + const envLocal = await readFile(join(projectPath, 'agentcore', '.env.local'), 'utf-8'); + + expect(envLocal, '.env.local should contain at least one enc:v1: envelope').toContain('enc:v1:'); + + for (const raw of [process.env.CDP_API_KEY_SECRET!, process.env.CDP_WALLET_SECRET!]) { + expect(envLocal, 'raw secret value must not appear in cleartext on disk').not.toContain(raw); + } + }); + it.skipIf(!canRun)('has payment capability code in agent', async () => { const config = JSON.parse(await readFile(join(projectPath, 'agentcore', 'agentcore.json'), 'utf-8')); const runtimeName = config.runtimes?.[0]?.name;