diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 30795e5..f13fb47 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -88,6 +88,16 @@ jobs: haxe-version: ${{ matrix.haxe }} cache-dependency-path: ${{ env.TEST_LIB_HXML }} + - name: "Working directory must stay clean after setup (issue #40)" + run: | + dirty="$(git status --porcelain)" + if [ -n "$dirty" ]; then + echo "::error::setup-haxe left files in the working directory (see issue #40):" + echo "$dirty" + exit 1 + fi + echo "Working directory is clean after setup-haxe." + - run: neko -version - run: haxe -version diff --git a/dist/index.js b/dist/index.js index b1343dc..b1123ef 100644 --- a/dist/index.js +++ b/dist/index.js @@ -245,8 +245,6 @@ var lib_core = __nccwpck_require__(2186); var lib_exec = __nccwpck_require__(1514); // EXTERNAL MODULE: external "node:crypto" var external_node_crypto_ = __nccwpck_require__(6005); -;// CONCATENATED MODULE: external "node:fs" -const external_node_fs_namespaceObject = require("node:fs"); ;// CONCATENATED MODULE: external "node:process" const external_node_process_namespaceObject = require("node:process"); // EXTERNAL MODULE: external "os" @@ -4008,7 +4006,6 @@ function _unique(values) { - function resolveTarget(input) { const { tool, platform, arch } = input; if (platform === 'win32' && arch === 'arm64') { @@ -4155,7 +4152,7 @@ class Asset { } async download() { const downloadPath = await this.downloadWithCurl(this.downloadUrl); - const extractPath = await this.extract(downloadPath, this.fileNameWithoutExt, this.fileExt); + const extractPath = await this.extract(downloadPath, this.fileExt); const toolRoot = await this.findToolRoot(extractPath, this.isDirectoryNested); if (!toolRoot) { throw new Error(`tool directory not found: ${extractPath}`); @@ -4190,10 +4187,10 @@ class Asset { lib_core.debug(`temporary directory: ${temporary}`); return temporary; } - async extract(file, dest, ext) { - if (external_node_fs_namespaceObject.existsSync(dest)) { - external_node_fs_namespaceObject.rmdirSync(dest, { recursive: true }); - } + // Extract under a temp dir rather than the cwd. + async extract(file, ext) { + const dest = external_node_path_namespaceObject.join(this.getTempDir(), external_node_crypto_.randomUUID()); + lib_core.debug(`extracting ${file} to ${dest}`); switch (ext) { case '.tar.gz': { return extractTar(file, dest); @@ -4307,6 +4304,8 @@ class HaxeAsset extends Asset { } } +;// CONCATENATED MODULE: external "node:fs" +const external_node_fs_namespaceObject = require("node:fs"); ;// CONCATENATED MODULE: ./node_modules/@actions/cache/node_modules/@actions/core/lib/utils.js // We use any as a valid input type /* eslint-disable @typescript-eslint/no-explicit-any */ diff --git a/src/asset.test.ts b/src/asset.test.ts index 988bc81..951c770 100644 --- a/src/asset.test.ts +++ b/src/asset.test.ts @@ -1,5 +1,7 @@ import type * as OsType from 'node:os'; import * as os from 'node:os'; +import * as path from 'node:path'; +import * as tc from '@actions/tool-cache'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { HaxeAsset, NekoAsset, resolveTarget } from './asset'; @@ -12,6 +14,13 @@ vi.mock('node:os', async () => { }; }); +vi.mock('@actions/tool-cache', () => ({ + extractTar: vi.fn(), + extractZip: vi.fn(), + find: vi.fn(), + cacheDir: vi.fn(), +})); + function setOs(platform: string, arch: string): void { vi.mocked(os.platform).mockReturnValue(platform as NodeJS.Platform); vi.mocked(os.arch).mockReturnValue(arch); @@ -144,7 +153,7 @@ describe('NekoAsset (nightly)', () => { ['darwin', 'x64'], ['darwin', 'arm64'], ['win32', 'x64'], - ] as const)('%s/%s extract dir name = neko_latest (symmetric with HaxeAsset)', (platform, arch) => { + ] as const)('%s/%s nightly fileNameWithoutExt = neko_latest (symmetric with HaxeAsset)', (platform, arch) => { setOs(platform, arch); const asset = new TestableNeko('latest', true, false); expect(asset.fileNameWithoutExt).toBe('neko_latest'); @@ -242,3 +251,50 @@ describe('resolveTarget cachePlatform (haxelib cache key compatibility)', () => } }); }); + +// Regression test for issue #40: extract must not write into the action's cwd +// ($GITHUB_WORKSPACE). It used to pass fileNameWithoutExt (a relative path) as +// the dest, leaving e.g. haxe_latest/.../std in the user's checkout. +describe('Asset.extract destination (issue #40)', () => { + type ExtractFn = (file: string, ext: '.tar.gz' | '.zip') => Promise; + + const originalRunnerTemp = process.env.RUNNER_TEMP; + + beforeEach(() => { + process.env.RUNNER_TEMP = '/runner/temp'; + }); + + afterEach(() => { + if (originalRunnerTemp === undefined) { + delete process.env.RUNNER_TEMP; + } else { + process.env.RUNNER_TEMP = originalRunnerTemp; + } + }); + + it('.tar.gz is extracted under RUNNER_TEMP, not a cwd-relative path', async () => { + const asset = new HaxeAsset('4.3.7', false); + const file = '/tmp/download/haxe.tar.gz'; + await (asset as unknown as { extract: ExtractFn }).extract(file, '.tar.gz'); + + expect(tc.extractTar).toHaveBeenCalledTimes(1); + const [calledFile, dest] = vi.mocked(tc.extractTar).mock.calls[0] as [string, string]; + expect(calledFile).toBe(file); + expect(path.isAbsolute(dest)).toBe(true); + expect(dest.startsWith('/runner/temp')).toBe(true); + expect(dest).not.toBe('haxe-4.3.7-linux64'); + }); + + it('.zip is extracted under RUNNER_TEMP, not a cwd-relative path', async () => { + const asset = new HaxeAsset('4.3.7', false); + const file = '/tmp/download/haxe.zip'; + await (asset as unknown as { extract: ExtractFn }).extract(file, '.zip'); + + expect(tc.extractZip).toHaveBeenCalledTimes(1); + const [calledFile, dest] = vi.mocked(tc.extractZip).mock.calls[0] as [string, string]; + expect(calledFile).toBe(file); + expect(path.isAbsolute(dest)).toBe(true); + expect(dest.startsWith('/runner/temp')).toBe(true); + expect(dest).not.toBe('haxe-4.3.7-linux64'); + }); +}); diff --git a/src/asset.ts b/src/asset.ts index 9bdcafc..275c160 100644 --- a/src/asset.ts +++ b/src/asset.ts @@ -5,7 +5,6 @@ import type { Buffer } from 'node:buffer'; import * as crypto from 'node:crypto'; -import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import * as process from 'node:process'; @@ -227,7 +226,7 @@ abstract class Asset { private async download() { const downloadPath = await this.downloadWithCurl(this.downloadUrl); - const extractPath = await this.extract(downloadPath, this.fileNameWithoutExt, this.fileExt); + const extractPath = await this.extract(downloadPath, this.fileExt); const toolRoot = await this.findToolRoot(extractPath, this.isDirectoryNested); if (!toolRoot) { @@ -270,10 +269,10 @@ abstract class Asset { return temporary; } - private async extract(file: string, dest: string, ext: AssetFileExt) { - if (fs.existsSync(dest)) { - fs.rmdirSync(dest, { recursive: true }); - } + // Extract under a temp dir rather than the cwd. + private async extract(file: string, ext: AssetFileExt) { + const dest = path.join(this.getTempDir(), crypto.randomUUID()); + core.debug(`extracting ${file} to ${dest}`); switch (ext) { case '.tar.gz': {