diff --git a/.claude/task-boards/feature-operability-design-delivery-governance.md b/.claude/task-boards/feature-operability-design-delivery-governance.md new file mode 100644 index 0000000..5cc4335 --- /dev/null +++ b/.claude/task-boards/feature-operability-design-delivery-governance.md @@ -0,0 +1,282 @@ +# Task board: feature/operability-design-delivery-governance + +Status: staging +Branch: feature/operability-design-delivery-governance +Last updated: 2026-07-27 +Not final product docs: this is implementation planning for the branch, not shipped user-facing documentation. + +## Goal + +Make CodeGuard evaluate whether production code is operable, locally well-designed, and safe to roll out. + +This branch owns: + +- observability and operations readiness; +- abstraction-quality and local software-design checks; +- delivery-governance and rollout-safety checks; +- enterprise/profile behavior for ownership, runbooks, service compatibility, supply-chain provenance, and deployment verification. + +The product target is to catch changes that are technically correct but hard to operate, hard to change, or unsafe to deploy. + +## Workstream D audit and reconciliation checklist + +Status: prep-audited. As of 2026-07-27, the branch task board lists the intended rule inventory, but the shipped rule catalogs, detector packages, config fields, profile defaults, and user-facing docs for this branch have not landed yet. Keep `docs/checks.md`, `docs/features.md`, `docs/production.md`, `README.md`, and `examples/codeguard.json` unchanged until matching behavior exists in code and tests. + +Workstream D owns final reconciliation after implementation slices merge: + +- Confirm new rule metadata exists for every implemented `observability.*`, `operations.*`, `delivery.*`, `ci.*`, `supply_chain.*`, `design.*`, and `quality.*` rule in scope. +- Confirm every new rule has language coverage, profile behavior, examples where useful, and a fix template or explicit guided remediation. +- Confirm SDK aliases/config API cover any new config structs or rule toggles. +- Confirm profile comparison output reflects startup, strict, enterprise, and AI-safe behavior for the landed rules. +- Update shipped docs only after detector behavior and tests exist. +- Keep this task board accurate as implementation workers land commits; mark a task Done only after code, tests, metadata, and docs/profile behavior are reconciled. +- Add the final PR-summary draft section once the branch has enough implementation to summarize accurately. + +Workstream D verification commands: + +```sh +env -u GOROOT GOCACHE=/private/tmp/codeguard-operability-go-cache go test ./internal/codeguard/config ./tests/cli ./tests/codeguard -run 'Test.*(Profile|Metadata|Config|Documentation|SDK)' +git diff --check +``` + +## Non-goals + +- Do not implement reliability/data-correctness detectors owned by `feature/production-reliability-data-readiness`. +- Do not implement change/refactor/testability metrics owned by `feature/change-safety-testability-refactors`. +- Do not duplicate existing architecture-boundary checks unless the new rule is about local abstraction quality or operability. +- Do not block rollout-governance findings by default in startup/strict without profile-specific staging. + +## Product split + +This branch owns: + +- Rule families: `observability.*`, `operations.*`, additional `design.*`, additional `quality.*`, additional `delivery.*`, additional `ci.*`, and `supply-chain.missing-provenance`. +- Enterprise behavior: ownership, observability, rollout safety, supply chain, runbooks, and service compatibility. +- Production-risk inputs: observability/delivery/operations findings can feed the `production_risk` metric once the shared `pr_summary` artifact exists. + +Adjacent branch contracts: + +- `feature/production-reliability-data-readiness` owns the initial `production_risk` artifact field and reliability/data signals. +- `feature/change-safety-testability-refactors` owns `maintainability_delta`; this branch may add design-governance findings that become inputs later. + +## Existing repo seams to reuse + +- Existing design rules/catalogs: `internal/codeguard/checks/design/*`, `internal/codeguard/rules/catalog_design.go`, `catalog_design_graph.go`, `catalog_design_policy.go`. +- Existing CI/release rules: `internal/codeguard/checks/ci/*`, `internal/codeguard/rules/catalog_test_quality.go`, `catalog_misc.go`. +- Existing supply-chain rules: `internal/codeguard/checks/supplychain/*`, `internal/codeguard/rules/catalog_supplychain.go`. +- Config surface: `internal/codeguard/core/config_types.go`, `internal/codeguard/core/config_rule_types.go`. +- Defaults/examples/validation: `internal/codeguard/config/defaults.go`, `defaults_rules.go`, `example.go`, `validate.go`. +- Runner section registry: `internal/codeguard/runner/checks/registry.go`. +- Rule metadata/fix templates: `internal/codeguard/rules/catalog*.go`, `internal/codeguard/rules/catalog_fix_templates*.go`. +- Report compatibility: `internal/codeguard/report/write.go`, `internal/codeguard/report/github_comment.go`, `internal/codeguard/report/sarif_builders.go`. +- Docs: `docs/checks.md`, `docs/features.md`, `docs/production.md`, `docs/integrations.md`, `README.md`. + +## Rule inventory + +### Observability and operations + +- `observability.unstructured-log` +- `observability.error-without-context` +- `observability.sensitive-log-data` +- `observability.high-cardinality-label` +- `observability.critical-path-uninstrumented` +- `observability.log-and-ignore` +- `observability.shallow-health-check` +- `operations.missing-owner` +- `operations.missing-runbook` + +### Abstraction quality and local design + +- `design.shallow-module` +- `design.excessive-public-surface` +- `design.pass-through-abstraction` +- `design.configuration-leak` +- `design.temporal-coupling` +- `quality.duplicated-knowledge` +- `design.infrastructure-type-leak` +- `design.persistence-model-leak` +- `design.domain-logic-in-handler` +- `quality.ambiguous-name` +- `quality.boolean-argument` +- `quality.mixed-abstraction-levels` +- `quality.excessive-parameters` +- `quality.primitive-obsession` +- `quality.hidden-side-effect` +- `quality.mutable-global-state` +- `quality.redundant-comment` + +### Delivery governance + +- `ci.missing-required-gate` +- `ci.mutable-deployment-reference` +- `delivery.missing-rollback-strategy` +- `delivery.unsafe-migration-order` +- `delivery.high-risk-change-without-kill-switch` +- `delivery.missing-post-deploy-verification` +- `supply-chain.missing-provenance` +- `quality.environment-branching` + +## Implementation phases + +### Phase 0: Decide section and profile shape + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Decide section IDs | `runner/checks/registry.go` | section smoke tests | Suggested new sections: `observability`, `operations`, `delivery`; extend existing `design`, `quality`, `ci`, and `supply_chain` where rule families already exist. | +| Todo | Define enterprise defaults | `config/profile.go` | profile tests | Enterprise should enable ownership, observability, rollout safety, supply-chain provenance, runbooks, and service compatibility. | +| Todo | Define strict/startup behavior | profile/docs | profile tests | Startup warns only. Strict can warn for observability/design and block only existing required CI/security gates. | +| Todo | Define evidence model | rule packages | report confidence tests | Most rules need confidence/evidence rather than binary proof. Avoid shallow style-lint behavior. | + +### Phase 1: Add config, catalogs, and section scaffolding + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Add `ObservabilityRulesConfig` | `core/config_rule_types.go`, `core/config_types.go` | config tests | Include structured logger patterns, sensitive-name patterns, metric label deny patterns, critical path patterns, healthcheck path patterns. | +| Todo | Add `OperationsRulesConfig` | config files | config tests | Include owner file patterns, runbook path patterns, critical service path patterns. | +| Todo | Add `DeliveryRulesConfig` | config files | config tests | Include required CI gates, allowed deployment refs, rollback docs patterns, migration ordering config, kill-switch patterns, post-deploy verification patterns. | +| Todo | Add defaults/examples/validation | `config/defaults*.go`, `config/example*.go`, `config/validate_*.go` | `go test ./internal/codeguard/config ./tests/codeguard` | Validate non-empty patterns, positive thresholds, and no conflicting allow/deny refs. | +| Todo | Add SDK aliases | `pkg/codeguard/sdk_types_config_checks.go` | SDK tests | Keep config API complete. | +| Todo | Add catalogs | `rules/catalog_observability.go`, `catalog_operations.go`, `catalog_delivery.go`, extend design/quality/ci/supplychain catalogs | metadata tests | Explicit `LanguageCoverage`. Keep `supply-chain.missing-provenance` spelling aligned with existing prefix convention; repo currently uses `supply_chain.*`, so decide whether to normalize to `supply_chain.missing-provenance` before implementation. | +| Todo | Add fix templates | `rules/catalog_fix_templates_observability.go`, `catalog_fix_templates_delivery.go`, design/quality template files | metadata tests | Mostly guided templates; deterministic only for pinning mutable refs or adding metadata files. | + +### Phase 2: Implement observability checks + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Create observability package | `internal/codeguard/checks/observability/observability.go` | `tests/checks/observability_test.go` | Follow section pattern and finalize as `observability`, `Observability`. | +| Todo | Register section | `runner/checks/registry.go` | section smoke test | Run after reliability/data when those exist; otherwise after security/design. | +| Todo | Detect unstructured logs | Go/TS/Python detectors | `TestObservabilityUnstructuredLog` | Flag `fmt.Println`, `console.log`, raw string logs, logger calls without fields in production code. Allow tests/scripts. | +| Todo | Detect errors without context | detectors | `TestObservabilityErrorWithoutContext` | Error logs should include operation/request/customer-safe context. Avoid requiring request IDs in low-level pure functions. | +| Todo | Detect sensitive log data | detectors | `TestObservabilitySensitiveLogData` | Reuse security secret/sensitive-name patterns. Flag tokens, passwords, auth headers, PII-like names in log fields/messages. | +| Todo | Detect high-cardinality metric labels | detectors | `TestObservabilityHighCardinalityLabel` | Flag labels with user_id, email, request_id, path with raw params, UUID/order IDs. Allow configured sanitized labels. | +| Todo | Detect critical paths without instrumentation | path/config + parser helpers | `TestObservabilityCriticalPathUninstrumented` | Critical paths: handlers, jobs, consumers, migrations, payment/write flows. Require span/metric/log evidence based on config. | +| Todo | Detect log-and-ignore | error/log detectors | `TestObservabilityLogAndIgnore` | Distinguish from `error.logged-and-ignored` sibling branch by placing operability-focused log-only failure under observability unless it changes reliability semantics. | +| Todo | Detect shallow health checks | route/config scanner | `TestObservabilityShallowHealthCheck` | Flag health endpoints that only return static OK while critical dependencies exist. Confidence based on dependency evidence. | + +### Phase 3: Implement operations ownership/runbook checks + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Create operations package | `internal/codeguard/checks/operations/operations.go` | `tests/checks/operations_test.go` | Repo-level and path-level findings. | +| Todo | Register section | `runner/checks/registry.go` | section smoke test | Enterprise profile should enable by default. | +| Todo | Detect missing service ownership | operations package | `TestOperationsMissingOwner` | Support CODEOWNERS, service catalog files, ownership metadata in config, package-level metadata. | +| Todo | Detect missing runbook metadata | operations package | `TestOperationsMissingRunbook` | Critical systems require runbook links or local runbook files. Allow configured critical path patterns. | +| Todo | Add ownership-gap cross-feed | operations + maintainability later | operations tests | Findings can feed maintainability/production-risk metrics in other branches. | + +### Phase 4: Implement abstraction-quality design checks + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Extend design config | `DesignRulesConfig` or new local-design config | config tests | Thresholds: public symbol count, pass-through ratio, temporal coupling evidence, handler/domain path patterns, infrastructure/domain path patterns. | +| Todo | Extend design catalog | `rules/catalog_design.go` or `catalog_design_local.go` | metadata tests | Keep existing design family rather than a competing family. | +| Todo | Detect shallow modules | design package | `TestDesignShallowModule` | Public API surface high but implementation depth/behavior low. Confidence-based. | +| Todo | Detect excessive public surface | design package | `TestDesignExcessivePublicSurface` | Exported symbols/public members per package/module. Exempt SDK packages via config. | +| Todo | Detect pass-through abstractions | design package | `TestDesignPassThroughAbstraction` | Methods/functions that only delegate without policy, translation, validation, or isolation. | +| Todo | Detect configuration leak | design package | `TestDesignConfigurationLeak` | Config structs/options crossing module boundaries or leaking env/deployment concerns into domain code. | +| Todo | Detect temporal coupling | design/history package | `TestDesignTemporalCoupling` | Required call order encoded implicitly. Start with obvious init/use/close or set-before-call patterns. | +| Todo | Detect duplicated business knowledge | quality/design package | `TestQualityDuplicatedKnowledge` | Constants/rules/calculations duplicated across layers. Not the same as token-level duplicate code. | +| Todo | Detect infrastructure type leak | design package | `TestDesignInfrastructureTypeLeak` | DB/HTTP/framework/logger/cloud SDK types in domain packages or public APIs. | +| Todo | Detect persistence model leak | design package | `TestDesignPersistenceModelLeak` | ORM/db model structs returned through public API/handler contracts. | +| Todo | Detect domain logic in handlers/controllers | design package | `TestDesignDomainLogicInHandler` | Handlers should orchestrate/validate/translate, not own business rules. | + +### Phase 5: Precision cleanup for quality rules in this branch + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Add/align local quality catalog entries | `rules/catalog_quality.go` or new local quality catalog | metadata tests | These are local design-quality rules that fit existing `quality.*` prefix. | +| Todo | Detect ambiguous names | quality parser helpers | `TestQualityAmbiguousName` | `data`, `manager`, `helper`, `process`, `thing`; avoid one-off test fixture false positives. | +| Todo | Detect boolean arguments | quality parser helpers | `TestQualityBooleanArgument` | Flag public/business functions with behavior-hiding booleans. Allow setters/options/builders. | +| Todo | Detect mixed abstraction levels | quality/design parser helpers | `TestQualityMixedAbstractionLevels` | Coordinate with `function.mixed-abstraction-level` branch later. | +| Todo | Detect primitive obsession | quality parser helpers | `TestQualityPrimitiveObsession` | Repeated raw strings/ints for domain concepts, especially IDs/units/currency. | +| Todo | Detect hidden side effects | quality parser helpers | `TestQualityHiddenSideEffect` | Function name implies query/format/build but mutates state, writes, logs, or performs I/O. | +| Todo | Detect mutable global state | quality parser helpers | `TestQualityMutableGlobalState` | Flag mutable package/module globals in production code; allow constants and guarded test hooks. | +| Todo | Detect redundant comments | quality text/parser helpers | `TestQualityRedundantComment` | Comments that only restate nearby code. Low confidence; warn only. | + +### Phase 6: Implement delivery governance checks + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Extend CI package or create delivery package | `internal/codeguard/checks/ci/*`, `internal/codeguard/checks/delivery/*` | `tests/checks/delivery_test.go` | Use `ci.*` for CI gate config; use `delivery.*` for rollout strategy. | +| Todo | Detect missing required CI gates | CI package | `TestCIMissingRequiredGate` | Validate required workflows/jobs/check names in `.github/workflows`, config, or CI provider files. | +| Todo | Detect mutable deployment references | CI/delivery package | `TestCIMutableDeploymentReference` | Floating GitHub Actions refs, image tags like `latest`, branch deploy refs, unpinned external actions. | +| Todo | Detect missing rollback strategy | delivery package | `TestDeliveryMissingRollbackStrategy` | High-risk deployment/migration changes require rollback docs/config/runbook reference. | +| Todo | Detect unsafe migration ordering | delivery + data migration scanner | `TestDeliveryUnsafeMigrationOrder` | Coordinate with data branch migration rule; this branch focuses rollout sequencing evidence. | +| Todo | Detect high-risk feature without kill switch | delivery package | `TestDeliveryHighRiskNoKillSwitch` | New critical path, payment/auth/data migration behavior needs feature flag/kill switch evidence. | +| Todo | Detect missing post-deploy verification | delivery package | `TestDeliveryMissingPostDeployVerification` | Deploy workflows should verify health/SLO/smoke checks after production rollout. | +| Todo | Detect missing artifact provenance | supplychain package | `TestSupplyChainMissingProvenance` | Prefer `supply_chain.missing-provenance` to match existing prefix unless compatibility requires hyphen. Check SBOM/attestation/provenance files or workflow steps. | +| Todo | Detect environment branching in source | quality/delivery package | `TestQualityEnvironmentBranching` | Flag production/staging/dev branching embedded in domain/source code. Allow config/bootstrap boundaries. | + +### Phase 7: Production-risk integration and docs + +| Status | Task | Files/area | Tests | Notes | +| --- | --- | --- | --- | --- | +| Todo | Feed observability/operations/delivery findings into production risk | `runner/pr_summary.go` if present | PR-summary tests | Additive only; do not make this branch depend on unmerged artifact work unless rebased after branch 1. | +| Todo | Render optional GitHub-comment block | `report/github_comment.go` | report tests | Only if `pr_summary` exists. Keep annotations finding-only. | +| Todo | Update docs after behavior lands | `docs/checks.md`, `docs/features.md`, `docs/production.md`, `docs/integrations.md`, `README.md` | docs/self-scan | Explain enterprise vs startup/strict behavior and tuning. | +| Todo | Update examples | `examples/codeguard.json`, `.codeguard/codeguard.yaml` if appropriate | `make codeguard-ci` | Enterprise-only checks may be too noisy for default example. | + +## Confidence policy + +- High confidence: mutable deployment refs, missing required CI gate, sensitive log fields, high-cardinality metric labels, infrastructure type leaked through public/domain APIs, mutable global state, unpinned provenance requirement. +- Medium confidence: shallow health checks, critical path without instrumentation, missing owner/runbook, domain logic in handler, pass-through abstraction, configuration leak. +- Low confidence: shallow module, temporal coupling, redundant comments, duplicated business knowledge without direct matched constants/rules. + +Findings should include safe metadata such as operation kind, logger/metric call kind, deployment reference type, owner source searched, runbook source searched, public surface count, pass-through ratio, and confidence evidence. Do not include secrets or raw source snippets. + +## Profile behavior target + +| Profile | Behavior | +| --- | --- | +| Startup | Warn only for clear mutable deployment refs, sensitive logs, and severe operability gaps. | +| Strict | Warn observability/design/delivery issues; block existing severe CI/security/supply-chain policy only. | +| Enterprise | Strict plus ownership, observability, rollout safety, provenance, runbooks, and service compatibility as hard or near-hard gates by threshold. | +| AI-safe | Strict plus unnecessary abstractions, duplicated knowledge, environment branching, weak operability metadata, and generated-code reviewability risks. | + +## Acceptance criteria + +- New config fields validate and round-trip in JSON/YAML. +- New rule metadata includes fix templates and explicit language coverage. +- Observability package can detect unstructured logs, error-without-context, sensitive log data, high-cardinality labels, critical path without instrumentation, log-and-ignore, and shallow health checks. +- Operations package can detect missing owners and runbooks for configured critical systems. +- Design extensions can detect at least infrastructure type leak, persistence model leak, domain logic in handler, pass-through abstraction, and excessive public surface. +- Delivery/CI extensions can detect mutable deployment refs, missing CI gates, missing rollback strategy, unsafe migration ordering, high-risk change without kill switch, missing post-deploy verification, missing provenance, and environment branching. +- Enterprise profile enables the intended checks without changing startup defaults aggressively. +- 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/checks +go test ./tests/codeguard ./tests/checks ./tests/cli -run 'Test.*(Observability|Operations|Delivery|Owner|Runbook|Provenance|PublicSurface|DomainLogic|InfrastructureLeak|Deployment|Profile|Metadata)' +go test ./tests/checks -run 'TestDesign|TestQuality|TestCI|TestSupplyChain|TestWriteReport' +``` + +Branch gate: + +```sh +make fmt-check +make test +make codeguard-ci +``` + +Pre-push/PR gate when practical: + +```sh +make ci +``` + +## Merge checklist + +- [ ] Rule IDs use existing prefix conventions where possible, especially `supply_chain.*`. +- [ ] Every built-in rule has a fix template. +- [ ] New config has defaults, validation, examples, and SDK aliases. +- [ ] Startup/strict defaults are not made unexpectedly noisy. +- [ ] Enterprise behavior is explicit and test-covered. +- [ ] Findings include actionable evidence and confidence. +- [ ] 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. diff --git a/docs/checks.md b/docs/checks.md index add786d..87cbbb1 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -33,6 +33,9 @@ This glossary is the quick map of every built-in check family and the main subse | 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 | | Change Safety | `Change Safety`, `Change Safety / Testability`, `Change Safety / Refactors` | `checks.change` | implemented diff-size and mixed-concern detectors; behavior changes without tests; failure-path coverage gaps; hardwired or nondeterministic domain dependencies; safe-refactor confidence checks; PR-summary rollups | +| Observability | `Observability` | `checks.observability` | unstructured logs; errors without operation/request context; sensitive log data; high-cardinality metric labels; critical paths without instrumentation; log-and-ignore failures; shallow health checks | +| Operations | `Operations` | `checks.operations` | missing service ownership; missing runbook metadata for critical production paths | +| Delivery | `Delivery` | `checks.delivery` | missing rollback evidence; unsafe migration sequencing; high-risk changes without kill switches; missing post-deploy verification | | 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 | @@ -71,6 +74,9 @@ Related report artifacts: "supply_chain": false, "reliability": false, "data": false, + "observability": false, + "operations": false, + "delivery": false, "change": false, "contracts": true, "context": true @@ -97,7 +103,7 @@ opt-in. after both the recommended baseline and explicit section enables are resolved. It is therefore the final precedence layer. Accepted names are `quality`, `performance`, `design`, `security`, `prompts`, `ci`, `supply_chain`, -`reliability`, `data`, `change`, `context`, and `contracts`; blank, duplicate, unknown, and alias names are +`reliability`, `data`, `observability`, `operations`, `delivery`, `change`, `context`, and `contracts`; blank, duplicate, unknown, and alias names are invalid. When `use_recommended_defaults` is absent or `false`, section behavior is @@ -115,6 +121,12 @@ baseline. `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. +`observability` covers production operability checks for Go, Python, TypeScript, JavaScript, and C++: structured logging evidence, contextual errors, sensitive log payloads, high-cardinality metric labels, critical-path instrumentation, log-and-ignore failures, and health-check depth. + +`operations` covers repository-level service readiness: ownership evidence and runbook metadata for critical production paths. + +`delivery` covers safe rollout checks: rollback evidence, expand/backfill/contract migration sequencing, feature-flag or kill-switch evidence for high-risk changes, and post-deploy health/smoke/SLO verification. + `change` covers diff-mode change safety, testability, and refactor-confidence checks. The startup profile leaves it off unless explicitly enabled; strict, enterprise, and AI-safe enable it through their profile defaults. It is designed for PR review and expects a diff/base revision for the strongest 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. @@ -331,6 +343,9 @@ the configuration tests. | `contracts` | scan-mode | scan-mode | true | true | scan-mode | | `reliability` | false | false | true | true | true | | `data` | false | false | false | true | true | +| `observability` | false | false | false | true | true | +| `operations` | false | false | false | true | false | +| `delivery` | false | false | false | true | true | | `change` | false | false | true | true | true | | `change_rules.max_changed_files` | 25 | 25 | 25 | 25 | 20 | | `change_rules.max_changed_directories` | 8 | 8 | 8 | 8 | 6 | @@ -955,6 +970,79 @@ Config keys: 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. +## Observability + +Purpose: +- Validate that production code is operable, not merely syntactically correct. +- Surface logging, metrics, instrumentation, and health-check gaps before they become incident-debugging gaps. + +Config keys: + +```json +{ + "checks": { + "observability": true, + "observability_rules": { + "detect_unstructured_log": true, + "detect_error_without_context": true, + "detect_sensitive_log_data": true, + "detect_high_cardinality_label": true, + "detect_critical_path_uninstrumented": true, + "detect_log_and_ignore": true, + "detect_shallow_health_check": true + } + } +} +``` + +Rules are implemented for Go, Python, TypeScript, JavaScript, and C++. They use confidence-based source evidence for structured logging, contextual errors, sensitive payloads, metric label cardinality, instrumentation on critical paths, logged-and-ignored failures, and shallow health/readiness endpoints. + +## Operations + +Purpose: +- Ensure critical services have ownership and runbook evidence. +- Make production responsibility visible to reviewers and agents. + +Config keys: + +```json +{ + "checks": { + "operations": true, + "operations_rules": { + "detect_missing_owner": true, + "detect_missing_runbook": true + } + } +} +``` + +Operations checks look for repository ownership files and runbook paths around critical production paths. Enterprise enables this family by default; AI-safe keeps it optional because ownership policy is often organization-specific. + +## Delivery + +Purpose: +- Catch rollout-safety gaps before production deployment. +- Surface migration ordering, rollback, kill-switch, and post-deploy verification risk. + +Config keys: + +```json +{ + "checks": { + "delivery": true, + "delivery_rules": { + "detect_missing_rollback_strategy": true, + "detect_unsafe_migration_order": true, + "detect_high_risk_change_without_kill_switch": true, + "detect_missing_post_deploy_verification": true + } + } +} +``` + +Delivery checks combine repository-wide deployment/migration evidence with source-path scanning for high-risk behavior. The kill-switch detector covers Go, Python, TypeScript, JavaScript, and C++; rollback, migration-order, and post-deploy checks operate over workflows, deployment files, release files, and migration paths. + ## Change Safety Purpose: @@ -1013,7 +1101,7 @@ Profile defaults: Current detector rollout: - Implemented `Change Safety` diff detectors: `change.oversized-diff`, `change.mixed-concerns`, `change.too-many-concerns`, `change.mixed-refactor-and-behavior`, `change.unnecessary-surface-area`, `change.one-use-abstraction`, `change.duplicate-helper`, `change.cleanup-regression`, `change.complexity-increased`, and `change.move-without-verification`. -- Implemented `Change Safety / Testability` detectors: `testing.behavior-change-without-test`, `testing.failure-path-missing`, `testing.hardwired-dependency`, and `testing.nondeterministic-domain-logic` for Go, Python, TypeScript, JavaScript, and C++ path/text evidence. `testing.legacy-hotspot-uncovered` is cataloged and configured, but intentionally skips when reliable history/hotspot inputs are unavailable. +- Implemented `Change Safety / Testability` detectors: `testing.behavior-change-without-test`, `testing.failure-path-missing`, `testing.hardwired-dependency`, `testing.nondeterministic-domain-logic`, and `testing.legacy-hotspot-uncovered` for Go, Python, TypeScript, JavaScript, and C++ path/text evidence. `testing.legacy-hotspot-uncovered` uses bounded local git history and skips when reliable history/hotspot inputs are unavailable. - Implemented `Change Safety / Refactors` detectors: the direct `refactor.*` family below has stable metadata, language coverage, fix templates, config toggles, and diff-mode safe-refactor detector tests. - Implemented local-quality support rules live in the `Code Quality` section: `naming.generic-identifier`, `function.excessive-parameters`, `function.mixed-abstraction-level`, `function.command-query-mix`, `error.logged-and-ignored`, `error.context-lost`, `defensive.unchecked-type-assertion`, `defensive.unsafe-numeric-conversion`, `maintainability.public-surface-growth`, and `maintainability.dependency-growth`. - Implemented history-aware maintainability/smell rules live in `Code Quality`-adjacent report sections and skip when git history is unavailable: `maintainability.hotspot`, `maintainability.high-churn-hotspot`, `maintainability.repeat-defect-area`, `maintainability.unstable-interface`, `maintainability.change-amplification`, `smell.shotgun-surgery-history`, and `smell.divergent-change-history`. diff --git a/docs/features.md b/docs/features.md index 23b1540..3c4d3a0 100644 --- a/docs/features.md +++ b/docs/features.md @@ -56,9 +56,18 @@ This page lists the current `codeguard` feature surface and the main config entr - `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 +- `observability` + - production operability checks for Go, Python, TypeScript, JavaScript, and C++ + - unstructured logs, errors without operation/request context, sensitive log data, high-cardinality metric labels, missing critical-path instrumentation, log-and-ignore failures, and shallow health checks +- `operations` + - ownership and runbook-readiness checks for critical production paths + - enterprise profile coverage for service ownership and operational handoff metadata +- `delivery` + - rollout-safety checks for workflows, deployment files, migrations, and high-risk source changes + - missing rollback strategies, unsafe migration ordering, high-risk behavior without feature flags or kill switches, and missing post-deploy verification - `change` - diff-mode change-safety, testability, and refactor-confidence checks for PR review - - implemented signals for oversized and mixed-concern diffs, too many concerns, mixed refactor/behavior diffs, broad public-surface edits, one-use abstractions, duplicate helpers, cleanup regressions, complexity increases, moves without verification, behavior changes without tests, failure-path coverage gaps, and hardwired or nondeterministic domain dependencies + - implemented signals for oversized and mixed-concern diffs, too many concerns, mixed refactor/behavior diffs, broad public-surface edits, one-use abstractions, duplicate helpers, cleanup regressions, complexity increases, moves without verification, behavior changes without tests, failure-path coverage gaps, legacy hotspots without characterization coverage, and hardwired or nondeterministic domain dependencies - implemented direct `refactor.*` IDs for behavior preservation checks, public-contract checks, error-path checks, side-effect ordering, visibility expansion, dependency direction, duplicate implementations left behind, and dead paths left behind - PR-summary signals for `change_safety`, `refactor_confidence`, and `maintainability_delta` when the change-summary postprocessor is available - `contracts` diff --git a/docs/production.md b/docs/production.md index eb88643..6a3f367 100644 --- a/docs/production.md +++ b/docs/production.md @@ -107,6 +107,7 @@ Use blocking failures for: - 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 +- sensitive data in logs - unsafe prompt or MCP config patterns - CI policy requirements @@ -115,6 +116,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 +- missing structured logging, contextual error logging, critical-path instrumentation, ownership, runbook, rollback, kill-switch, or post-deploy verification evidence - stability and reachability nudges - performance smells that still need human review @@ -143,7 +145,7 @@ For most teams: - pull requests: `codeguard scan -mode diff` - nightly or scheduled: `codeguard scan` -- release branches: `codeguard scan` plus reliability, data, contracts, and supply-chain enforcement +- release branches: `codeguard scan` plus reliability, data, observability, delivery, 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/internal/codeguard/checks/change/change_smells.go b/internal/codeguard/checks/change/change_smells.go index 42b2991..e2ba8e8 100644 --- a/internal/codeguard/checks/change/change_smells.go +++ b/internal/codeguard/checks/change/change_smells.go @@ -17,6 +17,8 @@ var ( goInterfaceDeclPattern = regexp.MustCompile(`^\s*type\s+([A-Za-z_][A-Za-z0-9_]*)\s+interface\b`) scriptInterfaceDeclPattern = regexp.MustCompile(`^\s*(?:export\s+)?interface\s+([A-Za-z_][A-Za-z0-9_]*)\b`) abstractClassDeclPattern = regexp.MustCompile(`^\s*(?:export\s+)?abstract\s+class\s+([A-Za-z_][A-Za-z0-9_]*)\b`) + scriptBoundaryClassPattern = regexp.MustCompile(`^\s*(?:export\s+)?class\s+([A-Za-z_][A-Za-z0-9_]*(?:Gateway|Provider|Port|Adapter|Boundary|Strategy))\b`) + pythonAbstractionDeclPattern = regexp.MustCompile(`^\s*class\s+([A-Za-z_][A-Za-z0-9_]*(?:Gateway|Provider|Port|Adapter|Boundary|Strategy|Protocol))\s*\((?:[^)]*(?:Protocol|ABC)[^)]*)\)\s*:`) cppAbstractClassDeclPattern = regexp.MustCompile(`^\s*class\s+([A-Za-z_][A-Za-z0-9_]*)\b`) functionDeclPattern = regexp.MustCompile(`^\s*(?:export\s+)?(?:async\s+)?(?:func|function|def)\s+([A-Za-z_][A-Za-z0-9_]*)\b|^\s*(?:export\s+)?(?:const|let|var)\s+([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(?:async\s*)?\([^)]*\)\s*=>`) cppFunctionDeclPattern = regexp.MustCompile(`^\s*(?:[A-Za-z_][A-Za-z0-9_:<>,*&\s]+)\s+([A-Za-z_][A-Za-z0-9_]*)\s*\([^;{}]*\)\s*(?:const\s*)?\{`) @@ -204,7 +206,14 @@ func abstractionNameForLine(rel string, line string) (string, bool) { if m := abstractClassDeclPattern.FindStringSubmatch(line); len(m) == 2 { return m[1], true } - case ".h", ".hpp", ".hh": + if m := scriptBoundaryClassPattern.FindStringSubmatch(line); len(m) == 2 { + return m[1], true + } + case ".py": + if m := pythonAbstractionDeclPattern.FindStringSubmatch(line); len(m) == 2 { + return m[1], true + } + case ".h", ".hpp", ".hh", ".cpp", ".cc", ".cxx": if m := cppAbstractClassDeclPattern.FindStringSubmatch(line); len(m) == 2 && strings.Contains(line, "virtual") { return m[1], true } diff --git a/internal/codeguard/checks/change/testability.go b/internal/codeguard/checks/change/testability.go index a19ec4a..3242717 100644 --- a/internal/codeguard/checks/change/testability.go +++ b/internal/codeguard/checks/change/testability.go @@ -4,13 +4,17 @@ package change import ( "context" "errors" + "fmt" "path/filepath" "regexp" "sort" + "strconv" "strings" + "time" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" "github.com/devr-tools/codeguard/internal/codeguard/core" + "github.com/devr-tools/codeguard/internal/codeguard/history" ) var ( @@ -21,6 +25,13 @@ var ( nondeterministicPattern = regexp.MustCompile(`\b(time\.now|date\.now|new\s+date\(|math\.random|rand\.|random\.|uuid\.|datetime\.(now|today)|time\.time|os\.getenv|process\.env|std::chrono::system_clock::now|std::random_device|std::getenv|getenv\()`) ) +const ( + legacyHotspotHistoryMaxCommits = 200 + legacyHotspotMinCommits = 4 + legacyHotspotMinChurn = 25 + legacyHotspotMinDefectCommits = 1 +) + type testabilityEvidence struct { path string line int @@ -56,6 +67,10 @@ func testabilityTargetFindings(ctx context.Context, env support.Context, target testFiles, testHasFailureEvidence := changedTestEvidence(ctx, env, target, changed) hasChangedTests := len(testFiles) > 0 + legacyHotspots := map[string]history.FileChangeMetrics{} + if enabled(env.Config.Checks.ChangeRules.DetectLegacyHotspotUncovered) && !hasChangedTests { + legacyHotspots = legacyHotspotMetrics(ctx, target) + } findings := make([]core.Finding, 0) for _, file := range changed { @@ -81,16 +96,50 @@ func testabilityTargetFindings(ctx context.Context, env support.Context, target }, })) } + if metric, ok := legacyHotspots[path]; ok { + findings = append(findings, legacyHotspotUncoveredFinding(env, path, firstChangedLine(diffScope[path]), metric)) + } } - // TODO(testing.legacy-hotspot-uncovered): emit only after the change section - // receives reliable per-file history/churn inputs. The current diff context - // can identify touched files, but cannot distinguish genuine legacy hotspots - // from ordinary modified code without risking misleading findings. - return findings } +func legacyHotspotMetrics(ctx context.Context, target core.TargetConfig) map[string]history.FileChangeMetrics { + historyCtx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + report, err := history.CollectChangeMetrics(historyCtx, history.ChangeMetricsOptions{ + RepoPath: target.Path, + MaxCommits: legacyHotspotHistoryMaxCommits, + }) + if err != nil || !report.Available { + return map[string]history.FileChangeMetrics{} + } + out := make(map[string]history.FileChangeMetrics) + for path, metric := range report.Files { + if metric.Commits >= legacyHotspotMinCommits && (metric.Churn >= legacyHotspotMinChurn || metric.DefectCommits >= legacyHotspotMinDefectCommits) { + out[filepath.ToSlash(path)] = metric + } + } + return out +} + +func legacyHotspotUncoveredFinding(env support.Context, path string, line int, metric history.FileChangeMetrics) core.Finding { + return env.NewFinding(support.FindingInput{ + RuleID: "testing.legacy-hotspot-uncovered", + Level: "warn", + Path: path, + Line: line, + Column: 1, + Message: fmt.Sprintf("touched legacy hotspot has no changed characterization or regression test evidence (%d commits, %d churn lines, %d defect-linked commits)", metric.Commits, metric.Churn, metric.DefectCommits), + Confidence: core.ConfidenceMedium, + Metadata: map[string]string{ + "commits": strconv.Itoa(metric.Commits), + "churn": strconv.Itoa(metric.Churn), + "defect_commits": strconv.Itoa(metric.DefectCommits), + }, + }) +} + func fileTestabilityEvidence(env support.Context, path string, data []byte, ranges core.ChangedLineRanges, hasChangedTests bool, testHasFailureEvidence bool) []testabilityEvidence { lines := strings.Split(string(data), "\n") out := make([]testabilityEvidence, 0, 4) diff --git a/internal/codeguard/checks/ci/ci.go b/internal/codeguard/checks/ci/ci.go index e1dfcbb..5a28ffa 100644 --- a/internal/codeguard/checks/ci/ci.go +++ b/internal/codeguard/checks/ci/ci.go @@ -24,6 +24,8 @@ func findingsForTarget(_ context.Context, env support.Context, target core.Targe findings = append(findings, requiredPathFindings(env, target, env.Config.Checks.CIRules.RequiredReleaseFiles, "required release file is missing")...) findings = append(findings, requiredPathFindings(env, target, env.Config.Checks.CIRules.RequiredAutomationPaths, "required automation path is missing")...) findings = append(findings, workflowContentFindings(env, target)...) + findings = append(findings, missingRequiredGateFindings(env, target)...) + findings = append(findings, mutableDeploymentReferenceFindings(env, target)...) findings = append(findings, testFileLocationFindings(env, target)...) findings = append(findings, testQualityFindings(env, target)...) return findings diff --git a/internal/codeguard/checks/ci/ci_delivery.go b/internal/codeguard/checks/ci/ci_delivery.go new file mode 100644 index 0000000..7e2cfcb --- /dev/null +++ b/internal/codeguard/checks/ci/ci_delivery.go @@ -0,0 +1,259 @@ +package ci + +import ( + "context" + "path/filepath" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + deploymentMarkerPattern = regexp.MustCompile(`(?i)\b(deploy|deployment|production|prod|release|rollout|kubectl|helm|goreleaser|terraform apply)\b`) + destructiveMigrationPattern = regexp.MustCompile(`(?i)\b(drop\s+(table|column|index)|alter\s+table.+drop|truncate\s+table|rename\s+column|set\s+not\s+null|delete\s+from)\b`) + migrationSafetyPattern = regexp.MustCompile(`(?i)\b(expand|contract|backfill|dual[-_\s]?write|concurrently|safe migration|two[-_\s]?phase|reversible|rollback|roll back|down\s*\(|down:)\b`) + highRiskBehaviorPattern = regexp.MustCompile(`(?i)\b(payment|checkout|billing|invoice|subscription|auth|authentication|authorization|migration|backfill|delete\s+from|drop\s+table|write|charge|refund)\b`) + sourceMutationPattern = regexp.MustCompile(`(?i)\b(save|insert|update|delete|charge|refund|create|write|migrate|backfill)\s*\(`) +) + +type fileSnapshot struct { + rel string + text string +} + +func RunDelivery(ctx context.Context, env support.Context) core.SectionResult { + return support.RunTargetSection(ctx, env, "delivery", "Delivery", deliveryFindingsForTarget) +} + +func deliveryFindingsForTarget(_ context.Context, env support.Context, target core.TargetConfig) []core.Finding { + cfg := env.Config.Checks.DeliveryRules + files := collectFiles(env, target) + if len(files) == 0 { + return nil + } + findings := make([]core.Finding, 0) + if enabled(cfg.DetectMissingRollbackStrategy) { + findings = append(findings, missingRollbackFindings(env, cfg, files)...) + } + if enabled(cfg.DetectUnsafeMigrationOrder) { + findings = append(findings, unsafeMigrationFindings(env, cfg, files)...) + } + if enabled(cfg.DetectHighRiskChangeWithoutKillSwitch) { + findings = append(findings, missingKillSwitchFindings(env, cfg, files)...) + } + if enabled(cfg.DetectMissingPostDeployVerification) { + findings = append(findings, missingPostDeployVerificationFindings(env, cfg, files)...) + } + return findings +} + +func missingRollbackFindings(env support.Context, cfg core.DeliveryRulesConfig, files []fileSnapshot) []core.Finding { + if hasAnyPattern(files, cfg.RollbackEvidencePatterns) { + return nil + } + findings := make([]core.Finding, 0) + for _, file := range files { + if !isDeploymentFile(file) && !hasDestructiveMigration(file.text) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "delivery.missing-rollback-strategy", + Level: "warn", + Path: file.rel, + Line: firstMarkerLine(file.text, deploymentMarkerPattern), + Column: 1, + Message: "deployment or destructive migration change has no rollback strategy evidence", + Confidence: core.ConfidenceMedium, + Metadata: map[string]string{ + "evidence": "deployment_or_migration", + }, + })) + } + return findings +} + +func unsafeMigrationFindings(env support.Context, cfg core.DeliveryRulesConfig, files []fileSnapshot) []core.Finding { + findings := make([]core.Finding, 0) + for _, file := range files { + if !isMigrationPath(cfg, file.rel) || !hasDestructiveMigration(file.text) || migrationSafetyPattern.MatchString(file.text) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "delivery.unsafe-migration-order", + Level: "warn", + Path: file.rel, + Line: firstMarkerLine(file.text, destructiveMigrationPattern), + Column: 1, + Message: "destructive migration lacks expand/backfill/contract or rollback sequencing evidence", + Confidence: core.ConfidenceMedium, + Metadata: map[string]string{ + "migration_risk": "destructive_change", + }, + })) + } + return findings +} + +func missingKillSwitchFindings(env support.Context, cfg core.DeliveryRulesConfig, files []fileSnapshot) []core.Finding { + if hasAnyPattern(files, cfg.KillSwitchPatterns) { + return nil + } + findings := make([]core.Finding, 0) + for _, file := range files { + if isBootstrapPath(cfg, file.rel) || !isSourcePath(file.rel) || !isHighRiskChange(cfg, file) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "delivery.high-risk-change-without-kill-switch", + Level: "warn", + Path: file.rel, + Line: firstMarkerLine(file.text, highRiskBehaviorPattern), + Column: 1, + Message: "high-risk production behavior has no feature flag or kill-switch evidence", + Confidence: core.ConfidenceMedium, + Metadata: map[string]string{ + "evidence": "critical_path_change", + }, + })) + } + return findings +} + +func missingPostDeployVerificationFindings(env support.Context, cfg core.DeliveryRulesConfig, files []fileSnapshot) []core.Finding { + findings := make([]core.Finding, 0) + for _, file := range files { + if !isDeploymentFile(file) || containsAnyFold(file.text, cfg.PostDeployVerificationPatterns) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "delivery.missing-post-deploy-verification", + Level: "warn", + Path: file.rel, + Line: firstMarkerLine(file.text, deploymentMarkerPattern), + Column: 1, + Message: "deployment workflow lacks post-deploy smoke, health, or SLO verification evidence", + Confidence: core.ConfidenceMedium, + Metadata: map[string]string{ + "verification": "missing", + }, + })) + } + return findings +} + +func collectFiles(env support.Context, target core.TargetConfig) []fileSnapshot { + files := make([]fileSnapshot, 0) + if env.VisitTargetFiles == nil { + return files + } + env.VisitTargetFiles(target, func(rel string) bool { + return isPotentialDeliveryPath(env.Config.Checks.DeliveryRules, rel) + }, func(rel string, data []byte) { + files = append(files, fileSnapshot{rel: filepath.ToSlash(rel), text: string(data)}) + }) + return files +} + +func isPotentialDeliveryPath(cfg core.DeliveryRulesConfig, rel string) bool { + normalized := strings.ToLower(filepath.ToSlash(rel)) + return strings.HasPrefix(normalized, ".github/workflows/") || + strings.Contains(normalized, "deploy") || + strings.Contains(normalized, "release") || + isMigrationPath(cfg, rel) || + isSourcePath(rel) +} + +func isDeploymentFile(file fileSnapshot) bool { + path := strings.ToLower(filepath.ToSlash(file.rel)) + if strings.HasPrefix(path, ".github/workflows/") || strings.Contains(path, "deploy") || strings.Contains(path, "release") { + return deploymentMarkerPattern.MatchString(file.text) + } + return false +} + +func isMigrationPath(cfg core.DeliveryRulesConfig, rel string) bool { + normalized := strings.ToLower(filepath.ToSlash(rel)) + for _, pattern := range cfg.MigrationPathPatterns { + if support.PathMatchesPattern(pattern, normalized) { + return true + } + } + return strings.Contains(normalized, "migrations/") || strings.Contains(normalized, "db/migrate/") || strings.Contains(normalized, "alembic/") +} + +func isHighRiskChange(cfg core.DeliveryRulesConfig, file fileSnapshot) bool { + for _, pattern := range cfg.HighRiskPathPatterns { + if support.PathMatchesPattern(pattern, strings.ToLower(filepath.ToSlash(file.rel))) { + return highRiskBehaviorPattern.MatchString(file.text) + } + } + return highRiskBehaviorPattern.MatchString(file.text) && sourceMutationPattern.MatchString(file.text) +} + +func isBootstrapPath(cfg core.DeliveryRulesConfig, rel string) bool { + normalized := strings.ToLower(filepath.ToSlash(rel)) + for _, pattern := range cfg.BootstrapPathPatterns { + if support.PathMatchesPattern(pattern, normalized) { + return true + } + } + return strings.HasPrefix(normalized, "cmd/") || + strings.Contains(normalized, "/config/") || + strings.Contains(normalized, "/bootstrap/") || + strings.HasPrefix(normalized, "scripts/") +} + +func isSourcePath(rel string) bool { + lowered := strings.ToLower(rel) + return strings.HasSuffix(lowered, ".go") || + strings.HasSuffix(lowered, ".py") || + strings.HasSuffix(lowered, ".ts") || + strings.HasSuffix(lowered, ".tsx") || + strings.HasSuffix(lowered, ".js") || + strings.HasSuffix(lowered, ".jsx") || + strings.HasSuffix(lowered, ".cpp") || + strings.HasSuffix(lowered, ".cc") || + strings.HasSuffix(lowered, ".cxx") || + strings.HasSuffix(lowered, ".hpp") || + strings.HasSuffix(lowered, ".hh") || + strings.HasSuffix(lowered, ".h") +} + +func hasDestructiveMigration(text string) bool { + return destructiveMigrationPattern.MatchString(text) +} + +func hasAnyPattern(files []fileSnapshot, patterns []string) bool { + for _, file := range files { + if containsAnyFold(file.text, patterns) { + return true + } + } + return false +} + +func containsAnyFold(text string, patterns []string) bool { + lowered := strings.ToLower(text) + for _, pattern := range patterns { + if strings.Contains(lowered, strings.ToLower(strings.TrimSpace(pattern))) { + return true + } + } + return false +} + +func firstMarkerLine(text string, pattern *regexp.Regexp) int { + lines := strings.Split(text, "\n") + for idx, line := range lines { + if pattern.MatchString(line) { + return idx + 1 + } + } + return 1 +} + +func enabled(value *bool) bool { + return value == nil || *value +} diff --git a/internal/codeguard/checks/ci/ci_delivery_safety.go b/internal/codeguard/checks/ci/ci_delivery_safety.go new file mode 100644 index 0000000..0651362 --- /dev/null +++ b/internal/codeguard/checks/ci/ci_delivery_safety.go @@ -0,0 +1,217 @@ +package ci + +import ( + "fmt" + "path/filepath" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + workflowUsesPattern = regexp.MustCompile(`(?i)\buses:\s*['"]?([^@\s'"]+)(?:@([^\s#'"]+))?`) + latestImagePattern = regexp.MustCompile(`(?i)\b(?:image:|from)\s+['"]?[^@\s'"]+:latest\b`) + deployImageRunPattern = regexp.MustCompile(`(?i)\b(?:docker|kubectl|helm)\b.*:latest\b`) +) + +func missingRequiredGateFindings(env support.Context, target core.TargetConfig) []core.Finding { + gates := normalizedNonEmpty(env.Config.Checks.CIRules.RequiredGates) + if len(gates) == 0 { + return nil + } + workflows := workflowFiles(env, target) + if len(workflows) == 0 { + return nil + } + allContent := strings.Builder{} + for _, file := range workflows { + allContent.WriteString("\n") + allContent.WriteString(strings.ToLower(string(file.data))) + } + content := allContent.String() + findings := make([]core.Finding, 0) + for _, gate := range gates { + if workflowHasGate(content, gate) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "ci.missing-required-gate", + Level: "fail", + Path: ".github/workflows", + Line: 1, + Column: 1, + Message: fmt.Sprintf("required CI gate %q is missing from configured workflows", gate), + Confidence: core.ConfidenceHigh, + Metadata: map[string]string{ + "gate": gate, + }, + })) + } + return findings +} + +func workflowHasGate(content string, gate string) bool { + gate = strings.ToLower(strings.TrimSpace(gate)) + if gate == "" { + return true + } + switch gate { + case "test", "tests": + return strings.Contains(content, "go test") || + strings.Contains(content, "npm test") || + strings.Contains(content, "pnpm test") || + strings.Contains(content, "yarn test") || + strings.Contains(content, "pytest") || + strings.Contains(content, "cargo test") || + strings.Contains(content, "dotnet test") || + strings.Contains(content, "mvn test") || + strings.Contains(content, "gradle test") || + strings.Contains(content, "\n test:") || + strings.Contains(content, "\n name: test") + case "security": + return strings.Contains(content, "govulncheck") || + strings.Contains(content, "gosec") || + strings.Contains(content, "codeql") || + strings.Contains(content, "trivy") || + strings.Contains(content, "snyk") || + strings.Contains(content, "semgrep") + default: + return strings.Contains(content, gate) + } +} + +func mutableDeploymentReferenceFindings(env support.Context, target core.TargetConfig) []core.Finding { + files := deliveryReferenceFiles(env, target) + findings := make([]core.Finding, 0) + for _, file := range files { + lines := strings.Split(string(file.data), "\n") + for idx, line := range lines { + if finding, ok := mutableActionRefFinding(env, file.rel, idx+1, line); ok { + findings = append(findings, finding) + } + if finding, ok := latestImageFinding(env, file.rel, idx+1, line); ok { + findings = append(findings, finding) + } + } + } + return findings +} + +func mutableActionRefFinding(env support.Context, rel string, lineNo int, line string) (core.Finding, bool) { + match := workflowUsesPattern.FindStringSubmatch(line) + if len(match) == 0 { + return core.Finding{}, false + } + action := match[1] + ref := "" + if len(match) > 2 { + ref = strings.TrimSpace(match[2]) + } + if strings.HasPrefix(action, "./") || strings.HasPrefix(strings.ToLower(action), "docker://") || action == "" { + return core.Finding{}, false + } + if ref != "" && !isMutableActionRef(ref) { + return core.Finding{}, false + } + message := "external GitHub Action uses a mutable or missing ref" + if ref != "" { + message = fmt.Sprintf("external GitHub Action ref %q is mutable", ref) + } + return env.NewFinding(support.FindingInput{ + RuleID: "ci.mutable-deployment-reference", + Level: "fail", + Path: rel, + Line: lineNo, + Column: 1, + Message: message, + Confidence: core.ConfidenceHigh, + Metadata: map[string]string{ + "reference_type": "github_action", + }, + }), true +} + +func latestImageFinding(env support.Context, rel string, lineNo int, line string) (core.Finding, bool) { + if !latestImagePattern.MatchString(line) && !deployImageRunPattern.MatchString(line) { + return core.Finding{}, false + } + return env.NewFinding(support.FindingInput{ + RuleID: "ci.mutable-deployment-reference", + Level: "fail", + Path: rel, + Line: lineNo, + Column: 1, + Message: "container image reference uses the mutable latest tag", + Confidence: core.ConfidenceHigh, + Metadata: map[string]string{ + "reference_type": "container_image", + }, + }), true +} + +func isMutableActionRef(ref string) bool { + normalized := strings.ToLower(strings.TrimSpace(ref)) + switch normalized { + case "", "head", "latest", "main", "master", "develop", "development", "dev", "trunk", "stable": + return true + } + return strings.HasPrefix(normalized, "refs/heads/") +} + +type ciFile struct { + rel string + data []byte +} + +func workflowFiles(env support.Context, target core.TargetConfig) []ciFile { + return collectCIFiles(env, target, func(rel string) bool { + normalized := strings.ToLower(filepath.ToSlash(rel)) + return strings.HasPrefix(normalized, ".github/workflows/") && + (strings.HasSuffix(normalized, ".yml") || strings.HasSuffix(normalized, ".yaml")) + }) +} + +func deliveryReferenceFiles(env support.Context, target core.TargetConfig) []ciFile { + return collectCIFiles(env, target, func(rel string) bool { + normalized := strings.ToLower(filepath.ToSlash(rel)) + base := strings.ToLower(filepath.Base(normalized)) + return strings.HasPrefix(normalized, ".github/workflows/") || + strings.HasPrefix(normalized, "deploy/") || + strings.HasPrefix(normalized, "deployment/") || + strings.HasPrefix(normalized, "k8s/") || + strings.HasPrefix(normalized, "kubernetes/") || + strings.Contains(normalized, "/deploy/") || + strings.Contains(normalized, "/deployment/") || + strings.HasPrefix(base, "dockerfile") + }) +} + +func collectCIFiles(env support.Context, target core.TargetConfig, include func(string) bool) []ciFile { + files := make([]ciFile, 0) + if env.VisitTargetFiles != nil { + env.VisitTargetFiles(target, include, func(rel string, data []byte) { + files = append(files, ciFile{rel: filepath.ToSlash(rel), data: append([]byte(nil), data...)}) + }) + return files + } + return files +} + +func normalizedNonEmpty(values []string) []string { + out := make([]string, 0, len(values)) + seen := map[string]struct{}{} + for _, value := range values { + normalized := strings.ToLower(strings.TrimSpace(value)) + if normalized == "" { + continue + } + if _, ok := seen[normalized]; ok { + continue + } + seen[normalized] = struct{}{} + out = append(out, normalized) + } + return out +} diff --git a/internal/codeguard/checks/data/data_cpp.go b/internal/codeguard/checks/data/data_cpp.go index f6987f6..6916462 100644 --- a/internal/codeguard/checks/data/data_cpp.go +++ b/internal/codeguard/checks/data/data_cpp.go @@ -39,6 +39,7 @@ type cppDataScan struct { env support.Context file string rules core.DataRulesConfig + readLines []int writeLines []int publishLines []int consumerLine int @@ -61,6 +62,9 @@ func (s *cppDataScan) consumeLine(lineNo int, line string) { if cppDataWrite.MatchString(line) { s.writeLines = append(s.writeLines, lineNo) } + if cppDataRead.MatchString(line) { + s.readLines = append(s.readLines, lineNo) + } if cppDataPublish.MatchString(line) { s.publishLines = append(s.publishLines, lineNo) } @@ -90,9 +94,15 @@ func (s *cppDataScan) consumeRawSource(source string) { } func (s *cppDataScan) finish() { + if enabled(s.rules.DetectReadModifyWriteRace) && len(s.readLines) > 0 && len(s.writeLines) > 0 && !s.hasTx { + s.add("data.read-modify-write-race", "fail", s.readLines[0], "C++ code reads state and writes derived state without transaction or atomic update evidence", "medium", "pattern", "read-modify-write") + } 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 enabled(s.rules.DetectSideEffectInTransaction) && s.hasTx && len(s.publishLines) > 0 && !s.hasOutbox { + s.add("data.side-effect-in-transaction", "fail", s.publishLines[0], "C++ transaction block performs an external side effect that may not roll back safely", "high", "transaction", "side-effect") + } 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") diff --git a/internal/codeguard/checks/data/data_python.go b/internal/codeguard/checks/data/data_python.go index 4ef4094..51e3f4f 100644 --- a/internal/codeguard/checks/data/data_python.go +++ b/internal/codeguard/checks/data/data_python.go @@ -41,6 +41,7 @@ type pythonDataScan struct { env support.Context file string rules core.DataRulesConfig + readLines []int writeLines []int publishLines []int consumerLine int @@ -64,6 +65,9 @@ func (s *pythonDataScan) consumeLine(lineNo int, line string) { if pyWriteCall.MatchString(line) { s.writeLines = append(s.writeLines, lineNo) } + if pySelectQuery.MatchString(line) { + s.readLines = append(s.readLines, lineNo) + } if pyPublishCall.MatchString(line) { s.publishLines = append(s.publishLines, lineNo) } @@ -85,9 +89,15 @@ func (s *pythonDataScan) consumeLine(lineNo int, line string) { } func (s *pythonDataScan) finish() { + if enabled(s.rules.DetectReadModifyWriteRace) && len(s.readLines) > 0 && len(s.writeLines) > 0 && !s.hasTx { + s.add("data.read-modify-write-race", "fail", s.readLines[0], "Python code reads state and writes derived state without transaction or atomic update evidence", "medium", "pattern", "read-modify-write") + } 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 enabled(s.rules.DetectSideEffectInTransaction) && s.hasTx && len(s.publishLines) > 0 && !s.hasOutbox { + s.add("data.side-effect-in-transaction", "fail", s.publishLines[0], "Python transaction block performs an external side effect that may not roll back safely", "high", "transaction", "side-effect") + } 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") diff --git a/internal/codeguard/checks/data/data_typescript.go b/internal/codeguard/checks/data/data_typescript.go index 75d5e2f..f40b296 100644 --- a/internal/codeguard/checks/data/data_typescript.go +++ b/internal/codeguard/checks/data/data_typescript.go @@ -48,6 +48,7 @@ type tsDataScan struct { env support.Context file string rules core.DataRulesConfig + readLines []int writeLines []int publishLines []int consumerLine int @@ -70,6 +71,9 @@ func (s *tsDataScan) consumeLine(lineNo int, line string) { if tsDataWrite.MatchString(line) { s.writeLines = append(s.writeLines, lineNo) } + if tsDataRead.MatchString(line) { + s.readLines = append(s.readLines, lineNo) + } if tsDataPublish.MatchString(line) { s.publishLines = append(s.publishLines, lineNo) } @@ -99,9 +103,15 @@ func (s *tsDataScan) consumeRawSource(source string) { } func (s *tsDataScan) finish() { + if enabled(s.rules.DetectReadModifyWriteRace) && len(s.readLines) > 0 && len(s.writeLines) > 0 && !s.hasTx { + s.add("data.read-modify-write-race", "fail", s.readLines[0], "TypeScript/JavaScript code reads state and writes derived state without transaction or atomic update evidence", "medium", "pattern", "read-modify-write") + } 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 enabled(s.rules.DetectSideEffectInTransaction) && s.hasTx && len(s.publishLines) > 0 && !s.hasOutbox { + s.add("data.side-effect-in-transaction", "fail", s.publishLines[0], "TypeScript/JavaScript transaction block performs an external side effect that may not roll back safely", "high", "transaction", "side-effect") + } 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") diff --git a/internal/codeguard/checks/design/design.go b/internal/codeguard/checks/design/design.go index 3353ca4..69664d7 100644 --- a/internal/codeguard/checks/design/design.go +++ b/internal/codeguard/checks/design/design.go @@ -22,6 +22,7 @@ func Run(ctx context.Context, env support.Context) core.SectionResult { graphs = append(graphs, targetModuleGraph{target: target, graph: graph}) } findings = append(findings, commandFindings(ctx, env, target)...) + findings = append(findings, localAbstractionFindings(env, target)...) } findings = append(findings, changeImpactFindings(env, graphs)...) return env.FinalizeSection("design", "Design Patterns", findings) diff --git a/internal/codeguard/checks/design/local_abstraction.go b/internal/codeguard/checks/design/local_abstraction.go new file mode 100644 index 0000000..2e29b69 --- /dev/null +++ b/internal/codeguard/checks/design/local_abstraction.go @@ -0,0 +1,438 @@ +package design + +import ( + "bytes" + "fmt" + "go/ast" + "go/parser" + "go/printer" + "go/token" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +const ( + ruleShallowModule = "design.shallow-module" + ruleExcessivePublicSurface = "design.excessive-public-surface" + rulePassThrough = "design.pass-through-abstraction" // #nosec G101 -- rule id, not a credential. + ruleConfigurationLeak = "design.configuration-leak" + ruleTemporalCoupling = "design.temporal-coupling" + ruleInfrastructureLeak = "design.infrastructure-type-leak" + rulePersistenceLeak = "design.persistence-model-leak" + ruleDomainLogicInHandler = "design.domain-logic-in-handler" +) + +var ( + tsExportPattern = regexp.MustCompile(`(?m)^\s*export\s+(?:async\s+)?(?:class|interface|type|enum|function|const|let|var)\s+([A-Za-z_$][\w$]*)\b`) + pythonPublicPattern = regexp.MustCompile(`(?m)^\s*(?:class|def)\s+([A-Za-z]\w*)\b`) + cppPublicPattern = regexp.MustCompile(`(?m)^\s*(?:class|struct)\s+([A-Z]\w*)\b|^\s*(?:[A-Za-z_][\w:<>,\s*&~]*\s+)+([A-Z]\w*)\s*\([^;{}]*\)\s*;`) + delegationCallPattern = regexp.MustCompile(`(?i)\b(delegate|client|inner|wrapped|service|repo|repository|store|gateway|adapter|api|impl)\s*(?:\.|->|::)\s*[A-Za-z_]\w*\s*\(`) + infraLeakPattern = regexp.MustCompile(`(?i)\b(sql\.|http\.Request|http\.ResponseWriter|gin\.Context|echo\.Context|fiber\.Ctx|gorm\.DB|redis\.Client|kafka\.|sqs\.|sns\.|boto3|requests\.|express\.Request|Request<|Response<|PDO|mysqli|std::istream|std::ostream)\b`) + persistenceLeakPattern = regexp.MustCompile( + `(?i)\b(db|dao|dto|entity|record|row|orm|model)\b|[A-Za-z_]*(DTO|Entity|Model|Record|Row)\b|gorm:|sequelize|typeorm|sqlalchemy|django\.db|ActiveRecord|@Entity|Prisma\.`, + ) + configLeakPattern = regexp.MustCompile(`(?i)\b(os\.Getenv|process\.env|getenv\(|System\.getenv|Config\b|Settings\b|Options\b|FeatureFlag|ENV\[)`) + domainTermPattern = regexp.MustCompile(`(?i)\b(discount|price|amount|currency|inventory|permission|role|status|quota|eligib|payment|order|invoice|customer|account)\b`) + mutationPattern = regexp.MustCompile(`(?i)\b(save|insert|update|delete|charge|refund|publish|emit|commit|execute|query)\b`) + temporalSetupPattern = regexp.MustCompile(`(?i)\b(init|initialize|configure|connect|open|set[A-Z_]|begin|prepare)\b`) + temporalActionPattern = regexp.MustCompile(`(?i)\b(start|run|execute|send|publish|commit|close|flush|use)\b`) +) + +type designFunction struct { + Name string + StartLine int + Statements []support.ParsedStatement + Calls []support.ParsedCall +} + +type publicSymbol struct { + Name string + Line int +} + +func localAbstractionFindings(env support.Context, target core.TargetConfig) []core.Finding { + return env.ScanTargetFiles(target, "design", func(rel string) bool { + return localDesignSupportsFile(target.Language, rel) + }, func(file string, data []byte) []core.Finding { + return localDesignFileFindings(env, target, file, data) + }) +} + +func localDesignSupportsFile(language string, rel string) bool { + switch support.NormalizedLanguage(language) { + case "", "go": + return strings.HasSuffix(rel, ".go") + case "python", "py": + return strings.HasSuffix(rel, ".py") + case "typescript", "javascript", "ts", "tsx", "js", "jsx": + return isTypeScriptLikeFile(rel) + case "c++", "cpp", "cxx", "cc": + return strings.HasSuffix(rel, ".cpp") || strings.HasSuffix(rel, ".cc") || strings.HasSuffix(rel, ".cxx") || + strings.HasSuffix(rel, ".hpp") || strings.HasSuffix(rel, ".hh") || strings.HasSuffix(rel, ".h") + default: + return false + } +} + +func localDesignFileFindings(env support.Context, target core.TargetConfig, file string, data []byte) []core.Finding { + source := strings.ReplaceAll(string(data), "\r\n", "\n") + symbols := publicSymbols(target.Language, file, source) + functions := designFunctions(env, target.Language, file, data) + findings := make([]core.Finding, 0, len(functions)+5) + findings = append(findings, localPublicSurfaceFindings(env, file, symbols, functions, source)...) + findings = append(findings, leakFindings(env, file, source)...) + for _, fn := range functions { + findings = append(findings, functionAbstractionFindings(env, file, fn)...) + } + return findings +} + +func localPublicSurfaceFindings(env support.Context, file string, symbols []publicSymbol, functions []designFunction, source string) []core.Finding { + findings := make([]core.Finding, 0, 2) + maxPublic := max(1, env.Config.Checks.DesignRules.MaxDeclsPerFile) + if len(symbols) > maxPublic { + findings = append(findings, designFinding(env, ruleExcessivePublicSurface, file, 1, + fmt.Sprintf("file exposes %d public symbols; max is %d", len(symbols), maxPublic), core.ConfidenceHigh)) + } + shallowThreshold := max(2, env.Config.Checks.DesignRules.MaxInterfaceMethods) + if len(symbols) >= shallowThreshold && averageFunctionStatements(functions) <= 1 && exportedWrapperDensity(source) >= 2 { + findings = append(findings, designFinding(env, ruleShallowModule, file, 1, + fmt.Sprintf("module exposes %d public symbols but most behavior is shallow delegation or declarations", len(symbols)), core.ConfidenceLow)) + } + return findings +} + +func leakFindings(env support.Context, file string, source string) []core.Finding { + lines := strings.Split(source, "\n") + findings := make([]core.Finding, 0, 3) + domainPath := isDomainPath(file) + apiPath := isAPIPath(file) + handlerPath := isHandlerPath(file) + for idx, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "#") { + continue + } + lineNo := idx + 1 + if domainPath && infraLeakPattern.MatchString(trimmed) { + findings = append(findings, designFinding(env, ruleInfrastructureLeak, file, lineNo, + "infrastructure/framework type leaks into a domain or public boundary", core.ConfidenceHigh)) + } + if (apiPath || handlerPath || isPublicDeclaration(trimmed)) && persistenceLeakPattern.MatchString(trimmed) { + findings = append(findings, designFinding(env, rulePersistenceLeak, file, lineNo, + "persistence model or ORM concept leaks through a public/API boundary", core.ConfidenceHigh)) + } + if domainPath && configLeakPattern.MatchString(trimmed) { + findings = append(findings, designFinding(env, ruleConfigurationLeak, file, lineNo, + "configuration or environment concern leaks into domain code", core.ConfidenceMedium)) + } + } + if handlerPath { + findings = append(findings, domainLogicHandlerFinding(env, file, lines)...) + } + return firstFindingPerRule(findings) +} + +func domainLogicHandlerFinding(env support.Context, file string, lines []string) []core.Finding { + score := 0 + lineNo := 1 + for idx, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "if ") || strings.Contains(trimmed, " if ") || strings.Contains(trimmed, " switch ") || strings.Contains(trimmed, " for ") { + if domainTermPattern.MatchString(trimmed) { + score += 2 + lineNo = idx + 1 + } + } + if domainTermPattern.MatchString(trimmed) && mutationPattern.MatchString(trimmed) { + score++ + lineNo = idx + 1 + } + } + if score < domainLogicStatementLimit(env) { + return nil + } + return []core.Finding{designFinding(env, ruleDomainLogicInHandler, file, lineNo, + "handler/controller contains business-rule branching and mutation instead of delegating to domain services", core.ConfidenceMedium)} +} + +func functionAbstractionFindings(env support.Context, file string, fn designFunction) []core.Finding { + findings := make([]core.Finding, 0, 2) + if isPassThroughFunction(fn) { + findings = append(findings, designFinding(env, rulePassThrough, file, fn.StartLine, + fmt.Sprintf("function %s mostly passes through to another dependency without policy, validation, or translation", fn.Name), core.ConfidenceMedium)) + } + if hasTemporalCoupling(fn) { + findings = append(findings, designFinding(env, ruleTemporalCoupling, file, fn.StartLine, + fmt.Sprintf("function %s relies on an implicit setup-before-action call order", fn.Name), core.ConfidenceLow)) + } + return findings +} + +func isPassThroughFunction(fn designFunction) bool { + if len(nonEmptyStatements(fn.Statements)) > 2 { + return false + } + for _, stmt := range fn.Statements { + line := strings.TrimSpace(stmt.Text) + if strings.HasPrefix(line, "return ") && delegationCallPattern.MatchString(line) { + return true + } + if delegationCallPattern.MatchString(line) && !strings.Contains(line, " if ") && !strings.Contains(line, "for ") { + return true + } + } + return false +} + +func domainLogicStatementLimit(env support.Context) int { + threshold := env.Config.Checks.DesignRules.MaxInterfaceMethods / 2 + return max(2, threshold) +} + +func hasTemporalCoupling(fn designFunction) bool { + seenSetup := false + for _, call := range fn.Calls { + if temporalSetupPattern.MatchString(call.Callee) { + seenSetup = true + continue + } + if seenSetup && temporalActionPattern.MatchString(call.Callee) { + return true + } + } + return false +} + +func designFunctions(env support.Context, language string, file string, data []byte) []designFunction { + switch support.NormalizedLanguage(language) { + case "", "go": + return goDesignFunctions(env, file, data) + case "python", "py": + return parsedDesignFunctions(support.ParsePython(string(data))) + case "typescript", "javascript", "ts", "tsx", "js", "jsx": + return parsedDesignFunctions(support.ParseCLike(string(data), support.CLikeTypeScript)) + case "c++", "cpp", "cxx", "cc": + return parsedDesignFunctions(support.ParseCLike(string(data), support.CLikeCPP)) + default: + return nil + } +} + +func goDesignFunctions(env support.Context, file string, data []byte) []designFunction { + fset, parsed, err := support.ParseGoSource(env, file, data) + if err != nil { + return nil + } + functions := make([]designFunction, 0) + for _, decl := range parsed.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil { + continue + } + functions = append(functions, goDesignFunction(fset, fn, data)) + } + return functions +} + +func goDesignFunction(fset *token.FileSet, fn *ast.FuncDecl, data []byte) designFunction { + out := designFunction{Name: fn.Name.Name, StartLine: fset.Position(fn.Pos()).Line} + start := fset.Position(fn.Body.Lbrace).Offset + end := fset.Position(fn.Body.Rbrace).Offset + if start >= 0 && end > start && end <= len(data) { + for idx, line := range strings.Split(string(data[start+1:end]), "\n") { + if strings.TrimSpace(line) != "" { + out.Statements = append(out.Statements, support.ParsedStatement{Line: fset.Position(fn.Body.Lbrace).Line + idx, Text: line, Raw: line}) + } + } + } + ast.Inspect(fn.Body, func(n ast.Node) bool { + if call, ok := n.(*ast.CallExpr); ok { + out.Calls = append(out.Calls, support.ParsedCall{Callee: goDesignCallName(call.Fun), Line: fset.Position(call.Pos()).Line}) + } + return true + }) + return out +} + +func goDesignCallName(expr ast.Expr) string { + switch value := expr.(type) { + case *ast.Ident: + return value.Name + case *ast.SelectorExpr: + prefix := goDesignCallName(value.X) + if prefix == "" { + return value.Sel.Name + } + return prefix + "." + value.Sel.Name + default: + var buf bytes.Buffer + _ = printer.Fprint(&buf, token.NewFileSet(), expr) + return buf.String() + } +} + +func parsedDesignFunctions(parsed *support.ParsedFile) []designFunction { + functions := parsed.AllFunctions() + out := make([]designFunction, 0, len(functions)) + for _, fn := range functions { + out = append(out, designFunction{Name: fn.Name, StartLine: fn.StartLine, Statements: fn.Statements, Calls: fn.Calls}) + } + return out +} + +func publicSymbols(language string, file string, source string) []publicSymbol { + switch support.NormalizedLanguage(language) { + case "", "go": + return goPublicSymbols(source) + case "python", "py": + return regexPublicSymbols(source, pythonPublicPattern) + case "typescript", "javascript", "ts", "tsx", "js", "jsx": + return regexPublicSymbols(source, tsExportPattern) + case "c++", "cpp", "cxx", "cc": + if !strings.HasSuffix(file, ".h") && !strings.HasSuffix(file, ".hh") && !strings.HasSuffix(file, ".hpp") { + return nil + } + return regexPublicSymbols(source, cppPublicPattern) + default: + return nil + } +} + +func goPublicSymbols(source string) []publicSymbol { + fset := token.NewFileSet() + parsed, err := parser.ParseFile(fset, "source.go", source, 0) + if err != nil { + return nil + } + symbols := make([]publicSymbol, 0) + for _, decl := range parsed.Decls { + switch node := decl.(type) { + case *ast.FuncDecl: + if node.Name.IsExported() { + symbols = append(symbols, publicSymbol{Name: node.Name.Name, Line: fset.Position(node.Pos()).Line}) + } + case *ast.GenDecl: + for _, spec := range node.Specs { + switch spec := spec.(type) { + case *ast.TypeSpec: + if spec.Name.IsExported() { + symbols = append(symbols, publicSymbol{Name: spec.Name.Name, Line: fset.Position(spec.Pos()).Line}) + } + case *ast.ValueSpec: + for _, name := range spec.Names { + if name.IsExported() { + symbols = append(symbols, publicSymbol{Name: name.Name, Line: fset.Position(name.Pos()).Line}) + } + } + } + } + } + } + return symbols +} + +func regexPublicSymbols(source string, pattern *regexp.Regexp) []publicSymbol { + matches := pattern.FindAllStringSubmatchIndex(source, -1) + symbols := make([]publicSymbol, 0, len(matches)) + for _, match := range matches { + name := "" + for idx := 2; idx+1 < len(match); idx += 2 { + if match[idx] >= 0 { + name = source[match[idx]:match[idx+1]] + break + } + } + if name != "" && !strings.HasPrefix(name, "_") { + symbols = append(symbols, publicSymbol{Name: name, Line: support.LineNumberForOffset(source, match[0])}) + } + } + return symbols +} + +func exportedWrapperDensity(source string) int { + count := 0 + for _, line := range strings.Split(source, "\n") { + if strings.Contains(line, "return ") && delegationCallPattern.MatchString(line) { + count++ + } + } + return count +} + +func averageFunctionStatements(functions []designFunction) int { + if len(functions) == 0 { + return 0 + } + total := 0 + for _, fn := range functions { + total += len(nonEmptyStatements(fn.Statements)) + } + return total / len(functions) +} + +func nonEmptyStatements(statements []support.ParsedStatement) []support.ParsedStatement { + out := make([]support.ParsedStatement, 0, len(statements)) + for _, statement := range statements { + if strings.TrimSpace(statement.Text) != "" { + out = append(out, statement) + } + } + return out +} + +func isDomainPath(file string) bool { + normalized := strings.ToLower(filepathSlash(file)) + return strings.Contains(normalized, "/domain/") || strings.Contains(normalized, "/core/") || + strings.Contains(normalized, "/model/") || strings.Contains(normalized, "/models/") +} + +func isAPIPath(file string) bool { + normalized := strings.ToLower(filepathSlash(file)) + return strings.Contains(normalized, "/api/") || strings.Contains(normalized, "/contract/") || + strings.Contains(normalized, "/contracts/") +} + +func isHandlerPath(file string) bool { + normalized := strings.ToLower(filepathSlash(file)) + return strings.Contains(normalized, "handler") || strings.Contains(normalized, "controller") || + strings.Contains(normalized, "/routes/") || strings.Contains(normalized, "/views/") +} + +func filepathSlash(path string) string { + return strings.ReplaceAll(path, "\\", "/") +} + +func isPublicDeclaration(line string) bool { + return strings.HasPrefix(line, "export ") || strings.HasPrefix(line, "public ") || + strings.HasPrefix(line, "func ") || strings.HasPrefix(line, "type ") || + strings.HasPrefix(line, "class ") || strings.HasPrefix(line, "def ") +} + +func firstFindingPerRule(findings []core.Finding) []core.Finding { + seen := map[string]struct{}{} + out := make([]core.Finding, 0, len(findings)) + for _, finding := range findings { + if _, ok := seen[finding.RuleID]; ok { + continue + } + seen[finding.RuleID] = struct{}{} + out = append(out, finding) + } + return out +} + +func designFinding(env support.Context, ruleID string, file string, line int, message string, confidence string) core.Finding { + return env.NewFinding(support.FindingInput{ + RuleID: ruleID, + Level: "warn", + Path: file, + Line: line, + Column: 1, + Message: message, + Confidence: confidence, + }) +} diff --git a/internal/codeguard/checks/quality/quality.go b/internal/codeguard/checks/quality/quality.go index 33ddf66..6167da8 100644 --- a/internal/codeguard/checks/quality/quality.go +++ b/internal/codeguard/checks/quality/quality.go @@ -19,6 +19,7 @@ func runQualitySection(ctx context.Context, env support.Context) core.SectionRes func qualityTargetFindings(ctx context.Context, env support.Context, target core.TargetConfig) []core.Finding { findings := languageQualityFindings(ctx, env, target) + findings = append(findings, environmentBranchingFindings(env, target)...) findings = append(findings, cppToolingFindings(ctx, env, target)...) findings = append(findings, cloneFindingsForTarget(env, target)...) findings = append(findings, aiTargetFindings(env, target)...) diff --git a/internal/codeguard/checks/quality/quality_environment.go b/internal/codeguard/checks/quality/quality_environment.go new file mode 100644 index 0000000..88e555e --- /dev/null +++ b/internal/codeguard/checks/quality/quality_environment.go @@ -0,0 +1,80 @@ +package quality + +import ( + "path/filepath" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + environmentBranchPattern = regexp.MustCompile(`(?i)\b(if|switch|case|when)\b[^\n]*(prod|production|staging|stage|dev|development|test)\b|process\.env\.NODE_ENV|Rails\.env\.(production|staging|development|test)\?|os\.(Getenv|getenv)\([^)]*(ENV|ENVIRONMENT|NODE_ENV)|\b(std::)?getenv\([^)]*(ENV|ENVIRONMENT|NODE_ENV)`) + environmentAllowedDirs = []string{"config/", "configs/", "cmd/", "scripts/", ".github/", "deploy/", "deployment/", "k8s/", "kubernetes/", "bootstrap/"} +) + +func environmentBranchingFindings(env support.Context, target core.TargetConfig) []core.Finding { + if !localPrecisionEnabled(env) { + return nil + } + return env.ScanTargetFiles(target, "quality-environment-branching", func(rel string) bool { + return environmentBranchingEligiblePath(env.Config.Checks.DeliveryRules, rel) + }, func(file string, data []byte) []core.Finding { + text := string(data) + if !environmentBranchPattern.MatchString(text) { + return nil + } + return []core.Finding{env.NewFinding(support.FindingInput{ + RuleID: "quality.environment-branching", + Level: "warn", + Path: file, + Line: environmentBranchLine(text), + Column: 1, + Message: "domain/source code branches on deployment environment; move environment policy to configuration or bootstrap boundaries", + Confidence: core.ConfidenceHigh, + Metadata: map[string]string{ + "boundary": "source_code", + }, + })} + }) +} + +func environmentBranchingEligiblePath(cfg core.DeliveryRulesConfig, rel string) bool { + normalized := strings.ToLower(filepath.ToSlash(rel)) + if isQualityFixturePath(normalized) { + return false + } + for _, pattern := range cfg.BootstrapPathPatterns { + if support.PathMatchesPattern(pattern, normalized) { + return false + } + } + for _, prefix := range environmentAllowedDirs { + if strings.HasPrefix(normalized, prefix) || strings.Contains(normalized, "/"+prefix) { + return false + } + } + return strings.HasSuffix(normalized, ".go") || + strings.HasSuffix(normalized, ".py") || + strings.HasSuffix(normalized, ".ts") || + strings.HasSuffix(normalized, ".tsx") || + strings.HasSuffix(normalized, ".js") || + strings.HasSuffix(normalized, ".jsx") || + strings.HasSuffix(normalized, ".cpp") || + strings.HasSuffix(normalized, ".cc") || + strings.HasSuffix(normalized, ".cxx") || + strings.HasSuffix(normalized, ".hpp") || + strings.HasSuffix(normalized, ".hh") || + strings.HasSuffix(normalized, ".h") || + strings.HasSuffix(normalized, ".rb") +} + +func environmentBranchLine(text string) int { + for idx, line := range strings.Split(text, "\n") { + if environmentBranchPattern.MatchString(line) { + return idx + 1 + } + } + return 1 +} diff --git a/internal/codeguard/checks/quality/quality_observability.go b/internal/codeguard/checks/quality/quality_observability.go new file mode 100644 index 0000000..90a8f6c --- /dev/null +++ b/internal/codeguard/checks/quality/quality_observability.go @@ -0,0 +1,250 @@ +package quality + +import ( + "context" + "fmt" + "path/filepath" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + rawLogPatterns = []*regexp.Regexp{ + regexp.MustCompile(`\bfmt\.Print(?:f|ln)?\s*\(`), + regexp.MustCompile(`\blog\.Print(?:f|ln)?\s*\(`), + regexp.MustCompile(`\bconsole\.(?:log|warn|error)\s*\(`), + regexp.MustCompile(`\bprint\s*\(`), + regexp.MustCompile(`\bstd::(?:cout|cerr)\b|\b(?:printf|fprintf)\s*\(`), + } + errorLogPattern = regexp.MustCompile(`(?i)\b(?:logger|log|logging|console|slog|zap)\.\w*(?:error|err|exception|fatal)\w*\s*\(`) + metricLabelPattern = regexp.MustCompile(`(?i)\b(?:label|labels|withlabelvalues|withlabels|tags|attributes?)\b`) + healthReturnOKPattern = regexp.MustCompile(`(?i)\b(?:ok|healthy|pong|200|http\.statusok|statusok)\b`) + dependencyPattern = regexp.MustCompile(`(?i)\b(?:db|database|sql|redis|cache|kafka|queue|http\.client|requests\.|fetch\(|axios\.|grpc|s3|pubsub)\b`) + logAndIgnoreNextPattern = regexp.MustCompile(`(?i)^\s*(?:return\s+(?:nil|none|null|nullptr|true|false|0)\s*;?|return\s*(?:;|$)|continue\s*;?|pass|//\s*ignore|#\s*ignore)`) +) + +func RunObservability(ctx context.Context, env support.Context) core.SectionResult { + return support.RunTargetSection(ctx, env, "observability", "Observability", observabilityTargetFindings) +} + +func observabilityTargetFindings(_ context.Context, env support.Context, target core.TargetConfig) []core.Finding { + return env.ScanTargetFiles(target, "observability", isObservableSourceFile, func(file string, data []byte) []core.Finding { + return findingsForFile(env, file, data) + }) +} + +func findingsForFile(env support.Context, file string, data []byte) []core.Finding { + rules := env.Config.Checks.ObservabilityRules + source := strings.ReplaceAll(string(data), "\r\n", "\n") + code := maskStringsForStructure(file, source) + lines := strings.Split(source, "\n") + codeLines := strings.Split(code, "\n") + findings := make([]core.Finding, 0) + hasDependency := dependencyPattern.MatchString(code) + hasInstrumentation := hasInstrumentationEvidence(source, rules) + healthRegionLine := 0 + + for idx, rawLine := range lines { + lineNo := idx + 1 + codeLine := "" + if idx < len(codeLines) { + codeLine = codeLines[idx] + } + trimmed := strings.TrimSpace(codeLine) + if trimmed == "" || isCommentLine(trimmed) || isTestPath(file) { + continue + } + if enabled(rules.DetectUnstructuredLog) && isRawLogLine(codeLine) && !hasStructuredContext(rawLine) { + findings = append(findings, newFinding(env, "observability.unstructured-log", "warn", file, lineNo, "raw log call has no structured field context", "medium", "log_kind", "raw")) + } + if enabled(rules.DetectErrorWithoutContext) && isErrorLogLine(codeLine) && !hasErrorContext(rawLine, rules) { + findings = append(findings, newFinding(env, "observability.error-without-context", "warn", file, lineNo, "error log lacks operation, request, or safe resource context", "medium", "log_kind", "error")) + } + if enabled(rules.DetectSensitiveLogData) && isLogLikeLine(codeLine) { + if token, ok := firstPatternEvidence(rawLine, rules.SensitiveNamePatterns); ok { + findings = append(findings, newFinding(env, "observability.sensitive-log-data", "fail", file, lineNo, "log call includes sensitive-name evidence", "high", "sensitive_name", token)) + } + } + if enabled(rules.DetectHighCardinalityLabel) && metricLabelPattern.MatchString(codeLine) { + if token, ok := firstPatternEvidence(rawLine, rules.HighCardinalityLabelPatterns); ok { + findings = append(findings, newFinding(env, "observability.high-cardinality-label", "warn", file, lineNo, "metric label appears to use a high-cardinality value", "high", "label_kind", token)) + } + } + if enabled(rules.DetectLogAndIgnore) && isErrorLogLine(codeLine) && nextFewLinesIgnoreError(codeLines, idx) { + findings = append(findings, newFinding(env, "observability.log-and-ignore", "warn", file, lineNo, "failure is logged and then ignored or reported as success", "high", "failure_handling", "log-and-ignore")) + } + if isHealthPathOrLine(file, codeLine, rules) { + healthRegionLine = lineNo + } + if enabled(rules.DetectShallowHealthCheck) && healthRegionLine > 0 && lineNo <= healthRegionLine+8 && healthReturnOKPattern.MatchString(codeLine) && hasDependency { + findings = append(findings, newFinding(env, "observability.shallow-health-check", "warn", file, lineNo, "health/readiness path returns static OK while dependency evidence exists in the file", "medium", "healthcheck", "static-ok")) + healthRegionLine = 0 + } + if enabled(rules.DetectCriticalPathUninstrumented) && isCriticalPath(file, codeLine, rules) && !hasInstrumentation { + findings = append(findings, newFinding(env, "observability.critical-path-uninstrumented", "warn", file, lineNo, "critical production path lacks visible metrics, tracing, or structured logging evidence", "medium", "critical_path", criticalKind(file, codeLine, rules))) + } + } + + return support.DedupeFindings(findings, func(finding core.Finding) string { + return finding.RuleID + "|" + finding.Path + "|" + fmt.Sprintf("%d", finding.Line) + }) +} + +func isObservableSourceFile(path string) bool { + ext := strings.ToLower(filepath.Ext(path)) + switch ext { + case ".go", ".py", ".ts", ".tsx", ".js", ".jsx", ".cc", ".cpp", ".cxx", ".hpp", ".hh", ".hxx", ".h": + return true + default: + return false + } +} + +func maskStringsForStructure(file string, source string) string { + switch strings.ToLower(filepath.Ext(file)) { + case ".ts", ".tsx", ".js", ".jsx": + return support.StripTypeScriptCommentsAndStrings(source) + default: + return source + } +} + +func isRawLogLine(line string) bool { + for _, pattern := range rawLogPatterns { + if pattern.MatchString(line) { + return true + } + } + return false +} + +func isLogLikeLine(line string) bool { + lower := strings.ToLower(line) + return isRawLogLine(line) || strings.Contains(lower, "logger.") || strings.Contains(lower, "logging.") || strings.Contains(lower, "slog.") || strings.Contains(lower, "zap.") +} + +func isErrorLogLine(line string) bool { + lower := strings.ToLower(line) + return errorLogPattern.MatchString(line) || strings.Contains(lower, "std::cerr") || strings.Contains(lower, "fprintf(stderr") +} + +func hasStructuredContext(line string) bool { + return strings.Contains(line, "{") || strings.Contains(line, "With(") || strings.Contains(line, "WithFields") || strings.Contains(line, "String(") || strings.Contains(line, "Int(") || strings.Contains(line, "extra=") +} + +func hasErrorContext(line string, rules core.ObservabilityRulesConfig) bool { + lower := strings.ToLower(line) + if observabilityContainsAny(lower, "operation", "op", "request", "request_id", "trace", "span", "route", "handler", "job", "consumer", "customer", "account", "order") { + return true + } + for _, pattern := range rules.InstrumentationEvidencePatterns { + pattern = strings.ToLower(strings.TrimSpace(pattern)) + if pattern != "" && pattern != "logger" && pattern != "logging" && strings.Contains(lower, pattern) { + return true + } + } + return false +} + +func firstPatternEvidence(line string, patterns []string) (string, bool) { + lower := strings.ToLower(line) + for _, pattern := range patterns { + pattern = strings.ToLower(strings.TrimSpace(pattern)) + if pattern != "" && strings.Contains(lower, pattern) { + return pattern, true + } + } + return "", false +} + +func nextFewLinesIgnoreError(lines []string, idx int) bool { + for next := idx + 1; next < len(lines) && next <= idx+3; next++ { + if logAndIgnoreNextPattern.MatchString(lines[next]) { + return true + } + } + return false +} + +func isHealthPathOrLine(file string, line string, rules core.ObservabilityRulesConfig) bool { + lower := strings.ToLower(file + "\n" + line) + for _, pattern := range rules.HealthcheckPathPatterns { + if strings.TrimSpace(pattern) != "" && strings.Contains(lower, strings.ToLower(pattern)) { + return true + } + } + return false +} + +func isCriticalPath(file string, line string, rules core.ObservabilityRulesConfig) bool { + lower := strings.ToLower(file + "\n" + line) + if !observabilityContainsAny(lower, "func ", "function ", "def ", "=>", "::") { + return false + } + for _, pattern := range rules.CriticalPathPatterns { + if strings.TrimSpace(pattern) != "" && strings.Contains(lower, strings.ToLower(pattern)) { + return true + } + } + return false +} + +func criticalKind(file string, line string, rules core.ObservabilityRulesConfig) string { + lower := strings.ToLower(file + "\n" + line) + for _, pattern := range rules.CriticalPathPatterns { + pattern = strings.ToLower(strings.TrimSpace(pattern)) + if pattern != "" && strings.Contains(lower, pattern) { + return pattern + } + } + return "critical-path" +} + +func hasInstrumentationEvidence(source string, rules core.ObservabilityRulesConfig) bool { + lower := strings.ToLower(source) + for _, pattern := range rules.InstrumentationEvidencePatterns { + pattern = strings.ToLower(strings.TrimSpace(pattern)) + if pattern != "" && strings.Contains(lower, pattern) { + return true + } + } + return false +} + +func isCommentLine(line string) bool { + return strings.HasPrefix(line, "//") || strings.HasPrefix(line, "#") || strings.HasPrefix(line, "*") +} + +func isTestPath(path string) bool { + lower := strings.ToLower(path) + return strings.Contains(lower, "testdata/") || strings.Contains(lower, "__tests__/") || strings.Contains(lower, "fixtures/") || strings.HasSuffix(lower, "_test.go") || strings.HasSuffix(lower, "_test.py") || strings.Contains(lower, ".test.") || strings.Contains(lower, ".spec.") +} + +func observabilityContainsAny(text string, needles ...string) bool { + for _, needle := range needles { + if strings.Contains(text, needle) { + return true + } + } + return false +} + +func enabled(toggle *bool) bool { + return toggle == nil || *toggle +} + +func newFinding(env support.Context, ruleID string, level string, path string, line 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: 1, + Message: message, + Confidence: confidence, + Metadata: map[string]string{metaKey: metaValue}, + }) +} diff --git a/internal/codeguard/checks/quality/quality_operations.go b/internal/codeguard/checks/quality/quality_operations.go new file mode 100644 index 0000000..f4640da --- /dev/null +++ b/internal/codeguard/checks/quality/quality_operations.go @@ -0,0 +1,116 @@ +package quality + +import ( + "context" + "fmt" + "path/filepath" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func RunOperations(ctx context.Context, env support.Context) core.SectionResult { + return support.RunTargetSection(ctx, env, "operations", "Operations", operationsTargetFindings) +} + +func operationsTargetFindings(_ context.Context, env support.Context, target core.TargetConfig) []core.Finding { + rules := env.Config.Checks.OperationsRules + files := listFiles(env, target) + findings := make([]core.Finding, 0, 2) + if !hasCriticalPath(files, rules.CriticalPathPatterns) { + return findings + } + if operationsEnabled(rules.DetectMissingOwner) && !hasAnyPattern(files, rules.OwnerFilePatterns) { + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "operations.missing-owner", + Level: "warn", + Message: "critical production paths have no CODEOWNERS, OWNERS, or service ownership metadata", + Confidence: "medium", + Metadata: map[string]string{ + "searched": fmt.Sprintf("%d", len(rules.OwnerFilePatterns)), + "scope": "target", + }, + })) + } + if operationsEnabled(rules.DetectMissingRunbook) && !hasAnyPattern(files, rules.RunbookPathPatterns) { + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "operations.missing-runbook", + Level: "warn", + Message: "critical production paths have no runbook or operations documentation evidence", + Confidence: "medium", + Metadata: map[string]string{ + "searched": fmt.Sprintf("%d", len(rules.RunbookPathPatterns)), + "scope": "target", + }, + })) + } + return findings +} + +func listFiles(env support.Context, target core.TargetConfig) []string { + if env.ListTargetFiles != nil { + files, err := env.ListTargetFiles(target) + if err == nil { + return files + } + } + files := make([]string, 0) + if env.VisitTargetFiles != nil { + env.VisitTargetFiles(target, func(string) bool { return true }, func(rel string, _ []byte) { + files = append(files, rel) + }) + } + return files +} + +func hasCriticalPath(files []string, patterns []string) bool { + for _, file := range files { + if isGeneratedOrTest(file) { + continue + } + lower := strings.ToLower(filepath.ToSlash(file)) + if isProductionSource(lower) && matchesPattern(lower, patterns) { + return true + } + } + return false +} + +func hasAnyPattern(files []string, patterns []string) bool { + for _, file := range files { + lower := strings.ToLower(filepath.ToSlash(file)) + if matchesPattern(lower, patterns) { + return true + } + } + return false +} + +func matchesPattern(path string, patterns []string) bool { + for _, pattern := range patterns { + pattern = strings.ToLower(strings.TrimSpace(filepath.ToSlash(pattern))) + if pattern != "" && strings.Contains(path, pattern) { + return true + } + } + return false +} + +func isProductionSource(path string) bool { + switch strings.ToLower(filepath.Ext(path)) { + case ".go", ".py", ".ts", ".tsx", ".js", ".jsx", ".cc", ".cpp", ".cxx", ".hpp", ".hh", ".hxx", ".h": + return true + default: + return false + } +} + +func isGeneratedOrTest(path string) bool { + lower := strings.ToLower(path) + return strings.Contains(lower, "testdata/") || strings.Contains(lower, "__tests__/") || strings.Contains(lower, "fixtures/") || strings.HasSuffix(lower, "_test.go") || strings.Contains(lower, ".test.") || strings.Contains(lower, ".spec.") +} + +func operationsEnabled(toggle *bool) bool { + return toggle == nil || *toggle +} diff --git a/internal/codeguard/checks/quality/quality_precision.go b/internal/codeguard/checks/quality/quality_precision.go index 89409fe..e59f505 100644 --- a/internal/codeguard/checks/quality/quality_precision.go +++ b/internal/codeguard/checks/quality/quality_precision.go @@ -1,13 +1,13 @@ package quality import ( + "bytes" "fmt" "go/ast" + "go/printer" "go/token" - "os" - "path/filepath" "regexp" - "sort" + "strconv" "strings" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" @@ -25,6 +25,14 @@ const ( defensiveUnsafeNumericConversionRuleID = "defensive.unsafe-numeric-conversion" maintainabilityPublicSurfaceGrowthID = "maintainability.public-surface-growth" maintainabilityDependencyGrowthID = "maintainability.dependency-growth" + qualityDuplicatedKnowledgeRuleID = "quality.duplicated-knowledge" + qualityAmbiguousNameRuleID = "quality.ambiguous-name" + qualityBooleanArgumentRuleID = "quality.boolean-argument" + qualityMixedAbstractionLevelsRuleID = "quality.mixed-abstraction-levels" + qualityPrimitiveObsessionRuleID = "quality.primitive-obsession" + qualityHiddenSideEffectRuleID = "quality.hidden-side-effect" + qualityMutableGlobalStateRuleID = "quality.mutable-global-state" + qualityRedundantCommentRuleID = "quality.redundant-comment" ) var ( @@ -32,9 +40,18 @@ var ( "foo": {}, "bar": {}, "baz": {}, "qux": {}, "tmp": {}, "temp": {}, "thing": {}, "stuff": {}, "obj": {}, "misc": {}, } + ambiguousIdentifierNames = map[string]struct{}{ + "data": {}, "manager": {}, "helper": {}, "helpers": {}, "process": {}, "processor": {}, + "thing": {}, "item": {}, "items": {}, "obj": {}, "object": {}, "util": {}, "utils": {}, + "misc": {}, "stuff": {}, "value": {}, "values": {}, + } queryFunctionPrefixPattern = regexp.MustCompile(`^(get|find|list|load|read|lookup|fetch|is|has|can|should|compute|calculate|build|format|parse)`) mutatingCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(add|append|assign|create|delete|emit|insert|mutate|persist|publish|remove|save|send|set|store|update|upsert|write)([A-Z_:\-.]|$)`) lowLevelOperationPattern = regexp.MustCompile(`(?i)(\bsql\.|\.query\(|\.exec\(|\bhttp\.|\bfetch\(|\baxios\.|\brequests\.|\bjson\.|\bJSON\.|\bos\.Getenv\b|\bprocess\.env\b|\bfs\.|#include\b)`) + primitiveTypePattern = regexp.MustCompile(`(?i)\b(string|str|int|int64|float|float64|double|decimal|number|boolean|bool|char|long|short)\b`) + domainPrimitiveNamePattern = regexp.MustCompile(`(?i)(id|status|state|type|kind|currency|amount|price|email|phone|country|role|permission|tenant|account|customer|order)`) + mutableGlobalLinePattern = regexp.MustCompile(`(?m)^\s*(?:export\s+)?(?:let|var)\s+[A-Za-z_$][\w$]*\s*=|^\s*[A-Za-z_]\w*\s*=`) + redundantCommentPattern = regexp.MustCompile(`(?i)^\s*(//|#)\s*(get|set|create|delete|update|save|return|initialize|validate|parse|build|handle)\b`) goPublicDeclPattern = regexp.MustCompile(`(?m)^(?:func|type|var|const)\s+(?:\([^)]*\)\s*)?([A-Z][A-Za-z0-9_]*)\b`) pythonPublicDeclPattern = regexp.MustCompile(`(?m)^class\s+([A-Za-z]\w*)\b|^def\s+([A-Za-z]\w*)\s*\(`) tsPublicDeclPattern = regexp.MustCompile(`(?m)^export\s+(?:declare\s+)?(?:async\s+)?(?:class|interface|type|enum|function|const|let|var)\s+([A-Za-z_$][\w$]*)\b`) @@ -92,9 +109,13 @@ func goPrecisionFindings(env support.Context, file string, fset *token.FileSet, } case *ast.GenDecl: findings = append(findings, goGenericDeclFindings(env, file, fset, node)...) + findings = append(findings, goMutableGlobalFindings(env, file, fset, node)...) + findings = append(findings, goDuplicatedKnowledgeFindings(env, file, fset, node)...) } return true }) + findings = append(findings, redundantCommentFindings(env, file, string(data))...) + findings = append(findings, sourceDuplicatedKnowledgeFindings(env, file, string(data))...) return findings } @@ -144,9 +165,7 @@ func goParsedParams(fn *ast.FuncDecl) []support.ParsedParam { params := make([]support.ParsedParam, 0) for _, field := range fn.Type.Params.List { typ := "" - if field.Type != nil { - typ = fmt.Sprintf("%T", field.Type) - } + typ = goExprText(field.Type) for _, name := range field.Names { params = append(params, support.ParsedParam{Name: name.Name, Type: typ}) } @@ -157,6 +176,15 @@ func goParsedParams(fn *ast.FuncDecl) []support.ParsedParam { return params } +func goExprText(expr ast.Expr) string { + if expr == nil { + return "" + } + var buf bytes.Buffer + _ = printer.Fprint(&buf, token.NewFileSet(), expr) + return buf.String() +} + func goFuncReturnsValue(fn *ast.FuncDecl) bool { return fn.Type != nil && fn.Type.Results != nil && len(fn.Type.Results.List) > 0 } @@ -203,6 +231,52 @@ func goGenericDeclFindings(env support.Context, file string, fset *token.FileSet return findings } +func goMutableGlobalFindings(env support.Context, file string, fset *token.FileSet, decl *ast.GenDecl) []core.Finding { + if decl.Tok != token.VAR || isQualityFixturePath(file) { + return nil + } + findings := make([]core.Finding, 0) + for _, spec := range decl.Specs { + value, ok := spec.(*ast.ValueSpec) + if !ok { + continue + } + for _, name := range value.Names { + if strings.HasPrefix(strings.ToLower(name.Name), "err") { + continue + } + findings = append(findings, precisionWarnFinding(env, qualityMutableGlobalStateRuleID, file, fset.Position(name.Pos()).Line, + fmt.Sprintf("mutable package-level variable %q makes behavior harder to isolate and test", name.Name), core.ConfidenceHigh)) + } + } + return findings +} + +func goDuplicatedKnowledgeFindings(env support.Context, file string, fset *token.FileSet, decl *ast.GenDecl) []core.Finding { + if decl.Tok != token.CONST || isQualityFixturePath(file) { + return nil + } + seen := map[string]token.Pos{} + for _, spec := range decl.Specs { + value, ok := spec.(*ast.ValueSpec) + if !ok { + continue + } + for _, expr := range value.Values { + lit, ok := expr.(*ast.BasicLit) + if !ok || !domainKnowledgeLiteral(lit.Value) { + continue + } + if first, exists := seen[lit.Value]; exists { + return []core.Finding{precisionWarnFinding(env, qualityDuplicatedKnowledgeRuleID, file, fset.Position(expr.Pos()).Line, + fmt.Sprintf("business literal is duplicated near line %d; centralize shared domain knowledge", fset.Position(first).Line), core.ConfidenceLow)} + } + seen[lit.Value] = expr.Pos() + } + } + return nil +} + func goDefensiveFindings(env support.Context, file string, fset *token.FileSet, body *ast.BlockStmt) []core.Finding { findings := make([]core.Finding, 0) ast.Inspect(body, func(n ast.Node) bool { @@ -272,6 +346,11 @@ func parsedPrecisionFindings(env support.Context, file string, parsed *support.P findings = append(findings, precisionFunctionFindings(env, file, parsedPrecisionFunction(fn))...) } findings = append(findings, parsedDefensiveFindings(env, file, parsed)...) + findings = append(findings, parsedMutableGlobalFindings(env, file, parsed)...) + findings = append(findings, parsedDuplicatedKnowledgeFindings(env, file, parsed)...) + findings = append(findings, sourceMutableGlobalFindings(env, file, parsed.Source)...) + findings = append(findings, sourceDuplicatedKnowledgeFindings(env, file, parsed.Source)...) + findings = append(findings, redundantCommentFindings(env, file, parsed.Source)...) return findings } @@ -300,26 +379,52 @@ func precisionFunctionFindings(env support.Context, file string, fn precisionFun findings = append(findings, precisionWarnFinding(env, namingGenericIdentifierRuleID, file, fn.StartLine, fmt.Sprintf("function name %q is too generic to communicate intent", fn.Name), core.ConfidenceHigh)) } + if isAmbiguousIdentifier(fn.Name) { + findings = append(findings, precisionWarnFinding(env, qualityAmbiguousNameRuleID, file, fn.StartLine, + fmt.Sprintf("function name %q is ambiguous without domain context", fn.Name), core.ConfidenceHigh)) + } for _, param := range fn.Params { if isGenericIdentifier(param.Name) { findings = append(findings, precisionWarnFinding(env, namingGenericIdentifierRuleID, file, fn.StartLine, fmt.Sprintf("parameter %q is too generic to communicate intent", param.Name), core.ConfidenceHigh)) } + if isAmbiguousIdentifier(param.Name) { + findings = append(findings, precisionWarnFinding(env, qualityAmbiguousNameRuleID, file, fn.StartLine, + fmt.Sprintf("parameter %q is ambiguous without domain context", param.Name), core.ConfidenceHigh)) + } + if isBooleanParameter(param) && !isAllowedBooleanArgumentFunction(fn.Name) { + findings = append(findings, precisionWarnFinding(env, qualityBooleanArgumentRuleID, file, fn.StartLine, + fmt.Sprintf("boolean parameter %q hides behavior behind a flag", param.Name), core.ConfidenceHigh)) + } } for _, assignment := range fn.Assignments { if isGenericIdentifier(assignment.Name) { findings = append(findings, precisionWarnFinding(env, namingGenericIdentifierRuleID, file, assignment.Line, fmt.Sprintf("identifier %q is too generic to explain its role", assignment.Name), core.ConfidenceHigh)) } + if isAmbiguousIdentifier(assignment.Name) { + findings = append(findings, precisionWarnFinding(env, qualityAmbiguousNameRuleID, file, assignment.Line, + fmt.Sprintf("identifier %q is ambiguous without domain context", assignment.Name), core.ConfidenceHigh)) + } } if mixedAbstractionLevel(fn) { findings = append(findings, precisionWarnFinding(env, functionMixedAbstractionLevelRuleID, file, fn.StartLine, fmt.Sprintf("function %s mixes orchestration calls with low-level infrastructure operations", fn.Name), core.ConfidenceMedium)) + findings = append(findings, precisionWarnFinding(env, qualityMixedAbstractionLevelsRuleID, file, fn.StartLine, + fmt.Sprintf("function %s mixes domain intent with low-level implementation details", fn.Name), core.ConfidenceMedium)) } if commandQueryMix(fn) { findings = append(findings, precisionWarnFinding(env, functionCommandQueryMixRuleID, file, fn.StartLine, fmt.Sprintf("function %s returns a value while also invoking mutating side-effect operations", fn.Name), core.ConfidenceMedium)) } + if primitiveObsession(fn) { + findings = append(findings, precisionWarnFinding(env, qualityPrimitiveObsessionRuleID, file, fn.StartLine, + fmt.Sprintf("function %s passes several domain concepts as raw primitives", fn.Name), core.ConfidenceMedium)) + } + if hiddenSideEffect(fn) { + findings = append(findings, precisionWarnFinding(env, qualityHiddenSideEffectRuleID, file, fn.StartLine, + fmt.Sprintf("function %s name implies a query/build operation but it performs side effects", fn.Name), core.ConfidenceMedium)) + } findings = append(findings, errorHandlingFindings(env, file, fn)...) return findings } @@ -333,6 +438,51 @@ func isGenericIdentifier(name string) bool { return ok } +func isAmbiguousIdentifier(name string) bool { + name = strings.Trim(name, "_$") + if name == "" { + return false + } + _, ok := ambiguousIdentifierNames[strings.ToLower(name)] + return ok +} + +func isBooleanParameter(param support.ParsedParam) bool { + return strings.EqualFold(strings.TrimSpace(param.Type), "bool") || + strings.EqualFold(strings.TrimSpace(param.Type), "boolean") || + strings.Contains(strings.ToLower(param.Type), " bool") || + strings.Contains(strings.ToLower(param.Type), ": boolean") +} + +func isAllowedBooleanArgumentFunction(name string) bool { + lowered := strings.ToLower(name) + return strings.HasPrefix(lowered, "set") || strings.HasPrefix(lowered, "with") || + strings.HasPrefix(lowered, "enable") || strings.HasPrefix(lowered, "disable") || + strings.Contains(lowered, "option") +} + +func primitiveObsession(fn precisionFunction) bool { + count := 0 + for _, param := range fn.Params { + if primitiveTypePattern.MatchString(param.Type) && domainPrimitiveNamePattern.MatchString(param.Name) { + count++ + } + } + return count >= 3 +} + +func hiddenSideEffect(fn precisionFunction) bool { + if !queryFunctionPrefixPattern.MatchString(strings.ToLower(fn.Name)) { + return false + } + for _, call := range fn.Calls { + if mutatingCallPattern.MatchString(call.Callee) { + return true + } + } + return false +} + func mixedAbstractionLevel(fn precisionFunction) bool { if fn.EndLine-fn.StartLine < 5 || !lowLevelOperationPattern.MatchString(fn.Body) { return false @@ -454,200 +604,136 @@ func defensiveStatementFindings(env support.Context, file string, statement supp return findings } -func unsafeScriptNumericConversion(text string) bool { - lowered := strings.ToLower(text) - return strings.Contains(lowered, "static_cast") || - strings.Contains(lowered, "static_cast") || - strings.Contains(lowered, "static_cast") || - strings.Contains(lowered, "static_cast") || - strings.Contains(lowered, "static_cast") || - strings.Contains(lowered, "static_cast") || - strings.Contains(lowered, "number(") && strings.Contains(lowered, "bigint") -} - -func maintainabilityDeltaFindings(env support.Context, target core.TargetConfig) []core.Finding { - if env.Mode != core.ScanModeDiff || env.ReadBaseFile == nil { - return nil - } - changed := changedFilesForTarget(env, target) - if len(changed) == 0 { +func parsedMutableGlobalFindings(env support.Context, file string, parsed *support.ParsedFile) []core.Finding { + if isQualityFixturePath(file) { return nil } findings := make([]core.Finding, 0) - for _, rel := range changed { - if !qualityPrecisionSupportsFile(target.Language, rel) { - continue - } - current, ok := readCurrentTargetFile(env, target, rel) - if !ok { + for _, statement := range parsed.Module.Statements { + text := strings.TrimSpace(statement.Text) + if text == "" || strings.HasPrefix(text, "const ") || strings.HasPrefix(text, "final ") { continue } - base, err := env.ReadBaseFile(target, rel) - if err != nil { - continue + if mutableGlobalLinePattern.MatchString(text) { + findings = append(findings, precisionWarnFinding(env, qualityMutableGlobalStateRuleID, file, statement.Line, + "mutable module-level state makes behavior harder to isolate and test", core.ConfidenceHigh)) } - line := deltaFindingLine(env, rel) - findings = append(findings, publicSurfaceGrowthFinding(env, target, rel, base, current, line)...) - findings = append(findings, dependencyGrowthFinding(env, target, rel, base, current, line)...) } return findings } -func changedFilesForTarget(env support.Context, target core.TargetConfig) []string { - seen := map[string]struct{}{} - if env.ListChangedFiles != nil { - if changed, err := env.ListChangedFiles(target); err == nil { - for _, file := range changed { - if file.Status == core.ChangedFileDeleted { - continue - } - seen[filepath.ToSlash(file.Path)] = struct{}{} - } - } +func parsedDuplicatedKnowledgeFindings(env support.Context, file string, parsed *support.ParsedFile) []core.Finding { + if isQualityFixturePath(file) { + return nil } - if len(seen) == 0 && env.DiffScope != nil { - for rel := range env.DiffScope() { - seen[filepath.ToSlash(rel)] = struct{}{} + seen := map[string]int{} + for _, statement := range parsed.Module.Statements { + for _, literal := range domainKnowledgeLiterals(statement.Raw) { + if first, exists := seen[literal]; exists { + return []core.Finding{precisionWarnFinding(env, qualityDuplicatedKnowledgeRuleID, file, statement.Line, + fmt.Sprintf("business literal is duplicated near line %d; centralize shared domain knowledge", first), core.ConfidenceLow)} + } + seen[literal] = statement.Line } } - paths := make([]string, 0, len(seen)) - for rel := range seen { - paths = append(paths, rel) - } - sort.Strings(paths) - return paths + return nil } -func readCurrentTargetFile(env support.Context, target core.TargetConfig, rel string) ([]byte, bool) { - if env.ReadTargetFile != nil { - if data, err := env.ReadTargetFile(target, rel); err == nil { - return data, true +func redundantCommentFindings(env support.Context, file string, source string) []core.Finding { + if isQualityFixturePath(file) { + return nil + } + lines := strings.Split(strings.ReplaceAll(source, "\r\n", "\n"), "\n") + for idx := 0; idx+1 < len(lines); idx++ { + comment := strings.TrimSpace(lines[idx]) + next := strings.TrimSpace(lines[idx+1]) + if !redundantCommentPattern.MatchString(comment) || next == "" { + continue + } + verb := redundantCommentVerb(comment) + if verb != "" && strings.Contains(strings.ToLower(next), verb) { + return []core.Finding{precisionWarnFinding(env, qualityRedundantCommentRuleID, file, idx+1, + "comment restates the next line without adding design intent or constraints", core.ConfidenceLow)} } } - data, err := os.ReadFile(filepath.Join(target.Path, filepath.FromSlash(rel))) //nolint:gosec // rel comes from the scan's own changed-file list - return data, err == nil + return nil } -func deltaFindingLine(env support.Context, rel string) int { - if env.DiffScope == nil { - return 1 - } - scope := env.DiffScope()[filepath.ToSlash(rel)] - if scope.AllChanged { - return 1 +func sourceMutableGlobalFindings(env support.Context, file string, source string) []core.Finding { + if isQualityFixturePath(file) { + return nil } - for _, r := range scope.Ranges { - if r[0] > 0 { - return r[0] + for idx, line := range strings.Split(strings.ReplaceAll(source, "\r\n", "\n"), "\n") { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "#") || + strings.HasPrefix(trimmed, "const ") || strings.HasPrefix(trimmed, "final ") { + continue + } + if mutableGlobalLinePattern.MatchString(trimmed) && !strings.Contains(trimmed, " := ") { + return []core.Finding{precisionWarnFinding(env, qualityMutableGlobalStateRuleID, file, idx+1, + "mutable module-level state makes behavior harder to isolate and test", core.ConfidenceHigh)} } } - return 1 + return nil } -func publicSurfaceGrowthFinding(env support.Context, target core.TargetConfig, rel string, base []byte, current []byte, line int) []core.Finding { - baseCount := publicSurfaceCount(target.Language, rel, string(base)) - currentCount := publicSurfaceCount(target.Language, rel, string(current)) - if currentCount <= baseCount { +func sourceDuplicatedKnowledgeFindings(env support.Context, file string, source string) []core.Finding { + if isQualityFixturePath(file) { return nil } - return []core.Finding{precisionWarnFinding(env, maintainabilityPublicSurfaceGrowthID, rel, line, - fmt.Sprintf("public surface grew from %d to %d symbols in this file; keep new exported API intentional", baseCount, currentCount), - core.ConfidenceHigh)} -} - -func dependencyGrowthFinding(env support.Context, target core.TargetConfig, rel string, base []byte, current []byte, line int) []core.Finding { - baseCount := len(dependencySet(target.Language, rel, string(base))) - currentCount := len(dependencySet(target.Language, rel, string(current))) - if currentCount <= baseCount { - return nil + seen := map[string]int{} + for idx, line := range strings.Split(strings.ReplaceAll(source, "\r\n", "\n"), "\n") { + if strings.TrimSpace(line) == "" { + continue + } + for _, literal := range domainKnowledgeLiterals(line) { + if first, exists := seen[literal]; exists { + return []core.Finding{precisionWarnFinding(env, qualityDuplicatedKnowledgeRuleID, file, idx+1, + fmt.Sprintf("business literal is duplicated near line %d; centralize shared domain knowledge", first), core.ConfidenceLow)} + } + seen[literal] = idx + 1 + } } - return []core.Finding{precisionWarnFinding(env, maintainabilityDependencyGrowthID, rel, line, - fmt.Sprintf("direct dependencies grew from %d to %d in this file; verify the added dependency surface is necessary", baseCount, currentCount), - core.ConfidenceHigh)} + return nil } -func qualityPrecisionSupportsFile(language string, rel string) bool { - switch support.NormalizedLanguage(language) { - case "", "go": - return strings.HasSuffix(rel, ".go") - case "python": - return strings.HasSuffix(rel, ".py") - case "typescript", "javascript": - return isTypeScriptLikeFile(rel) - case "c++", "cpp": - return strings.HasSuffix(rel, ".cpp") || strings.HasSuffix(rel, ".cc") || strings.HasSuffix(rel, ".cxx") || - strings.HasSuffix(rel, ".hpp") || strings.HasSuffix(rel, ".hh") || strings.HasSuffix(rel, ".h") - default: - return false +func redundantCommentVerb(comment string) string { + match := redundantCommentPattern.FindStringSubmatch(comment) + if len(match) < 3 { + return "" } + return strings.ToLower(match[2]) } -func publicSurfaceCount(language string, rel string, source string) int { - switch support.NormalizedLanguage(language) { - case "", "go": - return len(goPublicDeclPattern.FindAllStringSubmatch(source, -1)) - case "python": - count := 0 - for _, match := range pythonPublicDeclPattern.FindAllStringSubmatch(source, -1) { - name := firstNonEmptyString(match[1], match[2]) - if name != "" && !strings.HasPrefix(name, "_") { - count++ - } - } - return count - case "typescript", "javascript": - return len(tsPublicDeclPattern.FindAllStringSubmatch(source, -1)) - case "c++", "cpp": - if !strings.HasSuffix(rel, ".h") && !strings.HasSuffix(rel, ".hh") && !strings.HasSuffix(rel, ".hpp") { - return 0 +func domainKnowledgeLiterals(line string) []string { + matches := regexp.MustCompile(`"([^"]{2,80})"|'([^']{2,80})'|\b\d+(?:\.\d+)?\b`).FindAllString(line, -1) + out := make([]string, 0, len(matches)) + for _, match := range matches { + if domainKnowledgeLiteral(match) { + out = append(out, match) } - return len(cppPublicDeclPattern.FindAllStringSubmatch(source, -1)) - default: - return 0 } + return out } -func dependencySet(language string, rel string, source string) map[string]struct{} { - deps := map[string]struct{}{} - switch support.NormalizedLanguage(language) { - case "", "go": - if fset, parsed, err := support.ParseGoSource(support.Context{}, rel, []byte(source)); err == nil { - _ = fset - for _, imp := range parsed.Imports { - deps[strings.Trim(imp.Path.Value, `"`)] = struct{}{} - } - } - case "python": - for _, imp := range support.ParsePython(source).Imports { - deps[firstNonEmptyString(imp.Module, imp.Name, imp.Alias)] = struct{}{} - } - case "typescript", "javascript": - for _, imp := range support.ParseCLike(source, support.CLikeTypeScript).Imports { - deps[imp.Module] = struct{}{} - } - case "c++", "cpp": - for _, match := range cppIncludePattern.FindAllStringSubmatch(source, -1) { - deps[match[1]] = struct{}{} - } +func domainKnowledgeLiteral(value string) bool { + trimmed := strings.Trim(value, `"'`) + if trimmed == "" || len(trimmed) > 80 { + return false } - return deps -} - -func isQualityFixturePath(path string) bool { - normalized := strings.ToLower(filepath.ToSlash(path)) - if strings.Contains(normalized, "/testdata/") || strings.Contains(normalized, "/fixtures/") || strings.Contains(normalized, "/__fixtures__/") { + if _, err := strconv.Atoi(trimmed); err == nil { return true } - return strings.HasSuffix(normalized, "_test.go") || strings.HasSuffix(normalized, "_test.py") || - strings.HasSuffix(normalized, ".test.ts") || strings.HasSuffix(normalized, ".spec.ts") || - strings.HasSuffix(normalized, ".test.js") || strings.HasSuffix(normalized, ".spec.js") + return domainPrimitiveNamePattern.MatchString(trimmed) || strings.Contains(trimmed, "_") } -func firstNonEmptyString(values ...string) string { - for _, value := range values { - if strings.TrimSpace(value) != "" { - return strings.TrimSpace(value) - } - } - return "" +func unsafeScriptNumericConversion(text string) bool { + lowered := strings.ToLower(text) + return strings.Contains(lowered, "static_cast") || + strings.Contains(lowered, "static_cast") || + strings.Contains(lowered, "static_cast") || + strings.Contains(lowered, "static_cast") || + strings.Contains(lowered, "static_cast") || + strings.Contains(lowered, "static_cast") || + strings.Contains(lowered, "number(") && strings.Contains(lowered, "bigint") } diff --git a/internal/codeguard/checks/quality/quality_precision_delta.go b/internal/codeguard/checks/quality/quality_precision_delta.go new file mode 100644 index 0000000..dfbb44a --- /dev/null +++ b/internal/codeguard/checks/quality/quality_precision_delta.go @@ -0,0 +1,199 @@ +package quality + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func maintainabilityDeltaFindings(env support.Context, target core.TargetConfig) []core.Finding { + if env.Mode != core.ScanModeDiff || env.ReadBaseFile == nil { + return nil + } + changed := changedFilesForTarget(env, target) + if len(changed) == 0 { + return nil + } + findings := make([]core.Finding, 0) + for _, rel := range changed { + if !qualityPrecisionSupportsFile(target.Language, rel) { + continue + } + current, ok := readCurrentTargetFile(env, target, rel) + if !ok { + continue + } + base, err := env.ReadBaseFile(target, rel) + if err != nil { + continue + } + line := deltaFindingLine(env, rel) + findings = append(findings, publicSurfaceGrowthFinding(env, target, rel, base, current, line)...) + findings = append(findings, dependencyGrowthFinding(env, target, rel, base, current, line)...) + } + return findings +} + +func changedFilesForTarget(env support.Context, target core.TargetConfig) []string { + seen := map[string]struct{}{} + if env.ListChangedFiles != nil { + if changed, err := env.ListChangedFiles(target); err == nil { + for _, file := range changed { + if file.Status == core.ChangedFileDeleted { + continue + } + seen[filepath.ToSlash(file.Path)] = struct{}{} + } + } + } + if len(seen) == 0 && env.DiffScope != nil { + for rel := range env.DiffScope() { + seen[filepath.ToSlash(rel)] = struct{}{} + } + } + paths := make([]string, 0, len(seen)) + for rel := range seen { + paths = append(paths, rel) + } + sort.Strings(paths) + return paths +} + +func readCurrentTargetFile(env support.Context, target core.TargetConfig, rel string) ([]byte, bool) { + if env.ReadTargetFile != nil { + if data, err := env.ReadTargetFile(target, rel); err == nil { + return data, true + } + } + data, err := os.ReadFile(filepath.Join(target.Path, filepath.FromSlash(rel))) //nolint:gosec // rel comes from the scan's own changed-file list + return data, err == nil +} + +func deltaFindingLine(env support.Context, rel string) int { + if env.DiffScope == nil { + return 1 + } + scope := env.DiffScope()[filepath.ToSlash(rel)] + if scope.AllChanged { + return 1 + } + for _, r := range scope.Ranges { + if r[0] > 0 { + return r[0] + } + } + return 1 +} + +func publicSurfaceGrowthFinding(env support.Context, target core.TargetConfig, rel string, base []byte, current []byte, line int) []core.Finding { + baseCount := publicSurfaceCount(target.Language, rel, string(base)) + currentCount := publicSurfaceCount(target.Language, rel, string(current)) + if currentCount <= baseCount { + return nil + } + return []core.Finding{precisionWarnFinding(env, maintainabilityPublicSurfaceGrowthID, rel, line, + fmt.Sprintf("public surface grew from %d to %d symbols in this file; keep new exported API intentional", baseCount, currentCount), + core.ConfidenceHigh)} +} + +func dependencyGrowthFinding(env support.Context, target core.TargetConfig, rel string, base []byte, current []byte, line int) []core.Finding { + baseCount := len(dependencySet(target.Language, rel, string(base))) + currentCount := len(dependencySet(target.Language, rel, string(current))) + if currentCount <= baseCount { + return nil + } + return []core.Finding{precisionWarnFinding(env, maintainabilityDependencyGrowthID, rel, line, + fmt.Sprintf("direct dependencies grew from %d to %d in this file; verify the added dependency surface is necessary", baseCount, currentCount), + core.ConfidenceHigh)} +} + +func qualityPrecisionSupportsFile(language string, rel string) bool { + switch support.NormalizedLanguage(language) { + case "", "go": + return strings.HasSuffix(rel, ".go") + case "python": + return strings.HasSuffix(rel, ".py") + case "typescript", "javascript": + return isTypeScriptLikeFile(rel) + case "c++", "cpp": + return strings.HasSuffix(rel, ".cpp") || strings.HasSuffix(rel, ".cc") || strings.HasSuffix(rel, ".cxx") || + strings.HasSuffix(rel, ".hpp") || strings.HasSuffix(rel, ".hh") || strings.HasSuffix(rel, ".h") + default: + return false + } +} + +func publicSurfaceCount(language string, rel string, source string) int { + switch support.NormalizedLanguage(language) { + case "", "go": + return len(goPublicDeclPattern.FindAllStringSubmatch(source, -1)) + case "python": + count := 0 + for _, match := range pythonPublicDeclPattern.FindAllStringSubmatch(source, -1) { + name := firstNonEmptyString(match[1], match[2]) + if name != "" && !strings.HasPrefix(name, "_") { + count++ + } + } + return count + case "typescript", "javascript": + return len(tsPublicDeclPattern.FindAllStringSubmatch(source, -1)) + case "c++", "cpp": + if !strings.HasSuffix(rel, ".h") && !strings.HasSuffix(rel, ".hh") && !strings.HasSuffix(rel, ".hpp") { + return 0 + } + return len(cppPublicDeclPattern.FindAllStringSubmatch(source, -1)) + default: + return 0 + } +} + +func dependencySet(language string, rel string, source string) map[string]struct{} { + deps := map[string]struct{}{} + switch support.NormalizedLanguage(language) { + case "", "go": + if fset, parsed, err := support.ParseGoSource(support.Context{}, rel, []byte(source)); err == nil { + _ = fset + for _, imp := range parsed.Imports { + deps[strings.Trim(imp.Path.Value, `"`)] = struct{}{} + } + } + case "python": + for _, imp := range support.ParsePython(source).Imports { + deps[firstNonEmptyString(imp.Module, imp.Name, imp.Alias)] = struct{}{} + } + case "typescript", "javascript": + for _, imp := range support.ParseCLike(source, support.CLikeTypeScript).Imports { + deps[imp.Module] = struct{}{} + } + case "c++", "cpp": + for _, match := range cppIncludePattern.FindAllStringSubmatch(source, -1) { + deps[match[1]] = struct{}{} + } + } + return deps +} + +func isQualityFixturePath(path string) bool { + normalized := strings.ToLower(filepath.ToSlash(path)) + if strings.Contains(normalized, "/testdata/") || strings.Contains(normalized, "/fixtures/") || strings.Contains(normalized, "/__fixtures__/") { + return true + } + return strings.HasSuffix(normalized, "_test.go") || strings.HasSuffix(normalized, "_test.py") || + strings.HasSuffix(normalized, ".test.ts") || strings.HasSuffix(normalized, ".spec.ts") || + strings.HasSuffix(normalized, ".test.js") || strings.HasSuffix(normalized, ".spec.js") +} + +func firstNonEmptyString(values ...string) string { + for _, value := range values { + if strings.TrimSpace(value) != "" { + return strings.TrimSpace(value) + } + } + return "" +} diff --git a/internal/codeguard/checks/quality/quality_typescript_target.go b/internal/codeguard/checks/quality/quality_typescript_target.go index 02d08ee..d21c2ea 100644 --- a/internal/codeguard/checks/quality/quality_typescript_target.go +++ b/internal/codeguard/checks/quality/quality_typescript_target.go @@ -21,6 +21,12 @@ func typeScriptTargetFindings(ctx context.Context, env support.Context, target c findings = append(findings, env.ScanTargetFiles(target, "quality-typescript-ai", isTypeScriptLikeFile, func(file string, data []byte) []core.Finding { return typeScriptAIOnlyFindingsForFile(env, file, data) })...) + if localPrecisionEnabled(env) { + findings = append(findings, env.ScanTargetFiles(target, "quality-typescript-local-precision", isTypeScriptLikeFile, func(file string, data []byte) []core.Finding { + parsed := support.ParseCLike(string(data), support.CLikeTypeScript) + return parsedPrecisionFindings(env, file, parsed) + })...) + } return findings } return support.TypeScriptTargetFindings(ctx, env, target, support.TypeScriptTargetScan{ diff --git a/internal/codeguard/checks/reliability/reliability_cpp.go b/internal/codeguard/checks/reliability/reliability_cpp.go index 238c24a..cb5b3bb 100644 --- a/internal/codeguard/checks/reliability/reliability_cpp.go +++ b/internal/codeguard/checks/reliability/reliability_cpp.go @@ -34,6 +34,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.findings = append(scan.findings, partialFailureHiddenFindings(env, file, data)...) return scan.findings } diff --git a/internal/codeguard/checks/reliability/reliability_go.go b/internal/codeguard/checks/reliability/reliability_go.go index 1202577..f7a69f6 100644 --- a/internal/codeguard/checks/reliability/reliability_go.go +++ b/internal/codeguard/checks/reliability/reliability_go.go @@ -26,6 +26,7 @@ func goFindingsForFile(env support.Context, file string, data []byte) []core.Fin } return true }) + findings = append(findings, partialFailureHiddenFindings(env, file, data)...) return support.DedupeFindings(findings, func(finding core.Finding) string { return finding.RuleID + "|" + finding.Path + "|" + fmt.Sprintf("%d", finding.Line) + "|" + finding.Message diff --git a/internal/codeguard/checks/reliability/reliability_partial_failure.go b/internal/codeguard/checks/reliability/reliability_partial_failure.go new file mode 100644 index 0000000..0a93542 --- /dev/null +++ b/internal/codeguard/checks/reliability/reliability_partial_failure.go @@ -0,0 +1,62 @@ +package reliability + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + partialFailureLoopPattern = regexp.MustCompile(`(?i)\b(for|while)\b|\.forEach\s*\(|\.map\s*\(`) + partialFailureLogPattern = regexp.MustCompile(`(?i)\b(log|logger|logging|console\.(?:log|warn|error)|print|fprintf|cerr|cout)\b.*\b(err|error|exception|fail(?:ed|ure)?)\b`) + partialFailureContinuePattern = regexp.MustCompile(`^\s*continue\s*;?\s*(?://.*)?$`) + partialFailureSuccessReturn = regexp.MustCompile(`(?i)^\s*return(?:\s+(?:nil|none|null|true|0|\{\}))?\s*;?\s*$`) + partialFailurePropagatePattern = regexp.MustCompile(`(?i)\b(return\s+err|return\s+error|raise\b|throw\b)`) +) + +func partialFailureHiddenFindings(env support.Context, file string, data []byte) []core.Finding { + if !enabled(env.Config.Checks.ReliabilityRules.DetectPartialFailureHidden) { + return nil + } + lines := strings.Split(strings.ReplaceAll(string(data), "\r\n", "\n"), "\n") + loopDepth := 0 + pending := 0 + findings := make([]core.Finding, 0, 1) + for idx, line := range lines { + lineNo := idx + 1 + trimmed := strings.TrimSpace(line) + if trimmed == "" { + continue + } + if partialFailureLoopPattern.MatchString(line) { + loopDepth = 8 + } else if loopDepth > 0 { + loopDepth-- + } + if loopDepth > 0 && partialFailureLogPattern.MatchString(line) && !partialFailurePropagatePattern.MatchString(line) { + pending = lineNo + } + if pending > 0 && partialFailureContinuePattern.MatchString(trimmed) { + findings = append(findings, partialFailureFinding(env, file, pending, "logged failure is skipped and batch processing continues without surfacing partial failure")) + pending = 0 + continue + } + if pending > 0 && lineNo <= pending+12 && partialFailureSuccessReturn.MatchString(trimmed) { + findings = append(findings, partialFailureFinding(env, file, pending, "logged failure is followed by a success return, hiding partial failure from callers")) + pending = 0 + continue + } + if pending > 0 && lineNo > pending+12 { + pending = 0 + } + } + return support.DedupeFindings(findings, func(finding core.Finding) string { + return finding.RuleID + "|" + finding.Path + "|" + finding.Message + }) +} + +func partialFailureFinding(env support.Context, file string, line int, message string) core.Finding { + return newFinding(env, "reliability.partial-failure-hidden", "fail", file, line, 1, message, "medium", "failure_mode", "partial-failure-hidden") +} diff --git a/internal/codeguard/checks/reliability/reliability_python.go b/internal/codeguard/checks/reliability/reliability_python.go index 8e13dfe..963d78d 100644 --- a/internal/codeguard/checks/reliability/reliability_python.go +++ b/internal/codeguard/checks/reliability/reliability_python.go @@ -34,6 +34,7 @@ func pythonFindingsForFile(env support.Context, file string, data []byte) []core for idx, line := range strings.Split(source, "\n") { scan.consumeLine(idx+1, line) } + scan.findings = append(scan.findings, partialFailureHiddenFindings(env, file, data)...) return scan.findings } diff --git a/internal/codeguard/checks/reliability/reliability_typescript.go b/internal/codeguard/checks/reliability/reliability_typescript.go index efe36e4..dbf2c6b 100644 --- a/internal/codeguard/checks/reliability/reliability_typescript.go +++ b/internal/codeguard/checks/reliability/reliability_typescript.go @@ -50,6 +50,7 @@ func typeScriptFindingsForFile(env support.Context, file string, data []byte) [] } } } + scan.findings = append(scan.findings, partialFailureHiddenFindings(env, file, data)...) return scan.findings } diff --git a/internal/codeguard/checks/supplychain/policy.go b/internal/codeguard/checks/supplychain/policy.go index a7c43eb..7186b30 100644 --- a/internal/codeguard/checks/supplychain/policy.go +++ b/internal/codeguard/checks/supplychain/policy.go @@ -18,6 +18,7 @@ func targetFindings(_ context.Context, env support.Context, target core.TargetCo findings = append(findings, vulnerableDependencyFindings(env, target, manifest)...) findings = append(findings, cargoManifestFindings(env, manifest)...) } + findings = append(findings, missingProvenanceFindings(env, target, manifests)...) return findings } diff --git a/internal/codeguard/checks/supplychain/provenance.go b/internal/codeguard/checks/supplychain/provenance.go new file mode 100644 index 0000000..9aae32d --- /dev/null +++ b/internal/codeguard/checks/supplychain/provenance.go @@ -0,0 +1,87 @@ +package supplychain + +import ( + "path/filepath" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + artifactBuildPattern = regexp.MustCompile(`(?i)\b(docker\s+build|docker/build-push-action|goreleaser|npm\s+publish|cargo\s+publish|twine\s+upload|publish|release|upload-artifact|push\s+.+image|buildx)\b`) + provenancePattern = regexp.MustCompile(`(?i)\b(slsa|provenance|attest|attestation|cosign\s+attest|attest-build-provenance|sbom|cyclonedx|sigstore)\b`) +) + +func missingProvenanceFindings(env support.Context, target core.TargetConfig, manifests []core.SupplyChainManifest) []core.Finding { + if env.Config.Checks.SupplyChainRules.DetectProvenance == nil || !*env.Config.Checks.SupplyChainRules.DetectProvenance || len(manifests) == 0 { + return nil + } + files := provenanceEvidenceFiles(env, target) + if len(files) == 0 { + return nil + } + for _, file := range files { + if provenancePattern.MatchString(file.text) { + return nil + } + } + for _, file := range files { + if !artifactBuildPattern.MatchString(file.text) { + continue + } + return []core.Finding{env.NewFinding(support.FindingInput{ + RuleID: "supply_chain.missing-provenance", + Level: "fail", + Path: file.rel, + Line: firstProvenanceLine(file.text), + Column: 1, + Message: "artifact build or publish workflow lacks provenance or attestation evidence", + Confidence: core.ConfidenceHigh, + Metadata: map[string]string{ + "artifact_evidence": "build_or_publish", + }, + })} + } + return nil +} + +type provenanceFile struct { + rel string + text string +} + +func provenanceEvidenceFiles(env support.Context, target core.TargetConfig) []provenanceFile { + files := make([]provenanceFile, 0) + if env.VisitTargetFiles == nil { + return files + } + env.VisitTargetFiles(target, isProvenanceEvidencePath, func(rel string, data []byte) { + files = append(files, provenanceFile{rel: filepath.ToSlash(rel), text: string(data)}) + }) + return files +} + +func isProvenanceEvidencePath(rel string) bool { + normalized := strings.ToLower(filepath.ToSlash(rel)) + base := filepath.Base(normalized) + return strings.HasPrefix(normalized, ".github/workflows/") || + strings.HasPrefix(normalized, ".buildkite/") || + strings.HasPrefix(normalized, "buildkite") || + strings.Contains(normalized, "release") || + strings.Contains(normalized, "deploy") || + strings.HasPrefix(base, "dockerfile") || + base == ".goreleaser.yaml" || + base == ".goreleaser.yml" +} + +func firstProvenanceLine(text string) int { + lines := strings.Split(text, "\n") + for idx, line := range lines { + if artifactBuildPattern.MatchString(line) { + return idx + 1 + } + } + return 1 +} diff --git a/internal/codeguard/checks/support/path_patterns.go b/internal/codeguard/checks/support/path_patterns.go new file mode 100644 index 0000000..3e19256 --- /dev/null +++ b/internal/codeguard/checks/support/path_patterns.go @@ -0,0 +1,39 @@ +package support + +import ( + "path/filepath" + "strings" +) + +func PathMatchesPattern(pattern string, rel string) bool { + pattern = strings.ToLower(filepath.ToSlash(strings.TrimSpace(pattern))) + rel = strings.ToLower(filepath.ToSlash(rel)) + if pattern == "" { + return false + } + if ok, err := filepath.Match(filepath.FromSlash(pattern), filepath.FromSlash(rel)); err == nil && ok { + return true + } + if strings.HasPrefix(pattern, "**/") && strings.Contains(rel, strings.TrimPrefix(pattern, "**/")) { + return true + } + if strings.HasSuffix(pattern, "/**") && strings.HasPrefix(rel, strings.TrimSuffix(pattern, "/**")) { + return true + } + if strings.Contains(pattern, "**") { + parts := strings.Split(pattern, "**") + pos := 0 + for _, part := range parts { + if part == "" { + continue + } + next := strings.Index(rel[pos:], part) + if next < 0 { + return false + } + pos += next + len(part) + } + return true + } + return rel == pattern +} diff --git a/internal/codeguard/config/defaults.go b/internal/codeguard/config/defaults.go index 0bd9e6f..afa65f1 100644 --- a/internal/codeguard/config/defaults.go +++ b/internal/codeguard/config/defaults.go @@ -67,18 +67,30 @@ func applyCheckDefaults(cfg *core.Config, def core.Config) { if cfg.Checks.Data == nil { cfg.Checks.Data = def.Checks.Data } + if cfg.Checks.Observability == nil { + cfg.Checks.Observability = def.Checks.Observability + } + if cfg.Checks.Operations == nil { + cfg.Checks.Operations = def.Checks.Operations + } if cfg.Checks.Change == nil { cfg.Checks.Change = def.Checks.Change } + if cfg.Checks.Delivery == nil { + cfg.Checks.Delivery = def.Checks.Delivery + } applyQualityDefaults(&cfg.Checks.QualityRules, def.Checks.QualityRules) applyPerformanceDefaults(&cfg.Checks.PerformanceRules) applyDesignDefaults(&cfg.Checks.DesignRules, def.Checks.DesignRules) applyPromptDefaults(&cfg.Checks.PromptRules, def.Checks.PromptRules) applyCIDefaults(&cfg.Checks.CIRules, def.Checks.CIRules) + applyDeliveryDefaults(&cfg.Checks.DeliveryRules, def.Checks.DeliveryRules) 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) + applyObservabilityDefaults(&cfg.Checks.ObservabilityRules, def.Checks.ObservabilityRules) + applyOperationsDefaults(&cfg.Checks.OperationsRules, def.Checks.OperationsRules) applyChangeDefaults(&cfg.Checks.ChangeRules, def.Checks.ChangeRules) applyContextDefaults(&cfg.Checks.ContextRules, def.Checks.ContextRules) applyContractDefaults(&cfg.Checks.ContractRules, def.Checks.ContractRules) diff --git a/internal/codeguard/config/defaults_activation.go b/internal/codeguard/config/defaults_activation.go index 4c4c959..49aa5fd 100644 --- a/internal/codeguard/config/defaults_activation.go +++ b/internal/codeguard/config/defaults_activation.go @@ -29,6 +29,7 @@ var checkDisablers = map[string]func(*core.CheckConfig){ "prompts": func(checks *core.CheckConfig) { checks.Prompts = false }, "ci": func(checks *core.CheckConfig) { checks.CI = false }, "supply_chain": func(checks *core.CheckConfig) { checks.SupplyChain = false }, + "delivery": func(checks *core.CheckConfig) { checks.Delivery = boolPtr(false) }, "reliability": func(checks *core.CheckConfig) { checks.Reliability = boolPtr(false) }, "data": func(checks *core.CheckConfig) { checks.Data = boolPtr(false) }, "change": func(checks *core.CheckConfig) { checks.Change = boolPtr(false) }, diff --git a/internal/codeguard/config/defaults_helpers.go b/internal/codeguard/config/defaults_helpers.go index 41cfbba..f555687 100644 --- a/internal/codeguard/config/defaults_helpers.go +++ b/internal/codeguard/config/defaults_helpers.go @@ -44,9 +44,9 @@ func boolValueOrTrue(ptr *bool) bool { } // defaultStringSlice fills a string-slice setting with a copy of its default -// when unset. requireNonEmpty skips defaults that are empty. -func defaultStringSlice(dst *[]string, def []string, requireNonEmpty bool) { - if *dst != nil || (requireNonEmpty && len(def) == 0) { +// when unset. +func defaultStringSlice(dst *[]string, def []string) { + if *dst != nil { return } *dst = append([]string(nil), def...) diff --git a/internal/codeguard/config/defaults_rules.go b/internal/codeguard/config/defaults_rules.go index 4cca8e8..abccccf 100644 --- a/internal/codeguard/config/defaults_rules.go +++ b/internal/codeguard/config/defaults_rules.go @@ -79,7 +79,7 @@ func applyDesignDefaults(dst *core.DesignRulesConfig, def core.DesignRulesConfig defaultInt(&dst.MaxInterfaceMethods, def.MaxInterfaceMethods) defaultInt(&dst.GodModuleThreshold, def.GodModuleThreshold) defaultInt(&dst.HighImpactChangeThreshold, def.HighImpactChangeThreshold) - defaultStringSlice(&dst.ForbiddenPackageNames, def.ForbiddenPackageNames, false) + defaultStringSlice(&dst.ForbiddenPackageNames, def.ForbiddenPackageNames) applyDefaultBoolPtrs( &dst.DetectImportCycles, &dst.DetectGodModules, @@ -115,6 +115,9 @@ func applyCIDefaults(dst *core.CIRulesConfig, def core.CIRulesConfig) { if dst.RequiredWorkflowFiles == nil { dst.RequiredWorkflowFiles = append([]string(nil), def.RequiredWorkflowFiles...) } + if dst.RequiredGates == nil { + dst.RequiredGates = append([]string(nil), def.RequiredGates...) + } if dst.WorkflowContentRules == nil { dst.WorkflowContentRules = append([]core.WorkflowRuleConfig(nil), def.WorkflowContentRules...) } @@ -191,14 +194,30 @@ func applySupplyChainDefaults(dst *core.SupplyChainRulesConfig, def core.SupplyC defaultBoolPtr(&dst.RequireLockfile, boolValueOrTrue(def.RequireLockfile)) defaultBoolPtr(&dst.DetectLockfileDrift, boolValueOrTrue(def.DetectLockfileDrift)) defaultBoolPtr(&dst.DetectUnpinned, boolValueOrTrue(def.DetectUnpinned)) + defaultBoolPtr(&dst.DetectProvenance, boolValueOrTrue(def.DetectProvenance)) // Vulnerability matching is opt-in: a repository must choose and maintain // the advisory cache it trusts. defaultBoolPtr(&dst.DetectVulnerabilities, false) - defaultStringSlice(&dst.AllowedLicenses, def.AllowedLicenses, false) - defaultStringSlice(&dst.DeniedLicenses, def.DeniedLicenses, false) + defaultStringSlice(&dst.AllowedLicenses, def.AllowedLicenses) + defaultStringSlice(&dst.DeniedLicenses, def.DeniedLicenses) defaultSingleCommandMap(&dst.LicenseCommands, def.LicenseCommands) } +func applyDeliveryDefaults(dst *core.DeliveryRulesConfig, def core.DeliveryRulesConfig) { + applyDefaultBoolPtrs( + &dst.DetectMissingRollbackStrategy, + &dst.DetectUnsafeMigrationOrder, + &dst.DetectHighRiskChangeWithoutKillSwitch, + &dst.DetectMissingPostDeployVerification, + ) + defaultStringSlice(&dst.RollbackEvidencePatterns, def.RollbackEvidencePatterns) + defaultStringSlice(&dst.KillSwitchPatterns, def.KillSwitchPatterns) + defaultStringSlice(&dst.PostDeployVerificationPatterns, def.PostDeployVerificationPatterns) + defaultStringSlice(&dst.MigrationPathPatterns, def.MigrationPathPatterns) + defaultStringSlice(&dst.HighRiskPathPatterns, def.HighRiskPathPatterns) + defaultStringSlice(&dst.BootstrapPathPatterns, def.BootstrapPathPatterns) +} + func applyReliabilityDefaults(dst *core.ReliabilityRulesConfig, def core.ReliabilityRulesConfig) { applyDefaultBoolPtrs( &dst.DetectMissingTimeout, @@ -237,6 +256,31 @@ func applyDataDefaults(dst *core.DataRulesConfig, def core.DataRulesConfig) { defaultInt(&dst.MaxWritesWithoutTransaction, def.MaxWritesWithoutTransaction) } +func applyObservabilityDefaults(dst *core.ObservabilityRulesConfig, def core.ObservabilityRulesConfig) { + applyDefaultBoolPtrs( + &dst.DetectUnstructuredLog, + &dst.DetectErrorWithoutContext, + &dst.DetectSensitiveLogData, + &dst.DetectHighCardinalityLabel, + &dst.DetectCriticalPathUninstrumented, + &dst.DetectLogAndIgnore, + &dst.DetectShallowHealthCheck, + ) + defaultStringSlice(&dst.StructuredLoggerPatterns, def.StructuredLoggerPatterns) + defaultStringSlice(&dst.SensitiveNamePatterns, def.SensitiveNamePatterns) + defaultStringSlice(&dst.HighCardinalityLabelPatterns, def.HighCardinalityLabelPatterns) + defaultStringSlice(&dst.CriticalPathPatterns, def.CriticalPathPatterns) + defaultStringSlice(&dst.HealthcheckPathPatterns, def.HealthcheckPathPatterns) + defaultStringSlice(&dst.InstrumentationEvidencePatterns, def.InstrumentationEvidencePatterns) +} + +func applyOperationsDefaults(dst *core.OperationsRulesConfig, def core.OperationsRulesConfig) { + applyDefaultBoolPtrs(&dst.DetectMissingOwner, &dst.DetectMissingRunbook) + defaultStringSlice(&dst.OwnerFilePatterns, def.OwnerFilePatterns) + defaultStringSlice(&dst.RunbookPathPatterns, def.RunbookPathPatterns) + defaultStringSlice(&dst.CriticalPathPatterns, def.CriticalPathPatterns) +} + func applyChangeDefaults(dst *core.ChangeRulesConfig, def core.ChangeRulesConfig) { applyDefaultBoolPtrs( &dst.DetectBehaviorChangeWithoutTest, diff --git a/internal/codeguard/config/example.go b/internal/codeguard/config/example.go index 9afbc6d..43b40ca 100644 --- a/internal/codeguard/config/example.go +++ b/internal/codeguard/config/example.go @@ -31,27 +31,33 @@ func exampleChecks() core.CheckConfig { // they previously ran (enabled) inside quality under quality.* ids. The // explicit false (vs nil) writes the key into generated configs so new // users discover it, and suppresses the upgrade hint in scan output. - Performance: boolPtr(false), - Security: true, - Prompts: true, - CI: true, - SupplyChain: false, - Reliability: boolPtr(false), - Data: boolPtr(false), - Change: boolPtr(false), - QualityRules: exampleQualityRules(), - PerformanceRules: examplePerformanceRules(), - DesignRules: exampleDesignRules(), - PromptRules: examplePromptRules(), - CIRules: exampleCIRules(), - SecurityRules: exampleSecurityRules(), - SupplyChainRules: exampleSupplyChainRules(), - ReliabilityRules: exampleReliabilityRules(), - DataRules: exampleDataRules(), - ChangeRules: exampleChangeRules(), - ContractRules: exampleContractRules(), - ContextRules: exampleContextRules(), - ProductionRisk: exampleProductionRisk(), + Performance: boolPtr(false), + Security: true, + Prompts: true, + CI: true, + SupplyChain: false, + Delivery: boolPtr(false), + Reliability: boolPtr(false), + Data: boolPtr(false), + Observability: boolPtr(false), + Operations: boolPtr(false), + Change: boolPtr(false), + QualityRules: exampleQualityRules(), + PerformanceRules: examplePerformanceRules(), + DesignRules: exampleDesignRules(), + PromptRules: examplePromptRules(), + CIRules: exampleCIRules(), + DeliveryRules: exampleDeliveryRules(), + SecurityRules: exampleSecurityRules(), + SupplyChainRules: exampleSupplyChainRules(), + ReliabilityRules: exampleReliabilityRules(), + DataRules: exampleDataRules(), + ObservabilityRules: exampleObservabilityRules(), + OperationsRules: exampleOperationsRules(), + ChangeRules: exampleChangeRules(), + ContractRules: exampleContractRules(), + ContextRules: exampleContextRules(), + ProductionRisk: exampleProductionRisk(), } } @@ -76,6 +82,50 @@ func exampleSupplyChainRules() core.SupplyChainRulesConfig { RequireLockfile: boolPtr(true), DetectLockfileDrift: boolPtr(true), DetectUnpinned: boolPtr(true), + DetectProvenance: boolPtr(true), + } +} + +func exampleDeliveryRules() core.DeliveryRulesConfig { + return core.DeliveryRulesConfig{ + DetectMissingRollbackStrategy: boolPtr(true), + DetectUnsafeMigrationOrder: boolPtr(true), + DetectHighRiskChangeWithoutKillSwitch: boolPtr(true), + DetectMissingPostDeployVerification: boolPtr(true), + RollbackEvidencePatterns: []string{"rollback", "roll back", "revert", "restore", "previous version"}, + KillSwitchPatterns: []string{"feature flag", "feature_flag", "kill switch", "killswitch", "rollout", "launchdarkly"}, + PostDeployVerificationPatterns: []string{"smoke", "health", "synthetic", "post-deploy", "post deploy", "curl", "slo"}, + MigrationPathPatterns: []string{"migrations/**", "db/migrate/**", "alembic/**"}, + HighRiskPathPatterns: []string{"**/payment/**", "**/payments/**", "**/auth/**", "**/checkout/**", "**/billing/**", "**/migration/**", "**/migrations/**"}, + BootstrapPathPatterns: []string{"cmd/**", "config/**", "configs/**", "**/config/**", "**/bootstrap/**", "scripts/**", ".github/**"}, + } +} + +func exampleObservabilityRules() core.ObservabilityRulesConfig { + return core.ObservabilityRulesConfig{ + DetectUnstructuredLog: boolPtr(true), + DetectErrorWithoutContext: boolPtr(true), + DetectSensitiveLogData: boolPtr(true), + DetectHighCardinalityLabel: boolPtr(true), + DetectCriticalPathUninstrumented: boolPtr(true), + DetectLogAndIgnore: boolPtr(true), + DetectShallowHealthCheck: boolPtr(true), + StructuredLoggerPatterns: []string{"logger.", "logrus.", "zap.", "slog.", "zerolog.", "structlog.", "logging."}, + SensitiveNamePatterns: []string{"password", "passwd", "secret", "token", "api_key", "apikey", "authorization", "cookie", "ssn", "email"}, + HighCardinalityLabelPatterns: []string{"user_id", "userid", "email", "request_id", "requestid", "trace_id", "session_id", "uuid", "path", "url"}, + CriticalPathPatterns: []string{"handler", "controller", "consumer", "job", "worker", "payment", "checkout", "auth", "migration"}, + HealthcheckPathPatterns: []string{"health", "healthz", "ready", "readyz", "live", "livez"}, + InstrumentationEvidencePatterns: []string{"span", "trace", "metric", "counter", "histogram", "observe", "instrument", "prometheus"}, + } +} + +func exampleOperationsRules() core.OperationsRulesConfig { + return core.OperationsRulesConfig{ + DetectMissingOwner: boolPtr(true), + DetectMissingRunbook: boolPtr(true), + OwnerFilePatterns: []string{"CODEOWNERS", ".github/CODEOWNERS", "OWNERS", "owners.yaml", "catalog-info.yaml", "service.yaml", "service.yml"}, + RunbookPathPatterns: []string{"runbook", "runbooks", "docs/runbooks", "ops", "operations"}, + CriticalPathPatterns: []string{"cmd/", "internal/", "service", "api", "worker", "consumer", "job", "payment", "auth", "deploy", "migrations"}, } } diff --git a/internal/codeguard/config/example_rules.go b/internal/codeguard/config/example_rules.go index fcea878..6eddd70 100644 --- a/internal/codeguard/config/example_rules.go +++ b/internal/codeguard/config/example_rules.go @@ -66,6 +66,7 @@ func exampleCIRules() core.CIRulesConfig { RequiredWorkflowFiles: []string{ ".github/workflows/ci.yml", }, + RequiredGates: []string{"test"}, WorkflowContentRules: []core.WorkflowRuleConfig{{ Path: ".github/workflows/ci.yml", RequiredContains: []string{"actions/checkout", "go test ./..."}, diff --git a/internal/codeguard/config/profile.go b/internal/codeguard/config/profile.go index d9d24c6..f4e5601 100644 --- a/internal/codeguard/config/profile.go +++ b/internal/codeguard/config/profile.go @@ -14,6 +14,11 @@ type profileSpec struct { apply func(*core.Config) } +type profileComparisonColumn struct { + label string + name string +} + var profileCatalog = map[string]profileSpec{ "startup": { description: "Looser thresholds for fast-moving repos with lightweight release policy.", @@ -40,8 +45,13 @@ var profileCatalog = map[string]profileSpec{ applyStrictProfile(cfg) cfg.Checks.CIRules.RequiredReleaseFiles = []string{".goreleaser.yaml"} cfg.Checks.CIRules.RequiredAutomationPaths = []string{"Makefile", ".github/workflows/ci.yml"} + cfg.Checks.CIRules.RequiredGates = []string{"test", "security"} + cfg.Checks.SupplyChain = true + cfg.Checks.Delivery = boolPtr(true) cfg.Checks.Data = boolPtr(true) cfg.Checks.Change = boolPtr(true) + cfg.Checks.Observability = boolPtr(true) + cfg.Checks.Operations = boolPtr(true) }, }, "ai-safe": { @@ -61,6 +71,8 @@ var profileCatalog = map[string]profileSpec{ cfg.Checks.Reliability = boolPtr(true) cfg.Checks.Data = boolPtr(true) cfg.Checks.Change = boolPtr(true) + cfg.Checks.Observability = boolPtr(true) + cfg.Checks.Delivery = boolPtr(true) cfg.Checks.ChangeRules.MaxChangedFiles = 20 cfg.Checks.ChangeRules.MaxChangedDirectories = 6 cfg.Checks.ChangeRules.MaxChangedLines = 600 @@ -125,25 +137,36 @@ func ProfileList() []core.PolicyProfile { // documentation from the active profile definitions. Keeping this output // derived from profile data prevents documentation thresholds from drifting. func RenderPolicyProfileComparison() string { - profiles := []struct { - label string - name string - }{ + profiles := profileComparisonColumns() + configs := profileComparisonConfigs(profiles) + var b strings.Builder + b.WriteString("\n") + writeProfileComparisonHeader(&b, profiles) + writeProfileComparisonRows(&b, configs) + b.WriteString("\n") + return b.String() +} + +func profileComparisonColumns() []profileComparisonColumn { + return []profileComparisonColumn{ {label: "Baseline"}, {label: "Startup", name: "startup"}, {label: "Strict", name: "strict"}, {label: "Enterprise", name: "enterprise"}, {label: "AI-safe", name: "ai-safe"}, } +} +func profileComparisonConfigs(profiles []profileComparisonColumn) []core.Config { configs := make([]core.Config, len(profiles)) configs[0] = ExampleConfig() for i := 1; i < len(profiles); i++ { configs[i], _ = ExampleConfigForProfile(profiles[i].name) } + return configs +} - var b strings.Builder - b.WriteString("\n") +func writeProfileComparisonHeader(b *strings.Builder, profiles []profileComparisonColumn) { b.WriteString("| Setting") for _, profile := range profiles { b.WriteString(" | ") @@ -155,77 +178,96 @@ func RenderPolicyProfileComparison() string { b.WriteString(" | ---:") } b.WriteString(" |\n") - writeProfileComparisonRow(&b, "`quality_rules.max_file_lines`", configs, func(cfg core.Config) string { +} + +func writeProfileComparisonRows(b *strings.Builder, configs []core.Config) { + writeProfileComparisonRow(b, "`quality_rules.max_file_lines`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.QualityRules.MaxFileLines) }) - writeProfileComparisonRow(&b, "`quality_rules.max_function_lines`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`quality_rules.max_function_lines`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.QualityRules.MaxFunctionLines) }) - writeProfileComparisonRow(&b, "`quality_rules.max_parameters`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`quality_rules.max_parameters`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.QualityRules.MaxParameters) }) - writeProfileComparisonRow(&b, "`quality_rules.max_cyclomatic_complexity`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`quality_rules.max_cyclomatic_complexity`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.QualityRules.MaxCyclomaticComplexity) }) - writeProfileComparisonRow(&b, "`quality_rules.clone_token_threshold`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`quality_rules.clone_token_threshold`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.QualityRules.CloneTokenThreshold) }) - writeProfileComparisonRow(&b, "`design_rules.max_decls_per_file`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`design_rules.max_decls_per_file`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.DesignRules.MaxDeclsPerFile) }) - writeProfileComparisonRow(&b, "`design_rules.max_methods_per_type`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`design_rules.max_methods_per_type`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.DesignRules.MaxMethodsPerType) }) - writeProfileComparisonRow(&b, "`design_rules.max_interface_methods`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`design_rules.max_interface_methods`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.DesignRules.MaxInterfaceMethods) }) - writeProfileComparisonRow(&b, "`security_rules.govulncheck_mode`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`security_rules.govulncheck_mode`", configs, func(cfg core.Config) string { return cfg.Checks.SecurityRules.GovulncheckMode }) - writeProfileComparisonRow(&b, "`ci_rules.required_release_files`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`ci_rules.required_release_files`", configs, func(cfg core.Config) string { return profileStringSlice(cfg.Checks.CIRules.RequiredReleaseFiles) }) - writeProfileComparisonRow(&b, "`ci_rules.required_automation_paths`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`ci_rules.required_automation_paths`", configs, func(cfg core.Config) string { return profileStringSlice(cfg.Checks.CIRules.RequiredAutomationPaths) }) - writeProfileComparisonRow(&b, "`contracts`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`contracts`", configs, func(cfg core.Config) string { if cfg.Checks.Contracts == nil { return "scan-mode" } return strconv.FormatBool(*cfg.Checks.Contracts) }) - writeProfileComparisonRow(&b, "`reliability`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`reliability`", configs, func(cfg core.Config) string { if cfg.Checks.Reliability == nil { return "scan-mode" } return strconv.FormatBool(*cfg.Checks.Reliability) }) - writeProfileComparisonRow(&b, "`data`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`data`", configs, func(cfg core.Config) string { if cfg.Checks.Data == nil { return "scan-mode" } return strconv.FormatBool(*cfg.Checks.Data) }) - writeProfileComparisonRow(&b, "`change`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`observability`", configs, func(cfg core.Config) string { + if cfg.Checks.Observability == nil { + return "scan-mode" + } + return strconv.FormatBool(*cfg.Checks.Observability) + }) + writeProfileComparisonRow(b, "`operations`", configs, func(cfg core.Config) string { + if cfg.Checks.Operations == nil { + return "scan-mode" + } + return strconv.FormatBool(*cfg.Checks.Operations) + }) + writeProfileComparisonRow(b, "`delivery`", configs, func(cfg core.Config) string { + if cfg.Checks.Delivery == nil { + return "scan-mode" + } + return strconv.FormatBool(*cfg.Checks.Delivery) + }) + writeProfileComparisonRow(b, "`change`", configs, func(cfg core.Config) string { if cfg.Checks.Change == nil { return "scan-mode" } return strconv.FormatBool(*cfg.Checks.Change) }) - writeProfileComparisonRow(&b, "`change_rules.max_changed_files`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`change_rules.max_changed_files`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.ChangeRules.MaxChangedFiles) }) - writeProfileComparisonRow(&b, "`change_rules.max_changed_directories`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`change_rules.max_changed_directories`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.ChangeRules.MaxChangedDirectories) }) - writeProfileComparisonRow(&b, "`change_rules.max_changed_lines`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`change_rules.max_changed_lines`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.ChangeRules.MaxChangedLines) }) - writeProfileComparisonRow(&b, "`change_rules.min_test_to_production_ratio_percent`", configs, func(cfg core.Config) string { + writeProfileComparisonRow(b, "`change_rules.min_test_to_production_ratio_percent`", configs, func(cfg core.Config) string { return strconv.Itoa(cfg.Checks.ChangeRules.MinTestToProductionRatioPercent) }) - b.WriteString("\n") - return b.String() } func writeProfileComparisonRow(b *strings.Builder, setting string, configs []core.Config, value func(core.Config) string) { diff --git a/internal/codeguard/config/profile_test.go b/internal/codeguard/config/profile_test.go index 42e20df..08f1263 100644 --- a/internal/codeguard/config/profile_test.go +++ b/internal/codeguard/config/profile_test.go @@ -25,6 +25,9 @@ type profileThresholds struct { contracts *bool reliability *bool data *bool + observability *bool + operations *bool + delivery *bool change *bool maxChangedFiles int maxChangedDirectories int @@ -55,6 +58,14 @@ func TestProfilesPreserveExpectedPolicyValues(t *testing.T) { } func expectedProfileThresholds() map[string]profileThresholds { + profiles := baselineAndStartupProfileThresholds() + for name, thresholds := range strictProfileThresholds() { + profiles[name] = thresholds + } + return profiles +} + +func baselineAndStartupProfileThresholds() map[string]profileThresholds { return map[string]profileThresholds{ "baseline": { maxFileLines: 400, @@ -70,6 +81,9 @@ func expectedProfileThresholds() map[string]profileThresholds { requiredAutomationPaths: []string{"Makefile"}, reliability: boolPtr(false), data: boolPtr(false), + observability: boolPtr(false), + operations: boolPtr(false), + delivery: boolPtr(false), change: boolPtr(false), maxChangedFiles: 25, maxChangedDirectories: 8, @@ -89,12 +103,20 @@ func expectedProfileThresholds() map[string]profileThresholds { requiredAutomationPaths: []string{"Makefile"}, reliability: boolPtr(false), data: boolPtr(false), + observability: boolPtr(false), + operations: boolPtr(false), + delivery: boolPtr(false), change: boolPtr(false), maxChangedFiles: 25, maxChangedDirectories: 8, maxChangedLines: 800, minTestProdRatioPercent: 20, }, + } +} + +func strictProfileThresholds() map[string]profileThresholds { + return map[string]profileThresholds{ "strict": { maxFileLines: 300, maxFunctionLines: 60, @@ -110,6 +132,9 @@ func expectedProfileThresholds() map[string]profileThresholds { contracts: boolPtr(true), reliability: boolPtr(true), data: boolPtr(false), + observability: boolPtr(false), + operations: boolPtr(false), + delivery: boolPtr(false), change: boolPtr(true), maxChangedFiles: 25, maxChangedDirectories: 8, @@ -131,6 +156,9 @@ func expectedProfileThresholds() map[string]profileThresholds { contracts: boolPtr(true), reliability: boolPtr(true), data: boolPtr(true), + observability: boolPtr(true), + operations: boolPtr(true), + delivery: boolPtr(true), change: boolPtr(true), maxChangedFiles: 25, maxChangedDirectories: 8, @@ -151,6 +179,9 @@ func expectedProfileThresholds() map[string]profileThresholds { requiredAutomationPaths: []string{"Makefile"}, reliability: boolPtr(true), data: boolPtr(true), + observability: boolPtr(true), + operations: boolPtr(false), + delivery: boolPtr(true), change: boolPtr(true), maxChangedFiles: 20, maxChangedDirectories: 6, @@ -176,6 +207,9 @@ func profileThresholdsFromConfig(cfg core.Config) profileThresholds { contracts: cfg.Checks.Contracts, reliability: cfg.Checks.Reliability, data: cfg.Checks.Data, + observability: cfg.Checks.Observability, + operations: cfg.Checks.Operations, + delivery: cfg.Checks.Delivery, change: cfg.Checks.Change, maxChangedFiles: cfg.Checks.ChangeRules.MaxChangedFiles, maxChangedDirectories: cfg.Checks.ChangeRules.MaxChangedDirectories, diff --git a/internal/codeguard/config/recommended_defaults_test.go b/internal/codeguard/config/recommended_defaults_test.go index 64b321e..6f0f06b 100644 --- a/internal/codeguard/config/recommended_defaults_test.go +++ b/internal/codeguard/config/recommended_defaults_test.go @@ -51,6 +51,7 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults want: core.CheckConfig{ Performance: trueValue, SupplyChain: true, + Delivery: falseValue, Reliability: falseValue, Data: falseValue, Change: falseValue, @@ -70,6 +71,7 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults Security: true, Prompts: true, CI: true, + Delivery: falseValue, Reliability: falseValue, Data: falseValue, Change: falseValue, @@ -81,6 +83,7 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults UseRecommendedDefaults: true, Performance: trueValue, SupplyChain: true, + Delivery: trueValue, Reliability: trueValue, Data: trueValue, Change: trueValue, @@ -94,6 +97,7 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults CI: true, Performance: trueValue, SupplyChain: true, + Delivery: trueValue, Reliability: trueValue, Data: trueValue, Change: trueValue, @@ -111,7 +115,7 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults Data: trueValue, Change: trueValue, Disabled: []string{ - "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "reliability", "data", "change", "context", "contracts", + "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "delivery", "reliability", "data", "change", "context", "contracts", }, }, want: core.CheckConfig{ @@ -119,11 +123,12 @@ func recommendedDefaultsCases(trueValue, falseValue *bool) []recommendedDefaults Performance: falseValue, Context: falseValue, Contracts: falseValue, + Delivery: falseValue, Reliability: falseValue, Data: falseValue, Change: falseValue, Disabled: []string{ - "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "reliability", "data", "change", "context", "contracts", + "quality", "performance", "design", "security", "prompts", "ci", "supply_chain", "delivery", "reliability", "data", "change", "context", "contracts", }, }, }, @@ -139,6 +144,7 @@ func sameCheckActivation(got, want core.CheckConfig) bool { got.Prompts == want.Prompts && got.CI == want.CI && got.SupplyChain == want.SupplyChain && + reflect.DeepEqual(got.Delivery, want.Delivery) && reflect.DeepEqual(got.Performance, want.Performance) && reflect.DeepEqual(got.Reliability, want.Reliability) && reflect.DeepEqual(got.Data, want.Data) && diff --git a/internal/codeguard/config/validate.go b/internal/codeguard/config/validate.go index e73f732..23106a8 100644 --- a/internal/codeguard/config/validate.go +++ b/internal/codeguard/config/validate.go @@ -24,8 +24,11 @@ func Validate(cfg core.Config) error { validateRiskScoring(cfg.Checks.QualityRules.RiskScoring), validateAIChecks(cfg.Checks.QualityRules.AIChecks), validateSupplyChainRules(cfg.Checks.SupplyChainRules), + validateDeliveryRules(cfg.Checks.DeliveryRules), validateReliabilityRules(cfg.Checks.ReliabilityRules), validateDataRules(cfg.Checks.DataRules), + validateObservabilityRules(cfg.Checks.ObservabilityRules), + validateOperationsRules(cfg.Checks.OperationsRules), validateChangeRules(cfg.Checks.ChangeRules), validateProductionRisk(cfg.Checks.ProductionRisk), validateContractRules(cfg.Checks.ContractRules), diff --git a/internal/codeguard/config/validate_defaults.go b/internal/codeguard/config/validate_defaults.go index 1e35096..d00240e 100644 --- a/internal/codeguard/config/validate_defaults.go +++ b/internal/codeguard/config/validate_defaults.go @@ -9,7 +9,7 @@ import ( var recognizedDisabledChecks = map[string]struct{}{ "quality": {}, "performance": {}, "design": {}, "security": {}, "prompts": {}, - "ci": {}, "supply_chain": {}, "reliability": {}, "data": {}, "change": {}, "context": {}, "contracts": {}, + "ci": {}, "supply_chain": {}, "delivery": {}, "reliability": {}, "data": {}, "change": {}, "context": {}, "contracts": {}, } func validateDisabledChecks(disabled []string) error { diff --git a/internal/codeguard/config/validate_reliability_data.go b/internal/codeguard/config/validate_reliability_data.go index 800193d..f29c576 100644 --- a/internal/codeguard/config/validate_reliability_data.go +++ b/internal/codeguard/config/validate_reliability_data.go @@ -26,6 +26,60 @@ func validateDataRules(rules core.DataRulesConfig) error { return nil } +func validateObservabilityRules(rules core.ObservabilityRulesConfig) error { + for _, item := range []struct { + field string + values []string + }{ + {"observability_rules.structured_logger_patterns", rules.StructuredLoggerPatterns}, + {"observability_rules.sensitive_name_patterns", rules.SensitiveNamePatterns}, + {"observability_rules.high_cardinality_label_patterns", rules.HighCardinalityLabelPatterns}, + {"observability_rules.critical_path_patterns", rules.CriticalPathPatterns}, + {"observability_rules.healthcheck_path_patterns", rules.HealthcheckPathPatterns}, + {"observability_rules.instrumentation_evidence_patterns", rules.InstrumentationEvidencePatterns}, + } { + if err := validateNonEmptyStrings(item.field, item.values); err != nil { + return err + } + } + return nil +} + +func validateOperationsRules(rules core.OperationsRulesConfig) error { + for _, item := range []struct { + field string + values []string + }{ + {"operations_rules.owner_file_patterns", rules.OwnerFilePatterns}, + {"operations_rules.runbook_path_patterns", rules.RunbookPathPatterns}, + {"operations_rules.critical_path_patterns", rules.CriticalPathPatterns}, + } { + if err := validateNonEmptyStrings(item.field, item.values); err != nil { + return err + } + } + return nil +} + +func validateDeliveryRules(rules core.DeliveryRulesConfig) error { + for _, item := range []struct { + field string + values []string + }{ + {"delivery_rules.rollback_evidence_patterns", rules.RollbackEvidencePatterns}, + {"delivery_rules.kill_switch_patterns", rules.KillSwitchPatterns}, + {"delivery_rules.post_deploy_verification_patterns", rules.PostDeployVerificationPatterns}, + {"delivery_rules.migration_path_patterns", rules.MigrationPathPatterns}, + {"delivery_rules.high_risk_path_patterns", rules.HighRiskPathPatterns}, + {"delivery_rules.bootstrap_path_patterns", rules.BootstrapPathPatterns}, + } { + if err := validateNonEmptyStrings(item.field, item.values); err != nil { + return err + } + } + return nil +} + func validateChangeRules(rules core.ChangeRulesConfig) error { for _, item := range []struct { field string diff --git a/internal/codeguard/core/config_rule_types.go b/internal/codeguard/core/config_rule_types.go index ec99fe2..00fdefb 100644 --- a/internal/codeguard/core/config_rule_types.go +++ b/internal/codeguard/core/config_rule_types.go @@ -173,6 +173,7 @@ type PromptRulesConfig struct { type CIRulesConfig struct { RequireWorkflowDir *bool `json:"require_workflow_dir,omitempty" yaml:"require_workflow_dir,omitempty"` RequiredWorkflowFiles []string `json:"required_workflow_files,omitempty" yaml:"required_workflow_files,omitempty"` + RequiredGates []string `json:"required_gates,omitempty" yaml:"required_gates,omitempty"` WorkflowContentRules []WorkflowRuleConfig `json:"workflow_content_rules,omitempty" yaml:"workflow_content_rules,omitempty"` RequiredReleaseFiles []string `json:"required_release_files,omitempty" yaml:"required_release_files,omitempty"` RequiredAutomationPaths []string `json:"required_automation_paths,omitempty" yaml:"required_automation_paths,omitempty"` @@ -207,6 +208,7 @@ type SupplyChainRulesConfig struct { RequireLockfile *bool `json:"require_lockfile,omitempty" yaml:"require_lockfile,omitempty"` DetectLockfileDrift *bool `json:"detect_lockfile_drift,omitempty" yaml:"detect_lockfile_drift,omitempty"` DetectUnpinned *bool `json:"detect_unpinned,omitempty" yaml:"detect_unpinned,omitempty"` + DetectProvenance *bool `json:"detect_provenance,omitempty" yaml:"detect_provenance,omitempty"` // DetectVulnerabilities enables matching normalized dependencies against the // local advisory cache. It never contacts an advisory service during a scan. DetectVulnerabilities *bool `json:"detect_vulnerabilities,omitempty" yaml:"detect_vulnerabilities,omitempty"` @@ -218,6 +220,51 @@ type SupplyChainRulesConfig struct { LicenseCommands map[string]CommandCheckConfig `json:"license_commands,omitempty" yaml:"license_commands,omitempty"` } +// ObservabilityRulesConfig tunes the observability section. Nil rule toggles +// default to enabled when the section itself is enabled by configuration or a +// profile. +type ObservabilityRulesConfig struct { + DetectUnstructuredLog *bool `json:"detect_unstructured_log,omitempty" yaml:"detect_unstructured_log,omitempty"` + DetectErrorWithoutContext *bool `json:"detect_error_without_context,omitempty" yaml:"detect_error_without_context,omitempty"` + DetectSensitiveLogData *bool `json:"detect_sensitive_log_data,omitempty" yaml:"detect_sensitive_log_data,omitempty"` + DetectHighCardinalityLabel *bool `json:"detect_high_cardinality_label,omitempty" yaml:"detect_high_cardinality_label,omitempty"` + DetectCriticalPathUninstrumented *bool `json:"detect_critical_path_uninstrumented,omitempty" yaml:"detect_critical_path_uninstrumented,omitempty"` + DetectLogAndIgnore *bool `json:"detect_log_and_ignore,omitempty" yaml:"detect_log_and_ignore,omitempty"` + DetectShallowHealthCheck *bool `json:"detect_shallow_health_check,omitempty" yaml:"detect_shallow_health_check,omitempty"` + StructuredLoggerPatterns []string `json:"structured_logger_patterns,omitempty" yaml:"structured_logger_patterns,omitempty"` + SensitiveNamePatterns []string `json:"sensitive_name_patterns,omitempty" yaml:"sensitive_name_patterns,omitempty"` + HighCardinalityLabelPatterns []string `json:"high_cardinality_label_patterns,omitempty" yaml:"high_cardinality_label_patterns,omitempty"` + CriticalPathPatterns []string `json:"critical_path_patterns,omitempty" yaml:"critical_path_patterns,omitempty"` + HealthcheckPathPatterns []string `json:"healthcheck_path_patterns,omitempty" yaml:"healthcheck_path_patterns,omitempty"` + InstrumentationEvidencePatterns []string `json:"instrumentation_evidence_patterns,omitempty" yaml:"instrumentation_evidence_patterns,omitempty"` +} + +// OperationsRulesConfig tunes repository operations-readiness checks. Nil rule +// toggles default to enabled when the section itself is enabled by +// configuration or a profile. +type OperationsRulesConfig struct { + DetectMissingOwner *bool `json:"detect_missing_owner,omitempty" yaml:"detect_missing_owner,omitempty"` + DetectMissingRunbook *bool `json:"detect_missing_runbook,omitempty" yaml:"detect_missing_runbook,omitempty"` + OwnerFilePatterns []string `json:"owner_file_patterns,omitempty" yaml:"owner_file_patterns,omitempty"` + RunbookPathPatterns []string `json:"runbook_path_patterns,omitempty" yaml:"runbook_path_patterns,omitempty"` + CriticalPathPatterns []string `json:"critical_path_patterns,omitempty" yaml:"critical_path_patterns,omitempty"` +} + +// DeliveryRulesConfig tunes rollout-governance checks. Nil rule toggles +// default to enabled when the delivery section itself is enabled. +type DeliveryRulesConfig struct { + DetectMissingRollbackStrategy *bool `json:"detect_missing_rollback_strategy,omitempty" yaml:"detect_missing_rollback_strategy,omitempty"` + DetectUnsafeMigrationOrder *bool `json:"detect_unsafe_migration_order,omitempty" yaml:"detect_unsafe_migration_order,omitempty"` + DetectHighRiskChangeWithoutKillSwitch *bool `json:"detect_high_risk_change_without_kill_switch,omitempty" yaml:"detect_high_risk_change_without_kill_switch,omitempty"` + DetectMissingPostDeployVerification *bool `json:"detect_missing_post_deploy_verification,omitempty" yaml:"detect_missing_post_deploy_verification,omitempty"` + RollbackEvidencePatterns []string `json:"rollback_evidence_patterns,omitempty" yaml:"rollback_evidence_patterns,omitempty"` + KillSwitchPatterns []string `json:"kill_switch_patterns,omitempty" yaml:"kill_switch_patterns,omitempty"` + PostDeployVerificationPatterns []string `json:"post_deploy_verification_patterns,omitempty" yaml:"post_deploy_verification_patterns,omitempty"` + MigrationPathPatterns []string `json:"migration_path_patterns,omitempty" yaml:"migration_path_patterns,omitempty"` + HighRiskPathPatterns []string `json:"high_risk_path_patterns,omitempty" yaml:"high_risk_path_patterns,omitempty"` + BootstrapPathPatterns []string `json:"bootstrap_path_patterns,omitempty" yaml:"bootstrap_path_patterns,omitempty"` +} + // ReliabilityRulesConfig tunes the reliability section. Nil rule toggles // default to enabled when the section itself is enabled by configuration or a // profile. diff --git a/internal/codeguard/core/config_types.go b/internal/codeguard/core/config_types.go index 9b41f14..97c406a 100644 --- a/internal/codeguard/core/config_types.go +++ b/internal/codeguard/core/config_types.go @@ -77,6 +77,9 @@ 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"` + // Delivery toggles rollout-governance checks such as rollback evidence, + // migration sequencing, kill switches, and post-deploy verification. + Delivery *bool `json:"delivery,omitempty" yaml:"delivery,omitempty"` // Reliability toggles production reliability checks such as missing // timeouts, unbounded retries, cancellation propagation, concurrency bounds, // cleanup handling, and graceful shutdown. @@ -85,6 +88,12 @@ type CheckConfig struct { // missing transaction boundaries, unsafe dual writes, unbounded reads, // unstable pagination, and cache policy gaps. Data *bool `json:"data,omitempty" yaml:"data,omitempty"` + // Observability toggles production operability checks such as structured + // logging, contextual errors, metric cardinality, critical-path + // instrumentation, and health-check depth. + Observability *bool `json:"observability,omitempty" yaml:"observability,omitempty"` + // Operations toggles service ownership and runbook metadata checks. + Operations *bool `json:"operations,omitempty" yaml:"operations,omitempty"` // Change toggles change-safety, testability, and refactor-confidence checks // such as oversized diffs, mixed concerns, behavior changes without tests, // and refactor-only changes that alter observable behavior. @@ -99,21 +108,24 @@ type CheckConfig struct { // basename ambiguity). When nil it defaults to enabled in full scans and // disabled in diff scans, whose repo-level findings would repeat on every // PR regardless of the change under review. - Context *bool `json:"context,omitempty" yaml:"context,omitempty"` - QualityRules QualityRulesConfig `json:"quality_rules" yaml:"quality_rules"` - PerformanceRules PerformanceRulesConfig `json:"performance_rules,omitempty" yaml:"performance_rules,omitempty"` - DesignRulesFile string `json:"design_rules_file,omitempty" yaml:"design_rules_file,omitempty"` - DesignRules DesignRulesConfig `json:"design_rules" yaml:"design_rules"` - PromptRules PromptRulesConfig `json:"prompt_rules" yaml:"prompt_rules"` - 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"` - ChangeRules ChangeRulesConfig `json:"change_rules,omitempty" yaml:"change_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"` + Context *bool `json:"context,omitempty" yaml:"context,omitempty"` + QualityRules QualityRulesConfig `json:"quality_rules" yaml:"quality_rules"` + PerformanceRules PerformanceRulesConfig `json:"performance_rules,omitempty" yaml:"performance_rules,omitempty"` + DesignRulesFile string `json:"design_rules_file,omitempty" yaml:"design_rules_file,omitempty"` + DesignRules DesignRulesConfig `json:"design_rules" yaml:"design_rules"` + PromptRules PromptRulesConfig `json:"prompt_rules" yaml:"prompt_rules"` + CIRules CIRulesConfig `json:"ci_rules" yaml:"ci_rules"` + DeliveryRules DeliveryRulesConfig `json:"delivery_rules,omitempty" yaml:"delivery_rules,omitempty"` + 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"` + ObservabilityRules ObservabilityRulesConfig `json:"observability_rules,omitempty" yaml:"observability_rules,omitempty"` + OperationsRules OperationsRulesConfig `json:"operations_rules,omitempty" yaml:"operations_rules,omitempty"` + ChangeRules ChangeRulesConfig `json:"change_rules,omitempty" yaml:"change_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/rules/catalog.go b/internal/codeguard/rules/catalog.go index b205c7b..0bed1db 100644 --- a/internal/codeguard/rules/catalog.go +++ b/internal/codeguard/rules/catalog.go @@ -17,7 +17,10 @@ var catalog = withSecurityOWASP(mergeRuleCatalogs( securityExtraCatalog, reliabilityCatalog, dataCatalog, + observabilityCatalog, + operationsCatalog, changeSafetyCatalog, + deliveryCatalog, supplyChainCatalog, contextCatalog, contextReadinessCatalog, diff --git a/internal/codeguard/rules/catalog_delivery.go b/internal/codeguard/rules/catalog_delivery.go new file mode 100644 index 0000000..e915773 --- /dev/null +++ b/internal/codeguard/rules/catalog_delivery.go @@ -0,0 +1,46 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var deliveryCatalog = map[string]core.RuleMetadata{ + "delivery.missing-rollback-strategy": { + ID: "delivery.missing-rollback-strategy", + Section: "Delivery", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Missing rollback strategy", + Description: "Warns when deployment or destructive migration evidence appears without rollback, revert, or restore instructions.", + HowToFix: "Document or automate rollback steps next to the rollout workflow, migration, or runbook before shipping the change.", + }, + "delivery.unsafe-migration-order": { + ID: "delivery.unsafe-migration-order", + Section: "Delivery", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Unsafe migration order", + Description: "Warns when a destructive migration lacks expand/backfill/contract, concurrent, or rollback sequencing evidence.", + HowToFix: "Split the migration into compatible phases, backfill safely, verify readers, then remove old schema in a later rollout.", + }, + "delivery.high-risk-change-without-kill-switch": { + ID: "delivery.high-risk-change-without-kill-switch", + Section: "Delivery", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "High-risk change without kill switch", + Description: "Warns when critical payment, auth, checkout, billing, or migration behavior changes without feature flag or kill-switch evidence.", + HowToFix: "Gate the high-risk behavior behind a feature flag or operational kill switch and document the rollback path.", + }, + "delivery.missing-post-deploy-verification": { + ID: "delivery.missing-post-deploy-verification", + Section: "Delivery", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Missing post-deploy verification", + Description: "Warns when a deployment workflow has no smoke, health, synthetic, SLO, or equivalent verification evidence after rollout.", + HowToFix: "Add a post-deploy smoke test, health check, synthetic probe, or SLO verification step to the deployment workflow.", + }, +} diff --git a/internal/codeguard/rules/catalog_design.go b/internal/codeguard/rules/catalog_design.go index acb2dc6..f625804 100644 --- a/internal/codeguard/rules/catalog_design.go +++ b/internal/codeguard/rules/catalog_design.go @@ -85,13 +85,14 @@ var designCatalog = map[string]core.RuleMetadata{ HowToFix: "Move related declarations into smaller files with clearer ownership.", }, "design.typescript.generic-module-name": { - ID: "design.typescript.generic-module-name", - Section: "Design Patterns", - DefaultLevel: "warn", - ExecutionModel: core.RuleExecutionModelLanguageAgnostic, - Title: "TypeScript generic module name", - Description: "Warns when a TypeScript or JavaScript module name is too generic to communicate ownership or responsibility.", - HowToFix: "Rename the module to something specific to its responsibility.", + ID: "design.typescript.generic-module-name", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageTypeScript, core.RuleLanguageJavaScript), + Title: "TypeScript generic module name", + Description: "Warns when a TypeScript or JavaScript module name is too generic to communicate ownership or responsibility.", + HowToFix: "Rename the module to something specific to its responsibility.", }, "design.cpp.generic-module-name": { ID: "design.cpp.generic-module-name", @@ -134,13 +135,14 @@ var designCatalog = map[string]core.RuleMetadata{ HowToFix: "Break the public surface into smaller focused contracts or move implementation details behind non-public helpers.", }, "design.typescript.max-methods-per-type": { - ID: "design.typescript.max-methods-per-type", - Section: "Design Patterns", - DefaultLevel: "warn", - ExecutionModel: core.RuleExecutionModelLanguageAgnostic, - Title: "TypeScript methods per type", - Description: "Warns when a TypeScript class accumulates too many methods.", - HowToFix: "Split responsibilities across smaller classes or extracted collaborators.", + ID: "design.typescript.max-methods-per-type", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageTypeScript, core.RuleLanguageJavaScript), + Title: "TypeScript methods per type", + Description: "Warns when a TypeScript or JavaScript class accumulates too many methods.", + HowToFix: "Split responsibilities across smaller classes or extracted collaborators.", }, "design.typescript.max-interface-members": { ID: "design.typescript.max-interface-members", @@ -244,4 +246,84 @@ var designCatalog = map[string]core.RuleMetadata{ Description: "Warns when a Rust trait accumulates too many members.", HowToFix: "Break the trait into smaller focused contracts.", }, + "design.shallow-module": { + ID: "design.shallow-module", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Shallow module", + Description: "Warns when a module exposes many public symbols but provides little behavior or abstraction depth.", + HowToFix: "Collapse pass-through wrappers, hide internal symbols, or move policy/translation into the abstraction.", + }, + "design.excessive-public-surface": { + ID: "design.excessive-public-surface", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Excessive public surface", + Description: "Warns when one file exposes more public API than the configured local-design threshold.", + HowToFix: "Keep only intentional API public, split broad surfaces, and move implementation details behind private helpers.", + }, + "design.pass-through-abstraction": { + ID: "design.pass-through-abstraction", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Pass-through abstraction", + Description: "Warns when a wrapper delegates directly without policy, validation, translation, or isolation.", + HowToFix: "Remove the wrapper or make the abstraction own a meaningful responsibility.", + }, + "design.configuration-leak": { + ID: "design.configuration-leak", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Configuration leak", + Description: "Warns when config, feature-flag, or environment concerns leak into domain code.", + HowToFix: "Resolve configuration at the boundary and pass domain-specific values into the core model.", + }, + "design.temporal-coupling": { + ID: "design.temporal-coupling", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Temporal coupling", + Description: "Warns when a function relies on an implicit setup-before-action call order.", + HowToFix: "Represent the required state transition with types, constructors, or one operation that preserves the invariant.", + }, + "design.infrastructure-type-leak": { + ID: "design.infrastructure-type-leak", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Infrastructure type leak", + Description: "Warns when framework, database, cloud, or transport types cross into domain/public APIs.", + HowToFix: "Depend on domain-owned interfaces or DTOs and keep infrastructure types in adapters.", + }, + "design.persistence-model-leak": { + ID: "design.persistence-model-leak", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Persistence model leak", + Description: "Warns when ORM/database records appear in public API or handler contracts.", + HowToFix: "Translate persistence records to domain/API models at the boundary.", + }, + "design.domain-logic-in-handler": { + ID: "design.domain-logic-in-handler", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Domain logic in handler", + Description: "Warns when a handler/controller owns business-rule branching instead of orchestrating a domain service.", + HowToFix: "Move business rules into domain/application services and keep handlers focused on transport translation.", + }, } diff --git a/internal/codeguard/rules/catalog_fix_templates.go b/internal/codeguard/rules/catalog_fix_templates.go index 2a9be84..b67c24d 100644 --- a/internal/codeguard/rules/catalog_fix_templates.go +++ b/internal/codeguard/rules/catalog_fix_templates.go @@ -24,7 +24,10 @@ var fixTemplates = mergeFixTemplates( performanceMeasuredFixTemplates, reliabilityFixTemplates, dataFixTemplates, + observabilityFixTemplates, + operationsFixTemplates, changeSafetyFixTemplates, + deliveryFixTemplates, securityFixTemplates, securityLanguageFixTemplates, designFixTemplates, diff --git a/internal/codeguard/rules/catalog_fix_templates_delivery.go b/internal/codeguard/rules/catalog_fix_templates_delivery.go new file mode 100644 index 0000000..fccf0fd --- /dev/null +++ b/internal/codeguard/rules/catalog_fix_templates_delivery.go @@ -0,0 +1,13 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var deliveryFixTemplates = map[string]core.FixTemplate{ + "delivery.missing-rollback-strategy": {Kind: guided, Text: "Add rollback evidence next to the rollout or migration.\n\nBefore:\n# deploy production\nkubectl apply -f deploy/app.yaml\n\nAfter:\n# deploy production\nkubectl apply -f deploy/app.yaml\n# rollback: kubectl rollout undo deployment/app --namespace production\n# verify: curl -fsS https://service.example.com/health"}, + "delivery.unsafe-migration-order": {Kind: guided, Text: "Convert the destructive migration into an expand/backfill/contract rollout.\n\nBefore:\nALTER TABLE users DROP COLUMN legacy_email;\n\nAfter:\n-- release 1: add replacement column and dual-write\n-- release 2: backfill and switch readers\n-- release 3: drop legacy_email after rollback window and backup verification"}, + "delivery.high-risk-change-without-kill-switch": { + Kind: guided, + Text: "Gate the high-risk behavior behind an operational flag or kill switch.\n\nBefore:\nchargeCustomer(order)\n\nAfter:\nif flags.Enabled(ctx, \"new_checkout_charge\") {\n\tchargeCustomer(order)\n} else {\n\tchargeCustomerLegacy(order)\n}\n// document how to disable the flag during rollout", + }, + "delivery.missing-post-deploy-verification": {Kind: guided, Text: "Add a post-deploy verification step to the rollout workflow.\n\nBefore:\n- run: kubectl apply -f deploy/app.yaml\n\nAfter:\n- run: kubectl apply -f deploy/app.yaml\n- run: curl -fsS https://service.example.com/health\n# or call the service smoke/synthetic test used by your operations team"}, +} diff --git a/internal/codeguard/rules/catalog_fix_templates_design.go b/internal/codeguard/rules/catalog_fix_templates_design.go index c6ae178..0c8c4c1 100644 --- a/internal/codeguard/rules/catalog_fix_templates_design.go +++ b/internal/codeguard/rules/catalog_fix_templates_design.go @@ -46,4 +46,12 @@ var designFixTemplates = map[string]core.FixTemplate{ "design.rust.generic-module-name": {Kind: guided, Text: "Rename the Rust module after the responsibility it owns.\n\nBefore:\n// src/utils.rs\n\nAfter:\n// src/date_format.rs"}, "design.rust.max-methods-per-type": {Kind: guided, Text: "Split the Rust type across smaller collaborators, traits, or modules.\n\nBefore:\nstruct Service;\nimpl Service {\n // auth, billing, and reporting methods all live here\n}\n\nAfter:\nstruct AuthService;\nstruct BillingService;\nstruct ReportService;"}, "design.rust.max-trait-members": {Kind: guided, Text: "Break the Rust trait into smaller focused contracts and accept only the capability each consumer needs.\n\nBefore:\ntrait Store {\n fn get_user(&self, id: Id) -> User;\n fn save_user(&self, user: User);\n fn get_order(&self, id: Id) -> Order;\n fn save_order(&self, order: Order);\n}\n\nAfter:\ntrait UserStore {\n fn get_user(&self, id: Id) -> User;\n fn save_user(&self, user: User);\n}\n\ntrait OrderStore {\n fn get_order(&self, id: Id) -> Order;\n fn save_order(&self, order: Order);\n}"}, + "design.shallow-module": {Kind: guided, Text: "Either remove the shallow wrapper or give it real responsibility.\n\nBefore:\nexport function save(x) { return repo.save(x); }\n\nAfter:\n// call repo.save directly, or move validation/translation/policy into save"}, + "design.excessive-public-surface": {Kind: guided, Text: "Reduce public API to the smallest intentional surface.\n\nBefore:\n// one module exports every helper and type\n\nAfter:\n// export the public facade only; keep implementation helpers private or split into focused modules"}, + "design.pass-through-abstraction": {Kind: guided, Text: "Remove pass-through layers that do not add policy, validation, translation, or isolation.\n\nBefore:\nfunc Save(o Order) error { return store.Save(o) }\n\nAfter:\n// call store.Save directly, or make Save own validation/translation before delegating"}, + "design.configuration-leak": {Kind: guided, Text: "Resolve environment/configuration at the application boundary and pass domain-specific values inward.\n\nBefore:\n// domain code reads process.env or accepts Config\n\nAfter:\n// bootstrap reads config, then passes CurrencyPolicy or RetryLimit to the domain"}, + "design.temporal-coupling": {Kind: guided, Text: "Represent setup-before-use requirements with types or constructors.\n\nBefore:\nclient.Init(); client.Send(msg)\n\nAfter:\nclient := NewStartedClient(cfg)\nclient.Send(msg)"}, + "design.infrastructure-type-leak": {Kind: guided, Text: "Keep framework/database/cloud types in adapters and expose domain-owned contracts.\n\nBefore:\nfunc Authorize(ctx *gin.Context, db *sql.DB)\n\nAfter:\nfunc Authorize(ctx Context, users UserStore)"}, + "design.persistence-model-leak": {Kind: guided, Text: "Translate persistence records before they cross API or handler boundaries.\n\nBefore:\nreturn UserRecord{ID: row.ID}\n\nAfter:\nreturn UserDTO{ID: row.ID} // mapping stays at the boundary"}, + "design.domain-logic-in-handler": {Kind: guided, Text: "Keep handlers focused on transport concerns and move business rules into services.\n\nBefore:\n// handler calculates discounts, updates inventory, and writes records\n\nAfter:\nresult := checkoutService.Submit(command)\n// handler maps request/response only"}, } diff --git a/internal/codeguard/rules/catalog_fix_templates_misc.go b/internal/codeguard/rules/catalog_fix_templates_misc.go index 0ac4d8e..cfc4425 100644 --- a/internal/codeguard/rules/catalog_fix_templates_misc.go +++ b/internal/codeguard/rules/catalog_fix_templates_misc.go @@ -13,6 +13,7 @@ var miscFixTemplates = map[string]core.FixTemplate{ "supply_chain.unpinned-dependency": {Kind: deterministic, Text: "Pin the dependency to an exact reviewed version or digest.\n\nBefore:\n\"left-pad\": \"*\"\n\nAfter:\n\"left-pad\": \"1.3.0\"\n// commit the matching lockfile update in the same change"}, "supply_chain.missing-lockfile": {Kind: deterministic, Text: "Generate the lockfile for the manifest and commit them together.\n\nBefore:\n$ git ls-files\npackage.json # no package-lock.json\n\nAfter:\n$ npm install --package-lock-only\n$ git add package.json package-lock.json"}, "supply_chain.lockfile-drift": {Kind: deterministic, Text: "Regenerate the lockfile from the updated manifest and commit both files together.\n\nBefore:\n// package.json bumps express to ^4.19.0; package-lock.json still resolves 4.17.1\n\nAfter:\n$ npm install\n$ git add package.json package-lock.json"}, + "supply_chain.missing-provenance": {Kind: guided, Text: "Add provenance or attestation generation to the artifact workflow.\n\nBefore:\n- uses: docker/build-push-action@v6\n with:\n push: true\n\nAfter:\n- uses: docker/build-push-action@v6\n with:\n push: true\n- uses: actions/attest-build-provenance@v2\n with:\n subject-name: ghcr.io/acme/service\n subject-digest: ${{ steps.build.outputs.digest }}"}, "supply_chain.denied-license": {Kind: guided, Text: "Replace the dependency with an allowed-license alternative, or record an approved policy exception.\n\nBefore:\n// dependency \"copyleft-lib\" resolves to GPL-3.0, which the policy denies\n\nAfter:\n$ npm remove copyleft-lib && npm install permissive-lib\n// or, if the exception is intentional and approved, add the license to the configured allowlist"}, "supply_chain.vulnerable-dependency": {Kind: guided, Text: "Upgrade the dependency to a version outside the advisory's affected range, regenerate its lockfile, and refresh the approved local advisory cache.\n\nBefore:\n\"library\": \"1.2.3\" // affected by an advisory\n\nAfter:\n\"library\": \"1.2.4\" // fixed version\n// commit the matching lockfile update"}, "supply_chain.cargo.missing-package-license": { @@ -31,6 +32,8 @@ var miscFixTemplates = map[string]core.FixTemplate{ "ci.required-workflow-dir": {Kind: guided, Text: "Add the required workflow directory with a real workflow, or disable the policy explicitly.\n\nBefore:\n$ ls .github\n# no workflows directory\n\nAfter:\n$ mkdir -p .github/workflows\n$ git add .github/workflows/ci.yml"}, "ci.required-file": {Kind: guided, Text: "Add the required file at the configured path, or drop the requirement if it no longer applies.\n\nBefore:\n// policy requires .github/workflows/release.yml; the file is missing\n\nAfter:\n// commit .github/workflows/release.yml with the release steps\n// or remove the entry from the configured required files"}, "ci.workflow-content": {Kind: guided, Text: "Add the required step or marker to the workflow file.\n\nBefore:\njobs:\n build:\n steps:\n - run: go build ./...\n\nAfter:\njobs:\n build:\n steps:\n - run: go build ./...\n - run: go test ./... # satisfies the required \"go test\" marker"}, + "ci.missing-required-gate": {Kind: guided, Text: "Add the configured gate as a workflow job or step.\n\nBefore:\njobs:\n build:\n steps:\n - run: go build ./...\n\nAfter:\njobs:\n test:\n steps:\n - run: go test ./...\n build:\n needs: test\n steps:\n - run: go build ./..."}, + "ci.mutable-deployment-reference": {Kind: deterministic, Text: "Replace mutable refs with reviewed immutable references.\n\nBefore:\n- uses: acme/deploy-action@main\nimage: ghcr.io/acme/service:latest\n\nAfter:\n- uses: acme/deploy-action@8f3c2b1a4d5e6f7890abc1234567890abc123456\nimage: ghcr.io/acme/service@sha256:0123456789abcdef..."}, "ci.test-file-location": {Kind: deterministic, Text: "Move the test file under the configured test directory.\n\nBefore:\nsrc/parser_test.py\n\nAfter:\n$ git mv src/parser_test.py tests/parser_test.py\n# or update the CI policy if the layout is intentional"}, "ci.test-without-assertion": {Kind: guided, Text: "Add a real assertion or explicit failure path so the test verifies observable behavior.\n\nBefore:\nfunc TestProcess(t *testing.T) {\n\tProcess(input)\n}\n\nAfter:\nfunc TestProcess(t *testing.T) {\n\tgot := Process(input)\n\tif got != want {\n\t\tt.Fatalf(\"Process() = %v, want %v\", got, want)\n\t}\n}"}, "ci.always-true-test-assertion": {Kind: guided, Text: "Assert on values produced by the code under test instead of constants.\n\nBefore:\nexpect(true).toBe(true);\n\nAfter:\nexpect(parse(\"1,2\")).toEqual([1, 2]);"}, diff --git a/internal/codeguard/rules/catalog_fix_templates_observability.go b/internal/codeguard/rules/catalog_fix_templates_observability.go new file mode 100644 index 0000000..42d83df --- /dev/null +++ b/internal/codeguard/rules/catalog_fix_templates_observability.go @@ -0,0 +1,13 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var observabilityFixTemplates = map[string]core.FixTemplate{ + "observability.unstructured-log": {Kind: guided, Text: "Replace raw print/console logging with structured logging.\n\nBefore:\nconsole.log(\"failed\", err)\n\nAfter:\nlogger.error(\"checkout failed\", { operation: \"checkout\", err })"}, + "observability.error-without-context": {Kind: guided, Text: "Add safe operation/request context to error logs.\n\nBefore:\nlogger.error(err)\n\nAfter:\nlogger.error(\"load order failed\", { operation: \"load_order\", order_id: safeOrderID, err })"}, + "observability.sensitive-log-data": {Kind: guided, Text: "Remove, redact, or hash sensitive values before logging.\n\nBefore:\nlogger.info(\"login\", { token })\n\nAfter:\nlogger.info(\"login\", { token_present: token != \"\" })"}, + "observability.high-cardinality-label": {Kind: guided, Text: "Use bounded metric labels.\n\nBefore:\nrequests_total.WithLabelValues(userID, rawPath)\n\nAfter:\nrequests_total.WithLabelValues(routeTemplate, statusClass)"}, + "observability.critical-path-uninstrumented": {Kind: guided, Text: "Instrument critical entrypoints with tracing, metrics, or structured logs.\n\nAdd a span/metric/log at the handler, job, consumer, or payment boundary and record failures with safe context."}, + "observability.log-and-ignore": {Kind: guided, Text: "Do not log and silently continue unless the failure is explicitly safe.\n\nBefore:\nif err != nil { logger.error(err); return nil }\n\nAfter:\nif err != nil { return fmt.Errorf(\"send receipt: %w\", err) }"}, + "observability.shallow-health-check": {Kind: guided, Text: "Split liveness from readiness and make readiness verify critical dependencies.\n\nBefore:\n/health returns 200 OK unconditionally\n\nAfter:\n/live returns process liveness; /ready checks database, queue, and required downstream readiness with bounded timeouts."}, +} diff --git a/internal/codeguard/rules/catalog_fix_templates_operations.go b/internal/codeguard/rules/catalog_fix_templates_operations.go new file mode 100644 index 0000000..de347f5 --- /dev/null +++ b/internal/codeguard/rules/catalog_fix_templates_operations.go @@ -0,0 +1,8 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var operationsFixTemplates = map[string]core.FixTemplate{ + "operations.missing-owner": {Kind: guided, Text: "Add ownership metadata for production paths.\n\nExamples:\nCODEOWNERS:\n/service/payments/ @org/payments\n\nor service catalog metadata:\nowner: payments-platform"}, + "operations.missing-runbook": {Kind: guided, Text: "Add a runbook for critical systems.\n\nInclude: service overview, dashboards, alerts, deploy verification, common failures, escalation, and rollback steps."}, +} diff --git a/internal/codeguard/rules/catalog_fix_templates_quality.go b/internal/codeguard/rules/catalog_fix_templates_quality.go index 0340df0..0299886 100644 --- a/internal/codeguard/rules/catalog_fix_templates_quality.go +++ b/internal/codeguard/rules/catalog_fix_templates_quality.go @@ -32,4 +32,13 @@ var qualityFixTemplates = map[string]core.FixTemplate{ "quality.javascript.non-null-assertion": {Kind: guided, Text: "Prove nullability with a guard instead of asserting it away with !.\n\nBefore:\nconst name = user!.name;\n\nAfter:\nif (user === null) {\n throw new Error(\"user is required\");\n}\nconst name = user.name;"}, "quality.javascript.ts-expect-error": {Kind: guided, Text: "Fix the underlying type error and delete the @ts-expect-error suppression.\n\nBefore:\n// @ts-expect-error\nconst id = user.id;\n\nAfter:\nif (user === undefined) {\n throw new Error(\"user is required\");\n}\nconst id = user.id;"}, "quality.javascript.debugger-statement": {Kind: deterministic, Text: "Remove the committed debugger statement; use tests or structured logging instead.\n\nBefore:\nfunction onSubmit(data) {\n debugger;\n send(data);\n}\n\nAfter:\nfunction onSubmit(data) {\n send(data);\n}"}, + "quality.duplicated-knowledge": {Kind: guided, Text: "Centralize duplicated business literals or calculations behind one named domain concept.\n\nBefore:\nconst premiumLimit = 1000;\nconst vipLimit = 1000;\n\nAfter:\nconst highValueAccountLimit = 1000;"}, + "quality.ambiguous-name": {Kind: guided, Text: "Rename vague identifiers after the domain concept or role they carry.\n\nBefore:\nfunc process(data Data)\n\nAfter:\nfunc reconcileInvoice(invoice Invoice)"}, + "quality.boolean-argument": {Kind: guided, Text: "Replace behavior-hiding booleans with named operations or explicit options.\n\nBefore:\nrender(order, true)\n\nAfter:\nrenderPreview(order)\n// or render(order, RenderOptions{Preview: true})"}, + "quality.mixed-abstraction-levels": {Kind: guided, Text: "Separate domain orchestration from low-level I/O or serialization.\n\nBefore:\nfunc Checkout() { validateCart(); db.Exec(...); json.Marshal(...) }\n\nAfter:\nfunc Checkout() { command := validateCart(); checkoutStore.Save(command) }"}, + "quality.primitive-obsession": {Kind: guided, Text: "Group raw domain primitives into a value object that names invariants and units.\n\nBefore:\ncharge(customerID string, amount int, currency string)\n\nAfter:\ncharge(customer CustomerID, money Money)"}, + "quality.hidden-side-effect": {Kind: guided, Text: "Rename hidden mutations or split queries/builders from side effects.\n\nBefore:\nfunc buildInvoice() Invoice { repo.Save(...); return invoice }\n\nAfter:\nfunc saveInvoice() (Invoice, error)"}, + "quality.mutable-global-state": {Kind: guided, Text: "Move mutable global state behind an instance or synchronized owner.\n\nBefore:\nvar currentUser User\n\nAfter:\ntype SessionStore struct { currentUser User }"}, + "quality.redundant-comment": {Kind: deterministic, Text: "Delete comments that restate the next line, or replace them with intent/constraint context.\n\nBefore:\n// validate input\nvalidateInput(input)\n\nAfter:\nvalidateInput(input)"}, + "quality.environment-branching": {Kind: guided, Text: "Move environment-specific behavior out of domain code and into configuration or bootstrap wiring.\n\nBefore:\nif os.Getenv(\"ENV\") == \"production\" {\n\tclient = realGateway\n} else {\n\tclient = fakeGateway\n}\n\nAfter:\n// bootstrap/config selects Gateway once\nservice := NewService(configuredGateway)\n// domain code uses the injected Gateway without checking deployment environment"}, } diff --git a/internal/codeguard/rules/catalog_misc.go b/internal/codeguard/rules/catalog_misc.go index 7387059..53da679 100644 --- a/internal/codeguard/rules/catalog_misc.go +++ b/internal/codeguard/rules/catalog_misc.go @@ -75,6 +75,26 @@ var miscCatalog = map[string]core.RuleMetadata{ Description: "Fails when required workflow file markers are absent.", HowToFix: "Update the workflow file so the required steps or markers are present.", }, + "ci.missing-required-gate": { + ID: "ci.missing-required-gate", + Section: "CI/CD", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Missing required CI gate", + Description: "Fails when configured required gates such as test or security checks are absent from CI workflows.", + HowToFix: "Add the required workflow job or step, or update ci_rules.required_gates if the repository uses a different gate name.", + }, + "ci.mutable-deployment-reference": { + ID: "ci.mutable-deployment-reference", + Section: "CI/CD", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Mutable deployment reference", + Description: "Fails when deployment automation uses a mutable GitHub Action branch ref or a container image tagged latest.", + HowToFix: "Pin external actions to an immutable reviewed ref and replace latest image tags with versioned tags or digests.", + }, "ci.test-file-location": { ID: "ci.test-file-location", Section: "CI/CD", diff --git a/internal/codeguard/rules/catalog_observability.go b/internal/codeguard/rules/catalog_observability.go new file mode 100644 index 0000000..9bda057 --- /dev/null +++ b/internal/codeguard/rules/catalog_observability.go @@ -0,0 +1,32 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var observabilityCatalog = map[string]core.RuleMetadata{ + "observability.unstructured-log": observabilityRule("observability.unstructured-log", "warn", "Unstructured log", "Warns when production code writes raw logs without structured context fields.", "Use the repository's structured logger and include stable operation fields instead of raw console/print output."), + "observability.error-without-context": observabilityRule("observability.error-without-context", "warn", "Error log without context", "Warns when an error is logged without operation, request, or safe business context.", "Log the operation name and safe request or resource identifiers; avoid logging secrets or raw payloads."), + "observability.sensitive-log-data": observabilityRule("observability.sensitive-log-data", "fail", "Sensitive log data", "Fails when log calls include sensitive names such as tokens, passwords, authorization headers, cookies, or PII-like fields.", "Remove the value from logs, hash/redact it, or replace it with a non-sensitive correlation identifier."), + "observability.high-cardinality-label": observabilityRule("observability.high-cardinality-label", "warn", "High-cardinality metric label", "Warns when metric labels use identifiers or raw paths that can explode cardinality.", "Use bounded labels such as route templates, operation names, status classes, or stable enum-like dimensions."), + "observability.critical-path-uninstrumented": observabilityRule("observability.critical-path-uninstrumented", "warn", "Critical path without instrumentation", "Warns when handlers, consumers, jobs, or other critical paths lack visible logging, metrics, or tracing evidence.", "Add a span, metric, or structured log at the critical path boundary and record failures with safe context."), + "observability.log-and-ignore": observabilityRule("observability.log-and-ignore", "warn", "Logged and ignored failure", "Warns when code logs a failure and then continues or returns success without surfacing the error.", "Return or aggregate the error, mark the operation as partially failed, or document why the failure is safely ignorable."), + "observability.shallow-health-check": observabilityRule("observability.shallow-health-check", "warn", "Shallow health check", "Warns when a health/readiness endpoint returns a static OK response while dependency evidence exists nearby.", "Make readiness check critical dependencies or split liveness from readiness so deploys verify meaningful service health."), +} + +func observabilityRule(id string, level string, title string, description string, howToFix string) core.RuleMetadata { + return core.RuleMetadata{ + ID: id, + Section: "Observability", + DefaultLevel: level, + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage( + core.RuleLanguageGo, + core.RuleLanguageTypeScript, + core.RuleLanguageJavaScript, + core.RuleLanguagePython, + core.RuleLanguageCPP, + ), + Title: title, + Description: description, + HowToFix: howToFix, + } +} diff --git a/internal/codeguard/rules/catalog_operations.go b/internal/codeguard/rules/catalog_operations.go new file mode 100644 index 0000000..3643173 --- /dev/null +++ b/internal/codeguard/rules/catalog_operations.go @@ -0,0 +1,27 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var operationsCatalog = map[string]core.RuleMetadata{ + "operations.missing-owner": operationsRule("operations.missing-owner", "warn", "Missing service owner", "Warns when critical production code has no CODEOWNERS, service catalog, or configured ownership metadata.", "Add CODEOWNERS or service metadata that maps the service or path to an accountable team."), + "operations.missing-runbook": operationsRule("operations.missing-runbook", "warn", "Missing runbook", "Warns when critical systems lack runbook or operations documentation evidence.", "Add a runbook link or local runbook covering deploy verification, common failures, escalation, and rollback."), +} + +func operationsRule(id string, level string, title string, description string, howToFix string) core.RuleMetadata { + return core.RuleMetadata{ + ID: id, + Section: "Operations", + DefaultLevel: level, + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage( + core.RuleLanguageGo, + core.RuleLanguageTypeScript, + core.RuleLanguageJavaScript, + core.RuleLanguagePython, + core.RuleLanguageCPP, + ), + Title: title, + Description: description, + HowToFix: howToFix, + } +} diff --git a/internal/codeguard/rules/catalog_quality.go b/internal/codeguard/rules/catalog_quality.go index f89254e..d11798f 100644 --- a/internal/codeguard/rules/catalog_quality.go +++ b/internal/codeguard/rules/catalog_quality.go @@ -280,4 +280,101 @@ var qualityCatalog = map[string]core.RuleMetadata{ Description: "Warns when JavaScript code contains committed debugger statements.", HowToFix: "Remove the debugger statement or replace it with structured logging or tests.", }, + "quality.duplicated-knowledge": { + ID: "quality.duplicated-knowledge", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Duplicated business knowledge", + Description: "Warns when domain literals or rules appear duplicated instead of living behind one named concept.", + HowToFix: "Extract the duplicated rule, literal, or calculation into one named domain construct and reuse it.", + }, + "quality.ambiguous-name": { + ID: "quality.ambiguous-name", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Ambiguous identifier", + Description: "Warns when a local identifier uses vague names such as data, manager, helper, process, or thing.", + HowToFix: "Rename the identifier after the domain concept, role, or specific responsibility it represents.", + }, + "quality.boolean-argument": { + ID: "quality.boolean-argument", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Boolean argument", + Description: "Warns when function parameters hide alternate behavior behind boolean flags.", + HowToFix: "Split the behavior into named functions or replace the flag with an explicit options type.", + }, + "quality.mixed-abstraction-levels": { + ID: "quality.mixed-abstraction-levels", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Mixed abstraction levels", + Description: "Warns when one function mixes domain orchestration with low-level infrastructure operations.", + HowToFix: "Extract low-level I/O, serialization, or persistence details behind a helper or adapter.", + }, + "quality.primitive-obsession": { + ID: "quality.primitive-obsession", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Primitive obsession", + Description: "Warns when related domain values are passed around as raw strings, numbers, or booleans.", + HowToFix: "Introduce a domain value object or options struct that names the invariant and units.", + }, + "quality.hidden-side-effect": { + ID: "quality.hidden-side-effect", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Hidden side effect", + Description: "Warns when a function name implies a query or builder but the body mutates state, publishes, writes, or sends.", + HowToFix: "Rename the function to expose the side effect or split query/build behavior from mutation.", + }, + "quality.mutable-global-state": { + ID: "quality.mutable-global-state", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Mutable global state", + Description: "Warns when production code declares mutable module or package state.", + HowToFix: "Move mutable state behind an instance, dependency, or explicitly synchronized owner.", + }, + "quality.redundant-comment": { + ID: "quality.redundant-comment", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo, core.RuleLanguagePython, core.RuleLanguageTypeScript, core.RuleLanguageJavaScript, core.RuleLanguageCPP), + Title: "Redundant comment", + Description: "Warns when a comment repeats nearby code instead of explaining intent, constraints, or tradeoffs.", + HowToFix: "Delete the restatement or replace it with context that is not visible from the code.", + }, + "quality.environment-branching": { + ID: "quality.environment-branching", + Section: "Code Quality", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.FixedRuleLanguageCoverage( + core.RuleLanguageGo, + core.RuleLanguagePython, + core.RuleLanguageTypeScript, + core.RuleLanguageJavaScript, + core.RuleLanguageCPP, + core.RuleLanguageRuby, + ), + Title: "Environment branching", + Description: "Warns when domain or application source code branches on production/staging/development environment instead of isolating that policy in configuration or bootstrap boundaries.", + HowToFix: "Move environment-specific choices to configuration, dependency injection, or bootstrap code and keep domain behavior environment-agnostic.", + }, } diff --git a/internal/codeguard/rules/catalog_supplychain.go b/internal/codeguard/rules/catalog_supplychain.go index b4f6284..ac4aab2 100644 --- a/internal/codeguard/rules/catalog_supplychain.go +++ b/internal/codeguard/rules/catalog_supplychain.go @@ -43,6 +43,16 @@ var supplyChainCatalog = map[string]core.RuleMetadata{ Description: "Fails when a manifest change is not reflected in the associated lockfile.", HowToFix: "Regenerate the lockfile from the updated manifest and commit both files together.", }, + "supply_chain.missing-provenance": { + ID: "supply_chain.missing-provenance", + Section: "Supply Chain", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Missing artifact provenance", + Description: "Fails when artifact build or publish automation exists without provenance, attestation, SBOM, or signing evidence.", + HowToFix: "Add artifact provenance or attestation generation to the build/publish workflow, such as SLSA provenance or GitHub artifact attestations.", + }, "supply_chain.denied-license": { ID: "supply_chain.denied-license", Section: "Supply Chain", diff --git a/internal/codeguard/runner/checks/registry.go b/internal/codeguard/runner/checks/registry.go index 53a928e..a39457b 100644 --- a/internal/codeguard/runner/checks/registry.go +++ b/internal/codeguard/runner/checks/registry.go @@ -91,6 +91,26 @@ var sectionRegistry = []sectionDef{ return dataCheck.Run(ctx, checkEnv) }, }, + { + id: "observability", + name: "Observability", + enabled: func(sc runnersupport.Context) bool { + return sc.Cfg.Checks.Observability != nil && *sc.Cfg.Checks.Observability + }, + run: func(ctx context.Context, _ runnersupport.Context, checkEnv checkSupport.Context) core.SectionResult { + return qualityCheck.RunObservability(ctx, checkEnv) + }, + }, + { + id: "operations", + name: "Operations", + enabled: func(sc runnersupport.Context) bool { + return sc.Cfg.Checks.Operations != nil && *sc.Cfg.Checks.Operations + }, + run: func(ctx context.Context, _ runnersupport.Context, checkEnv checkSupport.Context) core.SectionResult { + return qualityCheck.RunOperations(ctx, checkEnv) + }, + }, { id: "design", name: "Design", @@ -123,6 +143,16 @@ var sectionRegistry = []sectionDef{ return ciCheck.Run(ctx, checkEnv) }, }, + { + id: "delivery", + name: "Delivery", + enabled: func(sc runnersupport.Context) bool { + return sc.Cfg.Checks.Delivery != nil && *sc.Cfg.Checks.Delivery + }, + run: func(ctx context.Context, _ runnersupport.Context, checkEnv checkSupport.Context) core.SectionResult { + return ciCheck.RunDelivery(ctx, checkEnv) + }, + }, { id: "supply-chain", name: "Supply Chain", diff --git a/pkg/codeguard/sdk_types_config_checks.go b/pkg/codeguard/sdk_types_config_checks.go index f89125d..e0545bf 100644 --- a/pkg/codeguard/sdk_types_config_checks.go +++ b/pkg/codeguard/sdk_types_config_checks.go @@ -7,9 +7,12 @@ type CPPToolingConfig = core.CPPToolingConfig type DesignRulesConfig = core.DesignRulesConfig type PromptRulesConfig = core.PromptRulesConfig type CIRulesConfig = core.CIRulesConfig +type DeliveryRulesConfig = core.DeliveryRulesConfig type SupplyChainRulesConfig = core.SupplyChainRulesConfig type ReliabilityRulesConfig = core.ReliabilityRulesConfig type DataRulesConfig = core.DataRulesConfig +type ObservabilityRulesConfig = core.ObservabilityRulesConfig +type OperationsRulesConfig = core.OperationsRulesConfig type ChangeRulesConfig = core.ChangeRulesConfig type ProductionRiskConfig = core.ProductionRiskConfig type ContractRulesConfig = core.ContractRulesConfig diff --git a/tests/checks/change_smells_test.go b/tests/checks/change_smells_test.go index 9f9b2d3..b2955e8 100644 --- a/tests/checks/change_smells_test.go +++ b/tests/checks/change_smells_test.go @@ -24,6 +24,25 @@ func changeSmellQuietConfig(name string, dir string) codeguard.Config { return cfg } +type changeSmellLanguageCase struct { + name string + language string + path string + before string + after string +} + +func runChangeSmellCase(t *testing.T, name string, tc changeSmellLanguageCase) codeguard.Report { + t.Helper() + dir := initChangeRepo(t) + writeFile(t, filepath.Join(dir, tc.path), tc.before) + commitAll(t, dir, "base") + writeFile(t, filepath.Join(dir, tc.path), tc.after) + cfg := changeSmellQuietConfig(name, dir) + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: tc.language}} + return runChangeDiff(t, cfg) +} + func TestChangeOneUseAbstractionDetectsGoInterface(t *testing.T) { dir := initChangeRepo(t) writeFile(t, filepath.Join(dir, "service", "payment.go"), "package service\n\nfunc Charge() error { return nil }\n") @@ -95,6 +114,62 @@ func TestChangeOneUseAbstractionDetectsTypeScriptInterface(t *testing.T) { assertFindingRulePresent(t, report, "Change Safety", "change.one-use-abstraction") } +func TestChangeOneUseAbstractionDetectsAdditionalLanguages(t *testing.T) { + for _, tc := range []changeSmellLanguageCase{ + { + name: "python", + language: "python", + path: "service/payment.py", + before: "def charge():\n return True\n", + after: strings.Join([]string{ + "from typing import Protocol", + "", + "class PaymentGateway(Protocol):", + " def charge(self) -> bool: ...", + "", + "def charge_with(gateway: PaymentGateway):", + " return gateway.charge()", + "", + }, "\n"), + }, + { + name: "javascript", + language: "javascript", + path: "src/billing.js", + before: "export function charge() { return true }\n", + after: strings.Join([]string{ + "export class BillingGateway {", + " charge() { return true }", + "}", + "", + "export function chargeWith(gateway) {", + " return gateway.charge()", + "}", + "", + }, "\n"), + }, + { + name: "cpp", + language: "c++", + path: "service/payment.cpp", + before: "bool Charge() { return true; }\n", + after: strings.Join([]string{ + "class PaymentGateway { public: virtual bool Charge() = 0; };", + "", + "bool ChargeWith(PaymentGateway& gateway) {", + " return gateway.Charge();", + "}", + "", + }, "\n"), + }, + } { + t.Run(tc.name, func(t *testing.T) { + report := runChangeSmellCase(t, "change-one-use-"+tc.name, tc) + assertFindingRulePresent(t, report, "Change Safety", "change.one-use-abstraction") + }) + } +} + func TestChangeDuplicateHelperDetectsGoDuplicate(t *testing.T) { dir := initChangeRepo(t) writeFile(t, filepath.Join(dir, "service", "email.go"), strings.Join([]string{ @@ -211,6 +286,20 @@ func TestChangeComplexityIncreasedDetectsPythonBranchGrowth(t *testing.T) { assertFindingRulePresent(t, report, "Change Safety", "change.complexity-increased") } +func TestChangeComplexityIncreasedAcrossAdditionalLanguages(t *testing.T) { + for _, tc := range []changeSmellLanguageCase{ + {name: "go", language: "go", path: "service/pricing.go", before: "package service\n\nfunc Price(total int) int {\n\treturn total\n}\n", after: "package service\n\nfunc Price(total int, vip bool) int {\n\tif vip { return total - 10 }\n\tif total > 100 { return total - 5 }\n\treturn total\n}\n"}, + {name: "typescript", language: "typescript", path: "src/pricing.ts", before: "export function price(total: number) { return total }\n", after: "export function price(total: number, vip: boolean) {\n if (vip) return total - 10\n if (total > 100) return total - 5\n return total\n}\n"}, + {name: "javascript", language: "javascript", path: "src/pricing.js", before: "export function price(total) { return total }\n", after: "export function price(total, vip) {\n if (vip) return total - 10\n if (total > 100) return total - 5\n return total\n}\n"}, + {name: "cpp", language: "c++", path: "service/pricing.cpp", before: "int Price(int total) { return total; }\n", after: "int Price(int total, bool vip) {\n if (vip) return total - 10;\n if (total > 100) return total - 5;\n return total;\n}\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + report := runChangeSmellCase(t, "change-complexity-"+tc.name, tc) + assertFindingRulePresent(t, report, "Change Safety", "change.complexity-increased") + }) + } +} + func TestChangeComplexityIncreasedAllowsLinearEdit(t *testing.T) { dir := initChangeRepo(t) writeFile(t, filepath.Join(dir, "service", "pricing.go"), "package service\n\nfunc Price(total int) int {\n\treturn total\n}\n") @@ -246,6 +335,20 @@ func TestChangeCleanupRegressionDetectsClaimedCleanupComplexityGrowth(t *testing assertFindingRulePresent(t, report, "Change Safety", "change.cleanup-regression") } +func TestChangeCleanupRegressionAcrossAdditionalLanguages(t *testing.T) { + for _, tc := range []changeSmellLanguageCase{ + {name: "python", language: "python", path: "service/cleanup.py", before: "def route(kind):\n return 'default'\n", after: "def route(kind, admin):\n if admin:\n return 'admin'\n if kind == 'vip':\n return 'vip'\n return 'default'\n"}, + {name: "typescript", language: "typescript", path: "src/cleanup.ts", before: "export function route(kind: string) { return 'default' }\n", after: "export function route(kind: string, admin: boolean) {\n if (admin) return 'admin'\n if (kind === 'vip') return 'vip'\n return 'default'\n}\n"}, + {name: "javascript", language: "javascript", path: "src/cleanup.js", before: "export function route(kind) { return 'default' }\n", after: "export function route(kind, admin) {\n if (admin) return 'admin'\n if (kind === 'vip') return 'vip'\n return 'default'\n}\n"}, + {name: "cpp", language: "c++", path: "service/cleanup.cpp", before: "std::string Route(std::string kind) { return \"default\"; }\n", after: "std::string Route(std::string kind, bool admin) {\n if (admin) return \"admin\";\n if (kind == \"vip\") return \"vip\";\n return \"default\";\n}\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + report := runChangeSmellCase(t, "cleanup-regression-"+tc.name, tc) + assertFindingRulePresent(t, report, "Change Safety", "change.cleanup-regression") + }) + } +} + func TestChangeCleanupRegressionRequiresCleanupClaim(t *testing.T) { dir := initChangeRepo(t) writeFile(t, filepath.Join(dir, "service", "route.go"), "package service\n\nfunc Route(kind string) string {\n\treturn \"default\"\n}\n") diff --git a/tests/checks/ci_test.go b/tests/checks/ci_test.go index 7bafe59..8f438ff 100644 --- a/tests/checks/ci_test.go +++ b/tests/checks/ci_test.go @@ -81,6 +81,53 @@ func TestCICheckFailsWhenWorkflowContentIsMissing(t *testing.T) { assertSectionStatus(t, report, "CI/CD", "fail") } +func TestCIMissingRequiredGate(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".github", "workflows", "ci.yml"), "name: ci\njobs:\n build:\n steps:\n - uses: actions/checkout@v4\n - run: go build ./...\n") + + cfg := ciSafetyTestConfig(dir, "ci-missing-gate") + cfg.Checks.CIRules.RequiredGates = []string{"test"} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "CI/CD", "ci.missing-required-gate") +} + +func TestCIMutableDeploymentReference(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".github", "workflows", "deploy.yml"), "name: deploy\njobs:\n prod:\n steps:\n - uses: acme/deploy-action@main\n - run: docker run ghcr.io/acme/service:latest\n") + + cfg := ciSafetyTestConfig(dir, "ci-mutable-ref") + cfg.Checks.CIRules.RequiredWorkflowFiles = []string{".github/workflows/deploy.yml"} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "CI/CD", "ci.mutable-deployment-reference") +} + +func TestCIMutableDeploymentReferenceAllowsVersionedRefs(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".github", "workflows", "deploy.yml"), "name: deploy\njobs:\n prod:\n steps:\n - uses: actions/checkout@v4\n - run: docker run ghcr.io/acme/service:v1.2.3\n") + + cfg := ciSafetyTestConfig(dir, "ci-pinned-ref") + cfg.Checks.CIRules.RequiredWorkflowFiles = []string{".github/workflows/deploy.yml"} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + if messages := ciRuleMessages(report, "ci.mutable-deployment-reference"); len(messages) != 0 { + t.Fatalf("unexpected mutable reference findings: %v", messages) + } +} + func TestCICheckAllowsRuleOverride(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "buildkite.yml"), "steps: []\n") @@ -111,6 +158,42 @@ func TestCICheckAllowsRuleOverride(t *testing.T) { assertSectionStatus(t, report, "CI/CD", "pass") } +func ciSafetyTestConfig(dir string, name 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 = true + off := false + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + cfg.Checks.CIRules.RequiredWorkflowFiles = []string{".github/workflows/ci.yml"} + cfg.Checks.CIRules.RequiredReleaseFiles = []string{} + cfg.Checks.CIRules.RequiredAutomationPaths = []string{} + cfg.Checks.CIRules.WorkflowContentRules = []codeguard.WorkflowRuleConfig{} + cfg.Checks.CIRules.RequiredGates = []string{} + cfg.Checks.CIRules.AllowedTestPaths = []string{} + return cfg +} + +func ciRuleMessages(report codeguard.Report, ruleID string) []string { + messages := make([]string, 0) + for _, section := range report.Sections { + if section.Name != "CI/CD" { + continue + } + for _, finding := range section.Findings { + if finding.RuleID == ruleID { + messages = append(messages, finding.Message) + } + } + } + return messages +} + func TestCICheckAllowsEmptyReleaseFileOverride(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, ".github", "workflows", "ci.yml"), "name: ci\njobs:\n test:\n steps:\n - uses: actions/checkout@v4\n - run: make codeguard-ci\n") diff --git a/tests/checks/data_multilang_test.go b/tests/checks/data_multilang_test.go index fbbd6ad..c5bd59e 100644 --- a/tests/checks/data_multilang_test.go +++ b/tests/checks/data_multilang_test.go @@ -313,3 +313,143 @@ void HandleMessage(Email& email, Event event) { assertFindingRuleAbsent(t, report, "Data Correctness", "data.cache-without-policy") assertFindingRuleAbsent(t, report, "Data Correctness", "data.exactly-once-assumption") } + +func TestDataDetectsReadModifyWriteRaceAcrossNonGoLanguages(t *testing.T) { + cases := []struct { + name string + language string + file string + source string + }{ + { + name: "python", + language: "python", + file: "counter.py", + source: ` +def increment(repo): + current = repo.query("SELECT count FROM counters WHERE id = 1") + repo.update(current + 1) +`, + }, + { + name: "typescript", + language: "typescript", + file: "counter.ts", + source: ` +async function increment(db) { + const current = await db.counter.findUnique({ where: { id: 1 } }); + await db.counter.update({ data: { count: current.count + 1 } }); +} +`, + }, + { + name: "javascript", + language: "javascript", + file: "counter.js", + source: ` +async function increment(db) { + const current = await db.counter.findUnique({ where: { id: 1 } }); + await db.counter.update({ data: { count: current.count + 1 } }); +} +`, + }, + { + name: "cpp", + language: "cpp", + file: "counter.cpp", + source: ` +void Increment(DB& db) { + auto current = db.findCounter(id); + db.updateCounter(id, current.count + 1); +} +`, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, tc.file), tc.source) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-rmw-"+tc.name, dir, tc.language)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.read-modify-write-race") + }) + } +} + +func TestDataDetectsSideEffectInTransactionAcrossNonGoLanguages(t *testing.T) { + cases := []struct { + name string + language string + file string + source string + }{ + { + name: "python", + language: "python", + file: "transaction.py", + source: ` +def save(repo, bus): + with transaction.atomic(): + repo.save(order) + bus.publish(event) +`, + }, + { + name: "typescript", + language: "typescript", + file: "transaction.ts", + source: ` +async function save(db, bus) { + await db.$transaction(async (tx) => { + await tx.order.create({}); + await bus.publish(event); + }); +} +`, + }, + { + name: "javascript", + language: "javascript", + file: "transaction.js", + source: ` +async function save(db, bus) { + await db.$transaction(async (tx) => { + await tx.order.create({}); + await bus.publish(event); + }); +} +`, + }, + { + name: "cpp", + language: "cpp", + file: "transaction.cpp", + source: ` +void Save(DB& db, Bus& bus) { + auto txn = db.begin_tx(); + repo.save(txn, order); + bus.publish(event); +} +`, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, tc.file), tc.source) + + report, err := codeguard.Run(context.Background(), dataLangConfig("data-side-effect-tx-"+tc.name, dir, tc.language)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Data Correctness", "data.side-effect-in-transaction") + }) + } +} diff --git a/tests/checks/delivery_test.go b/tests/checks/delivery_test.go new file mode 100644 index 0000000..2da810a --- /dev/null +++ b/tests/checks/delivery_test.go @@ -0,0 +1,91 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestDeliveryMissingRollbackStrategyAndPostDeployVerification(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".github", "workflows", "deploy.yml"), "name: deploy\njobs:\n prod:\n steps:\n - run: kubectl apply -f deploy/app.yaml\n") + + cfg := deliveryTestConfig(dir, "delivery-missing-rollback") + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Delivery", "warn") + assertFindingRulePresent(t, report, "Delivery", "delivery.missing-rollback-strategy") + assertFindingRulePresent(t, report, "Delivery", "delivery.missing-post-deploy-verification") +} + +func TestDeliveryUnsafeMigrationOrder(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "migrations", "001_drop_legacy_email.sql"), "ALTER TABLE users DROP COLUMN legacy_email;\n") + + report, err := codeguard.Run(context.Background(), deliveryTestConfig(dir, "delivery-unsafe-migration")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Delivery", "delivery.unsafe-migration-order") +} + +func TestDeliveryHighRiskChangeWithoutKillSwitch(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "payments", "charge.go"), "package payments\n\nfunc Charge(order Order) error {\n\treturn charge(order)\n}\n\ntype Order struct{}\n") + + report, err := codeguard.Run(context.Background(), deliveryTestConfig(dir, "delivery-no-kill-switch")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Delivery", "delivery.high-risk-change-without-kill-switch") +} + +func TestDeliveryHighRiskCPlusPlusChangeWithoutKillSwitch(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "payments", "charge.cpp"), "namespace payments {\n\nbool Charge(const Order& order) {\n return charge(order);\n}\n\n}\n") + + report, err := codeguard.Run(context.Background(), deliveryTestConfig(dir, "delivery-cpp-no-kill-switch")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Delivery", "delivery.high-risk-change-without-kill-switch") +} + +func TestDeliveryAllowsRollbackVerificationAndKillSwitchEvidence(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".github", "workflows", "deploy.yml"), "name: deploy\njobs:\n prod:\n steps:\n - run: kubectl apply -f deploy/app.yaml\n - run: curl -fsS https://example.invalid/health\n - run: echo rollback via kubectl rollout undo\n") + writeFile(t, filepath.Join(dir, "src", "payments", "charge.go"), "package payments\n\nfunc Charge(order Order, flags Flags) error {\n\tif flags.Enabled(\"new_charge\") {\n\t\treturn charge(order)\n\t}\n\treturn nil\n}\n\ntype Order struct{}\ntype Flags interface { Enabled(string) bool }\n") + + report, err := codeguard.Run(context.Background(), deliveryTestConfig(dir, "delivery-evidence")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Delivery", "pass") +} + +func deliveryTestConfig(dir string, name string) codeguard.Config { + cfg := codeguard.ExampleConfig() + cfg.Name = name + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: "go"}} + off := false + on := true + cfg.Checks.Quality = false + cfg.Checks.Design = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + cfg.Checks.SupplyChain = false + cfg.Checks.Context = &off + cfg.Checks.Delivery = &on + cfg.Cache.Enabled = &off + return cfg +} diff --git a/tests/checks/design_graph_languages_test.go b/tests/checks/design_graph_languages_test.go index 6248a6f..f91f8aa 100644 --- a/tests/checks/design_graph_languages_test.go +++ b/tests/checks/design_graph_languages_test.go @@ -35,6 +35,20 @@ func TestDesignCheckPassesForAcyclicTypeScriptImports(t *testing.T) { assertFindingRuleAbsent(t, report, "Design Patterns", "design.typescript.import-cycle") } +func TestDesignCheckFailsForJavaScriptImportCycle(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "alpha.js"), "import { beta } from \"./beta.js\";\n\nexport const alpha = () => beta();\n") + writeFile(t, filepath.Join(dir, "src", "beta.js"), "import { alpha } from \"./alpha.js\";\n\nexport const beta = () => alpha();\n") + + report, err := codeguard.Run(context.Background(), graphTestConfig("design-js-cycle", dir, "javascript")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Design Patterns", "fail") + assertFindingRulePresent(t, report, "Design Patterns", "design.javascript.import-cycle") +} + func TestDesignCheckFailsForTypeScriptImportCycleThroughTSConfigPaths(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "tsconfig.json"), "{\n // comment to exercise JSONC parsing\n \"compilerOptions\": {\n \"baseUrl\": \".\",\n \"paths\": {\n \"@app/*\": [\"src/*\",],\n },\n },\n}\n") diff --git a/tests/checks/design_local_abstraction_test.go b/tests/checks/design_local_abstraction_test.go new file mode 100644 index 0000000..1cbac89 --- /dev/null +++ b/tests/checks/design_local_abstraction_test.go @@ -0,0 +1,134 @@ +package checks_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func designLocalConfig(dir string, language string) codeguard.Config { + cfg := codeguard.ExampleConfig() + cfg.Name = "design-local-abstraction" + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: language}} + cfg.Checks.Design = true + cfg.Checks.Quality = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + off := false + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + cfg.Checks.DesignRules.MaxDeclsPerFile = 2 + cfg.Checks.DesignRules.MaxInterfaceMethods = 2 + return cfg +} + +func runDesignLocalScan(t *testing.T, cfg codeguard.Config) codeguard.Report { + t.Helper() + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + return report +} + +func TestDesignLocalAbstractionRules(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "pkg", "domain", "service.go"), strings.Join([]string{ + "package domain", + "", + "import \"database/sql\"", + "", + "type Config struct{}", + "type Order struct{}", + "type Repository struct { DB *sql.DB }", + "", + "func PublicA() error {", + "\treturn service.Save()", + "}", + "", + "func PublicB() error {", + "\treturn repo.Save()", + "}", + "", + "func Configure(cfg Config) string {", + "\treturn os.Getenv(\"REGION\")", + "}", + "", + "func SendOrder(client Client) {", + "\tclient.Init()", + "\tclient.Send()", + "}", + }, "\n")) + writeFile(t, filepath.Join(dir, "pkg", "api", "user.go"), strings.Join([]string{ + "package api", + "", + "type UserRecord struct{}", + "func GetUser() UserRecord { return UserRecord{} }", + }, "\n")) + writeFile(t, filepath.Join(dir, "pkg", "api", "order_handler.go"), strings.Join([]string{ + "package api", + "", + "func HandleOrder(order Order) {", + "\tif order.Status == \"paid\" {", + "\t\trepo.Save(order)", + "\t}", + "\tif order.Amount > 1000 {", + "\t\trepo.Update(order)", + "\t}", + "}", + }, "\n")) + + report := runDesignLocalScan(t, designLocalConfig(dir, "go")) + + for _, ruleID := range []string{ + "design.shallow-module", + "design.excessive-public-surface", + "design.pass-through-abstraction", + "design.configuration-leak", + "design.temporal-coupling", + "design.infrastructure-type-leak", + "design.persistence-model-leak", + "design.domain-logic-in-handler", + } { + assertFindingRulePresent(t, report, "Design Patterns", ruleID) + } + assertFindingConfidence(t, report, "Design Patterns", "design.infrastructure-type-leak", "high") +} + +func TestDesignLocalAbstractionAdditionalLanguages(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "domain", "payment.ts"), strings.Join([]string{ + "export function savePayment(req: express.Request) {", + " return service.save(req);", + "}", + "export function publishPayment(client: Client) {", + " client.init();", + " client.send();", + "}", + }, "\n")) + writeFile(t, filepath.Join(dir, "app", "controllers", "order_controller.py"), strings.Join([]string{ + "def handle_order(order):", + " if order.status == 'paid':", + " repo.save(order)", + " if order.amount > 1000:", + " repo.update(order)", + }, "\n")) + writeFile(t, filepath.Join(dir, "include", "api", "user.hpp"), strings.Join([]string{ + "class UserRecord {};", + "UserRecord GetUser();", + }, "\n")) + + tsReport := runDesignLocalScan(t, designLocalConfig(dir, "typescript")) + assertFindingRulePresent(t, tsReport, "Design Patterns", "design.infrastructure-type-leak") + assertFindingRulePresent(t, tsReport, "Design Patterns", "design.temporal-coupling") + + pythonReport := runDesignLocalScan(t, designLocalConfig(dir, "python")) + assertFindingRulePresent(t, pythonReport, "Design Patterns", "design.domain-logic-in-handler") + + cppReport := runDesignLocalScan(t, designLocalConfig(dir, "cpp")) + assertFindingRulePresent(t, cppReport, "Design Patterns", "design.persistence-model-leak") +} diff --git a/tests/checks/maintainability_history_test.go b/tests/checks/maintainability_history_test.go index fee6e4c..e72695c 100644 --- a/tests/checks/maintainability_history_test.go +++ b/tests/checks/maintainability_history_test.go @@ -103,7 +103,7 @@ func TestMaintainabilityHistoryHotspotRulesUseGitEvidence(t *testing.T) { func TestMaintainabilityHistoryUnavailableDoesNotFailScan(t *testing.T) { dir := t.TempDir() - writeFile(t, filepath.Join(dir, "plain.go"), "package sample\n\nfunc Value() int { return 1 }\n") + writeFile(t, filepath.Join(dir, "plain.go"), "package sample\n\nfunc StableRiskScore() int { return 1 }\n") report, err := codeguard.Run(context.Background(), qualityPrecisionConfig(dir)) if err != nil { diff --git a/tests/checks/observability_test.go b/tests/checks/observability_test.go new file mode 100644 index 0000000..2b0758b --- /dev/null +++ b/tests/checks/observability_test.go @@ -0,0 +1,226 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func observabilityConfig(name string, dir string, language string) codeguard.Config { + cfg := codeguard.ExampleConfig() + cfg.Name = name + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: language}} + 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 = &off + cfg.Checks.Change = &off + cfg.Checks.Observability = &on + cfg.Checks.Operations = &off + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + return cfg +} + +func TestObservabilityGoDetectsLoggingAndMetricsRisks(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "handler.go"), `package sample + +import "fmt" + +func CheckoutHandler(userID string, token string, err error) { + fmt.Println("checkout failed", err) + logger.Error(err) + logger.Info("token", token) + requests.WithLabelValues(userID).Inc() +} +`) + + report, err := codeguard.Run(context.Background(), observabilityConfig("observability-go", dir, "go")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Observability", "observability.unstructured-log") + assertFindingRulePresent(t, report, "Observability", "observability.error-without-context") + assertFindingRulePresent(t, report, "Observability", "observability.sensitive-log-data") + assertFindingRulePresent(t, report, "Observability", "observability.high-cardinality-label") +} + +func TestObservabilityDetectsCriticalPathLogIgnoreAndShallowHealth(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "api.py"), `import requests + +def payment_handler(err): + logging.error(err) + return None + +def healthz(): + db = requests.get("http://db") + return "ok" +`) + + report, err := codeguard.Run(context.Background(), observabilityConfig("observability-python", dir, "python")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Observability", "observability.log-and-ignore") + assertFindingRulePresent(t, report, "Observability", "observability.shallow-health-check") + assertFindingRulePresent(t, report, "Observability", "observability.critical-path-uninstrumented") +} + +func TestObservabilityTypeScriptAndCPPDetectors(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "consumer.ts"), `export function orderConsumer(event: Event, request_id: string) { + console.log("received", event) + logger.error("failed") + latency.labels({ request_id }).observe(1) +} +`) + writeFile(t, filepath.Join(dir, "worker.cpp"), `#include + +void auth_worker(const char* password) { + printf("password=%s", password); +} +`) + + tsReport, err := codeguard.Run(context.Background(), observabilityConfig("observability-ts", dir, "typescript")) + if err != nil { + t.Fatalf("run ts: %v", err) + } + assertFindingRulePresent(t, tsReport, "Observability", "observability.unstructured-log") + assertFindingRulePresent(t, tsReport, "Observability", "observability.high-cardinality-label") + + cppReport, err := codeguard.Run(context.Background(), observabilityConfig("observability-cpp", dir, "c++")) + if err != nil { + t.Fatalf("run cpp: %v", err) + } + assertFindingRulePresent(t, cppReport, "Observability", "observability.sensitive-log-data") +} + +func TestObservabilityAllowsStructuredInstrumentedCriticalPath(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "handler.go"), `package sample + +func CheckoutHandler(ctx context.Context, orderID string, err error) { + span := tracer.Start(ctx, "checkout") + defer span.End() + logger.Error("checkout failed", "operation", "checkout", "order", orderID, "err", err) + metrics.Counter("checkout_failed").Inc() +} +`) + + report, err := codeguard.Run(context.Background(), observabilityConfig("observability-safe", dir, "go")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Observability", "observability.error-without-context") + assertFindingRuleAbsent(t, report, "Observability", "observability.critical-path-uninstrumented") +} + +func TestObservabilityDetectorMatrixAcrossLanguages(t *testing.T) { + cases := []observabilityMatrixCase{ + observabilityCase("unstructured", "observability.unstructured-log", map[string]string{ + "go": "package sample\n\nimport \"fmt\"\n\nfunc Run() {\n\tfmt.Println(\"failed\")\n}\n", + "python": "def run():\n print('failed')\n", + "typescript": "export function run() {\n console.log('failed')\n}\n", + "javascript": "export function run() {\n console.log('failed')\n}\n", + "cpp": "#include \n\nvoid Run() {\n std::cout << \"failed\";\n}\n", + }), + observabilityCase("error-context", "observability.error-without-context", map[string]string{ + "go": "package sample\n\nfunc Run(err error) { logger.Error(err) }\n", + "python": "def run(error):\n logging.error(error)\n", + "typescript": "export function run(error: Error) { logger.error(error) }\n", + "javascript": "export function run(error) { logger.error(error) }\n", + "cpp": "#include \n\nvoid Run(const std::exception& error) { std::cerr << error.what(); }\n", + }), + observabilityCase("sensitive", "observability.sensitive-log-data", map[string]string{ + "go": "package sample\n\nfunc Run(token string) { logger.Info(\"token\", token) }\n", + "python": "def run(password):\n logging.info('password=%s', password)\n", + "typescript": "export function run(token: string) { logger.info('token', token) }\n", + "javascript": "export function run(token) { logger.info('token', token) }\n", + "cpp": "#include \n\nvoid Run(const char* password) { printf(\"password=%s\", password); }\n", + }), + observabilityCase("cardinality", "observability.high-cardinality-label", map[string]string{ + "go": "package sample\n\nfunc Run(userID string) { requests.WithLabelValues(userID).Inc() }\n", + "python": "def run(user_id):\n requests.labels(user_id=user_id).inc()\n", + "typescript": "export function run(request_id: string) { latency.labels({ request_id }).observe(1) }\n", + "javascript": "export function run(request_id) { latency.labels({ request_id }).observe(1) }\n", + "cpp": "void Run(std::string user_id) { latency.labels(user_id).Observe(1); }\n", + }), + observabilityCase("critical", "observability.critical-path-uninstrumented", map[string]string{ + "go": "package sample\n\nfunc CheckoutHandler() {}\n", + "python": "def payment_handler():\n return True\n", + "typescript": "export function orderConsumer(event: Event) { return event }\n", + "javascript": "export function orderConsumer(event) { return event }\n", + "cpp": "void Payments::Checkout() {}\n", + }), + observabilityCase("log-ignore", "observability.log-and-ignore", map[string]string{ + "go": "package sample\n\nfunc Run(err error) error {\n\tlogger.Error(err)\n\treturn nil\n}\n", + "python": "def run(error):\n logging.error(error)\n return None\n", + "typescript": "export function run(error: Error) {\n logger.error(error)\n return\n}\n", + "javascript": "export function run(error) {\n logger.error(error)\n return\n}\n", + "cpp": "#include \n\nbool Run(const std::exception& error) {\n std::cerr << error.what();\n return true;\n}\n", + }), + observabilityCase("health", "observability.shallow-health-check", map[string]string{ + "go": "package sample\n\nfunc Healthz() string {\n\tdb.Ping()\n\treturn \"ok\"\n}\n", + "python": "def healthz():\n db.ping()\n return 'ok'\n", + "typescript": "export function healthz(db: DB) {\n db.ping()\n return StatusOK\n}\n", + "javascript": "export function healthz(db) {\n db.ping()\n return StatusOK\n}\n", + "cpp": "std::string Healthz(DB& db) {\n db.Ping();\n return \"ok\";\n}\n", + }), + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for language, source := range tc.sources { + t.Run(language, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, observabilityMatrixFile(language)), source) + + report, err := codeguard.Run(context.Background(), observabilityConfig("observability-"+tc.name+"-"+language, dir, language)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Observability", tc.ruleID) + }) + } + }) + } +} + +type observabilityMatrixCase struct { + name string + ruleID string + sources map[string]string +} + +func observabilityCase(name string, ruleID string, sources map[string]string) observabilityMatrixCase { + return observabilityMatrixCase{name: name, ruleID: ruleID, sources: sources} +} + +func observabilityMatrixFile(language string) string { + switch language { + case "go": + return "handler.go" + case "python": + return "handler.py" + case "typescript": + return "handler.ts" + case "javascript": + return "handler.js" + default: + return "handler.cpp" + } +} diff --git a/tests/checks/operations_test.go b/tests/checks/operations_test.go new file mode 100644 index 0000000..305e735 --- /dev/null +++ b/tests/checks/operations_test.go @@ -0,0 +1,67 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func operationsConfig(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 = &off + cfg.Checks.Change = &off + cfg.Checks.Observability = &off + cfg.Checks.Operations = &on + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + return cfg +} + +func TestOperationsMissingOwnerAndRunbook(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "internal", "payment", "service.go"), `package payment + +func Charge() {} +`) + + report, err := codeguard.Run(context.Background(), operationsConfig("operations-missing", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Operations", "operations.missing-owner") + assertFindingRulePresent(t, report, "Operations", "operations.missing-runbook") +} + +func TestOperationsAcceptsOwnerAndRunbookEvidence(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "internal", "payment", "service.go"), `package payment + +func Charge() {} +`) + writeFile(t, filepath.Join(dir, "CODEOWNERS"), `/internal/payment/ @example/payments +`) + writeFile(t, filepath.Join(dir, "docs", "runbooks", "payment.md"), `# Payment runbook +`) + + report, err := codeguard.Run(context.Background(), operationsConfig("operations-owned", dir)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Operations", "operations.missing-owner") + assertFindingRuleAbsent(t, report, "Operations", "operations.missing-runbook") +} diff --git a/tests/checks/quality_environment_branching_test.go b/tests/checks/quality_environment_branching_test.go new file mode 100644 index 0000000..5be00de --- /dev/null +++ b/tests/checks/quality_environment_branching_test.go @@ -0,0 +1,66 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestQualityEnvironmentBranching(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "billing", "gateway.go"), "package billing\n\nimport \"os\"\n\nfunc Gateway() string {\n\tif os.Getenv(\"ENV\") == \"production\" {\n\t\treturn \"stripe\"\n\t}\n\treturn \"sandbox\"\n}\n") + + report, err := codeguard.Run(context.Background(), qualityEnvironmentTestConfig(dir, "environment-branching")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Code Quality", "quality.environment-branching") +} + +func TestQualityEnvironmentBranchingAllowsBootstrapConfig(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config", "gateway.go"), "package config\n\nimport \"os\"\n\nfunc Gateway() string {\n\tif os.Getenv(\"ENV\") == \"production\" {\n\t\treturn \"stripe\"\n\t}\n\treturn \"sandbox\"\n}\n") + + report, err := codeguard.Run(context.Background(), qualityEnvironmentTestConfig(dir, "environment-branching-config")) + if err != nil { + t.Fatalf("run: %v", err) + } + + if messages := qualityRuleMessages(report, "quality.environment-branching"); len(messages) != 0 { + t.Fatalf("unexpected environment branching findings: %v", messages) + } +} + +func qualityEnvironmentTestConfig(dir string, name string) codeguard.Config { + cfg := codeguard.ExampleConfig() + cfg.Name = name + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: "go"}} + off := false + cfg.Checks.Quality = true + cfg.Checks.Design = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + cfg.Checks.SupplyChain = false + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + return cfg +} + +func qualityRuleMessages(report codeguard.Report, ruleID string) []string { + messages := make([]string, 0) + for _, section := range report.Sections { + if section.Name != "Code Quality" { + continue + } + for _, finding := range section.Findings { + if finding.RuleID == ruleID { + messages = append(messages, finding.Message) + } + } + } + return messages +} diff --git a/tests/checks/quality_local_design_test.go b/tests/checks/quality_local_design_test.go new file mode 100644 index 0000000..fa0a01f --- /dev/null +++ b/tests/checks/quality_local_design_test.go @@ -0,0 +1,70 @@ +package checks_test + +import ( + "path/filepath" + "strings" + "testing" +) + +func TestQualityLocalDesignRules(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "local.go"), strings.Join([]string{ + "package sample", + "", + "var CurrentUser string", + "", + "const PremiumLimit = 1000", + "const VIPLimit = 1000", + "", + "// validate input", + "func validateInput(input string) {}", + "", + "func process(data string, active bool, customerID string, orderStatus string, currency string) string {", + "\ttmp := data", + "\trows.Query()", + "\tsaveOrder()", + "\treturn tmp", + "}", + "", + "func buildInvoice(customerID string) string {", + "\trepo.Save(customerID)", + "\treturn customerID", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + + for _, ruleID := range []string{ + "quality.duplicated-knowledge", + "quality.ambiguous-name", + "quality.boolean-argument", + "quality.mixed-abstraction-levels", + "quality.primitive-obsession", + "quality.hidden-side-effect", + "quality.mutable-global-state", + "quality.redundant-comment", + } { + assertFindingRulePresent(t, report, "Code Quality", ruleID) + assertFindingLevel(t, report, "Code Quality", ruleID, "warn") + } +} + +func TestQualityLocalDesignRulesForScriptLanguages(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "local.ts"), strings.Join([]string{ + "let currentUser = '';", + "", + "export function buildInvoice(data: string, active: boolean, customerId: string, orderStatus: string, currency: string): string {", + " repo.save(customerId);", + " return customerId;", + "}", + }, "\n")) + + cfg := qualityPrecisionConfig(dir) + cfg.Targets[0].Language = "typescript" + report := runQualityPrecisionScan(t, cfg) + + assertFindingRulePresent(t, report, "Code Quality", "quality.mutable-global-state") + assertFindingRulePresent(t, report, "Code Quality", "quality.boolean-argument") + assertFindingRulePresent(t, report, "Code Quality", "quality.hidden-side-effect") +} diff --git a/tests/checks/refactor_test.go b/tests/checks/refactor_test.go index 0f335c0..3c37d09 100644 --- a/tests/checks/refactor_test.go +++ b/tests/checks/refactor_test.go @@ -70,6 +70,23 @@ func Process(repo Repo, bus Bus, user User) error { assertFindingRulePresent(t, report, "Change Safety", "refactor.side-effect-order-changed") } +func TestRefactorDetectsBehaviorErrorAndSideEffectOrderAcrossNonGoLanguages(t *testing.T) { + for _, tc := range refactorBehaviorCases() { + t.Run(tc.name, func(t *testing.T) { + dir := initChangeRepo(t) + writeFile(t, filepath.Join(dir, tc.path), tc.before) + commitAll(t, dir, "base") + writeFile(t, filepath.Join(dir, tc.path), tc.after) + + report := runChangeDiff(t, refactorTestConfig(t, dir, tc.language)) + + assertFindingRulePresent(t, report, "Change Safety", "refactor.behavior-change-detected") + assertFindingRulePresent(t, report, "Change Safety", "refactor.error-path-changed") + assertFindingRulePresent(t, report, "Change Safety", "refactor.side-effect-order-changed") + }) + } +} + func TestRefactorDetectsPublicContractAndVisibilityExpansion(t *testing.T) { dir := initChangeRepo(t) writeFile(t, filepath.Join(dir, "pkg", "client", "api.go"), `package client @@ -129,6 +146,13 @@ func TestRefactorDetectsDependencyDirectionWorsenedAcrossLanguages(t *testing.T) before string after string }{ + { + name: "go", + language: "go", + path: "internal/domain/order.go", + before: "package domain\n\nfunc Total(value int) int {\n\treturn value\n}\n", + after: "package domain\n\nimport \"net/http\"\n\nfunc Total(value int) int {\n\t_ = http.DefaultClient\n\treturn value\n}\n", + }, { name: "python", language: "python", @@ -276,3 +300,44 @@ func refactorTestConfig(t *testing.T, dir string, language string) codeguard.Con cfg.Checks.ChangeRules.DetectNondeterministicDomain = boolValue(false) return cfg } + +type refactorBehaviorCase struct { + name string + language string + path string + before string + after string +} + +func refactorBehaviorCases() []refactorBehaviorCase { + return []refactorBehaviorCase{ + { + name: "python", + language: "python", + path: "app/refactor_processor.py", + before: "def process(repo, bus, allowed):\n if not allowed:\n raise Exception('denied')\n repo.save()\n bus.publish('saved')\n return True\n", + after: "def process(repo, bus, allowed):\n if not allowed:\n return True\n bus.publish('saved')\n repo.save()\n return True\n", + }, + { + name: "typescript", + language: "typescript", + path: "src/refactorProcessor.ts", + before: "export function process(repo, bus, allowed) {\n if (!allowed) { throw new Error('denied') }\n repo.save()\n bus.publish('saved')\n return true\n}\n", + after: "export function process(repo, bus, allowed) {\n if (!allowed) { return true }\n bus.publish('saved')\n repo.save()\n return true\n}\n", + }, + { + name: "javascript", + language: "javascript", + path: "src/refactorProcessor.js", + before: "export function process(repo, bus, allowed) {\n if (!allowed) { throw new Error('denied') }\n repo.save()\n bus.publish('saved')\n return true\n}\n", + after: "export function process(repo, bus, allowed) {\n if (!allowed) { return true }\n bus.publish('saved')\n repo.save()\n return true\n}\n", + }, + { + name: "cpp", + language: "c++", + path: "src/refactor_processor.cpp", + before: "bool Process(Repo& repo, Bus& bus, bool allowed) {\n if (!allowed) { throw std::runtime_error(\"denied\"); }\n repo.Save();\n bus.Publish(\"saved\");\n return true;\n}\n", + after: "bool Process(Repo& repo, Bus& bus, bool allowed) {\n if (!allowed) { return true; }\n bus.Publish(\"saved\");\n repo.Save();\n return true;\n}\n", + }, + } +} diff --git a/tests/checks/reliability_multilang_test.go b/tests/checks/reliability_multilang_test.go index 402e0df..b875b1c 100644 --- a/tests/checks/reliability_multilang_test.go +++ b/tests/checks/reliability_multilang_test.go @@ -241,6 +241,129 @@ function failPayment() { assertFindingRulePresent(t, report, "Reliability", "reliability.recoverable-panic") } +func TestReliabilityDetectsHiddenPartialFailuresAcrossLanguages(t *testing.T) { + for _, tc := range hiddenPartialFailureCases() { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, tc.file), tc.source) + + report, err := codeguard.Run(context.Background(), reliabilityLangConfig("reliability-partial-"+tc.name, dir, tc.language)) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Reliability", "reliability.partial-failure-hidden") + }) + } +} + +type reliabilityLanguageCase struct { + name string + language string + file string + source string +} + +func hiddenPartialFailureCases() []reliabilityLanguageCase { + return []reliabilityLanguageCase{ + { + name: "go", + language: "go", + file: "batch.go", + source: `package sample + +import "log" + +func Process(items []Item) error { + for _, item := range items { + if err := process(item); err != nil { + log.Printf("item error: %v", err) + continue + } + } + return nil +} + +type Item struct{} +func process(Item) error { return nil } +`, + }, + { + name: "python", + language: "python", + file: "batch.py", + source: ` +import logging + +def process_all(items): + for item in items: + try: + process(item) + except Exception as error: + logging.error("item failed: %s", error) + continue + return None +`, + }, + { + name: "typescript", + language: "typescript", + file: "batch.ts", + source: ` +async function processAll(items: Item[]): Promise { + for (const item of items) { + try { + await process(item); + } catch (error) { + console.error("item error", error); + continue; + } + } + return; +} +`, + }, + { + name: "javascript", + language: "javascript", + file: "batch.js", + source: ` +async function processAll(items) { + for (const item of items) { + try { + await process(item); + } catch (error) { + console.warn("item error", error); + continue; + } + } + return; +} +`, + }, + { + name: "cpp", + language: "cpp", + file: "batch.cpp", + source: ` +#include + +bool ProcessAll(const std::vector& items) { + for (const auto& item : items) { + try { + Process(item); + } catch (const std::exception& error) { + std::cerr << "item error: " << error.what(); + continue; + } + } + return true; +} +`, + }, + } +} + func TestReliabilityCPPDetectsUnboundedWorkAndResourceLeak(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "worker.cpp"), ` diff --git a/tests/checks/supplychain_test.go b/tests/checks/supplychain_test.go index f425081..85ded1c 100644 --- a/tests/checks/supplychain_test.go +++ b/tests/checks/supplychain_test.go @@ -150,6 +150,36 @@ func TestSupplyChainFailsForMissingLockfile(t *testing.T) { assertFindingRulePresent(t, report, "Supply Chain", "supply_chain.missing-lockfile") } +func TestSupplyChainMissingProvenance(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "package.json"), `{"dependencies": {"react": "18.2.0"}}`) + writeFile(t, filepath.Join(dir, "package-lock.json"), `{"lockfileVersion": 3, "packages": {"node_modules/react": {"version": "18.2.0"}}}`) + writeFile(t, filepath.Join(dir, ".github", "workflows", "release.yml"), "name: release\njobs:\n publish:\n steps:\n - uses: docker/build-push-action@v6\n with:\n push: true\n") + + report, err := codeguard.Run(context.Background(), supplyChainTestConfig(dir, "missing-provenance")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Supply Chain", "supply_chain.missing-provenance") +} + +func TestSupplyChainAllowsProvenanceEvidence(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "package.json"), `{"dependencies": {"react": "18.2.0"}}`) + writeFile(t, filepath.Join(dir, "package-lock.json"), `{"lockfileVersion": 3, "packages": {"node_modules/react": {"version": "18.2.0"}}}`) + writeFile(t, filepath.Join(dir, ".github", "workflows", "release.yml"), "name: release\njobs:\n publish:\n steps:\n - uses: docker/build-push-action@v6\n with:\n push: true\n - uses: actions/attest-build-provenance@v2\n") + + report, err := codeguard.Run(context.Background(), supplyChainTestConfig(dir, "provenance-evidence")) + if err != nil { + t.Fatalf("run: %v", err) + } + + if messages := supplyChainRuleMessages(report, "supply_chain.missing-provenance"); len(messages) != 0 { + t.Fatalf("unexpected provenance findings: %v", messages) + } +} + func TestSupplyChainFailsForLockfileDriftInDiffMode(t *testing.T) { dir := t.TempDir() runGit(t, dir, "init", "-b", "main") diff --git a/tests/checks/testing_testability_test.go b/tests/checks/testing_testability_test.go index df3f013..fb66c41 100644 --- a/tests/checks/testing_testability_test.go +++ b/tests/checks/testing_testability_test.go @@ -138,6 +138,45 @@ func TestTestingNondeterministicDomainLogicFindsDomainClock(t *testing.T) { assertFindingRulePresent(t, report, "Change Safety", "testing.nondeterministic-domain-logic") } +func TestTestingFailurePathMissingAcrossLanguages(t *testing.T) { + for _, tc := range testabilityFailurePathCases() { + t.Run(tc.name, func(t *testing.T) { + report := runTestabilityCase(t, tc, func(cfg *codeguard.Config) { + cfg.Checks.ChangeRules.DetectBehaviorChangeWithoutTest = boolValue(false) + }) + + assertFindingRulePresent(t, report, "Change Safety", "testing.failure-path-missing") + }) + } +} + +func TestTestingHardwiredDependencyAcrossLanguages(t *testing.T) { + for _, tc := range testabilityHardwiredCases() { + t.Run(tc.name, func(t *testing.T) { + report := runTestabilityCase(t, tc, func(cfg *codeguard.Config) { + cfg.Checks.ChangeRules.DetectBehaviorChangeWithoutTest = boolValue(false) + cfg.Checks.ChangeRules.DetectFailurePathMissing = boolValue(false) + }) + + assertFindingRulePresent(t, report, "Change Safety", "testing.hardwired-dependency") + }) + } +} + +func TestTestingNondeterministicDomainLogicAcrossLanguages(t *testing.T) { + for _, tc := range testabilityNondeterministicCases() { + t.Run(tc.name, func(t *testing.T) { + report := runTestabilityCase(t, tc, func(cfg *codeguard.Config) { + cfg.Checks.ChangeRules.DetectBehaviorChangeWithoutTest = boolValue(false) + cfg.Checks.ChangeRules.DetectFailurePathMissing = boolValue(false) + cfg.Checks.ChangeRules.DetectHardwiredDependency = boolValue(false) + }) + + assertFindingRulePresent(t, report, "Change Safety", "testing.nondeterministic-domain-logic") + }) + } +} + func TestTestingChangeRulesTogglesDisableDetectors(t *testing.T) { dir := testingGitRepo(t) writeFile(t, filepath.Join(dir, "pricing", "price.go"), "package pricing\n\nfunc Price(v int) int {\n\treturn v\n}\n") @@ -167,6 +206,29 @@ func TestTestingLegacyHotspotUncoveredDoesNotEmitWithoutHistory(t *testing.T) { assertFindingRuleAbsent(t, report, "Change Safety", "testing.legacy-hotspot-uncovered") } +func TestTestingLegacyHotspotUncoveredUsesHistoryEvidence(t *testing.T) { + dir := testingGitRepo(t) + path := filepath.Join("legacy", "calculator.py") + writeFile(t, filepath.Join(dir, path), "def calculate(value):\n return value\n") + commitAll(t, dir, "base") + writeFile(t, filepath.Join(dir, path), "def calculate(value):\n adjusted = value + 1\n return adjusted\n") + commitAll(t, dir, "fix calculator adjustment") + writeFile(t, filepath.Join(dir, path), "def calculate(value):\n adjusted = value + 2\n if adjusted > 10:\n return adjusted - 1\n return adjusted\n") + commitAll(t, dir, "bugfix calculator threshold") + writeFile(t, filepath.Join(dir, path), "def calculate(value):\n adjusted = value + 3\n if adjusted > 10:\n return adjusted - 1\n if adjusted < 0:\n return 0\n return adjusted\n") + commitAll(t, dir, "refactor calculator branch") + writeFile(t, filepath.Join(dir, path), "def calculate(value):\n adjusted = value + 4\n if adjusted > 10:\n return adjusted - 2\n if adjusted < 0:\n return 0\n return adjusted\n") + + cfg := testingChangeConfig(t, dir, "python") + cfg.Checks.ChangeRules.DetectBehaviorChangeWithoutTest = boolValue(false) + cfg.Checks.ChangeRules.DetectFailurePathMissing = boolValue(false) + cfg.Checks.ChangeRules.DetectHardwiredDependency = boolValue(false) + cfg.Checks.ChangeRules.DetectNondeterministicDomain = boolValue(false) + report := runTestingChangeScan(t, cfg) + + assertFindingRulePresent(t, report, "Change Safety", "testing.legacy-hotspot-uncovered") +} + func testingChangeConfig(t *testing.T, dir string, language string) codeguard.Config { t.Helper() cfg := codeguard.ExampleConfig() @@ -200,6 +262,58 @@ func runTestingChangeScan(t *testing.T, cfg codeguard.Config) codeguard.Report { return report } +type testabilityCase struct { + name string + language string + path string + before string + after string +} + +func runTestabilityCase(t *testing.T, tc testabilityCase, tune func(*codeguard.Config)) codeguard.Report { + t.Helper() + dir := testingGitRepo(t) + writeFile(t, filepath.Join(dir, tc.path), tc.before) + commitAll(t, dir, "base") + writeFile(t, filepath.Join(dir, tc.path), tc.after) + + cfg := testingChangeConfig(t, dir, tc.language) + if tune != nil { + tune(&cfg) + } + return runTestingChangeScan(t, cfg) +} + +func testabilityFailurePathCases() []testabilityCase { + return []testabilityCase{ + {name: "go", language: "go", path: "domain/payment.go", before: "package domain\n\nfunc Authorize(ok bool) bool { return ok }\n", after: "package domain\n\nimport \"errors\"\n\nfunc Authorize(ok bool) error {\n\tif !ok { return errors.New(\"denied\") }\n\treturn nil\n}\n"}, + {name: "python", language: "python", path: "app/domain/payment.py", before: "def authorize(ok):\n return ok\n", after: "def authorize(ok):\n if not ok:\n raise Exception('denied')\n return True\n"}, + {name: "typescript", language: "typescript", path: "src/domain/payment.ts", before: "export function authorize(ok: boolean) { return ok }\n", after: "export function authorize(ok: boolean) { if (!ok) { throw new Error('denied') } return true }\n"}, + {name: "javascript", language: "javascript", path: "src/domain/payment.js", before: "export function authorize(ok) { return ok }\n", after: "export function authorize(ok) { if (!ok) { throw new Error('denied') } return true }\n"}, + {name: "cpp", language: "c++", path: "src/domain/payment.cpp", before: "bool Authorize(bool ok) { return ok; }\n", after: "#include \n\nbool Authorize(bool ok) { if (!ok) { throw std::runtime_error(\"denied\"); } return true; }\n"}, + } +} + +func testabilityHardwiredCases() []testabilityCase { + return []testabilityCase{ + {name: "go", language: "go", path: "domain/profile.go", before: "package domain\n\nfunc Load() string { return \"ok\" }\n", after: "package domain\n\nimport \"net/http\"\n\nfunc Load() string { http.Get(\"https://example.test\"); return \"ok\" }\n"}, + {name: "python", language: "python", path: "app/domain/profile.py", before: "def load():\n return 'ok'\n", after: "import requests\n\ndef load():\n requests.get('https://example.test')\n return 'ok'\n"}, + {name: "typescript", language: "typescript", path: "src/domain/profile.ts", before: "export function load() { return 'ok' }\n", after: "export function load() { fetch('https://example.test'); return 'ok' }\n"}, + {name: "javascript", language: "javascript", path: "src/domain/profile.js", before: "export function load() { return 'ok' }\n", after: "export function load() { fetch('https://example.test'); return 'ok' }\n"}, + {name: "cpp", language: "c++", path: "src/domain/profile.cpp", before: "std::string Load() { return \"ok\"; }\n", after: "#include \n\nstd::string Load() { std::ifstream file(\"profile.txt\"); return \"ok\"; }\n"}, + } +} + +func testabilityNondeterministicCases() []testabilityCase { + return []testabilityCase{ + {name: "go", language: "go", path: "domain/coupon.go", before: "package domain\n\nfunc IssuedAt() int64 { return 0 }\n", after: "package domain\n\nimport \"time\"\n\nfunc IssuedAt() int64 { return time.Now().Unix() }\n"}, + {name: "python", language: "python", path: "app/domain/coupon.py", before: "def issued_at():\n return 0\n", after: "import datetime\n\ndef issued_at():\n return datetime.datetime.now().timestamp()\n"}, + {name: "typescript", language: "typescript", path: "src/domain/coupon.ts", before: "export function issuedAt() { return 0 }\n", after: "export function issuedAt() { return Date.now() }\n"}, + {name: "javascript", language: "javascript", path: "src/domain/coupon.js", before: "export function issuedAt() { return 0 }\n", after: "export function issuedAt() { return Math.random() }\n"}, + {name: "cpp", language: "c++", path: "src/domain/coupon.cpp", before: "long issued_at() { return 0; }\n", after: "#include \n\nlong issued_at() { return std::chrono::system_clock::now().time_since_epoch().count(); }\n"}, + } +} + func testingGitRepo(t *testing.T) string { t.Helper() dir := t.TempDir() diff --git a/tests/cli/features_metadata_test.go b/tests/cli/features_metadata_test.go index 9ccd0ce..50b95c7 100644 --- a/tests/cli/features_metadata_test.go +++ b/tests/cli/features_metadata_test.go @@ -36,6 +36,22 @@ func TestSDKRuleMetadataForTypeScriptRule(t *testing.T) { assertLanguageCoverage(t, rule, codeguard.RuleLanguageCoverageFixed, codeguard.RuleLanguageTypeScript) } +func TestSDKRuleMetadataForTypeScriptNamedJavaScriptDesignRules(t *testing.T) { + for _, ruleID := range []string{ + "design.typescript.generic-module-name", + "design.typescript.max-methods-per-type", + } { + rule := requireRuleMetadata(t, ruleID) + assertLanguageCoverage( + t, + rule, + codeguard.RuleLanguageCoverageFixed, + codeguard.RuleLanguageJavaScript, + codeguard.RuleLanguageTypeScript, + ) + } +} + func TestSDKRuleMetadataForCommandDrivenRule(t *testing.T) { rule := requireRuleMetadata(t, "security.command-check") assertExecutionModel(t, rule, codeguard.RuleExecutionModelCommandDriven) @@ -134,6 +150,28 @@ func TestSDKRuleMetadataForRefactorRule(t *testing.T) { } } +func TestSDKRuleMetadataForOperabilityAndDeliveryRules(t *testing.T) { + cases := []struct { + ruleID string + }{ + {ruleID: "observability.sensitive-log-data"}, + {ruleID: "operations.missing-runbook"}, + {ruleID: "delivery.missing-rollback-strategy"}, + {ruleID: "design.unreachable-module"}, + {ruleID: "design.stability-direction"}, + } + + for _, tc := range cases { + t.Run(tc.ruleID, func(t *testing.T) { + rule := requireRuleMetadata(t, tc.ruleID) + assertExecutionModel(t, rule, codeguard.RuleExecutionModelLanguageAgnostic) + if rule.FixTemplate.Kind == "" { + t.Fatalf("expected %s to expose a fix template", tc.ruleID) + } + }) + } +} + func TestSDKRuleMetadataForNonExpandContractMigration(t *testing.T) { rule := requireRuleMetadata(t, "contracts.non-expand-contract-migration") assertExecutionModel(t, rule, codeguard.RuleExecutionModelLanguageAgnostic)