From ec5cf677718b6a6a5fc9a5102b5ce1783592509a Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 14 Jul 2026 00:54:33 -0400 Subject: [PATCH 1/3] fix(workflow): cross-profile decision-lock cleanup and irrecoverable provenance (#709) Prevent merger-local empty decision locks from standing in for reviewer terminal cleanup, refuse silent re-init overwrite of unresolved terminal evidence, record post-merge recovery-required state when audit fails, and add a truthful irrecoverable-provenance path that never claims applied=true. Closes #709 --- gitea_mcp_server.py | 591 +++++++++++++++++- mcp_session_state.py | 144 ++++- stale_review_decision_lock.py | 213 +++++++ task_capability_map.py | 9 + ...t_issue_709_decision_lock_cross_profile.py | 523 ++++++++++++++++ 5 files changed, 1453 insertions(+), 27 deletions(-) create mode 100644 tests/test_issue_709_decision_lock_cross_profile.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index c433a1d..f84f6c0 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -3357,17 +3357,28 @@ def _review_decision_session_reasons(lock: dict | None) -> list[str]: def init_review_decision_lock(remote: str | None, task: str | None, force: bool = True): - """Seed read-only-until-ready state for reviewer PR review tasks.""" + """Seed read-only-until-ready state for reviewer PR review tasks. + + #709 AC2: never overwrite unresolved terminal decision-lock evidence with + an empty initialized lock — even when *force* is True. Terminal ledgers + are cleared only via moot cleanup or sanctioned recovery/archive paths. + """ if task != "review_pr": return - if not force: - lock = _load_review_decision_lock() - if lock is not None: + existing = _load_review_decision_lock() + if existing is not None: + overwrite = stale_review_decision_lock.assess_init_overwrite( + existing, force=force + ) + if not overwrite.get("overwrite_allowed"): + # Preserve terminal evidence; keep existing durable lock. + return + if not force: env_lock = (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip() - stored_lock = (lock.get("session_profile_lock") or "").strip() - same_remote = lock.get("remote") == remote + stored_lock = (lock_get_session_profile_lock(existing)).strip() + same_remote = existing.get("remote") == remote same_profile = (not env_lock or not stored_lock or env_lock == stored_lock) - if same_remote and same_profile and not _review_decision_session_reasons(lock): + if same_remote and same_profile and not _review_decision_session_reasons(existing): return review_workflow_load.clear_review_workflow_load() profile = get_profile() @@ -3400,6 +3411,17 @@ def init_review_decision_lock(remote: str | None, task: str | None, force: bool }) +def lock_get_session_profile_lock(lock: dict | None) -> str: + if not isinstance(lock, dict): + return "" + return ( + lock.get("session_profile_lock") + or lock.get("profile_identity") + or lock.get("session_profile") + or "" + ) + + def _review_workflow_load_gate_reasons() -> list[str]: """Fail closed when canonical review workflow was not loaded (#389).""" return review_workflow_load.review_workflow_load_blockers(PROJECT_ROOT) @@ -4470,6 +4492,288 @@ def gitea_authorize_review_correction( return {"authorized": True, "correction_reason": reason, "reasons": []} + +def _record_post_merge_decision_recovery( + *, + pr_number: int, + head_sha: str | None, + merge_commit_sha: str | None, + target_profile_identity: str | None, + failed_step: str, + error: str | None, + remote: str | None, + org: str | None, + repo: str | None, +) -> dict: + """Persist durable post-merge recovery-required state (#709 AC3).""" + try: + actor = None + try: + if remote in REMOTES: + h, _, _ = _resolve(remote, None, org, repo) + actor = _authenticated_username(h) + except Exception: + actor = None + profile = get_profile() + profile_name = (profile.get("profile_name") or "").strip() or None + payload = stale_review_decision_lock.build_post_merge_recovery_record( + pr_number=pr_number, + head_sha=head_sha, + merge_commit_sha=merge_commit_sha, + target_profile_identity=target_profile_identity, + failed_step=failed_step, + error=error, + remote=remote, + org=org, + repo=repo, + actor_username=actor, + profile_name=profile_name, + ) + # Key by merger/active profile so the merging session owns the recovery row. + binding = _decision_lock_binding() + saved = mcp_session_state.save_state( + kind=mcp_session_state.KIND_POST_MERGE_DECISION_RECOVERY, + payload=payload, + remote=remote, + org=org, + repo=repo, + profile_identity=binding.get("profile_identity"), + ) + return dict(saved or payload) + except Exception as exc: # noqa: BLE001 + return {"status": "recovery_record_failed", "error": _redact(str(exc))} + + +def _clear_decision_lock_for_profile( + *, + profile_identity: str, + pr_number: int, + expected_head_sha: str | None, + remote: str | None, + org: str | None, + repo: str | None, +) -> dict: + """Clear one profile's durable decision lock when it targets *pr_number* approve.""" + lock = mcp_session_state.load_state_for_profile( + kind=mcp_session_state.KIND_DECISION_LOCK, + profile_identity=profile_identity, + remote=remote, + org=org, + repo=repo, + skip_identity_match=True, + ) + if lock is None: + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": "no durable lock for profile", + } + if not stale_review_decision_lock.lock_targets_merged_pr_approval( + lock, pr_number=pr_number, expected_head_sha=expected_head_sha + ): + # Also allow any terminal for this PR once merged (request_changes history). + last = stale_review_decision_lock.last_terminal_mutation(lock) + if not last or last.get("pr_number") != pr_number: + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": "lock terminal does not target this PR approval", + } + # Archive then clear. + try: + mcp_session_state.save_state( + kind=mcp_session_state.KIND_DECISION_LOCK_ARCHIVE, + payload={ + **dict(lock), + "archived_reason": "post_merge_cross_profile_cleanup", + "archived_for_pr": pr_number, + "recovery_critical": True, + "kind": mcp_session_state.KIND_DECISION_LOCK_ARCHIVE, + }, + remote=remote or lock.get("remote"), + org=org or lock.get("org") or lock.get("ready_org"), + repo=repo or lock.get("repo") or lock.get("ready_repo"), + profile_identity=f"{profile_identity}-archive-pr{pr_number}", + ) + except Exception: + pass + mcp_session_state.clear_state( + kind=mcp_session_state.KIND_DECISION_LOCK, + profile_identity=profile_identity, + remote=remote or lock.get("remote"), + org=org or lock.get("org") or lock.get("ready_org"), + repo=repo or lock.get("repo") or lock.get("ready_repo"), + ) + # If this is the in-memory active profile lock, clear memory too. + active = _decision_lock_binding().get("profile_identity") + if active and active == profile_identity: + global _REVIEW_DECISION_LOCK + _REVIEW_DECISION_LOCK = None + return { + "profile_identity": profile_identity, + "cleared": True, + "reason": f"cleared terminal lock for merged PR #{pr_number}", + "prior_summary": stale_review_decision_lock.lock_summary(lock), + } + + +def _reconcile_decision_locks_after_merge( + *, + pr_number: int, + head_sha: str | None, + merge_commit_sha: str | None, + remote: str, + host: str, + org: str, + repo: str, + auth, +) -> dict: + """Cross-profile decision-lock reconcile after a successful merge (#709).""" + report: dict = { + "cleared_any": False, + "profiles_scanned": [], + "cleared_profiles": [], + "audit_comment_ids": [], + "recovery_required": False, + "reason_lines": [], + "applied": False, # overall "historical applied cleanup" never claimed blindly + } + try: + actor = _authenticated_username(host) + except Exception: + actor = None + profile = get_profile() + profile_name = (profile.get("profile_name") or "").strip() or None + + identities = list(mcp_session_state.list_decision_lock_profile_identities()) + active = _decision_lock_binding().get("profile_identity") + if active and active not in identities: + identities.append(active) + # Always consider common role profiles so empty local merger lock cannot + # hide a reviewer terminal ledger (#709 AC1). + for candidate in ("prgs-reviewer", "prgs-merger", "prgs-author", "prgs-reconciler"): + if candidate not in identities: + identities.append(candidate) + report["profiles_scanned"] = list(identities) + + for identity in identities: + try: + outcome = _clear_decision_lock_for_profile( + profile_identity=identity, + pr_number=pr_number, + expected_head_sha=head_sha, + remote=remote, + org=org, + repo=repo, + ) + except Exception as exc: # noqa: BLE001 + report["recovery_required"] = True + report["reason_lines"].append( + f"failed clearing decision lock for {identity}: {_redact(str(exc))}" + ) + _record_post_merge_decision_recovery( + pr_number=pr_number, + head_sha=head_sha, + merge_commit_sha=merge_commit_sha, + target_profile_identity=identity, + failed_step="clear_profile_lock", + error=_redact(str(exc)), + remote=remote, + org=org, + repo=repo, + ) + continue + if outcome.get("cleared"): + report["cleared_any"] = True + report["cleared_profiles"].append(identity) + report["reason_lines"].append( + f"cleared decision lock profile={identity} after merge of " + f"PR #{pr_number} (#709)" + ) + # Audit publication (AC4): post and require comment id for full reconcile. + audit = stale_review_decision_lock.build_cleanup_audit_record( + assessment={ + "last_terminal_pr": pr_number, + "last_terminal_action": "approve", + "pr_state": "closed", + "pr_merged": True, + "pr_merged_or_closed": True, + "merge_commit_sha": merge_commit_sha, + "cleanup_allowed": True, + "is_moot": True, + "lock_summary": outcome.get("prior_summary"), + "reasons": [outcome.get("reason") or "cleared"], + }, + actor_username=actor, + profile_name=profile_name, + applied=True, + ) + audit["cleanup_target_profile"] = identity + audit["issue_ref"] = "#709" + comment_block = _profile_operation_gate("gitea.pr.comment") + if comment_block: + report["recovery_required"] = True + report["reason_lines"].append( + f"audit comment blocked for {identity}: {comment_block}" + ) + _record_post_merge_decision_recovery( + pr_number=pr_number, + head_sha=head_sha, + merge_commit_sha=merge_commit_sha, + target_profile_identity=identity, + failed_step="audit_comment_permission", + error=str(comment_block), + remote=remote, + org=org, + repo=repo, + ) + continue + try: + body = stale_review_decision_lock.format_cleanup_audit_comment(audit) + comment_url = f"{repo_api_url(host, org, repo)}/issues/{pr_number}/comments" + with _audited( + "comment_pr", + host=host, + remote=remote, + org=org, + repo=repo, + pr_number=pr_number, + request_metadata={"source": "post_merge_decision_lock_reconcile"}, + ): + posted = api_request("POST", comment_url, auth, {"body": body}) + cid = (posted or {}).get("id") + if not cid: + raise RuntimeError("audit comment missing id on readback") + report["audit_comment_ids"].append(cid) + report["reason_lines"].append( + f"audit comment published id={cid} for profile={identity}" + ) + except Exception as exc: # noqa: BLE001 + report["recovery_required"] = True + report["reason_lines"].append( + f"audit comment failed for {identity}: {_redact(str(exc))}" + ) + _record_post_merge_decision_recovery( + pr_number=pr_number, + head_sha=head_sha, + merge_commit_sha=merge_commit_sha, + target_profile_identity=identity, + failed_step="audit_comment_publish", + error=_redact(str(exc)), + remote=remote, + org=org, + repo=repo, + ) + + if report["cleared_any"] and not report["recovery_required"]: + report["applied"] = True # this-session successful cleanup only + elif not report["cleared_any"]: + report["reason_lines"].append( + f"no cross-profile decision lock targeted PR #{pr_number} for cleanup" + ) + return report + + @mcp.tool() def gitea_cleanup_stale_review_decision_lock( apply: bool = False, @@ -4669,12 +4973,227 @@ def gitea_cleanup_stale_review_decision_lock( "POST", comment_url, auth, {"body": body} ) report["audit_comment_id"] = (posted or {}).get("id") + if not report["audit_comment_id"]: + report["reconciled"] = False + report["recovery_required"] = True + report["reasons"] = list(report.get("reasons") or []) + [ + "cleanup applied but audit comment id missing on readback " + "(#709 AC4 fail-closed for full reconcile)" + ] + _record_post_merge_decision_recovery( + pr_number=int(assessment["last_terminal_pr"]), + head_sha=assessment.get("locked_head_sha"), + merge_commit_sha=assessment.get("merge_commit_sha"), + target_profile_identity=active_identity, + failed_step="audit_comment_missing_id", + error="comment response missing id", + remote=remote, + org=o, + repo=r, + ) + else: + report["reconciled"] = True except Exception as exc: # noqa: BLE001 report["audit_comment_error"] = _redact(str(exc)) + report["reconciled"] = False + report["recovery_required"] = True + report["reasons"] = list(report.get("reasons") or []) + [ + "cleanup applied but audit comment failed; recovery-required " + "recorded (#709 AC4)" + ] + try: + _record_post_merge_decision_recovery( + pr_number=int(assessment["last_terminal_pr"]), + head_sha=assessment.get("locked_head_sha"), + merge_commit_sha=assessment.get("merge_commit_sha"), + target_profile_identity=active_identity, + failed_step="audit_comment_publish", + error=_redact(str(exc)), + remote=remote, + org=o, + repo=r, + ) + except Exception: + pass + if post_audit_comment is False: + report["reconciled"] = False + report["reasons"] = list(report.get("reasons") or []) + [ + "cleanup applied without audit comment (post_audit_comment=false); " + "not fully reconciled (#709 AC4)" + ] return report +@mcp.tool() +def gitea_record_irrecoverable_decision_lock_provenance( + pr_number: int, + reason: str, + confirmation: str = "", + operator_authorized: bool = False, + expected_head_sha: str | None = None, + incident_ref: str | None = None, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, + post_audit_comment: bool = True, +) -> dict: + """Record truthful absence of decision-lock cleanup proof (#709 AC5). + + Never emits applied=true or claims historical cleanup was proven. + Requires operator_authorized=True and confirmation exactly equal to + ``IRRECOVERABLE DECISION PROVENANCE PR ``. + """ + h, o, r = _resolve(remote, host, org, repo) + expected_confirm = f"IRRECOVERABLE DECISION PROVENANCE PR {int(pr_number)}" + report = { + "success": False, + "performed": False, + "applied": False, + "historical_cleanup_proven": False, + "status": "provenance_irrecoverable", + "pr_number": pr_number, + "expected_head_sha": expected_head_sha, + "reasons": [], + "record": None, + "audit_comment_id": None, + } + read_block = _profile_operation_gate("gitea.read") + if read_block: + report["reasons"] = read_block + report["permission_report"] = _permission_block_report("gitea.read") + return report + if not operator_authorized: + report["reasons"].append( + "operator_authorized must be true for irrecoverable provenance " + "recording (fail closed, #709 AC5)" + ) + return report + if (confirmation or "").strip() != expected_confirm: + report["reasons"].append( + f"confirmation must equal exactly {expected_confirm!r} (fail closed)" + ) + return report + if not (reason or "").strip(): + report["reasons"].append("reason is required (fail closed)") + return report + try: + actor = _authenticated_username(h) + except Exception: + actor = None + if not actor: + report["reasons"].append( + "authenticated identity could not be verified (fail closed)" + ) + return report + profile = get_profile() + profile_name = (profile.get("profile_name") or "").strip() or None + # Idempotent: if matching record already exists for pr+head, return it. + binding = _decision_lock_binding() + existing = mcp_session_state.load_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, + remote=remote, + org=o, + repo=r, + profile_identity=binding.get("profile_identity"), + ) + if ( + isinstance(existing, dict) + and existing.get("pr_number") == pr_number + and ( + not expected_head_sha + or stale_review_decision_lock.heads_equal( + existing.get("head_sha"), expected_head_sha + ) + ) + and existing.get("status") == "provenance_irrecoverable" + ): + report["success"] = True + report["performed"] = False + report["record"] = existing + report["reasons"].append("idempotent: matching irrecoverable record already present") + return report + + record = stale_review_decision_lock.build_irrecoverable_provenance_record( + pr_number=pr_number, + head_sha=expected_head_sha, + remote=remote, + org=o, + repo=r, + actor_username=actor, + profile_name=profile_name, + reason=reason.strip(), + incident_ref=incident_ref, + operator_authorized=True, + ) + # Stamp kind for TTL exemption + record["kind"] = mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE + saved = mcp_session_state.save_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, + payload=record, + remote=remote, + org=o, + repo=r, + profile_identity=binding.get("profile_identity"), + ) + report["record"] = dict(saved or record) + report["performed"] = True + report["success"] = True + report["reasons"].append( + "recorded provenance_irrecoverable (applied=false; historical cleanup not proven)" + ) + + if post_audit_comment: + comment_block = _profile_operation_gate("gitea.pr.comment") or _profile_operation_gate( + "gitea.issue.comment" + ) + # Prefer issue comment capability for discussion thread. + issue_block = _profile_operation_gate("gitea.issue.comment") + if issue_block: + report["reasons"].append(f"audit comment skipped: {issue_block}") + else: + try: + body = stale_review_decision_lock.format_irrecoverable_audit_comment( + report["record"] + ) + comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments" + with _audited( + "comment_issue", + host=h, + remote=remote, + org=o, + repo=r, + issue_number=pr_number, + request_metadata={ + "source": "record_irrecoverable_decision_lock_provenance" + }, + ): + posted = api_request( + "POST", + comment_url, + _auth(h), + {"body": body}, + ) + report["audit_comment_id"] = (posted or {}).get("id") + if report["audit_comment_id"]: + # Re-save with comment id for readback completeness. + report["record"]["audit_comment_id"] = report["audit_comment_id"] + mcp_session_state.save_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, + payload=report["record"], + remote=remote, + org=o, + repo=r, + profile_identity=binding.get("profile_identity"), + ) + except Exception as exc: # noqa: BLE001 + report["reasons"].append( + f"audit comment failed: {_redact(str(exc))} (record still durable)" + ) + return report + + @mcp.tool() def gitea_dry_run_pr_review( pr_number: int, @@ -5772,29 +6291,49 @@ def gitea_merge_pr( reviewer_pr_lease.get_session_lease() ) ) - # Same-profile auto-expire (#594): if *this* profile's decision lock ends - # with an approve of the PR just merged, clear it so durable state cannot - # block later unrelated reviews. Cross-profile locks (e.g. reviewer vs - # merger) still require gitea_cleanup_stale_review_decision_lock. + # #709 AC1/AC3/AC4: reconcile decision locks after irreversible merge. + # Clears the same-profile lock when it holds the approve, and also scans + # other durable profile locks (e.g. prgs-reviewer) so merger-local empty + # init cannot leave the reviewer terminal ledger behind. Failures write a + # recovery-required record (applied=false) — merge is never undone. try: - lock_after = _load_review_decision_lock() - last_term = stale_review_decision_lock.last_terminal_mutation(lock_after) - if ( - last_term - and last_term.get("action") == "approve" - and last_term.get("pr_number") == pr_number - ): - _save_review_decision_lock(None) - result["review_decision_lock_cleared_after_merge"] = True - reasons.append( - f"cleared same-profile review decision lock after merge of " - f"approved PR #{pr_number} (#594)" - ) - else: - result["review_decision_lock_cleared_after_merge"] = False + reconcile = _reconcile_decision_locks_after_merge( + pr_number=pr_number, + head_sha=expected_head_sha, + merge_commit_sha=( + (merged or {}).get("merge_commit_sha") + if isinstance(merged, dict) + else None + ), + remote=remote, + host=h, + org=o, + repo=r, + auth=auth, + ) + result["review_decision_lock_reconcile"] = reconcile + result["review_decision_lock_cleared_after_merge"] = bool( + reconcile.get("cleared_any") + ) + for line in reconcile.get("reason_lines") or []: + reasons.append(line) except Exception as clear_exc: # noqa: BLE001 — never fail the merge result["review_decision_lock_cleared_after_merge"] = False result["review_decision_lock_clear_error"] = _redact(str(clear_exc)) + try: + _record_post_merge_decision_recovery( + pr_number=pr_number, + head_sha=expected_head_sha, + merge_commit_sha=None, + target_profile_identity=None, + failed_step="reconcile_exception", + error=_redact(str(clear_exc)), + remote=remote, + org=o, + repo=r, + ) + except Exception: + pass reasons.append(f"all gates passed; merged PR #{pr_number} via '{do}'") return result diff --git a/mcp_session_state.py b/mcp_session_state.py index b94b90a..4ae119a 100644 --- a/mcp_session_state.py +++ b/mcp_session_state.py @@ -36,7 +36,21 @@ KIND_REVIEW_DRAFT = "review_draft" # other session proofs; a contaminated session fails closed on gated mutations # until a reconciler audits and clears it. KIND_STABLE_BRANCH_CONTAMINATION = "stable_branch_contamination" +# #709: archive prior terminal decision ledgers instead of silent overwrite. +KIND_DECISION_LOCK_ARCHIVE = "review_decision_lock_archive" +# #709: post-merge cleanup/audit reconciliation-required durable record. +KIND_POST_MERGE_DECISION_RECOVERY = "post_merge_decision_recovery" +# #709: truthful record when historical terminal evidence is irrecoverably gone. +KIND_IRRECOVERABLE_DECISION_PROVENANCE = "irrecoverable_decision_provenance" +# Kinds that must survive the default session-state TTL (forensic / recovery). +RECOVERY_CRITICAL_KINDS = frozenset( + { + KIND_DECISION_LOCK_ARCHIVE, + KIND_POST_MERGE_DECISION_RECOVERY, + KIND_IRRECOVERABLE_DECISION_PROVENANCE, + } +) _SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+") @@ -270,7 +284,11 @@ def identity_match_reasons( reasons.append("session state missing recorded_at timestamp (fail closed)") else: age = _now_utc() - recorded_at - if age > timedelta(hours=ttl_hours()): + kind = (record.get("kind") or "").strip() + ttl_exempt = kind in RECOVERY_CRITICAL_KINDS or bool( + record.get("recovery_critical") + ) + if age > timedelta(hours=ttl_hours()) and not ttl_exempt: reasons.append( f"session state expired after {ttl_hours():g}h (fail closed)" ) @@ -447,3 +465,127 @@ def clear_state( profile_identity=profile_identity, state_dir=state_dir, ) + +def list_decision_lock_profile_identities( + state_dir: str | None = None, +) -> list[str]: + """Return profile identities that have a durable review_decision_lock file (#709). + + Filename form: ``review_decision_lock-.json`` (see ``state_key``). + Does not validate TTL or identity — callers must load via ``load_state``. + """ + root = (state_dir or default_state_dir()).strip() + if not root or not os.path.isdir(root): + return [] + prefix = f"{_sanitize_segment(KIND_DECISION_LOCK)}-" + suffix = ".json" + found: list[str] = [] + try: + names = os.listdir(root) + except OSError: + return [] + for name in names: + if not name.startswith(prefix) or not name.endswith(suffix): + continue + if name.endswith(".lock"): + continue + mid = name[len(prefix) : -len(suffix)] + if mid: + found.append(mid) + return sorted(set(found)) + + +def load_state_for_profile( + *, + kind: str, + profile_identity: str, + remote: str | None = None, + org: str | None = None, + repo: str | None = None, + state_dir: str | None = None, + skip_identity_match: bool = False, +) -> dict[str, Any] | None: + """Load durable state for an explicit profile identity (#709 cross-profile). + + When *skip_identity_match* is True, still requires the file's recorded + profile_identity to equal the requested profile (anti-stomp), but does not + require the *active* session identity to match — needed so a merger can + inspect a reviewer lock after merge. + """ + profile = current_profile_identity(profile_identity=profile_identity) + root = _ensure_state_dir(state_dir) + path = state_file_path( + kind=kind, + remote=remote, + org=org, + repo=repo, + profile_identity=profile, + state_dir=root, + ) + lock_path = f"{path}.lock" + with _exclusive_file_lock(lock_path): + envelope = _read_json(path) + if not envelope: + return None + payload = envelope.get("payload") + if not isinstance(payload, dict): + return None + merged = dict(payload) + for key in ( + "kind", + "remote", + "org", + "repo", + "profile_identity", + "session_profile_lock", + "recorded_at", + "updated_at", + "writer_pid", + ): + if key in envelope and key not in merged: + merged[key] = envelope[key] + stored = (merged.get("profile_identity") or "").strip() + if stored and stored != profile: + return None + if skip_identity_match: + # Still enforce TTL / future-dated so dead records do not authorize cleanup. + reasons = identity_match_reasons( + merged, + remote=remote or merged.get("remote"), + org=org or merged.get("org"), + repo=repo or merged.get("repo"), + profile_identity=stored or profile, + ) + # Drop active-session-only mismatches; keep expiry / spoof reasons. + filtered = [ + r + for r in reasons + if "profile identity mismatch" not in r + or (stored and stored != profile) + ] + # Re-run only expiry/future/missing checks via identity when profile matches + expiry_reasons = [ + r + for r in identity_match_reasons( + merged, + remote=None, + org=None, + repo=None, + profile_identity=stored or profile, + ) + if any(x in r for x in ("expired", "future", "missing recorded_at")) + ] + if expiry_reasons: + return None + return merged + reasons = identity_match_reasons( + merged, + remote=remote, + org=org, + repo=repo, + profile_identity=profile, + ) + if reasons: + return None + return merged + diff --git a/stale_review_decision_lock.py b/stale_review_decision_lock.py index 5f47486..c5252c3 100644 --- a/stale_review_decision_lock.py +++ b/stale_review_decision_lock.py @@ -438,3 +438,216 @@ def format_cleanup_audit_comment(audit: dict[str, Any]) -> str: "This path only clears a lock when the referenced PR is merged/closed.", ] return "\n".join(lines) + + +# --------------------------------------------------------------------------- +# #709 cross-profile cleanup, overwrite protection, recovery provenance +# --------------------------------------------------------------------------- + + +def has_unresolved_terminal_evidence(lock: dict | None) -> bool: + """True when *lock* still carries a terminal live mutation ledger.""" + return last_terminal_mutation(lock) is not None + + +def assess_init_overwrite( + existing_lock: dict | None, + *, + force: bool = False, +) -> dict[str, Any]: + """Whether init_review_decision_lock may replace an existing durable lock (#709 AC2). + + Unresolved terminal evidence must never be silently replaced by an empty + initialized lock. *force* does not authorize destruction of terminal + ledgers — only explicit cleanup / archive transitions may remove them. + """ + result: dict[str, Any] = { + "overwrite_allowed": True, + "has_existing": existing_lock is not None, + "has_unresolved_terminal": False, + "reasons": [], + "last_terminal_pr": None, + "last_terminal_action": None, + "existing_profile_identity": None, + } + if existing_lock is None: + result["reasons"].append("no existing lock; empty init allowed") + return result + result["existing_profile_identity"] = ( + existing_lock.get("profile_identity") + or existing_lock.get("session_profile_lock") + or existing_lock.get("session_profile") + ) + last = last_terminal_mutation(existing_lock) + if last is None: + result["reasons"].append( + "existing lock has no terminal mutations; re-init allowed" + ) + return result + result["has_unresolved_terminal"] = True + result["overwrite_allowed"] = False + result["last_terminal_pr"] = last.get("pr_number") + result["last_terminal_action"] = last.get("action") + result["reasons"].append( + "refuse empty re-init: unresolved terminal decision-lock evidence " + f"present ({last.get('action')} on PR #{last.get('pr_number')}); " + "use gitea_cleanup_stale_review_decision_lock when moot, or archive " + f"via sanctioned recovery — force={force!r} does not authorize overwrite " + "(#709 AC2)" + ) + return result + + +def lock_targets_merged_pr_approval( + lock: dict | None, + *, + pr_number: int, + expected_head_sha: str | None = None, +) -> bool: + """True when *lock*'s last terminal mutation is approve of *pr_number*.""" + last = last_terminal_mutation(lock) + if last is None: + return False + if last.get("action") != "approve": + return False + if last.get("pr_number") != pr_number: + return False + if expected_head_sha: + locked = mutation_head_sha(last, lock) + if locked and not heads_equal(locked, expected_head_sha): + return False + return True + + +def build_post_merge_recovery_record( + *, + pr_number: int, + head_sha: str | None, + merge_commit_sha: str | None, + target_profile_identity: str | None, + failed_step: str, + error: str | None, + remote: str | None, + org: str | None, + repo: str | None, + actor_username: str | None, + profile_name: str | None, +) -> dict[str, Any]: + """Durable recovery-required payload after irreversible merge (#709 AC3).""" + return { + "event": "post_merge_decision_lock_recovery_required", + "status": "recovery_required", + "issue_ref": "#709", + "recovery_critical": True, + "applied": False, # never claim historical cleanup succeeded + "timestamp": datetime.now(timezone.utc).isoformat(), + "pr_number": pr_number, + "head_sha": normalize_head_sha(head_sha), + "merge_commit_sha": merge_commit_sha, + "target_profile_identity": target_profile_identity, + "failed_step": failed_step, + "error": error, + "remote": remote, + "org": org, + "repo": repo, + "actor_username": actor_username, + "profile_name": profile_name, + "required_recovery_action": ( + "retry cross-profile decision-lock cleanup and audit publication " + "for the merged PR; if terminal evidence is gone, use " + "gitea_record_irrecoverable_decision_lock_provenance" + ), + } + + +def build_irrecoverable_provenance_record( + *, + pr_number: int, + head_sha: str | None, + remote: str | None, + org: str | None, + repo: str | None, + actor_username: str | None, + profile_name: str | None, + reason: str, + incident_ref: str | None, + operator_authorized: bool, +) -> dict[str, Any]: + """Truthful absence-of-proof record (#709 AC5). Never sets applied=True.""" + return { + "event": "irrecoverable_decision_lock_provenance", + "status": "provenance_irrecoverable", + "operator_recovery_required": True, + "issue_ref": "#709", + "recovery_critical": True, + "applied": False, + "historical_cleanup_proven": False, + "timestamp": datetime.now(timezone.utc).isoformat(), + "pr_number": pr_number, + "head_sha": normalize_head_sha(head_sha), + "remote": remote, + "org": org, + "repo": repo, + "actor_username": actor_username, + "profile_name": profile_name, + "reason": reason, + "incident_ref": incident_ref, + "operator_authorized": bool(operator_authorized), + "merger_may_accept": bool(operator_authorized), + "acceptance_rule": ( + "Merger may accept this record only when operator_authorized=true, " + "repository/PR/head match the live target, the record is durable " + "and read back, and no conflicting terminal lock remains for a " + "different PR/head. This does not prove historical cleanup." + ), + } + + +def format_irrecoverable_audit_comment(record: dict[str, Any]) -> str: + """Markdown body for irrecoverable provenance audit (no applied=true claim).""" + lines = [ + "## Irrecoverable decision-lock provenance (#709)", + "", + "Status: **PROVENANCE_IRRECOVERABLE** (not applied cleanup)", + "", + f"- actor: `{record.get('actor_username')}`", + f"- profile: `{record.get('profile_name')}`", + f"- timestamp: `{record.get('timestamp')}`", + f"- PR: `#{record.get('pr_number')}`", + f"- head_sha: `{record.get('head_sha')}`", + f"- incident_ref: `{record.get('incident_ref')}`", + f"- operator_authorized: `{record.get('operator_authorized')}`", + f"- historical_cleanup_proven: `{record.get('historical_cleanup_proven')}`", + f"- applied: `{record.get('applied')}` (must remain false)", + "", + f"Reason: {record.get('reason')}", + "", + "This record documents **absence of proof**, not successful cleanup.", + "It must not be reused for a different PR or head (#709 AC6).", + ] + return "\n".join(lines) + + +def format_post_merge_recovery_comment(record: dict[str, Any]) -> str: + """Markdown body for post-merge recovery-required audit.""" + lines = [ + "## Post-merge decision-lock recovery required (#709)", + "", + "Status: **RECOVERY_REQUIRED** (merge is irreversible; cleanup/audit incomplete)", + "", + f"- actor: `{record.get('actor_username')}`", + f"- profile: `{record.get('profile_name')}`", + f"- timestamp: `{record.get('timestamp')}`", + f"- PR: `#{record.get('pr_number')}`", + f"- head_sha: `{record.get('head_sha')}`", + f"- merge_commit_sha: `{record.get('merge_commit_sha')}`", + f"- target_profile_identity: `{record.get('target_profile_identity')}`", + f"- failed_step: `{record.get('failed_step')}`", + f"- error: `{record.get('error')}`", + "", + f"Required action: {record.get('required_recovery_action')}", + "", + "applied=false — do not treat this as successful cleanup evidence.", + ] + return "\n".join(lines) + diff --git a/task_capability_map.py b/task_capability_map.py index fdf5583..f41475d 100644 --- a/task_capability_map.py +++ b/task_capability_map.py @@ -127,6 +127,15 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "permission": "gitea.pr.review", "role": "reviewer", }, + # #709: truthful absence-of-proof recovery record (not applied cleanup). + "record_irrecoverable_decision_lock_provenance": { + "permission": "gitea.issue.comment", + "role": "reconciler", + }, + "gitea_record_irrecoverable_decision_lock_provenance": { + "permission": "gitea.issue.comment", + "role": "reconciler", + }, "delete_branch": { "permission": "gitea.branch.delete", "role": "author", diff --git a/tests/test_issue_709_decision_lock_cross_profile.py b/tests/test_issue_709_decision_lock_cross_profile.py new file mode 100644 index 0000000..6e9d21a --- /dev/null +++ b/tests/test_issue_709_decision_lock_cross_profile.py @@ -0,0 +1,523 @@ +"""#709: cross-profile decision-lock cleanup, overwrite protection, recovery. + +Covers AC1–AC8 regression scenarios without fabricating historical PR #696 +provenance or special-casing live PR numbers in production code. +""" + +from __future__ import annotations + +import os +import tempfile +import unittest +from unittest.mock import MagicMock, patch + +import mcp_session_state as ss +import stale_review_decision_lock as srdl + + +def _lock( + mutations=None, + *, + profile="prgs-reviewer", + remote="prgs", + head=None, +): + muts = [] + for m in mutations or []: + row = dict(m) + if head and "head_sha" not in row: + row["head_sha"] = head + muts.append(row) + return { + "task": "review_pr", + "remote": remote, + "org": "Scaled-Tech-Consulting", + "repo": "Gitea-Tools", + "session_pid": os.getpid(), + "session_profile": profile, + "session_profile_lock": profile, + "profile_identity": profile, + "final_review_decision_ready": False, + "ready_pr_number": None, + "ready_action": None, + "ready_expected_head_sha": None, + "live_mutations": muts, + "correction_authorized": False, + "correction_reason": None, + "kind": ss.KIND_DECISION_LOCK, + } + + +APPROVE = {"pr_number": 100, "action": "approve", "review_id": 9} +APPROVE_OTHER = {"pr_number": 200, "action": "approve", "review_id": 10} +HEAD_A = "a" * 40 +HEAD_B = "b" * 40 + + +class TestAC2InitOverwrite(unittest.TestCase): + def test_empty_lock_allows_reinit(self): + a = srdl.assess_init_overwrite(_lock([]), force=True) + self.assertTrue(a["overwrite_allowed"]) + + def test_terminal_lock_blocks_force_reinit(self): + a = srdl.assess_init_overwrite(_lock([APPROVE]), force=True) + self.assertFalse(a["overwrite_allowed"]) + self.assertTrue(a["has_unresolved_terminal"]) + self.assertEqual(a["last_terminal_pr"], 100) + + def test_none_lock_allows_init(self): + a = srdl.assess_init_overwrite(None) + self.assertTrue(a["overwrite_allowed"]) + + +class TestAC1TargetApproval(unittest.TestCase): + def test_targets_matching_approve(self): + self.assertTrue( + srdl.lock_targets_merged_pr_approval( + _lock([APPROVE], head=HEAD_A), + pr_number=100, + expected_head_sha=HEAD_A, + ) + ) + + def test_rejects_other_pr(self): + self.assertFalse( + srdl.lock_targets_merged_pr_approval( + _lock([APPROVE_OTHER]), pr_number=100 + ) + ) + + def test_rejects_head_mismatch(self): + self.assertFalse( + srdl.lock_targets_merged_pr_approval( + _lock([APPROVE], head=HEAD_A), + pr_number=100, + expected_head_sha=HEAD_B, + ) + ) + + +class TestAC5IrrecoverableRecord(unittest.TestCase): + def test_never_sets_applied_true(self): + rec = srdl.build_irrecoverable_provenance_record( + pr_number=42, + head_sha=HEAD_A, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + actor_username="sysadmin", + profile_name="prgs-reconciler", + reason="evidence destroyed", + incident_ref="#700 comment 1", + operator_authorized=True, + ) + self.assertFalse(rec["applied"]) + self.assertFalse(rec["historical_cleanup_proven"]) + self.assertEqual(rec["status"], "provenance_irrecoverable") + self.assertTrue(rec["merger_may_accept"]) + body = srdl.format_irrecoverable_audit_comment(rec) + self.assertIn("applied: `False`", body) + self.assertIn("must remain false", body) + + def test_unauthorized_not_merger_acceptable(self): + rec = srdl.build_irrecoverable_provenance_record( + pr_number=42, + head_sha=HEAD_A, + remote="prgs", + org=None, + repo=None, + actor_username="x", + profile_name="y", + reason="r", + incident_ref=None, + operator_authorized=False, + ) + self.assertFalse(rec["merger_may_accept"]) + + +class TestAC3PostMergeRecoveryRecord(unittest.TestCase): + def test_recovery_record_is_not_applied_cleanup(self): + rec = srdl.build_post_merge_recovery_record( + pr_number=10, + head_sha=HEAD_A, + merge_commit_sha="m" * 40, + target_profile_identity="prgs-reviewer", + failed_step="audit_comment_publish", + error="timeout", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + actor_username="sysadmin", + profile_name="prgs-merger", + ) + self.assertEqual(rec["status"], "recovery_required") + self.assertFalse(rec["applied"]) + self.assertTrue(rec["recovery_critical"]) + + +class TestSessionStateCrossProfile(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.state_dir = self._tmp.name + os.chmod(self.state_dir, 0o700) + + def tearDown(self): + self._tmp.cleanup() + + def test_list_and_load_foreign_profile_lock(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([APPROVE], profile="prgs-reviewer"), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + # Merger-local empty lock + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([], profile="prgs-merger"), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity="prgs-merger", + state_dir=self.state_dir, + ) + ids = ss.list_decision_lock_profile_identities(state_dir=self.state_dir) + self.assertIn("prgs-reviewer", ids) + self.assertIn("prgs-merger", ids) + + foreign = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNotNone(foreign) + self.assertTrue( + srdl.lock_targets_merged_pr_approval(foreign, pr_number=100) + ) + empty = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-merger", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNotNone(empty) + self.assertFalse( + srdl.lock_targets_merged_pr_approval(empty, pr_number=100) + ) + + def test_clear_reviewer_not_merger_empty(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([APPROVE], profile="prgs-reviewer"), + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([], profile="prgs-merger"), + profile_identity="prgs-merger", + state_dir=self.state_dir, + ) + ss.clear_state( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + self.assertIsNone( + ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + skip_identity_match=True, + ) + ) + # Merger empty lock remains + self.assertIsNotNone( + ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-merger", + state_dir=self.state_dir, + skip_identity_match=True, + ) + ) + + def test_recovery_critical_kinds_ttl_exempt(self): + rec = srdl.build_irrecoverable_provenance_record( + pr_number=1, + head_sha=HEAD_A, + remote="prgs", + org=None, + repo=None, + actor_username="a", + profile_name="p", + reason="gone", + incident_ref=None, + operator_authorized=True, + ) + rec["kind"] = ss.KIND_IRRECOVERABLE_DECISION_PROVENANCE + # Force old recorded_at + rec["recorded_at"] = "2000-01-01T00:00:00Z" + rec["updated_at"] = rec["recorded_at"] + rec["profile_identity"] = "prgs-reconciler" + rec["session_profile_lock"] = "prgs-reconciler" + reasons = ss.identity_match_reasons( + rec, profile_identity="prgs-reconciler" + ) + self.assertFalse( + any("expired" in r for r in reasons), + msg=reasons, + ) + + +class TestInitReviewDecisionLockIntegration(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.env = patch.dict( + os.environ, + { + "GITEA_MCP_SESSION_STATE_DIR": self._tmp.name, + "GITEA_SESSION_PROFILE_LOCK": "prgs-reviewer", + "GITEA_PROFILE_NAME": "prgs-reviewer", + }, + clear=False, + ) + self.env.start() + import mcp_server + + self.mcp = mcp_server + self.mcp._REVIEW_DECISION_LOCK = None + + def tearDown(self): + self.mcp._REVIEW_DECISION_LOCK = None + self.env.stop() + self._tmp.cleanup() + + def test_init_does_not_wipe_terminal_ledger(self): + self.mcp._save_review_decision_lock(_lock([APPROVE], profile="prgs-reviewer")) + # force=True would previously wipe + self.mcp.init_review_decision_lock("prgs", "review_pr", force=True) + loaded = self.mcp._load_review_decision_lock() + self.assertIsNotNone(loaded) + last = srdl.last_terminal_mutation(loaded) + self.assertIsNotNone(last) + self.assertEqual(last.get("pr_number"), 100) + + def test_init_creates_empty_when_no_terminal(self): + self.mcp._save_review_decision_lock(None) + self.mcp.init_review_decision_lock("prgs", "review_pr", force=True) + loaded = self.mcp._load_review_decision_lock() + self.assertIsNotNone(loaded) + self.assertEqual(loaded.get("live_mutations"), []) + + +class TestIrrecoverableTool(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.env = patch.dict( + os.environ, + { + "GITEA_MCP_SESSION_STATE_DIR": self._tmp.name, + "GITEA_SESSION_PROFILE_LOCK": "prgs-reconciler", + "GITEA_PROFILE_NAME": "prgs-reconciler", + "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment,gitea.pr.comment", + }, + clear=False, + ) + self.env.start() + import mcp_server + + self.mcp = mcp_server + + def tearDown(self): + self.env.stop() + self._tmp.cleanup() + + def test_requires_confirmation_and_operator(self): + with patch.object( + self.mcp, + "get_profile", + return_value={ + "profile_name": "prgs-reconciler", + "allowed_operations": [ + "gitea.read", + "gitea.issue.comment", + "gitea.pr.comment", + ], + "forbidden_operations": [], + }, + ): + r = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( + pr_number=50, + reason="lost", + confirmation="", + operator_authorized=False, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + post_audit_comment=False, + ) + self.assertFalse(r["success"]) + self.assertFalse(r["applied"]) + + def test_records_without_applied_true(self): + with patch.object( + self.mcp, + "get_profile", + return_value={ + "profile_name": "prgs-reconciler", + "allowed_operations": [ + "gitea.read", + "gitea.issue.comment", + "gitea.pr.comment", + ], + "forbidden_operations": [], + }, + ), patch.object( + self.mcp, "_authenticated_username", return_value="sysadmin" + ), patch.object( + self.mcp, "_profile_operation_gate", return_value=None + ), patch.object( + self.mcp, "_resolve", return_value=("h", "Scaled-Tech-Consulting", "Gitea-Tools") + ): + r = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( + pr_number=50, + reason="terminal evidence overwritten", + confirmation="IRRECOVERABLE DECISION PROVENANCE PR 50", + operator_authorized=True, + expected_head_sha=HEAD_A, + incident_ref="issue-700-comment-11489", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + post_audit_comment=False, + ) + self.assertTrue(r["success"]) + self.assertFalse(r["applied"]) + self.assertFalse(r["historical_cleanup_proven"]) + self.assertEqual(r["record"]["status"], "provenance_irrecoverable") + + # Idempotent replay + with patch.object( + self.mcp, + "get_profile", + return_value={ + "profile_name": "prgs-reconciler", + "allowed_operations": [ + "gitea.read", + "gitea.issue.comment", + "gitea.pr.comment", + ], + "forbidden_operations": [], + }, + ), patch.object( + self.mcp, "_authenticated_username", return_value="sysadmin" + ), patch.object( + self.mcp, "_profile_operation_gate", return_value=None + ), patch.object( + self.mcp, "_resolve", return_value=("h", "Scaled-Tech-Consulting", "Gitea-Tools") + ): + r2 = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( + pr_number=50, + reason="terminal evidence overwritten", + confirmation="IRRECOVERABLE DECISION PROVENANCE PR 50", + operator_authorized=True, + expected_head_sha=HEAD_A, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + post_audit_comment=False, + ) + self.assertTrue(r2["success"]) + self.assertFalse(r2["performed"]) # idempotent hit + + def test_wrong_confirmation_cannot_unblock_other_pr(self): + with patch.object( + self.mcp, + "get_profile", + return_value={ + "profile_name": "prgs-reconciler", + "allowed_operations": ["gitea.read", "gitea.issue.comment"], + "forbidden_operations": [], + }, + ), patch.object( + self.mcp, "_authenticated_username", return_value="sysadmin" + ), patch.object( + self.mcp, "_profile_operation_gate", return_value=None + ), patch.object( + self.mcp, "_resolve", return_value=("h", "o", "r") + ): + r = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( + pr_number=50, + reason="x", + confirmation="IRRECOVERABLE DECISION PROVENANCE PR 51", + operator_authorized=True, + remote="prgs", + post_audit_comment=False, + ) + self.assertFalse(r["success"]) + + +class TestClearProfileHelper(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.env = patch.dict( + os.environ, + { + "GITEA_MCP_SESSION_STATE_DIR": self._tmp.name, + "GITEA_SESSION_PROFILE_LOCK": "prgs-merger", + }, + clear=False, + ) + self.env.start() + import mcp_server + + self.mcp = mcp_server + self.mcp._REVIEW_DECISION_LOCK = None + + def tearDown(self): + self.mcp._REVIEW_DECISION_LOCK = None + self.env.stop() + self._tmp.cleanup() + + def test_clear_only_matching_reviewer_approve(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([APPROVE], profile="prgs-reviewer", head=HEAD_A), + profile_identity="prgs-reviewer", + state_dir=self._tmp.name, + ) + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([], profile="prgs-merger"), + profile_identity="prgs-merger", + state_dir=self._tmp.name, + ) + out = self.mcp._clear_decision_lock_for_profile( + profile_identity="prgs-reviewer", + pr_number=100, + expected_head_sha=HEAD_A, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + ) + self.assertTrue(out["cleared"]) + skip = self.mcp._clear_decision_lock_for_profile( + profile_identity="prgs-merger", + pr_number=100, + expected_head_sha=HEAD_A, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + ) + self.assertFalse(skip["cleared"]) + + +if __name__ == "__main__": + unittest.main() -- 2.43.7 From 9cb12ee0f442a89535b6f81712367f1112a3e59e Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 14 Jul 2026 01:36:39 -0400 Subject: [PATCH 2/3] fix(workflow): non-forgeable irrecoverable auth, merger consumer, exact-scope cleanup (#709) Address formal review 434 REQUEST_CHANGES on PR #710: - F1: replace caller operator_authorized with server-side HMAC auth artifacts - F2: implement fail-closed merger consumption for prior-provenance only - F3: enforce remote/org/repo/head on cross-profile load and clear Co-Authored-By: Grok 4.5 (xAI) --- gitea_mcp_server.py | 914 ++++++++++++-- irrecoverable_provenance.py | 902 ++++++++++++++ mcp_session_state.py | 89 +- stale_review_decision_lock.py | 86 +- task_capability_map.py | 23 +- ...t_issue_709_decision_lock_cross_profile.py | 1052 +++++++++++++---- 6 files changed, 2730 insertions(+), 336 deletions(-) create mode 100644 irrecoverable_provenance.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index f84f6c0..0a9270b 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -4553,7 +4553,56 @@ def _clear_decision_lock_for_profile( org: str | None, repo: str | None, ) -> dict: - """Clear one profile's durable decision lock when it targets *pr_number* approve.""" + """Clear one profile's durable decision lock when it targets *pr_number* approve. + + #709 F3: require remote/org/repo scope match on load and exact head identity + for any terminal match. PR-number-only fallback is forbidden — malformed, + legacy incomplete, cross-repository, or wrong-head locks are never cleared. + """ + try: + import irrecoverable_provenance as _irp + + path_gate = _irp.assess_profile_path_identity(profile_identity) + if not path_gate.get("valid"): + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": "; ".join(path_gate.get("reasons") or ["invalid profile"]), + "recovery_required": True, + } + except Exception: + raw = str(profile_identity or "") + if ".." in raw or "/" in raw or "\\" in raw: + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": "profile identity path traversal rejected (fail closed, #709 F3)", + "recovery_required": True, + } + + # expected_head_sha is mandatory for destructive clear (#709 F3). + want_head = stale_review_decision_lock.normalize_head_sha(expected_head_sha) + if not want_head: + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": ( + "expected_head_sha required for decision-lock clear " + "(no PR-number-only fallback; fail closed, #709 F3)" + ), + "recovery_required": False, + } + if not (remote and org and repo): + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": ( + "remote/org/repo required for decision-lock clear " + "(no incomplete-identity clear; fail closed, #709 F3)" + ), + "recovery_required": False, + } + lock = mcp_session_state.load_state_for_profile( kind=mcp_session_state.KIND_DECISION_LOCK, profile_identity=profile_identity, @@ -4561,17 +4610,25 @@ def _clear_decision_lock_for_profile( org=org, repo=repo, skip_identity_match=True, + enforce_repo_scope=True, ) if lock is None: return { "profile_identity": profile_identity, "cleared": False, - "reason": "no durable lock for profile", + "reason": ( + "no durable lock for profile at exact remote/org/repo scope " + "(or identity/expiry mismatch; fail closed)" + ), } - if not stale_review_decision_lock.lock_targets_merged_pr_approval( - lock, pr_number=pr_number, expected_head_sha=expected_head_sha - ): - # Also allow any terminal for this PR once merged (request_changes history). + + # Primary: approve of this PR at exact head. + targets_approve = stale_review_decision_lock.lock_targets_merged_pr_approval( + lock, pr_number=pr_number, expected_head_sha=want_head + ) + if not targets_approve: + # Secondary: any terminal for this PR **only** when head also matches. + # Never fall back to PR-number alone (#709 F3 / review 434). last = stale_review_decision_lock.last_terminal_mutation(lock) if not last or last.get("pr_number") != pr_number: return { @@ -4579,7 +4636,29 @@ def _clear_decision_lock_for_profile( "cleared": False, "reason": "lock terminal does not target this PR approval", } - # Archive then clear. + locked_head = stale_review_decision_lock.mutation_head_sha(last, lock) + if not locked_head: + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": ( + "legacy/incomplete terminal head identity; refuse destructive " + "clear (inspect/report recovery-required only, #709 F3)" + ), + "recovery_required": True, + "prior_summary": stale_review_decision_lock.lock_summary(lock), + } + if not stale_review_decision_lock.heads_equal(locked_head, want_head): + return { + "profile_identity": profile_identity, + "cleared": False, + "reason": ( + "lock terminal head does not match expected_head_sha " + "(fail closed, #709 F3)" + ), + } + + # Archive then clear — only after exact identity validation. try: mcp_session_state.save_state( kind=mcp_session_state.KIND_DECISION_LOCK_ARCHIVE, @@ -4587,12 +4666,16 @@ def _clear_decision_lock_for_profile( **dict(lock), "archived_reason": "post_merge_cross_profile_cleanup", "archived_for_pr": pr_number, + "archived_for_head": want_head, + "archived_remote": remote, + "archived_org": org, + "archived_repo": repo, "recovery_critical": True, "kind": mcp_session_state.KIND_DECISION_LOCK_ARCHIVE, }, - remote=remote or lock.get("remote"), - org=org or lock.get("org") or lock.get("ready_org"), - repo=repo or lock.get("repo") or lock.get("ready_repo"), + remote=remote, + org=org, + repo=repo, profile_identity=f"{profile_identity}-archive-pr{pr_number}", ) except Exception: @@ -4600,9 +4683,9 @@ def _clear_decision_lock_for_profile( mcp_session_state.clear_state( kind=mcp_session_state.KIND_DECISION_LOCK, profile_identity=profile_identity, - remote=remote or lock.get("remote"), - org=org or lock.get("org") or lock.get("ready_org"), - repo=repo or lock.get("repo") or lock.get("ready_repo"), + remote=remote, + org=org, + repo=repo, ) # If this is the in-memory active profile lock, clear memory too. active = _decision_lock_binding().get("profile_identity") @@ -4612,7 +4695,10 @@ def _clear_decision_lock_for_profile( return { "profile_identity": profile_identity, "cleared": True, - "reason": f"cleared terminal lock for merged PR #{pr_number}", + "reason": ( + f"cleared terminal lock for merged PR #{pr_number} " + f"at head {want_head[:12]}… (exact-scope)" + ), "prior_summary": stale_review_decision_lock.lock_summary(lock), } @@ -5025,59 +5111,79 @@ def gitea_cleanup_stale_review_decision_lock( return report +def _irrecoverable_capability_gate() -> list[str] | None: + """Dedicated recovery capability (gitea.read alone is insufficient).""" + import irrecoverable_provenance as irp + + profile = get_profile() + assessment = irp.assess_capability_for_irrecoverable_recovery( + allowed_operations=profile.get("allowed_operations") or [], + forbidden_operations=profile.get("forbidden_operations") or [], + role_kind=profile.get("role") or profile.get("role_kind"), + profile_name=profile.get("profile_name"), + ) + if assessment.get("allowed"): + return None + return list(assessment.get("reasons") or ["capability denied"]) + + @mcp.tool() -def gitea_record_irrecoverable_decision_lock_provenance( +def gitea_issue_irrecoverable_provenance_authorization( pr_number: int, - reason: str, + expected_head_sha: str, + incident_issue: int, + incident_comment_id: int, confirmation: str = "", - operator_authorized: bool = False, - expected_head_sha: str | None = None, - incident_ref: str | None = None, + destroyed_subject: str | None = None, remote: str = "dadeschools", host: str | None = None, org: str | None = None, repo: str | None = None, - post_audit_comment: bool = True, ) -> dict: - """Record truthful absence of decision-lock cleanup proof (#709 AC5). + """Mint a server-side authorization artifact for irrecoverable recovery (#709 F1). - Never emits applied=true or claims historical cleanup was proven. - Requires operator_authorized=True and confirmation exactly equal to - ``IRRECOVERABLE DECISION PROVENANCE PR ``. + Non-forgeable: requires production native MCP transport (or pytest), a + dedicated/reconciler mutation capability, live head equality, and validated + incident evidence. Confirmation is human intent only — never authorization. + Caller Booleans are not accepted. """ + import irrecoverable_provenance as irp + h, o, r = _resolve(remote, host, org, repo) - expected_confirm = f"IRRECOVERABLE DECISION PROVENANCE PR {int(pr_number)}" - report = { + report: dict = { "success": False, "performed": False, - "applied": False, - "historical_cleanup_proven": False, - "status": "provenance_irrecoverable", + "authorization": None, + "authorization_id": None, "pr_number": pr_number, "expected_head_sha": expected_head_sha, + "incident_issue": incident_issue, + "incident_comment_id": incident_comment_id, "reasons": [], - "record": None, - "audit_comment_id": None, } - read_block = _profile_operation_gate("gitea.read") - if read_block: - report["reasons"] = read_block - report["permission_report"] = _permission_block_report("gitea.read") + + cap_block = _irrecoverable_capability_gate() + if cap_block: + report["reasons"].extend(cap_block) + report["permission_report"] = { + "required_operation": irp.CAPABILITY_IRRECOVERABLE_RECOVERY, + "reasons": cap_block, + } return report - if not operator_authorized: - report["reasons"].append( - "operator_authorized must be true for irrecoverable provenance " - "recording (fail closed, #709 AC5)" - ) + + transport = irp.assess_transport_for_auth_mint() + if not transport.get("allowed"): + report["reasons"].extend(transport.get("reasons") or []) return report + + expected_confirm = irp.expected_confirmation(pr_number) if (confirmation or "").strip() != expected_confirm: report["reasons"].append( - f"confirmation must equal exactly {expected_confirm!r} (fail closed)" + f"confirmation must equal exactly {expected_confirm!r} " + "(human intent only; not an authorization credential; fail closed)" ) return report - if not (reason or "").strip(): - report["reasons"].append("reason is required (fail closed)") - return report + try: actor = _authenticated_username(h) except Exception: @@ -5089,45 +5195,385 @@ def gitea_record_irrecoverable_decision_lock_provenance( return report profile = get_profile() profile_name = (profile.get("profile_name") or "").strip() or None - # Idempotent: if matching record already exists for pr+head, return it. - binding = _decision_lock_binding() + + # Live PR head (authoritative). + live_head = None + pr_state = None + pr_err = None + try: + pr_live = api_request( + "GET", f"{repo_api_url(h, o, r)}/pulls/{int(pr_number)}", _auth(h) + ) + live_head = (pr_live or {}).get("head", {}) + if isinstance(live_head, dict): + live_head = live_head.get("sha") + else: + live_head = (pr_live or {}).get("head_sha") or (pr_live or {}).get( + "head_commit_sha" + ) + pr_state = (pr_live or {}).get("state") + except Exception as exc: # noqa: BLE001 + pr_err = _redact(str(exc)) + head_gate = irp.assess_live_head_binding( + expected_head_sha=expected_head_sha, + live_head_sha=live_head, + pr_lookup_error=pr_err, + pr_state=pr_state, + ) + if not head_gate.get("valid"): + report["reasons"].extend(head_gate.get("reasons") or []) + return report + + # Incident evidence live validation. + comment_payload = None + comment_err = None + try: + comment_payload = api_request( + "GET", + f"{repo_api_url(h, o, r)}/issues/comments/{int(incident_comment_id)}", + _auth(h), + ) + except Exception as exc: # noqa: BLE001 + comment_err = _redact(str(exc)) + incident_gate = irp.assess_incident_evidence( + incident_issue=incident_issue, + incident_comment_id=incident_comment_id, + comment_payload=comment_payload if isinstance(comment_payload, dict) else None, + comment_lookup_error=comment_err, + expected_remote=remote, + expected_org=o, + expected_repo=r, + ) + if not incident_gate.get("valid"): + report["reasons"].extend(incident_gate.get("reasons") or []) + return report + + auth_profile = irp.auth_state_profile_identity( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + ) + # Idempotent: return unconsumed matching auth. + existing = mcp_session_state.load_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + remote=remote, + org=o, + repo=r, + profile_identity=auth_profile, + ) + if isinstance(existing, dict): + v = irp.verify_authorization_artifact( + existing, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + incident_issue=incident_issue, + incident_comment_id=incident_comment_id, + require_unconsumed=True, + ) + if v.get("valid"): + report["success"] = True + report["performed"] = False + report["authorization"] = existing + report["authorization_id"] = existing.get("authorization_id") + report["reasons"].append( + "idempotent: unconsumed matching authorization already present" + ) + return report + + artifact = irp.build_authorization_artifact( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + incident_issue=int(incident_issue), + incident_comment_id=int(incident_comment_id), + destroyed_subject=destroyed_subject, + issuer_username=actor, + issuer_profile=profile_name or "unknown", + native_provenance=mcp_daemon_guard.mutation_provenance_fields(), + ) + artifact["kind"] = mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH + saved = mcp_session_state.save_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + payload=artifact, + remote=remote, + org=o, + repo=r, + profile_identity=auth_profile, + ) + report["authorization"] = dict(saved or artifact) + report["authorization_id"] = report["authorization"].get("authorization_id") + report["performed"] = True + report["success"] = True + report["reasons"].append( + "issued server-side irrecoverable provenance authorization " + "(non-forgeable; bound to remote/org/repo/PR/head/incident)" + ) + return report + + +@mcp.tool() +def gitea_record_irrecoverable_decision_lock_provenance( + pr_number: int, + reason: str, + confirmation: str = "", + expected_head_sha: str | None = None, + incident_issue: int | None = None, + incident_comment_id: int | None = None, + authorization_id: str | None = None, + destroyed_subject: str | None = None, + incident_ref: str | None = None, + # Deprecated: retained so callers that still pass it get an explicit deny. + operator_authorized: bool = False, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, + post_audit_comment: bool = True, +) -> dict: + """Record truthful absence of decision-lock cleanup proof (#709 AC5). + + Never emits applied=true or claims historical cleanup was proven. + + Authorization is a **server-side artifact** (see + ``gitea_issue_irrecoverable_provenance_authorization``). Caller-supplied + ``operator_authorized`` is **never** authorization evidence (review 434 F1). + Confirmation is human intent only. ``expected_head_sha``, + ``incident_issue``, and ``incident_comment_id`` are mandatory. + """ + import irrecoverable_provenance as irp + + h, o, r = _resolve(remote, host, org, repo) + expected_confirm = irp.expected_confirmation(pr_number) + report = { + "success": False, + "performed": False, + "applied": False, + "historical_cleanup_proven": False, + "status": "provenance_irrecoverable", + "pr_number": pr_number, + "expected_head_sha": expected_head_sha, + "reasons": [], + "record": None, + "audit_comment_id": None, + "authorization_id": authorization_id, + "merger_may_accept": False, + } + + # Explicitly reject self-assertable Boolean as sole/any authorization. + if operator_authorized: + report["reasons"].append( + "operator_authorized is not accepted as authorization evidence " + "(#709 F1 / review 434); mint a server-side authorization via " + "gitea_issue_irrecoverable_provenance_authorization" + ) + # Do not return yet — still report other failures — but never authorize. + # Actually fail immediately so success cannot be claimed. + return report + + cap_block = _irrecoverable_capability_gate() + if cap_block: + report["reasons"].extend(cap_block) + report["permission_report"] = { + "required_operation": irp.CAPABILITY_IRRECOVERABLE_RECOVERY, + "reasons": cap_block, + } + return report + + transport = irp.assess_transport_for_auth_mint() + if not transport.get("allowed"): + report["reasons"].extend(transport.get("reasons") or []) + return report + + if (confirmation or "").strip() != expected_confirm: + report["reasons"].append( + f"confirmation must equal exactly {expected_confirm!r} " + "(human intent only; not authorization; fail closed)" + ) + return report + if not (reason or "").strip(): + report["reasons"].append("reason is required (fail closed)") + return report + if not expected_head_sha or not str(expected_head_sha).strip(): + report["reasons"].append( + "expected_head_sha is mandatory (fail closed, #709 F1)" + ) + return report + if incident_issue is None or int(incident_issue) <= 0: + report["reasons"].append( + "incident_issue is mandatory (canonical incident evidence, #709 F1)" + ) + return report + if incident_comment_id is None or int(incident_comment_id) <= 0: + report["reasons"].append( + "incident_comment_id is mandatory (canonical incident evidence, #709 F1)" + ) + return report + # incident_ref alone is never sufficient (legacy arg ignored as authority). + _ = incident_ref + + try: + actor = _authenticated_username(h) + except Exception: + actor = None + if not actor: + report["reasons"].append( + "authenticated identity could not be verified (fail closed)" + ) + return report + profile = get_profile() + profile_name = (profile.get("profile_name") or "").strip() or None + + # Live head must match. + live_head = None + pr_err = None + try: + pr_live = api_request( + "GET", f"{repo_api_url(h, o, r)}/pulls/{int(pr_number)}", _auth(h) + ) + live_head = (pr_live or {}).get("head", {}) + if isinstance(live_head, dict): + live_head = live_head.get("sha") + else: + live_head = (pr_live or {}).get("head_sha") or (pr_live or {}).get( + "head_commit_sha" + ) + except Exception as exc: # noqa: BLE001 + pr_err = _redact(str(exc)) + head_gate = irp.assess_live_head_binding( + expected_head_sha=expected_head_sha, + live_head_sha=live_head, + pr_lookup_error=pr_err, + ) + if not head_gate.get("valid"): + report["reasons"].extend(head_gate.get("reasons") or []) + return report + + # Re-validate incident evidence at record time. + comment_payload = None + comment_err = None + try: + comment_payload = api_request( + "GET", + f"{repo_api_url(h, o, r)}/issues/comments/{int(incident_comment_id)}", + _auth(h), + ) + except Exception as exc: # noqa: BLE001 + comment_err = _redact(str(exc)) + incident_gate = irp.assess_incident_evidence( + incident_issue=incident_issue, + incident_comment_id=incident_comment_id, + comment_payload=comment_payload if isinstance(comment_payload, dict) else None, + comment_lookup_error=comment_err, + expected_org=o, + expected_repo=r, + ) + if not incident_gate.get("valid"): + report["reasons"].extend(incident_gate.get("reasons") or []) + return report + + # Load server-side authorization artifact (by scope; optional id check). + auth_profile = irp.auth_state_profile_identity( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + ) + authorization = mcp_session_state.load_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + remote=remote, + org=o, + repo=r, + profile_identity=auth_profile, + ) + if not isinstance(authorization, dict): + report["reasons"].append( + "no server-side authorization artifact for this exact scope; call " + "gitea_issue_irrecoverable_provenance_authorization first (fail closed)" + ) + return report + if authorization_id and authorization.get("authorization_id") != authorization_id: + report["reasons"].append( + "authorization_id does not match durable artifact for this scope " + "(fail closed)" + ) + return report + auth_check = irp.verify_authorization_artifact( + authorization, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + incident_issue=int(incident_issue), + incident_comment_id=int(incident_comment_id), + require_unconsumed=True, + ) + if not auth_check.get("valid"): + report["reasons"].extend(auth_check.get("reasons") or []) + return report + + recovery_profile = irp.recovery_state_profile_identity( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + ) existing = mcp_session_state.load_state( kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, remote=remote, org=o, repo=r, - profile_identity=binding.get("profile_identity"), + profile_identity=recovery_profile, ) if ( isinstance(existing, dict) and existing.get("pr_number") == pr_number - and ( - not expected_head_sha - or stale_review_decision_lock.heads_equal( - existing.get("head_sha"), expected_head_sha - ) + and stale_review_decision_lock.heads_equal( + existing.get("head_sha"), expected_head_sha ) and existing.get("status") == "provenance_irrecoverable" + and existing.get("authorization_id") == authorization.get("authorization_id") ): report["success"] = True report["performed"] = False report["record"] = existing - report["reasons"].append("idempotent: matching irrecoverable record already present") + report["merger_may_accept"] = bool(existing.get("merger_may_accept")) + report["authorization_id"] = existing.get("authorization_id") + report["reasons"].append( + "idempotent: matching irrecoverable record already present" + ) return report - record = stale_review_decision_lock.build_irrecoverable_provenance_record( + record = irp.build_irrecoverable_provenance_record( pr_number=pr_number, - head_sha=expected_head_sha, + head_sha=str(expected_head_sha), remote=remote, org=o, repo=r, actor_username=actor, profile_name=profile_name, reason=reason.strip(), - incident_ref=incident_ref, - operator_authorized=True, + incident_issue=int(incident_issue), + incident_comment_id=int(incident_comment_id), + authorization=authorization, + destroyed_subject=destroyed_subject, ) - # Stamp kind for TTL exemption + if not record.get("merger_may_accept"): + report["reasons"].append( + "built record is not merger-acceptable (authorization verify failed)" + ) + report["record"] = record + return report + record["kind"] = mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE saved = mcp_session_state.save_state( kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, @@ -5135,28 +5581,25 @@ def gitea_record_irrecoverable_decision_lock_provenance( remote=remote, org=o, repo=r, - profile_identity=binding.get("profile_identity"), + profile_identity=recovery_profile, ) report["record"] = dict(saved or record) report["performed"] = True report["success"] = True + report["merger_may_accept"] = True + report["authorization_id"] = record.get("authorization_id") report["reasons"].append( - "recorded provenance_irrecoverable (applied=false; historical cleanup not proven)" + "recorded provenance_irrecoverable (applied=false; historical cleanup " + "not proven; server authorization bound)" ) if post_audit_comment: - comment_block = _profile_operation_gate("gitea.pr.comment") or _profile_operation_gate( - "gitea.issue.comment" - ) - # Prefer issue comment capability for discussion thread. issue_block = _profile_operation_gate("gitea.issue.comment") if issue_block: report["reasons"].append(f"audit comment skipped: {issue_block}") else: try: - body = stale_review_decision_lock.format_irrecoverable_audit_comment( - report["record"] - ) + body = irp.format_irrecoverable_audit_comment(report["record"]) comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments" with _audited( "comment_issue", @@ -5177,7 +5620,6 @@ def gitea_record_irrecoverable_decision_lock_provenance( ) report["audit_comment_id"] = (posted or {}).get("id") if report["audit_comment_id"]: - # Re-save with comment id for readback completeness. report["record"]["audit_comment_id"] = report["audit_comment_id"] mcp_session_state.save_state( kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, @@ -5185,7 +5627,7 @@ def gitea_record_irrecoverable_decision_lock_provenance( remote=remote, org=o, repo=r, - profile_identity=binding.get("profile_identity"), + profile_identity=recovery_profile, ) except Exception as exc: # noqa: BLE001 report["reasons"].append( @@ -5194,6 +5636,209 @@ def gitea_record_irrecoverable_decision_lock_provenance( return report +@mcp.tool() +def gitea_consume_irrecoverable_decision_lock_provenance( + pr_number: int, + expected_head_sha: str, + confirmation: str = "", + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Merger-side fail-closed consumption of an irrecoverable recovery record (#709 F2). + + Resolves **only** the historical prior-provenance blocker for the exact + remote/org/repo/PR/head. Never bypasses approval, change-requests, lease, + mergeability, anti-stomp, runtime, or workspace gates. Consumption is + durable, auditable, and idempotent. + """ + import irrecoverable_provenance as irp + + h, o, r = _resolve(remote, host, org, repo) + expected_confirm = f"CONSUME IRRECOVERABLE PROVENANCE PR {int(pr_number)}" + report: dict = { + "success": False, + "performed": False, + "pr_number": pr_number, + "expected_head_sha": expected_head_sha, + "historical_cleanup_proven": False, + "historical_cleanup_not_proven": True, + "irrecoverable_recovery_authorized": False, + "recovery_record_consumed": False, + "resolves_prior_provenance_blocker": False, + "normal_approval_and_merge_gates": "not_substituted", + "reasons": [], + "assessment": None, + } + + # Merger or reconciler may consume; gitea.read alone insufficient. + merge_block = _profile_operation_gate("gitea.pr.merge") + cap_block = _irrecoverable_capability_gate() + if merge_block and cap_block: + report["reasons"].append( + "consume requires gitea.pr.merge (merger) or irrecoverable-recovery " + "capability (reconciler); gitea.read alone is insufficient (#709 F2)" + ) + if merge_block: + report["reasons"].extend( + merge_block if isinstance(merge_block, list) else [str(merge_block)] + ) + report["reasons"].extend(cap_block) + return report + + if (confirmation or "").strip() != expected_confirm: + report["reasons"].append( + f"confirmation must equal exactly {expected_confirm!r} " + "(human intent; fail closed)" + ) + return report + + try: + actor = _authenticated_username(h) + except Exception: + actor = None + if not actor: + report["reasons"].append("authenticated identity unverified (fail closed)") + return report + profile = get_profile() + profile_name = (profile.get("profile_name") or "").strip() or None + + # Live head. + live_head = None + pr_err = None + try: + pr_live = api_request( + "GET", f"{repo_api_url(h, o, r)}/pulls/{int(pr_number)}", _auth(h) + ) + live_head = (pr_live or {}).get("head", {}) + if isinstance(live_head, dict): + live_head = live_head.get("sha") + else: + live_head = (pr_live or {}).get("head_sha") + except Exception as exc: # noqa: BLE001 + pr_err = _redact(str(exc)) + head_gate = irp.assess_live_head_binding( + expected_head_sha=expected_head_sha, + live_head_sha=live_head, + pr_lookup_error=pr_err, + ) + if not head_gate.get("valid"): + report["reasons"].extend(head_gate.get("reasons") or []) + return report + + recovery_profile = irp.recovery_state_profile_identity( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + ) + recovery = mcp_session_state.load_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, + remote=remote, + org=o, + repo=r, + profile_identity=recovery_profile, + ) + auth_profile = irp.auth_state_profile_identity( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + ) + authorization = mcp_session_state.load_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + remote=remote, + org=o, + repo=r, + profile_identity=auth_profile, + ) + + # Pull formal review state so consumption cannot be claimed when normal + # gates would fail (assessment only — does not merge). + feedback = gitea_get_pr_review_feedback( + pr_number=pr_number, remote=remote, host=host, org=org, repo=repo + ) + assessment = irp.assess_merger_consumption( + recovery if isinstance(recovery, dict) else None, + authorization if isinstance(authorization, dict) else None, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + live_head_sha=live_head, + approval_at_current_head=feedback.get("approval_at_current_head") + if feedback.get("success") + else None, + has_blocking_change_requests=feedback.get("has_blocking_change_requests") + if feedback.get("success") + else None, + ) + report["assessment"] = assessment + report["reasons"].extend(assessment.get("reasons") or []) + report["irrecoverable_recovery_authorized"] = bool( + assessment.get("irrecoverable_recovery_authorized") + ) + report["resolves_prior_provenance_blocker"] = bool( + assessment.get("resolves_prior_provenance_blocker") + ) + report["historical_cleanup_proven"] = False + report["historical_cleanup_not_proven"] = True + + if not assessment.get("allowed") and not assessment.get( + "recovery_record_consumed" + ): + return report + + # Atomic-ish durable consumption (auth then recovery under exclusive locks + # via save_state). + if isinstance(authorization, dict) and not authorization.get("consumed_at"): + consumed_auth = irp.mark_consumed( + authorization, + consumer_username=actor, + consumer_profile=profile_name, + ) + mcp_session_state.save_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + payload=consumed_auth, + remote=remote, + org=o, + repo=r, + profile_identity=auth_profile, + ) + if isinstance(recovery, dict) and not recovery.get("consumed_at"): + consumed_rec = irp.mark_consumed( + recovery, + consumer_username=actor, + consumer_profile=profile_name, + ) + consumed_rec["prior_provenance_blocker_resolved"] = True + saved = mcp_session_state.save_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, + payload=consumed_rec, + remote=remote, + org=o, + repo=r, + profile_identity=recovery_profile, + ) + report["record"] = dict(saved or consumed_rec) + report["performed"] = True + else: + report["record"] = recovery + report["performed"] = False + + report["recovery_record_consumed"] = True + report["success"] = True + report["resolves_prior_provenance_blocker"] = True + report["reasons"].append( + "prior-provenance blocker resolved for exact scope; historical cleanup " + "remains unproven; normal merge gates still required" + ) + return report + + @mcp.tool() def gitea_dry_run_pr_review( pr_number: int, @@ -6244,6 +6889,137 @@ def gitea_merge_pr( reasons.append(str(e)) return result + # Gate 8b — optional irrecoverable prior-provenance recovery report (#709 F2). + # Never substitutes for approval/lease/mergeability gates above. Surfaces + # truthful distinctions for controllers/mergers. + try: + import irrecoverable_provenance as _irp_merge + + _rec_prof = _irp_merge.recovery_state_profile_identity( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha or actual_sha or ""), + ) + _auth_prof = _irp_merge.auth_state_profile_identity( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha or actual_sha or ""), + ) + _recovery = mcp_session_state.load_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, + remote=remote, + org=o, + repo=r, + profile_identity=_rec_prof, + ) + _auth_art = mcp_session_state.load_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + remote=remote, + org=o, + repo=r, + profile_identity=_auth_prof, + ) + if isinstance(_recovery, dict) or isinstance(_auth_art, dict): + _assess = _irp_merge.assess_merger_consumption( + _recovery if isinstance(_recovery, dict) else None, + _auth_art if isinstance(_auth_art, dict) else None, + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + live_head_sha=actual_sha, + approval_at_current_head=bool( + feedback.get("approval_at_current_head") + ), + has_blocking_change_requests=bool( + feedback.get("has_blocking_change_requests") + ), + mergeable=result.get("mergeable"), + lease_ok=True, + runtime_ok=True, + workspace_ok=True, + anti_stomp_ok=True, + ) + result["irrecoverable_provenance"] = { + "historical_cleanup_proven": False, + "historical_cleanup_not_proven": True, + "irrecoverable_recovery_authorized": bool( + _assess.get("irrecoverable_recovery_authorized") + ), + "recovery_record_consumed": bool( + _assess.get("recovery_record_consumed") + or ( + isinstance(_recovery, dict) and _recovery.get("consumed_at") + ) + ), + "resolves_prior_provenance_blocker": bool( + _assess.get("resolves_prior_provenance_blocker") + ), + "assessment_reasons": list(_assess.get("reasons") or []), + } + # Consume on successful merge path when allowed and not yet consumed. + if _assess.get("allowed") and isinstance(_recovery, dict) and not _recovery.get( + "consumed_at" + ): + try: + if isinstance(_auth_art, dict) and not _auth_art.get("consumed_at"): + mcp_session_state.save_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + payload=_irp_merge.mark_consumed( + _auth_art, + consumer_username=auth_user, + consumer_profile=result.get("profile_name"), + ), + remote=remote, + org=o, + repo=r, + profile_identity=_auth_prof, + ) + mcp_session_state.save_state( + kind=mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE, + payload={ + **_irp_merge.mark_consumed( + _recovery, + consumer_username=auth_user, + consumer_profile=result.get("profile_name"), + ), + "prior_provenance_blocker_resolved": True, + }, + remote=remote, + org=o, + repo=r, + profile_identity=_rec_prof, + ) + result["irrecoverable_provenance"][ + "recovery_record_consumed" + ] = True + result["irrecoverable_provenance"][ + "consumed_at_merge_preflight" + ] = True + except Exception as _cons_exc: # noqa: BLE001 + result["irrecoverable_provenance"]["consume_error"] = _redact( + str(_cons_exc) + ) + else: + result["irrecoverable_provenance"] = { + "historical_cleanup_proven": False, + "historical_cleanup_not_proven": True, + "irrecoverable_recovery_authorized": False, + "recovery_record_consumed": False, + "resolves_prior_provenance_blocker": False, + "note": "no irrecoverable recovery record for this exact scope", + } + except Exception as _irp_exc: # noqa: BLE001 — never block merge path + result["irrecoverable_provenance"] = { + "error": _redact(str(_irp_exc)), + "historical_cleanup_proven": False, + "historical_cleanup_not_proven": True, + } + # All gates passed — perform the single merge mutation. try: auth = _auth(h) diff --git a/irrecoverable_provenance.py b/irrecoverable_provenance.py new file mode 100644 index 0000000..3a62086 --- /dev/null +++ b/irrecoverable_provenance.py @@ -0,0 +1,902 @@ +"""Server-side irrecoverable decision-lock provenance authorization (#709 AC5). + +Authorization is **not** a caller-supplied Boolean. A durable, non-forgeable +authorization artifact must be minted under production native MCP transport +(or pytest) with a dedicated mutation capability, live head binding, and +validated incident evidence. Merger consumption is fail-closed and resolves +only the historical-provenance blocker — never normal approval, lease, +mergeability, anti-stomp, or workspace gates. +""" + +from __future__ import annotations + +import hashlib +import hmac +import json +import os +import secrets +import uuid +from datetime import datetime, timedelta, timezone +from typing import Any + +import mcp_daemon_guard +import mcp_session_state +from stale_review_decision_lock import heads_equal, normalize_head_sha + +# Dedicated mutation capability (#709 review 434 F1). +CAPABILITY_IRRECOVERABLE_RECOVERY = "gitea.decision_lock.irrecoverable_recovery" + +KIND_AUTH = "irrecoverable_provenance_authorization" +KIND_RECOVERY = mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE + +CONFIRMATION_PREFIX = "IRRECOVERABLE DECISION PROVENANCE PR" +AUTH_TTL_HOURS = 24.0 +RECORD_TYPE = "irrecoverable_decision_provenance" +AUTH_TYPE = "irrecoverable_provenance_authorization" + +# Internal HMAC material is process-local and never caller-supplied. Pytest +# gets a deterministic salt so hermetic tests are stable; production uses +# transport fingerprint + random secret minted at process start. +_PROCESS_AUTH_SECRET: bytes | None = None + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +def _now_iso() -> str: + return _now().isoformat() + + +def _process_secret() -> bytes: + global _PROCESS_AUTH_SECRET + if _PROCESS_AUTH_SECRET is None: + if mcp_daemon_guard.is_pytest_runtime(): + _PROCESS_AUTH_SECRET = b"pytest-irrecoverable-auth-v1" + else: + _PROCESS_AUTH_SECRET = secrets.token_bytes(32) + return _PROCESS_AUTH_SECRET + + +def expected_confirmation(pr_number: int) -> str: + """Human intent confirmation text (not an authorization credential).""" + return f"{CONFIRMATION_PREFIX} {int(pr_number)}" + + +def auth_state_profile_identity( + *, + remote: str, + org: str, + repo: str, + pr_number: int, + expected_head_sha: str, +) -> str: + """Stable durable key segment for one exact-scope authorization.""" + head = normalize_head_sha(expected_head_sha) or "nohead" + segs = [ + KIND_AUTH, + mcp_session_state._sanitize_segment(remote), + mcp_session_state._sanitize_segment(org), + mcp_session_state._sanitize_segment(repo), + f"pr{int(pr_number)}", + head[:16], + ] + return "-".join(segs) + + +def recovery_state_profile_identity( + *, + remote: str, + org: str, + repo: str, + pr_number: int, + expected_head_sha: str, +) -> str: + head = normalize_head_sha(expected_head_sha) or "nohead" + segs = [ + KIND_RECOVERY, + mcp_session_state._sanitize_segment(remote), + mcp_session_state._sanitize_segment(org), + mcp_session_state._sanitize_segment(repo), + f"pr{int(pr_number)}", + head[:16], + ] + return "-".join(segs) + + +def _scope_payload( + *, + remote: str, + org: str, + repo: str, + pr_number: int, + expected_head_sha: str, + incident_issue: int, + incident_comment_id: int, + destroyed_subject: str | None, + issuer_username: str, + issuer_profile: str, + created_at: str, + expires_at: str, + authorization_id: str, +) -> dict[str, Any]: + return { + "authorization_id": authorization_id, + "remote": remote, + "org": org, + "repo": repo, + "blocked_pr_number": int(pr_number), + "expected_head_sha": normalize_head_sha(expected_head_sha), + "incident_issue": int(incident_issue), + "incident_comment_id": int(incident_comment_id), + "destroyed_subject": (destroyed_subject or "").strip() or None, + "issuer_username": issuer_username, + "issuer_profile": issuer_profile, + "created_at": created_at, + "expires_at": expires_at, + } + + +def _sign_scope(scope: dict[str, Any], native_provenance: dict[str, Any]) -> str: + """HMAC over canonical scope + native transport fingerprint (non-caller).""" + material = { + "scope": scope, + "native": { + "native_mcp_transport": bool( + native_provenance.get("native_mcp_transport") + ), + "production_native_mcp_transport": bool( + native_provenance.get("production_native_mcp_transport") + ), + "token_fingerprint": native_provenance.get("token_fingerprint"), + "entrypoint": native_provenance.get("entrypoint"), + "pid": native_provenance.get("pid"), + }, + } + blob = json.dumps(material, sort_keys=True, separators=(",", ":")).encode( + "utf-8" + ) + return hmac.new(_process_secret(), blob, hashlib.sha256).hexdigest() + + +def assess_capability_for_irrecoverable_recovery( + *, + allowed_operations: list[str] | None, + forbidden_operations: list[str] | None = None, + role_kind: str | None = None, + profile_name: str | None = None, +) -> dict[str, Any]: + """Whether the active profile may mint/use irrecoverable recovery (#709 F1). + + Accepts the dedicated capability, or a reconciler-shaped profile that + already holds issue-comment mutation rights (interim equivalence until + operators grant the dedicated op). Never treats bare ``gitea.read`` as + sufficient. + """ + import gitea_config + import reconciler_profile + + allowed = list(allowed_operations or []) + forbidden = list(forbidden_operations or []) + reasons: list[str] = [] + + dedicated_ok, _ = gitea_config.check_operation( + CAPABILITY_IRRECOVERABLE_RECOVERY, allowed, forbidden + ) + if dedicated_ok: + return { + "allowed": True, + "capability": CAPABILITY_IRRECOVERABLE_RECOVERY, + "via": "dedicated_capability", + "reasons": [], + } + + # Interim: reconciler profile with issue.comment (mutation, not read-only). + is_reconciler = reconciler_profile.is_reconciler_profile(allowed, forbidden) + comment_ok, _ = gitea_config.check_operation( + "gitea.issue.comment", allowed, forbidden + ) + role = (role_kind or "").strip().lower() + name = (profile_name or "").strip().lower() + role_looks_reconciler = role == "reconciler" or "reconciler" in name + if is_reconciler and comment_ok: + return { + "allowed": True, + "capability": CAPABILITY_IRRECOVERABLE_RECOVERY, + "via": "reconciler_profile_equivalence", + "reasons": [], + } + if role_looks_reconciler and comment_ok and not dedicated_ok: + # Role metadata says reconciler but ops incomplete — still fail if + # is_reconciler_profile is false (missing pr.close). + reasons.append( + "reconciler role metadata without reconciler-required operations " + f"(need {CAPABILITY_IRRECOVERABLE_RECOVERY} or reconciler profile " + "with gitea.pr.close + gitea.issue.comment; gitea.read alone is " + "insufficient, #709 F1)" + ) + else: + reasons.append( + f"missing dedicated capability {CAPABILITY_IRRECOVERABLE_RECOVERY} " + "(gitea.read is insufficient; require reconciler-capable mutation " + "profile or explicit grant, #709 F1)" + ) + return { + "allowed": False, + "capability": CAPABILITY_IRRECOVERABLE_RECOVERY, + "via": None, + "reasons": reasons, + } + + +def assess_transport_for_auth_mint() -> dict[str, Any]: + """Native transport required for minting non-forgeable auth artifacts.""" + reasons: list[str] = [] + native = mcp_daemon_guard.is_native_mcp_transport() + pytest = mcp_daemon_guard.is_pytest_runtime() + production = mcp_daemon_guard.is_production_native_mcp_transport() + if not native and not pytest: + reasons.append( + "irrecoverable provenance authorization requires production native " + "MCP transport; ordinary Python processes cannot mint acceptable " + "recovery authorization (#709 F1)" + ) + return { + "allowed": not reasons, + "native_mcp_transport": native, + "production_native_mcp_transport": production, + "pytest": pytest, + "reasons": reasons, + } + + +def assess_incident_evidence( + *, + incident_issue: int | None, + incident_comment_id: int | None, + comment_payload: dict[str, Any] | None, + comment_lookup_error: str | None = None, + expected_remote: str | None = None, + expected_org: str | None = None, + expected_repo: str | None = None, +) -> dict[str, Any]: + """Validate canonical incident evidence is present and live-fetched.""" + reasons: list[str] = [] + if incident_issue is None or int(incident_issue) <= 0: + reasons.append("incident_issue is required and must be a positive integer") + if incident_comment_id is None or int(incident_comment_id) <= 0: + reasons.append( + "incident_comment_id is required and must be a positive integer" + ) + if comment_lookup_error: + reasons.append( + f"incident evidence lookup failed: {comment_lookup_error} (fail closed)" + ) + if not isinstance(comment_payload, dict): + if not comment_lookup_error: + reasons.append( + "incident evidence not found or not a comment object (fail closed)" + ) + return {"valid": False, "reasons": reasons, "comment": None} + + # Gitea returns comment with id; optional issue_url / html_url for scope. + cid = comment_payload.get("id") + try: + if int(cid) != int(incident_comment_id): # type: ignore[arg-type] + reasons.append( + "incident comment id mismatch against live payload (fail closed)" + ) + except (TypeError, ValueError): + reasons.append("incident comment payload missing valid id (fail closed)") + + body = (comment_payload.get("body") or "").strip() + if not body: + reasons.append("incident comment body is empty (fail closed)") + + # Soft scope hints when URLs are present (never hard-code issue numbers). + issue_url = str( + comment_payload.get("issue_url") + or comment_payload.get("html_url") + or "" + ) + if expected_org and expected_org not in issue_url and issue_url: + # Only fail when URL is present and clearly wrong-org; missing URL ok. + if f"/{expected_org}/" not in issue_url: + # html_url may be /user/repo/issues/n — check repo if provided + if expected_repo and f"/{expected_repo}/" not in issue_url: + reasons.append( + "incident evidence URL does not match expected repository " + "(fail closed)" + ) + + return { + "valid": not reasons, + "reasons": reasons, + "comment": { + "id": comment_payload.get("id"), + "author": ( + (comment_payload.get("user") or {}).get("login") + if isinstance(comment_payload.get("user"), dict) + else comment_payload.get("user") + ), + "created_at": comment_payload.get("created_at"), + "body_len": len(body), + }, + } + + +def assess_live_head_binding( + *, + expected_head_sha: str | None, + live_head_sha: str | None, + pr_lookup_error: str | None = None, + pr_state: str | None = None, +) -> dict[str, Any]: + """expected_head_sha mandatory and must equal live PR head.""" + reasons: list[str] = [] + want = normalize_head_sha(expected_head_sha) + have = normalize_head_sha(live_head_sha) + if not want: + reasons.append( + "expected_head_sha is mandatory and must be a non-empty SHA " + "(fail closed, #709 F1)" + ) + if pr_lookup_error: + reasons.append(f"live PR head lookup failed: {pr_lookup_error} (fail closed)") + if want and not have: + reasons.append("live PR head SHA unavailable (fail closed)") + if want and have and not heads_equal(want, have): + reasons.append( + "expected_head_sha does not equal live PR head " + f"(expected={want[:12]}… live={have[:12]}…; fail closed, #709 F1)" + ) + return { + "valid": not reasons, + "expected_head_sha": want, + "live_head_sha": have, + "pr_state": pr_state, + "reasons": reasons, + } + + +def build_authorization_artifact( + *, + remote: str, + org: str, + repo: str, + pr_number: int, + expected_head_sha: str, + incident_issue: int, + incident_comment_id: int, + destroyed_subject: str | None, + issuer_username: str, + issuer_profile: str, + native_provenance: dict[str, Any] | None = None, + ttl_hours: float = AUTH_TTL_HOURS, +) -> dict[str, Any]: + """Build a server-side authorization artifact (caller cannot forge signature).""" + provenance = dict( + native_provenance or mcp_daemon_guard.mutation_provenance_fields() + ) + created = _now() + expires = created + timedelta(hours=float(ttl_hours)) + authorization_id = str(uuid.uuid4()) + scope = _scope_payload( + remote=remote, + org=org, + repo=repo, + pr_number=pr_number, + expected_head_sha=expected_head_sha, + incident_issue=incident_issue, + incident_comment_id=incident_comment_id, + destroyed_subject=destroyed_subject, + issuer_username=issuer_username, + issuer_profile=issuer_profile, + created_at=created.isoformat(), + expires_at=expires.isoformat(), + authorization_id=authorization_id, + ) + signature = _sign_scope(scope, provenance) + return { + "kind": KIND_AUTH, + "auth_type": AUTH_TYPE, + "record_type": AUTH_TYPE, + "status": "issued", + "consumption_state": "issued", + "consumed_at": None, + "recovery_critical": True, + "issue_ref": "#709", + "server_signature": signature, + "native_provenance": provenance, + **scope, + "timestamp": created.isoformat(), + "recorded_at": created.isoformat(), + "updated_at": created.isoformat(), + } + + +def verify_authorization_artifact( + auth: dict[str, Any] | None, + *, + remote: str, + org: str, + repo: str, + pr_number: int, + expected_head_sha: str, + incident_issue: int | None = None, + incident_comment_id: int | None = None, + require_unconsumed: bool = True, + now: datetime | None = None, +) -> dict[str, Any]: + """Fail-closed verification of a server-side authorization artifact.""" + reasons: list[str] = [] + if not isinstance(auth, dict): + return { + "valid": False, + "reasons": ["authorization artifact missing (fail closed)"], + } + if (auth.get("kind") or auth.get("auth_type") or "") not in ( + KIND_AUTH, + AUTH_TYPE, + ) and auth.get("record_type") != AUTH_TYPE: + if (auth.get("kind") or "") != KIND_AUTH: + reasons.append( + f"authorization kind mismatch (expected {KIND_AUTH!r}; fail closed)" + ) + + for field, want in ( + ("remote", remote), + ("org", org), + ("repo", repo), + ): + have = (str(auth.get(field) or "")).strip() + if not have or have != (want or "").strip(): + reasons.append( + f"authorization {field} mismatch " + f"(stored={have!r}, expected={want!r}; fail closed)" + ) + + try: + if int(auth.get("blocked_pr_number")) != int(pr_number): + reasons.append("authorization PR number mismatch (fail closed)") + except (TypeError, ValueError): + reasons.append("authorization missing blocked_pr_number (fail closed)") + + if not heads_equal(auth.get("expected_head_sha"), expected_head_sha): + reasons.append("authorization head SHA mismatch (fail closed)") + + if incident_issue is not None: + try: + if int(auth.get("incident_issue")) != int(incident_issue): + reasons.append("authorization incident_issue mismatch (fail closed)") + except (TypeError, ValueError): + reasons.append("authorization missing incident_issue (fail closed)") + if incident_comment_id is not None: + try: + if int(auth.get("incident_comment_id")) != int(incident_comment_id): + reasons.append( + "authorization incident_comment_id mismatch (fail closed)" + ) + except (TypeError, ValueError): + reasons.append("authorization missing incident_comment_id (fail closed)") + + if not (auth.get("issuer_username") or "").strip(): + reasons.append("authorization missing issuer_username (fail closed)") + if not (auth.get("issuer_profile") or "").strip(): + reasons.append("authorization missing issuer_profile (fail closed)") + if not (auth.get("server_signature") or "").strip(): + reasons.append("authorization missing server_signature (fail closed)") + + # Recompute signature over stored scope fields. + try: + scope = _scope_payload( + remote=str(auth.get("remote") or ""), + org=str(auth.get("org") or ""), + repo=str(auth.get("repo") or ""), + pr_number=int(auth.get("blocked_pr_number")), + expected_head_sha=str(auth.get("expected_head_sha") or ""), + incident_issue=int(auth.get("incident_issue")), + incident_comment_id=int(auth.get("incident_comment_id")), + destroyed_subject=auth.get("destroyed_subject"), + issuer_username=str(auth.get("issuer_username") or ""), + issuer_profile=str(auth.get("issuer_profile") or ""), + created_at=str(auth.get("created_at") or ""), + expires_at=str(auth.get("expires_at") or ""), + authorization_id=str(auth.get("authorization_id") or ""), + ) + native = auth.get("native_provenance") or {} + if not isinstance(native, dict): + native = {} + expected_sig = _sign_scope(scope, native) + if not hmac.compare_digest( + expected_sig, str(auth.get("server_signature") or "") + ): + reasons.append( + "authorization server_signature invalid (forged or corrupt; " + "fail closed, #709 F1)" + ) + except (TypeError, ValueError) as exc: + reasons.append(f"authorization scope incomplete: {exc} (fail closed)") + + # Native provenance required on the artifact itself. + native = auth.get("native_provenance") or {} + if not isinstance(native, dict) or not ( + native.get("native_mcp_transport") or native.get("pytest") + ): + # Pytest artifacts stamp pytest=True via mutation_provenance_fields. + if not mcp_daemon_guard.is_pytest_runtime(): + if not (isinstance(native, dict) and native.get("native_mcp_transport")): + reasons.append( + "authorization lacks native transport provenance (fail closed)" + ) + + # Expiry / consumption. + now_dt = now or _now() + expires_raw = auth.get("expires_at") + expires_dt = None + if expires_raw: + text = str(expires_raw).strip() + if text.endswith("Z"): + text = text[:-1] + "+00:00" + try: + expires_dt = datetime.fromisoformat(text) + if expires_dt.tzinfo is None: + expires_dt = expires_dt.replace(tzinfo=timezone.utc) + except ValueError: + reasons.append("authorization expires_at unparseable (fail closed)") + else: + reasons.append("authorization missing expires_at (fail closed)") + if expires_dt is not None and now_dt > expires_dt: + reasons.append("authorization expired (fail closed)") + + state = (auth.get("consumption_state") or auth.get("status") or "").strip() + if require_unconsumed and state in ("consumed", "expired"): + reasons.append( + f"authorization already {state}; cannot be replayed (fail closed)" + ) + if require_unconsumed and auth.get("consumed_at"): + reasons.append("authorization already consumed (fail closed)") + + return { + "valid": not reasons, + "reasons": reasons, + "authorization_id": auth.get("authorization_id"), + "consumption_state": state or None, + } + + +def build_irrecoverable_provenance_record( + *, + pr_number: int, + head_sha: str, + remote: str, + org: str, + repo: str, + actor_username: str | None, + profile_name: str | None, + reason: str, + incident_issue: int, + incident_comment_id: int, + authorization: dict[str, Any], + destroyed_subject: str | None = None, + historical_provenance_subject: str | None = None, +) -> dict[str, Any]: + """Truthful absence-of-proof record. Never sets applied=True. + + ``merger_may_accept`` is True only when *authorization* verifies for the + exact scope. Caller-supplied Booleans are never consulted. + """ + head = normalize_head_sha(head_sha) + auth_check = verify_authorization_artifact( + authorization, + remote=remote, + org=org, + repo=repo, + pr_number=pr_number, + expected_head_sha=head or "", + incident_issue=incident_issue, + incident_comment_id=incident_comment_id, + require_unconsumed=True, + ) + may_accept = bool(auth_check.get("valid")) + return { + "event": "irrecoverable_decision_lock_provenance", + "status": "provenance_irrecoverable", + "record_type": RECORD_TYPE, + "kind": KIND_RECOVERY, + "operator_recovery_required": True, + "issue_ref": "#709", + "recovery_critical": True, + "applied": False, + "historical_cleanup_proven": False, + "timestamp": _now_iso(), + "pr_number": int(pr_number), + "blocked_pr_number": int(pr_number), + "head_sha": head, + "remote": remote, + "org": org, + "repo": repo, + "actor_username": actor_username, + "profile_name": profile_name, + "reason": reason, + "incident_issue": int(incident_issue), + "incident_comment_id": int(incident_comment_id), + # Legacy field for audit readability; not a caller Boolean gate. + "incident_ref": f"issue:{int(incident_issue)}/comment:{int(incident_comment_id)}", + "authorization_id": authorization.get("authorization_id"), + "authorization_issuer": authorization.get("issuer_username"), + "authorization_issuer_profile": authorization.get("issuer_profile"), + "authorization_verified": may_accept, + "authorization_verify_reasons": list(auth_check.get("reasons") or []), + "destroyed_subject": (destroyed_subject or "").strip() or None, + "historical_provenance_subject": ( + (historical_provenance_subject or destroyed_subject or "").strip() + or None + ), + "consumption_state": "issued", + "consumed_at": None, + "merger_may_accept": may_accept, + "acceptance_rule": ( + "Merger may accept this record only when a server-side authorization " + "artifact verifies for remote/org/repo/PR/exact-head/incident, the " + "record is durable and read back, the auth is unexpired and unconsumed, " + "and normal merge gates still pass. Resolves only the historical " + "prior-provenance blocker; never proves historical cleanup " + "(applied=false, historical_cleanup_proven=false)." + ), + "native_provenance": mcp_daemon_guard.mutation_provenance_fields(), + } + + +def format_irrecoverable_audit_comment(record: dict[str, Any]) -> str: + """Markdown body for irrecoverable provenance audit (no applied=true claim).""" + lines = [ + "## Irrecoverable decision-lock provenance (#709)", + "", + "Status: **PROVENANCE_IRRECOVERABLE** (not applied cleanup)", + "", + f"- actor: `{record.get('actor_username')}`", + f"- profile: `{record.get('profile_name')}`", + f"- timestamp: `{record.get('timestamp')}`", + f"- PR: `#{record.get('pr_number')}`", + f"- head_sha: `{record.get('head_sha')}`", + f"- incident_issue: `{record.get('incident_issue')}`", + f"- incident_comment_id: `{record.get('incident_comment_id')}`", + f"- authorization_id: `{record.get('authorization_id')}`", + f"- authorization_issuer: `{record.get('authorization_issuer')}`", + f"- authorization_verified: `{record.get('authorization_verified')}`", + f"- historical_cleanup_proven: `{record.get('historical_cleanup_proven')}`", + f"- applied: `{record.get('applied')}` (must remain false)", + f"- merger_may_accept: `{record.get('merger_may_accept')}`", + f"- destroyed_subject: `{record.get('destroyed_subject')}`", + "", + f"Reason: {record.get('reason')}", + "", + "This record documents **absence of proof**, not successful cleanup.", + "It must not be reused for a different PR or head (#709 AC6).", + "Authorization is a server-side artifact — not a caller Boolean.", + ] + return "\n".join(lines) + + +def assess_merger_consumption( + recovery: dict[str, Any] | None, + authorization: dict[str, Any] | None, + *, + remote: str, + org: str, + repo: str, + pr_number: int, + live_head_sha: str | None, + # Normal merge gate outcomes (must still pass independently). + approval_at_current_head: bool | None = None, + has_blocking_change_requests: bool | None = None, + mergeable: bool | None = None, + lease_ok: bool | None = None, + runtime_ok: bool | None = None, + workspace_ok: bool | None = None, + anti_stomp_ok: bool | None = None, + now: datetime | None = None, +) -> dict[str, Any]: + """Fail-closed merger assessment for consuming an irrecoverable recovery. + + Resolves **only** the historical prior-provenance blocker when all + recovery checks pass. Never grants a pass when normal merge gates fail. + """ + reasons: list[str] = [] + result: dict[str, Any] = { + "allowed": False, + "resolves_prior_provenance_blocker": False, + "historical_cleanup_proven": False, + "irrecoverable_recovery_authorized": False, + "recovery_record_consumed": False, + "reasons": reasons, + "normal_gates": { + "approval_at_current_head": approval_at_current_head, + "has_blocking_change_requests": has_blocking_change_requests, + "mergeable": mergeable, + "lease_ok": lease_ok, + "runtime_ok": runtime_ok, + "workspace_ok": workspace_ok, + "anti_stomp_ok": anti_stomp_ok, + }, + } + + if not isinstance(recovery, dict): + reasons.append("recovery record missing (fail closed)") + return result + if (recovery.get("record_type") or recovery.get("kind")) not in ( + RECORD_TYPE, + KIND_RECOVERY, + "irrecoverable_decision_provenance", + ): + if recovery.get("status") != "provenance_irrecoverable": + reasons.append("recovery record type/status invalid (fail closed)") + + if recovery.get("applied") is True: + reasons.append( + "recovery record claims applied=true; refuse (fabrication, fail closed)" + ) + if recovery.get("historical_cleanup_proven") is True: + reasons.append( + "recovery record claims historical_cleanup_proven=true; refuse " + "(fail closed)" + ) + + for field, want in (("remote", remote), ("org", org), ("repo", repo)): + have = (str(recovery.get(field) or "")).strip() + if have != (want or "").strip(): + reasons.append( + f"recovery {field} mismatch (stored={have!r}, expected={want!r})" + ) + + try: + if int(recovery.get("pr_number") or recovery.get("blocked_pr_number")) != int( + pr_number + ): + reasons.append("recovery PR number mismatch (fail closed)") + except (TypeError, ValueError): + reasons.append("recovery missing pr_number (fail closed)") + + if not heads_equal(recovery.get("head_sha"), live_head_sha): + reasons.append( + "recovery head SHA does not match live PR head (fail closed)" + ) + + if recovery.get("consumption_state") == "consumed" or recovery.get("consumed_at"): + # Idempotent: already consumed for this exact scope is OK if head matches. + result["recovery_record_consumed"] = True + reasons.append("recovery record already consumed (idempotent check)") + + auth_check = verify_authorization_artifact( + authorization, + remote=remote, + org=org, + repo=repo, + pr_number=pr_number, + expected_head_sha=str(live_head_sha or ""), + incident_issue=recovery.get("incident_issue"), + incident_comment_id=recovery.get("incident_comment_id"), + # When recovery already consumed, allow already-consumed auth for + # idempotent re-report; otherwise require unconsumed. + require_unconsumed=not result["recovery_record_consumed"], + now=now, + ) + if not auth_check.get("valid"): + reasons.extend(auth_check.get("reasons") or ["authorization invalid"]) + else: + result["irrecoverable_recovery_authorized"] = True + + if not recovery.get("merger_may_accept") and not result["recovery_record_consumed"]: + reasons.append( + "recovery record merger_may_accept is false (fail closed)" + ) + + # Auth id binding. + if ( + authorization + and recovery.get("authorization_id") + and authorization.get("authorization_id") + and recovery.get("authorization_id") != authorization.get("authorization_id") + ): + reasons.append("recovery authorization_id does not match artifact (fail closed)") + + # Normal gates: if explicitly False, refuse consumption as merge-authorizing. + normal_blockers: list[str] = [] + if approval_at_current_head is False: + normal_blockers.append("missing/stale approval at current head") + if has_blocking_change_requests is True: + normal_blockers.append("blocking change requests present") + if mergeable is False: + normal_blockers.append("PR not mergeable") + if lease_ok is False: + normal_blockers.append("lease gate failed") + if runtime_ok is False: + normal_blockers.append("runtime gate failed") + if workspace_ok is False: + normal_blockers.append("workspace gate failed") + if anti_stomp_ok is False: + normal_blockers.append("anti-stomp gate failed") + if normal_blockers: + reasons.append( + "recovery cannot bypass normal merge gates: " + + "; ".join(normal_blockers) + + " (#709 F2)" + ) + result["resolves_prior_provenance_blocker"] = False + result["allowed"] = False + result["reasons"] = reasons + return result + + # Filter pure informational "already consumed" when everything else matches + # for idempotent success. + hard = [ + r + for r in reasons + if "already consumed" not in r + ] + if not hard and result["irrecoverable_recovery_authorized"]: + result["allowed"] = True + result["resolves_prior_provenance_blocker"] = True + result["historical_cleanup_proven"] = False + if result["recovery_record_consumed"]: + reasons.append( + "idempotent: prior-provenance blocker already resolved for this scope" + ) + else: + reasons.append( + "prior-provenance blocker may be resolved by consuming this record " + "(historical cleanup remains unproven)" + ) + result["reasons"] = reasons + return result + + +def mark_consumed( + record: dict[str, Any], + *, + consumer_username: str | None, + consumer_profile: str | None, +) -> dict[str, Any]: + """Return a copy of *record* marked consumed (crash-safe write is caller's job).""" + out = dict(record) + out["consumption_state"] = "consumed" + out["status"] = out.get("status") or "issued" + if out.get("kind") == KIND_AUTH or out.get("auth_type") == AUTH_TYPE: + out["status"] = "consumed" + out["consumed_at"] = _now_iso() + out["consumed_by"] = consumer_username + out["consumed_by_profile"] = consumer_profile + out["updated_at"] = out["consumed_at"] + return out + + +def assess_profile_path_identity(profile_identity: str | None) -> dict[str, Any]: + """Reject traversal / malformed profile identity segments (#709 F3).""" + reasons: list[str] = [] + raw = profile_identity if profile_identity is not None else "" + text = str(raw) + if not text.strip(): + reasons.append("profile identity empty (fail closed)") + return {"valid": False, "reasons": reasons, "sanitized": None} + if text != text.strip(): + reasons.append("profile identity has surrounding whitespace (fail closed)") + if ".." in text or "/" in text or "\\" in text or "\x00" in text: + reasons.append( + "profile identity contains path traversal or separator characters " + "(fail closed, #709 F3)" + ) + if text.startswith("-") or text.startswith("."): + reasons.append("profile identity has unsafe leading character (fail closed)") + # After sanitize, must not collapse to something that collides emptily. + sanitized = mcp_session_state._sanitize_segment(text) + if sanitized in ("_", ""): + reasons.append("profile identity sanitizes to empty (fail closed)") + if sanitized != text and any(c in text for c in ("..", "/", "\\")): + # Already covered; keep fail closed. + pass + return { + "valid": not reasons, + "reasons": reasons, + "sanitized": sanitized if not reasons else None, + } diff --git a/mcp_session_state.py b/mcp_session_state.py index 4ae119a..e6941a7 100644 --- a/mcp_session_state.py +++ b/mcp_session_state.py @@ -42,6 +42,8 @@ KIND_DECISION_LOCK_ARCHIVE = "review_decision_lock_archive" KIND_POST_MERGE_DECISION_RECOVERY = "post_merge_decision_recovery" # #709: truthful record when historical terminal evidence is irrecoverably gone. KIND_IRRECOVERABLE_DECISION_PROVENANCE = "irrecoverable_decision_provenance" +# #709 F1: server-side non-forgeable authorization artifact for recovery. +KIND_IRRECOVERABLE_PROVENANCE_AUTH = "irrecoverable_provenance_authorization" # Kinds that must survive the default session-state TTL (forensic / recovery). RECOVERY_CRITICAL_KINDS = frozenset( @@ -49,6 +51,7 @@ RECOVERY_CRITICAL_KINDS = frozenset( KIND_DECISION_LOCK_ARCHIVE, KIND_POST_MERGE_DECISION_RECOVERY, KIND_IRRECOVERABLE_DECISION_PROVENANCE, + KIND_IRRECOVERABLE_PROVENANCE_AUTH, } ) @@ -504,6 +507,7 @@ def load_state_for_profile( repo: str | None = None, state_dir: str | None = None, skip_identity_match: bool = False, + enforce_repo_scope: bool = True, ) -> dict[str, Any] | None: """Load durable state for an explicit profile identity (#709 cross-profile). @@ -511,9 +515,46 @@ def load_state_for_profile( profile_identity to equal the requested profile (anti-stomp), but does not require the *active* session identity to match — needed so a merger can inspect a reviewer lock after merge. + + #709 F3: remote/org/repo filter reasons are **enforced** (not merely + computed). When *enforce_repo_scope* is True (default) and the caller + supplies remote/org/repo, mismatches fail closed with ``None``. """ + # #709 F3: refuse traversal / malformed profile identities. + try: + from irrecoverable_provenance import assess_profile_path_identity + + path_gate = assess_profile_path_identity(profile_identity) + if not path_gate.get("valid"): + return None + except Exception: + # Module may be mid-import in edge bootstraps; fall through to + # conservative checks below. + raw = str(profile_identity or "") + if ".." in raw or "/" in raw or "\\" in raw or "\x00" in raw: + return None + profile = current_profile_identity(profile_identity=profile_identity) root = _ensure_state_dir(state_dir) + # Refuse symlink escape of the state root (#709 F3). + try: + real_root = os.path.realpath(root) + path = state_file_path( + kind=kind, + remote=remote, + org=org, + repo=repo, + profile_identity=profile, + state_dir=root, + ) + real_path = os.path.realpath(path) if os.path.exists(path) else path + if os.path.exists(path) and not str(real_path).startswith( + str(real_root) + os.sep + ) and str(real_path) != str(real_root): + return None + except OSError: + return None + path = state_file_path( kind=kind, remote=remote, @@ -548,34 +589,46 @@ def load_state_for_profile( if stored and stored != profile: return None if skip_identity_match: - # Still enforce TTL / future-dated so dead records do not authorize cleanup. + # Enforce remote/org/repo when caller provides them (#709 F3). + # Drop only *active session* profile-identity mismatches; keep + # expiry, spoof, and repository-scope reasons. + scope_remote = remote if enforce_repo_scope else None + scope_org = org if enforce_repo_scope else None + scope_repo = repo if enforce_repo_scope else None reasons = identity_match_reasons( merged, - remote=remote or merged.get("remote"), - org=org or merged.get("org"), - repo=repo or merged.get("repo"), + remote=scope_remote, + org=scope_org, + repo=scope_repo, profile_identity=stored or profile, ) - # Drop active-session-only mismatches; keep expiry / spoof reasons. filtered = [ r for r in reasons if "profile identity mismatch" not in r or (stored and stored != profile) ] - # Re-run only expiry/future/missing checks via identity when profile matches - expiry_reasons = [ - r - for r in identity_match_reasons( - merged, - remote=None, - org=None, - repo=None, - profile_identity=stored or profile, - ) - if any(x in r for x in ("expired", "future", "missing recorded_at")) - ] - if expiry_reasons: + # When caller requested a specific remote/org/repo, also fail if the + # stored record lacks those identity fields entirely (legacy incomplete). + if enforce_repo_scope: + for field, want in ( + ("remote", remote), + ("org", org), + ("repo", repo), + ): + want_s = (want or "").strip() + if not want_s: + continue + have = (str(merged.get(field) or "")).strip() + if not have: + filtered.append( + f"session state {field} missing on durable record " + f"(expected={want_s!r}; fail closed, #709 F3)" + ) + elif have != want_s: + # identity_match_reasons already adds mismatch; ensure kept + pass + if filtered: return None return merged reasons = identity_match_reasons( diff --git a/stale_review_decision_lock.py b/stale_review_decision_lock.py index c5252c3..3eee525 100644 --- a/stale_review_decision_lock.py +++ b/stale_review_decision_lock.py @@ -570,13 +570,52 @@ def build_irrecoverable_provenance_record( actor_username: str | None, profile_name: str | None, reason: str, - incident_ref: str | None, - operator_authorized: bool, + incident_ref: str | None = None, + # Deprecated kwargs retained only so stale call sites fail closed: + operator_authorized: bool | None = None, + # Required for merger-acceptable records (#709 F1): + authorization: dict[str, Any] | None = None, + incident_issue: int | None = None, + incident_comment_id: int | None = None, + destroyed_subject: str | None = None, + historical_provenance_subject: str | None = None, ) -> dict[str, Any]: - """Truthful absence-of-proof record (#709 AC5). Never sets applied=True.""" + """Truthful absence-of-proof record (#709 AC5). Never sets applied=True. + + Caller-supplied ``operator_authorized`` is **ignored** as authorization + evidence (review 434 F1). Prefer + :func:`irrecoverable_provenance.build_irrecoverable_provenance_record` + with a verified server-side authorization artifact. + """ + # Explicitly ignore deprecated self-assertable Boolean. + _ = operator_authorized + if authorization is not None and incident_issue is not None and incident_comment_id is not None: + from irrecoverable_provenance import ( + build_irrecoverable_provenance_record as _build, + ) + + return _build( + pr_number=int(pr_number), + head_sha=str(head_sha or ""), + remote=str(remote or ""), + org=str(org or ""), + repo=str(repo or ""), + actor_username=actor_username, + profile_name=profile_name, + reason=reason, + incident_issue=int(incident_issue), + incident_comment_id=int(incident_comment_id), + authorization=authorization, + destroyed_subject=destroyed_subject, + historical_provenance_subject=historical_provenance_subject + or destroyed_subject, + ) + # Fail-closed skeleton when no server authorization is supplied: never + # sets merger_may_accept True (even if operator_authorized was True). return { "event": "irrecoverable_decision_lock_provenance", "status": "provenance_irrecoverable", + "record_type": "irrecoverable_decision_provenance", "operator_recovery_required": True, "issue_ref": "#709", "recovery_critical": True, @@ -592,40 +631,27 @@ def build_irrecoverable_provenance_record( "profile_name": profile_name, "reason": reason, "incident_ref": incident_ref, - "operator_authorized": bool(operator_authorized), - "merger_may_accept": bool(operator_authorized), + "incident_issue": incident_issue, + "incident_comment_id": incident_comment_id, + "authorization_verified": False, + "merger_may_accept": False, "acceptance_rule": ( - "Merger may accept this record only when operator_authorized=true, " - "repository/PR/head match the live target, the record is durable " - "and read back, and no conflicting terminal lock remains for a " - "different PR/head. This does not prove historical cleanup." + "Merger may accept this record only when a server-side " + "authorization artifact verifies for remote/org/repo/PR/exact " + "head/incident, the record is durable and read back, and normal " + "merge gates still pass. Caller Booleans never authorize. This " + "does not prove historical cleanup." ), } def format_irrecoverable_audit_comment(record: dict[str, Any]) -> str: """Markdown body for irrecoverable provenance audit (no applied=true claim).""" - lines = [ - "## Irrecoverable decision-lock provenance (#709)", - "", - "Status: **PROVENANCE_IRRECOVERABLE** (not applied cleanup)", - "", - f"- actor: `{record.get('actor_username')}`", - f"- profile: `{record.get('profile_name')}`", - f"- timestamp: `{record.get('timestamp')}`", - f"- PR: `#{record.get('pr_number')}`", - f"- head_sha: `{record.get('head_sha')}`", - f"- incident_ref: `{record.get('incident_ref')}`", - f"- operator_authorized: `{record.get('operator_authorized')}`", - f"- historical_cleanup_proven: `{record.get('historical_cleanup_proven')}`", - f"- applied: `{record.get('applied')}` (must remain false)", - "", - f"Reason: {record.get('reason')}", - "", - "This record documents **absence of proof**, not successful cleanup.", - "It must not be reused for a different PR or head (#709 AC6).", - ] - return "\n".join(lines) + from irrecoverable_provenance import ( + format_irrecoverable_audit_comment as _fmt, + ) + + return _fmt(record) def format_post_merge_recovery_comment(record: dict[str, Any]) -> str: diff --git a/task_capability_map.py b/task_capability_map.py index f41475d..9b7e70a 100644 --- a/task_capability_map.py +++ b/task_capability_map.py @@ -127,15 +127,32 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "permission": "gitea.pr.review", "role": "reviewer", }, - # #709: truthful absence-of-proof recovery record (not applied cleanup). + # #709: truthful absence-of-proof recovery (server-side auth + record + consume). + # Dedicated mutation capability — gitea.read is insufficient (review 434 F1). + "issue_irrecoverable_provenance_authorization": { + "permission": "gitea.decision_lock.irrecoverable_recovery", + "role": "reconciler", + }, + "gitea_issue_irrecoverable_provenance_authorization": { + "permission": "gitea.decision_lock.irrecoverable_recovery", + "role": "reconciler", + }, "record_irrecoverable_decision_lock_provenance": { - "permission": "gitea.issue.comment", + "permission": "gitea.decision_lock.irrecoverable_recovery", "role": "reconciler", }, "gitea_record_irrecoverable_decision_lock_provenance": { - "permission": "gitea.issue.comment", + "permission": "gitea.decision_lock.irrecoverable_recovery", "role": "reconciler", }, + "consume_irrecoverable_decision_lock_provenance": { + "permission": "gitea.pr.merge", + "role": "merger", + }, + "gitea_consume_irrecoverable_decision_lock_provenance": { + "permission": "gitea.pr.merge", + "role": "merger", + }, "delete_branch": { "permission": "gitea.branch.delete", "role": "author", diff --git a/tests/test_issue_709_decision_lock_cross_profile.py b/tests/test_issue_709_decision_lock_cross_profile.py index 6e9d21a..8aae98e 100644 --- a/tests/test_issue_709_decision_lock_cross_profile.py +++ b/tests/test_issue_709_decision_lock_cross_profile.py @@ -1,16 +1,19 @@ """#709: cross-profile decision-lock cleanup, overwrite protection, recovery. -Covers AC1–AC8 regression scenarios without fabricating historical PR #696 -provenance or special-casing live PR numbers in production code. +Covers AC1–AC8 plus review-434 F1/F2/F3 remediations without fabricating +historical PR provenance or special-casing live PR numbers in production code. """ from __future__ import annotations import os +import subprocess +import sys import tempfile import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import patch +import irrecoverable_provenance as irp import mcp_session_state as ss import stale_review_decision_lock as srdl @@ -20,6 +23,8 @@ def _lock( *, profile="prgs-reviewer", remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", head=None, ): muts = [] @@ -31,8 +36,8 @@ def _lock( return { "task": "review_pr", "remote": remote, - "org": "Scaled-Tech-Consulting", - "repo": "Gitea-Tools", + "org": org, + "repo": repo, "session_pid": os.getpid(), "session_profile": profile, "session_profile_lock": profile, @@ -52,6 +57,47 @@ APPROVE = {"pr_number": 100, "action": "approve", "review_id": 9} APPROVE_OTHER = {"pr_number": 200, "action": "approve", "review_id": 10} HEAD_A = "a" * 40 HEAD_B = "b" * 40 +RECONCILER_OPS = [ + "gitea.read", + "gitea.pr.close", + "gitea.pr.comment", + "gitea.issue.comment", +] + + +def _mint_auth( + *, + pr_number=42, + head=HEAD_A, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + incident_issue=700, + incident_comment_id=11489, + issuer="sysadmin", + profile="prgs-reconciler", + destroyed_subject=None, +): + return irp.build_authorization_artifact( + remote=remote, + org=org, + repo=repo, + pr_number=pr_number, + expected_head_sha=head, + incident_issue=incident_issue, + incident_comment_id=incident_comment_id, + destroyed_subject=destroyed_subject, + issuer_username=issuer, + issuer_profile=profile, + native_provenance={ + "native_mcp_transport": True, + "production_native_mcp_transport": False, + "pytest": True, + "token_fingerprint": "testfp", + "entrypoint": "pytest", + "pid": os.getpid(), + }, + ) class TestAC2InitOverwrite(unittest.TestCase): @@ -97,8 +143,8 @@ class TestAC1TargetApproval(unittest.TestCase): ) -class TestAC5IrrecoverableRecord(unittest.TestCase): - def test_never_sets_applied_true(self): +class TestF1AuthorizationNotSelfAssertable(unittest.TestCase): + def test_operator_authorized_true_cannot_authorize_via_build(self): rec = srdl.build_irrecoverable_provenance_record( pr_number=42, head_sha=HEAD_A, @@ -108,32 +154,560 @@ class TestAC5IrrecoverableRecord(unittest.TestCase): actor_username="sysadmin", profile_name="prgs-reconciler", reason="evidence destroyed", - incident_ref="#700 comment 1", + incident_ref="anything", operator_authorized=True, ) self.assertFalse(rec["applied"]) self.assertFalse(rec["historical_cleanup_proven"]) - self.assertEqual(rec["status"], "provenance_irrecoverable") - self.assertTrue(rec["merger_may_accept"]) - body = srdl.format_irrecoverable_audit_comment(rec) - self.assertIn("applied: `False`", body) - self.assertIn("must remain false", body) + self.assertFalse(rec["merger_may_accept"]) - def test_unauthorized_not_merger_acceptable(self): + def test_confirmation_string_not_authorization(self): + # Confirmation is only intent text; capability assess ignores it. + conf = irp.expected_confirmation(99) + self.assertEqual(conf, "IRRECOVERABLE DECISION PROVENANCE PR 99") + # Without auth artifact, merger cannot accept. rec = srdl.build_irrecoverable_provenance_record( - pr_number=42, + pr_number=99, head_sha=HEAD_A, remote="prgs", - org=None, - repo=None, + org="o", + repo="r", actor_username="x", profile_name="y", reason="r", - incident_ref=None, operator_authorized=False, ) self.assertFalse(rec["merger_may_accept"]) + def test_gitea_read_alone_insufficient(self): + a = irp.assess_capability_for_irrecoverable_recovery( + allowed_operations=["gitea.read"], + forbidden_operations=[], + role_kind="author", + profile_name="prgs-author", + ) + self.assertFalse(a["allowed"]) + + def test_expected_head_missing_fails(self): + g = irp.assess_live_head_binding( + expected_head_sha=None, + live_head_sha=HEAD_A, + ) + self.assertFalse(g["valid"]) + + def test_expected_head_differs_fails(self): + g = irp.assess_live_head_binding( + expected_head_sha=HEAD_A, + live_head_sha=HEAD_B, + ) + self.assertFalse(g["valid"]) + + def test_missing_incident_fails(self): + g = irp.assess_incident_evidence( + incident_issue=None, + incident_comment_id=None, + comment_payload=None, + ) + self.assertFalse(g["valid"]) + + def test_nonexistent_incident_fails(self): + g = irp.assess_incident_evidence( + incident_issue=1, + incident_comment_id=2, + comment_payload=None, + comment_lookup_error="404", + ) + self.assertFalse(g["valid"]) + + def test_valid_auth_succeeds_exact_scope(self): + auth = _mint_auth() + v = irp.verify_authorization_artifact( + auth, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + pr_number=42, + expected_head_sha=HEAD_A, + incident_issue=700, + incident_comment_id=11489, + ) + self.assertTrue(v["valid"], v) + rec = irp.build_irrecoverable_provenance_record( + pr_number=42, + head_sha=HEAD_A, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + actor_username="sysadmin", + profile_name="prgs-reconciler", + reason="evidence destroyed", + incident_issue=700, + incident_comment_id=11489, + authorization=auth, + ) + self.assertTrue(rec["merger_may_accept"]) + self.assertFalse(rec["applied"]) + body = irp.format_irrecoverable_audit_comment(rec) + self.assertIn("applied: `False`", body) + + def test_wrong_repo_auth_fails(self): + auth = _mint_auth(repo="Other-Repo") + v = irp.verify_authorization_artifact( + auth, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + pr_number=42, + expected_head_sha=HEAD_A, + incident_issue=700, + incident_comment_id=11489, + ) + self.assertFalse(v["valid"]) + + def test_wrong_pr_auth_fails(self): + auth = _mint_auth(pr_number=1) + v = irp.verify_authorization_artifact( + auth, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + pr_number=42, + expected_head_sha=HEAD_A, + ) + self.assertFalse(v["valid"]) + + def test_wrong_head_auth_fails(self): + auth = _mint_auth(head=HEAD_B) + v = irp.verify_authorization_artifact( + auth, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + pr_number=42, + expected_head_sha=HEAD_A, + ) + self.assertFalse(v["valid"]) + + def test_altered_signature_fails(self): + auth = _mint_auth() + auth["server_signature"] = "0" * 64 + v = irp.verify_authorization_artifact( + auth, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + pr_number=42, + expected_head_sha=HEAD_A, + incident_issue=700, + incident_comment_id=11489, + ) + self.assertFalse(v["valid"]) + + def test_fresh_non_pytest_process_cannot_mint_accepted_record(self): + """Ordinary Python process: merger_may_accept stays False without server auth.""" + script = ( + "import stale_review_decision_lock as s\n" + "r=s.build_irrecoverable_provenance_record(\n" + " pr_number=1, head_sha=None, remote='prgs', org=None, repo=None,\n" + " actor_username='x', profile_name='y', reason='r',\n" + " incident_ref=None, operator_authorized=True)\n" + "print(r.get('merger_may_accept'), r.get('head_sha'))\n" + ) + env = {k: v for k, v in os.environ.items() if not k.startswith("PYTEST")} + env.pop("PYTEST_CURRENT_TEST", 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("False"), msg=out) + + def test_unauthorized_profile_capability(self): + a = irp.assess_capability_for_irrecoverable_recovery( + allowed_operations=["gitea.read", "gitea.pr.comment"], + forbidden_operations=["gitea.pr.close"], + role_kind="author", + profile_name="prgs-author", + ) + self.assertFalse(a["allowed"]) + + def test_reconciler_capability_allowed(self): + a = irp.assess_capability_for_irrecoverable_recovery( + allowed_operations=RECONCILER_OPS, + forbidden_operations=[ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.review", + ], + role_kind="reconciler", + profile_name="prgs-reconciler", + ) + self.assertTrue(a["allowed"], a) + + +class TestF2MergerConsumer(unittest.TestCase): + def test_merger_rejects_without_valid_auth(self): + rec = srdl.build_irrecoverable_provenance_record( + pr_number=10, + head_sha=HEAD_A, + remote="prgs", + org="o", + repo="r", + actor_username="a", + profile_name="p", + reason="x", + operator_authorized=True, + ) + a = irp.assess_merger_consumption( + rec, + None, + remote="prgs", + org="o", + repo="r", + pr_number=10, + live_head_sha=HEAD_A, + approval_at_current_head=True, + has_blocking_change_requests=False, + ) + self.assertFalse(a["allowed"]) + + def test_merger_rejects_wrong_head(self): + auth = _mint_auth(pr_number=10, head=HEAD_A, org="o", repo="r") + rec = irp.build_irrecoverable_provenance_record( + pr_number=10, + head_sha=HEAD_A, + remote="prgs", + org="o", + repo="r", + actor_username="a", + profile_name="p", + reason="x", + incident_issue=700, + incident_comment_id=1, + authorization=auth, + ) + a = irp.assess_merger_consumption( + rec, + auth, + remote="prgs", + org="o", + repo="r", + pr_number=10, + live_head_sha=HEAD_B, + approval_at_current_head=True, + has_blocking_change_requests=False, + ) + self.assertFalse(a["allowed"]) + + def test_merger_rejects_replayed_auth(self): + auth = _mint_auth(pr_number=10, head=HEAD_A, org="o", repo="r", incident_comment_id=1) + consumed = irp.mark_consumed(auth, consumer_username="m", consumer_profile="merger") + rec = irp.build_irrecoverable_provenance_record( + pr_number=10, + head_sha=HEAD_A, + remote="prgs", + org="o", + repo="r", + actor_username="a", + profile_name="p", + reason="x", + incident_issue=700, + incident_comment_id=1, + authorization=auth, # original unconsumed for record build + ) + a = irp.assess_merger_consumption( + rec, + consumed, + remote="prgs", + org="o", + repo="r", + pr_number=10, + live_head_sha=HEAD_A, + approval_at_current_head=True, + has_blocking_change_requests=False, + ) + self.assertFalse(a["allowed"]) + + def test_recovery_cannot_bypass_missing_approval(self): + auth = _mint_auth(pr_number=10, head=HEAD_A, org="o", repo="r", incident_comment_id=1) + rec = irp.build_irrecoverable_provenance_record( + pr_number=10, + head_sha=HEAD_A, + remote="prgs", + org="o", + repo="r", + actor_username="a", + profile_name="p", + reason="x", + incident_issue=700, + incident_comment_id=1, + authorization=auth, + ) + a = irp.assess_merger_consumption( + rec, + auth, + remote="prgs", + org="o", + repo="r", + pr_number=10, + live_head_sha=HEAD_A, + approval_at_current_head=False, + has_blocking_change_requests=False, + ) + self.assertFalse(a["allowed"]) + self.assertTrue(any("approval" in r for r in a["reasons"])) + + def test_recovery_cannot_bypass_blocking_crs(self): + auth = _mint_auth(pr_number=10, head=HEAD_A, org="o", repo="r", incident_comment_id=1) + rec = irp.build_irrecoverable_provenance_record( + pr_number=10, + head_sha=HEAD_A, + remote="prgs", + org="o", + repo="r", + actor_username="a", + profile_name="p", + reason="x", + incident_issue=700, + incident_comment_id=1, + authorization=auth, + ) + a = irp.assess_merger_consumption( + rec, + auth, + remote="prgs", + org="o", + repo="r", + pr_number=10, + live_head_sha=HEAD_A, + approval_at_current_head=True, + has_blocking_change_requests=True, + ) + self.assertFalse(a["allowed"]) + + def test_valid_recovery_resolves_only_prior_provenance(self): + auth = _mint_auth(pr_number=10, head=HEAD_A, org="o", repo="r", incident_comment_id=1) + rec = irp.build_irrecoverable_provenance_record( + pr_number=10, + head_sha=HEAD_A, + remote="prgs", + org="o", + repo="r", + actor_username="a", + profile_name="p", + reason="x", + incident_issue=700, + incident_comment_id=1, + authorization=auth, + ) + a = irp.assess_merger_consumption( + rec, + auth, + remote="prgs", + org="o", + repo="r", + pr_number=10, + live_head_sha=HEAD_A, + approval_at_current_head=True, + has_blocking_change_requests=False, + mergeable=True, + lease_ok=True, + runtime_ok=True, + workspace_ok=True, + anti_stomp_ok=True, + ) + self.assertTrue(a["allowed"], a) + self.assertTrue(a["resolves_prior_provenance_blocker"]) + self.assertFalse(a["historical_cleanup_proven"]) + + def test_duplicate_consume_idempotent(self): + auth = _mint_auth(pr_number=10, head=HEAD_A, org="o", repo="r", incident_comment_id=1) + rec = irp.build_irrecoverable_provenance_record( + pr_number=10, + head_sha=HEAD_A, + remote="prgs", + org="o", + repo="r", + actor_username="a", + profile_name="p", + reason="x", + incident_issue=700, + incident_comment_id=1, + authorization=auth, + ) + consumed_auth = irp.mark_consumed(auth, consumer_username="m", consumer_profile="mer") + consumed_rec = irp.mark_consumed(rec, consumer_username="m", consumer_profile="mer") + a = irp.assess_merger_consumption( + consumed_rec, + consumed_auth, + remote="prgs", + org="o", + repo="r", + pr_number=10, + live_head_sha=HEAD_A, + approval_at_current_head=True, + has_blocking_change_requests=False, + ) + self.assertTrue(a["allowed"], a) + self.assertTrue(a["recovery_record_consumed"]) + + +class TestF3ExactScopeEnforcement(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.state_dir = self._tmp.name + os.chmod(self.state_dir, 0o700) + + def tearDown(self): + self._tmp.cleanup() + + def test_same_pr_other_remote_not_loaded(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([APPROVE], profile="prgs-reviewer", remote="other"), + remote="other", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + loaded = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNone(loaded) + + def test_same_pr_other_org_not_loaded(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock( + [APPROVE], + profile="prgs-reviewer", + org="Other-Org", + ), + remote="prgs", + org="Other-Org", + repo="Gitea-Tools", + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + loaded = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNone(loaded) + + def test_same_pr_other_repo_not_loaded(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock( + [APPROVE], + profile="prgs-reviewer", + repo="Other-Repo", + ), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Other-Repo", + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + loaded = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNone(loaded) + + def test_path_traversal_profile_fails(self): + g = irp.assess_profile_path_identity("../evil") + self.assertFalse(g["valid"]) + loaded = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="../evil", + remote="prgs", + org="o", + repo="r", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNone(loaded) + + def test_malformed_legacy_missing_repo_fails_closed(self): + # Record without repo identity when caller requires repo. + payload = _lock([APPROVE], profile="prgs-reviewer") + del payload["repo"] + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=payload, + remote="prgs", + org="Scaled-Tech-Consulting", + repo=None, + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + loaded = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNone(loaded) + + def test_list_and_load_foreign_profile_lock(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([APPROVE], profile="prgs-reviewer"), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity="prgs-reviewer", + state_dir=self.state_dir, + ) + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([], profile="prgs-merger"), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity="prgs-merger", + state_dir=self.state_dir, + ) + foreign = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + state_dir=self.state_dir, + skip_identity_match=True, + ) + self.assertIsNotNone(foreign) + self.assertTrue( + srdl.lock_targets_merged_pr_approval(foreign, pr_number=100) + ) + class TestAC3PostMergeRecoveryRecord(unittest.TestCase): def test_recovery_record_is_not_applied_cleanup(self): @@ -155,114 +729,23 @@ class TestAC3PostMergeRecoveryRecord(unittest.TestCase): self.assertTrue(rec["recovery_critical"]) -class TestSessionStateCrossProfile(unittest.TestCase): - def setUp(self): - self._tmp = tempfile.TemporaryDirectory() - self.state_dir = self._tmp.name - os.chmod(self.state_dir, 0o700) - - def tearDown(self): - self._tmp.cleanup() - - def test_list_and_load_foreign_profile_lock(self): - ss.save_state( - kind=ss.KIND_DECISION_LOCK, - payload=_lock([APPROVE], profile="prgs-reviewer"), - remote="prgs", - org="Scaled-Tech-Consulting", - repo="Gitea-Tools", - profile_identity="prgs-reviewer", - state_dir=self.state_dir, - ) - # Merger-local empty lock - ss.save_state( - kind=ss.KIND_DECISION_LOCK, - payload=_lock([], profile="prgs-merger"), - remote="prgs", - org="Scaled-Tech-Consulting", - repo="Gitea-Tools", - profile_identity="prgs-merger", - state_dir=self.state_dir, - ) - ids = ss.list_decision_lock_profile_identities(state_dir=self.state_dir) - self.assertIn("prgs-reviewer", ids) - self.assertIn("prgs-merger", ids) - - foreign = ss.load_state_for_profile( - kind=ss.KIND_DECISION_LOCK, - profile_identity="prgs-reviewer", - remote="prgs", - org="Scaled-Tech-Consulting", - repo="Gitea-Tools", - state_dir=self.state_dir, - skip_identity_match=True, - ) - self.assertIsNotNone(foreign) - self.assertTrue( - srdl.lock_targets_merged_pr_approval(foreign, pr_number=100) - ) - empty = ss.load_state_for_profile( - kind=ss.KIND_DECISION_LOCK, - profile_identity="prgs-merger", - state_dir=self.state_dir, - skip_identity_match=True, - ) - self.assertIsNotNone(empty) - self.assertFalse( - srdl.lock_targets_merged_pr_approval(empty, pr_number=100) - ) - - def test_clear_reviewer_not_merger_empty(self): - ss.save_state( - kind=ss.KIND_DECISION_LOCK, - payload=_lock([APPROVE], profile="prgs-reviewer"), - profile_identity="prgs-reviewer", - state_dir=self.state_dir, - ) - ss.save_state( - kind=ss.KIND_DECISION_LOCK, - payload=_lock([], profile="prgs-merger"), - profile_identity="prgs-merger", - state_dir=self.state_dir, - ) - ss.clear_state( - kind=ss.KIND_DECISION_LOCK, - profile_identity="prgs-reviewer", - state_dir=self.state_dir, - ) - self.assertIsNone( - ss.load_state_for_profile( - kind=ss.KIND_DECISION_LOCK, - profile_identity="prgs-reviewer", - state_dir=self.state_dir, - skip_identity_match=True, - ) - ) - # Merger empty lock remains - self.assertIsNotNone( - ss.load_state_for_profile( - kind=ss.KIND_DECISION_LOCK, - profile_identity="prgs-merger", - state_dir=self.state_dir, - skip_identity_match=True, - ) - ) - +class TestSessionStateTTL(unittest.TestCase): def test_recovery_critical_kinds_ttl_exempt(self): - rec = srdl.build_irrecoverable_provenance_record( + auth = _mint_auth(pr_number=1) + rec = irp.build_irrecoverable_provenance_record( pr_number=1, head_sha=HEAD_A, remote="prgs", - org=None, - repo=None, + org="o", + repo="r", actor_username="a", profile_name="p", reason="gone", - incident_ref=None, - operator_authorized=True, + incident_issue=700, + incident_comment_id=1, + authorization=auth, ) rec["kind"] = ss.KIND_IRRECOVERABLE_DECISION_PROVENANCE - # Force old recorded_at rec["recorded_at"] = "2000-01-01T00:00:00Z" rec["updated_at"] = rec["recorded_at"] rec["profile_identity"] = "prgs-reconciler" @@ -270,10 +753,7 @@ class TestSessionStateCrossProfile(unittest.TestCase): reasons = ss.identity_match_reasons( rec, profile_identity="prgs-reconciler" ) - self.assertFalse( - any("expired" in r for r in reasons), - msg=reasons, - ) + self.assertFalse(any("expired" in r for r in reasons), msg=reasons) class TestInitReviewDecisionLockIntegration(unittest.TestCase): @@ -300,8 +780,9 @@ class TestInitReviewDecisionLockIntegration(unittest.TestCase): self._tmp.cleanup() def test_init_does_not_wipe_terminal_ledger(self): - self.mcp._save_review_decision_lock(_lock([APPROVE], profile="prgs-reviewer")) - # force=True would previously wipe + self.mcp._save_review_decision_lock( + _lock([APPROVE], profile="prgs-reviewer") + ) self.mcp.init_review_decision_lock("prgs", "review_pr", force=True) loaded = self.mcp._load_review_decision_lock() self.assertIsNotNone(loaded) @@ -317,7 +798,7 @@ class TestInitReviewDecisionLockIntegration(unittest.TestCase): self.assertEqual(loaded.get("live_mutations"), []) -class TestIrrecoverableTool(unittest.TestCase): +class TestIrrecoverableToolF1(unittest.TestCase): def setUp(self): self._tmp = tempfile.TemporaryDirectory() self.env = patch.dict( @@ -326,7 +807,7 @@ class TestIrrecoverableTool(unittest.TestCase): "GITEA_MCP_SESSION_STATE_DIR": self._tmp.name, "GITEA_SESSION_PROFILE_LOCK": "prgs-reconciler", "GITEA_PROFILE_NAME": "prgs-reconciler", - "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment,gitea.pr.comment", + "GITEA_ALLOWED_OPERATIONS": ",".join(RECONCILER_OPS), }, clear=False, ) @@ -339,132 +820,187 @@ class TestIrrecoverableTool(unittest.TestCase): self.env.stop() self._tmp.cleanup() - def test_requires_confirmation_and_operator(self): - with patch.object( - self.mcp, - "get_profile", - return_value={ - "profile_name": "prgs-reconciler", - "allowed_operations": [ - "gitea.read", - "gitea.issue.comment", - "gitea.pr.comment", - ], - "forbidden_operations": [], - }, - ): + def _profile(self): + return { + "profile_name": "prgs-reconciler", + "role": "reconciler", + "allowed_operations": RECONCILER_OPS, + "forbidden_operations": [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.review", + ], + } + + def test_operator_authorized_true_rejected(self): + with patch.object(self.mcp, "get_profile", return_value=self._profile()): r = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( pr_number=50, reason="lost", - confirmation="", - operator_authorized=False, + confirmation=irp.expected_confirmation(50), + operator_authorized=True, + expected_head_sha=HEAD_A, + incident_issue=700, + incident_comment_id=11489, remote="prgs", org="Scaled-Tech-Consulting", repo="Gitea-Tools", post_audit_comment=False, ) self.assertFalse(r["success"]) - self.assertFalse(r["applied"]) + self.assertTrue(any("operator_authorized" in x for x in r["reasons"])) - def test_records_without_applied_true(self): - with patch.object( - self.mcp, - "get_profile", - return_value={ - "profile_name": "prgs-reconciler", - "allowed_operations": [ - "gitea.read", - "gitea.issue.comment", - "gitea.pr.comment", - ], - "forbidden_operations": [], - }, - ), patch.object( + def test_missing_expected_head_fails(self): + with patch.object(self.mcp, "get_profile", return_value=self._profile()), patch.object( self.mcp, "_authenticated_username", return_value="sysadmin" ), patch.object( - self.mcp, "_profile_operation_gate", return_value=None + self.mcp, "_irrecoverable_capability_gate", return_value=None + ), patch.object( + irp, "assess_transport_for_auth_mint", return_value={"allowed": True, "reasons": []} ), patch.object( self.mcp, "_resolve", return_value=("h", "Scaled-Tech-Consulting", "Gitea-Tools") ): r = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( pr_number=50, - reason="terminal evidence overwritten", - confirmation="IRRECOVERABLE DECISION PROVENANCE PR 50", - operator_authorized=True, - expected_head_sha=HEAD_A, - incident_ref="issue-700-comment-11489", + reason="lost", + confirmation=irp.expected_confirmation(50), + expected_head_sha=None, + incident_issue=700, + incident_comment_id=11489, remote="prgs", org="Scaled-Tech-Consulting", repo="Gitea-Tools", post_audit_comment=False, ) - self.assertTrue(r["success"]) - self.assertFalse(r["applied"]) - self.assertFalse(r["historical_cleanup_proven"]) - self.assertEqual(r["record"]["status"], "provenance_irrecoverable") + self.assertFalse(r["success"]) - # Idempotent replay - with patch.object( - self.mcp, - "get_profile", - return_value={ - "profile_name": "prgs-reconciler", - "allowed_operations": [ - "gitea.read", - "gitea.issue.comment", - "gitea.pr.comment", - ], - "forbidden_operations": [], - }, + def test_wrong_confirmation_fails(self): + with patch.object(self.mcp, "get_profile", return_value=self._profile()), patch.object( + self.mcp, "_irrecoverable_capability_gate", return_value=None ), patch.object( - self.mcp, "_authenticated_username", return_value="sysadmin" - ), patch.object( - self.mcp, "_profile_operation_gate", return_value=None - ), patch.object( - self.mcp, "_resolve", return_value=("h", "Scaled-Tech-Consulting", "Gitea-Tools") - ): - r2 = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( - pr_number=50, - reason="terminal evidence overwritten", - confirmation="IRRECOVERABLE DECISION PROVENANCE PR 50", - operator_authorized=True, - expected_head_sha=HEAD_A, - remote="prgs", - org="Scaled-Tech-Consulting", - repo="Gitea-Tools", - post_audit_comment=False, - ) - self.assertTrue(r2["success"]) - self.assertFalse(r2["performed"]) # idempotent hit - - def test_wrong_confirmation_cannot_unblock_other_pr(self): - with patch.object( - self.mcp, - "get_profile", - return_value={ - "profile_name": "prgs-reconciler", - "allowed_operations": ["gitea.read", "gitea.issue.comment"], - "forbidden_operations": [], - }, - ), patch.object( - self.mcp, "_authenticated_username", return_value="sysadmin" - ), patch.object( - self.mcp, "_profile_operation_gate", return_value=None + irp, "assess_transport_for_auth_mint", return_value={"allowed": True, "reasons": []} ), patch.object( self.mcp, "_resolve", return_value=("h", "o", "r") ): r = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( pr_number=50, reason="x", - confirmation="IRRECOVERABLE DECISION PROVENANCE PR 51", - operator_authorized=True, + confirmation=irp.expected_confirmation(51), + expected_head_sha=HEAD_A, + incident_issue=1, + incident_comment_id=2, remote="prgs", post_audit_comment=False, ) self.assertFalse(r["success"]) + def test_records_with_server_auth(self): + auth = _mint_auth( + pr_number=50, + head=HEAD_A, + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + incident_issue=700, + incident_comment_id=11489, + ) + auth_profile = irp.auth_state_profile_identity( + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + pr_number=50, + expected_head_sha=HEAD_A, + ) + ss.save_state( + kind=ss.KIND_IRRECOVERABLE_PROVENANCE_AUTH, + payload=auth, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity=auth_profile, + state_dir=self._tmp.name, + ) -class TestClearProfileHelper(unittest.TestCase): + def _api(method, url, auth=None, data=None, **kwargs): + 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", + } + 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( + self.mcp, "_irrecoverable_capability_gate", return_value=None + ), patch.object( + irp, "assess_transport_for_auth_mint", return_value={"allowed": True, "reasons": []} + ), patch.object( + self.mcp, "_resolve", return_value=("h", "Scaled-Tech-Consulting", "Gitea-Tools") + ), patch.object( + self.mcp, "_auth", return_value={"Authorization": "token test"} + ), patch.object( + self.mcp, "api_request", side_effect=_api + ), patch.object( + self.mcp, "repo_api_url", return_value="https://example.test/api/v1/repos/o/r" + ): + r = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( + pr_number=50, + reason="terminal evidence overwritten", + confirmation=irp.expected_confirmation(50), + expected_head_sha=HEAD_A, + incident_issue=700, + incident_comment_id=11489, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + post_audit_comment=False, + ) + self.assertTrue(r["success"], r) + self.assertFalse(r["applied"]) + self.assertFalse(r["historical_cleanup_proven"]) + self.assertTrue(r["merger_may_accept"]) + self.assertEqual(r["record"]["status"], "provenance_irrecoverable") + + # Idempotent + with patch.object(self.mcp, "get_profile", return_value=self._profile()), patch.object( + self.mcp, "_authenticated_username", return_value="sysadmin" + ), patch.object( + self.mcp, "_irrecoverable_capability_gate", return_value=None + ), patch.object( + irp, "assess_transport_for_auth_mint", return_value={"allowed": True, "reasons": []} + ), patch.object( + self.mcp, "_resolve", return_value=("h", "Scaled-Tech-Consulting", "Gitea-Tools") + ), patch.object( + self.mcp, "_auth", return_value={"Authorization": "token test"} + ), patch.object( + self.mcp, "api_request", side_effect=_api + ), patch.object( + self.mcp, "repo_api_url", return_value="https://example.test/api/v1/repos/o/r" + ): + r2 = self.mcp.gitea_record_irrecoverable_decision_lock_provenance( + pr_number=50, + reason="terminal evidence overwritten", + confirmation=irp.expected_confirmation(50), + expected_head_sha=HEAD_A, + incident_issue=700, + incident_comment_id=11489, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + post_audit_comment=False, + ) + self.assertTrue(r2["success"], r2) + self.assertFalse(r2["performed"]) + + +class TestClearProfileHelperF3(unittest.TestCase): def setUp(self): self._tmp = tempfile.TemporaryDirectory() self.env = patch.dict( @@ -490,12 +1026,18 @@ class TestClearProfileHelper(unittest.TestCase): ss.save_state( kind=ss.KIND_DECISION_LOCK, payload=_lock([APPROVE], profile="prgs-reviewer", head=HEAD_A), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", profile_identity="prgs-reviewer", state_dir=self._tmp.name, ) ss.save_state( kind=ss.KIND_DECISION_LOCK, payload=_lock([], profile="prgs-merger"), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", profile_identity="prgs-merger", state_dir=self._tmp.name, ) @@ -507,7 +1049,7 @@ class TestClearProfileHelper(unittest.TestCase): org="Scaled-Tech-Consulting", repo="Gitea-Tools", ) - self.assertTrue(out["cleared"]) + self.assertTrue(out["cleared"], out) skip = self.mcp._clear_decision_lock_for_profile( profile_identity="prgs-merger", pr_number=100, @@ -518,6 +1060,84 @@ class TestClearProfileHelper(unittest.TestCase): ) self.assertFalse(skip["cleared"]) + def test_pr_number_only_fallback_impossible(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([APPROVE], profile="prgs-reviewer", head=HEAD_A), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity="prgs-reviewer", + state_dir=self._tmp.name, + ) + # Missing expected_head_sha + out = self.mcp._clear_decision_lock_for_profile( + profile_identity="prgs-reviewer", + pr_number=100, + expected_head_sha=None, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + ) + self.assertFalse(out["cleared"]) + self.assertIn("PR-number-only", out["reason"]) + + def test_wrong_head_not_cleared(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock([APPROVE], profile="prgs-reviewer", head=HEAD_A), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + profile_identity="prgs-reviewer", + state_dir=self._tmp.name, + ) + out = self.mcp._clear_decision_lock_for_profile( + profile_identity="prgs-reviewer", + pr_number=100, + expected_head_sha=HEAD_B, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + ) + self.assertFalse(out["cleared"]) + + def test_cross_repo_same_pr_number_not_cleared(self): + ss.save_state( + kind=ss.KIND_DECISION_LOCK, + payload=_lock( + [APPROVE], + profile="prgs-reviewer", + head=HEAD_A, + repo="Other-Repo", + ), + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Other-Repo", + profile_identity="prgs-reviewer", + state_dir=self._tmp.name, + ) + out = self.mcp._clear_decision_lock_for_profile( + profile_identity="prgs-reviewer", + pr_number=100, + expected_head_sha=HEAD_A, + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + ) + self.assertFalse(out["cleared"]) + # Original lock still present under Other-Repo scope + still = ss.load_state_for_profile( + kind=ss.KIND_DECISION_LOCK, + profile_identity="prgs-reviewer", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Other-Repo", + state_dir=self._tmp.name, + skip_identity_match=True, + ) + self.assertIsNotNone(still) + if __name__ == "__main__": unittest.main() -- 2.43.7 From 2b359e0c260a524863378291f1c2d24516f7502a Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 14 Jul 2026 02:13:47 -0400 Subject: [PATCH 3/3] 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) --- .env.example | 9 + gitea_mcp_server.py | 56 ++- irrecoverable_provenance.py | 443 +++++++++++++++--- stale_review_decision_lock.py | 14 +- ...t_issue_709_decision_lock_cross_profile.py | 335 ++++++++++++- 5 files changed, 753 insertions(+), 104 deletions(-) diff --git a/.env.example b/.env.example index 4777fac..3f82618 100644 --- a/.env.example +++ b/.env.example @@ -55,3 +55,12 @@ GITEA_MCP_PROFILE=prgs # GITEA_MERGER_WORKTREE=/path/to/repo/branches/merge-pr456 # GITEA_RECONCILER_WORKTREE=/path/to/repo/branches/reconcile-pr456 # GITEA_ACTIVE_WORKTREE=/path/to/repo/branches/session-override + +# Durable HMAC key for irrecoverable decision-lock provenance artifacts (#709 F4). +# REQUIRED in production native MCP processes that mint or verify recovery +# authorization (reconciler + merger must share the same key). Hex (preferred), +# base64, or utf-8 literal (>=16 bytes). Never generate an ephemeral per-process +# key — cross-process / post-restart verification would fail closed. +# GITEA_IRRECOVERABLE_AUTH_HMAC_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef +# Optional key version string bound into the signature (for rotation). +# GITEA_IRRECOVERABLE_AUTH_HMAC_KEY_VERSION=v1 diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 0a9270b..d32a213 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -5140,12 +5140,14 @@ def gitea_issue_irrecoverable_provenance_authorization( org: str | None = None, repo: str | None = None, ) -> dict: - """Mint a server-side authorization artifact for irrecoverable recovery (#709 F1). + """Mint a server-side authorization artifact for irrecoverable recovery (#709 F1/F5). - Non-forgeable: requires production native MCP transport (or pytest), a - dedicated/reconciler mutation capability, live head equality, and validated - incident evidence. Confirmation is human intent only — never authorization. - Caller Booleans are not accepted. + Non-forgeable: requires production native MCP transport (or pytest), the + dedicated ``gitea.decision_lock.irrecoverable_recovery`` capability (no + reconciler equivalence), live head equality, durable HMAC key, and + authoritative incident evidence (author + canonical content_digest). + Confirmation is human intent only — never authorization. Caller Booleans + are not accepted. """ import irrecoverable_provenance as irp @@ -5224,7 +5226,7 @@ def gitea_issue_irrecoverable_provenance_authorization( report["reasons"].extend(head_gate.get("reasons") or []) return report - # Incident evidence live validation. + # Incident evidence live validation (author + canonical digest, #709 F5). comment_payload = None comment_err = None try: @@ -5243,6 +5245,10 @@ def gitea_issue_irrecoverable_provenance_authorization( expected_remote=remote, expected_org=o, expected_repo=r, + expected_pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + mint_actor_username=actor, + reject_self_authored=True, ) if not incident_gate.get("valid"): report["reasons"].extend(incident_gate.get("reasons") or []) @@ -5285,19 +5291,23 @@ def gitea_issue_irrecoverable_provenance_authorization( ) return report - artifact = irp.build_authorization_artifact( - remote=remote, - org=o, - repo=r, - pr_number=pr_number, - expected_head_sha=str(expected_head_sha), - incident_issue=int(incident_issue), - incident_comment_id=int(incident_comment_id), - destroyed_subject=destroyed_subject, - issuer_username=actor, - issuer_profile=profile_name or "unknown", - native_provenance=mcp_daemon_guard.mutation_provenance_fields(), - ) + try: + artifact = irp.build_authorization_artifact( + remote=remote, + org=o, + repo=r, + pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + incident_issue=int(incident_issue), + incident_comment_id=int(incident_comment_id), + destroyed_subject=destroyed_subject, + issuer_username=actor, + issuer_profile=profile_name or "unknown", + native_provenance=mcp_daemon_guard.mutation_provenance_fields(), + ) + except irp.AuthSecretError as exc: + report["reasons"].append(str(exc)) + return report artifact["kind"] = mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH saved = mcp_session_state.save_state( kind=mcp_session_state.KIND_IRRECOVERABLE_PROVENANCE_AUTH, @@ -5313,7 +5323,8 @@ def gitea_issue_irrecoverable_provenance_authorization( report["success"] = True report["reasons"].append( "issued server-side irrecoverable provenance authorization " - "(non-forgeable; bound to remote/org/repo/PR/head/incident)" + "(non-forgeable; bound to remote/org/repo/PR/head/incident; " + f"key_version={artifact.get('key_version')})" ) return report @@ -5471,8 +5482,13 @@ def gitea_record_irrecoverable_decision_lock_provenance( incident_comment_id=incident_comment_id, comment_payload=comment_payload if isinstance(comment_payload, dict) else None, comment_lookup_error=comment_err, + expected_remote=remote, expected_org=o, expected_repo=r, + expected_pr_number=pr_number, + expected_head_sha=str(expected_head_sha), + mint_actor_username=actor, + reject_self_authored=True, ) if not incident_gate.get("valid"): report["reasons"].extend(incident_gate.get("reasons") or []) diff --git a/irrecoverable_provenance.py b/irrecoverable_provenance.py index 3a62086..3ec0e4c 100644 --- a/irrecoverable_provenance.py +++ b/irrecoverable_provenance.py @@ -14,7 +14,6 @@ import hashlib import hmac import json import os -import secrets import uuid from datetime import datetime, timedelta, timezone from typing import Any @@ -30,14 +29,28 @@ KIND_AUTH = "irrecoverable_provenance_authorization" KIND_RECOVERY = mcp_session_state.KIND_IRRECOVERABLE_DECISION_PROVENANCE CONFIRMATION_PREFIX = "IRRECOVERABLE DECISION PROVENANCE PR" +# Canonical incident-comment marker (#709 review 435 F5). +INCIDENT_MARKER = "IRRECOVERABLE DECISION PROVENANCE INCIDENT" AUTH_TTL_HOURS = 24.0 RECORD_TYPE = "irrecoverable_decision_provenance" AUTH_TYPE = "irrecoverable_provenance_authorization" -# Internal HMAC material is process-local and never caller-supplied. Pytest -# gets a deterministic salt so hermetic tests are stable; production uses -# transport fingerprint + random secret minted at process start. +# Durable HMAC key origin (#709 review 435 F4). Never generate an ephemeral +# production key — mint/verify across reconciler/merger processes and restarts +# requires a shared secret from env (or pytest constant / explicit env override). +ENV_AUTH_HMAC_KEY = "GITEA_IRRECOVERABLE_AUTH_HMAC_KEY" +ENV_AUTH_HMAC_KEY_VERSION = "GITEA_IRRECOVERABLE_AUTH_HMAC_KEY_VERSION" +DEFAULT_KEY_VERSION = "v1" +_PYTEST_AUTH_SECRET = b"pytest-irrecoverable-auth-v1" +_PYTEST_KEY_VERSION = "pytest-v1" + _PROCESS_AUTH_SECRET: bytes | None = None +_PROCESS_AUTH_KEY_VERSION: str | None = None +_PROCESS_AUTH_SECRET_SOURCE: str | None = None + + +class AuthSecretError(RuntimeError): + """Raised when the durable HMAC signing key cannot be resolved (fail closed).""" def _now() -> datetime: @@ -48,14 +61,91 @@ def _now_iso() -> str: return _now().isoformat() +def _decode_key_material(raw: str) -> bytes: + """Decode operator-supplied key material (hex, base64, or utf-8 literal).""" + text = (raw or "").strip() + if not text: + raise AuthSecretError("empty HMAC key material (fail closed, #709 F4)") + # Prefer hex (64 chars = 32 bytes). + if len(text) >= 32 and all(c in "0123456789abcdefABCDEF" for c in text): + try: + if len(text) % 2 == 0: + decoded = bytes.fromhex(text) + if len(decoded) >= 16: + return decoded + except ValueError: + pass + # base64 + try: + import base64 + + decoded = base64.b64decode(text, validate=True) + if len(decoded) >= 16: + return decoded + except Exception: + pass + # utf-8 literal (min 16 chars after strip) + encoded = text.encode("utf-8") + if len(encoded) < 16: + raise AuthSecretError( + "HMAC key material too short (need >=16 bytes; fail closed, #709 F4)" + ) + return encoded + + +def reset_process_auth_secret_for_tests() -> None: + """Clear cached HMAC key (tests only).""" + global _PROCESS_AUTH_SECRET, _PROCESS_AUTH_KEY_VERSION, _PROCESS_AUTH_SECRET_SOURCE + _PROCESS_AUTH_SECRET = None + _PROCESS_AUTH_KEY_VERSION = None + _PROCESS_AUTH_SECRET_SOURCE = None + + +def auth_key_version() -> str: + """Return the active signing-key version string (never secret material).""" + _process_secret() # ensure version is resolved with the secret + return _PROCESS_AUTH_KEY_VERSION or DEFAULT_KEY_VERSION + + def _process_secret() -> bytes: - global _PROCESS_AUTH_SECRET - if _PROCESS_AUTH_SECRET is None: - if mcp_daemon_guard.is_pytest_runtime(): - _PROCESS_AUTH_SECRET = b"pytest-irrecoverable-auth-v1" - else: - _PROCESS_AUTH_SECRET = secrets.token_bytes(32) - return _PROCESS_AUTH_SECRET + """Resolve durable HMAC key for irrecoverable auth artifacts (#709 F4). + + Production (non-pytest): **requires** ``GITEA_IRRECOVERABLE_AUTH_HMAC_KEY``. + Silently generating an ephemeral per-process key is forbidden — that made + mint+verify fail across merger/reconciler processes and restarts. + + Pytest: uses a fixed constant unless the env key is set (so cross-process + regression tests can inject a shared durable key). + """ + global _PROCESS_AUTH_SECRET, _PROCESS_AUTH_KEY_VERSION, _PROCESS_AUTH_SECRET_SOURCE + if _PROCESS_AUTH_SECRET is not None: + return _PROCESS_AUTH_SECRET + + env_raw = (os.environ.get(ENV_AUTH_HMAC_KEY) or "").strip() + env_ver = (os.environ.get(ENV_AUTH_HMAC_KEY_VERSION) or "").strip() + pytest = mcp_daemon_guard.is_pytest_runtime() + + if env_raw: + _PROCESS_AUTH_SECRET = _decode_key_material(env_raw) + _PROCESS_AUTH_KEY_VERSION = env_ver or ( + _PYTEST_KEY_VERSION if pytest else DEFAULT_KEY_VERSION + ) + _PROCESS_AUTH_SECRET_SOURCE = "env" + return _PROCESS_AUTH_SECRET + + if pytest: + _PROCESS_AUTH_SECRET = _PYTEST_AUTH_SECRET + _PROCESS_AUTH_KEY_VERSION = env_ver or _PYTEST_KEY_VERSION + _PROCESS_AUTH_SECRET_SOURCE = "pytest_constant" + return _PROCESS_AUTH_SECRET + + # Production fail-closed: do NOT call secrets.token_bytes. + raise AuthSecretError( + f"{ENV_AUTH_HMAC_KEY} is required for production irrecoverable auth HMAC; " + "ephemeral per-process key generation is forbidden (fail closed, #709 F4 " + "review 435). Configure a durable shared secret for mint+verify across " + "processes and restarts." + ) def expected_confirmation(pr_number: int) -> str: @@ -137,9 +227,19 @@ def _scope_payload( } -def _sign_scope(scope: dict[str, Any], native_provenance: dict[str, Any]) -> str: - """HMAC over canonical scope + native transport fingerprint (non-caller).""" +def _sign_scope( + scope: dict[str, Any], + native_provenance: dict[str, Any], + *, + key_version: str | None = None, +) -> str: + """HMAC over key_version + canonical scope + native transport fingerprint. + + The signing key itself is never serialized. Key *version* is bound into the + MAC so operators can rotate durable keys without ambiguous verification. + """ material = { + "key_version": (key_version or auth_key_version()), "scope": scope, "native": { "native_mcp_transport": bool( @@ -166,15 +266,14 @@ def assess_capability_for_irrecoverable_recovery( role_kind: str | None = None, profile_name: str | None = None, ) -> dict[str, Any]: - """Whether the active profile may mint/use irrecoverable recovery (#709 F1). + """Whether the active profile may mint/use irrecoverable recovery (#709 F5). - Accepts the dedicated capability, or a reconciler-shaped profile that - already holds issue-comment mutation rights (interim equivalence until - operators grant the dedicated op). Never treats bare ``gitea.read`` as - sufficient. + **Dedicated capability only.** Reconciler role / ``gitea.issue.comment`` + equivalence is intentionally rejected so a reconciler cannot self-mint + recovery authority by authoring its own incident comment (#709 review 435 + F5). Bare ``gitea.read`` is never sufficient. """ import gitea_config - import reconciler_profile allowed = list(allowed_operations or []) forbidden = list(forbidden_operations or []) @@ -191,36 +290,15 @@ def assess_capability_for_irrecoverable_recovery( "reasons": [], } - # Interim: reconciler profile with issue.comment (mutation, not read-only). - is_reconciler = reconciler_profile.is_reconciler_profile(allowed, forbidden) - comment_ok, _ = gitea_config.check_operation( - "gitea.issue.comment", allowed, forbidden - ) role = (role_kind or "").strip().lower() name = (profile_name or "").strip().lower() - role_looks_reconciler = role == "reconciler" or "reconciler" in name - if is_reconciler and comment_ok: - return { - "allowed": True, - "capability": CAPABILITY_IRRECOVERABLE_RECOVERY, - "via": "reconciler_profile_equivalence", - "reasons": [], - } - if role_looks_reconciler and comment_ok and not dedicated_ok: - # Role metadata says reconciler but ops incomplete — still fail if - # is_reconciler_profile is false (missing pr.close). - reasons.append( - "reconciler role metadata without reconciler-required operations " - f"(need {CAPABILITY_IRRECOVERABLE_RECOVERY} or reconciler profile " - "with gitea.pr.close + gitea.issue.comment; gitea.read alone is " - "insufficient, #709 F1)" - ) - else: - reasons.append( - f"missing dedicated capability {CAPABILITY_IRRECOVERABLE_RECOVERY} " - "(gitea.read is insufficient; require reconciler-capable mutation " - "profile or explicit grant, #709 F1)" - ) + role_hint = role or name or "unknown" + reasons.append( + f"missing dedicated capability {CAPABILITY_IRRECOVERABLE_RECOVERY} " + f"(profile={role_hint!r}; reconciler/issue.comment equivalence is " + "not accepted — dedicated grant required, #709 F5 review 435; " + "gitea.read alone is insufficient)" + ) return { "allowed": False, "capability": CAPABILITY_IRRECOVERABLE_RECOVERY, @@ -250,6 +328,108 @@ def assess_transport_for_auth_mint() -> dict[str, Any]: } +def _incident_author(comment_payload: dict[str, Any]) -> str | None: + user = comment_payload.get("user") + if isinstance(user, dict): + login = (user.get("login") or user.get("username") or "").strip() + return login or None + if isinstance(user, str) and user.strip(): + return user.strip() + # Some payloads flatten author. + for key in ("login", "author", "username"): + val = comment_payload.get(key) + if isinstance(val, str) and val.strip(): + return val.strip() + return None + + +def canonical_incident_fields( + *, + remote: str, + org: str, + repo: str, + pr_number: int, + expected_head_sha: str, + incident_issue: int, +) -> dict[str, str]: + """Ordered fields that form the authoritative incident content digest.""" + head = normalize_head_sha(expected_head_sha) or "" + return { + "marker": INCIDENT_MARKER, + "remote": str(remote or "").strip(), + "org": str(org or "").strip(), + "repo": str(repo or "").strip(), + "pr_number": str(int(pr_number)), + "expected_head_sha": head, + "incident_issue": str(int(incident_issue)), + } + + +def incident_content_digest(fields: dict[str, str]) -> str: + """SHA-256 hex digest over canonical field lines (stable, sort-free order).""" + # Fixed key order — do not sort; operator body must match this order. + order = ( + "marker", + "remote", + "org", + "repo", + "pr_number", + "expected_head_sha", + "incident_issue", + ) + lines = [f"{k}={fields[k]}" for k in order] + blob = "\n".join(lines).encode("utf-8") + return hashlib.sha256(blob).hexdigest() + + +def build_canonical_incident_body( + *, + remote: str, + org: str, + repo: str, + pr_number: int, + expected_head_sha: str, + incident_issue: int, + narrative: str | None = None, +) -> str: + """Build an operator-postable canonical incident comment body (#709 F5).""" + fields = canonical_incident_fields( + remote=remote, + org=org, + repo=repo, + pr_number=pr_number, + expected_head_sha=expected_head_sha, + incident_issue=incident_issue, + ) + digest = incident_content_digest(fields) + lines = [ + fields["marker"], + f"remote: {fields['remote']}", + f"org: {fields['org']}", + f"repo: {fields['repo']}", + f"pr_number: {fields['pr_number']}", + f"expected_head_sha: {fields['expected_head_sha']}", + f"incident_issue: {fields['incident_issue']}", + f"content_digest: {digest}", + ] + if narrative and narrative.strip(): + lines.extend(["", narrative.strip()]) + return "\n".join(lines) + + +def _parse_incident_field(body: str, name: str) -> str | None: + """Extract ``name: value`` or ``name=value`` from incident body lines.""" + for line in (body or "").splitlines(): + text = line.strip() + if not text or text.startswith("#"): + continue + for sep in (":", "="): + prefix = f"{name}{sep}" + if text.lower().startswith(prefix.lower()): + return text[len(prefix) :].strip() + return None + + def assess_incident_evidence( *, incident_issue: int | None, @@ -259,8 +439,19 @@ def assess_incident_evidence( expected_remote: str | None = None, expected_org: str | None = None, expected_repo: str | None = None, + expected_pr_number: int | None = None, + expected_head_sha: str | None = None, + mint_actor_username: str | None = None, + reject_self_authored: bool = True, ) -> dict[str, Any]: - """Validate canonical incident evidence is present and live-fetched.""" + """Validate authoritative incident evidence (live-fetched, #709 F5). + + Requires: + - live comment id match + - non-empty author identity + - canonical marker + scope fields + content_digest integrity + - optional mint-actor independence (mint actor ≠ comment author) + """ reasons: list[str] = [] if incident_issue is None or int(incident_issue) <= 0: reasons.append("incident_issue is required and must be a positive integer") @@ -293,34 +484,139 @@ def assess_incident_evidence( if not body: reasons.append("incident comment body is empty (fail closed)") - # Soft scope hints when URLs are present (never hard-code issue numbers). + author = _incident_author(comment_payload) + if not author: + reasons.append( + "incident comment missing author identity (fail closed, #709 F5)" + ) + + # Self-mint prevention: minting actor must not be the sole incident author. + if ( + reject_self_authored + and author + and mint_actor_username + and author.strip().lower() == str(mint_actor_username).strip().lower() + ): + reasons.append( + "incident comment author matches mint actor; self-authored incident " + "evidence is not accepted (fail closed, #709 F5 review 435)" + ) + + # Canonical content + digest (not any non-empty body). + if body and INCIDENT_MARKER not in body: + reasons.append( + f"incident comment missing canonical marker {INCIDENT_MARKER!r} " + "(fail closed, #709 F5)" + ) + elif body: + remote_f = _parse_incident_field(body, "remote") + org_f = _parse_incident_field(body, "org") + repo_f = _parse_incident_field(body, "repo") + pr_f = _parse_incident_field(body, "pr_number") + head_f = _parse_incident_field(body, "expected_head_sha") + issue_f = _parse_incident_field(body, "incident_issue") + digest_f = _parse_incident_field(body, "content_digest") + + if expected_remote and remote_f and remote_f != str(expected_remote).strip(): + reasons.append( + "incident body remote does not match expected remote (fail closed)" + ) + if expected_org and org_f and org_f != str(expected_org).strip(): + reasons.append( + "incident body org does not match expected org (fail closed)" + ) + if expected_repo and repo_f and repo_f != str(expected_repo).strip(): + reasons.append( + "incident body repo does not match expected repo (fail closed)" + ) + if expected_pr_number is not None: + try: + if pr_f is None or int(pr_f) != int(expected_pr_number): + reasons.append( + "incident body pr_number does not match mint PR " + "(fail closed, #709 F5)" + ) + except (TypeError, ValueError): + reasons.append( + "incident body pr_number missing or invalid (fail closed)" + ) + if expected_head_sha: + if not head_f or not heads_equal(head_f, expected_head_sha): + reasons.append( + "incident body expected_head_sha does not match live mint " + "head (fail closed, #709 F5)" + ) + try: + if issue_f is None or int(issue_f) != int(incident_issue): # type: ignore[arg-type] + reasons.append( + "incident body incident_issue does not match provided " + "incident_issue (fail closed, #709 F5)" + ) + except (TypeError, ValueError): + reasons.append( + "incident body incident_issue missing or invalid (fail closed)" + ) + + # Content digest integrity when we have enough scope to recompute. + if ( + remote_f + and org_f + and repo_f + and pr_f + and head_f + and issue_f + and digest_f + ): + try: + fields = canonical_incident_fields( + remote=remote_f, + org=org_f, + repo=repo_f, + pr_number=int(pr_f), + expected_head_sha=head_f, + incident_issue=int(issue_f), + ) + expected_digest = incident_content_digest(fields) + if not hmac.compare_digest( + expected_digest.lower(), str(digest_f).strip().lower() + ): + reasons.append( + "incident content_digest mismatch (forged or incomplete " + "canonical body; fail closed, #709 F5)" + ) + except (TypeError, ValueError) as exc: + reasons.append( + f"incident content_digest recompute failed: {exc} (fail closed)" + ) + else: + reasons.append( + "incident body missing required canonical fields " + "(remote/org/repo/pr_number/expected_head_sha/incident_issue/" + "content_digest; fail closed, #709 F5)" + ) + + # Hard scope check when URLs are present. issue_url = str( comment_payload.get("issue_url") or comment_payload.get("html_url") or "" ) - if expected_org and expected_org not in issue_url and issue_url: - # Only fail when URL is present and clearly wrong-org; missing URL ok. - if f"/{expected_org}/" not in issue_url: - # html_url may be /user/repo/issues/n — check repo if provided - if expected_repo and f"/{expected_repo}/" not in issue_url: - reasons.append( - "incident evidence URL does not match expected repository " - "(fail closed)" - ) + if expected_org and issue_url and f"/{expected_org}/" not in issue_url: + if expected_repo and f"/{expected_repo}/" not in issue_url: + reasons.append( + "incident evidence URL does not match expected repository " + "(fail closed)" + ) return { "valid": not reasons, "reasons": reasons, "comment": { "id": comment_payload.get("id"), - "author": ( - (comment_payload.get("user") or {}).get("login") - if isinstance(comment_payload.get("user"), dict) - else comment_payload.get("user") - ), + "author": author, "created_at": comment_payload.get("created_at"), "body_len": len(body), + "has_canonical_marker": INCIDENT_MARKER in body if body else False, }, } @@ -396,7 +692,12 @@ def build_authorization_artifact( expires_at=expires.isoformat(), authorization_id=authorization_id, ) - signature = _sign_scope(scope, provenance) + try: + kv = auth_key_version() + signature = _sign_scope(scope, provenance, key_version=kv) + except AuthSecretError as exc: + # Surface fail-closed mint: caller must not get a forgeable blank sig. + raise AuthSecretError(str(exc)) from exc return { "kind": KIND_AUTH, "auth_type": AUTH_TYPE, @@ -407,6 +708,8 @@ def build_authorization_artifact( "recovery_critical": True, "issue_ref": "#709", "server_signature": signature, + "key_version": kv, + # Never serialize the secret; only the version id. "native_provenance": provenance, **scope, "timestamp": created.isoformat(), @@ -487,7 +790,7 @@ def verify_authorization_artifact( if not (auth.get("server_signature") or "").strip(): reasons.append("authorization missing server_signature (fail closed)") - # Recompute signature over stored scope fields. + # Recompute signature over stored scope fields + key_version. try: scope = _scope_payload( remote=str(auth.get("remote") or ""), @@ -507,14 +810,18 @@ def verify_authorization_artifact( native = auth.get("native_provenance") or {} if not isinstance(native, dict): native = {} - expected_sig = _sign_scope(scope, native) + stored_kv = str(auth.get("key_version") or auth_key_version()) + expected_sig = _sign_scope(scope, native, key_version=stored_kv) if not hmac.compare_digest( expected_sig, str(auth.get("server_signature") or "") ): reasons.append( - "authorization server_signature invalid (forged or corrupt; " - "fail closed, #709 F1)" + "authorization server_signature invalid (forged, corrupt, or " + "HMAC key mismatch across process/restart; fail closed, " + "#709 F4/F1)" ) + except AuthSecretError as exc: + reasons.append(f"authorization HMAC key unavailable: {exc} (fail closed)") except (TypeError, ValueError) as exc: reasons.append(f"authorization scope incomplete: {exc} (fail closed)") diff --git a/stale_review_decision_lock.py b/stale_review_decision_lock.py index 3eee525..3d5d1df 100644 --- a/stale_review_decision_lock.py +++ b/stale_review_decision_lock.py @@ -504,7 +504,13 @@ def lock_targets_merged_pr_approval( pr_number: int, expected_head_sha: str | None = None, ) -> bool: - """True when *lock*'s last terminal mutation is approve of *pr_number*.""" + """True when *lock*'s last terminal mutation is approve of *pr_number*. + + When *expected_head_sha* is provided, a **recorded** terminal head must + match. Legacy same-repo approve locks with no recorded head do **not** + match (fail closed, #709 F3 residual / review 435) — callers must use the + strict secondary path that refuses no-head clears. + """ last = last_terminal_mutation(lock) if last is None: return False @@ -513,8 +519,12 @@ def lock_targets_merged_pr_approval( if last.get("pr_number") != pr_number: return False if expected_head_sha: + want = normalize_head_sha(expected_head_sha) + if not want: + return False locked = mutation_head_sha(last, lock) - if locked and not heads_equal(locked, expected_head_sha): + # Require recorded-head match; unrecorded head is not a match (#709 F3). + if not locked or not heads_equal(locked, want): return False return True diff --git a/tests/test_issue_709_decision_lock_cross_profile.py b/tests/test_issue_709_decision_lock_cross_profile.py index 8aae98e..f20270b 100644 --- a/tests/test_issue_709_decision_lock_cross_profile.py +++ b/tests/test_issue_709_decision_lock_cross_profile.py @@ -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( -- 2.43.7