From 5013d6d35c83d86356d914a78a8491e0a8028cd0 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 3 May 2026 21:25:59 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=AF=20fix:=20Harden=20Code=20Env=20Fil?= =?UTF-8?q?epath=20Uploads=20(#12936)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Harden code env filepath uploads * test: Cover code env filepath edge cases * fix: Scrub code env fallback filenames --- packages/api/src/files/code/form.spec.ts | 50 ++++++++++++++++++++++++ packages/api/src/files/code/form.ts | 38 ++++++++++++++++-- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/packages/api/src/files/code/form.spec.ts b/packages/api/src/files/code/form.spec.ts index 2bc572ab54..14f8d577b9 100644 --- a/packages/api/src/files/code/form.spec.ts +++ b/packages/api/src/files/code/form.spec.ts @@ -31,6 +31,13 @@ describe('code env FormData filenames', () => { expect(disposition).toContain('filename="pptx/pptx.py"'); }); + it('uses filepath for deeply nested safe filenames', () => { + expect(getCodeEnvFileOptions('a/b/c/d.py')).toEqual({ + filename: 'd.py', + filepath: 'a/b/c/d.py', + }); + }); + it('documents the form-data string overload regression', async () => { const disposition = await renderMultipartDisposition((form) => { form.append('file', Readable.from(['x']), 'pptx/pptx.py'); @@ -49,4 +56,47 @@ describe('code env FormData filenames', () => { filepath: 'pptx/pptx.py', }); }); + + it('does not preserve traversal paths as filepath options', async () => { + expect(getCodeEnvFileOptions('../../evil.py')).toEqual({ filename: 'evil.py' }); + expect(getCodeEnvFileOptions('..\\..\\evil.py')).toEqual({ filename: 'evil.py' }); + + const disposition = await renderMultipartDisposition((form) => { + appendCodeEnvFile(form, Readable.from(['x']), '../../evil.py'); + }); + + expect(disposition).toContain('filename="evil.py"'); + expect(disposition).not.toContain('../'); + }); + + it('does not preserve absolute paths as filepath options', () => { + expect(getCodeEnvFileOptions('/tmp/evil.py')).toEqual({ filename: 'evil.py' }); + expect(getCodeEnvFileOptions('C:\\tmp\\evil.py')).toEqual({ filename: 'evil.py' }); + }); + + it('does not preserve empty or dot path segments as filepath options', () => { + expect(getCodeEnvFileOptions('pptx//pptx.py')).toEqual({ filename: 'pptx.py' }); + expect(getCodeEnvFileOptions('pptx/./pptx.py')).toEqual({ filename: 'pptx.py' }); + expect(getCodeEnvFileOptions('pptx/..')).toEqual({ filename: 'file' }); + expect(getCodeEnvFileOptions('pptx/')).toEqual({ filename: 'pptx' }); + }); + + it('uses safe fallback names for empty or degenerate filenames', () => { + expect(getCodeEnvFileOptions('')).toEqual({ filename: 'file' }); + expect(getCodeEnvFileOptions('.')).toEqual({ filename: 'file' }); + expect(getCodeEnvFileOptions('..')).toEqual({ filename: 'file' }); + }); + + it('keeps unsafe flat filenames flat instead of rewriting user-visible names', () => { + expect(getCodeEnvFileOptions('my notes @ draft.py')).toEqual({ + filename: 'my notes @ draft.py', + }); + }); + + it('scrubs control characters from fallback filenames', () => { + expect(getCodeEnvFileOptions('report\nContent-Disposition.py')).toEqual({ + filename: 'report_Content-Disposition.py', + }); + expect(getCodeEnvFileOptions('../evil\r.py')).toEqual({ filename: 'evil_.py' }); + }); }); diff --git a/packages/api/src/files/code/form.ts b/packages/api/src/files/code/form.ts index 38f1f05c45..1565721b64 100644 --- a/packages/api/src/files/code/form.ts +++ b/packages/api/src/files/code/form.ts @@ -7,16 +7,46 @@ export interface CodeEnvFileOptions { filepath?: string; } +const CODE_ENV_SAFE_FILEPATH_PATTERN = /^[a-zA-Z0-9._\-/]+$/; +const CODE_ENV_FILENAME_CONTROL_CHARS_PATTERN = /[\x00-\x1f\x7f]/g; + +function isSafeCodeEnvFilepath(filepath: string): boolean { + if (!filepath || filepath.startsWith('/') || !CODE_ENV_SAFE_FILEPATH_PATTERN.test(filepath)) { + return false; + } + + const segments = filepath.split('/'); + return segments.every((segment) => segment !== '' && segment !== '.' && segment !== '..'); +} + +function getCodeEnvBasename(filepath: string): string { + const basename = getSafeCodeEnvFilename(path.posix.basename(filepath)); + + if (!basename || basename === '.' || basename === '..') { + return 'file'; + } + + return basename; +} + +function getSafeCodeEnvFilename(filename: string): string { + return filename.replace(CODE_ENV_FILENAME_CONTROL_CHARS_PATTERN, '_'); +} + /** * Uses `filepath` for nested names because `form-data` strips directories from - * the bare string filename overload before codeapi can preserve them. + * the bare string filename overload before codeapi can preserve safe paths. */ export function getCodeEnvFileOptions(filename: string): CodeEnvFileOptions { const normalized = filename.replace(/\\/g, '/'); - const basename = path.posix.basename(normalized); + const basename = getCodeEnvBasename(normalized); - if (normalized === basename) { - return { filename }; + if (normalized === filename && filename === basename) { + return { filename: getSafeCodeEnvFilename(filename) }; + } + + if (!isSafeCodeEnvFilepath(normalized)) { + return { filename: basename }; } return { filename: basename, filepath: normalized };