fix(#664): remediate PR #908 review #641 blockers B13, B1, B14, B6/B11, and B8

Register runtime.break_glass_restart in the multi-service operation normalizer
and enforce it through the real profile gate. Authorize only the exact trusted
prgs-controller profile plus that capability; remove substring controller
authority so declared reconciler roles and cleanup_merged_pr_branch semantics
are preserved. Route non-dry-run apply through a canonical injectable executor
delegate with truthful execution flags. Correct under/over-redaction for
credentials while preserving benign sec- text.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
2026-07-28 23:33:25 -04:00
co-authored by Claude Opus 4.8
parent 4463a300ba
commit c67f39b40e
6 changed files with 1030 additions and 538 deletions
+1 -1
View File
@@ -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: The dedicated MCP tool `gitea_break_glass_restart` provides the privileged emergency break-glass restart workflow when graceful drain cannot complete:
- **Role Authorization (#664 AC1)**: Ordinary LLM worker roles (`author`, `reviewer`, `merger`, `reconciler`) are denied fail-closed. Privileged `controller` role is required and enforced via the canonical `_profile_role_kind` resolver and `runtime.break_glass_restart` capability. `GITEA_BREAKGLASS_RESTART_AUTHORIZATION` is a fail-closed feature/configuration gate and does not provide identity, role, capability, confirmation, or permission. - **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.
- **Required Parameters (#664 AC2)**: - **Required Parameters (#664 AC2)**:
- `reason`: Mandatory non-empty string (min 10 characters). - `reason`: Mandatory non-empty string (min 10 characters).
- `confirmation`: Must equal exactly `"I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION"`. - `confirmation`: Must equal exactly `"I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION"`.
+64 -4
View File
@@ -34,10 +34,34 @@ PENDING = "pending"
REDACTED = "[REDACTED]" REDACTED = "[REDACTED]"
# A dict key containing any of these (case-insensitive) has its value redacted. # A dict key containing any of these (case-insensitive) has its value redacted.
_SECRET_KEY_HINTS = ("token", "password", "secret", "authorization", "auth") _SECRET_KEY_HINTS = (
"token",
"password",
"passwd",
"pwd",
"secret",
"authorization",
"auth",
"api_key",
"apikey",
"access_key",
"private_key",
"client_secret",
"credential",
)
# A string value starting with one of these has the following run redacted. # A string value starting with one of these has the following run redacted.
_SECRET_VALUE_PREFIXES = ("token ", "Basic ", "Bearer ", "token:", "bearer:", "basic:") # Space-terminated scheme prefixes only. Colon forms (``token:`` / ``api_key:``)
# and ``Authorization: Bearer …`` are handled by ``_ASSIGNMENT_SECRET_PATTERN``
# so policy/docs text that merely *names* a scheme is not itself flagged as a
# live secret by console detectors.
_SECRET_VALUE_PREFIXES = (
"token ",
"Basic ",
"Bearer ",
)
# Bare token-shaped values only — never a broad ``sec-`` prefix that erases
# ordinary words (#664 B8 over-redaction).
_BARE_SECRET_PATTERN = re.compile( _BARE_SECRET_PATTERN = re.compile(
r'(?i)\b(?:' r'(?i)\b(?:'
r'ghp_[A-Za-z0-9_]{16,}' r'ghp_[A-Za-z0-9_]{16,}'
@@ -49,10 +73,28 @@ _BARE_SECRET_PATTERN = re.compile(
r'|sk-proj-[A-Za-z0-9_-]{16,}' r'|sk-proj-[A-Za-z0-9_-]{16,}'
r'|sk-[A-Za-z0-9_-]{20,}' r'|sk-[A-Za-z0-9_-]{20,}'
r'|glpat-[A-Za-z0-9_-]{16,}' r'|glpat-[A-Za-z0-9_-]{16,}'
r'|sec-[A-Za-z0-9_-]{16,}'
r')\b' 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.
_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'"[^"]*"|\'[^\']*\'|'
r'(?:Bearer|Basic|Token)\s+\S+|'
r'\S+'
r')'
)
# Connection-string style credentials: Password=...; User ID=...; etc.
_CONN_STRING_SECRET_PATTERN = re.compile(
r'(?i)\b((?:password|pwd|user\s*id|uid|username|account)\s*=\s*)([^\s;\'"]+)'
)
# Known synthetic test-only domains/hostnames to preserve # Known synthetic test-only domains/hostnames to preserve
_SYNTHETIC_HOSTS = { _SYNTHETIC_HOSTS = {
@@ -133,11 +175,29 @@ def redact_urls(text: str) -> str:
return out return out
def _mask_assignment(match: re.Match) -> str:
"""Keep the key and separator; replace only the secret value."""
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."""
return f"{match.group(1)}{REDACTED}"
def _redact_str(text): def _redact_str(text):
"""Redact anything that looks like an Authorization credential, bare secret token, or raw URL in *text*.""" """Redact credentials, bare token shapes, and raw URLs in *text* (#664 B8).
Covers key/value credentials, authorization/bearer material, bare
token-shaped values, connection-string credentials, and secrets embedded
in larger sentences. Deliberately does **not** erase ordinary words that
merely begin with a broad ``sec-`` prefix.
"""
if not isinstance(text, str) or not text: if not isinstance(text, str) or not text:
return text return text
out = _BARE_SECRET_PATTERN.sub(REDACTED, text) out = _BARE_SECRET_PATTERN.sub(REDACTED, text)
out = _ASSIGNMENT_SECRET_PATTERN.sub(_mask_assignment, out)
out = _CONN_STRING_SECRET_PATTERN.sub(_mask_conn_secret, out)
out_lower = out.lower() out_lower = out.lower()
for prefix in _SECRET_VALUE_PREFIXES: for prefix in _SECRET_VALUE_PREFIXES:
prefix_lower = prefix.lower() prefix_lower = prefix.lower()
+32 -3
View File
@@ -97,6 +97,26 @@ GITEA_OPERATION_ALIASES = {
_REVIEW_MERGE_OPS = frozenset({"gitea.pr.approve", "gitea.pr.merge"}) _REVIEW_MERGE_OPS = frozenset({"gitea.pr.approve", "gitea.pr.merge"})
_AUTHOR_ONLY_OPS = frozenset({"gitea.pr.create", "gitea.branch.push"}) _AUTHOR_ONLY_OPS = frozenset({"gitea.pr.create", "gitea.branch.push"})
# First-class operation services that may appear in multi-service profile
# allowlists. ``runtime.*`` is the control-plane capability namespace used by
# non-Gitea MCP tools such as ``runtime.break_glass_restart`` (#664 B13).
# Unknown foreign prefixes (e.g. ``jenkins.*`` under service=gitea) still fail
# closed — they are not registered here.
KNOWN_OPERATION_SERVICES = frozenset({"gitea", "runtime"})
def service_for_operation(op, default="gitea"):
"""Return the registered service prefix for a fully-qualified *op*.
Unqualified names and unknown prefixes fall back to *default* so callers
keep the historical Gitea-centric gate behaviour.
"""
if isinstance(op, str) and "." in op:
prefix = op.split(".", 1)[0]
if prefix in KNOWN_OPERATION_SERVICES:
return prefix
return default
def normalize_operation(op, service="gitea"): def normalize_operation(op, service="gitea"):
"""Return the canonical namespaced name for *op*, or fail closed (#106). """Return the canonical namespaced name for *op*, or fail closed (#106).
@@ -133,6 +153,12 @@ def check_operation(op, allowed, forbidden=(), service="gitea"):
Reasons: ``allowed``, ``invalid-operation``, ``invalid-forbidden-entry``, Reasons: ``allowed``, ``invalid-operation``, ``invalid-forbidden-entry``,
``forbidden``, ``no-allowed-operations``, ``not-allowed``. ``forbidden``, ``no-allowed-operations``, ``not-allowed``.
Multi-service profile allowlists (#664 B13): each allow/forbid entry is
normalized with its own registered service prefix (``gitea.*`` or
``runtime.*``) so a gate defaulting to service=gitea can still enforce an
exact ``runtime.break_glass_restart`` grant. Unknown / misspelled
operations and foreign service prefixes remain fail-closed.
Fail-closed rules: Fail-closed rules:
- an *op* that cannot be normalized is denied (``invalid-operation``) - an *op* that cannot be normalized is denied (``invalid-operation``)
- a forbidden entry that cannot be normalized denies the request - a forbidden entry that cannot be normalized denies the request
@@ -143,14 +169,16 @@ def check_operation(op, allowed, forbidden=(), service="gitea"):
- ``forbidden`` always overrides ``allowed`` - ``forbidden`` always overrides ``allowed``
- an empty or missing allowed list denies everything - an empty or missing allowed list denies everything
""" """
op_service = service_for_operation(op, default=service)
try: try:
op_n = normalize_operation(op, service) op_n = normalize_operation(op, op_service)
except ConfigError: except ConfigError:
return (False, "invalid-operation") return (False, "invalid-operation")
forbidden_n = set() forbidden_n = set()
for entry in (forbidden or ()): for entry in (forbidden or ()):
try: try:
forbidden_n.add(normalize_operation(entry, service)) entry_service = service_for_operation(entry, default=service)
forbidden_n.add(normalize_operation(entry, entry_service))
except ConfigError: except ConfigError:
return (False, "invalid-forbidden-entry") return (False, "invalid-forbidden-entry")
if op_n in forbidden_n: if op_n in forbidden_n:
@@ -160,7 +188,8 @@ def check_operation(op, allowed, forbidden=(), service="gitea"):
allowed_n = set() allowed_n = set()
for entry in allowed: for entry in allowed:
try: try:
allowed_n.add(normalize_operation(entry, service)) entry_service = service_for_operation(entry, default=service)
allowed_n.add(normalize_operation(entry, entry_service))
except ConfigError: except ConfigError:
continue continue
if op_n in allowed_n: if op_n in allowed_n:
+145 -59
View File
@@ -233,27 +233,50 @@ def _effective_workspace_role() -> str:
) )
# Exact production profile → role mapping used only when no declared role is
# present. Substring lookalikes (fake-controller, not-controller, …) never
# match (#664 B1/B14).
_EXACT_PROFILE_ROLE_NAMES = {
"prgs-controller": "controller",
"prgs-reconciler": "reconciler",
"prgs-author": "author",
"prgs-reviewer": "reviewer",
"prgs-merger": "merger",
"mdcps-author": "author",
"mdcps-reviewer": "reviewer",
"mdcps-merger": "merger",
"controller": "controller",
"reconciler": "reconciler",
"author": "author",
"reviewer": "reviewer",
"merger": "merger",
}
# Exact trusted profile that may hold break-glass (#664 B1). Capability
# ``runtime.break_glass_restart`` is still required and enforced by the real
# operation gate; this set only rejects lookalike profile names.
TRUSTED_BREAK_GLASS_PROFILES = frozenset({"prgs-controller"})
def _profile_role_kind(profile: dict) -> str: def _profile_role_kind(profile: dict) -> str:
"""Resolve a profile's declared role before inferring from permissions. """Resolve a profile's declared role before inferring from permissions.
Declared ``role`` / ``role_kind`` or profile_name containing 'controller' Declared ``role`` / ``role_kind`` always wins so a controller profile is
always resolves to 'controller' (#840/#664). never reclassified as reconciler from permission inference (#840). Exact
match only profile-name / role *substrings* never grant controller (or
any) authority (#664 B1/B14). Lookalikes such as ``fake-controller``,
``not-controller``, or ``xcontrollerx`` do not become controller.
""" """
profile_name = (profile.get("profile_name") or "").strip().lower()
role = (profile.get("role") or profile.get("role_kind") or "").strip().lower() role = (profile.get("role") or profile.get("role_kind") or "").strip().lower()
if "controller" in profile_name or "control" in role or role == "controller":
return "controller"
if role: if role:
# Exact aliases only; never ``"control" in role`` (matches
# "not-controller" / "control-plane").
if role in ("controller", "control"):
return "controller"
return role return role
for candidate in ( profile_name = (profile.get("profile_name") or "").strip().lower()
"controller", if profile_name in _EXACT_PROFILE_ROLE_NAMES:
"reconciler", return _EXACT_PROFILE_ROLE_NAMES[profile_name]
"merger",
"reviewer",
"author",
):
if candidate in profile_name:
return candidate
return _role_kind( return _role_kind(
profile.get("allowed_operations") or [], profile.get("allowed_operations") or [],
profile.get("forbidden_operations") or [], profile.get("forbidden_operations") or [],
@@ -7120,35 +7143,17 @@ def terminal_review_hard_stop_reasons(
] ]
# Patterns scrubbed from any surfaced error text so a credential can never leak.
_SECRET_PREFIXES = ("token ", "Basic ")
def _redact(text: str) -> str: def _redact(text: str) -> str:
"""Strip anything that looks like an Authorization credential or raw URL from *text*. """Strip credentials, bare token shapes, and raw URLs from *text* (#664 B8).
Errors raised by ``api_request`` echo the server response body, not the Defers to the canonical :mod:`gitea_audit` redactor so MCP tool results,
request headers, so a token should never appear this is defence in depth incidents, audits, and delegated error surfaces share one boundary.
so a future change can't leak ``token …`` / ``Basic …`` material into a
tool result or log line.
""" """
if not text: if not text:
return text return text
out = text
for prefix in _SECRET_PREFIXES:
idx = 0
while True:
i = out.find(prefix, idx)
if i == -1:
break
j = i + len(prefix)
while j < len(out) and not out[j].isspace():
j += 1
out = out[:i] + prefix + "[REDACTED]" + out[j:]
idx = i + len(prefix) + len("[REDACTED]")
# Redact raw URLs, query secrets, hostnames, etc.
import gitea_audit import gitea_audit
return gitea_audit.redact_urls(out) redacted = gitea_audit._redact_str(str(text))
return redacted if isinstance(redacted, str) else str(redacted)
# Review states that carry a submitted verdict. Gitea also emits PENDING # Review states that carry a submitted verdict. Gitea also emits PENDING
@@ -23922,7 +23927,64 @@ def gitea_request_mcp_restart(
BREAK_GLASS_CONFIRMATION_PHRASE = "I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION" BREAK_GLASS_CONFIRMATION_PHRASE = "I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION"
PRIVILEGED_BREAK_GLASS_ROLES = frozenset({"controller"})
# Test-injectable canonical break-glass executor/delegate (#664 B6/B11).
# Production default never signals the live MCP cohort.
_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.
"""
hook = (os.environ.get("GITEA_SANCTIONED_RESTART_HOOK") or "").strip()
if not hook:
return {
"success": False,
"apply_supported": False,
"apply_authorized": False,
"restart_performed": False,
"break_glass_executed": False,
"execution_mode": "unsupported",
"reasons": [
"break-glass apply is unsupported: no sanctioned host restart "
"hook is configured (GITEA_SANCTIONED_RESTART_HOOK) (#664)"
],
}
# 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,
"reasons": [
"break-glass restart delegated to sanctioned host supervisor (#664)"
],
}
def _run_break_glass_restart_executor(request: dict) -> dict:
"""Invoke the installed or default break-glass executor (test-injectable)."""
executor = _break_glass_restart_executor or _default_break_glass_restart_executor
result = executor(request)
if not isinstance(result, dict):
return {
"success": False,
"apply_supported": True,
"apply_authorized": False,
"restart_performed": False,
"break_glass_executed": False,
"reasons": ["break-glass executor returned a non-dict result (fail closed)"],
}
return result
@mcp.tool() @mcp.tool()
@@ -23943,22 +24005,26 @@ def gitea_break_glass_restart(
Break-glass restart permits emergency recovery when graceful drain cannot Break-glass restart permits emergency recovery when graceful drain cannot
complete. It requires: complete. It requires:
1. Privileged caller authorization (explicit allowlist: controller; 1. Exact ``runtime.break_glass_restart`` capability on the trusted
ordinary roles and unknown/malformed roles fail closed). ``prgs-controller`` profile (ordinary roles, lookalike names, env vars,
and non-controller reconcilers fail closed).
2. Explicit non-empty reason (minimum 10 characters). 2. Explicit non-empty reason (minimum 10 characters).
3. Exact confirmation string matching 'I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION'. 3. Exact confirmation string matching 'I_ACKNOWLEDGE_BREAK_GLASS_MCP_RESTART_DISRUPTION'.
4. Mandatory impact acknowledgement (impact_ack=True). 4. Mandatory impact acknowledgement (impact_ack=True).
5. Immutable append-only audit entry recorded prior to execution and after terminal completion. 5. Immutable append-only audit entry recorded prior to execution and after terminal completion.
6. Automatic incident record created on Gitea prior to execution. 6. Automatic incident record created on Gitea prior to execution.
7. Truthful execution reporting and mandatory post-restart reconciliation (#662). 7. Truthful execution reporting via the canonical executor/delegate and
mandatory post-restart reconciliation (#662).
""" """
read_block = _profile_operation_gate("runtime.break_glass_restart") # B13: real production gate for the exact canonical operation — never
if read_block: # fall back to gitea.read and never stub this gate in production.
capability_block = _profile_operation_gate("runtime.break_glass_restart")
if capability_block:
return gitea_audit.redact({ return gitea_audit.redact({
"success": False, "success": False,
"performed": False, "performed": False,
"break_glass_executed": False, "break_glass_executed": False,
"reasons": read_block, "reasons": capability_block,
"permission_report": _permission_block_report("runtime.break_glass_restart"), "permission_report": _permission_block_report("runtime.break_glass_restart"),
"blocker_kind": "permission_denied", "blocker_kind": "permission_denied",
}) })
@@ -23966,20 +24032,29 @@ def gitea_break_glass_restart(
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
profile = get_profile() profile = get_profile()
active_role = _profile_role_kind(profile) active_role = _profile_role_kind(profile)
profile_name = (
(profile.get("profile_name") or profile.get("execution_profile") or "")
.strip()
)
break_glass_env_auth = bool( break_glass_env_auth = bool(
(os.environ.get("GITEA_BREAKGLASS_RESTART_AUTHORIZATION") or "").strip() (os.environ.get("GITEA_BREAKGLASS_RESTART_AUTHORIZATION") or "").strip()
) )
# Env is never an authorization channel for this endpoint (B2/B1).
_ = break_glass_env_auth
# 1. Privileged role authorization (explicit allowlist: controller only - B1) # B1: exact trusted profile only — not role substring, not lookalike names.
if active_role not in PRIVILEGED_BREAK_GLASS_ROLES: if profile_name not in TRUSTED_BREAK_GLASS_PROFILES:
return gitea_audit.redact({ return gitea_audit.redact({
"success": False, "success": False,
"performed": False, "performed": False,
"break_glass_executed": False, "break_glass_executed": False,
"active_role": active_role, "active_role": active_role,
"profile_name": profile_name or None,
"reasons": [ "reasons": [
f"role '{active_role}' is not in privileged break-glass allowlist " f"profile '{profile_name or '(unset)'}' is not the trusted "
f"({sorted(PRIVILEGED_BREAK_GLASS_ROLES)}); break-glass restart requires a privileged controller role (#664 AC1)" f"break-glass profile {sorted(TRUSTED_BREAK_GLASS_PROFILES)}; "
"lookalike names, ordinary roles, env vars, and non-controller "
"reconcilers cannot authorize break-glass (#664 AC1/B1)"
], ],
"blocker_kind": "role_authorization", "blocker_kind": "role_authorization",
}) })
@@ -24219,23 +24294,34 @@ def gitea_break_glass_restart(
incident_number = incident_issue_result.get("number") incident_number = incident_issue_result.get("number")
# Execute or delegate canonical non-dry-run restart (B6/B11) # B6/B11: reach the canonical non-dry-run executor/delegate.
restart_exec_result = gitea_request_mcp_restart( # Authorization and apply_authorized do not mean execution occurred.
remote=remote, # Dry-run never reaches this path (returned earlier).
host=host, restart_exec_result = _run_break_glass_restart_executor({
org=org, "remote": remote,
repo=repo, "host": host,
dry_run=False, "org": o,
restart_class=restart_class, "repo": r,
request_break_glass=True, "restart_class": restart_class,
) "correlation_id": correlation_id,
"reason": clean_reason,
"confirmation": clean_confirmation,
"profile_name": profile_name,
"active_role": active_role,
"incident_number": incident_number,
"disrupted_sessions_count": disrupted_count,
"request_break_glass": True,
"dry_run": False,
})
apply_supported = bool(restart_exec_result.get("apply_supported", False)) apply_supported = bool(restart_exec_result.get("apply_supported", False))
apply_authorized = bool(restart_exec_result.get("apply_authorized", False)) apply_authorized = bool(restart_exec_result.get("apply_authorized", False))
exec_success = bool(restart_exec_result.get("success", False)) exec_success = bool(restart_exec_result.get("success", False))
# break_glass_executed is true only when the executor contract proves
# execution (or accepted host delegation) occurred.
restart_performed = bool( restart_performed = bool(
restart_exec_result.get("restart_performed", False) restart_exec_result.get("restart_performed", False)
or restart_exec_result.get("break_glass_executed", False) and restart_exec_result.get("break_glass_executed", False)
) )
if not apply_supported: if not apply_supported:
+8 -3
View File
@@ -37,12 +37,17 @@ def normalize_role_kind(
*, *,
profile_name: str | None = None, profile_name: str | None = None,
) -> str: ) -> str:
"""Map profile/task role to a workspace namespace key.""" """Map profile/task role to a workspace namespace key.
Exact profile-name matches only for controller routing (#840 / #664 B1):
substring lookalikes such as ``fake-controller`` must not become
controller.
"""
role = (role_kind or "author").strip().lower() role = (role_kind or "author").strip().lower()
profile = (profile_name or "").strip().lower() profile = (profile_name or "").strip().lower()
if role == "reviewer" and "merger" in profile: if role == "reviewer" and profile in ("prgs-merger", "mdcps-merger", "merger"):
return "merger" return "merger"
if "controller" in profile or role == "controller": if role == "controller" or profile in ("prgs-controller", "controller"):
return "controller" return "controller"
if role in ROLE_WORKTREE_ENVS: if role in ROLE_WORKTREE_ENVS:
return role return role
File diff suppressed because it is too large Load Diff