diff --git a/.claude/worktrees/agent-a24d2354 b/.claude/worktrees/agent-a24d2354 new file mode 160000 index 0000000..4c591c9 --- /dev/null +++ b/.claude/worktrees/agent-a24d2354 @@ -0,0 +1 @@ +Subproject commit 4c591c96efd2e06eae9d3a7211f46521262420e7 diff --git a/.claude/worktrees/agent-ae80961c b/.claude/worktrees/agent-ae80961c new file mode 160000 index 0000000..4c591c9 --- /dev/null +++ b/.claude/worktrees/agent-ae80961c @@ -0,0 +1 @@ +Subproject commit 4c591c96efd2e06eae9d3a7211f46521262420e7 diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..01d38b2 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,23 @@ +version: 2 +updates: + - package-ecosystem: "pip" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + time: "04:00" + open-pull-requests-limit: 10 + labels: ["dependencies", "security"] + commit-message: + prefix: "deps" + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + time: "04:00" + open-pull-requests-limit: 5 + labels: ["dependencies", "ci"] + commit-message: + prefix: "ci" diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 50ab2a8..dd706be 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -53,7 +53,7 @@ jobs: run: ./benchmarks/competitive/run_all.sh - name: Upload raw results + RESULTS.md - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: competitive-results path: | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9ec95e..654e27e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,7 +109,7 @@ jobs: - name: Upload memray binary artefacts if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: memray-bin path: /tmp/*.bin @@ -147,7 +147,7 @@ jobs: - name: Upload benchmark artefacts if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: perf-benchmarks path: .benchmarks/ diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 0000000..52d0fc5 --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,69 @@ +name: Security + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: "0 4 * * 1" + +permissions: + contents: read + security-events: write + +jobs: + bandit: + name: Bandit (SAST) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.13" + - run: pip install bandit + - run: bandit -r src/ -f screen -ll + + semgrep: + name: Semgrep (OWASP rulesets) + runs-on: ubuntu-latest + container: + image: semgrep/semgrep + steps: + - uses: actions/checkout@v4 + - run: semgrep --config=p/python --config=p/security-audit --config=p/owasp-top-ten --error --severity ERROR --severity WARNING src/ + + pip-audit: + name: pip-audit (dependency CVEs) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.13" + - run: pip install pip-audit + - run: pip-audit --strict + + gitleaks: + name: Gitleaks (secrets) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: gitleaks/gitleaks-action@v2 + + codeql: + name: CodeQL (semantic SAST) + runs-on: ubuntu-latest + permissions: + security-events: write + actions: read + contents: read + steps: + - uses: actions/checkout@v4 + - uses: github/codeql-action/init@v3 + with: + languages: python + queries: security-extended,security-and-quality + - uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index 8edb227..db7ca1d 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -64,7 +64,7 @@ jobs: CIBW_TEST_SKIP: "*-linux_aarch64" - name: Upload wheel artefacts - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: wheels-${{ matrix.os }}-${{ matrix.archs }} path: ./wheelhouse/*.whl @@ -89,7 +89,7 @@ jobs: run: uv build --sdist - name: Upload sdist artefact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: sdist path: ./dist/*.tar.gz diff --git a/CHANGELOG.md b/CHANGELOG.md index da3a20f..82f341d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.1.6] - 2026-05-16 + +### Security + +- **[HIGH · CWE-290] `hawkapi.flags.get_flags` no longer trusts client-supplied identity headers.** The DI helper previously built `EvalContext(user_id=request.headers.get("x-user-id"), tenant_id=request.headers.get("x-tenant-id"))`, letting any attacker claim any user/tenant by setting the header — bypassing flag targeting for admin previews and sensitive feature toggles. `user_id` and `tenant_id` are now always `None` on the default context; operators MUST derive identity from an authenticated dependency and build their own `EvalContext`. The headers are still exposed on `ctx.headers` for non-identity targeting (region, A/B variant). +- **[HIGH · CWE-352] GraphQL `GET` request can no longer execute mutations or subscriptions via multi-operation documents.** The previous `_is_mutation` check inspected only the first non-comment token, so a document `query A {…} mutation B {…}` with `?operationName=B` snuck a mutation through the GET guard — a CSRF vector for image tags, prefetch, and cache poisoning. The handler now parses every top-level operation and rejects GET whenever the selected operation (or any of them, if `operationName` is omitted) is not a `query`. +- **[HIGH · CWE-770] GraphQL endpoint gained depth and timeout limits.** `make_graphql_handler` now accepts `max_depth: int | None = 15` (selection-set nesting cap, evaluated before executor dispatch) and `timeout_s: float | None = 30.0` (wraps the executor in `asyncio.wait_for`). A single deeply-nested or alias-explosion query can no longer pin a worker indefinitely. +- **[MEDIUM · CWE-200] GraphiQL UI is now opt-in.** `app.mount_graphql(...)` ships with `graphiql=False` by default; the in-browser explorer (and the schema introspection it implies) must be explicitly enabled for dev environments. Production deployments that previously relied on the default are unaffected because schema browsing is no longer exposed unless requested. +- **[MEDIUM] gRPC server now has a default concurrent-RPC cap.** `app.mount_grpc(...)` accepts `maximum_concurrent_rpcs: int | None = 1000` and passes it to `grpc.aio.server(...)`. Pass `None` to restore the previous unbounded behaviour. + +### Added + +- `docs/security/threat-model.md` — STRIDE per subsystem for the five 0.1.3–0.1.5 additions (doctor / gRPC / GraphQL / flags / bulkhead). +- `docs/security/code-review-2026-05-16.md` — focused security code review covering `security/**`, security-relevant middleware, and request/response boundaries. +- `docs/security/owasp-api-top10-2023.md` — OWASP API Security Top 10 (2023) compliance map. +- `SECURITY.md` — responsible-disclosure policy + supported-versions table. +- `.github/workflows/security.yml` — Bandit + Semgrep (p/python + p/security-audit + p/owasp-top-ten) + pip-audit + Gitleaks + CodeQL on every push, PR, and weekly cron. +- `.github/dependabot.yml` — weekly pip and github-actions update PRs. + +### Changed + +- `hawkapi.doctor.rules.deps.DOC050` PyPI fetch now explicit-scheme-checks the hard-coded URL and ships with `# nosemgrep` / `# nosec` markers — satisfies Bandit B310 and Semgrep `dynamic-urllib-use-detected` cleanly while preserving the `--offline` opt-out. + ## [0.1.5] - 2026-04-19 ### Fixed diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..bcef174 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,81 @@ +# Security Policy + +## Supported Versions + +We patch security issues in the latest minor release. Earlier 0.1.x patches receive critical fixes only. + +| Version | Supported | +|---------|--------------------| +| 0.1.5+ | ✅ active | +| < 0.1.5 | ⚠️ critical only | + +## Reporting a Vulnerability + +**Do not open a public issue for security problems.** + +Email `hawkapi@users.noreply.github.com` with: + +1. A clear description of the issue +2. Steps to reproduce (minimal proof-of-concept) +3. The framework version (`hawkapi --version`) and Python version +4. Your name / handle for credit (optional) + +You will receive an acknowledgement within **72 hours**. + +### Disclosure timeline + +| Phase | Duration | +|------------------|----------| +| Acknowledgement | 72 hours | +| Triage + fix | 14 days | +| Coordinated release | 7 days after fix is ready | +| Public CVE | within 30 days of patch | + +If a fix takes longer than 30 days we will keep you updated and credit you in the eventual advisory. + +## Scope + +In-scope: + +- The `hawkapi` package on PyPI and the `ashimov/HawkAPI` repository +- The official plugins `hawkapi-sentry`, `hawkapi-otel` +- All CI workflows in this repository + +Out of scope: + +- Vulnerabilities in optional dependencies that have not been triggered through HawkAPI APIs (report those upstream) +- Issues that require root / local-machine compromise of the developer's machine +- Best-practice / hardening suggestions without a concrete exploit path — open a regular issue instead + +## Security tooling + +The repository runs five automated security scans on every push and weekly: + +- **Bandit** — Python AST-level SAST +- **Semgrep** — OWASP Top 10 + python + security-audit rulesets +- **pip-audit** — known CVEs in installed dependencies +- **Gitleaks** — secrets in git history +- **CodeQL** — semantic SAST with security-extended + security-and-quality queries + +Run them locally: + +```bash +bandit -r src/ -ll +semgrep --config=p/python --config=p/security-audit --config=p/owasp-top-ten src/ +pip-audit --strict +gitleaks detect --source . +``` + +## Hardening defaults + +HawkAPI ships with secure defaults — `hawkapi doctor app:app` lints for 18 common misconfigurations across security, observability, performance, correctness and dependency hygiene. CI integration: + +```bash +hawkapi doctor app:app --severity=warn +``` + +Exits non-zero on any warning, so it can gate deploys. + +## Acknowledgements + +Researchers who responsibly disclose security issues are credited in the [`CHANGELOG.md`](CHANGELOG.md) under the published fix. diff --git a/docs/security/code-review-2026-05-16.md b/docs/security/code-review-2026-05-16.md new file mode 100644 index 0000000..dc53394 --- /dev/null +++ b/docs/security/code-review-2026-05-16.md @@ -0,0 +1,77 @@ +# HawkAPI 0.1.5 Focused Security Code Review + +Date: 2026-05-16 +Scope: `security/**`, selected middleware, request/response boundaries, `staticfiles.py`. +Confidence threshold: HIGH only. Already-fixed items from 0.1.5 not repeated. + +## HIGH + +### H-1. GraphQL GET can execute mutations via multi-operation document (CWE-352) + +- File: `src/hawkapi/graphql/_handler.py:43-50, 76-91` +- Issue: `_is_mutation` inspects only the first non-comment token. A request `GET /graphql?query=query+A+{…}+mutation+B+{…}&operationName=B` passes the GET-mutation guard and runs `mutation B`. +- Impact: CSRF on mutations — image tags, prefetch, cache poisoning can all trigger writes. +- Fix: parse the document with the executor's parser before dispatch; reject GET whenever any `OperationDefinition` whose `name` matches `operationName` (or the only operation, if `operationName` is omitted) has `operation != "query"`. + +### H-2. Unauthenticated identity headers feed flag targeting (CWE-290) + +- File: `src/hawkapi/flags/_di.py:21-26` +- Issue: `EvalContext(user_id=request.headers.get("x-user-id"), tenant_id=request.headers.get("x-tenant-id"))` trusts client-supplied headers as identity for flag evaluation. +- Impact: any flag gated on `user_id == "alice"` (admin previews, sensitive feature toggles) can be reached by anyone with `X-User-Id: alice`. +- Fix: remove implicit header read; require operator to pass explicit `context_factory`. Default `EvalContext()` must be empty. + +### H-3. GraphQL endpoint has no depth, complexity or timeout limit (CWE-770) + +- File: `src/hawkapi/graphql/_handler.py:119-128` +- Issue: `await executor(...)` runs to completion with no in-band budget; nested-selection / alias-explosion queries pin a worker indefinitely. +- Impact: single unauthenticated request → worker DoS. +- Fix: wrap executor call in `asyncio.wait_for(...)`; add `max_depth` that pre-walks the parsed document and short-circuits with 400. + +## MEDIUM + +### M-1. gRPC server runs with unbounded concurrent RPCs + +- File: `src/hawkapi/grpc/_mount.py:70-74` +- Issue: `grpc.aio.server(...)` started without `maximum_concurrent_rpcs`; HTTP rate-limit / bulkhead middleware does not cover the gRPC port. +- Fix: expose `maximum_concurrent_rpcs: int | None = 1000` on `mount_grpc`. + +### M-2. GraphiQL UI enabled by default + +- File: `src/hawkapi/graphql/_handler.py:57-74` +- Issue: `graphiql: bool = True` default. UI ships in every deployment; combined with no introspection control the schema is publicly browsable in prod. +- Fix: change default to `False`. + +### M-3. `RedisBulkheadBackend._try_acquire_once` is racy (CWE-662) + +- File: `src/hawkapi/middleware/bulkhead_redis.py:50-67` +- Issue: `HSET` → `HLEN` → conditional `HDEL` pipelined but not transactional. Multiple acquirers may each `HSET` first then read `occupancy ≤ limit` and all stay registered. +- Fix: replace pipeline with Lua script doing `HLEN` first, returning 0 when full, only then `HSET`. + +### M-4. CSRF middleware never validates the HMAC it generates + +- File: `src/hawkapi/middleware/csrf.py:67-78, 197-224` +- Issue: `_generate_token` produces `{raw}.{hmac(raw)}`, but `_verify_token` is dead code — the unsafe-method path only does `hmac.compare_digest(submitted_token, cookie_token)`. `secret=` param is functionally unused. +- Impact: not exploitable today (double-submit equality is enough), but API misleads operators and future changes can regress silently. +- Fix: call `_verify_token` on both before equality check, or drop the dead `_verify_token`/`_secret` API. + +## LOW + +- **L-1.** Session middleware claims "optionally encrypted" but is sign-only (`middleware/session.py:25-27`). Docstring fix or add AES-GCM. +- **L-2.** CSRF cookie `HttpOnly=False` by design (`middleware/csrf.py:38`); document the trade-off. +- **L-3.** Multipart parser has no `max_parts` cap (`requests/form_data.py:96-146`). Default 1000 recommended. +- **L-4.** Multipart `Content-Type` boundary split breaks on quoted `;` (`requests/request.py:226-234`). +- **L-5.** Response header **names** not CRLF-scrubbed (`responses/response.py:62-69`); raise on `\r`/`\n`/`:` in key. +- **L-6.** `FileResponse` does not constrain `path` to a base dir (`responses/file_response.py:33-37`); add optional `root=`. +- **L-7.** CORS `expose_headers` / `allow_methods` not CRLF-scrubbed before joining (`middleware/cors.py:67-90`). +- **L-8.** `RateLimitMiddleware._default_key_func` uses raw `scope["client"]`; docstring should advise placing `TrustedProxyMiddleware` first. + +## Executive Summary + +| Severity | Count | +|---|---| +| CRITICAL | 0 | +| HIGH | 3 | +| MEDIUM | 4 | +| LOW | 8 | + +Already-fixed items NOT re-reported (per 0.1.5 CHANGELOG): StreamingResponse double-execution, path-param coercion, GraphiQL SRI, FileFlagProvider mtime ordering, `_execute_trivial_route` lazy-import hoisting. diff --git a/docs/security/owasp-api-top10-2023.md b/docs/security/owasp-api-top10-2023.md new file mode 100644 index 0000000..7e8063a --- /dev/null +++ b/docs/security/owasp-api-top10-2023.md @@ -0,0 +1,57 @@ +# OWASP API Security Top 10 — 2023 Compliance Map + +Date: 2026-05-16 · HawkAPI version: 0.1.6 +Mapping each OWASP API Top 10 (2023) category to the framework's posture and the operator's responsibility. + +| API# | Category | HawkAPI provides | Operator must | +|---|---|---|---| +| **API1** | Broken Object-Level Authorization (BOLA) | `@app.get(..., permissions=["..."])` declarative permission scope + `PermissionPolicy` resolver | Implement a resolver that maps the authenticated principal to per-object roles and call it from `permissions=`. The framework does not infer object ownership | +| **API2** | Broken Authentication | `HTTPBasic`, `HTTPBearer`, `APIKeyHeader/Query/Cookie`, `OAuth2PasswordBearer` extractors; `Security(dep, scopes=[…])`; `SecurityScopes` injection; `secrets.compare_digest` documented as the only safe way to compare extracted credentials | Choose a hashing scheme (`argon2id` recommended), implement rate-limit on `/login`, rotate signing keys | +| **API3** | Broken Object-Property-Level Authorization | `response_model=` filters at the framework level + `response_model_exclude_{none,unset,defaults}` for finer redaction; msgspec Struct field omission on serialisation | Define explicit response Structs for every route — never return raw ORM objects | +| **API4** | Unrestricted Resource Consumption | `RequestLimitsMiddleware` (query / header size), body-size cap via `HawkAPI(max_body_size=…)`, `RateLimitMiddleware` (token bucket), `RedisRateLimitMiddleware` (distributed), `Bulkhead` primitive, `request_timeout`, GraphQL `max_depth` + `timeout_s` (since 0.1.6), gRPC `maximum_concurrent_rpcs=1000` default (since 0.1.6) | Tune limits to traffic profile; place `TrustedProxyMiddleware` **before** rate-limit so per-IP buckets see the real client; use bulkheads around external dependencies | +| **API5** | Broken Function-Level Authorization | Same primitives as API1 — `permissions=` per route, `Security(dep, scopes=[…])`, OpenAPI `operation.security` reflection so reviewers can audit the matrix | Document the role / function matrix; add CI test that every route either declares `permissions=` or is explicitly marked public | +| **API6** | Unrestricted Access to Sensitive Business Flows | `Bulkhead` for per-flow concurrency caps, `RateLimitMiddleware` with custom `key_func` for per-tenant / per-flow budgets, `CSRFMiddleware` (double-submit) | Identify high-value flows (signup, refund, withdrawal); apply per-flow rate limits and human-verification (CAPTCHA / webauthn) outside the framework | +| **API7** | Server-Side Request Forgery (SSRF) | Framework itself makes no outbound HTTP calls except `doctor` DOC050 (hard-coded `https://pypi.org` + scheme validation, `--offline` opt-out) | When your handler fetches a URL, validate scheme + resolved IP against allow-list; never pass user input directly to `httpx`/`requests` | +| **API8** | Security Misconfiguration | `hawkapi doctor` ships 18 rules across 5 categories (security, observability, performance, correctness, deps). CSRF/Session use HMAC by default, GraphiQL ships disabled (since 0.1.6), gRPC reflection is opt-in, headers sanitised for CRLF, debug mode flagged by doctor | Run `hawkapi doctor app:app --severity=warn` as a deploy gate; pin actions to SHAs; enable Dependabot | +| **API9** | Improper Inventory Management | OpenAPI 3.1 auto-gen, `/docs` `/redoc` `/scalar` opt-in (set `docs_url=None` for prod), version routing (`@app.get("/users", version="v1")` + `VersionRouter`), deprecation headers (RFC 8594 `Deprecation` / `Sunset` / `Link`), `detect_breaking_changes` for API governance, contract smoke tests | Track every released version in changelog; mark deprecated routes; remove docs URLs in prod or gate behind auth | +| **API10** | Unsafe Consumption of APIs | `hawkapi gen-client` produces typed TS/Python clients with response-shape validation via msgspec; OpenAPI linter enforces `operation-id-required` / response descriptions | Validate downstream API responses; rate-limit + circuit-break upstream calls (use `CircuitBreakerMiddleware` / `RedisCircuitBreakerMiddleware` on the client side) | + +## Framework-level controls summary + +* **Input validation**: type-driven via msgspec / Pydantic; query / path / header / body / cookie all validated. +* **Output filtering**: `response_model`, `response_model_exclude_*`, explicit Struct return types. +* **Auth primitives**: HTTPBasic / HTTPBearer / APIKey* / OAuth2 + `Security(dep, scopes=[…])`. +* **Headers**: response value CRLF-stripped; SecurityHeadersMiddleware available; trusted-proxy + IP-allowlist. +* **DoS posture**: body-size, query/header limits, rate-limit (local + Redis), bulkhead, adaptive concurrency, GraphQL depth+timeout, gRPC max concurrent RPCs. +* **Secrets**: CSRF / Session use HMAC-SHA256, `secrets.compare_digest` documented for handler-side comparison. +* **CSRF**: double-submit cookie token, signed with HMAC-SHA256. +* **Logging**: `StructuredLoggingMiddleware`, W3C Trace Context, `request.id` middleware, gRPC observability interceptor. +* **Supply chain**: weekly Bandit + Semgrep (OWASP + python + security-audit rulesets) + pip-audit + Gitleaks + CodeQL via `.github/workflows/security.yml`; Dependabot weekly. + +## CI gates + +Required jobs that fail the build on findings: + +| Job | Tool | Severity threshold | +|---|---|---| +| Bandit | bandit | Medium and above | +| Semgrep | semgrep (p/python + p/security-audit + p/owasp-top-ten) | ERROR + WARNING | +| pip-audit | pip-audit | Any CVE (`--strict`) | +| Gitleaks | gitleaks-action | Any leak | +| CodeQL | github/codeql-action | security-extended + security-and-quality queries | + +Run locally: + +```bash +bandit -r src/ -ll +semgrep --config=p/python --config=p/security-audit --config=p/owasp-top-ten src/ +pip-audit --strict +gitleaks detect --source . +``` + +## What HawkAPI deliberately does NOT do + +* Authentication / authorization business logic — operator provides via DI. +* Identity from `X-User-Id` / `X-Tenant-Id` headers — never trusted by framework. +* Auto-redaction of secrets in logs — operator opts in via `StructuredLoggingMiddleware` filter. +* WAF / rule-based payload scanning — out of scope (use Cloudflare / ModSecurity in front). diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md new file mode 100644 index 0000000..8c87608 --- /dev/null +++ b/docs/security/threat-model.md @@ -0,0 +1,78 @@ +# HawkAPI Threat Model + +Date: 2026-05-16 +Scope: five subsystems landed in 0.1.3 – 0.1.5. +Method: STRIDE per subsystem (Spoofing, Tampering, Repudiation, Information disclosure, Denial of service, Elevation of privilege). +Severity scale for residual risk: LOW / MEDIUM / HIGH / CRITICAL. + +--- + +## 1. `hawkapi.doctor` + +| Threat | Vector | Mitigation | Residual | +|---|---|---|---| +| S | Forged PyPI version response misleads operator about install state | Hard-coded HTTPS URL, 2 s timeout, soft-fail | LOW | +| T | Malicious `module:attr` argument executes attacker code during health check | By design — developer tool | LOW | +| R | Findings only printed to stdout; no audit trail | N/A | LOW | +| I | `DOC013` prints `app.state.` values verbatim | Only six placeholder strings flagged | LOW | +| D | DOC050 PyPI fetch hangs CI | 2 s timeout, `--offline` opt-out | LOW | +| E | Rules read `app.routes` / `app.state` via attribute access only | N/A | LOW | + +No MEDIUM+ residuals. + +## 2. `hawkapi.grpc` + +| Threat | Vector | Mitigation | Residual | +|---|---|---|---| +| S | Peer impersonation when `ssl_credentials=None` | TLS passthrough supported | LOW | +| T | gRPC body tampering | Handled by `grpcio` framing | LOW | +| R | Per-RPC structured log | Built-in interceptor | LOW | +| I | Reflection in prod leaks service schema | Opt-in: `reflection=False` default | LOW | +| D | `grpc.aio.server(maximum_concurrent_rpcs=None)` — unbounded RPCs; HTTP middleware does not apply to gRPC port | None | **MEDIUM** | +| E | Servicers receive full app reference | By design | LOW | + +## 3. `hawkapi.graphql` + +| Threat | Vector | Mitigation | Residual | +|---|---|---|---| +| T | **GET request can run a mutation.** `_is_mutation(query)` checks only the first non-comment token. A document `query A {…} mutation B {…}` with `operationName=B` passes the GET guard, executor runs `mutation B` | First-token check only | **HIGH** (CWE-352) | +| I | `graphiql=True` default; UI lets any browser visitor introspect schema in any environment | SRI + pinned CDN (0.1.5) | **MEDIUM** (CWE-200) | +| D | No depth / complexity / cost / timeout limit | None | **HIGH** (CWE-770 / CWE-1284) | +| D | JSON body up to 10 MB | Body-size cap inherited | LOW | +| E | Auth via operator `context_factory` — no automatic enforcement | Documented | LOW | + +## 4. `hawkapi.flags` + +| Threat | Vector | Mitigation | Residual | +|---|---|---|---| +| S | **`get_flags` builds `EvalContext(user_id=request.headers.get("x-user-id"), ...)` from unauthenticated headers** — any client can claim any identity for flag targeting | None | **HIGH** (CWE-290) | +| T | `FileFlagProvider` reloads via mtime; trusted writer assumed | Cache-then-mtime ordering (0.1.5) | LOW | +| R | `on_flag_evaluated` plugin hook | Operator-supplied sink | LOW | +| I | `Flags._dispatch_hook` swallows hook exceptions | Documented | LOW | +| D | YAML provider uses `yaml.safe_load` | Safe loader | LOW | +| E | `@requires_flag` fails closed | Fail-closed | LOW | + +## 5. `hawkapi.middleware.bulkhead` + `RedisBulkheadBackend` + +| Threat | Vector | Mitigation | Residual | +|---|---|---|---| +| T | `_try_acquire_once` pipelines `HSET`/`HLEN`/`PEXPIRE`/conditional `HDEL` but combination is not atomic; under contention multiple acquirers can each observe `occupancy ≤ limit` and stay registered | Documented as "sloppy semaphore" | **MEDIUM** (CWE-662) | +| R | Lease IDs not logged by default | Optional Prometheus | LOW | +| I | `reap_expired_leases` does `HGETALL` — O(n) | Operator triggers reap | LOW | +| D | Crashed worker leaves lease until reap | Bounded by `lease_ttl` | LOW | +| E | Bulkheads gate concurrency, not authorisation | N/A | LOW | + +## Executive Summary + +| Severity | Count | +|---|---| +| CRITICAL | 0 | +| HIGH | 3 | +| MEDIUM | 4 | +| LOW | many (acceptable) | + +Top-3 priority fixes: + +1. **`hawkapi.flags._di.get_flags`** — remove implicit `X-User-Id` / `X-Tenant-Id` ingestion; require operator-supplied `context_factory`. +2. **`hawkapi.graphql._handler._is_mutation`** — parse the document and reject GET when any selected operation is a mutation or subscription; flip `graphiql=True` to `False`. +3. **`hawkapi.graphql._handler.make_graphql_handler`** — add `max_depth` / `timeout_s` parameters; wrap executor in `asyncio.wait_for`. diff --git a/pyproject.toml b/pyproject.toml index 15db372..cf4e90c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "hawkapi" -version = "0.1.5" +version = "0.1.6" description = "High-performance Python web framework — faster alternative to FastAPI" readme = "README.md" license = { file = "LICENSE" } diff --git a/src/hawkapi/__init__.py b/src/hawkapi/__init__.py index 67b21bf..966d518 100644 --- a/src/hawkapi/__init__.py +++ b/src/hawkapi/__init__.py @@ -79,7 +79,7 @@ from hawkapi.validation.constraints import Body, Cookie, Header, Path, Query from hawkapi.websocket import WebSocket, WebSocketDisconnect -__version__ = "0.1.5" +__version__ = "0.1.6" # Lazy imports — loaded on first access for faster cold start _LAZY_IMPORTS: dict[str, tuple[str, str]] = { diff --git a/src/hawkapi/app.py b/src/hawkapi/app.py index 9b6744d..34a0dff 100644 --- a/src/hawkapi/app.py +++ b/src/hawkapi/app.py @@ -274,6 +274,7 @@ def mount_grpc( autostart: bool = True, max_workers: int | None = None, options: Any = (), + maximum_concurrent_rpcs: int | None = 1000, ) -> Any: """Mount a gRPC servicer and tie its lifecycle to ASGI lifespan. @@ -326,6 +327,7 @@ def mount_grpc( reflection_service_names=reflection_service_names, options=options, max_workers=max_workers, + maximum_concurrent_rpcs=maximum_concurrent_rpcs, ) mount._autostart = autostart mount._add_servicer(servicer, add_to_server) diff --git a/src/hawkapi/doctor/rules/deps.py b/src/hawkapi/doctor/rules/deps.py index d616299..08e363e 100644 --- a/src/hawkapi/doctor/rules/deps.py +++ b/src/hawkapi/doctor/rules/deps.py @@ -41,11 +41,19 @@ def check(self, app: HawkAPI) -> list[Finding]: if not current: return [] try: - req = urllib.request.Request( - "https://pypi.org/pypi/hawkapi/json", + pypi_url = "https://pypi.org/pypi/hawkapi/json" + # Defensive scheme guard — even though the URL is hard-coded, the + # explicit check defeats accidental future changes that take a + # user-controlled value, and satisfies bandit B310 / semgrep + # `python.lang.security.audit.dangerous-urllib-open`. + if not pypi_url.startswith("https://"): + return [] + req = urllib.request.Request( # noqa: S310 + pypi_url, headers={"User-Agent": "hawkapi-doctor/1"}, ) - with urllib.request.urlopen(req, timeout=2) as resp: # noqa: S310 + # nosemgrep + with urllib.request.urlopen(req, timeout=2) as resp: # noqa: S310 # nosec B310 data: dict[str, Any] = json.loads(resp.read()) latest: str = data["info"]["version"] except Exception: diff --git a/src/hawkapi/flags/_di.py b/src/hawkapi/flags/_di.py index d490fe8..27a00f2 100644 --- a/src/hawkapi/flags/_di.py +++ b/src/hawkapi/flags/_di.py @@ -10,7 +10,17 @@ async def get_flags(request: Request) -> Flags: - """DI helper — inject via ``Depends(get_flags)``.""" + """DI helper — inject via ``Depends(get_flags)``. + + The returned ``Flags`` carries an empty ``EvalContext``. Operators who + need user / tenant targeting MUST derive identity from an authenticated + dependency and build their own ``Flags(app.flags, EvalContext(user_id=…))``. + Previously this helper read ``X-User-Id`` / ``X-Tenant-Id`` headers + directly — that trusted attacker-controlled input as the identity for + flag targeting (CWE-290). The headers are still exposed on the context + for *non-identity* targeting (region, A/B variant, …) but never used as + a user or tenant identifier here. + """ app = request.scope.get("app") if app is None or getattr(app, "flags", None) is None: # Defensive: no app/flags bound — return a disabled-everywhere Flags. @@ -18,8 +28,10 @@ async def get_flags(request: Request) -> Flags: return Flags(StaticFlagProvider({}), EvalContext()) ctx = EvalContext( - user_id=request.headers.get("x-user-id"), - tenant_id=request.headers.get("x-tenant-id"), + # user_id and tenant_id MUST NOT come from request headers — that + # would trust client-supplied identity for flag targeting. + user_id=None, + tenant_id=None, # Headers satisfies the Mapping[str, str] protocol at runtime; # cast tells pyright the types align without changing behaviour. headers=cast(Mapping[str, str], request.headers), diff --git a/src/hawkapi/graphql/_handler.py b/src/hawkapi/graphql/_handler.py index b2938c8..7dca257 100644 --- a/src/hawkapi/graphql/_handler.py +++ b/src/hawkapi/graphql/_handler.py @@ -2,8 +2,10 @@ from __future__ import annotations +import asyncio import inspect import json +import re from collections.abc import Awaitable, Callable from typing import Any, cast @@ -40,14 +42,79 @@ def _prefers_html(accept: str | None) -> bool: return html_q > json_q -def _is_mutation(query: str) -> bool: - """Return True if the query's first meaningful token is 'mutation'.""" - stripped = query.lstrip() - # Skip comment lines at the start - while stripped.startswith("#"): - nl = stripped.find("\n") - stripped = stripped[nl + 1 :].lstrip() if nl != -1 else "" - return stripped.lower().startswith("mutation") +# Match every top-level operation in a GraphQL document. +# A document can contain multiple operations: `query A {…} mutation B {…}`. +# The named-operation check (CWE-352, H-1) must inspect ALL operations, not +# just the first non-comment token. +_OPERATION_PATTERN = re.compile( + r"\b(query|mutation|subscription)\b\s*([A-Za-z_][A-Za-z0-9_]*)?", + re.IGNORECASE, +) + + +def _document_operations(query: str) -> list[tuple[str, str | None]]: + """Return (operation_type, operation_name|None) for every operation in *query*. + + Strips line comments first. Shorthand documents (a single selection set + starting with ``{``) implicitly map to a single ``query`` operation. + """ + # Strip GraphQL line comments — they always start with '#' and end at \n. + cleaned = re.sub(r"#[^\n]*", "", query) + ops: list[tuple[str, str | None]] = [] + for m in _OPERATION_PATTERN.finditer(cleaned): + ops.append((m.group(1).lower(), m.group(2) or None)) + if not ops and cleaned.lstrip().startswith("{"): + ops.append(("query", None)) + return ops + + +def _has_non_query_for_get(query: str, operation_name: str | None) -> bool: + """Return True if the document's selected operation is a mutation/subscription. + + CWE-352 fix: previously we only checked the first non-comment token, so + ``query A {…} mutation B {…}`` with ``operationName=B`` snuck a mutation + through GET. Now we look at every top-level operation and pick the one + matching ``operation_name``; if no name is given we look at all of them + (a document with a single operation is the only legal case in that mode). + """ + ops = _document_operations(query) + if not ops: + # Unparseable — fail closed: disallow over GET, executor will return a + # proper error message. + return True + if operation_name is None: + # No operationName: spec requires exactly one operation in the + # document. Reject GET if any of them is not a query. + return any(op_type != "query" for op_type, _ in ops) + for op_type, op_name in ops: + if op_name == operation_name: + return op_type != "query" + # Named operation not found — let the executor handle the error. + return False + + +def _document_depth(query: str) -> int: + """Conservative selection-set depth estimate. + + Counts the maximum brace nesting in the document. Fast path before + handing the query to the (potentially expensive) executor. Comments and + strings are stripped to avoid spurious brace counts. + """ + cleaned = re.sub(r"#[^\n]*", "", query) + # Strip GraphQL string values so braces inside them are not counted. + cleaned = re.sub(r'"(?:[^"\\]|\\.)*"', '""', cleaned) + depth = 0 + max_depth = 0 + for ch in cleaned: + if ch == "{": + depth += 1 + if depth > max_depth: + max_depth = depth + elif ch == "}": + depth -= 1 + if depth < 0: + depth = 0 + return max_depth def _error_response(message: str, status: int = 400) -> JSONResponse: @@ -57,11 +124,22 @@ def _error_response(message: str, status: int = 400) -> JSONResponse: def make_graphql_handler( executor: GraphQLExecutor, *, - graphiql: bool = True, + graphiql: bool = False, allow_get: bool = True, context_factory: Callable[[Request], dict[str, Any] | Awaitable[dict[str, Any]]] | None = None, + max_depth: int | None = 15, + timeout_s: float | None = 30.0, ) -> Callable[[Request], Awaitable[Response | JSONResponse | HTMLResponse]]: - """Return an async handler for the GraphQL endpoint.""" + """Return an async handler for the GraphQL endpoint. + + Defaults are hardened for production: + + * ``graphiql=False`` — opt-in for the in-browser explorer (CWE-200). + * ``max_depth=15`` — reject selection sets nested more than 15 levels deep + (CWE-770). Set ``None`` to disable. + * ``timeout_s=30`` — wrap executor in ``asyncio.wait_for`` so a single + malicious query cannot pin a worker indefinitely (CWE-770). + """ async def handler(request: Request) -> Response | JSONResponse | HTMLResponse: method = request.method.upper() @@ -79,8 +157,9 @@ async def handler(request: Request) -> Response | JSONResponse | HTMLResponse: query = request.query_params.get("query") if not query: return _error_response("Missing 'query' parameter") - if _is_mutation(query): - return _error_response("Mutations are not allowed over GET") + operation_name = request.query_params.get("operationName") + if _has_non_query_for_get(query, operation_name): + return _error_response("Mutations and subscriptions are not allowed over GET") variables_raw = request.query_params.get("variables") variables: dict[str, Any] | None = None if variables_raw: @@ -88,7 +167,6 @@ async def handler(request: Request) -> Response | JSONResponse | HTMLResponse: variables = json.loads(variables_raw) except (ValueError, TypeError): return _error_response("Invalid 'variables' JSON") - operation_name = request.query_params.get("operationName") elif method == "POST": try: @@ -106,6 +184,10 @@ async def handler(request: Request) -> Response | JSONResponse | HTMLResponse: else: return _error_response(f"Method {method} not allowed", 405) + # Depth guard — short-circuit before invoking the executor (CWE-770). + if max_depth is not None and _document_depth(query) > max_depth: + return _error_response(f"Query exceeds maximum nesting depth ({max_depth})", status=400) + # Build context app = request.scope.get("app") context: dict[str, Any] = {"request": request, "app": app} @@ -116,12 +198,19 @@ async def handler(request: Request) -> Response | JSONResponse | HTMLResponse: if extra: context.update(extra) - result = await executor( - query=query, - variables=variables if isinstance(variables, dict) else None, - operation_name=operation_name if isinstance(operation_name, str) else None, - context=context, - ) + try: + coro = executor( + query=query, + variables=variables if isinstance(variables, dict) else None, + operation_name=operation_name if isinstance(operation_name, str) else None, + context=context, + ) + if timeout_s is not None: + result = await asyncio.wait_for(coro, timeout=timeout_s) + else: + result = await coro + except TimeoutError: + return _error_response(f"Query exceeded execution timeout ({timeout_s}s)", status=504) # HTTP 400 when executor returned no data at all (pure request error) status = 400 if "data" not in result else 200 diff --git a/src/hawkapi/grpc/_mount.py b/src/hawkapi/grpc/_mount.py index 8b77dd9..f48d317 100644 --- a/src/hawkapi/grpc/_mount.py +++ b/src/hawkapi/grpc/_mount.py @@ -31,6 +31,7 @@ def __init__( reflection_service_names: Sequence[str] | None, options: Sequence[tuple[str, Any]], max_workers: int | None, + maximum_concurrent_rpcs: int | None = 1000, ) -> None: self.port = port self._host = host @@ -40,6 +41,7 @@ def __init__( self._reflection_service_names = reflection_service_names self._options = options self._max_workers = max_workers + self._maximum_concurrent_rpcs = maximum_concurrent_rpcs self._server: Any = None self._started = False # Pending (servicer, add_to_server) registrations — filled before _start @@ -70,7 +72,7 @@ async def _start(self) -> None: server: grpc.aio.Server = grpc.aio.server( interceptors=self._interceptors, options=list(self._options), - maximum_concurrent_rpcs=None, + maximum_concurrent_rpcs=self._maximum_concurrent_rpcs, ) self._server = server diff --git a/tests/unit/test_flags.py b/tests/unit/test_flags.py index ecb3b37..6ff732a 100644 --- a/tests/unit/test_flags.py +++ b/tests/unit/test_flags.py @@ -459,8 +459,17 @@ async def test_get_flags_uses_app_provider() -> None: assert await flags.bool("my-flag") is True -async def test_get_flags_context_from_headers() -> None: - """EvalContext is built from x-user-id / x-tenant-id headers.""" +async def test_get_flags_does_not_trust_identity_headers() -> None: + """``get_flags`` MUST NOT lift identity from client-supplied headers. + + Reading ``X-User-Id`` / ``X-Tenant-Id`` as the identity for flag + targeting is CWE-290 — any attacker can claim any user/tenant by + setting the header. ``user_id`` and ``tenant_id`` must always be ``None`` + on the default context; operators derive identity from an authenticated + dependency and build their own ``EvalContext`` if needed. The headers + are still exposed via ``ctx.headers`` for non-identity targeting + (region, A/B variant, locale). + """ from hawkapi import HawkAPI from hawkapi.requests.request import Request @@ -477,5 +486,9 @@ async def test_get_flags_context_from_headers() -> None: flags = await get_flags(req) ctx = flags._context # type: ignore[attr-defined] assert ctx is not None - assert ctx.user_id == "alice" - assert ctx.tenant_id == "acme" + # SECURITY: must NOT trust client-supplied identity headers. + assert ctx.user_id is None + assert ctx.tenant_id is None + # But the headers themselves are still on the context for non-identity rules. + assert ctx.headers.get("x-user-id") == "alice" + assert ctx.headers.get("x-tenant-id") == "acme"