fix(remote): a proxied host is reachability-unknown; scope remote: SSE per session

Review round 2 on #439.

1. The bare TCP probe connects to host:port, which a host behind a jump host
   or SOCKS proxy does not answer even while ssh works. Acting on that
   verdict drew a permanent banner over a healthy session, replaced a real
   "needs tmux" error with "not reachable" in quick-start, and - with a wake
   target - buffered every HTTP input for the life of the session, since the
   readiness poll could never succeed. `WakeableRemote` now carries
   `jumpHost`/`socksProxy`/`extraSshOptions`, and `isProbeable()` turns such
   a host into reachability-UNKNOWN: input is delivered, `checkReachable` /
   `checkHostReachable` answer `null` (never `false`), `ensureHostAwake`
   returns `'unprobeable'` (handled like `'no-target'`), the quick-start gate
   fires on `=== false` only, and `GET …/reachability` reports
   `reachable: null, probeable: false` so the banner has nothing to key on.
   A wake target can still be fired for it, blind: no readiness poll, no
   reattach, no toast - the response says only whether the packet went out.

2. `'remote:'` joins the session-scoped SSE prefixes. The create/attach wake
   has no session yet, so the registry names the requesting user
   (`ensureHostAwake({ requestedBy })` -> `username` in the payload) and
   `deriveSseHint` routes on it; with neither it fails closed to admins.
   Single-user mode is unaffected.

Smaller, from the same review:

- A flush write that fails now drops the remaining buffer (logged) instead
  of retaining it: the wake still resolved and marked the host reachable, so
  the retained chunk waited for the NEXT wake and was replayed hours later,
  after everything typed since. Same policy as the oversized paste.
- The banner polls on tab activation (a user action) and on its 30 s timer
  only for a host with a wake target; a timer connecting to a host Codeman
  cannot wake is the traffic invariant #2 rejects keepalives for. A proxied
  host is never polled.
- `probeRemoteHostReachable`, `runRemoteWakeCommand` and the default UDP
  socket refuse under VITEST, as remote-files.ts does. The guard caught a
  leak on the spot: `createDefaultRemoteWakeDeps({ probe })` overrode the
  probe but still polled readiness with the real one, so the shutdown test
  had been connecting to a production address. The poll now uses the
  injected probe.
- docs/remote-sessions.md is additions only again (the reformatting is
  gone); the architecture-invariants overlap resolved itself in the merge.

Live, against a throwaway instance with a non-routable ghost host: proxied
-> no probe, no wake, the genuine ssh error after 10 s; direct (control) ->
probe, magic packet, "did not come back" after the 40 s budget.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdGP4jUTjc9J2RYYykDrCG
This commit is contained in:
Randalix
2026-09-18 22:46:11 +02:00
co-authored by Claude Opus 5
parent e271a65e79
commit 1040f6c489
12 changed files with 609 additions and 83 deletions
+64 -2
View File
@@ -26,8 +26,12 @@ function fakeElement(): El {
return { hidden: false, textContent: '', disabled: false, classList: { add() {}, remove() {} } };
}
const PROXIED_ID = 'remote-session-proxied';
const NOWOL_ID = 'remote-session-nowol';
/** Load `host-wake-ui.js` with the minimal DOM it touches, and return a wired app. */
function loadWakeApp() {
const fetches: string[] = [];
const elements = new Map<string, El>([
['hostWakeBanner', fakeElement()],
['hostWakeBannerText', fakeElement()],
@@ -40,7 +44,10 @@ function loadWakeApp() {
console,
setInterval: () => 1,
clearInterval: () => {},
fetch: () => Promise.resolve({ json: () => Promise.resolve({ success: false }) }),
fetch: (url: string) => {
fetches.push(url);
return Promise.resolve({ json: () => Promise.resolve({ success: false }) });
},
document: {
visibilityState: 'visible',
getElementById: (id: string) => elements.get(id) ?? null,
@@ -58,9 +65,27 @@ function loadWakeApp() {
REMOTE_ID,
{ remote: { hostId: 'hufflepuff', host: '192.168.50.137', label: 'Hufflepuff', wakeMac: '04:d9:f5:80:c6:58' } },
],
[
PROXIED_ID,
{
remote: {
hostId: 'bastioned',
host: '10.20.0.5',
label: 'Behind bastion',
jumpHost: 'bastion',
wakeMac: '04:d9:f5:80:c6:58',
},
},
],
[NOWOL_ID, { remote: { hostId: 'plain', host: '10.0.0.9', label: 'Plain' } }],
[LOCAL_ID, {}],
]);
return { app, banner: elements.get('hostWakeBanner') as El, text: elements.get('hostWakeBannerText') as El };
return {
app,
fetches,
banner: elements.get('hostWakeBanner') as El,
text: elements.get('hostWakeBannerText') as El,
};
}
describe('host wake banner visibility', () => {
@@ -120,3 +145,40 @@ describe('host wake banner visibility', () => {
expect(banner.hidden).toBe(true);
});
});
describe('host wake banner polling', () => {
// Each poll is a TCP connect to the host from the server. The timer is the one
// trigger that is not a user action, so it must not fire for a host Codeman could
// not wake anyway (it cannot wake it, but it can keep an activity-based suspend timer
// from firing), and a proxied host is never polled: the probe cannot reach it.
const tick = (app: Record<string, unknown>, periodic: boolean) =>
(app._hostWakeTick as (o: { periodic: boolean }) => void)({ periodic });
it('polls a wake-configured host on activation and on the timer', () => {
const { app, fetches } = loadWakeApp();
app.activeSessionId = REMOTE_ID;
tick(app, false);
tick(app, true);
tick(app, true);
expect(fetches).toHaveLength(3);
expect(fetches[0]).toContain(`/api/sessions/${REMOTE_ID}/reachability`);
});
it('polls a host without a wake target once on activation, never on the timer', () => {
const { app, fetches } = loadWakeApp();
app.activeSessionId = NOWOL_ID;
tick(app, false);
tick(app, true);
tick(app, true);
expect(fetches).toHaveLength(1);
});
it('never polls a host behind a jump host or SOCKS proxy', () => {
const { app, fetches } = loadWakeApp();
app.activeSessionId = PROXIED_ID;
tick(app, false);
tick(app, true);
expect(fetches).toHaveLength(0);
expect((app._hostWake as { probeable: boolean }).probeable).toBe(false);
});
});
+175 -3
View File
@@ -22,8 +22,11 @@ import {
buildMagicPacket,
createDefaultRemoteWakeDeps,
decideRemoteInputAction,
isProbeable,
parseMacList,
probeRemoteHostReachable,
resolveWakeTarget,
runRemoteWakeCommand,
sendWakePackets,
waitUntilRemoteReady,
wakeConfigured,
@@ -217,6 +220,7 @@ interface Harness {
writeViaMux: ReturnType<typeof vi.fn>;
noteReconnected: ReturnType<typeof vi.fn>;
events: string[];
payloads: Array<{ event: string; payload: Record<string, unknown> }>;
}
function harness(
@@ -229,6 +233,7 @@ function harness(
const writeViaMux = vi.fn(async () => !opts.writesFail);
const noteReconnected = vi.fn();
const events: string[] = [];
const payloads: Array<{ event: string; payload: Record<string, unknown> }> = [];
const deps: RemoteWakeDeps = {
probe,
@@ -236,7 +241,10 @@ function harness(
waitUntilReady,
delay: async () => {},
noteReconnected,
broadcast: (event) => events.push(event),
broadcast: (event, payload) => {
events.push(event);
payloads.push({ event, payload });
},
log: () => {},
...(opts.resolveRemote ? { resolveRemote: opts.resolveRemote } : {}),
};
@@ -258,6 +266,7 @@ function harness(
writeViaMux,
noteReconnected,
events,
payloads,
};
}
@@ -361,15 +370,24 @@ describe('RemoteWakeRegistry', () => {
expect(h.events).not.toContain('remote:sessionReconnected');
});
it('retains input that could not be written and reports nothing lost', async () => {
it('drops the buffer when a flush write fails, so nothing is replayed by a later wake', async () => {
// Retaining the chunk was the earlier behaviour, and it was worse: the wake still
// resolves and marks the host reachable, so the next input takes the deliver path
// while the retained chunk waits for the NEXT wake — replayed hours later, after
// everything typed since. Same policy as the oversized paste: dropped, logged.
const h = harness({ writesFail: true });
h.probe.mockResolvedValue(false);
await h.registry.handleInput(h.session, 'abc');
await h.registry.handleInput(h.session, 'def');
await h.registry.wake(h.session);
expect(h.writeViaMux).toHaveBeenCalledTimes(1);
expect(h.registry.pendingBytes('sess-1')).toBe(3);
expect(h.registry.pendingBytes('sess-1')).toBe(0);
// And the recovered host takes the deliver path from here, with nothing behind it.
h.probe.mockClear();
await expect(h.registry.handleInput(h.session, 'g')).resolves.toBe('deliver');
expect(h.registry.pendingBytes('sess-1')).toBe(0);
});
it('flushes the chunk it is writing out of the buffer first, so a concurrent enqueue cannot drop a different one', async () => {
@@ -528,6 +546,160 @@ describe('RemoteWakeRegistry', () => {
// ========== Host-scoped wake (session create/attach) ==========
describe('isProbeable', () => {
const base: WakeableRemote = { hostId: 'h', label: 'H', host: '10.0.0.9', wakeMac: '04:d9:f5:80:c6:58' };
it('is true for a host reached directly', () => {
expect(isProbeable(base)).toBe(true);
expect(isProbeable({ ...base, extraSshOptions: ['ServerAliveCountMax=3', 'StrictHostKeyChecking=no'] })).toBe(true);
});
it('is false behind a jump host, a SOCKS proxy, or a ProxyCommand/ProxyJump option', () => {
expect(isProbeable({ ...base, jumpHost: 'bastion.example' })).toBe(false);
expect(isProbeable({ ...base, socksProxy: '127.0.0.1:1080' })).toBe(false);
expect(isProbeable({ ...base, extraSshOptions: ['ProxyCommand=cloudflared access ssh --hostname %h'] })).toBe(
false
);
expect(isProbeable({ ...base, extraSshOptions: ['proxyjump=bastion'] })).toBe(false);
});
});
describe('RemoteWakeRegistry — a proxied host is reachability-unknown', () => {
// The bare TCP probe connects to `host:port`, which a jump-host/SOCKS host does not
// answer even while ssh works. Acting on that verdict buffered input for the life of
// the session (the readiness poll could never succeed), showed a permanent banner and
// hid the real ssh error behind "not reachable". Unknown is not asleep.
const proxied: WakeableRemote = {
hostId: 'behind-bastion',
label: 'Behind bastion',
host: '10.20.0.5',
jumpHost: 'bastion.example',
wakeCommand: '/usr/local/bin/wake-behind-bastion',
};
it('delivers every input without probing, buffering or waking', async () => {
const h = harness({ remote: proxied });
await expect(h.registry.handleInput(h.session, 'ls\r')).resolves.toBe('deliver');
await expect(h.registry.handleInput(h.session, 'pwd\r')).resolves.toBe('deliver');
expect(h.probe).not.toHaveBeenCalled();
expect(h.wake).not.toHaveBeenCalled();
expect(h.registry.pendingBytes('sess-1')).toBe(0);
});
it('answers null (unknown), never false, so the UI has no banner to raise', async () => {
const h = harness({ remote: proxied });
await expect(h.registry.checkReachable(h.session, { force: true })).resolves.toBeNull();
await expect(h.registry.checkHostReachable(proxied, { force: true })).resolves.toBeNull();
expect(h.probe).not.toHaveBeenCalled();
});
it('does not gate a create/attach request on it (unprobeable, like no-target)', async () => {
const h = harness({ remote: proxied });
await expect(h.registry.ensureHostAwake(proxied)).resolves.toBe('unprobeable');
expect(h.probe).not.toHaveBeenCalled();
expect(h.wake).not.toHaveBeenCalled();
});
it('lets the send-and-wait path through, and fires the manual wake blind', async () => {
const h = harness({ remote: proxied });
await expect(h.registry.ensureAwake(h.session)).resolves.toBe(true);
expect(h.wake).not.toHaveBeenCalled();
// The button: the user asked, so the target goes out — but nothing can verify the
// host came back, so there is no readiness poll, no reattach and no "waking" toast
// promising a wait that does not happen.
await expect(h.registry.ensureAwake(h.session, { force: true })).resolves.toBe(true);
expect(h.wake).toHaveBeenCalledTimes(1);
expect(h.waitUntilReady).not.toHaveBeenCalled();
expect(h.reattachRemote).not.toHaveBeenCalled();
expect(h.events).toEqual([]);
h.wake.mockResolvedValueOnce(false);
await expect(h.registry.ensureAwake(h.session, { force: true })).resolves.toBe(false);
// A wake IO that throws is a failed wake, not a rejected route — and the public
// `wake()` takes the same blind path, so nobody can poll readiness through a proxy.
h.wake.mockRejectedValueOnce(new Error('udp socket exploded'));
await expect(h.registry.wake(h.session)).resolves.toBe(false);
expect(h.waitUntilReady).not.toHaveBeenCalled();
});
});
describe('RemoteWakeRegistry — SSE payload routing', () => {
const hostRemote: WakeableRemote = {
hostId: 'hufflepuff',
label: 'Hufflepuff',
host: '192.168.50.137',
wakeMac: '04:d9:f5:80:c6:58',
};
it('a session wake names its session, so the server routes it to the owner', async () => {
const h = harness({ remote: hostRemote });
h.probe.mockResolvedValue(false);
await h.registry.handleInput(h.session, 'x');
await h.registry.wake(h.session);
const waking = h.payloads.find((p) => p.event === 'remote:hostWaking')!;
expect(waking.payload).toMatchObject({ sessionId: 'sess-1', hostId: 'hufflepuff', label: 'Hufflepuff' });
expect(waking.payload).not.toHaveProperty('username');
});
it('a create/attach wake has no session, so it names the requesting user instead', async () => {
// Without it the server can only fail closed (admins only) — the requester would
// never see their own wake. The payload carries `hostId`/`label`, which non-admins
// are not shown elsewhere, so it must not go global either.
const h = harness({ remote: hostRemote });
h.probe.mockResolvedValue(false);
h.waitUntilReady.mockResolvedValue(false);
await expect(h.registry.ensureHostAwake(hostRemote, { requestedBy: 'alice' })).resolves.toBe('failed');
const [waking, failed] = ['remote:hostWaking', 'remote:hostWakeFailed'].map(
(event) => h.payloads.find((p) => p.event === event)!.payload
);
expect(waking).toMatchObject({ forNewSession: true, username: 'alice' });
expect(failed).toMatchObject({ forNewSession: true, username: 'alice' });
expect(waking).not.toHaveProperty('sessionId');
});
it('omits the requester when the route did not name one (single-user mode)', async () => {
const h = harness({ remote: hostRemote });
h.probe.mockResolvedValue(false);
await h.registry.ensureHostAwake(hostRemote);
expect(h.payloads.find((p) => p.event === 'remote:hostWaking')!.payload).not.toHaveProperty('username');
});
});
describe('real IO is refused under vitest', () => {
// Every consumer injects its IO (RemoteWakeDeps, the socket factory). The guard is
// what makes that seam mandatory: a test that reaches the defaults fails loudly here
// instead of opening a TCP connection, spawning a process or broadcasting UDP from CI.
const target: WakeableRemote = { hostId: 'h', label: 'H', host: '127.0.0.1', port: 1 };
it('the TCP probe', () => {
expect(() => probeRemoteHostReachable(target)).toThrow(/disabled under test/);
});
it('the wake command', () => {
expect(() => runRemoteWakeCommand('/bin/true')).toThrow(/disabled under test/);
});
it('the UDP broadcast — only with the DEFAULT socket, an injected one still works', async () => {
await expect(sendWakePackets([[1, 2, 3, 4, 5, 6]])).rejects.toThrow(/disabled under test/);
});
it('the readiness poll, which probes by default', async () => {
await expect(waitUntilRemoteReady(target, { timeoutMs: 10, intervalMs: 1 })).rejects.toThrow(/disabled under test/);
});
it('the default deps poll readiness with the INJECTED probe, never the real one', async () => {
// `createDefaultRemoteWakeDeps({ probe })` used to override `probe` alone while
// `waitUntilReady` kept the module default — so a shutdown test polled a production
// address until the guard above made it fail instead of connecting.
const probe = vi.fn(async () => true);
const deps = createDefaultRemoteWakeDeps({ probe });
await expect(deps.waitUntilReady(target, { timeoutMs: 10 })).resolves.toBe(true);
expect(probe).toHaveBeenCalledWith(target);
});
});
describe('RemoteWakeRegistry — host-scoped wake for a request that waits on it', () => {
const hostRemote: WakeableRemote = {
hostId: 'hufflepuff',
+24
View File
@@ -151,6 +151,19 @@ describe('POST /api/sessions/:id/input — wake-on-LAN', () => {
expect(h.wake).not.toHaveBeenCalled();
});
it('writes straight into a proxied host with a wake target: no probe, no buffer, no wake', async () => {
// With a target configured, the old verdict buffered EVERY input for the life of
// the session: the readiness poll can never succeed through a proxy, so nothing was
// ever flushed (three inputs, nothing written, buffer non-empty — reproduced upstream).
const h = await harness({ remote: { ...remoteSession, socksProxy: '127.0.0.1:1080' }, hostUp: false });
const session = h.ctx.sessions.get(SESSION_ID)!;
for (const input of ['a', 'b', 'c']) expect((await send(h.app, { input, useMux: true })).statusCode).toBe(200);
expect(session.writeBuffer).toEqual(['a', 'b', 'c']);
expect(h.probe).not.toHaveBeenCalled();
expect(h.wake).not.toHaveBeenCalled();
expect(h.registry.pendingBytes(SESSION_ID)).toBe(0);
});
it('wakes before writing on the send-and-wait path (no buffering, the response waits anyway)', async () => {
const h = await harness({ hostUp: false });
const session = h.ctx.sessions.get(SESSION_ID)!;
@@ -188,6 +201,17 @@ describe('GET /api/sessions/:id/reachability', () => {
expect(body.data.reachable).toBe(false);
expect(body.data.wakeConfigured).toBe('none');
});
it('reports a proxied host as unknown, not unreachable, and never probes it', async () => {
// A jump-host / SOCKS host does not answer the bare TCP probe even while ssh works;
// `reachable:false` here drew a permanent banner over a healthy session.
const h = await harness({ remote: { ...remoteSession, jumpHost: 'bastion.example' }, hostUp: false });
const body = (await get(h.app, `/api/sessions/${SESSION_ID}/reachability`)).json();
expect(body.data.reachable).toBeNull();
expect(body.data.probeable).toBe(false);
expect(body.data.wakeConfigured).toBe('command');
expect(h.probe).not.toHaveBeenCalled();
});
});
describe('POST /api/sessions/:id/wake', () => {
+56
View File
@@ -0,0 +1,56 @@
/**
* @fileoverview Multi-user routing of the `remote:*` SSE family (server.ts `deriveSseHint`).
*
* The wake events carry `hostId`/`label`, which `GET /api/remote-hosts` withholds from
* non-admins, and their toast fires before any session check on the client — so an
* event that falls through to the global branch shows every logged-in user "Waking
* <label>" for a session they do not own. Constructs the server without starting it:
* the hint is a pure function of the event, the payload and the sessions map.
*/
import { describe, expect, it } from 'vitest';
import { WebServer } from '../src/web/server.js';
type Hint = { owner?: string; username?: string; adminOnly?: boolean; sessionScoped?: boolean } | undefined;
function hintFor(event: string, payload: Record<string, unknown>, owners: Record<string, string> = {}): Hint {
const server = new WebServer(3999, false, true) as unknown as {
sessions: Map<string, { owner?: string }>;
deriveSseHint(event: string, data: unknown): Hint;
};
for (const [id, owner] of Object.entries(owners)) server.sessions.set(id, { owner });
return server.deriveSseHint(event, payload);
}
describe('deriveSseHint — remote: events are session-scoped', () => {
it('routes a session wake to that session’s owner', () => {
expect(hintFor('remote:hostWaking', { sessionId: 's1', hostId: 'h', label: 'H' }, { s1: 'alice' })).toEqual({
owner: 'alice',
sessionScoped: true,
});
expect(hintFor('remote:sessionReconnected', { sessionId: 's1' }, { s1: 'alice' })).toEqual({
owner: 'alice',
sessionScoped: true,
});
});
it('routes a create/attach wake (no session yet) to the user who asked for it', () => {
expect(hintFor('remote:hostWaking', { forNewSession: true, username: 'bob', hostId: 'h', label: 'H' })).toEqual({
username: 'bob',
sessionScoped: true,
});
expect(hintFor('remote:hostWakeFailed', { forNewSession: true, username: 'bob', hostId: 'h' })).toEqual({
username: 'bob',
sessionScoped: true,
});
});
it('fails closed (admins only) when it names neither a session nor a requester', () => {
const hint = hintFor('remote:hostWaking', { forNewSession: true, hostId: 'h', label: 'H' });
expect(hint).toEqual({ owner: undefined, sessionScoped: true });
});
it('never lets a wake event reach the global branch', () => {
expect(hintFor('remote:hostWaking', {})).not.toBeUndefined();
expect(hintFor('remote:reconnectExhausted', {})).not.toBeUndefined();
});
});