feat(security): harden all actions following security recommendations#9
Open
feat(security): harden all actions following security recommendations#9
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPin upstream action references to commit SHAs and adjust runtime command/auth handling across multiple composite actions: switch build-push to a buildah-driven shell flow, execute pre-build commands line-by-line, use .netrc for Git auth, verify yq install, and move theme credentials to environment variables. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner
participant Shell as Shell script
participant Buildah as buildah
participant Registry as Registry
Runner->>Shell: checkout code (pinned actions)
Shell->>Shell: ensure curl, jq, buildah, qemu-user-static installed
Shell->>Buildah: buildah login (username + password via stdin)
Shell->>Shell: iterate PRE_BUILD_COMMANDS -> eval each non-empty line
Shell->>Buildah: buildah build (assemble --build-arg flags, optional --target)
alt PLATFORMS contains comma
Buildah->>Buildah: create multi-arch manifest list
Buildah->>Registry: push manifest list for each tag
else single platform
Buildah->>Buildah: build single-platform image(s) with -t tags
Buildah->>Registry: push each tag
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plone-theme-build-push-notify/theme_uploader.py (2)
72-75:⚠️ Potential issue | 🟡 MinorPotential KeyError if
set-cookieheader is missing.Line 73 directly accesses
response.headers["set-cookie"]which will raiseKeyErrorif the header is absent. Consider using.get()for safer access.🛡️ Proposed fix
- if "__ac=deleted" in response.headers["set-cookie"]: + set_cookie = response.headers.get("set-cookie", "") + if "__ac=deleted" in set_cookie: print("Authentication failed") sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plone-theme-build-push-notify/theme_uploader.py` around lines 72 - 75, The code checks response.headers["set-cookie"] directly which can raise KeyError if the header is missing; change the check in the authentication flow (around the call to authenticate and the subsequent conditional) to read the header safely using response.headers.get("set-cookie", "") (or store headers.get(...) into a variable like set_cookie) and then test if "__ac=deleted" is in that string, keeping the existing print("Authentication failed") and sys.exit(1) behavior if the token appears.
7-8:⚠️ Potential issue | 🟡 MinorOutdated usage documentation.
The docstring still references the old CLI signature with
USERNAME PASSWORDas arguments. Update to reflect the new usage where credentials come from environment variables:📝 Proposed fix
"""Plone theme uploader This script is used to upload a theme to a Plone instance. Usage: - python theme_uploader.py INSTANCE_URL USERNAME PASSWORD THEME_LOCATION + python theme_uploader.py INSTANCE_URL THEME_PATH THEME_FILENAME + +Environment variables: + PLONE_USERNAME: Username to login to Plone + PLONE_PASSWORD: Password to login to Plone """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plone-theme-build-push-notify/theme_uploader.py` around lines 7 - 8, Update the module docstring/usage line in theme_uploader.py to reflect the new CLI signature: remove USERNAME and PASSWORD and state that credentials are read from environment variables (e.g., use INSTNACE_URL THEME_LOCATION and mention ENV VARS like PLONE_USERNAME/PLONE_PASSWORD or whichever env names are used by the script). Locate and edit the top-level usage string (the current "Usage: python theme_uploader.py INSTANCE_URL USERNAME PASSWORD THEME_LOCATION") so it documents the current runtime behavior and lists the expected env var names used by the script (refer to any code that reads os.environ for the exact variable names).
🧹 Nitpick comments (3)
repository-dispatch-notify/action.yml (1)
73-79: Use a release tag for the internalmattermost-notifydependency.Pinning
IMIO/gha/mattermost-notifyto a raw commit decouples this action from the publishedmattermost-notifyrelease line and makes future fixes easy to miss when only the tag is bumped.@v6or@v6.1.0is a better fit here.Based on learnings: GitHub Actions should be referenced by callers using major version tags (e.g.,
v5) or full version tags (e.g.,v5.1.0).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@repository-dispatch-notify/action.yml` around lines 73 - 79, Replace the hard-pinned commit reference for the IMIO/gha/mattermost-notify action with a release tag so future fixes are picked up; update both occurrences of "uses: IMIO/gha/mattermost-notify@ec02349a0f486af868fc78489425e65e3b3e0c23" (the initial notify step and the "Send failure notification on Mattermost" step) to a stable tag such as "@v6" or a full release tag like "@v6.1.0" to follow the recommended major-version tagging strategy.plone-theme-build-push-notify/action.yml (1)
61-67: Consider SHA-pinning internal action references for consistency.The PR objective is to harden actions following security recommendations. Lines 61 and 67 reference
IMIO/gha/mattermost-notify@v3.7.2using a version tag. If the goal is to pin third-party actions to commit SHAs for reproducibility and supply-chain security, consider whether internal action references should follow the same pattern for consistency.That said, version tags are acceptable per project conventions. Based on learnings: "GitHub Actions should be referenced by callers using major version tags (e.g.,
v5) or full version tags (e.g.,v5.1.0)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plone-theme-build-push-notify/action.yml` around lines 61 - 67, The workflow currently references the Mattermost action twice using the tag IMIO/gha/mattermost-notify@v3.7.2; update both occurrences to a SHA-pinned commit (or at minimum a full version tag consistent with project policy) so the action is reproducible and secure — locate the two uses of IMIO/gha/mattermost-notify@v3.7.2 (the success notification block and the "Send failure notification on Mattermost" block) and replace the tag with the chosen commit SHA (or canonical full-version tag) ensuring both references match exactly.plone-theme-build-push-notify/theme_uploader.py (1)
25-36: Function signature has unused parameters.The
authenticatefunction acceptslogin_url,username, andpasswordparameters but uses the global constantsINSTANCE_URL,USERNAME, andPASSWORDinstead. Either remove the unused parameters or use them consistently.♻️ Proposed fix: Remove unused parameters
-def authenticate( - session: requests.Session, login_url: str, username: str, password: str -) -> requests.Response: +def authenticate(session: requests.Session) -> requests.Response: url = INSTANCE_URL + "/failsafe_login" data = { "__ac_name": USERNAME,Then update the call site at line 72:
- response = authenticate(session, INSTANCE_URL, USERNAME, PASSWORD) + response = authenticate(session)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plone-theme-build-push-notify/theme_uploader.py` around lines 25 - 36, The authenticate function currently takes login_url, username, and password but ignores them and uses global INSTANCE_URL/USERNAME/PASSWORD; fix by making authenticate use the passed parameters (use login_url to build the URL and username/password to populate "__ac_name"/"__ac_password") and keep the return value the same, and then update any call sites that pass those arguments to continue supplying the correct values; alternatively, if you prefer globals, remove the unused parameters from authenticate and update all call sites to match the new signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build-push-notify/action.yml`:
- Around line 72-74: The pre-build loop spawns a new bash for each line which
prevents commands like cd/export/source from persisting; change the execution so
each line runs in the current shell instead of via bash -c. Locate the while
loop that reads PRE_BUILD_COMMANDS (the block using "while IFS= read -r cmd; do
... done <<< \"$PRE_BUILD_COMMANDS\"") and replace the per-line "bash -c
\"$cmd\"" invocation with an in-shell execution method (e.g., use eval or direct
execution in the current shell) so multi-line pre-build scripts share shell
state and exported variables/functions persist across commands.
In `@k8s-update-tag/action.yml`:
- Around line 44-47: The step writes credentials to ~/.netrc with
printf/registers it via chmod then runs git clone, but cleanup only runs at the
end so failures leave creds on disk; ensure ~/.netrc is removed on all exit
paths by adding an always-run cleanup (either a shell trap 'trap "rm -f
~/.netrc" EXIT' immediately after creating ~/.netrc or a separate GitHub Action
step with if: always() that deletes ~/.netrc), and apply the same change for the
other occurrence that prints to ~/.netrc (the second printf/chmod sequence).
In `@plone-package-test-notify/action.yml`:
- Around line 91-92: The current code uses read -ra cmd and then "${cmd[@]}",
which tokenizes TEST_COMMAND without honoring shell quoting (so quoted arguments
like "not slow and smoke" are split); replace the read -ra approach with a shell
evaluation that preserves quoting, e.g. evaluate TEST_COMMAND into an array with
eval "cmd=($TEST_COMMAND)" and then call "${cmd[@]}" (ensure TEST_COMMAND is
trusted before using eval); update the references to TEST_COMMAND and the
invocation that currently uses read -ra cmd and "${cmd[@]}" accordingly.
In `@rundeck-notify/action.yml`:
- Around line 40-42: The current parsing uses mapfile -t which only splits
RUNDECK_PARAMETERS on newlines, causing multi-option single-line strings to
remain a single argv entry; either update the action docs to state
RUNDECK_PARAMETERS must be newline-separated (one parameter per line) or change
the parsing to split on whitespace by replacing newlines with spaces and using a
word-splitting read (e.g., transform RUNDECK_PARAMETERS -> single-line and then
use read -r -a params to populate the params array) so flags like
"--data-urlencode foo=bar --data-urlencode baz=qux" become separate argv
entries; update any usage of params accordingly.
---
Outside diff comments:
In `@plone-theme-build-push-notify/theme_uploader.py`:
- Around line 72-75: The code checks response.headers["set-cookie"] directly
which can raise KeyError if the header is missing; change the check in the
authentication flow (around the call to authenticate and the subsequent
conditional) to read the header safely using response.headers.get("set-cookie",
"") (or store headers.get(...) into a variable like set_cookie) and then test if
"__ac=deleted" is in that string, keeping the existing print("Authentication
failed") and sys.exit(1) behavior if the token appears.
- Around line 7-8: Update the module docstring/usage line in theme_uploader.py
to reflect the new CLI signature: remove USERNAME and PASSWORD and state that
credentials are read from environment variables (e.g., use INSTNACE_URL
THEME_LOCATION and mention ENV VARS like PLONE_USERNAME/PLONE_PASSWORD or
whichever env names are used by the script). Locate and edit the top-level usage
string (the current "Usage: python theme_uploader.py INSTANCE_URL USERNAME
PASSWORD THEME_LOCATION") so it documents the current runtime behavior and lists
the expected env var names used by the script (refer to any code that reads
os.environ for the exact variable names).
---
Nitpick comments:
In `@plone-theme-build-push-notify/action.yml`:
- Around line 61-67: The workflow currently references the Mattermost action
twice using the tag IMIO/gha/mattermost-notify@v3.7.2; update both occurrences
to a SHA-pinned commit (or at minimum a full version tag consistent with project
policy) so the action is reproducible and secure — locate the two uses of
IMIO/gha/mattermost-notify@v3.7.2 (the success notification block and the "Send
failure notification on Mattermost" block) and replace the tag with the chosen
commit SHA (or canonical full-version tag) ensuring both references match
exactly.
In `@plone-theme-build-push-notify/theme_uploader.py`:
- Around line 25-36: The authenticate function currently takes login_url,
username, and password but ignores them and uses global
INSTANCE_URL/USERNAME/PASSWORD; fix by making authenticate use the passed
parameters (use login_url to build the URL and username/password to populate
"__ac_name"/"__ac_password") and keep the return value the same, and then update
any call sites that pass those arguments to continue supplying the correct
values; alternatively, if you prefer globals, remove the unused parameters from
authenticate and update all call sites to match the new signature.
In `@repository-dispatch-notify/action.yml`:
- Around line 73-79: Replace the hard-pinned commit reference for the
IMIO/gha/mattermost-notify action with a release tag so future fixes are picked
up; update both occurrences of "uses:
IMIO/gha/mattermost-notify@ec02349a0f486af868fc78489425e65e3b3e0c23" (the
initial notify step and the "Send failure notification on Mattermost" step) to a
stable tag such as "@v6" or a full release tag like "@v6.1.0" to follow the
recommended major-version tagging strategy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf868867-b96c-4b14-83fc-27b30ce5d373
📒 Files selected for processing (11)
build-push-notify/action.ymlcode-analysis-notify/action.ymlhelm-release-notify/action.ymlhelm-test-notify/action.ymlk8s-update-tag/action.ymlplone-package-test-notify/action.ymlplone-theme-build-push-notify/action.ymlplone-theme-build-push-notify/theme_uploader.pyrepository-dispatch-notify/action.ymlrundeck-notify/action.ymltag-notify/action.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
build-push-notify/action.yml (1)
72-74:⚠️ Potential issue | 🟠 MajorReplace per-line
evalwith single-script execution to avoid parsing hazards.The current implementation on lines 72-74 evaluates each command line independently, which breaks multi-line shell constructs (e.g.,
if/fiblocks). The line-by-line approach causes syntax errors when users provide multi-line commands inPRE_BUILD_COMMANDS, as the parser receives incomplete constructs.Execute the entire block as a single script instead to preserve construct integrity and avoid unnecessary double parsing:
Proposed fix
- while IFS= read -r cmd; do - [ -n "$cmd" ] && eval "$cmd" - done <<< "$PRE_BUILD_COMMANDS" + tmp_script="$(mktemp)" + trap 'rm -f "$tmp_script"' EXIT + printf '%s\n' "$PRE_BUILD_COMMANDS" > "$tmp_script" + bash -e "$tmp_script"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-push-notify/action.yml` around lines 72 - 74, The per-line evaluation loop that reads PRE_BUILD_COMMANDS and runs each line with eval breaks multi-line shell constructs; replace that while/IFS/read/eval pattern by executing the entire PRE_BUILD_COMMANDS payload as a single script invocation so multi-line blocks (if/fi, functions, heredocs) are preserved. Locate the loop handling PRE_BUILD_COMMANDS and change it to check for non-empty PRE_BUILD_COMMANDS and run the whole variable once via a single shell execution (e.g., pass the full string to bash/sh in one invocation) instead of iterating and eval-ing each line.
🧹 Nitpick comments (1)
plone-package-test-notify/action.yml (1)
83-85:read -radoes not preserve quoted arguments inBUILDOUT_OPTIONS.Unlike the
TEST_COMMANDhandling (which usesevalto preserve quoting), the buildout execution usesread -rawhich only does IFS splitting. If a user passesBUILDOUT_OPTIONS: '-D "some value"', it will be incorrectly split into four tokens:-D,"some,value".For consistency with the approach used for
TEST_COMMAND, consider usingevalhere as well:♻️ Proposed fix to preserve quoted arguments
run: | - read -ra buildout_cmd <<< "$BUILDOUT_COMMAND" - read -ra buildout_opts <<< "$BUILDOUT_OPTIONS" - "${buildout_cmd[@]}" "${buildout_opts[@]}" -c "$BUILDOUT_CONFIG_FILE" buildout:eggs-directory=./eggs + eval "buildout_cmd=($BUILDOUT_COMMAND)" + eval "buildout_opts=($BUILDOUT_OPTIONS)" + "${buildout_cmd[@]}" "${buildout_opts[@]}" -c "$BUILDOUT_CONFIG_FILE" buildout:eggs-directory=./eggs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plone-package-test-notify/action.yml` around lines 83 - 85, The buildout invocation currently uses read -ra to split BUILDOUT_COMMAND into buildout_cmd and BUILDOUT_OPTIONS into buildout_opts which drops quoting; change the parsing to use eval (the same approach as TEST_COMMAND) so that quoted arguments in BUILDOUT_OPTIONS and BUILDOUT_COMMAND are preserved, then populate the buildout_cmd and buildout_opts arrays and invoke "${buildout_cmd[@]}" "${buildout_opts[@]}" -c "$BUILDOUT_CONFIG_FILE" buildout:eggs-directory=./eggs; update the lines that assign buildout_cmd and buildout_opts (and the execution line if needed) to use eval-based splitting while keeping the final array invocation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@build-push-notify/action.yml`:
- Around line 72-74: The per-line evaluation loop that reads PRE_BUILD_COMMANDS
and runs each line with eval breaks multi-line shell constructs; replace that
while/IFS/read/eval pattern by executing the entire PRE_BUILD_COMMANDS payload
as a single script invocation so multi-line blocks (if/fi, functions, heredocs)
are preserved. Locate the loop handling PRE_BUILD_COMMANDS and change it to
check for non-empty PRE_BUILD_COMMANDS and run the whole variable once via a
single shell execution (e.g., pass the full string to bash/sh in one invocation)
instead of iterating and eval-ing each line.
---
Nitpick comments:
In `@plone-package-test-notify/action.yml`:
- Around line 83-85: The buildout invocation currently uses read -ra to split
BUILDOUT_COMMAND into buildout_cmd and BUILDOUT_OPTIONS into buildout_opts which
drops quoting; change the parsing to use eval (the same approach as
TEST_COMMAND) so that quoted arguments in BUILDOUT_OPTIONS and BUILDOUT_COMMAND
are preserved, then populate the buildout_cmd and buildout_opts arrays and
invoke "${buildout_cmd[@]}" "${buildout_opts[@]}" -c "$BUILDOUT_CONFIG_FILE"
buildout:eggs-directory=./eggs; update the lines that assign buildout_cmd and
buildout_opts (and the execution line if needed) to use eval-based splitting
while keeping the final array invocation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ad19429-f0a0-494f-9579-a0b85cd58fab
📒 Files selected for processing (3)
build-push-notify/action.ymlk8s-update-tag/action.ymlplone-package-test-notify/action.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- k8s-update-tag/action.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
build-push-notify/action.yml (1)
72-74:⚠️ Potential issue | 🟠 MajorLine-by-line
evalstill breaks valid multiline pre-build scripts.Lines 72-74 fix the fresh-shell part of the earlier finding, but they still parse
PRE_BUILD_COMMANDSone line at a time.if/fi,for/do/done, heredocs, and multiline function bodies will still fail, and the input does not say “one command per line.”💡 Suggested fix
- while IFS= read -r cmd; do - [ -n "$cmd" ] && eval "$cmd" - done <<< "$PRE_BUILD_COMMANDS" + tmp_script="$(mktemp)" + trap 'rm -f "$tmp_script"' EXIT + printf '%s\n' "$PRE_BUILD_COMMANDS" > "$tmp_script" + bash -eo pipefail "$tmp_script"#!/bin/bash # Verifies that the current loop rejects valid multiline shell blocks. set -euo pipefail PRE_BUILD_COMMANDS=$'if [ -n "${HOME:-}" ]; then\n echo ok\nfi' echo '=== current implementation ===' set +e while IFS= read -r cmd; do [ -n "$cmd" ] && eval "$cmd" done <<< "$PRE_BUILD_COMMANDS" status=$? set -e echo "current_status=$status" echo '=== execute the whole block as one script ===' tmp_script="$(mktemp)" trap 'rm -f "$tmp_script"' EXIT printf '%s\n' "$PRE_BUILD_COMMANDS" > "$tmp_script" bash -eo pipefail "$tmp_script"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-push-notify/action.yml` around lines 72 - 74, The current while-IFS-read + eval loop that iterates over PRE_BUILD_COMMANDS line-by-line (the while ... read -r cmd; ... eval "$cmd" block) breaks valid multiline shell constructs; replace it by writing PRE_BUILD_COMMANDS to a temporary script (use mktemp), make it executable/execute it with bash -euo pipefail (or bash -eo pipefail) and remove the temp file via trap to ensure cleanup, and only skip execution when PRE_BUILD_COMMANDS is empty; this preserves heredocs, if/fi, for/do/done and multiline functions while keeping error handling and cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build-push-notify/action.yml`:
- Around line 47-55: The action accepts a PLATFORMS input and uses buildah build
--platform but the install step only installs buildah/curl/jq, so multi-platform
RUN steps will fail without QEMU/binfmt; update the install block to include
qemu-user-static and ensure binfmt is registered (e.g., install qemu-user-static
and run the appropriate binfmt registration command or use update-binfmts)
before any buildah --platform invocation, or alternatively gate the PLATFORMS
flow to require a pre-configured runner and add a clear README note; reference
the PLATFORMS input and the buildah build --platform invocations when making the
change.
---
Duplicate comments:
In `@build-push-notify/action.yml`:
- Around line 72-74: The current while-IFS-read + eval loop that iterates over
PRE_BUILD_COMMANDS line-by-line (the while ... read -r cmd; ... eval "$cmd"
block) breaks valid multiline shell constructs; replace it by writing
PRE_BUILD_COMMANDS to a temporary script (use mktemp), make it
executable/execute it with bash -euo pipefail (or bash -eo pipefail) and remove
the temp file via trap to ensure cleanup, and only skip execution when
PRE_BUILD_COMMANDS is empty; this preserves heredocs, if/fi, for/do/done and
multiline functions while keeping error handling and cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f0665a2-94b4-4fa3-a429-704eee445e72
📒 Files selected for processing (1)
build-push-notify/action.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
build-push-notify/action.yml (1)
69-77: Usingevalpreserves shell state—consider adding error handling.The switch from
bash -ctoevalcorrectly maintains shell state across commands (e.g.,export,cdnow persist). However, if a command fails, subsequent commands still execute since there's noset -ein the script.For stricter error handling, consider the temp-script approach or add
set -eto the run block.♻️ Optional: stop on first error
run: | + set -e while IFS= read -r cmd; do [ -n "$cmd" ] && eval "$cmd" done <<< "$PRE_BUILD_COMMANDS"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-push-notify/action.yml` around lines 69 - 77, The pre-build run block that iterates PRE_BUILD_COMMANDS using eval can allow later commands to run after a failure; update the run section that reads PRE_BUILD_COMMANDS and uses eval in the while loop to enable strict failure handling by adding a shell failure mode (e.g., invoke "set -euo pipefail" or equivalent at the top of the run script) so any command that fails aborts the script, or alternatively implement the temp-script approach: write PRE_BUILD_COMMANDS to a temporary executable file and execute it with strict error flags; target the run block that references PRE_BUILD_COMMANDS and the while IFS= read -r cmd; do ... eval "$cmd" ... done <<< "$PRE_BUILD_COMMANDS" when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@build-push-notify/action.yml`:
- Around line 69-77: The pre-build run block that iterates PRE_BUILD_COMMANDS
using eval can allow later commands to run after a failure; update the run
section that reads PRE_BUILD_COMMANDS and uses eval in the while loop to enable
strict failure handling by adding a shell failure mode (e.g., invoke "set -euo
pipefail" or equivalent at the top of the run script) so any command that fails
aborts the script, or alternatively implement the temp-script approach: write
PRE_BUILD_COMMANDS to a temporary executable file and execute it with strict
error flags; target the run block that references PRE_BUILD_COMMANDS and the
while IFS= read -r cmd; do ... eval "$cmd" ... done <<< "$PRE_BUILD_COMMANDS"
when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e1bf544-02fe-4b06-ad13-9de3f48c050d
📒 Files selected for processing (1)
build-push-notify/action.yml
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
DEVOPS-334
Summary by CodeRabbit
Chores
Bug Fixes