More secure auto-fixer (#44952)

Split running `cargo clippy` out of the job that has access to ZIPPY
secrets as
a precaution against accidentally leaking the secrets through build.rs
or
something...

Release Notes:

- N/A

.github/workflows/autofix_pr.yml

@@ -9,26 +9,23 @@ on:
         description: pr_number
         required: true
         type: string
+      run_clippy:
+        description: run_clippy
+        type: boolean
+        default: 'true'
 jobs:
   run_autofix:
     runs-on: namespace-profile-16x32-ubuntu-2204
     steps:
-    - id: get-app-token
-      name: autofix_pr::run_autofix::authenticate_as_zippy
-      uses: actions/create-github-app-token@bef1eaf1c0ac2b148ee2a0a74c65fbe6db0631f1
-      with:
-        app-id: ${{ secrets.ZED_ZIPPY_APP_ID }}
-        private-key: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }}
-    - name: steps::checkout_repo_with_token
+    - name: steps::checkout_repo
       uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
       with:
         clean: false
-        token: ${{ steps.get-app-token.outputs.token }}
     - name: autofix_pr::run_autofix::checkout_pr
       run: gh pr checkout ${{ inputs.pr_number }}
       shell: bash -euxo pipefail {0}
       env:
-        GITHUB_TOKEN: ${{ steps.get-app-token.outputs.token }}
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
     - name: steps::setup_cargo_config
       run: |
         mkdir -p ./../.cargo
@@ -58,26 +55,71 @@ jobs:
       run: cargo fmt --all
       shell: bash -euxo pipefail {0}
     - name: autofix_pr::run_autofix::run_clippy_fix
+      if: ${{ inputs.run_clippy }}
       run: cargo clippy --workspace --release --all-targets --all-features --fix --allow-dirty --allow-staged
       shell: bash -euxo pipefail {0}
-    - name: autofix_pr::run_autofix::commit_and_push
+    - id: create-patch
+      name: autofix_pr::run_autofix::create_patch
       run: |
         if git diff --quiet; then
             echo "No changes to commit"
+            echo "has_changes=false" >> "$GITHUB_OUTPUT"
         else
-            git add -A
-            git commit -m "Autofix"
-            git push
+            git diff > autofix.patch
+            echo "has_changes=true" >> "$GITHUB_OUTPUT"
         fi
       shell: bash -euxo pipefail {0}
+    - name: upload artifact autofix-patch
+      uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4
+      with:
+        name: autofix-patch
+        path: autofix.patch
+        if-no-files-found: ignore
+        retention-days: '1'
+    - name: steps::cleanup_cargo_config
+      if: always()
+      run: |
+        rm -rf ./../.cargo
+      shell: bash -euxo pipefail {0}
+    outputs:
+      has_changes: ${{ steps.create-patch.outputs.has_changes }}
+  commit_changes:
+    needs:
+    - run_autofix
+    if: needs.run_autofix.outputs.has_changes == 'true'
+    runs-on: namespace-profile-2x4-ubuntu-2404
+    steps:
+    - id: get-app-token
+      name: autofix_pr::commit_changes::authenticate_as_zippy
+      uses: actions/create-github-app-token@bef1eaf1c0ac2b148ee2a0a74c65fbe6db0631f1
+      with:
+        app-id: ${{ secrets.ZED_ZIPPY_APP_ID }}
+        private-key: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }}
+    - name: steps::checkout_repo_with_token
+      uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
+      with:
+        clean: false
+        token: ${{ steps.get-app-token.outputs.token }}
+    - name: autofix_pr::commit_changes::checkout_pr
+      run: gh pr checkout ${{ inputs.pr_number }}
+      shell: bash -euxo pipefail {0}
+      env:
+        GITHUB_TOKEN: ${{ steps.get-app-token.outputs.token }}
+    - name: autofix_pr::download_patch_artifact
+      uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53
+      with:
+        name: autofix-patch
+    - name: autofix_pr::commit_changes::apply_patch
+      run: git apply autofix.patch
+      shell: bash -euxo pipefail {0}
+    - name: autofix_pr::commit_changes::commit_and_push
+      run: |
+        git commit -am "Autofix"
+        git push
+      shell: bash -euxo pipefail {0}
       env:
         GIT_COMMITTER_NAME: Zed Zippy
         GIT_COMMITTER_EMAIL: 234243425+zed-zippy[bot]@users.noreply.github.com
         GIT_AUTHOR_NAME: Zed Zippy
         GIT_AUTHOR_EMAIL: 234243425+zed-zippy[bot]@users.noreply.github.com
         GITHUB_TOKEN: ${{ steps.get-app-token.outputs.token }}
-    - name: steps::cleanup_cargo_config
-      if: always()
-      run: |
-        rm -rf ./../.cargo
-      shell: bash -euxo pipefail {0}

tooling/xtask/src/tasks/workflows/autofix_pr.rs

@@ -8,16 +8,101 @@ use crate::tasks::workflows::{
 
 pub fn autofix_pr() -> Workflow {
     let pr_number = WorkflowInput::string("pr_number", None);
-    let autofix = run_autofix(&pr_number);
+    let run_clippy = WorkflowInput::bool("run_clippy", Some(true));
+    let run_autofix = run_autofix(&pr_number, &run_clippy);
+    let commit_changes = commit_changes(&pr_number, &run_autofix);
     named::workflow()
         .run_name(format!("autofix PR #{pr_number}"))
         .on(Event::default().workflow_dispatch(
-            WorkflowDispatch::default().add_input(pr_number.name, pr_number.input()),
+            WorkflowDispatch::default()
+                .add_input(pr_number.name, pr_number.input())
+                .add_input(run_clippy.name, run_clippy.input()),
         ))
-        .add_job(autofix.name, autofix.job)
+        .add_job(run_autofix.name.clone(), run_autofix.job)
+        .add_job(commit_changes.name, commit_changes.job)
 }
 
-fn run_autofix(pr_number: &WorkflowInput) -> NamedJob {
+const PATCH_ARTIFACT_NAME: &str = "autofix-patch";
+const PATCH_FILE_PATH: &str = "autofix.patch";
+
+fn upload_patch_artifact() -> Step<Use> {
+    Step::new(format!("upload artifact {}", PATCH_ARTIFACT_NAME))
+        .uses(
+            "actions",
+            "upload-artifact",
+            "330a01c490aca151604b8cf639adc76d48f6c5d4", // v5
+        )
+        .add_with(("name", PATCH_ARTIFACT_NAME))
+        .add_with(("path", PATCH_FILE_PATH))
+        .add_with(("if-no-files-found", "ignore"))
+        .add_with(("retention-days", "1"))
+}
+
+fn download_patch_artifact() -> Step<Use> {
+    named::uses(
+        "actions",
+        "download-artifact",
+        "018cc2cf5baa6db3ef3c5f8a56943fffe632ef53", // v6.0.0
+    )
+    .add_with(("name", PATCH_ARTIFACT_NAME))
+}
+
+fn run_autofix(pr_number: &WorkflowInput, run_clippy: &WorkflowInput) -> NamedJob {
+    fn checkout_pr(pr_number: &WorkflowInput) -> Step<Run> {
+        named::bash(&format!("gh pr checkout {pr_number}"))
+            .add_env(("GITHUB_TOKEN", vars::GITHUB_TOKEN))
+    }
+
+    fn run_cargo_fmt() -> Step<Run> {
+        named::bash("cargo fmt --all")
+    }
+
+    fn run_clippy_fix() -> Step<Run> {
+        named::bash(
+            "cargo clippy --workspace --release --all-targets --all-features --fix --allow-dirty --allow-staged",
+        )
+    }
+
+    fn run_prettier_fix() -> Step<Run> {
+        named::bash("./script/prettier --write")
+    }
+
+    fn create_patch() -> Step<Run> {
+        named::bash(indoc::indoc! {r#"
+            if git diff --quiet; then
+                echo "No changes to commit"
+                echo "has_changes=false" >> "$GITHUB_OUTPUT"
+            else
+                git diff > autofix.patch
+                echo "has_changes=true" >> "$GITHUB_OUTPUT"
+            fi
+        "#})
+        .id("create-patch")
+    }
+
+    named::job(
+        Job::default()
+            .runs_on(runners::LINUX_DEFAULT)
+            .outputs([(
+                "has_changes".to_owned(),
+                "${{ steps.create-patch.outputs.has_changes }}".to_owned(),
+            )])
+            .add_step(steps::checkout_repo())
+            .add_step(checkout_pr(pr_number))
+            .add_step(steps::setup_cargo_config(runners::Platform::Linux))
+            .add_step(steps::cache_rust_dependencies_namespace())
+            .map(steps::install_linux_dependencies)
+            .add_step(steps::setup_pnpm())
+            .add_step(run_prettier_fix())
+            .add_step(run_cargo_fmt())
+            .add_step(run_clippy_fix().if_condition(Expression::new(run_clippy.to_string())))
+            .add_step(create_patch())
+            .add_step(upload_patch_artifact())
+            .add_step(steps::cleanup_cargo_config(runners::Platform::Linux)),
+    )
+}
+
+fn commit_changes(pr_number: &WorkflowInput, autofix_job: &NamedJob) -> NamedJob {
     fn authenticate_as_zippy() -> (Step<Use>, StepOutput) {
         let step = named::uses(
             "actions",
@@ -35,29 +120,14 @@ fn run_autofix(pr_number: &WorkflowInput) -> NamedJob {
         named::bash(&format!("gh pr checkout {pr_number}")).add_env(("GITHUB_TOKEN", token))
     }
 
-    fn run_cargo_fmt() -> Step<Run> {
-        named::bash("cargo fmt --all")
-    }
-
-    fn run_clippy_fix() -> Step<Run> {
-        named::bash(
-            "cargo clippy --workspace --release --all-targets --all-features --fix --allow-dirty --allow-staged",
-        )
-    }
-
-    fn run_prettier_fix() -> Step<Run> {
-        named::bash("./script/prettier --write")
+    fn apply_patch() -> Step<Run> {
+        named::bash("git apply autofix.patch")
     }
 
     fn commit_and_push(token: &StepOutput) -> Step<Run> {
         named::bash(indoc::indoc! {r#"
-            if git diff --quiet; then
-                echo "No changes to commit"
-            else
-                git add -A
-                git commit -m "Autofix"
-                git push
-            fi
+            git commit -am "Autofix"
+            git push
         "#})
         .add_env(("GIT_COMMITTER_NAME", "Zed Zippy"))
         .add_env((
@@ -76,18 +146,17 @@ fn run_autofix(pr_number: &WorkflowInput) -> NamedJob {
 
     named::job(
         Job::default()
-            .runs_on(runners::LINUX_DEFAULT)
+            .runs_on(runners::LINUX_SMALL)
+            .needs(vec![autofix_job.name.clone()])
+            .cond(Expression::new(format!(
+                "needs.{}.outputs.has_changes == 'true'",
+                autofix_job.name
+            )))
             .add_step(authenticate)
             .add_step(steps::checkout_repo_with_token(&token))
             .add_step(checkout_pr(pr_number, &token))
-            .add_step(steps::setup_cargo_config(runners::Platform::Linux))
-            .add_step(steps::cache_rust_dependencies_namespace())
-            .map(steps::install_linux_dependencies)
-            .add_step(steps::setup_pnpm())
-            .add_step(run_prettier_fix())
-            .add_step(run_cargo_fmt())
-            .add_step(run_clippy_fix())
-            .add_step(commit_and_push(&token))
-            .add_step(steps::cleanup_cargo_config(runners::Platform::Linux)),
+            .add_step(download_patch_artifact())
+            .add_step(apply_patch())
+            .add_step(commit_and_push(&token)),
     )
 }