Still on Drupal 7? Security support for Drupal 7 ended on 5 January 2025. Please visit our Drupal 7 End of Life resources page to review all of your options.Problem/Motivation
The regex used to extract the type: when reading a projects .info.yml is too permissive.
Reported on #3492716-6: Fix CI for Stylelint 16
Proposed resolution
Thanks to @longwave for the solution. Only allow spaces before type: so replace leading ^.* with [[:space:]]*
Remaining tasks
Issue fork gitlab_templates-3492927
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #4
longwaveComment #5
jonathan1055 commentedYou got there before me.
Tested locally and it works as expected.
I suggest for extra safety we also add
| head -1afterwards. I know that having two "type:" would be invalid config, but we should avoid generating a badbuild.enveven in that scenario. I have made this suggestion in the MR.Comment #6
jonathan1055 commentedUpdated IS.
For info, the change that caused this is in the tagged 1.6.10 release, but the default-ref is still 1.6.9, so most of Contrib will not be affected.
See https://git.drupalcode.org/project/gitlab_templates/-/commits/1.6.x-late...
Comment #7
longwaveI wondered about adding the same thing after I did the initial fix, I think it's a good idea. I was trying to see if there was a way of doing it directly in
sedbutheadis easier to understand anyway.Comment #8
longwaveComment #9
longwaveIn fact let's be bold and mark this RTBC as we both agree on the solution here.
Comment #10
jonathan1055 commentedIs it valid yml to have a space after the keyword, before the colon? I know we don't do that much but I think it might be allowed.
Comment #12
longwaveProbably we shouldn't use regex at all, as there still could be false positives if we ever use nested structures in the YAML, or heredocs, or anything similar.
MR!306 uses a pure PHP solution instead which works for me inside
drupalci/php-8.3-ubuntu-apache:production.Comment #13
jonathan1055 commentedNice idea, but I would suggest it's a little bit over-the-top to use php to parse the yml file to get just this one key. I think that the regex version is fine for now, it is simple and if we take the first "type:" found it will nearly always be the required value. On the times when this does not, we already have the override capability to set the variable
PROJECT_TYPEin the project's .gitlab-ci.yml. If I hadn't asked the question in #10 then you would probably have left it at RTBC and not started the second MR :-)Comment #14
jonathan1055 commentedActually I have realized that we don't already cater for overriding the project type, only the project name. So I have pushed a change to MR305 for this - essentially just swapping round the two clauses to allow for a condition on the else too.
https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/305...
Comment #15
jonathan1055 commentedTested MR305 with a normal .info.yml file. The project name and project type are derived correctly.
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/364767
Tested with extra type: in a comment and a second type: keyword.
https://git.drupalcode.org/issue/scheduler-3445052/-/commit/77fdc469e936...
The yml is invalid (as shown by eslint) but the composer job still does not create a bad build.env
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/364793
Tested with an override in the UI form - worked as expected, the form value is shown in the composer log
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/364801
Comment #17
longwaveAlright, let's just go with the hardened regex. The suggested changes look good to me.
Comment #20
fjgarlin commentedMerged. Thanks both for working on this and reviewing/testing.
Comment #21
jonathan1055 commentedNow that MR305 has been merged can we close MR306 ?