Skip to content

feat: add recipient_allowlist declarative policy rule - #225

Open
br-to wants to merge 7 commits into
open-wallet-standard:mainfrom
br-to:feat/sign-allowlist-policy-rule
Open

feat: add recipient_allowlist declarative policy rule#225
br-to wants to merge 7 commits into
open-wallet-standard:mainfrom
br-to:feat/sign-allowlist-policy-rule

Conversation

@br-to

@br-to br-to commented May 17, 2026

Copy link
Copy Markdown

What

Add RecipientAllowlist variant to the PolicyRule serde enum — a declarative rule that restricts transaction recipients to an address allowlist (case-insensitive). Denies transactions with no to field (e.g. contract creation).

Why

Recipient allowlists were previously only possible via custom executable policies. Adding a built-in declarative rule lets agents be scoped to specific destination addresses without subprocess overhead.

Testing

  • cargo test --workspace passes (265 tests)
  • cargo clippy --workspace -- -D warnings is clean
  • npm test passes (if Node bindings changed) — N/A, no binding changes
  • Tested manually with ows CLI

Notes

  • New tests: address match, non-match, case-insensitive, no to field, empty list, serde roundtrip
  • Docs updated: added recipient_allowlist section to docs/03-policy-engine.md, removed recipient allowlists from the "not implemented" note

Note

Medium Risk
Changes the agent signing policy path before keys are decrypted; mis-parsed or legacy txs are denied when the rule is attached, and combined policies can block message/hash signing unintentionally.

Overview
Adds a built-in recipient_allowlist declarative policy so API keys can restrict transaction signing to specific EVM recipient addresses without a custom executable.

Policy evaluation compares PolicyContext.transaction.to to the allowlist (case-insensitive) and denies when to is missing, the recipient is not listed, or the list is empty. Agent transaction signing now populates to by parsing unsigned EIP-1559 / EIP-2930 bytes via new extract_to_address RLP decoding; legacy, malformed, contract-creation, and non-EVM cases leave to unset and the rule fails closed. Message, hash, and typed-data paths still pass to: None, so keys with this rule cannot use those operations unless policies are split later.

Persistence: saving/loading policies validates EVM address shape and lowercases addresses on recipient_allowlist and allowed_typed_data_contracts. CLI ows policy show prints the new rule type. Docs describe behavior and remove recipient allowlists from the “not implemented” list.

Reviewed by Cursor Bugbot for commit e243135. Bugbot is set up for automated code reviews on this repo. Configure here.

@br-to
br-to requested a review from njdawn as a code owner May 17, 2026 15:53
@vercel

vercel Bot commented May 17, 2026

Copy link
Copy Markdown

@br-to is attempting to deploy a commit to the MoonPay Team on Vercel.

A member of the Team first needs to authorize it.

…engine and update related documentation and tests
@br-to br-to changed the title feat: add sign_allowlist declarative policy rule feat: add recipient_allowlist declarative policy rule May 17, 2026
@Sertug17

Copy link
Copy Markdown
Contributor

Hi @br-to I'm the author of #169 which overlaps with this.
@0xultravioleta suggested converging on your structure as the base.

I'd suggest we keep your RecipientAllowlist as is and I open a
follow up PR that adds:

  1. MaxTransactionValue rule (per tx wei cap)
  2. SigningOperation on PolicyContext (operation gating)
  3. RLP decoder extension in ows signer (cleaner than policy layer parsing)

Would that split work for you?

@0xultravioleta

Copy link
Copy Markdown
Collaborator

Quick heads up on convergence with #169, which proposed an overlapping recipient-allowlist rule. We are aligning on this PR as the base, since it is the more focused version and updates the spec doc. @Sertug17 will add the value cap and the per-operation gating as a follow-up on top of this once it lands, rather than crowding this PR.

One thing worth adding here while it is open: validate that the allowlist entries are well-formed addresses and normalize them (lowercase, or EIP-55) at load time, so a malformed or mis-cased entry cannot be silently accepted and then never match. Happy to help review. Thanks.

@br-to

br-to commented Jun 11, 2026

Copy link
Copy Markdown
Author

Thanks @Sertug17, that split works great for me — happy to keep this focused on RecipientAllowlist and let you layer the value cap and operation gating on top.

@0xultravioleta good call on address validation. I'll add normalization (lowercase) and reject malformed entries at load time before merging. Will ping when ready for review.

Reject malformed Ethereum addresses and normalize to lowercase when
saving or loading policies, preventing silent mismatches from mis-cased
or invalid entries in recipient_allowlist / allowed_typed_data_contracts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ef45a83. Configure here.

Comment thread ows/crates/ows-lib/src/policy_engine.rs
br-to and others added 2 commits June 11, 2026 11:01
… works in signing paths

The API-key signing paths always built PolicyContext with to: None,
so any policy with recipient_allowlist denied every real transaction.
Extend the existing rlp module to extract the `to` field from unsigned
EIP-1559/EIP-2930 transactions and populate it for EVM chains before
policy evaluation. Legacy txs, malformed bytes, contract creation and
non-EVM chains still yield None and remain fail-closed.

Co-authored-by: Cursor <cursoragent@cursor.com>
…-policy-rule

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	ows/crates/ows-lib/src/key_ops.rs
@br-to

br-to commented Jun 11, 2026

Copy link
Copy Markdown
Author

Update: both items addressed and pushed!

  1. Address validation & normalization (ef45a83): reject malformed entries and lowercase at save time, normalize on load for defense-in-depth

  2. to extraction from raw tx (05d99ef): parse recipient from EVM typed transactions in the signing path so recipient_allowlist actually evaluates correctly — previously to was always None, which would deny everything unconditionally

For non-transaction operations (message signing, typed data), to remains None and recipient_allowlist passes through, matching the AllowedTypedDataContracts pattern.

Ready for re-review! @0xultravioleta @Sertug17

@Sertug17

Copy link
Copy Markdown
Contributor

Looks great @br-to address normalization and the to extraction are exactly what was needed. The pattern matches what I had in #169 too. LGTM from my side.

@0xultravioleta

Copy link
Copy Markdown
Collaborator

Hi @br-to, thanks, this looks much better on the re-read. Two notes.

First, the address handling is solid now. Validating entries as EVM addresses at save time, and only extracting to from EIP-1559/2930 typed transactions, means the lowercase comparison only ever runs on EVM hex, where case doesn't matter. My earlier worry was a case-sensitive non-EVM address (base58/base32) getting lowercased and colliding with an allowlisted entry, but since anything that isn't an EVM typed tx yields to = None and is denied, that path can't be reached. Good.

Second, one thing to confirm so we're on the same page: your note says messages and typed-data "pass through," but the code and the doc actually deny them. to is None, so eval_recipient_allowlist returns denied. Fail-closed is the right call for a security rule, I just want to make sure that's the intent and not a mismatch, because the practical effect is that attaching recipient_allowlist to an API key also blocks message, typed-data, non-EVM, and legacy-EVM signing on that key. The doc calls that out, and the per-operation gating in the #169 follow-up is the clean way to relax it later.

Assuming fail-closed is intended, this is in good shape. Nice catch on the to-always-None bug as well, that one would have denied everything.

@Sertug17

Copy link
Copy Markdown
Contributor

Yes, fail-closed is intentional for recipient_allowlist if you attach
this rule to an API key, you are saying "this key may only send to these
addresses." Message signing, typed data, and non EVM operations all
yield to = None, so they are denied. That is the correct conservative
posture for a recipient restriction.

The per-operation gating in the #169 follow up is exactly how you relax
this selectively e.g. allow message signing while still enforcing the
allowlist for sign_transaction. The two PRs are intentionally layered
that way.

@br-to

br-to commented Jun 11, 2026

Copy link
Copy Markdown
Author

Good catch on the wording — “passes through” was misleading on my part. You’re right that ‎to = None → deny is the intended behavior. Fail-closed is what we want here: if you attach ‎recipient_allowlist, only transactions with an explicit ‎to that matches the allowlist get through. Everything else (message signing, typed data, non‑EVM) is denied.

Thanks both for the review!⁠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants