Skip to content

Commit 2108530

Browse files
committed
fix(cli): close the Remote Control lock write race and tighten artifact modes
- an unreadable lock is a rival mid-write, not a stale one: `open(…, 'wx')` publishes an empty file before its JSON lands, so removing it let both processes believe they held the lock. Re-read before sweeping, and sweep only once it is still unreadable at the end - chmod the QR directory and image, since mode only applies to paths the call creates - resolve the relay origin inside the try, so a malformed setting reports through the startup handler
1 parent 31dc00b commit 2108530

4 files changed

Lines changed: 20 additions & 3 deletions

File tree

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { randomBytes } from 'node:crypto';
2+
import { setTimeout as sleep } from 'node:timers/promises';
23
import { mkdir, open, readFile, unlink } from 'node:fs/promises';
34
import { dirname, join } from 'node:path';
45

@@ -47,6 +48,7 @@ export interface RemoteControlLock {
4748
}
4849

4950
const MAX_ACQUIRE_ATTEMPTS = 3;
51+
const ACQUIRE_RETRY_DELAY_MS = 25;
5052

5153
export async function acquireRemoteControlLock(
5254
homeDir: string,
@@ -83,6 +85,15 @@ export async function acquireRemoteControlLock(
8385
if (holder !== undefined && pidAlive(holder.pid)) {
8486
throw new RemoteControlAlreadyRunningError(holder);
8587
}
88+
// `open(…, 'wx')` publishes an empty file before its JSON is written, so
89+
// an unreadable lock may simply be a rival mid-write. Deleting it there
90+
// would let both processes believe they hold the lock. Give the writer a
91+
// moment and re-read; only sweep it once it is still unreadable at the
92+
// end, which is the genuinely corrupt case.
93+
if (holder === undefined && attempt < MAX_ACQUIRE_ATTEMPTS) {
94+
await sleep(ACQUIRE_RETRY_DELAY_MS);
95+
continue;
96+
}
8697
if (attempt >= MAX_ACQUIRE_ATTEMPTS) {
8798
throw new Error(
8899
`Unable to acquire the Remote Control lock at ${lockPath}. Another process keeps recreating it.`, { cause: error },

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,11 @@ export async function handleRemoteControlCommand(host: SlashCommandHost): Promis
6262

6363
host.setExitForegroundTask(async () => {
6464
const options = parseServerOptions({});
65-
const relayOrigin = resolveRelayOrigin();
6665
let remoteControl: Awaited<ReturnType<typeof startRemoteControl>> | undefined;
6766
try {
67+
// Inside the try: a malformed relay setting throws here, and the user
68+
// should see it through the same handler as any other startup failure.
69+
const relayOrigin = resolveRelayOrigin();
6870
await startServerForeground(options, {
6971
onReady: async (origin) => {
7072
const dataDir = getDataDir();

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdir, writeFile } from 'node:fs/promises';
1+
import { chmod, mkdir, writeFile } from 'node:fs/promises';
22
import { resolve } from 'node:path';
33

44
import {
@@ -22,9 +22,13 @@ export async function generateRemoteControlQr(
2222
dataDir: string,
2323
): Promise<{ terminal: string; pngPath: string }> {
2424
await mkdir(dataDir, { recursive: true, mode: 0o700 });
25+
// `mode` only applies to paths these calls create; tighten an existing dir
26+
// or an earlier run's image too.
27+
await chmod(dataDir, 0o700);
2528
const pngPath = resolve(dataDir, 'rc-qrcode.png');
2629
const png = await QRCode.toBuffer(url, { type: 'png', margin: QR_PNG_MARGIN });
2730
await writeFile(pngPath, png, { mode: 0o600 });
31+
await chmod(pngPath, 0o600);
2832
const terminal = renderInlineImageQr(url, png) ?? renderTerminalQr(url);
2933
return { terminal, pngPath };
3034
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ describe('Remote Control HTTP forwarding', () => {
146146
).toString();
147147
expect(html).not.toContain('</script><script>alert(1)');
148148
expect(html).toContain('\\u003c/script\\u003e');
149-
expect(html.match(/<script>/g)).toHaveLength(1);
149+
expect(html.split('<script>')).toHaveLength(2);
150150
});
151151

152152
it('rejects absolute-form and malformed request targets', () => {

0 commit comments

Comments
 (0)