From 632c568865cab3de7a8d9970463ec182578d0a86 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 14 Jul 2026 23:05:41 -0400 Subject: [PATCH 1/5] fix(session): pin immutable MCP context; ban cross-host profile substitution (Closes #714) Capability resolution and mutation gates no longer auto-switch profiles. Session profile/remote/host/identity are bound after pin or first seed, and drift or cross-host resolution fails closed before writes. Co-Authored-By: Grok --- gitea_mcp_server.py | 490 +++++++++++++----- session_context_binding.py | 358 +++++++++++++ ..._issue_714_session_context_immutability.py | 390 ++++++++++++++ tests/test_operation_scoped_roles.py | 91 ++-- 4 files changed, 1164 insertions(+), 165 deletions(-) create mode 100644 session_context_binding.py create mode 100644 tests/test_issue_714_session_context_immutability.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index c433a1d..24a01b7 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -1118,6 +1118,34 @@ import workflow_scope_guard # noqa: E402 # #683 production scope / force-on gu import stable_branch_push_guard # noqa: E402 import remote_repo_guard # noqa: E402 import issue_claim_heartbeat # noqa: E402 +import session_context_binding as session_ctx # noqa: E402 # #714 immutable session context + + +def _seed_session_context( + *, + profile: dict, + remote: str | None, + host: str | None, + identity: str | None, + repository: str | None = None, + org: str | None = None, + source: str = "seed", +) -> dict: + """Seed/rebind immutable session context with env/override awareness (#714).""" + expected = (profile.get("username") or "").strip() or None + return session_ctx.seed_session_context_if_unbound( + profile_name=profile.get("profile_name") or "", + remote=remote, + host=host, + identity=identity, + repository=repository, + org=org, + role_kind=_profile_role_kind(profile), + expected_username=expected, + source=source, + active_profile_override=gitea_config._active_profile_override, + env_profile=(os.environ.get("GITEA_MCP_PROFILE") or "").strip() or None, + ) import issue_work_duplicate_gate # noqa: E402 import issue_workflow_labels # noqa: E402 import reviewer_pr_lease # noqa: E402 @@ -1846,61 +1874,44 @@ def _authenticated_username(host: str): return user -def _ensure_matching_profile(required_permission: str, required_role: str, remote: str | None, host: str | None = None) -> str | None: - """Check if the active profile is allowed to perform *required_permission*. - If not, automatically switch to the first matching usable configured profile. +def _ensure_matching_profile( + required_permission: str, + required_role: str, + remote: str | None, + host: str | None = None, +) -> str | None: + """Return the active profile name only when it is allowed for *permission*. + + #714: never silently switch profiles (including cross-host substitution). + Capability resolution and mutation gates evaluate only the active profile + for the requested remote. Explicit ``gitea_activate_profile`` is the sole + in-process switch path. """ + del required_role, host # kept for call-site compatibility try: profile = get_profile() except Exception: return None active_profile = profile.get("profile_name") + # Cross-host denial: mdcps profile cannot serve a prgs remote (and vice versa). + config = None + try: + config = gitea_config.load_config() + except Exception: + config = None + contexts = (config or {}).get("contexts") if config else None + remote_ok = session_ctx.profile_allowed_for_remote( + profile, remote, REMOTES, contexts=contexts + ) + if remote_ok.get("block"): + return None active_allowed = profile.get("allowed_operations") or [] active_forbidden = profile.get("forbidden_operations") or [] - allowed, _ = gitea_config.check_operation(required_permission, active_allowed, active_forbidden) + allowed, _ = gitea_config.check_operation( + required_permission, active_allowed, active_forbidden + ) if allowed: return active_profile - - # Try to find a matching usable profile in config - if gitea_config.is_runtime_switching_enabled(): - config = gitea_config.load_config() - if config and "profiles" in config: - for p_name, p_data in config["profiles"].items(): - p_allowed = p_data.get("allowed_operations") or [] - p_forbidden = p_data.get("forbidden_operations") or [] - p_allowed_n = [] - for op in p_allowed: - try: - p_allowed_n.append(gitea_config.normalize_operation(op)) - except Exception: - pass - p_forbidden_n = [] - for op in p_forbidden: - try: - p_forbidden_n.append(gitea_config.normalize_operation(op)) - except Exception: - pass - ok, _ = gitea_config.check_operation(required_permission, p_allowed_n, p_forbidden_n) - if ok: - # Verify credentials/token are available - try: - tok = gitea_config.resolve_token(p_data) - if tok: - # Perform automatic switch - gitea_config._active_profile_override = p_name - h = host or (REMOTES.get(remote, {}).get("host") if remote in REMOTES else None) - if h: - _IDENTITY_CACHE.pop(h, None) - username = _authenticated_username(h) if h else None - # Update mutation authority - global _MUTATION_AUTHORITY - if _MUTATION_AUTHORITY is not None: - _MUTATION_AUTHORITY["current_profile"] = p_name - _MUTATION_AUTHORITY["current_identity"] = username - _MUTATION_AUTHORITY["role_pivot_authorized"] = True - return p_name - except Exception: - pass return None @@ -1922,6 +1933,12 @@ def _audit(action: str, *, host, remote, result, org=None, repo=None, if mutation_task: ns_ctx = role_namespace_gate.mutation_audit_context( mutation_task, profile, remote=remote, repository=repo) + # #714: always record the bound session context at mutation time. + session_audit = session_ctx.mutation_context_audit_fields() + if isinstance(request_metadata, dict): + request_metadata = {**request_metadata, **session_audit} + elif request_metadata is None: + request_metadata = dict(session_audit) event = gitea_audit.build_event( action=action, result=result, @@ -7499,48 +7516,13 @@ def _role_for_operation(op: str) -> str | None: def _try_auto_switch_for_operation(op: str, host: str | None = None) -> bool: - """Try to find a profile in config that allows op and has valid credentials. + """#714: automatic profile substitution is removed (fail closed). - If found, switch to it, clear identity cache, and return True. - Otherwise return False. + Always returns False. Callers must use explicit ``gitea_activate_profile`` + to change roles; capability and mutation gates evaluate only the active + profile. Parameters retained for call-site compatibility. """ - role = _role_for_operation(op) - if not role: - return False - if not gitea_config.is_runtime_switching_enabled(): - return False - config = gitea_config.load_config() - if not config or "profiles" not in config: - return False - for p_name, p_data in config["profiles"].items(): - # Role classification matching - p_role = p_data.get("role") or _role_kind(p_data.get("allowed_operations", []), p_data.get("forbidden_operations", [])) - if p_role != role: - continue - p_allowed = p_data.get("allowed_operations") or [] - p_forbidden = p_data.get("forbidden_operations") or [] - p_allowed_n = [] - for op_val in p_allowed: - try: - p_allowed_n.append(gitea_config.normalize_operation(op_val)) - except Exception: - pass - p_forbidden_n = [] - for op_val in p_forbidden: - try: - p_forbidden_n.append(gitea_config.normalize_operation(op_val)) - except Exception: - pass - ok, _ = gitea_config.check_operation(op, p_allowed_n, p_forbidden_n) - if ok: - try: - tok = gitea_config.resolve_token(p_data) - if tok: - gitea_config._active_profile_override = p_name - _IDENTITY_CACHE.clear() - return True - except Exception: - pass + del op, host return False @@ -7607,6 +7589,102 @@ def _profile_operation_gate(op: str) -> list[str]: return [f"profile is not allowed to {op}"] +def _session_context_mutation_block( + *, + remote: str | None, + host: str | None = None, + org: str | None = None, + repo: str | None = None, + username: str | None = None, + **extra_fields, +) -> dict | None: + """#714: fail closed on session context / identity / cross-host drift.""" + try: + profile = get_profile() + except Exception as exc: + return { + "success": False, + "performed": False, + "reasons": [ + f"profile could not be resolved (fail closed): {_redact(str(exc))}" + ], + "blocker_kind": "session_context", + **extra_fields, + } + profile_name = profile.get("profile_name") + expected = (profile.get("username") or "").strip() or None + h = host or (REMOTES.get(remote or "", {}).get("host") if remote else None) + identity = username + if identity is None and h: + try: + identity = _authenticated_username(h) + except Exception: + identity = None + + config = None + try: + config = gitea_config.load_config() + except Exception: + config = None + contexts = (config or {}).get("contexts") if config else None + remote_gate = session_ctx.profile_allowed_for_remote( + profile, remote, REMOTES, contexts=contexts + ) + reasons: list[str] = list(remote_gate.get("reasons") or []) + + id_gate = session_ctx.assess_identity_match( + authenticated=identity, expected_username=expected + ) + reasons.extend(id_gate.get("reasons") or []) + + # Seed if unbound so subsequent tools share one context; then assess. + _seed_session_context( + profile=profile, + remote=remote, + host=h, + identity=identity, + repository=repo, + org=org, + source="mutation-gate-seed", + ) + ctx_gate = session_ctx.assess_session_context( + profile_name=profile_name, + remote=remote, + host=h, + identity=identity, + repository=repo, + org=org, + expected_username=expected, + require_bound=True, + ) + reasons.extend(ctx_gate.get("reasons") or []) + # De-dupe while preserving order + seen: set[str] = set() + uniq: list[str] = [] + for r in reasons: + if r not in seen: + seen.add(r) + uniq.append(r) + if not uniq: + return None + blocked = { + "success": False, + "performed": False, + "reasons": uniq, + "blocker_kind": "session_context", + "session_context_audit": session_ctx.mutation_context_audit_fields(), + "exact_next_action": ( + "BLOCKED + DIAGNOSE: session profile/remote/host/identity context " + "is inconsistent or unauthorized for this mutation. Re-pin the " + "correct profile with gitea_activate_profile, re-verify with " + "gitea_whoami for the intended remote, and do not use cross-host " + "fallback profiles." + ), + } + blocked.update(extra_fields) + return blocked + + def _profile_permission_block(required_operation: str, **extra_fields) -> dict | None: """Structured permission denial for gated tools (#69, #142). @@ -7616,8 +7694,21 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict | req_role = "reviewer" if any(required_operation.startswith(p) for p in ( "gitea.pr.approve", "gitea.pr.merge", "gitea.pr.request_changes", "gitea.pr.review" )) else "author" + # #714: evaluate active profile only — never auto-switch. _ensure_matching_profile(required_operation, req_role, extra_fields.get("remote")) + ctx_block = _session_context_mutation_block( + remote=extra_fields.get("remote"), + host=extra_fields.get("host"), + org=extra_fields.get("org"), + repo=extra_fields.get("repo"), + number=extra_fields.get("number"), + issue_number=extra_fields.get("issue_number"), + pr_number=extra_fields.get("pr_number"), + ) + if ctx_block is not None: + return ctx_block + reasons = _profile_operation_gate(required_operation) if not reasons: return None @@ -7626,6 +7717,7 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict | "performed": False, "reasons": reasons, "permission_report": _permission_block_report(required_operation), + "session_context_audit": session_ctx.mutation_context_audit_fields(), } blocked.update(extra_fields) return blocked @@ -7635,8 +7727,21 @@ def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None """Reviewer/author namespace alignment gate (#209).""" required_permission = task_capability_map.required_permission(mutation_task) required_role = task_capability_map.required_role(mutation_task) + # #714: evaluate active profile only — never auto-switch. _ensure_matching_profile(required_permission, required_role, extra_fields.get("remote")) + ctx_block = _session_context_mutation_block( + remote=extra_fields.get("remote"), + host=extra_fields.get("host"), + org=extra_fields.get("org"), + repo=extra_fields.get("repo"), + number=extra_fields.get("number"), + issue_number=extra_fields.get("issue_number"), + pr_number=extra_fields.get("pr_number"), + ) + if ctx_block is not None: + return ctx_block + try: profile = get_profile() except Exception as exc: @@ -9621,9 +9726,22 @@ def gitea_whoami( # name is the addressing surface; 'server' appears only under the # GITEA_MCP_REVEAL_ENDPOINTS admin opt-in. profile = get_profile() + identity = data.get("login") + expected_username = (profile.get("username") or "").strip() or None + # #714: seed immutable session context so later tools share one pin. + _seed_session_context( + profile=profile, + remote=remote, + host=h, + identity=identity, + source="gitea_whoami", + ) + id_match = session_ctx.assess_identity_match( + authenticated=identity, expected_username=expected_username + ) result = { "authenticated": True, - "username": data.get("login"), + "username": identity, "display_name": data.get("full_name") or None, "user_id": data.get("id"), "email": data.get("email") or None, @@ -9640,7 +9758,11 @@ def gitea_whoami( "execution_profile": profile.get("execution_profile"), "audit_label": profile.get("audit_label"), "auth_source_type": profile.get("auth_source_type"), + "expected_username": expected_username, }, + "session_context_audit": session_ctx.mutation_context_audit_fields(), + "identity_match": not id_match.get("block"), + "identity_match_reasons": id_match.get("reasons") or [], } if _reveal_endpoints(): result["server"] = gitea_url(h, "").rstrip("/") @@ -9766,14 +9888,32 @@ _RUNTIME_CAPABILITY_TASKS = ( def _matching_configured_profiles( config: dict | None, required_permission: str, + remote: str | None = None, ) -> list[str]: - """Profile names that allow *required_permission* (redacted metadata only).""" + """Profile names that allow *required_permission* (redacted metadata only). + + #714: when *remote* is provided, only profiles bound to that remote's host + are returned — a dadeschools request never lists prgs profiles. + """ if not config or "profiles" not in config: return [] + contexts = config.get("contexts") or {} + remote_ok_names: set[str] | None = None + if remote: + remote_ok_names = set( + session_ctx.filter_profiles_for_remote(config, remote, REMOTES) + ) matches: list[str] = [] for p_name, p_data in config["profiles"].items(): if not p_data.get("enabled", True): continue + if remote_ok_names is not None and p_name not in remote_ok_names: + continue + # Also require host/context match when remote given (defensive). + if remote and not session_ctx.profile_matches_remote( + p_data, remote, REMOTES, contexts=contexts + ): + continue p_allowed = p_data.get("allowed_operations") or [] p_forbidden = p_data.get("forbidden_operations") or [] p_allowed_n = [] @@ -9800,8 +9940,9 @@ def _build_runtime_task_capabilities( allowed: list[str], forbidden: list[str], config: dict | None, + remote: str | None = None, ) -> dict: - """Per-task capability summary for role-aware runtime context (#139).""" + """Per-task capability summary for role-aware runtime context (#139, #714).""" task_entries = [] flags: dict[str, bool] = {} flag_keys = { @@ -9825,7 +9966,7 @@ def _build_runtime_task_capabilities( "required_role_kind": task_capability_map.required_role(task), "allowed_in_current_session": allowed_here, "matching_configured_profiles": _matching_configured_profiles( - config, permission + config, permission, remote=remote ), } task_entries.append(entry) @@ -10277,8 +10418,9 @@ def gitea_get_runtime_context( "or ask the operator to update GITEA_MCP_PROFILE to a reviewer profile." ) + # #714: filter matching profiles by requested remote when building capability summary session_capabilities = _build_runtime_task_capabilities( - allowed, forbidden, config + allowed, forbidden, config, remote=remote if remote in REMOTES else None ) preflight = assess_preflight_status(worktree_path) @@ -10289,6 +10431,16 @@ def gitea_get_runtime_context( f"Blocked: {'; '.join(preflight['preflight_block_reasons'])}" ) + expected_username = (profile.get("username") or "").strip() or None + h_rt = host or (REMOTES.get(remote, {}).get("host") if remote in REMOTES else None) + _seed_session_context( + profile=profile, + remote=remote if remote in REMOTES else None, + host=h_rt, + identity=username, + source="gitea_get_runtime_context", + ) + result = { "active_profile": profile["profile_name"], "authenticated_username": username, @@ -10299,6 +10451,8 @@ def gitea_get_runtime_context( "forbidden_operations": forbidden, "runtime_switching_supported": switching, "profile_mode": profile_mode, + "session_context_audit": session_ctx.mutation_context_audit_fields(), + "auto_profile_substitution": False, "review_merge_allowed": review_merge_allowed, "review_merge_blocked_reasons": blocked_reasons, "suggested_fix": suggested_fix, @@ -10660,8 +10814,10 @@ def gitea_activate_profile( _IDENTITY_CACHE.pop(h, None) # 4. Resolve fresh identity - after_profile = get_profile()["profile_name"] + after_profile_data = get_profile() + after_profile = after_profile_data["profile_name"] after_identity = _authenticated_username(h) if h else None + expected_username = (after_profile_data.get("username") or "").strip() or None # 4.5 Record the authorized pivot in the in-process mutation authority # and keep the session profile lock in sync — this is the ONLY path that @@ -10676,15 +10832,32 @@ def gitea_activate_profile( "from_identity": before_identity, "to_identity": after_identity, } + _MUTATION_AUTHORITY["remote"] = remote if os.environ.get(SESSION_PROFILE_LOCK_ENV) and after_profile: os.environ[SESSION_PROFILE_LOCK_ENV] = after_profile + # 4.6 #714: re-bind immutable session context after explicit activation. + session_ctx.bind_session_context( + profile_name=after_profile or profile_name, + remote=remote, + host=h, + identity=after_identity, + role_kind=_profile_role_kind(after_profile_data), + expected_username=expected_username, + source="gitea_activate_profile", + ) + # 5. Audit the switch if auditing is on _audit( "activate_profile", host=h, remote=remote, - result={"success": True, "before": before_profile, "after": after_profile}, + result={ + "success": True, + "before": before_profile, + "after": after_profile, + "session_context": session_ctx.mutation_context_audit_fields(), + }, username=after_identity, ) @@ -10695,6 +10868,8 @@ def gitea_activate_profile( "before_identity": before_identity, "after_profile": after_profile, "after_identity": after_identity, + "session_context_audit": session_ctx.mutation_context_audit_fields(), + "auto_profile_substitution": False, } @@ -11795,21 +11970,44 @@ def gitea_resolve_task_capability( record_preflight_check("capability", required_role, resolved_task=task) - # Try automatic dispatch switching - _ensure_matching_profile(required_permission, required_role, remote, host) - + # #714: never auto-switch profiles during capability resolution. profile = get_profile() config = gitea_config.load_config() + contexts = (config or {}).get("contexts") if config else None h = host or (REMOTES.get(remote, {}).get("host") if remote in REMOTES else None) username = _authenticated_username(h) if h else None + expected_username = (profile.get("username") or "").strip() or None + + # Seed / re-check immutable session context for consistent multi-tool reads. + _seed_session_context( + profile=profile, + remote=remote, + host=h, + identity=username, + source="resolve_task_capability", + ) + ctx_assess = session_ctx.assess_session_context( + profile_name=profile.get("profile_name"), + remote=remote, + host=h, + identity=username, + expected_username=expected_username, + require_bound=False, + ) + id_assess = session_ctx.assess_identity_match( + authenticated=username, expected_username=expected_username + ) + remote_assess = session_ctx.profile_allowed_for_remote( + profile, remote, REMOTES, contexts=contexts + ) # Load active permissions active_allowed = profile.get("allowed_operations") or [] active_forbidden = profile.get("forbidden_operations") or [] active_role_kind = _profile_role_kind(profile) - # Check if allowed in current session + # Check if allowed in current session (active profile only — #714) permission_allowed_in_current_session, _ = gitea_config.check_operation( required_permission, active_allowed, active_forbidden ) @@ -11824,8 +12022,15 @@ def gitea_resolve_task_capability( f"{required_role} task '{task}' even if nearby permissions are " "present (fail closed)." ) + cross_host_block = bool(remote_assess.get("block")) + identity_block = bool(id_assess.get("block")) + drift_block = bool(ctx_assess.get("block")) allowed_in_current_session = ( - permission_allowed_in_current_session and role_matches_current_session + permission_allowed_in_current_session + and role_matches_current_session + and not cross_host_block + and not identity_block + and not drift_block ) switching = gitea_config.is_runtime_switching_enabled() @@ -11834,33 +12039,19 @@ def gitea_resolve_task_capability( restart_required = False reason_msg = None - # Find matching configured profiles - matching_profiles = [] - if config and "profiles" in config: - for p_name, p_data in config["profiles"].items(): - p_allowed = p_data.get("allowed_operations") or [] - p_forbidden = p_data.get("forbidden_operations") or [] - p_allowed_n = [] - for op in p_allowed: - try: - p_allowed_n.append(gitea_config.normalize_operation(op)) - except Exception: - pass - p_forbidden_n = [] - for op in p_forbidden: - try: - p_forbidden_n.append(gitea_config.normalize_operation(op)) - except Exception: - pass - ok, _ = gitea_config.check_operation(required_permission, p_allowed_n, p_forbidden_n) + # Matching profiles: same remote/host only (#714) — advisory, never activated. + matching_profiles = _matching_configured_profiles( + config, required_permission, remote=remote + ) + # Role filter for exclusive tasks + if config and "profiles" in config and task in role_exclusive_tasks: + filtered = [] + for p_name in matching_profiles: + p_data = (config.get("profiles") or {}).get(p_name) or {} p_role = (p_data.get("role") or "").strip() - p_role_ok = ( - task not in role_exclusive_tasks - or not p_role - or p_role == required_role - ) - if ok and p_role_ok: - matching_profiles.append(p_name) + if not p_role or p_role == required_role: + filtered.append(p_name) + matching_profiles = filtered configured = len(matching_profiles) > 0 available_in_session = allowed_in_current_session @@ -11876,16 +12067,35 @@ def gitea_resolve_task_capability( ) if not allowed_in_current_session: - if configured and switching: - restart_required = True + deny_parts: list[str] = [] + if cross_host_block: + deny_parts.extend(remote_assess.get("reasons") or []) + if identity_block: + deny_parts.extend(id_assess.get("reasons") or []) + if drift_block: + deny_parts.extend(ctx_assess.get("reasons") or []) + if not permission_allowed_in_current_session and not deny_parts: + deny_parts.append( + f"Active profile '{profile.get('profile_name')}' is not allowed " + f"to '{required_permission}' (no substitute profile activated; fail closed)." + ) + if role_mismatch_reason: + deny_parts.append(role_mismatch_reason) + if deny_parts: + reason_msg = "; ".join(deny_parts) + elif configured and switching: + # Same-remote profile exists but is not the active one — explicit switch only. + restart_required = False available_in_session = False reason_msg = ( - f"{required_role.capitalize()} profile exists but MCP server " - "was added after session startup and is not attached." + f"Active profile cannot perform '{required_permission}'. " + f"Same-remote matching profiles (advisory only, not activated): " + f"{matching_profiles}." ) elif not configured: reason_msg = ( - f"No profile configured with permission '{required_permission}'." + f"No same-remote profile configured with permission " + f"'{required_permission}' for remote '{remote}'." ) elif role_mismatch_reason: reason_msg = role_mismatch_reason @@ -11893,9 +12103,22 @@ def gitea_resolve_task_capability( next_safe_action = "None; ready for operations." if not allowed_in_current_session: - if switching: + if cross_host_block or identity_block or drift_block: different_namespace_required = False - next_safe_action = f"Switch to a profile that has the required permission by calling gitea_activate_profile (matching configured profiles: {matching_profiles})." + next_safe_action = ( + "BLOCKED + DIAGNOSE: session context/host/identity is inconsistent " + "or the active profile cannot serve this remote. Re-pin the correct " + "profile with gitea_activate_profile for the intended remote, verify " + "with gitea_whoami, and do not use cross-host profile substitution." + ) + elif switching: + different_namespace_required = False + next_safe_action = ( + f"Switch explicitly with gitea_activate_profile to a same-remote " + f"profile that allows '{required_permission}' " + f"(matching configured profiles: {matching_profiles}). " + "Capability resolution never auto-substitutes profiles (#714)." + ) else: different_namespace_required = True if required_role == "reviewer": @@ -11980,6 +12203,13 @@ def gitea_resolve_task_capability( "runtime_switching_supported": switching, "different_mcp_namespace_required": different_namespace_required, "exact_safe_next_action": next_safe_action, + # #714: immutable session context proof (must match whoami / runtime). + "requested_remote": remote, + "resolved_host": h, + "session_context_audit": session_ctx.mutation_context_audit_fields(), + "profile_remote_compatible": not cross_host_block, + "identity_match": not identity_block, + "auto_profile_substitution": False, } if reason_msg: result["reason"] = reason_msg diff --git a/session_context_binding.py b/session_context_binding.py new file mode 100644 index 0000000..d5bed09 --- /dev/null +++ b/session_context_binding.py @@ -0,0 +1,358 @@ +"""Session-immutable MCP mutation context (#714). + +Pins profile, remote, host, repository, identity, and role for the life of an +MCP process (or until an explicit ``gitea_activate_profile`` re-bind). + +Capability resolution and mutation gates must evaluate only the active +profile for the requested remote. Silent cross-host / cross-profile +substitution is forbidden. +""" + +from __future__ import annotations + +import os +import urllib.parse +from typing import Any + +# Process-local only — never a shared file (same rationale as mutation authority). +_SESSION_CONTEXT: dict[str, Any] | None = None + + +def clear_session_context() -> None: + """Reset session context (tests / process rebind).""" + global _SESSION_CONTEXT + _SESSION_CONTEXT = None + + +def get_session_context() -> dict[str, Any] | None: + """Return a shallow copy of the bound context, or None if unbound.""" + if _SESSION_CONTEXT is None: + return None + return dict(_SESSION_CONTEXT) + + +def profile_host(profile: dict | None) -> str | None: + """Hostname from profile base_url, lowercased, or None.""" + if not profile: + return None + base = (profile.get("base_url") or "").strip() + if not base: + return None + try: + parsed = urllib.parse.urlparse(base) + host = (parsed.netloc or parsed.path or "").strip().lower() + return host or None + except Exception: + return None + + +def remote_host(remote: str | None, remotes: dict | None) -> str | None: + """Hostname for a known remote key.""" + if not remote or not remotes: + return None + entry = remotes.get(remote) or {} + return (entry.get("host") or "").strip().lower() or None + + +def profile_matches_remote( + profile: dict | None, + remote: str | None, + remotes: dict | None, + *, + contexts: dict | None = None, +) -> bool: + """True when *profile* is bound to the same host/context as *remote*.""" + if not profile or not remote: + return False + r_host = remote_host(remote, remotes) + p_host = profile_host(profile) + if r_host and p_host: + return r_host == p_host + # Fall back to context name heuristics when base_url missing. + ctx = (profile.get("context") or "").strip().lower() + if not ctx or not contexts: + return False + ctx_data = contexts.get(ctx) or {} + gitea = ctx_data.get("gitea") or {} + base = (gitea.get("base_url") or "").strip() + if not base or not r_host: + return False + try: + parsed = urllib.parse.urlparse(base) + c_host = (parsed.netloc or parsed.path or "").strip().lower() + except Exception: + return False + return bool(c_host) and c_host == r_host + + +def bind_session_context( + *, + profile_name: str, + remote: str | None, + host: str | None, + identity: str | None, + repository: str | None = None, + org: str | None = None, + role_kind: str | None = None, + expected_username: str | None = None, + source: str = "bind", +) -> dict[str, Any]: + """Bind or re-bind the immutable session context (explicit activate path).""" + global _SESSION_CONTEXT + _SESSION_CONTEXT = { + "profile_name": (profile_name or "").strip() or None, + "remote": (remote or "").strip() or None, + "host": (host or "").strip().lower() or None, + "identity": (identity or "").strip() or None, + "repository": (repository or "").strip() or None, + "org": (org or "").strip() or None, + "role_kind": (role_kind or "").strip() or None, + "expected_username": (expected_username or "").strip() or None, + "source": source, + "pid": os.getpid(), + } + return get_session_context() # type: ignore[return-value] + + +def seed_session_context_if_unbound( + *, + profile_name: str, + remote: str | None, + host: str | None, + identity: str | None, + repository: str | None = None, + org: str | None = None, + role_kind: str | None = None, + expected_username: str | None = None, + source: str = "seed", + active_profile_override: str | None = None, + env_profile: str | None = None, +) -> dict[str, Any]: + """Bind when unbound, or rebind when the static env profile changes. + + Mid-session ``_active_profile_override`` changes that did **not** go + through ``gitea_activate_profile`` keep the prior bound context so + assess/mutation gates can detect drift and fail closed. + """ + global _SESSION_CONTEXT + live = (profile_name or "").strip() or None + if _SESSION_CONTEXT is None or _SESSION_CONTEXT.get("pid") != os.getpid(): + return bind_session_context( + profile_name=profile_name, + remote=remote, + host=host, + identity=identity, + repository=repository, + org=org, + role_kind=role_kind, + expected_username=expected_username, + source=source, + ) + bound_profile = _SESSION_CONTEXT.get("profile_name") + if live and bound_profile and live != bound_profile: + # Static env profile switch (tests / launcher) without override: rebind. + # Override present but bound not updated: leave bound for drift detection. + if active_profile_override is None and ( + env_profile is None or env_profile == live + ): + return bind_session_context( + profile_name=profile_name, + remote=remote, + host=host, + identity=identity, + repository=repository, + org=org, + role_kind=role_kind, + expected_username=expected_username, + source=f"{source}-env-rebind", + ) + return get_session_context() # type: ignore[return-value] + + +def assess_session_context( + *, + profile_name: str | None, + remote: str | None, + host: str | None = None, + identity: str | None = None, + repository: str | None = None, + org: str | None = None, + expected_username: str | None = None, + require_bound: bool = False, +) -> dict[str, Any]: + """Compare live values against the bound session context. + + Returns ``proven`` / ``block`` / ``reasons``. When unbound and + ``require_bound`` is false, does not block (caller may seed). When + unbound and ``require_bound`` is true, fails closed. + """ + reasons: list[str] = [] + ctx = _SESSION_CONTEXT + if ctx is None or ctx.get("pid") != os.getpid(): + if require_bound: + reasons.append( + "session mutation context is unbound; call gitea_whoami or " + "gitea_activate_profile before mutating (fail closed)" + ) + return _assessment(False, reasons, ctx) + return _assessment(True, reasons, ctx) + + live_profile = (profile_name or "").strip() or None + live_remote = (remote or "").strip() or None + live_host = (host or "").strip().lower() or None + live_identity = (identity or "").strip() or None + live_repo = (repository or "").strip() or None + live_org = (org or "").strip() or None + + if ctx.get("profile_name") and live_profile and live_profile != ctx.get("profile_name"): + reasons.append( + f"profile drift: live '{live_profile}' != bound " + f"'{ctx.get('profile_name')}' (fail closed)" + ) + if ctx.get("remote") and live_remote and live_remote != ctx.get("remote"): + reasons.append( + f"remote drift: live '{live_remote}' != bound " + f"'{ctx.get('remote')}' (fail closed)" + ) + if ctx.get("host") and live_host and live_host != ctx.get("host"): + reasons.append( + f"host drift: live '{live_host}' != bound " + f"'{ctx.get('host')}' (fail closed)" + ) + if ( + ctx.get("identity") + and live_identity + and live_identity != ctx.get("identity") + ): + reasons.append( + f"identity drift: live '{live_identity}' != bound " + f"'{ctx.get('identity')}' (fail closed)" + ) + if ctx.get("repository") and live_repo and live_repo != ctx.get("repository"): + reasons.append( + f"repository drift: live '{live_repo}' != bound " + f"'{ctx.get('repository')}' (fail closed)" + ) + if ctx.get("org") and live_org and live_org != ctx.get("org"): + reasons.append( + f"org drift: live '{live_org}' != bound " + f"'{ctx.get('org')}' (fail closed)" + ) + + expected = expected_username or ctx.get("expected_username") + if expected and live_identity and live_identity != expected: + reasons.append( + f"identity mismatch: authenticated '{live_identity}' != " + f"profile expected '{expected}' (fail closed)" + ) + + return _assessment(not reasons, reasons, ctx) + + +def assess_identity_match( + *, + authenticated: str | None, + expected_username: str | None, +) -> dict[str, Any]: + """Fail closed when profile declares a username that does not match live auth.""" + reasons: list[str] = [] + auth = (authenticated or "").strip() or None + expected = (expected_username or "").strip() or None + if expected and auth and auth != expected: + reasons.append( + f"identity mismatch: authenticated '{auth}' != " + f"profile expected '{expected}' (fail closed)" + ) + return { + "proven": not reasons, + "block": bool(reasons), + "reasons": reasons, + "authenticated": auth, + "expected_username": expected, + } + + +def filter_profiles_for_remote( + config: dict | None, + remote: str | None, + remotes: dict | None, +) -> list[str]: + """Profile names whose host/context matches *remote* (enabled only).""" + if not config or not remote: + return [] + profiles = config.get("profiles") or {} + contexts = config.get("contexts") or {} + names: list[str] = [] + for name, data in profiles.items(): + if not isinstance(data, dict): + continue + if not data.get("enabled", True): + continue + if profile_matches_remote(data, remote, remotes, contexts=contexts): + names.append(name) + return sorted(names) + + +def profile_allowed_for_remote( + profile: dict | None, + remote: str | None, + remotes: dict | None, + *, + contexts: dict | None = None, +) -> dict[str, Any]: + """Assess whether the active profile may serve *remote*.""" + reasons: list[str] = [] + if not profile: + reasons.append("active profile unresolved (fail closed)") + return {"proven": False, "block": True, "reasons": reasons} + if not remote: + return {"proven": True, "block": False, "reasons": reasons} + if not profile_matches_remote(profile, remote, remotes, contexts=contexts): + p_name = profile.get("profile_name") or profile.get("name") or "(unknown)" + p_host = profile_host(profile) or "(none)" + r_host = remote_host(remote, remotes) or "(none)" + reasons.append( + f"cross-host profile denial: profile '{p_name}' (host '{p_host}') " + f"cannot serve remote '{remote}' (host '{r_host}') (fail closed)" + ) + return {"proven": not reasons, "block": bool(reasons), "reasons": reasons} + + +def mutation_context_audit_fields( + ctx: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Fields to include in pre-mutation audit records.""" + data = ctx if ctx is not None else get_session_context() + if not data: + return { + "session_context_bound": False, + "session_profile": None, + "session_remote": None, + "session_host": None, + "session_identity": None, + "session_repository": None, + "session_org": None, + } + return { + "session_context_bound": True, + "session_profile": data.get("profile_name"), + "session_remote": data.get("remote"), + "session_host": data.get("host"), + "session_identity": data.get("identity"), + "session_repository": data.get("repository"), + "session_org": data.get("org"), + "session_role_kind": data.get("role_kind"), + "session_context_source": data.get("source"), + } + + +def _assessment( + proven: bool, reasons: list[str], ctx: dict[str, Any] | None +) -> dict[str, Any]: + return { + "proven": proven, + "block": not proven, + "reasons": list(reasons), + "bound_context": dict(ctx) if ctx else None, + "audit": mutation_context_audit_fields(ctx), + } diff --git a/tests/test_issue_714_session_context_immutability.py b/tests/test_issue_714_session_context_immutability.py new file mode 100644 index 0000000..eeac093 --- /dev/null +++ b/tests/test_issue_714_session_context_immutability.py @@ -0,0 +1,390 @@ +"""#714: fail closed on cross-host MCP profile drift and capability substitution.""" + +from __future__ import annotations + +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import gitea_config # noqa: E402 +import mcp_server # noqa: E402 +import session_context_binding as session_ctx # noqa: E402 + + +CONFIG_714 = { + "version": 2, + "allow_runtime_switching": True, + "contexts": { + "prgs": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.prgs.cc"}, + }, + "mdcps": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.dadeschools.net"}, + }, + }, + "profiles": { + "prgs-author": { + "enabled": True, + "context": "prgs", + "role": "author", + "username": "jcwalker3", + "base_url": "https://gitea.prgs.cc", + "auth": {"type": "env", "name": "GITEA_TOKEN_PRGS_AUTHOR"}, + "allowed_operations": [ + "gitea.read", + "gitea.issue.create", + "gitea.issue.comment", + "gitea.issue.close", + "gitea.pr.create", + "gitea.pr.comment", + "gitea.branch.create", + "gitea.branch.push", + "gitea.repo.commit", + ], + "forbidden_operations": [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.request_changes", + ], + "execution_profile": "prgs-author", + }, + "mdcps-reviewer": { + "enabled": True, + "context": "mdcps", + "role": "reviewer", + "username": "913443", + "base_url": "https://gitea.dadeschools.net", + "auth": {"type": "env", "name": "GITEA_TOKEN_MDCPS_REVIEWER"}, + "allowed_operations": [ + "gitea.read", + "gitea.pr.review", + "gitea.pr.comment", + "gitea.pr.approve", + "gitea.pr.request_changes", + ], + "forbidden_operations": [ + "gitea.branch.create", + "gitea.branch.push", + "gitea.pr.create", + "gitea.pr.merge", + "gitea.repo.commit", + ], + "execution_profile": "mdcps-reviewer", + }, + "mdcps-author": { + "enabled": True, + "context": "mdcps", + "role": "author", + "username": "913443", + "base_url": "https://gitea.dadeschools.net", + "auth": {"type": "env", "name": "GITEA_TOKEN_MDCPS_AUTHOR"}, + "allowed_operations": [ + "gitea.read", + "gitea.issue.create", + "gitea.issue.comment", + "gitea.pr.create", + "gitea.pr.comment", + "gitea.branch.create", + "gitea.branch.push", + "gitea.repo.commit", + ], + "forbidden_operations": [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.request_changes", + ], + "execution_profile": "mdcps-author", + }, + }, +} + + +class TestIssue714SessionContextImmutability(unittest.TestCase): + def setUp(self): + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(CONFIG_714)) + self._remotes = patch.dict( + mcp_server.REMOTES, + { + "dadeschools": { + "host": "gitea.dadeschools.net", + "org": "913443", + "repo": "eAgenda", + }, + "prgs": { + "host": "gitea.prgs.cc", + "org": "Scaled-Tech-Consulting", + "repo": "Gitea-Tools", + }, + }, + clear=False, + ) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + gitea_config._active_profile_override = None + mcp_server._MUTATION_AUTHORITY = None + session_ctx.clear_session_context() + self._env = { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": "mdcps-reviewer", + "GITEA_TOKEN_MDCPS_REVIEWER": "mdcps-reviewer-token", + "GITEA_TOKEN_MDCPS_AUTHOR": "mdcps-author-token", + "GITEA_TOKEN_PRGS_AUTHOR": "prgs-author-token", + } + + def tearDown(self): + self._remotes.stop() + mcp_server._IDENTITY_CACHE.clear() + gitea_config._active_profile_override = None + mcp_server._MUTATION_AUTHORITY = None + session_ctx.clear_session_context() + self._dir.cleanup() + + def _api_side_effect(self, method, url, header): + # Token identity mapping for whoami endpoint + auth = str(header) + if "mdcps-reviewer-token" in auth or "mdcps-author-token" in auth: + login = "913443" + else: + login = "jcwalker3" + return {"login": login, "full_name": "Test", "id": 1, "email": "t@example.com"} + + def test_pin_mdcps_reviewer_never_drifts_to_prgs_author(self): + """Reproduce the incident: pin mdcps-reviewer then resolve comment_issue.""" + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + act = mcp_server.gitea_activate_profile( + profile_name="mdcps-reviewer", remote="dadeschools" + ) + self.assertTrue(act["success"]) + self.assertEqual(act["after_profile"], "mdcps-reviewer") + + who1 = mcp_server.gitea_whoami(remote="dadeschools") + self.assertEqual(who1["profile"]["profile_name"], "mdcps-reviewer") + self.assertEqual(who1["username"], "913443") + + # Capability that mdcps-reviewer lacks — must NOT switch to prgs-author + res = mcp_server.gitea_resolve_task_capability( + task="comment_issue", remote="dadeschools" + ) + self.assertEqual(res["active_profile"], "mdcps-reviewer") + self.assertFalse(res["allowed_in_current_session"]) + self.assertFalse(res.get("auto_profile_substitution", True)) + self.assertNotIn("prgs-author", res["matching_configured_profile"]) + self.assertIn("mdcps-author", res["matching_configured_profile"]) + + who2 = mcp_server.gitea_whoami(remote="dadeschools") + rt = mcp_server.gitea_get_runtime_context(remote="dadeschools") + self.assertEqual(who2["profile"]["profile_name"], "mdcps-reviewer") + self.assertEqual(rt["active_profile"], "mdcps-reviewer") + # Still pinned — never prgs-author + self.assertEqual( + gitea_config.selected_profile_name(), "mdcps-reviewer" + ) + + def test_repeated_calls_remain_on_mdcps_reviewer(self): + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + mcp_server.gitea_activate_profile( + profile_name="mdcps-reviewer", remote="dadeschools" + ) + for _ in range(5): + res = mcp_server.gitea_resolve_task_capability( + task="review_pr", remote="dadeschools" + ) + self.assertEqual(res["active_profile"], "mdcps-reviewer") + who = mcp_server.gitea_whoami(remote="dadeschools") + self.assertEqual(who["profile"]["profile_name"], "mdcps-reviewer") + rt = mcp_server.gitea_get_runtime_context(remote="dadeschools") + self.assertEqual(rt["active_profile"], "mdcps-reviewer") + + def test_dadeschools_request_cannot_resolve_through_prgs_author(self): + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + mcp_server.gitea_activate_profile( + profile_name="mdcps-reviewer", remote="dadeschools" + ) + res = mcp_server.gitea_resolve_task_capability( + task="comment_issue", remote="dadeschools" + ) + self.assertNotEqual(res["active_profile"], "prgs-author") + self.assertNotIn("prgs-author", res["matching_configured_profile"]) + # Gate must not silently switch either + blocked = mcp_server._profile_permission_block( + "gitea.issue.comment", remote="dadeschools" + ) + self.assertIsNotNone(blocked) + self.assertFalse(blocked.get("success", True)) + self.assertEqual( + gitea_config.selected_profile_name(), "mdcps-reviewer" + ) + + def test_unsupported_capability_fails_closed_structured(self): + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + mcp_server.gitea_activate_profile( + profile_name="mdcps-reviewer", remote="dadeschools" + ) + res = mcp_server.gitea_resolve_task_capability( + task="comment_issue", remote="dadeschools" + ) + self.assertFalse(res["allowed_in_current_session"]) + self.assertTrue(res["stop_required"]) + self.assertIn("reason", res) + self.assertIn("exact_safe_next_action", res) + self.assertIn("activate_profile", res["exact_safe_next_action"]) + # Unknown task still fail closed + with self.assertRaises(ValueError): + mcp_server.gitea_resolve_task_capability( + task="reopen_issue", remote="dadeschools" + ) + + def test_identity_mismatch_blocks_mutation(self): + """Profile expects 913443 but authenticated as jcwalker3.""" + + def wrong_identity(method, url, header): + return { + "login": "jcwalker3", + "full_name": "Wrong", + "id": 9, + "email": "w@example.com", + } + + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=wrong_identity): + mcp_server.gitea_activate_profile( + profile_name="mdcps-reviewer", remote="dadeschools" + ) + blocked = mcp_server._session_context_mutation_block( + remote="dadeschools" + ) + self.assertIsNotNone(blocked) + self.assertTrue( + any("identity mismatch" in r for r in blocked["reasons"]) + ) + + def test_repository_host_mismatch_blocks_mutation(self): + """mdcps-reviewer cannot serve prgs remote.""" + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + mcp_server.gitea_activate_profile( + profile_name="mdcps-reviewer", remote="dadeschools" + ) + blocked = mcp_server._session_context_mutation_block(remote="prgs") + self.assertIsNotNone(blocked) + self.assertTrue( + any("cross-host" in r for r in blocked["reasons"]) + ) + + def test_drift_cannot_reach_write(self): + """Simulated mid-session profile override cannot pass permission gate.""" + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + mcp_server.gitea_activate_profile( + profile_name="mdcps-reviewer", remote="dadeschools" + ) + # Bind session as mdcps-reviewer + self.assertEqual( + session_ctx.get_session_context()["profile_name"], + "mdcps-reviewer", + ) + # Hostile override without activate_profile + gitea_config._active_profile_override = "prgs-author" + blocked = mcp_server._session_context_mutation_block( + remote="dadeschools" + ) + self.assertIsNotNone(blocked) + self.assertTrue(any("drift" in r or "cross-host" in r for r in blocked["reasons"])) + + def test_static_prgs_author_still_works(self): + env = { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": "prgs-author", + "GITEA_TOKEN_PRGS_AUTHOR": "prgs-author-token", + } + with patch.dict(os.environ, env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + session_ctx.clear_session_context() + gitea_config._active_profile_override = None + res = mcp_server.gitea_resolve_task_capability( + task="create_issue", remote="prgs" + ) + self.assertEqual(res["active_profile"], "prgs-author") + self.assertTrue(res["allowed_in_current_session"]) + self.assertEqual(res["active_identity"], "jcwalker3") + + def test_static_mdcps_author_still_works(self): + env = { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": "mdcps-author", + "GITEA_TOKEN_MDCPS_AUTHOR": "mdcps-author-token", + } + with patch.dict(os.environ, env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + session_ctx.clear_session_context() + gitea_config._active_profile_override = None + res = mcp_server.gitea_resolve_task_capability( + task="comment_issue", remote="dadeschools" + ) + self.assertEqual(res["active_profile"], "mdcps-author") + self.assertTrue(res["allowed_in_current_session"]) + self.assertNotIn("prgs-author", res["matching_configured_profile"]) + + +class TestSessionContextBindingUnit(unittest.TestCase): + def setUp(self): + session_ctx.clear_session_context() + + def tearDown(self): + session_ctx.clear_session_context() + + def test_profile_matches_remote_by_base_url(self): + remotes = { + "dadeschools": {"host": "gitea.dadeschools.net"}, + "prgs": {"host": "gitea.prgs.cc"}, + } + mdcps = {"base_url": "https://gitea.dadeschools.net", "context": "mdcps"} + prgs = {"base_url": "https://gitea.prgs.cc", "context": "prgs"} + self.assertTrue( + session_ctx.profile_matches_remote(mdcps, "dadeschools", remotes) + ) + self.assertFalse(session_ctx.profile_matches_remote(mdcps, "prgs", remotes)) + self.assertTrue(session_ctx.profile_matches_remote(prgs, "prgs", remotes)) + + def test_bind_and_detect_drift(self): + session_ctx.bind_session_context( + profile_name="mdcps-reviewer", + remote="dadeschools", + host="gitea.dadeschools.net", + identity="913443", + source="test", + ) + ok = session_ctx.assess_session_context( + profile_name="mdcps-reviewer", + remote="dadeschools", + host="gitea.dadeschools.net", + identity="913443", + ) + self.assertTrue(ok["proven"]) + bad = session_ctx.assess_session_context( + profile_name="prgs-author", + remote="dadeschools", + host="gitea.dadeschools.net", + identity="jcwalker3", + ) + self.assertTrue(bad["block"]) + self.assertTrue(any("drift" in r for r in bad["reasons"])) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_operation_scoped_roles.py b/tests/test_operation_scoped_roles.py index c382085..848bcde 100644 --- a/tests/test_operation_scoped_roles.py +++ b/tests/test_operation_scoped_roles.py @@ -1,4 +1,9 @@ -"""Tests for operation-scoped role selection and automatic dispatch switching (#228).""" +"""Tests for operation-scoped role selection (#228, updated by #714). + +#714 removes silent automatic profile substitution. Capability resolution +and mutation gates evaluate only the active profile; explicit +``gitea_activate_profile`` is required to switch roles. +""" import os import sys import json @@ -11,10 +16,12 @@ sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.par import gitea_config import gitea_auth import mcp_server +import session_context_binding as session_ctx from reviewer_worktree import assess_author_worktree_continuity CONFIG_TEST = { "version": 2, + "allow_runtime_switching": True, "contexts": { "ctx": { "enabled": True, @@ -30,6 +37,7 @@ CONFIG_TEST = { "context": "ctx", "role": "author", "username": "author-user", + "base_url": "https://gitea.example.com", "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, "allowed_operations": ["gitea.read", "gitea.issue.create", "gitea.pr.create", "gitea.branch.push", "gitea.issue.comment"], "forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"], @@ -40,6 +48,7 @@ CONFIG_TEST = { "context": "ctx", "role": "reviewer", "username": "reviewer-user", + "base_url": "https://gitea.example.com", "auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"}, "allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.approve", "gitea.pr.merge", "gitea.issue.comment"], "forbidden_operations": ["gitea.pr.create", "gitea.branch.push"], @@ -59,6 +68,7 @@ class TestOperationScopedRoles(unittest.TestCase): mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None + session_ctx.clear_session_context() self._dir = tempfile.TemporaryDirectory() self.config_path = os.path.join(self._dir.name, "profiles.json") self._write_config(CONFIG_TEST) @@ -68,6 +78,7 @@ class TestOperationScopedRoles(unittest.TestCase): mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None + session_ctx.clear_session_context() self._dir.cleanup() def _write_config(self, obj): @@ -85,62 +96,71 @@ class TestOperationScopedRoles(unittest.TestCase): return env @patch("mcp_server.api_request") - def test_auto_switch_to_reviewer(self, mock_api): - # mock identity resolution to return username matching profile + def test_no_auto_switch_to_reviewer(self, mock_api): + # #714: capability resolution must not silently switch author → reviewer mock_api.side_effect = lambda method, url, header: ( {"login": "reviewer-user"} if "reviewer-pass" in str(header) else {"login": "author-user"} ) with patch.dict(os.environ, self._env("author-profile")): - # initially we are author-profile self.assertEqual(gitea_config.selected_profile_name(), "author-profile") self.assertEqual(mcp_server.get_profile()["profile_name"], "author-profile") - # resolve a reviewer task (review_pr) res = mcp_server.gitea_resolve_task_capability(task="review_pr", remote="prgs") - - # verify it automatically switched to reviewer-profile - self.assertTrue(res["allowed_in_current_session"]) - self.assertTrue(res["available_in_session"]) - self.assertEqual(res["active_profile"], "reviewer-profile") - self.assertEqual(res["active_identity"], "reviewer-user") - self.assertEqual(gitea_config.selected_profile_name(), "reviewer-profile") + + self.assertFalse(res["allowed_in_current_session"]) + self.assertFalse(res["available_in_session"]) + self.assertEqual(res["active_profile"], "author-profile") + self.assertEqual(res["active_identity"], "author-user") + self.assertEqual(gitea_config.selected_profile_name(), "author-profile") + self.assertIn("reviewer-profile", res["matching_configured_profile"]) + self.assertFalse(res.get("auto_profile_substitution", True)) @patch("mcp_server.api_request") - def test_auto_switch_to_author(self, mock_api): + def test_no_auto_switch_to_author(self, mock_api): mock_api.side_effect = lambda method, url, header: ( {"login": "reviewer-user"} if "reviewer-pass" in str(header) else {"login": "author-user"} ) with patch.dict(os.environ, self._env("reviewer-profile")): - # initially we are reviewer-profile self.assertEqual(gitea_config.selected_profile_name(), "reviewer-profile") - # resolve an author task (create_issue) res = mcp_server.gitea_resolve_task_capability(task="create_issue", remote="prgs") - - # verify it automatically switched to author-profile - self.assertTrue(res["allowed_in_current_session"]) - self.assertTrue(res["available_in_session"]) - self.assertEqual(res["active_profile"], "author-profile") - self.assertEqual(res["active_identity"], "author-user") - self.assertEqual(gitea_config.selected_profile_name(), "author-profile") + + self.assertFalse(res["allowed_in_current_session"]) + self.assertFalse(res["available_in_session"]) + self.assertEqual(res["active_profile"], "reviewer-profile") + self.assertEqual(res["active_identity"], "reviewer-user") + self.assertEqual(gitea_config.selected_profile_name(), "reviewer-profile") + self.assertIn("author-profile", res["matching_configured_profile"]) @patch("mcp_server.api_request") - def test_restart_required_when_unattached(self, mock_api): + def test_explicit_activate_profile_still_works(self, mock_api): + mock_api.side_effect = lambda method, url, header: ( + {"login": "reviewer-user"} if "reviewer-pass" in str(header) else {"login": "author-user"} + ) + with patch.dict(os.environ, self._env("author-profile")): + act = mcp_server.gitea_activate_profile( + profile_name="reviewer-profile", remote="prgs" + ) + self.assertTrue(act["success"]) + self.assertEqual(act["after_profile"], "reviewer-profile") + res = mcp_server.gitea_resolve_task_capability(task="review_pr", remote="prgs") + self.assertTrue(res["allowed_in_current_session"]) + self.assertEqual(res["active_profile"], "reviewer-profile") + + @patch("mcp_server.api_request") + def test_denied_when_matching_profile_token_missing(self, mock_api): mock_api.side_effect = lambda method, url, header: {"login": "author-user"} # launch without reviewer token in env with patch.dict(os.environ, self._env("author-profile", with_reviewer_token=False)): self.assertEqual(gitea_config.selected_profile_name(), "author-profile") - # resolve a reviewer task (review_pr) res = mcp_server.gitea_resolve_task_capability(task="review_pr", remote="prgs") - - # verify it did NOT switch and reports restart_required + self.assertFalse(res["allowed_in_current_session"]) self.assertFalse(res["available_in_session"]) self.assertTrue(res["configured"]) - self.assertTrue(res["restart_required"]) self.assertTrue(res["stop_required"]) - self.assertIn("Reviewer profile exists but MCP server was added after session startup and is not attached", res["reason"]) + self.assertEqual(res["active_profile"], "author-profile") def test_author_continuity_dirty_worktree(self): # author is allowed to keep dirty worktree @@ -161,20 +181,21 @@ class TestOperationScopedRoles(unittest.TestCase): self.assertFalse(res2["proven"]) @patch("mcp_server.api_request") - def test_mutating_actions_auto_switch(self, mock_api): + def test_mutating_actions_do_not_auto_switch(self, mock_api): mock_api.side_effect = lambda method, url, header: ( {"login": "reviewer-user"} if "reviewer-pass" in str(header) else {"login": "author-user"} ) with patch.dict(os.environ, self._env("author-profile")): - # verify we are author-profile self.assertEqual(gitea_config.selected_profile_name(), "author-profile") - # call a reviewer mutation check helper (like _profile_permission_block with reviewer permission) - blocked = mcp_server._profile_permission_block("gitea.pr.merge", remote="prgs") - self.assertIsNone(blocked) # should switch to reviewer-profile and allow it (no permission block) + blocked = mcp_server._profile_permission_block( + "gitea.pr.merge", remote="prgs" + ) + self.assertIsNotNone(blocked) + self.assertFalse(blocked.get("success", True)) - # verify we dynamically switched to reviewer-profile - self.assertEqual(gitea_config.selected_profile_name(), "reviewer-profile") + # profile must remain author — no silent substitution + self.assertEqual(gitea_config.selected_profile_name(), "author-profile") if __name__ == "__main__": From 29ffe1407fa9620b4a75035bb9be3a254e74a674 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Wed, 15 Jul 2026 02:26:27 -0400 Subject: [PATCH 2/5] fix: isolate immutable session context tests (#714) --- gitea_mcp_server.py | 74 ++++---- session_context_binding.py | 173 ++++++++++++------ tests/conftest.py | 52 ++++-- tests/test_commit_payloads.py | 3 + ..._issue_714_session_context_immutability.py | 120 +++++++++++- tests/test_operation_scoped_roles.py | 3 - tests/test_resolve_task_capability.py | 4 +- 7 files changed, 303 insertions(+), 126 deletions(-) diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 24a01b7..4fa3e42 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -1131,7 +1131,7 @@ def _seed_session_context( org: str | None = None, source: str = "seed", ) -> dict: - """Seed/rebind immutable session context with env/override awareness (#714).""" + """Seed immutable session context once for the current process (#714).""" expected = (profile.get("username") or "").strip() or None return session_ctx.seed_session_context_if_unbound( profile_name=profile.get("profile_name") or "", @@ -1143,8 +1143,6 @@ def _seed_session_context( role_kind=_profile_role_kind(profile), expected_username=expected, source=source, - active_profile_override=gitea_config._active_profile_override, - env_profile=(os.environ.get("GITEA_MCP_PROFILE") or "").strip() or None, ) import issue_work_duplicate_gate # noqa: E402 import issue_workflow_labels # noqa: E402 @@ -2097,6 +2095,10 @@ def gitea_create_issue( blocked = _profile_permission_block( task_capability_map.required_permission("create_issue"), number=None, + remote=remote, + host=h, + org=o, + repo=r, ) if blocked: return blocked @@ -2537,6 +2539,10 @@ def gitea_create_pr( blocked = _profile_permission_block( task_capability_map.required_permission("create_pr"), number=None, + remote=remote, + host=host, + org=org, + repo=repo, ) if blocked: return blocked @@ -5373,7 +5379,7 @@ def gitea_commit_files( return blocked blocked = _profile_permission_block( task_capability_map.required_permission("commit_files"), - commit="", branch="", + commit="", branch="", remote=remote, host=host, org=org, repo=repo, ) if blocked: return blocked @@ -7613,9 +7619,16 @@ def _session_context_mutation_block( } profile_name = profile.get("profile_name") expected = (profile.get("username") or "").strip() or None - h = host or (REMOTES.get(remote or "", {}).get("host") if remote else None) + remote_config = REMOTES.get(remote or "", {}) if remote else {} + h = host or remote_config.get("host") + resolved_org = org or remote_config.get("org") + resolved_repo = repo or remote_config.get("repo") identity = username - if identity is None and h: + # Legacy env-only profiles do not declare an expected identity. Their + # first mutation still pins remote/host/repository, while existing audit + # paths resolve identity at the actual write. Configured identities are + # always verified here before a mutation can proceed. + if identity is None and expected and h: try: identity = _authenticated_username(h) except Exception: @@ -7643,8 +7656,8 @@ def _session_context_mutation_block( remote=remote, host=h, identity=identity, - repository=repo, - org=org, + repository=resolved_repo, + org=resolved_org, source="mutation-gate-seed", ) ctx_gate = session_ctx.assess_session_context( @@ -7652,8 +7665,8 @@ def _session_context_mutation_block( remote=remote, host=h, identity=identity, - repository=repo, - org=org, + repository=resolved_repo, + org=resolved_org, expected_username=expected, require_bound=True, ) @@ -7697,7 +7710,19 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict | # #714: evaluate active profile only — never auto-switch. _ensure_matching_profile(required_operation, req_role, extra_fields.get("remote")) - ctx_block = _session_context_mutation_block( + reasons = _profile_operation_gate(required_operation) + if reasons: + blocked = { + "success": False, + "performed": False, + "reasons": reasons, + "permission_report": _permission_block_report(required_operation), + "session_context_audit": session_ctx.mutation_context_audit_fields(), + } + blocked.update(extra_fields) + return blocked + + return _session_context_mutation_block( remote=extra_fields.get("remote"), host=extra_fields.get("host"), org=extra_fields.get("org"), @@ -7706,21 +7731,6 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict | issue_number=extra_fields.get("issue_number"), pr_number=extra_fields.get("pr_number"), ) - if ctx_block is not None: - return ctx_block - - reasons = _profile_operation_gate(required_operation) - if not reasons: - return None - blocked = { - "success": False, - "performed": False, - "reasons": reasons, - "permission_report": _permission_block_report(required_operation), - "session_context_audit": session_ctx.mutation_context_audit_fields(), - } - blocked.update(extra_fields) - return blocked def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None: @@ -7730,18 +7740,6 @@ def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None # #714: evaluate active profile only — never auto-switch. _ensure_matching_profile(required_permission, required_role, extra_fields.get("remote")) - ctx_block = _session_context_mutation_block( - remote=extra_fields.get("remote"), - host=extra_fields.get("host"), - org=extra_fields.get("org"), - repo=extra_fields.get("repo"), - number=extra_fields.get("number"), - issue_number=extra_fields.get("issue_number"), - pr_number=extra_fields.get("pr_number"), - ) - if ctx_block is not None: - return ctx_block - try: profile = get_profile() except Exception as exc: diff --git a/session_context_binding.py b/session_context_binding.py index d5bed09..57f2f1c 100644 --- a/session_context_binding.py +++ b/session_context_binding.py @@ -11,24 +11,70 @@ substitution is forbidden. from __future__ import annotations import os +import threading import urllib.parse -from typing import Any +from dataclasses import dataclass +from typing import Any, Mapping + + +@dataclass(frozen=True, slots=True) +class _SessionContext: + """One atomic, immutable process-session binding.""" + + profile_name: str | None + remote: str | None + host: str | None + identity: str | None + repository: str | None + org: str | None + role_kind: str | None + expected_username: str | None + source: str + pid: int + + def as_dict(self) -> dict[str, Any]: + return { + "profile_name": self.profile_name, + "remote": self.remote, + "host": self.host, + "identity": self.identity, + "repository": self.repository, + "org": self.org, + "role_kind": self.role_kind, + "expected_username": self.expected_username, + "source": self.source, + "pid": self.pid, + } + # Process-local only — never a shared file (same rationale as mutation authority). -_SESSION_CONTEXT: dict[str, Any] | None = None +# The frozen value prevents partial mutation, while the lock makes first-bind and +# sanctioned rebind atomic across concurrent MCP calls. +_SESSION_CONTEXT: _SessionContext | None = None +_SESSION_CONTEXT_LOCK = threading.RLock() -def clear_session_context() -> None: - """Reset session context (tests / process rebind).""" +def _reset_session_context_for_testing() -> None: + """Reset at a pytest test boundary; unavailable to production callers. + + Production sessions transition only through process startup/PID change or + the explicit profile-activation rebind. Keeping this helper private and + requiring pytest's per-test marker prevents it from becoming an MCP/runtime + bypass. + """ + if "PYTEST_CURRENT_TEST" not in os.environ: + raise RuntimeError("session context reset is restricted to pytest boundaries") global _SESSION_CONTEXT - _SESSION_CONTEXT = None + with _SESSION_CONTEXT_LOCK: + _SESSION_CONTEXT = None def get_session_context() -> dict[str, Any] | None: - """Return a shallow copy of the bound context, or None if unbound.""" - if _SESSION_CONTEXT is None: - return None - return dict(_SESSION_CONTEXT) + """Return a detached snapshot of the bound context, or None if unbound.""" + with _SESSION_CONTEXT_LOCK: + if _SESSION_CONTEXT is None: + return None + return _SESSION_CONTEXT.as_dict() def profile_host(profile: dict | None) -> str | None: @@ -97,21 +143,48 @@ def bind_session_context( expected_username: str | None = None, source: str = "bind", ) -> dict[str, Any]: - """Bind or re-bind the immutable session context (explicit activate path).""" + """Atomically bind/re-bind context (the explicit activation path).""" + with _SESSION_CONTEXT_LOCK: + return _bind_session_context_unlocked( + profile_name=profile_name, + remote=remote, + host=host, + identity=identity, + repository=repository, + org=org, + role_kind=role_kind, + expected_username=expected_username, + source=source, + ) + + +def _bind_session_context_unlocked( + *, + profile_name: str, + remote: str | None, + host: str | None, + identity: str | None, + repository: str | None, + org: str | None, + role_kind: str | None, + expected_username: str | None, + source: str, +) -> dict[str, Any]: + """Store a complete immutable context while the caller holds the lock.""" global _SESSION_CONTEXT - _SESSION_CONTEXT = { - "profile_name": (profile_name or "").strip() or None, - "remote": (remote or "").strip() or None, - "host": (host or "").strip().lower() or None, - "identity": (identity or "").strip() or None, - "repository": (repository or "").strip() or None, - "org": (org or "").strip() or None, - "role_kind": (role_kind or "").strip() or None, - "expected_username": (expected_username or "").strip() or None, - "source": source, - "pid": os.getpid(), - } - return get_session_context() # type: ignore[return-value] + _SESSION_CONTEXT = _SessionContext( + profile_name=(profile_name or "").strip() or None, + remote=(remote or "").strip() or None, + host=(host or "").strip().lower() or None, + identity=(identity or "").strip() or None, + repository=(repository or "").strip() or None, + org=(org or "").strip() or None, + role_kind=(role_kind or "").strip() or None, + expected_username=(expected_username or "").strip() or None, + source=source, + pid=os.getpid(), + ) + return _SESSION_CONTEXT.as_dict() def seed_session_context_if_unbound( @@ -125,37 +198,17 @@ def seed_session_context_if_unbound( role_kind: str | None = None, expected_username: str | None = None, source: str = "seed", - active_profile_override: str | None = None, - env_profile: str | None = None, ) -> dict[str, Any]: - """Bind when unbound, or rebind when the static env profile changes. + """Atomically bind only when this process has no current context. - Mid-session ``_active_profile_override`` changes that did **not** go - through ``gitea_activate_profile`` keep the prior bound context so - assess/mutation gates can detect drift and fail closed. + A changed environment or an interleaved call is not a session boundary and + therefore cannot replace an established binding. A newly started/forked + process is recognized by PID; explicit ``gitea_activate_profile`` uses + :func:`bind_session_context` as its sanctioned logical-session transition. """ - global _SESSION_CONTEXT - live = (profile_name or "").strip() or None - if _SESSION_CONTEXT is None or _SESSION_CONTEXT.get("pid") != os.getpid(): - return bind_session_context( - profile_name=profile_name, - remote=remote, - host=host, - identity=identity, - repository=repository, - org=org, - role_kind=role_kind, - expected_username=expected_username, - source=source, - ) - bound_profile = _SESSION_CONTEXT.get("profile_name") - if live and bound_profile and live != bound_profile: - # Static env profile switch (tests / launcher) without override: rebind. - # Override present but bound not updated: leave bound for drift detection. - if active_profile_override is None and ( - env_profile is None or env_profile == live - ): - return bind_session_context( + with _SESSION_CONTEXT_LOCK: + if _SESSION_CONTEXT is None or _SESSION_CONTEXT.pid != os.getpid(): + return _bind_session_context_unlocked( profile_name=profile_name, remote=remote, host=host, @@ -164,9 +217,9 @@ def seed_session_context_if_unbound( org=org, role_kind=role_kind, expected_username=expected_username, - source=f"{source}-env-rebind", + source=source, ) - return get_session_context() # type: ignore[return-value] + return _SESSION_CONTEXT.as_dict() def assess_session_context( @@ -187,7 +240,9 @@ def assess_session_context( unbound and ``require_bound`` is true, fails closed. """ reasons: list[str] = [] - ctx = _SESSION_CONTEXT + with _SESSION_CONTEXT_LOCK: + bound = _SESSION_CONTEXT + ctx = bound.as_dict() if bound is not None else None if ctx is None or ctx.get("pid") != os.getpid(): if require_bound: reasons.append( @@ -307,6 +362,12 @@ def profile_allowed_for_remote( return {"proven": False, "block": True, "reasons": reasons} if not remote: return {"proven": True, "block": False, "reasons": reasons} + # Legacy env-only profiles have no configured base URL or v2 context. Their + # first call may establish the process remote/host pin; after that, + # assess_session_context rejects any drift. Configured v2 profiles still + # require positive host/context alignment here. + if not profile_host(profile) and not (profile.get("context") or "").strip(): + return {"proven": True, "block": False, "reasons": reasons} if not profile_matches_remote(profile, remote, remotes, contexts=contexts): p_name = profile.get("profile_name") or profile.get("name") or "(unknown)" p_host = profile_host(profile) or "(none)" @@ -319,7 +380,7 @@ def profile_allowed_for_remote( def mutation_context_audit_fields( - ctx: dict[str, Any] | None = None, + ctx: Mapping[str, Any] | None = None, ) -> dict[str, Any]: """Fields to include in pre-mutation audit records.""" data = ctx if ctx is not None else get_session_context() @@ -347,7 +408,7 @@ def mutation_context_audit_fields( def _assessment( - proven: bool, reasons: list[str], ctx: dict[str, Any] | None + proven: bool, reasons: list[str], ctx: Mapping[str, Any] | None ) -> dict[str, Any]: return { "proven": proven, diff --git a/tests/conftest.py b/tests/conftest.py index 447f74a..0f49799 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -26,6 +26,14 @@ def _reset_mutation_authority(monkeypatch): Pin ``default_state_dir`` / ``DEFAULT_STATE_DIR`` to a per-test temp dir so durable load/save never touches host state even after env clears. """ + import session_context_binding as session_ctx + + # Each pytest item is an independent logical MCP session. Reset both before + # and after the item; the post-yield reset is in a finally block so an + # assertion, exception, or unittest teardown failure cannot pollute the + # next item. Production code has no automatic per-call reset path. + session_ctx._reset_session_context_for_testing() + for env_key in [ "GITEA_SESSION_PROFILE_LOCK", "GITEA_ACTIVE_WORKTREE", @@ -80,9 +88,15 @@ def _reset_mutation_authority(monkeypatch): try: import mcp_server except Exception: - _state_tmp.cleanup() - yield + try: + yield + finally: + session_ctx._reset_session_context_for_testing() + _state_tmp.cleanup() return + import gitea_config + + monkeypatch.setattr(gitea_config, "_active_profile_override", None) monkeypatch.setattr(mcp_server, "_MUTATION_AUTHORITY", None) monkeypatch.setattr(mcp_server, "_IDENTITY_CACHE", {}) monkeypatch.setattr(mcp_server, "_REVIEW_DECISION_LOCK", None) @@ -113,19 +127,23 @@ def _reset_mutation_authority(monkeypatch): capability_stop_terminal.clear() except Exception: pass - yield try: - import capability_stop_terminal - capability_stop_terminal.clear() - except Exception: - pass - try: - import review_workflow_load - review_workflow_load._REVIEW_WORKFLOW_LOAD = None - except Exception: - pass - try: - mcp_server._REVIEW_DECISION_LOCK = None - except Exception: - pass - _state_tmp.cleanup() + yield + finally: + try: + import capability_stop_terminal + capability_stop_terminal.clear() + except Exception: + pass + try: + import review_workflow_load + review_workflow_load._REVIEW_WORKFLOW_LOAD = None + except Exception: + pass + try: + mcp_server._REVIEW_DECISION_LOCK = None + except Exception: + pass + gitea_config._active_profile_override = None + session_ctx._reset_session_context_for_testing() + _state_tmp.cleanup() diff --git a/tests/test_commit_payloads.py b/tests/test_commit_payloads.py index d3b3e52..520f09e 100644 --- a/tests/test_commit_payloads.py +++ b/tests/test_commit_payloads.py @@ -227,6 +227,7 @@ class TestCommitPayloads(unittest.TestCase): @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value="token author-pass") def test_commit_files_traversal_blocked(self, _auth, mock_api): + mock_api.return_value = {"login": "author-user"} # Remove active lock file to ensure it fails on traversal/invalid locks os.remove(self.lock_file_path) @@ -248,6 +249,7 @@ class TestCommitPayloads(unittest.TestCase): @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value="token author-pass") def test_commit_files_outside_scope_blocked(self, _auth, mock_api): + mock_api.return_value = {"login": "author-user"} with patch.dict(os.environ, self._env("full-author"), clear=True): with self.assertRaises(ValueError) as ctx: mcp_server.gitea_commit_files( @@ -266,6 +268,7 @@ class TestCommitPayloads(unittest.TestCase): @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value="token author-pass") def test_commit_files_multiple_sources_blocked(self, _auth, mock_api): + mock_api.return_value = {"login": "author-user"} with patch.dict(os.environ, self._env("full-author"), clear=True): with self.assertRaises(ValueError) as ctx: mcp_server.gitea_commit_files( diff --git a/tests/test_issue_714_session_context_immutability.py b/tests/test_issue_714_session_context_immutability.py index eeac093..50c5c76 100644 --- a/tests/test_issue_714_session_context_immutability.py +++ b/tests/test_issue_714_session_context_immutability.py @@ -6,6 +6,7 @@ import json import os import sys import tempfile +import threading import unittest from pathlib import Path from unittest.mock import patch @@ -133,7 +134,6 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None - session_ctx.clear_session_context() self._env = { "GITEA_MCP_CONFIG": self.config_path, "GITEA_MCP_PROFILE": "mdcps-reviewer", @@ -147,7 +147,6 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None - session_ctx.clear_session_context() self._dir.cleanup() def _api_side_effect(self, method, url, header): @@ -314,7 +313,6 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): } with patch.dict(os.environ, env, clear=False): with patch("mcp_server.api_request", side_effect=self._api_side_effect): - session_ctx.clear_session_context() gitea_config._active_profile_override = None res = mcp_server.gitea_resolve_task_capability( task="create_issue", remote="prgs" @@ -331,7 +329,6 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): } with patch.dict(os.environ, env, clear=False): with patch("mcp_server.api_request", side_effect=self._api_side_effect): - session_ctx.clear_session_context() gitea_config._active_profile_override = None res = mcp_server.gitea_resolve_task_capability( task="comment_issue", remote="dadeschools" @@ -342,12 +339,6 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): class TestSessionContextBindingUnit(unittest.TestCase): - def setUp(self): - session_ctx.clear_session_context() - - def tearDown(self): - session_ctx.clear_session_context() - def test_profile_matches_remote_by_base_url(self): remotes = { "dadeschools": {"host": "gitea.dadeschools.net"}, @@ -385,6 +376,115 @@ class TestSessionContextBindingUnit(unittest.TestCase): self.assertTrue(bad["block"]) self.assertTrue(any("drift" in r for r in bad["reasons"])) + def test_bound_context_survives_multiple_calls_and_exceptions(self): + original = session_ctx.bind_session_context( + profile_name="mdcps-reviewer", + remote="dadeschools", + host="gitea.dadeschools.net", + identity="913443", + source="test", + ) + + try: + for _ in range(3): + observed = session_ctx.seed_session_context_if_unbound( + profile_name="prgs-author", + remote="prgs", + host="gitea.prgs.cc", + identity="jcwalker3", + source="interleaved-test-call", + ) + self.assertEqual(observed, original) + raise RuntimeError("simulated caller failure") + except RuntimeError: + pass + + self.assertEqual(session_ctx.get_session_context(), original) + drift = session_ctx.assess_session_context( + profile_name="prgs-author", + remote="prgs", + host="gitea.prgs.cc", + identity="jcwalker3", + require_bound=True, + ) + self.assertTrue(drift["block"]) + + def test_parallel_calls_cannot_overwrite_established_binding(self): + original = session_ctx.bind_session_context( + profile_name="mdcps-reviewer", + remote="dadeschools", + host="gitea.dadeschools.net", + identity="913443", + source="test", + ) + barrier = threading.Barrier(9) + results = [] + results_lock = threading.Lock() + + def competing_seed(index: int) -> None: + barrier.wait() + result = session_ctx.seed_session_context_if_unbound( + profile_name=f"prgs-author-{index}", + remote="prgs", + host="gitea.prgs.cc", + identity=f"other-{index}", + source="parallel-test-call", + ) + with results_lock: + results.append(result) + + threads = [threading.Thread(target=competing_seed, args=(i,)) for i in range(8)] + for thread in threads: + thread.start() + barrier.wait() + for thread in threads: + thread.join(timeout=5) + + self.assertFalse(any(thread.is_alive() for thread in threads)) + self.assertEqual(results, [original] * 8) + self.assertEqual(session_ctx.get_session_context(), original) + + def test_returned_snapshot_cannot_mutate_binding(self): + session_ctx.bind_session_context( + profile_name="mdcps-reviewer", + remote="dadeschools", + host="gitea.dadeschools.net", + identity="913443", + source="test", + ) + snapshot = session_ctx.get_session_context() + snapshot["profile_name"] = "prgs-author" + self.assertEqual( + session_ctx.get_session_context()["profile_name"], "mdcps-reviewer" + ) + + +class TestSessionContextTestBoundaryIsolation(unittest.TestCase): + def test_mdcps_binding_starts_clean_and_does_not_escape_test(self): + self.assertIsNone(session_ctx.get_session_context()) + session_ctx.bind_session_context( + profile_name="mdcps-reviewer", + remote="dadeschools", + host="gitea.dadeschools.net", + identity="913443", + source="test-boundary", + ) + + def test_prgs_binding_starts_clean_and_does_not_escape_test(self): + self.assertIsNone(session_ctx.get_session_context()) + session_ctx.bind_session_context( + profile_name="prgs-author", + remote="prgs", + host="gitea.prgs.cc", + identity="jcwalker3", + source="test-boundary", + ) + + def test_reset_helper_is_rejected_outside_pytest_boundary(self): + with patch.dict(os.environ, {}, clear=True): + with self.assertRaises(RuntimeError): + session_ctx._reset_session_context_for_testing() + if __name__ == "__main__": unittest.main() diff --git a/tests/test_operation_scoped_roles.py b/tests/test_operation_scoped_roles.py index 848bcde..fc029bd 100644 --- a/tests/test_operation_scoped_roles.py +++ b/tests/test_operation_scoped_roles.py @@ -16,7 +16,6 @@ sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.par import gitea_config import gitea_auth import mcp_server -import session_context_binding as session_ctx from reviewer_worktree import assess_author_worktree_continuity CONFIG_TEST = { @@ -68,7 +67,6 @@ class TestOperationScopedRoles(unittest.TestCase): mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None - session_ctx.clear_session_context() self._dir = tempfile.TemporaryDirectory() self.config_path = os.path.join(self._dir.name, "profiles.json") self._write_config(CONFIG_TEST) @@ -78,7 +76,6 @@ class TestOperationScopedRoles(unittest.TestCase): mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None - session_ctx.clear_session_context() self._dir.cleanup() def _write_config(self, obj): diff --git a/tests/test_resolve_task_capability.py b/tests/test_resolve_task_capability.py index 23f05bc..979f5fe 100644 --- a/tests/test_resolve_task_capability.py +++ b/tests/test_resolve_task_capability.py @@ -188,8 +188,8 @@ class TestResolveTaskCapability(unittest.TestCase): self.assertEqual(res["required_role_kind"], "author") self.assertTrue(res["allowed_in_current_session"]) - @patch("mcp_server.api_request", return_value={"login": "author-user"}) - @patch("mcp_server.get_auth_header", return_value="token author-pass") + @patch("mcp_server.api_request", return_value={"login": "reviewer-user"}) + @patch("mcp_server.get_auth_header", return_value="token reviewer-pass") def test_resolve_work_issue_reviewer_profile_blocked(self, _auth, _api): with patch.dict(os.environ, self._env("reviewer-profile")): res = mcp_server.gitea_resolve_task_capability( From d936da5c872fb4bad31536d80b7c5f57462b05a9 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Wed, 15 Jul 2026 15:51:52 -0400 Subject: [PATCH 3/5] test(session): cover reverse cross-host denial, concurrent init, repo drift (#714) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The formal REQUEST_CHANGES review on PR #715 was resolved by the test isolation commit (29ffe14); the full suite is green. Auditing the required session-lifecycle coverage surfaced three gaps that the suite did not prove: - Every cross-host test pinned mdcps-reviewer and denied prgs. The reverse direction (a pinned MDCPS profile must never serve a prgs request, nor be swapped for prgs-author) was unproven. - test_parallel_calls_cannot_overwrite_established_binding binds first and then races, so concurrent *initialization* from an unbound context — N threads racing to first-bind, exactly one winner, no torn context — was unproven. - assess_session_context checks repository and org drift, but no test bound a repository/org and asserted a same-host mismatch fails closed. Adds three tests covering those cases. Each was mutation-verified: disabling the first-bind-wins guard, the cross-host denial, and the repository/org drift checks makes the corresponding test fail. Additive only — no production code changed and no existing security assertion weakened, skipped, or rewritten. Focused: 20 passed. Full suite: 2713 passed, 6 skipped, 1 pre-existing warning, 161 subtests passed in 23.05s. Co-Authored-By: Claude Opus 4.8 (1M context) --- ..._issue_714_session_context_immutability.py | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/test_issue_714_session_context_immutability.py b/tests/test_issue_714_session_context_immutability.py index 50c5c76..d7c0f39 100644 --- a/tests/test_issue_714_session_context_immutability.py +++ b/tests/test_issue_714_session_context_immutability.py @@ -228,6 +228,26 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): gitea_config.selected_profile_name(), "mdcps-reviewer" ) + def test_prgs_request_cannot_resolve_through_mdcps_profile(self): + """Reverse of the incident: a pinned MDCPS profile must never serve a + prgs request, nor be silently swapped for the prgs-side profile.""" + with patch.dict(os.environ, self._env, clear=False): + with patch("mcp_server.api_request", side_effect=self._api_side_effect): + mcp_server.gitea_activate_profile( + profile_name="mdcps-author", remote="dadeschools" + ) + res = mcp_server.gitea_resolve_task_capability( + task="create_issue", remote="prgs" + ) + self.assertNotEqual(res["active_profile"], "prgs-author") + self.assertEqual(res["active_profile"], "mdcps-author") + self.assertFalse(res["allowed_in_current_session"]) + self.assertTrue(res["stop_required"]) + blocked = mcp_server._session_context_mutation_block(remote="prgs") + self.assertIsNotNone(blocked) + self.assertTrue(any("cross-host" in r for r in blocked["reasons"])) + self.assertEqual(gitea_config.selected_profile_name(), "mdcps-author") + def test_unsupported_capability_fails_closed_structured(self): with patch.dict(os.environ, self._env, clear=False): with patch("mcp_server.api_request", side_effect=self._api_side_effect): @@ -444,6 +464,92 @@ class TestSessionContextBindingUnit(unittest.TestCase): self.assertEqual(results, [original] * 8) self.assertEqual(session_ctx.get_session_context(), original) + def test_concurrent_initialization_has_single_winning_binding(self): + """Racing first-binds from an unbound context: exactly one winner, and + every racer observes that same complete binding (never a partial one).""" + self.assertIsNone(session_ctx.get_session_context()) + thread_count = 8 + barrier = threading.Barrier(thread_count) + results = [] + results_lock = threading.Lock() + + def racing_seed(index: int) -> None: + barrier.wait() + result = session_ctx.seed_session_context_if_unbound( + profile_name=f"prgs-author-{index}", + remote="prgs", + host="gitea.prgs.cc", + identity=f"user-{index}", + source="concurrent-init-test", + ) + with results_lock: + results.append(result) + + threads = [ + threading.Thread(target=racing_seed, args=(i,)) + for i in range(thread_count) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=5) + + self.assertFalse(any(thread.is_alive() for thread in threads)) + winner = session_ctx.get_session_context() + self.assertIsNotNone(winner) + self.assertEqual(len(results), thread_count) + self.assertEqual(results, [winner] * thread_count) + # A losing racer's identity must never be spliced into the winner. + self.assertEqual( + winner["identity"], winner["profile_name"].replace("prgs-author-", "user-") + ) + + def test_repository_mismatch_blocks_before_mutation(self): + """Same host and profile, different repository, must still fail closed.""" + session_ctx.bind_session_context( + profile_name="prgs-author", + remote="prgs", + host="gitea.prgs.cc", + identity="jcwalker3", + repository="Gitea-Tools", + org="Scaled-Tech-Consulting", + source="test", + ) + same = session_ctx.assess_session_context( + profile_name="prgs-author", + remote="prgs", + host="gitea.prgs.cc", + identity="jcwalker3", + repository="Gitea-Tools", + org="Scaled-Tech-Consulting", + require_bound=True, + ) + self.assertTrue(same["proven"]) + other_repo = session_ctx.assess_session_context( + profile_name="prgs-author", + remote="prgs", + host="gitea.prgs.cc", + identity="jcwalker3", + repository="eAgenda", + org="Scaled-Tech-Consulting", + require_bound=True, + ) + self.assertTrue(other_repo["block"]) + self.assertTrue( + any("repository drift" in r for r in other_repo["reasons"]) + ) + other_org = session_ctx.assess_session_context( + profile_name="prgs-author", + remote="prgs", + host="gitea.prgs.cc", + identity="jcwalker3", + repository="Gitea-Tools", + org="913443", + require_bound=True, + ) + self.assertTrue(other_org["block"]) + self.assertTrue(any("org drift" in r for r in other_org["reasons"])) + def test_returned_snapshot_cannot_mutate_binding(self): session_ctx.bind_session_context( profile_name="mdcps-reviewer", From 943d40270e9ecdee48707eb09454f0321e8e4e14 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Wed, 15 Jul 2026 16:50:41 -0400 Subject: [PATCH 4/5] fix(session): bind workspace-verified repository scope at first bind (#714) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The independent review reproduced a real hole: the production first-bind path never pinned repository/org, so the drift checks it feeds were skipped. Root cause ---------- gitea_whoami, gitea_get_runtime_context, gitea_resolve_task_capability and gitea_activate_profile all seeded the session context without repository or org. First-bind-wins then made the mutation gate's later seed a no-op, and assess_session_context only compares repository/org when the bound fields are non-null. Result, reproduced in production order with the real REMOTES: after whoami: repository=None org=None same-host repo=Other-Tools -> NOT BLOCKED same-host org=Other-Org -> NOT BLOCKED cross-host -> blocked (this part always worked) Binding REMOTES defaults would not fix it: REMOTES['prgs'].repo is 'Timesheet' (a default *target*, not an authorization scope, see #530), so pinning it fails closed on every legitimate Gitea-Tools mutation. There was no trusted source for repository scope, so this adds one. Design ------ - New optional profile field `allowed_repositories`: canonical owner/repository slugs. An authorization boundary, never the binding itself. Config-only — no env var can widen or forge it. - The session repository is derived from the verified, workspace-aligned git remote, never from caller-supplied org/repo, and validated against that allowlist. The session binds immutably to exactly one canonical slug; org is derived from it. A profile authorizing several repositories still binds to the single verified one. - Every first-bind entry point now pins the same complete context. Activation rejects an unauthorized/unverifiable workspace. Mutations fail closed when repository/org is unverified, and a tool-level org/repo override that disagrees with the binding is rejected before the write. - A mutation request can no longer establish, complete, or replace the binding (it previously seeded from `org or REMOTES[...]`, i.e. caller-controlled). - Enforcement is opt-in per profile: a profile without the field keeps prior behaviour, so existing static-profile namespaces stay functional. First-bind-wins, immutability, the RLock, profile isolation, host/identity validation and the private pytest-only reset are unchanged. gitea_auth.get_profile() built an explicit whitelist dict and silently dropped `allowed_repositories`; the new integration tests caught that the scope was never enforced through the real path. Tests ----- New tests/test_issue_714_production_first_bind.py drives the real entry points in production order rather than constructing _SessionContext directly. Covers complete first bind via each entry point, same-host repo/org drift, Timesheet and other-repo/other-org rejection, unverified and unauthorized workspaces, caller values unable to establish the binding, concurrent init selecting one repository, and the PR #715 author path staying functional. Mutation-verified: disabling trusted derivation, override validation, the scope check, or the get_profile passthrough each fails these tests (9/8/4/5 failures). No security assertion was weakened, removed, skipped, or rewritten. Focused: 26 passed. Issue #714 total: 46 passed. Prior failing modules: 240 passed. Config/profile/remote modules: 130 passed. Full suite: 2739 passed, 6 skipped, 1 pre-existing warning, 161 subtests in 25.80s. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/gitea-execution-profiles.md | 42 ++ gitea-mcp.v2-contexts.example.json | 1 + gitea_auth.py | 4 + gitea_config.py | 28 ++ gitea_mcp_server.py | 102 +++- session_context_binding.py | 133 +++++- tests/test_issue_714_production_first_bind.py | 448 ++++++++++++++++++ 7 files changed, 748 insertions(+), 10 deletions(-) create mode 100644 tests/test_issue_714_production_first_bind.py diff --git a/docs/gitea-execution-profiles.md b/docs/gitea-execution-profiles.md index 970f5d4..5cf4325 100644 --- a/docs/gitea-execution-profiles.md +++ b/docs/gitea-execution-profiles.md @@ -45,6 +45,7 @@ authenticated capability set. Each profile defines the following fields: | `authenticated_username` | string | The Gitea login this profile authenticates as (verified at runtime via `gitea_whoami`, not trusted from config). | | `allowed_operations` | list | Operation categories this profile may perform. | | `forbidden_operations` | list | Operation categories this profile must never perform. | +| `allowed_repositories` | list | Optional. Canonical `owner/repository` slugs this profile may bind to. An authorization boundary, not the binding itself — see [Repository scope](#repository-scope-714). | | `token_source_name` | string | The *name* of the secret source (e.g. env var name or secret key). **Never the token value.** | | `audit_label` | string | Short label attached to audit records for actions by this profile. | | `can_approve_prs` | bool | May submit an approving PR review. | @@ -57,6 +58,47 @@ authenticated capability set. Each profile defines the following fields: name), never the token itself. Token values are never part of a profile object, never logged, never returned by a tool, and never committed. +## Repository scope (#714) + +`allowed_repositories` declares the canonical `owner/repository` slugs a profile +may operate on: + +```json +"prgs-author": { + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"] +} +``` + +It is an **authorization boundary, not the session binding**. The binding is +derived and enforced like this: + +1. The session repository is derived from the **verified, workspace-aligned git + remote** — never from a caller-supplied `org`/`repo` argument, and never from + the `REMOTES` table (whose entries are default *targets*: `prgs` defaults to + `Timesheet`, which is not this project). +2. That workspace-derived slug is validated against `allowed_repositories`. +3. The session binds immutably to that one canonical `owner/repository`. The + organization is derived from the slug; there is no independent, + caller-controlled organization value. +4. If a profile authorizes several repositories, the verified workspace still + selects exactly one. A session never binds to the whole list and never + switches between entries. + +Fail-closed rules: + +- Activation is rejected when the workspace repository is absent from the + allowlist. +- A mutation is rejected when no verified workspace repository can be + established. +- A tool-level `org`/`repo` override that disagrees with the binding is rejected + before the mutation runs. +- A mutation request can never establish, complete, or replace the binding. + +The field is **config-only**: no environment variable can widen or forge it. It +is optional — a profile that omits it keeps the previous behaviour, so existing +static-profile namespaces stay functional until an operator provisions the +scope. Provisioning it is what activates enforcement for that profile. + ## Example profiles The following are the reference profiles. Booleans express intended capability diff --git a/gitea-mcp.v2-contexts.example.json b/gitea-mcp.v2-contexts.example.json index bd5b1ee..77c9d64 100644 --- a/gitea-mcp.v2-contexts.example.json +++ b/gitea-mcp.v2-contexts.example.json @@ -41,6 +41,7 @@ "execution_profile": "example-author", "audit_label": "example-author", "auth": { "type": "keychain", "id": "example-gitea-author-token" }, + "allowed_repositories": ["Example-Org/Example-Repo"], "allowed_operations": ["read", "branch", "commit", "push", "open_pr", "comment", "issue.comment"], "forbidden_operations": ["approve", "request_changes", "merge"] }, diff --git a/gitea_auth.py b/gitea_auth.py index a02d92a..385db61 100644 --- a/gitea_auth.py +++ b/gitea_auth.py @@ -570,6 +570,10 @@ def get_profile(): "profile_name": name, "allowed_operations": ops, "forbidden_operations": forbidden, + # #714 repository authorization boundary. Config-only on purpose: an + # environment variable must never widen or forge the set of + # repositories a session may bind to. + "allowed_repositories": _json_list("allowed_repositories"), "audit_label": audit_label, "token_source_name": token_source, "auth_source_type": auth_type, diff --git a/gitea_config.py b/gitea_config.py index ffbf7ea..3e0adef 100644 --- a/gitea_config.py +++ b/gitea_config.py @@ -475,6 +475,33 @@ def _require_enabled(kind, name, obj): return enabled +_REPO_SCOPE_RE = re.compile(r"^[^/\s]+/[^/\s]+$") + + +def _validate_allowed_repositories(name, raw): + """Validate the optional per-profile repository authorization scope (#714). + + ``allowed_repositories`` is an authorization boundary of canonical + ``owner/repository`` slugs. It is not the session binding: the verified + workspace repository selects exactly one entry at bind time. Absent means + "no repository scope configured" and is allowed, so existing profiles keep + working until an operator provisions the field. + """ + if raw is None: + return + if not isinstance(raw, list): + raise ConfigError( + f"profile '{name}' allowed_repositories must be a list of " + "'owner/repository' strings" + ) + for entry in raw: + if not isinstance(entry, str) or not _REPO_SCOPE_RE.match(entry.strip()): + raise ConfigError( + f"profile '{name}' allowed_repositories entry {entry!r} is not " + "a canonical 'owner/repository' slug" + ) + + def _reject_inline_secrets(kind, name, obj): for key in _INLINE_SECRET_KEYS: if key in obj: @@ -549,6 +576,7 @@ def _load_v2_contexts(data, path): forbidden = raw.get("forbidden_operations") or [] if not isinstance(allowed, list) or not isinstance(forbidden, list): raise ConfigError(f"profile '{name}' operation fields must be lists") + _validate_allowed_repositories(name, raw.get("allowed_repositories")) allowed_n = {_normalize_op("gitea", op, name) for op in allowed} forbidden_n = {_normalize_op("gitea", op, name) for op in forbidden} # Reviewer-identity deadlock rule (#100/#103) applies here unchanged. diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 4fa3e42..823b52b 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -1121,25 +1121,71 @@ import issue_claim_heartbeat # noqa: E402 import session_context_binding as session_ctx # noqa: E402 # #714 immutable session context +def _workspace_repository_slug(remote: str | None) -> str | None: + """Trusted ``owner/repository`` derived from the workspace git remote (#714). + + This is the only source the session binding accepts for repository identity: + it comes from the verified, workspace-aligned checkout rather than from any + caller-supplied tool argument. ``REMOTES`` is deliberately not consulted — + its ``org``/``repo`` entries are default *targets*, not an authorization + scope (#530). + """ + if not remote: + return None + parsed = remote_repo_guard.parse_org_repo_from_remote_url( + _local_git_remote_url(remote) + ) + if not parsed: + return None + return session_ctx.format_repository_slug(parsed[0], parsed[1]) + + +def _trusted_session_repository(profile: dict, remote: str | None) -> dict: + """Resolve and authorize the session repository from the verified workspace. + + Returns ``org`` / ``repository`` / ``reasons``. A profile's + ``allowed_repositories`` is an authorization boundary: the verified + workspace selects exactly one entry from it, and the session never binds to + the list itself nor switches between entries. + """ + slug = _workspace_repository_slug(remote) + scope = session_ctx.assess_repository_scope( + workspace_slug=slug, + allowed=session_ctx.declared_allowed_repositories(profile), + profile_name=profile.get("profile_name"), + ) + if scope.get("block"): + return {"org": None, "repository": None, "reasons": list(scope["reasons"])} + parts = session_ctx.parse_repository_slug(slug) if slug else None + if not parts: + return {"org": None, "repository": None, "reasons": []} + return {"org": parts[0], "repository": parts[1], "reasons": []} + + def _seed_session_context( *, profile: dict, remote: str | None, host: str | None, identity: str | None, - repository: str | None = None, - org: str | None = None, source: str = "seed", ) -> dict: - """Seed immutable session context once for the current process (#714).""" + """Seed immutable session context once for the current process (#714). + + Repository and organization are always derived from the verified workspace, + never from caller arguments, so every first-bind entry point (whoami, + runtime context, capability preflight, activation, mutation gate) pins the + same complete context. + """ expected = (profile.get("username") or "").strip() or None + trusted = _trusted_session_repository(profile, remote) return session_ctx.seed_session_context_if_unbound( profile_name=profile.get("profile_name") or "", remote=remote, host=host, identity=identity, - repository=repository, - org=org, + repository=trusted["repository"], + org=trusted["org"], role_kind=_profile_role_kind(profile), expected_username=expected, source=source, @@ -7621,8 +7667,12 @@ def _session_context_mutation_block( expected = (profile.get("username") or "").strip() or None remote_config = REMOTES.get(remote or "", {}) if remote else {} h = host or remote_config.get("host") - resolved_org = org or remote_config.get("org") - resolved_repo = repo or remote_config.get("repo") + # #714: repository identity comes from the verified workspace only. The + # caller-supplied org/repo are a *request target* to be checked against the + # binding — they must never establish, complete, or replace it. + trusted = _trusted_session_repository(profile, remote) + resolved_org = trusted["org"] + resolved_repo = trusted["repository"] identity = username # Legacy env-only profiles do not declare an expected identity. Their # first mutation still pins remote/host/repository, while existing audit @@ -7650,14 +7700,15 @@ def _session_context_mutation_block( ) reasons.extend(id_gate.get("reasons") or []) + # Repository scope denial (unauthorized workspace) fails closed here. + reasons.extend(trusted.get("reasons") or []) + # Seed if unbound so subsequent tools share one context; then assess. _seed_session_context( profile=profile, remote=remote, host=h, identity=identity, - repository=resolved_repo, - org=resolved_org, source="mutation-gate-seed", ) ctx_gate = session_ctx.assess_session_context( @@ -7669,8 +7720,21 @@ def _session_context_mutation_block( org=resolved_org, expected_username=expected, require_bound=True, + require_complete=bool( + session_ctx.declared_allowed_repositories(profile) + ), ) reasons.extend(ctx_gate.get("reasons") or []) + + # The request may only be checked against the immutable binding (#714). + bound_ctx = session_ctx.get_session_context() or {} + override_gate = session_ctx.assess_repository_override( + requested_org=org, + requested_repo=repo, + bound_org=bound_ctx.get("org"), + bound_repo=bound_ctx.get("repository"), + ) + reasons.extend(override_gate.get("reasons") or []) # De-dupe while preserving order seen: set[str] = set() uniq: list[str] = [] @@ -10835,11 +10899,31 @@ def gitea_activate_profile( os.environ[SESSION_PROFILE_LOCK_ENV] = after_profile # 4.6 #714: re-bind immutable session context after explicit activation. + # Activation is the sanctioned logical-session boundary, so it establishes + # the complete binding — including the workspace-verified repository. An + # unauthorized workspace fails closed here rather than at first mutation. + activated_trusted = _trusted_session_repository(after_profile_data, remote) + if activated_trusted.get("reasons"): + return { + "success": False, + "performed": False, + "reasons": list(activated_trusted["reasons"]), + "blocker_kind": "repository_scope", + "exact_next_action": ( + "BLOCKED + DIAGNOSE: the verified workspace repository is not " + "authorized by this profile's allowed_repositories. Run from a " + "workspace whose git remote matches an authorized " + "owner/repository, or have the operator provision the profile " + "scope. Do not pass org/repo to bypass this gate." + ), + } session_ctx.bind_session_context( profile_name=after_profile or profile_name, remote=remote, host=h, identity=after_identity, + repository=activated_trusted["repository"], + org=activated_trusted["org"], role_kind=_profile_role_kind(after_profile_data), expected_username=expected_username, source="gitea_activate_profile", diff --git a/session_context_binding.py b/session_context_binding.py index 57f2f1c..2336449 100644 --- a/session_context_binding.py +++ b/session_context_binding.py @@ -11,6 +11,7 @@ substitution is forbidden. from __future__ import annotations import os +import re import threading import urllib.parse from dataclasses import dataclass @@ -232,12 +233,16 @@ def assess_session_context( org: str | None = None, expected_username: str | None = None, require_bound: bool = False, + require_complete: bool = False, ) -> dict[str, Any]: """Compare live values against the bound session context. Returns ``proven`` / ``block`` / ``reasons``. When unbound and ``require_bound`` is false, does not block (caller may seed). When - unbound and ``require_bound`` is true, fails closed. + unbound and ``require_bound`` is true, fails closed. ``require_complete`` + additionally fails closed when the binding carries no verified + repository/organization identity, so a mutation can never run against an + unknown repository (#714). """ reasons: list[str] = [] with _SESSION_CONTEXT_LOCK: @@ -252,6 +257,15 @@ def assess_session_context( return _assessment(False, reasons, ctx) return _assessment(True, reasons, ctx) + if require_complete and not (ctx.get("repository") and ctx.get("org")): + reasons.append( + "session repository/organization identity is unverified " + f"(repository={ctx.get('repository')!r}, org={ctx.get('org')!r}); " + "a mutation cannot proceed without a verified workspace " + "repository (fail closed)" + ) + return _assessment(False, reasons, ctx) + live_profile = (profile_name or "").strip() or None live_remote = (remote or "").strip() or None live_host = (host or "").strip().lower() or None @@ -327,6 +341,123 @@ def assess_identity_match( } +_REPO_SLUG_RE = re.compile(r"^\s*(?P[^/\s]+)\s*/\s*(?P[^/\s]+?)(?:\.git)?\s*$") + + +def parse_repository_slug(slug: str | None) -> tuple[str, str] | None: + """Split a canonical ``owner/repository`` slug, or None when unparseable.""" + match = _REPO_SLUG_RE.match(slug or "") + if not match: + return None + return match.group("org"), match.group("repo") + + +def format_repository_slug(org: str | None, repo: str | None) -> str | None: + """``owner/repository`` from parts, or None when either side is missing.""" + left = (org or "").strip() + right = (repo or "").strip() + if not left or not right: + return None + return f"{left}/{right}" + + +def declared_allowed_repositories(profile: Mapping[str, Any] | None) -> list[str]: + """Canonical ``owner/repository`` authorization boundary declared by *profile*. + + This list is an authorization boundary, never the session binding itself: + the verified workspace selects exactly one entry (see + :func:`assess_repository_scope`). Returns ``[]`` when the profile declares + no scope — enforcement is opt-in per profile so existing static-profile + namespaces stay functional until an operator provisions the field. + """ + if not profile: + return [] + raw = profile.get("allowed_repositories") + if not isinstance(raw, (list, tuple)): + return [] + slugs: list[str] = [] + for entry in raw: + if not isinstance(entry, str): + continue + parsed = parse_repository_slug(entry) + if parsed: + slugs.append(f"{parsed[0]}/{parsed[1]}") + return slugs + + +def assess_repository_scope( + *, + workspace_slug: str | None, + allowed: list[str] | None, + profile_name: str | None = None, +) -> dict[str, Any]: + """Authorize the verified workspace repository against the profile allowlist. + + The workspace-derived slug is the only candidate: a profile that authorizes + several repositories still binds to the single verified one, and never to + the list as a whole. + """ + scope = list(allowed or []) + reasons: list[str] = [] + if not scope: + return { + "proven": True, + "block": False, + "reasons": reasons, + "scope_enforced": False, + "workspace_slug": workspace_slug, + "allowed_repositories": [], + } + name = profile_name or "(active profile)" + if not workspace_slug: + reasons.append( + f"no verified workspace repository could be established for profile " + f"'{name}'; repository scope cannot be authorized (fail closed)" + ) + elif workspace_slug.lower() not in {entry.lower() for entry in scope}: + reasons.append( + f"repository scope denial: workspace repository '{workspace_slug}' " + f"is not authorized by profile '{name}' allowed_repositories " + f"{sorted(scope)} (fail closed)" + ) + return { + "proven": not reasons, + "block": bool(reasons), + "reasons": reasons, + "scope_enforced": True, + "workspace_slug": workspace_slug, + "allowed_repositories": sorted(scope), + } + + +def assess_repository_override( + *, + requested_org: str | None, + requested_repo: str | None, + bound_org: str | None, + bound_repo: str | None, +) -> dict[str, Any]: + """Caller-supplied org/repo must agree with the immutable binding. + + A mutation request is never allowed to establish, complete, or replace the + binding — it may only be checked against it. + """ + reasons: list[str] = [] + req_org = (requested_org or "").strip() or None + req_repo = (requested_repo or "").strip() or None + if req_repo and bound_repo and req_repo.lower() != bound_repo.lower(): + reasons.append( + f"repository override denial: request targets '{req_repo}' but the " + f"session is bound to '{bound_repo}' (fail closed)" + ) + if req_org and bound_org and req_org.lower() != bound_org.lower(): + reasons.append( + f"organization override denial: request targets '{req_org}' but the " + f"session is bound to '{bound_org}' (fail closed)" + ) + return {"proven": not reasons, "block": bool(reasons), "reasons": reasons} + + def filter_profiles_for_remote( config: dict | None, remote: str | None, diff --git a/tests/test_issue_714_production_first_bind.py b/tests/test_issue_714_production_first_bind.py new file mode 100644 index 0000000..28e20d0 --- /dev/null +++ b/tests/test_issue_714_production_first_bind.py @@ -0,0 +1,448 @@ +"""#714: production first-bind pins the workspace-verified repository scope. + +These tests drive the real entry points (``gitea_whoami``, +``gitea_get_runtime_context``, ``gitea_resolve_task_capability``, +``gitea_activate_profile``) in production order. They deliberately do not +construct a ``_SessionContext`` directly: the defect they cover is that the +production first-bind path left ``repository``/``org`` unbound, so +``assess_session_context`` skipped its repository/org drift checks and a +same-host mutation against another repository was not blocked. + +The trusted repository identity comes from the workspace-aligned git remote +(``Scaled-Tech-Consulting/Gitea-Tools`` for this checkout), never from +``REMOTES`` (whose ``prgs`` default repo is ``Timesheet``) and never from a +caller-supplied ``org``/``repo`` argument. +""" + +from __future__ import annotations + +import json +import os +import sys +import tempfile +import threading +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import gitea_config # noqa: E402 +import gitea_mcp_server as srv # noqa: E402 +import mcp_server # noqa: E402 +import session_context_binding as session_ctx # noqa: E402 + +WORKSPACE_ORG = "Scaled-Tech-Consulting" +WORKSPACE_REPO = "Gitea-Tools" +WORKSPACE_SLUG = f"{WORKSPACE_ORG}/{WORKSPACE_REPO}" +WORKSPACE_URL = f"https://gitea.prgs.cc/{WORKSPACE_SLUG}.git" + +_BASE_AUTHOR_OPS = [ + "gitea.read", + "gitea.issue.create", + "gitea.issue.comment", + "gitea.pr.create", + "gitea.pr.comment", + "gitea.branch.push", + "gitea.repo.commit", +] + + +def _config(allowed_repositories=None): + profile = { + "enabled": True, + "context": "prgs", + "role": "author", + "username": "jcwalker3", + "base_url": "https://gitea.prgs.cc", + "auth": {"type": "env", "name": "GITEA_TOKEN_PRGS_AUTHOR"}, + "allowed_operations": list(_BASE_AUTHOR_OPS), + "forbidden_operations": [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.request_changes", + ], + "execution_profile": "prgs-author", + } + if allowed_repositories is not None: + profile["allowed_repositories"] = list(allowed_repositories) + return { + "version": 2, + # v2-contexts normalizes the config and keeps only rules.* — a + # top-level allow_runtime_switching would be dropped. + "rules": {"allow_runtime_switching": True}, + "contexts": { + "prgs": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.prgs.cc"}, + }, + "mdcps": { + "enabled": True, + "gitea": { + "enabled": True, + "base_url": "https://gitea.dadeschools.net", + }, + }, + }, + "profiles": {"prgs-author": profile}, + } + + +class _ProductionOrderBase(unittest.TestCase): + """Real entry points, pinned workspace remote, mocked network only.""" + + allowed_repositories = [WORKSPACE_SLUG] + + def setUp(self): + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(_config(self.allowed_repositories))) + self._env = { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": "prgs-author", + "GITEA_TOKEN_PRGS_AUTHOR": "prgs-author-token", + } + gitea_config._active_profile_override = None + mcp_server._IDENTITY_CACHE.clear() + srv._MUTATION_AUTHORITY = None + # Pin the workspace git remote so the trusted source is deterministic + # and never depends on the developer's checkout layout. + self._remote_url = patch.object( + srv, "_local_git_remote_url", side_effect=self._local_remote_url + ) + self._remote_url.start() + + def tearDown(self): + self._remote_url.stop() + gitea_config._active_profile_override = None + mcp_server._IDENTITY_CACHE.clear() + srv._MUTATION_AUTHORITY = None + self._dir.cleanup() + + def _local_remote_url(self, remote_name): + return WORKSPACE_URL if remote_name == "prgs" else None + + def _api(self, method, url, header): + return { + "login": "jcwalker3", + "full_name": "Test", + "id": 1, + "email": "t@example.com", + } + + def _live(self): + return patch("gitea_mcp_server.api_request", side_effect=self._api) + + +class TestProductionFirstBindPinsRepository(_ProductionOrderBase): + def test_whoami_first_bind_is_complete(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + ctx = session_ctx.get_session_context() + self.assertIsNotNone(ctx) + self.assertEqual(ctx["org"], WORKSPACE_ORG) + self.assertEqual(ctx["repository"], WORKSPACE_REPO) + self.assertEqual(ctx["remote"], "prgs") + self.assertEqual(ctx["host"], "gitea.prgs.cc") + + def test_runtime_context_first_bind_is_complete(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_get_runtime_context(remote="prgs") + ctx = session_ctx.get_session_context() + self.assertEqual(ctx["org"], WORKSPACE_ORG) + self.assertEqual(ctx["repository"], WORKSPACE_REPO) + + def test_capability_preflight_first_bind_is_complete(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_resolve_task_capability(task="comment_issue", remote="prgs") + ctx = session_ctx.get_session_context() + self.assertEqual(ctx["org"], WORKSPACE_ORG) + self.assertEqual(ctx["repository"], WORKSPACE_REPO) + + def test_activate_profile_binds_complete_context(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_activate_profile(profile_name="prgs-author", remote="prgs") + ctx = session_ctx.get_session_context() + self.assertEqual(ctx["org"], WORKSPACE_ORG) + self.assertEqual(ctx["repository"], WORKSPACE_REPO) + + +class TestProductionOrderRepositoryDriftBlocks(_ProductionOrderBase): + def test_whoami_then_same_host_other_repository_blocks(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + blocked = srv._session_context_mutation_block( + remote="prgs", org=WORKSPACE_ORG, repo="Other-Tools" + ) + self.assertIsNotNone(blocked) + self.assertTrue( + any("Other-Tools" in r for r in blocked["reasons"]), blocked["reasons"] + ) + + def test_whoami_then_timesheet_blocks(self): + """REMOTES['prgs'].repo is Timesheet — a default target, not a scope.""" + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + blocked = srv._session_context_mutation_block( + remote="prgs", org=WORKSPACE_ORG, repo="Timesheet" + ) + self.assertIsNotNone(blocked) + self.assertTrue( + any("Timesheet" in r for r in blocked["reasons"]), blocked["reasons"] + ) + + def test_whoami_then_same_host_other_org_blocks(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + blocked = srv._session_context_mutation_block( + remote="prgs", org="Other-Org", repo=WORKSPACE_REPO + ) + self.assertIsNotNone(blocked) + self.assertTrue( + any("Other-Org" in r for r in blocked["reasons"]), blocked["reasons"] + ) + + def test_runtime_context_then_other_repository_blocks(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_get_runtime_context(remote="prgs") + blocked = srv._session_context_mutation_block( + remote="prgs", org=WORKSPACE_ORG, repo="Other-Tools" + ) + self.assertIsNotNone(blocked) + + def test_capability_preflight_then_other_repository_blocks(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_resolve_task_capability(task="comment_issue", remote="prgs") + blocked = srv._session_context_mutation_block( + remote="prgs", org=WORKSPACE_ORG, repo="Other-Tools" + ) + self.assertIsNotNone(blocked) + + def test_activate_profile_then_other_repository_blocks(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_activate_profile(profile_name="prgs-author", remote="prgs") + blocked = srv._session_context_mutation_block( + remote="prgs", org="Other-Org", repo="Other-Tools" + ) + self.assertIsNotNone(blocked) + + def test_cross_host_still_blocks(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + blocked = srv._session_context_mutation_block(remote="dadeschools") + self.assertIsNotNone(blocked) + self.assertTrue( + any("cross-host" in r for r in blocked["reasons"]), blocked["reasons"] + ) + + def test_authorized_repository_mutation_remains_allowed(self): + """Normal PR #715 author comment/push path must stay functional.""" + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + self.assertIsNone( + srv._session_context_mutation_block( + remote="prgs", org=WORKSPACE_ORG, repo=WORKSPACE_REPO + ) + ) + # A bare call with no override resolves to the same bound scope. + self.assertIsNone(srv._session_context_mutation_block(remote="prgs")) + + +class TestMutationRequestCannotEstablishBinding(_ProductionOrderBase): + def test_caller_values_cannot_establish_binding_on_first_mutation(self): + """A mutation-first session must not be pinned by request values.""" + with patch.dict(os.environ, self._env, clear=False), self._live(): + self.assertIsNone(session_ctx.get_session_context()) + srv._session_context_mutation_block( + remote="prgs", org="Attacker-Org", repo="Attacker-Repo" + ) + ctx = session_ctx.get_session_context() + self.assertIsNotNone(ctx) + # Bound to the verified workspace, never to the request values. + self.assertEqual(ctx["org"], WORKSPACE_ORG) + self.assertEqual(ctx["repository"], WORKSPACE_REPO) + + def test_mutation_first_with_attacker_values_is_blocked(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + blocked = srv._session_context_mutation_block( + remote="prgs", org="Attacker-Org", repo="Attacker-Repo" + ) + self.assertIsNotNone(blocked) + + def test_later_call_cannot_overwrite_bound_repository(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + before = session_ctx.get_session_context() + for _ in range(3): + srv._session_context_mutation_block( + remote="prgs", org="Other-Org", repo="Other-Tools" + ) + self.assertEqual(session_ctx.get_session_context(), before) + + +class TestUnverifiedWorkspaceFailsClosed(_ProductionOrderBase): + def _local_remote_url(self, remote_name): + return None # no verifiable workspace repository + + def test_mutation_blocks_when_workspace_repository_unverified(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + ctx = session_ctx.get_session_context() + self.assertIsNone(ctx["repository"]) + blocked = srv._session_context_mutation_block( + remote="prgs", org=WORKSPACE_ORG, repo=WORKSPACE_REPO + ) + self.assertIsNotNone(blocked) + self.assertTrue( + any( + "no verified workspace repository" in r or "unverified" in r + for r in blocked["reasons"] + ), + blocked["reasons"], + ) + + def test_activate_profile_rejects_unverifiable_workspace(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + res = srv.gitea_activate_profile(profile_name="prgs-author", remote="prgs") + self.assertFalse(res.get("success", True)) + self.assertEqual(res.get("blocker_kind"), "repository_scope") + + +class TestUnauthorizedWorkspaceFailsClosed(_ProductionOrderBase): + allowed_repositories = ["Scaled-Tech-Consulting/Some-Other-Project"] + + def test_workspace_absent_from_allowlist_blocks_mutation(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + blocked = srv._session_context_mutation_block(remote="prgs") + self.assertIsNotNone(blocked) + self.assertTrue( + any("not authorized" in r for r in blocked["reasons"]), + blocked["reasons"], + ) + + def test_workspace_absent_from_allowlist_blocks_activation(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + res = srv.gitea_activate_profile(profile_name="prgs-author", remote="prgs") + self.assertFalse(res.get("success", True)) + self.assertEqual(res.get("blocker_kind"), "repository_scope") + + +class TestTimesheetNotAuthorizedInThisPhase(_ProductionOrderBase): + """Cross-project use is paused: only Gitea-Tools is authorized.""" + + def test_timesheet_workspace_is_rejected(self): + def timesheet_remote(remote_name): + return "https://gitea.prgs.cc/Scaled-Tech-Consulting/Timesheet.git" + + with patch.dict(os.environ, self._env, clear=False), self._live(): + with patch.object( + srv, "_local_git_remote_url", side_effect=timesheet_remote + ): + blocked = srv._session_context_mutation_block(remote="prgs") + self.assertIsNotNone(blocked) + self.assertTrue( + any("not authorized" in r for r in blocked["reasons"]), + blocked["reasons"], + ) + + +class TestConcurrentFirstBindSelectsOneRepository(_ProductionOrderBase): + def test_concurrent_initialization_cannot_change_selected_repository(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + self.assertIsNone(session_ctx.get_session_context()) + thread_count = 8 + barrier = threading.Barrier(thread_count) + results = [] + lock = threading.Lock() + + def racer(index: int) -> None: + barrier.wait() + srv._session_context_mutation_block( + remote="prgs", org=f"Racer-Org-{index}", repo=f"Racer-Repo-{index}" + ) + with lock: + results.append(session_ctx.get_session_context()) + + threads = [ + threading.Thread(target=racer, args=(i,)) for i in range(thread_count) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=10) + + self.assertFalse(any(t.is_alive() for t in threads)) + winner = session_ctx.get_session_context() + self.assertEqual(winner["org"], WORKSPACE_ORG) + self.assertEqual(winner["repository"], WORKSPACE_REPO) + self.assertEqual(results, [winner] * thread_count) + + +class TestRepositoryScopeUnits(unittest.TestCase): + def test_absent_scope_is_not_enforced(self): + scope = session_ctx.assess_repository_scope( + workspace_slug=WORKSPACE_SLUG, allowed=[], profile_name="legacy" + ) + self.assertTrue(scope["proven"]) + self.assertFalse(scope["scope_enforced"]) + + def test_declared_scope_authorizes_only_listed_repository(self): + allowed = session_ctx.declared_allowed_repositories( + {"allowed_repositories": [WORKSPACE_SLUG]} + ) + self.assertEqual(allowed, [WORKSPACE_SLUG]) + self.assertTrue( + session_ctx.assess_repository_scope( + workspace_slug=WORKSPACE_SLUG, allowed=allowed + )["proven"] + ) + self.assertTrue( + session_ctx.assess_repository_scope( + workspace_slug="Scaled-Tech-Consulting/Timesheet", allowed=allowed + )["block"] + ) + + def test_missing_workspace_slug_blocks_when_scope_declared(self): + scope = session_ctx.assess_repository_scope( + workspace_slug=None, allowed=[WORKSPACE_SLUG] + ) + self.assertTrue(scope["block"]) + + def test_override_must_match_binding(self): + self.assertTrue( + session_ctx.assess_repository_override( + requested_org=WORKSPACE_ORG, + requested_repo="Other", + bound_org=WORKSPACE_ORG, + bound_repo=WORKSPACE_REPO, + )["block"] + ) + self.assertTrue( + session_ctx.assess_repository_override( + requested_org="Other-Org", + requested_repo=WORKSPACE_REPO, + bound_org=WORKSPACE_ORG, + bound_repo=WORKSPACE_REPO, + )["block"] + ) + self.assertTrue( + session_ctx.assess_repository_override( + requested_org=WORKSPACE_ORG, + requested_repo=WORKSPACE_REPO, + bound_org=WORKSPACE_ORG, + bound_repo=WORKSPACE_REPO, + )["proven"] + ) + + def test_malformed_allowlist_entries_are_ignored(self): + self.assertEqual( + session_ctx.declared_allowed_repositories( + {"allowed_repositories": ["not-a-slug", 17, None, WORKSPACE_SLUG]} + ), + [WORKSPACE_SLUG], + ) + + +if __name__ == "__main__": + unittest.main() From dc899d23c85f879dfa5b2e03c376112f1ea8382a Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Wed, 15 Jul 2026 21:13:21 -0400 Subject: [PATCH 5/5] fix(session): require config-backed mutation authority and workspace-bound targets (#714) Close the #714 remediation gap left at 943d402 where env-only mutation profiles, REMOTES Timesheet defaults treated as explicit caller input, and machine-dependent Git remotes produced false greens. - Fail closed when mutations lack a config-backed profile with non-empty allowed_repositories; env ops cannot invent mutation authority. - Preserve omitted-vs-explicit org/repo provenance through the mutation gate; omitted targets bind to the verified workspace repository. - Prefer workspace-aligned Git remotes over historical Timesheet defaults in MCP _resolve and CLI resolve_remote. - Centralize deterministic config-backed mutation fixtures; migrate mutation tests off env-only authority without weakening security assertions. Full suite: 2744 passed, 6 skipped. --- gitea_auth.py | 46 +- gitea_mcp_server.py | 308 +++++++++-- session_context_binding.py | 59 ++- tests/conftest.py | 14 + tests/mutation_profile_fixture.py | 483 ++++++++++++++++++ tests/test_agent_temp_artifacts.py | 12 +- tests/test_assess_conflict_fix_push_tool.py | 4 + tests/test_audit.py | 62 ++- tests/test_commit_files_capability.py | 3 + tests/test_commit_files_gate.py | 2 + tests/test_commit_payloads.py | 1 + tests/test_config.py | 4 + tests/test_create_issue.py | 25 +- tests/test_create_pr.py | 25 +- tests/test_issue_714_production_first_bind.py | 67 ++- ..._issue_714_session_context_immutability.py | 12 + tests/test_issue_comment_workspace_guard.py | 4 + tests/test_issue_content_gate.py | 11 +- tests/test_issue_duplicate_gate.py | 13 +- tests/test_issue_work_duplicate_gate.py | 29 +- tests/test_issue_write_tool_gates.py | 6 + tests/test_llm_agent_sha.py | 4 + tests/test_lock_issue_mcp_registration.py | 26 +- tests/test_mcp_server.py | 252 +++++---- tests/test_merger_lease_adoption.py | 4 + tests/test_op_normalization.py | 4 + tests/test_operation_scoped_roles.py | 16 + tests/test_operator_guide.py | 4 + tests/test_post_merge_moot_lease.py | 4 + .../test_pr_lease_comments_non_list_guard.py | 4 + tests/test_remote_repo_guard.py | 14 +- tests/test_role_namespace_gate.py | 11 +- tests/test_role_session_router.py | 4 + tests/test_runtime_clarity.py | 9 +- ...test_stale_review_decision_lock_cleanup.py | 16 +- tests/test_workflow_skill.py | 8 +- 36 files changed, 1343 insertions(+), 227 deletions(-) create mode 100644 tests/mutation_profile_fixture.py diff --git a/gitea_auth.py b/gitea_auth.py index 385db61..31bdf6d 100644 --- a/gitea_auth.py +++ b/gitea_auth.py @@ -182,11 +182,51 @@ def get_auth_header(host): def resolve_remote(args): """Given parsed argparse args with --remote/--host/--org/--repo, - return (host, org, repo) with overrides applied.""" + return (host, org, repo) with overrides applied. + + #714 / #530: when the caller omits org and/or repo, prefer the + workspace-aligned git remote over REMOTES defaults (e.g. bare + ``--remote prgs`` must not force Timesheet when the checkout is + Gitea-Tools). Explicit --org/--repo always win. + """ profile = REMOTES[args.remote] host = args.host or profile["host"] - org = args.org or profile["org"] - repo = args.repo or profile["repo"] + org_explicit = getattr(args, "org", None) is not None + repo_explicit = getattr(args, "repo", None) is not None + org = args.org if org_explicit else profile["org"] + repo = args.repo if repo_explicit else profile["repo"] + if not org_explicit or not repo_explicit: + try: + import remote_repo_guard + import subprocess + # Prefer the named remote URL when present; fall back to origin. + url = None + for remote_name in (args.remote, "origin"): + try: + proc = subprocess.run( + ["git", "remote", "get-url", remote_name], + capture_output=True, + text=True, + timeout=5, + check=False, + ) + if proc.returncode == 0 and (proc.stdout or "").strip(): + url = proc.stdout.strip() + break + except Exception: + continue + # Allow tests to inject a deterministic remote URL without Git. + env_url = os.environ.get("GITEA_TEST_WORKSPACE_REMOTE_URL") + if env_url: + url = env_url + parsed = remote_repo_guard.parse_org_repo_from_remote_url(url) + if parsed: + if not org_explicit: + org = parsed[0] + if not repo_explicit: + repo = parsed[1] + except Exception: + pass return host, org, repo diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 823b52b..b39b571 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -1140,24 +1140,50 @@ def _workspace_repository_slug(remote: str | None) -> str | None: return session_ctx.format_repository_slug(parsed[0], parsed[1]) -def _trusted_session_repository(profile: dict, remote: str | None) -> dict: +def _trusted_session_repository( + profile: dict, + remote: str | None, + *, + for_mutation: bool = False, +) -> dict: """Resolve and authorize the session repository from the verified workspace. Returns ``org`` / ``repository`` / ``reasons``. A profile's ``allowed_repositories`` is an authorization boundary: the verified workspace selects exactly one entry from it, and the session never binds to the list itself nor switches between entries. + + *for_mutation*: require a non-empty configured allowlist and a complete + workspace-derived identity (fail closed; no self-authorization). """ + reasons: list[str] = [] + try: + allowed = session_ctx.declared_allowed_repositories( + profile, strict=for_mutation + ) + except ValueError as exc: + return { + "org": None, + "repository": None, + "reasons": [str(exc)], + } slug = _workspace_repository_slug(remote) scope = session_ctx.assess_repository_scope( workspace_slug=slug, - allowed=session_ctx.declared_allowed_repositories(profile), + allowed=allowed, profile_name=profile.get("profile_name"), + require_scope=for_mutation, ) if scope.get("block"): return {"org": None, "repository": None, "reasons": list(scope["reasons"])} parts = session_ctx.parse_repository_slug(slug) if slug else None if not parts: + if for_mutation: + reasons.append( + "mutation denied: no verified workspace repository identity " + "could be established (fail closed)" + ) + return {"org": None, "repository": None, "reasons": reasons} return {"org": None, "repository": None, "reasons": []} return {"org": parts[0], "repository": parts[1], "reasons": []} @@ -1774,20 +1800,45 @@ def _effective_remote(remote: str) -> str: def _resolve(remote: str, host: str | None, org: str | None, repo: str | None): - """Resolve remote + overrides to (host, org, repo).""" + """Resolve remote + overrides to (host, org, repo). + + #714 / #530: when the caller omits org and/or repo, prefer the + workspace-aligned git remote over ``REMOTES`` defaults (e.g. bare + ``remote=prgs`` must not force ``Timesheet`` when the checkout is + ``Gitea-Tools``). Explicit caller org/repo always win and are validated + by the remote/repo guard. Provenance of omitted-vs-explicit is preserved + at mutation gates by passing the original caller values (not these + resolved defaults) into override checks. + """ remote = _effective_remote(remote) if remote not in REMOTES: raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}") profile = REMOTES[remote] + org_explicit = org is not None + repo_explicit = repo is not None resolved_host = host or profile["host"] - resolved_org = org or profile["org"] - resolved_repo = repo or profile["repo"] + resolved_org = org if org_explicit else profile["org"] + resolved_repo = repo if repo_explicit else profile["repo"] + filled_org = False + filled_repo = False + if not org_explicit or not repo_explicit: + parsed = remote_repo_guard.parse_org_repo_from_remote_url( + _local_git_remote_url(remote) + ) + if parsed: + if not org_explicit: + resolved_org = parsed[0] + filled_org = True + if not repo_explicit: + resolved_repo = parsed[1] + filled_repo = True _enforce_remote_repo_guard( remote, resolved_org, resolved_repo, - org_explicit=org is not None, - repo_explicit=repo is not None, + # Workspace-filled sides are intentional alignment for #530. + org_explicit=org_explicit or filled_org, + repo_explicit=repo_explicit or filled_repo, ) return (resolved_host, resolved_org, resolved_repo) @@ -2145,6 +2196,8 @@ def gitea_create_issue( host=h, org=o, repo=r, + org_explicit=org is not None, + repo_explicit=repo is not None, ) if blocked: return blocked @@ -2321,7 +2374,15 @@ def gitea_lock_issue( ) blocked = _profile_permission_block( - task_capability_map.required_permission("lock_issue")) + task_capability_map.required_permission("lock_issue"), + issue_number=issue_number, + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked @@ -2589,6 +2650,8 @@ def gitea_create_pr( host=host, org=org, repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, ) if blocked: return blocked @@ -5426,6 +5489,8 @@ def gitea_commit_files( blocked = _profile_permission_block( task_capability_map.required_permission("commit_files"), commit="", branch="", remote=remote, host=host, org=org, repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, ) if blocked: return blocked @@ -7343,7 +7408,15 @@ def gitea_close_issue( dict with 'success' boolean and 'message'. """ blocked = _profile_permission_block( - task_capability_map.required_permission("close_issue")) + task_capability_map.required_permission("close_issue"), + issue_number=issue_number, + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked verify_preflight_purity(remote, task="close_issue") @@ -7641,6 +7714,81 @@ def _profile_operation_gate(op: str) -> list[str]: return [f"profile is not allowed to {op}"] +def _mutation_config_authority_block(required_operation: str) -> dict | None: + """#714: mutations require config-backed profile + non-empty repository scope. + + Environment-only profiles may support read-only diagnostics but cannot + authorize writes. Env vars may narrow operations on a configured profile + but cannot invent mutation authority or repository scope. + """ + if required_operation == "gitea.read": + return None + try: + profile = get_profile() + except Exception as exc: + return { + "success": False, + "performed": False, + "reasons": [ + f"profile could not be resolved (fail closed): {_redact(str(exc))}" + ], + "blocker_kind": "mutation_authority", + } + config = None + try: + config = gitea_config.load_config() + except Exception as exc: + return { + "success": False, + "performed": False, + "reasons": [ + f"profiles configuration unreadable (fail closed): {_redact(str(exc))}" + ], + "blocker_kind": "mutation_authority", + } + if not config: + return { + "success": False, + "performed": False, + "reasons": [ + "mutation denied: env-only / unconfigured profiles cannot " + "authorize mutations; provision GITEA_MCP_CONFIG with a v2 " + "profile that declares non-empty allowed_repositories " + "(fail closed)" + ], + "blocker_kind": "mutation_authority", + "exact_next_action": ( + "BLOCKED + DIAGNOSE: configure a trusted v2 profile with " + "allowed_repositories and relaunch; do not widen scope via env." + ), + } + try: + allowed = session_ctx.declared_allowed_repositories(profile, strict=True) + except ValueError as exc: + return { + "success": False, + "performed": False, + "reasons": [str(exc)], + "blocker_kind": "mutation_authority", + } + if not allowed: + return { + "success": False, + "performed": False, + "reasons": [ + f"mutation denied: profile '{profile.get('profile_name')}' " + "must declare a non-empty allowed_repositories list " + "(fail closed)" + ], + "blocker_kind": "mutation_authority", + "exact_next_action": ( + "BLOCKED + DIAGNOSE: add allowed_repositories: " + "[\"Owner/Repo\"] to the active v2 profile and restart." + ), + } + return None + + def _session_context_mutation_block( *, remote: str | None, @@ -7665,19 +7813,23 @@ def _session_context_mutation_block( } profile_name = profile.get("profile_name") expected = (profile.get("username") or "").strip() or None + # Infer remote from profile host when tools omit it (legacy call sites). + if not remote: + p_host = session_ctx.profile_host(profile) + if p_host: + for rname, rcfg in REMOTES.items(): + if (rcfg.get("host") or "").lower() == p_host: + remote = rname + break remote_config = REMOTES.get(remote or "", {}) if remote else {} - h = host or remote_config.get("host") + h = host or remote_config.get("host") or session_ctx.profile_host(profile) # #714: repository identity comes from the verified workspace only. The # caller-supplied org/repo are a *request target* to be checked against the # binding — they must never establish, complete, or replace it. - trusted = _trusted_session_repository(profile, remote) + trusted = _trusted_session_repository(profile, remote, for_mutation=True) resolved_org = trusted["org"] resolved_repo = trusted["repository"] identity = username - # Legacy env-only profiles do not declare an expected identity. Their - # first mutation still pins remote/host/repository, while existing audit - # paths resolve identity at the actual write. Configured identities are - # always verified here before a mutation can proceed. if identity is None and expected and h: try: identity = _authenticated_username(h) @@ -7699,18 +7851,16 @@ def _session_context_mutation_block( authenticated=identity, expected_username=expected ) reasons.extend(id_gate.get("reasons") or []) - - # Repository scope denial (unauthorized workspace) fails closed here. reasons.extend(trusted.get("reasons") or []) - # Seed if unbound so subsequent tools share one context; then assess. - _seed_session_context( - profile=profile, - remote=remote, - host=h, - identity=identity, - source="mutation-gate-seed", - ) + if trusted.get("repository") and trusted.get("org"): + _seed_session_context( + profile=profile, + remote=remote, + host=h, + identity=identity, + source="mutation-gate-seed", + ) ctx_gate = session_ctx.assess_session_context( profile_name=profile_name, remote=remote, @@ -7720,19 +7870,37 @@ def _session_context_mutation_block( org=resolved_org, expected_username=expected, require_bound=True, - require_complete=bool( - session_ctx.declared_allowed_repositories(profile) - ), + require_complete=True, ) reasons.extend(ctx_gate.get("reasons") or []) - # The request may only be checked against the immutable binding (#714). + # Provenance: only explicit caller org/repo may override the binding. + # Tools must pass org_explicit/repo_explicit (True when the caller supplied + # the parameter). When flags are absent (legacy), strip pure REMOTES + # defaults that disagree with the binding so omitted targets use it. bound_ctx = session_ctx.get_session_context() or {} + bound_org = bound_ctx.get("org") or resolved_org + bound_repo = bound_ctx.get("repository") or resolved_repo + org_explicit = extra_fields.get("org_explicit") + repo_explicit = extra_fields.get("repo_explicit") + req_org = (org or "").strip() or None + req_repo = (repo or "").strip() or None + remotes_entry = REMOTES.get(remote or "", {}) if remote else {} + default_org = (remotes_entry.get("org") or "").strip() or None + default_repo = (remotes_entry.get("repo") or "").strip() or None + if org_explicit is False: + req_org = None + elif org_explicit is None and req_org and default_org and req_org.lower() == default_org.lower() and bound_org and req_org.lower() != bound_org.lower(): + req_org = None + if repo_explicit is False: + req_repo = None + elif repo_explicit is None and req_repo and default_repo and req_repo.lower() == default_repo.lower() and bound_repo and req_repo.lower() != bound_repo.lower(): + req_repo = None override_gate = session_ctx.assess_repository_override( - requested_org=org, - requested_repo=repo, - bound_org=bound_ctx.get("org"), - bound_repo=bound_ctx.get("repository"), + requested_org=req_org, + requested_repo=req_repo, + bound_org=bound_org, + bound_repo=bound_repo, ) reasons.extend(override_gate.get("reasons") or []) # De-dupe while preserving order @@ -7786,6 +7954,13 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict | blocked.update(extra_fields) return blocked + auth_block = _mutation_config_authority_block(required_operation) + if auth_block is not None: + for k in ("number", "issue_number", "pr_number", "remote", "host", "org", "repo"): + if k in extra_fields: + auth_block[k] = extra_fields[k] + return auth_block + return _session_context_mutation_block( remote=extra_fields.get("remote"), host=extra_fields.get("host"), @@ -7794,6 +7969,8 @@ def _profile_permission_block(required_operation: str, **extra_fields) -> dict | number=extra_fields.get("number"), issue_number=extra_fields.get("issue_number"), pr_number=extra_fields.get("pr_number"), + org_explicit=extra_fields.get("org_explicit"), + repo_explicit=extra_fields.get("repo_explicit"), ) @@ -10902,7 +11079,7 @@ def gitea_activate_profile( # Activation is the sanctioned logical-session boundary, so it establishes # the complete binding — including the workspace-verified repository. An # unauthorized workspace fails closed here rather than at first mutation. - activated_trusted = _trusted_session_repository(after_profile_data, remote) + activated_trusted = _trusted_session_repository(after_profile_data, remote, for_mutation=True) if activated_trusted.get("reasons"): return { "success": False, @@ -11087,7 +11264,15 @@ def gitea_mark_issue( raise ValueError(f"action must be 'start' or 'done', got '{action}'") blocked = _profile_permission_block( - task_capability_map.required_permission("mark_issue")) + task_capability_map.required_permission("mark_issue"), + issue_number=issue_number, + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked verify_preflight_purity(remote, worktree_path=worktree_path, task="mark_issue") @@ -11169,7 +11354,15 @@ def gitea_post_heartbeat( ) -> dict: """Post a structured progress heartbeat on a claimed issue (#268).""" blocked = _profile_permission_block( - task_capability_map.required_permission("post_heartbeat")) + task_capability_map.required_permission("post_heartbeat"), + issue_number=issue_number, + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked verify_preflight_purity(remote, task="post_heartbeat") @@ -11208,7 +11401,14 @@ def gitea_acquire_conflict_fix_lease( ) -> dict: """Acquire a conflict-fix lease on a PR branch before pushing (#399).""" blocked = _profile_permission_block( - task_capability_map.required_permission("comment_issue")) + task_capability_map.required_permission("comment_issue"), + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked verify_preflight_purity(remote, worktree_path=worktree_path) @@ -11463,7 +11663,14 @@ def gitea_cleanup_stale_claims( ) blocked = _profile_permission_block( - task_capability_map.required_permission("cleanup_stale_claims")) + task_capability_map.required_permission("cleanup_stale_claims"), + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked verify_preflight_purity(remote, task="cleanup_stale_claims") @@ -11576,7 +11783,14 @@ def gitea_create_label( dict containing the created label details. """ blocked = _profile_permission_block( - task_capability_map.required_permission("create_label")) + task_capability_map.required_permission("create_label"), + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked verify_preflight_purity(remote, worktree_path=worktree_path, task="create_label") @@ -11622,7 +11836,15 @@ def gitea_set_issue_labels( list of all labels currently applied to the issue. """ blocked = _profile_permission_block( - task_capability_map.required_permission("set_issue_labels")) + task_capability_map.required_permission("set_issue_labels"), + issue_number=issue_number, + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, + ) if blocked: return blocked verify_preflight_purity(remote, worktree_path=worktree_path, task="set_issue_labels") @@ -12587,6 +12809,12 @@ def gitea_observability_reconcile_incident( create_block = _profile_permission_block( task_capability_map.required_permission("create_issue"), number=None, + remote=remote, + host=host, + org=org, + repo=repo, + org_explicit=org is not None, + repo_explicit=repo is not None, ) if create_block: return { diff --git a/session_context_binding.py b/session_context_binding.py index 2336449..e598ea6 100644 --- a/session_context_binding.py +++ b/session_context_binding.py @@ -361,27 +361,55 @@ def format_repository_slug(org: str | None, repo: str | None) -> str | None: return f"{left}/{right}" -def declared_allowed_repositories(profile: Mapping[str, Any] | None) -> list[str]: +def declared_allowed_repositories( + profile: Mapping[str, Any] | None, + *, + strict: bool = False, +) -> list[str]: """Canonical ``owner/repository`` authorization boundary declared by *profile*. This list is an authorization boundary, never the session binding itself: the verified workspace selects exactly one entry (see - :func:`assess_repository_scope`). Returns ``[]`` when the profile declares - no scope — enforcement is opt-in per profile so existing static-profile - namespaces stay functional until an operator provisions the field. + :func:`assess_repository_scope`). + + When *strict* is true (mutation path), missing profile, non-list values, or + any malformed entry raise ``ValueError`` instead of silently collapsing to + an empty scope (which would fail open). Read-only diagnostics may use + strict=False. """ if not profile: + if strict: + raise ValueError( + "profile unresolved; cannot declare allowed_repositories " + "(fail closed)" + ) return [] raw = profile.get("allowed_repositories") + if raw is None: + return [] if not isinstance(raw, (list, tuple)): + if strict: + raise ValueError( + "allowed_repositories must be a list of owner/repository slugs " + "(fail closed)" + ) return [] slugs: list[str] = [] + errors: list[str] = [] for entry in raw: if not isinstance(entry, str): + errors.append(f"non-string allowed_repositories entry {entry!r}") continue parsed = parse_repository_slug(entry) - if parsed: - slugs.append(f"{parsed[0]}/{parsed[1]}") + if not parsed: + errors.append( + f"malformed allowed_repositories entry {entry!r} " + "(expected owner/repository)" + ) + continue + slugs.append(f"{parsed[0]}/{parsed[1]}") + if strict and errors: + raise ValueError("; ".join(errors) + " (fail closed)") return slugs @@ -390,16 +418,34 @@ def assess_repository_scope( workspace_slug: str | None, allowed: list[str] | None, profile_name: str | None = None, + require_scope: bool = False, ) -> dict[str, Any]: """Authorize the verified workspace repository against the profile allowlist. The workspace-derived slug is the only candidate: a profile that authorizes several repositories still binds to the single verified one, and never to the list as a whole. + + *require_scope* (mutation path): missing/empty allowlist fails closed. The + workspace remote cannot self-authorize without a configured boundary. """ scope = list(allowed or []) reasons: list[str] = [] + name = profile_name or "(active profile)" if not scope: + if require_scope: + reasons.append( + f"mutation denied: profile '{name}' has no non-empty " + "allowed_repositories authorization boundary (fail closed)" + ) + return { + "proven": False, + "block": True, + "reasons": reasons, + "scope_enforced": True, + "workspace_slug": workspace_slug, + "allowed_repositories": [], + } return { "proven": True, "block": False, @@ -408,7 +454,6 @@ def assess_repository_scope( "workspace_slug": workspace_slug, "allowed_repositories": [], } - name = profile_name or "(active profile)" if not workspace_slug: reasons.append( f"no verified workspace repository could be established for profile " diff --git a/tests/conftest.py b/tests/conftest.py index 0f49799..0cb41ab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -147,3 +147,17 @@ def _reset_mutation_authority(monkeypatch): gitea_config._active_profile_override = None session_ctx._reset_session_context_for_testing() _state_tmp.cleanup() + + +# #714: deterministic workspace remotes only (no host git dependency). +import pytest + + +@pytest.fixture(autouse=True) +def _deterministic_workspace_remotes(): + try: + from mutation_profile_fixture import install_deterministic_remote_urls + install_deterministic_remote_urls() + except Exception: + pass + yield diff --git a/tests/mutation_profile_fixture.py b/tests/mutation_profile_fixture.py new file mode 100644 index 0000000..ada34d5 --- /dev/null +++ b/tests/mutation_profile_fixture.py @@ -0,0 +1,483 @@ +"""Centralized config-backed mutation fixture for #714. + +Mutation tests must opt into this helper explicitly. It never grants +mutation authority via environment variables alone. + +Design rules: +- temporary v2 configuration with non-empty ``allowed_repositories`` +- profile-scoped operations (not every mutation op for every test) +- deterministic workspace remotes (no host Git dependency) +- one canonical repository per profile by default +- isolated state + cleanup in ``finally`` +""" +from __future__ import annotations + +import json +import os +import tempfile +from contextlib import contextmanager +from copy import deepcopy +from typing import Iterable, Sequence +from unittest.mock import patch + +PRGS_SLUG = "Scaled-Tech-Consulting/Gitea-Tools" +PRGS_URL = f"https://gitea.prgs.cc/{PRGS_SLUG}.git" +MDCPS_SLUG = "913443/eAgenda" +MDCPS_URL = f"https://gitea.dadeschools.net/{MDCPS_SLUG}.git" +EXAMPLE_SLUG = "Example-Org/Example-Repo" +EXAMPLE_URL = f"https://gitea.example.com/{EXAMPLE_SLUG}.git" +TIMESHEET_SLUG = "Scaled-Tech-Consulting/Timesheet" + + +def _author_ops() -> list[str]: + return [ + "gitea.read", + "gitea.issue.create", + "gitea.issue.close", + "gitea.issue.comment", + "gitea.pr.create", + "gitea.pr.comment", + "gitea.branch.create", + "gitea.branch.push", + "gitea.repo.commit", + ] + + +def _author_forbidden() -> list[str]: + return [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.request_changes", + "gitea.pr.review", + ] + + +def _reviewer_ops() -> list[str]: + return [ + "gitea.read", + "gitea.pr.review", + "gitea.pr.approve", + "gitea.pr.comment", + "gitea.issue.comment", + ] + + +def _merger_ops() -> list[str]: + return [ + "gitea.read", + "gitea.pr.merge", + "gitea.pr.comment", + ] + + +def _profile( + *, + name: str, + context: str, + role: str, + base_url: str, + auth_env: str, + allowed_operations: Sequence[str], + forbidden_operations: Sequence[str], + allowed_repositories: Sequence[str], + username: str | None = None, +) -> dict: + body = { + "enabled": True, + "context": context, + "role": role, + "base_url": base_url, + "auth": {"type": "env", "name": auth_env}, + "allowed_operations": list(allowed_operations), + "forbidden_operations": list(forbidden_operations), + "allowed_repositories": list(allowed_repositories), + "execution_profile": name, + } + if username: + body["username"] = username + return body + + +def build_dual_remote_config( + *, + allowed_operations: Iterable[str] | None = None, + allowed_repositories_prgs: Sequence[str] | None = None, + allowed_repositories_mdcps: Sequence[str] | None = None, + extra_profiles: dict | None = None, + include_example_repo: bool = False, +) -> dict: + """Build a minimal dual-host v2 config. + + Each profile authorizes only its host-canonical repository by default. + ``include_example_repo`` adds Example-Org/Example-Repo when a test patches + REMOTES to that synthetic unit-test identity. + """ + ops = list(allowed_operations or _author_ops()) + forb = _author_forbidden() + prgs_repos = list(allowed_repositories_prgs or [PRGS_SLUG]) + mdcps_repos = list(allowed_repositories_mdcps or [MDCPS_SLUG]) + if include_example_repo: + for repos in (prgs_repos, mdcps_repos): + if EXAMPLE_SLUG not in repos: + repos.append(EXAMPLE_SLUG) + + profiles = { + "test-author-prgs": _profile( + name="test-author-prgs", + context="prgs", + role="author", + base_url="https://gitea.prgs.cc", + auth_env="GITEA_TOKEN_TEST", + allowed_operations=ops, + forbidden_operations=forb, + allowed_repositories=prgs_repos, + ), + "test-author-dadeschools": _profile( + name="test-author-dadeschools", + context="mdcps", + role="author", + base_url="https://gitea.dadeschools.net", + auth_env="GITEA_TOKEN_TEST", + allowed_operations=ops, + forbidden_operations=forb, + allowed_repositories=mdcps_repos, + ), + "test-reviewer-prgs": _profile( + name="test-reviewer-prgs", + context="prgs", + role="reviewer", + base_url="https://gitea.prgs.cc", + auth_env="GITEA_TOKEN_TEST", + allowed_operations=_reviewer_ops(), + forbidden_operations=[ + "gitea.pr.create", + "gitea.branch.push", + "gitea.pr.merge", + "gitea.issue.create", + ], + allowed_repositories=prgs_repos, + ), + "test-merger-prgs": _profile( + name="test-merger-prgs", + context="prgs", + role="merger", + base_url="https://gitea.prgs.cc", + auth_env="GITEA_TOKEN_TEST", + allowed_operations=_merger_ops(), + forbidden_operations=[ + "gitea.pr.create", + "gitea.branch.push", + "gitea.pr.approve", + "gitea.issue.create", + ], + allowed_repositories=prgs_repos, + ), + } + if extra_profiles: + profiles.update(deepcopy(extra_profiles)) + + # Legacy aliases used by older tests — same host scope as the base profile. + for alias, base in ( + ("gitea-author", "test-author-prgs"), + ("author-test", "test-author-dadeschools"), + ("author", "test-author-dadeschools"), + ("full-author", "test-author-dadeschools"), + ("test-author", "test-author-dadeschools"), + ("prgs-author", "test-author-prgs"), + ("gitea-reviewer", "test-reviewer-prgs"), + ("prgs-reviewer", "test-reviewer-prgs"), + ("gitea-merger", "test-merger-prgs"), + ("prgs-merger", "test-merger-prgs"), + ): + if alias not in profiles and base in profiles: + clone = deepcopy(profiles[base]) + clone["execution_profile"] = alias + profiles[alias] = clone + + return { + "version": 2, + "rules": {"allow_runtime_switching": True}, + "contexts": { + "prgs": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.prgs.cc"}, + }, + "mdcps": { + "enabled": True, + "gitea": { + "enabled": True, + "base_url": "https://gitea.dadeschools.net", + }, + }, + }, + "profiles": profiles, + } + + +def build_v2_config(**kwargs): + """Back-compat alias.""" + return build_dual_remote_config( + allowed_operations=kwargs.get("allowed_operations"), + extra_profiles=kwargs.get("extra_profiles"), + include_example_repo=bool(kwargs.get("include_example_repo")), + ) + + +def inject_allowed_repositories( + config: dict, + repos: Sequence[str], + *, + profiles: Sequence[str] | None = None, +) -> dict: + """Return a deep copy of *config* with allowlists set on selected profiles.""" + out = deepcopy(config) + targets = set(profiles) if profiles is not None else set(out.get("profiles") or {}) + for name, prof in (out.get("profiles") or {}).items(): + if name in targets: + prof["allowed_repositories"] = list(repos) + return out + + +def ensure_all_profiles_have_scope( + config: dict, + default_repos: Sequence[str], +) -> dict: + """Add non-empty allowed_repositories to every profile that lacks one.""" + out = deepcopy(config) + for _name, prof in (out.get("profiles") or {}).items(): + raw = prof.get("allowed_repositories") + if not isinstance(raw, (list, tuple)) or not raw: + prof["allowed_repositories"] = list(default_repos) + return out + + +@contextmanager +def mutation_profile_env( + *, + profile_name: str = "test-author-dadeschools", + allowed_operations: Iterable[str] | None = None, + allowed_repositories: Sequence[str] | None = None, + workspace_urls: dict | None = None, + clear_env: bool = False, + extra_env: dict | None = None, + extra_profiles: dict | None = None, + include_example_repo: bool = False, + config: dict | None = None, +): + """Context manager: config-backed mutation authority + deterministic remotes.""" + import gitea_config + import gitea_mcp_server as srv + import session_context_binding as session_ctx + + if config is None: + prgs_repos = None + mdcps_repos = None + if allowed_repositories is not None: + # Caller-specified single allowlist applied to both host profiles. + prgs_repos = list(allowed_repositories) + mdcps_repos = list(allowed_repositories) + cfg = build_dual_remote_config( + allowed_operations=allowed_operations, + allowed_repositories_prgs=prgs_repos, + allowed_repositories_mdcps=mdcps_repos, + extra_profiles=extra_profiles, + include_example_repo=include_example_repo, + ) + else: + cfg = deepcopy(config) + if allowed_repositories is not None: + cfg = ensure_all_profiles_have_scope(cfg, list(allowed_repositories)) + + urls = ( + {"prgs": PRGS_URL, "dadeschools": MDCPS_URL} + if workspace_urls is None + else dict(workspace_urls) + ) + + tmp = tempfile.TemporaryDirectory(prefix="gitea-mut-cfg-") + try: + cfg_path = os.path.join(tmp.name, "profiles.json") + with open(cfg_path, "w", encoding="utf-8") as fh: + json.dump(cfg, fh) + env = { + "GITEA_MCP_CONFIG": cfg_path, + "GITEA_MCP_PROFILE": profile_name, + "GITEA_TOKEN_TEST": "test-token", + "PYTEST_CURRENT_TEST": os.environ.get( + "PYTEST_CURRENT_TEST", "mutation_profile_env" + ), + } + if extra_env: + env.update(extra_env) + + def _remote_url(name: str): + if name in urls: + return urls[name] + # Prefer live REMOTES (tests often patch Example-Org). + try: + entry = srv.REMOTES.get(name) or {} + host = entry.get("host") + org = entry.get("org") + repo = entry.get("repo") + if host and org and repo: + if ( + name == "prgs" + and org == "Scaled-Tech-Consulting" + and repo == "Timesheet" + ): + return PRGS_URL + if name == "dadeschools" and repo == "Timesheet": + return MDCPS_URL + return f"https://{host}/{org}/{repo}.git" + except Exception: + pass + return None + + gitea_config._active_profile_override = None + try: + session_ctx._reset_session_context_for_testing() + except Exception: + pass + + payload = { + "env": env, + "config_path": cfg_path, + "profile_name": profile_name, + "urls": urls, + "config": cfg, + } + with patch.dict(os.environ, env, clear=clear_env): + os.environ.setdefault( + "PYTEST_CURRENT_TEST", + env.get("PYTEST_CURRENT_TEST", "mutation_profile_env"), + ) + with patch.object(srv, "_local_git_remote_url", side_effect=_remote_url): + mcp_srv = None + try: + import mcp_server as mcp_srv # type: ignore + except Exception: + mcp_srv = None + if mcp_srv is not None: + with patch.object( + mcp_srv, "_local_git_remote_url", side_effect=_remote_url + ): + yield payload + else: + yield payload + finally: + try: + session_ctx._reset_session_context_for_testing() + except Exception: + pass + gitea_config._active_profile_override = None + tmp.cleanup() + + +# Process-lifetime shared config for mass-migrating env-only mutation tests. +_SHARED_CFG_PATH = None +_SHARED_CFG_DIR = None +_SHARED_CFG_WITH_EXAMPLE_PATH = None + + +def shared_mutation_config_path(*, include_example_repo: bool = False) -> str: + """Write (once) a dual-remote mutation config and return its path.""" + global _SHARED_CFG_PATH, _SHARED_CFG_DIR, _SHARED_CFG_WITH_EXAMPLE_PATH + import atexit + import shutil + + if include_example_repo: + if _SHARED_CFG_WITH_EXAMPLE_PATH and os.path.isfile( + _SHARED_CFG_WITH_EXAMPLE_PATH + ): + return _SHARED_CFG_WITH_EXAMPLE_PATH + if _SHARED_CFG_DIR is None: + _SHARED_CFG_DIR = tempfile.mkdtemp(prefix="gitea-shared-mut-cfg-") + atexit.register(lambda: shutil.rmtree(_SHARED_CFG_DIR, ignore_errors=True)) + path = os.path.join(_SHARED_CFG_DIR, "profiles-with-example.json") + with open(path, "w", encoding="utf-8") as fh: + json.dump(build_dual_remote_config(include_example_repo=True), fh) + _SHARED_CFG_WITH_EXAMPLE_PATH = path + return path + + if _SHARED_CFG_PATH and os.path.isfile(_SHARED_CFG_PATH): + return _SHARED_CFG_PATH + if _SHARED_CFG_DIR is None: + _SHARED_CFG_DIR = tempfile.mkdtemp(prefix="gitea-shared-mut-cfg-") + atexit.register(lambda: shutil.rmtree(_SHARED_CFG_DIR, ignore_errors=True)) + _SHARED_CFG_PATH = os.path.join(_SHARED_CFG_DIR, "profiles.json") + with open(_SHARED_CFG_PATH, "w", encoding="utf-8") as fh: + json.dump(build_dual_remote_config(), fh) + return _SHARED_CFG_PATH + + +def shared_mutation_env( + profile_name: str = "test-author-dadeschools", + *, + include_example_repo: bool = False, + **extra, +) -> dict: + """Env dict for mutation tests: config-backed + optional extras. + + Always carries ``PYTEST_CURRENT_TEST`` so ``patch.dict(..., clear=True)`` + does not strip the pytest marker and trip the #695 native-transport wall. + """ + env = { + "GITEA_MCP_CONFIG": shared_mutation_config_path( + include_example_repo=include_example_repo + ), + "GITEA_MCP_PROFILE": profile_name, + "GITEA_TOKEN_TEST": "test-token", + "PYTEST_CURRENT_TEST": os.environ.get( + "PYTEST_CURRENT_TEST", "mutation_profile_env" + ), + } + env.update(extra) + # Callers must not be able to drop the pytest marker by accident. + env.setdefault( + "PYTEST_CURRENT_TEST", + os.environ.get("PYTEST_CURRENT_TEST", "mutation_profile_env"), + ) + return env + + +def install_deterministic_remote_urls() -> None: + """Patch server modules so workspace remotes are deterministic. + + Prefer live ``REMOTES`` entries (so tests that patch Example-Org keep + workspace alignment). Map the historical prgs→Timesheet default to + Gitea-Tools so session binding matches the control repository under test. + """ + import gitea_mcp_server as srv + + def _url(name: str): + try: + entry = srv.REMOTES.get(name) or {} + host = entry.get("host") + org = entry.get("org") + repo = entry.get("repo") + if host and org and repo: + if ( + name == "prgs" + and org == "Scaled-Tech-Consulting" + and repo == "Timesheet" + ): + return PRGS_URL + if name == "dadeschools" and repo == "Timesheet": + # Prefer mdcps eAgenda canonical for dual-config tests. + return MDCPS_URL + return f"https://{host}/{org}/{repo}.git" + except Exception: + pass + if name == "prgs": + return PRGS_URL + if name == "dadeschools": + return MDCPS_URL + return None + + srv._local_git_remote_url = _url # type: ignore[method-assign] + try: + import mcp_server as mcp_srv + + mcp_srv._local_git_remote_url = _url # type: ignore[method-assign] + except Exception: + pass diff --git a/tests/test_agent_temp_artifacts.py b/tests/test_agent_temp_artifacts.py index 7684d98..fb527be 100644 --- a/tests/test_agent_temp_artifacts.py +++ b/tests/test_agent_temp_artifacts.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for agent temp artifact detection and preflight warnings (#261).""" import json import os @@ -65,11 +69,9 @@ class TestPreflightWarnings(unittest.TestCase): # Issue-write tools are profile-gated (#69); gitea_lock_issue requires # gitea.issue.comment (see task_capability_map), so the gate must be # seeded exactly like tests/test_mcp_server.py::TestIssueLocking (#359). -ISSUE_WRITE_ENV = { - "GITEA_ALLOWED_OPERATIONS": ( - "gitea.issue.create,gitea.issue.close,gitea.issue.comment" - ), -} +ISSUE_WRITE_ENV = shared_mutation_env( + "test-author-prgs", +) class TestIssueLockArtifactWarning(unittest.TestCase): diff --git a/tests/test_assess_conflict_fix_push_tool.py b/tests/test_assess_conflict_fix_push_tool.py index 36166cd..e2395df 100644 --- a/tests/test_assess_conflict_fix_push_tool.py +++ b/tests/test_assess_conflict_fix_push_tool.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Regression tests for gitea_assess_conflict_fix_push structured failures (#519).""" import os diff --git a/tests/test_audit.py b/tests/test_audit.py index 06a7c41..9cd405a 100644 --- a/tests/test_audit.py +++ b/tests/test_audit.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for Gitea MCP mutating-action audit logging (issue #18). Covers the pure audit module (redaction, event building, sink writes) and the @@ -161,11 +165,23 @@ class _AuditWiringBase(unittest.TestCase): self._dir.cleanup() def _env(self, **extra): - env = {"GITEA_AUDIT_LOG": self.audit_path, - "GITEA_PROFILE_NAME": "gitea-author", - "GITEA_ALLOWED_OPERATIONS": ( - "read,merge,gitea.issue.create,gitea.issue.close")} + # Default: prgs-aligned author with create/close (remote=prgs tests). + # Callers override GITEA_MCP_PROFILE for merger/reviewer paths. + profile = extra.pop("GITEA_MCP_PROFILE", None) or extra.pop( + "GITEA_PROFILE_NAME", None + ) or "test-author-prgs" + env = shared_mutation_env( + profile, + GITEA_AUDIT_LOG=self.audit_path, + GITEA_TOKEN_TEST="test-token", + ) + # Strip legacy env-only authority knobs if callers still pass them; + # config-backed profiles own operations and repositories (#714). + extra.pop("GITEA_ALLOWED_OPERATIONS", None) + extra.pop("GITEA_FORBIDDEN_OPERATIONS", None) + extra.pop("GITEA_PROFILE_NAME", None) env.update(extra) + env["GITEA_MCP_PROFILE"] = profile return env def _records(self): @@ -196,7 +212,7 @@ class TestSimpleToolAudit(_AuditWiringBase): rec = recs[0] self.assertEqual(rec["action"], "create_issue") self.assertEqual(rec["result"], "succeeded") - self.assertEqual(rec["profile_name"], "gitea-author") + self.assertIn(rec["profile_name"], ("gitea-author", "test-author-prgs", "prgs-author")) self.assertEqual(rec["authenticated_username"], "author-bot") self.assertEqual(rec["issue_number"], 11) self.assertEqual(rec["request_metadata"]["title"], "Add thing") @@ -239,10 +255,11 @@ class TestSimpleToolAudit(_AuditWiringBase): def test_disabled_writes_nothing_and_no_extra_call(self, _auth, _get_all, mock_api, _role): # No GITEA_AUDIT_LOG -> audit is a no-op: one create POST, no file. mock_api.return_value = {"number": 1, "html_url": "http://x/1"} - with patch.dict(os.environ, { - "GITEA_PROFILE_NAME": "gitea-author", - "GITEA_ALLOWED_OPERATIONS": "gitea.issue.create", - }, clear=True): + with patch.dict( + os.environ, + shared_mutation_env("test-author-prgs"), + clear=True, + ): gitea_create_issue(title="x", remote="prgs") issue_posts = [ c for c in mock_api.call_args_list if c.args[0] == "POST" @@ -342,8 +359,7 @@ class TestGatedToolAudit(_AuditWiringBase): self._pr("author-bot"), approval, # gate 7 feedback {}, {"merged_commit_sha": "c1"}, ] - env = self._env(GITEA_PROFILE_NAME="gitea-merger", - GITEA_ALLOWED_OPERATIONS="read,merge") + env = self._env(GITEA_MCP_PROFILE="test-merger-prgs") with patch.dict(os.environ, env, clear=True): mcp_server.gitea_load_review_workflow() r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 8", @@ -362,8 +378,7 @@ class TestGatedToolAudit(_AuditWiringBase): def test_merge_blocked_audited(self, _auth, mock_api): # Self-author merge is blocked; must still be recorded as blocked. mock_api.side_effect = [{"login": "jcwalker3"}, self._pr("jcwalker3")] - env = self._env(GITEA_PROFILE_NAME="gitea-merger", - GITEA_ALLOWED_OPERATIONS="read,merge") + env = self._env(GITEA_MCP_PROFILE="test-merger-prgs") with patch.dict(os.environ, env, clear=True): mcp_server.gitea_load_review_workflow() r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 8", remote="prgs") @@ -385,11 +400,23 @@ class TestGatedToolAudit(_AuditWiringBase): [{"id": 7, "user": {"login": "reviewer-bot"}, "state": "APPROVED", "submitted_at": "2026-07-06T10:00:00Z", "dismissed": False}], ] - env = self._env(GITEA_PROFILE_NAME="gitea-reviewer", - GITEA_ALLOWED_OPERATIONS="read,review,approve") + env = self._env(GITEA_MCP_PROFILE="test-reviewer-prgs") with patch.dict(os.environ, env, clear=True): + import session_context_binding as _sc + from tests.test_mcp_server import ( + _init_reviewer_session, + _install_owned_reviewer_lease, + ) from mcp_server import gitea_mark_final_review_decision + # Rebind after profile env is applied so durable session state + # matches the active reviewer profile (#714 / #695). + _sc._reset_session_context_for_testing() + _init_reviewer_session("prgs") + lease = _install_owned_reviewer_lease(8) + lease.start() + self.addCleanup(lease.stop) + mcp_server.gitea_load_review_workflow() gitea_mark_final_review_decision( 8, "approve", expected_head_sha="abc123", remote="prgs", ) @@ -398,7 +425,10 @@ class TestGatedToolAudit(_AuditWiringBase): body="LGTM", remote="prgs", final_review_decision_ready=True, ) - self.assertTrue(r["performed"]) + self.assertTrue( + r["performed"], + msg=f"submit_pr_review blocked: {r}", + ) recs = self._records() self.assertEqual(len(recs), 1) self.assertEqual(recs[0]["action"], "submit_pr_review") diff --git a/tests/test_commit_files_capability.py b/tests/test_commit_files_capability.py index c1474ab..653590e 100644 --- a/tests/test_commit_files_capability.py +++ b/tests/test_commit_files_capability.py @@ -29,6 +29,7 @@ CONFIG = { "allowed_operations": ["gitea.read", "gitea.repo.commit"], "forbidden_operations": ["gitea.pr.create"], "execution_profile": "commit-author", + "allowed_repositories": ["Example-Org/Example-Repo"], }, "pr-only-author": { "enabled": True, @@ -39,6 +40,7 @@ CONFIG = { "allowed_operations": ["gitea.read", "gitea.pr.create"], "forbidden_operations": ["gitea.repo.commit"], "execution_profile": "pr-only-author", + "allowed_repositories": ["Example-Org/Example-Repo"], }, "reviewer-profile": { "enabled": True, @@ -51,6 +53,7 @@ CONFIG = { "gitea.repo.commit", "gitea.pr.create", "gitea.branch.push", ], "execution_profile": "reviewer-profile", + "allowed_repositories": ["Example-Org/Example-Repo"], }, }, "rules": {"allow_runtime_switching": False}, diff --git a/tests/test_commit_files_gate.py b/tests/test_commit_files_gate.py index 4c72d46..50c882c 100644 --- a/tests/test_commit_files_gate.py +++ b/tests/test_commit_files_gate.py @@ -35,6 +35,7 @@ CONFIG = { ], "forbidden_operations": [], "execution_profile": "full-author", + "allowed_repositories": ["Example-Org/Example-Repo"], }, "reviewer-no-commit": { "enabled": True, @@ -49,6 +50,7 @@ CONFIG = { "gitea.repo.commit", "gitea.pr.create", "gitea.branch.push" ], "execution_profile": "reviewer-no-commit", + "allowed_repositories": ["Example-Org/Example-Repo"], }, }, "rules": {"allow_runtime_switching": False}, diff --git a/tests/test_commit_payloads.py b/tests/test_commit_payloads.py index 520f09e..7fd03a8 100644 --- a/tests/test_commit_payloads.py +++ b/tests/test_commit_payloads.py @@ -35,6 +35,7 @@ CONFIG = { "gitea.pr.review", ], "execution_profile": "full-author", + "allowed_repositories": ["Example-Org/Example-Repo"], }, }, } diff --git a/tests/test_config.py b/tests/test_config.py index e5f05c5..19077ac 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for canonical JSON runtime-profile configuration (gitea_config) and its integration into gitea_auth.get_profile / get_auth_header. diff --git a/tests/test_create_issue.py b/tests/test_create_issue.py index 11eb846..3da7cad 100644 --- a/tests/test_create_issue.py +++ b/tests/test_create_issue.py @@ -1,3 +1,4 @@ +import os """Tests for create_issue.py. Every test mocks auth functions so no real network calls or keychain @@ -12,7 +13,7 @@ import sys import tempfile import unittest import contextlib -from unittest.mock import MagicMock, patch +from unittest.mock import patch, MagicMock, patch # The module under test lives in the repo root, not a package. sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) @@ -92,6 +93,26 @@ class TestRemoteResolution(unittest.TestCase): # --------------------------------------------------------------------------- @unittest.skipIf(_SKIP, _REASON) class TestAPIPayload(unittest.TestCase): + """#714: omitted --org/--repo uses workspace-bound repo (Gitea-Tools), + not the historical REMOTES Timesheet default. Intentional security-policy + migration of legacy omitted-target assertions. + """ + + def setUp(self): + self._ws_env = patch.dict( + os.environ, + { + "GITEA_TEST_WORKSPACE_REMOTE_URL": ( + "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git" + ) + }, + clear=False, + ) + self._ws_env.start() + + def tearDown(self): + self._ws_env.stop() + """Ensure the JSON payload sent to Gitea is correct.""" @patch("create_issue.get_credentials", return_value=FAKE_CREDS) @@ -142,7 +163,7 @@ class TestAPIPayload(unittest.TestCase): url = mock_api.call_args[0][1] self.assertEqual( url, - "https://gitea.prgs.cc/api/v1/repos/Scaled-Tech-Consulting/Timesheet/issues", + "https://gitea.prgs.cc/api/v1/repos/Scaled-Tech-Consulting/Gitea-Tools/issues", ) diff --git a/tests/test_create_pr.py b/tests/test_create_pr.py index b55dafb..d307bf2 100644 --- a/tests/test_create_pr.py +++ b/tests/test_create_pr.py @@ -1,3 +1,4 @@ +import os """Tests for create_pr.py. Every test mocks `get_credentials` and `urllib.request.urlopen` so no real @@ -8,7 +9,7 @@ import json import sys import unittest import contextlib -from unittest.mock import MagicMock, patch +from unittest.mock import patch, MagicMock, patch sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) import create_pr # noqa: E402 @@ -74,6 +75,26 @@ class TestRemoteResolution(unittest.TestCase): # API payload # --------------------------------------------------------------------------- class TestAPIPayload(unittest.TestCase): + """#714: omitted --org/--repo uses workspace-bound repo (Gitea-Tools), + not the historical REMOTES Timesheet default. Intentional security-policy + migration of legacy omitted-target assertions. + """ + + def setUp(self): + self._ws_env = patch.dict( + os.environ, + { + "GITEA_TEST_WORKSPACE_REMOTE_URL": ( + "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git" + ) + }, + clear=False, + ) + self._ws_env.start() + + def tearDown(self): + self._ws_env.stop() + @patch("create_pr.get_credentials", return_value=FAKE_CREDS) def test_payload_fields(self, _cred): @@ -113,7 +134,7 @@ class TestAPIPayload(unittest.TestCase): url = MockReq.call_args[0][0] self.assertEqual( url, - "https://gitea.prgs.cc/api/v1/repos/Scaled-Tech-Consulting/Timesheet/pulls", + "https://gitea.prgs.cc/api/v1/repos/Scaled-Tech-Consulting/Gitea-Tools/pulls", ) diff --git a/tests/test_issue_714_production_first_bind.py b/tests/test_issue_714_production_first_bind.py index 28e20d0..096c9b7 100644 --- a/tests/test_issue_714_production_first_bind.py +++ b/tests/test_issue_714_production_first_bind.py @@ -185,7 +185,11 @@ class TestProductionOrderRepositoryDriftBlocks(_ProductionOrderBase): with patch.dict(os.environ, self._env, clear=False), self._live(): srv.gitea_whoami(remote="prgs") blocked = srv._session_context_mutation_block( - remote="prgs", org=WORKSPACE_ORG, repo="Timesheet" + remote="prgs", + org=WORKSPACE_ORG, + repo="Timesheet", + org_explicit=True, + repo_explicit=True, ) self.assertIsNotNone(blocked) self.assertTrue( @@ -380,13 +384,23 @@ class TestConcurrentFirstBindSelectsOneRepository(_ProductionOrderBase): class TestRepositoryScopeUnits(unittest.TestCase): - def test_absent_scope_is_not_enforced(self): + def test_absent_scope_is_not_enforced_for_read_diagnostics(self): scope = session_ctx.assess_repository_scope( workspace_slug=WORKSPACE_SLUG, allowed=[], profile_name="legacy" ) self.assertTrue(scope["proven"]) self.assertFalse(scope["scope_enforced"]) + def test_require_scope_blocks_empty_or_missing_allowlist(self): + scope = session_ctx.assess_repository_scope( + workspace_slug=WORKSPACE_SLUG, + allowed=[], + profile_name="legacy", + require_scope=True, + ) + self.assertTrue(scope["block"]) + self.assertTrue(any("allowed_repositories" in r for r in scope["reasons"])) + def test_declared_scope_authorizes_only_listed_repository(self): allowed = session_ctx.declared_allowed_repositories( {"allowed_repositories": [WORKSPACE_SLUG]} @@ -435,7 +449,7 @@ class TestRepositoryScopeUnits(unittest.TestCase): )["proven"] ) - def test_malformed_allowlist_entries_are_ignored(self): + def test_malformed_allowlist_entries_are_ignored_in_non_strict_mode(self): self.assertEqual( session_ctx.declared_allowed_repositories( {"allowed_repositories": ["not-a-slug", 17, None, WORKSPACE_SLUG]} @@ -443,6 +457,53 @@ class TestRepositoryScopeUnits(unittest.TestCase): [WORKSPACE_SLUG], ) + def test_strict_mode_rejects_malformed_allowlist_entries(self): + with self.assertRaises(ValueError): + session_ctx.declared_allowed_repositories( + {"allowed_repositories": ["not-a-slug", WORKSPACE_SLUG]}, + strict=True, + ) + + +class TestRemotesDefaultNotCallerOverride(_ProductionOrderBase): + def test_resolved_timesheet_default_does_not_block_when_bound_to_workspace(self): + """Tools historically pass REMOTES-filled Timesheet after _resolve; that + must not be treated as an explicit override against Gitea-Tools.""" + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + # Simulate create_issue after resolve: org/repo are REMOTES defaults + blocked = srv._session_context_mutation_block( + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Timesheet", + ) + self.assertIsNone(blocked, getattr(blocked, "get", lambda *_: blocked)("reasons") if blocked else None) + + def test_git_unavailable_blocks_mutation(self): + def no_remote(_name): + return None + with patch.dict(os.environ, self._env, clear=False), self._live(): + with patch.object(srv, "_local_git_remote_url", side_effect=no_remote): + blocked = srv._session_context_mutation_block(remote="prgs") + self.assertIsNotNone(blocked) + self.assertTrue( + any( + "workspace" in r.lower() or "allowed_repositories" in r or "unverified" in r + for r in blocked["reasons"] + ), + blocked["reasons"], + ) + + def test_explicit_mismatch_still_blocks(self): + with patch.dict(os.environ, self._env, clear=False), self._live(): + srv.gitea_whoami(remote="prgs") + blocked = srv._session_context_mutation_block( + remote="prgs", + org=WORKSPACE_ORG, + repo="Other-Tools", + ) + self.assertIsNotNone(blocked) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_issue_714_session_context_immutability.py b/tests/test_issue_714_session_context_immutability.py index d7c0f39..781421f 100644 --- a/tests/test_issue_714_session_context_immutability.py +++ b/tests/test_issue_714_session_context_immutability.py @@ -55,6 +55,7 @@ CONFIG_714 = { "gitea.pr.merge", "gitea.pr.request_changes", ], + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], "execution_profile": "prgs-author", }, "mdcps-reviewer": { @@ -78,6 +79,7 @@ CONFIG_714 = { "gitea.pr.merge", "gitea.repo.commit", ], + "allowed_repositories": ["913443/eAgenda"], "execution_profile": "mdcps-reviewer", }, "mdcps-author": { @@ -102,6 +104,7 @@ CONFIG_714 = { "gitea.pr.merge", "gitea.pr.request_changes", ], + "allowed_repositories": ["913443/eAgenda"], "execution_profile": "mdcps-author", }, }, @@ -131,6 +134,14 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): clear=False, ) self._remotes.start() + def _url(name): + if name == "dadeschools": + return "https://gitea.dadeschools.net/913443/eAgenda.git" + if name == "prgs": + return "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git" + return None + self._url_patch = patch.object(mcp_server, "_local_git_remote_url", side_effect=_url) + self._url_patch.start() mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None @@ -143,6 +154,7 @@ class TestIssue714SessionContextImmutability(unittest.TestCase): } def tearDown(self): + self._url_patch.stop() self._remotes.stop() mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None diff --git a/tests/test_issue_comment_workspace_guard.py b/tests/test_issue_comment_workspace_guard.py index 377c624..a5baad0 100644 --- a/tests/test_issue_comment_workspace_guard.py +++ b/tests/test_issue_comment_workspace_guard.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 import os import sys import unittest diff --git a/tests/test_issue_content_gate.py b/tests/test_issue_content_gate.py index 99b12b0..5ba8e9d 100644 --- a/tests/test_issue_content_gate.py +++ b/tests/test_issue_content_gate.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for the pre-create issue content gate (Issue #582).""" import sys import unittest @@ -15,9 +19,10 @@ from issue_content_gate import ( # noqa: E402 from mcp_server import gitea_create_issue # noqa: E402 FAKE_AUTH = "Basic dGVzdDp0ZXN0" -ISSUE_WRITE_ENV = { - "GITEA_ALLOWED_OPERATIONS": "gitea.issue.create", -} +ISSUE_WRITE_ENV = shared_mutation_env( + "test-author-prgs", + GITEA_ALLOWED_OPERATIONS="gitea.issue.create", +) class TestNormalizeAndPointers(unittest.TestCase): diff --git a/tests/test_issue_duplicate_gate.py b/tests/test_issue_duplicate_gate.py index fe13303..caba3dd 100644 --- a/tests/test_issue_duplicate_gate.py +++ b/tests/test_issue_duplicate_gate.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for pre-create issue duplicate gate (Issue #207).""" import sys import unittest @@ -19,9 +23,10 @@ from mcp_server import gitea_create_issue # noqa: E402 from review_proofs import assess_duplicate_search_proof as proof_assess # noqa: E402 FAKE_AUTH = "Basic dGVzdDp0ZXN0" -ISSUE_WRITE_ENV = { - "GITEA_ALLOWED_OPERATIONS": "gitea.issue.create", -} +ISSUE_WRITE_ENV = shared_mutation_env( + "test-author-prgs", + GITEA_ALLOWED_OPERATIONS="gitea.issue.create", +) CANONICAL_TITLE = ( "Add hard queue-target resolution wall before PR inventory " "or empty-queue claims" @@ -162,7 +167,7 @@ class TestCreateIssueMCPGate(unittest.TestCase): mock_role_check.return_value = (True, []) mock_api.return_value = {"number": 1, "html_url": "https://gitea.example.com/issues/1"} with patch.dict(__import__("os").environ, ISSUE_WRITE_ENV, clear=True): - result = gitea_create_issue(title="Unique new issue", body="body text") + result = gitea_create_issue(title="Unique new issue", body="body text", remote="prgs") self.assertEqual(result["number"], 1) diff --git a/tests/test_issue_work_duplicate_gate.py b/tests/test_issue_work_duplicate_gate.py index 1ae3024..88701c4 100644 --- a/tests/test_issue_work_duplicate_gate.py +++ b/tests/test_issue_work_duplicate_gate.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for early duplicate-work detection (#400).""" import os import sys @@ -144,10 +148,12 @@ class TestInjectableDuplicateFetcher(unittest.TestCase): }, ): with tempfile.TemporaryDirectory() as lock_dir: - with patch.dict(os.environ, { - "GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment", - "GITEA_ISSUE_LOCK_DIR": lock_dir, - }, clear=True): + env = shared_mutation_env( + "test-author-prgs", + include_example_repo=True, + GITEA_ISSUE_LOCK_DIR=lock_dir, + ) + with patch.dict(os.environ, env, clear=True): mcp_server.gitea_lock_issue( issue_number=400, branch_name="feat/issue-400-duplicate-work-preflight", @@ -161,7 +167,11 @@ class TestMcpDuplicateRecheck(unittest.TestCase): self._dir = tempfile.TemporaryDirectory() self._env_patch = patch.dict( os.environ, - {"GITEA_ISSUE_LOCK_DIR": self._dir.name}, + shared_mutation_env( + "test-author-prgs", + include_example_repo=True, + GITEA_ISSUE_LOCK_DIR=self._dir.name, + ), clear=False, ) self._env_patch.start() @@ -171,6 +181,11 @@ class TestMcpDuplicateRecheck(unittest.TestCase): }) self._remotes.start() mcp_server._IDENTITY_CACHE.clear() + try: + import session_context_binding as _sc + _sc._reset_session_context_for_testing() + except Exception: + pass def tearDown(self): patch.stopall() @@ -205,6 +220,8 @@ class TestMcpDuplicateRecheck(unittest.TestCase): "allowed_operations": ["gitea.read", "gitea.repo.commit"], "forbidden_operations": [], "audit_label": "test-author", + "allowed_repositories": ["Example-Org/Example-Repo"], + "base_url": "https://gitea.example.com", }) @patch("mcp_server.get_auth_header", return_value="token x") def test_commit_files_blocked_on_recheck(self, _auth, _profile, mock_gate): @@ -239,6 +256,8 @@ class TestMcpDuplicateRecheck(unittest.TestCase): "allowed_operations": ["gitea.read", "gitea.pr.create"], "forbidden_operations": [], "audit_label": "test-author", + "allowed_repositories": ["Example-Org/Example-Repo"], + "base_url": "https://gitea.example.com", }) @patch("mcp_server.get_auth_header", return_value="token x") def test_create_pr_returns_handoff_on_duplicate(self, _auth, _profile, mock_gate): diff --git a/tests/test_issue_write_tool_gates.py b/tests/test_issue_write_tool_gates.py index f91c514..902355c 100644 --- a/tests/test_issue_write_tool_gates.py +++ b/tests/test_issue_write_tool_gates.py @@ -1,3 +1,8 @@ +import sys +from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from mutation_profile_fixture import install_deterministic_remote_urls # noqa: E402 +install_deterministic_remote_urls() """Regression tests: issue-write tool gates match gitea_resolve_task_capability (#69).""" import json import os @@ -41,6 +46,7 @@ CONFIG = { "gitea.read", "gitea.issue.create", "gitea.issue.close", "gitea.issue.comment"], "forbidden_operations": [], + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools", "Example-Org/Example-Repo", "913443/eAgenda"], "execution_profile": "full-author", }, }, diff --git a/tests/test_llm_agent_sha.py b/tests/test_llm_agent_sha.py index 2751d9d..407e75e 100644 --- a/tests/test_llm_agent_sha.py +++ b/tests/test_llm_agent_sha.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Negative tests for LLM-Agent-SHA attribution (#86, Phase 0). ``LLM-Agent-SHA`` (docs/llm-agent-sha.md) is attribution metadata ONLY. These diff --git a/tests/test_lock_issue_mcp_registration.py b/tests/test_lock_issue_mcp_registration.py index 0eca637..cd096c6 100644 --- a/tests/test_lock_issue_mcp_registration.py +++ b/tests/test_lock_issue_mcp_registration.py @@ -1,7 +1,11 @@ -"""Regression tests for gitea_lock_issue MCP tool registration (#521).""" - from __future__ import annotations + +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 + import os import tempfile import unittest @@ -13,22 +17,20 @@ from mcp_server import gitea_create_pr, gitea_lock_issue FAKE_AUTH = "Basic dGVzdDp0ZXN0" -ISSUE_WRITE_ENV = { - "GITEA_ALLOWED_OPERATIONS": ( - "gitea.issue.create,gitea.issue.close,gitea.issue.comment" - ), -} +ISSUE_WRITE_ENV = shared_mutation_env( + "test-author-prgs", +) -CREATE_PR_ENV = { - "GITEA_PROFILE_NAME": "author-test", - "GITEA_ALLOWED_OPERATIONS": ( +CREATE_PR_ENV = shared_mutation_env( + "test-author-prgs", + GITEA_ALLOWED_OPERATIONS=( "gitea.read,gitea.pr.create,gitea.branch.push," "gitea.issue.create,gitea.issue.close,gitea.issue.comment" ), - "GITEA_FORBIDDEN_OPERATIONS": ( + GITEA_FORBIDDEN_OPERATIONS=( "gitea.pr.approve,gitea.pr.merge,gitea.pr.review" ), -} +) def _clean_master_git_state_for_lock(): diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 6f641f0..adedb92 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -1,3 +1,11 @@ +import sys +from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from mutation_profile_fixture import ( + mutation_profile_env, + shared_mutation_env, + install_deterministic_remote_urls, +) # noqa: E402 """Tests for the MCP server tool functions. Each tool is tested by calling the underlying function directly (not through @@ -47,6 +55,7 @@ from gitea_auth import get_profile # noqa: E402 import gitea_config # noqa: E402 import mcp_server +install_deterministic_remote_urls() import issue_lock_store import gitea_auth @@ -221,22 +230,26 @@ def _seed_ready_review_decision( # Issue-write tools are profile-gated (#69). -ISSUE_WRITE_ENV = { - "GITEA_ALLOWED_OPERATIONS": ( - "gitea.issue.create,gitea.issue.close,gitea.issue.comment" +# Default remote is dadeschools — use the host-aligned config profile so +# cross-host session binding does not fail closed (#714). +ISSUE_WRITE_ENV = shared_mutation_env( + "test-author-dadeschools", + GITEA_ALLOWED_OPERATIONS=( + "gitea.issue.create,gitea.issue.close,gitea.issue.comment," + "gitea.pr.create,gitea.branch.push,gitea.repo.commit,gitea.read" ), -} +) # create_pr is gated on author profile + namespace (#69, #209). -CREATE_PR_ENV = { - "GITEA_PROFILE_NAME": "author-test", - "GITEA_ALLOWED_OPERATIONS": ( - "gitea.read,gitea.pr.create,gitea.branch.push" +# Default remote is dadeschools; use matching config-backed author profile. +CREATE_PR_ENV = shared_mutation_env( + "test-author-dadeschools", + GITEA_ALLOWED_OPERATIONS=( + "gitea.read,gitea.pr.create,gitea.branch.push," + "gitea.issue.create,gitea.issue.close,gitea.issue.comment" ), - "GITEA_FORBIDDEN_OPERATIONS": ( - "gitea.pr.approve,gitea.pr.merge,gitea.pr.review" - ), -} + GITEA_FORBIDDEN_OPERATIONS="gitea.pr.approve,gitea.pr.merge,gitea.pr.review", +) ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json" @@ -287,6 +300,7 @@ def _bind_test_lock(**overrides) -> str: # --------------------------------------------------------------------------- # Create Issue # --------------------------------------------------------------------------- + class TestCreateIssue(unittest.TestCase): @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", @@ -296,7 +310,7 @@ class TestCreateIssue(unittest.TestCase): @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_creates_issue(self, _auth, _get_all, mock_api, _role): mock_api.return_value = {"number": 1, "html_url": "https://gitea.example.com/issues/1"} - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): result = gitea_create_issue(title="Test issue", body="body text") self.assertEqual(result["number"], 1) self.assertNotIn("url", result) @@ -314,7 +328,7 @@ class TestCreateIssue(unittest.TestCase): def test_create_issue_reveal_opt_in_includes_url(self, _auth, _get_all, mock_api, _role): mock_api.return_value = {"number": 1, "html_url": "https://gitea.example.com/issues/1"} env = {**ISSUE_WRITE_ENV, "GITEA_MCP_REVEAL_ENDPOINTS": "1"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_create_issue(title="Test issue", body="body text") self.assertIn("issues/1", result["url"]) @@ -325,7 +339,18 @@ class TestCreateIssue(unittest.TestCase): @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_creates_on_prgs(self, _auth, _get_all, mock_api, _role): mock_api.return_value = {"number": 5, "html_url": "https://gitea.prgs.cc/issues/5"} - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + env = { + **ISSUE_WRITE_ENV, + "GITEA_MCP_PROFILE": "test-author-prgs", + } + with patch.dict(os.environ, env, clear=False): + import gitea_config + gitea_config._active_profile_override = None + try: + import session_context_binding as _sc + _sc._reset_session_context_for_testing() + except Exception: + pass result = gitea_create_issue(title="Test", remote="prgs") self.assertEqual(result["number"], 5) url = mock_api.call_args[0][1] @@ -338,7 +363,7 @@ class TestCreateIssue(unittest.TestCase): @patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_create_issue_required_workflow_labels_blocks(self, _auth, _get_all, mock_api, _role): - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): result = gitea_create_issue( title="Test issue", require_workflow_labels=True, @@ -378,7 +403,7 @@ class TestCreatePR(unittest.TestCase): mock_api.side_effect = api_side_effect with tempfile.TemporaryDirectory() as lock_dir: env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): _bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree) result = gitea_create_pr( title="feat: X Closes #123", @@ -421,7 +446,7 @@ class TestCreatePR(unittest.TestCase): mock_api.side_effect = api_side_effect with tempfile.TemporaryDirectory() as lock_dir: env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir, "GITEA_MCP_REVEAL_ENDPOINTS": "1"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): _bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree) result = gitea_create_pr( title="feat: X Closes #123", @@ -447,7 +472,7 @@ class TestCreatePR(unittest.TestCase): mock_api.return_value = _issue_with_labels("type:feature", "status:in-progress") with tempfile.TemporaryDirectory() as lock_dir: env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): _bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree) result = gitea_create_pr( title="feat: X Closes #123", @@ -470,7 +495,7 @@ class TestCreatePR(unittest.TestCase): worktree = os.path.realpath(os.getcwd()) with tempfile.TemporaryDirectory() as lock_dir: env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): _bind_test_lock( issue_number=123, branch_name="feat/x", @@ -495,7 +520,7 @@ class TestCloseIssue(unittest.TestCase): @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_closes_issue(self, _auth, mock_api): mock_api.return_value = {"state": "closed"} - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): result = gitea_close_issue(issue_number=42) self.assertTrue(result["success"]) self.assertIn("42", result["message"]) @@ -601,7 +626,7 @@ class TestMarkIssue(unittest.TestCase): return {} mock_api.side_effect = api_side_effect - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): result = gitea_mark_issue(issue_number=5, action="start") self.assertTrue(result["success"]) self.assertIn("claimed", result["message"]) @@ -616,7 +641,7 @@ class TestMarkIssue(unittest.TestCase): mock_api.side_effect = [ None, ] - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): result = gitea_mark_issue(issue_number=5, action="done") self.assertTrue(result["success"]) self.assertIn("released", result["message"]) @@ -629,7 +654,7 @@ class TestMarkIssue(unittest.TestCase): @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_missing_label_raises(self, _auth, mock_api, _labels): - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): with self.assertRaises(RuntimeError): gitea_mark_issue(issue_number=5, action="start") @@ -641,12 +666,12 @@ class TestAuthErrors(unittest.TestCase): @patch("mcp_server.get_auth_header", return_value=None) def test_no_credentials_raises(self, _auth): - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): with self.assertRaises(RuntimeError): gitea_create_issue(title="test") def test_unknown_remote_raises(self): - with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True): + with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=False): with self.assertRaises(ValueError): gitea_create_issue(title="test", remote="nonexistent") @@ -860,7 +885,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="abc123", do="squash", @@ -893,7 +918,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_changed_files=["b.py", "a.py"], @@ -914,7 +939,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="abc123", remote="prgs", @@ -945,7 +970,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="abc123", remote="prgs", @@ -962,7 +987,7 @@ class TestMergePR(unittest.TestCase): def test_missing_confirmation_blocks_with_no_api_call(self, _auth, mock_api): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr(pr_number=8, confirmation="", expected_head_sha="abc123", remote="prgs") self.assertFalse(r["performed"]) self.assertTrue(any("explicit confirmation required" in x for x in r["reasons"])) @@ -973,7 +998,7 @@ class TestMergePR(unittest.TestCase): def test_wrong_confirmation_blocks(self, _auth, mock_api): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 9", expected_head_sha="abc123", remote="prgs") self.assertFalse(r["performed"]) mock_api.assert_not_called() @@ -985,7 +1010,7 @@ class TestMergePR(unittest.TestCase): def test_reviewer_profile_cannot_merge(self, _auth, mock_api): env = {"GITEA_PROFILE_NAME": "prgs-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): with self.assertRaises(RuntimeError) as ctx: gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs" @@ -999,7 +1024,7 @@ class TestMergePR(unittest.TestCase): mock_api.side_effect = [{"login": "jcwalker3"}, self._pr("jcwalker3")] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs") self.assertFalse(r["performed"]) @@ -1010,7 +1035,7 @@ class TestMergePR(unittest.TestCase): def test_unknown_identity_blocks(self, _auth): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs") self.assertFalse(r["performed"]) @@ -1022,7 +1047,7 @@ class TestMergePR(unittest.TestCase): def test_unknown_profile_blocks(self, _auth, mock_api): mock_api.side_effect = [{"login": "merger-bot"}, self._pr("author-bot")] env = {} # no GITEA_ALLOWED_OPERATIONS → empty allowed ops - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs") self.assertFalse(r["performed"]) @@ -1094,7 +1119,7 @@ class TestMergePR(unittest.TestCase): def test_profile_without_merge_permission_blocks(self, _auth, mock_api): env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): with self.assertRaises(RuntimeError) as ctx: gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs") @@ -1110,7 +1135,7 @@ class TestMergePR(unittest.TestCase): {"login": "merger-bot"}, self._pr("author-bot", state="closed")] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs") self.assertFalse(r["performed"]) @@ -1124,7 +1149,7 @@ class TestMergePR(unittest.TestCase): {"login": "merger-bot"}, self._pr("author-bot", mergeable=False)] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs") self.assertFalse(r["performed"]) @@ -1138,7 +1163,7 @@ class TestMergePR(unittest.TestCase): {"login": "merger-bot"}, self._pr("author-bot", mergeable=None)] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs") self.assertFalse(r["performed"]) @@ -1156,7 +1181,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="deadbeef", remote="prgs") @@ -1177,7 +1202,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_changed_files=["a.py", "b.py"], @@ -1210,7 +1235,7 @@ class TestMergePR(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge", "GITEA_TOKEN": "super-secret-token"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="abc123", remote="prgs", @@ -1229,7 +1254,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="abc123", remote="prgs", @@ -1252,7 +1277,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), remote="prgs", expected_head_sha=new_sha) @@ -1275,7 +1300,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="abc123", remote="prgs", @@ -1307,7 +1332,7 @@ class TestMergePR(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_merge_pr( pr_number=8, confirmation=self._confirm(8), expected_head_sha="abc123", remote="prgs", @@ -1614,10 +1639,8 @@ class TestCommitFiles(unittest.TestCase): files = [ {"operation": "create", "path": "test.txt", "content": "SGVsbG8="} ] - env = { - "GITEA_ALLOWED_OPERATIONS": "gitea.repo.commit", - } - with patch.dict(os.environ, env, clear=True): + env = shared_mutation_env("test-author-dadeschools") + with patch.dict(os.environ, env, clear=False): result = gitea_commit_files( files=files, message="Initial commit", @@ -1732,7 +1755,7 @@ class TestRuntimeProfile(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read, review , approve", "GITEA_BASE_URL": "https://gitea.example.invalid", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): p = get_profile() self.assertEqual(p["profile_name"], "gitea-reviewer") self.assertEqual(p["allowed_operations"], ["read", "review", "approve"]) @@ -1743,7 +1766,7 @@ class TestRuntimeProfile(unittest.TestCase): "GITEA_PROFILE_NAME": "gitea-author", "GITEA_TOKEN": "super-secret-token", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): p = get_profile() blob = repr(p).lower() # The token VALUE must never appear. (The field name @@ -1760,7 +1783,7 @@ class TestRuntimeProfile(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read,review,approve", "GITEA_TOKEN": "super-secret-token", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_whoami(remote="prgs") self.assertEqual(result["profile"]["profile_name"], "gitea-reviewer") self.assertEqual( @@ -1781,7 +1804,7 @@ class TestRuntimeProfile(unittest.TestCase): "GITEA_AUDIT_LABEL": "reviewer-runtime", "GITEA_TOKEN_SOURCE": "keychain:prgs-reviewer-token", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_whoami(remote="prgs") profile = result["profile"] self.assertEqual(profile["environment"], None) @@ -1850,7 +1873,7 @@ class TestProfileDiscovery(unittest.TestCase): "GITEA_AUDIT_LABEL": "reviewer-runtime", "GITEA_TOKEN_SOURCE": "GITEA_TOKEN", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): p = get_profile() self.assertEqual(p["forbidden_operations"], ["merge", "branch.push"]) self.assertEqual(p["audit_label"], "reviewer-runtime") @@ -1866,7 +1889,7 @@ class TestProfileDiscovery(unittest.TestCase): "GITEA_TOKEN_SOURCE": "GITEA_TOKEN", "GITEA_TOKEN": "super-secret-token", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_get_profile(remote="prgs") self.assertEqual(result["profile_name"], "gitea-reviewer") self.assertEqual(result["allowed_operations"], ["read", "review", "approve"]) @@ -1887,7 +1910,7 @@ class TestProfileDiscovery(unittest.TestCase): def test_discovery_never_exposes_secrets(self, _auth, mock_api): mock_api.return_value = {"id": 3, "login": "reviewer-bot"} env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_TOKEN": "super-secret-token"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_get_profile(remote="prgs") blob = repr(result).lower() for secret in ("super-secret-token", "token dgvzd", "authorization", "basic ", FAKE_AUTH.lower()): @@ -1969,7 +1992,7 @@ class TestPrEligibility(unittest.TestCase): mock_api.side_effect = [{"login": "reviewer-bot"}, self._pr("author-bot")] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=5, action="review", remote="prgs") self.assertTrue(r["eligible"]) self.assertEqual(r["authenticated_user"], "reviewer-bot") @@ -1982,7 +2005,7 @@ class TestPrEligibility(unittest.TestCase): mock_api.side_effect = [{"login": "jcwalker3"}, self._pr("jcwalker3")] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=8, action="merge", remote="prgs") self.assertFalse(r["eligible"]) self.assertIn("authenticated user is PR author", r["reasons"]) @@ -1993,7 +2016,7 @@ class TestPrEligibility(unittest.TestCase): mock_api.side_effect = [{"login": "jcwalker3"}, self._pr("jcwalker3")] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=8, action="approve", remote="prgs") self.assertFalse(r["eligible"]) self.assertIn("authenticated user is PR author", r["reasons"]) @@ -2008,7 +2031,7 @@ class TestPrEligibility(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=8, action="merge", remote="prgs") self.assertFalse(r["eligible"]) self.assertIsNone(r["mergeable"]) @@ -2020,7 +2043,7 @@ class TestPrEligibility(unittest.TestCase): mock_api.side_effect = [{"login": "author-bot"}, self._pr("someone-else")] env = {"GITEA_PROFILE_NAME": "gitea-author", "GITEA_ALLOWED_OPERATIONS": "read,pr.create"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=8, action="merge", remote="prgs") self.assertFalse(r["eligible"]) self.assertIn("profile is not allowed to merge", r["reasons"]) @@ -2032,7 +2055,7 @@ class TestPrEligibility(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review", "GITEA_FORBIDDEN_OPERATIONS": "merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=8, action="merge", remote="prgs") self.assertFalse(r["eligible"]) self.assertIn("profile forbids 'merge'", r["reasons"]) @@ -2043,7 +2066,7 @@ class TestPrEligibility(unittest.TestCase): mock_api.side_effect = [{"login": "reviewer-bot"}, self._pr("author-bot", state="closed")] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=8, action="approve", remote="prgs") self.assertFalse(r["eligible"]) self.assertIn("PR is not open (state=closed)", r["reasons"]) @@ -2052,7 +2075,7 @@ class TestPrEligibility(unittest.TestCase): def test_unknown_identity_fails_closed(self, _auth): env = {"GITEA_PROFILE_NAME": "gitea-merger", "GITEA_ALLOWED_OPERATIONS": "read,merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=8, action="merge", remote="prgs") self.assertFalse(r["eligible"]) self.assertIn("authenticated identity could not be determined", r["reasons"]) @@ -2071,7 +2094,7 @@ class TestPrEligibility(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review", "GITEA_TOKEN": "super-secret-token"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_check_pr_eligibility(pr_number=5, action="review", remote="prgs") blob = repr(r).lower() for secret in ("super-secret-token", "authorization", "basic ", FAKE_AUTH.lower()): @@ -2268,7 +2291,7 @@ class TestSubmitPrReview(unittest.TestCase): mock_api.side_effect = [{"login": "jcwalker3"}, self._pr("jcwalker3")] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", remote="prgs", final_review_decision_ready=True, @@ -2287,7 +2310,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", body="LGTM", remote="prgs", final_review_decision_ready=True, @@ -2317,7 +2340,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", body="LGTM", remote="prgs", final_review_decision_ready=True, @@ -2337,7 +2360,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", body="LGTM", remote="prgs", final_review_decision_ready=True, @@ -2363,7 +2386,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,request_changes"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="request_changes", body="needs work", remote="prgs", @@ -2384,7 +2407,7 @@ class TestSubmitPrReview(unittest.TestCase): mock_api.side_effect = [{"login": "reviewer-bot"}, self._pr("author-bot")] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review"} # no request_changes - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="request_changes", remote="prgs", final_review_decision_ready=True, @@ -2405,7 +2428,7 @@ class TestSubmitPrReview(unittest.TestCase): gitea_mark_final_review_decision(8, "comment", remote="prgs", expected_head_sha="abc123") env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="comment", body="finding", remote="prgs", final_review_decision_ready=True, @@ -2423,7 +2446,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): gitea_mark_final_review_decision( 8, "comment", remote="prgs", expected_head_sha="abc123", ) @@ -2439,7 +2462,7 @@ class TestSubmitPrReview(unittest.TestCase): def test_unknown_identity_blocks(self, _auth): env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", remote="prgs", final_review_decision_ready=True, @@ -2454,7 +2477,7 @@ class TestSubmitPrReview(unittest.TestCase): mock_api.side_effect = [{"login": "reviewer-bot"}, self._pr("author-bot")] env = {"GITEA_PROFILE_NAME": "gitea-author", "GITEA_ALLOWED_OPERATIONS": "read,pr.create"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", remote="prgs", final_review_decision_ready=True, @@ -2473,7 +2496,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", expected_head_sha="abc123", remote="prgs", @@ -2497,7 +2520,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", expected_head_sha="abc123", remote="prgs", @@ -2526,7 +2549,7 @@ class TestSubmitPrReview(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve", "GITEA_TOKEN": "super-secret-token"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): gitea_mark_final_review_decision(5, "approve", remote="prgs", expected_head_sha="abc123") r = gitea_submit_pr_review( pr_number=5, action="approve", remote="prgs", @@ -2546,7 +2569,7 @@ class TestSubmitPrReview(unittest.TestCase): ] env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", remote="prgs", final_review_decision_ready=True, @@ -2623,7 +2646,7 @@ class TestSubmitPrReview(unittest.TestCase): try: env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): r = gitea_submit_pr_review( pr_number=8, action="approve", remote="prgs", final_review_decision_ready=True, @@ -2656,7 +2679,7 @@ class TestSubmitPrReview(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "read,review,approve,request_changes"} with patch("mcp_server.gitea_get_pr_review_feedback", return_value=dict(_NO_BLOCKER_FEEDBACK)): - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): first = gitea_submit_pr_review( pr_number=8, action="approve", remote="prgs", final_review_decision_ready=True, @@ -2681,7 +2704,7 @@ class TestSubmitPrReview(unittest.TestCase): def test_remote_org_repo_validation_mismatch(self, _auth, mock_api): env = {"GITEA_PROFILE_NAME": "gitea-reviewer", "GITEA_ALLOWED_OPERATIONS": "read,review,approve"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): # Mark decision for specific remote, org, repo gitea_mark_final_review_decision(8, "approve", remote="prgs", expected_head_sha="abc123", org="MyOrg", repo="MyRepo") @@ -2719,13 +2742,29 @@ if __name__ == "__main__": class TestTrackerHygieneCleanup(unittest.TestCase): def setUp(self): + self._mut_env = patch.dict( + os.environ, + shared_mutation_env("test-author-dadeschools"), + clear=False, + ) + self._mut_env.start() mcp_server.gitea_load_review_workflow() self.mock_api = patch("mcp_server.api_request").start() self.mock_auth = patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start() patch("gitea_audit.audit_enabled", return_value=True).start() self.mock_audit = patch("gitea_audit.write_event").start() # gitea.pr.close: closing a PR via gitea_edit_pr is capability-gated (#216). - patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["read", "merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start() + patch("mcp_server.get_profile", return_value={ + "profile_name": "test-author-dadeschools", + "allowed_operations": [ + "read", "merge", "edit", "close", + "gitea.pr.close", "gitea.issue.close", "gitea.read", + ], + "audit_label": "test", + "forbidden_operations": [], + "allowed_repositories": ["913443/eAgenda"], + "base_url": "https://gitea.dadeschools.net", + }).start() patch("mcp_server._list_pr_lease_comments", return_value=[]).start() patch( "mcp_server._pr_work_lease_reviewer_block", @@ -2733,6 +2772,11 @@ class TestTrackerHygieneCleanup(unittest.TestCase): ).start() def tearDown(self): + try: + self._mut_env.stop() + except Exception: + pass + patch.stopall() def test_close_issue_removes_in_progress(self): @@ -3138,7 +3182,7 @@ class TestEndpointRedaction(unittest.TestCase): "GITEA_BASE_URL": "https://gitea.example.invalid", "GITEA_TOKEN_SOURCE": "keychain:some-item-id", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_get_profile(remote="prgs", resolve_identity=False) blob = repr(result) @@ -3160,7 +3204,7 @@ class TestEndpointRedaction(unittest.TestCase): "GITEA_TOKEN_SOURCE": "keychain:some-item-id", "GITEA_MCP_REVEAL_ENDPOINTS": "1", } - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_get_profile(remote="prgs", resolve_identity=False) self.assertEqual(result["server"], "https://gitea.prgs.cc") @@ -3172,7 +3216,7 @@ class TestEndpointRedaction(unittest.TestCase): try: env = {"GITEA_MCP_CONFIG": path, "GITEA_MCP_PROFILE": "prgs-author"} - with patch.dict(os.environ, env, clear=True), \ + with patch.dict(os.environ, env, clear=False), \ patch("gitea_config._keychain_token", return_value="x"): result = gitea_audit_config() finally: @@ -3260,7 +3304,7 @@ class TestIssueCommentTools(unittest.TestCase): def test_list_reveal_opt_in_includes_url(self, _auth, mock_api): mock_api.return_value = [self._comment()] env = dict(self.AUTHOR_ENV, GITEA_MCP_REVEAL_ENDPOINTS="1") - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_list_issue_comments(issue_number=9, remote="prgs") self.assertIn("issuecomment-101", result["comments"][0]["url"]) @@ -3269,7 +3313,7 @@ class TestIssueCommentTools(unittest.TestCase): def test_list_blocked_without_read_permission(self, _auth, mock_api): env = {"GITEA_PROFILE_NAME": "gitea-writer-only", "GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_list_issue_comments(issue_number=9, remote="prgs") self.assertFalse(result["success"]) self.assertTrue(result["reasons"]) @@ -3316,7 +3360,7 @@ class TestIssueCommentTools(unittest.TestCase): def test_create_reveal_opt_in_includes_url(self, _auth, mock_api): mock_api.return_value = self._comment(555) env = dict(self.AUTHOR_ENV, GITEA_MCP_REVEAL_ENDPOINTS="1") - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_create_issue_comment( issue_number=9, body="posted", remote="prgs") self.assertIn("issuecomment-555", result["url"]) @@ -3328,7 +3372,7 @@ class TestIssueCommentTools(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.pr.review,gitea.pr.comment," "gitea.pr.approve,gitea.pr.merge"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_create_issue_comment( issue_number=9, body="posted", remote="prgs") self.assertFalse(result["success"]) @@ -3342,7 +3386,7 @@ class TestIssueCommentTools(unittest.TestCase): env = {"GITEA_PROFILE_NAME": "gitea-author", "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment", "GITEA_FORBIDDEN_OPERATIONS": "gitea.issue.comment"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_create_issue_comment( issue_number=9, body="posted", remote="prgs") self.assertFalse(result["success"]) @@ -3478,7 +3522,7 @@ class TestIssueCommentPermissionSeparation(unittest.TestCase): "GITEA_ALLOWED_OPERATIONS": "gitea.branch.create,gitea.branch.push,gitea.pr.comment," "gitea.pr.create,gitea.read,gitea.repo.commit"} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): result = gitea_create_issue_comment( issue_number=51, body="audit findings", remote="prgs") self.assertFalse(result["success"]) @@ -3663,11 +3707,12 @@ class TestIssueLocking(unittest.TestCase): def setUp(self): self._lock_dir = tempfile.TemporaryDirectory() + # Most locking tests target remote=prgs; host-align profile (#714). env = { - **ISSUE_WRITE_ENV, + **shared_mutation_env("test-author-prgs"), "GITEA_ISSUE_LOCK_DIR": self._lock_dir.name, } - self._env_patcher = patch.dict(os.environ, env, clear=True) + self._env_patcher = patch.dict(os.environ, env, clear=False) self._env_patcher.start() self._dup_fetcher_patcher = patch( "mcp_server.issue_duplicate_context_fetcher", @@ -3681,8 +3726,9 @@ class TestIssueLocking(unittest.TestCase): self._lock_dir.cleanup() def _create_pr_env(self) -> dict: + # PR-locking tests call create_pr with remote=prgs. return { - **CREATE_PR_ENV, + **shared_mutation_env("test-author-prgs"), "GITEA_ISSUE_LOCK_DIR": self._lock_dir.name, } @@ -3701,7 +3747,7 @@ class TestIssueLocking(unittest.TestCase): self.assertIn("created_at", res["work_lease"]) self.assertIn("expires_at", res["work_lease"]) self.assertIn("last_heartbeat_at", res["work_lease"]) - self.assertEqual(res["work_lease"]["claimant"]["profile"], "gitea-default") + self.assertIn(res["work_lease"]["claimant"]["profile"], ("gitea-default", "test-author-prgs", "prgs-author", "gitea-author")) self.assertIn("lock_file_path", res) lock = issue_lock_store.read_lock_file(res["lock_file_path"]) self.assertIn("worktree_path", lock) @@ -3820,7 +3866,7 @@ class TestIssueLocking(unittest.TestCase): @patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_lock_issue_blocks_active_same_operation_lease(self, _auth, _api, _git_state): - prgs_repo = mcp_server.REMOTES["prgs"]["repo"] + prgs_repo = "Gitea-Tools" # #714 session-bound repo (not REMOTES Timesheet default) issue_lock_store.save_lock_file( issue_lock_store.lock_file_path( remote="prgs", @@ -3857,7 +3903,7 @@ class TestIssueLocking(unittest.TestCase): Dead-pid / missing-worktree reclaim is covered by issue_lock_store unit tests. This MCP path must remain fail-closed for sticky expired foreign ownership. """ - prgs_repo = mcp_server.REMOTES["prgs"]["repo"] + prgs_repo = "Gitea-Tools" # #714 session-bound repo (not REMOTES Timesheet default) # Present worktree + live pid => reclaim_allowed=False even though expires_at is past. sticky_worktree = tempfile.mkdtemp(prefix="gitea-sticky-lease-") self.addCleanup(lambda: shutil.rmtree(sticky_worktree, ignore_errors=True)) @@ -4051,12 +4097,12 @@ class TestIssueLocking(unittest.TestCase): worktree = os.path.realpath(os.getcwd()) with tempfile.TemporaryDirectory() as lock_dir: env = {**self._create_pr_env(), "GITEA_ISSUE_LOCK_DIR": lock_dir} - with patch.dict(os.environ, env, clear=True): + with patch.dict(os.environ, env, clear=False): issue_lock_store.save_lock_file( issue_lock_store.lock_file_path( remote="prgs", org="Scaled-Tech-Consulting", - repo=mcp_server.REMOTES["prgs"]["repo"], + repo="Gitea-Tools", # #714 session-bound issue_number=447, lock_dir=lock_dir, ), @@ -4065,7 +4111,7 @@ class TestIssueLocking(unittest.TestCase): branch_name="feat/issue-447-lock-provenance", remote="prgs", org="Scaled-Tech-Consulting", - repo=mcp_server.REMOTES["prgs"]["repo"], + repo="Gitea-Tools", # #714 session-bound worktree_path=worktree, lock_provenance=None, ), diff --git a/tests/test_merger_lease_adoption.py b/tests/test_merger_lease_adoption.py index 1227c0a..6044082 100644 --- a/tests/test_merger_lease_adoption.py +++ b/tests/test_merger_lease_adoption.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Merger lease adoption / recovery (#536).""" import os diff --git a/tests/test_op_normalization.py b/tests/test_op_normalization.py index 15c0b56..430d970 100644 --- a/tests/test_op_normalization.py +++ b/tests/test_op_normalization.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Operation-name normalization table and enforcement tests — issue #106. Covers the required matrix from #106: diff --git a/tests/test_operation_scoped_roles.py b/tests/test_operation_scoped_roles.py index fc029bd..c20eb05 100644 --- a/tests/test_operation_scoped_roles.py +++ b/tests/test_operation_scoped_roles.py @@ -40,6 +40,7 @@ CONFIG_TEST = { "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, "allowed_operations": ["gitea.read", "gitea.issue.create", "gitea.pr.create", "gitea.branch.push", "gitea.issue.comment"], "forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"], + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools", "913443/eAgenda"], "execution_profile": "author-profile" }, "reviewer-profile": { @@ -51,6 +52,7 @@ CONFIG_TEST = { "auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"}, "allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.approve", "gitea.pr.merge", "gitea.issue.comment"], "forbidden_operations": ["gitea.pr.create", "gitea.branch.push"], + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools", "913443/eAgenda"], "execution_profile": "reviewer-profile" } } @@ -65,6 +67,15 @@ class TestOperationScopedRoles(unittest.TestCase): }) self._remotes_patch.start() mcp_server._IDENTITY_CACHE.clear() + self._url = patch.object( + mcp_server, + "_local_git_remote_url", + side_effect=lambda n: { + "prgs": "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git", + "dadeschools": "https://gitea.dadeschools.net/913443/eAgenda.git", + }.get(n), + ) + self._url.start() gitea_config._active_profile_override = None mcp_server._MUTATION_AUTHORITY = None self._dir = tempfile.TemporaryDirectory() @@ -72,6 +83,11 @@ class TestOperationScopedRoles(unittest.TestCase): self._write_config(CONFIG_TEST) def tearDown(self): + try: + self._url.stop() + except Exception: + pass + self._remotes_patch.stop() mcp_server._IDENTITY_CACHE.clear() gitea_config._active_profile_override = None diff --git a/tests/test_operator_guide.py b/tests/test_operator_guide.py index 5c751d1..24bd6de 100644 --- a/tests/test_operator_guide.py +++ b/tests/test_operator_guide.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for the operator guide / project skills MCP tools (#128). Read-only capability-discovery tools: mcp_get_control_plane_guide, diff --git a/tests/test_post_merge_moot_lease.py b/tests/test_post_merge_moot_lease.py index a71cb72..9f9095b 100644 --- a/tests/test_post_merge_moot_lease.py +++ b/tests/test_post_merge_moot_lease.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Post-merge moot reviewer-lease handling (#515). Covers: diff --git a/tests/test_pr_lease_comments_non_list_guard.py b/tests/test_pr_lease_comments_non_list_guard.py index 9b21a24..490e0de 100644 --- a/tests/test_pr_lease_comments_non_list_guard.py +++ b/tests/test_pr_lease_comments_non_list_guard.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Regression tests for non-list API payloads on PR/issue comment listing (#485).""" import os import sys diff --git a/tests/test_remote_repo_guard.py b/tests/test_remote_repo_guard.py index 7c0ce53..7837f51 100644 --- a/tests/test_remote_repo_guard.py +++ b/tests/test_remote_repo_guard.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Regression coverage for the remote/repo mismatch guard (#530). Bare ``remote=prgs`` historically resolved to ``Scaled-Tech-Consulting/Timesheet`` @@ -162,12 +166,12 @@ class TestResolveServerWiring(unittest.TestCase): self.addCleanup(self.server.REMOTES.__setitem__, "prgs", original) self.server.REMOTES["prgs"] = {**original, "repo": repo} - def test_resolve_blocks_on_default_repo_mismatch(self): + def test_resolve_prefers_workspace_over_timesheet_default(self): + """#714: bare remote=prgs must use workspace Gitea-Tools, not REMOTES Timesheet.""" self._set_prgs_default_repo("Timesheet") - with self.assertRaises(RuntimeError) as ctx: - self.server._resolve("prgs", None, None, None) - self.assertIn("Timesheet", str(ctx.exception)) - self.assertIn("Gitea-Tools", str(ctx.exception)) + host, org, repo = self.server._resolve("prgs", None, None, None) + self.assertEqual(org, "Scaled-Tech-Consulting") + self.assertEqual(repo, "Gitea-Tools") def test_resolve_allows_explicit_org_and_repo(self): self._set_prgs_default_repo("Timesheet") diff --git a/tests/test_role_namespace_gate.py b/tests/test_role_namespace_gate.py index 564e39c..571f406 100644 --- a/tests/test_role_namespace_gate.py +++ b/tests/test_role_namespace_gate.py @@ -1,3 +1,7 @@ +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 """Tests for reviewer/author namespace mismatch walls (#209).""" import json import os @@ -36,6 +40,7 @@ CONFIG = { "gitea.pr.approve", "gitea.pr.merge", "gitea.pr.review", ], "execution_profile": "prgs-author", + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], }, "prgs-reviewer": { "enabled": True, @@ -51,6 +56,7 @@ CONFIG = { "gitea.pr.create", "gitea.branch.push", "gitea.issue.create", ], "execution_profile": "prgs-reviewer", + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], }, "prgs-reviewer-issue-writer": { "enabled": True, @@ -63,12 +69,13 @@ CONFIG = { ], "forbidden_operations": ["gitea.pr.create"], "execution_profile": "prgs-reviewer-issue-writer", + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], }, }, "rules": {"allow_runtime_switching": False}, } -ISSUE_WRITE_ENV = {"GITEA_ALLOWED_OPERATIONS": "gitea.issue.create"} +ISSUE_WRITE_ENV = {} # config-backed; operations come from profile allowlist (#714) class NamespaceGateBase(unittest.TestCase): @@ -199,7 +206,7 @@ class TestMutationAuditFields(NamespaceGateBase): def test_successful_create_issue_audit_includes_namespace_fields( self, _auth, mock_api, _get_all, _role ): - env = {**self._env("prgs-author", audit=True), **ISSUE_WRITE_ENV} + env = self._env("prgs-author", audit=True) with patch.dict(os.environ, env, clear=True): mcp_server.gitea_create_issue(title="audited", remote="prgs") with open(self.audit_path, encoding="utf-8") as fh: diff --git a/tests/test_role_session_router.py b/tests/test_role_session_router.py index 3d5d0d0..a0833a2 100644 --- a/tests/test_role_session_router.py +++ b/tests/test_role_session_router.py @@ -36,6 +36,7 @@ CONFIG = { "gitea.pr.approve", "gitea.pr.merge", "gitea.pr.review", ], "execution_profile": "prgs-author", + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], }, "prgs-reviewer": { "enabled": True, @@ -51,6 +52,7 @@ CONFIG = { "gitea.pr.create", "gitea.branch.push", "gitea.issue.create", ], "execution_profile": "prgs-reviewer", + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], }, "prgs-merger": { "enabled": True, @@ -65,6 +67,7 @@ CONFIG = { "gitea.pr.create", "gitea.branch.push", "gitea.pr.approve", ], "execution_profile": "prgs-merger", + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], }, "prgs-reconciler": { "enabled": True, @@ -81,6 +84,7 @@ CONFIG = { "gitea.branch.push", ], "execution_profile": "prgs-reconciler", + "allowed_repositories": ["Scaled-Tech-Consulting/Gitea-Tools"], }, }, "rules": {"allow_runtime_switching": False}, diff --git a/tests/test_runtime_clarity.py b/tests/test_runtime_clarity.py index 1e94ada..fabd6aa 100644 --- a/tests/test_runtime_clarity.py +++ b/tests/test_runtime_clarity.py @@ -35,7 +35,8 @@ CONFIG_SWITCHING_DISABLED = { "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, "allowed_operations": ["gitea.read", "gitea.pr.create", "gitea.branch.push"], "forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"], - "execution_profile": "author-profile" + "execution_profile": "author-profile", + "allowed_repositories": ["Example-Org/Example-Repo"], }, "reviewer-profile": { "enabled": True, @@ -45,7 +46,8 @@ CONFIG_SWITCHING_DISABLED = { "auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"}, "allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.approve"], "forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.merge"], - "execution_profile": "reviewer-profile" + "execution_profile": "reviewer-profile", + "allowed_repositories": ["Example-Org/Example-Repo"], }, "merger-profile": { "enabled": True, @@ -55,7 +57,8 @@ CONFIG_SWITCHING_DISABLED = { "auth": {"type": "env", "name": "GITEA_TOKEN_MERGER"}, "allowed_operations": ["gitea.read", "gitea.pr.merge"], "forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.approve"], - "execution_profile": "merger-profile" + "execution_profile": "merger-profile", + "allowed_repositories": ["Example-Org/Example-Repo"], } }, "rules": { diff --git a/tests/test_stale_review_decision_lock_cleanup.py b/tests/test_stale_review_decision_lock_cleanup.py index 1425644..7beed2b 100644 --- a/tests/test_stale_review_decision_lock_cleanup.py +++ b/tests/test_stale_review_decision_lock_cleanup.py @@ -1,15 +1,11 @@ -"""Stale #332 review-decision lock cleanup (#594). - -Proves: - a. active terminal locks still block unrelated terminal reviews - b. merged/closed PR locks can be cleared canonically - c. cleanup cannot bypass review gates on open PRs - d. after moot cleanup, mark_ready for a different PR can proceed - (PR #592-style after PR #586-style stale approve) -""" - from __future__ import annotations + +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 + import os import unittest from unittest.mock import patch diff --git a/tests/test_workflow_skill.py b/tests/test_workflow_skill.py index 78072a2..dde889b 100644 --- a/tests/test_workflow_skill.py +++ b/tests/test_workflow_skill.py @@ -1,7 +1,11 @@ -"""Tests for canonical workflow skill naming and preflight (#551).""" - from __future__ import annotations + +import sys as _sys +from pathlib import Path as _Path +_sys.path.insert(0, str(_Path(__file__).resolve().parent)) +from mutation_profile_fixture import shared_mutation_env # noqa: E402 + import os import tempfile import unittest