fix(security): IM permission passthrough + workspace symlink escape#546
Open
whitelonng wants to merge 1 commit into
Open
fix(security): IM permission passthrough + workspace symlink escape#546whitelonng wants to merge 1 commit into
whitelonng wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supersedes #536 (closed). Two independent security fixes, audited from the same investigation. The IM permission approach is changed from #536's downgrade-to-
autoto pure passthrough, per review feedback.1. IM agent runs follow the user-chosen sandbox/approval policy (pure passthrough)
src/main/claw-runtime.tshardcoded IM-sourced turns (Feishu / WeChat / Telegram) toapprovalPolicy='auto'+sandboxMode='danger-full-access', silently overriding the unified tool-permission picker. A user who selectedread-onlyorneverstill got full-write auto-approved execution when an IM contact sent a message.#536 fixed this by downgrading GUI-required policies to
autofor 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:approvalPolicyandsandboxModeare read verbatim fromruntimeSettings.agents.kun(the same object the GUI reads viagetKunRuntimeSettings(settings)) and forwarded unchanged on both the thread-create body and the turn body.IM_GUI_REQUIRED_APPROVAL_POLICIES,imSandboxMode,imApprovalPolicy, and the downgradelogErrorare all removed. The claw-runtime no longer makes any permission decision for IM.disableUserInput: trueis 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.tsusedpath.resolve()which is purely lexical and does not follow symlinks. A symlink at<workspace>/link -> /etc/passwdpassed the escape check, letting read/write/edit/ls/find/grep/LSP tools reach files outside the workspace.fs.realpath()before therelative()escape check.resolveSymlinkSafehelper 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).EACCES/ELOOP/ENOTDIR→ treated as non-existent; unexpected errno rethrows./tmp→/private/tmp).awaitthe now-asyncresolveWorkspacePath.Changes
src/main/claw-runtime.ts— remove hardcodedCLAW_IM_APPROVAL_POLICY/CLAW_IM_SANDBOX_MODEconstants; passruntimeSettings.agents.kun.approvalPolicy/.sandboxModethrough verbatim on both thread-create and turn bodies; keepdisableUserInput: true.src/main/claw-runtime.test.ts— 4 new tests:untrusted/read-onlypassthrough (no downgrade)never/read-onlypassthrough (no escalation toauto)source: 'task'negative test (no IM-only fields on body)kun/src/adapters/tool/builtin-tool-utils.ts— asyncresolveWorkspacePath+safeRealpath+resolveSymlinkSafe; returns lexical paths.kun/src/adapters/tool/builtin-file-tools.ts,builtin-lsp-tool.ts,builtin-read-tool.ts,builtin-search-tools.ts—await resolveWorkspacePath(...).Tests
npx vitest run src/main/claw-runtime.test.ts— 50/50cd kun && npx vitest run src/adapters/tool/— 49/49npm run typecheck— cleannpm run lint— 0 errors