security + refactor: harden scaffold, clean code, re-export FastMCP#2
Closed
jieyao-MilestoneHub wants to merge 4 commits into
Closed
security + refactor: harden scaffold, clean code, re-export FastMCP#2jieyao-MilestoneHub wants to merge 4 commits into
jieyao-MilestoneHub wants to merge 4 commits into
Conversation
- Fix mcp-forge-core README: github.com/mcp-forge/mcp-forge (404) → github.com/CoreNovus/mcp-forge - Fix Quick Start example: mcp.run() → run_server(mcp) to use the mode-switching server factory - Remove PLAN.md: documented Protocol-based architecture but code uses ABCs, template filenames and workflow structure were outdated Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Scaffold (mcp-forge-cli):
- Use SandboxedEnvironment instead of Environment to prevent Jinja2
template introspection attacks via custom --templates directories
- Add path traversal guard in MCPServerScaffold.generate() — verify
output stays within the intended parent directory
- Add validate_text_field() to reject unsafe characters (", \n, \r, \)
in author/email/description fields before template interpolation
- Call validation in orchestrator before scaffolding
Providers (mcp-forge-core):
- InMemoryCache: add max_size parameter with oldest-first eviction to
prevent unbounded memory growth in long-running dev servers
- InMemoryTelemetry: add max_metrics parameter with rolling window
- Default server_host from 0.0.0.0 to 127.0.0.1 in config.py and
server_factory.py to prevent unintended network exposure in dev
AWS providers (mcp-forge-aws):
- Narrow exception handling: catch (ClientError, BotoCoreError) instead
of bare Exception in DynamoDB and CloudWatch providers — let
programming errors propagate instead of being silently swallowed
- CloudWatch: use asyncio.to_thread() for consistency with DynamoDB
providers instead of loop.run_in_executor(lambda)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- bedrock_llm.py: remove no-op message conversion loop — the ternary and subsequent for-loop were both identity operations - retry.py: extract _execute_with_retry() to eliminate duplicated retry loop between @Retry decorator and with_retry() function; log type(exc).__name__ instead of full exception message to avoid leaking sensitive data in retry warnings - cli.py: fix version command — use proper __version__ import instead of hacky ScaffoldConfig.__module__.split('.')[0] - test_scaffold.py: add encoding="utf-8" to all read_text() calls — fixes 2 pre-existing test failures on Windows cp950 locale caused by em-dash characters in generated server.py docstring Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Make the mcp SDK an invisible implementation detail of mcp-forge-core so that generated servers never import from mcp.server.fastmcp directly. - Re-export FastMCP from mcp_forge_core.__init__ — users can now write from mcp_forge_core import FastMCP for type annotations - Update test_sample.py.j2 to use create_mcp_app() instead of directly importing FastMCP from the mcp SDK - Remove redundant mcp>=1.0 dependency from pyproject.toml.j2 — it is already a transitive dependency via mcp-forge-core When mcp SDK 2.0 ships, only server_factory.py needs updating — no generated user code will break. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
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
1 participant
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.
Summary
mcp.run()→run_server(mcp), remove stale PLAN.mdmax_size/max_metrics), default bind127.0.0.1, narrow exception catching in AWS providers (ClientError, BotoCoreErrorinstead of bareException)FastMCPfrommcp_forge_core, update generated test template to usecreate_mcp_app(), remove redundantmcpdependency from generatedpyproject.tomlSecurity Findings Addressed
SandboxedEnvironmentvalidate_text_field()max_size/max_metricsparams127.0.0.1except Exceptionin AWS providersexcept (ClientError, BotoCoreError).resolve()+ parent checkTest plan
pytest packages/mcp-forge-core/tests/— 150 passedpytest packages/mcp-forge-aws/tests/— 67 passedpytest packages/mcp-forge-cli/tests/— 37 passed (includes 2 previously failing Windows tests now fixed)ruff check packages/— all checks passed🤖 Generated with Claude Code