mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-08-29 03:44:06 +08:00
Merge pull request #12414 from Kilo-Org/fix-sandbox-scandir-permission-error
fix(sandbox): tolerate unreadable directories during Linux writable-root scan
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@kilocode/cli": patch
|
||||
"kilo-code": patch
|
||||
---
|
||||
|
||||
Keep Linux sandbox setup working when a writable directory contains an unreadable subdirectory (for example a folder with mode 600); unreadable subdirectories are now protected with a read-only mount instead of failing every sandboxed tool call with an access error.
|
||||
@@ -98,6 +98,26 @@ function validate(allow: ReadonlyArray<PathRule>, executable: string, mounts: Re
|
||||
}
|
||||
}
|
||||
|
||||
function code(cause: unknown) {
|
||||
if (typeof cause !== "object" || cause === null || !("code" in cause)) return undefined
|
||||
const value = (cause as { code: unknown }).code
|
||||
return typeof value === "string" ? value : undefined
|
||||
}
|
||||
|
||||
// Lists one directory during the deny-name scan. A directory that vanished mid-scan
|
||||
// (ENOENT/ENOTDIR) is treated as empty, and an unreadable directory (EACCES/EPERM)
|
||||
// yields undefined so the caller can protect it instead of failing the whole scan.
|
||||
function list(dir: string) {
|
||||
try {
|
||||
return readdirSync(dir, { withFileTypes: true })
|
||||
} catch (cause) {
|
||||
const tag = code(cause)
|
||||
if (tag === "ENOENT" || tag === "ENOTDIR") return []
|
||||
if (tag === "EACCES" || tag === "EPERM") return undefined
|
||||
throw cause
|
||||
}
|
||||
}
|
||||
|
||||
function scan(root: string, names: ReadonlySet<string>, found: Set<string>) {
|
||||
if (names.has(path.basename(root))) {
|
||||
found.add(root)
|
||||
@@ -109,7 +129,16 @@ function scan(root: string, names: ReadonlySet<string>, found: Set<string>) {
|
||||
while (pending.length > 0) {
|
||||
const dir = pending.pop()
|
||||
if (!dir) continue
|
||||
for (const entry of readdirSync(dir, { withFileTypes: true })) {
|
||||
const entries = list(dir)
|
||||
if (!entries) {
|
||||
// Fail closed: a nested directory that cannot be enumerated might hide a deny-name
|
||||
// match, so it is re-bound read-only as a whole rather than aborting sandbox setup.
|
||||
// The writable root itself must stay readable, or there is nothing to scan.
|
||||
if (dir === root) throw new Error(`Writable root is not readable: ${root}`)
|
||||
found.add(dir)
|
||||
continue
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const target = path.join(dir, entry.name)
|
||||
if (names.has(entry.name)) {
|
||||
found.add(target)
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"
|
||||
import { chmodSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"
|
||||
import os from "node:os"
|
||||
import path from "node:path"
|
||||
import { Effect, PlatformError, Result } from "effect"
|
||||
@@ -34,6 +34,17 @@ const launch: Launch = {
|
||||
},
|
||||
}
|
||||
|
||||
// chmod-based permission tests only work when the test user is not root,
|
||||
// since root bypasses filesystem permission checks entirely.
|
||||
function readable(dir: string) {
|
||||
try {
|
||||
readdirSync(dir)
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
describe("sandbox launch preparation", () => {
|
||||
test("generates a globally overriding overlapping deny policy with parameterized paths", () => {
|
||||
const result = generate(makeProfile(), launch)
|
||||
@@ -126,6 +137,60 @@ describe("sandbox launch preparation", () => {
|
||||
}
|
||||
})
|
||||
|
||||
test("re-binds unreadable directories read-only instead of failing Linux setup", () => {
|
||||
const root = mkdtempSync(path.join(os.tmpdir(), "kilo-bubblewrap-unreadable-"))
|
||||
const git = path.join(root, ".git")
|
||||
const secrets = path.join(root, "secrets")
|
||||
mkdirSync(git)
|
||||
mkdirSync(secrets)
|
||||
chmodSync(secrets, 0o000)
|
||||
const profile: Profile = {
|
||||
...makeProfile("allow"),
|
||||
filesystem: {
|
||||
allowWrite: [{ path: root, kind: "subtree" }],
|
||||
denyWrite: [],
|
||||
denyNames: [".git"],
|
||||
},
|
||||
}
|
||||
|
||||
try {
|
||||
if (readable(secrets)) return
|
||||
const result = generateBubblewrap(profile, { ...launch, cwd: root }, "/opt/kilo/bwrap")
|
||||
const writable = result.args.indexOf("--bind")
|
||||
expect(result.args.slice(writable, writable + 3)).toEqual(["--bind", root, root])
|
||||
const first = result.args.indexOf("--ro-bind", writable + 3)
|
||||
expect(result.args.slice(first, first + 3)).toEqual(["--ro-bind", git, git])
|
||||
const second = result.args.indexOf("--ro-bind", first + 3)
|
||||
expect(result.args.slice(second, second + 3)).toEqual(["--ro-bind", secrets, secrets])
|
||||
} finally {
|
||||
chmodSync(secrets, 0o700)
|
||||
rmSync(root, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test("rejects an unreadable writable root", () => {
|
||||
const root = mkdtempSync(path.join(os.tmpdir(), "kilo-bubblewrap-root-"))
|
||||
chmodSync(root, 0o000)
|
||||
const profile: Profile = {
|
||||
...makeProfile("allow"),
|
||||
filesystem: {
|
||||
allowWrite: [{ path: root, kind: "subtree" }],
|
||||
denyWrite: [],
|
||||
denyNames: [".git"],
|
||||
},
|
||||
}
|
||||
|
||||
try {
|
||||
if (readable(root)) return
|
||||
expect(() => generateBubblewrap(profile, { ...launch, cwd: root }, "/opt/kilo/bwrap")).toThrow(
|
||||
`Writable root is not readable: ${root}`,
|
||||
)
|
||||
} finally {
|
||||
chmodSync(root, 0o700)
|
||||
rmSync(root, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test("parses escaped mount points from Linux mountinfo", () => {
|
||||
const content = [
|
||||
String.raw`36 25 0:32 / / rw,relatime - overlay overlay rw`,
|
||||
|
||||
Reference in New Issue
Block a user