Skip to content

Commit 220da28

Browse files
KJWhitesideKJWhiteside
committed
fix: implement secure default-deny pattern for doom_loop, edit, and bash permissions
CRITICAL SECURITY FIX: Additional permission bypass vulnerabilities found across the permission system. Permission checks were only checking for 'ask' or specific values, but ignoring 'deny' and undefined/null values, allowing operations to proceed when they should be blocked. Affected permissions and locations: - doom_loop in processor.ts - only checked 'ask', ignored deny - edit in write.ts - only checked 'ask', ignored deny - edit in edit.ts (2 locations) - only checked 'ask', ignored deny - edit in patch.ts - only checked 'ask', ignored deny - bash in bash.ts - only checked 'deny' and 'ask', but if wildcard returned undefined (no match), it allowed the command Changes: - Updated ALL permission checks to use secure default-deny pattern - Only explicitly allow operations when permission is 'allow' or 'ask' - Default to deny for 'deny', undefined, null, or any unexpected values - This ensures maximum security - any misconfiguration defaults to deny - Fixed bash.test.ts by mocking Permission.ask to auto-allow in tests Impact: - Prevents permission bypass attacks - Ensures deny settings are actually enforced - Protects against undefined/null permission values - Makes the system fail-secure instead of fail-open All 196 tests passing.
1 parent aeb2799 commit 220da28

File tree

6 files changed

+59
-13
lines changed

6 files changed

+59
-13
lines changed

‎packages/opencode/src/session/processor.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,10 @@ export namespace SessionProcessor {
147147
)
148148
) {
149149
const permission = await Agent.get(input.assistantMessage.mode).then((x) => x.permission)
150-
if (permission.doom_loop === "ask") {
150+
// Secure default: only allow if explicitly set to "allow" or "ask"
151+
if (permission.doom_loop === "allow") {
152+
// Explicitly allowed, proceed
153+
} else if (permission.doom_loop === "ask") {
151154
await Permission.ask({
152155
type: "doom_loop",
153156
pattern: value.toolName,
@@ -160,6 +163,11 @@ export namespace SessionProcessor {
160163
input: value.input,
161164
},
162165
})
166+
} else {
167+
// Default deny for "deny", undefined, null, or any other value
168+
throw new Error(
169+
`Permission denied: Possible doom loop detected - "${value.toolName}" called ${DOOM_LOOP_THRESHOLD} times with identical arguments`,
170+
)
163171
}
164172
}
165173
}

‎packages/opencode/src/tool/bash.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,10 @@ export const BashTool = Tool.define("bash", {
108108
// always allow cd if it passes above check
109109
if (command[0] !== "cd") {
110110
const action = Wildcard.allStructured({ head: command[0], tail: command.slice(1) }, permissions)
111-
if (action === "deny") {
112-
throw new Error(
113-
`The user has specifically restricted access to this command, you are not allowed to execute it. Here is the configuration: ${JSON.stringify(permissions)}`,
114-
)
115-
}
116-
if (action === "ask") {
111+
// Secure default: only allow if explicitly set to "allow" or "ask"
112+
if (action === "allow") {
113+
// Explicitly allowed, proceed
114+
} else if (action === "ask") {
117115
const pattern = (() => {
118116
if (command.length === 0) return
119117
const head = command[0]
@@ -124,6 +122,11 @@ export const BashTool = Tool.define("bash", {
124122
if (pattern) {
125123
askPatterns.add(pattern)
126124
}
125+
} else {
126+
// Default deny for "deny", undefined, null, or any other value
127+
throw new Error(
128+
`Permission denied: Command not allowed by bash permissions. Command: ${command[0]}. Configuration: ${JSON.stringify(permissions)}`,
129+
)
127130
}
128131
}
129132
}

‎packages/opencode/src/tool/edit.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ export const EditTool = Tool.define("edit", {
7373
if (params.oldString === "") {
7474
contentNew = params.newString
7575
diff = trimDiff(createTwoFilesPatch(filePath, filePath, contentOld, contentNew))
76-
if (agent.permission.edit === "ask") {
76+
// Secure default: only allow if explicitly set to "allow" or "ask"
77+
if (agent.permission.edit === "allow") {
78+
// Explicitly allowed, proceed
79+
} else if (agent.permission.edit === "ask") {
7780
await Permission.ask({
7881
type: "edit",
7982
sessionID: ctx.sessionID,
@@ -85,6 +88,9 @@ export const EditTool = Tool.define("edit", {
8588
diff,
8689
},
8790
})
91+
} else {
92+
// Default deny for "deny", undefined, null, or any other value
93+
throw new Error(`Permission denied: Cannot edit file: ${filePath}`)
8894
}
8995
await Bun.write(filePath, params.newString)
9096
await Bus.publish(File.Event.Edited, {
@@ -104,7 +110,10 @@ export const EditTool = Tool.define("edit", {
104110
diff = trimDiff(
105111
createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)),
106112
)
107-
if (agent.permission.edit === "ask") {
113+
// Secure default: only allow if explicitly set to "allow" or "ask"
114+
if (agent.permission.edit === "allow") {
115+
// Explicitly allowed, proceed
116+
} else if (agent.permission.edit === "ask") {
108117
await Permission.ask({
109118
type: "edit",
110119
sessionID: ctx.sessionID,
@@ -116,6 +125,9 @@ export const EditTool = Tool.define("edit", {
116125
diff,
117126
},
118127
})
128+
} else {
129+
// Default deny for "deny", undefined, null, or any other value
130+
throw new Error(`Permission denied: Cannot edit file: ${filePath}`)
119131
}
120132

121133
await file.write(contentNew)

‎packages/opencode/src/tool/patch.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,10 @@ export const PatchTool = Tool.define("patch", {
147147
}
148148

149149
// Check permissions if needed
150-
if (agent.permission.edit === "ask") {
150+
// Secure default: only allow if explicitly set to "allow" or "ask"
151+
if (agent.permission.edit === "allow") {
152+
// Explicitly allowed, proceed
153+
} else if (agent.permission.edit === "ask") {
151154
await Permission.ask({
152155
type: "edit",
153156
sessionID: ctx.sessionID,
@@ -158,6 +161,9 @@ export const PatchTool = Tool.define("patch", {
158161
diff: totalDiff,
159162
},
160163
})
164+
} else {
165+
// Default deny for "deny", undefined, null, or any other value
166+
throw new Error(`Permission denied: Cannot apply patch to ${fileChanges.length} files`)
161167
}
162168

163169
// Apply the changes

‎packages/opencode/src/tool/write.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ export const WriteTool = Tool.define("write", {
4949
const exists = await file.exists()
5050
if (exists) await FileTime.assert(ctx.sessionID, filepath)
5151

52-
if (agent.permission.edit === "ask")
52+
// Secure default: only allow if explicitly set to "allow" or "ask"
53+
if (agent.permission.edit === "allow") {
54+
// Explicitly allowed, proceed
55+
} else if (agent.permission.edit === "ask") {
5356
await Permission.ask({
5457
type: "write",
5558
sessionID: ctx.sessionID,
@@ -62,6 +65,10 @@ export const WriteTool = Tool.define("write", {
6265
exists,
6366
},
6467
})
68+
} else {
69+
// Default deny for "deny", undefined, null, or any other value
70+
throw new Error(`Permission denied: Cannot write file: ${filepath}`)
71+
}
6572

6673
await Bun.write(filepath, params.content)
6774
await Bus.publish(File.Event.Edited, {

‎packages/opencode/test/tool/bash.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
1-
import { describe, expect, test } from "bun:test"
1+
import { describe, expect, test, mock } from "bun:test"
22
import path from "path"
33
import { BashTool } from "../../src/tool/bash"
44
import { Instance } from "../../src/project/instance"
5+
import { Permission } from "../../src/permission"
6+
import { Log } from "../../src/util/log"
7+
8+
Log.init({ print: false })
9+
10+
// Mock Permission.ask to auto-allow in tests
11+
Permission.ask = mock(async () => {
12+
return
13+
})
514

615
const ctx = {
716
sessionID: "test",
@@ -12,14 +21,14 @@ const ctx = {
1221
metadata: () => {},
1322
}
1423

15-
const bash = await BashTool.init()
1624
const projectRoot = path.join(__dirname, "../..")
1725

1826
describe("tool.bash", () => {
1927
test("basic", async () => {
2028
await Instance.provide({
2129
directory: projectRoot,
2230
fn: async () => {
31+
const bash = await BashTool.init()
2332
const result = await bash.execute(
2433
{
2534
command: "echo 'test'",
@@ -37,6 +46,7 @@ describe("tool.bash", () => {
3746
await Instance.provide({
3847
directory: projectRoot,
3948
fn: async () => {
49+
const bash = await BashTool.init()
4050
expect(
4151
bash.execute(
4252
{

0 commit comments

Comments
 (0)