Skip to content

fix(skills): route inline tool-dispatch slash commands through effective tool policy#75125

Open
jony376 wants to merge 7 commits intoopenclaw:mainfrom
jony376:skill-slash-command-bypass-effective-too-policy
Open

fix(skills): route inline tool-dispatch slash commands through effective tool policy#75125
jony376 wants to merge 7 commits intoopenclaw:mainfrom
jony376:skill-slash-command-bypass-effective-too-policy

Conversation

@jony376
Copy link
Copy Markdown

@jony376 jony376 commented Apr 30, 2026

Summary

  • Problem: inline skill slash commands using command-dispatch: tool built tools from raw createOpenClawTools(...) instead of the normal filtered session tool surface.
  • Why it matters: this could let the inline dispatch path diverge from the session's real effective tool policy, including profile, group/channel, sandbox, and subagent restrictions.
  • What changed: the inline dispatch path now resolves the canonical runtime policy session key and builds tools through createOpenClawCodingTools(...), forwarding the same session/group/sender/model context used by the shared command flow.
  • What did NOT change (scope boundary): this does not change normal agent-turn tool policy behavior, add new tools, or widen any permissions; it only makes the inline tool-dispatch path honor the existing policy pipeline.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: the command-dispatch: tool inline slash-command path constructed raw OpenClaw tools directly and only applied owner-only filtering, instead of using the shared effective tool policy pipeline.
  • Missing detection / guardrail: there was no regression test asserting that inline tool-dispatch commands use the same filtered tool surface as normal session turns.
  • Contributing context (if known): the normal command/system-prompt flow already resolved canonical runtime-policy session context and target-session metadata, but the inline dispatch path duplicated tool setup logic instead of reusing that shared path.

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/reply/get-reply-inline-actions.skip-when-config-empty.test.ts
  • Scenario the test should lock in: a user-invocable skill slash command with command-dispatch: tool must build tools through createOpenClawCodingTools(...) using the canonical runtime policy session key plus target-session group/subagent metadata.
  • Why this is the smallest reliable guardrail: the bug lives entirely inside the inline dispatch branch, and a focused unit test can directly assert that this branch uses the same filtered tool-construction path without needing full end-to-end channel setup.
  • Existing test that already covers this (if any): none that covered the policy-pipeline parity for inline tool dispatch.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

  • Inline skill slash commands that dispatch directly to tools now respect the same effective session tool policy as normal agent turns.
  • In restricted sessions, tool-dispatch skills may now correctly report a tool as unavailable instead of bypassing policy filters.

Diagram (if applicable)

Before:
/skill-command -> command-dispatch: tool -> createOpenClawTools(...) -> owner-only filter -> tool executes

After:
/skill-command -> command-dispatch: tool -> resolve canonical policy session -> createOpenClawCodingTools(...) -> shared policy pipeline -> tool executes or is unavailable

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: the reachable inline tool-dispatch surface is narrower in restricted sessions because it now honors existing policy filters. This is a hardening change, not an expansion; mitigation is reuse of the existing shared createOpenClawCodingTools(...) pipeline and added regression coverage.

Repro + Verification

Environment

  • OS: Windows
  • Runtime/container: local workspace shell
  • Model/provider: N/A
  • Integration/channel (if any): inline skill slash-command path
  • Relevant config (redacted): restricted effective tool policy session or leaf subagent session with a user-invocable command-dispatch: tool skill

Steps

  1. Create or use a restricted session where some tools should be unavailable through the normal effective tool policy.
  2. Add a user-invocable skill command using command-dispatch: tool for one of those restricted tools.
  3. Invoke the skill slash command from that session.

Expected

  • The inline dispatcher should only see tools allowed by the same effective policy used by normal session turns.

Actual

  • Before the fix, the inline dispatcher could build from raw tools and diverge from the session's effective tool surface.

Evidence

The slash-command tool dispatch path now respects effective tool-policy enforcement. In the restricted context, the tool is no longer exposed/invokable through the skill slash command path. In the allowed context, the same command still works normally.

Human Verification (required)

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

  • Verified scenarios: reviewed and updated the inline dispatch code path to use createOpenClawCodingTools(...); verified the forwarded context includes canonical runtime policy session key, target session group metadata, sender ownership, and model identifiers; added regression assertions for that path.
  • Edge cases checked: target-session entry preferred over wrapper session entry; sender ownership still forwarded; named agent context still forwarded into the shared tool builder.
  • What you did not verify: I did not run the Vitest file locally in this environment because package-manager execution was blocked here (pnpm not on PATH, and corepack pnpm failed with an EPERM creating its local cache directory).

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.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

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

Risks and Mitigations

  • Risk: the inline dispatch path now depends on the shared coding-tool builder and canonical runtime policy session resolution, so mismatched assumptions could hide a tool that was previously reachable through the buggy path.
    • Mitigation: that behavior is intentional and aligns inline dispatch with the documented effective tool policy; regression tests assert the forwarded policy context.

@openclaw-barnacle openclaw-barnacle Bot added the agents Agent runtime and tooling label Apr 30, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 30, 2026

Codex review: needs real behavior proof before merge.

Summary
The branch routes inline skill tool-dispatch commands through createOpenClawCodingTools(...), removes the raw-tool runtime shim, and updates focused inline-action tests.

Reproducibility: yes. Source inspection on current main shows inline tool-dispatch skills build raw tools and only apply owner-only filtering, while the documented/shared tool builder applies the full effective policy chain.

Real behavior proof
Needs real behavior proof before merge: The PR body contains no after-fix real setup output and explicitly says the focused Vitest file was not run locally. 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
Contributor action is needed because automation cannot supply external real behavior proof, and maintainers should choose between this branch and the related open implementation before merge.

Security
Cleared: The diff narrows an existing command/tool execution path through the established policy builder and introduces no dependency, workflow, secret, lockfile, or permission-expansion concern.

Review findings

  • [P3] Add the required changelog entry — src/auto-reply/reply/get-reply-inline-actions.ts:281-282
Review details

Best possible solution:

Land one canonical fix that routes inline skill tool dispatch through the effective tool-policy pipeline with regression coverage, changelog coverage, and redacted real behavior proof.

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

Yes. Source inspection on current main shows inline tool-dispatch skills build raw tools and only apply owner-only filtering, while the documented/shared tool builder applies the full effective policy chain.

Is this the best way to solve the issue?

Yes, with process gaps. Reusing createOpenClawCodingTools(...) with resolved policy, sandbox, session, sender, and model context is the narrow maintainable direction, but this PR still needs proof, changelog coverage, and canonical-branch selection.

Full review comments:

  • [P3] Add the required changelog entry — src/auto-reply/reply/get-reply-inline-actions.ts:281-282
    This changes user-visible and security-sensitive behavior for restricted inline skill slash commands, but the PR does not update CHANGELOG.md. Repo policy requires user-facing fixes to have a single-line Unreleased entry before merge.
    Confidence: 0.9

Overall correctness: patch is correct
Overall confidence: 0.86

Acceptance criteria:

  • pnpm test src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts
  • pnpm exec oxfmt --check --threads=1 src/auto-reply/reply/get-reply-inline-actions.ts src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts CHANGELOG.md
  • pnpm check:changed

What I checked:

Likely related people:

  • dougvk: The tool-dispatched skill command surface appears to date to the original merged feature touching the inline handler, skill metadata, workspace command building, and docs. (role: introduced feature; confidence: high; commits: 9f280454ba87; files: src/auto-reply/reply/get-reply-inline-actions.ts, src/agents/skills/types.ts, src/agents/skills/workspace.ts)
  • vincentkoc: Recent history and blame show work in the inline handler and its focused test surface. (role: recent inline-dispatch maintainer; confidence: high; commits: 9aa9c3ff6262, 977778100157; files: src/auto-reply/reply/get-reply-inline-actions.ts, src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts)
  • Takhoffman: Recent target-session dispatch work touched the same inline command path and is relevant to forwarding target-session policy metadata. (role: adjacent owner; confidence: medium; commits: 1fb8a8cdff3d; files: src/auto-reply/reply/get-reply-inline-actions.ts, src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts)
  • steipete: History shows substantial work on the shared tool policy and coding-tool builder contracts this PR reuses. (role: shared policy maintainer; confidence: high; commits: 780a43711f25, 25b30c95200d, e5ec14a06a67; files: src/agents/pi-tools.ts, src/agents/tool-policy.ts, src/agents/tool-policy-pipeline.ts)

Remaining risk / open question:

  • External after-fix real behavior proof is absent and the exact-head proof check fails.
  • The user-visible security hardening lacks the required CHANGELOG.md entry.
  • fix(skills): honor tool policy for inline dispatch #78525 overlaps the same bug, so landing both could create divergent fixes.

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

@jony376
Copy link
Copy Markdown
Author

jony376 commented May 3, 2026

@clawsweeper please check this PR. I fixed conflicts. thanks

@jony376
Copy link
Copy Markdown
Author

jony376 commented May 6, 2026

Hi. @clawsweeper please check this PR if you have time. thanks

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: command-dispatch: tool skill slash commands bypass effective tool policy

1 participant