Skip to content

fix(security): IM permission passthrough + workspace symlink escape#546

Open
whitelonng wants to merge 1 commit into
KunAgent:developfrom
whitelonng:codex/im-permission-passthrough
Open

fix(security): IM permission passthrough + workspace symlink escape#546
whitelonng wants to merge 1 commit into
KunAgent:developfrom
whitelonng:codex/im-permission-passthrough

Conversation

@whitelonng

Copy link
Copy Markdown
Collaborator

Summary

Supersedes #536 (closed). Two independent security fixes, audited from the same investigation. The IM permission approach is changed from #536's downgrade-to-auto to pure passthrough, per review feedback.

1. IM agent runs follow the user-chosen sandbox/approval policy (pure passthrough)

src/main/claw-runtime.ts hardcoded IM-sourced turns (Feishu / WeChat / Telegram) to approvalPolicy='auto' + sandboxMode='danger-full-access', silently overriding the unified tool-permission picker. A user who selected read-only or never still got full-write auto-approved execution when an IM contact sent a message.

#536 fixed this by downgrading GUI-required policies to auto for IM. Review rejected that approach: the IM path was making a second permission decision on top of the agent settings, which the GUI never does. This PR instead makes IM identical to the GUI — pure passthrough:

  • approvalPolicy and sandboxMode are read verbatim from runtimeSettings.agents.kun (the same object the GUI reads via getKunRuntimeSettings(settings)) and forwarded unchanged on both the thread-create body and the turn body.
  • IM_GUI_REQUIRED_APPROVAL_POLICIES, imSandboxMode, imApprovalPolicy, and the downgrade logError are all removed. The claw-runtime no longer makes any permission decision for IM.
  • disableUserInput: true is retained — it is a UX constraint (IM senders cannot answer structured GUI prompts), not a permission.

Consequence (intended): if the user sets untrusted (GUI confirmation) and sends via IM while away from the computer, that turn's tool calls will wait at the approval gate. This is predictable, transparent behavior — the user chose a strict policy, so it is enforced strictly. No silent downgrade.

2. Workspace path escape check resolves symlinks

kun/src/adapters/tool/builtin-tool-utils.ts used path.resolve() which is purely lexical and does not follow symlinks. A symlink at <workspace>/link -> /etc/passwd passed the escape check, letting read/write/edit/ls/find/grep/LSP tools reach files outside the workspace.

  • Both workspace root and target path run through fs.realpath() before the relative() escape check.
  • New resolveSymlinkSafe helper handles the write/create case (target doesn't exist yet) by walking up to the nearest existing ancestor, realpath-ing it, and re-joining the suffix (loop-guarded at 128 iterations).
  • Error handling is fail-closed: EACCES / ELOOP / ENOTDIR → treated as non-existent; unexpected errno rethrows.
  • Realpath-resolved pair is used ONLY for the escape check; lexical paths are returned to callers so downstream code (subprocess cwd, display paths, LSP init) keeps seeing the user-facing path (important on symlinked roots like macOS /tmp/private/tmp).
  • All 6 tool call sites updated to await the now-async resolveWorkspacePath.

Changes

  • src/main/claw-runtime.ts — remove hardcoded CLAW_IM_APPROVAL_POLICY/CLAW_IM_SANDBOX_MODE constants; pass runtimeSettings.agents.kun.approvalPolicy/.sandboxMode through verbatim on both thread-create and turn bodies; keep disableUserInput: true.
  • src/main/claw-runtime.test.ts — 4 new tests:
    • untrusted/read-only passthrough (no downgrade)
    • never/read-only passthrough (no escalation to auto)
    • source: 'task' negative test (no IM-only fields on body)
    • end-to-end WeChat webhook test (non-default policy arrives on both bodies)
  • kun/src/adapters/tool/builtin-tool-utils.ts — async resolveWorkspacePath + safeRealpath + resolveSymlinkSafe; returns lexical paths.
  • kun/src/adapters/tool/builtin-file-tools.ts, builtin-lsp-tool.ts, builtin-read-tool.ts, builtin-search-tools.tsawait resolveWorkspacePath(...).

Tests

  • npx vitest run src/main/claw-runtime.test.ts — 50/50
  • cd kun && npx vitest run src/adapters/tool/ — 49/49
  • npm run typecheck — clean
  • npm run lint — 0 errors

IM sandbox bypass:
  src/main/claw-runtime.ts hardcoded IM-sourced turns to
  approvalPolicy='auto' + sandboxMode='danger-full-access', silently
  overriding the unified tool-permission picker. A user who selected
  read-only or never still got full-write auto-approved execution when
  an IM contact sent a message.

  Now IM turns pass through runtimeSettings.agents.kun.approvalPolicy
  and .sandboxMode verbatim, matching what the GUI does in
  kun-runtime.ts. The claw-runtime no longer makes any permission
  decision for IM; it reads the same agent settings and forwards them
  unchanged. disableUserInput: true is retained (UX constraint: IM
  senders cannot answer structured GUI prompts).

  Tests: untrusted/read-only passthrough, never/read-only no-escalation,
  source='task' negative test (no IM-only fields), and an end-to-end
  WeChat webhook test verifying non-default policy arrives on both the
  thread-create and turn bodies.

Workspace symlink escape:
  kun/src/adapters/tool/builtin-tool-utils.ts used path.resolve() which
  is purely lexical and does not follow symlinks. A symlink at
  <workspace>/link -> /etc/passwd passed the escape check.

  Both workspace root and target path now run through fs.realpath()
  before the relative() escape check. resolveSymlinkSafe handles the
  write/create case (target doesn't exist yet) by walking up to the
  nearest existing ancestor. Realpath-resolved pair is used ONLY for
  the escape check; lexical paths are returned to callers. All 6 tool
  call sites updated to await the now-async resolveWorkspacePath.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant