Skip to content

Fix GitHub Actions code injection and untrusted-checkout warnings#423

Merged
knoepfel merged 5 commits intomainfrom
copilot/address-medium-codeql-issues
Mar 13, 2026
Merged

Fix GitHub Actions code injection and untrusted-checkout warnings#423
knoepfel merged 5 commits intomainfrom
copilot/address-medium-codeql-issues

Conversation

Copy link
Contributor

Copilot AI commented Mar 13, 2026

${{ }} expressions interpolated directly inside run: and github-script blocks allow injection if any value contains shell/JS metacharacters. Additionally, the change-detection checkout lacked persist-credentials: false.

Fix pattern

Every ${{ }} inside a shell or JS block is moved to an env: mapping on that step and referenced as a plain environment variable:

# Before — injection vector
- name: Resolve outputs
  shell: bash
  run: |
    REF="${{ inputs.ref }}"
    if [ "${{ github.event_name }}" == "pull_request" ]; then
      BASE_SHA="${{ github.event.pull_request.base.sha }}"
    fi

# After — safe
- name: Resolve outputs
  shell: bash
  env:
    INPUT_REF: ${{ inputs.ref }}
  run: |
    REF="${INPUT_REF}"
    if [ "${GITHUB_EVENT_NAME}" == "pull_request" ]; then
      BASE_SHA="$(jq -r '.pull_request.base.sha // empty' "${GITHUB_EVENT_PATH}")"
    fi

Key details

  • Event-payload fields (github.event.pull_request.base.sha, github.event.issue.number, etc.) are read from $GITHUB_EVENT_PATH via jq -r '... // empty' — no injection risk and no extra env entries needed for fields that are already exposed as built-in runner vars (GITHUB_SHA, GITHUB_REPOSITORY, GITHUB_EVENT_NAME, GITHUB_REF, GITHUB_REF_NAME).
  • github-script blocks use process.env.VAR instead of inline ${{ }} interpolation; pr-number is passed through parseInt(..., 10) to enforce numeric type.
  • collect-format-results: removes the indirect $GITHUB_ENV heredoc detour — inputs.results-json is now passed directly as RESULTS_JSON and read via os.environ['RESULTS_JSON'] in Python.
  • run-change-detection: adds persist-credentials: false to actions/checkout to address the untrusted-checkout warning.

Files changed

  • .github/actions/prepare-check-outputs/action.yaml
  • .github/actions/prepare-fix-outputs/action.yaml
  • .github/actions/workflow-setup/action.yaml
  • .github/actions/run-change-detection/action.yaml
  • .github/actions/complete-pr-comment/action.yaml
  • .github/actions/collect-format-results/action.yaml
  • .github/actions/post-clang-tidy-results/action.yaml

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI changed the title [WIP] Fix medium level CodeQL issues in repository Address medium-level CodeQL alerts Mar 13, 2026
Copilot AI requested a review from greenc-FNAL March 13, 2026 19:09
Copilot AI changed the title Address medium-level CodeQL alerts Clarify CodeQL alert access limitations and advise on data handoff Mar 13, 2026
Copilot AI changed the title Clarify CodeQL alert access limitations and advise on data handoff Acknowledge CodeQL alert access limitations; advise on data handoff Mar 13, 2026
All ${{ inputs.* }}, ${{ github.event.* }}, and ${{ steps.*.outputs.* }}
expressions that were directly interpolated inside run: shell scripts or
github-script JavaScript blocks are moved to env: mappings.  Scripts
reference them as plain environment variables ($VAR / process.env.VAR).

Event-payload fields previously accessed via ${{ github.event.xxx }} are
now read from $GITHUB_EVENT_PATH with jq -r '.xxx // empty', eliminating
the injection vector while keeping the logic identical.

The untrusted-checkout warning in run-change-detection is addressed by
adding persist-credentials: false to the actions/checkout step.

The collect-format-results action is simplified: the indirect $GITHUB_ENV
heredoc detour is removed; the JSON is passed directly as RESULTS_JSON and
read via os.environ['RESULTS_JSON'] in the Python block.

Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Copilot AI changed the title Acknowledge CodeQL alert access limitations; advise on data handoff Fix GitHub Actions code injection and untrusted-checkout warnings Mar 13, 2026
@greenc-FNAL greenc-FNAL marked this pull request as ready for review March 13, 2026 19:36
Copilot AI review requested due to automatic review settings March 13, 2026 19:36
@greenc-FNAL
Copy link
Contributor

…mat-all workflows

- clang-tidy-fix.yaml: add missing pr_number to setup job outputs
- coverage.yaml: add missing repo and pr-base-sha workflow_dispatch inputs
- format-all.yaml: change skip-comment from string "true" to boolean true

Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 13, 2026

@copilot Please address actionlint failures at https://github.com/Framework-R-D/phlex/actions/runs/23067039571/job/67009191784?pr=423

Fixed in b1df5c8. Three pre-existing actionlint issues addressed:

  • clang-tidy-fix.yaml: Added missing pr_number to the setup job's outputs block (the report job was referencing it but it wasn't declared).
  • coverage.yaml: Added repo and pr-base-sha to the workflow_dispatch inputs (they were already being passed to workflow-setup but weren't declared as inputs to the workflow).
  • format-all.yaml: Changed skip-comment: "true"skip-comment: true (unquoted boolean) for all 7 reusable workflow calls — actionlint was correctly flagging that a string can't be assigned to a boolean-typed input.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens the repository’s composite GitHub Actions against code injection and untrusted-checkout warnings by eliminating direct ${{ }} interpolation inside run:/github-script bodies and tightening checkout credentials handling.

Changes:

  • Move ${{ ... }} expansions out of inline shell/JS bodies into step env: and reference them as plain environment variables.
  • Read event payload fields via $GITHUB_EVENT_PATH + jq instead of embedding ${{ github.event.* }} in scripts.
  • Add persist-credentials: false to the change-detection checkout and simplify passing formatter results JSON into the aggregator step.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/actions/prepare-check-outputs/action.yaml Ref resolution now uses env + jq over ${{ }} in shell.
.github/actions/prepare-fix-outputs/action.yaml Same ref/repo resolution hardening for fix-mode workflows.
.github/actions/workflow-setup/action.yaml Centralized outputs now come from env vars; PR number read via jq.
.github/actions/run-change-detection/action.yaml Checkout now uses persist-credentials: false; reporting uses env vars.
.github/actions/complete-pr-comment/action.yaml github-script reads inputs.status via process.env.
.github/actions/collect-format-results/action.yaml Pass results JSON directly via env instead of $GITHUB_ENV heredoc.
.github/actions/post-clang-tidy-results/action.yaml github-script reads inputs via process.env; PR number coerced to int.

You can also share your feedback on Copilot code review. Take the survey.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@greenc-FNAL
Copy link
Contributor

@copilot Treatment of skip_comment across all actions and workflows is inconsistent, and the "fix" in b1df5c8 has led to more actionlint errors reported at https://github.com/Framework-R-D/phlex/actions/runs/23067726936/job/67010165399?pr=423. Please investigate and resolve.

@greenc-FNAL
Copy link
Contributor

Review the full CodeQL report for details.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens GitHub Actions composites/workflows against code-injection risks by removing direct ${{ ... }} interpolation from run:/github-script bodies and addressing an untrusted-checkout warning.

Changes:

  • Move step-context values into env: and reference them via shell/JS env vars (and read event payload fields via $GITHUB_EVENT_PATH + jq).
  • Add persist-credentials: false to the change-detection checkout.
  • Minor workflow wiring updates (new coverage inputs; expose pr_number for clang-tidy fix reporting; normalize skip-comment values).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
.github/workflows/format-all.yaml Updates reusable-workflow inputs (skip-comment) when chaining fix workflows.
.github/workflows/coverage.yaml Adds workflow_dispatch inputs for repo/base-sha and passes them into workflow-setup.
.github/workflows/clang-tidy-fix.yaml Plumbs pr_number from workflow-setup outputs for result-posting.
.github/actions/workflow-setup/action.yaml Removes inline expressions in bash by using env: and jq to read event payload.
.github/actions/run-change-detection/action.yaml Adds persist-credentials: false and avoids inline expressions in logging step.
.github/actions/prepare-fix-outputs/action.yaml Resolves ref/repo/checkout path without inline expressions; uses $GITHUB_EVENT_PATH for dispatch inputs.
.github/actions/prepare-check-outputs/action.yaml Resolves ref/repo/base_sha/pr number without inline expressions; uses $GITHUB_EVENT_PATH parsing.
.github/actions/post-clang-tidy-results/action.yaml Removes inline interpolation in github-script; passes values via env and parses PR number.
.github/actions/complete-pr-comment/action.yaml Removes inline interpolation in github-script; passes status via env.
.github/actions/collect-format-results/action.yaml Passes results-json via env directly (no $GITHUB_ENV heredoc detour).

You can also share your feedback on Copilot code review. Take the survey.

- clang-format-fix, cmake-format-fix, header-guards-fix, jsonnet-format-fix,
  markdown-fix, python-fix, yaml-fix: change skip-comment from type: string
  to type: boolean, update default from "false" to false, remove || 'false'
  fallback in pass-through, and change != 'true' condition to !inputs.skip-comment
- This makes format-all.yaml's skip-comment: true (boolean) type-compatible
  and resolves actionlint 'bool value cannot be assigned' errors

Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 13, 2026

@copilot Treatment of skip_comment across all actions and workflows is inconsistent, and the "fix" in b1df5c8 has led to more actionlint errors reported at https:/...

Fixed in daef0d2. The root cause was that skip-comment was declared as type: string in all 7 reusable workflows, but actionlint treats both "true" (quoted) and true (unquoted) as boolean values in workflow with: blocks — so neither form was valid for a string-typed input.

The consistent fix: changed skip-comment from type: string to type: boolean across all 7 reusable workflows (clang-format-fix, cmake-format-fix, header-guards-fix, jsonnet-format-fix, markdown-fix, python-fix, yaml-fix):

  • type: boolean, default: false
  • Pass-through simplified: ${{ inputs.skip-comment }} (removed the || 'false' fallback that was only needed for the string type)
  • Condition updated: !inputs.skip-comment (instead of inputs.skip-comment != 'true')

format-all.yaml already has skip-comment: true (boolean) from the previous commit, which is now type-compatible. The handle-fix-commit composite action (which has no typed input declarations) receives the value coerced to the string "true" and continues to work correctly.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Phlex’s GitHub Actions workflows/composite actions against code-injection vectors by avoiding direct ${{ }} interpolation inside run:/github-script blocks, and it addresses an untrusted-checkout warning by disabling persisted credentials during checkout.

Changes:

  • Refactors several composite actions to pass inputs via env: and read event payload fields via $GITHUB_EVENT_PATH (jq) instead of inline ${{ }} interpolation.
  • Updates reusable “*-fix” workflows to use a boolean skip-comment input and boolean conditions.
  • Adds persist-credentials: false to the change-detection checkout step.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/actions/workflow-setup/action.yaml Moves step-level interpolations into env: and uses jq for event fields when standardizing outputs / has_changes.
.github/actions/prepare-check-outputs/action.yaml Resolves ref/repo/base SHA via env: + $GITHUB_EVENT_PATH instead of inline expressions.
.github/actions/prepare-fix-outputs/action.yaml Resolves ref/repo/checkout_path via env: + $GITHUB_EVENT_PATH / prior step outputs.
.github/actions/run-change-detection/action.yaml Adds persist-credentials: false and avoids inline interpolation in the reporting run: step.
.github/actions/post-clang-tidy-results/action.yaml Avoids inline ${{ }} inside github-script by using process.env.* and parses PR number as int.
.github/actions/complete-pr-comment/action.yaml Uses process.env.INPUT_STATUS instead of interpolating ${{ inputs.status }} in JS.
.github/actions/collect-format-results/action.yaml Passes JSON directly via env and parses in Python (removes $GITHUB_ENV heredoc indirection).
.github/workflows/*-fix.yaml (clang-format/cmake-format/yaml/python/markdown/jsonnet/header-guards) Switches skip-comment to boolean and updates conditions accordingly.
.github/workflows/format-all.yaml Updates calls to reusable workflows to pass boolean skip-comment: true.
.github/workflows/clang-tidy-fix.yaml Exposes pr_number from workflow-setup outputs.
.github/workflows/coverage.yaml Adds repo and pr-base-sha as workflow_dispatch inputs (but see review comment re: remaining inline interpolation).

You can also share your feedback on Copilot code review. Take the survey.

@knoepfel knoepfel merged commit 0471e2d into main Mar 13, 2026
39 checks passed
@knoepfel knoepfel deleted the copilot/address-medium-codeql-issues branch March 13, 2026 21:48
Copilot AI added a commit that referenced this pull request Mar 16, 2026
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
greenc-FNAL added a commit that referenced this pull request Mar 16, 2026
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
knoepfel pushed a commit that referenced this pull request Mar 16, 2026
…pressions (#427)

* Fix clang-tidy check regressions from #411/#423
* Add missing exclude-globs input to detect-relevant-changes action
* Fix CodeQL #111: use sparse checkout and git ls-tree to avoid materializing untrusted code
* Fix regressions in post-clang-tidy-results, clang-tidy-fix, clang-tidy-check, and coverage
* Fix all remaining ${{ }} in run: blocks and add persist-credentials: false to read-only checkouts
* Revert erroneous exit 1 in clang-tidy-check: finding issues should not fail CI
* Use workflow-setup outputs for repo name; inline safe step-outcome expressions
* Better discrimination between check failures and CMake/build failures.
* Address Kyle's review comments: working-directory and jq comment

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Co-authored-by: Chris Green <greenc@fnal.gov>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants