feat(gateway): add centralized AI prompt validation#247
Conversation
|
@karuna-1 is attempting to deploy a commit to the ankanmisra's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @karuna-1, thanks for opening this PR. Every contribution helps MicroAI-Paygate grow. If you find the project useful, consider starring the repository — it helps others discover it. Star MicroAI-Paygate on GitHub Looking forward to reviewing this PR. |
github-actions
Bot
added
go
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds shared prompt validation with a 4000-character limit and injection-pattern checks, wires it into gateway request handling, and updates the request schema and form input to match. Adds unit coverage for the validator. ChangesPrompt Validation
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CacheMiddleware
participant handleSummarize
participant validatePrompt
Client->>CacheMiddleware: submit summarize request
CacheMiddleware->>validatePrompt: validatePrompt(req.Text)
validatePrompt-->>CacheMiddleware: error or nil
CacheMiddleware->>handleSummarize: forward validated request
handleSummarize->>validatePrompt: validatePrompt(req.Text)
validatePrompt-->>handleSummarize: error or nil
handleSummarize-->>Client: 400 or summarize response
Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
gateway/prompt_validation_test.go (1)
10-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a boundary case and coverage for other injection patterns.
No test asserts that a prompt of exactly
MaxPromptLengthcharacters passes (onlyMaxPromptLength+1is tested), and only 1 of the 5injectionPatternsentries is exercised.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gateway/prompt_validation_test.go` around lines 10 - 41, Update TestValidatePrompt to cover the missing boundary and broader injection coverage: add a case in TestValidatePrompt that uses exactly MaxPromptLength characters and expects success, and expand the prompt injection coverage so ValidatePrompt exercises the other injectionPatterns entries in gateway/prompt_validation_test.go. Use the existing MaxPromptLength constant and the prompt validation test table to keep the cases aligned with the validation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/prompt_validation.go`:
- Around line 11-17: The broad "you are now" entry in injectionPatterns is
causing false positives in prompt validation. Tighten the matching in
prompt_validation.go by replacing the plain substring check with a more specific
rule in injectionPatterns/prompt validation logic, such as word-boundary or
regex-based matching, or require nearby role-assignment context before flagging
it. Keep the existing blacklist approach, but make this pattern less aggressive
so ordinary text is not rejected by the prompt validation path.
- Around line 25-32: The prompt length check in prompt validation is using byte
length instead of character count, so adjust the logic in the prompt validation
function to count Unicode characters rather than raw bytes. Update the
`len(prompt)` usage in the rejection check and the error formatting to use a
rune-based count so the `MaxPromptLength` limit matches the stated “characters”
semantics. Keep the existing logging and error path in `prompt_validation.go`,
but ensure the reported “received” value reflects character count consistently.
---
Nitpick comments:
In `@gateway/prompt_validation_test.go`:
- Around line 10-41: Update TestValidatePrompt to cover the missing boundary and
broader injection coverage: add a case in TestValidatePrompt that uses exactly
MaxPromptLength characters and expects success, and expand the prompt injection
coverage so ValidatePrompt exercises the other injectionPatterns entries in
gateway/prompt_validation_test.go. Use the existing MaxPromptLength constant and
the prompt validation test table to keep the cases aligned with the validation
logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d77e72ef-10c3-46bf-898e-cf507b23c312
📒 Files selected for processing (4)
gateway/cache.gogateway/main.gogateway/prompt_validation.gogateway/prompt_validation_test.go
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
AnkanMisra
added
bug
ChatGPT Codex Connector
|
@codex review the pr |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7fe5b9809
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
github-actions
Bot
added
documentation
|
Hi! Could you please add the GSSoC 2026 label to this PR if it's eligible? Thank you! |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a3d27cad5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a3d27cad5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| lower := strings.ToLower(prompt) | ||
| for _, pattern := range injectionPatterns { | ||
| if strings.Contains(lower, pattern) { |
There was a problem hiding this comment.
Because this endpoint summarizes arbitrary user-provided documents, this substring check rejects any otherwise valid text that merely quotes or discusses one of these phrases, such as an article about prompt injection containing ignore all previous instructions. In the browser flow that user is asked to sign the x402 challenge and then receives a 400 instead of a summary, so consider treating the input as quoted content in the AI prompt or using a narrower validation mechanism rather than blocking raw substrings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for the feedback. I agree that simple substring matching can reject some benign quoted content. For this PR, I kept the validation intentionally simple and centralized the existing behavior without changing the overall security policy. A more context-aware validation approach would be a good follow-up improvement.
| onChange={(e) => setInput(e.target.value)} | ||
| placeholder={`Paste any text. The summary returns after a ${DISPLAY_CHAIN_NAME} signature.`} | ||
| rows={9} | ||
| maxLength={4000} |
There was a problem hiding this comment.
For prompts containing non-BMP characters such as emoji, this browser limit is enforced in UTF-16 code units while validatePrompt counts Unicode runes with utf8.RuneCountInString, so the web app blocks or truncates inputs that the gateway contract would accept, e.g. a 3,000-emoji prompt is under the 4,000-character server limit but exceeds this textarea limit. Count/enforce the same units in the client or avoid relying on the native maxLength for the server's character limit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for the feedback. I used the native maxLength={4000} to enforce the documented API limit with minimal client-side logic. I agree that matching the backend's rune-based counting exactly would handle Unicode edge cases more accurately, but I kept this PR focused on the existing validation contract. A rune-aware client-side counter could be considered as a follow-up enhancement.
| // Validate text is not empty (also validated in cache middleware, but needed here for non-cached requests) | ||
| if req.Text == "" { | ||
| c.JSON(400, gin.H{"error": "Invalid request", "message": "text field cannot be empty"}) | ||
| if err := validatePrompt(req.Text); err != nil { |
There was a problem hiding this comment.
On cache misses or when CACHE_ENABLED=false, the signed retry reaches verifyPayment before this new prompt validation runs; the verifier claims the nonce once the signature is valid, so an over-limit or newly disallowed prompt is rejected with 400 after its payment context has already been burned. API clients that retry the same signed headers then get nonce_already_used without ever receiving a summary, so parse and validate the body before calling the verifier on this path, matching the cache middleware ordering.
Useful? React with 👍 / 👎.
2 participants
Related Issue
Fixes #244
Summary
This PR improves input validation for AI summarize requests by introducing a centralized validation helper that's used before requests are sent to the AI provider.
What Changed
Added a shared
validatePrompt()helper to avoid duplicated validation logic.Rejects empty and whitespace-only prompts.
Rejects prompts longer than 4000 characters.
Detects and blocks common prompt injection phrases.
Logs rejected prompts for easier monitoring and debugging.
Updated both:
handleSummarize()CacheMiddleware()to use the shared validation helper.
Added unit tests covering:
Testing
go test ./...Notes
While working on this issue, I noticed the current codebase uses a provider abstraction (
Generate(...)) instead of thecallLLM(...)example shown in the issue description. To keep the implementation consistent with the existing architecture, I applied the validation at the request layer without changing the provider interface.Summary by CodeRabbit
400 Invalid requestwith a clear validation message.