feat: author ${VAR} references, gated on whether the client expands them (#76) #82

Open
the_og wants to merge 1 commits from feat/76-env-var-refs into main
Owner

Refs #76

The blocker, resolved

I filed #76 saying not to build the expander until we knew whether the client does this itself, because if it doesn't, BCC writing a placeholder means the server receives literal ${GITHUB_TOKEN} and fails with a confusing auth error.

Checked Anthropic's docs. The answer splits by client, which is a better outcome than either branch I'd sketched:

  • Claude Code expands ${VAR} and ${VAR:-default} natively, in command, args, env, url and headers — for project .mcp.json and user-scope ~/.claude.json, which is the file BCC edits.
  • Claude Desktop has no documented support. A reference there reaches the server as literal text.

So this is a per-client capability, and BCC already knows which client a profile targets via the existing profile_targets_claude_desktop().

BCC does not expand on write

Resolving a reference into the file would put the secret back on disk — the entire thing the user is avoiding — and would defeat a feature the client already implements correctly. BCC authors, validates, and warns. expand_env_refs() exists only to preview what the client will do.

Semantics mirror the documented ones exactly, including the counterintuitive part: an unset variable with no default is left as literal ${VAR} text, not blanked. That's what Claude Code does (the config still loads, and it reports a missing-variable warning in claude mcp list), so matching it means BCC's preview tells the truth.

The two warnings are deliberately different

"This client will never expand these" and "this variable looks unset here" are different problems and get different wording. The unset check also states that it's read from BCC's environment, which may not be the client's — best-effort by nature, so it warns rather than blocks.

Two existing behaviours were backwards for this feature

Both are the kind that would have quietly made the feature useless:

Secret masking hid placeholders. is_secret_key("API_KEY") is true, so ${API_KEY} rendered as •••••••• — making a reference indistinguishable from a stored credential, which is precisely the distinction that makes the feature worth adopting. should_mask_value() now skips references, consistently across the table delegate, _redact_server_data, and redact_args.

args_secret_warning fired on placeholders. Moving a token into ${VAR} is the recommended fix for that warning; continuing to warn punishes the fix. It now skips references — while still flagging a real secret that follows one, so the state machine isn't blanket-suppressed.

Real secrets are still masked everywhere they were before. Asserted rather than assumed, including that raw values stay out of diagnostics_text (the surface people paste into bug reports).

Tests

444 passed, 1 skipped (+23). Ruff clean.

Covers all five documented expansion fields; non-references that must not match (${}, ${1BAD}, $NOTBRACED); ${VAR:-} empty-default; multiple refs in one string; unset-without-default left literal; the masking and args-warning reversals in both directions; and per-profile gating for Claude Code vs Desktop.

Scope note

This is the authoring and safety layer. Not included: a UI affordance for converting an existing plaintext secret into a ${VAR} reference (right-click → "move to environment variable"), which is the natural follow-up now that the warnings point users toward placeholders. Worth its own issue.

Verification limits

Same as the last two PRs — no PySide6 rendering in this sandbox, so the delegate change is verified through should_mask_value at the core level, not by looking at the table. Worth confirming that a ${VAR} in an env value renders visibly while a real token beside it still shows dots.

Sources: Connect Claude Code to tools via MCP

Refs #76 ## The blocker, resolved I filed #76 saying not to build the expander until we knew whether the client does this itself, because if it doesn't, BCC writing a placeholder means the server receives literal `${GITHUB_TOKEN}` and fails with a confusing auth error. Checked Anthropic's docs. **The answer splits by client**, which is a better outcome than either branch I'd sketched: - **Claude Code expands `${VAR}` and `${VAR:-default}` natively**, in `command`, `args`, `env`, `url` and `headers` — for project `.mcp.json` *and* user-scope `~/.claude.json`, which is the file BCC edits. - **Claude Desktop has no documented support.** A reference there reaches the server as literal text. So this is a **per-client capability**, and BCC already knows which client a profile targets via the existing `profile_targets_claude_desktop()`. ## BCC does not expand on write Resolving a reference into the file would put the secret back on disk — the entire thing the user is avoiding — and would defeat a feature the client already implements correctly. BCC authors, validates, and warns. `expand_env_refs()` exists only to preview what the client will do. Semantics mirror the documented ones exactly, including the counterintuitive part: **an unset variable with no default is left as literal `${VAR}` text**, not blanked. That's what Claude Code does (the config still loads, and it reports a missing-variable warning in `claude mcp list`), so matching it means BCC's preview tells the truth. ## The two warnings are deliberately different "This client will never expand these" and "this variable looks unset here" are different problems and get different wording. The unset check also states that it's read from *BCC's* environment, which may not be the client's — best-effort by nature, so it warns rather than blocks. ## Two existing behaviours were backwards for this feature Both are the kind that would have quietly made the feature useless: **Secret masking hid placeholders.** `is_secret_key("API_KEY")` is true, so `${API_KEY}` rendered as `••••••••` — making a reference indistinguishable from a stored credential, which is precisely the distinction that makes the feature worth adopting. `should_mask_value()` now skips references, consistently across the table delegate, `_redact_server_data`, and `redact_args`. **`args_secret_warning` fired on placeholders.** Moving a token into `${VAR}` is the recommended fix for that warning; continuing to warn punishes the fix. It now skips references — while still flagging a real secret that follows one, so the state machine isn't blanket-suppressed. Real secrets are still masked everywhere they were before. Asserted rather than assumed, including that raw values stay out of `diagnostics_text` (the surface people paste into bug reports). ## Tests 444 passed, 1 skipped (+23). Ruff clean. Covers all five documented expansion fields; non-references that must *not* match (`${}`, `${1BAD}`, `$NOTBRACED`); `${VAR:-}` empty-default; multiple refs in one string; unset-without-default left literal; the masking and args-warning reversals in both directions; and per-profile gating for Claude Code vs Desktop. ## Scope note This is the authoring and safety layer. Not included: a UI affordance for *converting* an existing plaintext secret into a `${VAR}` reference (right-click → "move to environment variable"), which is the natural follow-up now that the warnings point users toward placeholders. Worth its own issue. ## Verification limits Same as the last two PRs — no PySide6 rendering in this sandbox, so the delegate change is verified through `should_mask_value` at the core level, not by looking at the table. Worth confirming that a `${VAR}` in an env value renders visibly while a real token beside it still shows dots. Sources: [Connect Claude Code to tools via MCP](https://docs.claude.com/en/docs/claude-code/mcp)
the_og added 1 commit 2026-07-20 13:46:34 -04:00
feat: author ${VAR} references, gated on whether the client expands them (#76)
CI / Lint (ruff) (pull_request) Successful in 9s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 12s
CI / Catalog signature (pull_request) Successful in 7s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
a73f2e3883
The blocker on this issue was whether BCC or the client does the expanding.
Answer, from Anthropic's docs: Claude Code expands ${VAR} and
${VAR:-default} itself, in command, args, env, url and headers, for both
project .mcp.json and user-scope ~/.claude.json. Claude Desktop has no
documented support.

So this is a per-client capability, not a global one, and BCC does NOT
expand on write: resolving a reference into the file would put the secret
back on disk -- the whole thing the user is avoiding -- and would defeat a
feature the client already implements correctly. BCC authors, validates and
warns; expand_env_refs exists to preview what the client will do.

Semantics mirror the documented ones exactly, including the unusual bit:
an unset variable with no default is left as literal ${VAR} text rather
than blanked, because that is what Claude Code passes through.

Gating uses the existing profile_targets_claude_desktop(), so a config that
is correct under Claude Code and broken under Desktop is reported against
whichever profile is actually loaded. The two warnings are worded
differently on purpose -- 'this client will never expand these' is a
different problem from 'this variable looks unset here'.

Two existing behaviours were backwards for this feature and are fixed:

- Secret masking hid placeholders. is_secret_key('API_KEY') is true, so
  ${API_KEY} rendered as dots -- making a reference indistinguishable from
  a stored credential, which is the one distinction that makes the feature
  worth adopting. should_mask_value() now skips references, in the table
  delegate, _redact_server_data and redact_args alike.
- args_secret_warning fired on placeholders. Moving a token into ${VAR} is
  the recommended fix for that warning; continuing to warn punished the
  fix. It now skips references while still flagging a real secret that
  follows one.

Real secrets are still masked everywhere they were before -- asserted, not
assumed.

Refs #76
Some checks are pending
CI / Lint (ruff) (pull_request) Successful in 9s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 12s
CI / Catalog signature (pull_request) Successful in 7s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/76-env-var-refs:feat/76-env-var-refs
git checkout feat/76-env-var-refs
Sign in to join this conversation.