Security, perf, and robustness hardening (audit-driven) + README refresh#27
Merged
Security, perf, and robustness hardening (audit-driven) + README refresh#27
Conversation
Addresses critical findings from a security/correctness/performance audit.
Security (P0):
- voice.ts, claude/env.ts: replace `execSync(${shell} -ilc '...')` with
`execFileSync(shell, ['-ilc', ...])` and add a safe-shell allowlist to
block `$SHELL=` command-injection via template interpolation.
- claude-token.ts: remove `shell: true` from the `claude setup-token`
spawn and resolve the CLI to an absolute path via `which`/`where`
before invoking, so metacharacters in PATH can't be interpreted.
- auth-manager.ts + index.ts: the `/auth` deep-link handler now requires
an in-flight OAuth flow initiated by `startAuthFlow()`. A per-flow
`state` nonce is generated, attached to the auth URL, and verified
with constant-time compare on callback. Unsolicited deep links (drive-
by CSRF via `twentyfirst-agents://auth?code=...`) are rejected.
Performance (P1):
- db/index.ts: add `PRAGMA synchronous = NORMAL` under WAL to reduce
fsync load during high-frequency message persistence.
- db/schema/index.ts + drizzle/0009_*: add indexes on `chats.project_id`,
`sub_chats.chat_id`, and `sub_chats.stream_id` (previously full-table
scans on hot FK lookups).
- claude.ts: coalesce `text-delta` chunks in a 24ms buffer keyed by
text id. Non-delta chunks flush the buffer first to preserve ordering.
Cuts tRPC/IPC chatter during long streaming responses.
Robustness (P2):
- db/index.ts: migration failures now quarantine the DB file to
`agents.db.broken-<timestamp>` and start fresh instead of crashing
the app on every launch.
- index.ts: bound `cleanupGitWatchers()` in `before-quit` to 1.5s so a
hung chokidar instance can't block app quit indefinitely.
Dismissed during verification:
- `sandbox: false` in webPreferences is intentional for electron-trpc;
contextIsolation remains enabled.
- `activeSessions` map cleanup is already wired on unsubscribe and in
the stream's finally block.
- The stream's async IIFE is wrapped in try/catch/finally; no
unhandled rejection.
- Project deletion is atomic under FK cascade with `foreign_keys=ON`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the user-visible features shipped since the README was last touched (commits #18 through #26) and groups the fork additions into four sections for readability: Workflow & UI, Git/PRs/Worktrees, Models, and Stability & Polish. Highlights surfaced: split view with drag-to-split, Cmd+Shift+T, sortable sidebar, draggable tabs, queue reorder, copy popover, PR widget + branch switcher, two-column commit diff, Pull & Push recovery dialog, worktree deletion safety, Sonnet 4.6 1M context + recovery action, GPT-5.4, rich tool rendering, stream wedge timeout, and crash auto-recovery. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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.
Summary
Two commits shipped off a security/correctness/performance audit of
src/main/**. Every finding was verified by direct source reads — a few initial agent claims were dismissed (see commit body).Security (P0)
voice.tsandclaude/env.tsno longer interpolate$SHELLinto a shell string; they now useexecFileSync(shell, [...argv])with a safe-shell allowlist.claude-token.tsdropsshell: trueand resolves theclaudeCLI to an absolute path viawhich/wherebefore spawning./authhandler now requires an in-flight flow initiated bystartAuthFlow(). A per-flowstatenonce is generated, attached to the auth URL, and verified with constant-time compare on callback. Drive-by deep links (twentyfirst-agents://auth?code=...from a malicious page) are rejected.Performance (P1)
PRAGMA synchronous = NORMALunder WAL — cuts fsync load during streaming persistence.chats.project_id,sub_chats.chat_id, andsub_chats.stream_id(previously full-table scans). New migrationdrizzle/0009_wise_rumiko_fujikawa.sql.Robustness (P2)
agents.db.broken-<timestamp>and a fresh DB initialized, so corrupt-DB state can't brick app launch anymore.cleanupGitWatchers()on quit (1.5 s) so a hung chokidar instance can't block shutdown.Docs
Fork Additionsnow reflects commits fix(stability): remaining crash/hang fixes from stability pass #18–feat: drag-to-split, queue reorder, Cmd+Shift+T, PR auto-refresh #26 and is grouped into Workflow & UI, Git/PRs/Worktrees, Models, and Stability & Polish for readability.Out of scope / dismissed during audit
sandbox: falsein webPreferences is intentional for electron-trpc (contextIsolation remains on) — defense-in-depth, not a critical gap.activeSessionscleanup is already wired in both the stream'sfinallyand the observable's unsubscribe handler.claude.tsis wrapped intry/catch/finally; no unhandled-rejection path.foreign_keys=ON.Test plan
SHELL='/bin/zsh; echo pwned > /tmp/pwn', trigger the voice and shell-env bootstrap paths, confirm/tmp/pwnis not written.open "twentyfirst-agents-dev://auth?code=foo"without clicking Sign In first → rejected in log, no exchange attempted.EXPLAIN QUERY PLAN SELECT * FROM sub_chats WHERE chat_id=?→ usessub_chats_chat_id_idx.agents.dbto simulate corruption → relaunch produces a fresh DB withagents.db.broken-*sibling.bun run dev, create chat, send message in Plan and Agent mode, rollback, fork, switch sub-chats.