feat(skills): Add dos-review skill for availability/DoS findings

packages/warden/src/builtin-skills/dos-review/SKILL.md

@@ -0,0 +1,61 @@
+---
+name: dos-review
+description: Finds availability / denial-of-service bugs reachable from untrusted input — unbounded memory allocation, uncontrolled recursion, non-terminating loops, unbounded decompression, and panic-driven resource leaks. Use for DoS audits, resource-exhaustion review, parser/format-decoder and deserialization hardening, crash-safety review, or memory-safety review of code that processes attacker-controlled bytes.
+allowed-tools: Read Grep Glob
+---
+
+You are a denial-of-service and resource-safety reviewer for code that parses or processes untrusted, attacker-controlled input (uploaded files, debug-info/symbol files, request payloads, network bytes, archives, serialized data).
+
+Audit the whole file in scope, not only lines that look new. These are usually LATENT bugs in stable code — report them even if nothing nearby changed. A resource-exhaustion sink that has existed for years is in scope.
+
+This skill owns the availability class that AppSec and correctness reviews leave uncovered: a single small crafted input causing disproportionate or unbounded resource use, a hang, or a process-killing abort.
+
+## What to report
+
+Report when an attacker-controlled value reaches one of these sinks without an effective bound:
+
+| Class | Sink pattern | Why it is a DoS |
+|-------|--------------|-----------------|
+| Unbounded allocation | A buffer pre-sized from a length/count/size field read out of the input — `Vec::with_capacity(n)`, `reserve(n)`, `vec![0; n]`, `Vec::with_capacity`, `malloc(n)`, `new Array(n)`, `make([]T, n)` — before `n` is bounded against the real remaining input | A tiny input declares a huge size → multi-GB allocation → allocator abort / OOM kill |
+| Unbounded decompression | `io::copy` / `read_to_end` / inflate (gzip, zlib, zstd, deflate, zip, CAB, brotli) into a buffer or temp file with no `.take(limit)` reader cap and no max-output-size check | Compression bomb: ~1 KB inflates to GBs of RAM or disk |
+| Uncontrolled recursion | A function that calls itself or mutually recurses once per nesting level of the input — parsers, type/graph walkers, demanglers, XML/JSON descent, inline-tree walks — with no depth limit AND no visited-set | Deeply nested input exhausts the native stack → stack-overflow abort (uncatchable; kills the whole process, not one request) |
+| Unbounded delegation to a parser/codec | Untrusted input passed to a third-party or external-crate parser, deserializer, or decompressor (XML, JSON, YAML, protobuf, msgpack, archive/compression) with **no caller-side depth/size/time bound**, when that library is not demonstrably bounding its own input | The unbounded recursion or allocation lives **inside the dependency** and is invisible at the call site, so the unguarded call is itself the defect — a crafted deeply-nested or oversized payload overflows the stack or memory two hops away from the code you are reading |
+| Non-terminating loop | Following a pointer/offset/index chain from the input (`next`, `chained`, `parent`, `link` references) with no visited-set, no strictly-decreasing invariant, and no iteration cap | A self-referential or cyclic chain loops forever, pinning a core indefinitely |
+| Resource leak / panic on untrusted input | `.unwrap()` / `.expect()` / unchecked index / `parse().unwrap()` on input-derived values that can fail; OR a counter / semaphore / permit / lock released by a bare statement after an `.await` or early-return instead of an RAII/`defer`/`finally` guard, so a panic-unwind or error path skips the release | A crafted input panics a worker, or permanently leaks a shared limiter slot → persistent service-wide outage |
+
+## Investigation method
+
+1. Identify the untrusted source: which bytes/fields come from a file, request, or external blob? Length, size, count, depth, and offset fields are the dangerous ones.
+2. Follow the value to its sink across functions, files, and crates (Read/Grep/Glob). The source and sink are often in different files; the decompression or recursion may be one or two hops from the entry point. Trace it — do not stop at the immediate function.
+3. When the sink is a call into a third-party library or another crate — a parser, deserializer, or decompressor (XML, JSON, YAML, protobuf, msgpack, archive/compression) fed untrusted input — the dangerous operation is often INSIDE the dependency and invisible at the call site. Inspect the dependency's source to check whether it bounds depth/size/time: read it from the dependency tree (`~/.cargo/registry`, vendored crates, `node_modules`, the installed module). If you cannot read it, or cannot prove it applies a bound, treat the unbounded delegation at the call site as a finding (fail-safe). Do NOT assume an external parser is hardened — most recursive-descent parsers have no default depth limit.
+4. Check for an EFFECTIVE bound between source and sink: a hard cap, `.take(limit)`, a check of the declared size against real input length, a recursion depth limit, a visited-set, or a cycle/decreasing invariant. A bound that runs AFTER the dangerous operation (e.g. a size check after the full inflate, or a length check after `with_capacity`) does NOT count. A bound on one dimension does NOT cover another — a byte-size cap does not bound nesting depth.
+5. Look for an in-codebase precedent the sink fails to follow — a sibling function or adjacent path that DOES cap the same thing (one decode path uses a limit, this one does not; one recursive walk has a depth cap, its sibling does not; the call uses the unbounded `parse`/`from_reader` when a `parse_with_limit` or depth-bounded variant exists, or applies a size limit but no depth limit). An inconsistent guard is strong evidence the omission is a real bug, not intentional.
+6. Confirm reachability: can attacker-controlled input actually reach this sink in production (uploaded file, request body, parsed field)? Note when the crash is an uncatchable abort (stack overflow / allocator abort) that takes down a shared multi-tenant process and crash-loops on a retried poison input.
+
+## One finding per root cause
+
+Report each distinct unbounded SINK once. If the same sink is reachable through several formats, call sites, or entry points, report it a single time and list the reachable paths in `verification` — do not emit one finding per format or per caller. Two genuinely independent sinks (different functions, each needing its own fix) are two findings.
+
+## Severity
+
+| Level | Use for |
+|-------|---------|
+| high | A single crafted input causes a persistent or unrecoverable outage (permanently wedged limiter, crash-loop on a retried poison input with no self-heal), an uncatchable process-wide abort on a shared multi-tenant process, OR impact on a core pipeline (ingestion, the request path serving all tenants). |
+| medium | A single crafted input crashes or hangs a recoverable shared worker that restarts on its own and affects only a subset of traffic (e.g. an enrichment/symbolication worker), with affected work re-processable. |
+| low | Real but bounded: impact is self-limiting, needs unrealistic input size, or requires preconditions not supported by the code. |
+
+Use the lower level when reachability or impact depends on unproven preconditions.
+
+## What not to report
+
+- Sinks that already have an effective bound (a cap, `.take`, depth limit, or visited-set on the reachable path).
+- Bounded input whose worst case is small and recoverable.
+- Ordinary performance, allocation-count, or style concerns with no attacker-driven unbounded path.
+- AppSec exploitability (injection, XSS, SSRF, auth) — that is the `security-review` skill's job; report it there.
+- Pattern-only suspicion. No proof of an attacker-controlled unbounded path, no finding.
+
+## Finding format
+
+- Title: name the sink and the missing bound (e.g. "chained_info follow loop has no cycle guard").
+- Description: one or two sentences — the attacker-controlled source, the unbounded sink, and the impact.
+- `verification`: 2–5 short bullets tracing source → sink with concrete file:line facts: where the input field is read, the sink call, the absence of any cap, and any sibling that DOES cap the same thing. List additional reachable paths here rather than as separate findings.

packages/warden/src/builtin-skills/dos-review/SPEC.md

@@ -0,0 +1,75 @@
+# DoS Review Skill Specification
+
+## Intent
+
+The `dos-review` skill is Warden's availability / denial-of-service reviewer. It owns the resource-exhaustion class that the `security-review` (AppSec) and `code-review` (correctness-in-changes) skills leave uncovered: a single small crafted input that drives unbounded resource use, a hang, or a process-killing abort.
+
+It targets code that parses or processes untrusted, attacker-controlled bytes, and it audits the whole file for latent sinks rather than only diff-scoped regressions.
+
+## Scope
+
+In scope:
+
+- Unbounded memory allocation pre-sized from an attacker-controlled length/size/count field.
+- Unbounded decompression (compression bombs) with no output cap.
+- Uncontrolled recursion (no depth limit or visited-set) reaching a stack-overflow abort.
+- Non-terminating loops following input-controlled pointer/offset chains with no cycle guard.
+- Unbounded delegation of untrusted input to third-party parsers/decoders/decompressors that apply no caller-side depth/size/time bound (the unbounded sink lives inside the dependency).
+- Resource leaks and panics on untrusted input (missing RAII/finally release, `.unwrap()`/index on input-derived values).
+- General guidance across native (Rust/C/C++/Go), JVM, and service code that handles untrusted bytes.
+
+Out of scope:
+
+- AppSec exploitability (injection, XSS, SSRF, auth, secrets) — routed to `security-review`.
+- Non-security correctness bugs without a resource-exhaustion or crash impact — routed to `code-review`.
+- Performance tuning with no attacker-driven unbounded path.
+- Sinks that already have an effective bound on the reachable path.
+
+## Users And Trigger Context
+
+- Primary users: coding agents and Warden runs auditing parsers, format/codec decoders, deserialization, and request-handling code.
+- Should trigger for: "DoS review", "resource exhaustion", "decompression bomb", "compression bomb", "uncontrolled recursion", "stack overflow on input", "unbounded allocation", "denial of service", "crash safety", "memory-safety review of untrusted input".
+- Should not trigger for: AppSec/OWASP review, generic correctness review, style/architecture review, or Warden CLI usage.
+
+## Runtime Contract
+
+- Required first actions:
+  - Identify the untrusted source fields (length/size/count/depth/offset) in the target file.
+  - Trace each value to its sink across functions, files, and crates; read the surrounding guards and any sibling path that caps the same primitive.
+- Required finding evidence:
+  - attacker-controlled source field
+  - unbounded sink or missing/after-the-fact bound
+  - reachability of the sink from untrusted input
+  - impact (allocator/stack abort, hang, OOM, disk fill, or persistent limiter leak), noting whether the crash is an uncatchable process-wide abort
+- Required outputs:
+  - One finding per distinct root-cause sink; additional reachable paths listed in `verification`, not as separate findings.
+  - Severity calibrated: persistent/unrecoverable or core-pipeline = high; recoverable shared worker = medium; bounded = low.
+  - Empty findings when no attacker-controlled unbounded path is proven.
+- Non-negotiable constraints:
+  - Do not report sinks that already have an effective bound.
+  - Do not emit one finding per format/call site for the same sink.
+  - Do not report AppSec issues here.
+
+## Reference Architecture
+
+- `SKILL.md` contains the review contract, sink-class table, investigation method, dedup rule, severity rubric, and exclusions.
+- Add `references/<language>.md` only when recurring findings need language-specific calibration (e.g. Rust allocator-abort vs panic semantics, Go slice make, JVM array pre-sizing).
+
+## Evaluation
+
+- Lightweight validation:
+  - Run the skill validator against `src/builtin-skills/dos-review`.
+  - Run init command tests that install bundled skills.
+- Deeper evaluation:
+  - Eval cases per class: unbounded `with_capacity`, decompression bomb, uncontrolled recursion, self-referential pointer-chain loop, RAII/limiter leak on panic, plus safe counterexamples that already carry a cap, depth limit, visited-set, or guard.
+  - Confirm dedup: one sink reachable via N formats yields one finding.
+- Acceptance gates:
+  - `SKILL.md` stays concise.
+  - Findings require a proven attacker-controlled unbounded path, not keyword matches.
+  - Severity matches the recoverable-vs-persistent rubric.
+
+## Maintenance Notes
+
+- Add a language reference only when recurring findings need language-specific calibration.
+- Keep examples minimal and transformed; do not store proprietary code.
+- Origin: restores the DoS/resource-exhaustion coverage dropped when the `find-bugs` skill was removed (warden #195); that class was not migrated into `security-review` (AppSec-only) or `code-review` (correctness-in-changes), leaving it unowned.

packages/warden/src/builtin-skills/security-review/SPEC.md

@@ -20,6 +20,7 @@ Out of scope:
 - Full dependency CVE triage without reachable changed code.
 - Compliance checklists, infrastructure-only policy review, or exhaustive cloud IAM audits.
 - Style, maintainability, performance, or non-security correctness bugs.
+- Availability / denial-of-service (unbounded allocation, decompression bombs, uncontrolled recursion, resource exhaustion) — routed to the `dos-review` skill.
 - Benchmark-specific prompt compatibility.
 - Large language-specific catalogs in `SKILL.md`.
 

packages/warden/src/cli/commands/init.ts

@@ -425,6 +425,7 @@ export async function runInit(options: CLIOptions, reporter: Reporter): Promise<
   reporter.bold('Next steps:');
   reporter.text(`  1. Add built-in reviews: ${chalk.cyan('warden add security-review')}`);
   reporter.text(`     ${chalk.cyan('warden add code-review')}`);
+  reporter.text(`     ${chalk.cyan('warden add dos-review')}`);
   reporter.text(`  2. Set ${chalk.cyan('WARDEN_MODEL')} and the WARDEN-prefixed provider API key for that model`);
   reporter.text(`  3. Add the same values to organization or repository secrets`);
 

packages/warden/src/skills/loader.ts

@@ -58,6 +58,7 @@ export const BUILTIN_SKILL_DIRECTORIES = [
 export const BUILTIN_SKILL_NAMES = [
   'code-review',
   'security-review',
+  'dos-review',
 ] as const;
 
 const BUILTIN_SKILL_NAME_SET = new Set<string>(BUILTIN_SKILL_NAMES);