fix: translate guest paths inside --allowedTools and --disallowedTools (#411)

cleanSpawnArgs only translated --add-dir and --plugin-dir flag pairs.
The Electron app emits permission patterns like
Edit(//sessions/{name}/mnt/.auto-memory/**) and Write(...) inside the
single comma-separated --allowedTools value, and those reached the
spawned `claude` CLI verbatim. Permission rules referencing
non-existent guest paths cannot match the real on-disk locations, so
auto-memory grants silently no-op even after #389 made the underlying
path resolvable and #392 fixed the cwd resolution.

This adds two helpers and wires them into cleanSpawnArgs:

  splitToolList(csv):
    Paren-aware split so "Bash(npm test, npm build)" is one entry
    rather than two. Returns an array of raw entries.

  translateEmbeddedGuestPaths(csv, mountMap):
    Walks each entry. "Tool" is passed through. "Tool(pattern)" is
    translated when the pattern looks like a /sessions/ guest path.
    Defensively normalizes leading "//" (the Electron app emits double
    slashes via path.join('/', '/sessions/...')). Entries whose mount
    cannot be resolved are dropped from the CSV; the flag itself is
    kept (a permission rule that can never match is worse than absent).

cleanSpawnArgs now recognizes --allowedTools and --disallowedTools as
"tool-list flags" alongside the existing single-path flags. Single-path
behavior is unchanged.

BATS coverage in tests/cowork-path-translation.bats covers
splitToolList (paren handling, empty/null), translateEmbeddedGuestPaths
(passthrough, double/single-slash translation, drop-on-miss, host-path
passthrough, mcp__ tool names, empty/null), and the cleanSpawnArgs
integration for both new flag types.

Refs: #245 (umbrella), #389 (memory env translation), #392 (cwd fix).

Co-authored-by: Claude Opus 4 <noreply@anthropic.com>
This commit is contained in:
RayCharlizard
2026-04-18 18:51:38 -05:00
committed by GitHub
parent 37379b45ac
commit 951462363e
2 changed files with 319 additions and 17 deletions

View File

@@ -324,21 +324,91 @@ function buildSpawnEnv(appEnv, mountMap) {
return mergedEnv;
}
/**
* Split a CSV --allowedTools / --disallowedTools value into entries
* while respecting parentheses. Tool patterns may legitimately contain
* commas inside parens (e.g. "Bash(npm test, npm build)"), so a naive
* split on "," would corrupt them. Returns an array of entries with no
* trimming applied.
*/
function splitToolList(csv) {
const result = [];
if (!csv) return result;
let depth = 0;
let start = 0;
for (let i = 0; i < csv.length; i++) {
const ch = csv[i];
if (ch === '(') depth++;
else if (ch === ')') depth = Math.max(0, depth - 1);
else if (ch === ',' && depth === 0) {
result.push(csv.slice(start, i));
start = i + 1;
}
}
result.push(csv.slice(start));
return result;
}
/**
* Translate VM guest paths embedded inside a CSV tool-permission
* string (e.g. --allowedTools value). Each entry is either "Tool"
* (passed through) or "Tool(pattern)" (pattern is translated if it
* looks like a /sessions/ guest path). Entries whose guest path can't
* be mapped to a host path are dropped — a permission rule that
* can never match is worse than absent.
*
* Defensively normalizes leading double slashes (the Electron app
* emits "//sessions/..." due to an upstream path.join('/', ...) on an
* already-absolute path).
*/
function translateEmbeddedGuestPaths(csv, mountMap) {
if (!csv) return csv;
const out = [];
for (const entry of splitToolList(csv)) {
const m = entry.match(/^(\w+)\(([^)]+)\)$/);
if (!m) {
out.push(entry);
continue;
}
const tool = m[1];
const normalized = m[2].replace(/^\/+/, '/');
if (!normalized.startsWith('/sessions/')) {
out.push(entry);
continue;
}
const translated = translateGuestPath(normalized, mountMap || {});
if (!translated) {
log(`translateEmbeddedGuestPaths: dropping "${entry}" (no host mapping)`);
continue;
}
log(`translateEmbeddedGuestPaths: ${entry} -> ${tool}(${translated})`);
out.push(`${tool}(${translated})`);
}
return out.join(',');
}
/**
* Translate args that reference VM guest paths (/sessions/...) to host
* paths using mountMap. If translation fails, the flag pair is removed.
* paths using mountMap. Two flag styles are handled:
* - Single-path flags (--add-dir, --plugin-dir): the value is one
* guest path. Translation failure drops the whole flag pair.
* - Tool-list flags (--allowedTools, --disallowedTools): the value
* is a CSV of "Tool" or "Tool(pattern)" entries. Each entry is
* translated independently; entries that fail are dropped from
* the CSV but the flag itself is retained.
*/
function cleanSpawnArgs(rawArgs, mountMap) {
const cleanArgs = [];
const guestPathFlags = new Set(['--add-dir', '--plugin-dir']);
const toolListFlags = new Set(['--allowedTools', '--disallowedTools']);
for (let i = 0; i < rawArgs.length; i++) {
if (guestPathFlags.has(rawArgs[i]) &&
const flag = rawArgs[i];
const value = rawArgs[i + 1];
if (guestPathFlags.has(flag) &&
i + 1 < rawArgs.length &&
rawArgs[i + 1].startsWith('/sessions/')) {
const flag = rawArgs[i];
let hostPath = translateGuestPath(
rawArgs[i + 1], mountMap
);
value.startsWith('/sessions/')) {
let hostPath = translateGuestPath(value, mountMap);
if (hostPath) {
// --plugin-dir needs the plugin root, not a skills/
// subdirectory — walk up to find it.
@@ -347,15 +417,25 @@ function cleanSpawnArgs(rawArgs, mountMap) {
hostPath, os.homedir()
);
}
log(`cleanSpawnArgs: translated ${flag} ${rawArgs[i + 1]} -> ${hostPath}`);
log(`cleanSpawnArgs: translated ${flag} ${value} -> ${hostPath}`);
cleanArgs.push(flag, hostPath);
} else {
log(`cleanSpawnArgs: removing ${flag} ${rawArgs[i + 1]} (no host mapping)`);
log(`cleanSpawnArgs: removing ${flag} ${value} (no host mapping)`);
}
i++;
continue;
}
cleanArgs.push(rawArgs[i]);
if (toolListFlags.has(flag) && i + 1 < rawArgs.length) {
cleanArgs.push(
flag,
translateEmbeddedGuestPaths(value, mountMap),
);
i++;
continue;
}
cleanArgs.push(flag);
}
return cleanArgs;
}

View File

@@ -96,17 +96,58 @@ function resolvePluginRoot(pluginPath, mountBase) {
return pluginPath;
}
function splitToolList(csv) {
const result = [];
if (!csv) return result;
let depth = 0;
let start = 0;
for (let i = 0; i < csv.length; i++) {
const ch = csv[i];
if (ch === "(") depth++;
else if (ch === ")") depth = Math.max(0, depth - 1);
else if (ch === "," && depth === 0) {
result.push(csv.slice(start, i));
start = i + 1;
}
}
result.push(csv.slice(start));
return result;
}
function translateEmbeddedGuestPaths(csv, mountMap) {
if (!csv) return csv;
const out = [];
for (const entry of splitToolList(csv)) {
const m = entry.match(/^(\w+)\(([^)]+)\)$/);
if (!m) {
out.push(entry);
continue;
}
const tool = m[1];
const normalized = m[2].replace(/^\/+/, "/");
if (!normalized.startsWith("/sessions/")) {
out.push(entry);
continue;
}
const translated = translateGuestPath(normalized, mountMap || {});
if (!translated) continue;
out.push(`${tool}(${translated})`);
}
return out.join(",");
}
function cleanSpawnArgs(rawArgs, mountMap) {
const cleanArgs = [];
const guestPathFlags = new Set(["--add-dir", "--plugin-dir"]);
const toolListFlags = new Set(["--allowedTools", "--disallowedTools"]);
for (let i = 0; i < rawArgs.length; i++) {
if (guestPathFlags.has(rawArgs[i]) &&
const flag = rawArgs[i];
const value = rawArgs[i + 1];
if (guestPathFlags.has(flag) &&
i + 1 < rawArgs.length &&
rawArgs[i + 1].startsWith("/sessions/")) {
const flag = rawArgs[i];
let hostPath = translateGuestPath(
rawArgs[i + 1], mountMap || {}
);
value.startsWith("/sessions/")) {
let hostPath = translateGuestPath(value, mountMap || {});
if (hostPath) {
if (flag === "--plugin-dir") {
hostPath = resolvePluginRoot(
@@ -120,7 +161,17 @@ function cleanSpawnArgs(rawArgs, mountMap) {
i++;
continue;
}
cleanArgs.push(rawArgs[i]);
if (toolListFlags.has(flag) && i + 1 < rawArgs.length) {
cleanArgs.push(
flag,
translateEmbeddedGuestPaths(value, mountMap),
);
i++;
continue;
}
cleanArgs.push(flag);
}
return cleanArgs;
}
@@ -552,6 +603,177 @@ assertDeepEqual(result,
[[ "$status" -eq 0 ]]
}
@test "cleanSpawnArgs: translates --allowedTools embedded guest paths" {
run node -e "${NODE_PREAMBLE}
const result = cleanSpawnArgs(
[
'--allowedTools',
'Read,Edit,Edit(//sessions/abc/mnt/.auto-memory/**),Write(//sessions/abc/mnt/.auto-memory/**)'
],
{'.auto-memory': '/host/memory'}
);
assertDeepEqual(result,
[
'--allowedTools',
'Read,Edit,Edit(/host/memory/**),Write(/host/memory/**)'
],
'--allowedTools translated, plain entries preserved');
"
[[ "$status" -eq 0 ]]
}
@test "cleanSpawnArgs: translates --disallowedTools embedded guest paths" {
run node -e "${NODE_PREAMBLE}
const result = cleanSpawnArgs(
[
'--disallowedTools',
'Bash(rm),Edit(//sessions/abc/mnt/data/secret/**)'
],
{'data': '/host/data'}
);
assertDeepEqual(result,
[
'--disallowedTools',
'Bash(rm),Edit(/host/data/secret/**)'
],
'--disallowedTools translated');
"
[[ "$status" -eq 0 ]]
}
# =============================================================================
# splitToolList
# =============================================================================
@test "splitToolList: empty / null input" {
run node -e "${NODE_PREAMBLE}
assertDeepEqual(splitToolList(''), [], 'empty string -> []');
assertDeepEqual(splitToolList(null), [], 'null -> []');
assertDeepEqual(splitToolList(undefined), [], 'undefined -> []');
"
[[ "$status" -eq 0 ]]
}
@test "splitToolList: simple CSV with no parens" {
run node -e "${NODE_PREAMBLE}
assertDeepEqual(
splitToolList('Read,Edit,Write'),
['Read', 'Edit', 'Write'],
'plain CSV');
"
[[ "$status" -eq 0 ]]
}
@test "splitToolList: respects parentheses around commas" {
run node -e "${NODE_PREAMBLE}
assertDeepEqual(
splitToolList('Bash(npm test, npm build),Edit,Read'),
['Bash(npm test, npm build)', 'Edit', 'Read'],
'commas inside parens are preserved');
"
[[ "$status" -eq 0 ]]
}
@test "splitToolList: handles trailing entry without comma" {
run node -e "${NODE_PREAMBLE}
assertDeepEqual(
splitToolList('A,B(c,d)'),
['A', 'B(c,d)'],
'final entry includes nested commas');
"
[[ "$status" -eq 0 ]]
}
# =============================================================================
# translateEmbeddedGuestPaths
# =============================================================================
@test "translateEmbeddedGuestPaths: passes through entries without parens" {
run node -e "${NODE_PREAMBLE}
assertEqual(
translateEmbeddedGuestPaths(
'Read,Edit,Write',
{'.auto-memory': '/host/memory'}
),
'Read,Edit,Write',
'plain tool names unchanged');
"
[[ "$status" -eq 0 ]]
}
@test "translateEmbeddedGuestPaths: translates Edit() with double-slash guest path" {
run node -e "${NODE_PREAMBLE}
assertEqual(
translateEmbeddedGuestPaths(
'Edit(//sessions/abc/mnt/.auto-memory/**)',
{'.auto-memory': '/host/memory'}
),
'Edit(/host/memory/**)',
'leading // normalized and translated');
"
[[ "$status" -eq 0 ]]
}
@test "translateEmbeddedGuestPaths: translates entry with single-slash guest path" {
run node -e "${NODE_PREAMBLE}
assertEqual(
translateEmbeddedGuestPaths(
'Write(/sessions/abc/mnt/.auto-memory/**)',
{'.auto-memory': '/host/memory'}
),
'Write(/host/memory/**)',
'single-slash variant also translated');
"
[[ "$status" -eq 0 ]]
}
@test "translateEmbeddedGuestPaths: drops entries whose mount is unknown" {
run node -e "${NODE_PREAMBLE}
assertEqual(
translateEmbeddedGuestPaths(
'Read,Edit(//sessions/abc/mnt/unknown/**),Write',
{'.auto-memory': '/host/memory'}
),
'Read,Write',
'unresolvable entry is dropped, others retained');
"
[[ "$status" -eq 0 ]]
}
@test "translateEmbeddedGuestPaths: leaves non-/sessions paths alone" {
run node -e "${NODE_PREAMBLE}
assertEqual(
translateEmbeddedGuestPaths(
'Bash(rm),Edit(/home/user/explicit/file)',
{'.auto-memory': '/host/memory'}
),
'Bash(rm),Edit(/home/user/explicit/file)',
'host paths and non-paths unchanged');
"
[[ "$status" -eq 0 ]]
}
@test "translateEmbeddedGuestPaths: handles MCP-style tool names with underscores" {
run node -e "${NODE_PREAMBLE}
assertEqual(
translateEmbeddedGuestPaths(
'mcp__server__tool(//sessions/abc/mnt/data/**)',
{'data': '/host/data'}
),
'mcp__server__tool(/host/data/**)',
'mcp-style tool name preserved');
"
[[ "$status" -eq 0 ]]
}
@test "translateEmbeddedGuestPaths: empty / null input" {
run node -e "${NODE_PREAMBLE}
assertEqual(translateEmbeddedGuestPaths('', {}), '', 'empty -> empty');
assertEqual(translateEmbeddedGuestPaths(null, {}), null, 'null -> null');
"
[[ "$status" -eq 0 ]]
}
# =============================================================================
# resolvePluginRoot
# =============================================================================