Azure / Azure/azure-quickstart-templates
commit-generated-on-merge skipped for fork PRs leaves master without azuredeploy.json
- Dominant language
- Bicep
- Stars
- 14.9k
- Forks
- 16.2k
- Avg merge
- 6d 21h
- Merged PRs (30d)
- 6
Description
## Summary
`commit-generated-on-merge` (in `.github/workflows/ValidateSampleDeployments.yml`) is gated on `head.repo.full_name == github.repository`, which means it doesn't run for PRs from forks. This blocks the canonical contributor path: external (and most Microsoft-owned) contributors submit from a fork of `Azure/azure-quickstart-templates`, not from an upstream branch. When their PR merges, master is left without `azuredeploy.json` for the changed sample even though `/validate` already produced a valid one.
The inline comment on the job suggests this guard is intended as belt-and-braces:
> Fork PRs cannot reach this workflow at all (the /validate path requires ADX access), so the same-repo guard is defensive belt-and-braces only.
The premise of that comment isn't accurate today. Fork PRs can and do reach `/validate` — it's `issue_comment`-triggered, so the workflow runs in the upstream repository's context with upstream secrets and the `adx-readonly` environment. The validation succeeds and uploads `generated-azuredeploy--` to upstream's actions storage like any in-repo PR.
## Evidence
Concrete example: PR #14762 (from `msmbaldwin/azure-quickstart-templates`, a fork). The successful `/validate` run uploaded:
```bash
$ gh api "/repos/Azure/azure-quickstart-templates/actions/artifacts?name=generated-azuredeploy-14762-855ec9eaf6cec5e1701a95234eba43396710804c"
{
"id": 6979384358,
"name": "generated-azuredeploy-14762-855ec9eaf6cec5e1701a95234eba43396710804c",
"expired": false,
"size_in_bytes": 4382,
"workflow_run_id": 25820961383,
"created_at": "2026-05-13T19:17:37Z"
}
```
The artifact is sitting there, valid, with the path structure that #14754 fixed and with the metadata that d1f0fd9 looks up by name. The only thing that prevents `commit-generated-on-merge` from picking it up on merge is the fork-repo guard.
## Impact
For every fork PR that touches a sample template:
1. Contributor follows the (current) contribution guide, which says `azuredeploy.json` "must not be included in the PR as it will be built automatically when the sample is merged."
2. `/validate` succeeds, artifact uploads, maintainer approves and merges.
3. `commit-generated-on-merge` is skipped because of the fork gate. Master is left without `azuredeploy.json` for the sample.
4. The sample's "Deploy to Azure" button breaks until someone manually restores the JSON.
This happened on #14762 and triggered the manual-recovery loop in #14764 / #14765. Each fork PR that touches templates will hit it again.
## Suggested fix
Drop the fork-repo guard:
```diff
commit-generated-on-merge:
name: Commit generated azuredeploy.json on merge
runs-on: ubuntu-latest
if: >-
github.event_name == 'pull_request' &&
github.event.action == 'closed' &&
github.event.pull_request.merged == true &&
- github.event.pull_request.base.ref == github.event.repository.default_branch &&
- github.event.pull_request.head.repo.full_name == github.repository
+ github.event.pull_request.base.ref == github.event.repository.default_branch
```
### Safety analysis
Two concerns worth being explicit about, both of which seem fine to me:
1. **Token / push scope.** The merge event fires in upstream context, so `GITHUB_TOKEN` is upstream's repo-scoped token with `contents: write`. No fork-side credentials are involved at any point. The push to master uses the same identity it does for in-repo PRs today.
2. **Content trust.** The artifact's `azuredeploy.json` is produced by `selected-pipeline`'s deterministic `bicep build` of the PR's `main.bicep`, then validated against ADX (correlationId + templateHash match against an actual deployment). The trust boundary is maintainer review of the `main.bicep` change before merge — exactly the same boundary as for in-repo PRs. There's no path for a malicious contributor to substitute a different JSON; the artifact is built by upstream CI from PR content the maintainer has already approved.
If there's a residual concern, an alternative is to require the artifact's `workflow_run_id` to have been triggered by a MEMBER/OWNER/COLLABORATOR (which `/validate` already enforces via its commenter check). But I don't think that's necessary — the existing approval-then-merge flow is sufficient.
## Related
- #14765 (artifact-by-name lookup, fixed in d1f0fd9) — this issue is the second half of fully unblocking the fork-PR path.
- #14754 (artifact upload path stripping, fixed in ca5c5299) — preserves the repo-relative path so the on-merge step can land the file in the right folder.
- #14753 (Bicep version-skew in templateHash, fixed in b7d5acb) — version-pinning at hash time also removes a fork-contributor friction point. With this and d1f0fd9 in place, the `/validate` path works end-to-end for fork PRs; the fork gate is the last barrier to a clean fork → merge flow.
- #14762 / #14764 (the canary that initially surfaced the fork-gate behavior).
cc @ouldsid
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.