Skip to content

Commit ea55459

Browse files
anandgupta42claude
anandgupta42
and
claude
committed
fix(permissions): match normalized paths for read and bash deny rules
- Evaluate read and bash requests against the authored spelling plus normalized absolute aliases; deny/ask rules match any spelling, allow rules match only the authored spelling so normalization can only tighten a decision - Normalize relative and absolute read paths, resolve literal quoted operands, `cd` sequences and relative `workdir` for bash, and include `ls` - Add symlink-resolved targets as deny-only aliases for read - Treat background `&` lists as non-sequential for directory tracking - Legacy permission ask now checks all denies before prompting - Add regression tests for read, bash and permission evaluation Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 346f0f8 commit ea55459

8 files changed

Lines changed: 662 additions & 45 deletions

File tree

‎packages/opencode/src/permission/evaluate.ts‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,60 @@
1-
export { evaluate } from "."
1+
// altimate_change start — upstream_fix: match equivalent path spellings with one rule precedence
2+
import type { PermissionV1 } from "@opencode-ai/core/v1/permission"
3+
import { Wildcard } from "@opencode-ai/core/util/wildcard"
4+
5+
// `pattern` is the spelling the caller authored; `aliases` are equivalent normalized
6+
// spellings. Only deny/ask rules consider aliases, so normalization can tighten a
7+
// decision but never lets an allow rule apply to something it did not match before.
8+
function evaluatePatterns(
9+
permission: string,
10+
pattern: string,
11+
aliases: string[],
12+
...rulesets: PermissionV1.Ruleset[]
13+
): PermissionV1.Rule {
14+
return (
15+
rulesets
16+
.flat()
17+
.findLast(
18+
(rule) =>
19+
Wildcard.match(permission, rule.permission) &&
20+
(Wildcard.match(pattern, rule.pattern) ||
21+
(rule.action !== "allow" && aliases.some((alias) => Wildcard.match(alias, rule.pattern)))),
22+
) ?? {
23+
action: "ask",
24+
permission,
25+
pattern: "*",
26+
}
27+
)
28+
}
29+
30+
export function evaluate(permission: string, pattern: string, ...rulesets: PermissionV1.Ruleset[]): PermissionV1.Rule {
31+
return evaluatePatterns(permission, pattern, [], ...rulesets)
32+
}
33+
34+
export function evaluateRequest(
35+
request: Pick<PermissionV1.Request, "permission" | "metadata">,
36+
pattern: string,
37+
...rulesets: PermissionV1.Ruleset[]
38+
): PermissionV1.Rule {
39+
const aliases = request.metadata.permissionAliases
40+
const values = aliases && typeof aliases === "object" ? (aliases as Record<string, unknown>)[pattern] : undefined
41+
const groups = Array.isArray(values)
42+
? values.filter((group): group is string[] => Array.isArray(group) && group.every((value) => typeof value === "string"))
43+
: []
44+
if (!groups.length) return evaluate(request.permission, pattern, ...rulesets)
45+
// Preserve rule order within each access. Distinct invocations must all be allowed,
46+
// even when they normalize to the same command.
47+
const rules = groups.map((group) => evaluatePatterns(request.permission, pattern, group, ...rulesets))
48+
return rules.find((rule) => rule.action === "deny") ?? rules.find((rule) => rule.action === "ask") ?? rules[0]!
49+
}
50+
51+
export function evaluateRequestPatterns(
52+
request: Pick<PermissionV1.Request, "permission" | "metadata" | "patterns">,
53+
...rulesets: PermissionV1.Ruleset[]
54+
) {
55+
// Legacy callers return at the first prompt, so reject all denied accesses first.
56+
return request.patterns
57+
.map((pattern) => ({ pattern, rule: evaluateRequest(request, pattern, ...rulesets) }))
58+
.sort((a, b) => Number(b.rule.action === "deny") - Number(a.rule.action === "deny"))
59+
}
60+
// altimate_change end

‎packages/opencode/src/permission/index.ts‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import os from "os"
77
import { PermissionV1 } from "@opencode-ai/core/v1/permission"
88
import { EventV2Bridge } from "@/event-v2-bridge"
99
import { EventV2 } from "@opencode-ai/core/event"
10+
// altimate_change start — upstream_fix: evaluate normalized aliases without changing rule order
11+
import { evaluateRequest } from "./evaluate"
12+
// altimate_change end
1013
// altimate_change start — upstream_fix: load persisted permission approvals
1114
import { Database, eq } from "@/storage/db"
1215
import { PermissionTable } from "./permission.sql"
@@ -40,17 +43,9 @@ interface State {
4043
approved: PermissionV1.Rule[]
4144
}
4245

43-
export function evaluate(permission: string, pattern: string, ...rulesets: PermissionV1.Ruleset[]): PermissionV1.Rule {
44-
return (
45-
rulesets
46-
.flat()
47-
.findLast((rule) => Wildcard.match(permission, rule.permission) && Wildcard.match(pattern, rule.pattern)) ?? {
48-
action: "ask",
49-
permission,
50-
pattern: "*",
51-
}
52-
)
53-
}
46+
// altimate_change start — upstream_fix: share path-alias evaluation with legacy permissions
47+
export { evaluate } from "./evaluate"
48+
// altimate_change end
5449

5550
export class Service extends Context.Service<Service, Interface>()("@opencode/Permission") {}
5651

@@ -91,7 +86,9 @@ export const layer = Layer.effect(
9186
let needsAsk = false
9287

9388
for (const pattern of request.patterns) {
94-
const rule = evaluate(request.permission, pattern, ruleset, approved)
89+
// altimate_change start — upstream_fix: include normalized aliases in permission matching
90+
const rule = evaluateRequest(request, pattern, ruleset, approved)
91+
// altimate_change end
9592
yield* Effect.logInfo("evaluated", { permission: request.permission, pattern, action: rule })
9693
if (rule.action === "deny") {
9794
return yield* new PermissionV1.DeniedError({
@@ -174,7 +171,9 @@ export const layer = Layer.effect(
174171
for (const [id, item] of pending.entries()) {
175172
if (item.info.sessionID !== existing.info.sessionID) continue
176173
const ok = item.info.patterns.every(
177-
(pattern) => evaluate(item.info.permission, pattern, approved).action === "allow",
174+
// altimate_change start — upstream_fix: apply approvals to equivalent path spellings
175+
(pattern) => evaluateRequest(item.info, pattern, approved).action === "allow",
176+
// altimate_change end
178177
)
179178
if (!ok) continue
180179
pending.delete(id)

‎packages/opencode/src/permission/next.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import { ProjectID } from "@/project/schema"
1313
import { Wildcard } from "@/util/wildcard"
1414
import os from "os"
1515
import z from "zod"
16+
// altimate_change start — upstream_fix: share normalized path matching with Effect permissions
17+
import { evaluateRequest, evaluateRequestPatterns } from "./evaluate"
18+
// altimate_change end
1619

1720
export namespace PermissionNext {
1821
const log = Log.create({ service: "permission" })
@@ -135,8 +138,9 @@ export namespace PermissionNext {
135138
async (input) => {
136139
const s = await state()
137140
const { ruleset, ...request } = input
138-
for (const pattern of request.patterns ?? []) {
139-
const rule = evaluate(request.permission, pattern, ruleset, s.approved)
141+
// altimate_change start — upstream_fix: check normalized denies before any approval prompt
142+
for (const { pattern, rule } of evaluateRequestPatterns(request, ruleset, s.approved)) {
143+
// altimate_change end
140144
log.info("evaluated", { permission: request.permission, pattern, action: rule })
141145
if (rule.action === "deny") {
142146
Telemetry.track({
@@ -229,7 +233,9 @@ export namespace PermissionNext {
229233
for (const [id, pending] of s.pending) {
230234
if (pending.info.sessionID !== sessionID) continue
231235
const ok = pending.info.patterns.every(
232-
(pattern) => evaluate(pending.info.permission, pattern, s.approved).action === "allow",
236+
// altimate_change start — upstream_fix: apply approvals to equivalent path spellings
237+
(pattern) => evaluateRequest(pending.info, pattern, s.approved).action === "allow",
238+
// altimate_change end
233239
)
234240
if (!ok) continue
235241
s.pending.delete(id)

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

Lines changed: 97 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,53 @@ import { BashArity } from "@/permission/arity"
1818
import { Truncate } from "./truncation"
1919
import { Plugin } from "@/plugin"
2020
import { Global } from "@/global"
21+
// altimate_change start — upstream_fix: resolve literal shell paths for permission matching
22+
import type { Node } from "web-tree-sitter"
23+
24+
function shellWords(node: Node) {
25+
return node.namedChildren.filter(
26+
(child): child is Node =>
27+
child !== null && ["command_name", "word", "string", "raw_string", "concatenation"].includes(child.type),
28+
)
29+
}
30+
31+
function literalWord(node: Node | null): string | undefined {
32+
if (!node) return
33+
if (node.type === "raw_string") return node.text.slice(1, -1)
34+
if (node.type === "word") {
35+
if (/[~$*?{\[]/.test(node.text)) return
36+
return node.text.replace(/\\(.)/gs, "$1")
37+
}
38+
if (node.type === "string" && node.namedChildren.every((child) => child?.type === "string_content")) {
39+
return node.text.slice(1, -1).replace(/\\\n/g, "").replace(/\\([$`"\\])/g, "$1")
40+
}
41+
if (node.type === "command_name" || node.type === "concatenation") {
42+
const parts = node.namedChildren.map(literalWord)
43+
if (parts.every((part) => part !== undefined)) return parts.join("")
44+
}
45+
}
46+
47+
// Track literal directory changes in straight-line sequences. Analyze branches
48+
// independently and keep subshell, substitution, and pipeline changes local.
49+
function* shellCommands(node: Node | null, cwd: string): Generator<{ node: Node; cwd: string }, string> {
50+
if (!node) return cwd
51+
if (node.type === "command") {
52+
yield { node, cwd }
53+
for (const child of node.namedChildren) yield* shellCommands(child, cwd)
54+
const [name, arg] = shellWords(node).map(literalWord)
55+
return name === "cd" && arg && !arg.startsWith("-") ? path.resolve(cwd, arg) : cwd
56+
}
57+
const sequential =
58+
["program", "list", "subshell", "command_substitution", "compound_statement", "redirected_statement"].includes(node.type) &&
59+
!node.children.some((child) => child?.type === "||" || child?.type === "&")
60+
let next = cwd
61+
for (const child of node.namedChildren) {
62+
const result = yield* shellCommands(child, sequential ? next : cwd)
63+
if (sequential) next = result
64+
}
65+
return ["program", "list", "compound_statement", "redirected_statement"].includes(node.type) ? next : cwd
66+
}
67+
// altimate_change end
2168

2269
// altimate_change start — run-mode markers must not reach bash child processes.
2370
// `run` sets ALTIMATE_RUN_MODE on its own process to arm run-mode-only
@@ -132,7 +179,9 @@ export const BashTool = Tool.define("bash", async () => {
132179
),
133180
}),
134181
async execute(params, ctx) {
135-
const cwd = params.workdir || Instance.directory
182+
// altimate_change start — upstream_fix: resolve workdir relative to the active instance
183+
const cwd = path.resolve(Instance.directory, params.workdir || ".")
184+
// altimate_change end
136185
if (params.timeout !== undefined && params.timeout < 0) {
137186
throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`)
138187
}
@@ -145,34 +194,41 @@ export const BashTool = Tool.define("bash", async () => {
145194
if (!Instance.containsPath(cwd)) directories.add(cwd)
146195
const patterns = new Set<string>()
147196
const always = new Set<string>()
197+
// altimate_change start — upstream_fix: retain raw command rules alongside absolute path rules
198+
const permissionAliases = new Map<string, Set<string>>()
148199

149-
for (const node of tree.rootNode.descendantsOfType("command")) {
200+
for (const { node, cwd: commandCwd } of shellCommands(tree.rootNode, cwd)) {
201+
// altimate_change end
150202
if (!node) continue
151203

152204
// Get full command text including redirects if present
153205
let commandText = node.parent?.type === "redirected_statement" ? node.parent.text : node.text
154206

155-
const command = []
156-
for (let i = 0; i < node.childCount; i++) {
157-
const child = node.child(i)
158-
if (!child) continue
159-
if (
160-
child.type !== "command_name" &&
161-
child.type !== "word" &&
162-
child.type !== "string" &&
163-
child.type !== "raw_string" &&
164-
child.type !== "concatenation"
165-
) {
166-
continue
167-
}
168-
command.push(child.text)
169-
}
207+
// altimate_change start — upstream_fix: normalize static path operands, including directory listings
208+
const words = shellWords(node)
209+
const command = words.map((word) => word.text)
210+
const name = words[0] && literalWord(words[0])
211+
const source = node.parent?.type === "redirected_statement" ? node.parent : node
212+
const replacements: { start: number; end: number; text: string }[] = []
170213

171214
// not an exhaustive list, but covers most common cases
172-
if (["cd", "rm", "cp", "mv", "mkdir", "touch", "chmod", "chown", "cat"].includes(command[0])) {
173-
for (const arg of command.slice(1)) {
174-
if (arg.startsWith("-") || (command[0] === "chmod" && arg.startsWith("+"))) continue
175-
const resolved = await fs.realpath(path.resolve(cwd, arg)).catch(() => "")
215+
if (name && ["cd", "rm", "cp", "mv", "mkdir", "touch", "chmod", "chown", "cat", "ls"].includes(name)) {
216+
const operands = words.slice(1).filter((word) => !word.text.startsWith("-"))
217+
const targets = operands.flatMap((word) => {
218+
const arg = literalWord(word)
219+
if (arg === undefined || (name === "chmod" && arg.startsWith("+"))) return []
220+
const target = path.resolve(commandCwd, arg)
221+
if (!["chmod", "chown"].includes(name) || word !== operands[0]) {
222+
replacements.push({ start: word.startIndex, end: word.endIndex, text: target })
223+
}
224+
return [target]
225+
})
226+
if (name === "ls" && operands.length === 0) {
227+
targets.push(commandCwd)
228+
replacements.push({ start: node.endIndex, end: node.endIndex, text: ` ${commandCwd}` })
229+
}
230+
for (const arg of targets) {
231+
const resolved = await fs.realpath(arg).catch(() => "")
176232
log.info("resolved path", { arg, resolved })
177233
if (resolved) {
178234
const normalized =
@@ -184,10 +240,22 @@ export const BashTool = Tool.define("bash", async () => {
184240
}
185241
}
186242
}
243+
const original = commandText
244+
for (const replacement of replacements.reverse()) {
245+
const start = replacement.start - source.startIndex
246+
const end = replacement.end - source.startIndex
247+
commandText = commandText.slice(0, start) + replacement.text + commandText.slice(end)
248+
}
249+
const aliases = permissionAliases.get(original) ?? new Set<string>()
250+
aliases.add(commandText)
251+
permissionAliases.set(original, aliases)
252+
// altimate_change end
187253

188254
// cd covered by above check
189255
if (command.length && command[0] !== "cd") {
190-
patterns.add(commandText)
256+
// altimate_change start — upstream_fix: rules match the authored command text
257+
patterns.add(original)
258+
// altimate_change end
191259
always.add(BashArity.prefix(command).join(" ") + " *")
192260
}
193261
}
@@ -211,7 +279,13 @@ export const BashTool = Tool.define("bash", async () => {
211279
permission: "bash",
212280
patterns: Array.from(patterns),
213281
always: Array.from(always),
214-
metadata: {},
282+
// altimate_change start — upstream_fix: evaluate raw and normalized commands as equivalent spellings
283+
metadata: {
284+
permissionAliases: Object.fromEntries(
285+
Array.from(permissionAliases, ([key, values]) => [key, [...values].map((value) => [value])]),
286+
),
287+
},
288+
// altimate_change end
215289
})
216290
}
217291

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { isPdfAttachment, sniffAttachmentMime } from "@/util/media"
1212
// altimate_change start — upstream_fix: restore stale-file guard (dropped in v1.17.9 merge)
1313
import { FileTime } from "../file/time"
1414
import { Instance } from "../project/instance"
15+
import { realpath } from "fs/promises"
1516
// altimate_change end
1617

1718
const DEFAULT_READ_LIMIT = 2000
@@ -235,10 +236,9 @@ export const ReadTool = Tool.define<
235236
ctx: Tool.Context<Metadata>,
236237
) {
237238
const instance = yield* InstanceState.context
238-
let filepath = params.filePath
239-
if (!path.isAbsolute(filepath)) {
240-
filepath = path.resolve(instance.directory, filepath)
241-
}
239+
// altimate_change start — upstream_fix: normalize read paths before permission evaluation
240+
let filepath = path.resolve(instance.directory, params.filePath)
241+
// altimate_change end
242242
if (process.platform === "win32") {
243243
filepath = FSUtil.normalizePath(filepath)
244244
}
@@ -256,11 +256,18 @@ export const ReadTool = Tool.define<
256256
kind: stat?.type === "Directory" ? "directory" : "file",
257257
})
258258

259+
// altimate_change start — upstream_fix: match absolute and symlink-resolved deny rules; allow rules keep the authored spelling
260+
const resolved = yield* Effect.promise(() => realpath(filepath).catch(() => filepath))
259261
yield* ctx.ask({
260262
permission: "read",
261263
patterns: [path.relative(instance.worktree, filepath)],
262264
always: ["*"],
263-
metadata: {},
265+
metadata: {
266+
permissionAliases: {
267+
[path.relative(instance.worktree, filepath)]: [Array.from(new Set([filepath, resolved]))],
268+
},
269+
},
270+
// altimate_change end
264271
})
265272

266273
if (!stat) return yield* miss(filepath)

0 commit comments

Comments
 (0)