fix(exec): allow skills-store and user allow_paths as working_dir

[codebit0]

Summary

ExecTool's working_dir validation passed nil as the base allow-prefix list to resolvePathWithAllowed, while file tools (read_file, list_files, write_file, edit, send_file) all use t.allowedPrefixes seeded in wireExtraTools with the skills directories and user-configured allow_paths.

The asymmetry meant agents could read files inside a skill's directory but could not cd there to run its scripts — they had to fall back to absolute paths with cat, losing the natural cd skill && ./script idiom. Concretely, cron-triggered seed runs hit a deny for working_dir=/.../skills-store/<skill>/<ver> while harvesting keyword data, and the agent learned to route around it with absolute paths.

Changes

• Add allowedPrefixes to ExecTool and implement the existing PathAllowable interface so wireExtraTools wires it identically to the file tools.
• shell.go working_dir validation now uses t.allowedPrefixes as the base (still write-class, so team-root remains excluded for cross-chat safety).
• Generalize PathAllowable's doc comment — the interface is already used by write/edit tools and now exec, so the original "for read access" phrasing was misleading.
• Neutralize the "read_file: access denied" log in resolvePathWithAllowed. That function is shared by read_file, list_files, read_image, send_file, and exec working_dir validation; the message now reads "path access denied" and the actual caller is identifiable via the surrounding tool_call_update event.

Out of scope

Subagent ExecTool, constructed separately in buildSubagentToolsRegistry, deliberately does NOT inherit this wiring — that's a parallel inconsistency that does not affect top-level (cron-triggered) sessions and is left as a separate concern.

Test plan

• Direct MCP bridge tools/call name=exec working_dir=<skills-store/<skill>/<ver>> succeeds (previously denied)
• Cron-triggered agent runs a skill script with cd skill_dir && ./script pattern successfully
• read_file / list_files for paths outside skills-store still deny as before (no widening of read scope)
• Log entry on deny now reads path access denied (not read_file: access denied)

[mrgoonie]

Summary: This PR is directionally useful, but it is not merge-ready yet because the same working_dir allow-list fix is missing from the credentialed-exec path, and the branch is currently conflicted.
Risk level: Medium
Mandatory gates:

• Duplicate/prior implementation: clear; no matching merged PR/issue found for exec working_dir allow_paths/skills-store parity.
• Project standards: issue found; shell working_dir resolution is duplicated for credentialed and normal exec paths, but the PR updates only the normal shell branch.
• Strategic necessity: clear value; allowing skills to run scripts from their own directory reduces brittle absolute-path workarounds in cron/skill runs.
• CI/checks: previous checks were green, but current merge state is DIRTY.
  Findings:
• Critical: none.
• Important: The credentialed-exec branch in internal/tools/shell.go still resolves working_dir with allowedWriteWithTeamWorkspace(ctx, nil). Because credentialed CLIs are routed before the normal shell path, commands such as registered gws/gh still cannot use the newly wired ExecTool.AllowPaths prefixes as working_dir. This leaves the exact skills-store/user allow_paths asymmetry unresolved for secure CLI commands. Please apply the same t.allowedPrefixes base there too, or refactor working_dir resolution into one helper used by both branches.
• Important: GitHub reports mergeStateStatus=DIRTY, so this branch needs a rebase/merge conflict resolution before it can land.
• Suggestion: Add a regression test that covers both normal exec and credentialed-exec working_dir validation with an allowed skills-store or user allow_path prefix, not only the plain shell branch.
  Verdict: REQUEST_CHANGES

[clark-cant]

🦸‍♂️ Maintainer review — PR #1146 looks clean and well-scoped. The fix correctly addresses the asymmetry between exec and file tools for working_dir validation.

Code review:

• ✅ cmd/gateway_tools_wiring.go — properly wires exec into the same AllowPaths pipeline as file tools
• ✅ internal/tools/shell.go — allowedPrefixes field + AllowPaths() method implements PathAllowable correctly, allowedWriteWithTeamWorkspace now receives the base prefixes
• ✅ internal/tools/filesystem.go — caller-neutral log message is the right call since this function is shared across 5+ tools
• ✅ internal/tools/types.go — doc comment update accurately reflects the broader usage
• ✅ Scope is tight (+30/-3), no unrelated changes, subagent ExecTool exclusion is documented and intentional

Verdict: Approve — the code is correct and the fix is valuable (agents should be able to cd skill_dir && ./script).

One blocker: branch needs rebase. Merge state is DIRTY — the branch is behind dev. Please:

git fetch origin
git rebase origin/dev
git push --force-with-lease

After rebase, CI should re-run and this should be merge-ready.

Posted by github-maintain at 2026-06-29T18:53:00Z