Hardening sprint: tests + security + concurrency (v0.3.1) - #31
Merged
Conversation
Full review across soul / agent-engine / autonomy / surfaces / product positioning. Verdict: capability parity 9/10, differentiator skeleton 7/10, engineering hardness 3/10 (zero tests + no concurrency control is the systemic risk), surface polish 8/10. Tuning plan prioritized P0→P2; this doc is the source of truth for the hardening sprint that follows.
Uses Node's built-in test runner via the existing tsx loader — no vitest, no new dependencies, consistent with the project's 5-runtime/4-dev-dep ethos. `npm test` globs src/**/*.test.ts. First regression net: 24 tests over detectProvider — native prefixes, case-insensitivity (the Baichuan/MiniMax bug that motivated the case-insensitive fix), all 21 presets routing through openai, env overrides, and table integrity (no ambiguous prefixes, well-formed baseURL/apiKeyEnv).
Exercises parseSessionState against real temp jsonl fixtures: state derivation (end_turn→waiting, tool_use→working, user→working, is_error →error, hookErrors→error, permission subtype→waiting/permission), META_TYPES skipping (the pr-link-masks-state bug), and robustness (empty/missing/malformed files, cwd sniffing, only-meta).
Soul entries are one-file-per-slug under ~/.lisa/soul/{values,opinions,
desires,relationships}/<slug>.md, and the slug is untrusted (LLM- or
reflect-supplied). Nothing validated it, so a slug like "../../../etc/x"
or "a/b" could let a write escape the soul directory or nest into
subdirs.
New src/soul/slug.ts:
- assertSafeSlug(): hard gate — throws on path separators, "."/".."/
leading-dot, control chars (codepoint-checked so source stays ASCII),
empty, or >64 chars. Deliberately case/style-permissive so it never
breaks reads of slugs written by older versions.
- normalizeSlug(): soft cleaner for minting NEW slugs from free text.
Wired assertSafeSlug into every slug-bearing helper in paths.ts (value/
opinion/desire/desireProgress/journal/relationship) — single chokepoint,
no caller can bypass. 26 new tests incl. the canonical ../../../ exploit
and the invariant "normalizeSlug output always passes assertSafeSlug".
Two bugs in the emotion model:
1. Decay only ran on read (readSoulSummary), never at write. soul_feel
added its delta onto the STALE stored value, so after a week-long gap
curiosity 0.6 stayed 0.6 instead of decaying to ~0.42 first — stored
intensity drifted months out of date and jumped discontinuously.
Fix: decayEmotions() now runs at the start of soul_feel (decay-then-
add) and in soul_read("emotions") for display, matching the
system-prompt view. All three surfaces now agree.
2. decayEmotions() silently DROPPED the events array on every call —
decaying emotions erased the causal trail that makes them meaningful.
Fix: events (and decay rates) are now preserved.
decayEmotions is now synchronous and takes an optional nowMs for
deterministic testing. 7 tests: decay math, half-life, rate fallback,
negative intensities, events/rates preservation, and the
decay-then-add continuity invariant.
The private-host check ran only on the initial URL; redirect:"follow" then let a public URL 302 → http://127.0.0.1:8000 (or the cloud metadata IP 169.254.169.254) and the fetch reached the internal service. Now redirects are followed MANUALLY: assertAllowedUrl() re-runs the protocol + private-IP check on the initial URL AND every Location before following it, capped at 5 hops. isPrivateHost + assertAllowedUrl + fetchFollowingSafeRedirects are exported and covered by 26 tests, including the exploit (public→127.0.0.1 and public→metadata-IP both refused), normal pass-through, public redirect chains, and loop cap.
…urce (P1)
The old send() did `send "${text}"` with only double-quotes escaped, so a
message containing a newline broke the AppleScript string literal, and a
crafted payload like `" & (do shell script "...") & "` could inject
arbitrary AppleScript. Inbound iMessage text is untrusted (anyone who can
text the user), so this was a real injection vector.
buildOsascriptArgs() now emits a STATIC `on run argv` program and passes
recipient + text as positional argv items — AppleScript never parses them
as source, so no escaping is needed and newlines/quotes/backslashes pass
through verbatim. 5 tests assert the payload never appears in the script
source and survives intact as the text arg.
The in-process commit queue serializes writes within ONE process, but LISA runs as several against the same ~/.lisa/soul/: web server, CLI, and the launchd/cron heartbeat + idle runners. Concurrent appends to a desire-progress file did read-modify-write with no lock → last writer wins, losing the other's append; git commits could also collide on .git/index.lock. New src/soul/lock.ts: withFileLock() — a portable advisory mutex via exclusive file creation (open "wx"), self-healing from a crashed holder via a staleness timeout + best-effort pid liveness check. withSoulLock() wraps it at the canonical ~/.lisa/soul/.write.lock. appendDesireProgress() now does its read-modify-write + commit inside withSoulLock. 8 tests: value passthrough, release-on-success, release- on-throw, mutual-exclusion (no interleave), timeout, stale-steal, malformed-lock recovery.
Two runaway-autonomy guards: 1. Token budget — heartbeat.json gains `budgetTokens` (default 500k). The runner accumulates input+output tokens across tasks and stops launching new ones once the ceiling is crossed (logged, not silently dropped). Without this, `every:5m` × several actionable desires × deep tool loops could run to millions of tokens/day unbounded. Explicit 0 = no limit. 2. Run-lock — runHeartbeatOnce now wraps the run in withFileLock on ~/.lisa/heartbeat.lock with timeoutMs:0 (fail-fast). If launchd/cron fires a new tick while the previous heartbeat is still running, it skips rather than double-running and racing on soul state. 6h stale timeout reclaims a crashed run's lock. Also: tsconfig now excludes src/**/*.test.ts from the build so test files don't ship in dist/ (tests still run from src/ via tsx).
…ge (P2)
buildIndex() re-reads + re-tokenizes every past session — O(total
transcript bytes). memory_search called it fresh on every invocation, so
a chat searching its memory N times in one session paid that cost N
times even with nothing changed.
Now the built index is cached, keyed by a cheap fingerprint of the
sessions dir (each .jsonl's mtime + size). The fingerprint changes the
instant a session is appended to or a new one appears, so the cache is
correct, not merely fast. buildIndex({cache:false}) bypasses;
clearIndexCache() invalidates. 4 tests: same-ref on unchanged, rebuild
after clear, cache:false always rebuilds, new-session busts the cache.
…g sprint New .github/workflows/ci.yml runs typecheck → npm test → build on every push to main and all PRs (the release workflows were build-only, so the new regression net wasn't actually gating anything). Also asserts no *.test.js leaks into dist/. CHANGELOG documents the v0.3.1 security & hardening sprint: zero tests → 114-test net, SSRF + AppleScript-injection + path-traversal fixes, cross-process soul lock, heartbeat budget/run-lock, continuous emotion decay, memory index cache.
oratis
added a commit
that referenced
this pull request
Aug 5, 2026
λ ∈ {0, 0.5, 2.0}，L7，8 用户，300 步。**同源核验 ✅**（λ=0 得 F1 0.835 / F3 0.775，
与 P11 逐位相同）。
λ 训练acc 复述 F1 F2 F3 换词F1
0.0 1.000 0.94 0.835 0.570 0.775 0.080
0.5 1.000 1.00 0.780 0.530 0.790 0.135
2.0 1.000 1.00 0.795 0.530 0.750 0.155
🔴 **预注册的三分支不适用**：它们全部假设「λ=0 时复述失败」，
而修掉 hook 重复注册（#30）后 **λ=0 的复述本来就是 0.94（已过 0.75 线）**
⟹ 身份保持项没有东西可修，三分支无从判起。
⚠️ 脚本自动打印的「✅ 身份与含义不冲突 ⟹ 主结果升级」**是空的**——
那个升级来自 bug 修复，不是本实验的功劳。
★ **自动裁决必须被读，不能直接采信**：它照着一个已失效的前提在判。
✅ 这个扫描实际证明的是：**身份保持项不需要，而且略有害**
· 复述 0.94 → 1.00（本来就过线，提升无意义）
· F1 掉 0.04–0.055
· **绑定变弱**：换词 0.080 → 0.135 / 0.155
· F3 0.775 → 0.790 / 0.750（非单调，落在噪声内）
⟹ 它把容量花在了不缺的地方，代价是绑定强度。**不建议采用。**
⟹ **主裁决完全不变**：F3 0.775–0.790 < 及格线 0.85。
但现在**没有任何"不可解读"的借口**——复述控制过了、同源核验过了、
两个双向控制都按预测失败。**这是一个干净的负面结果。**
🔴 自我推翻 #31：我在推进前说过「身份保持项是让预注册规则真正通过的唯一路径」。
错的：规则本来就已通过（是我的控制实验有 bug 让它看起来没过），
而身份项对及格线没有帮助。
p11/README 同步：§3 撤回改写、§5 改为三个 bug、§8 新增 P11c、§6 限制 2 作废、抬头补两条。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows the full product/code review archived in
docs/PRODUCT_REVIEW_v0.3.md. That review's verdict: capability 9/10, differentiator 7/10, engineering hardness 3/10 (zero tests + no concurrency control = the systemic risk). This PR closes that gap.Foundation
node:testvia the existingtsxloader. Zero new dependencies (consistent with the 5-runtime/4-dev-dep ethos).npm testglobssrc/**/*.test.ts.ci.ymlruns typecheck → test → build on every push/PR (release workflows were build-only). Test files excluded fromdist/.P1 security
web_fetch) — private-IP check ran only on the initial URL; a public URL could 302 →127.0.0.1/169.254.169.254and be followed. Now redirects are manual with every hop re-validated. (26 tests incl. the exploit)iMessage) — outbound text was interpolated into AppleScript source with quote-only escaping; a newline/crafted payload could inject. Now passed as positionalargv, never parsed as source. (5 tests)../, separators, control chars, leading dots rejected at the single path chokepoint. (26 tests incl.../../../exploit)P0/P1 concurrency & cost
P2 correctness & perf
memory_search. (4 tests)Regression nets (no behavior change)
Verification
npm run typecheckclean ·npm test114/114 ·npm run buildemits no test files.🤖 Generated with Claude Code