Skip to content

fix(advisory): rank a withdrawn finding below every finding nobody withdrew - #6098

Merged
kubestellar-prow[bot] merged 1 commit into
hivecommons:v4from
Danathar:fix/2364-retracted-findings
Sep 8, 2026
Merged

fix(advisory): rank a withdrawn finding below every finding nobody withdrew#6098
kubestellar-prow[bot] merged 1 commit into
hivecommons:v4from
Danathar:fix/2364-retracted-findings

Conversation

@Danathar

@Danathar Danathar commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

The current digest on #2364 — updated 2026-09-05 13:32 UTC — renders 10 findings
out of 292, and one of its five CRITICAL slots is held by a finding the
reporting agent has already withdrawn in the finding's own detail
:

[regression-risk] heartbeatBearerOK per-hive binding removed without covering test

REVISED: Per-hive identity binding was MOVED from heartbeatBearerOK to
verifyHeartbeatBearer in hub_keys.go, not removed. … The change is
refactoring, not a security regression. Downgrading priority.

"Downgrading priority" never reached the bead. The agent edited the notes;
beadPriorityToSeverity reads b.Priority; so the finding goes on rendering at
critical and winning a slot every cycle. The maintainer working this tracker
wrote it up in their 2026-09-04 comment
as "refuted by its own detail line and should be closed rather than fixed — its
bead is still open at critical, which is why it keeps consuming a slot"
.

#5945 already taught applyTopN to demote on the three
nobody-re-checked-this signals. A retraction is a different kind of thing: the
one party who ever looked went back, looked again, and said it does not stand.
That was the only such signal the ranking ignored.

The one demotion that crosses severity bands

#5945's signals stay inside their band deliberately — unverified means nobody
re-checked, so an unverified critical may still be the worst thing in the report.
A retraction inverts that: the filed severity is the one claim its author no
longer makes.

It is demoted, not dropped. The bead is still open and only a maintainer
closes it, so a withdrawn finding still backfills any slot no live finding
claims, and arrives carrying a caption saying the bead wants closing rather
than fixing
.

Two signals are required, and that is the safety argument

REVISED: alone is not a withdrawal — an agent raising a finding to critical
writes the same lead-in, and demoting that below every other finding would bury
the most urgent item in the report under a caption telling the reader to
disregard it. So a finding is retracted only when its detail opens with a
revision marker
and says somewhere that it no longer stands.

The asymmetry is on purpose: missing a real retraction costs nothing beyond
today's behaviour; inventing one costs a live critical its slot.

What mutation testing found

revision was in the marker vocabulary at first. It is also this package's
provenance idiom
provenanceRefPattern matches revision <sha>, and
provenance_test.go builds a fixture exactly that way. Under a one-signal rule
that fixture's #5130 stale-provenance caption silently became a withdrawal
caption.

revision is now out of the marker list (revised is not), and both the
collision and the reason it exists are pinned by a test, because the next person
adding a marker word has no way to know the two vocabularies overlap.

Relationship to #6093

I have #6093 open on the same
package. They are complementary, not overlapping:

signal outcome
#6093 finding is provenance-stale and names GitHub work that has closed moved to Recently Resolved
this PR the reporting agent withdrew it in the finding's own detail ranked last, captioned, stays open

The heartbeatBearerOK finding names no issue, so #6093 cannot reach it. I
verified the two branches auto-merge cleanly and that go test ./pkg/advisory/...
passes on the merged result, then aborted the probe merge. Neither depends on the
other; they can land in either order.

On closing #2364

Deliberately not claimed. #2364 is a standing living document — its body says
"Do not close this issue", and a maintainer already
removed help wanted
from it for precisely that reason. This fixes the defect its current digest
exhibits, so the commit says Refs #2364, not Closes.

What else I checked in that digest, and did not change

Reviewing the other nine shown findings, none is a live unaddressed defect:

I am reporting the #3344 verification rather than acting on it: closing someone
else's security-shaped bead is a maintainer call, and this PR's mechanism does not
cover it.

Validation

Command Outcome
go test ./pkg/advisory/... pass
go build ./... clean
go vet ./pkg/advisory/... clean
gofmt -l on the changed files clean
merge probe against #6093 + go test clean merge, tests pass
changelog-fragment-guard rules, applied by hand filename and - shape both match

CI itself has not run on this PR: every workflow sits at action_required,
this repo's gating for a fork contributor. DCO and PR Verifier are the only
ones that run.

The tests were verified to discriminate — each rule removed, suite re-run:

Mutation Result
retracted findings not set aside (pre-fix behaviour) 3 fail, incl. the bead-level end-to-end
second signal dropped (marker alone retracts) 4 of mine fail plus the pre-existing TestFormatDigestMarkdownCaptionsStaleProvenance
marker no longer anchored to the start of the detail 1 fails — "revised" mid-sentence
withdrawal caption removed 1 fails

That second row is the one worth reading: a pre-existing test catching the loose
rule is what surfaced the revision/provenance collision above.

Skipped, stated rather than implied

  • No digest was generated against real GitHub. This is verified at the
    ranking and rendering level; nothing was run against the live 🐝 Hive Advisory Report #2364 comment.
  • The detection rule is prose matching over agent-written text. It will not catch
    a retraction phrased outside its vocabulary — that case keeps today's behaviour,
    which is the direction it is built to fail in.
  • Only pkg/advisory was run, plus a whole-module go build. Five packages fail
    in my sandbox on a clean v4 tree for environmental reasons, confirmed on an
    earlier task in this repo.

External state

None. No issues, comments, or workflow runs were created or mutated.

Refs #2364

— hive: backend=claude model=claude-opus-5

…thdrew

The 2026-09-05 digest on hivecommons#2364 rendered 10 findings out of 292, and one of its
five CRITICAL slots was held by a finding the reporting agent had already
withdrawn in the finding's own detail:

  [regression-risk] heartbeatBearerOK per-hive binding removed without
  covering test
  > REVISED: ... not removed ... The change is refactoring, not a security
  > regression. Downgrading priority.

"Downgrading priority" never reached the bead. The agent edited the notes;
beadPriorityToSeverity reads b.Priority; so the finding went on rendering at
critical and winning a slot every cycle. The maintainer working the tracker
wrote it up as "refuted by its own detail line and should be closed rather than
fixed -- its bead is still open at critical, which is why it keeps consuming a
slot".

hivecommons#5945 had already taught applyTopN to demote on the three
nobody-re-checked-this signals. A retraction is a different kind of thing: the
one party who ever looked went back, looked again, and said it does not stand.
That was the only such signal the ranking ignored.

THE ONE DEMOTION THAT CROSSES SEVERITY BANDS. hivecommons#5945's signals stay inside their
band on purpose -- unverified means nobody re-checked, so an unverified critical
may still be the worst thing in the report. A retraction inverts that: the
filed severity is the one claim its author no longer makes. It still backfills
an unclaimed slot rather than being dropped, because the bead is open and only a
maintainer closes it, and it arrives carrying a caption saying the bead wants
closing rather than fixing.

TWO SIGNALS ARE REQUIRED, and the asymmetry is the safety argument. "REVISED:"
alone is not a withdrawal -- an agent raising a finding to critical writes the
same lead-in, and demoting THAT below every other finding would bury the most
urgent item in the report under a caption telling the reader to disregard it. So
a finding is retracted only when its detail opens with a revision marker AND
says somewhere that it no longer stands. Missing a real retraction costs nothing
beyond today's behaviour; inventing one costs a live critical its slot.

Mutation testing found a collision worth recording: "revision" was in the marker
vocabulary at first, and it is also this package's PROVENANCE idiom --
provenanceRefPattern matches "revision <sha>", and provenance_test.go builds a
fixture exactly that way. Under a one-signal rule that fixture's hivecommons#5130
stale-provenance caption became a withdrawal caption. "revision" is now out of
the marker list, "revised" is not, and both the collision and the reason are
pinned by a test.

Closes hivecommons#2364 is deliberately NOT claimed: hivecommons#2364 is a standing living document
("Do not close this issue"), and a maintainer has already removed `help wanted`
from it for that reason. This fixes the defect its current digest exhibits.

Refs hivecommons#2364

Signed-off-by: Douglas Baggett <doug.baggett@gmail.com>
@kubestellar-prow kubestellar-prow Bot added dco-signoff: yes Indicates the PR's author has signed the DCO. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 5, 2026
@kubestellar-hive

Copy link
Copy Markdown
Contributor

Scanner review — no findings

Traced applyTopN for comparator/off-by-one issues: retracted findings are set aside before per-band unverified backfill, cross-band backfill preserves rank order, the len(kept) == cap early-break only skips collecting retracted findings when backfill would never run, overflow accounting unchanged, and the len(all) <= cap early return still gets the withdrawal caption via the independent findingRetracted check. go test ./pkg/advisory/ passes.


Advisory review by scanner agent (ACMM L5 — hold-gated mode). Not a merge gate.

🐝 Hive Agent: scanner | Instance: hosted-available-oke-11-placeholder-r05x | SHA: unknown

— hive: agent=scanner backend=copilot model=claude-fable-5

@Danathar

Danathar commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/approve

@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Danathar

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@kubestellar-prow kubestellar-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 7, 2026
@clubanderson clubanderson added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2026
@kubestellar-prow
kubestellar-prow Bot merged commit 38d0298 into hivecommons:v4 Sep 8, 2026
45 of 46 checks passed
kubestellar-hive Bot pushed a commit that referenced this pull request Sep 8, 2026
…thdrew (#6098)

The 2026-09-05 digest on #2364 rendered 10 findings out of 292, and one of its
five CRITICAL slots was held by a finding the reporting agent had already
withdrawn in the finding's own detail:

  [regression-risk] heartbeatBearerOK per-hive binding removed without
  covering test
  > REVISED: ... not removed ... The change is refactoring, not a security
  > regression. Downgrading priority.

"Downgrading priority" never reached the bead. The agent edited the notes;
beadPriorityToSeverity reads b.Priority; so the finding went on rendering at
critical and winning a slot every cycle. The maintainer working the tracker
wrote it up as "refuted by its own detail line and should be closed rather than
fixed -- its bead is still open at critical, which is why it keeps consuming a
slot".

#5945 had already taught applyTopN to demote on the three
nobody-re-checked-this signals. A retraction is a different kind of thing: the
one party who ever looked went back, looked again, and said it does not stand.
That was the only such signal the ranking ignored.

THE ONE DEMOTION THAT CROSSES SEVERITY BANDS. #5945's signals stay inside their
band on purpose -- unverified means nobody re-checked, so an unverified critical
may still be the worst thing in the report. A retraction inverts that: the
filed severity is the one claim its author no longer makes. It still backfills
an unclaimed slot rather than being dropped, because the bead is open and only a
maintainer closes it, and it arrives carrying a caption saying the bead wants
closing rather than fixing.

TWO SIGNALS ARE REQUIRED, and the asymmetry is the safety argument. "REVISED:"
alone is not a withdrawal -- an agent raising a finding to critical writes the
same lead-in, and demoting THAT below every other finding would bury the most
urgent item in the report under a caption telling the reader to disregard it. So
a finding is retracted only when its detail opens with a revision marker AND
says somewhere that it no longer stands. Missing a real retraction costs nothing
beyond today's behaviour; inventing one costs a live critical its slot.

Mutation testing found a collision worth recording: "revision" was in the marker
vocabulary at first, and it is also this package's PROVENANCE idiom --
provenanceRefPattern matches "revision <sha>", and provenance_test.go builds a
fixture exactly that way. Under a one-signal rule that fixture's #5130
stale-provenance caption became a withdrawal caption. "revision" is now out of
the marker list, "revised" is not, and both the collision and the reason are
pinned by a test.

Closes #2364 is deliberately NOT claimed: #2364 is a standing living document
("Do not close this issue"), and a maintainer has already removed `help wanted`
from it for that reason. This fixes the defect its current digest exhibits.

Refs #2364

Signed-off-by: Douglas Baggett <doug.baggett@gmail.com>
Signed-off-by: sec-check <sec-check@hive.kubestellar.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants