From 9111a586c5f006ce8d80a0eccbdcc30fbc324ea4 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Thu, 18 Jun 2026 13:04:51 -0400 Subject: [PATCH] feat: feat: add OWASP-aware security metadata, trust-policy enforcement, and quality-check cleanup --- .claude/knowledge/architecture-boundaries.md | 11 ++ .claude/knowledge/local-dev-setup.md | 7 - .claude/knowledge/testing-patterns.md | 7 + .codeguard/codeguard.yaml | 3 + .github/dependabot.yml | 30 ++++ .github/workflows/release.yml | 33 ++++ .goreleaser.yaml | 31 ++++ README.md | 1 + docs/checks.md | 2 +- docs/security.md | 123 +++++++++++++++ internal/cli/commands.go | 2 + internal/cli/fix.go | 1 + internal/cli/helpers.go | 3 +- internal/cli/info.go | 31 +++- internal/cli/owasp.go | 70 +++++++++ internal/cli/run.go | 1 + internal/cli/scan_flags.go | 39 +++-- internal/codeguard/ai/nlrule/runtime.go | 4 + internal/codeguard/ai/runtime/http_post.go | 3 +- internal/codeguard/ai/runtime/provider.go | 14 +- internal/codeguard/ai/safehttp/safehttp.go | 144 ++++++++++++++++++ internal/codeguard/ai/semantic/runtime.go | 5 + internal/codeguard/ai/triage/http.go | 15 +- internal/codeguard/ai/triage/runtime.go | 37 +++-- .../codeguard/checks/ci/ci_test_quality.go | 2 +- .../checks/ci/ci_test_quality_blocks.go | 17 +++ .../checks/security/security_common.go | 1 + .../checks/security/security_owasp_extra.go | 78 ++++++++++ .../checks/security/security_taint_go.go | 2 +- .../security/security_taint_go_sinks.go | 27 ++++ .../security/security_taint_python_sinks.go | 35 ++++- internal/codeguard/config/io.go | 81 +++++++--- internal/codeguard/config/io_codec.go | 28 ++++ internal/codeguard/config/io_path.go | 5 + internal/codeguard/core/owasp.go | 92 +++++++++++ .../codeguard/core/rule_metadata_types.go | 4 + .../{sarif_helpers.go => sarif_builders.go} | 59 ++----- .../codeguard/report/sarif_result_types.go | 30 ++++ internal/codeguard/report/sarif_rule_types.go | 41 +++++ internal/codeguard/report/sarif_taxonomy.go | 43 ++++++ internal/codeguard/report/write.go | 3 +- internal/codeguard/rules/catalog.go | 5 +- .../codeguard/rules/catalog_security_extra.go | 80 ++++++++++ .../codeguard/rules/catalog_security_owasp.go | 91 +++++++++++ .../codeguard/rules/catalog_security_taint.go | 20 +++ internal/codeguard/runner/support/commands.go | 4 + internal/codeguard/trust/guard.go | 40 +++++ internal/codeguard/trust/trust.go | 87 +++++++++++ internal/githubaction/comment_client.go | 28 ++-- internal/githubaction/comment_client_types.go | 10 ++ pkg/codeguard/sdk_config.go | 16 -- pkg/codeguard/sdk_coverage.go | 20 +++ pkg/codeguard/sdk_rules.go | 19 +++ tests/checks/ci_test_quality_helpers_test.go | 21 +++ tests/checks/security_owasp_extra_test.go | 119 +++++++++++++++ tests/checks/trust_main_test.go | 17 +++ tests/codeguard/trust_main_test.go | 17 +++ tests/security/config_containment_test.go | 57 +++++++ tests/security/owasp_test.go | 92 +++++++++++ tests/security/safehttp_test.go | 89 +++++++++++ tests/security/trust_test.go | 49 ++++++ 61 files changed, 1899 insertions(+), 147 deletions(-) create mode 100644 .claude/knowledge/architecture-boundaries.md delete mode 100644 .claude/knowledge/local-dev-setup.md create mode 100644 .claude/knowledge/testing-patterns.md create mode 100644 .github/dependabot.yml create mode 100644 docs/security.md create mode 100644 internal/cli/owasp.go create mode 100644 internal/codeguard/ai/safehttp/safehttp.go create mode 100644 internal/codeguard/checks/security/security_owasp_extra.go create mode 100644 internal/codeguard/config/io_codec.go create mode 100644 internal/codeguard/config/io_path.go create mode 100644 internal/codeguard/core/owasp.go rename internal/codeguard/report/{sarif_helpers.go => sarif_builders.go} (55%) create mode 100644 internal/codeguard/report/sarif_result_types.go create mode 100644 internal/codeguard/report/sarif_rule_types.go create mode 100644 internal/codeguard/report/sarif_taxonomy.go create mode 100644 internal/codeguard/rules/catalog_security_extra.go create mode 100644 internal/codeguard/rules/catalog_security_owasp.go create mode 100644 internal/codeguard/trust/guard.go create mode 100644 internal/codeguard/trust/trust.go create mode 100644 internal/githubaction/comment_client_types.go create mode 100644 pkg/codeguard/sdk_coverage.go create mode 100644 pkg/codeguard/sdk_rules.go create mode 100644 tests/checks/security_owasp_extra_test.go create mode 100644 tests/checks/trust_main_test.go create mode 100644 tests/codeguard/trust_main_test.go create mode 100644 tests/security/config_containment_test.go create mode 100644 tests/security/owasp_test.go create mode 100644 tests/security/safehttp_test.go create mode 100644 tests/security/trust_test.go diff --git a/.claude/knowledge/architecture-boundaries.md b/.claude/knowledge/architecture-boundaries.md new file mode 100644 index 0000000..efddbb5 --- /dev/null +++ b/.claude/knowledge/architecture-boundaries.md @@ -0,0 +1,11 @@ +# Architecture & System Boundaries + +Key architectural decisions, service boundaries, data flow, integration points, and why things are the way they are. + +- **Repository config is an untrusted input.** codeguard runs in CI on PRs, so `codeguard.yaml`/rule packs are attacker-controllable. Command execution and non-allowlisted AI provider base URLs are gated behind `internal/codeguard/trust` (a process-global `Policy`), off by default. The trust anchor is env vars / CLI flags (`--allow-config-commands`, `--allow-config-ai-endpoints`), never config. See `docs/security.md`. +- **All config-derived command execution must go through `trust.GuardConfigCommand`** before `exec`. Existing chokepoints: `runner/support/commands.go:runCommandCheck`, `ai/runtime/provider.go` (command provider), `ai/nlrule/runtime.go`, `ai/semantic/runtime.go`. codeguard's *own* tool invocations (git, govulncheck, the built-in `node -e` TS runner) are NOT gated — only commands sourced from config. +- **Outbound AI HTTP must use `internal/codeguard/ai/safehttp`**: `ValidateProviderURL` (allowlist of public hosts unless opted in) + `Client` (blocks loopback/private/link-local at dial time) + `MaxResponseBytes`. Triage (`ai/triage`) and the runtime providers (`ai/runtime`) both route through it. Triage merges env-then-config base URL; only the env-sourced value is treated as trusted (`BaseURLFromEnv`). +- **OWASP categories are baked into the rule catalog at var-init time**, not via `init()`. The merged `catalog` var calls `withSecurityOWASP(mergeRuleCatalogs(...))` in `rules/catalog.go`; an `init()` would run *after* the `catalog` var initializer copied stale entries. The mapping lives in `rules/catalog_security_owasp.go`. All read paths (`Catalog`, `RuleCatalogForConfig`, SARIF, CLI) flow through `rules.Catalog()`. +- Config-controlled artifact paths (`baseline.path`, `cache.path`, `ai.cache.path`) are resolved relative to the config dir and contained within it by `config.containConfigArtifactPaths` (in `config/io.go`, `LoadFile`). Paths escaping the config directory are rejected. +- The language-agnostic OWASP-gap line rules (CORS, debug, bind-all, weak-hash/cipher, deserialization, Dockerfile root) live in `checks/security/security_owasp_extra.go` and run per-line via `findingsForFile`. NOTE: `findingsForFile` is the **non-TypeScript** path — `securityTargetFindings` routes TS/JS targets to a separate `typeScriptTargetFindings` pipeline, so these rules do not run on TS/JS targets. String-literal signals (CORS, bind-all) match the raw line; call/identifier signals match the masked line. +- The taint engine emits a single rule ID per language, chosen by sink: `goTaintRuleID`/`pyTaintRuleID` return `security.ssrf.{go,python}` when the sink is an outbound-HTTP callee (`goHTTPSinkArgIndex`/`pyHTTPSinkArgIndex`), else the generic `security.taint.{go,python}`. To add an SSRF sink, add it to the arg-index map; the rule routing and OWASP A10 mapping follow automatically. diff --git a/.claude/knowledge/local-dev-setup.md b/.claude/knowledge/local-dev-setup.md deleted file mode 100644 index 027d538..0000000 --- a/.claude/knowledge/local-dev-setup.md +++ /dev/null @@ -1,7 +0,0 @@ -# Local Development Setup - -How to set up, run, and work with this project locally. Non-obvious dependencies, environment config, common setup issues. - -- This machine has a stale `GOROOT=/Users/alex/apps/go` env var that breaks the homebrew Go toolchain. Run go commands as `env -u GOROOT go ...` (build, test, vet, gofmt). -- When iterating on check/rule logic, delete `.codeguard/cache.json` before self-scanning (`make codeguard-ci`). The scan cache keys on file hash + config hash but NOT the codeguard binary version, so rule-logic changes replay stale per-file findings — cross-file analyses (duplicate-code, import graphs) can appear to report zero findings when they're actually being skipped. -- Cross-file checks (clone detection, dependency graphs) only observe every file via `VisitTargetFiles` (cache-bypassing); `ScanTargetFiles` skips evaluators on cache hits and silently produces empty cross-file state on a second scan. diff --git a/.claude/knowledge/testing-patterns.md b/.claude/knowledge/testing-patterns.md new file mode 100644 index 0000000..fbd1b1d --- /dev/null +++ b/.claude/knowledge/testing-patterns.md @@ -0,0 +1,7 @@ +# Testing Patterns + +Testing strategies, test infrastructure quirks, how to run/debug specific test suites, mocking conventions. + +- The trust policy (`internal/codeguard/trust`) is a process-global. Tests that exercise command-driven checks or local (127.0.0.1) AI endpoints must enable it. `tests/checks` and `tests/codeguard` each have a `TestMain` (`trust_main_test.go`) that sets `Policy{AllowConfigCommands: true, AllowConfigAIEndpoints: true}`. If you add a new test package that runs config commands or hits a loopback test server, add a similar `TestMain` or the tests will fail with "refusing to run config-supplied command" / "ssrf guard" errors. +- `tests/security` deliberately has NO trust `TestMain`: it verifies the secure *default*. Tests there set/restore the policy locally with `trust.Set(...)` + `t.Cleanup(trust.ResetFromEnv)`. Do not add a package-wide opt-in there. +- All tests live under `tests/` as external (`_test`) packages; there are no white-box tests inside `internal/`/`pkg/`. Internal packages are still importable from `tests/` because they are within the module. diff --git a/.codeguard/codeguard.yaml b/.codeguard/codeguard.yaml index a68a1ba..79bfdec 100644 --- a/.codeguard/codeguard.yaml +++ b/.codeguard/codeguard.yaml @@ -12,6 +12,9 @@ waivers: - rule: quality.max-file-lines path: tests/checks/features_test.go reason: consolidated feature coverage is intentionally broad and should still be scanned by other checks + - rule: ci.test-without-assertion + path: tests/**/trust_main_test.go + reason: Go TestMain functions bootstrap package-wide trust policy and are not assertion-bearing tests targets: - name: repository path: .. diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..bb5f4d3 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,30 @@ +version: 2 +updates: + # Go module dependencies. + - package-ecosystem: gomod + directory: "/" + schedule: + interval: weekly + open-pull-requests-limit: 10 + groups: + go-dependencies: + patterns: + - "*" + + # GitHub Actions used by the workflows (pin updates + security fixes). + - package-ecosystem: github-actions + directory: "/" + schedule: + interval: weekly + open-pull-requests-limit: 10 + groups: + github-actions: + patterns: + - "*" + + # Base images used by the release/runtime Dockerfiles. + - package-ecosystem: docker + directory: "/" + schedule: + interval: weekly + open-pull-requests-limit: 5 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 04cc571..5a0ff78 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -25,6 +25,8 @@ on: permissions: contents: write packages: write + # Required for keyless cosign signing and SLSA provenance (OIDC token). + id-token: write jobs: build-release: @@ -32,6 +34,7 @@ jobs: outputs: prerelease: ${{ steps.release.outputs.prerelease }} tag: ${{ steps.release.outputs.tag }} + hashes: ${{ steps.hashes.outputs.hashes }} steps: - name: Resolve release context id: release @@ -141,6 +144,12 @@ jobs: - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 + - name: Install cosign + uses: sigstore/cosign-installer@v3 + + - name: Install syft (SBOM generation) + uses: anchore/sbom-action/download-syft@v0 + - name: Log in to GHCR uses: docker/login-action@v3 with: @@ -156,6 +165,16 @@ jobs: args: release --clean --config .goreleaser.yaml env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COSIGN_YES: "true" + + - name: Compute artifact hashes for provenance + id: hashes + shell: bash + run: | + set -euo pipefail + # The SHA256SUMS file already covers every released artifact. + hashes="$(base64 -w0 dist/SHA256SUMS)" + echo "hashes=$hashes" >>"$GITHUB_OUTPUT" - name: Upload release bundle uses: actions/upload-artifact@v7 @@ -163,6 +182,20 @@ jobs: name: dist-${{ steps.release.outputs.tag }} path: dist/* + # SLSA build provenance for the released artifacts. The generic generator + # produces a signed in-toto attestation over the artifact hashes and uploads + # it to the GitHub release. + provenance: + needs: build-release + permissions: + actions: read + id-token: write + contents: write + uses: slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml@v2.1.0 + with: + base64-subjects: ${{ needs.build-release.outputs.hashes }} + upload-assets: true + sync-homebrew-formula: needs: build-release if: needs.build-release.outputs.prerelease != 'true' diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 0eb657b..9cd8d51 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -30,6 +30,37 @@ archives: checksum: name_template: SHA256SUMS +# Generate a CycloneDX SBOM per archive (requires `syft` on PATH in CI). +sboms: + - id: archive + artifacts: archive + +# Keyless cosign signing of the checksum file. Signing the checksums covers +# every released artifact. Requires `cosign` on PATH and an OIDC token +# (id-token: write) in CI; COSIGN_YES is set in the release workflow. +signs: + - cmd: cosign + certificate: "${artifact}.pem" + signature: "${artifact}.sig" + args: + - sign-blob + - "--output-certificate=${certificate}" + - "--output-signature=${signature}" + - "${artifact}" + - "--yes" + artifacts: checksum + output: true + +# Keyless cosign signing of the published container images. +docker_signs: + - cmd: cosign + artifacts: images + args: + - sign + - "${artifact}" + - "--yes" + output: true + dockers: - id: codeguard-amd64 ids: diff --git a/README.md b/README.md index a4aac3f..84a3700 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,7 @@ func main() { - [Getting started](/Users/alex/Documents/GitHub/codeguard/docs/getting-started.md:1) - [Features](/Users/alex/Documents/GitHub/codeguard/docs/features.md:1) +- [Security & OWASP](/Users/alex/Documents/GitHub/codeguard/docs/security.md:1) - [AI-generated code quality](/Users/alex/Documents/GitHub/codeguard/docs/ai-quality.md:1) - [Agent-native features](/Users/alex/Documents/GitHub/codeguard/docs/agent-native.md:1) - [Integrations](/Users/alex/Documents/GitHub/codeguard/docs/integrations.md:1) diff --git a/docs/checks.md b/docs/checks.md index fcc462d..be40e16 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -176,7 +176,7 @@ codeguard scan -config codeguard.yaml -profile strict ## Rule metadata -SDK and catalog discovery surfaces return both `execution_model` and `language_coverage` for each rule via `codeguard.Rules()`, `codeguard.RulesForConfig(...)`, `codeguard.ExplainRule(...)`, and `codeguard.ExplainRuleForConfig(...)`. +SDK and catalog discovery surfaces return `execution_model`, `language_coverage`, and (for security rules) `owasp_category` for each rule via `codeguard.Rules()`, `codeguard.RulesForConfig(...)`, `codeguard.ExplainRule(...)`, and `codeguard.ExplainRuleForConfig(...)`. The OWASP Top 10 (2021) mapping and per-category coverage are documented in [Security & OWASP](/Users/alex/Documents/GitHub/codeguard/docs/security.md:1) and reported by `codeguard owasp`. `execution_model` values: - `go-native`: built-in logic that currently depends on Go-specific source structure or Go-only integrations diff --git a/docs/security.md b/docs/security.md new file mode 100644 index 0000000..6924015 --- /dev/null +++ b/docs/security.md @@ -0,0 +1,123 @@ +# Security & OWASP + +This page documents codeguard's own trust model and its OWASP Top 10 (2021) +coverage. For the catalogue of checks codeguard runs against *your* repository, +see [Checks reference](/Users/alex/Documents/GitHub/codeguard/docs/checks.md:1). + +## Trust model: repository config is untrusted by default + +codeguard is frequently run in CI against pull requests, including from +untrusted contributors. Its behavior is driven by `codeguard.yaml` and rule +packs, which live in the repository and are therefore controllable by whoever +opens the PR. To prevent a code-review tool from becoming a remote-code-execution +or credential-exfiltration vector, the following capabilities are **disabled by +default** and must be explicitly enabled by the trusted operator (via the +process environment or a CLI flag — never from the repo config itself): + +| Capability | Default | Opt-in env | Opt-in flag | +| --- | --- | --- | --- | +| Run commands defined in config (`*_rules.language_commands`, `license_commands`, `ai.autofix.test_commands`, the `command` AI provider, nlrule/semantic command runtimes) | refused | `CODEGUARD_ALLOW_CONFIG_COMMANDS=1` | `--allow-config-commands` | +| Use an AI provider `baseURL` outside the built-in allowlist, and reach non-public addresses | refused | `CODEGUARD_ALLOW_CONFIG_AI_ENDPOINTS=1` | `--allow-config-ai-endpoints` | + +For a repository you control end-to-end, enable the capabilities you need: + +```bash +codeguard scan --allow-config-commands # run configured commands +CODEGUARD_ALLOW_CONFIG_AI_ENDPOINTS=1 codeguard scan --ai # custom/self-hosted LLM +``` + +### What the defaults protect against + +- **Command injection / RCE in CI (A03 / A08).** Without the opt-in, codeguard + refuses to execute any command supplied by the repository config. +- **Credential exfiltration & SSRF (A10 / A02).** AI provider base URLs from + config are restricted to a small allowlist of known public hosts + (`api.openai.com`, `api.anthropic.com`). The provider HTTP client also refuses + to connect to loopback, private, and link-local addresses (including the cloud + metadata endpoint), defending against DNS-rebinding and redirect-based SSRF. + Provider responses are size-bounded to prevent memory exhaustion. +- **Path traversal / arbitrary file write (A01).** Config-controlled artifact + paths (`baseline.path`, `cache.path`, `ai.cache.path`) are resolved relative + to the config directory and rejected if they escape that directory tree. + +Environment variables are the trust anchor because, in a standard +`pull_request` workflow, the environment is controlled by the workflow author +(base branch), not by the PR. + +## OWASP Top 10 (2021) coverage + +Every built-in security rule is tagged with its OWASP Top 10 (2021) category. +The mapping is surfaced in: + +- `codeguard rules` — appends the category as a trailing column for security rules. +- `codeguard explain ` — an `owasp:` line (text) / `owasp_category` field (`-format agent`). +- SARIF output — each rule carries `properties.tags` (`OWASP:Axx:2021`) and an `owasp` property, which GitHub code scanning surfaces. + +Use the coverage report to see which categories have rules and which are gaps: + +```bash +codeguard owasp # text report +codeguard owasp -format json # machine-readable +``` + +Example: + +``` +OWASP Top 10 (2021) coverage: 8/10 categories have rules + +[ok ] A01:2021-Broken Access Control (2 rules) +[ok ] A02:2021-Cryptographic Failures (11 rules) +[ok ] A03:2021-Injection (24 rules) +[gap ] A04:2021-Insecure Design (0 rules) +[ok ] A05:2021-Security Misconfiguration (4 rules) +[ok ] A06:2021-Vulnerable and Outdated Components (1 rules) +[ok ] A07:2021-Identification and Authentication Failures (1 rules) +[ok ] A08:2021-Software and Data Integrity Failures (1 rules) +[gap ] A09:2021-Security Logging and Monitoring Failures (0 rules) +[ok ] A10:2021-Server-Side Request Forgery (SSRF) (2 rules) +``` + +`A04` (Insecure Design) and `A09` (Security Logging and Monitoring) are left as +explicit gaps: both are design- and operations-level risks that static +heuristics cannot reliably detect, and a false "covered" there would be +misleading. + +### Newly added detection rules + +These heuristic rules close the previously-empty categories. The misconfiguration +and crypto rules are text-based and default to `warn`; the SSRF rules use the +taint engine and default to `fail`. + +| Rule | OWASP | Detects | +| --- | --- | --- | +| `security.cors-wildcard` | A05 | `Access-Control-Allow-Origin: *` | +| `security.debug-enabled` | A05 | framework debug flag enabled (`debug=True`) | +| `security.bind-all-interfaces` | A05 | binding to `0.0.0.0` | +| `security.dockerfile-root` | A05 | Dockerfile `USER root` | +| `security.weak-hash` | A02 | MD5 / SHA-1 used for security | +| `security.weak-cipher` | A02 | DES / RC4 / ECB mode | +| `security.insecure-deserialization` | A08 | `pickle`, unsafe `yaml.load`, Java `readObject`, `Marshal.load`, `unserialize` | +| `security.ssrf.go` / `security.ssrf.python` | A10 | untrusted input flowing into an outbound HTTP request URL | + +## Release integrity (supply chain) + +codeguard's own releases are hardened against tampering: + +- **Signed artifacts** — the `SHA256SUMS` checksum file is signed with keyless + cosign (Sigstore/Fulcio); container images are signed with `cosign sign`. +- **SBOM** — a CycloneDX SBOM is generated per archive via `syft`. +- **SLSA provenance** — a signed in-toto build provenance attestation is produced + over the artifact hashes and attached to the release. +- **Dependabot** — weekly updates for Go modules, GitHub Actions, and Docker base + images keep dependencies current (A06). + +To verify a downloaded release: + +```bash +cosign verify-blob \ + --certificate SHA256SUMS.pem \ + --signature SHA256SUMS.sig \ + --certificate-identity-regexp 'https://github.com/devr-tools/codeguard/.*' \ + --certificate-oidc-issuer https://token.actions.githubusercontent.com \ + SHA256SUMS +``` diff --git a/internal/cli/commands.go b/internal/cli/commands.go index c0eaf2b..0d44c65 100644 --- a/internal/cli/commands.go +++ b/internal/cli/commands.go @@ -75,6 +75,7 @@ func runScan(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) if err := fs.Parse(args); err != nil { return 1 } + flags.applyTrustPolicy() if err := promptScanInputs(*interactive, stdin, stdout, &inputs); err != nil { _, _ = fmt.Fprintf(stderr, "interactive scan: %v\n", err) @@ -161,6 +162,7 @@ func runBaseline(args []string, stdout io.Writer, stderr io.Writer) int { if err := fs.Parse(args); err != nil { return 1 } + flags.applyTrustPolicy() cfg, err := loadConfigWithProfile(*flags.configPath, *flags.profile) if err != nil { diff --git a/internal/cli/fix.go b/internal/cli/fix.go index 5cd7163..1b156a3 100644 --- a/internal/cli/fix.go +++ b/internal/cli/fix.go @@ -24,6 +24,7 @@ func runFix(args []string, stdout io.Writer, stderr io.Writer) int { if err := fs.Parse(args); err != nil { return 1 } + flags.applyTrustPolicy() if !*enableAI { _, _ = fmt.Fprintln(stderr, "fix requires -ai so unverified AI patch generation is never implicit") return 1 diff --git a/internal/cli/helpers.go b/internal/cli/helpers.go index b40ddc0..d6ed0b9 100644 --- a/internal/cli/helpers.go +++ b/internal/cli/helpers.go @@ -31,12 +31,13 @@ Usage: codeguard init [-output codeguard.yaml] [-interactive] [-profile startup|strict|enterprise|ai-safe] codeguard validate [-config codeguard.yaml] [-profile startup|strict|enterprise|ai-safe] codeguard validate-patch [-config codeguard.yaml] [-format text|json|sarif|github] [-profile startup|strict|enterprise|ai-safe] [-ai] < patch.diff - codeguard scan [-config codeguard.yaml] [-mode full|diff] [-base-ref main] [-format text|json|sarif|github] [-interactive] [-profile startup|strict|enterprise|ai-safe] [-ai] + codeguard scan [-config codeguard.yaml] [-mode full|diff] [-base-ref main] [-format text|json|sarif|github] [-interactive] [-profile startup|strict|enterprise|ai-safe] [-ai] [-allow-config-commands] [-allow-config-ai-endpoints] codeguard fix [-config codeguard.yaml] [-mode full|diff] [-base-ref main] [-profile startup|strict|enterprise|ai-safe] [-rule rule.id] [-path rel/path] [-line N] -ai codeguard baseline [-config codeguard.yaml] [-output codeguard-baseline.json] [-mode full|diff] [-base-ref main] [-profile startup|strict|enterprise|ai-safe] codeguard report -slop-history [-config codeguard.yaml] [-limit N] [-profile startup|strict|enterprise|ai-safe] codeguard rules [-config codeguard.yaml] codeguard explain [-config codeguard.yaml] [-format text|agent] + codeguard owasp [-config codeguard.yaml] [-format text|json] [-profile startup|strict|enterprise|ai-safe] codeguard serve --mcp [-config codeguard.yaml] [-profile startup|strict|enterprise|ai-safe] codeguard doctor [-config codeguard.yaml] [-profile startup|strict|enterprise|ai-safe] codeguard profiles diff --git a/internal/cli/info.go b/internal/cli/info.go index fe1b665..59d601b 100644 --- a/internal/cli/info.go +++ b/internal/cli/info.go @@ -29,7 +29,23 @@ func runRules(args []string, stdout io.Writer, stderr io.Writer) int { rules = service.RulesForConfig(cfg) } for _, rule := range rules { - _, _ = fmt.Fprintf(stdout, "%s\t%s\t%s\t%s\t%s\t%s\n", rule.ID, rule.DefaultLevel, rule.ExecutionModel, rule.LanguageCoverage, rule.Section, rule.Title) + var line strings.Builder + line.WriteString(rule.ID) + line.WriteByte('\t') + line.WriteString(string(rule.DefaultLevel)) + line.WriteByte('\t') + line.WriteString(string(rule.ExecutionModel)) + line.WriteByte('\t') + line.WriteString(rule.LanguageCoverage.String()) + line.WriteByte('\t') + line.WriteString(rule.Section) + line.WriteByte('\t') + line.WriteString(rule.Title) + if rule.OWASPCategory != "" { + line.WriteByte('\t') + line.WriteString(string(rule.OWASPCategory)) + } + _, _ = fmt.Fprintln(stdout, line.String()) } return 0 } @@ -90,6 +106,9 @@ func resolveExplainRule(configPath string, profile string, ruleID string) (servi func writeExplainText(stdout io.Writer, rule service.RuleMetadata) { _, _ = fmt.Fprintf(stdout, "%s\ntitle: %s\nsection: %s\nlevel: %s\nexecution model: %s\nlanguage coverage: %s\n%s\n", rule.ID, rule.Title, rule.Section, rule.DefaultLevel, rule.ExecutionModel, rule.LanguageCoverage, rule.Description) + if rule.OWASPCategory != "" { + _, _ = fmt.Fprintf(stdout, "owasp: %s\n", rule.OWASPCategory) + } if strings.TrimSpace(rule.HowToFix) != "" { _, _ = fmt.Fprintf(stdout, "how to fix: %s\n", rule.HowToFix) } @@ -112,6 +131,7 @@ type explainAgentOutput struct { Why string `json:"why"` HowToFix string `json:"how_to_fix"` FixTemplate string `json:"fix_template"` + OWASPCategory string `json:"owasp_category,omitempty"` } type explainLanguageCoverageOutput struct { @@ -130,10 +150,11 @@ func buildExplainAgentOutput(rule service.RuleMetadata) explainAgentOutput { Mode: string(rule.LanguageCoverage.Mode), Languages: explainLanguages(rule.LanguageCoverage.Languages), }, - Description: rule.Description, - Why: rule.Description, - HowToFix: rule.HowToFix, - FixTemplate: rule.FixTemplate, + Description: rule.Description, + Why: rule.Description, + HowToFix: rule.HowToFix, + FixTemplate: rule.FixTemplate, + OWASPCategory: string(rule.OWASPCategory), } } diff --git a/internal/cli/owasp.go b/internal/cli/owasp.go new file mode 100644 index 0000000..5689ee2 --- /dev/null +++ b/internal/cli/owasp.go @@ -0,0 +1,70 @@ +package cli + +import ( + "encoding/json" + "flag" + "fmt" + "io" + "strings" + + service "github.com/devr-tools/codeguard/pkg/codeguard" +) + +// runOWASP prints an OWASP Top 10 (2021) coverage report mapping codeguard's +// security rules to each category and highlighting categories with no rule. +func runOWASP(args []string, stdout io.Writer, stderr io.Writer) int { + fs := flag.NewFlagSet("owasp", flag.ContinueOnError) + fs.SetOutput(stderr) + configPath := fs.String("config", "", "optional config path to include custom rule packs") + profile := fs.String("profile", "", "optional policy profile override") + format := fs.String("format", "text", "output format: text or json") + if err := fs.Parse(args); err != nil { + return 1 + } + + coverage := service.OWASPCoverage() + if strings.TrimSpace(*configPath) != "" { + cfg, err := loadConfigWithProfile(*configPath, *profile) + if err != nil { + _, _ = fmt.Fprintf(stderr, "load config: %v\n", err) + return 1 + } + coverage = service.OWASPCoverageForConfig(cfg) + } + + switch strings.TrimSpace(*format) { + case "", "text": + writeOWASPText(stdout, coverage) + case "json": + encoder := json.NewEncoder(stdout) + encoder.SetIndent("", " ") + if err := encoder.Encode(coverage); err != nil { + _, _ = fmt.Fprintf(stderr, "write owasp output: %v\n", err) + return 1 + } + default: + _, _ = fmt.Fprintf(stderr, "invalid owasp format %q\n", *format) + return 1 + } + return 0 +} + +func writeOWASPText(stdout io.Writer, coverage []service.OWASPCoverageEntry) { + covered := 0 + for _, entry := range coverage { + if entry.Covered { + covered++ + } + } + _, _ = fmt.Fprintf(stdout, "OWASP Top 10 (2021) coverage: %d/%d categories have rules\n\n", covered, len(coverage)) + for _, entry := range coverage { + marker := "gap " + if entry.Covered { + marker = "ok " + } + _, _ = fmt.Fprintf(stdout, "[%s] %s (%d rules)\n", marker, entry.Category, len(entry.RuleIDs)) + for _, id := range entry.RuleIDs { + _, _ = fmt.Fprintf(stdout, " - %s\n", id) + } + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go index f8ff5b3..bc14050 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -15,6 +15,7 @@ var commandCatalog = map[string]commandRunner{ "explain": withoutStdin(runExplain), "fix": withoutStdin(runFix), "init": runInit, + "owasp": withoutStdin(runOWASP), "profiles": noArgs(runProfiles), "report": withoutStdin(runReport), "rules": withoutStdin(runRules), diff --git a/internal/cli/scan_flags.go b/internal/cli/scan_flags.go index 0df0910..45e63ec 100644 --- a/internal/cli/scan_flags.go +++ b/internal/cli/scan_flags.go @@ -3,23 +3,44 @@ package cli import ( "flag" + "github.com/devr-tools/codeguard/internal/codeguard/trust" service "github.com/devr-tools/codeguard/pkg/codeguard" ) // scanRunFlags bundles the flag values shared by commands that execute a scan -// against a config: the config path, scan mode, diff base ref, and profile. +// against a config: the config path, scan mode, diff base ref, profile, and the +// trust opt-ins that unlock config-driven command execution and custom AI +// endpoints (disabled by default; see internal/codeguard/trust). type scanRunFlags struct { - configPath *string - mode *string - baseRef *string - profile *string + configPath *string + mode *string + baseRef *string + profile *string + allowCommands *bool + allowAIEndpoints *bool } func registerScanRunFlags(fs *flag.FlagSet) scanRunFlags { return scanRunFlags{ - configPath: fs.String("config", service.DefaultConfigPath(), "config file or directory path"), - mode: fs.String("mode", string(service.ScanModeFull), "scan mode: full or diff"), - baseRef: fs.String("base-ref", "main", "base branch/ref for diff mode"), - profile: fs.String("profile", "", "optional policy profile override"), + configPath: fs.String("config", service.DefaultConfigPath(), "config file or directory path"), + mode: fs.String("mode", string(service.ScanModeFull), "scan mode: full or diff"), + baseRef: fs.String("base-ref", "main", "base branch/ref for diff mode"), + profile: fs.String("profile", "", "optional policy profile override"), + allowCommands: fs.Bool("allow-config-commands", false, "trust the repository config to run shell commands (off by default; only enable for trusted repos)"), + allowAIEndpoints: fs.Bool("allow-config-ai-endpoints", false, "trust the repository config to set non-allowlisted AI provider base URLs (off by default)"), } } + +// applyTrustPolicy merges the trust opt-in flags with any environment defaults +// and installs the resulting policy. A flag only ever widens trust; it never +// disables a capability the environment already enabled. +func (f scanRunFlags) applyTrustPolicy() { + p := trust.FromEnv() + if f.allowCommands != nil && *f.allowCommands { + p.AllowConfigCommands = true + } + if f.allowAIEndpoints != nil && *f.allowAIEndpoints { + p.AllowConfigAIEndpoints = true + } + trust.Set(p) +} diff --git a/internal/codeguard/ai/nlrule/runtime.go b/internal/codeguard/ai/nlrule/runtime.go index 202fa3f..a224a50 100644 --- a/internal/codeguard/ai/nlrule/runtime.go +++ b/internal/codeguard/ai/nlrule/runtime.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/devr-tools/codeguard/internal/codeguard/core" + "github.com/devr-tools/codeguard/internal/codeguard/trust" ) type disabledRuntime struct{} @@ -55,6 +56,9 @@ func (runtime commandRuntime) Fingerprint() string { } func (runtime commandRuntime) Evaluate(ctx context.Context, request EvaluationRequest) (EvaluationResponse, error) { + if err := trust.GuardConfigCommand("nlrule runtime", runtime.command); err != nil { + return EvaluationResponse{}, err + } payload, err := json.Marshal(request) if err != nil { return EvaluationResponse{}, err diff --git a/internal/codeguard/ai/runtime/http_post.go b/internal/codeguard/ai/runtime/http_post.go index 330b928..05b6ce3 100644 --- a/internal/codeguard/ai/runtime/http_post.go +++ b/internal/codeguard/ai/runtime/http_post.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/devr-tools/codeguard/internal/codeguard/ai/httpretry" + "github.com/devr-tools/codeguard/internal/codeguard/ai/safehttp" ) // postProviderJSON marshals body, POSTs it to url with the provider-specific @@ -34,7 +35,7 @@ func postProviderJSON(ctx context.Context, providerName string, url string, head return nil, err } defer resp.Body.Close() - respData, err := io.ReadAll(resp.Body) + respData, err := io.ReadAll(io.LimitReader(resp.Body, safehttp.MaxResponseBytes)) if err != nil { return nil, err } diff --git a/internal/codeguard/ai/runtime/provider.go b/internal/codeguard/ai/runtime/provider.go index ad88a0e..edfee4e 100644 --- a/internal/codeguard/ai/runtime/provider.go +++ b/internal/codeguard/ai/runtime/provider.go @@ -11,7 +11,9 @@ import ( "strings" "time" + "github.com/devr-tools/codeguard/internal/codeguard/ai/safehttp" "github.com/devr-tools/codeguard/internal/codeguard/core" + "github.com/devr-tools/codeguard/internal/codeguard/trust" ) const ( @@ -22,15 +24,25 @@ const ( func BuildProvider(cfg core.AIProviderConfig) (Provider, bool, error) { switch strings.TrimSpace(strings.ToLower(cfg.Type)) { case "", "openai": + // cfg.BaseURL comes from repository config, so it is an untrusted source. + if err := safehttp.ValidateProviderURL(cfg.BaseURL, false); err != nil { + return nil, false, err + } provider, ok := openAIProviderFromConfig(cfg) return provider, ok, nil case "anthropic": + if err := safehttp.ValidateProviderURL(cfg.BaseURL, false); err != nil { + return nil, false, err + } provider, ok := anthropicProviderFromConfig(cfg) return provider, ok, nil case "command": if strings.TrimSpace(cfg.Command) == "" { return nil, false, nil } + if err := trust.GuardConfigCommand("ai.provider", cfg.Command); err != nil { + return nil, false, err + } return commandProvider{command: cfg.Command, args: append([]string(nil), cfg.Args...)}, true, nil default: return nil, false, fmt.Errorf("unsupported ai provider %q", cfg.Type) @@ -123,7 +135,7 @@ func providerHTTPClient() *http.Client { timeout = parsed } } - return &http.Client{Timeout: timeout} + return safehttp.Client(timeout) } func openAIUserPrompt(req Request) string { diff --git a/internal/codeguard/ai/safehttp/safehttp.go b/internal/codeguard/ai/safehttp/safehttp.go new file mode 100644 index 0000000..79c1f08 --- /dev/null +++ b/internal/codeguard/ai/safehttp/safehttp.go @@ -0,0 +1,144 @@ +// Package safehttp hardens codeguard's outbound AI provider HTTP calls against +// SSRF and credential exfiltration. +// +// Because the AI provider base URL and the name of the environment variable +// holding the API key can be set from repository-checked-in config, an +// untrusted pull request could otherwise point codeguard at an attacker- +// controlled host and capture the operator's provider API key. By default this +// package: +// +// - restricts config-supplied provider base URLs to a small allowlist of +// known public provider hosts; and +// - blocks outbound connections to non-public (loopback/private/link-local) +// addresses, defending against DNS-rebinding and redirect-based SSRF. +// +// Both protections are relaxed when the operator opts in via the trust policy +// (CODEGUARD_ALLOW_CONFIG_AI_ENDPOINTS / --allow-config-ai-endpoints), which is +// required for self-hosted or internal provider endpoints. +package safehttp + +import ( + "fmt" + "net" + "net/http" + "net/url" + "sort" + "strings" + "syscall" + "time" + + "github.com/devr-tools/codeguard/internal/codeguard/trust" +) + +// MaxResponseBytes caps how much of an AI provider response body codeguard will +// read, so a malicious or compromised endpoint cannot exhaust memory. +const MaxResponseBytes = 16 << 20 // 16 MiB + +// allowedProviderHosts is the set of public provider hosts codeguard will talk +// to when the base URL comes from untrusted config without an opt-in. +var allowedProviderHosts = map[string]bool{ + "api.openai.com": true, + "api.anthropic.com": true, +} + +func allowedHostList() string { + hosts := make([]string, 0, len(allowedProviderHosts)) + for host := range allowedProviderHosts { + hosts = append(hosts, host) + } + sort.Strings(hosts) + return strings.Join(hosts, ", ") +} + +// ValidateProviderURL reports an error when rawURL is not an acceptable AI +// provider base URL under the active trust policy. An empty URL is accepted +// (the provider default is used). trustedSource should be true when the URL +// originates from a trusted source such as a process environment variable +// rather than repository config. +func ValidateProviderURL(rawURL string, trustedSource bool) error { + rawURL = strings.TrimSpace(rawURL) + if rawURL == "" { + return nil + } + parsed, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid AI provider base URL %q: %w", rawURL, err) + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return fmt.Errorf("AI provider base URL %q must use http or https", rawURL) + } + if parsed.Hostname() == "" { + return fmt.Errorf("AI provider base URL %q has no host", rawURL) + } + if trustedSource || trust.AllowConfigAIEndpoints() { + return nil + } + host := strings.ToLower(parsed.Hostname()) + if allowedProviderHosts[host] { + return nil + } + return fmt.Errorf( + "AI provider base URL host %q is not on the allowlist (%s); it was set from repository "+ + "configuration. Set %s=1 or pass --allow-config-ai-endpoints to use a custom endpoint.", + host, allowedHostList(), trust.AllowConfigAIEndpointsEnv) +} + +// Client returns an HTTP client for AI provider calls. Unless the operator has +// opted into custom AI endpoints, the client refuses to connect to non-public +// addresses at dial time. +func Client(timeout time.Duration) *http.Client { + if trust.AllowConfigAIEndpoints() { + return &http.Client{Timeout: timeout} + } + dialer := &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + Control: blockNonPublicAddress, + } + return &http.Client{ + Timeout: timeout, + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: dialer.DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: time.Second, + }, + } +} + +// blockNonPublicAddress is a net.Dialer Control hook that rejects connections to +// loopback, private, link-local, and otherwise non-routable addresses. It runs +// on the already-resolved address, so it also defends against a hostname that +// resolves to an internal IP (DNS rebinding). +func blockNonPublicAddress(_, address string, _ syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return fmt.Errorf("ssrf guard: cannot parse dial address %q: %w", address, err) + } + ip := net.ParseIP(host) + if ip == nil { + return fmt.Errorf("ssrf guard: cannot parse IP %q", host) + } + if isNonPublicIP(ip) { + return fmt.Errorf("ssrf guard: refusing to connect to non-public address %s", ip) + } + return nil +} + +func isNonPublicIP(ip net.IP) bool { + if ip.IsLoopback() || ip.IsPrivate() || ip.IsUnspecified() || + ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsMulticast() { + return true + } + // IPv4 broadcast and the IPv4-mapped/IPv6 documentation ranges are not + // covered by the helpers above; treat the cloud metadata address explicitly. + if v4 := ip.To4(); v4 != nil { + if v4[0] == 169 && v4[1] == 254 { // link-local incl. 169.254.169.254 metadata + return true + } + } + return false +} diff --git a/internal/codeguard/ai/semantic/runtime.go b/internal/codeguard/ai/semantic/runtime.go index 05bd662..a05c3da 100644 --- a/internal/codeguard/ai/semantic/runtime.go +++ b/internal/codeguard/ai/semantic/runtime.go @@ -7,6 +7,8 @@ import ( "fmt" "os/exec" "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/trust" ) const commandEnvKey = "CODEGUARD_SEMANTIC_COMMAND" @@ -20,6 +22,9 @@ func runCommand(ctx context.Context, command string, req Request) (Response, err if len(parts) == 0 { return Response{}, fmt.Errorf("semantic command is not configured") } + if err := trust.GuardConfigCommand("semantic command", parts[0]); err != nil { + return Response{}, err + } input, err := json.Marshal(req) if err != nil { return Response{}, err diff --git a/internal/codeguard/ai/triage/http.go b/internal/codeguard/ai/triage/http.go index 485ad99..82e7e36 100644 --- a/internal/codeguard/ai/triage/http.go +++ b/internal/codeguard/ai/triage/http.go @@ -3,9 +3,11 @@ package triage import ( "bytes" "context" + "io" "net/http" "github.com/devr-tools/codeguard/internal/codeguard/ai/httpretry" + "github.com/devr-tools/codeguard/internal/codeguard/ai/safehttp" ) // triageViaHTTP runs the shared triage flow: build the candidate prompt, @@ -35,8 +37,8 @@ func triageViaHTTP( // postTriageJSON POSTs a JSON triage request with retry, applying the // provider-specific headers on top of the shared Content-Type. func postTriageJSON(ctx context.Context, cfg runtimeConfig, url string, body []byte, headers map[string]string) (*http.Response, error) { - httpClient := &http.Client{Timeout: cfg.Timeout} - return httpretry.Do(ctx, httpClient, httpretry.FromEnv(), func() (*http.Request, error) { + httpClient := safehttp.Client(cfg.Timeout) + resp, err := httpretry.Do(ctx, httpClient, httpretry.FromEnv(), func() (*http.Request, error) { req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) if err != nil { return nil, err @@ -47,4 +49,13 @@ func postTriageJSON(ctx context.Context, cfg runtimeConfig, url string, body []b } return req, nil }) + if err != nil { + return nil, err + } + // Bound the response body so a malicious endpoint cannot exhaust memory. + resp.Body = struct { + io.Reader + io.Closer + }{io.LimitReader(resp.Body, safehttp.MaxResponseBytes), resp.Body} + return resp, nil } diff --git a/internal/codeguard/ai/triage/runtime.go b/internal/codeguard/ai/triage/runtime.go index 29be11d..d90afbd 100644 --- a/internal/codeguard/ai/triage/runtime.go +++ b/internal/codeguard/ai/triage/runtime.go @@ -6,17 +6,19 @@ import ( "strings" "time" + "github.com/devr-tools/codeguard/internal/codeguard/ai/safehttp" "github.com/devr-tools/codeguard/internal/codeguard/core" ) type runtimeConfig struct { - Provider string - Model string - BaseURL string - APIKey string - Timeout time.Duration - MockDecision string - MockSummary string + Provider string + Model string + BaseURL string + BaseURLFromEnv bool + APIKey string + Timeout time.Duration + MockDecision string + MockSummary string } func discoverRuntime(cfg core.AIConfig, opts core.ScanOptions) runtimeConfig { @@ -45,8 +47,9 @@ func discoverRuntime(cfg core.AIConfig, opts core.ScanOptions) runtimeConfig { os.Getenv("CODEGUARD_AI_TRIAGE_MODEL"), configProvider.Model, ) + envBaseURL := strings.TrimSpace(os.Getenv("CODEGUARD_AI_TRIAGE_BASE_URL")) baseURL := firstNonEmpty( - os.Getenv("CODEGUARD_AI_TRIAGE_BASE_URL"), + envBaseURL, configProvider.BaseURL, ) apiKey := firstNonEmpty( @@ -62,13 +65,14 @@ func discoverRuntime(cfg core.AIConfig, opts core.ScanOptions) runtimeConfig { } } return runtimeConfig{ - Provider: normalizedProvider, - Model: model, - BaseURL: baseURL, - APIKey: apiKey, - Timeout: timeout, - MockDecision: strings.ToLower(strings.TrimSpace(os.Getenv("CODEGUARD_AI_TRIAGE_DECISION"))), - MockSummary: strings.TrimSpace(os.Getenv("CODEGUARD_AI_TRIAGE_SUMMARY")), + Provider: normalizedProvider, + Model: model, + BaseURL: baseURL, + BaseURLFromEnv: envBaseURL != "", + APIKey: apiKey, + Timeout: timeout, + MockDecision: strings.ToLower(strings.TrimSpace(os.Getenv("CODEGUARD_AI_TRIAGE_DECISION"))), + MockSummary: strings.TrimSpace(os.Getenv("CODEGUARD_AI_TRIAGE_SUMMARY")), } } @@ -83,6 +87,9 @@ func (cfg runtimeConfig) validate() error { if cfg.Provider != "mock" && cfg.Model == "" { return fmt.Errorf("CODEGUARD_AI_TRIAGE_MODEL is required when CODEGUARD_AI_TRIAGE_PROVIDER is set") } + if err := safehttp.ValidateProviderURL(cfg.BaseURL, cfg.BaseURLFromEnv); err != nil { + return err + } switch cfg.Provider { case "mock": return nil diff --git a/internal/codeguard/checks/ci/ci_test_quality.go b/internal/codeguard/checks/ci/ci_test_quality.go index c447734..c98b6fd 100644 --- a/internal/codeguard/checks/ci/ci_test_quality.go +++ b/internal/codeguard/checks/ci/ci_test_quality.go @@ -26,7 +26,7 @@ func testQualityFindings(env support.Context, target core.TargetConfig) []core.F }, func(file string, data []byte) []core.Finding { findings := make([]core.Finding, 0) for _, block := range extractTestBlocks(language, string(data)) { - if isHelperProcessBlock(block) { + if shouldSkipTestQualityBlock(language, block) { continue } findings = append(findings, evaluateTestBlock(env, patterns, helpers, file, block)...) diff --git a/internal/codeguard/checks/ci/ci_test_quality_blocks.go b/internal/codeguard/checks/ci/ci_test_quality_blocks.go index 3eddb15..0b80e37 100644 --- a/internal/codeguard/checks/ci/ci_test_quality_blocks.go +++ b/internal/codeguard/checks/ci/ci_test_quality_blocks.go @@ -41,6 +41,23 @@ func isHelperProcessBlock(block testBlock) bool { return false } +func shouldSkipTestQualityBlock(language string, block testBlock) bool { + if isHelperProcessBlock(block) { + return true + } + return isGoTestMainBlock(language, block) +} + +func isGoTestMainBlock(language string, block testBlock) bool { + if language != "" && language != "go" { + return false + } + if block.name != "TestMain" || len(block.lines) == 0 { + return false + } + return strings.Contains(block.lines[0], "*testing.M") +} + func nextNonBlankLineIsReturn(lines []string, start int) bool { for idx := start; idx < len(lines); idx++ { trimmed := strings.TrimSpace(lines[idx]) diff --git a/internal/codeguard/checks/security/security_common.go b/internal/codeguard/checks/security/security_common.go index 4568c00..07b9fb6 100644 --- a/internal/codeguard/checks/security/security_common.go +++ b/internal/codeguard/checks/security/security_common.go @@ -28,6 +28,7 @@ func findingsForFile(env support.Context, file string, data []byte) []core.Findi lineNo := idx + 1 findings = append(findings, appendCommonLineFindings(env, file, lineNo, line)...) findings = append(findings, appendLanguageLineFindings(env, file, lineNo, line, maskedLines[idx])...) + findings = append(findings, appendOWASPExtraLineFindings(env, file, lineNo, line, maskedLines[idx])...) } if isTypeScriptFile(file) { findings = append(findings, typeScriptFindingsForFile(env, file, source)...) diff --git a/internal/codeguard/checks/security/security_owasp_extra.go b/internal/codeguard/checks/security/security_owasp_extra.go new file mode 100644 index 0000000..c337cce --- /dev/null +++ b/internal/codeguard/checks/security/security_owasp_extra.go @@ -0,0 +1,78 @@ +package security + +import ( + "path/filepath" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +var ( + // String-literal based: matched on the raw line because the signal lives + // inside string contents that source masking would blank. + corsWildcardPattern = regexp.MustCompile(`(?i)access-control-allow-origin["']?\s*[:,=]\s*["']?\*`) + bindAllPattern = regexp.MustCompile(`\b0\.0\.0\.0\b`) + dockerfileUserRoot = regexp.MustCompile(`(?i)^\s*USER\s+root\b`) + + // Code/identifier based: matched on the raw line so string algorithm names + // (e.g. getInstance("MD5")) are still visible. + debugEnabledPattern = regexp.MustCompile(`\bdebug\s*=\s*True\b`) + weakHashPattern = regexp.MustCompile(`(?i)\bhashlib\.(?:md5|sha1)\s*\(|\b(?:md5|sha1)\.New\s*\(|crypto/(?:md5|sha1)|messagedigest\.getinstance\s*\(\s*"(?:md5|sha-?1)"|createhash\s*\(\s*['"](?:md5|sha1)['"]|\bDigest::(?:MD5|SHA1)\b|\b(?:MD5|SHA1)(?:CryptoServiceProvider|Managed)\b`) + weakCipherPattern = regexp.MustCompile(`(?i)crypto/(?:des|rc4)|cipher\.getinstance\s*\(\s*"(?:des|desede|rc4|[^"]*ecb[^"]*)"|createcipheriv\s*\(\s*['"](?:des|des-ede3|rc4|aes-\d+-ecb)|\bnew\s+(?:DESCryptoServiceProvider|RC2CryptoServiceProvider|TripleDESCryptoServiceProvider)\b|\bMODE_ECB\b`) + deserializePattern = regexp.MustCompile(`(?i)\b(?:pickle|cpickle)\.loads?\s*\(|\bmarshal\.loads\s*\(|\byaml\.load\s*\(|\.readObject\s*\(|\bnew\s+ObjectInputStream\b|\bXMLDecoder\b|\bMarshal\.load\s*\(|\bunserialize\s*\(`) +) + +// appendOWASPExtraLineFindings runs the language-agnostic OWASP-gap heuristics +// (A05 misconfiguration, A02 weak crypto, A08 insecure deserialization) for one +// source line. raw is the unmodified line; masked has comments and string +// contents blanked where a masker exists for the language. +func appendOWASPExtraLineFindings(env support.Context, file string, lineNo int, raw string, masked string) []core.Finding { + findings := make([]core.Finding, 0) + + if corsWildcardPattern.MatchString(raw) { + findings = append(findings, env.NewFinding(support.FindingInput{RuleID: "security.cors-wildcard", Level: "warn", Path: file, Line: lineNo, Column: 1, Message: "wildcard CORS origin '*' allows any site to read responses"})) + } + if bindAllPattern.MatchString(raw) { + findings = append(findings, env.NewFinding(support.FindingInput{RuleID: "security.bind-all-interfaces", Level: "warn", Path: file, Line: lineNo, Column: 1, Message: "service binds to 0.0.0.0 (all network interfaces)"})) + } + if debugEnabledPattern.MatchString(masked) { + findings = append(findings, env.NewFinding(support.FindingInput{RuleID: "security.debug-enabled", Level: "warn", Path: file, Line: lineNo, Column: 1, Message: "debug mode enabled; disable in production"})) + } + if weakHashPattern.MatchString(raw) { + findings = append(findings, env.NewFinding(support.FindingInput{RuleID: "security.weak-hash", Level: "warn", Path: file, Line: lineNo, Column: 1, Message: "weak hash algorithm (MD5/SHA-1) should not be used for security"})) + } + if weakCipherPattern.MatchString(raw) { + findings = append(findings, env.NewFinding(support.FindingInput{RuleID: "security.weak-cipher", Level: "warn", Path: file, Line: lineNo, Column: 1, Message: "weak or insecure cipher (DES/RC4/ECB) should be replaced"})) + } + if isInsecureDeserialization(raw) { + findings = append(findings, env.NewFinding(support.FindingInput{RuleID: "security.insecure-deserialization", Level: "warn", Path: file, Line: lineNo, Column: 1, Message: "insecure deserialization of potentially untrusted data"})) + } + if isDockerfile(file) && dockerfileUserRoot.MatchString(raw) { + findings = append(findings, env.NewFinding(support.FindingInput{RuleID: "security.dockerfile-root", Level: "warn", Path: file, Line: lineNo, Column: 1, Message: "container runs as root; set a non-root USER"})) + } + return findings +} + +// isInsecureDeserialization matches dangerous deserialization calls while +// excluding explicitly-safe YAML loaders (RE2 has no negative lookahead, so the +// safe case is filtered in code). +func isInsecureDeserialization(line string) bool { + if !deserializePattern.MatchString(line) { + return false + } + lower := strings.ToLower(line) + if strings.Contains(lower, "yaml.load") && (strings.Contains(lower, "safeloader") || strings.Contains(lower, "csafeloader")) { + return false + } + return true +} + +func isDockerfile(path string) bool { + base := filepath.Base(path) + if strings.EqualFold(filepath.Ext(path), ".dockerfile") { + return true + } + return base == "Dockerfile" || strings.HasPrefix(base, "Dockerfile.") +} diff --git a/internal/codeguard/checks/security/security_taint_go.go b/internal/codeguard/checks/security/security_taint_go.go index 67374c8..840608d 100644 --- a/internal/codeguard/checks/security/security_taint_go.go +++ b/internal/codeguard/checks/security/security_taint_go.go @@ -71,7 +71,7 @@ func (a *goTaintAnalyzer) line(pos token.Pos) int { // emitFinding records one deduplicated source-to-sink finding. func (a *goTaintAnalyzer) emitFinding(taint *goTaint, sink string, sinkLine int) { a.findings = appendTaintFinding(a.env, a.file, a.seen, a.findings, taintSinkInput{ - ruleID: "security.taint.go", + ruleID: goTaintRuleID(sink), source: taint.source, sourceLine: taint.sourceLine, chain: taint.chain, diff --git a/internal/codeguard/checks/security/security_taint_go_sinks.go b/internal/codeguard/checks/security/security_taint_go_sinks.go index f5fd5d2..4f1c5d4 100644 --- a/internal/codeguard/checks/security/security_taint_go_sinks.go +++ b/internal/codeguard/checks/security/security_taint_go_sinks.go @@ -26,6 +26,17 @@ var goFileSinkCallees = map[string]bool{ "ioutil.ReadFile": true, } +// goHTTPSinkArgIndex maps an outbound-HTTP callee to the argument index that +// carries the request URL. Tainted input reaching that argument is SSRF. +var goHTTPSinkArgIndex = map[string]int{ + "http.Get": 0, + "http.Head": 0, + "http.Post": 0, + "http.PostForm": 0, + "http.NewRequest": 1, + "http.NewRequestWithContext": 2, +} + // checkSinks inspects one call expression for taint sinks. Tainted values // derived from parameters are recorded in the function summary instead of // being reported, so callers decide whether the flow is dangerous. @@ -38,11 +49,27 @@ func (s *goScope) checkSinks(call *ast.CallExpr, callee string, args []*goTaint) s.reportFirstTainted(args[1:], callee, line) case goFileSinkCallees[callee] && len(args) > 0: s.reportTainted(args[0], callee, line) + case isGoHTTPSink(callee): + if idx := goHTTPSinkArgIndex[callee]; idx < len(args) { + s.reportTainted(args[idx], callee, line) + } default: s.checkMethodSinks(call, callee, args, line) } } +func isGoHTTPSink(callee string) bool { + _, ok := goHTTPSinkArgIndex[callee] + return ok +} + +func goTaintRuleID(sink string) string { + if isGoHTTPSink(sink) { + return "security.ssrf.go" + } + return "security.taint.go" +} + func (s *goScope) checkMethodSinks(call *ast.CallExpr, callee string, args []*goTaint, line int) { method := callee if dot := strings.LastIndexByte(callee, '.'); dot >= 0 { diff --git a/internal/codeguard/checks/security/security_taint_python_sinks.go b/internal/codeguard/checks/security/security_taint_python_sinks.go index 68d5dbb..3752bc7 100644 --- a/internal/codeguard/checks/security/security_taint_python_sinks.go +++ b/internal/codeguard/checks/security/security_taint_python_sinks.go @@ -7,9 +7,40 @@ import ( "github.com/devr-tools/codeguard/internal/codeguard/checks/support" ) +// pyHTTPSinkArgIndex maps an outbound-HTTP callee to the argument index that +// carries the request URL. Tainted input reaching that argument is SSRF. +var pyHTTPSinkArgIndex = map[string]int{ + "requests.get": 0, + "requests.post": 0, + "requests.put": 0, + "requests.delete": 0, + "requests.patch": 0, + "requests.head": 0, + "requests.options": 0, + "requests.request": 1, + "urllib.request.urlopen": 0, + "urllib.request.Request": 0, + "urllib.urlopen": 0, + "urllib2.urlopen": 0, + "httpx.get": 0, + "httpx.post": 0, +} + +func isPyHTTPSink(callee string) bool { + _, ok := pyHTTPSinkArgIndex[callee] + return ok +} + +func pyTaintRuleID(sink string) string { + if isPyHTTPSink(sink) { + return "security.ssrf.python" + } + return "security.taint.python" +} + func (a *pyTaintAnalyzer) emitFinding(taint *pyTaint, sink string, sinkLine int) { a.findings = appendTaintFinding(a.env, a.file, a.seen, a.findings, taintSinkInput{ - ruleID: "security.taint.python", + ruleID: pyTaintRuleID(sink), source: taint.source, sourceLine: taint.sourceLine, chain: taint.chain, @@ -59,6 +90,8 @@ func (s *pyScope) checkCallSink(call support.ParsedCall) { case strings.HasSuffix(call.Callee, ".execute") || strings.HasSuffix(call.Callee, ".executemany"): // only the query text is dangerous; parameterized args are safe s.reportSink(s.argTaint(call, 0), call.Callee, call.Line) + case isPyHTTPSink(call.Callee): + s.reportSink(s.argTaint(call, pyHTTPSinkArgIndex[call.Callee]), call.Callee, call.Line) } } diff --git a/internal/codeguard/config/io.go b/internal/codeguard/config/io.go index b2f85ba..32b9f50 100644 --- a/internal/codeguard/config/io.go +++ b/internal/codeguard/config/io.go @@ -1,7 +1,6 @@ package config import ( - "encoding/json" "errors" "fmt" "os" @@ -9,7 +8,6 @@ import ( "strings" "github.com/devr-tools/codeguard/internal/codeguard/core" - "gopkg.in/yaml.v3" ) var ( @@ -18,10 +16,6 @@ var ( defaultConfigDirs = []string{".", ".codeguard"} ) -func DefaultConfigPath() string { - return defaultConfigNames[0] -} - func LoadFile(path string) (core.Config, error) { resolvedPath, err := resolveConfigPath(path) if err != nil { @@ -37,14 +31,69 @@ func LoadFile(path string) (core.Config, error) { if err := unmarshalConfig(data, resolvedPath, &cfg); err != nil { return core.Config{}, err } - resolveRelativePaths(&cfg, filepath.Dir(resolvedPath)) + baseDir := filepath.Dir(resolvedPath) + resolveRelativePaths(&cfg, baseDir) ApplyDefaults(&cfg) + if err := containConfigArtifactPaths(&cfg, baseDir); err != nil { + return core.Config{}, err + } if err := Validate(cfg); err != nil { return core.Config{}, err } return cfg, nil } +// containConfigArtifactPaths resolves codeguard's config-controlled output +// paths (baseline, scan cache, AI cache) relative to the config directory and +// rejects any path that escapes that directory tree. Because the config file is +// checked into the repository and may be authored by an untrusted contributor, +// this prevents a config from steering codeguard into reading or writing files +// outside the repository (path traversal / arbitrary file write). +func containConfigArtifactPaths(cfg *core.Config, baseDir string) error { + type artifact struct { + label string + path *string + } + for _, a := range []artifact{ + {"baseline.path", &cfg.Baseline.Path}, + {"cache.path", &cfg.Cache.Path}, + {"ai.cache.path", &cfg.AI.Cache.Path}, + } { + resolved, err := containedPath(baseDir, *a.path) + if err != nil { + return fmt.Errorf("%s: %w", a.label, err) + } + *a.path = resolved + } + return nil +} + +// containedPath resolves p (relative to baseDir if not absolute) and returns the +// cleaned path, erroring if it escapes baseDir. An empty path is returned +// unchanged. +func containedPath(baseDir, p string) (string, error) { + if strings.TrimSpace(p) == "" { + return p, nil + } + absBase, err := filepath.Abs(baseDir) + if err != nil { + return "", err + } + resolved := p + if !filepath.IsAbs(resolved) { + resolved = filepath.Join(absBase, resolved) + } + resolved = filepath.Clean(resolved) + rel, err := filepath.Rel(absBase, resolved) + if err != nil { + return "", fmt.Errorf("path %q is not within the config directory", p) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("path %q escapes the config directory %q", p, absBase) + } + return resolved, nil +} + func resolveRelativePaths(cfg *core.Config, baseDir string) { for i := range cfg.Targets { targetPath := strings.TrimSpace(cfg.Targets[i].Path) @@ -123,21 +172,3 @@ func findConfigInDirs(dirs []string, names []string) (string, bool) { } return "", false } - -func marshalConfig(path string, cfg core.Config) ([]byte, error) { - switch strings.ToLower(filepath.Ext(path)) { - case ".yaml", ".yml": - return yaml.Marshal(cfg) - default: - return json.MarshalIndent(cfg, "", " ") - } -} - -func unmarshalConfig(data []byte, path string, cfg *core.Config) error { - switch strings.ToLower(filepath.Ext(path)) { - case ".yaml", ".yml": - return yaml.Unmarshal(data, cfg) - default: - return json.Unmarshal(data, cfg) - } -} diff --git a/internal/codeguard/config/io_codec.go b/internal/codeguard/config/io_codec.go new file mode 100644 index 0000000..c30096b --- /dev/null +++ b/internal/codeguard/config/io_codec.go @@ -0,0 +1,28 @@ +package config + +import ( + "encoding/json" + "path/filepath" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" + "gopkg.in/yaml.v3" +) + +func marshalConfig(path string, cfg core.Config) ([]byte, error) { + switch strings.ToLower(filepath.Ext(path)) { + case ".yaml", ".yml": + return yaml.Marshal(cfg) + default: + return json.MarshalIndent(cfg, "", " ") + } +} + +func unmarshalConfig(data []byte, path string, cfg *core.Config) error { + switch strings.ToLower(filepath.Ext(path)) { + case ".yaml", ".yml": + return yaml.Unmarshal(data, cfg) + default: + return json.Unmarshal(data, cfg) + } +} diff --git a/internal/codeguard/config/io_path.go b/internal/codeguard/config/io_path.go new file mode 100644 index 0000000..ed98179 --- /dev/null +++ b/internal/codeguard/config/io_path.go @@ -0,0 +1,5 @@ +package config + +func DefaultConfigPath() string { + return defaultConfigNames[0] +} diff --git a/internal/codeguard/core/owasp.go b/internal/codeguard/core/owasp.go new file mode 100644 index 0000000..fffef30 --- /dev/null +++ b/internal/codeguard/core/owasp.go @@ -0,0 +1,92 @@ +package core + +import "sort" + +// OWASPCategory identifies an OWASP Top 10 (2021) risk category. Values use the +// canonical "Axx:2021-Name" form so they are stable to display and to match on. +type OWASPCategory string + +const ( + OWASPA01BrokenAccessControl OWASPCategory = "A01:2021-Broken Access Control" + OWASPA02CryptographicFailures OWASPCategory = "A02:2021-Cryptographic Failures" + OWASPA03Injection OWASPCategory = "A03:2021-Injection" + OWASPA04InsecureDesign OWASPCategory = "A04:2021-Insecure Design" + OWASPA05SecurityMisconfiguration OWASPCategory = "A05:2021-Security Misconfiguration" + OWASPA06VulnerableComponents OWASPCategory = "A06:2021-Vulnerable and Outdated Components" + OWASPA07AuthFailures OWASPCategory = "A07:2021-Identification and Authentication Failures" + OWASPA08IntegrityFailures OWASPCategory = "A08:2021-Software and Data Integrity Failures" + OWASPA09LoggingFailures OWASPCategory = "A09:2021-Security Logging and Monitoring Failures" + OWASPA10SSRF OWASPCategory = "A10:2021-Server-Side Request Forgery (SSRF)" +) + +// OWASPTop10 lists every OWASP Top 10 (2021) category in canonical order. Used +// for coverage reporting so categories without a matching rule are visible. +var OWASPTop10 = []OWASPCategory{ + OWASPA01BrokenAccessControl, + OWASPA02CryptographicFailures, + OWASPA03Injection, + OWASPA04InsecureDesign, + OWASPA05SecurityMisconfiguration, + OWASPA06VulnerableComponents, + OWASPA07AuthFailures, + OWASPA08IntegrityFailures, + OWASPA09LoggingFailures, + OWASPA10SSRF, +} + +// Code returns the short identifier of the category (e.g. "A03:2021"), or the +// empty string when the category is unset/malformed. +func (c OWASPCategory) Code() string { + s := string(c) + for i := 0; i < len(s); i++ { + if s[i] == '-' { + return s[:i] + } + } + return s +} + +// Name returns the human-readable category name without the code prefix +// (e.g. "Injection" for "A03:2021-Injection"). +func (c OWASPCategory) Name() string { + s := string(c) + for i := 0; i < len(s); i++ { + if s[i] == '-' { + return s[i+1:] + } + } + return s +} + +// OWASPCoverageEntry records which rules cover a single OWASP Top 10 category. +type OWASPCoverageEntry struct { + Category OWASPCategory `json:"category"` + Code string `json:"code"` + Covered bool `json:"covered"` + RuleIDs []string `json:"rule_ids"` +} + +// OWASPCoverageForRules computes, for every OWASP Top 10 (2021) category, the +// set of rules that map to it. Categories with no matching rule are returned +// with Covered=false so coverage gaps are explicit. +func OWASPCoverageForRules(rules []RuleMetadata) []OWASPCoverageEntry { + byCategory := make(map[OWASPCategory][]string) + for _, rule := range rules { + if rule.OWASPCategory == "" { + continue + } + byCategory[rule.OWASPCategory] = append(byCategory[rule.OWASPCategory], rule.ID) + } + entries := make([]OWASPCoverageEntry, 0, len(OWASPTop10)) + for _, category := range OWASPTop10 { + ids := byCategory[category] + sort.Strings(ids) + entries = append(entries, OWASPCoverageEntry{ + Category: category, + Code: category.Code(), + Covered: len(ids) > 0, + RuleIDs: ids, + }) + } + return entries +} diff --git a/internal/codeguard/core/rule_metadata_types.go b/internal/codeguard/core/rule_metadata_types.go index ae0b595..9b8f9ec 100644 --- a/internal/codeguard/core/rule_metadata_types.go +++ b/internal/codeguard/core/rule_metadata_types.go @@ -44,4 +44,8 @@ type RuleMetadata struct { Description string `json:"description"` HowToFix string `json:"how_to_fix,omitempty"` FixTemplate string `json:"fix_template,omitempty"` + // OWASPCategory maps the rule to an OWASP Top 10 (2021) category. Empty when + // the rule is not associated with a fixed category (e.g. command-driven + // rules whose category depends on the external tool). + OWASPCategory OWASPCategory `json:"owasp_category,omitempty"` } diff --git a/internal/codeguard/report/sarif_helpers.go b/internal/codeguard/report/sarif_builders.go similarity index 55% rename from internal/codeguard/report/sarif_helpers.go rename to internal/codeguard/report/sarif_builders.go index cd66416..2627005 100644 --- a/internal/codeguard/report/sarif_helpers.go +++ b/internal/codeguard/report/sarif_builders.go @@ -1,50 +1,6 @@ package report -import ( - "github.com/devr-tools/codeguard/internal/codeguard/core" -) - -type sarifRule struct { - ID string `json:"id"` - ShortDescription struct { - Text string `json:"text"` - } `json:"shortDescription"` - FullDescription struct { - Text string `json:"text"` - } `json:"fullDescription"` - Help struct { - Text string `json:"text,omitempty"` - } `json:"help,omitempty"` -} - -type sarifResult struct { - RuleID string `json:"ruleId"` - Level string `json:"level"` - Message sarifMessage `json:"message"` - Locations []sarifLocation `json:"locations,omitempty"` -} - -type sarifMessage struct { - Text string `json:"text"` -} - -type sarifLocation struct { - PhysicalLocation sarifPhysicalLocation `json:"physicalLocation"` -} - -type sarifPhysicalLocation struct { - ArtifactLocation sarifArtifactLocation `json:"artifactLocation"` - Region sarifRegion `json:"region"` -} - -type sarifArtifactLocation struct { - URI string `json:"uri"` -} - -type sarifRegion struct { - StartLine int `json:"startLine,omitempty"` - StartColumn int `json:"startColumn,omitempty"` -} +import "github.com/devr-tools/codeguard/internal/codeguard/core" func buildSARIFRule(catalog map[string]core.RuleMetadata, finding core.Finding) sarifRule { meta := catalog[finding.RuleID] @@ -58,6 +14,19 @@ func buildSARIFRule(catalog map[string]core.RuleMetadata, finding core.Finding) rule.ShortDescription.Text = meta.Title rule.FullDescription.Text = meta.Description rule.Help.Text = meta.HowToFix + if meta.OWASPCategory != "" { + rule.Properties = &sarifRuleProperties{ + Tags: []string{"security", "OWASP:" + meta.OWASPCategory.Code()}, + OWASP: string(meta.OWASPCategory), + } + rule.Relationships = []sarifRelationship{{ + Target: sarifReportingDescriptorReference{ + ID: meta.OWASPCategory.Code(), + ToolComponent: sarifToolComponent{Name: owaspTaxonomyName, GUID: owaspTaxonomyGUID}, + }, + Kinds: []string{"superset"}, + }} + } return rule } diff --git a/internal/codeguard/report/sarif_result_types.go b/internal/codeguard/report/sarif_result_types.go new file mode 100644 index 0000000..9e0cc7d --- /dev/null +++ b/internal/codeguard/report/sarif_result_types.go @@ -0,0 +1,30 @@ +package report + +type sarifResult struct { + RuleID string `json:"ruleId"` + Level string `json:"level"` + Message sarifMessage `json:"message"` + Locations []sarifLocation `json:"locations,omitempty"` +} + +type sarifMessage struct { + Text string `json:"text"` +} + +type sarifLocation struct { + PhysicalLocation sarifPhysicalLocation `json:"physicalLocation"` +} + +type sarifPhysicalLocation struct { + ArtifactLocation sarifArtifactLocation `json:"artifactLocation"` + Region sarifRegion `json:"region"` +} + +type sarifArtifactLocation struct { + URI string `json:"uri"` +} + +type sarifRegion struct { + StartLine int `json:"startLine,omitempty"` + StartColumn int `json:"startColumn,omitempty"` +} diff --git a/internal/codeguard/report/sarif_rule_types.go b/internal/codeguard/report/sarif_rule_types.go new file mode 100644 index 0000000..82abbc5 --- /dev/null +++ b/internal/codeguard/report/sarif_rule_types.go @@ -0,0 +1,41 @@ +package report + +type sarifRule struct { + ID string `json:"id"` + ShortDescription struct { + Text string `json:"text"` + } `json:"shortDescription"` + FullDescription struct { + Text string `json:"text"` + } `json:"fullDescription"` + Help struct { + Text string `json:"text,omitempty"` + } `json:"help,omitempty"` + Properties *sarifRuleProperties `json:"properties,omitempty"` + Relationships []sarifRelationship `json:"relationships,omitempty"` +} + +type sarifRuleProperties struct { + Tags []string `json:"tags,omitempty"` + OWASP string `json:"owasp,omitempty"` +} + +type sarifRelationship struct { + Target sarifReportingDescriptorReference `json:"target"` + Kinds []string `json:"kinds"` +} + +type sarifReportingDescriptorReference struct { + ID string `json:"id"` + ToolComponent sarifToolComponent `json:"toolComponent"` +} + +type sarifToolComponent struct { + Name string `json:"name"` + GUID string `json:"guid,omitempty"` +} + +const ( + owaspTaxonomyName = "OWASP Top 10" + owaspTaxonomyGUID = "f1b2c3d4-0a21-4e10-9b00-000000002021" +) diff --git a/internal/codeguard/report/sarif_taxonomy.go b/internal/codeguard/report/sarif_taxonomy.go new file mode 100644 index 0000000..20455ef --- /dev/null +++ b/internal/codeguard/report/sarif_taxonomy.go @@ -0,0 +1,43 @@ +package report + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +type sarifTaxonomy struct { + Name string `json:"name"` + Version string `json:"version"` + GUID string `json:"guid"` + ShortDescription sarifTextBlock `json:"shortDescription"` + Taxa []sarifTaxon `json:"taxa"` + InformationURI string `json:"informationUri,omitempty"` + IsComprehensive bool `json:"isComprehensive"` +} + +type sarifTaxon struct { + ID string `json:"id"` + Name string `json:"name"` + ShortDescription sarifTextBlock `json:"shortDescription"` +} + +type sarifTextBlock struct { + Text string `json:"text"` +} + +func owaspTaxonomy() sarifTaxonomy { + taxa := make([]sarifTaxon, 0, len(core.OWASPTop10)) + for _, category := range core.OWASPTop10 { + taxa = append(taxa, sarifTaxon{ + ID: category.Code(), + Name: category.Name(), + ShortDescription: sarifTextBlock{Text: string(category)}, + }) + } + return sarifTaxonomy{ + Name: owaspTaxonomyName, + Version: "2021", + GUID: owaspTaxonomyGUID, + ShortDescription: sarifTextBlock{Text: "OWASP Top 10 (2021)"}, + InformationURI: "https://owasp.org/Top10/", + IsComprehensive: true, + Taxa: taxa, + } +} diff --git a/internal/codeguard/report/write.go b/internal/codeguard/report/write.go index 19a5f86..fb17abe 100644 --- a/internal/codeguard/report/write.go +++ b/internal/codeguard/report/write.go @@ -100,7 +100,8 @@ func writeSARIF(w io.Writer, report core.Report) error { "rules": sarifRules, }, }, - "results": results, + "taxonomies": []any{owaspTaxonomy()}, + "results": results, }, } enc := json.NewEncoder(w) diff --git a/internal/codeguard/rules/catalog.go b/internal/codeguard/rules/catalog.go index caa0b82..d057d42 100644 --- a/internal/codeguard/rules/catalog.go +++ b/internal/codeguard/rules/catalog.go @@ -2,19 +2,20 @@ package rules import "github.com/devr-tools/codeguard/internal/codeguard/core" -var catalog = mergeRuleCatalogs( +var catalog = withSecurityOWASP(mergeRuleCatalogs( qualityCatalog, qualityPerformanceCatalog, designCatalog, designGraphCatalog, securityCatalog, + securityExtraCatalog, supplyChainCatalog, contractsCatalog, securityTaintCatalog, miscCatalog, coverageCatalog, testQualityCatalog, -) +)) func Catalog() map[string]core.RuleMetadata { out := make(map[string]core.RuleMetadata, len(catalog)) diff --git a/internal/codeguard/rules/catalog_security_extra.go b/internal/codeguard/rules/catalog_security_extra.go new file mode 100644 index 0000000..7796092 --- /dev/null +++ b/internal/codeguard/rules/catalog_security_extra.go @@ -0,0 +1,80 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +// securityExtraCatalog holds the language-agnostic OWASP-gap rules added to +// close coverage for A05 (Security Misconfiguration), A02 (Cryptographic +// Failures), and A08 (Software and Data Integrity Failures). They are +// heuristic, text-based, repository-wide checks and default to "warn". +var securityExtraCatalog = map[string]core.RuleMetadata{ + "security.cors-wildcard": { + ID: "security.cors-wildcard", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Wildcard CORS origin", + Description: "Warns when Access-Control-Allow-Origin is set to the wildcard '*', which lets any site read cross-origin responses.", + HowToFix: "Reflect a validated allowlist of trusted origins instead of returning '*', especially for credentialed endpoints.", + }, + "security.debug-enabled": { + ID: "security.debug-enabled", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Debug mode enabled", + Description: "Warns when a framework debug flag is enabled (e.g. debug=True), which can expose stack traces, consoles, or secrets in production.", + HowToFix: "Drive debug mode from environment configuration and ensure it is disabled in production builds.", + }, + "security.bind-all-interfaces": { + ID: "security.bind-all-interfaces", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Service binds to all interfaces", + Description: "Warns when a service binds to 0.0.0.0, exposing it on every network interface.", + HowToFix: "Bind to a specific interface (e.g. 127.0.0.1) or restrict exposure with a firewall or network policy.", + }, + "security.dockerfile-root": { + ID: "security.dockerfile-root", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Container runs as root", + Description: "Warns when a Dockerfile explicitly sets USER root, so the container process runs with root privileges.", + HowToFix: "Add a non-root USER instruction and run the container as an unprivileged user.", + }, + "security.weak-hash": { + ID: "security.weak-hash", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Weak hash algorithm", + Description: "Warns when a broken hash algorithm (MD5 or SHA-1) is used, which is unsafe for signatures, integrity, or password storage.", + HowToFix: "Use SHA-256 or stronger for integrity, and a dedicated password hash (bcrypt, scrypt, or Argon2) for credentials.", + }, + "security.weak-cipher": { + ID: "security.weak-cipher", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Weak or insecure cipher", + Description: "Warns when a weak cipher or mode is used (DES, 3DES, RC4, or ECB block mode).", + HowToFix: "Use AES-GCM or ChaCha20-Poly1305 with a unique nonce; avoid ECB mode and legacy ciphers.", + }, + "security.insecure-deserialization": { + ID: "security.insecure-deserialization", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Insecure deserialization", + Description: "Warns when untrusted data may be deserialized through a dangerous API (pickle, yaml.load, Java readObject, Marshal.load, unserialize), which can lead to remote code execution.", + HowToFix: "Deserialize only trusted data, prefer safe loaders (e.g. yaml.safe_load), or use a data-only format such as JSON.", + }, +} diff --git a/internal/codeguard/rules/catalog_security_owasp.go b/internal/codeguard/rules/catalog_security_owasp.go new file mode 100644 index 0000000..28bb3d4 --- /dev/null +++ b/internal/codeguard/rules/catalog_security_owasp.go @@ -0,0 +1,91 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +// securityRuleOWASP maps each security rule to its OWASP Top 10 (2021) category. +// +// Rules whose category depends on an external tool (security.command-check) are +// intentionally omitted: their category cannot be known statically. +var securityRuleOWASP = map[string]core.OWASPCategory{ + // Secrets and key material. + "security.hardcoded-secret": core.OWASPA07AuthFailures, + "security.private-key": core.OWASPA02CryptographicFailures, + + // Transport security. + "security.insecure-tls": core.OWASPA02CryptographicFailures, + "security.typescript.insecure-tls": core.OWASPA02CryptographicFailures, + "security.javascript.insecure-tls": core.OWASPA02CryptographicFailures, + "security.python.insecure-tls": core.OWASPA02CryptographicFailures, + "security.rust.insecure-tls": core.OWASPA02CryptographicFailures, + "security.java.insecure-tls": core.OWASPA02CryptographicFailures, + "security.csharp.insecure-tls": core.OWASPA02CryptographicFailures, + "security.ruby.insecure-tls": core.OWASPA02CryptographicFailures, + + // Command / code injection. + "security.shell-execution": core.OWASPA03Injection, + "security.typescript.shell-execution": core.OWASPA03Injection, + "security.javascript.shell-execution": core.OWASPA03Injection, + "security.python.shell-execution": core.OWASPA03Injection, + "security.rust.shell-execution": core.OWASPA03Injection, + "security.java.shell-execution": core.OWASPA03Injection, + "security.csharp.shell-execution": core.OWASPA03Injection, + "security.ruby.shell-execution": core.OWASPA03Injection, + "security.typescript.dynamic-code": core.OWASPA03Injection, + "security.javascript.dynamic-code": core.OWASPA03Injection, + "security.python.dynamic-code": core.OWASPA03Injection, + "security.ruby.dynamic-code": core.OWASPA03Injection, + "security.typescript.vm-dynamic-code": core.OWASPA03Injection, + "security.javascript.vm-dynamic-code": core.OWASPA03Injection, + "security.typescript.string-timer-code": core.OWASPA03Injection, + "security.javascript.string-timer-code": core.OWASPA03Injection, + + // Cross-site scripting (XSS is under A03:2021 Injection). + "security.typescript.unsafe-html-sink": core.OWASPA03Injection, + "security.javascript.unsafe-html-sink": core.OWASPA03Injection, + + // Untrusted-input taint flows reaching dangerous sinks. + "security.typescript.taint-flow": core.OWASPA03Injection, + "security.javascript.taint-flow": core.OWASPA03Injection, + "security.typescript.untrusted-input-flow": core.OWASPA03Injection, + "security.javascript.untrusted-input-flow": core.OWASPA03Injection, + "security.taint.go": core.OWASPA03Injection, + "security.taint.python": core.OWASPA03Injection, + + // Cross-origin message access control. + "security.typescript.postmessage-wildcard": core.OWASPA01BrokenAccessControl, + "security.javascript.postmessage-wildcard": core.OWASPA01BrokenAccessControl, + + // Known-vulnerable dependencies. + "security.govulncheck": core.OWASPA06VulnerableComponents, + + // Security misconfiguration (A05). + "security.cors-wildcard": core.OWASPA05SecurityMisconfiguration, + "security.debug-enabled": core.OWASPA05SecurityMisconfiguration, + "security.bind-all-interfaces": core.OWASPA05SecurityMisconfiguration, + "security.dockerfile-root": core.OWASPA05SecurityMisconfiguration, + + // Cryptographic failures (A02). + "security.weak-hash": core.OWASPA02CryptographicFailures, + "security.weak-cipher": core.OWASPA02CryptographicFailures, + + // Software and data integrity failures (A08). + "security.insecure-deserialization": core.OWASPA08IntegrityFailures, + + // Server-side request forgery (A10). + "security.ssrf.go": core.OWASPA10SSRF, + "security.ssrf.python": core.OWASPA10SSRF, +} + +// withSecurityOWASP returns a copy of catalog with OWASP Top 10 categories +// applied to the security rules in securityRuleOWASP. It is invoked from the +// `catalog` var initializer so the mapping is baked into every read path +// (Catalog, RuleCatalogForConfig, SARIF) regardless of init() ordering. +func withSecurityOWASP(catalog map[string]core.RuleMetadata) map[string]core.RuleMetadata { + for id, category := range securityRuleOWASP { + if rule, ok := catalog[id]; ok { + rule.OWASPCategory = category + catalog[id] = rule + } + } + return catalog +} diff --git a/internal/codeguard/rules/catalog_security_taint.go b/internal/codeguard/rules/catalog_security_taint.go index 0f2a49b..a52a6bc 100644 --- a/internal/codeguard/rules/catalog_security_taint.go +++ b/internal/codeguard/rules/catalog_security_taint.go @@ -23,4 +23,24 @@ var securityTaintCatalog = map[string]core.RuleMetadata{ Description: "Fails when untrusted input (input(), os.environ, sys.argv, web request attributes) flows into a dangerous sink such as os.system, subprocess with a shell or string command, eval/exec, or SQL execute with interpolated query text. The finding message includes the source-to-sink chain.", HowToFix: "Sanitize the value before the sink: use shlex.quote for shell arguments, parameterized cursor.execute arguments, or int/float parsing for numeric input.", }, + "security.ssrf.go": { + ID: "security.ssrf.go", + Section: "Security", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelGoNative, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguageGo), + Title: "Go server-side request forgery", + Description: "Fails when untrusted input flows into the URL of an outbound HTTP request (http.Get/Post/Head/PostForm/NewRequest), letting an attacker make the server reach arbitrary or internal hosts. The finding message includes the source-to-sink chain.", + HowToFix: "Validate the destination against an allowlist of trusted hosts and block private/link-local addresses before issuing the request.", + }, + "security.ssrf.python": { + ID: "security.ssrf.python", + Section: "Security", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelGoNative, + LanguageCoverage: core.FixedRuleLanguageCoverage(core.RuleLanguagePython), + Title: "Python server-side request forgery", + Description: "Fails when untrusted input flows into the URL of an outbound HTTP request (requests.get/post/etc., urllib urlopen), letting an attacker make the server reach arbitrary or internal hosts. The finding message includes the source-to-sink chain.", + HowToFix: "Validate the destination against an allowlist of trusted hosts and block private/link-local addresses before issuing the request.", + }, } diff --git a/internal/codeguard/runner/support/commands.go b/internal/codeguard/runner/support/commands.go index a9851d9..393d4bf 100644 --- a/internal/codeguard/runner/support/commands.go +++ b/internal/codeguard/runner/support/commands.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/devr-tools/codeguard/internal/codeguard/core" + "github.com/devr-tools/codeguard/internal/codeguard/trust" ) func RunCommandCheck(ctx context.Context, dir string, check core.CommandCheckConfig) (string, error) { @@ -48,6 +49,9 @@ func runDiffCommandCheck(ctx context.Context, diffEnv diffCommandEnv, baseRef st } func runCommandCheck(ctx context.Context, dir string, check core.CommandCheckConfig, env []string) (string, error) { + if err := trust.GuardConfigCommand(check.Name, check.Command); err != nil { + return "", err + } command := check.Command if strings.Contains(command, string(filepath.Separator)) && !filepath.IsAbs(command) { command = filepath.Join(dir, command) diff --git a/internal/codeguard/trust/guard.go b/internal/codeguard/trust/guard.go new file mode 100644 index 0000000..e854c25 --- /dev/null +++ b/internal/codeguard/trust/guard.go @@ -0,0 +1,40 @@ +package trust + +import "fmt" + +// ErrConfigCommandsDisabled is returned when codeguard is asked to run a +// command supplied by repository configuration while command execution from +// config is disabled (the default). +type ErrConfigCommandsDisabled struct { + // Context describes where the command came from (e.g. the check name). + Context string + // Command is the command codeguard refused to run. + Command string +} + +func (e ErrConfigCommandsDisabled) Error() string { + prefix := "" + if e.Context != "" { + prefix = e.Context + ": " + } + cmd := e.Command + if cmd == "" { + cmd = "" + } + return fmt.Sprintf( + "%srefusing to run config-supplied command %q: command execution from repository "+ + "configuration is disabled by default because codeguard may run against untrusted "+ + "pull requests. Enable it for trusted repositories by setting %s=1 or passing "+ + "--allow-config-commands.", + prefix, cmd, AllowConfigCommandsEnv) +} + +// GuardConfigCommand returns a non-nil error when execution of a config-supplied +// command is not permitted by the active trust policy. context describes the +// origin of the command (used in the error message); it may be empty. +func GuardConfigCommand(context, command string) error { + if AllowConfigCommands() { + return nil + } + return ErrConfigCommandsDisabled{Context: context, Command: command} +} diff --git a/internal/codeguard/trust/trust.go b/internal/codeguard/trust/trust.go new file mode 100644 index 0000000..2aab8eb --- /dev/null +++ b/internal/codeguard/trust/trust.go @@ -0,0 +1,87 @@ +// Package trust centralizes codeguard's trust-boundary policy. +// +// codeguard frequently runs in CI against pull requests authored by untrusted +// contributors, yet its behavior is driven by configuration (codeguard.yaml and +// rule packs) that is checked into the repository and therefore controllable by +// those same untrusted authors. To avoid turning a code review tool into a +// remote-code-execution or credential-exfiltration vector, potentially +// dangerous, config-driven capabilities are DISABLED BY DEFAULT and must be +// explicitly enabled by the trusted operator. +// +// The trust anchor is the process environment / CLI flags (controlled by the +// workflow author or local developer), never the repository config itself. +package trust + +import ( + "os" + "strings" + "sync" +) + +const ( + // AllowConfigCommandsEnv enables execution of commands supplied by the + // repository configuration (language/license/autofix commands and the + // "command" AI provider / nlrule / semantic runtimes). + AllowConfigCommandsEnv = "CODEGUARD_ALLOW_CONFIG_COMMANDS" + // AllowConfigAIEndpointsEnv enables AI provider base URLs that are not on + // the built-in public-provider allowlist, and relaxes the SSRF dial guard + // so self-hosted/internal endpoints can be reached. + AllowConfigAIEndpointsEnv = "CODEGUARD_ALLOW_CONFIG_AI_ENDPOINTS" +) + +// Policy captures which untrusted, config-driven capabilities are permitted. +type Policy struct { + // AllowConfigCommands permits execution of config-supplied commands. + AllowConfigCommands bool + // AllowConfigAIEndpoints permits non-allowlisted AI provider base URLs. + AllowConfigAIEndpoints bool +} + +var ( + mu sync.RWMutex + current = FromEnv() +) + +// FromEnv derives a Policy from environment variables. Unset/false by default. +func FromEnv() Policy { + return Policy{ + AllowConfigCommands: envEnabled(AllowConfigCommandsEnv), + AllowConfigAIEndpoints: envEnabled(AllowConfigAIEndpointsEnv), + } +} + +func envEnabled(name string) bool { + switch strings.ToLower(strings.TrimSpace(os.Getenv(name))) { + case "1", "true", "yes", "on": + return true + default: + return false + } +} + +// Current returns the active trust policy. +func Current() Policy { + mu.RLock() + defer mu.RUnlock() + return current +} + +// Set replaces the active trust policy. Used by CLI flag wiring and tests. +func Set(p Policy) { + mu.Lock() + defer mu.Unlock() + current = p +} + +// ResetFromEnv re-reads the policy from the environment. Used by tests. +func ResetFromEnv() { + Set(FromEnv()) +} + +// AllowConfigCommands reports whether config-supplied command execution is +// permitted under the active policy. +func AllowConfigCommands() bool { return Current().AllowConfigCommands } + +// AllowConfigAIEndpoints reports whether non-allowlisted AI provider base URLs +// are permitted under the active policy. +func AllowConfigAIEndpoints() bool { return Current().AllowConfigAIEndpoints } diff --git a/internal/githubaction/comment_client.go b/internal/githubaction/comment_client.go index 0953031..a342084 100644 --- a/internal/githubaction/comment_client.go +++ b/internal/githubaction/comment_client.go @@ -6,20 +6,12 @@ import ( "fmt" "io" "net/http" + "net/url" "strings" ) const MaxCommentBodyBytes = 65000 -type issueComment struct { - ID int64 `json:"id"` - Body string `json:"body"` -} - -type issueCommentRequest struct { - Body string `json:"body"` -} - type CommentPublisher struct { BaseURL string Token string @@ -50,8 +42,18 @@ func (p CommentPublisher) publishSticky(repository string, prNumber int, body st return p.createComment(repository, prNumber, body) } +// escapeRepository percent-encodes each segment of an "owner/repo" identifier +// so it cannot break out of or inject into the request path. +func escapeRepository(repository string) string { + parts := strings.Split(repository, "/") + for i, part := range parts { + parts[i] = url.PathEscape(part) + } + return strings.Join(parts, "/") +} + func (p CommentPublisher) listComments(repository string, prNumber int) ([]issueComment, error) { - req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/repos/%s/issues/%d/comments?per_page=100", p.BaseURL, repository, prNumber), nil) + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/repos/%s/issues/%d/comments?per_page=100", p.BaseURL, escapeRepository(repository), prNumber), nil) if err != nil { return nil, err } @@ -63,11 +65,11 @@ func (p CommentPublisher) listComments(repository string, prNumber int) ([]issue } func (p CommentPublisher) createComment(repository string, prNumber int, body string) error { - return p.sendCommentRequest(http.MethodPost, fmt.Sprintf("%s/repos/%s/issues/%d/comments", p.BaseURL, repository, prNumber), body, http.StatusCreated) + return p.sendCommentRequest(http.MethodPost, fmt.Sprintf("%s/repos/%s/issues/%d/comments", p.BaseURL, escapeRepository(repository), prNumber), body, http.StatusCreated) } func (p CommentPublisher) updateComment(repository string, commentID int64, body string) error { - return p.sendCommentRequest(http.MethodPatch, fmt.Sprintf("%s/repos/%s/issues/comments/%d", p.BaseURL, repository, commentID), body, http.StatusOK) + return p.sendCommentRequest(http.MethodPatch, fmt.Sprintf("%s/repos/%s/issues/comments/%d", p.BaseURL, escapeRepository(repository), commentID), body, http.StatusOK) } func (p CommentPublisher) sendCommentRequest(method string, url string, body string, wantStatus int) error { @@ -94,7 +96,7 @@ func (p CommentPublisher) doJSON(req *http.Request, wantStatus int, out any) err } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(io.LimitReader(resp.Body, 8<<20)) if err != nil { return err } diff --git a/internal/githubaction/comment_client_types.go b/internal/githubaction/comment_client_types.go new file mode 100644 index 0000000..b680946 --- /dev/null +++ b/internal/githubaction/comment_client_types.go @@ -0,0 +1,10 @@ +package githubaction + +type issueComment struct { + ID int64 `json:"id"` + Body string `json:"body"` +} + +type issueCommentRequest struct { + Body string `json:"body"` +} diff --git a/pkg/codeguard/sdk_config.go b/pkg/codeguard/sdk_config.go index 0b6515f..b0a6f11 100644 --- a/pkg/codeguard/sdk_config.go +++ b/pkg/codeguard/sdk_config.go @@ -29,19 +29,3 @@ func ValidateConfig(cfg Config) error { func ApplyDefaults(cfg *Config) { config.ApplyDefaults(cfg) } - -func Rules() []RuleMetadata { - return config.RuleList() -} - -func RulesForConfig(cfg Config) []RuleMetadata { - return config.RuleListForConfig(cfg) -} - -func ExplainRule(ruleID string) (RuleMetadata, bool) { - return config.ExplainRule(ruleID) -} - -func ExplainRuleForConfig(cfg Config, ruleID string) (RuleMetadata, bool) { - return config.ExplainRuleForConfig(cfg, ruleID) -} diff --git a/pkg/codeguard/sdk_coverage.go b/pkg/codeguard/sdk_coverage.go new file mode 100644 index 0000000..9150d45 --- /dev/null +++ b/pkg/codeguard/sdk_coverage.go @@ -0,0 +1,20 @@ +package codeguard + +import ( + "github.com/devr-tools/codeguard/internal/codeguard/config" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// OWASPCoverageEntry records which rules cover an OWASP Top 10 (2021) category. +type OWASPCoverageEntry = core.OWASPCoverageEntry + +// OWASPCoverage reports OWASP Top 10 (2021) coverage for the built-in rules. +func OWASPCoverage() []OWASPCoverageEntry { + return core.OWASPCoverageForRules(config.RuleList()) +} + +// OWASPCoverageForConfig reports OWASP Top 10 (2021) coverage for the rules +// active under cfg (including custom rule packs). +func OWASPCoverageForConfig(cfg Config) []OWASPCoverageEntry { + return core.OWASPCoverageForRules(config.RuleListForConfig(cfg)) +} diff --git a/pkg/codeguard/sdk_rules.go b/pkg/codeguard/sdk_rules.go new file mode 100644 index 0000000..f6404a3 --- /dev/null +++ b/pkg/codeguard/sdk_rules.go @@ -0,0 +1,19 @@ +package codeguard + +import "github.com/devr-tools/codeguard/internal/codeguard/config" + +func Rules() []RuleMetadata { + return config.RuleList() +} + +func RulesForConfig(cfg Config) []RuleMetadata { + return config.RuleListForConfig(cfg) +} + +func ExplainRule(ruleID string) (RuleMetadata, bool) { + return config.ExplainRule(ruleID) +} + +func ExplainRuleForConfig(cfg Config, ruleID string) (RuleMetadata, bool) { + return config.ExplainRuleForConfig(cfg, ruleID) +} diff --git a/tests/checks/ci_test_quality_helpers_test.go b/tests/checks/ci_test_quality_helpers_test.go index 9482ccd..3993d74 100644 --- a/tests/checks/ci_test_quality_helpers_test.go +++ b/tests/checks/ci_test_quality_helpers_test.go @@ -69,3 +69,24 @@ func TestCustomGuardHelperProcess(t *testing.T) { assertRuleCount(t, report, "ci.always-true-test-assertion", 0) assertRuleCount(t, report, "ci.conditional-assertion", 0) } + +func TestGoTestQualityExemptsTestMain(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main_test.go"), `package demo + +import ( + "os" + "testing" +) + +func TestMain(m *testing.M) { + os.Exit(m.Run()) +} +`) + + report := runScan(t, testQualityConfig(t, dir, "go")) + + assertRuleCount(t, report, "ci.test-without-assertion", 0) + assertRuleCount(t, report, "ci.always-true-test-assertion", 0) + assertRuleCount(t, report, "ci.conditional-assertion", 0) +} diff --git a/tests/checks/security_owasp_extra_test.go b/tests/checks/security_owasp_extra_test.go new file mode 100644 index 0000000..fead05d --- /dev/null +++ b/tests/checks/security_owasp_extra_test.go @@ -0,0 +1,119 @@ +package checks_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func runSecurity(t *testing.T, name, dir, language string) codeguard.Report { + t.Helper() + report, err := codeguard.Run(context.Background(), securityOnlyConfig(name, dir, language)) + if err != nil { + t.Fatalf("run: %v", err) + } + return report +} + +func TestSecurityDetectsMisconfiguration(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "server.py"), strings.Join([]string{ + "import flask", + "app = flask.Flask(__name__)", + "app.run(host='0.0.0.0', debug=True)", + "HEADERS = {'Access-Control-Allow-Origin': '*'}", + "", + }, "\n")) + writeFile(t, filepath.Join(dir, "Dockerfile"), "FROM alpine\nUSER root\n") + + report := runSecurity(t, "a05", dir, "python") + assertFindingRulePresent(t, report, "Security", "security.bind-all-interfaces") + assertFindingRulePresent(t, report, "Security", "security.debug-enabled") + assertFindingRulePresent(t, report, "Security", "security.cors-wildcard") + assertFindingRulePresent(t, report, "Security", "security.dockerfile-root") +} + +func TestSecurityDetectsWeakCrypto(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "crypto.py"), strings.Join([]string{ + "import hashlib", + "from Crypto.Cipher import AES, DES", + "digest = hashlib.md5(data).hexdigest()", + "cipher = AES.new(key, AES.MODE_ECB)", + "legacy = DES.new(key)", + "", + }, "\n")) + + report := runSecurity(t, "a02", dir, "python") + assertFindingRulePresent(t, report, "Security", "security.weak-hash") + assertFindingRulePresent(t, report, "Security", "security.weak-cipher") +} + +func TestSecurityDetectsInsecureDeserialization(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "loader.py"), strings.Join([]string{ + "import pickle, yaml", + "obj = pickle.loads(blob)", + "cfg = yaml.load(text)", + "", + }, "\n")) + + report := runSecurity(t, "a08", dir, "python") + assertFindingRulePresent(t, report, "Security", "security.insecure-deserialization") +} + +func TestSecurityIgnoresSafeYAMLLoad(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "safe.py"), strings.Join([]string{ + "import yaml", + "cfg = yaml.load(text, Loader=yaml.SafeLoader)", + "safe = yaml.safe_load(text)", + "", + }, "\n")) + + report := runSecurity(t, "a08-safe", dir, "python") + for _, section := range report.Sections { + for _, finding := range section.Findings { + if finding.RuleID == "security.insecure-deserialization" { + t.Fatalf("safe YAML loading should not be flagged: %s", finding.Message) + } + } + } +} + +func TestSecurityDetectsGoSSRF(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), strings.Join([]string{ + "package main", + "", + "import (", + "\t\"net/http\"", + "\t\"os\"", + ")", + "", + "func main() {", + "\ttarget := os.Getenv(\"TARGET_URL\")", + "\t_, _ = http.Get(target)", + "}", + "", + }, "\n")) + + report := runSecurity(t, "a10-go", dir, "go") + assertFindingRulePresent(t, report, "Security", "security.ssrf.go") +} + +func TestSecurityDetectsPythonSSRF(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "fetch.py"), strings.Join([]string{ + "import os, requests", + "url = os.environ['TARGET']", + "resp = requests.get(url)", + "", + }, "\n")) + + report := runSecurity(t, "a10-py", dir, "python") + assertFindingRulePresent(t, report, "Security", "security.ssrf.python") +} diff --git a/tests/checks/trust_main_test.go b/tests/checks/trust_main_test.go new file mode 100644 index 0000000..ffc1c65 --- /dev/null +++ b/tests/checks/trust_main_test.go @@ -0,0 +1,17 @@ +package checks_test + +import ( + "os" + "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/trust" +) + +// TestMain enables the trust opt-ins for this package. These tests exercise +// codeguard's command-driven checks and local (127.0.0.1) AI endpoints, which +// are gated off by default by the trust policy. The secure default itself is +// covered by dedicated tests in the trust, runner/support, and config packages. +func TestMain(m *testing.M) { + trust.Set(trust.Policy{AllowConfigCommands: true, AllowConfigAIEndpoints: true}) + os.Exit(m.Run()) +} diff --git a/tests/codeguard/trust_main_test.go b/tests/codeguard/trust_main_test.go new file mode 100644 index 0000000..5d59243 --- /dev/null +++ b/tests/codeguard/trust_main_test.go @@ -0,0 +1,17 @@ +package codeguard_test + +import ( + "os" + "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/trust" +) + +// TestMain enables the trust opt-ins for this package. These tests exercise +// command-driven AI providers and local (127.0.0.1) AI endpoints, which are +// gated off by default by the trust policy. The secure default itself is +// covered by dedicated tests in the trust, runner/support, and config packages. +func TestMain(m *testing.M) { + trust.Set(trust.Policy{AllowConfigCommands: true, AllowConfigAIEndpoints: true}) + os.Exit(m.Run()) +} diff --git a/tests/security/config_containment_test.go b/tests/security/config_containment_test.go new file mode 100644 index 0000000..f19bd10 --- /dev/null +++ b/tests/security/config_containment_test.go @@ -0,0 +1,57 @@ +package security_test + +import ( + "path/filepath" + "strings" + "testing" + + service "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func writeConfigWithBaseline(t *testing.T, dir, baselinePath string) string { + t.Helper() + cfg := service.ExampleConfig() + cfg.Baseline.Path = baselinePath + cfg.Cache.Path = "" + path := filepath.Join(dir, "codeguard.yaml") + if err := service.WriteConfigFile(path, cfg); err != nil { + t.Fatalf("write config: %v", err) + } + return path +} + +func TestLoadConfigRejectsBaselinePathEscape(t *testing.T) { + dir := t.TempDir() + path := writeConfigWithBaseline(t, dir, "../escape.json") + + _, err := service.LoadConfigFile(path) + if err == nil { + t.Fatal("expected config load to reject a baseline path escaping the config directory") + } + if !strings.Contains(err.Error(), "escape") { + t.Fatalf("expected containment error, got %v", err) + } +} + +func TestLoadConfigAllowsContainedBaselinePath(t *testing.T) { + dir := t.TempDir() + path := writeConfigWithBaseline(t, dir, ".codeguard/baseline.json") + + cfg, err := service.LoadConfigFile(path) + if err != nil { + t.Fatalf("contained baseline path should load, got %v", err) + } + want := filepath.Join(dir, ".codeguard/baseline.json") + if cfg.Baseline.Path != want { + t.Fatalf("baseline path = %q, want resolved %q", cfg.Baseline.Path, want) + } +} + +func TestLoadConfigRejectsAbsolutePathOutsideConfigDir(t *testing.T) { + dir := t.TempDir() + path := writeConfigWithBaseline(t, dir, "/etc/codeguard-baseline.json") + + if _, err := service.LoadConfigFile(path); err == nil { + t.Fatal("expected an absolute path outside the config directory to be rejected") + } +} diff --git a/tests/security/owasp_test.go b/tests/security/owasp_test.go new file mode 100644 index 0000000..8670145 --- /dev/null +++ b/tests/security/owasp_test.go @@ -0,0 +1,92 @@ +package security_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/core" + "github.com/devr-tools/codeguard/internal/codeguard/report" + service "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestEverySecurityRuleHasOWASPCategory(t *testing.T) { + valid := make(map[core.OWASPCategory]bool, len(core.OWASPTop10)) + for _, category := range core.OWASPTop10 { + valid[category] = true + } + + // security.command-check is intentionally unmapped: its category depends on + // the external command it wraps. + const exempt = "security.command-check" + + for _, rule := range service.Rules() { + if !strings.HasPrefix(rule.ID, "security.") || rule.ID == exempt { + continue + } + if rule.OWASPCategory == "" { + t.Errorf("security rule %q has no OWASP category", rule.ID) + continue + } + if !valid[rule.OWASPCategory] { + t.Errorf("security rule %q has unknown OWASP category %q", rule.ID, rule.OWASPCategory) + } + } +} + +func TestOWASPCoverageReportsGaps(t *testing.T) { + coverage := service.OWASPCoverage() + if len(coverage) != len(core.OWASPTop10) { + t.Fatalf("coverage entries = %d, want %d", len(coverage), len(core.OWASPTop10)) + } + byCode := map[string]service.OWASPCoverageEntry{} + for _, entry := range coverage { + byCode[entry.Code] = entry + } + if !byCode["A03:2021"].Covered || len(byCode["A03:2021"].RuleIDs) == 0 { + t.Error("expected A03 Injection to be covered") + } + if !byCode["A10:2021"].Covered { + t.Error("expected A10 SSRF to be covered by the SSRF taint rules") + } + // A04 Insecure Design and A09 Logging are intentional gaps: they are not + // reliably detectable by static heuristics. The report must surface them as + // gaps rather than imply coverage. + if byCode["A04:2021"].Covered { + t.Error("expected A04 Insecure Design to be reported as a coverage gap") + } + if byCode["A09:2021"].Covered { + t.Error("expected A09 Logging/Monitoring to be reported as a coverage gap") + } +} + +func TestSARIFOutputCarriesOWASPTag(t *testing.T) { + rep := core.Report{ + Sections: []core.SectionResult{{ + Name: "Security", + Findings: []core.Finding{{ + RuleID: "security.taint.go", + Level: "fail", + Message: "tainted input reaches exec.Command", + Path: "main.go", + Line: 10, + }}, + }}, + } + var buf bytes.Buffer + if err := report.Write(&buf, rep, "sarif"); err != nil { + t.Fatalf("write sarif: %v", err) + } + out := buf.String() + if !strings.Contains(out, "A03:2021") { + t.Fatalf("expected OWASP category in SARIF output, got: %s", out) + } + if !strings.Contains(out, "OWASP:A03:2021") { + t.Fatalf("expected OWASP tag in SARIF output, got: %s", out) + } + for _, want := range []string{"taxonomies", "OWASP Top 10", "relationships", "superset"} { + if !strings.Contains(out, want) { + t.Fatalf("expected SARIF to contain %q, got: %s", want, out) + } + } +} diff --git a/tests/security/safehttp_test.go b/tests/security/safehttp_test.go new file mode 100644 index 0000000..4b3f884 --- /dev/null +++ b/tests/security/safehttp_test.go @@ -0,0 +1,89 @@ +package security_test + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/devr-tools/codeguard/internal/codeguard/ai/safehttp" + "github.com/devr-tools/codeguard/internal/codeguard/trust" +) + +func TestValidateProviderURLAllowlist(t *testing.T) { + trust.Set(trust.Policy{}) + t.Cleanup(trust.ResetFromEnv) + + if err := safehttp.ValidateProviderURL("", false); err != nil { + t.Fatalf("empty URL should be accepted (provider default), got %v", err) + } + if err := safehttp.ValidateProviderURL("https://api.openai.com/v1", false); err != nil { + t.Fatalf("allowlisted host should be accepted, got %v", err) + } + if err := safehttp.ValidateProviderURL("https://evil.example.com/v1", false); err == nil { + t.Fatal("non-allowlisted host from untrusted config must be rejected") + } + if err := safehttp.ValidateProviderURL("file:///etc/passwd", false); err == nil { + t.Fatal("non-http scheme must be rejected") + } +} + +func TestValidateProviderURLTrustedSourceAndOptIn(t *testing.T) { + trust.Set(trust.Policy{}) + t.Cleanup(trust.ResetFromEnv) + + // Trusted source (e.g. an environment variable) bypasses the allowlist. + if err := safehttp.ValidateProviderURL("https://internal.host/v1", true); err != nil { + t.Fatalf("trusted-source URL should be accepted, got %v", err) + } + + // Operator opt-in bypasses the allowlist for config-sourced URLs too. + trust.Set(trust.Policy{AllowConfigAIEndpoints: true}) + if err := safehttp.ValidateProviderURL("https://internal.host/v1", false); err != nil { + t.Fatalf("opt-in should accept custom endpoint, got %v", err) + } +} + +func TestClientBlocksLoopbackByDefault(t *testing.T) { + trust.Set(trust.Policy{}) + t.Cleanup(trust.ResetFromEnv) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + client := safehttp.Client(5 * time.Second) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + if err != nil { + t.Fatal(err) + } + resp, err := client.Do(req) + if err == nil { + resp.Body.Close() + t.Fatal("expected SSRF guard to block connection to loopback test server") + } + if !strings.Contains(err.Error(), "ssrf guard") { + t.Fatalf("expected ssrf guard error, got %v", err) + } +} + +func TestClientAllowsLoopbackWhenOptedIn(t *testing.T) { + trust.Set(trust.Policy{AllowConfigAIEndpoints: true}) + t.Cleanup(trust.ResetFromEnv) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + client := safehttp.Client(5 * time.Second) + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + resp, err := client.Do(req) + if err != nil { + t.Fatalf("opt-in client should reach loopback endpoint, got %v", err) + } + resp.Body.Close() +} diff --git a/tests/security/trust_test.go b/tests/security/trust_test.go new file mode 100644 index 0000000..e3c9de6 --- /dev/null +++ b/tests/security/trust_test.go @@ -0,0 +1,49 @@ +package security_test + +import ( + "errors" + "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/trust" +) + +func TestGuardConfigCommandRefusesByDefault(t *testing.T) { + trust.Set(trust.Policy{}) + t.Cleanup(trust.ResetFromEnv) + + err := trust.GuardConfigCommand("language security command", "/bin/echo") + if err == nil { + t.Fatal("expected command execution to be refused under the default policy") + } + var disabled trust.ErrConfigCommandsDisabled + if !errors.As(err, &disabled) { + t.Fatalf("expected ErrConfigCommandsDisabled, got %T: %v", err, err) + } + if disabled.Command != "/bin/echo" { + t.Fatalf("error did not capture command, got %q", disabled.Command) + } +} + +func TestGuardConfigCommandAllowedWhenOptedIn(t *testing.T) { + trust.Set(trust.Policy{AllowConfigCommands: true}) + t.Cleanup(trust.ResetFromEnv) + + if err := trust.GuardConfigCommand("ctx", "/bin/echo"); err != nil { + t.Fatalf("expected command to be permitted, got %v", err) + } +} + +func TestFromEnvParsesTruthyValues(t *testing.T) { + for _, value := range []string{"1", "true", "TRUE", "yes", "on"} { + t.Setenv(trust.AllowConfigCommandsEnv, value) + if !trust.FromEnv().AllowConfigCommands { + t.Fatalf("value %q should enable config commands", value) + } + } + for _, value := range []string{"", "0", "false", "no", "off", "maybe"} { + t.Setenv(trust.AllowConfigCommandsEnv, value) + if trust.FromEnv().AllowConfigCommands { + t.Fatalf("value %q should not enable config commands", value) + } + } +}