Skip to content

Do not expand escaped sequences while expanding variables sent to Runner

What does this MR do?

When we enable the :variable_inside_variable FF, users will run into issues if they are leveraging escaping in their expressions. Let's say a user has defined the variable VAR_A=$$HOME, intending for the value to be expanded to $HOME by GitLab, and then to the actual HOME environment variable value by the Runner. In the current state, since ExpandVariables.VARIABLES_REGEXP is not smart enough to ignore escaped sequences, the value will get expanded to $ plus the value of $HOME by GitLab. The user will get a value for $HOME that is not the expected one (the one on the Runner where the build will be happening).

A user has come across this situation when we enabled the FF.

This MR follows up on !48627 (merged) and adds support for escape characters so that we pass-through the escaped sequences to Runner.

Performance benchmark

Note: I ran the benchmarks twice to warm up the cache.


[1] pry(main)> coll=Gitlab::Ci::Variables::Collection.new([{key: 'VAR_A', value: 'value'}, {key: 'VAR_B', value: '$$VAR_A'}, {key: 'VAR_C', value: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. $$$VAR_B'}])

[2] pry(main)> require 'benchmark'
[3] pry(main)> include Benchmark         # we need the CAPTION and FORMAT constants
[4] pry(main)> n = 5000000
=> 5000000
[5] pry(main)> Benchmark.benchmark(CAPTION, 7, FORMAT, ">total:", ">avg:") do |x|
  tf = x.report("value-VAR_C:")   { n.times do coll.expand_value("value-VAR_C"); end }
  tt = x.report("value-$VAR_C:") { n.times do coll.expand_value("value-$VAR_C"); end }
  tu = x.report("value-$$VAR_C:")  { n.times do coll.expand_value("value-$$VAR_C"); end }
end

On master (using ExpandVariables::VARIABLES_REGEXP)

value user system total real
value-VAR_A: 1.989302 0.034963 2.024265 2.028721
value-$VAR_A: 22.908994 0.734235 23.643229 23.731605
value-$$VAR_A: 24.298558 0.731617 25.030175 25.130424

With this branch (using new ExpandVariables::VARIABLES_REGEXP)

value user system total real
value-VAR_C: 2.589652 0.073374 2.663026 2.670081
value-$VAR_C: 14.910547 0.306363 15.216910 15.232970
value-$$VAR_C: 15.230110 0.351373 15.581483 15.608158

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Closes #321997 (closed)

Edited by Kamil Trzciński

Merge request reports