Skip to content

fix(api): use query() validator and check result for GET /api/search#502

Open
Rajeevydvv wants to merge 1 commit into
krkn-chaos:mainfrom
Rajeevydvv:fix-get-validation
Open

fix(api): use query() validator and check result for GET /api/search#502
Rajeevydvv wants to merge 1 commit into
krkn-chaos:mainfrom
Rajeevydvv:fix-get-validation

Conversation

@Rajeevydvv

@Rajeevydvv Rajeevydvv commented May 22, 2026

Copy link
Copy Markdown
Contributor

Bug Fix

Related Feature: Chatbot API — search endpoint (api/server.js)
Branch name: fix-get-validation

Changes:

The GET /api/search endpoint had two critical validation flaws that allowed malformed or oversized queries to bypass validation and reach the search service:

  1. Wrong Validator Location: It used body('query') from express-validator. Since this is a GET route, the query string is in the URL (req.query), not the request body (req.body). The validator silently found nothing and passed.
  2. Missing Result Check: Even if the validator worked, the handler never checked validationResult(req), meaning validation failures were entirely ignored.

Root Cause

express-validator provides separate functions for different input locations:

  • body() → reads req.body (POST/PUT data)
  • query() → reads req.query (URL query string)

Because the search endpoint is a GET route, it must use query().

What this PR does:

  • Replaces body('query') with queryValidator('query') so it correctly targets URL parameters instead of the request body.
  • Adds .isString() to the validation chain to reject array query parameters (e.g., preventing errors from ?query=a&query=b).
  • Adds a validationResult(req) check at the start of the handler that halts execution and returns a 400 Bad Request with the errors.array() if validation fails.

Note: The body() validators on the POST /api/chat endpoint (lines 162-173) are correct and remain unchanged, as POST requests do carry data in the body.

Files Changed

  • api/server.js — Fixed the validator function location and added the missing validationResult error handler for the GET search endpoint.

@netlify

netlify Bot commented May 22, 2026

Copy link
Copy Markdown

Deploy Preview for krkn-chaos ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit da4169a
🔍 Latest deploy log https://app.netlify.com/projects/krkn-chaos/deploys/6a19e026e989f700084a35e6
😎 Deploy Preview https://deploy-preview-502--krkn-chaos.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Fix GET /api/search validation using query() validator

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fixed GET /api/search endpoint validation using query() instead of body()
• Imported queryValidator from express-validator for URL query parameters
• Corrected validator to read from req.query instead of empty req.body
• Applied code formatting cleanup (trailing whitespace removal)
Diagram
flowchart LR
  A["GET /api/search endpoint"] -->|Previously used| B["body() validator"]
  B -->|Reads from| C["req.body empty"]
  C -->|Result| D["Validation never executed"]
  A -->|Now uses| E["queryValidator() validator"]
  E -->|Reads from| F["req.query URL params"]
  F -->|Result| G["Validation properly enforced"]

Loading

File Changes

1. api/server.js 🐞 Bug fix +24/-24

Replace body() with queryValidator() for GET search

• Added query: queryValidator to express-validator import statement
• Replaced body('query') with queryValidator('query') on GET /api/search endpoint
• Removed trailing whitespace throughout file for code formatting consistency
• Validation rules (min 1, max 200 characters) now properly execute on URL query parameters

api/server.js


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 22, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Context used
✅ Compliance rules (platform): 24 rules

Grey Divider


Action required

1. Search validation not enforced 🐞 Bug ≡ Correctness
Description
GET /api/search adds queryValidator('query') but never calls validationResult(req), so
express-validator errors are ignored and invalid/oversized query values still reach
documentationIndex.search(). This undermines the intended fix and can also cause downstream errors
when non-string query values (e.g., repeated query params) reach code paths that assume query is a
string.
Code

api/server.js[R255-268]

Evidence
The search route defines a query validator but does not read validation results, unlike the chat
route which correctly rejects invalid input. DocumentationIndex uses string operations on query
(e.g., toLowerCase()), so non-string query values can throw or degrade behavior if validation
isn’t enforced.

api/server.js[157-184]
api/server.js[254-295]
api/services/DocumentationIndex.js[94-134]
api/services/DocumentationIndex.js[136-140]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`GET /api/search` registers express-validator rules but never checks `validationResult(req)`, so requests with invalid `query` values are still processed.

### Issue Context
The `/api/chat` endpoint already demonstrates the intended pattern (validate + `validationResult` check + 400 on error). The search path calls `documentationIndex.search(query, ...)`, and downstream code assumes `query` is a string.

### Fix Focus Areas
- api/server.js[254-295]

### Proposed change
1. In the `/api/search` handler, add:
  - `const errors = validationResult(req);`
  - if errors exist, return `400` with `errors.array()`.
2. Consider strengthening the validator chain to enforce type:
  - `queryValidator('query').isString().trim().isLength({ min: 1, max: 200 }) ...`
  This prevents `?query=a&query=b` (array) from reaching the service.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread api/server.js
Comment on lines 255 to 268
app.get('/api/search',
[
body('query')
queryValidator('query')
.trim()
.isLength({ min: 1, max: 200 })
.withMessage('Query must be between 1 and 200 characters')
],
async (req, res) => {
try {
const { query, limit = 10 } = req.query;

if (!query) {
return res.status(400).json({
error: 'Query parameter is required'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Search validation not enforced 🐞 Bug ≡ Correctness

GET /api/search adds queryValidator('query') but never calls validationResult(req), so
express-validator errors are ignored and invalid/oversized query values still reach
documentationIndex.search(). This undermines the intended fix and can also cause downstream errors
when non-string query values (e.g., repeated query params) reach code paths that assume query is a
string.
Agent Prompt
### Issue description
`GET /api/search` registers express-validator rules but never checks `validationResult(req)`, so requests with invalid `query` values are still processed.

### Issue Context
The `/api/chat` endpoint already demonstrates the intended pattern (validate + `validationResult` check + 400 on error). The search path calls `documentationIndex.search(query, ...)`, and downstream code assumes `query` is a string.

### Fix Focus Areas
- api/server.js[254-295]

### Proposed change
1. In the `/api/search` handler, add:
   - `const errors = validationResult(req);`
   - if errors exist, return `400` with `errors.array()`.
2. Consider strengthening the validator chain to enforce type:
   - `queryValidator('query').isString().trim().isLength({ min: 1, max: 200 }) ...`
   This prevents `?query=a&query=b` (array) from reaching the service.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Thanks @qodo-code-review for catching the missing validationResult check! I've updated the PR to add .isString() to the validator chain and properly handle the validation errors

@Rajeevydvv Rajeevydvv force-pushed the fix-get-validation branch from 1e90c74 to 35ce654 Compare May 22, 2026 18:48
@Rajeevydvv Rajeevydvv changed the title fix(api): use query() validator instead of body() for GET /api/search fix(api): use query() validator and check result for GET /api/search May 22, 2026
@Rajeevydvv

Copy link
Copy Markdown
Contributor Author

Hi @paigepatton Whenever you have some free time, could you please review this PR?

It fixes a bug in the GET /api/search endpoint where the express-validator rules were silently failing because they were targeting the request body instead of the URL query string, and the validation results weren't being checked. I've also addressed the feedback from the Qodo bot by adding .isString() to prevent array inputs from reaching the search service.

Let me know if you need any changes!

Thank you

@github-actions github-actions Bot added the needs-dco Commits are missing a Signed-off-by line label May 29, 2026
@Rajeevydvv Rajeevydvv force-pushed the fix-get-validation branch from 556b89c to ee81df9 Compare May 29, 2026 18:50
@github-actions github-actions Bot removed the needs-dco Commits are missing a Signed-off-by line label May 29, 2026
Signed-off-by: Rajeev-Cryptographer <Rajeevofficial3@gmail.com>
@Rajeevydvv Rajeevydvv force-pushed the fix-get-validation branch from ee81df9 to da4169a Compare May 29, 2026 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant