Skip to content

fix: cap POST /download-zip at 50 documents#111

Open
bmersereau wants to merge 2 commits into
willchen96:mainfrom
bmersereau:fix/99-download-zip-limit
Open

fix: cap POST /download-zip at 50 documents#111
bmersereau wants to merge 2 commits into
willchen96:mainfrom
bmersereau:fix/99-download-zip-limit

Conversation

@bmersereau
Copy link
Copy Markdown

@bmersereau bmersereau commented May 13, 2026

Summary

  • Adds a MAX_ZIP_DOCUMENTS = 50 constant and a 400 guard to POST /single-documents/download-zip
  • Prevents an authenticated user from triggering simultaneous in-memory loading of an unbounded number of large files

Closes #99
Closes #114

Changes

  • backend/src/routes/documents.tsMAX_ZIP_DOCUMENTS constant + length check returning 400
  • backend/src/lib/__tests__/downloadZipLimit.test.ts — static analysis test verifying the limit exists
  • backend/vitest.config.ts — vitest config scoping tests to src/ (excludes compiled dist/)
  • backend/package.json"test": "vitest run" script added

Test plan

  • Unit tests added and passing
  • Backend build passes

Copy link
Copy Markdown
Author

@bmersereau bmersereau left a comment

Choose a reason for hiding this comment

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

PR Review

Summary

Adds MAX_ZIP_DOCUMENTS = 50 guard to POST /single-documents/download-zip, returning 400 with a clear message when the request exceeds the limit. Simple, correct, and exactly scoped to the issue.

Findings

  • [severity:praise] Named constant MAX_ZIP_DOCUMENTS makes the limit self-documenting and easy to tune
  • [severity:praise] Error message includes the actual limit value (Cannot download more than ${MAX_ZIP_DOCUMENTS} documents at once) — good UX
  • [severity:nit] Static test uses readFileSync — verifies pattern is present but not that the HTTP response is actually 400. Acceptable given the simplicity of the change

Specific checks

  • "test": "vitest run" in package.json ✓
  • vitest.config.ts include filter present ✓
  • Tests pass: 2/2 ✓
  • documents.ts unchanged from origin/main (aside from this fix) ✓

Verdict

Approve — clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant