From 7f19a7886a8b3b16c971a89cb787b850cb6a7980 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Fri, 20 Sep 2024 17:23:43 -0400 Subject: [PATCH 1/5] fix regression, auto readlink on symlinks again --- packages/artifact/RELEASES.md | 4 ++ .../__tests__/upload-artifact.test.ts | 45 +++++++++++++++---- .../upload-zip-specification.test.ts | 16 +++++++ packages/artifact/package-lock.json | 8 ++-- packages/artifact/package.json | 2 +- .../upload/upload-zip-specification.ts | 17 +++++-- packages/artifact/src/internal/upload/zip.ts | 11 ++++- 7 files changed, 83 insertions(+), 20 deletions(-) diff --git a/packages/artifact/RELEASES.md b/packages/artifact/RELEASES.md index 6688ad45..70351075 100644 --- a/packages/artifact/RELEASES.md +++ b/packages/artifact/RELEASES.md @@ -1,5 +1,9 @@ # @actions/artifact Releases +### 2.1.10 + +- Fixed a bug with symlinks not being automatically resolved. + ### 2.1.9 - Fixed artifact upload chunk timeout logic [#1774](https://github.com/actions/toolkit/pull/1774) diff --git a/packages/artifact/__tests__/upload-artifact.test.ts b/packages/artifact/__tests__/upload-artifact.test.ts index cd383db9..c92abfd6 100644 --- a/packages/artifact/__tests__/upload-artifact.test.ts +++ b/packages/artifact/__tests__/upload-artifact.test.ts @@ -27,9 +27,14 @@ jest.mock('@azure/storage-blob', () => ({ const fixtures = { uploadDirectory: path.join(__dirname, '_temp', 'plz-upload'), files: [ - ['file1.txt', 'test 1 file content'], - ['file2.txt', 'test 2 file content'], - ['file3.txt', 'test 3 file content'] + {name: 'file1.txt', content: 'test 1 file content'}, + {name: 'file2.txt', content: 'test 2 file content'}, + {name: 'file3.txt', content: 'test 3 file content'}, + { + name: 'from_symlink.txt', + content: 'from a symlink', + symlink: '../symlinked.txt' + } ], backendIDs: { workflowRunBackendId: '67dbcc20-e851-4452-a7c3-2cc0d2e0ec67', @@ -54,8 +59,23 @@ describe('upload-artifact', () => { fs.mkdirSync(fixtures.uploadDirectory, {recursive: true}) } - for (const [file, content] of fixtures.files) { - fs.writeFileSync(path.join(fixtures.uploadDirectory, file), content) + for (const file of fixtures.files) { + if (file.symlink) { + const symlinkPath = path.join(fixtures.uploadDirectory, file.symlink) + fs.writeFileSync(symlinkPath, file.content) + if (!fs.existsSync(path.join(fixtures.uploadDirectory, file.name))) { + fs.symlinkSync( + symlinkPath, + path.join(fixtures.uploadDirectory, file.name), + 'file' + ) + } + } else { + fs.writeFileSync( + path.join(fixtures.uploadDirectory, file.name), + file.content + ) + } } }) @@ -71,8 +91,9 @@ describe('upload-artifact', () => { .spyOn(uploadZipSpecification, 'getUploadZipSpecification') .mockReturnValue( fixtures.files.map(file => ({ - sourcePath: path.join(fixtures.uploadDirectory, file[0]), - destinationPath: file[0] + sourcePath: path.join(fixtures.uploadDirectory, file.name), + destinationPath: file.name, + stats: new fs.Stats() })) ) jest.spyOn(config, 'getRuntimeToken').mockReturnValue(fixtures.runtimeToken) @@ -185,6 +206,10 @@ describe('upload-artifact', () => { }) it('should successfully upload an artifact', async () => { + jest + .spyOn(uploadZipSpecification, 'getUploadZipSpecification') + .mockRestore() + jest .spyOn(ArtifactServiceClientJSON.prototype, 'CreateArtifact') .mockReturnValue( @@ -228,8 +253,10 @@ describe('upload-artifact', () => { const {id, size} = await uploadArtifact( fixtures.inputs.artifactName, - fixtures.inputs.files, - fixtures.inputs.rootDirectory + fixtures.files.map(file => + path.join(fixtures.uploadDirectory, file.name) + ), + fixtures.uploadDirectory ) expect(id).toBe(1) diff --git a/packages/artifact/__tests__/upload-zip-specification.test.ts b/packages/artifact/__tests__/upload-zip-specification.test.ts index 0b59bff7..3c6bbfb0 100644 --- a/packages/artifact/__tests__/upload-zip-specification.test.ts +++ b/packages/artifact/__tests__/upload-zip-specification.test.ts @@ -305,4 +305,20 @@ describe('Search', () => { } } }) + + it('Upload Specification - Includes symlinks', async () => { + const targetPath = path.join(root, 'link-dir', 'symlink-me.txt') + await fs.mkdir(path.dirname(targetPath), {recursive: true}) + await fs.writeFile(targetPath, 'symlink file content') + + const uploadPath = path.join(root, 'upload-dir', 'symlink.txt') + await fs.mkdir(path.dirname(uploadPath), {recursive: true}) + await fs.symlink(targetPath, uploadPath, 'file') + + const specifications = getUploadZipSpecification([uploadPath], root) + expect(specifications.length).toEqual(1) + expect(specifications[0].sourcePath).toEqual(uploadPath) + expect(specifications[0].destinationPath).toEqual('/upload-dir/symlink.txt') + expect(specifications[0].stats.isSymbolicLink()).toBe(true) + }) }) diff --git a/packages/artifact/package-lock.json b/packages/artifact/package-lock.json index 809562ab..d3318a97 100644 --- a/packages/artifact/package-lock.json +++ b/packages/artifact/package-lock.json @@ -1,6 +1,6 @@ { "name": "@actions/artifact", - "version": "2.1.9", + "version": "2.1.10", "lockfileVersion": 3, "requires": true, "packages": { @@ -1315,9 +1315,9 @@ } }, "node_modules/path-to-regexp": { - "version": "6.2.1", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-6.2.1.tgz", - "integrity": "sha512-JLyh7xT1kizaEvcaXOQwOc2/Yhw6KZOvPf1S8401UyLk86CU79LN3vl7ztXGm/pZ+YjoyAJ4rxmHwbkBXJX+yw==" + "version": "6.3.0", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-6.3.0.tgz", + "integrity": "sha512-Yhpw4T9C6hPpgPeA28us07OJeqZ5EzQTkbfwuhsUg0c237RomFoETJgmp2sa3F/41gfLE6G5cqcYwznmeEeOlQ==" }, "node_modules/prettier": { "version": "2.8.8", diff --git a/packages/artifact/package.json b/packages/artifact/package.json index fefa6abe..1f678e41 100644 --- a/packages/artifact/package.json +++ b/packages/artifact/package.json @@ -1,6 +1,6 @@ { "name": "@actions/artifact", - "version": "2.1.9", + "version": "2.1.10", "preview": true, "description": "Actions artifact lib", "keywords": [ diff --git a/packages/artifact/src/internal/upload/upload-zip-specification.ts b/packages/artifact/src/internal/upload/upload-zip-specification.ts index c6e807e6..54f34799 100644 --- a/packages/artifact/src/internal/upload/upload-zip-specification.ts +++ b/packages/artifact/src/internal/upload/upload-zip-specification.ts @@ -13,6 +13,12 @@ export interface UploadZipSpecification { * The destination path in a zip for a file */ destinationPath: string + + /** + * Information about the file + * https://nodejs.org/api/fs.html#class-fsstats + */ + stats: fs.Stats } /** @@ -75,10 +81,11 @@ export function getUploadZipSpecification( - file3.txt */ for (let file of filesToZip) { - if (!fs.existsSync(file)) { + const stats = fs.lstatSync(file, {throwIfNoEntry: false}) + if (!stats) { throw new Error(`File ${file} does not exist`) } - if (!fs.statSync(file).isDirectory()) { + if (!stats.isDirectory()) { // Normalize and resolve, this allows for either absolute or relative paths to be used file = normalize(file) file = resolve(file) @@ -94,7 +101,8 @@ export function getUploadZipSpecification( specification.push({ sourcePath: file, - destinationPath: uploadPath + destinationPath: uploadPath, + stats }) } else { // Empty directory @@ -103,7 +111,8 @@ export function getUploadZipSpecification( specification.push({ sourcePath: null, - destinationPath: directoryPath + destinationPath: directoryPath, + stats }) } } diff --git a/packages/artifact/src/internal/upload/zip.ts b/packages/artifact/src/internal/upload/zip.ts index 10433fb8..8cc3fd0c 100644 --- a/packages/artifact/src/internal/upload/zip.ts +++ b/packages/artifact/src/internal/upload/zip.ts @@ -1,4 +1,5 @@ import * as stream from 'stream' +import {readlink} from 'fs/promises' import * as archiver from 'archiver' import * as core from '@actions/core' import {UploadZipSpecification} from './upload-zip-specification' @@ -42,8 +43,14 @@ export async function createZipUploadStream( for (const file of uploadSpecification) { if (file.sourcePath !== null) { - // Add a normal file to the zip - zip.file(file.sourcePath, { + // Check if symlink and resolve the source path + let sourcePath = file.sourcePath + if (file.stats.isSymbolicLink()) { + sourcePath = await readlink(file.sourcePath) + } + + // Add the file to the zip + zip.file(sourcePath, { name: file.destinationPath }) } else { From d6694e491d778c715963d2e0050edc5fa88c1c43 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Fri, 20 Sep 2024 17:31:40 -0400 Subject: [PATCH 2/5] update release notes --- packages/artifact/RELEASES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/artifact/RELEASES.md b/packages/artifact/RELEASES.md index 70351075..219f8764 100644 --- a/packages/artifact/RELEASES.md +++ b/packages/artifact/RELEASES.md @@ -2,7 +2,8 @@ ### 2.1.10 -- Fixed a bug with symlinks not being automatically resolved. +- Fixed a regression with symlinks not being automatically resolved [#1830](https://github.com/actions/toolkit/pull/1830) +- Fixed a regression with chunk timeout [#1786](https://github.com/actions/toolkit/pull/1786) ### 2.1.9 From 8551843690e2650a845c7e7294b2a128aaf6c222 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Fri, 20 Sep 2024 17:45:55 -0400 Subject: [PATCH 3/5] fix assertion --- packages/artifact/__tests__/upload-zip-specification.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/artifact/__tests__/upload-zip-specification.test.ts b/packages/artifact/__tests__/upload-zip-specification.test.ts index 3c6bbfb0..e30b5a16 100644 --- a/packages/artifact/__tests__/upload-zip-specification.test.ts +++ b/packages/artifact/__tests__/upload-zip-specification.test.ts @@ -318,7 +318,9 @@ describe('Search', () => { const specifications = getUploadZipSpecification([uploadPath], root) expect(specifications.length).toEqual(1) expect(specifications[0].sourcePath).toEqual(uploadPath) - expect(specifications[0].destinationPath).toEqual('/upload-dir/symlink.txt') + expect(specifications[0].destinationPath).toEqual( + path.join('upload-dir', 'symlink.txt') + ) expect(specifications[0].stats.isSymbolicLink()).toBe(true) }) }) From 5a62022195adf3ff6ac6a2eb7162e2c4e4942e10 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Fri, 20 Sep 2024 17:52:14 -0400 Subject: [PATCH 4/5] / --- packages/artifact/__tests__/upload-zip-specification.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/__tests__/upload-zip-specification.test.ts b/packages/artifact/__tests__/upload-zip-specification.test.ts index e30b5a16..9688aa6f 100644 --- a/packages/artifact/__tests__/upload-zip-specification.test.ts +++ b/packages/artifact/__tests__/upload-zip-specification.test.ts @@ -319,7 +319,7 @@ describe('Search', () => { expect(specifications.length).toEqual(1) expect(specifications[0].sourcePath).toEqual(uploadPath) expect(specifications[0].destinationPath).toEqual( - path.join('upload-dir', 'symlink.txt') + path.join('/upload-dir', 'symlink.txt') ) expect(specifications[0].stats.isSymbolicLink()).toBe(true) }) From 2a8f1c5ddd92081dc1524a3d1308763b31742724 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 1 Oct 2024 16:43:30 -0400 Subject: [PATCH 5/5] bump package lock version --- packages/artifact/package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/package-lock.json b/packages/artifact/package-lock.json index d3318a97..2e94dfb7 100644 --- a/packages/artifact/package-lock.json +++ b/packages/artifact/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "@actions/artifact", - "version": "2.1.9", + "version": "2.1.10", "license": "MIT", "dependencies": { "@actions/core": "^1.10.0",