feat: Add input size validation for queries and attachments #3359
Conversation
…062) Implement backend validation to prevent users from submitting arbitrarily large queries or attachments that could consume backend memory and LCS resources. Changes: - Add size limit constants: MAX_QUERY_LENGTH (32K), MAX_ATTACHMENT_SIZE_BYTES (20MB), MAX_TOTAL_ATTACHMENTS_SIZE_BYTES (50MB) - Extend validateCompletionsRequest middleware with query length and attachment size validation - Add attachments field to QueryRequestBody interface - Add query length validation to notebooks query endpoint - Add comprehensive test coverage for all validation scenarios Addresses security vulnerability identified in Red Hat Product Security threat model (dated Mar 15, 2026). Co-Authored-By: Rajin Kichannagari <rkichann@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
rajin-kichannagari
requested review from
a team,
karthikjeeyar,
rohitkrai03 and
yangcao77
as code owners
Resolved conflicts: - validation.ts: Kept our size validation imports alongside upstream changes - validation.test.ts: Removed isAllowedProxyPath tests (function moved in upstream refactor), kept our new validateCompletionsRequest tests for RHIDP-13062
Changed Packages
|
rajin-kichannagari
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3359 +/- ##
==========================================
- Coverage 54.01% 54.01% -0.01%
==========================================
Files 2409 2404 -5
Lines 87705 87541 -164
Branches 24287 24250 -37
==========================================
- Hits 47373 47284 -89
+ Misses 40054 40020 -34
+ Partials 278 237 -41
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Jdubrick
left a comment
Jdubrick
left a comment
There was a problem hiding this comment.
I believe we will need to update the express.json(..) portion when defining the routers to override the allowable body limit, or else it will default to whatever Express' default is. Adding an integration test to test this might be beneficial too, since right now we just unit test the validation itself
Notebook: https://github.com/redhat-developer/rhdh-plugins/blob/main/workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts#L70
Lightspeed: https://github.com/redhat-developer/rhdh-plugins/blob/main/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts#L108
|
I also want to ping @karthikjeeyar to verify if there will need to be any follow up UI changes to display to users when their attachments / body was too large? |
Configure express.json() middleware with explicit 60mb limit to handle large query payloads with attachments (up to 50MB total attachment size). This ensures the body parser limit aligns with the validation constraints. Tested: Verified 60MB limit accommodates maximum valid requests (50MB attachments + 32K query) while providing a safety ceiling. Addresses reviewer feedback on PR redhat-developer#3359. Co-Authored-By: Rajin Kichannagari <rkichann@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Adds integration tests that exercise the full request pipeline via supertest, verifying that oversized queries are rejected with 400 before reaching the LCS proxy. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Jdubrick
left a comment
Jdubrick
left a comment
There was a problem hiding this comment.
One comment about how we should handle the allocation of the updated body size, otherwise looks good
|
Can we add a changeset too @rajin-kichannagari ? Since there is a possibility someone sending a query with the new changes might hit the limit, we should document it in the changes |
…eset
- Move express.json({ limit }) from router-level to route-level on /v1/query endpoints
- Add EXPRESS_JSON_BODY_LIMIT constant to constant.ts
- Add changeset for input validation changes
- Remove internal ticket references from code comments
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract attachment validation into validateAttachments helper - Extract notebooks query validation into validateNotebooksQuery helper - Use for-of loop instead of indexed for loop - Consolidate type assertions in tests via callValidate helper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
3 participants
Implement backend validation to prevent users from submitting arbitrarily large queries or attachments that could consume backend memory and LCS resources.
Changes:
Checklist
Note: Changeset not applicable for security fix. Frontend changes may be needed in a follow-up issue to communicate limits to users.