Skip to content

Post ClawSweeper Endor review alerts through Hermit - #30

Draft
jesse-merhi wants to merge 2 commits into
mainfrom
jesse/endor-notifications
Draft

Post ClawSweeper Endor review alerts through Hermit#30
jesse-merhi wants to merge 2 commits into
mainfrom
jesse/endor-notifications

Conversation

@jesse-merhi

@jesse-merhi jesse-merhi commented Aug 25, 2026

Copy link
Copy Markdown
Member

ClawSweeper has no strict way to send completed Endor remediation reviews through Hermit, so security results cannot reach a dedicated Discord channel with durable deduplication. Retrying delivery could otherwise create duplicate or mismatched messages. This PR adds an authenticated, schema-validated Hermit route with a server-owned channel and a D1 receipt. The same terminal event now yields one Carbon message and a stable Discord message receipt.

Change breakdown

Part Files +LOC -LOC
Implementation and migration 9 +2,630 -0
Tests 1 +310 -0
Documentation 1 +6 -0
Total 11 +2,946 -0

Proof

Before: direct base

POST /api/clawsweeper/endor-remediation/reviewed
404 route not found
D1 delivery receipt: absent

After: PR

first request:    200 delivered=true duplicate=false messageId=123456789012345678
same-event retry: 200 delivered=true duplicate=true  messageId=123456789012345678
D1 receipt: status=delivered, same idempotency key and Discord message ID
Discord request: fixed configured channel, allowed mentions empty, enforce_nonce=true

The controlled proof uses the real request handler, SQLite-backed D1 adapter, Carbon serializer, and Discord request shape. It does not send to live Discord because production configuration was deliberately left unchanged.

Linux proof passed 8/8 tests on Hetzner Crabbox lease cbx_cac45a64e489.

How to verify

  1. Run bun test tests/clawsweeperEndorApi.test.ts.
  2. Run bun run typecheck and bun run deploy:dry-run.
  3. Run bunx drizzle-kit check.

Implementation notes

Apply migration 0013_many_chameleon.sql, set CLAWSWEEPER_ENDOR_DISCORD_CHANNEL_ID and CLAWSWEEPER_HERMIT_TOKEN, then deploy Hermit before enabling openclaw/clawsweeper#1241. OpenClaw Bay is unaffected because this is an internal notification path.

@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

Comment thread src/clawsweeperEndor/api.ts Fixed
@clawsweeper clawsweeper Bot added merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 25, 2026
@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed August 24, 2026, 10:03 PM ET / August 25, 2026, 02:03 UTC.

ClawSweeper review

What this changes

Adds an authenticated Hermit endpoint that records and deduplicates ClawSweeper Endor review alerts before posting a Carbon Discord message to a configured channel.

Merge readiness

⚠️ Ready for maintainer review - 6 items remain

Keep this MEMBER-authored draft open: current main has no equivalent endpoint, but the new Discord authority path needs an explicit no-mentions fix and maintainer acceptance of the permanent integration boundary.

Priority: P1
Reviewed head: 3931677bd6d1b4822ce6c7a787b83c8720eb62eb
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The design and controlled coverage are substantial, but a focused security-boundary correction is required before merge.
Proof confidence 🌊 off-meta tidepool Not applicable: This MEMBER-authored draft is exempt from the external contributor proof gate; its controlled handler/D1/Discord-shape test is useful supplemental evidence, but does not prove the required mention-suppression repair.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This MEMBER-authored draft is exempt from the external contributor proof gate; its controlled handler/D1/Discord-shape test is useful supplemental evidence, but does not prove the required mention-suppression repair.
Evidence reviewed 6 items Current-main gap: Current main has no ClawSweeper Endor endpoint; this PR introduces the endpoint, receipt table, and root route registration.
Mention safety defect: The endpoint renders accepted review text but does not use the repository's explicit no-mention parse policy.
Existing safe convention: Hermit's shared webhook sender explicitly uses parse: [] when it must suppress Discord mention parsing.
Findings 1 actionable finding [P1] Disable parsed mentions for incoming review text
Security Needs attention Incoming review text can trigger broad Discord mentions: Titles and summaries are accepted from the bearer-authenticated caller and rendered in the channel; the payload must use an explicit empty parse list to prevent @everyone/@here notifications.

Live Verification

Command: bun test tests/clawsweeperEndorApi.test.ts

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.23.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.23.0.tgz

Assertions:

  • FAIL expect_output: ClawSweeper Endor notification API

How this fits together

Hermit is a Cloudflare Worker that receives HTTP requests before ordinary Discord interaction handling. The endpoint validates a ClawSweeper event, records a D1 receipt, then sends a Carbon message to one configured Discord channel.

flowchart LR
  A[ClawSweeper review event] --> B[Hermit HTTP endpoint]
  B --> C[Token and schema validation]
  C --> D[D1 delivery receipt]
  D --> E[Carbon message builder]
  E --> F[Configured Discord channel]
Loading

Decision needed

Question Recommendation
Should Hermit permanently own this token-authenticated ClawSweeper-to-Discord notification endpoint and its dedicated D1 receipt contract? Approve the scoped Hermit integration: After the mention-suppression fix and rollout configuration are verified, retain Hermit as the owner of this alert path.

Why: This adds a cross-repository authority boundary, persistent storage, two production secrets, and a server-owned notification channel; that scope needs explicit acceptance beyond the narrow code repair.

Before merge

  • Disable parsed mentions for incoming review text (P1) - The authenticated payload includes PR titles and review summaries, but this object omits parse: []; empty roles/users lists do not explicitly disable @everyone/@here parsing. Use the repository’s no-mention pattern and add a regression assertion so a valid alert cannot notify unintended principals.
  • Resolve security concern: Incoming review text can trigger broad Discord mentions - Titles and summaries are accepted from the bearer-authenticated caller and rendered in the channel; the payload must use an explicit empty parse list to prevent @everyone/@here notifications.
  • Resolve merge risk (P1) - Accepted review text can contain @everyone or @here; without an explicit empty parse list, the new bearer-authenticated endpoint can create unintended broad Discord notifications.
  • Resolve merge risk (P1) - The D1 migration and new production secrets must be configured before this new delivery path is enabled.
  • Complete next step (P2) - A MEMBER-authored draft adds a lasting cross-repository authority boundary and needs explicit product acceptance after the concrete mention-suppression fix.

Findings

  • [P1] Disable parsed mentions for incoming review text — src/clawsweeperEndor/api.ts:337-340
  • [medium] Incoming review text can trigger broad Discord mentions — src/clawsweeperEndor/api.ts:339
Agent review details

Security

Needs attention: The new authenticated route reaches Discord with caller-provided text but does not explicitly disable all mention parsing.

Review metrics

Metric Value Why it matters
Patch composition production +525, generated migration metadata +2,111, tests +310 The new HTTP-to-Discord authority path is substantial and includes persisted-schema metadata plus focused coverage.

Merge-risk options

Maintainer options:

  1. Disable every parsed mention before merge (recommended)
    Use an explicit empty parse list and cover @everyone/@here so review text cannot notify unintended Discord principals.
  2. Pause the cross-repository route
    Keep the draft unmerged if maintainers do not want Hermit to hold this notification authority and persistent receipt surface.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Use an explicit empty allowed-mentions parse list and add focused regression coverage for @everyone and @here in incoming review text.

Technical review

Best possible solution:

Retain the server-owned channel and durable receipt design, explicitly suppress all parsed mentions with regression coverage, then land it only if maintainers want Hermit to own this ClawSweeper notification boundary.

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

Yes—source inspection gives a current-head path: send a valid authenticated notification containing @everyone or @here and observe that the handler forwards it without an explicit empty parse list.

Is this the best way to solve the issue?

No—the endpoint and receipt design are coherent, but the final Discord payload must explicitly suppress all parsed mentions before this authority boundary is safe to merge.

Full review comments:

  • [P1] Disable parsed mentions for incoming review text — src/clawsweeperEndor/api.ts:337-340
    The authenticated payload includes PR titles and review summaries, but this object omits parse: []; empty roles/users lists do not explicitly disable @everyone/@here parsing. Use the repository’s no-mention pattern and add a regression assertion so a valid alert cannot notify unintended principals.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 17a621b47dba.

Labels

Label changes:

  • add P1: The new security-alert delivery path can turn accepted review text into an unintended broad Discord notification.
  • add merge-risk: 🚨 security-boundary: A bearer-authenticated external request controls text that reaches a Discord side effect, and mention parsing is not explicitly disabled.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This MEMBER-authored draft is exempt from the external contributor proof gate; its controlled handler/D1/Discord-shape test is useful supplemental evidence, but does not prove the required mention-suppression repair.

Label justifications:

  • P1: The new security-alert delivery path can turn accepted review text into an unintended broad Discord notification.
  • merge-risk: 🚨 security-boundary: A bearer-authenticated external request controls text that reaches a Discord side effect, and mention parsing is not explicitly disabled.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This MEMBER-authored draft is exempt from the external contributor proof gate; its controlled handler/D1/Discord-shape test is useful supplemental evidence, but does not prove the required mention-suppression repair.

Evidence

Security concerns:

  • [medium] Incoming review text can trigger broad Discord mentions — src/clawsweeperEndor/api.ts:339
    Titles and summaries are accepted from the bearer-authenticated caller and rendered in the channel; the payload must use an explicit empty parse list to prevent @everyone/@here notifications.
    Confidence: 0.94

What I checked:

Likely related people:

  • jesse-merhi: Authored the prior merged publisher-abuse endpoint and this draft's commits. (role: endpoint-pattern contributor; confidence: high; commits: 34b654fcb707, 3931677bd6d1; files: src/clawhubPublisherAbuse/api.ts, src/clawsweeperEndor/api.ts)
  • Hannes Rudolph: Recent main history shows sustained work on Hermit's root request-routing surface. (role: recent request-routing contributor; confidence: medium; commits: 7de378b8dc60, 11ce657eb1c9; files: src/index.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Set allowed mentions to an explicit empty parse list and prove @everyone/@here remain inert in the serialized Discord request.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants