From da266fa6fd965cc8cacd0a75517b32df049ae304 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 17 Sep 2026 15:27:15 +0200 Subject: [PATCH 1/6] fix(cache): isolate Windows BSD tar temporary files Run Windows BSD tar/zstd extraction and listing in private temporary directories. Concurrent operations previously overwrote cache.tar in the process working directory and could extract the wrong archive without reporting an error. Debug listings during cache saves have the same race. Resolve and quote archive and workspace paths before changing the child working directory, and remove scratch files on success and failure. Leave GNU tar, gzip, and the cache archive format unchanged. Add deterministic regression tests for concurrent reads, relative paths with spaces, and cleanup after failures in either subprocess. --- __tests__/tar-concurrency.test.ts | 219 ++++++++++++++++++++++++++++++ __tests__/tar.test.ts | 20 +-- src/internal/tar.ts | 71 ++++++---- 3 files changed, 278 insertions(+), 32 deletions(-) create mode 100644 __tests__/tar-concurrency.test.ts diff --git a/__tests__/tar-concurrency.test.ts b/__tests__/tar-concurrency.test.ts new file mode 100644 index 0000000..1e061e7 --- /dev/null +++ b/__tests__/tar-concurrency.test.ts @@ -0,0 +1,219 @@ +import * as exec from '@actions/exec' +import * as io from '@actions/io' +import * as fs from 'fs' +import * as path from 'path' +import * as tar from '../src/internal/tar' +import * as utils from '../src/internal/cacheUtils' +import {CompressionMethod} from '../src/internal/constants' + +jest.mock('@actions/exec') +jest.mock('@actions/io') + +const describeWindows = process.platform === 'win32' ? describe : describe.skip +type Operation = 'extract' | 'list' + +interface Fixture { + file: string + content: string +} + +describeWindows.each([ + CompressionMethod.Zstd, + CompressionMethod.ZstdWithoutLong +])('Windows BSD tar isolation (%s)', compression => { + const fixtures: Fixture[] = [ + {file: 'first.txt', content: 'first archive'}, + {file: 'second.txt', content: 'second archive'} + ] + const originalWorkspace = process.env['GITHUB_WORKSPACE'] + let root: string + let workspace: string + let archives: string[] + let listedFiles: string[] + let failStage: 'decompress' | 'tar' | undefined + let afterDecompression: () => Promise + + beforeEach(async () => { + jest.restoreAllMocks() + const tempRoot = path.join(__dirname, '_temp') + await fs.promises.mkdir(tempRoot, {recursive: true}) + root = await fs.promises.mkdtemp(path.join(tempRoot, 'tar-concurrency-')) + workspace = path.join(root, 'workspace') + const archiveDirectory = path.join(root, 'archives') + await fs.promises.mkdir(workspace) + await fs.promises.mkdir(archiveDirectory) + archives = fixtures.map((fixture, index) => { + const archive = path.join(archiveDirectory, `${index}.tzst`) + fs.writeFileSync(archive, JSON.stringify(fixture)) + return archive + }) + fs.writeFileSync(path.join(root, 'cache.tar'), 'unrelated existing file') + listedFiles = [] + failStage = undefined + afterDecompression = async () => {} + process.env['GITHUB_WORKSPACE'] = workspace + jest.spyOn(utils, 'getGnuTarPathOnWindows').mockResolvedValue('') + jest.spyOn(utils, 'createTempDirectory').mockImplementation(async () => { + return fs.promises.mkdtemp(path.join(root, 'scratch-')) + }) + jest.spyOn(io, 'mkdirP').mockImplementation(async directory => { + await fs.promises.mkdir(path.resolve(directory), {recursive: true}) + }) + jest.spyOn(io, 'rmRF').mockImplementation(async directory => { + await fs.promises.rm(directory, {recursive: true, force: true}) + }) + + // Model the filesystem effects of zstd and tar without requiring either + // program to be installed. The fixture content stands in for archive data. + jest + .spyOn(exec, 'exec') + .mockImplementation(async (command, args, options) => { + const argv = (command.match(/"[^"]*"|\S+/g) || []).map(argument => + argument.replace(/^"|"$/g, '') + ) + argv.push(...(args || [])) + // Redirect the default command cwd to avoid writing into the checkout. + const cwd = options?.cwd || root + if (argv[0] === 'zstd') { + const source = path.resolve(cwd, argv[argv.length - 1]) + const output = path.resolve(cwd, argv[argv.indexOf('-o') + 1]) + fs.copyFileSync(source, output) + if (failStage === 'decompress') { + throw new Error('simulated decompression failure') + } + await afterDecompression() + } else { + if (failStage === 'tar') { + throw new Error('simulated tar failure') + } + const extracting = argv.includes('-xf') + const source = path.resolve( + cwd, + argv[argv.indexOf(extracting ? '-xf' : '-tf') + 1] + ) + const fixture: Fixture = JSON.parse(fs.readFileSync(source, 'utf8')) + if (extracting) { + const destination = path.resolve(cwd, argv[argv.indexOf('-C') + 1]) + await fs.promises.mkdir(destination, {recursive: true}) + fs.writeFileSync( + path.join(destination, fixture.file), + fixture.content + ) + } else { + listedFiles.push(fixture.file) + } + } + return 0 + }) + }) + + afterEach(async () => { + jest.restoreAllMocks() + if (originalWorkspace === undefined) { + delete process.env['GITHUB_WORKSPACE'] + } else { + process.env['GITHUB_WORKSPACE'] = originalWorkspace + } + await fs.promises.rm(root, {recursive: true, force: true}) + }) + + async function readArchive( + operation: Operation, + archive: string + ): Promise { + if (operation === 'extract') { + await tar.extractTar(archive, compression) + } else { + await tar.listTar(archive, compression) + } + } + + function expectCleanScratch(): void { + expect(fs.readdirSync(root).sort()).toEqual([ + 'archives', + 'cache.tar', + 'workspace' + ]) + expect(fs.readFileSync(path.join(root, 'cache.tar'), 'utf8')).toBe( + 'unrelated existing file' + ) + for (const [index, archive] of archives.entries()) { + expect(JSON.parse(fs.readFileSync(archive, 'utf8'))).toEqual( + fixtures[index] + ) + } + } + + test.each<[Operation, Operation]>([ + ['extract', 'extract'], + ['list', 'list'], + ['extract', 'list'] + ])('keeps concurrent %s and %s results separate', async (first, second) => { + let arrivals = 0 + let release: () => void = () => {} + const bothDecompressed = new Promise(resolve => { + release = resolve + }) + afterDecompression = async () => { + if (++arrivals === 2) { + release() + } + await bothDecompressed + } + + // Both inputs share a parent directory. Force their decompressions to + // finish before either tar reads, reproducing the old shared-file race. + const operations = [first, second] + await Promise.all( + operations.map(async (operation, index) => { + await readArchive(operation, archives[index]) + }) + ) + + expect(fs.readdirSync(workspace).sort()).toEqual( + fixtures + .filter((_, index) => operations[index] === 'extract') + .map(f => f.file) + ) + expect(listedFiles.sort()).toEqual( + fixtures + .filter((_, index) => operations[index] === 'list') + .map(f => f.file) + ) + expectCleanScratch() + }) + + test('preserves relative archive and workspace paths containing spaces', async () => { + const archive = path.join(root, 'archives', 'archive with spaces.tar') + fs.renameSync(archives[0], archive) + archives[0] = archive + const relativeArchive = path.relative(process.cwd(), archive) + const destination = path.join(workspace, 'directory with spaces') + process.env['GITHUB_WORKSPACE'] = path.relative(process.cwd(), destination) + + await tar.extractTar(relativeArchive, compression) + + expect( + fs.readFileSync(path.join(destination, fixtures[0].file), 'utf8') + ).toBe(fixtures[0].content) + expectCleanScratch() + }) + + test.each<[Operation, 'decompress' | 'tar']>([ + ['extract', 'decompress'], + ['extract', 'tar'], + ['list', 'decompress'], + ['list', 'tar'] + ])( + 'cleans temporary files when %s fails during %s', + async (operation, stage) => { + failStage = stage + + await expect(readArchive(operation, archives[0])).rejects.toThrow( + 'simulated' + ) + + expectCleanScratch() + } + ) +}) diff --git a/__tests__/tar.test.ts b/__tests__/tar.test.ts index 4145d9a..2788b34 100644 --- a/__tests__/tar.test.ts +++ b/__tests__/tar.test.ts @@ -88,6 +88,8 @@ test('zstd extract tar with windows BSDtar', async () => { if (IS_WINDOWS) { const mkdirMock = jest.spyOn(io, 'mkdirP') const execMock = jest.spyOn(exec, 'exec') + const tempDirectory = getTempDir() + jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory) jest .spyOn(utils, 'getGnuTarPathOnWindows') .mockReturnValue(Promise.resolve('')) @@ -106,11 +108,11 @@ test('zstd extract tar with windows BSDtar', async () => { [ 'zstd -d --long=30 --force -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"` ].join(' '), undefined, { - cwd: undefined, + cwd: tempDirectory, env: expect.objectContaining(defaultEnv) } ) @@ -123,11 +125,11 @@ test('zstd extract tar with windows BSDtar', async () => { TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', - workspace?.replace(/\\/g, '/') + `"${workspace?.replace(/\\/g, '/')}"` ].join(' '), undefined, { - cwd: undefined, + cwd: tempDirectory, env: expect.objectContaining(defaultEnv) } ) @@ -278,7 +280,7 @@ test('zstd create tar with windows BSDtar', async () => { TarFilename.replace(/\\/g, '/'), '-P', '-C', - workspace?.replace(/\\/g, '/'), + `"${workspace?.replace(/\\/g, '/')}"`, '--files-from', ManifestFilename ].join(' '), @@ -381,6 +383,8 @@ test('zstd list tar', async () => { test('zstd list tar with windows BSDtar', async () => { if (IS_WINDOWS) { const execMock = jest.spyOn(exec, 'exec') + const tempDirectory = getTempDir() + jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory) jest .spyOn(utils, 'getGnuTarPathOnWindows') .mockReturnValue(Promise.resolve('')) @@ -396,11 +400,11 @@ test('zstd list tar with windows BSDtar', async () => { [ 'zstd -d --long=30 --force -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"` ].join(' '), undefined, { - cwd: undefined, + cwd: tempDirectory, env: expect.objectContaining(defaultEnv) } ) @@ -415,7 +419,7 @@ test('zstd list tar with windows BSDtar', async () => { ].join(' '), undefined, { - cwd: undefined, + cwd: tempDirectory, env: expect.objectContaining(defaultEnv) } ) diff --git a/src/internal/tar.ts b/src/internal/tar.ts index 73c126c..30bc044 100644 --- a/src/internal/tar.ts +++ b/src/internal/tar.ts @@ -14,6 +14,11 @@ import { const IS_WINDOWS = process.platform === 'win32' +interface TarCommands { + commands: string[] + requiresTempDirectory: boolean +} + // Returns tar path and type: BSD or GNU async function getTarPath(): Promise { switch (process.platform) { @@ -82,7 +87,9 @@ async function getTarArgs( : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', - workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + BSD_TAR_ZSTD + ? quoteAbsolutePath(workingDirectory) + : workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', ManifestFilename ) @@ -95,7 +102,9 @@ async function getTarArgs( : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', - workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + BSD_TAR_ZSTD + ? quoteAbsolutePath(workingDirectory) + : workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ) break case 'list': @@ -129,7 +138,7 @@ async function getCommands( compressionMethod: CompressionMethod, type: string, archivePath = '' -): Promise { +): Promise { let args const tarPath = await getTarPath() @@ -154,17 +163,20 @@ async function getCommands( args = [[...tarArgs].join(' '), [...compressionArgs].join(' ')] } - if (BSD_TAR_ZSTD) { - return args + return { + commands: BSD_TAR_ZSTD ? args : [args.join(' ')], + requiresTempDirectory: BSD_TAR_ZSTD && type !== 'create' } - - return [args.join(' ')] } function getWorkingDirectory(): string { return process.env['GITHUB_WORKSPACE'] ?? process.cwd() } +function quoteAbsolutePath(filePath: string): string { + return `"${path.resolve(filePath).replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"` +} + // Common function for extractTar and listTar to get the compression method async function getDecompressionProgram( tarPath: ArchiveTool, @@ -185,7 +197,7 @@ async function getDecompressionProgram( ? [ 'zstd -d --long=30 --force -o', TarFilename, - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + quoteAbsolutePath(archivePath) ] : [ '--use-compress-program', @@ -193,11 +205,7 @@ async function getDecompressionProgram( ] case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD - ? [ - 'zstd -d --force -o', - TarFilename, - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') - ] + ? ['zstd -d --force -o', TarFilename, quoteAbsolutePath(archivePath)] : ['--use-compress-program', IS_WINDOWS ? '"zstd -d"' : 'unzstd'] default: return ['-z'] @@ -245,17 +253,32 @@ async function getCompressionProgram( } // Executes all commands as separate processes -async function execCommands(commands: string[], cwd?: string): Promise { - for (const command of commands) { - try { - await exec(command, undefined, { - cwd, - env: {...(process.env as object), MSYS: 'winsymlinks:nativestrict'} - }) - } catch (error) { - throw new Error( - `${command.split(' ')[0]} failed with error: ${error?.message}` - ) +async function execCommands( + {commands, requiresTempDirectory}: TarCommands, + cwd?: string +): Promise { + // The Windows BSD-tar fallback decompresses into cache.tar before reading + // it. Isolate that intermediate for each list/extract operation, including + // debug listings during saves. Creation already has its own archive folder. + const tempDirectory = requiresTempDirectory + ? await utils.createTempDirectory() + : undefined + try { + for (const command of commands) { + try { + await exec(command, undefined, { + cwd: tempDirectory ?? cwd, + env: {...(process.env as object), MSYS: 'winsymlinks:nativestrict'} + }) + } catch (error) { + throw new Error( + `${command.split(' ')[0]} failed with error: ${error?.message}` + ) + } + } + } finally { + if (tempDirectory) { + await io.rmRF(tempDirectory) } } } From 90977320aa367bc02a61113ad529bd9df3dbb700 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 17 Sep 2026 15:36:35 +0200 Subject: [PATCH 2/6] test(cache): align tar concurrency test filename --- __tests__/{tar-concurrency.test.ts => tarConcurrency.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename __tests__/{tar-concurrency.test.ts => tarConcurrency.test.ts} (100%) diff --git a/__tests__/tar-concurrency.test.ts b/__tests__/tarConcurrency.test.ts similarity index 100% rename from __tests__/tar-concurrency.test.ts rename to __tests__/tarConcurrency.test.ts From dd137a579c6ffeea43f52a79b76619a6ed910167 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 17 Sep 2026 15:47:36 +0200 Subject: [PATCH 3/6] refactor(cache): consolidate tar fallback handling and tests Share the Windows BSD tar/zstd predicate and prepare archive and workspace arguments once per command. Parameterize duplicate BSD-tar read tests and use the real temporary-directory utilities in the concurrency fixture. Retain the existing concurrency, path-handling, and failure coverage. --- __tests__/tar.test.ts | 103 +++++++++---------------------- __tests__/tarConcurrency.test.ts | 85 ++++++++++--------------- src/internal/tar.ts | 76 +++++++++-------------- 3 files changed, 89 insertions(+), 175 deletions(-) diff --git a/__tests__/tar.test.ts b/__tests__/tar.test.ts index 2788b34..f85a51d 100644 --- a/__tests__/tar.test.ts +++ b/__tests__/tar.test.ts @@ -84,57 +84,56 @@ test('zstd extract tar', async () => { ) }) -test('zstd extract tar with windows BSDtar', async () => { - if (IS_WINDOWS) { +test.each([ + {operation: 'extract', read: tar.extractTar, flag: '-xf'}, + {operation: 'list', read: tar.listTar, flag: '-tf'} +])( + 'zstd $operation tar with windows BSDtar', + async ({operation, read, flag}) => { + if (!IS_WINDOWS) { + return + } const mkdirMock = jest.spyOn(io, 'mkdirP') const execMock = jest.spyOn(exec, 'exec') const tempDirectory = getTempDir() jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory) - jest - .spyOn(utils, 'getGnuTarPathOnWindows') - .mockReturnValue(Promise.resolve('')) + jest.spyOn(utils, 'getGnuTarPathOnWindows').mockResolvedValue('') const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` const workspace = process.env['GITHUB_WORKSPACE'] - const tarPath = SystemTarPathOnWindows + const options = { + cwd: tempDirectory, + env: expect.objectContaining(defaultEnv) + } + const tarArgs = [`"${SystemTarPathOnWindows}"`, flag, TarFilename, '-P'] + if (operation === 'extract') { + tarArgs.push('-C', `"${workspace?.replace(/\\/g, '/')}"`) + } - await tar.extractTar(archivePath, CompressionMethod.Zstd) + await read(archivePath, CompressionMethod.Zstd) - expect(mkdirMock).toHaveBeenCalledWith(workspace) + if (operation === 'extract') { + expect(mkdirMock).toHaveBeenCalledWith(workspace) + } expect(execMock).toHaveBeenCalledTimes(2) - expect(execMock).toHaveBeenNthCalledWith( 1, [ 'zstd -d --long=30 --force -o', - TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"` + TarFilename, + `"${archivePath.replace(/\\/g, '/')}"` ].join(' '), undefined, - { - cwd: tempDirectory, - env: expect.objectContaining(defaultEnv) - } + options ) - expect(execMock).toHaveBeenNthCalledWith( 2, - [ - `"${tarPath}"`, - '-xf', - TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P', - '-C', - `"${workspace?.replace(/\\/g, '/')}"` - ].join(' '), + tarArgs.join(' '), undefined, - { - cwd: tempDirectory, - env: expect.objectContaining(defaultEnv) - } + options ) } -}) +) test('gzip extract tar', async () => { const mkdirMock = jest.spyOn(io, 'mkdirP') @@ -380,52 +379,6 @@ test('zstd list tar', async () => { ) }) -test('zstd list tar with windows BSDtar', async () => { - if (IS_WINDOWS) { - const execMock = jest.spyOn(exec, 'exec') - const tempDirectory = getTempDir() - jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory) - jest - .spyOn(utils, 'getGnuTarPathOnWindows') - .mockReturnValue(Promise.resolve('')) - const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` - - await tar.listTar(archivePath, CompressionMethod.Zstd) - - const tarPath = SystemTarPathOnWindows - expect(execMock).toHaveBeenCalledTimes(2) - - expect(execMock).toHaveBeenNthCalledWith( - 1, - [ - 'zstd -d --long=30 --force -o', - TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"` - ].join(' '), - undefined, - { - cwd: tempDirectory, - env: expect.objectContaining(defaultEnv) - } - ) - - expect(execMock).toHaveBeenNthCalledWith( - 2, - [ - `"${tarPath}"`, - '-tf', - TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P' - ].join(' '), - undefined, - { - cwd: tempDirectory, - env: expect.objectContaining(defaultEnv) - } - ) - } -}) - test('zstdWithoutLong list tar', async () => { const execMock = jest.spyOn(exec, 'exec') diff --git a/__tests__/tarConcurrency.test.ts b/__tests__/tarConcurrency.test.ts index 1e061e7..0d84d89 100644 --- a/__tests__/tarConcurrency.test.ts +++ b/__tests__/tarConcurrency.test.ts @@ -1,5 +1,4 @@ import * as exec from '@actions/exec' -import * as io from '@actions/io' import * as fs from 'fs' import * as path from 'path' import * as tar from '../src/internal/tar' @@ -7,10 +6,10 @@ import * as utils from '../src/internal/cacheUtils' import {CompressionMethod} from '../src/internal/constants' jest.mock('@actions/exec') -jest.mock('@actions/io') const describeWindows = process.platform === 'win32' ? describe : describe.skip -type Operation = 'extract' | 'list' +const readers = {extract: tar.extractTar, list: tar.listTar} +type Operation = keyof typeof readers interface Fixture { file: string @@ -25,7 +24,7 @@ describeWindows.each([ {file: 'first.txt', content: 'first archive'}, {file: 'second.txt', content: 'second archive'} ] - const originalWorkspace = process.env['GITHUB_WORKSPACE'] + const originalEnv = {...process.env} let root: string let workspace: string let archives: string[] @@ -51,17 +50,12 @@ describeWindows.each([ listedFiles = [] failStage = undefined afterDecompression = async () => {} - process.env['GITHUB_WORKSPACE'] = workspace + process.env = { + ...originalEnv, + GITHUB_WORKSPACE: workspace, + RUNNER_TEMP: root + } jest.spyOn(utils, 'getGnuTarPathOnWindows').mockResolvedValue('') - jest.spyOn(utils, 'createTempDirectory').mockImplementation(async () => { - return fs.promises.mkdtemp(path.join(root, 'scratch-')) - }) - jest.spyOn(io, 'mkdirP').mockImplementation(async directory => { - await fs.promises.mkdir(path.resolve(directory), {recursive: true}) - }) - jest.spyOn(io, 'rmRF').mockImplementation(async directory => { - await fs.promises.rm(directory, {recursive: true, force: true}) - }) // Model the filesystem effects of zstd and tar without requiring either // program to be installed. The fixture content stands in for archive data. @@ -82,26 +76,26 @@ describeWindows.each([ throw new Error('simulated decompression failure') } await afterDecompression() - } else { - if (failStage === 'tar') { - throw new Error('simulated tar failure') - } - const extracting = argv.includes('-xf') - const source = path.resolve( - cwd, - argv[argv.indexOf(extracting ? '-xf' : '-tf') + 1] + return 0 + } + if (failStage === 'tar') { + throw new Error('simulated tar failure') + } + const extracting = argv.includes('-xf') + const source = path.resolve( + cwd, + argv[argv.indexOf(extracting ? '-xf' : '-tf') + 1] + ) + const fixture: Fixture = JSON.parse(fs.readFileSync(source, 'utf8')) + if (extracting) { + const destination = path.resolve(cwd, argv[argv.indexOf('-C') + 1]) + await fs.promises.mkdir(destination, {recursive: true}) + fs.writeFileSync( + path.join(destination, fixture.file), + fixture.content ) - const fixture: Fixture = JSON.parse(fs.readFileSync(source, 'utf8')) - if (extracting) { - const destination = path.resolve(cwd, argv[argv.indexOf('-C') + 1]) - await fs.promises.mkdir(destination, {recursive: true}) - fs.writeFileSync( - path.join(destination, fixture.file), - fixture.content - ) - } else { - listedFiles.push(fixture.file) - } + } else { + listedFiles.push(fixture.file) } return 0 }) @@ -109,25 +103,10 @@ describeWindows.each([ afterEach(async () => { jest.restoreAllMocks() - if (originalWorkspace === undefined) { - delete process.env['GITHUB_WORKSPACE'] - } else { - process.env['GITHUB_WORKSPACE'] = originalWorkspace - } + process.env = {...originalEnv} await fs.promises.rm(root, {recursive: true, force: true}) }) - async function readArchive( - operation: Operation, - archive: string - ): Promise { - if (operation === 'extract') { - await tar.extractTar(archive, compression) - } else { - await tar.listTar(archive, compression) - } - } - function expectCleanScratch(): void { expect(fs.readdirSync(root).sort()).toEqual([ 'archives', @@ -166,7 +145,7 @@ describeWindows.each([ const operations = [first, second] await Promise.all( operations.map(async (operation, index) => { - await readArchive(operation, archives[index]) + await readers[operation](archives[index], compression) }) ) @@ -209,9 +188,9 @@ describeWindows.each([ async (operation, stage) => { failStage = stage - await expect(readArchive(operation, archives[0])).rejects.toThrow( - 'simulated' - ) + await expect( + readers[operation](archives[0], compression) + ).rejects.toThrow('simulated') expectCleanScratch() } diff --git a/src/internal/tar.ts b/src/internal/tar.ts index 30bc044..1e80329 100644 --- a/src/internal/tar.ts +++ b/src/internal/tar.ts @@ -19,6 +19,17 @@ interface TarCommands { requiresTempDirectory: boolean } +function isBsdTarZstd( + tarPath: ArchiveTool, + compressionMethod: CompressionMethod +): boolean { + return ( + IS_WINDOWS && + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip + ) +} + // Returns tar path and type: BSD or GNU async function getTarPath(): Promise { switch (process.platform) { @@ -63,14 +74,16 @@ async function getTarArgs( archivePath = '' ): Promise { const args = [`"${tarPath.path}"`] - const cacheFileName = utils.getCacheFileName(compressionMethod) - const tarFile = 'cache.tar' - const workingDirectory = getWorkingDirectory() - // Speficic args for BSD tar on windows for workaround - const BSD_TAR_ZSTD = - tarPath.type === ArchiveToolType.BSD && - compressionMethod !== CompressionMethod.Gzip && - IS_WINDOWS + const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) + const tarFile = BSD_TAR_ZSTD + ? TarFilename + : (type === 'create' + ? utils.getCacheFileName(compressionMethod) + : archivePath + ).replace(new RegExp(`\\${path.sep}`, 'g'), '/') + const workingDirectory = BSD_TAR_ZSTD + ? quoteAbsolutePath(getWorkingDirectory()) + : getWorkingDirectory().replace(new RegExp(`\\${path.sep}`, 'g'), '/') // Method specific args switch (type) { @@ -78,43 +91,21 @@ async function getTarArgs( args.push( '--posix', '-cf', - BSD_TAR_ZSTD - ? tarFile - : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile, '--exclude', - BSD_TAR_ZSTD - ? tarFile - : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile, '-P', '-C', - BSD_TAR_ZSTD - ? quoteAbsolutePath(workingDirectory) - : workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + workingDirectory, '--files-from', ManifestFilename ) break case 'extract': - args.push( - '-xf', - BSD_TAR_ZSTD - ? tarFile - : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P', - '-C', - BSD_TAR_ZSTD - ? quoteAbsolutePath(workingDirectory) - : workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') - ) + args.push('-xf', tarFile, '-P', '-C', workingDirectory) break case 'list': - args.push( - '-tf', - BSD_TAR_ZSTD - ? tarFile - : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P' - ) + args.push('-tf', tarFile, '-P') break } @@ -152,10 +143,7 @@ async function getCommands( type !== 'create' ? await getDecompressionProgram(tarPath, compressionMethod, archivePath) : await getCompressionProgram(tarPath, compressionMethod) - const BSD_TAR_ZSTD = - tarPath.type === ArchiveToolType.BSD && - compressionMethod !== CompressionMethod.Gzip && - IS_WINDOWS + const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) if (BSD_TAR_ZSTD && type !== 'create') { args = [[...compressionArgs].join(' '), [...tarArgs].join(' ')] @@ -187,10 +175,7 @@ async function getDecompressionProgram( // unzstd is equivalent to 'zstd -d' // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit. // Using 30 here because we also support 32-bit self-hosted runners. - const BSD_TAR_ZSTD = - tarPath.type === ArchiveToolType.BSD && - compressionMethod !== CompressionMethod.Gzip && - IS_WINDOWS + const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) switch (compressionMethod) { case CompressionMethod.Zstd: return BSD_TAR_ZSTD @@ -223,10 +208,7 @@ async function getCompressionProgram( compressionMethod: CompressionMethod ): Promise { const cacheFileName = utils.getCacheFileName(compressionMethod) - const BSD_TAR_ZSTD = - tarPath.type === ArchiveToolType.BSD && - compressionMethod !== CompressionMethod.Gzip && - IS_WINDOWS + const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) switch (compressionMethod) { case CompressionMethod.Zstd: return BSD_TAR_ZSTD From fce871591384b3b26dc38d4386b762337b9c63be Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 17 Sep 2026 15:53:20 +0200 Subject: [PATCH 4/6] chore(cache): minimize tar isolation diff Restore the existing helper layout and separate test cases rather than bundling unrelated consolidation with the bug fix. Leave archive creation unchanged and retain only the read isolation, necessary path handling, and regression coverage. --- __tests__/tar.test.ts | 105 ++++++++++++++++++++++++++++++------------ src/internal/tar.ts | 74 +++++++++++++++++------------ 2 files changed, 121 insertions(+), 58 deletions(-) diff --git a/__tests__/tar.test.ts b/__tests__/tar.test.ts index f85a51d..ed4ab42 100644 --- a/__tests__/tar.test.ts +++ b/__tests__/tar.test.ts @@ -84,56 +84,57 @@ test('zstd extract tar', async () => { ) }) -test.each([ - {operation: 'extract', read: tar.extractTar, flag: '-xf'}, - {operation: 'list', read: tar.listTar, flag: '-tf'} -])( - 'zstd $operation tar with windows BSDtar', - async ({operation, read, flag}) => { - if (!IS_WINDOWS) { - return - } +test('zstd extract tar with windows BSDtar', async () => { + if (IS_WINDOWS) { const mkdirMock = jest.spyOn(io, 'mkdirP') const execMock = jest.spyOn(exec, 'exec') const tempDirectory = getTempDir() jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory) - jest.spyOn(utils, 'getGnuTarPathOnWindows').mockResolvedValue('') + jest + .spyOn(utils, 'getGnuTarPathOnWindows') + .mockReturnValue(Promise.resolve('')) const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` const workspace = process.env['GITHUB_WORKSPACE'] - const options = { - cwd: tempDirectory, - env: expect.objectContaining(defaultEnv) - } - const tarArgs = [`"${SystemTarPathOnWindows}"`, flag, TarFilename, '-P'] - if (operation === 'extract') { - tarArgs.push('-C', `"${workspace?.replace(/\\/g, '/')}"`) - } + const tarPath = SystemTarPathOnWindows - await read(archivePath, CompressionMethod.Zstd) + await tar.extractTar(archivePath, CompressionMethod.Zstd) - if (operation === 'extract') { - expect(mkdirMock).toHaveBeenCalledWith(workspace) - } + expect(mkdirMock).toHaveBeenCalledWith(workspace) expect(execMock).toHaveBeenCalledTimes(2) + expect(execMock).toHaveBeenNthCalledWith( 1, [ 'zstd -d --long=30 --force -o', - TarFilename, - `"${archivePath.replace(/\\/g, '/')}"` + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"` ].join(' '), undefined, - options + { + cwd: tempDirectory, + env: expect.objectContaining(defaultEnv) + } ) + expect(execMock).toHaveBeenNthCalledWith( 2, - tarArgs.join(' '), + [ + `"${tarPath}"`, + '-xf', + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P', + '-C', + `"${workspace?.replace(/\\/g, '/')}"` + ].join(' '), undefined, - options + { + cwd: tempDirectory, + env: expect.objectContaining(defaultEnv) + } ) } -) +}) test('gzip extract tar', async () => { const mkdirMock = jest.spyOn(io, 'mkdirP') @@ -279,7 +280,7 @@ test('zstd create tar with windows BSDtar', async () => { TarFilename.replace(/\\/g, '/'), '-P', '-C', - `"${workspace?.replace(/\\/g, '/')}"`, + workspace?.replace(/\\/g, '/'), '--files-from', ManifestFilename ].join(' '), @@ -379,6 +380,52 @@ test('zstd list tar', async () => { ) }) +test('zstd list tar with windows BSDtar', async () => { + if (IS_WINDOWS) { + const execMock = jest.spyOn(exec, 'exec') + const tempDirectory = getTempDir() + jest.spyOn(utils, 'createTempDirectory').mockResolvedValue(tempDirectory) + jest + .spyOn(utils, 'getGnuTarPathOnWindows') + .mockReturnValue(Promise.resolve('')) + const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` + + await tar.listTar(archivePath, CompressionMethod.Zstd) + + const tarPath = SystemTarPathOnWindows + expect(execMock).toHaveBeenCalledTimes(2) + + expect(execMock).toHaveBeenNthCalledWith( + 1, + [ + 'zstd -d --long=30 --force -o', + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + `"${archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/')}"` + ].join(' '), + undefined, + { + cwd: tempDirectory, + env: expect.objectContaining(defaultEnv) + } + ) + + expect(execMock).toHaveBeenNthCalledWith( + 2, + [ + `"${tarPath}"`, + '-tf', + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P' + ].join(' '), + undefined, + { + cwd: tempDirectory, + env: expect.objectContaining(defaultEnv) + } + ) + } +}) + test('zstdWithoutLong list tar', async () => { const execMock = jest.spyOn(exec, 'exec') diff --git a/src/internal/tar.ts b/src/internal/tar.ts index 1e80329..e3334df 100644 --- a/src/internal/tar.ts +++ b/src/internal/tar.ts @@ -19,17 +19,6 @@ interface TarCommands { requiresTempDirectory: boolean } -function isBsdTarZstd( - tarPath: ArchiveTool, - compressionMethod: CompressionMethod -): boolean { - return ( - IS_WINDOWS && - tarPath.type === ArchiveToolType.BSD && - compressionMethod !== CompressionMethod.Gzip - ) -} - // Returns tar path and type: BSD or GNU async function getTarPath(): Promise { switch (process.platform) { @@ -74,16 +63,14 @@ async function getTarArgs( archivePath = '' ): Promise { const args = [`"${tarPath.path}"`] - const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) - const tarFile = BSD_TAR_ZSTD - ? TarFilename - : (type === 'create' - ? utils.getCacheFileName(compressionMethod) - : archivePath - ).replace(new RegExp(`\\${path.sep}`, 'g'), '/') - const workingDirectory = BSD_TAR_ZSTD - ? quoteAbsolutePath(getWorkingDirectory()) - : getWorkingDirectory().replace(new RegExp(`\\${path.sep}`, 'g'), '/') + const cacheFileName = utils.getCacheFileName(compressionMethod) + const tarFile = 'cache.tar' + const workingDirectory = getWorkingDirectory() + // Speficic args for BSD tar on windows for workaround + const BSD_TAR_ZSTD = + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS // Method specific args switch (type) { @@ -91,21 +78,41 @@ async function getTarArgs( args.push( '--posix', '-cf', - tarFile, + BSD_TAR_ZSTD + ? tarFile + : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--exclude', - tarFile, + BSD_TAR_ZSTD + ? tarFile + : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', - workingDirectory, + workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', ManifestFilename ) break case 'extract': - args.push('-xf', tarFile, '-P', '-C', workingDirectory) + args.push( + '-xf', + BSD_TAR_ZSTD + ? tarFile + : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P', + '-C', + BSD_TAR_ZSTD + ? quoteAbsolutePath(workingDirectory) + : workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + ) break case 'list': - args.push('-tf', tarFile, '-P') + args.push( + '-tf', + BSD_TAR_ZSTD + ? tarFile + : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P' + ) break } @@ -143,7 +150,10 @@ async function getCommands( type !== 'create' ? await getDecompressionProgram(tarPath, compressionMethod, archivePath) : await getCompressionProgram(tarPath, compressionMethod) - const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) + const BSD_TAR_ZSTD = + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS if (BSD_TAR_ZSTD && type !== 'create') { args = [[...compressionArgs].join(' '), [...tarArgs].join(' ')] @@ -175,7 +185,10 @@ async function getDecompressionProgram( // unzstd is equivalent to 'zstd -d' // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit. // Using 30 here because we also support 32-bit self-hosted runners. - const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) + const BSD_TAR_ZSTD = + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS switch (compressionMethod) { case CompressionMethod.Zstd: return BSD_TAR_ZSTD @@ -208,7 +221,10 @@ async function getCompressionProgram( compressionMethod: CompressionMethod ): Promise { const cacheFileName = utils.getCacheFileName(compressionMethod) - const BSD_TAR_ZSTD = isBsdTarZstd(tarPath, compressionMethod) + const BSD_TAR_ZSTD = + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS switch (compressionMethod) { case CompressionMethod.Zstd: return BSD_TAR_ZSTD From cd705bb67f41ce0e18b78e0a8e8c50e0bbd65888 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 17 Sep 2026 16:55:25 +0200 Subject: [PATCH 5/6] refactor(cache): scope tar scratch directories to reads Keep execCommands as a plain executor that honors its cwd argument. Move scratch-directory allocation and cleanup into a shared list/extract helper, leaving archive creation on the caller-supplied directory. --- src/internal/tar.ts | 50 ++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/internal/tar.ts b/src/internal/tar.ts index e3334df..7b8d81a 100644 --- a/src/internal/tar.ts +++ b/src/internal/tar.ts @@ -251,29 +251,39 @@ async function getCompressionProgram( } // Executes all commands as separate processes -async function execCommands( - {commands, requiresTempDirectory}: TarCommands, - cwd?: string +async function execCommands(commands: string[], cwd?: string): Promise { + for (const command of commands) { + try { + await exec(command, undefined, { + cwd, + env: {...(process.env as object), MSYS: 'winsymlinks:nativestrict'} + }) + } catch (error) { + throw new Error( + `${command.split(' ')[0]} failed with error: ${error?.message}` + ) + } + } +} + +async function execReadCommands( + archivePath: string, + compressionMethod: CompressionMethod, + type: 'list' | 'extract' ): Promise { + const {commands, requiresTempDirectory} = await getCommands( + compressionMethod, + type, + archivePath + ) // The Windows BSD-tar fallback decompresses into cache.tar before reading // it. Isolate that intermediate for each list/extract operation, including - // debug listings during saves. Creation already has its own archive folder. + // debug listings during saves. const tempDirectory = requiresTempDirectory ? await utils.createTempDirectory() : undefined try { - for (const command of commands) { - try { - await exec(command, undefined, { - cwd: tempDirectory ?? cwd, - env: {...(process.env as object), MSYS: 'winsymlinks:nativestrict'} - }) - } catch (error) { - throw new Error( - `${command.split(' ')[0]} failed with error: ${error?.message}` - ) - } - } + await execCommands(commands, tempDirectory) } finally { if (tempDirectory) { await io.rmRF(tempDirectory) @@ -286,8 +296,7 @@ export async function listTar( archivePath: string, compressionMethod: CompressionMethod ): Promise { - const commands = await getCommands(compressionMethod, 'list', archivePath) - await execCommands(commands) + await execReadCommands(archivePath, compressionMethod, 'list') } // Extract a tar @@ -298,8 +307,7 @@ export async function extractTar( // Create directory to extract tar into const workingDirectory = getWorkingDirectory() await io.mkdirP(workingDirectory) - const commands = await getCommands(compressionMethod, 'extract', archivePath) - await execCommands(commands) + await execReadCommands(archivePath, compressionMethod, 'extract') } // Create a tar @@ -313,6 +321,6 @@ export async function createTar( path.join(archiveFolder, ManifestFilename), sourceDirectories.join('\n') ) - const commands = await getCommands(compressionMethod, 'create') + const {commands} = await getCommands(compressionMethod, 'create') await execCommands(commands, archiveFolder) } From 3607dcfa49c1fdb42dc14a447931c41f30b8d4e6 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 17 Sep 2026 17:04:23 +0200 Subject: [PATCH 6/6] fix(cache): make tar scratch cleanup best-effort Log scratch-directory cleanup failures without replacing a successful archive read or the original tool error. Keep scratch ownership in execReadCommands and cover both outcomes for extraction and listing. --- __tests__/tarConcurrency.test.ts | 28 ++++++++++++++++++++++++++++ src/internal/tar.ts | 7 ++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/__tests__/tarConcurrency.test.ts b/__tests__/tarConcurrency.test.ts index 0d84d89..b9ebde5 100644 --- a/__tests__/tarConcurrency.test.ts +++ b/__tests__/tarConcurrency.test.ts @@ -1,4 +1,6 @@ +import * as core from '@actions/core' import * as exec from '@actions/exec' +import * as io from '@actions/io' import * as fs from 'fs' import * as path from 'path' import * as tar from '../src/internal/tar' @@ -178,6 +180,32 @@ describeWindows.each([ expectCleanScratch() }) + test.each<[Operation, boolean]>([ + ['extract', false], + ['extract', true], + ['list', false], + ['list', true] + ])( + 'preserves the %s result when cleanup fails (command fails=%s)', + async (operation, commandFails) => { + const debugMock = jest.spyOn(core, 'debug').mockImplementation(() => {}) + jest + .spyOn(io, 'rmRF') + .mockRejectedValue(new Error('simulated cleanup failure')) + failStage = commandFails ? 'tar' : undefined + + const result = readers[operation](archives[0], compression) + if (commandFails) { + await expect(result).rejects.toThrow('simulated tar failure') + } else { + await expect(result).resolves.toBeUndefined() + } + expect(debugMock).toHaveBeenCalledWith( + expect.stringContaining('simulated cleanup failure') + ) + } + ) + test.each<[Operation, 'decompress' | 'tar']>([ ['extract', 'decompress'], ['extract', 'tar'], diff --git a/src/internal/tar.ts b/src/internal/tar.ts index 7b8d81a..cad3ac6 100644 --- a/src/internal/tar.ts +++ b/src/internal/tar.ts @@ -1,3 +1,4 @@ +import * as core from '@actions/core' import {exec} from '@actions/exec' import * as io from '@actions/io' import {existsSync, writeFileSync} from 'fs' @@ -286,7 +287,11 @@ async function execReadCommands( await execCommands(commands, tempDirectory) } finally { if (tempDirectory) { - await io.rmRF(tempDirectory) + try { + await io.rmRF(tempDirectory) + } catch (error) { + core.debug(`Failed to delete temporary tar directory: ${error}`) + } } } }