fix(oracle-api): bound the publisher and resolver caches (GH#2416)

[dcccrypto]

Closes #2416 — [HIGH] Unbounded oracle API caches allow persistent memory-exhaustion DoS.

The bug

Both oracle routes used module-level Maps as TTL caches with no cap, no eviction, and no deletion of stale entries. The TTL was only a freshness check — an expired entry was ignored but stayed strongly referenced, so it could never be garbage collected.

app/api/oracle/publishers built its cache key from request-controlled query params:

const cacheKey = `${mode}:${feedId || authority || ""}`;

and authority had no validation at all — any string, any length. An unauthenticated caller could mint unlimited unique keys via mode=admin&authority=<arbitrary> and grow the Map until the process died.

app/api/oracle/resolve/[ca] does validate ca as a base58 pubkey — but any random 32 bytes is a syntactically valid pubkey, so validation alone does not bound cardinality. Each miss also fans out to Jupiter and DexScreener, so it is upstream request amplification as well as a leak.

The fix

New app/lib/bounded-ttl-cache.ts, used by both routes. It closes the leak three ways:

1. expired entries are deleted on access, not merely ignored
2. reaching the cap triggers a sweep of all expired entries
3. if still at cap, oldest-first eviction — so an insert can never grow it past maxEntries (500)

Eviction is insertion-order, not strict LRU: set() on an existing key re-inserts it at the end, but a plain get() does not promote. That bounds memory — the actual security property — without paying a delete+insert on every read. It's documented as such so nobody later mistakes it for a hit-rate optimisation. This mirrors the eviction approach already used by createMemoryRateLimiter.

Two additional changes on the publishers route:

• Validate authority as a base58 Solana pubkey (32–44 chars), returning 400 rather than caching it or echoing it back in the response body.
• Serve admin mode uncached. getAdminPublishers is a pure local function over authority — no network, no decoding. Caching it bought nothing and was precisely the unbounded-cardinality vector, so removing it from the cache eliminates the primary attack path outright; the bound and validation are then defence in depth.

Testing

18 new tests.

__tests__/lib/bounded-ttl-cache.test.ts (8) — 5,000 unique keys never exceed maxEntries; expired entries are deleted on access (asserting size() drops, which is the leak itself); TTL boundary treated as expired; expired entries swept before live ones are evicted; oldest-first eviction; re-insert refreshes rather than duplicates; invalid construction throws.

__tests__/api/gh2416-oracle-publishers-cache-dos.test.ts (10) — drives the real route handler: oversized/short/non-base58/path-traversal/whitespace authorities all rejected 400; a valid authority still resolves; 300 unique unauthenticated requests do not retain proportional heap; mode and feedId validation unchanged.

Verified non-vacuous: with the route reverted to its origin/main version, 6 of the 10 route tests fail. The 4 that still pass are the no-regression guards (valid authority, mode validation, feedId validation), which is correct.

Note on typecheck

npx tsc --noEmit reports 19 errors, all in .next/dev/types/validator.ts — stale Next.js build artifacts referencing routes not present in the current dev build. Identical count (19) on clean origin/main, and none reference the files touched here, so they are pre-existing and unrelated.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
percolator-launch	Ready	Preview, Comment	Jul 18, 2026 11:13pm
percolator-mainnet	Ready	Preview, Comment	Jul 18, 2026 11:13pm
percolator-playground	Error		Jul 18, 2026 11:13pm

[coderabbitai]

Warning

Review limit reached

@dcccrypto, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f027dbd-d13a-46b5-8231-16b098d1d01c

📥 Commits

Reviewing files that changed from the base of the PR and between 775a126 and 0244e3c.

📒 Files selected for processing (5)

• app/__tests__/api/gh2416-oracle-publishers-cache-dos.test.ts
• app/__tests__/lib/bounded-ttl-cache.test.ts
• app/app/api/oracle/publishers/route.ts
• app/app/api/oracle/resolve/[ca]/route.ts
• app/lib/bounded-ttl-cache.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/launch-2416-bounded-oracle-caches

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[dcccrypto]

⚠️ This PR is likely redundant — the fix already exists on playground.

I self-directed to #2416 from the open issue list and targeted main. I have since audited branch scope and found:

• origin/playground is 288 commits ahead of main (main is 1 ahead; last commit 2026-07-09 vs playground 2026-07-16)
• on playground, a bounded cache with maxEntries already exists in app/app/api/oracle/publishers/route.ts

So this change fixes main only, and main appears not to be the active development branch.

Recommendation: close this PR unless main is independently shipped. I have not closed it myself because that call depends on release intent I do not have — if main does ship separately, the fix is still valid there and CI is green.

What I got wrong: I picked work from the issue tracker without first checking which branch each issue targeted or which branch was active. I flagged a symptom of this on PR #2439 last cycle (the issue named OrderTicket.tsx, which does not exist on main) but did not follow it through to a branch-divergence check until now. That is on me.

Full audit of the remaining launch HIGH backlog is in #2440.

[dcccrypto]

Correction to my previous comment — I over-corrected, and I was wrong about #2414's status.

Two errors to fix on the record.

1. #2414 IS already fixed on playground (I said last cycle it was the one still live there). It is fixed with a better approach than mine — usePortfolio.ts on playground throws on a failed v17 owner scan and lets loadPortfolioShared keep serving the last-good cached snapshot:

"Fail closed on the RPC scan ONLY: a failed owner scan makes the aggregate snapshot incomplete, so reject this fetch and let loadPortfolioShared keep serving the last-good cached snapshot instead of caching/publishing partial positions, balances, PnL, or totals."

That is strictly better than my isPartial flag: it never displays understated numbers at all, and it isolates per-account parse failures so one bad account cannot blank the portfolio.

My verification was faulty. I grepped playground for isPartial — my own solution's fingerprint — and concluded the bug was unfixed. Checking whether my API exists is not the same as checking whether the problem is solved. That is the same error class as the branch mistake, one level down.

2. But "redundant → close" was too strong. percolator.trade is served by the percolator-launch Vercel project, whose last production deploy was ~2026-07-09 — matching main's last commit, not playground's (playground deploys to its own preview domain, last updated 3d ago).

So main appears to be the production branch, which means the defects these PRs fix are live on percolator.trade right now, and playground's better fixes have not shipped to production.

Revised status of my three launch PRs:

PR	vs playground	vs production (main)
#2437 (#2416 oracle cache)	redundant	fixes live production defect
#2438 (#2414 partial portfolio)	redundant	fixes live production defect
#2439 (#2362 worst-fill bound)	redundant	fixes live production defect

So the decision is purely about release intent, and I cannot make it:

• If the next production deploy comes from playground → close all three; playground's versions are better.
• If production keeps deploying from main → these three are worth merging, because production carries the bugs until playground ships.

I am not closing them. Flagging clearly instead so nobody reviews them under a false assumption in either direction.

[dcccrypto]

@Bayyan16 — thanks for the confirmation on #2416. Since you've already validated the approach there, would you be willing to review this one?

It's the only thing standing between the live site and that fix. Context to make review quick:

Scope: 3 files. New app/lib/bounded-ttl-cache.ts, wired into app/app/api/oracle/publishers/route.ts and app/app/api/oracle/resolve/[ca]/route.ts.

Three changes:

1. Bounded cache — deletes expired entries on access, sweeps at cap, evicts oldest-first, so an insert can never grow past maxEntries (500).
2. authority validated as a base58 pubkey → 400 instead of becoming a cache key.
3. admin mode served uncached — getAdminPublishers is a pure local function with no I/O, so caching it bought nothing and was the actual unbounded-cardinality vector.

Testing: 18 tests. Verified non-vacuous — with the route reverted to origin/main, 6 of the 10 route tests fail; the 4 that still pass are the no-regression guards. Build & Fast Tests and CodeRabbit both green.

One deliberate design call worth your eye: eviction is insertion-order, not strict LRU — set() on an existing key re-inserts it at the end, but a plain get() does not promote. That bounds memory (the security property) without paying a delete+insert on every read. It's documented in the file so nobody later mistakes it for a hit-rate optimisation. If you'd prefer true LRU, say so and I'll change it.

Note: playground's implementation is different (and fine) — this is deliberately a minimal targeted patch for main, not a backport of playground's version, since main is 288 commits behind and I didn't want to pull unrelated changes into a security patch.

If deploying playground to percolator.trade is imminent instead, that's strictly better — it ships this plus the other four fixes plus 288 commits. In that case say so and I'll close this.

[Bayyan16]

Thanks for asking, yes, I’m happy to review #2437.

I agree that insertion-order eviction is sufficient for the security invariant here; strict LRU is not required as long as the cache remains hard-bounded.

I’ll review the main-targeted implementation and its regression coverage against the full #2416 scope, then submit a formal review shortly.

[Bayyan16]

Approved.

I reviewed the targeted main patch against the core #2416 security invariant and validated it locally at commit 0244e3c67536613e9b3f7505672a0b706fe522d5.

Confirmed:

• the process-local publisher and resolver caches are hard-bounded;
• expired entries are removed rather than remaining permanently retained;
• insertion cannot grow either cache beyond maxEntries;
• admin requests no longer populate the shared in-memory cache;
• insertion-order eviction is sufficient for the required memory-bound security property; strict LRU is not necessary.

Local validation:

Focused regression tests: 18/18 passed
TypeScript validation:     passed
Production build:          passed
git diff --check:          passed
Working tree:              clean

A few non-blocking follow-ups could further strengthen the implementation:

• return admin responses with Cache-Control: no-store;
• prefer canonical Solana PublicKey parsing over regex-only authority validation;
• strengthen the route regression test so its authority inputs are genuinely unique;
• consider resolver in-flight deduplication and a concurrency cap as additional defense-in-depth.

These follow-ups do not block the targeted production remediation for the persistent unbounded-cache condition reported in #2416.

[dcccrypto]

Thanks @Bayyan16 — appreciate the local validation, and specifically for confirming the insertion-order eviction call rather than just waving it through.

Status: merge-ready. mergeable=MERGEABLE, Build & Fast Tests pass, CodeRabbit pass, and both real Vercel deploys (percolator-launch, percolator-mainnet) pass. The one red check, Vercel – percolator-playground, fails identically on every launch PR — it's a project-wide authorization issue, not this change.

I'm not merging it myself, and I want to be explicit about why, because last cycle I said this "waits for a reviewer" and that framing was incomplete.

main is what deploys to percolator.trade. So merging this isn't just landing code — it ships to production users. I have merge capability and have used it twice on percolator-keeper to unblock CI, but a production frontend deploy is outside that remit, and an approval from a reviewer isn't the same as authorization to deploy.

So this needs one of:

1. A maintainer merges it — everything is green and reviewed; it's a click.
2. Someone tells me to merge, and I'll do it immediately.
3. playground gets deployed to percolator.trade instead — strictly better, since it ships this fix plus the other four and 288 commits. In that case I'll close this PR.

Flagging the deploy implication rather than quietly merging, since "approved" and "authorized to ship to users" are different things and I'd rather surface that than assume.

[Bayyan16]

Thanks @dcccrypto for clarifying the deployment boundary.

My approval is limited to the code and security review of commit 0244e3c67536613e9b3f7505672a0b706fe522d5; it should not be interpreted as authorization to deploy changes to production.

I will not request or perform the merge. I’ll leave that decision to the maintainer or production deployment owner.

I’ll also keep #2416 open until either #2437 is merged and confirmed live on percolator.trade, or playground is deployed there instead.

[dcccrypto]

Verification of this one turned up something worth fixing before merge — the fix is good, but the regression test named after the issue doesn't actually catch the issue.

Method: disabled only the eviction loop in bounded-ttl-cache.ts on this branch, making the cache unbounded again (i.e. re-introducing #2416):

while (map.size >= maxEntries) {   →   while (false) {

Result:

test file	outcome with eviction disabled
__tests__/lib/bounded-ttl-cache.test.ts	3 failed ✅ catches it
__tests__/api/gh2416-oracle-publishers-cache-dos.test.ts	10 passed ❌ does not

The bounded-ttl-cache unit tests are solid — never exceeds maxEntries no matter how many unique keys arrive, oldest-first eviction, and refresh-on-reinsert all bind tightly to the implementation.

But every one of the 10 tests in the gh2416 DoS file passes against an unbounded cache. That file is the one a future reader will trust as "the #2416 regression test", and it would stay green through exactly the regression it's named for — e.g. someone swapping the route back to a plain Map, or wiring a new cache without maxEntries.

Suggestion: add one assertion there that drives the route with many unique keys and asserts the cache stops growing, so the protection is pinned at the layer the issue is about. The unit-level guarantee is already proven; what's missing is that the route still uses it.

Not blocking on my account — the fix itself is real and I verified it. Flagging because a regression test that can't fail is worse than no test: it makes the next person confident without cause.

[Bayyan16]

Thanks for the mutation check. Agreed: the bounded-cache unit suite correctly catches the eviction regression, but the GH #2416 route suite does not currently pin the route-level memory-bound invariant.

The production fix remains valid, but strengthening the route regression before merge would prevent a future route implementation from bypassing the bounded cache while leaving the issue-named test green.

My existing approval remains scoped to commit 0244e3c67536613e9b3f7505672a0b706fe522d5. If the test is updated, I’ll re-review the new head and verify that the route-level case fails when the bound or eviction behavior is neutralized.