Merge pull request #36402 from overleaf/bg-zod-move-safe-path-tests

move safe path tests from zodHelpers to web

GitOrigin-RevId: 3bb25fa108a4a0da0bed1ae60fabfdd47ed76283
This commit is contained in:
Brian Gough
2026-09-17 11:58:29 +00:00
committed by Copybot
parent 31d766c4b9
commit a4b9b18109
3 changed files with 92 additions and 53 deletions
@@ -1,7 +1,6 @@
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
@@ -426,7 +425,6 @@ describe('zodHelpers', () => {
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',
@@ -441,7 +439,6 @@ describe('zodHelpers', () => {
const inputPath = '/output.pdf'
const parsed = zz.safePath().safeParse(inputPath)
expect(parsed.success).toBe(true)
expect(SafePath.isCleanPath(inputPath)).toBe(true)
expect(parsed.data).toBe(inputPath)
})
@@ -449,7 +446,6 @@ describe('zodHelpers', () => {
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',
@@ -461,7 +457,6 @@ describe('zodHelpers', () => {
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',
@@ -476,7 +471,6 @@ describe('zodHelpers', () => {
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',
@@ -488,7 +482,6 @@ describe('zodHelpers', () => {
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',
@@ -500,7 +493,6 @@ describe('zodHelpers', () => {
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',
@@ -512,7 +504,6 @@ describe('zodHelpers', () => {
const inputPath = 'foo' + String.fromCharCode(0) + '.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',
@@ -525,7 +516,6 @@ describe('zodHelpers', () => {
const inputPath = 'foo' + String.fromCharCode(0x90) + '.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',
@@ -538,7 +528,6 @@ describe('zodHelpers', () => {
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',
@@ -550,7 +539,6 @@ describe('zodHelpers', () => {
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',
@@ -562,7 +550,6 @@ describe('zodHelpers', () => {
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',
@@ -577,7 +564,6 @@ describe('zodHelpers', () => {
const inputPath = '__proto__/output.pdf'
const parsed = zz.safePath().safeParse(inputPath)
expect(parsed.success).toBe(true)
expect(SafePath.isCleanPath(inputPath)).toBe(true)
expect(parsed.data).toBe(inputPath)
})
@@ -585,7 +571,6 @@ describe('zodHelpers', () => {
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 is an unsafe property name',
@@ -600,7 +585,6 @@ describe('zodHelpers', () => {
const inputPath = 'foo/toString'
const parsed = zz.safePath().safeParse(inputPath)
expect(parsed.success).toBe(true)
expect(SafePath.isCleanPath(inputPath)).toBe(true)
expect(parsed.data).toBe(inputPath)
})
@@ -608,7 +592,6 @@ describe('zodHelpers', () => {
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 is an unsafe property name',
@@ -620,7 +603,6 @@ describe('zodHelpers', () => {
const inputPath = 'foo/output.pdf'
const parsed = zz.safePath().safeParse(inputPath)
expect(parsed.success).toBe(true)
expect(SafePath.isCleanPath(inputPath)).toBe(true)
expect(parsed.data).toBe(inputPath)
})
})
+3 -3
View File
@@ -90,9 +90,9 @@ const zz = {
* 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). Parity with
* isCleanPath is asserted directly in zodHelpers.test.ts, which imports
* SafePath.mjs and checks it against every safePath() test case; the only
* remaining difference from that reference is deliberate: BADCHAR_RX's
* isCleanPath is asserted in web's SafePath.test.mjs, which imports
* zz.safePath() and checks it against every isCleanPath test case; the
* only remaining difference from that reference is deliberate: BADCHAR_RX's
* leading "/" is not ported here, since "/" is the segment separator,
* not a disallowed character, so a traversal payload is still caught
* (as "." or "..") per split segment below, just not via this character
@@ -11,6 +11,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
import { assert, beforeEach, describe, expect, it } from 'vitest'
import { zz } from '@overleaf/validation-tools'
import sinon from 'sinon'
const modulePath = '../../../../app/src/Features/Project/SafePath'
@@ -124,84 +125,140 @@ describe('SafePath', function () {
})
describe('isCleanPath', function () {
// Cross-checked against zz.safePath() (libraries/validation-tools/
// zodHelpers.js), which validates project doc/file paths for
// document-updater -- every case here is expected to agree with it, so
// the two validators can't silently drift apart again.
function checkIsCleanPath(ctx, inputPath, expected) {
const result = ctx.SafePath.isCleanPath(inputPath)
expect(zz.safePath().safeParse(inputPath).success).toBe(expected)
return result.should.equal(expected)
}
it('should not accept an empty path', function (ctx) {
return checkIsCleanPath(ctx, '', false)
})
it('should accept a valid filename "main.tex"', function (ctx) {
const result = ctx.SafePath.isCleanPath('main.tex')
return result.should.equal(true)
return checkIsCleanPath(ctx, 'main.tex', true)
})
it('should accept a valid path "foo/main.tex"', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo/main.tex')
return result.should.equal(true)
return checkIsCleanPath(ctx, 'foo/main.tex', true)
})
it('should accept empty path elements', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo//main.tex')
return result.should.equal(true)
return checkIsCleanPath(ctx, 'foo//main.tex', true)
})
it('should not accept an empty filename', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo/bar/')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'foo/bar/', false)
})
it('should accept a path that starts with a slash', function (ctx) {
const result = ctx.SafePath.isCleanPath('/etc/passwd')
return result.should.equal(true)
return checkIsCleanPath(ctx, '/etc/passwd', true)
})
it('should not accept path traversal', function (ctx) {
return checkIsCleanPath(ctx, '../output.pdf', false)
})
it('should not accept a path that is exactly "."', function (ctx) {
return checkIsCleanPath(ctx, '.', false)
})
it('should not accept a path segment with leading whitespace', function (ctx) {
return checkIsCleanPath(ctx, 'foo/ bar.tex', false)
})
it('should not accept a path segment with trailing whitespace', function (ctx) {
return checkIsCleanPath(ctx, 'foo/bar.tex ', false)
})
it('should not accept a path containing a null byte', function (ctx) {
return checkIsCleanPath(
ctx,
'foo/' + String.fromCharCode(0) + '.tex',
false
)
})
it('should not accept a path containing a C1 control character', function (ctx) {
// \x80-\x9F, per BADCHAR_RX
return checkIsCleanPath(
ctx,
'foo/' + String.fromCharCode(0x90) + '.tex',
false
)
})
it('should not accept a path containing a lone surrogate', function (ctx) {
// \uD800-\uDFFF, per BADCHAR_RX
return checkIsCleanPath(ctx, 'foo/\uD800.tex', false)
})
it('should not accept a path containing a backslash', function (ctx) {
return checkIsCleanPath(ctx, 'foo/bar\\baz.tex', false)
})
it('should not accept a path that has an asterisk as the 0th element', function (ctx) {
const result = ctx.SafePath.isCleanPath('*/foo/bar')
return result.should.equal(false)
return checkIsCleanPath(ctx, '*/foo/bar', false)
})
it('should not accept a path that has an asterisk as a middle element', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo/*/bar')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'foo/*/bar', false)
})
it('should not accept a path that has an asterisk as the filename', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo/bar/*')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'foo/bar/*', false)
})
it('should not accept a path that contains an asterisk in the 0th element', function (ctx) {
const result = ctx.SafePath.isCleanPath('f*o/bar/baz')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'f*o/bar/baz', false)
})
it('should not accept a path that contains an asterisk in a middle element', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo/b*r/baz')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'foo/b*r/baz', false)
})
it('should not accept a path that contains an asterisk in the filename', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo/bar/b*z')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'foo/bar/b*z', false)
})
it('should not accept multiple problematic elements', function (ctx) {
const result = ctx.SafePath.isCleanPath('f*o/b*r/b*z')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'f*o/b*r/b*z', false)
})
it('should not accept a problematic path with an empty element', function (ctx) {
const result = ctx.SafePath.isCleanPath('foo//*/bar')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'foo//*/bar', false)
})
it('should not accept javascript property names', function (ctx) {
const result = ctx.SafePath.isCleanPath('prototype')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'prototype', false)
})
it('should not accept a path that is exactly "constructor"', function (ctx) {
return checkIsCleanPath(ctx, 'constructor', false)
})
it('should not accept javascript property names in the prototype', function (ctx) {
const result = ctx.SafePath.isCleanPath('hasOwnProperty')
return result.should.equal(false)
return checkIsCleanPath(ctx, 'hasOwnProperty', false)
})
it('should not accept javascript property names resulting from substitutions', function (ctx) {
const result = ctx.SafePath.isCleanPath(' proto ')
return result.should.equal(false)
return checkIsCleanPath(ctx, ' proto ', false)
})
it('should accept a nested javascript property name (only the top-level path is checked)', function (ctx) {
return checkIsCleanPath(ctx, 'foo/prototype', true)
})
it('should accept a __proto__ path segment as a non-empty folder', function (ctx) {
return checkIsCleanPath(ctx, '__proto__/output.pdf', true)
})
it('should accept a nested segment matching a built-in Object.prototype method name', function (ctx) {
return checkIsCleanPath(ctx, 'foo/toString', true)
})
})