Compare commits
27
Commits
7368dcdbff
...
v1.4.0
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f73b49f17c | ||
|
|
451e04cd96 | ||
|
|
675490e3c3 | ||
|
|
3768113938 | ||
|
|
583e4e4af0 | ||
|
|
38e9cf2b26 | ||
|
|
d4ce2647ae | ||
|
|
ece8c99f79 | ||
|
|
66b0101dea | ||
|
|
c5a6bdd1d1 | ||
|
|
ded1eef2dd | ||
|
|
cdda60da1b | ||
|
|
0920846c2c | ||
|
|
ce82f7b5c3 | ||
|
|
d2c126a60c | ||
|
|
603d24566d | ||
|
|
0b2827e6b8 | ||
|
|
2e5d0351b4 | ||
|
|
2cd8e0fb3b | ||
|
|
8c51c25211 | ||
|
|
e087107710 | ||
|
|
436524bf00 | ||
|
|
e542ff6e8f | ||
|
|
dc9e035781 | ||
|
|
694439b6f3 | ||
|
|
4743c4a995 | ||
|
|
8fdbe90b37 |
@@ -3,13 +3,16 @@ name: CI
|
||||
on:
|
||||
push:
|
||||
branches: [main]
|
||||
paths-ignore: ["**/*.md"]
|
||||
pull_request:
|
||||
paths-ignore: ["**/*.md"]
|
||||
workflow_dispatch:
|
||||
|
||||
jobs:
|
||||
lint:
|
||||
runs-on: ubuntu-latest
|
||||
name: Lint (ruff)
|
||||
timeout-minutes: 10
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -31,6 +34,11 @@ jobs:
|
||||
test:
|
||||
runs-on: ${{ matrix.os }}
|
||||
name: Tests (py${{ matrix.python }} / ${{ matrix.os }})
|
||||
# Guards against a job that hangs mid-run (e.g. a wedged test). Note: this
|
||||
# counts from when a runner PICKS UP the job, so it does not rescue a job
|
||||
# stuck "Waiting to run" because the self-hosted Windows runner is offline —
|
||||
# for that, bring the runner back or skip via paths-ignore (docs).
|
||||
timeout-minutes: 15
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
@@ -85,6 +93,7 @@ jobs:
|
||||
catalog-signature:
|
||||
name: Catalog signature
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
# Changelog
|
||||
|
||||
All notable changes to **BetterClaudeConfig** are recorded here. The format is
|
||||
based on [Keep a Changelog](https://keepachangelog.com/), and this project
|
||||
follows [Semantic Versioning](https://semver.org/): breaking changes bump the
|
||||
major, new features bump the minor, fixes bump the patch.
|
||||
|
||||
**When you open a PR, add a line under `[Unreleased]`.** At release time, that
|
||||
section is renamed to the new version + date and a fresh `[Unreleased]` is
|
||||
started.
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [1.4.0] — 2026-08-13
|
||||
|
||||
> ⚠️ **Using the `ssh-mcp` server?** Upstream `ssh-mcp` shipped a **breaking
|
||||
> v2**: it removed the `--password`, `--sudoPassword`, `--suPassword` and
|
||||
> `--disableSudo` command-line flags, and it now reads a `config.toml` sidecar
|
||||
> file that overrides your command-line arguments. A config written for v1
|
||||
> crashes on v2's startup. This release adds tools to detect and fix that.
|
||||
> **BetterClaudeConfig itself has no breaking changes** — your existing configs
|
||||
> keep working; nothing is changed without your click.
|
||||
|
||||
### Added
|
||||
- **ssh-mcp v2 flag migration.** Detects the credential flags v2 removed and
|
||||
one-click-moves each into the environment variable ssh-mcp now reads
|
||||
(`--password` → `SSH_MCP_PASSWORD`; `--sudoPassword` / `--suPassword` →
|
||||
`SSH_MCP_SUDO_PASSWORD`). Also flags `--maxChars=none`, whose meaning changed
|
||||
between versions.
|
||||
- **Sidecar-config awareness.** When a server reads a separate config file
|
||||
(ssh-mcp's `config.toml`), BCC shows where that file actually lives on your
|
||||
platform and warns when your command-line args are **inert** because the file
|
||||
takes precedence — including when a file sits at the wrong, documented-but-
|
||||
unused path.
|
||||
- **In-app sidecar editor.** Edit that external config from inside BCC —
|
||||
pick-lists for known fields, a raw-text fallback — written atomically with a
|
||||
timestamped backup and locked to `0600`. No dropping to a terminal.
|
||||
- **Live hot-reload.** External changes to a sidecar (or to your Claude config)
|
||||
now surface without restarting BCC.
|
||||
- **Version pinning for `npx` servers.** Spots servers launched with `-y` /
|
||||
`@latest` that resolve a fresh version every run, shows the version currently
|
||||
resolved, and offers a one-click **Pin to this version** — with a drift note
|
||||
when a pin has fallen behind.
|
||||
- **Permission pre-flight.** Warns when a credential-bearing config is readable
|
||||
by other users on the machine and offers a one-click fix to `0600`/`0700`.
|
||||
- **Light theme + system-following** (the dark theme is preserved exactly).
|
||||
- **"Move to environment variable."** Convert a plaintext secret in a config
|
||||
into a `${VAR}` reference in place — offered only on clients that actually
|
||||
expand references, so it can't silently break a Claude Desktop config.
|
||||
- **Cross-client foundation.** Claude Desktop and Claude Code now flow through a
|
||||
single adapter — groundwork for supporting more clients.
|
||||
|
||||
### Changed
|
||||
- The update checker is now **visible** — a persistent banner plus a Help-menu
|
||||
item — instead of being buried in the About dialog.
|
||||
- Config writes share one atomic-write path; the temp file is created `0600`, so
|
||||
a secret is never briefly world-readable mid-write.
|
||||
|
||||
### Fixed
|
||||
- Loading a config with a non-object server value no longer crashes, and named
|
||||
server sets survive an external-change merge.
|
||||
- Project profiles that share a directory basename are disambiguated, so you
|
||||
can't accidentally edit the wrong `.mcp.json`.
|
||||
|
||||
### Internal / maintainer
|
||||
- Signed-catalog core and a maintainer-only **Catalog Console** (review + sign),
|
||||
with hardening of the review gate and a split of the signing keys. No
|
||||
user-facing catalog browser ships yet.
|
||||
|
||||
## [1.3.0] — 2026-07-12
|
||||
Named server sets, Claude Code project `.mcp.json` discovery, structural schema
|
||||
lint, and UX polish (Ctrl+S to save, enable-all / disable-all).
|
||||
|
||||
## [1.2.1] — 2026-07-12
|
||||
First shipped binaries: app-icon fix, a batch of audit fixes, and Windows
|
||||
process-tree cleanup on spawn-tests.
|
||||
|
||||
## [1.2.0] — 2026-07-08
|
||||
Server log viewer, duplicate-name conflict handling, a Restart-Claude button,
|
||||
stale-file protection, an About dialog, and a notify-only update checker.
|
||||
|
||||
## [1.1.0] — 2026-07-04
|
||||
Backup / restore UI and secret masking.
|
||||
|
||||
## [1.0.1] — 2026-06-29
|
||||
## [1.0.0] — 2026-06-29
|
||||
Initial releases: the core `mcpServers` editor with the lenient paste/repair
|
||||
pipeline that tolerates malformed JSON.
|
||||
|
||||
<!-- Backfill for 1.0.0–1.3.0 is summarised from release notes; the Unreleased
|
||||
section onward is maintained per-PR. -->
|
||||
@@ -0,0 +1,50 @@
|
||||
# PR #87 — "Move to environment variable" (#83): manual test checklist
|
||||
|
||||
The logic is covered by 15 unit tests in CI; what CI **can't** exercise is the GUI (no PySide6). This checklist is only the parts a human needs to click. Should take ~10 minutes.
|
||||
|
||||
## Setup
|
||||
|
||||
```bash
|
||||
cd ~/Documents/Claude/Projects/BetterClaudeConfig/better-claude-config
|
||||
git fetch origin
|
||||
git checkout feat/83-move-to-env-var
|
||||
git pull # ensure you're on 8fdbe90 or later
|
||||
source .venv/bin/activate # or recreate: python3 -m venv .venv && source .venv/bin/activate && pip install -r requirements.txt
|
||||
python bcc.py
|
||||
```
|
||||
|
||||
Pick a **Claude Code** profile (e.g. `~/.claude.json`) that has, or add, a server with an env value that looks like a secret — e.g. `env: { "API_KEY": "ghp_test123" }`. (You can use a throwaway value; nothing is sent anywhere.)
|
||||
|
||||
## The checklist
|
||||
|
||||
### Gating — where the action appears
|
||||
- [ ] Right-click the **value cell** of a secret env row (`API_KEY`) on a **Claude Code** profile → a **"Move to environment variable…"** item appears.
|
||||
- [ ] Right-click a **non-secret** row (e.g. `REGION` = `us-east-1`) → the item does **not** appear.
|
||||
- [ ] Right-click a row whose value is already a reference (`${API_KEY}`) → the item does **not** appear.
|
||||
- [ ] Switch to a **Claude Desktop** profile (a `claude_desktop_config.json`), right-click the same kind of secret row → the item does **not** appear. (Desktop doesn't expand `${VAR}`, so offering it would break the config — this is the important gate.)
|
||||
|
||||
### The dialog
|
||||
- [ ] Trigger the action → dialog opens with **Variable** pre-filled from the key, sanitized to a legal shell name (e.g. `api-key` → `API_KEY`).
|
||||
- [ ] Edit the variable name → the shown **shell line updates live** and matches your platform (`export VAR='…'` on macOS/Linux, `setx VAR "…"` on Windows), with the other platform shown in parentheses.
|
||||
- [ ] If you type a variable name that **is already set** in your shell environment, the green "already looks set" note appears; if not, it's hidden.
|
||||
- [ ] **Cancel** → nothing changes (value still the raw secret, no dirty state).
|
||||
|
||||
### The conversion
|
||||
- [ ] **Move && copy secret** → the cell now shows the reference `${VAR}` (visible, **not** masked to dots), and the window goes dirty (Save enabled).
|
||||
- [ ] Paste from your clipboard somewhere → it's the **original secret value** (handed back before removal).
|
||||
- [ ] The reference value is **not** flagged as a secret warning anymore (it's the recommended state).
|
||||
|
||||
### Headers + persistence
|
||||
- [ ] Repeat on a **remote server's Headers** table (e.g. an `Authorization` header) → same behavior.
|
||||
- [ ] **Save**, then open the config file on disk in a text editor → the servers block holds `${VAR}`, and the **plaintext secret is gone** from the file.
|
||||
- [ ] Re-open the profile in BCC → the row still shows `${VAR}` (round-trips).
|
||||
|
||||
### Undo (nice-to-have)
|
||||
- [ ] After a conversion, **Ctrl+Z / Cmd+Z** restores the previous value.
|
||||
|
||||
## Known scope (not bugs)
|
||||
- **Args rows** are out of scope for this PR — the core supports them, but the args editor is a free-text widget, so wiring that UI is a deliberate follow-up. Right-clicking args won't offer the action yet.
|
||||
- The "already set" check reads **BCC's** environment, which may differ from the client's — it's advisory, worded that way.
|
||||
|
||||
## If anything's off
|
||||
Tell me which checkbox failed and what you saw; I'll fix on the branch and re-push. If everything passes, approve/merge #87 (or tell me to merge it).
|
||||
+1438
-48
File diff suppressed because it is too large
Load Diff
+1
-1
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "better-claude-config"
|
||||
version = "1.3.0"
|
||||
version = "1.4.0"
|
||||
description = "Cross-platform GUI for editing the mcpServers block of Claude Desktop and Claude Code configs"
|
||||
readme = "README.md"
|
||||
license = { file = "LICENSE" }
|
||||
|
||||
+883
-3
@@ -3169,13 +3169,38 @@ def test_removed_flag_warnings_covers_migratable_and_manual():
|
||||
assert "--password" in text
|
||||
assert "sudoPassword" in text
|
||||
assert "disableSudo" in text
|
||||
# migrate only touches the confirmed --password mapping
|
||||
# #90: --password AND --sudoPassword are now both auto-migratable (the
|
||||
# verified ssh-mcp facts map sudo/su → SSH_MCP_SUDO_PASSWORD). The migration
|
||||
# LOGIC is unchanged — only the registry data grew — so both move into env.
|
||||
# --disableSudo has no env replacement (sudo is now a role/policy) → warn-only.
|
||||
new, _ = c.migrate_removed_flags(data)
|
||||
assert new["env"] == {"SSH_MCP_PASSWORD": "p"}
|
||||
assert "--sudoPassword=s" in new["args"]
|
||||
assert new["env"] == {"SSH_MCP_PASSWORD": "p", "SSH_MCP_SUDO_PASSWORD": "s"}
|
||||
assert "--sudoPassword=s" not in new["args"]
|
||||
assert "--disableSudo" in new["args"]
|
||||
|
||||
|
||||
def test_migrate_su_password_maps_to_sudo_env():
|
||||
# --suPassword is the second flag that maps to the same SSH_MCP_SUDO_PASSWORD var.
|
||||
data = {"command": "ssh-mcp", "args": ["--suPassword", "rootpw"]}
|
||||
new, notes = c.migrate_removed_flags(data)
|
||||
assert new["env"] == {"SSH_MCP_SUDO_PASSWORD": "rootpw"}
|
||||
assert "args" not in new
|
||||
assert notes
|
||||
|
||||
|
||||
def test_migrate_sudo_and_su_two_flags_one_var_no_clobber():
|
||||
# Both sudo flags map to one var; the no-clobber path keeps the first, drops
|
||||
# the second (different value) with a note rather than silently overwriting.
|
||||
data = {
|
||||
"command": "ssh-mcp",
|
||||
"args": ["--sudoPassword=first", "--suPassword=second"],
|
||||
}
|
||||
new, notes = c.migrate_removed_flags(data)
|
||||
assert new["env"] == {"SSH_MCP_SUDO_PASSWORD": "first"}
|
||||
assert "args" not in new
|
||||
assert any("already set" in n for n in notes)
|
||||
|
||||
|
||||
def test_removed_flag_warnings_empty_when_clean():
|
||||
assert c.removed_flag_warnings({"command": "ssh-mcp", "args": ["--host=h"]}) == []
|
||||
assert c.removed_flag_warning({"command": "ssh-mcp", "args": ["--host=h"]}) is None
|
||||
@@ -3206,3 +3231,858 @@ def test_ssh_membermatters_end_to_end():
|
||||
]
|
||||
assert new["env"] == {"SSH_MCP_PASSWORD": "topsecret"}
|
||||
assert notes
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# ServerSpec spine (issue #90)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_server_spec_registry_seeds_ssh_mcp():
|
||||
spec = c.SERVER_SPECS["ssh-mcp"]
|
||||
assert spec.package == "ssh-mcp"
|
||||
# sudo/su both map to the one sudo env var (verified facts).
|
||||
assert spec.env_flags["--sudoPassword"] == "SSH_MCP_SUDO_PASSWORD"
|
||||
assert spec.env_flags["--suPassword"] == "SSH_MCP_SUDO_PASSWORD"
|
||||
assert spec.env_flags["--password"] == "SSH_MCP_PASSWORD"
|
||||
# disableSudo stays warn-only (no env replacement).
|
||||
assert "--disableSudo" in spec.removed_flags
|
||||
assert "--disableSudo" not in spec.env_flags
|
||||
# Verified sidecar paths seeded per platform (README path differs → doc_path).
|
||||
assert "ssh-mcp/config.toml" in spec.sidecar_paths["darwin"]
|
||||
assert "Application Support" in spec.sidecar_paths["darwin"]
|
||||
assert "APPDATA" in spec.sidecar_paths["win32"]
|
||||
assert spec.sidecar_doc_path == "~/.config/ssh-mcp/config.toml"
|
||||
# zod enums seeded as data (pick-lists later — #7).
|
||||
assert spec.schema["auth"] == ["agent", "key", "password", "keychain"]
|
||||
assert spec.schema["approvalMode"] == ["auto", "ask-destructive", "ask-all", "deny"]
|
||||
assert spec.schema["role"] == ["viewer", "operator", "admin"]
|
||||
assert spec.schema["port"] == {"min": 1, "max": 65535}
|
||||
|
||||
|
||||
def test_flag_env_migrations_is_derived_from_server_specs():
|
||||
# The legacy constant is now a derived view; it must mirror the registry.
|
||||
assert set(c.FLAG_ENV_MIGRATIONS) == set(c.SERVER_SPECS)
|
||||
assert c.FLAG_ENV_MIGRATIONS["ssh-mcp"]["env"] == c.SERVER_SPECS["ssh-mcp"].env_flags
|
||||
assert c.FLAG_ENV_MIGRATIONS["ssh-mcp"]["removed"] == c.SERVER_SPECS["ssh-mcp"].removed_flags
|
||||
|
||||
|
||||
def test_resolve_server_spec_matches_detect_migratable_package():
|
||||
for data in (
|
||||
{"command": "npx", "args": ["-y", "ssh-mcp"]},
|
||||
{"command": "ssh-mcp", "args": []},
|
||||
{"command": "/usr/local/bin/ssh-mcp", "args": []},
|
||||
{"command": "npx", "args": ["some-other"]},
|
||||
{},
|
||||
{"url": "https://x"},
|
||||
):
|
||||
spec = c.resolve_server_spec(data)
|
||||
pkg = c.detect_migratable_package(data)
|
||||
assert (spec.package if spec else None) == pkg
|
||||
|
||||
|
||||
def test_drift_warning_maxchars_none_inline_and_separate():
|
||||
inline = {"command": "ssh-mcp", "args": ["--maxChars=none"]}
|
||||
separate = {"command": "ssh-mcp", "args": ["--maxChars", "none"]}
|
||||
for data in (inline, separate):
|
||||
warnings = c.drift_warnings(data)
|
||||
assert len(warnings) == 1
|
||||
assert "--maxChars=none" in warnings[0]
|
||||
assert "5000" in warnings[0]
|
||||
|
||||
|
||||
def test_drift_warning_quiet_for_other_values_and_unknown_pkg():
|
||||
# A real numeric cap is fine; only "none" drifts.
|
||||
assert c.drift_warnings({"command": "ssh-mcp", "args": ["--maxChars=8000"]}) == []
|
||||
assert c.drift_warnings({"command": "ssh-mcp", "args": ["--host=h"]}) == []
|
||||
# Unknown package: nothing to say.
|
||||
assert c.drift_warnings({"command": "npx", "args": ["other", "--maxChars=none"]}) == []
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Sidecar config detection + precedence (issue #91)
|
||||
# --------------------------------------------------------------------------- #
|
||||
_SSH = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h", "--user=u"]}
|
||||
_HOME = "/Users/tester"
|
||||
|
||||
|
||||
def test_sidecar_path_verified_per_platform():
|
||||
# NOTE: assert on .as_posix() — the CI matrix includes a Windows runner where
|
||||
# str(Path("/Users/…")) would render with backslashes. as_posix() normalises
|
||||
# separators so these platform-parameterised checks are portable.
|
||||
spec = c.SERVER_SPECS["ssh-mcp"]
|
||||
mac = c.sidecar_path(spec, platform="darwin", environ={}, home=_HOME)
|
||||
assert mac.as_posix() == "/Users/tester/Library/Application Support/ssh-mcp/config.toml"
|
||||
# Windows resolves under %APPDATA%, not ~/.config.
|
||||
win = c.sidecar_path(
|
||||
spec, platform="win32", environ={"APPDATA": "C:/Users/t/AppData/Roaming"}, home=_HOME
|
||||
)
|
||||
assert "ssh-mcp/config.toml" in win.as_posix()
|
||||
assert "AppData/Roaming" in win.as_posix()
|
||||
# POSIX honours XDG_CONFIG_HOME, else ~/.config.
|
||||
xdg = c.sidecar_path(spec, platform="linux", environ={"XDG_CONFIG_HOME": "/cfg"}, home=_HOME)
|
||||
assert xdg.as_posix() == "/cfg/ssh-mcp/config.toml"
|
||||
default = c.sidecar_path(spec, platform="linux", environ={}, home=_HOME)
|
||||
assert default.as_posix() == "/Users/tester/.config/ssh-mcp/config.toml"
|
||||
|
||||
|
||||
def test_sidecar_path_none_for_non_sidecar_package():
|
||||
assert c.sidecar_path(None) is None
|
||||
assert c.sidecar_path(c.ServerSpec(package="nope")) is None
|
||||
|
||||
|
||||
def test_sidecar_args_inert_when_file_exists():
|
||||
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||
st = c.sidecar_status(
|
||||
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == real
|
||||
)
|
||||
assert st["exists"] is True
|
||||
assert st["has_managed_args"] is True
|
||||
assert st["args_inert"] is True
|
||||
assert st["wrong_path"] is False
|
||||
warns = c.sidecar_warnings(
|
||||
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == real
|
||||
)
|
||||
assert warns and "inert" in warns[0]
|
||||
assert str(real) in warns[0]
|
||||
|
||||
|
||||
def test_sidecar_args_live_when_file_absent():
|
||||
st = c.sidecar_status(_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: False)
|
||||
assert st["exists"] is False
|
||||
assert st["args_inert"] is False
|
||||
assert (
|
||||
c.sidecar_warnings(_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: False)
|
||||
== []
|
||||
)
|
||||
|
||||
|
||||
def test_sidecar_wrong_path_flag_on_macos():
|
||||
# A TOML written at the README's ~/.config path is never read on macOS.
|
||||
doc = c.sidecar_doc_path(c.SERVER_SPECS["ssh-mcp"], home=_HOME)
|
||||
assert doc.as_posix() == "/Users/tester/.config/ssh-mcp/config.toml"
|
||||
st = c.sidecar_status(
|
||||
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == doc
|
||||
)
|
||||
assert st["wrong_path"] is True
|
||||
assert st["args_inert"] is False # the real file doesn't exist, so args still apply
|
||||
warns = c.sidecar_warnings(
|
||||
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == doc
|
||||
)
|
||||
assert warns and "never loaded" in warns[0]
|
||||
|
||||
|
||||
def test_sidecar_no_wrong_path_on_linux_where_paths_coincide():
|
||||
# On Linux the real path and the doc path are the same, so a file there is
|
||||
# correctly loaded — no wrong-path warning.
|
||||
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="linux", environ={}, home=_HOME)
|
||||
st = c.sidecar_status(
|
||||
_SSH, platform="linux", environ={}, home=_HOME, exists=lambda p: p == real
|
||||
)
|
||||
assert st["wrong_path"] is False
|
||||
assert st["args_inert"] is True
|
||||
|
||||
|
||||
def test_count_toml_profiles():
|
||||
assert c.count_toml_profiles("") == 0
|
||||
assert c.count_toml_profiles("[server]\nhost='h'\n") == 1
|
||||
text = "# comment\n[[hosts]]\nname='a'\n[[hosts]]\nname='b'\n[settings]\nx=1\n"
|
||||
# two [[hosts]] share a top-level name -> one profile group; [settings] -> another.
|
||||
assert c.count_toml_profiles(text) == 2
|
||||
multi = "[prod]\nhost='p'\n[prod.auth]\nkey='k'\n[staging]\nhost='s'\n"
|
||||
assert c.count_toml_profiles(multi) == 2
|
||||
|
||||
|
||||
def test_unscoped_credential_warning_11():
|
||||
data = dict(_SSH, env={"SSH_MCP_PASSWORD": "shared"})
|
||||
# Single profile: no sharing concern.
|
||||
assert c.unscoped_credential_warning(data, profile_count=1) is None
|
||||
# Unknown count: claim nothing.
|
||||
assert c.unscoped_credential_warning(data, profile_count=None) is None
|
||||
# Multiple profiles + bare credential: warn.
|
||||
w = c.unscoped_credential_warning(data, profile_count=3)
|
||||
assert w and "every one of the 3 profiles" in w
|
||||
# No bare credential: nothing to warn.
|
||||
assert c.unscoped_credential_warning(_SSH, profile_count=3) is None
|
||||
|
||||
|
||||
def test_sidecar_warnings_includes_credential_scoping():
|
||||
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||
data = dict(_SSH, env={"SSH_MCP_PASSWORD": "shared"})
|
||||
toml = "[prod]\nhost='p'\n[staging]\nhost='s'\n"
|
||||
warns = c.sidecar_warnings(
|
||||
data,
|
||||
platform="darwin",
|
||||
environ={},
|
||||
home=_HOME,
|
||||
exists=lambda p: p == real,
|
||||
read_text=lambda p: toml,
|
||||
)
|
||||
assert any("profiles" in w for w in warns)
|
||||
|
||||
|
||||
def test_sidecar_status_none_for_unknown_package():
|
||||
assert c.sidecar_status({"command": "npx", "args": ["other"]}) is None
|
||||
assert c.sidecar_warnings({"command": "npx", "args": ["other"]}) == []
|
||||
|
||||
|
||||
# Version resolve + pin + drift (issue #92)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_server_package_spec_and_name():
|
||||
assert c.server_package_spec({"command": "npx", "args": ["-y", "ssh-mcp"]}) == "ssh-mcp"
|
||||
assert c.server_package_spec({"command": "npx", "args": ["ssh-mcp@2.1.0"]}) == "ssh-mcp@2.1.0"
|
||||
assert c.server_package_name({"command": "npx", "args": ["ssh-mcp@2.1.0"]}) == "ssh-mcp"
|
||||
# A direct binary launch has no run-time spec to pin.
|
||||
assert c.server_package_spec({"command": "ssh-mcp", "args": []}) is None
|
||||
assert c.server_package_spec({"url": "https://x"}) is None
|
||||
|
||||
|
||||
def test_is_unpinned_spec():
|
||||
assert c.is_unpinned_spec({"command": "npx", "args": ["-y", "ssh-mcp"]}) is True
|
||||
assert c.is_unpinned_spec({"command": "npx", "args": ["ssh-mcp@latest"]}) is True
|
||||
assert c.is_unpinned_spec({"command": "npx", "args": ["ssh-mcp@next"]}) is True
|
||||
assert c.is_unpinned_spec({"command": "npx", "args": ["ssh-mcp@2.1.0"]}) is False
|
||||
assert c.is_unpinned_spec({"command": "ssh-mcp", "args": []}) is False
|
||||
|
||||
|
||||
def test_parse_package_json_version():
|
||||
assert c.parse_package_json_version('{"name":"ssh-mcp","version":"2.1.0"}') == "2.1.0"
|
||||
assert c.parse_package_json_version('{"name":"x"}') is None
|
||||
assert c.parse_package_json_version("not json") is None
|
||||
assert c.parse_package_json_version('{"version":""}') is None
|
||||
|
||||
|
||||
def test_resolved_npx_version_reads_cache_highest_wins():
|
||||
# Two cache entries for ssh-mcp; the highest version wins. No real filesystem.
|
||||
files = {
|
||||
"/h/.npm/_npx/aaa/node_modules/ssh-mcp/package.json": '{"version":"1.9.0"}',
|
||||
"/h/.npm/_npx/bbb/node_modules/ssh-mcp/package.json": '{"version":"2.1.0"}',
|
||||
}
|
||||
got = c.resolved_npx_version(
|
||||
"ssh-mcp",
|
||||
home="/h",
|
||||
find=lambda pat: list(files),
|
||||
read=lambda p: files[p],
|
||||
)
|
||||
assert got == "2.1.0"
|
||||
|
||||
|
||||
def test_resolved_npx_version_unknown_degrades_to_none():
|
||||
assert (
|
||||
c.resolved_npx_version("ssh-mcp", home="/h", find=lambda pat: [], read=lambda p: "") is None
|
||||
)
|
||||
assert c.resolved_npx_version("", home="/h") is None
|
||||
|
||||
|
||||
def test_pin_spec_transform():
|
||||
data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h"]}
|
||||
new, note = c.pin_spec_transform(data, "2.1.0")
|
||||
assert new["args"] == ["-y", "ssh-mcp@2.1.0", "--host=h"]
|
||||
assert note and "2.1.0" in note
|
||||
# Already pinned to that exact version -> no-op.
|
||||
again, note2 = c.pin_spec_transform(new, "2.1.0")
|
||||
assert again == new
|
||||
assert note2 is None
|
||||
# Bad / empty version -> no-op.
|
||||
assert c.pin_spec_transform(data, "")[1] is None
|
||||
assert c.pin_spec_transform(data, "latest")[1] is None
|
||||
# Not an npx server -> no-op.
|
||||
assert c.pin_spec_transform({"command": "ssh-mcp", "args": []}, "2.1.0")[1] is None
|
||||
|
||||
|
||||
def test_version_drift_note():
|
||||
assert c.version_drift_note("2.1.0", "3.0.0") == "moved 2.1.0 → 3.0.0 since you pinned"
|
||||
assert c.version_drift_note("3.0.0", "3.0.0") is None
|
||||
assert c.version_drift_note("3.0.0", "2.1.0") is None # never a backwards "drift"
|
||||
assert c.version_drift_note(None, "3.0.0") is None
|
||||
assert c.version_drift_note("2.1.0", None) is None
|
||||
# Numeric, not lexical: 2 < 10.
|
||||
assert c.version_drift_note("2.0.0", "10.0.0") is not None
|
||||
|
||||
|
||||
def test_version_status_unpinned_offers_pin():
|
||||
st = c.version_status({"command": "npx", "args": ["-y", "ssh-mcp"]}, resolved="2.1.0")
|
||||
assert st["package"] == "ssh-mcp"
|
||||
assert st["unpinned"] is True
|
||||
assert st["pinned_version"] is None
|
||||
assert st["resolved_version"] == "2.1.0"
|
||||
assert st["can_pin"] is True
|
||||
assert st["drift"] is None
|
||||
|
||||
|
||||
def test_version_status_pinned_reports_drift():
|
||||
st = c.version_status({"command": "npx", "args": ["ssh-mcp@2.1.0"]}, resolved="3.0.0")
|
||||
assert st["unpinned"] is False
|
||||
assert st["pinned_version"] == "2.1.0"
|
||||
assert st["can_pin"] is False # already pinned
|
||||
assert st["drift"] == "moved 2.1.0 → 3.0.0 since you pinned"
|
||||
|
||||
|
||||
def test_version_status_unknown_resolved_cannot_pin():
|
||||
st = c.version_status({"command": "npx", "args": ["-y", "ssh-mcp"]}, resolved=None)
|
||||
assert st["unpinned"] is True
|
||||
assert st["resolved_version"] is None
|
||||
assert st["can_pin"] is False # nothing to pin TO
|
||||
assert st["drift"] is None
|
||||
|
||||
|
||||
def test_version_status_none_for_non_npx():
|
||||
assert c.version_status({"command": "ssh-mcp", "args": []}) is None
|
||||
assert c.version_status({"url": "https://x"}) is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Filesystem permission pre-flight (issue #93)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_permission_status_ok_when_0600():
|
||||
# NOTE: key the injected mode map on Path(p).as_posix() — on the Windows CI
|
||||
# runner permission_status wraps the path in a WindowsPath, so str(p) would
|
||||
# use backslashes and miss the lookup (the same portability trap as #91).
|
||||
st = c.permission_status(
|
||||
"/cfg/config.toml",
|
||||
platform="darwin",
|
||||
stat_mode=lambda p: {"/cfg/config.toml": 0o600, "/cfg": 0o700}.get(Path(p).as_posix()),
|
||||
)
|
||||
assert st["ok"] is True
|
||||
assert st["mode"] == 0o600
|
||||
assert st["problems"] == []
|
||||
|
||||
|
||||
def test_permission_status_blocks_group_world_readable_file():
|
||||
st = c.permission_status(
|
||||
"/cfg/config.toml",
|
||||
platform="linux",
|
||||
stat_mode=lambda p: {"/cfg/config.toml": 0o644, "/cfg": 0o755}.get(Path(p).as_posix()),
|
||||
)
|
||||
assert st["ok"] is False
|
||||
assert st["file_ok"] is False
|
||||
assert st["dir_ok"] is False
|
||||
# Two plain-language problems, naming the offending octal modes.
|
||||
text = " ".join(st["problems"])
|
||||
assert "0644" in text and "0600" in text
|
||||
assert "0755" in text and "0700" in text
|
||||
|
||||
|
||||
def test_permission_status_file_bad_dir_ok():
|
||||
st = c.permission_status(
|
||||
"/cfg/config.toml",
|
||||
platform="linux",
|
||||
stat_mode=lambda p: {"/cfg/config.toml": 0o640, "/cfg": 0o700}.get(Path(p).as_posix()),
|
||||
)
|
||||
assert st["file_ok"] is False
|
||||
assert st["dir_ok"] is True
|
||||
assert len(st["problems"]) == 1
|
||||
|
||||
|
||||
def test_permission_status_none_on_windows_and_missing_file():
|
||||
# Windows: POSIX modes don't apply -> None (clean no-op).
|
||||
assert c.permission_status("/cfg/config.toml", platform="win32") is None
|
||||
# Absent file -> nothing to pre-flight.
|
||||
assert c.permission_status("/cfg/gone.toml", platform="linux", stat_mode=lambda p: None) is None
|
||||
|
||||
|
||||
def test_fix_permissions_chmods_file_and_dir():
|
||||
# as_posix() so the recorded paths compare equal on the Windows CI runner too.
|
||||
calls = []
|
||||
changed, note = c.fix_permissions(
|
||||
"/cfg/config.toml",
|
||||
platform="linux",
|
||||
chmod=lambda p, m: calls.append((Path(p).as_posix(), m)),
|
||||
)
|
||||
assert changed is True
|
||||
assert ("/cfg/config.toml", 0o600) in calls
|
||||
assert ("/cfg", 0o700) in calls
|
||||
assert note and "0600" in note
|
||||
|
||||
|
||||
def test_fix_permissions_noop_on_windows():
|
||||
calls = []
|
||||
changed, note = c.fix_permissions(
|
||||
"/cfg/config.toml", platform="win32", chmod=lambda p, m: calls.append((p, m))
|
||||
)
|
||||
assert changed is False
|
||||
assert note is None
|
||||
assert calls == []
|
||||
|
||||
|
||||
def test_fix_permissions_reports_oserror():
|
||||
def boom(p, m):
|
||||
raise OSError("nope")
|
||||
|
||||
changed, note = c.fix_permissions("/cfg/config.toml", platform="linux", chmod=boom)
|
||||
assert changed is False
|
||||
assert "could not change permissions" in note
|
||||
|
||||
|
||||
def test_sidecar_permission_warnings_over_real_path():
|
||||
data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h"]}
|
||||
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home="/Users/t")
|
||||
|
||||
def stat_mode(p):
|
||||
return 0o644 if Path(p) == real else 0o700
|
||||
|
||||
warns = c.sidecar_permission_warnings(
|
||||
data, platform="darwin", environ={}, home="/Users/t", stat_mode=stat_mode
|
||||
)
|
||||
assert warns and warns[0].startswith("ssh-mcp:")
|
||||
assert "0600" in warns[0]
|
||||
# Windows / unknown package -> nothing.
|
||||
assert c.sidecar_permission_warnings(data, platform="win32") == []
|
||||
assert c.sidecar_permission_warnings({"command": "npx", "args": ["other"]}) == []
|
||||
|
||||
|
||||
def test_sidecar_permission_warnings_quiet_when_tight():
|
||||
data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h"]}
|
||||
warns = c.sidecar_permission_warnings(
|
||||
data, platform="darwin", environ={}, home="/Users/t", stat_mode=lambda p: 0o600
|
||||
)
|
||||
assert warns == []
|
||||
|
||||
|
||||
def test_sidecar_permission_fix_target():
|
||||
data = {"command": "npx", "args": ["-y", "ssh-mcp"]}
|
||||
tgt = c.sidecar_permission_fix_target(data, platform="darwin", environ={}, home="/Users/t")
|
||||
assert tgt is not None and tgt.name == "config.toml"
|
||||
assert c.sidecar_permission_fix_target(data, platform="win32") is None
|
||||
assert c.sidecar_permission_fix_target({"command": "npx", "args": ["other"]}) is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Hot-reload — live external-change detection (issue #101)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_sidecar_watch_paths_covers_file_dir_and_doc():
|
||||
data = dict(_SSH)
|
||||
paths = c.sidecar_watch_paths(data, platform="darwin", environ={}, home=_HOME)
|
||||
posix = [p.as_posix() for p in paths]
|
||||
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||
# The resolved sidecar file AND its directory (so create/delete registers).
|
||||
assert real.as_posix() in posix
|
||||
assert real.parent.as_posix() in posix
|
||||
# The README/doc path is distinct on macOS -> also watched.
|
||||
doc = c.sidecar_doc_path(c.SERVER_SPECS["ssh-mcp"], home=_HOME)
|
||||
assert doc.as_posix() in posix
|
||||
# No duplicates.
|
||||
assert len(posix) == len(set(posix))
|
||||
|
||||
|
||||
def test_sidecar_watch_paths_dedupes_on_linux():
|
||||
# On Linux the doc path coincides with the real path, so the file + dir are
|
||||
# each listed once, not twice.
|
||||
data = dict(_SSH)
|
||||
paths = c.sidecar_watch_paths(
|
||||
data, platform="linux", environ={"XDG_CONFIG_HOME": "/cfg"}, home=_HOME
|
||||
)
|
||||
posix = [p.as_posix() for p in paths]
|
||||
assert posix == list(dict.fromkeys(posix)) # order-preserving de-dup is a no-op
|
||||
assert "/cfg/ssh-mcp/config.toml" in posix
|
||||
assert "/cfg/ssh-mcp" in posix
|
||||
|
||||
|
||||
def test_sidecar_watch_paths_empty_for_non_sidecar():
|
||||
assert c.sidecar_watch_paths({"command": "npx", "args": ["other"]}) == []
|
||||
|
||||
|
||||
def test_sidecar_state_fingerprint_reflects_appearance_and_perms():
|
||||
data = dict(_SSH)
|
||||
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||
|
||||
# File absent -> a fingerprint that encodes "not there, no perms".
|
||||
absent = c.sidecar_state_fingerprint(
|
||||
data, platform="darwin", environ={}, home=_HOME, exists=lambda p: False
|
||||
)
|
||||
# File present but world-readable.
|
||||
bad = c.sidecar_state_fingerprint(
|
||||
data,
|
||||
platform="darwin",
|
||||
environ={},
|
||||
home=_HOME,
|
||||
exists=lambda p: Path(p) == real,
|
||||
stat_mode=lambda p: 0o644 if Path(p) == real else 0o700,
|
||||
)
|
||||
# File present and tight.
|
||||
good = c.sidecar_state_fingerprint(
|
||||
data,
|
||||
platform="darwin",
|
||||
environ={},
|
||||
home=_HOME,
|
||||
exists=lambda p: Path(p) == real,
|
||||
stat_mode=lambda p: 0o600 if Path(p) == real else 0o700,
|
||||
)
|
||||
assert absent is not None
|
||||
# Each observable transition changes the fingerprint.
|
||||
assert c.sidecar_state_changed(absent, bad)
|
||||
assert c.sidecar_state_changed(bad, good)
|
||||
assert c.sidecar_state_changed(absent, good)
|
||||
# Stable when nothing changed.
|
||||
assert not c.sidecar_state_changed(good, good)
|
||||
|
||||
|
||||
def test_sidecar_state_fingerprint_none_for_non_sidecar():
|
||||
assert c.sidecar_state_fingerprint({"command": "npx", "args": ["other"]}) is None
|
||||
|
||||
|
||||
def test_sidecar_state_fingerprint_tracks_args_inert_flip():
|
||||
# args_inert only becomes true once the sidecar file exists AND managed args
|
||||
# are present; the fingerprint must capture that precedence flip.
|
||||
data = dict(_SSH) # carries --host/--user (managed args)
|
||||
real = c.sidecar_path(
|
||||
c.SERVER_SPECS["ssh-mcp"], platform="linux", environ={"XDG_CONFIG_HOME": "/cfg"}, home=_HOME
|
||||
)
|
||||
before = c.sidecar_state_fingerprint(
|
||||
data,
|
||||
platform="linux",
|
||||
environ={"XDG_CONFIG_HOME": "/cfg"},
|
||||
home=_HOME,
|
||||
exists=lambda p: False,
|
||||
stat_mode=lambda p: None,
|
||||
)
|
||||
after = c.sidecar_state_fingerprint(
|
||||
data,
|
||||
platform="linux",
|
||||
environ={"XDG_CONFIG_HOME": "/cfg"},
|
||||
home=_HOME,
|
||||
exists=lambda p: Path(p) == real,
|
||||
stat_mode=lambda p: 0o600,
|
||||
)
|
||||
assert c.sidecar_state_changed(before, after)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Sidecar TOML editing (issue #102)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_read_toml_section_top_level_and_named():
|
||||
text = (
|
||||
"# a comment\n"
|
||||
'title = "root"\n'
|
||||
"port = 22\n"
|
||||
"enabled = true\n"
|
||||
"\n"
|
||||
"[server]\n"
|
||||
'host = "h"\n'
|
||||
"auth = 'key'\n" # literal string
|
||||
"weight = 1.5\n"
|
||||
)
|
||||
top = c.read_toml_section(text)
|
||||
assert top == {"title": "root", "port": 22, "enabled": True}
|
||||
srv = c.read_toml_section(text, "server")
|
||||
assert srv == {"host": "h", "auth": "key", "weight": 1.5}
|
||||
# An absent section reads as empty.
|
||||
assert c.read_toml_section(text, "nope") == {}
|
||||
|
||||
|
||||
def test_read_toml_section_skips_unparseable_values():
|
||||
text = '[server]\nhosts = [1, 2, 3]\ninline = {a = 1}\nname = "ok"\n'
|
||||
# Arrays / inline tables are omitted (surgical writer preserves them); the
|
||||
# simple scalar is read.
|
||||
assert c.read_toml_section(text, "server") == {"name": "ok"}
|
||||
|
||||
|
||||
def test_toml_sections_lists_groups_in_order():
|
||||
text = "top = 1\n[server]\nx=1\n[prod]\na=1\n[prod.auth]\nk=1\n[[hosts]]\nn=1\n"
|
||||
# top-level present -> "" first; [prod] and [prod.auth] collapse to one group.
|
||||
assert c.toml_sections(text) == ["", "server", "prod", "hosts"]
|
||||
assert c.toml_sections("[only]\nx=1\n") == ["only"]
|
||||
|
||||
|
||||
def test_set_toml_value_replaces_and_preserves_comment():
|
||||
text = '[server]\nport = 22 # ssh\nhost = "h"\n'
|
||||
out = c.set_toml_value(text, "port", 2222, section="server")
|
||||
assert "port = 2222 # ssh" in out
|
||||
# Nothing else changed.
|
||||
assert 'host = "h"' in out
|
||||
assert out.startswith("[server]\n")
|
||||
|
||||
|
||||
def test_set_toml_value_appends_missing_key_within_section():
|
||||
text = '[server]\nhost = "h"\n\n[other]\nx = 1\n'
|
||||
out = c.set_toml_value(text, "auth", "key", section="server")
|
||||
lines = out.splitlines()
|
||||
# The new key lands inside [server], before the blank line and [other].
|
||||
assert lines.index('auth = "key"') < lines.index("[other]")
|
||||
assert lines.index('auth = "key"') > lines.index("[server]")
|
||||
assert "[other]" in out and "x = 1" in out
|
||||
|
||||
|
||||
def test_set_toml_value_creates_missing_section():
|
||||
text = 'title = "root"\n'
|
||||
out = c.set_toml_value(text, "role", "admin", section="server")
|
||||
assert 'title = "root"' in out
|
||||
assert "[server]" in out
|
||||
assert 'role = "admin"' in out
|
||||
# And it round-trips back through the reader.
|
||||
assert c.read_toml_section(out, "server") == {"role": "admin"}
|
||||
|
||||
|
||||
def test_set_toml_value_top_level_key():
|
||||
text = "a = 1\n[server]\nb = 2\n"
|
||||
out = c.set_toml_value(text, "a", 5) # section=None -> top-level
|
||||
assert out.startswith("a = 5\n")
|
||||
# The [server] b is untouched.
|
||||
assert c.read_toml_section(out, "server") == {"b": 2}
|
||||
|
||||
|
||||
def test_set_toml_value_delete_key():
|
||||
text = '[server]\nhost = "h"\nport = 22\n'
|
||||
out = c.set_toml_value(text, "port", None, section="server")
|
||||
assert "port" not in out
|
||||
assert 'host = "h"' in out
|
||||
# Deleting an absent key is a no-op.
|
||||
assert c.set_toml_value(text, "ghost", None, section="server") == text
|
||||
|
||||
|
||||
def test_set_toml_value_preserves_crlf():
|
||||
text = "[server]\r\nport = 22\r\n"
|
||||
out = c.set_toml_value(text, "port", 23, section="server")
|
||||
assert "port = 23\r\n" in out
|
||||
assert "\r\n" in out
|
||||
|
||||
|
||||
def test_set_toml_value_escapes_strings():
|
||||
out = c.set_toml_value("", "path", 'C:\\a\\"b"', section="win")
|
||||
# Backslashes and quotes are escaped; it reads back to the exact original.
|
||||
assert c.read_toml_section(out, "win") == {"path": 'C:\\a\\"b"'}
|
||||
|
||||
|
||||
def test_update_toml_batch_and_roundtrip():
|
||||
text = '[server]\nhost = "h" # keep me\n'
|
||||
out = c.update_toml(text, {"host": "newhost", "port": 22, "auth": "key"}, section="server")
|
||||
assert c.read_toml_section(out, "server") == {"host": "newhost", "port": 22, "auth": "key"}
|
||||
# The comment on the pre-existing host line survives the value change.
|
||||
assert "# keep me" in out
|
||||
|
||||
|
||||
def test_validate_sidecar_values_enums_and_port():
|
||||
schema = c.SERVER_SPECS["ssh-mcp"].schema
|
||||
# All valid -> no problems.
|
||||
assert (
|
||||
c.validate_sidecar_values(
|
||||
{"auth": "key", "approvalMode": "ask-all", "role": "admin", "port": 22}, schema
|
||||
)
|
||||
== []
|
||||
)
|
||||
# Bad enum value.
|
||||
probs = c.validate_sidecar_values({"auth": "sshkey"}, schema)
|
||||
assert probs and "auth" in probs[0] and "sshkey" in probs[0]
|
||||
# Out-of-range port, and a bool is not a valid int port.
|
||||
assert c.validate_sidecar_values({"port": 70000}, schema)
|
||||
assert c.validate_sidecar_values({"port": True}, schema)
|
||||
assert c.validate_sidecar_values({"port": 1}, schema) == []
|
||||
|
||||
|
||||
def test_validate_sidecar_values_passes_through_unknown_keys():
|
||||
schema = c.SERVER_SPECS["ssh-mcp"].schema
|
||||
# An unmanaged key is preserved, never a save-blocker.
|
||||
assert c.validate_sidecar_values({"customThing": "whatever"}, schema) == []
|
||||
|
||||
|
||||
def test_write_sidecar_atomic_backup_and_chmod(tmp_path):
|
||||
p = tmp_path / "ssh-mcp" / "config.toml"
|
||||
p.parent.mkdir()
|
||||
p.write_text("[server]\nport = 22\n", encoding="utf-8")
|
||||
|
||||
chmods: list[tuple[str, int]] = []
|
||||
new_text = c.set_toml_value(p.read_text(encoding="utf-8"), "port", 2222, section="server")
|
||||
backup = c.write_sidecar(
|
||||
p,
|
||||
new_text,
|
||||
platform="linux",
|
||||
chmod=lambda path, mode: chmods.append((Path(path).as_posix(), mode)),
|
||||
)
|
||||
|
||||
# File updated atomically with the new content.
|
||||
assert c.read_toml_section(p.read_text(encoding="utf-8"), "server") == {"port": 2222}
|
||||
# A timestamped backup of the prior content was made, under .toml.
|
||||
assert backup is not None and backup.suffix == ".toml"
|
||||
assert c.read_toml_section(backup.read_text(encoding="utf-8"), "server") == {"port": 22}
|
||||
# Permissions tightened: file 0600, dir 0700 (via #93's fix_permissions).
|
||||
assert (p.as_posix(), 0o600) in chmods
|
||||
assert (p.parent.as_posix(), 0o700) in chmods
|
||||
|
||||
|
||||
def test_write_sidecar_new_file_no_backup(tmp_path):
|
||||
p = tmp_path / "config.toml"
|
||||
backup = c.write_sidecar(
|
||||
p, '[server]\nrole = "viewer"\n', platform="linux", chmod=lambda *_: None
|
||||
)
|
||||
assert backup is None # nothing pre-existing to back up
|
||||
assert p.read_text(encoding="utf-8").endswith("\n")
|
||||
assert c.read_toml_section(p.read_text(encoding="utf-8"), "server") == {"role": "viewer"}
|
||||
|
||||
|
||||
def test_write_sidecar_does_not_touch_apply_servers(tmp_path):
|
||||
# Guard the cardinal rule: write_sidecar writes ONLY the text it is given —
|
||||
# no mcpServers / _disabledMcpServers keys are introduced.
|
||||
p = tmp_path / "config.toml"
|
||||
c.write_sidecar(p, '[server]\nhost = "h"\n', platform="linux", chmod=lambda *_: None)
|
||||
body = p.read_text(encoding="utf-8")
|
||||
assert "mcpServers" not in body and "_disabledMcpServers" not in body
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Move to environment variable (issue #83)
|
||||
# --------------------------------------------------------------------------- #
|
||||
@pytest.mark.parametrize(
|
||||
"raw,expected",
|
||||
[
|
||||
("API_KEY", "API_KEY"),
|
||||
("api-key", "API_KEY"),
|
||||
("x.y z", "X_Y_Z"),
|
||||
("2fa", "_2FA"),
|
||||
("", "VAR"),
|
||||
("***", "VAR"),
|
||||
("clé", "CL_"), # non-ASCII becomes _
|
||||
],
|
||||
)
|
||||
def test_sanitize_env_var_name(raw, expected):
|
||||
assert c.sanitize_env_var_name(raw) == expected
|
||||
|
||||
|
||||
def test_shell_export_lines_quote_safely():
|
||||
lines = c.shell_export_lines("TOKEN", "ab'cd")
|
||||
assert lines["posix"] == "export TOKEN='ab'\\''cd'"
|
||||
assert lines["windows"] == 'setx TOKEN "ab\'cd"'
|
||||
|
||||
|
||||
def test_can_move_gate_requires_secret_and_expanding_client():
|
||||
desktop = c.Profile(label="d", path="/x/Claude/claude_desktop_config.json", config_exists=True)
|
||||
code = c.Profile(label="c", path=Path.home() / ".claude.json", config_exists=True)
|
||||
# real secret on an expanding client -> offer
|
||||
assert c.can_move_value_to_env_ref("API_KEY", "ghp_abc", code) is True
|
||||
assert c.can_move_value_to_env_ref("API_KEY", "ghp_abc", None) is True
|
||||
# non-secret key -> no
|
||||
assert c.can_move_value_to_env_ref("REGION", "us-east-1", code) is False
|
||||
# already a reference -> no
|
||||
assert c.can_move_value_to_env_ref("API_KEY", "${API_KEY}", code) is False
|
||||
# non-expanding client (Claude Desktop) -> refuse even a real secret
|
||||
assert c.can_move_value_to_env_ref("API_KEY", "ghp_abc", desktop) is False
|
||||
|
||||
|
||||
def test_move_env_value_replaces_with_reference_and_returns_secret():
|
||||
data = {"command": "x", "env": {"API_KEY": "ghp_secret", "REGION": "us"}}
|
||||
conv = c.move_value_to_env_ref(data, field="env", key="API_KEY")
|
||||
assert conv is not None
|
||||
assert conv.var_name == "API_KEY"
|
||||
assert conv.reference == "${API_KEY}"
|
||||
assert conv.secret == "ghp_secret"
|
||||
assert conv.data["env"]["API_KEY"] == "${API_KEY}"
|
||||
# non-secret row untouched
|
||||
assert conv.data["env"]["REGION"] == "us"
|
||||
# input never mutated
|
||||
assert data["env"]["API_KEY"] == "ghp_secret"
|
||||
|
||||
|
||||
def test_move_derives_and_sanitises_var_name_from_key():
|
||||
data = {"headers": {"x-api-key": "sekret"}}
|
||||
conv = c.move_value_to_env_ref(data, field="headers", key="x-api-key")
|
||||
assert conv.var_name == "X_API_KEY"
|
||||
assert conv.data["headers"]["x-api-key"] == "${X_API_KEY}"
|
||||
|
||||
|
||||
def test_move_honours_explicit_var_name():
|
||||
data = {"env": {"tok": "sekret"}}
|
||||
conv = c.move_value_to_env_ref(data, field="env", key="tok", var_name="GITHUB_TOKEN")
|
||||
assert conv.reference == "${GITHUB_TOKEN}"
|
||||
assert conv.data["env"]["tok"] == "${GITHUB_TOKEN}"
|
||||
|
||||
|
||||
def test_move_args_by_index():
|
||||
data = {"command": "x", "args": ["--token", "ghp_secret"]}
|
||||
conv = c.move_value_to_env_ref(data, field="args", index=1, var_name="GH_TOKEN")
|
||||
assert conv.secret == "ghp_secret"
|
||||
assert conv.data["args"] == ["--token", "${GH_TOKEN}"]
|
||||
|
||||
|
||||
def test_move_returns_none_on_missing_or_nonstring_or_already_ref():
|
||||
data = {"env": {"API_KEY": "${API_KEY}", "N": 5}}
|
||||
assert c.move_value_to_env_ref(data, field="env", key="ABSENT") is None
|
||||
assert c.move_value_to_env_ref(data, field="env", key="N") is None # not a string
|
||||
assert c.move_value_to_env_ref(data, field="env", key="API_KEY") is None # already a ref
|
||||
assert c.move_value_to_env_ref({}, field="bogus") is None
|
||||
assert c.move_value_to_env_ref({"args": ["a"]}, field="args", index=9) is None
|
||||
|
||||
|
||||
def test_is_env_var_set():
|
||||
assert c.is_env_var_set("FOO", {"FOO": "x"}) is True
|
||||
assert c.is_env_var_set("FOO", {"FOO": ""}) is False
|
||||
assert c.is_env_var_set("FOO", {}) is False
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Args secret indices + suggested var name (issue #83, args surface)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_secret_arg_indices_flags_token_and_flag_value():
|
||||
args = ["--port", "8080", "ghp_deadbeef", "--token", "sk-abc", "--flag=val"]
|
||||
idxs = c.secret_arg_indices(args)
|
||||
assert 2 in idxs # ghp_ token prefix
|
||||
assert 4 in idxs # value following --token
|
||||
assert 1 not in idxs # 8080
|
||||
assert 5 not in idxs # --flag=val inline pair
|
||||
|
||||
|
||||
def test_secret_arg_indices_flags_embedded_url_credentials():
|
||||
args = ["postgres://user:pass@host/db"]
|
||||
assert c.secret_arg_indices(args) == [0]
|
||||
|
||||
|
||||
def test_secret_arg_indices_excludes_existing_references():
|
||||
args = ["--token", "${GH_TOKEN}"]
|
||||
assert c.secret_arg_indices(args) == []
|
||||
|
||||
|
||||
def test_suggested_env_var_for_arg_uses_preceding_flag():
|
||||
args = ["--api-key", "sk-secret"]
|
||||
assert c.suggested_env_var_for_arg(args, 1) == "API_KEY"
|
||||
|
||||
|
||||
def test_suggested_env_var_for_arg_falls_back_when_no_flag():
|
||||
args = ["ghp_secret"]
|
||||
assert c.suggested_env_var_for_arg(args, 0) == "SECRET"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Referenced-variables readout + args->env relocation (issue #83, "both")
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_referenced_env_vars_dedupes_and_reports_status():
|
||||
data = {
|
||||
"command": "npx",
|
||||
"args": ["--token", "${GH_TOKEN}", "${GH_TOKEN}"],
|
||||
"env": {"API_KEY": "${API_KEY}", "REGION": "${REGION:-us-east-1}"},
|
||||
}
|
||||
usages = c.referenced_env_vars(data, environ={"GH_TOKEN": "x"})
|
||||
by = {u.name: u for u in usages}
|
||||
assert set(by) == {"GH_TOKEN", "API_KEY", "REGION"}
|
||||
# GH_TOKEN appears only in args, deduped to one entry, set in env -> resolved
|
||||
assert by["GH_TOKEN"].fields == ("args",)
|
||||
assert by["GH_TOKEN"].resolved is True
|
||||
# API_KEY not set, no default -> unresolved
|
||||
assert by["API_KEY"].resolved is False
|
||||
# REGION has a default -> resolved regardless of environment
|
||||
assert by["REGION"].has_default is True
|
||||
assert by["REGION"].resolved is True
|
||||
# sorted by name
|
||||
assert [u.name for u in usages] == sorted(u.name for u in usages)
|
||||
|
||||
|
||||
def test_referenced_env_vars_empty_when_no_refs():
|
||||
assert c.referenced_env_vars({"command": "npx", "args": ["-y", "pkg"]}) == []
|
||||
|
||||
|
||||
def test_move_arg_to_env_block_removes_flag_and_value():
|
||||
data = {"command": "x", "args": ["--api-key", "sk-secret", "run"], "env": {"KEEP": "1"}}
|
||||
out = c.move_arg_to_env_block(data, 1)
|
||||
assert out["args"] == ["run"] # flag + value both gone
|
||||
assert out["env"]["API_KEY"] == "sk-secret"
|
||||
assert out["env"]["KEEP"] == "1" # existing env preserved
|
||||
# input not mutated
|
||||
assert data["args"] == ["--api-key", "sk-secret", "run"]
|
||||
|
||||
|
||||
def test_move_arg_to_env_block_bare_positional_keeps_no_flag():
|
||||
data = {"command": "x", "args": ["ghp_secret", "serve"]}
|
||||
out = c.move_arg_to_env_block(data, 0, var_name="GITHUB_TOKEN")
|
||||
assert out["args"] == ["serve"]
|
||||
assert out["env"] == {"GITHUB_TOKEN": "ghp_secret"}
|
||||
|
||||
|
||||
def test_move_arg_to_env_block_none_on_bad_target():
|
||||
assert c.move_arg_to_env_block({"args": ["a"]}, 5) is None
|
||||
assert c.move_arg_to_env_block({"args": ["${REF}"]}, 0) is None # already a ref
|
||||
assert c.move_arg_to_env_block({}, 0) is None
|
||||
|
||||
Reference in New Issue
Block a user