fix(payment-manager): warn when auto-payment is enabled by default#1556
Merged
Conversation
aidandaly24
had a problem deploying
to
e2e-testing
GitHub Actions
Failure
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
github-actions
Bot
added
the
agentcore-harness-reviewing
Contributor
Package TarballHow to installgh release download pr-1556-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
Contributor
Coverage Report
|
agentcore-cli-automation
previously requested changes
Jun 17, 2026
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
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:
- 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 toprocess.stderrwhile Ink is mid-render will corrupt the rendered frame. readCredentialsFilereads 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.)
github-actions
Bot
added
size/l
aidandaly24
had a problem deploying
to
e2e-testing
GitHub Actions
Failure
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
…abled Auto-payment is enabled by default so an agent can transparently settle 402 Payment Required responses, but that means it can move money with no human in the loop. The add flow now emits a prominent stderr warning naming the per-session spend limit and the --auto-payment false opt-out, so the unattended-spend posture is never enabled silently.
… enabled Adds a per-manager post-deploy warning naming the per-session spend limit whenever auto-payment is active, so the unattended-spend posture is surfaced at deploy time as well as at add time. Set --auto-payment false on the manager to require manual approval.
…to stderr add() is shared by the CLI and the Ink TUI flow (useCreatePayment), so writing the auto-payment warning to process.stderr corrupted the TUI's rendered frame. Return the warning text on the AddResult instead and let each caller render it through its own channel: the CLI action prints it to stderr, and the TUI confirm screen surfaces it as a warning field.
aidandaly24
force-pushed
the
fix/payments-security
branch
from
fc627a4 to
1c3e6b8
Compare
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
aidandaly24
changed the title
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
…nd deploy exit code Two bug-bash findings on the auto-payment warning: - The warning was serialized into `add payment-manager --json` stdout via the autoPaymentWarning result field. Strip it on the --json branch so stdout stays a clean machine-readable result; the notice still renders on the human path. - The deploy-time auto-payment notice was pushed into postDeployWarnings, which the deploy command maps to `process.exit(2)` — so every successful payment deploy exited non-zero (a CI failure signal). Route it through the `notes` channel (exit 0) instead, which is the correct place for informational notices.
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
nborges-aws
previously approved these changes
Jun 26, 2026
nborges-aws
reviewed
Jun 26, 2026
…isplay The confirm-screen 'Default Spend Limit' field and the auto-payment warning both interpolate flow.managerConfig.defaultSpendLimit directly. The field is typed string and defaulted to '10.00', so '$undefined/session' is not reachable today — but every other touch point of this field (the wizard setter, the manager-screen editor) already coerces to the default, leaving these two display sites as the only undefended ones. Fall back to the shared DEFAULT_SPEND_LIMIT constant at both interpolations so the rendered warning stays correct if a future refactor ever loosens the type.
Resolves the e2e PR-check cancellation: the change-detector in e2e-tests.yml diffs against the main tip, and main's drift (incl. changes to e2e-tests/harness-e2e-helper.ts) was tripping the run-the-entire-suite path, which overran the 30-min PR-check budget. Up to date with main, the diff reflects only this branch's changes (which touch no e2e files), so the check runs just the default e2e set.
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
aidandaly24
dismissed
agentcore-cli-automation’s stale review
Dismissing: this CHANGES_REQUESTED was submitted against commit 595c0c1 (the original broad version of this PR). The PR has since been reduced to the auto-payment warning only — all three flagged code paths (credential-file.ts stdin, agentcore-payments.ts error redaction, PaymentManagerPrimitive stderr) have either been removed from the diff or addressed (the stderr write was replaced by the returned-warning pattern the review recommended). All review threads are resolved with explanations.
nborges-aws
approved these changes
Jun 29, 2026
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
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_callhook on the agent. Whenauto_paymentis enabled and a tool returns a402 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 the402propagates 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):
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 falseopt-out. The warning is returned fromadd()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--jsonoutput.agentcore deployadds a per-manager entry topostDeployWarningsfor 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.
Scope: this branch was reduced to the auto-payment warning only. The error-body redaction work (a separate finding) and the
--credentials-filesecret-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
Testing
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsNotes:
typecheck0 errors;PaymentManagerPrimitiveunit tests pass (including the add-time-warning-emitted and suppressed-on---auto-payment falsecases). Nosrc/assets/changes.AddResult(autoPaymentWarning) and rendered by the caller, so it stays out of--jsonstdout and does not write to stderr during an Ink TUI render.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.