diff --git a/README.md b/README.md index c031a9c..b622529 100644 --- a/README.md +++ b/README.md @@ -12,9 +12,11 @@ -`codeguard` is a standalone Go service and CLI for repository checks across code quality, design boundaries, security, CI/CD hygiene, AI prompt governance, and repo-specific policy rules. +`codeguard` is a standalone Go service and CLI for repository checks across code quality, production reliability, data correctness, design boundaries, security, CI/CD hygiene, AI prompt governance, and repo-specific policy rules. -It now supports repository exclusions, baselines, waivers, changed-lines diff scans, SARIF output, GitHub annotations, custom rule packs, natural-language custom rules through an optional AI runtime, policy profiles, scan caching, doctor checks, rule discovery from the CLI, native TypeScript/Python quality, design, and security heuristics, and language-specific command checks. +It now supports repository exclusions, baselines, waivers, changed-lines diff scans, SARIF output, GitHub annotations, custom rule packs, natural-language custom rules through an optional AI runtime, policy profiles, scan caching, doctor checks, rule discovery from the CLI, native TypeScript/Python quality, design, security, reliability, and data-correctness heuristics, and language-specific command checks. + +For a user-facing glossary of every check family and subsection, see [docs/checks.md](docs/checks.md). AI-generated-code quality coverage includes an AI-failure-mode rule pack, `slop_score` artifacts, provenance-aware review policy hooks, local idiom drift checks, optional provider-backed hybrid triage and semantic review passes, natural-language custom rules through an optional AI runtime, and a verified-fix flow that only returns patches after isolated patch validation plus test reruns succeed. @@ -110,7 +112,7 @@ For production rollout, start in a narrow mode and expand deliberately: 1. Run `codeguard doctor` and `codeguard validate` in CI first so config and toolchain issues fail early. 2. Start with `codeguard scan -mode diff` on pull requests so only changed lines and diff-aware checks gate merges. 3. Create a baseline for legacy findings with `codeguard baseline` before turning on full-repo enforcement. -4. Enable stricter families such as `design`, `security`, `contracts`, `performance`, and `supply_chain` incrementally per repository. +4. Enable stricter families such as `security`, `reliability`, `data`, `design`, `contracts`, `performance`, and `supply_chain` incrementally per repository. 5. Use `codeguard rules` and `codeguard explain ` to document what a failure means before asking teams to act on it. When a scan fails: diff --git a/docs/checks.md b/docs/checks.md index 24b742b..3dc331e 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -22,6 +22,35 @@ codeguard explain quality.ai.semantic-runtime Use `codeguard rules` to discover what exists and `codeguard explain ` to understand why a specific finding failed and what remediation path it expects. +## Check glossary + +This glossary is the quick map of every built-in check family and the main subsections users will see in reports, rule IDs, and config. + +| Check family | Report section | Config key | Main subsections / rule themes | +| --- | --- | --- | --- | +| Quality | `Code Quality` | `checks.quality` | formatting and parseability; maintainability thresholds; file/function size; cyclomatic complexity; clone detection; language-specific quality rules; TypeScript/JavaScript type-safety rules; AI-failure-mode checks; semantic review; changed-line coverage; C++ formatter/compiler validation | +| Performance | `Performance` | `checks.performance` | N+1 query/fetch patterns; allocation-heavy loops; repeated work in loops; blocking I/O in request paths; unbounded concurrency; sequential await; timer/listener leaks; unbounded whole-input reads; framework-aware performance smells; rebuild-cascade analysis; complexity regression; size budgets; build regression; benchmark regression | +| Reliability | `Reliability` | `checks.reliability` | missing outbound timeouts; unbounded retries; retries without backoff/jitter; non-idempotent retries; missing cancellation propagation; unbounded work; missing concurrency limits; resource leaks; hidden partial failures; missing graceful shutdown; swallowed errors; lost error context; recoverable panics/exceptions | +| Data Correctness | `Data Correctness` | `checks.data` | read-modify-write races; missing transaction boundaries; external side effects inside transactions; non-idempotent consumers; missing deduplication; unsafe dual writes; missing outbox strategy; unstable pagination; unbounded reads; exactly-once assumptions; cache writes without TTL/policy | +| API Contracts | `API Contracts` | `checks.contracts` | exported Go API breaks; public C++ header breaks; OpenAPI breaking changes; protobuf breaking changes; destructive migrations; non-expand/contract schema migration risk | +| Design | `Design Patterns` | `checks.design` | architecture boundaries; import/module cycles; god modules; graph reachability and stability; high-impact changes; public surface policy; production/test isolation; package/module naming; declarations per file; methods per type; interface/protocol size | +| Security | `Security` | `checks.security` | hardcoded secrets and credentials; private keys; insecure TLS; shell execution; dynamic code execution; unsafe HTML sinks; SSRF and taint-style flow; unsafe C string APIs; optional `govulncheck`; OWASP category metadata | +| Supply Chain | `Supply Chain` | `checks.supply_chain` | manifest normalization; SBOM output; missing lockfiles; lockfile drift; unpinned dependencies; license policy; offline advisory-cache vulnerability matching; Cargo manifest hygiene; C++ package-manager metadata for vcpkg, Conan, and CMake | +| Prompts | `Prompts` | `checks.prompts` | prompt-asset governance; secret interpolation; unsafe instructions; dangerous agent instructions; standing permissions; MCP config risk | +| CI/CD | `CI/CD` | `checks.ci` | required workflow directories/files; workflow content policy; release automation files; test file location; test assertions; conditional/always-true assertions; cross-language test-quality heuristics | +| Agent Context | `Agent Context` | `checks.context` | missing agent docs; README/doc drift; oversized files; ambiguous symbols; undocumented commands; oversized agent docs; doc link rot; repository readiness for coding agents | +| External Reports | `External Reports` | `external_reports` | imported SARIF, Gitleaks JSON, and Trivy JSON findings from scanners that already ran; normalized into CodeGuard report sections with namespaced rule IDs | + +Related report artifacts: + +| Artifact | Config key | Purpose | +| --- | --- | --- | +| `slop_score` | `quality_rules.ai_checks.slop_history` | Trends AI-failure-mode signals over time. | +| `change_risk` | `quality_rules.ai_change_risk` | Aggregates AI-quality and review-risk signals. | +| `file_risk` / `pr_hotspots` | `quality_rules.risk_scoring` | Ranks changed files by configurable risk evidence. | +| `performance_score` | `performance_rules.score_history` | Tracks performance-smell trends. | +| `pr_summary.production_risk` | `checks.production_risk` | Rolls reliability, data-correctness, and non-expand/contract migration findings into PR-level production-risk evidence. | + ## Top-level shape ```json @@ -36,6 +65,8 @@ understand why a specific finding failed and what remediation path it expects. "prompts": true, "ci": true, "supply_chain": false, + "reliability": false, + "data": false, "contracts": true, "context": true } @@ -69,6 +100,10 @@ baseline. `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, dependency license policy resolved from local manifest and installed metadata where available, local advisory-cache vulnerability matching, and Cargo manifest hygiene for missing package licenses and nonhermetic dependency sources. +`reliability` covers production reliability checks for Go, Python, TypeScript, JavaScript, and C++: missing outbound timeouts, unbounded or immediate retries, non-idempotent retries, cancellation propagation gaps, unbounded work, missing concurrency limits, resource cleanup gaps, hidden partial failures, missing graceful shutdown evidence, swallowed/lost errors, and recoverable panics. + +`data` covers distributed-system and data-correctness checks for Go, Python, TypeScript, JavaScript, and C++: read-modify-write race patterns, missing transaction boundaries, side effects in transaction callbacks, non-idempotent consumers, missing deduplication, unsafe dual writes, missing outbox strategy, unstable pagination, unbounded reads, exactly-once assumptions, and cache writes without TTL/policy evidence. + Set `output.format` to `cyclonedx` (or pass `codeguard scan -format cyclonedx`) to emit the normalized dependency artifacts as deterministic CycloneDX 1.6 JSON. The SBOM contains declared dependency versions or requirements when a resolver version is unavailable; it does not execute project code or contact a registry. ### Offline advisory cache @@ -105,7 +140,7 @@ The first supported cache schema is `schema_version: 1`. Each advisory has an ec Findings contain the advisory identifier, source, generated timestamp, and cache age as non-sensitive metadata. Refreshing the cache is intentionally outside scan execution and should be handled by an approved, auditable update process. -`contracts` covers API compatibility against a diff base. When omitted, it is enabled in diff scans and disabled in full scans. It checks exported Go declarations, public C++ headers, OpenAPI documents, protobuf schemas, and destructive migrations. +`contracts` covers API compatibility against a diff base. When omitted, it is enabled in diff scans and disabled in full scans. It checks exported Go declarations, public C++ headers, OpenAPI documents, protobuf schemas, destructive migrations, and non-expand/contract migration risk. 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. @@ -330,6 +365,8 @@ Current inference behavior: | Quality | `gofmt`, parseability, maintainability thresholds | maintainability thresholds across sources, headers, templates, and modules; optional `clang-format` and sanitized `clang++` validation | maintainability thresholds | maintainability thresholds, `@ts-ignore`, `@ts-nocheck`, `@ts-expect-error`, `explicit any`, double assertions, non-null assertions, `debugger` statements | maintainability thresholds | maintainability thresholds | maintainability thresholds | maintainability thresholds | | Design | package boundary rules, generic package names, declarations per file, methods per type, interface size, graph reachability/stability/impact | include and named-module cycles, reachability, stability, graph impact, generic filenames, declarations per file, method counts, contract surface checks, boundary-policy enforcement | public/private and entrypoint coupling, import cycles, generic module names, methods per type, protocol size | generic module names, max methods per class, max members per interface/object type, graph resolution through `tsconfig` paths, package `imports`, and workspace package exports | module cycles, graph impact, generic module names, methods per type, trait size | import cycles and graph impact | - | - | | Security | insecure TLS, shell execution review, optional `govulncheck` | insecure TLS, shell execution review, unsafe C string APIs, taint flow, SSRF | insecure TLS, shell execution review, dynamic code | insecure TLS, shell execution review, dynamic code, string timer execution, wildcard `postMessage`, Node `vm` execution, unsafe HTML sinks | insecure TLS, shell execution review | insecure TLS, shell execution review | insecure TLS, shell execution review | insecure TLS, shell execution review, dynamic code | +| Reliability | missing timeouts, cancellation gaps, retry policy gaps, non-idempotent retry evidence, unbounded goroutines/work, resource cleanup, swallowed/lost errors, recoverable panic, graceful shutdown | retry policy gaps, non-idempotent retry evidence, unbounded thread/task launch, raw allocation cleanup gaps, generic runtime throws | missing HTTP timeouts, retry policy gaps, non-idempotent retry evidence, unbounded asyncio work, swallowed exceptions, generic raises, resource cleanup | missing timeout/abort evidence, promise/HTTP work in loops, retry policy gaps, non-idempotent retry evidence, swallowed catches, generic throws | - | - | - | - | +| Data Correctness | read-modify-write races, transaction gaps, side effects in transactions, consumer idempotency/dedupe, dual writes/outbox, pagination, unbounded SQL reads, exactly-once assumptions, cache policy | transaction gaps, consumer idempotency/dedupe, dual writes/outbox, pagination, unbounded reads, exactly-once assumptions, cache policy | transaction gaps, consumer idempotency/dedupe, dual writes/outbox, pagination, unbounded reads, exactly-once assumptions, cache policy | transaction gaps, consumer idempotency/dedupe, dual writes/outbox, pagination, unbounded reads, exactly-once assumptions, cache policy | - | - | - | - | | Commands | language command mappings via config | language command mappings via config | language command mappings via config | language command mappings via config | language command mappings via config | language command mappings via config | language command mappings via config | language command mappings via config | TypeScript semantic runtime: @@ -833,6 +870,86 @@ Repo-specific performance policies can also be expressed as natural-language cus **Future work:** pprof profile ingestion/fusion (attributing regressions to functions by diffing CPU/heap profiles) is deliberately out of scope for this version. +## Reliability + +Purpose: +- Detect production failure modes that are not captured by style linters. +- Surface risky outbound calls, retries, cancellation, fan-out, cleanup, shutdown, and error-handling paths. + +Config keys: + +```json +{ + "checks": { + "reliability": true, + "reliability_rules": { + "detect_missing_timeout": true, + "detect_unbounded_retry": true, + "detect_retry_without_backoff": true, + "detect_non_idempotent_retry": true, + "detect_missing_cancellation": true, + "detect_unbounded_work": true, + "detect_missing_concurrency_limit": true, + "detect_resource_leak": true, + "detect_partial_failure_hidden": true, + "detect_missing_graceful_shutdown": true, + "detect_swallowed_error": true, + "detect_lost_error_context": true, + "detect_recoverable_panic": true + } + } +} +``` + +Rules are implemented for Go, Python, TypeScript, JavaScript, and C++. Some rules are high-confidence syntax checks, such as Go `http.Get` without a timeout or response bodies without `Close`; others are confidence-based heuristics around retry/idempotency naming, concurrency limits, and shutdown evidence. + +## Data Correctness + +Purpose: +- Detect distributed-system and persistence risks that can cause data loss, duplicated side effects, or unsafe production rollout. +- Surface transaction, idempotency, outbox, pagination, unbounded-read, delivery-semantics, and cache-policy gaps. + +Config keys: + +```json +{ + "checks": { + "data": true, + "data_rules": { + "detect_read_modify_write_race": true, + "detect_missing_transaction": true, + "detect_side_effect_in_transaction": true, + "detect_non_idempotent_consumer": true, + "detect_missing_deduplication": true, + "detect_unsafe_dual_write": true, + "detect_missing_outbox_strategy": true, + "detect_unstable_pagination": true, + "detect_unbounded_read": true, + "detect_exactly_once_assumption": true, + "detect_cache_without_policy": true + } + } +} +``` + +Rules are implemented for Go, Python, TypeScript, JavaScript, and C++. Contracting database migrations are also surfaced as `contracts.non-expand-contract-migration` in the `API Contracts` section so production-risk scoring can treat unsafe rolling schema changes as data-correctness evidence without renaming the legacy destructive-migration rule. + +## PR Summary Production Risk + +`checks.production_risk` enables an additive diff-mode `pr_summary.production_risk` artifact. It scores reliability, data-correctness, and `contracts.non-expand-contract-migration` findings into deterministic PR-level evidence. It does not change individual rule severities, SARIF output, GitHub annotations, or the text summary line. + +```json +{ + "checks": { + "production_risk": { + "enabled": true, + "warn_threshold": 25, + "fail_threshold": 70 + } + } +} +``` + ## Supply Chain Purpose: @@ -891,6 +1008,8 @@ Config keys: `contracts.cpp-public-breaking` compares changed or deleted `.h`, `.hh`, `.hpp`, `.hxx`, and `.h++` files under an `include`, `public`, or `api` directory with the base ref. It conservatively reports removed/renamed types and aliases plus removed or changed function declarations. Private implementation headers outside those public roots are ignored. +Destructive migration evidence is reported both as the legacy warning `contracts.migration-destructive` and as the production-readiness failure `contracts.non-expand-contract-migration`. The dual reporting preserves existing waivers/baselines while making unsafe rolling schema migration risk available to production-risk scoring. + The contracts family needs a base revision, so it runs in diff mode. When `checks.contracts` is omitted it defaults to enabled for diff scans and disabled for full scans. ## Design diff --git a/docs/features.md b/docs/features.md index 5dce1ed..222353a 100644 --- a/docs/features.md +++ b/docs/features.md @@ -49,9 +49,15 @@ This page lists the current `codeguard` feature surface and the main config entr - Rust and C++ loop-smell coverage for regex construction, non-preallocated string growth, and polling sleeps - C++ loop-driven unbounded thread/task launch detection - build regression, benchmark regression, artifact-size budgets, and clang `-ftime-trace` budgets +- `reliability` + - production-readiness checks for Go, Python, TypeScript, JavaScript, and C++ + - missing outbound timeouts, retry policy gaps, non-idempotent retries, cancellation gaps, unbounded work, resource cleanup, swallowed/lost errors, recoverable panics, and graceful-shutdown evidence +- `data` + - distributed-system and data-correctness checks for Go, Python, TypeScript, JavaScript, and C++ + - read-modify-write race patterns, missing transaction boundaries, side effects in transactions, consumer idempotency/deduplication gaps, unsafe dual writes, missing outbox strategy, unstable pagination, unbounded reads, exactly-once assumptions, and cache policy gaps - `contracts` - exported Go and public C++ API compatibility against a diff base - - OpenAPI, protobuf, and destructive migration checks + - OpenAPI, protobuf, destructive migration checks, and non-expand/contract migration risk ## Agent-native features @@ -105,6 +111,9 @@ Imported reports are never passed to AI triage. - Diff-mode file risk and PR hotspots - emits `file_risk` and `pr_hotspots` artifacts that rank every changed file without changing finding severity - explains each score with stable, configurable contributions from findings, security and supply-chain signals, changed-line coverage, AI provenance, and slop-score artifacts where available +- Diff-mode production risk + - emits `pr_summary.production_risk` when configured, using reliability, data-correctness, and non-expand/contract migration findings as deterministic PR-level risk evidence + - does not change SARIF, GitHub annotations, or individual finding severity ## Parsers diff --git a/docs/production.md b/docs/production.md index 098f38d..eb88643 100644 --- a/docs/production.md +++ b/docs/production.md @@ -51,6 +51,8 @@ In production, `codeguard` should do three things well: A practical order is: - `security` + - `reliability` + - `data` - `quality` - `ci` - `design` @@ -60,6 +62,13 @@ In production, `codeguard` should do three things well: That ordering usually gives the fastest signal-to-noise improvement. + `reliability` and `data` are production-readiness families. Enable them first + in PR diff scans, review the confidence/noise profile, then decide which + fail-level findings should block in your profile. `checks.production_risk` + can add a diff-mode `pr_summary.production_risk` artifact that rolls these + findings up into PR-level evidence without changing SARIF or GitHub + annotations. + ## How to read a failed scan Every finding has four pieces that matter operationally: @@ -94,6 +103,8 @@ team or agent needs the meaning of one specific failure. Use blocking failures for: - credential leaks +- missing timeouts, non-idempotent retries, resource leaks, or unbounded retry/work patterns with high-confidence evidence +- unsafe dual writes, missing transaction boundaries, missing outbox strategy, non-idempotent consumers, or non-expand/contract migrations - contract breaks - architecture violations with clear ownership boundaries - unsafe prompt or MCP config patterns @@ -103,6 +114,7 @@ Use warnings for: - maintainability drift - cleanup-oriented design heuristics +- confidence-based retry, concurrency, pagination, unbounded-read, cache-policy, or exactly-once-delivery signals that need repository-specific review - stability and reachability nudges - performance smells that still need human review @@ -131,7 +143,7 @@ For most teams: - pull requests: `codeguard scan -mode diff` - nightly or scheduled: `codeguard scan` -- release branches: `codeguard scan` plus contracts and supply-chain enforcement +- release branches: `codeguard scan` plus reliability, data, contracts, and supply-chain enforcement Prefer SARIF or GitHub output when you want code-host annotations, and JSON when another system or agent will consume the report programmatically. diff --git a/examples/codeguard.json b/examples/codeguard.json index a5a9557..7c511c3 100644 --- a/examples/codeguard.json +++ b/examples/codeguard.json @@ -18,6 +18,8 @@ "prompts": true, "ci": true, "context": true, + "reliability": false, + "data": false, "quality_rules": { "max_file_lines": 400, "max_function_lines": 80, @@ -149,6 +151,47 @@ "max_file_lines": 1500, "ambiguous_symbol_threshold": 4, "max_agent_doc_lines": 600 + }, + "reliability_rules": { + "detect_missing_timeout": true, + "detect_unbounded_retry": true, + "detect_retry_without_backoff": true, + "detect_non_idempotent_retry": true, + "detect_missing_cancellation": true, + "detect_unbounded_work": true, + "detect_missing_concurrency_limit": true, + "detect_resource_leak": true, + "detect_partial_failure_hidden": true, + "detect_missing_graceful_shutdown": true, + "detect_swallowed_error": true, + "detect_lost_error_context": true, + "detect_recoverable_panic": true, + "max_retry_attempts": 3, + "max_inline_goroutines_per_function": 8 + }, + "data_rules": { + "detect_read_modify_write_race": true, + "detect_missing_transaction": true, + "detect_side_effect_in_transaction": true, + "detect_non_idempotent_consumer": true, + "detect_missing_deduplication": true, + "detect_unsafe_dual_write": true, + "detect_missing_outbox_strategy": true, + "detect_unstable_pagination": true, + "detect_unbounded_read": true, + "detect_exactly_once_assumption": true, + "detect_cache_without_policy": true, + "max_unbounded_read_rows": 1000, + "max_writes_without_transaction": 1 + }, + "production_risk": { + "enabled": true, + "warn_threshold": 25, + "fail_threshold": 70, + "reliability_weight": 10, + "data_weight": 12, + "fail_weight": 10, + "warn_weight": 4 } }, "exclude": ["vendor/**", "**/testdata/**"], diff --git a/internal/codeguard/checks/contracts/migrations.go b/internal/codeguard/checks/contracts/migrations.go index c0a115d..10ba377 100644 --- a/internal/codeguard/checks/contracts/migrations.go +++ b/internal/codeguard/checks/contracts/migrations.go @@ -86,13 +86,13 @@ func statementFindings(env support.Context, file string, statement string, start findings := make([]core.Finding, 0) for _, pattern := range destructivePatterns { for _, loc := range pattern.re.FindAllStringIndex(statement, -1) { - findings = append(findings, newMigrationFinding(env, file, lineAt(statement, startLine, loc[0]), - fmt.Sprintf("destructive migration operation: %s", pattern.summary))) + findings = append(findings, newMigrationFindings(env, file, lineAt(statement, startLine, loc[0]), + fmt.Sprintf("destructive migration operation: %s", pattern.summary))...) } } if loc := alterNotNullRe.FindStringIndex(statement); loc != nil && !defaultClauseRe.MatchString(statement) { - findings = append(findings, newMigrationFinding(env, file, lineAt(statement, startLine, loc[0]), - "destructive migration operation: ALTER ... NOT NULL without DEFAULT")) + findings = append(findings, newMigrationFindings(env, file, lineAt(statement, startLine, loc[0]), + "destructive migration operation: ALTER ... NOT NULL without DEFAULT")...) } return findings } @@ -101,13 +101,23 @@ func lineAt(statement string, startLine int, offset int) int { return startLine + strings.Count(statement[:offset], "\n") } -func newMigrationFinding(env support.Context, file string, line int, message string) core.Finding { - return env.NewFinding(support.FindingInput{ - RuleID: "contracts.migration-destructive", - Level: "warn", - Path: file, - Line: line, - Column: 1, - Message: message, - }) +func newMigrationFindings(env support.Context, file string, line int, message string) []core.Finding { + return []core.Finding{ + env.NewFinding(support.FindingInput{ + RuleID: "contracts.migration-destructive", + Level: "warn", + Path: file, + Line: line, + Column: 1, + Message: message, + }), + env.NewFinding(support.FindingInput{ + RuleID: "contracts.non-expand-contract-migration", + Level: "fail", + Path: file, + Line: line, + Column: 1, + Message: "non-expand/contract schema migration risk: " + message, + }), + } } diff --git a/internal/codeguard/checks/data/data.go b/internal/codeguard/checks/data/data.go new file mode 100644 index 0000000..6b64763 --- /dev/null +++ b/internal/codeguard/checks/data/data.go @@ -0,0 +1,38 @@ +// Package data implements distributed-system and data-correctness checks. +package data + +import ( + "context" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func Run(ctx context.Context, env support.Context) core.SectionResult { + return support.RunTargetSection(ctx, env, "data", "Data Correctness", dataTargetFindings) +} + +func dataTargetFindings(_ context.Context, env support.Context, target core.TargetConfig) []core.Finding { + switch support.NormalizedLanguage(target.Language) { + case "go", "golang": + return support.ScanGoFiles(env, target, "data", func(file string, data []byte) []core.Finding { + return goFindingsForFile(env, file, data) + }) + case "python", "py": + return support.ScanPythonFiles(env, target, "data", func(file string, data []byte) []core.Finding { + return pythonFindingsForFile(env, file, data) + }) + case "typescript", "javascript", "ts", "tsx", "js", "jsx": + return typeScriptTargetFindings(env, target) + case "c++", "cpp", "cxx", "cc": + return support.ScanCPPFiles(env, target, "data", func(file string, data []byte) []core.Finding { + return cppFindingsForFile(env, file, data) + }) + default: + return nil + } +} + +func enabled(toggle *bool) bool { + return toggle == nil || *toggle +} diff --git a/internal/codeguard/checks/data/data_cpp.go b/internal/codeguard/checks/data/data_cpp.go new file mode 100644 index 0000000..f6987f6 --- /dev/null +++ b/internal/codeguard/checks/data/data_cpp.go @@ -0,0 +1,116 @@ +package data + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + cppDataRead = regexp.MustCompile(`(?i)\b(?:select|query|execute|find|load)\w*\s*\(`) + cppDataLimit = regexp.MustCompile(`(?i)\blimit\s*\(|\bwhere\s*\(|cursor|stream`) + cppDataOffset = regexp.MustCompile(`(?i)\boffset\s*\(`) + cppDataOrder = regexp.MustCompile(`(?i)\border_by\s*\(|order\s+by`) + cppDataWrite = regexp.MustCompile(`(?i)\b(?:create|update|delete|save|insert|upsert|exec)\w*\s*\(`) + cppDataPublish = regexp.MustCompile(`(?i)\b(?:publish|emit|send|enqueue|dispatch)\w*\s*\(`) + cppDataTx = regexp.MustCompile(`(?i)transaction|begin_tx|with_tx|txn`) + cppDataOutbox = regexp.MustCompile(`(?i)outbox`) + cppDataDedupe = regexp.MustCompile(`(?i)idempot|dedupe|dedup|processed|message_id|event_id`) + cppDataConsumer = regexp.MustCompile(`(?i)\b(?:Handle|Consume|Process|OnMessage|OnEvent)\w*\s*\(`) + cppDataCacheSet = regexp.MustCompile(`(?i)\bcache\.(?:set|put)\s*\(`) + cppDataTTL = regexp.MustCompile(`(?i)ttl|expire|expires`) +) + +func cppFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + source := strings.ReplaceAll(string(data), "\r\n", "\n") + masked := support.MaskCLikeSource(source, support.CLikeCPP) + scan := &cppDataScan{env: env, file: file, rules: env.Config.Checks.DataRules} + for idx, line := range strings.Split(masked, "\n") { + scan.consumeLine(idx+1, line) + } + scan.consumeRawSource(source) + scan.finish() + return scan.findings +} + +type cppDataScan struct { + env support.Context + file string + rules core.DataRulesConfig + writeLines []int + publishLines []int + consumerLine int + hasTx bool + hasOutbox bool + hasDedupe bool + findings []core.Finding +} + +func (s *cppDataScan) consumeLine(lineNo int, line string) { + if cppDataTx.MatchString(line) { + s.hasTx = true + } + if cppDataOutbox.MatchString(line) { + s.hasOutbox = true + } + if cppDataDedupe.MatchString(line) { + s.hasDedupe = true + } + if cppDataWrite.MatchString(line) { + s.writeLines = append(s.writeLines, lineNo) + } + if cppDataPublish.MatchString(line) { + s.publishLines = append(s.publishLines, lineNo) + } + if enabled(s.rules.DetectUnstablePagination) && cppDataOffset.MatchString(line) && !cppDataOrder.MatchString(line) { + s.add("data.unstable-pagination", "warn", lineNo, "C++ query uses offset pagination without deterministic ordering", "high", "query", "offset-without-order") + } + if enabled(s.rules.DetectUnboundedRead) && cppDataRead.MatchString(line) && !cppDataLimit.MatchString(line) { + s.add("data.unbounded-read", "warn", lineNo, "C++ database read has no visible limit, stream, cursor, or bounded filter", "medium", "query", "unbounded-read") + } + if cppDataConsumer.MatchString(line) { + s.consumerLine = lineNo + } + if enabled(s.rules.DetectCacheWithoutPolicy) && cppDataCacheSet.MatchString(line) && !cppDataTTL.MatchString(line) { + s.add("data.cache-without-policy", "warn", lineNo, "C++ cache write lacks TTL or expiration policy evidence", "medium", "cache", "set-without-ttl") + } +} + +func (s *cppDataScan) consumeRawSource(source string) { + if !enabled(s.rules.DetectExactlyOnceAssumption) { + return + } + for idx, line := range strings.Split(source, "\n") { + if strings.Contains(strings.ToLower(line), "exactly once") && !cppDataDedupe.MatchString(line) { + s.add("data.exactly-once-assumption", "warn", idx+1, "C++ code assumes exactly-once delivery without idempotency evidence", "low", "comment", "exactly-once") + } + } +} + +func (s *cppDataScan) finish() { + if len(s.writeLines) > s.rules.MaxWritesWithoutTransaction && !s.hasTx { + s.add("data.missing-transaction-boundary", "fail", s.writeLines[0], "C++ code performs multiple persistence writes without transaction evidence", "medium", "writes", "multiple") + } + if len(s.writeLines) > 0 && len(s.publishLines) > 0 && !s.hasOutbox { + if enabled(s.rules.DetectUnsafeDualWrite) { + s.add("data.unsafe-dual-write", "fail", s.writeLines[0], "C++ code writes state and publishes/sends work without a consistency strategy", "medium", "pattern", "write-plus-side-effect") + } + if enabled(s.rules.DetectMissingOutboxStrategy) { + s.add("data.missing-outbox-strategy", "fail", s.publishLines[0], "C++ state write is paired with publish/send without outbox evidence", "medium", "pattern", "missing-outbox") + } + } + if s.consumerLine > 0 && len(s.publishLines) > 0 && !s.hasDedupe { + if enabled(s.rules.DetectNonIdempotentConsumer) { + s.add("data.non-idempotent-consumer", "fail", s.consumerLine, "C++ consumer performs side effects without idempotency evidence", "medium", "consumer", "handler") + } + if enabled(s.rules.DetectMissingDeduplication) { + s.add("data.missing-deduplication", "warn", s.consumerLine, "C++ consumer has no visible deduplication guard", "medium", "consumer", "handler") + } + } +} + +func (s *cppDataScan) add(ruleID string, level string, lineNo int, message string, confidence string, metaKey string, metaValue string) { + s.findings = append(s.findings, newFinding(s.env, ruleID, level, s.file, lineNo, 1, message, confidence, metaKey, metaValue)) +} diff --git a/internal/codeguard/checks/data/data_go.go b/internal/codeguard/checks/data/data_go.go new file mode 100644 index 0000000..86628c2 --- /dev/null +++ b/internal/codeguard/checks/data/data_go.go @@ -0,0 +1,267 @@ +package data + +import ( + "fmt" + "go/ast" + "go/token" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func goFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + fset, parsed, err := support.ParseGoSource(env, file, data) + if err != nil { + return nil + } + rules := env.Config.Checks.DataRules + findings := make([]core.Finding, 0) + findings = append(findings, sqlTextFindings(env, file, fset, parsed, rules)...) + findings = append(findings, commentFindings(env, file, fset, parsed, rules)...) + + ast.Inspect(parsed, func(node ast.Node) bool { + fn, ok := node.(*ast.FuncDecl) + if !ok || fn.Body == nil { + return true + } + findings = append(findings, functionDataFindings(env, file, fset, fn, rules)...) + return false + }) + + return support.DedupeFindings(findings, func(finding core.Finding) string { + return finding.RuleID + "|" + finding.Path + "|" + fmt.Sprintf("%d", finding.Line) + "|" + finding.Message + }) +} + +func functionDataFindings(env support.Context, file string, fset *token.FileSet, fn *ast.FuncDecl, rules core.DataRulesConfig) []core.Finding { + summary := summarizeFunction(fn.Body) + findings := make([]core.Finding, 0) + pos := fset.Position(fn.Pos()) + inTransaction := summary.transactionCalls > 0 + + if enabled(rules.DetectReadModifyWriteRace) && summary.readCalls > 0 && summary.writeCalls > 0 && !inTransaction { + findings = append(findings, newFinding(env, "data.read-modify-write-race", "fail", file, pos.Line, pos.Column, "function reads state and writes derived state without a transaction or atomic update boundary", "medium", "pattern", "read-modify-write")) + } + if enabled(rules.DetectMissingTransaction) && rules.MaxWritesWithoutTransaction >= 0 && summary.writeCalls > rules.MaxWritesWithoutTransaction && !inTransaction { + findings = append(findings, newFinding(env, "data.missing-transaction-boundary", "fail", file, pos.Line, pos.Column, fmt.Sprintf("function performs %d persistence writes without an obvious transaction boundary", summary.writeCalls), "medium", "writes", fmt.Sprintf("%d", summary.writeCalls))) + } + if enabled(rules.DetectUnsafeDualWrite) && summary.writeCalls > 0 && summary.sideEffectCalls > 0 && !summary.hasOutbox { + findings = append(findings, newFinding(env, "data.unsafe-dual-write", "fail", file, pos.Line, pos.Column, "function writes state and performs an external side effect without an obvious consistency strategy", "medium", "pattern", "write-plus-side-effect")) + } + if enabled(rules.DetectMissingOutboxStrategy) && summary.writeCalls > 0 && summary.publishCalls > 0 && !summary.hasOutbox { + findings = append(findings, newFinding(env, "data.missing-outbox-strategy", "fail", file, pos.Line, pos.Column, "state write is paired with event publishing without outbox evidence", "medium", "pattern", "write-plus-publish")) + } + if enabled(rules.DetectNonIdempotentConsumer) && looksLikeConsumer(fn) && summary.sideEffectCalls > 0 && !summary.hasIdempotency { + findings = append(findings, newFinding(env, "data.non-idempotent-consumer", "fail", file, pos.Line, pos.Column, "message or event handler performs side effects without idempotency evidence", "medium", "consumer", fn.Name.Name)) + } + if enabled(rules.DetectMissingDeduplication) && looksLikeConsumer(fn) && !summary.hasIdempotency { + findings = append(findings, newFinding(env, "data.missing-deduplication", "warn", file, pos.Line, pos.Column, "message or event handler has no visible deduplication guard", "medium", "consumer", fn.Name.Name)) + } + if enabled(rules.DetectCacheWithoutPolicy) && summary.cacheSetWithoutTTL > 0 { + findings = append(findings, newFinding(env, "data.cache-without-policy", "warn", file, pos.Line, pos.Column, "cache writes lack visible TTL, invalidation, or ownership policy", "medium", "cache_sets", fmt.Sprintf("%d", summary.cacheSetWithoutTTL))) + } + if enabled(rules.DetectSideEffectInTransaction) { + findings = append(findings, sideEffectsInTransactions(env, file, fset, fn.Body)...) + } + return findings +} + +type functionSummary struct { + readCalls int + writeCalls int + publishCalls int + sideEffectCalls int + transactionCalls int + cacheSetWithoutTTL int + hasOutbox bool + hasIdempotency bool +} + +func summarizeFunction(block *ast.BlockStmt) functionSummary { + var summary functionSummary + ast.Inspect(block, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + name := strings.ToLower(callName(call)) + switch { + case containsAny(name, "transaction", "withtx", "intx", "begintx", ".tx"): + summary.transactionCalls++ + case containsAny(name, "get", "select", "query", "find", "load", "read"): + summary.readCalls++ + } + if containsAny(name, "create", "update", "delete", "save", "insert", "upsert", "exec") { + summary.writeCalls++ + } + if containsAny(name, "publish", "emit", "enqueue") { + summary.publishCalls++ + } + if containsAny(name, "publish", "emit", "enqueue", "send", "post", "put", "patch", "delete", "charge", "email", "webhook") { + summary.sideEffectCalls++ + } + if containsAny(name, "outbox") { + summary.hasOutbox = true + } + if containsAny(name, "idempot", "dedupe", "dedup", "processed", "messageid", "eventid") { + summary.hasIdempotency = true + } + if isCacheSetWithoutTTL(call) { + summary.cacheSetWithoutTTL++ + } + return true + }) + return summary +} + +func sideEffectsInTransactions(env support.Context, file string, fset *token.FileSet, block *ast.BlockStmt) []core.Finding { + findings := make([]core.Finding, 0) + ast.Inspect(block, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok || !looksLikeTransactionCall(call) { + return true + } + for _, arg := range call.Args { + fn, ok := arg.(*ast.FuncLit) + if !ok || fn.Body == nil { + continue + } + if blockHasExternalSideEffect(fn.Body) { + pos := fset.Position(call.Pos()) + findings = append(findings, newFinding(env, "data.side-effect-in-transaction", "fail", file, pos.Line, pos.Column, "transaction callback performs an external side effect that may not roll back safely", "high", "transaction", callName(call))) + } + } + return true + }) + return findings +} + +func sqlTextFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File, rules core.DataRulesConfig) []core.Finding { + findings := make([]core.Finding, 0) + ast.Inspect(parsed, func(node ast.Node) bool { + lit, ok := node.(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + return true + } + text := strings.ToLower(strings.Trim(lit.Value, "`\"")) + if enabled(rules.DetectUnstablePagination) && strings.Contains(text, " limit ") && strings.Contains(text, " offset ") && !strings.Contains(text, " order by ") { + pos := fset.Position(lit.Pos()) + findings = append(findings, newFinding(env, "data.unstable-pagination", "warn", file, pos.Line, pos.Column, "SQL pagination uses LIMIT/OFFSET without deterministic ORDER BY", "high", "query", "limit-offset")) + } + if enabled(rules.DetectUnboundedRead) && strings.Contains(text, "select ") && !strings.Contains(text, " limit ") && !strings.Contains(text, " where ") { + pos := fset.Position(lit.Pos()) + findings = append(findings, newFinding(env, "data.unbounded-read", "warn", file, pos.Line, pos.Column, "SQL read has no visible WHERE, LIMIT, cursor, or streaming bound", "medium", "query", "select-without-bound")) + } + return true + }) + return findings +} + +func commentFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File, rules core.DataRulesConfig) []core.Finding { + if !enabled(rules.DetectExactlyOnceAssumption) { + return nil + } + findings := make([]core.Finding, 0) + for _, group := range parsed.Comments { + text := strings.ToLower(group.Text()) + if strings.Contains(text, "exactly once") && !containsAny(text, "idempot", "dedupe", "dedup") { + pos := fset.Position(group.Pos()) + findings = append(findings, newFinding(env, "data.exactly-once-assumption", "warn", file, pos.Line, pos.Column, "comment assumes exactly-once delivery without idempotency or deduplication evidence", "low", "comment", "exactly-once")) + } + } + return findings +} + +func newFinding(env support.Context, ruleID string, level string, path string, line int, column int, message string, confidence string, metaKey string, metaValue string) core.Finding { + return env.NewFinding(support.FindingInput{ + RuleID: ruleID, + Level: level, + Path: path, + Line: line, + Column: column, + Message: message, + Confidence: confidence, + Metadata: map[string]string{metaKey: metaValue}, + }) +} + +func looksLikeConsumer(fn *ast.FuncDecl) bool { + name := strings.ToLower(fn.Name.Name) + if containsAny(name, "handle", "consume", "process", "onevent", "onmessage") { + return true + } + if fn.Type == nil || fn.Type.Params == nil { + return false + } + for _, field := range fn.Type.Params.List { + if containsAny(strings.ToLower(exprString(field.Type)), "event", "message", "msg") { + return true + } + } + return false +} + +func looksLikeTransactionCall(call *ast.CallExpr) bool { + name := strings.ToLower(callName(call)) + return containsAny(name, "transaction", "withtx", "intx", "begintx") +} + +func blockHasExternalSideEffect(block *ast.BlockStmt) bool { + found := false + ast.Inspect(block, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + if containsAny(strings.ToLower(callName(call)), "publish", "emit", "send", "post", "charge", "email", "webhook") { + found = true + return false + } + return true + }) + return found +} + +func isCacheSetWithoutTTL(call *ast.CallExpr) bool { + name := strings.ToLower(callName(call)) + if !strings.Contains(name, "cache") || !strings.HasSuffix(name, ".set") { + return false + } + if len(call.Args) >= 3 { + return false + } + return !containsAny(name, "ttl", "expire") +} + +func containsAny(value string, tokens ...string) bool { + for _, token := range tokens { + if strings.Contains(value, token) { + return true + } + } + return false +} + +func callName(call *ast.CallExpr) string { + return exprString(call.Fun) +} + +func exprString(expr ast.Expr) string { + switch n := expr.(type) { + case *ast.Ident: + return n.Name + case *ast.SelectorExpr: + left := exprString(n.X) + if left == "" { + return n.Sel.Name + } + return left + "." + n.Sel.Name + case *ast.StarExpr: + return "*" + exprString(n.X) + case *ast.CallExpr: + return exprString(n.Fun) + default: + return fmt.Sprintf("%T", expr) + } +} diff --git a/internal/codeguard/checks/data/data_python.go b/internal/codeguard/checks/data/data_python.go new file mode 100644 index 0000000..4ef4094 --- /dev/null +++ b/internal/codeguard/checks/data/data_python.go @@ -0,0 +1,111 @@ +package data + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + pySelectQuery = regexp.MustCompile(`(?i)(?:select\s+\*|\.query\s*\(|\.execute\s*\()`) + pyLimitOffset = regexp.MustCompile(`(?i)limit\s+\S+\s+offset|\.offset\s*\(`) + pyOrderBy = regexp.MustCompile(`(?i)order\s+by|\.order_by\s*\(`) + pyLimitBound = regexp.MustCompile(`(?i)limit\s+\S+|\.limit\s*\(|yield_per\s*\(|stream\s*=`) + pyWriteCall = regexp.MustCompile(`(?i)\.(?:create|update|delete|save|insert|upsert|bulk_create|bulk_update)\s*\(`) + pyPublishCall = regexp.MustCompile(`(?i)\.(?:publish|emit|send|enqueue|delay|apply_async)\s*\(`) + pyTransactionHint = regexp.MustCompile(`(?i)transaction|atomic|begin_nested|with_for_update`) + pyOutboxHint = regexp.MustCompile(`(?i)outbox`) + pyDedupeHint = regexp.MustCompile(`(?i)idempot|dedupe|dedup|processed|message_id|event_id`) + pyConsumerDef = regexp.MustCompile(`(?i)^\s*(?:async\s+)?def\s+(?:handle|consume|process|on_)\w*`) + pyCacheSet = regexp.MustCompile(`(?i)\bcache\.(?:set|put)\s*\(`) + pyTTLHint = regexp.MustCompile(`(?i)ttl|timeout|expire`) +) + +func pythonFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + source := strings.ReplaceAll(string(data), "\r\n", "\n") + scan := &pythonDataScan{ + env: env, + file: file, + rules: env.Config.Checks.DataRules, + } + for idx, line := range strings.Split(source, "\n") { + scan.consumeLine(idx+1, line) + } + scan.finish() + return scan.findings +} + +type pythonDataScan struct { + env support.Context + file string + rules core.DataRulesConfig + writeLines []int + publishLines []int + consumerLine int + hasTx bool + hasOutbox bool + hasDedupe bool + findings []core.Finding +} + +func (s *pythonDataScan) consumeLine(lineNo int, line string) { + lower := strings.ToLower(line) + if pyTransactionHint.MatchString(line) { + s.hasTx = true + } + if pyOutboxHint.MatchString(line) { + s.hasOutbox = true + } + if pyDedupeHint.MatchString(line) { + s.hasDedupe = true + } + if pyWriteCall.MatchString(line) { + s.writeLines = append(s.writeLines, lineNo) + } + if pyPublishCall.MatchString(line) { + s.publishLines = append(s.publishLines, lineNo) + } + if enabled(s.rules.DetectUnstablePagination) && pyLimitOffset.MatchString(line) && !pyOrderBy.MatchString(line) { + s.add("data.unstable-pagination", "warn", lineNo, "Python query paginates with offset without deterministic ordering", "high", "query", "offset-without-order") + } + if enabled(s.rules.DetectUnboundedRead) && pySelectQuery.MatchString(line) && !pyLimitBound.MatchString(line) && !strings.Contains(lower, "where ") { + s.add("data.unbounded-read", "warn", lineNo, "Python database read has no visible limit, stream, cursor, or bounded filter", "medium", "query", "unbounded-read") + } + if pyConsumerDef.MatchString(line) { + s.consumerLine = lineNo + } + if enabled(s.rules.DetectCacheWithoutPolicy) && pyCacheSet.MatchString(line) && !pyTTLHint.MatchString(line) { + s.add("data.cache-without-policy", "warn", lineNo, "Python cache write lacks TTL or expiration policy evidence", "medium", "cache", "set-without-ttl") + } + if enabled(s.rules.DetectExactlyOnceAssumption) && strings.Contains(lower, "exactly once") && !pyDedupeHint.MatchString(line) { + s.add("data.exactly-once-assumption", "warn", lineNo, "Python code assumes exactly-once delivery without idempotency or deduplication evidence", "low", "comment", "exactly-once") + } +} + +func (s *pythonDataScan) finish() { + if len(s.writeLines) > s.rules.MaxWritesWithoutTransaction && !s.hasTx { + s.add("data.missing-transaction-boundary", "fail", s.writeLines[0], "Python code performs multiple persistence writes without transaction evidence", "medium", "writes", "multiple") + } + if len(s.writeLines) > 0 && len(s.publishLines) > 0 && !s.hasOutbox { + if enabled(s.rules.DetectUnsafeDualWrite) { + s.add("data.unsafe-dual-write", "fail", s.writeLines[0], "Python code writes state and publishes/sends work without a consistency strategy", "medium", "pattern", "write-plus-side-effect") + } + if enabled(s.rules.DetectMissingOutboxStrategy) { + s.add("data.missing-outbox-strategy", "fail", s.publishLines[0], "Python state write is paired with publish/send without outbox evidence", "medium", "pattern", "missing-outbox") + } + } + if s.consumerLine > 0 && len(s.publishLines) > 0 && !s.hasDedupe { + if enabled(s.rules.DetectNonIdempotentConsumer) { + s.add("data.non-idempotent-consumer", "fail", s.consumerLine, "Python consumer performs side effects without idempotency evidence", "medium", "consumer", "handler") + } + if enabled(s.rules.DetectMissingDeduplication) { + s.add("data.missing-deduplication", "warn", s.consumerLine, "Python consumer has no visible deduplication guard", "medium", "consumer", "handler") + } + } +} + +func (s *pythonDataScan) add(ruleID string, level string, lineNo int, message string, confidence string, metaKey string, metaValue string) { + s.findings = append(s.findings, newFinding(s.env, ruleID, level, s.file, lineNo, 1, message, confidence, metaKey, metaValue)) +} diff --git a/internal/codeguard/checks/data/data_typescript.go b/internal/codeguard/checks/data/data_typescript.go new file mode 100644 index 0000000..75d5e2f --- /dev/null +++ b/internal/codeguard/checks/data/data_typescript.go @@ -0,0 +1,125 @@ +package data + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + tsDataRead = regexp.MustCompile(`(?i)\.(?:findMany|findFirst|findUnique|query|execute|select)\s*\(`) + tsDataLimitOffset = regexp.MustCompile(`(?i)\b(?:skip|offset)\s*:`) + tsDataOrder = regexp.MustCompile(`(?i)\borderBy\s*:|order\s+by`) + tsDataLimit = regexp.MustCompile(`(?i)\b(?:take|limit)\s*:|limit\s+\d+|cursor\s*:`) + tsDataWrite = regexp.MustCompile(`(?i)\.(?:create|update|delete|upsert|insert|save)\s*\(`) + tsDataPublish = regexp.MustCompile(`(?i)\.(?:publish|emit|send|enqueue|dispatch)\s*\(`) + tsDataTx = regexp.MustCompile(`(?i)transaction|\$transaction|withTransaction`) + tsDataOutbox = regexp.MustCompile(`(?i)outbox`) + tsDataDedupe = regexp.MustCompile(`(?i)idempot|dedupe|dedup|processed|messageId|eventId`) + tsDataConsumer = regexp.MustCompile(`(?i)\b(?:handle|consume|process|onMessage|onEvent)\w*\s*\(`) + tsDataCacheSet = regexp.MustCompile(`(?i)\bcache\.(?:set|put)\s*\(`) + tsDataTTL = regexp.MustCompile(`(?i)ttl|expires|expire|maxAge`) + tsExactlyOnce = regexp.MustCompile(`(?i)exactly once`) +) + +func typeScriptTargetFindings(env support.Context, target core.TargetConfig) []core.Finding { + findings := make([]core.Finding, 0) + env.VisitTargetFiles(target, support.IsTypeScriptLikeFile, func(rel string, data []byte) { + findings = append(findings, typeScriptFindingsForFile(env, rel, data)...) + }) + return findings +} + +func typeScriptFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + source := strings.ReplaceAll(string(data), "\r\n", "\n") + code := support.StripTypeScriptCommentsAndStrings(source) + scan := &tsDataScan{env: env, file: file, rules: env.Config.Checks.DataRules} + for idx, line := range strings.Split(code, "\n") { + scan.consumeLine(idx+1, line) + } + scan.consumeRawSource(source) + scan.finish() + return scan.findings +} + +type tsDataScan struct { + env support.Context + file string + rules core.DataRulesConfig + writeLines []int + publishLines []int + consumerLine int + hasTx bool + hasOutbox bool + hasDedupe bool + findings []core.Finding +} + +func (s *tsDataScan) consumeLine(lineNo int, line string) { + if tsDataTx.MatchString(line) { + s.hasTx = true + } + if tsDataOutbox.MatchString(line) { + s.hasOutbox = true + } + if tsDataDedupe.MatchString(line) { + s.hasDedupe = true + } + if tsDataWrite.MatchString(line) { + s.writeLines = append(s.writeLines, lineNo) + } + if tsDataPublish.MatchString(line) { + s.publishLines = append(s.publishLines, lineNo) + } + if enabled(s.rules.DetectUnstablePagination) && tsDataLimitOffset.MatchString(line) && !tsDataOrder.MatchString(line) { + s.add("data.unstable-pagination", "warn", lineNo, "TypeScript/JavaScript query uses offset pagination without deterministic ordering", "high", "query", "offset-without-order") + } + if enabled(s.rules.DetectUnboundedRead) && tsDataRead.MatchString(line) && !tsDataLimit.MatchString(line) { + s.add("data.unbounded-read", "warn", lineNo, "TypeScript/JavaScript database read has no visible limit or cursor bound", "medium", "query", "unbounded-read") + } + if tsDataConsumer.MatchString(line) { + s.consumerLine = lineNo + } + if enabled(s.rules.DetectCacheWithoutPolicy) && tsDataCacheSet.MatchString(line) && !tsDataTTL.MatchString(line) { + s.add("data.cache-without-policy", "warn", lineNo, "TypeScript/JavaScript cache write lacks TTL or expiration policy evidence", "medium", "cache", "set-without-ttl") + } +} + +func (s *tsDataScan) consumeRawSource(source string) { + if !enabled(s.rules.DetectExactlyOnceAssumption) { + return + } + for idx, line := range strings.Split(source, "\n") { + if tsExactlyOnce.MatchString(line) && !tsDataDedupe.MatchString(line) { + s.add("data.exactly-once-assumption", "warn", idx+1, "TypeScript/JavaScript code assumes exactly-once delivery without idempotency evidence", "low", "comment", "exactly-once") + } + } +} + +func (s *tsDataScan) finish() { + if len(s.writeLines) > s.rules.MaxWritesWithoutTransaction && !s.hasTx { + s.add("data.missing-transaction-boundary", "fail", s.writeLines[0], "TypeScript/JavaScript code performs multiple persistence writes without transaction evidence", "medium", "writes", "multiple") + } + if len(s.writeLines) > 0 && len(s.publishLines) > 0 && !s.hasOutbox { + if enabled(s.rules.DetectUnsafeDualWrite) { + s.add("data.unsafe-dual-write", "fail", s.writeLines[0], "TypeScript/JavaScript code writes state and publishes/sends work without a consistency strategy", "medium", "pattern", "write-plus-side-effect") + } + if enabled(s.rules.DetectMissingOutboxStrategy) { + s.add("data.missing-outbox-strategy", "fail", s.publishLines[0], "TypeScript/JavaScript state write is paired with publish/send without outbox evidence", "medium", "pattern", "missing-outbox") + } + } + if s.consumerLine > 0 && len(s.publishLines) > 0 && !s.hasDedupe { + if enabled(s.rules.DetectNonIdempotentConsumer) { + s.add("data.non-idempotent-consumer", "fail", s.consumerLine, "TypeScript/JavaScript consumer performs side effects without idempotency evidence", "medium", "consumer", "handler") + } + if enabled(s.rules.DetectMissingDeduplication) { + s.add("data.missing-deduplication", "warn", s.consumerLine, "TypeScript/JavaScript consumer has no visible deduplication guard", "medium", "consumer", "handler") + } + } +} + +func (s *tsDataScan) add(ruleID string, level string, lineNo int, message string, confidence string, metaKey string, metaValue string) { + s.findings = append(s.findings, newFinding(s.env, ruleID, level, s.file, lineNo, 1, message, confidence, metaKey, metaValue)) +} diff --git a/internal/codeguard/checks/reliability/reliability.go b/internal/codeguard/checks/reliability/reliability.go new file mode 100644 index 0000000..3493113 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability.go @@ -0,0 +1,40 @@ +// Package reliability implements production reliability checks: bounded +// outbound calls, cancellation propagation, bounded work, cleanup, and +// recoverable error handling. +package reliability + +import ( + "context" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func Run(ctx context.Context, env support.Context) core.SectionResult { + return support.RunTargetSection(ctx, env, "reliability", "Reliability", reliabilityTargetFindings) +} + +func reliabilityTargetFindings(_ context.Context, env support.Context, target core.TargetConfig) []core.Finding { + switch support.NormalizedLanguage(target.Language) { + case "go", "golang": + return support.ScanGoFiles(env, target, "reliability", func(file string, data []byte) []core.Finding { + return goFindingsForFile(env, file, data) + }) + case "python", "py": + return support.ScanPythonFiles(env, target, "reliability", func(file string, data []byte) []core.Finding { + return pythonFindingsForFile(env, file, data) + }) + case "typescript", "javascript", "ts", "tsx", "js", "jsx": + return typeScriptTargetFindings(env, target) + case "c++", "cpp", "cxx", "cc": + return support.ScanCPPFiles(env, target, "reliability", func(file string, data []byte) []core.Finding { + return cppFindingsForFile(env, file, data) + }) + default: + return nil + } +} + +func enabled(toggle *bool) bool { + return toggle == nil || *toggle +} diff --git a/internal/codeguard/checks/reliability/reliability_cpp.go b/internal/codeguard/checks/reliability/reliability_cpp.go new file mode 100644 index 0000000..238c24a --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_cpp.go @@ -0,0 +1,113 @@ +package reliability + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + cppLoopStart = regexp.MustCompile(`(?:^|[^\w])(?:for|while)\b`) + cppUnboundedLoop = regexp.MustCompile(`\bwhile\s*\(\s*true\s*\)|\bfor\s*\(\s*;\s*;\s*\)`) + cppRetryHint = regexp.MustCompile(`(?i)retry|attempt|transient`) + cppBackoffHint = regexp.MustCompile(`(?i)sleep_for|sleep_until|backoff|jitter`) + cppThreadLaunch = regexp.MustCompile(`\bstd::(?:thread|jthread|async)\s*\(`) + cppConcurrencyLimit = regexp.MustCompile(`semaphore|latch|barrier|thread_pool|executor|queue`) + cppRawNew = regexp.MustCompile(`\bnew\s+[A-Za-z_:]\w*`) + cppDeleteCall = regexp.MustCompile(`\bdelete\s+`) + cppThrowRuntime = regexp.MustCompile(`\bthrow\s+std::(?:runtime_error|exception)\s*\(`) + cppNonIdempotent = regexp.MustCompile(`(?i)\b(?:post|put|patch|delete|create|update|save|insert|publish|send|charge|write)\w*\s*\(`) + cppIdempotency = regexp.MustCompile(`(?i)idempot|dedupe|dedup|processed|message_id|event_id`) +) + +func cppFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + source := strings.ReplaceAll(string(data), "\r\n", "\n") + masked := support.MaskCLikeSource(source, support.CLikeCPP) + scan := &cppReliabilityScan{ + env: env, + file: file, + rules: env.Config.Checks.ReliabilityRules, + limited: cppConcurrencyLimit.MatchString(masked), + } + for idx, line := range strings.Split(masked, "\n") { + scan.consumeLine(idx+1, line) + } + return scan.findings +} + +type cppReliabilityScan struct { + env support.Context + file string + rules core.ReliabilityRulesConfig + limited bool + depth int + loops []int + unboundedLoops []int + newLine int + findings []core.Finding +} + +func (s *cppReliabilityScan) consumeLine(lineNo int, line string) { + startsLoop := cppLoopStart.MatchString(line) + inLoop := len(s.loops) > 0 || startsLoop + inUnboundedLoop := len(s.unboundedLoops) > 0 || cppUnboundedLoop.MatchString(line) + s.checkLine(lineNo, line, inLoop, inUnboundedLoop) + next := s.depth + strings.Count(line, "{") - strings.Count(line, "}") + if startsLoop && next > s.depth { + s.loops = append(s.loops, s.depth) + if cppUnboundedLoop.MatchString(line) { + s.unboundedLoops = append(s.unboundedLoops, s.depth) + } + } + for len(s.loops) > 0 && next <= s.loops[len(s.loops)-1] { + s.loops = s.loops[:len(s.loops)-1] + } + for len(s.unboundedLoops) > 0 && next <= s.unboundedLoops[len(s.unboundedLoops)-1] { + s.unboundedLoops = s.unboundedLoops[:len(s.unboundedLoops)-1] + } + s.depth = next +} + +func (s *cppReliabilityScan) checkLine(lineNo int, line string, inLoop bool, inUnboundedLoop bool) { + if enabled(s.rules.DetectRetryWithoutBackoff) && inLoop && cppRetryHint.MatchString(line) && !cppBackoffHint.MatchString(line) { + s.add("reliability.retry-without-backoff", "warn", lineNo, "retry-like C++ loop has no visible backoff or jitter", "medium", "retry", "no-backoff") + } + if enabled(s.rules.DetectUnboundedRetry) && inUnboundedLoop && cppRetryHint.MatchString(line) { + s.add("reliability.unbounded-retry", "fail", lineNo, "retry-like C++ loop can run forever without an attempt limit", "medium", "retry", "while-true") + } + if enabled(s.rules.DetectNonIdempotentRetry) && inLoop && cppNonIdempotent.MatchString(line) && !cppIdempotency.MatchString(line) { + s.add("reliability.non-idempotent-retry", "fail", lineNo, "retry-like C++ loop wraps a non-idempotent side effect without idempotency evidence", "medium", "retry", "side-effect") + } + if enabled(s.rules.DetectUnboundedWork) && inLoop && !s.limited && cppThreadLaunch.MatchString(line) { + s.add("reliability.unbounded-work", "warn", lineNo, "C++ thread/task is launched inside a loop without a visible concurrency bound", "high", "work", "thread-in-loop") + } + if enabled(s.rules.DetectRecoverablePanic) && cppThrowRuntime.MatchString(line) { + s.add("reliability.recoverable-panic", "fail", lineNo, "production C++ code throws a generic runtime exception for a recoverable failure path", "medium", "exception", "runtime-error") + } + if enabled(s.rules.DetectResourceLeak) { + s.trackRawNew(lineNo, line) + } +} + +func (s *cppReliabilityScan) trackRawNew(lineNo int, line string) { + if strings.Contains(line, "unique_ptr") || strings.Contains(line, "shared_ptr") || strings.Contains(line, "make_unique") || strings.Contains(line, "make_shared") { + return + } + if cppRawNew.MatchString(line) { + s.newLine = lineNo + return + } + if s.newLine > 0 && cppDeleteCall.MatchString(line) { + s.newLine = 0 + } + if s.newLine > 0 && lineNo > s.newLine+8 { + s.add("reliability.resource-leak", "fail", s.newLine, "raw C++ allocation has no nearby delete or smart-pointer ownership evidence", "medium", "resource", "raw-new") + s.newLine = 0 + } +} + +func (s *cppReliabilityScan) add(ruleID string, level string, lineNo int, message string, confidence string, metaKey string, metaValue string) { + s.findings = append(s.findings, newFinding(s.env, ruleID, level, s.file, lineNo, 1, message, confidence, metaKey, metaValue)) +} diff --git a/internal/codeguard/checks/reliability/reliability_go.go b/internal/codeguard/checks/reliability/reliability_go.go new file mode 100644 index 0000000..1202577 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_go.go @@ -0,0 +1,156 @@ +package reliability + +import ( + "fmt" + "go/ast" + "go/token" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func goFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + fset, parsed, err := support.ParseGoSource(env, file, data) + if err != nil { + return nil + } + rules := env.Config.Checks.ReliabilityRules + findings := make([]core.Finding, 0) + httpAliases := importAliases(parsed, "net/http") + hasShutdown := fileHasSelector(parsed, "Shutdown") || fileHasSelector(parsed, "NotifyContext") || fileHasSelector(parsed, "Notify") + + ast.Inspect(parsed, func(node ast.Node) bool { + if n, ok := node.(*ast.FuncDecl); ok { + findings = append(findings, functionReliabilityFindings(env, file, fset, n, rules, httpAliases, hasShutdown)...) + return false + } + return true + }) + + return support.DedupeFindings(findings, func(finding core.Finding) string { + return finding.RuleID + "|" + finding.Path + "|" + fmt.Sprintf("%d", finding.Line) + "|" + finding.Message + }) +} + +func functionReliabilityFindings(env support.Context, file string, fset *token.FileSet, fn *ast.FuncDecl, rules core.ReliabilityRulesConfig, httpAliases map[string]struct{}, hasShutdown bool) []core.Finding { + if fn.Body == nil { + return nil + } + findings := make([]core.Finding, 0) + hasContextParam := funcHasContextParam(fn) + deferCloseVars := deferredCloseVars(fn.Body) + boundedClients, boundedRequests := boundedHTTPValues(fn.Body, httpAliases) + goroutines := 0 + + ast.Inspect(fn.Body, func(node ast.Node) bool { + switch n := node.(type) { + case *ast.GoStmt: + goroutines++ + if enabled(rules.DetectUnboundedWork) && isInsideLoop(fn.Body, n) { + pos := fset.Position(n.Go) + findings = append(findings, newFinding(env, "reliability.unbounded-work", "warn", file, pos.Line, pos.Column, "goroutine launched inside a loop without an obvious work bound", "high", "work", "goroutine-in-loop")) + } + case *ast.ForStmt: + findings = append(findings, retryFindings(env, file, fset, n, rules)...) + case *ast.RangeStmt: + if enabled(rules.DetectRetryWithoutBackoff) && loopLooksLikeRetry(n.Body) && !blockHasBackoff(n.Body) { + pos := fset.Position(n.For) + findings = append(findings, newFinding(env, "reliability.retry-without-backoff", "warn", file, pos.Line, pos.Column, "retry-like loop has no visible backoff or jitter", "medium", "retry", "range-loop")) + } + case *ast.CallExpr: + findings = append(findings, callReliabilityFindings(env, file, fset, n, rules, httpAliases, boundedClients, boundedRequests, hasContextParam, hasShutdown)...) + case *ast.AssignStmt: + findings = append(findings, assignmentReliabilityFindings(env, file, fset, n, rules, httpAliases, deferCloseVars)...) + case *ast.ReturnStmt: + if enabled(rules.DetectLostErrorContext) { + findings = append(findings, lostErrorContextFindings(env, file, fset, n)...) + } + } + return true + }) + + if enabled(rules.DetectMissingConcurrencyLimit) && rules.MaxInlineGoroutinesPerFunction > 0 && goroutines > rules.MaxInlineGoroutinesPerFunction { + pos := fset.Position(fn.Pos()) + findings = append(findings, newFinding(env, "reliability.missing-concurrency-limit", "warn", file, pos.Line, pos.Column, fmt.Sprintf("function launches %d goroutines without an obvious concurrency limit", goroutines), "medium", "goroutines", fmt.Sprintf("%d", goroutines))) + } + + return findings +} + +func callReliabilityFindings(env support.Context, file string, fset *token.FileSet, call *ast.CallExpr, rules core.ReliabilityRulesConfig, httpAliases map[string]struct{}, boundedClients map[string]struct{}, boundedRequests map[string]struct{}, hasContextParam bool, hasShutdown bool) []core.Finding { + findings := make([]core.Finding, 0, 2) + pos := fset.Position(call.Pos()) + if enabled(rules.DetectMissingTimeout) && isUnboundedHTTPCall(call, httpAliases, boundedClients, boundedRequests) { + findings = append(findings, newFinding(env, "reliability.missing-timeout", "fail", file, pos.Line, pos.Column, "outbound HTTP call is made without a request context or client timeout", "high", "call", callName(call))) + } + if enabled(rules.DetectMissingCancellation) && hasContextParam && isBackgroundContextCall(call) { + findings = append(findings, newFinding(env, "reliability.missing-cancellation", "warn", file, pos.Line, pos.Column, "function with caller context creates detached background context for downstream work", "high", "context", callName(call))) + } + if enabled(rules.DetectRecoverablePanic) && isPanicCall(call) { + findings = append(findings, newFinding(env, "reliability.recoverable-panic", "fail", file, pos.Line, pos.Column, "production code uses panic for a recoverable failure path", "medium", "call", "panic")) + } + if enabled(rules.DetectMissingGracefulShutdown) && !hasShutdown && isListenAndServeCall(call, httpAliases) { + findings = append(findings, newFinding(env, "reliability.missing-graceful-shutdown", "warn", file, pos.Line, pos.Column, "server starts without visible signal handling or graceful shutdown", "medium", "server", callName(call))) + } + return findings +} + +func assignmentReliabilityFindings(env support.Context, file string, fset *token.FileSet, assign *ast.AssignStmt, rules core.ReliabilityRulesConfig, httpAliases map[string]struct{}, deferCloseVars map[string]struct{}) []core.Finding { + findings := make([]core.Finding, 0, 2) + if enabled(rules.DetectSwallowedError) && assignmentSwallowsCallError(assign) { + pos := fset.Position(assign.Pos()) + findings = append(findings, newFinding(env, "reliability.swallowed-error", "fail", file, pos.Line, pos.Column, "call result error is assigned to the blank identifier", "high", "assignment", "blank-error")) + } + if enabled(rules.DetectResourceLeak) { + for _, name := range assignedHTTPResponseVars(assign, httpAliases) { + if _, closed := deferCloseVars[name]; !closed { + pos := fset.Position(assign.Pos()) + findings = append(findings, newFinding(env, "reliability.resource-leak", "fail", file, pos.Line, pos.Column, "HTTP response body is not closed on the acquisition path", "high", "resource", "http-response-body")) + } + } + } + return findings +} + +func retryFindings(env support.Context, file string, fset *token.FileSet, loop *ast.ForStmt, rules core.ReliabilityRulesConfig) []core.Finding { + if !loopLooksLikeRetry(loop.Body) && loop.Cond != nil { + return nil + } + findings := make([]core.Finding, 0, 3) + pos := fset.Position(loop.For) + if enabled(rules.DetectUnboundedRetry) && loop.Cond == nil { + findings = append(findings, newFinding(env, "reliability.unbounded-retry", "fail", file, pos.Line, pos.Column, "retry-like loop has no condition limiting attempts", "high", "retry", "unbounded-for")) + } + if enabled(rules.DetectRetryWithoutBackoff) && !blockHasBackoff(loop.Body) { + findings = append(findings, newFinding(env, "reliability.retry-without-backoff", "warn", file, pos.Line, pos.Column, "retry-like loop has no visible backoff, sleep, ticker, or jitter", "medium", "retry", "no-backoff")) + } + if enabled(rules.DetectNonIdempotentRetry) && blockHasNonIdempotentCall(loop.Body) && !blockHasIdempotencyEvidence(loop.Body) { + findings = append(findings, newFinding(env, "reliability.non-idempotent-retry", "fail", file, pos.Line, pos.Column, "retry-like loop wraps a non-idempotent side effect without idempotency evidence", "medium", "retry", "side-effect")) + } + return findings +} + +func lostErrorContextFindings(env support.Context, file string, fset *token.FileSet, ret *ast.ReturnStmt) []core.Finding { + for _, result := range ret.Results { + call, ok := result.(*ast.CallExpr) + if !ok || !isErrorsNewCall(call) { + continue + } + pos := fset.Position(call.Pos()) + return []core.Finding{newFinding(env, "reliability.lost-error-context", "warn", file, pos.Line, pos.Column, "error is replaced with a new generic error instead of wrapping the original cause", "medium", "error", "errors.New")} + } + return nil +} + +func newFinding(env support.Context, ruleID string, level string, path string, line int, column int, message string, confidence string, metaKey string, metaValue string) core.Finding { + return env.NewFinding(support.FindingInput{ + RuleID: ruleID, + Level: level, + Path: path, + Line: line, + Column: column, + Message: message, + Confidence: confidence, + Metadata: map[string]string{metaKey: metaValue}, + }) +} diff --git a/internal/codeguard/checks/reliability/reliability_go_helpers.go b/internal/codeguard/checks/reliability/reliability_go_helpers.go new file mode 100644 index 0000000..03392e1 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_go_helpers.go @@ -0,0 +1,399 @@ +package reliability + +import ( + "fmt" + "go/ast" + "strings" +) + +func importAliases(parsed *ast.File, importPath string) map[string]struct{} { + aliases := map[string]struct{}{} + for _, spec := range parsed.Imports { + if strings.Trim(spec.Path.Value, "\"") != importPath { + continue + } + if spec.Name != nil { + aliases[spec.Name.Name] = struct{}{} + continue + } + parts := strings.Split(importPath, "/") + aliases[parts[len(parts)-1]] = struct{}{} + } + return aliases +} + +func funcHasContextParam(fn *ast.FuncDecl) bool { + if fn.Type == nil || fn.Type.Params == nil { + return false + } + for _, field := range fn.Type.Params.List { + if strings.Contains(exprString(field.Type), "context.Context") { + return true + } + } + return false +} + +func isUnboundedHTTPCall(call *ast.CallExpr, aliases map[string]struct{}, boundedClients map[string]struct{}, boundedRequests map[string]struct{}) bool { + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + ident, ok := selector.X.(*ast.Ident) + if ok { + if _, exists := aliases[ident.Name]; exists { + switch selector.Sel.Name { + case "Get", "Head", "Post", "PostForm": + return true + } + } + } + if selector.Sel.Name != "Do" { + return false + } + if len(call.Args) > 0 { + if arg, ok := call.Args[0].(*ast.Ident); ok { + if _, exists := boundedRequests[arg.Name]; exists { + return false + } + } + } + if ident != nil { + if _, exists := boundedClients[ident.Name]; exists { + return false + } + } + return true +} + +func isBackgroundContextCall(call *ast.CallExpr) bool { + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + ident, ok := selector.X.(*ast.Ident) + return ok && ident.Name == "context" && (selector.Sel.Name == "Background" || selector.Sel.Name == "TODO") +} + +func isPanicCall(call *ast.CallExpr) bool { + ident, ok := call.Fun.(*ast.Ident) + return ok && ident.Name == "panic" +} + +func isListenAndServeCall(call *ast.CallExpr, aliases map[string]struct{}) bool { + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + if selector.Sel.Name != "ListenAndServe" && selector.Sel.Name != "ListenAndServeTLS" { + return false + } + if ident, ok := selector.X.(*ast.Ident); ok { + _, exists := aliases[ident.Name] + return exists || ident.Name == "http" + } + return true +} + +func assignmentSwallowsCallError(assign *ast.AssignStmt) bool { + hasBlank := false + for _, lhs := range assign.Lhs { + if ident, ok := lhs.(*ast.Ident); ok && ident.Name == "_" { + hasBlank = true + } + } + if !hasBlank { + return false + } + for _, rhs := range assign.Rhs { + if _, ok := rhs.(*ast.CallExpr); ok { + return true + } + } + return false +} + +func assignedHTTPResponseVars(assign *ast.AssignStmt, aliases map[string]struct{}) []string { + names := make([]string, 0, 1) + for _, rhs := range assign.Rhs { + call, ok := rhs.(*ast.CallExpr) + if !ok || !isHTTPResponseAcquisition(call, aliases) { + continue + } + for _, lhs := range assign.Lhs { + ident, ok := lhs.(*ast.Ident) + if ok && ident.Name != "_" && strings.Contains(strings.ToLower(ident.Name), "resp") { + names = append(names, ident.Name) + } + } + } + return names +} + +func boundedHTTPValues(block *ast.BlockStmt, aliases map[string]struct{}) (map[string]struct{}, map[string]struct{}) { + clients := map[string]struct{}{} + requests := map[string]struct{}{} + ast.Inspect(block, func(node ast.Node) bool { + switch n := node.(type) { + case *ast.AssignStmt: + for idx, rhs := range n.Rhs { + name := assignedNameAt(n.Lhs, idx) + if name == "" { + continue + } + if isHTTPClientWithTimeout(rhs, aliases) { + clients[name] = struct{}{} + } + if isRequestWithContext(rhs) { + requests[name] = struct{}{} + } + } + case *ast.ValueSpec: + for idx, rhs := range n.Values { + if idx >= len(n.Names) { + continue + } + name := n.Names[idx].Name + if isHTTPClientWithTimeout(rhs, aliases) { + clients[name] = struct{}{} + } + if isRequestWithContext(rhs) { + requests[name] = struct{}{} + } + } + } + return true + }) + return clients, requests +} + +func assignedNameAt(lhs []ast.Expr, idx int) string { + if len(lhs) == 0 { + return "" + } + if idx >= len(lhs) { + idx = len(lhs) - 1 + } + ident, ok := lhs[idx].(*ast.Ident) + if !ok || ident.Name == "_" { + return "" + } + return ident.Name +} + +func isHTTPClientWithTimeout(expr ast.Expr, aliases map[string]struct{}) bool { + switch n := expr.(type) { + case *ast.UnaryExpr: + return isHTTPClientWithTimeout(n.X, aliases) + case *ast.CompositeLit: + if !isHTTPClientType(n.Type, aliases) { + return false + } + for _, elt := range n.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok { + continue + } + if key, ok := kv.Key.(*ast.Ident); ok && key.Name == "Timeout" { + return true + } + } + case *ast.CallExpr: + return callName(n) == "http.Client" || strings.HasSuffix(callName(n), ".Client") + } + return false +} + +func isHTTPClientType(expr ast.Expr, aliases map[string]struct{}) bool { + selector, ok := expr.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "Client" { + return false + } + ident, ok := selector.X.(*ast.Ident) + if !ok { + return false + } + if ident.Name == "http" { + return true + } + _, exists := aliases[ident.Name] + return exists +} + +func isRequestWithContext(expr ast.Expr) bool { + call, ok := expr.(*ast.CallExpr) + if !ok { + return false + } + name := callName(call) + return strings.HasSuffix(name, ".NewRequestWithContext") || strings.HasSuffix(name, ".WithContext") +} + +func isHTTPResponseAcquisition(call *ast.CallExpr, aliases map[string]struct{}) bool { + if isUnboundedHTTPCall(call, aliases, nil, nil) { + return true + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + return selector.Sel.Name == "Do" +} + +func deferredCloseVars(block *ast.BlockStmt) map[string]struct{} { + closed := map[string]struct{}{} + ast.Inspect(block, func(node ast.Node) bool { + deferStmt, ok := node.(*ast.DeferStmt) + if !ok { + return true + } + chain := selectorChain(deferStmt.Call) + if chain == "" { + return true + } + parts := strings.Split(chain, ".") + if len(parts) >= 3 && parts[len(parts)-2] == "Body" && parts[len(parts)-1] == "Close" { + closed[parts[0]] = struct{}{} + } + return true + }) + return closed +} + +func loopLooksLikeRetry(block *ast.BlockStmt) bool { + return blockHasNameToken(block, "retry", "attempt", "backoff", "transient") +} + +func blockHasBackoff(block *ast.BlockStmt) bool { + return blockHasNameToken(block, "sleep", "after", "ticker", "backoff", "jitter") +} + +func blockHasIdempotencyEvidence(block *ast.BlockStmt) bool { + return blockHasNameToken(block, "idempot", "dedupe", "dedup", "once", "processed") +} + +func blockHasNonIdempotentCall(block *ast.BlockStmt) bool { + found := false + ast.Inspect(block, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + name := strings.ToLower(callName(call)) + for _, token := range []string{"post", "put", "patch", "delete", "create", "update", "save", "insert", "publish", "send", "charge", "write"} { + if strings.Contains(name, token) { + found = true + return false + } + } + return true + }) + return found +} + +func blockHasNameToken(block *ast.BlockStmt, tokens ...string) bool { + found := false + ast.Inspect(block, func(node ast.Node) bool { + switch n := node.(type) { + case *ast.Ident: + if nameHasToken(n.Name, tokens...) { + found = true + return false + } + case *ast.SelectorExpr: + if nameHasToken(n.Sel.Name, tokens...) { + found = true + return false + } + } + return true + }) + return found +} + +func nameHasToken(name string, tokens ...string) bool { + name = strings.ToLower(name) + for _, token := range tokens { + if strings.Contains(name, token) { + return true + } + } + return false +} + +func fileHasSelector(parsed *ast.File, selector string) bool { + found := false + ast.Inspect(parsed, func(node ast.Node) bool { + sel, ok := node.(*ast.SelectorExpr) + if ok && sel.Sel.Name == selector { + found = true + return false + } + return true + }) + return found +} + +func isInsideLoop(root ast.Node, target ast.Node) bool { + inside := false + var stack []ast.Node + ast.Inspect(root, func(node ast.Node) bool { + if node == nil { + if len(stack) > 0 { + stack = stack[:len(stack)-1] + } + return true + } + if node == target { + for _, item := range stack { + switch item.(type) { + case *ast.ForStmt, *ast.RangeStmt: + inside = true + return false + } + } + } + stack = append(stack, node) + return true + }) + return inside +} + +func isErrorsNewCall(call *ast.CallExpr) bool { + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "New" { + return false + } + ident, ok := selector.X.(*ast.Ident) + return ok && ident.Name == "errors" +} + +func callName(call *ast.CallExpr) string { + return exprString(call.Fun) +} + +func selectorChain(call *ast.CallExpr) string { + if call == nil { + return "" + } + return exprString(call.Fun) +} + +func exprString(expr ast.Expr) string { + switch n := expr.(type) { + case *ast.Ident: + return n.Name + case *ast.SelectorExpr: + left := exprString(n.X) + if left == "" { + return n.Sel.Name + } + return left + "." + n.Sel.Name + case *ast.StarExpr: + return "*" + exprString(n.X) + case *ast.CallExpr: + return exprString(n.Fun) + default: + return fmt.Sprintf("%T", expr) + } +} diff --git a/internal/codeguard/checks/reliability/reliability_python.go b/internal/codeguard/checks/reliability/reliability_python.go new file mode 100644 index 0000000..8e13dfe --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_python.go @@ -0,0 +1,177 @@ +package reliability + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + pyHTTPCall = regexp.MustCompile(`\b(?:requests|httpx)\.(?:get|post|put|patch|delete|head)\s*\(`) + pyAioHTTPCall = regexp.MustCompile(`\b(?:session|client)\.(?:get|post|put|patch|delete|head)\s*\(`) + pyRetryLoop = regexp.MustCompile(`^\s*(?:while\s+True\s*:|for\s+\w+\s+in\s+range\s*\()`) + pyBackoffHint = regexp.MustCompile(`\b(?:sleep|backoff|jitter|wait_random|wait_exponential)\b`) + pyTaskCreate = regexp.MustCompile(`\basyncio\.(?:create_task|ensure_future)\s*\(`) + pyConcurrencyLimit = regexp.MustCompile(`\b(?:Semaphore|BoundedSemaphore|TaskGroup|CapacityLimiter)\s*\(`) + pySwallowedExcept = regexp.MustCompile(`^\s*(?:pass|return\s+None|return\s*$|continue)\s*(?:#.*)?$`) + pyRecoverableRaise = regexp.MustCompile(`\braise\s+(?:RuntimeError|Exception)\s*\(`) + pyCloseCall = regexp.MustCompile(`\.(?:close|aclose)\s*\(`) + pyOpenCall = regexp.MustCompile(`\bopen\s*\(`) + pyIdempotencyHint = regexp.MustCompile(`(?i)idempot|dedupe|dedup|processed|message_id|event_id`) + pyNonIdempotentRetryCall = regexp.MustCompile(`(?i)\b(?:post|put|patch|delete|create|update|save|insert|publish|send|charge|write)\w*\s*\(`) +) + +func pythonFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + source := strings.ReplaceAll(string(data), "\r\n", "\n") + scan := &pythonReliabilityScan{ + env: env, + file: file, + rules: env.Config.Checks.ReliabilityRules, + limited: pyConcurrencyLimit.MatchString(source), + } + for idx, line := range strings.Split(source, "\n") { + scan.consumeLine(idx+1, line) + } + return scan.findings +} + +type pythonReliabilityScan struct { + env support.Context + file string + rules core.ReliabilityRulesConfig + limited bool + loops []pythonLoopRegion + excepts []int + openLine int + openLineClosed bool + findings []core.Finding +} + +func (s *pythonReliabilityScan) consumeLine(lineNo int, line string) { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + return + } + indent := leadingIndentWidth(line) + s.loops = popPythonLoopRegions(s.loops, indent) + s.excepts = popIndentedRegions(s.excepts, indent) + inLoop := len(s.loops) > 0 + inUnboundedLoop := inUnboundedPythonLoop(s.loops) + inExcept := len(s.excepts) > 0 + if pyRetryLoop.MatchString(line) { + s.loops = append(s.loops, pythonLoopRegion{indent: indent, unbounded: strings.HasPrefix(trimmed, "while True")}) + inLoop = true + inUnboundedLoop = strings.HasPrefix(trimmed, "while True") || inUnboundedLoop + } + if strings.HasPrefix(trimmed, "except ") || strings.HasPrefix(trimmed, "except:") { + s.excepts = append(s.excepts, indent) + inExcept = true + } + s.checkLine(lineNo, line, trimmed, inLoop, inUnboundedLoop, inExcept) +} + +func (s *pythonReliabilityScan) checkLine(lineNo int, line string, trimmed string, inLoop bool, inUnboundedLoop bool, inExcept bool) { + if enabled(s.rules.DetectMissingTimeout) && (pyHTTPCall.MatchString(line) || pyAioHTTPCall.MatchString(line)) && !strings.Contains(line, "timeout=") { + s.add("reliability.missing-timeout", "fail", lineNo, "outbound Python HTTP call has no timeout argument", "high", "call", "http-without-timeout") + } + if enabled(s.rules.DetectRetryWithoutBackoff) && inLoop && looksRetryish(line) && !pyBackoffHint.MatchString(line) { + s.add("reliability.retry-without-backoff", "warn", lineNo, "retry-like Python loop has no visible backoff or jitter", "medium", "retry", "no-backoff") + } + if enabled(s.rules.DetectUnboundedRetry) && inUnboundedLoop && looksRetryish(line) { + s.add("reliability.unbounded-retry", "fail", lineNo, "retry-like Python loop can run forever without an attempt limit", "medium", "retry", "while-true") + } + if enabled(s.rules.DetectNonIdempotentRetry) && inLoop && pyNonIdempotentRetryCall.MatchString(line) && !pyIdempotencyHint.MatchString(line) { + s.add("reliability.non-idempotent-retry", "fail", lineNo, "retry-like Python loop wraps a non-idempotent side effect without idempotency evidence", "medium", "retry", "side-effect") + } + if enabled(s.rules.DetectUnboundedWork) && pyTaskCreate.MatchString(line) && !s.limited { + detail := "asyncio-task" + message := "asyncio task is created without an obvious concurrency bound" + if inLoop { + detail = "asyncio-task-in-loop" + message = "asyncio task is created inside a loop without an obvious concurrency bound" + } + s.add("reliability.unbounded-work", "warn", lineNo, message, "high", "work", detail) + } + if enabled(s.rules.DetectSwallowedError) && inExcept && pySwallowedExcept.MatchString(trimmed) { + s.add("reliability.swallowed-error", "fail", lineNo, "exception handler swallows the error without reporting or returning it", "high", "error", "except-swallowed") + } + if enabled(s.rules.DetectRecoverablePanic) && pyRecoverableRaise.MatchString(line) { + s.add("reliability.recoverable-panic", "fail", lineNo, "production code raises a generic exception for a recoverable failure path", "medium", "exception", "generic-raise") + } + if enabled(s.rules.DetectResourceLeak) { + s.trackResourceLeak(lineNo, line) + } +} + +type pythonLoopRegion struct { + indent int + unbounded bool +} + +func (s *pythonReliabilityScan) trackResourceLeak(lineNo int, line string) { + if strings.Contains(line, "with ") { + return + } + if pyOpenCall.MatchString(line) { + s.openLine = lineNo + s.openLineClosed = pyCloseCall.MatchString(line) + return + } + if s.openLine > 0 && pyCloseCall.MatchString(line) { + s.openLineClosed = true + } + if s.openLine > 0 && lineNo > s.openLine+5 && !s.openLineClosed { + s.add("reliability.resource-leak", "fail", s.openLine, "opened Python resource is not closed near the acquisition path", "medium", "resource", "python-open") + s.openLine = 0 + } +} + +func (s *pythonReliabilityScan) add(ruleID string, level string, lineNo int, message string, confidence string, metaKey string, metaValue string) { + s.findings = append(s.findings, newFinding(s.env, ruleID, level, s.file, lineNo, 1, message, confidence, metaKey, metaValue)) +} + +func looksRetryish(line string) bool { + lower := strings.ToLower(line) + return strings.Contains(lower, "retry") || strings.Contains(lower, "attempt") || strings.Contains(lower, "transient") +} + +func leadingIndentWidth(line string) int { + width := 0 + for _, ch := range line { + if ch == ' ' { + width++ + continue + } + if ch == '\t' { + width += 4 + continue + } + break + } + return width +} + +func popIndentedRegions(regions []int, indent int) []int { + for len(regions) > 0 && indent <= regions[len(regions)-1] { + regions = regions[:len(regions)-1] + } + return regions +} + +func popPythonLoopRegions(regions []pythonLoopRegion, indent int) []pythonLoopRegion { + for len(regions) > 0 && indent <= regions[len(regions)-1].indent { + regions = regions[:len(regions)-1] + } + return regions +} + +func inUnboundedPythonLoop(regions []pythonLoopRegion) bool { + for _, region := range regions { + if region.unbounded { + return true + } + } + return false +} diff --git a/internal/codeguard/checks/reliability/reliability_typescript.go b/internal/codeguard/checks/reliability/reliability_typescript.go new file mode 100644 index 0000000..efe36e4 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_typescript.go @@ -0,0 +1,111 @@ +package reliability + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + tsFetchCall = regexp.MustCompile(`\bfetch\s*\(`) + tsAxiosCall = regexp.MustCompile(`\baxios\.(?:get|post|put|patch|delete)\s*\(`) + tsLoopStart = regexp.MustCompile(`(?:^|[^\w$])(?:for|while)\s*\(|\.(?:forEach|map|flatMap)\s*\(`) + tsUnboundedLoop = regexp.MustCompile(`\bwhile\s*\(\s*true\s*\)|\bfor\s*\(\s*;\s*;\s*\)`) + tsRetryHint = regexp.MustCompile(`(?i)retry|attempt|transient`) + tsBackoffHint = regexp.MustCompile(`(?i)backoff|jitter|setTimeout|sleep|delay`) + tsPromiseInLoop = regexp.MustCompile(`\b(?:new\s+Promise|fetch|axios\.|Promise\.all|[A-Za-z_$][\w$]*Async|fetch[A-Za-z_$][\w$]*)\s*\(`) + tsLimitHint = regexp.MustCompile(`p-limit|pLimit|Bottleneck|PQueue|Semaphore|AbortSignal|AbortController`) + tsSwallowedCatch = regexp.MustCompile(`catch\s*\([^)]*\)\s*\{\s*(?:return\s+undefined\s*;?|return\s*;?|console\.(?:log|warn|error)\([^)]*\)\s*;?)?\s*\}`) + tsGenericThrow = regexp.MustCompile(`throw\s+new\s+(?:Error|TypeError|RuntimeError)\s*\(`) + tsNonIdempotentCall = regexp.MustCompile(`(?i)\b(?:post|put|patch|delete|create|update|save|insert|publish|send|charge|write)\w*\s*\(`) + tsIdempotencyHint = regexp.MustCompile(`(?i)idempot|dedupe|dedup|processed|messageId|eventId`) +) + +func typeScriptTargetFindings(env support.Context, target core.TargetConfig) []core.Finding { + findings := make([]core.Finding, 0) + env.VisitTargetFiles(target, support.IsTypeScriptLikeFile, func(rel string, data []byte) { + findings = append(findings, typeScriptFindingsForFile(env, rel, data)...) + }) + return findings +} + +func typeScriptFindingsForFile(env support.Context, file string, data []byte) []core.Finding { + source := strings.ReplaceAll(string(data), "\r\n", "\n") + code := support.StripTypeScriptCommentsAndStrings(source) + scan := &tsReliabilityScan{ + env: env, + file: file, + rules: env.Config.Checks.ReliabilityRules, + limited: tsLimitHint.MatchString(source), + } + for idx, line := range strings.Split(code, "\n") { + scan.consumeLine(idx+1, line) + } + if enabled(scan.rules.DetectSwallowedError) { + for idx, rawLine := range strings.Split(source, "\n") { + if tsSwallowedCatch.MatchString(rawLine) { + scan.add("reliability.swallowed-error", "fail", idx+1, "catch block swallows an error without returning or propagating it", "high", "error", "catch-swallowed") + } + } + } + return scan.findings +} + +type tsReliabilityScan struct { + env support.Context + file string + rules core.ReliabilityRulesConfig + limited bool + depth int + loops []int + unboundedLoops []int + findings []core.Finding +} + +func (s *tsReliabilityScan) consumeLine(lineNo int, line string) { + startsLoop := tsLoopStart.MatchString(line) + inLoop := len(s.loops) > 0 || startsLoop + inUnboundedLoop := len(s.unboundedLoops) > 0 || tsUnboundedLoop.MatchString(line) + s.checkLine(lineNo, line, inLoop, inUnboundedLoop) + next := s.depth + strings.Count(line, "{") - strings.Count(line, "}") + if startsLoop && next > s.depth { + s.loops = append(s.loops, s.depth) + if tsUnboundedLoop.MatchString(line) { + s.unboundedLoops = append(s.unboundedLoops, s.depth) + } + } + for len(s.loops) > 0 && next <= s.loops[len(s.loops)-1] { + s.loops = s.loops[:len(s.loops)-1] + } + for len(s.unboundedLoops) > 0 && next <= s.unboundedLoops[len(s.unboundedLoops)-1] { + s.unboundedLoops = s.unboundedLoops[:len(s.unboundedLoops)-1] + } + s.depth = next +} + +func (s *tsReliabilityScan) checkLine(lineNo int, line string, inLoop bool, inUnboundedLoop bool) { + if enabled(s.rules.DetectMissingTimeout) && (tsFetchCall.MatchString(line) || tsAxiosCall.MatchString(line)) && !strings.Contains(line, "timeout") && !strings.Contains(line, "signal") { + s.add("reliability.missing-timeout", "fail", lineNo, "outbound TypeScript/JavaScript HTTP call lacks timeout or abort signal evidence", "medium", "call", "http-without-timeout") + } + if enabled(s.rules.DetectUnboundedWork) && inLoop && !s.limited && tsPromiseInLoop.MatchString(line) { + s.add("reliability.unbounded-work", "warn", lineNo, "promise or HTTP work starts inside a loop without a visible concurrency limit", "medium", "work", "promise-in-loop") + } + if enabled(s.rules.DetectRetryWithoutBackoff) && inLoop && tsRetryHint.MatchString(line) && !tsBackoffHint.MatchString(line) { + s.add("reliability.retry-without-backoff", "warn", lineNo, "retry-like JavaScript loop has no visible backoff or jitter", "medium", "retry", "no-backoff") + } + if enabled(s.rules.DetectUnboundedRetry) && inUnboundedLoop && tsRetryHint.MatchString(line) { + s.add("reliability.unbounded-retry", "fail", lineNo, "retry-like JavaScript loop can run forever without an attempt limit", "medium", "retry", "while-true") + } + if enabled(s.rules.DetectNonIdempotentRetry) && inLoop && tsNonIdempotentCall.MatchString(line) && !tsIdempotencyHint.MatchString(line) { + s.add("reliability.non-idempotent-retry", "fail", lineNo, "retry-like JavaScript loop wraps a non-idempotent side effect without idempotency evidence", "medium", "retry", "side-effect") + } + if enabled(s.rules.DetectRecoverablePanic) && tsGenericThrow.MatchString(line) { + s.add("reliability.recoverable-panic", "fail", lineNo, "production code throws a generic exception for a recoverable failure path", "medium", "exception", "generic-throw") + } +} + +func (s *tsReliabilityScan) add(ruleID string, level string, lineNo int, message string, confidence string, metaKey string, metaValue string) { + s.findings = append(s.findings, newFinding(s.env, ruleID, level, s.file, lineNo, 1, message, confidence, metaKey, metaValue)) +} diff --git a/internal/codeguard/checks/support/artifacts.go b/internal/codeguard/checks/support/artifacts.go index ccd8a67..221f731 100644 --- a/internal/codeguard/checks/support/artifacts.go +++ b/internal/codeguard/checks/support/artifacts.go @@ -7,6 +7,7 @@ const ArtifactKindSlopScore = "slop_score" const ArtifactKindPerformanceScore = "performance_score" const ArtifactKindChangeRisk = "change_risk" const ArtifactKindRepoLegibility = "repo_legibility" +const ArtifactKindPRSummary = "pr_summary" func NewDependencyGraphArtifact(id string, language string, target string, graph DependencyGraph) core.Artifact { nodes := make([]core.DependencyGraphNode, 0, len(graph.Order)) @@ -105,3 +106,24 @@ func NewChangeRiskArtifact(id string, language string, target string, risk core. }, } } + +func NewPRSummaryArtifact(summary core.PRSummaryArtifact) core.Artifact { + return core.Artifact{ + ID: "pr_summary", + Kind: ArtifactKindPRSummary, + PRSummary: clonePRSummary(summary), + } +} + +func clonePRSummary(summary core.PRSummaryArtifact) *core.PRSummaryArtifact { + out := core.PRSummaryArtifact{} + if summary.ProductionRisk != nil { + components := append([]core.PRSummaryComponent(nil), summary.ProductionRisk.Components...) + out.ProductionRisk = &core.PRSummaryMetric{ + Score: summary.ProductionRisk.Score, + Level: summary.ProductionRisk.Level, + Components: components, + } + } + return &out +} diff --git a/internal/codeguard/config/defaults.go b/internal/codeguard/config/defaults.go index d7c3497..9e59565 100644 --- a/internal/codeguard/config/defaults.go +++ b/internal/codeguard/config/defaults.go @@ -61,6 +61,12 @@ func applyCheckDefaults(cfg *core.Config, def core.Config) { if cfg.Checks.Context == nil { cfg.Checks.Context = def.Checks.Context } + if cfg.Checks.Reliability == nil { + cfg.Checks.Reliability = def.Checks.Reliability + } + if cfg.Checks.Data == nil { + cfg.Checks.Data = def.Checks.Data + } applyQualityDefaults(&cfg.Checks.QualityRules, def.Checks.QualityRules) applyPerformanceDefaults(&cfg.Checks.PerformanceRules) applyDesignDefaults(&cfg.Checks.DesignRules, def.Checks.DesignRules) @@ -68,8 +74,11 @@ func applyCheckDefaults(cfg *core.Config, def core.Config) { applyCIDefaults(&cfg.Checks.CIRules, def.Checks.CIRules) applySecurityDefaults(&cfg.Checks.SecurityRules, def.Checks.SecurityRules) applySupplyChainDefaults(&cfg.Checks.SupplyChainRules, def.Checks.SupplyChainRules) + applyReliabilityDefaults(&cfg.Checks.ReliabilityRules, def.Checks.ReliabilityRules) + applyDataDefaults(&cfg.Checks.DataRules, def.Checks.DataRules) applyContextDefaults(&cfg.Checks.ContextRules, def.Checks.ContextRules) applyContractDefaults(&cfg.Checks.ContractRules, def.Checks.ContractRules) + applyProductionRiskDefaults(&cfg.Checks.ProductionRisk, def.Checks.ProductionRisk) applyAIDefaults(&cfg.AI, def.AI) applyCheckActivationDefaults(&cfg.Checks) } diff --git a/internal/codeguard/config/defaults_activation.go b/internal/codeguard/config/defaults_activation.go index 461c9e2..7666eb5 100644 --- a/internal/codeguard/config/defaults_activation.go +++ b/internal/codeguard/config/defaults_activation.go @@ -29,6 +29,8 @@ var checkDisablers = map[string]func(*core.CheckConfig){ "prompts": func(checks *core.CheckConfig) { checks.Prompts = false }, "ci": func(checks *core.CheckConfig) { checks.CI = false }, "supply_chain": func(checks *core.CheckConfig) { checks.SupplyChain = false }, + "reliability": func(checks *core.CheckConfig) { checks.Reliability = boolPtr(false) }, + "data": func(checks *core.CheckConfig) { checks.Data = boolPtr(false) }, "context": func(checks *core.CheckConfig) { checks.Context = boolPtr(false) }, "contracts": func(checks *core.CheckConfig) { checks.Contracts = boolPtr(false) }, } diff --git a/internal/codeguard/config/defaults_rules.go b/internal/codeguard/config/defaults_rules.go index 0126946..38844e5 100644 --- a/internal/codeguard/config/defaults_rules.go +++ b/internal/codeguard/config/defaults_rules.go @@ -197,3 +197,51 @@ func applySupplyChainDefaults(dst *core.SupplyChainRulesConfig, def core.SupplyC defaultStringSlice(&dst.DeniedLicenses, def.DeniedLicenses, false) defaultSingleCommandMap(&dst.LicenseCommands, def.LicenseCommands) } + +func applyReliabilityDefaults(dst *core.ReliabilityRulesConfig, def core.ReliabilityRulesConfig) { + applyDefaultBoolPtrs( + &dst.DetectMissingTimeout, + &dst.DetectUnboundedRetry, + &dst.DetectRetryWithoutBackoff, + &dst.DetectNonIdempotentRetry, + &dst.DetectMissingCancellation, + &dst.DetectUnboundedWork, + &dst.DetectMissingConcurrencyLimit, + &dst.DetectResourceLeak, + &dst.DetectPartialFailureHidden, + &dst.DetectMissingGracefulShutdown, + &dst.DetectSwallowedError, + &dst.DetectLostErrorContext, + &dst.DetectRecoverablePanic, + ) + defaultInt(&dst.MaxRetryAttempts, def.MaxRetryAttempts) + defaultInt(&dst.MaxInlineGoroutinesPerFunction, def.MaxInlineGoroutinesPerFunction) +} + +func applyDataDefaults(dst *core.DataRulesConfig, def core.DataRulesConfig) { + applyDefaultBoolPtrs( + &dst.DetectReadModifyWriteRace, + &dst.DetectMissingTransaction, + &dst.DetectSideEffectInTransaction, + &dst.DetectNonIdempotentConsumer, + &dst.DetectMissingDeduplication, + &dst.DetectUnsafeDualWrite, + &dst.DetectMissingOutboxStrategy, + &dst.DetectUnstablePagination, + &dst.DetectUnboundedRead, + &dst.DetectExactlyOnceAssumption, + &dst.DetectCacheWithoutPolicy, + ) + defaultInt(&dst.MaxUnboundedReadRows, def.MaxUnboundedReadRows) + defaultInt(&dst.MaxWritesWithoutTransaction, def.MaxWritesWithoutTransaction) +} + +func applyProductionRiskDefaults(dst *core.ProductionRiskConfig, def core.ProductionRiskConfig) { + defaultBoolPtr(&dst.Enabled, boolValueOrTrue(def.Enabled)) + defaultInt(&dst.WarnThreshold, def.WarnThreshold) + defaultInt(&dst.FailThreshold, def.FailThreshold) + defaultInt(&dst.ReliabilityWeight, def.ReliabilityWeight) + defaultInt(&dst.DataWeight, def.DataWeight) + defaultInt(&dst.FailWeight, def.FailWeight) + defaultInt(&dst.WarnWeight, def.WarnWeight) +} diff --git a/internal/codeguard/config/example.go b/internal/codeguard/config/example.go index 2f95d5e..d23d0d8 100644 --- a/internal/codeguard/config/example.go +++ b/internal/codeguard/config/example.go @@ -36,6 +36,8 @@ func exampleChecks() core.CheckConfig { Prompts: true, CI: true, SupplyChain: false, + Reliability: boolPtr(false), + Data: boolPtr(false), QualityRules: exampleQualityRules(), PerformanceRules: examplePerformanceRules(), DesignRules: exampleDesignRules(), @@ -43,8 +45,11 @@ func exampleChecks() core.CheckConfig { CIRules: exampleCIRules(), SecurityRules: exampleSecurityRules(), SupplyChainRules: exampleSupplyChainRules(), + ReliabilityRules: exampleReliabilityRules(), + DataRules: exampleDataRules(), ContractRules: exampleContractRules(), ContextRules: exampleContextRules(), + ProductionRisk: exampleProductionRisk(), } } @@ -72,6 +77,56 @@ func exampleSupplyChainRules() core.SupplyChainRulesConfig { } } +func exampleReliabilityRules() core.ReliabilityRulesConfig { + return core.ReliabilityRulesConfig{ + DetectMissingTimeout: boolPtr(true), + DetectUnboundedRetry: boolPtr(true), + DetectRetryWithoutBackoff: boolPtr(true), + DetectNonIdempotentRetry: boolPtr(true), + DetectMissingCancellation: boolPtr(true), + DetectUnboundedWork: boolPtr(true), + DetectMissingConcurrencyLimit: boolPtr(true), + DetectResourceLeak: boolPtr(true), + DetectPartialFailureHidden: boolPtr(true), + DetectMissingGracefulShutdown: boolPtr(true), + DetectSwallowedError: boolPtr(true), + DetectLostErrorContext: boolPtr(true), + DetectRecoverablePanic: boolPtr(true), + MaxRetryAttempts: 3, + MaxInlineGoroutinesPerFunction: 4, + } +} + +func exampleDataRules() core.DataRulesConfig { + return core.DataRulesConfig{ + DetectReadModifyWriteRace: boolPtr(true), + DetectMissingTransaction: boolPtr(true), + DetectSideEffectInTransaction: boolPtr(true), + DetectNonIdempotentConsumer: boolPtr(true), + DetectMissingDeduplication: boolPtr(true), + DetectUnsafeDualWrite: boolPtr(true), + DetectMissingOutboxStrategy: boolPtr(true), + DetectUnstablePagination: boolPtr(true), + DetectUnboundedRead: boolPtr(true), + DetectExactlyOnceAssumption: boolPtr(true), + DetectCacheWithoutPolicy: boolPtr(true), + MaxUnboundedReadRows: 1000, + MaxWritesWithoutTransaction: 1, + } +} + +func exampleProductionRisk() core.ProductionRiskConfig { + return core.ProductionRiskConfig{ + Enabled: boolPtr(true), + WarnThreshold: 35, + FailThreshold: 70, + ReliabilityWeight: 12, + DataWeight: 15, + FailWeight: 25, + WarnWeight: 10, + } +} + func exampleContractRules() core.ContractRulesConfig { return core.ContractRulesConfig{ GoExportedBreaking: boolPtr(true), diff --git a/internal/codeguard/config/profile.go b/internal/codeguard/config/profile.go index 1b0ca58..79d22cb 100644 --- a/internal/codeguard/config/profile.go +++ b/internal/codeguard/config/profile.go @@ -40,6 +40,7 @@ var profileCatalog = map[string]profileSpec{ applyStrictProfile(cfg) cfg.Checks.CIRules.RequiredReleaseFiles = []string{".goreleaser.yaml"} cfg.Checks.CIRules.RequiredAutomationPaths = []string{"Makefile", ".github/workflows/ci.yml"} + cfg.Checks.Data = boolPtr(true) }, }, "ai-safe": { @@ -56,6 +57,8 @@ var profileCatalog = map[string]profileSpec{ cfg.Checks.QualityRules.AIProvenance.Enabled = boolPtr(true) cfg.Checks.QualityRules.AIProvenance.SlopScoreWarnThreshold = 10 cfg.Checks.QualityRules.AIProvenance.SlopScoreFailThreshold = 25 + cfg.Checks.Reliability = boolPtr(true) + cfg.Checks.Data = boolPtr(true) }, }, } @@ -71,6 +74,7 @@ func applyStrictProfile(cfg *core.Config) { cfg.Checks.DesignRules.MaxInterfaceMethods = 4 cfg.Checks.SecurityRules.GovulncheckMode = "required" cfg.Checks.Contracts = boolPtr(true) + cfg.Checks.Reliability = boolPtr(true) } func ExampleConfig() core.Config { diff --git a/internal/codeguard/config/recommended_defaults_test.go b/internal/codeguard/config/recommended_defaults_test.go index e775d41..8c46842 100644 --- a/internal/codeguard/config/recommended_defaults_test.go +++ b/internal/codeguard/config/recommended_defaults_test.go @@ -51,6 +51,8 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults want: core.CheckConfig{ Performance: trueValue, SupplyChain: true, + Reliability: falseValue, + Data: falseValue, Context: falseValue, Contracts: trueValue, }, @@ -67,6 +69,8 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults Security: true, Prompts: true, CI: true, + Reliability: falseValue, + Data: falseValue, }, }, { @@ -75,6 +79,8 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults UseRecommendedDefaults: true, Performance: trueValue, SupplyChain: true, + Reliability: trueValue, + Data: trueValue, }, want: core.CheckConfig{ UseRecommendedDefaults: true, @@ -85,6 +91,8 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults CI: true, Performance: trueValue, SupplyChain: true, + Reliability: trueValue, + Data: trueValue, }, }, { @@ -95,8 +103,10 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults SupplyChain: true, Context: trueValue, Contracts: trueValue, + Reliability: trueValue, + Data: trueValue, Disabled: []string{ - "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "context", "contracts", + "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "reliability", "data", "context", "contracts", }, }, want: core.CheckConfig{ @@ -104,8 +114,10 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults Performance: falseValue, Context: falseValue, Contracts: falseValue, + Reliability: falseValue, + Data: falseValue, Disabled: []string{ - "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "context", "contracts", + "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "reliability", "data", "context", "contracts", }, }, }, @@ -122,6 +134,8 @@ func sameCheckActivation(got, want core.CheckConfig) bool { got.CI == want.CI && got.SupplyChain == want.SupplyChain && reflect.DeepEqual(got.Performance, want.Performance) && + reflect.DeepEqual(got.Reliability, want.Reliability) && + reflect.DeepEqual(got.Data, want.Data) && reflect.DeepEqual(got.Context, want.Context) && reflect.DeepEqual(got.Contracts, want.Contracts) } diff --git a/internal/codeguard/config/validate.go b/internal/codeguard/config/validate.go index 23104a9..dae3181 100644 --- a/internal/codeguard/config/validate.go +++ b/internal/codeguard/config/validate.go @@ -24,6 +24,9 @@ func Validate(cfg core.Config) error { validateRiskScoring(cfg.Checks.QualityRules.RiskScoring), validateAIChecks(cfg.Checks.QualityRules.AIChecks), validateSupplyChainRules(cfg.Checks.SupplyChainRules), + validateReliabilityRules(cfg.Checks.ReliabilityRules), + validateDataRules(cfg.Checks.DataRules), + validateProductionRisk(cfg.Checks.ProductionRisk), validateContractRules(cfg.Checks.ContractRules), validateContextRules(cfg.Checks.ContextRules), validateCoverageDelta(cfg.Checks.QualityRules.CoverageDelta), diff --git a/internal/codeguard/config/validate_defaults.go b/internal/codeguard/config/validate_defaults.go index 3df377f..bab235b 100644 --- a/internal/codeguard/config/validate_defaults.go +++ b/internal/codeguard/config/validate_defaults.go @@ -9,7 +9,7 @@ import ( var recognizedDisabledChecks = map[string]struct{}{ "quality": {}, "performance": {}, "design": {}, "security": {}, "prompts": {}, - "ci": {}, "supply_chain": {}, "context": {}, "contracts": {}, + "ci": {}, "supply_chain": {}, "reliability": {}, "data": {}, "context": {}, "contracts": {}, } func validateDisabledChecks(disabled []string) error { diff --git a/internal/codeguard/config/validate_reliability_data.go b/internal/codeguard/config/validate_reliability_data.go new file mode 100644 index 0000000..0350dce --- /dev/null +++ b/internal/codeguard/config/validate_reliability_data.go @@ -0,0 +1,53 @@ +package config + +import ( + "fmt" + + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func validateReliabilityRules(rules core.ReliabilityRulesConfig) error { + if rules.MaxRetryAttempts < 0 { + return fmt.Errorf("reliability_rules.max_retry_attempts must not be negative") + } + if rules.MaxInlineGoroutinesPerFunction < 0 { + return fmt.Errorf("reliability_rules.max_inline_goroutines_per_function must not be negative") + } + return nil +} + +func validateDataRules(rules core.DataRulesConfig) error { + if rules.MaxUnboundedReadRows < 0 { + return fmt.Errorf("data_rules.max_unbounded_read_rows must not be negative") + } + if rules.MaxWritesWithoutTransaction < 0 { + return fmt.Errorf("data_rules.max_writes_without_transaction must not be negative") + } + return nil +} + +func validateProductionRisk(risk core.ProductionRiskConfig) error { + if risk.WarnThreshold < 0 || risk.WarnThreshold > 100 { + return fmt.Errorf("production_risk.warn_threshold must be between 0 and 100") + } + if risk.FailThreshold < 0 || risk.FailThreshold > 100 { + return fmt.Errorf("production_risk.fail_threshold must be between 0 and 100") + } + if risk.WarnThreshold > 0 && risk.FailThreshold > 0 && risk.WarnThreshold > risk.FailThreshold { + return fmt.Errorf("production_risk.warn_threshold must not exceed fail_threshold") + } + for _, item := range []struct { + field string + value int + }{ + {"production_risk.reliability_weight", risk.ReliabilityWeight}, + {"production_risk.data_weight", risk.DataWeight}, + {"production_risk.fail_weight", risk.FailWeight}, + {"production_risk.warn_weight", risk.WarnWeight}, + } { + if item.value < 0 { + return fmt.Errorf("%s must not be negative", item.field) + } + } + return nil +} diff --git a/internal/codeguard/core/config_rule_types.go b/internal/codeguard/core/config_rule_types.go index f980864..f2819df 100644 --- a/internal/codeguard/core/config_rule_types.go +++ b/internal/codeguard/core/config_rule_types.go @@ -216,3 +216,54 @@ type SupplyChainRulesConfig struct { DeniedLicenses []string `json:"denied_licenses,omitempty" yaml:"denied_licenses,omitempty"` LicenseCommands map[string]CommandCheckConfig `json:"license_commands,omitempty" yaml:"license_commands,omitempty"` } + +// ReliabilityRulesConfig tunes the reliability section. Nil rule toggles +// default to enabled when the section itself is enabled by configuration or a +// profile. +type ReliabilityRulesConfig struct { + DetectMissingTimeout *bool `json:"detect_missing_timeout,omitempty" yaml:"detect_missing_timeout,omitempty"` + DetectUnboundedRetry *bool `json:"detect_unbounded_retry,omitempty" yaml:"detect_unbounded_retry,omitempty"` + DetectRetryWithoutBackoff *bool `json:"detect_retry_without_backoff,omitempty" yaml:"detect_retry_without_backoff,omitempty"` + DetectNonIdempotentRetry *bool `json:"detect_non_idempotent_retry,omitempty" yaml:"detect_non_idempotent_retry,omitempty"` + DetectMissingCancellation *bool `json:"detect_missing_cancellation,omitempty" yaml:"detect_missing_cancellation,omitempty"` + DetectUnboundedWork *bool `json:"detect_unbounded_work,omitempty" yaml:"detect_unbounded_work,omitempty"` + DetectMissingConcurrencyLimit *bool `json:"detect_missing_concurrency_limit,omitempty" yaml:"detect_missing_concurrency_limit,omitempty"` + DetectResourceLeak *bool `json:"detect_resource_leak,omitempty" yaml:"detect_resource_leak,omitempty"` + DetectPartialFailureHidden *bool `json:"detect_partial_failure_hidden,omitempty" yaml:"detect_partial_failure_hidden,omitempty"` + DetectMissingGracefulShutdown *bool `json:"detect_missing_graceful_shutdown,omitempty" yaml:"detect_missing_graceful_shutdown,omitempty"` + DetectSwallowedError *bool `json:"detect_swallowed_error,omitempty" yaml:"detect_swallowed_error,omitempty"` + DetectLostErrorContext *bool `json:"detect_lost_error_context,omitempty" yaml:"detect_lost_error_context,omitempty"` + DetectRecoverablePanic *bool `json:"detect_recoverable_panic,omitempty" yaml:"detect_recoverable_panic,omitempty"` + MaxRetryAttempts int `json:"max_retry_attempts,omitempty" yaml:"max_retry_attempts,omitempty"` + MaxInlineGoroutinesPerFunction int `json:"max_inline_goroutines_per_function,omitempty" yaml:"max_inline_goroutines_per_function,omitempty"` +} + +// DataRulesConfig tunes the data-correctness section. Nil rule toggles default +// to enabled when the section itself is enabled by configuration or a profile. +type DataRulesConfig struct { + DetectReadModifyWriteRace *bool `json:"detect_read_modify_write_race,omitempty" yaml:"detect_read_modify_write_race,omitempty"` + DetectMissingTransaction *bool `json:"detect_missing_transaction,omitempty" yaml:"detect_missing_transaction,omitempty"` + DetectSideEffectInTransaction *bool `json:"detect_side_effect_in_transaction,omitempty" yaml:"detect_side_effect_in_transaction,omitempty"` + DetectNonIdempotentConsumer *bool `json:"detect_non_idempotent_consumer,omitempty" yaml:"detect_non_idempotent_consumer,omitempty"` + DetectMissingDeduplication *bool `json:"detect_missing_deduplication,omitempty" yaml:"detect_missing_deduplication,omitempty"` + DetectUnsafeDualWrite *bool `json:"detect_unsafe_dual_write,omitempty" yaml:"detect_unsafe_dual_write,omitempty"` + DetectMissingOutboxStrategy *bool `json:"detect_missing_outbox_strategy,omitempty" yaml:"detect_missing_outbox_strategy,omitempty"` + DetectUnstablePagination *bool `json:"detect_unstable_pagination,omitempty" yaml:"detect_unstable_pagination,omitempty"` + DetectUnboundedRead *bool `json:"detect_unbounded_read,omitempty" yaml:"detect_unbounded_read,omitempty"` + DetectExactlyOnceAssumption *bool `json:"detect_exactly_once_assumption,omitempty" yaml:"detect_exactly_once_assumption,omitempty"` + DetectCacheWithoutPolicy *bool `json:"detect_cache_without_policy,omitempty" yaml:"detect_cache_without_policy,omitempty"` + MaxUnboundedReadRows int `json:"max_unbounded_read_rows,omitempty" yaml:"max_unbounded_read_rows,omitempty"` + MaxWritesWithoutTransaction int `json:"max_writes_without_transaction,omitempty" yaml:"max_writes_without_transaction,omitempty"` +} + +// ProductionRiskConfig controls the additive PR-summary production-risk +// artifact. It never changes individual rule severities. +type ProductionRiskConfig struct { + Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"` + WarnThreshold int `json:"warn_threshold,omitempty" yaml:"warn_threshold,omitempty"` + FailThreshold int `json:"fail_threshold,omitempty" yaml:"fail_threshold,omitempty"` + ReliabilityWeight int `json:"reliability_weight,omitempty" yaml:"reliability_weight,omitempty"` + DataWeight int `json:"data_weight,omitempty" yaml:"data_weight,omitempty"` + FailWeight int `json:"fail_weight,omitempty" yaml:"fail_weight,omitempty"` + WarnWeight int `json:"warn_weight,omitempty" yaml:"warn_weight,omitempty"` +} diff --git a/internal/codeguard/core/config_types.go b/internal/codeguard/core/config_types.go index 7bbaa46..f060d0e 100644 --- a/internal/codeguard/core/config_types.go +++ b/internal/codeguard/core/config_types.go @@ -77,6 +77,14 @@ type CheckConfig struct { // SupplyChain toggles dependency-policy checks such as manifest hygiene, // lockfile drift, license policy, and SBOM-oriented validation. SupplyChain bool `json:"supply_chain,omitempty" yaml:"supply_chain,omitempty"` + // Reliability toggles production reliability checks such as missing + // timeouts, unbounded retries, cancellation propagation, concurrency bounds, + // cleanup handling, and graceful shutdown. + Reliability *bool `json:"reliability,omitempty" yaml:"reliability,omitempty"` + // Data toggles distributed-system and data-correctness checks such as + // missing transaction boundaries, unsafe dual writes, unbounded reads, + // unstable pagination, and cache policy gaps. + Data *bool `json:"data,omitempty" yaml:"data,omitempty"` // Contracts toggles the API contract drift family. When nil it defaults // to enabled in diff scans and disabled in full scans; the strict and // enterprise profiles enable it unconditionally. @@ -96,8 +104,11 @@ type CheckConfig struct { CIRules CIRulesConfig `json:"ci_rules" yaml:"ci_rules"` SecurityRules SecurityRulesConfig `json:"security_rules" yaml:"security_rules"` SupplyChainRules SupplyChainRulesConfig `json:"supply_chain_rules" yaml:"supply_chain_rules"` + ReliabilityRules ReliabilityRulesConfig `json:"reliability_rules,omitempty" yaml:"reliability_rules,omitempty"` + DataRules DataRulesConfig `json:"data_rules,omitempty" yaml:"data_rules,omitempty"` ContractRules ContractRulesConfig `json:"contract_rules" yaml:"contract_rules"` ContextRules ContextRulesConfig `json:"context_rules" yaml:"context_rules"` + ProductionRisk ProductionRiskConfig `json:"production_risk,omitempty" yaml:"production_risk,omitempty"` } type OutputConfig struct { diff --git a/internal/codeguard/core/report_artifact_types.go b/internal/codeguard/core/report_artifact_types.go index 7d67c7c..6328b05 100644 --- a/internal/codeguard/core/report_artifact_types.go +++ b/internal/codeguard/core/report_artifact_types.go @@ -17,13 +17,33 @@ type Artifact struct { RepoLegibility *RepoLegibilityArtifact `json:"repo_legibility,omitempty"` FileRisk *FileRiskArtifact `json:"file_risk,omitempty"` PRHotspots *PRHotspotsArtifact `json:"pr_hotspots,omitempty"` + PRSummary *PRSummaryArtifact `json:"pr_summary,omitempty"` } const ( ReportArtifactKindFileRisk = "file_risk" ReportArtifactKindPRHotspots = "pr_hotspots" + ReportArtifactKindPRSummary = "pr_summary" ) +type PRSummaryArtifact struct { + ProductionRisk *PRSummaryMetric `json:"production_risk,omitempty"` +} + +type PRSummaryMetric struct { + Score int `json:"score"` + Level string `json:"level,omitempty"` + Components []PRSummaryComponent `json:"components,omitempty"` +} + +type PRSummaryComponent struct { + Label string `json:"label"` + Weight int `json:"weight"` + Count int `json:"count"` + Contribution int `json:"contribution"` + Detail string `json:"detail,omitempty"` +} + // FileRiskArtifact ranks every changed file in a diff scan. Components make // every score auditable instead of changing the severity of underlying findings. type FileRiskArtifact struct { diff --git a/internal/codeguard/rules/catalog.go b/internal/codeguard/rules/catalog.go index 563e3f6..2c71488 100644 --- a/internal/codeguard/rules/catalog.go +++ b/internal/codeguard/rules/catalog.go @@ -15,6 +15,8 @@ var catalog = withSecurityOWASP(mergeRuleCatalogs( designPolicyCatalog, securityCatalog, securityExtraCatalog, + reliabilityCatalog, + dataCatalog, supplyChainCatalog, contextCatalog, contextReadinessCatalog, diff --git a/internal/codeguard/rules/catalog_contracts.go b/internal/codeguard/rules/catalog_contracts.go index 85a9cf8..4ccb51a 100644 --- a/internal/codeguard/rules/catalog_contracts.go +++ b/internal/codeguard/rules/catalog_contracts.go @@ -53,4 +53,14 @@ var contractsCatalog = map[string]core.RuleMetadata{ Description: "Warns when migration files contain destructive operations such as DROP TABLE, DROP COLUMN, TRUNCATE, or ALTER ... NOT NULL without a DEFAULT.", HowToFix: "Confirm the data loss is intended, back up affected data first, and prefer additive or reversible migrations.", }, + "contracts.non-expand-contract-migration": { + ID: "contracts.non-expand-contract-migration", + Section: "API Contracts", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Non-expand/contract schema migration", + Description: "Fails when a rolling database schema migration contracts a schema before compatible code and backfill steps are in place.", + HowToFix: "Split the migration into expand, migrate/backfill, and contract releases with compatibility across rolling deploys.", + }, } diff --git a/internal/codeguard/rules/catalog_data.go b/internal/codeguard/rules/catalog_data.go new file mode 100644 index 0000000..c839b56 --- /dev/null +++ b/internal/codeguard/rules/catalog_data.go @@ -0,0 +1,36 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var dataCatalog = map[string]core.RuleMetadata{ + "data.read-modify-write-race": dataRule("data.read-modify-write-race", "fail", "Read-modify-write race", "Fails when code reads state and writes derived state without a transaction, lock, or compare-and-swap boundary.", "Move the read and write into one transaction or use an atomic conditional update."), + "data.missing-transaction-boundary": dataRule("data.missing-transaction-boundary", "fail", "Missing transaction boundary", "Fails when related persistence writes can commit independently without a transaction boundary.", "Wrap related writes in a transaction and define rollback behavior."), + "data.side-effect-in-transaction": dataRule("data.side-effect-in-transaction", "fail", "External side effect in transaction", "Fails when external side effects are performed inside retried or rollback-capable transactions.", "Move external side effects after commit, or persist an outbox record inside the transaction."), + "data.non-idempotent-consumer": dataRule("data.non-idempotent-consumer", "fail", "Non-idempotent consumer", "Fails when a message/event consumer performs side effects without idempotency evidence.", "Use a deduplication key, inbox table, processed-message guard, or idempotent operation."), + "data.missing-deduplication": dataRule("data.missing-deduplication", "warn", "Missing deduplication", "Warns when message or event handling lacks deduplication evidence.", "Record and check a stable message, event, or idempotency key before side effects."), + "data.unsafe-dual-write": dataRule("data.unsafe-dual-write", "fail", "Unsafe dual write", "Fails when code writes to two systems without a strategy for partial failure.", "Use a transaction plus outbox, saga, reconciliation job, or another explicit consistency strategy."), + "data.missing-outbox-strategy": dataRule("data.missing-outbox-strategy", "fail", "Missing outbox strategy", "Fails when code persists state and publishes an event without an outbox or equivalent strategy.", "Persist an outbox record in the same transaction and publish asynchronously after commit."), + "data.unstable-pagination": dataRule("data.unstable-pagination", "warn", "Unstable pagination", "Warns when paginated reads lack deterministic ordering or cursor stability.", "Add deterministic ordering and prefer cursor/keyset pagination for changing datasets."), + "data.unbounded-read": dataRule("data.unbounded-read", "warn", "Unbounded database read", "Warns when database reads lack a limit, cursor, stream, or bounded filter.", "Add a limit, cursor, stream, or explicit bounded query policy."), + "data.exactly-once-assumption": dataRule("data.exactly-once-assumption", "warn", "Exactly-once delivery assumption", "Warns when code assumes exactly-once message delivery without idempotency or deduplication evidence.", "Treat delivery as at-least-once and make handlers idempotent."), + "data.cache-without-policy": dataRule("data.cache-without-policy", "warn", "Cache without policy", "Warns when production caches lack TTL, invalidation, or ownership policy evidence.", "Define TTL, invalidation ownership, and stale-data behavior."), +} + +func dataRule(id string, level string, title string, description string, howToFix string) core.RuleMetadata { + return core.RuleMetadata{ + ID: id, + Section: "Data Correctness", + DefaultLevel: level, + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage( + core.RuleLanguageGo, + core.RuleLanguageTypeScript, + core.RuleLanguageJavaScript, + core.RuleLanguagePython, + core.RuleLanguageCPP, + ), + Title: title, + Description: description, + HowToFix: howToFix, + } +} diff --git a/internal/codeguard/rules/catalog_fix_templates.go b/internal/codeguard/rules/catalog_fix_templates.go index 1d3c192..38e0ff0 100644 --- a/internal/codeguard/rules/catalog_fix_templates.go +++ b/internal/codeguard/rules/catalog_fix_templates.go @@ -22,6 +22,8 @@ var fixTemplates = mergeFixTemplates( performanceFrameworkFixTemplates, performanceAIFixTemplates, performanceMeasuredFixTemplates, + reliabilityFixTemplates, + dataFixTemplates, securityFixTemplates, securityLanguageFixTemplates, designFixTemplates, diff --git a/internal/codeguard/rules/catalog_fix_templates_data.go b/internal/codeguard/rules/catalog_fix_templates_data.go new file mode 100644 index 0000000..b293d7d --- /dev/null +++ b/internal/codeguard/rules/catalog_fix_templates_data.go @@ -0,0 +1,18 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var dataFixTemplates = map[string]core.FixTemplate{ + "data.read-modify-write-race": {Kind: guided, Text: "Make the read and write atomic.\n\nBefore:\nrow := repo.Get(id)\nrow.Count++\nrepo.Update(row)\n\nAfter:\nrepo.InTx(ctx, func(tx Tx) error {\n\trow := tx.GetForUpdate(id)\n\trow.Count++\n\treturn tx.Update(row)\n})\n// or use a single conditional update"}, + "data.missing-transaction-boundary": {Kind: guided, Text: "Group related writes in one transaction.\n\nBefore:\nrepo.CreateOrder(order)\nrepo.CreateLineItems(items)\n\nAfter:\nrepo.InTx(ctx, func(tx Tx) error {\n\tif err := tx.CreateOrder(order); err != nil { return err }\n\treturn tx.CreateLineItems(items)\n})"}, + "data.side-effect-in-transaction": {Kind: guided, Text: "Move external side effects out of rollback-capable transaction bodies.\n\nBefore:\ntx.Do(func() error { save(); return email.Send() })\n\nAfter:\ntx.Do(func() error { save(); return outbox.Insert(emailEvent) })\n// send asynchronously after commit"}, + "data.non-idempotent-consumer": {Kind: guided, Text: "Guard message handlers with a stable idempotency or deduplication key.\n\nBefore:\nfunc Handle(msg Message) error { return charge(msg.Payment) }\n\nAfter:\nfunc Handle(msg Message) error {\n\tif seen(msg.ID) { return nil }\n\treturn recordAndCharge(msg.ID, msg.Payment)\n}"}, + "data.missing-deduplication": {Kind: guided, Text: "Record processed message/event IDs before side effects.\n\nBefore:\nprocess(event)\n\nAfter:\nif dedupe.AlreadyProcessed(event.ID) { return nil }\nreturn dedupe.ProcessOnce(event.ID, func() error { return process(event) })"}, + "data.unsafe-dual-write": {Kind: guided, Text: "Replace independent writes to multiple systems with an explicit consistency strategy.\n\nBefore:\ndb.Save(order)\npublisher.Publish(orderCreated)\n\nAfter:\ntx.Save(order)\ntx.InsertOutbox(orderCreated)\n// relay publishes from the outbox with retry/reconciliation"}, + "data.missing-outbox-strategy": {Kind: guided, Text: "Persist events in an outbox inside the same transaction as the state change.\n\nBefore:\nrepo.Save(entity)\nevents.Publish(EntitySaved{ID: entity.ID})\n\nAfter:\nrepo.InTx(ctx, func(tx Tx) error {\n\ttx.Save(entity)\n\treturn tx.OutboxInsert(EntitySaved{ID: entity.ID})\n})"}, + "data.unstable-pagination": {Kind: deterministic, Text: "Make paginated reads deterministic.\n\nBefore:\nSELECT * FROM orders LIMIT 50 OFFSET 100\n\nAfter:\nSELECT * FROM orders WHERE id > $cursor ORDER BY id ASC LIMIT 50"}, + "data.unbounded-read": {Kind: deterministic, Text: "Bound database reads with a limit, cursor, stream, or selective predicate.\n\nBefore:\nSELECT * FROM events\n\nAfter:\nSELECT * FROM events WHERE created_at >= $since ORDER BY created_at LIMIT 1000"}, + "data.exactly-once-assumption": {Kind: guided, Text: "Design consumers for at-least-once delivery.\n\nBefore:\n// broker delivers exactly once, so no dedupe needed\n\nAfter:\n// delivery is treated as at-least-once; event_id is recorded before side effects"}, + "data.cache-without-policy": {Kind: guided, Text: "Give caches explicit freshness and ownership rules.\n\nBefore:\ncache.Set(key, value)\n\nAfter:\ncache.Set(key, value, cache.WithTTL(5*time.Minute))\n// document who invalidates the key and what stale reads mean"}, + "contracts.non-expand-contract-migration": {Kind: guided, Text: "Split rolling schema changes into expand, migrate, and contract phases.\n\nBefore:\nALTER TABLE users DROP COLUMN legacy_email;\n\nAfter:\n-- release 1: add new_email and dual-write\n-- release 2: backfill and switch reads\n-- release 3: drop legacy_email after verification"}, +} diff --git a/internal/codeguard/rules/catalog_fix_templates_reliability.go b/internal/codeguard/rules/catalog_fix_templates_reliability.go new file mode 100644 index 0000000..12a8fd2 --- /dev/null +++ b/internal/codeguard/rules/catalog_fix_templates_reliability.go @@ -0,0 +1,19 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var reliabilityFixTemplates = map[string]core.FixTemplate{ + "reliability.missing-timeout": {Kind: deterministic, Text: "Bind outbound work to a deadline or timeout.\n\nBefore:\nresp, err := http.Get(url)\n\nAfter:\nctx, cancel := context.WithTimeout(ctx, 5*time.Second)\ndefer cancel()\nreq, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)\nresp, err := client.Do(req)\n// or configure http.Client{Timeout: 5 * time.Second}"}, + "reliability.unbounded-retry": {Kind: guided, Text: "Add a bounded retry policy that stops on context cancellation.\n\nBefore:\nfor {\n\tif err := call(); err == nil { break }\n}\n\nAfter:\nfor attempt := 0; attempt < maxAttempts; attempt++ {\n\tif err := ctx.Err(); err != nil { return err }\n\tif err := call(); err == nil { return nil }\n}"}, + "reliability.retry-without-backoff": {Kind: guided, Text: "Sleep with capped exponential backoff and jitter between attempts.\n\nBefore:\nfor attempt := 0; attempt < maxAttempts; attempt++ { _ = call() }\n\nAfter:\nfor attempt := 0; attempt < maxAttempts; attempt++ {\n\tif err := call(); err == nil { return nil }\n\ttime.Sleep(backoff.WithJitter(attempt))\n}"}, + "reliability.non-idempotent-retry": {Kind: guided, Text: "Do not retry side effects unless the operation is idempotent.\n\nBefore:\nretry(func() error { return chargeCard(ctx, payment) })\n\nAfter:\npayment.IdempotencyKey = stableKey\nretry(func() error { return chargeCard(ctx, payment) })\n// or avoid retrying the non-idempotent side effect"}, + "reliability.missing-cancellation": {Kind: deterministic, Text: "Propagate the caller context instead of creating a detached background context.\n\nBefore:\nreq, _ := http.NewRequestWithContext(context.Background(), method, url, body)\n\nAfter:\nreq, _ := http.NewRequestWithContext(ctx, method, url, body)"}, + "reliability.unbounded-work": {Kind: guided, Text: "Bound fan-out with a worker pool, queue, or semaphore.\n\nBefore:\nfor _, item := range items { go process(item) }\n\nAfter:\nsem := make(chan struct{}, maxConcurrency)\nfor _, item := range items {\n\tsem <- struct{}{}\n\tgo func() { defer func(){ <-sem }(); process(item) }()\n}"}, + "reliability.missing-concurrency-limit": {Kind: guided, Text: "Put an explicit limit around concurrent work.\n\nBefore:\ng.Go(func() error { return process(item) })\n\nAfter:\ng.SetLimit(maxConcurrency)\ng.Go(func() error { return process(item) })"}, + "reliability.resource-leak": {Kind: deterministic, Text: "Close or stop resources on every successful acquisition path.\n\nBefore:\nresp, err := client.Do(req)\nif err != nil { return err }\nreturn decode(resp.Body)\n\nAfter:\nresp, err := client.Do(req)\nif err != nil { return err }\ndefer resp.Body.Close()\nreturn decode(resp.Body)"}, + "reliability.partial-failure-hidden": {Kind: guided, Text: "Return partial-failure evidence instead of reporting overall success.\n\nBefore:\nfor _, item := range items { _ = process(item) }\nreturn nil\n\nAfter:\nvar errs []error\nfor _, item := range items { if err := process(item); err != nil { errs = append(errs, err) } }\nreturn errors.Join(errs...)"}, + "reliability.missing-graceful-shutdown": {Kind: guided, Text: "Handle termination, stop accepting work, and drain in-flight work with a timeout.\n\nBefore:\nlog.Fatal(http.ListenAndServe(addr, mux))\n\nAfter:\nsrv := &http.Server{Addr: addr, Handler: mux}\n// start srv, listen for SIGTERM, then call srv.Shutdown(ctx) with a deadline"}, + "reliability.swallowed-error": {Kind: deterministic, Text: "Do not discard production errors silently.\n\nBefore:\n_, _ = io.Copy(dst, src)\n\nAfter:\nif _, err := io.Copy(dst, src); err != nil {\n\treturn fmt.Errorf(\"copy payload: %w\", err)\n}"}, + "reliability.lost-error-context": {Kind: guided, Text: "Preserve the original error and add operation context.\n\nBefore:\nif err != nil { return errors.New(\"failed\") }\n\nAfter:\nif err != nil { return fmt.Errorf(\"load account %s: %w\", accountID, err) }"}, + "reliability.recoverable-panic": {Kind: guided, Text: "Return an error for recoverable failures instead of panicking.\n\nBefore:\nif err != nil { panic(err) }\n\nAfter:\nif err != nil { return fmt.Errorf(\"initialize worker: %w\", err) }"}, +} diff --git a/internal/codeguard/rules/catalog_reliability.go b/internal/codeguard/rules/catalog_reliability.go new file mode 100644 index 0000000..f4c7f51 --- /dev/null +++ b/internal/codeguard/rules/catalog_reliability.go @@ -0,0 +1,38 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var reliabilityCatalog = map[string]core.RuleMetadata{ + "reliability.missing-timeout": reliabilityRule("reliability.missing-timeout", "fail", "Missing timeout", "Fails when outbound production calls are made without a bounded timeout or deadline.", "Pass a context with deadline/timeout, configure the client timeout, or route the call through a bounded project wrapper."), + "reliability.unbounded-retry": reliabilityRule("reliability.unbounded-retry", "fail", "Unbounded retry", "Fails when retry logic can continue without a clear attempt limit or cancellation boundary.", "Add a maximum attempt count and stop retrying when the caller context is cancelled."), + "reliability.retry-without-backoff": reliabilityRule("reliability.retry-without-backoff", "warn", "Retry without backoff", "Warns when retry logic repeats immediately without backoff or jitter.", "Use exponential backoff with jitter and cap the maximum delay."), + "reliability.non-idempotent-retry": reliabilityRule("reliability.non-idempotent-retry", "fail", "Non-idempotent retry", "Fails when retry logic wraps a non-idempotent operation without idempotency evidence.", "Add an idempotency key, deduplication guard, or avoid retrying the side effect."), + "reliability.missing-cancellation": reliabilityRule("reliability.missing-cancellation", "warn", "Missing cancellation propagation", "Warns when request or job code drops caller cancellation by using a background context for downstream work.", "Propagate the caller context into downstream calls and goroutines."), + "reliability.unbounded-work": reliabilityRule("reliability.unbounded-work", "warn", "Unbounded work", "Warns when goroutines, workers, queues, or buffers can grow without an explicit bound.", "Add a worker pool, semaphore, bounded queue, or backpressure strategy."), + "reliability.missing-concurrency-limit": reliabilityRule("reliability.missing-concurrency-limit", "warn", "Missing concurrency limit", "Warns when concurrent work is launched without an obvious limit.", "Limit concurrency with a semaphore, worker pool, errgroup limit, or bounded queue."), + "reliability.resource-leak": reliabilityRule("reliability.resource-leak", "fail", "Resource leak", "Fails when opened resources such as response bodies, rows, files, tickers, or timers are not closed or stopped.", "Close or stop the resource on every path, and handle cleanup errors where they matter."), + "reliability.partial-failure-hidden": reliabilityRule("reliability.partial-failure-hidden", "fail", "Partial failure hidden", "Fails when batch or multi-step work can fail partially but still report success.", "Return structured partial-failure information or fail the operation explicitly."), + "reliability.missing-graceful-shutdown": reliabilityRule("reliability.missing-graceful-shutdown", "warn", "Missing graceful shutdown", "Warns when servers or long-running workers start without shutdown/drain handling.", "Handle termination signals, stop accepting new work, drain in-flight work, and close resources with a timeout."), + "reliability.swallowed-error": reliabilityRule("reliability.swallowed-error", "fail", "Swallowed error", "Fails when an error is discarded or ignored in production code.", "Return, wrap, or explicitly handle the error; only ignore errors with a documented safe reason."), + "reliability.lost-error-context": reliabilityRule("reliability.lost-error-context", "warn", "Lost error context", "Warns when an error is replaced without operation context or wrapping.", "Wrap errors with operation context so callers can diagnose the failing dependency or step."), + "reliability.recoverable-panic": reliabilityRule("reliability.recoverable-panic", "fail", "Recoverable failure handled with panic", "Fails when a recoverable runtime condition is handled with panic in production code.", "Return an error or use an explicit failure result for recoverable conditions."), +} + +func reliabilityRule(id string, level string, title string, description string, howToFix string) core.RuleMetadata { + return core.RuleMetadata{ + ID: id, + Section: "Reliability", + DefaultLevel: level, + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage( + core.RuleLanguageGo, + core.RuleLanguageTypeScript, + core.RuleLanguageJavaScript, + core.RuleLanguagePython, + core.RuleLanguageCPP, + ), + Title: title, + Description: description, + HowToFix: howToFix, + } +} diff --git a/internal/codeguard/runner/checks/registry.go b/internal/codeguard/runner/checks/registry.go index b7e30e6..e8b1106 100644 --- a/internal/codeguard/runner/checks/registry.go +++ b/internal/codeguard/runner/checks/registry.go @@ -6,10 +6,12 @@ import ( agentContextCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/agentcontext" ciCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/ci" contractsCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/contracts" + dataCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/data" designCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/design" performanceCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/performance" promptsCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/prompts" qualityCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/quality" + reliabilityCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/reliability" securityCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/security" supplyChainCheck "github.com/devr-tools/codeguard/internal/codeguard/checks/supplychain" checkSupport "github.com/devr-tools/codeguard/internal/codeguard/checks/support" @@ -58,6 +60,26 @@ var sectionRegistry = []sectionDef{ return performanceCheck.Run(ctx, checkEnv) }, }, + { + id: "reliability", + name: "Reliability", + enabled: func(sc runnersupport.Context) bool { + return sc.Cfg.Checks.Reliability != nil && *sc.Cfg.Checks.Reliability + }, + run: func(ctx context.Context, _ runnersupport.Context, checkEnv checkSupport.Context) core.SectionResult { + return reliabilityCheck.Run(ctx, checkEnv) + }, + }, + { + id: "data", + name: "Data Correctness", + enabled: func(sc runnersupport.Context) bool { + return sc.Cfg.Checks.Data != nil && *sc.Cfg.Checks.Data + }, + run: func(ctx context.Context, _ runnersupport.Context, checkEnv checkSupport.Context) core.SectionResult { + return dataCheck.Run(ctx, checkEnv) + }, + }, { id: "design", name: "Design", diff --git a/internal/codeguard/runner/pr_summary.go b/internal/codeguard/runner/pr_summary.go new file mode 100644 index 0000000..e496a8d --- /dev/null +++ b/internal/codeguard/runner/pr_summary.go @@ -0,0 +1,119 @@ +package runner + +import ( + "sort" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" +) + +func addPRSummaryArtifact(sc runnersupport.Context, sections []core.SectionResult) { + cfg := sc.Cfg.Checks.ProductionRisk + if cfg.Enabled == nil || !*cfg.Enabled || sc.Opts.Mode != core.ScanModeDiff { + return + } + metric := productionRiskMetric(cfg, sections) + if metric == nil { + return + } + sc.Artifacts.Put(core.Artifact{ + ID: "pr_summary", + Kind: core.ReportArtifactKindPRSummary, + PRSummary: &core.PRSummaryArtifact{ + ProductionRisk: metric, + }, + }) +} + +func productionRiskMetric(cfg core.ProductionRiskConfig, sections []core.SectionResult) *core.PRSummaryMetric { + componentsByLabel := map[string]*core.PRSummaryComponent{} + for _, section := range sections { + for _, finding := range section.Findings { + weight := 0 + switch { + case strings.HasPrefix(finding.RuleID, "reliability."): + weight += cfg.ReliabilityWeight + case strings.HasPrefix(finding.RuleID, "data."): + weight += cfg.DataWeight + case finding.RuleID == "contracts.non-expand-contract-migration": + weight += cfg.DataWeight + default: + continue + } + switch strings.ToLower(finding.Level) { + case "fail", "error": + weight += cfg.FailWeight + case "warn", "warning": + weight += cfg.WarnWeight + } + label := productionRiskLabel(finding) + component := componentsByLabel[label] + if component == nil { + component = &core.PRSummaryComponent{ + Label: label, + Weight: weight, + Detail: productionRiskDetail(finding), + } + componentsByLabel[label] = component + } + component.Count++ + component.Contribution += weight + } + } + if len(componentsByLabel) == 0 { + return nil + } + components := make([]core.PRSummaryComponent, 0, len(componentsByLabel)) + total := 0 + for _, component := range componentsByLabel { + components = append(components, *component) + total += component.Contribution + } + sort.Slice(components, func(i, j int) bool { + if components[i].Contribution != components[j].Contribution { + return components[i].Contribution > components[j].Contribution + } + return components[i].Label < components[j].Label + }) + score := minRiskScore(total) + return &core.PRSummaryMetric{ + Score: score, + Level: productionRiskLevel(score, cfg), + Components: components, + } +} + +func productionRiskLabel(finding core.Finding) string { + if strings.HasPrefix(finding.RuleID, "reliability.") { + return "reliability" + } + if strings.HasPrefix(finding.RuleID, "data.") || finding.RuleID == "contracts.non-expand-contract-migration" { + return "data_correctness" + } + return "production" +} + +func productionRiskDetail(finding core.Finding) string { + if strings.HasPrefix(finding.RuleID, "reliability.") { + return "active reliability findings in changed code" + } + if strings.HasPrefix(finding.RuleID, "data.") { + return "active data-correctness findings in changed code" + } + if finding.RuleID == "contracts.non-expand-contract-migration" { + return "unsafe rolling schema migration finding" + } + return finding.RuleID +} + +func productionRiskLevel(score int, cfg core.ProductionRiskConfig) string { + switch { + case cfg.FailThreshold > 0 && score >= cfg.FailThreshold: + return "fail" + case cfg.WarnThreshold > 0 && score >= cfg.WarnThreshold: + return "warn" + default: + return "pass" + } +} diff --git a/internal/codeguard/runner/pr_summary_test.go b/internal/codeguard/runner/pr_summary_test.go new file mode 100644 index 0000000..90a4d63 --- /dev/null +++ b/internal/codeguard/runner/pr_summary_test.go @@ -0,0 +1,77 @@ +package runner + +import ( + "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/core" + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" +) + +func TestAddPRSummaryArtifactScoresProductionRisk(t *testing.T) { + enabled := true + sc := runnersupport.Context{ + Opts: core.ScanOptions{Mode: core.ScanModeDiff}, + Cfg: core.Config{Checks: core.CheckConfig{ProductionRisk: core.ProductionRiskConfig{ + Enabled: &enabled, WarnThreshold: 35, FailThreshold: 70, + ReliabilityWeight: 12, DataWeight: 15, FailWeight: 25, WarnWeight: 10, + }}}, + Artifacts: runnersupport.NewArtifactStore(), + } + sections := []core.SectionResult{ + {Name: "Reliability", Findings: []core.Finding{ + {RuleID: "reliability.missing-timeout", Level: "fail", Path: "client.go"}, + {RuleID: "reliability.unbounded-work", Level: "warn", Path: "worker.go"}, + }}, + {Name: "Data Correctness", Findings: []core.Finding{ + {RuleID: "data.missing-outbox-strategy", Level: "fail", Path: "service.go"}, + }}, + } + + addPRSummaryArtifact(sc, sections) + + artifact := requirePRSummaryArtifact(t, sc.Artifacts.List()) + if artifact.ProductionRisk == nil { + t.Fatal("expected production risk metric") + } + if artifact.ProductionRisk.Score != 99 { + t.Fatalf("production risk score = %d, want 99", artifact.ProductionRisk.Score) + } + if artifact.ProductionRisk.Level != "fail" { + t.Fatalf("production risk level = %q, want fail", artifact.ProductionRisk.Level) + } + if len(artifact.ProductionRisk.Components) != 2 { + t.Fatalf("components = %#v, want reliability and data", artifact.ProductionRisk.Components) + } + if artifact.ProductionRisk.Components[0].Label != "reliability" { + t.Fatalf("first component = %#v, want reliability first by contribution", artifact.ProductionRisk.Components[0]) + } +} + +func TestAddPRSummaryArtifactSkipsFullScans(t *testing.T) { + enabled := true + sc := runnersupport.Context{ + Opts: core.ScanOptions{Mode: core.ScanModeFull}, + Cfg: core.Config{Checks: core.CheckConfig{ProductionRisk: core.ProductionRiskConfig{Enabled: &enabled}}}, + Artifacts: runnersupport.NewArtifactStore(), + } + + addPRSummaryArtifact(sc, []core.SectionResult{{Findings: []core.Finding{{RuleID: "reliability.missing-timeout", Level: "fail"}}}}) + + if got := sc.Artifacts.List(); len(got) != 0 { + t.Fatalf("artifacts = %#v, want none for full scan", got) + } +} + +func requirePRSummaryArtifact(t *testing.T, artifacts []core.Artifact) *core.PRSummaryArtifact { + t.Helper() + for _, artifact := range artifacts { + if artifact.Kind == core.ReportArtifactKindPRSummary { + if artifact.PRSummary == nil { + t.Fatal("pr_summary artifact missing payload") + } + return artifact.PRSummary + } + } + t.Fatalf("pr_summary artifact not found: %#v", artifacts) + return nil +} diff --git a/internal/codeguard/runner/runner.go b/internal/codeguard/runner/runner.go index 0d6b075..125b3d0 100644 --- a/internal/codeguard/runner/runner.go +++ b/internal/codeguard/runner/runner.go @@ -61,6 +61,7 @@ func RunWithOptions(ctx context.Context, cfg core.Config, opts core.ScanOptions) sc.Artifacts.Put(*triageArtifact) } addRiskArtifacts(sc, sections) + addPRSummaryArtifact(sc, sections) if ruleStats := sc.RuleStats.Snapshot(); len(ruleStats) > 0 { sc.Artifacts.Put(core.NewRuleStatsArtifact(ruleStats)) runnersupport.RecordRuleStatsHistory(sc, ruleStats) diff --git a/pkg/codeguard/sdk_types_config_checks.go b/pkg/codeguard/sdk_types_config_checks.go index 6fe2319..cb7d6c6 100644 --- a/pkg/codeguard/sdk_types_config_checks.go +++ b/pkg/codeguard/sdk_types_config_checks.go @@ -8,6 +8,9 @@ type DesignRulesConfig = core.DesignRulesConfig type PromptRulesConfig = core.PromptRulesConfig type CIRulesConfig = core.CIRulesConfig type SupplyChainRulesConfig = core.SupplyChainRulesConfig +type ReliabilityRulesConfig = core.ReliabilityRulesConfig +type DataRulesConfig = core.DataRulesConfig +type ProductionRiskConfig = core.ProductionRiskConfig type ContractRulesConfig = core.ContractRulesConfig type ContextRulesConfig = core.ContextRulesConfig diff --git a/pkg/codeguard/sdk_types_runtime_report.go b/pkg/codeguard/sdk_types_runtime_report.go index bc67d58..d5828da 100644 --- a/pkg/codeguard/sdk_types_runtime_report.go +++ b/pkg/codeguard/sdk_types_runtime_report.go @@ -17,6 +17,9 @@ type ( FileRiskEntry = core.FileRiskEntry FileRiskComponent = core.FileRiskComponent PRHotspotsArtifact = core.PRHotspotsArtifact + PRSummaryArtifact = core.PRSummaryArtifact + PRSummaryMetric = core.PRSummaryMetric + PRSummaryComponent = core.PRSummaryComponent SlopHistoryEntry = core.SlopHistoryEntry PerformanceScoreArtifact = core.PerformanceScoreArtifact PerformanceHistoryEntry = core.PerformanceHistoryEntry diff --git a/tests/checks/contracts_test.go b/tests/checks/contracts_test.go index b32bcbf..2223350 100644 --- a/tests/checks/contracts_test.go +++ b/tests/checks/contracts_test.go @@ -262,12 +262,16 @@ func TestContractsMigrationDestructiveFlagsNewMigrationsOnly(t *testing.T) { runGit(t, dir, "add", ".") report := runContractsDiff(t, contractsTestConfig(dir)) - assertSectionStatus(t, report, "API Contracts", "warn") - findings := contractsRuleFindings(report, "contracts.migration-destructive") - if len(findings) != 4 { - t.Fatalf("migration findings = %d, want 4: %+v", len(findings), findings) + assertSectionStatus(t, report, "API Contracts", "fail") + legacyFindings := contractsRuleFindings(report, "contracts.migration-destructive") + if len(legacyFindings) != 4 { + t.Fatalf("legacy migration findings = %d, want 4: %+v", len(legacyFindings), legacyFindings) + } + nonExpandFindings := contractsRuleFindings(report, "contracts.non-expand-contract-migration") + if len(nonExpandFindings) != 4 { + t.Fatalf("non-expand migration findings = %d, want 4: %+v", len(nonExpandFindings), nonExpandFindings) } - for _, finding := range findings { + for _, finding := range append(legacyFindings, nonExpandFindings...) { if finding.Path != "migrations/0002_cleanup.sql" { t.Fatalf("unexpected finding path %q (only the new migration should be flagged)", finding.Path) } @@ -293,13 +297,16 @@ func TestContractsFullScanRunsOnlyMigrationRule(t *testing.T) { if err != nil { t.Fatalf("full scan: %v", err) } - assertSectionStatus(t, report, "API Contracts", "warn") + assertSectionStatus(t, report, "API Contracts", "fail") if findings := contractsRuleFindings(report, "contracts.migration-destructive"); len(findings) != 1 { - t.Fatalf("migration findings = %d, want 1", len(findings)) + t.Fatalf("legacy migration findings = %d, want 1", len(findings)) + } + if findings := contractsRuleFindings(report, "contracts.non-expand-contract-migration"); len(findings) != 1 { + t.Fatalf("non-expand migration findings = %d, want 1", len(findings)) } for _, section := range report.Sections { for _, finding := range section.Findings { - if finding.RuleID != "contracts.migration-destructive" { + if finding.RuleID != "contracts.migration-destructive" && finding.RuleID != "contracts.non-expand-contract-migration" { t.Fatalf("unexpected non-migration finding in full scan: %s", finding.RuleID) } } diff --git a/tests/checks/data_multilang_test.go b/tests/checks/data_multilang_test.go new file mode 100644 index 0000000..fbbd6ad --- /dev/null +++ b/tests/checks/data_multilang_test.go @@ -0,0 +1,315 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func dataLangConfig(name string, dir string, language string) codeguard.Config { + cfg := dataConfig(name, dir) + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: language}} + return cfg +} + +func TestDataPythonDetectsTransactionPaginationAndConsumerGaps(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "service.py"), ` +def save_and_publish(repo, bus, cache): + repo.save(order) + repo.update(order) + bus.publish(event) + cache.set("order", order) + session.execute("SELECT * FROM orders LIMIT 20 OFFSET 40") + +def handle_message(email, event): + email.send(event) +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-python", dir, "python")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRulePresent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRulePresent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRulePresent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-deduplication") + assertFindingRulePresent(t, report, "Data Correctness", "data.cache-without-policy") +} + +func TestDataPythonDetectsUnboundedReadAndExactlyOnceAssumption(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "queries.py"), ` +def list_orders(session): + return session.execute("SELECT * FROM orders") + +# broker provides exactly once delivery +def consume(event): + return event +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-python-read", dir, "python")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.unbounded-read") + assertFindingRulePresent(t, report, "Data Correctness", "data.exactly-once-assumption") +} + +func TestDataPythonAcceptsTransactionalOutboxAndBoundedPolicies(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe_service.py"), ` +def save_and_publish(repo, outbox, cache, session, event): + with transaction.atomic(): + repo.save(order) + repo.update(order) + outbox.publish(event) + cache.set("order", order, timeout=60) + session.execute("SELECT * FROM orders WHERE account_id = :id ORDER BY id LIMIT 20") + +# exactly once is handled by idempotency and dedupe keys +def handle_message(email, event): + if processed.exists(event.message_id): + return + email.send(event) +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-python-safe", dir, "python")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unbounded-read") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-deduplication") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.cache-without-policy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.exactly-once-assumption") +} + +func TestDataTypeScriptDetectsTransactionPaginationAndConsumerGaps(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "service.ts"), ` +async function saveAndPublish(db, bus, cache) { + await db.order.create({}); + await db.order.update({}); + await bus.publish(event); + cache.set("order", order); + await db.order.findMany({ skip: 20 }); +} + +async function handleMessage(email, event) { + await email.send(event); +} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-ts", dir, "typescript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRulePresent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRulePresent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRulePresent(t, report, "Data Correctness", "data.unbounded-read") + assertFindingRulePresent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-deduplication") + assertFindingRulePresent(t, report, "Data Correctness", "data.cache-without-policy") +} + +func TestDataTypeScriptDetectsExactlyOnceAssumption(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "consumer.ts"), ` +// broker gives exactly once delivery +export function consume(event: Event) { + return event; +} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-ts-exactly-once", dir, "typescript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.exactly-once-assumption") +} + +func TestDataTypeScriptAcceptsTransactionalOutboxAndBoundedPolicies(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe_service.ts"), ` +async function saveAndPublish(db, outbox, cache, event) { + await db.$transaction(async (tx) => { + await tx.order.create({}); + await tx.order.update({}); + await outbox.publish(event); + }); + cache.set("order", event, { ttl: 60 }); + await db.order.findMany({ where: { accountId: event.accountId }, orderBy: { id: "asc" }, take: 20 }); +} + +// exactly once is handled by idempotency and dedupe keys +async function handleMessage(email, event) { + if (await processed.has(event.messageId)) return; + await email.send(event); +} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-ts-safe", dir, "typescript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unbounded-read") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-deduplication") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.cache-without-policy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.exactly-once-assumption") +} + +func TestDataJavaScriptUsesSameDetector(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "service.js"), ` +async function list(db) { + return db.order.findMany({}); +} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-js", dir, "javascript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.unbounded-read") +} + +func TestDataJavaScriptDetectsTransactionPaginationConsumerAndExactlyOnceGaps(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "service.js"), ` +async function saveAndPublish(db, bus, cache) { + await db.order.create({}); + await db.order.update({}); + await bus.publish(event); + cache.set("order", order); + await db.order.findMany({ skip: 20 }); +} + +// broker gives exactly once delivery +async function handleMessage(email, event) { + await email.send(event); +} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-js-gaps", dir, "javascript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRulePresent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRulePresent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRulePresent(t, report, "Data Correctness", "data.unbounded-read") + assertFindingRulePresent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-deduplication") + assertFindingRulePresent(t, report, "Data Correctness", "data.cache-without-policy") + assertFindingRulePresent(t, report, "Data Correctness", "data.exactly-once-assumption") +} + +func TestDataCPPDetectsTransactionPaginationAndConsumerGaps(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "service.cpp"), ` +void SaveAndPublish(Repo& repo, Bus& bus, Cache& cache) { + repo.save(order); + repo.update(order); + bus.publish(event); + cache.set("order", order); + db.offset(20); +} + +void HandleMessage(Email& email, Event event) { + email.send(event); +} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-cpp", dir, "cpp")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRulePresent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRulePresent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRulePresent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-deduplication") + assertFindingRulePresent(t, report, "Data Correctness", "data.cache-without-policy") +} + +func TestDataCPPDetectsUnboundedReadAndExactlyOnceAssumption(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "queries.cpp"), ` +void ListOrders(DB& db) { + db.query("SELECT * FROM orders"); +} + +// broker gives exactly once delivery +void Consume(Event event) {} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-cpp-read", dir, "cpp")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.unbounded-read") + assertFindingRulePresent(t, report, "Data Correctness", "data.exactly-once-assumption") +} + +func TestDataCPPAcceptsTransactionalOutboxAndBoundedPolicies(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe_service.cpp"), ` +void SaveAndPublish(Repo& repo, Outbox& outbox, Cache& cache, DB& db, Event event) { + auto txn = db.begin_tx(); + repo.save(txn, order); + repo.update(txn, order); + outbox.publish(event); + cache.set("order", order, ttl); + db.query().where(account_id).order_by(id).limit(20); +} + +// exactly once is handled by idempotency and dedupe keys +void HandleMessage(Email& email, Event event) { + if (processed.contains(event.message_id)) return; + email.send(event); +} +`) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-cpp-safe", dir, "cpp")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unbounded-read") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-deduplication") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.cache-without-policy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.exactly-once-assumption") +} diff --git a/tests/checks/data_test.go b/tests/checks/data_test.go new file mode 100644 index 0000000..2ebaf92 --- /dev/null +++ b/tests/checks/data_test.go @@ -0,0 +1,229 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func dataConfig(name string, dir string) codeguard.Config { + cfg := codeguard.ExampleConfig() + cfg.Name = name + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: "go"}} + cfg.Checks.Quality = false + cfg.Checks.Design = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + cfg.Checks.SupplyChain = false + on := true + off := false + cfg.Checks.Reliability = &off + cfg.Checks.Data = &on + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + return cfg +} + +func TestDataGoDetectsMissingTransactionAndDualWrite(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "service.go"), `package sample + +type Repo interface { + Save(any) error + Update(any) error +} + +type Publisher interface { + Publish(any) error +} + +func SaveAndPublish(repo Repo, publisher Publisher, order any, event any) error { + if err := repo.Save(order); err != nil { + return err + } + if err := repo.Update(order); err != nil { + return err + } + return publisher.Publish(event) +} +`) + + report, err := codeguard.Run(context.Background(), dataConfig("data-dual-write", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRulePresent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-outbox-strategy") +} + +func TestDataGoDetectsUnstablePaginationAndUnboundedRead(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "queries.go"), `package sample + +const unstable = "SELECT * FROM orders LIMIT 50 OFFSET 100" +const unbounded = "SELECT * FROM events" +`) + + report, err := codeguard.Run(context.Background(), dataConfig("data-sql", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.unstable-pagination") + assertFindingRulePresent(t, report, "Data Correctness", "data.unbounded-read") +} + +func TestDataGoDetectsConsumerWithoutDeduplication(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "consumer.go"), `package sample + +type Emailer interface { + Send(any) error +} + +func HandleMessage(email Emailer, event any) error { + return email.Send(event) +} +`) + + report, err := codeguard.Run(context.Background(), dataConfig("data-consumer", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.non-idempotent-consumer") + assertFindingRulePresent(t, report, "Data Correctness", "data.missing-deduplication") +} + +func TestDataGoDetectsReadModifyWriteTransactionSideEffectCacheAndExactlyOnce(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "risks.go"), `package sample + +type Repo interface { + Get(string) (Order, error) + Save(Order) error +} + +type DB interface { + Transaction(func(Tx) error) error +} + +type Tx interface{} + +type Publisher interface { + Publish(any) error +} + +type Cache interface { + Set(string, any) +} + +type Order struct{} + +func UpdateDerived(repo Repo, id string) error { + order, err := repo.Get(id) + if err != nil { + return err + } + return repo.Save(order) +} + +func PublishInsideTransaction(db DB, publisher Publisher, event any) error { + return db.Transaction(func(tx Tx) error { + return publisher.Publish(event) + }) +} + +func CacheOrder(cache Cache, order Order) { + cache.Set("order", order) +} + +// consumer assumes exactly once delivery from the broker +func Consumer() {} +`) + + report, err := codeguard.Run(context.Background(), dataConfig("data-more-go-risks", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.read-modify-write-race") + assertFindingRulePresent(t, report, "Data Correctness", "data.side-effect-in-transaction") + assertFindingRulePresent(t, report, "Data Correctness", "data.cache-without-policy") + assertFindingRulePresent(t, report, "Data Correctness", "data.exactly-once-assumption") +} + +func TestDataGoAcceptsTransactionalOutboxBoundedQueryAndPolicy(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe.go"), `package sample + +type Repo interface { + WithTx(func(Tx) error) error + Query(string) ([]Order, error) +} + +type Tx interface { + Get(string) (Order, error) + Save(Order) error +} + +type Outbox interface { + Publish(any) error +} + +type Cache interface { + Set(string, any, int) +} + +type Event struct { + MessageID string +} + +type Order struct{} + +func UpdateWithOutbox(repo Repo, outbox Outbox, cache Cache, event Event) error { + if err := repo.WithTx(func(tx Tx) error { + order, err := tx.Get(event.MessageID) + if err != nil { + return err + } + if err := tx.Save(order); err != nil { + return err + } + return outbox.Publish(event) + }); err != nil { + return err + } + cache.Set("order", event, 60) + _, err := repo.Query("SELECT * FROM orders WHERE account_id = ? ORDER BY id LIMIT 20") + return err +} + +// exactly once is handled through idempotency and dedupe records +func HandleEvent(event Event) error { + if processedByMessageID(event.MessageID) { + return nil + } + return nil +} + +func processedByMessageID(string) bool { return false } +`) + + report, err := codeguard.Run(context.Background(), dataConfig("data-safe-go", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Data Correctness", "data.read-modify-write-race") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-transaction-boundary") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.unsafe-dual-write") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.missing-outbox-strategy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.cache-without-policy") + assertFindingRuleAbsent(t, report, "Data Correctness", "data.exactly-once-assumption") +} diff --git a/tests/checks/reliability_multilang_test.go b/tests/checks/reliability_multilang_test.go new file mode 100644 index 0000000..402e0df --- /dev/null +++ b/tests/checks/reliability_multilang_test.go @@ -0,0 +1,334 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func reliabilityLangConfig(name string, dir string, language string) codeguard.Config { + cfg := reliabilityConfig(name, dir) + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: language}} + return cfg +} + +func TestReliabilityPythonDetectsTimeoutWorkAndSwallowedError(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "worker.py"), ` +import asyncio +import requests + +def fetch(url): + return requests.get(url) + +async def run(items): + for item in items: + asyncio.create_task(fetch(item)) + +def handle(): + try: + fetch("https://example.com") + except Exception: + pass +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-python", dir, "python")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.missing-timeout") + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-work") + assertFindingRulePresent(t, report, "Reliability", "reliability.swallowed-error") +} + +func TestReliabilityPythonDetectsRetryResourceAndGenericRaise(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "payments.py"), ` +def charge_with_retry(payments, card): + while True: + retry_attempt = payments.charge(card) + if retry_attempt: + return retry_attempt + +def load_widget(): + handle = open("/tmp/widget.txt") + use(handle) + use(handle) + use(handle) + use(handle) + use(handle) + use(handle) + raise RuntimeError("widget failed") +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-python-retry", dir, "python")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.retry-without-backoff") + assertFindingRulePresent(t, report, "Reliability", "reliability.non-idempotent-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.resource-leak") + assertFindingRulePresent(t, report, "Reliability", "reliability.recoverable-panic") +} + +func TestReliabilityPythonAcceptsBoundedPatterns(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe_worker.py"), ` +import asyncio +import logging +import requests + +async def run(items): + limit = asyncio.Semaphore(4) + async def worker(item): + async with limit: + return requests.get(item, timeout=5) + return [asyncio.create_task(worker(item)) for item in items] + +def load(): + try: + with open("/tmp/widget.txt") as handle: + return handle.read() + except Exception as err: + logging.exception("load failed", exc_info=err) + raise +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-python-safe", dir, "python")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Reliability", "reliability.missing-timeout") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.unbounded-work") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.swallowed-error") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.resource-leak") +} + +func TestReliabilityTypeScriptDetectsTimeoutWorkAndSwallowedError(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "worker.ts"), ` +async function fetchUser(id: string) { + return fetch("/users/" + id); +} + +async function run(ids: string[]) { + for (const id of ids) { + fetchUser(id); + } +} + +async function handle() { + try { + await fetchUser("1"); + } catch (err) { return; } +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-ts", dir, "typescript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.missing-timeout") + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-work") + assertFindingRulePresent(t, report, "Reliability", "reliability.swallowed-error") +} + +func TestReliabilityTypeScriptDetectsRetryAndGenericThrow(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "payments.ts"), ` +async function retryCharge(payments: Payments, card: Card) { + while (true) { + const retryAttempt = await payments.charge(card); + if (retryAttempt) return retryAttempt; + } +} + +function failPayment() { + throw new Error("payment failed"); +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-ts-retry", dir, "typescript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.retry-without-backoff") + assertFindingRulePresent(t, report, "Reliability", "reliability.non-idempotent-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.recoverable-panic") +} + +func TestReliabilityTypeScriptAcceptsBoundedPatterns(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe_worker.ts"), ` +import pLimit from "p-limit"; + +async function fetchUser(url: string, signal: AbortSignal) { + return fetch(url, { signal }); +} + +async function run(ids: string[], signal: AbortSignal) { + const limit = pLimit(4); + return Promise.all(ids.map((id) => limit(() => fetchUser("/users/" + id, signal)))); +} + +async function handle() { + try { + await fetchUser("/users/1", new AbortController().signal); + } catch (err) { + throw err; + } +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-ts-safe", dir, "typescript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Reliability", "reliability.missing-timeout") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.unbounded-work") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.swallowed-error") +} + +func TestReliabilityJavaScriptUsesSameDetector(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "worker.js"), ` +async function fetchUser(id) { + return fetch("/users/" + id); +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-js", dir, "javascript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.missing-timeout") +} + +func TestReliabilityJavaScriptDetectsRetryAndGenericThrow(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "payments.js"), ` +async function retryCharge(payments, card) { + while (true) { + const retryAttempt = await payments.charge(card); + if (retryAttempt) return retryAttempt; + } +} + +function failPayment() { + throw new Error("payment failed"); +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-js-retry", dir, "javascript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.retry-without-backoff") + assertFindingRulePresent(t, report, "Reliability", "reliability.non-idempotent-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.recoverable-panic") +} + +func TestReliabilityCPPDetectsUnboundedWorkAndResourceLeak(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "worker.cpp"), ` +#include + +void run(int n) { + for (int i = 0; i < n; ++i) { + std::thread([]{}).detach(); + } + auto* item = new Widget(); + use(item); + more(); + more(); + more(); + more(); + more(); + more(); + more(); + more(); + more(); +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-cpp", dir, "cpp")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-work") + assertFindingRulePresent(t, report, "Reliability", "reliability.resource-leak") +} + +func TestReliabilityCPPDetectsRetryAndGenericThrow(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "payments.cpp"), ` +#include + +void ChargeWithRetry(Payments& payments, Card card) { + while (true) { + auto retry_attempt = payments.Charge(card); + if (retry_attempt.ok()) return; + } +} + +void FailPayment() { + throw std::runtime_error("payment failed"); +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-cpp-retry", dir, "cpp")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.retry-without-backoff") + assertFindingRulePresent(t, report, "Reliability", "reliability.non-idempotent-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.recoverable-panic") +} + +func TestReliabilityCPPAcceptsBoundedPatterns(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe_worker.cpp"), ` +#include +#include +#include + +void Run(const std::vector& items, thread_pool& pool) { + for (auto item : items) { + pool.enqueue([item] {}); + } + auto item = std::make_unique(); + for (int backoff_attempt = 0; backoff_attempt < 3; ++backoff_attempt) { + auto retry_attempt_with_backoff = client.Get(idempotency_key); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + if (retry_attempt_with_backoff.ok()) return; + } +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-cpp-safe", dir, "cpp")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Reliability", "reliability.unbounded-work") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.resource-leak") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.retry-without-backoff") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.unbounded-retry") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.non-idempotent-retry") +} diff --git a/tests/checks/reliability_test.go b/tests/checks/reliability_test.go new file mode 100644 index 0000000..bbfb4d3 --- /dev/null +++ b/tests/checks/reliability_test.go @@ -0,0 +1,164 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func reliabilityConfig(name string, dir string) codeguard.Config { + cfg := codeguard.ExampleConfig() + cfg.Name = name + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: "go"}} + cfg.Checks.Quality = false + cfg.Checks.Design = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + cfg.Checks.SupplyChain = false + on := true + off := false + cfg.Checks.Reliability = &on + cfg.Checks.Data = &off + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + return cfg +} + +func TestReliabilityGoDetectsMissingTimeoutAndResourceLeak(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "client.go"), `package sample + +import "net/http" + +func Fetch(url string) error { + resp, err := http.Get(url) + if err != nil { + return err + } + _ = resp + return nil +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityConfig("reliability-http", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.missing-timeout") + assertFindingRulePresent(t, report, "Reliability", "reliability.resource-leak") +} + +func TestReliabilityGoDetectsCancellationAndUnboundedWork(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "worker.go"), `package sample + +import "context" + +func Run(ctx context.Context, items []int) { + child := context.Background() + _ = child + for _, item := range items { + go process(item) + } +} + +func process(int) {} +`) + + report, err := codeguard.Run(context.Background(), reliabilityConfig("reliability-work", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.missing-cancellation") + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-work") +} + +func TestReliabilityGoDetectsSwallowedErrorAndPanic(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "errors.go"), `package sample + +import "io" + +func Copy(dst io.Writer, src io.Reader) { + _, _ = io.Copy(dst, src) + panic("copy failed") +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityConfig("reliability-errors", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.swallowed-error") + assertFindingRulePresent(t, report, "Reliability", "reliability.recoverable-panic") +} + +func TestReliabilityGoDetectsRetryRisk(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "retry.go"), `package sample + +func ChargeWithRetry(payments Payments, card Card) error { + for { + if err := payments.Charge(card); err != nil { + continue + } + return nil + } +} + +type Payments interface { + Charge(Card) error +} + +type Card struct{} +`) + + report, err := codeguard.Run(context.Background(), reliabilityConfig("reliability-retry", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.unbounded-retry") + assertFindingRulePresent(t, report, "Reliability", "reliability.retry-without-backoff") + assertFindingRulePresent(t, report, "Reliability", "reliability.non-idempotent-retry") +} + +func TestReliabilityGoDoesNotFlagBoundedHTTPWithContextAndCleanup(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "client.go"), `package sample + +import ( + "context" + "net/http" + "time" +) + +func Fetch(ctx context.Context, url string) error { + client := &http.Client{Timeout: 2 * time.Second} + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return err + } + resp, err := client.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + return nil +} +`) + + report, err := codeguard.Run(context.Background(), reliabilityConfig("reliability-bounded-http", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Reliability", "reliability.missing-timeout") + assertFindingRuleAbsent(t, report, "Reliability", "reliability.resource-leak") +} diff --git a/tests/cli/features_metadata_test.go b/tests/cli/features_metadata_test.go index 78ae234..862d77b 100644 --- a/tests/cli/features_metadata_test.go +++ b/tests/cli/features_metadata_test.go @@ -53,6 +53,51 @@ func TestSDKRuleMetadataForSupplyChainRule(t *testing.T) { assertLanguageCoverage(t, rule, codeguard.RuleLanguageCoverageRepositoryWide) } +func TestSDKRuleMetadataForReliabilityRule(t *testing.T) { + rule := requireRuleMetadata(t, "reliability.missing-timeout") + assertExecutionModel(t, rule, codeguard.RuleExecutionModelLanguageAgnostic) + assertLanguageCoverage( + t, + rule, + codeguard.RuleLanguageCoverageFixed, + codeguard.RuleLanguageCPP, + codeguard.RuleLanguageGo, + codeguard.RuleLanguageJavaScript, + codeguard.RuleLanguagePython, + codeguard.RuleLanguageTypeScript, + ) + if rule.FixTemplate.Kind != codeguard.FixTemplateKindDeterministic { + t.Fatalf("expected deterministic reliability fix template, got %q", rule.FixTemplate.Kind) + } +} + +func TestSDKRuleMetadataForDataRule(t *testing.T) { + rule := requireRuleMetadata(t, "data.missing-outbox-strategy") + assertExecutionModel(t, rule, codeguard.RuleExecutionModelLanguageAgnostic) + assertLanguageCoverage( + t, + rule, + codeguard.RuleLanguageCoverageFixed, + codeguard.RuleLanguageCPP, + codeguard.RuleLanguageGo, + codeguard.RuleLanguageJavaScript, + codeguard.RuleLanguagePython, + codeguard.RuleLanguageTypeScript, + ) + if rule.FixTemplate.Kind != codeguard.FixTemplateKindGuided { + t.Fatalf("expected guided data fix template, got %q", rule.FixTemplate.Kind) + } +} + +func TestSDKRuleMetadataForNonExpandContractMigration(t *testing.T) { + rule := requireRuleMetadata(t, "contracts.non-expand-contract-migration") + assertExecutionModel(t, rule, codeguard.RuleExecutionModelLanguageAgnostic) + assertLanguageCoverage(t, rule, codeguard.RuleLanguageCoverageRepositoryWide) + if rule.FixTemplate.Kind != codeguard.FixTemplateKindGuided { + t.Fatalf("expected guided migration fix template, got %q", rule.FixTemplate.Kind) + } +} + func TestSDKRuleMetadataFixTemplateIncludesBeforeAfterSnippet(t *testing.T) { rule := requireRuleMetadata(t, "quality.gofmt") if !strings.Contains(rule.FixTemplate.Text, "Before:") || !strings.Contains(rule.FixTemplate.Text, "After:") {