revert(sandbox): withdraw the credential-document read denial
The `readDenyPaths` policy field shipped in the previous commit broke Linux confinement outright. bwrap has to create the `/dev/null` bind's mount point inside a tree its own profile has already made read-only, so it refused the entire confinement whenever the parent directory was absent — every host that has not stored a credential yet, including a fresh install: bwrap: Can't mkdir parents for /home/runner/.dsh/.env: Read-only file system which the executor correctly classifies as SANDBOX_UNAVAILABLE, so every confined bash call failed closed. Landlock cannot subtract from its own `/` read grant, so it reported `partial` enforcement on every confined call for a file it never hid, with no way to switch the denial off (schemastery fills an omitted array with `[]`, so empty and omitted were indistinguishable). A protection that breaks confinement where it works and misreports it where it does not is worse than a documented absence. Revert the field, both expressible backends, the enforcement downgrade, and the policy default; state the residue plainly in the credentials-local READMEs — file mode stops other OS users, not the model — and keep the OS-keychain provider recorded as the real answer. The narrower discipline stands: no surface hoists the credential document into `process.env`, and the model is never handed a resolved path to it.
This commit is contained in:
@@ -228,12 +228,7 @@ export class LocalSandboxProvider extends SandboxProvider {
|
||||
const selected = this.selectRunner(policy.mode)
|
||||
return {
|
||||
argv: [...this.runnerArgv(selected.runner, policy), '--', ...argv],
|
||||
// Landlock grants are a pure allow-list, so it cannot subtract a read
|
||||
// denial from its own `/` read grant: promising `full` there would
|
||||
// misreport a boundary the process does not have.
|
||||
enforcement: selected.runner === 'landlock' && (policy.readDenyPaths?.length ?? 0) > 0
|
||||
? 'partial'
|
||||
: selected.enforcement,
|
||||
enforcement: selected.enforcement,
|
||||
denialSignatures: DENIAL_SIGNATURES[selected.runner],
|
||||
runnerFailureSignatures: RUNNER_FAILURE_SIGNATURES[selected.runner],
|
||||
}
|
||||
|
||||
@@ -5,14 +5,9 @@
|
||||
*/
|
||||
|
||||
import { grantArgs as landlockGrantArgs } from 'node-addon-landlock-run'
|
||||
import { canonicalPath, writableRoots } from '@deepseek-ai/dsh-sandbox'
|
||||
import { writableRoots } from '@deepseek-ai/dsh-sandbox'
|
||||
import type { SandboxPolicy } from '@deepseek-ai/dsh-sandbox'
|
||||
|
||||
/** This policy's read denials, canonical and deduplicated like the writable roots. */
|
||||
function denyPaths(policy: SandboxPolicy): string[] {
|
||||
return [...new Set((policy.readDenyPaths ?? []).map(path => canonicalPath(path)))]
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the bwrap profile arguments for one file-effect policy.
|
||||
* @param policy - file-effect policy to express as bwrap mounts.
|
||||
@@ -24,10 +19,6 @@ export function bwrapProfileArgs(policy: SandboxPolicy): string[] {
|
||||
args.push('--tmpfs', '/tmp')
|
||||
args.push('--bind', policy.workspaceRoot, policy.workspaceRoot)
|
||||
}
|
||||
// Read denials come last so a workspace bind can never re-expose one.
|
||||
// `/dev/null` over the path reads as empty; the `-try` form tolerates a
|
||||
// path that does not exist yet (no credential stored so far).
|
||||
for (const path of denyPaths(policy)) args.push('--ro-bind-try', '/dev/null', path)
|
||||
return args
|
||||
}
|
||||
|
||||
@@ -37,10 +28,6 @@ export function bwrapProfileArgs(policy: SandboxPolicy): string[] {
|
||||
* @returns launcher grant arguments before the trailing separator and command argv.
|
||||
*/
|
||||
export function landlockProfileArgs(policy: SandboxPolicy): string[] {
|
||||
// Landlock grants are a pure allow-list: a read grant on `/` cannot be
|
||||
// subtracted from, so a requested read denial is unenforceable here. The
|
||||
// provider reports `partial` enforcement for exactly this case rather than
|
||||
// pretending the boundary exists.
|
||||
const readWrite = ['/dev/null']
|
||||
if (policy.mode === 'workspace-write') {
|
||||
readWrite.push('/tmp', policy.workspaceRoot)
|
||||
@@ -67,13 +54,5 @@ export function seatbeltProfileArgs(policy: SandboxPolicy): string[] {
|
||||
if (roots.length > 0) {
|
||||
forms.push(`(allow file-write* ${roots.map(root => `(subpath ${sbplString(root)})`).join(' ')})`)
|
||||
}
|
||||
// SBPL applies the last matching rule, so the read denial is appended after
|
||||
// every allow above and governs both reads and writes of those paths. Both
|
||||
// filters are emitted so a denial may name a file or a directory.
|
||||
const denied = denyPaths(policy)
|
||||
if (denied.length > 0) {
|
||||
const filters = denied.map(path => `(literal ${sbplString(path)}) (subpath ${sbplString(path)})`).join(' ')
|
||||
forms.push(`(deny file-read* file-write* ${filters})`)
|
||||
}
|
||||
return ['-p', forms.join(' ')]
|
||||
}
|
||||
|
||||
@@ -62,27 +62,6 @@ describe('profile dialects', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('bwrap read denial: /dev/null over each denied path, after any workspace bind', () => {
|
||||
expect(bwrapProfileArgs({ ...WW, readDenyPaths: ['/ws/secret.env'] })).toEqual([
|
||||
'--ro-bind', '/', '/', '--dev', '/dev', '--proc', '/proc', '--die-with-parent',
|
||||
'--tmpfs', '/tmp', '--bind', '/ws', '/ws',
|
||||
// The workspace bind above would otherwise re-expose the file.
|
||||
'--ro-bind-try', '/dev/null', '/ws/secret.env',
|
||||
])
|
||||
})
|
||||
|
||||
it('landlock ignores read denials: a `/` read grant cannot subtract from itself', () => {
|
||||
expect(landlockProfileArgs({ ...RO, readDenyPaths: ['/ws/secret.env'] }))
|
||||
.toEqual(landlockProfileArgs(RO))
|
||||
})
|
||||
|
||||
it('seatbelt read denial: a trailing deny naming the path as both a file and a directory', () => {
|
||||
expect(seatbeltProfileArgs({ ...RO, readDenyPaths: ['/ws/secret.env'] })).toEqual([
|
||||
'-p',
|
||||
`${SEATBELT_RO_PROFILE} (deny file-read* file-write* (literal "/ws/secret.env") (subpath "/ws/secret.env"))`,
|
||||
])
|
||||
})
|
||||
|
||||
it('landlock read-only: readable tree plus a writable /dev/null, nothing else', () => {
|
||||
// /dev/null specifically, NOT /dev: a whole-/dev grant would let confined
|
||||
// commands write real host paths beneath it (/dev/shm) under read-only.
|
||||
@@ -329,15 +308,6 @@ describe('the default landlock probe (launcher CLI contract)', () => {
|
||||
expect(sandbox.confine(['true'], RO).enforcement).toBe('partial')
|
||||
})
|
||||
|
||||
it('reports partial enforcement when a read denial is requested it cannot express', async () => {
|
||||
const launcher = fakeLauncher()
|
||||
const { sandbox } = await setup({}, { platform: 'linux', probeBwrap: () => false, landlockLauncher: launcher })
|
||||
// Fully enforced for the write policy, yet the read denial is
|
||||
// unexpressible in an allow-list that already grants `/` for reads.
|
||||
expect(sandbox.confine(['true'], RO).enforcement).toBe('full')
|
||||
expect(sandbox.confine(['true'], { ...RO, readDenyPaths: ['/ws/secret.env'] }).enforcement).toBe('partial')
|
||||
})
|
||||
|
||||
it('reads a failing launcher as unusable: the chain ends and fails closed', async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'dsh-fake-landlock-'))
|
||||
const launcher = join(dir, 'landlock-run')
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { spawnSync } from 'node:child_process'
|
||||
import { existsSync, readFileSync } from 'node:fs'
|
||||
import { mkdtemp, rm, writeFile } from 'node:fs/promises'
|
||||
import { mkdtemp, rm } from 'node:fs/promises'
|
||||
import { homedir, tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
@@ -70,37 +70,6 @@ describe.skipIf(!seatbeltUsable)('sandbox-local: real Seatbelt confinement throu
|
||||
expect(result.stdout).toBe('dev-ok\n')
|
||||
})
|
||||
|
||||
it('denies reading a credential document the mode would otherwise allow', async () => {
|
||||
// The harness's own secret store: readable to the user, and the model's
|
||||
// bash runs as that user — only the confinement can take it away.
|
||||
const workdir = await tempDir(tmpdir())
|
||||
const secret = join(workdir, '.env')
|
||||
await writeFile(secret, 'DEEPSEEK_API_KEY=sk-must-not-leak\n', { mode: 0o600 })
|
||||
const sandbox = await provider()
|
||||
|
||||
const allowed = runConfined(sandbox, `cat ${secret}`, { mode: 'read-only', workspaceRoot: workdir })
|
||||
expect(allowed.result.stdout).toContain('sk-must-not-leak')
|
||||
|
||||
const denied = runConfined(sandbox, `cat ${secret}`, {
|
||||
mode: 'read-only',
|
||||
workspaceRoot: workdir,
|
||||
readDenyPaths: [secret],
|
||||
})
|
||||
expect(denied.result.stdout).not.toContain('sk-must-not-leak')
|
||||
expect(denied.result.status).not.toBe(0)
|
||||
expect(denied.confined.enforcement).toBe('full')
|
||||
// Everything else under the same directory stays readable: the denial is
|
||||
// the credential document, not the harness home.
|
||||
const sibling = join(workdir, 'notes.txt')
|
||||
await writeFile(sibling, 'ordinary\n')
|
||||
const neighbour = runConfined(sandbox, `cat ${sibling}`, {
|
||||
mode: 'read-only',
|
||||
workspaceRoot: workdir,
|
||||
readDenyPaths: [secret],
|
||||
})
|
||||
expect(neighbour.result.stdout).toBe('ordinary\n')
|
||||
})
|
||||
|
||||
it('read-only grants no temp area: a write under the user temp dir is denied too', async () => {
|
||||
// The per-user darwin temp dir is a workspace-write grant, not a
|
||||
// read-only one — under read-only the only write-shaped path is /dev/null.
|
||||
|
||||
Reference in New Issue
Block a user