plugins rollback: switches back on, without saying so, every plugin disabled since the last install #600

Closed
opened 2026-10-01 10:48:11 +00:00 by tiagoagueda · 0 comments
Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the snapshot code and running set_disabled, remove and roll_back against a temporary plugins directory.

Related to #246.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at plugins/installing.py:553-589 (take_snapshot), :609-628 (restore_snapshot), :642-659 (roll_back), :949-957 (set_disabled) and :852-862 (remove); plugins/management/commands/plugins.py:123-146 has not moved. Reading and writing the record now live in plugins/record.py (write_record at :162, #248), which installing hands out under the same names.

The snapshot is taken only before an install, and it includes the record, plugins.json. roll_back() puts back the whole directory and the whole record as they were then, so every later set_disabled() and remove() is undone too. The command compares only versions, so a plugin that was switched off and is now on again is not mentioned at all.

  • Where: plugins/installing.py:727-763 (take_snapshot, which copies _copyable(directory), record included), plugins/installing.py:783-802 (restore_snapshot), plugins/installing.py:816-833 (roll_back), plugins/installing.py:1123-1131 (set_disabled) and plugins/installing.py:1026-1036 (remove), neither of which touches the snapshot, and plugins/management/commands/plugins.py:123-146 (_rollback, which compares {entry.name: entry.version} only).
  • What happens: an administrator installs plugin X. Later they disable plugin Y because it misbehaves with people's credentials, and remove plugin Z. Then they run manage.py plugins rollback to undo X. Y is enabled again in every process, and nothing in the output says so; Z's files and record line return, with only "Put Z 1.0 back." to show for it. Checked: "before rollback [('plug-a', True), ('plug-b', False)]", then "after rollback [('plug-a', False), ('plug-c', False)]": plug-a re-enabled and the removed plug-c restored.
  • Why it matters: a plugin switched off on purpose, possibly for a security reason, starts running again for everybody as a side effect of undoing an unrelated install.
  • Fix: on rollback, restore the files, then rewrite the record from the current one, replacing only the lines of the plugins the last install changed and keeping every other entry's disabled flag; or spend the snapshot whenever set_disabled or remove writes the record, so a rollback never crosses a later decision. Have the command print changes to the disabled state as well as versions.

Done when

  • tests/test_plugin_rollback.py has a test that installs two plugins, disables the first, rolls back the second, and asserts the first is still disabled (or that the rollback is refused), and a command test that the output names any plugin whose disabled state changed.
Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the snapshot code and running `set_disabled`, `remove` and `roll_back` against a temporary plugins directory. Related to #246. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `plugins/installing.py:553-589` (`take_snapshot`), `:609-628` (`restore_snapshot`), `:642-659` (`roll_back`), `:949-957` (`set_disabled`) and `:852-862` (`remove`); `plugins/management/commands/plugins.py:123-146` has not moved. Reading and writing the record now live in `plugins/record.py` (`write_record` at `:162`, #248), which `installing` hands out under the same names. The snapshot is taken only before an install, and it includes the record, `plugins.json`. `roll_back()` puts back the whole directory and the whole record as they were then, so every later `set_disabled()` and `remove()` is undone too. The command compares only versions, so a plugin that was switched off and is now on again is not mentioned at all. - **Where:** `plugins/installing.py:727-763` (`take_snapshot`, which copies `_copyable(directory)`, record included), `plugins/installing.py:783-802` (`restore_snapshot`), `plugins/installing.py:816-833` (`roll_back`), `plugins/installing.py:1123-1131` (`set_disabled`) and `plugins/installing.py:1026-1036` (`remove`), neither of which touches the snapshot, and `plugins/management/commands/plugins.py:123-146` (`_rollback`, which compares `{entry.name: entry.version}` only). - **What happens:** an administrator installs plugin X. Later they disable plugin Y because it misbehaves with people's credentials, and remove plugin Z. Then they run `manage.py plugins rollback` to undo X. Y is enabled again in every process, and nothing in the output says so; Z's files and record line return, with only "Put Z 1.0 back." to show for it. Checked: "before rollback [('plug-a', True), ('plug-b', False)]", then "after rollback [('plug-a', False), ('plug-c', False)]": plug-a re-enabled and the removed plug-c restored. - **Why it matters:** a plugin switched off on purpose, possibly for a security reason, starts running again for everybody as a side effect of undoing an unrelated install. - **Fix:** on rollback, restore the files, then rewrite the record from the current one, replacing only the lines of the plugins the last install changed and keeping every other entry's `disabled` flag; or spend the snapshot whenever `set_disabled` or `remove` writes the record, so a rollback never crosses a later decision. Have the command print changes to the disabled state as well as versions. ## Done when - `tests/test_plugin_rollback.py` has a test that installs two plugins, disables the first, rolls back the second, and asserts the first is still disabled (or that the rollback is refused), and a command test that the output names any plugin whose disabled state changed.
tiagoagueda added this to the 0.5.0 milestone 2026-10-01 10:48:11 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Postulo/postulo#600
No description provided.