Merge pull request #391 from GhadimiR/main

Fix bug introduced in 4.2.0

.github/workflows/publish-immutable-actions.yml

@@ -0,0 +1,20 @@
+name: 'Publish Immutable Action Version'
+
+on:
+  release:
+    types: [published]
+
+jobs:
+  publish:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      id-token: write
+      packages: write
+
+    steps:
+      - name: Checking out
+        uses: actions/checkout@v4
+      - name: Publish
+        id: publish
+        uses: actions/publish-immutable-action@0.0.3

.github/workflows/release-new-action-version.yml

@@ -21,7 +21,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - name: Update the ${{ env.TAG_NAME }} tag
-      uses: actions/publish-action@v0.2.1
+      uses: actions/publish-action@v0.3.0
       with:
         source-tag: ${{ env.TAG_NAME }}
         slack-webhook: ${{ secrets.SLACK_WEBHOOK }}

.github/workflows/test.yml

@@ -40,6 +40,9 @@ jobs:
     - name: Format
       run: npm run format-check
 
+    - name: Run Unit Tests
+      run: npm test
+
     - name: Create artifacts
       run: |
         mkdir -p path/to/artifact-A

.licenses/npm/@actions/artifact.dep.yml

@@ -1,9 +1,9 @@
 ---
 name: "@actions/artifact"
-version: 2.0.1
+version: 2.3.2
 type: npm
-summary: 
-homepage: 
+summary: Actions artifact lib
+homepage: https://github.com/actions/toolkit/tree/main/packages/artifact
 license: mit
 licenses:
 - sources: LICENSE.md

.licenses/npm/@actions/core.dep.yml

@@ -1,6 +1,6 @@
 ---
 name: "@actions/core"
-version: 1.10.0
+version: 1.10.1
 type: npm
 summary: Actions core lib
 homepage: https://github.com/actions/toolkit/tree/main/packages/core

__tests__/download.test.ts

@@ -0,0 +1,224 @@
+import * as core from '@actions/core'
+import artifact, {ArtifactNotFoundError} from '@actions/artifact'
+import {run} from '../src/download-artifact'
+import {Inputs} from '../src/constants'
+
+jest.mock('@actions/github', () => ({
+  context: {
+    repo: {
+      owner: 'actions',
+      repo: 'toolkit'
+    },
+    runId: 123,
+    serverUrl: 'https://github.com'
+  }
+}))
+
+jest.mock('@actions/core')
+
+/* eslint-disable no-unused-vars */ /* eslint-disable  @typescript-eslint/no-explicit-any */
+const mockInputs = (overrides?: Partial<{[K in Inputs]?: any}>) => {
+  const inputs = {
+    [Inputs.Name]: 'artifact-name',
+    [Inputs.Path]: '/some/artifact/path',
+    [Inputs.GitHubToken]: 'warn',
+    [Inputs.Repository]: 'owner/some-repository',
+    [Inputs.RunID]: 'some-run-id',
+    [Inputs.Pattern]: 'some-pattern',
+    ...overrides
+  }
+
+  ;(core.getInput as jest.Mock).mockImplementation((name: string) => {
+    return inputs[name]
+  })
+  ;(core.getBooleanInput as jest.Mock).mockImplementation((name: string) => {
+    return inputs[name]
+  })
+
+  return inputs
+}
+
+describe('download', () => {
+  beforeEach(async () => {
+    mockInputs()
+    jest.clearAllMocks()
+
+    // Mock artifact client methods
+    jest
+      .spyOn(artifact, 'listArtifacts')
+      .mockImplementation(() => Promise.resolve({artifacts: []}))
+    jest.spyOn(artifact, 'getArtifact').mockImplementation(name => {
+      throw new ArtifactNotFoundError(`Artifact '${name}' not found`)
+    })
+    jest
+      .spyOn(artifact, 'downloadArtifact')
+      .mockImplementation(() => Promise.resolve({digestMismatch: false}))
+  })
+
+  test('downloads a single artifact by name', async () => {
+    const mockArtifact = {
+      id: 123,
+      name: 'artifact-name',
+      size: 1024,
+      digest: 'abc123'
+    }
+
+    jest
+      .spyOn(artifact, 'getArtifact')
+      .mockImplementation(() => Promise.resolve({artifact: mockArtifact}))
+
+    await run()
+
+    expect(artifact.downloadArtifact).toHaveBeenCalledWith(
+      mockArtifact.id,
+      expect.objectContaining({
+        expectedHash: mockArtifact.digest
+      })
+    )
+    expect(core.info).toHaveBeenCalledWith('Total of 1 artifact(s) downloaded')
+
+    expect(core.setOutput).toHaveBeenCalledWith(
+      'download-path',
+      expect.any(String)
+    )
+
+    expect(core.info).toHaveBeenCalledWith(
+      'Download artifact has finished successfully'
+    )
+  })
+
+  test('downloads multiple artifacts when no name or pattern provided', async () => {
+    jest.clearAllMocks()
+    mockInputs({
+      [Inputs.Name]: '',
+      [Inputs.Pattern]: ''
+    })
+
+    const mockArtifacts = [
+      {id: 123, name: 'artifact1', size: 1024, digest: 'abc123'},
+      {id: 456, name: 'artifact2', size: 2048, digest: 'def456'}
+    ]
+
+    // Set up artifact mock after clearing mocks
+    jest
+      .spyOn(artifact, 'listArtifacts')
+      .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts}))
+
+    // Reset downloadArtifact mock as well
+    jest
+      .spyOn(artifact, 'downloadArtifact')
+      .mockImplementation(() => Promise.resolve({digestMismatch: false}))
+
+    await run()
+
+    expect(core.info).toHaveBeenCalledWith(
+      'No input name or pattern filtered specified, downloading all artifacts'
+    )
+
+    expect(core.info).toHaveBeenCalledWith('Total of 2 artifact(s) downloaded')
+    expect(artifact.downloadArtifact).toHaveBeenCalledTimes(2)
+  })
+
+  test('sets download path output even when no artifacts are found', async () => {
+    mockInputs({[Inputs.Name]: ''})
+
+    await run()
+
+    expect(core.setOutput).toHaveBeenCalledWith(
+      'download-path',
+      expect.any(String)
+    )
+
+    expect(core.info).toHaveBeenCalledWith(
+      'Download artifact has finished successfully'
+    )
+
+    expect(core.info).toHaveBeenCalledWith('Total of 0 artifact(s) downloaded')
+  })
+
+  test('filters artifacts by pattern', async () => {
+    const mockArtifacts = [
+      {id: 123, name: 'test-artifact', size: 1024, digest: 'abc123'},
+      {id: 456, name: 'prod-artifact', size: 2048, digest: 'def456'}
+    ]
+
+    jest
+      .spyOn(artifact, 'listArtifacts')
+      .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts}))
+
+    mockInputs({
+      [Inputs.Name]: '',
+      [Inputs.Pattern]: 'test-*'
+    })
+
+    await run()
+
+    expect(artifact.downloadArtifact).toHaveBeenCalledTimes(1)
+    expect(artifact.downloadArtifact).toHaveBeenCalledWith(
+      123,
+      expect.anything()
+    )
+  })
+
+  test('uses token and repository information when provided', async () => {
+    const token = 'ghp_testtoken123'
+
+    mockInputs({
+      [Inputs.Name]: '',
+      [Inputs.GitHubToken]: token,
+      [Inputs.Repository]: 'myorg/myrepo',
+      [Inputs.RunID]: '789'
+    })
+
+    jest
+      .spyOn(artifact, 'listArtifacts')
+      .mockImplementation(() => Promise.resolve({artifacts: []}))
+
+    await run()
+
+    expect(artifact.listArtifacts).toHaveBeenCalledWith(
+      expect.objectContaining({
+        findBy: {
+          token,
+          workflowRunId: 789,
+          repositoryName: 'myrepo',
+          repositoryOwner: 'myorg'
+        }
+      })
+    )
+  })
+
+  test('throws error when repository format is invalid', async () => {
+    mockInputs({
+      [Inputs.GitHubToken]: 'some-token',
+      [Inputs.Repository]: 'invalid-format' // Missing the owner/repo format
+    })
+
+    await expect(run()).rejects.toThrow(
+      "Invalid repository: 'invalid-format'. Must be in format owner/repo"
+    )
+  })
+
+  test('warns when digest validation fails', async () => {
+    const mockArtifact = {
+      id: 123,
+      name: 'corrupted-artifact',
+      size: 1024,
+      digest: 'abc123'
+    }
+
+    jest
+      .spyOn(artifact, 'getArtifact')
+      .mockImplementation(() => Promise.resolve({artifact: mockArtifact}))
+
+    jest
+      .spyOn(artifact, 'downloadArtifact')
+      .mockImplementation(() => Promise.resolve({digestMismatch: true}))
+
+    await run()
+
+    expect(core.warning).toHaveBeenCalledWith(
+      expect.stringContaining('digest validation failed')
+    )
+  })
+})

docs/MIGRATION.md

@@ -2,6 +2,8 @@
 
 - [Migration](#migration)
   - [Multiple uploads to the same named Artifact](#multiple-uploads-to-the-same-named-artifact)
+  - [Overwriting an Artifact](#overwriting-an-artifact)
+  - [Merging multiple artifacts](#merging-multiple-artifacts)
 
 Several behavioral differences exist between Artifact actions `v3` and below vs `v4`. This document outlines common scenarios in `v3`, and how they would be handled in `v4`.
 
@@ -31,6 +33,7 @@ jobs:
       - name: Download All Artifacts
         uses: actions/download-artifact@v3
         with:
+          name: my-artifact
           path: my-artifact
       - run: ls -R my-artifact
 ```
@@ -71,6 +74,7 @@ jobs:
 -     uses: actions/download-artifact@v3
 +     uses: actions/download-artifact@v4
       with:
+-       name: my-artifact
         path: my-artifact
 +       pattern: my-artifact-*
 +       merge-multiple: true
@@ -78,3 +82,128 @@ jobs:
 ```
 
 In `v4`, the new `pattern:` input will filter the downloaded Artifacts to match the name specified. The new `merge-multiple:` input will support downloading multiple Artifacts to the same directory. If the files within the Artifacts have the same name, the last writer wins.
+
+## Overwriting an Artifact
+
+In `v3`, the contents of an Artifact were mutable so something like the following was possible:
+
+```yaml
+jobs:
+  upload:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Create a file
+        run: echo "hello world" > my-file.txt
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v3
+        with:
+          name: my-artifact # NOTE: same artifact name
+          path: my-file.txt
+  upload-again:
+    needs: upload
+    runs-on: ubuntu-latest
+    steps:
+      - name: Create a different file
+        run: echo "goodbye world" > my-file.txt
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v3
+        with:
+          name: my-artifact # NOTE: same artifact name
+          path: my-file.txt
+```
+
+The resulting `my-file.txt` in `my-artifact` will have "goodbye world" as the content.
+
+In `v4`, Artifacts are immutable unless deleted. To achieve this same behavior, you can use `overwrite: true` to delete the Artifact before a new one is created:
+
+```diff
+jobs:
+  upload:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Create a file
+        run: echo "hello world" > my-file.txt
+      - name: Upload Artifact
+-       uses: actions/upload-artifact@v3
++       uses: actions/upload-artifact@v4
+        with:
+          name: my-artifact # NOTE: same artifact name
+          path: my-file.txt
+  upload-again:
+    needs: upload
+    runs-on: ubuntu-latest
+    steps:
+      - name: Create a different file
+        run: echo "goodbye world" > my-file.txt
+      - name: Upload Artifact
+-       uses: actions/upload-artifact@v3
++       uses: actions/upload-artifact@v4
+        with:
+          name: my-artifact # NOTE: same artifact name
+          path: my-file.txt
++         overwrite: true
+```
+
+Note that this will create an _entirely_ new Artifact, with a different ID from the previous.
+
+## Merging multiple artifacts
+
+In `v3`, multiple uploads from multiple jobs could be done to the same Artifact. This would result in a single archive, which could be useful for sending to upstream systems outside of Actions via API or UI downloads.
+
+```yaml
+jobs:
+  upload:
+    strategy:
+      matrix:
+        runs-on: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.runs-on }}
+    steps:
+      - name: Create a File
+        run: echo "hello from ${{ matrix.runs-on }}" > file-${{ matrix.runs-on }}.txt
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v3
+        with:
+          name: all-my-files # NOTE: same artifact name
+          path: file-${{ matrix.runs-on }}.txt
+```
+
+The single `all-my-files` artifact would contain the following:
+
+```
+.
+  ∟ file-ubuntu-latest.txt
+  ∟ file-macos-latest.txt
+  ∟ file-windows-latest.txt
+```
+
+To achieve the same in `v4` you can change it like so:
+
+```diff
+jobs:
+  upload:
+    strategy:
+      matrix:
+        runs-on: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.runs-on }}
+    steps:
+      - name: Create a File
+        run: echo "hello from ${{ matrix.runs-on }}" > file-${{ matrix.runs-on }}.txt
+      - name: Upload Artifact
+-       uses: actions/upload-artifact@v3
++       uses: actions/upload-artifact@v4
+        with:
+-         name: all-my-files
++         name: my-artifact-${{ matrix.runs-on }}
+          path: file-${{ matrix.runs-on }}.txt
++ merge:
++   runs-on: ubuntu-latest
++   needs: upload
++   steps:
++     - name: Merge Artifacts
++       uses: actions/upload-artifact/merge@v4
++       with:
++         name: all-my-files
++         pattern: my-artifact-*
+```
+
+Note that this will download all artifacts to a temporary directory and reupload them as a single artifact. For more information on inputs and other use cases for `actions/upload-artifact/merge@v4`, see [the action documentation](https://github.com/actions/upload-artifact/blob/main/merge/README.md).

jest.config.ts

@@ -0,0 +1,12 @@
+module.exports = {
+  clearMocks: true,
+  moduleFileExtensions: ['js', 'ts'],
+  roots: ['<rootDir>'],
+  testEnvironment: 'node',
+  testMatch: ['**/*.test.ts'],
+  testRunner: 'jest-circus/runner',
+  transform: {
+    '^.+\\.ts$': 'ts-jest'
+  },
+  verbose: true
+}

package.json

@@ -1,6 +1,6 @@
 {
   "name": "download-artifact",
-  "version": "4.0.1",
+  "version": "4.2.0",
   "description": "Download an Actions Artifact from a workflow run",
   "main": "dist/index.js",
   "scripts": {
@@ -9,7 +9,8 @@
     "check-all": "concurrently \"npm:format-check\" \"npm:lint\" \"npm:build\"",
     "format": "prettier --write **/*.ts",
     "format-check": "prettier --check **/*.ts",
-    "lint": "eslint **/*.ts"
+    "lint": "eslint **/*.ts",
+    "test": "jest"
   },
   "repository": {
     "type": "git",
@@ -28,12 +29,13 @@
   },
   "homepage": "https://github.com/actions/download-artifact#readme",
   "dependencies": {
-    "@actions/artifact": "^2.0.1",
-    "@actions/core": "^1.10.0",
+    "@actions/artifact": "^2.3.2",
+    "@actions/core": "^1.10.1",
     "@actions/github": "^5.1.1",
     "minimatch": "^9.0.3"
   },
   "devDependencies": {
+    "@types/jest": "^29.5.14",
     "@types/node": "^12.12.6",
     "@typescript-eslint/eslint-plugin": "^6.14.0",
     "@vercel/ncc": "^0.33.4",
@@ -41,7 +43,10 @@
     "eslint": "^8.55.0",
     "eslint-plugin-github": "^4.10.1",
     "eslint-plugin-prettier": "^5.0.1",
+    "jest": "^29.7.0",
     "prettier": "^3.1.1",
+    "ts-jest": "^29.2.6",
+    "ts-node": "^10.9.2",
     "typescript": "^5.3.3"
   }
-}
+}

src/download-artifact.ts

@@ -15,7 +15,7 @@ export const chunk = <T>(arr: T[], n: number): T[][] =>
     return acc
   }, [] as T[][])
 
-async function run(): Promise<void> {
+export async function run(): Promise<void> {
   const inputs = {
     name: core.getInput(Inputs.Name, {required: false}),
     path: core.getInput(Inputs.Path, {required: false}),
@@ -75,10 +75,6 @@ async function run(): Promise<void> {
 
     artifacts = [targetArtifact]
   } else {
-    core.info(
-      `No input name specified, downloading all artifacts. Extra directory with the artifact name will be created for each download`
-    )
-
     const listArtifactResponse = await artifactClient.listArtifacts({
       latest: true,
       ...options
@@ -94,6 +90,15 @@ async function run(): Promise<void> {
       core.debug(
         `Filtered from ${listArtifactResponse.artifacts.length} to ${artifacts.length} artifacts`
       )
+    } else {
+      core.info(
+        'No input name or pattern filtered specified, downloading all artifacts'
+      )
+      if (!inputs.mergeMultiple) {
+        core.info(
+          'An extra directory with the artifact name will be created for each download'
+        )
+      }
     }
   }
 
@@ -101,26 +106,39 @@ async function run(): Promise<void> {
     core.info(`Preparing to download the following artifacts:`)
     artifacts.forEach(artifact => {
       core.info(
-        `- ${artifact.name} (ID: ${artifact.id}, Size: ${artifact.size})`
+        `- ${artifact.name} (ID: ${artifact.id}, Size: ${artifact.size}, Expected Digest: ${artifact.digest})`
       )
     })
   }
 
-  const downloadPromises = artifacts.map(artifact =>
-    artifactClient.downloadArtifact(artifact.id, {
+  const downloadPromises = artifacts.map(artifact => ({
+    name: artifact.name,
+    promise: artifactClient.downloadArtifact(artifact.id, {
       ...options,
       path:
         isSingleArtifactDownload || inputs.mergeMultiple
           ? resolvedPath
-          : path.join(resolvedPath, artifact.name)
+          : path.join(resolvedPath, artifact.name),
+      expectedHash: artifact.digest
     })
-  )
+  }))
 
   const chunkedPromises = chunk(downloadPromises, PARALLEL_DOWNLOADS)
   for (const chunk of chunkedPromises) {
-    await Promise.all(chunk)
-  }
+    const chunkPromises = chunk.map(item => item.promise)
+    const results = await Promise.all(chunkPromises)
 
+    for (let i = 0; i < results.length; i++) {
+      const outcome = results[i]
+      const artifactName = chunk[i].name
+
+      if (outcome.digestMismatch) {
+        core.warning(
+          `Artifact '${artifactName}' digest validation failed. Please verify the integrity of the artifact.`
+        )
+      }
+    }
+  }
   core.info(`Total of ${artifacts.length} artifact(s) downloaded`)
   core.setOutput(Outputs.DownloadPath, resolvedPath)
   core.info('Download artifact has finished successfully')

tsconfig.json

@@ -9,5 +9,5 @@
     "moduleResolution": "node",
     "esModuleInterop": true
   },
-  "exclude": ["node_modules", "**/*.test.ts"]
+  "exclude": ["node_modules", "**/*.test.ts", "jest.config.ts", "__tests__"]
 }