feat: #2375 add approval exploit lookup

[Dozie2001]

Issue

Closes #2375

What this changes

Adds web3/check-approval-exploits, a credential-free read action that matches up to 100 supplied token/spender pairs against RevokeCash/approval-exploit-list for the selected EVM chain.

The action resolves one exact upstream commit from GitHub's public commit feed, then fetches the index and all incident records from that immutable revision through safeFetch. Retrieval uses bounded concurrency, timeouts, response-size limits, schema validation, and a short-lived cache that is populated only after the complete snapshot validates.

Results preserve the input index, token address, and spender address. Matching is case-insensitive for EVM addresses and exact for chain ID. Each match includes the incident slug, context, historical source amount, article URLs, optional fixed value, and revision-pinned source URL.

A source or validation failure cannot produce a clean negative result. With the default failOnError, the step fails. When the option is disabled, the step continues with lookupStatus: "error" and null results and counts.

The action description and coverage output state that it does not discover approvals, read allowances or Permit2 state, or certify wallet safety.

No npm dependency, credential, database change, pricing change, signing path, or existing response shape changes.

Scope

This is one read-only Web3 action. The registry entry, step wrapper, source retrieval and matching core, and focused tests are interdependent parts of that action.

How it was verified

• pnpm check
• pnpm type-check
• pnpm vitest run tests/unit/check-approval-exploits.test.ts
• Full unit-suite run: 23,025 tests passed; the two existing loopback-binding suites were rerun with local socket access and all 124 tests passed.
• Live source verification loaded and validated all 67 current Revoke.cash incident records by immutable revision through KeeperHub's safeFetch path.

The focused tests cover normalized address matching, exact chain matching, duplicate input association, multiple incident matches, revision-pinned evidence, invalid inputs, partial retrieval failure, softened fail-closed output, and disabled workflow retries.

---

• Targets staging
• Title carries the issue number
• pnpm check and pnpm type-check pass
• No secrets, .env files, or credentials committed

[github-actions]

About the build check on this pull request

This pull request comes from a fork, so GitHub does not pass it the credentials build normally uses for our image registry cache and staging build configuration. The build still runs and still compiles the image, so a red build here is real; it just takes longer than on team branches.

Every workflow run on a pull request from a fork also waits for a maintainer to approve it, so checks can sit at "awaiting approval" for a while after each push. Nothing is needed from you for either of these.

[suisuss]

Before the review: #2387 implements the same issue and has been open since 10 September, with @subheeksh5599 having said on #2375 that they were taking it. You could not have known from the issue alone - I should have recorded the claim on the issue more visibly than a comment, and that is on me. Both PRs insert into the same hunk of plugins/web3/index.ts, so only one can land as-is. I read yours on its own merits first and then compared; the comparison is at the end, and parts of your design are going in whichever PR carries it.

What this changes

plugins/web3/steps/check-approval-exploits-core.ts resolves the list's head commit from the repository's Atom feed, fetches index.json and each record at that revision, and builds a spender -> incidents[] map filtered to the selected chain. check-approval-exploits.ts is a thin "use step" wrapper applying applyReadFailOnError once to the final result, with maxRetries = 0. The action exposes lookupStatus, results[] with one entry per input pair carrying inputIndex and status, matchedPairCount, source with the pinned revision, and coverage.

The core/step split is right and matches the dominant web3 layout, the single soften point is what the read-fail-on-error-core.ts docstring prescribes, and maxRetries = 0 matches every web3 sibling.

Fail-closed is structural rather than guarded here, which is the stronger form: :206-213 throws unless index.json is a non-empty array and :263-268 requires at least one address per record, so an empty corpus is unreachable by construction rather than caught by a tripwire. Addresses are lowercased on both sides. Timeouts on every fetch. No assertUrlIsPublic finding - both hosts are compile-time constants and the only interpolated segments are a 40-hex SHA and a pattern-matched slug.

Blocking

• check-approval-exploits.ts - there is no logUserError or logSystemError anywhere in the step. withStepLogging records an error step-log and an error metric only when result.success === false, and under failOnError: false the softened result is success: true. -> raw.githubusercontent.com degrades fleet-wide, every execution silently reports a successful step with a success metric and no Sentry event, and the only trace is a lookupStatus: "error" field nobody alerts on. failOnError: false is the setting a cautious author is most likely to pick, so this is the common case, not the edge. -> Log before softening.

• check-approval-exploits-core.ts:344-356 - on failure the cache is never written, so the next execution reissues all 69 requests. Combined with CACHE_TTL_MS of 5 minutes at :23 - about 828 requests per hour per pod against a rate-limited unauthenticated endpoint - a 429 becomes self-sustaining across the fleet. -> A negative cache, and a TTL matched to a list that changes on the order of weeks rather than minutes.

• check-approval-exploits-core.ts:195-204 - fetchRevision extracts the head SHA from Grit::Commit/<40 hex> in the Atom feed. That is GitHub's internal Rails identifier scheme, not a documented API, and failing it is fail-closed. -> If GitHub changes that <id> format the step is hard-down for every user until we ship a fix, over a cosmetic upstream change. The 64 KB cap at :196 has the same property: the feed embeds full commit bodies for the last 20 commits, so a few verbose commits trip it with the same total blast radius. -> Keep the pinning, take the SHA from something documented.

Mechanical - actionable as-is

• :246-306 - strict per-record validation aborts the whole snapshot on one bad record. I validated all 67 live records against your schema and none fail today, so it is latent - but upstream adding one record with a null amount takes down the other 66. Per-record skip plus a skipped-ratio guard plus a skippedRecords output is the shape that keeps both properties.

• :206-213, :23 - MAX_INCIDENTS = 250 against 67 today, and 1 MB per record against a current maximum of 639,118 bytes. The second is 1.56x headroom on a record that has grown before, and crossing either is a permanent hard failure with no partial-degradation path.

• :171-178 - the size cap is checked after arrayBuffer(), so it bounds parsing, not allocation.

• Untested, failure paths first: the empty and non-array index.json branches, slug rejection, the cache and its TTL and the inFlight coalescing, the feed with no Grit::Commit match, both size guards, the non-JSON body, every parseRecord branch, MAX_TOTAL_ADDRESSES, five of the six parsePairs branches, mapConcurrent with more slugs than FETCH_CONCURRENCY, the timeout signal, and that destinationError: true is actually set on the unknown-chain path.

• approvalPairs is a json-editor and the help tip says template references may be used inside the JSON string. processTemplates skips arrays, and lib/workflow/validation/action-config.ts:448-458 shows a json-editor value may be stored as either a string or a parsed array. If the editor persists a parsed array, every {{...}} in it is silently never rendered. I could not determine which shape it persists - UNSURE, and worth checking before this field type ships. #2387 used template-textarea for exactly this reason.

With the team

• Which of the two goes forward, and I am not leaving that open long. My position: #2387 carries it, because resilience, observability and evidence are where a security check earns its keep - it retries, it negative-caches, it logs before softening, and it has 23 tests against your 7, with the gap almost entirely in failure paths. That is not a first-mover preference; if the positions were reversed I would say the same.

  What yours does better goes in regardless, and I will hold #2387 until it does: the per-pair results[] with inputIndex, because a caller inside a For Each currently cannot map results back to inputs or tell "checked and clean" from "dropped"; the lookupStatus discriminator; revision pinning and source, which makes an answer reproducible where #2387's is not; all matching incidents rather than the first; and the anti-claim text shipped into the output rather than left in the action description. Your input address validation too - #2387 accepts any string as a spender, so a template rendering to undefined sails through and reports zero matches, which is a silent false negative in the wrong direction. I have raised that on #2387 as a defect and credited it there.

  Neither of you has the thing both need: a per-chain coverage count in the output, so not_listed on a chain with no entries is distinguishable from not_listed on a chain with 186. arcadia-finance alone holds 6,514 of the list's 7,134 addresses, all on Base, so that distinction is not hypothetical.

Verdict

Changes requested, and the immediate question is not the findings - it is whether to fold this into #2387 or keep going here. Tell me which you would rather do and I will make it work; if you would rather not carry a second round on a PR that may not be the one that lands, say so and I will port your design across with credit on the commits.

[Dozie2001]

Thanks for the careful review and for clarifying the overlap with #2387. Given that #2387 was claimed first and you prefer it as the landing PR, I would rather fold this into #2387 than run a competing second review round here.

Please port the parts you identified from this PR: the per-pair results[] with inputIndex and status, the lookupStatus discriminator, revision pinning and source metadata, all matching incidents, input address validation, the anti-claim coverage text, and the per-chain coverage count that both implementations still need.

I appreciate the offer to preserve credit. I would like to keep #2417 open as the reviewable source until those changes are visible on #2387. I am also happy to review and manually test the combined implementation before closing this PR as superseded.

This decision is only about which upstream PR carries the KeeperHub action. I will continue the hackathon integration independently, as advised on #2331.