fix: prevent prompt injection via systemInstruction separation#274
Open
akshara200829-lgtm wants to merge 2 commits into
Open
fix: prevent prompt injection via systemInstruction separation#274akshara200829-lgtm wants to merge 2 commits into
akshara200829-lgtm wants to merge 2 commits into
Conversation
- Split static instructions from user-supplied data in prompts.js and aiController.js using Gemini's systemInstruction parameter, closing the structural injection vector described in the issue. - Added systemInstruction to the /api/generate and /api/ai/generate routes in aiRoutes.js, since this route accepts open-ended prompts and can't use the same data/instruction split as structured routes. - Fixed a crash in sanitizeAiPrompt.js where role/topic were sanitized unconditionally without checking they were present, causing a TypeError on requests that omit those fields. - Corrected aiPromptSchema.js: role and topic are now optional (they are not part of the /api/generate route's actual contract), and added a structural character allow-list as defense-in-depth alongside the existing regex blacklist.
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
1 participant
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.
Related Issue
Closes #246
Summary
Fixes a structural prompt injection vulnerability in prompts.js and aiController.js, where user-supplied values (role, topic, experience, topicsToFocus, question) were interpolated directly into the same string that carried the model's output-format instructions. This allowed crafted input to override the expected response structure or attempt to leak the system prompt, and it bypassed validateAiPrompt/sanitizeAiPrompt entirely, since the exploit lived downstream of both, inside prompt construction itself.
Also fixes a crash in sanitizeAiPrompt.js (unconditional sanitization of fields that may not be present) and corrects a schema/route mismatch in aiPromptSchema.js.
Type of Change
How Has This Been Tested?
Verified locally that prompts.js / aiController.js now build prompts as a static systemInstruction (no user data) plus a separate plain-data payload, for both generate-questions and generate-explanation routes.
Verified aiRoutes.js's /api/generate and /api/ai/generate routes now pass a systemInstruction into every candidate model in the fallback loop, hardening the open-ended-prompt route against persona override / instruction-leak attempts.
Reproduced and fixed the sanitizeAiPrompt.js crash: previously, requests omitting role/topic threw a TypeError on .replace(); each field now has an independent typeof === "string" guard before sanitizing.
Corrected aiPromptSchema.js to match the route's actual contract (role/topic are optional, not required) and added a structural character allow-list as defense-in-depth alongside the existing regex blacklist.
Confirmed end-to-end locally that a request flows through validateAiPrompt → sanitizeAiPrompt → reaches the Gemini API call correctly with systemInstruction attached (confirmed via direct SDK call bypassing Express). Hit a free-tier quota limit (limit: 0 for gemini-2.0-flash) specific to my personal API key when trying to observe the final model response, so live output wasn't directly inspected, but the full pipeline up to and including the outbound API call was verified working.
Screenshots (if applicable)
Add screenshots or videos to demonstrate the changes.
Checklist