feat(secrets): encrypt sensitive .env.local values at rest#1621
Merged
Conversation
…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
github-actions
Bot
added
the
agentcore-harness-reviewing
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
Package TarballHow to installgh release download pr-1621-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.21.1.tgz |
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
agentcore-cli-automation
suggested changes
Jun 23, 2026
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
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.
github-actions
Bot
removed
the
agentcore-harness-reviewing
Contributor
Coverage Report
|
…and error text
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.
…ncrypted An API-key credential stores its secret in the bare AGENTCORE_CREDENTIAL_<NAME> env var. If <NAME> 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.
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
added
claude-security-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
aidandaly24
had a problem deploying
to
e2e-testing
GitHub Actions
Failure
nborges-aws
approved these changes
Jun 26, 2026
Keeps the branch current and clears the cancelled e2e PR check: the e2e-tests.yml change-detector diffs against the main tip, and main's drift (incl. e2e-tests/harness-e2e-helper.ts) was tripping the run-the-entire-suite path, overrunning the 30-min PR-check budget. Up to date with main, the diff reflects only this branch's changes (which touch one e2e test file), so the check runs the default set plus that file.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Encrypts sensitive credential values in
agentcore/.env.localat 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
src/lib/secrets/module: a sensitive-key registry (isSensitiveKey), an AES-256-GCM cipher (enc:v1:envelope), and a key-provider chain..env.localreader/writer already uses (readEnvFile/writeEnvFileinsrc/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.@napi-rs/keyring), falling back to a0600~/.agentcore/secrets.keyon machines/CI without a keychain. A copiedagentcore/folder therefore contains only ciphertext.enc:v1:...; reference IDs (_API_KEY_ID,_APP_ID,_CLIENT_ID,_AUTHORIZATION_ID) stay readable..env.localfiles are read as-is and re-encrypted on the next write. No migration command, no breakage.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.mdis updated in this PR (Secret encryption at rest + rotation guidance).Type of Change
Testing
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsNotes:
typecheck(0 errors),lint(0 errors),build(succeeds —@napi-rs/keyringis marked esbuild-external so its native binary isn't bundled),test:unit(5434 passed),test:integ(291 passed). Nosrc/assets/changes.0600; sensitive-key registry; env.ts encrypt-on-write, decrypt-on-read, legacy self-migration, no-double-encrypt, removeEnvVars-keeps-encrypted.enc:v1:on disk AND round-trips back viareadEnvFile(OAuth client secret; model-provider keys).add payment-connectorwritesenc:v1:for the secret (reference ID stays plaintext) and prints the leak warning;agentcore validatereads the secret back and passes — confirming the cross-process encrypt→decrypt round-trip deploy depends on.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.