diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ddc8ab..ccd3297 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed (older, pre-1.4 notes) - Removed broken references to non-existent `security-threat-analyzer.yaml` template and `SECURITY_TEMPLATES.md` from REFERENCE.md. +## [1.6.1] - 2026-08-31 + +### Security +- **Declared behavioural contracts now fail closed** — if a resolved task declares a contract but the `behavioural-contracts` enforcement dependency is unavailable, execution stops before the affected model call with `CONTRACTS_UNAVAILABLE` instead of logging a warning and continuing without the promised constraint. Direct dependencies and locally delegated tasks are preflighted before their chain starts. (#103) +- **Sandbox domain rules cover ports and MCP endpoints** — `http.allow_domains` entries may pin `host:port` while bare hosts retain backwards-compatible any-port semantics. Statically configured MCP endpoints are preflighted across the selected task and direct dependencies before discovery or model execution. Malformed allowlist entries now fail validation. (#104) + +### Breaking +- Existing specs that combine MCP tools with `sandbox.http.allow_domains` must add each MCP endpoint host (or `host:port`) to the effective allowlist. Runtimes now enforce the declared network boundary for MCP instead of limiting it to native HTTP tools. +- Specs that declare `behavioural_contract` must install `open-agent-spec[contracts]`; execution no longer continues without enforcement. + ## [1.6.0] - 2026-07-28 ### Added diff --git a/README.md b/README.md index 1be742d..5d1a594 100644 --- a/README.md +++ b/README.md @@ -375,6 +375,8 @@ See [`examples/sandboxed-agent/`](examples/sandboxed-agent/). Declare what the model output must contain. The `behavioural-contracts` library enforces the contract after parsing, before the result is returned. +Declared contracts fail closed with `CONTRACTS_UNAVAILABLE` before the affected task invokes a model when enforcement is unavailable. + ```yaml behavioural_contract: version: "1.0" diff --git a/Website/app/page.tsx b/Website/app/page.tsx index a807381..28f2f56 100644 --- a/Website/app/page.tsx +++ b/Website/app/page.tsx @@ -252,7 +252,7 @@ tasks:
Behavioural contracts (optional)
-

Attach output contracts to tasks with the behavioural-contracts library. Validate required fields, confidence scores, and custom rules, after parsing, before returning. Degrades gracefully when not installed.

+

Attach output contracts to tasks with the behavioural-contracts library. Validate required fields, confidence scores, and custom rules after parsing and before returning. Declared contracts fail closed when enforcement is unavailable.

diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 337fb8d..c6bb8e4 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -455,7 +455,7 @@ Notes: Behavioural contracts let you declare constraints on a task's output — required fields, policy rules, behavioural flags — and have them enforced automatically at run time by the [`behavioural-contracts`](https://pypi.org/project/behavioural-contracts/) library. -Contracts are **entirely optional**. Specs without them run exactly as before. When the library is not installed, OA logs a hard warning and continues. +Contracts are **entirely optional**. Specs without them run exactly as before. When a resolved task declares a contract but the enforcement library is not installed, OA fails closed with `CONTRACTS_UNAVAILABLE` before the affected task makes a model call. Locally delegated tasks are recursively preflighted before their containing chain starts; remote delegated tasks are checked immediately after fetch. ### Install @@ -528,12 +528,12 @@ summarize → [parse] → [contract check] → return result A contract violation on a dependency stops the chain immediately and raises `CONTRACT_VIOLATION` before the dependent task ever runs. -### Skipped cases (with warning) +### Skipped and unavailable cases | Condition | Behaviour | |---|---| | `response_format: text` | Validation skipped — field checks are meaningless on raw strings | -| `behavioural-contracts` not installed | Hard warning logged; execution continues | +| `behavioural-contracts` not installed | Execution fails before the affected task's model call with `CONTRACTS_UNAVAILABLE` | | Output is not a dict (JSON parse failed) | Validation skipped with warning | ### Error on violation @@ -566,7 +566,8 @@ sandbox: # deny: [file.write] # denylist alternative (use one or the other) http: allow_domains: - - api.example.com # exact match or any subdomain + - api.example.com # exact match or any subdomain, any port + - localhost:3000 # optional port pinning file: allow_paths: - ./data/ # resolved to absolute paths at check time @@ -597,11 +598,13 @@ All sandbox violations raise `OARunError` immediately with one of three structur | Code | Trigger | |------|---------| | `SANDBOX_TOOL_VIOLATION` | Tool name not in `allow` list, or in `deny` list | -| `SANDBOX_DOMAIN_VIOLATION` | HTTP host not in `allow_domains` (for `http.get` / `http.post`) | +| `SANDBOX_DOMAIN_VIOLATION` | HTTP or MCP destination not in `allow_domains`; `host:port` rules require that port | | `SANDBOX_PATH_VIOLATION` | File path outside `allow_paths` (for `file.read` / `file.write`) | Path traversal (`../../`) is caught automatically — paths are resolved to absolute before comparison. +MCP endpoints are static spec configuration and are checked against the effective `http.allow_domains` policy before tool discovery or model execution. A bare hostname preserves the original any-port behaviour; use `host:port` when the agent must reach only one service on that host. + ### Input immutability Every task receives a **deep copy** of its input. Chain outputs merged into downstream inputs never mutate the caller's original dict. This is enforced at three levels: diff --git a/npm/package-lock.json b/npm/package-lock.json index ed5ca00..fc0bd21 100644 --- a/npm/package-lock.json +++ b/npm/package-lock.json @@ -1,12 +1,12 @@ { "name": "@prime-vector/open-agent-spec", - "version": "1.5.2", + "version": "1.6.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@prime-vector/open-agent-spec", - "version": "1.5.2", + "version": "1.6.1", "license": "MIT", "dependencies": { "commander": "^12.0.0", diff --git a/npm/package.json b/npm/package.json index acee7e4..403037b 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "@prime-vector/open-agent-spec", - "version": "1.6.0", + "version": "1.6.1", "description": "Run Open Agent Spec YAML files from Node.js — no Python required.", "keywords": [ "ai", diff --git a/npm/src/loader.ts b/npm/src/loader.ts index eba2d3f..80ecca3 100644 --- a/npm/src/loader.ts +++ b/npm/src/loader.ts @@ -42,8 +42,32 @@ const VERSION_PATTERN = /^(1\.(0\.[4-9]|[1-9]\.[0-9]+)|[2-9]\.[0-9]+\.[0-9]+)$/; // Spec features this runtime does not implement. Per the conformance honesty // rule, specs declaring them are REFUSED rather than silently degraded — // particularly important for sandbox, which is a security feature. -const UNSUPPORTED_ROOT_KEYS = ["tools", "sandbox", "behavioural_contract"] as const; -const UNSUPPORTED_TASK_KEYS = ["tools", "sandbox", "behavioural_contract"] as const; +const UNSUPPORTED_ROOT_KEYS = ["tools", "sandbox"] as const; +const UNSUPPORTED_TASK_KEYS = ["tools", "sandbox"] as const; +const ALLOW_DOMAIN_PATTERN = /^(?![A-Za-z][A-Za-z0-9+.-]*:\/\/)(?:\[[^\]]+\]|[^:/\s]+)(?::[0-9]{1,5})?$/; + +function validateAllowDomains(sandbox: unknown, source: string): void { + if (!sandbox || typeof sandbox !== "object" || Array.isArray(sandbox)) return; + const http = (sandbox as Record)["http"]; + if (!http || typeof http !== "object" || Array.isArray(http)) return; + const rules = (http as Record)["allow_domains"]; + if (rules === undefined) return; + if (!Array.isArray(rules)) { + fail(source, "sandbox.http.allow_domains must be an array"); + } + for (const [index, rule] of rules.entries()) { + if (typeof rule !== "string" || !ALLOW_DOMAIN_PATTERN.test(rule)) { + fail(source, `sandbox.http.allow_domains[${index}] must be a host or host:port`); + } + const portMatch = rule.match(/:([0-9]{1,5})$/); + if (portMatch) { + const port = Number(portMatch[1]); + if (port < 1 || port > 65535) { + fail(source, `sandbox.http.allow_domains[${index}] port must be between 1 and 65535`); + } + } + } +} export function loadSpecFromFile(specPath: string): OASpec { let raw: string; @@ -135,6 +159,38 @@ function validateSpec(data: Record, source: string): void { fail(source, "'tasks' must contain at least one task"); } + validateAllowDomains(data["sandbox"], source); + for (const [taskName, taskDef] of Object.entries(taskMap)) { + if (taskDef && typeof taskDef === "object") { + validateAllowDomains( + (taskDef as Record)["sandbox"], + `${source}: task '${taskName}'`, + ); + } + } + + if ("behavioural_contract" in data) { + throw new OAError( + `${source}: spec declares 'behavioural_contract:' but this runtime cannot enforce contracts.`, + "CONTRACTS_UNAVAILABLE", + "contract", + ); + } + for (const [taskName, taskDef] of Object.entries(taskMap)) { + if ( + taskDef && + typeof taskDef === "object" && + "behavioural_contract" in (taskDef as Record) + ) { + throw new OAError( + `${source}: task '${taskName}' declares 'behavioural_contract:' but this runtime cannot enforce contracts.`, + "CONTRACTS_UNAVAILABLE", + "contract", + taskName, + ); + } + } + // ── Unsupported feature guard (conformance honesty rule) ─────────────── for (const key of UNSUPPORTED_ROOT_KEYS) { if (key in data) { diff --git a/npm/tests/contracts.test.ts b/npm/tests/contracts.test.ts new file mode 100644 index 0000000..dbcff08 --- /dev/null +++ b/npm/tests/contracts.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, test } from "@jest/globals"; +import { parseSpec } from "../src/loader.js"; + +const base = ` +open_agent_spec: "1.6.0" +agent: + name: contract-test + description: test +intelligence: + type: llm + engine: openai + model: gpt-4o +tasks: + run: + description: run + output: {type: object} + prompts: {system: run, user: run} +`; + +describe("contract capability honesty", () => { + test("a root contract fails closed with CONTRACTS_UNAVAILABLE", () => { + const spec = `${base}\nbehavioural_contract:\n version: "1.0"\n`; + expect(() => parseSpec(spec)).toThrow( + expect.objectContaining({ + code: "CONTRACTS_UNAVAILABLE", + stage: "contract", + }), + ); + }); + + test("a task contract fails closed and identifies the task", () => { + const spec = base.replace( + " output: {type: object}", + ' behavioural_contract: {version: "1.0"}\n output: {type: object}', + ); + expect(() => parseSpec(spec)).toThrow( + expect.objectContaining({ + code: "CONTRACTS_UNAVAILABLE", + stage: "contract", + task: "run", + }), + ); + }); +}); + +describe("sandbox declaration validation", () => { + test("a URL-shaped allowlist entry is a spec-load error before unsupported-feature refusal", () => { + const spec = `${base}\nsandbox:\n http:\n allow_domains: [https://api.example.com]\n`; + expect(() => parseSpec(spec)).toThrow( + expect.objectContaining({ + code: "SPEC_LOAD_ERROR", + stage: "load", + }), + ); + }); +}); diff --git a/oas_cli/runner.py b/oas_cli/runner.py index d920377..9cee770 100644 --- a/oas_cli/runner.py +++ b/oas_cli/runner.py @@ -40,7 +40,8 @@ logger = logging.getLogger(__name__) -# Optional BCE integration — degrades gracefully when library is not installed. +# Optional dependency at import time; specs that declare contracts fail closed +# during preflight when the dependency is unavailable. try: from behavioural_contracts import validate_task_output # type: ignore[import] @@ -469,6 +470,65 @@ def _resolve_contract( return _merge_contracts(global_contract, task_contract) +def _require_contract_support(spec_data: dict[str, Any], task_name: str) -> None: + """Fail before model execution when a declared contract cannot be enforced.""" + if _resolve_contract(spec_data, task_name) is not None and not CONTRACTS_ENABLED: + raise OARunError( + "Behavioural contract enforcement is unavailable for task " + f"'{task_name}'. Install with: pip install 'open-agent-spec[contracts]'", + code="CONTRACTS_UNAVAILABLE", + stage="contract", + task=task_name, + ) + + +def _preflight_runtime_guards( + spec_data: dict[str, Any], + task_name: str, + *, + spec_path: Path | None = None, + _visited_specs: frozenset[Path] = frozenset(), +) -> None: + """Check static runtime guards before any task in a chain invokes a model. + + The selected task and its direct dependencies are checked together. Local + delegated specs are recursively inspectable and are therefore included in + the same preflight. Remote specs are guarded after fetch at their execution + boundary because their contents are not locally available. + """ + tasks = spec_data.get("tasks") or {} + task_def = tasks.get(task_name) or {} + for resolved_task in [*(task_def.get("depends_on") or []), task_name]: + _require_contract_support(spec_data, resolved_task) + sandbox = _resolve_sandbox(spec_data, resolved_task) + _check_mcp_endpoints(spec_data, resolved_task, sandbox) + + resolved_def = tasks.get(resolved_task) or {} + delegation_ref = resolved_def.get("spec") + if not isinstance(delegation_ref, str) or not delegation_ref.strip(): + continue + raw_ref = delegation_ref.strip() + if _is_remote_ref(raw_ref): + continue + + delegated_path = Path(raw_ref) + if not delegated_path.is_absolute() and spec_path is not None: + delegated_path = (spec_path.parent / delegated_path).resolve() + else: + delegated_path = delegated_path.resolve() + if delegated_path in _visited_specs: + continue + + delegated_spec = _load_spec(delegated_path) + delegated_task = resolved_def.get("task") or resolved_task + _preflight_runtime_guards( + delegated_spec, + delegated_task, + spec_path=delegated_path, + _visited_specs=_visited_specs | {delegated_path}, + ) + + def _resolve_sandbox(spec_data: dict[str, Any], task_name: str) -> dict[str, Any]: """Effective sandbox for a task: root-level merged with task-level (task wins). @@ -522,15 +582,7 @@ def _check_sandbox( allow_domains = (sandbox.get("http") or {}).get("allow_domains") if allow_domains is not None: url = arguments.get("url", "") - host = _urlparse(url).netloc.split(":")[0] - if not any(host == d or host.endswith(f".{d}") for d in allow_domains): - raise OARunError( - f"Sandbox violation: domain '{host}' is not in allow_domains for task " - f"'{task_name}'. Allowed: {allow_domains}", - code="SANDBOX_DOMAIN_VIOLATION", - stage="sandbox", - task=task_name, - ) + _check_url_domain(url, allow_domains, task_name) if tool_name in ("file.read", "file.write"): allow_paths = (sandbox.get("file") or {}).get("allow_paths") @@ -549,6 +601,75 @@ def _check_sandbox( ) +def _effective_port(parsed: Any) -> int | None: + """Return an explicit URL port, or the well-known port for HTTP(S).""" + try: + if parsed.port is not None: + return parsed.port + except ValueError: + return None + return {"http": 80, "https": 443}.get(parsed.scheme.lower()) + + +def _parse_domain_rule(rule: str) -> tuple[str, int | None]: + """Parse an allow_domains entry of the form host or host:port.""" + parsed = _urlparse(f"//{rule.strip()}") + try: + port = parsed.port + except ValueError: + return "", None + return (parsed.hostname or "").lower().rstrip("."), port + + +def _check_url_domain( + url: str, allow_domains: list[str], task_name: str, *, source: str = "HTTP request" +) -> None: + """Enforce hostname and optional port rules for a network destination.""" + parsed = _urlparse(url) + host = (parsed.hostname or "").lower().rstrip(".") + port = _effective_port(parsed) + + def _matches(rule: str) -> bool: + allowed_host, allowed_port = _parse_domain_rule(rule) + host_matches = host == allowed_host or host.endswith(f".{allowed_host}") + return bool( + allowed_host + and host_matches + and (allowed_port is None or port == allowed_port) + ) + + if not any(_matches(str(rule)) for rule in allow_domains): + destination = f"{host}:{port}" if port is not None else host + raise OARunError( + f"Sandbox violation: {source} domain '{destination}' is not in " + f"allow_domains for task '{task_name}'. Allowed: {allow_domains}", + code="SANDBOX_DOMAIN_VIOLATION", + stage="sandbox", + task=task_name, + ) + + +def _check_mcp_endpoints( + spec_data: dict[str, Any], task_name: str, sandbox: dict[str, Any] +) -> None: + """Preflight static MCP endpoints against the effective HTTP allowlist.""" + allow_domains = (sandbox.get("http") or {}).get("allow_domains") + if allow_domains is None: + return + tasks = spec_data.get("tasks") or {} + task_tools = (tasks.get(task_name) or {}).get("tools") or [] + spec_tools = spec_data.get("tools") or {} + for tool_name in task_tools: + tool_config = spec_tools.get(tool_name) or {} + if tool_config.get("type") == "mcp": + _check_url_domain( + str(tool_config.get("endpoint", "")), + allow_domains, + task_name, + source=f"MCP tool '{tool_name}' endpoint", + ) + + _MAX_TOOL_ITERATIONS = 10 @@ -715,6 +836,7 @@ def _run_single_task( tasks = spec_data.get("tasks") or {} task_def = tasks.get(task_name) or {} + _require_contract_support(spec_data, task_name) # ── Spec delegation ─────────────────────────────────────────────────── delegation_spec_ref: str | None = task_def.get("spec") @@ -831,6 +953,7 @@ def _run_single_task( # history is a reserved input convention — never stored by OA, just forwarded. history: list[dict] | None = input_data.get("history") or None sandbox = _resolve_sandbox(spec_data, task_name) + _check_mcp_endpoints(spec_data, task_name, sandbox) # Token usage for this task's model call, attached to the envelope below. usage: dict[str, Any] | None = None @@ -985,6 +1108,14 @@ def run_task_from_spec( # even when a chain merges upstream outputs into downstream inputs. base_input: dict[str, Any] = copy.deepcopy(dict(input_data or {})) chosen_task, _ = _choose_task(spec_data, task_name) + # Check the selected task before dependencies can spend tokens. Each + # dependency is checked again at its own task boundary. + _preflight_runtime_guards( + spec_data, + chosen_task, + spec_path=spec_path, + _visited_specs=frozenset({spec_path.resolve()}) if spec_path else frozenset(), + ) # Seed the visited set with the calling spec so direct self-delegation is caught. visited: frozenset[Path] = frozenset() diff --git a/oas_cli/schemas/oas-schema.json b/oas_cli/schemas/oas-schema.json index cd5b0e6..5564391 100644 --- a/oas_cli/schemas/oas-schema.json +++ b/oas_cli/schemas/oas-schema.json @@ -389,7 +389,7 @@ }, "behavioural_contract": { "type": "object", - "description": "Per-task behavioural contract. Merged with the top-level behavioural_contract (arrays are unioned, scalars use per-task-wins). Install behavioural-contracts to enable runtime enforcement." + "description": "Per-task behavioural contract. Merged with the top-level behavioural_contract (arrays are unioned, scalars use per-task-wins). A runtime without contract enforcement must fail closed before model execution." }, "tools": { "type": "array", @@ -586,12 +586,13 @@ "allow_domains": { "type": "array", "items": { - "type": "string" + "type": "string", + "pattern": "^(?![A-Za-z][A-Za-z0-9+.-]*://)(?:\\[[^\\]]+\\]|[^:/\\s]+)(?::[0-9]{1,5})?$" }, - "description": "Hosts permitted for http.get/http.post. A request host must equal a listed domain or be a subdomain of one; otherwise SANDBOX_DOMAIN_VIOLATION." + "description": "Hosts or host:port destinations permitted for http.get/http.post and MCP endpoints. A destination host must equal a listed domain or be a subdomain; an optional port must match the effective destination port. Otherwise SANDBOX_DOMAIN_VIOLATION." } }, - "description": "HTTP constraints for the native http.get/http.post tools." + "description": "Network destination constraints for native HTTP tools and MCP endpoints." }, "file": { "type": "object", diff --git a/oas_cli/validators.py b/oas_cli/validators.py index a057308..6c80b61 100644 --- a/oas_cli/validators.py +++ b/oas_cli/validators.py @@ -5,6 +5,7 @@ import json import logging +from urllib.parse import urlparse from jsonschema import validate from jsonschema.exceptions import SchemaError, ValidationError @@ -126,6 +127,49 @@ def _validate_behavioural_contract(spec_data: dict) -> None: _VALID_NATIVE_IDS = {"file.read", "file.write", "http.get", "http.post", "env.read"} +def _validate_allow_domain(rule: object, location: str) -> None: + if not isinstance(rule, str) or not rule.strip(): + raise ValueError(f"{location} must be a non-empty host or host:port string.") + value = rule.strip() + if "://" in value or any(char in value for char in "/?#@"): + raise ValueError( + f"{location} must be a host or host:port, not a URL: '{rule}'." + ) + parsed = urlparse(f"//{value}") + try: + port = parsed.port + except ValueError as exc: + raise ValueError(f"{location} has an invalid port in '{rule}'.") from exc + if not parsed.hostname: + raise ValueError(f"{location} has an invalid host in '{rule}'.") + if port is not None and not 1 <= port <= 65535: + raise ValueError(f"{location} port must be between 1 and 65535.") + + +def _validate_sandbox_domains(spec_data: dict) -> None: + """Validate root and task-level allow_domains entries.""" + + def _validate_block(sandbox: object, location: str) -> None: + if not isinstance(sandbox, dict): + return + http = sandbox.get("http") + if not isinstance(http, dict) or "allow_domains" not in http: + return + rules = http["allow_domains"] + if not isinstance(rules, list): + raise ValueError(f"{location}.http.allow_domains must be a list.") + for index, rule in enumerate(rules): + _validate_allow_domain(rule, f"{location}.http.allow_domains[{index}]") + + _validate_block(spec_data.get("sandbox"), "sandbox") + tasks = spec_data.get("tasks") or {} + if not isinstance(tasks, dict): + return # _validate_tasks provides the canonical type error. + for task_name, task_def in tasks.items(): + if isinstance(task_def, dict): + _validate_block(task_def.get("sandbox"), f"tasks.{task_name}.sandbox") + + def _validate_tools(spec_data: dict) -> None: """Validate the top-level tools: block (dict of named tool declarations).""" tools = spec_data.get("tools") @@ -459,6 +503,7 @@ def validate_spec(spec_data: dict) -> tuple[str, str]: _validate_agent(spec_data) _validate_behavioural_contract(spec_data) _validate_tools(spec_data) + _validate_sandbox_domains(spec_data) _validate_tasks(spec_data) _validate_integration(spec_data) _validate_prompts(spec_data) diff --git a/pyproject.toml b/pyproject.toml index a17acb5..98cd4b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "hatchling.build" [project] name = "open-agent-spec" -version = "1.6.0" +version = "1.6.1" description = "YAML-first agent specs: run with `oa run` or generate a full Python project with `oa init`." authors = [{ name = "Andrew Whitehouse", email = "andrewswhitehouse@gmail.com" }] license = { text = "MIT" } @@ -124,4 +124,3 @@ ignore = [ [tool.ruff.lint.per-file-ignores] "examples/multi-agent/*" = ["N999"] # directory name contains hyphen - diff --git a/spec/conformance/PROTOCOL.md b/spec/conformance/PROTOCOL.md index e899903..82449fb 100644 --- a/spec/conformance/PROTOCOL.md +++ b/spec/conformance/PROTOCOL.md @@ -158,6 +158,10 @@ list). The harness skips cases whose requirements are not in the adapter's declared capabilities and reports them as **UNSUPPORTED** — distinct from PASS and FAIL. Cases without `requires:` are implicitly `core`. +Honesty cases may also declare `requires_absent:`. Such a case runs only when +the named capability is not declared, allowing the suite to verify that a +runtime refuses a feature it cannot enforce instead of silently degrading it. + **Honesty rule:** a runtime MUST NOT declare a capability it does not enforce. In particular, a runtime that does not implement `sandbox` MUST refuse to run specs that declare a `sandbox:` block (rather than silently ignoring it). diff --git a/spec/conformance/cases/errors/contracts-unavailable.yaml b/spec/conformance/cases/errors/contracts-unavailable.yaml new file mode 100644 index 0000000..abe6ee2 --- /dev/null +++ b/spec/conformance/cases/errors/contracts-unavailable.yaml @@ -0,0 +1,38 @@ +# Spec §12.4 / §13.2 — unsupported declared contracts MUST fail closed. +description: "A runtime without contract capability raises CONTRACTS_UNAVAILABLE" +spec_section: "12.4" +requires_absent: contracts + +spec: | + open_agent_spec: "1.6.0" + agent: + name: test + description: test + intelligence: + type: llm + engine: openai + model: gpt-4o + behavioural_contract: + version: "1.0" + response_contract: + output_format: + required_fields: [answer] + tasks: + analyse: + description: analyse + output: + type: object + prompts: + system: "Analyse." + user: "analyse" + +mock_responses: + analyse: '{"answer": "yes"}' + +invoke: + task: analyse + input: {} + +expect_error: + code: CONTRACTS_UNAVAILABLE + stage: contract diff --git a/spec/conformance/cases/sandbox/domain-port-mismatch.yaml b/spec/conformance/cases/sandbox/domain-port-mismatch.yaml new file mode 100644 index 0000000..97447ff --- /dev/null +++ b/spec/conformance/cases/sandbox/domain-port-mismatch.yaml @@ -0,0 +1,38 @@ +# Spec §11.2 — host:port rules MUST match the effective destination port. +description: "A pinned domain port blocks a different MCP endpoint port" +spec_section: "11.2" +requires: sandbox + +spec: | + open_agent_spec: "1.6.0" + agent: + name: test + description: test + intelligence: + type: llm + engine: openai + model: gpt-4o + sandbox: + http: + allow_domains: [localhost:3000] + tools: + remote: + type: mcp + endpoint: http://localhost:9000/mcp + tasks: + run: + description: run + tools: [remote] + output: + type: object + prompts: + system: "Run." + user: "run" + +invoke: + task: run + input: {} + +expect_error: + code: SANDBOX_DOMAIN_VIOLATION + stage: sandbox diff --git a/spec/conformance/cases/sandbox/mcp-domain-preflight.yaml b/spec/conformance/cases/sandbox/mcp-domain-preflight.yaml new file mode 100644 index 0000000..f9357ec --- /dev/null +++ b/spec/conformance/cases/sandbox/mcp-domain-preflight.yaml @@ -0,0 +1,47 @@ +# Spec §11.3 — MCP endpoints MUST be checked before discovery/model execution. +description: "A blocked MCP dependency endpoint fails during chain preflight" +spec_section: "11.3" +requires: sandbox + +spec: | + open_agent_spec: "1.6.0" + agent: + name: test + description: test + intelligence: + type: llm + engine: openai + model: gpt-4o + sandbox: + http: + allow_domains: [safe.example] + tools: + remote: + type: mcp + endpoint: https://blocked.example/mcp + tasks: + first: + description: first + output: {type: object} + prompts: {system: "First.", user: "first"} + remote_task: + description: remote + tools: [remote] + output: {type: object} + prompts: {system: "Remote.", user: "remote_task"} + run: + description: run + depends_on: [first, remote_task] + output: {type: object} + prompts: {system: "Run.", user: "run"} + +mock_responses: + first: '{}' + +invoke: + task: run + input: {} + +expect_error: + code: SANDBOX_DOMAIN_VIOLATION + stage: sandbox diff --git a/spec/conformance/cases/schema/invalid-allow-domain.yaml b/spec/conformance/cases/schema/invalid-allow-domain.yaml new file mode 100644 index 0000000..f8c34b0 --- /dev/null +++ b/spec/conformance/cases/schema/invalid-allow-domain.yaml @@ -0,0 +1,28 @@ +description: "allow_domains entries must be host or host:port, not URLs" +spec_section: "11.2" + +spec: | + open_agent_spec: "1.6.0" + agent: + name: invalid-domain + description: test + intelligence: + type: llm + engine: openai + model: gpt-4o + sandbox: + http: + allow_domains: [https://api.example.com] + tasks: + run: + description: run + output: {type: object} + prompts: {system: "Run.", user: "run"} + +invoke: + task: run + input: {} + +expect_error: + code: SPEC_LOAD_ERROR + stage: load diff --git a/spec/conformance/harness/harness.py b/spec/conformance/harness/harness.py index 1ad975e..d6f2fb7 100644 --- a/spec/conformance/harness/harness.py +++ b/spec/conformance/harness/harness.py @@ -82,6 +82,16 @@ def _case_requirements(case: dict) -> set[str]: return set(req) +def _case_absent_requirements(case: dict) -> set[str]: + """Capabilities that must be absent for an honesty case to apply.""" + req = case.get("requires_absent") + if req is None: + return set() + if isinstance(req, str): + return {req} + return set(req) + + def discover_cases(category: str | None = None) -> list[tuple[str, Path]]: cases: list[tuple[str, Path]] = [] categories = [category] if category else _CATEGORIES @@ -291,6 +301,9 @@ def run_case_against(adapter: Adapter, case: dict, case_path: Path) -> tuple[str missing = requirements - adapter.capabilities if missing: return UNSUPPORTED, f"requires {sorted(missing)}" + unexpectedly_present = _case_absent_requirements(case) & adapter.capabilities + if unexpectedly_present: + return UNSUPPORTED, f"requires absent {sorted(unexpectedly_present)}" invoke = case["invoke"] payload: dict[str, Any] = { diff --git a/spec/extensions/README.md b/spec/extensions/README.md index 29c230b..754eff3 100644 --- a/spec/extensions/README.md +++ b/spec/extensions/README.md @@ -81,7 +81,7 @@ tasks: pip install 'open-agent-spec[contracts]' ``` -When the library is not installed, contract validation is skipped with a warning — the runtime degrades gracefully. +When a resolved task declares a contract and the library is not installed, the runtime fails closed with `CONTRACTS_UNAVAILABLE` before model execution. Specs without contracts are unaffected. --- diff --git a/spec/open-agent-spec-1.6.md b/spec/open-agent-spec-1.6.md index 1496e5c..e5c1daa 100644 --- a/spec/open-agent-spec-1.6.md +++ b/spec/open-agent-spec-1.6.md @@ -577,7 +577,7 @@ sandbox: # root level — applies to all tasks allow: [file.read, http.get] # only these tools may be called deny: [env.read] # these tools may never be called http: - allow_domains: [api.example.com] # http.get/http.post restricted to these hosts + allow_domains: [api.example.com] # HTTP and MCP restricted to these hosts file: allow_paths: [./data/] # file.read/file.write restricted to these prefixes @@ -598,7 +598,7 @@ tasks: |-----|-----------|-----------| | `tools.allow` | every tool dispatch | If present, a tool not in the list MUST be refused (`SANDBOX_TOOL_VIOLATION`) | | `tools.deny` | every tool dispatch | If present, a tool in the list MUST be refused (`SANDBOX_TOOL_VIOLATION`). Deny is checked in addition to allow. | -| `http.allow_domains` | `http.get`, `http.post` | The request URL's hostname MUST equal a listed domain or be a subdomain of one (`host == d` or `host` ends with `.d`); otherwise `SANDBOX_DOMAIN_VIOLATION` | +| `http.allow_domains` | `http.get`, `http.post`, MCP endpoints | The destination hostname MUST equal a listed domain or be a subdomain. A bare hostname permits any port for backwards compatibility; a `host:port` entry MUST match the destination's effective port. Otherwise raise `SANDBOX_DOMAIN_VIOLATION`. | | `file.allow_paths` | `file.read`, `file.write` | The resolved absolute path MUST fall under one of the listed path prefixes (after resolving symlinks and `..`); otherwise `SANDBOX_PATH_VIOLATION` | An absent constraint key imposes no restriction of that type. An empty `allow` list denies everything of that type. @@ -611,6 +611,7 @@ A runtime that supports sandboxing MUST: 2. Enforce path constraints against the **resolved** path (symlinks and relative segments resolved), not the literal argument. 3. Surface violations as structured errors with stage `sandbox` and the specific code for the constraint type (Section 13.2) — never as a generic run failure. 4. Continue to enforce the sandbox regardless of what the model requests; sandbox constraints are not visible to or negotiable by the model. +5. Validate statically configured MCP endpoints against `http.allow_domains` before MCP tool discovery or model execution. **Honesty rule.** A runtime that does not implement sandboxing MUST refuse to run a spec that declares a `sandbox:` block, rather than silently ignoring it. Silent degradation of a declared security constraint is itself a conformance violation (see `spec/conformance/PROTOCOL.md`). @@ -713,7 +714,8 @@ Note the asymmetry with sandbox resolution (Section 11.1): contracts **merge** ( **Contract enforcement is skipped when:** - `response_format: "text"` (field checks are meaningless on raw strings) - The output could not be parsed as a dict (warning logged, execution continues) -- The behavioural contracts library is not installed (warning logged, execution continues) + +If a resolved task declares a contract but the runtime's contract-enforcement capability is unavailable, the runtime MUST fail before that task's model execution with `CONTRACTS_UNAVAILABLE`. Statically resolvable local delegated tasks SHOULD be preflighted before their containing dependency chain starts; remote delegated tasks MUST be checked immediately after fetch and before their model execution. A runtime MUST NOT silently ignore the declared contract. A contract violation MUST raise `CONTRACT_VIOLATION`. @@ -744,13 +746,14 @@ A runtime MUST surface errors as structured objects with the following fields: | `CHAIN_CYCLE_ERROR` | `routing` | Circular `depends_on` chain detected | | `CHAIN_INPUT_MISSING` | `input_validation` | Required input field missing after dependency merge | | `CONTRACT_VIOLATION` | `contract` | Task output failed behavioural contract validation | +| `CONTRACTS_UNAVAILABLE` | `contract` | A resolved task declares a behavioural contract but enforcement is unavailable | | `DELEGATION_CYCLE_ERROR` | `delegation` | Circular spec delegation detected (A→B→A) | | `PRICING_CONFIG_ERROR` | `cost` | A cost-rate override (`config.pricing` or an implementation-defined global override) is present but invalid | | `SANDBOX_TOOL_VIOLATION` | `sandbox` | Tool blocked by the effective `tools.allow`/`tools.deny` sandbox constraint | -| `SANDBOX_DOMAIN_VIOLATION` | `sandbox` | HTTP request host not permitted by `http.allow_domains` | +| `SANDBOX_DOMAIN_VIOLATION` | `sandbox` | HTTP or MCP destination host/port not permitted by `http.allow_domains` | | `SANDBOX_PATH_VIOLATION` | `sandbox` | File path outside `file.allow_paths` after resolution | -A runtime MUST detect and raise `CHAIN_CYCLE_ERROR`, `DELEGATION_CYCLE_ERROR`, and `PRICING_CONFIG_ERROR` before any model call is made. +A runtime MUST detect and raise `CHAIN_CYCLE_ERROR`, `DELEGATION_CYCLE_ERROR`, and `PRICING_CONFIG_ERROR` before any model call is made. It MUST raise `CONTRACTS_UNAVAILABLE` before the affected task invokes a model; statically resolvable tasks SHOULD be checked before their containing chain starts. --- diff --git a/spec/schema/oas-schema-1.6.json b/spec/schema/oas-schema-1.6.json index cd5b0e6..5564391 100644 --- a/spec/schema/oas-schema-1.6.json +++ b/spec/schema/oas-schema-1.6.json @@ -389,7 +389,7 @@ }, "behavioural_contract": { "type": "object", - "description": "Per-task behavioural contract. Merged with the top-level behavioural_contract (arrays are unioned, scalars use per-task-wins). Install behavioural-contracts to enable runtime enforcement." + "description": "Per-task behavioural contract. Merged with the top-level behavioural_contract (arrays are unioned, scalars use per-task-wins). A runtime without contract enforcement must fail closed before model execution." }, "tools": { "type": "array", @@ -586,12 +586,13 @@ "allow_domains": { "type": "array", "items": { - "type": "string" + "type": "string", + "pattern": "^(?![A-Za-z][A-Za-z0-9+.-]*://)(?:\\[[^\\]]+\\]|[^:/\\s]+)(?::[0-9]{1,5})?$" }, - "description": "Hosts permitted for http.get/http.post. A request host must equal a listed domain or be a subdomain of one; otherwise SANDBOX_DOMAIN_VIOLATION." + "description": "Hosts or host:port destinations permitted for http.get/http.post and MCP endpoints. A destination host must equal a listed domain or be a subdomain; an optional port must match the effective destination port. Otherwise SANDBOX_DOMAIN_VIOLATION." } }, - "description": "HTTP constraints for the native http.get/http.post tools." + "description": "Network destination constraints for native HTTP tools and MCP endpoints." }, "file": { "type": "object", diff --git a/tests/test_iis.py b/tests/test_iis.py index 3553571..06d0e16 100644 --- a/tests/test_iis.py +++ b/tests/test_iis.py @@ -17,6 +17,7 @@ from oas_cli.runner import ( OARunError, + _check_mcp_endpoints, _check_sandbox, _resolve_sandbox, run_task_from_spec, @@ -162,6 +163,32 @@ def test_blocks_domain_with_port(self): ) assert exc_info.value.code == "SANDBOX_DOMAIN_VIOLATION" + def test_bare_domain_allows_any_port_for_backwards_compatibility(self): + sandbox = {"http": {"allow_domains": ["localhost"]}} + _check_sandbox( + "http.get", {"url": "http://localhost:8765/health"}, sandbox, "task" + ) + + def test_domain_with_port_allows_matching_effective_port(self): + sandbox = {"http": {"allow_domains": ["api.example.com:443"]}} + _check_sandbox( + "http.get", {"url": "https://api.example.com/v1"}, sandbox, "task" + ) + + def test_domain_with_port_blocks_different_port(self): + sandbox = {"http": {"allow_domains": ["localhost:3000"]}} + with pytest.raises(OARunError) as exc_info: + _check_sandbox( + "http.get", {"url": "http://localhost:8080/admin"}, sandbox, "task" + ) + assert exc_info.value.code == "SANDBOX_DOMAIN_VIOLATION" + + def test_domain_and_port_matching_is_case_insensitive(self): + sandbox = {"http": {"allow_domains": ["API.Example.COM:443"]}} + _check_sandbox( + "http.get", {"url": "https://api.example.com/data"}, sandbox, "task" + ) + def test_domain_check_skipped_for_non_http_tool(self): sandbox = {"http": {"allow_domains": ["api.example.com"]}} # file.read with a "url"-like argument should not trigger domain check @@ -172,6 +199,41 @@ def test_no_allow_domains_permits_any_url(self): _check_sandbox("http.get", {"url": "https://any-domain.io"}, sandbox, "task") +class TestMCPDomainPreflight: + def _spec(self, endpoint: str) -> dict: + return { + "tools": {"search": {"type": "mcp", "endpoint": endpoint}}, + "tasks": {"run": {"tools": ["search"]}}, + } + + def test_allows_mcp_endpoint_on_allowlist(self): + sandbox = {"http": {"allow_domains": ["mcp.example.com:443"]}} + _check_mcp_endpoints(self._spec("https://mcp.example.com/rpc"), "run", sandbox) + + def test_blocks_mcp_endpoint_outside_allowlist(self): + sandbox = {"http": {"allow_domains": ["mcp.example.com"]}} + with pytest.raises(OARunError) as exc_info: + _check_mcp_endpoints( + self._spec("https://evil.example.net/rpc"), "run", sandbox + ) + err = exc_info.value + assert err.code == "SANDBOX_DOMAIN_VIOLATION" + assert "MCP tool 'search' endpoint" in str(err) + + def test_blocks_mcp_endpoint_on_different_port(self): + sandbox = {"http": {"allow_domains": ["localhost:3000"]}} + with pytest.raises(OARunError) as exc_info: + _check_mcp_endpoints( + self._spec("http://localhost:9000/mcp"), "run", sandbox + ) + assert exc_info.value.code == "SANDBOX_DOMAIN_VIOLATION" + + def test_no_allowlist_does_not_restrict_mcp(self): + _check_mcp_endpoints( + self._spec("https://mcp.example.com/rpc"), "run", {"http": {}} + ) + + # ── _check_sandbox — path enforcement ──────────────────────────────────────── @@ -319,6 +381,65 @@ def test_permitted_tool_is_dispatched(self): mock_dispatch.assert_called_once() assert result["output"]["content"] == "hello" + def test_mcp_endpoint_violation_precedes_discovery_and_model_call(self): + spec = _minimal_spec(sandbox={"http": {"allow_domains": ["safe.example"]}}) + spec["tools"] = { + "remote": { + "type": "mcp", + "endpoint": "https://blocked.example/mcp", + } + } + spec["tasks"]["run"]["tools"] = ["remote"] + + with ( + patch("oas_cli.runner.resolve_task_tools") as mock_resolve_tools, + patch("oas_cli.runner.invoke_intelligence") as mock_invoke, + pytest.raises(OARunError) as exc_info, + ): + run_task_from_spec(spec, task_name="run", input_data={}) + + assert exc_info.value.code == "SANDBOX_DOMAIN_VIOLATION" + mock_resolve_tools.assert_not_called() + mock_invoke.assert_not_called() + + def test_mcp_dependency_violation_is_preflighted_before_chain(self): + spec = _minimal_spec(sandbox={"http": {"allow_domains": ["safe.example"]}}) + spec["tools"] = { + "remote": { + "type": "mcp", + "endpoint": "https://blocked.example/mcp", + } + } + spec["tasks"] = { + "first": { + "description": "would spend tokens", + "output": {"type": "object"}, + "prompts": {"system": "first", "user": "first"}, + }, + "mcp_task": { + "description": "blocked MCP task", + "tools": ["remote"], + "output": {"type": "object"}, + "prompts": {"system": "mcp", "user": "mcp"}, + }, + "run": { + "description": "run", + "depends_on": ["first", "mcp_task"], + "output": {"type": "object"}, + "prompts": {"system": "run", "user": "run"}, + }, + } + + with ( + patch("oas_cli.runner.invoke_intelligence") as mock_invoke, + pytest.raises(OARunError) as exc_info, + ): + run_task_from_spec(spec, task_name="run", input_data={}) + + assert exc_info.value.code == "SANDBOX_DOMAIN_VIOLATION" + assert exc_info.value.task == "mcp_task" + mock_invoke.assert_not_called() + # ── Chain-wide immutability ─────────────────────────────────────────────────── diff --git a/tests/test_runner.py b/tests/test_runner.py index 24aa3b6..a630576 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -787,6 +787,129 @@ def test_global_plus_task_merged(self): # --------------------------------------------------------------------------- +class TestContractEnforcementUnavailable: + def _contract_spec(self, *, dependency_contract: bool = False) -> dict: + contract = { + "version": "1.0", + "description": "required enforcement", + "response_contract": {"output_format": {"required_fields": ["result"]}}, + } + tasks: dict = { + "run": { + "description": "run", + "output": {"type": "object"}, + "prompts": {"system": "sys", "user": "run"}, + } + } + if dependency_contract: + tasks["first"] = { + "description": "first dependency without a contract", + "output": {"type": "object"}, + "prompts": {"system": "sys", "user": "first"}, + } + tasks["prepare"] = { + "description": "prepare", + "output": {"type": "object"}, + "prompts": {"system": "sys", "user": "prepare"}, + "behavioural_contract": contract, + } + tasks["run"]["depends_on"] = ["first", "prepare"] + else: + tasks["run"]["behavioural_contract"] = contract + return { + "open_agent_spec": "1.6.0", + "agent": {"name": "contract-test", "description": "test"}, + "intelligence": {"type": "llm", "engine": "openai", "model": "gpt-4o"}, + "tasks": tasks, + } + + def test_declared_contract_fails_closed_before_model_call(self, monkeypatch): + def invoke(*args, **kwargs): + pytest.fail("model must not be called") + + monkeypatch.setattr("oas_cli.runner.CONTRACTS_ENABLED", False) + monkeypatch.setattr("oas_cli.runner.invoke_intelligence", invoke) + + with pytest.raises(OARunError) as exc_info: + run_task_from_spec(self._contract_spec(), task_name="run") + + err = exc_info.value + assert err.code == "CONTRACTS_UNAVAILABLE" + assert err.stage == "contract" + assert err.task == "run" + + def test_dependency_contract_fails_before_any_dependency_model_call( + self, monkeypatch + ): + def invoke(*args, **kwargs): + pytest.fail("model must not be called") + + monkeypatch.setattr("oas_cli.runner.CONTRACTS_ENABLED", False) + monkeypatch.setattr("oas_cli.runner.invoke_intelligence", invoke) + + with pytest.raises(OARunError) as exc_info: + run_task_from_spec( + self._contract_spec(dependency_contract=True), task_name="run" + ) + + err = exc_info.value + assert err.code == "CONTRACTS_UNAVAILABLE" + assert err.task == "prepare" + + def test_local_delegated_contract_is_preflighted_before_chain( + self, monkeypatch, tmp_path + ): + delegated = tmp_path / "delegated.yaml" + delegated.write_text( + """ +open_agent_spec: "1.6.0" +agent: {name: delegated, description: test} +intelligence: {type: llm, engine: openai, model: gpt-4o} +tasks: + work: + description: delegated work + behavioural_contract: + version: "1.0" + response_contract: + output_format: {required_fields: [result]} + output: {type: object} + prompts: {system: delegated, user: work} +""".strip() + ) + spec = self._contract_spec() + spec["tasks"]["run"].pop("behavioural_contract") + spec["tasks"]["first"] = { + "description": "first", + "output": {"type": "object"}, + "prompts": {"system": "first", "user": "first"}, + } + spec["tasks"]["delegated"] = { + "description": "delegated", + "spec": "delegated.yaml", + "task": "work", + } + spec["tasks"]["run"]["depends_on"] = ["first", "delegated"] + calls: list[str] = [] + + def invoke(*args, **kwargs): + calls.append("called") + return "{}" + + monkeypatch.setattr("oas_cli.runner.CONTRACTS_ENABLED", False) + monkeypatch.setattr("oas_cli.runner.invoke_intelligence", invoke) + + with pytest.raises(OARunError) as exc_info: + run_task_from_spec( + spec, + task_name="run", + spec_path=tmp_path / "main.yaml", + ) + + assert exc_info.value.code == "CONTRACTS_UNAVAILABLE" + assert exc_info.value.task == "work" + assert calls == [] + + @pytest.mark.skipif(not CONTRACTS_ENABLED, reason="behavioural-contracts not installed") class TestContractEnforcementLive: """Tests that require the actual behavioural-contracts library.""" diff --git a/tests/test_validation_errors.py b/tests/test_validation_errors.py index 17eb934..76d314d 100644 --- a/tests/test_validation_errors.py +++ b/tests/test_validation_errors.py @@ -146,6 +146,38 @@ def test_custom_tool_missing_module(self): validate_spec(spec) +class TestSandboxDomainErrors: + @pytest.mark.parametrize( + "rule", + ["localhost:abc", "https://api.example.com", "api.example.com/path", ""], + ) + def test_rejects_malformed_root_allow_domain(self, rule): + spec = _valid_spec( + sandbox={"http": {"allow_domains": ["api.example.com", rule]}} + ) + with pytest.raises(ValueError, match=r"allow_domains\[1\]"): + validate_spec(spec) + + def test_rejects_out_of_range_port(self): + spec = _valid_spec(sandbox={"http": {"allow_domains": ["localhost:70000"]}}) + with pytest.raises(ValueError, match=r"invalid port|between 1 and 65535"): + validate_spec(spec) + + def test_accepts_host_and_host_port(self): + spec = _valid_spec( + sandbox={"http": {"allow_domains": ["api.example.com", "localhost:3000"]}} + ) + validate_spec(spec) + + def test_rejects_malformed_task_allow_domain(self): + spec = _valid_spec() + spec["tasks"]["task1"]["sandbox"] = { + "http": {"allow_domains": ["https://api.example.com"]} + } + with pytest.raises(ValueError, match=r"tasks\.task1\.sandbox"): + validate_spec(spec) + + # -- Task validation ---------------------------------------------------