Plugin policy: the 'switched off for the instance' step compares a plugin's name with package names #596

New issue

Closed

opened 2026-10-01 10:48:09 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-01 10:48:09 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading plugins/policy.py against the plugins record.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at plugins/policy.py:109-115 (_switched_off_for_the_instance), called at plugins/policy.py:175 in _decide; disabled_names moved to plugins/record.py:208-210 (#248) and is still imported through plugins/installing.py. The registry's filter and the test at tests/test_plugin_policy.py:100 are where they were.

Step 1 of the order the policy module documents ("Switched off for the instance") is implemented by _switched_off_for_the_instance, which checks canonicalise(name) in disabled_names(). decide() is asked about plugin names (imap, apprise), but disabled_names() holds the record's distribution names (postulo-imap, postulo-apprise). For every official plugin the step can never match. Nothing breaks today, because the registry already leaves out every plugin from a disabled distribution, and every caller of decide() asks about a plugin it found in the registry. Still, the guard the docstring promises is not the one doing the work, and the test that covers it passes only because it uses one string for both names.

Where: plugins/policy.py:104-110 (_switched_off_for_the_instance), called at plugins/policy.py:184 in _decide; plugins/installing.py:316-318 (disabled_names, built from entry.name, the distribution name); the filter that really applies is in plugins/registry.py (_disabled and _distribution_of, which compare entry-point distributions); test tests/test_plugin_policy.py:100 (test_switched_off_for_the_instance_beats_everything) writes a record line named "schema.org" and asks decide("schema.org", …).
What happens: with postulo-imap disabled, policy._decide("imap", person) skips step 1 and goes on to the policy rows. The answer comes out right today only because a disabled plugin never reaches decide(). A future caller that asks about a plugin by a stored name (a Connection row's plugin, for example) would get "available" for a plugin the administrator switched off. The opposite also holds: a plugin whose name happens to equal a disabled package's canonical name is reported "Switched off for this whole instance" though it is loaded.
Why it matters: this is a maintainability cost, not a present defect. The check looks like protection and is not, so the next change that relies on it inherits a gap that no test will catch.
Fix: either map the plugin name to its distribution before comparing (the registry already knows each entry point's distribution), or drop the step and change the docstring to say that the registry's filter is the instance-wide switch. In both cases, make the test use a distribution name and a plugin name that differ.

Done when

tests/test_plugin_policy.py disables a record line named postulo-example that provides a plugin named example, and asserts that decide("example", person) answers decided_by == "instance" (if the step is kept), or the step is removed along with the docstring's claim and the test is rewritten to show the registry hiding the plugin.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `plugins/policy.py` against the plugins record. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `plugins/policy.py:109-115` (`_switched_off_for_the_instance`), called at `plugins/policy.py:175` in `_decide`; `disabled_names` moved to `plugins/record.py:208-210` (#248) and is still imported through `plugins/installing.py`. The registry's filter and the test at `tests/test_plugin_policy.py:100` are where they were. Step 1 of the order the `policy` module documents ("Switched off for the instance") is implemented by `_switched_off_for_the_instance`, which checks `canonicalise(name) in disabled_names()`. `decide()` is asked about plugin names (`imap`, `apprise`), but `disabled_names()` holds the record's distribution names (`postulo-imap`, `postulo-apprise`). For every official plugin the step can never match. Nothing breaks today, because the registry already leaves out every plugin from a disabled distribution, and every caller of `decide()` asks about a plugin it found in the registry. Still, the guard the docstring promises is not the one doing the work, and the test that covers it passes only because it uses one string for both names. - **Where:** `plugins/policy.py:104-110` (`_switched_off_for_the_instance`), called at `plugins/policy.py:184` in `_decide`; `plugins/installing.py:316-318` (`disabled_names`, built from `entry.name`, the distribution name); the filter that really applies is in `plugins/registry.py` (`_disabled` and `_distribution_of`, which compare entry-point distributions); test `tests/test_plugin_policy.py:100` (`test_switched_off_for_the_instance_beats_everything`) writes a record line named `"schema.org"` and asks `decide("schema.org", …)`. - **What happens:** with `postulo-imap` disabled, `policy._decide("imap", person)` skips step 1 and goes on to the policy rows. The answer comes out right today only because a disabled plugin never reaches `decide()`. A future caller that asks about a plugin by a stored name (a `Connection` row's `plugin`, for example) would get "available" for a plugin the administrator switched off. The opposite also holds: a plugin whose name happens to equal a disabled package's canonical name is reported "Switched off for this whole instance" though it is loaded. - **Why it matters:** this is a maintainability cost, not a present defect. The check looks like protection and is not, so the next change that relies on it inherits a gap that no test will catch. - **Fix:** either map the plugin name to its distribution before comparing (the registry already knows each entry point's distribution), or drop the step and change the docstring to say that the registry's filter is the instance-wide switch. In both cases, make the test use a distribution name and a plugin name that differ. ## Done when - `tests/test_plugin_policy.py` disables a record line named `postulo-example` that provides a plugin named `example`, and asserts that `decide("example", person)` answers `decided_by == "instance"` (if the step is kept), or the step is removed along with the docstring's claim and the test is rewritten to show the registry hiding the plugin.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:48:09 +00:00

tiagoagueda added the

tier

enhancement

labels

2026-10-01 10:48:09 +00:00

tiagoagueda referenced this issue from a commit

2026-10-06 03:03:42 +00:00

Drop the policy step that compared a plugin's name with package names

tiagoagueda commented

2026-10-06 09:32:10 +00:00