fix: safely resolve MCP environment overrides

src/index.test.ts

@@ -0,0 +1,209 @@
+import { promises as fs } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+
+import type { PluginInput } from '@opencode-ai/plugin';
+import type { Config } from '@opencode-ai/sdk';
+import { describe, expect, it, vi } from 'vitest';
+
+vi.mock('@opencode-ai/plugin', () => {
+  const createSchemaChain = (): Record<string, () => unknown> => {
+    const chain = {
+      describe: () => chain,
+      optional: () => chain,
+    };
+    return chain;
+  };
+  return {
+    tool: Object.assign(<T>(definition: T): T => definition, {
+      schema: {
+        enum: () => createSchemaChain(),
+        string: () => createSchemaChain(),
+        boolean: () => createSchemaChain(),
+        array: () => createSchemaChain(),
+      },
+    }),
+  };
+});
+
+import { opencodeConfigSync } from './index.js';
+import { syncLocalToRepo } from './sync/apply.js';
+import { normalizeSyncConfig, parseJsonc } from './sync/config.js';
+import { buildSyncPlan, resolveSyncLocations } from './sync/paths.js';
+
+const ENV_KEYS = [
+  'HOME',
+  'XDG_CONFIG_HOME',
+  'XDG_DATA_HOME',
+  'XDG_STATE_HOME',
+  'PR47_GITHUB_PAT',
+  'MISSING_PAT',
+] as const;
+
+function createPluginInput(logs: unknown[]): PluginInput {
+  return {
+    $: (() => {
+      throw new Error('Shell execution is not expected in config-hook tests.');
+    }) as unknown as PluginInput['$'],
+    client: {
+      app: { log: async (entry: unknown) => logs.push(entry) },
+      config: { get: async () => ({ data: {} }) },
+      session: {
+        create: async () => ({ data: null }),
+        delete: async () => ({}),
+        prompt: async () => ({ data: null }),
+        status: async () => ({ data: {} }),
+      },
+      tui: { showToast: async () => ({}) },
+    } as unknown as PluginInput['client'],
+  } as PluginInput;
+}
+
+async function createPluginHooks(logs: unknown[]): ReturnType<typeof opencodeConfigSync> {
+  const originalSetTimeout = globalThis.setTimeout;
+  globalThis.setTimeout = (() => 0) as unknown as typeof setTimeout;
+  try {
+    return await opencodeConfigSync(createPluginInput(logs));
+  } finally {
+    globalThis.setTimeout = originalSetTimeout;
+  }
+}
+
+async function withIsolatedPluginHome(run: (homeDir: string) => Promise<void>): Promise<void> {
+  const original = new Map(ENV_KEYS.map((key) => [key, process.env[key]]));
+  const homeDir = await fs.mkdtemp(path.join(tmpdir(), 'opencode-sync-plugin-'));
+  process.env.HOME = homeDir;
+  process.env.XDG_CONFIG_HOME = path.join(homeDir, 'config');
+  process.env.XDG_DATA_HOME = path.join(homeDir, 'data');
+  process.env.XDG_STATE_HOME = path.join(homeDir, 'state');
+
+  try {
+    await run(homeDir);
+  } finally {
+    for (const [key, value] of original) {
+      if (value === undefined) delete process.env[key];
+      else process.env[key] = value;
+    }
+    await fs.rm(homeDir, { recursive: true, force: true });
+  }
+}
+
+describe('opencode plugin config hook', () => {
+  it('resolves a local placeholder at runtime without exposing it to the synced repo or logs', async () => {
+    await withIsolatedPluginHome(async (homeDir) => {
+      const locations = resolveSyncLocations();
+      const repoRoot = path.join(homeDir, 'sync-repo');
+      const localConfigPath = path.join(locations.configRoot, 'opencode.json');
+      const repoConfigPath = path.join(repoRoot, 'config', 'opencode.json');
+      const secret = 'runtime-only-secret-sentinel';
+      await fs.mkdir(locations.configRoot, { recursive: true });
+      await fs.writeFile(
+        localConfigPath,
+        JSON.stringify({
+          mcp: {
+            github: {
+              type: 'remote',
+              url: 'https://example.test/mcp',
+              enabled: true,
+              headers: { Authorization: `Bearer ${secret}` },
+            },
+          },
+        }),
+        'utf8'
+      );
+      const syncConfig = normalizeSyncConfig({
+        repo: { owner: 'acme', name: 'config' },
+        includeOpencodeSkills: false,
+        includeAgentsDir: false,
+        includeModelFavorites: false,
+      });
+      const plan = buildSyncPlan(syncConfig, locations, repoRoot, 'linux');
+      await syncLocalToRepo(plan, null, { overridesPath: locations.overridesPath });
+
+      await fs.writeFile(
+        locations.overridesPath,
+        `{
+          // Parsed after OpenCode's file-level environment substitution.
+          "mcp": {
+            "github": {
+              "headers": { "Authorization": "Bearer {env:PR47_GITHUB_PAT}" },
+            },
+          },
+        }\n`,
+        'utf8'
+      );
+      process.env.PR47_GITHUB_PAT = secret;
+      const logs: unknown[] = [];
+      const hooks = await createPluginHooks(logs);
+      const unrelated = { keep: true };
+      const runtime = parseJsonc<Config & { unrelated: typeof unrelated }>(
+        await fs.readFile(repoConfigPath, 'utf8')
+      );
+      runtime.unrelated = unrelated;
+
+      await hooks.config?.(runtime);
+
+      expect(runtime.unrelated).toBe(unrelated);
+      expect(runtime.mcp).toEqual({
+        github: {
+          type: 'remote',
+          url: 'https://example.test/mcp',
+          enabled: true,
+          headers: { Authorization: `Bearer ${secret}` },
+        },
+      });
+      expect(await fs.readFile(repoConfigPath, 'utf8')).not.toContain(secret);
+      expect(await fs.readFile(locations.overridesPath, 'utf8')).not.toContain(secret);
+      expect(JSON.stringify(logs)).not.toContain(secret);
+    });
+  });
+
+  it('rejects a missing placeholder with field context and without exposing other env values', async () => {
+    await withIsolatedPluginHome(async () => {
+      const locations = resolveSyncLocations();
+      await fs.mkdir(locations.configRoot, { recursive: true });
+      await fs.writeFile(
+        locations.overridesPath,
+        '{"mcp":{"github":{"headers":{"Authorization":"{env:MISSING_PAT}"}}}}\n',
+        'utf8'
+      );
+      delete process.env.MISSING_PAT;
+      const logs: unknown[] = [];
+      const hooks = await createPluginHooks(logs);
+      const runtime: Config & { keep: boolean } = {
+        keep: true,
+        mcp: {
+          github: {
+            type: 'remote',
+            url: 'https://example.test/mcp',
+            enabled: true,
+            headers: { Authorization: '' },
+          },
+        },
+      };
+
+      try {
+        await hooks.config?.(runtime);
+      } catch {
+        // OpenCode isolates plugin-hook failures and continues with the mutated runtime config.
+      }
+      expect(runtime).toEqual({
+        keep: true,
+        command: expect.any(Object),
+        mcp: {
+          github: {
+            type: 'remote',
+            url: 'https://example.test/mcp',
+            enabled: false,
+            headers: { Authorization: '' },
+          },
+        },
+      });
+      expect(JSON.stringify(logs)).toContain(
+        'Missing environment variable \\"MISSING_PAT\\" required by local override ' +
+          '\\"overrides.mcp.github.headers.Authorization\\".'
+      );
+      expect(JSON.stringify(logs)).not.toContain('runtime-only-secret-sentinel');
+    });
+  });
+});

src/index.ts

@@ -4,7 +4,13 @@ import { fileURLToPath } from 'node:url';
 import type { Plugin } from '@opencode-ai/plugin';
 import { tool } from '@opencode-ai/plugin';
 
-import { applyOverridesToRuntimeConfig, loadOverrides } from './sync/config.js';
+import {
+  applyOverridesToRuntimeConfig,
+  EnvPlaceholderResolutionError,
+  hasOwn,
+  isPlainObject,
+  loadOverrides,
+} from './sync/config.js';
 import { SyncCommandError, SyncConfigMissingError } from './sync/errors.js';
 import { resolveSyncLocations } from './sync/paths.js';
 import { createSyncService } from './sync/service.js';
@@ -303,13 +309,26 @@ export const opencodeConfigSync: Plugin = async (ctx) => {
         };
       }
 
-      try {
-        const overrides = await loadOverrides(resolveSyncLocations());
-        if (overrides) {
+      const overrides = await loadOverrides(resolveSyncLocations());
+      if (overrides) {
+        try {
           applyOverridesToRuntimeConfig(config as Record<string, unknown>, overrides);
+        } catch (error) {
+          if (error instanceof EnvPlaceholderResolutionError) {
+            disableMcpServerForResolutionFailure(
+              config as Record<string, unknown>,
+              error.fieldPath
+            );
+            await ctx.client.app.log({
+              body: {
+                service: 'opencode-synced',
+                level: 'error',
+                message: error.message,
+              },
+            });
+          }
+          throw error;
         }
-      } catch {
-        return;
       }
     },
   };
@@ -318,6 +337,25 @@ export const opencodeConfigSync: Plugin = async (ctx) => {
 export const opencodeSynced = opencodeConfigSync;
 export default opencodeConfigSync;
 
+function disableMcpServerForResolutionFailure(
+  config: Record<string, unknown>,
+  fieldPath: readonly string[]
+): void {
+  if (fieldPath[0] !== 'overrides' || fieldPath[1] !== 'mcp') return;
+  const serverName = fieldPath[2];
+  if (!serverName) return;
+
+  const mcp = isPlainObject(config.mcp) ? config.mcp : null;
+  if (!mcp || !hasOwn(mcp, serverName) || !isPlainObject(mcp[serverName])) return;
+
+  Object.defineProperty(mcp[serverName], 'enabled', {
+    value: false,
+    enumerable: true,
+    configurable: true,
+    writable: true,
+  });
+}
+
 function formatError(error: unknown): string {
   if (error instanceof Error) return error.message;
   return String(error);

src/sync/apply.test.ts

@@ -3,7 +3,7 @@ import { tmpdir } from 'node:os';
 import path from 'node:path';
 import { describe, expect, it } from 'vitest';
 import { syncLocalToRepo, syncRepoToLocal } from './apply.js';
-import { normalizeSyncConfig } from './config.js';
+import { loadOverrides, normalizeSyncConfig, parseJsonc } from './config.js';
 import type { ExtraPathPlan, SyncItem, SyncPlan } from './paths.js';
 import { buildSyncPlan, resolveSyncLocations } from './paths.js';
 
@@ -320,3 +320,97 @@ describe('syncing plural OpenCode config directories', () => {
     });
   });
 });
+
+describe('MCP secret scrub round trip', () => {
+  it('keeps the secret local, writes a placeholder to the repo, and protects overrides', async () => {
+    await withTempDir(async (root) => {
+      const homeDir = path.join(root, 'home');
+      const repoRoot = path.join(root, 'repo');
+      const locations = resolveSyncLocations({ HOME: homeDir }, 'linux');
+      const localConfigPath = path.join(locations.configRoot, 'opencode.jsonc');
+      const repoConfigPath = path.join(repoRoot, 'config', 'opencode.jsonc');
+      const secret = 'focused-runtime-secret';
+      await fs.mkdir(locations.configRoot, { recursive: true });
+      await fs.writeFile(
+        localConfigPath,
+        `{
+          // The secret must never reach Git.
+          "mcp": {
+            "github": {
+              "headers": { "Authorization": "Bearer ${secret}" },
+            },
+          },
+        }\n`,
+        'utf8'
+      );
+
+      const config = normalizeSyncConfig({
+        repo: { owner: 'acme', name: 'config' },
+        includeOpencodeSkills: false,
+        includeAgentsDir: false,
+        includeModelFavorites: false,
+      });
+      const plan = buildSyncPlan(config, locations, repoRoot, 'linux');
+      await syncLocalToRepo(plan, null, { overridesPath: locations.overridesPath });
+
+      const repoContent = await fs.readFile(repoConfigPath, 'utf8');
+      const overridesContent = await fs.readFile(locations.overridesPath, 'utf8');
+      const overrideMode = (await fs.stat(locations.overridesPath)).mode & 0o777;
+      expect(repoContent).toContain('Bearer {env:opencode_mcp_GITHUB_AUTHORIZATION}');
+      expect(repoContent).not.toContain(secret);
+      expect(overridesContent).toContain(secret);
+      expect(overrideMode).toBe(0o600);
+
+      const overrides = await loadOverrides(locations);
+      expect(overrides).not.toBeNull();
+      await fs.writeFile(localConfigPath, '{}\n', 'utf8');
+      await syncRepoToLocal(plan, overrides);
+
+      const restored = parseJsonc<Record<string, unknown>>(
+        await fs.readFile(localConfigPath, 'utf8')
+      );
+      expect(restored).toEqual({
+        mcp: {
+          github: {
+            headers: { Authorization: `Bearer ${secret}` },
+          },
+        },
+      });
+    });
+  });
+
+  it('rejects malformed credential values before mutating the synced repository', async () => {
+    await withTempDir(async (root) => {
+      const homeDir = path.join(root, 'home');
+      const repoRoot = path.join(root, 'repo');
+      const locations = resolveSyncLocations({ HOME: homeDir }, 'linux');
+      const localConfigPath = path.join(locations.configRoot, 'opencode.json');
+      const repoConfigPath = path.join(repoRoot, 'config', 'opencode.json');
+      await fs.mkdir(path.dirname(localConfigPath), { recursive: true });
+      await fs.mkdir(path.dirname(repoConfigPath), { recursive: true });
+      await fs.writeFile(
+        localConfigPath,
+        '{"mcp":{"github":{"headers":{"Authorization":false}}}}\n',
+        'utf8'
+      );
+      const originalRepoContent = '{"existing":"must-remain"}\n';
+      await fs.writeFile(repoConfigPath, originalRepoContent, 'utf8');
+      const config = normalizeSyncConfig({
+        repo: { owner: 'acme', name: 'config' },
+        includeOpencodeSkills: false,
+        includeAgentsDir: false,
+        includeModelFavorites: false,
+      });
+      const plan = buildSyncPlan(config, locations, repoRoot, 'linux');
+
+      await expect(
+        syncLocalToRepo(plan, null, { overridesPath: locations.overridesPath })
+      ).rejects.toThrow(
+        'MCP credential field "mcp.github.headers.Authorization" must be a string before it can ' +
+          'be synchronized.'
+      );
+      expect(await fs.readFile(repoConfigPath, 'utf8')).toBe(originalRepoContent);
+      await expect(fs.stat(locations.overridesPath)).rejects.toMatchObject({ code: 'ENOENT' });
+    });
+  });
+});