From 8c617301f71b42e133f9e5fa44451d63e651dfc6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 4 Oct 2026 03:15:12 -0700 Subject: [PATCH] fix(opencode): keep overlay manifest cleanup inside owned directories (#24763) * fix: wait for OpenCode worker composer before first dispatch Reuse captured composer readiness on local and paired execution hosts and revoke launching-shell paste anchors. Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> * feat(opencode): probe execution-host CLI capabilities * fix(opencode): select plugin default for execution host loader * fix(opencode): limit prompt prefill capability to verified release * feat(opencode): probe launch capabilities on the execution host * fix(opencode): select plugin loader for the launched host binary * fix(opencode): match WSL probe cwd and declared guest environment * fix(opencode): preserve launch environment deletion boundaries * wip(opencode): authorize native startup prompt intent at execution owner * fix(opencode): atomically replace status plugin entrypoints * fix(opencode): retain plugin permissions across restrictive umasks * test(opencode): resolve permission fixture from primary cwd * feat(opencode): install startup prompt plugin independently of status hooks * fix(opencode): wait for admitted startup intent and preserve failed-launch briefs * fix(opencode): confine overlay manifest cleanup to owned directories Co-authored-by: Adnan Khan * fix: wait for OpenCode worker composer before first dispatch Reuse captured composer readiness on local and paired execution hosts and revoke launching-shell paste anchors. Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> * feat(opencode): probe execution-host CLI capabilities * fix(opencode): select plugin default for execution host loader * fix(opencode): limit prompt prefill capability to verified release * feat(opencode): probe launch capabilities on the execution host * fix(opencode): select plugin loader for the launched host binary * fix(opencode): match WSL probe cwd and declared guest environment * fix(opencode): preserve launch environment deletion boundaries * wip(opencode): authorize native startup prompt intent at execution owner * fix(opencode): atomically replace status plugin entrypoints * fix(opencode): retain plugin permissions across restrictive umasks * test(opencode): resolve permission fixture from primary cwd * feat(opencode): install startup prompt plugin independently of status hooks * fix(opencode): wait for admitted startup intent and preserve failed-launch briefs * fix(opencode): unsubscribe hook settings during async host shutdown * STRICT launch CI contract correction * CAPS launch CI contract correction * INTENT launch CI contract correction * test: initialize Claude prompt state in output retention fixture * Wait for OpenCode location hydration in intent startup * Bind OpenCode startup readiness to the current location in intent startup --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Co-authored-by: Ahmed Nagy Co-authored-by: Adnan Khan Co-authored-by: Orca startup hydration review --- .../hook-service-overlay-confinement.test.ts | 98 +++++++++++++++++++ src/main/opencode/hook-service.ts | 14 ++- .../opencode-overlay-manifest.test.ts | 32 ++++++ src/main/pty/overlay-mirror.test.ts | 29 +++++- src/main/pty/overlay-mirror.ts | 35 ++++++- 5 files changed, 202 insertions(+), 6 deletions(-) create mode 100644 src/main/opencode/hook-service-overlay-confinement.test.ts create mode 100644 src/main/opencode/opencode-overlay-manifest.test.ts diff --git a/src/main/opencode/hook-service-overlay-confinement.test.ts b/src/main/opencode/hook-service-overlay-confinement.test.ts new file mode 100644 index 00000000000..46a5bee5884 --- /dev/null +++ b/src/main/opencode/hook-service-overlay-confinement.test.ts @@ -0,0 +1,98 @@ +import { + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + symlinkSync, + writeFileSync +} from 'node:fs' +import { tmpdir } from 'node:os' +import { basename, dirname, join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { setAppEnvironment } from '../../shared/app-environment' +import { safeRemoveTree } from '../pty/overlay-mirror' +import { OpenCodeHookService } from './hook-service' +import { OPENCODE_OVERLAY_MANIFEST_FILE } from './opencode-overlay-manifest' + +let root: string +let source: string +let service: OpenCodeHookService + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-overlay-confinement-')) + source = join(root, 'source') + mkdirSync(join(source, 'plugins'), { recursive: true }) + writeFileSync(join(source, 'plugins', 'user.js'), 'export default {}') + vi.stubEnv('XDG_CONFIG_HOME', join(root, 'xdg')) + setAppEnvironment({ + getPath: () => join(root, 'profile'), + getAppPath: () => process.cwd(), + getVersion: () => '0.0.0-test', + isPackaged: () => false, + onWillQuit: () => {}, + exit: () => {}, + getAppMetrics: () => [] + }) + service = new OpenCodeHookService({ + pluginFileName: 'orca-test.js', + legacyHooksDir: 'legacy-test', + overlayDir: 'overlay-test', + pluginSource: () => 'export default {}' + }) +}) + +afterEach(() => { + vi.unstubAllEnvs() + rmSync(root, { recursive: true, force: true }) +}) + +it.each(['overlay-root', 'source-overlay', 'plugins'] as const)( + 'preserves outside files when the owned %s is replaced by a directory link', + (boundary) => { + const overlay = service.buildPtyEnv('pane-1', source).OPENCODE_CONFIG_DIR + if (!overlay) { + throw new Error('Expected an isolated overlay') + } + const outside = join(root, 'outside') + const externalOverlay = boundary === 'overlay-root' ? join(outside, basename(overlay)) : outside + mkdirSync(join(externalOverlay, 'plugins'), { recursive: true }) + const sentinel = join(externalOverlay, 'plugins', 'keep.js') + writeFileSync(sentinel, 'export const keep = true') + writeFileSync( + join(externalOverlay, OPENCODE_OVERLAY_MANIFEST_FILE), + JSON.stringify({ topLevelEntries: [], pluginEntries: ['keep.js'] }) + ) + const replaced = + boundary === 'overlay-root' + ? dirname(overlay) + : boundary === 'source-overlay' + ? overlay + : join(overlay, 'plugins') + const target = boundary === 'plugins' ? join(outside, 'plugins') : outside + if (boundary === 'plugins') { + writeFileSync( + join(overlay, OPENCODE_OVERLAY_MANIFEST_FILE), + JSON.stringify({ topLevelEntries: [], pluginEntries: ['keep.js'] }) + ) + } + safeRemoveTree(replaced) + symlinkSync(target, replaced, process.platform === 'win32' ? 'junction' : 'dir') + const before = readdirSync(outside, { recursive: true }).toSorted() + + const result = service.buildPtyEnv('pane-2', source) + expect(readFileSync(sentinel, 'utf8')).toBe('export const keep = true') + expect(readdirSync(outside, { recursive: true }).toSorted()).toEqual(before) + expect(result).toEqual({ OPENCODE_CONFIG_DIR: source }) + } +) + +it('still removes a stale mirrored entry from a real owned overlay', () => { + const overlay = service.buildPtyEnv('pane-1', source).OPENCODE_CONFIG_DIR + if (!overlay) { + throw new Error('Expected an isolated overlay') + } + rmSync(join(source, 'plugins', 'user.js')) + expect(service.buildPtyEnv('pane-2', source)).toEqual({ OPENCODE_CONFIG_DIR: overlay }) + expect(readdirSync(join(overlay, 'plugins'))).toEqual(['orca-test.js']) +}) diff --git a/src/main/opencode/hook-service.ts b/src/main/opencode/hook-service.ts index 9162499472f..1461b8773dc 100644 --- a/src/main/opencode/hook-service.ts +++ b/src/main/opencode/hook-service.ts @@ -11,7 +11,7 @@ import { writeFileSync } from 'node:fs' import { createHash } from 'node:crypto' -import { isSafeDescendCandidate, mirrorEntry, safeRemoveTree } from '../pty/overlay-mirror' +import { isSafeDescendCandidate, mirrorEntry, safeRemoveOverlay } from '../pty/overlay-mirror' import { getOpenCode2PluginSource, getOpenCodeFamilyPluginSource, @@ -141,7 +141,13 @@ export class OpenCodeHookService { } const overlayDir = this.getSourceOverlayDir(existingConfigDir) try { - mkdirSync(overlayDir, { recursive: true }) + // Owned directories must stay real; a replaced parent redirects both cleanup and writes. + for (const directory of [this.getOverlayRoot(), overlayDir, join(overlayDir, 'plugins')]) { + mkdirSync(directory, { recursive: true }) + if (!isSafeDescendCandidate(lstatSync(directory))) { + return { OPENCODE_CONFIG_DIR: existingConfigDir } + } + } if (existsSync(existingConfigDir)) { this.mirrorUserConfig(existingConfigDir, overlayDir) } @@ -230,7 +236,7 @@ export class OpenCodeHookService { private clearManifestEntries(overlayDir: string, manifest: OpenCodeOverlayManifest): void { for (const entryName of manifest.topLevelEntries) { - safeRemoveTree(join(overlayDir, entryName)) + safeRemoveOverlay(join(overlayDir, entryName), overlayDir) } const overlayPluginsDir = join(overlayDir, 'plugins') @@ -238,7 +244,7 @@ export class OpenCodeHookService { if (entryName === this.pluginFileName) { continue } - safeRemoveTree(join(overlayPluginsDir, entryName)) + safeRemoveOverlay(join(overlayPluginsDir, entryName), overlayPluginsDir) } } diff --git a/src/main/opencode/opencode-overlay-manifest.test.ts b/src/main/opencode/opencode-overlay-manifest.test.ts new file mode 100644 index 00000000000..7dc0404a53e --- /dev/null +++ b/src/main/opencode/opencode-overlay-manifest.test.ts @@ -0,0 +1,32 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { expect, it } from 'vitest' +import { + readOpenCodeOverlayManifest, + OPENCODE_OVERLAY_MANIFEST_FILE +} from './opencode-overlay-manifest' + +it('accepts only string manifest entries and tolerates invalid persisted shapes', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-overlay-manifest-')) + try { + const path = join(root, OPENCODE_OVERLAY_MANIFEST_FILE) + for (const text of ['null', '1', '{']) { + writeFileSync(path, text) + expect(readOpenCodeOverlayManifest(root)).toEqual({ topLevelEntries: [], pluginEntries: [] }) + } + writeFileSync( + path, + JSON.stringify({ + topLevelEntries: ['valid', 1, null], + pluginEntries: [{ bad: true }, 'plugin.js'] + }) + ) + expect(readOpenCodeOverlayManifest(root)).toEqual({ + topLevelEntries: ['valid'], + pluginEntries: ['plugin.js'] + }) + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) diff --git a/src/main/pty/overlay-mirror.test.ts b/src/main/pty/overlay-mirror.test.ts index 5499d226422..1a741552719 100644 --- a/src/main/pty/overlay-mirror.test.ts +++ b/src/main/pty/overlay-mirror.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, rmSync, writeFileSync, symlinkSync } from 'node:fs' import { mkdtemp } from 'node:fs/promises' import { tmpdir } from 'node:os' import type * as NodePath from 'node:path' @@ -19,6 +19,33 @@ afterEach(() => { }) describe('safeRemoveOverlay', () => { + it('keeps missing owned paths a silent no-op', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-overlay-missing-')) + tempRoots.push(root) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + safeRemoveOverlay(join(root, 'missing', 'leaf'), join(root, 'missing')) + expect(warn).not.toHaveBeenCalled() + }) + + it('refuses cleanup through an intermediate symlink and unlinks only a leaf symlink', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-overlay-symlink-')) + tempRoots.push(root) + const overlay = join(root, 'overlay') + const outside = join(root, 'outside') + mkdirSync(overlay) + mkdirSync(outside) + const sentinel = join(outside, 'keep.txt') + writeFileSync(sentinel, 'private sentinel') + const linked = join(overlay, 'linked') + symlinkSync(outside, linked, process.platform === 'win32' ? 'junction' : 'dir') + vi.spyOn(console, 'warn').mockImplementation(() => {}) + safeRemoveOverlay(join(linked, 'keep.txt'), overlay) + expect(existsSync(sentinel)).toBe(true) + safeRemoveOverlay(linked, overlay) + expect(existsSync(linked)).toBe(false) + expect(existsSync(sentinel)).toBe(true) + }) + it('removes valid overlay children whose names start with dot-dot', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-overlay-root-')) tempRoots.push(root) diff --git a/src/main/pty/overlay-mirror.ts b/src/main/pty/overlay-mirror.ts index ec1b99d2b92..6a35844f871 100644 --- a/src/main/pty/overlay-mirror.ts +++ b/src/main/pty/overlay-mirror.ts @@ -138,6 +138,33 @@ export function safeRemoveTree(path: string): void { } } +// Why: cleanup must not traverse a symlink parent inside the owned overlay. +function hasSafeOverlayAncestors(root: string, relativeTarget: string): boolean { + let current = root + try { + if (lstatSync(current).isSymbolicLink()) { + return false + } + } catch (error) { + return !!error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT' + } + const segments = relativeTarget.split(sep).filter(Boolean) + for (const [index, segment] of segments.entries()) { + current = join(current, segment) + if (index === segments.length - 1) { + break + } + try { + if (lstatSync(current).isSymbolicLink()) { + return false + } + } catch (error) { + return !!error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT' + } + } + return true +} + // Why: last-line guard against an overlay-root constant ever being // mis-resolved. Any caller that points safeRemoveTree at a path outside its // designated overlay root is refused so a misconfiguration cannot turn into @@ -147,7 +174,13 @@ export function safeRemoveOverlay(overlayDir: string, overlayRoot: string): void const resolvedRoot = resolve(overlayRoot) const resolvedTarget = resolve(overlayDir) const rel = relative(resolvedRoot, resolvedTarget) - if (rel === '' || rel === '..' || rel.startsWith(`..${sep}`) || isAbsolute(rel)) { + if ( + rel === '' || + rel === '..' || + rel.startsWith(`..${sep}`) || + isAbsolute(rel) || + !hasSafeOverlayAncestors(resolvedRoot, rel) + ) { console.warn( `[overlay-mirror] refusing to remove overlay outside root: target=${resolvedTarget} root=${resolvedRoot}` )