initialize CI - #4
Open
MatthiasBSchaefer wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughA new GitHub Actions workflow responds to ChangesExternal CI Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/external_triggered_CI.yml:
- Line 20: The CI workflow is using a mutable container tag, so update the image
references in the workflow to use immutable digests instead of latest. Replace
the ghcr.io/matthiasbschaefer/dymola_image:latest reference (and the other image
entries noted in the comment) with pinned digest-based values in the relevant
job definitions so the workflow always runs against the reviewed image versions.
- Around line 34-37: The shell invocations in the workflow are directly
interpolating client_payload-derived values, which can allow injected shell
syntax. Update the affected job steps around prepare_tool.sh and run_tool.sh to
pass EVENT_NUMBER, BASE_SHA, PR_SHA, and similar values via env, then reference
them inside the command with proper quoting rather than expanding template
expressions inline. Apply the same fix to the other matching steps mentioned in
this workflow so all script calls use the same safe pattern.
- Around line 1-15: The workflow triggered by repository_dispatch currently
inherits the repository default GITHUB_TOKEN scope, so add a workflow-level
permissions block to constrain it. Set the permissions needed for the jobs that
only checkout or read repository content, and disable unnecessary access
everywhere else since the external review posting already uses MODELICA_TOKEN.
Keep the change in the external_triggered_CI workflow so the permission model is
explicit at the top level and applies to all jobs.
- Around line 95-100: The approval gate currently only checks run_openmodelica
and run_dymola, so a failed overview step can still emit APPROVE even when
linked reports are missing. Update the gating logic in the workflow step that
sets GITHUB_OUTPUT to include the overview job result alongside the existing
tool jobs, and only set event=APPROVE when all required jobs succeed; otherwise
keep REQUEST_CHANGES. Use the existing needs.run_openmodelica, needs.run_dymola,
and overview job/result references in this workflow block to locate the fix.
- Around line 104-105: The external-triggered CI workflow is hardcoding the
wrong target repository in the review dispatch, so the review can be sent to the
wrong PR. Update the TARGET_REPO value in external_triggered_CI.yml to use the
dispatched PR repository instead of the fixed
matthiasbschaefer/ModelicaStandardLibrary value, keeping TARGET_PR tied to
github.event.client_payload.number. Use the workflow’s dispatch context or
payload fields to locate the correct repository dynamically in the
review-posting step.
- Around line 109-115: The review creation step in the workflow silently
succeeds on GitHub API 4xx/5xx responses because the current curl invocation
does not fail the job; update the review-posting command to surface API failures
and stop the job when the request is rejected. Adjust the curl usage in the
workflow step that builds the GitHub reviews API call so it returns a nonzero
exit status on HTTP errors and emits the error response for debugging, ensuring
failures from missing tokens, invalid repo/PR values, or validation issues are
not ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a3ea1b6-6610-4183-937b-0920455e0ad4
📒 Files selected for processing (1)
.github/workflows/external_triggered_CI.yml
| runs-on: ubuntu-latest | ||
|
|
||
| container: | ||
| image: ghcr.io/matthiasbschaefer/dymola_image:latest |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin CI containers by digest instead of latest.
These images contain the CI tools/scripts that decide and publish PR results. A mutable tag can change outcomes without a reviewed workflow change.
📌 Proposed pinning pattern
- image: ghcr.io/matthiasbschaefer/dymola_image:latest
+ image: ghcr.io/matthiasbschaefer/dymola_image@sha256:<vetted-digest>
@@
- image: ghcr.io/matthiasbschaefer/om_image:latest
+ image: ghcr.io/matthiasbschaefer/om_image@sha256:<vetted-digest>
@@
- image: ghcr.io/matthiasbschaefer/om_image:latest
+ image: ghcr.io/matthiasbschaefer/om_image@sha256:<vetted-digest>Also applies to: 45-45, 71-71
🧰 Tools
🪛 zizmor (1.26.1)
[error] 20-20: unpinned image references (unpinned-images): container image is pinned to latest
(unpinned-images)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/external_triggered_CI.yml at line 20, The CI workflow is
using a mutable container tag, so update the image references in the workflow to
use immutable digests instead of latest. Replace the
ghcr.io/matthiasbschaefer/dymola_image:latest reference (and the other image
entries noted in the comment) with pinned digest-based values in the relevant
job definitions so the workflow always runs against the reviewed image versions.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/external_triggered_CI.yml:
- Around line 107-115: The review dispatch flow in the workflow step that builds
the GitHub API request should validate TARGET_REPO and TARGET_PR before
MODELICA_TOKEN is used. Add an allowlist or strict pattern check in the same run
block (before constructing API and payload) so only the expected repository and
a valid PR number are accepted, and fail fast if the repository_dispatch payload
is unexpected. Refer to the TARGET_REPO, TARGET_PR, and API/payload setup in the
external_triggered_CI job when applying the guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 29bf9fe4-bffb-4b9a-93c4-fb5845147111
📒 Files selected for processing (1)
.github/workflows/external_triggered_CI.yml
Comment on lines
+107
to
+115
| TARGET_REPO: ${{ env.REPO_NAME }} | ||
| TARGET_PR: ${{ env.EVENT_NUMBER }} | ||
| EVENT: ${{ steps.decide.outputs.event }} | ||
| BODY: "Please look here for details:${{ env.PR_SERVER }}/${{ env.EVENT_NUMBER }}/Modelica/PR_comparison_report.html and ${{ env.PR_SERVER }}/${{ env.EVENT_NUMBER }}/ModelicaTest/PR_comparison_report.html" | ||
|
|
||
| run: | | ||
| set -euo pipefail | ||
| API="https://api.github.com/repos/${TARGET_REPO}/pulls/${TARGET_PR}/reviews" | ||
| payload="$(jq -n --arg event "$EVENT" --arg body "$BODY" '{event: $event, body: $body}')" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate the dispatched review target before using MODELICA_TOKEN.
TARGET_REPO and TARGET_PR come from repository_dispatch payload and are used with a privileged secret. Add an allowlist/pattern check before building the API URL so this workflow cannot be used to post reviews to an unintended repository/PR if dispatch access or token scope is broader than expected.
🔒 Proposed validation guard
run: |
set -euo pipefail
+ case "$TARGET_REPO" in
+ ltx-simulation/ModelicaStandardLibrary) ;;
+ *) echo "Unexpected TARGET_REPO: $TARGET_REPO" >&2; exit 1 ;;
+ esac
+ [[ "$TARGET_PR" =~ ^[0-9]+$ ]] || { echo "Invalid TARGET_PR: $TARGET_PR" >&2; exit 1; }
API="https://api.github.com/repos/${TARGET_REPO}/pulls/${TARGET_PR}/reviews"
payload="$(jq -n --arg event "$EVENT" --arg body "$BODY" '{event: $event, body: $body}')"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| TARGET_REPO: ${{ env.REPO_NAME }} | |
| TARGET_PR: ${{ env.EVENT_NUMBER }} | |
| EVENT: ${{ steps.decide.outputs.event }} | |
| BODY: "Please look here for details:${{ env.PR_SERVER }}/${{ env.EVENT_NUMBER }}/Modelica/PR_comparison_report.html and ${{ env.PR_SERVER }}/${{ env.EVENT_NUMBER }}/ModelicaTest/PR_comparison_report.html" | |
| run: | | |
| set -euo pipefail | |
| API="https://api.github.com/repos/${TARGET_REPO}/pulls/${TARGET_PR}/reviews" | |
| payload="$(jq -n --arg event "$EVENT" --arg body "$BODY" '{event: $event, body: $body}')" | |
| TARGET_REPO: ${{ env.REPO_NAME }} | |
| TARGET_PR: ${{ env.EVENT_NUMBER }} | |
| EVENT: ${{ steps.decide.outputs.event }} | |
| BODY: "Please look here for details:${{ env.PR_SERVER }}/${{ env.EVENT_NUMBER }}/Modelica/PR_comparison_report.html and ${{ env.PR_SERVER }}/${{ env.EVENT_NUMBER }}/ModelicaTest/PR_comparison_report.html" | |
| run: | | |
| set -euo pipefail | |
| case "$TARGET_REPO" in | |
| ltx-simulation/ModelicaStandardLibrary) ;; | |
| *) echo "Unexpected TARGET_REPO: $TARGET_REPO" >&2; exit 1 ;; | |
| esac | |
| [[ "$TARGET_PR" =~ ^[0-9]+$ ]] || { echo "Invalid TARGET_PR: $TARGET_PR" >&2; exit 1; } | |
| API="https://api.github.com/repos/${TARGET_REPO}/pulls/${TARGET_PR}/reviews" | |
| payload="$(jq -n --arg event "$EVENT" --arg body "$BODY" '{event: $event, body: $body}')" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/external_triggered_CI.yml around lines 107 - 115, The
review dispatch flow in the workflow step that builds the GitHub API request
should validate TARGET_REPO and TARGET_PR before MODELICA_TOKEN is used. Add an
allowlist or strict pattern check in the same run block (before constructing API
and payload) so only the expected repository and a valid PR number are accepted,
and fail fast if the repository_dispatch payload is unexpected. Refer to the
TARGET_REPO, TARGET_PR, and API/payload setup in the external_triggered_CI job
when applying the guard.
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.
Summary by CodeRabbit