fix(repair): require a trusted status comment author

[dwin-gharibi]

Fixes #1072.

Both status-comment consumers previously accepted a missing author through !author. Share a fail-closed comparator: an unreadable or empty login cannot authorize status selection, acknowledgement recovery, or canonical-landing status evaluation. Preserve the router's configurable bot set, the executor's fixed bot set, and case-insensitive matching without trimming.

This rewrites the original proposal on current main while preserving @dwin-gharibi's commit ancestry and credit. The final patch is 61 insertions / 10 deletions across five files, including focused tests and the active trust documentation. The historical proof-only scaffold is removed from the proposed tree. Existing marker, receipt, freshness, and callback gates remain intact.

OpenClaw Bay impact: no status schema, observer API, or UI change. A genuine bot comment with lost author metadata is deliberately ignored and may lead to a replacement status comment. No new dependency or changelog change.

Controlled real behavior proof

Candidate 44dd657628dc7664e2074d025354c036986dd25a was compared with current-main base 7cc443fe363386e197a0b19fde0c4b534df6c943 on Node 24.20.0. The same probe exercised 12 author scenarios and 103 built-production boundary calls per phase. The candidate proof was replayed after committing, and source hashes still match the reviewed and remotely checked files.

The command modules have executable top levels, so temporary adapters export their exact compiled private function bodies without running the CLI. Public readiness and receipt helpers are imported directly. The probe intercepts network/storage I/O and records selections, return values and callbacks; it does not replace the author comparator.

Input	Before	After
Missing/null user, missing/null/empty login	Selected newest status 11; recovered acknowledgement; update callback reached; needs-human status accepted	Selected older trusted status 10; recovery false; skipped_untrusted; needs-human status rejected
Built-in trusted logins, including mixed case	Status11 selected, acknowledgement recovered	Same
Router custom bot	Accepted by router, rejected by executor	Same
Unknown or padded login	Rejected	Same
Wrong marker/item or stale receipt/head	Existing negative controls hold	Same

The exact-head merge-readiness negative controls were already waiting before the change and remain waiting. This proof does not claim a demonstrated unauthorized merge or live exploit. No live GitHub mutation, workflow dispatch, or merge was performed by the probe.

The shared predicate can also be exercised directly after pnpm run build:node:

import assert from 'node:assert/strict';
import { isTrustedStatusCommentAuthor } from './dist/repair/comment-router-core.js';
const bots = new Set(['', 'clawsweeper[bot]']);
assert.equal(isTrustedStatusCommentAuthor({}, bots), false);
assert.equal(isTrustedStatusCommentAuthor({user:{login:''}}, bots), false);
assert.equal(isTrustedStatusCommentAuthor({user:{login:'ClawSweeper[bot]'}}, bots), true);
assert.equal(isTrustedStatusCommentAuthor({user:{login:' clawsweeper[bot] '}}, bots), false);

Validation

• Node 24 builds and 167 focused tests passed.
• Full pnpm run check: 4,253 passed, 14 skipped, zero failures, plus 13/13 changed-coverage tests.
• AWS Crabbox run_ad9347037c13, lease cbx_30cef78d916c, image ami-0461d919be7deb53c, Node 24.18.1, pnpm 11.10.0. The tested candidate files match this committed head; the lease is released.
• Managed Codex reviews before commit and on the committed branch both found no accepted/actionable P0 findings. Current-head CI is required before landing.

[Copilot]

Pull request overview

This PR fixes a fail-open condition in the repair lane’s “trusted status comment” guard by ensuring that an unreadable/absent comment author is not treated as trusted. It centralizes the trust check into a single exported helper in comment-router-core.ts, updates both prior call sites to use it, and adds focused regression coverage plus a proof contract.

Changes:

• Introduce isTrustedStatusCommentAuthor(...) that trims/normalizes the login and fails closed when the author cannot be read.
• Replace the two prior private predicate copies in comment-router.ts and execute-fix-artifact.ts with the shared helper (preserving the hardcoded repair trusted authors set).
• Add unit tests and a reproducible proof package under docs/proof/trusted-status-comment-fail-open/.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/repair/comment-router-core.ts	Adds the shared exported trust comparator that fails closed on missing authors.
src/repair/comment-router.ts	Switches status-comment trust to the shared comparator using trustedBots.
src/repair/execute-fix-artifact.ts	Switches status-comment trust to the shared comparator with a fixed trusted-author set.
test/repair/comment-router-core.test.ts	Adds behavioral coverage for unreadable authors and trusted logins.
docs/proof/trusted-status-comment-fail-open/run-proof.sh	Adds container proof harness to build, extract pre-fix source, and run proof + focused tests.
docs/proof/trusted-status-comment-fail-open/run-proof.mjs	Adds proof script validating fail-closed behavior, no-loss, and deduplication in dist/.
docs/proof/trusted-status-comment-fail-open/README.md	Documents the proof contract, claims, commands, and limits.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 1, 2026, 5:09 AM ET / 09:09 UTC.

ClawSweeper review

What this changes

The branch shares a status-comment author predicate between the repair router and executor so missing, empty, unknown, and padded author logins cannot authorize status adoption or updates.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep this PR open: current main still accepts unreadable status-comment authors, while the reviewed head replaces both consumers with a shared fail-closed predicate for the linked security issue #1072. The bounded current-main integration delta is coherent and no blocking defect was found.

Priority: P3
Reviewed head: 44dd657628dc7664e2074d025354c036986dd25a

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused security hardening with substantial exact-head behavior evidence and no supported blocking finding.
Proof confidence	🦞 diamond lobster (5/6) ✨ media proof bonus	Sufficient (linked_artifact): The changed production owners are the router and executor status selectors; the exact-head controlled Crabbox probe exercised their compiled function bodies with missing, trusted, custom, padded, and stale inputs and recorded that missing authors were skipped before recovery or update callbacks. The evidence intentionally intercepts external I/O to observe the no-mutation boundary and does not claim a live GitHub mutation or exploit.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (linked_artifact): The changed production owners are the router and executor status selectors; the exact-head controlled Crabbox probe exercised their compiled function bodies with missing, trusted, custom, padded, and stale inputs and recorded that missing authors were skipped before recovery or update callbacks. The evidence intentionally intercepts external I/O to observe the no-mutation boundary and does not claim a live GitHub mutation or exploit.
Evidence reviewed	6 items	Current main remains fail-open: The fetched current-main revision still returns true when the normalized author login is empty in both status-comment consumers, so the linked issue is not already implemented on main. Shared fail-closed guard: The head's shared predicate requires a nonempty author before matching the built-in identity or consumer-provided allowlist. Both mutation-path consumers use the guard: The router and repair executor now delegate status trust decisions to the same comparator before selecting or updating status comments.
Findings	None	None.
Security	None	None.

How this fits together

Repair automation reads GitHub issue comments to find durable status messages before routing commands or updating repair/automerge status. The router and repair executor each consume those comments and can otherwise send a GitHub comment mutation downstream.

flowchart LR
  A[GitHub issue comments] --> B[Author identity check]
  B -->|trusted| C[Router status selection]
  B -->|trusted| D[Repair executor]
  B -->|missing or untrusted| E[Skip adoption and recovery]
  C --> F[GitHub status comment update]
  D --> F

Loading

Before merge

• Resolve merge risk (P1) - A genuine status comment returned by GitHub without readable author metadata will now be ignored and may be replaced rather than updated; this deliberate fail-closed behavior is preferable to adopting an unauthenticated comment.
• Complete next step (P2) - No repair finding remains; this PR needs ordinary maintainer landing review rather than an automated repair job.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Current-main integration delta	production +17/-10, docs +5, tests +39; 5 files	The merge-parent delta is focused on the two status-comment consumers, their shared helper, documentation, and focused regression coverage.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #1072
Summary: This PR explicitly targets the still-open fail-open status-author issue and current main still contains that behavior.

Members:

• canonical: Trusted status comment fails open on an unreadable author #1072 - The issue describes the same two fail-open status-comment consumers that this head replaces with the shared guard.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Accept fail-closed status recovery (recommended)
   Land the change with the documented behavior that an authorless status comment is replaced rather than adopted.
2. Pause for a different recovery policy
   Keep the PR open if maintainers want a separate authenticated recovery mechanism for authorless GitHub comments.

Technical review

Best possible solution:

Retain one fail-closed comparator for both consumers, keep their distinct allowlists, and land the narrowly scoped hardening after normal maintainer review.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main's two status predicates explicitly authorize an empty normalized author, and the exact-head proof exercises missing-author inputs through the compiled consumers.

Is this the best way to solve the issue?

Yes. A shared nonempty-author predicate removes the duplicated fail-open condition while preserving separate router and executor allowlists and untrimmed case-insensitive matching.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1d79ae6c9f2d.

Labels

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The changed production owners are the router and executor status selectors; the exact-head controlled Crabbox probe exercised their compiled function bodies with missing, trusted, custom, padded, and stale inputs and recorded that missing authors were skipped before recovery or update callbacks. The evidence intentionally intercepts external I/O to observe the no-mutation boundary and does not claim a live GitHub mutation or exploit.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The changed production owners are the router and executor status selectors; the exact-head controlled Crabbox probe exercised their compiled function bodies with missing, trusted, custom, padded, and stale inputs and recorded that missing authors were skipped before recovery or update callbacks. The evidence intentionally intercepts external I/O to observe the no-mutation boundary and does not claim a live GitHub mutation or exploit.
• remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

• P3: This hardens a latent trust default with no demonstrated live exploit or current user outage.
• merge-risk: 🚨 security-boundary: The PR changes the authorization predicate that determines whether repair automation may adopt and update status comments.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The changed production owners are the router and executor status selectors; the exact-head controlled Crabbox probe exercised their compiled function bodies with missing, trusted, custom, padded, and stale inputs and recorded that missing authors were skipped before recovery or update callbacks. The evidence intentionally intercepts external I/O to observe the no-mutation boundary and does not claim a live GitHub mutation or exploit.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owners are the router and executor status selectors; the exact-head controlled Crabbox probe exercised their compiled function bodies with missing, trusted, custom, padded, and stale inputs and recorded that missing authors were skipped before recovery or update callbacks. The evidence intentionally intercepts external I/O to observe the no-mutation boundary and does not claim a live GitHub mutation or exploit.

Evidence

What I checked:

• Current main remains fail-open: The fetched current-main revision still returns true when the normalized author login is empty in both status-comment consumers, so the linked issue is not already implemented on main. (src/repair/comment-router.ts:5451, 1d79ae6c9f2d)
• Shared fail-closed guard: The head's shared predicate requires a nonempty author before matching the built-in identity or consumer-provided allowlist. (src/repair/comment-router-core.ts:825, 44dd657628dc)
• Both mutation-path consumers use the guard: The router and repair executor now delegate status trust decisions to the same comparator before selecting or updating status comments. (src/repair/execute-fix-artifact.ts:4590, 44dd657628dc)
• Focused regression coverage: Tests cover absent and empty metadata, unknown and padded names, mixed-case trusted names, and distinct router/executor allowlists. (test/repair/comment-router-core.test.ts:89, 44dd657628dc)
• Integration provenance and bounded delta: The current head is a merge authored by Peter Steinberger; its current-main parent is 7cc443f and its first-parent delta is five files with 61 additions and 10 deletions. (src/repair/comment-router-core.ts:825, 44dd657628dc)
• Controlled behavior proof: The PR body records an exact-head Crabbox run using compiled production function bodies across 12 author scenarios and 103 boundary calls per phase; absent author metadata selected the older trusted status, skipped acknowledgement recovery, and did not reach the update callback. (test/repair/comment-router-core.test.ts:89, 44dd657628dc)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (5 earlier review cycles)

• reviewed 2026-08-08T08:56:12.272Z sha be849eb :: needs changes before merge. :: [P2] Reject padded author logins | [P3] Soften the orphan-status guarantee
• reviewed 2026-08-08T17:17:11.151Z sha 8f4be80 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-12T11:58:48.748Z sha dc1b5e1 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-12T12:36:29.109Z sha 307173f :: needs changes before merge. :: [P2] Remove the full pre-fix source snapshot
• reviewed 2026-08-19T06:18:03.244Z sha b3ecd01 :: needs real behavior proof before merge. :: [P2] Stage the fixture before the Docker proof run

[dwin-gharibi]

@steipete

[steipete]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Failed
• Detail: The exact review could not verify the complete source. The durable queue will not retry this unchanged revision.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/33479986967
• Updated: 2026-09-01T07:00:35.277Z

[steipete]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/33490868221
• Updated: 2026-09-01T09:11:17.199Z