Honor owner enforcement for native commands [AI] by pgondhi987 · Pull Request #78864 · openclaw/openclaw

pgondhi987 · 2026-05-07T08:36:17Z

Summary

Problem: Native command authorization could still pass the authorized-sender gate when a plugin enabled owner enforcement without an explicit owner allowlist.
Why it matters: Owner-enforced command surfaces should use the same owner requirement for native slash commands as they do for text commands.
What changed: The native command fallback now disables itself whenever owner authorization is required, and regression coverage was added for resolver, stop, and subagent spawn paths.
What did NOT change (scope boundary): Channels without owner enforcement or explicit owner allowlists keep the existing native command fallback behavior.

AI-assisted: Yes

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Related: N/A
This PR addresses a bug or regression

Real behavior proof (required for external PRs)

Behavior or issue addressed: Native command authorization under plugin owner enforcement.
Real environment tested: Not run in this metadata step.
Exact steps or command run after this patch: Not run; validation pending in maintainer/CI environment.
Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): Pending targeted test output.
Observed result after fix: Assertions now expect non-owner native commands to be rejected before stop mutation or subagent spawn work.
What was not tested: Live OpenClaw setup and local command execution.
Before evidence (optional but encouraged): Existing test expectation covered the old native fallback behavior.

Root Cause (if applicable)

Root cause: The native command fallback only checked whether an explicit owner allowlist existed, so plugin-level owner enforcement without an explicit list did not disable the fallback.
Missing detection / guardrail: Resolver coverage asserted the old behavior, and handler-level tests did not cover native stop or subagent spawn rejection under plugin owner enforcement.
Contributing context (if known): Native slash-command handling needed a channel-auth fallback for non-owner-enforced surfaces.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/auto-reply/command-control.test.ts, src/auto-reply/reply/commands-stop-target.test.ts, src/auto-reply/reply/commands-subagents-routing.test.ts
Scenario the test should lock in: A channel-authorized native command sender who is not an owner is rejected when the plugin enforces owner-only commands.
Why this is the smallest reliable guardrail: It checks the shared authorization resolver and the two command gates most directly affected by isAuthorizedSender.
Existing test that already covers this (if any): The resolver test existed but expected the old behavior; this PR updates it.
If no new test is added, why not: N/A

User-visible / Behavior Changes

Native slash commands on plugins that enforce owner-only commands now require owner authorization for authorized-sender-only command handlers.

Diagram (if applicable)

N/A

Security Impact (required)

New permissions/capabilities? (Yes/No) No
Secrets/tokens handling changed? (Yes/No) No
New/changed network calls? (Yes/No) No
Command/tool execution surface changed? (Yes/No) Yes
Data access scope changed? (Yes/No) No
If any Yes, explain risk + mitigation: Native command eligibility is narrowed when owner enforcement is active. Regression tests cover the resolver and representative command handlers.

Repro + Verification

Environment

OS: Not run in this metadata step
Runtime/container: Not run in this metadata step
Model/provider: N/A
Integration/channel (if any): Native-command-capable channel plugin test fixtures
Relevant config (redacted): commands.enforceOwnerForCommands=true, channel allowFrom includes wildcard, no explicit owner allowlist

Steps

Register a native-command-capable channel plugin with owner enforcement enabled.
Resolve command authorization for a channel-authorized non-owner native command sender.
Route /stop and /subagents spawn using the resolved command authorization.

Expected

The sender is not treated as owner.
The sender is not an authorized command sender.
Stop handling does not mutate session state.
Subagent spawn routing does not spawn work.

Actual

Not run locally in this metadata step; tests were added or updated to assert the expected behavior.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Pending targeted validation output.

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: Reviewed the resolver condition and handler-level assertions for native stop and subagent spawn rejection.
Edge cases checked: Existing native fallback remains available when no owner requirement is active.
What you did not verify: Local test execution, live channel behavior, and CI.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes/No) Yes
Config/env changes? (Yes/No) No
Migration needed? (Yes/No) No
If yes, exact upgrade steps: N/A

Risks and Mitigations

Risk: A native command surface that intentionally combined owner enforcement with channel-only native fallback will now require owner identity.
- Mitigation: This matches the owner-enforcement contract, and coverage preserves native fallback behavior for surfaces without an owner requirement.

clawsweeper · 2026-05-07T08:39:41Z

Codex review: needs real behavior proof before merge.

Summary
The PR changes command authorization so native command fallback is disabled under plugin owner enforcement, then adds resolver, /stop, and /subagents spawn regression coverage.

Reproducibility: yes. from source inspection: current main computes native fallback without checking plugin owner enforcement, and the existing resolver test asserts the old allowed-native behavior. I did not run tests because this review was required to keep the checkout read-only.

Real behavior proof
Needs real behavior proof before merge: The PR body says no real environment or local command execution was run, and the Real behavior proof check failed on the head SHA. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
External real behavior proof is missing, the proof check failed, and the protected maintainer label requires explicit maintainer handling before merge.

Security
Cleared: The diff narrows native command eligibility and adds tests; I found no new dependency, secret handling, CI, or supply-chain expansion.

Review findings

[P3] Add the required changelog entry — src/auto-reply/command-auth.ts:441-443

Review details

Best possible solution:

Land a narrow auth fix that makes native and text command authorization honor the existing owner-enforcement contract, with targeted regression tests, a changelog entry, and redacted real behavior proof from the contributor.

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

Yes, from source inspection: current main computes native fallback without checking plugin owner enforcement, and the existing resolver test asserts the old allowed-native behavior. I did not run tests because this review was required to keep the checkout read-only.

Is this the best way to solve the issue?

Mostly yes: changing the native fallback to respect requireOwner is the narrow maintainable fix for the reported gap. Before merge, maintainers should require real behavior proof and a changelog entry, and confirm the intended commands.allowFrom precedence under plugin owner enforcement.

Full review comments:

[P3] Add the required changelog entry — src/auto-reply/command-auth.ts:441-443
This changes user-visible command authorization behavior, but the PR does not update CHANGELOG.md. OpenClaw policy requires a single-line changelog entry for user-facing fix/security behavior changes before merge.
Confidence: 0.86

Overall correctness: patch is correct
Overall confidence: 0.82

Acceptance criteria:

pnpm test src/auto-reply/command-control.test.ts src/auto-reply/reply/commands-stop-target.test.ts src/auto-reply/reply/commands-subagents-routing.test.ts
pnpm check:changed

What I checked:

Current main native fallback gap: nativeCommandAuthorized is computed from !ownerAllowlistConfigured, so plugin enforceOwnerForCommands does not currently disable native fallback when no explicit owner allowlist is configured. (src/auto-reply/command-auth.ts:709, 1831e124b221)
Existing regression expectation proves current behavior: The current resolver test explicitly expects a non-owner native command sender to remain authorized under plugin owner enforcement with wildcard channel allowlist. (src/auto-reply/command-control.test.ts:205, 1831e124b221)
Owner enforcement contract: Slash-command docs say enforceOwnerForCommands requires owner identity and that wildcard channel allowlists are not sufficient for owner-only commands. Public docs: docs/tools/slash-commands.md. (docs/tools/slash-commands.md:98, 1831e124b221)
Bundled owner-enforced plugin exists: The WhatsApp command policy enables enforceOwnerForCommands, making this a real command-auth surface rather than a dead contract. (extensions/whatsapp/src/command-policy.ts:3, 1831e124b221)
Real behavior proof is missing: The PR body says the real environment and local command execution were not run, and the head SHA has a failing Real behavior proof check. (5c9e75ce088a)
Patch surface lacks changelog: The PR diff changes user-visible command authorization behavior but only touches command auth and tests, not CHANGELOG.md. (5c9e75ce088a)

Likely related people:

@drobison00: GitHub commit history shows commit 2aa93d4 introduced the owner-enforced command identity requirement and its initial regression coverage. (role: introduced behavior; confidence: high; commits: 2aa93d44a1b2; files: src/auto-reply/command-auth.ts, src/auto-reply/command-control.test.ts, docs/tools/slash-commands.md)
@obviyus: GitHub history shows recent native command authorization fixes, including channel-native command auth and preserving owner allowlists for native auth. (role: adjacent owner; confidence: high; commits: 7b91f06384e1, 8af50b5b4c2a; files: src/auto-reply/command-auth.ts, src/auto-reply/command-control.test.ts)
@steipete: GitHub history shows major command sender identity refactoring and command policy extraction adjacent to this auth path. (role: recent maintainer; confidence: medium; commits: 3fd2a94404d2, 0c95e3f0737f; files: src/auto-reply/command-auth.ts, extensions/whatsapp/src/command-policy.ts)

Remaining risk / open question:

External real behavior proof is absent and the dedicated proof check failed, so the contributor still needs to show the changed native command path running after the patch.
The PR changes user-facing command authorization behavior without the required CHANGELOG.md entry.

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

pgondhi987 · 2026-05-07T09:03:39Z

Not applicable; changelog is handled at merge time.

Quoted comment from @clawsweeper:

Codex review: needs real behavior proof before merge.

Summary
The PR changes command authorization so native command fallback is disabled under plugin owner enforcement, then adds resolver, /stop, and /subagents spawn regression coverage.

Reproducibility: yes. from source inspection: current main computes native fallback without checking plugin owner enforcement, and the existing resolver test asserts the old allowed-native behavior. I did not run tests because this review was required to keep the checkout read-only.

Real behavior proof
Needs real behavior proof before merge: The PR body says no real environment or local command execution was run, and the Real behavior proof check failed on the head SHA. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
External real behavior proof is missing, the proof check failed, and the protected maintainer label requires explicit maintainer handling before merge.

Security
Cleared: The diff narrows native command eligibility and adds tests; I found no new dependency, secret handling, CI, or supply-chain expansion.

Review findings

[P3] Add the required changelog entry — src/auto-reply/command-auth.ts:441-443

Review details

Best possible solution:

Land a narrow auth fix that makes native and text command authorization honor the existing owner-enforcement contract, with targeted regression tests, a changelog entry, and redacted real behavior proof from the contributor.

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

Yes, from source inspection: current main computes native fallback without checking plugin owner enforcement, and the existing resolver test asserts the old allowed-native behavior. I did not run tests because this review was required to keep the checkout read-only.

Is this the best way to solve the issue?

Mostly yes: changing the native fallback to respect requireOwner is the narrow maintainable fix for the reported gap. Before merge, maintainers should require real behavior proof and a changelog entry, and confirm the intended commands.allowFrom precedence under plugin owner enforcement.

Full review comments:

[P3] Add the required changelog entry — src/auto-reply/command-auth.ts:441-443
This changes user-visible command authorization behavior, but the PR does not update CHANGELOG.md. OpenClaw policy requires a single-line changelog entry for user-facing fix/security behavior changes before merge.
Confidence: 0.86

Overall correctness: patch is correct
Overall confidence: 0.82

Acceptance criteria:

pnpm test src/auto-reply/command-control.test.ts src/auto-reply/reply/commands-stop-target.test.ts src/auto-reply/reply/commands-subagents-routing.test.ts

pnpm check:changed

What I checked:

Current main native fallback gap: nativeCommandAuthorized is computed from !ownerAllowlistConfigured, so plugin enforceOwnerForCommands does not currently disable native fallback when no explicit owner allowlist is configured. (src/auto-reply/command-auth.ts:709, 1831e124b221)

Existing regression expectation proves current behavior: The current resolver test explicitly expects a non-owner native command sender to remain authorized under plugin owner enforcement with wildcard channel allowlist. (src/auto-reply/command-control.test.ts:205, 1831e124b221)

Owner enforcement contract: Slash-command docs say enforceOwnerForCommands requires owner identity and that wildcard channel allowlists are not sufficient for owner-only commands. Public docs: docs/tools/slash-commands.md. (docs/tools/slash-commands.md:98, 1831e124b221)

Bundled owner-enforced plugin exists: The WhatsApp command policy enables enforceOwnerForCommands, making this a real command-auth surface rather than a dead contract. (extensions/whatsapp/src/command-policy.ts:3, 1831e124b221)

Real behavior proof is missing: The PR body says the real environment and local command execution were not run, and the head SHA has a failing Real behavior proof check. (5c9e75ce088a)

Patch surface lacks changelog: The PR diff changes user-visible command authorization behavior but only touches command auth and tests, not CHANGELOG.md. (5c9e75ce088a)

Likely related people:

@drobison00: GitHub commit history shows commit 2aa93d4 introduced the owner-enforced command identity requirement and its initial regression coverage. (role: introduced behavior; confidence: high; commits: 2aa93d44a1b2; files: src/auto-reply/command-auth.ts, src/auto-reply/command-control.test.ts, docs/tools/slash-commands.md)

@obviyus: GitHub history shows recent native command authorization fixes, including channel-native command auth and preserving owner allowlists for native auth. (role: adjacent owner; confidence: high; commits: 7b91f06384e1, 8af50b5b4c2a; files: src/auto-reply/command-auth.ts, src/auto-reply/command-control.test.ts)

@steipete: GitHub history shows major command sender identity refactoring and command policy extraction adjacent to this auth path. (role: recent maintainer; confidence: medium; commits: 3fd2a94404d2, 0c95e3f0737f; files: src/auto-reply/command-auth.ts, extensions/whatsapp/src/command-policy.ts)

Remaining risk / open question:

External real behavior proof is absent and the dedicated proof check failed, so the contributor still needs to show the changed native command path running after the patch.

The PR changes user-facing command authorization behavior without the required CHANGELOG.md entry.

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

* fix: honor owner enforcement for native commands * addressing codex review * addressing codex review * docs: add changelog entry for PR merge

fix: honor owner enforcement for native commands

6d4ac6a

openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 7, 2026

addressing codex review

5de82c4

openclaw-barnacle Bot added size: M and removed size: S labels May 7, 2026

addressing codex review

5c9e75c

pgondhi987 added 2 commits May 7, 2026 09:56

Merge remote-tracking branch 'origin/main' into fix/fix-572

0d1fc21

docs: add changelog entry for PR merge

bf4c321

pgondhi987 merged commit 7580513 into openclaw:main May 7, 2026
21 of 22 checks passed

github-actions Bot mentioned this pull request May 7, 2026

📡 Upstream Digest — 2026-05-07 10:14 UTC curtismercier/openclaw-mods#790

Open

steipete pushed a commit that referenced this pull request May 7, 2026

Honor owner enforcement for native commands [AI] (#78864)

cd070c2

* fix: honor owner enforcement for native commands * addressing codex review * addressing codex review * docs: add changelog entry for PR merge

github-actions Bot mentioned this pull request May 9, 2026

Update ghcr.io/openclaw/openclaw Docker tag to v2026.5.7 - autoclosed claytono/infra#1977

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Honor owner enforcement for native commands [AI]#78864

Honor owner enforcement for native commands [AI]#78864
pgondhi987 merged 5 commits intoopenclaw:mainfrom
pgondhi987:fix/fix-572

pgondhi987 commented May 7, 2026

Uh oh!

clawsweeper Bot commented May 7, 2026 •

edited

Loading

Uh oh!

pgondhi987 commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

pgondhi987 commented May 7, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Real behavior proof (required for external PRs)

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

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

Uh oh!

Uh oh!

pgondhi987 commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented May 7, 2026 •

edited

Loading