Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 77 Pull Requests 10 Actions Packages Projects Releases Wiki Activity

fix: isolate immutable session context tests (#714)

Browse Source
This commit is contained in:
sysadmin
2026-07-15 02:26:27 -04:00
parent 632c568865
commit 29ffe1407f
7 changed files with 303 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files
gitea_mcp_server.py
+36 -38
View File
@@ -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:
session_context_binding.py
+117 -56
View File
@@ -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,
tests/conftest.py
+35 -17
View File
@@ -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()
tests/test_commit_payloads.py
+3
View File
@@ -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(
tests/test_issue_714_session_context_immutability.py
+110 -10
View File
@@ -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()
tests/test_operation_scoped_roles.py
-3
View File
@@ -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):
tests/test_resolve_task_capability.py
+2 -2
View File
@@ -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(