Skip to content

Draft: Fix to prevent unlimited number of CI includes

Leaminn Ma requested to merge 367150-prevent-unlimited-recursive-includes into master

What does this MR do and why?

Currently there is a bug that allows the user to have unlimited includes. This is because file.validate! in Config::External::Mapper::Verifier expands all its nested includes before adding itself to the structure that we use for counting the includes (expandset).

This MR implements the suggested changes as well as a change to the legacy_process_without_instrumentation method, so that both the new code (behind the other FF ci_batch_request_for_local_and_project_includes) and the existing old code has the bug fix.

This MR also updates the verify_max_includes! logic so that the maximum includes count is inclusive <= instead of exclusive <.

The changes have been made behind a new FF: ci_fix_max_includes

How to set up and validate locally

  1. Update the max includes value to 2 in lib/gitlab/ci/config/external/context.rb:12.
MAX_INCLUDES = 2
  1. Enable the ci_fix_max_includes feature flag.
Feature.enable('ci_fix_max_includes')
  1. Set up a simple config of 3 nested includes by creating the following files in your project root.

File1: template1.yml

job1:
  script: exit 0

File2: template2.yml

include:
  - local: template1.yml
job2:
  script: exit 0

File3: template3.yml

include:
  - local: template2.yml
job3:
  script: exit 0
  1. Go to the CI/CD Editor and update the contents with:
include:
  - local: template3.yml

And observe that it does not pass validation and you see the following error:

image

  1. Update the CI/CD Editor with:
include:
  - local: template2.yml

And observe you no longer get the error.

  1. Before you commit the change and run the pipeline, here's the catch! There's actually another bug that allows the validation to pass in the CI/CD Editor but fails when you run the pipeline. This happens because when the pipeline runs, the files list additionally includes the .gitlab-ci.yml file, bringing the total file count up from 2 to 3. This will be addressed in a follow-up issue: #391279 (closed)

So if you run the pipeline at this point, you'll see this error, which ironically also demonstrates that this fix is working. 😄 Screenshot_2023-02-08_at_4.49.34_PM

  1. To run the pipeline successfully, you'll have to update the included file to be template1.yml.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #367150 (closed)

Edited by Leaminn Ma

Merge request reports