Remove compromised GH action #1714
Conversation
WalkthroughThe workflow file has been updated to replace the previous action-based change detection with a custom shell script. The new script uses Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Runner
participant Script as Custom Shell Script
participant Steps as Workflow Steps
Runner->>Script: Execute git diff to check for changes
Script-->>Runner: Return ANY_CHANGED=true/false
alt ANY_CHANGED true
Runner->>Steps: Run dependent workflow steps
else ANY_CHANGED false
Runner->>Steps: Skip dependent workflow steps
end
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/examples.yml (1)
24-36: Replace the compromised action with a custom diff check
The new shell script correctly usesgit diffto detect changes across the specified paths and sets theANY_CHANGEDflag accordingly. To improve robustness, consider adding error handling (for example, aset -eat the start of the script) to ensure that any errors in the diff command cause an immediate failure rather than proceeding with possibly invalid output.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/examples.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: build
alexeyr-ci
force-pushed
the
alexeyr/fix-ci-changed-files
branch
2 times, most recently
from
446df53 to
460fd1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/examples.yml (1)
24-37: Robustness Improvement: Enhance Error Handling in Changed Files ScriptThe custom shell script accurately replaces the compromised GitHub action by using
git diffto detect changes. To further improve its reliability, consider adding error handling (for example, by usingset -euo pipefail) at the beginning of the script. This will ensure that any failure (such as ingit fetchorgit diff) is immediately caught, preventing the script from continuing with an unexpected state.Below is a diff suggestion for integrating this improvement:
- run: | - git fetch origin ${{ github.event.base_ref }} - CHANGED_FILES=$(git diff --name-only ${{ github.event.base_ref }} ${{ github.sha }} -- \ - lib/generators/ \ - rakelib/example_type.rb \ - rakelib/example_config.yml \ - rakelib/examples.rake \ - rakelib/run_rspec.rake) - if [ -n "$CHANGED_FILES" ]; then - ANY_CHANGED=true - else - ANY_CHANGED=false - fi - echo "any_changed=$ANY_CHANGED" >> "$GITHUB_OUTPUT" + run: | + set -euo pipefail + git fetch origin ${{ github.event.base_ref }} + CHANGED_FILES=$(git diff --name-only ${{ github.event.base_ref }} ${{ github.sha }} -- \ + lib/generators/ \ + rakelib/example_type.rb \ + rakelib/example_config.yml \ + rakelib/examples.rake \ + rakelib/run_rspec.rake) + if [ -n "$CHANGED_FILES" ]; then + ANY_CHANGED=true + else + ANY_CHANGED=false + fi + echo "any_changed=$ANY_CHANGED" >> "$GITHUB_OUTPUT"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/examples.yml(2 hunks)lib/generators/react_on_rails/dev_tests_generator.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/generators/react_on_rails/dev_tests_generator.rb
🔇 Additional comments (1)
.github/workflows/examples.yml (1)
88-89: Correct Numeric Comparison for Chrome VersionThe update to using the numeric
-ltoperator in the version check for Google Chrome is a sound improvement—this operator correctly compares numeric values, ensuring proper detection of versions below the set minimum.
alexeyr-ci
force-pushed
the
alexeyr/fix-ci-changed-files
branch
2 times, most recently
from
023b364 to
7cf77f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/examples.yml (2)
24-38: Custom Changed-Files Detection Script
The new shell script block correctly replaces the compromised GitHub action by usinggit diffto detect changes in the specified paths. It sets theANY_CHANGEDflag appropriately based on whether any changes were found. One suggestion is to add a safeguard or log an error ifBASE_SHAis unexpectedly empty; this can help diagnose issues in non-PR scenarios.
1-106: Reminder: Address Pending Checklist Items
The PR checklist indicates that tests, documentation updates, and the CHANGELOG have not been updated. While the workflow changes meet the technical requirements for replacing the compromised action, please consider adding the corresponding tests and documentation updates to ensure completeness of the PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/examples.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: build
- GitHub Check: rspec-package-tests (newest)
🔇 Additional comments (1)
.github/workflows/examples.yml (1)
89-89: Correct Numeric Comparison for Chrome Version
Changing the comparison operator to-ltensures a proper numeric evaluation of the installed Chrome version. This fixes the potential bug from using a string comparison operator and helps avoid unexpected behavior in the version check.
Pull Request checklist
[ ] Add/update test to cover these changes[ ] Update documentation[ ] Update CHANGELOG fileThis change is
Summary by CodeRabbit