diff --git a/docs/sanctioned-restart-controls.md b/docs/sanctioned-restart-controls.md new file mode 100644 index 0000000..8801695 --- /dev/null +++ b/docs/sanctioned-restart-controls.md @@ -0,0 +1,122 @@ +# Sanctioned restart and graceful reload controls (#642) + +Sessions used to recover MCP connectivity by killing the host daemon +(`pkill -f mcp_server.py`, #630). That is forbidden and stays forbidden: it +kills every namespace on the host, contaminates whichever session survives, and +leaves no audit trail. This document describes the sanctioned replacement, +implemented in `webui/sanctioned_restart.py`. + +## What the console will and will not do + +The console **never** restarts anything. It authorizes an intent, records it, +and hands off to a host supervisor. There is no code path in which the console +sends a signal, spawns a process, or renders a kill command — a regression test +asserts the module contains no `subprocess`, `signal`, `os.kill`, `os.system`, +or `popen` reference, and that no returned payload contains a kill command. + +## Operations + +| Mode | Action | Minimum role | Behaviour | +|------|--------|--------------|-----------| +| `reload` | `system.reload_namespace` | controller | Host supervisor reloads the namespace in place, draining in-flight requests. | +| `restart` | `system.restart_namespace` | admin | Host supervisor restarts the namespace. In-flight requests are lost. | + +Scope is always exactly one namespace. A fleet-wide restart is an explicit +non-goal: `all`, `*`, `fleet`, and an empty scope are refused with +`fleet_scope_not_permitted`, because that is precisely the blast radius the +forbidden kill already had. An unrecognised namespace is refused rather than +passed through to the host. + +## The gate sequence + +`assess_restart_request()` applies every gate in order and reports the first +failure with a stable reason code: + +| Order | Gate | Reason code on failure | +|-------|------|------------------------| +| 1 | Mode is `restart` or `reload` | `unknown_mode` | +| 2 | Scope is a single known namespace | `fleet_scope_not_permitted`, `unknown_namespace` | +| 3 | Principal holds the required console role | `unauthorized` | +| 4 | Confirmation phrase supplied | `confirmation_required` | +| 5 | Confirmation names this namespace and mode | `confirmation_mismatch` | +| 6 | Out-of-band operator authorization present | `operator_authorization_missing` | +| 7 | Runtime is not contaminated | `contaminated_runtime` | +| 8 | Host restart hook configured | `restart_hook_not_configured` | + +Passing every gate yields `host_action_required`, never "restarted". + +### Confirmation binds the namespace + +The required phrase is `" "` — for example +`restart gitea-author`. Binding the namespace into the phrase is the point: a +confirmation typed for one namespace cannot be replayed against another. + +### Operator authorization is not self-assertable + +Host daemon maintenance is authorized out of band through +`GITEA_OPERATOR_DAEMON_MAINTENANCE_AUTHORIZATION`, read from the process +environment and nowhere else (#630; #710 finding F1). A worker session cannot +set an environment variable for an already-running daemon, so this cannot be +faked the way a tool argument could. + +### The host hook + +`GITEA_SANCTIONED_RESTART_HOOK` holds an opaque reference the *host* resolves — +a supervisor label such as a launchd job name, never a command line. With no +hook configured the request is refused; the console does not fall back to a +process kill. The value is read server-side and never rendered to a client. + +## Manual kill remains contamination + +`classify_restart_command()` classifies an operator-proposed recovery command. +A manual `pkill`/`kill`/`killall` of the MCP daemon is contamination, not a +restart: it returns `clean_claim_allowed: false` and builds a durable +contamination marker (redacted command only, never secrets) naming +`system.restart_namespace` as the sanctioned alternative. + +A live, uncleared contamination marker also blocks a restart. This is stricter +than #630's task-scoped gate, which deliberately lets a contaminated worker keep +commenting and handing off: restarting a contaminated runtime would launder the +contamination rather than resolve it. Clear the marker through the reconciler +path first. + +## Post-restart health verification + +After the host supervisor acts, `verify_post_restart_health()` decides whether +the session may claim to be clean: + +| Status | Meaning | Clean claim | +|--------|---------|-------------| +| `clean` | Required tool callable, proven through the live client namespace | Allowed | +| `unproven` | Reported healthy without live client-namespace evidence | Refused | +| `unhealthy` | Probe failed | Refused | + +Only `probe_source=client_namespace` evidence clears a session. Static tool +registration is not proof, and neither is an offline subprocess probe — an IDE +client can hold a registered tool list while live calls fail with +`client is closing: EOF` (see +[`mcp-namespace-health.md`](mcp-namespace-health.md)). + +## Audit + +Every attempt — allowed or denied — is recorded through +`webui.console_audit` with actor, target namespace, mode, result, and reason +code, and is redacted before it is persisted. `system.restart_namespace` is +break-glass, so its records are retained for 730 days. Records carry +`process_kill_executed: false`, which is a fact about the code path rather than +a claim: no such path exists. + +## Environment variables + +| Variable | Purpose | +|----------|---------| +| `GITEA_SANCTIONED_RESTART_HOOK` | Host supervisor reference; absent means restart is refused. | +| `GITEA_OPERATOR_DAEMON_MAINTENANCE_AUTHORIZATION` | Out-of-band operator authorization reference. | +| `WEBUI_AUDIT_LOG` | Console audit sink; absent means records are built but not persisted. | + +## Non-goals + +* No unrestricted `kill` from the UI, in any role, in any phase. +* No fleet-wide restart. +* No silent auto-restart loop: every attempt is confirmed and audited. +* This does not implement the Phase 1 health API (#634). diff --git a/docs/webui-authz-audit.md b/docs/webui-authz-audit.md index 8776429..9e010e9 100644 --- a/docs/webui-authz-audit.md +++ b/docs/webui-authz-audit.md @@ -91,6 +91,8 @@ already define, and a regression test asserts each mapping matches. | `close_pr` | controller | privileged | `gitea.pr.close` | Yes | No | No | 3 | | `merge_pr` | controller | privileged | `gitea.pr.merge` | Yes | **Yes** | **Yes** | 3 | | `delete_branch` | admin | destructive | `gitea.branch.delete` | Yes | **Yes** | **Yes** | 3 | +| `system.reload_namespace` | controller | privileged | `runtime.reload_namespace` | Yes | No | No | 2 | +| `system.restart_namespace` | admin | destructive | `runtime.restart_namespace` | Yes | **Yes** | **Yes** | 2 | **Dual control** means the acting principal may not be the sole authority: a second distinct principal must confirm. **Break-glass** means the action is @@ -102,6 +104,13 @@ honouring it. `delete_branch` is admin-only rather than controller because it is the one irreversible action in the set. +`system.restart_namespace` is admin-only for the same reason: restarting a +namespace drops every in-flight request on it. `system.reload_namespace` drains +first, so it is privileged but not destructive. Neither action is ever executed +by the console — both hand off to a host supervisor, and neither exposes a raw +process kill. See +[`sanctioned-restart-controls.md`](sanctioned-restart-controls.md) (#642). + ### Authorization decision `authorize(action_id, principal, for_execution=False)` returns a decision @@ -199,8 +208,8 @@ breaking the request it describes. | Class | Applies to | Default | |-------|-----------|---------| | `standard` | Routine gated writes | 90 days | -| `privileged` | `review_pr`, `close_pr`, and any unclassifiable action | 365 days | -| `break_glass` | `merge_pr`, `delete_branch` | 730 days | +| `privileged` | `review_pr`, `close_pr`, `system.reload_namespace`, and any unclassifiable action | 365 days | +| `break_glass` | `merge_pr`, `delete_branch`, `system.restart_namespace` | 730 days | Each record carries its own class, day count, and computed `expires_at`, so retention is auditable per record rather than inferred from file age. An diff --git a/task_capability_map.py b/task_capability_map.py index 0b8ac0b..c7a54e5 100644 --- a/task_capability_map.py +++ b/task_capability_map.py @@ -335,6 +335,21 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "role": "controller", }, + # #642: sanctioned host-daemon lifecycle controls. Deliberately *not* a + # ``gitea.*`` operation — restarting an MCP namespace is a host action, not + # a Gitea API call, and no configured Gitea profile should be able to + # satisfy it by accident. Authority comes from the console RBAC model plus + # out-of-band operator authorization (#630); these entries exist so the + # console cannot invent an authority the capability layer never declared. + "restart_namespace": { + "permission": "runtime.restart_namespace", + "role": "controller", + }, + "reload_namespace": { + "permission": "runtime.reload_namespace", + "role": "controller", + }, + # #601 first-class lease lifecycle — inspect/list need read; mutations gate on # ownership in the control-plane DB (not a separate Gitea write permission). "list_workflow_leases": { diff --git a/tests/test_webui_sanctioned_restart.py b/tests/test_webui_sanctioned_restart.py new file mode 100644 index 0000000..a495935 --- /dev/null +++ b/tests/test_webui_sanctioned_restart.py @@ -0,0 +1,502 @@ +"""Sanctioned restart / graceful reload control tests (#642). + +Acceptance criteria under test: + +1. The sanctioned restart path is implemented behind gates (capability, + confirmation, operator authorization, host hook). +2. Manual ``pkill`` stays forbidden and is classified as contamination. +3. Post-restart mutations require clean health/session proof. +4. Authorized restart preview, unauthorized deny, contamination classification. +5. No entry point exposes a raw kill. +""" + +import json +import os +import tempfile +import unittest + +import mcp_namespace_health +import runtime_recovery_guard +from task_capability_map import TASK_CAPABILITY_MAP +from webui import console_audit, console_authz, gated_actions, sanctioned_restart + +NAMESPACE = "gitea-author" + +# An operator-authorized, hook-configured host. Passed explicitly so no test +# depends on (or mutates) the real process environment. +READY_ENV = { + sanctioned_restart.RESTART_HOOK_ENV: "launchd:cc.prgs.gitea-author", + runtime_recovery_guard.OPERATOR_AUTHORIZATION_ENV: "ops-ticket-4821", +} + + +def admin(subject: str = "admin@example.test") -> console_authz.Principal: + return console_authz.Principal( + subject=subject, + role=console_authz.ADMIN, + identity_source=console_authz.IDENTITY_ACCESS_PROXY, + authenticated=True, + ) + + +def viewer() -> console_authz.Principal: + return console_authz.Principal( + subject="viewer@example.test", + role=console_authz.VIEWER, + identity_source=console_authz.IDENTITY_ACCESS_PROXY, + authenticated=True, + ) + + +class TestCapabilityWiring(unittest.TestCase): + """AC1: authority is declared, not invented by the console.""" + + def test_actions_resolve_through_the_capability_map(self): + for action_id in ( + sanctioned_restart.ACTION_RESTART_NAMESPACE, + sanctioned_restart.ACTION_RELOAD_NAMESPACE, + ): + with self.subTest(action=action_id): + action = console_authz.get_action(action_id) + self.assertIsNotNone(action) + self.assertIn(action.task_key, TASK_CAPABILITY_MAP) + self.assertEqual( + action.mcp_permission, + TASK_CAPABILITY_MAP[action.task_key]["permission"], + ) + + def test_restart_permission_is_not_a_gitea_operation(self): + """No configured Gitea profile should satisfy a host restart.""" + permission = TASK_CAPABILITY_MAP["restart_namespace"]["permission"] + self.assertFalse(permission.startswith("gitea.")) + + def test_restart_is_destructive_dual_control_break_glass(self): + action = console_authz.get_action( + sanctioned_restart.ACTION_RESTART_NAMESPACE + ) + self.assertEqual(action.action_class, console_authz.CLASS_DESTRUCTIVE) + self.assertEqual(action.minimum_role, console_authz.ADMIN) + self.assertTrue(action.dual_control) + self.assertTrue(action.break_glass) + self.assertTrue(action.requires_confirmation) + + def test_reload_is_privileged_but_not_destructive(self): + action = console_authz.get_action( + sanctioned_restart.ACTION_RELOAD_NAMESPACE + ) + self.assertEqual(action.action_class, console_authz.CLASS_PRIVILEGED) + self.assertTrue(action.requires_confirmation) + + +class TestPreview(unittest.TestCase): + """AC4: an authorized preview renders the plan without executing it.""" + + def test_preview_lists_the_mutation_ledger(self): + preview = sanctioned_restart.build_restart_preview( + NAMESPACE, principal=admin(), env=READY_ENV + ) + steps = [entry["step"] for entry in preview["mutation_ledger"]] + self.assertEqual( + steps, ["quiesce", "host_restart_hook", "health_recheck", "audit"] + ) + self.assertTrue(preview["scope_valid"]) + self.assertTrue(preview["post_restart_verification_required"]) + + def test_reload_preview_drains_instead_of_restarting(self): + preview = sanctioned_restart.build_restart_preview( + NAMESPACE, sanctioned_restart.MODE_RELOAD, + principal=admin(), env=READY_ENV, + ) + steps = [entry["step"] for entry in preview["mutation_ledger"]] + self.assertIn("host_graceful_reload", steps) + self.assertNotIn("host_restart_hook", steps) + + def test_preview_never_enables_execution(self): + preview = sanctioned_restart.build_restart_preview( + NAMESPACE, principal=admin(), env=READY_ENV + ) + self.assertFalse(preview["execution_enabled"]) + self.assertFalse(preview["authorization"]["execution_enabled"]) + + def test_confirmation_phrase_binds_the_namespace(self): + self.assertTrue( + sanctioned_restart.confirmation_matches( + NAMESPACE, sanctioned_restart.MODE_RESTART, + "restart gitea-author", + ) + ) + # A phrase typed for one namespace must not authorize another. + self.assertFalse( + sanctioned_restart.confirmation_matches( + "gitea-merger", sanctioned_restart.MODE_RESTART, + "restart gitea-author", + ) + ) + + +class TestGates(unittest.TestCase): + """AC1/AC4: every gate denies with a stable reason code.""" + + def _assess(self, **kwargs): + params = { + "principal": admin(), + "confirmation": f"restart {NAMESPACE}", + "env": READY_ENV, + } + params.update(kwargs) + namespace = params.pop("namespace", NAMESPACE) + mode = params.pop("mode", sanctioned_restart.MODE_RESTART) + return sanctioned_restart.assess_restart_request( + namespace, mode, **params + ) + + def test_authorized_confirmed_request_passes_every_gate(self): + result = self._assess() + self.assertTrue(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.ALLOW_HOST_ACTION_REQUIRED + ) + + def test_passing_every_gate_is_not_an_execution_grant(self): + """An allowed request still never lets the console touch the process.""" + result = self._assess() + self.assertTrue(result["allowed"]) + self.assertFalse(result["execution_enabled"]) + self.assertFalse(result["console_executes"]) + + def test_unauthorized_principal_is_denied(self): + result = self._assess(principal=viewer()) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_UNAUTHORIZED + ) + + def test_anonymous_principal_is_denied(self): + result = self._assess(principal=None) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_UNAUTHORIZED + ) + + def test_missing_confirmation_is_denied(self): + result = self._assess(confirmation=None) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_CONFIRMATION_MISSING + ) + + def test_confirmation_for_another_namespace_is_denied(self): + result = self._assess(confirmation="restart gitea-merger") + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_CONFIRMATION_MISMATCH + ) + + def test_missing_operator_authorization_is_denied(self): + env = {sanctioned_restart.RESTART_HOOK_ENV: "launchd:cc.prgs.author"} + result = self._assess(env=env) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], + sanctioned_restart.DENY_OPERATOR_AUTHORIZATION, + ) + + def test_missing_host_hook_is_denied_without_kill_fallback(self): + env = { + runtime_recovery_guard.OPERATOR_AUTHORIZATION_ENV: "ops-ticket-1", + } + result = self._assess(env=env) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_HOOK_NOT_CONFIGURED + ) + + def test_fleet_scope_is_refused(self): + for scope in ("all", "*", "fleet"): + with self.subTest(scope=scope): + result = self._assess( + namespace=scope, confirmation=f"restart {scope}" + ) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_FLEET_SCOPE + ) + + def test_unknown_namespace_is_refused(self): + result = self._assess( + namespace="gitea-nope", confirmation="restart gitea-nope" + ) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_UNKNOWN_NAMESPACE + ) + + def test_unknown_mode_is_refused(self): + result = self._assess(mode="obliterate") + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_UNKNOWN_MODE + ) + + def test_live_contamination_marker_blocks_restart(self): + marker = runtime_recovery_guard.build_contamination_record( + reason_class=runtime_recovery_guard.REASON_MANUAL_DAEMON_KILL, + command_redacted="pkill -f mcp_server.py", + ) + result = self._assess(contamination_marker=marker) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["reason_code"], sanctioned_restart.DENY_CONTAMINATED_RUNTIME + ) + + def test_reconciler_cleared_marker_no_longer_blocks(self): + marker = runtime_recovery_guard.build_contamination_record( + reason_class=runtime_recovery_guard.REASON_MANUAL_DAEMON_KILL, + command_redacted="pkill -f mcp_server.py", + ) + marker = dict(marker, cleared_by_reconciler=True) + result = self._assess(contamination_marker=marker) + self.assertTrue(result["allowed"]) + + +class TestExecutionNeverKills(unittest.TestCase): + """AC5: no path exposes or runs a raw process kill.""" + + def test_authorized_execution_defers_to_the_host_supervisor(self): + result = sanctioned_restart.execute_restart( + NAMESPACE, + principal=admin(), + confirmation=f"restart {NAMESPACE}", + env=READY_ENV, + ) + self.assertTrue(result["allowed"]) + self.assertFalse(result["success"]) + self.assertFalse(result["process_kill_executed"]) + self.assertEqual( + result["outcome"], sanctioned_restart.ALLOW_HOST_ACTION_REQUIRED + ) + + def test_denied_execution_reports_the_refusing_gate(self): + result = sanctioned_restart.execute_restart( + NAMESPACE, principal=viewer(), confirmation=f"restart {NAMESPACE}", + env=READY_ENV, + ) + self.assertFalse(result["allowed"]) + self.assertEqual( + result["outcome"], sanctioned_restart.DENY_UNAUTHORIZED + ) + self.assertFalse(result["process_kill_executed"]) + + def test_module_never_spawns_a_process(self): + path = os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + "webui", "sanctioned_restart.py", + ) + with open(path, encoding="utf-8") as handle: + source = handle.read() + for forbidden in ( + "import subprocess", "import signal", "os.kill", "os.system", + "popen", + ): + with self.subTest(forbidden=forbidden): + self.assertNotIn(forbidden, source.lower()) + + def test_no_surface_returns_a_kill_command(self): + payloads = [ + sanctioned_restart.build_restart_preview( + NAMESPACE, principal=admin(), env=READY_ENV + ), + sanctioned_restart.restart_policy(), + sanctioned_restart.execute_restart( + NAMESPACE, principal=admin(), + confirmation=f"restart {NAMESPACE}", env=READY_ENV, + ), + ] + for payload in payloads: + rendered = json.dumps(payload, default=str).lower() + self.assertNotIn("kill -9", rendered) + self.assertNotIn("pkill -f", rendered) + + def test_policy_declares_no_raw_kill_and_no_silent_restart(self): + policy = sanctioned_restart.restart_policy() + self.assertFalse(policy["raw_kill_exposed"]) + self.assertFalse(policy["console_executes_process_kill"]) + self.assertFalse(policy["fleet_scope_permitted"]) + self.assertFalse(policy["silent_auto_restart_permitted"]) + self.assertTrue(policy["audit_required"]) + + +class TestContaminationClassification(unittest.TestCase): + """AC2: manual pkill is contamination, and it blocks clean claims.""" + + def test_manual_daemon_pkill_is_contamination(self): + result = sanctioned_restart.classify_restart_command( + "pkill -f mcp_server.py" + ) + self.assertTrue(result["contamination"]) + self.assertFalse(result["clean_claim_allowed"]) + self.assertIsNotNone(result["contamination_marker"]) + self.assertEqual( + result["sanctioned_alternative"], + sanctioned_restart.ACTION_RESTART_NAMESPACE, + ) + + def test_broad_process_kill_is_contamination(self): + result = sanctioned_restart.classify_restart_command("killall -9 Python") + self.assertTrue(result["contamination"]) + self.assertFalse(result["clean_claim_allowed"]) + + def test_marker_names_the_sanctioned_alternative(self): + result = sanctioned_restart.classify_restart_command( + "pkill -f mcp_server.py" + ) + marker = result["contamination_marker"] + self.assertIn( + sanctioned_restart.ACTION_RESTART_NAMESPACE, marker["detail"] + ) + + def test_benign_command_is_not_contamination(self): + result = sanctioned_restart.classify_restart_command("git status") + self.assertFalse(result["contamination"]) + self.assertTrue(result["clean_claim_allowed"]) + + def test_no_command_is_not_contamination(self): + result = sanctioned_restart.classify_restart_command(None) + self.assertFalse(result["contamination"]) + self.assertTrue(result["clean_claim_allowed"]) + + +class TestPostRestartHealth(unittest.TestCase): + """AC3: a clean post-restart claim needs live client-namespace proof.""" + + def test_live_client_probe_clears_the_session(self): + result = sanctioned_restart.verify_post_restart_health( + NAMESPACE, + probe_result={"success": True}, + probe_source=mcp_namespace_health.PROBE_SOURCE_CLIENT, + registered_tools=["gitea_whoami"], + required_tool="gitea_whoami", + ) + self.assertEqual(result["status"], sanctioned_restart.HEALTH_CLEAN) + self.assertTrue(result["clean_claim_allowed"]) + self.assertTrue(result["mutations_allowed"]) + + def test_offline_probe_does_not_clear_the_session(self): + result = sanctioned_restart.verify_post_restart_health( + NAMESPACE, + probe_result={"success": True}, + probe_source=mcp_namespace_health.PROBE_SOURCE_OFFLINE, + registered_tools=["gitea_whoami"], + required_tool="gitea_whoami", + ) + self.assertFalse(result["clean_claim_allowed"]) + self.assertFalse(result["mutations_allowed"]) + + def test_failed_probe_is_unhealthy(self): + result = sanctioned_restart.verify_post_restart_health( + NAMESPACE, + probe_result={"success": False, "error": "client is closing: EOF"}, + probe_source=mcp_namespace_health.PROBE_SOURCE_CLIENT, + registered_tools=["gitea_whoami"], + required_tool="gitea_whoami", + ) + self.assertEqual(result["status"], sanctioned_restart.HEALTH_UNHEALTHY) + self.assertFalse(result["clean_claim_allowed"]) + + def test_static_registration_alone_never_clears_the_session(self): + result = sanctioned_restart.verify_post_restart_health( + NAMESPACE, + registered_tools=["gitea_whoami"], + required_tool="gitea_whoami", + ) + self.assertFalse(result["clean_claim_allowed"]) + + +class TestAuditEmission(unittest.TestCase): + """Every restart attempt is audited with actor, target, and result.""" + + def _run(self, principal, sink): + prior = os.environ.get(console_audit.AUDIT_LOG_ENV) + os.environ[console_audit.AUDIT_LOG_ENV] = sink + try: + return sanctioned_restart.execute_restart( + NAMESPACE, + principal=principal, + confirmation=f"restart {NAMESPACE}", + env=READY_ENV, + request_id="req-642", + ) + finally: + if prior is None: + os.environ.pop(console_audit.AUDIT_LOG_ENV, None) + else: + os.environ[console_audit.AUDIT_LOG_ENV] = prior + + def test_allowed_attempt_is_written_with_actor_and_target(self): + with tempfile.TemporaryDirectory() as tmp: + sink = os.path.join(tmp, "audit.jsonl") + result = self._run(admin(), sink) + self.assertTrue(result["audit"]["written"]) + with open(sink, encoding="utf-8") as handle: + record = json.loads(handle.read().strip()) + self.assertEqual( + record["action"], sanctioned_restart.ACTION_RESTART_NAMESPACE + ) + self.assertEqual(record["target"]["namespace"], NAMESPACE) + self.assertEqual(record["target"]["mode"], "restart") + self.assertEqual(record["result"], console_audit.RESULT_ALLOWED) + self.assertEqual(record["actor"]["subject"], "admin@example.test") + self.assertFalse(record["metadata"]["process_kill_executed"]) + + def test_denied_attempt_is_audited_too(self): + with tempfile.TemporaryDirectory() as tmp: + sink = os.path.join(tmp, "audit.jsonl") + self._run(viewer(), sink) + with open(sink, encoding="utf-8") as handle: + record = json.loads(handle.read().strip()) + self.assertEqual(record["result"], console_audit.RESULT_DENIED) + self.assertEqual( + record["reason_code"], sanctioned_restart.DENY_UNAUTHORIZED + ) + + def test_restart_audit_uses_break_glass_retention(self): + action = console_authz.get_action( + sanctioned_restart.ACTION_RESTART_NAMESPACE + ) + self.assertEqual( + console_audit.retention_class_for(action), + console_audit.RETENTION_BREAK_GLASS, + ) + + +class TestRegistrySurface(unittest.TestCase): + """AC5: the console surfaces the control, still disabled, with no kill.""" + + def test_registry_exposes_both_actions_disabled(self): + registry = gated_actions.load_action_registry() + for action_id in ( + sanctioned_restart.ACTION_RESTART_NAMESPACE, + sanctioned_restart.ACTION_RELOAD_NAMESPACE, + ): + with self.subTest(action=action_id): + action = registry.get(action_id) + self.assertIsNotNone(action) + self.assertFalse(action.enabled) + + def test_registry_preview_names_the_namespace_target(self): + preview = gated_actions.preview_action( + sanctioned_restart.ACTION_RESTART_NAMESPACE, namespace=NAMESPACE + ) + target = preview["mutation_ledger"][0]["target"] + self.assertIn(NAMESPACE, target) + self.assertFalse(preview["enabled"]) + + def test_registry_attempt_fails_closed(self): + result = gated_actions.attempt_action( + sanctioned_restart.ACTION_RESTART_NAMESPACE, namespace=NAMESPACE + ) + self.assertFalse(result["success"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/webui/console_authz.py b/webui/console_authz.py index 2e64b2c..06efe86 100644 --- a/webui/console_authz.py +++ b/webui/console_authz.py @@ -236,6 +236,34 @@ _ACTION_SPECS: tuple[ConsoleAction, ...] = ( phase=3, summary="Remove a remote feature branch.", ), + # #642: sanctioned daemon lifecycle. These exist so operators have an + # audited path off `pkill -f mcp_server.py` (#630). Restart drops every + # in-flight request on a namespace, so it carries the same dual-control and + # break-glass weight as a merge; reload drains first and is privileged but + # not destructive. Neither ever exposes a raw kill: execution is handed to + # a host supervisor by ``webui.sanctioned_restart``. + ConsoleAction( + action_id="system.reload_namespace", + task_key="reload_namespace", + action_class=CLASS_PRIVILEGED, + minimum_role=CONTROLLER, + requires_confirmation=True, + dual_control=False, + break_glass=False, + phase=2, + summary="Gracefully reload one MCP namespace via the host supervisor.", + ), + ConsoleAction( + action_id="system.restart_namespace", + task_key="restart_namespace", + action_class=CLASS_DESTRUCTIVE, + minimum_role=ADMIN, + requires_confirmation=True, + dual_control=True, + break_glass=True, + phase=2, + summary="Restart one MCP namespace via the host supervisor.", + ), ) ACTIONS: dict[str, ConsoleAction] = {a.action_id: a for a in _ACTION_SPECS} diff --git a/webui/gated_actions.py b/webui/gated_actions.py index baefab8..d914874 100644 --- a/webui/gated_actions.py +++ b/webui/gated_actions.py @@ -110,6 +110,8 @@ def _format_target(action_id: str, params: dict[str, Any]) -> str: ) if action_id == "create_issue": return f"issue {params.get('title', '?')!r}" + if action_id in {"system.restart_namespace", "system.reload_namespace"}: + return f"MCP namespace {params.get('namespace', '?')!r}" return "unspecified" @@ -165,6 +167,17 @@ def build_action_registry() -> ActionRegistry: "gitea_create_issue_comment", "Post a PR review thread comment."), ("close_pr", "Close PR", "close_pr", "gitea_edit_pr", "Close a pull request without merge."), + # #642: the sanctioned replacement for the forbidden manual daemon-kill + # recovery path (#630). The "tool" is a host supervisor hook, not an MCP + # call — the console never signals a process. Preview and gating live in + # ``webui.sanctioned_restart``; these stay disabled like every other + # registry entry. + ("system.reload_namespace", "Reload MCP namespace", "reload_namespace", + "host.supervisor_reload", + "Gracefully reload one MCP namespace via the host supervisor."), + ("system.restart_namespace", "Restart MCP namespace", + "restart_namespace", "host.supervisor_restart", + "Restart one MCP namespace via the host supervisor."), ) actions = tuple( GatedAction( diff --git a/webui/sanctioned_restart.py b/webui/sanctioned_restart.py new file mode 100644 index 0000000..f413829 --- /dev/null +++ b/webui/sanctioned_restart.py @@ -0,0 +1,613 @@ +"""Sanctioned MCP restart and graceful reload controls (#642, Phase 2). + +Sessions have historically recovered MCP connectivity by killing the host +daemon (``pkill -f mcp_server.py``, #630). That path stays forbidden: it kills +every namespace on the host, contaminates the surviving session, and leaves no +audit trail. This module is the sanctioned replacement. + +A restart is modelled as a *gated action*, never as a command: + +1. **Capability** — the console action resolves through ``console_authz`` + against ``task_capability_map``, so the console cannot invent an authority + the MCP layer does not already define. +2. **Preview** — :func:`build_restart_preview` renders a mutation ledger and + the exact confirmation phrase. It never returns a shell command. +3. **Confirmation** — the operator echoes a phrase naming the exact namespace + and mode. A phrase for one namespace never authorizes another. +4. **Operator authorization** — host daemon maintenance is authorized out of + band through the environment (#630, and #710 finding F1: a worker session + cannot set an env var for an already-running daemon, so this cannot be + self-asserted the way a tool argument could). +5. **Execution** — :func:`execute_restart` never spawns a process. Once every + gate passes it hands the request to the configured host-managed restart + hook; with no hook configured it fails closed. +6. **Health recheck** — :func:`verify_post_restart_health` requires live + client-namespace probe evidence before any post-restart clean claim. + +Manual ``pkill`` remains forbidden and is classified as contamination by +:func:`classify_restart_command`, which blocks clean claims (#630 AC3). + +This module performs no I/O beyond reading its own environment configuration, +imports no MCP client, and holds no credential. +""" + +from __future__ import annotations + +import os +from dataclasses import asdict, dataclass +from typing import Any + +import mcp_namespace_health +import runtime_recovery_guard +from webui import console_audit, console_authz + +# --- Operations ------------------------------------------------------------- + +MODE_RESTART = "restart" +MODE_RELOAD = "reload" +MODES: tuple[str, ...] = (MODE_RESTART, MODE_RELOAD) + +ACTION_RESTART_NAMESPACE = "system.restart_namespace" +ACTION_RELOAD_NAMESPACE = "system.reload_namespace" + +ACTION_FOR_MODE: dict[str, str] = { + MODE_RESTART: ACTION_RESTART_NAMESPACE, + MODE_RELOAD: ACTION_RELOAD_NAMESPACE, +} + +# Namespaces the console may target. An unlisted name fails closed rather than +# being passed through to a host hook. +KNOWN_NAMESPACES: tuple[str, ...] = tuple( + sorted( + set(mcp_namespace_health.DEFAULT_NAMESPACES) + | {"gitea-author", "gitea-reviewer", "gitea-merger", + "gitea-reconciler", "gitea-controller"} + ) +) + +# Scope tokens that would mean "everything at once". Explicit non-goal: the +# console never offers a fleet-wide restart, because that is the blast radius +# `pkill -f mcp_server.py` already had. +_FLEET_TOKENS = frozenset({"*", "all", "fleet", "any", ""}) + +# --- Environment configuration ---------------------------------------------- +# Read server-side only; the value is an opaque host hook reference (e.g. a +# launchd label), never a command line, and is never rendered to a client. +RESTART_HOOK_ENV = "GITEA_SANCTIONED_RESTART_HOOK" + +# --- Reason codes ----------------------------------------------------------- + +DENY_UNKNOWN_MODE = "unknown_mode" +DENY_UNKNOWN_NAMESPACE = "unknown_namespace" +DENY_FLEET_SCOPE = "fleet_scope_not_permitted" +DENY_UNAUTHORIZED = "unauthorized" +DENY_CONFIRMATION_MISSING = "confirmation_required" +DENY_CONFIRMATION_MISMATCH = "confirmation_mismatch" +DENY_OPERATOR_AUTHORIZATION = "operator_authorization_missing" +DENY_HOOK_NOT_CONFIGURED = "restart_hook_not_configured" +DENY_CONTAMINATED_RUNTIME = "contaminated_runtime" + +ALLOW_HOST_ACTION_REQUIRED = "host_action_required" + +# Post-restart verification outcomes. +HEALTH_CLEAN = "clean" +HEALTH_UNPROVEN = "unproven" +HEALTH_UNHEALTHY = "unhealthy" + + +def _clean(value: Any) -> str: + return str(value or "").strip() + + +# --- Mutation ledger -------------------------------------------------------- + + +@dataclass(frozen=True) +class RestartLedgerEntry: + """One planned step, shown before anything is asked of the host.""" + + sequence: int + step: str + summary: str + executes_process_kill: bool = False + + +def _mutation_ledger(namespace: str, mode: str) -> tuple[RestartLedgerEntry, ...]: + if mode == MODE_RELOAD: + middle = RestartLedgerEntry( + sequence=2, + step="host_graceful_reload", + summary=( + f"Ask the configured host supervisor to reload {namespace} " + "in place, draining in-flight requests. The console does not " + "signal the process itself." + ), + ) + else: + middle = RestartLedgerEntry( + sequence=2, + step="host_restart_hook", + summary=( + f"Ask the configured host supervisor to restart {namespace}. " + "The console never sends a signal and never runs a kill." + ), + ) + return ( + RestartLedgerEntry( + sequence=1, + step="quiesce", + summary=( + f"Stop admitting new gated mutations for {namespace} and " + "record the intent before anything restarts." + ), + ), + middle, + RestartLedgerEntry( + sequence=3, + step="health_recheck", + summary=( + f"Re-probe {namespace} through the live client namespace and " + "prove the required tool is callable again." + ), + ), + RestartLedgerEntry( + sequence=4, + step="audit", + summary=( + "Append actor, target namespace, mode, and result to the " + "console audit log." + ), + ), + ) + + +# --- Confirmation ----------------------------------------------------------- + + +def confirmation_phrase(namespace: str, mode: str) -> str: + """Exact phrase an operator must echo, naming the namespace and mode. + + Binding the namespace into the phrase is the point: a confirmation typed + for ``gitea-author`` cannot be replayed against ``gitea-merger``. + """ + return f"{_clean(mode)} {_clean(namespace)}" + + +def confirmation_matches( + namespace: str, mode: str, confirmation: str | None +) -> bool: + """Compare *confirmation* to the required phrase (exact, whitespace-trimmed).""" + return _clean(confirmation) == confirmation_phrase(namespace, mode) + + +# --- Scope validation ------------------------------------------------------- + + +def _validate_scope(namespace: str, mode: str) -> tuple[str, str] | None: + """Return ``(reason_code, detail)`` when the scope is refused.""" + ns = _clean(namespace) + md = _clean(mode) + + if md not in MODES: + return ( + DENY_UNKNOWN_MODE, + f"Mode {md!r} is not one of {', '.join(MODES)}.", + ) + if ns.lower() in _FLEET_TOKENS: + return ( + DENY_FLEET_SCOPE, + ( + "Fleet-wide restart is an explicit non-goal: it reproduces the " + "blast radius of `pkill -f mcp_server.py` (#630). Restart one " + "namespace at a time." + ), + ) + if ns not in KNOWN_NAMESPACES: + return ( + DENY_UNKNOWN_NAMESPACE, + f"Namespace {ns!r} is not a known MCP namespace.", + ) + return None + + +# --- Host hook -------------------------------------------------------------- + + +def restart_hook(env: dict[str, str] | None = None) -> dict[str, Any]: + """Report the configured host-managed restart hook. + + The hook is a reference the *host* resolves (a supervisor label), not a + command this process runs. ``configured=False`` fails restart closed. + """ + source = env if env is not None else os.environ + reference = _clean(source.get(RESTART_HOOK_ENV)) + return { + "configured": bool(reference), + "reference": reference or None, + "source": RESTART_HOOK_ENV if reference else None, + "self_assertable": False, + "console_executes_process": False, + } + + +# --- Preview ---------------------------------------------------------------- + + +def build_restart_preview( + namespace: str, + mode: str = MODE_RESTART, + *, + principal: console_authz.Principal | None = None, + env: dict[str, str] | None = None, +) -> dict[str, Any]: + """Render the dry-run preview for a restart/reload request. + + Read-only: no authorization is granted, no host is contacted, and the + result never contains a shell command. + """ + ns = _clean(namespace) + md = _clean(mode) + action_id = ACTION_FOR_MODE.get(md, ACTION_RESTART_NAMESPACE) + action = console_authz.get_action(action_id) + decision = console_authz.authorize(action_id, principal) + scope_error = _validate_scope(ns, md) + hook = restart_hook(env) + operator = runtime_recovery_guard.operator_authorization(env) + + return { + "action_id": action_id, + "namespace": ns, + "mode": md, + "scope_valid": scope_error is None, + "scope_reason_code": scope_error[0] if scope_error else None, + "scope_detail": scope_error[1] if scope_error else None, + "required_role": action.minimum_role if action else None, + "required_permission": action.mcp_permission if action else None, + "action_class": action.action_class if action else None, + "dual_control": action.dual_control if action else True, + "break_glass": action.break_glass if action else True, + "requires_confirmation": True, + "confirmation_phrase": confirmation_phrase(ns, md), + "mutation_ledger": [asdict(entry) for entry in _mutation_ledger(ns, md)], + "authorization": decision.to_dict(), + "operator_authorization": operator, + "restart_hook": hook, + "execution_enabled": False, + "raw_process_kill_exposed": False, + "known_namespaces": list(KNOWN_NAMESPACES), + "post_restart_verification_required": True, + } + + +# --- Gate ------------------------------------------------------------------- + + +def assess_restart_request( + namespace: str, + mode: str = MODE_RESTART, + *, + principal: console_authz.Principal | None = None, + confirmation: str | None = None, + contamination_marker: dict[str, Any] | None = None, + env: dict[str, str] | None = None, +) -> dict[str, Any]: + """Decide whether a restart request may proceed to the host hook. + + Every gate must pass. The first failure wins and is reported with a stable + reason code; a pass never means "restarted", only "may be handed to the + configured host hook". + """ + ns = _clean(namespace) + md = _clean(mode) + action_id = ACTION_FOR_MODE.get(md, ACTION_RESTART_NAMESPACE) + preview = build_restart_preview(ns, md, principal=principal, env=env) + + def refuse(reason_code: str, detail: str) -> dict[str, Any]: + return { + "allowed": False, + "gates_passed": False, + "reason_code": reason_code, + "detail": detail, + "action_id": action_id, + "namespace": ns, + "mode": md, + "preview": preview, + "execution_enabled": False, + } + + scope_error = _validate_scope(ns, md) + if scope_error is not None: + return refuse(*scope_error) + + # Authority is checked as an authorization decision, not an execution + # grant. ``for_execution=True`` asks "may the console perform this write?", + # and the answer here is permanently no: step 2 of the ledger is a request + # to the host supervisor, so the console's Phase 2 execution gate is not + # the relevant gate. Every branch below keeps ``execution_enabled`` False + # and :func:`execute_restart` never touches a process. + decision = console_authz.authorize(action_id, principal) + if not decision.allowed: + return refuse(DENY_UNAUTHORIZED, decision.detail) + + if not _clean(confirmation): + return refuse( + DENY_CONFIRMATION_MISSING, + ( + "Type the confirmation phrase " + f"{preview['confirmation_phrase']!r} to proceed." + ), + ) + if not confirmation_matches(ns, md, confirmation): + return refuse( + DENY_CONFIRMATION_MISMATCH, + ( + "Confirmation does not name this namespace and mode; expected " + f"{preview['confirmation_phrase']!r}." + ), + ) + + operator = preview["operator_authorization"] + if not operator["authorized"]: + return refuse( + DENY_OPERATOR_AUTHORIZATION, + ( + "Host daemon maintenance requires out-of-band operator " + "authorization via " + f"{runtime_recovery_guard.OPERATOR_AUTHORIZATION_ENV}." + ), + ) + + # #630's task-scoped gate deliberately lets a contaminated worker keep + # commenting and handing off. Restart is stricter and unconditional: a + # runtime already contaminated by a manual kill must be reconciled before + # it is restarted again, or the restart just launders the contamination. + if contamination_marker and not contamination_marker.get( + "cleared_by_reconciler" + ): + return refuse( + DENY_CONTAMINATED_RUNTIME, + ( + "A live contamination marker is present; clear it through the " + "reconciler path before restarting." + ), + ) + + hook = preview["restart_hook"] + if not hook["configured"]: + return refuse( + DENY_HOOK_NOT_CONFIGURED, + ( + "No host-managed restart hook is configured " + f"({RESTART_HOOK_ENV}). The console will not fall back to a " + "process kill." + ), + ) + + return { + "allowed": True, + "gates_passed": True, + "reason_code": ALLOW_HOST_ACTION_REQUIRED, + "detail": ( + "Every gate passed. The restart must be performed by the " + "configured host supervisor; the console does not signal the " + "process." + ), + "action_id": action_id, + "namespace": ns, + "mode": md, + "preview": preview, + "execution_enabled": False, + "console_executes": False, + "console_active_phase": console_authz.ACTIVE_PHASE, + } + + +# --- Execution -------------------------------------------------------------- + + +def execute_restart( + namespace: str, + mode: str = MODE_RESTART, + *, + principal: console_authz.Principal | None = None, + confirmation: str | None = None, + contamination_marker: dict[str, Any] | None = None, + env: dict[str, str] | None = None, + request_id: str | None = None, + session_id: str | None = None, +) -> dict[str, Any]: + """Run every gate, audit the outcome, and hand off to the host. + + This function never spawns a process, never sends a signal, and never + builds a command line. ``success`` is False in both directions: a refused + request is refused, and an authorized request still requires the host + supervisor to act. + """ + assessment = assess_restart_request( + namespace, + mode, + principal=principal, + confirmation=confirmation, + contamination_marker=contamination_marker, + env=env, + ) + action_id = assessment["action_id"] + allowed = assessment["allowed"] + + audit = console_audit.record_event( + action_id=action_id, + result=( + console_audit.RESULT_ALLOWED if allowed + else console_audit.RESULT_DENIED + ), + principal=principal, + target={"namespace": assessment["namespace"], "mode": assessment["mode"]}, + reason_code=assessment["reason_code"], + detail=assessment["detail"], + request_id=request_id, + session_id=session_id, + metadata={ + "gates_passed": assessment["gates_passed"], + "process_kill_executed": False, + "post_restart_verification_required": True, + }, + ) + + return { + "success": False, + "outcome": ( + ALLOW_HOST_ACTION_REQUIRED if allowed else assessment["reason_code"] + ), + "allowed": allowed, + "detail": assessment["detail"], + "namespace": assessment["namespace"], + "mode": assessment["mode"], + "action_id": action_id, + "process_kill_executed": False, + "host_hook": assessment["preview"]["restart_hook"], + "next_action": ( + "Have the host supervisor perform the restart, then call " + "verify_post_restart_health with live client-namespace evidence " + "before claiming a clean session." + if allowed + else assessment["detail"] + ), + "assessment": assessment, + "audit": audit, + } + + +# --- Contamination classification ------------------------------------------- + + +def classify_restart_command( + command: str | None, + *, + mcp_pids: list[Any] | tuple[Any, ...] | None = None, + session_id: str | None = None, + remote: str | None = None, + role: str | None = None, +) -> dict[str, Any]: + """Classify an operator-proposed recovery command (#630 AC2). + + A manual ``pkill``/``kill`` of the MCP daemon is contamination, not a + restart. When contaminating, a durable marker is returned so downstream + gated mutations and clean claims fail closed. + """ + classification = runtime_recovery_guard.classify_recovery_command( + command, mcp_pids=mcp_pids + ) + contaminating = bool(classification.get("contamination")) + + marker = None + if contaminating: + marker = runtime_recovery_guard.build_contamination_record( + reason_class=( + classification.get("reason_class") + or runtime_recovery_guard.REASON_MANUAL_DAEMON_KILL + ), + command_redacted=classification.get("redacted_command"), + session_id=session_id, + remote=remote, + role=role, + detail=( + "Manual daemon kill is forbidden; use the sanctioned " + f"{ACTION_RESTART_NAMESPACE} gated action instead." + ), + ) + + return { + "contamination": contaminating, + "sanctioned": not contaminating and not classification.get("process_kill"), + "clean_claim_allowed": not contaminating, + "reason_class": classification.get("reason_class"), + "redacted_command": classification.get("redacted_command"), + "classification": classification, + "contamination_marker": marker, + "sanctioned_alternative": ACTION_RESTART_NAMESPACE, + } + + +# --- Post-restart health verification --------------------------------------- + + +def verify_post_restart_health( + namespace: str, + *, + probe_result: dict[str, Any] | None = None, + probe_source: str | None = None, + registered_tools: list[str] | tuple[str, ...] | None = None, + required_tool: str | None = None, + profile: str | None = None, +) -> dict[str, Any]: + """Require live proof a namespace is callable before any clean claim (AC3). + + Static registration is not proof and neither is an offline subprocess + probe: only ``probe_source=client_namespace`` evidence can clear a + post-restart session for mutations. + """ + ns = _clean(namespace) + health = mcp_namespace_health.classify_namespace_probe( + ns, + required_tool=required_tool, + registered_tools=registered_tools, + probe_result=probe_result, + profile=profile, + probe_source=probe_source, + ) + healthy = bool(health.get("healthy")) + proven = bool(health.get("ide_namespace_proven")) + + if healthy and proven: + status = HEALTH_CLEAN + elif healthy: + status = HEALTH_UNPROVEN + else: + status = HEALTH_UNHEALTHY + + reasons = list(health.get("reasons") or []) + if status == HEALTH_UNPROVEN: + reasons.append( + "Namespace reported healthy without live client-namespace " + "evidence; a post-restart clean claim requires " + f"probe_source={mcp_namespace_health.PROBE_SOURCE_CLIENT!r}." + ) + + return { + "namespace": ns, + "status": status, + "healthy": healthy, + "ide_namespace_proven": proven, + "clean_claim_allowed": status == HEALTH_CLEAN, + "mutations_allowed": status == HEALTH_CLEAN, + "reasons": reasons, + "health": health, + } + + +# --- Policy surface --------------------------------------------------------- + + +def restart_policy() -> dict[str, Any]: + """Machine-readable description of the sanctioned restart contract.""" + return { + "policy_version": 1, + "modes": list(MODES), + "actions": [ACTION_RESTART_NAMESPACE, ACTION_RELOAD_NAMESPACE], + "known_namespaces": list(KNOWN_NAMESPACES), + "fleet_scope_permitted": False, + "console_executes_process_kill": False, + "raw_kill_exposed": False, + "requires_confirmation": True, + "confirmation_binds_namespace": True, + "operator_authorization_env": ( + runtime_recovery_guard.OPERATOR_AUTHORIZATION_ENV + ), + "restart_hook_env": RESTART_HOOK_ENV, + "manual_kill_classified_as": runtime_recovery_guard.CONTAMINATION_KIND, + "post_restart_clean_claim_requires": ( + mcp_namespace_health.PROBE_SOURCE_CLIENT + ), + "audit_required": True, + "silent_auto_restart_permitted": False, + }