diff --git a/tests/tools/test_permanent_allowlist_reconcile.py b/tests/tools/test_permanent_allowlist_reconcile.py index 67158ce0fc..12c4cd4ff1 100644 --- a/tests/tools/test_permanent_allowlist_reconcile.py +++ b/tests/tools/test_permanent_allowlist_reconcile.py @@ -14,6 +14,8 @@ Reconciling at write time fixes both. The file is re-read and the result is what is on disk now, plus what this process approved since its own baseline. """ +import logging + import pytest import tools.approval as approval @@ -135,7 +137,7 @@ def test_empty_start_and_first_approval(fake_config): assert fake_config["command_allowlist"] == ["ls *"] -def test_save_failure_is_logged_not_raised(fake_config, monkeypatch): +def test_save_failure_is_logged_not_raised(fake_config, monkeypatch, caplog): """The existing contract: a config write failure must not break approval.""" _start_process_with(fake_config, ["ls *"]) @@ -144,5 +146,21 @@ def test_save_failure_is_logged_not_raised(fake_config, monkeypatch): monkeypatch.setattr("hermes_cli.config.load_config", _boom, raising=False) approval.approve_permanent("docker *") - approval.save_permanent_allowlist(approval._permanent_approved) # must not raise + with caplog.at_level(logging.WARNING, logger=approval.logger.name): + approval.save_permanent_allowlist(approval._permanent_approved) # must not raise + assert "Could not save allowlist" in caplog.text assert "docker *" in approval._permanent_approved + + +def test_a_caller_that_passes_a_smaller_set_does_not_remove(fake_config): + """Documented consequence of reconciling: ``patterns`` may only add. + + A future `allowlist remove` built on this function would silently no-op. + The docstring says so; this pins it so the next reader finds out here + rather than in production. + """ + _start_process_with(fake_config, ["ls *", "docker *"]) + + approval.save_permanent_allowlist({"ls *"}) # tries to drop docker * + + assert "docker *" in fake_config["command_allowlist"] diff --git a/tools/approval.py b/tools/approval.py index 1c50dc66b7..6544cc242e 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -413,6 +413,10 @@ def save_permanent_allowlist(patterns: set): plus ``what this process approved since its own baseline``; revoked entries are also dropped from the governing permanent set so ``is_approved()`` stops honouring them. Nothing re-reads the file on the approval hot path. + + ``patterns`` may only ADD: an entry left out of it is not removed, because the + on-disk list wins for anything this process did not approve itself. Remove + entries by editing ``command_allowlist`` in config.yaml. """ try: from hermes_cli.config import load_config, save_config diff --git a/website/docs/user-guide/security.md b/website/docs/user-guide/security.md index f809dcb09d..a3927fba12 100644 --- a/website/docs/user-guide/security.md +++ b/website/docs/user-guide/security.md @@ -275,6 +275,13 @@ your configuration file. Use `hermes config edit` to review or remove patterns from your permanent allowlist. ::: +:::caution +The list is read when Hermes starts. A pattern you remove while a session is +already running stays approved in that session until it next writes the file +(the next time you answer `always` to a prompt) or you restart Hermes. If you +removed it for safety reasons, restart. +::: + ### Mining Approval History (`hermes approvals suggest`) Instead of answering the same prompt session after session, you can mine your