fix: isolate remote Claude config from local credentials (#7676)
Fix remote Claude sandbox config isolation and permissions handling.\n\nPR: https://github.com/paperclipai/paperclip/pull/7676
This commit is contained in:
parent
1ca3331c33
commit
2e2da3bc2f
|
|
@ -42,7 +42,7 @@ Core fields:
|
|||
- chrome (boolean, optional): pass --chrome when running Claude
|
||||
- promptTemplate (string, optional): run prompt template
|
||||
- maxTurnsPerRun (number, optional): max turns for one run
|
||||
- dangerouslySkipPermissions (boolean, optional, default true): pass --dangerously-skip-permissions to claude; defaults to true because Paperclip runs Claude in headless --print mode where interactive permission prompts cannot be answered
|
||||
- dangerouslySkipPermissions (boolean, optional, default true): allow non-interactive Claude runs to proceed without approval prompts. Local targets receive --dangerously-skip-permissions; remote targets receive a curated --allowedTools list so they do not inherit local bypass permissions.
|
||||
- command (string, optional): defaults to "claude"
|
||||
- extraArgs (string[], optional): additional CLI args
|
||||
- env (object, optional): KEY=VALUE environment variables
|
||||
|
|
|
|||
|
|
@ -30,7 +30,11 @@ describe("prepareClaudeConfigSeed", () => {
|
|||
cleanupDirs.push(root);
|
||||
const sourceDir = path.join(root, "claude-source");
|
||||
await fs.mkdir(sourceDir, { recursive: true });
|
||||
await fs.writeFile(path.join(sourceDir, "settings.json"), JSON.stringify({ theme: "light" }), "utf8");
|
||||
await fs.writeFile(path.join(sourceDir, "settings.json"), JSON.stringify({
|
||||
theme: "light",
|
||||
permissions: { defaultMode: "bypassPermissions" },
|
||||
}), "utf8");
|
||||
await fs.writeFile(path.join(sourceDir, ".credentials.json"), JSON.stringify({ token: "local" }), "utf8");
|
||||
|
||||
const onLog = vi.fn(async () => {});
|
||||
const env = createEnv(root, sourceDir);
|
||||
|
|
@ -40,7 +44,9 @@ describe("prepareClaudeConfigSeed", () => {
|
|||
|
||||
expect(first).toBe(second);
|
||||
await expect(fs.readFile(path.join(first, "settings.json"), "utf8"))
|
||||
.resolves.toBe(JSON.stringify({ theme: "light" }));
|
||||
.resolves.toBe(JSON.stringify({ theme: "light", permissions: { defaultMode: "default" } }));
|
||||
await expect(fs.access(path.join(first, ".credentials.json")))
|
||||
.rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
|
||||
it("keeps an existing snapshot intact when the seeded files change", async () => {
|
||||
|
|
@ -59,8 +65,47 @@ describe("prepareClaudeConfigSeed", () => {
|
|||
|
||||
expect(second).not.toBe(first);
|
||||
await expect(fs.readFile(path.join(first, "settings.json"), "utf8"))
|
||||
.resolves.toBe(JSON.stringify({ theme: "light" }));
|
||||
.resolves.toBe(JSON.stringify({ theme: "light", permissions: { defaultMode: "default" } }));
|
||||
await expect(fs.readFile(path.join(second, "settings.json"), "utf8"))
|
||||
.resolves.toBe(JSON.stringify({ theme: "dark" }));
|
||||
.resolves.toBe(JSON.stringify({ theme: "dark", permissions: { defaultMode: "default" } }));
|
||||
});
|
||||
|
||||
it("strips local-only settings from remote Claude config seeds", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-config-boundary-"));
|
||||
cleanupDirs.push(root);
|
||||
const sourceDir = path.join(root, "claude-source");
|
||||
await fs.mkdir(sourceDir, { recursive: true });
|
||||
await fs.writeFile(path.join(sourceDir, "settings.json"), JSON.stringify({
|
||||
permissions: {
|
||||
defaultMode: "dontAsk",
|
||||
allow: ["Bash(op item *)"],
|
||||
},
|
||||
hooks: { PreToolUse: [{ matcher: "*" }] },
|
||||
mcpServers: { local: { command: "secret-local-server" } },
|
||||
permissionMode: "dontAsk",
|
||||
skipDangerousModePermissionPrompt: true,
|
||||
}), "utf8");
|
||||
await fs.writeFile(path.join(sourceDir, "settings.local.json"), JSON.stringify({
|
||||
permissions: { defaultMode: "bypassPermissions" },
|
||||
}), "utf8");
|
||||
await fs.writeFile(path.join(sourceDir, "credentials.json"), JSON.stringify({ token: "local" }), "utf8");
|
||||
await fs.writeFile(path.join(sourceDir, "CLAUDE.md"), "local instructions", "utf8");
|
||||
|
||||
const onLog = vi.fn(async () => {});
|
||||
const env = createEnv(root, sourceDir);
|
||||
const seedDir = await prepareClaudeConfigSeed(env, onLog, "company-1");
|
||||
const remoteSettings = JSON.parse(await fs.readFile(path.join(seedDir, "settings.json"), "utf8"));
|
||||
|
||||
expect(remoteSettings.permissions).toEqual({ defaultMode: "default" });
|
||||
expect(remoteSettings.hooks).toBeUndefined();
|
||||
expect(remoteSettings.mcpServers).toBeUndefined();
|
||||
expect(remoteSettings.permissionMode).toBeUndefined();
|
||||
expect(remoteSettings.skipDangerousModePermissionPrompt).toBeUndefined();
|
||||
await expect(fs.access(path.join(seedDir, "settings.local.json")))
|
||||
.rejects.toMatchObject({ code: "ENOENT" });
|
||||
await expect(fs.access(path.join(seedDir, "credentials.json")))
|
||||
.rejects.toMatchObject({ code: "ENOENT" });
|
||||
await expect(fs.readFile(path.join(seedDir, "CLAUDE.md"), "utf8"))
|
||||
.resolves.toBe("local instructions");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5,13 +5,13 @@ import path from "node:path";
|
|||
import type { AdapterExecutionContext } from "@paperclipai/adapter-utils";
|
||||
import { resolvePaperclipInstanceRootForAdapter } from "@paperclipai/adapter-utils/server-utils";
|
||||
|
||||
const SEEDED_SHARED_FILES = [
|
||||
".credentials.json",
|
||||
"credentials.json",
|
||||
"settings.json",
|
||||
"settings.local.json",
|
||||
"CLAUDE.md",
|
||||
] as const;
|
||||
const SEEDED_SHARED_FILES = ["settings.json", "CLAUDE.md"] as const;
|
||||
|
||||
interface SeedFile {
|
||||
name: string;
|
||||
sourcePath: string;
|
||||
contents: Buffer;
|
||||
}
|
||||
|
||||
function nonEmpty(value: string | undefined): string | null {
|
||||
return typeof value === "string" && value.trim().length > 0 ? value.trim() : null;
|
||||
|
|
@ -27,23 +27,48 @@ function isAlreadyExistsError(error: unknown): boolean {
|
|||
return code === "EEXIST" || code === "ENOTEMPTY";
|
||||
}
|
||||
|
||||
async function collectSeedFiles(sourceDir: string): Promise<Array<{ name: string; sourcePath: string }>> {
|
||||
const files: Array<{ name: string; sourcePath: string }> = [];
|
||||
function sanitizeRemoteClaudeSettings(raw: string): string {
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(raw);
|
||||
} catch {
|
||||
return JSON.stringify({ permissions: { defaultMode: "default" } });
|
||||
}
|
||||
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
|
||||
return JSON.stringify({ permissions: { defaultMode: "default" } });
|
||||
}
|
||||
|
||||
const settings = { ...(parsed as Record<string, unknown>) };
|
||||
settings.permissions = { defaultMode: "default" };
|
||||
delete settings.hooks;
|
||||
delete settings.mcpServers;
|
||||
delete settings.permissionMode;
|
||||
delete settings.skipDangerousModePermissionPrompt;
|
||||
return JSON.stringify(settings);
|
||||
}
|
||||
|
||||
async function collectSeedFiles(sourceDir: string): Promise<SeedFile[]> {
|
||||
const files: SeedFile[] = [];
|
||||
for (const name of SEEDED_SHARED_FILES) {
|
||||
const sourcePath = path.join(sourceDir, name);
|
||||
if (!(await pathExists(sourcePath))) continue;
|
||||
files.push({ name, sourcePath });
|
||||
const rawContents = await fs.readFile(sourcePath);
|
||||
const contents = name === "settings.json"
|
||||
? Buffer.from(sanitizeRemoteClaudeSettings(rawContents.toString("utf8")), "utf8")
|
||||
: rawContents;
|
||||
files.push({ name, sourcePath, contents });
|
||||
}
|
||||
return files;
|
||||
}
|
||||
|
||||
async function buildSeedSnapshotKey(files: Array<{ name: string; sourcePath: string }>): Promise<string> {
|
||||
async function buildSeedSnapshotKey(files: SeedFile[]): Promise<string> {
|
||||
if (files.length === 0) return "empty";
|
||||
const hash = createHash("sha256");
|
||||
for (const file of files) {
|
||||
hash.update(file.name);
|
||||
hash.update("\0");
|
||||
hash.update(await fs.readFile(file.sourcePath));
|
||||
hash.update(file.contents);
|
||||
hash.update("\0");
|
||||
}
|
||||
return hash.digest("hex").slice(0, 16);
|
||||
|
|
@ -52,7 +77,7 @@ async function buildSeedSnapshotKey(files: Array<{ name: string; sourcePath: str
|
|||
async function materializeSeedSnapshot(input: {
|
||||
rootDir: string;
|
||||
snapshotKey: string;
|
||||
files: Array<{ name: string; sourcePath: string }>;
|
||||
files: SeedFile[];
|
||||
}): Promise<string> {
|
||||
const targetDir = path.join(input.rootDir, input.snapshotKey);
|
||||
if (await pathExists(targetDir)) {
|
||||
|
|
@ -63,7 +88,7 @@ async function materializeSeedSnapshot(input: {
|
|||
const stagingDir = await fs.mkdtemp(path.join(input.rootDir, ".tmp-"));
|
||||
try {
|
||||
for (const file of input.files) {
|
||||
await fs.copyFile(file.sourcePath, path.join(stagingDir, file.name));
|
||||
await fs.writeFile(path.join(stagingDir, file.name), file.contents);
|
||||
}
|
||||
try {
|
||||
await fs.rename(stagingDir, targetDir);
|
||||
|
|
|
|||
|
|
@ -178,6 +178,11 @@ describe("claude remote execution", () => {
|
|||
const call = runChildProcess.mock.calls[0] as unknown as
|
||||
| [string, string, string[], { env: Record<string, string>; remoteExecution?: { remoteCwd: string } | null }]
|
||||
| undefined;
|
||||
expect(call?.[2]).toContain("--allowedTools");
|
||||
expect(call?.[2]).toContain(
|
||||
"Task AskUserQuestion Bash CronCreate CronDelete CronList Edit EnterPlanMode EnterWorktree ExitPlanMode ExitWorktree Glob Grep Monitor NotebookEdit PushNotification Read RemoteTrigger ScheduleWakeup Skill TaskOutput TaskStop TodoWrite ToolSearch WebFetch WebSearch Write",
|
||||
);
|
||||
expect(call?.[2]).not.toContain("--dangerously-skip-permissions");
|
||||
expect(call?.[2]).toContain("--append-system-prompt-file");
|
||||
expect(call?.[2]).toContain(
|
||||
`${managedRemoteWorkspace}/.paperclip-runtime/claude/skills/agent-instructions.md`,
|
||||
|
|
|
|||
|
|
@ -714,7 +714,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
|||
if (resumeSessionId) args.push("--resume", resumeSessionId);
|
||||
args.push(...buildClaudeExecutionPermissionArgs({
|
||||
dangerouslySkipPermissions,
|
||||
targetIsSandbox: executionTargetIsSandbox,
|
||||
targetIsRemote: executionTargetIsRemote,
|
||||
}));
|
||||
if (chrome) args.push("--chrome");
|
||||
// For Bedrock: only pass --model when the ID is a Bedrock-native identifier
|
||||
|
|
@ -759,9 +759,9 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
|||
if (!resumeSessionId) {
|
||||
commandNotes.push(`Using stable Claude prompt bundle ${promptBundle.bundleKey}.`);
|
||||
}
|
||||
if (dangerouslySkipPermissions && executionTargetIsSandbox) {
|
||||
if (dangerouslySkipPermissions && executionTargetIsRemote) {
|
||||
commandNotes.push(
|
||||
"Using a broad --allowedTools whitelist for sandbox execution because Claude rejects --dangerously-skip-permissions under root/sudo.",
|
||||
"Using a broad --allowedTools whitelist for remote execution so hosted targets do not inherit local Claude bypass permissions.",
|
||||
);
|
||||
}
|
||||
if (attemptInstructionsFilePath && !resumeSessionId) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,51 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { buildClaudeExecutionPermissionArgs, buildClaudeProbePermissionArgs } from "./permissions.js";
|
||||
|
||||
const SANDBOX_ALLOWED_TOOLS =
|
||||
"Task AskUserQuestion Bash CronCreate CronDelete CronList Edit " +
|
||||
"EnterPlanMode EnterWorktree ExitPlanMode ExitWorktree Glob Grep Monitor " +
|
||||
"NotebookEdit PushNotification Read RemoteTrigger ScheduleWakeup Skill " +
|
||||
"TaskOutput TaskStop TodoWrite ToolSearch WebFetch WebSearch Write";
|
||||
|
||||
describe("claude-local remote permission args", () => {
|
||||
it("uses the canonical Bash tool grant for remote execution", () => {
|
||||
expect(buildClaudeExecutionPermissionArgs({ dangerouslySkipPermissions: true, targetIsRemote: true })).toEqual([
|
||||
"--allowedTools",
|
||||
SANDBOX_ALLOWED_TOOLS,
|
||||
]);
|
||||
});
|
||||
|
||||
it("uses the canonical Bash tool grant for remote probes", () => {
|
||||
expect(buildClaudeProbePermissionArgs({ dangerouslySkipPermissions: true, targetIsRemote: true })).toEqual([
|
||||
"--allowedTools",
|
||||
SANDBOX_ALLOWED_TOOLS,
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not use Bash(*) because Claude Code treats Bash grants as command-prefix patterns", () => {
|
||||
const [, allowedTools] = buildClaudeExecutionPermissionArgs({
|
||||
dangerouslySkipPermissions: true,
|
||||
targetIsRemote: true,
|
||||
});
|
||||
|
||||
expect(allowedTools.split(" ")).toContain("Bash");
|
||||
expect(allowedTools).not.toContain("Bash(*)");
|
||||
});
|
||||
|
||||
it("does not pass permission flags when skip-permissions is disabled", () => {
|
||||
expect(buildClaudeExecutionPermissionArgs({ dangerouslySkipPermissions: false, targetIsRemote: true })).toEqual([]);
|
||||
expect(buildClaudeProbePermissionArgs({ dangerouslySkipPermissions: false, targetIsRemote: true })).toEqual([]);
|
||||
});
|
||||
|
||||
it("uses dangerously-skip-permissions for local execution", () => {
|
||||
expect(buildClaudeExecutionPermissionArgs({ dangerouslySkipPermissions: true, targetIsRemote: false })).toEqual([
|
||||
"--dangerously-skip-permissions",
|
||||
]);
|
||||
});
|
||||
|
||||
it("uses dangerously-skip-permissions for local probes", () => {
|
||||
expect(buildClaudeProbePermissionArgs({ dangerouslySkipPermissions: true, targetIsRemote: false })).toEqual([
|
||||
"--dangerously-skip-permissions",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,42 +1,42 @@
|
|||
// Explicit allowlist of Claude Code tools we permit when running inside a
|
||||
// sandbox. We use this instead of `--dangerously-skip-permissions` for sandbox
|
||||
// Explicit allowlist of Claude Code tools we permit when running on a remote
|
||||
// target. We use this instead of `--dangerously-skip-permissions` for remote
|
||||
// targets because the permission-approval prompts can't be answered by a
|
||||
// human inside a non-interactive sandbox, but blanket-allowing every tool
|
||||
// would defeat the point of having a separate sandbox code path.
|
||||
// human inside a non-interactive run, but blanket-allowing every tool would
|
||||
// defeat the point of having a separate hosted/sandbox code path.
|
||||
//
|
||||
// Maintenance: this list must be reviewed when Claude Code releases a new
|
||||
// tool. The canonical list of built-in tools is documented at
|
||||
// https://docs.claude.com/en/docs/claude-code/built-in-tools — when a tool
|
||||
// is added there, decide whether it should be allowed in sandbox runs and
|
||||
// is added there, decide whether it should be allowed in remote runs and
|
||||
// either add it here or document the deliberate exclusion. Omitting a tool
|
||||
// silently disables it inside sandboxes, which can look like the tool is
|
||||
// silently disables it inside remote targets, which can look like the tool is
|
||||
// "broken" rather than intentionally gated.
|
||||
const SANDBOX_ALLOWED_TOOLS =
|
||||
"Task AskUserQuestion Bash(*) CronCreate CronDelete CronList Edit " +
|
||||
"Task AskUserQuestion Bash CronCreate CronDelete CronList Edit " +
|
||||
"EnterPlanMode EnterWorktree ExitPlanMode ExitWorktree Glob Grep Monitor " +
|
||||
"NotebookEdit PushNotification Read RemoteTrigger ScheduleWakeup Skill " +
|
||||
"TaskOutput TaskStop TodoWrite ToolSearch WebFetch WebSearch Write";
|
||||
|
||||
export function buildClaudeProbePermissionArgs(input: {
|
||||
dangerouslySkipPermissions: boolean;
|
||||
targetIsSandbox: boolean;
|
||||
targetIsRemote: boolean;
|
||||
}): string[] {
|
||||
if (!input.dangerouslySkipPermissions) return [];
|
||||
// For sandbox targets, mirror the execution path: pass `--allowedTools`
|
||||
// For remote targets, mirror the execution path: pass `--allowedTools`
|
||||
// with the curated allowlist instead of dropping the flag entirely. The
|
||||
// hello probe is a one-shot prompt that should never trigger a tool, but
|
||||
// if a future probe prompt does, we don't want Claude CLI to stall on an
|
||||
// interactive permission prompt that no human can answer.
|
||||
if (input.targetIsSandbox) return ["--allowedTools", SANDBOX_ALLOWED_TOOLS];
|
||||
if (input.targetIsRemote) return ["--allowedTools", SANDBOX_ALLOWED_TOOLS];
|
||||
return ["--dangerously-skip-permissions"];
|
||||
}
|
||||
|
||||
export function buildClaudeExecutionPermissionArgs(input: {
|
||||
dangerouslySkipPermissions: boolean;
|
||||
targetIsSandbox: boolean;
|
||||
targetIsRemote: boolean;
|
||||
}): string[] {
|
||||
if (!input.dangerouslySkipPermissions) return [];
|
||||
if (input.targetIsSandbox) {
|
||||
if (input.targetIsRemote) {
|
||||
return ["--allowedTools", SANDBOX_ALLOWED_TOOLS];
|
||||
}
|
||||
return ["--dangerously-skip-permissions"];
|
||||
|
|
|
|||
|
|
@ -220,7 +220,7 @@ export async function testEnvironment(
|
|||
}
|
||||
|
||||
const args = ["--print", "-", "--output-format", "stream-json", "--verbose"];
|
||||
args.push(...buildClaudeProbePermissionArgs({ dangerouslySkipPermissions, targetIsSandbox }));
|
||||
args.push(...buildClaudeProbePermissionArgs({ dangerouslySkipPermissions, targetIsRemote }));
|
||||
if (chrome) args.push("--chrome");
|
||||
// For Bedrock: only pass --model when the ID is a Bedrock-native identifier.
|
||||
if (model && (!hasBedrock || isBedrockModelId(model))) {
|
||||
|
|
|
|||
|
|
@ -800,7 +800,7 @@ describe("claude execute", () => {
|
|||
const capture = JSON.parse(await fs.readFile(capturePath1, "utf8")) as CapturePayload;
|
||||
expect(capture.argv).toContain("--allowedTools");
|
||||
expect(capture.argv).toContain(
|
||||
"Task AskUserQuestion Bash(*) CronCreate CronDelete CronList Edit EnterPlanMode EnterWorktree ExitPlanMode ExitWorktree Glob Grep Monitor NotebookEdit PushNotification Read RemoteTrigger ScheduleWakeup Skill TaskOutput TaskStop TodoWrite ToolSearch WebFetch WebSearch Write",
|
||||
"Task AskUserQuestion Bash CronCreate CronDelete CronList Edit EnterPlanMode EnterWorktree ExitPlanMode ExitWorktree Glob Grep Monitor NotebookEdit PushNotification Read RemoteTrigger ScheduleWakeup Skill TaskOutput TaskStop TodoWrite ToolSearch WebFetch WebSearch Write",
|
||||
);
|
||||
expect(capture.argv).not.toContain("--dangerously-skip-permissions");
|
||||
expect(capture.claudeConfigDir).toBe(path.join(remoteWorkspace, ".paperclip-runtime", "claude", "config"));
|
||||
|
|
|
|||
Loading…
Reference in New Issue