Skip to content

Fix actions concurrency groups cross-branch leak (#37311)#37331

Merged
silverwind merged 1 commit into
go-gitea:release/v1.26from
GiteaBot:backport-37311-v1.26
Apr 21, 2026
Merged

Fix actions concurrency groups cross-branch leak (#37311)#37331
silverwind merged 1 commit into
go-gitea:release/v1.26from
GiteaBot:backport-37311-v1.26

Conversation

@GiteaBot
Copy link
Copy Markdown
Collaborator

Backport #37311 by @silverwind

Problem

Workflow-level concurrency groups were evaluated — and jobs were parsed — before the run was persisted, so run.ID was 0 and github.run_id in the expression context resolved to an empty string. Expressions like:

concurrency:
  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
  cancel-in-progress: true

collapsed to <workflow>- on every push event (head_ref is empty on push), so cancel-in-progress cancelled in-progress runs across unrelated branches, not just the current one.

Reproduced on a 1.26 instance:

  • push to masterci run starts
  • push to feature-branch → the master run gets cancelled

GitHub Actions' documented semantic: on push events github.run_id is unique per run, so the group is unique → no cancellation; on PR events github.head_ref is the source branch → cancellation is per-PR.

Fix

Insert the run before parsing jobs or evaluating workflow-level concurrency, so run.ID is populated in time for every expression that reads github.run_id — not just the concurrency group, but also run-name, job names, and runs-on.

jobparser.Parse now runs inside the InsertRun transaction, after db.Insert(ctx, run). Workflow-level concurrency evaluation runs next and only mutates run in memory. All concurrency-derived fields (raw_concurrency, concurrency_group, concurrency_cancel) plus status and title are persisted in a single final UpdateRun at end-of-transaction — one INSERT + one UPDATE per run in both the concurrency and non-concurrency paths (matches pre-branch parity, one fewer UpdateRepoRunsNumbers COUNT than the interim state).

GenerateGiteaContext now sets run_id from run.ID unconditionally; every caller passes a persisted run.

Verification: tested end-to-end on a 1.26 deployment. Before the patch, two successive ci pushes (one to master, one to a feature branch) cross-cancelled each other. After the patch, the same pushes — in both orders (master→branch, branch→master) — run to completion simultaneously across 15+ runs with zero cancellations.

Regression tests in services/actions/context_test.go:

  • TestEvaluateRunConcurrency_RunIDFallback — unit check that EvaluateRunConcurrencyFillModel resolves github.run_id from run.ID.
  • TestPrepareRunAndInsert_ExpressionsSeeRunID — full-flow check: calls PrepareRunAndInsert with ${{ github.run_id }} in both run-name and the concurrency group, then asserts the persisted Title, ConcurrencyGroup, and RawConcurrency contain / survive the run's ID. Re-ordering db.Insert relative to either parse or concurrency eval fails this test.

Relation to #37119

#37119 also moves concurrency evaluation into InsertRun but keeps it before db.Insert, then tries to populate run_id only when run.ID > 0 — which is still 0 at that call site, so the cross-branch leak would survive that PR as written. This PR fixes the ordering so that run.ID is actually populated at eval time, and broadens it to cover parse-time expression interpolation too.


This PR was written with the help of Claude Opus 4.7

## Problem

Workflow-level concurrency groups were evaluated — and jobs were parsed
— before the run was persisted, so `run.ID` was `0` and `github.run_id`
in the expression context resolved to an empty string. Expressions like:

```yaml
concurrency:
  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
  cancel-in-progress: true
```

collapsed to `<workflow>-` on every push event (`head_ref` is empty on
push), so `cancel-in-progress` cancelled in-progress runs across
**unrelated branches**, not just the current one.

Reproduced on a 1.26 instance:
- push to `master` → `ci` run starts
- push to `feature-branch` → the `master` run gets cancelled

GitHub Actions' documented semantic: on push events `github.run_id` is
unique per run, so the group is unique → no cancellation; on PR events
`github.head_ref` is the source branch → cancellation is per-PR.

## Fix

Insert the run **before** parsing jobs or evaluating workflow-level
concurrency, so `run.ID` is populated in time for every expression that
reads `github.run_id` — not just the concurrency group, but also
`run-name`, job names, and `runs-on`.

`jobparser.Parse` now runs inside the `InsertRun` transaction, after
`db.Insert(ctx, run)`. Workflow-level concurrency evaluation runs next
and only mutates `run` in memory. All concurrency-derived fields
(`raw_concurrency`, `concurrency_group`, `concurrency_cancel`) plus
`status` and `title` are persisted in a single final `UpdateRun` at
end-of-transaction — one `INSERT` + one `UPDATE` per run in both the
concurrency and non-concurrency paths (matches pre-branch parity, one
fewer `UpdateRepoRunsNumbers` `COUNT` than the interim state).

`GenerateGiteaContext` now sets `run_id` from `run.ID` unconditionally;
every caller passes a persisted run.

**Verification**: tested end-to-end on a 1.26 deployment. Before the
patch, two successive `ci` pushes (one to master, one to a feature
branch) cross-cancelled each other. After the patch, the same pushes —
in both orders (master→branch, branch→master) — run to completion
simultaneously across 15+ runs with zero cancellations.

**Regression tests** in `services/actions/context_test.go`:
- `TestEvaluateRunConcurrency_RunIDFallback` — unit check that
`EvaluateRunConcurrencyFillModel` resolves `github.run_id` from
`run.ID`.
- `TestPrepareRunAndInsert_ExpressionsSeeRunID` — full-flow check: calls
`PrepareRunAndInsert` with `${{ github.run_id }}` in both `run-name` and
the concurrency group, then asserts the persisted `Title`,
`ConcurrencyGroup`, and `RawConcurrency` contain / survive the run's ID.
Re-ordering `db.Insert` relative to either parse or concurrency eval
fails this test.

## Relation to go-gitea#37119

[go-gitea#37119](go-gitea#37119) also moves
concurrency evaluation into `InsertRun` but keeps it **before**
`db.Insert`, then tries to populate `run_id` only when `run.ID > 0` —
which is still `0` at that call site, so the cross-branch leak would
survive that PR as written. This PR fixes the ordering so that `run.ID`
is actually populated at eval time, and broadens it to cover parse-time
expression interpolation too.

Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 21, 2026
@GiteaBot GiteaBot added this to the 1.26.1 milestone Apr 21, 2026
@GiteaBot GiteaBot requested review from Zettat123 and lunny April 21, 2026 05:58
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 21, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 21, 2026
@silverwind silverwind merged commit 7bd55de into go-gitea:release/v1.26 Apr 21, 2026
26 checks passed
yamz8 pushed a commit to kerneliushq/kernelius-forge-cli that referenced this pull request May 11, 2026
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [docker.gitea.com/gitea](https://github.com/go-gitea/gitea) | service | patch | `1.26.0` → `1.26.1` |

---

### Release Notes

<details>
<summary>go-gitea/gitea (docker.gitea.com/gitea)</summary>

### [`v1.26.1`](https://github.com/go-gitea/gitea/releases/tag/v1.26.1)

[Compare Source](go-gitea/gitea@v1.26.0...v1.26.1)

- BUGFIXES   \* Add event.schedule context for schedule actions task ([#&#8203;37320](go-gitea/gitea#37320)) ([#&#8203;37348](go-gitea/gitea#37348))   \* Fix an issue where changing an organization's visibility caused problems when users had forked its repositories. ([#&#8203;37324](go-gitea/gitea#37324)) ([#&#8203;37344](go-gitea/gitea#37344))   \* Use modern "git update-index --cacheinfo" syntax to support more file names ([#&#8203;37338](go-gitea/gitea#37338)) ([#&#8203;37343](go-gitea/gitea#37343))   \* Fix URL related escaping for oauth2 ([#&#8203;37334](go-gitea/gitea#37334)) ([#&#8203;37340](go-gitea/gitea#37340))   \* When the requested arch rpm is missing fall back to noarch ([#&#8203;37236](go-gitea/gitea#37236)) ([#&#8203;37339](go-gitea/gitea#37339))   \* Fix actions concurrency groups cross-branch leak ([#&#8203;37311](go-gitea/gitea#37311)) ([#&#8203;37331](go-gitea/gitea#37331))   \* Fix bug when accessing user badges ([#&#8203;37321](go-gitea/gitea#37321)) ([#&#8203;37329](go-gitea/gitea#37329))   \* Fix AppFullLink ([#&#8203;37325](go-gitea/gitea#37325)) ([#&#8203;37328](go-gitea/gitea#37328))   \* Fix container auth for public instance ([#&#8203;37290](go-gitea/gitea#37290)) ([#&#8203;37294](go-gitea/gitea#37294))   \* Enhance GetActionWorkflow to support fallback references ([#&#8203;37189](go-gitea/gitea#37189)) ([#&#8203;37283](go-gitea/gitea#37283))   \* Fix vite manifest update masking build errors ([#&#8203;37279](go-gitea/gitea#37279)) ([#&#8203;37310](go-gitea/gitea#37310))   \* Fix Mermaid diagrams failing when node labels contain line breaks ([#&#8203;37296](go-gitea/gitea#37296)) ([#&#8203;37299](go-gitea/gitea#37299))   \* Use TriggerEvent instead of Event in workflow runs API response for scheduled runs ([#&#8203;37288](go-gitea/gitea#37288)) [#&#8203;37360](go-gitea/gitea#37360)   \* Add URL to Learn more about blocking a user. ([#&#8203;37355](go-gitea/gitea#37355)) [#&#8203;37367](go-gitea/gitea#37367)   \* Fix button layout shift when collapsing file tree in editor ([#&#8203;37363](go-gitea/gitea#37363)) [#&#8203;37375](go-gitea/gitea#37375)   \* Fix org team assignee/reviewer lookups for team member permissions ([#&#8203;37365](go-gitea/gitea#37365)) [#&#8203;37391](go-gitea/gitea#37391)   \* Fix repo init README EOL ([#&#8203;37388](go-gitea/gitea#37388)) [#&#8203;37399](go-gitea/gitea#37399)   \* Fix: dump with default zip type produces uncompressed zip ([#&#8203;37401](https://github.com/go-gitea/gitea/issues/37401))[#&#8203;37402](https://github.com/go-gitea/gitea/issues/37402)

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNDAuMCIsInVwZGF0ZWRJblZlciI6IjQzLjE0MC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Reviewed-on: https://gitea.com/gitea/tea/pulls/968
Co-authored-by: Renovate Bot <renovate-bot@gitea.com>
Co-committed-by: Renovate Bot <renovate-bot@gitea.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants