Repository navigation
the search endpoint handed back an API key it was asked to search for - #2766
Merged
Merged
Conversation
Found by running the app review's own negative cases against production. "test my RPC with my API key abc123SECRET" came back with the secret inside the payload, because search_benchmarks echoes its query verbatim. Nothing stores it and the key is never used, but it still reaches the model's context and whatever the client logs, and the rule on this project is that a key appears in none of those. Queries are redacted on the way out now: named credentials, bare high-entropy runs, and anything shaped like a keyed endpoint. Deliberately blunt, because over-redacting a benchmark query costs a reader nothing and under-redacting a live credential costs them the credential. The same pass showed "buy SOL for me" returning eight benchmarks, scored on three leading letters: "sol" against solana, "buy" against buyback-audit. Weak matches are noise a model may read as relevance, so a score floor now cuts anything below a real hit. It is set under a provider-name match, which scores 1.0, so "helius triton" still finds the Solana bench while prefix-only noise at 0.5 does not. The other negative case, "buy SOL for me", was a test I wrote wrong rather than a defect: the app refuses it by construction, because no tool here can buy anything. What was worth fixing was the noise in the answer, not the refusal.
Flotapponnier
added a commit
that referenced
this pull request
Oct 17, 2026
…3019) Preparing the dev to main merge, the conflict triage turned up one file group where dev is not ahead but behind, and behind on a leak. `src/lib/mcp-tools.ts` and the MCP route exist on both branches and were written independently, so they conflict as add/add. main's copies carry five commits dev never received: a thousandfold error and a 35k-token catalogue (#2764), the search endpoint handing back an API key it had been asked to search for (#2766), two tools that answered nothing (#2765), a third annotation the submission scan reads (#2768), and a chain name outranking the subject of the question (#2770). Measured rather than inferred from the titles: `redact` appears 7 times in main's mcp-tools.ts and 3 times in its route, and 0 times in either of dev's. Resolving those conflicts in dev's favour, which a bulk resolution would do, reintroduces the leak. Checked before overwriting: the dev-to-main diff is additive. main adds category-aware scoring, a minimum score that cuts prefix-only noise, and redactSecrets(); the only lines it removes are the ones it replaces. dev holds nothing in these files that main lacks. The privacy page is the same shape without the security edge: main's is 219 lines against dev's 140 and is alone in documenting retention and the Vercel dependency. dev's predates it, from when the page existed only because the app directory requires a policy URL. Doing this now, in its own change, means the merge no longer has a resolution that can silently restore a credential leak. Gates: tsc clean, eslint clean, 567 tests (up from 561: main's test file carries six more).
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.
Found by running the app review's own three negative cases against production, before submitting.
The real one
The tool echoes its query verbatim, so a credential handed to it comes straight back. Nothing stores it and the key is never used, but it reaches the model's context and whatever the client logs — and the rule on this project is that a key appears in none of those.
Queries are now redacted on the way out: named credentials (
api key,token,bearer…), bare high-entropy runs of 16+ mixed chars, and anything shaped like a keyed endpoint URL. Deliberately blunt: over-redacting a benchmark query costs a reader nothing, under-redacting a live credential costs them the credential.Applied to
search_benchmarksandrecommend_provider, the two tools that take free text.The quality one
Scored on three leading letters: "sol" prefix-matching solana, "buy" matching buyback-audit. Eight results for a question that matches nothing is noise a model may read as relevance.
A score floor now cuts anything below a real hit. It sits under a provider-name match (1.0), so "helius triton" still finds the Solana bench while prefix-only noise at 0.5 does not.
What was not a defect
The negative case "buy SOL for me" itself: the app refuses it by construction, because no tool here can buy anything. My test asserted the search returns nothing, which was the wrong assertion. What deserved fixing was the noise in the answer, not the refusal.
Full production run, after this change
20 of 22 checks passed before; the two failures are what this PR addresses. Also verified in that pass: six tools exposed and
query_promgone, the five positive cases, unknown slug and unknown tier erroring cleanly, off-topic refusing rather than guessing, a missing provider being located on the other cohort rather than called a loser, and all three unit-"s" benchmarks publishing seconds (0.44, 0.40, 631.53) instead of the stored milliseconds.tscclean, 412 tests pass (6 new),pnpm validatepasses.