Skip to content

mcp-startup-config-validation-and-credential-redaction - #835

Merged
general-kroll-4-life merged 3 commits into
mainfrom
security-hardening
Oct 9, 2026
Merged

mcp-startup-config-validation-and-credential-redaction#835
general-kroll-4-life merged 3 commits into
mainfrom
security-hardening

Conversation

@jeffreyaven

@jeffreyaven jeffreyaven commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Description

Reject malformed MCP configuration before either mcp or srv exposes listeners or tools. Both entrypoints now require an inline JSON object, reject unknown fields and decoding/validation errors, and return an error without echoing the configuration. Legacy read_only and tool restrictions remain effective.

Sanitize a diagnostic copy of registered startup arguments using the field and URL rules in #825. Preserve environment/file references, ordinary registry URLs, Jsonnet variables, usernames and sibling settings. Mask inline credentials, DSNs, OTLP header values and TLS key contents. Startup masking lives in internal/stackql/diagnostics behind Diagnoser, with the policy selected by NewMCPServerDiagnoser. The implementation is private; no new DTOs or mutable configuration carriers are introduced.

Authentication, reload, queries, flag precedence and stdio session lifetime are unchanged. #828 is excluded.

Type of change

  • Bug fix.
  • Breaking change: invalid/unknown MCP configuration is now rejected instead of silently ignored. File paths and YAML are unsupported by the CLI; help now matches the existing inline JSON contract.

Issues referenced

Closes #824
Closes #825

Evidence

  • CI: all 29 checks passed on review update bb0aeb4, including platform builds, full robot runs, Docker SQLite/PostgreSQL shards, shard verification, lint, CodeQL and GitGuardian. Build run.
  • Build: CGO_ENABLED=0 python cicd/python/build.py --build passed with Go 1.25.3.
  • Full unit suite: python cicd/python/build.py --test passed; affected packages also passed after final edits.
  • Lint: CI's golangci-lint v2.5.0 release binary, Go 1.25.3, Linux, clean cache: 0 issues.
  • Robot: all 77 MCP cases passed in the review-update run, including command HTTP defaulting and explicit stdio precedence. New cases cover both entrypoints/transports, rejected configuration, bundle-style flags, disclosure/non-disclosure, effective restrictions, credential use and idle stdio. Existing reload and protocol tests pass.
  • Unit coverage includes repeats, aliases, both flag syntaxes, malformed JSON/YAML and URLs, nested credentials, case/duplicate keys and non-mutation.

Checklist

  • Local build, unit, lint and relevant robot checks pass.
  • Changes have black-box robot coverage.
  • CI passes on supported platforms, including the full robot suite.

Variations

Local build/unit/robot checks ran on Windows; CI-pinned lint ran under Linux/WSL. Full platform and database coverage passed in CI on the review update.

Diagnostic configuration is reserialized; flag aliases are shown by their registered names. Unknown options and positional payloads are omitted. This does not sanitize all application errors: an existing malformed --auth DTO error can echo a short value before the startup diagnostic (#816 scope). No arbitrary-string secret detection is added.

Tech Debt

No new deferred work. Inline-auth policy changes and request deadlines remain separate issues.

@gitguardian

gitguardian Bot commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

general-kroll-4-life
general-kroll-4-life reviewed Oct 9, 2026
View reviewed changes
Comment thread internal/stackql/cmd/mcp.go Outdated
}
if config.Server.Transport == "" {
config.Server.Transport = mcpServerType
if config.Server.Transport == "" {

@general-kroll-4-life general-kroll-4-life Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is not needed

general-kroll-4-life
general-kroll-4-life reviewed Oct 9, 2026
View reviewed changes
Comment thread internal/stackql/diagnostics/mcp.go

@general-kroll-4-life general-kroll-4-life Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does not belong in the cmd package. Please move this functionality to a dedicated pacakge with abstraction, behaviour differentiated at contruction, contrived example:

diagnoser := diagnostics.NewMcpServerDiagnoser(...)
diagnosis, diagnosisErr := diagnoser.Diagnose()
...

jeffreyaven and others added 2 commits October 9, 2026 12:12
@jeffreyaven
isolate-mcp-startup-diagnostics
bb0aeb4
@jeffreyaven
Add mcp-wringer MCP robustness gate for pull requests to main
0ea624f 
Fuzzes the MCP server built from the pull request with the published
stackql/mcp-wringer@v0 Action on each platform the project ships
(linux amd64 and arm64, darwin amd64 and arm64, windows amd64, and the
amd64 exe under emulation on windows-11-arm) in every server mode the
fuzzer can drive: stdio at 2025-11-25, stdio pinned to 2026-07-28,
Streamable HTTP stateful at 2025-11-25, and Streamable HTTP stateless
at 2026-07-28. The server runs hermetically with a throwaway approot,
the repository's file:// registry, audit off and no credentials.

cicd/mcp-wringer/mcp-server.mjs owns the server invocation: it emits
the Action's command and args, and starts and stops the detached HTTP
servers with a readiness wait. Reports and reproducers are uploaded as
artifacts per platform.

Verified locally on Windows amd64 with the published 0.1.0 package:
all four modes are clean at the standard profile (256 cases over HTTP;
stdio was capped by the profile's 120 s budget at 197 cases, so the
workflow raises MCP_WRINGER_DURATION_MS so every platform runs the same
256 cases). A stateful server fuzzed as a 2026-07-28 client is rejected
with -32022 and is not a fuzzable mode. actionlint 1.6.23 passes.
Not verified: the non-Windows runners, which the pull request run will
exercise.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@general-kroll-4-life
general-kroll-4-life merged commit 6d65b49 into main Oct 9, 2026
36 checks passed
@general-kroll-4-life
general-kroll-4-life deleted the security-hardening branch October 9, 2026 10:10
@jeffreyaven jeffreyaven mentioned this pull request Oct 12, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@general-kroll-4-life general-kroll-4-life general-kroll-4-life approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[SECURITY] Stop logging secret-bearing process arguments at MCP startup [SECURITY] Fail startup when MCP security configuration cannot be parsed

2 participants

@jeffreyaven @general-kroll-4-life