Skip to content

fix: repair local approval resolution#87131

Merged
steipete merged 2 commits into
mainfrom
maint/pr-86771-approval-runtime-token
May 27, 2026
Merged

fix: repair local approval resolution#87131
steipete merged 2 commits into
mainfrom
maint/pr-86771-approval-runtime-token

Conversation

@steipete

@steipete steipete commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Follow-up to fix: repair local approval resolution #86771: reworks approval runtime token forwarding so authority follows the gateway URL source, not whether the URL host is loopback.
  • Keeps the token for generated local gateway targets and local fallback targets; omits it for CLI, env, and configured remote URLs, including loopback SSH-tunnel URLs.
  • Adds focused mocked coverage plus a real Gateway e2e that proves generated-local resolution succeeds and configured-loopback remote resolution is not promoted.

Verification

  • autoreview: clean, no accepted/actionable findings.
  • Local: pnpm exec oxfmt --check src/gateway/operator-approvals-client.ts src/gateway/operator-approvals-client.test.ts src/gateway/operator-approvals-client.e2e.test.ts extensions/discord/src/monitor/exec-approvals.ts extensions/discord/src/monitor/exec-approvals.test.ts
  • Local: git diff --check origin/main...HEAD
  • Crabbox AWS run run_c36357c075dc on cbx_79c88d07570f: node scripts/run-vitest.mjs run src/gateway/operator-approvals-client.test.ts src/gateway/operator-approvals-client.e2e.test.ts src/agents/tools/gateway.test.ts extensions/discord/src/monitor/exec-approvals.test.ts --reporter=dot --pool=forks --no-file-parallelism --testTimeout=120000
  • Crabbox AWS run run_c36357c075dc on cbx_79c88d07570f: pnpm exec oxfmt --check src/gateway/operator-approvals-client.ts src/gateway/operator-approvals-client.test.ts src/gateway/operator-approvals-client.e2e.test.ts extensions/discord/src/monitor/exec-approvals.ts extensions/discord/src/monitor/exec-approvals.test.ts

Behavior addressed: approval resolution from local channel/plugin surfaces keeps same-process runtime authority, while configured or override loopback gateway URLs are treated as remote/tunnel targets and do not receive the process-local approval runtime token.
Real environment tested: Crabbox AWS Linux c7a.8xlarge, Node v24.16.0, pnpm 11.2.2, real loopback Gateway server in src/gateway/operator-approvals-client.e2e.test.ts.
Exact steps or command run after this patch: node scripts/run-vitest.mjs run src/gateway/operator-approvals-client.test.ts src/gateway/operator-approvals-client.e2e.test.ts src/agents/tools/gateway.test.ts extensions/discord/src/monitor/exec-approvals.test.ts --reporter=dot --pool=forks --no-file-parallelism --testTimeout=120000
Evidence after fix: 5 test files passed, 68 tests passed; formatter check passed on all touched files.
Observed result after fix: default generated local gateway resolution succeeds with runtime authority; configured loopback remote URL resolution is rejected as approval not found without runtime authority.
What was not tested: full repository check suite and live Discord click against Discord API.

Keeps contributor credit from @fuller-stack-dev.

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord gateway Gateway runtime labels May 27, 2026
@steipete steipete mentioned this pull request May 27, 2026
Merged
@openclaw-barnacle openclaw-barnacle Bot added size: M maintainer Maintainer-authored PR labels May 27, 2026
@steipete steipete force-pushed the maint/pr-86771-approval-runtime-token branch from 13784d3 to bb5ed5e Compare May 27, 2026 03:05
@openclaw-barnacle openclaw-barnacle Bot removed the channel: discord Channel integration: discord label May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed May 26, 2026, 11:10 PM ET / 03:10 UTC.

Summary
The branch changes operator approval gateway clients to send the process-local approval runtime token based on resolved gateway URL source, with mocked and real Gateway coverage for local, remote, env, and configured loopback cases.

PR surface: Source +6, Tests +217. Total +223 across 3 files.

Reproducibility: yes. current main's source path sends the runtime token based on loopback URL shape, and the PR's real Gateway E2E gives a concrete before/after scenario for generated local versus configured loopback remote approval resolution. I did not rerun the E2E locally because this was a read-only review.

Review metrics: 1 noteworthy metric.

  • Approval token source matrix: 2 allowed sources, 3 denied override/remote cases tested. The runtime authority decision now depends on gateway URL provenance, so maintainers should see the intended allow/deny boundary before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Confirm the intended fail-closed behavior for CLI, env, and configured loopback gateway URLs before merge.

Risk before merge

  • This is approval-runtime auth boundary code: if the urlSource allowlist is wrong, local approvals can either lose same-process authority or a tunnel/remote gateway can receive it.
  • The PR intentionally fails closed for CLI, env, and configured loopback targets; maintainers should confirm that boundary before merge.

Maintainer options:

  1. Accept source-based approval boundary (recommended)
    Merge as-is if maintainers agree that only generated local and fallback-local gateway sources may receive process-local approval authority.
  2. Pause for stronger transport proof
    Hold the PR only if maintainers want an additional live channel approval click proof beyond the real Gateway E2E coverage already supplied.

Next step before merge
Maintainer review is the right next action because the PR has a protected maintainer label and changes approval-runtime auth boundary behavior; no narrow automated repair is needed.

Security
Cleared: No concrete security or supply-chain defect was found; the diff narrows approvalRuntimeToken authority and adds focused coverage without dependency, workflow, or secret-handling changes.

Review details

Best possible solution:

Land the source-based approval-runtime boundary after maintainer review accepts the fail-closed behavior for explicit, env, and configured loopback gateway URLs.

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

Yes; current main's source path sends the runtime token based on loopback URL shape, and the PR's real Gateway E2E gives a concrete before/after scenario for generated local versus configured loopback remote approval resolution. I did not rerun the E2E locally because this was a read-only review.

Is this the best way to solve the issue?

Yes; using the bootstrap urlSource is narrower and more maintainable than inferring authority from loopback hostnames. A typed urlSource union would make future edits harder to drift, but the current patch and tests are sufficient for this PR.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against e7f644c7b137.

Label changes

Label changes:

  • add P2: This is a focused approval-runtime bug/security-boundary repair with limited files and targeted proof, not an emergency runtime outage.
  • add merge-risk: 🚨 security-boundary: The diff changes when process-local approval authority is sent during Gateway authentication.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix Crabbox AWS live output for a real loopback Gateway E2E, including commands, environment, passing counts, and observed generated-local versus configured-loopback behavior.
  • 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 (live_output): The PR body provides after-fix Crabbox AWS live output for a real loopback Gateway E2E, including commands, environment, passing counts, and observed generated-local versus configured-loopback behavior.

Label justifications:

  • P2: This is a focused approval-runtime bug/security-boundary repair with limited files and targeted proof, not an emergency runtime outage.
  • merge-risk: 🚨 security-boundary: The diff changes when process-local approval authority is sent during Gateway authentication.
  • 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 (live_output): The PR body provides after-fix Crabbox AWS live output for a real loopback Gateway E2E, including commands, environment, passing counts, and observed generated-local versus configured-loopback behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix Crabbox AWS live output for a real loopback Gateway E2E, including commands, environment, passing counts, and observed generated-local versus configured-loopback behavior.
Evidence reviewed

PR surface:

Source +6, Tests +217. Total +223 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 8 2 +6
Tests 2 219 2 +217
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 227 4 +223

What I checked:

  • Root policy read: Root AGENTS.md was read fully; approval/auth, fallback behavior, and gateway changes are compatibility- and security-sensitive review surfaces. (AGENTS.md:23, e7f644c7b137)
  • Scoped gateway policy read: src/gateway/AGENTS.md was read fully; it requires gateway tests to keep runtime state explicit and focused on the gateway behavior under test. (src/gateway/AGENTS.md:1, e7f644c7b137)
  • Current-main behavior: Current main sends approvalRuntimeToken when no explicit gatewayUrl is provided or when the resolved URL host is loopback, which treats configured loopback remote URLs as local authority. (src/gateway/operator-approvals-client.ts:47, e7f644c7b137)
  • Patch behavior: The PR replaces the loopback-host heuristic with an allowlist for urlSource values local loopback and missing gateway.remote.url fallback local before attaching approvalRuntimeToken. (src/gateway/operator-approvals-client.ts:28, bb5ed5ed88f7)
  • URL source contract: connection-details emits distinct urlSource strings for cli --url, env OPENCLAW_GATEWAY_URL, config gateway.remote.url, fallback local, and local loopback, matching the PR's source-based decision. (src/gateway/connection-details.ts:56, e7f644c7b137)
  • Focused unit coverage: The updated unit tests assert that explicit loopback, remote explicit, configured remote loopback, and env loopback sources omit approvalRuntimeToken, while fallback local keeps it. (src/gateway/operator-approvals-client.test.ts:124, bb5ed5ed88f7)

Likely related people:

  • fuller-stack-dev: The current-main token heuristic was merged through fix: repair local approval resolution #86771, whose PR author was fuller-stack-dev and whose merge commit is blamed on the relevant approvalRuntimeToken lines. (role: introduced related behavior; confidence: medium; commits: 13cfb77c10f9; files: src/gateway/operator-approvals-client.ts)
  • steipete: Available history shows steipete introduced the operator approval client file in the current shallow history and authored the two commits in this follow-up branch. (role: recent area contributor and current repair author; confidence: high; commits: 1954468efc2a, b35d9cb236de, bb5ed5ed88f7; files: src/gateway/operator-approvals-client.ts, src/gateway/operator-approvals-client.test.ts, src/gateway/operator-approvals-client.e2e.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: 💎 rare Neon Patch Peep

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 💎 rare.
Trait: polishes edge cases.
Image traits: location proof lagoon; accessory lint brush; palette coral, mint, and warm cream; mood patient; pose holding its accessory up for inspection; shell paper lantern shell; lighting bright celebratory glints; background small green status lights.
Share on X: post this hatch
Copy: My PR egg hatched a 💎 rare Neon Patch Peep in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

steipete and others added 2 commits May 27, 2026 04:15
@steipete @fuller-stack-dev
fix: keep approval runtime token local-only
1ffcaf0 
Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com>
@steipete @fuller-stack-dev
test: isolate approval client e2e env
cad41b1 
Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com>
@steipete steipete force-pushed the maint/pr-86771-approval-runtime-token branch from bb5ed5e to cad41b1 Compare May 27, 2026 03:15
@steipete steipete merged commit 96c5766 into main May 27, 2026
96 of 97 checks passed
@steipete steipete deleted the maint/pr-86771-approval-runtime-token branch May 27, 2026 03:20
@steipete

steipete commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Landed via squash merge.

  • Gate: autoreview clean, no accepted/actionable findings
  • Gate: Crabbox AWS run_5f28c413194d on cbx_ec9ef82cf95a: 5 focused files / 68 tests passed, plus formatter
  • Merge commit: 96c576674de21f7082362f966fdfce24a907412a

Thanks @fuller-stack-dev for the original report and patch.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete