Skip to content

Payments security hardening - #705

Closed
leewc wants to merge 2 commits into
aws:mainfrom
leewc:payments-security-hardening
Closed

leewc wants to merge 2 commits into
aws:mainfrom
leewc:payments-security-hardening

Conversation

@leewc

@leewc leewc commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available: N/A

Description of changes:

This PR adds two defense-in-depth hardening controls to the AgentCore Payments
integrations (Strands plugin and LangGraph middleware). Both are backward
compatible: no tool schema changes, and the content-inferred 402 default is
preserved so existing integrations behave exactly as before unless they opt in.

1. Make the configured user_id authoritative in agent tools

The Strands plugin and LangGraph tools exposed user_id as an LLM-controllable
tool argument whose value won over the configured identity. A prompt-injected
agent could supply a different user_id to attempt an identity override. The
managed service already scopes every read to the server-resolved identity, so
no tenant boundary was crossed — but the SDK should not let the model attempt
the override in the first place.

  • New resolve_authoritative_user_id() helper in
    payments/integrations/config.py. When an identity is configured, it is
    authoritative and a differing tool-supplied user_id is ignored and logged.
    When no identity is configured (bearer/JWT auth, where the server resolves the
    user from the token), the requested value passes through unchanged.
  • All 8 tool call sites (4 Strands + 4 LangGraph) route through the helper.
  • Tool signatures are unchanged (no schema break); docstrings updated.

2. Gate content-inferred 402 detection and add a pre-sign policy (LangGraph)

The LangGraph middleware's lenient fallback infers an x402 payment challenge
from raw tool content that lacks the trusted PAYMENT_REQUIRED: marker, so any
tool's output could cause a signed authorization to be minted for a recipient
and amount taken from untrusted content (CWE-863, improper authorization on an
untrusted channel). The Strands plugin is marker-only and is unaffected.

Two new, opt-in controls on AgentCorePaymentsPluginConfig:

  • allow_unmarked_402_detection: bool = True (default preserves current
    behavior). When False, both _fallback_detect_402 call sites are skipped,
    so a payment is derived only from the integration's own payment-bearing tools
    via the trusted marker.
  • pre_sign_policy: Optional[Callable[[Any], bool]] = None. A callback invoked
    at the single signing chokepoint (_generate_payment_header) on every
    detection path (marked, unmarked, and error-retry) before any value moves. It
    receives the payment-required request (carrying payTo and amount) and
    returns True to allow or False to block. It is fail-closed: a falsy return
    or a raised exception blocks signing. Integrators can use it to enforce a
    recipient/amount allowlist regardless of the detection default.

Tests

  • Unit tests for the helper, config validation, flag-gated detection, and the
    pre-sign policy (allow/block/fail-closed).
  • An end-to-end exploit-reproduction suite that drives the full
    wrap_tool_call path with an attacker-controlled, unmarked payload naming an
    attacker-chosen payTo/amount. It asserts (a) the permissive default would
    sign to the attacker (pinning the attack surface) and (b) no authorization is
    minted once either control is active, while a legitimate allowlisted recipient
    still signs.
  • Prior tests that asserted the old user_id override-wins precedence are
    updated to assert the hardened behavior.

Full payments test suite: 940 passed, 9 skipped. ruff check clean.


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

leewc added 2 commits October 19, 2026 20:55
The Strands plugin and LangGraph tools exposed user_id as an LLM-controllable
tool argument whose value won over the configured identity. A prompt-injected
agent could supply a different user_id to attempt an identity override. The
managed service already scopes every read to the server-resolved identity, so
no tenant boundary was crossed, but the SDK should not let the model attempt
the override in the first place.

Add resolve_authoritative_user_id() in config.py and route all 8 tool sites
(4 Strands + 4 LangGraph) through it: the configured identity is authoritative,
a differing tool-supplied value is ignored and logged. Tool signatures are
unchanged (no schema break). Adds regression tests and updates the prior
precedence tests to assert the hardened behavior.
…gGraph)

The LangGraph middleware's lenient fallback infers an x402 payment challenge
from raw tool content that lacks the trusted PAYMENT_REQUIRED: marker, so any
tool's output could mint a signed authorization to an actor-chosen recipient
and amount (CWE-863: improper authorization on an untrusted channel). The
Strands plugin is marker-only and unaffected.

Add two defense-in-depth controls on AgentCorePaymentsPluginConfig:
- allow_unmarked_402_detection (default True, preserves current behavior):
  when False, both _fallback_detect_402 call sites are skipped so only the
  trusted marker path derives a payment.
- pre_sign_policy callback at the single signing chokepoint
  (_generate_payment_header), invoked on every detection path before any
  value moves; fail-closed on a falsy return or a raise.

Adds regression tests plus an end-to-end exploit-reproduction suite that
drives the full wrap_tool_call path with an attacker-controlled unmarked
payload and asserts no authorization is minted once either control is active.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Oct 20, 2026
@leewc

leewc commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

RequestError [HttpError]: Resource not accessible by integration
at /home/runner/work/_actions/actions/github-script/v8/dist/index.js:9537:21
at process.processTicksAndRejections (node:internal/process/task_queues:104:5)
at async eval (eval at callAsyncFunction (/home/runner/work/_actions/actions/github-script/v8/dist/index.js:36187:16), :27:3)
at async main (/home/runner/work/_actions/actions/github-script/v8/dist/index.js:36285:20) {
status: 403,

Looks like I lack perms.. @aidandaly24 I wonder if you can grant them to me! Thank you.

@codecov-commenter

codecov-commenter commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@5d85607). Learn more about missing BASE report.

Files with missing lines Patch % Lines
..._agentcore/payments/integrations/strands/plugin.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #705   +/-   ##
=======================================
  Coverage        ?   89.62%           
=======================================
  Files           ?      124           
  Lines           ?    10921           
  Branches        ?     1713           
=======================================
  Hits            ?     9788           
  Misses          ?      744           
  Partials        ?      389           
Flag Coverage Δ
unittests 89.62% <97.14%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leewc leewc closed this Oct 21, 2026
@leewc

leewc commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

#706 -- retrying with permissions

This branch had an error being deployed

1 failed deployment
auto-approve — 90f16cd1 Deployed Oct 21, 2026 by leewc via Test (payments) #1642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants