From f89314a5bf5da7e4fb79da9b7d348b769112f26a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 13:06:49 +0200 Subject: [PATCH 1/5] fix(remote): stop persisting the daemon bearer token in connection state ADR 0007 requires generated connection profiles to strip daemon and Metro bearer tokens; only the Metro half was honored. `connect` was writing the daemon bearer token into the 0600 connection-state file, and every later command read it back out. Stop writing `authToken` into `RemoteConnectionState['daemon']` and resolve it at each reader from the existing flag -> environment (AGENT_DEVICE_DAEMON_AUTH_TOKEN) -> remote-config-profile chain instead, matching src/cli/auth-session.ts's precedence. Behavior change: a user who ran `connect --daemon-auth-token ` and relied on later commands picking the token back up from the state file will now get an auth failure. They must export AGENT_DEVICE_DAEMON_AUTH_TOKEN, set daemonAuthToken in their remote config, or pass --daemon-auth-token on each command. website/docs/docs/remote-proxy.md is updated to show the supported env-var workflow. --- src/__tests__/remote-connection.test.ts | 9 +- src/cli/commands/connection-runtime.ts | 8 +- src/cli/commands/connection.ts | 4 +- .../__tests__/remote-connection-state.test.ts | 102 ++++++++++++++++++ src/remote/remote-connection-state.ts | 8 +- .../smoke-provider-cli-disconnect.test.ts | 4 + website/docs/docs/remote-proxy.md | 9 +- 7 files changed, 129 insertions(+), 15 deletions(-) create mode 100644 src/remote/__tests__/remote-connection-state.test.ts diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index 8ba8895b3e..d05a0aa8d6 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -233,7 +233,6 @@ test('connect proxy writes normal remote state with generated non-secret profile assert.equal(state.leaseId, undefined); assert.deepEqual(state.daemon, { baseUrl: 'http://proxy.example.test/agent-device', - authToken: 'proxy-secret', transport: 'http', }); assert.match(state.remoteConfigPath, /remote-connections\/generated\/proxy-[a-f0-9]{16}\.json$/); @@ -277,7 +276,6 @@ test('connect daemon-base-url shortcut uses proxy profile for direct proxy URLs' assert.match(state.clientId ?? '', /^[a-f0-9]{16}$/); assert.deepEqual(state.daemon, { baseUrl: 'http://127.0.0.1:4310/agent-device', - authToken: 'proxy-secret', transport: 'http', }); assert.equal(state.leaseId, undefined); @@ -2269,7 +2267,6 @@ test('disconnect releases proxy lease with provider client and device metadata', leaseId: 'abc123abc123abc1', daemon: { baseUrl: 'http://proxy.example.test/agent-device', - authToken: 'proxy-secret', }, leaseBackend: 'ios-instance', leaseProvider: 'proxy', @@ -2290,6 +2287,10 @@ test('disconnect releases proxy lease with provider client and device metadata', version: false, stateDir, shutdown: true, + // Not persisted on connection state (ADR 0007): the caller supplies + // it per-command via flag/env/CLI-session, mirroring how the CLI + // dispatcher resolves it before invoking this handler. + daemonAuthToken: 'test-not-a-real-token', }, client: createTestClient({ release: async (request) => { @@ -2306,7 +2307,7 @@ test('disconnect releases proxy lease with provider client and device metadata', assert.equal(releaseRequest?.leaseId, 'abc123abc123abc1'); assert.equal(releaseRequest?.leaseBackend, 'ios-instance'); assert.equal(releaseRequest?.daemonBaseUrl, 'http://proxy.example.test/agent-device'); - assert.equal(releaseRequest?.daemonAuthToken, 'proxy-secret'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-not-a-real-token'); assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' }), null); fs.rmSync(tempRoot, { recursive: true, force: true }); }); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index bc10ba64fe..79f6436f91 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -483,6 +483,9 @@ export async function stopReactDevtoolsCleanup(options: { export async function releaseRemoteConnectionLease( client: AgentDeviceClient, state: RemoteConnectionState, + // The daemon bearer token is never persisted on `state` (ADR 0007); callers + // pass the token already resolved via the flag/env/CLI-session chain. + daemonAuthToken?: string, ): Promise<{ released: boolean; provider?: CloudProviderSessionResult }> { if (!state.leaseId) return { released: false }; const result = await client.leases.release({ @@ -491,7 +494,7 @@ export async function releaseRemoteConnectionLease( leaseId: state.leaseId, leaseBackend: state.leaseBackend, daemonBaseUrl: state.daemon?.baseUrl, - daemonAuthToken: state.daemon?.authToken, + daemonAuthToken, daemonTransport: state.daemon?.transport, daemonServerMode: state.daemon?.serverMode, leaseProvider: state.leaseProvider, @@ -504,10 +507,11 @@ export async function releaseRemoteConnectionLease( export async function releasePreviousLease( client: AgentDeviceClient, previous: RemoteConnectionState, + daemonAuthToken?: string, ): Promise { if (!previous.leaseId) return; try { - await releaseRemoteConnectionLease(client, previous); + await releaseRemoteConnectionLease(client, previous, daemonAuthToken); } catch { // Reconnect must succeed even if the old lease was already released. } diff --git a/src/cli/commands/connection.ts b/src/cli/commands/connection.ts index 91989a9a76..f3c8e7d10e 100644 --- a/src/cli/commands/connection.ts +++ b/src/cli/commands/connection.ts @@ -244,7 +244,7 @@ async function cleanupForcedPreviousConnection( if (!previous || !flags.force) return; await stopMetroCleanup(previous.metro); await stopReactDevtoolsCleanup({ stateDir, state: previous }); - await releasePreviousLease(client, previous); + await releasePreviousLease(client, previous, flags.daemonAuthToken); } function readRemoteConfigConnectionMetadata( @@ -286,7 +286,7 @@ export const disconnectCommand: ClientCommandHandler = async ({ flags, client }) let released = false; if (state.leaseId) { try { - const release = await releaseRemoteConnectionLease(client, state); + const release = await releaseRemoteConnectionLease(client, state, flags.daemonAuthToken); released = release.released; providerData ??= release.provider; } catch { diff --git a/src/remote/__tests__/remote-connection-state.test.ts b/src/remote/__tests__/remote-connection-state.test.ts new file mode 100644 index 0000000000..96587bca70 --- /dev/null +++ b/src/remote/__tests__/remote-connection-state.test.ts @@ -0,0 +1,102 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { mkdtempForTest } from '../../__tests__/test-utils/tmp-dir.ts'; +import { + buildRemoteConnectionDaemonState, + hashRemoteConfigFile, + resolveRemoteConnectionDefaults, + writeRemoteConnectionState, + type RemoteConnectionState, +} from '../remote-connection-state.ts'; + +// Regression coverage for ADR 0007: generated connection profiles must strip +// the daemon bearer token. `connect` used to write it straight into the +// persisted connection-state file; these tests guard against that shadow +// coming back. + +const FAKE_DAEMON_TOKEN = 'test-not-a-real-daemon-token'; + +test('buildRemoteConnectionDaemonState does not persist the daemon auth token', () => { + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: FAKE_DAEMON_TOKEN, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + + assert.equal(Object.hasOwn(daemon ?? {}, 'authToken'), false); + assert.equal(daemon?.baseUrl, 'https://daemon.example.test'); + assert.equal(daemon?.transport, 'http'); + assert.equal(daemon?.serverMode, 'http'); +}); + +test('written connection state contains no daemon auth token', async () => { + const tempRoot = await mkdtempForTest('agent-device-remote-connection-state-write-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + fs.writeFileSync(remoteConfigPath, '{}'); + + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: FAKE_DAEMON_TOKEN, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + const now = new Date().toISOString(); + const state: RemoteConnectionState = { + version: 1, + session: 'adc-write-test', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + daemon, + tenant: 'acme', + runId: 'run-1', + connectedAt: now, + updatedAt: now, + }; + + writeRemoteConnectionState({ stateDir, state }); + + const writtenPath = path.join(stateDir, 'remote-connections', 'adc-write-test.json'); + const written = fs.readFileSync(writtenPath, 'utf8'); + assert.equal(written.includes(FAKE_DAEMON_TOKEN), false); + assert.equal(written.includes('authToken'), false); +}); + +test('resolveRemoteConnectionDefaults falls back to the environment token', async () => { + const tempRoot = await mkdtempForTest('agent-device-remote-connection-state-defaults-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + fs.writeFileSync(remoteConfigPath, '{}'); + + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: undefined, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + const now = new Date().toISOString(); + const state: RemoteConnectionState = { + version: 1, + session: 'adc-env-fallback', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + daemon, + tenant: 'acme', + runId: 'run-1', + connectedAt: now, + updatedAt: now, + }; + writeRemoteConnectionState({ stateDir, state }); + + const defaults = resolveRemoteConnectionDefaults({ + stateDir, + session: 'adc-env-fallback', + cwd: tempRoot, + env: { AGENT_DEVICE_DAEMON_AUTH_TOKEN: FAKE_DAEMON_TOKEN }, + }); + + assert.equal(defaults?.flags.daemonAuthToken, FAKE_DAEMON_TOKEN); +}); diff --git a/src/remote/remote-connection-state.ts b/src/remote/remote-connection-state.ts index 81bae0e3f5..89f5dea1b4 100644 --- a/src/remote/remote-connection-state.ts +++ b/src/remote/remote-connection-state.ts @@ -19,7 +19,6 @@ export type RemoteConnectionState = { remoteConfigHash: string; daemon?: { baseUrl?: string; - authToken?: string; transport?: CliFlags['daemonTransport']; serverMode?: CliFlags['daemonServerMode']; }; @@ -94,7 +93,6 @@ export function buildRemoteConnectionDaemonState( ): RemoteConnectionState['daemon'] { return { baseUrl: sanitizeDaemonBaseUrl(flags.daemonBaseUrl), - authToken: flags.daemonAuthToken, transport: flags.daemonTransport, serverMode: flags.daemonServerMode, }; @@ -154,7 +152,11 @@ export function resolveRemoteConnectionDefaults(options: { ...profile, remoteConfig: state.remoteConfigPath, daemonBaseUrl: state.daemon?.baseUrl ?? profile.daemonBaseUrl, - daemonAuthToken: state.daemon?.authToken ?? profile.daemonAuthToken, + // Deliberately not sourced from state: the daemon bearer token is never + // persisted to the connection-state file (ADR 0007). It is resolved + // from the profile here, and from the flag/env/CLI-session chain in + // resolveRemoteAuth (src/cli/auth-session.ts) at command dispatch time. + daemonAuthToken: profile.daemonAuthToken, daemonTransport: state.daemon?.transport ?? profile.daemonTransport, daemonServerMode: state.daemon?.serverMode ?? profile.daemonServerMode, ...leaseScopeToCommandFlags(leaseScope), diff --git a/test/integration/smoke-provider-cli-disconnect.test.ts b/test/integration/smoke-provider-cli-disconnect.test.ts index ff739cd156..de8e3daeca 100644 --- a/test/integration/smoke-provider-cli-disconnect.test.ts +++ b/test/integration/smoke-provider-cli-disconnect.test.ts @@ -48,6 +48,10 @@ function createProviderEnv(fixture: ProviderDaemonFixture): NodeJS.ProcessEnv { BROWSERSTACK_USERNAME: 'browser-user', BROWSERSTACK_ACCESS_KEY: 'browser-key', AGENT_DEVICE_TEST_RPC_LOG_PATH: fixture.rpcLogPath, + // Generated connection state never persists the daemon bearer token + // (ADR 0007), so commands after `connect` must resolve it from the + // environment/CLI chain, same as real usage. + AGENT_DEVICE_DAEMON_AUTH_TOKEN: 'test-daemon-token', NODE_OPTIONS: [process.env.NODE_OPTIONS, `--import=${fetchFixtureUrl}`] .filter(Boolean) .join(' '), diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index 869acfa155..bf97512b63 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -31,12 +31,11 @@ By default the proxy binds `127.0.0.1`. Use `--host 0.0.0.0` only when you inten ## Remote Client -On the machine running the agent, connect to the public tunnel origin with the `/agent-device` base path and the printed token: +On the machine running the agent, connect to the public tunnel origin with the `/agent-device` base path and the printed token. The generated connection profile never stores the token (only routing metadata), so export it once and every command in the session picks it up: ```bash -agent-device connect proxy \ - --daemon-base-url https://example.trycloudflare.com/agent-device \ - --daemon-auth-token +export AGENT_DEVICE_DAEMON_AUTH_TOKEN= +agent-device connect proxy --daemon-base-url https://example.trycloudflare.com/agent-device agent-device devices --platform ios agent-device open MyApp --platform ios agent-device snapshot --platform ios @@ -44,6 +43,8 @@ agent-device close agent-device disconnect ``` +Passing `--daemon-auth-token ` instead of exporting the environment variable also works, but only authenticates the single command it is passed to; subsequent commands need the token again through the env var, a `daemonAuthToken` entry in your remote config profile, or a repeated `--daemon-auth-token` flag. + `connect proxy` stores the proxy profile and client identity. Device leases are automatic on `open` and expire after five minutes without commands. `close` releases the active session and device lease; `disconnect` clears local connection state. Multiple agents can share one proxy when each uses the normal `connect proxy`, `open`, commands, `close`, and `disconnect` flow. A busy device error means another agent owns the device until it closes or its inactivity lease expires. From 818167b206467f8009f939719e1dbb6944c6bb35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 13:54:39 +0200 Subject: [PATCH 2/5] fix(remote): authenticate forced-reconnect lease release with the previous endpoint's own credential MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit connect --force released the previous connection's lease using the new connection's ambient daemonAuthToken instead of the previous endpoint's own credential, and swallowed the resulting auth failure — silently orphaning the old lease when replacing a connection with a differently-authenticated one. Resolve the release token from the previous connection's own remote-config profile first, fall back to the ambient token only when the two connections share the same daemon endpoint, and otherwise skip the release and surface an actionable notice (tenant, run id, lease id, endpoint) through the existing connect notice channel instead of hiding the failure. --- src/__tests__/remote-connection.test.ts | 208 +++++++++++++++++++- src/cli/commands/connection-presentation.ts | 76 ++++--- src/cli/commands/connection-runtime.ts | 87 +++++++- src/cli/commands/connection.ts | 25 ++- 4 files changed, 360 insertions(+), 36 deletions(-) diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index d05a0aa8d6..dcb6a56921 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -1881,7 +1881,16 @@ test('connect --force stops replaced Metro companion after state is updated', as const stateDir = path.join(tempRoot, '.state'); const oldRemoteConfigPath = path.join(tempRoot, 'old-remote.json'); const newRemoteConfigPath = path.join(tempRoot, 'new-remote.json'); - fs.writeFileSync(oldRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://old.example' })); + fs.writeFileSync( + oldRemoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://old.example', + // Recoverable from the previous connection's own profile (plan 007 + // rule 1) so the forced release authenticates against old.example with + // its own credential, not the new connection's. + daemonAuthToken: 'test-old-not-a-real-token', + }), + ); fs.writeFileSync(newRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://new.example' })); writeRemoteConnectionState({ stateDir, @@ -1920,6 +1929,7 @@ test('connect --force stops replaced Metro companion after state is updated', as stateDir, remoteConfig: newRemoteConfigPath, daemonBaseUrl: 'https://new.example', + daemonAuthToken: 'test-new-not-a-real-token', tenant: 'acme', runId: 'run-new', session: 'adc-android', @@ -1942,6 +1952,7 @@ test('connect --force stops replaced Metro companion after state is updated', as assert.equal(releaseRequest?.leaseId, 'lease-old'); assert.equal(releaseRequest?.daemonBaseUrl, 'https://old.example'); assert.equal(releaseRequest?.daemonTransport, 'http'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-old-not-a-real-token'); assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' })?.runId, 'run-new'); fs.rmSync(tempRoot, { recursive: true, force: true }); }); @@ -1951,7 +1962,13 @@ test('connect --force without a session replaces the active generated connection const stateDir = path.join(tempRoot, '.state'); const oldRemoteConfigPath = path.join(tempRoot, 'old-remote.json'); const newRemoteConfigPath = path.join(tempRoot, 'new-remote.json'); - fs.writeFileSync(oldRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://old.example' })); + fs.writeFileSync( + oldRemoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://old.example', + daemonAuthToken: 'test-old-not-a-real-token', + }), + ); fs.writeFileSync(newRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://new.example' })); writeRemoteConnectionState({ stateDir, @@ -1990,6 +2007,7 @@ test('connect --force without a session replaces the active generated connection stateDir, remoteConfig: newRemoteConfigPath, daemonBaseUrl: 'https://new.example', + daemonAuthToken: 'test-new-not-a-real-token', tenant: 'acme', runId: 'run-new', platform: 'android', @@ -2012,6 +2030,7 @@ test('connect --force without a session replaces the active generated connection assert.equal(activeState?.runId, 'run-new'); assert.equal(activeState?.remoteConfigPath, newRemoteConfigPath); assert.equal(releaseRequest?.leaseId, 'lease-old'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-old-not-a-real-token'); assert.deepEqual(vi.mocked(stopMetroCompanion).mock.calls[0]?.[0], { projectRoot: '/tmp/old-project', profileKey: oldRemoteConfigPath, @@ -2022,6 +2041,191 @@ test('connect --force without a session replaces the active generated connection fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test("connect --force releases the previous lease with the previous connection's own token, not the new one", async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-force-prev-token-'); + const stateDir = path.join(tempRoot, '.state'); + const oldRemoteConfigPath = path.join(tempRoot, 'old-remote.json'); + const newRemoteConfigPath = path.join(tempRoot, 'new-remote.json'); + fs.writeFileSync( + oldRemoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://old.example', + // Token A: belongs to the previous (old) connection's own profile. + daemonAuthToken: 'test-old-not-a-real-token', + }), + ); + fs.writeFileSync(newRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://new.example' })); + writeRemoteConnectionState({ + stateDir, + state: { + version: 1, + session: 'adc-android', + remoteConfigPath: oldRemoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(oldRemoteConfigPath), + tenant: 'acme', + runId: 'run-old', + leaseId: 'lease-old', + leaseBackend: 'android-instance', + daemon: { baseUrl: 'https://old.example' }, + connectedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + }); + let releaseRequest: Parameters[0] | undefined; + + await captureStdout(async () => { + await connectCommand({ + positionals: [], + flags: { + json: true, + help: false, + version: false, + force: true, + stateDir, + remoteConfig: newRemoteConfigPath, + daemonBaseUrl: 'https://new.example', + // Token B: the new connection's credential; must never reach old.example. + daemonAuthToken: 'test-new-not-a-real-token', + tenant: 'acme', + runId: 'run-new', + session: 'adc-android', + platform: 'android', + }, + client: createTestClient({ + release: async (request) => { + releaseRequest = request; + return { released: true }; + }, + }), + }); + }); + + assert.equal(releaseRequest?.leaseId, 'lease-old'); + assert.equal(releaseRequest?.daemonBaseUrl, 'https://old.example'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-old-not-a-real-token'); + assert.notEqual(releaseRequest?.daemonAuthToken, 'test-new-not-a-real-token'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('connect --force skips releasing the previous lease when its token cannot be recovered and the endpoint differs', async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-force-unreleasable-'); + const stateDir = path.join(tempRoot, '.state'); + const oldRemoteConfigPath = path.join(tempRoot, 'old-remote.json'); + const newRemoteConfigPath = path.join(tempRoot, 'new-remote.json'); + // No daemonAuthToken in the previous connection's own profile: its + // credential cannot be recovered, and the new endpoint differs. + fs.writeFileSync(oldRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://old.example' })); + fs.writeFileSync(newRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://new.example' })); + writeRemoteConnectionState({ + stateDir, + state: { + version: 1, + session: 'adc-android', + remoteConfigPath: oldRemoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(oldRemoteConfigPath), + tenant: 'acme', + runId: 'run-old', + leaseId: 'lease-old', + leaseBackend: 'android-instance', + daemon: { baseUrl: 'https://old.example' }, + connectedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + }); + let releaseCalled = false; + + const stdout = await captureStdout(async () => { + await connectCommand({ + positionals: [], + flags: { + json: false, + help: false, + version: false, + force: true, + stateDir, + remoteConfig: newRemoteConfigPath, + daemonBaseUrl: 'https://new.example', + daemonAuthToken: 'test-new-not-a-real-token', + tenant: 'acme', + runId: 'run-new', + session: 'adc-android', + platform: 'android', + }, + client: createTestClient({ + release: async () => { + releaseCalled = true; + return { released: true }; + }, + }), + }); + }); + + assert.equal(releaseCalled, false); + assert.match(stdout, /Could not release the previous lease lease-old/); + assert.match(stdout, /tenant acme, run run-old/); + assert.match(stdout, /old\.example/); + // Reconnect still succeeds despite the orphaned previous lease. + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' })?.runId, 'run-new'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('connect --force reuses the ambient token to release the previous lease when the endpoint is unchanged', async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-force-same-endpoint-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + // No daemonAuthToken on the profile itself: the ambient flag is the only + // source, matching an ordinary same-profile --force reconnect. + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + writeRemoteConnectionState({ + stateDir, + state: { + version: 1, + session: 'adc-android', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + tenant: 'acme', + runId: 'run-old', + leaseId: 'lease-old', + leaseBackend: 'android-instance', + daemon: { baseUrl: 'https://daemon.example' }, + connectedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + }); + let releaseRequest: Parameters[0] | undefined; + + await captureStdout(async () => { + await connectCommand({ + positionals: [], + flags: { + json: true, + help: false, + version: false, + force: true, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + daemonAuthToken: 'test-ambient-not-a-real-token', + tenant: 'acme', + runId: 'run-new', + session: 'adc-android', + platform: 'android', + }, + client: createTestClient({ + release: async (request) => { + releaseRequest = request; + return { released: true }; + }, + }), + }); + }); + + assert.equal(releaseRequest?.leaseId, 'lease-old'); + assert.equal(releaseRequest?.daemonBaseUrl, 'https://daemon.example'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-ambient-not-a-real-token'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('disconnect tolerates prior close and removes local connection state', async () => { const tempRoot = mkdtempForTestSync('agent-device-disconnect-'); const stateDir = path.join(tempRoot, '.state'); diff --git a/src/cli/commands/connection-presentation.ts b/src/cli/commands/connection-presentation.ts index c4ed9674fc..14bf9a67e9 100644 --- a/src/cli/commands/connection-presentation.ts +++ b/src/cli/commands/connection-presentation.ts @@ -20,6 +20,11 @@ export type LeasePreparationNotice = { nextSteps: string[]; }; +export type PreviousLeaseReleaseNotice = { + status: 'unreleased'; + message: string; +}; + export function buildLeasePreparationNotice( state: RemoteConnectionState, verification?: ConnectVerification, @@ -77,14 +82,16 @@ export function renderConnectSuccess(options: { state: RemoteConnectionState; readiness?: ConnectReadiness; runtimePreparation?: RuntimePreparationNotice; + previousLeaseNotice?: PreviousLeaseReleaseNotice; }): string { - const { state, readiness, runtimePreparation } = options; + const { state, readiness, runtimePreparation, previousLeaseNotice } = options; if (!readiness) { const leasePreparation = buildLeasePreparationNotice(state); return [ `Configured remote session "${state.session}" tenant "${state.tenant}" run "${state.runId}"${state.leaseId ? ` lease ${state.leaseId}` : ''}.`, leasePreparation?.message, runtimePreparation?.message, + previousLeaseNotice?.message, ] .filter((line): line is string => Boolean(line)) .join('\n'); @@ -104,6 +111,7 @@ export function renderConnectSuccess(options: { lines.push(...readiness.nextSteps.map((step) => ` ${step}`)); lines.push(...(readiness.notes ?? [])); if (runtimePreparation) lines.push(runtimePreparation.message); + if (previousLeaseNotice) lines.push(previousLeaseNotice.message); return lines.join('\n'); } @@ -111,10 +119,10 @@ export function serializeConnectionState(options: { state: RemoteConnectionState; runtimePreparation?: RuntimePreparationNotice; readiness?: ConnectReadiness; + previousLeaseNotice?: PreviousLeaseReleaseNotice; }): Record { - const { state, runtimePreparation, readiness } = options; + const { state, runtimePreparation, readiness, previousLeaseNotice } = options; const leasePreparation = buildLeasePreparationNotice(state, readiness); - const nextSteps = readiness?.nextSteps ?? leasePreparation?.nextSteps ?? []; return { connected: true, session: state.session, @@ -129,34 +137,56 @@ export function serializeConnectionState(options: { remoteConfig: state.remoteConfigPath, remoteConfigHash: state.remoteConfigHash, daemonBaseUrlFingerprint: fingerprint(state.daemon?.baseUrl), - liveSession: { - status: state.leaseId ? 'created' : 'not-created', - ...(state.leaseId ? { leaseId: state.leaseId } : {}), - }, - ...(readiness - ? { - verification: { - status: connectionVerificationStatus(readiness), - service: readiness.service, - message: readiness.verificationMessage, - ...(readiness.project ? { project: readiness.project } : {}), - }, - ...(readiness.device ? { device: readiness.device } : {}), - ...(readiness.app ? { app: readiness.app } : {}), - nextSteps, - ...(readiness.notes ? { notes: readiness.notes } : {}), - } - : {}), + liveSession: buildLiveSessionField(state), + ...buildReadinessFields(readiness, leasePreparation), metro: state.metro ? { prepared: true, projectRoot: state.metro.projectRoot } : { prepared: false }, - ...(leasePreparation ? { leasePreparation } : {}), - ...(runtimePreparation ? { runtimePreparation } : {}), + ...buildConnectionNoticeFields({ leasePreparation, runtimePreparation, previousLeaseNotice }), connectedAt: state.connectedAt, updatedAt: state.updatedAt, }; } +function buildLiveSessionField(state: RemoteConnectionState): Record { + return { + status: state.leaseId ? 'created' : 'not-created', + ...(state.leaseId ? { leaseId: state.leaseId } : {}), + }; +} + +function buildReadinessFields( + readiness: ConnectReadiness | undefined, + leasePreparation: LeasePreparationNotice | undefined, +): Record { + if (!readiness) return {}; + return { + verification: { + status: connectionVerificationStatus(readiness), + service: readiness.service, + message: readiness.verificationMessage, + ...(readiness.project ? { project: readiness.project } : {}), + }, + ...(readiness.device ? { device: readiness.device } : {}), + ...(readiness.app ? { app: readiness.app } : {}), + nextSteps: readiness.nextSteps ?? leasePreparation?.nextSteps ?? [], + ...(readiness.notes ? { notes: readiness.notes } : {}), + }; +} + +function buildConnectionNoticeFields(options: { + leasePreparation?: LeasePreparationNotice; + runtimePreparation?: RuntimePreparationNotice; + previousLeaseNotice?: PreviousLeaseReleaseNotice; +}): Record { + const { leasePreparation, runtimePreparation, previousLeaseNotice } = options; + return { + ...(leasePreparation ? { leasePreparation } : {}), + ...(runtimePreparation ? { runtimePreparation } : {}), + ...(previousLeaseNotice ? { previousLeaseNotice } : {}), + }; +} + function renderDevice(device: NonNullable): string { const osVersion = 'osVersion' in device ? device.osVersion : undefined; const os = [device.platform, osVersion].filter(Boolean).join(' '); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 79f6436f91..fc44daf2e5 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -32,6 +32,7 @@ import { readMetroPrepareKind } from '../../commands/metro/prepare-kind.ts'; import { connectionProviderRequiresRemoteDaemon } from '../connection/provider-policy.ts'; import { readCloudDeviceFeatureProfileFields } from '../connection/profile-fields.ts'; import { isCloudWebDriverProviderName } from '@agent-device/provider-webdriver'; +import type { PreviousLeaseReleaseNotice } from './connection-presentation.ts'; const leaseDeferredCommands = new Set([ 'artifacts', @@ -504,19 +505,95 @@ export async function releaseRemoteConnectionLease( return result; } +// A forced reconnect releases the *previous* connection's lease, which must be +// authenticated against the *previous* endpoint's own credential — never the +// new connection's token (that would send an unrelated endpoint's secret to +// an endpoint it was never issued for). See plans/007 for the full rule. +type PreviousLeaseAuthResolution = + | { canAuthenticate: true; daemonAuthToken?: string } + | { canAuthenticate: false }; + +function resolvePreviousLeaseAuth(options: { + previous: RemoteConnectionState; + nextDaemonBaseUrl?: string; + ambientDaemonAuthToken?: string; + cwd: string; + env: Record; +}): PreviousLeaseAuthResolution { + const ownToken = resolvePreviousOwnDaemonAuthToken(options.previous, options.cwd, options.env); + if (ownToken) return { canAuthenticate: true, daemonAuthToken: ownToken }; + if (options.previous.daemon?.baseUrl === options.nextDaemonBaseUrl) { + // Same endpoint: the ambient credential plausibly belongs to it too. + return { canAuthenticate: true, daemonAuthToken: options.ambientDaemonAuthToken }; + } + return { canAuthenticate: false }; +} + +function resolvePreviousOwnDaemonAuthToken( + previous: RemoteConnectionState, + cwd: string, + env: Record, +): string | undefined { + try { + return resolveRemoteConfigProfile({ + configPath: previous.remoteConfigPath, + cwd, + env, + }).profile.daemonAuthToken; + } catch { + // A missing/unparseable previous config is the "cannot authenticate" + // case handled by the caller, not an error to propagate here. + return undefined; + } +} + export async function releasePreviousLease( client: AgentDeviceClient, previous: RemoteConnectionState, - daemonAuthToken?: string, -): Promise { - if (!previous.leaseId) return; + options: { + nextDaemonBaseUrl?: string; + ambientDaemonAuthToken?: string; + cwd: string; + env: Record; + }, +): Promise { + if (!previous.leaseId) return undefined; + const auth = resolvePreviousLeaseAuth({ + previous, + nextDaemonBaseUrl: options.nextDaemonBaseUrl, + ambientDaemonAuthToken: options.ambientDaemonAuthToken, + cwd: options.cwd, + env: options.env, + }); + if (!auth.canAuthenticate) { + return buildUnreleasedPreviousLeaseNotice( + previous, + 'no credential known to belong to that endpoint was available', + ); + } try { - await releaseRemoteConnectionLease(client, previous, daemonAuthToken); + await releaseRemoteConnectionLease(client, previous, auth.daemonAuthToken); + return undefined; } catch { - // Reconnect must succeed even if the old lease was already released. + // Reconnect must still succeed; surface the failure instead of hiding it. + return buildUnreleasedPreviousLeaseNotice(previous, 'the release request failed'); } } +function buildUnreleasedPreviousLeaseNotice( + previous: RemoteConnectionState, + reason: string, +): PreviousLeaseReleaseNotice { + return { + status: 'unreleased', + message: + `Could not release the previous lease ${previous.leaseId} ` + + `(tenant ${previous.tenant}, run ${previous.runId}) ` + + `at ${previous.daemon?.baseUrl ?? 'its daemon'}: ${reason}. ` + + 'It was left in place — release it manually if it is still active.', + }; +} + async function releaseAcquiredLeaseOnWriteFailure( client: AgentDeviceClient, state: RemoteConnectionState, diff --git a/src/cli/commands/connection.ts b/src/cli/commands/connection.ts index f3c8e7d10e..ebcd76c827 100644 --- a/src/cli/commands/connection.ts +++ b/src/cli/commands/connection.ts @@ -43,6 +43,7 @@ import { presentConnectReadiness, renderConnectSuccess, serializeConnectionState, + type PreviousLeaseReleaseNotice, type RuntimePreparationNotice, } from './connection-presentation.ts'; @@ -81,14 +82,20 @@ export const connectCommand: ClientCommandHandler = async ({ positionals, flags, remoteConfigPath: resolved.remoteConfigPath, }); writeRemoteConnectionState({ stateDir, state }); - await cleanupForcedPreviousConnection(client, stateDir, connectFlags, context.previous); + const previousLeaseNotice = await cleanupForcedPreviousConnection( + client, + stateDir, + connectFlags, + context.previous, + state.daemon?.baseUrl, + ); const runtimePreparation = buildRuntimePreparationNotice(connectFlags, state); const readiness = presentConnectReadiness(state, verification); writeCommandOutput( connectFlags, - serializeConnectionState({ state, runtimePreparation, readiness }), - () => renderConnectSuccess({ state, runtimePreparation, readiness }), + serializeConnectionState({ state, runtimePreparation, readiness, previousLeaseNotice }), + () => renderConnectSuccess({ state, runtimePreparation, readiness, previousLeaseNotice }), ); return true; }; @@ -240,11 +247,17 @@ async function cleanupForcedPreviousConnection( stateDir: string, flags: CliFlags, previous: RemoteConnectionState | null, -): Promise { - if (!previous || !flags.force) return; + nextDaemonBaseUrl: string | undefined, +): Promise { + if (!previous || !flags.force) return undefined; await stopMetroCleanup(previous.metro); await stopReactDevtoolsCleanup({ stateDir, state: previous }); - await releasePreviousLease(client, previous, flags.daemonAuthToken); + return await releasePreviousLease(client, previous, { + nextDaemonBaseUrl, + ambientDaemonAuthToken: flags.daemonAuthToken, + cwd: process.cwd(), + env: process.env, + }); } function readRemoteConfigConnectionMetadata( From f42eeb4336baf8bbf06e14ea89103b1997ae5bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 17:20:52 +0200 Subject: [PATCH 3/5] fix(remote): stop merging ambient env defaults into the previous lease's own token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolvePreviousOwnDaemonAuthToken read the previous connection's profile through resolveRemoteConfigProfile, which folds AGENT_DEVICE_DAEMON_AUTH_TOKEN (and other env defaults) into the result. When the previous config file declared no token and the new connection's credential came from that same global env var, it was misclassified as belonging to the previous endpoint and sent there on forced-reconnect release — recreating the credential leak the prior fix was meant to close, just via env instead of --daemon-auth-token. Read the previous profile with the new readRemoteConfigFile (a provenance- preserving, file-only load with no ambient env/CLI merging), so only a token the previous config file itself declares can satisfy rule 1. Rules 2 and 3 are unchanged. --- src/__tests__/remote-connection.test.ts | 67 +++++++++++++++++++++++++ src/cli/commands/connection-runtime.ts | 12 ++++- src/remote/remote-config-core.ts | 10 +++- 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index dcb6a56921..370837664b 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -37,6 +37,7 @@ import type { AgentDeviceClient } from '../agent-device-client.ts'; afterEach(() => { vi.clearAllMocks(); vi.restoreAllMocks(); + vi.unstubAllEnvs(); }); const unexpectedCommandCall = async (): Promise => { @@ -2169,6 +2170,72 @@ test('connect --force skips releasing the previous lease when its token cannot b fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('connect --force does not misclassify an env-sourced new token as the previous connection’s own credential', async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-force-env-token-'); + const stateDir = path.join(tempRoot, '.state'); + const oldRemoteConfigPath = path.join(tempRoot, 'old-remote.json'); + const newRemoteConfigPath = path.join(tempRoot, 'new-remote.json'); + // Neither config file declares daemonAuthToken; the only source of a token + // anywhere is the environment, which is global and belongs to the *new* + // connection, not provably to old.example. + fs.writeFileSync(oldRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://old.example' })); + fs.writeFileSync(newRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://new.example' })); + // Token B, supplied through the environment — not via --daemon-auth-token — + // which is precisely the gap a merged-profile read would miss. + vi.stubEnv('AGENT_DEVICE_DAEMON_AUTH_TOKEN', 'test-env-not-a-real-token'); + writeRemoteConnectionState({ + stateDir, + state: { + version: 1, + session: 'adc-android', + remoteConfigPath: oldRemoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(oldRemoteConfigPath), + tenant: 'acme', + runId: 'run-old', + leaseId: 'lease-old', + leaseBackend: 'android-instance', + daemon: { baseUrl: 'https://old.example' }, + connectedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + }); + let releaseRequest: Parameters[0] | undefined; + + const stdout = await captureStdout(async () => { + await connectCommand({ + positionals: [], + flags: { + json: false, + help: false, + version: false, + force: true, + stateDir, + remoteConfig: newRemoteConfigPath, + daemonBaseUrl: 'https://new.example', + // No daemonAuthToken flag: the ambient token below flows in purely + // through the environment, matching production's resolution chain. + tenant: 'acme', + runId: 'run-new', + session: 'adc-android', + platform: 'android', + }, + client: createTestClient({ + release: async (request) => { + releaseRequest = request; + return { released: true }; + }, + }), + }); + }); + + assert.equal(releaseRequest, undefined); + assert.match(stdout, /Could not release the previous lease lease-old/); + assert.match(stdout, /tenant acme, run run-old/); + assert.match(stdout, /old\.example/); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' })?.runId, 'run-new'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('connect --force reuses the ambient token to release the previous lease when the endpoint is unchanged', async () => { const tempRoot = mkdtempForTestSync('agent-device-connect-force-same-endpoint-'); const stateDir = path.join(tempRoot, '.state'); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index fc44daf2e5..129b57762c 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -3,6 +3,10 @@ import { resolveDaemonPaths } from '../../daemon/config.ts'; import { stopReactDevtoolsCompanion } from '../../client/client-react-devtools-companion.ts'; import { stopMetroTunnel } from '../../metro/metro.ts'; import { resolveRemoteConfigProfile } from '../../remote/remote-config.ts'; +// Provenance-preserving file-only read (no ambient env defaults merged in) — +// see resolvePreviousOwnDaemonAuthToken below for why this must not be +// resolveRemoteConfigProfile. +import { readRemoteConfigFile } from '../../remote/remote-config-core.ts'; import { deviceFieldsFromPublicPlatform, isIosFamily, @@ -535,7 +539,13 @@ function resolvePreviousOwnDaemonAuthToken( env: Record, ): string | undefined { try { - return resolveRemoteConfigProfile({ + // readRemoteConfigFile, not resolveRemoteConfigProfile: the latter merges + // ambient environment defaults (e.g. AGENT_DEVICE_DAEMON_AUTH_TOKEN) into + // the profile, which would let the *new* connection's env-sourced token + // masquerade as a credential that provably belongs to the *previous* + // endpoint. Only a token the previous config file itself declares counts + // here; the env fallback is rule 2's job, gated on matching endpoints. + return readRemoteConfigFile({ configPath: previous.remoteConfigPath, cwd, env, diff --git a/src/remote/remote-config-core.ts b/src/remote/remote-config-core.ts index 948e53ce70..1a768065ad 100644 --- a/src/remote/remote-config-core.ts +++ b/src/remote/remote-config-core.ts @@ -12,7 +12,15 @@ import { AppError } from '@agent-device/kernel/errors'; import { resolveUserPath } from '../utils/path-resolution.ts'; import { parseSourceValue } from '../utils/source-value.ts'; -function readRemoteConfigFile(options: RemoteConfigProfileOptions): ResolvedRemoteConfigProfile { +// Deliberately narrower than `resolveRemoteConfigProfile`: this reads only +// what the config *file itself* declares, with no ambient environment +// defaults merged in. Callers that need to know a credential provably +// belongs to a specific profile (not "some token was available from +// somewhere") must use this, not the env-merged resolver, or provenance is +// lost — an env var is global and cannot say which endpoint it belongs to. +export function readRemoteConfigFile( + options: RemoteConfigProfileOptions, +): ResolvedRemoteConfigProfile { const env = options.env ?? process.env; const resolvedPath = resolveRemoteConfigPath(options); if (!fs.existsSync(resolvedPath)) { From 2a8dbba738d10b28ba32f58606859fd10b07c370 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 18:43:57 +0000 Subject: [PATCH 4/5] fix(remote): verify the previous config file still speaks for its endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule 1 reads the previous connection's own config file to recover a credential that provably belongs to the previous endpoint. It re-read `previous.remoteConfigPath` and trusted whatever token that file holds *now* — but a config path is routinely reused, so "connect to A from ./remote.json, re-point ./remote.json at B, connect --force" classified B's token as A's own and sent it to A during lease release. Same cross-endpoint leak the env-merge fix closed, arriving through the file instead of the environment. The file must now still vouch for the previous endpoint, by either of two independent facts: its bytes still hash to the `remoteConfigHash` recorded at connect time (so it is literally the declaration that stood up the previous connection), or — if it changed — it still declares the same daemon base URL. The second is what keeps an ordinary credential rotation releasing its lease instead of orphaning one; endpoint equality, not the fact of an edit, is what separates rotation from re-pointing. Endpoint comparison runs both sides through `buildRemoteConnectionDaemonState`, the same normalizer that produced the stored `daemon.baseUrl`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017Rva4YGtSCAKJqH5PbpcCU --- src/__tests__/remote-connection.test.ts | 151 ++++++++++++++++++++++++ src/cli/commands/connection-runtime.ts | 53 ++++++++- 2 files changed, 202 insertions(+), 2 deletions(-) diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index 370837664b..2ae063749d 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -2236,6 +2236,157 @@ test('connect --force does not misclassify an env-sourced new token as the previ fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('connect --force does not treat a re-pointed config path’s token as the previous endpoint’s own', async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-force-repointed-path-'); + const stateDir = path.join(tempRoot, '.state'); + // ONE path, reused. The previous connection was made against old.example + // through this file; the file is then edited in place to describe a + // different endpoint with a different credential. Distinct old/new paths + // cannot express this: the leak is that `remoteConfigPath` still resolves, + // and still parses, while no longer describing the connection it is being + // consulted about. + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + fs.writeFileSync( + remoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://old.example', + daemonAuthToken: 'test-old-not-a-real-token', + }), + ); + writeRemoteConnectionState({ + stateDir, + state: { + version: 1, + session: 'adc-android', + remoteConfigPath, + // Recorded while the file still described old.example — the fact that + // makes the later edit detectable. + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + tenant: 'acme', + runId: 'run-old', + leaseId: 'lease-old', + leaseBackend: 'android-instance', + daemon: { baseUrl: 'https://old.example' }, + connectedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + }); + // The edit: same path, now endpoint B with token B. + fs.writeFileSync( + remoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://new.example', + daemonAuthToken: 'test-new-not-a-real-token', + }), + ); + let releaseRequest: Parameters[0] | undefined; + + const stdout = await captureStdout(async () => { + await connectCommand({ + positionals: [], + flags: { + json: false, + help: false, + version: false, + force: true, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://new.example', + tenant: 'acme', + runId: 'run-new', + session: 'adc-android', + platform: 'android', + }, + client: createTestClient({ + release: async (request) => { + releaseRequest = request; + return { released: true }; + }, + }), + }); + }); + + // No request at all — not merely a request carrying a different token. + assert.equal(releaseRequest, undefined); + assert.match(stdout, /Could not release the previous lease lease-old/); + assert.match(stdout, /old\.example/); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' })?.runId, 'run-new'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('connect --force still releases with a rotated credential when the config keeps the same endpoint', async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-force-rotated-token-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + const newRemoteConfigPath = path.join(tempRoot, 'new-remote.json'); + fs.writeFileSync( + remoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://old.example', + daemonAuthToken: 'test-old-not-a-real-token', + }), + ); + fs.writeFileSync(newRemoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://new.example' })); + writeRemoteConnectionState({ + stateDir, + state: { + version: 1, + session: 'adc-android', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + tenant: 'acme', + runId: 'run-old', + leaseId: 'lease-old', + leaseBackend: 'android-instance', + daemon: { baseUrl: 'https://old.example' }, + connectedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + }); + // The file changed — so the hash no longer matches — but it still describes + // old.example, so the rotated token is still that endpoint's own credential + // and must still release its lease. This is the case an edit-detecting rule + // must not break: refusing here would orphan leases on every key rotation. + fs.writeFileSync( + remoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://old.example', + daemonAuthToken: 'test-rotated-not-a-real-token', + }), + ); + let releaseRequest: Parameters[0] | undefined; + + await captureStdout(async () => { + await connectCommand({ + positionals: [], + flags: { + json: true, + help: false, + version: false, + force: true, + stateDir, + remoteConfig: newRemoteConfigPath, + daemonBaseUrl: 'https://new.example', + tenant: 'acme', + runId: 'run-new', + session: 'adc-android', + platform: 'android', + }, + client: createTestClient({ + release: async (request) => { + releaseRequest = request; + return { released: true }; + }, + }), + }); + }); + + assert.equal(releaseRequest?.leaseId, 'lease-old'); + assert.equal(releaseRequest?.daemonBaseUrl, 'https://old.example'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-rotated-not-a-real-token'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('connect --force reuses the ambient token to release the previous lease when the endpoint is unchanged', async () => { const tempRoot = mkdtempForTestSync('agent-device-connect-force-same-endpoint-'); const stateDir = path.join(tempRoot, '.state'); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 129b57762c..2c9289eb28 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -545,11 +545,23 @@ function resolvePreviousOwnDaemonAuthToken( // masquerade as a credential that provably belongs to the *previous* // endpoint. Only a token the previous config file itself declares counts // here; the env fallback is rule 2's job, gated on matching endpoints. - return readRemoteConfigFile({ + const { profile } = readRemoteConfigFile({ configPath: previous.remoteConfigPath, cwd, env, - }).profile.daemonAuthToken; + }); + if (!profile.daemonAuthToken) return undefined; + // The path alone is not provenance. `remoteConfigPath` names a file *now*, + // while the claim being made is about what that file declared when the + // previous connection was established — and a config path is routinely + // reused (edited in place, re-pointed at a second environment) between the + // two. Without this check, "connect to A from ./remote.json, re-point + // ./remote.json at B, connect --force" reads B's token as A's own and + // sends it to A during lease release: the same cross-endpoint leak the + // env-merge fix closed, arriving through the file instead. + return previousConfigStillSpeaksForPreviousEndpoint(previous, profile.daemonBaseUrl) + ? profile.daemonAuthToken + : undefined; } catch { // A missing/unparseable previous config is the "cannot authenticate" // case handled by the caller, not an error to propagate here. @@ -557,6 +569,43 @@ function resolvePreviousOwnDaemonAuthToken( } } +/** + * Whether the previous connection's config file can still vouch for a token as + * belonging to the previous connection's endpoint. + * + * Two independent ways to establish that, in order: + * + * 1. **The file has not changed since connect time.** `remoteConfigHash` is a + * hash of the file's bytes taken when the connection was recorded, so an + * exact match means this is literally the declaration that stood up the + * previous connection. Nothing further to verify. + * 2. **The file changed, but still declares the same endpoint.** The common + * benign case is a rotated credential in an otherwise unchanged profile, + * which is still the previous endpoint's own credential and should still + * release its lease. Endpoint equality is what separates that from the + * re-pointed-file case, so it — not the mere fact of an edit — is the test. + * + * The endpoint comparison runs both sides through + * `buildRemoteConnectionDaemonState`, the same normalizer that produced the + * stored `daemon.baseUrl`, so it compares like with like rather than raw + * strings that differ only by a trailing slash. + * + * A file that changed and no longer declares an endpoint at all cannot vouch + * for anything: the caller then falls back to rule 2 (matching endpoints) or + * reports the lease as unreleasable, which is a warning and an orphaned lease + * — the correct price for not sending a credential somewhere it may not belong. + */ +function previousConfigStillSpeaksForPreviousEndpoint( + previous: RemoteConnectionState, + declaredDaemonBaseUrl: string | undefined, +): boolean { + if (hashRemoteConfigFile(previous.remoteConfigPath) === previous.remoteConfigHash) return true; + const declared = buildRemoteConnectionDaemonState({ + daemonBaseUrl: declaredDaemonBaseUrl, + })?.baseUrl; + return declared !== undefined && declared === previous.daemon?.baseUrl; +} + export async function releasePreviousLease( client: AgentDeviceClient, previous: RemoteConnectionState, From 8fca92148b3667cd64c9ca8563e0bd49acb01207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 21:27:26 +0200 Subject: [PATCH 5/5] fix(remote): bind previous config token to its endpoint --- src/__tests__/remote-connection.test.ts | 67 +++++++++++++++++++++++++ src/cli/commands/connection-runtime.ts | 17 ++----- 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index 2ae063749d..f43370dd41 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -2314,6 +2314,73 @@ test('connect --force does not treat a re-pointed config path’s token as the p fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('connect --force does not trust an unchanged profile token for a CLI-overridden previous endpoint', async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-force-cli-override-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + // The profile has always described endpoint B. The previous connection used + // explicit CLI credentials for endpoint A, so the unchanged file hash proves + // only which file was loaded — not that its token authenticated endpoint A. + fs.writeFileSync( + remoteConfigPath, + JSON.stringify({ + daemonBaseUrl: 'https://new.example', + daemonAuthToken: 'test-new-not-a-real-token', + }), + ); + writeRemoteConnectionState({ + stateDir, + state: { + version: 1, + session: 'adc-android', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + tenant: 'acme', + runId: 'run-old', + leaseId: 'lease-old', + leaseBackend: 'android-instance', + // Effective previous endpoint A came from --daemon-base-url, overriding + // the profile's endpoint B when this state was recorded. + daemon: { baseUrl: 'https://old.example' }, + connectedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + }); + let releaseRequest: Parameters[0] | undefined; + + const stdout = await captureStdout(async () => { + await connectCommand({ + positionals: [], + flags: { + json: false, + help: false, + version: false, + force: true, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://new.example', + daemonAuthToken: 'test-new-not-a-real-token', + tenant: 'acme', + runId: 'run-new', + session: 'adc-android', + platform: 'android', + }, + client: createTestClient({ + release: async (request) => { + releaseRequest = request; + return { released: true }; + }, + }), + }); + }); + + assert.equal(releaseRequest, undefined); + assert.match(stdout, /Could not release the previous lease lease-old/); + assert.match(stdout, /old\.example/); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' })?.runId, 'run-new'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('connect --force still releases with a rotated credential when the config keeps the same endpoint', async () => { const tempRoot = mkdtempForTestSync('agent-device-connect-force-rotated-token-'); const stateDir = path.join(tempRoot, '.state'); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 2c9289eb28..51a769b367 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -573,17 +573,11 @@ function resolvePreviousOwnDaemonAuthToken( * Whether the previous connection's config file can still vouch for a token as * belonging to the previous connection's endpoint. * - * Two independent ways to establish that, in order: - * - * 1. **The file has not changed since connect time.** `remoteConfigHash` is a - * hash of the file's bytes taken when the connection was recorded, so an - * exact match means this is literally the declaration that stood up the - * previous connection. Nothing further to verify. - * 2. **The file changed, but still declares the same endpoint.** The common - * benign case is a rotated credential in an otherwise unchanged profile, - * which is still the previous endpoint's own credential and should still - * release its lease. Endpoint equality is what separates that from the - * re-pointed-file case, so it — not the mere fact of an edit — is the test. + * The file must explicitly declare the same endpoint recorded in the previous + * connection state. A matching file hash proves only that the file itself did + * not change; it does not prove that its endpoint/token were effective when + * CLI flags may have overridden them. Endpoint equality is the provenance + * boundary and also preserves the benign rotated-credential case. * * The endpoint comparison runs both sides through * `buildRemoteConnectionDaemonState`, the same normalizer that produced the @@ -599,7 +593,6 @@ function previousConfigStillSpeaksForPreviousEndpoint( previous: RemoteConnectionState, declaredDaemonBaseUrl: string | undefined, ): boolean { - if (hashRemoteConfigFile(previous.remoteConfigPath) === previous.remoteConfigHash) return true; const declared = buildRemoteConnectionDaemonState({ daemonBaseUrl: declaredDaemonBaseUrl, })?.baseUrl;