Merge pull request 'fix(gate): preserve exact-owner renewal evidence across duplicate rechecks (Closes #945)' (#946) from fix/issue-945-owning-pr-renewal-evidence into master
This commit was merged in pull request #946.
This commit is contained in:
+95
-7
@@ -2782,6 +2782,87 @@ def _collect_issue_duplicate_context(
|
||||
return issue_duplicate_context_fetcher(h, o, r, auth, issue_number)
|
||||
|
||||
|
||||
# Every field of the owning-PR continuation token is security relevant: the
|
||||
# issue and PR it names, the branch it is scoped to, and each head the waiver
|
||||
# was measured against. Two evidence blocks that disagree on any of them cannot
|
||||
# both describe the single sanctioned decision the lock is supposed to record.
|
||||
_CONTINUATION_EVIDENCE_BINDINGS = (
|
||||
"issue_number",
|
||||
"pr_number",
|
||||
"branch_name",
|
||||
"head_sha",
|
||||
"recorded_head",
|
||||
"accepted_head",
|
||||
"head_relation",
|
||||
)
|
||||
|
||||
|
||||
def _continuation_evidence_agrees(recovered: dict, renewed: dict) -> bool:
|
||||
"""Do two rebuilt continuation tokens bind to exactly the same thing (#945)?"""
|
||||
return all(
|
||||
recovered.get(field) == renewed.get(field)
|
||||
for field in _CONTINUATION_EVIDENCE_BINDINGS
|
||||
)
|
||||
|
||||
|
||||
def _owning_pr_continuation_from_lock(lock_record: dict | None) -> dict | None:
|
||||
"""Owning-PR continuation evidence a persisted lock still proves (#945).
|
||||
|
||||
``gitea_lock_issue`` grants the duplicate-work waiver from either a
|
||||
sanctioned dead-session recovery (#755) or a sanctioned exact-owner renewal
|
||||
(#760), in that precedence. Every later enforcement path — commit,
|
||||
create-PR, push-ownership, and the read-only duplicate assessor — re-derives
|
||||
ownership from the durable lock instead of that live assessment.
|
||||
|
||||
Until #945 only the recovery half was rebuilt there, so an ordinary
|
||||
exact-owner renewal lost its waiver the moment ``gitea_lock_issue``
|
||||
returned: the author renewed successfully and was then refused
|
||||
``duplicate_commit_prevented`` with ``owning_pr_recovery_exempted: false``
|
||||
on the very PR the renewal had just proved it owned.
|
||||
|
||||
Resolving both halves here, in the same precedence the lock path applies,
|
||||
keeps the answer from drifting between the gate that grants the waiver and
|
||||
the gates that enforce it. This only decides which server-written block the
|
||||
token is rebuilt from — the token is still re-validated against live PR
|
||||
state by ``issue_work_duplicate_gate._assess_owning_pr_exemption``, which
|
||||
remains the single authoritative policy for whether an exemption applies.
|
||||
|
||||
**Both blocks present is a reachable, legitimate state, and it must agree.**
|
||||
The two dispositions are not mutually exclusive at the writer. Recovery is
|
||||
assessed whenever the lease is not live and requires the recorded PID to be
|
||||
dead; renewal is assessed whenever the lease has *expired* — which is itself
|
||||
one way to be non-live — and deliberately does not branch on PID liveness
|
||||
(#760 AC16). An expired lease whose recorded owner has also died therefore
|
||||
satisfies both, and ``gitea_lock_issue`` writes ``dead_session_recovery``
|
||||
and ``lease_renewal`` into the same freshly built ``data`` dict. Because a
|
||||
sanctioned pair was derived from one live observation in one call, it always
|
||||
describes the same issue, PR, branch and head. Disagreement means the
|
||||
persisted lock is no longer a faithful record of a single sanctioned
|
||||
decision, so no continuation authority is returned.
|
||||
|
||||
Ambiguity never broadens authority. A ``dead_session_recovery`` block that
|
||||
is present but does not rebuild — conflicting, stale, malformed, or only
|
||||
partially valid — fails closed here rather than falling through to renewal:
|
||||
otherwise a recovery record naming one PR could be bypassed by valid-looking
|
||||
renewal evidence naming another. Recovery-only and renewal-only locks keep
|
||||
their existing behaviour exactly, and provenance stays server-controlled —
|
||||
this still only ever re-reads blocks the server itself wrote.
|
||||
"""
|
||||
if not lock_record:
|
||||
return None
|
||||
recovery_present = isinstance(lock_record.get("dead_session_recovery"), dict)
|
||||
recovered = issue_lock_recovery.recovered_owning_pr_from_lock(lock_record)
|
||||
# Present but unusable recovery evidence is ambiguous, not absent.
|
||||
if recovery_present and not recovered:
|
||||
return None
|
||||
renewed = issue_lock_renewal.owning_pr_renewal_from_lock(lock_record)
|
||||
if recovered and renewed and not _continuation_evidence_agrees(recovered, renewed):
|
||||
return None
|
||||
if recovered:
|
||||
return recovered
|
||||
return renewed
|
||||
|
||||
|
||||
def _assess_issue_duplicate_gate(
|
||||
issue_number: int,
|
||||
*,
|
||||
@@ -2834,6 +2915,11 @@ def _enforce_locked_issue_duplicate_recheck(
|
||||
commit and create-PR phases run in their own calls, long after the recovery
|
||||
assessment ended, so without this they re-block the very PR the recovery
|
||||
already proved belongs to this author.
|
||||
|
||||
#945: an exact-owner *renewal* (#760) owns its open PR for exactly the same
|
||||
reason, and ``gitea_lock_issue`` already waives the blocker for both. Both
|
||||
halves are resolved together here so the renewal waiver survives past the
|
||||
lock call instead of expiring with it.
|
||||
"""
|
||||
lock_data = _load_existing_issue_lock()
|
||||
if not lock_data:
|
||||
@@ -2857,9 +2943,7 @@ def _enforce_locked_issue_duplicate_recheck(
|
||||
auth=auth,
|
||||
locked_branch=locked_branch,
|
||||
phase=phase,
|
||||
recovered_owning_pr=issue_lock_recovery.recovered_owning_pr_from_lock(
|
||||
lock_data
|
||||
),
|
||||
recovered_owning_pr=_owning_pr_continuation_from_lock(lock_data),
|
||||
)
|
||||
if gate.get("block"):
|
||||
return gate
|
||||
@@ -5141,9 +5225,10 @@ def gitea_assess_work_issue_duplicate(
|
||||
recovered_owning_pr = None
|
||||
lock_data = _load_existing_issue_lock()
|
||||
if lock_data and int(lock_data.get("issue_number") or 0) == int(issue_number):
|
||||
recovered_owning_pr = issue_lock_recovery.recovered_owning_pr_from_lock(
|
||||
lock_data
|
||||
)
|
||||
# #945: rebuilt from a sanctioned recovery *or* a sanctioned exact-owner
|
||||
# renewal, so this read-only assessor reports the same disposition the
|
||||
# commit and create-PR gates will enforce.
|
||||
recovered_owning_pr = _owning_pr_continuation_from_lock(lock_data)
|
||||
gate = _assess_issue_duplicate_gate(
|
||||
issue_number,
|
||||
h=h,
|
||||
@@ -19430,7 +19515,10 @@ def _prove_author_ownership_for_pr(
|
||||
# advance is the one recovery already sanctioned, not a foreign head.
|
||||
recovered_owning_pr = None
|
||||
if proven and lock_record:
|
||||
candidate = issue_lock_recovery.recovered_owning_pr_from_lock(lock_record)
|
||||
# #945: a sanctioned exact-owner renewal proves the same ownership of
|
||||
# the same PR, so the push gate resolves both halves rather than seeing
|
||||
# only the recovery one.
|
||||
candidate = _owning_pr_continuation_from_lock(lock_record)
|
||||
if candidate and int(candidate.get("pr_number") or 0) == int(pr_number):
|
||||
recovered_owning_pr = candidate
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user