feat(preflight): block workspace edits before identity/capability verification (Closes #210)

This commit is contained in:
2026-07-05 20:31:51 -04:00
parent 8dd1b2ee34
commit ab95f1b253
4 changed files with 226 additions and 10 deletions
+99
View File
@@ -157,6 +157,89 @@ PROJECT_ROOT = os.path.dirname(os.path.abspath(__file__))
if PROJECT_ROOT not in sys.path: if PROJECT_ROOT not in sys.path:
sys.path.insert(0, PROJECT_ROOT) sys.path.insert(0, PROJECT_ROOT)
import json
_preflight_whoami_called = False
_preflight_capability_called = False
_preflight_whoami_violation = False
_preflight_capability_violation = False
_preflight_resolved_role = None
def record_preflight_check(type_name: str, resolved_role: str | None = None):
"""Record a pre-flight check (whoami or capability) and check for workspace edits."""
global _preflight_whoami_called, _preflight_capability_called
global _preflight_whoami_violation, _preflight_capability_violation
global _preflight_resolved_role
in_test = "pytest" in sys.modules or "unittest" in sys.modules
if os.environ.get("GITEA_TEST_FORCE_DIRTY"):
is_dirty = True
elif in_test:
is_dirty = False
else:
is_dirty = False
try:
res = subprocess.run(
["git", "status", "--porcelain"],
capture_output=True, text=True, cwd=PROJECT_ROOT
)
for line in res.stdout.splitlines():
if line and not line.startswith("??"):
is_dirty = True
break
except Exception:
pass
if is_dirty:
if type_name == "whoami" and not _preflight_whoami_called:
_preflight_whoami_violation = True
if type_name == "capability" and not _preflight_capability_called:
_preflight_capability_violation = True
if type_name == "whoami":
_preflight_whoami_called = True
elif type_name == "capability":
_preflight_capability_called = True
if resolved_role:
_preflight_resolved_role = resolved_role
def verify_preflight_purity(remote: str | None = None):
"""Verify that identity and capability were verified prior to edits, and that reviewers made no edits."""
in_test = "pytest" in sys.modules or "unittest" in sys.modules
if in_test and not os.environ.get("GITEA_TEST_FORCE_DIRTY"):
return
if not _preflight_whoami_called:
raise RuntimeError("Pre-flight order violation: Identity (gitea_whoami) has not been verified (fail closed)")
if not _preflight_capability_called:
raise RuntimeError("Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)")
if _preflight_whoami_violation:
raise RuntimeError("Pre-flight order violation: Workspace file edits occurred before gitea_whoami verification (fail closed)")
if _preflight_capability_violation:
raise RuntimeError("Pre-flight order violation: Workspace file edits occurred before gitea_resolve_task_capability verification (fail closed)")
if os.environ.get("GITEA_TEST_FORCE_DIRTY"):
is_dirty = True
elif in_test:
is_dirty = False
else:
is_dirty = False
try:
res = subprocess.run(
["git", "status", "--porcelain"],
capture_output=True, text=True, cwd=PROJECT_ROOT
)
for line in res.stdout.splitlines():
if line and not line.startswith("??"):
is_dirty = True
break
except Exception:
pass
if _preflight_resolved_role == "reviewer" and is_dirty:
raise RuntimeError("Reviewer role violation: Reviewer profile is forbidden from modifying tracked workspace files (fail closed)")
from mcp.server.fastmcp import FastMCP # noqa: E402 from mcp.server.fastmcp import FastMCP # noqa: E402
from gitea_auth import ( # noqa: E402 from gitea_auth import ( # noqa: E402
@@ -471,6 +554,7 @@ def gitea_create_issue(
"number": None, "number": None,
"reasons": block_reasons, "reasons": block_reasons,
} }
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
@@ -615,6 +699,7 @@ def gitea_create_pr(
Returns: Returns:
dict with 'number' of the created PR ('url' only with the reveal opt-in). dict with 'number' of the created PR ('url' only with the reveal opt-in).
""" """
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
# ── Issue Lock Validation (Issue #194 / #196) ── # ── Issue Lock Validation (Issue #194 / #196) ──
@@ -1388,6 +1473,7 @@ def _evaluate_pr_review_submission(
final_review_decision_ready: bool = False, final_review_decision_ready: bool = False,
) -> dict: ) -> dict:
"""Shared gate chain for live submit and dry-run review tools.""" """Shared gate chain for live submit and dry-run review tools."""
verify_preflight_purity(remote)
action = (action or "").strip().lower() action = (action or "").strip().lower()
result = { result = {
"requested_action": action, "requested_action": action,
@@ -1763,6 +1849,8 @@ def gitea_edit_pr(
if not payload: if not payload:
raise ValueError("At least one field to edit (title, body, state, base) must be provided.") raise ValueError("At least one field to edit (title, body, state, base) must be provided.")
verify_preflight_purity(remote)
# PR closure is a first-class capability, distinct from retitling or # PR closure is a first-class capability, distinct from retitling or
# rebasing edits (#216). Gate BEFORE auth/API setup so a blocked close # rebasing edits (#216). Gate BEFORE auth/API setup so a blocked close
# never touches the network. # never touches the network.
@@ -1880,6 +1968,7 @@ def gitea_commit_files(
Returns: Returns:
dict with success status and commit/branch information. dict with success status and commit/branch information.
""" """
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
url = f"{repo_api_url(h, o, r)}/contents" url = f"{repo_api_url(h, o, r)}/contents"
@@ -1974,6 +2063,7 @@ def gitea_merge_pr(
reasons/gates passed or blocked, and merge result / merge commit if reasons/gates passed or blocked, and merge result / merge commit if
available. Never secrets. available. Never secrets.
""" """
verify_preflight_purity(remote)
do = (do or "").strip().lower() do = (do or "").strip().lower()
result = { result = {
"performed": False, "performed": False,
@@ -2355,6 +2445,7 @@ def gitea_delete_branch(
Returns: Returns:
dict with 'success' and 'message'. dict with 'success' and 'message'.
""" """
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
import urllib.parse import urllib.parse
@@ -2386,6 +2477,7 @@ def gitea_close_issue(
Returns: Returns:
dict with 'success' boolean and 'message'. dict with 'success' boolean and 'message'.
""" """
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
url = f"{repo_api_url(h, o, r)}/issues/{issue_number}" url = f"{repo_api_url(h, o, r)}/issues/{issue_number}"
@@ -2684,6 +2776,7 @@ def gitea_create_issue_comment(
(permission blocks also carry a structured 'permission_report', (permission blocks also carry a structured 'permission_report',
#142). #142).
""" """
verify_preflight_purity(remote)
gate_reasons = _profile_operation_gate("gitea.issue.comment") gate_reasons = _profile_operation_gate("gitea.issue.comment")
reasons = list(gate_reasons) reasons = list(gate_reasons)
if not (body or "").strip(): if not (body or "").strip():
@@ -3252,6 +3345,7 @@ def gitea_whoami(
'email', 'server', 'remote', and 'profile' (safe runtime profile 'email', 'server', 'remote', and 'profile' (safe runtime profile
metadata: profile_name + allowed_operations; never the token). metadata: profile_name + allowed_operations; never the token).
""" """
record_preflight_check("whoami")
if remote not in REMOTES: if remote not in REMOTES:
raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}") raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}")
h = host or REMOTES[remote]["host"] h = host or REMOTES[remote]["host"]
@@ -3771,6 +3865,7 @@ def gitea_mark_issue(
if action not in ("start", "done"): if action not in ("start", "done"):
raise ValueError(f"action must be 'start' or 'done', got '{action}'") raise ValueError(f"action must be 'start' or 'done', got '{action}'")
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
@@ -3853,6 +3948,7 @@ def gitea_create_label(
Returns: Returns:
dict containing the created label details. dict containing the created label details.
""" """
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
@@ -3892,6 +3988,7 @@ def gitea_set_issue_labels(
Returns: Returns:
list of all labels currently applied to the issue. list of all labels currently applied to the issue.
""" """
verify_preflight_purity(remote)
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
@@ -4162,6 +4259,8 @@ def gitea_resolve_task_capability(
required_permission = TASK_MAP[task]["permission"] required_permission = TASK_MAP[task]["permission"]
required_role = TASK_MAP[task]["role"] required_role = TASK_MAP[task]["role"]
record_preflight_check("capability", required_role)
profile = get_profile() profile = get_profile()
config = gitea_config.load_config() config = gitea_config.load_config()
+19 -10
View File
@@ -822,6 +822,7 @@ HANDOFF_BASE_FIELDS = (
("Files changed", ("files changed", "changed", "files")), ("Files changed", ("files changed", "changed", "files")),
("Validation", ("validation",)), ("Validation", ("validation",)),
("Mutations", ("mutations",)), ("Mutations", ("mutations",)),
("Workspace mutations", ("workspace mutations",)),
("Current status", ("current status", "status")), ("Current status", ("current status", "status")),
("Blockers", ("blockers",)), ("Blockers", ("blockers",)),
("Next", ("next",)), ("Next", ("next",)),
@@ -872,7 +873,7 @@ def _handoff_section_lines(report_text):
return lines[start:] return lines[start:]
def assess_controller_handoff(report_text, role=None): def assess_controller_handoff(report_text, role=None, local_edits=False):
"""Issue #182: final reports without a Controller Handoff downgrade. """Issue #182: final reports without a Controller Handoff downgrade.
Verdicts: Verdicts:
@@ -897,10 +898,14 @@ def assess_controller_handoff(report_text, role=None):
} }
labels = [] labels = []
fields_dict = {}
for line in section: for line in section:
stripped = line.strip().lstrip("-*").strip() stripped = line.strip().lstrip("-*").strip()
if ":" in stripped: if ":" in stripped:
labels.append(stripped.split(":", 1)[0].strip().lower()) k, v = stripped.split(":", 1)
label = k.strip().lower()
labels.append(label)
fields_dict[label] = v.strip()
required = list(HANDOFF_BASE_FIELDS) required = list(HANDOFF_BASE_FIELDS)
required.extend(HANDOFF_ROLE_FIELDS.get(role or "", ())) required.extend(HANDOFF_ROLE_FIELDS.get(role or "", ()))
@@ -919,15 +924,7 @@ def assess_controller_handoff(report_text, role=None):
"reasons": [f"handoff missing required field: {m}" "reasons": [f"handoff missing required field: {m}"
for m in missing], for m in missing],
} }
# Validate issue/PR references for exact number and no forbidden terms (Issue #194 / #196) # Validate issue/PR references for exact number and no forbidden terms (Issue #194 / #196)
fields_dict = {}
for line in section:
stripped = line.strip().lstrip("-*").strip()
if ":" in stripped:
k, v = stripped.split(":", 1)
fields_dict[k.strip().lower()] = v.strip()
for alias in ("selected issue", "pr number opened", "pr opened", "pr number", "selected pr"): for alias in ("selected issue", "pr number opened", "pr opened", "pr number", "selected pr"):
val = fields_dict.get(alias) val = fields_dict.get(alias)
if val: if val:
@@ -948,6 +945,18 @@ def assess_controller_handoff(report_text, role=None):
], ],
} }
if local_edits:
workspace_mutations_val = fields_dict.get("workspace mutations", "").strip().lower()
if not workspace_mutations_val or workspace_mutations_val == "none":
return {
"verdict": "incomplete",
"downgraded": True,
"missing_fields": ["Workspace mutations"],
"reasons": [
"Workspace mutations cannot be 'none' or empty when local edits exist"
],
}
return { return {
"verdict": "complete", "verdict": "complete",
"downgraded": False, "downgraded": False,
+84
View File
@@ -2813,3 +2813,87 @@ class TestIssueLocking(unittest.TestCase):
with self.assertRaises(ValueError) as ctx: with self.assertRaises(ValueError) as ctx:
gitea_create_pr(title="feat: X refs #196", head="feat/issue-196-mutations", remote="prgs") gitea_create_pr(title="feat: X refs #196", head="feat/issue-196-mutations", remote="prgs")
self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception)) self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception))
# ---------------------------------------------------------------------------
# Pre-flight ordering and workspace edit block (#210)
# ---------------------------------------------------------------------------
class TestPreflightVerification(unittest.TestCase):
"""Assert that pre-flight verification gates order correctly."""
def setUp(self):
# Save real global variables
import mcp_server
self.orig_whoami_called = mcp_server._preflight_whoami_called
self.orig_capability_called = mcp_server._preflight_capability_called
self.orig_whoami_violation = mcp_server._preflight_whoami_violation
self.orig_capability_violation = mcp_server._preflight_capability_violation
self.orig_resolved_role = mcp_server._preflight_resolved_role
# Reset state for each test
mcp_server._preflight_whoami_called = False
mcp_server._preflight_capability_called = False
mcp_server._preflight_whoami_violation = False
mcp_server._preflight_capability_violation = False
mcp_server._preflight_resolved_role = None
def tearDown(self):
# Restore real global variables
import mcp_server
mcp_server._preflight_whoami_called = self.orig_whoami_called
mcp_server._preflight_capability_called = self.orig_capability_called
mcp_server._preflight_whoami_violation = self.orig_whoami_violation
mcp_server._preflight_capability_violation = self.orig_capability_violation
mcp_server._preflight_resolved_role = self.orig_resolved_role
if "GITEA_TEST_FORCE_DIRTY" in os.environ:
del os.environ["GITEA_TEST_FORCE_DIRTY"]
def test_preflight_whoami_violation(self):
import mcp_server
os.environ["GITEA_TEST_FORCE_DIRTY"] = "1"
mcp_server._preflight_capability_called = True
mcp_server.record_preflight_check("whoami")
self.assertTrue(mcp_server._preflight_whoami_violation)
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity()
self.assertIn("Workspace file edits occurred before gitea_whoami verification", str(ctx.exception))
def test_preflight_capability_violation(self):
import mcp_server
os.environ["GITEA_TEST_FORCE_DIRTY"] = "1"
mcp_server._preflight_whoami_called = True
mcp_server.record_preflight_check("capability", resolved_role="author")
self.assertTrue(mcp_server._preflight_capability_violation)
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity()
self.assertIn("Workspace file edits occurred before gitea_resolve_task_capability verification", str(ctx.exception))
def test_preflight_not_called_fails_closed(self):
import mcp_server
os.environ["GITEA_TEST_FORCE_DIRTY"] = "1"
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity()
self.assertIn("Identity (gitea_whoami) has not been verified", str(ctx.exception))
mcp_server._preflight_whoami_called = True
with self.assertRaises(RuntimeError) as ctx2:
mcp_server.verify_preflight_purity()
self.assertIn("Task capability (gitea_resolve_task_capability) has not been resolved", str(ctx2.exception))
def test_preflight_reviewer_mutation_violation(self):
import mcp_server
mcp_server._preflight_whoami_called = True
mcp_server._preflight_capability_called = True
mcp_server._preflight_resolved_role = "reviewer"
# When dirty, reviewer edits are blocked
os.environ["GITEA_TEST_FORCE_DIRTY"] = "1"
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity()
self.assertIn("Reviewer profile is forbidden from modifying tracked workspace files", str(ctx.exception))
# When clean, reviewer is allowed
del os.environ["GITEA_TEST_FORCE_DIRTY"]
mcp_server.verify_preflight_purity()
+24
View File
@@ -789,6 +789,7 @@ class TestControllerHandoff(unittest.TestCase):
"- Files changed: review_proofs.py", "- Files changed: review_proofs.py",
"- Validation: 700 passed, 6 skipped", "- Validation: 700 passed, 6 skipped",
"- Mutations: one PR opened", "- Mutations: one PR opened",
"- Workspace mutations: none",
"- Current status: PR open", "- Current status: PR open",
"- Blockers: none", "- Blockers: none",
"- Next: review PR #999", "- Next: review PR #999",
@@ -909,6 +910,29 @@ class TestControllerHandoff(unittest.TestCase):
self.assertIn("assess_controller_handoff", skill) self.assertIn("assess_controller_handoff", skill)
self.assertIn("issue #182", skill) self.assertIn("issue #182", skill)
def test_handoff_rejects_none_workspace_mutations_when_local_edits_exist(self):
# 1. Workspace mutations: none is rejected when local_edits is True
incomplete_eq = self.BASE_HANDOFF + "\n" + "\n".join([
"- Selected issue: #196",
"- Claim/comment status: comment-claimed",
"- PR number opened: #203",
"- No review/merge: confirmed",
])
res = assess_controller_handoff(incomplete_eq, role="author", local_edits=True)
self.assertEqual(res["verdict"], "incomplete")
self.assertIn("Workspace mutations", res["missing_fields"])
# 2. Workspace mutations: edited files is allowed when local_edits is True
complete_eq = self.BASE_HANDOFF.replace("- Workspace mutations: none", "- Workspace mutations: edited review_proofs.py") + "\n" + "\n".join([
"- Selected issue: #196",
"- Claim/comment status: comment-claimed",
"- PR number opened: #203",
"- No review/merge: confirmed",
])
res2 = assess_controller_handoff(complete_eq, role="author", local_edits=True)
self.assertEqual(res2["verdict"], "complete")
class TestReviewMutationFinalReport(unittest.TestCase): class TestReviewMutationFinalReport(unittest.TestCase):
"""Final reports must list exactly one live review mutation.""" """Final reports must list exactly one live review mutation."""