Merge pull request #36219 from overleaf/jpa-zod-web-platform

[web] platform: adopt strict zod schema and extend test coverage

GitOrigin-RevId: e3a0b7b4c52ac116f28be0d284284e3bd3994cda

services/filestore/app.js

@@ -69,13 +69,8 @@ if (settings.filestore.stores.template_files) {
   )
 }
 
-// Not a named `:key(.*)` param: Express 4.22.1's bundled path-to-regexp has a
-// bug where combining a named param with a custom (.*) regex both duplicates
-// the match under a numeric key AND corrupts the captured value (off-by-one
-// substring). The anonymous `*` wildcard doesn't have this bug, so keep it
-// and validate the numeric params[0] key directly (see schemas.js).
 app.get(
-  '/bucket/:bucket/key/*',
+  '/bucket/:bucket/key/:key(.+)',
   keyBuilder.bucketFileKeyMiddleware,
   fileController.getFile
 )

services/filestore/app/js/FileController.js

@@ -4,11 +4,7 @@ import parseRange from 'range-parser'
 import Errors from './Errors.js'
 import { pipeline } from 'node:stream'
 import { parseReq } from '@overleaf/validation-tools'
-import {
-  getFileQuerySchema,
-  getFileQueryFallbackSchema,
-  insertFileSchema,
-} from './schemas.js'
+import { getFileQuerySchema, getFileQueryFallbackSchema } from './schemas.js'
 
 const maxSizeInBytes = 1024 * 1024 * 1024 // 1GB
 
@@ -125,7 +121,6 @@ function getFileHead(req, res, next) {
 }
 
 function insertFile(req, res, next) {
-  parseReq(req, insertFileSchema, { logOnly: true })
   metrics.inc('insertFile')
   const { key, bucket } = req
 

services/filestore/app/js/KeyBuilder.js

@@ -44,7 +44,7 @@ function bucketFileKeyMiddleware(req, res, next) {
     logOnly: true,
   })
   req.bucket = params.bucket
-  req.key = params[0]
+  req.key = params.key
   next()
 }
 

services/filestore/app/js/schemas.js

@@ -46,10 +46,7 @@ export const templateFileParamsFallbackSchema = z.object({
 export const bucketFileParamsSchema = z.object({
   params: z.strictObject({
     bucket: z.string().regex(/^[a-z0-9-]+$/),
-    // the anonymous `*` wildcard in app.js (kept anonymous rather than a
-    // named `:key(.*)` -- see the comment there for why); Express exposes
-    // its capture under the numeric key '0'. May contain nested segments.
-    0: zz.filepath(),
+    key: zz.filepath(),
   }),
 })
 
@@ -111,13 +108,3 @@ export const getFileQueryFallbackSchema = z.object({
     range: z.string().optional(),
   }),
 })
-
-// insertFile reads the raw request stream itself (no body parser is
-// mounted), so the body is validated as a streamed body. No fallbackSchema:
-// insertFile never reads parseReq()'s return value (FileHandler.insertFile
-// reads the stream off `req` directly), so a raw passthrough on failure
-// can't hand the handler a wrong type -- a fallback here would just
-// duplicate the primary schema.
-export const insertFileSchema = z.object({
-  body: zz.streamedBody(),
-})

services/filestore/test/unit/js/schemas.test.js

@@ -5,21 +5,21 @@ describe('schemas', function () {
   describe('bucketFileParamsSchema', function () {
     it('accepts a well-formed bucket name', function () {
       const result = bucketFileParamsSchema.safeParse({
-        params: { bucket: 'my-bucket-123', 0: 'some/key' },
+        params: { bucket: 'my-bucket-123', key: 'some/key' },
       })
       expect(result.success).to.equal(true)
     })
 
     it('rejects a bucket name with disallowed characters', function () {
       const result = bucketFileParamsSchema.safeParse({
-        params: { bucket: 'My_Bucket!', 0: 'some/key' },
+        params: { bucket: 'My_Bucket!', key: 'some/key' },
       })
       expect(result.success).to.equal(false)
     })
 
     it('rejects an empty bucket name', function () {
       const result = bucketFileParamsSchema.safeParse({
-        params: { bucket: '', 0: 'some/key' },
+        params: { bucket: '', key: 'some/key' },
       })
       expect(result.success).to.equal(false)
     })