🧯 fix: Harden Code Env Filepath Uploads (#12936)

* fix: Harden code env filepath uploads

* test: Cover code env filepath edge cases

* fix: Scrub code env fallback filenames
This commit is contained in:
Danny Avila 2026-05-03 21:25:59 -04:00 committed by GitHub
parent 41a6d6d11c
commit 5013d6d35c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 84 additions and 4 deletions

View file

@ -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' });
});
});

View file

@ -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 };