Improvments (#27)

Author: alxxjohn

.claude/knowledge/testing-patterns.md

@@ -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.

.codeguard/codeguard.yaml

@@ -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
@@ -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: ..

.gitignore

@@ -36,3 +36,4 @@ go.work.sum
 .idea/
 .vscode/
 **/.codeguard/cache.slop-history.json
+**/.codeguard/cache.rule-stats-history.json

.goreleaser.yaml

@@ -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 }}
 

Makefile

@@ -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
@@ -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
 

docs/checks.md

@@ -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.
@@ -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
@@ -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:
@@ -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:

docs/features.md

@@ -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:

docs/security.md

@@ -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)
@@ -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
 
@@ -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)