Merge remote-tracking branch 'origin/main' into feat/removed-flag-env-migration

# Conflicts:
#	tests/test_core.py

bcc_core.py

@@ -1837,6 +1837,211 @@ 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))
+
+
+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 ""))
@@ -1928,6 +2133,54 @@ def args_secret_warning(data: dict) -> str | None:
     return None
 
 
+def secret_arg_indices(args: list) -> list[int]:
+    """Indices of args that look like a raw credential.
+
+    Same detection as `args_secret_warning`, but per-arg so the "move to
+    environment variable" action (#83) knows exactly which arg to offer on.
+    Flags a token-prefixed positional (ghp_..., sk-...), an embedded-credential
+    URL, or the value following a secret-named flag (`--token abc`). A `${VAR}`
+    reference is never flagged -- it's the fix, not the problem.
+    """
+    out: list[int] = []
+    mask_next = False
+    for i, a in enumerate(str(x) for x in args):
+        if is_env_ref(a):
+            mask_next = False
+            continue
+        if mask_next:
+            mask_next = False
+            if not a.startswith("-"):
+                out.append(i)
+            continue
+        if a.startswith("-") and "=" in a:
+            continue
+        if a.startswith("-") and is_secret_key(a):
+            mask_next = True
+            continue
+        if a.startswith("-"):
+            continue
+        if _is_secret_value(a) or _EMBEDDED_CRED_RE.search(a):
+            out.append(i)
+    return out
+
+
+def suggested_env_var_for_arg(args: list, index: int) -> str:
+    """A default variable name for moving `args[index]` to a reference.
+
+    Uses the preceding flag when there is one (`--api-key <secret>` ->
+    API_KEY), since that names what the value is; otherwise falls back to a
+    generic SECRET. Always a legal shell name.
+    """
+    if 0 < index <= len(args):
+        prev = str(args[index - 1]) if index - 1 < len(args) else ""
+        if prev.startswith("-"):
+            base = prev.lstrip("-").split("=", 1)[0]
+            if base:
+                return sanitize_env_var_name(base)
+    return "SECRET"
+
+
 def split_suspicious_args(args: list[str]) -> tuple[list[str], list[str]]:
     """
     Detect the classic argument-entry mistake: several argv tokens typed on one