Skip to content

Commit 3adad9a

Browse files
committed
fix(cli): keep the Remote Control link token-free and make the relay configurable
The tunnel client already adds the bearer token to every request it replays against the local server, so the link needs no credential. Drop the token fragment that was leaking the machine-wide server token into the terminal, the QR image on disk, and the remote browser's URL bar. Add --relay-origin and PYTHINKER_CODE_REMOTE_CONTROL_RELAY: this build ships no relay, so the relay had no way to be set outside tests. Restrict the lock file and the QR image to owner-only permissions.
1 parent b418982 commit 3adad9a

9 files changed

Lines changed: 113 additions & 17 deletions

File tree

apps/pythinker-code/src/cli/sub/web/remote-control-lock.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ export async function acquireRemoteControlLock(
5353
details: { localOrigin: string; deviceId: string; url: string },
5454
): Promise<RemoteControlLock> {
5555
const lockPath = remoteControlLockPath(homeDir);
56-
await mkdir(dirname(lockPath), { recursive: true });
56+
// The lock sits beside `server.token`; keep the same owner-only permissions.
57+
await mkdir(dirname(lockPath), { recursive: true, mode: 0o700 });
5758
const info: RemoteControlLockInfo = {
5859
pid: process.pid,
5960
nonce: randomBytes(8).toString('hex'),
@@ -64,7 +65,7 @@ export async function acquireRemoteControlLock(
6465
};
6566
for (let attempt = 0; ; attempt += 1) {
6667
try {
67-
const handle = await open(lockPath, 'wx');
68+
const handle = await open(lockPath, 'wx', 0o600);
6869
try {
6970
await handle.writeFile(encodeLock(info));
7071
} finally {

apps/pythinker-code/src/cli/sub/web/remote-control.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,26 @@ export const REMOTE_CONTROL_RELAY_ORIGIN = 'https://code-rc.pythinker.com';
1515

1616
export const REMOTE_CONTROL_FLAG_ENV = 'PYTHINKER_CODE_EXPERIMENTAL_REMOTE_CONTROL';
1717

18+
export const REMOTE_CONTROL_RELAY_ENV = 'PYTHINKER_CODE_REMOTE_CONTROL_RELAY';
19+
20+
/**
21+
* Resolve the relay to tunnel through. Pythinker ships no relay, so an operator
22+
* running their own points at it with `--relay-origin` or the env var; the
23+
* default constant is the last resort.
24+
*/
25+
export function resolveRelayOrigin(
26+
explicit?: string,
27+
env: Readonly<Record<string, string | undefined>> = process.env,
28+
): string {
29+
const candidate = explicit?.trim() ?? env[REMOTE_CONTROL_RELAY_ENV]?.trim() ?? '';
30+
if (candidate.length === 0) return REMOTE_CONTROL_RELAY_ORIGIN;
31+
const url = new URL(candidate);
32+
if (url.protocol !== 'http:' && url.protocol !== 'https:') {
33+
throw new Error(`Remote Control relay must be an http(s) URL: ${candidate}`);
34+
}
35+
return candidate;
36+
}
37+
1838
const TRUTHY_ENV_VALUES = new Set(['1', 'true', 'yes', 'on']);
1939

2040
export function isRemoteControlEnabled(
@@ -181,7 +201,6 @@ export function buildRemoteControlUrl(
181201
deviceId: string,
182202
sessionId?: string,
183203
relayOrigin = REMOTE_CONTROL_RELAY_ORIGIN,
184-
serverToken?: string,
185204
): string {
186205
const url = new URL(relayOrigin);
187206
const relayPath = url.pathname.replace(/\/+$/, '');
@@ -191,7 +210,7 @@ export function buildRemoteControlUrl(
191210
? `${devicePath}/`
192211
: `${devicePath}/sessions/${encodeURIComponent(sessionId)}`;
193212
url.search = new URLSearchParams({ rc: '1', from: 'pythinker_code_cli' }).toString();
194-
url.hash = serverToken === undefined ? '' : `token=${serverToken}`;
213+
url.hash = '';
195214
return url.toString();
196215
}
197216

@@ -294,7 +313,7 @@ export async function startRemoteControl(
294313
const relayOrigin = options.relayOrigin ?? REMOTE_CONTROL_RELAY_ORIGIN;
295314
const deviceId = createPythinkerDeviceId(options.homeDir);
296315
const deviceName = hostname();
297-
const url = buildRemoteControlUrl(deviceId, undefined, relayOrigin, options.localServerToken);
316+
const url = buildRemoteControlUrl(deviceId, undefined, relayOrigin);
298317
const lock = await acquireRemoteControlLock(options.homeDir, {
299318
localOrigin: options.localOrigin.replace(/\/+$/, ''),
300319
deviceId,

apps/pythinker-code/src/cli/sub/web/run.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
formatRemoteControlStatus,
4646
isRemoteControlEnabled,
4747
REMOTE_CONTROL_FLAG_ENV,
48+
resolveRelayOrigin,
4849
startRemoteControl,
4950
type RemoteControlHandle,
5051
type RemoteControlOptions,
@@ -78,6 +79,7 @@ interface RoutedServer {
7879
export interface WebCliOptions extends ServerCliOptions {
7980
open?: boolean;
8081
remoteControl?: boolean;
82+
relayOrigin?: string;
8183
}
8284

8385
export interface StartForegroundHooks {
@@ -182,6 +184,12 @@ export function buildWebCommand(
182184
.hideHelp(!isRemoteControlEnabled()),
183185
);
184186
}
187+
withServerOptions.addOption(
188+
new Option(
189+
'--relay-origin <url>',
190+
'Remote Control relay to tunnel through. Defaults to $PYTHINKER_CODE_REMOTE_CONTROL_RELAY.',
191+
).hideHelp(!isRemoteControlEnabled()),
192+
);
185193
return withServerOptions
186194
.option('--no-open', 'Do not open the web UI in the default browser.', true)
187195
.action(async (opts: WebCliOptions) => {
@@ -212,6 +220,7 @@ export async function handleWebCommand(
212220
if (opts.remoteControl === true && !isLoopbackHost(parsed.host)) {
213221
throw new Error('--remote-control requires a loopback host.');
214222
}
223+
const relayOrigin = opts.remoteControl === true ? resolveRelayOrigin(opts.relayOrigin) : undefined;
215224
const run = deps.startServerForeground ?? startServerForeground;
216225
let remoteControl: RemoteControlHandle | undefined;
217226
await run(parsed, {
@@ -241,6 +250,7 @@ export async function handleWebCommand(
241250
homeDir: dataDir,
242251
localOrigin: origin,
243252
localServerToken: token,
253+
relayOrigin,
244254
stderr: deps.stderr,
245255
onStatus,
246256
});

apps/pythinker-code/src/tui/commands/web.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
buildRemoteControlUrl,
66
formatRemoteControlOutput,
77
formatRemoteControlStatus,
8+
resolveRelayOrigin,
89
startRemoteControl,
910
type RemoteControlStatus,
1011
} from '#/cli/sub/web/remote-control';
@@ -61,6 +62,7 @@ export async function handleRemoteControlCommand(host: SlashCommandHost): Promis
6162

6263
host.setExitForegroundTask(async () => {
6364
const options = parseServerOptions({});
65+
const relayOrigin = resolveRelayOrigin();
6466
let remoteControl: Awaited<ReturnType<typeof startRemoteControl>> | undefined;
6567
try {
6668
await startServerForeground(options, {
@@ -79,9 +81,10 @@ export async function handleRemoteControlCommand(host: SlashCommandHost): Promis
7981
homeDir: dataDir,
8082
localOrigin: origin,
8183
localServerToken: token,
84+
relayOrigin,
8285
onStatus,
8386
});
84-
const url = buildRemoteControlUrl(remoteControl.deviceId, session?.id, undefined, token);
87+
const url = buildRemoteControlUrl(remoteControl.deviceId, session?.id, relayOrigin);
8588
const qrCode = await generateRemoteControlQr(url, dataDir);
8689
process.stdout.write(
8790
formatRemoteControlOutput({

apps/pythinker-code/src/utils/remote-control-qr.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ export async function generateRemoteControlQr(
2121
url: string,
2222
dataDir: string,
2323
): Promise<{ terminal: string; pngPath: string }> {
24-
await mkdir(dataDir, { recursive: true });
24+
await mkdir(dataDir, { recursive: true, mode: 0o700 });
2525
const pngPath = resolve(dataDir, 'rc-qrcode.png');
2626
const png = await QRCode.toBuffer(url, { type: 'png', margin: QR_PNG_MARGIN });
27-
await writeFile(pngPath, png);
27+
await writeFile(pngPath, png, { mode: 0o600 });
2828
const terminal = renderInlineImageQr(url, png) ?? renderTerminalQr(url);
2929
return { terminal, pngPath };
3030
}

apps/pythinker-code/test/cli/web/remote-control.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,32 @@ describe('Remote Control HTTP forwarding', () => {
168168
});
169169
});
170170

171+
describe('resolveRelayOrigin', () => {
172+
it('prefers the explicit value, then the env var, then the built-in default', async () => {
173+
const { resolveRelayOrigin, REMOTE_CONTROL_RELAY_ORIGIN } = await import(
174+
'#/cli/sub/web/remote-control'
175+
);
176+
expect(resolveRelayOrigin('https://relay.example.test', {})).toBe('https://relay.example.test');
177+
expect(
178+
resolveRelayOrigin(undefined, {
179+
PYTHINKER_CODE_REMOTE_CONTROL_RELAY: 'https://env.example.test',
180+
}),
181+
).toBe('https://env.example.test');
182+
expect(resolveRelayOrigin(undefined, {})).toBe(REMOTE_CONTROL_RELAY_ORIGIN);
183+
expect(resolveRelayOrigin(' ', { PYTHINKER_CODE_REMOTE_CONTROL_RELAY: ' ' })).toBe(
184+
REMOTE_CONTROL_RELAY_ORIGIN,
185+
);
186+
});
187+
188+
it('rejects a relay that is not http(s)', async () => {
189+
const { resolveRelayOrigin } = await import('#/cli/sub/web/remote-control');
190+
expect(() => resolveRelayOrigin('ws://relay.example.test', {})).toThrow(
191+
'Remote Control relay must be an http(s) URL',
192+
);
193+
expect(() => resolveRelayOrigin('not-a-url', {})).toThrow();
194+
});
195+
});
196+
171197
describe('Remote Control tunnel', () => {
172198
it('surfaces register_nak details', async () => {
173199
const homeDir = mkdtempSync(join(tmpdir(), 'pythinker-rc-nak-'));

apps/pythinker-code/test/cli/web/web.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,35 @@ describe('`pythinker web` opens the browser', () => {
404404
}
405405
});
406406

407+
it('passes the resolved relay origin to the tunnel', async () => {
408+
vi.stubEnv('PYTHINKER_CODE_EXPERIMENTAL_REMOTE_CONTROL', '1');
409+
const { handleWebCommand } = await import('#/cli/sub/web/run');
410+
const { runner } = makeRunner();
411+
const { stdout, stderr } = makeIo();
412+
const startRemoteControl = vi.fn(async () => ({
413+
deviceId: 'device-1',
414+
deviceName: 'example-device',
415+
url: 'https://relay.example.test/devices/device-1/?rc=1&from=pythinker_code_cli',
416+
close: async () => {},
417+
}));
418+
419+
await handleWebCommand(
420+
{ remoteControl: true, relayOrigin: 'https://relay.example.test', open: false },
421+
{
422+
startServerForeground: runner,
423+
openUrl: vi.fn(),
424+
resolveToken: () => 'tok-1',
425+
startRemoteControl,
426+
stdout,
427+
stderr,
428+
},
429+
);
430+
431+
expect(startRemoteControl).toHaveBeenCalledWith(
432+
expect.objectContaining({ relayOrigin: 'https://relay.example.test' }),
433+
);
434+
});
435+
407436
it('rejects Remote Control on a non-loopback host', async () => {
408437
vi.stubEnv('PYTHINKER_CODE_EXPERIMENTAL_REMOTE_CONTROL', '1');
409438
const { handleWebCommand } = await import('#/cli/sub/web/run');

apps/pythinker-code/test/tui/commands/web.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ describe('handleRemoteControlCommand', () => {
184184
}
185185
});
186186

187-
it('starts the tunnel and saves a session QR code carrying the server token', async () => {
187+
it('starts the tunnel and saves a token-free session QR code', async () => {
188188
vi.clearAllMocks();
189189
setCapabilities({ images: null, trueColor: true, hyperlinks: false });
190190
const { mkdtempSync, readFileSync, rmSync } = await import('node:fs');
@@ -196,7 +196,7 @@ describe('handleRemoteControlCommand', () => {
196196
const entryUrl =
197197
'https://code-rc.pythinker.com/devices/device-1/?rc=1&from=pythinker_code_cli';
198198
const sessionUrl =
199-
'https://code-rc.pythinker.com/devices/device-1/sessions/ses-1?rc=1&from=pythinker_code_cli#token=local-server-token';
199+
'https://code-rc.pythinker.com/devices/device-1/sessions/ses-1?rc=1&from=pythinker_code_cli';
200200
const pngPath = join(dataDir, 'rc-qrcode.png');
201201
mocks.getDataDir.mockReturnValue(dataDir);
202202
mocks.tryResolveServerToken.mockReturnValue('local-server-token');
@@ -244,7 +244,8 @@ describe('handleRemoteControlCommand', () => {
244244
const png = readFileSync(pngPath);
245245
expect(png.subarray(0, 8)).toEqual(Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]));
246246
expect(png).toEqual(await QRCode.toBuffer(sessionUrl));
247-
expect(written).toContain('#token=local-server-token');
247+
expect(written).not.toContain('local-server-token');
248+
expect(written).not.toContain('#token=');
248249
expect(close).toHaveBeenCalledOnce();
249250
} finally {
250251
writeSpy.mockRestore();
@@ -263,7 +264,6 @@ describe('handleRemoteControlCommand', () => {
263264
const dataDir = join(tempRoot, 'custom-home');
264265
const entryUrl =
265266
'https://code-rc.pythinker.com/devices/device-1/?rc=1&from=pythinker_code_cli';
266-
const entryUrlWithToken = `${entryUrl}#token=local-server-token`;
267267
mocks.getDataDir.mockReturnValue(dataDir);
268268
mocks.tryResolveServerToken.mockReturnValue('local-server-token');
269269
const close = vi.fn(async () => {});
@@ -300,12 +300,12 @@ describe('handleRemoteControlCommand', () => {
300300
const task = host.setExitForegroundTask.mock.calls[0]![0] as () => Promise<void>;
301301
await task();
302302

303-
expect(mocks.openUrl).toHaveBeenCalledWith(entryUrlWithToken);
303+
expect(mocks.openUrl).toHaveBeenCalledWith(entryUrl);
304304
const written = writeSpy.mock.calls.map((call) => String(call[0])).join('');
305-
expect(written).toContain(indentedQr(entryUrlWithToken));
305+
expect(written).toContain(indentedQr(entryUrl));
306306
expect(written).not.toContain('/sessions/');
307307
expect(readFileSync(join(dataDir, 'rc-qrcode.png'))).toEqual(
308-
await QRCode.toBuffer(entryUrlWithToken),
308+
await QRCode.toBuffer(entryUrl),
309309
);
310310
expect(close).toHaveBeenCalledOnce();
311311
} finally {

docs/guides/remote-control.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,19 @@ The terminal prints a QR code, a link, and the path of a PNG copy of the QR code
2424

2525
## Security
2626

27-
The link grants control of this machine. It carries the access token in its URL fragment, which the browser keeps to itself and never sends to the relay. Do not share the link or the QR code.
27+
The link grants control of this machine. Do not share the link or the QR code.
28+
29+
The link carries no access token. Requests reach the local server through the tunnel, and the Pythinker Code process on this machine adds the bearer token to each one, so the token never leaves this machine.
2830

2931
## Relay
3032

31-
Traffic reaches the remote device through a relay. Set `relayOrigin` to point at your own relay; the default is `https://code-rc.pythinker.com`.
33+
Traffic reaches the remote device through a relay. Point Remote Control at your own relay with `--relay-origin`:
34+
35+
```sh
36+
pythinker rc --relay-origin https://relay.example.com
37+
```
38+
39+
`PYTHINKER_CODE_REMOTE_CONTROL_RELAY` sets the same thing for `/rc` in the terminal UI and for every run in a shell.
3240

3341
## Stop it
3442

0 commit comments

Comments
 (0)