Skip to content

[audit] Ordering, generation, and TOCTOU races across durable state #6557

Description

@7jrxt42BxFZo4iAnN4CX

Source: static audit of main@384439634. Candidates are static and not reproduced — confirm each before changing code.
Template: agent-task (agent-ready, docs/ISSUE_TRIAGE.md). Labels: bug, agent-ready.

Goal / Why

Cursor/sequence commits before the side effect, missing generation/session fencing, and check-then-use (TOCTOU) windows let stale events, restarted sequences, or concurrent writers clobber newer state. An event from an old turn, a restarted generator, or a second process is applied without a session/turn generation check, and a cursor is sometimes advanced before the effect it describes. The result is newer state silently overwritten by an older event, or a duplicated effect from a re-run sequence. Fence mutations by generation, commit cursors after the side effect, and close TOCTOU windows with atomic create/open.

Confirmed static findings in this class: 22 (reassessed severity noted per line).

Scope / Plan

  1. Re-read each candidate below at the cited file:line on current main; drop anything already fixed or misread.
  2. Fence mutations by session/turn generation; commit cursors/checkpoints after the side effect; close TOCTOU with atomic create/open.
  3. Add the smallest regression/gate that would catch a re-introduction (focused test, budget script, or grep guard).

Key files

  • crates/tui/src/tui/ui/event_loop.rs
  • crates/core/src/lib.rs
  • crates/tui/plugins/computer-use/src/backends/harmonyos.mjs
  • crates/tui/src/mcp/oauth.rs
  • crates/tui/src/tui/views/fleet_setup.rs
  • crates/state/src/lib.rs
  • crates/tui/src/core/engine.rs
  • crates/tui/src/tools/subagent/delivery.rs
  • crates/tui/src/models_dev_live.rs
  • crates/tui/src/compaction.rs
  • crates/tui/src/tui/ui/frame.rs
  • crates/tui/src/tui/pet_watch/mod.rs
  • crates/tui/src/tui/pet_watch/live.rs
  • crates/config/src/lib.rs

Acceptance criteria

  • Every listed candidate is either fixed with a test, or downgraded with a written reason in the PR.
  • No new instance of this class is introduced (guarded by the test/budget).
  • Existing behavior for unrelated paths is unchanged.

Verification

cargo check -p codewhale-tui -p codewhale-core -p codewhale-state -p codewhale-config
cargo test  -p codewhale-tui --lib
cargo test  -p codewhale-state -p codewhale-config
cargo clippy --workspace --all-targets --locked -- -D warnings
( cd pet && npm run check )
( cd web && npm run check )

Out of scope

  • Findings that belong to another systemic class or to the localized backlog.
  • Re-architecture beyond the listed sites.

Related existing work (do not duplicate)

#6184 (engine freeze mid-run), #6207 (session picker), #6224, #5151, #5929, #5735

Candidate findings (22)

  • CU01-10 · CU01 — Computer-use · crates/tui/plugins/computer-use/src/backends/harmonyos.mjs:107 — stale element index after tree reorder clicks a different control.
    audit severity: high; reassessed: major
  • T06-08 · T06 — Remote control, MCP, OAuth, cloud dispatch · crates/tui/src/mcp/oauth.rs:829 — rejected refresh can delete a newer concurrent login due to a load→delete race.
    audit severity: high; reassessed: major
  • U09-03 · U09 — Dashboards, Fleet/settings/status/help · crates/tui/src/tui/views/fleet_setup.rs:1227 — late async draft without request generation is applied to changed wizard state.
    audit severity: high; reassessed: major
  • R03-07 · R03 — Agent, app-server, models, protocol, state, shared tools · crates/state/src/lib.rs:368 — two-process fresh DB migrations race after pre-transaction schema checks.
    audit severity: high; reassessed: major
  • C02-08 · C02 — TUI engine and turn loop · crates/tui/src/core/engine.rs:2253 — session replacement does not cancel in-flight MCP boot; generation/in-flight stay stale.
    audit severity: major; reassessed: major
  • D02-11 · D02 — agent/subagent runtime · crates/tui/src/tools/subagent/delivery.rs:297 — checkpoint >1000 paths switches to git add -A, capturing foreign edits.
    audit severity: major; reassessed: major
  • T03-07 · T03 — Models, providers, routing, pricing · crates/tui/src/models_dev_live.rs:230 — cache fingerprint is not checked, a future timestamp becomes age=0 and suppresses refresh.
    audit severity: major; reassessed: major
  • T07-05 · T07 — Sessions, context, compaction, prompts, memory · crates/tui/src/compaction.rs:324 — legacy restore deletes a user message merely containing a checkpoint marker.
    audit severity: major; reassessed: major
  • U02-07 · U02 — Event loop, dispatch/apply · crates/tui/src/tui/ui/event_loop.rs:2503 — TurnComplete without session/turn generation can close a new turn.
    audit severity: major; reassessed: major
  • U02-09 · U02 — Event loop, dispatch/apply · crates/tui/src/tui/ui/event_loop.rs:123 — shutdown after local cancel considers the session settled before the authoritative terminal event.
    audit severity: major; reassessed: major
  • U03-11 · U03 — Frame, terminal/session UI state · crates/tui/src/tui/ui/frame.rs:1819 — modal overlay leaves the native cursor on the composer.
    audit severity: major; reassessed: major
  • U06-01 · U06 — Ambient, pet watch, ocean, notifications · crates/tui/src/tui/pet_watch/mod.rs:313 — stale TurnComplete without turn ID closes the wrong turn.
    audit severity: major; reassessed: major
  • U06-03 · U06 — Ambient, pet watch, ocean, notifications · crates/tui/src/tui/pet_watch/live.rs:339 — queue overflow/POST error clears events/sequence, creating a producer gap; 64×16KiB exceeds the 64KiB HTTP cap.
    audit severity: major; reassessed: major
  • R01-12 · R01 — codewhale-config · crates/config/src/lib.rs:3411 — route resolve().ok() discards the error and returns a stale model/base URL as success.
    audit severity: major; reassessed: major
  • R05-01 · R05 — Core, memory, telemetry client/ingest · crates/core/src/lib.rs:651 — persist/resume writes current_leaf_id=None, losing the active branch cursor.
    audit severity: major; reassessed: major
  • R05-05 · R05 — Core, memory, telemetry client/ingest · crates/core/src/lib.rs:584 — thread/message/checkpoint mutations are not atomic; partial history possible.
    audit severity: major; reassessed: major
  • D03-m3 · D03 — Git/GitHub/review/web tools · crates/tui/src/tools/finance.rs:68 — f64 without finite/decimal policy, stale market time, and doubled full timeout.
    audit severity: medium; reassessed: medium
  • U04-m1 · U04 — History, transcript, markdown, diff, streaming · crates/tui/src/tui/live_transcript.rs:204, crates/tui/src/tui/transcript_cache.rs:45 — Cache key does not include render options; theme/locale/thinking changes reuse stale cells.
    audit severity: medium; reassessed: medium
  • O01-m1 · O01 — CI/release/installers/packaging · .github/workflows/release-republish.yml:78 — Republish accepts partial asset inventory and stale checksum-derived channels.
    audit severity: medium; reassessed: medium
  • W02-07 · W02 — Website API, Cloudflare/KV, generators · web/lib/cloud-facts.ts:300 — Stale 30-day KV fallback re-exposes signed facts after revoke uncertainty.
    audit severity: high; reassessed: high
  • P01-01 · P01 — Cross-platform Pet · pet/src/core/pet-telemetry.ts:40, pet/src/core/pet-world.ts:290 — First packet/reset baseline and non-monotonic sequence semantics admit stale packets.
    audit severity: major; reassessed: major
  • P01-05 · P01 — Cross-platform Pet · pet/scripts/build-pet.mjs:20 — TypeScript not enforced as single source; native artifacts may stale/diverge while the owned build is green.
    audit severity: major; reassessed: major

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageNew external report awaiting maintainer triage; repro, logs and version output help

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions