fix: Skip automatic plugin license writes in read-only mode

[mvanhorn]

Description

Gate only the automatic plugin-license persistence block in Api::processResponseHeaders() on the project config service’s readOnly state. Continue processing non-mutating response metadata, including license-domain and license-info cache updates, so read-only environments can still consume update responses. Leave Plugins::setPluginLicenseKey() and its explicit controller/install callers unchanged, preserving their current validation and read-only enforcement when a user directly requests a license change.

Craftnet API responses can include X-Craft-Plugin-Licenses, which Api::processResponseHeaders() currently persists through Plugins::setPluginLicenseKey(). When project config is read-only, a returned key that differs from the deployed config reaches ProjectConfig::set() and throws NotSupportedException, preventing the Updates utility from displaying otherwise valid update information. The thread’s stack trace identifies this automatic response-header path, and a missing plugin license key supplies a concrete reproduction. A later report about project-config writes during database migrations is a separate deployment flow and is outside this fix.

Fixes #17750

Related issues

Not applicable to this change.

[brandonkelly]

Are you setting craft\services\ProjectConfig::readOnly independently of allowAdminChanges?

[mvanhorn]

Yes — independently. That is the case this gates.

In core ProjectConfig::$readOnly defaults to false and nothing derives it from allowAdminChanges. The only writers are Plugins.php and BaseSystemStatusController, and both set it temporarily and restore the prior value. The $this->readOnly = !...allowAdminChanges assignments in the settings controllers and Cp.php are a separate property on those objects, not the service flag, so allowAdminChanges => false on its own never reaches ProjectConfig::set().

So the failing path is a deployment that sets the service flag itself — project config committed to the repo and applied at deploy, with writes locked at runtime. Api::processResponseHeaders() then persists X-Craft-Plugin-Licenses through setPluginLicenseKey(), a returned key differs from the deployed config, and set() throws NotSupportedException, which takes down the whole Updates utility.

If you would rather it keyed on allowAdminChanges (or on both), say so and I will change it — I gated on readOnly because that is the flag set() actually throws on.

[brandonkelly]

@mvanhorn what is your use case for setting readOnly directly? That property is not intended to be set on its own, independenly of allowAdminChanges.

[mvanhorn]

Sorry, you're right that I keyed this on the wrong property.

To answer directly: I'm not setting ProjectConfig::$readOnly myself. I came to this from #17750, where the reporter's stack trace puts the throw at ProjectConfig::_setInternal() via Plugins::setPluginLicenseKey() from Api::processResponseHeaders(), on a plain GET of the Updates utility. I went looking for how $readOnly became true in that environment and couldn't find a core path: ProjectConfig never reads allowAdminChanges, and the only core writes set the flag to false (MigrateController::runAction(), and the save/restore in rebuild()). So it came from that deployment's own bootstrap, which I have no visibility into.

Which means my guard is built on a property you're telling me isn't meant to stand alone, and I shouldn't have used it as the signal.

The part I think holds independently of $readOnly: Api::processResponseHeaders() performs a project config write as a side effect of rendering the Updates utility. In any environment where that write can fail, a read-only page turns into an error instead of just showing the update info. Happy to re-cut this against allowAdminChanges, or make the license persistence tolerant of a failed write, or close it and leave the fix to you.

[brandonkelly]

The fix doesn’t make any sense to me, because we only pass __REQUEST__ for Craft/plugin license keys when allowAdminChanges is enabled, and on the API side, a new license key will only be generated/returned if __REQUEST__ was passed.

[mvanhorn]

You're right about the mechanism, and I had the wrong model of it. But I don't think the throw needs a new key.

_setInternal throws on $valueHasChanged, and it computes that by comparing against the stored project config value, not against what the API sent. setPluginLicenseKey calls set() for every handle in the response header with no "is this new" check, so the question is only whether the returned key can differ from the stored string.

It can, without any key being issued, when the stored value is an env var reference:

• Project config holds plugins.<handle>.licenseKey: $PLUGIN_LICENSE_KEY.
• getPluginLicenseKey() runs it through App::parseEnv and returns the resolved, normalized key.
• In headers() that key is truthy, so the first arm of if ($licenseKey || ($licenseKey !== false && $allowAdminChanges)) sends it. No __REQUEST__, and allowAdminChanges never enters it.
• If the response carries that same key back, setPluginLicenseKey compares it to the stored $PLUGIN_LICENSE_KEY. The env var branch above it is skipped, because that one requires App::env($matches[1]) to be empty and here it is populated.
• So set() sees $PLUGIN_LICENSE_KEY becoming ABCD…, $valueHasChanged is true, and read-only throws.

That path is worth a look on its own merits, separately from this PR: with writes allowed it doesn't throw, it just silently rewrites the env var reference in project config to the literal key.

The decisive question is one only you can answer: does the API echo back plugin license keys it did not issue? If it returns no X-Craft-Plugin-Licenses at all when nothing was generated, your account is complete, nothing reaches set(), and I'll close this.

For what it's worth #17750 is still open, from a plain Updates view on 5.8.15 in production, so something is reaching set() in that install.