Account created on 31 January 2012, over 13 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States cmlara

Considering punishment has already been announced in 🐛 Clarify policy for revoking security advisory coverage Active is there anything left for this issue to discuss?

All the remaining items I see appear to be "what is the actual policy" and "what changes to the policy do we want to make to retroactively allow the taken action".

🇺🇸United States cmlara

I don't want to get into the specifics. I will say, it was not a single action or single violation of one term in the terms and conditions that resulted in removing this user's ability to opt projects into security coverage.

We will need that data here, the community can not participate in clarifying the policies if the alleged issues are not made clear to us.

As I noted in my above the only item I can see is the reporter tagged a release early. Everything appears to be either a dislike in communications or an annoyance at an unskilled developer who by all accounts meets the minimum defined standards as outlined in their previous approval to the Security Opt In permission.

At worst perhaps they failed to adhere to the Credit Abuse Policy in reviewing their code, however that is separate from the Security Opt In permission and contains its own punishment procedures.

Side note:
The maintainer did offer to have one of their project that had all the security concerns removed from Security Coverage, the security team did NOT appear to take this offer which would have been much less controversial, as such the Security Team needs to be held to an even tighter standard here.

🇺🇸United States cmlara

Note: As far as I can see at least some level of fix has been committed to the repo, I'm still in a 30 day promissory lock down on specifics, however I will discuss general discussions key points that I believe lead to this scenario.

It could be argued this belongs over in 📌 Bulk LLM-generated module publishing by bigbabert Active however as the security team made its announcement post here and this is related to the fact that policy was unclear I'm going to continue with the discussion in this issue.

however I do not know how else to describe 200kb 'vibe coded' patches with dozens of out of scope changes, which despite repeated requests to fix only the security issue in a security release,

What I observed was:

  1. An issue was assigned to the maintainer, they generated a patch.
  2. The patch was refined a few times. (I agree the patch itself was somewhat questionable)
  3. During this time the maintainer makes comments they do not believe it is actually a security fault, no direct dispute was made of this until much later.
  4. Another 2 issues were assigned to the maintainer.
  5. The maintainer asserted the original issue patch was related and provided it for testing. This does have security concerns in leaking issues. At no point did the Drupal security team inform the maintainer not to leak the code over. As a reporter it is not my place to tell them not to provide me this patch.
  6. The maintainer was informed the patches provided no fixes to the additional reported issues
  7. The maintainer provided the additional fixes ON TOP of the patch from the first issue
  8. Patches were refined based on (basic) "resolved/not resolved" feedback.
  9. The maintainer asked to make a single release with multiple issues.
  10. A security team member marked an issue RTBC and concurred on a stacked release. RTBC also includes the automatic "release window" text as a footer in italic. At this point we now have a scenario where it is reasonable that the maintainer believed they have permission to combine the issues and that when reviewed they can be posted)
  11. A second issue was reviewed by the reporter and noted that they were unable to duplicate the issue (the patch was heavily stacked at this point with 3 issues of fixes combined). It is reasonable to believe the maintainer might have taken this as 'consent' albeit they missed the DST approval stage.
  12. The commits were made to the repo for issues 2 and 3 with partial of the 1st issue since it was already stacked in and a release tagged for DST release. This is the first 'major' breach in policy in that parts of two not fully approved issues were made, however from the previous steps we have reasonable consent from the Drupal Security Team. It is also important to note that these issues were constrained to the same reporter while the 3rd was a separate reporter indicating another delineation to a possible release separation
  13. The concerns continue as the maintainer is informed that release is waiting on an additional issue, the maintainer appears to have not realized
  14. The maintainer continued working on the 3rd issue in a stacked setup including the code from the fixes for issues 1 and 2.
  15. The reporter confirmed no longer being able to duplicate the issue, however re-affirmed as they had in other issues they were providing a bare basic functional test and did not review the code.
  16. The Drupal Security Team makes its first attempt to unstack the patches and remove out of scope code.
  17. The maintainer removes unrelated cleanups from the patch however does not unstack them, mentioning the previous agreements for a single release. At this point we clearly have a maintainer that is of the mindset that everything needs to combine into one single patch to resolve without a firm grasp of a delineated workflow. The situation is also degraded by the commit of the previous 'release' which large size created an environment were patches are less likely to apply.
  18. A reporter makes a note that normally security issues are handled in isolation, however as it is not their responsibility (and the reporter has some question on their authority as they were added after initial report to confirm/deny duplicate status) makes no clear direction to the maintainer that the security team absolutely is asking for them to be unstacked and for the maintainer to combine them just before tagging the release.
  19. The Drupal Security Team announced revocation of security coverage without trying to clarify what a 'single release with multiple vulnerabilities' would look like

Given the above my personal opinion both the maintainer and the Drupal Security Team contributed to the situation of the patch files growing in size. I do not assert that the DST could have foreseen it would have taken this path until it had reached its ultimate conclusion.

I will also add their was significant tension in the issues regarding "Needs team response" which was used any time the post needed DST guidance. I personally find this fitting as the issue was in a state where neither the reporter nor the owner are really could make a ruling and asking for guidance. The issue thus contained a bit of "issue status flame warm" as we see on D.O.

that the module had approximately three installs

Unless the security team pulled the Download logs directly they used a known unreliable data set (for context, zero of my sites allow telemetry updates to be sent, its just standard operating procedure for deployments). Again I still agree this is very unlikely, however it points out to me further concerns in the teams decision making process.

- the existing policy covers it,

I disagree, as far as I can see the only real transgression I see is we have is tagging a release which as noted above there is reason to believe the DST played a significant role in this portion of the incident occurring.

has been open in the public issue queue for three weeks.

I will note I have experienced a similar issue, I followed a published procedure on D.O. and was in a release timeline for a security release, the documentation was changed in the Documentation Queue without ever informing me as the code owner. The change was made when a release and been committed and tagged in the middle of a security release window to prevent publication. T

here is a know behavioral problem from the Drupal Security Team not informing users of relevant discussions and pretending the fact they are in the public queue and could be found as sufficient. Even in this case where we are aware of the issue existing there are limitations on what could be discussed until the security issues clear.

As someone who has had my own D.O. access threatened for discussing what I perceive as mishandling of security reports by the DST I am reserved how much I was going to go to bat for someone of lower quality skill until the issues are public As such directly related comments were NOT publicly discussed in that issue which means the issues are incomplete on their feedback from relevant parties especially as being aware of the private issues I expected they were still progressing per policy.

🇺🇸United States cmlara

Disclosure: I was one of the reporters of the vulnerabilities, I know first hand what happened on multiple issues however I can not assert that I am aware of 100% of issues that may have been opened. I am listed on the Unsupported SA by my request (with primary intention of there being public record that yes I did indeed raise concerns through the private tracker). I understand why the DST would want to revoke the users opt-in permission, however I do not agree with how they handled it given the currently documented policies.

Do I trust any code this individual writes, no, would I recommend site owners consider uninstall any code this individual maintains, yes, Do I wonder how this user made it through the first opt-in review process, yes, do I feel this indicates flaws in the opt-in process, yes, do I believe D.O./DST should be removing users without them being given a a policy to refer to, no. This issue existing is evidence that D.O. was not ready for this. In the future I encourage D.O. to write its policies before the action is taken, not after.

In fairness to the DST this incident did reveal a set of scenarios that I had not yet encountered when dealing with maintainers, it was certainly inevitable that some day I would. When the DST did being to involve themselves in more minute details of the fixes they started wrangling the code owner back in, however by that time much of the "damage" was already done.

From the SA:

The project maintainer did not follow the terms and conditions for hosting projects on drupal.org that are opted into security coverage, so the module is losing its security coverage

I want to say this is technically valid to a single term, and that have seen other modules do similar on my own reports and other cases where the DST has admitted a module owner failed to adhere to policies. I will suggest that the DST had some involvement in some of these concerns as they did not provide corrections sooner in some cases. Factor in some communications faults and this was a recipe for disaster that has been growing for some time.

Over in 📌 Bulk LLM-generated module publishing by bigbabert Active it was described as

While @bigbabert has co-operated in the sense of replying on issues, the manner in which they're doing so constitutes malicious compliance for me.

. I suspect the latter part of that statement plays a larger role here. The saying "never attribute to malice what can be attributed to a lack of knowledge" seems relevant here.

I will agree the maintainer showed significant lack of skill, and significant lack of knowledge especially of 'their own code' which is extremely frustrating, however by all interactions I had they did respond which as far as I can tell is the rules as they exist today. I did not necessarily see the 'malicious compliance' on vulnerabilities they received, at most it was a lack of competence in the subject matter, and perhaps some language barriers.

The code owners lack of knowledge did indeed lead to sceanrios where as a reporter I would perform bare minimal support assistance (however any code owner should assume reporters will not assist them and should be happy when we do). This relates to above that as a reporter it is not my responsibility to guide a code owner on how to fix the flaws and is not to manage the DST reporting process.

Other modules, including Drupal Core, have done as bad or worse in my opinion with no punishment over the years.

As one of the individuals who received a 'reported vulnerability' after opening an issue on the maintainers modules I will agree with the DA assessment of questioning if it was malicious, however at the same time I have had a maintainer look at my code (in response to an issue I opened on their module) and find flaws that did need to be addressed so not every cross report is malicious.

I will however note that the concerns could in some cases have been out of module scope. Even in those issues I'm not sure chances to avoid the issues spiraling were well observed or communicated and that the DST could have possible reigned in the issues by being more direct. I also believe some of the DST responses lead to similar deflection in code owners issues as is shown in this thread of at least one incident without 'clear reproduction steps' that I would contend the source code itself was prima facie evidence of a fault, and that a developer should have been able to confirm on visual without full reproduction steps.

As a reporter: There is also a chilling effect if one believes their reports may also lead to being de-listed.

Because if there is security team concern that was agreed to address in a way and then this was done after all the decision was changed in an unilateral way, without notice or nothing, i don't think this is healty community approach to follow,

As a code owner and issue reporter I do have similar concerns. I can not say what the DST did in the background to inform the individual, what I can say is at no point was this raised in the actual issue, I as the reporter was informed my concerns were being dismissed by seeing the public SA and only after the fact received notice in the issue.

At the very least this is questionable as it relates to coordinated disclosure by the Drupal Security Team. As a reporter I was given no time to prepare for official public release and the code owner may have been given no time to prepare either.

A big area I'm not sure about is if we should issue a CVE for any privately reported issues while the project was covered. I think it could be appropriate to issue a CVE.

This is one of the first thoughts that came to my mind when I saw the single SA published yesterday. From both an acknowledgment of security reporters who spent time working the issue (finding the flaws took minutes, working with the maintainer took much more time) to the DST upholding a responsibility to make possible site owners aware of how their sites may be vulnerable (yes this module has only a few reported installs, and yes its possible they are all dev sites however does the the DST has 100% conclusive proof of such).

There is still technically the 10,000 install 'procedure' however that was never adopted into the CNA SCOPE and has not been strictly adhered to since CVE publishing was revamped this year allowing reporters to request the issuance of such ID. I have not yet decided if I was going to raise these issues to a formal request (was OOO yesterday).

Given CVE ID's allow reporters a unified identification of each flaw to facilitate communication I would recommend there is reason to proactively issue a CVE-ID for each flaw that otherwise qualified as a vulnerability.

The modules affected do not have many users - it's possible (even likely) that all the users are development or test sites of the author.

there is auto_translation module that has also post on drop times and is used by more than 250 sites

The other projects did not get an advisory.

As a site owner this one concerns me, 265 installs is indeed not a lot when compared to core, however if I were one one of those 265 installs this decision is absolutely a critical change in the module that I need to know about through an SA (or at least a PSA). That module has releases dating back to January of 2024.

As a site owner this causes me to question the very foundation of the security of any site I operate running code from the Drupal Ecosystem if security coverage will possibly be silently removed at any time how can I recommend Drupal?

I was criticized for using AI. The same people then went on to report security issues.

In my case you were criticized for your assertion about the module that could be contradicted by clearly obvious contradictions in the code, or impossibilities (testing against an unreleased D12, D9 support when using ENUM's only added as a backport to a minor release of D10, etc).

I found myself asking "does this individual understand PHP code" and even "how is this a user with the Security opt In permission, how low are the standards?".

The security vulnerabilities took longer to confirm on a test site than they did to observe in a manual code review (I do mean literal single digit minutes to see the flaws in code). The surrounding code lines I did spot read were so questionably written that I did not expect to be able to actually exploit any of my discoveries as I suspected unrelated bugs would prevent code execution to reach as far as the vulnerability. That said I do wish to temper this that when we stare at the same code over and over again it becomes much easier to become blind to flaws and that could possibly be involved here. On more than one occasions I've had a flaw against a module I maintain that in the hundreds of times I have read the code I never registered the flaw, only for when it to be pointed out it becomes blatantly obvious to the extent of thinking "how did I miss that".

🇺🇸United States cmlara

Linking 🐛 Full Setup not working on 8.x-1.7 Active as it proposed similar for the core portion of the patch.

does this really need blocking on additional tests for multistep forms?

I will note I would not be expecting a test to test everything, only that we end up up without being redirected back to the same plugin, essentially only needing to explicitly test this particular (that it doesn't redirect back to the same setup page).

I will give it a a bit more thought before giving a final decision on this, there is part of me that does want to cleanup bugs, and wishes to avoid excessive burden, there is also a part of me that sees this as one of very few faults that can arise from multi-step forms making it relevant as a 'first start to a test'

🐛 Redirect correctly after first time plugin setup Needs review seems relevant, once the code hits

 $this->messenger()->addStatus($this->t('TFA setup complete.'));

we should not end up on the setup page for another module. We may not need the wording change, although that may indicate there is still something else at play here that we should not be reaching that far into the code execution.

🇺🇸United States cmlara

@smustgrave I was just filling in the record explaining why the tag and why no-comment since it was quesiton by a Project Ownership Queue admin.

Based on the statements from @avpaderno it is not policy that the permission is actually required so should not be a concern. There is some ongoing discussion in 📌 Create governance dcoumentation stored in repo/GitLab Pages Active to try and avoid this type of confusion going forward.

🇺🇸United States cmlara

For context of others.
#19 was part of an attempt Discussed on Slack with @hestenet to avoid increased tensions on issues by only tagging them without comments that might tensions. @avpaderno was in the Slack thread where it was discussed.

The concerned being tagged in #19 was that @zenphp does not appear to have the "Can opt projects into security advisory coverage" permission and that the Password Suggestions (PUG) project is enrolled in the Security Advisory Program.

It has since been stated by @avpaderno in https://www.drupal.org/project/drupalorg/issues/3452333#comment-16095263 📌 Automate the majority of the ownership transfer process (retain human approval) Active that project moderators will not add as maintainer/co-maintainer, is treated by Project Ownership Admins to mean that Project Ownership Admins have the free will to approve/deny such requests and that there is no policy prohibiting such transfers.

🇺🇸United States cmlara

Nothing of what you describe changes moving the documentation pages in the repository,

My understanding from previous conversations is that the pages are editable by even average users today (I can open the edit page, I have not tried saving to confirm), if so that would change as moving it to the repository.

to which any project maintainer for the Drupal.org project ownership would have access, especially when they are project moderators or have a higher role.

Are you saying we do not have a policy for when the document is allowed to be updated or are you saying that we can't trust the Project Ownership Queue admins to not make make nu-approved changes?

That documentation page has been created time ago to describe

If we do not think the current page as-is is useful I'm willing to help write something more governance like based on the current documentation published.

🇺🇸United States cmlara

Should QueueInterface::claimItem() return mapping be updated to include the claim_count?
Can we add a phpstan return type showing the object shape?

Can we gurantee that the claim_count is available on all possible queue types such that putting this on the primary interface will not cause issues?

I don't see an answer to #9 regarding the metadata object lcass, if a dedicated object is not created can the new parameter be object-shaped to aid phpstan diagnosis of what properties are available?

🇺🇸United States cmlara

Is there any edit which defaced the documentation page? F

Your text provides a key concern I have with these pages. It is being described as a "documentation page' not a 'Policy page".

I'm not aware of any deliberate modification to deface the page, although I am aware of lines that may not be written the best as they could be as they use phrases such as "Must" and "will" in a manner that appears to imply these are mandatory non-discretionary steps when other posts by site moderators appear to imply these are general cases, however not mandatory.

Without a formal policy I am unable to ascertain with certainty which of the two interpretations is correct in order to propose documentation edits.

In the past i have tried quoting from the relevant pages and been told these are not policy.

Offering to become a project owner, maintainer, or co-maintainer is not a policy, nor is a documentation page describing the terms of service. Please stop making false claims.

-- https://www.drupal.org/project/drupalorg/issues/3452333#comment-16094934 📌 Automate the majority of the ownership transfer process (retain human approval) Active
The page links both to

https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

At this point I keep receiving conflicting statement a large part of it may be a language barrier with the involved individuals. Having a well defined policy page with a tracked record of every change linked directly to the discussion involved (where not just the text of the policy but the thought process behind the wording is present) would provide more clear context to both understand the policy and to provide references when communications difficulties may be involved.

It is not necessary to move the documentation to GitLab Pages.
While perhaps not 100% necessary it would provide assertions on authenticity.

Drupal Core governance has done this in 📌 Make gitlab pages for governance.drupal.org Active .
The coding standards group is considering the same in 📌 Convert Coding Standards to GitLab pages Active .

I am a fan of governance policies in repo as well

- Tim Lehnen
CTO of the Drupal Association
https://drupal.slack.com/archives/C5ABFBZ5Z/p1746562504563009?thread_ts=...

🇺🇸United States cmlara

Re: #20 I believe that would need a large D.A. policy discussion first, separate from this issue.

Though I will warn I would suggest the D.A. refuse to accept such a policy. The Drupal Community lost uncountable hours of possible contributions in the old system. The long term problem of "unable to find maintainers' can likely track some of situation back to those early policies.

🇺🇸United States cmlara

No further discussion has occurred on this issue.

Closing this as Works as Designed as the 3rd party contrib module has made changes to the Image Generation process.

🇺🇸United States cmlara

I wonder if this will solve that too.

Thinking deeper my issue would likely not be solved by this solution. The module would still be disabled so code would still not exist in the auto loader even if the process occurred sooner.

I believe there is an issue(s) somewhere..?

I thought I saw something in the past, however on a quick search the only issue I turn up is #2536610: Ensure all modules are autoloaded with PSR-4 only if enabled advocating for the exact opposite.

🇺🇸United States cmlara

I'm going to use myself as an example.

I went through the Vetted Process in #3247119: [D9] S3FS SNS Metadata Update .

Never during the process was I instructed to sign an agreement or even asked any of the following:

  • Do you believe you know how to write secure Drupal PHP Code without being prompted?
  • Did you personally write the code being reviewed?
  • Do you agree to diligently review your commits for security concerns prior to submitting them to D.O. (
  • Do you agree to work with the Security Team to resolve issues in modules you maintain? (one could argue this was implied somewhere, however it was never actually explicitly asked when applying for the role.)

During the process is D.O.'s chance to establish a 'contract' with the applicant.

Admittedly none of those questions actually stop me from doing so in the future, however the fact they were not asked I would contend is a an indicator how D.O. does not expect me to act in a special manner going forward, that D.O. failed to reaffirm its expectations of me at the time of role grant.

These are just examples based on some of the limited comments, and why I mention that each revocation should inherently trigger a process review, not every incident will identify a part of the process that can be improved, however those that do will help better define the program and (hopefully) reduce the number of incidents.

🇺🇸United States cmlara

Looks good.

I did add the new page to the menu above exempting authentication provides.

Minor suggestion on possible text change to be a bit more agnostic on providers provided in text.

🇺🇸United States cmlara

Please stop making false claims.

I am referring to the following statements where users have been refused project transfer for not having the security opt in permission.

The project is covered by the security advisory policy; project moderators will not add as maintainer/co-maintainer, or project owner, somebody who cannot opt projects into security advisory coverage, as in this case.
I sometimes contact the maintainers, hoping they will reply back accepting the offer. They are the only ones who can decide to add as maintainers/co-maintainers people who cannot opt projects into security advisory coverage, though.

— avpaderno
https://www.drupal.org/project/marvelous/issues/3492162#comment-16028940 💬 Offering to maintain Marvelous Theme Active

Project moderators will not add as co-maintainers/maintainers people who cannot opt projects into security advisory coverage

— avpaderno
https://www.drupal.org/project/simple_cron/issues/3453929#comment-15651081

@cmlara You are right. I thought I already checked that before, but I obviously got confused.

@Chandreshgiri Gauswami I have to remove you from the maintainers list, for the moment.

— avpaderno
https://www.drupal.org/project/adminer/issues/3441835#comment-15594906 💬 Offering to maintain Adminer Active

🇺🇸United States cmlara

IIRX I’ve hit something similar, in the past.

IIRC I was trying to use a decorator with autowiring and an “ignore if service doesn’t exist” attribute for an optional core service. However since the core module wasn’t loaded IIRC the references to the class caused similar faults.

Maybe that’s not in scope here though I wonder if this will solve that too.

I wonder is this is a sign that Drupal should consider adopting a “more standard” composer layout of having modules always register their namespaces into the composer autoloader instead of the Kernel registering enabled modules?

Obviously adjusting the Kernel would make more sense here as an immediate fix, I’m just spitballing “long term” if it might be time to let composer handle more of this.

🇺🇸United States cmlara

Linking two issues where a Project Ownership Queue Admin transferred projects that were opted in to Security Coverage to a user who did not appear to have the opt in permission required per policy.

The proposed automated gates in this issue would reduce the risk of such incidents.

🇺🇸United States cmlara

After talking with the team, we are curious about the “open issue for logging of nudges.” Can you provide a link

#3177770: Nudge reporting(?) which already attached as a related issue.

My suggestion would be without doing a proper post incident analysis of everyone involved (those warned, those requesting the warning, and those who only observed the warning) the CWG is missing out to understand if the nudges are actually having a positive effect.

I only observe a fraction of the nudges, its possible a majority of the time they have a positive outcome, however without doing proper, data collection an analysis none of us truly know if these should continue in public, moved private, or just be discontinued.

Anecdotal observations:
I've seen a number of incidents where it does inflame the tensions, perhaps not in the thread itself, however every other thread after that will remember the incident. Humans hold grudges and those grudges will make every single discussion after that more tense.

Worse than that, I recall an issue having been silent for an extended period of time when a nudge was posted, this added 'fuel to the fire' of an issue that was otherwise no longer being discussed. Had the nudge never been sent the issue would of likely been forgotten, with the nudge it was remembered much longer and not in a positive view (refer to humans hold grudges).

📌 Bulk LLM-generated module publishing by bigbabert Active may be an example of a 'too soon' nudge, and the chilling effect it may have. Never under estimate the damage of "the thread goes silent". I have observed too many times in my life where continuously internalizing problems has disrupted projects.

🇺🇸United States cmlara

📌 Drupal core will require Queue Factories to implement QueueFactoryInterface Active would be the QueuFactoryInterface issue.

The problem was solved by changing:
(I'm not sure why the previous name doesn't work, might be another bug)

That error sounds like an issue related to Drupal Core moving to closures.

That may be a more significant problem as it implies the queue factory no longer can access our services added during the container build (I hit something similar on another module). The container build process will need to be investigated. The workaround shown will work for the default queue, however adding specific non defaults will be a likely trouble spot.

🇺🇸United States cmlara
if it is deemed unsupportable by the security team.

Is this related to the recent LLM discussions?

What would make a project or user unsupportable in light that the DST is primarily a clerical position? The only item I can think of is “excessive reports”?

Removing projects:
The security team is impacting site owners more than they are impacting the maintainers. The rules for this I would assume would be limited in order to protect site owners.

The Drupal CNA may still need to process a request for an advisory for unsupported projects, refuse it and process a MITRE dispute response (in other words the Drupal Security Team may create significantly more paperwork for itself rather than reducing the paperwork when they revoke a project).

Removing Vetted Users permissions:
I would assume the allowed reasons for this would be fairly limited, the only one I’ve seen published so far is when a user fails to respond to the security team leading to a module being marked unsupported the Security team may revoke the access (not sure which of the pages I saw that on).

Wouldn’t users generally be operating within the context of the permission? The Vetting permission is suppose to determine if a user is a “qualified” developer with a knowledge or secure PHP coding principals.

Any revocation of the user permission raises a question of “how did the user pass vetting originally (and how will the vetting process be updated to prevent it from reoccurring)”.

I do no longer see that part in my user profile, and I cannot see that part when editing other accounts.)

IIRC that was moved to GitLab as part of migration and GitLab now prompts on first use (or presumably whenever the policy inside GitLab is updated).

🇺🇸United States cmlara

@rajeshreeputra

That seems significantly out of scope for this issue.

The latest MR makes the API problem worse.

Adding type hints and return types this would generally need to be a new major only change as it breaks compatibility for those of us who have built modules implementing these interface.

Suggest addressing on the feedback from #9, that rather than assuming the API is wrong perhaps the code itself is wrong.

Consider the fact that most of ecosystem is likely not checking for a NULL return (due to API spec) and allowing it does r make sense. An exception would be better since it should be an abnormal condition.

🇺🇸United States cmlara

Has any post nudge surveying been done? I see there is an open issue for logging of nudges however it appears to never been moved forward.

Would suggest data from those directly involved in the thread, and those who are observing the thread would be useful.

Some questions that come across my mind:

Do they actually defuse the situation or do they raise tensions in the threads?
Does anyone feel that a CWG Nudge was used too soon in a discussion?
Are contributors more aggressive out of concern a nudge will be given on a sensitive topic shutting down discussion before a point can be made?
Are nudges used by 'less experienced' developers or only used by 'well established public figures' of the Drupal Community? (Are they being used to shut-down discussion creating a non-collaborative environment.)

I'm sure others would have suggestions for questions that could be considered in a post incident survey (say 7 days after a nudge is delivered)

🇺🇸United States cmlara
This method can in fact accept arrays for use with multi-value keys.

Can it ?

KeyTypeMultivalueInterface calls for a serialize method to be present which allows converting the multiple value to string. To me that would imply that these should only be strings by the time it reaches the storage layer.

This sounds more like the referenced project was actually using the Key module outside the API and PHPstan correctly flagged it.

Needs work for method signature change, though I believe this is more likley works as designed.

🇺🇸United States cmlara

From MR comments it appears this should be Postponed on 🐛 KeyProviderInterface Active as there is question if NULL is an acceptable return type.

It is used in one location in existing code however is not documented in the API to be allowed.

Until a decison is made to update the API new deviations should likely not be added. (Avoid creating more work for others if the API is correct.)

🇺🇸United States cmlara

The current MR proposes adding return types to the method that conflict with the interface.

The alternative here is that the interface is 100% accurate and the methods returning NULL/FALSE are wrong.

I will note there is no API documented method to indicate an error, it appears the interface was written with the assumption of success always occurring.

🇺🇸United States cmlara

Updating title to better reflect the status of this issue after #14's creation of policy by D.A. staff.

🇺🇸United States cmlara

I am not sure why we mention it as required, but it probably should be changed to recommended for now pending policies.

That appears to be a direct pull from the Issue Etiquette page .

Hestnet added basic policy in comment #14 to the Cresir A use policy and added the Etiquette page text.

Discussion on this issue switched to “does the policy need changes” after that post.

🇺🇸United States cmlara

For the 2.x we are embracing using GitLab Pages for the documentation/manual. This is rendered by mkdocs from the markdown files in the docs folder and published on each 2.x pipeline execution. It is also currently included in each download for offline viewing.

We likely should place this somewhere in the docs (there may be no great location for it at the moment as they are sparsly populated, a new section may be necessary).

I'm going to set back to NW for including in the

For REST logins I would suggest sites I would recommend the site owner consider using an API token auth provider (unless we are talking about the post login to obtain a cookie).

This will impact password access that uses the user.auth service to validate a use. Known scenarios this is relevant for:

  • Password confirmation forms that do not check the database directly
  • HTTP Post to /user/login (this is always enabled in Drupal Core).
  • http_basic authentication

Given the above not sure it if makes sense to limit this to just the rest module enabled.

To me it feels a bit unusual to inline this, though at the moment it is indeed not documented anywhere except in the deep issues logs.

🇺🇸United States cmlara

Note: I am in both the linked Slack threads, I am one of the individual who first hand saw significant flaws in minutes of glancing at the code. I am in full agreement that Advanced File Destination is unsuitable for install on any site in its current condition. I agree that other commenters made points that raise reasonable questions regarding Static Node being functional for the described purpose.

Adding a bit of complication to this discussion:

The individual has published a few other AI written modules in the past, I make no comment at this time in regards to the quality of the code related to the modules in discussion here.

Projects with Stable and Security Covered releases:
https://www.drupal.org/project/auto_translation - 290 reported installs.
https://www.drupal.org/project/graph_element - 10 reported installs.
https://www.drupal.org/project/jw_player_media_source - 6 reported installs

Projects enrolled in security opt in with no stable release:
https://www.drupal.org/project/content_reporting - 42 reported installs

I think that revoking git access and unpublishing the projects temporarily might be warranted

I certainly understand the desire to stop the publication of significantly buggy code before site owners install it and to avoid new code from being published.

I am however a bit concerned that this could lead to a repeat of community disruption that was discussed in #3094854: Plan for dealing with projects that may be abandoned .

Is there a plan to minimize the impact? Thankfully these are smaller install count modules compared to the previous incident, however that does not help the unlucky site owner who is using the module and finds the maintainers unable to commit code.

🇺🇸United States cmlara

Thanks @scott_euser.

I should also put a note in this issue that I have mentioned in a few issues now.

I see many of the "Should have" list as long term essential, however in the past year since this list was originally started each new vulnerability has given more insight into limitations of the 8.x-1.x branch that can not be resolved without the 2.x branch.

While the "Should have" list is long and incomplete at this time, it may be time to consider focusing in on narrowing the 2.x target to more essentials, releasing 2.x (through an alpha/beta/stable) and opening a 3.x branch for additional API breaks.

🇺🇸United States cmlara

Speaking as a dev who targeted D8+ while other devs were responsible for the D7 releases:

Some contrib issues never were assigned to D8 releases and only existed in D7 branches. After EOL I went through my modules issue by issue and migrating several still relevant into the active release branch.

It would seem treasonable to suspect similar to exist across the ecosystem which makes closing contrib issues far more questionable.

If contrib issues were to be closed I highly recommend they be given a unique issue tag so that a future developer can easily query them without needing to sort through unrelated issues.

🇺🇸United States cmlara

First off: I'm aware of the user and modules refereed to in #23, and agree they are poorly written (to the extent I've added them to an internal list of modules to not to utilize and maintainers to avoid).

These modules should be removed from security coverage and their publisher needs to lose their security coverage privileges IMO perhaps based on a strike system.

I am weary of this as an option. Responsible/Ethical/Coordinated disclosure is a key aspect of security. It would be one concept if D.O. did not use wording such as Security issues do not need to be privately reported for the module_name project., with this wording D.O. is actively encouraging public uncoordinated disclosure for those not 'opted in'.

Any action D.O. takes to remove the ability for maintainers to compel private disclosure is an affront to security. D.O. requiring a test and to remain in the 'good blessings' of the Security Team would be conflict with security industry norms.

To put this in another light, how would the community feel if I demanded the Core Team pass an arbitrary test that I personally create, with a drawn out process of months to approve with the ability to revoke based on a condition I decide,, before I would cease disclosing vulnerabilities publicly? I presume the community would be very upset, especially in the case of a Drupelgeddon level vulnerability, and yet that is the standard applied to contrib maintainers today that this suggests extending.

Whatever that occurs for a policy regarding AI generated code there should not be considering the compromising of security privileges in the equation. Consider LLM code spam, consider LLM code unacceptable for commit due to copyright laws, consider it unwanted and target the user on those grounds, whatever is done, do not make a policy that would compromise the ability to request private disclosure.

🇺🇸United States cmlara

Could you please let me know if there's anything I'm missing that would enable me to provide a file path located on a different server or outside the Drupal project scope?

Using the S3fs module with the file at s3://test.txt would be an example that works key when this patch is not applied. Once this patch enforces realpath I would expect the following to no longer function.

> file_put_contents('s3://test.txt', 'testkey');
= 7

> file_get_contents('s3://test.txt');
= "testkey"

> Drupal::service('key.repository')->getKey('s3key')->getKeyValue()
= "testkey"
🇺🇸United States cmlara

@srtmdev I suggest reading the links in #3

Specifically in the what to expect:

Unfortunately, the application queue does occasionally experience a large backlog, and applications may sit in the queue up to a year before getting reviewed. 

There is no Service Level Agreement(SLA) in this process.

The application will be processed, when is a 'wait and see', it could be today, it could be several months before anyone makes another review. Following the priority steps listed in the links given may reduce the amount of time waiting, although there is no gurantee it will.

Note that the process does not enroll your project in the security advisory opt in process, it tests a developer(yourself) to have the permission to do so. It is not the module being tested here, it is yourself as an individual and if/when process completes you will opt the project in not the reviewers in this queue.

I'm assuming based on your issues that your primarily doing this so that the company module is not subject to D.O. suggesting users publicly disclose security vulnerabilities, if so you are unfortunately caught in a portion of D.O. that is not very corporate or security industry friendly and will just have to 'wait it out'.

🇺🇸United States cmlara

I will suggest something along the lines of paratest may be better option for the performance gain and that we phase out the use of run-tests.sh in a v2 in the templates, (or add its as a 3rd option that is preferred).

Especially for those of us who use code coverage that is our primary reason for not using the run-tests.sh job.

I've heard of some issues with paratest in the past though I do not believe I hit any game stoppers in my testing. IIRC I did run across some phpunit options not supported however I believe they all had workarounds. I need to restore working on that project again.

Long term in my eyes run-tests.sh is non-viable, as it is harder to push fixes for.

🇺🇸United States cmlara

As a site owner, I'm going to miss Parentheses, Ampersand and Space., A little less Brackets, Pound Sign and Exclamation Point though I encounter them.

As a security engineer: This is not a bad hardening, however it is certainly not a fix for wherever the faults actually occur.

As someone who has used this in the past to perform sample RCE's against site setups; I can say yes this would make it much harder to exploit.

I also wish to reiterate that for these characters to cause issue someone has had to make some very serious mistakes down stream, mistakes that this change likely does not actually remove the vulnerabilities , just at most makes it significantly harder to exploit (move from Basic to Complex, saving 1 point on the Drupal Vulnerability Scoring ) and maybe move the Impacted Environment from All to Uncommon (2 points on the Drupal Score scale).

I agree with @smustgrave the 'excluded' ideas would be nice to have listed for historical purposes.

🇺🇸United States cmlara
IIRC the reason to keep only latest image is security because distros getting updates

Although the image is avaliable doesn’t mean a site has to use it. We see this all the time on DockerHub older releases are kept so they can be referred to in the future.

This is analogous to arguing all the Drupal 9 core releases should be deleted due to the fact they are known vulnerable.

and PHP has monthly release cycle, so no reason to bloat storage...

First off, retention and semver are different.

Semver would be about ensuring images are generally the same and compatible between build, PHP majors and minors. This could be done even with the current system of never retaining anything except the latest build.

Additional one could set a retention window for images (6 months of same “major” and at least the last major of a semver for a few years?) to reduce storage bloat and yet still allow retaining images.

I will also add that shared storage of being in the same repository could allow image layer reuse further reducing size (this would require streamlining the builds a bit).

🇺🇸United States cmlara

Pulling note in from Slack:

I recommended evaluate including both versions of the package on the images (since these are PHP Extensions this should be fairly straight forward) and possibly add a helper script to allow switching versions.

DDEV does similar in its image design that all the versions of PHP are present and a script sets the active version.

I will note that this is an example of what #3404082: Adopt (semver) versioning for DrupalCi images is trying to address.

Major changes to the CI images that are hard for contrib to work with should generally not be done to existing images without some way to differentiate them. Including both versions and allowing a test run to 'opt in' to the newer version would better align with having well designed and versioned images.

🇺🇸United States cmlara
We can use the realpath() method of FileSystem.

Given the following documentation of the API Method can you provide more justification?

    * The use of this method is discouraged, because it does not work for
     * remote URIs. Except in rare cases, URIs should not be manually resolved.
     *
     * Only use this function if you know that the stream wrapper in the URI uses
     * the local file system, and you need to pass an absolute path to a function
     * that is incompatible with stream URIs.
     *

— FileSystemInterface::realpath()

Does the key module know that a path is a local filesystem? Is this a rare case? If so why? Does the module need to pass an absolute path to a function? If so which function only accepts absolute paths?

🇺🇸United States cmlara

Triaging older open Merge Requests:

I'm inclined to believe this, as currently proposed, should be a 3rd party module and not included in the base code.

I tend to align that plugins should be kept outside of TFA to allow more flexibility and allow TFA to focus primarily on API.

Is there a strong use case that the overwhelming majority of sites would use this feature?

Side note: Also consider IPv6 when this is implemented.

🇺🇸United States cmlara

Only sets the status to enabled in the user settings if a default validation plugin is configured and the plugin has been configured

See #5. We do not want to force that a single plugin must always be used.

Only shows the number of times validation skipped text if TFA is enabled for the user's account

See #3. We absolutely do need this to be present when TFA is disabled for a user.

🇺🇸United States cmlara

Duplicate of 🐛 Deprecations PHP 8.4 Active

Additionally, the patch file workflow is no longer utilized on D.O., submissions should be made using MR’s. See https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... for more details

🇺🇸United States cmlara

While at it we can cleanup the PHPCS, disable the nullability rule on the 1.x branch as we may not be able to implement it, see 🌱 PHP 7.0 Support Active .

And cleanup 2 warnings caused by PHPCS not recognizing a cspell line

🇺🇸United States cmlara

Committed to dev on 2.x and 1.x.

Thank you again for the report on this issue.

🇺🇸United States cmlara

Given the data from #3 (I was under the impression packagist would use releases) it seems reasonable to won't-fix this and instead find (or create) a separate meta to remove packaging customization.

🇺🇸United States cmlara

Please take that into account that while the comments below are not intended to be an 'alarmist' reaction, I am well aware that this particular incident is minor at the moment (and overall is one of the least significant I've encountered in my history with software), the goal is to ensure it does not become more significant and to ensure any future incidents also have a well grounded base in place to avoid them possibly becoming significant.

Should we not seek an opinion from a the licensing working group?

I would suggest no matter what the Core Team/The Project Lead (Dries) will want to run this past legal to ensure they properly unwind this to ensure no future legal entanglement.

I don't think so. This is an existing issue that has already been released. If it would be the first release with it in maybe but that's not the case.

The phrase "Knowingly, willfully, and repeatedly" could be used to describe publishing additional releases or commits after the core team is made aware of the issue. It is one thing for something like this to happen without knowledge there is some room for compassion and more lenient mitigation.

The Linksys GPLv2 case is one of the most notable example of case law on this subject. The platform went from being Closed Source to Open Source GPLv2. An equivalent scenario for the Drupal Project in this case could be that the code no longer qualify as GPLv2 and would be GPLv3 only, not the worst situation that could happen, however I also suspect this is not desired by the Core Team.

Agree, let's just remove the offending code for now, we don't strictly need it here.

Seems reasonable.

🇺🇸United States cmlara

Saw blog post that the next Alpha will be released in the near future.

Should this be classified as an Alpha blocker and release blocker?

The monthly security window I believe is tomorrow, doss this have any impact on that window?

🇺🇸United States cmlara

Responded to MR comment.

wondering whether this needs to be a deprecation (or unsilenced E_USER_WARNING) changing to an exception in 12.x

My personal opinion is no. I can understand why this would be asked since this can be disruptive and given by the code being corrected there is clear proof that some implementations have misused this

However the API is well defined, the proper response is an exception. Code that has properly implemented SerilizationInterface::decode would already have a try/catch block surrounding it.

Today 3rd parties could replace the decoder using a class alias at the auto loader meaning the code that is broken by us fixing this can already be broken by a contrib module following reasonable design standards.

The FALSE return was/is an internal component of the JSON decode method that leaked and should not have been utilized.

As noted in the issue summary, this failure to throw an exception as documented on the API can lead to undetected data loss for those applications that properly follow the documented API.

This bug fundamentally breaks the ability to use DI of the serialization services.

I have no proven case to show this however a worst case fear is that silent data corruption could lead to security incidents.

🇺🇸United States cmlara

Note: Some answers have been provided to this request in Slack #contrib-tfa https://drupal.slack.com/archives/C7SR7TWMS/p1744252987515989 and are duplicated below.

My personal recommendation is that 2.x should not be run by anyone except though who are developing for the next release.

My personal opinion is also that we should not be cutting releases for 2.x until it both has all the known security fixes/hardening committed and any major API overhauls have been completed. Some of the other maintainers have expressed differing opinions encouraging more frequent releases, some of this can be seen in #3416791-6: Resolve SA-CONTRIB-2024-003 in 2.x branch Comment #6 and newer.

I would likely want to see 🐛 Insufficient entropy in loginHash generation Active completed as the security evaluation for it was done before committing 📌 Use an EventSubscriber to process one time login links Needs work , and while I'm not sure there is any more risk now I haven't actually done a full in depth review, rather than auditing the line by line just hardening the hash is preferred.

That said there is starting to become a good argument that it may be worth cutting a 2.x before a full API is established with the intent that a 3.x branch be opened up to continue API breaking changes on the roadmap 🌱 Roadmap for 2.0.0 release Active to get away from fundamental implementation flaws that can not be fixed in 1.x.

🇺🇸United States cmlara

Added a few still open issues into the roadmap

🇺🇸United States cmlara

No, there have not been 19 approvals in two hours. There have been 19 issues whose status was Fixed which have been edited.

Fair, point.

I had performed a random spot audit on the list and all the ones sampled appeared to be NR->RTBC->Fixed in that window.

I have gone through the list and there may have been a 5th that was already resolved prior to yesterday.. Additionally while reviewing I did notice (however did not account for time on every single issue) that one of those may have been reviewed several hours prior as well before being marked fixed in that ~2 hour window.

See also comment #17, to which none of the reviewers replied. It is not that an application is hold on because no reviewer has been able to make a correct review.

I am a bit confused as to what is being said here. Can you clarify for me?

I'm not sure if you are saying saying that it is policy that an applications will be automatically approved if its sits for an extended period of time without review, or if you were saying that applications should be held until they are thoroughly reviewed.

🇺🇸United States cmlara

Apologies for the noise.

Adding a tracking flag for auditing when modules have been reviewed by the queue only for it to be announced after approval that reviewed code contained a security vulnerability.

SA-CONTRIB-2022-051

🇺🇸United States cmlara

Apologies for the noise.

Adding a tracking flag for auditing when modules have been reviewed by the queue only for it to be announced after approval that reviewed code contained a security vulnerability.

SA-CONTRIB-2022-046

🇺🇸United States cmlara

Per SA-CONTRIB-2025-030 versions older than 1.1.0 posses an Access Bypass and DoS. These versions would have been part of the review process.

Tagging as “Vulnerable when approved” for tracking purposes.

🇺🇸United States cmlara

It appears this has self resolved.

I assume it was some form of library that was updated inside of the mkdocs plugins.

🇺🇸United States cmlara

I usually do not like to object in this queue as it is used to provide access to the security opt in feature which should be a common feature of all projects on D.O.

I am flagging as a questionable approval for future tracking purposes.

As noted in #17 the module is essentially an exact duplicate of an existing module.

Looking at the number of approvals in the queue today (19 approvals in 2 hours) I am concerned that the queue maintainers may have rushed through the process after the queue was allowed to develop a sizeable backlog.

I am uploading a diff between the two modules for posterity.

The majority of the changes are:

  • Twig changes of image sources from /modules/contrib/a11y/... to /modules/contrib/accesstools/... (itself likely being wrong since it presumes the module will be installed at the root of the server and under modules/contrib)
  • PHPCS Changes
  • Additional lines in hook_help()
  • A couple of variants of using the logger to record errors.
🇺🇸United States cmlara

Nothing further needed. Just had sent it to tests last night intending to followup after they finished.

No need to port to 2.x as we do appear to use use the library there.

Committed to Dev.

Thank you for tracking this down.

🇺🇸United States cmlara

I have put some comments on Slack however making them more formal:

As a Contrib maintainer I do not see the value.

For me PHPStan exists to identify bugs in code, not future compatibility problems. Not having a return type when the parent method itself does not have a return type is not a bug.

PHPStan scans your whole codebase and looks for both obvious & tricky bugs. Even in those rarely executed if statements that certainly aren't covered by tests.

-- https://phpstan.org/

Looking at a few of the projects I maintain there is a good chance there will be a different branch for D12 and my current branches will never need to implement a hard return type (although in many cases I already added strong code typing as part of implementing Level 9 PHPStan).

A 'feature' like this will just be noise to me, and distract from working on issues more pressing (security hardening, bug fixes, and features).

My opinion is that 🌱 Consider always having the next major branch open Active is the better method to implement, it would allow modules to opt-in to scanning against the next major core branch early to identify where they need to make changes, and reduce creating future technical debt (no need to come back in D12 to remove the type-hint warning).

As an aside. I have suggested in Slack that if this is felt to be useful it might be worth considering putting this in its own third party repository to allow it to be available to others as a way for the Drupal Community to "give back" for all it receives. If this does not make sense to be an independent project I would suggest that should be treated as a warning that indicates other methods should be preferred.

Looking at the MR itself: I note the presence of // @phpstan-ignore phpstanApi.method, should code that uses PHPStan off-api be allowed considering the maintenance burden that could create?

🇺🇸United States cmlara

Edit: Correction on the provenance, it started as CC BY-SA 3.0 not 2.5 (no change in statements above regarding compatibility concerns)

🇺🇸United States cmlara

The motivation to release the information should be separated from publishing CVEs.

CNA rules
5.1.7 a CVE MUST identify the type of Vulnerability.
5.1.1 SHOULD contain sufficient information to uniquely identify the Vulnerability and distinguish it from similar Vulnerabilities.

5.1.7 is the one that likely puts us at most risk It requires we disclose at least a vulnerability type in the CVE. We can get away with just saying a basic (sample I have not looked at this particular advisory lately) "Remote Code exploit", "XSS" , "Authentication Bypass" or other similar category to maintain that compliance, however even to do that we have to disclose that specific bit of information which should likely be done under D.O. coordinating its disclosure of more details (why hide on the D.O. SA that it is a specific type when an attacker can go to the CVE to find that out).

5.1.1, we need to have a very good reason to refute why we do not align ourselves with what we are instructed that we should do. Will the CNA loose its contract for failing to comply with a should, no, will they loose respect in the industry, possibly, will they waste other organizations resources, likely.

We published sa-contrib-2024-075 as an unsupported module with CVE-2024-13311. This report does not appear to uniquely identify the vulnerability or indicate vulnerability type.

Looking at the enrichment data for CVE-2024-13311 if I am reading it correctly it appears that CISA was unable to provide full enrichment due to "not enough information".

Being flagged as "not enough information" should itself be a warning to us that we need to work on our disclosure process.

🇺🇸United States cmlara

it's possible to create CVEs without specifying the CWE/CAPEC/risk score, so the title and some of the description could be updated to reflect that. It may be desirable for some to know that information.

Can you clarify what you are trying to say? You want this issues title updated? Or do you mean that that the title for modules previously published as "unsupported module" should be updated?

🇺🇸United States cmlara

SA-CONTRIB-2025-023:
Curious how documented in the Security Advisory by the reporter and fixer as CAPEC-115 became CAPEC-85?

🇺🇸United States cmlara

Disables users' ability to set up validation plugins except for the default validation plugin if the default validation isn't set up.

I believe we ruled that out in #5.

Even if we were to implement it, that is purely a TFA issue, the plugins should not be involved. Especially not injecting the @internal TfaPluginManager. While TFA could call that service adding it to the TfaBasePlugin would put the internal service into external plugins.

Last I understood we viewed putting default plugin first in the list as reasonable and we were open for researching if it would be possible for Recovery Tokens to not meet the "tfa is configured" provision (without breaking the edge cases such as a user needing to run on recovery tokens while awaiting for new tokens to arrive)

🇺🇸United States cmlara

Does it mean that we can't merge https://git.drupalcode.org/project/rabbitmq/-/merge_requests/58 ? These changes are quite straightforward and allow the module to work in Drupal 11 as well.

It means that at this time I do not feel comfortable merging any more commits into the repository without a clear approval from maintainers added through a less ethically questionable process.

I've reached out to the current owner (who was seen active a few days ago) and have started looking at the fork process as an option if needed.

🇺🇸United States cmlara

Could we start by looking at how we would change \Drupal\Core\File\FileSystem::tempnam() to not use ::getDirectoryPath()?

::realpath() of the scheme would accomplish the same outcome.

Do we need more strict type checks for LocalStream?

If using ::realpath() no, just validate the return is not FALSE. This allows classes that may implement local storage, however not extend LocalStream (such as extending a 3rd party framework to interface with core) to still function.

🇺🇸United States cmlara

This sounds like you've identified a new Feature request for the module. Maybe a test encryption plugin that only gets enabled via a drush command to make testing in general easier?

One already exists as a testing module and we use it as part of unit Functional tests, however Drupal must be configured to use such modules on 'production' sites (not UI accessible to my knowledge).

After this is merged, on UX issues it can help people see the change visually or on multiple devices which screenshots can't really do.

We have very little in the way of UX issues directly, and those that do touch UX do not have any real customization to them, renderings would generally be controlled by site themes and the Drupal Core API meaning we have little need for multi-platform testing on a daily basis, if something is broken it is (in the current code) most likely going to be transferred to core as a FormAPI flaw.

When you are working in your "lab" setup, how easy is it for you to preview the changes on your mobile device?

Fairly easily, un-comment a port setting on the lab config and the site is directly exposed, even easier when its on one of internally routed dev machines where it has an internal hostname assigned to it (although that is rare).

Tugboat previews makes it very easy. It makes it easy for others, without extensive lab set-up to see it too.

That is the part I am wondering how useful it is, I'm involvded in many of the deeper issue so perhaps my vision is a bit narrow. I haven't kept any statistical logging of how often I reached an issue that would be acceptable to approve on just a visual without a lab setup.

Doing so means all co-maintainers and any new co-maintainers would have access to the power and benefit of your local lab type setup.

Fair, though these would likely involve a little more complex of a setup as some of the data is to my knowledge not exposed via direct commands. Nothing impossible to do, just another maintenance point to consider if the burden is worth the return, which is the root question, how often tugboat would actually help move issues forward, if it is is frequent enough it may be worth the maintenance burden

I think you are right, usefulness will differ module to module so it really up to you and the folks who are invested in this module.

The simple community question would be "What issues have you worked on in the TFA queue in the past year or two that Tugboat would have allowed you to avoid a local lab and expend less time/effort testing , and in what way would it have done so?"

🇺🇸United States cmlara

Critically there is no encryption plugin installable through the UI which renders TFA impossible to setup and test inside of Tugboat. NW for resolution

As a maintainer it would be great to have MR environments so I can save tons of time evaluating MRs without needing to pull it locally.

As an actual (co)maintainer, I wonder how useful this is:

With previews only existing 5 days and most issues taking longer will we see an increase in issue noise? (although that may have just increased to 30 days in 📌 Increase Tugboat Preview expiration to 30 days Active , better but still possible noise for longer lived issues if users are closing/opening mr's to generate previews).

Is code updated when new code is pushed? If so are these new environments or continuations of the previous environment?

In this basic form there is a decent amount of setup that still needs to be done from an admin before they can test much/any of TFA's code. Add an encryption key, setup an encryption profile (which requires an encryption module), before TFA can be enabled, add a user, add a token, etc.

As a maintainer In the amount of time that takes I can (if at a terminal) easily checkout the MR and use an existing deployed lab to test (for context in my local lab I have multiple users, multiple tokens, multiple token types, user roles, and requirements setups configured to test most usage scenarios). There may be an advantage to this when I'm on a mobile device without being near a development terminal.

Presumably some of that setup time could be reduced by configuring the environment inside the template.

Beyond that I'm trying to think how often would I use this. I often review code first and then load into the IDE to validate API usage, sometimes with a requirement for a breakpoint in XDEBUG. To perform those operations I would already have the code in my local lab where I can view the changes.

How often would users use this? How many that do use this it will perform an actual test rather than a "it loaded" test which provide us little to no validation. We are not a theme, our issues are not usually visual they are technical. How much of our code is testable without edge cases that require server changes?

I don't have a good answer for those questions. Leaving this open for opinions from other maintainers and community on if tugboat adds value for our workflow.

🇺🇸United States cmlara

Unable to reproduce and the reporter has not provided more information.

I will also add on that had this been able to be traced to an actual bug it likely should have been reported as a private security issue.

🇺🇸United States cmlara

This project is in a bit of a weird spot right now.

I was added as a maintainer through the accelerated adoption process some time back.

That process (along with the entire D.O. adoption process) is subject to some rather significant ethic questions as it is essentially a social engineering and supply chain attack.

Factor in that one maintainers returned for a short period of time last yesr (before disappearing again) raising questions with some of my design choices and it raises even more ethical questions about my right to commit code to the project.

The project has been heavily on hold since those incidents.

I’ve been pondering my options, at the moment the following two look to be the most likely paths:

  1. Owner transfers the project of their own free will (do not use the Project Ownershop Queue) (may not happen due to non responsive maintainer)
  2. Fork into a new project.

It’s a situation that needs to be sorted out for this project to be able to continue on.

Side note:
I’ll note no one has actually tested the code above and reported back.
The user rajan kumar@2026 is well known for not testing code/credit farming leading to disciplinary action on D.O.

🇺🇸United States cmlara

The other options beyond rolling back the API is setting the environment variables or placing the configuration parameters in the shared config file (not the credentials config file).

I have read that CloudFront R2 is also impacted

Minio and Ceph appear to have support for this header (as does LocalStack that we use for the GItLab pipelines) which means the impacted hosts should be limited in general.

🇺🇸United States cmlara

Merged to 8.x-1.x

For 2.x we do not have the same concerns for design, however we can learn from 8.x-1.x fix and consider adding hook_requirements () checks for our core security concerns.

TfaUserSetSubscriber::class would be our prime candidate to monitor, to warn if another subscriber is before it so that site owners can validate security. We would likely want to implement this with an 'approve' list so site owners can approve a subscriber to move it from warning to notice.

We may also wish to monitor the logon routes unless 📌 Evaluate restoring using a form alter instead of extending UserLoginForm Active is adopted, however unlike 8.x-1.x where that monitoring is security critical in 2.x it (should) be only advisory as TfaUserSetSubscriber provides the backend security.

🇺🇸United States cmlara

Was primarily allowing a little more time for anyone who might want to provide feedback since this decently significant code.

This is one the areas that has had security vulnerabilities in the past, including 2 weeks ago (note: the report that triggered two weeks ago would not have been allowed either with existing 2.x branch or the 2.x branch and this code.

That said I believe this is ready to go in and anything else can be followups.

Letting final test run occur after removing unused code and will merge.

Regarding Simple Password Reset:
Recent data indicates Password Reset Landing Page (PRLP) may be better aligned to API and provides a similar feature set. Currently PRLP does not work with this code however PRLP may be in a better spot than Simple Password Reset to modify their process to work with TFA.

Production build 0.71.5 2024