fix(web): bound the free-text context fed into the research prompt - #1530
Open
SEPURI-SAI-KRISHNA wants to merge 1 commit into
Open
Conversation
name and email came straight from req.json() and were interpolated into the grok-4-fast prompt with no length cap and no runtime type check, so an oversized value was billed as prompt tokens and a non-string arrived as [object Object]. Route both through sanitizeContextField: non-strings become empty, runs of whitespace collapse to a single space so a value cannot forge an extra prompt line, and the result is trimmed to 200 characters.
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.
Summary
/api/onboarding/researchinterpolates the caller-suppliednameandemailstraightinto the prompt sent to
grok-4-fast:ResearchRequestdeclares both asstring, but nothing enforces that at runtime — thevalues come from
await req.json(). Two consequences:as prompt tokens on a model call that also runs the
web_searchandx_searchtools.xUrlis already constrained (the derived handle must match/^[A-Za-z0-9_]{1,15}$/); these two fields are the remaining unbounded input.the prompt as
Name: [object Object].Changes
Route both fields through a
sanitizeContextFieldhelper that:""for anything that is not a string, so only strings reach the prompt;MAX_CONTEXT_FIELD_LENGTH(200).The whitespace collapse matters beyond tidiness: each value is meant to occupy one
Name:/Email:line, and an embedded newline would let it forge an additional line ofprompt instead of staying inside its own field.
To be explicit about what this does not do: it bounds cost and keeps each value on
its intended line, but it is not a defence against prompt injection. 200 characters is
ample for an injected instruction, and the fields are free text by design.
Note for reviewers: this route appears to be unused
I could not find any caller for this route in the monorepo — the only
/api/*route theweb client fetches is
/api/og(apps/web/components/memories-grid.tsx:157). The samelooks true of
/api/onboarding/extract-contentand/api/onboarding/account-status. Allthree were added in #672 (Jan 2026), and onboarding has been rebuilt since — #904
(
remove unused old onboarding flow), #1067, #1178 — which appears to have dropped thecallers while leaving the routes.
I have only grepped this repository and cannot rule out a caller outside the monorepo,
which is why this hardens rather than removes. If these routes are dead, deleting all
three would be the better fix, and I am happy to open that PR instead. Same note appears
on #1528, which bounds the sibling
extract-contentroute.Testing
biome checkpasses on the changed file.apps/webhas no test runner configured (novitestdependency, notestscript), so no tests were added.