Skip to content

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

Open
oioki wants to merge 1 commit into
mainfrom
feat/dos-review-skill
Open

feat(skills): Add dos-review skill for availability/DoS findings#415
oioki wants to merge 1 commit into
mainfrom
feat/dos-review-skill

Conversation

@oioki

@oioki oioki commented Jun 30, 2026

Copy link
Copy Markdown
Member

Add a built-in dos-review skill for the availability / denial-of-service (resource-exhaustion) bug class — unbounded allocation, decompression bombs, uncontrolled recursion, non-terminating loops, panic/resource-leak, and unbounded delegation to third-party parsers.

Why

This class is currently unowned. When the find-bugs skill was removed in #195 its DoS checklist wasn't migrated: security-review is scoped to AppSec exploitability (its SPEC lists performance / non-security-correctness as out of scope), and code-review only reaches the allocation subset and gates findings on changed code. A decompression-bomb / unbounded-recursion DoS is both an availability-security issue and a resource-exhaustion correctness issue, so each existing skill's scope boundary pushes it to the other — and neither reports it.

What

  • New src/builtin-skills/dos-review/{SKILL.md,SPEC.md} — sink-class table (alloc / decompression / recursion / non-termination / resource-leak / unbounded delegation), an investigation method that traces untrusted input to its sink across files and crates (including reading dependency source), a one-finding-per-root-cause dedup rule, and a severity rubric calibrated to recoverable-vs-persistent impact. Framed for whole-file / latent-bug audit rather than diff-regression, since these are usually pre-existing sinks.
  • Register dos-review in BUILTIN_SKILL_NAMES (src/skills/loader.ts) and surface it in warden init next steps.
  • One-line pointer from security-review's SPEC routing DoS / resource-exhaustion to dos-review (mirrors how code-review routes AppSec to security-review), so the class is discoverable from the skill teams already run.

Notes for review

  • Kept as a separate skill rather than folded into security-review: the two have opposite precision/severity calibration (exploitability-gated high-confidence vs high-recall availability), and per-skill focus is why find-bugs was split in ref: Migrate to dotagents and add skills reference to AGENTS.md #195 — merging would re-create the orphan. The cross-pointer gives discoverability without the dilution.
  • Validated internally against a corpus of known availability findings; detailed evaluation/evidence is held by the security team.

Add a built-in `dos-review` skill covering the availability /
resource-exhaustion (DoS) bug class: unbounded allocation, decompression
bombs, uncontrolled recursion, non-terminating loops, panic/resource-leak,
and unbounded delegation to third-party parsers.

This class was left unowned when the find-bugs skill was removed in #195:
security-review is scoped to AppSec exploitability (DoS explicitly out of
scope) and code-review only reaches the allocation subset and gates on
changed code. dos-review owns the class and is framed for whole-file /
latent-bug audit rather than diff-regression.

Register the skill in BUILTIN_SKILL_NAMES (loader.ts), surface it in the
`warden init` next steps, and add a one-line pointer from security-review's
SPEC routing DoS findings to dos-review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9104839. Configure here.

export const BUILTIN_SKILL_NAMES = [
'code-review',
'security-review',
'dos-review',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale built-in skill names test

Medium Severity

Adding dos-review to BUILTIN_SKILL_NAMES without updating the test that asserts the full built-in name list leaves loader.test.ts expecting only code-review and security-review, so the suite fails even though registration is correct.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9104839. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant