fix(opencode): accept daemon-shaped sessionIds, hash to safe dir name (#1155)

Co-authored-by: Orca <help@stably.ai>

src/main/opencode/hook-service.test.ts

@@ -1,6 +1,8 @@
 import { describe, expect, it } from 'vitest'
 import { _internals } from './hook-service'
 
+const { isUsableId, toSafeDirName } = _internals
+
 describe('OpenCode hook plugin source', () => {
   it('filters child sessions via parentID lookup before forwarding events', () => {
     const source = _internals.getOpenCodePluginSource()
@@ -19,3 +21,48 @@ describe('OpenCode hook plugin source', () => {
     expect(source).toContain('const client = _ctx?.client;')
   })
 })
+
+describe('OpenCode id safety guard', () => {
+  it('accepts the daemon-path sessionId shape (worktreeId@@uuid with ::/...)', () => {
+    // Why: after the daemon-parity refactor (#1148) pty.ts mints sessionIds
+    // like `<worktreeId>@@<uuid>` where worktreeId contains "::" and a
+    // filesystem path. The previous strict regex rejected every real id and
+    // silently dropped OPENCODE_CONFIG_DIR. Lock in that such ids are now
+    // accepted so the plugin dir is actually written.
+    const daemonSessionId =
+      '50c010a2-bc8e-4eb1-8847-5812133ad6df::/Users/thebr/ghostx/workspaces/noqa/autoheal@@a1b2c3d4'
+    expect(isUsableId(daemonSessionId)).toBe(true)
+  })
+
+  it('accepts ids at the inclusive upper length bound', () => {
+    expect(isUsableId('x'.repeat(1024))).toBe(true)
+  })
+
+  it('rejects empty or oversized ids', () => {
+    expect(isUsableId('')).toBe(false)
+    expect(isUsableId('x'.repeat(1025))).toBe(false)
+  })
+
+  it('rejects non-string runtime values even though the type says string', () => {
+    // Why: the typeof guard is defense-in-depth for any-typed callers;
+    // without a test, a future refactor could delete the guard silently.
+    expect(isUsableId(undefined as unknown as string)).toBe(false)
+    expect(isUsableId(null as unknown as string)).toBe(false)
+    expect(isUsableId(42 as unknown as string)).toBe(false)
+  })
+
+  it('derives a filesystem-safe directory name independent of the raw id', () => {
+    const name = toSafeDirName('50c010::/Users/thebr/x/y@@uuid')
+    // Pure hex, bounded length — no slashes, colons, or caller content.
+    expect(name).toMatch(/^[0-9a-f]{32}$/)
+  })
+
+  it('is stable across calls for the same id', () => {
+    const id = 'some-session-id'
+    expect(toSafeDirName(id)).toBe(toSafeDirName(id))
+  })
+
+  it('produces different names for different ids', () => {
+    expect(toSafeDirName('a')).not.toBe(toSafeDirName('b'))
+  })
+})

src/main/opencode/hook-service.ts

@@ -1,21 +1,38 @@
 import { app } from 'electron'
 import { join } from 'path'
 import { mkdirSync, writeFileSync, rmSync } from 'fs'
+import { createHash } from 'crypto'
 
 const ORCA_OPENCODE_PLUGIN_FILE = 'orca-opencode-status.js'
 
-// Why: ptyId today is allocated by Orca (safe UUID-shape), but both entry
-// points construct a filesystem path with it and one of them calls
-// rmSync(..., recursive) on the result. Reject obviously unsafe IDs as a
-// belt-and-braces guard so a future caller (or a bug that forwards an
-// external ID) cannot escape userData/opencode-hooks/.
-function isSafePtyId(ptyId: string): boolean {
-  if (!ptyId || ptyId.length === 0 || ptyId.length > 128) {
-    return false
-  }
-  // Allow alphanumeric, dash, underscore, period (but not leading period or
-  // any slashes/backslashes).
-  return /^[A-Za-z0-9_-][A-Za-z0-9_.-]*$/.test(ptyId) && !ptyId.includes('..')
+// Why: the id passed in by pty.ts's daemon path is a sessionId shaped like
+// "<worktreeId>@@<uuid>" where worktreeId itself contains "::" and a
+// filesystem path (slashes, colons). Earlier the id was a simple numeric
+// counter, so rejecting anything with "/" or ":" was a safe guard against
+// path traversal. After the daemon-parity refactor (#1148) the sessionId
+// shape changed, and the old regex silently rejected every legitimate id,
+// leaving OPENCODE_CONFIG_DIR unset and the plugin never loading.
+//
+// Keep an input-bounds guard (non-empty, bounded length) for defense in
+// depth, and derive the on-disk directory name via hash so any caller's id —
+// including ones containing path separators — produces a short, stable,
+// filesystem-safe name. Hashing also eliminates path-traversal risk at the
+// source: the directory name is always 32 hex chars, never a prefix/suffix
+// of the caller's input.
+// Why: 1024 is a generous sanity cap — daemon-shaped ids embed a worktree
+// filesystem path plus "@@<uuid>", and this bound prevents pathological inputs
+// from burning CPU in the SHA-256 step. Since the id is hashed anyway, 1024
+// is decoupled from PATH_MAX.
+function isUsableId(id: string): boolean {
+  return typeof id === 'string' && id.length > 0 && id.length <= 1024
+}
+
+function toSafeDirName(id: string): string {
+  // Why: SHA-256 truncated to 32 hex chars (128 bits) is ample for a
+  // per-session directory name — collisions require ~2^64 concurrent sessions
+  // to become likely, far beyond any real workload. Hex keeps the name
+  // portable across all filesystems (no base64 padding, no `/`).
+  return createHash('sha256').update(id).digest('hex').slice(0, 32)
 }
 
 function getOpenCodePluginSource(): string {
@@ -210,13 +227,13 @@ function getOpenCodePluginSource(): string {
 // pipeline as Claude/Codex/Gemini.
 export class OpenCodeHookService {
   clearPty(ptyId: string): void {
-    if (!isSafePtyId(ptyId)) {
+    if (!isUsableId(ptyId)) {
       return
     }
-    // Why: writePluginConfig creates a directory per PTY under userData. Without
-    // cleanup these accumulate across sessions since ptyId is a monotonically
-    // increasing counter. Remove the directory when the PTY is torn down.
-    const configDir = join(app.getPath('userData'), 'opencode-hooks', ptyId)
+    // Why: writePluginConfig creates a directory per PTY under userData.
+    // Without cleanup these accumulate across sessions. Using getConfigDir
+    // keeps cleanup aligned with the path writePluginConfig created.
+    const configDir = this.getConfigDir(ptyId)
     try {
       rmSync(configDir, { recursive: true, force: true })
     } catch {
@@ -242,11 +259,15 @@ export class OpenCodeHookService {
     return { OPENCODE_CONFIG_DIR: configDir }
   }
 
+  private getConfigDir(ptyId: string): string {
+    return join(app.getPath('userData'), 'opencode-hooks', toSafeDirName(ptyId))
+  }
+
   private writePluginConfig(ptyId: string): string | null {
-    if (!isSafePtyId(ptyId)) {
+    if (!isUsableId(ptyId)) {
       return null
     }
-    const configDir = join(app.getPath('userData'), 'opencode-hooks', ptyId)
+    const configDir = this.getConfigDir(ptyId)
     const pluginsDir = join(configDir, 'plugins')
     try {
       mkdirSync(pluginsDir, { recursive: true })
@@ -264,5 +285,6 @@ export class OpenCodeHookService {
 export const openCodeHookService = new OpenCodeHookService()
 export const _internals = {
   getOpenCodePluginSource,
-  isSafePtyId
+  isUsableId,
+  toSafeDirName
 }