Remove pull_request_target by consolidating stale preview cleanup - #2876
Open
gcl-sekoia wants to merge 1 commit into
Open
Remove pull_request_target by consolidating stale preview cleanup#2876gcl-sekoia wants to merge 1 commit into
gcl-sekoia wants to merge 1 commit into
Conversation
gcl-sekoia
marked this pull request as ready for review
March 19, 2026 12:28
|
Newest code from gcl-sekoia has been published to preview environment 🚀 Latest deployment was built on 2026-08-07 19:00:21 (4502140383512c379a9d3b354a4267b2474f3cab). |
|
This PR was marked as stale because it has been open for 30 days with no activity. |
otetard
approved these changes
Aug 7, 2026
gcl-sekoia
force-pushed
the
consolidate-stale-preview-cleanup
branch
from
August 7, 2026 11:41
b4af4b6 to
e634f09
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the pull_request_target-based stale preview cleanup workflow by consolidating stale-preview deletion into the scheduled mark-as-stale workflow, reducing the pull_request_target attack surface while keeping equivalent behavior.
Changes:
- Deleted
.github/workflows/delete-preview-for-stale-prs.yaml(previously triggered onpull_request_target+stalelabel). - Extended
.github/workflows/mark-as-stale.yamlto extract newly-staled PR numbers fromactions/staleand run a matrix job to delete previews via the existing./.github/actions/delete-previewcomposite action. - Configured the matrix to be non-fail-fast and adjusted checkout options for the deletion job.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/mark-as-stale.yaml | Adds extraction of newly-staled PRs and a matrix-driven delete-stale-previews job to delete previews without pull_request_target. |
| .github/workflows/delete-preview-for-stale-prs.yaml | Removes the old pull_request_target workflow that deleted previews when the stale label was applied. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+41
| run: | | ||
| prs=$(echo "$STALED" | jq -c '[.[] | select(.pull_request != null) | .number]') | ||
| echo "prs=$prs" >> "$GITHUB_OUTPUT" |
…anup Replace the dedicated delete-preview-for-stale-prs workflow (which used pull_request_target) by extending mark-as-stale to delete previews for newly staled PRs directly, using the staled-issues-prs output from actions/stale. This is not the drop-in replacement it looks like. The old workflow could never react to the stale bot: the label was applied with secrets.GITHUB_TOKEN, and token-generated events do not start new workflow runs. Across 175 historical runs it only ever fired for humans labelling by hand -- github-actions[bot] accounts for zero. Scheduled preview cleanup has therefore never worked. Driving it from the action's own output makes it work for the first time, and removes the pull_request_target attack surface. Two consequences worth recording: hand-applying the stale label no longer deletes a preview, and the previews of PRs already labelled stale need a one-off sweep since they will not be re-staled. Also set fail-fast: false so one failed deletion does not cancel the others, and default the extracted list to [] so an unparsable staled-issues-prs warns instead of failing a run that is never retried.
gcl-sekoia
force-pushed
the
consolidate-stale-preview-cleanup
branch
from
August 7, 2026 18:52
e634f09 to
d0818eb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
delete-preview-for-stale-prs.yamlwhich usedpull_request_targetto react to thestalelabel being addedmark-as-stale.yamlto handle preview deletion directly: thestaled-issues-prsoutput fromactions/stalefeeds a matrix job that calls the existingdelete-previewcomposite action for each newly staled PRpull_request_targetnow appears in zero workflows in this repo.This is not the drop-in replacement it looks like
An earlier version of this description claimed the change was behaviour-preserving. That was wrong, and the correction matters more than the change itself.
The old workflow could never react to the stale bot. The label is applied with
secrets.GITHUB_TOKEN, and token-generated events do not start new workflow runs. Across all 175 historical runs of that workflow,github-actions[bot]accounts for zero:All 28 successes were a single manual bulk-labelling session on 2026-01-21. Every run since 2026-02-01 was
skipped, while the stale bot went on labelling PRs throughout.So scheduled stale-preview cleanup has never actually worked. This PR does not preserve that behaviour — it makes it work for the first time. The new path is therefore untested in production and will fire for real on the first
0 3 * * *run after merge; that run is worth watching.Two consequences to decide on separately:
stalewas the only path that ever worked (28 times). If it's worth keeping, aworkflow_dispatchinput taking a PR number is the cheap route that doesn't reintroducepull_request_target.stale. They won't be re-staled, so_markStalenever fires for them and their previews leak indefinitely. Needs a one-offswift deletesweep, out of scope here.Other changes
fail-fast: falseon the matrix: a PR is only ever staled once, so a failed deletion is never retried — one failure must not cancel the other previews[], so an unparsablestaled-issues-prslogs a warning instead of failing a run that cannot be retried. Notejqon empty input exits 0 with no output, so the real hazard wasfromJson('')raising an expression error in the job-levelif:, not ajqcrashpersist-credentials: falseand a.github/actions/sparse checkout, matchingdelete-preview.yamlenv:and quoted rather than interpolated into therunbody — it embeds attacker-controlled PR titles, and this job holds the OVH Swift credentialsKnown limitations (accepted, not fixed here)
delete-preview'sfind-commentstep matches onbody-includes: Newest code from ${{ github.actor }}, but the original comment is written byupload-preview.yamlwheregithub.actoris the PR author, whereas in aschedulerun it is always one fixed account. The match fails,create-or-update-commentfalls through to its create branch, and the original comment survives still linking a deleted prefix. Pre-existing and shared with the merge path, but newly visible now that this path runs.staled-issues-prsis far larger than it looks. TypeScriptprivateis compile-time only, so each element embeds the whole ~60-key options object (includingrepoToken, masked by the runner). At roughly 2–3 KB per item, a single env var hitsMAX_ARG_STRLENsomewhere around 50 staled PRs. Not reachable in steady state; would be reachable if thestalelabel were bulk-removed from the current 36. Never add a step that echoes$STALEDor setsset -x.Test plan
delete-stale-previewsmust expand to exactly those PR numbers, and the preview URL must 404 afterwardspreview-cleanup.yaml→delete-preview.yaml, unchanged)workflow_dispatchwithdebug-only: trueis not a safe dry run:_markStalepopulatesstaled-issues-prsbefore thedebugOnlycheck, so the matrix job would still delete real previews.