diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index fc263b3d0..27906161d 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -451,9 +451,16 @@ function activationIsInactive(kind, operation, content) { return content.toString('utf8') === getDisabledOpenCodePluginContent(); } const text = content.toString('utf8'); + const relativePath = operation.sourceRelativePath || operation.destinationPath; // JSONC-tolerant parse so a comment in the user's config does not crash // the install; the ownership guard never writes this file (issue #3546). - const config = parseJsoncConfig(text, operation.sourceRelativePath || operation.destinationPath); + const config = parseJsoncConfig(text, relativePath); + // Keep the same shape error the strict parser surfaced via + // disableOpenCodeHookPluginRegistration: a non-array plugin must not + // silently count as inactive. + if (config.plugin !== undefined && !Array.isArray(config.plugin)) { + throw new Error(`Invalid ${relativePath}: plugin must be an array`); + } return !Array.isArray(config.plugin) || !config.plugin.includes('./plugins'); } diff --git a/scripts/lib/install/hook-consent.js b/scripts/lib/install/hook-consent.js index 689bd25e6..07cff400e 100644 --- a/scripts/lib/install/hook-consent.js +++ b/scripts/lib/install/hook-consent.js @@ -65,6 +65,10 @@ function parseJsoncConfig(content, sourceRelativePath) { return config; } +function isLineBreak(ch) { + return ch === '\n' || ch === '\r'; +} + /** * Advance past whitespace and // or /* *\/ comments; returns the index of * the next significant character. @@ -74,12 +78,13 @@ function skipJsoncTrivia(input, start) { for (;;) { while (j < input.length && /\s/.test(input[j])) j += 1; if (input[j] === '/' && input[j + 1] === '/') { - while (j < input.length && input[j] !== '\n') j += 1; + while (j < input.length && !isLineBreak(input[j])) j += 1; continue; } if (input[j] === '/' && input[j + 1] === '*') { j += 2; while (j < input.length && !(input[j] === '*' && input[j + 1] === '/')) j += 1; + if (j >= input.length) throw new Error('Unterminated block comment'); j += 2; continue; } @@ -110,13 +115,14 @@ function stripJsonc(input) { continue; } if (ch === '/' && input[i + 1] === '/') { - while (i < input.length && input[i] !== '\n') i += 1; + while (i < input.length && !isLineBreak(input[i])) i += 1; out += '\n'; continue; } if (ch === '/' && input[i + 1] === '*') { i += 2; while (i < input.length && !(input[i] === '*' && input[i + 1] === '/')) i += 1; + if (i >= input.length) throw new Error('Unterminated block comment'); i += 1; // consume the closing '/' out += ' '; continue; diff --git a/tests/lib/opencode-hook-consent-safety.test.js b/tests/lib/opencode-hook-consent-safety.test.js index bb4389a25..a068d2a14 100644 --- a/tests/lib/opencode-hook-consent-safety.test.js +++ b/tests/lib/opencode-hook-consent-safety.test.js @@ -248,6 +248,20 @@ function runTests() { assert.strictEqual(fs.readFileSync(destination, 'utf8'), '{ malformed'); 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"}'); + const before = fs.readFileSync(value.installStatePath); + if (mode === 'apply') assert.throws(() => applyInstallPlan(value.declinePlan), /Invalid .*opencode\.json: plugin must be an array/); + else if (mode === 'repair') assert.match(repair(value).results[0].error, /plugin must be an array/); + else { + const result = buildDoctorReport({ repoRoot: value.sourceRoot, homeDir: value.homeDir, + projectRoot: value.homeDir, targets: ['opencode'] }).results[0]; + assert.match(JSON.stringify(result.issues), /plugin must be an array/); + } + assert.strictEqual(fs.readFileSync(destination, 'utf8'), '{"plugin":"./plugins"}'); + assert.deepStrictEqual(fs.readFileSync(value.installStatePath), before); + })); } test('apply rejects another enabled writer and repair while holding its target lease', () => fixture(value => { let checked = false; @@ -328,6 +342,16 @@ function runTests() { assert.strictEqual(config.endpoint, 'https://example.com/path'); assert.strictEqual(config.block, '/* not a comment */'); }); + test('parseJsoncConfig rejects an unterminated block comment with source context', () => { + assert.throws( + () => parseJsoncConfig('{"plugin":[]} /* unfinished', 'opencode.json'), + /Failed to parse opencode\.json: Unterminated block comment/, + ); + }); + test('parseJsoncConfig stops line comments at CR-only line endings', () => { + const config = parseJsoncConfig('{"plugin":[] // note\r}', 'opencode.json'); + assert.deepStrictEqual(config.plugin, []); + }); test(`fresh ${consent || 'default'} apply preserves inactive JSONC user config with comments`, () => fixture(value => { fs.unlinkSync(value.installStatePath); fs.unlinkSync(path.join(value.targetRoot, 'plugins/ecc-hooks.ts'));