feat: implement new artifact-ids input
#401
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new input, artifact-ids, which allows users to download artifacts by their unique IDs rather than by name to leverage artifact immutability for enhanced security.
- Adds the artifact-ids input and associated validation logic in the download artifact code
- Updates documentation, tests, and constants to support this new input
- Modifies error messages and info logs to reflect the mutually exclusive usage of name and artifact-ids
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/download-artifact.ts | Added artifact-ids input with validation and branch handling for downloading by IDs |
| src/constants.ts | Updated enumeration to include artifact-ids |
| docs/MIGRATION.md | Documented new immutable artifacts workflow |
| action.yml | Introduced new input definition for artifact-ids |
| tests/download.test.ts | Updated test expectation messages to include artifact-ids |
| README.md | Enhanced documentation with examples for downloading artifacts by ID |
Comments suppressed due to low confidence (1)
src/download-artifact.ts:89
- Ensure dedicated tests are added for the new artifact-ids branch, including scenarios for non-numeric input and cases where provided IDs are missing, to increase test coverage.
else if (isDownloadByIds) {
robherley
reviewed
Apr 17, 2025
…esulting id returned
…`isSingleArtifactDownload` logic
…y artifact-ids until a `getArtifactById()` public method exists
robherley
reviewed
Apr 17, 2025
robherley
approved these changes
Apr 22, 2025
|
I'm not quite sure what the release process looks like for this project so I'll just merge and leave it at that. Thanks! 🙇 |
Comment on lines
+220
to
+222
| upload: | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
This example needs a small correction to add an outputs: section.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
artifact-idsinputThis pull request implements a new
artifact-idsinput which takes a comma separated list of artifact ids for this Action to download.Why?
In
v4, artifacts are immutable by default and each artifact gets a unique ID when uploaded. When an artifact with the same name is uploaded again (with or withoutoverwrite: true), it gets a new artifact ID.To take advantage of this immutability for security purposes (to avoid potential TOCTOU issues where an artifact might be replaced between upload and download), the new
artifact-idsinput allows you to download artifacts by their specific ID rather than by name:This approach provides stronger guarantees about which artifact version you're downloading compared to using just the artifact name.
Testing
I gave this a go in another project and it works great!
resolves: #349