Skip to content

[github] Add autoformat-v2.yml: single zizmor-clean workflow#25510

Merged
rolfbjarne merged 3 commits into
mainfrom
dev/rolf/zizmor-autoformat
May 26, 2026
Merged

[github] Add autoformat-v2.yml: single zizmor-clean workflow#25510
rolfbjarne merged 3 commits into
mainfrom
dev/rolf/zizmor-autoformat

Conversation

@rolfbjarne
Copy link
Copy Markdown
Member

Create a new autoformat workflow that avoids the security concerns
flagged by zizmor in the existing autoformat.yml + autoformat2.yml:

  • No workflow_run trigger (eliminates dangerous-triggers)
  • All actions pinned to SHA hashes (eliminates unpinned-uses)
  • persist-credentials: false on checkout (eliminates artipacked)
  • Job-level permissions (eliminates excessive-permissions)

Two jobs handle same-repo and fork PRs differently:

  • Same-repo: format + commit + push directly
  • Fork: format + upload patch as artifact

All logic is inlined (no rolfbjarne/autoformat references).

Create a new autoformat workflow that avoids the security concerns
flagged by zizmor in the existing autoformat.yml + autoformat2.yml:

- No workflow_run trigger (eliminates dangerous-triggers)
- All actions pinned to SHA hashes (eliminates unpinned-uses)
- persist-credentials: false on checkout (eliminates artipacked)
- Job-level permissions (eliminates excessive-permissions)

Two jobs handle same-repo and fork PRs differently:
- Same-repo: format + commit + push directly
- Fork: format + upload patch as artifact

All logic is inlined (no rolfbjarne/autoformat references).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
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 adds a new GitHub Actions workflow (autoformat-v2.yml) intended to replace/avoid security concerns in the existing autoformat workflows by removing workflow_run, pinning actions to SHAs, disabling persisted checkout credentials, and using job-level permissions. It splits handling into same-repo PRs (autoformat + push) and fork PRs (autoformat + upload patch artifact).

Changes:

  • Add a new pull_request-triggered autoformat workflow with two separate jobs for same-repo vs fork PRs.
  • Pin actions/checkout, actions/setup-dotnet, and actions/upload-artifact to specific SHAs and disable persist-credentials.
  • For fork PRs, generate an autoformat.patch artifact instead of pushing changes.
Comments suppressed due to low confidence (1)

.github/workflows/autoformat-v2.yml:87

  • git format-patch HEAD~1 --stdout requires the parent commit to be present (which is why this currently needs fetch-depth: 0). If you switch to a shallow fetch, generate the patch from the new commit directly (e.g., format the last commit) so the workflow doesn't depend on having the full history available.
          git commit -m 'Auto-format source code'
          mkdir -p autoformat-output
          git format-patch HEAD~1 --stdout > autoformat-output/autoformat.patch
        fi

Comment thread .github/workflows/autoformat-v2.yml
Comment thread .github/workflows/autoformat-v2.yml
Comment thread .github/workflows/autoformat-v2.yml
rolfbjarne and others added 2 commits May 25, 2026 11:56
- Add permissions: {} at workflow level for least-privilege
- Reduce fetch-depth from 0 to 1 (full clone unnecessary)
- Add repository: for fork checkout to reliably fetch fork code

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These are superseded by autoformat-v2.yml which handles both same-repo
and fork PRs in a single zizmor-clean workflow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
Copy link
Copy Markdown
Member Author

/review

@rolfbjarne rolfbjarne marked this pull request as ready for review May 25, 2026 10:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

.NET for Apple Platforms PR Reviewer completed successfully!

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Review Summary

Overall assessment: Good security improvement

This PR successfully consolidates the two-workflow autoformat setup into a single workflow that addresses the security concerns flagged by zizmor:

  • ✅ Eliminates workflow_run trigger (dangerous-triggers)
  • ✅ All actions pinned to SHA hashes (unpinned-uses)
  • persist-credentials: false on checkout (artipacked)
  • ✅ Minimal job-level permissions (excessive-permissions)

The architecture is clean: same-repo PRs get auto-commit + push, fork PRs get a patch artifact for manual application.

Issues Found

  • 0 ❌ errors
  • 1 ⚠️ warning
  • 2 💡 suggestions

Key Points

Warning:

  • The push authentication pattern needs verification across different branch protection scenarios. The x-access-token:${GH_TOKEN} approach is standard, but confirm it works with the repository's branch protection rules.

Positive aspects:

  • Security improvements are thorough and well-reasoned
  • SHA-pinned actions enhance supply chain security
  • Clear separation between same-repo and fork workflows
  • Minimal permissions following principle of least privilege
  • All logic inlined (no external action dependencies beyond official GitHub actions)

The workflow YAML structure and indentation are correct (2 spaces, following .editorconfig for YAML files).

Generated by .NET for Apple Platforms PR Reviewer for issue #25510 · ● 2.4M

run: |
git config user.email 'github-actions-autoformatter@xamarin.com'
git config user.name 'GitHub Actions Autoformatter'
git remote set-url origin "https://x-access-token:${GH_TOKEN}@github.com/${{ github.repository }}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 ⚠️ Security — The push operation sets credentials via git config and remote URL but uses persist-credentials: false on checkout (line 21). To push successfully, this needs to authenticate the remote URL.

The pattern (xaccesstoken/redacted):${GH_TOKEN}@github.com/... is correct for pushing with a token when persist-credentials: false is used. However, verify that github.token has write permissions to the PR branch. For same-repo PRs on user-owned branches, this should work, but for protected branches or organization-level restrictions, this may fail.

Consider adding error handling or a test to verify this works across different branch protection scenarios.

Rule: Security and workflow reliability

- name: 'Checkout'
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
with:
ref: ${{ github.head_ref }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 💡 Patterns — Using github.head_ref instead of github.event.pull_request.head.ref is more concise and correct for pull request workflows. Good choice.

Note that github.head_ref is only populated for pull_request events, so this workflow is correctly scoped to that trigger.

Rule: GitHub Actions best practices

autoformat-push:
name: Autoformat and push
runs-on: ubuntu-latest
if: github.event.pull_request.head.repo.full_name == github.repository
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 💡 Code organization — The condition check is clear and correct. For fork PRs, github.event.pull_request.head.repo.full_name will differ from github.repository.

One edge case: if the PR is from a branch in the same repo but the head ref gets deleted before this workflow runs, the checkout on line 19 may fail. This is acceptable behavior (the PR is invalid at that point), but consider whether any cleanup or error messaging would be helpful.

Rule: Workflow reliability

@rolfbjarne rolfbjarne merged commit c7e3139 into main May 26, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants