fix(workflow): durable HMAC, dedicated mint capability, head-exact approve match (#709)
Address formal review 435 REQUEST_CHANGES on PR #710: - F4: require durable GITEA_IRRECOVERABLE_AUTH_HMAC_KEY (fail closed; no ephemeral per-process secret); bind key_version into HMAC; cross-process verify works - F5: dedicated gitea.decision_lock.irrecoverable_recovery only; reject reconciler equivalence; authoritative incident body + author + content_digest; reject self-authored incident evidence - F3 residual: lock_targets_merged_pr_approval requires recorded-head match when expected_head_sha is provided (legacy no-head approve no longer primary-clears) Co-Authored-By: Grok 4.5 (xAI) <[email protected]>
This commit is contained in:
@@ -1,7 +1,8 @@
|
||||
"""#709: cross-profile decision-lock cleanup, overwrite protection, recovery.
|
||||
|
||||
Covers AC1–AC8 plus review-434 F1/F2/F3 remediations without fabricating
|
||||
historical PR provenance or special-casing live PR numbers in production code.
|
||||
Covers AC1–AC8 plus review-434 F1/F2/F3 and review-435 F3-residual/F4/F5
|
||||
remediations without fabricating historical PR provenance or special-casing
|
||||
live PR numbers in production code.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -63,6 +64,58 @@ RECONCILER_OPS = [
|
||||
"gitea.pr.comment",
|
||||
"gitea.issue.comment",
|
||||
]
|
||||
DEDICATED_RECOVERY_OPS = RECONCILER_OPS + [
|
||||
irp.CAPABILITY_IRRECOVERABLE_RECOVERY,
|
||||
]
|
||||
DURABLE_TEST_HMAC_KEY = "0" * 64 # 32-byte hex durable key for F4 tests
|
||||
|
||||
|
||||
def _canonical_incident_body(
|
||||
*,
|
||||
pr_number=42,
|
||||
head=HEAD_A,
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
incident_issue=700,
|
||||
narrative="forensic diagnosis",
|
||||
):
|
||||
return irp.build_canonical_incident_body(
|
||||
remote=remote,
|
||||
org=org,
|
||||
repo=repo,
|
||||
pr_number=pr_number,
|
||||
expected_head_sha=head,
|
||||
incident_issue=incident_issue,
|
||||
narrative=narrative,
|
||||
)
|
||||
|
||||
|
||||
def _incident_comment_payload(
|
||||
*,
|
||||
comment_id=11489,
|
||||
author="controller-ops",
|
||||
pr_number=42,
|
||||
head=HEAD_A,
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
incident_issue=700,
|
||||
):
|
||||
return {
|
||||
"id": comment_id,
|
||||
"body": _canonical_incident_body(
|
||||
pr_number=pr_number,
|
||||
head=head,
|
||||
remote=remote,
|
||||
org=org,
|
||||
repo=repo,
|
||||
incident_issue=incident_issue,
|
||||
),
|
||||
"user": {"login": author},
|
||||
"created_at": "2026-07-13T00:00:00Z",
|
||||
"html_url": f"https://gitea.example/{org}/{repo}/issues/{incident_issue}#issuecomment-{comment_id}",
|
||||
}
|
||||
|
||||
|
||||
def _mint_auth(
|
||||
@@ -142,6 +195,24 @@ class TestAC1TargetApproval(unittest.TestCase):
|
||||
)
|
||||
)
|
||||
|
||||
def test_f3_residual_rejects_legacy_no_head_when_expected_head_given(self):
|
||||
"""Primary approve-match requires recorded-head (#709 F3 residual / 435)."""
|
||||
# APPROVE without head fields — legacy ledger.
|
||||
legacy = _lock([APPROVE]) # no head=
|
||||
self.assertIsNone(srdl.mutation_head_sha(APPROVE, legacy))
|
||||
self.assertFalse(
|
||||
srdl.lock_targets_merged_pr_approval(
|
||||
legacy,
|
||||
pr_number=100,
|
||||
expected_head_sha=HEAD_A,
|
||||
)
|
||||
)
|
||||
# Without expected_head_sha, PR-number-only match still works for
|
||||
# non-destructive callers that do not pass a head pin.
|
||||
self.assertTrue(
|
||||
srdl.lock_targets_merged_pr_approval(legacy, pr_number=100)
|
||||
)
|
||||
|
||||
|
||||
class TestF1AuthorizationNotSelfAssertable(unittest.TestCase):
|
||||
def test_operator_authorized_true_cannot_authorize_via_build(self):
|
||||
@@ -336,7 +407,8 @@ class TestF1AuthorizationNotSelfAssertable(unittest.TestCase):
|
||||
)
|
||||
self.assertFalse(a["allowed"])
|
||||
|
||||
def test_reconciler_capability_allowed(self):
|
||||
def test_f5_reconciler_equivalence_rejected(self):
|
||||
"""Reconciler profile without dedicated capability cannot mint (#709 F5)."""
|
||||
a = irp.assess_capability_for_irrecoverable_recovery(
|
||||
allowed_operations=RECONCILER_OPS,
|
||||
forbidden_operations=[
|
||||
@@ -347,7 +419,241 @@ class TestF1AuthorizationNotSelfAssertable(unittest.TestCase):
|
||||
role_kind="reconciler",
|
||||
profile_name="prgs-reconciler",
|
||||
)
|
||||
self.assertFalse(a["allowed"], a)
|
||||
self.assertTrue(
|
||||
any("dedicated" in r for r in a["reasons"]),
|
||||
msg=a["reasons"],
|
||||
)
|
||||
|
||||
def test_f5_dedicated_capability_allowed(self):
|
||||
a = irp.assess_capability_for_irrecoverable_recovery(
|
||||
allowed_operations=DEDICATED_RECOVERY_OPS,
|
||||
forbidden_operations=[
|
||||
"gitea.pr.approve",
|
||||
"gitea.pr.merge",
|
||||
"gitea.pr.review",
|
||||
],
|
||||
role_kind="reconciler",
|
||||
profile_name="prgs-reconciler",
|
||||
)
|
||||
self.assertTrue(a["allowed"], a)
|
||||
self.assertEqual(a["via"], "dedicated_capability")
|
||||
|
||||
|
||||
class TestF5AuthoritativeIncidentEvidence(unittest.TestCase):
|
||||
def test_any_nonempty_body_rejected(self):
|
||||
g = irp.assess_incident_evidence(
|
||||
incident_issue=700,
|
||||
incident_comment_id=11489,
|
||||
comment_payload={
|
||||
"id": 11489,
|
||||
"body": "random forensic note without canonical fields",
|
||||
"user": {"login": "controller-ops"},
|
||||
},
|
||||
expected_remote="prgs",
|
||||
expected_org="Scaled-Tech-Consulting",
|
||||
expected_repo="Gitea-Tools",
|
||||
expected_pr_number=42,
|
||||
expected_head_sha=HEAD_A,
|
||||
)
|
||||
self.assertFalse(g["valid"], g)
|
||||
|
||||
def test_missing_author_rejected(self):
|
||||
body = _canonical_incident_body()
|
||||
g = irp.assess_incident_evidence(
|
||||
incident_issue=700,
|
||||
incident_comment_id=11489,
|
||||
comment_payload={"id": 11489, "body": body, "user": {}},
|
||||
expected_remote="prgs",
|
||||
expected_org="Scaled-Tech-Consulting",
|
||||
expected_repo="Gitea-Tools",
|
||||
expected_pr_number=42,
|
||||
expected_head_sha=HEAD_A,
|
||||
)
|
||||
self.assertFalse(g["valid"], g)
|
||||
self.assertTrue(any("author" in r for r in g["reasons"]), g["reasons"])
|
||||
|
||||
def test_self_authored_rejected(self):
|
||||
g = irp.assess_incident_evidence(
|
||||
incident_issue=700,
|
||||
incident_comment_id=11489,
|
||||
comment_payload=_incident_comment_payload(author="sysadmin"),
|
||||
expected_remote="prgs",
|
||||
expected_org="Scaled-Tech-Consulting",
|
||||
expected_repo="Gitea-Tools",
|
||||
expected_pr_number=42,
|
||||
expected_head_sha=HEAD_A,
|
||||
mint_actor_username="sysadmin",
|
||||
reject_self_authored=True,
|
||||
)
|
||||
self.assertFalse(g["valid"], g)
|
||||
self.assertTrue(
|
||||
any("self-authored" in r for r in g["reasons"]), g["reasons"]
|
||||
)
|
||||
|
||||
def test_canonical_body_with_independent_author_accepted(self):
|
||||
g = irp.assess_incident_evidence(
|
||||
incident_issue=700,
|
||||
incident_comment_id=11489,
|
||||
comment_payload=_incident_comment_payload(author="controller-ops"),
|
||||
expected_remote="prgs",
|
||||
expected_org="Scaled-Tech-Consulting",
|
||||
expected_repo="Gitea-Tools",
|
||||
expected_pr_number=42,
|
||||
expected_head_sha=HEAD_A,
|
||||
mint_actor_username="sysadmin",
|
||||
reject_self_authored=True,
|
||||
)
|
||||
self.assertTrue(g["valid"], g)
|
||||
|
||||
def test_tampered_content_digest_rejected(self):
|
||||
payload = _incident_comment_payload()
|
||||
payload["body"] = payload["body"].replace(
|
||||
"content_digest: ", "content_digest: " + "f" * 64 + "x"
|
||||
)
|
||||
# Force bad digest line
|
||||
lines = []
|
||||
for line in payload["body"].splitlines():
|
||||
if line.startswith("content_digest:"):
|
||||
lines.append("content_digest: " + "0" * 64)
|
||||
else:
|
||||
lines.append(line)
|
||||
payload["body"] = "\n".join(lines)
|
||||
g = irp.assess_incident_evidence(
|
||||
incident_issue=700,
|
||||
incident_comment_id=11489,
|
||||
comment_payload=payload,
|
||||
expected_remote="prgs",
|
||||
expected_org="Scaled-Tech-Consulting",
|
||||
expected_repo="Gitea-Tools",
|
||||
expected_pr_number=42,
|
||||
expected_head_sha=HEAD_A,
|
||||
mint_actor_username="sysadmin",
|
||||
)
|
||||
self.assertFalse(g["valid"], g)
|
||||
self.assertTrue(
|
||||
any("content_digest" in r for r in g["reasons"]), g["reasons"]
|
||||
)
|
||||
|
||||
|
||||
class TestF4DurableHmacKey(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
os.environ.pop(irp.ENV_AUTH_HMAC_KEY, None)
|
||||
os.environ.pop(irp.ENV_AUTH_HMAC_KEY_VERSION, None)
|
||||
irp.reset_process_auth_secret_for_tests()
|
||||
|
||||
def test_key_version_bound_into_artifact(self):
|
||||
irp.reset_process_auth_secret_for_tests()
|
||||
auth = _mint_auth()
|
||||
self.assertIn("key_version", auth)
|
||||
self.assertTrue(auth["key_version"])
|
||||
self.assertNotIn("server_secret", auth)
|
||||
self.assertNotIn("hmac_key", auth)
|
||||
|
||||
def test_production_fails_closed_without_durable_key(self):
|
||||
"""Non-pytest process without env key must not generate ephemeral secret."""
|
||||
script = (
|
||||
"import os, sys\n"
|
||||
"os.environ.pop('PYTEST_CURRENT_TEST', None)\n"
|
||||
"os.environ.pop('GITEA_IRRECOVERABLE_AUTH_HMAC_KEY', None)\n"
|
||||
# Force non-pytest path by patching guard after import.
|
||||
"import mcp_daemon_guard as g\n"
|
||||
"g.is_pytest_runtime = lambda: False\n"
|
||||
"import irrecoverable_provenance as irp\n"
|
||||
"irp.reset_process_auth_secret_for_tests()\n"
|
||||
"try:\n"
|
||||
" irp._process_secret()\n"
|
||||
" print('UNEXPECTED_OK')\n"
|
||||
"except irp.AuthSecretError as e:\n"
|
||||
" print('FAIL_CLOSED', 'ephemeral' in str(e).lower() or 'required' in str(e).lower())\n"
|
||||
)
|
||||
env = {k: v for k, v in os.environ.items() if not k.startswith("PYTEST")}
|
||||
env.pop("PYTEST_CURRENT_TEST", None)
|
||||
env.pop(irp.ENV_AUTH_HMAC_KEY, None)
|
||||
proc = subprocess.run(
|
||||
[sys.executable, "-c", script],
|
||||
cwd=os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
timeout=30,
|
||||
)
|
||||
self.assertEqual(proc.returncode, 0, proc.stderr)
|
||||
out = (proc.stdout or "").strip()
|
||||
self.assertTrue(out.startswith("FAIL_CLOSED"), msg=out)
|
||||
|
||||
def test_cross_process_verify_with_durable_key(self):
|
||||
"""Mint in process A, verify in process B with same durable key (#709 F4)."""
|
||||
import json as _json
|
||||
|
||||
mint_script = (
|
||||
"import json, os, irrecoverable_provenance as irp\n"
|
||||
"irp.reset_process_auth_secret_for_tests()\n"
|
||||
"auth = irp.build_authorization_artifact(\n"
|
||||
" remote='prgs', org='o', repo='r', pr_number=10,\n"
|
||||
f" expected_head_sha={HEAD_A!r}, incident_issue=1, incident_comment_id=2,\n"
|
||||
" destroyed_subject=None, issuer_username='sysadmin',\n"
|
||||
" issuer_profile='prgs-reconciler',\n"
|
||||
" native_provenance={'native_mcp_transport': True, 'pytest': True,\n"
|
||||
" 'token_fingerprint': 'fp', 'entrypoint': 'pytest', 'pid': 1})\n"
|
||||
"print(json.dumps(auth))\n"
|
||||
)
|
||||
env = dict(os.environ)
|
||||
env[irp.ENV_AUTH_HMAC_KEY] = DURABLE_TEST_HMAC_KEY
|
||||
env[irp.ENV_AUTH_HMAC_KEY_VERSION] = "test-v1"
|
||||
mint = subprocess.run(
|
||||
[sys.executable, "-c", mint_script],
|
||||
cwd=os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
timeout=30,
|
||||
)
|
||||
self.assertEqual(mint.returncode, 0, mint.stderr)
|
||||
auth = _json.loads(mint.stdout.strip())
|
||||
self.assertEqual(auth.get("key_version"), "test-v1")
|
||||
|
||||
verify_script = (
|
||||
"import json, sys, irrecoverable_provenance as irp\n"
|
||||
"irp.reset_process_auth_secret_for_tests()\n"
|
||||
"auth = json.loads(sys.stdin.read())\n"
|
||||
"v = irp.verify_authorization_artifact(\n"
|
||||
" auth, remote='prgs', org='o', repo='r', pr_number=10,\n"
|
||||
f" expected_head_sha={HEAD_A!r}, incident_issue=1, incident_comment_id=2)\n"
|
||||
"print(v.get('valid'), v.get('reasons'))\n"
|
||||
)
|
||||
verify = subprocess.run(
|
||||
[sys.executable, "-c", verify_script],
|
||||
cwd=os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
|
||||
input=_json.dumps(auth),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
timeout=30,
|
||||
)
|
||||
self.assertEqual(verify.returncode, 0, verify.stderr)
|
||||
self.assertTrue(
|
||||
(verify.stdout or "").strip().startswith("True"),
|
||||
msg=verify.stdout + verify.stderr,
|
||||
)
|
||||
|
||||
# Different durable key must fail verification (cross-process mismatch).
|
||||
env_bad = dict(env)
|
||||
env_bad[irp.ENV_AUTH_HMAC_KEY] = "1" * 64
|
||||
verify_bad = subprocess.run(
|
||||
[sys.executable, "-c", verify_script],
|
||||
cwd=os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
|
||||
input=_json.dumps(auth),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env_bad,
|
||||
timeout=30,
|
||||
)
|
||||
self.assertEqual(verify_bad.returncode, 0, verify_bad.stderr)
|
||||
self.assertTrue(
|
||||
(verify_bad.stdout or "").strip().startswith("False"),
|
||||
msg=verify_bad.stdout,
|
||||
)
|
||||
|
||||
|
||||
class TestF2MergerConsumer(unittest.TestCase):
|
||||
@@ -807,7 +1113,7 @@ class TestIrrecoverableToolF1(unittest.TestCase):
|
||||
"GITEA_MCP_SESSION_STATE_DIR": self._tmp.name,
|
||||
"GITEA_SESSION_PROFILE_LOCK": "prgs-reconciler",
|
||||
"GITEA_PROFILE_NAME": "prgs-reconciler",
|
||||
"GITEA_ALLOWED_OPERATIONS": ",".join(RECONCILER_OPS),
|
||||
"GITEA_ALLOWED_OPERATIONS": ",".join(DEDICATED_RECOVERY_OPS),
|
||||
},
|
||||
clear=False,
|
||||
)
|
||||
@@ -824,7 +1130,7 @@ class TestIrrecoverableToolF1(unittest.TestCase):
|
||||
return {
|
||||
"profile_name": "prgs-reconciler",
|
||||
"role": "reconciler",
|
||||
"allowed_operations": RECONCILER_OPS,
|
||||
"allowed_operations": DEDICATED_RECOVERY_OPS,
|
||||
"forbidden_operations": [
|
||||
"gitea.pr.approve",
|
||||
"gitea.pr.merge",
|
||||
@@ -924,17 +1230,18 @@ class TestIrrecoverableToolF1(unittest.TestCase):
|
||||
if "/pulls/" in str(url):
|
||||
return {"head": {"sha": HEAD_A}, "state": "open"}
|
||||
if "/issues/comments/" in str(url):
|
||||
return {
|
||||
"id": 11489,
|
||||
"body": "forensic diagnosis",
|
||||
"user": {"login": "sysadmin"},
|
||||
"created_at": "2026-07-13T00:00:00Z",
|
||||
}
|
||||
return _incident_comment_payload(
|
||||
comment_id=11489,
|
||||
author="controller-ops",
|
||||
pr_number=50,
|
||||
head=HEAD_A,
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
incident_issue=700,
|
||||
)
|
||||
raise AssertionError(f"unexpected API {method} {url}")
|
||||
|
||||
common = dict(
|
||||
get_profile=self._profile(),
|
||||
)
|
||||
with patch.object(self.mcp, "get_profile", return_value=self._profile()), patch.object(
|
||||
self.mcp, "_authenticated_username", return_value="sysadmin"
|
||||
), patch.object(
|
||||
|
||||
Reference in New Issue
Block a user