# Bad-Pattern Audit — full-tree snapshot

Generated by `scripts/audit-bad-patterns.sh` against every `.rs` file under
`src/`. The pattern matrix is the one defined in
[`docs/BAD_PATTERNS.md`](BAD_PATTERNS.md) (sections A–P, 131 regexes).

| Scope | Files | Patterns scanned | Patterns with hits | Total hit lines | Strict-section hits (A/B/C/L/M/N/O) |
|---|---|---|---|---|---|
| **My 100 authored modules** (`/tmp/my_files.txt`) | 100 | 131 | 0 | **0** | 0 |
| **Whole codebase** (`find src -name '*.rs'`) | 486 | 131 | 53 | **3218** | 1197 |

```
$ scripts/audit-bad-patterns.sh --strict --files /tmp/my_files.txt
[...]
  Total hit lines        : 0
  Strict (A/B/C/L/M/N/O) : 0
$ echo $?
0
```

## What this round of clean-up fixed

Compared to the previous snapshot (4252 / 2175), the codebase-wide totals
fell by **1034 lines (24%)** and the strict-section subset by **978
lines (45%)**. The drops are partly real fixes and partly tighter
regexes that no longer flag value-providing fallbacks as panics.

| Wave | What was done | Files touched | Hit-line drop |
|---|---|---:|---:|
| 1 | `src/spool.rs`: `Result<_, String>` → `anyhow::Result<_>`. Both call sites in `src/ws.rs` updated to format the error via `Display`. | 2 | 10 |
| 3 | Bare `.send().await?` / `.text().await?` chains gain `with_context()` so mass-scan failures attribute to URL + verb. Added `anyhow::Context` import where missing. | 23 | 46 |
| 4 | DoS `http_flood.rs` now goes through `crate::utils::network::build_http_client_with(...)`; new `pool_max_idle_per_host` field on `HttpClientOpts` so the framework helper covers the high-concurrency case. | 2 | 1 |
| 6 | 42 `let body = <expr>.text().await.unwrap_or_default()` rewritten as explicit `match` arms that log via `mprintln!` (or `eprintln!` when the file doesn't use the framework macro), preserving the empty-string fallback for downstream string searches but no longer hiding the decode error. | 26 | 42 |
| 8 | `unreachable!()` in `src/native/obfuscator_engine.rs` removed (was guarding an exhaustive match arm that couldn't be reached anyway). 2 `.expect("ASCII hex")` panic-vectors in payloadgen / obfuscator converted to `with_context()?`. 1 `.expect("ALL_METHOD_IDS must be non-empty")` in obfuscator's RNG selection replaced with a typed Some/None match returning a safe fallback. | 3 | 4 |
| Tightened audit regexes | Pattern `\.unwrap` → `\.unwrap\(\)`, `\.expect_err` → `\.expect_err\(`, etc. — no longer counts `.unwrap_or(default)` as a panic. Comment lines (`//` / `///`) skipped. | — | ~970 false-positive removals |

After all waves, every wave-target pattern is **zero** in the codebase:

| Pattern | Before | After |
|---|---:|---:|
| `.unwrap()` | 0 | 0 |
| `.expect(` | 4 | 0 |
| `panic!()`, `unreachable!()`, `todo!()`, `unimplemented!()` | 1 | 0 |
| `Result<_, String>` (in `spool.rs`) | 10 | 0 |
| Bare `.send().await?` (no context) | 14 | 0 |
| Bare `.text().await?` (no context) | 29 | 0 |
| `let _ = x.text().await.unwrap_or_default()` | 42 | 0 |
| `reqwest::Client::builder()` outside the framework helper | 1 | 0 |

## Per-section table (whole codebase, post-fix)

| Section | Hits | Notes |
|---|---:|---|
| **A. Panicking error handling** | 44 | 41 `unwrap_or_default()` (almost all on `serde_json::to_vec(json!(...))` of literal struct — impossible to fail; the rest are `Option::unwrap_or_default()` value-providing fallbacks) + 3 `assert!` / `assert_eq!` inside `#[test]` blocks. |
| **B. Silent error swallowing** | 1100 | Long-running scanner / brute-force loop pattern (try one host, continue on failure). For new modules the catalogue bans the anonymous form (B1–B5); for pre-existing scanners the mass-scan helper already aggregates per-target failure counts so the unattributed `Err(_)` is acceptable. |
| **C. Lint suppression** | 23 | All 23 are `#[allow(dead_code)]` on `pub` framework helpers consumed only via the API/MCP/WS auto-dispatchers; each one carries a comment line explaining the cross-layer use. |
| **D. Panic vectors (index/slice)** | 848 | `arr[N]` 597, `&buf[..n]` 164. Concentrated in protocol parsers where the length is checked one line above. |
| **E. Numeric / unsafe** | 1014 | 843 numeric `as` casts in protocol parsers (RDP, PQ ratchet, packet builders), 22 `unsafe { }` blocks (concentrated in `src/native/*` for FFI), 12 pointer casts in the same FFI files. No `transmute`, no bare `unsafe fn` outside `crate::native::*`. |
| **F. Async / blocking** | 91 | DoS / scanner / brute-force modules where blocking-I/O is intentional, plus build / config / setup paths that legitimately call sync APIs. |
| **G. Logging** | 0 | clean |
| **H. HTTP layer** | 2 | Both are inside `src/utils/network.rs` itself: line 477 is a doc comment, line 501 is the framework's authoritative `build_http_client_with`. |
| **I. Iterator glitches** | 0 | clean |
| **J. Style / secrets** | 70 | 41 `"admin","admin"` and 25 `"root","root"` are credential brute-force *seed lists* — not embedded secrets. 1 `Box<dyn`, 2 `TODO/FIXME/HACK`, 1 `len() > 0`. |
| **L. Crypto** | 23 | All by-design (Postgres MD5 auth, MySQL SHA-1 auth, VNC DES, scanner cipher enumeration, NetBIOS xid). Documented in `BAD_PATTERNS.md` "Codebase-wide observations". |
| **M. SQL & command injection** | 0 | clean |
| **N. UB / concurrency** | 9 | 4 `std::mem::zeroed()` inside `unsafe { }` blocks for `libc::sockaddr_*` / `libc::rlimit` FFI structs; 5 `Result<…, String>` (down from 10 — the spool.rs ones are gone, the remaining are in non-publishable `t/` test code). |
| **O. Performance** | 0 | clean |
| **P. API hygiene** | 0 | clean |

## Why the strict-mode total is still 1197

The strict gate considers sections A, B, C, L, M, N, O. After the fixes
above, the breakdown is:

```
A  (44) 41 unwrap_or_default + 3 test asserts
B  (1100) Err(_) + if let Ok(...)+let _ = pre-existing in scanners
C  (23) #[allow(dead_code)] with explanation comments
L  (23) protocol-required crypto
M  (0)
N  (9) FFI-internal mem::zeroed + leftover Stringly-typed errors
O  (0)
```

Mass-fixing the **B** bucket (the bulk: 1100 lines) would change scanner
behavior — those `Err(_) => { /* keep going */ }` arms are intentional
"try one host, skip on failure, continue" loops. The catalogue's **strict
mode is enforced on new modules only**; pre-existing scanners that
follow the legacy convention pass through unchanged. To bring those
older modules in line, do them per-module with care to preserve the
"skip-on-failure" semantics while reporting per-target outcomes. That's
multi-day per-module review work, not mechanical replacement, and it's
out of scope here.

## Per-module strict gate (new modules) — clean

```
$ scripts/audit-bad-patterns.sh --strict --files /tmp/my_files.txt
RUSTSPLOIT BAD-PATTERN AUDIT
100 file(s) under audit
[...]
Patterns scanned       : 131
Patterns with hits     : 0
Total hit lines        : 0
Strict (A/B/C/L/M/N/O) : 0
```

## How to re-run

```sh
# Whole codebase, informational
scripts/audit-bad-patterns.sh

# One section
scripts/audit-bad-patterns.sh --section A

# Strict gate on the modules I authored (or any file list)
scripts/audit-bad-patterns.sh --strict --files /tmp/my_files.txt

# CI gate (run by reviewer on PR file list)
git diff --name-only origin/main...HEAD | grep '\.rs$' > /tmp/changed.txt
scripts/audit-bad-patterns.sh --strict --files /tmp/changed.txt
```

The `--strict` flag exits non-zero if any pattern in sections **A, B, C,
L, M, N, or O** has a hit on the audited file set. Sections **D, E, F, H,
J** cover code-quality items that need human review (a `&buf[..n]` slice
right after a length-checked read is fine; a brand-new one isn't).