Merge pull request 'feat(#88): detect & migrate removed CLI flags into env (ssh-mcp v2)' (#89) from feat/removed-flag-env-migration into main

bcc.py

@@ -886,6 +886,19 @@ class ServerEditor(QFrame):
         self.secret_warn.setWordWrap(True)
         self.secret_warn.hide()
         v.addWidget(self.secret_warn)
+        # Shown when args carry a CLI flag a package removed in a major version
+        # (e.g. ssh-mcp v2's --password), with a one-click move into env.
+        self.removed_flag_warn = QLabel("")
+        self.removed_flag_warn.setStyleSheet(f"color: {WARN};")
+        self.removed_flag_warn.setWordWrap(True)
+        self.removed_flag_warn.hide()
+        self.removed_flag_fix_btn = QPushButton("Fix: move to environment variables")
+        self.removed_flag_fix_btn.clicked.connect(self._fix_removed_flags)
+        self.removed_flag_fix_btn.hide()
+        rf_row = QHBoxLayout()
+        rf_row.addWidget(self.removed_flag_warn, 1)
+        rf_row.addWidget(self.removed_flag_fix_btn)
+        v.addLayout(rf_row)
         v.addWidget(self._lbl("Environment variables"))
         self.env = KeyValueTable(
             "Variable", "Value", on_change=self._emit, before_change=self._before_change
@@ -985,6 +998,8 @@ class ServerEditor(QFrame):
             self.args_warn.hide()
             self.args_fix_btn.hide()
             self.secret_warn.hide()
+            self.removed_flag_warn.hide()
+            self.removed_flag_fix_btn.hide()
             self._loading = False
             return
         self.setEnabled(True)
@@ -1072,6 +1087,8 @@ class ServerEditor(QFrame):
             self.args_warn.hide()
             self.args_fix_btn.hide()
             self.secret_warn.hide()
+            self.removed_flag_warn.hide()
+            self.removed_flag_fix_btn.hide()
             return
         _, notes = core.split_suspicious_args(self._current_arg_lines())
         if notes:
@@ -1087,6 +1104,23 @@ class ServerEditor(QFrame):
             self.secret_warn.show()
         else:
             self.secret_warn.hide()
+        # Flags a package removed across a major version (e.g. ssh-mcp v2's
+        # --password). Needs command + env too, so build from the live form.
+        stdio = {
+            "command": self.command.text().strip(),
+            "args": self._current_arg_lines(),
+            "env": self.env.dump(),
+        }
+        rf_warnings = core.removed_flag_warnings(stdio)
+        if rf_warnings:
+            self.removed_flag_warn.setText("⚠  " + "\n".join(rf_warnings))
+            self.removed_flag_warn.show()
+            # Only offer the button when something is actually auto-migratable.
+            migrated, _ = core.migrate_removed_flags(stdio)
+            self.removed_flag_fix_btn.setVisible(migrated is not stdio)
+        else:
+            self.removed_flag_warn.hide()
+            self.removed_flag_fix_btn.hide()
 
     def _fix_args(self):
         if self._before_change:
@@ -1094,6 +1128,19 @@ class ServerEditor(QFrame):
         fixed, _ = core.split_suspicious_args(self._current_arg_lines())
         self.args.setPlainText("\n".join(fixed))  # triggers _emit -> recheck
 
+    def _fix_removed_flags(self):
+        if self._before_change:
+            self._before_change()
+        stdio = {
+            "command": self.command.text().strip(),
+            "args": self._current_arg_lines(),
+            "env": self.env.dump(),
+        }
+        migrated, _ = core.migrate_removed_flags(stdio)
+        # Reload env first, then args; setting args text triggers _emit -> recheck.
+        self.env.load(migrated.get("env", {}))
+        self.args.setPlainText("\n".join(migrated.get("args", [])))
+
     # --- dependency ------------------------------------------------------ #
     def refresh_dependency(self, auto_open=False):
         if not self.isEnabled():
@@ -3016,6 +3063,8 @@ class MainWindow(QMainWindow):
         for entry in self.servers:
             for warning in core.env_ref_warnings(entry.data, self.current_profile):
                 lint_warnings.append(f"'{entry.name}': {warning}")
+            for warning in core.removed_flag_warnings(entry.data):
+                lint_warnings.append(f"'{entry.name}': {warning}")
         if lint_warnings:
             self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
             self.validation_lbl.setStyleSheet(f"color: {WARN};")

bcc_core.py

@@ -2217,6 +2217,205 @@ def _looks_like_multiple_args(a: str) -> bool:
     return len(toks) > 1 and any(t.startswith("-") for t in toks)
 
 
+# --------------------------------------------------------------------------- #
+# Removed-flag → env migration
+#
+# Some MCP servers moved credential CLI flags into environment variables across
+# a major version (secrets on the command line are visible to any local user in
+# process listings). A config written for the old version then fails hard on the
+# new one — e.g. ssh-mcp v2 exits with "These flags were removed in v2:
+# --password". This registry lets BCC recognise that shape, warn about it, and
+# offer a one-click migration that lifts the value out of `args` into `env`.
+#
+# Per package:
+#   "env"      removed flag -> env var that now supplies it. BCC auto-migrates
+#              these: the flag (and its value) leave `args`, the value lands in
+#              `env` under the mapped name.
+#   "removed"  flags that no longer exist but have no confirmed 1:1 env
+#              replacement (e.g. a boolean, or an env name not documented here).
+#              BCC only warns; the user resolves these by hand.
+#
+# Extending it: add a package key and its flag maps. Keep "env" limited to
+# mappings whose env-var name is verified, so the auto-fix stays trustworthy.
+# --------------------------------------------------------------------------- #
+FLAG_ENV_MIGRATIONS: dict[str, dict[str, dict[str, str]]] = {
+    "ssh-mcp": {
+        "env": {
+            "--password": "SSH_MCP_PASSWORD",
+        },
+        "removed": {
+            "--sudoPassword": (
+                "removed in ssh-mcp v2 — supply it via an environment variable "
+                "instead (see the ssh-mcp README's 'Migrating from v1' section)"
+            ),
+            "--suPassword": (
+                "removed in ssh-mcp v2 — supply it via an environment variable "
+                "instead (see the ssh-mcp README's 'Migrating from v1' section)"
+            ),
+            "--disableSudo": (
+                "removed in ssh-mcp v2 — sudo is now governed by the server "
+                "config/policy rather than a CLI flag"
+            ),
+        },
+    },
+}
+
+
+def _normalize_pkg_token(token: str) -> str:
+    """Reduce an argv token to a bare npm package name for registry lookup.
+
+    Strips any leading path (``/usr/local/bin/ssh-mcp`` -> ``ssh-mcp``) and a
+    trailing ``@version`` (``ssh-mcp@2.0.1`` -> ``ssh-mcp``), while preserving a
+    leading scope (``@acme/ssh-mcp`` stays intact).
+    """
+    t = str(token or "").strip()
+    if not t or t.startswith("-"):
+        return ""
+    # Drop a path prefix but keep an npm scope (leading '@' with no earlier '/').
+    if "/" in t and not t.startswith("@"):
+        t = t.rsplit("/", 1)[-1]
+    # Strip a trailing @version. For scoped names, only the version '@' counts:
+    # split on the LAST '@' when it isn't the scope's leading one.
+    at = t.rfind("@")
+    if at > 0:  # >0 so a scope's leading '@' at index 0 is untouched
+        t = t[:at]
+    return t
+
+
+def detect_migratable_package(data: dict) -> str | None:
+    """Return the FLAG_ENV_MIGRATIONS key this stdio server runs, or None.
+
+    Scans the command and every arg (so ``npx -y ssh-mcp`` and a direct
+    ``command: ssh-mcp`` both resolve), matching on the normalised package name.
+    """
+    if not isinstance(data, dict):
+        return None
+    tokens = [data.get("command", "")]
+    tokens.extend(data.get("args") or [])
+    for tok in tokens:
+        name = _normalize_pkg_token(tok)
+        if name in FLAG_ENV_MIGRATIONS:
+            return name
+    return None
+
+
+def migrate_removed_flags(data: dict) -> tuple[dict, list[str]]:
+    """Move known removed credential flags out of `args` and into `env`.
+
+    Returns ``(new_data, notes)``. When there is nothing to migrate the original
+    dict is returned unchanged with an empty notes list, so callers can cheaply
+    treat an empty notes list as "no change".
+
+    Only flags in the package's ``env`` map are rewritten. Both spellings are
+    handled: ``--password secret`` (value on the next token) and
+    ``--password=secret`` (inline). An existing ``env`` value for the target
+    name is never overwritten — the redundant flag is dropped and noted instead.
+    A ``${VAR}`` reference is migrated verbatim (env is the right home for it).
+    Flags listed under ``removed`` are left untouched here; see
+    ``removed_flag_warnings`` for those.
+    """
+    pkg = detect_migratable_package(data)
+    if pkg is None:
+        return data, []
+    env_map = FLAG_ENV_MIGRATIONS[pkg]["env"]
+    args = [str(a) for a in (data.get("args") or [])]
+    if not args:
+        return data, []
+
+    new_args: list[str] = []
+    new_env: dict = dict(data.get("env") or {})
+    notes: list[str] = []
+    i = 0
+    n = len(args)
+    changed = False
+    while i < n:
+        a = args[i]
+        # Inline form: --flag=value
+        if a.startswith("-") and "=" in a and a.split("=", 1)[0] in env_map:
+            flag, value = a.split("=", 1)
+            env_name = env_map[flag]
+            changed = True
+            if env_name in new_env and new_env[env_name] != value:
+                notes.append(f"{flag} dropped from args ({env_name} is already set in env)")
+            else:
+                new_env[env_name] = value
+                notes.append(f"moved {flag} into env as {env_name}")
+            i += 1
+            continue
+        # Separate form: --flag  value
+        if a in env_map:
+            flag = a
+            env_name = env_map[flag]
+            has_value = i + 1 < n and not args[i + 1].startswith("-")
+            if not has_value:
+                # No value to move (unexpected for a credential flag). Drop the
+                # bare flag so the server can start, and say so.
+                changed = True
+                notes.append(f"removed {flag} from args (no value found to move)")
+                i += 1
+                continue
+            value = args[i + 1]
+            changed = True
+            if env_name in new_env and new_env[env_name] != value:
+                notes.append(f"{flag} dropped from args ({env_name} is already set in env)")
+            else:
+                new_env[env_name] = value
+                notes.append(f"moved {flag} into env as {env_name}")
+            i += 2
+            continue
+        new_args.append(a)
+        i += 1
+
+    if not changed:
+        return data, []
+
+    new_data = dict(data)
+    if new_args:
+        new_data["args"] = new_args
+    else:
+        new_data.pop("args", None)
+    if new_env:
+        new_data["env"] = new_env
+    return new_data, notes
+
+
+def removed_flag_warnings(data: dict) -> list[str]:
+    """Advisory lines for every removed-in-a-major-version flag still present.
+
+    Covers both the auto-migratable flags (reported as a one-click fix being
+    available) and the manual-only ones. Empty list == nothing to flag.
+    """
+    pkg = detect_migratable_package(data)
+    if pkg is None:
+        return []
+    spec = FLAG_ENV_MIGRATIONS[pkg]
+    env_map = spec["env"]
+    removed_map = spec.get("removed", {})
+    present = set()
+    for a in data.get("args") or []:
+        s = str(a)
+        present.add(s.split("=", 1)[0] if s.startswith("-") and "=" in s else s)
+
+    out: list[str] = []
+    migratable = [f for f in env_map if f in present]
+    if migratable:
+        flags = ", ".join(sorted(migratable))
+        out.append(
+            f"{pkg}: {flags} was removed from the command line — its value should "
+            f"live in an environment variable. Use “Fix” to move it into env."
+        )
+    for flag in sorted(removed_map):
+        if flag in present:
+            out.append(f"{pkg}: {flag} {removed_map[flag]}")
+    return out
+
+
+def removed_flag_warning(data: dict) -> str | None:
+    """First removed-flag advisory for a single-line UI label, or None."""
+    warnings = removed_flag_warnings(data)
+    return warnings[0] if warnings else None
+
+
 # --------------------------------------------------------------------------- #
 # Validation
 # --------------------------------------------------------------------------- #

tests/test_core.py

@@ -3061,6 +3061,153 @@ def test_discover_project_configs_skips_non_object_and_garbage(tmp_path):
     assert str(missing / ".mcp.json") not in paths
 
 
+# --------------------------------------------------------------------------- #
+# Removed-flag -> env migration (ssh-mcp v2 and the general mechanism)
+# --------------------------------------------------------------------------- #
+def test_detect_migratable_package_via_npx_args():
+    data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h", "--password=p"]}
+    assert c.detect_migratable_package(data) == "ssh-mcp"
+
+
+def test_detect_migratable_package_direct_command_and_path_and_version():
+    assert c.detect_migratable_package({"command": "ssh-mcp", "args": []}) == "ssh-mcp"
+    assert (
+        c.detect_migratable_package({"command": "/usr/local/bin/ssh-mcp", "args": []}) == "ssh-mcp"
+    )
+    assert (
+        c.detect_migratable_package({"command": "npx", "args": ["-y", "ssh-mcp@2.0.1"]})
+        == "ssh-mcp"
+    )
+
+
+def test_detect_migratable_package_unknown_returns_none():
+    assert c.detect_migratable_package({"command": "npx", "args": ["some-other"]}) is None
+    assert c.detect_migratable_package({}) is None
+    assert c.detect_migratable_package({"url": "https://x"}) is None
+
+
+def test_migrate_removed_flags_inline_form():
+    data = {
+        "command": "npx",
+        "args": ["-y", "ssh-mcp", "--", "--host=1.2.3.4", "--password=hunter2"],
+    }
+    new, notes = c.migrate_removed_flags(data)
+    assert new["args"] == ["-y", "ssh-mcp", "--", "--host=1.2.3.4"]
+    assert new["env"] == {"SSH_MCP_PASSWORD": "hunter2"}
+    assert any("SSH_MCP_PASSWORD" in n for n in notes)
+    # Original untouched (pure function).
+    assert "env" not in data
+
+
+def test_migrate_removed_flags_separate_form():
+    data = {
+        "command": "npx",
+        "args": ["-y", "ssh-mcp", "--user", "root", "--password", "s3cret"],
+    }
+    new, _ = c.migrate_removed_flags(data)
+    assert new["args"] == ["-y", "ssh-mcp", "--user", "root"]
+    assert new["env"] == {"SSH_MCP_PASSWORD": "s3cret"}
+
+
+def test_migrate_removed_flags_preserves_existing_env_and_merges():
+    data = {
+        "command": "npx",
+        "args": ["ssh-mcp", "--password=p"],
+        "env": {"OTHER": "keep"},
+    }
+    new, _ = c.migrate_removed_flags(data)
+    assert new["env"] == {"OTHER": "keep", "SSH_MCP_PASSWORD": "p"}
+
+
+def test_migrate_removed_flags_does_not_clobber_existing_secret():
+    data = {
+        "command": "npx",
+        "args": ["ssh-mcp", "--password=fromargs"],
+        "env": {"SSH_MCP_PASSWORD": "fromenv"},
+    }
+    new, notes = c.migrate_removed_flags(data)
+    # env value wins; the redundant flag is still stripped so v2 can start.
+    assert new["env"] == {"SSH_MCP_PASSWORD": "fromenv"}
+    assert "--password=fromargs" not in new["args"]
+    assert any("already set" in n for n in notes)
+
+
+def test_migrate_removed_flags_moves_env_ref_verbatim():
+    data = {"command": "npx", "args": ["ssh-mcp", "--password", "${MY_PW}"]}
+    new, _ = c.migrate_removed_flags(data)
+    assert new["env"] == {"SSH_MCP_PASSWORD": "${MY_PW}"}
+    assert "${MY_PW}" not in new["args"]
+
+
+def test_migrate_removed_flags_noop_returns_same_object():
+    data = {"command": "npx", "args": ["ssh-mcp", "--host=h", "--user=u"]}
+    new, notes = c.migrate_removed_flags(data)
+    assert new is data
+    assert notes == []
+
+
+def test_migrate_removed_flags_unknown_package_noop():
+    data = {"command": "npx", "args": ["mystery", "--password=p"]}
+    new, notes = c.migrate_removed_flags(data)
+    assert new is data and notes == []
+
+
+def test_migrate_removed_flags_only_args_left_drops_args_key():
+    data = {"command": "ssh-mcp", "args": ["--password=p"]}
+    new, _ = c.migrate_removed_flags(data)
+    assert "args" not in new
+    assert new["env"] == {"SSH_MCP_PASSWORD": "p"}
+
+
+def test_removed_flag_warnings_covers_migratable_and_manual():
+    data = {
+        "command": "npx",
+        "args": ["ssh-mcp", "--password=p", "--sudoPassword=s", "--disableSudo"],
+    }
+    warnings = c.removed_flag_warnings(data)
+    text = " ".join(warnings)
+    assert "--password" in text
+    assert "sudoPassword" in text
+    assert "disableSudo" in text
+    # migrate only touches the confirmed --password mapping
+    new, _ = c.migrate_removed_flags(data)
+    assert new["env"] == {"SSH_MCP_PASSWORD": "p"}
+    assert "--sudoPassword=s" in new["args"]
+    assert "--disableSudo" in new["args"]
+
+
+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
+
+
+def test_ssh_membermatters_end_to_end():
+    # The exact shape from AJ's failing server.
+    data = {
+        "command": "npx",
+        "args": [
+            "-y",
+            "ssh-mcp",
+            "--",
+            "--host=member.example.io",
+            "--user=deploy",
+            "--port=22",
+            "--password=topsecret",
+        ],
+    }
+    new, notes = c.migrate_removed_flags(data)
+    assert new["args"] == [
+        "-y",
+        "ssh-mcp",
+        "--",
+        "--host=member.example.io",
+        "--user=deploy",
+        "--port=22",
+    ]
+    assert new["env"] == {"SSH_MCP_PASSWORD": "topsecret"}
+    assert notes
+
+
 # --------------------------------------------------------------------------- #
 # Move to environment variable (issue #83)
 # --------------------------------------------------------------------------- #