fix(orcad): seal integration credentials on the headless server inste… · tmchow/orca@3074852 · GitHub
Skip to content

Commit 3074852

Browse files
nwparkernwparker
authored
fix(orcad): seal integration credentials on the headless server instead of storing them in plaintext (stablyai#29412)
* fix(orcad): seal integration credentials on the headless server instead of storing them in plaintext The Node SecretStore always reported encryption unavailable, so every credential writer on orcad fell back to plaintext. orcad now seals with AES-256-GCM using a host-owned master key kept in the OS keyring (macOS Keychain, Linux Secret Service) when reachable at first start, else in a 0600 profile file bound to the machine ID. A key that cannot load fails closed instead of downgrading. Existing plaintext credentials are sealed on startup without loss. * docs: describe how the headless server seals credentials and its limits * fix(orcad): unique credential staging files, no keyring items for temp profiles, isolate startup test * fix(orcad): keep the credential key with the credentials, one record per machine * fix(orcad): never re-key over an unreadable record, serialize first-start key creation, refuse an unrestricted key file * fix(orcad): verify a key record's Windows ACL and wait out a concurrent first start's keyring write * fix(orcad): keep a keyring claim and fail closed when its store result is uncertain * fix(credentials): restrict the staged file's Windows ACL before it replaces a credential * fix(orcad): a temporary profile never creates a key in, or re-seals, the real credential home * ci: baseline the two single-file credential ACL restrictions * fix(credentials): stage Windows credentials born restricted and verify the key record asynchronously; document rollback * test(bitbucket): inject the write failure into the Windows staging writer too
1 parent 9b8a29c commit 3074852

28 files changed

Lines changed: 1591 additions & 80 deletions

‎docs/reference/headless-secret-storage.md‎

Lines changed: 64 additions & 0 deletions

‎src/main/bitbucket/credential-store.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { existsSync, mkdtempSync, statSync } from 'node:fs'
1+
import { existsSync, mkdtempSync, readdirSync, statSync } from 'node:fs'
22
import type * as Fs from 'node:fs'
33
import { tmpdir } from 'node:os'
44
import type * as Os from 'node:os'
5+
import type * as WindowsAcl from '../../shared/secure-path-windows-acl'
56
import { join } from 'node:path'
67
import { beforeEach, describe, expect, it, vi } from 'vitest'
78

@@ -19,6 +20,7 @@ async function loadStore(
1920
// Why: doMock registrations outlive resetModules, so an injected failure from
2021
// one case would leak into every later one in this file.
2122
vi.doUnmock('node:fs')
23+
vi.doUnmock('../../shared/secure-path-windows-acl')
2224
const { setSecretStore } = await import('../../shared/secret-store')
2325
setSecretStore({
2426
isEncryptionAvailable: () => true,
@@ -53,6 +55,13 @@ async function loadStore(
5355
}
5456
}
5557
})
58+
// Why: on Windows the staged credential is written by the ACL helper, not writeSync.
59+
vi.doMock('../../shared/secure-path-windows-acl', async (importOriginal) => ({
60+
...(await importOriginal<typeof WindowsAcl>()),
61+
writeRestrictedWindowsFile: () => {
62+
throw error
63+
}
64+
}))
5665
}
5766
if (options.unlinkError) {
5867
// Why mocked: chmod-based failure injection is not portable — Windows has no
@@ -235,7 +244,7 @@ describe('Bitbucket credential store', () => {
235244
).toThrow(/disk full/)
236245

237246
expect(readFileSync(join(tempHome, '.orca', 'bitbucket-credential.enc'))).toEqual(before)
238-
expect(existsSync(join(tempHome, '.orca', 'bitbucket-credential.enc.tmp'))).toBe(false)
247+
expect(readdirSync(join(tempHome, '.orca')).filter((name) => name.endsWith('.tmp'))).toEqual([])
239248
})
240249

241250
it('authenticates from the envelope when metadata is stale (STA-3941)', async () => {

‎src/main/bitbucket/credential-store.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ function getMetadataPath(): string {
5858
return join(getOrcaDir(), 'bitbucket-credential.json')
5959
}
6060

61-
function getSecretPath(): string {
61+
export function getSecretPath(): string {
6262
return join(getOrcaDir(), 'bitbucket-credential.enc')
6363
}
6464

‎src/main/credentials/encrypted-api-key-file-store.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import type { SecretAtRestProtection } from '../../shared/secret-at-rest-protect
1111
import { ApiKeyFileUnreadableError } from './api-key-file-unreadable-error'
1212

1313
type EncryptedApiKeyFileStore = {
14+
/** Where the envelope lives and how it starts, for orcad's plaintext migration. */
15+
envelopeFile: () => { path: string; envelopePrefix: string }
1416
protection: () => SecretAtRestProtection | null
1517
has: () => boolean
1618
save: (key: string) => void
@@ -173,5 +175,12 @@ export function createEncryptedApiKeyFileStore({
173175
rmSync(getApiKeyPath(), { force: true })
174176
}
175177

176-
return { has, save, read, clear, protection }
178+
return {
179+
has,
180+
save,
181+
read,
182+
clear,
183+
protection,
184+
envelopeFile: () => ({ path: getApiKeyPath(), envelopePrefix })
185+
}
177186
}

‎src/main/integration-credential-file-windows-acl.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
2+
import { tmpdir } from 'node:os'
3+
import { join } from 'node:path'
4+
import { afterEach, describe, expect, it, vi } from 'vitest'
5+
6+
const created = vi.hoisted((): string[] => [])
7+
vi.mock('../shared/secure-path-windows-acl', () => ({
8+
writeRestrictedWindowsFile: (path: string, contents: string | Buffer) => {
9+
created.push(path)
10+
writeFileSync(path, contents)
11+
return true
12+
}
13+
}))
14+
15+
describe('writeCredentialFileAtomic on Windows', () => {
16+
const platform = process.platform
17+
afterEach(() => {
18+
Object.defineProperty(process, 'platform', { value: platform })
19+
})
20+
21+
it('stages the replacement as a born-restricted file, so the owner-only ACL survives', async () => {
22+
const { writeCredentialFileAtomic } = await import('./integration-credential-file')
23+
const dir = mkdtempSync(join(tmpdir(), 'credential-acl-'))
24+
try {
25+
Object.defineProperty(process, 'platform', { value: 'win32' })
26+
const target = join(dir, 'linear-token.enc')
27+
expect(writeCredentialFileAtomic(target, Buffer.from([0, 1, 2, 3]))).toBe(true)
28+
expect(created).toHaveLength(1)
29+
expect(created[0]).toMatch(/linear-token\.enc\..+\.tmp$/)
30+
expect(readFileSync(target)).toEqual(Buffer.from([0, 1, 2, 3]))
31+
} finally {
32+
Object.defineProperty(process, 'platform', { value: platform })
33+
rmSync(dir, { recursive: true, force: true })
34+
}
35+
})
36+
})

‎src/main/integration-credential-file.ts‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { randomBytes } from 'node:crypto'
12
import {
23
chmodSync,
34
closeSync,
@@ -9,6 +10,8 @@ import {
910
writeSync
1011
} from 'node:fs'
1112
import { getSecretStore } from '../shared/secret-store'
13+
import { fsyncFileSync } from '../shared/secure-file'
14+
import { writeRestrictedWindowsFile } from '../shared/secure-path-windows-acl'
1215
import {
1316
credentialDecryptionMessage,
1417
type IntegrationCredentialService
@@ -45,10 +48,22 @@ export function writeEncryptedCredential(
4548
// fails or the app dies mid-write, leaving nothing to authenticate with. Write
4649
// a sibling temp file, fsync it, then rename — readers only ever observe the
4750
// old bytes or the complete new ones.
48-
export function writeCredentialFileAtomic(path: string, data: Buffer): void {
49-
const tempPath = `${path}.tmp`
51+
export function writeCredentialFileAtomic(
52+
path: string,
53+
data: Buffer,
54+
shouldPublish?: () => boolean
55+
): boolean {
56+
// Why unique: a concurrent writer sharing one temp name could truncate the bytes we publish.
57+
const tempPath = `${path}.${process.pid}.${randomBytes(4).toString('hex')}.tmp`
5058
let handle: number | null = null
5159
try {
60+
if (process.platform === 'win32') {
61+
// Why born restricted: a sibling would otherwise inherit the directory's DACL and replace
62+
// the previous credential's owner-only ACL.
63+
writeRestrictedWindowsFile(tempPath, data)
64+
fsyncFileSync(tempPath)
65+
return publishStagedCredential(tempPath, path, shouldPublish)
66+
}
5267
handle = openSync(tempPath, 'w', 0o600)
5368
// Why: write(2) is allowed to return a short count, so a single writeSync
5469
// could publish a truncated credential — the very corruption this avoids.
@@ -63,8 +78,7 @@ export function writeCredentialFileAtomic(path: string, data: Buffer): void {
6378
fsyncSync(handle)
6479
closeSync(handle)
6580
handle = null
66-
renameSync(tempPath, path)
67-
restrictCredentialFileToOwner(path)
81+
return publishStagedCredential(tempPath, path, shouldPublish)
6882
} catch (error) {
6983
if (handle !== null) {
7084
try {
@@ -82,9 +96,23 @@ export function writeCredentialFileAtomic(path: string, data: Buffer): void {
8296
}
8397
}
8498

99+
function publishStagedCredential(
100+
tempPath: string,
101+
path: string,
102+
shouldPublish: (() => boolean) | undefined
103+
): boolean {
104+
if (shouldPublish && !shouldPublish()) {
105+
unlinkSync(tempPath)
106+
return false
107+
}
108+
renameSync(tempPath, path)
109+
restrictCredentialFileToOwner(path)
110+
return true
111+
}
112+
85113
// Why: writeFileSync's `mode` only applies when it creates the file, so
86114
// rewriting an existing credential would silently keep looser permissions.
87-
// Best-effort: Windows has no POSIX modes, so chmod is a documented no-op there.
115+
// Best-effort: Windows has no POSIX modes; its staging file is created owner-only instead.
88116
export function restrictCredentialFileToOwner(path: string): void {
89117
try {
90118
chmodSync(path, 0o600)

‎src/main/jira/site-credential-store.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs'
22
import { homedir } from 'node:os'
33
import { join } from 'node:path'
4-
import { getSecretStore } from '../../shared/secret-store'
4+
import { writeEncryptedCredential } from '../integration-credential-file'
55
import { readCredentialFileProtection } from '../credential-file-protection'
66
import type { SecretAtRestProtection } from '../../shared/secret-at-rest-protection'
77
import type { JiraSite, JiraSiteSelection } from '../../shared/jira-types'
@@ -34,7 +34,7 @@ function getSiteFilePath(): string {
3434
return join(getOrcaDir(), 'jira-sites.json')
3535
}
3636

37-
function getTokenDir(): string {
37+
export function getTokenDir(): string {
3838
return join(getOrcaDir(), 'jira-tokens')
3939
}
4040

@@ -168,15 +168,6 @@ export function writeSiteFile(file: JiraSiteFile): void {
168168
siteFileCache.invalidate()
169169
}
170170

171-
function writeEncryptedToken(path: string, apiToken: string): void {
172-
if (getSecretStore().isEncryptionAvailable()) {
173-
writeFileSync(path, getSecretStore().encryptString(apiToken), { mode: 0o600 })
174-
return
175-
}
176-
console.warn('[jira] secret encryption unavailable — storing token in plaintext')
177-
writeFileSync(path, apiToken, { encoding: 'utf-8', mode: 0o600 })
178-
}
179-
180171
export function readToken(siteId: string): string | null {
181172
return tokenFileCache.read(siteId)
182173
}
@@ -189,7 +180,7 @@ export function getSiteTokenProtection(siteId: string): SecretAtRestProtection |
189180
export function saveToken(siteId: string, apiToken: string): void {
190181
ensureOrcaDir()
191182
ensureTokenDir()
192-
writeEncryptedToken(getTokenPath(siteId), apiToken)
183+
writeEncryptedCredential('Jira', getTokenPath(siteId), apiToken)
193184
// The next action reads the actual file, including any overlapping writer's replacement.
194185
tokenFileCache.forget(siteId)
195186
}

‎src/main/linear/linear-credential-paths.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export function getWorkspaceFilePath(): string {
2020
return join(getOrcaDir(), 'linear-workspaces.json')
2121
}
2222

23-
function getWorkspaceTokenDir(): string {
23+
export function getWorkspaceTokenDir(): string {
2424
return join(getOrcaDir(), 'linear-tokens')
2525
}
2626

‎src/main/linear/linear-token-store.ts‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { getSecretStore } from '../../shared/secret-store'
2-
import { unlinkSync, writeFileSync } from 'node:fs'
1+
import { unlinkSync } from 'node:fs'
2+
import { writeEncryptedCredential } from '../integration-credential-file'
33
import {
44
LEGACY_WORKSPACE_ID,
55
ensureOrcaDir,
@@ -26,24 +26,14 @@ import type { LinearWorkspace } from '../../shared/linear/workspace-types'
2626
import { readCredentialFileProtection } from '../credential-file-protection'
2727
import type { SecretAtRestProtection } from '../../shared/secret-at-rest-protection'
2828

29-
function writeEncryptedToken(path: string, apiKey: string): void {
30-
if (getSecretStore().isEncryptionAvailable()) {
31-
const encrypted = getSecretStore().encryptString(apiKey)
32-
writeFileSync(path, encrypted, { mode: 0o600 })
33-
return
34-
}
35-
36-
console.warn('[linear] secret encryption unavailable — storing token in plaintext')
37-
writeFileSync(path, apiKey, { encoding: 'utf-8', mode: 0o600 })
38-
}
39-
4029
export function saveWorkspaceToken(workspaceId: string, apiKey: string): void {
4130
ensureOrcaDir()
4231
if (workspaceId !== LEGACY_WORKSPACE_ID) {
4332
ensureWorkspaceTokenDir()
4433
}
4534
const tokenPath = getWorkspaceTokenPath(workspaceId)
46-
writeEncryptedToken(tokenPath, apiKey)
35+
// Why the shared writer: it re-applies 0600 to an existing file and never truncates it mid-write.
36+
writeEncryptedCredential('Linear', tokenPath, apiKey)
4737
workspaceTokenFileCache.forget(workspaceId)
4838
}
4939

‎src/main/minimax/minimax-api-key-store.ts‎

Lines changed: 1 addition & 0 deletions

0 commit comments

Comments
 (0)