1 Commits

Author SHA1 Message Date
Cowork Supervisor dffa0e152f Browse catalog dialog (issue #10 phase 2)
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / windows-latest) (pull_request) Failing after 23s
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 11s
CI / Catalog signature (pull_request) Successful in 7s
Adds the user-facing half of the server catalog: a searchable, signed
catalog browser wired to the existing paste/import path.

bcc_core.py (pure, GUI-free, per the design comment on #10):
- CATALOG_CATEGORY_GROUPS / catalog_category_group(): collapses the
  raw taxonomy to the 7 UI chips (unknown categories fall back to
  Other, never drop an entry).
- catalog_entry_matches_query() / filter_catalog_entries() /
  catalog_entries_in_group(): search-as-you-type + category chip
  filtering over catalog entries.
- format_freshness_hint(): last_release ISO date -> "Last updated N
  months/years ago", '' when missing/unparseable/future.
- first_unfilled_focus_target(): finds the first <PLACEHOLDER> arg or
  blank env_required key so the GUI can focus it after Add.
- load_bundled_catalog_entries(): reads+verifies+validates the
  bundled catalog.json/.sig pair, returns servers or an EMPTY list on
  ANY failure -- the load-bearing guarantee behind the dialog's empty
  state.
- Bug fix: catalog_entry_to_paste_json() only copied config.env,
  ignoring env_required -- the field where 9 of the 19 seed entries
  (postgres, github, notion, obsidian, brave-search, tavily,
  home-assistant, n8n, grafana) actually declare their secret var
  names. Add would have silently added these servers with no env
  field for the user to fill in. Now env_required keys are seeded as
  empty-string placeholders unless config.env already sets them.

bcc.py:
- BrowseCatalogDialog: search box, category chips, list, detail pane
  (description/notes/homepage/freshness hint/verbatim command
  preview), per-tier action (Add for basic, Open setup docs for
  link-only). Every catalog-derived string goes through plain_label()
  (Qt.PlainText + html.escape, matching catalog_console.py's
  approach) or an inherently-plain QPlainTextEdit for the command
  preview -- catalog.json takes community PRs, so every field is
  attacker-influenceable.
- "Browse catalog…" button next to Paste JSON.
- ServerEditor.focus_target(): selects the first unfilled arg line or
  opens the env-table cell editor for the first blank env row.
- Save-time placeholder guard: warns (Yes/No, defaults No) when any
  server still has an unfilled <PLACEHOLDER>; never blocks a
  deliberate save, never saves one unnoticed.

bcc.spec: bundle data/catalog.json.sig alongside catalog.json -- the
signature file was missing from datas, so a frozen build would have
had a catalog.json with no matching .sig for resolve_catalog() to
verify against.

tests/test_core.py: +82 tests covering category-group mapping,
catalog search/filter, freshness-hint formatting (including the
month/year rounding boundary), first_unfilled_focus_target, the
catalog_entry_to_paste_json env_required fix (incl. a regression
against the real shipped postgres entry), and
load_bundled_catalog_entries under valid/tampered/wrong-key/missing-
file/invalid-but-signed conditions plus an end-to-end check against
the real data/catalog.json + .sig (19 entries).

GUI code (BrowseCatalogDialog, ServerEditor.focus_target,
MainWindow.browse_catalog) is unavoidably untested here -- PySide6
cannot import in this sandbox (missing libEGL/system GL libs) -- but
all the logic it depends on is pushed into bcc_core.py and covered
above.
2026-07-12 19:00:54 -04:00
11 changed files with 1089 additions and 2123 deletions
+2 -50
View File
@@ -95,39 +95,12 @@ jobs:
- name: Install dependencies - name: Install dependencies
run: pip install cryptography run: pip install cryptography
# 🔴 TRUST ANCHOR — issue #68 finding 4.
#
# This step used to do `import bcc_core as c` FROM THE CHECKED-OUT PR
# BRANCH and verify the catalog against c.CATALOG_PUBKEYS — i.e. it
# trusted the public key shipped in the very diff it was reviewing. A
# PR that changed data/catalog.json AND bcc_core.CATALOG_PUBKEYS (to
# an attacker key, with a matching signature produced by the attacker's
# matching private key) went green, because there was nothing outside
# the PR's own content to check the key against. The gate's whole
# point is catching a friendly-looking PR the maintainer merges
# without really reading it — and that hole made it a two-file diff.
#
# EXPECTED_CATALOG_PUBKEY_B64 below is hardcoded HERE, in the workflow
# file, independent of whatever bcc_core.py says on the PR branch. It
# is intentionally the only line in this step that matters for
# security review: changing it changes what this gate is willing to
# trust. THIS CONSTANT IS A TRUST ANCHOR. A PR that changes this line
# in the same diff as a catalog change is exactly the attack this gate
# exists to prevent — review a change to this line on its own,
# never bundled with a catalog update.
#
# NOTE for the next key rotation: update EXPECTED_CATALOG_PUBKEY_B64
# below to the new key's base64 form, as its own reviewed change.
- name: Verify data/catalog.json.sig - name: Verify data/catalog.json.sig
env:
EXPECTED_CATALOG_PUBKEY_B64: "082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="
run: | run: |
python - <<'PY' python - <<'PY'
import base64, os, pathlib, sys import pathlib, sys
import bcc_core as c import bcc_core as c
expected_pubkey_b64 = os.environ["EXPECTED_CATALOG_PUBKEY_B64"]
raw = pathlib.Path("data/catalog.json").read_bytes() raw = pathlib.Path("data/catalog.json").read_bytes()
sig_path = pathlib.Path("data/catalog.json.sig") sig_path = pathlib.Path("data/catalog.json.sig")
@@ -138,26 +111,6 @@ jobs:
if b"\x00" * 32 in c.CATALOG_PUBKEYS: if b"\x00" * 32 in c.CATALOG_PUBKEYS:
sys.exit("FAIL: CATALOG_PUBKEYS still holds the placeholder key.") sys.exit("FAIL: CATALOG_PUBKEYS still holds the placeholder key.")
# Trust anchor check FIRST, before verifying anything against
# bcc_core.CATALOG_PUBKEYS: a PR is not allowed to bring its own
# key. CATALOG_PUBKEYS on the checked-out branch must be EXACTLY
# the key(s) this workflow file itself expects -- no more, no
# fewer, no substitutions.
actual_pubkeys_b64 = [base64.b64encode(k).decode() for k in c.CATALOG_PUBKEYS]
if actual_pubkeys_b64 != [expected_pubkey_b64]:
sys.exit(
"FAIL: bcc_core.CATALOG_PUBKEYS on this branch does not match the "
"trust anchor hardcoded in .github/workflows/ci.yml.\n"
f" expected: {[expected_pubkey_b64]}\n"
f" actual: {actual_pubkeys_b64}\n"
"\n"
"This PR is changing (or has changed) the catalog signing key. That "
"change must be reviewed on its own, separately from any catalog "
"content change, and the workflow's EXPECTED_CATALOG_PUBKEY_B64 "
"updated deliberately -- not accepted because it happened to match "
"whatever bcc_core.py says on this branch."
)
if not c.verify_catalog_signature(raw, sig_path.read_bytes(), c.CATALOG_PUBKEYS): if not c.verify_catalog_signature(raw, sig_path.read_bytes(), c.CATALOG_PUBKEYS):
sys.exit( sys.exit(
"FAIL: data/catalog.json does NOT match its signature.\n" "FAIL: data/catalog.json does NOT match its signature.\n"
@@ -171,6 +124,5 @@ jobs:
if problems: if problems:
sys.exit("FAIL: catalog failed validation:\n " + "\n ".join(problems)) sys.exit("FAIL: catalog failed validation:\n " + "\n ".join(problems))
print("OK: catalog signature verifies, the pubkey matches the CI trust anchor, " print("OK: catalog signature verifies and the catalog validates clean.")
"and the catalog validates clean.")
PY PY
+22 -49
View File
@@ -101,20 +101,9 @@ jobs:
# signing step — which means a wrong/missing RELEASE_SIGNING_KEY secret # signing step — which means a wrong/missing RELEASE_SIGNING_KEY secret
# would only be discovered at the worst possible moment: during a real # would only be discovered at the worst possible moment: during a real
# release. This job signs a throwaway manifest with the secret and verifies # release. This job signs a throwaway manifest with the secret and verifies
# the result against scripts/sign_checksums.RELEASE_PUBKEYS. # the result against the PUBLIC key already compiled into bcc_core.
# #
# IMPORTANT (issue #68 finding 5): this must verify against the RELEASE # It proves the two halves of the keypair actually match, without
# public key, never bcc_core.CATALOG_PUBKEYS. The catalog key is the
# offline, maintainer-held root of trust for what BCC executes; it must
# NEVER be compared against a value that lives in a CI secret, because
# that comparison is itself a way to smuggle a catalog-trusted key through
# CI review ("does this repo secret match the catalog key" is a question
# this workflow must never even ask). The release key is a SEPARATE
# keypair, generated via `catalog_console.py keygen --release`, that only
# ever signs release SHA256SUMS manifests -- a CI/secret compromise burns
# this key, not the catalog key.
#
# It proves the two halves of the RELEASE keypair actually match, without
# publishing anything. Run it from the Actions tab after setting or # publishing anything. Run it from the Actions tab after setting or
# rotating the secret. # rotating the secret.
signing-smoke-test: signing-smoke-test:
@@ -131,54 +120,42 @@ jobs:
- name: Install dependencies - name: Install dependencies
run: pip install cryptography run: pip install cryptography
- name: Sign a throwaway manifest and verify against the RELEASE pubkey - name: Sign a throwaway manifest and verify against the shipped pubkey
env: env:
RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }} RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }}
run: | run: |
if [ -z "$RELEASE_SIGNING_KEY" ]; then if [ -z "$RELEASE_SIGNING_KEY" ]; then
echo "FAIL: RELEASE_SIGNING_KEY secret is not set." echo "FAIL: RELEASE_SIGNING_KEY secret is not set."
echo "Generate the RELEASE key (NOT the catalog key) with:" echo "Generate it with: python catalog_console.py show-seed-b64"
echo " python catalog_console.py keygen --release" echo "then add it under Settings -> Actions -> Secrets."
echo "then add its seed under Settings -> Actions -> Secrets, via:"
echo " python catalog_console.py show-seed-b64 --release"
exit 1 exit 1
fi fi
mkdir -p smoke && echo "smoke test payload" > smoke/hello.txt mkdir -p smoke && echo "smoke test payload" > smoke/hello.txt
python3 scripts/sign_checksums.py generate smoke --out smoke/SHA256SUMS python3 scripts/sign_checksums.py generate smoke --out smoke/SHA256SUMS
python3 scripts/sign_checksums.py sign --sums smoke/SHA256SUMS --out smoke/SHA256SUMS.sig python3 scripts/sign_checksums.py sign --sums smoke/SHA256SUMS --out smoke/SHA256SUMS.sig
python - <<'PY' python - <<'PY'
import pathlib, sys import base64, pathlib, sys
from scripts.sign_checksums import RELEASE_PUBKEYS, verify_checksums_against_any import bcc_core as c
from scripts.sign_checksums import verify_checksums
# Deliberately does NOT import bcc_core / CATALOG_PUBKEYS at all -- # The public half that ships inside the binary. If the secret is a
# this smoke test must never be able to compare the CI secret # DIFFERENT key than the one users' copies trust, this fails here --
# against the catalog's root of trust (issue #68 finding 5). Only # which is the entire point of the job.
# RELEASE_PUBKEYS (scripts/sign_checksums.py) is a legitimate pub_b64 = base64.b64encode(c.CATALOG_PUBKEYS[0]).decode()
# target for a CI-resident key.
if not RELEASE_PUBKEYS:
sys.exit(
"FAIL: scripts/sign_checksums.RELEASE_PUBKEYS is empty.\n"
"\n"
"Generate the release keypair with:\n"
" python catalog_console.py keygen --release\n"
"then paste the printed public key into RELEASE_PUBKEYS in\n"
"scripts/sign_checksums.py and commit that change."
)
sums = pathlib.Path("smoke/SHA256SUMS").read_text() sums = pathlib.Path("smoke/SHA256SUMS").read_text()
sig = pathlib.Path("smoke/SHA256SUMS.sig").read_bytes() sig = pathlib.Path("smoke/SHA256SUMS.sig").read_bytes()
if not verify_checksums_against_any(RELEASE_PUBKEYS, sums, sig): if not verify_checksums(pub_b64, sums, sig):
sys.exit( sys.exit(
"FAIL: the signature produced by RELEASE_SIGNING_KEY does NOT verify\n" "FAIL: the signature produced by RELEASE_SIGNING_KEY does NOT verify\n"
"against any key in scripts/sign_checksums.RELEASE_PUBKEYS.\n" "against the public key in bcc_core.CATALOG_PUBKEYS.\n"
"\n" "\n"
"The secret and the shipped release public key are different keypairs.\n" "The secret and the shipped public key are different keypairs. Users\n"
"Downloaders would reject every signature this CI produces. Re-copy the\n" "would reject every signature this CI produces. Re-copy the seed from\n"
"seed from `catalog_console.py show-seed-b64 --release`, or update\n" "`catalog_console.py show-seed-b64`, or update CATALOG_PUBKEYS."
"RELEASE_PUBKEYS with the matching public key."
) )
print("OK: RELEASE_SIGNING_KEY matches a key in RELEASE_PUBKEYS.") print("OK: RELEASE_SIGNING_KEY matches the public key shipped in bcc_core.")
PY PY
# ── Create GitHub Release with all three artifacts ────────────────────── # ── Create GitHub Release with all three artifacts ──────────────────────
@@ -229,13 +206,9 @@ jobs:
# checks. It does NOT remove Gatekeeper/SmartScreen warnings. # checks. It does NOT remove Gatekeeper/SmartScreen warnings.
# #
# The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw # The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw
# Ed25519 seed) for the RELEASE key -- a SEPARATE keypair from the # Ed25519 seed) generated via the Catalog Console (#62). If it's not
# catalog key, generated via `python catalog_console.py keygen # set, we still publish the release — just without a .sig — rather
# --release` (issue #68 finding 5; #62). This key is intentionally # than fail the release outright.
# CI-resident and signs ONLY this checksum manifest; it is never
# trusted to sign data/catalog.json. If it's not set, we still
# publish the release — just without a .sig — rather than fail the
# release outright.
- name: Check for signing key - name: Check for signing key
id: signing id: signing
run: | run: |
@@ -261,7 +234,7 @@ jobs:
- name: Warn — release will be unsigned - name: Warn — release will be unsigned
if: steps.signing.outputs.has_key != 'true' if: steps.signing.outputs.has_key != 'true'
run: | run: |
echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Generate the RELEASE key (python catalog_console.py keygen --release) and add its seed (python catalog_console.py show-seed-b64 --release) as this secret before the next tag." echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Add the secret (base64 raw Ed25519 seed, generated via the Catalog Console, #62) before the next tag."
- name: Create GitHub Release - name: Create GitHub Release
uses: softprops/action-gh-release@v2 uses: softprops/action-gh-release@v2
+6 -45
View File
@@ -36,10 +36,11 @@ are only suppressed by a paid OS-vendor certificate, which this project
doesn't have. Verifying checksums is about detecting tampering in transit or doesn't have. Verifying checksums is about detecting tampering in transit or
on a mirror, not about vouching for the software. on a mirror, not about vouching for the software.
This manifest is signed with BCC's **release key**, which is a different **Release signing public key** (Ed25519, base64, raw 32 bytes):
key from the one that signs the MCP server catalog — see
[Signing keys](#signing-keys) below for why, and for the public key value ```
to use with `--pubkey-b64` below. <PLACEHOLDER — AJ: paste the public key from the Catalog Console (#62) here>
```
### macOS / Linux ### macOS / Linux
@@ -58,7 +59,7 @@ To also verify the manifest's signature (optional, requires Python +
```bash ```bash
python3 scripts/sign_checksums.py verify \ python3 scripts/sign_checksums.py verify \
--sums SHA256SUMS --sig SHA256SUMS.sig \ --sums SHA256SUMS --sig SHA256SUMS.sig \
--pubkey-b64 "<the release public key from Signing keys, below>" --pubkey-b64 "<the public key above>"
``` ```
### Windows (PowerShell) ### Windows (PowerShell)
@@ -77,45 +78,6 @@ release is missing the `.sig` file, the checksums themselves are still
valid and safe to check against — the release workflow only skips signing, valid and safe to check against — the release workflow only skips signing,
never checksum generation. never checksum generation.
## Signing keys
BCC uses **two separate Ed25519 keypairs**, deliberately never the same
key, because they protect different things and live in different places:
| | Catalog key | Release key |
|---|---|---|
| Signs | `data/catalog.json` (the MCP server catalog every user's app trusts) | `SHA256SUMS` (the checksum manifest for release binaries) |
| Verified by | `bcc_core.CATALOG_PUBKEYS` | `scripts/sign_checksums.RELEASE_PUBKEYS` |
| Lives | Offline, passphrase-encrypted, maintainer's machine only (OS keychain or an encrypted file outside the repo — see the [Catalog Console](#files), issue #62) | A Gitea Actions repo secret, `RELEASE_SIGNING_KEY`**intentionally CI-resident** |
| Generated with | `python catalog_console.py keygen` | `python catalog_console.py keygen --release` |
| Exported for CI with | *(never — there is no supported way to export this key)* | `python catalog_console.py show-seed-b64 --release` |
**Why two keys:** the catalog key is the root of trust for what BCC
actually *executes* on a user's machine — every `command`/`args` pair in
the shipped catalog is only there because this key signed it. If that key
and the release-checksum key were the same (as they briefly were — see
[issue #68](../../issues/68)), then anything that can exfiltrate a Gitea
Actions secret (a malicious workflow-file PR, a compromised runner, a leaky
log) could sign a catalog every user's copy of BCC would trust, not just a
checksum manifest. Splitting them means **a CI/secret compromise burns the
release key, never the catalog key** — checksums for a future release could
be forged, which is bad, but no attacker gains the ability to make BCC run
arbitrary commands on installs that trust the catalog. That asymmetry is
the entire point of having two keys instead of one.
The catalog key is **never** meant to leave the maintainer's machine: it's
generated, stored, unlocked, and used to sign entirely inside the Catalog
Console (`catalog_console.py`), and `catalog_console.py show-seed-b64`
refuses to run without `--release` specifically so the catalog seed can't
be exported by habit or muscle memory.
**Release signing public key** (Ed25519, base64, raw 32 bytes) — this is
the RELEASE key, not the catalog key:
```
<PLACEHOLDER — AJ: paste the release public key from `catalog_console.py keygen --release` here>
```
## Run from source ## Run from source
```bash ```bash
@@ -190,7 +152,6 @@ file is also listed, marked *legacy*, so you can copy them over.
- `bcc.spec` — PyInstaller build spec (cross-platform). - `bcc.spec` — PyInstaller build spec (cross-platform).
- `scripts/build_icons.py` — regenerates `icons/app.icns` and `icons/app.ico` from source PNGs. - `scripts/build_icons.py` — regenerates `icons/app.icns` and `icons/app.ico` from source PNGs.
- `scripts/sign_checksums.py` — generates and Ed25519-signs the release `SHA256SUMS` manifest (see [Verifying your download](#verifying-your-download)). - `scripts/sign_checksums.py` — generates and Ed25519-signs the release `SHA256SUMS` manifest (see [Verifying your download](#verifying-your-download)).
- `catalog_console.py` / `catalog_review.py`**maintainer-only**, never shipped to users (excluded from `bcc.spec`; see `tests/test_catalog_console_packaging.py`). The Catalog Console: review + sign `data/catalog.json`, and generate/manage both signing keys (`keygen`, `keygen --release`) — see [Signing keys](#signing-keys).
## Building from source ## Building from source
+435 -285
View File
@@ -10,7 +10,7 @@ Run: python mcp_manager.py
from __future__ import annotations from __future__ import annotations
import contextlib import html
import sys import sys
import time import time
from pathlib import Path from pathlib import Path
@@ -19,7 +19,6 @@ from typing import ClassVar
from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal
from PySide6.QtGui import ( from PySide6.QtGui import (
QAction, QAction,
QActionGroup,
QColor, QColor,
QCursor, QCursor,
QDesktopServices, QDesktopServices,
@@ -27,12 +26,13 @@ from PySide6.QtGui import (
QIcon, QIcon,
QKeySequence, QKeySequence,
QPainter, QPainter,
QPalette,
QPixmap, QPixmap,
QTextCursor,
) )
from PySide6.QtWidgets import ( from PySide6.QtWidgets import (
QAbstractItemView, QAbstractItemView,
QApplication, QApplication,
QButtonGroup,
QCheckBox, QCheckBox,
QComboBox, QComboBox,
QDialog, QDialog,
@@ -68,122 +68,105 @@ import bcc_core as core
# thread during drag-and-drop import, so skip anything larger than this. # thread during drag-and-drop import, so skip anything larger than this.
MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024 # 5 MB MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024 # 5 MB
# --- Theming (issue #75) -------------------------------------------------- #
# The palette lives in bcc_core (testable without a Qt app); these module-level
# names are rebound by `apply_palette()` whenever the theme changes.
#
# Why globals rather than passing a palette around: ~20 inline
# `setStyleSheet(f"color: {MUTED}")` calls are scattered through this file, and
# an f-string resolves its names when it runs, not when it's compiled. Rebinding
# the globals means every one of those call sites picks up the new colour on its
# next render, with no change to the call sites themselves.
PALETTE = core.DARK_PALETTE
ACCENT = ACCENT_DIM = BG = PANEL = PANEL_2 = TEXT = MUTED = BORDER = ""
GOOD = BAD = WARN = REMOTE = ON_ACCENT = DISABLED_BG = MONO_BG = SEL_TEXT = ""
STATUS_COLORS: dict[str, str] = {}
HEALTH_COLORS: dict[str, str] = {}
STATUS_GLYPH = { def plain_label(text: object) -> QLabel:
"ok": "\u25cf", """A QLabel guaranteed to render `text` as plain text, never HTML.
"missing": "\u25cf",
"warn": "\u25b2", Qt's QLabel auto-interprets HTML by default (Qt.AutoText). Every catalog
"remote": "\u25c6", entry field (description, notes, display name, urls -- and especially
"unknown": "\u25cb", args) is attacker-influenceable: catalog.json accepts community PRs, and
} only a valid Ed25519 signature stands between a PR and what a user sees
here. A `<b>` or `<img onerror=...>` in a description must render as
visible text, not markup -- exactly the same reasoning catalog_console.py
documents for its own plain_label(). Every catalog-derived string shown
by the Browse dialog MUST go through this helper (or an inherently
plain-text widget like QPlainTextEdit) rather than a bare QLabel(...).
"""
label = QLabel(html.escape(str(text)))
label.setTextFormat(Qt.TextFormat.PlainText)
label.setWordWrap(True)
return label
# --- One-line rebrand: change this to recolor the whole app --------------- #
ACCENT = "#f97316" # warm orange
ACCENT_DIM = "#c2570b"
BG = "#1b1d23"
PANEL = "#23262e"
PANEL_2 = "#2b2f39"
TEXT = "#e7e9ee"
MUTED = "#9aa0ad"
BORDER = "#3a3f4b"
GOOD = "#4ade80"
BAD = "#f87171"
WARN = "#fbbf24"
STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
STATUS_GLYPH = {"ok": "", "missing": "", "warn": "", "remote": "", "unknown": ""}
# Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the # Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the
# server tables' "Health" column -- distinct from the PATH-dependency Status # server tables' "Health" column -- distinct from the PATH-dependency Status
# column above. # column above.
HEALTH_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"} HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
HEALTH_GLYPH = {"ok": "", "failed": "", "untested": ""}
STYLESHEET = f"""
def build_stylesheet(p: core.Palette) -> str:
"""Render the global QSS for a palette."""
return f"""
/* No font-family here on purpose: Qt already uses the native system UI font /* No font-family here on purpose: Qt already uses the native system UI font
on every platform (San Francisco / Segoe UI / desktop default). Naming on every platform (San Francisco / Segoe UI / desktop default). Naming
web-CSS aliases like -apple-system forces a costly font-alias scan. */ web-CSS aliases like -apple-system forces a costly font-alias scan. */
* {{ font-size: 13px; color: {p.text}; }} * {{ font-size: 13px; color: {TEXT}; }}
QMainWindow, QDialog {{ background: {p.bg}; }} QMainWindow, QDialog {{ background: {BG}; }}
QLabel#h1 {{ font-size: 15px; font-weight: 600; }} QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
QLabel#muted {{ color: {p.muted}; }} QLabel#muted {{ color: {MUTED}; }}
QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }} QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }}
QLineEdit, QPlainTextEdit, QComboBox {{ QLineEdit, QPlainTextEdit, QComboBox {{
background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px; background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
padding: 6px 8px; selection-background-color: {p.accent}; selection-color: {p.on_accent}; padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205;
}} }}
QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {p.accent}; }} QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {ACCENT}; }}
QComboBox::drop-down {{ border: none; width: 22px; }} QComboBox::drop-down {{ border: none; width: 22px; }}
QComboBox QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border}; QComboBox QAbstractItemView {{ background: {PANEL_2}; border: 1px solid {BORDER};
selection-background-color: {p.accent}; outline: none; }} selection-background-color: {ACCENT}; outline: none; }}
QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px; QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
padding: 7px 13px; }} padding: 7px 13px; }}
QPushButton:hover {{ border: 1px solid {p.accent}; }} QPushButton:hover {{ border: 1px solid {ACCENT}; }}
QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }} QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }}
QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }} QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }}
QPushButton#primary:hover {{ background: {p.accent_dim}; }} QPushButton#primary:hover {{ background: {ACCENT_DIM}; }}
QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }} QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }}
QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }} QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }}
QTableWidget {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px;
gridline-color: transparent; outline: none; }} gridline-color: transparent; outline: none; }}
QTableWidget::item {{ padding: 6px 8px; border: none; }} QTableWidget::item {{ padding: 6px 8px; border: none; }}
QTableWidget::item:selected {{ background: {p.accent}; color: {p.on_accent}; }} QTableWidget::item:selected {{ background: {ACCENT}; color: #1a1205; }}
/* Inline cell editors: the global QLineEdit padding/radius clips the text /* Inline cell editors: the global QLineEdit padding/radius clips the text
inside a table row, so give editors a compact, flat style instead. */ inside a table row, so give editors a compact, flat style instead. */
QTableWidget QLineEdit {{ QTableWidget QLineEdit {{
background: {p.panel_2}; color: {p.text}; border: 1px solid {p.accent}; background: {PANEL_2}; color: {TEXT}; border: 1px solid {ACCENT};
border-radius: 3px; padding: 0px 4px; margin: 0px; border-radius: 3px; padding: 0px 4px; margin: 0px;
selection-background-color: {p.accent_dim}; selection-color: {p.selection_text}; selection-background-color: {ACCENT_DIM}; selection-color: #ffffff;
}} }}
QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none; QHeaderView::section {{ background: {PANEL}; color: {MUTED}; border: none;
border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }} border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }}
QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }} QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }}
QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-height: 24px; }} QScrollBar::handle:vertical {{ background: {BORDER}; border-radius: 5px; min-height: 24px; }}
QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }} QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }} QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }}
QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }} QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
QFrame#noticeBanner {{ background: {p.panel_2}; border: 1px solid {p.accent}; border-radius: 8px; }} QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
QLabel#noticeText {{ color: {p.text}; }} QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }} QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }}
QPushButton#noticeClose:hover {{ color: {p.text}; }} QTableWidget#disabledTable {{ background: #202229; }}
QLabel#section {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }} QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }}
QLabel#sectionDisabled {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
QLabel#placeholder {{ color: {p.muted}; padding: 12px; background: {p.panel_2}; border: 1px dashed {p.border}; border-radius: 8px; }}
QTableWidget#disabledTable {{ background: {p.disabled_bg}; }}
QTableWidget#disabledTable::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
QPlainTextEdit#diag {{ font-family: "Menlo", "Cascadia Code", "Consolas", "DejaVu Sans Mono", monospace; QPlainTextEdit#diag {{ font-family: "Menlo", "Cascadia Code", "Consolas", "DejaVu Sans Mono", monospace;
font-size: 12px; background: {p.mono_bg}; border: 1px solid {p.border}; border-radius: 8px; }} font-size: 12px; background: #16181d; border: 1px solid {BORDER}; border-radius: 8px; }}
QFrame#diagCard {{ background: transparent; border: none; }} QFrame#diagCard {{ background: transparent; border: none; }}
QSplitter::handle {{ background: transparent; }} QSplitter::handle {{ background: transparent; }}
QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }} QSplitter::handle:hover {{ background: {BORDER}; border-radius: 4px; }}
QSplitter::handle:pressed {{ background: {p.accent}; border-radius: 4px; }} QSplitter::handle:pressed {{ background: {ACCENT}; border-radius: 4px; }}
""" """
def apply_palette(p: core.Palette) -> str:
"""Rebind the module-level colour names to `p` and return its stylesheet."""
global PALETTE, ACCENT, ACCENT_DIM, BG, PANEL, PANEL_2, TEXT, MUTED, BORDER
global GOOD, BAD, WARN, REMOTE, ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT
global STATUS_COLORS, HEALTH_COLORS
PALETTE = p
ACCENT, ACCENT_DIM = p.accent, p.accent_dim
BG, PANEL, PANEL_2 = p.bg, p.panel, p.panel_2
TEXT, MUTED, BORDER = p.text, p.muted, p.border
GOOD, BAD, WARN, REMOTE = p.good, p.bad, p.warn, p.remote
ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT = (
p.on_accent,
p.disabled_bg,
p.mono_bg,
p.selection_text,
)
STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": REMOTE, "unknown": WARN}
HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
return build_stylesheet(p)
STYLESHEET = apply_palette(core.DARK_PALETTE)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Background reachability tester (keeps the UI responsive during the request) # Background reachability tester (keeps the UI responsive during the request)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
@@ -716,6 +699,39 @@ class ServerEditor(QFrame):
def current_name(self) -> str: def current_name(self) -> str:
return self.name.text().strip() return self.name.text().strip()
def focus_target(self, target: tuple[str, int | str] | None):
"""
Focus the field a catalog Add left unfilled -- `target` is whatever
core.first_unfilled_focus_target() returned: ("args", line_index),
("env", var_name), or None (nothing to fill, so do nothing).
Only meaningful on the stdio page, which is the only page a catalog
entry ever populates (link-only entries never reach dump_data()).
"""
if not target or self.type.currentIndex() != 0:
return
kind, value = target
if kind == "args":
self.args.setFocus()
cursor = self.args.textCursor()
cursor.movePosition(QTextCursor.MoveOperation.Start)
cursor.movePosition(
QTextCursor.MoveOperation.Down, QTextCursor.MoveMode.MoveAnchor, int(value)
)
cursor.movePosition(
QTextCursor.MoveOperation.EndOfLine, QTextCursor.MoveMode.KeepAnchor
)
self.args.setTextCursor(cursor)
elif kind == "env":
for r in range(self.env.table.rowCount()):
key_item = self.env.table.item(r, 0)
if key_item and key_item.text() == value:
self.env.table.setCurrentCell(r, 1)
val_item = self.env.table.item(r, 1)
if val_item:
self.env.table.editItem(val_item)
break
def _type_switched(self): def _type_switched(self):
self.stack.setCurrentIndex(self.type.currentIndex()) self.stack.setCurrentIndex(self.type.currentIndex())
self._emit() self._emit()
@@ -1244,6 +1260,262 @@ class PasteDialog(QDialog):
self.err.setText(str(e)) self.err.setText(str(e))
# --------------------------------------------------------------------------- #
# Browse catalog dialog (issue #10 phase 2): search/filter the signed,
# bundled server catalog and add a "basic" entry through the existing
# paste/import path, or send a "link-only" entry to its setup docs.
#
# Every widget here that shows catalog-derived text uses plain_label() or an
# inherently-plain widget (QPlainTextEdit) -- see plain_label()'s docstring.
# The dialog itself does no signature/schema work: MainWindow hands it an
# already-verified `entries` list (bcc_core.load_bundled_catalog_entries()),
# and an empty list here means "show the empty state", never "fall back to
# something less trusted".
# --------------------------------------------------------------------------- #
class BrowseCatalogDialog(QDialog):
def __init__(self, parent, entries: list[dict]):
super().__init__(parent)
self.setWindowTitle("Browse catalog")
self.resize(880, 560)
self.entries = entries or []
self.result_entry: dict | None = None
self._current_entry: dict | None = None
self._current_homepage: str | None = None
self._current_docs_url: str | None = None
self._current_group = "All"
outer = QVBoxLayout(self)
if not self.entries:
# Signature verification failed, or nothing was bundled -- never
# show a half-trusted list, and never explain WHY beyond this;
# a stale/tampered catalog isn't the user's problem to diagnose.
msg = plain_label("Catalog unavailable.")
msg.setObjectName("placeholder")
msg.setAlignment(Qt.AlignmentFlag.AlignCenter)
outer.addWidget(msg, 1)
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
btns.rejected.connect(self.reject)
outer.addWidget(btns)
return
search_row = QHBoxLayout()
self.search_box = QLineEdit()
self.search_box.setPlaceholderText("Search by name, description, or category…")
self.search_box.setClearButtonEnabled(True)
self.search_box.textChanged.connect(self._refresh_list)
search_row.addWidget(self.search_box, 1)
outer.addLayout(search_row)
chip_row = QHBoxLayout()
self._chip_group = QButtonGroup(self)
self._chip_group.setExclusive(True)
for label in core.CATALOG_CATEGORY_CHIPS:
btn = QPushButton(label)
btn.setCheckable(True)
btn.setChecked(label == "All")
btn.clicked.connect(lambda _checked=False, g=label: self._set_group(g))
self._chip_group.addButton(btn)
chip_row.addWidget(btn)
chip_row.addStretch()
outer.addLayout(chip_row)
splitter = QSplitter(Qt.Orientation.Horizontal)
left = QWidget()
lv = QVBoxLayout(left)
lv.setContentsMargins(0, 0, 0, 0)
self.list = QListWidget()
self.list.currentItemChanged.connect(self._on_selected)
lv.addWidget(self.list, 1)
splitter.addWidget(left)
right = QFrame()
right.setObjectName("card")
rv = QVBoxLayout(right)
self.detail_title = plain_label("")
self.detail_title.setObjectName("h1")
rv.addWidget(self.detail_title)
self.detail_meta = plain_label("")
self.detail_meta.setObjectName("muted")
rv.addWidget(self.detail_meta)
self.detail_freshness = plain_label("")
self.detail_freshness.setObjectName("muted")
rv.addWidget(self.detail_freshness)
self.detail_desc = plain_label("")
rv.addWidget(self.detail_desc)
self.detail_notes = plain_label("")
self.detail_notes.setObjectName("muted")
rv.addWidget(self.detail_notes)
self.detail_homepage_btn = QPushButton("Open homepage")
self.detail_homepage_btn.clicked.connect(self._open_homepage)
rv.addWidget(self.detail_homepage_btn)
rv.addWidget(plain_label("Exact command this will add:"))
self.detail_command_preview = QPlainTextEdit()
self.detail_command_preview.setObjectName("diag")
self.detail_command_preview.setReadOnly(True)
# Read-only QPlainTextEdit never interprets HTML, regardless of what
# a compromised/careless catalog entry's command/args contain -- this
# is the field that renders "the exact bytes that will be written".
rv.addWidget(self.detail_command_preview, 1)
self.detail_action_btn = QPushButton("")
self.detail_action_btn.setObjectName("primary")
self.detail_action_btn.clicked.connect(self._on_action)
rv.addWidget(self.detail_action_btn)
splitter.addWidget(right)
splitter.setSizes([360, 480])
outer.addWidget(splitter, 1)
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
btns.rejected.connect(self.reject)
outer.addWidget(btns)
self._refresh_list()
# --- list / filtering -------------------------------------------------- #
def _set_group(self, group: str):
self._current_group = group
self._refresh_list()
def _visible_entries(self) -> list[dict]:
filtered = core.filter_catalog_entries(self.entries, self.search_box.text())
return core.catalog_entries_in_group(filtered, self._current_group)
def _format_row(self, entry: dict) -> str:
display = str(entry.get("display") or entry.get("id") or "")
official = "" if entry.get("official") else ""
stars = entry.get("stars")
star_txt = f"{stars:,}" if isinstance(stars, int) else ""
group = core.catalog_category_group(entry.get("category", ""))
desc = str(entry.get("description") or "")
if len(desc) > 88:
desc = desc[:87] + ""
# QListWidgetItem text is always rendered literally by Qt (no HTML
# interpretation), so no escaping is needed here -- unlike QLabel.
return f"{official}{display}{star_txt}\n{desc} · {group}"
def _refresh_list(self):
self.list.blockSignals(True)
self.list.clear()
for entry in self._visible_entries():
item = QListWidgetItem(self._format_row(entry))
item.setData(Qt.ItemDataRole.UserRole, entry)
# Tooltips DO auto-detect rich text in Qt, so escape defensively
# even though descriptions are already shown, unescaped-but-safe,
# in the QListWidgetItem text above.
item.setToolTip(html.escape(str(entry.get("description", ""))))
self.list.addItem(item)
self.list.blockSignals(False)
if self.list.count():
self.list.setCurrentRow(0)
else:
self._on_selected(None, None)
# --- detail pane -------------------------------------------------------- #
def _on_selected(self, current, _previous=None):
if current is None:
self._current_entry = None
self._current_homepage = None
self._current_docs_url = None
self.detail_title.setText("")
self.detail_meta.setText("")
self.detail_freshness.setText("")
self.detail_desc.setText("No matching servers." if self.entries else "")
self.detail_notes.setText("")
self.detail_homepage_btn.setVisible(False)
self.detail_command_preview.setPlainText("")
self.detail_action_btn.setEnabled(False)
self.detail_action_btn.setText("Add")
return
entry = current.data(Qt.ItemDataRole.UserRole)
self._current_entry = entry
self.detail_title.setText(str(entry.get("display") or entry.get("id") or ""))
official = "✓ Official" if entry.get("official") else ""
stars = entry.get("stars")
star_txt = f"{stars:,}" if isinstance(stars, int) else ""
group = core.catalog_category_group(entry.get("category", ""))
meta_bits = [b for b in (official, star_txt, group) if b]
self.detail_meta.setText(" · ".join(meta_bits))
freshness = core.format_freshness_hint(entry.get("last_release"))
self.detail_freshness.setText(freshness)
self.detail_freshness.setVisible(bool(freshness))
self.detail_desc.setText(str(entry.get("description") or ""))
notes = entry.get("notes") or ""
self.detail_notes.setText(notes)
self.detail_notes.setVisible(bool(notes))
homepage = entry.get("homepage")
self._current_homepage = homepage if isinstance(homepage, str) else None
self.detail_homepage_btn.setVisible(bool(self._current_homepage))
self._current_docs_url = (
entry.get("docs_url") if isinstance(entry.get("docs_url"), str) else None
)
self.detail_command_preview.setPlainText(self._render_command_preview(entry))
if entry.get("setup") == "basic":
self.detail_action_btn.setText("Add")
self.detail_action_btn.setEnabled(True)
else:
self.detail_action_btn.setText("Open setup docs")
self.detail_action_btn.setEnabled(bool(self._current_docs_url))
def _render_command_preview(self, entry: dict) -> str:
"""
The exact command that will be written, rendered verbatim. Every
value here comes straight from the (signature-verified) catalog
entry with no interpretation beyond str() -- this must never be the
place a markup-laced description sneaks back in as "helpful"
formatting.
"""
if entry.get("setup") != "basic":
docs = entry.get("docs_url") or "(none provided)"
return (
"This is a hosted/managed integration -- there is no local "
"command to add.\n\nSetup docs:\n " + str(docs)
)
config = entry.get("config") or {}
lines = [f"command: {config.get('command', '')}"]
args = config.get("args") or []
if args:
lines.append("args:")
lines.extend(f" {a}" for a in args)
env = config.get("env") or {}
env_required = entry.get("env_required") or {}
env_keys = list(env.keys()) + [k for k in env_required if k not in env]
if env_keys:
lines.append("env (names only -- you provide the values):")
lines.extend(f" {k}" for k in env_keys)
return "\n".join(lines)
def _open_homepage(self):
url = self._current_homepage
if url and url.startswith("https://"):
QDesktopServices.openUrl(QUrl(url))
def _on_action(self):
entry = self._current_entry
if not entry:
return
if entry.get("setup") == "basic":
self.result_entry = entry
self.accept()
else:
url = self._current_docs_url
if url and url.startswith("https://"):
QDesktopServices.openUrl(QUrl(url))
# link-only never auto-adds and never closes the dialog -- the
# user can keep browsing after opening the docs in their browser.
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Log viewer dialog (issue #6): a read-only, auto-tailing view of a single # Log viewer dialog (issue #6): a read-only, auto-tailing view of a single
# server's MCP log file. Polls on a QTimer instead of watching the filesystem # server's MCP log file. Polls on a QTimer instead of watching the filesystem
@@ -1533,54 +1805,6 @@ class AboutDialog(QDialog):
QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL)) QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL))
class NoticeBanner(QFrame):
"""A persistent, dismissible notice with an optional action button.
The status bar is the wrong home for anything the user needs to act on --
21 call sites rewrite it, so a message posted there is gone by the next
click. That wiped the MSIX warning (#35) and then the update notice (#78).
This is the shared mechanism so it doesn't happen a third time.
"""
def __init__(self, parent=None):
super().__init__(parent)
self.setObjectName("noticeBanner")
row = QHBoxLayout(self)
row.setContentsMargins(10, 8, 8, 8)
row.setSpacing(8)
self._label = QLabel("")
self._label.setObjectName("noticeText")
self._label.setWordWrap(True)
row.addWidget(self._label, 1)
self._action_btn = QPushButton("")
self._action_btn.setCursor(Qt.CursorShape.PointingHandCursor)
self._action_btn.hide()
row.addWidget(self._action_btn)
self._close_btn = QPushButton("\u2715")
self._close_btn.setObjectName("noticeClose")
self._close_btn.setCursor(Qt.CursorShape.PointingHandCursor)
self._close_btn.setFixedWidth(26)
self._close_btn.setToolTip("Dismiss")
self._close_btn.clicked.connect(self.hide)
row.addWidget(self._close_btn)
self.hide()
def show_notice(self, text: str, action_label: str = "", on_action=None):
self._label.setText(text)
self._label.setToolTip(text)
# Reconnect cleanly: a banner reused for a second notice would
# otherwise fire the previous notice's action too.
with contextlib.suppress(RuntimeError, TypeError):
self._action_btn.clicked.disconnect()
if action_label and on_action is not None:
self._action_btn.setText(action_label)
self._action_btn.clicked.connect(lambda _=False: on_action())
self._action_btn.show()
else:
self._action_btn.hide()
self.show()
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS # Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS
# waiting for the old instance to exit, so it must run off the UI thread. # waiting for the old instance to exit, so it must run off the UI thread.
@@ -1638,11 +1862,6 @@ class MainWindow(QMainWindow):
self.warn_banner.hide() self.warn_banner.hide()
root.addWidget(self.warn_banner) root.addWidget(self.warn_banner)
# Update availability gets its own persistent banner rather than a
# status-line write, which the next UI action overwrites (#78).
self.update_banner = NoticeBanner(self)
root.addWidget(self.update_banner)
# User-draggable divider between the server list and the editor. # User-draggable divider between the server list and the editor.
split = QSplitter(Qt.Orientation.Horizontal) split = QSplitter(Qt.Orientation.Horizontal)
split.setChildrenCollapsible(False) split.setChildrenCollapsible(False)
@@ -1678,96 +1897,11 @@ class MainWindow(QMainWindow):
# --- menu bar ---------------------------------------------------------- # # --- menu bar ---------------------------------------------------------- #
def _build_menu_bar(self): def _build_menu_bar(self):
view_menu = self.menuBar().addMenu("&View")
theme_menu = view_menu.addMenu("Theme")
self._theme_group = QActionGroup(self)
self._theme_group.setExclusive(True)
current = stored_theme_setting()
for setting, label in (
(core.THEME_SYSTEM, "Match system"),
(core.THEME_LIGHT, "Light"),
(core.THEME_DARK, "Dark"),
):
act = QAction(label, self, checkable=True)
act.setChecked(setting == current)
act.triggered.connect(lambda _checked=False, s=setting: self._set_theme(s))
self._theme_group.addAction(act)
theme_menu.addAction(act)
help_menu = self.menuBar().addMenu("&Help") help_menu = self.menuBar().addMenu("&Help")
# "Check for updates" used to exist only as a button inside the About
# dialog, which is not somewhere anyone looks for it (#79).
update_action = QAction("Check for updates…", self)
# Explicit role: macOS relocates actions it recognises by text, and
# some Qt versions treat "update" as application-menu material. Pin it
# so the item stays where the menu says it is on every platform.
update_action.setMenuRole(QAction.MenuRole.ApplicationSpecificRole)
update_action.triggered.connect(self.check_for_updates)
help_menu.addAction(update_action)
help_menu.addSeparator()
about_action = QAction("About Better Claude Config…", self) about_action = QAction("About Better Claude Config…", self)
# Qt auto-assigns AboutRole to actions whose text starts with "About",
# which moves this into the application menu on macOS. That is the
# right home there -- state it explicitly rather than inheriting it by
# accident, since the behaviour is invisible from this call site.
about_action.setMenuRole(QAction.MenuRole.AboutRole)
about_action.triggered.connect(self._show_about) about_action.triggered.connect(self._show_about)
help_menu.addAction(about_action) help_menu.addAction(about_action)
def _show_update_notice(self, notice: dict):
"""Surface an available update where it survives the next click."""
url = notice["url"]
self.update_banner.show_notice(
notice["text"],
action_label="Open releases page",
on_action=lambda: QDesktopServices.openUrl(QUrl(url)),
)
def check_for_updates(self):
"""Menu-driven check. Unlike the startup check this is never throttled
and always reports back -- the user asked, so silence would read as a
broken button."""
self.status.setText("Checking for updates…")
self._menu_update_worker = UpdateCheckWorker()
self._menu_update_worker.done.connect(self._on_menu_update_checked)
self._menu_update_worker.start()
def _on_menu_update_checked(self, release: dict | None):
self._menu_update_worker = None
if release is None:
self.status.setText("Couldn't check for updates (offline?).")
return
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
notice = core.update_notice(core.__version__, release)
if notice:
self._show_update_notice(notice)
self.status.setText(f"Update available: {notice['version']}")
else:
self.update_banner.hide()
self.status.setText(f"You're up to date ({core.__version__}).")
def _set_theme(self, setting: str):
"""Persist the theme choice and repaint the running window."""
QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting)
app = QApplication.instance()
if app is None: # pragma: no cover - only in a headless test harness
return
app.setStyleSheet(theme_stylesheet_for(app, setting))
# The global stylesheet covers most of the UI, but the inline
# setStyleSheet calls (status dots, warning labels, update banner) only
# pick up the new palette when their widget next renders -- so re-render
# them now rather than leaving dark-on-light text behind.
self._repaint_themed_widgets()
def _repaint_themed_widgets(self):
"""Re-run the inline-styled bits after a palette change."""
self.status.setStyleSheet(f"color: {MUTED};")
idx = self._current_index()
self._refresh_tables(select_index=idx if idx >= 0 else -1)
self._update_status(saved=False)
def _show_about(self): def _show_about(self):
AboutDialog(self).exec() AboutDialog(self).exec()
@@ -1788,9 +1922,10 @@ class MainWindow(QMainWindow):
if release is None: if release is None:
return # offline/failed check: don't advance lastCheck, allow retry return # offline/failed check: don't advance lastCheck, allow retry
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time()) QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
notice = core.update_notice(core.__version__, release) if core.is_newer_version(core.__version__, release["version"]):
if notice: self.status.setText(
self._show_update_notice(notice) f"Update available: {release['version']} · Help ▸ About to view it."
)
# --- layout persistence ---------------------------------------------- # # --- layout persistence ---------------------------------------------- #
def _restore_layout(self): def _restore_layout(self):
@@ -1957,6 +2092,10 @@ class MainWindow(QMainWindow):
self.del_btn = QPushButton("Delete") self.del_btn = QPushButton("Delete")
self.del_btn.setObjectName("danger") self.del_btn.setObjectName("danger")
self.paste_btn = QPushButton("Paste JSON...") self.paste_btn = QPushButton("Paste JSON...")
self.browse_catalog_btn = QPushButton("Browse catalog…")
self.browse_catalog_btn.setToolTip(
"Add a popular MCP server from the curated, signed catalog"
)
self.copy_btn = QPushButton("Copy to ▸") self.copy_btn = QPushButton("Copy to ▸")
self.undo_btn = QPushButton("Undo") self.undo_btn = QPushButton("Undo")
self.undo_btn.setEnabled(False) self.undo_btn.setEnabled(False)
@@ -1969,6 +2108,7 @@ class MainWindow(QMainWindow):
self.dup_btn.clicked.connect(self.duplicate_server) self.dup_btn.clicked.connect(self.duplicate_server)
self.del_btn.clicked.connect(self.delete_server) self.del_btn.clicked.connect(self.delete_server)
self.paste_btn.clicked.connect(self.paste_json) self.paste_btn.clicked.connect(self.paste_json)
self.browse_catalog_btn.clicked.connect(self.browse_catalog)
self.copy_btn.clicked.connect(self.copy_to_menu) self.copy_btn.clicked.connect(self.copy_to_menu)
self.undo_btn.clicked.connect(self._undo) self.undo_btn.clicked.connect(self._undo)
self.test_all_btn.clicked.connect(self._test_all_servers) self.test_all_btn.clicked.connect(self._test_all_servers)
@@ -1977,6 +2117,7 @@ class MainWindow(QMainWindow):
self.dup_btn, self.dup_btn,
self.del_btn, self.del_btn,
self.paste_btn, self.paste_btn,
self.browse_catalog_btn,
self.copy_btn, self.copy_btn,
self.undo_btn, self.undo_btn,
self.test_all_btn, self.test_all_btn,
@@ -2116,21 +2257,9 @@ class MainWindow(QMainWindow):
return return
self.full_config = cfg self.full_config = cfg
repaired = True repaired = True
# extract_servers tolerates malformed entries rather than raising (#72),
# but keep it inside the guard: a load failure must leave the previously
# loaded profile intact instead of half-swapping the window's state.
try:
servers = core.extract_servers(self.full_config)
except Exception as exc: # pragma: no cover - defence in depth
QMessageBox.critical(
self,
"Could not read config",
f"{profile.path}\n\nThe server list couldn't be read: {exc}",
)
return
self._loaded_stat = core.config_fingerprint(profile.path) self._loaded_stat = core.config_fingerprint(profile.path)
self.current_profile = profile self.current_profile = profile
self.servers = servers self.servers = core.extract_servers(self.full_config)
self.dirty = False self.dirty = False
self.restart_btn.hide() self.restart_btn.hide()
self._undo_stack.clear() self._undo_stack.clear()
@@ -2453,7 +2582,7 @@ class MainWindow(QMainWindow):
entry = self.servers[idx] entry = self.servers[idx]
old_name = entry.name old_name = entry.name
entry.name = self.editor.current_name() entry.name = self.editor.current_name()
entry.set_data(self.editor.dump_data()) entry.data = self.editor.dump_data()
# The server stays in its section (enable state unchanged), so update # The server stays in its section (enable state unchanged), so update
# its existing row in place rather than re-rendering. # its existing row in place rather than re-rendering.
# An edit invalidates any cached "Test all" result -- the server that # An edit invalidates any cached "Test all" result -- the server that
@@ -2547,7 +2676,7 @@ class MainWindow(QMainWindow):
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
) )
if ans == QMessageBox.StandardButton.Yes: if ans == QMessageBox.StandardButton.Yes:
self.servers[existing[name]].set_data(data) self.servers[existing[name]].data = data
return False, True return False, True
name = core.resolve_name_collision(name, {s.name for s in self.servers}) name = core.resolve_name_collision(name, {s.name for s in self.servers})
self.servers.append(core.ServerEntry(name, data, True)) self.servers.append(core.ServerEntry(name, data, True))
@@ -2567,6 +2696,41 @@ class MainWindow(QMainWindow):
self._mark_dirty() self._mark_dirty()
self.status.setText(f"Imported {added} added, {replaced} replaced. Review and Save.") self.status.setText(f"Imported {added} added, {replaced} replaced. Review and Save.")
def browse_catalog(self):
"""
Open the Browse-catalog dialog (issue #10 phase 2). The catalog is
loaded and signature-verified fresh every time the dialog opens --
never cached across app runs at this phase (remote fetch/cache is
#61, not yet built) -- so a bundled-catalog swap only takes effect
on next dialog open, never mid-session in a stale way.
"""
entries = core.load_bundled_catalog_entries(
_asset_dir() / "data" / "catalog.json", _asset_dir() / "data" / "catalog.json.sig"
)
dlg = BrowseCatalogDialog(self, entries)
if dlg.exec() != QDialog.DialogCode.Accepted or not dlg.result_entry:
return
entry = dlg.result_entry
paste = core.catalog_entry_to_paste_json(entry)
name, data = next(iter(paste.items()))
existing_before = {s.name: i for i, s in enumerate(self.servers)}
self._push_undo()
_added, replaced = self._import_server(name, data)
idx = (
existing_before.get(name, len(self.servers) - 1) if replaced else len(self.servers) - 1
)
self._refresh_tables(select_index=idx)
self._mark_dirty()
target = core.first_unfilled_focus_target(data)
self.editor.focus_target(target)
verb = "Replaced" if replaced else "Added"
self.status.setText(
f"{verb}{name}” from the catalog. Fill in the highlighted field and Save."
)
def copy_to_menu(self): def copy_to_menu(self):
idx = self._current_index() idx = self._current_index()
if not (0 <= idx < len(self.servers)): if not (0 <= idx < len(self.servers)):
@@ -2645,6 +2809,29 @@ class MainWindow(QMainWindow):
QMessageBox.warning(self, "Can't save yet", "Fix the highlighted problem first.") QMessageBox.warning(self, "Can't save yet", "Fix the highlighted problem first.")
return return
# Placeholder guard (issue #10): a catalog Add can leave a
# <PLACEHOLDER>-style token in args/env until the user fills it in.
# This warns, it does not block -- the user may be deliberately
# saving a stub to finish later -- but it must never save silently,
# since a server launched with a literal "<PLACEHOLDER>" argument
# just fails in a confusing way at spawn time.
placeholder_names = [
s.name for s in self.servers if core.config_has_unfilled_placeholders(s.data)
]
if placeholder_names:
names = ", ".join(f"{n}" for n in placeholder_names)
ans = QMessageBox.warning(
self,
"Unfilled placeholder",
f"{names} still has a <PLACEHOLDER> value that hasn't been "
"replaced with a real value. Claude won't be able to use "
"it as-is.\n\nSave anyway?",
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
QMessageBox.StandardButton.No,
)
if ans != QMessageBox.StandardButton.Yes:
return
# Stale-file check: if the file changed on disk since we loaded it, prompt. # Stale-file check: if the file changed on disk since we loaded it, prompt.
# Compare mtime AND size (not mtime alone) so a concurrent external write # Compare mtime AND size (not mtime alone) so a concurrent external write
# that lands within the mtime resolution window, or that restores the # that lands within the mtime resolution window, or that restores the
@@ -2667,11 +2854,6 @@ class MainWindow(QMainWindow):
except Exception as e: except Exception as e:
QMessageBox.critical(self, "Reload failed", str(e)) QMessageBox.critical(self, "Reload failed", str(e))
return return
# The reload above is the on-disk truth for everything the user
# didn't touch -- but it also wipes BCC-authored keys the user
# changed in this session (named sets), which apply_servers
# doesn't write. Carry them over before saving (#73).
contested = core.carry_owned_keys(self.full_config, fresh)
core.apply_servers(fresh, self.servers) core.apply_servers(fresh, self.servers)
try: try:
backup = core.write_config(self.current_profile.path, fresh) backup = core.write_config(self.current_profile.path, fresh)
@@ -2684,13 +2866,8 @@ class MainWindow(QMainWindow):
self.dirty = False self.dirty = False
self.save_btn.setEnabled(False) self.save_btn.setEnabled(False)
bnote = f" · backup: {backup.name}" if backup else " · (new file)" bnote = f" · backup: {backup.name}" if backup else " · (new file)"
cnote = (
f" · kept your {', '.join(contested)} (the file on disk had a different copy)"
if contested
else ""
)
self.status.setText( self.status.setText(
f"Merged & saved {self.current_profile.path}{bnote}{cnote}" f"Merged & saved {self.current_profile.path}{bnote}"
f" · Restart {self.current_profile.label} to apply." f" · Restart {self.current_profile.label} to apply."
) )
self._offer_restart_button() self._offer_restart_button()
@@ -2848,33 +3025,6 @@ class MainWindow(QMainWindow):
e.accept() e.accept()
def system_is_dark(app: QApplication) -> bool:
"""Whether the desktop is currently using a dark appearance.
Read from the style's own window colour rather than per-platform APIs --
Qt has already resolved the OS appearance by the time it builds the
default palette, so this works the same on all three platforms.
"""
try:
return app.palette().color(QPalette.ColorRole.Window).lightness() < 128
except Exception: # pragma: no cover - defensive; never block startup on theming
return True
def stored_theme_setting() -> str:
"""The user's theme choice, defaulting to following the system."""
value = QSettings("BCC", "BetterClaudeConfig").value("ui/theme", core.THEME_SYSTEM)
return value if value in core.THEME_CHOICES else core.THEME_SYSTEM
def theme_stylesheet_for(app: QApplication, setting: str | None = None) -> str:
"""Resolve setting + OS appearance into a palette, apply it, return the QSS."""
if setting is None:
setting = stored_theme_setting()
theme = core.resolve_theme(setting, system_is_dark(app))
return apply_palette(core.palette_for(theme))
def main(): def main():
if sys.platform == "win32": if sys.platform == "win32":
# Without an explicit AppUserModelID, Windows taskbar groups the app # Without an explicit AppUserModelID, Windows taskbar groups the app
@@ -2893,7 +3043,7 @@ def main():
icon = _app_icon() icon = _app_icon()
if not icon.isNull(): if not icon.isNull():
app.setWindowIcon(icon) app.setWindowIcon(icon)
app.setStyleSheet(theme_stylesheet_for(app)) app.setStyleSheet(STYLESHEET)
win = MainWindow() win = MainWindow()
win.show() win.show()
sys.exit(app.exec()) sys.exit(app.exec())
+5 -1
View File
@@ -31,7 +31,11 @@ a = Analysis(
["bcc.py"], ["bcc.py"],
pathex=[], pathex=[],
binaries=[], binaries=[],
datas=[("icons", "icons"), ("data/catalog.json", "data")], datas=[
("icons", "icons"),
("data/catalog.json", "data"),
("data/catalog.json.sig", "data"),
],
hiddenimports=[], hiddenimports=[],
hookspath=[], hookspath=[],
hooksconfig={}, hooksconfig={},
+249 -585
View File
@@ -15,7 +15,6 @@ from __future__ import annotations
import base64 import base64
import contextlib import contextlib
import copy
import difflib import difflib
import functools import functools
import glob import glob
@@ -30,6 +29,7 @@ import tempfile
import threading import threading
import time import time
from dataclasses import dataclass from dataclasses import dataclass
from datetime import date
from pathlib import Path from pathlib import Path
from typing import NamedTuple from typing import NamedTuple
from urllib.parse import urlparse from urllib.parse import urlparse
@@ -50,12 +50,6 @@ DISABLED_KEY = "_disabledMcpServers"
# parks the rest under DISABLED_KEY. # parks the rest under DISABLED_KEY.
SETS_KEY = "_bccServerSets" SETS_KEY = "_bccServerSets"
# Top-level keys BCC itself authors. They live in the client's config file, but
# BCC is their owner, so on a stale-file merge the in-memory copy wins over the
# on-disk one (see `carry_owned_keys`). Any future BCC-authored key belongs
# here -- forgetting to add one is exactly how #73 happened.
BCC_OWNED_KEYS = (SETS_KEY,)
BACKUP_DIRNAME = ".bcc_backups" BACKUP_DIRNAME = ".bcc_backups"
MAX_BACKUPS = 15 MAX_BACKUPS = 15
@@ -170,39 +164,6 @@ def fetch_latest_release(timeout: float = 4.0) -> dict | None:
return {"version": tag, "url": payload.get("html_url") or RELEASES_URL} return {"version": tag, "url": payload.get("html_url") or RELEASES_URL}
def update_notice(
current: str, release: dict | None, url_fallback: str = RELEASES_URL
) -> dict | None:
"""Decide whether to tell the user about a release, and what to say.
Returns {"version", "text", "url"} when `release` is newer than `current`,
or None when it isn't, when the check failed, or when the payload is
malformed. Kept here rather than in the GUI so the wording and the
should-we-notify decision are testable -- bcc.py can't be imported by the
test suite, which has no PySide6.
The text deliberately names no menu path. The old status-line notice read
"Help > About to view it", which is wrong on macOS: Qt relocates the About
action into the application menu (#79). A notice that carries its own
action can't drift out of sync with the platform.
"""
if not isinstance(release, dict):
return None
version = release.get("version")
if not version or not isinstance(version, str):
return None
if not is_newer_version(current, version):
return None
# Tags carry a "v" prefix and __version__ doesn't; render both the same way
# so the notice doesn't read "Version v1.3.0 ... you're running 1.2.0".
shown = version.lstrip("vV")
return {
"version": version,
"text": f"Version {shown} is available. You're running {current.lstrip('vV')}.",
"url": release.get("url") or url_fallback,
}
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Data model # Data model
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
@@ -218,195 +179,16 @@ class Profile:
self.path = Path(self.path) self.path = Path(self.path)
class _NoRaw:
"""Sentinel for ServerEntry.raw.
`None` can't do this job: `{"mcpServers": {"foo": null}}` is legal JSON and
a real malformed-entry case, so None has to mean "the config said null",
not "there was nothing here".
"""
__slots__ = ()
def __repr__(self) -> str: # keeps ServerEntry reprs readable in test output
return "<no raw>"
NO_RAW = _NoRaw()
@dataclass @dataclass
class ServerEntry: class ServerEntry:
"""One server definition.
`data` is always a dict so every consumer can treat it as one. When the
config held something that wasn't a JSON object for this server (a string,
a number, a list -- all legal JSON, all wrong here), `data` is empty and
the original value is preserved verbatim in `raw` so Save round-trips it
instead of silently deleting the user's line. `lint_servers` surfaces it.
`raw` defaults to the NO_RAW sentinel rather than None, because a config
value of literal `null` is itself a malformed entry worth preserving.
Assigning `data` means the user replaced the definition through the editor,
which retires `raw` -- use `set_data` so that can't be forgotten.
"""
name: str name: str
data: dict data: dict
enabled: bool = True enabled: bool = True
raw: object = NO_RAW
@property @property
def kind(self) -> str: def kind(self) -> str:
return "remote" if "url" in self.data and "command" not in self.data else "stdio" return "remote" if "url" in self.data and "command" not in self.data else "stdio"
@property
def malformed(self) -> bool:
"""True when the config value for this server wasn't a JSON object."""
return self.raw is not NO_RAW
def set_data(self, data: dict) -> None:
"""Replace the definition from the editor, clearing any malformed original."""
self.data = data
self.raw = NO_RAW
def config_value(self):
"""What to write back to the config: the edited dict, or the untouched
malformed original when the user never edited it."""
return self.data if self.raw is NO_RAW else self.raw
# --------------------------------------------------------------------------- #
# Theming (issue #75)
# --------------------------------------------------------------------------- #
THEME_SYSTEM = "system"
THEME_LIGHT = "light"
THEME_DARK = "dark"
THEME_CHOICES = (THEME_SYSTEM, THEME_LIGHT, THEME_DARK)
@dataclass(frozen=True)
class Palette:
"""Every colour the UI draws with.
Deliberately exhaustive: the stylesheet used to inline a handful of
near-black literals (`#1a1205` for text on the accent, `#202229` for the
disabled table, `#16181d` for the diagnostics pane), which is fine while
there's one theme and invisible breakage the moment there are two. Each
gets a slot here so a light palette can't silently inherit a dark value.
"""
name: str
accent: str
accent_dim: str
bg: str
panel: str
panel_2: str
text: str
muted: str
border: str
good: str
bad: str
warn: str
remote: str
on_accent: str # text drawn on top of an accent fill
disabled_bg: str # the parked-servers table
mono_bg: str # diagnostics / log panes
selection_text: str
# The shipping theme through v1.3.0. These values are carried over verbatim --
# adding a light theme must not restyle the dark one.
DARK_PALETTE = Palette(
name="dark",
accent="#f97316",
accent_dim="#c2570b",
bg="#1b1d23",
panel="#23262e",
panel_2="#2b2f39",
text="#e7e9ee",
muted="#9aa0ad",
border="#3a3f4b",
good="#4ade80",
bad="#f87171",
warn="#fbbf24",
remote="#60a5fa",
on_accent="#1a1205",
disabled_bg="#202229",
mono_bg="#16181d",
selection_text="#ffffff",
)
# The semantic colours are NOT the dark ones lightened. #4ade80 / #fbbf24 sit
# around 1.7:1 against white -- illegible. These are darkened to clear 4.5:1,
# which `test_light_palette_meets_contrast` enforces so nobody "tidies" them
# back toward the dark hues later.
LIGHT_PALETTE = Palette(
name="light",
accent="#c2410c",
accent_dim="#9a3412",
bg="#f6f7f9",
panel="#ffffff",
panel_2="#eef0f4",
text="#1b1d23",
muted="#5c6270",
border="#d3d7de",
good="#15803d",
bad="#b91c1c",
warn="#a16207",
remote="#1d4ed8",
on_accent="#ffffff",
disabled_bg="#e9ebef",
mono_bg="#f0f2f5",
selection_text="#ffffff",
)
PALETTES = {DARK_PALETTE.name: DARK_PALETTE, LIGHT_PALETTE.name: LIGHT_PALETTE}
def resolve_theme(setting: str, system_is_dark: bool) -> str:
"""Map a stored theme setting + the OS appearance onto a concrete palette name.
Anything unrecognised (a hand-edited QSettings value, a setting written by
a future version) falls back to following the system rather than to a
fixed theme -- the user's desktop is the better guess.
"""
if setting == THEME_DARK:
return THEME_DARK
if setting == THEME_LIGHT:
return THEME_LIGHT
return THEME_DARK if system_is_dark else THEME_LIGHT
def palette_for(theme: str) -> Palette:
"""Concrete palette by name; unknown names fall back to dark (the historical look)."""
return PALETTES.get(theme, DARK_PALETTE)
def _hex_to_rgb(value: str) -> tuple[int, int, int]:
v = value.lstrip("#")
if len(v) == 3:
v = "".join(ch * 2 for ch in v)
return int(v[0:2], 16), int(v[2:4], 16), int(v[4:6], 16)
def relative_luminance(color: str) -> float:
"""WCAG relative luminance for a #rrggbb colour."""
def chan(c: int) -> float:
srgb = c / 255.0
return srgb / 12.92 if srgb <= 0.04045 else ((srgb + 0.055) / 1.055) ** 2.4
r, g, b = (chan(c) for c in _hex_to_rgb(color))
return 0.2126 * r + 0.7152 * g + 0.0722 * b
def contrast_ratio(fg: str, bg: str) -> float:
"""WCAG contrast ratio between two #rrggbb colours (1.0 to 21.0)."""
a, b = relative_luminance(fg), relative_luminance(bg)
lighter, darker = max(a, b), min(a, b)
return (lighter + 0.05) / (darker + 0.05)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Discovery # Discovery
@@ -671,30 +453,13 @@ def repair_config_file(path: str | os.PathLike) -> tuple[dict, list[str], str]:
return obj, notes, pretty return obj, notes, pretty
def _server_entry(name: str, data, enabled: bool) -> ServerEntry:
"""Build a ServerEntry, tolerating a value that isn't a JSON object.
A hand-edited config can legally hold `{"mcpServers": {"foo": "oops"}}` --
valid JSON, wrong shape. Calling dict() on that raises, which used to take
the whole load down before the linter ever got a look at it (#72). Keep the
original instead and let the linter report it.
"""
if isinstance(data, dict):
return ServerEntry(name=name, data=dict(data), enabled=enabled)
return ServerEntry(name=name, data={}, enabled=enabled, raw=data)
def extract_servers(cfg: dict) -> list[ServerEntry]: def extract_servers(cfg: dict) -> list[ServerEntry]:
"""Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers. """Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers."""
Never raises on a structurally-odd config -- malformed entries come back as
empty-data entries carrying their original value (see `_server_entry`).
"""
out: list[ServerEntry] = [] out: list[ServerEntry] = []
for name, data in (cfg.get("mcpServers") or {}).items(): for name, data in (cfg.get("mcpServers") or {}).items():
out.append(_server_entry(name, data, True)) out.append(ServerEntry(name=name, data=dict(data), enabled=True))
for name, data in (cfg.get(DISABLED_KEY) or {}).items(): for name, data in (cfg.get(DISABLED_KEY) or {}).items():
out.append(_server_entry(name, data, False)) out.append(ServerEntry(name=name, data=dict(data), enabled=False))
return out return out
@@ -786,8 +551,8 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
Write the server list back into `cfg` in place, preserving every other key Write the server list back into `cfg` in place, preserving every other key
and the position of `mcpServers`. Returns the same dict for convenience. and the position of `mcpServers`. Returns the same dict for convenience.
""" """
enabled = {s.name: s.config_value() for s in servers if s.enabled} enabled = {s.name: s.data for s in servers if s.enabled}
disabled = {s.name: s.config_value() for s in servers if not s.enabled} disabled = {s.name: s.data for s in servers if not s.enabled}
cfg["mcpServers"] = enabled # replaces value if key existed; appends otherwise cfg["mcpServers"] = enabled # replaces value if key existed; appends otherwise
if disabled: if disabled:
@@ -800,34 +565,6 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Write (atomic, with rotating backups) # Write (atomic, with rotating backups)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
def carry_owned_keys(local_cfg: dict, fresh_cfg: dict) -> list[str]:
"""Carry BCC-authored top-level keys from `local_cfg` onto `fresh_cfg`.
Used by the stale-file "Merge & save" path, which reloads the file from
disk and re-applies the user's server edits. That reload used to drop
anything BCC owns but `apply_servers` doesn't write -- named server sets
vanished without a word (#73). BCC owns these keys, so the in-memory copy
wins; mutates `fresh_cfg` in place.
Returns the keys where the on-disk copy differed and was overwritten, so
the caller can tell the user something was actually contested rather than
merely carried across.
Deliberately one-directional: a key absent locally is left alone on disk.
We can't tell "user deleted their last set" from "user never had sets and
another machine just added some", and silently deleting someone else's
data is the worse of the two failures.
"""
conflicts: list[str] = []
for key in BCC_OWNED_KEYS:
if key not in local_cfg:
continue
if key in fresh_cfg and fresh_cfg[key] != local_cfg[key]:
conflicts.append(key)
fresh_cfg[key] = copy.deepcopy(local_cfg[key])
return conflicts
def _make_backup(path: Path) -> Path: def _make_backup(path: Path) -> Path:
bdir = path.parent / BACKUP_DIRNAME bdir = path.parent / BACKUP_DIRNAME
bdir.mkdir(exist_ok=True) bdir.mkdir(exist_ok=True)
@@ -1644,21 +1381,9 @@ def lint_server(name: str, data: dict) -> list[str]:
def lint_servers(servers: list[ServerEntry]) -> list[str]: def lint_servers(servers: list[ServerEntry]) -> list[str]:
"""Concatenate lint_server warnings across every entry, in order. """Concatenate lint_server warnings across every entry, in order."""
Entries whose config value wasn't a JSON object at all are reported here
rather than in lint_server, which takes an already-dict `data` (#72).
"""
out: list[str] = [] out: list[str] = []
for s in servers: for s in servers:
if s.malformed:
nm = s.name.strip() or "(unnamed)"
out.append(
f"'{nm}': server definition is not an object "
f"(found {type(s.raw).__name__}) -- it is preserved as-is; "
f"edit it to replace it with a proper definition"
)
continue
out.extend(lint_server(s.name, s.data)) out.extend(lint_server(s.name, s.data))
return out return out
@@ -2440,30 +2165,6 @@ def restart_claude_desktop() -> RestartResult:
# is rejected by validate_catalog() regardless of how plausible it looks. # is rejected by validate_catalog() regardless of how plausible it looks.
CATALOG_ALLOWED_COMMANDS = frozenset({"npx", "uvx", "docker", "node", "python", "python3"}) CATALOG_ALLOWED_COMMANDS = frozenset({"npx", "uvx", "docker", "node", "python", "python3"})
# Env var keys a catalog entry's config.env must never set. Every one of
# these is a loader/interpreter override that lets a value walk straight
# past CATALOG_ALLOWED_COMMANDS and the -e/--eval/-c deny-rule below: e.g.
# NODE_OPTIONS="--require /tmp/x.js" turns an allowlisted `npx` entry into
# arbitrary code execution without ever touching config.args, which is the
# only field the allowlist/deny-rules/ASCII/secret checks used to cover.
# Matched case-insensitively -- env keys are case-sensitive on POSIX, but a
# `node_options` lookalike is exactly the kind of thing this must catch.
CATALOG_DENIED_ENV_KEYS = frozenset(
{
"NODE_OPTIONS",
"PYTHONSTARTUP",
"PYTHONPATH",
"PYTHONHOME",
"LD_PRELOAD",
"LD_LIBRARY_PATH",
"DYLD_INSERT_LIBRARIES",
"DYLD_LIBRARY_PATH",
"BROWSER",
"PATH",
"NODE_REPL_EXTERNAL_MODULE",
}
)
# Ed25519 public keys allowed to sign a catalog, raw 32-byte form. A LIST # Ed25519 public keys allowed to sign a catalog, raw 32-byte form. A LIST
# (not a single key) so keys can be rotated without bricking installs that # (not a single key) so keys can be rotated without bricking installs that
# still trust an older key: verify_catalog_signature() accepts a match # still trust an older key: verify_catalog_signature() accepts a match
@@ -2489,36 +2190,6 @@ _CATALOG_SECRET_ARG_RE = re.compile(r"(?i)--api[-_]?key=|--token=|--password=")
# <PLACEHOLDER>-style tokens the GUI must have the user fill in before Save. # <PLACEHOLDER>-style tokens the GUI must have the user fill in before Save.
_PLACEHOLDER_RE = re.compile(r"<[^<>\s]+>") _PLACEHOLDER_RE = re.compile(r"<[^<>\s]+>")
# A catalog entry's id becomes an mcpServers JSON key AND is interpolated
# into Qt.AutoText widgets (status bar, QMessageBox) -- an id like
# "<b>Verified</b>" renders as markup there. Not RCE, but UI spoofing, so
# ids are constrained to a plain lowercase slug.
_CATALOG_ID_RE = re.compile(r"^[a-z0-9][a-z0-9._-]{0,63}$")
# Docker flags that consume the next arg as a value (so that value must not
# be mistaken for the image reference when locating it in config.args).
_DOCKER_VALUE_FLAGS = frozenset(
{
"-e",
"--env",
"-v",
"--volume",
"-p",
"--publish",
"--name",
"-w",
"--workdir",
"-u",
"--user",
"--entrypoint",
"--network",
"--platform",
"--add-host",
"-l",
"--label",
}
)
# How many versions a single accepted catalog jump may leap in one go. Bounds # How many versions a single accepted catalog jump may leap in one go. Bounds
# a "freeze" attack: a compromised/leaked signing key claiming an absurd # a "freeze" attack: a compromised/leaked signing key claiming an absurd
# future version would otherwise permanently outrank every legitimate # future version would otherwise permanently outrank every legitimate
@@ -2593,121 +2264,6 @@ def _docker_arg_violations(tag: str, args: list[str]) -> list[str]:
return problems return problems
def _catalog_package_spec_version(spec: str) -> str | None:
"""
Extract the version pin from an npm-style package spec, or None if the
spec carries no pin.
Handles unscoped "name@version" and scoped "@scope/name@version" --
scoped names have a leading "@" that is NOT the version separator, so a
naive split on the first/only "@" misparses "@scope/pkg" (no version)
as pinned to "scope/pkg". Splitting from the right side instead is safe
for both forms because a package name may contain "@" only as the
scope's leading character.
"""
if spec.startswith("@"):
rest = spec[1:]
if "@" not in rest:
return None
_, _, version = rest.rpartition("@")
return version or None
if "@" not in spec:
return None
_, _, version = spec.rpartition("@")
return version or None
def _catalog_package_spec_pinned(spec: str) -> bool:
"""
True if `spec` carries an exact version pin. Covers npm's "name@version"
/ "@scope/name@version" and uv's documented PyPI pin forms
"name@version" and "name==version".
"""
if "==" in spec:
_, _, version = spec.partition("==")
return bool(version)
return bool(_catalog_package_spec_version(spec))
def _first_catalog_package_spec(args: list[str]) -> str | None:
"""
The first arg that could plausibly BE a package spec: skip flags
(leading "-") and <PLACEHOLDER> tokens (which can't be validated and
are filled in by the user later, never shipped by the catalog as the
package name itself). Everything after the first hit is ignored --
trailing flags, paths, and placeholders are not package specs.
"""
for a in args:
if a.startswith("-"):
continue
if _PLACEHOLDER_RE.fullmatch(a):
continue
return a
return None
def _catalog_pin_violations(tag: str, command: str, args: list[str]) -> list[str]:
"""
Version-pinning enforcement (finding #3): a catalog PR can otherwise
ship `npx -y @scope/pkg` or `docker run img:latest` and the *next*
resolve of that package/image is whatever the registry serves that day
-- outside review, outside the signature's meaning. This is the only
place that enforces pinning at runtime; catalog_review.py's
risk_unpinned_package() is a maintainer-facing hint, not a gate.
"""
if command in ("npx", "uvx"):
spec = _first_catalog_package_spec(args)
if spec is None:
return [f"{tag}: config.args must include a package spec to pin (e.g. name@1.2.3)."]
if not _catalog_package_spec_pinned(spec):
return [
f"{tag}: config.args package {spec!r} is not version-pinned; use "
"name@version, @scope/name@version, or name==version."
]
return []
if command == "docker":
image = _docker_image_ref(args)
if image is None:
return [f"{tag}: config.args docker command has no image reference to pin."]
_, sep, image_tag = image.rpartition(":")
if not sep or "/" in image_tag:
return [
f"{tag}: config.args docker image {image!r} has no explicit tag; "
"pin an exact version (not 'latest', not untagged)."
]
if image_tag == "latest":
return [
f"{tag}: config.args docker image {image!r} uses the 'latest' tag, "
"which is not allowed; pin an exact version."
]
return []
return []
def _docker_image_ref(args: list[str]) -> str | None:
"""
Locate the image reference in a `docker run ...` args list: skip the
"run" subcommand and any flags, including ones that consume the next
token as a value (-e, -v, --name, ...) so that value isn't mistaken for
the image. The first remaining positional token is the image.
"""
i = 0
if i < len(args) and args[i] == "run":
i += 1
while i < len(args):
a = args[i]
if a.startswith("-"):
if a in _DOCKER_VALUE_FLAGS and "=" not in a:
i += 2
else:
i += 1
continue
return a
return None
def _validate_catalog_config(tag: str, config) -> list[str]: def _validate_catalog_config(tag: str, config) -> list[str]:
"""Validate the `config` block of a basic-tier catalog entry.""" """Validate the `config` block of a basic-tier catalog entry."""
if not isinstance(config, dict): if not isinstance(config, dict):
@@ -2728,11 +2284,10 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
f"({', '.join(sorted(CATALOG_ALLOWED_COMMANDS))})." f"({', '.join(sorted(CATALOG_ALLOWED_COMMANDS))})."
) )
raw_args = config.get("args") args = config.get("args")
args_ok = isinstance(raw_args, list) and all(isinstance(a, str) for a in raw_args) if not isinstance(args, list) or not all(isinstance(a, str) for a in args):
args = raw_args if args_ok else []
if not args_ok:
problems.append(f"{tag}: config.args must be a list of strings.") problems.append(f"{tag}: config.args must be a list of strings.")
args = []
for a in args: for a in args:
if not a.isascii(): if not a.isascii():
@@ -2751,55 +2306,11 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
if command == "docker": if command == "docker":
problems.extend(_docker_arg_violations(tag, args)) problems.extend(_docker_arg_violations(tag, args))
# Version pinning (finding #3) -- only meaningful once command/args are
# actually well-formed; a malformed args list already got its own
# problem above and has nothing left to pin-check.
if args_ok and command in ("npx", "uvx", "docker"):
problems.extend(_catalog_pin_violations(tag, command, args))
env = config.get("env") env = config.get("env")
if env is not None: if env is not None and (
env_ok = isinstance(env, dict) and all( not isinstance(env, dict) or any(not isinstance(v, str) for v in env.values())
isinstance(k, str) and isinstance(v, str) for k, v in env.items() ):
) problems.append(f"{tag}: config.env must be an object of string values.")
if not env_ok:
problems.append(f"{tag}: config.env must be an object of string values.")
else:
# config.env (finding #2): unlike args, env was previously
# type-checked ONLY -- no allowlist, no deny-rule, no ASCII
# check, no secret check. That made it the single easiest way
# to smuggle a payload past every other guard in this
# function: an allowlisted `command: npx` plus
# NODE_OPTIONS=--require /tmp/x.js in env walks straight past
# the command allowlist AND the -e/--eval/-c deny-rule above,
# because neither of those ever looks at env.
for key, value in env.items():
if not key.isascii():
problems.append(
f"{tag}: config.env key {key!r} must be ASCII "
"(non-ASCII code points rejected)."
)
if key.upper() in CATALOG_DENIED_ENV_KEYS:
problems.append(
f"{tag}: config.env key {key!r} is on the catalog deny-list "
"(interpreter/loader override) and is not allowed."
)
if not value.isascii():
problems.append(
f"{tag}: config.env value for {key!r} must be ASCII "
"(non-ASCII code points rejected)."
)
if _is_secret_value(value):
problems.append(
f"{tag}: config.env[{key!r}] looks like a real secret value; "
"catalog entries must never ship secret values."
)
if value != "" and not _PLACEHOLDER_RE.fullmatch(value):
problems.append(
f"{tag}: config.env[{key!r}] must be an empty string or a "
"single <PLACEHOLDER> token -- the catalog declares which env "
"vars a server needs, it never supplies their values."
)
return problems return problems
@@ -2819,12 +2330,6 @@ def _validate_catalog_entry(idx: int, entry, seen_ids: set[str]) -> list[str]:
tag = f"servers[{idx}] ({entry_id!r})" tag = f"servers[{idx}] ({entry_id!r})"
if not entry_id.isascii(): if not entry_id.isascii():
problems.append(f"{tag}: 'id' must be ASCII (non-ASCII code points rejected).") problems.append(f"{tag}: 'id' must be ASCII (non-ASCII code points rejected).")
elif not _CATALOG_ID_RE.match(entry_id):
problems.append(
f"{tag}: 'id' must match ^[a-z0-9][a-z0-9._-]{{0,63}}$ "
"(it becomes an mcpServers JSON key and is interpolated into "
"Qt.AutoText widgets)."
)
if entry_id in seen_ids: if entry_id in seen_ids:
problems.append(f"{tag}: duplicate id.") problems.append(f"{tag}: duplicate id.")
seen_ids.add(entry_id) seen_ids.add(entry_id)
@@ -2938,32 +2443,15 @@ def verify_catalog_signature(raw: bytes, sig: bytes, pubkeys: list[bytes]) -> bo
return False return False
def _verify_catalog_candidate(candidate: tuple[bytes, bytes] | None) -> tuple[dict | None, int]:
"""Verify+load+validate one (raw, sig) candidate. Returns (None, -1) on any failure."""
if not candidate:
return None, -1
raw, sig = candidate
if not verify_catalog_signature(raw, sig, CATALOG_PUBKEYS):
return None, -1
try:
data = load_catalog(raw)
except (ValueError, TypeError):
return None, -1
if validate_catalog(data):
return None, -1
return data, catalog_version(data)
def resolve_catalog( def resolve_catalog(
bundled: tuple[bytes, bytes] | None, bundled: tuple[bytes, bytes] | None,
cached: tuple[bytes, bytes] | None, cached: tuple[bytes, bytes] | None,
remote: tuple[bytes, bytes] | None, remote: tuple[bytes, bytes] | None,
floor: int = 0,
) -> dict: ) -> dict:
""" """
Pick the highest-version catalog among bundled/cached/remote. Each of Pick the highest-version catalog among bundled/cached/remote. Each
bundled/cached/remote is either None (unavailable) or an (raw_bytes, argument is either None (unavailable) or an (raw_bytes, signature_bytes)
signature_bytes) pair. pair.
🔴 SECURITY: every candidate — including `bundled`, the copy frozen into 🔴 SECURITY: every candidate — including `bundled`, the copy frozen into
this binary — is verified against CATALOG_PUBKEYS and re-validated from this binary — is verified against CATALOG_PUBKEYS and re-validated from
@@ -2974,68 +2462,46 @@ def resolve_catalog(
by virtue of being local. Signing (and checking the signature at by virtue of being local. Signing (and checking the signature at
runtime, every time) closes that. runtime, every time) closes that.
`floor` is a pure, caller-supplied lower bound (e.g. a persisted Anti-rollback: a candidate's version is never accepted if it's lower
"last accepted version" the GUI can load from disk and pass in) — this than the best verified candidate already found in this same resolution
function does no storage of its own. pass — an attacker replaying an old, since-superseded signed catalog
can't downgrade you.
Anti-rollback / anti-freeze, and WHY they apply to every candidate Anti-freeze: a candidate whose version leaps more than
including the first one evaluated: the previous version of this _CATALOG_MAX_VERSION_JUMP past the current best is also rejected. A
function only ran these checks `if best_version >= 0`, i.e. once a compromised/leaked signing key claiming an absurd future version would
candidate had already been accepted in this pass. That let the FIRST otherwise permanently outrank every legitimate release from then on,
verified candidate through unconditionally — a signed catalog claiming since the resolver always prefers the highest verified version — this
version=999999999 sailed straight past both guards if it happened to be caps how far a single accepted jump can go.
evaluated first, and rollback protection reset on every call anyway
(nothing persisted across restarts). Now both guards are anchored to
something that doesn't depend on iteration order:
- The anti-freeze cap is measured against the BUNDLED catalog's version
(verified independently, once), not against "whatever was accepted
so far in this loop." Bundled ships inside the binary, so it's the
one candidate that isn't attacker-supplied at resolve time — the
natural trust anchor. If bundled itself doesn't verify, `floor` is
the anchor instead.
- The anti-rollback floor is max(floor, bundled's version), so a
caller that persists `floor` across restarts gets real rollback
protection; a caller that doesn't still gets "never below bundled."
On a version TIE, the bundled candidate wins over cached/remote (it
previously lost ties to whichever candidate happened to be evaluated
last, silently preferring remote over bundled at equal version).
Returns the winning catalog dict, or {} if nothing verified and Returns the winning catalog dict, or {} if nothing verified and
validated. validated.
""" """
bundled_data, bundled_version = _verify_catalog_candidate(bundled)
anchor = bundled_version if bundled_version >= 0 else floor
min_accepted = max(floor, bundled_version if bundled_version >= 0 else 0)
candidates = (
("bundled", bundled_data, bundled_version),
("cached", *_verify_catalog_candidate(cached)),
("remote", *_verify_catalog_candidate(remote)),
)
best: dict = {} best: dict = {}
best_version = -1 best_version = -1
best_is_bundled = False
for source, data, version in candidates: for candidate in (bundled, cached, remote):
if data is None: if not candidate:
continue
raw, sig = candidate
if not verify_catalog_signature(raw, sig, CATALOG_PUBKEYS):
continue
try:
data = load_catalog(raw)
except (ValueError, TypeError):
continue
if validate_catalog(data):
continue continue
if version < min_accepted:
continue # anti-rollback / below the persisted floor
if version > anchor + _CATALOG_MAX_VERSION_JUMP:
continue # anti-freeze, capped against the bundled trust anchor
is_bundled = source == "bundled" version = catalog_version(data)
better = version > best_version or ( if best_version >= 0:
version == best_version and is_bundled and not best_is_bundled if version < best_version:
) continue # anti-rollback
if better: if version > best_version + _CATALOG_MAX_VERSION_JUMP:
best = data continue # anti-freeze
best_version = version
best_is_bundled = is_bundled best = data
best_version = version
return best return best
@@ -3044,8 +2510,17 @@ def catalog_entry_to_paste_json(entry: dict) -> dict:
""" """
Convert a basic-tier catalog entry into the {name: {command, args, env}} Convert a basic-tier catalog entry into the {name: {command, args, env}}
shape parse_pasted_json()/_import_server() already understand, so the shape parse_pasted_json()/_import_server() already understand, so the
(future) catalog picker dialog can feed a selection straight into the Browse-catalog dialog can feed a selection straight into the existing
existing paste-import path instead of growing a parallel one. paste-import path instead of growing a parallel one.
`env` is seeded from two sources: config.env (rare -- e.g. grafana's
non-secret GRAFANA_URL) and, for every key in `env_required` not already
present, an empty-string placeholder. env_required is where the seed
data actually keeps its secret VAR NAMES (validate_catalog requires its
values to be "" -- never a real secret); config.env alone, without this,
would silently drop those names on Add for the 9 of 19 seed entries that
need a secret and only declare it via env_required -- the user would
see a server added with no field prompting them for the key it needs.
""" """
config = entry.get("config") or {} config = entry.get("config") or {}
name = entry.get("id") or entry.get("display") or "server" name = entry.get("id") or entry.get("display") or "server"
@@ -3053,9 +2528,11 @@ def catalog_entry_to_paste_json(entry: dict) -> dict:
"command": config.get("command", ""), "command": config.get("command", ""),
"args": list(config.get("args") or []), "args": list(config.get("args") or []),
} }
env = config.get("env") env = dict(config.get("env") or {})
for key in entry.get("env_required") or {}:
env.setdefault(key, "")
if env: if env:
data["env"] = dict(env) data["env"] = env
return {str(name): data} return {str(name): data}
@@ -3075,3 +2552,190 @@ def config_has_unfilled_placeholders(cfg: dict) -> bool:
if isinstance(env, dict): if isinstance(env, dict):
values.extend(v for v in env.values() if isinstance(v, str)) values.extend(v for v in env.values() if isinstance(v, str))
return any(_PLACEHOLDER_RE.search(v) for v in values) return any(_PLACEHOLDER_RE.search(v) for v in values)
# --------------------------------------------------------------------------- #
# Catalog: Browse-dialog helpers (issue #10 phase 2)
#
# Everything below is pure and GUI-free on purpose (per the design comment on
# #10): the dialog itself should be a thin shell that calls into this module,
# the same relationship bcc.py already has with the rest of bcc_core.py.
# --------------------------------------------------------------------------- #
# Collapses the 20-value category taxonomy from the catalog research pass
# down to the 7 chips shown in the Browse dialog. Any category not listed
# here (including one a future catalog entry introduces that we don't yet
# know about) falls back to "Other" rather than raising -- an unrecognized
# category must never make an entry disappear from the dialog.
CATALOG_CATEGORY_GROUPS: dict[str, str] = {
"files": "Files & Dev",
"dev": "Files & Dev",
"code-hosting": "Files & Dev",
"browser": "Files & Dev",
"database": "Data",
"data": "Data",
"search": "Search & AI",
"ai": "Search & AI",
"cloud": "Cloud & Infra",
"infra": "Cloud & Infra",
"observability": "Cloud & Infra",
"productivity": "Work",
"communication": "Work",
"crm": "Work",
"finance": "Work",
"design": "Work",
"media": "Home & Personal",
"smart-home": "Home & Personal",
"personal": "Home & Personal",
}
# Ordered for chip display: "All" first, the 6 named groups next in the order
# given in the #10 design comment, "Other" last as the catch-all.
CATALOG_CATEGORY_CHIPS: tuple[str, ...] = (
"All",
"Files & Dev",
"Data",
"Search & AI",
"Cloud & Infra",
"Work",
"Home & Personal",
"Other",
)
def catalog_category_group(category: str) -> str:
"""Collapse a raw catalog `category` value to one of the 7 UI chips.
Unknown/missing categories map to "Other" -- never raises, never drops
an entry from the list just because its category tag doesn't match one
of the ones known at the time this mapping was written.
"""
return CATALOG_CATEGORY_GROUPS.get(str(category or "").strip().lower(), "Other")
def catalog_entry_matches_query(entry: dict, query: str) -> bool:
"""
Case-insensitive substring match against a catalog entry's id, display
name, description, and category. An empty/whitespace-only query matches
everything, so the search box doubles as "no filter" when cleared --
the same convention server_matches_filter() uses for the main table.
"""
q = (query or "").strip().lower()
if not q:
return True
haystacks = (
str(entry.get("id", "")),
str(entry.get("display", "")),
str(entry.get("description", "")),
str(entry.get("category", "")),
)
return any(q in h.lower() for h in haystacks)
def filter_catalog_entries(entries: list[dict], query: str) -> list[dict]:
"""Return only the catalog entries that match `query` (see
catalog_entry_matches_query)."""
return [e for e in entries if catalog_entry_matches_query(e, query)]
def catalog_entries_in_group(entries: list[dict], group: str) -> list[dict]:
"""
Return only the entries whose category collapses into `group` (one of
CATALOG_CATEGORY_CHIPS). "All" (or a falsy/unrecognized group) returns
every entry unfiltered -- that's the default chip state.
"""
if not group or group == "All":
return list(entries)
return [e for e in entries if catalog_category_group(e.get("category", "")) == group]
def format_freshness_hint(last_release: str | None, today: date | None = None) -> str:
"""
Turn a catalog entry's `last_release` (an ISO "YYYY-MM-DD" date, or None
when the registry didn't expose one) into a short freshness hint for the
detail pane, e.g. "Last updated 14 months ago".
Returns "" (nothing to show) when `last_release` is missing or
unparseable, or when it's somehow in the future relative to `today` --
a bogus "-3 months ago" would undermine the one signal this hint exists
to give the user, so we'd rather show nothing than something wrong.
`today` is an injectable override so this is exactly reproducible in
tests without depending on the wall clock.
"""
if not last_release or not isinstance(last_release, str):
return ""
try:
released = date.fromisoformat(last_release)
except ValueError:
return ""
now = today or date.today()
if released > now:
return ""
months = (now.year - released.year) * 12 + (now.month - released.month)
if now.day < released.day:
months -= 1
months = max(months, 0)
if months == 0:
return "Last updated this month"
if months == 1:
return "Last updated 1 month ago"
if months < 24:
return f"Last updated {months} months ago"
years = months // 12
return f"Last updated {years} year{'s' if years != 1 else ''} ago"
def first_unfilled_focus_target(data: dict) -> tuple[str, int | str] | None:
"""
Given a server config dict shaped like catalog_entry_to_paste_json()'s
output (command/args/env), find the first thing a user must fill in
after a catalog Add: a <PLACEHOLDER>-style arg (checked first, since a
missing path/target usually blocks the server from starting at all) or
else the first env var the catalog left blank.
Returns ("args", index) or ("env", key), or None when there's nothing
left to fill (e.g. a server with no placeholders and no required env).
The GUI uses this to focus+select the right field right after Add,
instead of leaving the user to hunt for what still needs a value.
"""
args = data.get("args") or []
for i, a in enumerate(args):
if isinstance(a, str) and _PLACEHOLDER_RE.search(a):
return ("args", i)
env = data.get("env") or {}
if isinstance(env, dict):
for k, v in env.items():
if not isinstance(v, str) or not v.strip() or _PLACEHOLDER_RE.search(v):
return ("env", k)
return None
def load_bundled_catalog_entries(catalog_path: Path, sig_path: Path) -> list[dict]:
"""
Read+verify+validate the bundled catalog.json/.sig pair from disk and
return its `servers` list -- or an EMPTY list if anything at all is
wrong: files missing/unreadable, signature doesn't verify, JSON doesn't
parse, or validate_catalog() finds a problem.
🔴 SECURITY: this is the load-bearing guarantee for the Browse dialog.
There is deliberately no partial-success path here -- a signature
failure must never surface a half-trusted list, only an empty one, so
the GUI's only job is to render "Catalog unavailable" when this comes
back empty. All the real trust decisions (signature, schema, command
allowlist) already live in resolve_catalog()/validate_catalog(); this
is a thin disk-reading wrapper around them so the GUI never touches
catalog bytes directly.
"""
try:
raw = catalog_path.read_bytes()
sig = sig_path.read_bytes()
except OSError:
return []
data = resolve_catalog(bundled=(raw, sig), cached=None, remote=None)
servers = data.get("servers") if isinstance(data, dict) else None
return servers if isinstance(servers, list) else []
+61 -262
View File
@@ -12,12 +12,7 @@ Flow: Load -> Review -> Sign.
1. Load -- pick a source: an open Gitea PR touching data/catalog.json, 1. Load -- pick a source: an open Gitea PR touching data/catalog.json,
or the current tip of `main`. The Console fetches the exact or the current tip of `main`. The Console fetches the exact
git blob (via a local clone's git plumbing) and PINS its git blob (via a local clone's git plumbing) and PINS its
blob SHA *and the ref it came from* for the rest of this blob SHA for the rest of this review pass.
review pass. For a PR, the diff is against `main`; for
`main`, the diff is against the last catalog a maintainer
actually SIGNED (the bytes covered by the current
data/catalog.json.sig), never against itself -- an empty
diff must mean "nothing to sign", never "sign unlocked".
2. Review -- a semantic diff (catalog_review.diff_catalogs), one card per 2. Review -- a semantic diff (catalog_review.diff_catalogs), one card per
changed entry, with risk annotations changed entry, with risk annotations
(catalog_review.entry_risk_findings). A registry lookup for (catalog_review.entry_risk_findings). A registry lookup for
@@ -30,34 +25,20 @@ Flow: Load -> Review -> Sign.
changed entry must be individually acknowledged (its changed entry must be individually acknowledged (its
checkbox ticked) before Sign unlocks. There is no checkbox ticked) before Sign unlocks. There is no
"acknowledge all" -- see catalog_review.py. "acknowledge all" -- see catalog_review.py.
3. Sign -- re-resolves the current blob SHA from the SAME ref that was 3. Sign -- re-fetches the current blob SHA and refuses to sign unless
reviewed (never a hardcoded "main") and refuses to sign it still matches the pinned SHA from step 1 (TOCTOU fix:
unless it still matches the pinned SHA from step 1, the diff catalog_review.can_sign). On success, writes
is non-empty, and no changed entry has an outstanding
blocking risk finding (catalog_review.sign_precondition /
can_sign -- the TOCTOU fix). On success, writes
data/catalog.json + data/catalog.json.sig and commits BOTH data/catalog.json + data/catalog.json.sig and commits BOTH
in a single commit, then pushes -- so main is never red in a single commit, then pushes -- so main is never red
between a catalog merge and its signature. Signing uses the between a catalog merge and its signature.
CATALOG key ONLY -- see "Two signing keys" below.
The signature must be the artefact of an actual review, not a step that The signature must be the artefact of an actual review, not a step that
follows one. Signing IS the approval act. follows one. Signing IS the approval act.
Two signing keys (issue #68 finding 5): the CATALOG key (offline,
Console-only, `keygen` / `keygen --release` picks which) is the root of
trust for what BCC executes and must never touch CI. The RELEASE key is
CI-resident and signs ONLY the release SHA256SUMS manifest
(scripts/sign_checksums.py) -- `show-seed-b64 --release` is the only
supported way to get a seed out of this tool, and it refuses to run without
`--release` so the catalog seed can never be exported by habit. See the
README's "Signing keys" section.
""" """
from __future__ import annotations from __future__ import annotations
import argparse import argparse
import base64
import contextlib import contextlib
import getpass import getpass
import html import html
@@ -88,28 +69,8 @@ SIG_PATH = "data/catalog.json.sig"
# maintainer-only tool, so a dotfile under $HOME is an acceptable fallback # maintainer-only tool, so a dotfile under $HOME is an acceptable fallback
# when the OS keychain isn't available -- the blob stored there is always # when the OS keychain isn't available -- the blob stored there is always
# passphrase-encrypted (see catalog_review.encrypt_private_key), never raw. # passphrase-encrypted (see catalog_review.encrypt_private_key), never raw.
#
# Two SEPARATE keys are stored here, never conflated (issue #68 finding 5):
# "catalog" -- offline, Console-only. Verified by bcc_core.CATALOG_PUBKEYS.
# Roots of trust for every catalog entry BCC ships. Must
# NEVER leave this machine, never touch CI, never become an
# env var or a repo secret.
# "release" -- CI-resident. Verified by scripts.sign_checksums.
# RELEASE_PUBKEYS. Signs ONLY the release SHA256SUMS
# manifest. Its private seed is deliberately meant to be
# pasted into the RELEASE_SIGNING_KEY Gitea Actions secret
# (via `show-seed-b64 --release`) -- that is its normal,
# intended flow. A CI compromise burns this key, not the
# catalog key: that asymmetry is the whole point of having
# two keys instead of one.
KEY_STORAGE_DIR = Path.home() / ".bcc-catalog-console" KEY_STORAGE_DIR = Path.home() / ".bcc-catalog-console"
_KEY_KINDS = ("catalog", "release") KEY_STORAGE_FILE = KEY_STORAGE_DIR / "signing_key.enc"
def _key_storage_file(kind: str) -> Path:
assert kind in _KEY_KINDS, f"unknown key kind {kind!r}, expected one of {_KEY_KINDS}"
return KEY_STORAGE_DIR / f"signing_key_{kind}.enc"
HTTP_TIMEOUT = 6.0 HTTP_TIMEOUT = 6.0
@@ -136,64 +97,51 @@ def _keyring_module():
_KEYRING_SERVICE = "bcc-catalog-console" _KEYRING_SERVICE = "bcc-catalog-console"
_KEYRING_USERNAME = "signing-key"
def _keyring_username(kind: str) -> str: def store_encrypted_key(blob: bytes) -> str:
assert kind in _KEY_KINDS, f"unknown key kind {kind!r}, expected one of {_KEY_KINDS}"
return f"signing-key-{kind}"
def store_encrypted_key(blob: bytes, kind: str = "catalog") -> str:
"""Persist an already-encrypted key blob (see """Persist an already-encrypted key blob (see
catalog_review.encrypt_private_key) under the given `kind` catalog_review.encrypt_private_key). Prefers the OS keychain; falls back
("catalog" or "release" -- see the KEY_STORAGE_DIR comment above; the to a file under KEY_STORAGE_DIR (outside the repo) with restrictive
two are stored under different keychain entries / filenames so they can
never be loaded interchangeably). Prefers the OS keychain; falls back to
a file under KEY_STORAGE_DIR (outside the repo) with restrictive
permissions. Returns a human-readable description of where it went.""" permissions. Returns a human-readable description of where it went."""
keyring = _keyring_module() keyring = _keyring_module()
if keyring is not None: if keyring is not None:
try: try:
keyring.set_password(_KEYRING_SERVICE, _keyring_username(kind), blob.hex()) keyring.set_password(_KEYRING_SERVICE, _KEYRING_USERNAME, blob.hex())
return "OS keychain (via the `keyring` package)" return "OS keychain (via the `keyring` package)"
except Exception: except Exception:
pass # fall through to the file-based path pass # fall through to the file-based path
KEY_STORAGE_DIR.mkdir(parents=True, exist_ok=True) KEY_STORAGE_DIR.mkdir(parents=True, exist_ok=True)
key_file = _key_storage_file(kind) KEY_STORAGE_FILE.write_bytes(blob)
key_file.write_bytes(blob)
with contextlib.suppress(OSError): # best-effort on platforms without POSIX perm bits with contextlib.suppress(OSError): # best-effort on platforms without POSIX perm bits
key_file.chmod(0o600) KEY_STORAGE_FILE.chmod(0o600)
return f"encrypted file at {key_file}" return f"encrypted file at {KEY_STORAGE_FILE}"
def load_encrypted_key(kind: str = "catalog") -> bytes: def load_encrypted_key() -> bytes:
"""Load the encrypted key blob of the given `kind` from wherever """Load the encrypted key blob from wherever store_encrypted_key() put
store_encrypted_key() put it. Raises FileNotFoundError if no key of that it. Raises FileNotFoundError if no key has been generated yet."""
kind has been generated yet."""
keyring = _keyring_module() keyring = _keyring_module()
if keyring is not None: if keyring is not None:
try: try:
hex_blob = keyring.get_password(_KEYRING_SERVICE, _keyring_username(kind)) hex_blob = keyring.get_password(_KEYRING_SERVICE, _KEYRING_USERNAME)
if hex_blob: if hex_blob:
return bytes.fromhex(hex_blob) return bytes.fromhex(hex_blob)
except Exception: except Exception:
pass pass
key_file = _key_storage_file(kind) if not KEY_STORAGE_FILE.exists():
if not key_file.exists():
flag = " --release" if kind == "release" else ""
raise FileNotFoundError( raise FileNotFoundError(
f"No {kind} signing key found (checked the OS keychain and {key_file}). " f"No signing key found (checked the OS keychain and {KEY_STORAGE_FILE}). "
f"Run `python catalog_console.py keygen{flag}` first." "Run `python catalog_console.py keygen` first."
) )
return key_file.read_bytes() return KEY_STORAGE_FILE.read_bytes()
def unlock_signing_key(passphrase: str, kind: str = "catalog") -> bytes: def unlock_signing_key(passphrase: str) -> bytes:
"""Load + decrypt the signing key seed of the given `kind`. Raises """Load + decrypt the signing key seed. Raises ValueError on a wrong
ValueError on a wrong passphrase, FileNotFoundError if no key of that passphrase, FileNotFoundError if no key exists yet."""
kind exists yet. Defaults to "catalog" because that's the key blob = load_encrypted_key()
ReviewWindow._on_sign uses -- the GUI never touches the release key."""
blob = load_encrypted_key(kind)
return review.decrypt_private_key(blob, passphrase) return review.decrypt_private_key(blob, passphrase)
@@ -240,49 +188,6 @@ def read_catalog_at_commit(repo_dir: Path, commit: str) -> tuple[bytes, str]:
return blob_bytes(repo_dir, sha), sha return blob_bytes(repo_dir, sha), sha
def catalog_blob_history(repo_dir: Path, ref: str, limit: int = 200) -> list[str]:
"""Blob SHAs of CATALOG_PATH at each commit that touched it, walking
back from `ref`, most-recent-first. Used by last_signed_catalog_raw() to
find "the version of the catalog the current signature actually covers"
without assuming it's the tip commit."""
log_out = _git(repo_dir, "log", f"--max-count={limit}", "--format=%H", ref, "--", CATALOG_PATH)
commits = [line for line in log_out.splitlines() if line]
shas: list[str] = []
for commit in commits:
try:
shas.append(blob_sha_at(repo_dir, commit, CATALOG_PATH))
except GitError:
continue
return shas
def last_signed_catalog_raw(repo_dir: Path, commit: str) -> bytes | None:
"""The raw data/catalog.json bytes that verify against
data/catalog.json.sig as of `commit` -- i.e. "the last catalog a
maintainer actually signed", found by walking the catalog's git history
on that ref until a version verifies against the *current* signature.
This is what source="main (current tip)" diffs against (issue #68
finding 1): if main's tip catalog.json already matches its .sig, this
returns that same content and the diff is correctly empty (nothing new
to sign). If someone merged a catalog change to main without running it
through the Console -- the exact bypass that produced commit b08cf21 --
the signature still covers the OLDER content, so this returns that older
version and the diff surfaces exactly what was never actually reviewed.
Returns None if there's no signature yet, or none of the recent history
verifies against it (caller should treat this as "diff against nothing
ever signed", i.e. every current entry shows as newly added).
"""
try:
sig_sha = blob_sha_at(repo_dir, commit, SIG_PATH)
except GitError:
return None
sig_bytes = blob_bytes(repo_dir, sig_sha)
candidates = [blob_bytes(repo_dir, sha) for sha in catalog_blob_history(repo_dir, commit)]
return review.find_last_signed_catalog_raw(candidates, sig_bytes, core.CATALOG_PUBKEYS)
def commit_and_push_signed_catalog( def commit_and_push_signed_catalog(
repo_dir: Path, raw_bytes: bytes, signature: bytes, *, branch: str = "main" repo_dir: Path, raw_bytes: bytes, signature: bytes, *, branch: str = "main"
) -> str: ) -> str:
@@ -739,50 +644,28 @@ class ReviewWindow(QMainWindow):
row = self.source_list.currentRow() row = self.source_list.currentRow()
try: try:
if row <= 0: if row <= 0:
# source = main: diff against the last catalog a maintainer commit = fetch_ref(self.repo_dir, "main")
# actually SIGNED (the bytes covered by the current old_commit = None # main vs itself has no "old" -- nothing to diff without a base
# data/catalog.json.sig), never against itself. Diffing
# main-vs-main is what made an empty diff -> instantly
# "signable" in the first place (issue #68 finding 1) --
# this is the only path that reaches sign_catalog_bytes(),
# so if it can't be trusted nothing can.
loaded_ref = "main"
commit = fetch_ref(self.repo_dir, loaded_ref)
new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
new_catalog = core.load_catalog(new_raw)
old_raw = last_signed_catalog_raw(self.repo_dir, commit)
old_catalog = (
core.load_catalog(old_raw)
if old_raw is not None
else {
"schema": 1,
"version": 0,
"servers": [],
}
)
else: else:
pr = self._prs[row - 1] pr = self._prs[row - 1]
loaded_ref = pr.head_ref commit = fetch_ref(self.repo_dir, pr.head_ref)
commit = fetch_ref(self.repo_dir, loaded_ref)
old_commit = fetch_ref(self.repo_dir, "main") old_commit = fetch_ref(self.repo_dir, "main")
new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit) new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
new_catalog = core.load_catalog(new_raw) new_catalog = core.load_catalog(new_raw)
if old_commit:
old_raw, _old_sha = read_catalog_at_commit(self.repo_dir, old_commit) old_raw, _old_sha = read_catalog_at_commit(self.repo_dir, old_commit)
old_catalog = core.load_catalog(old_raw) old_catalog = core.load_catalog(old_raw)
else:
old_catalog = new_catalog
except (GitError, ValueError) as e: except (GitError, ValueError) as e:
QMessageBox.critical(self, "Load failed", html.escape(str(e))) QMessageBox.critical(self, "Load failed", html.escape(str(e)))
return return
self._new_raw = new_raw self._new_raw = new_raw
# `loaded_ref` is pinned into the session (not just this method's self.session = review.start_review(new_blob_sha, old_catalog, new_catalog)
# local variable) so _on_sign can re-resolve the TOCTOU blob SHA
# from the SAME ref that was reviewed, instead of a hardcoded
# "main" -- see review.sign_precondition and issue #68 finding 1.
self.session = review.start_review(
new_blob_sha, old_catalog, new_catalog, loaded_ref=loaded_ref
)
self._render_cards() self._render_cards()
def _render_cards(self): def _render_cards(self):
@@ -823,43 +706,20 @@ class ReviewWindow(QMainWindow):
def _on_sign(self): def _on_sign(self):
assert self.session is not None assert self.session is not None
def _resolve_blob_sha(ref: str) -> str:
# Re-fetch fresh, immediately before signing, from the SAME ref
# that was reviewed (session.loaded_ref) -- NEVER hardcode
# "main" here. Hardcoding "main" is the bug that made the PR
# review path unable to sign at all: _on_load() pins the PR
# head's blob SHA, so comparing against main's SHA differs by
# definition for any PR that actually changes the catalog, and
# this refused every PR review permanently (see issue #68
# finding 1 and review.sign_precondition's docstring).
return blob_sha_at(self.repo_dir, fetch_ref(self.repo_dir, ref), CATALOG_PATH)
try: try:
decision = review.sign_precondition(self.session, _resolve_blob_sha) current_sha = blob_sha_at(self.repo_dir, fetch_ref(self.repo_dir, "main"), CATALOG_PATH)
except GitError as e: except GitError as e:
QMessageBox.critical(self, "Sign failed", html.escape(str(e))) QMessageBox.critical(self, "Sign failed", html.escape(str(e)))
return return
decision = review.can_sign(self.session, current_sha)
if not decision.ok: if not decision.ok:
QMessageBox.warning(self, "Cannot sign", html.escape(decision.reason or "")) QMessageBox.warning(self, "Cannot sign", html.escape(decision.reason or ""))
# Only the TOCTOU blob-SHA-mismatch reason should trigger a if decision.reason and "changed" in decision.reason.lower():
# reload -- "mismatch" appears ONLY in that reason (deliberately self._on_load() # force a re-review against the new bytes
# checked instead of the broader "changed", which also matches
# "Not every CHANGED entry has been acknowledged yet" and would
# wrongly force a reload -- and with it a fresh review.py
# ReviewSession -- every time a reviewer pauses partway through
# ticking checkboxes).
if decision.reason and "mismatch" in decision.reason.lower():
# Force a genuine re-review against the new bytes -- this
# must call _on_load() (which re-fetches and re-diffs), NOT
# re-invoke the same stale resolver, or this becomes the
# infinite loop described in issue #68 finding 1: re-pinning
# the same wrong SHA forever instead of ever converging.
self._on_load()
return return
dialog = PassphraseDialog("Enter CATALOG signing key passphrase:", self) dialog = PassphraseDialog("Enter signing key passphrase:", self)
if dialog.exec() != QDialog.DialogCode.Accepted: if dialog.exec() != QDialog.DialogCode.Accepted:
return return
try: try:
@@ -884,17 +744,9 @@ class ReviewWindow(QMainWindow):
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
def cmd_keygen(args: argparse.Namespace) -> int: def cmd_keygen(_args: argparse.Namespace) -> int:
# Two SEPARATE keypairs, never conflated (issue #68 finding 5): the
# catalog key is the offline root of trust for what BCC executes and
# must never touch CI; the release key is CI-resident and signs ONLY
# the release SHA256SUMS manifest. Which one this run generates is
# explicit via --release, and the printed instructions differ sharply
# so it's obvious which key is safe to paste into a CI secret (release)
# and which one never is (catalog).
kind = "release" if getattr(args, "release", False) else "catalog"
seed, pubkey = review.generate_keypair() seed, pubkey = review.generate_keypair()
passphrase = getpass.getpass(f"Choose a passphrase to encrypt the new {kind} signing key: ") passphrase = getpass.getpass("Choose a passphrase to encrypt the new signing key: ")
confirm = getpass.getpass("Confirm passphrase: ") confirm = getpass.getpass("Confirm passphrase: ")
if passphrase != confirm: if passphrase != confirm:
print("error: passphrases did not match", file=sys.stderr) print("error: passphrases did not match", file=sys.stderr)
@@ -904,67 +756,31 @@ def cmd_keygen(args: argparse.Namespace) -> int:
return 1 return 1
blob = review.encrypt_private_key(seed, passphrase) blob = review.encrypt_private_key(seed, passphrase)
where = store_encrypted_key(blob, kind=kind) where = store_encrypted_key(blob)
pubkey_b64 = base64.b64encode(pubkey).decode("ascii") pubkey_b64 = __import__("base64").b64encode(pubkey).decode("ascii")
print(f"{kind.capitalize()} private key encrypted and stored in: {where}") print(f"Private key encrypted and stored in: {where}")
print() print()
if kind == "catalog": print("Public key (base64, paste into bcc_core.CATALOG_PUBKEYS):")
print( print(f" {pubkey_b64}")
"This is the CATALOG key. It is the root of trust for every catalog " print()
"entry BCC ships -- it must stay offline and Console-only. NEVER paste " print(
"it, its seed, or `show-seed-b64` output into CI, an env var, or a repo " "Also add it as the Gitea repo secret RELEASE_SIGNING_KEY (base64 of the "
"secret. (If the key currently in bcc_core.CATALOG_PUBKEYS has ever been " "32-byte private seed) used by release.yml -- get that value with:"
"pasted into a CI secret, treat it as burned for catalog use -- generate " )
"a fresh one with this command and rotate.)" print(" python catalog_console.py show-seed-b64 # careful: prints the raw key")
)
print()
print(
"Public key (base64) -- hand this to whoever maintains bcc_core.py so "
"they can add it to CATALOG_PUBKEYS (this tool does not edit that file):"
)
print(f" {pubkey_b64}")
else:
print(
"This is the RELEASE key. It signs ONLY the release SHA256SUMS "
"manifest in CI -- it is intentionally CI-resident and is NOT trusted "
"to sign the catalog (bcc_core.CATALOG_PUBKEYS does not and must not "
"contain it)."
)
print()
print("1. Public key (base64) -- paste into scripts/sign_checksums.py RELEASE_PUBKEYS:")
print(f" {pubkey_b64}")
print()
print(
"2. Private key -- add it as the Gitea repo secret RELEASE_SIGNING_KEY "
"(base64 of the 32-byte private seed). Get that value with:"
)
print(" python catalog_console.py show-seed-b64 --release # prints the raw key")
return 0 return 0
def cmd_show_seed_b64(args: argparse.Namespace) -> int: def cmd_show_seed_b64(_args: argparse.Namespace) -> int:
# Deliberately requires --release: this command's whole purpose is to passphrase = getpass.getpass("Signing key passphrase: ")
# produce a value that gets pasted into a CI secret, and the catalog key
# must NEVER be pasted into CI (issue #68 finding 5 -- that is exactly
# how the catalog key ended up burned in the first place). Refusing to
# run without --release makes "export the catalog seed for CI" a
# structurally different, more deliberate action than a typo away.
if not getattr(args, "release", False):
print(
"error: show-seed-b64 only ever exports the RELEASE key -- it is the "
"only key allowed to leave this machine, for the RELEASE_SIGNING_KEY CI "
"secret. Re-run as `show-seed-b64 --release`. The catalog key must never "
"be exported this way; see issue #68 finding 5.",
file=sys.stderr,
)
return 1
passphrase = getpass.getpass("Release signing key passphrase: ")
try: try:
seed = unlock_signing_key(passphrase, kind="release") seed = unlock_signing_key(passphrase)
except (FileNotFoundError, ValueError) as e: except (FileNotFoundError, ValueError) as e:
print(f"error: {e}", file=sys.stderr) print(f"error: {e}", file=sys.stderr)
return 1 return 1
import base64
print(base64.b64encode(seed).decode("ascii")) print(base64.b64encode(seed).decode("ascii"))
return 0 return 0
@@ -994,28 +810,11 @@ def build_parser() -> argparse.ArgumentParser:
p_gui.add_argument("--repo", default=".", help="path to a BCC git checkout (default: cwd)") p_gui.add_argument("--repo", default=".", help="path to a BCC git checkout (default: cwd)")
p_gui.set_defaults(func=cmd_gui) p_gui.set_defaults(func=cmd_gui)
p_keygen = sub.add_parser( p_keygen = sub.add_parser("keygen", help="generate a new Ed25519 signing keypair")
"keygen", help="generate a new Ed25519 signing keypair (catalog key by default)"
)
p_keygen.add_argument(
"--release",
action="store_true",
help=(
"generate the RELEASE key (CI-resident, signs SHA256SUMS only) instead "
"of the CATALOG key (offline, Console-only, signs data/catalog.json -- "
"see issue #68 finding 5)"
),
)
p_keygen.set_defaults(func=cmd_keygen) p_keygen.set_defaults(func=cmd_keygen)
p_seed = sub.add_parser( p_seed = sub.add_parser(
"show-seed-b64", "show-seed-b64", help="print the base64 private seed (for the RELEASE_SIGNING_KEY secret)"
help="print a base64 private seed for a CI secret -- RELEASE key only",
)
p_seed.add_argument(
"--release",
action="store_true",
help="required: only the release key may ever be exported this way",
) )
p_seed.set_defaults(func=cmd_show_seed_b64) p_seed.set_defaults(func=cmd_show_seed_b64)
+6 -97
View File
@@ -24,7 +24,6 @@ from urllib.parse import urlsplit
from bcc_core import _CATALOG_SIG_DOMAIN as CATALOG_SIG_DOMAIN from bcc_core import _CATALOG_SIG_DOMAIN as CATALOG_SIG_DOMAIN
from bcc_core import CATALOG_ALLOWED_COMMANDS from bcc_core import CATALOG_ALLOWED_COMMANDS
from bcc_core import verify_catalog_signature as _verify_catalog_signature
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Semantic diff # Semantic diff
@@ -433,21 +432,11 @@ def has_blocking_risk(change: EntryChange) -> bool:
class ReviewSession: class ReviewSession:
"""State for one review pass. `pinned_blob_sha` is the git blob SHA of """State for one review pass. `pinned_blob_sha` is the git blob SHA of
data/catalog.json as it existed the moment review began -- see data/catalog.json as it existed the moment review began -- see
can_sign()/sign_precondition(). can_sign()."""
`loaded_ref` is the exact ref this review was loaded from ("main", or a
PR's `refs/pull/<n>/head`) -- see issue #68 finding 1. It exists so the
Sign path can re-resolve the TOCTOU blob SHA from *the ref that was
actually reviewed*, instead of a hardcoded "main" that silently diverges
from the reviewed ref on every PR review (the bug that made the PR path
unable to sign at all, and forced everyone onto the vacuous
main-vs-itself path instead).
"""
pinned_blob_sha: str pinned_blob_sha: str
old_catalog: dict old_catalog: dict
new_catalog: dict new_catalog: dict
loaded_ref: str = "main"
changes: list[EntryChange] = field(default_factory=list) changes: list[EntryChange] = field(default_factory=list)
acknowledged: set[str] = field(default_factory=set) acknowledged: set[str] = field(default_factory=set)
@@ -456,39 +445,12 @@ class ReviewSession:
self.changes = diff_catalogs(self.old_catalog, self.new_catalog) self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
def start_review( def start_review(pinned_blob_sha: str, old_catalog: dict, new_catalog: dict) -> ReviewSession:
pinned_blob_sha: str,
old_catalog: dict,
new_catalog: dict,
loaded_ref: str = "main",
) -> ReviewSession:
return ReviewSession( return ReviewSession(
pinned_blob_sha=pinned_blob_sha, pinned_blob_sha=pinned_blob_sha, old_catalog=old_catalog, new_catalog=new_catalog
old_catalog=old_catalog,
new_catalog=new_catalog,
loaded_ref=loaded_ref,
) )
def find_last_signed_catalog_raw(
candidates: list[bytes], sig: bytes, pubkeys: list[bytes]
) -> bytes | None:
"""Given `candidates` (candidate raw catalog.json byte-strings -- e.g.
successive historical versions from git log, most-recent-first),
return the first one whose signature verifies against `sig`/`pubkeys`,
or None if none do.
This is how source="main" review diffs against "the last catalog a
maintainer actually signed" instead of against itself (issue #68
finding 1): `catalog_console.last_signed_catalog_raw` walks
data/catalog.json's git history on main and hands the candidates here.
"""
for raw in candidates:
if _verify_catalog_signature(raw, sig, pubkeys):
return raw
return None
def acknowledge_entry(session: ReviewSession, entry_id: str) -> None: def acknowledge_entry(session: ReviewSession, entry_id: str) -> None:
ids = {c.entry_id for c in session.changes} ids = {c.entry_id for c in session.changes}
if entry_id not in ids: if entry_id not in ids:
@@ -521,53 +483,22 @@ class SignDecision:
def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision: def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
"""Whether the Sign button may fire right now. """Whether the Sign button may fire right now.
Four independent gates, all required, checked in this order: Two independent gates, both required:
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing)
0. The diff must be non-empty. An empty diff historically meant "Sign
unlocks instantly" (`set() <= set()` is vacuously True), which is
exactly backwards: a vacuously-satisfied gate is worse than no gate
at all, because it *manufactures confidence* -- the signature looks
identical to one produced by a real review. "Nothing changed" must
mean "nothing to sign", never "sign unlocked". (Issue #68 finding 1;
this is what let commit b08cf21 sign all 19 entries with zero of them
ever reviewed.)
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing,
from the ref that was actually reviewed -- see sign_precondition())
must match the blob SHA pinned when review began. If the bytes on the must match the blob SHA pinned when review began. If the bytes on the
remote changed since -- a new commit pushed to the same PR, a remote changed since -- a new commit pushed to the same PR, a
force-push, another PR merged in between -- signing is refused and a force-push, another PR merged in between -- signing is refused and a
re-review is forced. This is what makes "signing is the approval act" re-review is forced. This is what makes "signing is the approval act"
true rather than aspirational: the signature is bound to the exact true rather than aspirational: the signature is bound to the exact
reviewed bytes, not to "whatever the file happens to be now". reviewed bytes, not to "whatever the file happens to be now".
2. No blocking risk finding may be outstanding on ANY changed entry, full 2. Every changed entry in the diff must be individually acknowledged.
stop -- checked here, not just in the GUI. The GUI additionally
disables the acknowledge checkbox for a blocking entry, but that is a
UI nicety, not the enforcement point: if this pure gate didn't also
check it, a blocking risk would only be stopped by the GUI happening
to have wired the checkbox correctly, and nothing would catch a
regression in that wiring. The GUI must not be the only thing
standing between a blocking risk and a signature.
3. Every changed entry in the diff must be individually acknowledged.
""" """
if not session.changes:
return SignDecision(
False,
"Nothing to sign: this review's diff is empty. If you expected "
"changes here, you may be diffing the wrong source/ref.",
)
if current_blob_sha != session.pinned_blob_sha: if current_blob_sha != session.pinned_blob_sha:
return SignDecision( return SignDecision(
False, False,
"The reviewed bytes changed since this review began (blob SHA " "The reviewed bytes changed since this review began (blob SHA "
"mismatch) -- re-review required before signing.", "mismatch) -- re-review required before signing.",
) )
blocking_ids = sorted({c.entry_id for c in session.changes if has_blocking_risk(c)})
if blocking_ids:
return SignDecision(
False,
"Blocking risk finding(s) outstanding on: "
f"{', '.join(blocking_ids)} -- fix the underlying change, do not sign around it.",
)
if not all_entries_acknowledged(session): if not all_entries_acknowledged(session):
pending = sorted({c.entry_id for c in session.changes} - session.acknowledged) pending = sorted({c.entry_id for c in session.changes} - session.acknowledged)
return SignDecision( return SignDecision(
@@ -576,28 +507,6 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
return SignDecision(True, None) return SignDecision(True, None)
def sign_precondition(
session: ReviewSession, resolve_blob_sha: Callable[[str], str]
) -> SignDecision:
"""The real Sign-button gate: resolves the current TOCTOU blob SHA from
*the ref this session was actually loaded from* (`session.loaded_ref`),
never a hardcoded "main", then delegates to can_sign().
`resolve_blob_sha` is injected so this stays testable without git/Qt --
catalog_console.ReviewWindow._on_sign passes a real resolver
(fetch_ref + blob_sha_at against self.repo_dir); tests pass a fake
dict-backed lookup. This is the fix for issue #68 finding 1's first bug:
`_on_sign` used to hardcode `fetch_ref(self.repo_dir, "main")` as the
comparison ref, so for any PR review (where `loaded_ref` is the PR's
head, not main) the SHAs differed by definition and Sign could never
fire -- and the retry path re-called the same hardcoded resolver, so it
re-pinned the same wrong value and looped forever instead of forcing a
genuine re-review.
"""
current_blob_sha = resolve_blob_sha(session.loaded_ref)
return can_sign(session, current_blob_sha)
def catalog_signing_message(raw_bytes: bytes) -> bytes: def catalog_signing_message(raw_bytes: bytes) -> bytes:
"""The exact bytes that get signed: bcc_core's domain-separation prefix """The exact bytes that get signed: bcc_core's domain-separation prefix
(imported, never retyped) + the raw catalog bytes. Using this function (imported, never retyped) + the raw catalog bytes. Using this function
-32
View File
@@ -42,25 +42,6 @@ from pathlib import Path
# message signed by the same key. # message signed by the same key.
DOMAIN_PREFIX = b"bcc-release-v1|" DOMAIN_PREFIX = b"bcc-release-v1|"
# Public half of the RELEASE signing key(s) -- a SEPARATE keypair from
# bcc_core.CATALOG_PUBKEYS (issue #68 finding 5). The catalog key is the
# offline, Console-only root of trust for what BCC executes; this key is
# CI-resident and signs ONLY the release SHA256SUMS manifest, never the
# catalog. Keeping them apart means a CI/repo-secret compromise burns the
# release key -- annoying, but it never lets an attacker sign a catalog a
# user's binary would trust. A LIST (not a single key), mirroring
# CATALOG_PUBKEYS, so the release key can be rotated without invalidating
# the signature on every past release: verification accepts a match against
# ANY key here.
#
# Empty until the maintainer generates the release keypair (separately from
# the catalog keypair) and pastes the public half in:
# python catalog_console.py keygen --release
# This is intentionally NOT pre-populated with a placeholder that looks
# like a real key -- release.yml's signing-smoke-test fails closed (loudly)
# on an empty list rather than silently verifying against nothing.
RELEASE_PUBKEYS: list[bytes] = []
CHUNK_SIZE = 1024 * 1024 CHUNK_SIZE = 1024 * 1024
@@ -154,19 +135,6 @@ def public_key_b64_from_seed(seed_b64: str) -> str:
return base64.b64encode(raw).decode("ascii") return base64.b64encode(raw).decode("ascii")
def verify_checksums_against_any(pubkeys: list[bytes], sums_text: str, signature: bytes) -> bool:
"""Verify `signature` against ANY key in `pubkeys` (each a raw 32-byte
Ed25519 public key). Mirrors bcc_core.verify_catalog_signature's
rotation-friendly "any currently-trusted key" semantics, applied to
RELEASE_PUBKEYS instead of the catalog's key list. Returns False (never
raises) for an empty `pubkeys` list -- fails closed rather than
vacuously verifying against nothing."""
return any(
verify_checksums(base64.b64encode(pk).decode("ascii"), sums_text, signature)
for pk in pubkeys
)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# CLI # CLI
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
+3 -143
View File
@@ -334,29 +334,13 @@ def test_acknowledge_gating_requires_every_entry():
assert r.all_entries_acknowledged(session) is True assert r.all_entries_acknowledged(session) is True
def test_no_acknowledge_all_shortcut_and_gate_is_real(): def test_no_acknowledge_all_function_exists():
"""Two things, both load-bearing (issue #68: the original version of """Deliberate: there must be no shortcut to acknowledge every entry at
this test asserted ONLY the first half, and passed the entire time the once. See the comment in catalog_review.py above SignDecision."""
gate below it was vacuously satisfiable -- 'no function named
acknowledge_all' is worthless if signing doesn't actually require
acknowledgement in practice).
1. No bulk-acknowledge shortcut exists (see the comment in
catalog_review.py above SignDecision -- deliberate friction).
2. The gate that friction protects is actually enforced: with entries
still unacknowledged, can_sign() must refuse, not just "some GUI
checkbox happens to be unticked".
"""
names = [n for n in dir(r) if "acknowledge" in n.lower()] names = [n for n in dir(r) if "acknowledge" in n.lower()]
assert "acknowledge_all" not in names assert "acknowledge_all" not in names
assert "acknowledge_all_entries" not in names assert "acknowledge_all_entries" not in names
session = r.start_review("sha1", _catalog(), _catalog(_entry(id="a"), _entry(id="b")))
r.acknowledge_entry(session, "a") # only one of two -- not a bulk call
decision = r.can_sign(session, "sha1")
assert decision.ok is False
assert "acknowledged" in decision.reason.lower()
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# can_sign: TOCTOU blob pinning + acknowledge gating combined # can_sign: TOCTOU blob pinning + acknowledge gating combined
@@ -393,130 +377,6 @@ def test_can_sign_blob_mismatch_takes_priority_message():
assert "blob" in decision.reason.lower() or "changed" in decision.reason.lower() assert "blob" in decision.reason.lower() or "changed" in decision.reason.lower()
def test_can_sign_false_on_empty_changeset():
"""The exact bug behind issue #68 finding 1: 'main' loaded against
itself diffs to [], and an empty changeset used to leave can_sign()
with nothing to refuse on (set() <= set() is vacuously True). Commit
b08cf21 signed 19 entries through precisely this path -- zero of them
were ever reviewed. An empty diff must mean 'nothing to sign', never
'sign unlocked'."""
same_catalog = _catalog(_entry())
session = r.start_review("sha1", same_catalog, same_catalog)
assert session.changes == [] # diff_catalogs(x, x) -> []
assert r.all_entries_acknowledged(session) is True # vacuously -- this is the trap
decision = r.can_sign(session, "sha1") # blob matches, "everything" acknowledged
assert decision.ok is False
assert "nothing to sign" in decision.reason.lower()
def test_can_sign_false_with_outstanding_blocking_risk_even_if_acknowledged():
"""can_sign() must itself refuse a blocking risk finding -- today a
blocking finding only disables the GUI checkbox, so the pure gate must
not simply trust that the caller never acknowledged a blocking entry.
Acknowledge it directly here (bypassing any GUI checkbox-disable logic
entirely) to prove the gate catches it independently of the GUI."""
session = r.start_review(
"sha1",
_catalog(),
_catalog(_entry(config={"command": "bash", "args": ["-c", "evil"]})),
)
r.acknowledge_entry(session, "filesystem")
assert r.all_entries_acknowledged(session) is True
decision = r.can_sign(session, "sha1")
assert decision.ok is False
assert "blocking" in decision.reason.lower()
# --------------------------------------------------------------------------- #
# sign_precondition: the ref-resolution seam that used to hardcode "main"
# --------------------------------------------------------------------------- #
def test_sign_precondition_resolves_against_loaded_ref_not_hardcoded_main():
"""The regression test for issue #68 finding 1's first bug:
ReviewWindow._on_sign used to hardcode fetch_ref(repo, "main") as the
TOCTOU comparison ref. For a PR review, _on_load pins the PR HEAD's
blob SHA, so comparing against main's SHA differs by definition and
Sign could never fire on the PR path.
The fake resolver below returns a DIFFERENT (deliberately wrong) SHA for
"main" than for the PR ref that was actually loaded. If
sign_precondition ever resolves against "main" instead of
session.loaded_ref, this test fails -- both via the recorded `calls`
list and via decision.ok flipping to False.
"""
pr_ref = "refs/pull/42/head"
session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
r.acknowledge_entry(session, "filesystem")
calls: list[str] = []
def fake_resolver(ref: str) -> str:
calls.append(ref)
return {"main": "main-blob-sha-WRONG", pr_ref: "pr-blob-sha"}[ref]
decision = r.sign_precondition(session, fake_resolver)
assert calls == [pr_ref] # never asked the resolver for "main"
assert decision.ok is True
assert decision.reason is None
def test_sign_precondition_refuses_when_loaded_ref_blob_moved():
"""Same seam, the negative case: if the loaded ref's blob SHA has moved
since review began (a new commit landed on the reviewed PR/branch), the
resolver reflects that and sign_precondition must refuse -- proving this
isn't just a hardcoded pass-through."""
pr_ref = "refs/pull/42/head"
session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
r.acknowledge_entry(session, "filesystem")
def fake_resolver(_ref: str) -> str:
return "pr-blob-sha-AFTER-A-NEW-PUSH"
decision = r.sign_precondition(session, fake_resolver)
assert decision.ok is False
assert "mismatch" in decision.reason.lower() or "changed" in decision.reason.lower()
def test_sign_precondition_defaults_to_main_when_loaded_ref_unset():
"""start_review()'s loaded_ref defaults to 'main' for source=main
reviews (and backward-compat with callers that don't pass it)."""
session = r.start_review("sha1", _catalog(), _catalog(_entry()))
assert session.loaded_ref == "main"
r.acknowledge_entry(session, "filesystem")
def fake_resolver(ref: str) -> str:
assert ref == "main"
return "sha1"
decision = r.sign_precondition(session, fake_resolver)
assert decision.ok is True
# --------------------------------------------------------------------------- #
# find_last_signed_catalog_raw: what source=main diffs against
# --------------------------------------------------------------------------- #
def test_find_last_signed_catalog_raw_returns_matching_candidate():
"""Simulates walking catalog.json's git history: the CURRENT signature
covers an OLDER version of the bytes (a later commit changed
catalog.json without re-signing -- the exact bypass that produced
commit b08cf21). The first candidate that verifies against that
signature is 'the last catalog a maintainer actually signed'."""
seed, pubkey = r.generate_keypair()
old_raw = b'{"schema":1,"version":1,"servers":[]}'
new_raw = b'{"schema":1,"version":2,"servers":[]}'
sig = r.sign_catalog_bytes(old_raw, seed) # signature covers the OLD bytes
found = r.find_last_signed_catalog_raw([new_raw, old_raw], sig, [pubkey])
assert found == old_raw
def test_find_last_signed_catalog_raw_none_when_nothing_verifies():
seed, _pubkey = r.generate_keypair()
_other_seed, other_pubkey = r.generate_keypair()
raw = b'{"schema":1,"version":1,"servers":[]}'
sig = r.sign_catalog_bytes(raw, seed)
# Check against a pubkey list that does NOT include the signer's key.
assert r.find_last_signed_catalog_raw([raw], sig, [other_pubkey]) is None
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# catalog_signing_message: domain separation must match bcc_core exactly # catalog_signing_message: domain separation must match bcc_core exactly
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
+300 -574
View File
@@ -1,7 +1,6 @@
"""Pytest port of the original test_core.py script (same 23 behaviours, now """Pytest port of the original test_core.py script (same 23 behaviours, now
proper test functions with tmp_path/monkeypatch fixtures).""" proper test functions with tmp_path/monkeypatch fixtures)."""
import dataclasses
import json import json
import os import os
import re import re
@@ -1691,12 +1690,8 @@ def _minimal_catalog(version: int = 1) -> dict:
"official": True, "official": True,
"setup": "basic", "setup": "basic",
"config": { "config": {
# Pinned on purpose (issue #68 finding 3): an earlier
# version of this fixture used an unpinned package and
# asserted it validated clean, which enshrined the bug
# instead of catching it.
"command": "npx", "command": "npx",
"args": ["-y", "widget-mcp@1.0.0"], "args": ["-y", "widget-mcp"],
}, },
"placeholders": {}, "placeholders": {},
"env_required": {}, "env_required": {},
@@ -1957,213 +1952,6 @@ def test_validate_catalog_rejects_duplicate_ids():
assert any("duplicate id" in p for p in problems) assert any("duplicate id" in p for p in problems)
# --- validate_catalog: config.env (issue #68 finding 2) -------------------- #
def test_validate_catalog_rejects_each_denied_env_key():
for key in sorted(c.CATALOG_DENIED_ENV_KEYS):
data = _catalog_with(
{"config": {"command": "npx", "args": ["-y", "widget-mcp@1.0.0"], "env": {key: ""}}}
)
problems = c.validate_catalog(data)
assert any("deny-list" in p for p in problems), (key, problems)
def test_validate_catalog_rejects_denied_env_key_case_insensitively():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"node_options": ""},
}
}
)
problems = c.validate_catalog(data)
assert any("deny-list" in p for p in problems)
def test_validate_catalog_rejects_nonempty_nonplaceholder_env_value():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FOO_URL": "https://example.com"},
}
}
)
problems = c.validate_catalog(data)
assert any("empty string or a single <PLACEHOLDER>" in p for p in problems)
def test_validate_catalog_accepts_empty_and_placeholder_env_values():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FOO": "", "BAR_URL": "<BAR_URL>"},
}
}
)
assert c.validate_catalog(data) == []
def test_validate_catalog_rejects_non_ascii_env_key():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FÖO": ""},
}
}
)
problems = c.validate_catalog(data)
assert any("config.env key" in p and "ASCII" in p for p in problems)
def test_validate_catalog_rejects_non_ascii_env_value():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FOO": "<Bäd>"},
}
}
)
problems = c.validate_catalog(data)
assert any("config.env value" in p and "ASCII" in p for p in problems)
def test_validate_catalog_rejects_secret_looking_env_value():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"SOME_TOKEN": "ghp_abcdef1234567890"},
}
}
)
problems = c.validate_catalog(data)
assert any("real secret value" in p for p in problems)
def test_validate_catalog_rejects_node_options_env_walking_past_allowlist():
# The exact reproduction from issue #68 finding 2: an allowlisted
# `npx` command carrying NODE_OPTIONS in env, which previously passed
# validation and would have flowed straight into the executed
# subprocess via catalog_entry_to_paste_json().
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"NODE_OPTIONS": "--require /tmp/payload.js"},
}
}
)
problems = c.validate_catalog(data)
assert problems != []
# --- validate_catalog: version pinning (issue #68 finding 3) --------------- #
def test_validate_catalog_rejects_unpinned_npx_package():
data = _catalog_with({"config": {"command": "npx", "args": ["-y", "widget-mcp"]}})
problems = c.validate_catalog(data)
assert any("not version-pinned" in p for p in problems)
def test_validate_catalog_rejects_unpinned_scoped_npx_package():
data = _catalog_with({"config": {"command": "npx", "args": ["-y", "@scope/pkg"]}})
problems = c.validate_catalog(data)
assert any("not version-pinned" in p for p in problems)
def test_validate_catalog_accepts_pinned_scoped_npx_package():
data = _catalog_with({"config": {"command": "npx", "args": ["-y", "@scope/pkg@1.2.3"]}})
assert c.validate_catalog(data) == []
def test_validate_catalog_rejects_unpinned_uvx_package():
data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool"]}})
problems = c.validate_catalog(data)
assert any("not version-pinned" in p for p in problems)
def test_validate_catalog_accepts_uvx_at_version_pin():
data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool@1.0.0"]}})
assert c.validate_catalog(data) == []
def test_validate_catalog_accepts_uvx_double_equals_pin():
data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool==1.0.0"]}})
assert c.validate_catalog(data) == []
def test_validate_catalog_rejects_docker_latest_tag():
data = _catalog_with({"config": {"command": "docker", "args": ["run", "some/image:latest"]}})
problems = c.validate_catalog(data)
assert any("'latest'" in p for p in problems)
def test_validate_catalog_rejects_docker_untagged_image():
data = _catalog_with({"config": {"command": "docker", "args": ["run", "some/image"]}})
problems = c.validate_catalog(data)
assert any("no explicit tag" in p for p in problems)
def test_validate_catalog_accepts_pinned_docker_image_with_flags():
data = _catalog_with(
{
"config": {
"command": "docker",
"args": ["run", "-i", "--rm", "-e", "SOME_TOKEN", "some/image:1.2.3"],
}
}
)
assert c.validate_catalog(data) == []
def test_validate_catalog_does_not_pin_check_placeholders_flags_or_subcommand():
# A pinned uvx spec followed by flags and a <PLACEHOLDER> positional
# must not itself get mistaken for an unpinned package.
data = _catalog_with(
{
"config": {
"command": "uvx",
"args": ["mcp-server-git@2026.7.10", "--repository", "<REPO_PATH>"],
}
}
)
assert c.validate_catalog(data) == []
# --- validate_catalog: id constraint (issue #68 finding 7) ----------------- #
def test_validate_catalog_rejects_id_with_markup():
data = _catalog_with({"id": "<b>Verified</b>"})
problems = c.validate_catalog(data)
assert any("must match" in p for p in problems)
def test_validate_catalog_rejects_id_with_uppercase():
data = _catalog_with({"id": "Widget"})
problems = c.validate_catalog(data)
assert any("must match" in p for p in problems)
def test_validate_catalog_rejects_id_starting_with_dash():
data = _catalog_with({"id": "-widget"})
problems = c.validate_catalog(data)
assert any("must match" in p for p in problems)
def test_validate_catalog_accepts_valid_slug_id():
data = _catalog_with({"id": "widget-2.thing-ok"})
assert c.validate_catalog(data) == []
# --- resolve_catalog -------------------------------------------------------- # # --- resolve_catalog -------------------------------------------------------- #
def test_resolve_catalog_nothing_available_returns_empty_dict(): def test_resolve_catalog_nothing_available_returns_empty_dict():
assert c.resolve_catalog(None, None, None) == {} assert c.resolve_catalog(None, None, None) == {}
@@ -2215,93 +2003,13 @@ def test_resolve_catalog_rejects_absurd_version_jump(monkeypatch):
pub = priv.public_key().public_bytes_raw() pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub]) monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
# The freeze attempt goes FIRST (as `cached`), the legitimate catalog cached = _signed(_minimal_catalog(version=5), priv)
# SECOND (as `remote`) -- on purpose. Putting the good catalog first
# (as an earlier version of this test did) never exercises the
# vulnerable path: the old implementation only guarded a candidate
# against "the best accepted so far," so whichever candidate was
# evaluated FIRST got in unconditionally, uncapped. Ordering the freeze
# attempt first is what actually proves the cap holds regardless of
# evaluation order.
freeze_attempt = _signed(_minimal_catalog(version=999999), priv) freeze_attempt = _signed(_minimal_catalog(version=999999), priv)
good = _signed(_minimal_catalog(version=5), priv)
result = c.resolve_catalog(None, freeze_attempt, good) result = c.resolve_catalog(None, cached, freeze_attempt)
assert c.catalog_version(result) == 5 assert c.catalog_version(result) == 5
def test_resolve_catalog_caps_first_and_only_candidate(monkeypatch):
# issue #68 finding 6: with no bundled catalog to anchor against, a
# signed catalog claiming an absurd version must still be capped even
# when it is the ONLY candidate resolve_catalog() ever sees -- there is
# no "best so far" for it to be compared against, so the cap has to
# apply unconditionally, not "once something else has already landed."
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
freeze_attempt = _signed(_minimal_catalog(version=999999999), priv)
result = c.resolve_catalog(None, None, freeze_attempt)
assert result == {}
def test_resolve_catalog_anchors_cap_to_bundled_not_a_chained_best(monkeypatch):
# Anti-freeze must be measured against the BUNDLED version specifically,
# not against "whatever the best-so-far happens to be after each
# candidate is accepted" -- a chained anchor lets each accepted
# candidate ratchet the allowed ceiling upward, so a legitimate
# moderate bump (cached) plus a second, much larger jump (remote) can
# each individually look "within _CATALOG_MAX_VERSION_JUMP of the
# previous one" while remote is nowhere near bundled's version.
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
bundled = _signed(_minimal_catalog(version=2), priv)
cached = _signed(_minimal_catalog(version=1000), priv) # within 1000 of bundled
remote = _signed(_minimal_catalog(version=1900), priv) # within 1000 of cached,
# NOT of bundled
result = c.resolve_catalog(bundled, cached, remote)
assert c.catalog_version(result) == 1000
def test_resolve_catalog_prefers_bundled_on_version_tie(monkeypatch):
# issue #68 finding 6: on a tie the LAST candidate evaluated used to
# win, so remote silently beat bundled at equal version. Bundled --
# the copy frozen into the binary -- must win ties.
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
bundled_data = _minimal_catalog(version=5)
bundled = _signed(bundled_data, priv)
remote_data = _minimal_catalog(version=5)
remote_data["servers"][0]["display"] = "Remote Impostor"
remote = _signed(remote_data, priv)
result = c.resolve_catalog(bundled, None, remote)
assert c.catalog_version(result) == 5
assert result["servers"][0]["display"] == "Widget"
def test_resolve_catalog_floor_rejects_below_persisted_version(monkeypatch):
# `floor` is a pure parameter: the caller (eventually the GUI, from
# persisted storage) can pass a previously-accepted version, and
# nothing below it may be accepted even with no bundled catalog to
# anchor against.
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
stale = _signed(_minimal_catalog(version=3), priv)
result = c.resolve_catalog(None, None, stale, floor=10)
assert result == {}
def test_resolve_catalog_malformed_candidate_does_not_raise(monkeypatch): def test_resolve_catalog_malformed_candidate_does_not_raise(monkeypatch):
priv = Ed25519PrivateKey.generate() priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw() pub = priv.public_key().public_bytes_raw()
@@ -2331,31 +2039,51 @@ def test_resolve_catalog_invalid_but_signed_candidate_is_skipped(monkeypatch):
def test_catalog_entry_to_paste_json_basic_shape(): def test_catalog_entry_to_paste_json_basic_shape():
entry = _minimal_catalog()["servers"][0] entry = _minimal_catalog()["servers"][0]
result = c.catalog_entry_to_paste_json(entry) result = c.catalog_entry_to_paste_json(entry)
assert result == {"widget": {"command": "npx", "args": ["-y", "widget-mcp@1.0.0"]}} assert result == {"widget": {"command": "npx", "args": ["-y", "widget-mcp"]}}
def test_catalog_entry_to_paste_json_includes_env_when_present(): def test_catalog_entry_to_paste_json_includes_env_when_present():
# NOTE: catalog_entry_to_paste_json() is a pure shape-converter for an
# entry that has ALREADY passed validate_catalog() -- it is correct for
# it to carry env through verbatim. The bug (issue #68 finding 2) was
# never in this function; it was that validate_catalog() let entries
# with dangerous/non-placeholder env values reach this function in the
# first place. This test now proves that boundary explicitly: a
# validation-legal env value (a <PLACEHOLDER> token) survives the
# conversion, and a value validate_catalog() would have rejected is
# confirmed rejected before it ever gets here.
entry = _minimal_catalog()["servers"][0] entry = _minimal_catalog()["servers"][0]
entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"} entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
result = c.catalog_entry_to_paste_json(entry) result = c.catalog_entry_to_paste_json(entry)
assert result["widget"]["env"] == {"GRAFANA_URL": "<GRAFANA_URL>"} assert result["widget"]["env"] == {"GRAFANA_URL": "<GRAFANA_URL>"}
catalog = _minimal_catalog()
catalog["servers"][0]["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
assert c.validate_catalog(catalog) == []
malicious = _minimal_catalog() def test_catalog_entry_to_paste_json_seeds_env_required_keys():
malicious["servers"][0]["config"]["env"] = {"NODE_OPTIONS": "--require /tmp/payload.js"} # Regression: env_required is where the seed data actually keeps its
assert c.validate_catalog(malicious) != [] # secret VAR NAMES (postgres/github/notion/etc. all declare their secret
# here with config.env left empty) -- catalog_entry_to_paste_json must
# surface those names as blank env rows, not silently drop them.
entry = _minimal_catalog()["servers"][0]
entry["env_required"] = {"DATABASE_URI": ""}
result = c.catalog_entry_to_paste_json(entry)
assert result["widget"]["env"] == {"DATABASE_URI": ""}
def test_catalog_entry_to_paste_json_config_env_wins_over_env_required_default():
entry = _minimal_catalog()["servers"][0]
entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
entry["env_required"] = {"GRAFANA_URL": "", "GRAFANA_SERVICE_ACCOUNT_TOKEN": ""}
result = c.catalog_entry_to_paste_json(entry)
assert result["widget"]["env"] == {
"GRAFANA_URL": "<GRAFANA_URL>",
"GRAFANA_SERVICE_ACCOUNT_TOKEN": "",
}
def test_catalog_entry_to_paste_json_real_postgres_entry_seeds_database_uri():
"""End-to-end regression against the actual shipped postgres entry,
which needs DATABASE_URI via env_required and has no config.env at
all -- this is exactly the shape that was silently dropping the env
field before catalog_entry_to_paste_json accounted for env_required."""
root = Path(__file__).resolve().parent.parent
raw = (root / "data" / "catalog.json").read_bytes()
data = c.load_catalog(raw)
entry = next(s for s in data["servers"] if s["id"] == "postgres")
result = c.catalog_entry_to_paste_json(entry)
assert result["postgres"]["env"] == {"DATABASE_URI": ""}
# And the focus-target helper now has something to point the user at.
assert c.first_unfilled_focus_target(result["postgres"]) == ("env", "DATABASE_URI")
def test_config_has_unfilled_placeholders_true_for_token(): def test_config_has_unfilled_placeholders_true_for_token():
@@ -2374,309 +2102,307 @@ def test_config_has_unfilled_placeholders_checks_env_too():
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# #72 -- a server value that isn't a JSON object must not take the load down # Browse-catalog dialog helpers (issue #10 phase 2)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
@pytest.mark.parametrize("bad", ["not-a-dict", 123, ["a", "b"], None, True, 1.5])
def test_extract_servers_survives_non_dict_server_value(bad):
entries = c.extract_servers({"mcpServers": {"foo": bad}})
assert len(entries) == 1
assert entries[0].name == "foo"
assert entries[0].data == {}
assert entries[0].malformed is True
assert entries[0].raw == bad
def test_extract_servers_marks_only_the_bad_entry(): # --- catalog_category_group / CATALOG_CATEGORY_GROUPS --------------------- #
cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
by_name = {e.name: e for e in c.extract_servers(cfg)}
assert by_name["good"].malformed is False
assert by_name["good"].data == {"command": "npx"}
assert by_name["bad"].malformed is True
def test_extract_servers_handles_malformed_disabled_entry():
entries = c.extract_servers({c.DISABLED_KEY: {"parked": ["nope"]}})
assert entries[0].enabled is False
assert entries[0].malformed is True
def test_malformed_entry_round_trips_through_save_unchanged():
"""The cardinal rule: never silently delete what the user had on disk."""
cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
servers = c.extract_servers(cfg)
out = c.apply_servers(dict(cfg), servers)
assert out["mcpServers"]["bad"] == "oops"
assert out["mcpServers"]["good"] == {"command": "npx"}
def test_editing_a_malformed_entry_retires_the_raw_value():
entry = c.extract_servers({"mcpServers": {"bad": "oops"}})[0]
entry.set_data({"command": "npx"})
assert entry.malformed is False
assert entry.config_value() == {"command": "npx"}
assert c.apply_servers({}, [entry])["mcpServers"]["bad"] == {"command": "npx"}
def test_lint_reports_the_malformed_entry_by_name():
servers = c.extract_servers({"mcpServers": {"bad": "oops"}})
warnings = c.lint_servers(servers)
assert len(warnings) == 1
assert "'bad'" in warnings[0]
assert "not an object" in warnings[0]
assert "str" in warnings[0]
def test_lint_still_reports_normal_warnings_alongside_malformed():
cfg = {"mcpServers": {"bad": "oops", "sloppy": {"command": "npx", "args": "one two"}}}
warnings = c.lint_servers(c.extract_servers(cfg))
assert any("not an object" in w for w in warnings)
assert any("'args' should be a list" in w for w in warnings)
# --------------------------------------------------------------------------- #
# #73 -- the stale-file merge must not discard BCC-authored keys
# --------------------------------------------------------------------------- #
def test_carry_owned_keys_moves_sets_onto_the_reloaded_config():
local = {"mcpServers": {}, c.SETS_KEY: {"work": ["a", "b"]}}
fresh = {"mcpServers": {"external": {"command": "npx"}}}
contested = c.carry_owned_keys(local, fresh)
assert contested == []
assert fresh[c.SETS_KEY] == {"work": ["a", "b"]}
assert fresh["mcpServers"] == {"external": {"command": "npx"}}
def test_carry_owned_keys_reports_a_genuine_conflict():
local = {c.SETS_KEY: {"work": ["a"]}}
fresh = {c.SETS_KEY: {"work": ["a", "b"]}}
assert c.carry_owned_keys(local, fresh) == [c.SETS_KEY]
assert fresh[c.SETS_KEY] == {"work": ["a"]} # local wins: BCC owns the key
def test_carry_owned_keys_is_quiet_when_both_sides_agree():
local = {c.SETS_KEY: {"work": ["a"]}}
fresh = {c.SETS_KEY: {"work": ["a"]}}
assert c.carry_owned_keys(local, fresh) == []
def test_carry_owned_keys_leaves_disk_alone_when_absent_locally():
"""Can't distinguish 'deleted my last set' from 'never had sets'; keep theirs."""
fresh = {c.SETS_KEY: {"remote": ["a"]}}
assert c.carry_owned_keys({}, fresh) == []
assert fresh[c.SETS_KEY] == {"remote": ["a"]}
def test_carry_owned_keys_deep_copies_so_later_edits_do_not_leak():
local = {c.SETS_KEY: {"work": ["a"]}}
fresh = {}
c.carry_owned_keys(local, fresh)
local[c.SETS_KEY]["work"].append("b")
assert fresh[c.SETS_KEY] == {"work": ["a"]}
def test_merge_flow_preserves_sets_and_external_servers(tmp_path):
"""End-to-end shape of the Merge & save path that lost sets in #73."""
path = tmp_path / "claude.json"
path.write_text(json.dumps({"mcpServers": {"old": {"command": "old"}}}))
# BCC loads, user saves a named set and edits servers in memory.
local = c.load_config(path)
servers = c.extract_servers(local)
c.save_server_set(local, "work", servers)
# Something else rewrites the file underneath us.
path.write_text(json.dumps({"mcpServers": {"external": {"command": "new"}}, "other": 1}))
# Merge & save: reload disk, carry BCC keys, re-apply the user's servers.
fresh = c.load_config(path)
c.carry_owned_keys(local, fresh)
c.apply_servers(fresh, servers)
c.write_config(path, fresh)
saved = c.load_config(path)
assert saved[c.SETS_KEY] == {"work": ["old"]} # the set survived
assert saved["other"] == 1 # unrelated external key preserved
assert "old" in saved["mcpServers"] # user's servers re-applied
def test_null_server_value_is_malformed_not_mistaken_for_absent():
"""`{"mcpServers": {"foo": null}}` is legal JSON and a real malformed case,
so None must not double as the 'nothing here' sentinel."""
entry = c.extract_servers({"mcpServers": {"foo": None}})[0]
assert entry.malformed is True
assert entry.raw is None
assert c.apply_servers({}, [entry])["mcpServers"]["foo"] is None
def test_a_normal_entry_is_not_malformed():
entry = c.extract_servers({"mcpServers": {"foo": {"command": "npx"}}})[0]
assert entry.malformed is False
assert entry.raw is c.NO_RAW
# --------------------------------------------------------------------------- #
# #75 -- theming
# --------------------------------------------------------------------------- #
@pytest.mark.parametrize( @pytest.mark.parametrize(
"setting,system_dark,expected", "category,expected_group",
[ [
(c.THEME_DARK, False, "dark"), ("files", "Files & Dev"),
(c.THEME_DARK, True, "dark"), ("dev", "Files & Dev"),
(c.THEME_LIGHT, False, "light"), ("code-hosting", "Files & Dev"),
(c.THEME_LIGHT, True, "light"), ("browser", "Files & Dev"),
(c.THEME_SYSTEM, True, "dark"), ("database", "Data"),
(c.THEME_SYSTEM, False, "light"), ("data", "Data"),
("search", "Search & AI"),
("ai", "Search & AI"),
("cloud", "Cloud & Infra"),
("infra", "Cloud & Infra"),
("observability", "Cloud & Infra"),
("productivity", "Work"),
("communication", "Work"),
("crm", "Work"),
("finance", "Work"),
("design", "Work"),
("media", "Home & Personal"),
("smart-home", "Home & Personal"),
("personal", "Home & Personal"),
], ],
) )
def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected): def test_catalog_category_group_maps_every_taxonomy_value(category, expected_group):
assert c.resolve_theme(setting, system_dark) == expected assert c.catalog_category_group(category) == expected_group
@pytest.mark.parametrize("junk", ["", "solarized", None, "DARK", 3]) def test_catalog_category_group_unknown_falls_back_to_other():
def test_resolve_theme_falls_back_to_following_the_system(junk): assert c.catalog_category_group("some-future-category-nobody-has-seen-yet") == "Other"
"""A hand-edited or future QSettings value should follow the desktop, assert c.catalog_category_group("") == "Other"
not pin a fixed theme.""" assert c.catalog_category_group(None) == "Other"
assert c.resolve_theme(junk, True) == "dark"
assert c.resolve_theme(junk, False) == "light"
def test_palette_for_known_names(): def test_catalog_category_group_is_case_insensitive():
assert c.palette_for("dark") is c.DARK_PALETTE assert c.catalog_category_group("Files") == "Files & Dev"
assert c.palette_for("light") is c.LIGHT_PALETTE assert c.catalog_category_group("DATABASE") == "Data"
def test_palette_for_unknown_name_falls_back_to_dark(): def test_shipped_catalog_categories_all_have_a_known_group():
assert c.palette_for("chartreuse") is c.DARK_PALETTE """Regression: every category actually used in data/catalog.json must
collapse to one of the 7 chips, never silently drop an entry."""
root = Path(__file__).resolve().parent.parent
raw = (root / "data" / "catalog.json").read_bytes()
data = c.load_catalog(raw)
for entry in data["servers"]:
group = c.catalog_category_group(entry["category"])
assert group in c.CATALOG_CATEGORY_CHIPS
def test_dark_palette_is_unchanged_from_the_shipped_look(): # --- catalog_entry_matches_query / filter_catalog_entries ------------------ #
"""v1.3.0 shipped these exact colours; adding a light theme must not def _catalog_entries():
quietly restyle the dark one.""" return [
p = c.DARK_PALETTE {
assert (p.accent, p.bg, p.panel, p.panel_2) == ("#f97316", "#1b1d23", "#23262e", "#2b2f39") "id": "filesystem",
assert (p.text, p.muted, p.border) == ("#e7e9ee", "#9aa0ad", "#3a3f4b") "display": "Filesystem",
assert (p.good, p.bad, p.warn, p.remote) == ("#4ade80", "#f87171", "#fbbf24", "#60a5fa") "description": "Read/write access to local directories you choose.",
assert (p.on_accent, p.disabled_bg, p.mono_bg) == ("#1a1205", "#202229", "#16181d") "category": "files",
},
{
"id": "postgres",
"display": "Postgres MCP Pro",
"description": "Query and inspect a PostgreSQL database.",
"category": "database",
},
{
"id": "slack",
"display": "Slack",
"description": "Search messages and send messages from your assistant.",
"category": "communication",
},
]
def test_both_palettes_define_every_slot(): def test_catalog_entry_matches_query_empty_matches_everything():
"""A missing slot should fail here rather than render a broken window.""" entries = _catalog_entries()
for pal in (c.DARK_PALETTE, c.LIGHT_PALETTE): assert c.filter_catalog_entries(entries, "") == entries
for f in dataclasses.fields(c.Palette): assert c.filter_catalog_entries(entries, " ") == entries
value = getattr(pal, f.name)
assert value, f"{pal.name}.{f.name} is empty"
if f.name != "name":
assert re.fullmatch(r"#[0-9a-fA-F]{6}", value), f"{pal.name}.{f.name}={value!r}"
@pytest.mark.parametrize("pal_name", ["dark", "light"]) def test_catalog_entry_matches_query_matches_id():
@pytest.mark.parametrize("slot", ["text", "muted", "good", "bad", "warn", "remote", "accent"]) result = c.filter_catalog_entries(_catalog_entries(), "postgres")
def test_palette_meets_contrast_on_panel(pal_name, slot): assert [e["id"] for e in result] == ["postgres"]
"""Every colour drawn as text/glyph must clear WCAG AA (4.5:1) against the
surface it sits on. The light palette's semantic colours are NOT the dark
ones lightened -- #4ade80 sits near 1.7:1 on white -- so this guards
against someone 'harmonising' them back toward the dark hues."""
pal = c.palette_for(pal_name)
assert c.contrast_ratio(getattr(pal, slot), pal.panel) >= 4.5
@pytest.mark.parametrize("pal_name", ["dark", "light"]) def test_catalog_entry_matches_query_matches_display_case_insensitive():
def test_on_accent_is_legible_against_the_accent_fill(pal_name): result = c.filter_catalog_entries(_catalog_entries(), "SLACK")
"""Primary buttons and selected rows draw on_accent on top of accent.""" assert [e["id"] for e in result] == ["slack"]
pal = c.palette_for(pal_name)
assert c.contrast_ratio(pal.on_accent, pal.accent) >= 4.5
def test_contrast_ratio_endpoints(): def test_catalog_entry_matches_query_matches_description():
assert c.contrast_ratio("#000000", "#ffffff") == pytest.approx(21.0, abs=0.01) result = c.filter_catalog_entries(_catalog_entries(), "PostgreSQL database")
assert c.contrast_ratio("#123456", "#123456") == pytest.approx(1.0, abs=0.001) assert [e["id"] for e in result] == ["postgres"]
assert c.contrast_ratio("#ffffff", "#000000") == pytest.approx(21.0, abs=0.01)
def test_relative_luminance_extremes(): def test_catalog_entry_matches_query_matches_category():
assert c.relative_luminance("#000000") == pytest.approx(0.0) result = c.filter_catalog_entries(_catalog_entries(), "database")
assert c.relative_luminance("#ffffff") == pytest.approx(1.0) assert [e["id"] for e in result] == ["postgres"]
def test_stylesheet_builder_has_no_hardcoded_colours(): def test_catalog_entry_matches_query_no_match_returns_empty():
"""Every colour in the QSS must come from the palette. assert c.filter_catalog_entries(_catalog_entries(), "kubernetes") == []
Three near-black literals used to be inlined here (#1a1205, #202229,
#16181d). Harmless with one theme; with two, they silently render dark
chrome on a light window. Reads the source rather than importing bcc,
which needs PySide6.
"""
src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8")
start = src.index("def build_stylesheet")
body = src[start : src.index("def apply_palette")]
assert re.findall(r"#[0-9a-fA-F]{6}", body) == []
def test_every_palette_slot_is_consumed(): def test_catalog_entries_in_group_all_returns_everything():
"""A slot added to Palette but never wired up is dead weight. entries = _catalog_entries()
assert c.catalog_entries_in_group(entries, "All") == entries
Checks for `p.<slot>` anywhere in bcc.py, which covers both the QSS and assert c.catalog_entries_in_group(entries, "") == entries
apply_palette's global bindings -- not every slot belongs in the assert c.catalog_entries_in_group(entries, None) == entries
stylesheet (`good` and `remote` feed the inline status dots via
STATUS_COLORS/HEALTH_COLORS, never the QSS). This won't catch a slot bound
to a global that nothing then uses; it does catch the common mistake of
extending the dataclass and forgetting to plumb it through.
"""
src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8")
for f in dataclasses.fields(c.Palette):
if f.name == "name":
continue
assert f"p.{f.name}" in src, f"palette slot {f.name!r} is never consumed"
# --------------------------------------------------------------------------- # def test_catalog_entries_in_group_filters_by_collapsed_category():
# #78/#79 -- update notice: when to show it, and what it says result = c.catalog_entries_in_group(_catalog_entries(), "Data")
# --------------------------------------------------------------------------- # assert [e["id"] for e in result] == ["postgres"]
def test_update_notice_when_a_newer_release_exists():
n = c.update_notice("1.2.0", {"version": "v1.3.0", "url": "https://example.test/rel"}) result = c.catalog_entries_in_group(_catalog_entries(), "Work")
assert n is not None assert [e["id"] for e in result] == ["slack"]
assert n["version"] == "v1.3.0"
assert n["url"] == "https://example.test/rel"
assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
def test_update_notice_is_silent_when_current(): # --- format_freshness_hint -------------------------------------------------- #
assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None def test_format_freshness_hint_none_returns_empty_string():
assert c.update_notice("1.4.0", {"version": "v1.3.0"}) is None assert c.format_freshness_hint(None) == ""
assert c.format_freshness_hint("") == ""
@pytest.mark.parametrize("bad", [None, {}, {"version": ""}, {"version": None}, {"version": 3}, []]) def test_format_freshness_hint_unparseable_returns_empty_string():
def test_update_notice_is_silent_on_a_failed_or_malformed_check(bad): assert c.format_freshness_hint("not-a-date") == ""
"""fetch_latest_release returns None on any failure; a half-formed payload
must not produce a notice pointing at nothing."""
assert c.update_notice("1.0.0", bad) is None
def test_update_notice_falls_back_to_the_releases_page_without_a_url(): def test_format_freshness_hint_this_month():
n = c.update_notice("1.0.0", {"version": "v2.0.0"}) assert (
assert n["url"] == c.RELEASES_URL c.format_freshness_hint("2026-07-01", today=c.date(2026, 7, 12))
== "Last updated this month"
)
def test_update_notice_names_no_menu_path(): def test_format_freshness_hint_one_month_singular():
"""The old status-line text said 'Help > About to view it', which is wrong assert (
on macOS -- Qt moves the About action into the application menu (#79). The c.format_freshness_hint("2026-06-01", today=c.date(2026, 7, 12))
notice carries its own action, so it must not describe a menu path.""" == "Last updated 1 month ago"
n = c.update_notice("1.0.0", {"version": "v2.0.0"}) )
lowered = n["text"].lower()
for phrase in ("help", "about", "menu", "", ">"):
assert phrase not in lowered, f"notice text should not reference {phrase!r}"
def test_update_notice_handles_the_v_prefix_consistently(): def test_format_freshness_hint_months_ago():
assert c.update_notice("1.2.0", {"version": "1.3.0"}) is not None # Exactly 14 full months elapsed, no day-of-month remainder to round off.
assert c.update_notice("v1.2.0", {"version": "v1.3.0"}) is not None assert (
assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None c.format_freshness_hint("2025-01-15", today=c.date(2026, 3, 15))
== "Last updated 14 months ago"
)
def test_update_notice_renders_both_versions_the_same_way(): def test_format_freshness_hint_rounds_down_partial_month():
"""Tags carry a 'v' prefix, __version__ doesn't -- don't show both forms # 2025-05-16 -> 2026-07-12 is 13 full months, not 14: the 14th month
in one sentence.""" # would only complete on 2026-07-16.
n = c.update_notice("1.2.0", {"version": "v1.3.0"}) assert (
assert "v1.3.0" not in n["text"] c.format_freshness_hint("2025-05-16", today=c.date(2026, 7, 12))
assert "1.3.0" in n["text"] and "1.2.0" in n["text"] == "Last updated 13 months ago"
# the machine-readable field keeps the real tag )
assert n["version"] == "v1.3.0"
def test_format_freshness_hint_years_ago():
assert (
c.format_freshness_hint("2024-01-01", today=c.date(2026, 7, 12))
== "Last updated 2 years ago"
)
def test_format_freshness_hint_23_months_stays_in_months_not_years():
# The switch to "N years ago" happens at 24 full months, not 12 -- the
# whole point of this hint is the granular "14 months ago" phrasing the
# design comment on #10 asked for, so 13-23 months must stay in months.
assert (
c.format_freshness_hint("2024-08-12", today=c.date(2026, 7, 12))
== "Last updated 23 months ago"
)
def test_format_freshness_hint_future_date_returns_empty_string():
# A last_release "in the future" relative to `today` is nonsensical --
# show nothing rather than a misleading negative offset.
assert c.format_freshness_hint("2027-01-01", today=c.date(2026, 7, 12)) == ""
# --- first_unfilled_focus_target -------------------------------------------- #
def test_first_unfilled_focus_target_prefers_placeholder_arg():
data = {
"command": "npx",
"args": ["-y", "server", "<ALLOWED_DIR>"],
"env": {"API_KEY": ""},
}
assert c.first_unfilled_focus_target(data) == ("args", 2)
def test_first_unfilled_focus_target_falls_back_to_first_blank_env():
data = {"command": "uvx", "args": ["mcp-grafana"], "env": {"GRAFANA_URL": ""}}
assert c.first_unfilled_focus_target(data) == ("env", "GRAFANA_URL")
def test_first_unfilled_focus_target_none_when_fully_filled():
data = {"command": "npx", "args": ["-y", "server"], "env": {"API_KEY": "sk-real-value"}}
assert c.first_unfilled_focus_target(data) is None
def test_first_unfilled_focus_target_none_for_config_with_no_env_or_args():
assert c.first_unfilled_focus_target({"command": "npx", "args": []}) is None
# --- load_bundled_catalog_entries ------------------------------------------- #
def test_load_bundled_catalog_entries_valid_signature(tmp_path, monkeypatch):
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
raw, sig = _signed(_minimal_catalog(version=1), priv)
catalog_path = tmp_path / "catalog.json"
sig_path = tmp_path / "catalog.json.sig"
catalog_path.write_bytes(raw)
sig_path.write_bytes(sig)
entries = c.load_bundled_catalog_entries(catalog_path, sig_path)
assert len(entries) == 1
assert entries[0]["id"] == "widget"
def test_load_bundled_catalog_entries_tampered_payload_returns_empty_list(tmp_path, monkeypatch):
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
raw, sig = _signed(_minimal_catalog(version=1), priv)
tampered = bytearray(raw)
tampered[-2] ^= 0xFF # flip a byte inside the trailing bytes, still valid-ish JSON shape
catalog_path = tmp_path / "catalog.json"
sig_path = tmp_path / "catalog.json.sig"
catalog_path.write_bytes(bytes(tampered))
sig_path.write_bytes(sig)
assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
def test_load_bundled_catalog_entries_wrong_key_returns_empty_list(tmp_path, monkeypatch):
priv = Ed25519PrivateKey.generate()
other_priv = Ed25519PrivateKey.generate()
other_pub = other_priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [other_pub])
raw, sig = _signed(_minimal_catalog(version=1), priv) # signed by the WRONG key
catalog_path = tmp_path / "catalog.json"
sig_path = tmp_path / "catalog.json.sig"
catalog_path.write_bytes(raw)
sig_path.write_bytes(sig)
assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
def test_load_bundled_catalog_entries_missing_files_returns_empty_list(tmp_path):
assert c.load_bundled_catalog_entries(tmp_path / "nope.json", tmp_path / "nope.json.sig") == []
def test_load_bundled_catalog_entries_missing_sig_returns_empty_list(tmp_path, monkeypatch):
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
raw, _sig = _signed(_minimal_catalog(version=1), priv)
catalog_path = tmp_path / "catalog.json"
catalog_path.write_bytes(raw)
missing_sig_path = tmp_path / "catalog.json.sig" # never written
assert c.load_bundled_catalog_entries(catalog_path, missing_sig_path) == []
def test_load_bundled_catalog_entries_invalid_but_signed_returns_empty_list(tmp_path, monkeypatch):
"""A payload that verifies but fails validate_catalog() (disallowed
command) must still come back empty -- signing is necessary, not
sufficient."""
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
raw, sig = _signed(_catalog_with({"config": {"command": "bash", "args": []}}), priv)
catalog_path = tmp_path / "catalog.json"
sig_path = tmp_path / "catalog.json.sig"
catalog_path.write_bytes(raw)
sig_path.write_bytes(sig)
assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
def test_load_bundled_catalog_entries_real_shipped_catalog():
"""End-to-end regression against the actual bundled data/catalog.json +
.sig, using the real CATALOG_PUBKEYS (no monkeypatch) -- this is what
the Browse dialog actually calls on startup."""
root = Path(__file__).resolve().parent.parent
entries = c.load_bundled_catalog_entries(
root / "data" / "catalog.json", root / "data" / "catalog.json.sig"
)
assert len(entries) == 19
assert {e["id"] for e in entries} >= {"filesystem", "github", "slack", "postgres"}