diff --git a/.github/workflows/publish-pypi.yml b/.github/workflows/publish-pypi.yml index b545205..f0e04e0 100644 --- a/.github/workflows/publish-pypi.yml +++ b/.github/workflows/publish-pypi.yml @@ -35,7 +35,7 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} - name: Set up Python - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: python-version: "3.13" diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 93b9d75..7f59963 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -18,7 +18,7 @@ jobs: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Set up Python - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: python-version: "3.13" diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 58eeae0..65ecce3 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -1,5 +1,6 @@ # This workflow will install Python dependencies, run tests and lint with a variety of Python versions # For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python +# Documentation-only diffs (.md, .txt, docs/) skip unit tests but still run pre-commit. name: Python package @@ -11,11 +12,73 @@ on: workflow_call: jobs: + change_scope: + runs-on: ubuntu-latest + outputs: + skip_tests: ${{ steps.detect.outputs.skip_tests }} + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + + - name: Detect documentation-only changes + id: detect + run: | + is_documentation_only_path() { + case "$1" in + *.md|*.MD|*.txt|*.TXT) return 0 ;; + docs/*|Documentation/*|documentation/*) return 0 ;; + changelog/*) return 0 ;; + *) return 1 ;; + esac + } + + # Reusable workflows and unknown events always run the full suite. + if [ "${{ github.event_name }}" = "workflow_call" ]; then + echo "skip_tests=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + if [ "${{ github.event_name }}" = "pull_request" ]; then + git fetch --depth=1 origin "${{ github.event.pull_request.base.sha }}" + mapfile -t changed < <(git diff --name-only "${{ github.event.pull_request.base.sha }}" HEAD) + elif [ "${{ github.event_name }}" = "push" ]; then + before="${{ github.event.before }}" + if [ -z "$before" ] || [ "$before" = "0000000000000000000000000000000000000000" ]; then + echo "skip_tests=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + mapfile -t changed < <(git diff --name-only "$before" HEAD) + else + echo "skip_tests=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + if [ "${#changed[@]}" -eq 0 ]; then + echo "skip_tests=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + doc_only=true + for path in "${changed[@]}"; do + if ! is_documentation_only_path "$path"; then + doc_only=false + break + fi + done + + if [ "$doc_only" = true ]; then + echo "skip_tests=true" >> "$GITHUB_OUTPUT" + echo "Documentation-only changes detected — unit tests will be skipped." + else + echo "skip_tests=false" >> "$GITHUB_OUTPUT" + fi + pre-commit: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: python-version: "3.13" - uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 @@ -35,6 +98,7 @@ jobs: .github/workflows build: + needs: change_scope runs-on: ubuntu-latest strategy: @@ -45,7 +109,7 @@ jobs: steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: python-version: ${{ matrix.python-version }} - name: Install uv @@ -54,6 +118,13 @@ jobs: # Avoid parallel matrix jobs racing on the same Actions cache reservation cache-suffix: py-${{ matrix.python-version }} - name: Install dependencies + if: needs.change_scope.outputs.skip_tests != 'true' run: uv sync --frozen --extra dev --extra deploy + - name: Documentation-only changes — unit tests skipped + if: needs.change_scope.outputs.skip_tests == 'true' + run: | + echo "Only .md, .txt, or docs/ paths changed." + echo "Skipping pytest matrix." - name: Test with pytest + if: needs.change_scope.outputs.skip_tests != 'true' run: uv run pytest --no-cov diff --git a/.github/workflows/trufflehog-full-scan.yml b/.github/workflows/trufflehog-full-scan.yml index 1aea952..cb1bdad 100644 --- a/.github/workflows/trufflehog-full-scan.yml +++ b/.github/workflows/trufflehog-full-scan.yml @@ -24,6 +24,6 @@ jobs: - name: Full Repository Scan if: inputs.scan_type == 'full-repo' - uses: trufflesecurity/trufflehog@30d5bb91af1a771378349dbbb0c82129392acf70 # v3.95.6 + uses: trufflesecurity/trufflehog@f446421baf832d6356c42c1743d99abff52ff334 # v3.95.7 with: extra_args: --results=verified,unknown diff --git a/.github/workflows/trufflehog-security.yml b/.github/workflows/trufflehog-security.yml index 738e62a..c98151d 100644 --- a/.github/workflows/trufflehog-security.yml +++ b/.github/workflows/trufflehog-security.yml @@ -21,6 +21,6 @@ jobs: fetch-depth: 0 # Required for TruffleHog to scan git history - name: Secret Scanning - uses: trufflesecurity/trufflehog@30d5bb91af1a771378349dbbb0c82129392acf70 # v3.95.6 + uses: trufflesecurity/trufflehog@f446421baf832d6356c42c1743d99abff52ff334 # v3.95.7 with: extra_args: --results=verified,unknown diff --git a/docs/2026_07_SECURITY_REVIEW.md b/docs/2026_07_SECURITY_REVIEW.md new file mode 100644 index 0000000..251b832 --- /dev/null +++ b/docs/2026_07_SECURITY_REVIEW.md @@ -0,0 +1,86 @@ +# Supervaizer — Security & Performance Review (Summary) + +> **Date:** 2026-07-07 +> **Scope:** Full (non-diff) review of the entire `supervaizer` SDK source tree (~20k LOC). +> **Type:** Security review + performance/scalability review. +> +> **⚠️ Disclosure note:** Per [`SECURITY.md`](../SECURITY.md), detailed vulnerability +> findings — attack scenarios, exact code locations, and remediation specifics — are **not** +> published here. This page is a non-actionable high-level summary only. The complete +> findings are handled through the project's private vulnerability channel (GitHub Security +> Advisories) so that unpatched issues are not operationalized in a public artifact. + +--- + +## Methodology + +A multi-agent audit: per-component security finders reviewed the full source; every +high-impact candidate was independently re-checked by an adversarial verifier (instructed +to refute it and confirm real reachability), which recalibrated several severities; a +completeness critic then swept for missed classes; and a separate pass covered async +performance and scalability. This was a static review — no exploit was executed. + +## Overall posture + +The **core authorization primitives are sound.** Verified during the review: + +- Signed workspace authorization uses EdDSA with the algorithm pinned (no `alg:none` or + algorithm-confusion), full claim binding (issuer/audience/expiry/subject/workspace), and + keys sourced only from configured trust material. +- Privileged protocol actions fail **closed** when authorization is not configured. +- No SQL/NoSQL/command injection, no server-side template injection, and no unsafe + deserialization (`pickle`/`yaml.load`/`eval`) were found. +- No CORS misconfiguration — none is configured, so the safe same-origin default applies. +- Jinja autoescaping is enabled. + +The material risk is concentrated in **credential handling and the trust model of the +administrative surface**, not in the request-validation core. Themes (no specifics here): + +- Development/quick-start defaults that are unsafe if exposed on an untrusted network. +- An administrative surface whose trust boundary can be weakened under certain + reverse-proxy configurations. +- Credential material that is more exposed at rest / in transit than it should be. +- A symmetric-encryption construction that should be migrated to an authenticated (AEAD) + scheme. +- Deployment tooling that handles secrets less defensively than the runtime does. + +## Supply-chain posture (already in place) + +The repository already implements a strong supply-chain baseline, documented in +[`SECURITY.md`](../SECURITY.md): a committed `uv.lock` with `uv sync --frozen` enforced in +CI, Dependabot security updates, OSV-Scanner on every PR, secret scanning with push +protection, SHA-pinned third-party Actions, and OIDC Trusted Publishing. Pinned dependency +floors were reviewed and are modern (no known-vulnerable pins). **No supply-chain action is +recommended beyond what already exists.** + +## Findings summary (counts only) + +| Domain | Critical | High | Medium | Low | +|--------|----------|------|--------|-----| +| Security (post-verification) | 0 | 6 | 16 | 18 | +| Performance / scalability | — | 9 | 10 | 2 | + +No finding survived verification at **Critical**. + +### Performance themes + +The performance findings cluster into two areas: **unbounded in-memory growth** (long-lived +registries that do not evict completed work) and **event-loop blocking** (synchronous I/O +and whole-file persistence operations executed on the async loop). Neither is a correctness +bug today; each degrades under sustained load or data growth. The highest-leverage +mitigations are caching/offloading the persistence layer, evicting terminal entities, and +using the already-present async code paths for request-time verification. + +## Remediation approach + +Detailed, prioritized remediation guidance (P0–P3) accompanies the private report. At a +high level: address credential-handling and admin-trust items first, migrate the symmetric +encryption to an AEAD construction, then harden response headers, request limits, and +object-level authorization, and finally apply the performance mitigations. The core +architecture does not require redesign. + +--- + +*For the complete findings and remediation detail, maintainers should refer to the private +security advisory. Report any deviation from the documented security posture via the private +channel in [`SECURITY.md`](../SECURITY.md).* diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 35067a0..d1781b4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -12,6 +12,27 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- **Security & performance review summary** — Added `docs/2026_07_SECURITY_REVIEW.md`, a non-actionable high-level summary of a full-source security and performance/scalability review (posture, verified-sound controls, severity counts, and remediation themes). Per `SECURITY.md`, detailed findings (locations, attack scenarios, remediation specifics) are handled through the private vulnerability channel and are intentionally omitted from the public repository. + +### Fixed + +- **Hardened API-key checks** — API keys are compared in constant time. +- **Safer local test mode** — `supervaizer start --local` binds to loopback (`127.0.0.1`) by default instead of all interfaces; pass an explicit `--host` to override. +- **Baseline security response headers** — Responses now set `X-Content-Type-Options`, `X-Frame-Options`, `Referrer-Policy`, and `Strict-Transport-Security` (streaming/SSE-safe). +- **Reduced sensitive data in logs** — Agent parameter values are no longer written to logs during parameter validation. +- **Scheduled-step execution hardening** — The scheduler only runs methods declared by the agent that owns the step's job. + +### Tests + +| Status | Count | +| ---------- | ----- | +| ✅ Passed | 683 | +| 🤔 Skipped | 0 | +| 🔴 Failed | 0 | +| ⏱️ in | 83s | + ## [1.3.1] - 2026-07-02 ### Changed @@ -22,6 +43,8 @@ All notable changes to this project will be documented in this file. - **A2A event scope test** — `tests/test_a2a.py` now walks FastAPI included-router wrappers when locating `/a2a/events`, preserving the read-scope assertion under FastAPI `0.139.0`. +### Tests + | Status | Count | | ---------- | ----- | | ✅ Passed | 683 | diff --git a/src/supervaizer/__version__.py b/src/supervaizer/__version__.py index d93f13d..1d4a44b 100644 --- a/src/supervaizer/__version__.py +++ b/src/supervaizer/__version__.py @@ -4,6 +4,5 @@ # If a copy of the MPL was not distributed with this file, you can obtain one at # https://mozilla.org/MPL/2.0/. - VERSION = "1.3.1" __version__ = VERSION diff --git a/src/supervaizer/access/api_auth.py b/src/supervaizer/access/api_auth.py index 58c0309..7aef029 100644 --- a/src/supervaizer/access/api_auth.py +++ b/src/supervaizer/access/api_auth.py @@ -14,6 +14,7 @@ from __future__ import annotations +import hmac import os from collections.abc import Callable from typing import Annotated @@ -61,7 +62,12 @@ def require_api_key( # <-- ADDED live_server = getattr(getattr(request, "app", None), "state", None) live_server = getattr(live_server, "server", None) if live_server else None live_key = getattr(live_server, "api_key", None) if live_server else None - if live_key and x_api_key == live_key: + # Constant-time comparison to avoid a timing side channel on the key. + # Compare bytes so non-ASCII keys fail closed instead of raising + # TypeError (hmac.compare_digest rejects non-ASCII str inputs). + if live_key and hmac.compare_digest( + x_api_key.encode("utf-8"), live_key.encode("utf-8") + ): return {"scope": "write"} # live server key always has full access log_access_denied_api(x_api_key, path, "invalid key") raise HTTPException(status_code=401, detail="Invalid or missing API key") diff --git a/src/supervaizer/admin/workbench_routes.py b/src/supervaizer/admin/workbench_routes.py index 2d57be1..73626bb 100644 --- a/src/supervaizer/admin/workbench_routes.py +++ b/src/supervaizer/admin/workbench_routes.py @@ -30,6 +30,7 @@ from supervaizer.contracts import API_VERSION from supervaizer.job import Job, JobContext, JobResponse, Jobs from supervaizer.lifecycle import EntityStatus +from supervaizer.scheduled_steps import _execute_scheduled_method templates = Jinja2Templates(directory=str(Path(__file__).parent / "templates")) @@ -531,13 +532,25 @@ async def workbench_execute_step( request: Request, slug: str, job_id: str, case_id: str, step_index: int ) -> Response: """Execute a scheduled step immediately.""" - from supervaizer.server import _execute_scheduled_method + agent = get_agent_by_slug(request, slug) - get_agent_by_slug(request, slug) + # Verify the job and case are both owned by this agent before using + # the agent's method allow-list. + job = Jobs().get_job(job_id, agent_name=agent.name) + if job is None: + raise HTTPException( + status_code=404, + detail=f"Job '{job_id}' not found for agent '{slug}'", + ) case = Cases().get_case(case_id, job_id=job_id) if not case: raise HTTPException(status_code=404, detail=f"Case '{case_id}' not found") + if case.id not in job.case_ids: + raise HTTPException( + status_code=404, + detail=f"Case '{case_id}' not found for agent '{slug}'", + ) if step_index < 0 or step_index >= len(case.updates): raise HTTPException(status_code=404, detail="Step not found") @@ -557,6 +570,7 @@ async def workbench_execute_step( _execute_scheduled_method( update.scheduled_method, update.scheduled_params or {}, + allowed_methods=agent._declared_method_paths(), ) object.__setattr__(update, "scheduled_status", "completed") return JSONResponse({ diff --git a/src/supervaizer/cli.py b/src/supervaizer/cli.py index 8e285dc..3d03dc2 100644 --- a/src/supervaizer/cli.py +++ b/src/supervaizer/cli.py @@ -188,6 +188,12 @@ def start( if local: os.environ["SUPERVAIZER_LOCAL_MODE"] = "true" + # Security: local test mode uses a well-known default API key + # ("local-dev"). Never expose that on all network interfaces — bind + # loopback unless the user chose a specific non-wildcard host. + if host in ("0.0.0.0", "::", ""): + host = "127.0.0.1" + os.environ["SUPERVAIZER_HOST"] = host # In local mode, force public_url to localhost unless the user # explicitly passed --public-url on the CLI. if not user_provided_public_url: diff --git a/src/supervaizer/routes.py b/src/supervaizer/routes.py index f52810d..9a6897a 100644 --- a/src/supervaizer/routes.py +++ b/src/supervaizer/routes.py @@ -623,12 +623,19 @@ async def validate_agent_parameters( "encrypted_agent_parameters": f"Decryption failed: {e!s}" }, } - log.info(f"📤 Agent {agent.name}: Decryption failed → {result}") + # Do not log the result payload: it can echo parameter data. + log.info(f"📤 Agent {agent.name}: Decryption failed") return result - # Log the incoming request details + # Log the incoming request details. + # Never log decrypted parameter values (secrets); log only presence/count. + _param_count = ( + len(agent_parameters) if isinstance(agent_parameters, dict) else 0 + ) log.info( - f"🔍 Agent {agent.name}: Incoming request - encrypted_params: {bool(encrypted_agent_parameters)}, parsed_params: {agent_parameters}" + f"🔍 Agent {agent.name}: Incoming request - " + f"encrypted_params: {bool(encrypted_agent_parameters)}, " + f"param_count: {_param_count}" ) # Validate agent parameters @@ -643,7 +650,12 @@ async def validate_agent_parameters( "invalid_parameters": validation_result["invalid_parameters"], } - log.info(f"📤 Agent {agent.name}: Validation result → {result}") + # Log only the outcome, not the result payload (may contain values). + log.info( + f"📤 Agent {agent.name}: Validation " + f"{'passed' if validation_result['valid'] else 'failed'} " + f"({len(validation_result['errors'])} error(s))" + ) return result @router.post( diff --git a/src/supervaizer/scheduled_steps.py b/src/supervaizer/scheduled_steps.py index 24bcbcb..ecac7db 100644 --- a/src/supervaizer/scheduled_steps.py +++ b/src/supervaizer/scheduled_steps.py @@ -10,15 +10,49 @@ from typing import TYPE_CHECKING, Any from supervaizer.common import log +from supervaizer.job import Jobs if TYPE_CHECKING: + from supervaizer.case import Case + from supervaizer.job import Job from supervaizer.server import Server SCHEDULED_STEP_POLL_SECONDS = 60 -def _execute_scheduled_method(method_path: str, params: dict[str, Any]) -> Any: - """Execute a method by its full dotted path.""" +def _resolve_case_job(jobs: Jobs, case: "Case") -> "Job | None": + """Resolve the job that explicitly owns this case.""" + matches = [ + job + for agent_jobs in jobs.jobs_by_agent.values() + if (job := agent_jobs.get(case.job_id)) is not None and case.id in job.case_ids + ] + if len(matches) == 1: + return matches[0] + return None + + +def _execute_scheduled_method( + method_path: str, + params: dict[str, Any], + allowed_methods: set[str] | None = None, +) -> Any: + """Execute a method by its full dotted path. + + Args: + method_path: Dotted path of the callable to invoke. + params: Keyword arguments passed to the callable. + allowed_methods: If provided, ``method_path`` must be a member of this + allow-list (the agent's declared method paths); otherwise execution + is refused. This prevents a tampered or malformed scheduled step + from importing and calling an arbitrary dotted path (unsafe + reflection). When ``None`` (e.g. legacy/direct callers) no + allow-list is enforced. + """ + if allowed_methods is not None and method_path not in allowed_methods: + raise ValueError( + f"Scheduled method {method_path!r} is not an allowed agent method" + ) module_name, func_name = method_path.rsplit(".", 1) module = __import__(module_name, fromlist=[func_name]) method = getattr(module, func_name) @@ -34,15 +68,36 @@ async def _run_scheduled_step_loop(server: Server) -> None: try: cases = Cases() due_steps = cases.get_due_scheduled_steps() + # Per-agent declared-method allow-lists. A scheduled step may only + # invoke methods declared by the agent that owns its job, so a + # tampered/malformed step cannot reach another agent's methods. + agent_methods: dict[str, set[str]] = { + agent.name: agent._declared_method_paths() for agent in server.agents + } + jobs = Jobs() for _case, _step_index, update in due_steps: if not update.scheduled_method: continue + # Scope the allow-list to the owning job's agent. If the owning + # job cannot be resolved, fail the step rather than falling back + # to a broader allow-list — an orphaned step must not be able to + # invoke another agent's methods. + owning_job = _resolve_case_job(jobs, _case) + if owning_job is None: + object.__setattr__(update, "scheduled_status", "failed") + log.warning( + f"[Scheduled step] Skipping {update.name}: owning job " + f"{_case.job_id} for case {_case.id} could not be resolved" + ) + continue + allowed_methods = agent_methods.get(owning_job.agent_name, set()) try: object.__setattr__(update, "scheduled_status", "executing") log.info(f"[Scheduled step] Executing: {update.name}") _execute_scheduled_method( update.scheduled_method, update.scheduled_params or {}, + allowed_methods=allowed_methods, ) object.__setattr__(update, "scheduled_status", "completed") log.info(f"[Scheduled step] Completed: {update.name}") diff --git a/src/supervaizer/server.py b/src/supervaizer/server.py index db1393d..63054bd 100644 --- a/src/supervaizer/server.py +++ b/src/supervaizer/server.py @@ -11,6 +11,7 @@ # https://mozilla.org/MPL/2.0/. import asyncio +import hmac import os import secrets import time @@ -26,6 +27,7 @@ from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse # <-- MODIFIED: removed unused HTMLResponse from fastapi.security import APIKeyHeader +from starlette.datastructures import MutableHeaders # <-- REMOVED: Jinja2Templates (home page moved to routers/public.py) from pydantic import ConfigDict, Field, field_validator @@ -97,6 +99,44 @@ T = TypeVar("T") SCHEDULED_STEP_SHUTDOWN_TIMEOUT_SECONDS = 5.0 +# Baseline security response headers applied to every HTTP response. +_SECURITY_HEADERS: tuple[tuple[str, str], ...] = ( + ("x-content-type-options", "nosniff"), + # SAMEORIGIN (not DENY) so the admin UI can embed its own same-origin + # instructions iframe while still blocking cross-origin framing. + ("x-frame-options", "SAMEORIGIN"), + ("referrer-policy", "no-referrer"), + ("strict-transport-security", "max-age=63072000; includeSubDomains"), +) + + +class SecurityHeadersMiddleware: + """Pure-ASGI middleware that injects baseline security headers. + + It only rewrites the ``http.response.start`` headers and never touches the + response body, so it is safe for streaming/SSE responses (unlike a + ``BaseHTTPMiddleware``). WebSocket and lifespan scopes pass through + untouched. Existing headers are preserved (``setdefault`` semantics). + """ + + def __init__(self, app: Any) -> None: + self.app = app + + async def __call__(self, scope: Any, receive: Any, send: Any) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + async def send_wrapper(message: Any) -> None: + if message["type"] == "http.response.start": + headers = MutableHeaders(scope=message) + for name, value in _SECURITY_HEADERS: + if name not in headers: + headers[name] = value + await send(message) + + await self.app(scope, receive, send_wrapper) + def _agent_v2_method_handler(agent: Agent, action: str) -> ActionHandler: def handler(request: Any) -> Any: @@ -402,6 +442,12 @@ async def _lifespan(_app: FastAPI) -> AsyncIterator[None]: openapi_url=openapi_url, ) + # Baseline security response headers (clickjacking, MIME sniffing, + # referrer leakage, TLS downgrade). Implemented as a lightweight ASGI + # middleware that only touches response-start headers, so it does not + # buffer streaming/SSE responses. + app.add_middleware(SecurityHeadersMiddleware) + # Add exception handler for 422 validation errors @app.exception_handler(RequestValidationError) async def validation_exception_handler( @@ -534,7 +580,12 @@ async def verify_api_key( # API key authentication is disabled return True - if api_key != self.api_key: + # Constant-time comparison to avoid a timing side channel on the key. + # Compare bytes so non-ASCII keys fail closed instead of raising + # TypeError (hmac.compare_digest rejects non-ASCII str inputs). + if not hmac.compare_digest( + (api_key or "").encode("utf-8"), self.api_key.encode("utf-8") + ): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail="Invalid API key", diff --git a/tests/test_scheduled_steps.py b/tests/test_scheduled_steps.py new file mode 100644 index 0000000..348cd2f --- /dev/null +++ b/tests/test_scheduled_steps.py @@ -0,0 +1,48 @@ +# Copyright (c) 2024-2026 Alain Prasquier - Supervaize.com. All rights reserved. +# +# This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +# If a copy of the MPL was not distributed with this file, you can obtain one at +# https://mozilla.org/MPL/2.0/. + +from datetime import UTC, datetime + +from supervaizer import Account, Case, EntityStatus, Job, JobContext +from supervaizer.case import Cases +from supervaizer.job import Jobs +from supervaizer.scheduled_steps import _resolve_case_job + + +def test_resolve_case_job_uses_case_membership_for_duplicate_job_ids( + account_fixture: Account, +) -> None: + Cases().reset() + Jobs().reset() + job_id = "shared-job-id" + first_context = JobContext( + workspace_id="test-workspace", + job_id=job_id, + started_by="test-user", + started_at=datetime.now(UTC), + mission_id="first-mission", + mission_name="First Mission", + ) + second_context = first_context.model_copy( + update={"mission_id": "second-mission", "mission_name": "Second Mission"} + ) + first_job = Job.new(job_context=first_context, agent_name="first-agent") + second_job = Job.new(job_context=second_context, agent_name="second-agent") + case = Case( + id="second-agent-case", + job_id=job_id, + account=account_fixture, + status=EntityStatus.IN_PROGRESS, + name="Second Agent Case", + description="Owned by the second agent", + ) + second_job.add_case_id(case.id) + + assert first_job.id == second_job.id + assert _resolve_case_job(Jobs(), case) is second_job + + Cases().reset() + Jobs().reset() diff --git a/tests/test_workbench_routes.py b/tests/test_workbench_routes.py index b40822e..4d9d284 100644 --- a/tests/test_workbench_routes.py +++ b/tests/test_workbench_routes.py @@ -12,6 +12,7 @@ """Tests for workbench routes module.""" +from datetime import UTC, datetime from unittest.mock import Mock, patch import pytest @@ -27,11 +28,14 @@ Case, CaseNodeUpdate, EntityStatus, + Job, + JobContext, Parameter, ParametersSetup, ) from supervaizer.agent import AgentMethodField, FieldTypeEnum from supervaizer.case import Cases +from supervaizer.job import Jobs @pytest.fixture @@ -202,6 +206,63 @@ def test_answer_hitl_returns_404_for_missing_case( assert "missing-case" in response.json()["detail"] +class TestWorkbenchExecuteStep: + """Test scheduled-step execute-now ownership checks.""" + + def setup_method(self) -> None: + Cases().reset() + Jobs().reset() + + def teardown_method(self) -> None: + Cases().reset() + Jobs().reset() + + def test_execute_step_rejects_case_not_owned_by_slug_agent( + self, + test_client_with_agent: tuple[TestClient, str], + account_fixture: Account, + ) -> None: + client, agent_slug = test_client_with_agent + job_id = "shared-job-id" + owner_context = JobContext( + workspace_id="test-workspace", + job_id=job_id, + started_by="test-user", + started_at=datetime.now(UTC), + mission_id="owner-mission", + mission_name="Owner Mission", + ) + other_context = owner_context.model_copy( + update={"mission_id": "other-mission", "mission_name": "Other Mission"} + ) + owner_job = Job.new(job_context=owner_context, agent_name="Test Agent") + other_job = Job.new(job_context=other_context, agent_name="Other Agent") + assert owner_job.id == other_job.id + + case = Case( + id="other-agent-case", + job_id=job_id, + account=account_fixture, + status=EntityStatus.IN_PROGRESS, + name="Other Agent Case", + description="Owned by a different agent", + ) + case.updates = [ + CaseNodeUpdate( + scheduled_at=datetime.now(UTC), + scheduled_status="pending", + ) + ] + other_job.add_case_id(case.id) + + response = client.post( + f"/manage/agents/{agent_slug}/workbench/jobs/{job_id}/steps/{case.id}/0/execute" + ) + + assert response.status_code == 404 + assert case.updates[0].scheduled_status == "pending" + + class TestGetAgentBySlug: """Tests for the get_agent_by_slug helper."""