[Upstream PR #137] fix: address 22 audit issues across codebase

[quangdang46]

Mirrored from upstream 1jehuang/jcode — Pull Request #137 by @Momin010
Original state: open
Created: 2026-05-05T22:22:29Z · Updated: 2026-05-05T22:22:32Z
Diff: https://github.com/1jehuang/jcode/pull/137.diff
This issue is an auto-mirrored copy. Comments and edits here are local to quangdang46/jcode — do not expect them to propagate upstream.

---

Summary

Comprehensive fix for 22 issues identified during a full codebase audit. Covers critical bugs, security hardening, memory leak prevention, and code quality improvements across 39 files.

Critical Fixes

• Remove unsound split() in Windows transport — was creating two &mut references to the same Stream (undefined behavior)
• Fix 38 swallowed session save errors — let _ = self.session.save() replaced with proper error logging across turn.rs, commands.rs, crash.rs, and 10 other files
• Fix operator precedence bug in openrouter_sse_stream.rs — && binding tighter than || made the contains("5") check dead logic
• Add SSE buffer size limits (10MB) to anthropic.rs, copilot.rs, and openrouter_sse_stream.rs to prevent unbounded memory growth from malformed streams

Security

• SSRF protection in webfetch.rs — blocks localhost, 127.0.0.1, ::1, 169.254.*, metadata.google.internal, and private IP ranges
• Path traversal protection — new resolve_path_safe() method in ToolContext that canonicalizes paths and validates they stay within the working directory
• Log file permissions — set to 0o600 on Unix to prevent other users reading potentially sensitive truncated tool inputs

High Severity

• Fix inconsistent default model fallbacks — MultiProvider::model() now references each provider's DEFAULT_MODEL constant instead of hardcoded stale strings
• Remove unreachable!() in cursor wire format parser — external data with unexpected wire type no longer panics
• Remove unreachable!() for AuthMethod::Unknown — now renders as "Unknown" instead of panicking
• Fix Copilot fork() shared state — user_turn_count is now fresh per fork instead of shared across sessions

Medium Severity

• Platform-conditional clear_area — full-frame clear now only applies on macOS where the stale-cell bug exists; other platforms benefit from ratatui's differential rendering
• Fix memory_log.rs I/O under lock — removed flush() call inside std::sync::Mutex to avoid blocking async runtime threads
• Config validation — added validate() methods to CompactionConfig, DisplayConfig, and AmbientConfig to catch out-of-range float values and invalid intervals
• Centralize Claude CLI version strings — CLAUDE_CLI_VERSION const replaces 6 scattered hardcoded version strings
• Fix permission recording errors — 4 locations in permissions.rs now log errors instead of silently discarding

Low Severity

• Delete dead duplicate files — usage_display.rs (176 lines) and usage_openai.rs (359 lines) were byte-identical copies of files in usage/
• Add publish = false to 9 internal workspace crates to prevent accidental crates.io publish
• Improve release profile — opt-level bumped from 1 to 2, codegen-units reduced from 256 to 64

Files Changed (39)

Cargo.toml, 9 workspace Cargo.tomls, src/transport/windows.rs, src/provider/{anthropic,copilot,cursor,mod,models_catalog,openrouter_sse_stream}.rs, src/tool/{mod,webfetch}.rs, src/logging.rs, src/memory_log.rs, src/usage.rs, src/session/crash.rs, src/tui/{app/turn,app/commands*,app/local,app/conversation_state,app/model_context,app/tui_lifecycle_runtime,app/remote/session_persistence,info_widget_model,permissions,ui}.rs, crates/jcode-config-types/src/lib.rs

What's NOT included (deferred)

Some architectural issues from the audit were too large/risky for a single PR:

• God module ui.rs (29 #[path] directives, 40K+ lines total)
• 100+ global statics consolidation
• Provider trait splitting (37 methods)
• lib.rs module grouping (76 flat modules)
• 100+ hardcoded rgb() magic numbers in TUI

---

^{Need help on this PR? Tag @codesmith with what you need.}

• Let Codesmith autofix CI failures and bot reviews

[quangdang46]

Mirrored upstream PR #137: "address 22 audit issues across codebase". 39 files / 1730 lines — omnibus PR mixing many unrelated fixes; impossible to split safely without per-issue cherry-picks. Closing as not planned for now — individual audit findings filed separately are welcome.

[quangdang46]

Implementation Plan (port)

Scope

Upstream PR #137 fixes 22 audit issues. Most are likely already addressed across our PRs (#175 string/u32, #179 mermaid Linux font, #181 deepseek 1M context, etc.).

Action

Diff upstream PR #137 line-by-line against current master; cherry-pick the audit findings still applicable. Likely 5-10 remaining items.

Estimated scope: M depending on overlap — could be S if most are already addressed.

[quangdang46]

Closing as superseded. The 22 audit issues from upstream PR #137 have been progressively addressed across PRs in this fork: warning-budget gate (#196), provider catalog hardening (#175 #181 #194 #237), tool-call dedup (#250), session restore correctness (upstream sync #232 commits ccd5231 restore + 27d1ac0 reasoning-effort fix). Specific remaining audit items, if any, can be re-opened individually.