fix(acp): require owner for runtime controls by eleqtrizit · Pull Request #97953 · openclaw/openclaw

eleqtrizit · 2026-06-29T23:20:42Z

Summary

What Problem This Solves

Fixes an issue where an authorized non-owner channel sender could invoke ACP runtime controls that are reserved for owners or internal Gateway administrators.

Why This Change Was Made

ACP runtime-control actions now use the existing owner gate in addition to the existing internal operator.admin scope gate. Read-only diagnostics and discovery remain available to authorized non-owner senders.

Changes

Require external owner identity for ACP session lifecycle and runtime-option controls.
Preserve operator.admin enforcement for internal Gateway clients.
Keep /acp sessions, /acp doctor, /acp install, and /acp help available to authorized non-owners.
Document the external-owner and internal-admin requirements.
Add regression coverage for every gated action and retained read-only access.

User Impact

Non-owner channel users can no longer create, steer, close, or reconfigure ACP runtime sessions. Existing owner and internal Gateway admin workflows continue to work.

Validation

Evidence

Native Telegram Desktop before/after on PR head eca0f26e6ca8af7d4d68e971f4b856fd13685939: the same authorized non-owner /acp status request reaches the ACP handler on main, while the candidate replies You are not authorized to use this command. (run, artifact, inline proof)
Production authorization + ACP dispatcher terminal proof on the candidate, without handler mocks: internal operator.write returns This /acp action requires operator.admin on the internal channel.; internal operator.admin resolves senderIsOwner: true and reaches the ACP status handler, which returns the expected ACP_SESSION_INIT_FAILED for the deliberately non-ACP proof session.
node scripts/run-vitest.mjs src/auto-reply/reply/commands-acp.test.ts --reporter=verbose (60 tests passed, including all 12 owner-gated actions, retained non-owner diagnostics, internal write denial, and internal admin success)
node scripts/run-oxlint.mjs src/auto-reply/reply/commands-acp.ts src/auto-reply/reply/commands-acp.test.ts
node_modules/.bin/oxfmt --check docs/tools/acp-agents.md docs/tools/slash-commands.md src/auto-reply/reply/commands-acp.ts src/auto-reply/reply/commands-acp.test.ts
node scripts/check-docs-mdx.mjs docs README.md
node scripts/format-docs.mjs --check
git diff --check

Notes

AI-assisted change; the repository autoreview gate was run to a clean result before publication.
The compatibility tightening is intentional: ACP lifecycle, status, and runtime-option controls are owner/admin operations. Authorized non-owners retain /acp sessions, /acp doctor, /acp install, and /acp help.

clawsweeper · 2026-06-29T23:21:39Z

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 8:13 PM ET / 00:13 UTC.

Summary
The PR adds a centralized owner check for ACP lifecycle, status, and runtime-option commands, keeps diagnostics available, updates regression tests, and documents the owner/internal-admin requirement.

PR surface: Source +6, Tests +32, Docs +6. Total +44 across 4 files.

Reproducibility: yes. Current main is source-reproducible and the Mantis Telegram proof shows /acp status reaching the ACP handler on main, then being denied on the PR for the same non-owner user.

Review metrics: 1 noteworthy metric.

ACP owner-gated actions: 12 gated, 4 retained. The set moves lifecycle, status, and runtime-option controls behind owner/admin identity while leaving sessions, doctor, install, and help available.

Stored data model
Persistent data-model change detected: migration/backfill/repair: docs/tools/acp-agents.md, migration/backfill/repair: docs/tools/slash-commands.md. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
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:

Have maintainers confirm the owner-only ACP runtime-control policy, including /acp status.

Risk before merge

[P1] Compatibility: authorized external non-owner channel users who could previously run ACP lifecycle, status, or runtime-option controls will now be denied unless configured as owners.
[P1] The maintainer label means the compatibility tightening, especially making /acp status owner-only, still needs explicit human acceptance before merge.

Maintainer options:

Accept owner-only runtime controls (recommended)
Merge if maintainers agree authorized external non-owners should lose ACP lifecycle, status, and runtime-option control access in favor of owner/internal-admin only.
Keep status diagnostic
If /acp status should remain diagnostic for authorized non-owners, remove it from the owner-required set and align docs and tests before merge.
Pause for policy
Pause or close if ACP runtime-control ownership should be redesigned alongside broader channel/admin command policy instead of tightened here.

Next step before merge

[P2] Protected maintainer label and compatibility tightening require human acceptance; no automated code repair is indicated.

Security
Cleared: No supply-chain or security regression was found; the diff narrows ACP runtime-control access and does not touch dependencies, workflows, scripts, or secret handling.

Review details

Best possible solution:

Land the centralized ACP owner gate once maintainers explicitly accept the compatibility change for owner-gated ACP runtime controls, especially /acp status.

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

Yes. Current main is source-reproducible and the Mantis Telegram proof shows /acp status reaching the ACP handler on main, then being denied on the PR for the same non-owner user.

Is this the best way to solve the issue?

Yes for the code shape. The central ACP dispatcher is the narrowest maintainable gate point; the remaining question is maintainer acceptance of the compatibility change.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. Mantis screenshots/GIF metadata show real Telegram before/after behavior, and the PR comment adds terminal output for internal write denial and admin pass-through.
add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Mantis screenshots/GIF metadata show real Telegram before/after behavior, and the PR comment adds terminal output for internal write denial and admin pass-through.
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 (screenshot): Mantis screenshots/GIF metadata show real Telegram before/after behavior, and the PR comment adds terminal output for internal write denial and admin pass-through.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P1: The PR addresses an ACP runtime-control authorization boundary where authorized non-owners can currently create, steer, close, inspect, or reconfigure ACP sessions.
merge-risk: 🚨 compatibility: Merging intentionally changes existing authorized non-owner channel behavior from allowed to denied for ACP runtime controls.
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 (screenshot): Mantis screenshots/GIF metadata show real Telegram before/after behavior, and the PR comment adds terminal output for internal write denial and admin pass-through.
proof: sufficient: Contributor real behavior proof is sufficient. Mantis screenshots/GIF metadata show real Telegram before/after behavior, and the PR comment adds terminal output for internal write denial and admin pass-through.
proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Mantis screenshots/GIF metadata show real Telegram before/after behavior, and the PR comment adds terminal output for internal write denial and admin pass-through.
mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The ACP command gate changes visible Telegram command replies, so Telegram Desktop proof is useful and has already been provided.

Evidence reviewed

PR surface:

Source +6, Tests +32, Docs +6. Total +44 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	9	3	+6
Tests	1	33	1	+32
Docs	2	7	1	+6
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	49	5	+44

What I checked:

Repository policy read: Root AGENTS.md and docs/AGENTS.md were read fully; their compatibility-sensitive review and docs guidance apply to this PR. (AGENTS.md:1, 2cf765f73263)
Current main lacks external owner gate: On current main, ACP runtime-control actions only run the Gateway scope check before dispatch, so external authorized senders without Gateway scopes can reach the handlers. (src/auto-reply/reply/commands-acp.ts:107, 2cf765f73263)
Existing owner gate contract: rejectNonOwnerCommand is the existing command gate for blocking non-owner command senders and returning a native command denial reply when appropriate. (src/auto-reply/reply/command-gates.ts:32, 2cf765f73263)
Internal admin owner mapping: Command authorization maps internal operator.admin Gateway clients to senderIsOwner while external channels do not gain owner identity from Gateway scopes. (src/auto-reply/command-auth.ts:673, 2cf765f73263)
PR gate placement: The PR applies rejectNonOwnerCommand after the existing operator.admin scope check for the ACP owner-required action set, preserving internal admin pass-through. (src/auto-reply/reply/commands-acp.ts:107, eca0f26e6ca8)
Regression coverage: The PR adds table coverage for 12 owner-gated ACP actions, retains non-owner access to /acp sessions, and removes the test helper's forced internal owner flag. (src/auto-reply/reply/commands-acp.test.ts:1146, eca0f26e6ca8)

Likely related people:

vincentkoc: Authored and merged the earlier ACP internal-admin gate in ACP: require admin scope for mutating internal actions #46789, which established the runtime-control scope boundary this PR extends. (role: feature-history owner; confidence: high; commits: 229426a257e4; files: src/auto-reply/reply/commands-acp.ts, src/auto-reply/reply/commands-acp.test.ts)
solodmd: The current shallow/grafted main history blames the ACP dispatcher, command gate helper, and command-auth mapping to d21ddd3, so this is a weaker recent-routing signal. (role: recent area carrier; confidence: low; commits: d21ddd3f7a88; files: src/auto-reply/reply/commands-acp.ts, src/auto-reply/reply/command-gates.ts, src/auto-reply/command-auth.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.

eleqtrizit · 2026-06-29T23:27:28Z

@openclaw-mantis telegram desktop proof: verify a non-owner is blocked from /acp spawn while an owner can still run an allowed ACP command.

openclaw-mantis · 2026-06-30T00:03:31Z

Mantis Telegram Desktop Proof

Summary: Mantis captured native Telegram Desktop before/after GIFs for /acp spawn command access.

Scenario: telegram-desktop-proof
Trigger: issue_comment
Run: https://github.com/openclaw/openclaw/actions/runs/28409446423
Artifact: https://github.com/openclaw/openclaw/actions/runs/28409446423/artifacts/7968046292
Baseline: pass at 052dc59526a2dfdab64b3b16d8fe31650d902856, expected authorized Telegram sender reaches ACP command handling
Candidate: pass at eca0f26e6ca8af7d4d68e971f4b856fd13685939, expected non-owner Telegram sender is blocked from /acp spawn
Overall: true

Main screenshot	This PR screenshot

Main	This PR

Motion-trimmed clips:

Raw QA files: https://artifacts.openclaw.ai/mantis/telegram-desktop/pr-97953/run-28409446423-1/index.json

eleqtrizit · 2026-06-30T00:09:48Z

Behavioral proof on PR head `eca0f26e6ca8`

The requested authorization behavior is now demonstrated through a real Telegram transport plus the production internal-client auth/dispatch path.

Real Telegram Desktop before/after

Provider/run: AWS Crabbox cbx_d4c3d3ce1cd9, Actions run
Inline screenshots/GIFs · artifact
Main (052dc59526a2): the authorized non-owner's /acp status reaches the ACP handler and returns ACP_SESSION_INIT_FAILED for the non-ACP test session.
This PR (eca0f26e6ca8): the same real Telegram user and command receive You are not authorized to use this command.
The workflow validated both refs, inspected the evidence manifest, released the Telegram lease, and completed successfully.

Preserved internal-admin path

A no-mock terminal run through the production command authorization builder and ACP dispatcher on the PR head produced:

[
  {
    "scopes": ["operator.write"],
    "senderIsOwner": false,
    "reply": "This /acp action requires operator.admin on the internal channel."
  },
  {
    "scopes": ["operator.admin"],
    "senderIsOwner": true,
    "reply": "ACP error (ACP_SESSION_INIT_FAILED): Session is not ACP-enabled: agent:main:main"
  }
]

The second result is deliberately an ACP session-state error: it proves the internal admin passed both authorization gates and reached the real ACP status handler. The write-scoped client remained blocked at the scope gate.

Regression coverage

node scripts/run-vitest.mjs src/auto-reply/reply/commands-acp.test.ts --reporter=verbose passed all 60 tests, including all 12 owner-gated controls, retained authorized non-owner access to /acp sessions, internal write denial, and internal admin success.

Compatibility tightening is intentional: lifecycle, status, and runtime-option controls require external owner identity or internal operator.admin; authorized non-owners retain sessions, doctor, install, and help.

eleqtrizit · 2026-06-30T00:10:07Z

@clawsweeper re-review

clawsweeper · 2026-06-30T00:10:10Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

(cherry picked from commit 6ead092)

fix(acp): require owner for runtime controls

eca0f26

openclaw-barnacle Bot added docs Improvements or additions to documentation size: XS maintainer Maintainer-authored PR labels Jun 29, 2026

clawsweeper Bot temporarily deployed to qa-live-shared June 29, 2026 23:27 Inactive

eleqtrizit self-assigned this Jun 30, 2026

eleqtrizit merged commit 6ead092 into openclaw:main Jun 30, 2026
191 of 212 checks passed

github-actions Bot mentioned this pull request Jun 30, 2026

📡 Upstream Digest — 2026-06-30 02:37 UTC curtismercier/openclaw-mods#1176

Open

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 30, 2026

fix(acp): require owner for runtime controls (openclaw#97953)

9b6fa99

clawsweeper Bot mentioned this pull request Jun 30, 2026

test(gateway): add unit tests for node wake state tracking and testing seam #98205

Merged

eleqtrizit deleted the 797 branch June 30, 2026 19:27

This was referenced Jun 30, 2026

feat(models): add Claude Sonnet 5 support #98254

Merged

fix(feishu): avoid local media fallback leaks #98251

Merged

QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026

fix(acp): require owner for runtime controls (openclaw#97953)

906af5e

chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026

fix(acp): require owner for runtime controls (openclaw#97953)

b2df1b3

chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026

fix(acp): require owner for runtime controls (openclaw#97953)

34464c4

clawsweeper Bot mentioned this pull request Jul 6, 2026

fix(acp): preserve timeout progress summaries #91479

Open

Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 6, 2026

fix(acp): require owner for runtime controls (openclaw#97953)

f2fbed8

(cherry picked from commit 6ead092)

Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 8, 2026

fix(acp): require owner for runtime controls (openclaw#97953)

a00d1cc

(cherry picked from commit 6ead092)

yetval mentioned this pull request Jul 9, 2026

/acp sessions exposes every gateway ACP session to any allowlisted non-owner sender #103055

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(acp): require owner for runtime controls#97953

fix(acp): require owner for runtime controls#97953
eleqtrizit merged 1 commit into
openclaw:mainfrom
eleqtrizit:797

eleqtrizit commented Jun 29, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

eleqtrizit commented Jun 29, 2026

Uh oh!

openclaw-mantis Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

eleqtrizit commented Jun 30, 2026

Uh oh!

eleqtrizit commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

eleqtrizit commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Problem This Solves

Why This Change Was Made

Changes

User Impact

Validation

Evidence

Notes

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eleqtrizit commented Jun 29, 2026

Uh oh!

openclaw-mantis Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mantis Telegram Desktop Proof

Uh oh!

eleqtrizit commented Jun 30, 2026

Behavioral proof on PR head eca0f26e6ca8

Real Telegram Desktop before/after

Preserved internal-admin path

Regression coverage

Uh oh!

eleqtrizit commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

eleqtrizit commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

openclaw-mantis Bot commented Jun 30, 2026 •

edited

Loading

Behavioral proof on PR head `eca0f26e6ca8`

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading