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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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<string, unknown> => {
|
||||
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)) {
|
||||
|
||||
Reference in New Issue
Block a user