fix(payments): harden secret handling and auto-payment defaults

[aidandaly24]

Description

Hardens the Agent Payments feature against three secret-exposure / secure-default issues found in a security review. Each fix is small and follows an existing pattern in the codebase.

1. Payment API error bodies could leak provider secrets (src/cli/aws/agentcore-payments.ts)
On a non-2xx payment API response the raw body was interpolated into the thrown Error after a fixed-key denylist regex. That regex only matched a known set of top-level field names with exact casing, so nested objects (the create/update bodies nest secrets under coinbaseCdpConfiguration / stripePrivyConfiguration), snake_case server fields, or a secret embedded in a message string survived redaction. Inverted to an allowlist: the new buildPaymentApiError(status, body, opts?) surfaces only the HTTP status and the parsed error code/__type and never echoes the body. Applied to both signedRequest and signedDataPlaneRequest. error.code is preserved through the wrapping create/update/delete/get functions so 404 and service-quota detection keep working.

2. Auto-payment is enabled by default with no prominent warning (PaymentManagerPrimitive, deploy/actions.ts)
A payment-enabled agent auto-settles 402 Payment Required responses up to the per-session spend limit with no human in the loop. Auto-payment stays enabled by default (it is the core of the feature — transparent 402 settlement), but it is no longer enabled silently: a prominent stderr warning now fires at add time (naming the spend limit and the --auto-payment false opt-out) and a per-manager warning is surfaced at deploy time for any manager with auto-payment active.

3. Provider secrets had to be passed as CLI flags (PaymentConnectorPrimitive, new credential-file.ts)
add payment-connector only accepted secrets (--api-key-secret, --wallet-secret, --app-secret, --authorization-private-key) as literal flags, which leak into shell history and the process table. Added a single --credentials-file <path> option that reads the provider secrets from a JSON file, or from stdin when the path is -, so nothing sensitive reaches argv. The literal secret flags still work but now print a warning recommending --credentials-file, and a field set by both a flag and the file errors out instead of silently picking one. docs/payments.md documents the new flow.

Related Issue

Closes #

Documentation PR

N/A — docs/payments.md is updated in this PR (documents --credentials-file).

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Other (please describe):

Testing

How have you tested the change?

• 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), test:unit (5411 passed), test:integ (274 passed) all green. No src/assets/ files changed, so no snapshot regeneration was needed.
• New/updated unit tests: buildPaymentApiError + getPaymentManager 404/quota (agentcore-payments.test.ts), readCredentialsFile (credential-file.test.ts), resolvePaymentSecrets (PaymentConnectorPrimitive.test.ts), add-time warning (PaymentManagerPrimitive.test.ts).
• Verified end-to-end against the built CLI in a scratch project: the add-time warning fires on default and is silent with --auto-payment false; --credentials-file (file and stdin) writes .env.local with no leak warning; a literal secret flag prints the leak warning; a flag+file conflict fails closed with no partial write; and buildPaymentApiError never leaks secret-bearing (nested / snake_case / unparseable) bodies while preserving status + code.

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.

[github-actions]

Package Tarball

aws-agentcore-0.19.0.tgz

How to install

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

[agentcore-devx-automation]

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

[agentcore-cli-automation]

test

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	36.94%	13526 / 36609
🔵	Statements	36.24%	14386 / 39695
🔵	Functions	31.32%	2297 / 7332
🔵	Branches	30.69%	8932 / 29099

Generated in workflow #3679 for commit fc627a4 by the Vitest Coverage Report Action

[agentcore-cli-automation]

test2

[agentcore-cli-automation]

test3

[agentcore-cli-automation]

Thanks for hardening the payment flows — the allowlist-based error builder and the credentials-file pattern both look like solid improvements over what was there. Two issues that I think need to be addressed before this can merge:

1. The new add-time auto-payment warning is written from PaymentManagerPrimitive.add(), which is also called from the Ink TUI flow. Writing ANSI text directly to process.stderr while Ink is mid-render will corrupt the rendered frame.
2. readCredentialsFile reads stdin via '/dev/stdin', which doesn't exist on Windows, so --credentials-file - will silently break for Windows users of the new secure-stdin path.

Details inline. Also one optional follow-up suggestion on the error builder.

(Please disregard the three earlier test / test2 / test3 review entries on this PR — those were artifacts from validating which inline-line numbers GitHub would accept; I wasn't able to delete the parent review records, only the inline comments under them.)

[agentcore-cli-automation]

This process.stderr.write is invoked from inside add(), which means it fires for every caller of the primitive — including the Ink TUI flow in src/cli/tui/screens/payment/useCreatePayment.ts (paymentManagerPrimitive.add(config) at line 24). Writing ANSI-coded text directly to stderr while Ink is mid-render will leak the warning into the TUI output and corrupt the rendered frame.

Compare with the equivalent literal-secret-flag warning you added in PaymentConnectorPrimitive.ts (line 440 in the new file) — that one is correctly placed inside the commander .action() callback in registerCommands, so it only fires on the non-interactive CLI path. The auto-payment warning needs the same separation.

A few options:

1. Move the stderr write into the CLI action handler (mirror the connector primitive's pattern). add.payment-manager is wired in registerCommands; do the warning there after a successful add() call.
2. Return the warning via the AddResult (e.g., add warnings?: string[] to the success shape) and let each caller decide how to render it — the CLI prints to stderr, the TUI displays it in a confirm/result panel (the TUI already has a warningFields slot in AddPaymentFlow.tsx).
3. Gate on !process.stdout.isTTY before writing — but this is fragile because the TUI runs on a TTY too, so it would suppress the warning in the very environment where the user most needs to see it untangled from the UI.

Option 2 is probably the most robust since the warning then surfaces in both modes through the right channel.

The per-deploy warning in deploy/actions.ts is fine as-is — that path already uses postDeployWarnings, which is the proper channel.

[aidandaly24]

Fixed in 22a27ff (Option 2). add() no longer writes to stderr — it returns the warning text as autoPaymentWarning on the AddResult. The CLI action prints it to stderr on the non-interactive path (kept out of --json stdout), and the TUI surfaces it as a warning field on the confirm screen via flow.managerConfig.autoPayment, so it shows before the user commits. Verified the useCreatePayment → add() path no longer emits to stderr; test now asserts the returned field instead of spying on stderr.

[agentcore-cli-automation]

readFileSync('/dev/stdin', 'utf-8') is POSIX-only; on Windows the path /dev/stdin doesn't exist and this will throw ENOENT when a user pipes JSON via --credentials-file -. The CLI is otherwise cross-platform (e.g., src/cli/operations/dev/utils.ts branches on process.platform === 'win32', and package.json only declares node >=20), so this would silently break Windows users of the new secure-stdin path that docs/payments.md now recommends.

Node supports reading stdin by file descriptor on every platform — swap to readFileSync(0, 'utf-8'):

readStdin: () => string = () => readFileSync(0, 'utf-8')

This works on Linux, macOS, and Windows. Worth also adding a quick test that exercises the stdin path with the default readStdin (the current test only covers the injected fake), or at minimum an integration smoke test, since the documented --credentials-file - flow is the recommended secure path.

[aidandaly24]

Fixed in f4cfc3e — default is now readFileSync(0, "utf-8"). Added a test that pipes real JSON into a child process invoking readCredentialsFile("-") with the default reader, so the fd-0 path is exercised end-to-end rather than only via the injected stub (a vi.spyOn(fs, "readFileSync") approach is blocked by ESM module-namespace immutability, hence the subprocess pipe).

[agentcore-cli-automation]

Optional / non-blocking heads-up: when a response body contains a useful message (e.g. "validation failed: paymentManagerId must be ≤ 64 chars") alongside a code, the new error message collapses to just Payment API error (400): ValidationException — operators lose the human-readable hint they used to get from the server.

The nested-config / snake_case body-leak risk you're addressing is real, but a server-side message field shouldn't typically echo request fields the way the coinbaseCdpConfiguration / stripePrivyConfiguration blobs do. If you'd like to preserve operator-facing detail without re-introducing the leak risk, one safe approach is to surface parsed.message only when (a) it's a string, (b) none of the known secret field names appear anywhere in a recursive walk of the parsed object, and (c) the message itself doesn't contain any secret-shaped substrings (e.g. via a structural check, not a fixed-key regex).

Happy to defer this to a follow-up if you'd rather keep this PR strictly focused on the leak fix — just flagging that the new builder is more aggressive than strictly necessary in the common case.

[aidandaly24]

Done in fc627a4 rather than deferred. buildPaymentApiError now surfaces parsed.message alongside the code, but only when it is provably safe: a recursive walk of the parsed body finds no provider-secret field name at any depth, AND the message text itself names no secret field (keys normalized to fold camelCase/snake_case). Otherwise it still collapses to status + code. Added tests for: safe message surfaced, message surfaced without a code, suppression when a secret field appears anywhere in the body, suppression when the message itself names a secret field, and overlong-message truncation.

[agentcore-devx-automation]

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