Hardening sprint: tests + security + concurrency (v0.3.1)

.github/workflows/ci.yml

@@ -0,0 +1,38 @@
+name: CI — typecheck + tests
+
+# Gates every push and PR with the regression net. Tests use Node's built-in
+# runner (node --test) via the tsx loader — no extra test deps to install.
+
+on:
+  push:
+    branches: ["main"]
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: "22"
+          cache: "npm"
+
+      - name: Install
+        run: npm ci
+
+      - name: Typecheck
+        run: npm run typecheck
+
+      - name: Test
+        run: npm test
+
+      - name: Build (verify dist compiles, no test files emitted)
+        run: |
+          npm run build
+          if find dist -name '*.test.js' | grep -q .; then
+            echo "::error ::test files leaked into dist/"; exit 1
+          fi

CHANGELOG.md

@@ -5,6 +5,35 @@ versioning follows [SemVer](https://semver.org/).
 
 ## [Unreleased]
 
+### Security & hardening (v0.3.1 sprint)
+
+Following a full product/code review (`docs/PRODUCT_REVIEW_v0.3.md`), the
+engineering-hardness gaps were addressed — the project went from **zero
+automated tests** to a 114-test regression net (Node's built-in `node:test`
+via tsx, no new dependencies) plus a CI gate.
+
+- **Tests + CI** — `npm test` runs `src/**/*.test.ts`; new `.github/workflows/ci.yml`
+  gates every push/PR on typecheck + tests + build. Test files excluded from `dist/`.
+- **SSRF redirect bypass closed** (`web_fetch`) — the private-IP check ran only on
+  the initial URL; a public URL could 302 → `127.0.0.1` / the cloud metadata IP and
+  be followed. Redirects are now followed manually with every hop re-validated.
+- **AppleScript injection closed** (`iMessage`) — outbound text was interpolated into
+  the AppleScript source with only quote-escaping; a newline or crafted payload could
+  inject script. Text now passes as positional `argv`, never parsed as source.
+- **Path traversal blocked** (soul slugs) — value/opinion/desire/journal/relationship
+  slugs are validated at the single path chokepoint; `../`, separators, control chars,
+  and leading dots are rejected.
+- **Cross-process soul lock** — desire-progress appends (read-modify-write) now run
+  under an advisory file lock, so a heartbeat/idle run can't interleave with a chat
+  turn and lose data.
+- **Heartbeat token budget + run-lock** — per-run token ceiling (`budgetTokens`,
+  default 500k) stops runaway autonomous cost; a run-lock skips overlapping heartbeat
+  ticks instead of double-running.
+- **Continuous emotion decay** — decay now applies on write (soul_feel) and in
+  soul_read, not just in the system-prompt view, and no longer drops the event trail.
+- **Memory index cache** — the TF-IDF index is cached and rebuilt only when the
+  sessions dir changes, instead of on every `memory_search`.
+
 ### Added
 
 - **7 new LLM provider presets** — Lisa now auto-routes 7 additional providers by model-name prefix, no `LISA_BASE_URL` plumbing needed:

docs/PRODUCT_REVIEW_v0.3.md

@@ -0,0 +1,155 @@
+# LISA — Product Capability Review & Tuning Plan (v0.3)
+
+> Full review of ~15k LOC TypeScript + ~800 LOC Swift + all product docs.
+> Method: five deep-探查 agents (one per subsystem — soul, agent engine,
+> autonomy, surfaces, product positioning), cross-verified, plus direct
+> inspection of product docs + test setup. Conducted on the v0.3.0 tree.
+
+---
+
+## 1. Overall verdict
+
+LISA is a **real, architecturally-considered, well-past-demo** open-source AI
+agent whose core differentiator ("an individual with a self") is **backed by
+code, not just marketing**. Maturity by axis:
+
+> **Capability parity = 9/10 · Differentiator skeleton (soul/autonomy) = 7/10 ·
+> Engineering hardness (tests/concurrency/security) = 3/10 · Surface polish = 8/10**
+
+One line: **product imagination A, single-user/trusted-env usability A−,
+engineering resilience D**. The biggest systemic risk is not features — it's
+**zero automated tests + no concurrency protection**. For an agent that markets
+"self-modification", those two are existential.
+
+---
+
+## 2. Capability matrix (real completeness)
+
+| Subsystem | What it does | Maturity | Reality |
+|---|---|---|---|
+| Agent loop | streaming loop, tool-calling, mid-session prompt hot-reload, soul_object forced-surface | ★★★★★ | production-grade, no placeholders |
+| Multi-provider | 3 native protocols + 21 OpenAI-compat presets + catch-all, routed by model-name prefix | ★★★★★ | elegant routing, case-insensitive |
+| Tools | 23 built-ins (file/web/search/memory/soul/ops) + skills + MCP | ★★★★ | broad coverage |
+| Soul | birth ritual, identity/purpose/constitution, values/opinions/desires/emotions, git history, tamper detection, hot-reload | ★★★★ | genuinely implemented; emotion event-trail is the standout |
+| Heartbeat | launchd/cron, runs actionable desires, progress carries across runs, weekly examen | ★★★★ | real but reactive |
+| Idle/Dreams | idle 1h+ → single reflection → ★while-you-were-away | ★★★ | pragmatic, not narrative "dreams" |
+| Memory | TF-IDF over session history | ★★ | works but rebuilds index every search |
+| Skills | SHA256 + human approval, then dynamic import | ★★★ | intentionally un-sandboxed (documented) |
+| Web GUI | glass-morphism chat, mood portraits, soul/skills panels, PWA, birth ritual | ★★★★★ | strongest surface |
+| Island | pill + expand + Claude monitor + native drag | ★★★★ | clever product design |
+| Mac apps | Lisa.app + LisaIsland.app, signed+notarized DMG | ★★★★ | freshly shipped |
+| IM channels | Telegram/Discord/Slack/Feishu/iMessage/Webhook | ★★★ | breadth-first, edge cases |
+| Voice | macOS `say` + OpenAI Whisper | ★★ | thinnest |
+| Claude Code monitor | privacy-first metadata watch, state derivation | ★★★★ | thoughtful |
+| Tests | — | 0/10 | **zero automated tests** |
+
+---
+
+## 3. The real moat (the differentiation is real)
+
+1. **Soul hot-reload + git history** — `soul_patch` takes effect on turn N+1
+   of the *same* conversation (fingerprint mechanism); every change commits to
+   `~/.lisa/soul/.git` with caller attribution. "She can look at who she was 3
+   months ago" actually runs. Rare in the LLM-agent space.
+2. **Emotion event causal trail** — emotions are `{emotion, delta, trigger, ts}`
+   event streams + exponential decay (per-emotion half-lives), not bare numbers.
+3. **Constraint-driven autonomy** — every autonomy increment is paired with a
+   stability hedge (roadmap §0). Weekly examen can *suggest* corrective desires
+   but cannot rewrite identity/purpose; skills require human approval; soul_object
+   objections must surface. Deliberate friction = mature design judgment.
+
+---
+
+## 4. Marketing vs reality (honest reconciliation)
+
+| PITCH claims | Reality | Gap |
+|---|---|---|
+| "she has motivation/desires" | desires drive heartbeat ✓ | but desires are **not self-generated** — user or reflect must create them. No "I notice I want X → add a desire" loop |
+| "architectural sovereignty, no reset" | mutation sovereignty real ✓ | but external edits can only be *noticed*, not *prevented*; "forget who you are" has no technical enforcement, relies on LLM compliance |
+| "she's evolving" | git/examen/desire tracking real ✓ | evolution is largely **reactive**; self-improvement loop (spot gap → add desire → pursue → refine) is **absent** |
+| "~11k LOC TypeScript" | actual src ~15k LOC | undercount |
+| "capability superset of 5 agents" | breadth is there | depth gaps: MCP tools-only (no resources/prompts), sandbox macOS-only, approval sync-readline-only |
+
+**Conclusion**: marketing isn't lying, but the "inner life / motivation" story is
+**~70% delivered** — the skeleton is real, but it's still mostly passive
+execution, not active emergence.
+
+---
+
+## 5. Critical issues (must-fix, by severity)
+
+### P0 — systemic risk
+1. **Zero automated tests** — no `test` script, 0 `.test.ts`. A self-modifying
+   agent with no regression net on birth/store/reflect/parser.
+   → introduce vitest; cover soul CRUD, emotion decay, reflect JSON parse,
+   claude-code parser, provider routing. CI gate.
+2. **No concurrency protection** (named by both soul + autonomy reviews) —
+   heartbeat 35min run vs 30min interval → two instances writing
+   `~/.lisa/soul/` concurrently; `appendDesireProgress()` uses bare
+   `fs.appendFile`; git `index.lock` races → data corruption.
+   → `flock` on `~/.lisa/soul.lock` before any soul write; mutex on
+   heartbeat/idle runner entry.
+
+### P1 — security & correctness
+3. **SSRF redirect bypass** (web_fetch) — entry validates private IPs but
+   `redirect: "follow"` lets a public domain 301 → `127.0.0.1:8000`.
+   → validate redirect targets before following.
+4. **iMessage osascript escaping** — only escapes `"`, not newlines →
+   AppleScript-syntax injection. → base64 / arg-array.
+5. **Tool input has no schema validation** — LLM-generated input goes straight
+   into `tool.execute()`. → validate at dispatch.
+6. **Webhook: no rate limit + 5min timeout** — token-holder can spam; caller
+   waits 5min if Lisa hangs. → per-sender limit + 30s timeout.
+
+### P2 — cost & performance
+7. **Autonomous loop has no token budget** — `every:5m` + 3 actionable desires
+   × 32 iters ≈ ~173M tokens/day ≈ $5/day, no budget/breaker.
+   → heartbeat `budget_tokens`, skip/abort over limit.
+8. **Memory rebuilds index every search** — `buildIndex()` O(N sessions) every
+   `memory_search`. → cache per session lifetime.
+9. **Emotion decay discontinuous** — only decays on `readSoulSummary`. → decay
+   on write too.
+
+---
+
+## 6. Tuning recommendations (by theme)
+
+### A. Engineering hardness (highest priority — the real weak spot)
+- vitest + CI running tests (release workflow currently build-only, no test)
+- file locks across soul/heartbeat
+- slug validation (value/opinion/desire slugs currently allow `../` traversal)
+- reflect idempotency marker (running twice double-applies operations)
+
+### B. Make autonomy active, not reactive (the next step for the moat)
+- desire self-generation loop: examen can suggest desires, but there's no
+  "heartbeat analyzes its own output → refines next prompt" learning loop.
+  This is the key to pushing 70% → 90% delivered.
+- desire prioritization: all actionable desires run every heartbeat, no
+  ranking/time-budget.
+- `opinion_update(slug, delta, evidence)` so confidence updates don't require
+  rewriting the whole opinion.
+
+### C. Performance & scale (will bite in months)
+- swap/augment memory with semantic retrieval (small embeddings, TF-IDF fallback)
+- system prompt injects ALL values/opinions/desires in full — after a year of
+  100+ entries it bloats. → top-N + reference `soul_read`.
+- turn-level cache for `readSoulSummary` (currently 7 I/O + 3 dir scans per turn)
+
+### D. Surface polish (nice-to-have)
+- transcript export / chat search
+- voice streaming + local Whisper fallback
+- per-channel setup runbooks (esp. iMessage Full Disk Access)
+- MCP resources/prompts support
+
+---
+
+## 7. Suggested next sprint (if only 3 things)
+
+1. **vitest + soul/parser/provider core tests + CI gate** — removes the "no net
+   under soul changes" existential anxiety.
+2. **soul/heartbeat file locks + token budget** — plugs the two P0/P1 holes
+   (data corruption + runaway cost).
+3. **desire self-generation + learning loop** — pushes "she has motivation" from
+   70% → 90%. The product story's true last mile.
+
+First two are "stop the foundation cracking"; the third is "deepen the moat".

package.json

@@ -55,6 +55,8 @@
     "start": "node dist/cli.js",
     "lisa": "node --enable-source-maps dist/cli.js",
     "typecheck": "tsc -p tsconfig.json --noEmit",
+    "test": "node --import tsx --test \"src/**/*.test.ts\"",
+    "test:watch": "node --import tsx --test --watch \"src/**/*.test.ts\"",
     "generate-assets": "tsx scripts/generate-pixel-assets.ts",
     "prepublishOnly": "npm run build && rm -rf dist/web/assets && cp -R src/web/assets dist/web/assets",
     "postpublish": "npm run copy-assets"

src/channels/imessage.test.ts

@@ -0,0 +1,45 @@
+import { test, describe } from "node:test";
+import assert from "node:assert/strict";
+import { buildOsascriptArgs } from "./imessage.js";
+
+describe("buildOsascriptArgs — injection-proof argv passing", () => {
+  test("recipient + text are passed as positional args, not interpolated", () => {
+    const args = buildOsascriptArgs("+15551234567", "hello");
+    // Shape: ["-e", <script>, recipient, text]
+    assert.equal(args[0], "-e");
+    assert.equal(args[2], "+15551234567");
+    assert.equal(args[3], "hello");
+  });
+
+  test("the AppleScript source is STATIC — no user text embedded in it", () => {
+    const payload = 'evil" & (do shell script "rm -rf ~") & "';
+    const args = buildOsascriptArgs("buddy", payload);
+    const script = args[1]!;
+    // The dangerous payload must NOT appear anywhere in the script source.
+    assert.equal(script.includes("evil"), false);
+    assert.equal(script.includes("do shell script"), false);
+    assert.equal(script.includes("rm -rf"), false);
+    // It must instead arrive as the positional text arg, verbatim.
+    assert.equal(args[3], payload);
+  });
+
+  test("newlines survive verbatim in the text arg (the original bug)", () => {
+    const multiline = "line one\nline two\nline three";
+    const args = buildOsascriptArgs("buddy", multiline);
+    assert.equal(args[3], multiline, "newlines must be preserved, not escaped/broken");
+    assert.equal(args[1]!.includes("line one"), false, "text not in source");
+  });
+
+  test("quotes and backslashes pass through untouched", () => {
+    const tricky = 'she said "hi" \\ then left';
+    const args = buildOsascriptArgs("buddy", tricky);
+    assert.equal(args[3], tricky);
+  });
+
+  test("script references argv positionally", () => {
+    const script = buildOsascriptArgs("a", "b")[1]!;
+    assert.match(script, /on run argv/);
+    assert.match(script, /item 1 of argv/);
+    assert.match(script, /item 2 of argv/);
+  });
+});

src/channels/imessage.ts

@@ -60,15 +60,9 @@ export class IMessageChannel implements ChannelAdapter {
   }
 
   async send(msg: OutgoingMessage): Promise<void> {
-    const recipient = msg.to.replace(/"/g, '\\"');
-    const text = msg.text.replace(/"/g, '\\"');
-    const script = `tell application "Messages"
-  set targetService to first service whose service type = iMessage
-  set targetBuddy to buddy "${recipient}" of targetService
-  send "${text}" to targetBuddy
-end tell`;
+    const args = buildOsascriptArgs(msg.to, msg.text);
     await new Promise<void>((resolve, reject) => {
-      const child = spawn("/usr/bin/osascript", ["-e", script]);
+      const child = spawn("/usr/bin/osascript", args);
       let stderr = "";
       child.stderr.on("data", (b) => (stderr += b.toString("utf8")));
       child.on("error", reject);
@@ -140,6 +134,34 @@ end tell`;
   }
 }
 
+/**
+ * Build the osascript argv for sending an iMessage WITHOUT interpolating
+ * user/LLM text into the AppleScript source.
+ *
+ * The old approach (`send "${text}"`) only escaped double-quotes, so a
+ * message containing a newline — or a crafted `" & (do shell script "...")`
+ * payload — could break out of the string literal and inject AppleScript.
+ * Inbound iMessage text is untrusted (anyone who can text the user), so this
+ * was a real injection vector.
+ *
+ * Instead the script is a STATIC `on run argv` program; recipient and text
+ * arrive as positional argv items (`item 1`/`item 2 of argv`) that AppleScript
+ * never parses as source. No escaping needed; newlines/quotes/backslashes
+ * survive verbatim.
+ */
+export function buildOsascriptArgs(recipient: string, text: string): string[] {
+  const script = `on run argv
+  set theRecipient to item 1 of argv
+  set theText to item 2 of argv
+  tell application "Messages"
+    set targetService to first service whose service type = iMessage
+    set targetBuddy to buddy theRecipient of targetService
+    send theText to targetBuddy
+  end tell
+end run`;
+  return ["-e", script, recipient, text];
+}
+
 registerChannel("imessage", (cfg) => {
   const intervalMs = typeof cfg.intervalMs === "number" ? cfg.intervalMs : undefined;
   return new IMessageChannel(intervalMs ? { intervalMs } : {});

src/heartbeat/config.ts

@@ -13,20 +13,36 @@ export interface HeartbeatTask {
 
 export interface HeartbeatConfig {
   tasks: HeartbeatTask[];
+  /**
+   * Max combined (input+output) tokens to spend across all tasks in a single
+   * heartbeat invocation. Once exceeded, remaining tasks are skipped (logged,
+   * not silently dropped). Guards against the runaway-cost case: a short
+   * interval × several actionable desires × deep tool loops can otherwise run
+   * to millions of tokens/day unbounded. 0 / unset = no limit.
+   */
+  budgetTokens?: number;
 }
 
+/** Default per-run token ceiling when heartbeat.json doesn't set one. */
+export const DEFAULT_HEARTBEAT_BUDGET_TOKENS = 500_000;
+
 const FILE = path.join(LISA_HOME, "heartbeat.json");
 
 export async function loadHeartbeatConfig(): Promise<HeartbeatConfig> {
-  if (!(await pathExists(FILE))) return { tasks: [] };
+  if (!(await pathExists(FILE))) return { tasks: [], budgetTokens: DEFAULT_HEARTBEAT_BUDGET_TOKENS };
   const raw = await fs.readFile(FILE, "utf8");
   let parsed: HeartbeatConfig;
   try {
     parsed = JSON.parse(raw) as HeartbeatConfig;
   } catch (err) {
     throw new Error(`failed to parse ${FILE}: ${(err as Error).message}`);
   }
-  return { tasks: parsed.tasks ?? [] };
+  return {
+    tasks: parsed.tasks ?? [],
+    // Explicit 0 means "no limit"; undefined means "use the default ceiling".
+    budgetTokens:
+      parsed.budgetTokens === undefined ? DEFAULT_HEARTBEAT_BUDGET_TOKENS : parsed.budgetTokens,
+  };
 }
 
 export const HEARTBEAT_CONFIG_PATH = FILE;