fix(cli): redact doctor credentials, honour global -c/-d, print errors once, explain locked set_profile (Spec fix-usertest-cli) - #1472
Merged
Conversation
…client set_profile refusals
Deploying mcpproxy-docs with
|
| Latest commit: |
3405aae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ecc6715c.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-usertest-cli-security.mcpproxy-docs.pages.dev |
loadRegistryConfig (registry and catalog) now goes through loadCLIConfig so a --data-dir with no config file anywhere no longer creates HOME/.mcpproxy/mcp_config.json (O1).
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 37061695743 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
11 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
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.
Fixes four findings from the first-run user test (audit IDs F-01, F-06, F-09, F-11). Go CLI and MCP only; REST, the Web UI and the macOS app are unchanged. Refs #1396 (same family as F-06, different root cause).
What changes
mcpproxy doctorno longer leaks the admin key. The?apikey=URL inGET /api/v1/infowas dumped verbatim intodoctor -o json. Every doctor format now redacts credential query parameters (apikey,token,secret, and the other shared sensitive parameters) in any URL and scrubs the literal admin key wherever it would appear; both print asREDACTED.doctor -o yamlwas silently empty and is now a real format (pretty,json,yaml); the global--jsonmaps to json and an unknown format is an error instead of no output.statusalready masked the key and is covered by a regression test;status --show-keyand--web-urlstay the explicit opt-ins.GET /api/v1/infostill returns the keyed URL to an authenticated administrator, as the tray needs it.-cand-dare authoritative for every management command. The reported symptom reproduces with an empty value (-c "", for example an unset shell variable): it created$HOME/.mcpproxy/mcp_config.jsonand reported an empty list. An empty--configor--data-diris now rejected at parse time on the root flags and on every command-local--config, with no help or--help-jsonchange. A single resolver (command flag, then global-c, then<data-dir>/mcp_config.jsonwhen it exists, then legacy discovery) is used by upstream, doctor, auth, call, code, tools, token and registry/catalog loaders, somcpproxy -d DIR upstream listreadsDIR/mcp_config.jsoninstead of fabricating a HOME default, and config-mode writes land in the file that was read.serveis unchanged.main()both printedError: ..., so every unsilenced RunE error (including the connect binding-guardFixes:list) appeared twice. A singleexecuteRootprints it once, keeps theunknown commandhelp hint and the exit codes.set_profiletells a client credential why a switch failed. A locked client gotunknown profile 'work-full'for a profile that exists. Client credentials now getcannot switch to profile '<slug>': this client's profile is locked(locked) orcannot switch to profile '<slug>': it is not a profile this client may switch to(switchable). The text depends only on the credential's own mode, never on the slug, and never names the bound profile, so it stays non-enumerating; agent tokens, anonymous callers and administrators keep the Spec 105 text, and/mcp/p/<slug>keeps its 404. The Spec 057/105 suites are untouched and the frozen tools/list goldens are unchanged.Spec and docs
Spec 108 contracts/refusals.md, contracts/mcp-tools.md, FR-018 and the user-story 4 scenario are updated; tasks.md gains Phase 17 (T153 to T156). New golden
internal/profile/testdata/contract/set_profile_refusals.json.docs/cli-management-commands.md(doctor formats and redaction, global flags) anddocs/features/profiles.md(set_profile texts) are updated.Tests
TestDoctorOutput_*,TestStatusOutput_MasksKeyInEveryFormat,TestConfigFlagsRejectEmptyValue,TestLoadersHonorGlobalConfigFlag,TestLoadersPreferDataDirConfig,TestLoadCLIConfigDataDirWithoutFileCreatesNothingInHome,TestUpstreamConfigFilePathMatchesLoadPath,TestResolveCLIConfigPathPrecedence,TestExecuteRoot_*,TestSetProfileRefusalGolden,TestSetProfileV3_ClientRefusalTextMatchesGolden, plus the updated v3 set_profile expectations.Follow-ups (not in this PR)
/mcp/p/<slug>404 for a locked client still saysunknown profile(needs a Spec 105 contract decision).serve -d DIRwithout-cstill reads the cwd or HOME config.statusmasks the URL key percent-encoded rather than asapikey=REDACTED.