feat(mcp): wire trusted-execution gate and function-layer guards - #1082
Conversation
Co-Authored-By: AJ Steers <aj@airbyte.io>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1784595371-mcp-trusted-execution' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1784595371-mcp-trusted-execution'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
…ation Co-Authored-By: AJ Steers <aj@airbyte.io>
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an environment-controlled trusted-execution gate for MCP secret resolution and local tools, blocks trusted execution over HTTP, validates Airbyte domains at startup, and adds exceptions plus unit tests for the new behavior. ChangesMCP trusted execution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPServer
participant TrustedExecutionGuard
participant LocalMCPTool
participant SecretResolver
MCPClient->>MCPServer: Invoke MCP operation
MCPServer->>TrustedExecutionGuard: Check trusted execution
TrustedExecutionGuard-->>MCPServer: Allow or raise trusted-execution error
MCPServer->>LocalMCPTool: Execute protected operation
LocalMCPTool->>SecretResolver: Resolve protected configuration or secrets
SecretResolver-->>LocalMCPTool: Return resolved values
LocalMCPTool-->>MCPClient: Return operation result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code Coverage OverviewLanguages: Python Python / code-coverage/pytest-fastThe overall coverage in commit ffadce2 in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytest-no-credsThe overall coverage in commit ffadce2 in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytestThe overall coverage in commit ffadce2 in the Show a code coverage summary of the most impacted files.
Updated |
Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
airbyte/exceptions.py (1)
229-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCould we update the stale
PyAirbytewording?
AirbyteMCPErrorwas renamed to remove thePyAirbyteprefix, but its docstring still says “PyAirbyte MCP server.” Updating this to “Airbyte MCP server” would keep the public exception documentation consistent, wdyt?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@airbyte/exceptions.py` around lines 229 - 230, Update the docstring of AirbyteMCPError to say “Airbyte MCP server” instead of “PyAirbyte MCP server,” keeping the exception’s public documentation consistent with its current name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@airbyte/exceptions.py`:
- Around line 229-230: Update the docstring of AirbyteMCPError to say “Airbyte
MCP server” instead of “PyAirbyte MCP server,” keeping the exception’s public
documentation consistent with its current name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c581438b-f104-469c-955f-6f7a35942df6
📒 Files selected for processing (3)
airbyte/exceptions.pyairbyte/mcp/_guards.pytests/unit_tests/test_mcp_trusted_execution.py
🚧 Files skipped from review as they are similar to previous changes (2)
- airbyte/mcp/_guards.py
- tests/unit_tests/test_mcp_trusted_execution.py
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
349726d
into
main
Summary
Wires the Airbyte MCP server into the generic secure-by-default trusted-execution gate landed in
fastmcp-extensionsv0.10.3 (airbytehq/fastmcp-extensions#71), and adds an independent function-layer of enforcement so trusted-machine capabilities can never be reached by an untrusted (e.g. hosted HTTP) caller — even if a future registration mistake left a tool visible.This is PR2 of the MCP security hardening effort. It maps to remediations R1 (trusted-execution gate), R3 (server-side secret suppression / refusal), and the domain-config part of R6 for the public
cloud-mcpthreat model. No infrastructure (Cloud Run / Pulumi), OAuth-client, or network-egress changes are in scope here.Two independent layers of defense
trusted_executionconfig arg letsfastmcp-extensionshiderequires_client_filesystemtools when the gate is off.require_trusted_execution(...)themselves and hard-fail when the gate is off, independently of tool visibility.Because the layers are independent, a mistake in either one alone cannot expose a trusted-machine capability.
What changed
Generic gate mapped to the Airbyte env var. New
TRUSTED_EXECUTION_CONFIG_ARGmaps the generictrusted_executionconfignametoAIRBYTE_MCP_TRUSTED_EXECUTION(default"0"), keeping the generic library Airbyte-agnostic. Registered inserver.py. It has nohttp_header_key— the gate widens the surface, so it must never be caller-controllable.Permanent HTTP prohibition.
http_main.main()callsassert_http_trusted_execution_disabled(app)immediately beforeapp.run(...), hard-failing startup if trusted execution was explicitly enabled on the hosted entrypoint.Reusable function-layer guard (
airbyte/mcp/_guards.py):Raises
PyAirbyteTrustedExecutionRequiredError(new, under a newPyAirbyteMCPErrorbase) with remediation inguidance, not in the deterministic message.Guarded backend helpers. Guards were added so a direct call hard-fails when untrusted:
_arg_resolvers.resolve_connector_config— gates only the dangerous branches (config_file,config_secret_name, and inlinesecret_reference::values). An ordinary inlineconfigdict is still allowed untrusted, becausecloud.pylegitimately resolves caller-provided config that way.local._get_mcp_source(connector install/execution) and the local cache / secret-inventory / destructive tools:list_cached_streams,describe_default_cache,run_sql_query,list_dotenv_secrets,list_connector_config_secrets,destination_smoke_test.Secret-manager suppression when untrusted (
_config.load_secrets_to_env_vars). Dotenv and Google Secret Manager backends are no longer registered as secret sources when trusted execution is off, soconfig_secret_name/secret_reference::cannot resolve server-side secrets. Ordinary environment variables are still loaded from any referenced dotenv file so safe, non-secret-manager config keeps working.Domain-config hard-fail (
_tool_utils.validate_airbyte_domains, called after tool registration inserver.py). Instead of silently dropping a requested domain, it hard-fails on (a) setting bothAIRBYTE_MCP_DOMAINSandAIRBYTE_MCP_DOMAINS_DISABLED, and (b) naming a domain with no registered tools. Remediation names the setting to clear and says to restart.Test plan
New
tests/unit_tests/test_mcp_trusted_execution.py(31 cases), covering:require_trusted_executionraise-when-off / pass-when-on;resolve_connector_config: inline config allowed untrusted;config_file,config_secret_name, and inlinesecret_reference::rejected untrusted;config_file+secret_reference::allowed when trusted;localhelpers when untrusted;assert_http_trusted_execution_disabledraises when trusted is on, passes when off;validate_airbyte_domainsinclude+exclude conflict, unknown-domain, and valid-single-domain paths.Local verification (uv):
Limitations / out of scope
fastmcp-extensionsread API is used as-is; no new raising helper was added there (per maintainer direction).CONTRIBUTING.mdguidance (R8) is a separate follow-up PR.https://app.devin.ai/sessions/1696a2c620134d61a5255f959ef4c4ef
Requested by: Aaron ("AJ") Steers (@aaronsteers)
Summary by CodeRabbit