test(session): cover reverse cross-host denial, concurrent init, repo drift (#714)
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) <[email protected]>
This commit is contained in:
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user