feat: "Move to environment variable" — convert a plaintext secret to ${VAR} (#83)

Follow-up to #76/#82: BCC warns when a config holds a raw credential and
points at ${VAR}, but gave no way to make the change. This adds the
one-click conversion, right-click a secret row in the env or headers table.

The value is about to leave the file, so the action's real job is handing
the secret back before it does:

- Core (pure, tested): sanitize_env_var_name (key -> legal upper-case shell
  name; 'api-key' -> API_KEY, '2fa' -> _2FA, non-ASCII/empty handled),
  shell_export_lines (the exact export/setx line, POSIX single-quoted
  safely), move_value_to_env_ref (data in -> new data out, replaces one
  env/header/args value with ${VAR}, returns the removed secret; never
  mutates the input; None if the target is missing, non-string, or already a
  reference), can_move_value_to_env_ref (offer only a real stored secret, not
  already a ref, AND only on a client that expands references -- offering it
  on Claude Desktop would author a config that reaches the server as literal
  ${VAR}, the exact failure #76 exists to prevent), and is_env_var_set (skip
  the ceremony when the variable already looks set).

- GUI: KeyValueTable gains a context menu gated on can_move_value_to_env_ref
  (so it never appears on a non-secret row or a Claude Desktop profile).
  MoveToEnvDialog lets the user name the variable (defaulting to the
  sanitised key), shows the platform-appropriate shell line live, notes when
  the variable already looks set, and on accept copies the secret to the
  clipboard before the cell is replaced with the reference. Wired through
  ServerEditor.set_profile_provider so the tables know which client is loaded.

Scope note: env and headers rows for now. The core already handles args by
index; wiring the args editor (a free-text widget, not a table) is a small
follow-up, deliberately not bundled here.

Tests: +15 core (name sanitisation incl. non-ASCII/leading-digit/empty,
POSIX quote safety, the gate across secret/non-secret/already-ref/
non-expanding-client, env+headers+args rewrite, input-not-mutated,
missing/non-string/already-ref -> None, is_env_var_set). 478 passed, ruff
clean. GUI is untestable in CI (no PySide6); the decision logic all lives in
bcc_core and is tested there.

Closes #83

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EKwBecy6N83jnqQmw8ezwE
This commit is contained in:
2026-08-04 03:35:21 +00:00
co-authored by Claude Opus 4.8
parent 0191a93eb9
commit 1b12489913
3 changed files with 349 additions and 0 deletions
+123
View File
@@ -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 ""))