feat(secrets): encrypt sensitive .env.local values at rest by aidandaly24 · Pull Request #1621 · aws/agentcore-cli

aidandaly24 · 2026-06-23T17:56:33Z

Description

Encrypts sensitive credential values in agentcore/.env.local at rest, closing a security finding that provider secrets (payment provider keys, OAuth client secrets, model-provider API keys) were persisted in plaintext on disk — so copying, syncing, or backing up a project directory exposed live credentials.

How it works

A new src/lib/secrets/ module: a sensitive-key registry (isSensitiveKey), an AES-256-GCM cipher (enc:v1: envelope), and a key-provider chain.
Encryption is transparent at the single chokepoint every .env.local reader/writer already uses (readEnvFile/writeEnvFile in src/lib/utils/env.ts): sensitive values are encrypted on write and decrypted on read, so deploy/dev/validate/fetch-access are unaffected and need no changes.
The encryption key lives outside the project directory — the OS keychain (optional @napi-rs/keyring), falling back to a 0600 ~/.agentcore/secrets.key on machines/CI without a keychain. A copied agentcore/ folder therefore contains only ciphertext.
The file stays dotenv-parseable: only sensitive values become enc:v1:...; reference IDs (_API_KEY_ID, _APP_ID, _CLIENT_ID, _AUTHORIZATION_ID) stay readable.
Self-migrating: existing plaintext .env.local files are read as-is and re-encrypted on the next write. No migration command, no breakage.
Covers the whole CLI uniformly (payment secrets, OAuth client secrets, model-provider keys), not just payments.
Also adds a one-time stderr warning when payment connector secrets are passed as literal CLI flags (they leak to shell history / process table), recommending interactive entry.

Decryption failure throws a SecretDecryptionError — ciphertext is never silently surfaced as if it were the real secret, and never sent to an AWS API.

Related Issue

Closes the SIM-T AppSec finding for plaintext-at-rest payment/credential secrets (tracked internally; not a GitHub issue).

Closes #

Documentation PR

N/A — docs/payments.md is updated in this PR (Secret encryption at rest + rotation guidance).

Type of Change

Testing

I ran npm run test:unit and npm run test:integ
I ran npm run typecheck
I ran npm run lint
If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Notes:

typecheck (0 errors), lint (0 errors), build (succeeds — @napi-rs/keyring is marked esbuild-external so its native binary isn't bundled), test:unit (5434 passed), test:integ (291 passed). No src/assets/ changes.
New unit tests: cipher round-trip / tamper-detection / wrong-key; keyfile creation + 0600; sensitive-key registry; env.ts encrypt-on-write, decrypt-on-read, legacy self-migration, no-double-encrypt, removeEnvVars-keeps-encrypted.
Two pre-existing integ/unit tests that asserted plaintext secrets on disk were updated to assert the stronger property: the secret is enc:v1: on disk AND round-trips back via readEnvFile (OAuth client secret; model-provider keys).
Behavioral E2E against the built+installed CLI: add payment-connector writes enc:v1: for the secret (reference ID stays plaintext) and prints the leak warning; agentcore validate reads the secret back and passes — confirming the cross-process encrypt→decrypt round-trip deploy depends on.

Checklist

I have read the CONTRIBUTING document
I have added any necessary tests that prove my fix is effective or my feature works
I have updated the documentation accordingly
I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
My changes generate no new warnings
Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…lags

@ts-expect-error

…yption 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.

… its native binary

…nv.local

…-agent-auth

github-actions · 2026-06-23T17:57:34Z

Package Tarball

aws-agentcore-0.20.2.tgz

How to install

gh release download pr-1621-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.20.2.tgz

agentcore-devx-automation · 2026-06-23T17:59:33Z

Claude Security Review: no high-confidence findings. (run)

agentcore-cli-automation

Nice scoped, transparent design — single chokepoint, self-migrating, opt-out keychain. A few issues to address before merge, mostly around the keyfile-fallback path and integ-test isolation. Inline comments below.

agentcore-cli-automation · 2026-06-23T18:01:12Z

+    mkdirSync(resolveConfigDir(), { recursive: true });
+    const key = randomBytes(KEY_BYTES);
+    writeFileSync(path, key, { mode: 0o600 });
+    return key;


Silent data-loss on a wrong-sized keyfile. If secrets.key exists but isn't 32 bytes (e.g., truncated by a bad backup/sync, partially-written from a crash, or hand-edited), keyfileKey() silently falls through to the writeFileSync below and overwrites it with a fresh random key. Every encrypted secret in every project on this machine becomes permanently undecryptable, with no warning — the next readEnvFile will surface as SecretDecryptionError: Re-add the credential, but the user has no signal that the key (not the ciphertext) is what got clobbered.

Options:

Throw SecretEncryptionError when the keyfile exists with the wrong size (treat as corruption — fail loudly, let the user investigate / move it aside).

Only write the keyfile when it does not exist; if it exists with the wrong size, refuse and require explicit deletion.

Either way, the create-new-key branch should be reached only when the file genuinely doesn't exist.

agentcore-cli-automation · 2026-06-23T18:01:12Z

+    }
+    mkdirSync(resolveConfigDir(), { recursive: true });
+    const key = randomBytes(KEY_BYTES);
+    writeFileSync(path, key, { mode: 0o600 });


Race on first-time keyfile creation. Two CLI processes calling resolveEncryptionKey() concurrently (e.g., a script doing agentcore add … & in parallel, or two terminals) can both see existsSync(path) === false, both randomBytes(32), and both writeFileSync — the second one wins and silently invalidates whatever the first one just encrypted.

Use writeFileSync(path, key, { mode: 0o600, flag: 'wx' }) and, on EEXIST, re-read the file (and validate its length). That makes first-time creation last-writer-safe.

agentcore-cli-automation · 2026-06-23T18:01:12Z

+      // no stored password yet
+    }
+    const key = randomBytes(KEY_BYTES);
+    entry.setPassword(key.toString('base64'));


Same race exists in the keychain path: between entry.getPassword() returning null and entry.setPassword(...), a concurrent process can win and store a different key. After the setPassword, you should re-read with getPassword() and use whichever value is now persisted, rather than trusting the local key variable. (Less likely than the keyfile race, but the failure mode is identical: encrypted-with-one-key, decrypted-with-another.)

agentcore-cli-automation · 2026-06-23T18:01:12Z

@@ -47,6 +48,10 @@ describe('multi-agent credential behavior', () => {
    }


Integ tests pollute the developer's real keychain / ~/.agentcore. runCLI inherits process.env, and these tests don't set AGENTCORE_DISABLE_KEYCHAIN=1 or AGENTCORE_CONFIG_DIR. On a dev/CI machine with a keychain (macOS, Linux with libsecret), the test creates/uses the aws-agentcore / env-local-secret-key entry in the user's keychain; without a keychain, it writes to ~/.agentcore/secrets.key. That's a real side effect from npm run test:integ and (more concerning) means once the developer's keychain has a key from a previous run, these tests will validate using that key rather than a fresh one — cross-run state leakage.

Same issue in integ-tests/add-agent-auth.test.ts (the test added on line 5/120-129 of that file).

Fix: set AGENTCORE_DISABLE_KEYCHAIN=1 and AGENTCORE_CONFIG_DIR=<tmpdir> in the spawned env (via runCLI(..., { env: { ... } }) and createTestProject/beforeAll).

agentcore-cli-automation · 2026-06-23T18:01:12Z

+                  `${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`
+                );


The CLI-flag leak warning only fires for add payment-connector. The same hazard applies to add agent --api-key … and add agent --client-secret … (both also persist secrets to .env.local and both currently accept secret values via positional flags in non-interactive mode). Either:

Extract this warning into a shared helper (e.g., warnOnLiteralSecretFlag(flagNames: string[]) in cli/primitives/) and call it from AgentPrimitive / CredentialPrimitive / auth-utils wherever a secret flag is consumed; or

Document why the warning is payment-only (e.g., "agent api-keys are typed differently" — but I don't see a reason).

Given the PR scope says "Covers the whole CLI uniformly … not just payments," option 1 fits the stated goal.

github-actions · 2026-06-23T18:01:54Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	37.16%	13626 / 36665
🔵	Statements	36.44%	14496 / 39774
🔵	Functions	31.87%	2346 / 7361
🔵	Branches	30.87%	8965 / 29033

Generated in workflow #3775 for commit 2f53e08 by the Vitest Coverage Report Action

aidandaly24 added 11 commits June 23, 2026 16:58

feat(secrets): add sensitive-key registry for at-rest encryption

2a8babe

feat(errors): add SecretEncryptionError and SecretDecryptionError

52a95e6

feat(secrets): add AES-256-GCM cipher for at-rest secret envelopes

f6cb1d2

feat(secrets): add key-provider chain (OS keychain -> keyfile fallback)

dd410f0

refactor(secrets): extract resolveConfigDir helper in key-provider

0df7129

feat(secrets): encrypt sensitive .env.local values at rest in env.ts

49f7938

fix(payment-connector): warn when secrets are passed as literal CLI f…

9a0426e

…lags

fix(build): mark @napi-rs/keyring external so esbuild does not bundle…

e9b66f3

… its native binary

test(secrets): assert model-provider keys are encrypted at rest in .e…

5d42e0e

…nv.local

test(secrets): assert OAuth client secret is encrypted at rest in add…

2f53e08

…-agent-auth

aidandaly24 requested a review from a team June 23, 2026 17:56

github-actions Bot added the size/l PR size: L label Jun 23, 2026

aidandaly24 had a problem deploying to e2e-testing June 23, 2026 17:56 — with GitHub Actions Error

github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 23, 2026

agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026

agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026

agentcore-cli-automation suggested changes Jun 23, 2026

View reviewed changes

github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(secrets): encrypt sensitive .env.local values at rest#1621

feat(secrets): encrypt sensitive .env.local values at rest#1621
aidandaly24 wants to merge 11 commits into
mainfrom
feat/secret-at-rest-encryption

aidandaly24 commented Jun 23, 2026

Uh oh!

github-actions Bot commented Jun 23, 2026

Uh oh!

agentcore-devx-automation Bot commented Jun 23, 2026

Uh oh!

agentcore-cli-automation left a comment

Uh oh!

agentcore-cli-automation Jun 23, 2026

Uh oh!

agentcore-cli-automation Jun 23, 2026

Uh oh!

agentcore-cli-automation Jun 23, 2026

Uh oh!

agentcore-cli-automation Jun 23, 2026

Uh oh!

agentcore-cli-automation Jun 23, 2026

Uh oh!

github-actions Bot commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -47,6 +48,10 @@ describe('multi-agent credential behavior', () => {
		}

Conversation

aidandaly24 commented Jun 23, 2026

Description

Related Issue

Documentation PR

Type of Change

Testing

Checklist

Uh oh!

github-actions Bot commented Jun 23, 2026

Package Tarball

How to install

Uh oh!

agentcore-devx-automation Bot commented Jun 23, 2026

Uh oh!

agentcore-cli-automation left a comment

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 23, 2026

Coverage Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants