chore: cleanup external_dir perm logic (#11845)
This commit is contained in:
@@ -55,7 +55,6 @@ export namespace Agent {
|
|||||||
doom_loop: "ask",
|
doom_loop: "ask",
|
||||||
external_directory: {
|
external_directory: {
|
||||||
"*": "ask",
|
"*": "ask",
|
||||||
[Truncate.DIR]: "allow",
|
|
||||||
[Truncate.GLOB]: "allow",
|
[Truncate.GLOB]: "allow",
|
||||||
},
|
},
|
||||||
question: "deny",
|
question: "deny",
|
||||||
@@ -140,7 +139,6 @@ export namespace Agent {
|
|||||||
codesearch: "allow",
|
codesearch: "allow",
|
||||||
read: "allow",
|
read: "allow",
|
||||||
external_directory: {
|
external_directory: {
|
||||||
[Truncate.DIR]: "allow",
|
|
||||||
[Truncate.GLOB]: "allow",
|
[Truncate.GLOB]: "allow",
|
||||||
},
|
},
|
||||||
}),
|
}),
|
||||||
@@ -229,19 +227,19 @@ export namespace Agent {
|
|||||||
item.permission = PermissionNext.merge(item.permission, PermissionNext.fromConfig(value.permission ?? {}))
|
item.permission = PermissionNext.merge(item.permission, PermissionNext.fromConfig(value.permission ?? {}))
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ensure Truncate.DIR is allowed unless explicitly configured
|
// Ensure Truncate.GLOB is allowed unless explicitly configured
|
||||||
for (const name in result) {
|
for (const name in result) {
|
||||||
const agent = result[name]
|
const agent = result[name]
|
||||||
const explicit = agent.permission.some((r) => {
|
const explicit = agent.permission.some((r) => {
|
||||||
if (r.permission !== "external_directory") return false
|
if (r.permission !== "external_directory") return false
|
||||||
if (r.action !== "deny") return false
|
if (r.action !== "deny") return false
|
||||||
return r.pattern === Truncate.DIR || r.pattern === Truncate.GLOB
|
return r.pattern === Truncate.GLOB
|
||||||
})
|
})
|
||||||
if (explicit) continue
|
if (explicit) continue
|
||||||
|
|
||||||
result[name].permission = PermissionNext.merge(
|
result[name].permission = PermissionNext.merge(
|
||||||
result[name].permission,
|
result[name].permission,
|
||||||
PermissionNext.fromConfig({ external_directory: { [Truncate.DIR]: "allow", [Truncate.GLOB]: "allow" } }),
|
PermissionNext.fromConfig({ external_directory: { [Truncate.GLOB]: "allow" } }),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -128,7 +128,10 @@ export const BashTool = Tool.define("bash", async () => {
|
|||||||
process.platform === "win32" && resolved.match(/^\/[a-z]\//)
|
process.platform === "win32" && resolved.match(/^\/[a-z]\//)
|
||||||
? resolved.replace(/^\/([a-z])\//, (_, drive) => `${drive.toUpperCase()}:\\`).replace(/\//g, "\\")
|
? resolved.replace(/^\/([a-z])\//, (_, drive) => `${drive.toUpperCase()}:\\`).replace(/\//g, "\\")
|
||||||
: resolved
|
: resolved
|
||||||
if (!Instance.containsPath(normalized)) directories.add(normalized)
|
if (!Instance.containsPath(normalized)) {
|
||||||
|
const dir = (await Filesystem.isDir(normalized)) ? normalized : path.dirname(normalized)
|
||||||
|
directories.add(dir)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -141,10 +144,11 @@ export const BashTool = Tool.define("bash", async () => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (directories.size > 0) {
|
if (directories.size > 0) {
|
||||||
|
const globs = Array.from(directories).map((dir) => path.join(dir, "*"))
|
||||||
await ctx.ask({
|
await ctx.ask({
|
||||||
permission: "external_directory",
|
permission: "external_directory",
|
||||||
patterns: Array.from(directories),
|
patterns: globs,
|
||||||
always: Array.from(directories).map((x) => path.dirname(x) + "*"),
|
always: globs,
|
||||||
metadata: {},
|
metadata: {},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -447,7 +447,7 @@ test("legacy tools config maps write/edit/patch/multiedit to edit permission", a
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
test("Truncate.DIR is allowed even when user denies external_directory globally", async () => {
|
test("Truncate.GLOB is allowed even when user denies external_directory globally", async () => {
|
||||||
const { Truncate } = await import("../../src/tool/truncation")
|
const { Truncate } = await import("../../src/tool/truncation")
|
||||||
await using tmp = await tmpdir({
|
await using tmp = await tmpdir({
|
||||||
config: {
|
config: {
|
||||||
@@ -460,14 +460,14 @@ test("Truncate.DIR is allowed even when user denies external_directory globally"
|
|||||||
directory: tmp.path,
|
directory: tmp.path,
|
||||||
fn: async () => {
|
fn: async () => {
|
||||||
const build = await Agent.get("build")
|
const build = await Agent.get("build")
|
||||||
expect(PermissionNext.evaluate("external_directory", Truncate.DIR, build!.permission).action).toBe("allow")
|
|
||||||
expect(PermissionNext.evaluate("external_directory", Truncate.GLOB, build!.permission).action).toBe("allow")
|
expect(PermissionNext.evaluate("external_directory", Truncate.GLOB, build!.permission).action).toBe("allow")
|
||||||
|
expect(PermissionNext.evaluate("external_directory", Truncate.DIR, build!.permission).action).toBe("deny")
|
||||||
expect(PermissionNext.evaluate("external_directory", "/some/other/path", build!.permission).action).toBe("deny")
|
expect(PermissionNext.evaluate("external_directory", "/some/other/path", build!.permission).action).toBe("deny")
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
test("Truncate.DIR is allowed even when user denies external_directory per-agent", async () => {
|
test("Truncate.GLOB is allowed even when user denies external_directory per-agent", async () => {
|
||||||
const { Truncate } = await import("../../src/tool/truncation")
|
const { Truncate } = await import("../../src/tool/truncation")
|
||||||
await using tmp = await tmpdir({
|
await using tmp = await tmpdir({
|
||||||
config: {
|
config: {
|
||||||
@@ -484,21 +484,21 @@ test("Truncate.DIR is allowed even when user denies external_directory per-agent
|
|||||||
directory: tmp.path,
|
directory: tmp.path,
|
||||||
fn: async () => {
|
fn: async () => {
|
||||||
const build = await Agent.get("build")
|
const build = await Agent.get("build")
|
||||||
expect(PermissionNext.evaluate("external_directory", Truncate.DIR, build!.permission).action).toBe("allow")
|
|
||||||
expect(PermissionNext.evaluate("external_directory", Truncate.GLOB, build!.permission).action).toBe("allow")
|
expect(PermissionNext.evaluate("external_directory", Truncate.GLOB, build!.permission).action).toBe("allow")
|
||||||
|
expect(PermissionNext.evaluate("external_directory", Truncate.DIR, build!.permission).action).toBe("deny")
|
||||||
expect(PermissionNext.evaluate("external_directory", "/some/other/path", build!.permission).action).toBe("deny")
|
expect(PermissionNext.evaluate("external_directory", "/some/other/path", build!.permission).action).toBe("deny")
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
test("explicit Truncate.DIR deny is respected", async () => {
|
test("explicit Truncate.GLOB deny is respected", async () => {
|
||||||
const { Truncate } = await import("../../src/tool/truncation")
|
const { Truncate } = await import("../../src/tool/truncation")
|
||||||
await using tmp = await tmpdir({
|
await using tmp = await tmpdir({
|
||||||
config: {
|
config: {
|
||||||
permission: {
|
permission: {
|
||||||
external_directory: {
|
external_directory: {
|
||||||
"*": "deny",
|
"*": "deny",
|
||||||
[Truncate.DIR]: "deny",
|
[Truncate.GLOB]: "deny",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -507,8 +507,8 @@ test("explicit Truncate.DIR deny is respected", async () => {
|
|||||||
directory: tmp.path,
|
directory: tmp.path,
|
||||||
fn: async () => {
|
fn: async () => {
|
||||||
const build = await Agent.get("build")
|
const build = await Agent.get("build")
|
||||||
expect(PermissionNext.evaluate("external_directory", Truncate.DIR, build!.permission).action).toBe("deny")
|
|
||||||
expect(PermissionNext.evaluate("external_directory", Truncate.GLOB, build!.permission).action).toBe("deny")
|
expect(PermissionNext.evaluate("external_directory", Truncate.GLOB, build!.permission).action).toBe("deny")
|
||||||
|
expect(PermissionNext.evaluate("external_directory", Truncate.DIR, build!.permission).action).toBe("deny")
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -144,7 +144,42 @@ describe("tool.bash permissions", () => {
|
|||||||
)
|
)
|
||||||
const extDirReq = requests.find((r) => r.permission === "external_directory")
|
const extDirReq = requests.find((r) => r.permission === "external_directory")
|
||||||
expect(extDirReq).toBeDefined()
|
expect(extDirReq).toBeDefined()
|
||||||
expect(extDirReq!.patterns).toContain("/tmp")
|
expect(extDirReq!.patterns).toContain("/tmp/*")
|
||||||
|
},
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test("asks for external_directory permission when file arg is outside project", async () => {
|
||||||
|
await using outerTmp = await tmpdir({
|
||||||
|
init: async (dir) => {
|
||||||
|
await Bun.write(path.join(dir, "outside.txt"), "x")
|
||||||
|
},
|
||||||
|
})
|
||||||
|
await using tmp = await tmpdir({ git: true })
|
||||||
|
await Instance.provide({
|
||||||
|
directory: tmp.path,
|
||||||
|
fn: async () => {
|
||||||
|
const bash = await BashTool.init()
|
||||||
|
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
|
||||||
|
const testCtx = {
|
||||||
|
...ctx,
|
||||||
|
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
|
||||||
|
requests.push(req)
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const filepath = path.join(outerTmp.path, "outside.txt")
|
||||||
|
await bash.execute(
|
||||||
|
{
|
||||||
|
command: `cat ${filepath}`,
|
||||||
|
description: "Read external file",
|
||||||
|
},
|
||||||
|
testCtx,
|
||||||
|
)
|
||||||
|
const extDirReq = requests.find((r) => r.permission === "external_directory")
|
||||||
|
const expected = path.join(outerTmp.path, "*")
|
||||||
|
expect(extDirReq).toBeDefined()
|
||||||
|
expect(extDirReq!.patterns).toContain(expected)
|
||||||
|
expect(extDirReq!.always).toContain(expected)
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user