Making WordPress.org

Opened 4 years ago

Closed 3 years ago

Last modified 7 months ago

#5352 closed enhancement (fixed)

Plugin Security - Add email confirmation prior to releases being processed

Reported by: dd32's profile dd32 Owned by:
Milestone: Priority: normal
Component: Plugin Directory Keywords: has-patch
Cc:

Description

With auto-updates in WordPress 5.5 becoming as simple as a few mouse clicks, it's time to take a closer look at the Plugin Release flow, to mitigate potential disasters such as accidental and malicious plugin releases.

I'd like to propose that we add an extra optional step into the release flow for plugins, not intended on adding friction, but intending to ensure that plugin releases only get made when they're intended to. A simple Email confirmation.

How it would work:

  • A committer would be able to click a link in an email triggered by their commit that updated their Stable Tag value
  • That link would be a special page on the plugin repo (only accessible via the email link, that link would be timed so maybe 24hrs? expiring once used?)
  • That page would have a confirm button, that signifies the release is intentional (and not just an accident on a committers part, error on WordPress.org's part, and definitely not by someone else who had managed to spot the committers 64-digit crazy-random password by in a video from a WordCamp ..last year).

This would be optional for most plugins, but required for high-usage plugins (after initial opt-in testing proved reliable). I'm not sure what cut-off point should be used for that, 1 Million installs? 100k installs?

The plugins team would be able to bypass the release restrictions to approve a release in an emergency, but only when doing so through a secure tunnel (on WordPress.org, that's is_super_admin()). They would be the only ones who could disable this for a plugin as well once enabled.

Multi-factor authentication (See #77) is often touted as a solution here, but this is intended on being larger than just protection against a single compromised account, consider this protection against malicious intent as well.
By requiring that multiple people sign off on a release we can ensure that we only ship code to WordPress users that those who support a plugin intended on being released, and not just by an accident or disgruntled employee while everyone else is asleep.

Implementation details and sticky points:

  1. This would be implemented by seeing the Stable Tag header change in the plugins readme.
  2. For plugins with it enabled, initially I'm thinking they'll be required to use tags for releases, releases from trunk will no longer be an option. This is so we can use the Stable Tag change as the trigger for confirmation.
  3. Changes to tags after the release is made will be forbidden (or at least ignored), that's so a malicious tag change is ignored. Yes, I know about Tested Up To we'll figure something out.
  4. Once opt'd in, only the plugins team can disable it for a plugin. This is to avoid a compromised account being able to disable it.
  5. Ideally, the committer who committed the release would not be able to be the sole person who approves the release as well, which would effectively make this always a 2+ person scenario. Maybe an exception would be the same person can sign it off, as long as it's not forcefully enabled for the plugin due to level of usage.
  6. The confirmation-of-release page would list all of the releases that are pending that the logged in user can approve (and the status of those releases), in order to make it easier on those who manage multiple plugins to bulk-approve releases at the same time.
  7. Only those who have been a committer on a plugin for >1 week should be able to sign off a release. (Also, Committers should know when a new committer is added - #5351)
  8. Commits to trunk would still be OK and not require sign-off as long as they don't alter Stable Tag, that allows for plugins to preload their strings into translate.wordpress.org or use it as their primary development platform.
  9. Release Pipelines and other tooling should remain unaffected, as long as they use tags for releases. Authors would have an extra email confirmation step, but that wouldn't break their release script I would hope.
  10. Email isn't perfect, and not all emails are received. That's going to be frustrating for authors and the plugins team alike, so perhaps we can allow a logged in user to trigger the email by an "admin alert" for a pending action on their plugin page, or if they follow an expired link.

Thoughts? Have I missed something major here?

Change History (44)

#1 @tobifjellner
4 years ago

Perhaps you'd also want to have a similar confirmation cycle for any changes to existing tagged versions?

#2 @dd32
4 years ago

similar confirmation cycle for any changes to existing tagged versions?

That'd be point 3:

  1. Changes to tags after the release is made will be forbidden (or at least ignored), that's so a malicious tag change is ignored. Yes, I know about Tested Up To we'll figure something out.

Changes to tagged versions has a few other problems for WordPress.org already, so any chance to block those is a chance I'd take.. Having multiple "versions" of a plugin released under one version number is just confusing for security tools.

#3 follow-up: @casiepa
4 years ago

Random thoughts, but I suppose most is covered:

  1. The committer will get an email to click on. Should all committers get this or just the one that committed? Probably just the one that committed as it's just the final step of the process
  2. Putting 'abc' in the Stable Tag would trigger this process. If 'abc' does not exist under /tags the current flow is to consider trunk for further steps. Any safety measure needed here?
  3. Let's say the limit goes on 100k. Today I have 99k and tomorrow I have 100k, will I get a warning about the new way of releasing my plugin?
  4. 1 million? 100k? I would think that even a plugin with 20k installs that would have an issue could damage w.org reputation, so don't put the limit too high

#4 in reply to: ↑ 3 @dd32
4 years ago

Replying to casiepa:

Random thoughts, but I suppose most is covered:

  1. The committer will get an email to click on. Should all committers get this or just the one that committed? Probably just the one that committed as it's just the final step of the process

all committers, as preferably it'd be a different committer to sign off (so it's 2+ committers, not a solo action). But since many plugins are single active committer it's really just a final confirmation in those cases.

  1. Putting 'abc' in the Stable Tag would trigger this process. If 'abc' does not exist under /tags the current flow is to consider trunk for further steps. Any safety measure needed here?

If the tag doesn't exist then it's treated like trunk and processing should abort then and there.

  1. Let's say the limit goes on 100k. Today I have 99k and tomorrow I have 100k, will I get a warning about the new way of releasing my plugin?

I think we'd have to have a "congratulations! You've got 100k active installs, now.. here's what it means for you.." email. For default settings it'd just be an extra two clicks.

  1. 1 million? 100k? I would think that even a plugin with 20k installs that would have an issue could damage w.org reputation, so don't put the limit too high

I would hope that we can decrease the limit over time, realistically there should be no reason why it couldn't be required for every plugin to have at least one committer sign off, and to have higher usage plugins require two.
But that would require us to also support releases from trunk which I'm not too enthusiastic about adding at first :)

#5 follow-up: @Ipstenu
4 years ago

It's opt IN? I'm in :)

I would like to EVENTUALLY make it opt-out and not opt-in, as updates are a big deal and this would give folks an extra layer of protection. But I can fight for that one later.

Two major thoughts, and they're both around semver stuff :)

First, I see a small drama with

  1. Changes to tags after the release is made will be forbidden (or at least ignored), that's so a malicious tag change is ignored. Yes, I know about Tested Up To we'll figure something out.

and

  1. Commits to trunk would still be OK and not require sign-off as long as they don't alter Stable Tag, that allows for plugins to preload their strings into translate.wordpress.org or use it as their primary development platform.

Which is this. When a new version of WP is released, and I update my plugins, sometimes I only update the readmes in trunk and the latest tag. That should (IMO) not require a sign off, but also should be 'honored' and my zips should be rebuilt. Perhaps we could change that to the size of the change? A readme-change to bump the tested up to is fairly small, it should only be at most two readme.txt files (trunk and latest tag).

Second, what about people who use TRUNK as their stable tag? Can we detect that and require confirmation for every single push? While annoying to the developer, it would hammer home the gosh darn point of trunk is not meant like that, please use tags.

Also by using tags, you get the possibility of incorporating rollbacks later! See? Future us'es will be happy!

Less major thought:

  1. Ideally, the committer who committed the release would not be able to be the sole person who approves the release as well, which would effectively make this always a 2+ person scenario. Maybe an exception would be the same person can sign it off, as long as it's not forcefully enabled for the plugin due to level of usage.

As long as this is an "IF you are the only committer, you can sign off. ELSE you need someone else." Then yeah, that's okay. I would prefer to see exceptions to it, personally, since if we could eventually make this a requirement, it's 'okay' to self-approve in some situations. Not everyone is always available to approve in a pinch, and not all dev-shops are created equal in size/performance/etc.

#6 in reply to: ↑ 5 @dd32
4 years ago

Replying to Ipstenu:

It's opt IN? I'm in :)

I would like to EVENTUALLY make it opt-out and not opt-in, as updates are a big deal and this would give folks an extra layer of protection. But I can fight for that one later.

IMHO:

  • Opt-In for all for now
  • Forced-Opt-In and no Opt-out for high usage plugins soon
  • Opt-Out some day in the future.

Which is this. When a new version of WP is released, and I update my plugins, sometimes I only update the readmes in trunk and the latest tag.

Let's be blunt, that's why I included Yes, I know about Tested Up To we'll figure something out. in point three.
It's a silly scenario we find ourselves in where people need to change their readme's in /tags/ to make these kind of changes, they shouldn't have to update the files that exist on sites just to flag that.

I would rather find a way we can work around this, without changing ZIPs, because having multiple versions of a plugin out there with varied file hashes sucks. Off the top of my head:

  • Readme changes are allowed, but doesn't rebuild the ZIP
  • Readme changes are not needed, because Tested up to can be set somewhere else for the latest release (UI option?)
  • Directory Readme isn't included in plugins at all, and it's replaced with a document that links to the WordPress.org plugins page for further details (This won't be liked by some..)

This is only complicated in my mind by the fact that sometimes people want to update a typo in their plugins readme without releasing a new version, or those who constantly change their tags to find the perfect tags to give them the most search ranking.

That should (IMO) not require a sign off, but also should be 'honored' and my zips should be rebuilt.

It's not really about malicious changes (although that's part of it), it's more about plugin.1.2.3.zip changing and not being able to be cached for a long period of time. Just like how plugins that use Stable Tag: trunk don't get versioned zips and plugin.zip is both version 1.0 and 2.0 depending on what point in time you access it. It's a caching and signing nightmare. (Yes, we can (and probably will) start to use plugin.zip?v=$unique_version_id as the url soon)

Second, what about people who use TRUNK as their stable tag? Can we detect that and require confirmation for every single push? While annoying to the developer, it would hammer home the gosh darn point of trunk is not meant like that, please use tags.

Point 2: initially I'm thinking they'll be required to use tags for releases, releases from trunk will no longer be an option.

The alternative is just as you've said, email on every commit.
If a plugin wants the higher-level of security (and they should) they should do things the right way and use Tags! Thankfully only 15% of plugins with >= 100k active installs use trunk, but that's still not a small number.

I'd almost prefer to remove the ability to release from /trunk/ entirely, and instead, add a UI button for "Release version x.y" that Checked the trunk files versions were correct, tagged it for them, and then updated the trunk Stable Tag readme field for them.
(That's complicated for other reasons, but I would totally support it and probably use it myself)

  1. Ideally, the committer who committed the release would not be able to be the sole person who approves the release as well [....].

As long as this is an "IF you are the only committer, you can sign off. ELSE you need someone else." Then yeah, that's okay. [....]

Agreed. I think it should be opt-in as well. Having a single committer should not be a detriment to the author, and we shouldn't result in a state where every committer ultimately just has two accounts just to work it.
I would like to have three states:

  1. No confirmation needed (initial default)
  2. 1 Confirmation needed (default for high-usage plugins eventually)
  3. 2 Confirmations needed (Opt-in, Committer cannot make the release alone but can be 1 of 2 confirmations)

If it makes it easier, I'd even be willing to make the 3rd choice NOT be selectable via the UI, and it be something that only Plugin Admins can enable for a plugin, as long as they're aware it's an option.

For example, I own plugin ACME Inc Gallery I'm a two-person agency, we opt-in to have this confirmation enabled and either of us can sign it off. We require 1 confirmation which can be either of us (So the Committer can self-release, or Person1 can commit and Person2 can do the Email confirmation when they come online later and verify the diff looked correct).

EMCA Inc Best Gallery is our competitor, the owner doesn't trust any single individual because they believe I've infiltrated his employee pool and will destroy them from the inside out. They require 2 Confirmations, Person1 Commits, Person1 Confirms via email, Person2 Confirms it via email, and only once Person2 confirms it triggers the release to go out.

Being able to opt-in for 2 confirmations rather than 1 is intended ONLY for those larger plugins that have enough people backing it, where no individual should be able to unilaterally decide to release an update that changes their plugins from embedding Cat pictures to embedding Dog pictures.

Last edited 4 years ago by dd32 (previous) (diff)

#7 @casiepa
4 years ago

I'd almost prefer to remove the ability to release from /trunk/ entirely

Might be for another trac ticket, but we, translators, would be extremely happy with this!

A translator cannot translate dev if only trunk is used, because the moment it goes on wordpress.org it is immediately production, so people already start upgrading to the new version while the translator still needs to translate the new strings.

And as an extra: There is no easy rollback in case the new version of the plugin does not work in your own environment because the previous version is no longer existing on wordpress.org

#8 follow-up: @chriscct7
4 years ago

Ideally, the committer who committed the release would not be able to be the sole person who approves the release as well, which would effectively make this always a 2+ person scenario. Maybe an exception would be the same person can sign it off, as long as it's not forcefully enabled for the plugin due to level of usage.

I really disagree with the premise of this idea currently, as I think it results in a system that actually has a far more likely tendency to hurt WordPress users than the problem it's attempting to solve.

There's 2 distinct cases:

  • Adding more security for releases, because Autoupdates means anyone with access pushes to lots of sites
  • Trying to prevent rogue employees/committers from issuing releases.

I think these cases are discrete from each other.

I think a really critical, and perhaps incorrect assumption that can be made, is that larger plugins == more employees === more committers. I haven't looked at any data yet, but I suspect from personal knowledge that this is not true for most plugins in the top 40 (the ones with the millions and hundreds of thousands described as this feature being beneficial in protecting and therefore opt-out).

For example, I know a lot of companies who operate the larger plugins only have a handful, generally 1 - 3 committers with access to commit, because currently there is no 2FA support on commits (and less people having commit access therefore is a good thing). Generally most companies that run the larger plugins give the committer status to owners + lead/senior developers.

The result of this is that the above proposal has a tendency in a lot more common scenarios (common than a rogue employee) of causing more harm than will likely be solved.

For example,

  • A plugin with 2 committers for example, would not be able to issue releases if one of the committers is currently on vacation or is sick or just generally away.
  • A particularly concrete example of why this idea can harm users is to consider the autoupdates feature itself. Let's say as a hypothetical example, a plugin pushes an update at 4pm in the company's operating time. 5 hours later, hundreds of users start reporting PHP fatal errors, and an update needs to go out immediately. T his is a huge problem, particularly because of automatic updates in 5.5, because the users now automatically would be updating over the rolling period into the fatal error. The earlier the fix release can get out the earlier the less number of people with broken sites. Currently a committer can push out a fix, and that would save many/most of the sites. If however, this idea of 2 people signoff is implemented, the committer now needs to track down another person, in the middle of the company's operating nighttime, and potentially wake them up to get a release out the door.

I think 2 person signoff for rogue employees should be an opt-in experience for all plugins, as plugins (regardless of size) with large numbers of committers might find it useful.

I think that instead the email sent on committing a tag should be a time limited hash that allows the committer to confirm a release. I know many larger plugins have wanted 2FA on SVN for a while to try to add security for the case of a committer (whose status was knowingly granted and not actively revoked) having their SVN credentials accidentally leaked (for example like in the example Dion gave above with the WordCamp speaker). Adding email confirmation, while not my first choice over say TOTP based 2FA, adds meaningful security to plugins, and I think provides a real value in that it avoids the 2 above examples, and in doing so doesn't have the potentiality to harm end-users, while adding an extra layer of confirmation. One note on TOTP vs Email while I have a preference to TOTP, since .org already requires working emails for committers for the repo, I think email should be offered at least as an option, whether or not TOTP is additionally offered.

Agreed. I think it should be opt-in as well. Having a single committer should not be a detriment to the author, and we shouldn't result in a state where every committer ultimately just has two accounts just to work it.

Agreed with

To summarize, I think the best course of action would be:

2 Person signoff:

  • Large Plugins: Optin always
  • Smaller ones: Also optin always

2FA (via email or TOTP for the committer to confirm a release):

  • Large plugins: optin for a month (to give some time to work out any issues with it and have time to committers to set theirs up), then mandatory
  • Smaller plugins: optin for a month(like previous), then opt-out

#9 follow-up: @chriscct7
4 years ago

As for

Thankfully only 15% of plugins with >= 100k active installs use trunk, but that's still not a small number.

What if we simply require that new, non-security updates for plugins moving forward must use tags instead of trunk for updates?

Last edited 4 years ago by chriscct7 (previous) (diff)

#10 in reply to: ↑ 9 @chriscct7
4 years ago

Replying to chriscct7:

As for

Thankfully only 15% of plugins with >= 100k active installs use trunk, but that's still not a small number.

What if we simply require that new, non-security updates for plugins moving forward must use tags instead of trunk for updates?

Not sure how easy it is to do so, but there could be a git commit hook on trunk that looks to see if version changes in the readme.txt in trunk and rejects the commit with a "Please use tags for new releases. For more information see: (url)" as an idea

Last edited 4 years ago by chriscct7 (previous) (diff)

#11 @Ipstenu
4 years ago

  • Opt-In for all for now
  • Forced-Opt-In and no Opt-out for high usage plugins soon
  • Opt-Out some day in the future.

That would be nice. Any plugin with other a million users for sure should be no-opt out. I understand what Chris' concern is, but I don't think allowing them to NOT click-to-approve will help that.

I think a really critical, and perhaps incorrect assumption that can be made, is that larger plugins == more employees === more committers.

I thought the assumption being made was "More committers == more people who could approve a change." A plugin with 5 million users could still have one committer, and that's (terrifying but) fine. An email confirmation to push a new release in THAT scenario is that it could prevent a hacker from pushing code without permission.

FWIW that actually did happen at least once. Someone's account was compromised and code was pushed.

Anyway. I don't think requiring a second person to sign off is ideal globally. Maybe we should separate those?

Thinking out loud here...

  1. We allow Opt-In click-on-email to approve new versions for all plugins
  2. Plugins over X installs are auto-opt-ed in, no opt out
  3. We allow OPT IN "submitter cannot self-approve a release."
  4. Plugins over X installs must opt OUT of submitter-cannot-self-approve UNLESS they're the only committer

So we would have two options

  • Require confirmation to release new versions
  • Disallow self-approvals

They're both 'opt in' for now, but eventually it would shift over. We could grey out the 'disallow' and not let it be checked if there's only one committer, and conversely could grey it out and not allow it to be UN checked if there are at least 5 committers and the plugin has over a million users or something.

Yes, I said five committers. I got there by looking at https://wordpress.org/plugins/browse/popular/ and grabbing the 4mil+ user ones.

  • CF7 -- 1 committer: They would NOT need an alternate to sign off on release
  • Yoast SEO -- 5 committers: They would need an alternate to sign off
  • Akismet -- 5 committers: They would need an alternate to sign off
  • Classic Editor -- 1 committer: They would NOT need an alternate to sign off on release
  • Woo -- 15 committers: They would need an alternate to sign off
  • Elementor -- 3 committers: They would NOT need an alternate to sign off on release
  • Jetpack -- 9 committers: They would need an alternate to sign off
  • Really Simple SSL -- 2 committers: They would NOT need an alternate to sign off on release

And just to grab one more....

  • Monster Insights -- 2 committers: They would NOT need an alternate to sign off on release

I feel like this would allow us a little more flexibility, while still giving space for people who want to run a tighter shop with regards to deployment.

#12 @chriscct7
4 years ago

Yeah I agree with not allowing opt out from the email confirm, I just disagreed with the requirement to have 2 person signoff.

And agree in that I think the email confirmation should be a seperate option from the 2 person signoff.

If we use Mika's thinking out loud item list (which I agree with the 4 item list) the only thing I'd disagree with is:

conversely could grey it out and not allow it to be UN checked if there are at least 5 committers and the plugin has over a million users or something

There's only a handful of plugins that'd qualify at this level, and I feel like they're best equipped to figure out require 2 person or not works for them and do so responsibly. Maybe that's a bit naive but I digress.

Last edited 4 years ago by chriscct7 (previous) (diff)

#13 @casiepa
4 years ago

@Ipstenu

So we would have two options

Require confirmation to release new versions
Disallow self-approvals

And do we want to force the use of Tags folders? So no release from trunk?
(I would be in favor)

#14 follow-up: @Ipstenu
4 years ago

I'm sitting on the fence about disallowing 'trunk' as a stable version. I (personally) laud it. I'm also aware that people like to dev how they like to dev.

Thankfully only 15% of plugins with >= 100k active installs use trunk, but that's still not a small number.

We have ~57k active plugins at the moment. How many of those (raw numbers) use trunk?

My gut tells me we should make it by steps.

  1. Write something for developer/plugins that explains why you don't use trunk
  2. If you use TRUNK as stable, you get alerts/warnings on every commit. An email "Hi, using TRUNK for your stable release is not recommended [link to article we need to write]"
  3. If you use TRUNK as stable you see (when you visit your plugin page) a warning about using trunk.
  4. Make a make/plugins post about trunk, and give a time-frame for no more support
  5. Email everyone left using trunk with a link to the article and the documentation
  6. On the date, stop allowing trunk

We can also say "disable auto updates for plugins using trunk as stable" though I don't know if the API (as is) is robust enough to handle that. It's something we should consider.

#15 @casiepa
4 years ago

  1. Write something for developer/plugins that explains why you don't use trunk

A very very basic start you can find in my article from some years ago: https://wp-info.org/pa-qrg/#faq-only-trunk but I'm sure we can find other convincing topics or wording

  1. If you use TRUNK as stable you see (when you visit your plugin page) a warning about using trunk

Don't forget also the readme validator and similar. I'm adding warnings also in mine, e.g. https://wp-info.org/tools/checkplugini18n.php?slug=formlift

#16 in reply to: ↑ 8 @dd32
4 years ago

Replying to chriscct7:

There's 2 distinct cases:

  • Adding more security for releases, because Autoupdates means anyone with access pushes to lots of sites
  • Trying to prevent rogue employees/committers from issuing releases.

I agree that they're different concerns, but disagree that they should be treated as distinct cases. When a solution covers multiple questions, it's best not to look at them separate from one another as you can end up with a solution that works for one case but fails to work for another in a "good manner"

I think a really critical, and perhaps incorrect assumption that can be made, is that larger plugins == more employees === more committers.

I agree, and that's why double-sign-off should be optional and opt-in - that's to prevent the self-confirm/self-sign-off process.

  1. Disabled - default
  2. Confirmation from at least 1 person (can be Committer) - Minimum for large plugins
  3. Confirmation from at least 2 people (Committer + someone else, or 2 other people) - Ideal situation from a security POV, but optional

I know many larger plugins have wanted 2FA on SVN

I also want 2FA for SVN, unfortunately, SVN doesn't support Multi-factor authentication, unless you switch over to ssh+svn:// which has a whole other set of authentication and security issues that mean it's not currently viable for a shared SVN like plugins.svn with untrusted users.
The only way to do 2FA with HTTPS SVN is something like.. svn --user dd32 --password supersecret123456 (where 123456 is my TOTP code).

#17 in reply to: ↑ 14 @dd32
4 years ago

Replying to Ipstenu:

I'm sitting on the fence about disallowing 'trunk' as a stable version. I (personally) laud it. I'm also aware that people like to dev how they like to dev.

My only want for it, is that it makes the implementation of this a LOT simpler and has other benefits.
When I say it makes it a lot simpler, I really mean it's a lot simpler (to the point, that I actually consider it a requirement), the edge cases that need to be handled are significantly decreased and errors on WordPress.org would be significantly reduced.
If it is a massive issue, then we could consider a "You bumped the version in trunk, let's tag a release!" email that has a button (like I mentioned above) that both a) confirms the release action and b) tags everything for them.

Thankfully only 15% of plugins with >= 100k active installs use trunk, but that's still not a small number.

We have ~57k active plugins at the moment. How many of those (raw numbers) use trunk?

Raw numbers. (This differs from the 15% number quoted above, as I used >100k instead of >=100k)

Active InstallsPluginsUsing Tags Using Trunk
>=100k40432981%7919%
>=10k2,3991,67170%72830%
>=1k8,845 5,50962%3,33633%
All56,87827,09748%29,78152%

That shows much closer to what I thought - the larger a plugin the more likely they are to use tags.

We can also say "disable auto updates for plugins using trunk as stable" though I don't know if the API (as is) is robust enough to handle that. It's something we should consider.

The API is, but it won't be a good user-experience as core implementation isn't robust enough for it.

This ticket was mentioned in PR #17 on WordPress/wordpress.org by dd32.


4 years ago
#18

  • Keywords has-patch added

This is a Work in progress PR for adding release confirmations for plugin releases.

What this means, is that an email will be sent upon a new version of a plugin being released, and the import of that plugin will be deferred until appropriate responses are received from the required number of people.

This is a draft PR, for visibility, logic is still being discussed on the trac ticket.

Trac Ticket: https://meta.trac.wordpress.org/ticket/5352

#19 follow-up: @Ipstenu
4 years ago

So basically half of ALL plugins use trunk. Ew.

When I say it makes it a lot simpler, I really mean it's a lot simpler (to the point, that I actually consider it a requirement), the edge cases that need to be handled are significantly decreased and errors on WordPress.org would be significantly reduced.

Okay. Then we need to figure out how to get the half of plugins to do it :D

We can also say "disable auto updates for plugins using trunk as stable" though I don't know if the API (as is) is robust enough to handle that. It's something we should consider.

The API is, but it won't be a good user-experience as core implementation isn't robust enough for it.

Maybe we tackle that first. "Plugins that use trunk are not eligible for auto-update because we can't be sure you're getting the right version." It's not a GREAT user experience, but it's actually considering their needs first, which is we don't want to break sites.

#20 in reply to: ↑ 19 @dd32
4 years ago

Replying to Ipstenu:

When I say it makes it a lot simpler, I really mean it's a lot simpler (to the point, that I actually consider it a requirement), the edge cases that need to be handled are significantly decreased and errors on WordPress.org would be significantly reduced.

Okay. Then we need to figure out how to get the half of plugins to do it :D

What's your thoughts on comment:6 from me:

I'd almost prefer to remove the ability to release from /trunk/ entirely, and instead, add a UI button for "Release version x.y" that Checked the trunk files versions were correct, tagged it for them, and then updated the trunk Stable Tag readme field for them.
(That's complicated for other reasons, but I would totally support it and probably use it myself)

That could be done through this email validation too - The approve-release option could trigger the make-a-tag action for trunk users and they'd realistically see no change in workflow, commit to trunk, get email, approve, tag happens. Remove their need to deal with svn cp trunk tags/1.2 (since most do it badly anyway)

#21 follow-up: @Ipstenu
4 years ago

I'd almost prefer to remove the ability to release from /trunk/ entirely, and instead, add a UI button for "Release version x.y" that Checked the trunk files versions were correct, tagged it for them, and then updated the trunk Stable Tag readme field for them.

My biggest concern there would be miles of re-education.

Second biggest is ... how does that take into account readme only updates for WP compat?

Our own checks would need to improve greatly and make sure the tag doesn't _already_ exist, etc. but that can be done.

Remove their need to deal with svn cp trunk tags/1.2 (since most do it badly anyway)

Ouch but true :D (I mess that up too).

#22 follow-up: @casiepa
4 years ago

That could be done through this email validation too - The approve-release option could trigger the make-a-tag action for trunk users and they'd realistically see no change in workflow, commit to trunk, get email, approve, tag happens. Remove their need to deal with svn cp trunk tags/1.2 (since most do it badly anyway)

And the tag would be the 'Version' from the main .php file currently in /trunk ?
Please make sure it's a valid PHP version number (ok, I know a tag could be ABC123, but I think using PHP versionning is easier for everyone).

#23 @Ipstenu
4 years ago

You mean like SemVer?

#24 in reply to: ↑ 21 @dd32
4 years ago

Replying to Ipstenu:

My biggest concern there would be miles of re-education.

Is there really any there though? If it's a plugin using trunk that were opt'd in to using release confirmation (either now, or forced in future) they'd see no major change other than SVN commits that they didn't make after confirming a release - and that act of having to confirm the release IS the re-education step.

Second biggest is ... how does that take into account readme only updates for WP compat?

Let's stop that behaviour then, make people actually bump their tested up to value by making it easier for them to do - anecdotally a lot of plugins don't bump them because it's not worth making a commit for it.

All the other readme updates though.. FAQ, Screenshots, typos.. those kind of things make me think that the readme is really an asset or should use the trunk file and not the latest stable.. but then you have things like Changelogs in trunk which shouldn't be displayed until released, etc..

My point is that it's a messy experience currently because of the old choices made, and we should consider whether we really need to have ongoing support for it.

#25 in reply to: ↑ 22 @dd32
4 years ago

Replying to casiepa:

That could be done through this email validation too - The approve-release option could trigger the make-a-tag action for trunk users and they'd realistically see no change in workflow, commit to trunk, get email, approve, tag happens. Remove their need to deal with svn cp trunk tags/1.2 (since most do it badly anyway)

And the tag would be the 'Version' from the main .php file currently in /trunk ?

I would assume so yes, for simplicity

  • Version: 1.2.3 => /tags/1.2.3/
  • Version: 1.2 => Invalid Version, I'm not taggin' that, do it yourself if you're sure.
  • Version: I.Wanna.Be.Special => Invalid Version, I'm not taggin' that, do it yourself if you're sure.

#26 @dd32
4 years ago

In 10214:

Plugin Directory: Add an initial run at Release Confirmation for plugins.

This is currently only enabled for plugin review members, as it needs some testing in production prior to being available to others.

Notably, this requires that a plugin be using tagged releases, it doesn't handle trunk releases (yet), that will be added next.

See: #5352

#27 @dd32
4 years ago

In 10215:

Plugin Directory: Release Confirmation: Ensure that only one release can exist per tag in the releases array.

See #5352.

#28 @dd32
4 years ago

In 10216:

Plugin Directory: Release Confirmation: add missing variable in svn handler.

See #5352.

#29 @dd32
4 years ago

In 10217:

Plugin Directory: Release Confirmation: Fix the approval flow, by adding a missing variable.

See #5352.

#30 @dd32
4 years ago

In 10218:

Plugin Directory: Release Confirmation: add missing variable in svn handler.

See #5352.

#31 @dd32
4 years ago

In 10219:

Plugin Directory: Release Confirmation: Fix the pending confirmation message on individual plugins, and ensure that the approval text is correct.

See #5352.

#32 follow-up: @Ipstenu
4 years ago

@dd32 There appears to be a bug. Every time I save a plugin (approval, rejection, just making a note), the internal notes are updated to add "Plugin release approval disabled."

#33 @dd32
4 years ago

In 10221:

Plugin Directory: Release Confirmations: Don't log disabling the feature when it was already disabled.

See #5352.

#34 in reply to: ↑ 32 @dd32
4 years ago

Replying to Ipstenu:

@dd32 There appears to be a bug. Every time I save a plugin (approval, rejection, just making a note), the internal notes are updated to add "Plugin release approval disabled."

Bah! Bug or a feature? Just reminding you that they really don't have release approvals enabled!!

Fixed and I'll clean out those irrelevant audit log entries.

#35 @dd32
4 years ago

In 10224:

Plugin Directory: Release Confirmation: Don't disable when it's not enabled. Attempt two.

Previously [10221].
See #5352.

#36 @dd32
4 years ago

In 10255:

Plugin Directory: Allow plugin developers to opt-in to Release Confirmation emails.

See #5352.

dd32 commented on PR #17:


4 years ago
#37

Merged

#38 @dd32
3 years ago

In 10291:

Plugin Directory: Release Confirmations: Link the committer to their profile.

See #5352.

#39 @dd32
3 years ago

In 10292:

Plugin Directory: Release Confirmations: Update the intro text.

Props tellyworth for text.
See #5352.

#40 @dd32
3 years ago

In 10390:

Plugin Directory: Remove unused undeclared variable.

See #5352.

#41 @dd32
3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

I'm marking this ticket as fixed, but I've created two follow up tasks:

#5483 Force-enable release confirmations for high-target plugins
#5484 Add a "Tag and release" button

#42 @dd32
3 years ago

In 11238:

Plugin Directory: When release confirmations are enabled, always build the trunk plugin-name.zip zip.

When release confirmations are enabled, trunk can never be set as the stable tag, so it's always safe to build the trunk zip.

There's still a limitation here - If a commit introduces a new stable release AND modifies trunk in the same commit, the trunk ZIP won't be updated until the release is confirmed.
It'd be nice to still rebuild the ZIP in that case, so a follow up change may still be required.

See #5352.
See #5901.

#43 @dd32
10 months ago

In 12587:

Plugin Directory: Release Confirmations: Allow for releases to be confirmed (and built), that are not the stable version.

See #5352.
See #5901.

#44 @dd32
7 months ago

In 12813:

Plugin Directory: Allow plugins releases to be discarded instead of confirmed.

Sometimes a release should not be released, this allows discarding those releases to cease the warnings.

See #5352.

Note: See TracTickets for help on using tickets.