feat: mask secret values in the UI and redact them from diagnostics
- Env/header values whose key looks secret (TOKEN, API_KEY, PASSWORD, AUTH, ...) render as •••••••• via a display-only delegate; a 'Show secrets' toggle reveals them. Underlying data, editing, and save are untouched. - The Add dialog switches the value field to password echo when the key name looks secret. - Copy-diagnostics now redacts secrets from args (--token <v>, --api-key=<v>, and well-known token prefixes like ghp_/sk-/xoxb-), since those reports get pasted into public bug reports. Env and header values were already omitted from diagnostics.
This commit is contained in:
@@ -35,6 +35,7 @@ from PySide6.QtWidgets import (
|
|||||||
QPushButton,
|
QPushButton,
|
||||||
QSplitter,
|
QSplitter,
|
||||||
QStackedWidget,
|
QStackedWidget,
|
||||||
|
QStyledItemDelegate,
|
||||||
QTableWidget,
|
QTableWidget,
|
||||||
QTableWidgetItem,
|
QTableWidgetItem,
|
||||||
QVBoxLayout,
|
QVBoxLayout,
|
||||||
@@ -134,6 +135,25 @@ class ConnTester(QThread):
|
|||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Small reusable: key/value editor (for env and headers)
|
# Small reusable: key/value editor (for env and headers)
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
|
class _SecretMaskDelegate(QStyledItemDelegate):
|
||||||
|
"""Masks the DISPLAY of values whose key looks like a secret (API_KEY,
|
||||||
|
TOKEN, ...). The underlying item text stays real, so editing, dump() and
|
||||||
|
save are untouched — only what's painted on screen changes."""
|
||||||
|
|
||||||
|
def __init__(self, table):
|
||||||
|
super().__init__(table)
|
||||||
|
self._table = table
|
||||||
|
self.revealed = False
|
||||||
|
|
||||||
|
def initStyleOption(self, option, index):
|
||||||
|
super().initStyleOption(option, index)
|
||||||
|
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()):
|
||||||
|
option.text = core.MASK
|
||||||
|
|
||||||
|
|
||||||
class KeyValueTable(QWidget):
|
class KeyValueTable(QWidget):
|
||||||
def __init__(self, key_label="Key", val_label="Value", on_change=None, before_change=None):
|
def __init__(self, key_label="Key", val_label="Value", on_change=None, before_change=None):
|
||||||
super().__init__()
|
super().__init__()
|
||||||
@@ -154,17 +174,29 @@ class KeyValueTable(QWidget):
|
|||||||
self.table.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
|
self.table.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
|
||||||
self.table.setMinimumHeight(90)
|
self.table.setMinimumHeight(90)
|
||||||
self.table.itemChanged.connect(self._changed)
|
self.table.itemChanged.connect(self._changed)
|
||||||
|
# Secret-looking values (API_KEY, TOKEN, ...) render masked by default.
|
||||||
|
self._mask_delegate = _SecretMaskDelegate(self.table)
|
||||||
|
self.table.setItemDelegateForColumn(1, self._mask_delegate)
|
||||||
lay.addWidget(self.table, 1)
|
lay.addWidget(self.table, 1)
|
||||||
row = QHBoxLayout()
|
row = QHBoxLayout()
|
||||||
add = QPushButton("+ Add")
|
add = QPushButton("+ Add")
|
||||||
rem = QPushButton("− Remove")
|
rem = QPushButton("− Remove")
|
||||||
add.clicked.connect(self._add_row)
|
add.clicked.connect(self._add_row)
|
||||||
rem.clicked.connect(self._remove_row)
|
rem.clicked.connect(self._remove_row)
|
||||||
|
self.reveal_btn = QPushButton("Show secrets")
|
||||||
|
self.reveal_btn.setCheckable(True)
|
||||||
|
self.reveal_btn.toggled.connect(self._toggle_reveal)
|
||||||
row.addWidget(add)
|
row.addWidget(add)
|
||||||
row.addWidget(rem)
|
row.addWidget(rem)
|
||||||
row.addStretch()
|
row.addStretch()
|
||||||
|
row.addWidget(self.reveal_btn)
|
||||||
lay.addLayout(row)
|
lay.addLayout(row)
|
||||||
|
|
||||||
|
def _toggle_reveal(self, on):
|
||||||
|
self._mask_delegate.revealed = on
|
||||||
|
self.reveal_btn.setText("Hide secrets" if on else "Show secrets")
|
||||||
|
self.table.viewport().update()
|
||||||
|
|
||||||
def _changed(self, *_):
|
def _changed(self, *_):
|
||||||
if self._on_change:
|
if self._on_change:
|
||||||
self._on_change()
|
self._on_change()
|
||||||
@@ -195,6 +227,15 @@ class KeyValueTable(QWidget):
|
|||||||
grid.addWidget(val_edit, 1, 1)
|
grid.addWidget(val_edit, 1, 1)
|
||||||
v.addLayout(grid)
|
v.addLayout(grid)
|
||||||
|
|
||||||
|
# Type an API_KEY/TOKEN-style name and the value field masks itself.
|
||||||
|
def _sync_echo(text):
|
||||||
|
secret = core.is_secret_key(text)
|
||||||
|
val_edit.setEchoMode(
|
||||||
|
QLineEdit.EchoMode.Password if secret else QLineEdit.EchoMode.Normal
|
||||||
|
)
|
||||||
|
|
||||||
|
key_edit.textChanged.connect(_sync_echo)
|
||||||
|
|
||||||
btns = QDialogButtonBox(
|
btns = QDialogButtonBox(
|
||||||
QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel
|
QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel
|
||||||
)
|
)
|
||||||
|
|||||||
+80
-1
@@ -596,6 +596,84 @@ def parse_pasted_json(text: str) -> dict[str, dict]:
|
|||||||
return servers
|
return servers
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Secrets: detection + redaction
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
_SECRET_KEY_RE = re.compile(
|
||||||
|
r"(?i)(token|secret|passw|api[-_]?key|apikey|auth|credential|bearer|private[-_]?key|access[-_]?key)"
|
||||||
|
)
|
||||||
|
|
||||||
|
# Well-known token prefixes that identify a bare value as a secret even
|
||||||
|
# without a telling key/flag name next to it.
|
||||||
|
_TOKEN_PREFIXES = (
|
||||||
|
"ghp_",
|
||||||
|
"gho_",
|
||||||
|
"ghu_",
|
||||||
|
"ghs_",
|
||||||
|
"github_pat_", # GitHub
|
||||||
|
"sk-",
|
||||||
|
"sk_live_",
|
||||||
|
"sk_test_",
|
||||||
|
"rk_live_", # OpenAI / Stripe
|
||||||
|
"xoxb-",
|
||||||
|
"xoxp-",
|
||||||
|
"xoxc-",
|
||||||
|
"xoxs-",
|
||||||
|
"xapp-", # Slack
|
||||||
|
"glpat-",
|
||||||
|
"gldt-", # GitLab
|
||||||
|
"AKIA",
|
||||||
|
"ASIA", # AWS access key IDs
|
||||||
|
"ya29.",
|
||||||
|
"AIza", # Google
|
||||||
|
"pypi-",
|
||||||
|
"npm_",
|
||||||
|
"dop_v1_",
|
||||||
|
"figd_",
|
||||||
|
)
|
||||||
|
|
||||||
|
MASK = "••••••••"
|
||||||
|
|
||||||
|
|
||||||
|
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 ""))
|
||||||
|
|
||||||
|
|
||||||
|
def _is_secret_value(value: str) -> bool:
|
||||||
|
return isinstance(value, str) and value.startswith(_TOKEN_PREFIXES)
|
||||||
|
|
||||||
|
|
||||||
|
def redact_args(args: list[str]) -> list[str]:
|
||||||
|
"""
|
||||||
|
Mask secret values in an args list for display/diagnostics:
|
||||||
|
--token abc123 -> --token •••••••• (value after a secret flag)
|
||||||
|
--api-key=abc123 -> --api-key=•••••••• (inline flag=value)
|
||||||
|
ghp_abc123 -> •••••••• (well-known token prefix)
|
||||||
|
Everything else passes through untouched.
|
||||||
|
"""
|
||||||
|
out: list[str] = []
|
||||||
|
mask_next = False
|
||||||
|
for a in args:
|
||||||
|
s = str(a)
|
||||||
|
if mask_next:
|
||||||
|
out.append(MASK)
|
||||||
|
mask_next = False
|
||||||
|
continue
|
||||||
|
if s.startswith("-") and "=" in s and is_secret_key(s.split("=", 1)[0]):
|
||||||
|
out.append(s.split("=", 1)[0] + "=" + MASK)
|
||||||
|
continue
|
||||||
|
if s.startswith("-") and is_secret_key(s):
|
||||||
|
out.append(s)
|
||||||
|
mask_next = True
|
||||||
|
continue
|
||||||
|
if _is_secret_value(s):
|
||||||
|
out.append(MASK)
|
||||||
|
continue
|
||||||
|
out.append(s)
|
||||||
|
return out
|
||||||
|
|
||||||
|
|
||||||
def split_suspicious_args(args: list[str]) -> tuple[list[str], list[str]]:
|
def split_suspicious_args(args: list[str]) -> tuple[list[str], list[str]]:
|
||||||
"""
|
"""
|
||||||
Detect the classic argument-entry mistake: several argv tokens typed on one
|
Detect the classic argument-entry mistake: several argv tokens typed on one
|
||||||
@@ -944,7 +1022,8 @@ def diagnostics_text(name: str, data: dict) -> str:
|
|||||||
L.append(f"Server: {name or '(unnamed)'} [local / stdio]")
|
L.append(f"Server: {name or '(unnamed)'} [local / stdio]")
|
||||||
L.append(f"Command: {data.get('command', '')}")
|
L.append(f"Command: {data.get('command', '')}")
|
||||||
if data.get("args"):
|
if data.get("args"):
|
||||||
L.append("Args: " + " ".join(str(a) for a in data["args"]))
|
# Redacted: this report is designed to be pasted into bug reports.
|
||||||
|
L.append("Args: " + " ".join(redact_args(list(data["args"]))))
|
||||||
if data.get("env"):
|
if data.get("env"):
|
||||||
L.append("Env keys: " + ", ".join(data["env"].keys()))
|
L.append("Env keys: " + ", ".join(data["env"].keys()))
|
||||||
L.append(f"Status: {res['status'].upper()}")
|
L.append(f"Status: {res['status'].upper()}")
|
||||||
|
|||||||
+46
-1
@@ -176,7 +176,52 @@ def test_valid_set_passes_clean():
|
|||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# 5. Dependency / PATH checking
|
# 5. Secrets: detection + redaction
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_is_secret_key_positives():
|
||||||
|
for k in (
|
||||||
|
"BRAVE_API_KEY",
|
||||||
|
"BEARER_TOKEN",
|
||||||
|
"password",
|
||||||
|
"GH_TOKEN",
|
||||||
|
"Authorization",
|
||||||
|
"client_secret",
|
||||||
|
"PRIVATE_KEY",
|
||||||
|
"aws_access_key_id",
|
||||||
|
):
|
||||||
|
assert c.is_secret_key(k), k
|
||||||
|
|
||||||
|
|
||||||
|
def test_is_secret_key_negatives():
|
||||||
|
for k in ("JOOMLA_BASE_URL", "PORT", "HOME", "PATH", "debug", "TIMEOUT"):
|
||||||
|
assert not c.is_secret_key(k), k
|
||||||
|
|
||||||
|
|
||||||
|
def test_redact_args_flag_value():
|
||||||
|
assert c.redact_args(["--token", "abc123", "run"]) == ["--token", c.MASK, "run"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_redact_args_inline_flag():
|
||||||
|
assert c.redact_args(["--api-key=abc123"]) == [f"--api-key={c.MASK}"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_redact_args_known_prefix():
|
||||||
|
assert c.redact_args(["ghp_abcdef123456"]) == [c.MASK]
|
||||||
|
|
||||||
|
|
||||||
|
def test_redact_args_leaves_normal_args():
|
||||||
|
args = ["--directory", "/path/to/server", "run", "main.py", "-y"]
|
||||||
|
assert c.redact_args(args) == args
|
||||||
|
|
||||||
|
|
||||||
|
def test_diagnostics_redacts_token_args():
|
||||||
|
txt = c.diagnostics_text("t", {"command": "npx", "args": ["--token", "s3cret-value"]})
|
||||||
|
assert "s3cret-value" not in txt
|
||||||
|
assert c.MASK in txt
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# 6. Dependency / PATH checking
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
def test_dep_check_finds_python3():
|
def test_dep_check_finds_python3():
|
||||||
assert c.check_dependency({"command": "python3"})["status"] == "ok"
|
assert c.check_dependency({"command": "python3"})["status"] == "ok"
|
||||||
|
|||||||
Reference in New Issue
Block a user