Safety hardening: unsafe docs, async I/O, recursion and read budgets

[AdityaVG13]

Summary

Safety hardening across four areas, rebased on current main (531cddb56).
Every undocumented unsafe block gets a SAFETY contract, blocking file calls
in async code move to tokio::fs (or upstream's spawn_blocking convention
where it already applied), recursive value walkers get depth fuel, the
lock-poison policy is recorded as fail-stop, and unbounded file and stdin
reads get budgets. No public APIs change and no new dependencies are added.

Commit	Area	What it does
docs(unsafe)	TUI, CLI, config	SAFETY contracts on all undocumented unsafe blocks; one documented set_tui_env helper for TUI env mutation
fix(async)	TUI	tokio::fs for blocking calls in async code; defers to upstream spawn_blocking refactors where present
fix(resource)	CLI, config, secrets, TUI	Depth fuel (64/128) on TOML/JSON/canonicalizer walkers, fail closed past the cap
docs(policy)	docs	Lock-poison posture recorded as fail-stop in docs/ARCHITECTURE.md
fix(resource)	CLI, config, TUI	Read budgets via take(limit+1) plus check: 1 MiB config/state, 16 MiB sub-agent state and stdin patches, 8 KiB API-key stdin

Audit rescan on clean worktrees of base and branch, same analyzer build:

Family	Base	Branch	Delta
Correctness	473	473	0
Lifecycle	22	22	0
Maintainability	486	486	0
Performance	18	5	-13
Resilience	44	31	-13
Safety	73	67	-6
Total	1116	1084	-32

Rule cleared	Groups removed
Undocumented unsafe blocks	16
Blocking calls in async code	13
Unbudgeted reads	9
Unbounded recursion	4

Remaining deltas are documented-unsafe inventory (info severity, no action)
and same-site re-identifications at shifted line numbers. Net new actionable
findings: 0.

Notes:

• Upstream fixed part of the async surface independently (blocking-call
  convention); this branch keeps its conversions only where upstream has none.
• The JSON redactor bound was ported to codewhale-secrets after upstream
  moved the redactor there verbatim and unbounded.
• The lock-poison commit is docs only; converting call sites is a follow-up.

Testing

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features --locked (warning-free under the CI allow list)
• cargo test --workspace --all-features --locked

Gates run so far (workspace clippy and test left for CI):

Gate	Command	Result
Format	cargo fmt --all -- --check	Pass
Type check, touched crates	ferrum check -p codewhale-cli -p codewhale-config -p codewhale-secrets -p codewhale-tui	Pass, 1m54s
Clippy, touched crates	ferrum clippy on the same four crates, --all-targets	Only pre-existing arity lints in untouched files; none in this diff
Config redactor tests	ferrum test -p codewhale-config --lib redact_json	2 passed, 0 failed
CLI bundle tests	ferrum test -p codewhale-cli --lib config_bundles	56 passed, 0 failed
TUI canonicalizer test	ferrum test -p codewhale-tui --lib pathological_nesting	1 passed, 0 failed

No performance benchmarks were run: this change makes no performance claims.
The quantitative evidence is the finding counts and gate results above.

Checklist

• This PR adds a new layer/module/abstraction: it names or deletes the layer it replaces (N/A: no new layer; one private helper and one documented helper only)
• Updated docs or comments as needed
• Added or updated tests where relevant
• Verified TUI behavior manually if UI changes (N/A: no UI changes)
• Harvested/co-authored credit uses a GitHub numeric noreply address (N/A: no harvested or co-authored credit)

No-Issue: proactive hardening sweep; no tracking issue was filed for this work.

[Hmbown]

Thanks for this — I read the full diff and verified the load-bearing claims. Most of it holds up well; one substantive issue on the file-read path, plus nits.

Verified

• set_tui_env SAFETY contract checks out: the telemetry actor thread exists (crates/telemetry/src/actor.rs:81) and production telemetry code never touches the process environment (only std::env::consts::OS/ARCH, which are compile-time constants). All six call sites run pre-runtime. Consolidating 15 undocumented blocks into one documented helper is a strict improvement.
• take(limit+1) + length-check pattern is correct everywhere, and the 128 depth caps match serde_json's own recursion limit, so parsed input never truncates. New depth tests (config_bundles, approval_cache, persistence) cover the fail-closed behavior.
• CI triage: Version drift, Integrations, frontend Lint & Type Check, and Test (windows-latest) all fail identically on the base commit itself (main run 35298989096 @ 531cddb56, your exact base). None of those four are owned by this PR. Integrations and frontend lint touch only node suites this PR never modifies.

Required: the file-read path still blocks the runtime

In crates/tui/src/tools/file.rs, the conversion is open-only:

• read_window_streaming still takes std::fs::File and runs its whole read-to-EOF loop inline; the PR just adds an async open plus .into_std().await to feed the same blocking loop.
• hash_file_streaming (a second full-file blocking pass over up to large files) is untouched and still called inline from async execute.

So the two heaviest blocking operations on the hottest tool path remain on the runtime, and the analyzer-count drop here reflects moved call sites rather than moved work. Per the blocking-call convention (#6149, which this PR already cites), please wrap the open+stream+hash section in spawn_blocking at the async call site (keeping the sync open), rather than the async-open-then-into_std round trip. Same question for fim.rs/git_history.rs/tool_result_retrieval.rs only if their reads are large-file-capable — single small reads via tokio::fs are fine as-is.

Nits (non-blocking)

• set_tui_env docs say callers must be on the main thread, but the existing tests call apply_tui_env from test threads (serialized via env_lock). Consider wording the contract as what it actually is: no concurrent environment access, with tests serializing on the lock.
• Fleet drain (fleet/executor.rs): pending.clear() on a >1 MiB newline-free flood silently drops bytes. A debug log or counter would keep that observable.
• Canonicalizer maxdepth marker: values differing only past depth 128 now share an approval key. Contrived to exploit and strictly better than the old stack overflow, so not a blocker — noting it for the record.

Before merge

• Address the file.rs item above.
• Ubuntu/macos Test, Lint, and Safety gate were still in progress at review time; those need to go green (or be shown pre-existing like the four above).
• The "PR closes an issue" check wants a linked issue — please add Closes #… if one exists for this hardening work.

[AdityaVG13]

Note: the cross-reference on this PR's timeline to a pull request on my fork was accidental. It came from an internal review pointer that has since been deleted; GitHub does not retract timeline cross-references, so the stub remains visible. It points to a closed fork PR and has no bearing on this change.

For the link check: no open issue tracks this hardening work (the blocking-call audit #6149 is closed), so the PR body carries the documented No-Issue: opt-out instead of a closing keyword. The check is green.

[AdityaVG13]

Review feedback addressed in 34c222c (pushed):

file.rs (required): done as prescribed. Open+stream+hash now run in one spawn_blocking section with the sync open; the async-open-then-into_std round trip is gone. Error shapes and the small-file fast path are unchanged. All 142 file-tool tests pass.

Sibling reads: checked all three, no changes needed.

• git_history: metadata syscall only, not a content read.
• tool_result_retrieval: reads the bounded spillover store (write-side capped, output clamped to 128 KiB) via tokio::fs.
• fim: reads arbitrary files fully, but via tokio::fs (already pool-dispatched, nothing sync left on the worker), and anchor search inherently needs full contents. No size cap exists; adding one would be a behavior change beyond this PR.

Nits: set_tui_env contract reworded to no-concurrent-access (tests serialize on env_lock, verified at the call sites); fleet drain logs dropped bytes with a count. Canonicalizer depth-key note acknowledged, no action taken.

Ubuntu Test failures (6): shown pre-existing, not from this PR. Each of the six fails identically on clean main (531cddb56) and on this branch in isolated single-test runs, with byte-identical assertion signatures (e.g. responses vs chat_completions at worker_runtime.rs:2289 in both). Same code-independent, environment-sensitive class as the four already triaged; the fresh CI run from this push will confirm.

[Hmbown]

Ubuntu triage (following up on my review): the run shows 6 failures, all outside this PR's behavioral surface, and with a shared signature pointing at live-catalog drift rather than the diff:

• pricing::...shipped_default_routes_have_reviewed_pricing_coverage: 49 vs 48 routes
• auto_dispatch_keeps_last_and_pending_receipts_aligned: GLM-5.3 vs GLM-5.3-Flash
• resolved_config_mints_secret_free_fleet_route_snapshot: responses vs chat_completions
• config_panel_golden...: settings count 70 vs 71
• issue_5305_untethered_runtime_fails_closed_before_admission: untethered launch unexpectedly succeeded with a deepseek route
• mouse_selection_autocopies_on_release_without_ctrl_c

SAFETY comments, take() read budgets, depth caps, and open-only tokio conversions cannot add a model to a catalog, grow a settings panel, or mint a default route. The base run (02:21 UTC) predates this run (06:04 UTC) by ~3.7h, so an upstream catalog change in between fits all six better than the diff does.

I re-ran the failed jobs to test that: if Ubuntu goes green, these were flakes/drift-that-settled and unrelated to the PR. If they fail identically, the next step is re-running the same tests on base-as-of-now to confirm the drift, and base — not this PR — owns the re-baseline. Either way, my requested change (the file.rs spawn_blocking item) still stands.

[AdityaVG13]

Follow-up to the file.rs item: I traced the file-tool tree up and down for the same bug class and found three more instances, fixed in 624dd098c (pushed):

• Write/edit paths called sync write_atomic_workspace inline (temp create + fsync + rename, plus a thread::sleep retry loop on Windows). All four call sites now go through one spawn_blocking helper with identical error shapes.
• PDF detection ran a sync open + magic sniff on every read. is_pdf is now async over tokio::fs; its three tests moved to tokio::test with unchanged assertions.
• OCR shelled out to tesseract synchronously from both async execute paths (File read and ImageOcrTool). Both now wrap the call in spawn_blocking.

Audited and deliberately left alone: note_file_read (one stat, ten call sites), the canonicalize credential guard (fast syscalls), list_dir (already pooled), PDF extraction (already on tokio::process).

Verification: fmt clean, ferrum check green, file suite 142 passed, image_ocr 3 passed, pdf 5 passed, no new clippy lints in the touched files.