fix(#2204): restrict getProgramAccounts to Percolator programs, not just to filtered queries - #2530
Merged
Merged
Conversation
…ust to filtered queries The filter requirement already in place (`dataSize` or `memcmp`) is necessary but NOT sufficient, and the issue's own example is the proof: dumping the SPL Token Program supplies `dataSize: 165` — the token-account size — quite happily. The filter check passes and the upstream node still returns every token account on the cluster. A filter bounds the result SHAPE. It does not bound the PROGRAM, and on a program with millions of matching accounts that is no bound at all. Both halves of the report were real; only one had been closed. So the program is pinned too. This proxy exists to serve THIS app, and the app only ever queries programs it owns: wrapper + every slab tier getAllProgramIds() (userAccountScan) matcher matcherProgramId (matcherCaps) NFT program nftProgramId (userAccountScan) stake/vault vaultProgramId Anything else is not a query we need to serve, so it is refused with -32602 rather than forwarded to a paid endpoint. Built from config rather than a hardcoded list, so adding a slab tier extends the allowlist with no change here — the same property getAllProgramIds() already gives assertKnownProgram. A non-string `params[0]` is rejected rather than coerced, so a missing target cannot fall through to the upstream node. One test pins that the allowlist is NON-EMPTY. An allowlist that silently degrades to empty still "works" — it rejects everything — but the inverse shape, where an empty set is treated as permissive, is the dangerous one, and this makes a config regression visible either way. Launch suite: 3128 passed / 16 skipped / 0 failed. Refs: #2204 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NgoNgagkvw7i5SSRC3FJ8D
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedNext included review available in 45 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
Closes #2204.
A filter requirement (
dataSize/memcmp) was already in place. It is necessary but not sufficient, and the issue's own example is the proof:That query supplies
dataSize: 165— the SPL token-account size — quite happily. The filter check passes, and the upstream node still returns every token account on the cluster.A filter bounds the result shape. It does not bound the program. On a program with millions of matching accounts, that is no bound at all. Both halves of the report were real; only one had been closed.
The program is now pinned too
Built from config, not a hardcoded list:
getAllProgramIds()userAccountScanmatcherProgramIdmatcherCapsnftProgramIduserAccountScanvaultProgramIdAnything else is refused with
-32602rather than forwarded to a paid endpoint. Adding a slab tier extends the allowlist with no change here — the same propertygetAllProgramIds()already givesassertKnownProgram.A non-string
params[0]is rejected rather than coerced, so a missing target cannot fall through.One test worth calling out
It pins that the allowlist is non-empty. An allowlist that degrades to empty still "works" — it rejects everything — but the inverse shape, where an empty set is treated as permissive, is the dangerous one. This makes a config regression visible either way.
Launch suite: 3128 passed / 16 skipped / 0 failed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NgoNgagkvw7i5SSRC3FJ8D