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
1 change: 1 addition & 0 deletions .claude/knowledge/local-dev-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ How to set up, run, and work with this project locally. Non-obvious dependencies

- **The self-scan cache lives at `.codeguard/.codeguard/cache.json`, not `.codeguard/cache.json`.** `cache.path` resolves relative to the config directory (`config.containConfigArtifactPaths`), and `make codeguard-ci` uses `-config .codeguard/codeguard.yaml`, so the default `.codeguard/cache.json` nests. When iterating on **rule logic**, delete that file before re-running `make codeguard-ci`: cache keys cover config + file content + the release-bumped scanner-version constant, so a code-only change to a check serves stale findings from the cache and looks like your fix didn't work.
- The Makefile runs Go via `env -u GOROOT go` (Makefile:6), so `make` targets work even with the shell's stale GOROOT; direct `go` commands need `export GOROOT=/opt/homebrew/opt/go/libexec && export PATH=$GOROOT/bin:$PATH` first.
- Local `golangci-lint run` can fail from the shell's stale `GOROOT` or unwritable `~/Library/Caches/golangci-lint` even when CI is clean. Use `env -u GOROOT GOCACHE=/private/tmp/codeguard-go-cache GOLANGCI_LINT_CACHE=/private/tmp/golangci-lint-cache golangci-lint run` to reproduce CI lint locally.
1 change: 1 addition & 0 deletions .claude/knowledge/testing-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ Testing strategies, test infrastructure quirks, how to run/debug specific test s
- **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.
- Defensive precision positive fixtures should avoid UI-ish names such as `render*` unless the test is explicitly covering UI suppression. The defensive boundary/null rules intentionally skip React/UI helper contexts, so a fixture named like a renderer can stop emitting the server-side defensive finding the test expects.
- Precision-rule retunes should include both the false-positive fixture and a nearby positive that still fires. Dogfood the local binary against a real target repo before committing; several naming/mutation rules only showed meaningful movement after scanning Legal Nest-style React, route, and integration code together.
110 changes: 99 additions & 11 deletions internal/codeguard/checks/quality/quality_defensive.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,37 @@ func isValidationOrExtractionHelperName(name string) bool {

func hasBoundaryParam(params []support.ParsedParam) bool {
for _, param := range params {
name := strings.ToLower(param.Name)
if containsAny(name, []string{"req", "request", "event", "payload", "body", "input"}) {
name := strings.ToLower(strings.Trim(param.Name, "_$"))
typ := strings.ToLower(param.Type)
switch name {
case "req", "event", "payload", "body", "params", "query":
return true
case "request":
if typ == "" || isTransportRequestType(typ) {
return true
}
case "input":
if typ == "" || containsAny(typ, []string{"unknown", "any", "record", "json", "request", "payload", "body", "params", "query"}) {
return true
}
default:
if strings.HasSuffix(name, "payload") || strings.HasSuffix(name, "body") || strings.HasSuffix(name, "params") || strings.HasSuffix(name, "query") {
return true
}
}
}
return false
}

func isTransportRequestType(typ string) bool {
typ = strings.TrimSpace(strings.ToLower(typ))
return typ == "request" ||
strings.Contains(typ, "nextrequest") ||
strings.Contains(typ, "httprequest") ||
strings.Contains(typ, "express.request") ||
strings.Contains(typ, "incomingmessage")
}

func nullAssumptionLine(file string, fn precisionFunction, loweredBody string) (int, bool) {
if isUIHelperOrMappingContext(file, fn) || isReactComponentOrHookBoundary(file, fn) {
return 0, false
Expand All @@ -211,30 +234,78 @@ func nullAssumptionLine(file string, fn precisionFunction, loweredBody string) (
if name == "" || !nullableParam(param) {
continue
}
if containsAny(loweredBody, []string{name + " == nil", name + " != nil", name + " is none", name + " is not none", name + " === null", name + " !== null", name + " == nullptr", name + " != nullptr"}) {
if nullableParamGuarded(loweredBody, name) {
continue
}
if containsAny(loweredBody, []string{name + ".", name + "->", name + "[", "*" + name}) {
if nullableUseLine(fn, name) > 0 {
return firstUseLine(fn, name), true
}
}
return 0, false
}

func nullableParam(param support.ParsedParam) bool {
typ := strings.ToLower(param.Type)
return strings.Contains(typ, "*") || strings.Contains(typ, "optional") ||
strings.Contains(typ, "null") || strings.Contains(typ, "none") ||
strings.Contains(typ, "maybe") || strings.Contains(typ, "?")
typ := strings.ToLower(strings.TrimSpace(param.Type))
if typ == "" {
return false
}
if strings.Contains(typ, "*") || strings.Contains(typ, "optional") || strings.Contains(typ, "maybe") {
return true
}
if strings.Contains(typ, "{") && strings.Contains(typ, "}") &&
!containsAny(typ, []string{"} | null", "}|null", "} | undefined", "}|undefined", "} | none", "}|none"}) {
return false
}
if strings.HasSuffix(strings.TrimSpace(typ), "[]") &&
!regexp.MustCompile(`\]\s*\|\s*(null|undefined|none)\b`).MatchString(typ) {
return false
}
return regexp.MustCompile(`(^|\|)\s*(null|undefined|none)\b|\b(null|undefined|none)\s*\|`).MatchString(typ) ||
strings.Contains(typ, "?")
}

func nullableParamGuarded(loweredBody string, name string) bool {
guards := []string{
name + " == nil",
name + " != nil",
name + " is none",
name + " is not none",
name + " === null",
name + " !== null",
name + " == null",
name + " != null",
name + " == nullptr",
name + " != nullptr",
"typeof " + name + " === ",
"typeof " + name + " == ",
}
if containsAny(loweredBody, guards) {
return true
}
return regexp.MustCompile(`if\s*\(\s*!\s*` + regexp.QuoteMeta(name) + `\s*\)\s*(?:return|throw|continue|break)\b`).MatchString(loweredBody)
}

func firstUseLine(fn precisionFunction, name string) int {
func nullableUseLine(fn precisionFunction, name string) int {
for _, statement := range fn.Statements {
lowered := strings.ToLower(firstNonEmptyString(statement.Raw, statement.Text))
if nullableStatementUsesOnlyNullSafeOperators(lowered, name) {
continue
}
if containsAny(lowered, []string{name + ".", name + "->", name + "[", "*" + name}) {
return statement.Line
}
}
return 0
}

func nullableStatementUsesOnlyNullSafeOperators(statement string, name string) bool {
return containsAny(statement, []string{name + "?.", name + "?.[", name + " ??"})
}

func firstUseLine(fn precisionFunction, name string) int {
if line := nullableUseLine(fn, name); line > 0 {
return line
}
return fn.StartLine
}

Expand All @@ -251,12 +322,26 @@ func integerOverflowLine(file string, fn precisionFunction, loweredBody string)
if !regexp.MustCompile(`(?i)\b(count|size|length|len|capacity|offset|total|bytes)\b`).MatchString(loweredBody) {
return 0, false
}
if regexp.MustCompile(`[A-Za-z_][\w$]*\s*(\*|\+|<<)\s*[A-Za-z0-9_]`).MatchString(loweredBody) {
return fn.StartLine, true
if !resourceAllocationArithmeticContext(loweredBody) {
return 0, false
}
arithmetic := regexp.MustCompile(`[A-Za-z_][\w$]*\s*(\*|\+|<<)\s*[A-Za-z0-9_]`)
for _, statement := range fn.Statements {
if arithmetic.MatchString(firstNonEmptyString(statement.Raw, statement.Text)) {
return statement.Line, true
}
}
return 0, false
}

func resourceAllocationArithmeticContext(loweredBody string) bool {
return containsAny(loweredBody, []string{
"buffer.alloc", "allocunsafe", "new uint8array", "new arraybuffer", "new array(",
"make([]", "bytearray(", "vector<", "reserve(", "resize(", "setlength(", "content-length", "contentlength",
"readall", "read_all", "readtoend", "read_to_end",
})
}

func sequenceCollisionRiskLine(file string, fn precisionFunction, loweredBody string) (int, string, string, bool) {
if isSeedOrScriptSourcePath(file) || !sequenceAllocationArithmetic(loweredBody) {
return 0, "", core.ConfidenceLow, false
Expand Down Expand Up @@ -354,8 +439,11 @@ func isSeedOrScriptSourcePath(file string) bool {
strings.Contains(normalized, "/backfill") ||
strings.Contains(normalized, "/import") ||
strings.HasPrefix(base, "seed") ||
strings.Contains(base, "seed") ||
strings.HasPrefix(base, "backfill") ||
strings.Contains(base, "backfill") ||
strings.HasPrefix(base, "import") ||
strings.Contains(base, "import") ||
strings.HasPrefix(base, "cleanup")
}

Expand Down
20 changes: 19 additions & 1 deletion internal/codeguard/checks/quality/quality_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,19 @@ func exceptionControlFlowLine(fn precisionFunction) (int, bool) {
for idx, statement := range fn.Statements {
raw := firstNonEmptyString(statement.Raw, statement.Text)
lowered := strings.ToLower(raw)
if !throwRaisePattern.MatchString(raw) || !containsAny(lowered, []string{"invalid", "not found", "missing", "stop", "continue", "break"}) {
if !throwRaisePattern.MatchString(raw) || validationExceptionThrow(lowered) {
continue
}
if strings.Contains(lowered, "panic(") {
continue
}
controlWord := containsAny(lowered, []string{"stop", "continue", "break"})
genericLookupSentinel := containsAny(strings.ToLower(fn.Name), []string{"find", "lookup", "get", "control"}) &&
containsAny(lowered, []string{"not found", "missing"}) &&
containsAny(lowered, []string{"exception", "runtimeerror", "runtime_error", "valueerror"})
if !controlWord && !genericLookupSentinel {
continue
}
if strings.Contains(lowered, " if ") || strings.HasPrefix(strings.TrimSpace(lowered), "if ") ||
(idx > 0 && strings.HasPrefix(strings.TrimSpace(strings.ToLower(fn.Statements[idx-1].Text)), "if ")) {
return statement.Line, true
Expand All @@ -316,6 +323,17 @@ func exceptionControlFlowLine(fn precisionFunction) (int, bool) {
return 0, false
}

func validationExceptionThrow(lowered string) bool {
if containsAny(lowered, []string{"stop", "continue", "break"}) {
return false
}
return containsAny(lowered, []string{
"new trpcerror", "new apperror", "new error", "bad_request", "unauthorized",
"forbidden", "unsupported", "invalid json", "invalid input", "invalid response",
"missing required", "source file not found",
})
}

func boundaryFunctionName(name string) bool {
lowered := strings.ToLower(name)
return containsAny(lowered, []string{"handler", "handle", "controller", "route", "api", "endpoint", "render", "respond", "response"})
Expand Down
8 changes: 4 additions & 4 deletions internal/codeguard/checks/quality/quality_precision.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func primitiveObsession(fn precisionFunction) bool {
}

func hiddenSideEffect(file string, fn precisionFunction) bool {
if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) {
if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) || isUICommandHelperName(file, fn.Name) {
return false
}
if !queryFunctionPrefixPattern.MatchString(strings.ToLower(fn.Name)) {
Expand All @@ -488,7 +488,7 @@ func hiddenSideEffect(file string, fn precisionFunction) bool {
}
localTargets := localMutationTargets(fn)
for _, call := range directCalls(fn) {
if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) {
if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call, localTargets) && !isLocalBuilderMutationCall(fn, call) && !isBuilderAccumulatorMutationCall(fn, call) {
return true
}
}
Expand Down Expand Up @@ -522,7 +522,7 @@ func isDomainLevelCall(callee string) bool {
}

func commandQueryMix(file string, fn precisionFunction) bool {
if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) {
if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) || isUICommandHelperName(file, fn.Name) {
return false
}
if !fn.Returns {
Expand All @@ -537,7 +537,7 @@ func commandQueryMix(file string, fn precisionFunction) bool {
}
localTargets := localMutationTargets(fn)
for _, call := range directCalls(fn) {
if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) {
if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call, localTargets) && !isLocalBuilderMutationCall(fn, call) && !isBuilderAccumulatorMutationCall(fn, call) {
return true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

var conventionalMutationBoundaryPattern = regexp.MustCompile(`^(accept|apply|approve|archive|clear|close|commit|deliver|download|drop|ensure|exists|fetch|import|list|notify|open|process|read|reconcile|record|run|seed|submit|sync|toggle|upload)`)

var localAccumulatorExprPattern = regexp.MustCompile(`(?i)^(?:new\s+)?(?:array|formdata|map|object|set|urlsearchparams|weakmap|weakset)\b|^\[|^\{|^make\s*\(|^array\.from\b|\.map\s*\(|\.filter\s*\(|\.reduce\s*\(|\.split\s*\(|cheerio\.load\s*\(|^(?:bytes|strings)\.buffer\b|^strings\.builder\b`)
var localAccumulatorExprPattern = regexp.MustCompile(`(?i)^(?:new\s+)?(?:array|date|filereader|formdata|image|map|object|set|url|urlsearchparams|weakmap|weakset)\b|^\[|^\{|^make\s*\(|^array\.from\b|\.map\s*\(|\.filter\s*\(|\.reduce\s*\(|\.split\s*\(|cheerio\.load\s*\(|document\.createelement\s*\(|^(?:bytes|strings)\.buffer\b|^strings\.builder\b`)

func localMutationTargets(fn precisionFunction) map[string]struct{} {
params := paramNames(fn)
Expand All @@ -27,6 +27,10 @@ func localMutationTargets(fn precisionFunction) map[string]struct{} {
targets[name] = struct{}{}
continue
}
if assignmentDerivedFromLocalMutationTarget(assignment, targets) {
targets[name] = struct{}{}
continue
}
if fn.Returns && isAccumulatorLikeLocalName(name) && assignmentLooksLocalBuilder(fn, assignment) {
targets[name] = struct{}{}
}
Expand Down Expand Up @@ -127,11 +131,27 @@ func assignmentLooksLocalBuilder(fn precisionFunction, assignment support.Parsed
strings.Contains(expr, "make")
}

func assignmentDerivedFromLocalMutationTarget(assignment support.ParsedAssignment, localTargets map[string]struct{}) bool {
expr := strings.TrimSpace(assignment.Expr)
if expr == "" || len(localTargets) == 0 {
return false
}
for target := range localTargets {
if target == "" {
continue
}
if strings.HasPrefix(expr, target+".") || strings.HasPrefix(expr, target+"->") || strings.HasPrefix(expr, target+"::") {
return true
}
}
return false
}

func isAccumulatorLikeLocalName(name string) bool {
lowered := strings.ToLower(strings.Trim(name, "_$"))
for _, token := range []string{
"bucket", "buckets", "buffer", "builder", "calendar", "cells", "copy", "doc",
"document", "filter", "filters", "form", "items", "lines", "params", "parts",
"canvas", "ctx", "cursor", "date", "document", "filter", "filters", "form", "img", "items", "lines", "params", "parts",
"payload", "primarycells", "query", "result", "rows", "scopes", "sections",
"serializer", "text", "urlparams", "values", "csv", "export", "map", "$",
} {
Expand Down Expand Up @@ -177,7 +197,26 @@ func paramNames(fn precisionFunction) map[string]struct{} {
return params
}

func isLocalMutationCall(callee string, localTargets map[string]struct{}) bool {
func isLocalMutationCall(call support.ParsedCall, localTargets map[string]struct{}) bool {
if isObjectAssignToLocalTarget(call, localTargets) {
return true
}
return isLocalMutationCallee(call.Callee, localTargets)
}

func isLocalBuilderMutationCall(fn precisionFunction, call support.ParsedCall) bool {
loweredCallee := strings.ToLower(strings.ReplaceAll(strings.TrimSpace(call.Callee), " ", ""))
if loweredCallee == "createhmac" || loweredCallee == "createhash" {
return true
}
if loweredCallee != "update" && !strings.HasSuffix(loweredCallee, ".update") {
return false
}
statement := strings.ToLower(assignmentStatement(fn, call.Line))
return strings.Contains(statement, "createhmac(") || strings.Contains(statement, "createhash(")
}

func isLocalMutationCallee(callee string, localTargets map[string]struct{}) bool {
if isDerivedCollectionMutationCall(callee) {
return true
}
Expand All @@ -191,6 +230,35 @@ func isLocalMutationCall(callee string, localTargets map[string]struct{}) bool {
return isLocalMutationTarget(target, localTargets)
}

func isObjectAssignToLocalTarget(call support.ParsedCall, localTargets map[string]struct{}) bool {
if !isObjectAssignCall(call) {
return false
}
target := firstCallArgName(call)
return target != "" && isLocalMutationTarget(target, localTargets)
}

func isObjectAssignCall(call support.ParsedCall) bool {
return strings.EqualFold(strings.TrimSpace(call.Callee), "Object.assign")
}

func firstCallArgName(call support.ParsedCall) string {
if len(call.Args) == 0 {
return ""
}
arg := strings.TrimSpace(call.Args[0])
if arg == "" {
return ""
}
if idx := strings.IndexAny(arg, ".[("); idx > 0 {
arg = strings.TrimSpace(arg[:idx])
}
if regexp.MustCompile(`^[A-Za-z_$][\w$]*$`).MatchString(arg) {
return arg
}
return ""
}

func isDerivedCollectionMutationCall(callee string) bool {
lowered := strings.ToLower(strings.ReplaceAll(strings.TrimSpace(callee), " ", ""))
return strings.Contains(lowered, ".split.") && (strings.HasSuffix(lowered, ".pop") || strings.HasSuffix(lowered, ".sort") || strings.HasSuffix(lowered, ".reverse"))
Expand Down Expand Up @@ -231,6 +299,9 @@ func isBuilderAccumulatorMutationCall(fn precisionFunction, call support.ParsedC
return false
}
target := mutationCallTarget(call.Callee)
if isObjectAssignCall(call) {
target = firstCallArgName(call)
}
if target == "" {
return isBareLocalMutationCall(call.Callee)
}
Expand Down
Loading
Loading