Skip to content

Enhance reusable workflows#80

Merged
James Bruten (james-bruten-mo) merged 174 commits into
mainfrom
develop
Jul 22, 2026
Merged

Enhance reusable workflows#80
James Bruten (james-bruten-mo) merged 174 commits into
mainfrom
develop

Conversation

@yaswant

@yaswant Yaswant Pradhan (yaswant) commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

PR Summary

Hardens the GitHub Actions security baseline across all workflows.

Code Reviewer: James Bruten (@james-bruten-mo)

Downstream pull requests

List of downstream PR's need coordinated merge.

Action pinning

  • All third-party actions replaced with immutable 40-char commit SHAs (e.g. actions/checkout@v6@df4cb1c…).
  • actions/github-script in track-review-project.yaml upgraded from v8 → v9 as part of the pin.

Credential & permission scoping

  • persist-credentials: false added to every actions/checkout step
  • Top-level permissions: {} set on caller workflows; granular contents: read / pull-requests: write / actions: read pushed down to job level in call-track-review-project.yaml, call-trigger-project-workflow.yaml, and trigger-project-workflow.yaml
  • fortran-lint.yaml gains an explicit contents: read job permission.

Template-injection fixes (zizmor)

  • cla-check.yaml: step outputs and inputs.cla-url moved to env: vars, read via process.env.* in the github-script block.
  • fortran-lint.yaml: all string/path inputs moved to env: vars; boolean flags resolved in shell using a bash array.
  • track-review-project.yaml: inputs.project_org and inputs.project_number moved to env: vars. Also, included PROJECT_ACTION_PAT secret as required parameter to avoid secret inheritance in caller workflow (breaking change! See updated README for usage).

CLA workflow logic

  • Merge-ref detection replaced: git ls-remote → gh api repos/.../pulls/… (avoids unauthenticated git network call).
  • CONTRIBUTORS.md modification check rewritten: git diff → GitHub Contents API + base64 | tr | cmp (avoids authenticated git fetch from fork). File modification now checked against content instead of just file state.

Tooling & config

  • New dependabot.yaml: monthly schedule, major-version updates blocked, all action updates grouped into a single PR
  • New zizmor.yaml: suppresses unpinned-uses, dangerous-triggers, and secrets-inherit for the two caller workflows that use secrets: inherit
  • .yamllint: updated ignore syntax; added comments and comments-indentation rules.
  • PR template: minor formatting tidy-up, emoji section headers, few typo fixes.

To enforce strict GitHub Actions security baselines, we now use immutable 40-character commit SHAs.

✅ Code Quality Checklist

(Some checks are automatically carried out via the CI pipeline)

  • I have performed a self-review of my own code
  • My code follows the project's style guidelines
  • The modified workflow's README has been updated, if required
  • The changes have been sufficiently tested (see MetOffice/git_playground/pull/137)

🤖 AI Assistance and Attribution

  • Some of the content of this change has been produced with the assistance of Generative AI tool name (e.g., Met Office Github Copilot Enterprise, Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the Simulation Systems AI policy (including attribution labels)

💻 Code Review

  • The changes are approriate and testing has been sufficient

Co-authored-by: Yaswant Pradhan <2984440+yaswant@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 21, 2026 11:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (1)

.github/workflows/call-track-review-project.yaml:31

  • In reusable-workflow calls, the caller job permissions sets the maximum permissions the called workflow can use. This caller only grants actions: read and repository-projects: write, but the called track-review-project.yaml job requires at least contents: read and pull-requests: write (it runs gh pr edit). As written, the called workflow will fail when it tries to read/modify PR metadata.
    permissions:
      actions: read  # Required to view the status of the upstream triggered workflow run
      repository-projects: write  # Required to add or mutate tasks inside GitHub Project 376

Comment on lines +114 to 121
# Use gh api to query pull request metadata directly
# without network git ls-remote authentication
mergeable=$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER" --jq ".mergeable" 2>/dev/null || echo "undefined")
if [ "$mergeable" != "undefined" ]; then
echo "merge_ref_defined=true" >> "$GITHUB_ENV"
else
echo "merge_ref_defined=true" >> $GITHUB_ENV
echo "merge_ref_defined=false" >> "$GITHUB_ENV"
fi
Comment thread .github/pull_request_template.md Outdated
Comment thread .github/workflows/validate.yaml Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 13:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (3)

.github/workflows/call-track-review-project.yaml:31

  • MetOffice/growss/.github/workflows/track-review-project.yaml requests actions: read, contents: read, and pull-requests: write at job level. For reusable workflows, the called workflow's GITHUB_TOKEN permissions are constrained by the caller; this caller currently omits contents/pull-requests, which can cause the called workflow to fail when checking out or editing PR metadata.
    permissions:
      actions: read  # Required to view the status of the upstream triggered workflow run
      repository-projects: write  # Required to add or mutate tasks inside GitHub Project 376

.github/workflows/cla-check.yaml:118

  • mergeable is not a reliable proxy for whether refs/pull/<n>/merge exists (it can be null while GitHub computes mergeability, and can be present even when merge refs are unavailable due to conflicts). This can cause the workflow to attempt a merge-ref checkout when the ref doesn't exist (or skip it when it does). Prefer checking the ref directly via the Git refs API.
          # Use gh api to query pull request metadata directly
          # without network git ls-remote authentication
          mergeable=$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER" --jq ".mergeable" 2>/dev/null || echo "undefined")
          if [ "$mergeable" != "undefined" ]; then
            echo "merge_ref_defined=true" >> "$GITHUB_ENV"

.github/workflows/fortran-lint.yaml:117

  • ${CONFIG_PATH:++...} is expanded unquoted, which allows word-splitting/globbing if config-path contains spaces or shell metacharacters. Since these are workflow inputs, it's safer to build the argument list via the ARGS array (as you already do for the boolean flags) and keep everything quoted.
          uvx --from "fortitude-lint==$FORTITUDE_VERSION" fortitude check \
            "${ARGS[@]}" \
            "--file-extensions=$FILE_EXTENSIONS" \
            ${CONFIG_PATH:+"--config-file=$CONFIG_PATH"} \
            "$SOURCE_PATH"

Comment on lines +21 to +27
if: >-
github.event_name == 'workflow_dispatch' ||
(
github.event_name == 'workflow_run' &&
github.event.workflow_run.event == 'push' &&
github.event.workflow_run.conclusion == 'success'
)
Copilot AI review requested due to automatic review settings July 22, 2026 13:35
@james-bruten-mo
James Bruten (james-bruten-mo) merged commit 0f9ffe6 into main Jul 22, 2026
4 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (2)

.github/workflows/call-track-review-project.yaml:25

  • The caller workflow grants only actions: read and repository-projects: write, but the called reusable workflow (track-review-project.yaml) requests contents: read (checkout) and pull-requests: write (PR edits). For reusable workflows, the effective GITHUB_TOKEN permissions are capped by the caller, so this will break checkout and PR updates.
    permissions:
      actions: read  # Required to view the status of the upstream triggered workflow run
      repository-projects: write  # Required to add or mutate tasks inside GitHub Project 376
    uses: MetOffice/growss/.github/workflows/track-review-project.yaml@main

.github/workflows/cla-check.yaml:118

  • merge_ref_defined is currently set to true whenever the PR API call succeeds, regardless of whether a merge ref actually exists (e.g., for merge conflicts). This makes the later conditional logic misleading and can trigger a failing checkout of refs/pull/.../merge. Use the Git refs API to test existence of pull/<PR>/merge instead.
          # Use gh api to query pull request metadata directly
          # without network git ls-remote authentication
          mergeable=$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER" --jq ".mergeable" 2>/dev/null || echo "undefined")
          if [ "$mergeable" != "undefined" ]; then
            echo "merge_ref_defined=true" >> "$GITHUB_ENV"

Comment on lines 29 to 31
with:
runner: "ubuntu-22.04"
runner: "ubuntu-slim"
project_org: "MetOffice"
Comment on lines +19 to +21
permissions:
contents: read # Required for checking out code or referencing internal workflow actions
pull-requests: write # Required for the composite action to add or remove labels on the PR
Comment on lines +26 to 29
permissions:
contents: read
pull-requests: write # Required to add labels and comments
runs-on: ${{ inputs.runner }}
James Bruten (james-bruten-mo) pushed a commit to MetOffice/fab that referenced this pull request Jul 22, 2026
# PR Summary
Align repository actions with
MetOffice/growss#80

Code Reviewer: @james-bruten-mo 

- linked MetOffice/growss#80
@yaswant
Yaswant Pradhan (yaswant) deleted the develop branch July 22, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Changes to prevent code vulnerabilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security hardening: Pin third-party actions to immutable commit SHAs

4 participants