Merge pull request #461 from actions/danwkennedy/digest-mismatch-behavior

Add a setting to specify what to do on hash mismatch and default it to `error`

.github/workflows/test.yml

@@ -63,14 +63,14 @@ jobs:
         path: path/to/artifact-B
 
     # Test downloading a single artifact
-    - name: Download artifact A
+    - name: Download artifact A (absolute path)
       uses: ./
       with:
         name: Artifact-A-${{ matrix.runs-on }}
         path: some/new/path
 
     # Test downloading an artifact using tilde expansion
-    - name: Download artifact A
+    - name: Download artifact A (tilde expansion)
       uses: ./
       with:
         name: Artifact-A-${{ matrix.runs-on }}
@@ -142,7 +142,7 @@ jobs:
 
     - name: Verify skip-decompress download
       run: |
-        $rawFile = "skip-decompress-test/artifact"
+        $rawFile = "skip-decompress-test/Artifact-A-${{ matrix.runs-on }}.zip"
         if(!(Test-Path -path $rawFile))
         {
             Write-Error "Expected raw artifact file does not exist at $rawFile"

README.md

@@ -23,6 +23,21 @@ See also [upload-artifact](https://github.com/actions/upload-artifact).
   - [Limitations](#limitations)
     - [Permission Loss](#permission-loss)
 
+## v8 - What's new
+
+> [!IMPORTANT]
+> actions/download-artifact@v8 has been migrated to an ESM module. This should be transparent to the caller but forks might need to make significant changes.
+
+> [!IMPORTANT]
+> Hash mismatches will now error by default. Users can override this behavior with a setting change (see below).
+
+- Downloads will check the content-type returned to determine if a file can be decompressed and skip the decompression stage if so. This removes previous failures where we were trying to decompress a non-zip file. Since this is making a big change to the default behavior, we're making it opt-in via a version bump.
+- Users can also download a zip file without decompressing it with the new `skip-decompress` flag.
+
+- Introduces a new parameter `digest-mismatch` that allows callers to specify what to do when the downloaded hash doesn't match the expected hash (`ignore`, `info`, `warn`, `error`). To ensure security by default, the default value is `error`.
+
+- Chore: we've bumped versions on a lot of our dev packages to get them up to date with the latest bugfixes/security patches.
+
 ## v7 - What's new
 
 > [!IMPORTANT]
@@ -77,7 +92,7 @@ We are taking the following steps to better direct requests related to GitHub Ac
 
 1. We will be directing questions and support requests to our [Community Discussions area](https://github.com/orgs/community/discussions/categories/actions)
 
-2. High Priority bugs can be reported through Community Discussions or you can report these to our support team https://support.github.com/contact/bug-report.
+2. High Priority bugs can be reported through Community Discussions or you can report these to our support team <https://support.github.com/contact/bug-report>.
 
 3. Security Issues should be handled as per our [security.md](SECURITY.md).
 
@@ -216,7 +231,7 @@ If the `name` input parameter is not provided, all artifacts will be downloaded.
 
 Example, if there are two artifacts `Artifact-A` and `Artifact-B`, and the directory is `etc/usr/artifacts/`, the directory structure will look like this:
 
-```
+```bash
 etc/usr/artifacts/
     Artifact-A/
         ... contents of Artifact-A
@@ -258,7 +273,7 @@ steps:
 
 Which will result in:
 
-```
+```bash
 path/to/artifacts/
     ... contents of Artifact-A
     ... contents of Artifact-B
@@ -298,7 +313,7 @@ jobs:
 
 This results in a directory like so:
 
-```
+```bash
 my-artifact/
   file-macos-latest.txt
   file-ubuntu-latest.txt

__tests__/download.test.ts

@@ -234,7 +234,7 @@ describe('download', () => {
     )
   })
 
-  test('warns when digest validation fails', async () => {
+  test('errors when digest validation fails (default behavior)', async () => {
     const mockArtifact = {
       id: 123,
       name: 'corrupted-artifact',
@@ -242,6 +242,31 @@ describe('download', () => {
       digest: 'abc123'
     }
 
+    jest
+      .spyOn(artifact.default, 'getArtifact')
+      .mockImplementation(() => Promise.resolve({artifact: mockArtifact}))
+
+    jest
+      .spyOn(artifact.default, 'downloadArtifact')
+      .mockImplementation(() => Promise.resolve({digestMismatch: true}))
+
+    await expect(run()).rejects.toThrow(
+      'Digest validation failed for artifact(s): corrupted-artifact'
+    )
+  })
+
+  test('warns when digest validation fails with digest-mismatch set to warn', async () => {
+    const mockArtifact = {
+      id: 123,
+      name: 'corrupted-artifact',
+      size: 1024,
+      digest: 'abc123'
+    }
+
+    mockInputs({
+      [Inputs.DigestMismatch]: 'warn'
+    })
+
     jest
       .spyOn(artifact.default, 'getArtifact')
       .mockImplementation(() => Promise.resolve({artifact: mockArtifact}))
@@ -257,6 +282,61 @@ describe('download', () => {
     )
   })
 
+  test('logs info when digest validation fails with digest-mismatch set to info', async () => {
+    const mockArtifact = {
+      id: 123,
+      name: 'corrupted-artifact',
+      size: 1024,
+      digest: 'abc123'
+    }
+
+    mockInputs({
+      [Inputs.DigestMismatch]: 'info'
+    })
+
+    jest
+      .spyOn(artifact.default, 'getArtifact')
+      .mockImplementation(() => Promise.resolve({artifact: mockArtifact}))
+
+    jest
+      .spyOn(artifact.default, 'downloadArtifact')
+      .mockImplementation(() => Promise.resolve({digestMismatch: true}))
+
+    await run()
+
+    expect(core.info).toHaveBeenCalledWith(
+      expect.stringContaining('digest validation failed')
+    )
+  })
+
+  test('silently continues when digest validation fails with digest-mismatch set to ignore', async () => {
+    const mockArtifact = {
+      id: 123,
+      name: 'corrupted-artifact',
+      size: 1024,
+      digest: 'abc123'
+    }
+
+    mockInputs({
+      [Inputs.DigestMismatch]: 'ignore'
+    })
+
+    jest
+      .spyOn(artifact.default, 'getArtifact')
+      .mockImplementation(() => Promise.resolve({artifact: mockArtifact}))
+
+    jest
+      .spyOn(artifact.default, 'downloadArtifact')
+      .mockImplementation(() => Promise.resolve({digestMismatch: true}))
+
+    await run()
+
+    expect(core.warning).not.toHaveBeenCalledWith(
+      expect.stringContaining('digest validation failed')
+    )
+    expect(core.info).toHaveBeenCalledWith('Total of 1 artifact(s) downloaded')
+  })
+
   test('downloads a single artifact by ID', async () => {
     const mockArtifact = {
       id: 456,

action.yml

@@ -40,6 +40,11 @@ inputs:
       This is useful when you want to handle the artifact as-is without extraction.'
     required: false
     default: 'false'
+  digest-mismatch:
+    description: 'The behavior when a downloaded artifact''s digest does not match the expected digest.
+      Options: ignore, info, warn, error. Default is error which will fail the action.'
+    required: false
+    default: 'error'
 outputs:
   download-path:
     description: 'Path of artifact download'

dist/index.js

@@ -129353,7 +129353,15 @@ var Inputs;
     Inputs["MergeMultiple"] = "merge-multiple";
     Inputs["ArtifactIds"] = "artifact-ids";
     Inputs["SkipDecompress"] = "skip-decompress";
+    Inputs["DigestMismatch"] = "digest-mismatch";
 })(Inputs || (Inputs = {}));
+var DigestMismatchBehavior;
+(function (DigestMismatchBehavior) {
+    DigestMismatchBehavior["Ignore"] = "ignore";
+    DigestMismatchBehavior["Info"] = "info";
+    DigestMismatchBehavior["Warn"] = "warn";
+    DigestMismatchBehavior["Error"] = "error";
+})(DigestMismatchBehavior || (DigestMismatchBehavior = {}));
 var Outputs;
 (function (Outputs) {
     Outputs["DownloadPath"] = "download-path";
@@ -129386,8 +129394,15 @@ async function run() {
         artifactIds: getInput(Inputs.ArtifactIds, { required: false }),
         skipDecompress: getBooleanInput(Inputs.SkipDecompress, {
             required: false
-        })
+        }),
+        digestMismatch: (getInput(Inputs.DigestMismatch, { required: false }) ||
+            DigestMismatchBehavior.Error)
     };
+    // Validate digest-mismatch input
+    const validBehaviors = Object.values(DigestMismatchBehavior);
+    if (!validBehaviors.includes(inputs.digestMismatch)) {
+        throw new Error(`Invalid value for 'digest-mismatch': '${inputs.digestMismatch}'. Valid options are: ${validBehaviors.join(', ')}`);
+    }
     if (!inputs.path) {
         inputs.path = process.env['GITHUB_WORKSPACE'] || process.cwd();
     }
@@ -129500,6 +129515,7 @@ async function run() {
         })
     }));
     const chunkedPromises = chunk(downloadPromises, PARALLEL_DOWNLOADS);
+    const digestMismatches = [];
     for (const chunk of chunkedPromises) {
         const chunkPromises = chunk.map(item => item.promise);
         const results = await Promise.all(chunkPromises);
@@ -129507,10 +129523,31 @@ async function run() {
             const outcome = results[i];
             const artifactName = chunk[i].name;
             if (outcome.digestMismatch) {
-                warning(`Artifact '${artifactName}' digest validation failed. Please verify the integrity of the artifact.`);
+                digestMismatches.push(artifactName);
+                const message = `Artifact '${artifactName}' digest validation failed. Please verify the integrity of the artifact.`;
+                switch (inputs.digestMismatch) {
+                    case DigestMismatchBehavior.Ignore:
+                        // Do nothing
+                        break;
+                    case DigestMismatchBehavior.Info:
+                        info(message);
+                        break;
+                    case DigestMismatchBehavior.Warn:
+                        warning(message);
+                        break;
+                    case DigestMismatchBehavior.Error:
+                        // Collect all errors and fail at the end
+                        break;
                 }
             }
         }
+    }
+    // If there were digest mismatches and behavior is 'error', fail the action
+    if (digestMismatches.length > 0 &&
+        inputs.digestMismatch === DigestMismatchBehavior.Error) {
+        throw new Error(`Digest validation failed for artifact(s): ${digestMismatches.join(', ')}. ` +
+            `Use 'digest-mismatch: warn' to continue on mismatch.`);
+    }
     info(`Total of ${artifacts.length} artifact(s) downloaded`);
     setOutput(Outputs.DownloadPath, resolvedPath);
     info('Download artifact has finished successfully');

package.json

@@ -1,6 +1,6 @@
 {
   "name": "download-artifact",
-  "version": "7.0.0",
+  "version": "8.0.0",
   "description": "Download an Actions Artifact from a workflow run",
   "type": "module",
   "engines": {

src/constants.ts

@@ -7,7 +7,15 @@ export enum Inputs {
   Pattern = 'pattern',
   MergeMultiple = 'merge-multiple',
   ArtifactIds = 'artifact-ids',
-  SkipDecompress = 'skip-decompress'
+  SkipDecompress = 'skip-decompress',
+  DigestMismatch = 'digest-mismatch'
+}
+
+export enum DigestMismatchBehavior {
+  Ignore = 'ignore',
+  Info = 'info',
+  Warn = 'warn',
+  Error = 'error'
 }
 
 export enum Outputs {

src/download-artifact.ts

@@ -4,7 +4,7 @@ import * as core from '@actions/core'
 import artifactClient from '@actions/artifact'
 import type {Artifact, FindOptions} from '@actions/artifact'
 import {Minimatch} from 'minimatch'
-import {Inputs, Outputs} from './constants.js'
+import {Inputs, Outputs, DigestMismatchBehavior} from './constants.js'
 
 const PARALLEL_DOWNLOADS = 5
 
@@ -29,7 +29,17 @@ export async function run(): Promise<void> {
     artifactIds: core.getInput(Inputs.ArtifactIds, {required: false}),
     skipDecompress: core.getBooleanInput(Inputs.SkipDecompress, {
       required: false
-    })
+    }),
+    digestMismatch: (core.getInput(Inputs.DigestMismatch, {required: false}) ||
+      DigestMismatchBehavior.Error) as DigestMismatchBehavior
+  }
+
+  // Validate digest-mismatch input
+  const validBehaviors = Object.values(DigestMismatchBehavior)
+  if (!validBehaviors.includes(inputs.digestMismatch)) {
+    throw new Error(
+      `Invalid value for 'digest-mismatch': '${inputs.digestMismatch}'. Valid options are: ${validBehaviors.join(', ')}`
+    )
   }
 
   if (!inputs.path) {
@@ -188,6 +198,8 @@ export async function run(): Promise<void> {
   }))
 
   const chunkedPromises = chunk(downloadPromises, PARALLEL_DOWNLOADS)
+  const digestMismatches: string[] = []
+
   for (const chunk of chunkedPromises) {
     const chunkPromises = chunk.map(item => item.promise)
     const results = await Promise.all(chunkPromises)
@@ -197,12 +209,38 @@ export async function run(): Promise<void> {
       const artifactName = chunk[i].name
 
       if (outcome.digestMismatch) {
-        core.warning(
+        digestMismatches.push(artifactName)
-          `Artifact '${artifactName}' digest validation failed. Please verify the integrity of the artifact.`
+        const message = `Artifact '${artifactName}' digest validation failed. Please verify the integrity of the artifact.`
+
+        switch (inputs.digestMismatch) {
+          case DigestMismatchBehavior.Ignore:
+            // Do nothing
+            break
+          case DigestMismatchBehavior.Info:
+            core.info(message)
+            break
+          case DigestMismatchBehavior.Warn:
+            core.warning(message)
+            break
+          case DigestMismatchBehavior.Error:
+            // Collect all errors and fail at the end
+            break
+        }
+      }
+    }
+  }
+
+  // If there were digest mismatches and behavior is 'error', fail the action
+  if (
+    digestMismatches.length > 0 &&
+    inputs.digestMismatch === DigestMismatchBehavior.Error
+  ) {
+    throw new Error(
+      `Digest validation failed for artifact(s): ${digestMismatches.join(', ')}. ` +
+        `Use 'digest-mismatch: warn' to continue on mismatch.`
     )
   }
-    }
+
-  }
   core.info(`Total of ${artifacts.length} artifact(s) downloaded`)
   core.setOutput(Outputs.DownloadPath, resolvedPath)
   core.info('Download artifact has finished successfully')