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.
feat: add
build-artifactsinput to pass along interim build artifacts between runs #532New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
feat: add
build-artifactsinput to pass along interim build artifacts between runs #532Changes from all commits
f9b64b0dae9626File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
Check warning
Code scanning / CodeQL
Code injection Medium
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path resolution allows absolute paths to be used as-is without validation. This could be a security concern if the build-artifacts input is not properly controlled, as it allows writing to any location on the filesystem during restore. Consider validating that absolute paths, if allowed, are within expected boundaries, or document this as a security consideration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the cp command fails during restoration (e.g., due to permission issues or disk space), the script continues without detecting the error. Consider adding error checking after the cp commands to ensure artifacts are successfully restored before proceeding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path handling does not sanitize or validate relative paths that contain parent directory references (..). This could allow paths like "../../../etc/passwd" to be processed, potentially causing artifacts to be staged from or restored to unintended locations. Consider validating that paths do not traverse outside the working directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the cp command fails during staging (e.g., due to permission issues or disk space), the script continues without detecting the error. This could result in an incomplete manifest that references artifacts that were not actually staged. Consider adding error checking after the cp commands to ensure artifacts are successfully copied before adding them to the manifest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON generation does not properly escape special characters in the path values. If a path contains double quotes, backslashes, or newlines, it could corrupt the JSON manifest or cause parsing errors during restoration. Consider using jq to properly generate the JSON entries instead of using echo with manual JSON string concatenation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the staging step fails partway through (e.g., after creating the staging directory but before successful completion), the cleanup step will not run because it's conditional on upload success. This could leave the .tfviapr-artifacts directory behind. Consider adding error handling or a cleanup on failure.
Uh oh!
There was an error while loading. Please reload this page.