Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .claude/knowledge/testing-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,8 @@ Testing strategies, test infrastructure quirks, how to run/debug specific test s
- All tests live under `tests/` as external (`_test`) packages; there are no white-box tests inside `internal/`/`pkg/`. Internal packages are still importable from `tests/` because they are within the module. This is enforced by `ci.test-file-location` (the repo's own scan fails on `_test.go` files outside `tests/**`), so do not add tests next to the code even for unexported access — import the internal package from `tests/` and test via its exported API instead.
- **Never commit a contiguous real-format secret as a test fixture.** GitHub push protection rejects the push (it flags GitLab/Stripe/SendGrid/Twilio/etc. tokens even in `_test.go` files), and it is poor practice in a secret-detection repo. Assemble fixtures at runtime from a prefix + body via the `cred(prefix, body)` helper in `tests/checks/test_helpers_test.go` (e.g. `cred("AKIA", "1234567890ABCDEF")`), splitting the recognizable provider prefix from the rest so no full token literal appears in source. The reconstructed value still exercises the scanner identically. `goConst(value)` wraps a value as a minimal Go source fixture.
- The MCP server smoke tests (`tests/mcp/`) drive the **real binary in a subprocess**, not the in-process handler, to keep tests external. Pattern: a `Test...HelperProcess` func gated by an env var (`GO_WANT_MCP_HELPER_PROCESS` for stdio, `GO_WANT_MCP_HTTP_HELPER_PROCESS` for HTTP) re-execs `os.Args[0]` and calls `cli.Run(...)`. stdio replays NDJSON transcripts over stdin; HTTP reserves a free `127.0.0.1:0` port, launches `serve --mcp --http`, polls `/healthz` for readiness, then issues real HTTP requests. Add new MCP behavior as a transcript + assertion (stdio) or a subtest in `http_test.go` (HTTP).
- **Detector precision corpus** (`tests/corpus/`): fixtures live under `testdata/<language-group>/<rule>/{vulnerable,clean}/` with ground truth in `expectations.yaml`. `known_gaps` entries assert a documented FP/FN *still exists* — when you improve a detector the corpus test fails on purpose with a "promote it to must_fire / remove the known_gaps entry" message; update the manifest in the same change. Fixture credentials must be synthetic but pattern-shaped (see the secret-fixture rule above).
- **Every catalog rule must ship a `FixTemplate`** with a valid `Kind` (`deterministic`|`guided`) — `TestSDKRuleMetadataFixTemplatesPopulated` iterates the full catalog and fails on any rule without one. When adding a rule, add its template to the family map in `internal/codeguard/rules/catalog_fix_templates_*.go` (or inline in the catalog entry; inline wins over the registry).
- `gofmt -l .` at repo root is polluted by `.claude/worktrees/` (live agent worktrees) and `.gomodcache/`; scope it to `gofmt -l cmd internal pkg tests changelog.go` or use `make fmt-check`.
- **Bidirectional (server→client) MCP tests** live in `tests/mcp/sampling_test.go`: the test acts as the MCP client, advertises `sampling`/`roots` at `initialize`, and answers the server's server-initiated requests. stdio uses interactive `StdinPipe`/`StdoutPipe` (not the replay harness). HTTP opens the `GET /mcp` SSE stream (waits for the `: ready` comment to avoid the attach race), reads the request off the stream, and POSTs the response with the matching `Mcp-Session-Id`. propose_fix verification is expected to fail on the throwaway diff — assert the round trip fired, not a verified patch. The HTTP helper passes `-config` via `CODEGUARD_TEST_HTTP_CONFIG`.
- **TS tests can be hijacked by the Node semantic engine**: on hosts with a discoverable `typescript.js` (e.g. VS Code installed), TypeScript targets route through the Node runner instead of the per-file Go path. Tests that must exercise the per-file path (tree-sitter differential tests, corpus TS groups) set `CODEGUARD_TYPESCRIPT_LIB_PATH` to an existing-but-invalid lib to force the fallback.
13 changes: 13 additions & 0 deletions .codeguard/codeguard.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ exclude:
- .codeguard/cache.slop-history.json
- .gomodcache/**
- tests/**/.codeguard/cache.json
# Detection-precision fixtures: intentionally vulnerable-looking sample
# files asserted by tests/corpus/corpus_test.go; keep them out of the
# repository self-scan.
- tests/corpus/testdata/**
waivers:
- rule: quality.max-file-lines
path: internal/codeguard/rules/catalog_quality.go
Expand All @@ -18,6 +22,15 @@ waivers:
- rule: quality.unbounded-goroutines-in-loop
path: internal/codeguard/runner/checks/checks.go
reason: section workers are bounded by a NumCPU-sized semaphore before each goroutine is launched
- rule: ci.test-file-location
path: internal/codeguard/checks/support/treesitter/*_test.go
reason: the tree-sitter design spike is an isolated Go module whose differential tests must live beside the prototype they validate (docs/treesitter-spike.md)
- rule: supply_chain.lockfile-drift
path: internal/codeguard/checks/support/treesitter/go.mod
reason: the spike module resolves the root module through a local replace directive, which never records a go.sum entry
- rule: quality.ai.hallucinated-import
path: internal/codeguard/checks/support/treesitter/*.go
reason: the spike directory is a nested Go module with its own go.mod; its imports resolve there, not against the root module
targets:
- name: repository
path: ..
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ go.work.sum
.idea/
.vscode/
**/.codeguard/cache.slop-history.json
**/.codeguard/cache.rule-stats-history.json
5 changes: 5 additions & 0 deletions .goreleaser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ builds:
goarch:
- amd64
- arm64
# Embed only the grammars codeguard parses (TS/TSX/JS) instead of the
# full ~206-grammar registry: ~+4 MB binary delta instead of ~+22 MB.
# See docs/treesitter-spike.md §5.3.
flags:
- -tags=grammar_subset,grammar_subset_typescript,grammar_subset_tsx,grammar_subset_javascript
ldflags:
- -s -w -X github.com/devr-tools/codeguard/internal/version.Number=v{{ .Version }}

Expand Down
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ GOFILES := $(shell find cmd internal pkg tests -type f -name '*.go' 2>/dev/null)
MANIFEST_VERSION := $(shell grep -oE '[0-9]+\.[0-9]+\.[0-9]+' .release-please-manifest.json 2>/dev/null | head -1)
MENU_VERSION ?= $(if $(MANIFEST_VERSION),$(MANIFEST_VERSION),$(VERSION))
MENU_LDFLAGS := -X github.com/devr-tools/codeguard/internal/version.Number=v$(MENU_VERSION)
# GRAMMAR_TAGS restricts the gotreesitter grammar registry to the languages
# codeguard parses (docs/treesitter-spike.md §5.3): the subset build embeds
# only the TypeScript/TSX/JavaScript grammar blobs (~+4 MB) instead of all
# ~206 (~+22 MB). Builds without these tags (plain `go build`, `go install`)
# still work; they just embed every grammar.
GRAMMAR_TAGS := grammar_subset,grammar_subset_typescript,grammar_subset_tsx,grammar_subset_javascript

export GOCACHE
export GOMODCACHE
Expand Down Expand Up @@ -85,7 +91,7 @@ ci: check

build:
@mkdir -p dist
$(GO) build -trimpath -o $(CODEGUARD_BIN) ./cmd/codeguard
$(GO) build -trimpath -tags $(GRAMMAR_TAGS) -o $(CODEGUARD_BIN) ./cmd/codeguard

release: release-snapshot

Expand Down
86 changes: 85 additions & 1 deletion docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ This file documents the current check categories in `codeguard` and the config k
"security": true,
"prompts": true,
"ci": true,
"supply_chain": false
"supply_chain": false,
"context": true
}
}
```

Each top-level boolean enables or disables an entire check family.

`context` covers agent-context legibility: when the key is omitted the family defaults to enabled in full scans and disabled in diff scans; see [Agent Context](#agent-context).

`supply_chain` is opt-in and currently covers normalized manifest parsing plus initial policy checks for missing lockfiles, content-based lockfile drift validation, unpinned dependencies, and dependency license policy resolved from local manifest and installed metadata where available.

For ecosystems where local metadata is not present, `supply_chain_rules.license_commands` can provide an opt-in per-ecosystem command that prints JSON license mappings for unresolved dependencies.
Expand Down Expand Up @@ -146,6 +149,16 @@ codeguard baseline -config codeguard.yaml -output codeguard-baseline.json

Current behavior:
- baseline fingerprints are filtered before section status is computed
- each entry stores two fingerprints: the legacy line-based one
(`fingerprint`) and a context fingerprint (`context_fingerprint`) hashed
from the rule, path, and the whitespace-normalized source lines around the
finding (2 lines either side), so unrelated edits that only shift line
numbers do not break suppression
- a finding is suppressed when either fingerprint matches; two identical
findings in the same file (same rule and surrounding source) share a context
fingerprint, so baselining one also baselines its identical twins
- baseline files written before context fingerprints existed keep working:
their legacy fingerprints still match unchanged findings
- suppressed counts remain visible in the report summary

## Policy profiles
Expand Down Expand Up @@ -221,6 +234,21 @@ TypeScript semantic runtime:
- discovery order is `CODEGUARD_TYPESCRIPT_LIB_PATH`, then `node_modules/typescript/lib/typescript.js` from the target path upward, then the bundled VS Code TypeScript runtime
- if no runtime is available, codeguard falls back to the lightweight parser-based checks for TypeScript and JavaScript

Tree-sitter parsing (opt-in):
- `parsers.treesitter: "auto"` (default `"off"`) routes the lightweight
TypeScript/TSX/JavaScript checks through embedded tree-sitter grammars
instead of regexes for `quality.typescript.explicit-any`,
`quality.typescript.non-null-assertion`,
`quality.typescript.double-assertion`, and
`security.typescript.unsafe-html-sink` (plus their `*.javascript.*`
mirrors where the syntax exists in JavaScript)
- tree-based findings keep the same rule IDs, levels, and messages and set
`confidence: high`; they see through template-literal interpolations,
regex literals, JSX text, formatter-split expressions, and compound
assignments where the regex path cannot
- oversized files (> 256 KiB), parse failures, and error-heavy trees fall
back to the regex path per file

## Quality

Purpose:
Expand Down Expand Up @@ -685,6 +713,62 @@ Rules:
- `ci.always-true-test-assertion` warns when every assertion in a test only compares constants (`expect(true).toBe(true)`, `assert 1 == 1`, `require.True(t, true)`), so the test can never fail
- `ci.conditional-assertion` warns when every assertion in a test sits inside a conditional without an else branch, so the assertions may silently never run; idiomatic Go failure checks (`if got != want { t.Errorf(...) }`) are not flagged

## Agent Context

Purpose:
- Agent instruction file presence (CLAUDE.md, AGENTS.md, .cursorrules, .github/copilot-instructions.md)
- Drift between agent docs / README commands and the actual repository
- Agent context budget for individual source files
- Basename ambiguity that defeats filename-based navigation
- A `repo_legibility` artifact scoring how legible the repository is to AI agents

Config keys:

```json
{
"checks": {
"context": true,
"context_rules": {
"detect_missing_agent_docs": true,
"detect_agent_docs_drift": true,
"detect_readme_drift": true,
"detect_oversized_files": true,
"detect_ambiguous_symbols": true,
"max_file_lines": 1500,
"ambiguous_symbol_threshold": 4
}
}
}
```

When `checks.context` is omitted the family runs in full scans and is skipped in diff scans: its signature findings are repo-level (missing agent docs, duplicated basenames) and would repeat on every PR regardless of the change under review. Set `"context": true` to force it on in diff scans, or `false` to disable it entirely.

Current behavior:
- `context.agent-docs-missing` warns once at repo level when none of the recognized agent instruction files exist at the target root
- `context.agent-docs-drift` warns when an agent instruction file references a file or directory path, a `make` target, or an npm/pnpm/yarn `run` script that provably does not exist
- `context.readme-drift` applies the same resolution to fenced `bash`/`sh`/`shell` blocks in the root README.md: `./`-prefixed paths, make targets, and run scripts that resolve nowhere
- `context.oversized-context-unit` warns when a source file exceeds `context_rules.max_file_lines` (default 1500); the message is framed as agent context cost, distinct from `quality.max-file-lines` maintainability thresholds; generated and vendored files are skipped
- `context.ambiguous-symbol` warns once per source-file basename shared by at least `context_rules.ambiguous_symbol_threshold` files (default 4), listing up to five locations

Drift resolution is deliberately conservative — precision over recall. It only flags references it can positively prove broken, and skips:
- URLs, module/domain paths (`github.com/...`), absolute paths, and `..` traversals
- placeholders and expansions (`<name>`, `$VAR`), globs, and template syntax
- all fenced blocks except shell command fences (code samples and captured output are never treated as paths)
- shell blocks after a `cd`/`pushd` or a heredoc, and `make -C`/`-f` invocations that select another makefile
- make targets when no root Makefile exists or the Makefile uses `include` or pattern rules
- npm scripts when there is no root package.json or it declares workspaces

`repo_legibility` artifact:

Every context run publishes one `repo_legibility` artifact per target with a 0-100 score (higher is more legible) and an explainable component breakdown:
- `agent_docs` (25): any agent instruction file present
- `readme` (10): root README.md present
- `doc_accuracy` (20): minus 4 points per unresolvable doc/README reference
- `context_economy` (25): scaled down by the share of source files over the context budget (10% oversized zeroes it)
- `navigability` (20): scaled down by the share of source files caught in ambiguous basename groups (20% affected zeroes it)

The artifact is emitted even when individual rules are toggled off, so the score always reports reality.

## Output

Config keys:
Expand Down
34 changes: 34 additions & 0 deletions docs/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,42 @@ This page lists the current `codeguard` feature surface and the main config entr
- `quality.ai.change-risk`
- aggregates AI-quality and review-risk signals into a target-level artifact plus a `Code Quality` finding when thresholds are crossed

## Parsers

- `parsers.treesitter: "off" | "auto"` (default `"off"`) selects the parsing
substrate for TypeScript/TSX/JavaScript rules (`docs/treesitter-spike.md`).
- `"off"`: the regex-based scanners run exactly as before.
- `"auto"`: script files parse through embedded tree-sitter grammars; the
migrated rules (`quality.typescript.explicit-any`,
`quality.typescript.non-null-assertion`,
`quality.typescript.double-assertion`,
`security.typescript.unsafe-html-sink` and their `*.javascript.*`
mirrors) evaluate grammar queries instead of regexes and report
`confidence: high`. Files that exceed the 256 KiB parse cap, fail to
parse, or produce error-heavy trees fall back to the regex path per file,
so enabling the flag can never lose coverage.

## JSON/YAML config examples

### Tree-sitter parsing for TypeScript/JavaScript

YAML:

```yaml
parsers:
treesitter: auto
```

JSON:

```json
{
"parsers": {
"treesitter": "auto"
}
}
```

### Enable AI change risk

YAML:
Expand Down
15 changes: 9 additions & 6 deletions docs/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ codeguard owasp -format json # machine-readable
Example:

```
OWASP Top 10 (2021) coverage: 8/10 categories have rules
OWASP Top 10 (2021) coverage: 9/10 categories have rules

[ok ] A01:2021-Broken Access Control (2 rules)
[ok ] A02:2021-Cryptographic Failures (11 rules)
Expand All @@ -73,14 +73,15 @@ OWASP Top 10 (2021) coverage: 8/10 categories have rules
[ok ] A06:2021-Vulnerable and Outdated Components (1 rules)
[ok ] A07:2021-Identification and Authentication Failures (1 rules)
[ok ] A08:2021-Software and Data Integrity Failures (1 rules)
[gap ] A09:2021-Security Logging and Monitoring Failures (0 rules)
[ok ] A09:2021-Security Logging and Monitoring Failures (2 rules)
[ok ] A10:2021-Server-Side Request Forgery (SSRF) (2 rules)
```

`A04` (Insecure Design) and `A09` (Security Logging and Monitoring) are left as
explicit gaps: both are design- and operations-level risks that static
heuristics cannot reliably detect, and a false "covered" there would be
misleading.
`A04` (Insecure Design) is left as an explicit gap: it is a design-level risk
that static heuristics cannot reliably detect, and a false "covered" there
would be misleading. `A09` is covered by two heuristics that target the
code-visible slice of the category: secrets flowing into log output and raw
errors leaking to HTTP clients instead of being logged server-side.

### Newly added detection rules

Expand All @@ -99,6 +100,8 @@ taint engine and default to `fail`.
| `security.weak-hash` | A02 | MD5 / SHA-1 used for security |
| `security.weak-cipher` | A02 | DES / RC4 / ECB mode |
| `security.insecure-deserialization` | A08 | `pickle`, unsafe `yaml.load`, Java `readObject`, `Marshal.load`, `unserialize` |
| `security.log-secret-exposure` | A09 | secret-named identifiers (password, token, api_key, …) inside the argument list of a Go/Python/TS/JS logging call, secret-named structured-log keys, and secret-labeled string literals concatenated or format-directed into log output |
| `security.unsanitized-error-response` | A09 | raw error values written directly into HTTP responses: Go `http.Error(w, err.Error(), …)` / `fmt.Fprintf(w, …, err)`, TS/JS `res.send(err)` / `res.json(err)` / `res.status(…).send(err.stack \|\| err.message)`, Python `return str(e)` / `HttpResponse(str(e))` inside `except` blocks |
| `security.ssrf.go` / `security.ssrf.python` | A10 | untrusted input flowing into an outbound HTTP request URL |

## Release integrity (supply chain)
Expand Down
Loading
Loading