fix(mcp): remediate B8, B6/B11, and B15 break-glass restart blockers (#664)

- B8: Correct redaction boundary for GITEA_TOKEN= and URI userinfo without destroying adjacent audit evidence or benign sec- text - B6/B11: Remove false restart execution claims from default executor when GITEA_SANCTIONED_RESTART_HOOK is non-empty - B15: Document deployable production grant set (runtime.break_glass_restart and gitea.issue.create) for prgs-controller - Preserve B13, B1, B14 and previously accepted corrections
2026-07-29 01:23:10 -04:00
parent c67f39b40e
commit e423dd5870
4 changed files with 163 additions and 36 deletions
@@ -148,7 +148,7 @@ so a bypass is never silent.

 The dedicated MCP tool `gitea_break_glass_restart` provides the privileged emergency break-glass restart workflow when graceful drain cannot complete:

- **Authorization (#664 AC1 / B1 / B13)**: Requires the exact trusted profile `prgs-controller` **and** an explicit `runtime.break_glass_restart` grant enforced by the real production operation gate (no `gitea.read` fallback). Ordinary roles, non-controller reconcilers, lookalike profile names (`fake-controller`, …), and env vars cannot authorize. A narrow break-glass capability does **not** redefine the profile's declared global role.
+- **Authorization (#664 AC1 / B1 / B13 / B15)**: Requires the exact trusted profile `prgs-controller` **and** explicit `runtime.break_glass_restart` and `gitea.issue.create` grants enforced by the real production operation gate (no `gitea.read` fallback). Incident creation is mandatory prior to execution (`gitea.issue.create`), so the deployable production policy for `prgs-controller` includes `allowed_operations`: `["gitea.read", "gitea.pr.close", "gitea.pr.comment", "gitea.issue.comment", "gitea.issue.create", "runtime.break_glass_restart", "gitea.branch.delete", "gitea.decision_lock.irrecoverable_recovery"]`. Ordinary roles, non-controller reconcilers, lookalike profile names (`fake-controller`, …), and env vars cannot authorize. A narrow break-glass capability does **not** redefine the profile's declared global role. Updating a live running `prgs-controller` profile in production requires an operator configuration update and daemon reload post-merge.
 - **Required Parameters (#664 AC2)**:
  - `reason`: Mandatory non-empty string (min 10 characters).
  - `confirmation`: Must equal exactly `"I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION"`.
@@ -76,17 +76,16 @@ _BARE_SECRET_PATTERN = re.compile(
    r')\b'
 )

-# Key/value credentials embedded in free text (password=..., api_key: ..., etc.).
-# Value group accepts quoted strings, Authorization scheme + credential
-# (Bearer/Basic/Token + token), or a single non-space run.
+# Key/value credentials embedded in free text (password=..., api_key: ..., GITEA_TOKEN=..., etc.).
+# Group 1 captures the key name (e.g. GITEA_TOKEN, password, api_key).
+# Group 2 captures delimiter/whitespace (=, : ).
+# Group 3 captures the secret value, stopping at whitespace or non-secret delimiters (&, ;, ,, quotes, closing brackets).
 _ASSIGNMENT_SECRET_PATTERN = re.compile(
-    r'(?i)\b('
-    r'token|password|passwd|pwd|secret|api[_-]?key|access[_-]?key|'
-    r'client[_-]?secret|private[_-]?key|authorization|credential'
-    r')\b(\s*[:=]\s*)('
+    r'(?i)\b([A-Za-z0-9_]*?(?:token|password|passwd|pwd|secret|api[_-]?key|access[_-]?key|'
+    r'client[_-]?secret|private[_-]?key|authorization|credential))\b(\s*[:=]\s*)('
    r'"[^"]*"|\'[^\']*\'|'
-    r'(?:Bearer|Basic|Token)\s+\S+|'
-    r'\S+'
+    r'(?:Bearer|Basic|Token)\s+[^\s;&,"\'\)\}\]\>]+|'
+    r'[^\s;&,"\'\)\}\]\>]+'
    r')'
 )

@@ -120,7 +119,8 @@ def redact_urls(text: str) -> str:
    if not isinstance(text, str) or not text:
        return text

-    url_pattern = re.compile(r'(https?://[^\s)>\]}]+)', re.IGNORECASE)
+    # Match any URI scheme (http, https, postgres, mysql, mongodb, redis, etc.)
+    url_pattern = re.compile(r'([a-z0-9\+\.\-]+://[^\s)>\]}]+)', re.IGNORECASE)

    def replace_url(match):
        url_str = match.group(1)
@@ -134,11 +134,11 @@ def redact_urls(text: str) -> str:
                    is_synthetic = True
                    break

-            if is_synthetic:
-                # Rebuild synthetic URL to redact any credentials or query secrets
+            if is_synthetic or (parsed.username or parsed.password) or parsed.scheme.lower() not in ("http", "https"):
+                # Rebuild URL to redact any credentials or query secrets
                new_netloc = parsed.netloc
                if parsed.username or parsed.password:
-                    netloc_clean = parsed.hostname
+                    netloc_clean = parsed.hostname or ""
                    if parsed.port:
                        netloc_clean = f"{netloc_clean}:{parsed.port}"
                    new_netloc = f"[REDACTED_USER]:[REDACTED_PASS]@{netloc_clean}"
@@ -177,11 +177,17 @@ def redact_urls(text: str) -> str:

 def _mask_assignment(match: re.Match) -> str:
    """Keep the key and separator; replace only the secret value."""
+    val = match.group(3)
+    if val.startswith(REDACTED) or val.startswith("%5BREDACTED") or val.startswith("[REDACTED"):
+        return f"{match.group(1)}{match.group(2)}{val}"
    return f"{match.group(1)}{match.group(2)}{REDACTED}"


 def _mask_conn_secret(match: re.Match) -> str:
    """Keep the connection-string key; replace only the credential value."""
+    val = match.group(2)
+    if val.startswith(REDACTED) or val.startswith("%5BREDACTED") or val.startswith("[REDACTED"):
+        return f"{match.group(1)}{val}"
    return f"{match.group(1)}{REDACTED}"


@@ -195,7 +201,8 @@ def _redact_str(text):
    """
    if not isinstance(text, str) or not text:
        return text
-    out = _BARE_SECRET_PATTERN.sub(REDACTED, text)
+    out = redact_urls(text)
+    out = _BARE_SECRET_PATTERN.sub(REDACTED, out)
    out = _ASSIGNMENT_SECRET_PATTERN.sub(_mask_assignment, out)
    out = _CONN_STRING_SECRET_PATTERN.sub(_mask_conn_secret, out)
    out_lower = out.lower()
@@ -212,7 +219,7 @@ def _redact_str(text):
            out = out[:i] + prefix + REDACTED + out[j:]
            out_lower = out.lower()
            idx = i + len(prefix) + len(REDACTED)
-    return redact_urls(out)
+    return out


 def redact(value):
@@ -23936,37 +23936,35 @@ _break_glass_restart_executor = None
 def _default_break_glass_restart_executor(request: dict) -> dict:
    """Canonical non-dry-run break-glass executor/delegate (#664 B6/B11).

-    Never kills, signals, or restarts the live MCP cohort in-process. When a
-    sanctioned host-managed restart hook reference is configured
-    (``GITEA_SANCTIONED_RESTART_HOOK``), the request is *delegated* to that
-    host supervisor and the contract reports proven execution only for the
-    accepted handoff. Without a configured hook, apply is unsupported.
+    Never kills, signals, or restarts the live MCP cohort in-process.
+    An environment string alone (``GITEA_SANCTIONED_RESTART_HOOK``) does not
+    prove restart execution without an active confirmed delegate handoff.
    """
    hook = (os.environ.get("GITEA_SANCTIONED_RESTART_HOOK") or "").strip()
-    if not hook:
+    if hook:
        return {
            "success": False,
-            "apply_supported": False,
-            "apply_authorized": False,
+            "apply_supported": True,
+            "apply_authorized": True,
            "restart_performed": False,
            "break_glass_executed": False,
-            "execution_mode": "unsupported",
+            "execution_mode": "accepted_not_executed",
+            "host_hook_configured": True,
            "reasons": [
-                "break-glass apply is unsupported: no sanctioned host restart "
-                "hook is configured (GITEA_SANCTIONED_RESTART_HOOK) (#664)"
+                "sanctioned host restart hook reference is configured, but environment text "
+                "cannot prove execution without a confirmed delegate handoff (fail closed) (#664 B6/B11)"
            ],
        }
-    # Opaque host reference only — never treat the hook string as a command.
    return {
-        "success": True,
-        "apply_supported": True,
-        "apply_authorized": True,
-        "restart_performed": True,
-        "break_glass_executed": True,
-        "execution_mode": "host_delegate_accepted",
-        "host_hook_configured": True,
+        "success": False,
+        "apply_supported": False,
+        "apply_authorized": False,
+        "restart_performed": False,
+        "break_glass_executed": False,
+        "execution_mode": "unsupported",
        "reasons": [
-            "break-glass restart delegated to sanctioned host supervisor (#664)"
+            "break-glass apply is unsupported: no sanctioned host restart "
+            "executor is configured (#664)"
        ],
    }

@@ -855,6 +855,128 @@ class TestBreakGlassRestart(unittest.TestCase):
        entry2 = TASK_CAPABILITY_MAP.get("break_glass_restart")
        self.assertEqual(entry2["permission"], "runtime.break_glass_restart")

+    # ── B8 / B6 / B15 remediation tests ─────────────────────────────────────
+
+    def test_b8_redaction_gitea_token_and_uri_credentials(self) -> None:
+        """B8: GITEA_TOKEN= and URI userinfo credentials redacted without erasing neighbours."""
+        # GITEA_TOKEN= with underscore key
+        out1 = gitea_audit._redact_str("failed with GITEA_TOKEN=synthetic_tok_123456789")
+        self.assertIn("GITEA_TOKEN=[REDACTED]", out1)
+        self.assertNotIn("synthetic_tok_123456789", out1)
+
+        # Connection string with URI userinfo
+        out2 = gitea_audit._redact_str("conn postgres://user:[email protected]:5432/app")
+        self.assertIn("postgres://[REDACTED_USER]:[REDACTED_PASS]@db.internal:5432/app", out2)
+        self.assertNotIn("s3cr3tpw", out2)
+
+        # Value boundary preserving adjacent audit evidence (correlation_id, incident_number)
+        raw_audit = "password=secret123;correlation_id=bg-7f2a1c;incident_number=4242"
+        out3 = gitea_audit._redact_str(raw_audit)
+        self.assertIn("password=[REDACTED]", out3)
+        self.assertIn("correlation_id=bg-7f2a1c", out3)
+        self.assertIn("incident_number=4242", out3)
+        self.assertNotIn("secret123", out3)
+
+        # Query param boundary in URL preserving adjacent parameters
+        raw_url = "token=abc-123&pr=908&issue=664&head=c67f39b4"
+        out4 = gitea_audit._redact_str(raw_url)
+        self.assertIn("token=[REDACTED]", out4)
+        self.assertIn("pr=908", out4)
+        self.assertIn("issue=664", out4)
+        self.assertIn("head=c67f39b4", out4)
+
+    def test_b6_default_executor_environment_text_cannot_imply_execution(self) -> None:
+        """B6/B11: GITEA_SANCTIONED_RESTART_HOOK string alone returns break_glass_executed=False."""
+        os.environ["GITEA_SANCTIONED_RESTART_HOOK"] = "this-string-is-never-invoked"
+        prof = _controller_profile()
+        p_parity, p_runtime, p_switch = _gate_open_patches()
+        with p_parity, p_runtime, p_switch:
+            with patch.object(gitea_mcp_server, "get_profile", return_value=prof), patch.object(
+                gitea_mcp_server, "_auth", return_value={"Authorization": "token test"}
+            ), patch.object(
+                gitea_mcp_server, "_authenticated_username", return_value="sysadmin"
+            ), patch.object(
+                gitea_mcp_server, "gitea_request_mcp_restart",
+                return_value={"affected_sessions": []},
+            ), patch.object(
+                gitea_audit, "audit_enabled", return_value=True
+            ), patch.object(
+                gitea_audit, "write_event", return_value=True
+            ), patch.object(
+                gitea_mcp_server, "api_request",
+                return_value={"number": 555, "title": "[INCIDENT]"},
+            ):
+                res = gitea_mcp_server.gitea_break_glass_restart(
+                    reason="Privileged restart request with non-empty hook env var",
+                    confirmation="I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION",
+                    impact_ack=True,
+                    dry_run=False,
+                    create_incident_issue=True,
+                    remote="prgs",
+                )
+        self.assertFalse(res["success"])
+        self.assertFalse(res["performed"])
+        self.assertFalse(res["break_glass_executed"])
+        self.assertEqual(res["blocker_kind"], "restart_delegation_failed")
+
+    def test_b15_production_prgs_controller_grant_set_and_gates(self) -> None:
+        """B15: Genuine prgs-controller carrying runtime.break_glass_restart and gitea.issue.create passes real gates."""
+        # Full production-shaped prgs-controller profile
+        prod_profile = {
+            "profile_name": "prgs-controller",
+            "execution_profile": "prgs-controller",
+            "role": "reconciler",
+            "allowed_operations": [
+                "gitea.read",
+                "gitea.pr.close",
+                "gitea.pr.comment",
+                "gitea.issue.comment",
+                "gitea.issue.create",
+                "runtime.break_glass_restart",
+                "gitea.branch.delete",
+            ],
+            "forbidden_operations": [
+                "gitea.pr.approve",
+                "gitea.pr.merge",
+                "gitea.pr.create",
+                "gitea.branch.push",
+            ],
+        }
+
+        # 1. Real _profile_operation_gate checks
+        p_parity, p_runtime, p_switch = _gate_open_patches()
+        with p_parity, p_runtime, p_switch:
+            with patch.object(gitea_mcp_server, "get_profile", return_value=prod_profile):
+                # Both required operations pass the real operation gate (no stubs)
+                self.assertEqual(
+                    gitea_mcp_server._profile_operation_gate("runtime.break_glass_restart"),
+                    [],
+                )
+                self.assertEqual(
+                    gitea_mcp_server._profile_operation_gate("gitea.issue.create"),
+                    [],
+                )
+
+        # 2. Missing gitea.issue.create fails incident creation gate
+        no_issue_create = dict(prod_profile)
+        no_issue_create["allowed_operations"] = [
+            "gitea.read", "gitea.pr.close", "runtime.break_glass_restart"
+        ]
+        with p_parity, p_runtime, p_switch:
+            with patch.object(gitea_mcp_server, "get_profile", return_value=no_issue_create):
+                res = gitea_mcp_server.gitea_break_glass_restart(
+                    reason="Restart testing missing gitea.issue.create permission",
+                    confirmation="I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION",
+                    impact_ack=True,
+                    dry_run=False,
+                    create_incident_issue=True,
+                    remote="prgs",
+                )
+        self.assertFalse(res["success"])
+        self.assertEqual(res["blocker_kind"], "permission_denied")
+        self.assertIn("gitea.issue.create", " ".join(res.get("reasons") or []))
+

 if __name__ == "__main__":
    unittest.main()
+