From bd14216d90b3e91945635969a879c2f061b5508c Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Mon, 27 Jul 2026 10:22:57 -0400 Subject: [PATCH 1/7] docs(backlog): plan production reliability readiness --- ...e-production-reliability-data-readiness.md | 226 ++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 .claude/task-boards/feature-production-reliability-data-readiness.md diff --git a/.claude/task-boards/feature-production-reliability-data-readiness.md b/.claude/task-boards/feature-production-reliability-data-readiness.md new file mode 100644 index 0000000..67baf5c --- /dev/null +++ b/.claude/task-boards/feature-production-reliability-data-readiness.md @@ -0,0 +1,226 @@ +# Task board: feature/production-reliability-data-readiness + +Status: staging +Branch: feature/production-reliability-data-readiness +Last updated: 2026-07-27 +Not final product docs: this is implementation planning for the branch, not shipped user-facing documentation. + +## Goal + +Make CodeGuard detect production-readiness failures that commonly cause outages or data loss: + +- reliability failures in outbound calls, retry loops, cancellation, concurrency, cleanup, shutdown, and partial-failure handling; +- distributed-system and data-correctness failures around transactions, idempotency, dual writes, pagination, unbounded reads, migrations, caches, and delivery semantics; +- a first production-risk rollup that turns these findings into PR-level risk evidence. + +This branch should move CodeGuard beyond general code quality and into “will this change behave safely in production?” + +## Non-goals + +- Do not implement broad local design smells, naming checks, or reviewability metrics here. Those belong to `feature/change-safety-testability-refactors`. +- Do not implement observability, ownership, runbook, or rollout-governance checks here except where needed as production-risk inputs. Those belong to `feature/operability-design-delivery-governance`. +- Do not rename existing rule IDs. Waivers and baselines depend on stable IDs. +- Do not make new rules blocking in every profile without a staged rollout path. + +## Product split + +This branch owns: + +- Rule families: `reliability.*`, `data.*`, and `contracts.non-expand-contract-migration`. +- New sections/config: `reliability`, `data`, and production-risk artifact/config. +- Product metric: `production_risk` as part of a new `pr_summary` artifact. + +Adjacent branch contracts: + +- `feature/change-safety-testability-refactors` will add `change_safety`, `maintainability_delta`, and `refactor_confidence` to the same `pr_summary` artifact. +- `feature/operability-design-delivery-governance` will add observability/delivery signals that can feed `production_risk` after the artifact shape exists. + +## Existing repo seams to reuse + +- Rule catalog merge point: `internal/codeguard/rules/catalog.go`. +- Rule metadata schema: `internal/codeguard/core/rule_metadata_types.go`. +- Fix-template requirement: `internal/codeguard/rules/catalog_fix_templates*.go`; every built-in rule needs fix guidance. +- Config surface: `internal/codeguard/core/config_types.go`, `internal/codeguard/core/config_rule_types.go`. +- Defaults/examples/validation: `internal/codeguard/config/defaults.go`, `internal/codeguard/config/defaults_rules.go`, `internal/codeguard/config/example.go`, `internal/codeguard/config/validate.go`. +- Profile behavior: `internal/codeguard/config/profile.go`. +- Check family runner pattern: `internal/codeguard/checks/supplychain/supplychain.go`. +- Runner section registration: `internal/codeguard/runner/checks/registry.go`. +- Finding construction/finalization: `internal/codeguard/runner/support/findings.go`, `internal/codeguard/runner/support/findings_section.go`. +- Diff-aware inputs: `internal/codeguard/runner/support/diff_scope.go`, `internal/codeguard/runner/support/changed_files.go`, `internal/codeguard/core/diff_types.go`. +- Existing risk artifacts: `internal/codeguard/runner/risk_scoring.go`, `internal/codeguard/core/report_artifact_types.go`. + +## Rule inventory + +### Reliability + +Initial rule IDs: + +- `reliability.missing-timeout` +- `reliability.unbounded-retry` +- `reliability.retry-without-backoff` +- `reliability.non-idempotent-retry` +- `reliability.missing-cancellation` +- `reliability.unbounded-work` +- `reliability.missing-concurrency-limit` +- `reliability.resource-leak` +- `reliability.partial-failure-hidden` +- `reliability.missing-graceful-shutdown` +- `reliability.swallowed-error` +- `reliability.lost-error-context` +- `reliability.recoverable-panic` + +### Data correctness + +Initial rule IDs: + +- `data.read-modify-write-race` +- `data.missing-transaction-boundary` +- `data.side-effect-in-transaction` +- `data.non-idempotent-consumer` +- `data.missing-deduplication` +- `data.unsafe-dual-write` +- `data.missing-outbox-strategy` +- `data.unstable-pagination` +- `data.unbounded-read` +- `data.exactly-once-assumption` +- `data.cache-without-policy` +- `contracts.non-expand-contract-migration` + +## Implementation phases + +### Phase 0: Design the rollout contract + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Decide section IDs and display names | `internal/codeguard/runner/checks/registry.go` | `go test ./tests/checks ./tests/cli` | Prefer stable snake_case final IDs: `reliability`, `data`. Avoid the existing supply-chain hyphen/underscore mismatch. | +| Todo | Decide default enablement | `internal/codeguard/config/defaults.go`, `internal/codeguard/config/profile.go` | `go test ./internal/codeguard/config ./tests/codeguard` | Suggested: startup warns only for severe reliability; strict enables reliability; enterprise enables reliability + data; ai-safe enables reliability + data diff signals. | +| Todo | Define severity policy | rule catalogs + profile docs | metadata tests | Suggested: block only high-confidence outage/data-loss patterns; warn confidence-based heuristics. | +| Todo | Define language priority | check packages | targeted check tests | Start Go first, then TypeScript/JavaScript, then Python. C++/Rust/Java can begin as catalog/config placeholders only when detectors are not ready. | + +### Phase 1: Add family scaffolding + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Add `ReliabilityRulesConfig` and `DataRulesConfig` | `internal/codeguard/core/config_rule_types.go` | config tests | Use `*bool` per rule toggle so omitted values can get defaults. Add thresholds for max retry count, max queue/buffer size, unbounded-read row limit, and trusted boundary patterns. | +| Todo | Add top-level toggles | `internal/codeguard/core/config_types.go` | config IO tests | `Reliability *bool` if omitted should support profile defaults; `Data *bool` if data rules should start opt-in outside enterprise. | +| Todo | Add defaults/examples | `internal/codeguard/config/defaults.go`, `defaults_rules.go`, `example.go`, `example_rules.go` | `go test ./internal/codeguard/config ./tests/codeguard` | Mirror supply-chain/performance patterns. | +| Todo | Add validation | `internal/codeguard/config/validate_reliability.go`, `validate_data.go`, `validate.go` | config validation tests | Validate thresholds are positive, pattern entries are non-empty, and rule dependencies are coherent. | +| Todo | Add SDK aliases | `pkg/codeguard/sdk_types_config_checks.go` | `go test ./pkg/codeguard` | Keep public SDK config usable. | +| Todo | Add catalogs | `internal/codeguard/rules/catalog_reliability.go`, `catalog_data.go`, `catalog.go` | `go test ./tests/cli` | Explicit `LanguageCoverage` for all non-language-prefixed IDs. | +| Todo | Add fix templates | `internal/codeguard/rules/catalog_fix_templates_reliability.go`, `catalog_fix_templates_data.go` | metadata tests | Use guided templates for concurrency/data risks; deterministic templates only for mechanical timeout/context cases. | + +### Phase 2: Implement reliability detectors + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Create check package | `internal/codeguard/checks/reliability/reliability.go` | `tests/checks/reliability_test.go` | Follow `supplychain.Run` shape and use `env.FinalizeSection("reliability", "Reliability", findings)`. | +| Todo | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Place after performance and before design/security so production-readiness issues appear early. | +| Todo | Detect Go outbound calls without timeout/context | `internal/codeguard/checks/reliability/*go*.go` | `TestReliabilityGoMissingTimeout` | Flag `http.Get`, `http.Post`, `http.DefaultClient.Do`, `exec.Command`, raw network calls, and DB calls lacking context where applicable. Avoid false positives for explicit `http.Client{Timeout: ...}` and context-bound requests. | +| Todo | Detect retry loops without limits/backoff/jitter | Go detector files | `TestReliabilityGoRetryPolicy` | Identify loops around calls with `retry`, `attempt`, transient errors, or status-code checks. Evidence should include limit/backoff/jitter absence separately. | +| Todo | Detect non-idempotent retries | Go detector files | `TestReliabilityGoNonIdempotentRetry` | Flag retried `POST`, writes, DB mutations, event publishes, or side-effect calls unless idempotency key/dedup marker is present. Confidence-based. | +| Todo | Detect missing cancellation propagation | Go detector files | `TestReliabilityGoMissingCancellation` | Flag background goroutines or downstream calls using `context.Background()`/`TODO()` inside request/job flows. | +| Todo | Detect unbounded work/concurrency | Go detector files | `TestReliabilityGoUnboundedWork` | Flag unbounded goroutine spawn in loops, unbounded channel buffers, unbounded worker queues, and `errgroup` without limits where supported. | +| Todo | Detect resource leaks | Go detector files | `TestReliabilityGoResourceLeak` | Track opened files, response bodies, rows, tickers, and timers. Reuse existing parser/support helpers if available. | +| Todo | Detect missing graceful shutdown | Go detector files | `TestReliabilityGoMissingGracefulShutdown` | Flag servers/workers started without signal handling, shutdown context, drain/close path, or wait group. Keep confidence low/medium unless evidence is strong. | +| Todo | Detect swallowed/lost errors and recoverable panic | Go detector files | `TestReliabilityGoErrorHandling` | Coordinate with existing `quality.ai.*` error signals to avoid duplicate rule spam. Prefer reliability IDs for production failure semantics. | + +### Phase 3: Implement data-correctness detectors + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Create check package | `internal/codeguard/checks/data/data.go` | `tests/checks/data_test.go` | Use a dedicated `Data Correctness` section. | +| Todo | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Run after reliability; many data findings will be repo/path-level and diff-filtered by line when possible. | +| Todo | Detect read-modify-write race | Go detector files | `TestDataGoReadModifyWriteRace` | Look for select/read followed by update/write outside transaction or conditional update. Evidence: same key/entity, mutation after read, no transaction/lock/compare-and-swap. | +| Todo | Detect missing transaction boundary | Go detector files | `TestDataGoMissingTransactionBoundary` | Flag multiple related DB writes without transaction wrapper. Keep configurable DB API patterns. | +| Todo | Detect external side effects inside retried transaction | Go detector files | `TestDataGoSideEffectInTransaction` | Flag HTTP/email/event calls inside transaction/retry closures. High production risk. | +| Todo | Detect consumer idempotency gaps | Go/TS/Python detectors | `TestDataConsumerIdempotency` | Flag message handlers without dedup/idempotency key checks around side effects. Start with naming/framework heuristics and confidence evidence. | +| Todo | Detect unsafe dual writes and missing outbox | Go detector files | `TestDataGoOutbox` | Flag DB write plus event publish without outbox, transactional event table, or equivalent configured strategy. | +| Todo | Detect unstable pagination | Go/TS/Python detectors | `TestDataUnstablePagination` | Flag limit/offset without deterministic order or cursor stability. | +| Todo | Detect unbounded DB reads | Go/TS/Python detectors | `TestDataUnboundedRead` | Flag `Find/Select/Query` without limit, streaming, pagination, or bounded filters. | +| Todo | Detect unsafe schema migrations | migration file scanner | `TestDataUnsafeMigration` | Coordinate with `contracts.non-expand-contract-migration`; detect destructive/contracting migrations without expand/contract staging metadata. | +| Todo | Detect exactly-once assumptions | text/code scanner | `TestDataExactlyOnceAssumption` | Flag comments/config/code that assert exactly-once without idempotency/dedup. Low/medium confidence unless tied to consumer code. | +| Todo | Detect cache without policy | Go/TS/Python detectors | `TestDataCacheWithoutPolicy` | Require TTL/invalidation/ownership policy for production caches. Allow configured cache wrappers. | + +### Phase 4: Add production-risk artifact + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Add artifact schema | `internal/codeguard/core/report_artifact_types.go`, maybe new `report_artifact_pr_summary_types.go` | serialization tests | Add `ReportArtifactKindPRSummary = "pr_summary"` and `PRSummaryArtifact` with `production_risk`. Keep fields additive and `omitempty`. | +| Todo | Add artifact helper | `internal/codeguard/checks/support/artifacts.go` or new support file | artifact tests | Follow `NewSlopScoreArtifact`/`NewChangeRiskArtifact`; defensively copy evidence slices. | +| Todo | Add runner postprocessor | `internal/codeguard/runner/pr_summary.go` | `internal/codeguard/runner/*test.go` | Publish once with `sc.Artifacts.Put(...)`, sorted evidence, deterministic scoring. | +| Todo | Wire production risk inputs | `internal/codeguard/runner/pr_summary.go` | risk tests | Inputs: reliability/data fail/warn findings, non-idempotent retry, missing transaction/outbox, resource leak, unbounded work/read, suppressed findings excluded by current pipeline. | +| Todo | Preserve outputs | `internal/codeguard/report/write.go`, `github_comment.go` if rendered | report tests | JSON should include artifact automatically. Do not emit PR metrics as GitHub annotations. Do not mutate the existing text `Summary:` line. | +| Todo | SDK aliases | `pkg/codeguard/sdk_types_runtime_report.go` | SDK tests | Export new runtime types if public consumers need them. | + +### Phase 5: Documentation and examples + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Update product docs when behavior exists | `docs/checks.md`, `docs/features.md`, `docs/production.md`, `README.md` | docs checks/self-scan | Do not advertise catalog-only rules as fully implemented. Mark staged/confidence-based behavior clearly. | +| Todo | Update examples | `examples/codeguard.json`, `.codeguard/codeguard.yaml` if appropriate | `make codeguard-ci` | Consider keeping new families opt-in until false-positive rate is measured. | +| Todo | Add migration notes | `docs/production.md` or release notes | n/a | Explain profile behavior and how to tune/waive noisy reliability/data checks. | + +## Detector confidence policy + +- High confidence: syntactic evidence directly proves missing timeout, ignored cleanup error, unbounded goroutine in loop, multiple DB writes without transaction, or DB write plus event publish without outbox. +- Medium confidence: naming/API heuristics imply retry, idempotency, consumer, cache, transaction, or side effect but repository-specific wrapper may exist. +- Low confidence: comment/text/history-derived signals such as exactly-once assumptions or cascading synchronous dependency risk. + +Findings should include evidence metadata that is safe for reports: operation kind, call kind, retry loop evidence, transaction wrapper evidence, idempotency evidence, and configured framework match. Do not include source snippets or secrets in metadata. + +## Profile behavior target + +| Profile | Reliability | Data correctness | Production risk | +| --- | --- | --- | --- | +| Startup | Warn severe/high-confidence reliability only | Off by default | Warn only | +| Strict | Block new high-confidence reliability regressions; warn medium confidence | Warn high-confidence data risks in diff mode | Warn elevated risk | +| Enterprise | Block severe reliability and data-loss risks | Block unsafe dual writes, missing transaction boundaries, unsafe migrations | Warn/block by threshold | +| AI-safe | Strict plus weak error handling, oversized risk, and missing tests from sibling branch | Strict data diff signals | Warn/block by threshold | + +## Acceptance criteria + +- New family config validates and round-trips in JSON/YAML. +- `codeguard rules` exposes reliability/data metadata with language coverage and fix templates. +- Enabling the new sections runs without panics on empty repos and on this repo. +- Go reliability detectors cover at least missing timeout, unbounded retry, missing cancellation, unbounded work, resource leak, swallowed/lost errors, and recoverable panic. +- Go data detectors cover at least read-modify-write race, missing transaction, side-effect-in-transaction, unsafe dual write/missing outbox, unstable pagination, unbounded read, and cache-without-policy. +- The `pr_summary` artifact includes deterministic `production_risk` score/evidence in diff scans. +- Existing JSON/SARIF/GitHub annotation/text summary compatibility is preserved. +- Targeted tests and `make test` pass before push/PR. + +## Verification plan + +Targeted during implementation: + +```sh +go test ./internal/codeguard/config ./internal/codeguard/rules ./internal/codeguard/runner +go test ./tests/codeguard ./tests/checks ./tests/cli -run 'Test.*(Reliability|Data|ProductionRisk|Rules|Profiles|Metadata)' +go test ./tests/security ./tests/checks -run 'TestWriteReport|TestSARIF|TestGitHub' +``` + +Branch gate: + +```sh +make fmt-check +make test +make codeguard-ci +``` + +Pre-push/PR gate when practical: + +```sh +make ci +``` + +## Merge checklist + +- [ ] Rule IDs are stable and documented. +- [ ] Every built-in rule has a fix template. +- [ ] New config fields have defaults, validation, examples, and SDK aliases. +- [ ] New sections use stable section IDs and deterministic output. +- [ ] Findings are diff-filtered correctly where line-level evidence exists. +- [ ] Production-risk scoring has deterministic evidence ordering. +- [ ] SARIF/GitHub annotations remain finding-only. +- [ ] Product docs describe implemented behavior, not planned behavior. +- [ ] `make test` passes. +- [ ] `make ci` passes or any skipped gate is explicitly documented. From d58e8eb61e418bf8f2b1f00f06a05fcb7f18f464 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Mon, 27 Jul 2026 10:59:36 -0400 Subject: [PATCH 2/7] feat: add production readiness rule families --- ...e-production-reliability-data-readiness.md | 18 +- internal/codeguard/checks/data/data.go | 28 ++ internal/codeguard/checks/data/data_go.go | 267 +++++++++++++++++ .../checks/reliability/reliability.go | 30 ++ .../checks/reliability/reliability_go.go | 156 ++++++++++ .../reliability/reliability_go_helpers.go | 274 ++++++++++++++++++ .../codeguard/checks/support/artifacts.go | 22 ++ internal/codeguard/config/defaults.go | 9 + internal/codeguard/config/defaults_rules.go | 48 +++ internal/codeguard/config/example.go | 55 ++++ internal/codeguard/config/profile.go | 5 + internal/codeguard/config/validate.go | 3 + .../config/validate_reliability_data.go | 53 ++++ internal/codeguard/core/config_rule_types.go | 51 ++++ internal/codeguard/core/config_types.go | 11 + .../codeguard/core/report_artifact_types.go | 20 ++ internal/codeguard/rules/catalog.go | 2 + internal/codeguard/rules/catalog_contracts.go | 10 + internal/codeguard/rules/catalog_data.go | 35 +++ .../codeguard/rules/catalog_fix_templates.go | 2 + .../rules/catalog_fix_templates_data.go | 18 ++ .../catalog_fix_templates_reliability.go | 19 ++ .../codeguard/rules/catalog_reliability.go | 37 +++ internal/codeguard/runner/checks/registry.go | 22 ++ internal/codeguard/runner/pr_summary.go | 119 ++++++++ internal/codeguard/runner/pr_summary_test.go | 77 +++++ internal/codeguard/runner/runner.go | 1 + pkg/codeguard/sdk_types_config_checks.go | 3 + pkg/codeguard/sdk_types_runtime_report.go | 3 + tests/checks/data_test.go | 101 +++++++ tests/checks/reliability_test.go | 100 +++++++ tests/cli/features_metadata_test.go | 34 +++ 32 files changed, 1632 insertions(+), 1 deletion(-) create mode 100644 internal/codeguard/checks/data/data.go create mode 100644 internal/codeguard/checks/data/data_go.go create mode 100644 internal/codeguard/checks/reliability/reliability.go create mode 100644 internal/codeguard/checks/reliability/reliability_go.go create mode 100644 internal/codeguard/checks/reliability/reliability_go_helpers.go create mode 100644 internal/codeguard/config/validate_reliability_data.go create mode 100644 internal/codeguard/rules/catalog_data.go create mode 100644 internal/codeguard/rules/catalog_fix_templates_data.go create mode 100644 internal/codeguard/rules/catalog_fix_templates_reliability.go create mode 100644 internal/codeguard/rules/catalog_reliability.go create mode 100644 internal/codeguard/runner/pr_summary.go create mode 100644 internal/codeguard/runner/pr_summary_test.go create mode 100644 tests/checks/data_test.go create mode 100644 tests/checks/reliability_test.go diff --git a/.claude/task-boards/feature-production-reliability-data-readiness.md b/.claude/task-boards/feature-production-reliability-data-readiness.md index 67baf5c..413a33d 100644 --- a/.claude/task-boards/feature-production-reliability-data-readiness.md +++ b/.claude/task-boards/feature-production-reliability-data-readiness.md @@ -1,10 +1,26 @@ # Task board: feature/production-reliability-data-readiness -Status: staging +Status: active Branch: feature/production-reliability-data-readiness Last updated: 2026-07-27 Not final product docs: this is implementation planning for the branch, not shipped user-facing documentation. +## Progress update: first implementation slice + +Completed in the first implementation pass: + +- Added Reliability and Data Correctness config surfaces, defaults, validation, profile enablement, SDK aliases, rule catalogs, and fix templates. +- Added `Reliability` and `Data Correctness` runner sections. +- Added Go reliability detectors for missing HTTP timeouts, missing cancellation propagation, unbounded goroutine work, retry policy gaps, HTTP response body leaks, swallowed errors, lost error context, recoverable panic, and missing graceful shutdown evidence. +- Added Go data-correctness detectors for read-modify-write/multi-write transaction gaps, side effects inside transaction callbacks, unsafe dual writes, missing outbox evidence, consumer idempotency/dedupe gaps, unstable pagination, unbounded SQL reads, exactly-once assumptions, and cache policy gaps. +- Added additive `pr_summary.production_risk` report artifact and SDK aliases. The artifact is diff-only and does not change SARIF/GitHub annotation/text summary compatibility. +- Added focused tests in `tests/checks/reliability_test.go`, `tests/checks/data_test.go`, `internal/codeguard/runner/pr_summary_test.go`, and representative metadata tests. + +Verification completed: + +- `go test ./...` with localhost test escalation. +- `make codeguard-ci`. + ## Goal Make CodeGuard detect production-readiness failures that commonly cause outages or data loss: diff --git a/internal/codeguard/checks/data/data.go b/internal/codeguard/checks/data/data.go new file mode 100644 index 0000000..8d274f7 --- /dev/null +++ b/internal/codeguard/checks/data/data.go @@ -0,0 +1,28 @@ +// 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) + }) + default: + return nil + } +} + +func enabled(toggle *bool) bool { + return toggle == nil || *toggle +} 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/reliability/reliability.go b/internal/codeguard/checks/reliability/reliability.go new file mode 100644 index 0000000..3c659c0 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability.go @@ -0,0 +1,30 @@ +// 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) + }) + default: + return nil + } +} + +func enabled(toggle *bool) bool { + return toggle == nil || *toggle +} diff --git a/internal/codeguard/checks/reliability/reliability_go.go b/internal/codeguard/checks/reliability/reliability_go.go new file mode 100644 index 0000000..52d1feb --- /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 { + switch n := node.(type) { + case *ast.FuncDecl: + 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) + 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, 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{}, hasContextParam bool, hasShutdown bool) []core.Finding { + findings := make([]core.Finding, 0, 2) + pos := fset.Position(call.Pos()) + if enabled(rules.DetectMissingTimeout) && isUnboundedHTTPCall(call, httpAliases) { + 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..2decb38 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_go_helpers.go @@ -0,0 +1,274 @@ +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{}) 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 + } + } + } + return selector.Sel.Name == "Do" +} + +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 || !isUnboundedHTTPCall(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 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/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 f538313..3a796a6 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) } 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 5a5d21a..7d22dda 100644 --- a/internal/codeguard/config/profile.go +++ b/internal/codeguard/config/profile.go @@ -42,6 +42,7 @@ var profileCatalog = map[string]profileSpec{ cfg.Checks.DesignRules.MaxInterfaceMethods = 4 cfg.Checks.SecurityRules.GovulncheckMode = "required" cfg.Checks.Contracts = boolPtr(true) + cfg.Checks.Reliability = boolPtr(true) }, }, "enterprise": { @@ -59,6 +60,8 @@ var profileCatalog = map[string]profileSpec{ cfg.Checks.CIRules.RequiredReleaseFiles = []string{".goreleaser.yaml"} cfg.Checks.CIRules.RequiredAutomationPaths = []string{"Makefile", ".github/workflows/ci.yml"} cfg.Checks.Contracts = boolPtr(true) + cfg.Checks.Reliability = boolPtr(true) + cfg.Checks.Data = boolPtr(true) }, }, "ai-safe": { @@ -75,6 +78,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) }, }, } diff --git a/internal/codeguard/config/validate.go b/internal/codeguard/config/validate.go index ce1e191..f0e490e 100644 --- a/internal/codeguard/config/validate.go +++ b/internal/codeguard/config/validate.go @@ -23,6 +23,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_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 440c37a..96e8d71 100644 --- a/internal/codeguard/core/config_types.go +++ b/internal/codeguard/core/config_types.go @@ -75,6 +75,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. @@ -94,8 +102,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..96d20e4 --- /dev/null +++ b/internal/codeguard/rules/catalog_data.go @@ -0,0 +1,35 @@ +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, + ), + 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..d3f2232 --- /dev/null +++ b/internal/codeguard/rules/catalog_reliability.go @@ -0,0 +1,37 @@ +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, + ), + 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/data_test.go b/tests/checks/data_test.go new file mode 100644 index 0000000..092764f --- /dev/null +++ b/tests/checks/data_test.go @@ -0,0 +1,101 @@ +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") +} diff --git a/tests/checks/reliability_test.go b/tests/checks/reliability_test.go new file mode 100644 index 0000000..323d11a --- /dev/null +++ b/tests/checks/reliability_test.go @@ -0,0 +1,100 @@ +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") +} diff --git a/tests/cli/features_metadata_test.go b/tests/cli/features_metadata_test.go index 78ae234..672cef7 100644 --- a/tests/cli/features_metadata_test.go +++ b/tests/cli/features_metadata_test.go @@ -53,6 +53,40 @@ 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.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.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 TestSDKRuleMetadataFixTemplateIncludesBeforeAfterSnippet(t *testing.T) { rule := requireRuleMetadata(t, "quality.gofmt") if !strings.Contains(rule.FixTemplate.Text, "Before:") || !strings.Contains(rule.FixTemplate.Text, "After:") { From 588bf19e73579dc566a3510baecb679c69157ce3 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Mon, 27 Jul 2026 11:14:27 -0400 Subject: [PATCH 3/7] feat: expand production readiness language coverage --- ...e-production-reliability-data-readiness.md | 19 +++ internal/codeguard/checks/data/data.go | 10 ++ internal/codeguard/checks/data/data_cpp.go | 107 ++++++++++++ internal/codeguard/checks/data/data_python.go | 111 +++++++++++++ .../codeguard/checks/data/data_typescript.go | 116 +++++++++++++ .../checks/reliability/reliability.go | 10 ++ .../checks/reliability/reliability_cpp.go | 101 ++++++++++++ .../checks/reliability/reliability_python.go | 154 ++++++++++++++++++ .../reliability/reliability_typescript.go | 99 +++++++++++ internal/codeguard/rules/catalog_data.go | 1 + .../codeguard/rules/catalog_reliability.go | 1 + tests/checks/data_multilang_test.go | 113 +++++++++++++ tests/checks/reliability_multilang_test.go | 123 ++++++++++++++ tests/cli/features_metadata_test.go | 2 + 14 files changed, 967 insertions(+) create mode 100644 internal/codeguard/checks/data/data_cpp.go create mode 100644 internal/codeguard/checks/data/data_python.go create mode 100644 internal/codeguard/checks/data/data_typescript.go create mode 100644 internal/codeguard/checks/reliability/reliability_cpp.go create mode 100644 internal/codeguard/checks/reliability/reliability_python.go create mode 100644 internal/codeguard/checks/reliability/reliability_typescript.go create mode 100644 tests/checks/data_multilang_test.go create mode 100644 tests/checks/reliability_multilang_test.go diff --git a/.claude/task-boards/feature-production-reliability-data-readiness.md b/.claude/task-boards/feature-production-reliability-data-readiness.md index 413a33d..c397670 100644 --- a/.claude/task-boards/feature-production-reliability-data-readiness.md +++ b/.claude/task-boards/feature-production-reliability-data-readiness.md @@ -21,6 +21,25 @@ Verification completed: - `go test ./...` with localhost test escalation. - `make codeguard-ci`. +## Progress update: multi-language production-readiness slice + +Completed in the second implementation pass: + +- Expanded Reliability and Data Correctness rule language coverage to include C++ in addition to Go, Python, TypeScript, and JavaScript. +- Added Python reliability detectors for outbound HTTP calls without timeouts, retry/backoff gaps, non-idempotent retry evidence, unbounded asyncio work, swallowed exceptions, generic recoverable raises, and nearby resource-leak evidence. +- Added TypeScript/JavaScript reliability detectors for HTTP calls without timeout/abort evidence, promise/HTTP work in loops without concurrency limits, retry/backoff gaps, non-idempotent retry evidence, swallowed catch blocks, and generic recoverable throws. +- Added C++ reliability detectors for retry/backoff gaps, non-idempotent retry evidence, thread/task launches in loops without concurrency bounds, generic runtime throws, and raw allocation without nearby ownership cleanup. +- Added Python data-correctness detectors for unbounded reads, unstable pagination, multi-write transaction gaps, write+publish/outbox gaps, consumer idempotency/dedupe gaps, exactly-once assumptions, and cache writes without TTL evidence. +- Added TypeScript/JavaScript data-correctness detectors for unbounded reads, unstable pagination, multi-write transaction gaps, write+publish/outbox gaps, consumer idempotency/dedupe gaps, exactly-once assumptions, and cache writes without TTL evidence. +- Added C++ data-correctness detectors for unbounded reads, unstable pagination, multi-write transaction gaps, write+publish/outbox gaps, consumer idempotency/dedupe gaps, exactly-once assumptions, and cache writes without TTL evidence. +- Added focused multi-language tests for Python, TypeScript, JavaScript, and C++ reliability/data behavior. + +Verification completed: + +- `go test ./internal/codeguard/config ./internal/codeguard/rules ./internal/codeguard/runner ./internal/codeguard/runner/checks ./internal/codeguard/checks/reliability ./internal/codeguard/checks/data ./pkg/codeguard ./tests/checks ./tests/cli`. +- `go test ./...` with localhost test escalation. +- `make codeguard-ci`. + ## Goal Make CodeGuard detect production-readiness failures that commonly cause outages or data loss: diff --git a/internal/codeguard/checks/data/data.go b/internal/codeguard/checks/data/data.go index 8d274f7..6b64763 100644 --- a/internal/codeguard/checks/data/data.go +++ b/internal/codeguard/checks/data/data.go @@ -18,6 +18,16 @@ func dataTargetFindings(_ context.Context, env support.Context, target core.Targ 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 } diff --git a/internal/codeguard/checks/data/data_cpp.go b/internal/codeguard/checks/data/data_cpp.go new file mode 100644 index 0000000..e058b6d --- /dev/null +++ b/internal/codeguard/checks/data/data_cpp.go @@ -0,0 +1,107 @@ +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.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") + } + if enabled(s.rules.DetectExactlyOnceAssumption) && strings.Contains(strings.ToLower(line), "exactly once") && !cppDataDedupe.MatchString(line) { + s.add("data.exactly-once-assumption", "warn", lineNo, "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_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..23c750c --- /dev/null +++ b/internal/codeguard/checks/data/data_typescript.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 ( + 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.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") + } + if enabled(s.rules.DetectExactlyOnceAssumption) && tsExactlyOnce.MatchString(line) && !tsDataDedupe.MatchString(line) { + s.add("data.exactly-once-assumption", "warn", lineNo, "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 index 3c659c0..3493113 100644 --- a/internal/codeguard/checks/reliability/reliability.go +++ b/internal/codeguard/checks/reliability/reliability.go @@ -20,6 +20,16 @@ func reliabilityTargetFindings(_ context.Context, env support.Context, target co 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 } diff --git a/internal/codeguard/checks/reliability/reliability_cpp.go b/internal/codeguard/checks/reliability/reliability_cpp.go new file mode 100644 index 0000000..b6c65a5 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_cpp.go @@ -0,0 +1,101 @@ +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`) + 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 + newLine int + findings []core.Finding +} + +func (s *cppReliabilityScan) consumeLine(lineNo int, line string) { + startsLoop := cppLoopStart.MatchString(line) + inLoop := len(s.loops) > 0 || startsLoop + s.checkLine(lineNo, line, inLoop) + next := s.depth + strings.Count(line, "{") - strings.Count(line, "}") + if startsLoop && next > s.depth { + s.loops = append(s.loops, s.depth) + } + for len(s.loops) > 0 && next <= s.loops[len(s.loops)-1] { + s.loops = s.loops[:len(s.loops)-1] + } + s.depth = next +} + +func (s *cppReliabilityScan) checkLine(lineNo int, line string, inLoop 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.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_python.go b/internal/codeguard/checks/reliability/reliability_python.go new file mode 100644 index 0000000..43c76f5 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_python.go @@ -0,0 +1,154 @@ +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*\(|\brequests\.(?:get|post|put|patch|delete|head)\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 []int + 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 = popIndentedRegions(s.loops, indent) + s.excepts = popIndentedRegions(s.excepts, indent) + inLoop := len(s.loops) > 0 + inExcept := len(s.excepts) > 0 + if pyRetryLoop.MatchString(line) { + s.loops = append(s.loops, indent) + inLoop = true + } + if strings.HasPrefix(trimmed, "except ") || strings.HasPrefix(trimmed, "except:") { + s.excepts = append(s.excepts, indent) + inExcept = true + } + s.checkLine(lineNo, line, trimmed, inLoop, inExcept) +} + +func (s *pythonReliabilityScan) checkLine(lineNo int, line string, trimmed string, inLoop 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) && strings.HasPrefix(trimmed, "while True") && 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) + } +} + +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 +} diff --git a/internal/codeguard/checks/reliability/reliability_typescript.go b/internal/codeguard/checks/reliability/reliability_typescript.go new file mode 100644 index 0000000..f769245 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_typescript.go @@ -0,0 +1,99 @@ +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*\(`) + 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 + findings []core.Finding +} + +func (s *tsReliabilityScan) consumeLine(lineNo int, line string) { + startsLoop := tsLoopStart.MatchString(line) + inLoop := len(s.loops) > 0 || startsLoop + s.checkLine(lineNo, line, inLoop) + next := s.depth + strings.Count(line, "{") - strings.Count(line, "}") + if startsLoop && next > s.depth { + s.loops = append(s.loops, s.depth) + } + for len(s.loops) > 0 && next <= s.loops[len(s.loops)-1] { + s.loops = s.loops[:len(s.loops)-1] + } + s.depth = next +} + +func (s *tsReliabilityScan) checkLine(lineNo int, line string, inLoop 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.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/rules/catalog_data.go b/internal/codeguard/rules/catalog_data.go index 96d20e4..c839b56 100644 --- a/internal/codeguard/rules/catalog_data.go +++ b/internal/codeguard/rules/catalog_data.go @@ -27,6 +27,7 @@ func dataRule(id string, level string, title string, description string, howToFi core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguagePython, + core.RuleLanguageCPP, ), Title: title, Description: description, diff --git a/internal/codeguard/rules/catalog_reliability.go b/internal/codeguard/rules/catalog_reliability.go index d3f2232..f4c7f51 100644 --- a/internal/codeguard/rules/catalog_reliability.go +++ b/internal/codeguard/rules/catalog_reliability.go @@ -29,6 +29,7 @@ func reliabilityRule(id string, level string, title string, description string, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguagePython, + core.RuleLanguageCPP, ), Title: title, Description: description, diff --git a/tests/checks/data_multilang_test.go b/tests/checks/data_multilang_test.go new file mode 100644 index 0000000..0e5a9ed --- /dev/null +++ b/tests/checks/data_multilang_test.go @@ -0,0 +1,113 @@ +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.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.cache-without-policy") +} + +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.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.cache-without-policy") +} + +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 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.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.cache-without-policy") +} diff --git a/tests/checks/reliability_multilang_test.go b/tests/checks/reliability_multilang_test.go new file mode 100644 index 0000000..f16851c --- /dev/null +++ b/tests/checks/reliability_multilang_test.go @@ -0,0 +1,123 @@ +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 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 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 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") +} diff --git a/tests/cli/features_metadata_test.go b/tests/cli/features_metadata_test.go index 672cef7..f08aa81 100644 --- a/tests/cli/features_metadata_test.go +++ b/tests/cli/features_metadata_test.go @@ -60,6 +60,7 @@ func TestSDKRuleMetadataForReliabilityRule(t *testing.T) { t, rule, codeguard.RuleLanguageCoverageFixed, + codeguard.RuleLanguageCPP, codeguard.RuleLanguageGo, codeguard.RuleLanguageJavaScript, codeguard.RuleLanguagePython, @@ -77,6 +78,7 @@ func TestSDKRuleMetadataForDataRule(t *testing.T) { t, rule, codeguard.RuleLanguageCoverageFixed, + codeguard.RuleLanguageCPP, codeguard.RuleLanguageGo, codeguard.RuleLanguageJavaScript, codeguard.RuleLanguagePython, From b41b543030bf7a1f0c1e4bf49216c9f08dba6d46 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Mon, 27 Jul 2026 11:29:56 -0400 Subject: [PATCH 4/7] test: expand production readiness language coverage --- internal/codeguard/checks/data/data_cpp.go | 13 +- .../codeguard/checks/data/data_typescript.go | 13 +- .../checks/reliability/reliability_cpp.go | 32 ++- .../checks/reliability/reliability_go.go | 7 +- .../reliability/reliability_go_helpers.go | 131 ++++++++++- .../checks/reliability/reliability_python.go | 37 ++- .../reliability/reliability_typescript.go | 30 ++- tests/checks/data_multilang_test.go | 202 +++++++++++++++++ tests/checks/data_test.go | 128 +++++++++++ tests/checks/reliability_multilang_test.go | 211 ++++++++++++++++++ tests/checks/reliability_test.go | 64 ++++++ 11 files changed, 832 insertions(+), 36 deletions(-) diff --git a/internal/codeguard/checks/data/data_cpp.go b/internal/codeguard/checks/data/data_cpp.go index e058b6d..f6987f6 100644 --- a/internal/codeguard/checks/data/data_cpp.go +++ b/internal/codeguard/checks/data/data_cpp.go @@ -30,6 +30,7 @@ func cppFindingsForFile(env support.Context, file string, data []byte) []core.Fi for idx, line := range strings.Split(masked, "\n") { scan.consumeLine(idx+1, line) } + scan.consumeRawSource(source) scan.finish() return scan.findings } @@ -75,8 +76,16 @@ func (s *cppDataScan) consumeLine(lineNo int, line string) { 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") } - if enabled(s.rules.DetectExactlyOnceAssumption) && strings.Contains(strings.ToLower(line), "exactly once") && !cppDataDedupe.MatchString(line) { - s.add("data.exactly-once-assumption", "warn", lineNo, "C++ code assumes exactly-once delivery without idempotency evidence", "low", "comment", "exactly-once") +} + +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") + } } } diff --git a/internal/codeguard/checks/data/data_typescript.go b/internal/codeguard/checks/data/data_typescript.go index 23c750c..75d5e2f 100644 --- a/internal/codeguard/checks/data/data_typescript.go +++ b/internal/codeguard/checks/data/data_typescript.go @@ -39,6 +39,7 @@ func typeScriptFindingsForFile(env support.Context, file string, data []byte) [] for idx, line := range strings.Split(code, "\n") { scan.consumeLine(idx+1, line) } + scan.consumeRawSource(source) scan.finish() return scan.findings } @@ -84,8 +85,16 @@ func (s *tsDataScan) consumeLine(lineNo int, line string) { 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") } - if enabled(s.rules.DetectExactlyOnceAssumption) && tsExactlyOnce.MatchString(line) && !tsDataDedupe.MatchString(line) { - s.add("data.exactly-once-assumption", "warn", lineNo, "TypeScript/JavaScript code assumes exactly-once delivery without idempotency evidence", "low", "comment", "exactly-once") +} + +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") + } } } diff --git a/internal/codeguard/checks/reliability/reliability_cpp.go b/internal/codeguard/checks/reliability/reliability_cpp.go index b6c65a5..238c24a 100644 --- a/internal/codeguard/checks/reliability/reliability_cpp.go +++ b/internal/codeguard/checks/reliability/reliability_cpp.go @@ -10,6 +10,7 @@ import ( 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*\(`) @@ -37,34 +38,45 @@ func cppFindingsForFile(env support.Context, file string, data []byte) []core.Fi } type cppReliabilityScan struct { - env support.Context - file string - rules core.ReliabilityRulesConfig - limited bool - depth int - loops []int - newLine int - findings []core.Finding + 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 - s.checkLine(lineNo, line, inLoop) + 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) { +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") } diff --git a/internal/codeguard/checks/reliability/reliability_go.go b/internal/codeguard/checks/reliability/reliability_go.go index 52d1feb..75ac219 100644 --- a/internal/codeguard/checks/reliability/reliability_go.go +++ b/internal/codeguard/checks/reliability/reliability_go.go @@ -40,6 +40,7 @@ func functionReliabilityFindings(env support.Context, file string, fset *token.F 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 { @@ -58,7 +59,7 @@ func functionReliabilityFindings(env support.Context, file string, fset *token.F 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, hasContextParam, hasShutdown)...) + 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: @@ -77,10 +78,10 @@ func functionReliabilityFindings(env support.Context, file string, fset *token.F return findings } -func callReliabilityFindings(env support.Context, file string, fset *token.FileSet, call *ast.CallExpr, rules core.ReliabilityRulesConfig, httpAliases map[string]struct{}, hasContextParam bool, hasShutdown bool) []core.Finding { +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) { + 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) { diff --git a/internal/codeguard/checks/reliability/reliability_go_helpers.go b/internal/codeguard/checks/reliability/reliability_go_helpers.go index 2decb38..03392e1 100644 --- a/internal/codeguard/checks/reliability/reliability_go_helpers.go +++ b/internal/codeguard/checks/reliability/reliability_go_helpers.go @@ -34,7 +34,7 @@ func funcHasContextParam(fn *ast.FuncDecl) bool { return false } -func isUnboundedHTTPCall(call *ast.CallExpr, aliases map[string]struct{}) bool { +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 @@ -48,7 +48,22 @@ func isUnboundedHTTPCall(call *ast.CallExpr, aliases map[string]struct{}) bool { } } } - return selector.Sel.Name == "Do" + 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 { @@ -102,7 +117,7 @@ func assignedHTTPResponseVars(assign *ast.AssignStmt, aliases map[string]struct{ names := make([]string, 0, 1) for _, rhs := range assign.Rhs { call, ok := rhs.(*ast.CallExpr) - if !ok || !isUnboundedHTTPCall(call, aliases) { + if !ok || !isHTTPResponseAcquisition(call, aliases) { continue } for _, lhs := range assign.Lhs { @@ -115,6 +130,116 @@ func assignedHTTPResponseVars(assign *ast.AssignStmt, aliases map[string]struct{ 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 { diff --git a/internal/codeguard/checks/reliability/reliability_python.go b/internal/codeguard/checks/reliability/reliability_python.go index 43c76f5..8e13dfe 100644 --- a/internal/codeguard/checks/reliability/reliability_python.go +++ b/internal/codeguard/checks/reliability/reliability_python.go @@ -18,7 +18,7 @@ var ( 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*\(|\brequests\.(?:get|post|put|patch|delete|head)\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*\(`) ) @@ -42,7 +42,7 @@ type pythonReliabilityScan struct { file string rules core.ReliabilityRulesConfig limited bool - loops []int + loops []pythonLoopRegion excepts []int openLine int openLineClosed bool @@ -55,29 +55,31 @@ func (s *pythonReliabilityScan) consumeLine(lineNo int, line string) { return } indent := leadingIndentWidth(line) - s.loops = popIndentedRegions(s.loops, indent) + 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, indent) + 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, inExcept) + s.checkLine(lineNo, line, trimmed, inLoop, inUnboundedLoop, inExcept) } -func (s *pythonReliabilityScan) checkLine(lineNo int, line string, trimmed string, inLoop bool, inExcept bool) { +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) && strings.HasPrefix(trimmed, "while True") && looksRetryish(line) { + 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) { @@ -103,6 +105,11 @@ func (s *pythonReliabilityScan) checkLine(lineNo int, line string, trimmed strin } } +type pythonLoopRegion struct { + indent int + unbounded bool +} + func (s *pythonReliabilityScan) trackResourceLeak(lineNo int, line string) { if strings.Contains(line, "with ") { return @@ -152,3 +159,19 @@ func popIndentedRegions(regions []int, indent int) []int { } 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 index f769245..efe36e4 100644 --- a/internal/codeguard/checks/reliability/reliability_typescript.go +++ b/internal/codeguard/checks/reliability/reliability_typescript.go @@ -12,6 +12,7 @@ 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*\(`) @@ -53,30 +54,38 @@ func typeScriptFindingsForFile(env support.Context, file string, data []byte) [] } type tsReliabilityScan struct { - env support.Context - file string - rules core.ReliabilityRulesConfig - limited bool - depth int - loops []int - findings []core.Finding + 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 - s.checkLine(lineNo, line, inLoop) + 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) { +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") } @@ -86,6 +95,9 @@ func (s *tsReliabilityScan) checkLine(lineNo int, line string, inLoop bool) { 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") } diff --git a/tests/checks/data_multilang_test.go b/tests/checks/data_multilang_test.go index 0e5a9ed..fbbd6ad 100644 --- a/tests/checks/data_multilang_test.go +++ b/tests/checks/data_multilang_test.go @@ -34,12 +34,68 @@ def handle_message(email, event): } 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"), ` @@ -62,12 +118,68 @@ async function handleMessage(email, event) { } 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"), ` @@ -84,6 +196,39 @@ async function list(db) { 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"), ` @@ -106,8 +251,65 @@ void HandleMessage(Email& email, Event event) { } 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 index 092764f..2ebaf92 100644 --- a/tests/checks/data_test.go +++ b/tests/checks/data_test.go @@ -99,3 +99,131 @@ func HandleMessage(email Emailer, event any) error { 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 index f16851c..402e0df 100644 --- a/tests/checks/reliability_multilang_test.go +++ b/tests/checks/reliability_multilang_test.go @@ -44,6 +44,72 @@ def handle(): 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"), ` @@ -74,6 +140,65 @@ async function handle() { 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"), ` @@ -90,6 +215,32 @@ async function fetchUser(id) { 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"), ` @@ -121,3 +272,63 @@ void run(int n) { 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 index 323d11a..bbfb4d3 100644 --- a/tests/checks/reliability_test.go +++ b/tests/checks/reliability_test.go @@ -98,3 +98,67 @@ func Copy(dst io.Writer, src io.Reader) { 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") +} From af989c7f1207a873252551c256da12e11be47560 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Mon, 27 Jul 2026 11:41:25 -0400 Subject: [PATCH 5/7] docs: complete production readiness task board --- ...e-production-reliability-data-readiness.md | 125 ++++++++++-------- docs/checks.md | 92 ++++++++++++- docs/features.md | 11 +- docs/production.md | 14 +- examples/codeguard.json | 43 ++++++ .../codeguard/checks/contracts/migrations.go | 36 +++-- .../checks/reliability/reliability_go.go | 3 +- tests/checks/contracts_test.go | 23 ++-- tests/cli/features_metadata_test.go | 9 ++ 9 files changed, 277 insertions(+), 79 deletions(-) diff --git a/.claude/task-boards/feature-production-reliability-data-readiness.md b/.claude/task-boards/feature-production-reliability-data-readiness.md index c397670..61aef86 100644 --- a/.claude/task-boards/feature-production-reliability-data-readiness.md +++ b/.claude/task-boards/feature-production-reliability-data-readiness.md @@ -1,6 +1,6 @@ # Task board: feature/production-reliability-data-readiness -Status: active +Status: complete Branch: feature/production-reliability-data-readiness Last updated: 2026-07-27 Not final product docs: this is implementation planning for the branch, not shipped user-facing documentation. @@ -40,6 +40,25 @@ Verification completed: - `go test ./...` with localhost test escalation. - `make codeguard-ci`. +## Progress update: completion audit + +Completed in the final audit pass: + +- Verified the task-board inventory against implemented Reliability, Data Correctness, API Contracts, and `pr_summary.production_risk` code paths. +- Added targeted positive and negative tests for Go, Python, TypeScript, JavaScript, and C++ reliability/data detectors. +- Wired `contracts.non-expand-contract-migration` into actual migration scan output while preserving the legacy `contracts.migration-destructive` finding for existing waivers and baselines. +- Updated shipped docs and examples for `reliability`, `data`, `production_risk`, and non-expand/contract migration behavior. + +Verification completed: + +- `go test ./tests/checks -run 'Test(Reliability|Data)'` +- `go test ./internal/codeguard/checks/reliability ./internal/codeguard/checks/data ./tests/cli -run 'TestSDKRuleMetadataFor(Reliability|Data)'` +- `go test ./tests/checks -run 'TestContracts(Migration|FullScan)'` +- `go test ./tests/cli -run 'TestSDKRuleMetadataFor(Reliability|Data|NonExpand)'` +- `go test ./...` with localhost test escalation. +- `make codeguard-ci`. +- `make ci` with localhost test escalation. + ## Goal Make CodeGuard detect production-readiness failures that commonly cause outages or data loss: @@ -127,73 +146,73 @@ Initial rule IDs: | Status | Task | Files/area | Tests | Notes | | --- | --- | --- | --- | --- | -| Todo | Decide section IDs and display names | `internal/codeguard/runner/checks/registry.go` | `go test ./tests/checks ./tests/cli` | Prefer stable snake_case final IDs: `reliability`, `data`. Avoid the existing supply-chain hyphen/underscore mismatch. | -| Todo | Decide default enablement | `internal/codeguard/config/defaults.go`, `internal/codeguard/config/profile.go` | `go test ./internal/codeguard/config ./tests/codeguard` | Suggested: startup warns only for severe reliability; strict enables reliability; enterprise enables reliability + data; ai-safe enables reliability + data diff signals. | -| Todo | Define severity policy | rule catalogs + profile docs | metadata tests | Suggested: block only high-confidence outage/data-loss patterns; warn confidence-based heuristics. | -| Todo | Define language priority | check packages | targeted check tests | Start Go first, then TypeScript/JavaScript, then Python. C++/Rust/Java can begin as catalog/config placeholders only when detectors are not ready. | +| Done | Decide section IDs and display names | `internal/codeguard/runner/checks/registry.go` | `go test ./tests/checks ./tests/cli` | Stable section IDs: `reliability`, `data`. | +| Done | Decide default enablement | `internal/codeguard/config/defaults.go`, `internal/codeguard/config/profile.go` | `go test ./internal/codeguard/config ./tests/codeguard` | Profile-gated rollout implemented. | +| Done | Define severity policy | rule catalogs + profile docs | metadata tests | High-confidence outage/data-loss patterns fail; confidence-based heuristics warn. | +| Done | Define language priority | check packages | targeted check tests | Implemented for Go, Python, TypeScript, JavaScript, and C++. | ### Phase 1: Add family scaffolding | Status | Task | Files/area | Tests | Notes | | --- | --- | --- | --- | --- | -| Todo | Add `ReliabilityRulesConfig` and `DataRulesConfig` | `internal/codeguard/core/config_rule_types.go` | config tests | Use `*bool` per rule toggle so omitted values can get defaults. Add thresholds for max retry count, max queue/buffer size, unbounded-read row limit, and trusted boundary patterns. | -| Todo | Add top-level toggles | `internal/codeguard/core/config_types.go` | config IO tests | `Reliability *bool` if omitted should support profile defaults; `Data *bool` if data rules should start opt-in outside enterprise. | -| Todo | Add defaults/examples | `internal/codeguard/config/defaults.go`, `defaults_rules.go`, `example.go`, `example_rules.go` | `go test ./internal/codeguard/config ./tests/codeguard` | Mirror supply-chain/performance patterns. | -| Todo | Add validation | `internal/codeguard/config/validate_reliability.go`, `validate_data.go`, `validate.go` | config validation tests | Validate thresholds are positive, pattern entries are non-empty, and rule dependencies are coherent. | -| Todo | Add SDK aliases | `pkg/codeguard/sdk_types_config_checks.go` | `go test ./pkg/codeguard` | Keep public SDK config usable. | -| Todo | Add catalogs | `internal/codeguard/rules/catalog_reliability.go`, `catalog_data.go`, `catalog.go` | `go test ./tests/cli` | Explicit `LanguageCoverage` for all non-language-prefixed IDs. | -| Todo | Add fix templates | `internal/codeguard/rules/catalog_fix_templates_reliability.go`, `catalog_fix_templates_data.go` | metadata tests | Use guided templates for concurrency/data risks; deterministic templates only for mechanical timeout/context cases. | +| Done | Add `ReliabilityRulesConfig` and `DataRulesConfig` | `internal/codeguard/core/config_rule_types.go` | config tests | Implemented with `*bool` toggles and thresholds. | +| Done | Add top-level toggles | `internal/codeguard/core/config_types.go` | config IO tests | `Reliability *bool` and `Data *bool` implemented. | +| Done | Add defaults/examples | `internal/codeguard/config/defaults.go`, `defaults_rules.go`, `example.go`, `example_rules.go`, `examples/codeguard.json` | `go test ./internal/codeguard/config ./tests/codeguard` | Defaults and example config updated. | +| Done | Add validation | `internal/codeguard/config/validate_reliability_data.go`, `validate.go` | config validation tests | Threshold validation implemented. | +| Done | Add SDK aliases | `pkg/codeguard/sdk_types_config_checks.go` | `go test ./pkg/codeguard` | Public SDK aliases implemented. | +| Done | Add catalogs | `internal/codeguard/rules/catalog_reliability.go`, `catalog_data.go`, `catalog_contracts.go`, `catalog.go` | `go test ./tests/cli` | Explicit language coverage implemented. | +| Done | Add fix templates | `internal/codeguard/rules/catalog_fix_templates_reliability.go`, `catalog_fix_templates_data.go`, `catalog_fix_templates_misc.go` | metadata tests | Fix templates implemented. | ### Phase 2: Implement reliability detectors | Status | Task | Files/area | Tests | Notes | | --- | --- | --- | --- | --- | -| Todo | Create check package | `internal/codeguard/checks/reliability/reliability.go` | `tests/checks/reliability_test.go` | Follow `supplychain.Run` shape and use `env.FinalizeSection("reliability", "Reliability", findings)`. | -| Todo | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Place after performance and before design/security so production-readiness issues appear early. | -| Todo | Detect Go outbound calls without timeout/context | `internal/codeguard/checks/reliability/*go*.go` | `TestReliabilityGoMissingTimeout` | Flag `http.Get`, `http.Post`, `http.DefaultClient.Do`, `exec.Command`, raw network calls, and DB calls lacking context where applicable. Avoid false positives for explicit `http.Client{Timeout: ...}` and context-bound requests. | -| Todo | Detect retry loops without limits/backoff/jitter | Go detector files | `TestReliabilityGoRetryPolicy` | Identify loops around calls with `retry`, `attempt`, transient errors, or status-code checks. Evidence should include limit/backoff/jitter absence separately. | -| Todo | Detect non-idempotent retries | Go detector files | `TestReliabilityGoNonIdempotentRetry` | Flag retried `POST`, writes, DB mutations, event publishes, or side-effect calls unless idempotency key/dedup marker is present. Confidence-based. | -| Todo | Detect missing cancellation propagation | Go detector files | `TestReliabilityGoMissingCancellation` | Flag background goroutines or downstream calls using `context.Background()`/`TODO()` inside request/job flows. | -| Todo | Detect unbounded work/concurrency | Go detector files | `TestReliabilityGoUnboundedWork` | Flag unbounded goroutine spawn in loops, unbounded channel buffers, unbounded worker queues, and `errgroup` without limits where supported. | -| Todo | Detect resource leaks | Go detector files | `TestReliabilityGoResourceLeak` | Track opened files, response bodies, rows, tickers, and timers. Reuse existing parser/support helpers if available. | -| Todo | Detect missing graceful shutdown | Go detector files | `TestReliabilityGoMissingGracefulShutdown` | Flag servers/workers started without signal handling, shutdown context, drain/close path, or wait group. Keep confidence low/medium unless evidence is strong. | -| Todo | Detect swallowed/lost errors and recoverable panic | Go detector files | `TestReliabilityGoErrorHandling` | Coordinate with existing `quality.ai.*` error signals to avoid duplicate rule spam. Prefer reliability IDs for production failure semantics. | +| Done | Create check package | `internal/codeguard/checks/reliability/reliability.go` | `tests/checks/reliability_test.go` | Implemented with `Reliability` section. | +| Done | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Registered in runner. | +| Done | Detect Go outbound calls without timeout/context | `internal/codeguard/checks/reliability/*go*.go` | `TestReliabilityGoMissingTimeout` | Includes safe-pattern coverage for `http.Client{Timeout: ...}` and context-bound requests. | +| Done | Detect retry loops without limits/backoff/jitter | Go/Python/TS/JS/C++ detector files | `TestReliabilityGoRetryRisk`, multi-language reliability tests | Implemented with cross-language coverage. | +| Done | Detect non-idempotent retries | Go/Python/TS/JS/C++ detector files | retry-risk tests | Implemented with idempotency/dedupe evidence checks. | +| Done | Detect missing cancellation propagation | Go detector files | `TestReliabilityGoDetectsCancellationAndUnboundedWork` | Implemented for Go context propagation gaps. | +| Done | Detect unbounded work/concurrency | Go/Python/TS/JS/C++ detector files | unbounded-work tests | Implemented with safe-pattern negative coverage. | +| Done | Detect resource leaks | Go/Python/C++ detector files | resource-leak tests | Implemented with safe cleanup negative coverage. | +| Done | Detect missing graceful shutdown | Go detector files | reliability tests/catalog coverage | Implemented for Go server start without shutdown evidence. | +| Done | Detect swallowed/lost errors and recoverable panic | Go/Python/TS/JS/C++ detector files | error-handling tests | Implemented with reliability IDs for production failure semantics. | ### Phase 3: Implement data-correctness detectors | Status | Task | Files/area | Tests | Notes | | --- | --- | --- | --- | --- | -| Todo | Create check package | `internal/codeguard/checks/data/data.go` | `tests/checks/data_test.go` | Use a dedicated `Data Correctness` section. | -| Todo | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Run after reliability; many data findings will be repo/path-level and diff-filtered by line when possible. | -| Todo | Detect read-modify-write race | Go detector files | `TestDataGoReadModifyWriteRace` | Look for select/read followed by update/write outside transaction or conditional update. Evidence: same key/entity, mutation after read, no transaction/lock/compare-and-swap. | -| Todo | Detect missing transaction boundary | Go detector files | `TestDataGoMissingTransactionBoundary` | Flag multiple related DB writes without transaction wrapper. Keep configurable DB API patterns. | -| Todo | Detect external side effects inside retried transaction | Go detector files | `TestDataGoSideEffectInTransaction` | Flag HTTP/email/event calls inside transaction/retry closures. High production risk. | -| Todo | Detect consumer idempotency gaps | Go/TS/Python detectors | `TestDataConsumerIdempotency` | Flag message handlers without dedup/idempotency key checks around side effects. Start with naming/framework heuristics and confidence evidence. | -| Todo | Detect unsafe dual writes and missing outbox | Go detector files | `TestDataGoOutbox` | Flag DB write plus event publish without outbox, transactional event table, or equivalent configured strategy. | -| Todo | Detect unstable pagination | Go/TS/Python detectors | `TestDataUnstablePagination` | Flag limit/offset without deterministic order or cursor stability. | -| Todo | Detect unbounded DB reads | Go/TS/Python detectors | `TestDataUnboundedRead` | Flag `Find/Select/Query` without limit, streaming, pagination, or bounded filters. | -| Todo | Detect unsafe schema migrations | migration file scanner | `TestDataUnsafeMigration` | Coordinate with `contracts.non-expand-contract-migration`; detect destructive/contracting migrations without expand/contract staging metadata. | -| Todo | Detect exactly-once assumptions | text/code scanner | `TestDataExactlyOnceAssumption` | Flag comments/config/code that assert exactly-once without idempotency/dedup. Low/medium confidence unless tied to consumer code. | -| Todo | Detect cache without policy | Go/TS/Python detectors | `TestDataCacheWithoutPolicy` | Require TTL/invalidation/ownership policy for production caches. Allow configured cache wrappers. | +| Done | Create check package | `internal/codeguard/checks/data/data.go` | `tests/checks/data_test.go` | Implemented with `Data Correctness` section. | +| Done | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Registered in runner after reliability. | +| Done | Detect read-modify-write race | Go detector files | `TestDataGoDetectsReadModifyWriteTransactionSideEffectCacheAndExactlyOnce` | Implemented for Go. | +| Done | Detect missing transaction boundary | Go/Python/TS/JS/C++ detector files | transaction tests | Implemented with cross-language coverage. | +| Done | Detect external side effects inside retried transaction | Go detector files | side-effect transaction tests | Implemented for Go transaction callbacks. | +| Done | Detect consumer idempotency gaps | Go/Python/TS/JS/C++ detectors | consumer idempotency tests | Implemented with dedupe/idempotency evidence checks. | +| Done | Detect unsafe dual writes and missing outbox | Go/Python/TS/JS/C++ detector files | outbox tests | Implemented with outbox negative coverage. | +| Done | Detect unstable pagination | Go/Python/TS/JS/C++ detectors | pagination tests | Implemented with order/bound negative coverage. | +| Done | Detect unbounded DB reads | Go/Python/TS/JS/C++ detectors | unbounded-read tests | Implemented with bound/filter negative coverage. | +| Done | Detect unsafe schema migrations | `internal/codeguard/checks/contracts/migrations.go` | `TestContractsMigrationDestructiveFlagsNewMigrationsOnly` | Emits `contracts.non-expand-contract-migration` while preserving legacy migration rule. | +| Done | Detect exactly-once assumptions | Go/Python/TS/JS/C++ scanners | exactly-once tests | Implemented with idempotency/dedupe evidence exceptions. | +| Done | Detect cache without policy | Go/Python/TS/JS/C++ detectors | cache-policy tests | Implemented with TTL/policy negative coverage. | ### Phase 4: Add production-risk artifact | Status | Task | Files/area | Tests | Notes | | --- | --- | --- | --- | --- | -| Todo | Add artifact schema | `internal/codeguard/core/report_artifact_types.go`, maybe new `report_artifact_pr_summary_types.go` | serialization tests | Add `ReportArtifactKindPRSummary = "pr_summary"` and `PRSummaryArtifact` with `production_risk`. Keep fields additive and `omitempty`. | -| Todo | Add artifact helper | `internal/codeguard/checks/support/artifacts.go` or new support file | artifact tests | Follow `NewSlopScoreArtifact`/`NewChangeRiskArtifact`; defensively copy evidence slices. | -| Todo | Add runner postprocessor | `internal/codeguard/runner/pr_summary.go` | `internal/codeguard/runner/*test.go` | Publish once with `sc.Artifacts.Put(...)`, sorted evidence, deterministic scoring. | -| Todo | Wire production risk inputs | `internal/codeguard/runner/pr_summary.go` | risk tests | Inputs: reliability/data fail/warn findings, non-idempotent retry, missing transaction/outbox, resource leak, unbounded work/read, suppressed findings excluded by current pipeline. | -| Todo | Preserve outputs | `internal/codeguard/report/write.go`, `github_comment.go` if rendered | report tests | JSON should include artifact automatically. Do not emit PR metrics as GitHub annotations. Do not mutate the existing text `Summary:` line. | -| Todo | SDK aliases | `pkg/codeguard/sdk_types_runtime_report.go` | SDK tests | Export new runtime types if public consumers need them. | +| Done | Add artifact schema | `internal/codeguard/core/report_artifact_types.go` | serialization tests | `ReportArtifactKindPRSummary` and `PRSummaryArtifact` implemented. | +| Done | Add artifact helper | `internal/codeguard/checks/support/artifacts.go` | artifact tests | PR summary artifact helper implemented. | +| Done | Add runner postprocessor | `internal/codeguard/runner/pr_summary.go` | `internal/codeguard/runner/pr_summary_test.go` | Deterministic scoring and artifact publication implemented. | +| Done | Wire production risk inputs | `internal/codeguard/runner/pr_summary.go` | risk tests | Reliability/data/non-expand migration inputs wired. | +| Done | Preserve outputs | report/SARIF/GitHub paths | report tests | Artifact remains additive; annotations remain finding-only. | +| Done | SDK aliases | `pkg/codeguard/sdk_types_runtime_report.go` | SDK tests | Runtime report aliases implemented. | ### Phase 5: Documentation and examples | Status | Task | Files/area | Tests | Notes | | --- | --- | --- | --- | --- | -| Todo | Update product docs when behavior exists | `docs/checks.md`, `docs/features.md`, `docs/production.md`, `README.md` | docs checks/self-scan | Do not advertise catalog-only rules as fully implemented. Mark staged/confidence-based behavior clearly. | -| Todo | Update examples | `examples/codeguard.json`, `.codeguard/codeguard.yaml` if appropriate | `make codeguard-ci` | Consider keeping new families opt-in until false-positive rate is measured. | -| Todo | Add migration notes | `docs/production.md` or release notes | n/a | Explain profile behavior and how to tune/waive noisy reliability/data checks. | +| Done | Update product docs when behavior exists | `docs/checks.md`, `docs/features.md`, `docs/production.md` | docs checks/self-scan | Docs now describe implemented behavior and confidence-based heuristics. | +| Done | Update examples | `examples/codeguard.json` | `make codeguard-ci` | Example config includes opt-in reliability/data/production-risk knobs. | +| Done | Add migration notes | `docs/checks.md`, `docs/production.md` | n/a | Non-expand/contract migration and rollout notes added. | ## Detector confidence policy @@ -249,13 +268,13 @@ make ci ## Merge checklist -- [ ] Rule IDs are stable and documented. -- [ ] Every built-in rule has a fix template. -- [ ] New config fields have defaults, validation, examples, and SDK aliases. -- [ ] New sections use stable section IDs and deterministic output. -- [ ] Findings are diff-filtered correctly where line-level evidence exists. -- [ ] Production-risk scoring has deterministic evidence ordering. -- [ ] SARIF/GitHub annotations remain finding-only. -- [ ] Product docs describe implemented behavior, not planned behavior. -- [ ] `make test` passes. -- [ ] `make ci` passes or any skipped gate is explicitly documented. +- [x] Rule IDs are stable and documented. +- [x] Every built-in rule has a fix template. +- [x] New config fields have defaults, validation, examples, and SDK aliases. +- [x] New sections use stable section IDs and deterministic output. +- [x] Findings are diff-filtered correctly where line-level evidence exists. +- [x] Production-risk scoring has deterministic evidence ordering. +- [x] SARIF/GitHub annotations remain finding-only. +- [x] Product docs describe implemented behavior, not planned behavior. +- [x] `make test` passes. +- [x] `make ci` passes or any skipped gate is explicitly documented. diff --git a/docs/checks.md b/docs/checks.md index 12fa082..a0bc415 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -34,6 +34,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 } @@ -48,6 +50,10 @@ Each top-level boolean enables or disables an entire check family. `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 @@ -84,7 +90,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. @@ -289,6 +295,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: @@ -792,6 +800,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: @@ -850,6 +938,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/reliability/reliability_go.go b/internal/codeguard/checks/reliability/reliability_go.go index 75ac219..1202577 100644 --- a/internal/codeguard/checks/reliability/reliability_go.go +++ b/internal/codeguard/checks/reliability/reliability_go.go @@ -20,8 +20,7 @@ func goFindingsForFile(env support.Context, file string, data []byte) []core.Fin hasShutdown := fileHasSelector(parsed, "Shutdown") || fileHasSelector(parsed, "NotifyContext") || fileHasSelector(parsed, "Notify") ast.Inspect(parsed, func(node ast.Node) bool { - switch n := node.(type) { - case *ast.FuncDecl: + if n, ok := node.(*ast.FuncDecl); ok { findings = append(findings, functionReliabilityFindings(env, file, fset, n, rules, httpAliases, hasShutdown)...) return false } 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/cli/features_metadata_test.go b/tests/cli/features_metadata_test.go index f08aa81..862d77b 100644 --- a/tests/cli/features_metadata_test.go +++ b/tests/cli/features_metadata_test.go @@ -89,6 +89,15 @@ func TestSDKRuleMetadataForDataRule(t *testing.T) { } } +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:") { From fda485b4a1e5f0e8bdc6ad39fea4170138ddc363 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Mon, 27 Jul 2026 11:47:33 -0400 Subject: [PATCH 6/7] docs: remove production readiness task board --- ...e-production-reliability-data-readiness.md | 280 ------------------ 1 file changed, 280 deletions(-) delete mode 100644 .claude/task-boards/feature-production-reliability-data-readiness.md diff --git a/.claude/task-boards/feature-production-reliability-data-readiness.md b/.claude/task-boards/feature-production-reliability-data-readiness.md deleted file mode 100644 index 61aef86..0000000 --- a/.claude/task-boards/feature-production-reliability-data-readiness.md +++ /dev/null @@ -1,280 +0,0 @@ -# Task board: feature/production-reliability-data-readiness - -Status: complete -Branch: feature/production-reliability-data-readiness -Last updated: 2026-07-27 -Not final product docs: this is implementation planning for the branch, not shipped user-facing documentation. - -## Progress update: first implementation slice - -Completed in the first implementation pass: - -- Added Reliability and Data Correctness config surfaces, defaults, validation, profile enablement, SDK aliases, rule catalogs, and fix templates. -- Added `Reliability` and `Data Correctness` runner sections. -- Added Go reliability detectors for missing HTTP timeouts, missing cancellation propagation, unbounded goroutine work, retry policy gaps, HTTP response body leaks, swallowed errors, lost error context, recoverable panic, and missing graceful shutdown evidence. -- Added Go data-correctness detectors for read-modify-write/multi-write transaction gaps, side effects inside transaction callbacks, unsafe dual writes, missing outbox evidence, consumer idempotency/dedupe gaps, unstable pagination, unbounded SQL reads, exactly-once assumptions, and cache policy gaps. -- Added additive `pr_summary.production_risk` report artifact and SDK aliases. The artifact is diff-only and does not change SARIF/GitHub annotation/text summary compatibility. -- Added focused tests in `tests/checks/reliability_test.go`, `tests/checks/data_test.go`, `internal/codeguard/runner/pr_summary_test.go`, and representative metadata tests. - -Verification completed: - -- `go test ./...` with localhost test escalation. -- `make codeguard-ci`. - -## Progress update: multi-language production-readiness slice - -Completed in the second implementation pass: - -- Expanded Reliability and Data Correctness rule language coverage to include C++ in addition to Go, Python, TypeScript, and JavaScript. -- Added Python reliability detectors for outbound HTTP calls without timeouts, retry/backoff gaps, non-idempotent retry evidence, unbounded asyncio work, swallowed exceptions, generic recoverable raises, and nearby resource-leak evidence. -- Added TypeScript/JavaScript reliability detectors for HTTP calls without timeout/abort evidence, promise/HTTP work in loops without concurrency limits, retry/backoff gaps, non-idempotent retry evidence, swallowed catch blocks, and generic recoverable throws. -- Added C++ reliability detectors for retry/backoff gaps, non-idempotent retry evidence, thread/task launches in loops without concurrency bounds, generic runtime throws, and raw allocation without nearby ownership cleanup. -- Added Python data-correctness detectors for unbounded reads, unstable pagination, multi-write transaction gaps, write+publish/outbox gaps, consumer idempotency/dedupe gaps, exactly-once assumptions, and cache writes without TTL evidence. -- Added TypeScript/JavaScript data-correctness detectors for unbounded reads, unstable pagination, multi-write transaction gaps, write+publish/outbox gaps, consumer idempotency/dedupe gaps, exactly-once assumptions, and cache writes without TTL evidence. -- Added C++ data-correctness detectors for unbounded reads, unstable pagination, multi-write transaction gaps, write+publish/outbox gaps, consumer idempotency/dedupe gaps, exactly-once assumptions, and cache writes without TTL evidence. -- Added focused multi-language tests for Python, TypeScript, JavaScript, and C++ reliability/data behavior. - -Verification completed: - -- `go test ./internal/codeguard/config ./internal/codeguard/rules ./internal/codeguard/runner ./internal/codeguard/runner/checks ./internal/codeguard/checks/reliability ./internal/codeguard/checks/data ./pkg/codeguard ./tests/checks ./tests/cli`. -- `go test ./...` with localhost test escalation. -- `make codeguard-ci`. - -## Progress update: completion audit - -Completed in the final audit pass: - -- Verified the task-board inventory against implemented Reliability, Data Correctness, API Contracts, and `pr_summary.production_risk` code paths. -- Added targeted positive and negative tests for Go, Python, TypeScript, JavaScript, and C++ reliability/data detectors. -- Wired `contracts.non-expand-contract-migration` into actual migration scan output while preserving the legacy `contracts.migration-destructive` finding for existing waivers and baselines. -- Updated shipped docs and examples for `reliability`, `data`, `production_risk`, and non-expand/contract migration behavior. - -Verification completed: - -- `go test ./tests/checks -run 'Test(Reliability|Data)'` -- `go test ./internal/codeguard/checks/reliability ./internal/codeguard/checks/data ./tests/cli -run 'TestSDKRuleMetadataFor(Reliability|Data)'` -- `go test ./tests/checks -run 'TestContracts(Migration|FullScan)'` -- `go test ./tests/cli -run 'TestSDKRuleMetadataFor(Reliability|Data|NonExpand)'` -- `go test ./...` with localhost test escalation. -- `make codeguard-ci`. -- `make ci` with localhost test escalation. - -## Goal - -Make CodeGuard detect production-readiness failures that commonly cause outages or data loss: - -- reliability failures in outbound calls, retry loops, cancellation, concurrency, cleanup, shutdown, and partial-failure handling; -- distributed-system and data-correctness failures around transactions, idempotency, dual writes, pagination, unbounded reads, migrations, caches, and delivery semantics; -- a first production-risk rollup that turns these findings into PR-level risk evidence. - -This branch should move CodeGuard beyond general code quality and into “will this change behave safely in production?” - -## Non-goals - -- Do not implement broad local design smells, naming checks, or reviewability metrics here. Those belong to `feature/change-safety-testability-refactors`. -- Do not implement observability, ownership, runbook, or rollout-governance checks here except where needed as production-risk inputs. Those belong to `feature/operability-design-delivery-governance`. -- Do not rename existing rule IDs. Waivers and baselines depend on stable IDs. -- Do not make new rules blocking in every profile without a staged rollout path. - -## Product split - -This branch owns: - -- Rule families: `reliability.*`, `data.*`, and `contracts.non-expand-contract-migration`. -- New sections/config: `reliability`, `data`, and production-risk artifact/config. -- Product metric: `production_risk` as part of a new `pr_summary` artifact. - -Adjacent branch contracts: - -- `feature/change-safety-testability-refactors` will add `change_safety`, `maintainability_delta`, and `refactor_confidence` to the same `pr_summary` artifact. -- `feature/operability-design-delivery-governance` will add observability/delivery signals that can feed `production_risk` after the artifact shape exists. - -## Existing repo seams to reuse - -- Rule catalog merge point: `internal/codeguard/rules/catalog.go`. -- Rule metadata schema: `internal/codeguard/core/rule_metadata_types.go`. -- Fix-template requirement: `internal/codeguard/rules/catalog_fix_templates*.go`; every built-in rule needs fix guidance. -- Config surface: `internal/codeguard/core/config_types.go`, `internal/codeguard/core/config_rule_types.go`. -- Defaults/examples/validation: `internal/codeguard/config/defaults.go`, `internal/codeguard/config/defaults_rules.go`, `internal/codeguard/config/example.go`, `internal/codeguard/config/validate.go`. -- Profile behavior: `internal/codeguard/config/profile.go`. -- Check family runner pattern: `internal/codeguard/checks/supplychain/supplychain.go`. -- Runner section registration: `internal/codeguard/runner/checks/registry.go`. -- Finding construction/finalization: `internal/codeguard/runner/support/findings.go`, `internal/codeguard/runner/support/findings_section.go`. -- Diff-aware inputs: `internal/codeguard/runner/support/diff_scope.go`, `internal/codeguard/runner/support/changed_files.go`, `internal/codeguard/core/diff_types.go`. -- Existing risk artifacts: `internal/codeguard/runner/risk_scoring.go`, `internal/codeguard/core/report_artifact_types.go`. - -## Rule inventory - -### Reliability - -Initial rule IDs: - -- `reliability.missing-timeout` -- `reliability.unbounded-retry` -- `reliability.retry-without-backoff` -- `reliability.non-idempotent-retry` -- `reliability.missing-cancellation` -- `reliability.unbounded-work` -- `reliability.missing-concurrency-limit` -- `reliability.resource-leak` -- `reliability.partial-failure-hidden` -- `reliability.missing-graceful-shutdown` -- `reliability.swallowed-error` -- `reliability.lost-error-context` -- `reliability.recoverable-panic` - -### Data correctness - -Initial rule IDs: - -- `data.read-modify-write-race` -- `data.missing-transaction-boundary` -- `data.side-effect-in-transaction` -- `data.non-idempotent-consumer` -- `data.missing-deduplication` -- `data.unsafe-dual-write` -- `data.missing-outbox-strategy` -- `data.unstable-pagination` -- `data.unbounded-read` -- `data.exactly-once-assumption` -- `data.cache-without-policy` -- `contracts.non-expand-contract-migration` - -## Implementation phases - -### Phase 0: Design the rollout contract - -| Status | Task | Files/area | Tests | Notes | -| --- | --- | --- | --- | --- | -| Done | Decide section IDs and display names | `internal/codeguard/runner/checks/registry.go` | `go test ./tests/checks ./tests/cli` | Stable section IDs: `reliability`, `data`. | -| Done | Decide default enablement | `internal/codeguard/config/defaults.go`, `internal/codeguard/config/profile.go` | `go test ./internal/codeguard/config ./tests/codeguard` | Profile-gated rollout implemented. | -| Done | Define severity policy | rule catalogs + profile docs | metadata tests | High-confidence outage/data-loss patterns fail; confidence-based heuristics warn. | -| Done | Define language priority | check packages | targeted check tests | Implemented for Go, Python, TypeScript, JavaScript, and C++. | - -### Phase 1: Add family scaffolding - -| Status | Task | Files/area | Tests | Notes | -| --- | --- | --- | --- | --- | -| Done | Add `ReliabilityRulesConfig` and `DataRulesConfig` | `internal/codeguard/core/config_rule_types.go` | config tests | Implemented with `*bool` toggles and thresholds. | -| Done | Add top-level toggles | `internal/codeguard/core/config_types.go` | config IO tests | `Reliability *bool` and `Data *bool` implemented. | -| Done | Add defaults/examples | `internal/codeguard/config/defaults.go`, `defaults_rules.go`, `example.go`, `example_rules.go`, `examples/codeguard.json` | `go test ./internal/codeguard/config ./tests/codeguard` | Defaults and example config updated. | -| Done | Add validation | `internal/codeguard/config/validate_reliability_data.go`, `validate.go` | config validation tests | Threshold validation implemented. | -| Done | Add SDK aliases | `pkg/codeguard/sdk_types_config_checks.go` | `go test ./pkg/codeguard` | Public SDK aliases implemented. | -| Done | Add catalogs | `internal/codeguard/rules/catalog_reliability.go`, `catalog_data.go`, `catalog_contracts.go`, `catalog.go` | `go test ./tests/cli` | Explicit language coverage implemented. | -| Done | Add fix templates | `internal/codeguard/rules/catalog_fix_templates_reliability.go`, `catalog_fix_templates_data.go`, `catalog_fix_templates_misc.go` | metadata tests | Fix templates implemented. | - -### Phase 2: Implement reliability detectors - -| Status | Task | Files/area | Tests | Notes | -| --- | --- | --- | --- | --- | -| Done | Create check package | `internal/codeguard/checks/reliability/reliability.go` | `tests/checks/reliability_test.go` | Implemented with `Reliability` section. | -| Done | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Registered in runner. | -| Done | Detect Go outbound calls without timeout/context | `internal/codeguard/checks/reliability/*go*.go` | `TestReliabilityGoMissingTimeout` | Includes safe-pattern coverage for `http.Client{Timeout: ...}` and context-bound requests. | -| Done | Detect retry loops without limits/backoff/jitter | Go/Python/TS/JS/C++ detector files | `TestReliabilityGoRetryRisk`, multi-language reliability tests | Implemented with cross-language coverage. | -| Done | Detect non-idempotent retries | Go/Python/TS/JS/C++ detector files | retry-risk tests | Implemented with idempotency/dedupe evidence checks. | -| Done | Detect missing cancellation propagation | Go detector files | `TestReliabilityGoDetectsCancellationAndUnboundedWork` | Implemented for Go context propagation gaps. | -| Done | Detect unbounded work/concurrency | Go/Python/TS/JS/C++ detector files | unbounded-work tests | Implemented with safe-pattern negative coverage. | -| Done | Detect resource leaks | Go/Python/C++ detector files | resource-leak tests | Implemented with safe cleanup negative coverage. | -| Done | Detect missing graceful shutdown | Go detector files | reliability tests/catalog coverage | Implemented for Go server start without shutdown evidence. | -| Done | Detect swallowed/lost errors and recoverable panic | Go/Python/TS/JS/C++ detector files | error-handling tests | Implemented with reliability IDs for production failure semantics. | - -### Phase 3: Implement data-correctness detectors - -| Status | Task | Files/area | Tests | Notes | -| --- | --- | --- | --- | --- | -| Done | Create check package | `internal/codeguard/checks/data/data.go` | `tests/checks/data_test.go` | Implemented with `Data Correctness` section. | -| Done | Register section | `internal/codeguard/runner/checks/registry.go` | section smoke test | Registered in runner after reliability. | -| Done | Detect read-modify-write race | Go detector files | `TestDataGoDetectsReadModifyWriteTransactionSideEffectCacheAndExactlyOnce` | Implemented for Go. | -| Done | Detect missing transaction boundary | Go/Python/TS/JS/C++ detector files | transaction tests | Implemented with cross-language coverage. | -| Done | Detect external side effects inside retried transaction | Go detector files | side-effect transaction tests | Implemented for Go transaction callbacks. | -| Done | Detect consumer idempotency gaps | Go/Python/TS/JS/C++ detectors | consumer idempotency tests | Implemented with dedupe/idempotency evidence checks. | -| Done | Detect unsafe dual writes and missing outbox | Go/Python/TS/JS/C++ detector files | outbox tests | Implemented with outbox negative coverage. | -| Done | Detect unstable pagination | Go/Python/TS/JS/C++ detectors | pagination tests | Implemented with order/bound negative coverage. | -| Done | Detect unbounded DB reads | Go/Python/TS/JS/C++ detectors | unbounded-read tests | Implemented with bound/filter negative coverage. | -| Done | Detect unsafe schema migrations | `internal/codeguard/checks/contracts/migrations.go` | `TestContractsMigrationDestructiveFlagsNewMigrationsOnly` | Emits `contracts.non-expand-contract-migration` while preserving legacy migration rule. | -| Done | Detect exactly-once assumptions | Go/Python/TS/JS/C++ scanners | exactly-once tests | Implemented with idempotency/dedupe evidence exceptions. | -| Done | Detect cache without policy | Go/Python/TS/JS/C++ detectors | cache-policy tests | Implemented with TTL/policy negative coverage. | - -### Phase 4: Add production-risk artifact - -| Status | Task | Files/area | Tests | Notes | -| --- | --- | --- | --- | --- | -| Done | Add artifact schema | `internal/codeguard/core/report_artifact_types.go` | serialization tests | `ReportArtifactKindPRSummary` and `PRSummaryArtifact` implemented. | -| Done | Add artifact helper | `internal/codeguard/checks/support/artifacts.go` | artifact tests | PR summary artifact helper implemented. | -| Done | Add runner postprocessor | `internal/codeguard/runner/pr_summary.go` | `internal/codeguard/runner/pr_summary_test.go` | Deterministic scoring and artifact publication implemented. | -| Done | Wire production risk inputs | `internal/codeguard/runner/pr_summary.go` | risk tests | Reliability/data/non-expand migration inputs wired. | -| Done | Preserve outputs | report/SARIF/GitHub paths | report tests | Artifact remains additive; annotations remain finding-only. | -| Done | SDK aliases | `pkg/codeguard/sdk_types_runtime_report.go` | SDK tests | Runtime report aliases implemented. | - -### Phase 5: Documentation and examples - -| Status | Task | Files/area | Tests | Notes | -| --- | --- | --- | --- | --- | -| Done | Update product docs when behavior exists | `docs/checks.md`, `docs/features.md`, `docs/production.md` | docs checks/self-scan | Docs now describe implemented behavior and confidence-based heuristics. | -| Done | Update examples | `examples/codeguard.json` | `make codeguard-ci` | Example config includes opt-in reliability/data/production-risk knobs. | -| Done | Add migration notes | `docs/checks.md`, `docs/production.md` | n/a | Non-expand/contract migration and rollout notes added. | - -## Detector confidence policy - -- High confidence: syntactic evidence directly proves missing timeout, ignored cleanup error, unbounded goroutine in loop, multiple DB writes without transaction, or DB write plus event publish without outbox. -- Medium confidence: naming/API heuristics imply retry, idempotency, consumer, cache, transaction, or side effect but repository-specific wrapper may exist. -- Low confidence: comment/text/history-derived signals such as exactly-once assumptions or cascading synchronous dependency risk. - -Findings should include evidence metadata that is safe for reports: operation kind, call kind, retry loop evidence, transaction wrapper evidence, idempotency evidence, and configured framework match. Do not include source snippets or secrets in metadata. - -## Profile behavior target - -| Profile | Reliability | Data correctness | Production risk | -| --- | --- | --- | --- | -| Startup | Warn severe/high-confidence reliability only | Off by default | Warn only | -| Strict | Block new high-confidence reliability regressions; warn medium confidence | Warn high-confidence data risks in diff mode | Warn elevated risk | -| Enterprise | Block severe reliability and data-loss risks | Block unsafe dual writes, missing transaction boundaries, unsafe migrations | Warn/block by threshold | -| AI-safe | Strict plus weak error handling, oversized risk, and missing tests from sibling branch | Strict data diff signals | Warn/block by threshold | - -## Acceptance criteria - -- New family config validates and round-trips in JSON/YAML. -- `codeguard rules` exposes reliability/data metadata with language coverage and fix templates. -- Enabling the new sections runs without panics on empty repos and on this repo. -- Go reliability detectors cover at least missing timeout, unbounded retry, missing cancellation, unbounded work, resource leak, swallowed/lost errors, and recoverable panic. -- Go data detectors cover at least read-modify-write race, missing transaction, side-effect-in-transaction, unsafe dual write/missing outbox, unstable pagination, unbounded read, and cache-without-policy. -- The `pr_summary` artifact includes deterministic `production_risk` score/evidence in diff scans. -- Existing JSON/SARIF/GitHub annotation/text summary compatibility is preserved. -- Targeted tests and `make test` pass before push/PR. - -## Verification plan - -Targeted during implementation: - -```sh -go test ./internal/codeguard/config ./internal/codeguard/rules ./internal/codeguard/runner -go test ./tests/codeguard ./tests/checks ./tests/cli -run 'Test.*(Reliability|Data|ProductionRisk|Rules|Profiles|Metadata)' -go test ./tests/security ./tests/checks -run 'TestWriteReport|TestSARIF|TestGitHub' -``` - -Branch gate: - -```sh -make fmt-check -make test -make codeguard-ci -``` - -Pre-push/PR gate when practical: - -```sh -make ci -``` - -## Merge checklist - -- [x] Rule IDs are stable and documented. -- [x] Every built-in rule has a fix template. -- [x] New config fields have defaults, validation, examples, and SDK aliases. -- [x] New sections use stable section IDs and deterministic output. -- [x] Findings are diff-filtered correctly where line-level evidence exists. -- [x] Production-risk scoring has deterministic evidence ordering. -- [x] SARIF/GitHub annotations remain finding-only. -- [x] Product docs describe implemented behavior, not planned behavior. -- [x] `make test` passes. -- [x] `make ci` passes or any skipped gate is explicitly documented. From a3bd98f7873bde3ca2b1e54a372796b08517c53d Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Mon, 27 Jul 2026 11:52:35 -0400 Subject: [PATCH 7/7] docs: add check glossary --- README.md | 8 +++++--- docs/checks.md | 29 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) 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 a0bc415..a680060 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