Skip to content

fix(config): validate persisted settings and MCP hostnames - #35

Open
jung233 wants to merge 2 commits into
Rimagination:masterfrom
jung233:codex/config-mcp
Open

fix(config): validate persisted settings and MCP hostnames#35
jung233 wants to merge 2 commits into
Rimagination:masterfrom
jung233:codex/config-mcp

Conversation

@jung233

@jung233 jung233 commented Jul 17, 2026

Copy link
Copy Markdown

Summary

  • allow configured external MCP hostnames without disabling DNS rebinding protection
  • validate persisted worker, timeout, cache, delay, and PDF size settings on load and update
  • reject booleans, non-finite numbers, wrong types, and out-of-range values

Verification

  • standalone branch rebased directly onto upstream v1.9.0 at 5e4a6f2; GitHub reports mergeable=true
  • Python 3.11 and Python 3.12 source compilation: passed
  • configuration and FastMCP server imports: passed
  • no files under tests/ are changed

@jung233
jung233 force-pushed the codex/config-mcp branch from 951d7f8 to ea42a6c Compare July 17, 2026 08:38
@jung233
jung233 marked this pull request as ready for review July 17, 2026 09:31
Copilot AI review requested due to automatic review settings July 17, 2026 09:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens configuration handling and MCP transport security by introducing validation of persisted numeric settings and adding support for allowing a configured external MCP hostname while keeping DNS rebinding protection enabled.

Changes:

  • Add MCP transport security configuration in the server, driven by a new mcp_server_name config value.
  • Add centralized config value validation on load and update (type checks, finite numbers, and range checks).
  • Update README to document configuring the accepted MCP hostname (including via MCP_SERVER_NAME).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/scansci_pdf/server.py Loads configured MCP hostname and applies TransportSecuritySettings/server naming accordingly.
src/scansci_pdf/config.py Adds validation rules and applies them during config load and updates; introduces mcp_server_name.
README.md Documents how to configure MCP accepted hostname and adds related config table entries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/scansci_pdf/server.py
Comment on lines +22 to +26
_config = load_config()
_mcp_name = _config.get("mcp_server_name", "scansci-pdf")

# Configure transport security for non-localhost hostnames
_transport_security = None
Comment thread src/scansci_pdf/config.py
Comment on lines +105 to +109
def _validate_config_value(key: str, value: Any) -> Any:
rule = _VALIDATION_RULES.get(key)
if rule is None:
return value

Comment thread src/scansci_pdf/config.py
Comment on lines 59 to +62
"json_probe_cache_seconds": 3600,
"cache_ttl_hours": 168,
"parallel_sources": True,
"source_workers": 4,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants