diff --git a/src/application/ClientConfigurationDocument.ts b/src/application/ClientConfigurationDocument.ts index a226f8cb6..f7ab8f0e2 100644 --- a/src/application/ClientConfigurationDocument.ts +++ b/src/application/ClientConfigurationDocument.ts @@ -1520,12 +1520,25 @@ export const parseGeminiMcpPolicy = (parsed: ClientConfigurationDocument) => .passthrough() .parse(parsed.document.mcp ?? {}); +/** One server-name comparison for the exclusion, intersection and membership rules below. */ +const normalizeGeminiServerName = (name: string): string => + name.toLowerCase().trim(); + /** Gemini consolidates excludes by union and allowlists by normalized intersection. */ export const geminiServerPolicyBlock = ( policies: readonly ReturnType[], serverKey: string, ): string | undefined => { - if (policies.some(({ excluded }) => excluded?.includes(serverKey))) + // Hand-authored settings cannot be assumed to spell the product key exactly, + // so every rule compares normalized names. Testing the consolidated list + // against the raw key instead would make the outcome depend on which policy + // supplied the spelling that survived the intersection. + const key = normalizeGeminiServerName(serverKey); + if ( + policies.some(({ excluded }) => + excluded?.some((name) => normalizeGeminiServerName(name) === key), + ) + ) return `Gemini mcp.excluded blocks ${serverKey}. Review the selected settings policies before rerunning setup.`; const allowlists = policies.flatMap(({ allowed }) => allowed === undefined ? [] : [allowed], @@ -1533,11 +1546,9 @@ export const geminiServerPolicyBlock = ( const allowed = allowlists.reduce( (previous, current) => { if (previous === undefined) return current; - const normalized = new Set( - current.map((name) => name.toLowerCase().trim()), - ); + const normalized = new Set(current.map(normalizeGeminiServerName)); return previous.filter((name) => - normalized.has(name.toLowerCase().trim()), + normalized.has(normalizeGeminiServerName(name)), ); }, undefined, @@ -1546,7 +1557,7 @@ export const geminiServerPolicyBlock = ( if ( allowed !== undefined && allowed.length > 0 && - !allowed.includes(serverKey) + !allowed.some((name) => normalizeGeminiServerName(name) === key) ) return `Gemini mcp.allowed does not include ${serverKey}. Review the selected settings policies before rerunning setup.`; return undefined; diff --git a/tests/boundary/cli/clientProfiles.test.ts b/tests/boundary/cli/clientProfiles.test.ts index bab6dedd8..092ce65d0 100644 --- a/tests/boundary/cli/clientProfiles.test.ts +++ b/tests/boundary/cli/clientProfiles.test.ts @@ -262,6 +262,91 @@ cliTest( }, ); +cliTest( + "Gemini matches allowlist spellings independently of their policy order", + async ({ cli }) => { + const home = await createTestTempDirectory("rea-gemini-case-"); + const system = join(home, "system.json"); + const defaults = join(home, "defaults.json"); + await writeFile(system, JSON.stringify({ mcp: { allowed: ["REA"] } })); + const environment = { + HOME: home, + USERPROFILE: home, + GEMINI_CLI_SYSTEM_SETTINGS_PATH: system, + GEMINI_CLI_SYSTEM_DEFAULTS_PATH: defaults, + }; + const run = () => + cli.run({ + arguments: ["setup", "--client", "gemini_cli", "--yes", "--json"], + cwd: home, + environment, + }); + // The intersection compares names case-insensitively, so the spelling that + // survives must not decide the outcome: the same pair of policies in + // either arrangement has to consolidate to the same decision. + for (const [systemSpelling, defaultSpelling] of [ + ["REA", "rea"], + ["rea", "REA"], + ]) { + await writeFile( + system, + JSON.stringify({ mcp: { allowed: [systemSpelling] } }), + ); + await writeFile( + defaults, + JSON.stringify({ mcp: { allowed: [defaultSpelling] } }), + ); + const result = await run(); + expect(result.exitCode, `${systemSpelling}+${defaultSpelling}`).toBe(0); + expect(result.json, `${systemSpelling}+${defaultSpelling}`).toMatchObject( + { + status: "ready", + }, + ); + } + }, +); + +cliTest( + "Gemini compares exclusions and padded allowlist entries through one normalization", + async ({ cli }) => { + const home = await createTestTempDirectory("rea-gemini-normalized-"); + const system = join(home, "system.json"); + const defaults = join(home, "defaults.json"); + const environment = { + HOME: home, + USERPROFILE: home, + GEMINI_CLI_SYSTEM_SETTINGS_PATH: system, + GEMINI_CLI_SYSTEM_DEFAULTS_PATH: defaults, + }; + const run = () => + cli.run({ + arguments: ["setup", "--client", "gemini_cli", "--yes", "--json"], + cwd: home, + environment, + }); + await writeFile(defaults, JSON.stringify({ mcp: {} })); + // An exclusion is a membership test over the same names the intersection + // compares, so a differently spelled and padded entry blocks setup rather + // than slipping past the raw comparison. + await writeFile(system, JSON.stringify({ mcp: { excluded: [" REA "] } })); + const blocked = await run(); + expect(blocked.exitCode).toBe(1); + expect(blocked.json).toMatchObject({ + status: "needs_human", + plannedActions: [], + appliedActions: [], + remediation: expect.stringContaining("mcp.excluded"), + }); + // Padding is presentation, not identity: a padded allowlist entry permits + // the same server its exact spelling permits. + await writeFile(system, JSON.stringify({ mcp: { allowed: [" REA "] } })); + const allowed = await run(); + expect(allowed.exitCode).toBe(0); + expect(allowed.json).toMatchObject({ status: "ready" }); + }, +); + cliTest( "OpenCode doctor sees a global registration when the custom directory is absent", async ({ cli }) => {