Skip to content

Commit 65eee77

Browse files
fengjikuifengjikui
authored
fix(web): enforce permissions for spawned sessions (#46)
* fix(web): enforce permissions for spawned sessions * test: bound spawn repeat concurrency
1 parent 599b165 commit 65eee77

4 files changed

Lines changed: 190 additions & 38 deletions

File tree

‎src/web/server/handlers/sessions.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { manager } from '../../../plugin/pty/manager.ts'
2+
import { checkCommandPermission, checkWorkdirPermission } from '../../../plugin/pty/permissions.ts'
23
import type { BunRequest } from 'bun'
34
import { JsonResponse, ErrorResponse } from './responses.ts'
45
import type { routes } from '../../shared/routes.ts'
@@ -28,9 +29,15 @@ export async function createSession(req: Request) {
2829
}
2930

3031
try {
32+
const args = body.args || []
33+
await checkCommandPermission(body.command, args)
34+
if (body.workdir) {
35+
await checkWorkdirPermission(body.workdir)
36+
}
37+
3138
const session = manager.spawn({
3239
command: body.command,
33-
args: body.args || [],
40+
args,
3441
title: body.description,
3542
description: body.description,
3643
workdir: body.workdir,

‎src/web/server/handlers/websocket.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ServerWebSocket } from 'bun'
22
import { manager } from '../../../plugin/pty/manager'
3+
import { checkCommandPermission, checkWorkdirPermission } from '../../../plugin/pty/permissions'
34
import {
45
type WSMessageServerSessionList,
56
type WSMessageClientSubscribeSession,
@@ -101,7 +102,7 @@ class WebSocketHandler {
101102
break
102103

103104
case 'spawn':
104-
this.handleSpawn(ws, message as WSMessageClientSpawnSession)
105+
void this.handleSpawn(ws, message as WSMessageClientSpawnSession)
105106
break
106107

107108
case 'input':
@@ -124,10 +125,23 @@ class WebSocketHandler {
124125
}
125126
}
126127

127-
private handleSpawn(ws: ServerWebSocket<undefined>, message: WSMessageClientSpawnSession) {
128-
const sessionInfo = manager.spawn(message)
129-
if (message.subscribe) {
130-
this.handleSubscribe(ws, { type: 'subscribe', sessionId: sessionInfo.id })
128+
private async handleSpawn(ws: ServerWebSocket<undefined>, message: WSMessageClientSpawnSession) {
129+
try {
130+
await checkCommandPermission(message.command, message.args ?? [])
131+
if (message.workdir) {
132+
await checkWorkdirPermission(message.workdir)
133+
}
134+
135+
const sessionInfo = manager.spawn(message)
136+
if (message.subscribe) {
137+
this.handleSubscribe(ws, { type: 'subscribe', sessionId: sessionInfo.id })
138+
}
139+
} catch (err) {
140+
const error: WSMessageServerError = {
141+
type: 'error',
142+
error: new CustomError(err instanceof Error ? err.message : Bun.inspect(err)),
143+
}
144+
ws.send(JSON.stringify(error))
131145
}
132146
}
133147

‎test/spawn-repeat.test.ts‎

Lines changed: 55 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ import {
88
} from '../src/plugin/pty/manager.ts'
99
import type { Subprocess } from 'bun'
1010

11+
function positiveIntFromEnv(name: string, fallback: number): number {
12+
const value = process.env[name]
13+
if (value === undefined) return fallback
14+
15+
const parsed = Number.parseInt(value, 10)
16+
return Number.isInteger(parsed) && parsed > 0 ? parsed : fallback
17+
}
18+
1119
describe('PTY Echo Behavior', () => {
1220
beforeEach(() => {
1321
initManager(new OpencodeClient())
@@ -72,41 +80,56 @@ describe('PTY Echo Behavior', () => {
7280
}
7381

7482
it('should receive initial data reproducibly', async () => {
75-
const start = Date.now()
76-
const maxRuntime = 1000
77-
let runnings = 1
78-
const spawned: TestSpawner[] = []
79-
while (Date.now() - start < maxRuntime) {
80-
runnings++
81-
const testSpawner = new TestSpawner(runnings)
82-
spawned.push(testSpawner)
83-
}
84-
let errorMessage = ''
85-
errorMessage += `[TEST] Spawned ${runnings} subprocesses in ${Date.now() - start}ms.\n`
86-
const timeout = new Promise<void>((resolve) => {
87-
setTimeout(() => resolve(), 20000)
88-
})
89-
const all = Promise.all(spawned.map((s) => s.subprocess.exited))
90-
await Promise.race([all, timeout])
91-
const stillRunning = spawned.filter((s) => s.subprocess.exitCode === null)
92-
if (stillRunning.length > 0) {
93-
errorMessage += `[TEST] Timeout reached after 20s with ${stillRunning.length} subprocesses still running.\n`
94-
stillRunning.forEach((s) => {
95-
errorMessage += `[TEST] Subprocess ${s.testNumber} stderr: ${s.stderrOutput}\n`
96-
errorMessage += `[TEST] Subprocess ${s.testNumber} stdout: ${s.stdoutOutput}\n`
97-
})
98-
}
99-
const exitCodeNonZero = spawned.filter(
100-
(s) => s.subprocess.exitCode !== null && s.subprocess.exitCode !== 0
83+
const repeatRuns = positiveIntFromEnv('SPAWN_REPEAT_RUNS', 64)
84+
const repeatConcurrency = Math.min(
85+
positiveIntFromEnv('SPAWN_REPEAT_CONCURRENCY', 8),
86+
repeatRuns
10187
)
102-
if (exitCodeNonZero.length > 0) {
103-
errorMessage += `[TEST] ${exitCodeNonZero.length} subprocesses exited with non-zero exit code.\n`
104-
exitCodeNonZero.forEach((s) => {
105-
errorMessage += `[TEST] Subprocess ${s.testNumber} stderr: ${s.stderrOutput}\n`
106-
errorMessage += `[TEST] Subprocess ${s.testNumber} stdout: ${s.stdoutOutput}\n`
88+
let errorMessage = ''
89+
let completedRuns = 0
90+
let failureCount = 0
91+
92+
while (completedRuns < repeatRuns) {
93+
const batchSize = Math.min(repeatConcurrency, repeatRuns - completedRuns)
94+
const spawned = Array.from(
95+
{ length: batchSize },
96+
(_, index) => new TestSpawner(completedRuns + index + 1)
97+
)
98+
99+
const timeout = new Promise<void>((resolve) => {
100+
setTimeout(() => resolve(), 20000)
107101
})
102+
const all = Promise.all(spawned.map((s) => s.subprocess.exited))
103+
await Promise.race([all, timeout])
104+
105+
const stillRunning = spawned.filter((s) => s.subprocess.exitCode === null)
106+
if (stillRunning.length > 0) {
107+
failureCount += stillRunning.length
108+
errorMessage += `[TEST] Timeout reached after 20s with ${stillRunning.length} subprocesses still running.\n`
109+
stillRunning.forEach((s) => {
110+
errorMessage += `[TEST] Subprocess ${s.testNumber} stderr: ${s.stderrOutput}\n`
111+
errorMessage += `[TEST] Subprocess ${s.testNumber} stdout: ${s.stdoutOutput}\n`
112+
s.subprocess.kill()
113+
})
114+
}
115+
116+
const exitCodeNonZero = spawned.filter(
117+
(s) => s.subprocess.exitCode !== null && s.subprocess.exitCode !== 0
118+
)
119+
if (exitCodeNonZero.length > 0) {
120+
failureCount += exitCodeNonZero.length
121+
errorMessage += `[TEST] ${exitCodeNonZero.length} subprocesses exited with non-zero exit code.\n`
122+
exitCodeNonZero.forEach((s) => {
123+
errorMessage += `[TEST] Subprocess ${s.testNumber} stderr: ${s.stderrOutput}\n`
124+
errorMessage += `[TEST] Subprocess ${s.testNumber} stdout: ${s.stdoutOutput}\n`
125+
})
126+
}
127+
128+
completedRuns += batchSize
108129
}
109-
expect(stillRunning.length + exitCodeNonZero.length, errorMessage).toBe(0)
130+
131+
errorMessage = `[TEST] Spawned ${completedRuns} subprocesses with concurrency ${repeatConcurrency}.\n${errorMessage}`
132+
expect(failureCount, errorMessage).toBe(0)
110133
}, 60000)
111134

112135
it.skipIf(!process.env.SYNC_TESTS)(

‎test/web-permissions.test.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { afterEach, describe, expect, it, mock, spyOn } from 'bun:test'
2+
import type { ServerWebSocket } from 'bun'
3+
import { manager } from '../src/plugin/pty/manager.ts'
4+
import { initPermissions } from '../src/plugin/pty/permissions.ts'
5+
import type { PTYSessionInfo } from '../src/plugin/pty/types.ts'
6+
import { createSession } from '../src/web/server/handlers/sessions.ts'
7+
import { handleWebSocketMessage } from '../src/web/server/handlers/websocket.ts'
8+
import type { WSMessageServerError } from '../src/web/shared/types.ts'
9+
10+
type TestPermissionConfig = {
11+
bash?: 'allow' | 'ask' | 'deny' | Record<string, 'allow' | 'ask' | 'deny'>
12+
external_directory?: 'allow' | 'ask' | 'deny'
13+
}
14+
15+
function initTestPermissions(permission: TestPermissionConfig = {}) {
16+
initPermissions(
17+
{
18+
config: {
19+
get: async () => ({ data: { permission } }),
20+
},
21+
tui: {
22+
showToast: async () => {},
23+
},
24+
} as unknown as Parameters<typeof initPermissions>[0],
25+
process.cwd()
26+
)
27+
}
28+
29+
function buildSpawnInfo(overrides: Partial<PTYSessionInfo> = {}): PTYSessionInfo {
30+
return {
31+
id: 'pty_denied_test',
32+
title: 'Denied test',
33+
command: 'echo',
34+
args: ['blocked'],
35+
workdir: process.cwd(),
36+
status: 'running',
37+
notifyOnExit: false,
38+
timedOut: false,
39+
pid: 1234,
40+
createdAt: new Date().toISOString(),
41+
lineCount: 0,
42+
...overrides,
43+
}
44+
}
45+
46+
function createFakeWebSocket(sentMessages: string[]): ServerWebSocket<undefined> {
47+
return {
48+
send: (message: string) => {
49+
sentMessages.push(message)
50+
return 0
51+
},
52+
subscribe: () => {},
53+
unsubscribe: () => {},
54+
subscriptions: new Set<string>(),
55+
} as unknown as ServerWebSocket<undefined>
56+
}
57+
58+
describe('web permission enforcement', () => {
59+
afterEach(() => {
60+
initTestPermissions()
61+
mock.restore()
62+
})
63+
64+
it('rejects REST session creation when bash permissions deny commands', async () => {
65+
initTestPermissions({ bash: 'deny' })
66+
const spawnSpy = spyOn(manager, 'spawn').mockReturnValue(buildSpawnInfo())
67+
68+
const response = await createSession(
69+
new Request('http://localhost/api/sessions', {
70+
method: 'POST',
71+
headers: { 'Content-Type': 'application/json' },
72+
body: JSON.stringify({
73+
command: 'echo',
74+
args: ['blocked'],
75+
description: 'Denied REST session',
76+
}),
77+
})
78+
)
79+
80+
expect(response.status).toBe(400)
81+
expect(await response.text()).toContain('All bash commands are disabled')
82+
expect(spawnSpy).not.toHaveBeenCalled()
83+
})
84+
85+
it('rejects WebSocket session creation when bash permissions deny commands', async () => {
86+
initTestPermissions({ bash: 'deny' })
87+
const spawnSpy = spyOn(manager, 'spawn').mockReturnValue(buildSpawnInfo())
88+
const sentMessages: string[] = []
89+
90+
handleWebSocketMessage(
91+
createFakeWebSocket(sentMessages),
92+
JSON.stringify({
93+
type: 'spawn',
94+
command: 'echo',
95+
args: ['blocked'],
96+
description: 'Denied WebSocket session',
97+
parentSessionId: 'websocket-test',
98+
})
99+
)
100+
await new Promise(setImmediate)
101+
102+
expect(spawnSpy).not.toHaveBeenCalled()
103+
expect(sentMessages).toHaveLength(1)
104+
const message = JSON.parse(sentMessages[0] ?? '{}') as WSMessageServerError
105+
expect(message.type).toBe('error')
106+
expect(message.error.message).toContain('All bash commands are disabled')
107+
})
108+
})

0 commit comments

Comments
 (0)