fix(orcad): run the remote daemon at the login umask so worktrees it creates are not owner-only (#31135)

* fix(orcad): run the remote daemon at the login umask so worktrees it creates are not owner-only (#31125)

* fix(orcad): keep refreshed Gemini OAuth tokens owner-only now that orcad runs at the login umask

The Gemini usage fetcher rewrote ~/.gemini/oauth_creds.json through a temp file and rename with
no mode, so the replacement took the process umask. Under the old inherited umask 077 that
happened to be 0600 on SSH hosts; at the login umask it would be 0644. Write it through
writeCredentialFileAtomic (0600 staging file, owner-only ACL on Windows).

The hostile-host matrix now checks that the deployed orcad runs at the SSH login's umask and that
its readiness file stays 0600.

* fix(orcad): keep rewritten agent configs at their mode and create the launch PID file owner-only

writeHooksJson now keeps an existing config's mode unless a caller opts out. Rewriting
~/.claude/settings.json (which can hold API keys) and the other agent configs through a temp file
used to take the process umask; inside orcad that was 077, and at the login umask it would widen an
owner-only file to 0644.

The launch runs `umask 077` in the foreground shell rather than inside the backgrounded nohup
list, so the PID file the foreground writes is created 0600 instead of relying on a racing chmod.

The Gemini credential test now pins umask 022 rather than inheriting the runner's.

* fix(agent-hooks): do not create a read-only temp file for a read-only Windows config

A preserved Windows mode can be read-only, which would leave an undeletable temp file behind when
the rename fails. POSIX keeps the preserved mode.

---------

Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
Jinjing
2026-10-20 13:39:32 -07:00
committed by GitHub
co-authored by Neil
parent 177acb83f7
commit 0a5a7b1446
12 changed files with 190 additions and 36 deletions
@@ -120,6 +120,18 @@ describe('writeHooksJson', () => {
expect(existsSync(targetPath)).toBe(false)
})
it.skipIf(process.platform === 'win32')(
'keeps an owner-only config owner-only when it rewrites it',
() => {
writeFileSync(configPath, '{"hooks":{}}\n')
chmodSync(configPath, 0o600)
writeHooksJson(configPath, { hooks: { Stop: [] } })
expect(statSync(configPath).mode & 0o777).toBe(0o600)
}
)
it('writes the config as formatted JSON', () => {
const config: HooksConfig = {
hooks: { Stop: [{ hooks: [{ type: 'command', command: 'foo' }] }] }
+6 -2
View File
@@ -310,8 +310,10 @@ export function writeHooksJson(
// Why randomUUID: avoids tmp-path collisions when two install() calls fire in the same millisecond.
const tmpPath = join(dir, `.${Date.now()}-${randomUUID()}.tmp`)
const serialized = options?.serialized ?? `${JSON.stringify(config, null, 2)}\n`
// Why preserve by default: a rewrite must not widen an owner-only config (settings.json can
// hold API keys) to the process umask, which on SSH hosts is the login's (#31125).
const existingMode =
options?.preserveMode === true && existsSync(writePath)
options?.preserveMode !== false && existsSync(writePath)
? statSync(writePath).mode & 0o777
: options?.defaultMode
@@ -330,7 +332,9 @@ export function writeHooksJson(
}
try {
writeFileSync(tmpPath, serialized, { encoding: 'utf-8', mode: existingMode, flag: 'wx' })
// Why not on Windows: a read-only config's mode would make the temp file undeletable on failure.
const tmpMode = process.platform === 'win32' ? undefined : existingMode
writeFileSync(tmpPath, serialized, { encoding: 'utf-8', mode: tmpMode, flag: 'wx' })
if (existingMode !== undefined && process.platform !== 'win32') {
// Preserve requested permissions even with a stricter process umask.
chmodSync(tmpPath, existingMode)
@@ -91,6 +91,22 @@ describe.skipIf(process.platform === 'win32')('orcad launch file modes', () => {
expect(mode(baseDir)).toBe(0o700)
})
// Regression (#31125): git, terminals and agents the daemon spawns create the user's files.
it('runs the daemon itself at the login umask', async () => {
writeFileSync(
join(slotDir, 'orcad.js'),
`mkdir daemon-dir && : > daemon-file && printf '%s\\n' '${READY_LINE}'\n`
)
await launchUnderLoginUmask()
expect(mode(join(slotDir, 'daemon-file'))).toBe(0o644)
expect(mode(join(slotDir, 'daemon-dir'))).toBe(0o755)
expect(mode(join(slotDir, ORCAD_READINESS_FILENAME))).toBe(0o600)
expect(mode(join(slotDir, ORCAD_PID_FILENAME))).toBe(0o600)
expect(mode(join(slotDir, ORCAD_LOG_FILENAME))).toBe(0o600)
})
it('leaves a parent that is not ~/.orca-remote alone', async () => {
slotDir = join(home, 'custom-slot')
mkdirSync(slotDir, { mode: 0o755 })
@@ -1,5 +1,5 @@
import { execFileSync } from 'node:child_process'
import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { chmodSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, describe, expect, it } from 'vitest'
@@ -47,7 +47,7 @@ describe('the WSL launch (setsid)', () => {
it('has the child record its own PID, since the setsid -f parent exits at once', () => {
const command = orcadLaunchCommand(host, spec('/d/orcad-1'), 'setsid')
const removeStale = command.indexOf(`rm -f '/d/orcad-1/${ORCAD_PID_FILENAME}'`)
const recordOwn = command.indexOf(`echo $$ > "$1" && shift && exec "$@"`)
const recordOwn = command.indexOf(`echo $$ > "$1" && umask "$2" && shift 2 && exec "$@"`)
expect(removeStale).toBeGreaterThan(-1)
expect(recordOwn).toBeGreaterThan(removeStale)
expect(command.trimEnd().endsWith(`cat '/d/orcad-1/${ORCAD_PID_FILENAME}'`)).toBe(true)
@@ -106,4 +106,25 @@ describe('the WSL launch (setsid)', () => {
}
}
)
// Regression (#31125): git, terminals and agents the daemon spawns create the user's files.
it.skipIf(process.platform !== 'linux')(
'runs the daemon at the login umask while its PID and readiness files stay owner-only',
() => {
const dir = mkdtempSync(join(tmpdir(), 'orcad-setsid-umask-'))
dirs.push(dir)
const runtime = join(dir, 'fake-node')
writeFileSync(runtime, `#!/bin/sh\n: > daemon-file\necho '{"type":"orca_server_ready"}'\n`)
chmodSync(runtime, 0o755)
const command = orcadLaunchCommand(host, spec(dir, runtime), 'setsid')
execFileSync('sh', ['-c', `umask 022; ${command}`])
for (let i = 0; i < 50 && !readFileSync(join(dir, ORCAD_READINESS_FILENAME), 'utf8'); i++) {
execFileSync('sleep', ['0.05'])
}
const mode = (name: string): number => statSync(join(dir, name)).mode & 0o777
expect(mode('daemon-file')).toBe(0o644)
expect(mode(ORCAD_PID_FILENAME)).toBe(0o600)
expect(mode(ORCAD_READINESS_FILENAME)).toBe(0o600)
}
)
})
@@ -51,9 +51,11 @@ describe('orcadLaunchCommand', () => {
it('truncates the readiness file, so a stale line cannot be activated on', () => {
const command = orcadLaunchCommand(posix, SPEC)
const truncate = command.indexOf(`: > '${SPEC.remoteInstallDir}/${ORCAD_READINESS_FILENAME}'`)
const cd = command.indexOf(`cd '${SPEC.remoteInstallDir}'`)
const truncate = command.indexOf(`: > '${ORCAD_READINESS_FILENAME}'`)
const launch = command.indexOf('nohup')
expect(truncate).toBeGreaterThan(-1)
expect(cd).toBeGreaterThan(-1)
expect(truncate).toBeGreaterThan(cd)
expect(truncate).toBeLessThan(launch)
})
@@ -100,11 +100,13 @@ export function orcadLaunchCommand(
): string {
assertPosixHost(host)
const dir = quotePosixShell(spec.remoteInstallDir)
const readiness = quotePosixShell(
joinRemotePath(host, spec.remoteInstallDir, ORCAD_READINESS_FILENAME)
)
const log = quotePosixShell(joinRemotePath(host, spec.remoteInstallDir, ORCAD_LOG_FILENAME))
// Relative to the `cd` below, which every use follows in the same shell: the slot path is most
// of the sent bytes.
const readiness = quotePosixShell(ORCAD_READINESS_FILENAME)
const log = quotePosixShell(ORCAD_LOG_FILENAME)
// Absolute: a nohup launch backgrounds the whole list, `cd` included, then writes this outside it.
const pidFile = quotePosixShell(joinRemotePath(host, spec.remoteInstallDir, ORCAD_PID_FILENAME))
// Absolute: liveness matches the runtime's args against this path.
const entry = quotePosixShell(
joinRemotePath(host, spec.remoteInstallDir, ORCAD_LAUNCHER_FILENAME)
)
@@ -116,11 +118,19 @@ export function orcadLaunchCommand(
]
// Why in the launching shell: the detached list below runs in a subshell whose variables are lost.
const recordSession = detach === 'nohup' && recordsOrcadLaunchSshSession(host)
const launchEnv = [
`ORCA_VERSION=${quotePosixShell(spec.fullVersion)}`,
`ORCA_USER_DATA=${quotePosixShell(spec.userDataDir)}`,
...orcadManagedLaunchEnv(spec).map(([name, value]) => `${name}=${quotePosixShell(value)}`)
]
return [
...(recordSession ? [captureOrcadLaunchSshSessionCommand()] : []),
// Why first: the readiness file carries the pairing offer's device token, so every file this
// launch creates must be owner-only from birth, not only the ones after the exec.
'umask 077 &&',
// The daemon gets this back: the git, terminals and agents it spawns create the user's files.
'orcad_login_umask=$(umask);',
// Why before anything is created: the readiness file carries the pairing offer's device token,
// so every file this launch creates must be owner-only from birth. Why `;`: a nohup launch
// backgrounds the `&&` list below, and the PID file is written outside it.
'umask 077;',
// Best effort: directories and files an earlier build left 0755/0644 keep their old modes.
`{ chmod 700 ${privateDirs.map(quotePosixShell).join(' ')} 2>/dev/null || :; } &&`,
`cd ${dir} &&`,
@@ -132,19 +142,19 @@ export function orcadLaunchCommand(
`chmod 600 ${readiness} &&`,
`{ chmod 600 ${pidFile} ${log} 2>/dev/null || :; } &&`,
// A stop request the previous process never consumed must not stop this one.
`rm -f ${quotePosixShell(joinRemotePath(host, spec.remoteInstallDir, ORCAD_STOP_REQUEST_FILENAME))} &&`,
`rm -f ${quotePosixShell(ORCAD_STOP_REQUEST_FILENAME)} &&`,
...(detach === 'setsid' ? [`rm -f ${pidFile} &&`] : []),
`ORCA_VERSION=${quotePosixShell(spec.fullVersion)}`,
`ORCA_USER_DATA=${quotePosixShell(spec.userDataDir)}`,
...orcadManagedLaunchEnv(spec).map(([name, value]) => `${name}=${quotePosixShell(value)}`),
...(detach === 'setsid'
? setsidLaunchTail(entry, spec, readiness, log, pidFile, hostEnv)
? setsidLaunchTail(entry, spec, readiness, log, pidFile, launchEnv, hostEnv)
: [
// Why a group: its redirects open under 077 before the daemon gets the login umask back.
'{ umask "$orcad_login_umask" &&',
...launchEnv,
// Keep $! equal to the runtime PID rather than a waiting shell's PID.
// Older clients identify the live PID by orcad.js; the pinned launcher loads the body in-place.
`exec nohup "$orcad_runtime" ${entry}`,
`--json --bind ${quotePosixShell(spec.bindHost)} --port ${String(spec.port)}`,
`> ${readiness} 2>> ${log} < /dev/null &`,
`--json --bind ${quotePosixShell(spec.bindHost)} --port ${String(spec.port)};`,
`} > ${readiness} 2>> ${log} < /dev/null &`,
`echo $! > ${pidFile} &&`,
...(recordSession
? [`${writeOrcadLaunchSshSessionCommand(host, spec.userDataDir)} &&`]
@@ -165,15 +175,19 @@ function setsidLaunchTail(
readiness: string,
log: string,
pidFile: string,
launchEnv: readonly string[],
hostEnv: readonly OrcadLaunchEnvEntry[]
): string[] {
return [
...launchEnv,
// Only a setsid (WSL host) launch marks itself; SSH keeps its byte-for-byte command.
`${ORCAD_WSL_HOST_ENV}=1`,
...hostEnv.map(([name, value]) => `${assertEnvName(name)}=${quotePosixShell(value)}`),
'sh -c \'orcad_detach() { if setsid -f true >/dev/null 2>&1; then setsid -f "$@"; ' +
'else setsid "$@" & fi; }; orcad_detach "$@"\' orcad-detach',
`sh -c 'echo $$ > "$1" && shift && exec "$@"' orcad-launch ${pidFile}`,
// The PID file is written under 077 before the daemon gets the login umask back.
`sh -c 'echo $$ > "$1" && umask "$2" && shift 2 && exec "$@"' orcad-launch ${pidFile}`,
'"$orcad_login_umask"',
`"$orcad_runtime" ${entry}`,
`--json --bind ${quotePosixShell(spec.bindHost)} --port ${String(spec.port)}`,
`> ${readiness} 2>> ${log} < /dev/null &&`,
@@ -98,8 +98,7 @@ const POSIX_HOST_OP_BYTE_BUDGET = 6 * 1024
// Ops already past the budget, at their measured size. Ceilings may only shrink.
const OVER_BUDGET_CEILINGS: Readonly<Record<string, number>> = {
tryStealInstallLockCommand: 9_022,
orcadLaunchCommand: 8_122,
'orcadLaunchCommand (WSL setsid)': 6_439,
orcadLaunchCommand: 7_130,
probeRemoteInstallCompleteCommand: 12_189
}
@@ -0,0 +1,51 @@
import {
chmodSync,
mkdirSync,
mkdtempSync,
readFileSync,
rmSync,
statSync,
writeFileSync
} from 'node:fs'
import type * as NodeOs from 'node:os'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
const state = vi.hoisted(() => ({ home: '' }))
vi.mock('node:os', async (importOriginal) => ({
...(await importOriginal<typeof NodeOs>()),
homedir: () => state.home
}))
const CREDS = { access_token: 'access', refresh_token: 'refresh', expiry_date: 1 }
describe.skipIf(process.platform === 'win32')('saveGeminiCredentials', () => {
let previousUmask = 0
beforeAll(() => {
previousUmask = process.umask(0o022)
state.home = mkdtempSync(join(tmpdir(), 'gemini-oauth-sources-'))
})
afterAll(() => {
process.umask(previousUmask)
rmSync(state.home, { recursive: true, force: true })
})
// Regression (#31125): orcad no longer inherits umask 077, so the replacement must be born 0600.
it('keeps the refreshed OAuth tokens owner-only under a permissive umask', async () => {
// Imported here: the module resolves the credential path from homedir() at load.
const { saveGeminiCredentials } = await import('./gemini-oauth-sources')
const path = join(state.home, '.gemini', 'oauth_creds.json')
mkdirSync(join(state.home, '.gemini'))
writeFileSync(path, '{}')
chmodSync(path, 0o600)
await saveGeminiCredentials(CREDS)
expect(statSync(path).mode & 0o777).toBe(0o600)
expect(JSON.parse(readFileSync(path, 'utf8'))).toEqual(CREDS)
})
})
+5 -4
View File
@@ -1,8 +1,9 @@
import { readFile, writeFile, rename } from 'node:fs/promises'
import { readFile } from 'node:fs/promises'
import { homedir } from 'node:os'
import path from 'node:path'
import { getMainHttpClient } from '../network/http-client'
import { extractOAuthClientCredentials } from './gemini-cli-oauth-extractor'
import { writeCredentialFileAtomic } from '../integration-credential-file'
const API_TIMEOUT_MS = 10_000
const OAUTH_CREDS_PATH = path.join(homedir(), '.gemini', 'oauth_creds.json')
@@ -77,10 +78,10 @@ export async function readGeminiCredentials(): Promise<GeminiCredentials | null>
}
}
// Why an explicit owner-only mode: the rename replaces Gemini's 0600 file, and orcad runs at the
// login umask (#31125).
export async function saveGeminiCredentials(creds: GeminiCredentials): Promise<void> {
const tmpPath = `${OAUTH_CREDS_PATH}.${process.pid}.tmp`
await writeFile(tmpPath, JSON.stringify(creds, null, 2), 'utf-8')
await rename(tmpPath, OAUTH_CREDS_PATH)
writeCredentialFileAtomic(OAUTH_CREDS_PATH, Buffer.from(JSON.stringify(creds, null, 2), 'utf-8'))
}
export type RefreshTokenResult = {
@@ -24,12 +24,12 @@ vi.mock('./gemini-cli-oauth-extractor', () => ({
}))
vi.mock('node:fs/promises', () => ({
readFile: readFileMock,
// Why: saveGeminiCredentials is exercised on the refresh path. The atomic
// tmp+rename write has no observable side effect in these tests, so the
// stubs just resolve.
writeFile: vi.fn().mockResolvedValue(undefined),
rename: vi.fn().mockResolvedValue(undefined)
readFile: readFileMock
}))
// Why: the refresh path saves credentials; it must never write the real ~/.gemini.
vi.mock('../integration-credential-file', () => ({
writeCredentialFileAtomic: vi.fn().mockReturnValue(true)
}))
import { fetchGeminiRateLimits } from './gemini-usage-fetcher'
@@ -23,9 +23,12 @@ vi.mock('./gemini-cli-oauth-extractor', () => ({
}))
vi.mock('node:fs/promises', () => ({
readFile: readFileMock,
writeFile: vi.fn().mockResolvedValue(undefined),
rename: vi.fn().mockResolvedValue(undefined)
readFile: readFileMock
}))
// Why: the refresh path saves credentials; it must never write the real ~/.gemini.
vi.mock('../integration-credential-file', () => ({
writeCredentialFileAtomic: vi.fn().mockReturnValue(true)
}))
import { fetchGeminiRateLimits } from './gemini-usage-fetcher'
@@ -23,6 +23,10 @@ import {
parseOrcadLiveness
} from '../../managed-server/install/orcad-remote-launch'
import { execOrcadRemote } from '../../managed-server/install/orcad-remote-runtime-control'
import {
ORCAD_PID_FILENAME,
ORCAD_READINESS_FILENAME
} from '../../managed-server/install/orcad-remote-host-support'
import { decommissionRemoteOrcad } from '../../managed-server/install/orcad-remote-decommission'
import { quotePosixShell } from '../../../shared/shell-argument-quoting'
import {
@@ -111,6 +115,7 @@ async function assertLiveThenDecommissioned(
const liveness = async (): Promise<string> =>
parseOrcadLiveness(await execOrcadRemote(slot, orcadLivenessProbeCommand(slot.host, slotDir)))
expect(await liveness()).toBe('LIVE')
await assertOrcadRunsAtLoginUmask(slot, slotDir)
const decommission = await decommissionRemoteOrcad({
...slot,
record: await readOrcadActivationRecord(slot),
@@ -123,6 +128,32 @@ async function assertLiveThenDecommissioned(
expect(await liveness()).toBe('DEAD')
}
/**
* #31125: orcad runs at the SSH login's umask, so the git, terminals and agents it spawns create
* the user's files, while the readiness file holding the pairing token stays owner-only.
* Linux only: macOS has no /proc to read another process's umask from.
*/
async function assertOrcadRunsAtLoginUmask(slot: OrcadSlotOptions, slotDir: string): Promise<void> {
if (slot.host.os !== 'linux') {
return
}
const pidFile = quotePosixShell(posix.join(slotDir, ORCAD_PID_FILENAME))
const readiness = quotePosixShell(posix.join(slotDir, ORCAD_READINESS_FILENAME))
const [login, daemon, readinessMode] = (
await execOrcadRemote(
slot,
`umask; sed -n 's/^Umask:[[:space:]]*//p' "/proc/$(cat ${pidFile})/status"; stat -c %a ${readiness}`
)
)
.trim()
.split('\n')
.map((line) => line.trim())
expect(Number.parseInt(daemon ?? '', 8), `login ${login}, orcad ${daemon}`).toBe(
Number.parseInt(login ?? '', 8)
)
expect(readinessMode).toBe('600')
}
/** An empty runtime cache and a refusing network on this computer, until `restore`. */
function refuseDesktopDownloads(): {
cacheRoot: string