Skip to content

Commit 45d1c7f

Browse files
PerishCodeLooper
andauthored
fix(byok): withdraw Windows DPAPI backend (#6308)
* fix(byok): withdraw Windows DPAPI backend * fix(byok): clean up retired Windows secret blobs Generated-By: looper 0.11.2 (runner=fixer, agent=codex) --------- Co-authored-by: Looper <looper@noreply.github.com>
1 parent 0c5f98e commit 45d1c7f

4 files changed

Lines changed: 60 additions & 351 deletions

File tree

apps/daemon/src/byok/credential-service.ts

Lines changed: 24 additions & 195 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { spawn } from 'node:child_process';
22
import { randomUUID } from 'node:crypto';
3-
import { access, mkdir, readFile, rename, writeFile } from 'node:fs/promises';
3+
import { access, mkdir, readFile, rename, unlink, writeFile } from 'node:fs/promises';
44
import path from 'node:path';
55
import type {
66
ByokChatProviderConfig,
@@ -346,7 +346,9 @@ export function createPlatformByokSecretBackend(
346346
if (platform === 'darwin') return new MacOsKeychainBackend();
347347
if (platform === 'linux') return new LinuxSecretServiceBackend();
348348
if (platform === 'win32' && dataDir) {
349-
return new WindowsDpapiBackend(path.join(dataDir, 'byok', 'secrets'));
349+
return new RetiredWindowsSecretCleanupBackend(
350+
path.join(dataDir, 'byok', 'secrets'),
351+
);
350352
}
351353
return new UnavailableSecretBackend(platform);
352354
}
@@ -435,137 +437,6 @@ class LinuxSecretServiceBackend implements ByokSecretBackend {
435437
}
436438
}
437439

438-
const WINDOWS_DPAPI_SCRIPT = `
439-
$ErrorActionPreference = 'Stop'
440-
Add-Type -AssemblyName System.Security
441-
$operation = $env:OD_BYOK_DPAPI_OPERATION
442-
$secretPath = $env:OD_BYOK_DPAPI_PATH
443-
444-
try {
445-
switch ($operation) {
446-
'probe' {
447-
$plain = [System.Text.Encoding]::UTF8.GetBytes('open-design-dpapi-probe')
448-
$cipher = [System.Security.Cryptography.ProtectedData]::Protect(
449-
$plain,
450-
$null,
451-
[System.Security.Cryptography.DataProtectionScope]::CurrentUser
452-
)
453-
$roundTrip = [System.Security.Cryptography.ProtectedData]::Unprotect(
454-
$cipher,
455-
$null,
456-
[System.Security.Cryptography.DataProtectionScope]::CurrentUser
457-
)
458-
if ([System.Text.Encoding]::UTF8.GetString($roundTrip) -ne 'open-design-dpapi-probe') {
459-
throw 'DPAPI probe failed'
460-
}
461-
}
462-
'set' {
463-
$secret = [Console]::In.ReadToEnd()
464-
if ([string]::IsNullOrWhiteSpace($secret)) {
465-
throw 'Secret must not be empty'
466-
}
467-
$plain = [System.Text.Encoding]::UTF8.GetBytes($secret)
468-
$cipher = [System.Security.Cryptography.ProtectedData]::Protect(
469-
$plain,
470-
$null,
471-
[System.Security.Cryptography.DataProtectionScope]::CurrentUser
472-
)
473-
$directory = [System.IO.Path]::GetDirectoryName($secretPath)
474-
[System.IO.Directory]::CreateDirectory($directory) | Out-Null
475-
$temporaryPath = "$secretPath.$([Guid]::NewGuid().ToString('N')).tmp"
476-
try {
477-
[System.IO.File]::WriteAllBytes($temporaryPath, $cipher)
478-
Move-Item -LiteralPath $temporaryPath -Destination $secretPath -Force
479-
} finally {
480-
if (Test-Path -LiteralPath $temporaryPath) {
481-
Remove-Item -LiteralPath $temporaryPath -Force
482-
}
483-
}
484-
}
485-
'get' {
486-
if (-not (Test-Path -LiteralPath $secretPath -PathType Leaf)) {
487-
exit 44
488-
}
489-
$cipher = [System.IO.File]::ReadAllBytes($secretPath)
490-
$plain = [System.Security.Cryptography.ProtectedData]::Unprotect(
491-
$cipher,
492-
$null,
493-
[System.Security.Cryptography.DataProtectionScope]::CurrentUser
494-
)
495-
[Console]::Out.Write([System.Text.Encoding]::UTF8.GetString($plain))
496-
}
497-
'delete' {
498-
if (-not (Test-Path -LiteralPath $secretPath -PathType Leaf)) {
499-
exit 44
500-
}
501-
Remove-Item -LiteralPath $secretPath -Force
502-
}
503-
default {
504-
throw 'Unsupported DPAPI operation'
505-
}
506-
}
507-
} catch {
508-
[Console]::Error.WriteLine('Open Design secure credential operation failed.')
509-
exit 1
510-
}
511-
`;
512-
513-
const WINDOWS_DPAPI_ENCODED_SCRIPT = Buffer.from(
514-
WINDOWS_DPAPI_SCRIPT,
515-
'utf16le',
516-
).toString('base64');
517-
518-
class WindowsDpapiBackend implements ByokSecretBackend {
519-
readonly kind = 'windows-dpapi';
520-
private availability: Promise<boolean> | null = null;
521-
522-
constructor(private readonly secretsDir: string) {}
523-
524-
async available() {
525-
this.availability ??= this.probeAvailability();
526-
return this.availability;
527-
}
528-
529-
private async probeAvailability() {
530-
if (!(await commandAvailable('powershell.exe'))) return false;
531-
try {
532-
await runWindowsDpapiCommand('probe', this.secretsDir);
533-
return true;
534-
} catch {
535-
return false;
536-
}
537-
}
538-
539-
async set(profileId: string, secret: string) {
540-
assertProfileId(profileId);
541-
await runWindowsDpapiCommand(
542-
'set',
543-
path.join(this.secretsDir, `${profileId}.bin`),
544-
secret,
545-
);
546-
}
547-
548-
async get(profileId: string) {
549-
assertProfileId(profileId);
550-
return runWindowsDpapiCommand(
551-
'get',
552-
path.join(this.secretsDir, `${profileId}.bin`),
553-
undefined,
554-
true,
555-
);
556-
}
557-
558-
async delete(profileId: string) {
559-
assertProfileId(profileId);
560-
return (await runWindowsDpapiCommand(
561-
'delete',
562-
path.join(this.secretsDir, `${profileId}.bin`),
563-
undefined,
564-
true,
565-
)) !== null;
566-
}
567-
}
568-
569440
class UnavailableSecretBackend implements ByokSecretBackend {
570441
readonly kind: string;
571442

@@ -574,70 +445,28 @@ class UnavailableSecretBackend implements ByokSecretBackend {
574445
}
575446

576447
async available() { return false; }
577-
async set() { throw new Error('Secure credential storage is unavailable on this system.'); }
578-
async get() { return null; }
579-
async delete() { return false; }
448+
async set(_profileId: string, _secret: string) {
449+
throw new Error('Secure credential storage is unavailable on this system.');
450+
}
451+
async get(_profileId: string) { return null; }
452+
async delete(_profileId: string) { return false; }
580453
}
581454

582-
async function runWindowsDpapiCommand(
583-
operation: 'probe' | 'set' | 'get' | 'delete',
584-
secretPath: string,
585-
secretInput?: string,
586-
allowNotFound = false,
587-
): Promise<string | null> {
588-
return new Promise((resolve, reject) => {
589-
const child = spawn('powershell.exe', [
590-
'-NoLogo',
591-
'-NoProfile',
592-
'-NonInteractive',
593-
'-ExecutionPolicy',
594-
'Bypass',
595-
'-EncodedCommand',
596-
WINDOWS_DPAPI_ENCODED_SCRIPT,
597-
], {
598-
env: {
599-
...process.env,
600-
OD_BYOK_DPAPI_OPERATION: operation,
601-
OD_BYOK_DPAPI_PATH: secretPath,
602-
},
603-
stdio: ['pipe', 'pipe', 'pipe'],
604-
windowsHide: true,
605-
});
606-
const stdout: Buffer[] = [];
607-
let stdoutBytes = 0;
608-
let settled = false;
609-
const finish = (error: Error | null, output?: string | null) => {
610-
if (settled) return;
611-
settled = true;
612-
if (error) reject(error);
613-
else resolve(output === undefined ? '' : output);
614-
};
615-
child.stdout.on('data', (chunk: Buffer) => {
616-
stdoutBytes += chunk.length;
617-
if (stdoutBytes <= MAX_SECRET_OUTPUT_BYTES) stdout.push(chunk);
618-
});
619-
child.stderr.resume();
620-
child.on('error', () => {
621-
finish(new Error('Secure credential backend command failed.'));
622-
});
623-
child.on('close', (code) => {
624-
if (stdoutBytes > MAX_SECRET_OUTPUT_BYTES) {
625-
finish(new Error('Secure credential backend command failed.'));
626-
return;
627-
}
628-
if (code === 0) {
629-
finish(null, Buffer.concat(stdout).toString('utf8'));
630-
return;
631-
}
632-
if (allowNotFound && code === 44) {
633-
finish(null, null);
634-
return;
635-
}
636-
finish(new Error('Secure credential backend command failed.'));
637-
});
638-
if (secretInput === undefined) child.stdin.end();
639-
else child.stdin.end(secretInput);
640-
});
455+
class RetiredWindowsSecretCleanupBackend extends UnavailableSecretBackend {
456+
constructor(private readonly secretsDir: string) {
457+
super('win32');
458+
}
459+
460+
override async delete(profileId: string) {
461+
assertProfileId(profileId);
462+
try {
463+
await unlink(path.join(this.secretsDir, `${profileId}.bin`));
464+
return true;
465+
} catch (error) {
466+
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return false;
467+
throw error;
468+
}
469+
}
641470
}
642471

643472
async function commandAvailable(command: string): Promise<boolean> {

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

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,45 @@ describe('BYOK credential service', () => {
8888
})).rejects.toThrow(/secure credential storage is unavailable/i);
8989
});
9090

91-
it('dispatches native Windows credentials to a DPAPI backend rooted in OD_DATA_DIR', async () => {
92-
const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-windows-dispatch-'));
91+
it('fails closed when Windows has no supported secure credential backend', async () => {
92+
const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-'));
9393
roots.push(dataDir);
94-
9594
const backend = createPlatformByokSecretBackend('win32', dataDir);
9695

97-
expect(backend.kind).toBe('windows-dpapi');
96+
expect(backend.kind).toBe('unavailable-win32');
97+
await expect(backend.available()).resolves.toBe(false);
98+
});
99+
100+
it('deletes a retired Windows DPAPI blob with its profile metadata', async () => {
101+
const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-'));
102+
roots.push(dataDir);
103+
const profileId = 'byok-retired-windows';
104+
const byokDir = path.join(dataDir, 'byok');
105+
const secretsDir = path.join(byokDir, 'secrets');
106+
const secretPath = path.join(secretsDir, `${profileId}.bin`);
107+
await mkdir(secretsDir, { recursive: true });
108+
await writeFile(path.join(byokDir, 'profiles.json'), JSON.stringify({
109+
version: 1,
110+
profiles: [{
111+
id: profileId,
112+
label: 'Retired Windows profile',
113+
protocol: 'openai',
114+
baseUrl: 'https://example.test/v1',
115+
model: 'model',
116+
requiresApiKey: true,
117+
createdAt: 1,
118+
updatedAt: 1,
119+
}],
120+
}));
121+
await writeFile(secretPath, 'retired-dpapi-blob');
122+
const service = new ByokCredentialService({
123+
dataDir,
124+
backend: createPlatformByokSecretBackend('win32', dataDir),
125+
});
126+
127+
await expect(service.delete(profileId)).resolves.toBe(true);
128+
await expect(service.get(profileId)).resolves.toBeNull();
129+
await expect(readFile(secretPath)).rejects.toMatchObject({ code: 'ENOENT' });
98130
});
99131

100132
it('serializes concurrent metadata mutations so profiles cannot overwrite each other', async () => {

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

Lines changed: 0 additions & 64 deletions
This file was deleted.

0 commit comments

Comments
 (0)