fix(payment-manager): warn when auto-payment is enabled by default

[aidandaly24]

Description

Surfaces a prominent warning when a payment manager is configured with auto-payment enabled (the default), so the behavior is never enabled silently.

What auto-payment does: the AgentCore payments plugin registers an after_tool_call hook on the agent. When auto_payment is enabled and a tool returns a 402 Payment Required, the plugin signs and submits an x402 payment and retries the tool call automatically, with no agent/human decision in between. When disabled, the plugin skips this and the 402 propagates back to the agent as a normal tool result. (Settlement still requires a funded instrument + session in the invocation context, so this is the "settle without asking" switch rather than a standalone money-mover.)

What this PR adds (warnings only — no behavior change):

• Add-time warning — agentcore add payment-manager (when auto-payment is on, i.e. the default) prints a stderr warning naming the per-session spend limit and the --auto-payment false opt-out. The warning is returned from add() as a result field and rendered by each caller (CLI prints to stderr, TUI shows it as a confirm-screen warning field) so it never corrupts the Ink render or the --json output.
• Deploy-time warning — agentcore deploy adds a per-manager entry to postDeployWarnings for any manager with auto-payment active.

The default is intentionally left enabled (transparent 402 settlement is the core of the feature); this PR makes the posture visible rather than changing it.

Note / open question for the Payments team: the underlying AppSec finding frames default-on auto-payment as an unattended-spend risk, but auto-payment only results in real settlement when a funded instrument and session are already in the invocation context. The practical severity in the CLI's own invoke path (where the caller passes --payment-instrument-id / --payment-session-id explicitly) is debatable. This PR takes the conservative "make it visible" route pending that discussion; flipping the default is a separate decision.

Scope: this branch was reduced to the auto-payment warning only. The error-body redaction work (a separate finding) and the --credentials-file secret-intake work have been removed from this PR and will be handled separately.

Related Issue

Internal AppSec finding (auto-payment enabled by default). Not a GitHub issue.

Closes #

Documentation PR

N/A — no doc changes in this PR.

Type of Change

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

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; PaymentManagerPrimitive unit tests pass (including the add-time-warning-emitted and suppressed-on---auto-payment false cases). No src/assets/ changes.
• The warning is emitted via the AddResult (autoPaymentWarning) and rendered by the caller, so it stays out of --json stdout and does not write to stderr during an Ink TUI render.

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.20.2.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.20.2.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	37.15%	13595 / 36588
🔵	Statements	36.42%	14455 / 39679
🔵	Functions	31.8%	2333 / 7336
🔵	Branches	31.11%	9009 / 28956

Generated in workflow #3813 for commit 1c3e6b8 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)

[agentcore-devx-automation]

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