bcc.py

@@ -272,6 +272,90 @@ class _SecretMaskDelegate(QStyledItemDelegate):
             option.text = core.MASK
 
 
+class MoveToEnvDialog(QDialog):
+    """Confirm moving a plaintext secret out to a ${VAR} reference (#83).
+
+    The secret is about to leave the config file, so this dialog's whole job is
+    to hand it back first: it lets the user name the variable, shows the exact
+    shell line to set it, and (on accept) the caller copies the secret to the
+    clipboard. If the variable already looks set in this environment, it says so
+    and drops the urgency.
+    """
+
+    def __init__(self, parent, key: str, secret: str):
+        super().__init__(parent)
+        self.setWindowTitle("Move to environment variable")
+        self.setMinimumWidth(460)
+        self._secret = secret
+        v = QVBoxLayout(self)
+        v.setSpacing(10)
+
+        intro = QLabel(
+            "The value will be removed from the config and replaced with a "
+            "reference. Set the variable in your environment first, or the "
+            "server won't authenticate."
+        )
+        intro.setWordWrap(True)
+        v.addWidget(intro)
+
+        grid = QGridLayout()
+        grid.setSpacing(8)
+        lbl = QLabel("Variable:")
+        lbl.setObjectName("muted")
+        grid.addWidget(lbl, 0, 0)
+        self._name_edit = QLineEdit(core.sanitize_env_var_name(key))
+        self._name_edit.textChanged.connect(self._refresh)
+        grid.addWidget(self._name_edit, 0, 1)
+        v.addLayout(grid)
+
+        self._already = QLabel("")
+        self._already.setWordWrap(True)
+        self._already.setStyleSheet(f"color: {GOOD};")
+        v.addWidget(self._already)
+
+        set_lbl = QLabel("Set it with:")
+        set_lbl.setObjectName("muted")
+        v.addWidget(set_lbl)
+        self._cmd = QLabel("")
+        self._cmd.setWordWrap(True)
+        self._cmd.setTextInteractionFlags(Qt.TextInteractionFlag.TextSelectableByMouse)
+        self._cmd.setStyleSheet("font-family: monospace;")
+        v.addWidget(self._cmd)
+
+        note = QLabel("The secret will be copied to your clipboard when you continue.")
+        note.setObjectName("muted")
+        note.setWordWrap(True)
+        v.addWidget(note)
+
+        btns = QDialogButtonBox(
+            QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel
+        )
+        ok = btns.button(QDialogButtonBox.StandardButton.Ok)
+        ok.setText("Move && copy secret")
+        ok.setObjectName("primary")
+        btns.accepted.connect(self.accept)
+        btns.rejected.connect(self.reject)
+        v.addWidget(btns)
+
+        self._refresh()
+
+    def var_name(self) -> str:
+        return core.sanitize_env_var_name(self._name_edit.text())
+
+    def _refresh(self, *_):
+        name = self.var_name()
+        lines = core.shell_export_lines(name, self._secret)
+        if sys.platform == "win32":
+            self._cmd.setText(f"{lines['windows']}\n\n(macOS/Linux: {lines['posix']})")
+        else:
+            self._cmd.setText(f"{lines['posix']}\n\n(Windows: {lines['windows']})")
+        if core.is_env_var_set(name):
+            self._already.setText(f"{name} already looks set in this environment.")
+            self._already.show()
+        else:
+            self._already.hide()
+
+
 class KeyValueTable(QWidget):
     def __init__(self, key_label="Key", val_label="Value", on_change=None, before_change=None):
         super().__init__()
@@ -292,6 +376,11 @@ class KeyValueTable(QWidget):
         self.table.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
         self.table.setMinimumHeight(90)
         self.table.itemChanged.connect(self._changed)
+        # Right-click a secret row to move it out to a ${VAR} reference (#83).
+        # Set by the owner (ServerEditor) so the action can gate on the client.
+        self.profile_provider = None
+        self.table.setContextMenuPolicy(Qt.ContextMenuPolicy.CustomContextMenu)
+        self.table.customContextMenuRequested.connect(self._context_menu)
         # Secret-looking values (API_KEY, TOKEN, ...) render masked by default.
         self._mask_delegate = _SecretMaskDelegate(self.table)
         self.table.setItemDelegateForColumn(1, self._mask_delegate)
@@ -315,6 +404,47 @@ class KeyValueTable(QWidget):
         self.reveal_btn.setText("Hide secrets" if on else "Show secrets")
         self.table.viewport().update()
 
+    def _row_key_value(self, row: int):
+        key_item = self.table.item(row, 0)
+        val_item = self.table.item(row, 1)
+        key = key_item.text().strip() if key_item else ""
+        # The mask is display-only (a delegate); the model text is the real value.
+        value = val_item.text() if val_item else ""
+        return key, value
+
+    def _context_menu(self, pos):
+        item = self.table.itemAt(pos)
+        if item is None:
+            return
+        row = item.row()
+        key, value = self._row_key_value(row)
+        profile = self.profile_provider() if self.profile_provider else None
+        if not core.can_move_value_to_env_ref(key, value, profile):
+            return
+        menu = QMenu(self)
+        act = QAction("Move to environment variable…", self)
+        act.triggered.connect(lambda: self._move_row_to_env(row))
+        menu.addAction(act)
+        menu.exec(self.table.viewport().mapToGlobal(pos))
+
+    def _move_row_to_env(self, row: int):
+        key, secret = self._row_key_value(row)
+        if not secret:
+            return
+        dlg = MoveToEnvDialog(self.window(), key, secret)
+        if not dlg.exec():
+            return
+        var_name = dlg.var_name()
+        # Hand the secret back before it leaves the file: clipboard now holds it,
+        # and the dialog showed the exact shell line to set it.
+        QGuiApplication.clipboard().setText(secret)
+        if self._before_change:
+            self._before_change()
+        val_item = self.table.item(row, 1)
+        if val_item is not None:
+            # setText fires itemChanged -> _changed -> on_change (dirty + revalidate).
+            val_item.setText(f"${{{var_name}}}")
+
     def _changed(self, item=None, *_):
         if item is not None and item.column() == 0:
             new_key = item.text().strip()
@@ -631,6 +761,12 @@ class ServerEditor(QFrame):
         v.addWidget(self.headers, 1)
         return w
 
+    def set_profile_provider(self, provider):
+        """Let the env/headers tables gate "move to environment variable" on
+        which client the loaded profile targets (#83)."""
+        self.env.profile_provider = provider
+        self.headers.profile_provider = provider
+
     # --- model <-> form -------------------------------------------------- #
     def load_entry(self, entry: core.ServerEntry | None):
         self._loading = True
@@ -1649,6 +1785,7 @@ class MainWindow(QMainWindow):
         split.setHandleWidth(10)
         split.addWidget(self._build_left())
         self.editor = ServerEditor(on_change=self._editor_changed, before_change=self._push_undo)
+        self.editor.set_profile_provider(lambda: self.current_profile)
         split.addWidget(self.editor)
         split.setStretchFactor(0, 3)
         split.setStretchFactor(1, 4)

bcc_core.py

@@ -1837,6 +1837,129 @@ def env_ref_warnings(
     ]
 
 
+# --------------------------------------------------------------------------- #
+# "Move to environment variable" (issue #83)
+#
+# Turn a plaintext secret in a config into a ${VAR} reference, so the secret
+# stops living in the file. Two things make this more than a string swap:
+#   * the replaced value is the ONLY copy of the secret the user may have, so
+#     the caller must hand it back (clipboard + the exact shell line) before it
+#     leaves the config -- shell_export_lines builds that;
+#   * only clients that expand ${VAR} (Claude Code, not Desktop) should be
+#     offered this, or it walks the user straight into a broken config -- that
+#     gate is can_move_value_to_env_ref, reusing client_expands_env_refs.
+# The rewrite itself is pure (data in -> data out) and lives here; the GUI does
+# clipboard, the confirm dialog, and the already-set check.
+# --------------------------------------------------------------------------- #
+@dataclass(frozen=True)
+class EnvRefConversion:
+    """The result of moving one secret out to a ${VAR} reference."""
+
+    var_name: str  # the sanitised shell variable name chosen
+    reference: str  # "${VAR_NAME}" -- what now sits in the config
+    secret: str  # the plaintext value that was removed (hand this back!)
+    data: dict  # a NEW server-definition dict with the value replaced
+
+
+def sanitize_env_var_name(name: str) -> str:
+    """Coerce an arbitrary key into a legal, conventional shell variable name.
+
+    POSIX names are `[A-Za-z_][A-Za-z0-9_]*`; env vars are conventionally
+    upper-case. Non-alphanumerics (and any non-ASCII) become `_`, a leading
+    digit gets an `_` prefix, and an empty/degenerate result falls back to VAR.
+    'api-key' -> 'API_KEY'; '2fa' -> '_2FA'; '' -> 'VAR'.
+    """
+    cleaned = "".join(
+        ch if (ch.isascii() and (ch.isalnum() or ch == "_")) else "_" for ch in (name or "")
+    )
+    if not cleaned.strip("_"):
+        return "VAR"
+    if cleaned[0].isdigit():
+        cleaned = "_" + cleaned
+    return cleaned.upper()
+
+
+def _posix_single_quote(s: str) -> str:
+    """Wrap s in single quotes, safely, for a POSIX shell (handles embedded ')."""
+    return "'" + s.replace("'", "'\\''") + "'"
+
+
+def shell_export_lines(var_name: str, secret: str) -> dict[str, str]:
+    """The exact command to set `var_name`=`secret` in the user's shell.
+
+    Returned per-platform so the UI can show the one that fits (or both). This
+    is what makes moving the secret out safe: the user gets the setter before
+    the value leaves the file.
+    """
+    return {
+        "posix": f"export {var_name}={_posix_single_quote(secret)}",
+        "windows": f'setx {var_name} "{secret}"',
+    }
+
+
+def can_move_value_to_env_ref(key: str, value, profile: Profile | None = None) -> bool:
+    """Whether to offer "move to environment variable" for one env/header row.
+
+    True only for a real stored secret (`should_mask_value`) that isn't already
+    a reference, on a client that expands references. Offering it on Claude
+    Desktop would produce a config that reaches the server as literal `${VAR}`
+    text -- the exact failure #76 exists to prevent -- so a non-expanding
+    profile refuses outright.
+    """
+    if profile is not None and not client_expands_env_refs(profile):
+        return False
+    if is_env_ref(value):
+        return False
+    return should_mask_value(key, value)
+
+
+def move_value_to_env_ref(
+    data: dict, *, field: str, key: str | None = None, index: int | None = None, var_name=None
+) -> EnvRefConversion | None:
+    """Replace one secret value in `data` with a `${VAR}` reference.
+
+    `field` is "env" or "headers" (addressed by `key`) or "args" (addressed by
+    `index`). `var_name` defaults to the row's key (sanitised); args have no
+    key, so a var name should be supplied there. Returns an EnvRefConversion
+    carrying a NEW data dict (the input is never mutated) and the removed
+    secret, or None if the target isn't found or isn't a string to move.
+    """
+    new = copy.deepcopy(data)
+
+    if field in ("env", "headers"):
+        block = new.get(field)
+        if not isinstance(block, dict) or key not in block:
+            return None
+        current = block[key]
+        if not isinstance(current, str) or is_env_ref(current):
+            return None
+        vn = sanitize_env_var_name(var_name if var_name is not None else key)
+        block[key] = f"${{{vn}}}"
+    elif field == "args":
+        args = new.get("args")
+        if not isinstance(args, list) or index is None or not (0 <= index < len(args)):
+            return None
+        current = args[index]
+        if not isinstance(current, str) or is_env_ref(current):
+            return None
+        vn = sanitize_env_var_name(var_name if var_name is not None else f"ARG_{index}")
+        args[index] = f"${{{vn}}}"
+    else:
+        return None
+
+    return EnvRefConversion(var_name=vn, reference=f"${{{vn}}}", secret=current, data=new)
+
+
+def is_env_var_set(var_name: str, environ: dict | None = None) -> bool:
+    """Whether `var_name` is already present (non-empty) in the environment.
+
+    Lets the UI skip the copy-the-secret ceremony when the variable is already
+    set. Best-effort: BCC's environment may differ from the client's.
+    """
+    env = os.environ if environ is None else environ
+    return bool(env.get(var_name))
+
+
 def is_secret_key(name: str) -> bool:
     """Does this env-var / header / flag name look like it holds a secret?"""
     return bool(_SECRET_KEY_RE.search(name or ""))

tests/test_core.py

@@ -3059,3 +3059,92 @@ def test_discover_project_configs_skips_non_object_and_garbage(tmp_path):
     assert str(array / ".mcp.json") not in paths
     assert str(garbage / ".mcp.json") not in paths
     assert str(missing / ".mcp.json") not in paths
+
+
+# --------------------------------------------------------------------------- #
+# 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