Problem/Motivation

twig/twig Released version 3.14.0 (See https://github.com/twigphp/Twig/releases/tag/v3.14.0)
which addressed the security issue https://github.com/advisories/GHSA-6j75-5wfj-gh66

Not able to update on drupal 10.3.3 sites

$ lando composer why twig/twig
chi-teck/drupal-code-generator 3.6.1   requires  twig/twig (^3.4)     
drupal/core-recommended        10.3.3  requires  twig/twig (~v3.10.2) 
drupal/twig_tweak              3.4.0   requires  twig/twig (^3.10.3)  
symfony/http-kernel            v6.4.11 conflicts twig/twig (<2.13)   

because drupal/core-recommended sticking with v3.10.*

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3473195

Command icon 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:

Comments

naveenvalecha created an issue. See original summary.

catch made their first commit to this issue’s fork.

catch’s picture

Category: Bug report » Task
Priority: Normal » Critical
Status: Active » Needs review

MRs up for 11.x, 10.4.x, 10.3.x. It looks like the 11.x and 11.0.x change is identical so I didn't bother creating a dedicated 11.0.x MR, but would be great if someone could validate.

We might need to do an out-of-band 10.2.x patch release too.

quietone’s picture

Status: Needs review » Needs work

I get different changes on 11.0.x and 11.x. Looking at the composer.json for each, the twig version for 11.0.x is "twig/twig": "^3.9.3" and in 11.x it is "twig/twig": "^3.11.0".

longwave made their first commit to this issue’s fork.

longwave’s picture

We should do a 10.2 release as well, a 10.2 site I work on that uses core-recommended is complaining and won't upgrade.

I tried to fix the deprecations but one of them looks a bit tricky, I think we should ignore these here and work on them in a followup.

longwave’s picture

I think also we should bump the minimum version of Twig in composer.json to the secure version.

jurgenhaas’s picture

Have review the 3 MRs and the code is looking good. No errors found.

Then I wanted to also give it a try and updated a project's composer.json to provide the repository of the issue fork (https://git.drupalcode.org/issue/drupal-3473195.git) and to require the branch/version dev-3473195-10-3, but composer is not pulling that as it seems to always grab drupal/core from GitHub.

Any advise on how I could require the issue fork instead? I think that's required to test the twig/twig dependency.

longwave’s picture

@jurgenhass you don't need the issue fork, you can override the version with Composer, for example if you are on a 10.2 project and currently using Twig 3.8 you can say

$ composer require twig/twig:'3.14 as 3.8'

to install 3.14 but satisfy the dependency for 3.8.

jurgenhaas’s picture

@longwave confirmed, just had to remove roave/security-advisories as well, since it still checked again 3.8

Went through some heavy tests on a 10.3 site and can confirm that all is working as before, can't find any issue with the updated twig.

Leaving status at NW, could otherwise go to RTBC.

jurgenhaas’s picture

What's interesting: drupal/core-recommended requires Twig ~3.8 while drupal/core requires Twig ^3.9. So, for projects that require drupal core directly without drupal/core-recommended, then we already get the latest twig version. Maybe off-topic, but shouldn't those dependency be in sync?

longwave’s picture

Status: Needs work » Needs review

Fixed one of the deprecations because it only appears in a specific place in one test, otherwise they can wait for a followup. Synced all five active branches so they all depend on ^3.14.0, backported the deprecation skips and some other fixes that were not yet in 10.2/10.3.

longwave’s picture

@jurgenhass re #15 drupal/core-recommended means you are closer to the versions that we tested on Drupal CI, with just drupal/core you are on your own. Previously core-recommended was even more strict and we relaxed it to patch-level updates in #3198340: Strict constraints in drupal/core-recommended make it harder for Composer-managed sites to apply their own security updates when a core update is not available, I think at the time we considered that we might want to relax it to minor-level updates for some dependencies that have good BC policies and Twig is one of those; definitely worth discussing in a followup.

khiminrm’s picture

Hi!
I've faced with similar issue today when tried to update composer lock file.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires drupal/core-recommended == 10.2.6.0 -> satisfiable by drupal/core-recommended[10.2.6].
    - roave/security-advisories dev-latest conflicts with twig/twig <1.44.8|>=2,<2.16.1|>=3,<3.14.
    - drupal/core-recommended 10.2.6 requires twig/twig ~v3.8.0 -> satisfiable by twig/twig[3.8 (alias of v3.14.0)].
    - Root composer.json requires roave/security-advisories == dev-latest -> satisfiable by roave/security-advisories[dev-latest].

How I can fix it with this version of the Drupal core without removing roave/security-advisories?
I've tried to apply patch from the https://git.drupalcode.org/project/drupal/-/merge_requests/9461.diff, but composer couldn't apply the patch.
We should wait when the fixes will be merged and update the core on next release?
Maybe we can use some temp solution until update?

Thanks!

feyp’s picture

@khiminrm As a temporary solution for 10.2, you can use a cross-alias as shown by @longwave in #13. Note that you should remove the cross-alias again when updating to the official release, which will hopefully come soon.

khiminrm’s picture

@feyp Thanks! I will try.

_andrew’s picture

For 10.3, take the 3.8 from comment #13 and change it to 3.10.3.

cmlara’s picture

It’s been slightly mentioned in Slack and implied in this issue but not explicitly stated here:

The other option is to not use drupal/core-recommended. It is not required for a Drupal site to function.

This is not a solution for every site, as it does have a slightly increased risk of breakage with third party dependencies, however it does not have the issues that have prevented security updates several times in the past. Additionally in the past some contrib modules have been incompatible with drupal/core-recommended (needing Guzzle7 in D9 is one issue that I encountered).

Really up to you as a site owner which you prefer. Short term the “require as” fix is not bad, and it involves less immediate risk of a site breakage that you may not be prepared for. Long term, especially with the existing policy fore release timelines, maybe migrating your sites away from drupal/core-recommended will help you more than it might hurt you.

loopy1492’s picture

If anyone knows the maintainers, I've made pull requests for core-recommended.

https://github.com/drupal/core-recommended/pulls

Although something tells me the core requirements will need to be changed to 10.3.4, 11.0.2, and 10.2.8 later.

longwave’s picture

drupal/core-recommended project is maintained here, the merge requests in this issue will update Twig and then we will issue new releases of Drupal - pull requests over on GitHub are not used.

It would be helpful if someone can review the MRs here and mark them RTBC if they agree with the changes.

loopy1492’s picture

@longwave I'd love to help out but it's been a minute since I've referenced a gitlab branch for core in composer and google is failing me. If you can point me to a reference here on d.o where I can tell my site to use the gitlab branch for core, I can help out.

I will close the github PRs.

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed all 5 MRs and they all look consistent and have the expected changes.
Thanks everyone

longwave’s picture

Opened #3473440: [meta] Fix Twig 3 deprecations for the deprecations added here.

  • catch committed 83a80f66 on 10.3.x
    Issue #3473195 by longwave, catch, jurgenhaas, naveenvalecha, quietone:...

  • catch committed 0468d4b2 on 10.4.x
    Issue #3473195 by longwave, catch, jurgenhaas, naveenvalecha, quietone:...

  • catch committed 4675459f on 11.0.x
    Issue #3473195 by longwave, catch, jurgenhaas, naveenvalecha, quietone:...

  • catch committed 44ae8844 on 11.x
    Issue #3473195 by longwave, catch, jurgenhaas, naveenvalecha, quietone:...

  • catch committed 91d88123 on 10.2.x
    Issue #3473195 by longwave, catch, jurgenhaas, naveenvalecha, quietone:...
catch’s picture

Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed the individual MRs from 11.x back to 10.2.x, thanks!

damienmckenna’s picture

Thanks everyone for getting a quick turnaround on this!

caesius’s picture

Since the security vulnerability is now public, will Drupal core get a security release on-cycle next week on the 18th or might it be released sooner?

feyp’s picture

@caesius This fix has already been released today in 10.2.8, 10.3.4 and 11.0.3.

catch’s picture

@caesius Drupal isn't affected by the vulnerability itself, however we released off-schedule patch releases so that there's something to update to today. e.g. https://www.drupal.org/project/drupal/releases/10.2.8 (and 10.3, 10.4, 11.0 equivalents).

caesius’s picture

Ah, I didn't realize there was a new release with this since there wasn't a security advisory. Thanks!

bbrala’s picture

Just so you know.

This complely broke upgrade status and the update bot. Since we now ship deprecations with core. The d11 readiness dashboard showed like +200k errors suddenly and we eventually tracked it back here.

ivnish’s picture

+1 @bbrala, I have a lot of errors in Upgrade Status report

longwave’s picture

@bbrala is there anything we could do to help or should have done differently?

catch’s picture

I think we probably could have updated to https://github.com/twigphp/Twig/releases/tag/v3.11.1 in 10.2 (and maybe 10.3 and 11.0) but I for one didn't know that release existed until after the commits/release here.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

levendclk’s picture

I still have this issue when I upgrade the Twig module from version 3.14.0 to 3.14.1, so I reverted back to version 3.14.0.

jamesoakley’s picture

@levendclk, there is a fresh and different advisory for Twig < 3.14.1. https://symfony.com/blog/unguarded-calls-to-__isset-and-to-array-accesses-when-the-sandbox-is-enabled

The issue being discussed here was composer configuration for Drupal core that wouldn't let you manually update Twig to 3.14.0 when that was the secured version. That is now fixed, you can update to 3.14.0, and as I've just verified in response to the latest vulnerability you can also update to 3.14.1.

levendclk’s picture

Thank you, James. As you mentioned, it’s locked to 3.14.0 and works fine. I also want to thank you especially for the quick response.