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]
@@ -69,7 +84,7 @@ For assistance with breaking changes, see [MIGRATION.md](docs/MIGRATION.md).
 
 ## Note
 
-Thank you for your interest in this GitHub repo, however, right now we are not taking contributions. 
+Thank you for your interest in this GitHub repo, however, right now we are not taking contributions.
 
 We continue to focus our resources on strategic areas that help our customers be successful while making developers' lives easier. While GitHub Actions remains a key part of this vision, we are allocating resources towards other areas of Actions and are not taking contributions to this repository at this time. The GitHub public roadmap is the best place to follow along for any updates on features we’re working on and what stage they’re in.
 
@@ -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(
-          `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:
+            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')