update zod safePath validation to match existing behaviour in web (#36357)

* update zod safePath validation to match existing behaviour in web

* test SafePath from web against zod validation

GitOrigin-RevId: a159f5078b5bf75eac4365c97e34e21e716290eb

libraries/validation-tools/test/unit/src/zodHelpers.test.ts

@@ -1,6 +1,7 @@
 import { zz } from '../../../zodHelpers'
 import { describe, expect, it } from 'vitest'
 import mongodb from 'mongodb'
+import SafePath from '../../../../../services/web/app/src/Features/Project/SafePath.mjs'
 
 const { ObjectId } = mongodb
 
@@ -422,8 +423,10 @@ describe('zodHelpers', () => {
 
   describe('safePath', () => {
     it('fails to parse with empty input', () => {
-      const parsed = zz.safePath().safeParse('')
+      const inputPath = ''
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path is empty',
@@ -435,14 +438,18 @@ describe('zodHelpers', () => {
       // unlike filepath(), a leading "/" is allowed -- project doc/file
       // paths are root-relative in production (see web's
       // ProjectEntityHandler.getAllEntitiesFromProject).
-      const parsed = zz.safePath().safeParse('/output.pdf')
+      const inputPath = '/output.pdf'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(true)
-      expect(parsed.data).toBe('/output.pdf')
+      expect(SafePath.isCleanPath(inputPath)).toBe(true)
+      expect(parsed.data).toBe(inputPath)
     })
 
     it('fails to parse a path ending in a slash (a folder, not a file)', () => {
-      const parsed = zz.safePath().safeParse('foo/')
+      const inputPath = 'foo/'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path is a folder, not a file',
@@ -451,8 +458,10 @@ describe('zodHelpers', () => {
     })
 
     it('fails to parse when provided with path traversal', () => {
-      const parsed = zz.safePath().safeParse('../output.pdf')
+      const inputPath = '../output.pdf'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path traversal detected',
@@ -464,8 +473,10 @@ describe('zodHelpers', () => {
       // matches SafePath.mjs's BADFILE_RX (^\.$), not just "..": a lone "."
       // component is also rejected there, cross-checked against
       // SafePath.test.mjs's isCleanFilename/isCleanPath cases.
-      const parsed = zz.safePath().safeParse('.')
+      const inputPath = '.'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path segment is "." or has leading/trailing whitespace',
@@ -474,8 +485,10 @@ describe('zodHelpers', () => {
     })
 
     it('fails to parse a path segment with leading whitespace', () => {
-      const parsed = zz.safePath().safeParse(' foobar.tex')
+      const inputPath = ' foobar.tex'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path segment is "." or has leading/trailing whitespace',
@@ -484,8 +497,10 @@ describe('zodHelpers', () => {
     })
 
     it('fails to parse a path segment with trailing whitespace', () => {
-      const parsed = zz.safePath().safeParse('foobar.tex ')
+      const inputPath = 'foobar.tex '
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path segment is "." or has leading/trailing whitespace',
@@ -494,10 +509,10 @@ describe('zodHelpers', () => {
     })
 
     it('fails to parse a path containing a null byte', () => {
-      const parsed = zz
+      const inputPath = 'foo' + String.fromCharCode(0) + '.tex'
-        .safePath()
+      const parsed = zz.safePath().safeParse(inputPath)
-        .safeParse('foo' + String.fromCharCode(0) + '.tex')
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path contains a disallowed character',
@@ -507,10 +522,10 @@ describe('zodHelpers', () => {
 
     it('fails to parse a path containing a C1 control character', () => {
       // \x80-\x9F, per SafePath.mjs's BADCHAR_RX
-      const parsed = zz
+      const inputPath = 'foo' + String.fromCharCode(0x90) + '.tex'
-        .safePath()
+      const parsed = zz.safePath().safeParse(inputPath)
-        .safeParse('foo' + String.fromCharCode(0x90) + '.tex')
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path contains a disallowed character',
@@ -520,8 +535,10 @@ describe('zodHelpers', () => {
 
     it('fails to parse a path containing a lone surrogate', () => {
       // \uD800-\uDFFF, per SafePath.mjs's BADCHAR_RX
-      const parsed = zz.safePath().safeParse('foo\uD800.tex')
+      const inputPath = 'foo\uD800.tex'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path contains a disallowed character',
@@ -530,8 +547,10 @@ describe('zodHelpers', () => {
     })
 
     it('fails to parse a path containing an asterisk', () => {
-      const parsed = zz.safePath().safeParse('foo*.tex')
+      const inputPath = 'foo*.tex'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path contains a disallowed character',
@@ -540,8 +559,10 @@ describe('zodHelpers', () => {
     })
 
     it('fails to parse a path containing a backslash', () => {
-      const parsed = zz.safePath().safeParse('foo\\bar.tex')
+      const inputPath = 'foo\\bar.tex'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
           message: 'path contains a disallowed character',
@@ -549,53 +570,58 @@ describe('zodHelpers', () => {
       ])
     })
 
-    it('fails to parse a path with a __proto__ segment', () => {
+    it('parses successfully with a nested __proto__ segment', () => {
-      const parsed = zz.safePath().safeParse('__proto__/output.pdf')
+      // web's SafePath.mjs allows a reserved name below the top level (e.g.
-      expect(parsed.success).toBe(false)
+      // a folder literally named "__proto__" is permitted), so this schema
-      expect(parsed.error?.issues).toMatchObject([
+      // must not reject a path web itself considers valid.
-        expect.objectContaining({
+      const inputPath = '__proto__/output.pdf'
-          message: 'path segment is an unsafe property name',
+      const parsed = zz.safePath().safeParse(inputPath)
-        }),
+      expect(parsed.success).toBe(true)
-      ])
+      expect(SafePath.isCleanPath(inputPath)).toBe(true)
+      expect(parsed.data).toBe(inputPath)
     })
 
     it('fails to parse a path that is exactly "constructor"', () => {
-      const parsed = zz.safePath().safeParse('constructor')
+      const inputPath = 'constructor'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
-          message: 'path segment is an unsafe property name',
+          message: 'path is an unsafe property name',
         }),
       ])
     })
 
-    it('fails to parse a nested segment matching a built-in Object.prototype method name', () => {
+    it('parses successfully with a nested segment matching a built-in Object.prototype method name', () => {
       // full BLOCKEDFILE_RX parity, not just the prototype-pollution subset
-      // (__proto__/constructor/prototype) -- and checked per segment, not
+      // (__proto__/constructor/prototype) -- checked only against the
-      // only the top-level path as SafePath.mjs's isCleanPath does.
+      // top-level path, same as SafePath.mjs's isCleanPath.
-      const parsed = zz.safePath().safeParse('foo/toString')
+      const inputPath = 'foo/toString'
-      expect(parsed.success).toBe(false)
+      const parsed = zz.safePath().safeParse(inputPath)
-      expect(parsed.error?.issues).toMatchObject([
+      expect(parsed.success).toBe(true)
-        expect.objectContaining({
+      expect(SafePath.isCleanPath(inputPath)).toBe(true)
-          message: 'path segment is an unsafe property name',
+      expect(parsed.data).toBe(inputPath)
-        }),
-      ])
     })
 
     it('fails to parse a path that is exactly "hasOwnProperty"', () => {
-      const parsed = zz.safePath().safeParse('hasOwnProperty')
+      const inputPath = 'hasOwnProperty'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(false)
+      expect(SafePath.isCleanPath(inputPath)).toBe(false)
       expect(parsed.error?.issues).toMatchObject([
         expect.objectContaining({
-          message: 'path segment is an unsafe property name',
+          message: 'path is an unsafe property name',
         }),
       ])
     })
 
     it('parses successfully when provided a valid nested path', () => {
-      const parsed = zz.safePath().safeParse('foo/output.pdf')
+      const inputPath = 'foo/output.pdf'
+      const parsed = zz.safePath().safeParse(inputPath)
       expect(parsed.success).toBe(true)
-      expect(parsed.data).toBe('foo/output.pdf')
+      expect(SafePath.isCleanPath(inputPath)).toBe(true)
+      expect(parsed.data).toBe(inputPath)
     })
   })
 

libraries/validation-tools/zodHelpers.js

@@ -89,18 +89,18 @@ const zz = {
    * BADFILE_RX, and BLOCKEDFILE_RX), which every path in a project's file
    * tree is already validated against at the point of origin -- this
    * closes the gap for services that receive that path secondhand (e.g.
-   * forwarded into history-v1's archive/zip builder). Cross-checked
+   * forwarded into history-v1's archive/zip builder). Parity with
-   * against every case in web's SafePath.test.mjs (isCleanPath) via a
+   * isCleanPath is asserted directly in zodHelpers.test.ts, which imports
-   * throwaway script requiring both modules independently; the only
+   * SafePath.mjs and checks it against every safePath() test case; the only
-   * remaining differences from that reference are two deliberate ones:
+   * remaining difference from that reference is deliberate: BADCHAR_RX's
-   * (1) BADCHAR_RX's leading "/" is not ported -- here "/" is the segment
+   * leading "/" is not ported here, since "/" is the segment separator,
-   * separator, not a disallowed character, so a traversal payload is
+   * not a disallowed character, so a traversal payload is still caught
-   * still caught (as "." or "..") per split segment below, just not via
+   * (as "." or "..") per split segment below, just not via this character
-   * this character check; (2) BLOCKEDFILE_RX's property-name check is
+   * check. The unsafe-property-name check (BLOCKEDFILE_RX) matches
-   * applied per segment, not only to the top-level path the way
+   * isCleanPath exactly -- only the top-level path, not nested segments --
-   * SafePath.mjs's isCleanPath does -- stricter on purpose, since any
+   * since web already allows a reserved name below the top level (e.g. a
-   * segment here (not just the first) could end up as an object key or
+   * folder literally named "prototype" is permitted), and this schema
-   * archive/zip entry name downstream.
+   * shouldn't reject paths web itself considers valid.
    */
   safePath: () => {
     // eslint-disable-next-line no-control-regex
@@ -123,8 +123,8 @@ const zz = {
       .refine(s => !BAD_CHAR_RX.test(s), {
         message: 'path contains a disallowed character',
       })
-      .refine(s => !s.split('/').some(seg => UNSAFE_SEGMENT_RX.test(seg)), {
+      .refine(s => !UNSAFE_SEGMENT_RX.test(s.replace(/^\//, '')), {
-        message: 'path segment is an unsafe property name',
+        message: 'path is an unsafe property name',
       })
   },
   /**