Skip to content

Commit a2de105

Browse files
committed
harness: address PR review (per-dataDir cache, archive edit deny)
resolveHarnessDir cached extraction with a single module-level promise keyed by nothing, so a second call with a different dataDir returned the first call's path. In production opencode passes a singleton Global.Path.data so this never bit, but tests and any future multi-instance scenario would silently get cross-dataDir contamination. Switch to a Map<dataDir, Promise<path>> — same dataDir still deduplicates, distinct dataDirs each get their own extraction. harness-archive/ was whitelisted in external_directory:allow, which let edit/write/apply_patch silently mutate snapshots that are intended to be read-only history. Keep the dir-level whitelist (so reads stay silent — the agent is supposed to browse the archive when migrating helpers across upgrades), but add an edit:deny rule keyed on '*/harness-archive/*'. The leading * absorbs the worktree-relative prefix that edit/write/apply_patch produce; the dir name is the anchor. All three edit-class tools route through permission='edit' so one rule covers them. Bash-level mutations (rm -rf) are still possible, but the agent has no prompt-driven path to them and the user can deny bash explicitly via config if desired.
1 parent 95cbc63 commit a2de105

2 files changed

Lines changed: 26 additions & 5 deletions

File tree

packages/bcode-browser/src/harness.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,19 @@ const extractEmbeddedHarness = async (dataDir: string): Promise<string> => {
133133
return target
134134
}
135135

136-
let extractPromise: Promise<string> | null = null
136+
// Per-dataDir cache. In production opencode passes the same Global.Path.data
137+
// every call, so this is effectively a singleton; tests and any future
138+
// multi-instance setup that resolves against multiple dataDirs each get their
139+
// own deduplicated extraction without cross-directory contamination.
140+
const extractCache = new Map<string, Promise<string>>()
137141

138142
export const resolveHarnessDir = (dataDir: string): Promise<string> => {
139143
if (!isCompiled) return Promise.resolve(DEV_HARNESS_DIR)
140-
if (!extractPromise) extractPromise = extractEmbeddedHarness(dataDir)
141-
return extractPromise
144+
const cached = extractCache.get(dataDir)
145+
if (cached) return cached
146+
const fresh = extractEmbeddedHarness(dataDir)
147+
extractCache.set(dataDir, fresh)
148+
return fresh
142149
}
143150

144151
export * as Harness from "./harness"

packages/opencode/src/agent/agent.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,20 @@ export const layer = Layer.effect(
9494
// In dev mode the harness lives inside the worktree, so this glob is a
9595
// no-op there.
9696
const harnessGlob = path.join(Harness.harnessDir(Global.Path.data), "*")
97-
// Past-version snapshots taken at upgrade time. Read-only browsing for
98-
// the agent when migrating its own helpers across upgrades.
97+
// Past-version snapshots taken at upgrade time. Read-only history for
98+
// the agent when migrating its own helpers across upgrades — silent
99+
// reads via the external_directory whitelist, but edits/writes/
100+
// apply_patch are denied below to keep snapshots immutable. Bash-level
101+
// mutations are still possible but the agent has no prompt-driven
102+
// reason to delete the dir.
99103
const harnessArchiveGlob = path.join(Harness.harnessArchiveDir(Global.Path.data), "*")
104+
// edit/write/apply_patch all `ctx.ask({ permission: "edit", ... })`
105+
// with a path that's `path.relative(worktree, filepath)` — which for
106+
// an out-of-worktree archive file looks like
107+
// `../../.local/share/bcode/harness-archive/<hash>/foo.py`. A leading
108+
// `*` (greedy `.*`) absorbs that prefix; the dir name itself is the
109+
// anchor.
110+
const harnessArchiveEditDeny = "*/harness-archive/*"
100111
const whitelistedDirs = [
101112
Truncate.GLOB,
102113
browserSessionsGlob,
@@ -113,6 +124,9 @@ export const layer = Layer.effect(
113124
"*": "ask",
114125
...Object.fromEntries(whitelistedDirs.map((dir) => [dir, "allow"])),
115126
},
127+
// Covers `edit`, `write`, `apply_patch` — all three tools route
128+
// through the `edit` permission key (see EDIT_TOOLS in permission/).
129+
edit: { [harnessArchiveEditDeny]: "deny" },
116130
question: "deny",
117131
plan_enter: "deny",
118132
plan_exit: "deny",

0 commit comments

Comments
 (0)