From fb9b6affb6b0a2e21e214d077546a512725c024c Mon Sep 17 00:00:00 2001 From: Muqsit Date: Mon, 7 Sep 2026 11:22:43 -0700 Subject: [PATCH 1/2] feat(push): remoteSecretsHome pins the remote secrets to the consumer state root on every transport agents-cli runs its local standalone under SECRETS_HOME=~/.agents and reads pushed bundles from that root on the receiving box, but every remote invocation of pushBundleToHost (import, read-back verify, literal restoration) ran with no env prologue, so the remote defaulted to ~/.secrets: the import reported success and the consumer daemon never saw the bundle. Observed 2026-09-07: zion logged 'pushed __cursor__ (1 key(s)) to yosemite-m0' while the worker kept answering 'no readable durable key on this box yet'. The file store is keyed per root, so the two roots cannot even share ciphertext. PushBundleOptions.remoteSecretsHome names the root; remoteStateRootEnv turns a leading ~/ into $HOME/ (POSIX) or $env:USERPROFILE\ (PowerShell) in the expanding quote form (SECRETS_HOME joins PATH as an expanding env key), and the env rides buildRemoteFileImportCommand, buildWindowsStdinImportCommand, remoteSecretsRaw[Async] and verifyRemoteKeychainPush[Async]. Unset keeps today's behaviour. Version 0.1.2. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_014qq5fiWVRBmBV9f5KR58dY --- CHANGELOG.md | 4 +++ package.json | 2 +- src/lib/secrets/push.test.ts | 24 ++++++++++++++++++ src/lib/secrets/push.ts | 34 ++++++++++++++++++++----- src/lib/secrets/remote.ts | 21 +++++++++------- src/lib/transports/ssh.test.ts | 32 ++++++++++++++++++++++++ src/lib/transports/ssh.ts | 45 +++++++++++++++++++++++++++++++--- 7 files changed, 143 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5868ac7..c826933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.1.2 + +- **A push can pin the remote `secrets` to the consumer's state root (`remoteSecretsHome`), so a bundle lands where the consumer reads it.** agents-cli runs its local standalone under `SECRETS_HOME=~/.agents` (MIG-1) and reads pushed bundles from that same root on the receiving box, but every remote invocation of `pushBundleToHost` — the `import`, the read-back verify, literal restoration — ran with no env prologue, so the remote defaulted to `~/.secrets`, the import reported success, and the consumer's daemon never saw the bundle (observed 2026-09-07: zion logged `pushed __cursor__ (1 key(s)) to yosemite-m0` while the worker kept answering `no readable durable key on this box yet`; the file store is keyed per root, so the two roots cannot even share ciphertext). `PushBundleOptions.remoteSecretsHome` names that root; `remoteStateRootEnv` turns a leading `~/` into `$HOME/` (POSIX) or `$env:USERPROFILE\` (PowerShell), emitted in the expanding quote form (`SECRETS_HOME` joins `PATH` as an expanding env key), and the env rides `buildRemoteFileImportCommand`, `buildWindowsStdinImportCommand`, `remoteSecretsRaw[Async]` (`env` option) and `verifyRemoteKeychainPush[Async]`. Unset keeps today's behaviour. Source: `src/lib/secrets/push.ts`, `src/lib/secrets/remote.ts`, `src/lib/transports/ssh.ts`, and their tests. + ## 0.1.1 - **Bundle names accept the reserved `____` shape, so a consumer-managed store moves through the ordinary bundle transport.** agents-cli keeps one `____` file-backed store per harness for durable worker credentials (`CURSOR_API_KEY_`, `OPENAI_API_KEY_`, …) and pushes it to worker devices with the same `pushBundleToHost` every other bundle uses — which reads the store through `readAndResolveBundleEnv` and imports it remotely with `secrets import --from - --backend file`. Both ends ran `validateBundleName`, whose pattern rejected any name starting with `_`, so every push of a `__cursor__` store failed before the SSH hop with `Invalid bundle name`, surfaced to the daemon only as `Secrets operation failed (OPERATION_FAILED)`, and no worker ever received a Cursor, Codex or Grok worker key. `BUNDLE_NAME_PATTERN` now also matches exactly `____` (a bare leading `__` is still invalid); `create`, `rename`, `add` and `describe` refuse that shape when typed by hand (`assertNotReservedShape`), since the store belongs to the consumer that manages it — a hand `rename --force` onto it would purge its items. Source: `src/lib/secrets/bundles.ts`, `src/lib/secrets/__tests__/bundles.test.ts`, `src/commands/secrets.ts`. diff --git a/package.json b/package.json index 6d3531f..62dfb8c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@phnx-labs/secrets-cli", - "version": "0.1.1", + "version": "0.1.2", "description": "Portable secret bundles, native stores and provider-backed injection", "type": "module", "bin": { diff --git a/src/lib/secrets/push.test.ts b/src/lib/secrets/push.test.ts index bc85ffc..e747489 100644 --- a/src/lib/secrets/push.test.ts +++ b/src/lib/secrets/push.test.ts @@ -94,6 +94,7 @@ describe('planPushTransport — which transport a backend/OS pair selects', () = kind: 'remote-secrets', args: ['import', 'apple.com', '--from', '-'], input: RESOLVED.dotenv, + env: {}, }); }); @@ -144,6 +145,29 @@ describe('planPushTransport — which transport a backend/OS pair selects', () = expect(t.input).toBe(RESOLVED.dotenv); }); + it('pins the remote secrets to the consumer state root on every transport when remoteSecretsHome is named', () => { + // agents-cli runs its local standalone under SECRETS_HOME=~/.agents and reads + // pushed bundles from that root on the worker; a push without this landed in + // the remote's default ~/.secrets, where the consumer never looked. + const file = planPushTransport(RESOLVED, '__cursor__', 'push-test-linux', { remoteBackend: 'file', operation: 'push.test', remoteSecretsHome: '~/.agents' }); + if (file.kind !== 'ssh') throw new Error('unreachable'); + expect(file.remoteCmd).toContain('export SECRETS_HOME="$HOME/.agents"; secrets import'); + expect(file.remoteCmd).not.toContain('SECRETS_PASSPHRASE'); + + const keychain = planPushTransport(RESOLVED, 'apple.com', 'push-test-linux', { remoteBackend: 'keychain', operation: 'push.test', remoteSecretsHome: '~/.agents' }); + if (keychain.kind !== 'remote-secrets') throw new Error('unreachable'); + expect(keychain.env).toEqual({ SECRETS_HOME: '$HOME/.agents' }); + + const win = planPushTransport(RESOLVED, 'apple.com', 'push-test-win', { remoteBackend: 'keychain', operation: 'push.test', remoteSecretsHome: '~/.agents' }); + if (win.kind !== 'ssh') throw new Error('unreachable'); + expect(win.remoteCmd).toContain('-EncodedCommand '); + + // Unset: no prologue, the remote keeps its own default root. + const bare = plan('push-test-linux', 'file'); + if (bare.kind !== 'ssh') throw new Error('unreachable'); + expect(bare.remoteCmd).not.toContain('SECRETS_HOME'); + }); + it('drops a caller-supplied passphrase rather than forwarding it (PHNX-2371)', () => { const t = planPushTransport(RESOLVED, 'apple.com', 'push-test-linux', { remoteBackend: 'file', diff --git a/src/lib/secrets/push.ts b/src/lib/secrets/push.ts index bcdb2e0..87fc240 100644 --- a/src/lib/secrets/push.ts +++ b/src/lib/secrets/push.ts @@ -26,6 +26,7 @@ import { buildWindowsStdinImportCommand, resolveRemoteOs, resolveRemoteOsAsync, + remoteStateRootEnv, type SshExecResult, } from '../transports/ssh.js'; import { @@ -92,6 +93,15 @@ export interface PushBundleOptions { literalValues?: Record; /** Per-SSH-operation deadline. Async daemon callers must set this explicitly. */ timeoutMs?: number; + /** + * State root (`SECRETS_HOME`) the remote `secrets` runs under for the import, + * the read-back verify and any literal restoration. Remote-relative: a leading + * `~/` resolves against the remote user's home on both shells + * (`remoteStateRootEnv`). A consumer that runs its local standalone under its + * own root (agents-cli: `~/.agents`) must name that root here, or the bundle + * lands in the remote's default `~/.secrets` where the consumer never reads it. + */ + remoteSecretsHome?: string; } export interface PushBundleResult { @@ -173,7 +183,7 @@ export type PushTransport = */ | { kind: 'ssh'; remoteCmd: string; input: string } /** The OS-aware `secrets` wrapper, the READ inverse's own path. */ - | { kind: 'remote-secrets'; args: string[]; input: string }; + | { kind: 'remote-secrets'; args: string[]; input: string; env: Record }; /** Choose the transport for one push. Pure: registry read in, plan out. */ export function planPushTransport( @@ -183,6 +193,7 @@ export function planPushTransport( opts: PushBundleOptions, ): PushTransport { const powershell = isPowershellTarget(host); + const env = remoteStateRootEnv(opts.remoteSecretsHome, resolveRemoteOs(host)); if (opts.remoteBackend === 'file') { // Both file-backend paths build a POSIX `bash -lc` command. Refuse a Windows // target cleanly rather than emit broken PowerShell (fail loud at the @@ -193,6 +204,7 @@ export function planPushTransport( const { remoteCmd, input } = buildRemoteFileImportCommand(bundle, resolved.dotenv, { force: opts.force, policyNever: opts.policyNever, + env, }); return { kind: 'ssh', remoteCmd, input }; } @@ -204,7 +216,7 @@ export function planPushTransport( // the wire over ssh stdin. return { kind: 'ssh', - remoteCmd: buildWindowsStdinImportCommand(bundle, { force: opts.force, policyNever: opts.policyNever }), + remoteCmd: buildWindowsStdinImportCommand(bundle, { force: opts.force, policyNever: opts.policyNever, env }), input: resolved.dotenv, }; } @@ -219,6 +231,7 @@ export function planPushTransport( ...(opts.policyNever ? ['--policy', 'never', '--i-understand'] : []), ], input: resolved.dotenv, + env, }; } @@ -240,6 +253,10 @@ export function pushResolvedBundleToHost( const plan = planPushTransport(resolved, bundle, host, opts); if (plan.kind === 'refuse') return fail(plan.message); + // The consumer state root rides EVERY remote invocation below, not just the + // import: a verify or literal step run under the remote's default root would + // read (or write) a different store than the one the import filled. + const env = remoteStateRootEnv(opts.remoteSecretsHome, resolveRemoteOs(host)); // Every ssh below rides the credential-transport posture: the managed host key // is pinned (a changed key is refused). There is no multiplexing to disable — // every connection is its own (REMOTE-1, `credentialTransportSshOpts`). The @@ -247,7 +264,7 @@ export function pushResolvedBundleToHost( // `secret: true` for the same posture. const res: SshExecResult = plan.kind === 'ssh' ? sshExec(host, plan.remoteCmd, { input: plan.input, timeoutMs: opts.timeoutMs, hostKeyOpts: credentialTransportSshOpts(host).hostKeyOpts }) - : remoteSecretsRaw(host, plan.args, { input: plan.input, timeoutMs: opts.timeoutMs, osLookupName: host, secret: true }); + : remoteSecretsRaw(host, plan.args, { input: plan.input, timeoutMs: opts.timeoutMs, osLookupName: host, secret: true, env: plan.env }); if (res.code === null) { return fail(res.stderr.trim() || (res.timedOut ? 'ssh timed out' : 'ssh failed')); @@ -264,7 +281,7 @@ export function pushResolvedBundleToHost( // - file: a forwarded SECRETS_PASSPHRASE keys ciphertext to a secret // the destination daemon does not hold (PHNX-2371). // Read the bundle back the way a later resolve will and FAIL LOUDLY. - const verdict = verifyRemoteKeychainPush(host, bundle, Object.keys(resolved.env), { osLookupName: host, secret: true, timeoutMs: opts.timeoutMs }); + const verdict = verifyRemoteKeychainPush(host, bundle, Object.keys(resolved.env), { osLookupName: host, secret: true, timeoutMs: opts.timeoutMs, env }); if (!verdict.ok) { if (opts.remoteBackend === 'keychain' && verdict.kind === 'locked-keychain') { return fail(keychainWriteFailureMessage(host, bundle, verdict.reason)); @@ -281,12 +298,12 @@ export function pushResolvedBundleToHost( } for (const step of planLiteralRestoration(bundle, opts.literalValues)) { - const removed = remoteSecretsRaw(host, step.removeArgs, { osLookupName: host, secret: true, timeoutMs: opts.timeoutMs }); + const removed = remoteSecretsRaw(host, step.removeArgs, { osLookupName: host, secret: true, timeoutMs: opts.timeoutMs, env }); if (removed.code !== 0) { const msg = (removed.stderr || removed.stdout || '').trim(); return fail(`pushed '${bundle}' but could not replace transported ${step.key}${msg ? `: ${msg}` : ''}`); } - const literal = remoteSecretsRaw(host, step.addArgs, { osLookupName: host, secret: true, timeoutMs: opts.timeoutMs }); + const literal = remoteSecretsRaw(host, step.addArgs, { osLookupName: host, secret: true, timeoutMs: opts.timeoutMs, env }); if (literal.code !== 0) { const msg = (literal.stderr || literal.stdout || '').trim(); return fail(`pushed '${bundle}' but could not preserve literal ${step.key}${msg ? `: ${msg}` : ''}`); @@ -331,6 +348,7 @@ export async function pushResolvedBundleToHostAsync( await resolveRemoteOsAsync(host, { timeoutMs: opts.timeoutMs }); const plan = planPushTransport(resolved, bundle, host, opts); if (plan.kind === 'refuse') return fail(plan.message); + const env = remoteStateRootEnv(opts.remoteSecretsHome, resolveRemoteOs(host)); const res = plan.kind === 'ssh' ? await sshExecAsync(host, plan.remoteCmd, { input: plan.input, @@ -341,6 +359,7 @@ export async function pushResolvedBundleToHostAsync( input: plan.input, timeoutMs: opts.timeoutMs, osLookupName: host, + env: plan.env, secret: true, }); @@ -352,6 +371,7 @@ export async function pushResolvedBundleToHostAsync( const verdict = await verifyRemoteKeychainPushAsync(host, bundle, Object.keys(resolved.env), { osLookupName: host, + env, secret: true, timeoutMs: opts.timeoutMs, }); @@ -373,6 +393,7 @@ export async function pushResolvedBundleToHostAsync( for (const step of planLiteralRestoration(bundle, opts.literalValues)) { const removed = await remoteSecretsRawAsync(host, step.removeArgs, { osLookupName: host, + env, secret: true, timeoutMs: opts.timeoutMs, }); @@ -382,6 +403,7 @@ export async function pushResolvedBundleToHostAsync( } const literal = await remoteSecretsRawAsync(host, step.addArgs, { osLookupName: host, + env, secret: true, timeoutMs: opts.timeoutMs, }); diff --git a/src/lib/secrets/remote.ts b/src/lib/secrets/remote.ts index 06337cc..b6d1416 100644 --- a/src/lib/secrets/remote.ts +++ b/src/lib/secrets/remote.ts @@ -33,6 +33,8 @@ export interface RemoteSecretsRawOptions { osLookupName?: string; secret?: boolean; timeoutMs?: number; + /** Env scoped to the remote invocation (e.g. the consumer state root, `remoteStateRootEnv`). */ + env?: Record; } /** The host part of an ssh target (`user@host` -> `host`) for known_hosts matching. */ @@ -143,7 +145,7 @@ export function remoteSecretsRaw( args: string[], opts: RemoteSecretsRawOptions = {}, ): SshExecResult { - const remoteCmd = buildRemoteSecretsInvocation(args, osForTarget(target, opts.osLookupName)); + const remoteCmd = buildRemoteSecretsInvocation(args, osForTarget(target, opts.osLookupName), opts.env); // `secret: true` pins the managed host key; `-tt` allocates a PTY. They compose. const posture = opts.secret ? credentialTransportSshOpts(target) : {}; const conn = opts.tty ? { ...posture, extraSshArgs: ['-tt'] } : posture; @@ -162,7 +164,7 @@ export async function remoteSecretsRawAsync( ): Promise { // Prime the OS cache off the event loop so the sync command build below never blocks. await resolveRemoteOsAsync(opts.osLookupName ?? target, { timeoutMs: opts.timeoutMs }); - const remoteCmd = buildRemoteSecretsInvocation(args, osForTarget(target, opts.osLookupName)); + const remoteCmd = buildRemoteSecretsInvocation(args, osForTarget(target, opts.osLookupName), opts.env); const posture = opts.secret ? credentialTransportSshOpts(target) : {}; const conn = opts.tty ? { ...posture, extraSshArgs: ['-tt'] } : posture; return sshExecAsync(target, remoteCmd, { @@ -356,12 +358,12 @@ export function verifyRemoteKeychainPush( target: string, bundle: string, pushedKeys: string[], - opts: { osLookupName?: string; secret?: boolean; timeoutMs?: number } = {}, + opts: { osLookupName?: string; secret?: boolean; timeoutMs?: number; env?: Record } = {}, ): RemoteKeychainWriteVerification { const remoteCmd = buildRemoteSecretsInvocation( ['export', bundle, '--plaintext', '--format', 'json'], osForTarget(target, opts.osLookupName), - { SECRETS_REMOTE_TRANSPORT: '1' }, + { ...opts.env, SECRETS_REMOTE_TRANSPORT: '1' }, ); const res: SshExecResult = sshExec(target, remoteCmd, { timeoutMs: opts.timeoutMs ?? REMOTE_TIMEOUT_MS, @@ -375,13 +377,13 @@ export async function verifyRemoteKeychainPushAsync( target: string, bundle: string, pushedKeys: string[], - opts: { osLookupName?: string; secret?: boolean; timeoutMs?: number } = {}, + opts: { osLookupName?: string; secret?: boolean; timeoutMs?: number; env?: Record } = {}, ): Promise { await resolveRemoteOsAsync(opts.osLookupName ?? target, { timeoutMs: opts.timeoutMs }); const remoteCmd = buildRemoteSecretsInvocation( ['export', bundle, '--plaintext', '--format', 'json'], osForTarget(target, opts.osLookupName), - { SECRETS_REMOTE_TRANSPORT: '1' }, + { ...opts.env, SECRETS_REMOTE_TRANSPORT: '1' }, ); const res = await sshExecAsync(target, remoteCmd, { timeoutMs: opts.timeoutMs ?? REMOTE_TIMEOUT_MS, @@ -434,7 +436,7 @@ function evaluateRemoteKeychainPushResult( export function buildRemoteFileImportCommand( bundle: string, dotenv: string, - opts: { force?: boolean; policyNever?: boolean } = {}, + opts: { force?: boolean; policyNever?: boolean; env?: Record } = {}, ): { remoteCmd: string; input: string } { const args = [ 'import', bundle, '--from', '-', '--backend', 'file', @@ -442,6 +444,7 @@ export function buildRemoteFileImportCommand( ...(opts.policyNever ? ['--policy', 'never', '--i-understand'] : []), ]; // POSIX-only path (planPushTransport refuses a Windows file-backend target). - // No env prologue — SECRETS_PASSPHRASE stays unset on the remote. - return { remoteCmd: buildRemoteSecretsInvocation(args), input: dotenv }; + // The only env prologue is the consumer state root (`remoteStateRootEnv`); + // SECRETS_PASSPHRASE stays unset on the remote. + return { remoteCmd: buildRemoteSecretsInvocation(args, undefined, opts.env), input: dotenv }; } diff --git a/src/lib/transports/ssh.test.ts b/src/lib/transports/ssh.test.ts index c8f02bb..2a1ae02 100644 --- a/src/lib/transports/ssh.test.ts +++ b/src/lib/transports/ssh.test.ts @@ -42,6 +42,8 @@ import { posixEnvExports, buildRemoteSecretsInvocation, buildWindowsStdinImportCommand, + remoteStateRootEnv, + windowsSecretsScript, resolveRemoteOs, resolveRemoteOsAsync, rememberRemoteOs, @@ -215,6 +217,36 @@ describe('posixEnvExports', () => { .toBe('export SECRETS_REMOTE_TRANSPORT=1'); expect(posixEnvExports({ EVIL: '$(rm -rf /)' })).toBe("export EVIL='$(rm -rf /)'"); expect(posixEnvExports({ PATH: '$HOME/bin:$PATH' })).toBe('export PATH="$HOME/bin:$PATH"'); + // The consumer state root is the other expanding key: it is always + // home-relative on the remote, so `$HOME` must resolve there, not here. + expect(posixEnvExports({ SECRETS_HOME: '$HOME/.agents' })).toBe('export SECRETS_HOME="$HOME/.agents"'); + }); +}); + +describe('remoteStateRootEnv — the consumer state root a push runs the remote secrets under', () => { + it('is empty when no root is named, so the remote keeps its own default', () => { + expect(remoteStateRootEnv(undefined)).toEqual({}); + expect(remoteStateRootEnv('')).toEqual({}); + expect(remoteStateRootEnv(' ', 'linux')).toEqual({}); + }); + + it('resolves a leading ~/ against the REMOTE home on each shell and passes absolute roots through', () => { + expect(remoteStateRootEnv('~/.agents', 'linux')).toEqual({ SECRETS_HOME: '$HOME/.agents' }); + expect(remoteStateRootEnv('~/.agents', 'darwin')).toEqual({ SECRETS_HOME: '$HOME/.agents' }); + expect(remoteStateRootEnv('~/.agents', 'windows')).toEqual({ SECRETS_HOME: '$env:USERPROFILE\\.agents' }); + expect(remoteStateRootEnv('~/a/b', 'windows')).toEqual({ SECRETS_HOME: '$env:USERPROFILE\\a\\b' }); + expect(remoteStateRootEnv('/srv/agents', 'linux')).toEqual({ SECRETS_HOME: '/srv/agents' }); + }); + + it('rides the POSIX invocation as an expanding export and the PowerShell script as a double-quoted env line', () => { + const posix = buildRemoteSecretsInvocation(['import', '__cursor__', '--from', '-', '--backend', 'file'], 'linux', remoteStateRootEnv('~/.agents', 'linux')); + expect(posix).toContain('export SECRETS_HOME="$HOME/.agents"; secrets import'); + const ps = windowsSecretsScript({ args: ['import', 'apple.com', '--from', '-'], env: remoteStateRootEnv('~/.agents', 'windows') }); + expect(ps).toContain('$env:SECRETS_HOME = "$env:USERPROFILE\\.agents"'); + // A non-expanding key stays a single-quoted literal on both shells. + expect(windowsSecretsScript({ args: ['status'], env: { SECRETS_REMOTE_TRANSPORT: '1' } })).toContain("$env:SECRETS_REMOTE_TRANSPORT = '1'"); + const stdin = buildWindowsStdinImportCommand('apple.com', { env: remoteStateRootEnv('~/.agents', 'windows') }); + expect(decodePowershell(stdin.split('-EncodedCommand ')[1])).toContain('$env:SECRETS_HOME = "$env:USERPROFILE\\.agents"'); }); }); diff --git a/src/lib/transports/ssh.ts b/src/lib/transports/ssh.ts index 4c72a1b..063b3e7 100644 --- a/src/lib/transports/ssh.ts +++ b/src/lib/transports/ssh.ts @@ -605,7 +605,37 @@ export function decodePowershell(encoded: string): string { * rendered as a shell literal, so a value carrying `$(...)` or a backtick can * never inject shell into a dispatch. */ -const EXPAND_KEYS = new Set(['PATH']); +/** + * Env keys whose value is emitted in the EXPANDING form so the remote shell + * resolves `$HOME` / `$PATH` / `$env:USERPROFILE` itself: the caller's absolute + * paths mean nothing on another box, and the consumer state root + * (`SECRETS_HOME`, see {@link remoteStateRootEnv}) is always home-relative. + */ +const EXPAND_KEYS = new Set(['PATH', 'SECRETS_HOME']); + +/** + * The env that pins the remote `secrets` to a consumer's state root. A consumer + * (agents-cli) runs the LOCAL standalone under `SECRETS_HOME=~/.agents` (MIG-1) + * and reads pushed bundles from that same root on the receiving box, so every + * remote invocation of a push — import, read-back verify, literal restoration — + * must run under it too. Without this the remote defaults to `~/.secrets`, the + * import succeeds, and the consumer never sees the bundle. + * + * `root` is remote-relative: a leading `~/` becomes `$HOME/` on POSIX and + * `$env:USERPROFILE\` on PowerShell, expanded by the remote shell (the two + * builders emit `SECRETS_HOME` in the expanding quote form). Any other value is + * passed through as an absolute remote path. Unset → `{}` → the remote's own + * default root. + */ +export function remoteStateRootEnv(root: string | undefined, os?: string): Record { + const trimmed = root?.trim(); + if (!trimmed) return {}; + if (remoteShellFor(os) === 'powershell') { + const rest = trimmed.startsWith('~/') ? trimmed.slice(2) : null; + return { SECRETS_HOME: rest === null ? trimmed : `$env:USERPROFILE\\${rest.replace(/\//g, '\\')}` }; + } + return { SECRETS_HOME: trimmed.startsWith('~/') ? `$HOME/${trimmed.slice(2)}` : trimmed }; +} /** * Build a POSIX `export K=V; …` prefix from an env map — empty string when the @@ -647,7 +677,13 @@ interface WindowsSecretsCommand { export function windowsSecretsScript(cmd: WindowsSecretsCommand): string { const { args, env } = cmd; const parts: string[] = [POWERSHELL_PROGRESS_SILENCE]; - if (env) for (const [k, v] of Object.entries(env)) parts.push(`$env:${k} = ${powershellQuote(v)}`); + if (env) { + for (const [k, v] of Object.entries(env)) { + // The expanding keys keep PowerShell's double-quote form so `$env:USERPROFILE` + // resolves on the remote; everything else is a single-quoted literal. + parts.push(EXPAND_KEYS.has(k) ? `$env:${k} = "${v.replace(/[`"]/g, (c) => `\`${c}`)}"` : `$env:${k} = ${powershellQuote(v)}`); + } + } parts.push(`& ${['secrets', ...args].map(powershellQuote).join(' ')}`); parts.push('exit $LASTEXITCODE'); return parts.join('; '); @@ -699,16 +735,19 @@ export function buildRemoteSecretsInvocation( */ export function buildWindowsStdinImportCommand( bundle: string, - opts: { force?: boolean; policyNever?: boolean } = {}, + opts: { force?: boolean; policyNever?: boolean; env?: Record } = {}, ): string { const force = opts.force ? ' --force' : ''; const policy = opts.policyNever ? ' --policy never --i-understand' : ''; + const envLines = Object.entries(opts.env ?? {}).map(([k, v]) => + EXPAND_KEYS.has(k) ? `$env:${k} = "${v.replace(/[`"]/g, (c) => `\`${c}`)}"` : `$env:${k} = ${powershellQuote(v)}`); // Create AND write the temp file INSIDE the try so its finally always cleans up: // if GetTempFileName succeeds but WriteAllText (or the import) then throws, the // secret-bearing temp file would otherwise be left behind. $tmp starts null so a // GetTempFileName that itself throws leaves nothing to remove. const script = [ POWERSHELL_PROGRESS_SILENCE, + ...envLines, '$in = [Console]::In.ReadToEnd()', '$tmp = $null', `try { $tmp = [System.IO.Path]::GetTempFileName(); [System.IO.File]::WriteAllText($tmp, $in); ` + From 6dc76ff29001757659faa6172be0bf1292db5f48 Mon Sep 17 00:00:00 2001 From: Muqsit Date: Mon, 7 Sep 2026 11:28:51 -0700 Subject: [PATCH 2/2] fix(push): remoteStateRootEnv admits only a plain path, since SECRETS_HOME is emitted in the expanding form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on #5: any other value ($(...), a backtick, quotes) would run on the remote as a side effect of setting the env. The gate is the charset — ~/, / or C:/ of [A-Za-z0-9._-/] — refused before the value reaches either shell; the stale 'trusted-static' EXPAND_KEYS comment is gone and the metacharacter cases are pinned in tests. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_014qq5fiWVRBmBV9f5KR58dY --- src/lib/transports/ssh.test.ts | 11 +++++++++++ src/lib/transports/ssh.ts | 17 +++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/lib/transports/ssh.test.ts b/src/lib/transports/ssh.test.ts index 2a1ae02..220b9ce 100644 --- a/src/lib/transports/ssh.test.ts +++ b/src/lib/transports/ssh.test.ts @@ -238,6 +238,17 @@ describe('remoteStateRootEnv — the consumer state root a push runs the remote expect(remoteStateRootEnv('/srv/agents', 'linux')).toEqual({ SECRETS_HOME: '/srv/agents' }); }); + it('refuses anything but a plain path, because the value is emitted in the expanding quote form', () => { + // Double quotes escape `\` and `"` only: `$(…)` and a backtick would still + // run on the remote as a side effect of setting the env, before `secrets` + // is even invoked. The gate is the charset, not the escaping. + for (const bad of ['$(id)', '~/.agents$(touch /tmp/x)', '`id`', '~/a b', "~/a'b", '~/a"b', 'relative/path', '~', '$HOME/.agents', '~/.agents;id']) { + expect(() => remoteStateRootEnv(bad, 'linux'), bad).toThrow(/Invalid remote state root/); + expect(() => remoteStateRootEnv(bad, 'windows'), bad).toThrow(/Invalid remote state root/); + } + expect(remoteStateRootEnv('C:/agents', 'windows')).toEqual({ SECRETS_HOME: 'C:/agents' }); + }); + it('rides the POSIX invocation as an expanding export and the PowerShell script as a double-quoted env line', () => { const posix = buildRemoteSecretsInvocation(['import', '__cursor__', '--from', '-', '--backend', 'file'], 'linux', remoteStateRootEnv('~/.agents', 'linux')); expect(posix).toContain('export SECRETS_HOME="$HOME/.agents"; secrets import'); diff --git a/src/lib/transports/ssh.ts b/src/lib/transports/ssh.ts index 063b3e7..95aa077 100644 --- a/src/lib/transports/ssh.ts +++ b/src/lib/transports/ssh.ts @@ -599,12 +599,6 @@ export function decodePowershell(encoded: string): string { return Buffer.from(encoded, 'base64').toString('utf16le'); } -/** - * Keys whose values are trusted-static and legitimately need remote shell - * expansion — `PATH` references the remote `$HOME`/`$PATH`. Every other key is - * rendered as a shell literal, so a value carrying `$(...)` or a backtick can - * never inject shell into a dispatch. - */ /** * Env keys whose value is emitted in the EXPANDING form so the remote shell * resolves `$HOME` / `$PATH` / `$env:USERPROFILE` itself: the caller's absolute @@ -613,6 +607,9 @@ export function decodePowershell(encoded: string): string { */ const EXPAND_KEYS = new Set(['PATH', 'SECRETS_HOME']); +/** A remote state root: `~/…`, `/…` or `C:/…`, path characters only — no shell metacharacters, spaces or quotes. */ +const REMOTE_STATE_ROOT_RE = /^(?:~\/|\/|[A-Za-z]:\/)[A-Za-z0-9._\-\/]*$/; + /** * The env that pins the remote `secrets` to a consumer's state root. A consumer * (agents-cli) runs the LOCAL standalone under `SECRETS_HOME=~/.agents` (MIG-1) @@ -630,6 +627,14 @@ const EXPAND_KEYS = new Set(['PATH', 'SECRETS_HOME']); export function remoteStateRootEnv(root: string | undefined, os?: string): Record { const trimmed = root?.trim(); if (!trimmed) return {}; + // The value is emitted in the EXPANDING quote form, where neither shell's + // escaping neutralizes `$(…)` or a backtick — so the ONLY thing allowed + // through is a plain path: `~/` or `/` (or a drive letter) followed by + // path characters. Anything else is refused here, before it can reach a + // remote shell as a side effect of merely setting the env. + if (!REMOTE_STATE_ROOT_RE.test(trimmed)) { + throw new Error(`Invalid remote state root '${trimmed}': use ~/ or an absolute path made of letters, digits, '.', '_', '-' and '/'.`); + } if (remoteShellFor(os) === 'powershell') { const rest = trimmed.startsWith('~/') ? trimmed.slice(2) : null; return { SECRETS_HOME: rest === null ? trimmed : `$env:USERPROFILE\\${rest.replace(/\//g, '\\')}` };