fix: audit batch — #133 #129

CONTRIBUTING.md

@@ -0,0 +1,67 @@
+# Contributing
+
+## Dev Setup
+
+Requires [Bun](https://bun.sh/) (v1.2+) and a [Rust toolchain](https://rustup.rs/) for the broker.
+
+```bash
+git clone https://github.com/almogdepaz/wolfpack.git
+cd wolfpack
+bun install
+bun run scripts/gen-assets.ts                            # generate embedded assets (required once)
+cargo build --release --manifest-path broker/Cargo.toml  # build the broker
+bun run src/cli/index.ts                                 # start the server locally
+```
+
+For an end-to-end local install (build + service install + restart), use `scripts/deploy-local.sh`.
+
+## Testing
+
+```bash
+bun test                                  # all bun tests
+bun test tests/unit/                      # unit tests only
+bun test tests/unit/plan-parsing.test.ts  # single file
+bunx playwright test                      # e2e (uses test-server harness)
+```
+
+Layout:
+
+- `tests/unit/` — pure-logic tests (plan parsing, ralph log parsing, escaping, validation, grid logic, broker codec, etc.)
+- `tests/integration/` — API routes, broker backend, ralph loop endpoints, WS dispatch
+- `tests/snapshot/` — launchd plist and systemd unit generation
+- `tests/e2e/` — Playwright end-to-end (`test:e2e` / `test:e2e:headed`)
+
+The Rust broker has its own tests under `broker/tests/` — run with `cargo test` from `broker/`.
+
+## Asset Pipeline
+
+Frontend files live in `public/`. The server doesn't serve from disk — everything is embedded into the binary:
+
+1. Edit files in `public/` (HTML, TS, CSS, manifest, etc.)
+2. Run `bun run scripts/gen-assets.ts` — bundles `public/app.ts` and ghostty-web, then embeds every file from `public/` into `src/public-assets.ts` (binary → base64, text → string)
+3. **Do NOT edit `src/public-assets.ts` manually** — it's auto-generated
+
+## Building Release Binaries
+
+```bash
+bun run scripts/build.ts
+```
+
+Produces `wolfpack` for linux-x64, linux-arm64, darwin-x64, darwin-arm64 plus per-platform npm package directories in `dist/`. Also stages `wolfpack-broker` per platform — in CI it expects pre-built broker binaries under `dist/broker/<target>/`; locally it falls back to a host-arch-only `cargo build --release`.
+
+## PR Conventions
+
+- Branch off `main`
+- Tests must pass (`bun test`)
+- Keep PRs focused — one feature or fix per PR
+- Match existing style; no large unrelated refactors mixed in
+
+## Migrating Old Plan Files
+
+If you have a Ralph plan file from before the `## N. Title` header convention:
+
+```bash
+wolfpack migrate-plan PLAN.md
+```
+
+This rewrites the file in place.

src/server/routes.ts

@@ -8,8 +8,6 @@ import {
   readdirSync,
   mkdirSync,
   statSync,
-  lstatSync,
-  realpathSync,
   existsSync,
   unlinkSync,
   openSync,
@@ -39,7 +37,8 @@ import { getVapidPublicKey, addSubscription, removeSubscription, sendPush, valid
 import pkg from "../../package.json";
 
 const log = createLogger("routes");
-import { DEV_DIR, isUnderDevDir } from "./dev-dir.js";
+import { DEV_DIR } from "./dev-dir.js";
+import { validateProjectDir as validateProjectDirPure } from "./validate-project-dir.js";
 import { RALPH_AGENTS } from "./shell.js";
 import { getBackend, getRouter } from "./backend.js";
 import {
@@ -128,23 +127,14 @@ function validateProject(res: ServerResponse, project: string | null | undefined
   return true;
 }
 
-/** Validate project directory exists, is not a symlink, and resolves under DEV_DIR. */
+/** Validate project directory exists, is not a symlink, and resolves under DEV_DIR.
+ *  Thin HTTP wrapper around the pure `validateProjectDirPure` so the security-sensitive
+ *  containment logic lives in one tested place. */
 function validateProjectDir(res: ServerResponse, projectDir: string): boolean {
-  try {
-    if (lstatSync(projectDir).isSymbolicLink() || !statSync(projectDir).isDirectory()) {
-      json(res, { error: "not a directory" }, 400);
-      return false;
-    }
-    // defense-in-depth: verify realpath is contained under DEV_DIR
-    if (!isUnderDevDir(realpathSync(projectDir))) {
-      json(res, { error: "not a directory" }, 400);
-      return false;
-    }
-  } catch { /* expected: stat fails when project dir doesn't exist */
-    json(res, { error: "project directory not found" }, 404);
-    return false;
-  }
-  return true;
+  const result = validateProjectDirPure(projectDir);
+  if (result.ok) return true;
+  json(res, { error: result.error }, result.code === "not_found" ? 404 : 400);
+  return false;
 }
 
 import {

src/server/validate-project-dir.ts

@@ -0,0 +1,37 @@
+/**
+ * Project-directory validation — pure module (no HTTP side-effects).
+ *
+ * Extracted from routes.ts so regression tests can exercise the real
+ * implementation instead of a copy. The routes.ts call sites wrap this
+ * with response-emitting helpers.
+ */
+import { lstatSync, statSync, realpathSync } from "node:fs";
+import { isUnderDevDir } from "./dev-dir.js";
+
+export type ValidateProjectDirResult =
+  | { ok: true; projectDir: string }
+  | { ok: false; code: "not_dir" | "not_found"; error: string };
+
+/**
+ * Validate that `projectDir`:
+ *   1. exists,
+ *   2. is not a symlink (lstat),
+ *   3. is a directory,
+ *   4. and its realpath resolves under DEV_DIR (defense-in-depth).
+ *
+ * Returns a discriminated result rather than mutating an HTTP response.
+ * Callers translate `code` to a status code (`not_dir` → 400, `not_found` → 404).
+ */
+export function validateProjectDir(projectDir: string): ValidateProjectDirResult {
+  try {
+    if (lstatSync(projectDir).isSymbolicLink() || !statSync(projectDir).isDirectory()) {
+      return { ok: false, code: "not_dir", error: "not a directory" };
+    }
+    if (!isUnderDevDir(realpathSync(projectDir))) {
+      return { ok: false, code: "not_dir", error: "not a directory" };
+    }
+  } catch {
+    return { ok: false, code: "not_found", error: "project directory not found" };
+  }
+  return { ok: true, projectDir };
+}

src/server/websocket.ts

@@ -88,6 +88,49 @@ const QUIESCE_WINDOW_MS = 100;
 const QUIESCE_BYTE_THRESHOLD = 1024;
 const QUIESCE_TIMEOUT_MS = 800;
 const QUIESCE_MIN_WAIT_MS = 80;
+// Hard cap for sessions that never quiesce (animated TUIs: spinners,
+// progress bars, htop, watch). Without this, an always-busy session waits
+// the full QUIESCE_TIMEOUT_MS (800ms) before snapshotting, producing a
+// visibly garbled mid-redraw prefill that the live stream then has to
+// overwrite. Capping at 200ms cuts the worst-case prefill-garble window
+// by 4x for animated sessions while leaving the common quiet-path
+// behavior unchanged: real apps quiet inside QUIESCE_WINDOW_MS well
+// before this cap is reached. Issue #129.
+const QUIESCE_ANIMATED_CAP_MS = 200;
+
+/**
+ * Pure decision for the quiescence loop. Returns one of:
+ *  - "continue" — keep observing
+ *  - "quiet"     — recent byte rate below threshold, safe to snapshot
+ *  - "animated_cap" — byte rate stayed high through the animated-cap
+ *    window; we snapshot a (potentially mid-redraw) frame rather than
+ *    wait the full timeout
+ *  - "timeout"  — absolute settle timeout reached
+ *
+ * Exported for unit tests. The loop's setTimeout(16) wait and the resize
+ * side-effects live at the call site.
+ */
+export function quiescenceDecision(args: {
+  samples: Array<{ t: number; bytes: number }>;
+  now: number;
+  lastResizeAt: number;
+  settleStart: number;
+}): "continue" | "quiet" | "animated_cap" | "timeout" {
+  const { samples, now, lastResizeAt, settleStart } = args;
+  const elapsedTotal = now - settleStart;
+  if (elapsedTotal >= QUIESCE_TIMEOUT_MS) return "timeout";
+  const elapsedSinceResize = now - lastResizeAt;
+  if (elapsedSinceResize < QUIESCE_MIN_WAIT_MS) return "continue";
+  const cutoff = now - QUIESCE_WINDOW_MS;
+  let recentBytes = 0;
+  for (const s of samples) if (s.t >= cutoff) recentBytes += s.bytes;
+  if (recentBytes < QUIESCE_BYTE_THRESHOLD) return "quiet";
+  // Byte rate stayed high through the entire animated-cap window measured
+  // from settle start. This is the always-busy TUI signature — don't wait
+  // out the full 800ms timeout.
+  if (elapsedTotal >= QUIESCE_ANIMATED_CAP_MS) return "animated_cap";
+  return "continue";
+}
 // Adaptive coalescing of broker output frames before forwarding to viewer.
 // See call site for full reasoning.
 const COALESCE_FLUSH_MS = 16;
@@ -507,14 +550,17 @@ function setupNewPtyEntry(
               await backend.resize(session, appliedSize.cols, appliedSize.rows);
               lastResizeAt = Date.now();
             }
-            const elapsedTotal = Date.now() - settleStart;
-            if (elapsedTotal >= QUIESCE_TIMEOUT_MS) break;
-            const elapsedSinceResize = Date.now() - lastResizeAt;
-            if (elapsedSinceResize >= QUIESCE_MIN_WAIT_MS) {
-              const cutoff = Date.now() - QUIESCE_WINDOW_MS;
-              while (samples.length > 0 && samples[0].t < cutoff) samples.shift();
-              const recentBytes = samples.reduce((s, x) => s + x.bytes, 0);
-              if (recentBytes < QUIESCE_BYTE_THRESHOLD) break;
+            const now = Date.now();
+            // Trim samples outside the rolling window so the array stays
+            // bounded across long never-quiet sessions.
+            const cutoff = now - QUIESCE_WINDOW_MS;
+            while (samples.length > 0 && samples[0].t < cutoff) samples.shift();
+            const decision = quiescenceDecision({ samples, now, lastResizeAt, settleStart });
+            if (decision !== "continue") {
+              if (decision === "animated_cap" || decision === "timeout") {
+                log.debug("quiescence loop exited without quiet", { session, reason: decision, elapsedMs: now - settleStart });
+              }
+              break;
             }
             await new Promise(resolve => setTimeout(resolve, 16));
           }

tests/unit/audit-regressions.test.ts

@@ -39,56 +39,47 @@ describe("isUnderDevDir — path containment boundary", () => {
 });
 
 // ── 1b. validateProjectDir realpath containment ──
+// Imports the real `validateProjectDir` from src/ so this test cannot drift
+// from production. `isUnderDevDir` resolves DEV_DIR from process.env.WOLFPACK_DEV_DIR
+// at call time (set at top of file to /Users/home/Dev/).
 
-import { mkdtempSync, mkdirSync, symlinkSync, rmSync, realpathSync, lstatSync, statSync } from "node:fs";
+import { mkdtempSync, mkdirSync, symlinkSync, rmSync, realpathSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
-
-/**
- * Mirrors the core logic of routes.ts validateProjectDir():
- * rejects symlinks, rejects dirs whose realpath escapes DEV_DIR.
- */
-function validateProjectDir(projectDir: string, devDir: string): "ok" | "not_dir" | "not_found" {
-  try {
-    if (lstatSync(projectDir).isSymbolicLink() || !statSync(projectDir).isDirectory()) return "not_dir";
-    if (!isUnderDevDir(realpathSync(projectDir))) return "not_dir";
-  } catch {
-    return "not_found";
-  }
-  return "ok";
-}
+const { validateProjectDir } = await import("../../src/server/validate-project-dir.js");
 
 describe("validateProjectDir — realpath containment", () => {
-  let testDevDir: string;
-  let outsideDir: string;
-
-  test("accepts real directory under DEV_DIR", () => {
-    testDevDir = mkdtempSync(join(tmpdir(), "wolfpack-devdir-"));
+  test("rejects directory whose realpath escapes DEV_DIR", () => {
+    const testDevDir = mkdtempSync(join(tmpdir(), "wolfpack-devdir-"));
     const project = join(testDevDir, "legit-project");
     mkdirSync(project);
-    // isUnderDevDir checks against process.env.WOLFPACK_DEV_DIR which is /Users/home/Dev/
-    // so we test the realpathSync + isUnderDevDir logic directly
+    // testDevDir is under /tmp (or macOS equivalent), not under DEV_DIR (/Users/home/Dev/),
+    // so containment must fail.
     const real = realpathSync(project);
-    // the project's realpath is under testDevDir (not under DEV_DIR), so isUnderDevDir returns false
-    // This verifies the containment check works
     expect(isUnderDevDir(real)).toBe(false);
-    expect(validateProjectDir(project, testDevDir)).toBe("not_dir");
+    const result = validateProjectDir(project);
+    expect(result.ok).toBe(false);
+    if (!result.ok) expect(result.code).toBe("not_dir");
     rmSync(testDevDir, { recursive: true, force: true });
   });
 
-  test("rejects symlink pointing outside DEV_DIR", () => {
-    testDevDir = mkdtempSync(join(tmpdir(), "wolfpack-devdir-"));
-    outsideDir = mkdtempSync(join(tmpdir(), "wolfpack-outside-"));
+  test("rejects symlink even when target would otherwise be valid", () => {
+    const testDevDir = mkdtempSync(join(tmpdir(), "wolfpack-devdir-"));
+    const outsideDir = mkdtempSync(join(tmpdir(), "wolfpack-outside-"));
     const symlink = join(testDevDir, "sneaky-link");
     symlinkSync(outsideDir, symlink);
-    // lstatSync catches the symlink before realpath even runs
-    expect(validateProjectDir(symlink, testDevDir)).toBe("not_dir");
+    // lstat catches the symlink before realpath even runs.
+    const result = validateProjectDir(symlink);
+    expect(result.ok).toBe(false);
+    if (!result.ok) expect(result.code).toBe("not_dir");
     rmSync(testDevDir, { recursive: true, force: true });
     rmSync(outsideDir, { recursive: true, force: true });
   });
 
   test("rejects nonexistent directory", () => {
-    expect(validateProjectDir("/nonexistent/path/xyz", "/tmp")).toBe("not_found");
+    const result = validateProjectDir("/nonexistent/path/xyz");
+    expect(result.ok).toBe(false);
+    if (!result.ok) expect(result.code).toBe("not_found");
   });
 });