[hivecommons-hive]

Security Fix

workflowFenceApproval treated a typed /hive decision approve (or an unmarked dashboard /approve) as bound to the current head when the comment post-dated headPushedAt. headPushedAt serves the min_head_age soak gate and, while that window is open, prefers the head commit's committer date — a value the pusher's git writes. A later push with a backdated commit therefore inherited an approval given on an earlier head and released the workflow-path fence for content the approver never saw.

Why this shape: the soak gate's tolerance for an older committer date is a convenience the operator accepted (it can only shorten a wait), but reusing the same clock as an authorization binding turns that tolerance into a way to re-attribute a person's approval. The new headObservedAt returns only the PR updated_at pinned on the head's first observation — server-side, never earlier than the push — and headPushedAt is left untouched for the gate it was built for. Fail-closed by construction: a restart can only over-hold, and the merger re-approves the head.

The APPROVED-review path (bound by commit_id) and marker-bearing dashboard approvals were already safe and are unchanged.

Test: TestContributorLaneWorkflowFenceApprovalIgnoresCommitterDate runs with MinHeadAge wider than the head's observed age so the committer-date branch is live, a commit claiming 10:00 against a head GitHub first showed at 12:40, and asserts a 12:00 approval leaves the fence armed while a 12:50 one releases it. The first two cases fail without the change.

Fixes #14754

---

Filed by sec-check agent (ACMM L6 — full mode)

— hive: agent=sec-check backend=copilot model=claude-fable-5.1 copilot=1.0.88

[github-actions]

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

PR	Title	Relation	Files
#14822	🐛 fix: preserve owner intent through PR decision paths	same files	src/pkg/github/automerge/automerge_contributor_lane_test.go

[hivecommons-hive]

Decision needed: @clubanderson should confirm that unmarked owner approvals in the contributor lane may now bind to the sweep's first-observed head time (fail-closed: an approval posted between a push and the hive's first sweep of that head, followed by any other updated_at bump, will need to be re-typed) — this changes merge-trust policy for the workflow-path fence.

The code is correct as far as I can verify; routing only because it changes how an owner approval is bound to a head on the workflow-path fence, which is merge-trust policy.

What I checked in the v5 tree:

• The mechanism described in the body is real. headPushedAtWith (src/pkg/github/automerge/automerge_head_age.go:120-154) pins updated_at on first observation, but while min_head_age is open it consults HeadCommitPushedAt and returns committed when committed.Before(pinned). That value is the committer date, so a backdated commit on a later push could make a pre-push /hive decision approve count via !created.Before(headPushedAt) at automerge_contributor_workflow_approval.go:142.
• The new headObservedAt returns only tracker.observe(...) (the pinned updated_at) and never the commit time, so an approval typed before the push can no longer bind. Zero when nothing is pinned, which binds nothing.
• Comparison is !created.Before(...), so an approval that is itself the event that bumped updated_at before the first sweep still binds (equal timestamps pass).
• headPushedAt is untouched and still serves the min_head_age gate; only the approval binding changed.
• TestContributorLaneWorkflowFenceApprovalIgnoresCommitterDate sets committer date 10:00, observed 12:40, approval 12:00 with MinHeadAge: time.Hour, so it exercises the committer-date branch and fails without the fix.

One behaviour change worth stating plainly (info, not a defect): if an owner approves after a push but before the hive first observes the head, and something else (label flip, hive comment) bumps updated_at before that first sweep, the pinned time is later than the approval and the fence stays armed until the owner re-approves. The changelog fragment says as much; I just want the owner to have chosen it.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Decision needed: @clubanderson should decide whether the workflow-path fence may silently discard a legitimate owner approval typed between a push and the hive's first observation of that head (forcing a re-approve), as the price of no longer trusting pusher-written committer dates.

The fix is correct and fail-closed; it also narrows what counts as a valid approval more than the body says, and that is a merge-policy choice for an owner.

What I verified in the tree at the PR head:

• automerge_head_age.go:46-62 — observe() pins the first updated_at the sweep sees for a head and returns it forever after. Server-side, so a pusher cannot move it backwards. ✔
• automerge_contributor_workflow_approval.go:144 — the binding test is !created.Before(headPushedAt), i.e. approval ≥ pinned time counts. With headObservedAt feeding it, a backdated committer date can no longer let an old approval through — the two new negative cases genuinely fail on the old headPushedAt path because headPushedAtWith (:164-173) prefers the committer date while min_head_age is open.
• APPROVED-review path (latestTrustedHumanApproval, by commit_id) and marker-bearing /approve are untouched, as stated.

The gap I want an owner to accept explicitly (low/medium, not a bug in the diff):

The pinned updated_at is "never earlier than the push", but it can be later than a legitimate approval. updated_at is bumped by every comment and label write, and headPushedAtWith's own doc (automerge_head_age.go:146-147) names "a head first seen right after a hive comment/label write" as the normal case. Sequence: owner pushes 12:00 → owner types /hive decision approve 12:03 → hive posts a stage comment / flips a label 12:04 → automerge sweep first observes the head 12:05 with updated_at = 12:04. The 12:03 approval is now Before the pin and is dropped; the fence stays armed with no signal to the approver that their approval was discarded. The old code tolerated this via the committer date; this PR removes that tolerance on purpose, and the body only states the "never earlier" half.

That trade is probably right for a security fence — re-approving is cheap, inheriting an approval is not — but it changes when a maintainer's typed approval counts, so it should be a person's call rather than mine. If accepted, a one-line note in the changelog fragment that an approval typed before the hive first sees the new head must be repeated would save a confused re-approve later.

Checks: only Sign-off survives the squash is CANCELLED (same as the other open v5 PRs this hour — looks like a runner/concurrency cancel, not this diff).

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Needs a decision, not a merge:

• the reviewer swarm returned requires_human @ab64a95
• reviewer question: @clubanderson should decide whether the workflow-path fence may silently discard a legitimate owner approval typed between a push and the hive's first observation of that head (forcing a re-approve), as the price of no longer trusting pusher-written committer dates.
• the automerge lane will not merge this unattended

Head: verdict requires_human @ab64a95, 65/65 checks green, DCO yes, soak 0s of 30m0s.

What to do: approve this head (ab64a95) to merge as-is, or leave a changes requested review (or reply /hive decision <what to change>) and the hive will push the fix.

How to release it:

• After reading the verdict, a trusted merger can submit a GitHub Approve review for the current head and the lane clears the hold on its next pass; /hive approve replies on a PR are acknowledged but not consumed.

_{Clear the cause and the hive re-evaluates on its next pass; the stage moves to stage/090-ready-for-human-merge, stage/095-pre-merge-soak or the merge queue on its own. Approval does not bypass CI, hold labels or other merge gates.}

[clubanderson]

/hive decision approve

Approving head ab64a95. The fence must not trust a pusher-chosen committer date as an authorization binding; binding unmarked owner approvals to the server-side first-observed time is the right fix, and it fails closed (unknown/zero observed time binds nothing, so an approval typed between a push and the hive's first observation just has to be re-typed — an acceptable price). headPushedAt is left alone for the soak gate, and TestContributorLaneWorkflowFenceApprovalIgnoresCommitterDate covers the backdated-commit case in both directions.

[clubanderson]

Owner approval for head ab64a95 — see /hive decision approve above. Fail-closed binding of unmarked approvals to the observed push time, with tests.

[kubestellar-prow]

LGTM label has been added.

Details

Git tree hash: 5d189e10540a5679a628c74e3d9cae7eec9564ef

[hivecommons-hive]

Decision recorded by @clubanderson for head ab64a95. The hive is treating it as a changes requested review: this PR moves to the fixing stage and the author agent pushes a new head following your instruction verbatim. The review swarm then re-reviews that head; if it approves, the PR comes back for your merge approval.

[kubestellar-prow]

New changes are detected. LGTM label has been removed.

[kubestellar-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from clubanderson. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hivecommons-hive]

Reviewed — no findings from this perspective.

— hive: agent=adjudicator-4 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Reviewed (correctness, delta ab64a95→700a640) — no blocking findings.

Delta is documentation only: headObservedAt logic in src/pkg/github/automerge/automerge_head_age.go:118-134 is byte-identical to the prior head; only the doc comment (lines 112-115) and the changelog fragment changed.

Prior finding re-checked — approval typed between push and first sweep is discarded (automerge_head_age.go:134, observe pins PR updated_at on first sight): behaviour still present, but it is the fail-closed choice recorded in #14754 ("Decision taken by hive: fail-closed on the pinned observation time … rather than querying the timeline API") and is now stated in the code comment and changelog. In-contract; the approver re-types. No longer blocks.

— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Reviewed (intent-alignment, delta ab64a95→700a640) — no findings from this perspective.

Prior finding (approval typed between push and first sweep is discarded, automerge_head_age.go:117) is still present in behavior but is now documented as the deliberate fail-closed trade-off in the function comment (automerge_head_age.go:111-115) and the changelog fragment; this matches the decision recorded in #14754 ("fail-closed on the pinned observation time … rather than querying the timeline API"), so it no longer blocks on intent.

— hive: agent=adjudicator-3 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Reviewed — no findings from this perspective.

— hive: agent=adjudicator-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

docs-currency (delta ab64a95→700a640; read at 700a640)

Previous finding (automerge_head_age.go:117, approval typed before first observation discarded): now documented as deliberate fail-closed in the doc comment (automerge_head_age.go:111-115) and the changelog fragment, matching the decision recorded in #14754 — resolved as intended behavior.

One operator-facing doc still describes the old binding:

• src/docs/hive-merge.md:495-496 — "a trusted merger's own /hive decision approve comment posted after the head" — after this PR the comment must post-date the hive's first observation of the head, not the push; an approval typed between push and sweep is treated as stale and must be re-typed (per changelog.d/fixed-workflow-fence-approval-head-binding.md). Suggest: "…posted after the hive first observed the head (an approval typed between the push and the hive's next sweep is treated as stale; re-type it)". src/docs/operator-reference.md:450 ("bound to it") could take the same clause.

— hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Approval recorded by @clubanderson for head 700a640. The hive is treating it as your answer to the review swarm's question: nothing is sent back to the author, and the PR follows the merge path on the next pass (merged when the merge gates pass, or handed to you at the ready-for-merge stage where a person has to press the button).

Recorded from the hive dashboard.

[hivecommons-hive]

Reviewed — no findings from this perspective.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Reviewed — no findings from this perspective.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Reviewed (style) — approve; three low-severity advisories, none blocking.

1. Naming still says headPushedAt on the path the fix moved off it. src/pkg/github/automerge/automerge_contributor_workflow_approval.go:112 binds headPushedAt := c.headObservedAt(...), passes it into workflowApprovalCommentActor(..., headPushedAt time.Time, ...) (line 133), and that function's doc (line 132) still says "posted after the head (headPushedAt, when known)". The whole point of #14754 is that this value is not headPushedAt; a reader of the callee alone will assume the committer-date clock is still in play. Rename the local/param to headObservedAt and update the line-132 comment.

2. Duplicated pin logic. automerge_head_age.go:118-131 (headObservedAt) re-implements the updated_at extraction, tracker selection, now selection and observe call already at automerge_head_age.go:156-168 (headPushedAtWith). A small shared helper (e.g. pinnedHeadObservedAt(tracker, key, pr, now)) returning the pin would let headPushedAtWith build on it and keep the two clocks visibly derived from one source.

3. Changelog fragment has no issue link. changelog.d/fixed-workflow-fence-approval-head-binding.md:1 omits #14754, while changelog.d/README.md ("what changed, why an operator cares, issue links") and the two doc edits in this PR (src/docs/hive-merge.md:501, src/docs/operator-reference.md:450) all cite it.

— hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Reviewed — no findings from this perspective.

— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Reviewed (docs-currency) — one wording-accuracy note, non-blocking.

• src/docs/hive-merge.md:498-500 says an unmarked approval "typed between the push and that sweep is judged stale and must be typed again". The code is less strict than that: HeadFirstSeenTracker.observe (automerge_head_age.go:46-60) pins the PR updated_at on first sight, and the approval comment is itself a mutation that moves updated_at to its own created_at; workflowApprovalCommentActor (automerge_contributor_workflow_approval.go:144) accepts !created.Before(headPushedAt), so an approval that is the last mutation before the first sweep still releases the fence. It is only stale when another comment/label lands after it and before the sweep. The changelog fragment states the mechanism correctly ("any comment or label that lands between the push and that sweep moves it later"); suggest mirroring that qualifier in hive-merge.md and operator-reference.md:450 so operators aren't told to always re-type.

Code/doc parity otherwise verified: headObservedAt zero → binds nothing (automerge_contributor_workflow_approval.go:144), headPushedAt untouched for the soak gate, 085 note at automerge_stage_human.go:206 remains accurate.

— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Evidence bundle `#14758244c091` sha256:18e4641cdecb824e5f773e8b9bb484297bc6c2f423d7f7d88b1e8a4c4c5aec35 — download from the dashboard or `hivectl review evidence #14758`.

[github-actions]

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback