From 4cc090130cae6e1eda1f7d6fda21739d80bd901c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 04:25:40 +0000 Subject: [PATCH 1/4] fix(config): refuse partial JSONC parses that wipe opencode.jsonc jsonc-parser was returning truncated trees (often only `$schema`) when configs contained JSON5-style unquoted keys. Config mutations then backed up and overwrote the full file with that stub. Check parse errors on read and refuse to overwrite unparseable files on write in web and VS Code. Fixes #2923 Co-authored-by: serkraser --- CHANGELOG.md | 1 + packages/vscode/CHANGELOG.md | 4 + packages/vscode/src/DOCUMENTATION.md | 1 + .../src/opencodeConfig.config-parse.test.ts | 96 ++++++++++++ packages/vscode/src/opencodeConfig.ts | 30 +++- .../web/server/lib/opencode/DOCUMENTATION.md | 3 +- packages/web/server/lib/opencode/shared.js | 43 +++++- .../web/server/lib/opencode/shared.test.js | 140 +++++++++++++++++- 8 files changed, 312 insertions(+), 6 deletions(-) create mode 100644 packages/vscode/src/opencodeConfig.config-parse.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a980e698..ddc38b08 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). - **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. - **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: 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/CHANGELOG.md b/packages/vscode/CHANGELOG.md index 8132a578..18c11df8 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -1,3 +1,7 @@ +## [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). + ## [1.18.4] - 2026-08-14 - **Chat:** new messages now remain at the end of the conversation instead of jumping before older messages after the message ID sequence rolls over; history loading, revert, and redo follow the same chronological order. diff --git a/packages/vscode/src/DOCUMENTATION.md b/packages/vscode/src/DOCUMENTATION.md index 3a4d3dde..e7d6e610 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 any `jsonc-parser` error or non-object result (`INVALID_JSONC`) so mutations cannot rewrite a partial `$schema`-only stub over an existing config. - `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..bd6f08b2 --- /dev/null +++ b/packages/vscode/src/opencodeConfig.config-parse.test.ts @@ -0,0 +1,96 @@ +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 { 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'); + +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 }), + (error: unknown) => ( + error instanceof Error + && /cannot be loaded safely/.test(error.message) + && (error as Error & { code?: string }).code === 'INVALID_JSONC' + ), + ); + 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); + }); +}); diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index a14ef031..199f348e 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,33 @@ 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 parseConfigObject = (content: string, filePath: string): Record => { + const errors: ParseError[] = []; + const parsed = parseJsonc(content, errors, { allowTrailingComma: true }); + 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 => @@ -695,6 +716,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); diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index 3b6c453c..cbe9f6c8 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -71,7 +71,8 @@ This module provides OpenCode server integration utilities for the web server ru - `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. +- `readConfigFile(filePath)`: Reads one config file. Empty/missing files return `{}`. Any `jsonc-parser` error or non-object result throws `INVALID_JSONC` — partial parse trees must never be treated as authoritative (avoids rewriting a `$schema`-only stub over a full config). +- `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. - `getJsonWriteTarget(layers, preferredScope)`: Determines write target for config updates. - `getAncestors(startDir, stopDir)`, `findWorktreeRoot(startDir)`: Git worktree helpers. diff --git a/packages/web/server/lib/opencode/shared.js b/packages/web/server/lib/opencode/shared.js index 062d3d0f..f99360d8 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,31 @@ 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 parseConfigObject(content, filePath) { + const errors = []; + const parsed = parseJsonc(content, errors, { allowTrailingComma: true }); + 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 +203,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'); } @@ -246,6 +276,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,6 +291,9 @@ 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'); } diff --git a/packages/web/server/lib/opencode/shared.test.js b/packages/web/server/lib/opencode/shared.test.js index e131aa0c..a3e646b7 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, 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,140 @@ 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; + } + }); +}); From 35563dd78d5310deb0b2711064d3bd11b9c1c42a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 05:13:32 +0000 Subject: [PATCH 2/4] fix(config): isolate broken JSONC layers and treat comment-only as empty Address review findings on the #2923 fail-closed parse fix: - Comment-only files produce ValueExpected with no JSON value; treat that as empty config instead of INVALID_JSONC. Partial object trees still throw. - readConfigLayers no longer lets one unparseable layer abort valid sibling layers. Mutations still fail closed on the custom/user write target. Co-authored-by: serkraser --- packages/vscode/src/DOCUMENTATION.md | 2 +- .../src/opencodeConfig.config-parse.test.ts | 18 ++++ packages/vscode/src/opencodeConfig.ts | 92 ++++++++++++++++--- .../web/server/lib/opencode/DOCUMENTATION.md | 8 +- packages/web/server/lib/opencode/shared.js | 86 ++++++++++++++--- .../web/server/lib/opencode/shared.test.js | 38 +++++++- 6 files changed, 210 insertions(+), 34 deletions(-) diff --git a/packages/vscode/src/DOCUMENTATION.md b/packages/vscode/src/DOCUMENTATION.md index e7d6e610..fe638f97 100644 --- a/packages/vscode/src/DOCUMENTATION.md +++ b/packages/vscode/src/DOCUMENTATION.md @@ -58,7 +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 any `jsonc-parser` error or non-object result (`INVALID_JSONC`) so mutations cannot rewrite a partial `$schema`-only stub over an existing config. + - 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. A broken layer is omitted from the merge and recorded on `layerErrors`; valid sibling layers still load, and 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 index bd6f08b2..c12572e8 100644 --- a/packages/vscode/src/opencodeConfig.config-parse.test.ts +++ b/packages/vscode/src/opencodeConfig.config-parse.test.ts @@ -93,4 +93,22 @@ describe('opencodeConfig JSONC parse safety (issue #2923)', () => { assert.equal(rewritten.mcp.openproject.enabled, false); assert.equal(fs.readFileSync(`${configPath}.openchamber.backup`, 'utf8'), VALID_CONFIG); }); + + 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 199f348e..50a40082 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -564,9 +564,17 @@ const formatJsoncParseError = (filePath: string, errors: ParseError[]): string = 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); + const parseConfigObject = (content: string, filePath: string): Record => { const errors: ParseError[] = []; const parsed = parseJsonc(content, errors, { allowTrailingComma: true }); + // Comment-only / no JSON value: jsonc-parser returns undefined plus ValueExpected. + // That is empty config, not a partial tree. The data-loss bug is errors + object. + if (parsed === undefined) { + return {}; + } if (errors.length > 0 || !parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { throw codedError(formatJsoncParseError(filePath, errors), INVALID_JSONC); } @@ -603,20 +611,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, }; }; @@ -1402,22 +1440,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 }; @@ -1432,11 +1493,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 cbe9f6c8..cb35ed11 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -70,11 +70,11 @@ 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). -- `readConfigFile(filePath)`: Reads one config file. Empty/missing files return `{}`. Any `jsonc-parser` error or non-object result throws `INVALID_JSONC` — partial parse trees must never be treated as authoritative (avoids rewriting a `$schema`-only stub over a full config). +- `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 (no JSON value) return `{}`. 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). - `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. -- `getJsonWriteTarget(layers, preferredScope)`: Determines write target for config updates. +- `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/shared.js b/packages/web/server/lib/opencode/shared.js index f99360d8..87a105d4 100644 --- a/packages/web/server/lib/opencode/shared.js +++ b/packages/web/server/lib/opencode/shared.js @@ -185,6 +185,11 @@ function formatJsoncParseError(filePath, errors) { function parseConfigObject(content, filePath) { const errors = []; const parsed = parseJsonc(content, errors, { allowTrailingComma: true }); + // Comment-only / no JSON value: jsonc-parser returns undefined plus ValueExpected. + // That is empty config, not a partial tree. The data-loss bug is errors + object. + if (parsed === undefined) { + return {}; + } if (errors.length > 0 || !parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { const error = new Error(formatJsoncParseError(filePath, errors)); error.code = INVALID_JSONC; @@ -239,20 +244,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, }; } @@ -299,18 +331,41 @@ function writeConfig(config, filePath = CONFIG_FILE) { } } +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 }; @@ -322,11 +377,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 }; } diff --git a/packages/web/server/lib/opencode/shared.test.js b/packages/web/server/lib/opencode/shared.test.js index a3e646b7..88384ae9 100644 --- a/packages/web/server/lib/opencode/shared.test.js +++ b/packages/web/server/lib/opencode/shared.test.js @@ -3,7 +3,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; -import { parseMdFile, writeMdFile, readConfigFile, writeConfig } from './shared.js'; +import { parseMdFile, writeMdFile, readConfigFile, readConfigLayers, writeConfig } from './shared.js'; import { updateAgent } from './agents.js'; import { updateMcpConfig } from './mcp.js'; @@ -337,4 +337,40 @@ describe('readConfigFile / writeConfig JSONC safety (issue #2923)', () => { 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('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; + } + }); }); From 6751c7dc7ad9c4e95272cd6495e37f314297b793 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 06:30:36 +0000 Subject: [PATCH 3/4] fix(config): isolate plugin list reads from a broken JSONC layer Plugin listing still called readConfigFile per layer, so one unparseable project file made GET /api/config/plugins and VS Code listPluginEntries fail. Reuse readConfigLayer isolation and pin comment-only empty parse in the VS Code suite. Co-authored-by: serkraser --- packages/vscode/src/DOCUMENTATION.md | 2 +- .../src/opencodeConfig.config-parse.test.ts | 25 ++++++++++++++++++- packages/vscode/src/opencodeConfig.ts | 6 ++--- .../web/server/lib/opencode/DOCUMENTATION.md | 1 + packages/web/server/lib/opencode/plugins.js | 15 ++++++++--- .../web/server/lib/opencode/plugins.test.js | 18 +++++++++++++ packages/web/server/lib/opencode/shared.js | 1 + 7 files changed, 60 insertions(+), 8 deletions(-) diff --git a/packages/vscode/src/DOCUMENTATION.md b/packages/vscode/src/DOCUMENTATION.md index fe638f97..f6345cae 100644 --- a/packages/vscode/src/DOCUMENTATION.md +++ b/packages/vscode/src/DOCUMENTATION.md @@ -58,7 +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. A broken layer is omitted from the merge and recorded on `layerErrors`; valid sibling layers still load, and writes still refuse to overwrite the broken file. + - 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. 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 index c12572e8..7c696696 100644 --- a/packages/vscode/src/opencodeConfig.config-parse.test.ts +++ b/packages/vscode/src/opencodeConfig.config-parse.test.ts @@ -4,7 +4,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { updateMcpConfig } from './opencodeConfig'; +import { listPluginEntries, updateMcpConfig } from './opencodeConfig'; const PARTIAL_PARSE_CONFIG = [ '{', @@ -94,6 +94,29 @@ describe('opencodeConfig JSONC parse safety (issue #2923)', () => { 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('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'); diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index 50a40082..66142c36 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -926,10 +926,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 }] : []), ]; }; diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index cb35ed11..e783bb36 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -72,6 +72,7 @@ This module provides OpenCode server integration utilities for the web server ru - `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). `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 (no JSON value) return `{}`. 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). +- `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. 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 87a105d4..bc90ff31 100644 --- a/packages/web/server/lib/opencode/shared.js +++ b/packages/web/server/lib/opencode/shared.js @@ -644,6 +644,7 @@ export { parseMdFile, writeMdFile, readConfigFile, + readConfigLayer, isPlainObject, readConfigLayers, readConfig, From 6d6ece685627b3e6ded9439be5ac88c33b3eb5dd Mon Sep 17 00:00:00 2001 From: Serhii Dziupin Date: Mon, 17 Aug 2026 16:29:12 +0300 Subject: [PATCH 4/4] fix(config): fail closed when config content yields no JSON value Treating every undefined parse as empty config let a file that is not JSON at all (YAML, plain text) read as {}, so a later write would back it up and replace it - the same data loss this fix is meant to prevent. Only a comment-only parse, where ValueExpected is the sole error, counts as empty. --- packages/vscode/src/DOCUMENTATION.md | 2 +- .../src/opencodeConfig.config-parse.test.ts | 28 +++++++++++++------ packages/vscode/src/opencodeConfig.ts | 11 ++++++-- .../web/server/lib/opencode/DOCUMENTATION.md | 2 +- packages/web/server/lib/opencode/shared.js | 12 ++++++-- .../web/server/lib/opencode/shared.test.js | 10 +++++++ 6 files changed, 49 insertions(+), 16 deletions(-) diff --git a/packages/vscode/src/DOCUMENTATION.md b/packages/vscode/src/DOCUMENTATION.md index f6345cae..e3447e21 100644 --- a/packages/vscode/src/DOCUMENTATION.md +++ b/packages/vscode/src/DOCUMENTATION.md @@ -58,7 +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. 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. + - 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 index 7c696696..74d6d737 100644 --- a/packages/vscode/src/opencodeConfig.config-parse.test.ts +++ b/packages/vscode/src/opencodeConfig.config-parse.test.ts @@ -48,6 +48,14 @@ const VALID_CONFIG = [ '', ].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; @@ -68,14 +76,7 @@ describe('opencodeConfig JSONC parse safety (issue #2923)', () => { fs.writeFileSync(configPath, PARTIAL_PARSE_CONFIG, 'utf8'); process.env.OPENCODE_CONFIG = configPath; - assert.throws( - () => updateMcpConfig('openproject', { enabled: true }), - (error: unknown) => ( - error instanceof Error - && /cannot be loaded safely/.test(error.message) - && (error as Error & { code?: string }).code === 'INVALID_JSONC' - ), - ); + assert.throws(() => updateMcpConfig('openproject', { enabled: true }), isInvalidJsoncError); assert.equal(fs.readFileSync(configPath, 'utf8'), PARTIAL_PARSE_CONFIG); assert.equal(fs.existsSync(`${configPath}.openchamber.backup`), false); }); @@ -102,6 +103,17 @@ describe('opencodeConfig JSONC parse safety (issue #2923)', () => { 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'); diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index 66142c36..78297fc6 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -567,12 +567,17 @@ const formatJsoncParseError = (filePath: string, errors: ParseError[]): string = 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 }); - // Comment-only / no JSON value: jsonc-parser returns undefined plus ValueExpected. - // That is empty config, not a partial tree. The data-loss bug is errors + object. - if (parsed === undefined) { + if (isCommentOnlyParse(parsed, errors)) { return {}; } if (errors.length > 0 || !parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index e783bb36..de3afd8d 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -71,7 +71,7 @@ This module provides OpenCode server integration utilities for the web server ru - `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). `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 (no JSON value) return `{}`. 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). +- `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. diff --git a/packages/web/server/lib/opencode/shared.js b/packages/web/server/lib/opencode/shared.js index bc90ff31..d01168f3 100644 --- a/packages/web/server/lib/opencode/shared.js +++ b/packages/web/server/lib/opencode/shared.js @@ -182,12 +182,18 @@ function formatJsoncParseError(filePath, errors) { 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 }); - // Comment-only / no JSON value: jsonc-parser returns undefined plus ValueExpected. - // That is empty config, not a partial tree. The data-loss bug is errors + object. - if (parsed === undefined) { + if (isCommentOnlyParse(parsed, errors)) { return {}; } if (errors.length > 0 || !parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { diff --git a/packages/web/server/lib/opencode/shared.test.js b/packages/web/server/lib/opencode/shared.test.js index 88384ae9..2c644b19 100644 --- a/packages/web/server/lib/opencode/shared.test.js +++ b/packages/web/server/lib/opencode/shared.test.js @@ -343,6 +343,16 @@ describe('readConfigFile / writeConfig JSONC safety (issue #2923)', () => { 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');