[policy, no patch] How much of The Update Framework integration is needed for alpha-level review/commit of Package Manager?

Created on 21 March 2023, about 1 year ago
Updated 19 March 2024, 2 months ago

Problem/Motivation

The Automatic Updates Initiative and the Project Browser Initiative both are creating modules that will add user interfaces for running Composer commands to add and update projects.

While this does not introduce completely new supply side attack vectors it does mean this can be triggered via the UI and in the Automatic Updates case it will introduce an unattended process that updates Drupal core(and maybe eventually other projects).

There 3 modules that will be added to core which will go through the experimental module process

  1. Package Manager: the API only module that makes the composer commands. It is currently a sub-module of Automatic Updates in contrib but would be its own module in core.

    In addition to eventual adding of "The Update Framework" integration, this module also provides additional security hardening over using Composer directly:

    1. Requires https to be used for Composer
    2. Requires https to be used for fetching Drupal.org update XML
    3. Ensures that all drupal.org projects are on secure versions according to drupal.org update XML

    Both the other 2 modules require this module and get this extra security hardening

  2. Automatic Updates: the MVP for core only updates Drupal core. It provides both a form and a cron process that updates core. It allows 3rd party vendor packages to be updated or added if required for core update. It does not allow other Drupal projects to be updated during a core update.
  3. Project Browser: It installs new modules and themes via Composer. 3rd party vendor packages are allowed to be updated and other dependencies It does not allow core to be updated while a new project is being installed

It has been decided to that Drupal will implement The Update Framework(TUF) to enhance security against supply side attacks.

We need to determine, or just have written down if already decided by Drupal core Release and Product managers, at what stages of the core experimental process integration with The Update Framework is required.

Technically it is possible to do the validation either

  1. In package manager so that all dependencies require it
  2. In either or both Automatic Updates or Project Browser. Or only when using those modules in certain ways, for instance only for cron updates in Automatic Updates

It should be noted since TUF validation is done on the Composer plugin level that once a Drupal site is setup to use the TUF integration all Composer commands whether through new UIs or through the command line will require TUF validation for operations on drupal.org projects. Composer configuration would have to be updated to opt out of TUF integration

Questions that need to be answered or clarified:

  1. Is TUF integration an Alpha requirement for Package Manager?
  2. Is TUF integration requirement the same for Package Manager, Project Browser, and Automatic Updates?

Proposed resolution

A production set up of the TUF is not required prior to an initial commit of Package Manager in Drupal core. This also applies to Automatic Updates and Project Browser.

A testing module to allow testing package manager/automatic updates against d.o's dev packagist endpoint might be needed for the reviews but might not be necessary especially if TUF is deployed on d.o production before an alpha commit, or if there's a way to easily configure this without a testing module for people testing locally. Even though it hasn't been identified as a hard blocker at this point, let's try to implement a way to opt-in for TUF testing to help testing and reviewing it in the meantime, even if it's only very temporary.

Remaining tasks

Make TUF testable against Dev Repe

See #40

https://github.com/php-tuf/drupal-project/ will create a test project that will use the TUF protect DEV metadata but this does not validate this inside of Package Manager.
Here are follow-ups for this

  1. Github issue: Create test module to enable TUF validation in Package Manager
  2. Contrib module: 📌 Make PhpTufValidator able to switch TUF protected repo URL for testing purposes Active

Release notes snippet

🌱 Plan
Status

Fixed

Version

11.0 🔥

Component
Other 

Last updated about 15 hours ago

Created by

🇺🇸United States tedbow Ithaca, NY, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @tedbow
  • 🇺🇸United States xjm

    The signing and verification toolchain and d.o-side support for core and contrib release signing is required prior to Package Manager (and therefore the modules dependent on it) being beta experimental in core.

  • Status changed to Needs review about 1 year ago
  • 🇺🇸United States tedbow Ithaca, NY, USA

    @xjm thanks for clarificaion.

    I updated the summary per #2

  • 🇺🇸United States effulgentsia

    It is not an Alpha requirement of any of the 3 related modules

    @xjm and I discussed this, and this is incorrect. TUF blocks alpha for all 3 modules, because a security review blocks alpha, and TUF is needed for a proper security review.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Updated the summary based on #4

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Since I am not sure where all of this is documented in public I am trying to give more context that I am aware of. Obviously I am not in core governance so please correct me if I am wrong or add more context but here is what I have found in Slack.

    Need for TUF before Alpha

    From November 22, key points but see the link for exact conversation:

    @xjm

    We can't do a proper security review or code review until it's available and the core client part integrating with it it also complete.

    @tedbow responded

    I would say a code/security review for the Drupal module itself could be done because the php-tuf integration part is really on composer side with the module having to very little knowledge about it except

    and

    I wonder if there is any chance the module could get into core as Alpha without the TUF integration complete because the Alpha experimental module would be removed from releases anyways.
    ....

    but yes a complete security review of the full system can't be done until TUF integration is complete
    .....
    what about the idea of getting AU in core as Alpha before the full security? since https://www.drupal.org/about/core/policies/core-change-policies/experime...

    From the link I quoted

    Alpha experimental modules are still under development, but available for initial testing. They may include bugs, including security or data integrity issues.

    we will no longer ship alpha-level experimental modules in stable releases

    @xjm pointed out

    We're still opposed for the same reasons -- the initial alpha code review by FMs etc. is a big investment and happens typically for the commit for alpha, rather than after the fact for beta

    From my understanding from in person conversations with @xjm this is a major reason for blocking any commit of Package Manager, Project Browser and Automatic Updates in core.

    @xjm added

    (We talked about it among the committers on the security team back when we were talking about what would be OK for contrib)

    Pivot to Project Browser

    I took the conversation above to be related to specifically getting Automatic Updates at the Alpha stage into core because of its unique security implications. Since we could not get Automatic Updates into core without TUF integration which was blocked by implementation on the drupal.org side the Automatic Updates team switched from working on trying to get Automatic Updates into core as Alpha to working on Package Manager Alpha blockers to unblock getting Project Browser into core as alpha.

    We believed(wrongly as it turns out😔) that Project Browser getting into core as Alpha was not blocked by TUF because it did not have the same security implications of unattended operations used to deliver critical security updates.

    see from slack #autoUpdates

    Auto Updates team currently focusing on Package Manager issues only to help unblock Project Browser getting into Drupal core

    and #project browser

    The AU team is shifting focus from general AU work to specifically PM blockers

    At this point we opened up Add Alpha level Experimental Package Manager module Needs review and postpone #3253158-20: [PP-2] Add Alpha level Experimental Automatic Updates module
    see @wim leers

    Because Automatic Updates needs a comprehensive review, including a security review. But that means drupal.org's PHP-TUF support needs to be deployed, otherwise core committers would need to do multiple security review rounds: once for the Automatic Updates module, once for the d.o PHP-TUF support.

  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    Being new to this initiative since November, this definitely was a shocking surprise 😳

    Looking forward:

    1. Is it then an accurate conclusion that 100% of package_manager, automatic_updates and project_browser are blocked on #3325040: [Packaging Pipeline] Securely sign packages hosted on Drupal.org using the TUF framework and Rugged ?
    2. Are there other d.o infrastructure issues that are blockers?
  • 🇺🇸United States tedbow Ithaca, NY, USA

    #7

    I am not sure how "[policy, no patch]" issues get closed but I was hoping that the "Proposed resolution" would reflect what the policy is so it is clear for everyone. So I'm adding the needs release manager review, and needs product manager Review tags, Because I'm assuming somebody in one of those roles would have to give the sign off on or update the proposed solution is the policy.

  • 🇬🇧United Kingdom catch

    I have not been following this closely, but I guess now I'm following it a bit closer because this has come up.

    A thing I am not clear on is this, which is a copy and paste of Ted's last comment from that slack thread (assume it's fine to quote it since it was linked):

    @xjm
    I had a question about the scope of the full review and when that could be done.
    Will the full security review include all the code that generates the TUF artifacts as part of the packaging pipeline on the drupal.org server side, including https://gitlab.com/rugged/rugged? I also assume there would at least be some non-public configuration(maybe code) outside of Rugged.
    Or will the full review be limited to TUF artifacts that the drupal.org server side provides and the client code that ships with drupal (either directly or as a composer dependency) and interacts with those artifacts?
    Wondering because this question probably determines when we can consider TUF server side portion done as far the AutoUpdates system being in place where it can be ready for a review

    It's my understanding that rugged is deployed to d.o staging, that there is one known rugged blocker for php-tuf (hashed bins). Once hashed bins is available (in the client and server), then it should be theoretically possible for php-tuf, package_manager, and d.o staging to work together as a proof of concept.

    However there are various other rugged blockers (queue crashes, terabytes of storage duplication) which won't affect php-tuf but do make it not deployable to d.o production yet. And it might not be possible to sign everything on d.o staging due to the scalability/storage blockers or at least not without as-yet-unimplemented workarounds.

    That means, assuming the above is accurate now, and stays accurate, that there is a window between php-tuf and the code that depends on it being theoretically alpha-complete, and TUF being deployed to Drupal.org production.

    It seems like a code review of php-tuf and friends could happen at that point - at least, that would be an option for when reviewers could start reviewing if not finishing.

    However, even if review can start then, it probably wouldn't make sense to review rugged until it's about ready to deploy to d.o production. Also it might not be possible to sign all packages on d.o staging, meaning user testing would be hampered (assuming we can even point end-user composer.json to d.o staging in the first place) and probably pen testing too. But we're not reviewing rugged for core inclusion, it's context for a php-tuf review as far as core is concerned if not the entire system.

    For me it would make sense to at least inform people that different aspects are possible to review when they are, then people can either review early and often as different elements are available, or wait, depending on availability and how they want to approach things. But there would need to be a clear outline somewhere of what needs reviewing, what can be reviewed, what can't etc. at the moment I have no idea where to find this information and asking 2-3 different people gives 2-3 different answers.

  • 🇺🇸United States effulgentsia

    I'm retitling the issue to reflect a more nuanced question.

    #9 highlights some of the things that would need to happen for a full security review of Package Manager. My concern with that is if we require all of that for initial, alpha-level, commit, then we're effectively putting Project Browser reviews on hold for months, potentially making the difference between whether Project Browser can be released in 10.2 vs. 10.3. Unless we think Project Browser reviews can happen without Package Manager being in core, but I'm not sure if we have experience with prior core additions being successfully reviewed ahead of their dependencies landing.

    I wonder if it would make sense for an alpha-level review (and commit?) of Package Manager to include TUF integration, but against the staging TUF repo, without hashed bins, with only some (10? 100?) modules signed, and without solving the other Rugged issues that still need to be solved before it can be deployed to production.

    The downsides of the above would be:

    • If you enable Package Manager (and when it's ready, Project Browser) in their alpha state (which only core testers can do, since alpha modules aren't released in tagged releases), you'll only be able to install modules that are in that subset of modules that are signed on staging.
    • The security review performed for Package Manager alpha would lack the ability to review Rugged's hashed bins implementation and the other Rugged issues that aren't done yet. So, additional security review will be needed after those issues are done. That would potentially add work for security reviewers, and risk some security bugs not getting caught due to the reviews being separated.

    The upside would be:

    • Project Browser (and possibly Automatic Updates?) reviews wouldn't be blocked on finishing Rugged.
    • As a result of ^, PB and AU would be more likely to ship in 10.2 vs 10.3.
  • 🇬🇧United Kingdom catch

    I wonder if it would make sense for an alpha-level review (and commit?) of Package Manager to include TUF integration, but against the staging TUF repo, without hashed bins,

    I don't think this is feasible, from comments by @phenaproxima in slack somewhere, the problem is that the metadata file without hashed bins is something like 200mb, which will crash when you try to parse the JSON. Also the client needs to support hashed bins for when it's available, and there is other stuff to do in package_manager before it's ready to review anyway, so hashed bins at the moment is unlikely to block review for long or at all, and package_manager can't work without it.

  • 🇺🇸United States xjm

    There's some misunderstandings here still so I'm just going to summarize some more from the ongoing Slack discussion.

    TUF blocks Package Manager alpha for the following reasons:

    1. TUF is a significant part of the Package Manager MVP and will be a nontrivial amount of code, so Package Manager doesn't make sense as an alpha module without it.
       

    2. For the same reason, doing security review without TUF implemented doesn't make sense; Package Manager is fundamentally an insecure design so there's no reason to do the needed security review for alpha without that code being implemented.
       

    3. The contrib AU module currently mitigates the lack of TUF signing with:

      • Attended-only updates
      • Requiring HTTPS

      We can't require HTTPS for core, even for alpha, because not all hosts support it and so the module could put a site on such hosts in a state where nothing can be installed or updated. This is a serious data integrity and upgrade path problem, which is one of the few reasons release management will get involved in alpha requirements and say "no" to something.

    However, we were willing to allow a Project Manager beta that just provided composer commands (and did not install anything) which therefore would not be blocked on Package Manager. We offered that option to the initiative team, but from their perspective it wouldn't actually be helpful to them due to the extra overhead etc. For PB, they are comfortable targeting 10.2 once Rugged/TUF is implemented and deployed.

    A similar approach is not available for Autoupdates, because the whole point of getting it into core is to review and test the tool that writes updated Drupal core code to the filesystem, and so there's no functionality to make an alpha of if you take away Package Manager support completely.

    The decision to require functional TUF integration code prior to any core commit has now been re-discussed and re-made in:

    1. The original initiative core conversation at DrupalCon Baltimore in 2017 (for the abstract concept of package signing and validation)
    2. August 2018 (for the csig equivalent in the 1.x version of the module)
    3. August 2020 (for TUF, which we'd decided to adopt by Feb. 2020)
    4. June 2022
    5. November 2022
    6. February 2023
    7. March 2023
  • 🇨🇦Canada ergonlogic Montréal, Québec 🇨🇦
  • 🇺🇸United States xjm

    @effulgentsia and I spoke more about this issue.

    We both agree that a working TUF implementation is a requirement prior to a core commit of autoupdates.

    The main point we disagree on is whether TUF is also a requirement for an alpha of Project Browser, and whether the tradeoffs for the review process are a net loss or gain overall.

    @effulgentsia believes what Project Browser does is sufficiently similar to the current module download functionality, and that we could safely require HTTPS instead of signing and verification as a temporary alpha requirement.

    I on the other hand am concerned about the drawbacks of adding such an incomplete Package Manager to core in terms of the review process, user testing, and security, and I'm also concerned about the complications and possible critical disruptions to alpha testers from adding a temporary HTTPS requirement. I furthermore think it adds overhead and risk to the review process without improving the release timeline for the modules.

    However, we both have the same goal: Start code reviews and user testing for all three of Package Manager, Automatic Updates, and Project Browser as soon as possible, and smooth the path to core inclusion.

    The compromise we agreed on is that we should begin core maintainer reviews of the Project Browser contributed project as soon as possible. We also should expand efforts for user testing of composer-stager, Automatic Updates, and Project Browser using the contributed projects. Finally, we can potentially open a core merge request for Project Browser sooner rather than later, even though it may not be committed until the working TUF implementation is added.

    To this end, I think we should create a core issue for maintainer code review of the Project Browser contributed project (tagged for release manager, framework manager, and JavaScript review). We should also plan user testing sprints at upcoming events, and have some communications promoting user testing of the modules.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    re #14

    We can't require HTTPS for core, even for alpha, because not all hosts support it and so the module could put a site on such hosts in a state where nothing can be installed or updated.

    After chatting with @catch I created 🌱 [policy, no patch] Should Package Manager require Composer HTTPS? Active instead of just adding the discussion here because it might be more technical discussion about host support. But that issue goes over the implications, as I understand them, of not requiring HTTPS(in addition to TUF not instead of) given the scope of TUF protection proposed in #3325040: [Packaging Pipeline] Securely sign packages hosted on Drupal.org using the TUF framework and Rugged , no TUF protection for 3rd party dependencies. I pinged @drumm and @hestenet unless in case I am wrong about the scope of #3325040

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Just FYI, even though it seems like from the above that Add Alpha level Experimental Package Manager module Needs review is not going to get a review right away, I wanted to let people know we have update the summary there to be much more complete if anyone wants some light reading😜

  • Status changed to Needs work about 1 year ago
  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    First round of review! This is looking great already 😊

  • Status changed to Needs review about 1 year ago
  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    Wrong browser tab 🙈 … and so consider this a serendipitous "bump" 😅

    Since #18, 🌱 [policy, no patch] Should Package Manager require Composer HTTPS? Active has had further discussion and the corresponding issue for the Package Manager module ( 📌 Harden our HTTPS requirement Fixed ) landed 5 days ago.

    That means this issue indeed needs review. Restoring that state.

  • 🇬🇧United Kingdom catch

    Hashed bins appears to be implemented in rugged as of a week ago, that leaves one outstanding ruggeed issue listed on #3325040: [Packaging Pipeline] Securely sign packages hosted on Drupal.org using the TUF framework and Rugged , which as far as I know is more of a nice to have or possibly production blocker. I think that means TUF on d.o staging is unblocked now.

    That's all within just over a month since this ticket was opened, so to me it doesn't look like TUF is a long term blocker, it looks like the last few bits are being ironed out in testing and getting fixed promptly enough - at least enough to run on staging so everything can be reviewed.

  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    📌 Add a validator to check that PHP-TUF's Composer integration is present and configured correctly Fixed has landed. That means we're now just hard-blocked on d.o infrastructure at #3325040: [Packaging Pipeline] Securely sign packages hosted on Drupal.org using the TUF framework and Rugged to actually enable what #3316617 introduced, and we have 📌 [PP-1] Require PHP-TUF's Composer integration plugin Postponed ready and waiting for that 👍

  • 🇬🇧United Kingdom catch

    While TUF isn't deployed on d.o packagist in production yet, it's mostly deployed on staging, production blockers are well documented and in progress, the security audit has started and is going well - so for me all the things I was concerned about in March have been met by now (or months ago, but I also wanted to get 🌱 [policy, no patch] Should Package Manager require Composer HTTPS? Active resolved properly too). i.e. it should be possible to holistically review package manager with PHP TUF together, and it'll be valid for when it's all deployed and working together because major changes aren't still in the pipeline. I don't know the situation of the external audit for PHP-TUF and friends but sounds like that also might start sooner than later.

    I would also like to see us commit PM/PB/AU as early as possible in the 10.3.x/11.x cycle so that they get a good round of alpha/beta testing before the next minor release.

    Additionally, because we'll have the 10.3.x and 11.x branches open at the same time (which we have not had an equivalent situation since 9.5.x and 10.0.x), if we remove/disable guardrails, it should be possible to test updating from 10.3.x to 11.x and/or from 11.x alpha releases to later 11.x alpha releases, without actually having to have automatic updates in a tagged stable release as beta. This would make a core commit as alpha a lot more useful than it would have been when it would only have been alpha in one branch.

    Are there any remaining concerns here? Or if not it would be useful to sort out the actual alpha reviews a bit more systematically.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    @catch #24 sounds positive!

    Are there any remaining concerns here? Or if not it would be useful to sort out the actual alpha reviews a bit more systematically.

    📌 [policy, no patch] Use Update XML in Package manager to determine release support status Active is the big unknown as far as remaining work on the Package Manager and Automatic Updates module. The issue will probably lead to more work in the modules, though not a ton

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Just to super clear regarding my response in #25

    from #24

    I would also like to see us commit PM/PB/AU as early as possible in the 10.3.x/11.x cycle so that they get a good round of alpha/beta testing before the next minor release.

    • If we don't get a decision very soon on 📌 [policy, no patch] Use Update XML in Package manager to determine release support status Active will not get in early.

      For the benefit of others who haven't read #3394754(catch has) our team is not working on this problem because depending what is decided by core committers the solution will be different. From my understanding as the tech lead I can only suggest a solution to this problem but the ultimate decision is for core committers, because how much of the systems needs TUF protection seems to be product level decision and #3394754 deals with this . Not complaining just want to be clear this is stopping us doing the work to solve this problem because I don't want to do the work 2x. Once the way I think we should and once they committers ultimately decide(I could guess right only have to do it once but 🎲)

      This is why I have marked #3394754 as critical

    • Regarding "alpha/beta" right now we restrict target version to RCs and above but that could easily be removed with a test module that beta testers could use.

      The bigger problem is "alpha" testing. Since alpha experimental modules are not in releases I think this will be much more difficult. The module is not meant to be used with dev versions so that would problem with starting from dev version at the alpha commit. We could probably get around that for Alpha testing but I think the bigger problem is the target version would *not* have the updater present is so it would fail(assuming haven't actually tried it and not intended to work that way).

      Maybe there would be some way to hack that by using Composer aliases that pointed an alpha version to an actual dev commit but I haven't looked into this.

      I have documented this elsewhere but this is I think we can't actually have people manually testing until it is beta and in a release. Of course you can test with the contrib module which as of now is actually to automatically create the core merge request.

  • 🇬🇧United Kingdom catch

    @tedbow

    Regarding "alpha/beta" right now we restrict target version to RCs and above but that could easily be removed with a test module that beta testers could use.
    The bigger problem is "alpha" testing. Since alpha experimental modules are not in releases I think this will be much more difficult. The module is not meant to be used with dev versions so that would problem with starting from dev version at the alpha commit.

    No it should be fine from that point of view too, this is what I meant with:

    if we remove/disable guardrails, it should be possible to test updating from 10.3.x to 11.x and/or from 11.x alpha releases to later 11.x alpha releases

    We release alphas (actual tagged releases with release nodes) of the new major version well in advance of a beta release - we did this for 9.0.0 and also 10.0.0. This is usually one once changes like Symfony major updates and similar are in, in case contrib modules want to track compatibility against alphas rather than a moving dev target. The alphas are getting less urgent from that perspective the further along things like rector get, but we'll still do them, and then that provides actual tagged 11.x releases to update to/from with automatic updates.

    I also think we could do that for 10.3.x too if it helps with AU testing in a way that 11.x alphas don't (like trying an actual major update?), but didn't fully think through the consequences of that - obviously we don't want an actual site getting confused and trying to run on a 10.3 alpha released in January in production, which they're unlikely to do with 11.x.

    Having both branches in tandem, definitely with early 11.x alpha releases and possibly early 10.3.x alpha releases is the best possible timing for an alpha commit of AU then. Maybe we could do alpha releases of 11.1 and 10.4 but it's not built in the same way.

  • 🇫🇮Finland lauriii Finland

    Discussed with @Gábor Hojtsy and we agreed that from our perspective TUF should be at maximum a stable blocker for both Automatic Updates and Package Manager. We acknowledge the integral role of TUF in enhancing security for automated updates, especially when updates are done at scale. However, not having TUF should not prevent us from getting the Automatic Updates in front of the core maintainers, core developers, and early testers. We believe we should do this as soon as possible because now the module is being built in its own silo.

    We understand the need for thorough testing before enabling Automatic Updates by default, including testing updates with TUF. However, tying its development to alpha/beta milestones seems counterproductive. We can see that there are benefits to being able to do a holistic security review but it sounds like a lot of this is already possible before the TUF has been deployed on d.o packagist in production per #24.

    Concerning the adoption of Automatic Updates, it's clear that a majority of the user base requires a high level of confidence before enabling such features in production. While the contrib module already offers automatic updates, its limited adoption hints at the community's cautious stance on this. In our research, we found that users prefer to wait until Automatic Updates has proven its reliability over time before starting to use it themself.

  • 🇬🇧United Kingdom catch

    If it's not a beta blocker, then how will sites running it as beta be TUF-enabled prior to the module being marked stable, so that we don't end up with a rump of sites that never opt-in?

    I think package manager can technically run any composer command, so it is theoretically possible to update to a version of package manager that opts you in to TUF for d.o (which might then require a second update to a version of package manager that enforces TUF for d.o - no idea if this would be possible in a one-stage update).

    Such a mechanism doesn't exist at the moment, and while it would be useful to be able to opt-in other packagist endpoints if/when they add TUF support (like the main one if that ever happens), it's adding additional stable-blocking technical debt to the module that wouldn't be necessary if it just starts off with TUF support from the outset.

  • 🇬🇧United Kingdom catch

    To expand on #29, I think dropping this as a beta requirement adds a lot of additional work, requirements, timeline, and risk.

    Let's take a site that installs 10.3.0, and enables automatic updates (experimental) without TUF support.

    For automatic updates to be stable, we want the d.o endpoint to be TUF-enabled, so we now have to add a mechanism to TUF-enable this site to core, currently that feature doesn't exist, and it's not on the roadmap, as far as I know it's only brought up once or twice by me as a possibility in various issues. This is what I'm talking about in #29.

    Let's say this mechanism wasn't added as a beta requirement, so we still need to add it during beta.

    To test the mechanism, we need to take a non-TUF site on 10.3.0, and update to 10.3.1, and make sure it's TUF-enabled after the update.

    But now, we also need to check that once that site is newly TUF-enabled, that it can self-update again (i.e. that TUF-enabling didn't break anything like corrupting composer.json), so that requires another patch release, 10.3.2.

    We probably also want to add validation that d.o is TUF protected (so it can't be un-protected), but can we even add that validation until we have no 10.3.0 sites left? I don't know the answer to this, but that might be yet another patch release to add that validation - but we can't mandate that sites update 10.3.0, 10.3.1, 10.3.2 etc., they could skip from 10.3.0 to 10.3.2 so that also needs to work and be tested.

    All of this is adding a transition from non-TUF to TUF that simply would not exist if the beta includes TUF from the outset - so we wouldn't need to do any of that work.

    Now the work above wouldn't be wasted, because it could be used for packagist.org down the line if they ever add TUF support, but I don't see how that would be stable blocking if we're not trying to change the TUF-status of any endpoints before stable. Only the proposal above would do that.

  • 🇫🇮Finland lauriii Finland

    I agree that the complexity introduced by not having TUF enabled by beta Automatic Updates is not ideal. Thanks for explaining in detail the complexity involved with that. However, this risk would be only valid if we didn't have TUF-enabled with the Automatic Updates in the next release. Not having Automatic Updates in the next release seems even worse because it would mean that we would have to wait at least until 2025 for stable Automatic Updates. Potentially even until June 2025, if we follow our current policy where experimental module statuses are only changed in minor releases.

    It sounds like we could try to minimize the risk related to enabling TUF after it has been shipped by introducing a beta blocker to evaluate feasibility of #30 if TUF has not been enabled. We could decide on the approach closer to the release depending on the status of TUF. If #30 isn't feasible, then Automatic Updates could not reach beta until TUF has been enabled.

    The ideal scenario is still that by 10.3.0 we have TUF enabled Automatic Updates. Unless #30 introduces too much complexity, having Automatic Updates without TUF would be still better than not having Automatic Updates at all. This would enable us to start testing TUF whenever it's done, potentially well before 10.4.0. This way Automatic Updates could potentially even reach stable in 10.4.0, and be enabled by default.

  • 🇬🇧United Kingdom catch

    Thanks for explaining in detail the complexity involved with that.

    That took time that I could have spent doing literally anything else. Meanwhile [PP-1] Add Alpha level Experimental Automatic Updates module Needs review hasn't been updated for two months. Why are people constantly bringing up different versions of this and not instead spending that time getting the module ready to commit? How many hours have been spent talking about it in the past year?

    However, this risk would be only valid if we didn't have TUF-enabled with the Automatic Updates in the next release.

    No not really. Beta-stability modules are expected to provide upgrade paths, so #30 comes into play as soon as package manager/automatic updates is marked beta. There is no need for it to be marked beta before a 10.3.x/11.x beta release because there's no real advantage to it being more stable than the core release it's in. The only way to avoid the extra work is to mark it beta after TUF is enabled/required because then there's no upgrade path to deal with at all. This means waiting for TUF reduces work significantly right up to the very last possible beta deadline for 10.3.

    The proposal only 'works' if you were to get to the very last beta deadline for 10.3/11.x, know that TUF is not on production, and then mark PM/AU beta at that point, knowing that you'll then have to do all the TUF work and the extra upgrade path during the 10.4/11.1 cycle. But you're then putting that additional, complex, upgrade path in the way of every single beta tester of automatic updates into patch releases of Drupal core, the number of which will increase over time the longer the window is between it being available and that update being added, increasing the chance of buggy updates for something that is supposed to make them seamless.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    I have not caught up on all comments from #28 but want to clarify.

    [PP-1] Add Alpha level Experimental Automatic Updates module Needs review hasn't been updated for two months.Why are people constantly bringing up different versions of this and not instead spending that time getting the module ready to commit?

    We are working on the contrib which is the source of the core MR constantly. I had not run the conversion script in 2 months because it always take a bit of time because we always find little things that breaks tests, mostly in changes in 11.x, new deprecations, etc.

    So I haven't been running the conversion because it takes some time and before last week I was the only person who commented on the issue since April. Because of that I thought it was a better use of time to work on the module. The Needs Review bot marked it as Needs Works

    I should have made that clear on #3253158. Sorry about that. I have I have pushed up the latest commits to that issue. The test will still fail because we need to make some adjustments for gitlabci. But I still think this issue is reviewable. The tests pass in contrib which is basically the same module.

    We are handling this in the contrib module 📌 Start using gitlab ci in addition to Drupalci for testing Needs work . Basically our plan is we will use gitlabci to run the conversion for the core merge request and then run the tests on every contrib issue as core merge request. We will still use drupalci to test the unconverted contrib module.

    Once this is in place we should have no more (or many fewer) surprises when run the core merge request conversion. That will allow us to keep it in sync with the contrib module more easily.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    re #30 and @catch's concerns about sites that would run the beta before require TUF

    Yes you are correct that it would add some work but I don't think that much. Let me explain how we have the checking now in the core MR/contrib module.

    I will use the contrib module code as an example but this is the same in the core with namespace changes(which happen in the conversion)

    1. We have \Drupal\package_manager\Validator\PhpTufValidator which checks if you have a composer repository using https://packages.drupal.org and that it is TUF enabled, meaning the config values in the composer.json are correct and the php-tuf/composer-integration is present. both of these must be true for you to be TUF protected
    2. Because the contrib module(and converted core MR) does not require TUF yet(it won't work anyways because d.o work not done) we do not add the event_subscriber tag to this service so the validation does not happen.
    3. \Drupal\Tests\package_manager\Kernel\PhpTufValidatorTest we add the event_subscriber tag to this service. In this test we test for, correct configuration and that we proper error messages if it is not setup correctly

    so

    We probably also want to add validation that d.o is TUF protected (so it can't be un-protected), but can we even add that validation until we have no 10.3.0 sites left?

    So yes if we add TUF validation in 10.3.1 this would basically mean adding the event_subscriber tag to this service. Once it is added then any operation that Package Manager tries would fail if d.o was not TUF protected.

    For 10.3.0 sites they would not be affected because the validation would not happen for them. Composer operations are only TUF protected if the repository in your composer.json is marked correctly and php-tuf/composer-integration is preset. So even after d.o is provides TUF metadata it will not be automatically enforced unless your config is correct. The validation happens on the Composer level through php-tuf/composer-integration. Package manager just validates that you have it setup correctly. php-tuf/composer-integration has its own test suit.

    As soons as a 10.3.0 updates to 10.3.1 they would have the TUF protection. If we didn't a way to update their composer configuration they would not be able to do any further Package Manager/AutoUpdates operations

    alternatively leaving a rump of sites that would never get TUF-enabled unless they manually run composer commands (which would defeat the object of having package manager in the first place and also penalise beta testers).

    We already have 📌 Automatically configure TUF Composer settings after the module is installed Postponed . This was probably also going to stable blocker regardless of whether TUF was beta requirement for core or not. This is because TUF protection is requirement for using Package Manager(and therefore AutoUpdates) not Drupal core itself. This means effectively a site that installed Package Manager in 10.3.0 when TUF was not required and then updated to 10.3.1 where TUF is required will be in the same situation as a site that installed Package Manager in 10.3.1 for the first time but the site has been around since 10.3.0 or before. The both likely will not have the configuration in their composer.json unless they did it themselves and therefore if a core requirement is that they should not have to update their composer manually then we need to be able to set this config for them whether not the first version of Package Manager/Automatic Updates requires TUF.

    As far as whether this will be hard I don't think it will. Like all of the Package Manager operations it would be run in staged area first so we could do validation on their composer.json after we ran the command to ensure it was not corrupted.

  • 🇬🇧United Kingdom catch

    So we would need 📌 Automatically configure TUF Composer settings after the module is installed Postponed for new sites, and then a hook_update_N() or hook_post_update_NAME() to call the same code for sites with AU installed already. Because it'd include an update handler, it wouldn't be able to be run unattended though I assume, but those can still be done semi-manually with the AU UI?

    In general I think it's fine to not require TUF for AU alpha per #24 although I would like to understand @xjm's concerns more. Bringing the beta requirement into this issue is just making what could have been a much easier decision more difficult - why not have opened a new issue for this instead of completely changing the scope of this one?

  • 🇫🇮Finland lauriii Finland

    Why are people constantly bringing up different versions of this and not instead spending that time getting the module ready to commit? How many hours have been spent talking about it in the past year?

    I believe this conversation has kept going because there hasn't been a decision on this. From my end, the conversation has mostly ended because I have been tired of having the discussion, but not because I believed there was an actual decision.

    Bringing the beta requirement into this issue is just making what could have been a much easier decision more difficult - why not have opened a new issue for this instead of completely changing the scope of this one?

    I wanted to bring the beta issue here, because I thought it was better to be transparent about our thoughts regarding the beta requirements before making a decision regarding the alpha requirements. Bringing it up later could have come up as a surprise, which could have made it look like we're trying to get rid of the requirement altogether by removing it from the requirements whenever we're deciding on requirements for the current milestone at hand.

    I'm happy to see that by bringing this up, we surfaced some blockers to removing TUF from the beta requirements that seems like could be addressed. I'm fine with moving that discussion to a follow-up issue.

  • 🇬🇧United Kingdom catch

    Bringing it up later

    In its own issue doesn't imply later.

  • 🇫🇮Finland lauriii Finland

    We should probably update the issue summary since it specifically asks the following questions at the moment:

    Is TUF integration an Alpha, Beta, or Stable requirement for core inclusion?
    Is TUF integration requirement the same for Package Manager, Project Browser, and Automatic Updates?

  • 🇬🇧United Kingdom catch

    @hestenet gave an update on timelines today.

    The code delivery To support root key management should be end of January - but we need to actually implement and test the root key ritual in Feb.

    [A production deployment of TUF] could happen by end of Feb, but more likely,

    This brings up two things for me:

    We definitely can't rely on a production TUF until very end of February or into March, if we want to merge automatic updates as alpha before then (which I think we do), we need to make a proper decision here on alpha requirements. It would have been nice if a production deployment happened and this entire issue became moot, but we're not there yet.

    I'm wondering given the DA has TUF on a dev endpoint, would it be possible (and if so, desirable) for us to point an alpha package manager to the dev endpoint? That would completely break the hard dependency between the production deployment and an alpha commit with TUF support - we'd be able to test package manager with TUF etc.

    But, it might involve work both to implement it, and to back it out again when the production endpoint is available. But assuming that all happens in alpha we could skip any upgrade path for that bit I think.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Using the Dev TUF endpoint as default

    Re #39

    I'm wondering given the DA has TUF on a dev endpoint, would it be possible (and if so, desirable) for us to point an alpha package manager to the dev endpoint?

    I don't think this would be possible unless we are ok for Alpha testers to experience major disruption to using Composer.

    Because the TUF protection provided by the system is enforced on the Composer Plugin level that means if TUF does not validate then probably most Composer operations will not work. This means even command line Composer operations would be halted not just Composer operation through Package Manager. This is by design if TUF doesn't validate then no operations against the Composer repository can be considered safe. Even operations that don't deal with drupal.org endpoint directly may behind the scenes make calls to drupal.org endpoint. This is Composer design choice not anything to do with our plugin, we just through errors for the Composer repositories that are TUF protected.

    So if the DA TUF dev endpoint is really dev status and wouldn't be up and reliable as much as the production one then it could really cause headaches. Keep in mind this is for all Composer metadata not just TUF signing.

    so for instance say the DA’s dev TUF metadata expired. or since it is dev say they just had to take if offline for a couple hours to redeploy it, this would mean those sites that were pointing to that endpoint could do no Composer operations until they updated their Composer.json files manually to point to the current production Composer repository and disabled TUF on the repo.

    Test with Dev TUF endpoint

    Although I think enabling the TUF protection via dev endpoint would be too disruptive for tester there is a way we could test TUF protection even if it is not on by default.

    We have created https://github.com/php-tuf/drupal-project(we should probably rename) which is way to setup a drupal/recommended-project style project but have Composer pointed to dev endpoint instead of https://packages.drupal.org/8. A project create with this template will be TUF protected. On the Drupal level we have a PhpTufValidator that simply ensures the composer plugin is present and is configured to protect https://packages.drupal.org/* repos. PhpTufValidator is present in the current Package Manager module it just is not tagged with event_subscriber so its logic is not enforced. In tests we add the tag.

    So either we could leave PhpTufValidator off for this testing, though the Composer plugin would still provide TUF protection or we could make the https://packages.drupal.org/8 URL overridable and have https://github.com/php-tuf/drupal-project depend on test module that would PhpTufValidator extend to protect the Dev endpoint and add the event_subscriber tag.

    I think this method would allow people reviewing the module to test full TUF protection against the dev endpoint in the Alpha stage but then have most testers not have to worry about the unreliability of the TUF dev endpoint(though they obviously would not get TUF protection).

  • 🇬🇧United Kingdom catch

    I think this method would allow people reviewing the module to test full TUF protection against the dev endpoint in the Alpha stage but then have most testers not have to worry about the unreliability of the TUF dev endpoint(though they obviously would not get TUF protection).

    In principle this seems like a very good workaround that would allow end-to-end testing with TUF, without it actually being deployed on d.o production yet, and more flexible than #39.

  • 🇺🇸United States drumm NY, US

    I'm wondering given the DA has TUF on a dev endpoint, would it be possible (and if so, desirable) for us to point an alpha package manager to the dev endpoint?

    Right now this is hooked up to staging.devdrupal.org, which does not have an active packing pipeline. All updates are done by somewhat-manually pulling releases down from production and setting everything off. This currently only includes the 150 most-used projects. So probably not useful to use it at al beyond development.

  • 🇫🇮Finland lauriii Finland

    Updating the proposal based on the latest discussions on the issue. I'm removing the "needs product manager review", but I'm adding the "needs framework manager" review.

  • 🇫🇮Finland lauriii Finland

    We need a follow-up for #40. Including that in the proposed resolution too.

  • 🇬🇧United Kingdom longwave UK

    So the proposal in #40 is effectively using a module as a feature flag to enable TUF for testing purposes. I think this is a good idea; currently we seem to be a bit deadlocked because we only have the TUF dev endpoint and we want to test it, but to migrate confidently to the production endpoint we ideally need some testing to be completed by real world users. So if this can be enabled and disabled at will by testers, this seems like the best of both worlds.

    PhpTufValidator is present in the current Package Manager module it just is not tagged with event_subscriber so its logic is not enforced.

    In 10.2.0 we enabled autoconfiguration for event subscribers, so the tag is no longer necessary - so just double checking that the above statement is still true, as IIRC you are using other "new" features like autowiring in AU already: see https://www.drupal.org/node/3357408

  • 🇬🇧United Kingdom catch

    I've tweaked the issue summary a bit to outline what's described in #40 a bit more, hopefully correctly. I'm happy to remove the RM review tag from this issue once we've got the follow-up open.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Ok here are the 2 follow-ups

    In https://github.com/php-tuf/drupal-project I added Create test module to enable TUF validation in Package Manager. This will mean if you are start a project with this project you will also get this test module you can turn on to point to the dev TUF repo.

    The test module will need a change in the Package Manager contrib module to make the URL Composer repo it expects to be TUF protected changeable, so I created 📌 Make PhpTufValidator able to switch TUF protected repo URL for testing purposes Active .

    Note, the change to the Package Manager contrib will automatically become part of the core merge request when the conversion is run after the issue is resolved. We will make sure the test module in https://github.com/php-tuf/drupal-project will handle working with the contrib module or the core version., for Composer and other reasons the machine of the module is different, automatic_updates in contrib and auto_updates in core

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Updated the summary

  • 🇬🇧United Kingdom catch

    I think the proposal here will allow a sufficiently realistic set-up for people to test this prior to alpha commit (whether it's the test module or because we can do it some other way), so removing the 'needs framework manager review' tag. Will leave it to another release manager to remove the RM review tag so I'm not doing double duty.

  • 🇬🇧United Kingdom longwave UK

    My view from #45 hasn't changed, I don't see an easier way of moving this forward and starting to get test results from real users. Removing the release manager tag as @catch has also weighed in here and there have been no comments or objections from the other release managers.

    I think this still needs the followup for beta discussions requested by @Lauriii in #36 and could do with an issue summary update/review before it can be marked RTBC.

  • Status changed to RTBC 4 months ago
  • Status changed to Fixed 3 months ago
  • 🇬🇧United Kingdom catch

    I think we can mark this fixed, still need to figure out what actually happens with it depending on what's easiest/ready in time but the overall proposal that we enable people to test with TUF without it necessarily being on d.o production seems like a good position compared to where everything is.

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

Production build 0.69.0 2024