Skip to content

Commit f0fa213

Browse files
rekram1-noderekram1-node
committed
fix: permission checks not enforcing deny settings
1 parent 008ccb4 commit f0fa213

File tree

11 files changed

+708
-29
lines changed

11 files changed

+708
-29
lines changed

‎.opencode/opencode.jsonc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,7 @@
88
},
99
},
1010
},
11+
"permission": {
12+
"external_directory": "ask"
13+
}
1114
}

‎packages/opencode/src/agent/agent.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,8 @@ function mergeAgentPermissions(basePermission: any, overridePermission: any): Ag
250250
edit: merged.edit ?? "allow",
251251
webfetch: merged.webfetch ?? "allow",
252252
bash: mergedBash ?? { "*": "allow" },
253-
doom_loop: merged.doom_loop,
254-
external_directory: merged.external_directory,
253+
doom_loop: merged.doom_loop ?? "ask",
254+
external_directory: merged.external_directory ?? "ask",
255255
}
256256

257257
return result

‎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: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ export const EditTool = Tool.define("edit", {
4444
const filePath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath)
4545
if (!Filesystem.contains(Instance.directory, filePath)) {
4646
const parentDir = path.dirname(filePath)
47-
if (agent.permission.external_directory === "ask") {
47+
// Secure default: only allow if explicitly set to "allow" or "ask"
48+
if (agent.permission.external_directory === "allow") {
49+
// Explicitly allowed, proceed
50+
} else if (agent.permission.external_directory === "ask") {
4851
await Permission.ask({
4952
type: "external_directory",
5053
pattern: parentDir,
@@ -57,6 +60,9 @@ export const EditTool = Tool.define("edit", {
5760
parentDir,
5861
},
5962
})
63+
} else {
64+
// Default deny for "deny", undefined, null, or any other value
65+
throw new Error(`Permission denied: Cannot edit file outside working directory: ${filePath}`)
6066
}
6167
}
6268

@@ -67,7 +73,10 @@ export const EditTool = Tool.define("edit", {
6773
if (params.oldString === "") {
6874
contentNew = params.newString
6975
diff = trimDiff(createTwoFilesPatch(filePath, filePath, contentOld, contentNew))
70-
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") {
7180
await Permission.ask({
7281
type: "edit",
7382
sessionID: ctx.sessionID,
@@ -79,6 +88,9 @@ export const EditTool = Tool.define("edit", {
7988
diff,
8089
},
8190
})
91+
} else {
92+
// Default deny for "deny", undefined, null, or any other value
93+
throw new Error(`Permission denied: Cannot edit file: ${filePath}`)
8294
}
8395
await Bun.write(filePath, params.newString)
8496
await Bus.publish(File.Event.Edited, {
@@ -98,7 +110,10 @@ export const EditTool = Tool.define("edit", {
98110
diff = trimDiff(
99111
createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)),
100112
)
101-
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") {
102117
await Permission.ask({
103118
type: "edit",
104119
sessionID: ctx.sessionID,
@@ -110,6 +125,9 @@ export const EditTool = Tool.define("edit", {
110125
diff,
111126
},
112127
})
128+
} else {
129+
// Default deny for "deny", undefined, null, or any other value
130+
throw new Error(`Permission denied: Cannot edit file: ${filePath}`)
113131
}
114132

115133
await file.write(contentNew)

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,10 @@ export const PatchTool = Tool.define("patch", {
5555

5656
if (!Filesystem.contains(Instance.directory, filePath)) {
5757
const parentDir = path.dirname(filePath)
58-
if (agent.permission.external_directory === "ask") {
58+
// Secure default: only allow if explicitly set to "allow" or "ask"
59+
if (agent.permission.external_directory === "allow") {
60+
// Explicitly allowed, proceed
61+
} else if (agent.permission.external_directory === "ask") {
5962
await Permission.ask({
6063
type: "external_directory",
6164
pattern: parentDir,
@@ -68,6 +71,9 @@ export const PatchTool = Tool.define("patch", {
6871
parentDir,
6972
},
7073
})
74+
} else {
75+
// Default deny for "deny", undefined, null, or any other value
76+
throw new Error(`Permission denied: Cannot patch file outside working directory: ${filePath}`)
7177
}
7278
}
7379

@@ -141,7 +147,10 @@ export const PatchTool = Tool.define("patch", {
141147
}
142148

143149
// Check permissions if needed
144-
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") {
145154
await Permission.ask({
146155
type: "edit",
147156
sessionID: ctx.sessionID,
@@ -152,6 +161,9 @@ export const PatchTool = Tool.define("patch", {
152161
diff: totalDiff,
153162
},
154163
})
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`)
155167
}
156168

157169
// Apply the changes

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ export const ReadTool = Tool.define("read", {
3333

3434
if (!ctx.extra?.["bypassCwdCheck"] && !Filesystem.contains(Instance.directory, filepath)) {
3535
const parentDir = path.dirname(filepath)
36-
if (agent.permission.external_directory === "ask") {
36+
// Secure default: only allow if explicitly set to "allow" or "ask"
37+
if (agent.permission.external_directory === "allow") {
38+
// Explicitly allowed, proceed
39+
} else if (agent.permission.external_directory === "ask") {
3740
await Permission.ask({
3841
type: "external_directory",
3942
pattern: parentDir,
@@ -46,6 +49,9 @@ export const ReadTool = Tool.define("read", {
4649
parentDir,
4750
},
4851
})
52+
} else {
53+
// Default deny for "deny", undefined, null, or any other value
54+
throw new Error(`Permission denied: Cannot access file outside working directory: ${filepath}`)
4955
}
5056
}
5157

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ export const WriteTool = Tool.define("write", {
2323
const filepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath)
2424
if (!Filesystem.contains(Instance.directory, filepath)) {
2525
const parentDir = path.dirname(filepath)
26-
if (agent.permission.external_directory === "ask") {
26+
// Secure default: only allow if explicitly set to "allow" or "ask"
27+
if (agent.permission.external_directory === "allow") {
28+
// Explicitly allowed, proceed
29+
} else if (agent.permission.external_directory === "ask") {
2730
await Permission.ask({
2831
type: "external_directory",
2932
pattern: parentDir,
@@ -36,14 +39,20 @@ export const WriteTool = Tool.define("write", {
3639
parentDir,
3740
},
3841
})
42+
} else {
43+
// Default deny for "deny", undefined, null, or any other value
44+
throw new Error(`Permission denied: Cannot write file outside working directory: ${filepath}`)
3945
}
4046
}
4147

4248
const file = Bun.file(filepath)
4349
const exists = await file.exists()
4450
if (exists) await FileTime.assert(ctx.sessionID, filepath)
4551

46-
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") {
4756
await Permission.ask({
4857
type: "write",
4958
sessionID: ctx.sessionID,
@@ -56,6 +65,10 @@ export const WriteTool = Tool.define("write", {
5665
exists,
5766
},
5867
})
68+
} else {
69+
// Default deny for "deny", undefined, null, or any other value
70+
throw new Error(`Permission denied: Cannot write file: ${filepath}`)
71+
}
5972

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

0 commit comments

Comments
 (0)