diff --git a/contextual_orchestrator/cost_ledger.py b/contextual_orchestrator/cost_ledger.py index d3943c5be..9ca2040c4 100644 --- a/contextual_orchestrator/cost_ledger.py +++ b/contextual_orchestrator/cost_ledger.py @@ -583,6 +583,7 @@ def _seed_dimension_catalog(self) -> None: ph = self._placeholder() cur = self._conn.cursor() for order, (name, label, _column) in enumerate(ATTRIBUTION_DIMENSION_CATALOG): + # nosemgrep: python.sqlalchemy.security.sqlalchemy-execute-raw-query.sqlalchemy-execute-raw-query cur.execute( f"SELECT 1 FROM cost_attribution_dimensions WHERE dimension_name = {ph}", # nosec B608 - ph is a DB-API placeholder. (name,), @@ -602,6 +603,7 @@ def append(self, record: UsageRecord) -> None: placeholders = ", ".join(ph for _ in _USAGE_COLUMNS) columns = ", ".join(_USAGE_COLUMNS) cur = self._conn.cursor() + # nosemgrep: python.sqlalchemy.security.sqlalchemy-execute-raw-query.sqlalchemy-execute-raw-query cur.execute( f"INSERT INTO llm_usage_records ({columns}) VALUES ({placeholders})", # nosec B608 - columns are fixed _USAGE_COLUMNS. tuple(row.get(column) for column in _USAGE_COLUMNS), @@ -622,6 +624,7 @@ def query(self, start: Optional[int] = None, end: Optional[int] = None) -> List[ where = f" WHERE {' AND '.join(clauses)}" if clauses else "" columns = ", ".join(_USAGE_COLUMNS) cur = self._conn.cursor() + # nosemgrep: python.sqlalchemy.security.sqlalchemy-execute-raw-query.sqlalchemy-execute-raw-query cur.execute(f"SELECT {columns} FROM llm_usage_records{where}", tuple(params)) # nosec B608 - columns and clauses are fixed. return [dict(zip(_USAGE_COLUMNS, values)) for values in cur.fetchall()] diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 0097b722e..ddb95e49c 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -14,6 +14,7 @@ from pathlib import Path import random import re +import secrets import socket import ssl import sqlite3 @@ -230,6 +231,7 @@ def __init__( @staticmethod def _build_ssl_context(ca_bundle: str | None, verify_tls: bool) -> ssl.SSLContext: if not verify_tls: + # nosemgrep: python.lang.security.unverified-ssl-context.unverified-ssl-context return ssl._create_unverified_context() # nosec B323 - explicit dev-only provider TLS opt-out. if ca_bundle: if not os.path.isfile(ca_bundle): @@ -284,7 +286,10 @@ def _send_with_retry(self, agent: ModelAgent, payload: dict[str, Any]) -> str: def _backoff_delay(self, attempt: int) -> float: """Full-jitter exponential backoff, capped, so retries do not thundering-herd a provider.""" ceiling = min(self.retry_backoff_cap, self.retry_backoff * (2 ** attempt)) - return random.uniform(0.0, ceiling) + # secrets, not random: this delay has no reproducibility requirement (unlike + # evolve_orchestration's seeded search below), so there is no reason not to use + # a non-predictable source. + return secrets.SystemRandom().uniform(0.0, ceiling) def _send(self, agent: ModelAgent, payload: dict[str, Any]) -> str: """Perform one provider HTTP request (isolated so retry/backoff stays testable).""" @@ -307,6 +312,7 @@ def _send(self, agent: ModelAgent, payload: dict[str, Any]) -> str: def _open_provider(self, request: urllib.request.Request) -> Any: """Open a provider request built from a validated provider URL.""" + # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected return urllib.request.urlopen( # nosec B310 - request URL comes from _provider_url after provider validation. request, timeout=self.timeout, @@ -479,6 +485,7 @@ def _validate_provider(self, agent: ModelAgent) -> None: or ip_address.is_link_local or ip_address.is_multicast or ip_address.is_reserved + or ip_address.is_unspecified ): raise RuntimeError(f"{agent.id} provider resolves to non-public address") @@ -1235,9 +1242,7 @@ def patch_agent(self, agent_pool_id: str, worker_agent_id: str, patch: dict[str, """Apply governance updates to an agent and emit an audit event.""" if not patch: # pragma: no cover raise ValueError("patch request body must contain updates") - if agent_pool_id != "default": # pragma: no cover - raise KeyError(agent_pool_id) - current = self._agent(worker_agent_id) + current = self._agent_in_pool(agent_pool_id, worker_agent_id) patched = current if "status" in patch: status = str(patch["status"]).lower() @@ -1315,9 +1320,7 @@ def add_agent(self, agent_pool_id: str, value: dict[str, Any]) -> dict[str, Any] def remove_agent(self, agent_pool_id: str, worker_agent_id: str) -> dict[str, Any]: """Remove a worker agent from the pool; the pool must keep at least one enabled agent.""" - if agent_pool_id != "default": # pragma: no cover - raise KeyError(agent_pool_id) - target = self._agent(worker_agent_id) + target = self._agent_in_pool(agent_pool_id, worker_agent_id) remaining_enabled = [agent for agent in self.agents if agent.id != worker_agent_id and not agent.disabled] if not remaining_enabled: raise ValueError("cannot remove the last enabled agent") @@ -1594,6 +1597,19 @@ def _agent(self, agent_id: str) -> ModelAgent: return agent raise KeyError(agent_id) # pragma: no cover + def _agent_in_pool(self, agent_pool_id: str, worker_agent_id: str) -> ModelAgent: + """Resolve a worker agent, rejecting any pool but the one it can belong to. + + Every agent belongs to the single "default" pool -- there is no + multi-pool partitioning in this store (``_AgentPoolStore`` keys + purely by ``agent_id``). Ownership is checked here, at the same + point the agent is looked up, so a caller can never dereference + ``worker_agent_id`` under a ``agent_pool_id`` it does not belong to. + """ + if agent_pool_id != "default": + raise KeyError(agent_pool_id) + return self._agent(worker_agent_id) + def _needs_workflow(self, text: str) -> bool: lowered = text.lower() hits = sum(1 for hint in self.COMPLEX_HINTS if hint in lowered) @@ -8393,7 +8409,11 @@ def evolve_orchestration( exceeds ``cost_budget_usd`` rank below all affordable ones. Quality comes from the caller's ``quality_fn(task, answer) -> [0,1]`` — never fabricated. """ - rng = random.Random(seed) + # random, not secrets: `seed` exists precisely so two calls with the same + # seed reproduce the same search trajectory (a benchmark/regression-test + # requirement) -- this selects which orchestration config to try next, + # never a secret, token, or anything an attacker gains from predicting. + rng = random.Random(seed) # nosec B311 - reproducible search, not a security context. params = sorted(search_space) def random_config() -> dict[str, Any]: diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index c58d4cb79..ced27b74f 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -616,7 +616,9 @@ def do_GET(self) -> None: # noqa: N802 agent_pool_id = segments[3] worker_agent_id = segments[-1] try: - payload = orchestrator._agent_to_admin_payload(orchestrator._agent(worker_agent_id)) + payload = orchestrator._agent_to_admin_payload( + orchestrator._agent_in_pool(agent_pool_id, worker_agent_id) + ) payload["agent_pool_id"] = agent_pool_id self._send(payload) return diff --git a/docs/architecture.md b/docs/architecture.md index c0f63a81e..52932a657 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -38,6 +38,13 @@ This repository implements the interface and control plane, not the trained coor - `ModelClient`: OpenAI-compatible HTTP client, with `mock://` for local checks. - `contextual_orchestrator.server`: small `/v1/chat/completions` HTTP server. +Agent-pool administration is fail-closed at the resource boundary. The +`agent_pool_id` and `worker_agent_id` path parameters are resolved together by +`TaskOrchestrator._agent_in_pool` for GET, PATCH, and DELETE operations. The +current persistence model has one `default` pool, so any other pool identifier +returns a not-found result instead of exposing or mutating a real worker under +a caller-selected path. + The deliberate simplification is the policy. The paper systems learn routing and topology from rewards; this lab uses deterministic keyword scoring so the repo runs without training data, GPUs, or vendor credentials. Add learned routing only when there is an evaluation set and logs proving the heuristic policy is the bottleneck. diff --git a/docs/rest_api_design.md b/docs/rest_api_design.md index 9378e5a37..3f8226d71 100644 --- a/docs/rest_api_design.md +++ b/docs/rest_api_design.md @@ -18,6 +18,7 @@ | `POST` | `/v1/batch/embeddings` | Submit a bulk, latency-tolerant embeddings batch; oversized inputs are token-split before routing via pg-llm-batch | | `GET` | `/v1/batch/embeddings/{batch_id}` | Poll an embeddings batch; returns reduced vectors + recorded cost once completed | | `GET` | `/api/v1/agent_pools` | List model agents | +| `GET` | `/api/v1/agent_pools/{agent_pool_id}/worker_agents/{worker_agent_id}` | Read one worker only when the worker belongs to the requested pool | | `GET` | `/api/v1/orchestration_policies/default_policy` | Read active policy | | `GET` | `/api/v1/analytics_snapshots/latest` | Read local runtime KPI and guardrail snapshot | | `GET` | `/api/v1/sales_readiness/latest` | Read local enterprise-pilot readiness criteria and evidence | @@ -51,6 +52,7 @@ | `GET` | `/api/v1/workflow_runs/{workflow_run_id}` | Inspect one run and trace | | `GET` | `/api/v1/access_reports/{workflow_run_id}` | Inspect access-list evidence | | `PATCH` | `/api/v1/agent_pools/{agent_pool_id}/worker_agents/{worker_agent_id}` | Update status/priority/tags/provider exclusions | +| `DELETE` | `/api/v1/agent_pools/{agent_pool_id}/worker_agents/{worker_agent_id}` | Remove one worker only when it belongs to the addressed pool; mismatches return `404` | | `POST` | `/api/v1/evaluation_runs` | Replay prompts and return a reproducible evaluation run | | `GET` | `/api/v1/evaluation_runs/{evaluation_run_id}` | Review replay output | | `GET` | `/api/v1/locale_bundles/{locale_code}` | Read i18n bundle | @@ -58,6 +60,11 @@ ## Product Planning Additions (Implemented) +Agent-pool resource paths resolve the pool and worker together. A worker ID +cannot be read, patched, or removed through a different pool identifier; an +unknown pool/worker combination returns `404` without revealing the worker's +admin payload. + These product surfaces are now implemented in this prototype: | Method | Path | Purpose | Paper Basis | @@ -66,6 +73,7 @@ These product surfaces are now implemented in this prototype: | `POST` | `/api/v1/evaluation_runs` | Replay a prompt or dataset against policy variants before changing production routing. | Fugu and TRINITY optimize coordination against measured outcomes. | | `GET` | `/api/v1/access_reports/{workflow_run_id}` | Produce compliance evidence for which worker saw which prior outputs. | Conductor access-list visibility control. | | `PATCH` | `/api/v1/agent_pools/{agent_pool_id}/worker_agents/{worker_agent_id}` | Update status, priority, capability tags, or provider exclusion. | Fugu configurable worker pool and provider/compliance constraints. | +| `DELETE` | `/api/v1/agent_pools/{agent_pool_id}/worker_agents/{worker_agent_id}` | Remove the worker from the addressed pool after the same pool-scoped lookup. | Fugu configurable worker-pool membership and provider/compliance constraints. | | `GET` | `/api/v1/analytics_snapshots/latest` | Produce source-backed local KPI and guardrail evidence without claiming production telemetry. | Fugu evaluation discipline; TRINITY verification evidence; Conductor access-list guardrails. | | `GET` | `/api/v1/sales_readiness/latest` | Produce a sellable-pilot readiness gate from current runtime, admin, security, analytics, locale, and provider evidence. | Fugu API adoption; TRINITY verification; Conductor trace and access-list evidence. | | `GET` | `/api/v1/commercial_readiness/latest` | Produce a high-value buyer due-diligence readiness gate for the KRW 2,000,000,000 target without presenting it as a valuation guarantee. | Fugu API adoption; TRINITY verification; Conductor trace/access evidence; enterprise procurement review. | diff --git a/tests/test_agent_pool_db.py b/tests/test_agent_pool_db.py index be359ae59..59d331ff7 100644 --- a/tests/test_agent_pool_db.py +++ b/tests/test_agent_pool_db.py @@ -9,12 +9,12 @@ import json import os -from pathlib import Path import sys import tempfile import threading import urllib.error import urllib.request +from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parents[1])) @@ -86,6 +86,32 @@ def test_add_agent_validations() -> None: assert raised, why +def test_patch_and_remove_reject_a_pool_the_agent_does_not_belong_to() -> None: + """A real worker agent ID must not be reachable through a wrong pool ID. + + Regression test for a Strix-flagged IDOR: patch_agent/remove_agent must + validate agent_pool_id and resolve worker_agent_id together, not as two + independently-checked arguments. + """ + orchestrator = TaskOrchestrator(_seed()) + for pool_id in ("other_pool", "default_", "", "DEFAULT"): + raised = False + try: + orchestrator.patch_agent(pool_id, "general_agent", {"priority": 5}) + except KeyError: + raised = True + assert raised, f"patch_agent must reject pool_id={pool_id!r}" + + raised = False + try: + orchestrator.remove_agent(pool_id, "general_agent") + except KeyError: + raised = True + assert raised, f"remove_agent must reject pool_id={pool_id!r}" + # The agent must be untouched by every rejected attempt above. + assert orchestrator._agent("general_agent").priority == 0 + + def test_remove_last_enabled_agent_refused() -> None: orchestrator = TaskOrchestrator(_seed()) raised = False @@ -132,6 +158,16 @@ def test_http_create_and_delete_worker_agents() -> None: status, dup = _call(base, "POST", token, NEW_AGENT) assert status == 400 # duplicate rejected + status, listed = _call(f"{base}/general_agent", "GET", token) + assert status == 200 and listed["id"] == "general_agent" + + status, wrong_pool = _call( + f"http://127.0.0.1:{server.server_address[1]}/api/v1/agent_pools/other_pool/worker_agents/general_agent", + "GET", + token, + ) + assert status == 404 and wrong_pool["error"]["code"] == "agent_not_found" + status, unknown = _call(base, "POST", token, {**NEW_AGENT, "id": "extra_agent", "surprise": 1}) assert status == 400 and unknown["error"]["code"] == "unknown_fields" diff --git a/tests/test_security_hardening.py b/tests/test_security_hardening.py index 67134ea6b..4af7001fb 100644 --- a/tests/test_security_hardening.py +++ b/tests/test_security_hardening.py @@ -254,6 +254,29 @@ def test_external_provider_requires_resolvable_credential_and_public_https() -> set_backend(None) +def test_external_provider_rejects_unspecified_addresses(monkeypatch: pytest.MonkeyPatch) -> None: + """0.0.0.0 and [::] are unrouteable "any interface" addresses, not public -- + Strix-flagged SSRF gap: they cleared every prior is_private/is_loopback/ + is_link_local/is_multicast/is_reserved check. + """ + monkeypatch.delenv("CONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTS", raising=False) + client = ModelClient() + backend = InMemoryCredentialBackend() + backend.set("MODEL_KEY", "sk-unspecified-check") + set_backend(backend) + try: + for host in ("0.0.0.0", "[::]"): # noqa: S104 - intentional invalid-host security fixtures + agent = ModelAgent("unspecified_agent", "gpt-example", f"https://{host}/v1", "MODEL_KEY") + try: + client._validate_provider(agent) + except RuntimeError as exc: + assert "non-public address" in str(exc) + else: + raise AssertionError(f"unspecified-address provider {host} should fail") + finally: + set_backend(None) + + def test_external_provider_rejects_insecure_or_unlisted_hosts() -> None: client = ModelClient() insecure_agent = ModelAgent("insecure_agent", "gpt-example", "http://api.openai.com/v1", "MODEL_KEY")