Commit status API returns 403 when latest pipeline for the commit is archived
Summary
When trying to Set the pipeline status of a commit where the latest pipeline for the sha is archived (through the Archive older jobs setting), a 403 response is returned.
Steps to reproduce
- Enable Archive older jobs setting
- Pick a pipeline that is older than the setting configured in step 1
- You can verify this by going to the job detail page of a job in the pipeline and checking for the yellow info message on top of the job log
- Note the SHA for which this pipeline ran
- Execute the API call to set a status for said commit, replacing
123with the project ID andabcdef123456with the SHA from step 3:curl -v --request POST --header "PRIVATE-TOKEN: $TOKEN" "http://gdk.test:3000/api/v4/projects/123/statuses/abcdef123456?name=some-name&state=success" - Observe 403 response
Example Project
I can't set the Archive older jobs setting on .com, so no repro project here.
What is the current bug behavior?
A 403 response.
What is the expected correct behavior?
The API call should succeed and create a new external status for the SHA.
Relevant logs and/or screenshots
We determined the reason for the 403 response by debugging the permission check in the CreateCommitStatusService via the Rails console:
irb(main):004:0> current_user = User.find_by_username('redactedUser')
=> #<User id:1337 @redactedUser>
irb(main):005:0> user_project = Project.find(123)
=> #<Project id:123 redacted/project/Path>>
irb(main):006:0> sha = "abcdef123456"
=> "abcdef123456"
irb(main):007:0> pipeline = user_project.ci_pipelines.newest_first(sha: sha, limit: 100).first
=>
#<Ci::Pipeline:0x00007f054befd268
...
irb(main):008:0> policy = Ci::PipelinePolicy.new(current_user, pipeline)
=> #<Ci::PipelinePolicy (@redactedUser : Ci::Pipeline/555)>
irb(main):009:0> policy.debug(:update_pipeline)
- [0] prevent when placeholder_user ((@redactedUser : Ci::Pipeline/555))
- [0] prevent when import_user ((@redactedUser : Ci::Pipeline/555))
- [0] prevent when archived ((@redactedUser : Project/123))
- [0] prevent when placeholder_user ((@redactedUser : Project/123))
- [0] prevent when import_user ((@redactedUser : Project/123))
- [0] prevent when all?(anonymous, ~public_project) ((@redactedUser : Project/123))
- [0] prevent when visual_review_bot ((@redactedUser : Project/123))
+ [7] prevent when archived ((@redactedUser : Ci::Pipeline/555))
[7] prevent when read_only ((@redactedUser : Project/123))
[7] prevent when all?(duo_workflow_token, ~duo_features_enabled) ((@redactedUser : Project/123))
[14] prevent when protected_ref ((@redactedUser : Ci::Pipeline/555))
[14] prevent when ~project_allowed_for_job_token ((@redactedUser : Project/123))
[14] prevent when user_banned_from_namespace ((@redactedUser : Project/123))
[14] prevent when all?(ip_enforcement_prevents_access, ~admin, ~auditor) ((@redactedUser : Project/123))
[28] prevent when repository_disabled ((@redactedUser : Project/123))
[42] prevent when all?(builds_disabled, ~internal_builds_disabled) ((@redactedUser : Project/123))
[388] enable when all?(can?(:public_access), branch_allows_collaboration) ((@redactedUser : Ci::Pipeline/555))
[434] enable when all?(can?(:developer_access), user_confirmed?) ((@redactedUser : Project/123))
=>
#<DeclarativePolicy::Runner::State:0x00007f054bc57c48
@called_conditions=
#<Set:
{"/dp/condition/BasePolicy/placeholder_user/User:1337",
"/dp/condition/BasePolicy/import_user/User:1337",
"/dp/condition/ProjectPolicy/archived/Project:123",
"/dp/condition/DeclarativePolicy::Base/anonymous/User:1337",
"/dp/condition/BasePolicy/visual_review_bot/User:1337",
"/dp/condition/Ci::PipelinePolicy/archived/Ci::Pipeline:555"}>,
@enabled=false,
@prevented=true>
Output of checks
I don't think anything beyond the GitLab version is relevant – it's most likely to be a regression in 17.10, see below. I verified that in 17.6 the behavior was still different.
Possible fixes
As I understand it, this is a consequence of us moving the archived? check from the job level to the pipeline level in Extend job archival mechanism to the whole pipe... (!178865 - merged) – based on this comment thread it looks like this part of the PipelinePolicy is unintentionally blocking the API call from getting beyond the aforementioned permission check due to being archived. We'd want it to get past that check, so that the code down below can create a new pipeline to which the external status would be added.
I'm not 100% clear on the timing because there was a revert and then re-introduction of some of that code, but I do think ultimately it was first released in %17.10, which would line up with when the user reporting this first experienced the changed behavior.
Proposal
Consider only non-archived pipelines for the API:
diff --git i/app/services/ci/create_commit_status_service.rb w/app/services/ci/create_commit_status_service.rb
index 1feab9e71354..885ffc87f147 100644
--- i/app/services/ci/create_commit_status_service.rb
+++ w/app/services/ci/create_commit_status_service.rb
@@ -87,6 +87,7 @@ def first_matching_pipeline
pipelines = project.ci_pipelines.newest_first(sha: sha, limit: limit)
pipelines = pipelines.for_ref(params[:ref]) if params[:ref]
pipelines = pipelines.id_in(params[:pipeline_id]) if params[:pipeline_id]
+ pipelines = pipelines.created_after(Gitlab::CurrentSettings.current_application_settings.archive_builds_older_than)
pipelines.first
end
strong_memoize_attr :first_matching_pipeline
This will have to be defined as a scope on the pipelines model and be applied only if the archive_builds_older_than value is set.