[pkaeding]

Why

[Wiz issue f0dca03e](https://app.wiz.io/issues#~(issue~'f0dca03e-007b-49a4-a459-05b4aad4f094) — CRITICAL, "Publicly exposed CI workflow with access to secrets vulnerable to PWN request." Opened 2026-07-31, reopened 2026-08-09.

Heads up for anyone reading that issue cold: the issue's affected entity says .github/workflows/main.yml, but Wiz's own IaC scanner names the real file as .github/workflows/test-fork-pr.yml. main.yml uses pull_request, which never hands secrets to fork code. Don't chase main.yml.

Wiz has four line-level findings, all in test-fork-pr.yml:

Job	Finding	Cleared here?
go-test	untrusted checkout ref: head.sha in a privileged context	✅
go-test	local action uses: ./.github/actions/publish-junit after untrusted checkout	✅
e2e-tests	untrusted checkout ref: head.sha in a privileged context	❌ by design
e2e-tests	local action uses: ./ after untrusted checkout	❌ by design

The finding is a true positive in mechanism. Under pull_request_target, fork-authored code executes before the secret-bearing step, so it can poison $GITHUB_ENV/PATH/the local action definition and capture the secret. The safe-to-test label gate (#238) is a real control, but it is a human-review control — it depends on a maintainer catching a hostile action.yml, .pre-commit-config.yaml, or Dockerfile in the diff.

What changed

The two go-test findings are avoidable, and this PR removes them. Unit tests need no repository secrets — only the "Publish JUnit Tests" step does, and that step already carries its own same-repo if: guard. So fork unit tests never needed pull_request_target:

• main.yml — dropped the job-level same-repo gate on go-test, so fork PRs get unit test coverage under the unprivileged pull_request trigger. The Datadog publish step stays gated, so DATADOG_API_KEY is now never present in any job that runs fork code.
• test-fork-pr.yml — deleted the redundant privileged go-test job; updated check-success's needs:.
• All checkouts of fork-authored code — added persist-credentials: false, so poisoned code can't reuse the workflow token out of .git/config.

Net: 4 findings → 2, and DATADOG_API_KEY leaves the privileged blast radius entirely.

What this PR does not fix, and why

The e2e-tests job is irreducibly privileged. Its entire purpose is to run the fork's own action code against the real LaunchDarkly API, so it needs both the untrusted checkout and LD_ACCESS_TOKEN_WRITER in the same job. Wiz's suggested Strategies A/B/C (check out base instead of head; isolate fork code to a subdirectory; hand off artifacts via workflow_run) all break the test's purpose — there is nothing to test if you don't run the fork's code. So the two e2e-tests findings survive this PR by design, still mitigated by the label gate.

So merging this PR will not close the Wiz issue, and that's expected. I checked how this rule has been closed before: tenant-wide it has been resolved exactly once, as ISSUE_FIXED, and the resolution evidence is deletion of the IAC_FINDING resource — i.e. the pattern was removed from the YAML. It has never been closed via a compensating control. Wiz's two remaining checks are IaC pattern matchers on pull_request_target + ref: head.sha + uses: ./, so any runtime control leaves them matching. Closing the residual will need an explicit risk acceptance in Wiz, not another workflow edit. ProdSec (me) will handle that side.

Optional hardening worth considering on its own merits (not a route to closing the Wiz findings): move LD_ACCESS_TOKEN_WRITER into a GitHub Environment with required reviewers and add environment: fork-e2e to e2e-tests. Per GitHub's docs, "a job that references an environment must follow any protection rules for the environment before running or accessing the environment's secrets" — the job is held in Waiting before any step, so the untrusted checkout never happens without a per-run human approval. That's genuinely stronger than the label gate: it can't go stale and can't be raced by a push. I didn't wire it here because an Environment with no protection rules is a silent no-op that looks like a control, and picking the reviewer set is your call. Happy to open that PR if you want it.

Also worth confirming: LD_ACCESS_TOKEN_WRITER should be scoped to the developer-toolbar-sandbox project only. If it already is, the residual blast radius is one sandbox project and the remaining risk is low.

On secret rotation

Wiz's remediation plan says rotate both secrets. I checked whether the exposure was ever actually exercised, since exploitation requires a maintainer to have labeled a hostile fork PR safe-to-test:

• Only two fork PRs have ever carried the label: feat: allow skip posting comment #234 and chore: bump code refs version #235, both from the Fieldguide fork, both authored by @jazanne.
• All six test-fork-pr.yml runs to date were on those two PRs, labeled by @cspath1 and @tknopp-ld.

So no untrusted third-party code has ever run in the privileged context. Rotation is precautionary, not incident-driven — your call whether it's worth the churn. Nothing here indicates compromise.

Review notes / risk

• go-test is a required status check. It's still produced for fork PRs — by main.yml now instead of test-fork-pr.yml — and it no longer needs a maintainer label to report, which is a small improvement to fork contributor experience. Check Success (Fork) is not a required check, so the needs: change is safe.
• More CI on unreviewed forks? No. The repo already has Actions → approval_policy: all_external_contributors, so an external fork PR can't run any workflow until a maintainer approves the run.
• Lost capability: fork PRs no longer publish JUnit results to Datadog. They already didn't before this PR (the step was gated), so this is unchanged in practice.
• One thing I could not exercise, please sanity-check in review: test-fork-pr.yml only fires on pull_request_target + labeled + fork head, so this PR (same-repo) never ran it — the green e2e-tests check above is main.yml's job. That means the persist-credentials: false I added directly above uses: ./ is unexercised. The go-test/generate-docs runs here do prove persist-credentials: false survives go mod tidy and pre-commit run --all-files, but not that step. You'd know better than I do whether ld-find-code-refs needs a persisted git credential; if it does, drop that one line. Alternatively label a throwaway fork PR safe-to-test to confirm.
• Verified with actionlint: no new findings. The SC2209 count actually drops 2 → 1 because the duplicated test block is gone. Remaining shellcheck nits are pre-existing and on untouched lines.

via LD Research 🤖