diff --git a/airbyte/mcp/_arg_resolvers.py b/airbyte/mcp/_arg_resolvers.py index 52f600937..a1d822f8f 100644 --- a/airbyte/mcp/_arg_resolvers.py +++ b/airbyte/mcp/_arg_resolvers.py @@ -16,7 +16,6 @@ from airbyte.constants import SECRETS_HYDRATION_PREFIX from airbyte.exceptions import AirbyteLibInputError from airbyte.mcp._guards import ( - is_trusted_execution_enabled, raise_if_untrusted_execution_context, ) from airbyte.secrets.hydration import deep_update, detect_hardcoded_secrets @@ -92,28 +91,24 @@ def resolve_list_of_strings(value: str | list[str] | set[str] | None) -> list[st return [item.strip() for item in value.split(",") if item.strip()] -def resolve_manifest_yaml(manifest_yaml: str | Path | None) -> str | Path | None: - """Resolve inline YAML or gate local manifest paths behind trusted execution. - - A local manifest path is a trusted-machine input and is rejected when trusted execution is - disabled. - """ - if manifest_yaml is None: - return None +def resolve_manifest_yaml( + *, + manifest_yaml_text: str | None, + manifest_yaml_file: str | None, +) -> str | Path | None: + """Resolve manifest text or gate an explicit local manifest path behind trusted execution.""" + if manifest_yaml_text is not None and manifest_yaml_file is not None: + raise AirbyteLibInputError( + message="Provide only one of `manifest_yaml_text` or `manifest_yaml_file`." + ) - if isinstance(manifest_yaml, Path): + if manifest_yaml_file is not None: raise_if_untrusted_execution_context( - "Reading a connector manifest from a local file (`manifest_yaml` path)" + "Reading a connector manifest from a local file (`manifest_yaml_file`)" ) - return manifest_yaml - - if "\n" in manifest_yaml: - return manifest_yaml - - if is_trusted_execution_enabled(): - return Path(manifest_yaml) + return Path(manifest_yaml_file) - return manifest_yaml + return manifest_yaml_text def resolve_connector_config( # noqa: PLR0912 diff --git a/airbyte/mcp/_tool_utils.py b/airbyte/mcp/_tool_utils.py index 2473e0e92..9c950a087 100644 --- a/airbyte/mcp/_tool_utils.py +++ b/airbyte/mcp/_tool_utils.py @@ -453,7 +453,7 @@ def mcp_tool( # noqa: PLR0913 - mirrors the upstream decorator's kwargs meta: Mapping[str, object] | None = None, app: AppConfig | None = None, required_capabilities: Iterable[Capability | str] | None = None, - extra_help_text: str | None = None, + extra_help_text: str | Callable[[], str | None] | None = None, pipeline_change: bool | None = None, external_access: bool = False, tracing: bool | Callable[[Mapping[str, object]], Mapping[str, object]] = True, diff --git a/airbyte/mcp/cloud.py b/airbyte/mcp/cloud.py index e9bfb4ffd..f760ddbe9 100644 --- a/airbyte/mcp/cloud.py +++ b/airbyte/mcp/cloud.py @@ -12,7 +12,6 @@ from collections.abc import Callable from http import HTTPStatus -from pathlib import Path from typing import TYPE_CHECKING, Annotated, Any, Final, Literal, TypeVar, cast import requests @@ -88,6 +87,7 @@ render_connector_docs_result, ) from airbyte.mcp._error_handling import format_user_facing_error, redact_agent_text +from airbyte.mcp._guards import is_trusted_execution_enabled from airbyte.mcp._scope import record_default_workspace, record_resolved_organization from airbyte.mcp._tool_utils import ( AIRBYTE_CLOUD_WORKSPACE_ID_IS_SET, @@ -118,6 +118,15 @@ "rename it themselves first." ) WORKSPACE_ID_TIP_TEXT = "Optional; defaults to the session's workspace." +MANIFEST_YAML_SOURCE_HINT = ( + "Provide the manifest as either `manifest_yaml_text` or `manifest_yaml_file`, not both." +) + + +def _manifest_yaml_source_hint() -> str | None: + return MANIFEST_YAML_SOURCE_HINT if is_trusted_execution_enabled() else None + + SKILL_DOCS_SECTION_HINT = """\ Skill doc sections for direct API queries and actions are named with the pattern `actions..`, matching the `entity_type` and `action` arguments of the @@ -2969,8 +2978,8 @@ def _get_custom_source_definition_description( ) -@mcp_tool() -def publish_custom_source_definition( +@mcp_tool(extra_help_text=_manifest_yaml_source_hint) +def publish_custom_source_definition( # noqa: PLR0913 # Explicit args mirror MCP tool schema. ctx: Context, name: Annotated[ str, @@ -2986,13 +2995,21 @@ def publish_custom_source_definition( ), TraceArg.OMIT, ], - manifest_yaml: Annotated[ - str | Path | None, + manifest_yaml_text: Annotated[ + str | None, + Field( + description=( + "Full YAML contents of the declarative (low-code) connector manifest: the " + "file's text, not a filename or path." + ), + default=None, + ), + ] = None, + manifest_yaml_file: Annotated[ + str | None, Field( description=( - "The Low-code CDK manifest as inline YAML or a file path. File paths are " - "honored only when trusted execution is enabled; otherwise, the value is " - "treated as inline YAML. Required for YAML connectors." + "Absolute path to a manifest YAML file on the machine running this MCP server." ), default=None, ), @@ -3044,7 +3061,20 @@ def publish_custom_source_definition( Note: Only YAML (declarative) connectors are currently supported. Docker-based custom sources are not yet available. """ - processed_manifest = resolve_manifest_yaml(manifest_yaml) + processed_manifest = resolve_manifest_yaml( + manifest_yaml_text=manifest_yaml_text, + manifest_yaml_file=manifest_yaml_file, + ) + if processed_manifest is None: + error_message = ( + "Provide the manifest in `manifest_yaml_text` (full YAML contents) or " + "`manifest_yaml_file` (local path)." + if is_trusted_execution_enabled() + else "Provide the manifest's full YAML contents in `manifest_yaml_text`." + ) + raise AirbyteLibInputError( + message=error_message, + ) # Resolve testing values from inline config and/or secret testing_values_dict: dict[str, Any] | None = None @@ -3220,27 +3250,34 @@ def get_connector_builder_draft_manifest( } -@mcp_tool( - destructive=True, -) -def update_custom_source_definition( +@mcp_tool(destructive=True, extra_help_text=_manifest_yaml_source_hint) +def update_custom_source_definition( # noqa: PLR0913 # Explicit args mirror MCP tool schema. ctx: Context, definition_id: Annotated[ str, Field(description="The ID of the definition to update."), ], - manifest_yaml: Annotated[ - str | Path | None, + *, + manifest_yaml_text: Annotated[ + str | None, Field( description=( - "New manifest as inline YAML or a file path. File paths are honored only when " - "trusted execution is enabled; otherwise, the value is treated as inline YAML. " - "Optional; omit to update only testing values." + "Optional. New manifest. Full YAML contents of the declarative (low-code) " + "connector manifest: the file's text, not a filename or path." + ), + default=None, + ), + ] = None, + manifest_yaml_file: Annotated[ + str | None, + Field( + description=( + "Optional. Absolute path to a manifest YAML file on the machine running this " + "MCP server." ), default=None, ), ] = None, - *, workspace_id: Annotated[ str | None, Field( @@ -3300,8 +3337,7 @@ def update_custom_source_definition( Updates the definition name, manifest, and/or testing values for an existing custom source definition. Renaming also updates the Connector Builder project name. - At least one of name, manifest_yaml, testing_values, or testing_values_secret_name must be - provided. + Provide at least one value to update. """ check_guid_created_in_session(definition_id) @@ -3309,14 +3345,19 @@ def update_custom_source_definition( if ( name is None - and manifest_yaml is None + and manifest_yaml_text is None + and manifest_yaml_file is None and testing_values is None and testing_values_secret_name is None ): + required_args = ["name", "manifest_yaml_text"] + if is_trusted_execution_enabled(): + required_args.append("manifest_yaml_file") + required_args.extend(("testing_values", "testing_values_secret_name")) raise AirbyteLibInputError( message=( - "At least one of name, manifest_yaml, testing_values, or " - "testing_values_secret_name must be provided to update a custom source definition." + f"At least one of {', '.join(required_args)} must be provided to update " + "a custom source definition." ), context={ "definition_id": definition_id, @@ -3324,7 +3365,10 @@ def update_custom_source_definition( }, ) - processed_manifest = resolve_manifest_yaml(manifest_yaml) + processed_manifest = resolve_manifest_yaml( + manifest_yaml_text=manifest_yaml_text, + manifest_yaml_file=manifest_yaml_file, + ) # Resolve testing values from inline config and/or secret testing_values_dict: dict[str, Any] | None = None @@ -4035,11 +4079,13 @@ def register_cloud_tools(app: FastMCP) -> None: Args: app: FastMCP application instance """ - exclude_args = ["workspace_id"] if AIRBYTE_CLOUD_WORKSPACE_ID_IS_SET else None + exclude_args = ["workspace_id"] if AIRBYTE_CLOUD_WORKSPACE_ID_IS_SET else [] + if not is_trusted_execution_enabled(): + exclude_args.append("manifest_yaml_file") if exclude_args: _add_defaults_for_exclude_args(exclude_args) register_mcp_tools( app, mcp_module=__name__, - exclude_args=exclude_args, + exclude_args=exclude_args or None, ) diff --git a/pyproject.toml b/pyproject.toml index 1573fa58f..07f3c5cb5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,7 +46,7 @@ dependencies = [ "typing-extensions", "uuid7>=0.1.0,<1.0", "fastmcp>=4.0.9,<5.0", - "fastmcp-extensions>=0.34.2,<1.0.0", + "fastmcp-extensions>=0.35.0,<1.0.0", "starlette", # Transitive dependency of fastmcp, imported directly "httpx", # Transitive dependency of fastmcp, imported directly "mcp", # Transitive dependency of fastmcp, imported directly diff --git a/tests/unit_tests/fixtures/arg_trace_plan.json b/tests/unit_tests/fixtures/arg_trace_plan.json index 90abfd011..5e31e3781 100644 --- a/tests/unit_tests/fixtures/arg_trace_plan.json +++ b/tests/unit_tests/fixtures/arg_trace_plan.json @@ -230,7 +230,8 @@ "workspace_id": "omit" }, "publish_custom_source_definition": { - "manifest_yaml": "presence", + "manifest_yaml_file": "hash", + "manifest_yaml_text": "hash", "name": "fingerprint", "pre_validate": "value allowed=[false,true]", "testing_values": "presence", @@ -335,7 +336,8 @@ }, "update_custom_source_definition": { "definition_id": "hash", - "manifest_yaml": "presence", + "manifest_yaml_file": "hash", + "manifest_yaml_text": "hash", "name": "fingerprint", "pre_validate": "value allowed=[false,true]", "testing_values": "presence", diff --git a/tests/unit_tests/test_mcp_arg_trace_parity.py b/tests/unit_tests/test_mcp_arg_trace_parity.py index 5302276c6..c074876bc 100644 --- a/tests/unit_tests/test_mcp_arg_trace_parity.py +++ b/tests/unit_tests/test_mcp_arg_trace_parity.py @@ -68,8 +68,20 @@ def _plan() -> dict[str, dict[str, str]]: def test_inventory_matches_fixture() -> None: plan = _plan() if os.environ.get("UPDATE_ARG_TRACE_FIXTURE"): + assert all( + "manifest_yaml_file" in plan[name] + for name in ( + "publish_custom_source_definition", + "update_custom_source_definition", + ) + ), "Enable trusted execution when regenerating the fixture." FIXTURE.write_text(json.dumps(plan, indent=2, sort_keys=True) + "\n") - assert plan == json.loads(FIXTURE.read_text()) + + expected = json.loads(FIXTURE.read_text()) + for name in ("publish_custom_source_definition", "update_custom_source_definition"): + if "manifest_yaml_file" not in plan[name]: + expected[name].pop("manifest_yaml_file", None) + assert plan == expected @pytest.fixture diff --git a/tests/unit_tests/test_mcp_trusted_execution.py b/tests/unit_tests/test_mcp_trusted_execution.py index 75ea06a5d..1dafff09a 100644 --- a/tests/unit_tests/test_mcp_trusted_execution.py +++ b/tests/unit_tests/test_mcp_trusted_execution.py @@ -3,6 +3,7 @@ from __future__ import annotations +import asyncio import json import logging import os @@ -10,12 +11,14 @@ from typing import TYPE_CHECKING, Callable import pytest +from fastmcp import Client, FastMCP + from airbyte.constants import MCP_TRUSTED_EXECUTION_ENV_VAR from airbyte.exceptions import ( - AirbyteTrustedExecutionRequiredError, AirbyteLibInputError, + AirbyteTrustedExecutionRequiredError, ) -from airbyte.mcp import local +from airbyte.mcp import cloud, local from airbyte.mcp._arg_resolvers import resolve_connector_config, resolve_manifest_yaml from airbyte.mcp._guards import ( is_trusted_execution_enabled, @@ -165,18 +168,21 @@ def test_resolve_connector_config_allows_when_trusted( assert resolve_connector_config(**kwargs) == expected -def test_resolve_manifest_yaml_rejects_path_when_untrusted( +def test_resolve_manifest_yaml_rejects_file_when_untrusted( monkeypatch: MonkeyPatch, tmp_path: Path, ) -> None: - """A `Path` manifest input requires trusted execution.""" + """A local manifest file input requires trusted execution.""" _set_trusted(monkeypatch, enabled=False) with pytest.raises(AirbyteTrustedExecutionRequiredError): - resolve_manifest_yaml(tmp_path / "manifest.yaml") + resolve_manifest_yaml( + manifest_yaml_text=None, + manifest_yaml_file=str(tmp_path / "manifest.yaml"), + ) @pytest.mark.parametrize( - "manifest_yaml", + "manifest_yaml_text", [ pytest.param("manifest.yaml", id="single-line-text"), pytest.param( @@ -185,56 +191,125 @@ def test_resolve_manifest_yaml_rejects_path_when_untrusted( pytest.param("version: 0.1.0\ntype: DeclarativeSource", id="multiline-yaml"), ], ) -def test_resolve_manifest_yaml_preserves_inline_values_when_untrusted( +@pytest.mark.parametrize("enabled", [False, True], ids=["untrusted", "trusted"]) +def test_resolve_manifest_yaml_preserves_text( monkeypatch: MonkeyPatch, - manifest_yaml: str, + manifest_yaml_text: str, + enabled: bool, ) -> None: - """Untrusted execution preserves manifest strings as inline YAML.""" - _set_trusted(monkeypatch, enabled=False) - resolved = resolve_manifest_yaml(manifest_yaml) - assert isinstance(resolved, str) - assert resolved == manifest_yaml + """Manifest text is never reinterpreted as a path.""" + _set_trusted(monkeypatch, enabled=enabled) + resolved = resolve_manifest_yaml( + manifest_yaml_text=manifest_yaml_text, + manifest_yaml_file=None, + ) + assert resolved == manifest_yaml_text -def test_resolve_manifest_yaml_preserves_existing_path_when_trusted( +def test_resolve_manifest_yaml_returns_file_path_when_trusted( monkeypatch: MonkeyPatch, tmp_path: Path, ) -> None: - """A `Path` manifest input is returned when trusted execution is enabled.""" + """A local manifest file is resolved to a `Path` when trusted execution is enabled.""" _set_trusted(monkeypatch, enabled=True) manifest_path = tmp_path / "manifest.yaml" - assert resolve_manifest_yaml(manifest_path) is manifest_path + assert ( + resolve_manifest_yaml( + manifest_yaml_text=None, + manifest_yaml_file=str(manifest_path), + ) + == manifest_path + ) -def test_resolve_manifest_yaml_converts_single_line_string_when_trusted( +def test_resolve_manifest_yaml_rejects_both_inputs( monkeypatch: MonkeyPatch, ) -> None: - """A single-line manifest string is treated as a file path when trusted.""" + """Manifest text and a local file path cannot be supplied together.""" _set_trusted(monkeypatch, enabled=True) - assert resolve_manifest_yaml("manifest.yaml") == Path("manifest.yaml") + with pytest.raises( + AirbyteLibInputError, + match="Provide only one of `manifest_yaml_text` or `manifest_yaml_file`\\.", + ): + resolve_manifest_yaml( + manifest_yaml_text="version: 0.1.0", + manifest_yaml_file="manifest.yaml", + ) @pytest.mark.parametrize("enabled", [False, True], ids=["untrusted", "trusted"]) -def test_resolve_manifest_yaml_preserves_none( +def test_resolve_manifest_yaml_returns_none_when_inputs_omitted( monkeypatch: MonkeyPatch, enabled: bool, ) -> None: """A missing manifest stays missing in either execution mode.""" _set_trusted(monkeypatch, enabled=enabled) - assert resolve_manifest_yaml(None) is None + assert ( + resolve_manifest_yaml( + manifest_yaml_text=None, + manifest_yaml_file=None, + ) + is None + ) -def test_resolve_manifest_yaml_does_not_read_file_when_untrusted( +def test_manifest_file_schema_depends_on_trusted_execution( monkeypatch: MonkeyPatch, - tmp_path: Path, ) -> None: - """A path string remains inline YAML when trusted execution is disabled.""" - _set_trusted(monkeypatch, enabled=False) - manifest_path = tmp_path / "manifest.yaml" - manifest_path.write_text("version: 0.1.0\ntype: DeclarativeSource\n") - resolved = resolve_manifest_yaml(str(manifest_path)) - assert isinstance(resolved, str) - assert resolved == str(manifest_path) + """The file argument is hidden from untrusted schemas and exposed to trusted callers.""" + for enabled in (False, True): + _set_trusted(monkeypatch, enabled=enabled) + app = FastMCP(f"manifest-schema-{enabled}") + cloud.register_cloud_tools(app) + + async def list_tools() -> dict[str, object]: + async with Client(app) as client: + return {tool.name: tool for tool in await client.list_tools()} + + tools = asyncio.run(list_tools()) + + for tool_name in ( + "publish_custom_source_definition", + "update_custom_source_definition", + ): + tool = tools[tool_name] + properties = tool.input_schema["properties"] # type: ignore[attr-defined] + assert "manifest_yaml_text" in properties + assert ("manifest_yaml_file" in properties) is enabled + assert "manifest_yaml" not in properties + assert ( + cloud.MANIFEST_YAML_SOURCE_HINT + in (getattr(tool, "description", None) or "") + ) is enabled + if enabled: + assert ( + "manifest_yaml_text" + not in properties["manifest_yaml_file"]["description"] + ) + + +@pytest.mark.parametrize("enabled", [False, True], ids=["untrusted", "trusted"]) +def test_publish_missing_manifest_error_depends_on_trusted_execution( + monkeypatch: MonkeyPatch, + enabled: bool, +) -> None: + """The missing-manifest error only mentions the file argument when it is available.""" + _set_trusted(monkeypatch, enabled=enabled) + app = FastMCP(f"manifest-error-{enabled}") + cloud.register_cloud_tools(app) + + async def call_publish() -> object: + async with Client(app) as client: + return await client.call_tool( + "publish_custom_source_definition", + {"name": "x"}, + raise_on_error=False, + ) + + result = asyncio.run(call_publish()) + assert result.is_error # type: ignore[attr-defined] + error_text = result.content[0].text # type: ignore[attr-defined] + assert ("manifest_yaml_file" in error_text) is enabled _UNTRUSTED_LOCAL_HELPERS: list[Callable[[], object]] = [ diff --git a/uv.lock b/uv.lock index 6a8507054..27b1dbfbe 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 3 +revision = 5 requires-python = ">=3.11, <3.15" resolution-markers = [ "python_full_version >= '3.14'", @@ -248,7 +248,7 @@ requires-dist = [ { name = "duckdb", specifier = "==1.4.3" }, { name = "duckdb-engine", specifier = "==0.17.0" }, { name = "fastmcp", specifier = ">=4.0.9,<5.0" }, - { name = "fastmcp-extensions", specifier = ">=0.34.2,<1.0.0" }, + { name = "fastmcp-extensions", specifier = ">=0.35.0,<1.0.0" }, { name = "google-auth", specifier = ">=2.27.0,<3.0" }, { name = "google-cloud-bigquery", specifier = ">=3.12.0,<4.0" }, { name = "google-cloud-bigquery-storage", specifier = ">=2.25.0,<3.0" }, @@ -1352,7 +1352,7 @@ wheels = [ [[package]] name = "fastmcp-extensions" -version = "0.34.2" +version = "0.35.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "fastmcp" }, @@ -1367,9 +1367,9 @@ dependencies = [ { name = "starlette" }, { name = "uvicorn" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/ec/6d/379330f660ba2a4ed9fa7a6b0fe8c467f905469faa6aa26b6c55ca0a603a/fastmcp_extensions-0.34.2.tar.gz", hash = "sha256:b724514f60b65c3e73f0b4b6aaf2fc5b555bf30cd18f27bb6e8994497725e133", size = 356897, upload-time = "2026-10-10T06:22:20.759Z" } +sdist = { url = "https://files.pythonhosted.org/packages/74/45/dcf55ab5ba27c238654c71d92af320eec2312ae3fad62572b78ea0b9f8fc/fastmcp_extensions-0.35.0.tar.gz", hash = "sha256:bad04d428433c424407277928bc44d77e356f9c2ff73b42aa9e058ad54173038", size = 357967, upload-time = "2026-10-14T05:17:25.382Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/92/77/5cc2e6de8ab239969811cdb1be44a59359c879e7494f8dcf71b037d04e4e/fastmcp_extensions-0.34.2-py3-none-any.whl", hash = "sha256:8628542fb17ea016519ae755c6aaa429b62ee042929009fc7d961cef3a0a7cbd", size = 156076, upload-time = "2026-10-10T06:22:19.108Z" }, + { url = "https://files.pythonhosted.org/packages/1d/5f/3836a5aea53632be7a7629b7514003138377cfc5e37958308839cb627d7e/fastmcp_extensions-0.35.0-py3-none-any.whl", hash = "sha256:86d31337876faec30dcc336c68b7103d84f553ce8f1555540888c717ddad9e8f", size = 156435, upload-time = "2026-10-14T05:17:23.977Z" }, ] [[package]]