The skill — skills/appsec/appsec-vuln-audit/SKILL.md

name: appsec-vuln-audit
description: >-
  Audit code for vulnerabilities against a threat model, then deduplicate,
  verify in context, and filter to what is actually reachable. Use when asked
  to review code for security bugs, run or interpret SAST, check whether a
  finding is a false positive, or reduce a noisy findings list to the ones
  worth a human's time.
allowed-tools: Read, Grep, Glob, Bash

AppSec pipeline · Phase 3 — Analysis and filtering

Covers stages 7–10. This is where findings are produced — and, more importantly, where most of them are thrown away.

The hard problem in application security is not finding candidate defects. It is that a scanner emits hundreds and a human can act on ten. Every stage after 7 exists to shrink the list without losing the true positives.

When to use this

When you have a threat model and a budget, or when handed a raw findings file that nobody trusts. Stages 8–10 work on any findings list, including one from a third-party scanner.

Inputs

Procedure

Stage 7 — Vulnerability auditing. For each selected threat, examine the path from entry to sink and decide whether the weakness is actually present. Record for each finding: cwe, file, line, unit, the evidence (the specific expression that is unsafe), and the sanitiser you looked for and did not find. A finding that cannot name what was missing is a guess.

Three generations of analysis, and they are complementary, not competing: grep-class pattern matching (fast, no dataflow), taint analysis (dataflow, no semantics), and model-assisted review (semantics, no guarantees). Use the cheapest one that can answer the question, and never let the third overrule the second on a question of reachability — the model does not execute the program.

Stage 8 — Deduplication. The same defect appears many times: once per scanner, once per path, once per call site. Collapse on the defect identity — (cwe, file, unit, sink_expression) — not on the message text. Keep the count: occurrences is signal about how exposed the defect is.

Match paths by parent directory plus filename tail. Deduplicating on a bare basename silently merges two different files and loses a real finding.

Stage 9 — Contextual verification. For each surviving finding, look at the surrounding code for the thing that makes it not-a-bug: a validator upstream, a framework escaping the parameter, a type that cannot hold the payload, a caller that only ever passes a constant. Record the verdict and the reason: confirmed, mitigated_by <what>, or needs_human.

needs_human is a legitimate verdict and must stay available. A pipeline that must decide will decide wrongly under uncertainty.

Stage 10 — Feasibility filtering. Drop what an attacker cannot actually reach: code behind a feature flag that is off, an admin-only path in a service with no admin, a sink whose input is fully constant. Record why each drop was made, because the next scan will rediscover it and the reason is what stops that work being repeated.

Example

Input — the fixture committed at the top of scripts/appsec_vuln_audit.py. Edit it and re-run: the buckets, counts and verdicts below are derived from it, not hard-coded.

Output — the opening lines of a real run:

call graph:
   http_get_report   → ['load_report']
   http_health       → —
   load_report       → —
   legacy_export     → —
   debug_dump        → —
   dispatch          → —

The run continues past this. test_skills.py executes the script on every build, with no model configured — so what CI proves is that it runs and refuses legibly, not that it prints these lines. The output above is a recorded run against a real model, and nothing re-diffs it: a model's answer is not reproducible, and this repository does not claim otherwise anywhere it can be checked.

Output contract

{
  "findings": [
    {"id": "str", "cwe": "CWE-89", "file": "str", "line": 0, "unit": "str",
     "evidence": "str", "missing_control": "str",
     "occurrences": 1, "verdict": "confirmed|mitigated|needs_human",
     "verdict_reason": "str", "feasible": true, "confidence": 0.0}
  ],
  "dropped": [{"id": "str", "stage": 8, "why": "str"}],
  "counts": {"raw": 0, "deduped": 0, "verified": 0, "feasible": 0}
}

counts must be monotonically non-increasing across the four stages. If it is not, the pipeline invented findings after the audit stage — stop and report.

Failure modes

• Confusing conformance with accuracy. Output that matches this schema perfectly can still be entirely wrong. Schema validity is close to free; correctness is the expensive part. Never report conformance as a quality metric.
• Dropping silently. Every drop needs a stage and a reason.
• Letting a model overrule dataflow on reachability. It may propose a path; it may not confirm one.
• Suppressing needs_human to look decisive. Uncertainty that is hidden becomes someone's incident.

Handoff

Feasible, confirmed findings go to appsec-exploit-validate for proof. Everything else goes to appsec-triage-report with its verdict intact.

The skill — skills/appsec/dead-code-ast-reachability/SKILL.md

name: dead-code-ast-reachability
description: >-
  Build a call graph by parsing source to an abstract syntax tree, and use it to
  separate findings that are false positives about risk from findings that are
  false positives about code. Use when a queue is full of unreachable findings,
  when deciding whether a function is dead, or when a scanner reports a defect
  in code nothing calls.
allowed-tools: Read, Grep, Glob

Dead code is a true positive about the code and a false positive about the risk

Both halves matter and teams act on only one.

The finding is correct: the concatenation is there, the sink is real, and a reviewer who opens the file will agree. What is wrong is the implied consequence, because nothing untrusted reaches it. Triaging it costs exactly as much as triaging one on the login path, and there are usually far more of them — so a queue that does not separate the two is a queue engineers learn to ignore, which costs you the reachable ones as well.

Deciding which is which needs a call graph, and a call graph needs the abstract syntax tree. Grep cannot do it: def report and report( and # report are the same string to a regex, and a function named run appears in every file in the repository. Parsing to an AST gives you the two node types the question actually needs — FunctionDef for what exists, Call for what invokes it — with their real nesting, so "which functions call this one" stops being a text search and becomes a graph walk.

The AST is also honest about its own limits, which is the more useful half. It resolves a literal call and it cannot resolve getattr(mod, name)(), a dispatch dictionary, or a handler a framework wires by decorator at import time. Those are not unreachable. They are undecided, and filing them as unreachable is how a pipeline drops real bugs quietly.

When to use this

When reachability analysis produces a large unreachable bucket, before any bulk suppression, and during any dead-code or deprecation sweep — the security queue is the cheapest available list of which dead code to delete first. Also whenever a finding is about to be dismissed as "that code isn't called": this is how that sentence gets evidence behind it.

Step-by-step

1 — Parse, do not grep. ast.parse(source) per file. Collect every FunctionDef and AsyncFunctionDef as a node; collect every Call as an edge from its enclosing function.

2 — Resolve each call to a name. Call.func is a Name for f() and an Attribute for obj.f(). Take .id or .attr. This is deliberately naive about which f is meant — two functions of the same name in different modules merge — and it errs toward reachable, which is the safe direction.

3 — Mark entry points. Anything decorated as a route or handler, anything exported, anything a scheduler names. An entry point is reachable by definition, and getting this list wrong is the largest source of error in the whole procedure.

4 — Walk from the entry points. Everything reached is reachable. Everything not reached is either dead or undecided, and step 5 decides.

5 — Separate unreachable from unknown, never collapse them. If the file contains a dynamic call the AST could not resolve — getattr, a dispatch table, a decorator that registers — every unreached function in that module is unknown, not unreachable. Report the three buckets separately.

6 — Only then classify the findings. A finding on a reachable unit keeps its severity. One on a genuinely dead unit is a false positive about risk and a true positive about code, and belongs in a deletion list rather than a triage queue. One on an unknown unit is unresolved work, not a clean result.

Example

Input — two files, one entry point, one dynamic call:

# api.py
@route("/reports")            # entry point
def list_reports(request):
    return render(load(request.args["id"]))

def load(rid): ...            # called by list_reports
def legacy_export(path): ...  # called by nothing

# jobs.py
def dispatch(name):
    return getattr(handlers, name)()   # unresolvable by AST
def nightly(): ...

Output:

reachable    list_reports, load           reached from an entry point
unreachable  legacy_export                no caller, no dynamic call in api.py
unknown      dispatch, nightly            jobs.py contains getattr(...)()

nightly is not dead. It is in a module the AST could not fully resolve, so every unreached function in that module is undecided — and saying so is the whole difference between this procedure and a shorter one.

Output contract

{
  "graph": {"functions": 0, "edges": 0, "entry_points": ["str"]},
  "buckets": {"reachable": 0, "unreachable": 0, "unknown": 0},
  "unresolved_calls": [{"file": "str", "why": "getattr|dispatch-table|decorator"}],
  "findings": [{"id": "str", "unit": "str", "bucket": "str",
                "true_about_code": true, "true_about_risk": false}],
  "queue": {"before": 0, "after": 0}
}

unresolved_calls is what makes the unknown bucket auditable. A report with an empty unknown bucket and no unresolved_calls entry has either a very simple codebase or a bug.

Common edge cases

• Two functions with the same name. The naive resolver merges them and over-reports reachability. That is the safe direction; the unsafe one is a clever resolver that guesses wrong and marks a live function dead.
• A decorator that registers the function. @app.route, @celery.task, @click.command. The function has no caller in the source and is reachable from outside it. Treat every decorated function as an entry point unless you know the decorator.
• Tests as callers. A function called only from tests is dead in production. Exclude test files from the graph or you will never find any.
• A method called through an instance. Attribute gives you .attr, which is the method name without the class. Same over-reporting, same safe direction.
• __init__.py re-exports. A function imported and re-exported has no call edge at all and is reachable by import. Follow ImportFrom or accept it as unknown.

Failure modes

• Grepping for the name instead of parsing. Comments, strings and unrelated functions with the same name all match, and the answer is unusable in either direction.
• Collapsing unknown into unreachable. It makes the queue shorter and it is how framework-wired handlers get dropped.
• Trusting the entry-point list. Every function is unreachable if you forgot the routes file.
• Deleting on the graph's word alone. Reachability from untrusted entry points is not reachability from anywhere; check both before a deletion.