From 3d05a1c31bb9ac817ed782aed3cff6c39549a595 Mon Sep 17 00:00:00 2001 From: Daniel Kennedy Date: Mon, 26 Jan 2026 14:48:39 -0500 Subject: [PATCH] Prevent path traversal attacks --- .../__tests__/download-artifact.test.ts | 79 +++++++++++++++++++ .../internal/download/download-artifact.ts | 4 +- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index 82c6d740..db7e0df3 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -841,5 +841,84 @@ describe('download-artifact', () => { const originalZipContent = fs.readFileSync(fixtures.exampleArtifact.path) expect(savedContent).toEqual(originalZipContent) }) + + it('should sanitize path traversal attempts in Content-Disposition filename', async () => { + const rawFileContent = 'malicious content' + const maliciousFileName = '../../../etc/passwd' + + const mockGetMaliciousFile = jest.fn(() => { + const message = new http.IncomingMessage(new net.Socket()) + message.statusCode = 200 + message.headers['content-type'] = 'text/plain' + message.headers['content-disposition'] = `attachment; filename="${maliciousFileName}"` + message.push(Buffer.from(rawFileContent)) + message.push(null) + return { + message + } + }) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetMaliciousFile + } + } + ) + + await streamExtractExternal( + fixtures.blobStorageUrl, + fixtures.workspaceDir + ) + + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + // Verify file was saved with sanitized name (just 'passwd', not the full path) + const sanitizedFileName = 'passwd' + const savedFilePath = path.join(fixtures.workspaceDir, sanitizedFileName) + expect(fs.existsSync(savedFilePath)).toBe(true) + expect(fs.readFileSync(savedFilePath, 'utf8')).toBe(rawFileContent) + + // Verify the file was NOT written outside the workspace directory + const maliciousPath = path.resolve(fixtures.workspaceDir, maliciousFileName) + expect(fs.existsSync(maliciousPath)).toBe(false) + }) + + it('should handle encoded path traversal attempts in Content-Disposition filename', async () => { + const rawFileContent = 'encoded malicious content' + // URL encoded version of ../../../etc/passwd + const encodedMaliciousFileName = '..%2F..%2F..%2Fetc%2Fpasswd' + + const mockGetEncodedMaliciousFile = jest.fn(() => { + const message = new http.IncomingMessage(new net.Socket()) + message.statusCode = 200 + message.headers['content-type'] = 'application/octet-stream' + message.headers['content-disposition'] = `attachment; filename="${encodedMaliciousFileName}"` + message.push(Buffer.from(rawFileContent)) + message.push(null) + return { + message + } + }) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetEncodedMaliciousFile + } + } + ) + + await streamExtractExternal( + fixtures.blobStorageUrl, + fixtures.workspaceDir + ) + + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + // After decoding and sanitizing, should just be 'passwd' + const sanitizedFileName = 'passwd' + const savedFilePath = path.join(fixtures.workspaceDir, sanitizedFileName) + expect(fs.existsSync(savedFilePath)).toBe(true) + expect(fs.readFileSync(savedFilePath, 'utf8')).toBe(rawFileContent) + }) }) }) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 173c0b57..8ebe66df 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -93,7 +93,9 @@ export async function streamExtractExternal( /filename\*?=['"]?(?:UTF-\d['"]*)?([^;\r\n"']*)['"]?/i ) if (filenameMatch && filenameMatch[1]) { - fileName = decodeURIComponent(filenameMatch[1].trim()) + // Sanitize fileName to prevent path traversal attacks + // Use path.basename to extract only the filename component + fileName = path.basename(decodeURIComponent(filenameMatch[1].trim())) } core.debug(`Content-Type: ${contentType}, isZip: ${isZip}, skipDecompress: ${skipDecompress}`)