feat(backend/blocks): MCP message signing and tool integrity verification - #12455
Open
dataCenter430 wants to merge 12 commits into
Open
Conversation
…S signing Made-with: Cursor
Made-with: Cursor
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
Contributor
|
This PR targets the Automatically setting the base branch to |
github-actions
Bot
added
platform/backend
github-actions
Bot
requested review from
Pwuts and
Swiftyos
and removed request for
a team
March 17, 2026 12:56
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds per-tool SHA-256 integrity hashes and optional MCPS message signing/verification: compute and attach Changes
Sequence DiagramsequenceDiagram
participant Caller as Caller
participant Block as MCPToolBlock
participant Client as MCPClient
participant Server as MCP Server
participant Sec as MCPSecurityContext
Caller->>Block: execute(tool_name, args, integrity_hash?)
Block->>Client: _call_mcp_tool(tool_name, args, auth, tool_integrity_hash)
alt integrity_hash provided
Client->>Client: list_tools() / fetch metadata
Client->>Sec: verify_incoming(tool_list) (if security_ctx)
Sec->>Client: verified tool list
Client->>Client: compute_tool_hash(tool) compare expected_hash
alt mismatch or missing
Client-->>Block: raise MCPToolIntegrityError
Block->>Caller: return integrity error
else match
Client->>Server: call_tool (signed if security_ctx)
Server-->>Client: response (signed if MCPS)
Client->>Sec: verify_incoming(response)
Client-->>Block: execution result
Block-->>Caller: result
end
else no hash
Client->>Server: call_tool (signed or plain)
Server-->>Client: response
Client-->>Block: result
Block-->>Caller: result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/blocks/mcp/security.py (1)
128-156: Clarify the public key fallback behavior inverify_incoming.At line 149, when
server_public_keyisNone, the method falls back toself.public_key(the agent's own key). This would only be meaningful if the agent signed the message itself, which wouldn't apply to server responses.This fallback triggers when a server sends an MCPS-signed response but the client doesn't have the server's public key (e.g., server didn't advertise it in capabilities). Consider:
- Raising an explicit error instead of using the agent's key as fallback
- Adding a docstring note explaining when this fallback is valid (if ever)
♻️ Option: Explicit error for missing server key
- key = server_public_key or self.public_key + if server_public_key is None: + raise MCPSignatureError( + "Cannot verify MCPS-signed response: server did not advertise its public key. " + "Ensure the server includes _mcps.public_key in its capabilities." + ) + key = server_public_key result = verify_message(response, key)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/blocks/mcp/security.py` around lines 128 - 156, The verify_incoming method currently falls back to self.public_key when server_public_key is None, which is inappropriate for verifying server-signed responses; change verify_incoming to raise a clear error (e.g., ValueError or MCPSignatureError) when server_public_key is not provided instead of using self.public_key, and update the verify_incoming docstring to note that server_public_key must be supplied for server-signed responses (the self.public_key fallback is not valid for responses from other parties).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/pyproject.toml`:
- Line 41: pyproject.toml now declares the new dependency mcp-secure but
poetry.lock was not regenerated; run `poetry lock` locally (or `poetry lock
--no-update` if you want to avoid updating other packages) to regenerate
poetry.lock so the new mcp-secure entry is included, then commit the updated
poetry.lock alongside the pyproject.toml change so Docker builds and CI will
install the new dependency correctly.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/blocks/mcp/security.py`:
- Around line 128-156: The verify_incoming method currently falls back to
self.public_key when server_public_key is None, which is inappropriate for
verifying server-signed responses; change verify_incoming to raise a clear error
(e.g., ValueError or MCPSignatureError) when server_public_key is not provided
instead of using self.public_key, and update the verify_incoming docstring to
note that server_public_key must be supplied for server-signed responses (the
self.public_key fallback is not valid for responses from other parties).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8503018-2c48-4c91-a4b3-dddf5291185e
📒 Files selected for processing (6)
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/security.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/pyproject.toml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: types
- GitHub Check: Seer Code Review
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (python)
- GitHub Check: Check PR Status
- GitHub Check: conflicts
🧰 Additional context used
📓 Path-based instructions (8)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
autogpt_platform/backend/backend/api/features/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update routes in '/backend/backend/api/features/' and add/update Pydantic models in the same directory for API development
When modifying API routes, update corresponding Pydantic models in the same directory and write tests alongside the route file
Files:
autogpt_platform/backend/backend/api/features/mcp/routes.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
autogpt_platform/backend/backend/api/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/backend/api/**/*.py: Use FastAPI for building REST and WebSocket endpoints
Use JWT-based authentication with Supabase integration
Files:
autogpt_platform/backend/backend/api/features/mcp/routes.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
autogpt_platform/backend/backend/blocks/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/backend/blocks/**/*.py: Inherit from 'Block' base class with input/output schemas when adding new blocks in backend
Implement 'run' method with proper error handling in backend blocks
Generate block UUID using 'uuid.uuid4()' when creating new blocks in backend
Write tests alongside block implementation when adding new blocks in backend
Files:
autogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
autogpt_platform/backend/**/*test*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run
poetry run testfor backend testing (runs pytest with docker based postgres + prisma)
Files:
autogpt_platform/backend/backend/blocks/mcp/test_security.py
🧠 Learnings (22)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
📚 Learning: 2026-02-27T10:45:55.700Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Applied to files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-03-01T07:59:02.311Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
Applied to files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/pyproject.tomlautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Applied to files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/pyproject.tomlautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Applied to files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/pyproject.tomlautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/pyproject.tomlautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/api/features/mcp/routes.pyautogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/**/*.py : Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Applied to files:
autogpt_platform/backend/pyproject.toml
📚 Learning: 2026-02-04T16:50:20.508Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-02-04T16:50:20.508Z
Learning: Applies to autogpt_platform/backend/**/*.{py,txt} : Use `poetry run` prefix for all Python commands, including testing, linting, formatting, and migrations
Applied to files:
autogpt_platform/backend/pyproject.toml
📚 Learning: 2026-01-31T18:44:56.328Z
Learnt from: Bentlybro
Repo: Significant-Gravitas/AutoGPT PR: 0
File: :0-0
Timestamp: 2026-01-31T18:44:56.328Z
Learning: In the AutoGPT backend Docker build with Poetry path dependencies, `autogpt_libs` requires a double-copy pattern: first COPY from builder brings the installed package state (so venv references work), then second COPY from build context overwrites with latest source (to ensure freshness). Both copies are necessary for correct functionality.
Applied to files:
autogpt_platform/backend/pyproject.toml
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/schema.prisma : Run database migrations with 'poetry run prisma migrate dev' and 'poetry run prisma generate' after schema changes in backend
Applied to files:
autogpt_platform/backend/pyproject.toml
📚 Learning: 2026-02-04T16:50:20.508Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-02-04T16:50:20.508Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Always review snapshot changes with `git diff` before committing when updating snapshots with `poetry run pytest --snapshot-update`
Applied to files:
autogpt_platform/backend/pyproject.toml
📚 Learning: 2026-01-23T19:58:10.520Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 11796
File: autogpt_platform/backend/backend/blocks/video/loop.py:80-87
Timestamp: 2026-01-23T19:58:10.520Z
Learning: Ensure MoviePy is constrained to version ^2.1.2 (2.x) in pyproject.toml files where MoviePy is declared, so the backend video processing relies on a compatible API. This should cover all relevant pyproject.toml files (e.g., autogpt_platform/backend/pyproject.toml) to maintain consistency.
Applied to files:
autogpt_platform/backend/pyproject.toml
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/backend/blocks/**/*.py : Write tests alongside block implementation when adding new blocks in backend
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/test_security.py
📚 Learning: 2026-02-05T04:11:00.596Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 11796
File: autogpt_platform/backend/backend/blocks/video/concat.py:3-4
Timestamp: 2026-02-05T04:11:00.596Z
Learning: In autogpt_platform/backend/backend/blocks/**/*.py, when creating a new block, generate a UUID once with uuid.uuid4() and hard-code the resulting string as the block's id parameter. Do not call uuid.uuid4() at runtime; IDs must be constant across all imports and runs to ensure stability.
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-03-16T16:32:21.686Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:32:21.686Z
Learning: In autogpt_platform/backend/backend/blocks/, the Block base class execute() already wraps run() in a try/except to convert uncaught exceptions into BlockExecutionError/BlockUnknownError. Do not add per-block try/except in individual block run() methods, as this is not the established pattern (e.g., Gmail, Slack, Todoist blocks omit it). Only use explicit try/except within blocks that need to distinguish between success and error yield paths inside a generator (e.g., attachment blocks). This guidance applies to all Python files under autogpt_platform/backend/backend/blocks/ and similar block implementations; avoid duplicating error handling in run() unless a block requires generator-based branching.
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-03-16T16:30:11.452Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/threads.py:80-102
Timestamp: 2026-03-16T16:30:11.452Z
Learning: In autogpt_platform/backend/backend/blocks/ (and related blocks under autogpt_platform/backend/backend/blocks/), do not add try/except blocks around a block's run() method for standard error propagation. The block executor framework (backend/executor/manager.py) catches uncaught exceptions from run() and emits them on the 'error' output. Only add explicit try/except blocks when you need to control partial outputs in failure cases (e.g., certain outputs must not be yielded on error, as in attachment blocks). This is the standard pattern across the codebase; apply it broadly to blocks' run() implementations.
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-03-16T16:30:23.196Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:30:23.196Z
Learning: In any Python file under autogpt_platform/backend/backend/blocks, do not add a try/except around run() solely for standard error handling. The block framework’s _execute() in _base.py already catches unhandled exceptions and re-raises as BlockExecutionError or BlockUnknownError. If you yield ("error", message), _execute() raises BlockExecutionError immediately, so the error port will not propagate downstream. Reserve explicit try/except for scenarios where you must control partial output (e.g., attachment blocks that must skip yielding content_base64 on failure).
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-03-16T16:30:11.452Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/threads.py:80-102
Timestamp: 2026-03-16T16:30:11.452Z
Learning: Do not wrap synchronous AgentMail SDK calls with asyncio.to_thread() in blocks under autogpt_platform/backend/backend/blocks (and across the codebase). The block executor runs node execution in dedicated threads via asyncio.run_coroutine_threadsafe (see manager.py around lines ~745-752 and ~1079). The existing pattern avoids using asyncio.to_thread for SDK calls inside async run() methods, so maintain that approach and do not add to_thread usage in these code paths.
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/test_security.pyautogpt_platform/backend/backend/blocks/mcp/client.pyautogpt_platform/backend/backend/blocks/mcp/block.pyautogpt_platform/backend/backend/blocks/mcp/security.py
📚 Learning: 2026-02-04T16:50:20.508Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-02-04T16:50:20.508Z
Learning: Applies to autogpt_platform/backend/backend/blocks/*.py : When adding new blocks, analyze block interfaces to ensure inputs and outputs tie well together for productive graph-based editor connections
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/block.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/backend/blocks/**/*.py : Implement 'run' method with proper error handling in backend blocks
Applied to files:
autogpt_platform/backend/backend/blocks/mcp/block.py
🪛 GitHub Actions: Block Documentation Sync Check
autogpt_platform/backend/pyproject.toml
[error] 1-1: pyproject.toml changed significantly since poetry.lock was last generated. Run 'poetry lock' to fix the lock file.
🔇 Additional comments (10)
autogpt_platform/backend/backend/api/features/mcp/routes.py (1)
56-62: LGTM!The
integrity_hashfield is correctly added toMCPToolResponsewith a sensible default for backward compatibility. The docstring clearly documents the intended workflow for storing and verifying the hash. The propagation at line 131 correctly passes the hash from discovered tools to the API response.Also applies to: 131-131
autogpt_platform/backend/backend/blocks/mcp/client.py (3)
183-188: Verify the public key fallback behavior for response verification.When
security_ctx.verify_incoming()is called withserver_public_key=None(which happens when the server doesn't advertise MCPS), the fallback uses the agent's own public key. This seems incorrect for verifying server responses—you'd typically need the server's public key to verify its signatures.However, reviewing
security.pyline 139-140: plain JSON-RPC responses without_mcpsorsignaturekeys pass through unchanged. So this fallback only triggers when a server sends a signed response but didn't advertise its key—an edge case that may warrant explicit handling or documentation.
320-348: LGTM!The
list_tools()implementation correctly computes integrity hashes using canonical JSON and attaches them to eachMCPTool. The lazy import ofcompute_tool_hashis appropriate.
350-377: LGTM!The
verify_tool_before_call()method correctly re-fetches the tool list, finds the target tool, and verifies its hash. Appropriate errors are raised when the tool is missing or when verification fails.autogpt_platform/backend/backend/blocks/mcp/block.py (2)
100-106: LGTM!The
tool_integrity_hashfield is correctly added as a hidden input with an empty default. The docstring clearly explains the auto-population and verification behavior.
264-266: LGTM!The
MCPToolIntegrityErrorhandling correctly yields a user-friendly error message. This explicit catch is appropriate since it provides specific feedback for integrity failures rather than a generic error.autogpt_platform/backend/backend/blocks/mcp/test_security.py (1)
1-451: Comprehensive test coverage!The test suite thoroughly covers:
- Hash computation properties (determinism, key-order independence, field sensitivity)
- Verification success/failure scenarios including error messages
MCPSecurityContextgeneration, signing, and verification with proper mocking- Client-level integrity checks and MCPS integration
- Block-level integrity hash forwarding and error handling
The mocking approach using
patch.dict("sys.modules", ...)correctly simulates the optionalmcp-securedependency scenarios.autogpt_platform/backend/backend/blocks/mcp/security.py (3)
29-44: LGTM!The
compute_tool_hashimplementation correctly produces a canonical SHA-256 fingerprint. Key points:
- Sorted keys ensure deterministic output regardless of JSON key order
- Supports both
inputSchema(MCP wire format) andinput_schema(Python convention)- Compact separators prevent whitespace variations from affecting the hash
47-59: LGTM!The
verify_tool_hashfunction provides clear, actionable error messages with truncated hashes for readability while still being useful for debugging.
62-115: LGTM!The
MCPSecurityContextdataclass andgenerate()method correctly:
- Use lazy imports to make
mcp-securetruly optional- Generate separate agent and trust anchor key pairs
- Create and sign a passport with appropriate capabilities
- Provide clear error messages when the dependency is missing
Contributor
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
github-actions
Bot
added
the
conflicts
github-actions
Bot
removed
the
conflicts
Contributor
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
8 tasks
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This was referenced Apr 3, 2026
This was referenced May 19, 2026
This was referenced Jun 3, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #12455 +/- ##
=======================================
Coverage 75.99% 75.99%
=======================================
Files 2694 2694
Lines 204309 204304 -5
Branches 19677 19676 -1
=======================================
+ Hits 155270 155271 +1
- Misses 44685 44738 +53
+ Partials 4354 4295 -59
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
|
@dataCenter430 Thank you for the contribution, could you please fix the CI checks, so we can review&merge |
This was referenced Jul 30, 2026
This was referenced Aug 17, 2026
ntindle
requested review from
a team and
Abhi1992002
and removed request for
a team and
Swiftyos
September 1, 2026 18:48
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MCP (Model Context Protocol) has no built-in security: tool calls travel unsigned, tool definitions can change after deployment without detection, and there is no replay protection. Issue #12431 requests message signing and tool integrity verification. This PR adds (1) tool integrity hashing, SHA-256 fingerprint of tool definitions at discovery, with optional re-verification at execution to detect tool poisoning or rug pulls, and (2) optional MCPS message signing via the
mcp-securepackage (ECDSA-signed envelopes, replay protection) when the server supports it. Both layers are backward compatible: integrity is opt-in via a pinned hash; MCPS is opt-in via an optional security context.Changes 🏗️
backend/blocks/mcp/security.py(new) —compute_tool_hash/verify_tool_hash(canonical JSON SHA-256);MCPSecurityContextfor ECDSA P-256 signing and verification (requiresmcp-secure).backend/blocks/mcp/client.py—MCPTool.integrity_hash; optionalsecurity_ctxonMCPClient;_send_requestsigns outgoing and verifies incoming when MCPS is set;list_tools()attaches integrity hash per tool;verify_tool_before_call(tool_name, expected_hash);initialize()stores server_mcps.public_keyfrom capabilities.backend/blocks/mcp/block.py— Hidden inputtool_integrity_hash;_call_mcp_tool(..., tool_integrity_hash=...)runs integrity check when hash is set;run()catchesMCPToolIntegrityErrorand yields error output.backend/api/features/mcp/routes.py—MCPToolResponse.integrity_hash;/discover-toolsreturns it so the frontend can store it in block config when a tool is selected.pyproject.toml— Addedmcp-securedependency.backend/blocks/mcp/test_security.py(new) — Tests for hashing,MCPSecurityContext(generate/sign/verify), client integrity and MCPS wiring, and block integrity flow.Checklist 📋
For code changes:
poetry run pytest backend/blocks/mcp/test_security.py -v— all security and integrity tests passpoetry run pytest backend/blocks/mcp/test_mcp.py -v— existing MCP tests still passintegrity_hash, re-run after server change to confirm integrity error when definition mutatesFor configuration changes:
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changes