From 220361ad940cbad7617386a8b6c50c740cddd0bc Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 02:36:41 -0400 Subject: [PATCH] fix(restart): require both authorizations for apply, correct coordinator doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two blockers raised in the PR #886 review (comment 16559) for issue #663. B1 — apply_authorized ignored restart-class authorization. The #663 restart-class matrix and the #661 drain-proof hard gate are independent authorizations that first coexisted when PR #882 landed on master and this branch merged it. The union preserved both, but the apply decision consulted only the drain gate: payload["apply_authorized"] = gate.allow so a clean drain proof — or an authorized break-glass, which needs no proof at all — reported apply_authorized: True for a class the least-privilege matrix had just denied, in the same payload carrying allow_restart: False and "role 'author' may not request full_mcp_restart". One environment variable therefore collapsed the whole nine-class matrix for the apply decision, including host_restart. The apply decision is now the conjunction of both authorizations, and apply_gate carries drain_gate_allow and restart_class_authorized so a denial is attributable to the authorization that produced it. Break-glass keeps its purpose — bypassing the drain proof — and never bypasses the class matrix. No existing fail-closed behaviour is weakened: allow_restart, drain-proof verification, fingerprint binding, and requester authorization are untouched. B2 — docs/mcp-restart-coordinator.md described pre-#661 behaviour. The document still called the drain proof "a separate child" and omitted drain_proof_json and request_break_glass from the published signature, so a safety document asserted there was no gate where a gate now exists. It now documents both parameters, states that the gate executes inside this tool, and records dry-run versus apply behaviour, authorization ordering, the break-glass scope, and fail-closed conditions as implemented. Regression coverage. tests/test_issue_886_apply_authorization_conjunction.py exercises the MCP tool itself, which previously had no test at all — that absence is why the defect shipped. It pins both conjunction directions, proves a clean proof cannot override a role, approval, unknown-class, or missing-target denial, proves break-glass does not collapse the matrix for any worker role or restricted class, and proves the existing scoped and unscoped paths and the #661 denials still hold. Against the pre-fix tree 24 of these fail; against this commit all 19 pass with 45 subtests. tests/test_mcp_restart_governance_docs.py now binds the published signature to inspect.signature() of the real tool and forbids the stale pre-#661 phrasing, so the drift that produced B2 cannot return unnoticed. Verification: targeted restart/drain/governance/webui suites 194 passed, 113 subtests. Full suite 23 failed, 5230 passed, 6 skipped, 912 subtests — the failure set is identical to the reviewed baseline at 9bc021e (23 failed, 5201 passed), with +29 passing from the added tests and no new or changed failure. Zero conflict markers; py_compile passes; the #882 union remains intact in both directions. Refs #663, PR #886 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V6xFqovhbArPv61j9KCGkL --- docs/mcp-restart-coordinator.md | 65 ++- gitea_mcp_server.py | 30 +- ...sue_886_apply_authorization_conjunction.py | 381 ++++++++++++++++++ tests/test_mcp_restart_governance_docs.py | 117 ++++++ 4 files changed, 582 insertions(+), 11 deletions(-) create mode 100644 tests/test_issue_886_apply_authorization_conjunction.py diff --git a/docs/mcp-restart-coordinator.md b/docs/mcp-restart-coordinator.md index 0d0f114..628895f 100644 --- a/docs/mcp-restart-coordinator.md +++ b/docs/mcp-restart-coordinator.md @@ -6,11 +6,17 @@ console (#642 / #652) can see the blast radius *before* concurrent LLM work is disrupted. Uncoordinated restarts destroy in-flight author/reviewer/merger work and give operators no way to see what they are about to break. -This lands the coordinator + impact DTO + a dry-run MCP tool. It is the single +This lands the coordinator + impact DTO + the MCP tool. It is the single sanctioned entry point for restart evaluation post-#657 (which inventoried the -restart/reload/kill paths). The **mutative apply** path — actually performing a -restart — is a later child gated by a drain proof and is explicitly out of -scope here. +restart/reload/kill paths). + +The **drain-proof hard gate now executes inside this tool** (#661, via PR #882): +an apply request (`dry_run=False`) is evaluated against a drain proof here and +denied when that proof is missing, expired, unclean, tampered with, or stale. +It is no longer a separate child operation. What remains a later child is only +the **execution** step — actually stopping and restoring a process. This tool +still never restarts anything: `apply_supported` is always `false` and +`restart_performed` is always `false`. The coordinator now routes every request through the restart-class policy matrix defined for #663. See @@ -25,7 +31,8 @@ recovery behavior for all nine classes. | `restart_coordinator.evaluate_restart_impact` | `restart_coordinator.py` | Pure classification: inventory → impact report DTO. No I/O, no restart. | | `RestartImpactReport` / `SessionImpact` / `LeaseImpact` | `restart_coordinator.py` | Console-facing DTO (`.as_dict()` is JSON-serializable). | | `ControlPlaneDB.list_sessions` | `control_plane_db.py` | Read-only session inventory (the process-level unit a restart kills). | -| `gitea_request_mcp_restart` | `gitea_mcp_server.py` | MCP tool: gathers inventory from the #613 DB, calls the coordinator, returns the report. Dry-run only. | +| `gitea_request_mcp_restart` | `gitea_mcp_server.py` | MCP tool: gathers inventory from the #613 DB, calls the coordinator, returns the report, and on `dry_run=False` runs the #661 drain-proof hard gate. Never restarts a process. | +| `drain_proof.gate_apply_restart` | `drain_proof.py` | The #661 hard gate: verifies a drain proof against the current impact fingerprint, or records an authorized break-glass bypass. | ## Dimensions evaluated @@ -86,12 +93,52 @@ gitea_request_mcp_restart(remote, host, org, repo, session_id=None, limit=200, restart_class="full_mcp_restart", target_session_id=None, target_role=None, - target_connector=None) + target_connector=None, + drain_proof_json=None, + request_break_glass=False) ``` -Read-only, dry-run, and it **never restarts anything**. `apply_supported` is -always `false`; passing `dry_run=False` performs no restart and reports that -apply is gated by a drain proof (a separate child). +It **never restarts anything**: `apply_supported` is always `false` and +`restart_performed` is always `false`. + +### Dry-run versus apply + +| Call | Behavior | +|------|----------| +| `dry_run=True` (default) | Read-only impact preview. No drain proof is required or consulted. | +| `dry_run=False` | The #661 drain-proof hard gate runs **in this tool**. The outcome is reported under `apply_gate` / `apply_authorized`; a denial also returns a durable `incident` descriptor. Still no restart. | + +### Authorization ordering + +An apply requires **both** authorizations, and they are independent: + +1. **Restart-class authorization** (#663) — the requester's role and permissions + must allow the requested class, the class's approval requirement must be + satisfied, and any target-scoped class must name its target. Failing any of + these makes `allow_restart` `false`. +2. **Drain-proof gate** (#661) — a valid, unexpired, clean proof bound to the + current impact fingerprint, or an authorized break-glass. + +`apply_authorized` is the conjunction: `gate.allow and allow_restart`. A clean +drain proof therefore cannot override a class or requester-role denial, and a +denied class never reports an authorized apply. `apply_gate` carries +`drain_gate_allow` and `restart_class_authorized` so a denial is attributable to +the authorization that produced it. + +### Break-glass + +Break-glass bypasses the **drain proof only** — never the restart-class matrix. +It is honoured solely when `request_break_glass` is set *and* the environment +carries `GITEA_BREAKGLASS_RESTART_AUTHORIZATION`; like operator override, the +tool argument expresses caller intent and cannot be self-asserted by a worker +session. `break_glass_requested` and `break_glass_authorized` are both reported, +so a bypass is never silent. + +### Fail closed on apply + +A missing, malformed, expired, unclean, tampered, or fingerprint-stale drain +proof denies the apply and returns an `incident` descriptor. An unknown restart +class denies before any of this. Ambiguity always denies. ## Audit diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index d479fd9..6e2ae8d 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -22367,7 +22367,16 @@ def gitea_request_mcp_restart( only when ``request_break_glass`` is set *and* the environment carries ``GITEA_BREAKGLASS_RESTART_AUTHORIZATION``. Even an authorized gate performs no restart here; actual execution is a further child. The gate outcome is - reported under ``apply_gate`` / ``apply_authorized``. + reported under ``apply_gate``. + + ``apply_authorized`` requires **both** authorizations to pass: the #661 drain + gate *and* the #663 restart-class matrix (``allow_restart``). They are + independent — the drain gate proves the blast radius was drained and knows + nothing about whether this requester may request this class — so a class the + matrix denied never reports an authorized apply. Break-glass bypasses the + drain proof only; it never bypasses the class matrix. ``apply_gate`` carries + ``drain_gate_allow`` and ``restart_class_authorized`` so a denial is + attributable to the authorization that produced it. Operator override authority is read from the process environment (``GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION``), never self-asserted by @@ -22546,8 +22555,25 @@ def gitea_request_mcp_restart( gate_payload["reasons"] = [proof_parse_error] + list( gate_payload.get("reasons") or [] ) + # The #663 restart-class matrix and the #661 drain gate are two + # independent authorizations, and an apply requires BOTH. ``gate.allow`` + # proves only that the blast radius was drained — or that break-glass + # was authorized — and knows nothing about whether this requester may + # request this class at all. Conjoining them keeps a class the matrix + # denied from ever reporting an authorized apply, and keeps break-glass + # scoped to what it is for: bypassing the drain proof, never the + # least-privilege class matrix. + restart_class_authorized = bool(report.allow_restart) + gate_payload["drain_gate_allow"] = bool(gate.allow) + gate_payload["restart_class_authorized"] = restart_class_authorized + if not restart_class_authorized: + gate_payload["reasons"] = list(gate_payload.get("reasons") or []) + [ + "restart class authorization denied; apply denied regardless of " + "drain proof or break-glass (fail closed, #663)", + *(report.authorization_reasons or []), + ] payload["apply_gate"] = gate_payload - payload["apply_authorized"] = gate.allow + payload["apply_authorized"] = bool(gate.allow and restart_class_authorized) payload["break_glass_requested"] = bool(request_break_glass) payload["break_glass_authorized"] = break_glass_authorized # Even an authorized gate performs no restart here: execution is a later diff --git a/tests/test_issue_886_apply_authorization_conjunction.py b/tests/test_issue_886_apply_authorization_conjunction.py new file mode 100644 index 0000000..6fa2888 --- /dev/null +++ b/tests/test_issue_886_apply_authorization_conjunction.py @@ -0,0 +1,381 @@ +"""``apply_authorized`` requires BOTH authorizations (#886 review blocker B1). + +The #663 restart-class matrix and the #661 drain-proof hard gate are independent +authorizations that first coexisted when PR #882 landed on master and PR #886 +merged it into the restart-class branch. The union preserved both, but the apply +decision consulted only the drain gate:: + + payload["apply_authorized"] = gate.allow # pre-fix + +so a clean drain proof — or an authorized break-glass, which needs no proof at +all — reported ``apply_authorized: True`` for a restart class the least-privilege +matrix had just denied, in the same payload that carried +``allow_restart: False`` and "role 'author' may not request full_mcp_restart". + +These tests pin the conjunction and the properties that must survive it. They +exercise the real MCP tool, which previously had no test coverage at all — that +absence is why the defect shipped. +""" + +from __future__ import annotations + +import json +import os +import unittest +from unittest.mock import patch + +import drain_proof +import gitea_mcp_server as srv + +CONTROLLER_APPROVAL_ENV = "GITEA_CONTROLLER_RESTART_APPROVAL_AUTHORIZATION" +BREAK_GLASS_ENV = "GITEA_BREAKGLASS_RESTART_AUTHORIZATION" + +# A quiet control plane: nothing live, so the blast radius never masks the +# authorization outcome under test. +QUIET_SESSIONS: list[dict] = [] +QUIET_LEASES: list[dict] = [] + + +class _FakeDB: + """Minimal control-plane DB stand-in for the restart inventory.""" + + def __init__(self, sessions=QUIET_SESSIONS, terminal=None): + self._sessions = list(sessions) + self._terminal = terminal + + def list_sessions(self, statuses=None, limit=None): + return list(self._sessions) + + def get_active_terminal_lock(self, remote=None, org=None, repo=None): + return self._terminal + + +def _profile(role: str) -> dict: + return {"profile_name": f"prgs-{role}", "role_kind": role, "role": role} + + +class _RestartToolHarness(unittest.TestCase): + """Drives the real ``gitea_request_mcp_restart`` with a stubbed inventory.""" + + def _call(self, *, role: str, env: dict | None = None, **kwargs) -> dict: + environ = {k: v for k, v in os.environ.items() + if k not in (CONTROLLER_APPROVAL_ENV, BREAK_GLASS_ENV)} + environ.update(env or {}) + with patch.object(srv, "_profile_operation_gate", return_value=None), \ + patch.object(srv, "_resolve", + return_value=("gitea.prgs.cc", + "Scaled-Tech-Consulting", + "Gitea-Tools")), \ + patch.object(srv, "get_profile", return_value=_profile(role)), \ + patch.object(srv, "_control_plane_db_or_error", + return_value=(_FakeDB(), [])), \ + patch.object(srv.lease_lifecycle, "list_active_leases", + return_value={"leases": list(QUIET_LEASES)}), \ + patch.dict(os.environ, environ, clear=True): + return srv.gitea_request_mcp_restart( + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + session_id="probe-session", + **kwargs, + ) + + def _clean_proof_for(self, preview: dict) -> str: + """Mint a genuinely clean, signature-valid proof bound to *preview*. + + Built from the tool's own dry-run report, so the fingerprint matches and + the proof is rejected for authorization reasons only — never because it + was stale or forged. + """ + proof = drain_proof.build_drain_proof( + impact_report=preview, + drain_state={ + "assignments_stopped": True, + "checkpoints_complete": True, + "handoffs_verified": True, + "leases_handled": True, + "acks": {}, + }, + requesting_session_id="probe-session", + ) + self.assertTrue(proof.clean, "harness must mint a clean proof") + return json.dumps(proof.as_dict()) + + +class TestConjunction(_RestartToolHarness): + """AC1/AC2 — the two authorizations are ANDed, in both directions.""" + + def test_gate_allow_with_class_denied_yields_apply_authorized_false(self): + # An author may not request full_mcp_restart (CONTROL_ROLES only). + preview = self._call(role="author", restart_class="full_mcp_restart") + self.assertFalse(preview["allow_restart"]) + + result = self._call( + role="author", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + + self.assertTrue(result["apply_gate"]["drain_gate_allow"], + "drain gate itself should have allowed this proof") + self.assertFalse(result["apply_gate"]["restart_class_authorized"]) + self.assertFalse(result["apply_authorized"], + "a clean proof must not authorize a denied class") + self.assertFalse(result["allow_restart"]) + + def test_gate_allow_with_class_allowed_can_yield_apply_authorized_true(self): + preview = self._call( + role="operator", + restart_class="full_mcp_restart", + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertTrue(preview["allow_restart"], + "operator + controller approval must authorize the class") + + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + + self.assertTrue(result["apply_gate"]["drain_gate_allow"]) + self.assertTrue(result["apply_gate"]["restart_class_authorized"]) + self.assertTrue(result["apply_authorized"], + "both authorizations pass; apply must be authorized") + + def test_denial_is_attributable_to_the_authorization_that_caused_it(self): + preview = self._call(role="author", restart_class="full_mcp_restart") + result = self._call( + role="author", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + blob = " ".join(result["apply_gate"]["reasons"]).lower() + self.assertIn("restart class authorization denied", blob) + self.assertIn("full_mcp_restart", blob) + + +class TestProofCannotOverrideAuthorization(_RestartToolHarness): + """AC3 — a clean proof never overrides a class or requester-role denial.""" + + def test_clean_proof_cannot_override_role_denial(self): + for role in ("author", "reviewer", "merger", "reconciler"): + with self.subTest(role=role): + preview = self._call(role=role, restart_class="full_mcp_restart") + result = self._call( + role=role, + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + self.assertFalse(result["apply_authorized"]) + + def test_clean_proof_cannot_override_missing_controller_approval(self): + # Correct role, but the class demands controller approval and the + # environment carries none. + preview = self._call(role="operator", restart_class="full_mcp_restart") + self.assertFalse(preview["allow_restart"]) + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + self.assertFalse(result["apply_authorized"]) + + def test_clean_proof_cannot_override_unknown_class(self): + preview = self._call(role="operator", restart_class="not_a_real_class", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertFalse(preview["allow_restart"]) + result = self._call( + role="operator", + restart_class="not_a_real_class", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env={CONTROLLER_APPROVAL_ENV: "yes"}, + ) + self.assertFalse(result["apply_authorized"]) + + def test_clean_proof_cannot_override_missing_scope_target(self): + # worker_restart without target_session_id fails closed on scoping. + preview = self._call(role="operator", restart_class="worker_restart", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertFalse(preview["allow_restart"]) + result = self._call( + role="operator", + restart_class="worker_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env={CONTROLLER_APPROVAL_ENV: "yes"}, + ) + self.assertFalse(result["apply_authorized"]) + + +class TestBreakGlassDoesNotCollapseTheMatrix(_RestartToolHarness): + """AC4 — break-glass bypasses the drain proof only, never the class matrix.""" + + def test_break_glass_does_not_authorize_a_denied_class(self): + result = self._call( + role="author", + restart_class="host_restart", + dry_run=False, + request_break_glass=True, + env={BREAK_GLASS_ENV: "operator-issued"}, + ) + self.assertTrue(result["break_glass_authorized"]) + self.assertTrue(result["apply_gate"]["drain_gate_allow"], + "break-glass does satisfy the drain gate") + self.assertFalse(result["apply_gate"]["restart_class_authorized"]) + self.assertFalse(result["apply_authorized"], + "break-glass must not collapse the class matrix") + + def test_break_glass_across_every_worker_role_and_restricted_class(self): + for role in ("author", "reviewer", "merger", "reconciler"): + for klass in ("rolling_mcp_restart", "full_mcp_restart", + "host_restart"): + with self.subTest(role=role, restart_class=klass): + result = self._call( + role=role, + restart_class=klass, + dry_run=False, + request_break_glass=True, + env={BREAK_GLASS_ENV: "operator-issued"}, + ) + self.assertFalse(result["apply_authorized"]) + + def test_break_glass_still_works_when_the_class_is_authorized(self): + # Break-glass keeps its purpose: skipping the drain proof for a caller + # the matrix does allow. + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + request_break_glass=True, + env={BREAK_GLASS_ENV: "operator-issued", + CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertTrue(result["apply_authorized"]) + self.assertEqual(result["apply_gate"]["verdict"], "break_glass") + + def test_break_glass_is_not_self_assertable(self): + # Requested but no environment authorization -> no bypass, and the + # unproven apply is denied. + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + request_break_glass=True, + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertTrue(result["break_glass_requested"]) + self.assertFalse(result["break_glass_authorized"]) + self.assertFalse(result["apply_authorized"]) + self.assertIn("incident", result) + + +class TestRestrictedClassesStayDenied(_RestartToolHarness): + """AC5 — restricted classes remain denied to unauthorized requesters.""" + + def test_restricted_classes_denied_for_worker_roles(self): + for role in ("author", "reviewer", "merger", "reconciler"): + for klass in ("rolling_mcp_restart", "full_mcp_restart", + "host_restart"): + with self.subTest(role=role, restart_class=klass): + preview = self._call( + role=role, + restart_class=klass, + env={CONTROLLER_APPROVAL_ENV: "yes"}, + ) + self.assertFalse(preview["allow_restart"]) + self.assertFalse(preview["permission_authorized"]) + self.assertFalse(preview["role_authorized"]) + + def test_host_restart_needs_controller_and_infrastructure_operator(self): + # controller approval alone is not enough for host_restart. + preview = self._call(role="controller", restart_class="host_restart", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertFalse(preview["approval_satisfied"]) + self.assertFalse(preview["allow_restart"]) + + +class TestExistingPathsStillWork(_RestartToolHarness): + """AC6 — valid scoped and unscoped restart paths are unaffected.""" + + def test_dry_run_never_reports_apply_authorization(self): + result = self._call(role="operator", restart_class="full_mcp_restart", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertNotIn("apply_authorized", result) + self.assertNotIn("apply_gate", result) + self.assertFalse(result["apply_supported"]) + self.assertFalse(result["restart_performed"]) + + def test_self_service_unscoped_classes_authorize_for_every_role(self): + for role in ("author", "reviewer", "merger", "reconciler", + "controller", "operator", "admin"): + for klass in ("client_reconnect", "session_reconnect"): + with self.subTest(role=role, restart_class=klass): + preview = self._call(role=role, restart_class=klass) + self.assertTrue(preview["allow_restart"]) + + def test_scoped_class_with_target_authorizes_and_applies(self): + env = {CONTROLLER_APPROVAL_ENV: "operator-approved"} + preview = self._call(role="operator", restart_class="worker_restart", + target_session_id="worker-1", env=env) + self.assertTrue(preview["allow_restart"]) + + result = self._call( + role="operator", + restart_class="worker_restart", + target_session_id="worker-1", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env=env, + ) + self.assertTrue(result["apply_authorized"]) + + def test_apply_still_denies_without_any_proof(self): + # The #661 hard gate is untouched by the conjunction. + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertFalse(result["apply_gate"]["drain_gate_allow"]) + self.assertTrue(result["apply_gate"]["restart_class_authorized"]) + self.assertFalse(result["apply_authorized"]) + self.assertEqual(result["incident"]["kind"], "restart_drain_gate_denied") + + def test_apply_denies_on_malformed_proof(self): + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json="{not valid json", + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertFalse(result["apply_authorized"]) + self.assertTrue(any("invalid drain_proof_json" in reason + for reason in result["apply_gate"]["reasons"])) + + def test_tool_never_restarts_on_any_path(self): + for kwargs in ( + {"restart_class": "client_reconnect"}, + {"restart_class": "full_mcp_restart", "dry_run": False}, + {"restart_class": "host_restart", "dry_run": False, + "request_break_glass": True}, + ): + with self.subTest(**kwargs): + result = self._call(role="operator", env={ + CONTROLLER_APPROVAL_ENV: "yes", BREAK_GLASS_ENV: "yes"}, + **kwargs) + self.assertFalse(result["restart_performed"]) + self.assertFalse(result["apply_supported"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_mcp_restart_governance_docs.py b/tests/test_mcp_restart_governance_docs.py index 5b12e50..e49bcb4 100644 --- a/tests/test_mcp_restart_governance_docs.py +++ b/tests/test_mcp_restart_governance_docs.py @@ -105,3 +105,120 @@ def test_cross_links_do_not_embed_secrets(): text = _read(path) for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"): assert marker not in text, f"{path} contains {marker!r}" + + +# --- Coordinator doc stays in lock-step with the tool (#886 review blocker B2) -- +# +# PR #882 moved the #661 drain-proof hard gate *into* gitea_request_mcp_restart, +# but the coordinator document still described the proof as "a separate child" +# and omitted both new parameters. Nothing referenced that document, so nothing +# caught the drift. These tests bind the prose to the real signature. + +COORDINATOR_DOC = REPO_ROOT / "docs" / "mcp-restart-coordinator.md" + +# Affirmative claims that were accurate before #661 landed and are now false. +# Matched against whitespace-normalized text so re-wrapping cannot hide them. +# Deliberately not the bare phrase "a separate child": the corrected prose uses +# it in a negation ("no longer a separate child operation"), and a guard that +# forbids naming the old behaviour would block explaining that it changed. +STALE_PRE_661_PHRASES = ( + "gated by a drain proof (a separate child)", + "is a later child gated by a drain proof", + "mutative apply path is explicitly out of scope", + "apply is gated by a drain proof (a separate child)", +) + + +def _documented_signature_block() -> str: + """The fenced signature block for the tool, as published in the doc.""" + text = _read(COORDINATOR_DOC) + marker = "gitea_request_mcp_restart(" + start = text.index(marker) + end = text.index("```", start) + return text[start:end] + + +def test_documented_signature_matches_the_real_tool_signature(): + import inspect + + import gitea_mcp_server + + block = _documented_signature_block() + real = inspect.signature(gitea_mcp_server.gitea_request_mcp_restart) + for name in real.parameters: + assert name in block, ( + f"docs/mcp-restart-coordinator.md documents no {name!r} parameter; " + "the published signature has drifted from the tool" + ) + + +def test_drain_proof_and_break_glass_parameters_are_documented(): + block = _documented_signature_block() + for name in ("drain_proof_json", "request_break_glass"): + assert name in block, f"signature block missing {name}" + + +def test_restart_class_and_target_scoping_parameters_survive(): + block = _documented_signature_block() + for name in ("restart_class", "target_session_id", "target_role", + "target_connector"): + assert name in block, f"signature block lost #663 parameter {name}" + + +def test_gate_is_documented_as_executing_inside_this_tool(): + lower = _read(COORDINATOR_DOC).lower() + assert "inside this tool" in lower, ( + "the coordinator doc must state that the drain-proof gate executes in " + "gitea_request_mcp_restart, not in a later child" + ) + assert "no longer a separate child operation" in lower + + +def test_stale_pre_661_wording_cannot_return(): + normalized = " ".join(_read(COORDINATOR_DOC).split()).lower() + for phrase in STALE_PRE_661_PHRASES: + assert phrase not in normalized, ( + f"stale pre-#661 wording returned to the coordinator doc: {phrase!r}" + ) + + +def test_dry_run_versus_apply_behavior_is_documented(): + lower = _read(COORDINATOR_DOC).lower() + assert "dry_run=true" in lower and "dry_run=false" in lower + assert "apply_supported" in lower and "restart_performed" in lower + assert "never restarts anything" in lower + + +def test_authorization_ordering_and_conjunction_are_documented(): + text = _read(COORDINATOR_DOC) + lower = text.lower() + assert "authorization ordering" in lower + assert "allow_restart" in text + assert "apply_authorized" in text + # The conjunction itself, and the attribution fields behind it. + assert "gate.allow and allow_restart" in text + for field in ("drain_gate_allow", "restart_class_authorized"): + assert field in text, f"doc omits apply_gate.{field}" + + +def test_break_glass_scope_is_documented_as_drain_proof_only(): + text = _read(COORDINATOR_DOC) + lower = text.lower() + assert "break-glass" in lower + assert "drain proof only" in lower, ( + "doc must state break-glass never bypasses the restart-class matrix" + ) + assert "GITEA_BREAKGLASS_RESTART_AUTHORIZATION" in text + + +def test_fail_closed_on_apply_is_documented(): + lower = _read(COORDINATOR_DOC).lower() + assert "fail closed" in lower + for condition in ("expired", "unclean", "tampered", "stale"): + assert condition in lower, f"fail-closed list omits {condition!r}" + + +def test_coordinator_doc_embeds_no_secrets(): + text = _read(COORDINATOR_DOC) + for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"): + assert marker not in text, f"{COORDINATOR_DOC} contains {marker!r}"