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');