From c703db2745bd4d20515ef3d6483ad604fb000385 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Fri, 12 Jun 2026 01:53:38 +0300 Subject: [PATCH] fix: stop forwarding client auth to OpenCode and harden home/session state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Packaged desktop showed no sessions in 1.12.4. Root cause: the sanitized session-list proxy path added in #1538 forwarded the renderer's "authorization" header (the OpenChamber UI client token) to the managed OpenCode upstream alongside the managed "Authorization" credential. OpenCode does not recognize UI client tokens, so every session-list request answered 401 — only in the packaged app, because only its renderer (openchamber-ui:// origin) attaches a bearer token; dev web and dev Electron run same-origin without one. The legacy http-proxy path overwrote the header correctly, which is why everything except session lists kept working. Proxy fix: - proxy-headers: filter the client "authorization" header out of forwarded request headers; the OpenCode upstream must only ever see its own managed credentials. Covered by tests. Desktop cwd: - electron: launch the managed OpenCode CLI from the user home instead of app userData, matching upstream desktop behavior. userData-as-cwd made OpenCode treat the app-data folder as a separate empty workspace. Home directory poisoning loop: - directoryPersistence: stop replaying localStorage homeDirectory through synchronizeHomeDirectory on boot/auth resync. The persisted value is only a boot-time cache; replaying it re-wrote stale values (e.g. a project path) into desktop settings on every start, overriding the authoritative /api/fs/home resolution. - persistence: never overwrite an injected window.__OPENCHAMBER_HOME__ with a persisted value. - useDirectoryStore: host switches happen in place (no reload), so re-resolve home from the new runtime's /api/fs/home on endpoint change instead of keeping the previous host's value. - opencode client: only short-circuit to the injected desktop home when the active runtime is local; remote runtimes ask /api/fs/home. Settings hygiene: - persistSettings: log field names only — change payloads can carry credentials (UI password, client tokens, tunnel tokens) that must not reach the log file; drop step-by-step log chatter. - validateProjectEntries: only stat project paths when the incoming update actually touches the projects list, not on every settings save. - remove the write-only approvedDirectories setting everywhere and add a migration that strips the stale key from persisted settings. Tests: - usePluginsStore.test: register an own runtime-fetch module mock so the suite is independent of process-global mock.module leakage from other files, and restore globalThis.fetch after the suite. - persistence.test: clean up the window global created for the suite. --- packages/electron/main.mjs | 12 ++-- packages/electron/opencode-cwd.mjs | 11 ++++ packages/electron/opencode-cwd.test.mjs | 23 ++++++++ packages/ui/src/lib/api/types.ts | 1 - packages/ui/src/lib/desktop.ts | 1 - packages/ui/src/lib/directoryPersistence.ts | 15 +++-- packages/ui/src/lib/opencode/client.ts | 16 +++-- packages/ui/src/lib/persistence.test.ts | 47 +++++++++++++++ packages/ui/src/lib/persistence.ts | 30 +++++----- packages/ui/src/stores/useDirectoryStore.ts | 14 +++++ .../ui/src/stores/usePluginsStore.test.ts | 18 +++++- packages/ui/src/types/bun-test.d.ts | 1 + packages/web/server/lib/opencode/routes.js | 2 - .../server/lib/opencode/settings-helpers.js | 39 ------------ .../settings-normalization-runtime.js | 1 - .../server/lib/opencode/settings-runtime.js | 59 +++++++++++-------- packages/web/server/proxy-headers.js | 4 ++ packages/web/server/proxy-headers.test.js | 21 +++++++ 18 files changed, 215 insertions(+), 100 deletions(-) create mode 100644 packages/electron/opencode-cwd.mjs create mode 100644 packages/electron/opencode-cwd.test.mjs create mode 100644 packages/ui/src/lib/persistence.test.ts diff --git a/packages/electron/main.mjs b/packages/electron/main.mjs index b3c81744..a1aa16de 100644 --- a/packages/electron/main.mjs +++ b/packages/electron/main.mjs @@ -12,6 +12,7 @@ import { promisify } from 'node:util'; import updaterPkg from 'electron-updater'; import { ElectronSshManager } from './ssh-manager.mjs'; import { createTrayController } from './tray.mjs'; +import { resolveManagedOpenCodeCwd } from './opencode-cwd.mjs'; const execFileAsync = promisify(execFile); @@ -1089,12 +1090,13 @@ const spawnLocalServer = async () => { process.env.OPENCHAMBER_HOST = bindHost; process.env.OPENCHAMBER_DIST_DIR = resolveWebDistDir(); process.env.OPENCHAMBER_RUNTIME = 'desktop'; - process.env.OPENCHAMBER_OPENCODE_CWD = app.getPath('userData'); + // OpenCode uses process cwd as a fallback directory; app userData would make + // packaged desktop look like a separate empty workspace. + process.env.OPENCHAMBER_OPENCODE_CWD = resolveManagedOpenCodeCwd({ + env: process.env, + homedir: () => os.homedir(), + }); process.env.OPENCHAMBER_DESKTOP_NOTIFY = 'true'; - try { - fs.mkdirSync(process.env.OPENCHAMBER_OPENCODE_CWD, { recursive: true }); - } catch { - } if (desktopUiPassword) { process.env.OPENCHAMBER_UI_PASSWORD = desktopUiPassword; } else { diff --git a/packages/electron/opencode-cwd.mjs b/packages/electron/opencode-cwd.mjs new file mode 100644 index 00000000..4ef1e89e --- /dev/null +++ b/packages/electron/opencode-cwd.mjs @@ -0,0 +1,11 @@ +export const resolveManagedOpenCodeCwd = ({ env, homedir }) => { + const configured = typeof env?.OPENCHAMBER_OPENCODE_CWD === 'string' + ? env.OPENCHAMBER_OPENCODE_CWD.trim() + : ''; + if (configured) { + return configured; + } + + const home = typeof homedir === 'function' ? homedir() : ''; + return typeof home === 'string' && home.trim() ? home : process.cwd(); +}; diff --git a/packages/electron/opencode-cwd.test.mjs b/packages/electron/opencode-cwd.test.mjs new file mode 100644 index 00000000..84805c4a --- /dev/null +++ b/packages/electron/opencode-cwd.test.mjs @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vitest'; + +import { resolveManagedOpenCodeCwd } from './opencode-cwd.mjs'; + +describe('resolveManagedOpenCodeCwd', () => { + it('defaults managed OpenCode cwd to the user home directory', () => { + expect(resolveManagedOpenCodeCwd({ env: {}, homedir: () => '/Users/example' })).toBe('/Users/example'); + }); + + it('preserves an explicit cwd override', () => { + expect(resolveManagedOpenCodeCwd({ + env: { OPENCHAMBER_OPENCODE_CWD: '/tmp/opencode-cwd' }, + homedir: () => '/Users/example', + })).toBe('/tmp/opencode-cwd'); + }); + + it('ignores a blank cwd override', () => { + expect(resolveManagedOpenCodeCwd({ + env: { OPENCHAMBER_OPENCODE_CWD: ' ' }, + homedir: () => '/Users/example', + })).toBe('/Users/example'); + }); +}); diff --git a/packages/ui/src/lib/api/types.ts b/packages/ui/src/lib/api/types.ts index c2c13623..6fe294f6 100644 --- a/packages/ui/src/lib/api/types.ts +++ b/packages/ui/src/lib/api/types.ts @@ -638,7 +638,6 @@ export interface SettingsPayload { opencodeBinary?: string; projects?: ProjectEntry[]; activeProjectId?: string; - approvedDirectories?: string[]; securityScopedBookmarks?: string[]; pinnedDirectories?: string[]; showReasoningTraces?: boolean; diff --git a/packages/ui/src/lib/desktop.ts b/packages/ui/src/lib/desktop.ts index 7cecccdc..d36067dc 100644 --- a/packages/ui/src/lib/desktop.ts +++ b/packages/ui/src/lib/desktop.ts @@ -59,7 +59,6 @@ export type DesktopSettings = { desktopUiPassword?: string; projects?: ProjectEntry[]; activeProjectId?: string; - approvedDirectories?: string[]; securityScopedBookmarks?: string[]; pinnedDirectories?: string[]; showReasoningTraces?: boolean; diff --git a/packages/ui/src/lib/directoryPersistence.ts b/packages/ui/src/lib/directoryPersistence.ts index 826208d5..4819c8de 100644 --- a/packages/ui/src/lib/directoryPersistence.ts +++ b/packages/ui/src/lib/directoryPersistence.ts @@ -6,23 +6,22 @@ export const applyPersistedDirectoryPreferences = async (): Promise => { return; } - let savedHome: string | null = null; let savedDirectory: string | null = null; try { - savedHome = window.localStorage.getItem('homeDirectory'); savedDirectory = window.localStorage.getItem('lastDirectory'); } catch (error) { console.warn('Failed to read saved directory preferences:', error); } - const directoryStore = useDirectoryStore.getState(); - - if (savedHome && directoryStore.homeDirectory !== savedHome) { - directoryStore.synchronizeHomeDirectory(savedHome); - } + // Home directory is intentionally NOT restored from localStorage here. + // The persisted value is only a boot-time cache already consumed by the + // directory store's initial state; replaying it through + // synchronizeHomeDirectory would persist a possibly stale value back into + // desktop settings, overriding the authoritative resolution + // (initializeHomeDirectory → /api/fs/home) that runs on every startup. if (savedDirectory && !isVSCodeRuntime()) { - directoryStore.setDirectory(savedDirectory, { showOverlay: false }); + useDirectoryStore.getState().setDirectory(savedDirectory, { showOverlay: false }); } }; diff --git a/packages/ui/src/lib/opencode/client.ts b/packages/ui/src/lib/opencode/client.ts index 285355dc..58713305 100644 --- a/packages/ui/src/lib/opencode/client.ts +++ b/packages/ui/src/lib/opencode/client.ts @@ -16,6 +16,7 @@ import type { PermissionRequest } from "@/types/permission"; import type { QuestionRequest } from "@/types/question"; import { getRuntimeUrlResolver } from "@/lib/runtime-url"; import { runtimeFetch } from "@/lib/runtime-fetch"; +import { getRuntimeKey } from "@/lib/runtime-switch"; import { getRegisteredRuntimeAPIs } from "@/contexts/runtimeAPIRegistry"; import { markStartupTrace } from "@/lib/startupTrace"; import { @@ -1627,11 +1628,16 @@ class OpencodeService { } async getFilesystemHome(): Promise { - // Optimization: Check for desktop runtime first to avoid unnecessary network calls - // and fix the "SyntaxError" warning when the endpoint is missing - const desktopHome = await getDesktopHomeDirectory(); - if (desktopHome) { - return desktopHome; + // The injected desktop home describes the LOCAL machine. It is only a + // valid answer while the active runtime is the local one — after an + // in-place switch to a remote host the home must come from that host's + // /api/fs/home, not from the local Electron global. + const runtimeKey = getRuntimeKey(); + if (!runtimeKey || runtimeKey === 'local') { + const desktopHome = await getDesktopHomeDirectory(); + if (desktopHome) { + return desktopHome; + } } try { diff --git a/packages/ui/src/lib/persistence.test.ts b/packages/ui/src/lib/persistence.test.ts new file mode 100644 index 00000000..2d63e89e --- /dev/null +++ b/packages/ui/src/lib/persistence.test.ts @@ -0,0 +1,47 @@ +import { afterAll, beforeEach, describe, expect, test } from 'bun:test'; + +import { applyPersistedHomeDirectoryToWindow } from './persistence'; + +type TestWindow = { __OPENCHAMBER_HOME__?: string }; + +let createdWindow = false; + +const getWindow = (): TestWindow => { + if (typeof window === 'undefined') { + Object.defineProperty(globalThis, 'window', { + value: {}, + configurable: true, + writable: true, + }); + createdWindow = true; + } + return window as unknown as TestWindow; +}; + +describe('applyPersistedHomeDirectoryToWindow', () => { + beforeEach(() => { + delete getWindow().__OPENCHAMBER_HOME__; + }); + + afterAll(() => { + if (createdWindow) { + delete (globalThis as { window?: unknown }).window; + } else { + delete getWindow().__OPENCHAMBER_HOME__; + } + }); + + test('does not overwrite an injected desktop home directory', () => { + getWindow().__OPENCHAMBER_HOME__ = '/Users/example'; + + applyPersistedHomeDirectoryToWindow('/Users/example/projects/app'); + + expect(getWindow().__OPENCHAMBER_HOME__).toBe('/Users/example'); + }); + + test('uses persisted home when no runtime home was injected', () => { + applyPersistedHomeDirectoryToWindow('/Users/example/projects/app'); + + expect(getWindow().__OPENCHAMBER_HOME__).toBe('/Users/example/projects/app'); + }); +}); diff --git a/packages/ui/src/lib/persistence.ts b/packages/ui/src/lib/persistence.ts index 9a832804..689a83dd 100644 --- a/packages/ui/src/lib/persistence.ts +++ b/packages/ui/src/lib/persistence.ts @@ -11,6 +11,21 @@ import { sanitizeStarterRefs } from '@/lib/draftStarters'; import { normalizeMobileKeyboardMode, setStoredMobileKeyboardMode } from '@/lib/mobileKeyboardMode'; import { runtimeFetch } from '@/lib/runtime-fetch'; +export const applyPersistedHomeDirectoryToWindow = (homeDirectory: string): void => { + if (typeof window === 'undefined') { + return; + } + if (typeof window.__OPENCHAMBER_HOME__ === 'string' && window.__OPENCHAMBER_HOME__.length > 0) { + return; + } + + try { + window.__OPENCHAMBER_HOME__ = homeDirectory; + } catch { + /* read-only contextBridge property — leave preload-seeded value */ + } +}; + const persistToLocalStorage = (settings: DesktopSettings) => { if (typeof window === 'undefined') { return; @@ -36,15 +51,7 @@ const persistToLocalStorage = (settings: DesktopSettings) => { } if (settings.homeDirectory) { localStorage.setItem('homeDirectory', settings.homeDirectory); - // Electron's preload exposes __OPENCHAMBER_HOME__ as a read-only - // contextBridge property; assignment throws TypeError there. In VSCode - // webview and plain web runtime the property is writable. Swallow the - // error in Electron — preload already seeded the value correctly. - try { - window.__OPENCHAMBER_HOME__ = settings.homeDirectory; - } catch { - /* read-only contextBridge property — leave preload-seeded value */ - } + applyPersistedHomeDirectoryToWindow(settings.homeDirectory); } if (Array.isArray(settings.projects) && settings.projects.length > 0) { localStorage.setItem('projects', JSON.stringify(settings.projects)); @@ -681,11 +688,6 @@ const sanitizeWebSettings = (payload: unknown): DesktopSettings | null => { result.activeProjectId = candidate.activeProjectId; } - if (Array.isArray(candidate.approvedDirectories)) { - result.approvedDirectories = candidate.approvedDirectories.filter( - (entry): entry is string => typeof entry === 'string' && entry.length > 0 - ); - } if (Array.isArray(candidate.securityScopedBookmarks)) { result.securityScopedBookmarks = candidate.securityScopedBookmarks.filter( (entry): entry is string => typeof entry === 'string' && entry.length > 0 diff --git a/packages/ui/src/stores/useDirectoryStore.ts b/packages/ui/src/stores/useDirectoryStore.ts index 372cc6a0..ea7d59ff 100644 --- a/packages/ui/src/stores/useDirectoryStore.ts +++ b/packages/ui/src/stores/useDirectoryStore.ts @@ -2,6 +2,7 @@ import { create } from 'zustand'; import { devtools } from 'zustand/middleware'; import { opencodeClient } from '@/lib/opencode/client'; import { getDesktopHomeDirectory, isVSCodeRuntime } from '@/lib/desktop'; +import { subscribeRuntimeEndpointChanged } from '@/lib/runtime-switch'; import { updateDesktopSettings } from '@/lib/persistence'; import { useFileSearchStore } from '@/stores/useFileSearchStore'; import { streamDebugEnabled } from '@/stores/utils/streamDebug'; @@ -26,6 +27,7 @@ interface DirectoryStore { } let cachedHomeDirectory: string | null = null; +let homeResolveGeneration = 0; const safeStorage = getSafeStorage(); const persistedLastDirectory = safeStorage.getItem('lastDirectory'); const initialHasPersistedDirectory = @@ -437,4 +439,16 @@ if (typeof window !== 'undefined') { initializeHomeDirectory().then((home) => { useDirectoryStore.getState().synchronizeHomeDirectory(home); }); + + // Host switches happen in place (no page reload), so the home directory + // must be re-resolved from the new runtime's authoritative source instead + // of keeping the previous host's value cached. + subscribeRuntimeEndpointChanged(() => { + cachedHomeDirectory = null; + const generation = ++homeResolveGeneration; + initializeHomeDirectory().then((home) => { + if (generation !== homeResolveGeneration) return; + useDirectoryStore.getState().synchronizeHomeDirectory(home); + }); + }); } diff --git a/packages/ui/src/stores/usePluginsStore.test.ts b/packages/ui/src/stores/usePluginsStore.test.ts index 2aab0482..7c8c7cd2 100644 --- a/packages/ui/src/stores/usePluginsStore.test.ts +++ b/packages/ui/src/stores/usePluginsStore.test.ts @@ -1,4 +1,6 @@ -import { beforeEach, describe, expect, mock, test } from 'bun:test'; +import { afterAll, beforeEach, describe, expect, mock, test } from 'bun:test'; + +const originalFetch = globalThis.fetch; import type { PluginEntry, PluginFile, RegistryResult } from './usePluginsStore'; @@ -31,6 +33,16 @@ mock.module('@/lib/configUpdate', () => ({ finishConfigUpdate: finishConfigUpdateMock, })); +// mock.module is process-global in bun: another test file (e.g. +// useCommandsStore.test.ts) may have replaced '@/lib/runtime-fetch' with its +// own stub before this file runs. Register our own mock so this suite always +// reaches its fetch double regardless of test file ordering. Delegating to +// globalThis.fetch (instead of this file's double directly) keeps later test +// files that stub global fetch working if this registration outlives us. +mock.module('@/lib/runtime-fetch', () => ({ + runtimeFetch: (input: RequestInfo | URL, init?: RequestInit) => globalThis.fetch(input, init), +})); + const { usePluginsStore } = await import('./usePluginsStore'); const entry: PluginEntry = { @@ -126,6 +138,10 @@ describe('usePluginsStore', () => { globalThis.fetch = fetchMock as unknown as typeof fetch; }); + afterAll(() => { + globalThis.fetch = originalFetch; + }); + test('loadPlugins calls config plugins endpoint once and populates entries/files', async () => { queueFetchResponses([jsonResponse(pluginListPayload), jsonResponse({ results: [registryOk] })]); diff --git a/packages/ui/src/types/bun-test.d.ts b/packages/ui/src/types/bun-test.d.ts index da75b83b..728dcd8d 100644 --- a/packages/ui/src/types/bun-test.d.ts +++ b/packages/ui/src/types/bun-test.d.ts @@ -24,6 +24,7 @@ declare module "bun:test" { }; }; export function beforeEach(fn: () => void | Promise): void; + export function afterAll(fn: () => void | Promise): void; export function mock unknown>(fn?: T): T; export namespace mock { function module(moduleName: string, factory: () => Record): void; diff --git a/packages/web/server/lib/opencode/routes.js b/packages/web/server/lib/opencode/routes.js index 2bbe93ca..f7c54db5 100644 --- a/packages/web/server/lib/opencode/routes.js +++ b/packages/web/server/lib/opencode/routes.js @@ -258,10 +258,8 @@ export const registerOpenCodeRoutes = (app, dependencies) => { }); app.put('/api/config/settings', async (req, res) => { - console.log('[API:PUT /api/config/settings] Received request'); try { const updated = await persistSettings(req.body ?? {}); - console.log(`[API:PUT /api/config/settings] Success, returning ${updated.projects?.length || 0} projects`); res.json(updated); } catch (error) { console.error('[API:PUT /api/config/settings] Failed to save settings:', error); diff --git a/packages/web/server/lib/opencode/settings-helpers.js b/packages/web/server/lib/opencode/settings-helpers.js index 57469b97..4664992f 100644 --- a/packages/web/server/lib/opencode/settings-helpers.js +++ b/packages/web/server/lib/opencode/settings-helpers.js @@ -145,13 +145,6 @@ export const createSettingsHelpers = (dependencies) => { result.activeProjectId = candidate.activeProjectId; } - if (Array.isArray(candidate.approvedDirectories)) { - result.approvedDirectories = normalizeStringArray( - candidate.approvedDirectories - .map((entry) => (typeof entry === 'string' ? normalizePathForPersistence(entry) : entry)) - .filter((entry) => typeof entry === 'string' && entry.length > 0) - ); - } if (Array.isArray(candidate.securityScopedBookmarks)) { result.securityScopedBookmarks = normalizeStringArray(candidate.securityScopedBookmarks); } @@ -702,31 +695,6 @@ export const createSettingsHelpers = (dependencies) => { }; const mergePersistedSettings = (current, changes) => { - const baseApproved = Array.isArray(changes.approvedDirectories) - ? changes.approvedDirectories - : Array.isArray(current.approvedDirectories) - ? current.approvedDirectories - : []; - - const additionalApproved = []; - if (typeof changes.lastDirectory === 'string' && changes.lastDirectory.length > 0) { - additionalApproved.push(changes.lastDirectory); - } - if (typeof changes.homeDirectory === 'string' && changes.homeDirectory.length > 0) { - additionalApproved.push(changes.homeDirectory); - } - const projectEntries = Array.isArray(changes.projects) - ? changes.projects - : Array.isArray(current.projects) - ? current.projects - : []; - projectEntries.forEach((project) => { - if (project && typeof project.path === 'string' && project.path.length > 0) { - additionalApproved.push(project.path); - } - }); - const approvedSource = [...baseApproved, ...additionalApproved]; - const baseBookmarks = Array.isArray(changes.securityScopedBookmarks) ? changes.securityScopedBookmarks : Array.isArray(current.securityScopedBookmarks) @@ -743,11 +711,6 @@ export const createSettingsHelpers = (dependencies) => { const next = { ...current, ...changes, - approvedDirectories: Array.from( - new Set( - approvedSource.filter((entry) => typeof entry === 'string' && entry.length > 0) - ) - ), securityScopedBookmarks: Array.from( new Set( baseBookmarks.filter((entry) => typeof entry === 'string' && entry.length > 0) @@ -762,7 +725,6 @@ export const createSettingsHelpers = (dependencies) => { const formatSettingsResponse = (settings) => { const sanitized = sanitizeSettingsUpdate(settings); delete sanitized.managedRemoteTunnelToken; - const approved = normalizeStringArray(settings.approvedDirectories); const bookmarks = normalizeStringArray(settings.securityScopedBookmarks); const hasManagedRemoteTunnelToken = typeof settings?.managedRemoteTunnelToken === 'string' && settings.managedRemoteTunnelToken.trim().length > 0; const pwaAppName = normalizePwaAppName(settings?.pwaAppName, ''); @@ -775,7 +737,6 @@ export const createSettingsHelpers = (dependencies) => { ...(pwaAppName ? { pwaAppName } : {}), pwaOrientation, mobileKeyboardMode, - approvedDirectories: approved, securityScopedBookmarks: bookmarks, pinnedDirectories: normalizeStringArray(settings.pinnedDirectories), typographySizes: sanitizeTypographySizesPartial(settings.typographySizes), diff --git a/packages/web/server/lib/opencode/settings-normalization-runtime.js b/packages/web/server/lib/opencode/settings-normalization-runtime.js index 89810edd..f237e559 100644 --- a/packages/web/server/lib/opencode/settings-normalization-runtime.js +++ b/packages/web/server/lib/opencode/settings-normalization-runtime.js @@ -226,7 +226,6 @@ export const createSettingsNormalizationRuntime = (dependencies) => { normalizePathField('lastDirectory'); normalizePathField('homeDirectory'); - normalizePathArrayField('approvedDirectories'); normalizePathArrayField('pinnedDirectories'); if (Array.isArray(settings.projects)) { diff --git a/packages/web/server/lib/opencode/settings-runtime.js b/packages/web/server/lib/opencode/settings-runtime.js index 638d4aea..795e89d5 100644 --- a/packages/web/server/lib/opencode/settings-runtime.js +++ b/packages/web/server/lib/opencode/settings-runtime.js @@ -493,41 +493,34 @@ export const createSettingsRuntime = (deps) => { }; const validateProjectEntries = async (projects) => { - console.log(`[validateProjectEntries] Starting validation for ${projects.length} projects`); - if (!Array.isArray(projects)) { - console.warn('[validateProjectEntries] Input is not an array, returning empty'); return []; } const validations = projects.map(async (project) => { if (!project || typeof project.path !== 'string' || project.path.length === 0) { - console.error('[validateProjectEntries] Invalid project entry: missing or empty path', project); + console.warn('[validateProjectEntries] Dropping project entry with missing or empty path'); return null; } try { const stats = await fsPromises.stat(project.path); if (!stats.isDirectory()) { - console.error(`[validateProjectEntries] Project path is not a directory: ${project.path}`); + console.warn(`[validateProjectEntries] Dropping project — path is not a directory: ${project.path}`); return null; } return project; } catch (error) { - const err = error; - console.error(`[validateProjectEntries] Failed to validate project "${project.path}": ${err.code || err.message || err}`); - if (err && typeof err === 'object' && err.code === 'ENOENT') { - console.log(`[validateProjectEntries] Removing project with ENOENT: ${project.path}`); + if (error && typeof error === 'object' && error.code === 'ENOENT') { + console.warn(`[validateProjectEntries] Dropping project — directory no longer exists: ${project.path}`); return null; } - console.log(`[validateProjectEntries] Keeping project despite non-ENOENT error: ${project.path}`); + // Permission or transient fs error — keep the project rather than + // silently losing it from the user's list. return project; } }); - const results = (await Promise.all(validations)).filter((p) => p !== null); - - console.log(`[validateProjectEntries] Validation complete: ${results.length}/${projects.length} projects valid`); - return results; + return (await Promise.all(validations)).filter((p) => p !== null); }; const migrateSettingsFromLegacyLastDirectory = async (current) => { @@ -769,6 +762,19 @@ export const createSettingsRuntime = (deps) => { return { settings: changed ? next : settings, changed }; }; + // `approvedDirectories` was a write-only registry: every project path and + // visited directory was appended forever, but nothing ever read it. Strip + // the stale key from persisted settings on upgrade. + const migrateSettingsRemoveApprovedDirectories = (current) => { + const settings = current && typeof current === 'object' ? current : {}; + if (!Object.prototype.hasOwnProperty.call(settings, 'approvedDirectories')) { + return { settings, changed: false }; + } + const next = { ...settings }; + delete next.approvedDirectories; + return { settings: next, changed: true }; + }; + const readSettingsFromDiskMigrated = async () => { const current = await readSettingsFromDisk(); const migration1 = await migrateSettingsFromLegacyLastDirectory(current); @@ -778,17 +784,19 @@ export const createSettingsRuntime = (deps) => { const migration5 = await migrateSettingsFromLegacyNamedTunnelKeys(migration4.settings); const migration6 = normalizeSettingsPaths(migration5.settings); const migration7 = await migrateSettingsToDeterministicProjectIds(migration6.settings); - if (migration1.changed || migration2.changed || migration3.changed || migration4.changed || migration5.changed || migration6.changed || migration7.changed) { - await writeSettingsToDisk(migration7.settings); + const migration8 = migrateSettingsRemoveApprovedDirectories(migration7.settings); + if (migration1.changed || migration2.changed || migration3.changed || migration4.changed || migration5.changed || migration6.changed || migration7.changed || migration8.changed) { + await writeSettingsToDisk(migration8.settings); } - return migration7.settings; + return migration8.settings; }; const persistSettings = async (changes) => { persistSettingsLock = persistSettingsLock.then(async () => { - console.log('[persistSettings] Called with changes:', JSON.stringify(changes, null, 2)); + // Log field names only — changes can carry credentials (UI password, + // client tokens, tunnel tokens) that must never reach the log file. + console.log('[persistSettings] Updating fields:', Object.keys(changes || {}).join(', ') || '(none)'); const current = await readSettingsFromDisk(); - console.log('[persistSettings] Current projects count:', Array.isArray(current.projects) ? current.projects.length : 'N/A'); const sanitized = sanitizeSettingsUpdate(changes); let next = mergePersistedSettings(current, sanitized); @@ -802,10 +810,16 @@ export const createSettingsRuntime = (deps) => { next = deterministicProjectIdMigration.settings; } - if (Array.isArray(next.projects)) { - console.log(`[persistSettings] Validating ${next.projects.length} projects...`); + const approvedDirectoriesMigration = migrateSettingsRemoveApprovedDirectories(next); + if (approvedDirectoriesMigration.changed) { + next = approvedDirectoriesMigration.settings; + } + + // Validating project paths hits the filesystem for every entry, so only + // do it when the incoming update actually touches the projects list — + // not on every theme/window-state/etc. save. + if (Object.prototype.hasOwnProperty.call(sanitized, 'projects') && Array.isArray(next.projects)) { const validated = await validateProjectEntries(next.projects); - console.log(`[persistSettings] After validation: ${validated.length} projects remain`); next = { ...next, projects: validated }; } @@ -848,7 +862,6 @@ export const createSettingsRuntime = (deps) => { } await writeSettingsToDisk(next); - console.log(`[persistSettings] Successfully saved ${next.projects?.length || 0} projects to disk`); return formatSettingsResponse(next); }); diff --git a/packages/web/server/proxy-headers.js b/packages/web/server/proxy-headers.js index e82cef12..92600d22 100644 --- a/packages/web/server/proxy-headers.js +++ b/packages/web/server/proxy-headers.js @@ -1,4 +1,8 @@ const filteredRequestHeaders = new Set([ + // Client credentials for the OpenChamber server (UI client tokens) must + // never reach the managed OpenCode upstream — it only accepts its own auth, + // so a forwarded client bearer turns every upstream response into a 401. + 'authorization', 'host', 'connection', 'content-length', diff --git a/packages/web/server/proxy-headers.test.js b/packages/web/server/proxy-headers.test.js index 0041e6a9..101f270f 100644 --- a/packages/web/server/proxy-headers.test.js +++ b/packages/web/server/proxy-headers.test.js @@ -18,6 +18,27 @@ describe('OpenCode proxy header handling', () => { expect(headers['accept-encoding']).toBeUndefined(); }); + it('replaces client authorization with managed OpenCode auth', () => { + const headers = collectForwardProxyHeaders( + { authorization: 'Bearer oc_client_stale-ui-token' }, + { Authorization: 'Bearer managed-opencode-token' }, + ); + + expect(headers.Authorization).toBe('Bearer managed-opencode-token'); + expect(headers['authorization']).toBeUndefined(); + }); + + it('drops client authorization when upstream has no managed auth', () => { + const headers = collectForwardProxyHeaders({ + accept: 'application/json', + authorization: 'Bearer oc_client_stale-ui-token', + }); + + expect(headers['authorization']).toBeUndefined(); + expect(headers.Authorization).toBeUndefined(); + expect(headers.accept).toBe('application/json'); + }); + it('drops content-encoding from forwarded response headers', () => { expect(shouldForwardProxyResponseHeader('content-encoding')).toBe(false); expect(shouldForwardProxyResponseHeader('Content-Encoding')).toBe(false);