Git Updater headers now listed as an ERROR for plugin_header_restricted_fields

[afragen]

I'm not sure when this check was added but I think it should be removed or at least made much less stringent. These are plugin headers or comments. I do understand a check for Updates URI for a repository plugin but I don't understand why it would list GitHub Plugin URI as an error.

I would like some clarification and guidance. Thanks.

[afragen]

For the record, the default Git Updater behavior is to update from the Plugin Repository if the plugin is installed from the primary branch.

[ernilambar]

Related #669

[afragen]

FWIW the WordPress Beta Tester plugin has had this header in place for years.

[dd32]

In light of recent events, I can understand why many would be concerned about the inclusion.

But I personally think it's fair and expected that WordPress.org hosted plugins don't include any code to trigger third-party update mechanisms, even if that code is a simple header and/or requires the user to explicitly install another plugin.

Whether something has existed in the directory in the past makes little difference, plugin check simply lets more things be checked ongoing, I doubt the header existed in those plugins at time of submission.

If the check is in the plugin repo category, I think it's fine 🤷 I understand I'll get some hate for this opinion.

[sybrew]

This is against the Open Source ethos.

Headers do not affect WP Core update procedures unless WP Core listens to them.

Those that aren't affecting WP Core directly should be removed.

[sybrew]

The side loader must also be installed via an external plugin and cannot be hosted on .org. This issue should not concern .org.

Blocking that is Apple-esque behavior, for which they're being sued into the ground, and it has no place in a Free and Open Source community and serves no purpose other than imposing undeserved control.

Ref:

1. https://en.wikipedia.org/wiki/Epic_Games_v._Apple
2. https://en.wikipedia.org/wiki/Epic_Games_v._Google
3. https://www.reuters.com/legal/epic-games-accuses-samsung-google-scheme-block-app-rivals-2024-09-30/

[swissspidy]

I think there's a bit of a misunderstanding here about the intention of the change.

My understanding of #669 is that PCP wants to block such side loading mechanisms from being added to the repo (because it's not allowed in the repo). This is a tricky thing to do with static analysis. Flagging such headers is the next best thing I suppose (though I think we also have some other code checks for that).

I see how this causes such unfortunate side effects like in the examples given here. A header by itself indeed does not cause any harm. So maybe that can be revisited?

[octalmage]

I really do hope it can be revisited, not allowing developers to link to their development repositories using the method that’s been used for years is unfortunate. I’ve always considered this a best practice, and a way to encourage bug fixes and other patches.

[alvarogois]

Surely just a coincidence.

[danieliser]

It’s not consistent with guidelines at all. They state you cannot include code that will update from another source, this is neither code nor capable update updating from another source, never has and they know it.

Let’s be clear, remove GitHub updater from the equation since it is neither part of a submitted plugin or available in the repository. Its existence is of no relevance.

Given that fact, the header is benign with no real meaning.

Are we to start banning any random comments now effectively?

You won’t convince me that this is just ill-timed coincidence. No such thing at this point.

Show me one ☝️ issue this has caused for a user who didn’t have the updater manually installed from GitHub.

For the record we use this to let users test beta and bug fixes reliably without sending them zip files that gmail eats or having them have to download from got themselves.

This has a stink all over it of forcing lockin, making all compliant plugins even more susceptible to Matt’s hijacking antics in the future.

[jb510]

To extend what Daniel said very well, plugin developers have LONG added their own information to the plugin header comment block. There has never been a restriction to doing so, nor should there be a restriction on doing so, including but not limited to alternative update URIs.

[afragen]

#720

[chriscct7]

On October 1st, we made Plugin Check mandatory for new plugins into the Directory.

At the time the post was written, we talked about the Team's expectations for seeing great improvements to the new plugin review queue as a result. The post also mentioned how the Team was going to continue to work on adding more checks to Plugin Check to allow plugin authors to catch and fix issues in their plugins before submitting them into the repository.

For the last several months, the team has been working through a list of checks to build and implement in various categories. One of the regularly seen items that is caught during initial review of plugins is plugin authors bundling external updaters (not allowed) or setting the Plugin Update URI Header (generally due to a misunderstanding where they think it's a mandatory header they must set). As a result, about a month ago, a series of checks were written with the intent of detecting these common issues and see if there was a way we could stop them by having Plugin Check flag them. By catching these before the plugin is submitted, it saves valuable reviewer time, which can go into other tasks like reviewing more plugins, improving Plugin Check, and so forth.

Something important to remember, is with over 60,000 plugins in the WordPress.org repository, there sometimes will be mistakes made by the team members implementing these checks. Even with the volume of plugins in the repository, not every reviewer has seen every edge case that exists from the tens of thousands of plugin authors who write code that goes into the Directory.

In this case, one of the two checks of a particular set, the one designed to catch plugins that included a bypassing copy of an external updater, was written and checked only for the presence of the header. Just checking the header makes it easy for this checker case to be written, and was designed to be a quick, low-impact win. However, the use case of plugins including only the header wasn't as well understood by the team members working on this issue, and it unfortunately was missed while trying to stop people from bundling the entire Git Updater Library.

Yesterday (Sunday), @afragen reached out to the Plugins Team about this case. As is standard for all cases where a suspected false positive is occurring, the Plugins Team asked him to open a GitHub ticket (this ticket) with the details of the case (what check is failing, examples, etc.), and that the Team would investigate it. The Team is highly active in the development of the plugin, and investigates these cases, but they can, particularly when reported over a weekend, take a couple days to review and sort out, as a determination around the false-positive needs to be made, and then (because the check was designed with a goal in mind that would still like to be accomplished) alternatives are researched thoroughly for the check — and bear in mind, the team members working on Plugin Check also have to do reviews of new plugins submitted, re-reviews of existing plugins, and so forth while this is occurring.

At the end of the day, the Team is aligned on making Plugin Check the best experience possible — the Team has seen the amazing results from it already in many ways (including the plugins-waiting-initial-review-queue hit 0 for the first time last week in many, many years). From time to time, mistakes will be made — this isn't the first, nor going to be the last, issue or false positive — and hard lessons were learned. For example, moving forward, the Team will try to include more background information and intent information on Checks we're working on, so they are easier for community members to help us detect issues with and we're also planning to introduce a Beta check category (or similar) to let people more easily test future proposed repository-required checks before they go live on .Org. But the Team also is committed to fixing any issues with checks and make Plugin Check better every week (more on this in an upcoming Make Plugins roadmap post for Plugin Check).

With regards to this Check, per the Team's much discussion on it today, I'm going to go ahead and merge the PR #720 that @kraftbj wrote (on behalf of the Team, thanks!), and we will continue investigating alternatives to more accurately capture the intent of this check.

Note that the Directory's implementation of Plugin Check is not real-time with this repo, but, as per the case with any false positive, so long as this is the only item it is flagging for your plugin, feel free to reach out to the team at [email protected] and we can help get your plugin into the queue — we're here to help!

[afragen]

Thanks for the support everyone. ❤️

It seems looking to stop the inclusion of Git Updater in submitted plugins, a very reasonable goal, had a slight bump in the implementation. I trust someone will reach out with the next implementation attempt as I'm certain we can find an effective search method.

For the record, the Git Updater plugin should not be included in code submitted to .org. Besides it's not really designed as a library for inclusion in a project.

[danieliser]

Thanks for the clarifications as well @chriscct7, your transparency there is superb..

[sybrew]

Something important to remember, is with over 60,000 plugins in the WordPress.org repository, there sometimes will be mistakes made by the team members implementing these checks.

You only need to implement this check once, whether you have 1 plugin or 50 billion in the repo.
The impact might increase, but the work is all just the same.

The problem is that this restriction was even considered.

Open Source means Freedom to redistribute, and the Plugin Review Team does not appear to align with this Freedom by randomly adding blocks and restricting code.

I praise you for crawling back and admitting the mistake but like many other issues on WordPress.org, this reeks of conflicts of interest and puppeteering.

Now, while I finally have your attention, please remove this unjustifiable asininity next. Thanks:

plugin-check/phpcs-rulesets/plugin-review.xml

Lines 76 to 78 in e993242

	<rule ref="Generic.PHP.DisallowShortOpenTag.EchoFound">
	<severity>7</severity>
	</rule>

[jessuppi]

I will probably be banned after sharing this, but RepoMan 1.5.0 now blocks WordPress.org from sending update notices to any plugin it detects with the GitHub Plugin URI string in the main file: littlebizzy/repoman@1.4.4...1.5.0

Related: afragen/git-updater#1070

From a FOSS point of view, this helps resolve potential conflicts between Git Updater and WordPress.org. And I know that's what we are all fighting for, right guys? The open web 🥹