diff --git a/CHANGELOG.md b/CHANGELOG.md index 46731e19..e1e393a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- **Settings:** OpenChamber no longer replaces a full OpenCode config with an empty `$schema`-only stub when the file uses JSON5-style unquoted keys; Settings changes now fail instead of wiping plugins, MCP servers, and providers (thanks to @makeittech). - **Settings/Integrations:** a new Integrations settings page lists Claude Code, Command Code, and Cursor plugins with install, update, setup, and remove actions, plus Discord and Telegram Coming soon placeholders. - Usage/Claude: Claude plan limits now work when you are signed in through Claude Code, without also signing into Anthropic in OpenCode; the account is read from Claude Code's own login on macOS, Linux, and WSL. The page shows your session and weekly limits again, adds per-model weekly limits and extra usage spending, and names your plan. Limits are kept on screen instead of disappearing when Anthropic temporarily blocks refreshes. - Git: the pull request panel now follows the branch's current open PR, and an open PR always wins over an older merged or closed one. After a PR is merged or closed the panel keeps showing it as the branch's last PR and offers creating the next one right below it (thanks to @makeittech). diff --git a/packages/vscode/CHANGELOG.md b/packages/vscode/CHANGELOG.md index f0bfc64e..fc94aca5 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -1,5 +1,6 @@ ## [Unreleased] +- **Settings:** OpenChamber no longer replaces a full OpenCode config with an empty `$schema`-only stub when the file uses JSON5-style unquoted keys; Settings changes now fail instead of wiping plugins, MCP servers, and providers (thanks to @makeittech). - **Settings/Integrations:** a new Integrations settings page lists Claude Code, Command Code, and Cursor plugins with install, update, setup, and remove actions, plus Discord and Telegram Coming soon placeholders. - Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. - Chat: opening a busy subagent in the context panel now shows its history instead of only the working-status line (thanks to @makeittech). diff --git a/packages/vscode/src/DOCUMENTATION.md b/packages/vscode/src/DOCUMENTATION.md index 3a4d3dde..e3447e21 100644 --- a/packages/vscode/src/DOCUMENTATION.md +++ b/packages/vscode/src/DOCUMENTATION.md @@ -58,6 +58,7 @@ The webview CSP permits `blob:` only for `worker-src` so shared UI parsers can r - `bridge-config-runtime.ts` - Config and skills message handlers (`api:config/*`). - Includes OpenCode resolution diagnostics parity handler used by shared UI (`/api/config/opencode-resolution`). + - OpenCode JSONC reads in `opencodeConfig.ts` fail closed on a partial or non-object `jsonc-parser` tree (`INVALID_JSONC`) so mutations cannot rewrite a `$schema`-only stub over an existing config. Comment-only files read as empty, while other content that yields no JSON value (YAML, plain text) fails closed. A broken layer is omitted from the merge and recorded on `layerErrors`; valid sibling layers still load, including plugin list/read via `getPluginConfigSources`. Writes still refuse to overwrite the broken file. - `bridge-settings-runtime.ts` - Settings read/write and OpenCode skills discovery via API for bridge consumers. diff --git a/packages/vscode/src/opencodeConfig.config-parse.test.ts b/packages/vscode/src/opencodeConfig.config-parse.test.ts new file mode 100644 index 00000000..74d6d737 --- /dev/null +++ b/packages/vscode/src/opencodeConfig.config-parse.test.ts @@ -0,0 +1,149 @@ +import { afterEach, beforeEach, describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { listPluginEntries, updateMcpConfig } from './opencodeConfig'; + +const PARTIAL_PARSE_CONFIG = [ + '{', + ' "$schema": "https://opencode.ai/config.json",', + ' plugin: ["opencode-see-image"],', + ' mcp: {', + ' openproject: {', + ' type: "remote",', + ' url: "https://openproject.example.com/mcp",', + ' enabled: true', + ' }', + ' },', + ' provider: {', + ' "ollama-cloud": {', + ' npm: "@ai-sdk/openai-compatible",', + ' name: "Ollama Cloud"', + ' }', + ' }', + '}', + '', +].join('\n'); + +const VALID_CONFIG = [ + '{', + ' "$schema": "https://opencode.ai/config.json",', + ' "plugin": ["opencode-see-image"],', + ' "mcp": {', + ' "openproject": {', + ' "type": "remote",', + ' "url": "https://openproject.example.com/mcp",', + ' "enabled": true', + ' }', + ' },', + ' "provider": {', + ' "ollama-cloud": {', + ' "npm": "@ai-sdk/openai-compatible",', + ' "name": "Ollama Cloud"', + ' }', + ' }', + '}', + '', +].join('\n'); + +const isInvalidJsoncError = (error: unknown): boolean => { + if (!(error instanceof Error) || !/cannot be loaded safely/.test(error.message)) { + return false; + } + // SAFETY: the config layer throws Error instances carrying the coded `code` field. + return (error as Error & { code?: string }).code === 'INVALID_JSONC'; +}; + +describe('opencodeConfig JSONC parse safety (issue #2923)', () => { + let tempDir: string; + let previousOpenCodeConfig: string | undefined; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openchamber-vscode-config-parse-')); + previousOpenCodeConfig = process.env.OPENCODE_CONFIG; + }); + + afterEach(() => { + if (previousOpenCodeConfig === undefined) delete process.env.OPENCODE_CONFIG; + else process.env.OPENCODE_CONFIG = previousOpenCodeConfig; + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + test('refuses MCP updates that would overwrite a partial-parse config', () => { + const configPath = path.join(tempDir, 'opencode.jsonc'); + fs.writeFileSync(configPath, PARTIAL_PARSE_CONFIG, 'utf8'); + process.env.OPENCODE_CONFIG = configPath; + + assert.throws(() => updateMcpConfig('openproject', { enabled: true }), isInvalidJsoncError); + assert.equal(fs.readFileSync(configPath, 'utf8'), PARTIAL_PARSE_CONFIG); + assert.equal(fs.existsSync(`${configPath}.openchamber.backup`), false); + }); + + test('preserves unrelated keys when updating a valid MCP config', () => { + const configPath = path.join(tempDir, 'opencode.jsonc'); + fs.writeFileSync(configPath, VALID_CONFIG, 'utf8'); + process.env.OPENCODE_CONFIG = configPath; + + updateMcpConfig('openproject', { enabled: false }); + + const rewritten = JSON.parse(fs.readFileSync(configPath, 'utf8')); + assert.deepEqual(rewritten.plugin, ['opencode-see-image']); + assert.equal(rewritten.provider['ollama-cloud'].name, 'Ollama Cloud'); + assert.equal(rewritten.mcp.openproject.enabled, false); + assert.equal(fs.readFileSync(`${configPath}.openchamber.backup`, 'utf8'), VALID_CONFIG); + }); + + test('returns an empty object for a comment-only config file', () => { + const configPath = path.join(tempDir, 'comments.jsonc'); + fs.writeFileSync(configPath, '// placeholder\n/* still empty */\n', 'utf8'); + process.env.OPENCODE_CONFIG = configPath; + + assert.deepEqual(listPluginEntries(), []); + }); + + test('refuses MCP updates against content that yields no JSON value at all', () => { + const configPath = path.join(tempDir, 'yamlish.jsonc'); + const contents = 'mcp:\n openproject:\n type: remote\n'; + fs.writeFileSync(configPath, contents, 'utf8'); + process.env.OPENCODE_CONFIG = configPath; + + assert.throws(() => updateMcpConfig('openproject', { enabled: true }), isInvalidJsoncError); + assert.equal(fs.readFileSync(configPath, 'utf8'), contents); + assert.equal(fs.existsSync(`${configPath}.openchamber.backup`), false); + }); + + test('lists custom-layer plugins when a project layer is unparseable', () => { + const customPath = path.join(tempDir, 'custom.jsonc'); + const projectDir = path.join(tempDir, 'project'); + const projectFile = path.join(projectDir, '.opencode', 'opencode.jsonc'); + fs.writeFileSync(customPath, VALID_CONFIG, 'utf8'); + fs.mkdirSync(path.dirname(projectFile), { recursive: true }); + fs.writeFileSync(projectFile, PARTIAL_PARSE_CONFIG, 'utf8'); + process.env.OPENCODE_CONFIG = customPath; + + const specs = listPluginEntries(projectDir).map((entry) => entry.spec); + assert.deepEqual(specs, ['opencode-see-image']); + assert.equal(fs.readFileSync(projectFile, 'utf8'), PARTIAL_PARSE_CONFIG); + assert.equal(fs.existsSync(`${projectFile}.openchamber.backup`), false); + }); + + test('keeps a valid custom layer writable when a project layer is unparseable', () => { + const customPath = path.join(tempDir, 'custom.jsonc'); + const projectDir = path.join(tempDir, 'project'); + const projectFile = path.join(projectDir, '.opencode', 'opencode.jsonc'); + fs.writeFileSync(customPath, VALID_CONFIG, 'utf8'); + fs.mkdirSync(path.dirname(projectFile), { recursive: true }); + fs.writeFileSync(projectFile, PARTIAL_PARSE_CONFIG, 'utf8'); + process.env.OPENCODE_CONFIG = customPath; + + updateMcpConfig('openproject', { enabled: false }, projectDir); + + const rewritten = JSON.parse(fs.readFileSync(customPath, 'utf8')); + assert.deepEqual(rewritten.plugin, ['opencode-see-image']); + assert.equal(rewritten.mcp.openproject.enabled, false); + assert.equal(fs.readFileSync(projectFile, 'utf8'), PARTIAL_PARSE_CONFIG); + assert.equal(fs.existsSync(`${projectFile}.openchamber.backup`), false); + }); +}); diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index a14ef031..78297fc6 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -2,7 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; import yaml from 'yaml'; -import { parse as parseJsonc } from 'jsonc-parser'; +import { parse as parseJsonc, printParseErrorCode, type ParseError } from 'jsonc-parser'; const OPENCODE_CONFIG_DIR = path.join(os.homedir(), '.config', 'opencode'); const AGENT_DIR = path.join(OPENCODE_CONFIG_DIR, 'agents'); @@ -554,12 +554,46 @@ const getPrimaryUserConfigPath = (userPaths: string[]): string => { return CONFIG_FILE; }; +const INVALID_JSONC = 'INVALID_JSONC'; + +const formatJsoncParseError = (filePath: string, errors: ParseError[]): string => { + const first = errors.length > 0 ? errors[0] : null; + const location = first && Number.isFinite(first.offset) + ? ` (${printParseErrorCode(first.error)} at offset ${first.offset})` + : ''; + return `OpenCode configuration at ${filePath} contains invalid JSONC and cannot be loaded safely${location}`; +}; + +const isInvalidJsoncError = (error: unknown): error is Error & { code: string } => + Boolean(error && typeof error === 'object' && 'code' in error && error.code === INVALID_JSONC); + +// Comment-only / whitespace-only files parse to undefined with nothing but +// ValueExpected. Any other error means real content we failed to understand +// (YAML, plain text, a stray leading token), which must not read as empty. +const isCommentOnlyParse = (parsed: unknown, errors: ParseError[]): boolean => + parsed === undefined + && errors.every((entry) => printParseErrorCode(entry.error) === 'ValueExpected'); + +const parseConfigObject = (content: string, filePath: string): Record => { + const errors: ParseError[] = []; + const parsed = parseJsonc(content, errors, { allowTrailingComma: true }); + if (isCommentOnlyParse(parsed, errors)) { + return {}; + } + if (errors.length > 0 || !parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw codedError(formatJsoncParseError(filePath, errors), INVALID_JSONC); + } + return parsed as Record; +}; + const readConfigFile = (filePath?: string | null): Record => { if (!filePath || !fs.existsSync(filePath)) return {}; const content = fs.readFileSync(filePath, 'utf8'); const normalized = content.trim(); if (!normalized) return {}; - return parseJsonc(normalized, [], { allowTrailingComma: true }) as Record; + // Refuse partial jsonc-parser trees. Ignoring errors previously let mutations + // rewrite a truncated object (often only `$schema`) over the full config. + return parseConfigObject(normalized, filePath); }; const isPlainObject = (value: unknown): value is Record => @@ -582,20 +616,50 @@ const mergeConfigs = (base: Record, override: Record; + error: (Error & { code: string }) | null; +} => { + try { + return { config: readConfigFile(filePath), error: null }; + } catch (error) { + if (isInvalidJsoncError(error)) { + console.error(error.message); + return { config: {}, error }; + } + throw error; + } +}; + const readConfigLayers = (workingDirectory?: string) => { const { userPaths, projectPath, customPath } = getConfigPaths(workingDirectory); const userPath = getPrimaryUserConfigPath(userPaths); - const userConfig = readConfigFile(userPath); - const projectConfig = readConfigFile(projectPath); - const customConfig = readConfigFile(customPath); - const mergedConfig = mergeConfigs(mergeConfigs(userConfig, projectConfig), customConfig); + const userLayer = readConfigLayer(userPath); + const projectLayer = readConfigLayer(projectPath); + const customLayer = readConfigLayer(customPath); + const mergedConfig = mergeConfigs( + mergeConfigs(userLayer.config, projectLayer.config), + customLayer.config, + ); + + const layerErrors: Array<{ path: string; code: string; message: string }> = []; + if (userLayer.error) { + layerErrors.push({ path: userPath, code: userLayer.error.code, message: userLayer.error.message }); + } + if (projectLayer.error && projectPath) { + layerErrors.push({ path: projectPath, code: projectLayer.error.code, message: projectLayer.error.message }); + } + if (customLayer.error && customPath) { + layerErrors.push({ path: customPath, code: customLayer.error.code, message: customLayer.error.message }); + } return { - userConfig, - projectConfig, - customConfig, + userConfig: userLayer.config, + projectConfig: projectLayer.config, + customConfig: customLayer.config, mergedConfig, - paths: { userPath, projectPath, customPath } + paths: { userPath, projectPath, customPath }, + layerErrors, }; }; @@ -695,6 +759,11 @@ const getConfigForPath = (layers: ReturnType, targetPat const writeConfig = (config: Record, filePath: string = CONFIG_FILE) => { if (fs.existsSync(filePath)) { + // Defense in depth: never overwrite a file we cannot fully parse. + const existing = fs.readFileSync(filePath, 'utf8').trim(); + if (existing) { + parseConfigObject(existing, filePath); + } const backupFile = `${filePath}.openchamber.backup`; try { fs.copyFileSync(filePath, backupFile); @@ -862,10 +931,10 @@ const getPluginConfigSources = (workingDirectory?: string | null): Array<{ scope const projectPath = getProjectConfigPath(workingDirectory || undefined); return [ customPath - ? { scope: 'user', path: customPath, config: readConfigFile(customPath) } - : { scope: 'user', path: userPath, config: readConfigFile(userPath) }, + ? { scope: 'user', path: customPath, config: readConfigLayer(customPath).config } + : { scope: 'user', path: userPath, config: readConfigLayer(userPath).config }, ...(projectPath - ? [{ scope: 'project' as const, path: projectPath, config: readConfigFile(projectPath) }] + ? [{ scope: 'project' as const, path: projectPath, config: readConfigLayer(projectPath).config }] : []), ]; }; @@ -1376,22 +1445,45 @@ export const deleteMcpConfig = (name: string, workingDirectory?: string): void = writeConfig(config, targetPath); }; +const getLayerError = ( + layers: ReturnType, + filePath?: string | null, +) => { + if (!filePath) return null; + return layers.layerErrors.find((entry) => entry.path === filePath) || null; +}; + +const throwIfLayerError = ( + layers: ReturnType, + filePath?: string | null, +) => { + const failed = getLayerError(layers, filePath); + if (!failed) return; + throw codedError(failed.message, failed.code); +}; + const getJsonEntrySource = ( layers: ReturnType, sectionKey: 'agent' | 'command' | 'mcp', entryName: string ) => { const { userConfig, projectConfig, customConfig, paths } = layers; - const customSection = (customConfig as Record)?.[sectionKey] as Record | undefined; - if (customSection?.[entryName] !== undefined) { - return { section: customSection[entryName], config: customConfig, path: paths.customPath, exists: true }; + if (paths.customPath) { + throwIfLayerError(layers, paths.customPath); + const customSection = (customConfig as Record)?.[sectionKey] as Record | undefined; + if (customSection?.[entryName] !== undefined) { + return { section: customSection[entryName], config: customConfig, path: paths.customPath, exists: true }; + } } - const projectSection = (projectConfig as Record)?.[sectionKey] as Record | undefined; - if (projectSection?.[entryName] !== undefined) { - return { section: projectSection[entryName], config: projectConfig, path: paths.projectPath, exists: true }; + if (paths.projectPath && !getLayerError(layers, paths.projectPath)) { + const projectSection = (projectConfig as Record)?.[sectionKey] as Record | undefined; + if (projectSection?.[entryName] !== undefined) { + return { section: projectSection[entryName], config: projectConfig, path: paths.projectPath, exists: true }; + } } + throwIfLayerError(layers, paths.userPath); const userSection = (userConfig as Record)?.[sectionKey] as Record | undefined; if (userSection?.[entryName] !== undefined) { return { section: userSection[entryName], config: userConfig, path: paths.userPath, exists: true }; @@ -1406,11 +1498,14 @@ const getJsonWriteTarget = ( ) => { const { userConfig, projectConfig, customConfig, paths } = layers; if (paths.customPath) { + throwIfLayerError(layers, paths.customPath); return { config: customConfig, path: paths.customPath }; } if (preferredScope === AGENT_SCOPE.PROJECT && paths.projectPath) { + throwIfLayerError(layers, paths.projectPath); return { config: projectConfig, path: paths.projectPath }; } + throwIfLayerError(layers, paths.userPath); return { config: userConfig, path: paths.userPath }; }; diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index 3b6c453c..de3afd8d 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -70,10 +70,12 @@ This module provides OpenCode server integration utilities for the web server ru - `AGENT_SCOPE`, `COMMAND_SCOPE`, `SKILL_SCOPE`: Scope constants with USER and PROJECT values. - `ensureDirs()`: Creates required OpenCode directories. - `parseMdFile(filePath)`, `writeMdFile(filePath, frontmatter, body)`: Markdown file operations with YAML frontmatter. -- `getConfigPaths(workingDirectory)`, `readConfigLayers(workingDirectory)`, `readConfig(workingDirectory)`: Config file operations with layer merging (user, project, custom). -- `writeConfig(config, filePath)`: Writes config with automatic backup. -- `getJsonEntrySource(layers, sectionKey, entryName)`: Resolves which config layer provides an entry. -- `getJsonWriteTarget(layers, preferredScope)`: Determines write target for config updates. +- `getConfigPaths(workingDirectory)`, `readConfigLayers(workingDirectory)`, `readConfig(workingDirectory)`: Config file operations with layer merging (user, project, custom). `readConfigLayers` isolates `INVALID_JSONC` per layer: a broken file is omitted from the merge (`{}` for that layer only), recorded on `layerErrors`, and does not block valid sibling layers. Writes still refuse to overwrite the broken file. +- `readConfigFile(filePath)`: Reads one config file. Missing, whitespace-only, and comment-only files return `{}`; a comment-only file is recognized by `ValueExpected` being the only parse error. A `jsonc-parser` error that produces a partial or non-object tree throws `INVALID_JSONC` — partial parse trees must never be treated as authoritative (avoids rewriting a `$schema`-only stub over a full config). Content that yields no JSON value for any other reason (YAML, plain text) also throws instead of reading as empty. +- `readConfigLayer(filePath)`: Same parse as `readConfigFile`, but isolates `INVALID_JSONC` to `{ config: {}, error }` so plugin/MCP/agent readers can skip one broken layer without aborting valid siblings. Writes still refuse to overwrite the broken file. +- `writeConfig(config, filePath)`: Writes config with automatic backup. Refuses to overwrite an existing non-empty file that fails the same JSONC parse check. +- `getJsonEntrySource(layers, sectionKey, entryName)`: Resolves which config layer provides an entry. A failed custom or user layer throws `INVALID_JSONC` instead of treating that file as empty. A failed project layer is skipped so a valid user/custom entry can still be found. +- `getJsonWriteTarget(layers, preferredScope)`: Determines write target for config updates. Throws `INVALID_JSONC` when the chosen target file is the unparseable layer. - `getAncestors(startDir, stopDir)`, `findWorktreeRoot(startDir)`: Git worktree helpers. - `isPromptFileReference(value)`, `resolvePromptFilePath(reference)`, `writePromptFile(filePath, content)`: Prompt file reference handling. - `walkSkillMdFiles(rootDir)`: Recursively finds all SKILL.md files. diff --git a/packages/web/server/lib/opencode/plugins.js b/packages/web/server/lib/opencode/plugins.js index 3eebfce9..143298fd 100644 --- a/packages/web/server/lib/opencode/plugins.js +++ b/packages/web/server/lib/opencode/plugins.js @@ -4,6 +4,7 @@ import path from 'path'; import { AGENT_SCOPE, readConfigFile, + readConfigLayer, writeConfig, } from './shared.js'; import { isPathSpec } from './plugin-spec.js'; @@ -111,15 +112,23 @@ function readPluginConfigLayers(workingDirectory) { const customPath = getActiveCustomConfigPath(); const userPath = getPrimaryUserConfigPath(); const projectPath = getProjectConfigPath(workingDirectory); + const userLayer = readConfigLayer(userPath); + const projectLayer = readConfigLayer(projectPath); + const customLayer = readConfigLayer(customPath); return { - userConfig: readConfigFile(userPath), - projectConfig: readConfigFile(projectPath), - customConfig: readConfigFile(customPath), + userConfig: userLayer.config, + projectConfig: projectLayer.config, + customConfig: customLayer.config, paths: { userPath, projectPath, customPath, }, + layerErrors: [ + userLayer.error && { path: userPath, code: userLayer.error.code, message: userLayer.error.message }, + projectLayer.error && projectPath && { path: projectPath, code: projectLayer.error.code, message: projectLayer.error.message }, + customLayer.error && customPath && { path: customPath, code: customLayer.error.code, message: customLayer.error.message }, + ].filter(Boolean), }; } diff --git a/packages/web/server/lib/opencode/plugins.test.js b/packages/web/server/lib/opencode/plugins.test.js index 1e624445..974b788a 100644 --- a/packages/web/server/lib/opencode/plugins.test.js +++ b/packages/web/server/lib/opencode/plugins.test.js @@ -121,6 +121,24 @@ describe('opencode plugins data layer', () => { expect(readJson(userConfigPath)).toEqual({}); }); + test('lists user plugins when a project layer is unparseable', () => { + const partialProject = [ + '{', + ' "$schema": "https://opencode.ai/config.json",', + ' plugin: ["broken-project-plugin"],', + '}', + '', + ].join('\n'); + writeJson(userConfigPath, { plugin: ['user-plugin'] }); + const projectFile = path.join(projectDir, '.opencode', 'opencode.jsonc'); + fs.mkdirSync(path.dirname(projectFile), { recursive: true }); + fs.writeFileSync(projectFile, partialProject, 'utf8'); + + expect(plugins.listPluginEntries(projectDir).map((entry) => entry.spec)).toEqual(['user-plugin']); + expect(fs.readFileSync(projectFile, 'utf8')).toBe(partialProject); + expect(fs.existsSync(`${projectFile}.openchamber.backup`)).toBe(false); + }); + test('lists entries from user and project layers with scopes and parsed kinds', () => { writeJson(userConfigPath, { plugin: ['npm-plugin', '/abs/plugin.js', '@scope/pkg@1.0.0'] }); writeJson(path.join(projectDir, '.opencode', 'opencode.json'), { plugin: ['./local-plugin.js'] }); diff --git a/packages/web/server/lib/opencode/shared.js b/packages/web/server/lib/opencode/shared.js index 062d3d0f..d01168f3 100644 --- a/packages/web/server/lib/opencode/shared.js +++ b/packages/web/server/lib/opencode/shared.js @@ -2,7 +2,7 @@ import fs from 'fs'; import path from 'path'; import os from 'os'; import yaml from 'yaml'; -import { parse as parseJsonc } from 'jsonc-parser'; +import { parse as parseJsonc, printParseErrorCode } from 'jsonc-parser'; // ============== PATH CONSTANTS ============== @@ -168,6 +168,42 @@ function getPrimaryUserConfigPath(userPaths) { return CONFIG_FILE; } +const INVALID_JSONC = 'INVALID_JSONC'; + +function isInvalidJsoncError(error) { + return Boolean(error && typeof error === 'object' && error.code === INVALID_JSONC); +} + +function formatJsoncParseError(filePath, errors) { + const first = Array.isArray(errors) && errors.length > 0 ? errors[0] : null; + const location = first && Number.isFinite(first.offset) + ? ` (${printParseErrorCode(first.error)} at offset ${first.offset})` + : ''; + return `OpenCode configuration at ${filePath} contains invalid JSONC and cannot be loaded safely${location}`; +} + +function isCommentOnlyParse(parsed, errors) { + // Comment-only / whitespace-only files parse to undefined with nothing but + // ValueExpected. Any other error means real content we failed to understand + // (YAML, plain text, a stray leading token), which must not read as empty. + return parsed === undefined + && errors.every((entry) => printParseErrorCode(entry.error) === 'ValueExpected'); +} + +function parseConfigObject(content, filePath) { + const errors = []; + const parsed = parseJsonc(content, errors, { allowTrailingComma: true }); + if (isCommentOnlyParse(parsed, errors)) { + return {}; + } + if (errors.length > 0 || !parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + const error = new Error(formatJsoncParseError(filePath, errors)); + error.code = INVALID_JSONC; + throw error; + } + return parsed; +} + function readConfigFile(filePath) { if (!filePath || !fs.existsSync(filePath)) { return {}; @@ -178,8 +214,13 @@ function readConfigFile(filePath) { if (!normalized) { return {}; } - return parseJsonc(normalized, [], { allowTrailingComma: true }); + // Refuse partial jsonc-parser trees. Ignoring errors previously let mutations + // rewrite a truncated object (often only `$schema`) over the full config. + return parseConfigObject(normalized, filePath); } catch (error) { + if (isInvalidJsoncError(error)) { + throw error; + } console.error(`Failed to read config file: ${filePath}`, error); throw new Error('Failed to read OpenCode configuration'); } @@ -209,20 +250,47 @@ function mergeConfigs(base, override) { return result; } +function readConfigLayer(filePath) { + try { + return { config: readConfigFile(filePath), error: null }; + } catch (error) { + if (isInvalidJsoncError(error)) { + console.error(error.message); + return { config: {}, error }; + } + throw error; + } +} + function readConfigLayers(workingDirectory) { const { userPaths, projectPath, customPath } = getConfigPaths(workingDirectory); const userPath = getPrimaryUserConfigPath(userPaths); - const userConfig = readConfigFile(userPath); - const projectConfig = readConfigFile(projectPath); - const customConfig = readConfigFile(customPath); - const mergedConfig = mergeConfigs(mergeConfigs(userConfig, projectConfig), customConfig); + const userLayer = readConfigLayer(userPath); + const projectLayer = readConfigLayer(projectPath); + const customLayer = readConfigLayer(customPath); + const mergedConfig = mergeConfigs( + mergeConfigs(userLayer.config, projectLayer.config), + customLayer.config, + ); + + const layerErrors = []; + if (userLayer.error) { + layerErrors.push({ path: userPath, code: userLayer.error.code, message: userLayer.error.message }); + } + if (projectLayer.error && projectPath) { + layerErrors.push({ path: projectPath, code: projectLayer.error.code, message: projectLayer.error.message }); + } + if (customLayer.error && customPath) { + layerErrors.push({ path: customPath, code: customLayer.error.code, message: customLayer.error.message }); + } return { - userConfig, - projectConfig, - customConfig, + userConfig: userLayer.config, + projectConfig: projectLayer.config, + customConfig: customLayer.config, mergedConfig, - paths: { userPath, projectPath, customPath } + paths: { userPath, projectPath, customPath }, + layerErrors, }; } @@ -246,6 +314,12 @@ function getConfigForPath(layers, targetPath) { function writeConfig(config, filePath = CONFIG_FILE) { try { if (fs.existsSync(filePath)) { + // Defense in depth: never overwrite a file we cannot fully parse. + const existing = fs.readFileSync(filePath, 'utf8').trim(); + if (existing) { + parseConfigObject(existing, filePath); + } + const backupFile = `${filePath}.openchamber.backup`; fs.copyFileSync(filePath, backupFile); console.log(`Created config backup: ${backupFile}`); @@ -255,23 +329,49 @@ function writeConfig(config, filePath = CONFIG_FILE) { fs.writeFileSync(filePath, JSON.stringify(config, null, 2), 'utf8'); console.log(`Successfully wrote config file: ${filePath}`); } catch (error) { + if (isInvalidJsoncError(error)) { + throw error; + } console.error(`Failed to write config file: ${filePath}`, error); throw new Error('Failed to write OpenCode configuration'); } } +function getLayerError(layers, filePath) { + if (!filePath || !Array.isArray(layers?.layerErrors)) { + return null; + } + return layers.layerErrors.find((entry) => entry.path === filePath) || null; +} + +function throwIfLayerError(layers, filePath) { + const failed = getLayerError(layers, filePath); + if (!failed) { + return; + } + const error = new Error(failed.message); + error.code = failed.code; + throw error; +} + function getJsonEntrySource(layers, sectionKey, entryName) { const { userConfig, projectConfig, customConfig, paths } = layers; - const customSection = customConfig?.[sectionKey]?.[entryName]; - if (customSection !== undefined) { - return { section: customSection, config: customConfig, path: paths.customPath, exists: true }; + if (paths.customPath) { + throwIfLayerError(layers, paths.customPath); + const customSection = customConfig?.[sectionKey]?.[entryName]; + if (customSection !== undefined) { + return { section: customSection, config: customConfig, path: paths.customPath, exists: true }; + } } - const projectSection = projectConfig?.[sectionKey]?.[entryName]; - if (projectSection !== undefined) { - return { section: projectSection, config: projectConfig, path: paths.projectPath, exists: true }; + if (paths.projectPath && !getLayerError(layers, paths.projectPath)) { + const projectSection = projectConfig?.[sectionKey]?.[entryName]; + if (projectSection !== undefined) { + return { section: projectSection, config: projectConfig, path: paths.projectPath, exists: true }; + } } + throwIfLayerError(layers, paths.userPath); const userSection = userConfig?.[sectionKey]?.[entryName]; if (userSection !== undefined) { return { section: userSection, config: userConfig, path: paths.userPath, exists: true }; @@ -283,11 +383,14 @@ function getJsonEntrySource(layers, sectionKey, entryName) { function getJsonWriteTarget(layers, preferredScope) { const { userConfig, projectConfig, customConfig, paths } = layers; if (paths.customPath) { + throwIfLayerError(layers, paths.customPath); return { config: customConfig, path: paths.customPath }; } if (preferredScope === AGENT_SCOPE.PROJECT && paths.projectPath) { + throwIfLayerError(layers, paths.projectPath); return { config: projectConfig, path: paths.projectPath }; } + throwIfLayerError(layers, paths.userPath); return { config: userConfig, path: paths.userPath }; } @@ -547,6 +650,7 @@ export { parseMdFile, writeMdFile, readConfigFile, + readConfigLayer, isPlainObject, readConfigLayers, readConfig, diff --git a/packages/web/server/lib/opencode/shared.test.js b/packages/web/server/lib/opencode/shared.test.js index e131aa0c..2c644b19 100644 --- a/packages/web/server/lib/opencode/shared.test.js +++ b/packages/web/server/lib/opencode/shared.test.js @@ -3,8 +3,9 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; -import { parseMdFile, writeMdFile } from './shared.js'; +import { parseMdFile, writeMdFile, readConfigFile, readConfigLayers, writeConfig } from './shared.js'; import { updateAgent } from './agents.js'; +import { updateMcpConfig } from './mcp.js'; const FIXTURE_DIR = path.join(os.tmpdir(), `openchamber-shared-test-${process.pid}`); @@ -200,3 +201,186 @@ describe('updateAgent frontmatter preservation', () => { expect(parsed.body).toBe('Body of strateg.'); }); }); + +describe('readConfigFile / writeConfig JSONC safety (issue #2923)', () => { + beforeEach(() => { + fs.rmSync(FIXTURE_DIR, { recursive: true, force: true }); + fs.mkdirSync(FIXTURE_DIR, { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(FIXTURE_DIR, { recursive: true, force: true }); + }); + + const VALID_CONFIG = [ + '{', + ' "$schema": "https://opencode.ai/config.json",', + ' // keep me', + ' "plugin": ["opencode-see-image"],', + ' "mcp": {', + ' "openproject": {', + ' "type": "remote",', + ' "url": "https://openproject.example.com/mcp",', + ' "enabled": true,', + ' }', + ' },', + ' "provider": {', + ' "ollama-cloud": {', + ' "npm": "@ai-sdk/openai-compatible",', + ' "name": "Ollama Cloud"', + ' }', + ' }', + '}', + '', + ].join('\n'); + + // JSON5-style unquoted keys after $schema — jsonc-parser returns a partial + // tree of only `{ $schema }` when errors are ignored. + const PARTIAL_PARSE_CONFIG = [ + '{', + ' "$schema": "https://opencode.ai/config.json",', + ' plugin: ["opencode-see-image"],', + ' mcp: {', + ' openproject: {', + ' type: "remote",', + ' url: "https://openproject.example.com/mcp",', + ' enabled: true', + ' }', + ' },', + ' provider: {', + ' "ollama-cloud": {', + ' npm: "@ai-sdk/openai-compatible",', + ' name: "Ollama Cloud"', + ' }', + ' }', + '}', + '', + ].join('\n'); + + it('parses valid JSONC with comments and trailing commas without dropping keys', () => { + const file = writeFixture('opencode.jsonc', VALID_CONFIG); + expect(readConfigFile(file)).toEqual({ + $schema: 'https://opencode.ai/config.json', + plugin: ['opencode-see-image'], + mcp: { + openproject: { + type: 'remote', + url: 'https://openproject.example.com/mcp', + enabled: true, + }, + }, + provider: { + 'ollama-cloud': { + npm: '@ai-sdk/openai-compatible', + name: 'Ollama Cloud', + }, + }, + }); + }); + + it('returns an empty object for a missing or whitespace-only file', () => { + expect(readConfigFile(path.join(FIXTURE_DIR, 'missing.jsonc'))).toEqual({}); + const empty = writeFixture('empty.jsonc', ' \n'); + expect(readConfigFile(empty)).toEqual({}); + }); + + it('throws INVALID_JSONC on partial-parse JSONC instead of returning a $schema-only stub', () => { + const file = writeFixture('opencode.jsonc', PARTIAL_PARSE_CONFIG); + expect(() => readConfigFile(file)).toThrow(/cannot be loaded safely/); + try { + readConfigFile(file); + } catch (error) { + expect(error.code).toBe('INVALID_JSONC'); + } + }); + + it('throws INVALID_JSONC for a non-object JSONC root', () => { + const file = writeFixture('array.jsonc', '["plugin"]\n'); + expect(() => readConfigFile(file)).toThrow(/cannot be loaded safely/); + }); + + it('refuses to overwrite an unparseable config file', () => { + const file = writeFixture('opencode.jsonc', PARTIAL_PARSE_CONFIG); + expect(() => writeConfig({ $schema: 'https://opencode.ai/config.json' }, file)).toThrow( + /cannot be loaded safely/, + ); + expect(fs.readFileSync(file, 'utf8')).toBe(PARTIAL_PARSE_CONFIG); + expect(fs.existsSync(`${file}.openchamber.backup`)).toBe(false); + }); + + it('preserves a valid config across MCP updates', () => { + const file = writeFixture('opencode.jsonc', VALID_CONFIG); + const config = readConfigFile(file); + config.mcp.openproject.enabled = false; + writeConfig(config, file); + + const rewritten = JSON.parse(fs.readFileSync(file, 'utf8')); + expect(rewritten.plugin).toEqual(['opencode-see-image']); + expect(rewritten.provider['ollama-cloud'].name).toBe('Ollama Cloud'); + expect(rewritten.mcp.openproject.enabled).toBe(false); + expect(fs.readFileSync(`${file}.openchamber.backup`, 'utf8')).toBe(VALID_CONFIG); + }); + + it('does not wipe an unparseable user config during MCP mutation attempts', () => { + const file = writeFixture('opencode.jsonc', PARTIAL_PARSE_CONFIG); + const previousOpenCodeConfig = process.env.OPENCODE_CONFIG; + + try { + process.env.OPENCODE_CONFIG = file; + expect(() => updateMcpConfig('openproject', { enabled: true })).toThrow( + /cannot be loaded safely/, + ); + expect(fs.readFileSync(file, 'utf8')).toBe(PARTIAL_PARSE_CONFIG); + expect(fs.existsSync(`${file}.openchamber.backup`)).toBe(false); + } finally { + if (previousOpenCodeConfig === undefined) delete process.env.OPENCODE_CONFIG; + else process.env.OPENCODE_CONFIG = previousOpenCodeConfig; + } + }); + + it('returns an empty object for a comment-only config file', () => { + const file = writeFixture('comments.jsonc', '// placeholder\n/* still empty */\n'); + expect(readConfigFile(file)).toEqual({}); + }); + + it('throws INVALID_JSONC for content that yields no JSON value at all', () => { + const yamlish = writeFixture('yamlish.jsonc', 'mcp:\n openproject:\n type: remote\n'); + expect(() => readConfigFile(yamlish)).toThrow(/cannot be loaded safely/); + expect(() => writeConfig({ $schema: 'https://opencode.ai/config.json' }, yamlish)).toThrow( + /cannot be loaded safely/, + ); + expect(fs.readFileSync(yamlish, 'utf8')).toBe('mcp:\n openproject:\n type: remote\n'); + expect(fs.existsSync(`${yamlish}.openchamber.backup`)).toBe(false); + }); + + it('keeps a valid custom layer readable when a project layer is unparseable', () => { + const custom = writeFixture('custom.jsonc', VALID_CONFIG); + const projectDir = path.join(FIXTURE_DIR, 'project'); + const projectFile = writeFixture(path.join('project', '.opencode', 'opencode.jsonc'), PARTIAL_PARSE_CONFIG); + const previousOpenCodeConfig = process.env.OPENCODE_CONFIG; + + try { + process.env.OPENCODE_CONFIG = custom; + const layers = readConfigLayers(projectDir); + expect(layers.customConfig.plugin).toEqual(['opencode-see-image']); + expect(layers.projectConfig).toEqual({}); + expect(layers.mergedConfig.plugin).toEqual(['opencode-see-image']); + expect(layers.layerErrors).toEqual([ + expect.objectContaining({ + path: projectFile, + code: 'INVALID_JSONC', + }), + ]); + + updateMcpConfig('openproject', { enabled: false }, projectDir); + const rewritten = JSON.parse(fs.readFileSync(custom, 'utf8')); + expect(rewritten.plugin).toEqual(['opencode-see-image']); + expect(rewritten.mcp.openproject.enabled).toBe(false); + expect(fs.readFileSync(projectFile, 'utf8')).toBe(PARTIAL_PARSE_CONFIG); + expect(fs.existsSync(`${projectFile}.openchamber.backup`)).toBe(false); + } finally { + if (previousOpenCodeConfig === undefined) delete process.env.OPENCODE_CONFIG; + else process.env.OPENCODE_CONFIG = previousOpenCodeConfig; + } + }); +});