fix(security): SAST audit — shell injection, exit codes, lazy regex + telemetry default

[celstnblacc]

Summary

Full SAST audit of the celstnblacc/rtk fork. All findings are in code we own — no upstream changes involved.

Critical / High

• C-1 — Shell injection via sh -c: run_err / run_test in runner.rs passed the command string directly to Command::new("sh").args(["-c", command]). A semicolon in a test command name (e.g. make test; rm -rf /) would spawn extra shell processes. Fixed with shell_words::split() + Command::new(bin).args(rest) — metacharacters are now literal arguments, not shell syntax.

• C-2 — Exit code not propagated: run_err / run_test returned Ok(()) even when the child process exited non-zero, causing CI to silently pass on failure. Fixed with exit_code_from_output() → std::process::exit(code). Signal-killed processes return 128 + signal per Unix convention.

High

• H-1 — Regex::new() inside function: summary.rs::extract_number() called Regex::new() on every invocation. Migrated to lazy_static! with one static per pattern (RE_PASSED, RE_FAILED, RE_SKIPPED, RE_IGNORED).

• H-2 — Divide-by-zero panic: cc_economics.rs computed savings_blended by multiplying rtk_saved_tokens by totals.blended_cpt.unwrap() before blended_cpt was set. Reordered to derive blended_cpt = cc_cost / cc_total_tokens first, then assign both fields.

• H-3 — Infallible unwrap() on stash subcommand: git.rs matched Some("pop") | Some("apply") | ... then immediately called subcommand.unwrap(). Replaced with Some(sub @ ("pop" | "apply" | "drop" | "push")) binding pattern — no unwrap needed.

Medium

• M-1 — Telemetry on by default: TelemetryConfig::default() set enabled: true. This fork has stripped telemetry; the default now reflects that (enabled: false).

• M-2 — CWD fallback produces empty path: resolved_command used unwrap_or_default() for the CWD fallback, which returns an empty PathBuf. Changed to unwrap_or_else(|_| PathBuf::from(".")).

• M-3 — unwrap() on HashMap::get in report.rs: grouped.get(&base_cmd).unwrap() would panic if a key appeared in the iterator but not the map (possible under concurrent modification). Replaced with let Some(rules) = grouped.get(&base_cmd) else { continue }.

Low

• L-2 — status.code().unwrap_or(1) throughout: All remaining ExitStatus::code().unwrap_or(1) calls in gh_cmd.rs, gt_cmd.rs, git.rs, and main.rs migrated to exit_code_from_status() — returns 128 + signal on signal termination instead of 1.

Test plan

• cargo test --all — 1152 tests passing (5 ignored integration tests)
• cargo clippy --all-targets — zero warnings
• cargo build --release — clean build
• New TDD tests added: test_split_command_semicolon_is_literal, test_split_exec_blocks_semicolon_injection, test_run_err_propagates_exit_code (#[ignore] — integration)
• ShipGuard SAST scan — 0 findings on changed files

🤖 Generated with Claude Code

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

newblacc seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[celstnblacc]

Closing — this is a fork-internal security audit for celstnblacc/rtk, not intended for upstream.