fix(relay): stop bystander instances from capturing the relay host
Two mitigations for local multi-instance contention over the shared relay identity: - A standby instance now waits a 2-minute grace period after the host claim frees before taking over, so a cleanly restarting host (app update or relaunch) — which reclaims at boot with no wait — always wins the restart window instead of stranding paired devices on another process. - Dev instances never host the relay passively: dev scripts set OPENCHAMBER_RELAY_HOST=off and the Electron dev shell is detected via OPENCHAMBER_ELECTRON_DEV. Explicit enable/pairing on such an instance still force-claims; OPENCHAMBER_RELAY_HOST=on overrides.
This commit is contained in:
@@ -13,8 +13,8 @@
|
||||
},
|
||||
"scripts": {
|
||||
"dev": "bun run build:watch",
|
||||
"dev:server": "bun server/index.js --port ${OPENCHAMBER_PORT:-3001}",
|
||||
"dev:server:watch": "nodemon --watch server --ext js --exec \"bun server/index.js --port ${OPENCHAMBER_PORT:-3001}\"",
|
||||
"dev:server": "OPENCHAMBER_RELAY_HOST=${OPENCHAMBER_RELAY_HOST:-off} bun server/index.js --port ${OPENCHAMBER_PORT:-3001}",
|
||||
"dev:server:watch": "OPENCHAMBER_RELAY_HOST=${OPENCHAMBER_RELAY_HOST:-off} nodemon --watch server --ext js --exec \"bun server/index.js --port ${OPENCHAMBER_PORT:-3001}\"",
|
||||
"build": "vite build",
|
||||
"build:watch": "vite build --watch",
|
||||
"type-check": "tsc --noEmit",
|
||||
|
||||
@@ -1815,6 +1815,14 @@ async function main(options = {}) {
|
||||
fs,
|
||||
process,
|
||||
}),
|
||||
// Dev/debug instances share the data dir (and thus the relay identity) with
|
||||
// the production instance, so they must not host the relay on their own —
|
||||
// paired devices would land on them. OPENCHAMBER_RELAY_HOST=off disables
|
||||
// passive hosting explicitly (dev scripts set it); the Electron dev shell is
|
||||
// covered via OPENCHAMBER_ELECTRON_DEV. OPENCHAMBER_RELAY_HOST=on overrides
|
||||
// both. Explicit enable/pairing on the instance still hosts regardless.
|
||||
allowPassiveHost: process.env.OPENCHAMBER_RELAY_HOST === 'on'
|
||||
|| (process.env.OPENCHAMBER_RELAY_HOST !== 'off' && process.env.OPENCHAMBER_ELECTRON_DEV !== '1'),
|
||||
// Relay demand = any paired device or pending pairing session that uses the
|
||||
// relay transport. Drives the auto on/off lifecycle.
|
||||
hasRelayDemand: async () => {
|
||||
|
||||
@@ -23,7 +23,7 @@ Host side (`packages/web/server/lib/relay/`):
|
||||
- `identity.js` — the host's stable identity: the long-lived signing keypair (shared with the push relay, defines the routing id) plus a long-lived encryption keypair (the E2EE trust anchor). Reused across restarts; never rotated implicitly.
|
||||
- `signing-key.js` — storage/derivation of the signing keypair and the routing id, shared with the notifications runtime.
|
||||
- `host-client.js` — the long-lived connection manager: one outbound control connection to the relay, a per-client data connection for each connected device, reconnect/backoff, and the E2EE responder handshake per connection.
|
||||
- `host-lock.js` — the per-machine host claim. Every local instance sharing the data dir shares the relay identity (same serverId), so concurrent relay hosts evict each other at the relay worker (`4001: Control replaced`) and paired devices land on whichever local process won last. The claim file (`<data-dir>/relay-host.lock`, `{ pid }`) makes this deterministic: `service.js` only starts the host when no LIVE process holds the claim (stale claims from dead pids are ignored), goes to `standby` otherwise, and a 30s watcher both takes over when the claimant dies and stands down when another process claims. Explicit user intent — creating a pairing link or hitting `/relay/enable` — force-claims; the previous holder's watcher sees the takeover and backs off instead of fighting. The claim is cooperative (the relay worker still enforces the single host slot); it only decides which process keeps retrying.
|
||||
- `host-lock.js` — the per-machine host claim. Every local instance sharing the data dir shares the relay identity (same serverId), so concurrent relay hosts evict each other at the relay worker (`4001: Control replaced`) and paired devices land on whichever local process won last. The claim file (`<data-dir>/relay-host.lock`, `{ pid }`) makes this deterministic: `service.js` only starts the host when no LIVE process holds the claim (stale claims from dead pids are ignored), goes to `standby` otherwise, and a 30s watcher both takes over when the claimant dies and stands down when another process claims. A standby watcher waits a 2-minute grace after the claim frees before taking over, so a cleanly restarting host (app update/relaunch) — which reclaims at boot with no wait — always wins the restart window over a bystander instance. Explicit user intent — creating a pairing link or hitting `/relay/enable` — force-claims; the previous holder's watcher sees the takeover and backs off instead of fighting. Instances created with `allowPassiveHost: false` (dev servers via `OPENCHAMBER_RELAY_HOST=off`, the Electron dev shell via `OPENCHAMBER_ELECTRON_DEV`; `OPENCHAMBER_RELAY_HOST=on` overrides) never start the host passively at all — boot, demand reconcile, and watcher takeover leave them in `standby`; only explicit enable/pairing hosts there. The claim is cooperative (the relay worker still enforces the single host slot); it only decides which process keeps retrying.
|
||||
- `tunnel-host.js` — the per-connection dispatcher: decrypts tunnel frames and forwards HTTP/SSE/WS to the local server over loopback, then streams responses back. Enforces a path allowlist and never injects credentials.
|
||||
- `e2ee.js`, `tunnel-codec.js` — host-side (JS) mirrors of the shared crypto and framing (see "Two implementations" below).
|
||||
|
||||
|
||||
@@ -70,6 +70,11 @@ export const createRelayService = ({
|
||||
// evict each other at the relay worker ("Control replaced") and devices land
|
||||
// on a random instance. Optional: without it, behavior is pre-lock.
|
||||
hostLock = null,
|
||||
// When false, this instance never starts the relay host on its own (boot,
|
||||
// demand reconcile, or claim-watch takeover) — only an explicit user action
|
||||
// (enable, pairing) force-claims. Dev/debug instances set this so they do not
|
||||
// capture paired devices from the production instance sharing the data dir.
|
||||
allowPassiveHost = true,
|
||||
logger = console,
|
||||
}) => {
|
||||
const identityRuntime = createRelayIdentityRuntime({ crypto, readSettingsFromDiskMigrated, writeSettingsToDisk, readSettingsStrict });
|
||||
@@ -80,6 +85,13 @@ export const createRelayService = ({
|
||||
// claimant dies; a running host stands down when another process claims.
|
||||
let claimWatchTimer = null;
|
||||
const CLAIM_WATCH_INTERVAL_MS = 30_000;
|
||||
// A standby instance does not grab a freed claim immediately: a clean restart
|
||||
// of the previous host (app update, relaunch) releases the claim for a short
|
||||
// while, and taking it during that window strands the devices on this —
|
||||
// possibly older — instance. The restarting host reclaims at boot without any
|
||||
// wait, so it always wins the window.
|
||||
const CLAIM_TAKEOVER_GRACE_MS = 120_000;
|
||||
let claimFreeSinceMs = null;
|
||||
|
||||
const readConfig = async () => {
|
||||
const settings = await readSettingsFromDiskMigrated();
|
||||
@@ -133,8 +145,19 @@ export const createRelayService = ({
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (status.state === 'standby' && hostLock.tryClaim()) {
|
||||
logger.warn('[Relay] host claim is free — taking over the relay host');
|
||||
if (status.state !== 'standby' || !allowPassiveHost) return;
|
||||
if (hostLock.liveClaimantPid() !== null) {
|
||||
claimFreeSinceMs = null;
|
||||
return;
|
||||
}
|
||||
if (claimFreeSinceMs === null) {
|
||||
claimFreeSinceMs = Date.now();
|
||||
return;
|
||||
}
|
||||
if (Date.now() - claimFreeSinceMs < CLAIM_TAKEOVER_GRACE_MS) return;
|
||||
if (hostLock.tryClaim()) {
|
||||
claimFreeSinceMs = null;
|
||||
logger.warn('[Relay] host claim stayed free — taking over the relay host');
|
||||
await start(relayUrl);
|
||||
}
|
||||
} catch (error) {
|
||||
@@ -149,10 +172,19 @@ export const createRelayService = ({
|
||||
if (!claimWatchTimer) return;
|
||||
clearInterval(claimWatchTimer);
|
||||
claimWatchTimer = null;
|
||||
claimFreeSinceMs = null;
|
||||
};
|
||||
|
||||
const start = async (relayUrl, { claim = 'try' } = {}) => {
|
||||
if (hostClient) return;
|
||||
if (claim !== 'force' && !allowPassiveHost) {
|
||||
status = {
|
||||
state: 'standby',
|
||||
lastError: 'passive relay hosting is disabled on this instance — enable the relay or create a pairing link to host here',
|
||||
connectedClients: 0,
|
||||
};
|
||||
return;
|
||||
}
|
||||
if (hostLock) {
|
||||
const claimed = claim === 'force' ? hostLock.forceClaim() : hostLock.tryClaim();
|
||||
if (!claimed) {
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import crypto from 'node:crypto';
|
||||
|
||||
import { createRelayService } from './service.js';
|
||||
|
||||
const makeService = (options = {}) => {
|
||||
// In-memory settings store with a pre-seeded relay identity so the service
|
||||
// never regenerates a signing key during the test.
|
||||
const { privateKey, publicKey } = crypto.generateKeyPairSync('ec', { namedCurve: 'P-256' });
|
||||
let settings = {
|
||||
relaySigningKey: {
|
||||
privateJwk: privateKey.export({ format: 'jwk' }),
|
||||
publicJwk: publicKey.export({ format: 'jwk' }),
|
||||
},
|
||||
privateRelay: { enabled: true, relayUrl: 'wss://relay.example.test/ws' },
|
||||
...options.settings,
|
||||
};
|
||||
const hostLock = {
|
||||
tryClaim: vi.fn(() => true),
|
||||
forceClaim: vi.fn(() => true),
|
||||
holdsClaim: vi.fn(() => true),
|
||||
liveClaimantPid: vi.fn(() => null),
|
||||
release: vi.fn(),
|
||||
};
|
||||
const service = createRelayService({
|
||||
crypto,
|
||||
readSettingsFromDiskMigrated: async () => settings,
|
||||
writeSettingsToDisk: async (next) => { settings = next; },
|
||||
readSettingsStrict: async () => settings,
|
||||
getLocalPort: () => 0,
|
||||
hasRelayDemand: options.hasRelayDemand ?? (async () => true),
|
||||
hostLock,
|
||||
allowPassiveHost: options.allowPassiveHost,
|
||||
logger: { warn: () => {} },
|
||||
});
|
||||
return { service, hostLock, getSettings: () => settings };
|
||||
};
|
||||
|
||||
describe('relay service passive hosting', () => {
|
||||
it('never claims or starts the host passively when passive hosting is disabled', async () => {
|
||||
const { service, hostLock } = makeService({ allowPassiveHost: false });
|
||||
try {
|
||||
await service.startIfEnabled();
|
||||
let status = await service.getStatus();
|
||||
expect(status.state).toBe('standby');
|
||||
expect(hostLock.tryClaim).not.toHaveBeenCalled();
|
||||
expect(hostLock.forceClaim).not.toHaveBeenCalled();
|
||||
|
||||
await service.reconcile();
|
||||
status = await service.getStatus();
|
||||
expect(status.state).toBe('standby');
|
||||
expect(status.lastError).toContain('passive relay hosting is disabled');
|
||||
expect(hostLock.tryClaim).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
service.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it('force-claims for an explicit pairing even when passive hosting is disabled', async () => {
|
||||
const { service, hostLock } = makeService({ allowPassiveHost: false });
|
||||
try {
|
||||
const candidate = await service.ensureEnabledForPairing();
|
||||
expect(candidate?.type).toBe('relay');
|
||||
expect(hostLock.forceClaim).toHaveBeenCalled();
|
||||
} finally {
|
||||
service.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user