fix(decode): independently surface safeTransferFrom recipient + tokenId - #679
Merged
Conversation
Adds ERC-721 safeTransferFrom selectors (0x42842e0e, 0xb88d4fde) to the independent calldata decoder's HIGH_RISK_STANDARD_SELECTORS, mirroring the #573 setApprovalForAll fallback, so the NFT recipient and tokenId surface in CHECKS PERFORMED on uncurated destinations (rogue-MCP-only / cooperating-agent defense-in-depth). Re: #670. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Cross-repo companion filed: szhygulin/vaultpilot-security-skill#50 — Inv #17, the skill-side ERC-721 / collection-offer decoder that surfaces the proceeds recipient distinct from the asset recipient. Scope: cooperating-agent guidance only; a rogue agent ignores it. |
…ality Review follow-up: the HighRiskSelector doc comment claimed a module-load `assertSelectorMatches` guard that does not exist. Corrected to describe the actual pinning mechanism — the per-entry decode tests assert each selector against its calldata, so a mistyped selector fails in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Re: #670 (which is closed as architectural — full agent+MCP collusion is undefendable here; this is the cooperating-agent / rogue-MCP-only defense-in-depth slice).
Adds the two ERC-721
safeTransferFromselectors (0x42842e0e,0xb88d4fde) to the independent calldata decoder'sHIGH_RISK_STANDARD_SELECTORSinsrc/signing/decode-calldata.ts, mirroring the merged #573setApprovalForAllfallback. On an uncurated NFT destination reached viaprepare_custom_call, the recipient (to) andtokenId(andfrom) now surface in CHECKS PERFORMED, independently decoded — so a cooperating agent + user see the real recipient even if the MCP-reported summary lies.safeTransferFrom(...,bytes)form: added adynamicTailflag with a minimum-length head guard (the dynamicbytestail makes total length variable); viem'sdecodeFunctionDatavalidates the encoding and thedatapayload is surfaced so a hook-bearing transfer can't hide it.test/nft-operator-approval-decode.test.ts(+12): genuine fail-before (3 positive-decode tests fail without the selectors) / pass-after.Scope: this does NOT defend the full agent+MCP collusion in #670 (an independent decode that runs inside a rogue MCP is adversary-controlled; the residual anchor is on-device clear-sign, tracked at #453). It hardens the rogue-MCP-only case. The skill-side ERC-721 / collection-offer decoder companion (Inv #17) is being filed in
szhygulin/vaultpilot-security-skill(linked in a comment below).🤖 Generated with Claude Code