Skip to content

fix(review): restrict hosted reviews to public targets - #1292

Draft
vincentkoc wants to merge 1 commit into
mainfrom
fix/hosted-public-targets
Draft

fix(review): restrict hosted reviews to public targets#1292
vincentkoc wants to merge 1 commit into
mainfrom
fix/hosted-public-targets

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Aug 29, 2026

Copy link
Copy Markdown
Member

What Problem This Solves

Fixes an issue where hosted review eligibility was inconsistent across event
intake, maintainer commands, scheduled fanout, lifecycle acknowledgements, and
resumed publication. Private, internal, unavailable, and out-of-scope
repositories could reach parts of the hosted lifecycle before the final target
boundary was known.

Why This Change Was Made

Centralizes hosted-target eligibility and fresh public-visibility admission
before target credentials, acknowledgements, durable state, dispatch, or
publication. Hosted scope is limited to explicitly configured repositories and
the approved public openclaw/* and steipete/* owner fallbacks; explicit
denials override those fallbacks. Arbitrary public repositories are rejected.

Lifecycle and Bay acknowledgements pass both Worker admission and a queue-side
fresh visibility recheck immediately before persistence. Private and internal
review remains local-only. Author-wide repair remains limited to explicitly
configured public repositories.

This is the best fix because the admission result is owned once and enforced at
each durable lifecycle boundary. Intake-only validation would leave scheduled,
resumed, fanout, acknowledgement, and publication paths able to bypass the same
invariant.

User Impact

External contributors can use hosted ClawSweeper review on eligible public
repositories. Private/internal repositories and unconfigured repositories
outside the approved owner fallbacks receive no hosted processing or target
acknowledgement; maintainers can review them locally.

During GitHub metadata outages, affected hosted work deliberately fails closed
and may be delayed until visibility can be verified.

OpenClaw Bay Impact

Bay keeps the same observer-only contract. Its lifecycle completion projection
now persists only after the target passes the same fresh hosted-public boundary.

Documentation Impact

Updated the active dispatcher, scheduler, target, and README guidance to name
the hosted admission owner, eligible public-target scope, and local-only
private/internal boundary. Added one unreleased changelog entry.

Evidence

  • Exact head: e00cb5bd4546f8087ec412778cd184614644150c
  • Base: e1eefe217304f0f510acb3890db1155cef61682e
  • Current-head CI: https://github.com/openclaw/clawsweeper/actions/runs/33252115308
    passed, including pnpm check; the PR has 13 successful or expected-skipped
    checks and no failing checks.
  • Independent exact-head admission, lifecycle, fanout, dispatcher, Worker,
    queue, and triage-harness proof: 348/348 tests passed.
  • Earlier complete changed-test qualification: 863/863 tests passed; later
    boundary and CI-repair amendments are covered by exact-head focused sets.
  • TypeScript builds, scoped lint/format, actionlint, documentation/static
    checks, git diff --check, signature verification, and private-data scrub
    passed.
  • Mandatory uncommitted Codex review completed with no actionable regression;
    an independent exact-head audit found no remaining admission bypass.

Real Behavior Proof

  • Claim: eligible public targets proceed, while private/internal,
    unavailable, explicitly denied, and out-of-scope public targets stop before
    target credentials or durable/target-side effects.
  • Exact revision: head
    e00cb5bd4546f8087ec412778cd184614644150c, tree
    1f9a5862201fce97ba7fd2e9c1d61bc80a81680e, base
    e1eefe217304f0f510acb3890db1155cef61682e.
  • Exercised surface: production dashboard/worker.ts and
    dashboard/exact-review-queue.ts under Node v26.7.0 and
    wrangler@4.107.0 dev --local, with real SQLite Durable Objects and
    disposable persistence.
  • Fixture: synthetic GitHub App credentials and webhook secret, a loopback
    GitHub metadata API, signed webhook ingress, and acknowledgement-pending
    lifecycle projections seeded in disposable SQLite.
  • Network boundary: target metadata requests were loopback-only. No real
    repository credential, private repository identity, production service, or
    production state was used.

Redacted final-effect authority trace:

[
  {
    "path": "eligible-public",
    "worker_visibility": "public",
    "queue_visibility": "public",
    "http": 202,
    "result": "recorded Bay journey completion",
    "github_requests": 6,
    "metadata_token_mints": 2,
    "visibility_probes": 2,
    "target_credential_mints": 0,
    "acknowledgement_persisted": true,
    "bay_completion_persisted": true,
    "queue_items": 0,
    "delivery_receipts": 0
  },
  {
    "path": "explicit-deny",
    "http": 202,
    "result": "target not eligible",
    "github_requests": 0,
    "metadata_token_mints": 0,
    "visibility_probes": 0,
    "target_credential_mints": 0,
    "acknowledgement_persisted": false,
    "bay_completion_persisted": false,
    "queue_items": 0,
    "delivery_receipts": 0
  },
  {
    "path": "public-then-not-found",
    "worker_visibility": "public",
    "queue_visibility": "not_found",
    "http": 202,
    "result": "private target unsupported",
    "target_credential_mints": 0,
    "acknowledgement_persisted": false,
    "bay_completion_persisted": false,
    "queue_items": 0,
    "delivery_receipts": 0
  },
  {
    "path": "retryable-metadata-failure",
    "worker_visibility": "public",
    "queue_metadata_token": "rate_limited",
    "http": 503,
    "result": "target_visibility_unverified",
    "github_requests": 5,
    "metadata_token_mints": 2,
    "visibility_probes": 1,
    "target_credential_mints": 0,
    "acknowledgement_persisted": false,
    "bay_completion_persisted": false,
    "queue_items": 0,
    "delivery_receipts": 0,
    "standalone_retry_record": false
  }
]

The explicit-deny path stops before any metadata I/O. The stale-private and
retryable paths perform only the bounded metadata authority check, then stop
before target credential minting, acknowledgement persistence, Bay completion,
queue insertion, or delivery receipts. Only the public/public path changes the
durable acknowledgement and Bay projection.

Local proof package provenance:

PROOF.md                    7519ae6cd2ad1b030f5862944dab6c6025f7fc289f84cb71395eb6e95f5d8607
authority-trace.jsonl       fd033645e2fa170c17761a2d267970ece38c1c09d7cfd26390fe076759dcc552
state-before-after.json     fd1cc49582c610a4286d80adff97e7dd8ab2d41ee2b9d75b630a50a712a5d92c
github-api-trace.json       20e53c191cf5f359953acbdfd842da0090fbc10ee36b8d9322bf9aac13e6c57f
SHA256SUMS                  aba6bd2db5a552da91eb7605c6c1f14d95daadb0d9e7b46525a94f87c25494f9

Limits: the historical acknowledgement-pending prerequisite was seeded
directly into disposable SQLite because no public endpoint creates that prior
state. Admission, both visibility probes, queue-owner final-effect decisions,
durable mutations, and HTTP responses all ran through the production Worker
code. This proof does not mutate the deployed service.

Prior Review Disposition

  • Maintainer decision: approve hosted use for eligible public contributor
    targets; private/internal use remains local-only. Arbitrary public
    repositories are not in scope.
  • Maintainer accepts the intentional fail-closed availability tradeoff:
    unverifiable metadata may delay hosted review, acknowledgement, and
    publication rather than allowing authority to become stale.
  • Configured membership plus approved public owner fallbacks is restored before
    hosted work.
  • The reusable admission job has a two-minute job timeout and a bounded
    metadata request.
  • Lifecycle/Bay acknowledgements now pass Worker admission plus a queue-side
    fresh visibility recheck before durable persistence.
  • Allowed, terminally rejected, retryable, and visibility-change authority
    paths are covered at exact head.
  • Latest rank-up moves are satisfied by the redacted final-effect trace and
    current-head proof package recorded above.

@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

@vincentkoc

Copy link
Copy Markdown
Member Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 29, 2026
@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 29, 2026, 8:40 AM ET / 12:40 UTC.

ClawSweeper review

What this changes

This PR centralizes configured-target eligibility and live public-visibility checks for hosted review dispatch, queueing, lifecycle acknowledgement, publication, and fanout.

Merge readiness

Blocked until stronger real behavior proof is added - 8 items remain

Keep open: the intended security boundary has a P1 gap because authority reservations can still be persisted after a target changes from public to private.

Priority: P1
Reviewed head: e00cb5bd4546f8087ec412778cd184614644150c

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The real production-path trace is valuable, but it does not cover the discovered authority-transition bypass.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Authority-chain proof required: the submitted exact-head Worker and Durable Object trace demonstrates several allowed and rejected paths, but it does not exercise the nearest forbidden case where visibility changes after Worker admission and before authority-reservation persistence; source shows that reservation is written first. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦪 silver shellfish (2/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Authority-chain proof required: the submitted exact-head Worker and Durable Object trace demonstrates several allowed and rejected paths, but it does not exercise the nearest forbidden case where visibility changes after Worker admission and before authority-reservation persistence; source shows that reservation is written first. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items Introduced reservation gate is eligibility-only: The PR-added source-authority gate checks registry eligibility, then persists the reservation; it does not perform the live public-visibility probe before the Durable Object write.
Live visibility check occurs after persistence: The source-authority worker later calls hostedTargetAdmission while processing an already-stored reservation, so a visibility transition in the intervening window is not rejected before durable state is written.
Initial Worker probe does not close the race: The Worker probes visibility before reserving source authority, but the subsequent queue reservation has no equivalent fresh probe.
Findings 1 actionable finding [P1] Recheck visibility before persisting authority reservations
Security Needs attention Visibility transition reaches durable authority state: Eligibility is not a current-public authorization decision; the source-authority route writes a reservation after an eligibility-only check, while the live visibility probe runs later during reservation processing.

How this fits together

Hosted review events enter the Worker and Durable Object queue before credentials are minted and review results are published. The new admission layer is meant to stop private or unsupported targets before those durable and credentialed stages.

flowchart LR
  A[GitHub event or command] --> B[Worker admission]
  B --> C[Public visibility probe]
  C --> D[Authority reservation]
  D --> E[Durable review queue]
  E --> F[Credentialed review and publication]
  F --> G[Bay lifecycle projection]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: the submitted exact-head Worker and Durable Object trace demonstrates several allowed and rejected paths, but it does not exercise the nearest forbidden case where visibility changes after Worker admission and before authority-reservation persistence; source shows that reservation is written first. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Recheck visibility before persisting authority reservations (P1) - This is a late finding on the same reviewed head. These new gates only check registry eligibility, then persist branch/source authority reservations. A configured target can become private after the Worker’s initial visibility probe and before this transaction; the first queue-side live probe occurs only when processing that stored reservation. Call the live admission helper before each reservation write and cover that transition path.
  • Resolve security concern: Visibility transition reaches durable authority state - Eligibility is not a current-public authorization decision; the source-authority route writes a reservation after an eligibility-only check, while the live visibility probe runs later during reservation processing.
  • Resolve merge risk (P1) - A configured target that becomes private after the Worker probe can still leave a durable authority reservation before the queue detects the transition.
  • Resolve merge risk (P0) - A GitHub metadata outage deliberately delays hosted reviews and publication; this fail-closed availability tradeoff is documented in the PR body.
  • Complete next step (P2) - A focused mechanical repair can close the authority-reservation visibility race and add regression coverage before another exact-head review.
  • Improve patch quality - Recheck public visibility before all authority-reservation writes.
  • Improve patch quality - Add an exact-head production-boundary trace showing a public-to-private transition is rejected before any reservation or queue state is persisted.

Findings

  • [P1] Recheck visibility before persisting authority reservations — dashboard/exact-review-queue.ts:1053-1057
  • [high] Visibility transition reaches durable authority state — dashboard/exact-review-queue.ts:1053
Agent review details

Security

Needs attention: The introduced authority boundary permits a private-transition target to persist an authority reservation before its fresh visibility check.

Review metrics

Metric Value Why it matters
Patch scope 43 files, +6,086 / -783 lines The security boundary spans Worker, queue, workflows, repair intake, documentation, and tests.
Production versus tests production +2,020, tests +4,066 The substantial test expansion supports the broad admission change, but misses the authority-reservation transition case.

Merge-risk options

Maintainer options:

  1. Close the reservation race (recommended)
    Use the existing live admission helper before command, branch, and source authority reservations are persisted, then add a transition-path regression case.
  2. Accept persisted private-target metadata
    Merge with the documented limitation that a target may become private after Worker admission and still create a short-lived durable reservation.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Recheck live public visibility before every authority-reservation write and add focused transition-path coverage.

Technical review

Best possible solution:

Probe live public visibility at every authority-reservation persistence boundary, reject before writing state, and prove the public-to-private transition through the production queue path.

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

Yes—source inspection gives a high-confidence path: let a configured repository become private after the Worker probe and before the source-authority reservation transaction.

Is this the best way to solve the issue?

No—the centralized design is appropriate, but it must recheck live visibility before each durable authority reservation rather than only during later processing.

Full review comments:

  • [P1] Recheck visibility before persisting authority reservations — dashboard/exact-review-queue.ts:1053-1057
    This is a late finding on the same reviewed head. These new gates only check registry eligibility, then persist branch/source authority reservations. A configured target can become private after the Worker’s initial visibility probe and before this transaction; the first queue-side live probe occurs only when processing that stored reservation. Call the live admission helper before each reservation write and cover that transition path.
    Confidence: 0.96
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P1: A private-target transition can bypass the claimed pre-persistence authority boundary in hosted review processing.
  • merge-risk: 🚨 security-boundary: The PR changes which repositories may reach durable review authority and credentialed processing.
  • merge-risk: 🚨 availability: The new design intentionally fails closed when visibility metadata cannot be verified, delaying hosted work.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦐 gold shrimp and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: the submitted exact-head Worker and Durable Object trace demonstrates several allowed and rejected paths, but it does not exercise the nearest forbidden case where visibility changes after Worker admission and before authority-reservation persistence; source shows that reservation is written first. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

  • [high] Visibility transition reaches durable authority state — dashboard/exact-review-queue.ts:1053
    Eligibility is not a current-public authorization decision; the source-authority route writes a reservation after an eligibility-only check, while the live visibility probe runs later during reservation processing.
    Confidence: 0.96

Acceptance criteria:

  • [P1] pnpm exec tsx --test test/dashboard-worker-command-intake.test.ts test/dashboard-worker-webhook-ingress.test.ts test/dashboard-worker-queue-runtime.test.ts.
  • [P1] pnpm run check.

What I checked:

  • Introduced reservation gate is eligibility-only: The PR-added source-authority gate checks registry eligibility, then persists the reservation; it does not perform the live public-visibility probe before the Durable Object write. (dashboard/exact-review-queue.ts:1053, e00cb5bd4546)
  • Live visibility check occurs after persistence: The source-authority worker later calls hostedTargetAdmission while processing an already-stored reservation, so a visibility transition in the intervening window is not rejected before durable state is written. (dashboard/exact-review-queue.ts:11185, e00cb5bd4546)
  • Initial Worker probe does not close the race: The Worker probes visibility before reserving source authority, but the subsequent queue reservation has no equivalent fresh probe. (dashboard/worker.ts:3537, e00cb5bd4546)
  • Introduction boundary verified: Host-provided introduction evidence identifies the exact base-to-head delta and verified test merge; the affected queue gate is part of this PR's introduced behavior. (dashboard/exact-review-queue.ts:1053, e00cb5bd4546)
  • Submitted real-behavior trace has a coverage gap: The PR body supplies a redacted Worker/queue trace for allowed, denied, stale-private, and retryable cases, but not a target that changes visibility between the initial Worker check and authority-reservation persistence. (e00cb5bd4546)

Likely related people:

  • Vincent Koc: Current-main history includes recent queue and publication work by Vincent Koc, alongside this admission implementation. (role: recent queue contributor; confidence: high; commits: af8f89fe69e1, 648ad3538d98; files: dashboard/exact-review-queue.ts, dashboard/worker.ts)
  • Martin Cleary: Recent lifecycle and Bay changes establish the adjacent durable acknowledgement and projection behavior affected by this boundary. (role: recent lifecycle contributor; confidence: high; commits: 91a2bf645327, 2da70c23b043; files: dashboard/exact-review-queue.ts, dashboard/worker.ts)

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 (7 earlier review cycles)
  • reviewed 2026-08-29T03:23:38.729Z sha d4fcc84 :: needs real behavior proof before merge. :: [P2] Bound the hosted-admission HTTP probe | [P2] Keep generic hosted review within an approved target scope
  • reviewed 2026-08-29T03:29:02.715Z sha d4fcc84 :: needs real behavior proof before merge. :: [P1] Restrict hosted work to configured repository targets | [P2] Add a deadline to the reusable admission probe
  • reviewed 2026-08-29T05:34:52.449Z sha d4fcc84 :: needs real behavior proof before merge. :: [P1] Keep hosted work within the configured target scope | [P2] Bound the reusable admission probe
  • reviewed 2026-08-29T11:33:54.971Z sha 1234bc1 :: needs real behavior proof before merge. :: [P1] Gate lifecycle acknowledgement on fresh public admission
  • reviewed 2026-08-29T11:49:01.890Z sha e4e6b80 :: needs real behavior proof before merge. :: [P1] Gate lifecycle acknowledgements on fresh public admission
  • reviewed 2026-08-29T11:57:35.609Z sha e4e6b80 :: needs real behavior proof before merge. :: [P1] Gate lifecycle acknowledgements on fresh public admission
  • reviewed 2026-08-29T12:21:39.001Z sha e00cb5b :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 29, 2026
@vincentkoc
vincentkoc force-pushed the fix/hosted-public-targets branch from d4fcc84 to 1234bc1 Compare August 29, 2026 11:29
@clawsweeper clawsweeper Bot added P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Aug 29, 2026
@vincentkoc
vincentkoc force-pushed the fix/hosted-public-targets branch from 1234bc1 to e4e6b80 Compare August 29, 2026 11:44
@vincentkoc
vincentkoc force-pushed the fix/hosted-public-targets branch from e4e6b80 to e00cb5b Compare August 29, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. 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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant