Plugin policy: a plugin switched off for a person, by an administrator or by themselves, still runs for them #362

New issue

Closed

opened 2026-10-01 10:46:09 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

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

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, following every path that runs a plugin for a person back to plugins/policy.py, and trying each with a policy row set.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at plugins/models.py:246-249 (Connection.plugin_instance); plugins/views.py:44-50 (_plugin_or_404), :205-212 (ConnectionFormView._load), :271-461 (the consent, test, backfill and sync-now views) and :97-100 (the logo view's decide); plugins/registry.py:344-369 (parse_page) and :328-330 (available_sources); plugins/policy.py:51 (GOVERNED_KINDS), :61 (UNGOVERNED_KINDS), :277 (the person's overview) and :318-319 (plugins_for returning an ungoverned kind unfiltered); and core/server_views.py:1021-1029 (_governed_plugins). connected_plugins, which fills the connection pick list, moved from the registry to plugins/policy.py:323 and still only lists. The notifier, store, sync, consent, importer and outbox locations have not moved.

policy.plugins_for() calls itself "the one place the rest of the application asks", and the pages do ask it: Settings → Plugins, the connection pick list and the capture form's list of sources hide or mark a plugin that was switched off. The code that runs a plugin does not ask. An administrator's unavailable or always off, for one person or for everybody, and a person's own off switch (profile.plugins_off), change what is shown and not what the account does: notifiers keep delivering, stores keep receiving documents, syncs keep running, and a connection to an unavailable plugin can still be created. Only a plugin switched off for the whole instance stops, because the registry does not load it.

Related to #95 (closed), which introduced the policy, promised that "the runtime asks the policy before a plugin acts for somebody, in one place rather than at every call site" and asked for a tests/security/ test that "a forced-off plugin does not run for that person". This issue adds the runtime check that was left out and the test: the only test of that name, test_a_plugin_switched_off_does_not_act_for_that_person in tests/test_plugin_policy.py:189, checks the list plugins_for returns and runs nothing.

Where:
- the shared lookup: plugins/models.py:245-248, Connection.plugin_instance is a plain find_plugin(self.kind, self.plugin), and every path below uses it or the registry directly;
- notifiers: notifications/service.py:58-93, _deliver iterates Connection.objects.for_user(user).enabled().of_kind("notifier"); notifications/slow.py:120-136, anybody_wants does the same, so errands are still queued; notifications/webhooks.py:77-90 (pending) and :103 (deliver) send rows already queued and look only at connection.enabled;
- stores: documents/archiving.py:57-58 (store_connections, used by schedule_copies at :77), :108-129 (send_copy) and :173-190 (pending_copies);
- syncs: plugins/syncing.py:99-107, due_connections takes Connection.objects.filter(kind="sync", enabled=True) for the whole instance, and :70-73, sync_connection;
- consent: plugins/consent.py:136 (finish, plugin looked up at :158) and :176 (access_token, at :189);
- connection views: plugins/views.py:43-49, _plugin_or_404 checks only the kind and that the plugin is installed, and :204-211, ConnectionFormView._load, relies on it for create and on plugin_instance for edit. ConnectionConsentView, ConnectionTestView, ConnectionBackfillView and ConnectionSyncNowView (:270-460) run without the policy. The one decide call in the module is the logo view (:96-99);
- sources: plugins/registry.py:362-387, parse_page walks available_sources() (:346-348), the plain registry, and is called from jobs/remembered.py:89 without the person;
- importers: resume/views.py:443-457, the career-file import picks from registry.plugins("importer");
- outboxes: core/correspondence.py:52-63, outbox_for does ask plugins_for(person, "outbox"), but outbox is in neither GOVERNED_KINDS nor UNGOVERNED_KINDS (plugins/policy.py:46, :56), so plugins_for returns every outbox unfiltered (:327-328). The administrator's rows are built from GOVERNED_KINDS only (core/server_views.py:1020-1028), as is the person's overview (plugins/policy.py:286), so an outbox cannot be decided for anybody, and the docstring's "an administrator may switch it off, and then this returns nothing" is not true;
- the wording: templates/settings/plugins.html:119, "Switching one off stops it being used".
What happens: checked against the snapshot with policy rows written directly and one enabled connection:
1. A notifier forced off or unavailable, for the person or for everybody: policy.decide(...).on is False, and notify() still returns 1 and the message is sent. With the webhook notifier, a WebhookDelivery row is written and the scheduler posts it. The same holds with the plugin in the person's own plugins_off.
2. A store or sync in the same states: documents are still queued and copied, and the scheduler still runs the sync, for every connection already made.
3. A notifier unavailable for everybody: it is not on the pick page, but GET /settings/connections/add/notifier/<name>/ answers 200, the POST answers 302 and saves the Connection, and Test sends a message through it.
4. The schema.org source forced off: it is missing from plugins_for(person, "source") and still reads the person's captures. The importer choice has the same shape: it never consults the policy.
Who can trigger it: nobody has to do anything unusual. An account that already had a connection keeps using it after the administrator's decision, and any signed-in person can open the create address of a plugin that is not offered to them. Nothing needs to be forged.
Why it matters: notifiers, stores and syncs exist to move a person's data somewhere else, and switching one off on Server settings → Plugins is how an administrator stops that class of data leaving the instance. The administrator's page and the person's page both say the plugin is off while the scheduler goes on posting events, copying CVs and syncing contacts. A person who switches a notifier off for themselves is told this "stops it being used" and keeps receiving its messages.
Fix: make the policy the gate where a plugin runs, not only where it is listed.
- Give Connection one method that answers whether it may act now, for example active_plugin(), returning the plugin only when policy.decide(self.plugin, self.owner).on is true. Use it instead of plugin_instance in _deliver, anybody_wants, store_connections/schedule_copies/send_copy, due_connections/sync_connection, webhooks.pending/deliver, consent.finish/access_token and the Test, Send everything, Sync now and Consent views. Keep plugin_instance for the list page, which must still show the row.
- Delete nothing, and leave queued deliveries and copies waiting without spending their attempts, as #243 does for a switched-off connection, so that reversing the decision resumes them.
- In _plugin_or_404 and ConnectionFormView._load, answer 404 when decide(...).offered is false and refuse to save when .on is false.
- Pass the owner to parse_page and take its sources, and the importer choice in resume/views.py, from plugins_for(person, kind).
- Add outbox to GOVERNED_KINDS (or govern every kind in CONNECTED_KINDS), so that the administrator's page lists it and plugins_for filters it.

Done when

tests/security/test_plugin_policy_runtime.py, for each of notifier (including a queued webhook row), store, sync, source, importer and outbox: with a FORCED_OFF row, with an UNAVAILABLE row and, for an installed plugin, with its name in the person's plugins_off, nothing of that plugin runs for the person (notify() delivers nothing, send_pending/send_copy and run_syncs do nothing, the capture and the import do not use it); removing the decision resumes the waiting deliveries and copies; and nothing was deleted.
The same file: the create address of a plugin unavailable to the person answers 404, a POST for one forced off saves nothing, and Test, Sync now, Send everything and Consent do not call a plugin that is off for the connection's owner.
All of these fail today and pass after the fix.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, following every path that runs a plugin for a person back to `plugins/policy.py`, and trying each with a policy row set. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `plugins/models.py:246-249` (`Connection.plugin_instance`); `plugins/views.py:44-50` (`_plugin_or_404`), `:205-212` (`ConnectionFormView._load`), `:271-461` (the consent, test, backfill and sync-now views) and `:97-100` (the logo view's `decide`); `plugins/registry.py:344-369` (`parse_page`) and `:328-330` (`available_sources`); `plugins/policy.py:51` (`GOVERNED_KINDS`), `:61` (`UNGOVERNED_KINDS`), `:277` (the person's overview) and `:318-319` (`plugins_for` returning an ungoverned kind unfiltered); and `core/server_views.py:1021-1029` (`_governed_plugins`). `connected_plugins`, which fills the connection pick list, moved from the registry to `plugins/policy.py:323` and still only lists. The notifier, store, sync, consent, importer and outbox locations have not moved. `policy.plugins_for()` calls itself "the one place the rest of the application asks", and the pages do ask it: Settings → Plugins, the connection pick list and the capture form's list of sources hide or mark a plugin that was switched off. The code that runs a plugin does not ask. An administrator's *unavailable* or *always off*, for one person or for everybody, and a person's own off switch (`profile.plugins_off`), change what is shown and not what the account does: notifiers keep delivering, stores keep receiving documents, syncs keep running, and a connection to an *unavailable* plugin can still be created. Only a plugin switched off for the whole instance stops, because the registry does not load it. Related to #95 (closed), which introduced the policy, promised that "the runtime asks the policy before a plugin acts for somebody, in one place rather than at every call site" and asked for a `tests/security/` test that "a forced-off plugin does not run for that person". This issue adds the runtime check that was left out and the test: the only test of that name, `test_a_plugin_switched_off_does_not_act_for_that_person` in `tests/test_plugin_policy.py:189`, checks the list `plugins_for` returns and runs nothing. - **Where:** - the shared lookup: `plugins/models.py:245-248`, `Connection.plugin_instance` is a plain `find_plugin(self.kind, self.plugin)`, and every path below uses it or the registry directly; - notifiers: `notifications/service.py:58-93`, `_deliver` iterates `Connection.objects.for_user(user).enabled().of_kind("notifier")`; `notifications/slow.py:120-136`, `anybody_wants` does the same, so errands are still queued; `notifications/webhooks.py:77-90` (`pending`) and `:103` (`deliver`) send rows already queued and look only at `connection.enabled`; - stores: `documents/archiving.py:57-58` (`store_connections`, used by `schedule_copies` at `:77`), `:108-129` (`send_copy`) and `:173-190` (`pending_copies`); - syncs: `plugins/syncing.py:99-107`, `due_connections` takes `Connection.objects.filter(kind="sync", enabled=True)` for the whole instance, and `:70-73`, `sync_connection`; - consent: `plugins/consent.py:136` (`finish`, plugin looked up at `:158`) and `:176` (`access_token`, at `:189`); - connection views: `plugins/views.py:43-49`, `_plugin_or_404` checks only the kind and that the plugin is installed, and `:204-211`, `ConnectionFormView._load`, relies on it for create and on `plugin_instance` for edit. `ConnectionConsentView`, `ConnectionTestView`, `ConnectionBackfillView` and `ConnectionSyncNowView` (`:270-460`) run without the policy. The one `decide` call in the module is the logo view (`:96-99`); - sources: `plugins/registry.py:362-387`, `parse_page` walks `available_sources()` (`:346-348`), the plain registry, and is called from `jobs/remembered.py:89` without the person; - importers: `resume/views.py:443-457`, the career-file import picks from `registry.plugins("importer")`; - outboxes: `core/correspondence.py:52-63`, `outbox_for` does ask `plugins_for(person, "outbox")`, but `outbox` is in neither `GOVERNED_KINDS` nor `UNGOVERNED_KINDS` (`plugins/policy.py:46`, `:56`), so `plugins_for` returns every outbox unfiltered (`:327-328`). The administrator's rows are built from `GOVERNED_KINDS` only (`core/server_views.py:1020-1028`), as is the person's overview (`plugins/policy.py:286`), so an outbox cannot be decided for anybody, and the docstring's "an administrator may switch it off, and then this returns nothing" is not true; - the wording: `templates/settings/plugins.html:119`, "Switching one off stops it being used". - **What happens:** checked against the snapshot with policy rows written directly and one enabled connection: 1. A notifier *forced off* or *unavailable*, for the person or for everybody: `policy.decide(...).on` is `False`, and `notify()` still returns 1 and the message is sent. With the webhook notifier, a `WebhookDelivery` row is written and the scheduler posts it. The same holds with the plugin in the person's own `plugins_off`. 2. A store or sync in the same states: documents are still queued and copied, and the scheduler still runs the sync, for every connection already made. 3. A notifier *unavailable* for everybody: it is not on the pick page, but `GET /settings/connections/add/notifier/<name>/` answers 200, the `POST` answers 302 and saves the `Connection`, and *Test* sends a message through it. 4. The schema.org source *forced off*: it is missing from `plugins_for(person, "source")` and still reads the person's captures. The importer choice has the same shape: it never consults the policy. - **Who can trigger it:** nobody has to do anything unusual. An account that already had a connection keeps using it after the administrator's decision, and any signed-in person can open the create address of a plugin that is not offered to them. Nothing needs to be forged. - **Why it matters:** notifiers, stores and syncs exist to move a person's data somewhere else, and switching one off on Server settings → Plugins is how an administrator stops that class of data leaving the instance. The administrator's page and the person's page both say the plugin is off while the scheduler goes on posting events, copying CVs and syncing contacts. A person who switches a notifier off for themselves is told this "stops it being used" and keeps receiving its messages. - **Fix:** make the policy the gate where a plugin runs, not only where it is listed. - Give `Connection` one method that answers whether it may act now, for example `active_plugin()`, returning the plugin only when `policy.decide(self.plugin, self.owner).on` is true. Use it instead of `plugin_instance` in `_deliver`, `anybody_wants`, `store_connections`/`schedule_copies`/`send_copy`, `due_connections`/`sync_connection`, `webhooks.pending`/`deliver`, `consent.finish`/`access_token` and the Test, Send everything, Sync now and Consent views. Keep `plugin_instance` for the list page, which must still show the row. - Delete nothing, and leave queued deliveries and copies waiting without spending their attempts, as #243 does for a switched-off connection, so that reversing the decision resumes them. - In `_plugin_or_404` and `ConnectionFormView._load`, answer 404 when `decide(...).offered` is false and refuse to save when `.on` is false. - Pass the owner to `parse_page` and take its sources, and the importer choice in `resume/views.py`, from `plugins_for(person, kind)`. - Add `outbox` to `GOVERNED_KINDS` (or govern every kind in `CONNECTED_KINDS`), so that the administrator's page lists it and `plugins_for` filters it. ## Done when - `tests/security/test_plugin_policy_runtime.py`, for each of notifier (including a queued webhook row), store, sync, source, importer and outbox: with a `FORCED_OFF` row, with an `UNAVAILABLE` row and, for an installed plugin, with its name in the person's `plugins_off`, nothing of that plugin runs for the person (`notify()` delivers nothing, `send_pending`/`send_copy` and `run_syncs` do nothing, the capture and the import do not use it); removing the decision resumes the waiting deliveries and copies; and nothing was deleted. - The same file: the create address of a plugin *unavailable* to the person answers 404, a `POST` for one *forced off* saves nothing, and Test, Sync now, Send everything and Consent do not call a plugin that is off for the connection's owner. - All of these fail today and pass after the fix.

tiagoagueda added this to the 0.5.0 milestone

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

tiagoagueda added the

security

bug

labels

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

tiagoagueda referenced this issue from a commit

2026-10-04 10:04:58 +00:00

Ask the plugin policy where a plugin runs, not only where it is listed

tiagoagueda closed this issue

2026-10-04 10:04:58 +00:00