Repository navigation
Auth revamp: MCP connection binding and reference-only credentials - #15
Merged
Merged
Conversation
…--name/--dsn-env (REF-297) Decouple `plenum mcp` connection resolution from the launcher's cwd. The long-lived MCP server is started from arbitrary directories (Claude Desktop, editors, daemons), so cwd is not a meaningful signal for which project's config to use. New `plenum mcp` flags, captured into an McpBinding at startup: - --project-path <path>: pins config resolution to that project. Loads the project's LOCAL `.plenum/config.json` via the new config::resolve_connection_in_project (not CWD's local config), plus global config keyed by that path — so it works regardless of launcher cwd. - --name <connection>: selects a named connection within the resolved project. - --dsn-env <VAR>: reads the DSN from ONLY the named env var at connection time. No ambient credential pickup (DATABASE_URL, PGPASSWORD, ...). Per-call tool arguments still take precedence; the binding only supplies the default when a call omits its own connection selectors. Safety / correctness: - --dsn-env conflicts with --project-path/--name (rejected at the CLI via clap and defensively in McpBinding::new). - When no binding flag is given and cwd has no config, tool calls fail with a structured error naming exactly which flags to add — never a silent fallback. Tests: - 9 unit tests (src/mcp.rs) covering binding construction/conflicts and credential-sourcing safety (named-var-only, no ambient fallback). - 9 integration tests (tests/mcp_bound_connection.rs) driving the compiled binary over stdio: project-path pinning from local and global config, name selection, dsn-env resolution, no-ambient-DATABASE_URL, structured no-config error, and CLI conflict rejection. Verified: cargo build --all-targets, cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, cargo test (all offline suites), and scripts/test-live.sh (MySQL 8.0/8.4, PostgreSQL 16) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pect/query (REF-298)
Restore the reference-based connect design so agents can persist connection
config without ever handling plaintext secrets.
- connect: reject inline plaintext `password` with a CAPABILITY_VIOLATION; add
`save` ("local"|"global"), `project_path`, `name`, and the credential-reference
fields `password_env`/`password_command`/`keychain_service`+`keychain_account`.
Save mode persists by reference (never plaintext) via a new project-path-aware
`save_connection_in_project` (local writes go to <project>/.plenum/config.json,
independent of the MCP server's cwd).
- introspect & query: expose the same credential-reference fields for one-off
explicit connections; the secret is resolved from the reference at connect
time and never passes through the tool args.
- Enforce reference invariants at the MCP boundary: at most one source,
keychain_service/keychain_account required together.
Tests: unit tests for parse_credential_refs, one-off reference resolution, and
save-by-reference; config test for save_connection_in_project; E2E tests driving
the real `plenum mcp` binary for inline-password rejection and reference-based
local save. Full offline suite (350) + live MySQL/Postgres suites (56) green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (REF-299) The interactive wizard now offers plaintext, env var, shell command, and OS keychain credential sources — the user always types the source name; the wizard never scans the environment, keychain, or filesystem. Indirect sources are validated at connect time with errors that name the exact field to fix. plenum connect save output now includes a ready-to-paste mcp_stanza (mcpServers block) reproducing the exact --project-path/--name binding for the saved connection, so operators can wire up the MCP server without docs. Co-Authored-By: Paperclip <noreply@paperclip.ing>
…-303) `plenum connect --project-path X --save local` run from a different cwd reported success but persisted nothing: the local config format is single-project and keyed by its own directory, so writing under cwd while keying the entry under an explicit --project-path silently dropped the connection. Use save_connection_in_project so --save local targets <project_path>/.plenum/config.json, and route CLI connection resolution through --project-path consistently. Adds a regression test. Surfaced during REF-303 whole-flow end-to-end verification of REF-292. Co-Authored-By: Paperclip <noreply@paperclip.ing>
…truct (REF-304) Two non-blocking follow-ups from the REF-292 SRE review: 1. Type the connect --save success `data` through a new `ConnectSaveResult` struct instead of building it inline via `serde_json::json!` in main.rs. Wired into generate-schemas (schemas/connect_save_success.json) and tests/schema_drift.rs, so the emitted stanza shape — including the nested McpStanza — is now covered by schema-drift and can no longer drift silently. 2. Convert the 7-field `type ConnectArgs = (...)` tuple alias into a named struct with documented fields for readability across the three resolution paths (picker, wizard, non-interactive) and the save step. Output is byte-identical: cli_connect integration tests pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`password_command` is executed via `sh -c`. Accepting it from MCP tool arguments gave any connected agent arbitrary shell execution on the host, which breaks Plenum's least-privilege execution surface. - handle_call_tool rejects `password_command` with CAPABILITY_VIOLATION for connect, introspect, and query before any tool logic runs — including named-connection calls, so the field is never silently ignored. - Remove `password_command` from the three tool input schemas and from parse_credential_refs; MCP credential references are now `password_env` or `keychain_service` + `keychain_account`. - The CLI `--password-command` flag and the connect wizard are unchanged: a human types the command there. Residual risk (documented in SECURITY.md): a connection already saved with `password_command` still runs it when resolved through the MCP server; the command comes from a config file, not from tool arguments. Tests: 5 E2E tests drive `plenum mcp` with a marker-file command and assert CAPABILITY_VIOLATION, that the command never ran, and nothing was written. Offline suite (502) and live MySQL 8.0/8.4 + PostgreSQL 16 suites (56) green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…anges - README: "Zero to Query" CLI-first and MCP-first paths; `plenum mcp` --project-path/--name/--dsn-env binding; cwd-coupling warning; MCP tool field reference; credential sourcing safety rules. password_command is documented as CLI-only. - SECURITY.md: MCP credential references, password_command rejection and its residual risk for saved configs, and MCP connection binding. - CHANGELOG: entries for REF-297, REF-298, REF-299, REF-303, REF-304 and the password_command rejection. - CLAUDE.md: MCP connection binding flags and credential-reference rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI failed to compile: Cargo.lock is not tracked, so CI resolved duckdb 1.10506.0, where `duckdb::types::Value` is `#[non_exhaustive]` and gains `UHugeInt(u128)` and `Geometry(Vec<u8>)`. The match in duckdb_value_to_json had no wildcard arm (E0004). - UHUGEINT converts to a string, like HUGEINT, to preserve precision. - GEOMETRY (WKB bytes) converts to Base64, like BLOB. - duckdb_value_to_json now returns Result: an unknown future variant is a structured QUERY_FAILED error naming the type, never a guessed value. - Raise the duckdb minimum to 1.10506.0 (the new variants exist only there). Tests: unit tests for UHugeInt and Geometry conversion; an E2E query of the UHUGEINT maximum. Verified with all dependencies updated to latest (matching CI): fmt, clippy -D warnings, cargo test --all-features (505 passed), and live MySQL 8.0/8.4 + PostgreSQL 16 suites (56 passed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rt_is_empty CI runs clippy on unpinned `stable`, now Rust 1.99, which adds two lints: - `clippy::unused_async_trait_impl` (split from `unused_async` in 1.98) fires on the 3 DatabaseEngine methods in the SQLite and DuckDB impls. Their drivers are synchronous, but the trait requires `async fn`, so the lint cannot be satisfied without hand-rolled `impl Future` bodies. Allow it on the two impl blocks. `#[allow(unknown_lints)]` keeps older toolchains (e.g. 1.97) from rejecting the new lint name under -D warnings. Same approach as the closed PR #14 (8407dbe). - `clippy::assert_is_empty` (new in 1.99) fires on 13 test asserts. Rewrite them as assert_eq!/assert_ne! so a failure shows the value. Verified: clippy --all-targets --all-features -D warnings clean on both 1.97 and 1.99; fmt on both; cargo test --all-features (505 passed); live MySQL 8.0/8.4 + PostgreSQL 16 suites (56 passed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
The MCP server now finds the correct connection without the launcher's cwd, and agents never handle plaintext secrets or shell commands.
plenum mcp --project-path <path>,--name <connection>,--dsn-env <VAR>.--dsn-envreads only the named variable — neverDATABASE_URLor other ambient vars. No binding and no cwd config → structured error that names the flags to add.connectrejects inline plaintextpasswordwithCAPABILITY_VIOLATION.connect,introspect, andqueryacceptpassword_envorkeychain_service+keychain_account.connectgainssave("local"|"global"),project_path, andname, and stores only the reference.password_commandrejected on all MCP tools (new in this PR): it executessh -c, so accepting it from tool arguments gave agents arbitrary shell execution. The CLI flag and wizard still offer it. Residual risk for configs already saved withpassword_commandis documented inSECURITY.md.connect --saveoutput includes a ready-to-pastemcp_stanza.plenum connect --project-path X --save localfrom another directory reported success but saved nothing; it now writesX/.plenum/config.json.ConnectSaveResultwithschemas/connect_save_success.jsonunder schema drift;ConnectArgsis a named struct.Test plan
cargo build --all-targetscargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test— 502 passed, 0 failed (82 live tests ignored)scripts/test-live.sh— MySQL 8.0/8.4 (39) and PostgreSQL 16 (17) passedcargo test --test schema_drift— passes withconnect_save_success.json🤖 Generated with Claude Code