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

fix(workflow): fail-closed key version, strict incident evidence, archive-gated clear (#709)

Browse Source 
Address formal review 438 REQUEST_CHANGES on PR #710 (F6/F7/F8).

F6 — HMAC key-version validation fails closed:
- verify_authorization_artifact validates key_version BEFORE any MAC work, so an
  attacker-chosen version can never select the signing key.
- Require exactly one nonempty, well-formed key-version field; missing, empty,
  unknown, malformed, duplicated (including identical-valued and nested
  aliases), and mismatched versions all fail. No versionless legacy fallback.
- Artifact version must equal the configured active version; production now
  requires GITEA_IRRECOVERABLE_AUTH_HMAC_KEY_VERSION explicitly (an implicit
  default made rotation ambiguous). Version stays inside the signed material.

F7 — strictly canonical incident evidence:
- Replace substring/first-match parsing with an exact schema: marker on line 1,
  every field once, fixed order, no duplicate/unknown/empty/conflicting fields
  in or outside the signed block. The parsed body is re-rendered and compared
  for exact equality before acceptance.
- content_digest now binds the full recovery scope: repository identity, PR,
  decision-lock identity, destroyed subject, recovery action, recorded and
  expected head, incident issue, evidence author, minting actor, key version,
  nonce and issued_at.
- Actor identity is the immutable user id with login consistency; conflicting
  ids/logins and display-name-only identities fail closed. Edited comments are
  rejected. The independent-author rule is preserved and enforced by stable id.
- build_canonical_incident_body is the single source of the accepted format and
  refuses to emit ambiguous evidence.

F8 — archival is a prerequisite for clearing terminal evidence:
- _clear_decision_lock_for_profile no longer swallows archive failures. It
  requires a successful write plus a durable read-back matching the PR/head,
  and otherwise returns a structured, retry-safe failure that retains the lock
  and records actionable recovery evidence.
- Fixes a latent bug the read-back exposed: the archive payload inherited the
  source lock's session_profile_lock, so save_state keyed the archive under the
  reviewer profile instead of the archive identity and it never read back.

Adversarial regressions added for every listed case: key-version missing/empty/
unknown/malformed/duplicate/rotation/wrong-key-after-restart, reordered fields,
duplicate identical and conflicting fields, conflicting actor ids/names,
digest-preserving substitution, decision-lock and recovery-action substitution,
cross-PR/repo/org/remote/head replay, archive exception/timeout/false/empty/
partial-readback with proof the lock survives, exactly-one permitted clear, and
retry after archive failure. No existing assertion was weakened.

Validation: focused 150 passed in three module orders; full tests/ 2809 passed,
6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in
tests/test_webui_audit.py:8), 161 subtests passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
sysadmin
2026-07-15 13:55:06 -04:00
co-authored by Claude Opus 4.8
parent 2b359e0c26
commit 573e721437
3 changed files with 1659 additions and 211 deletions
Download Patch File Download Diff File Expand all files Collapse all files
gitea_mcp_server.py
+167 -17
View File
@@ -1824,6 +1824,8 @@ _UNSET = object()
# Best-effort identity cache keyed by host, so an enabled audit trail resolves
# the authenticated username at most once per host per process.
_IDENTITY_CACHE: dict = {}
# Stable actor identity (id + login) for recovery evidence binding (#709 F7).
_ACTOR_IDENTITY_CACHE: dict = {}
def _authenticated_username(host: str):
@@ -1846,6 +1848,33 @@ def _authenticated_username(host: str):
return user
def _authenticated_actor(host: str) -> dict:
"""Resolve the authenticated actor's stable identity (#709 F7 review 438).
Display names are mutable, so recovery evidence is bound to the immutable
numeric user id with the login carried alongside for consistency checks.
Read-only and fail-soft; never surfaces credential material.
"""
cached = _ACTOR_IDENTITY_CACHE.get(host)
if cached is not None:
return dict(cached)
actor: dict = {"user_id": None, "login": None}
try:
header = get_auth_header(host)
if header:
who = api_request("GET", gitea_url(host, "/api/v1/user"), header)
if isinstance(who, dict):
raw_id = who.get("id")
actor = {
"user_id": int(raw_id) if isinstance(raw_id, int) else None,
"login": (who.get("login") or None),
}
except Exception:
actor = {"user_id": None, "login": None}
_ACTOR_IDENTITY_CACHE[host] = dict(actor)
return dict(actor)
def _ensure_matching_profile(required_permission: str, required_role: str, remote: str | None, host: str | None = None) -> str | None:
"""Check if the active profile is allowed to perform *required_permission*.
If not, automatically switch to the first matching usable configured profile.
@@ -1891,6 +1920,7 @@ def _ensure_matching_profile(required_permission: str, required_role: str, remot
h = host or (REMOTES.get(remote, {}).get("host") if remote in REMOTES else None)
if h:
_IDENTITY_CACHE.pop(h, None)
_ACTOR_IDENTITY_CACHE.pop(h, None)
username = _authenticated_username(h) if h else None
# Update mutation authority
global _MUTATION_AUTHORITY
@@ -4658,28 +4688,108 @@ def _clear_decision_lock_for_profile(
),
}
# Archive then clear — only after exact identity validation.
# Archive then clear — archival is a hard prerequisite (#709 F8 review 438).
# A failed, empty, or unconfirmed archive must never be followed by a clear:
# that is exactly the terminal-evidence destruction #709 exists to prevent.
archive_identity = f"{profile_identity}-archive-pr{pr_number}"
archive_payload = {
**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,
# The copied lock carries the *source* profile's identity. Leaving it in
# place makes save_state key the archive under that profile instead of
# the archive identity, so the archive would silently overwrite/land
# elsewhere and never read back (#709 F8 review 438).
"profile_identity": archive_identity,
"session_profile_lock": archive_identity,
"archived_from_profile_identity": profile_identity,
}
def _archive_failure(step: str, detail: str) -> dict:
"""Retain the terminal lock and report an actionable, retryable failure."""
try:
_record_post_merge_decision_recovery(
pr_number=int(pr_number),
head_sha=want_head,
merge_commit_sha=None,
target_profile_identity=profile_identity,
failed_step=step,
error=detail,
remote=remote,
org=org,
repo=repo,
)
except Exception as exc: # noqa: BLE001
detail = f"{detail}; recovery record write also failed: {_redact(str(exc))}"
return {
"profile_identity": profile_identity,
"cleared": False,
"archive_ok": False,
"archive_failed_step": step,
"archive_identity": archive_identity,
"terminal_lock_retained": True,
"recovery_required": True,
"retry_safe": True,
"reason": (
f"decision-lock archival failed at {step}: {detail}. Terminal "
"evidence retained and NOT cleared; resolve the archive failure "
"and retry this cleanup (fail closed, #709 F8 review 438)"
),
"prior_summary": stale_review_decision_lock.lock_summary(lock),
}
try:
mcp_session_state.save_state(
archived = 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,
"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,
},
payload=archive_payload,
remote=remote,
org=org,
repo=repo,
profile_identity=f"{profile_identity}-archive-pr{pr_number}",
profile_identity=archive_identity,
)
except Exception:
pass
except Exception as exc: # noqa: BLE001
return _archive_failure("archive_save_state", _redact(str(exc)))
if not archived:
return _archive_failure(
"archive_save_state",
"save_state returned no durable archive record (false/empty result)",
)
# Durable read-back: a write that cannot be re-read is not an archive.
try:
confirmed = mcp_session_state.load_state_for_profile(
kind=mcp_session_state.KIND_DECISION_LOCK_ARCHIVE,
profile_identity=archive_identity,
remote=remote,
org=org,
repo=repo,
skip_identity_match=True,
enforce_repo_scope=True,
)
except Exception as exc: # noqa: BLE001
return _archive_failure("archive_readback", _redact(str(exc)))
if not confirmed:
return _archive_failure(
"archive_readback",
"archive record could not be read back from durable session state",
)
if int(confirmed.get("archived_for_pr") or -1) != int(
pr_number
) or not stale_review_decision_lock.heads_equal(
confirmed.get("archived_for_head"), want_head
):
return _archive_failure(
"archive_readback",
"archive read-back does not match the PR/head being cleaned "
"(partial or stale archive)",
)
mcp_session_state.clear_state(
kind=mcp_session_state.KIND_DECISION_LOCK,
profile_identity=profile_identity,
@@ -4695,9 +4805,11 @@ def _clear_decision_lock_for_profile(
return {
"profile_identity": profile_identity,
"cleared": True,
"archive_ok": True,
"archive_identity": archive_identity,
"reason": (
f"cleared terminal lock for merged PR #{pr_number} "
f"at head {want_head[:12]}… (exact-scope)"
f"at head {want_head[:12]}… (exact-scope; durable archive confirmed)"
),
"prior_summary": stale_review_decision_lock.lock_summary(lock),
}
@@ -5133,8 +5245,10 @@ def gitea_issue_irrecoverable_provenance_authorization(
expected_head_sha: str,
incident_issue: int,
incident_comment_id: int,
decision_lock_id: str = "",
confirmation: str = "",
destroyed_subject: str | None = None,
recorded_head_sha: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
@@ -5186,6 +5300,13 @@ def gitea_issue_irrecoverable_provenance_authorization(
)
return report
if not (decision_lock_id or "").strip():
report["reasons"].append(
"decision_lock_id is required so evidence is bound to the exact "
"decision lock being recovered (fail closed, #709 F7 review 438)"
)
return report
try:
actor = _authenticated_username(h)
except Exception:
@@ -5195,6 +5316,13 @@ def gitea_issue_irrecoverable_provenance_authorization(
"authenticated identity could not be verified (fail closed)"
)
return report
actor_identity = _authenticated_actor(h)
if actor_identity.get("user_id") is None:
report["reasons"].append(
"authenticated actor id could not be verified; display-name-only "
"identity is not accepted (fail closed, #709 F7 review 438)"
)
return report
profile = get_profile()
profile_name = (profile.get("profile_name") or "").strip() or None
@@ -5237,6 +5365,11 @@ def gitea_issue_irrecoverable_provenance_authorization(
)
except Exception as exc: # noqa: BLE001
comment_err = _redact(str(exc))
try:
active_key_version = irp.auth_key_version()
except irp.AuthSecretError as exc:
report["reasons"].append(str(exc))
return report
incident_gate = irp.assess_incident_evidence(
incident_issue=incident_issue,
incident_comment_id=incident_comment_id,
@@ -5247,6 +5380,11 @@ def gitea_issue_irrecoverable_provenance_authorization(
expected_repo=r,
expected_pr_number=pr_number,
expected_head_sha=str(expected_head_sha),
expected_recorded_head_sha=recorded_head_sha,
expected_decision_lock_id=decision_lock_id.strip(),
expected_recovery_action=irp.RECOVERY_ACTION_IRRECOVERABLE_PROVENANCE,
expected_key_version=active_key_version,
mint_actor_id=actor_identity.get("user_id"),
mint_actor_username=actor,
reject_self_authored=True,
)
@@ -5338,6 +5476,7 @@ def gitea_record_irrecoverable_decision_lock_provenance(
incident_issue: int | None = None,
incident_comment_id: int | None = None,
authorization_id: str | None = None,
decision_lock_id: str = "",
destroyed_subject: str | None = None,
incident_ref: str | None = None,
# Deprecated: retained so callers that still pass it get an explicit deny.
@@ -5477,6 +5616,11 @@ def gitea_record_irrecoverable_decision_lock_provenance(
)
except Exception as exc: # noqa: BLE001
comment_err = _redact(str(exc))
try:
active_key_version = irp.auth_key_version()
except irp.AuthSecretError as exc:
report["reasons"].append(str(exc))
return report
incident_gate = irp.assess_incident_evidence(
incident_issue=incident_issue,
incident_comment_id=incident_comment_id,
@@ -5487,6 +5631,10 @@ def gitea_record_irrecoverable_decision_lock_provenance(
expected_repo=r,
expected_pr_number=pr_number,
expected_head_sha=str(expected_head_sha),
expected_decision_lock_id=(decision_lock_id or "").strip() or None,
expected_recovery_action=irp.RECOVERY_ACTION_IRRECOVERABLE_PROVENANCE,
expected_key_version=active_key_version,
mint_actor_id=_authenticated_actor(h).get("user_id"),
mint_actor_username=actor,
reject_self_authored=True,
)
@@ -8869,6 +9017,7 @@ def _try_auto_switch_for_operation(op: str, host: str | None = None) -> bool:
if tok:
gitea_config._active_profile_override = p_name
_IDENTITY_CACHE.clear()
_ACTOR_IDENTITY_CACHE.clear()
return True
except Exception:
pass
@@ -11989,6 +12138,7 @@ def gitea_activate_profile(
# 3. Clear identity cache to force a fresh verification
if h:
_IDENTITY_CACHE.pop(h, None)
_ACTOR_IDENTITY_CACHE.pop(h, None)
# 4. Resolve fresh identity
after_profile = get_profile()["profile_name"]