diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 3041071..193f6bc 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -146,11 +146,30 @@ jobs: ensure_release + python - <<'PY' + import hashlib + from pathlib import Path + + dist_dir = Path("dist") + lines = [] + for asset in sorted( + ( + path + for path in dist_dir.iterdir() + if path.is_file() and path.name.endswith((".whl", ".tar.gz")) + ), + key=lambda item: item.name, + ): + digest = hashlib.sha256(asset.read_bytes()).hexdigest() + lines.append(f"{digest} {asset.name}") + (dist_dir / "SHA256SUMS").write_text("\n".join(lines) + "\n", encoding="utf-8") + PY + mapfile -t existing_assets < <( retry 3 5 gh release view "$RELEASE_TAG" --json assets --jq '.assets[].name' ) - for asset in dist/*.tar.gz dist/*.whl; do + for asset in dist/*.tar.gz dist/*.whl dist/SHA256SUMS; do asset_name="$(basename "$asset")" if printf '%s\n' "${existing_assets[@]}" | grep -Fxq "$asset_name"; then echo "Release asset already present: $asset_name" diff --git a/docs/security-architecture.md b/docs/security-architecture.md index 2fde0f9..739edbc 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -99,6 +99,47 @@ All REST routes are served at the root path (no `/v1` prefix). - The upstream OpenCode client (`OPENCODE_BASE_URL`, auth, timeouts, concurrency caps) is the only other outbound path. +## Security Controls + +### Error Text Redaction + +Client-visible error text must never expose absolute local filesystem paths. +All error responses that can leave the process pass through a single +deterministic masker (`opencode_a2a.redact.redact_absolute_paths`) before +serialization; masked output uses the fixed placeholder ``. + +Boundaries covered: + +- Streaming task error messages — `execution/executor.py:_emit_error` + (task status messages and streamed error artifacts). +- JSON-RPC error responses — `jsonrpc/error_responses.py:adapt_jsonrpc_error` + (message, metadata values, and standard JSON-RPC error-code `data`). +- REST/HTTP error bodies — `jsonrpc/error_responses.py:build_http_error_body`. +- Raw-exception fallback — `jsonrpc/application.py:_generate_error_response` + (SDK base-class wrapping of non-JSON-RPC exceptions). + +Masked: POSIX/Windows/UNC absolute paths and `file://` local URIs. Preserved: +remote URLs (`scheme://host/path`), relative paths, ordinary prose, and +slash-prefixed API route tokens such as `/message:send` or `/tasks/{id}:cancel` +(a path-like token immediately followed by a `:` method suffix or `{` route +template is treated as an API route, not a local path). Server-side logs +intentionally retain full exception context for diagnosability; logs that +leave the host must be redacted or access-restricted before export. + +Remote peer error text (for example the upstream `detail` field surfaced by +`execution/upstream_error_translator.py`) is remote content, not a local path +leak; it is out of scope for this control and should be assessed separately if +it becomes a trust concern. + +### Release Integrity + +Every GitHub Release must ship a `SHA256SUMS` checksum manifest alongside the +wheel and sdist artifacts so consumers can verify artifact integrity +independently of the registry. `.github/workflows/publish.yml` regenerates +`dist/SHA256SUMS` at publish time from the built assets (sorted by basename, +`sha256sum -c` compatible) and uploads it as a release asset; existing assets +are skipped idempotently. + ## End-to-End Mapping: Remote A2A Input → OpenCode Side Effects | A2A input | Adapter path | OpenCode / host side effects | @@ -127,7 +168,8 @@ All REST routes are served at the root path (no `/v1` prefix). - This document is the canonical security-surface reference. Update the route table, mapping, or risk register in the same change that alters listeners, routes, authentication/authorization, input limits, outbound policy, - persistence hardening, or known residual risks. + persistence hardening, error-text redaction boundaries, release integrity, + or known residual risks. - Do not turn this document into a report for a specific review cycle: keep it focused on facts that remain true for the current code and must stay maintainable. One-off conclusions belong in issue/PR history. diff --git a/src/opencode_a2a/execution/executor.py b/src/opencode_a2a/execution/executor.py index 5d2dfe8..19fffcd 100644 --- a/src/opencode_a2a/execution/executor.py +++ b/src/opencode_a2a/execution/executor.py @@ -40,6 +40,7 @@ map_a2a_parts_to_opencode_parts, summarize_a2a_parts, ) +from ..redact import redact_absolute_paths from ..sandbox_policy import SandboxPolicy from .coordinator import ExecutionCoordinator, PreparedExecution, build_session_binding_context_id from .event_helpers import _enqueue_artifact_update @@ -437,6 +438,7 @@ async def _emit_error( upstream_status: int | None = None, streaming_request: bool, ) -> None: + message = redact_absolute_paths(message) error_message = Message( message_id=str(uuid.uuid4()), role=Role.ROLE_AGENT, diff --git a/src/opencode_a2a/jsonrpc/application.py b/src/opencode_a2a/jsonrpc/application.py index e84d5eb..ee11343 100644 --- a/src/opencode_a2a/jsonrpc/application.py +++ b/src/opencode_a2a/jsonrpc/application.py @@ -6,10 +6,16 @@ from typing import Any, cast from a2a.server.events import Event +from a2a.server.jsonrpc_models import JSONRPCError as SDKJSONRPCError from a2a.server.request_handlers.response_helpers import agent_card_to_dict, build_error_response from a2a.server.routes.jsonrpc_dispatcher import JsonRpcDispatcher from a2a.utils import proto_utils -from a2a.utils.errors import JSON_RPC_ERROR_CODE_MAP, A2AError, UnsupportedOperationError +from a2a.utils.errors import ( + JSON_RPC_ERROR_CODE_MAP, + A2AError, + InternalError, + UnsupportedOperationError, +) from fastapi import FastAPI from fastapi.responses import JSONResponse from google.protobuf.json_format import MessageToDict, ParseDict @@ -22,6 +28,7 @@ requested_extensions_from_call_context, ) from ..opencode_upstream_client import OpencodeUpstreamClient +from ..redact import redact_absolute_paths from ..server.runtime_limits import apply_stream_budget from .dispatch import ( ExtensionHandlerContext, @@ -177,28 +184,64 @@ def __init__( def add_routes_to_app(self, app: FastAPI, *, rpc_url: str = "/") -> None: app.add_api_route(rpc_url, self.handle_requests, methods=["POST"]) + @staticmethod + def _build_error_payload(error: JSONRPCError | A2AError) -> dict[str, Any]: + """Serialize an adapted error into a JSON-RPC ``error`` payload.""" + if isinstance(error, A2AError): + error_payload: dict[str, Any] = { + "code": JSON_RPC_ERROR_CODE_MAP.get(type(error), -32603), + "message": error.message, + } + if error.data is not None: + error_payload["data"] = error.data + else: + error_payload = { + "code": error.code, + "message": error.message, + } + if error.data is not None: + error_payload["data"] = error.data + return error_payload + + def _generate_error_response( + self, + request_id: str | int | None, + error: Exception | SDKJSONRPCError | A2AError, + ) -> JSONResponse: + """Adapt and redact errors before serializing the JSON-RPC response. + + The response is built here (rather than delegated to the SDK base + class) so adapted ``opencode_a2a`` errors keep their structured code, + message, and data instead of being stringified as a wrapped internal + error. + """ + if isinstance(error, A2AError | JSONRPCError): + adapted = adapt_jsonrpc_error(error) + elif isinstance(error, SDKJSONRPCError): + adapted = adapt_jsonrpc_error(cast(Any, error)) + else: + adapted = InternalError(message=redact_absolute_paths(str(error))) + return JSONResponse( + { + "jsonrpc": "2.0", + "id": request_id, + "error": self._build_error_payload(adapted), + }, + status_code=200, + ) + def _generate_protocol_error_response( self, request_id: str | int | None, error: JSONRPCError | A2AError, ) -> JSONResponse: adapted = adapt_jsonrpc_error(error) - if isinstance(adapted, A2AError): - error_payload = { - "code": JSON_RPC_ERROR_CODE_MAP.get(type(adapted), -32603), - "message": adapted.message, - } - if adapted.data is not None: - error_payload["data"] = adapted.data - else: - error_payload = { - "code": adapted.code, - "message": adapted.message, - } - if adapted.data is not None: - error_payload["data"] = adapted.data return JSONResponse( - {"jsonrpc": "2.0", "id": request_id, "error": error_payload}, + { + "jsonrpc": "2.0", + "id": request_id, + "error": self._build_error_payload(adapted), + }, status_code=200, ) diff --git a/src/opencode_a2a/jsonrpc/error_responses.py b/src/opencode_a2a/jsonrpc/error_responses.py index df8a736..bb951fe 100644 --- a/src/opencode_a2a/jsonrpc/error_responses.py +++ b/src/opencode_a2a/jsonrpc/error_responses.py @@ -11,6 +11,7 @@ ) from ..protocol_versions import A2A_PROTOCOL_VERSION +from ..redact import redact_absolute_paths, redact_paths_in_value from .models import JSONRPCError A2A_ERROR_DOMAIN = "a2a-protocol.org" @@ -57,10 +58,12 @@ def _camelize(value: Any) -> Any: def _stringify_metadata_value(value: Any) -> str: if isinstance(value, str): - return value + return redact_absolute_paths(value) if isinstance(value, bool | int | float): return str(value) - return json.dumps(value, ensure_ascii=False, separators=(",", ":"), sort_keys=True) + return redact_absolute_paths( + json.dumps(value, ensure_ascii=False, separators=(",", ":"), sort_keys=True) + ) def _build_error_info_detail( @@ -85,7 +88,7 @@ def _build_error_info_detail( def _build_context_detail(type_name: str, payload: Mapping[str, Any]) -> dict[str, Any]: return { "@type": f"type.googleapis.com/opencode_a2a.{type_name}", - **_camelize(dict(payload)), + **_camelize({str(key): redact_paths_in_value(value) for key, value in payload.items()}), } @@ -124,11 +127,11 @@ def adapt_jsonrpc_error(error: JSONRPCError | A2AError) -> JSONRPCError | A2AErr if root_error.code in STANDARD_JSONRPC_ERROR_CODES: adapted_data = None if isinstance(root_data, Mapping): - adapted_data = _camelize( - {str(key): value for key, value in root_data.items() if key != "type"} + adapted_data = redact_paths_in_value( + _camelize({str(key): value for key, value in root_data.items() if key != "type"}) ) elif root_data is not None: - adapted_data = root_data + adapted_data = redact_paths_in_value(root_data) return JSONRPCError( code=root_error.code, message=STANDARD_JSONRPC_ERROR_MESSAGES[root_error.code], @@ -146,6 +149,7 @@ def adapt_jsonrpc_error(error: JSONRPCError | A2AError) -> JSONRPCError | A2AErr message = root_error.message if message is None: message = STANDARD_JSONRPC_ERROR_MESSAGES.get(root_error.code, "Internal error") + message = redact_absolute_paths(message) return JSONRPCError( code=root_error.code, @@ -171,7 +175,7 @@ def build_http_error_body( error_payload: dict[str, Any] = { "code": status_code, "status": status, - "message": message, + "message": redact_absolute_paths(message), } if details: error_payload["details"] = details diff --git a/src/opencode_a2a/redact.py b/src/opencode_a2a/redact.py new file mode 100644 index 0000000..4e5efa6 --- /dev/null +++ b/src/opencode_a2a/redact.py @@ -0,0 +1,82 @@ +"""Deterministic masking of absolute filesystem paths in error-facing text. + +Client-visible error messages (JSON-RPC/REST error bodies and streaming task +messages) can embed exception text that includes host-local absolute paths, +for example:: + + FileNotFoundError: [Errno 2] No such file or directory: '/home/ubuntu/x' + +This module replaces such paths with a fixed placeholder before the text +leaves the process, while leaving URLs, relative paths, and ordinary prose +untouched. The replacement is deterministic and idempotent. +""" + +from __future__ import annotations + +import re +from typing import Any + +REDACTED_PATH_PLACEHOLDER = "" + +# A path segment: word characters plus common path-safe punctuation +# (``- _ + @ %``), with dots allowed only between word characters so trailing +# sentence punctuation (``.``/``,``/``;``) is not swallowed by the match. +_SEGMENT = r"[\w@+%-]+(?:\.[\w@+%-]+)*" + +# POSIX absolute paths: ``/a/b`` with at least one segment. The leading slash +# must not be preceded by a word character (which would make it a relative +# ``a/b`` fragment) or another slash or dot (which would make it part of a URL +# authority such as ``https://host/path``, a ``//host/path`` reference, or a +# ``../path`` traversal fragment). A match is not taken when it is immediately +# followed by a word character, ``:`` method suffix, ``/`` or ``{`` route +# template, so API route tokens such as ``/message:send`` or +# ``/tasks/{id}:cancel`` are preserved rather than mistaken for local paths. +_POSIX_ABSOLUTE_PATH = re.compile("(? str: + """Return ``text`` with absolute filesystem paths replaced by a placeholder. + + Both POSIX (``/a/b``) and Windows (``C:\\a\\b``, ``\\\\server\\share``) + absolute paths are replaced. URLs (``scheme://host/path``), relative paths + (``a/b``), and prose without absolute paths are left unchanged. ``file://`` + URIs and Windows paths are processed before POSIX paths so a + drive-qualified local path is replaced as one unit rather than leaving the + ``file://C:`` or ``C:`` prefix behind. + """ + redacted = _FILE_URL_PATH.sub(REDACTED_PATH_PLACEHOLDER, text) + redacted = _WINDOWS_ABSOLUTE_PATH.sub(REDACTED_PATH_PLACEHOLDER, redacted) + return _POSIX_ABSOLUTE_PATH.sub(REDACTED_PATH_PLACEHOLDER, redacted) + + +def redact_paths_in_value(value: Any) -> Any: + """Recursively redact absolute paths in every string leaf of ``value``. + + ``value`` is expected to be JSON-compatible (dict/list/tuple/str/scalar). + Keys are kept as-is so error metadata remains machine-readable. + """ + if isinstance(value, str): + return redact_absolute_paths(value) + if isinstance(value, list): + return [redact_paths_in_value(item) for item in value] + if isinstance(value, tuple): + return tuple(redact_paths_in_value(item) for item in value) + if isinstance(value, dict): + return {str(key): redact_paths_in_value(item) for key, item in value.items()} + return value diff --git a/tests/execution/test_error_redaction.py b/tests/execution/test_error_redaction.py new file mode 100644 index 0000000..0a73f8d --- /dev/null +++ b/tests/execution/test_error_redaction.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock + +import pytest +from a2a.server.events.event_queue import EventQueue +from a2a.types import TaskArtifactUpdateEvent, TaskState + +from opencode_a2a.execution.executor import OpencodeAgentExecutor +from opencode_a2a.redact import REDACTED_PATH_PLACEHOLDER + + +def _part_text(part) -> str: # noqa: ANN001 + return getattr(part, "text", None) or getattr(getattr(part, "root", None), "text", "") + + +@pytest.mark.asyncio +async def test_emit_error_redacts_absolute_paths() -> None: + client = MagicMock() + executor = OpencodeAgentExecutor(client, streaming_enabled=False) + event_queue = AsyncMock(spec=EventQueue) + + await executor._emit_error( + event_queue, + task_id="task-1", + context_id="context-1", + message="Cannot open session file '/home/ubuntu/sessions/session-1.json': No such file", + state=TaskState.TASK_STATE_FAILED, + streaming_request=False, + ) + + task = event_queue.enqueue_event.call_args[0][0] + text = _part_text(task.status.message.parts[0]) + assert REDACTED_PATH_PLACEHOLDER in text + assert "/home/ubuntu/sessions/session-1.json" not in text + + +@pytest.mark.asyncio +async def test_emit_error_redacts_paths_in_streaming_artifact() -> None: + client = MagicMock() + executor = OpencodeAgentExecutor(client, streaming_enabled=True) + event_queue = AsyncMock(spec=EventQueue) + + await executor._emit_error( + event_queue, + task_id="task-2", + context_id="context-2", + message="Timeout writing to /var/log/opencode/app.log", + state=TaskState.TASK_STATE_FAILED, + streaming_request=True, + ) + + events = [call.args[0] for call in event_queue.enqueue_event.call_args_list] + artifact_event = next(event for event in events if isinstance(event, TaskArtifactUpdateEvent)) + text = _part_text(artifact_event.artifact.parts[0]) + assert REDACTED_PATH_PLACEHOLDER in text + assert "/var/log/opencode/app.log" not in text diff --git a/tests/jsonrpc/test_error_redaction.py b/tests/jsonrpc/test_error_redaction.py new file mode 100644 index 0000000..4b863dd --- /dev/null +++ b/tests/jsonrpc/test_error_redaction.py @@ -0,0 +1,116 @@ +from __future__ import annotations + +import json + +from a2a.types import InvalidParamsError + +from opencode_a2a.jsonrpc.error_responses import ( + adapt_jsonrpc_error, + build_http_error_body, + session_not_found_error, + upstream_payload_error, +) +from opencode_a2a.jsonrpc.models import JSONRPCError +from opencode_a2a.redact import REDACTED_PATH_PLACEHOLDER +from opencode_a2a.server.application import create_app +from tests.support.settings import make_settings + + +def test_adapt_jsonrpc_error_redacts_message_and_metadata() -> None: + error = JSONRPCError( + code=-32001, + message="Session file '/home/ubuntu/sessions/s1.json' missing", + data={ + "type": "SESSION_NOT_FOUND", + "path": "/home/ubuntu/sessions/s1.json", + "nested": {"location": r"C:\Users\alice\x"}, + }, + ) + + adapted = adapt_jsonrpc_error(error) + + assert adapted.code == -32001 + assert REDACTED_PATH_PLACEHOLDER in adapted.message + assert "/home/ubuntu/sessions/s1.json" not in adapted.message + dumped = json.dumps(adapted.data) + assert REDACTED_PATH_PLACEHOLDER in dumped + assert "/home/ubuntu/sessions/s1.json" not in dumped + assert r"C:\Users\alice\x" not in dumped + + +def test_adapt_jsonrpc_error_redacts_data_for_standard_codes() -> None: + error = InvalidParamsError( + message="Invalid params", + data={"field": "directory", "value": "/home/ubuntu/project"}, + ) + + adapted = adapt_jsonrpc_error(error) + + assert adapted.code == -32602 + dumped = json.dumps(adapted.data) + assert REDACTED_PATH_PLACEHOLDER in dumped + assert "/home/ubuntu/project" not in dumped + + +def test_build_http_error_body_redacts_message_and_metadata() -> None: + body = build_http_error_body( + status_code=500, + status="INTERNAL", + message="Unhandled path /opt/opencode/bin/tool", + metadata={"directory": "/opt/opencode/bin"}, + ) + + payload = body["error"] + assert payload["message"] == f"Unhandled path {REDACTED_PATH_PLACEHOLDER}" + dumped = json.dumps(payload["details"]) + assert REDACTED_PATH_PLACEHOLDER in dumped + assert "/opt/opencode/bin" not in dumped + + +def test_generate_error_response_redacts_raw_exception_text() -> None: + app = create_app(make_settings()) + jsonrpc_app = app.state._jsonrpc_app + + response = jsonrpc_app._generate_error_response("1", ValueError("broken at /home/ubuntu/x")) + + body = response.body.decode("utf-8") + assert REDACTED_PATH_PLACEHOLDER in body + assert "/home/ubuntu/x" not in body + + +def test_generate_error_response_serializes_opencode_jsonrpc_errors() -> None: + app = create_app(make_settings()) + jsonrpc_app = app.state._jsonrpc_app + + response = jsonrpc_app._generate_error_response( + "1", + session_not_found_error(-32001, session_id="s-404"), + ) + + payload = json.loads(response.body.decode("utf-8")) + error = payload["error"] + assert error["code"] == -32001 + assert error["message"] == "Session not found" + assert error["data"][0]["reason"] == "SESSION_NOT_FOUND" + assert "-32603" not in payload["error"]["message"] + + +def test_generate_error_response_redacts_and_serializes_upstream_payload_error() -> None: + app = create_app(make_settings()) + jsonrpc_app = app.state._jsonrpc_app + + response = jsonrpc_app._generate_error_response( + "2", + upstream_payload_error( + -32005, + detail="failed at /home/ubuntu/sessions/s1.json", + method="list_sessions", + ), + ) + + payload = json.loads(response.body.decode("utf-8")) + error = payload["error"] + assert error["code"] == -32005 + dumped = json.dumps(error) + assert REDACTED_PATH_PLACEHOLDER in dumped + assert "/home/ubuntu/sessions/s1.json" not in dumped diff --git a/tests/test_redact.py b/tests/test_redact.py new file mode 100644 index 0000000..1769391 --- /dev/null +++ b/tests/test_redact.py @@ -0,0 +1,89 @@ +from __future__ import annotations + +from opencode_a2a.redact import ( + REDACTED_PATH_PLACEHOLDER, + redact_absolute_paths, + redact_paths_in_value, +) + + +def test_masks_posix_absolute_paths() -> None: + text = "FileNotFoundError: [Errno 2] No such file or directory: '/home/ubuntu/secret/data.txt'" + assert redact_absolute_paths(text) == ( + f"FileNotFoundError: [Errno 2] No such file or directory: '{REDACTED_PATH_PLACEHOLDER}'" + ) + + +def test_masks_multiple_paths() -> None: + text = "paths /tmp/a and /var/log/opencode/app.log" + assert redact_absolute_paths(text) == ( + f"paths {REDACTED_PATH_PLACEHOLDER} and {REDACTED_PATH_PLACEHOLDER}" + ) + + +def test_masks_windows_drive_paths() -> None: + assert redact_absolute_paths(r"C:\Users\alice\secret.txt") == REDACTED_PATH_PLACEHOLDER + assert redact_absolute_paths("C:/Users/alice/secret.txt") == REDACTED_PATH_PLACEHOLDER + + +def test_masks_unc_paths() -> None: + assert redact_absolute_paths(r"\\server\share\data\file.txt") == REDACTED_PATH_PLACEHOLDER + + +def test_masks_file_url_paths() -> None: + assert redact_absolute_paths("file:///tmp/secret/config.json") == REDACTED_PATH_PLACEHOLDER + assert redact_absolute_paths("file://C:/tmp/secret.txt") == REDACTED_PATH_PLACEHOLDER + + +def test_preserves_remote_urls() -> None: + text = "upstream https://example.com/path/to/file failed; local http://localhost:8000/a2a" + assert redact_absolute_paths(text) == text + + +def test_preserves_relative_paths() -> None: + text = "module src/opencode_a2a/redact.py and ./local and ../parent/file" + assert redact_absolute_paths(text) == text + + +def test_preserves_api_route_tokens() -> None: + text = "REST route /message:send failed; try /tasks/{id}:cancel or /message:stream" + assert redact_absolute_paths(text) == text + + +def test_preserves_trailing_punctuation() -> None: + assert redact_absolute_paths("/tmp/x.") == f"{REDACTED_PATH_PLACEHOLDER}." + assert redact_absolute_paths("/tmp/x,") == f"{REDACTED_PATH_PLACEHOLDER}," + + +def test_redaction_is_idempotent() -> None: + text = "error /home/u/x with https://example.com/a/b and C:\\Users\\a\\y" + once = redact_absolute_paths(text) + assert redact_absolute_paths(once) == once + + +def test_plain_text_unchanged() -> None: + text = "hello world; method=send_message; code=-32001" + assert redact_absolute_paths(text) == text + + +def test_redact_paths_in_value_recurses() -> None: + value = { + "directory": "/home/ubuntu/project", + "nested": [ + {"path": r"C:\Users\alice\x"}, + {"url": "https://example.com/a"}, + ], + "count": 3, + "flag": True, + } + result = redact_paths_in_value(value) + assert result["directory"] == REDACTED_PATH_PLACEHOLDER + assert result["nested"][0]["path"] == REDACTED_PATH_PLACEHOLDER + assert result["nested"][1]["url"] == "https://example.com/a" + assert result["count"] == 3 + assert result["flag"] is True + + +def test_redact_paths_in_value_handles_tuples() -> None: + result = redact_paths_in_value(("ok", "/tmp/x", ["/var/log/y"])) + assert result == ("ok", REDACTED_PATH_PLACEHOLDER, [REDACTED_PATH_PLACEHOLDER])