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 <serkraser@gmail.com>
This commit is contained in:
co-authored by
serkraser
parent
6b1e677aaf
commit
4cc090130c
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<string, unknown> => {
|
||||
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<string, unknown>;
|
||||
};
|
||||
|
||||
const readConfigFile = (filePath?: string | null): Record<string, unknown> => {
|
||||
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<string, unknown>;
|
||||
// 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<string, unknown> =>
|
||||
@@ -695,6 +716,11 @@ const getConfigForPath = (layers: ReturnType<typeof readConfigLayers>, targetPat
|
||||
|
||||
const writeConfig = (config: Record<string, unknown>, 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);
|
||||
|
||||
Reference in New Issue
Block a user