# Review code for security issues

`CodeReviewPipeline` in `tnsai-quality` reviews a source tree for vulnerabilities. Regex matchers pick out files that look risky, an LLM agent reviews each of those files, and an optional second agent cross-checks the results. The findings come out as SARIF, JSON or Markdown.

The pipeline reads files and sends them to the model. It never runs the code it reviews.

## Run a review

<!-- java-contract: src/main/java/com/example/tnsai/docs/SecurityCodeReviewPipelineExample.java -->
```java
static List<StageResult> review(Path repo, LLMClient reviewer, LLMClient crossChecker) {
    CodeReviewPipeline.Builder builder = CodeReviewPipeline.builder()
        .projectRoot(repo)
        .stateStore(new FileSystemPipelineStateStore(Path.of(".tnsai/security")))
        .agent(ReviewAgent.claudeOpus(reviewer))       // the process stage uses the first agent
        .agent(ReviewAgent.gpt(crossChecker))          // the revalidate stage uses the second
        .config("process.maxFiles", "50")
        .stage(new ScanStage())
        .stage(new ProcessStage())
        .stage(new RevalidateStage())
        .stage(new EnrichStage())
        .stage(new ExportStage(ExportFormat.SARIF))
        .stage(new ExportStage(ExportFormat.MARKDOWN));
    BuiltInMatchers.all().forEach(builder::matcher);

    return builder.build().run();                      // stages run in the order you added them
}
```

`projectRoot` and `stateStore` are required, or `build()` throws `IllegalStateException`. `projectId` defaults to the name of the root directory. The builder registers no stages and no matchers by default, so add every one you want. `run()` runs the stages in the order you added them and returns one `StageResult` per stage; `execute("scan")` runs a single stage by id.

The state store keeps everything under `<baseDir>/<projectId>/`:

```text
project.json                 project id, root and creation time
files/<path-hash>.json       one FileRecord per flagged file: matchers, findings, agent runs
runs/<epoch-ms>-<stage>.json one entry per stage run, with its StageResult
reports/findings.sarif       written by the export stages (also findings.json, findings.md)
```

## Stages

| Stage | Id | Does | Skips |
|-------|----|------|-------|
| `ScanStage` | `scan` | Runs every matcher over the source files and stores a record for each file that at least one matcher flags | Files whose content and flagging matchers are unchanged |
| `ProcessStage` | `process` | Sends each flagged file to the first agent and stores its findings | Files the same agent and model already reviewed successfully at the same content |
| `RevalidateStage` | `revalidate` | Sends each file that has findings to the second agent and reconciles the two results | The same, for the second agent; with fewer than two agents the stage does nothing |
| `EnrichStage` | `enrich` | Adds the CWE name and link to each finding whose CWE is one of the ten built-in ones | Findings that already have them |
| `ExportStage(format)` | `export-sarif`, `export-json`, `export-markdown` | Writes every stored finding to a report | Nothing; it rewrites the report each time |

Changing a file's content replaces its record, so its old findings and agent runs are dropped and the file is reviewed again. A file that no longer trips any matcher keeps its old record and findings. To start from a clean slate, delete the project's state directory.

`ProcessStage` throws `IllegalStateException` when no agent is registered. An agent call that throws is stored as a failed run, and the file is retried on the next run.

## Built-in matchers

A matcher only decides which files the agent sees. Its hits do not become findings; the agent receives the ids of the matchers that fired as hints. Every finding comes from an agent, with the severity the model gives it.

| Factory | Id | CWE | Severity | Files |
|---------|----|-----|----------|-------|
| `sqlInjection()` | `sql-injection` | CWE-89 | `HIGH` | Java, Python, JavaScript, TypeScript |
| `pathTraversal()` | `path-traversal` | CWE-22 | `HIGH` | Java, Python, JavaScript, TypeScript |
| `commandInjection()` | `command-injection` | CWE-78 | `CRITICAL` | Java, Python, JavaScript, TypeScript |
| `hardcodedSecrets()` | `hardcoded-secrets` | CWE-798 | `CRITICAL` | Every scanned file |
| `weakCrypto()` | `weak-crypto` | CWE-327 | `MEDIUM` | Java, Python, JavaScript, TypeScript |
| `crossSiteScripting()` | `cross-site-scripting` | CWE-79 | `HIGH` | JavaScript, TypeScript |
| `unsafeDeserialization()` | `unsafe-deserialization` | CWE-502 | `CRITICAL` | Java, Python, JavaScript, TypeScript |
| `serverSideRequestForgery()` | `ssrf` | CWE-918 | `HIGH` | Java, Python, JavaScript, TypeScript |
| `xmlExternalEntity()` | `xxe` | CWE-611 | `HIGH` | Java, Python, JavaScript, TypeScript |
| `evalOnInput()` | `eval-on-input` | CWE-95 | `CRITICAL` | Java, Python, JavaScript, TypeScript |

`BuiltInMatchers.all()` returns all ten. JavaScript and TypeScript cover `.js`, `.jsx`, `.mjs`, `.cjs`, `.ts` and `.tsx`.

### Add your own matcher

<!-- java-contract: src/main/java/com/example/tnsai/docs/SecurityCodeReviewMatcherExample.java -->
```java
static MatcherSpi logInjection() {
    return new RegexMatcher(
        "log-injection", "CWE-117", Severity.MEDIUM,
        "User input written to a log without stripping line breaks.",
        List.of(Pattern.compile("log\\.(?:info|warn|error)\\s*\\(\\s*\\w+\\s*\\+")),
        Set.of(".java"));                              // an empty set scans every file
}
```

`RegexMatcher` reports each match of each pattern with its line range. For anything other than regex, implement `MatcherSpi`: `id`, `cwe`, `defaultSeverity`, `description` and `scan(file, content)`, which returns `MatcherSpi.Hit` values. `EnrichStage` only knows the ten built-in CWEs, so findings for other CWEs get no name or link.

## Choose the files

`ScanStage` reads these settings from `config(key, value)`:

| Key | Default |
|-----|---------|
| `scan.includeGlobs` | Java, Python, JavaScript, TypeScript, Go, Ruby, PHP, C, C++, C#, Kotlin, Rust, YAML, JSON and XML files |
| `scan.excludeGlobs` | `node_modules`, `.git`, `target`, `build`, `dist`, `.tnsai`, `.deepsec` and `__pycache__` directories, and `*.min.js` |

Each value is a comma-separated list that replaces the default. The `glob:` prefix is optional. Globs are matched against absolute paths, so start them with `**/`. Files over 1 MiB are skipped, and symbolic links are followed.

## Limit the model calls

| Key | Effect |
|-----|--------|
| `process.maxFiles` | Stops `ProcessStage` after this many files |
| `process.maxFindings` | Stops `ProcessStage` once this many findings have been added; the check runs before each file, so the last file can push the total past the limit |

Both default to no limit. Files skipped as already reviewed do not count. Neither key applies to `RevalidateStage`.

## Use a review agent

`ReviewAgent` wraps any `LLMClient`. It sends the file path, the matchers that fired and the file content, and asks for a JSON array of findings.

| Factory | Agent id | Version |
|---------|----------|---------|
| `ReviewAgent.claudeOpus(llm)` | `claude-opus` | `llm.getModel()` |
| `ReviewAgent.gpt(llm)` | `gpt` | `llm.getModel()` |
| `new ReviewAgent(id, version, llm)` | your id | your version |

The factories only set the id; the model is whatever the client uses.

- A reply wrapped in a code fence is unwrapped first.
- A reply that is not a JSON array produces no findings and is logged. The run still counts as successful, so the file is not reviewed again until its content changes.
- A finding without an id gets `<agent>-<cwe>-<startLine>`. A missing severity becomes `INFO`, and an unknown one drops that finding.

To use something other than an LLM, implement `ReviewAgentSpi`: `id`, `version` and `runOnFile(file, context)`.

### Cross-check with a second agent

`RevalidateStage` asks the second agent to review the same file and then:

- drops every finding from the first agent if the second agent finds nothing;
- otherwise keeps a first-agent finding only if the second agent reported the same CWE, at any line;
- adds the second agent's findings whose ids are not already kept.

## Read the reports

`findings.json` is the list of `ReviewFinding` records. `findings.md` groups the findings by file. `findings.sarif` is SARIF 2.1.0 with one rule per CWE:

| Finding | SARIF |
|---------|-------|
| `cwe` | `ruleId`; the rule gets the CWE name and link after `EnrichStage` |
| `severity` | `level`: `INFO` is `note`, `LOW` and `MEDIUM` are `warning`, `HIGH` and `CRITICAL` are `error` |
| `title` | `message.text` |
| `file`, `startLine`, `endLine` | `physicalLocation` |
| `evidence`, `mitigation`, `agentBackend`, `matcher` | `properties` |

Run `EnrichStage` before the export stages so the rules carry CWE names. To read findings in code, use `stateStore.listFiles()` and each record's `findings()`.

## Related

- [Run code in a sandbox](sandbox.md)
- [Detect prompt injection](prompt-injection.md)
- [Redact sensitive data](redaction.md)
- [Audit agent actions](accountability.md)
