Closed (fixed)
Project:
Drupal core
Version:
10.2.x-dev
Component:
base system
Priority:
Critical
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
10 Sep 2024 at 00:06 UTC
Updated:
7 Nov 2024 at 21:36 UTC
Jump to comment: Most recent
Comments
Comment #6
catchMRs 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.
Comment #7
quietone commentedI 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".
Comment #9
longwaveWe 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.
Comment #10
longwaveI think also we should bump the minimum version of Twig in composer.json to the secure version.
Comment #12
jurgenhaasHave 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.
Comment #13
longwave@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
to install 3.14 but satisfy the dependency for 3.8.
Comment #14
jurgenhaas@longwave confirmed, just had to remove
roave/security-advisoriesas well, since it still checked again 3.8Went 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.
Comment #15
jurgenhaasWhat's interesting:
drupal/core-recommendedrequires Twig~3.8whiledrupal/corerequires 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?Comment #17
longwaveFixed 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.
Comment #18
longwave@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.
Comment #19
khiminrm commentedHi!
I've faced with similar issue today when tried to update composer lock file.
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!
Comment #20
feyp commented@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.
Comment #21
khiminrm commented@feyp Thanks! I will try.
Comment #22
_andrew commentedFor 10.3, take the 3.8 from comment #13 and change it to 3.10.3.
Comment #23
cmlaraIt’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-recommendedwill help you more than it might hurt you.Comment #24
loopy1492 commentedIf 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.
Comment #25
longwavedrupal/core-recommendedproject 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.
Comment #26
loopy1492 commented@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.
Comment #27
larowlanReviewed all 5 MRs and they all look consistent and have the expected changes.
Thanks everyone
Comment #28
longwaveOpened #3473440: [meta] Fix Twig 3 deprecations for the deprecations added here.
Comment #39
catchCommitted/pushed the individual MRs from 11.x back to 10.2.x, thanks!
Comment #40
damienmckennaThanks everyone for getting a quick turnaround on this!
Comment #41
caesius commentedSince 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?
Comment #42
feyp commented@caesius This fix has already been released today in 10.2.8, 10.3.4 and 11.0.3.
Comment #43
catch@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).
Comment #44
caesius commentedAh, I didn't realize there was a new release with this since there wasn't a security advisory. Thanks!
Comment #45
bbralaJust 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.
Comment #46
ivnish+1 @bbrala, I have a lot of errors in Upgrade Status report
Comment #47
longwave@bbrala is there anything we could do to help or should have done differently?
Comment #48
catchI 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.
Comment #50
levendclk commentedI 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.
Comment #51
jamesoakley@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.
Comment #52
levendclk commentedThank 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.
Comment #53
cilefen commented3.41.1 exhibits bugs in Drupal. See #3485956: Recursion limit exceeded with Twig v3.14.1 when editing a node or a block.