feat: warn when a secret-shaped value is found in args instead of env (#1r)
CI / Lint (ruff) (pull_request) Successful in 8s
CI / Tests (py3.10) (pull_request) Successful in 10s
CI / Tests (py3.12) (pull_request) Successful in 9s

Adds `args_secret_warning(data)` to bcc_core — returns a warning string
when any arg positional value looks like a raw credential (token prefix,
value following a secret-named flag, or URL with embedded user:pass like
postgres://user:pass@host).  `--flag=value` inline forms are intentionally
skipped (the flag name already labels the value).

Adds `secret_warn` QLabel in ServerEditor's stdio page; shown/hidden by
`_check_args()` on every field change, and cleared on deselect or
stdio→remote type switch.  Non-blocking — save path is not touched.

Closes #1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
AJ
2026-07-02 11:59:17 -04:00
parent cda7d72e44
commit f4648b3c06
3 changed files with 112 additions and 0 deletions
+14
View File
@@ -449,6 +449,12 @@ class ServerEditor(QFrame):
warn_row.addWidget(self.args_warn, 1)
warn_row.addWidget(self.args_fix_btn)
v.addLayout(warn_row)
# Shown when an arg value looks like a raw credential.
self.secret_warn = QLabel("")
self.secret_warn.setStyleSheet(f"color: {WARN};")
self.secret_warn.setWordWrap(True)
self.secret_warn.hide()
v.addWidget(self.secret_warn)
v.addWidget(self._lbl("Environment variables"))
self.env = KeyValueTable(
"Variable", "Value", on_change=self._emit, before_change=self._before_change
@@ -495,6 +501,7 @@ class ServerEditor(QFrame):
self._set_dep({"status": "unknown", "label": ""})
self.args_warn.hide()
self.args_fix_btn.hide()
self.secret_warn.hide()
self._loading = False
return
self.setEnabled(True)
@@ -581,6 +588,7 @@ class ServerEditor(QFrame):
if self.type.currentIndex() != 0: # stdio page only
self.args_warn.hide()
self.args_fix_btn.hide()
self.secret_warn.hide()
return
_, notes = core.split_suspicious_args(self._current_arg_lines())
if notes:
@@ -590,6 +598,12 @@ class ServerEditor(QFrame):
else:
self.args_warn.hide()
self.args_fix_btn.hide()
warning = core.args_secret_warning({"args": self._current_arg_lines()})
if warning:
self.secret_warn.setText("" + warning)
self.secret_warn.show()
else:
self.secret_warn.hide()
def _fix_args(self):
if self._before_change:
+46
View File
@@ -801,6 +801,10 @@ _TOKEN_PREFIXES = (
MASK = "••••••••"
# Matches userinfo credentials embedded in a URL: scheme://user:pass@host
# Fires on postgres://user:pass@host but NOT on https://host/path or ssh://user@host.
_EMBEDDED_CRED_RE = re.compile(r"://[^:@/\s]+:[^:@/\s]+@")
def is_secret_key(name: str) -> bool:
"""Does this env-var / header / flag name look like it holds a secret?"""
@@ -841,6 +845,48 @@ def redact_args(args: list[str]) -> list[str]:
return out
def args_secret_warning(data: dict) -> str | None:
"""
Return a warning string when any arg looks like a raw secret that would
be better placed in `env`. Returns None when no concern is found.
Skips --flag=value inline pairs (already partially self-documenting).
Fires on:
- positional values that start with a well-known token prefix (ghp_, sk-, …)
- values that follow a secret-named flag (--token abc, --api-key abc)
- URLs with embedded user:pass credentials (postgres://user:pass@host)
"""
args = [str(a) for a in (data.get("args") or [])]
mask_next = False
for a in args:
if mask_next:
mask_next = False
if not a.startswith("-"):
return (
"An arg value following a secret-named flag looks like a credential. "
"Where the server supports it, prefer Environment variables — "
"args are visible in process listings."
)
continue
# --flag=value inline: skip (the flag name already labels it)
if a.startswith("-") and "=" in a:
continue
# --secretflag (no inline value): flag the next positional arg
if a.startswith("-") and is_secret_key(a):
mask_next = True
continue
if a.startswith("-"):
continue
# Positional value: check for token prefix or embedded URL credentials
if _is_secret_value(a) or _EMBEDDED_CRED_RE.search(a):
return (
"An arg value looks like a credential. "
"Where the server supports it, prefer Environment variables — "
"args are visible in process listings."
)
return None
def split_suspicious_args(args: list[str]) -> tuple[list[str], list[str]]:
"""
Detect the classic argument-entry mistake: several argv tokens typed on one
+52
View File
@@ -583,3 +583,55 @@ def test_merge_and_reapply_preserves_both_edits(tmp_path):
assert result["numStartups"] == 42 # external change preserved
assert "s2" in result["mcpServers"] # user's server edit preserved
assert "s1" not in result["mcpServers"] # old server replaced
# --------------------------------------------------------------------------- #
# 10 — Secret-in-args warning
# --------------------------------------------------------------------------- #
def test_args_secret_warning_embedded_url_creds():
"""AC case: postgres://user:pass@host triggers a warning."""
data = {"args": ["--connection", "postgres://user:pass@host/db"]}
assert c.args_secret_warning(data) is not None
def test_args_secret_warning_token_prefix_positional():
"""A bare ghp_… token as a positional arg triggers a warning."""
data = {"args": ["ghp_abc123def456"]}
assert c.args_secret_warning(data) is not None
def test_args_secret_warning_secret_flag_value_pair():
"""Value following a secret-named flag triggers a warning."""
data = {"args": ["--token", "supersecret"]}
assert c.args_secret_warning(data) is not None
def test_args_secret_warning_benign_url_no_password():
"""https:// URL without user:pass should NOT warn."""
data = {"args": ["--endpoint", "https://mcp.example.com/sse"]}
assert c.args_secret_warning(data) is None
def test_args_secret_warning_user_at_host_no_password():
"""ssh://user@host without a password should NOT warn."""
data = {"args": ["ssh://git@github.com"]}
assert c.args_secret_warning(data) is None
def test_args_secret_warning_inline_flag_value_skipped():
"""--flag=value inline pairs are self-documenting and should NOT warn."""
data = {"args": ["--api-key=sk-abc123"]}
assert c.args_secret_warning(data) is None
def test_args_secret_warning_env_not_triggered():
"""Secrets in env (not args) must not trigger args warning, even with benign args present."""
data = {"args": ["--verbose"], "env": {"DATABASE_URL": "postgres://user:pass@host/db"}}
assert c.args_secret_warning(data) is None
def test_args_secret_warning_empty():
assert c.args_secret_warning({}) is None
assert c.args_secret_warning({"args": []}) is None