bcc.py

@@ -268,7 +268,7 @@ class _SecretMaskDelegate(QStyledItemDelegate):
         if self.revealed or not option.text:
             return
         key_item = self._table.item(index.row(), 0)
-        if key_item and core.is_secret_key(key_item.text()):
+        if key_item and core.should_mask_value(key_item.text(), option.text):
             option.text = core.MASK
 
 
@@ -2622,6 +2622,12 @@ class MainWindow(QMainWindow):
             self.save_btn.setEnabled(False)
             return False
         lint_warnings = core.lint_servers(self.servers)
+        # ${VAR} references are only meaningful if the target client expands
+        # them -- Claude Desktop doesn't, so the same config is fine in one
+        # profile and broken in another (#76). Report against the loaded one.
+        for entry in self.servers:
+            for warning in core.env_ref_warnings(entry.data, self.current_profile):
+                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

@@ -893,13 +893,28 @@ def backup_label(backup_path: Path | str) -> str:
     return f"{ts[:4]}-{ts[4:6]}-{ts[6:8]}  {ts[9:11]}:{ts[11:13]}:{ts[13:]}"
 
 
+def should_mask_value(key: str, value) -> bool:
+    """Whether an env/header value should be masked for display.
+
+    A ${VAR} reference is NOT a secret -- it's a pointer to one, and it's the
+    thing we want users to adopt. Masking it to dots would make a reference
+    indistinguishable from a stored credential, hiding exactly the distinction
+    that makes the feature worth using (#76).
+    """
+    if not is_secret_key(key):
+        return False
+    return not is_env_ref(value) if isinstance(value, str) else True
+
+
 def _redact_server_data(data: dict) -> dict:
     """Return a copy of a server definition with secrets masked for display."""
     out = dict(data)
     if "args" in out:
         out["args"] = redact_args(list(out["args"] or []))
     if "env" in out:
-        out["env"] = {k: (MASK if is_secret_key(k) else v) for k, v in (out["env"] or {}).items()}
+        out["env"] = {
+            k: (MASK if should_mask_value(k, v) else v) for k, v in (out["env"] or {}).items()
+        }
     return out
 
 
@@ -1450,6 +1465,155 @@ MASK = "••••••••"
 _EMBEDDED_CRED_RE = re.compile(r"://[^:@/\s]+:[^:@/\s]+@")
 
 
+# --------------------------------------------------------------------------- #
+# Environment-variable references (issue #76)
+#
+# Claude Code expands ${VAR} and ${VAR:-default} itself, in command, args, env,
+# url and headers. So BCC does NOT expand these on write -- resolving them into
+# the file would put the secret back on disk, which is the whole thing the user
+# is avoiding, and would defeat a feature the client already implements. BCC
+# authors, validates and warns.
+#
+# Claude Desktop has no documented support, so the same text there is passed to
+# the server literally. That makes this a per-client capability, not a global
+# one -- see client_expands_env_refs().
+# --------------------------------------------------------------------------- #
+# ${NAME} or ${NAME:-default}. Names follow the shell convention (letter or
+# underscore first) so a bare "${}" or "${1}" isn't mistaken for a reference.
+_ENV_REF_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}")
+
+# The five fields Claude Code documents as expansion sites.
+ENV_REF_FIELDS = ("command", "args", "env", "url", "headers")
+
+
+class EnvRef(NamedTuple):
+    """One ${VAR} / ${VAR:-default} occurrence found in a server definition."""
+
+    name: str
+    default: str | None
+    field: str  # which of ENV_REF_FIELDS it was found in
+
+    @property
+    def has_default(self) -> bool:
+        return self.default is not None
+
+
+def find_env_refs(text: str, field: str = "") -> list[EnvRef]:
+    """Every ${VAR} / ${VAR:-default} reference in a single string."""
+    if not isinstance(text, str):
+        return []
+    return [EnvRef(m.group(1), m.group(2), field) for m in _ENV_REF_RE.finditer(text)]
+
+
+def is_env_ref(value: str) -> bool:
+    """True when the value contains at least one ${VAR} reference.
+
+    Used to keep placeholders OUT of secret masking: `${API_KEY}` under a
+    secret-looking key is a reference, not a secret, and masking it to dots
+    would hide the one distinction the user needs to see.
+    """
+    return bool(find_env_refs(value))
+
+
+def server_env_refs(data: dict) -> list[EnvRef]:
+    """Every env reference in a server definition, tagged with its field.
+
+    Only inspects the fields Claude Code actually expands; a ${VAR} written
+    into some other key is not a reference and shouldn't be reported as one.
+    """
+    out: list[EnvRef] = []
+    if not isinstance(data, dict):
+        return out
+    for field in ENV_REF_FIELDS:
+        value = data.get(field)
+        if isinstance(value, str):
+            out.extend(find_env_refs(value, field))
+        elif isinstance(value, list):
+            for item in value:
+                out.extend(find_env_refs(item, field))
+        elif isinstance(value, dict):
+            for v in value.values():
+                out.extend(find_env_refs(v, field))
+    return out
+
+
+def expand_env_refs(text: str, environ: dict | None = None) -> str:
+    """Expand ${VAR} / ${VAR:-default} the way Claude Code documents it.
+
+    Provided for previewing what the client will do -- BCC never writes the
+    expanded form back to the config. Unset with no default is left as the
+    literal ${VAR} text, matching Claude Code: the config still loads and the
+    unexpanded text is passed through.
+    """
+    if not isinstance(text, str):
+        return text
+    env = os.environ if environ is None else environ
+
+    def repl(m: re.Match) -> str:
+        name, default = m.group(1), m.group(2)
+        if name in env:
+            return env[name]
+        return default if default is not None else m.group(0)
+
+    return _ENV_REF_RE.sub(repl, text)
+
+
+def unresolved_env_refs(data: dict, environ: dict | None = None) -> list[EnvRef]:
+    """References that would not resolve: variable unset AND no default.
+
+    Best-effort by nature -- BCC's environment isn't necessarily the client's,
+    so this warns rather than blocks, and the warning text says so.
+    """
+    env = os.environ if environ is None else environ
+    return [r for r in server_env_refs(data) if not r.has_default and r.name not in env]
+
+
+def client_expands_env_refs(profile: Profile) -> bool:
+    """Whether the client behind `profile` expands ${VAR} itself.
+
+    Claude Code does, in command/args/env/url/headers, for both project
+    `.mcp.json` and user-scope `~/.claude.json`. Claude Desktop has no
+    documented support, so a reference there reaches the server as literal
+    text -- which surfaces as a confusing auth failure rather than an obvious
+    config error, hence the warning.
+    """
+    return not profile_targets_claude_desktop(profile)
+
+
+def env_ref_warnings(
+    data: dict, profile: Profile | None = None, environ: dict | None = None
+) -> list[str]:
+    """Advisory warnings about env references in one server definition.
+
+    Two distinct problems, deliberately worded differently:
+      - the target client won't expand them at all (Claude Desktop)
+      - the client will expand them, but a variable looks unset here
+    """
+    refs = server_env_refs(data)
+    if not refs:
+        return []
+
+    if profile is not None and not client_expands_env_refs(profile):
+        names = ", ".join(sorted({f"${{{r.name}}}" for r in refs}))
+        return [
+            f"{names} will NOT be expanded by Claude Desktop -- it has no "
+            f"documented support for variable references, so the server "
+            f"receives the literal text. Use a real value here, or move this "
+            f"server to a Claude Code config."
+        ]
+
+    missing = unresolved_env_refs(data, environ)
+    if not missing:
+        return []
+    names = ", ".join(sorted({r.name for r in missing}))
+    return [
+        f"{names} is not set in this environment and has no ':-default'. "
+        f"Claude Code will pass the reference through unexpanded. "
+        f"(Checked against BCC's environment, which may differ from the "
+        f"client's.)"
+    ]
+
+
 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 ""))
@@ -1466,17 +1630,22 @@ def redact_args(args: list[str]) -> list[str]:
       --api-key=abc123      ->  --api-key=••••••••   (inline flag=value)
       ghp_abc123            ->  ••••••••             (well-known token prefix)
     Everything else passes through untouched.
+
+    ${VAR} references are left visible: they name a secret rather than being
+    one, and hiding them would obscure the difference between "this config
+    leaks a token" and "this config points at one" (#76).
     """
     out: list[str] = []
     mask_next = False
     for a in args:
         s = str(a)
         if mask_next:
-            out.append(MASK)
             mask_next = False
+            out.append(s if is_env_ref(s) else MASK)
             continue
         if s.startswith("-") and "=" in s and is_secret_key(s.split("=", 1)[0]):
-            out.append(s.split("=", 1)[0] + "=" + MASK)
+            flag, value = s.split("=", 1)
+            out.append(f"{flag}={value}" if is_env_ref(value) else f"{flag}={MASK}")
             continue
         if s.startswith("-") and is_secret_key(s):
             out.append(s)
@@ -1503,6 +1672,11 @@ def args_secret_warning(data: dict) -> str | None:
     args = [str(a) for a in (data.get("args") or [])]
     mask_next = False
     for a in args:
+        # A ${VAR} reference is the recommended fix for this very warning --
+        # continuing to warn after the user adopts it punishes the fix (#76).
+        if is_env_ref(a):
+            mask_next = False
+            continue
         if mask_next:
             mask_next = False
             if not a.startswith("-"):

tests/test_core.py

@@ -2680,3 +2680,138 @@ def test_update_notice_renders_both_versions_the_same_way():
     assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
     # the machine-readable field keeps the real tag
     assert n["version"] == "v1.3.0"
+
+
+# --------------------------------------------------------------------------- #
+# #76 -- ${VAR} references. Semantics mirror Claude Code's documented
+# behaviour: ${VAR} and ${VAR:-default}, expanded in command/args/env/url/
+# headers, and an unset variable with no default left as literal text.
+# --------------------------------------------------------------------------- #
+def test_find_env_refs_plain_and_defaulted():
+    refs = c.find_env_refs("${A} and ${B:-fallback}")
+    assert [(r.name, r.default) for r in refs] == [("A", None), ("B", "fallback")]
+
+
+@pytest.mark.parametrize("text", ["${}", "${1BAD}", "$NOTBRACED", "{NOPE}", "plain", "$${X"])
+def test_find_env_refs_ignores_non_references(text):
+    assert c.find_env_refs(text) == []
+
+
+def test_find_env_refs_allows_an_empty_default():
+    """`${VAR:-}` is a documented way to say 'blank if unset'."""
+    refs = c.find_env_refs("${A:-}")
+    assert refs[0].default == ""
+    assert refs[0].has_default is True
+
+
+def test_server_env_refs_covers_all_five_documented_fields():
+    data = {
+        "command": "${BIN}",
+        "args": ["--x", "${ARG}"],
+        "env": {"K": "${ENVV}"},
+        "url": "${URL}/mcp",
+        "headers": {"Authorization": "Bearer ${HDR}"},
+    }
+    found = {(r.name, r.field) for r in c.server_env_refs(data)}
+    assert found == {
+        ("BIN", "command"),
+        ("ARG", "args"),
+        ("ENVV", "env"),
+        ("URL", "url"),
+        ("HDR", "headers"),
+    }
+
+
+def test_server_env_refs_ignores_unexpanded_fields():
+    """Claude Code expands five fields; a ${VAR} elsewhere isn't a reference."""
+    assert c.server_env_refs({"description": "${NOPE}", "timeout": "${ALSO_NO}"}) == []
+
+
+def test_expand_env_refs_matches_documented_semantics():
+    env = {"SET": "value"}
+    assert c.expand_env_refs("${SET}", env) == "value"
+    assert c.expand_env_refs("${MISSING:-dflt}", env) == "dflt"
+    assert c.expand_env_refs("${SET:-dflt}", env) == "value"
+    # unset with no default: left as literal text, exactly as Claude Code does
+    assert c.expand_env_refs("${MISSING}", env) == "${MISSING}"
+
+
+def test_expand_env_refs_handles_several_in_one_string():
+    assert c.expand_env_refs("${A}/${B:-two}/${C}", {"A": "one"}) == "one/two/${C}"
+
+
+def test_unresolved_env_refs_only_flags_unset_without_default():
+    data = {"env": {"A": "${SET}", "B": "${UNSET}", "C": "${OTHER:-has_default}"}}
+    assert [r.name for r in c.unresolved_env_refs(data, {"SET": "x"})] == ["UNSET"]
+
+
+# --- the two interactions that were backwards for this feature ------------
+def test_placeholder_under_a_secret_key_is_not_masked():
+    """A ${VAR} names a secret rather than being one. Masking it would make a
+    reference indistinguishable from a stored credential."""
+    assert c.should_mask_value("API_KEY", "${API_KEY}") is False
+    assert c.should_mask_value("API_KEY", "ghp_realsecret") is True
+    assert c.should_mask_value("NOT_SECRET", "${API_KEY}") is False
+
+
+def test_redacted_display_keeps_placeholders_but_masks_real_secrets():
+    out = c._redact_server_data({"env": {"API_KEY": "${API_KEY}", "TOKEN": "ghp_real"}})
+    assert out["env"]["API_KEY"] == "${API_KEY}"
+    assert out["env"]["TOKEN"] == c.MASK
+
+
+def test_redact_args_keeps_placeholders_visible():
+    assert c.redact_args(["--token", "${GH_TOKEN}"]) == ["--token", "${GH_TOKEN}"]
+    assert c.redact_args(["--api-key=${K}"]) == ["--api-key=${K}"]
+    # real secrets still masked
+    assert c.redact_args(["--token", "ghp_real"]) == ["--token", c.MASK]
+    assert c.redact_args(["--api-key=sk-real"]) == [f"--api-key={c.MASK}"]
+
+
+def test_args_secret_warning_is_silenced_by_a_placeholder():
+    """Moving a token into ${VAR} is the recommended fix for this warning --
+    still warning afterwards would punish the fix."""
+    assert c.args_secret_warning({"args": ["--token", "ghp_real"]}) is not None
+    assert c.args_secret_warning({"args": ["--token", "${GH_TOKEN}"]}) is None
+
+
+def test_args_secret_warning_still_fires_on_the_arg_after_a_placeholder():
+    """A placeholder must clear the pending-flag state, not blanket-suppress."""
+    assert c.args_secret_warning({"args": ["${SAFE}", "--token", "ghp_real"]}) is not None
+
+
+# --- per-client gating ----------------------------------------------------
+def _profile(path):
+    return c.Profile(label="p", path=Path(path), config_exists=True)
+
+
+def test_claude_code_profiles_expand_references():
+    assert c.client_expands_env_refs(_profile(Path.home() / ".claude.json")) is True
+    assert c.client_expands_env_refs(_profile("/repo/.mcp.json")) is True
+
+
+def test_claude_desktop_profile_does_not_expand_references():
+    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
+    assert c.client_expands_env_refs(desktop) is False
+
+
+def test_desktop_profile_warns_that_references_are_literal():
+    data = {"env": {"API_KEY": "${API_KEY}"}}
+    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
+    warnings = c.env_ref_warnings(data, desktop, {"API_KEY": "set"})
+    assert len(warnings) == 1
+    assert "NOT be expanded" in warnings[0]
+    assert "${API_KEY}" in warnings[0]
+
+
+def test_claude_code_profile_warns_only_about_unset_variables():
+    code = _profile(Path.home() / ".claude.json")
+    data = {"env": {"A": "${UNSET_ONE}"}}
+    assert c.env_ref_warnings(data, code, {}) != []
+    assert c.env_ref_warnings(data, code, {"UNSET_ONE": "x"}) == []
+    # a default means it always resolves
+    assert c.env_ref_warnings({"env": {"A": "${X:-d}"}}, code, {}) == []
+
+
+def test_no_references_means_no_warnings():
+    assert c.env_ref_warnings({"command": "npx", "args": ["-y", "pkg"]}, None) == []