Merge pull request #1844 from actions/robherley/artifact-2.1.11

Properly resolve relative symlinks

packages/artifact/RELEASES.md

@@ -1,5 +1,9 @@
 # @actions/artifact Releases
 
+### 2.1.11
+
+- Fixed a bug with relative symlinks resolution [#1844](https://github.com/actions/toolkit/pull/1844)
+
 ### 2.1.10
 
 - Fixed a regression with symlinks not being automatically resolved [#1830](https://github.com/actions/toolkit/pull/1830)

packages/artifact/__tests__/upload-artifact.test.ts

@@ -10,6 +10,7 @@ import {FilesNotFoundError} from '../src/internal/shared/errors'
 import {BlockBlobUploadStreamOptions} from '@azure/storage-blob'
 import * as fs from 'fs'
 import * as path from 'path'
+import unzip from 'unzip-stream'
 
 const uploadStreamMock = jest.fn()
 const blockBlobClientMock = jest.fn().mockImplementation(() => ({
@@ -31,9 +32,20 @@ const fixtures = {
     {name: 'file2.txt', content: 'test 2 file content'},
     {name: 'file3.txt', content: 'test 3 file content'},
     {
-      name: 'from_symlink.txt',
+      name: 'real.txt',
+      content: 'from a symlink'
+    },
+    {
+      name: 'relative.txt',
       content: 'from a symlink',
-      symlink: '../symlinked.txt'
+      symlink: 'real.txt',
+      relative: true
+    },
+    {
+      name: 'absolute.txt',
+      content: 'from a symlink',
+      symlink: 'real.txt',
+      relative: false
     }
   ],
   backendIDs: {
@@ -55,14 +67,17 @@ const fixtures = {
 
 describe('upload-artifact', () => {
   beforeAll(() => {
-    if (!fs.existsSync(fixtures.uploadDirectory)) {
-      fs.mkdirSync(fixtures.uploadDirectory, {recursive: true})
-    }
+    fs.mkdirSync(fixtures.uploadDirectory, {
+      recursive: true
+    })
 
     for (const file of fixtures.files) {
       if (file.symlink) {
-        const symlinkPath = path.join(fixtures.uploadDirectory, file.symlink)
-        fs.writeFileSync(symlinkPath, file.content)
+        let symlinkPath = file.symlink
+        if (!file.relative) {
+          symlinkPath = path.join(fixtures.uploadDirectory, file.symlink)
+        }
+
         if (!fs.existsSync(path.join(fixtures.uploadDirectory, file.name))) {
           fs.symlinkSync(
             symlinkPath,
@@ -227,6 +242,12 @@ describe('upload-artifact', () => {
         })
       )
 
+    let loadedBytes = 0
+    const uploadedZip = path.join(
+      fixtures.uploadDirectory,
+      '..',
+      'uploaded.zip'
+    )
     uploadStreamMock.mockImplementation(
       async (
         stream: NodeJS.ReadableStream,
@@ -234,19 +255,28 @@ describe('upload-artifact', () => {
         maxConcurrency?: number,
         options?: BlockBlobUploadStreamOptions
       ) => {
-        const {onProgress, abortSignal} = options || {}
+        const {onProgress} = options || {}
+
+        if (fs.existsSync(uploadedZip)) {
+          fs.unlinkSync(uploadedZip)
+        }
+        const uploadedZipStream = fs.createWriteStream(uploadedZip)
 
         onProgress?.({loadedBytes: 0})
-
-        return new Promise(resolve => {
-          const timerId = setTimeout(() => {
-            onProgress?.({loadedBytes: 256})
-            resolve({})
-          }, 1_000)
-          abortSignal?.addEventListener('abort', () => {
-            clearTimeout(timerId)
+        return new Promise((resolve, reject) => {
+          stream.on('data', chunk => {
+            loadedBytes += chunk.length
+            uploadedZipStream.write(chunk)
+            onProgress?.({loadedBytes})
+          })
+          stream.on('end', () => {
+            onProgress?.({loadedBytes})
+            uploadedZipStream.end()
             resolve({})
           })
+          stream.on('error', err => {
+            reject(err)
+          })
         })
       }
     )
@@ -260,7 +290,34 @@ describe('upload-artifact', () => {
     )
 
     expect(id).toBe(1)
-    expect(size).toBe(256)
+    expect(size).toBe(loadedBytes)
+
+    const extractedDirectory = path.join(
+      fixtures.uploadDirectory,
+      '..',
+      'extracted'
+    )
+    if (fs.existsSync(extractedDirectory)) {
+      fs.rmdirSync(extractedDirectory, {recursive: true})
+    }
+
+    const extract = new Promise((resolve, reject) => {
+      fs.createReadStream(uploadedZip)
+        .pipe(unzip.Extract({path: extractedDirectory}))
+        .on('close', () => {
+          resolve(true)
+        })
+        .on('error', err => {
+          reject(err)
+        })
+    })
+
+    await expect(extract).resolves.toBe(true)
+    for (const file of fixtures.files) {
+      const filePath = path.join(extractedDirectory, file.name)
+      expect(fs.existsSync(filePath)).toBe(true)
+      expect(fs.readFileSync(filePath, 'utf8')).toBe(file.content)
+    }
   })
 
   it('should throw an error uploading blob chunks get delayed', async () => {

packages/artifact/package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "@actions/artifact",
-  "version": "2.1.10",
+  "version": "2.1.11",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "@actions/artifact",
-      "version": "2.1.10",
+      "version": "2.1.11",
       "license": "MIT",
       "dependencies": {
         "@actions/core": "^1.10.0",

packages/artifact/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/artifact",
-  "version": "2.1.10",
+  "version": "2.1.11",
   "preview": true,
   "description": "Actions artifact lib",
   "keywords": [

packages/artifact/src/internal/upload/zip.ts

@@ -1,5 +1,5 @@
 import * as stream from 'stream'
-import {readlink} from 'fs/promises'
+import {realpath} from 'fs/promises'
 import * as archiver from 'archiver'
 import * as core from '@actions/core'
 import {UploadZipSpecification} from './upload-zip-specification'
@@ -46,7 +46,7 @@ export async function createZipUploadStream(
       // Check if symlink and resolve the source path
       let sourcePath = file.sourcePath
       if (file.stats.isSymbolicLink()) {
-        sourcePath = await readlink(file.sourcePath)
+        sourcePath = await realpath(file.sourcePath)
       }
 
       // Add the file to the zip