"Move to environment variable" — convert a plaintext secret to ${VAR} (#83) #87
@@ -291,9 +291,10 @@ class MoveToEnvDialog(QDialog):
|
||||
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."
|
||||
"This replaces the value in place with a ${VAR} reference. The secret "
|
||||
"moves to your shell/OS environment — not this config file, and not the "
|
||||
"Environment variables table below. Run the line below to set it there, "
|
||||
"or the server won't authenticate."
|
||||
)
|
||||
intro.setWordWrap(True)
|
||||
v.addWidget(intro)
|
||||
@@ -356,6 +357,141 @@ class MoveToEnvDialog(QDialog):
|
||||
self._already.hide()
|
||||
|
||||
|
||||
class MoveArgToEnvDialog(QDialog):
|
||||
"""Confirm relocating a secret arg into the env block (#83, kept in file).
|
||||
|
||||
Unlike the reference move, this keeps the value in the config -- it just
|
||||
moves it out of the argument list (visible in process listings) and into
|
||||
the Environment variables table, where the user can see and edit it. It
|
||||
changes how the server is launched, so it says so plainly.
|
||||
"""
|
||||
|
||||
def __init__(self, parent, key: str, value: str):
|
||||
super().__init__(parent)
|
||||
self.setWindowTitle("Move into environment variables")
|
||||
self.setMinimumWidth(460)
|
||||
v = QVBoxLayout(self)
|
||||
v.setSpacing(10)
|
||||
|
||||
intro = QLabel(
|
||||
"This moves the secret out of the arguments and into the Environment "
|
||||
"variables table below, where you can see and edit its value. The value "
|
||||
"stays in this config file."
|
||||
)
|
||||
intro.setWordWrap(True)
|
||||
v.addWidget(intro)
|
||||
|
||||
warn = QLabel(
|
||||
"⚠ This changes how the server is launched: the flag is dropped and the "
|
||||
"value is set as an environment variable instead. It only works if the "
|
||||
"server reads this secret from that variable."
|
||||
)
|
||||
warn.setWordWrap(True)
|
||||
warn.setStyleSheet(f"color: {WARN};")
|
||||
v.addWidget(warn)
|
||||
|
||||
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))
|
||||
grid.addWidget(self._name_edit, 0, 1)
|
||||
v.addLayout(grid)
|
||||
|
||||
btns = QDialogButtonBox(
|
||||
QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel
|
||||
)
|
||||
ok = btns.button(QDialogButtonBox.StandardButton.Ok)
|
||||
ok.setText("Move into env")
|
||||
ok.setObjectName("primary")
|
||||
btns.accepted.connect(self.accept)
|
||||
btns.rejected.connect(self.reject)
|
||||
v.addWidget(btns)
|
||||
|
||||
def var_name(self) -> str:
|
||||
return core.sanitize_env_var_name(self._name_edit.text())
|
||||
|
||||
|
||||
class ReferencedVarsDialog(QDialog):
|
||||
"""Show every ${VAR} the loaded server references and whether it's set (#83).
|
||||
|
||||
After a secret becomes a reference, the variable lives in the user's
|
||||
environment, not the config -- so this is where they confirm it exists and
|
||||
get the command to set it. Read-only; BCC can't (and shouldn't) store the
|
||||
value.
|
||||
"""
|
||||
|
||||
def __init__(self, parent, data: dict):
|
||||
super().__init__(parent)
|
||||
self.setWindowTitle("Referenced variables")
|
||||
self.setMinimumWidth(560)
|
||||
v = QVBoxLayout(self)
|
||||
v.setSpacing(10)
|
||||
|
||||
self._usages = core.referenced_env_vars(data)
|
||||
if not self._usages:
|
||||
v.addWidget(QLabel("This server references no ${VAR} variables."))
|
||||
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
|
||||
btns.rejected.connect(self.reject)
|
||||
btns.accepted.connect(self.accept)
|
||||
v.addWidget(btns)
|
||||
return
|
||||
|
||||
intro = QLabel(
|
||||
"These references are read from your shell/OS environment when the client "
|
||||
"runs. ✓ means it's set in BCC's environment (which may differ from the "
|
||||
"client's) or has a default; ✗ means nothing would fill it."
|
||||
)
|
||||
intro.setWordWrap(True)
|
||||
v.addWidget(intro)
|
||||
|
||||
self._table = QTableWidget(len(self._usages), 3)
|
||||
self._table.setHorizontalHeaderLabels(["Variable", "Status", "Used in"])
|
||||
self._table.horizontalHeader().setSectionResizeMode(0, QHeaderView.ResizeMode.Stretch)
|
||||
self._table.horizontalHeader().setSectionResizeMode(2, QHeaderView.ResizeMode.Stretch)
|
||||
self._table.verticalHeader().setVisible(False)
|
||||
self._table.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
|
||||
self._table.setEditTriggers(QAbstractItemView.EditTrigger.NoEditTriggers)
|
||||
for r, u in enumerate(self._usages):
|
||||
is_set = core.is_env_var_set(u.name)
|
||||
status = "✓ set" if is_set else ("✓ default" if u.has_default else "✗ not set")
|
||||
self._table.setItem(r, 0, QTableWidgetItem(u.name))
|
||||
self._table.setItem(r, 1, QTableWidgetItem(status))
|
||||
self._table.setItem(r, 2, QTableWidgetItem(", ".join(u.fields)))
|
||||
self._table.selectionModel().selectionChanged.connect(self._refresh_cmd)
|
||||
v.addWidget(self._table, 1)
|
||||
|
||||
set_lbl = QLabel("Set the selected variable 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)
|
||||
|
||||
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
|
||||
btns.rejected.connect(self.reject)
|
||||
btns.accepted.connect(self.accept)
|
||||
v.addWidget(btns)
|
||||
|
||||
self._table.selectRow(0)
|
||||
|
||||
def _refresh_cmd(self, *_):
|
||||
rows = self._table.selectionModel().selectedRows()
|
||||
if not rows:
|
||||
self._cmd.setText("")
|
||||
return
|
||||
name = self._usages[rows[0].row()].name
|
||||
# A placeholder value -- BCC doesn't hold the secret, this shows the shape.
|
||||
lines = core.shell_export_lines(name, "<value>")
|
||||
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']})")
|
||||
|
||||
|
||||
class KeyValueTable(QWidget):
|
||||
def __init__(self, key_label="Key", val_label="Value", on_change=None, before_change=None):
|
||||
super().__init__()
|
||||
@@ -423,14 +559,14 @@ class KeyValueTable(QWidget):
|
||||
return
|
||||
profile = self.profile_provider() if self.profile_provider else None
|
||||
menu = QMenu(self)
|
||||
act = QAction("Move to environment variable…", self)
|
||||
act = QAction("Replace with a ${VAR} reference (out of file)…", self)
|
||||
if core.can_move_value_to_env_ref(key, value, profile):
|
||||
act.triggered.connect(lambda: self._move_row_to_env(row))
|
||||
else:
|
||||
# Show it disabled with the reason rather than an empty menu, so the
|
||||
# feature is discoverable and Claude Desktop's gating is explained.
|
||||
act.setEnabled(False)
|
||||
act.setText("Move to environment variable — unavailable for Claude Desktop")
|
||||
act.setText("Replace with ${VAR} reference — unavailable for Claude Desktop")
|
||||
act.setToolTip(
|
||||
"Claude Desktop doesn't expand ${VAR}, so a reference would reach "
|
||||
"the server as literal text."
|
||||
@@ -661,6 +797,12 @@ class ServerEditor(QFrame):
|
||||
self.logs_btn = QPushButton("View logs")
|
||||
self.logs_btn.setToolTip("Open this server's MCP log in a read-only, auto-tailing viewer")
|
||||
self.logs_btn.clicked.connect(self._view_logs)
|
||||
self.vars_btn = QPushButton("Variables…")
|
||||
self.vars_btn.setToolTip(
|
||||
"Show the ${VAR} references this server uses and whether each is set in "
|
||||
"your environment"
|
||||
)
|
||||
self.vars_btn.clicked.connect(self._show_referenced_vars)
|
||||
self.details_btn = QPushButton("Details ▸")
|
||||
self.details_btn.setCheckable(True)
|
||||
self.details_btn.toggled.connect(self._toggle_diag)
|
||||
@@ -672,6 +814,7 @@ class ServerEditor(QFrame):
|
||||
dep.addWidget(self.test_btn)
|
||||
dep.addWidget(self.spawn_btn)
|
||||
dep.addWidget(self.logs_btn)
|
||||
dep.addWidget(self.vars_btn)
|
||||
dep.addWidget(self.details_btn)
|
||||
dep.addWidget(recheck)
|
||||
outer.addLayout(dep)
|
||||
@@ -773,11 +916,56 @@ class ServerEditor(QFrame):
|
||||
return w
|
||||
|
||||
def set_profile_provider(self, provider):
|
||||
"""Let the env/headers tables and the args editor gate "move to
|
||||
environment variable" on which client the loaded profile targets (#83)."""
|
||||
"""Let the env/headers tables and the args editor gate the secret-move
|
||||
actions on which client the loaded profile targets (#83), and wire the
|
||||
args editor's two move actions back to this editor (which owns the whole
|
||||
form, since moving an arg into env touches both fields)."""
|
||||
self.env.profile_provider = provider
|
||||
self.headers.profile_provider = provider
|
||||
self.args.profile_provider = provider
|
||||
self.args.on_move_to_ref = self.move_arg_to_reference
|
||||
self.args.on_move_to_env = self.move_arg_into_env
|
||||
|
||||
# --- secret moves from args (#83) ------------------------------------ #
|
||||
def _reload_from_data(self, new_data: dict):
|
||||
"""Repopulate the form from a transformed data dict and mark dirty."""
|
||||
self.load_entry(core.ServerEntry(self.current_name(), new_data, True))
|
||||
self._emit()
|
||||
|
||||
def move_arg_to_reference(self, index: int):
|
||||
"""Args secret -> ${VAR} reference in place (secret leaves the file)."""
|
||||
data = self.dump_data()
|
||||
args = data.get("args") or []
|
||||
if not (0 <= index < len(args)):
|
||||
return
|
||||
dlg = MoveToEnvDialog(
|
||||
self.window(), core.suggested_env_var_for_arg(args, index), args[index]
|
||||
)
|
||||
if not dlg.exec():
|
||||
return
|
||||
conv = core.move_value_to_env_ref(data, field="args", index=index, var_name=dlg.var_name())
|
||||
if conv is None:
|
||||
return
|
||||
QGuiApplication.clipboard().setText(conv.secret)
|
||||
self._reload_from_data(conv.data)
|
||||
|
||||
def move_arg_into_env(self, index: int):
|
||||
"""Args secret -> env block, kept in this config (visible/editable)."""
|
||||
data = self.dump_data()
|
||||
args = data.get("args") or []
|
||||
if not (0 <= index < len(args)):
|
||||
return
|
||||
default_name = core.suggested_env_var_for_arg(args, index)
|
||||
dlg = MoveArgToEnvDialog(self.window(), default_name, args[index])
|
||||
if not dlg.exec():
|
||||
return
|
||||
new = core.move_arg_to_env_block(data, index, var_name=dlg.var_name())
|
||||
if new is None:
|
||||
return
|
||||
self._reload_from_data(new)
|
||||
|
||||
def _show_referenced_vars(self):
|
||||
ReferencedVarsDialog(self.window(), self.dump_data()).exec()
|
||||
|
||||
# --- model <-> form -------------------------------------------------- #
|
||||
def load_entry(self, entry: core.ServerEntry | None):
|
||||
@@ -1056,9 +1244,13 @@ class ArgsEdit(QPlainTextEdit):
|
||||
self.blockCountChanged.connect(self._update_gutter_width)
|
||||
self.updateRequest.connect(self._on_update_request)
|
||||
self._update_gutter_width()
|
||||
# Set by ServerEditor so "move to environment variable" (#83) can gate on
|
||||
# the loaded client, same as the env/headers tables.
|
||||
# Wired by ServerEditor: gate on the loaded client, and the two move
|
||||
# actions (which the editor performs, since moving an arg into env
|
||||
# touches both the args and the env table). Indices are into the
|
||||
# non-blank arg list, matching dump_data()'s args.
|
||||
self.profile_provider = None
|
||||
self.on_move_to_ref = None
|
||||
self.on_move_to_env = None
|
||||
|
||||
def contextMenuEvent(self, event):
|
||||
menu = self.createStandardContextMenu() # keep cut/copy/paste
|
||||
@@ -1071,36 +1263,34 @@ class ArgsEdit(QPlainTextEdit):
|
||||
idx = sum(1 for ln in lines[:block] if ln.strip() != "")
|
||||
if idx in set(core.secret_arg_indices(cleaned)):
|
||||
profile = self.profile_provider() if self.profile_provider else None
|
||||
act = QAction("Move to environment variable…", self)
|
||||
if profile is None or core.client_expands_env_refs(profile):
|
||||
act.triggered.connect(lambda: self._move_arg_to_env(block, idx, cleaned))
|
||||
expands = profile is None or core.client_expands_env_refs(profile)
|
||||
first = menu.actions()[0] if menu.actions() else None
|
||||
|
||||
# Reference (secret leaves the file) -- needs an expanding client.
|
||||
ref_act = QAction("Replace with a ${VAR} reference (out of file)…", self)
|
||||
if expands and self.on_move_to_ref:
|
||||
ref_act.triggered.connect(lambda: self.on_move_to_ref(idx))
|
||||
else:
|
||||
act.setEnabled(False)
|
||||
act.setText("Move to environment variable — unavailable for Claude Desktop")
|
||||
act.setToolTip(
|
||||
ref_act.setEnabled(False)
|
||||
ref_act.setText(
|
||||
"Replace with ${VAR} reference — unavailable for Claude Desktop"
|
||||
)
|
||||
ref_act.setToolTip(
|
||||
"Claude Desktop doesn't expand ${VAR}, so a reference would "
|
||||
"reach the server as literal text."
|
||||
)
|
||||
first = menu.actions()[0] if menu.actions() else None
|
||||
menu.insertAction(first, act)
|
||||
|
||||
# Move into the env block (kept in file) -- works on any client.
|
||||
env_act = QAction("Move into Environment variables (kept in this config)…", self)
|
||||
if self.on_move_to_env:
|
||||
env_act.triggered.connect(lambda: self.on_move_to_env(idx))
|
||||
|
||||
menu.insertAction(first, ref_act)
|
||||
menu.insertAction(first, env_act)
|
||||
if first is not None:
|
||||
menu.insertSeparator(first)
|
||||
menu.exec(event.globalPos())
|
||||
|
||||
def _move_arg_to_env(self, block: int, idx: int, cleaned: list):
|
||||
value = cleaned[idx]
|
||||
default_name = core.suggested_env_var_for_arg(cleaned, idx)
|
||||
dlg = MoveToEnvDialog(self.window(), default_name, value)
|
||||
if not dlg.exec():
|
||||
return
|
||||
var_name = dlg.var_name()
|
||||
QGuiApplication.clipboard().setText(value)
|
||||
lines = self.toPlainText().splitlines()
|
||||
if 0 <= block < len(lines):
|
||||
lines[block] = f"${{{var_name}}}"
|
||||
# setPlainText fires textChanged -> _emit (dirty + arg recheck).
|
||||
self.setPlainText("\n".join(lines))
|
||||
|
||||
def gutter_width(self) -> int:
|
||||
digits = max(1, len(str(self.blockCount())))
|
||||
return 14 + self.fontMetrics().horizontalAdvance("9") * digits
|
||||
|
||||
+82
@@ -1960,6 +1960,88 @@ def is_env_var_set(var_name: str, environ: dict | None = None) -> bool:
|
||||
return bool(env.get(var_name))
|
||||
|
||||
|
||||
class EnvVarUsage(NamedTuple):
|
||||
"""One ${VAR} referenced by a server definition, and whether it resolves.
|
||||
|
||||
`resolved` is best-effort: a variable is considered resolvable if it's set
|
||||
in the checked environment OR any occurrence carries a `:-default`. Since
|
||||
BCC's environment isn't necessarily the client's, this is advisory (the UI
|
||||
says so).
|
||||
"""
|
||||
|
||||
name: str
|
||||
fields: tuple[str, ...] # which config fields it appears in (sorted)
|
||||
has_default: bool
|
||||
resolved: bool
|
||||
|
||||
|
||||
def referenced_env_vars(data: dict, environ: dict | None = None) -> list[EnvVarUsage]:
|
||||
"""Every distinct ${VAR} a server definition references, with its status.
|
||||
|
||||
This is the "where did my secret go / is it wired up?" readout for #83:
|
||||
after a value becomes `${VAR}`, the variable lives in the user's
|
||||
environment, not the config, so BCC surfaces the list and whether each one
|
||||
currently resolves. Sorted by name; deduped across fields.
|
||||
"""
|
||||
env = os.environ if environ is None else environ
|
||||
by_name: dict[str, dict] = {}
|
||||
for ref in server_env_refs(data):
|
||||
slot = by_name.setdefault(ref.name, {"fields": set(), "has_default": False})
|
||||
slot["fields"].add(ref.field)
|
||||
slot["has_default"] = slot["has_default"] or ref.has_default
|
||||
out: list[EnvVarUsage] = []
|
||||
for name in sorted(by_name):
|
||||
slot = by_name[name]
|
||||
has_default = slot["has_default"]
|
||||
out.append(
|
||||
EnvVarUsage(
|
||||
name=name,
|
||||
fields=tuple(sorted(slot["fields"])),
|
||||
has_default=has_default,
|
||||
resolved=has_default or name in env,
|
||||
)
|
||||
)
|
||||
return out
|
||||
|
||||
|
||||
def move_arg_to_env_block(data: dict, index: int, var_name=None) -> dict | None:
|
||||
"""Relocate one secret arg into the `env` block, keeping the value in-file.
|
||||
|
||||
The "managed in-file" alternative to a ${VAR} reference: the secret leaves
|
||||
the args (where it's visible in process listings) and becomes an env entry
|
||||
the user can see and edit in BCC's table. Returns a NEW data dict, or None
|
||||
if the target isn't a movable string.
|
||||
|
||||
When the arg follows a `--flag`, the flag is removed too, since a server
|
||||
that reads the secret from an env var no longer needs the switch. This
|
||||
changes how the server is launched -- the GUI warns before doing it.
|
||||
"""
|
||||
new = copy.deepcopy(data)
|
||||
args = new.get("args")
|
||||
if not isinstance(args, list) or not (0 <= index < len(args)):
|
||||
return None
|
||||
value = args[index]
|
||||
if not isinstance(value, str) or is_env_ref(value):
|
||||
return None
|
||||
|
||||
vn = sanitize_env_var_name(
|
||||
var_name if var_name is not None else suggested_env_var_for_arg(args, index)
|
||||
)
|
||||
|
||||
# Drop the value, and the preceding flag if there is one (--api-key SECRET).
|
||||
remove_from = index
|
||||
if index > 0 and isinstance(args[index - 1], str) and args[index - 1].startswith("-"):
|
||||
remove_from = index - 1
|
||||
del args[remove_from : index + 1]
|
||||
|
||||
env = new.get("env")
|
||||
if not isinstance(env, dict):
|
||||
env = {}
|
||||
new["env"] = env
|
||||
env[vn] = value
|
||||
return new
|
||||
|
||||
|
||||
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 ""))
|
||||
|
||||
@@ -3180,3 +3180,54 @@ def test_suggested_env_var_for_arg_uses_preceding_flag():
|
||||
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