fix(cli): harden the Remote Control tunnel and lock

- align ws with the rest of the workspace and restage the web bundle
- report the lock holder when acquisition retries run out, instead of a
  raw EEXIST
- reject a non-positive pid in the lock file; process.kill(0, 0) signals
  our own process group and would pin the lock forever
- settle the startup promise when the relay loop throws outside its
  retry block
- buffer management frames between register_ack and the HTTP tunnel, so
  an immediate open_ws is not dropped
- refuse chunked request bodies and derive Content-Length from the bytes
  actually forwarded
- run every startup-cleanup step and keep the original error

Correct the guide: the relay does receive the token in the WebSocket
handshake, so it has to be trusted.

Author: elkaix

apps/pythinker-code/dist-web/.web-bundle-manifest.json

@@ -1,4 +1,4 @@
 {
-  "sourceHash": "c05b9ce73324f5a446966d9f931dc804f0f1e73e78daf14e1479d86e58669c1a",
+  "sourceHash": "d84e17f04092f5fb9afa9f4d323b614d3945ec8e33abbbc92f5793ad2c30959e",
   "sourceFileCount": 404
 }

apps/pythinker-code/package.json

@@ -108,7 +108,7 @@
     "semver": "^7.7.4",
     "smol-toml": "^1.6.1",
     "tsx": "^4.23.5",
-    "ws": "^8.18.0",
+    "ws": "^8.21.3",
     "yazl": "^3.3.1",
     "zod": "^4.3.6"
   },

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

@@ -73,13 +73,21 @@ export async function acquireRemoteControlLock(
       }
       return { release: () => releaseRemoteControlLock(lockPath, info.nonce) };
     } catch (error) {
-      if ((error as NodeJS.ErrnoException).code !== 'EEXIST' || attempt >= MAX_ACQUIRE_ATTEMPTS) {
+      if ((error as NodeJS.ErrnoException).code !== 'EEXIST') {
         throw error;
       }
+      // Read the holder even on the last attempt: a second process can recreate
+      // the lock between our unlink and our open, and a raw EEXIST tells the
+      // user nothing about who holds it or how to stop them.
       const holder = await readRemoteControlLock(lockPath);
       if (holder !== undefined && pidAlive(holder.pid)) {
         throw new RemoteControlAlreadyRunningError(holder);
       }
+      if (attempt >= MAX_ACQUIRE_ATTEMPTS) {
+        throw new Error(
+          `Unable to acquire the Remote Control lock at ${lockPath}. Another process keeps recreating it.`, { cause: error },
+        );
+      }
       await removeFile(lockPath);
     }
   }
@@ -131,6 +139,10 @@ function decodeLock(raw: string): RemoteControlLockInfo | undefined {
     const parsed = JSON.parse(raw) as Partial<RemoteControlLockDisk>;
     if (
       typeof parsed.pid === 'number' &&
+      // `process.kill(0, 0)` signals our own process group and reports "alive",
+      // so a corrupt `"pid": 0` would pin the lock forever.
+      Number.isInteger(parsed.pid) &&
+      parsed.pid > 0 &&
       typeof parsed.nonce === 'string' &&
       typeof parsed.local_origin === 'string' &&
       typeof parsed.device_id === 'string' &&

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

@@ -54,6 +54,7 @@ const RELAY_PING_INTERVAL_MS = 30_000;
 const RELAY_SILENCE_TIMEOUT_MS = 300_000;
 const BLOCKED_REQUEST_HEADERS = new Set([
   'authorization',
+  'content-length',
   'cookie',
   'host',
   'origin',
@@ -242,6 +243,17 @@ export function parseRawHttpRequest(raw: Buffer): ParsedRawHttpRequest {
     }
     headers.push([name, value]);
   }
+  // `transfer-encoding` is stripped before forwarding, so a chunked body would
+  // reach the local server with its chunk framing as entity data. The relay
+  // sends whole requests, so refuse the framing instead of decoding it.
+  if (
+    headers.some(
+      ([name, value]) =>
+        name.toLowerCase() === 'transfer-encoding' && value.toLowerCase().includes('chunked'),
+    )
+  ) {
+    throw new SyntaxError('chunked HTTP request bodies are not supported');
+  }
   return {
     method: match[1]!,
     path: match[2]!,
@@ -390,7 +402,11 @@ class RemoteControlClient {
       this.initialResolve = resolve;
       this.initialReject = reject;
     });
-    this.runPromise = this.run();
+    // `run()` settles `initial` from inside its loop, but a throw from outside
+    // that loop's try would leave the caller waiting forever.
+    this.runPromise = this.run().catch((error: unknown) => {
+      this.rejectInitial(error instanceof Error ? error : new Error(String(error)));
+    });
     await initial;
   }
 
@@ -473,6 +489,14 @@ class RemoteControlClient {
     }
 
     const managementEnd = waitForSocketEnd(management);
+    // The relay may send `open_ws` the moment it acknowledges registration.
+    // `waitForRelayMessage` has just detached its own listener, so buffer
+    // everything that lands before the HTTP tunnel is up and replay it.
+    const earlyManagement: RawData[] = [];
+    const bufferManagement = (data: RawData): void => {
+      earlyManagement.push(data);
+    };
+    management.on('message', bufferManagement);
     const http = await this.connectRelay(
       `/v1/remote/http?device_id=${encodeURIComponent(this.deviceId)}`,
     );
@@ -481,8 +505,10 @@ class RemoteControlClient {
     if (management.readyState !== WebSocket.OPEN) {
       throw new Error('management connection closed');
     }
+    management.off('message', bufferManagement);
     management.on('message', (data) => this.handleManagementMessage(data));
     http.on('message', (data) => this.handleHttpMessage(data));
+    for (const data of earlyManagement) this.handleManagementMessage(data);
     this.reconnectAttempt = 0;
     this.relayOnline = true;
     this.onStatus('relay_connected');
@@ -870,6 +896,8 @@ function requestLocalHttp(
         path: parsed.path,
         headers: [
           ...filterForwardRequestHeaders(parsed.headers, serverToken),
+          'Content-Length',
+          String(parsed.body.length),
           'Host',
           origin.host,
         ],

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

@@ -426,11 +426,24 @@ async function runServerInProcess(
   try {
     await hooks.onReady?.(running.address);
   } catch (error) {
-    try {
-      await hooks.onShutdown?.('startup_failed');
-    } finally {
-      await running.close();
-      await shutdownTelemetry({ timeoutMs: CLI_SHUTDOWN_TIMEOUT_MS });
+    // Every cleanup step runs even when an earlier one fails, and none of them
+    // may replace the startup error the caller needs to see.
+    for (const step of [
+      async () => hooks.onShutdown?.('startup_failed'),
+      async () => running.close(),
+      async () => shutdownTelemetry({ timeoutMs: CLI_SHUTDOWN_TIMEOUT_MS }),
+    ]) {
+      try {
+        await step();
+      } catch (cleanupError) {
+        running.logger.error(
+          {
+            err:
+              cleanupError instanceof Error ? cleanupError : new Error(String(cleanupError)),
+          },
+          'startup cleanup step failed',
+        );
+      }
     }
     throw error;
   }

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

@@ -117,16 +117,27 @@ describe('Remote Control HTTP forwarding', () => {
     );
     expect(parsed).toMatchObject({ method: 'POST', path: '/api/v1/messages?q=1' });
     expect(parsed.body.toString()).toBe('data');
+    // Content-Length is dropped here and re-derived from the body that is
+    // actually forwarded, so a relay cannot desync the local server with a
+    // length that disagrees with the bytes.
     expect(filterForwardRequestHeaders(parsed.headers, 'local-token')).toEqual([
       'X-Keep',
       'yes',
-      'Content-Length',
-      '4',
       'Authorization',
       'Bearer local-token',
     ]);
   });
 
+  it('refuses a chunked request body instead of forwarding its framing as data', () => {
+    expect(() =>
+      parseRawHttpRequest(
+        Buffer.from(
+          'POST /api/v1/messages HTTP/1.1\r\nHost: relay.example\r\nTransfer-Encoding: chunked\r\n\r\n4\r\ndata\r\n0\r\n\r\n',
+        ),
+      ),
+    ).toThrow(SyntaxError);
+  });
+
   it('rejects absolute-form and malformed request targets', () => {
     expect(() =>
       parseRawHttpRequest(Buffer.from('GET https://example.test/ HTTP/1.1\r\n\r\n')),

docs/guides/remote-control.md

@@ -26,7 +26,9 @@ The terminal prints a QR code, a link, and the path of a PNG copy of the QR code
 
 The link grants control of this machine. Do not share the link or the QR code.
 
-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.
+The link itself carries no access token: requests arrive through the tunnel, and the Pythinker Code process on this machine adds the bearer token to each one before it reaches the local server. The QR code, the printed link, and the PNG on disk hold no credential.
+
+The relay is a different matter. Pythinker Code authenticates to it with the same token, sent in the WebSocket handshake, and every request and response passes through it in the clear. Use a relay you operate or otherwise trust. Rotate the token with `pythinker web rotate-token` if a relay is ever compromised.
 
 ## Relay
 
@@ -40,4 +42,4 @@ pythinker rc --relay-origin https://relay.example.com
 
 ## Stop it
 
-Press `Ctrl+C`. The tunnel closes with the server.
+Press `Ctrl-C`. The tunnel closes with the server.