fix(byok): harden the Windows DPAPI worker lifecycle

apps/daemon/src/server.ts

@@ -859,9 +859,6 @@ const SANDBOX_MODE_ENABLED = isSandboxModeEnabled(process.env);
 const RUNTIME_DATA_DIR = resolveDataDir(process.env.OD_DATA_DIR, PROJECT_ROOT, {
   requireExplicit: SANDBOX_MODE_ENABLED,
 });
-const defaultByokCredentialService = new ByokCredentialService({
-  dataDir: RUNTIME_DATA_DIR,
-});
 const SANDBOX_RUNTIME = resolveSandboxRuntimeConfig(SANDBOX_MODE_ENABLED, RUNTIME_DATA_DIR);
 ensureSandboxRuntimeDirs(SANDBOX_RUNTIME);
 const PLUGIN_LOCKFILE_PATH = path.join(RUNTIME_DATA_DIR, 'od-plugin-lock.json');
@@ -2053,12 +2050,12 @@ export interface StartServerOptions {
 export interface StartServerResult {
   url: string;
   server: import('node:http').Server;
-  shutdown: () => Promise<void> | void;
+  shutdown: () => Promise<void>;
   routeInventory: import('./route-registration-guard.js').RouteRegistration[];
 }
 
 export async function startServer({
-  byokCredentialService = defaultByokCredentialService,
+  byokCredentialService = new ByokCredentialService({ dataDir: RUNTIME_DATA_DIR }),
   port = 7456,
   host = normalizeDaemonBindHost(process.env.OD_BIND_HOST),
   returnServer = false,
@@ -9110,19 +9107,23 @@ export async function startServer({
   //   - `apps/daemon/sidecar/server.ts`     → expects `{ url, server }`
   //   - `apps/daemon/tests/version-route.test.ts` → expects `{ url, server }`
   return await new Promise((resolve, reject) => {
-    let daemonShutdownStarted = false;
+    let daemonShutdownPromise: Promise<void> | null = null;
     const cleanupDaemonBackgroundWork = () => {
       composioConnectorProvider.stopCatalogRefreshLoop();
       orbitService.stop();
       routineService?.stop();
     };
-    const shutdownDaemonRuns = async () => {
-      if (daemonShutdownStarted) return;
-      daemonShutdownStarted = true;
-      daemonShuttingDown = true;
-      await design.runs.shutdownActive({ graceMs: resolveChatRunShutdownGraceMs() });
-      await terminalService.shutdownActive();
-      await design.analytics.shutdown();
+    const shutdownDaemonRuns = () => {
+      daemonShutdownPromise ??= (async () => {
+        daemonShuttingDown = true;
+        const byokShutdown = byokCredentialService.close();
+        void byokShutdown.catch(() => undefined);
+        await design.runs.shutdownActive({ graceMs: resolveChatRunShutdownGraceMs() });
+        await terminalService.shutdownActive();
+        await byokShutdown;
+        await design.analytics.shutdown();
+      })();
+      return daemonShutdownPromise;
     };
     let server;
     try {
@@ -9190,7 +9191,11 @@ export async function startServer({
       return;
     }
     server.once('close', () => {
-      void shutdownDaemonRuns().finally(cleanupDaemonBackgroundWork);
+      void shutdownDaemonRuns()
+        .catch(() => {
+          console.error('[od] daemon shutdown cleanup failed');
+        })
+        .finally(cleanupDaemonBackgroundWork);
     });
     // `app.listen` throws synchronously when the port is already in use on
     // some Node versions, but emits an `error` event on others (and for

apps/daemon/tests/byok/credential-service.test.ts

@@ -1,10 +1,11 @@
 import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import path from 'node:path';
-import { afterEach, describe, expect, it } from 'vitest';
+import { afterEach, describe, expect, it, vi } from 'vitest';
 
 import {
   ByokCredentialService,
+  WindowsDpapiBackend,
   createPlatformByokSecretBackend,
   type ByokSecretBackend,
 } from '../../src/byok/credential-service.js';
@@ -28,6 +29,8 @@ class MemorySecretBackend implements ByokSecretBackend {
   async delete(profileId: string) {
     return this.secrets.delete(profileId);
   }
+
+  async close() {}
 }
 
 describe('BYOK credential service', () => {
@@ -88,6 +91,26 @@ describe('BYOK credential service', () => {
     })).rejects.toThrow(/secure credential storage is unavailable/i);
   });
 
+  it('rejects oversized UTF-8 API keys before they can overflow the worker protocol', async () => {
+    const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-'));
+    roots.push(dataDir);
+    const backend = new MemorySecretBackend();
+    backend.set = vi.fn(backend.set.bind(backend));
+    const service = new ByokCredentialService({ dataDir, backend });
+
+    await expect(service.upsert({
+      id: 'byok-oversized-secret',
+      label: 'Oversized',
+      protocol: 'openai',
+      baseUrl: 'https://example.test/v1',
+      model: 'model',
+      apiKey: '密'.repeat(16_385),
+    })).rejects.toThrow('apiKey must be at most 32768 UTF-8 bytes.');
+
+    expect(backend.set).not.toHaveBeenCalled();
+    expect(backend.secrets).toEqual(new Map());
+  });
+
   it('dispatches native Windows credentials to a DPAPI backend rooted in OD_DATA_DIR', async () => {
     const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-windows-dispatch-'));
     roots.push(dataDir);
@@ -97,6 +120,38 @@ describe('BYOK credential service', () => {
     expect(backend.kind).toBe('windows-dpapi');
   });
 
+  it('reuses one Windows DPAPI worker across availability and credential operations', async () => {
+    const worker = {
+      close: vi.fn(async () => undefined),
+      ready: vi.fn(async () => undefined),
+      run: vi.fn(async (operation: 'set' | 'get' | 'delete') => {
+        if (operation === 'get') {
+          return { found: true, value: 'test-secret' };
+        }
+        return { found: true, value: null };
+      }),
+    };
+    const createWorker = vi.fn(async () => worker);
+    const backend = new WindowsDpapiBackend('/test/byok/secrets', {
+      commandAvailable: async () => true,
+      createWorker,
+    });
+
+    await expect(backend.available()).resolves.toBe(true);
+    await expect(backend.available()).resolves.toBe(true);
+    await expect(backend.set('byok-worker-reuse', 'test-secret')).resolves.toBeUndefined();
+    await expect(backend.get('byok-worker-reuse')).resolves.toBe('test-secret');
+    await expect(backend.delete('byok-worker-reuse')).resolves.toBe(true);
+
+    expect(createWorker).toHaveBeenCalledTimes(1);
+    expect(worker.ready).toHaveBeenCalledTimes(1);
+    expect(worker.run.mock.calls.map(([operation]) => operation)).toEqual([
+      'set',
+      'get',
+      'delete',
+    ]);
+  });
+
   it('serializes concurrent metadata mutations so profiles cannot overwrite each other', async () => {
     const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-'));
     roots.push(dataDir);
@@ -130,6 +185,44 @@ describe('BYOK credential service', () => {
     ]);
   });
 
+  it('stops accepting work and waits for an accepted mutation before closing the backend', async () => {
+    const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-'));
+    roots.push(dataDir);
+    const backend = new MemorySecretBackend();
+    let releaseSet: (() => void) | undefined;
+    const setReleased = new Promise<void>((resolve) => {
+      releaseSet = resolve;
+    });
+    backend.set = vi.fn(async (profileId: string, secret: string) => {
+      await setReleased;
+      backend.secrets.set(profileId, secret);
+    });
+    backend.close = vi.fn(async () => undefined);
+    const service = new ByokCredentialService({ dataDir, backend });
+    const upsert = service.upsert({
+      id: 'byok-close-boundary',
+      label: 'Close boundary',
+      protocol: 'openai',
+      baseUrl: 'https://example.test/v1',
+      model: 'model',
+      apiKey: 'accepted-secret',
+    });
+    await vi.waitFor(() => {
+      expect(backend.set).toHaveBeenCalledTimes(1);
+    });
+
+    const close = service.close();
+    await expect(service.status()).rejects.toThrow('Secure credential service is closed.');
+    expect(backend.close).not.toHaveBeenCalled();
+
+    releaseSet?.();
+    await expect(upsert).resolves.toMatchObject({ id: 'byok-close-boundary' });
+    await expect(close).resolves.toBeUndefined();
+    await expect(service.close()).resolves.toBeUndefined();
+
+    expect(backend.close).toHaveBeenCalledTimes(1);
+  });
+
   it('removes a newly created secret when metadata persistence fails', async () => {
     const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-'));
     roots.push(dataDir);

apps/daemon/tests/byok/credential-service.windows.test.ts

@@ -25,40 +25,44 @@ describe.runIf(process.platform === 'win32')('Windows DPAPI BYOK credential smok
     const service = new ByokCredentialService({ dataDir, backend });
     const apiKey = 'windows-dpapi-smoke-secret';
 
-    await expect(service.status()).resolves.toEqual({
-      available: true,
-      backend: 'windows-dpapi',
-    });
-    const profile = await service.upsert({
-      id: 'byok-windows-dpapi-smoke',
-      label: 'Windows DPAPI smoke',
-      protocol: 'openai',
-      baseUrl: 'https://api.openai.com/v1',
-      model: 'gpt-5.4',
-      apiKey,
-    });
+    try {
+      await expect(service.status()).resolves.toEqual({
+        available: true,
+        backend: 'windows-dpapi',
+      });
+      const profile = await service.upsert({
+        id: 'byok-windows-dpapi-smoke',
+        label: 'Windows DPAPI smoke',
+        protocol: 'openai',
+        baseUrl: 'https://api.openai.com/v1',
+        model: 'gpt-5.4',
+        apiKey,
+      });
 
-    expect(profile).toMatchObject({
-      id: 'byok-windows-dpapi-smoke',
-      configured: true,
-      keyTail: 'cret',
-    });
-    expect(await service.resolve(profile.id)).toMatchObject({
-      apiKey,
-      provider: { apiKey },
-    });
-    expect(
-      await readFile(
-        path.join(dataDir, 'byok', 'profiles.json'),
-        'utf8',
-      ),
-    ).not.toContain(apiKey);
-    const encrypted = await readFile(
-      path.join(dataDir, 'byok', 'secrets', `${profile.id}.bin`),
-    );
-    expect(encrypted.includes(Buffer.from(apiKey, 'utf8'))).toBe(false);
+      expect(profile).toMatchObject({
+        id: 'byok-windows-dpapi-smoke',
+        configured: true,
+        keyTail: 'cret',
+      });
+      expect(await service.resolve(profile.id)).toMatchObject({
+        apiKey,
+        provider: { apiKey },
+      });
+      expect(
+        await readFile(
+          path.join(dataDir, 'byok', 'profiles.json'),
+          'utf8',
+        ),
+      ).not.toContain(apiKey);
+      const encrypted = await readFile(
+        path.join(dataDir, 'byok', 'secrets', `${profile.id}.bin`),
+      );
+      expect(encrypted.includes(Buffer.from(apiKey, 'utf8'))).toBe(false);
 
-    await expect(service.delete(profile.id)).resolves.toBe(true);
-    await expect(service.resolve(profile.id)).resolves.toBeNull();
+      await expect(service.delete(profile.id)).resolves.toBe(true);
+      await expect(service.resolve(profile.id)).resolves.toBeNull();
+    } finally {
+      await service.close();
+    }
   });
 });

apps/daemon/tests/byok/server-shutdown.test.ts

@@ -0,0 +1,113 @@
+import type { Server } from 'node:http';
+
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+import {
+  ByokCredentialService,
+  type ByokSecretBackend,
+} from '../../src/byok/credential-service.js';
+import {
+  startServer,
+  type StartServerResult,
+} from '../../src/server.js';
+
+class ShutdownTestBackend implements ByokSecretBackend {
+  readonly kind = 'shutdown-test';
+
+  async available() {
+    return true;
+  }
+
+  async close() {}
+
+  async set() {}
+
+  async get() {
+    return null;
+  }
+
+  async delete() {
+    return false;
+  }
+}
+
+describe('daemon BYOK shutdown ownership', () => {
+  let server: Server | undefined;
+  let releaseClose: (() => void) | undefined;
+
+  afterEach(async () => {
+    releaseClose?.();
+    if (server?.listening) {
+      await new Promise<void>((resolve) => server?.close(() => resolve()));
+    }
+    server = undefined;
+    releaseClose = undefined;
+  });
+
+  it('shares one shutdown promise and waits for the credential worker owner to close', async () => {
+    const service = new ByokCredentialService({
+      dataDir: '/test/byok-shutdown',
+      backend: new ShutdownTestBackend(),
+    });
+    let closeStarted: (() => void) | undefined;
+    const closeObserved = new Promise<void>((resolve) => {
+      closeStarted = resolve;
+    });
+    const closeGate = new Promise<void>((resolve) => {
+      releaseClose = resolve;
+    });
+    service.close = vi.fn(async () => {
+      closeStarted?.();
+      await closeGate;
+    });
+    const started = await startServer({
+      byokCredentialService: service,
+      port: 0,
+      returnServer: true,
+    }) as StartServerResult;
+    server = started.server;
+
+    const firstShutdown = started.shutdown() as Promise<void>;
+    const secondShutdown = started.shutdown() as Promise<void>;
+    expect(secondShutdown).toBe(firstShutdown);
+    await closeObserved;
+
+    let settled = false;
+    void firstShutdown.then(() => {
+      settled = true;
+    });
+    await Promise.resolve();
+    expect(settled).toBe(false);
+
+    releaseClose?.();
+    await expect(Promise.all([firstShutdown, secondShutdown])).resolves.toEqual([
+      undefined,
+      undefined,
+    ]);
+    expect(service.close).toHaveBeenCalledTimes(1);
+  });
+
+  it('closes the credential worker owner when the HTTP server closes first', async () => {
+    const service = new ByokCredentialService({
+      dataDir: '/test/byok-server-close',
+      backend: new ShutdownTestBackend(),
+    });
+    service.close = vi.fn(async () => undefined);
+    const started = await startServer({
+      byokCredentialService: service,
+      port: 0,
+      returnServer: true,
+    }) as StartServerResult;
+    server = started.server;
+
+    await new Promise<void>((resolve, reject) => {
+      started.server.close((error) => {
+        if (error) reject(error);
+        else resolve();
+      });
+    });
+    await vi.waitFor(() => {
+      expect(service.close).toHaveBeenCalledTimes(1);
+    });
+  });
+});