From 766179e0511882f894fd3e543cbb782197e359ba Mon Sep 17 00:00:00 2001 From: Ryan Ghods Date: Sat, 22 Aug 2026 13:42:22 -0700 Subject: [PATCH 1/3] fix(ci): restrict the !fix autofix workflow to maintainers and repo branches Autofix Linting triggers on issue_comment, which runs in the base repository context with contents: write. It gated only on the comment body, so any GitHub user could start it, and it resolved the pull request head branch by name against this repository. On 2026-05-22 an outside account opened a fork pull request whose head branch was named main, commented !fix, and the job checked out this repository's main, ran forge fmt and pushed two commits to it (49e1a78, 4cbaa91). - gate on comment author_association so only OWNER, MEMBER or COLLABORATOR can trigger the job - refuse fork pull requests and refuse the default branch - write the branch through $GITHUB_OUTPUT. ::set-output still works on hosted runners, so this is hygiene rather than the bug - state the whole if condition as one expression. The old form mixed a bare operand with ${{ ... }}, which compiles to a format() call whose non-empty string result is always truthy - quote every expansion, pass event data through env, validate the branch name - drop checks: write, default the workflow to no permissions, and scope contents: write plus pull-requests: read to the job - upgrade and pin actions/checkout and foundry-rs/foundry-toolchain by SHA - use printf so .git-blame-ignore-revs entries get a real newline Co-Authored-By: Claude Opus 5 --- .github/workflows/fix-lint.yml | 118 +++++++++++++++++++++++++-------- .github/workflows/lint.yml | 2 +- 2 files changed, 92 insertions(+), 28 deletions(-) diff --git a/.github/workflows/fix-lint.yml b/.github/workflows/fix-lint.yml index f1fe6a0..fb58883 100644 --- a/.github/workflows/fix-lint.yml +++ b/.github/workflows/fix-lint.yml @@ -4,52 +4,116 @@ on: issue_comment: types: [created] -# Down scope as necessary via https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token -permissions: - checks: write - contents: write - pull-requests: write +# An issue_comment event always runs in the base repository context, on the +# default branch's copy of this file, with this repository's GITHUB_TOKEN. It +# never runs in the context of the pull request being commented on. Anything +# this workflow checks out therefore runs as trusted code, so the job below is +# restricted to commenters with write access and to head branches that live in +# this repository. +permissions: {} jobs: run-linters: name: Run linters - if: github.event.issue.pull_request && ${{ github.event.comment.body == '!fix' }} + # Keep the whole condition inside a single expression. A value that mixes + # bare operands with ${{ ... }} is compiled into a format() call whose + # result is a non-empty string, and a non-empty string is truthy, so such a + # condition never rejects anything. + if: >- + github.event.issue.pull_request != null + && github.event.comment.body == '!fix' + && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: write + pull-requests: read + concurrency: + group: autofix-linting-${{ github.event.issue.number }} + cancel-in-progress: false steps: - - name: GetBranch - id: 'get-branch' - run: echo ::set-output name=branch::$(gh pr view $PR_NO --repo $REPO --json headRefName --jq '.headRefName') + - name: Resolve the pull request head branch + id: get-branch env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPO: ${{ github.repository }} PR_NO: ${{ github.event.issue.number }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: | + set -euo pipefail + + pr_json="$(gh pr view "$PR_NO" --repo "$REPO" --json headRefName,isCrossRepository)" + branch="$(jq -r '.headRefName // ""' <<<"$pr_json")" + cross_repository="$(jq -r '.isCrossRepository' <<<"$pr_json")" + + if [[ -z "$branch" ]]; then + echo "::error::Could not resolve the head branch of PR #${PR_NO}." + exit 1 + fi + + # A fork's head branch cannot be pushed to with this repository's + # GITHUB_TOKEN, and checking it out here would execute fork code in a + # trusted context. actions/checkout refuses that only for + # pull_request_target and workflow_run, so refuse it explicitly. + # Without this check a fork branch that shares a name with a branch + # here (for example "main") is silently resolved against this + # repository and formatted and pushed instead. + if [[ "$cross_repository" != "false" ]]; then + echo "::error::PR #${PR_NO} comes from a fork. Autofix only runs on branches in ${REPO}; run 'forge fmt' locally and push instead." + exit 1 + fi + + if [[ "$branch" == "$DEFAULT_BRANCH" ]]; then + echo "::error::Refusing to autofix ${DEFAULT_BRANCH}." + exit 1 + fi + + # git accepts ';', '$', backticks and similar in a branch name. None + # of them reach a shell below, but keeping the name to an ordinary + # character set means a surprising name fails loudly here rather than + # somewhere later. + if [[ ! "$branch" =~ ^[A-Za-z0-9._/-]+$ ]] || ! git check-ref-format "refs/heads/$branch"; then + echo "::error::Refusing to act on head branch name '${branch}'." + exit 1 + fi + + printf 'branch=%s\n' "$branch" >> "$GITHUB_OUTPUT" - name: Check out Git repository - uses: actions/checkout@v3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ steps.get-branch.outputs.branch }} - - name: Install Foundry - uses: foundry-rs/foundry-toolchain@v1 + uses: foundry-rs/foundry-toolchain@908c540300062bd5a7e473851cdb4282204cee09 # v1.9.1 with: version: nightly - + - name: Lint + env: + BRANCH_NAME: ${{ steps.get-branch.outputs.branch }} run: | + set -euo pipefail + forge fmt - pwd - if [[ `git diff --exit-code` ]]; then - git config --local user.name 'GitHub Actions Bot' - git config --local user.email '<>' - git add . - git commit -m "Github Actions automatically updated formatting with forge fmt" - COMMIT_HASH=$(git rev-parse HEAD) - echo "# Github Actions automatically updated formatting with forge fmt\n$COMMIT_HASH" >> .git-blame-ignore-revs - git add .git-blame-ignore-revs - git commit -m "Updated .git-blame-ignore-revs with commit $COMMIT_HASH" - BRANCH_NAME=$(git symbolic-ref --short HEAD) - git push origin $BRANCH_NAME + + if git diff --quiet; then + echo "forge fmt made no changes." + exit 0 fi - id: update + + git config --local user.name 'GitHub Actions Bot' + git config --local user.email '41898282+github-actions[bot]@users.noreply.github.com' + git add -A + git commit -m 'Github Actions automatically updated formatting with forge fmt' + + commit_hash="$(git rev-parse HEAD)" + # printf, not echo: echo writes "\n" literally, which put the comment + # and the revision on one line and made every entry in + # .git-blame-ignore-revs a no-op. + printf '# Github Actions automatically updated formatting with forge fmt\n%s\n' "$commit_hash" >> .git-blame-ignore-revs + git add .git-blame-ignore-revs + git commit -m "Updated .git-blame-ignore-revs with commit ${commit_hash}" + + git push origin "HEAD:refs/heads/${BRANCH_NAME}" diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 3101011..1b75c05 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -32,6 +32,6 @@ jobs: run: | echo "Running forge fmt --check" if ! forge fmt --check; then - echo "The linting check failed. You can fix it locally with 'forge fmt' and then push, or you can have a GitHub action take care of it for you by commenting '!fix' on the PR." + echo "The linting check failed. You can fix it locally with 'forge fmt' and then push. Maintainers can also comment '!fix' on a pull request whose branch lives in this repository to have a GitHub action do it." exit 1 fi \ No newline at end of file From c22988c4e925cb939f10cc866f38456d2a4cde9f Mon Sep 17 00:00:00 2001 From: Ryan Ghods Date: Sat, 22 Aug 2026 13:45:40 -0700 Subject: [PATCH 2/3] docs(ci): state the author_association allowlist exactly in the header comment Co-Authored-By: Claude Opus 5 --- .github/workflows/fix-lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/fix-lint.yml b/.github/workflows/fix-lint.yml index fb58883..8ba2bbe 100644 --- a/.github/workflows/fix-lint.yml +++ b/.github/workflows/fix-lint.yml @@ -8,8 +8,8 @@ on: # default branch's copy of this file, with this repository's GITHUB_TOKEN. It # never runs in the context of the pull request being commented on. Anything # this workflow checks out therefore runs as trusted code, so the job below is -# restricted to commenters with write access and to head branches that live in -# this repository. +# restricted to OWNER, MEMBER and COLLABORATOR comments and to head branches +# that live in this repository. permissions: {} jobs: From cc078aa65e5023234fb4e5687695d5267f2b4f8e Mon Sep 17 00:00:00 2001 From: Ryan Ghods Date: Sat, 22 Aug 2026 14:02:28 -0700 Subject: [PATCH 3/3] ci: remove the !fix autofix workflow, enforce formatting in lint instead The autofix workflow was reachable by anyone who could comment. On 2026-05-22 an external user with author_association NONE commented "!fix" on a fork pull request whose head branch was named "main"; checkout defaults `repository` to the base repo, so `ref: main` resolved against shipyard and the bot committed and pushed 49e1a78 and 4cbaa91 to this repository's default branch. Hardening it means keeping an issue_comment trigger with contents: write that checks out a pull request's code. Deleting it removes that surface entirely and costs a convenience that "forge fmt" already provides locally. Formatting is still enforced, by the lint workflow, which now fails the build instead of pointing at the bot. That workflow also stops overriding the checkout ref with github.head_ref, which is the same bare-branch-name resolution that made the May incident possible, and now declares permissions: contents: read. --- .github/workflows/fix-lint.yml | 119 --------------------------------- .github/workflows/lint.yml | 23 ++++--- 2 files changed, 15 insertions(+), 127 deletions(-) delete mode 100644 .github/workflows/fix-lint.yml diff --git a/.github/workflows/fix-lint.yml b/.github/workflows/fix-lint.yml deleted file mode 100644 index 8ba2bbe..0000000 --- a/.github/workflows/fix-lint.yml +++ /dev/null @@ -1,119 +0,0 @@ -name: Autofix Linting - -on: - issue_comment: - types: [created] - -# An issue_comment event always runs in the base repository context, on the -# default branch's copy of this file, with this repository's GITHUB_TOKEN. It -# never runs in the context of the pull request being commented on. Anything -# this workflow checks out therefore runs as trusted code, so the job below is -# restricted to OWNER, MEMBER and COLLABORATOR comments and to head branches -# that live in this repository. -permissions: {} - -jobs: - run-linters: - name: Run linters - # Keep the whole condition inside a single expression. A value that mixes - # bare operands with ${{ ... }} is compiled into a format() call whose - # result is a non-empty string, and a non-empty string is truthy, so such a - # condition never rejects anything. - if: >- - github.event.issue.pull_request != null - && github.event.comment.body == '!fix' - && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) - runs-on: ubuntu-latest - timeout-minutes: 15 - permissions: - contents: write - pull-requests: read - concurrency: - group: autofix-linting-${{ github.event.issue.number }} - cancel-in-progress: false - - steps: - - name: Resolve the pull request head branch - id: get-branch - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} - PR_NO: ${{ github.event.issue.number }} - DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} - run: | - set -euo pipefail - - pr_json="$(gh pr view "$PR_NO" --repo "$REPO" --json headRefName,isCrossRepository)" - branch="$(jq -r '.headRefName // ""' <<<"$pr_json")" - cross_repository="$(jq -r '.isCrossRepository' <<<"$pr_json")" - - if [[ -z "$branch" ]]; then - echo "::error::Could not resolve the head branch of PR #${PR_NO}." - exit 1 - fi - - # A fork's head branch cannot be pushed to with this repository's - # GITHUB_TOKEN, and checking it out here would execute fork code in a - # trusted context. actions/checkout refuses that only for - # pull_request_target and workflow_run, so refuse it explicitly. - # Without this check a fork branch that shares a name with a branch - # here (for example "main") is silently resolved against this - # repository and formatted and pushed instead. - if [[ "$cross_repository" != "false" ]]; then - echo "::error::PR #${PR_NO} comes from a fork. Autofix only runs on branches in ${REPO}; run 'forge fmt' locally and push instead." - exit 1 - fi - - if [[ "$branch" == "$DEFAULT_BRANCH" ]]; then - echo "::error::Refusing to autofix ${DEFAULT_BRANCH}." - exit 1 - fi - - # git accepts ';', '$', backticks and similar in a branch name. None - # of them reach a shell below, but keeping the name to an ordinary - # character set means a surprising name fails loudly here rather than - # somewhere later. - if [[ ! "$branch" =~ ^[A-Za-z0-9._/-]+$ ]] || ! git check-ref-format "refs/heads/$branch"; then - echo "::error::Refusing to act on head branch name '${branch}'." - exit 1 - fi - - printf 'branch=%s\n' "$branch" >> "$GITHUB_OUTPUT" - - - name: Check out Git repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - ref: ${{ steps.get-branch.outputs.branch }} - - - name: Install Foundry - uses: foundry-rs/foundry-toolchain@908c540300062bd5a7e473851cdb4282204cee09 # v1.9.1 - with: - version: nightly - - - name: Lint - env: - BRANCH_NAME: ${{ steps.get-branch.outputs.branch }} - run: | - set -euo pipefail - - forge fmt - - if git diff --quiet; then - echo "forge fmt made no changes." - exit 0 - fi - - git config --local user.name 'GitHub Actions Bot' - git config --local user.email '41898282+github-actions[bot]@users.noreply.github.com' - git add -A - git commit -m 'Github Actions automatically updated formatting with forge fmt' - - commit_hash="$(git rev-parse HEAD)" - # printf, not echo: echo writes "\n" literally, which put the comment - # and the revision on one line and made every entry in - # .git-blame-ignore-revs a no-op. - printf '# Github Actions automatically updated formatting with forge fmt\n%s\n' "$commit_hash" >> .git-blame-ignore-revs - git add .git-blame-ignore-revs - git commit -m "Updated .git-blame-ignore-revs with commit ${commit_hash}" - - git push origin "HEAD:refs/heads/${BRANCH_NAME}" diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 1b75c05..f89fe9d 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,6 +7,10 @@ on: pull_request: types: [opened, reopened, synchronize] +# Read-only. This workflow reports on formatting, it does not change anything. +permissions: + contents: read + env: FOUNDRY_PROFILE: ci @@ -18,20 +22,23 @@ jobs: name: Foundry project runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + # No `ref:` override. On a pull_request event checkout defaults to the + # merge commit, which is what CI should be judging. Naming + # `github.head_ref` instead resolves a bare branch name against this + # repository, so a fork PR whose branch is called `main` gets this + # repository's `main` checked out rather than the contributor's code. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ github.head_ref }} submodules: recursive - name: Install Foundry - uses: foundry-rs/foundry-toolchain@v1 + uses: foundry-rs/foundry-toolchain@908c540300062bd5a7e473851cdb4282204cee09 # v1.9.1 with: version: nightly - - name: Lint + - name: Check formatting run: | - echo "Running forge fmt --check" - if ! forge fmt --check; then - echo "The linting check failed. You can fix it locally with 'forge fmt' and then push. Maintainers can also comment '!fix' on a pull request whose branch lives in this repository to have a GitHub action do it." + if ! forge fmt --check; then + echo "::error::Formatting check failed. Run 'forge fmt' locally and commit the result." exit 1 - fi \ No newline at end of file + fi