Browse Source

feat(interview): add configurable port + harden server lifecycle (#266)

* feat(interview): add configurable port option to interview server

Add `port` field to InterviewConfigSchema allowing the interview HTTP
server to bind to a fixed port instead of a random ephemeral one.

- Schema: port 0-65535, default 0 (preserves current behavior)
- Server: accepts port via deps, passes to server.listen()
- Server: close() method for graceful shutdown
- Server: friendly EADDRINUSE error message
- Server: close failed instance on startup error
- Manager: threads config.interview.port to server
- Docs: config option + remote access section (Tailscale/Cloudflare/SSH)
- Docs: note privileged ports 1-1023
- Tests: fixed/random port binding, EADDRINUSE, Zod schema boundaries

* fix(interview): harden server lifecycle, security, and reliability

Server (server.ts):
- Destroy request stream on body-too-large (socket leak)
- closeAllConnections() on server shutdown
- Add requestTimeout/headersTimeout (30s/10s)
- Guard malformed URL with 400 instead of 500
- Return 500 for non-not-found errors in getState

Service (service.ts):
- Guard path traversal in resolveExistingInterviewPath
- Add monotonic counter to interview IDs

UI (ui.ts):
- Stop polling when interview is abandoned (setTimeout chain)
- Escape </script> in JS context to prevent XSS
ReqX 2 days ago
parent
commit
0cdca34714

+ 25 - 1
docs/interview.md

@@ -117,7 +117,8 @@ Inside the interview page:
     "interview": {
       "maxQuestions": 2,
       "outputFolder": "interview",
-      "autoOpenBrowser": true
+      "autoOpenBrowser": true,
+      "port": 0
     }
   }
 }
@@ -128,6 +129,29 @@ Inside the interview page:
 - `maxQuestions` — max questions per round, `1-10`, default `2`
 - `outputFolder` — where markdown files are written, default `interview`
 - `autoOpenBrowser` — open the localhost UI in your default browser, default `true`
+- `port` — fixed port for the interview UI server, `0-65535`, default `0` (OS-assigned). Set a fixed port for remote access via Tailscale Serve, Cloudflare Tunnel, or SSH tunneling. Note: ports 1-1023 require elevated privileges on most systems.
+
+## Remote access
+
+The interview UI binds to `127.0.0.1`. To access it from a remote machine:
+
+### Tailscale Serve
+
+```text
+tailscale serve --bg --https=443 http://127.0.0.1:<port>
+```
+
+### Cloudflare Tunnel
+
+```text
+cloudflared tunnel --url http://127.0.0.1:<port>
+```
+
+### SSH tunnel
+
+```text
+ssh -L <port>:127.0.0.1:<port> your-server
+```
 
 ## Good use cases
 

+ 1 - 0
src/config/schema.ts

@@ -168,6 +168,7 @@ export const InterviewConfigSchema = z.object({
   maxQuestions: z.number().int().min(1).max(10).default(2),
   outputFolder: z.string().min(1).default('interview'),
   autoOpenBrowser: z.boolean().default(true),
+  port: z.number().int().min(0).max(65535).default(0),
 });
 
 export type InterviewConfig = z.infer<typeof InterviewConfigSchema>;

+ 4 - 5
src/index.ts

@@ -473,10 +473,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
     'chat.headers': chatHeadersHook['chat.headers'],
 
     // Track which agent each session uses (needed for serve-mode prompt injection)
-    'chat.message': async (input: {
-      sessionID: string;
-      agent?: string;
-    }) => {
+    'chat.message': async (input: { sessionID: string; agent?: string }) => {
       if (input.agent) {
         sessionAgentMap.set(input.sessionID, input.agent);
       }
@@ -502,7 +499,9 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
         if (!alreadyInjected) {
           // Prepend the orchestrator prompt to the system array
           const { ORCHESTRATOR_PROMPT } = await import('./agents/orchestrator');
-          output.system[0] = ORCHESTRATOR_PROMPT + (output.system[0] ? '\n\n' + output.system[0] : '');
+          output.system[0] =
+            ORCHESTRATOR_PROMPT +
+            (output.system[0] ? '\n\n' + output.system[0] : '');
         }
       }
     },

+ 151 - 0
src/interview/interview.test.ts

@@ -1,6 +1,9 @@
 import { describe, expect, mock, test } from 'bun:test';
 import * as fs from 'node:fs/promises';
+import { createServer } from 'node:http';
 import * as path from 'node:path';
+import { InterviewConfigSchema } from '../config/schema';
+import { createInterviewServer } from './server';
 import { createInterviewService as createRealInterviewService } from './service';
 import type { InterviewAnswer } from './types';
 import { renderInterviewPage } from './ui';
@@ -1507,3 +1510,151 @@ describe('renderInterviewPage', () => {
     expect(html).not.toContain('https://ohmyopencodeslim.com');
   });
 });
+
+/** Discover a free port by briefly binding to port 0, then closing. */
+async function findFreePort(): Promise<number> {
+  return new Promise((resolve, reject) => {
+    const srv = createServer();
+    srv.listen(0, '127.0.0.1', () => {
+      const addr = srv.address();
+      srv.close(() => {
+        if (!addr || typeof addr === 'string') {
+          reject(new Error('Failed to find free port'));
+          return;
+        }
+        resolve(addr.port);
+      });
+    });
+    srv.on('error', reject);
+  });
+}
+
+describe('interview server port configuration', () => {
+  const noopDeps = {
+    getState: mock(
+      async (_id: string) =>
+        ({
+          interview: {
+            id: 'x',
+            idea: 'x',
+            status: 'active',
+            markdownPath: 'x',
+          },
+          questions: [],
+          mode: 'awaiting-agent' as const,
+          isBusy: false,
+        }) as any,
+    ),
+    submitAnswers: mock(async (_id: string, _answers: InterviewAnswer[]) => {}),
+  };
+
+  test('server starts on a specific port when port is non-zero', async () => {
+    const freePort = await findFreePort();
+    const server = createInterviewServer({ ...noopDeps, port: freePort });
+    try {
+      const baseUrl = await server.ensureStarted();
+      expect(baseUrl).toBe(`http://127.0.0.1:${freePort}`);
+    } finally {
+      server.close();
+    }
+  });
+
+  test('server starts on a random port when port is 0', async () => {
+    const server = createInterviewServer({ ...noopDeps, port: 0 });
+    try {
+      const baseUrl = await server.ensureStarted();
+      expect(baseUrl).toMatch(/^http:\/\/127\.0\.0\.1:\d+$/);
+      const port = Number.parseInt(baseUrl.split(':').pop()!, 10);
+      expect(port).toBeGreaterThan(0);
+    } finally {
+      server.close();
+    }
+  });
+
+  test('baseUrl contains the correct port number for fixed port', async () => {
+    const freePort = await findFreePort();
+    const server = createInterviewServer({ ...noopDeps, port: freePort });
+    try {
+      const baseUrl = await server.ensureStarted();
+      const port = Number.parseInt(baseUrl.split(':').pop()!, 10);
+      expect(port).toBe(freePort);
+    } finally {
+      server.close();
+    }
+  });
+
+  test('baseUrl contains a valid port number for random port', async () => {
+    const server = createInterviewServer({ ...noopDeps, port: 0 });
+    try {
+      const baseUrl = await server.ensureStarted();
+      const port = Number.parseInt(baseUrl.split(':').pop()!, 10);
+      expect(port).toBeGreaterThanOrEqual(1);
+      expect(port).toBeLessThanOrEqual(65535);
+    } finally {
+      server.close();
+    }
+  });
+
+  test('rejects with friendly error when port is already in use', async () => {
+    // Occupy a port first
+    const blocker = createServer();
+    const occupiedPort = await new Promise<number>((resolve, reject) => {
+      blocker.listen(0, '127.0.0.1', () => {
+        const addr = blocker.address();
+        if (!addr || typeof addr === 'string') {
+          reject(new Error('Failed to bind blocker'));
+          return;
+        }
+        resolve(addr.port);
+      });
+      blocker.on('error', reject);
+    });
+
+    const server = createInterviewServer({
+      ...noopDeps,
+      port: occupiedPort,
+    });
+    try {
+      await expect(server.ensureStarted()).rejects.toThrow(
+        `Interview server port ${occupiedPort} is already in use`,
+      );
+    } finally {
+      server.close();
+      blocker.close();
+    }
+  });
+});
+
+describe('InterviewConfigSchema port validation', () => {
+  test('accepts valid port 0', () => {
+    const result = InterviewConfigSchema.parse({ port: 0 });
+    expect(result.port).toBe(0);
+  });
+
+  test('accepts valid port 8080', () => {
+    const result = InterviewConfigSchema.parse({ port: 8080 });
+    expect(result.port).toBe(8080);
+  });
+
+  test('accepts valid port 65535', () => {
+    const result = InterviewConfigSchema.parse({ port: 65535 });
+    expect(result.port).toBe(65535);
+  });
+
+  test('defaults port to 0 when omitted', () => {
+    const result = InterviewConfigSchema.parse({});
+    expect(result.port).toBe(0);
+  });
+
+  test('rejects negative port', () => {
+    expect(() => InterviewConfigSchema.parse({ port: -1 })).toThrow();
+  });
+
+  test('rejects port above 65535', () => {
+    expect(() => InterviewConfigSchema.parse({ port: 70000 })).toThrow();
+  });
+
+  test('rejects float port', () => {
+    expect(() => InterviewConfigSchema.parse({ port: 3.5 })).toThrow();
+  });
+});

+ 1 - 0
src/interview/manager.ts

@@ -39,6 +39,7 @@ export function createInterviewManager(
     getState: async (interviewId) => service.getInterviewState(interviewId),
     submitAnswers: async (interviewId, answers) =>
       service.submitAnswers(interviewId, answers),
+    port: config.interview?.port ?? 0,
   });
 
   // Inject server URL resolver into service (lazy: server starts on first request)

+ 42 - 7
src/interview/server.ts

@@ -1,6 +1,7 @@
 import {
   createServer,
   type IncomingMessage,
+  type Server,
   type ServerResponse,
 } from 'node:http';
 import { URL } from 'node:url';
@@ -71,6 +72,7 @@ async function readJsonBody(request: IncomingMessage): Promise<unknown> {
     const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
     size += buffer.length;
     if (size > 64 * 1024) {
+      request.destroy();
       throw new Error('Request body too large');
     }
     chunks.push(buffer);
@@ -102,17 +104,26 @@ export function createInterviewServer(deps: {
     interviewId: string,
     answers: InterviewAnswer[],
   ) => Promise<void>;
+  port: number;
 }): {
   ensureStarted: () => Promise<string>;
+  close: () => void;
 } {
   let baseUrl: string | null = null;
   let startPromise: Promise<string> | null = null;
+  let activeServer: Server | null = null;
 
   async function handle(
     request: IncomingMessage,
     response: ServerResponse,
   ): Promise<void> {
-    const url = new URL(request.url ?? '/', 'http://127.0.0.1');
+    let url: URL;
+    try {
+      url = new URL(request.url ?? '/', 'http://127.0.0.1');
+    } catch {
+      sendJson(response, 400, { error: 'Invalid request URL' });
+      return;
+    }
     const pathname = url.pathname;
 
     if (request.method === 'GET' && pathname.startsWith('/interview/')) {
@@ -129,9 +140,10 @@ export function createInterviewServer(deps: {
         const state = await deps.getState(stateMatch[1]);
         sendJson(response, 200, state);
       } catch (error) {
-        sendJson(response, 404, {
-          error: error instanceof Error ? error.message : 'Interview not found',
-        });
+        const message =
+          error instanceof Error ? error.message : 'Interview not found';
+        const status = message === 'Interview not found' ? 404 : 500;
+        sendJson(response, status, { error: message });
       }
       return;
     }
@@ -180,13 +192,27 @@ export function createInterviewServer(deps: {
           });
         });
       });
+      server.requestTimeout = 30_000;
+      server.headersTimeout = 10_000;
 
-      server.on('error', (error) => {
+      activeServer = server;
+
+      server.on('error', (error: NodeJS.ErrnoException) => {
+        server.close();
+        activeServer = null;
         startPromise = null;
-        reject(error);
+        if (error.code === 'EADDRINUSE') {
+          reject(
+            new Error(
+              `Interview server port ${deps.port} is already in use. Choose a different port or set port to 0 for an OS-assigned port.`,
+            ),
+          );
+        } else {
+          reject(error);
+        }
       });
 
-      server.listen(0, '127.0.0.1', () => {
+      server.listen(deps.port, '127.0.0.1', () => {
         const address = server.address();
         if (!address || typeof address === 'string') {
           startPromise = null;
@@ -204,5 +230,14 @@ export function createInterviewServer(deps: {
 
   return {
     ensureStarted,
+    close: () => {
+      if (activeServer) {
+        activeServer.closeAllConnections();
+        activeServer.close();
+        activeServer = null;
+      }
+      baseUrl = null;
+      startPromise = null;
+    },
   };
 }

+ 23 - 2
src/interview/service.ts

@@ -164,6 +164,18 @@ async function ensureInterviewFile(record: InterviewRecord): Promise<void> {
 }
 
 async function readInterviewDocument(record: InterviewRecord): Promise<string> {
+  try {
+    return await fs.readFile(record.markdownPath, 'utf8');
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
+      // Path may have been updated by concurrent maybeRenameWithTitle
+      try {
+        return await fs.readFile(record.markdownPath, 'utf8');
+      } catch {
+        // Fall through to ensure + create
+      }
+    }
+  }
   await ensureInterviewFile(record);
   return fs.readFile(record.markdownPath, 'utf8');
 }
@@ -221,6 +233,7 @@ function resolveExistingInterviewPath(
 
   const outputDir = createInterviewDirectoryPath(directory, outputFolder);
   const candidates = new Set<string>();
+  const resolvedRoot = path.resolve(directory);
 
   if (path.isAbsolute(trimmed)) {
     candidates.add(trimmed);
@@ -236,6 +249,13 @@ function resolveExistingInterviewPath(
     if (path.extname(candidate) !== '.md') {
       continue;
     }
+    const resolved = path.resolve(candidate);
+    if (
+      !resolved.startsWith(resolvedRoot + path.sep) &&
+      resolved !== resolvedRoot
+    ) {
+      continue;
+    }
     if (fsSync.existsSync(candidate)) {
       return candidate;
     }
@@ -277,6 +297,7 @@ export function createInterviewService(
   const sessionBusy = new Map<string, boolean>();
   const browserOpened = new Set<string>(); // Track interviews that have opened browser
   let resolveBaseUrl: (() => Promise<string>) | null = null;
+  let idCounter = 0;
 
   function setBaseUrlResolver(resolver: () => Promise<string>): void {
     resolveBaseUrl = resolver;
@@ -380,7 +401,7 @@ export function createInterviewService(
 
     const messages = await loadMessages(sessionID);
     const record: InterviewRecord = {
-      id: `${Date.now()}-${slugify(idea) || 'interview'}`,
+      id: `${Date.now()}-${++idCounter}-${slugify(idea) || 'interview'}`,
       sessionID,
       idea: normalizedIdea,
       markdownPath: createInterviewFilePath(ctx.directory, outputFolder, idea),
@@ -415,7 +436,7 @@ export function createInterviewService(
     const messages = await loadMessages(sessionID);
     const title = extractTitle(document);
     const record: InterviewRecord = {
-      id: `${Date.now()}-${slugify(path.basename(markdownPath, '.md')) || 'interview'}`,
+      id: `${Date.now()}-${++idCounter}-${slugify(path.basename(markdownPath, '.md')) || 'interview'}`,
       sessionID,
       idea: title || path.basename(markdownPath, '.md'),
       markdownPath,

+ 9 - 4
src/interview/ui.ts

@@ -309,7 +309,7 @@ export function renderInterviewPage(interviewId: string): string {
     </div>
 
     <script>
-      const interviewId = ${JSON.stringify(interviewId)};
+      const interviewId = ${JSON.stringify(interviewId).replace(/</g, '\\u003c')};
       const state = { data: null, answers: {}, activeQuestionIndex: 0, lastSig: null, customMode: {} };
 
       function updateSubmitButton() {
@@ -666,12 +666,17 @@ export function renderInterviewPage(interviewId: string): string {
         }
       });
 
+      function schedulePoll() {
+        setTimeout(async () => {
+          try { await refresh(); } catch (_) {}
+          if (state.data?.mode !== 'abandoned') schedulePoll();
+        }, 2500);
+      }
+
       refresh().catch((error) => {
         document.getElementById('submitStatus').textContent = error.message || 'Failed to load interview.';
       });
-      setInterval(() => {
-        refresh().catch(() => {});
-      }, 2500);
+      schedulePoll();
     </script>
   </body>
 </html>`;