fix(author): harden dirty-session rebind inventory and journal identity (#868)

Complete dirty-inventory revalidation immediately before and after
bind_session_lock so added, removed, or renamed paths fail closed.
Persist and validate full recovery-journal operation identity (remote,
org, repo, claimant identity, claimant profile) on execute, resume,
retry, and already_rebound. Add focused regression coverage and the
reconciler success-path integration test.

Closes #868.
This commit is contained in:
2026-07-24 01:13:50 -04:00
parent ef14622ba0
commit ba3ea3012c
2 changed files with 904 additions and 70 deletions
+401 -68
View File
@@ -1,4 +1,4 @@
"""Dirty-preserving same-claimant author-session rebind (#864).
"""Dirty-preserving same-claimant author-session rebind (#864 / #868).
A registered issue worktree can be dirty while its durable lock owner PID is
provably dead. Ordinary ``gitea_lock_issue`` refuses dirty trees, and dead-session
@@ -14,6 +14,14 @@ This operation:
heartbeat)
* preserves every tracked/untracked byte
* does NOT sync remote, create recovery worktrees, clean, reset, or change heads
#868 hardens:
* complete dirty-inventory revalidation (full path set + fingerprints)
immediately before and after ``bind_session_lock``
* durable recovery-journal operation identity (remote, org, repo, claimant
identity, claimant profile) validated on execute / resume / retry /
already_rebound
"""
from __future__ import annotations
@@ -66,6 +74,16 @@ REQUIRED_LOCK_FIELDS = (
"repo",
)
# Durable journal operation identity (#868 F2). All five must be persisted on
# JOURNAL_PHASE_ASSESSED and re-validated on resume / retry / already_rebound.
REQUIRED_JOURNAL_IDENTITY_FIELDS = (
"remote",
"org",
"repo",
"claimant_identity",
"claimant_profile",
)
def _utc_now_iso() -> str:
return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
@@ -196,6 +214,174 @@ def collect_dirty_inventory(worktree_path: str) -> dict[str, Any]:
}
def revalidate_complete_dirty_inventory(
worktree_path: str,
*,
expected_dirty_paths: Sequence[str] | None,
expected_fingerprints: Mapping[str, str] | None,
phase: str = "inventory",
) -> dict[str, Any]:
"""Collect the full dirty inventory and require exact pin equality (#868 F1).
Unlike fingerprint-only checks over the expected path list, this recollects
the authoritative tracked+untracked inventory and refuses added, removed,
or renamed paths as well as fingerprint movement.
"""
reasons: list[str] = []
inv = collect_dirty_inventory(worktree_path)
if inv.get("ok") is False:
reasons.extend(list(inv.get("reasons") or []) or [f"{phase}: dirty inventory collection failed"])
observed_paths = sorted(
{_text(p) for p in (inv.get("dirty_paths") or []) if _text(p)}
)
pin_paths = sorted(
{_text(p) for p in (expected_dirty_paths or []) if _text(p)}
)
if not pin_paths:
reasons.append(
f"{phase}: expected_dirty_paths pin is empty; complete inventory "
"revalidation requires a non-empty pin (fail closed)"
)
if set(observed_paths) != set(pin_paths):
extra = sorted(set(observed_paths) - set(pin_paths))
missing = sorted(set(pin_paths) - set(observed_paths))
if extra:
reasons.append(
f"{phase}: complete dirty inventory path-set disagreement: "
f"unexpected paths {extra}"
)
if missing:
reasons.append(
f"{phase}: complete dirty inventory path-set disagreement: "
f"missing expected paths {missing}"
)
obs_fps = {
_text(k): _text(v)
for k, v in dict(inv.get("fingerprints") or {}).items()
if _text(k)
}
pin_fps = {
_text(k): _text(v)
for k, v in dict(expected_fingerprints or {}).items()
if _text(k)
}
if not pin_fps:
reasons.append(
f"{phase}: expected_fingerprints pin is empty; byte-level pins "
"are required (fail closed)"
)
else:
for rel, expected_hash in pin_fps.items():
if rel not in set(pin_paths):
reasons.append(
f"{phase}: expected_fingerprints contains '{rel}' which is "
"not in expected_dirty_paths"
)
continue
actual_hash = obs_fps.get(rel)
if not actual_hash:
reasons.append(
f"{phase}: fingerprint missing for dirty path '{rel}'"
)
elif actual_hash != expected_hash:
reasons.append(
f"{phase}: fingerprint disagreement for '{rel}': "
f"observed {actual_hash}, expected {expected_hash}"
)
for rel in observed_paths:
if rel not in pin_fps:
reasons.append(
f"{phase}: observed dirty path '{rel}' has no fingerprint pin"
)
return {
"ok": not reasons,
"reasons": reasons,
"inventory": inv,
"observed_dirty_paths": observed_paths,
"expected_dirty_paths": pin_paths,
"observed_fingerprints": obs_fps,
"expected_fingerprints": pin_fps,
"phase": phase,
}
def build_journal_operation_identity(
*,
remote: str,
org: str,
repo: str,
claimant_identity: str | None,
claimant_profile: str | None,
) -> dict[str, str]:
"""Return the five-field durable operation identity for the recovery journal."""
return {
"remote": _text(remote),
"org": _text(org),
"repo": _text(repo),
"claimant_identity": _text(claimant_identity),
"claimant_profile": _text(claimant_profile),
}
def validate_journal_operation_identity(
journal: Mapping[str, Any] | None,
*,
remote: str,
org: str,
repo: str,
claimant_identity: str | None,
claimant_profile: str | None,
require_present: bool = True,
) -> list[str]:
"""Validate durable journal identity fields (#868 F2).
Rejects missing, mismatched, stale, cross-repository, or cross-claimant
journal state. When *require_present* is True, incomplete legacy journals
(any of the five fields absent/empty) fail closed.
"""
reasons: list[str] = []
if not isinstance(journal, Mapping):
if require_present:
reasons.append(
"recovery journal is missing or unreadable; complete operation "
"identity cannot be proven (fail closed)"
)
return reasons
expected = build_journal_operation_identity(
remote=remote,
org=org,
repo=repo,
claimant_identity=claimant_identity,
claimant_profile=claimant_profile,
)
for field in REQUIRED_JOURNAL_IDENTITY_FIELDS:
observed = _text(journal.get(field))
want = expected[field]
if not observed:
reasons.append(
f"recovery journal omits operation identity field '{field}' "
"(incomplete legacy or malformed journal identity; fail closed)"
)
continue
if not want:
reasons.append(
f"caller pin for journal identity field '{field}' is empty "
"(fail closed)"
)
continue
if observed != want:
reasons.append(
f"recovery journal identity mismatch for '{field}': "
f"journal={observed!r}, expected={want!r} "
"(cross-repository / cross-claimant / replay refused)"
)
return reasons
def journal_path(lock_dir: str, issue_number: int) -> str:
root = (lock_dir or "").strip()
return os.path.join(root, f".rebind-journal-{int(issue_number)}.json")
@@ -789,10 +975,22 @@ def _already_rebound(
existing_lock: Mapping[str, Any],
current_pid: int,
worktree_path: str,
expected_dirty_paths: Sequence[str] | None,
expected_fingerprints: Mapping[str, str],
worktree_for_fps: str,
remote: str,
org: str,
repo: str,
claimant_identity: str | None,
claimant_profile: str | None,
journal: Mapping[str, Any] | None = None,
) -> tuple[bool, list[str]]:
"""Return (True, notes) when lock is already rebound to this session."""
"""Return (True, notes) when lock is already rebound to this session.
#868: require complete matching operation identity (remote/org/repo/
claimant) and complete dirty-inventory revalidation, not fingerprint-only
checks. Incomplete or mismatched journal identity fails closed.
"""
notes: list[str] = []
pid = _recorded_pid(existing_lock)
try:
@@ -803,19 +1001,61 @@ def _already_rebound(
return False, []
if not _same_realpath(_text(existing_lock.get("worktree_path")), worktree_path):
return False, []
# Fingerprints must still match pins (byte preservation).
for rel, expected in (expected_fingerprints or {}).items():
abs_path = os.path.join(worktree_for_fps, rel)
try:
actual = content_fingerprint(abs_path)
except OSError as exc:
notes.append(f"could not re-fingerprint '{rel}' for already_rebound: {exc}")
return False, notes
if actual != _text(expected):
# Durable lock repo binding must still match the caller's target.
for field, expected in (("remote", remote), ("org", org), ("repo", repo)):
observed = _text(existing_lock.get(field))
want = _text(expected)
if observed and want and observed != want:
notes.append(
f"fingerprint drift on already-rebound check for '{rel}'"
f"already_rebound refused: lock {field}={observed!r} does not "
f"match expected {want!r} (cross-repository replay)"
)
return False, notes
lock_claimant = _lock_claimant(existing_lock)
locked_identity = _text(lock_claimant.get("username"))
locked_profile = _text(lock_claimant.get("profile"))
pin_identity = _text(claimant_identity)
pin_profile = _text(claimant_profile)
if pin_identity and locked_identity and pin_identity != locked_identity:
notes.append(
f"already_rebound refused: lock claimant '{locked_identity}' does "
f"not match pin '{pin_identity}' (cross-claimant replay)"
)
return False, notes
if pin_profile and locked_profile and pin_profile != locked_profile:
notes.append(
f"already_rebound refused: lock profile '{locked_profile}' does "
f"not match pin '{pin_profile}' (cross-claimant replay)"
)
return False, notes
# When a durable journal is present, require complete matching identity.
if isinstance(journal, Mapping) and journal:
id_reasons = validate_journal_operation_identity(
journal,
remote=remote,
org=org,
repo=repo,
claimant_identity=claimant_identity,
claimant_profile=claimant_profile,
require_present=True,
)
if id_reasons:
notes.extend(id_reasons)
return False, notes
inv_check = revalidate_complete_dirty_inventory(
worktree_for_fps,
expected_dirty_paths=expected_dirty_paths,
expected_fingerprints=expected_fingerprints,
phase="already_rebound",
)
if not inv_check["ok"]:
notes.extend(list(inv_check["reasons"] or []))
return False, notes
gen = lock_generation(existing_lock)
if gen < 1:
# A never-written generation is suspicious for a completed rebind, but
@@ -917,6 +1157,53 @@ def apply_dirty_same_claimant_session_rebind(
"remote_head": remote_head,
}
root_for_journal = (lock_dir or "").strip() or None
if root_for_journal is None and isinstance(existing_lock, Mapping):
root_for_journal = os.path.dirname(
_text(existing_lock.get("lock_file_path"))
or lock_file_path(
remote=remote, org=org, repo=repo, issue_number=issue_number
)
)
jpath_probe = (
journal_path(root_for_journal, issue_number) if root_for_journal else ""
)
existing_journal = _read_json(jpath_probe) if jpath_probe else None
# Resume / retry: reject incomplete, mismatched, or cross-repo journal
# identity before treating any prior journal as authoritative (#868 F2).
if isinstance(existing_journal, Mapping) and existing_journal:
journal_id_reasons = validate_journal_operation_identity(
existing_journal,
remote=remote,
org=org,
repo=repo,
claimant_identity=claimant_identity,
claimant_profile=claimant_profile,
require_present=True,
)
# Incomplete legacy journals from pre-#868 apply paths must fail closed
# when any identity field is missing — even if the rest of the payload
# looks familiar. Only a complete matching identity may proceed.
phase = _text(existing_journal.get("phase"))
if journal_id_reasons and phase not in ("", JOURNAL_PHASE_COMPLETE):
# Allow a completed journal with missing legacy identity only when
# already_rebound path will re-validate lock + inventory; for
# mid-flight incomplete journals, refuse.
if phase in (
JOURNAL_PHASE_ASSESSED,
JOURNAL_PHASE_PRE_BIND,
JOURNAL_PHASE_BOUND,
"bind_failed",
):
return {
**base_result,
"success": False,
"reasons": journal_id_reasons,
"journal_path": jpath_probe,
"journal_phase": phase or None,
}
# Retry-safe: if already rebound to this session, succeed even when assess
# refuses because old_pid no longer matches the (updated) lock.
if (
@@ -928,8 +1215,15 @@ def apply_dirty_same_claimant_session_rebind(
existing_lock=existing_lock,
current_pid=pid_now,
worktree_path=worktree_path,
expected_dirty_paths=expected_dirty_paths,
expected_fingerprints=expected_fingerprints,
worktree_for_fps=wt,
remote=remote,
org=org,
repo=repo,
claimant_identity=claimant_identity,
claimant_profile=claimant_profile,
journal=existing_journal,
)
if done:
lock_path = _text(existing_lock.get("lock_file_path")) or lock_file_path(
@@ -956,6 +1250,20 @@ def apply_dirty_same_claimant_session_rebind(
"generation_after": lock_generation(existing_lock),
"journal_phase": JOURNAL_PHASE_ALREADY_REBOUND,
}
# Same-pid candidate that failed complete identity/inventory checks
# must not fall through into a fresh bind that would re-mint authority.
if _recorded_pid(existing_lock) is not None:
try:
if int(_recorded_pid(existing_lock)) == int(pid_now) and notes:
return {
**base_result,
"success": False,
"already_rebound": False,
"reasons": notes,
"journal_path": jpath_probe or None,
}
except (TypeError, ValueError):
pass
if not assessment["rebind_sanctioned"]:
return base_result
@@ -982,6 +1290,25 @@ def apply_dirty_same_claimant_session_rebind(
issue_number,
)
op_identity = build_journal_operation_identity(
remote=remote,
org=org,
repo=repo,
claimant_identity=claimant_identity,
claimant_profile=claimant_profile,
)
# Refuse incomplete caller identity before any durable write.
for field, value in op_identity.items():
if not value:
return {
**base_result,
"success": False,
"reasons": [
f"cannot write recovery journal: operation identity field "
f"'{field}' is empty (fail closed)"
],
}
journal = {
"phase": JOURNAL_PHASE_ASSESSED,
"issue_number": issue_number,
@@ -996,36 +1323,40 @@ def apply_dirty_same_claimant_session_rebind(
"remote_head": remote_head,
"started_at": _utc_now_iso(),
"source": SOURCE,
# #868 F2 — complete durable operation identity
**op_identity,
}
_atomic_write_json(jpath, journal)
# Immediate pre-bind fingerprint re-verification.
pre_fps: dict[str, str] = {}
for rel in expected_dirty_paths or []:
abs_path = os.path.join(wt, rel)
try:
pre_fps[rel] = content_fingerprint(abs_path)
except OSError as exc:
return {
**base_result,
"success": False,
"reasons": [f"pre-bind fingerprint failed for '{rel}': {exc}"],
"journal_path": jpath,
}
for rel, expected in (expected_fingerprints or {}).items():
if pre_fps.get(rel) != _text(expected):
return {
**base_result,
"success": False,
"reasons": [
f"pre-bind fingerprint drift for '{rel}': "
f"observed {pre_fps.get(rel)}, expected {expected}"
],
"journal_path": jpath,
}
# #868 F1 — complete dirty-inventory revalidation immediately before mutation.
# Fail closed with no bind so failures cannot leave a newly authoritative
# live session.
pre_inv = revalidate_complete_dirty_inventory(
wt,
expected_dirty_paths=expected_dirty_paths,
expected_fingerprints=expected_fingerprints,
phase="pre-bind",
)
if not pre_inv["ok"]:
journal["phase"] = "pre_bind_inventory_failed"
journal["pre_bind_inventory"] = {
"observed_dirty_paths": pre_inv.get("observed_dirty_paths"),
"reasons": pre_inv.get("reasons"),
}
_atomic_write_json(jpath, journal)
return {
**base_result,
"success": False,
"reasons": list(pre_inv["reasons"] or []),
"journal_path": jpath,
"journal_phase": "pre_bind_inventory_failed",
}
pre_fps = dict(pre_inv.get("observed_fingerprints") or {})
journal["phase"] = JOURNAL_PHASE_PRE_BIND
journal["pre_bind_fingerprints"] = pre_fps
journal["pre_bind_dirty_paths"] = list(pre_inv.get("observed_dirty_paths") or [])
_atomic_write_json(jpath, journal)
now = _utc_now_iso()
@@ -1095,38 +1426,37 @@ def apply_dirty_same_claimant_session_rebind(
journal["lock_path"] = lock_path
_atomic_write_json(jpath, journal)
# Post-bind fingerprint verification — every byte unchanged.
post_fps: dict[str, str] = {}
for rel in expected_dirty_paths or []:
abs_path = os.path.join(wt, rel)
try:
post_fps[rel] = content_fingerprint(abs_path)
except OSError as exc:
return {
**base_result,
"success": False,
"reasons": [
f"post-bind fingerprint failed for '{rel}': {exc}; "
"lock may be rebound but content verification failed"
],
"lock_path": lock_path,
"journal_path": jpath,
"generation_before": gen_before,
}
for rel, expected in (expected_fingerprints or {}).items():
if post_fps.get(rel) != _text(expected):
return {
**base_result,
"success": False,
"reasons": [
f"post-bind fingerprint drift for '{rel}': "
f"observed {post_fps.get(rel)}, expected {expected}"
],
"lock_path": lock_path,
"journal_path": jpath,
"generation_before": gen_before,
"fingerprints_after": post_fps,
}
# #868 F1 — complete dirty-inventory revalidation immediately after mutation.
# Path set must remain exactly equal; fingerprints must be unchanged.
post_inv = revalidate_complete_dirty_inventory(
wt,
expected_dirty_paths=expected_dirty_paths,
expected_fingerprints=expected_fingerprints,
phase="post-bind",
)
post_fps = dict(post_inv.get("observed_fingerprints") or {})
if not post_inv["ok"]:
journal["phase"] = "post_bind_inventory_failed"
journal["post_bind_inventory"] = {
"observed_dirty_paths": post_inv.get("observed_dirty_paths"),
"reasons": post_inv.get("reasons"),
}
journal["post_bind_fingerprints"] = post_fps
_atomic_write_json(jpath, journal)
return {
**base_result,
"success": False,
"reasons": list(post_inv["reasons"] or []) + [
"post-bind complete inventory revalidation failed after "
"bind_session_lock; lock may be rebound but content/path "
"verification failed (fail closed)"
],
"lock_path": lock_path,
"journal_path": jpath,
"journal_phase": "post_bind_inventory_failed",
"generation_before": gen_before,
"fingerprints_after": post_fps,
}
# Remove stale session pointer for old_pid when it points at this lock.
removed_old_pointer = False
@@ -1154,6 +1484,9 @@ def apply_dirty_same_claimant_session_rebind(
journal["generation_after"] = gen_after
journal["removed_old_session_pointer"] = removed_old_pointer
journal["post_bind_fingerprints"] = post_fps
journal["post_bind_dirty_paths"] = list(
post_inv.get("observed_dirty_paths") or []
)
_atomic_write_json(jpath, journal)
return {