diff --git a/packages/core/src/tools/record-artifact.test.ts b/packages/core/src/tools/record-artifact.test.ts index d785d23e46..d8be93fca6 100644 --- a/packages/core/src/tools/record-artifact.test.ts +++ b/packages/core/src/tools/record-artifact.test.ts @@ -93,15 +93,58 @@ describe('RecordArtifactTool', () => { ).toThrow(/exactly one/); }); - it('rejects workspace traversal and unsafe urls before reporting success', () => { + it('rejects workspace paths that escape the workspace', () => { const tool = new RecordArtifactTool(); - expect(() => - tool.build({ - title: 'Escape', - workspacePath: '../secret.txt', - }), - ).toThrow(/workspacePath/); + for (const workspacePath of [ + '../secret.txt', + '..\\secret.txt', + '..\\..\\secret.txt', + 'reports\\..\\..\\secret.txt', + 'reports/..\\..\\secret.txt', + 'C:\\tmp\\report.html', + 'C:/tmp/report.html', + 'C:tmp\\report.html', + '\\\\server\\share\\report.html', + '\\tmp\\report.html', + ]) { + expect(() => + tool.build({ + title: 'Escape', + workspacePath, + }), + ).toThrow(/workspacePath/); + } + }); + + it('accepts safe workspace-relative artifact paths', async () => { + const tool = new RecordArtifactTool(); + + await expect( + tool + .build({ + title: 'Safe report', + workspacePath: 'reports/summary.html', + }) + .execute(signal), + ).resolves.toMatchObject({ + artifacts: [{ workspacePath: 'reports/summary.html' }], + }); + + await expect( + tool + .build({ + title: 'Windows-style relative report', + workspacePath: 'reports\\summary.html', + }) + .execute(signal), + ).resolves.toMatchObject({ + artifacts: [{ workspacePath: 'reports\\summary.html' }], + }); + }); + + it('rejects unsafe urls before reporting success', () => { + const tool = new RecordArtifactTool(); expect(() => tool.build({ diff --git a/packages/core/src/tools/record-artifact.ts b/packages/core/src/tools/record-artifact.ts index f1ebf43a5d..3e627e620a 100644 --- a/packages/core/src/tools/record-artifact.ts +++ b/packages/core/src/tools/record-artifact.ts @@ -360,14 +360,18 @@ function validateWorkspacePath(value: string): string | null { if (stringError) { return stringError; } - if (path.isAbsolute(trimmed)) { + if ( + path.isAbsolute(trimmed) || + path.win32.isAbsolute(trimmed) || + /^[A-Za-z]:/.test(trimmed) + ) { return '"workspacePath" must be relative to the workspace'; } - const normalized = path.normalize(trimmed); + const portableNormalized = path.posix.normalize(trimmed.replace(/\\/g, '/')); if ( - normalized === '..' || - normalized.startsWith(`..${path.sep}`) || - path.isAbsolute(normalized) + portableNormalized === '..' || + portableNormalized.startsWith('../') || + path.posix.isAbsolute(portableNormalized) ) { return '"workspacePath" must stay inside the workspace'; }