From 7f0b6ce50043179bffe2ff5f4292b879ffab7f76 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 25 Oct 2026 18:20:40 -0400 Subject: [PATCH] fix(install): reject malformed empty-container JSONC commas --- scripts/lib/install/hook-consent.js | 16 ++++++--- .../lib/opencode-hook-consent-safety.test.js | 35 +++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/scripts/lib/install/hook-consent.js b/scripts/lib/install/hook-consent.js index 07cff400e..25a54386a 100644 --- a/scripts/lib/install/hook-consent.js +++ b/scripts/lib/install/hook-consent.js @@ -92,6 +92,14 @@ function skipJsoncTrivia(input, start) { } } +function isJsoncTrailingComma(input, index, previousSignificant) { + const next = skipJsoncTrivia(input, index + 1); + // Preserve commas in empty containers or after another separator so + // JSON.parse rejects them; it validates the remaining value syntax. + return (input[next] === '}' || input[next] === ']') + && previousSignificant !== null && !'[{,:'.includes(previousSignificant); +} + /** * Strip // and /* *\/ comments and the trailing commas they hide, without * touching string contents (URLs like "https://..." must survive intact). @@ -100,6 +108,7 @@ function stripJsonc(input) { let out = ''; let inString = false; let escaped = false; + let previousSignificant = null; for (let i = 0; i < input.length; i += 1) { const ch = input[i]; if (inString) { @@ -112,6 +121,7 @@ function stripJsonc(input) { if (ch === '"') { inString = true; out += ch; + previousSignificant = '"'; continue; } if (ch === '/' && input[i + 1] === '/') { @@ -128,12 +138,10 @@ function stripJsonc(input) { continue; } if (ch === ',') { - const next = skipJsoncTrivia(input, i + 1); - // A comma before '}' or ']' (ignoring trivia) is a trailing comma - // JSON forbids. - if (input[next] === '}' || input[next] === ']') continue; + if (isJsoncTrailingComma(input, i, previousSignificant)) continue; } out += ch; + if (!/\s/.test(ch)) previousSignificant = ch; } return out; } diff --git a/tests/lib/opencode-hook-consent-safety.test.js b/tests/lib/opencode-hook-consent-safety.test.js index a068d2a14..2e2908b3b 100644 --- a/tests/lib/opencode-hook-consent-safety.test.js +++ b/tests/lib/opencode-hook-consent-safety.test.js @@ -169,6 +169,15 @@ function runTests() { try { callback(); passed++; console.log(` PASS ${name}`); } catch (error) { failed++; console.error(` FAIL ${name}: ${error.stack}`); } }; + test('JSONC rejects commas without a preceding value but keeps valid trailing commas', () => { + for (const content of ['{"plugin":[,]}', '{"plugin":[/* note */,]}', '{"plugin":[1,,]}', '{,}', '{"plugin":,}']) { + assert.throws(() => parseJsoncConfig(content, 'opencode.json'), /Failed to parse opencode\.json/); + } + assert.deepStrictEqual( + parseJsoncConfig('{"plugin":["user", /* note */],}', 'opencode.json').plugin, + ['user'], + ); + }); test('relative-root fixture remains relative across Windows checkout and temp drives', () => { const source = 'C:\\private\\source'; assert.strictEqual(path.win32.isAbsolute(path.win32.relative('D:\\checkout', source)), true); @@ -248,6 +257,21 @@ function runTests() { assert.strictEqual(fs.readFileSync(destination, 'utf8'), '{ malformed'); assert.deepStrictEqual(fs.readFileSync(value.installStatePath), before); })); + test(`${mode} rejects an empty-array comma with source context`, () => fixture(value => { + const destination = path.join(value.targetRoot, 'opencode.json'); + const content = '{"plugin":[,]}'; + fs.writeFileSync(destination, content); + const before = fs.readFileSync(value.installStatePath); + if (mode === 'apply') assert.throws(() => applyInstallPlan(value.declinePlan), /Failed to parse .*opencode\.json/); + else if (mode === 'repair') assert.match(repair(value).results[0].error, /Failed to parse .*opencode\.json/); + else { + const result = buildDoctorReport({ repoRoot: value.sourceRoot, homeDir: value.homeDir, + projectRoot: value.homeDir, targets: ['opencode'] }).results[0]; + assert.match(JSON.stringify(result.issues), /Failed to parse .*opencode\.json/); + } + assert.strictEqual(fs.readFileSync(destination, 'utf8'), content); + assert.deepStrictEqual(fs.readFileSync(value.installStatePath), before); + })); test(`${mode} reports a non-array plugin with source context`, () => fixture(value => { const destination = path.join(value.targetRoot, 'opencode.json'); fs.writeFileSync(destination, '{"plugin":"./plugins"}'); @@ -375,6 +399,17 @@ function runTests() { assert.ok(result.skippedOperations.some(operation => operation.destinationPath === destination)); assert.strictEqual(fs.existsSync(`${value.installStatePath}.ecc.lock`), false); })); + test(`fresh ${consent || 'default'} apply rejects malformed user JSONC without adopting it`, () => fixture(value => { + fs.unlinkSync(value.installStatePath); + fs.unlinkSync(path.join(value.targetRoot, 'plugins/ecc-hooks.ts')); + const destination = path.join(value.targetRoot, 'opencode.json'); + const content = '{"plugin":[,]}'; + fs.writeFileSync(destination, content); + assert.throws(() => applyInstallPlan(withHookConsent(value.basePlan, consent)), /Failed to parse .*opencode\.json/); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), content); + assert.strictEqual(fs.existsSync(value.installStatePath), false); + assert.strictEqual(fs.existsSync(`${value.installStatePath}.ecc.lock`), false); + })); test(`fresh ${consent || 'default'} apply still refuses active unrecorded config`, () => fixture(value => { fs.unlinkSync(value.installStatePath); fs.unlinkSync(path.join(value.targetRoot, 'plugins/ecc-hooks.ts'));