Skip to content

Treat all 403s as cancellation, regardless of Job-Status - #133

Open
refi64 wants to merge 1 commit into
mainfrom
wip/refi64/403-cancel
Open

Treat all 403s as cancellation, regardless of Job-Status#133
refi64 wants to merge 1 commit into
mainfrom
wip/refi64/403-cancel

Conversation

@refi64

@refi64 refi64 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Back when cancellation was first added to gitlab-runner, GitLab would include the current job status in the response's Job-Status header, so we could check if the job was actually cancelled.

This situation changed all the way back in 2023:

https://gitlab.com/gitlab-org/gitlab/-/work_items/386871

In theory, GitLab does set Job-Status when the job is not in a state for the request to go through, but now it will also invalidate the job token after the job reaches a completed state. When a job token is invalidated, it cannot be used for authentication anymore, and thus we get a 403 in the sense of "you do not have permission to access this", rather than "the modification you're trying to perform is forbidden".

Thus, job cancellation has actually been broken for several years.

For gitlab-runner itself, this isn't an issue for two reasons:

The simplest fix for now is to just treat 403s as the single "incorrect job status" indicator and remove the code related to checking Job-Status. If/when we decide to add cancel_gracefully support, we can bring parts of it back to check for the canceling status as needed.

Potentially related to the lava-gitlab-runner hangs:

collabora/lava-gitlab-runner#129

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes job cancellation handling by treating any HTTP 403 response from GitLab as a cancellation signal, removing reliance on the Job-Status response header (which is no longer reliably set for this purpose).

Changes:

  • Remove Job-Status-header-based cancellation detection from the client error model and request handling.
  • In the runner loop, interpret UnexpectedStatus(FORBIDDEN) from job update/trace calls as cancellation and trigger the cancellation token.
  • Adjust the mock GitLab API to return 403 when the token is invalid or the job is not running, aligning with the new cancellation semantics.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
gitlab-runner/src/run.rs Cancels the running job when update/trace calls return 403 (FORBIDDEN).
gitlab-runner/src/client.rs Removes Job-Status cancellation checks and the JobCancelled error variant.
gitlab-runner-mock/src/api/update.rs Makes update endpoints return 403 when job isn’t running / token mismatches; simplifies state transition handling.
gitlab-runner-mock/src/api/trace.rs Makes trace endpoint return 403 when job isn’t running / token mismatches (no longer relying on Job-Status for forbidden responses).
gitlab-runner-mock/src/api/request.rs Updates inline documentation comment to match the new 403 behavior expectation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gitlab-runner-mock/src/api/request.rs
Comment thread gitlab-runner-mock/src/api/update.rs Outdated
Comment thread gitlab-runner-mock/src/api/update.rs Outdated
Comment thread gitlab-runner/src/run.rs Outdated
@refi64
refi64 force-pushed the wip/refi64/403-cancel branch from 87b7cb4 to 9b428e5 Compare August 3, 2026 15:49
Back when cancellation was first added to gitlab-runner, GitLab would
include the current job status in the response's `Job-Status` header, so
we could check if the job was actually cancelled.

This situation changed all the way back in 2023:

https://gitlab.com/gitlab-org/gitlab/-/work_items/386871

In theory, GitLab does set Job-Status when the job is not in a state for
the request to go through, but now it will also *invalidate the job
token after the job reaches a completed state*. When a job token is
invalidated, it cannot be used for authentication anymore, and thus we
get a 403 in the sense of "you do not have permission to access this",
rather than "the modification you're trying to perform is forbidden".

Thus, job cancellation has actually been broken for several years.

For gitlab-runner itself, this isn't an issue for two reasons:

- All 403s result in job termination, regardless of job status:

  https://gitlab.com/gitlab-org/gitlab-runner/-/blob/241271310f4c357edd09ade516512a5b8b709c10/network/remote_job_state_response.go#L32

- gitlab-runner implements an additional runner feature,
  `cancel_gracefully`, that lets the after_script run after cancellation:

  https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/4578

The simplest fix for now is to just treat 403s as the single "incorrect
job status" indicator and remove the code related to checking
`Job-Status`. If/when we decide to add `cancel_gracefully` support, we
can bring parts of it back to check for the `canceling` status as
needed.

Potentially related to the lava-gitlab-runner hangs:

collabora/lava-gitlab-runner#129
@refi64
refi64 force-pushed the wip/refi64/403-cancel branch from 9b428e5 to c5dc8ad Compare August 3, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants