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 1063 additions and 2412 deletions
+2 -50
View File
@@ -95,39 +95,12 @@ jobs:
- name: Install dependencies
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
env:
EXPECTED_CATALOG_PUBKEY_B64: "082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="
run: |
python - <<'PY'
import base64, os, pathlib, sys
import pathlib, sys
import bcc_core as c
expected_pubkey_b64 = os.environ["EXPECTED_CATALOG_PUBKEY_B64"]
raw = pathlib.Path("data/catalog.json").read_bytes()
sig_path = pathlib.Path("data/catalog.json.sig")
@@ -138,26 +111,6 @@ jobs:
if b"\x00" * 32 in c.CATALOG_PUBKEYS:
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):
sys.exit(
"FAIL: data/catalog.json does NOT match its signature.\n"
@@ -171,6 +124,5 @@ jobs:
if problems:
sys.exit("FAIL: catalog failed validation:\n " + "\n ".join(problems))
print("OK: catalog signature verifies, the pubkey matches the CI trust anchor, "
"and the catalog validates clean.")
print("OK: catalog signature verifies and the catalog validates clean.")
PY
+22 -49
View File
@@ -101,20 +101,9 @@ jobs:
# signing step — which means a wrong/missing RELEASE_SIGNING_KEY secret
# would only be discovered at the worst possible moment: during a real
# 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
# 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
# It proves the two halves of the keypair actually match, without
# publishing anything. Run it from the Actions tab after setting or
# rotating the secret.
signing-smoke-test:
@@ -131,54 +120,42 @@ jobs:
- name: Install dependencies
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:
RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }}
run: |
if [ -z "$RELEASE_SIGNING_KEY" ]; then
echo "FAIL: RELEASE_SIGNING_KEY secret is not set."
echo "Generate the RELEASE key (NOT the catalog key) with:"
echo " python catalog_console.py keygen --release"
echo "then add its seed under Settings -> Actions -> Secrets, via:"
echo " python catalog_console.py show-seed-b64 --release"
echo "Generate it with: python catalog_console.py show-seed-b64"
echo "then add it under Settings -> Actions -> Secrets."
exit 1
fi
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 sign --sums smoke/SHA256SUMS --out smoke/SHA256SUMS.sig
python - <<'PY'
import pathlib, sys
from scripts.sign_checksums import RELEASE_PUBKEYS, verify_checksums_against_any
import base64, pathlib, sys
import bcc_core as c
from scripts.sign_checksums import verify_checksums
# Deliberately does NOT import bcc_core / CATALOG_PUBKEYS at all --
# this smoke test must never be able to compare the CI secret
# against the catalog's root of trust (issue #68 finding 5). Only
# RELEASE_PUBKEYS (scripts/sign_checksums.py) is a legitimate
# 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."
)
# The public half that ships inside the binary. If the secret is a
# DIFFERENT key than the one users' copies trust, this fails here --
# which is the entire point of the job.
pub_b64 = base64.b64encode(c.CATALOG_PUBKEYS[0]).decode()
sums = pathlib.Path("smoke/SHA256SUMS").read_text()
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(
"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"
"The secret and the shipped release public key are different keypairs.\n"
"Downloaders would reject every signature this CI produces. Re-copy the\n"
"seed from `catalog_console.py show-seed-b64 --release`, or update\n"
"RELEASE_PUBKEYS with the matching public key."
"The secret and the shipped public key are different keypairs. Users\n"
"would reject every signature this CI produces. Re-copy the seed from\n"
"`catalog_console.py show-seed-b64`, or update CATALOG_PUBKEYS."
)
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
# ── Create GitHub Release with all three artifacts ──────────────────────
@@ -229,13 +206,9 @@ jobs:
# checks. It does NOT remove Gatekeeper/SmartScreen warnings.
#
# The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw
# Ed25519 seed) for the RELEASE key -- a SEPARATE keypair from the
# catalog key, generated via `python catalog_console.py keygen
# --release` (issue #68 finding 5; #62). This key is intentionally
# 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.
# Ed25519 seed) generated via the Catalog Console (#62). 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
id: signing
run: |
@@ -261,7 +234,7 @@ jobs:
- name: Warn — release will be unsigned
if: steps.signing.outputs.has_key != 'true'
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
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
on a mirror, not about vouching for the software.
This manifest is signed with BCC's **release key**, which is a different
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.
**Release signing public key** (Ed25519, base64, raw 32 bytes):
```
<PLACEHOLDER — AJ: paste the public key from the Catalog Console (#62) here>
```
### macOS / Linux
@@ -58,7 +59,7 @@ To also verify the manifest's signature (optional, requires Python +
```bash
python3 scripts/sign_checksums.py verify \
--sums SHA256SUMS --sig SHA256SUMS.sig \
--pubkey-b64 "<the release public key from Signing keys, below>"
--pubkey-b64 "<the public key above>"
```
### 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,
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
```bash
@@ -190,7 +152,6 @@ file is also listed, marked *legacy*, so you can copy them over.
- `bcc.spec` — PyInstaller build spec (cross-platform).
- `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)).
- `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
+436 -292
View File
@@ -10,7 +10,7 @@ Run: python mcp_manager.py
from __future__ import annotations
import contextlib
import html
import sys
import time
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.QtGui import (
QAction,
QActionGroup,
QColor,
QCursor,
QDesktopServices,
@@ -27,12 +26,13 @@ from PySide6.QtGui import (
QIcon,
QKeySequence,
QPainter,
QPalette,
QPixmap,
QTextCursor,
)
from PySide6.QtWidgets import (
QAbstractItemView,
QApplication,
QButtonGroup,
QCheckBox,
QComboBox,
QDialog,
@@ -68,122 +68,105 @@ import bcc_core as core
# thread during drag-and-drop import, so skip anything larger than this.
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 = {
"ok": "\u25cf",
"missing": "\u25cf",
"warn": "\u25b2",
"remote": "\u25c6",
"unknown": "\u25cb",
}
def plain_label(text: object) -> QLabel:
"""A QLabel guaranteed to render `text` as plain text, never HTML.
Qt's QLabel auto-interprets HTML by default (Qt.AutoText). Every catalog
entry field (description, notes, display name, urls -- and especially
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
# server tables' "Health" column -- distinct from the PATH-dependency Status
# column above.
HEALTH_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"}
HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
HEALTH_GLYPH = {"ok": "", "failed": "", "untested": ""}
def build_stylesheet(p: core.Palette) -> str:
"""Render the global QSS for a palette."""
return f"""
STYLESHEET = f"""
/* No font-family here on purpose: Qt already uses the native system UI font
on every platform (San Francisco / Segoe UI / desktop default). Naming
web-CSS aliases like -apple-system forces a costly font-alias scan. */
* {{ font-size: 13px; color: {p.text}; }}
QMainWindow, QDialog {{ background: {p.bg}; }}
* {{ font-size: 13px; color: {TEXT}; }}
QMainWindow, QDialog {{ background: {BG}; }}
QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
QLabel#muted {{ color: {p.muted}; }}
QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }}
QLabel#muted {{ color: {MUTED}; }}
QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }}
QLineEdit, QPlainTextEdit, QComboBox {{
background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
padding: 6px 8px; selection-background-color: {p.accent}; selection-color: {p.on_accent};
background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
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 QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border};
selection-background-color: {p.accent}; outline: none; }}
QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
QComboBox QAbstractItemView {{ background: {PANEL_2}; border: 1px solid {BORDER};
selection-background-color: {ACCENT}; outline: none; }}
QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
padding: 7px 13px; }}
QPushButton:hover {{ border: 1px solid {p.accent}; }}
QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }}
QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }}
QPushButton#primary:hover {{ background: {p.accent_dim}; }}
QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }}
QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }}
QTableWidget {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px;
QPushButton:hover {{ border: 1px solid {ACCENT}; }}
QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }}
QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }}
QPushButton#primary:hover {{ background: {ACCENT_DIM}; }}
QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }}
QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }}
QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px;
gridline-color: transparent; outline: 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
inside a table row, so give editors a compact, flat style instead. */
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;
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;
border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }}
QHeaderView::section {{ background: {PANEL}; color: {MUTED}; border: none;
border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }}
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; }}
QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
QLabel#warnBanner {{ color: {p.on_accent}; background: {p.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#noticeText {{ color: {p.text}; }}
QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }}
QPushButton#noticeClose:hover {{ color: {p.text}; }}
QLabel#section {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
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}; }}
QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }}
QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }}
QTableWidget#disabledTable {{ background: #202229; }}
QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }}
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; }}
QSplitter::handle {{ background: transparent; }}
QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }}
QSplitter::handle:pressed {{ background: {p.accent}; border-radius: 4px; }}
QSplitter::handle:hover {{ background: {BORDER}; 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)
# --------------------------------------------------------------------------- #
@@ -268,7 +251,7 @@ class _SecretMaskDelegate(QStyledItemDelegate):
if self.revealed or not option.text:
return
key_item = self._table.item(index.row(), 0)
if key_item and core.should_mask_value(key_item.text(), option.text):
if key_item and core.is_secret_key(key_item.text()):
option.text = core.MASK
@@ -716,6 +699,39 @@ class ServerEditor(QFrame):
def current_name(self) -> str:
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):
self.stack.setCurrentIndex(self.type.currentIndex())
self._emit()
@@ -1244,6 +1260,262 @@ class PasteDialog(QDialog):
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
# 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))
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
# 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()
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.
split = QSplitter(Qt.Orientation.Horizontal)
split.setChildrenCollapsible(False)
@@ -1678,96 +1897,11 @@ class MainWindow(QMainWindow):
# --- menu bar ---------------------------------------------------------- #
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")
# "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)
# 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)
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):
AboutDialog(self).exec()
@@ -1788,9 +1922,10 @@ class MainWindow(QMainWindow):
if release is None:
return # offline/failed check: don't advance lastCheck, allow retry
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
notice = core.update_notice(core.__version__, release)
if notice:
self._show_update_notice(notice)
if core.is_newer_version(core.__version__, release["version"]):
self.status.setText(
f"Update available: {release['version']} · Help ▸ About to view it."
)
# --- layout persistence ---------------------------------------------- #
def _restore_layout(self):
@@ -1957,6 +2092,10 @@ class MainWindow(QMainWindow):
self.del_btn = QPushButton("Delete")
self.del_btn.setObjectName("danger")
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.undo_btn = QPushButton("Undo")
self.undo_btn.setEnabled(False)
@@ -1969,6 +2108,7 @@ class MainWindow(QMainWindow):
self.dup_btn.clicked.connect(self.duplicate_server)
self.del_btn.clicked.connect(self.delete_server)
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.undo_btn.clicked.connect(self._undo)
self.test_all_btn.clicked.connect(self._test_all_servers)
@@ -1977,6 +2117,7 @@ class MainWindow(QMainWindow):
self.dup_btn,
self.del_btn,
self.paste_btn,
self.browse_catalog_btn,
self.copy_btn,
self.undo_btn,
self.test_all_btn,
@@ -2116,21 +2257,9 @@ class MainWindow(QMainWindow):
return
self.full_config = cfg
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.current_profile = profile
self.servers = servers
self.servers = core.extract_servers(self.full_config)
self.dirty = False
self.restart_btn.hide()
self._undo_stack.clear()
@@ -2453,7 +2582,7 @@ class MainWindow(QMainWindow):
entry = self.servers[idx]
old_name = entry.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
# its existing row in place rather than re-rendering.
# An edit invalidates any cached "Test all" result -- the server that
@@ -2547,7 +2676,7 @@ class MainWindow(QMainWindow):
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
)
if ans == QMessageBox.StandardButton.Yes:
self.servers[existing[name]].set_data(data)
self.servers[existing[name]].data = data
return False, True
name = core.resolve_name_collision(name, {s.name for s in self.servers})
self.servers.append(core.ServerEntry(name, data, True))
@@ -2567,6 +2696,41 @@ class MainWindow(QMainWindow):
self._mark_dirty()
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):
idx = self._current_index()
if not (0 <= idx < len(self.servers)):
@@ -2622,12 +2786,6 @@ class MainWindow(QMainWindow):
self.save_btn.setEnabled(False)
return False
lint_warnings = core.lint_servers(self.servers)
# ${VAR} references are only meaningful if the target client expands
# them -- Claude Desktop doesn't, so the same config is fine in one
# profile and broken in another (#76). Report against the loaded one.
for entry in self.servers:
for warning in core.env_ref_warnings(entry.data, self.current_profile):
lint_warnings.append(f"'{entry.name}': {warning}")
if lint_warnings:
self.validation_lbl.setText(f"{lint_warnings[0]}")
self.validation_lbl.setStyleSheet(f"color: {WARN};")
@@ -2651,6 +2809,29 @@ class MainWindow(QMainWindow):
QMessageBox.warning(self, "Can't save yet", "Fix the highlighted problem first.")
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.
# Compare mtime AND size (not mtime alone) so a concurrent external write
# that lands within the mtime resolution window, or that restores the
@@ -2673,11 +2854,6 @@ class MainWindow(QMainWindow):
except Exception as e:
QMessageBox.critical(self, "Reload failed", str(e))
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)
try:
backup = core.write_config(self.current_profile.path, fresh)
@@ -2690,13 +2866,8 @@ class MainWindow(QMainWindow):
self.dirty = False
self.save_btn.setEnabled(False)
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(
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."
)
self._offer_restart_button()
@@ -2854,33 +3025,6 @@ class MainWindow(QMainWindow):
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():
if sys.platform == "win32":
# Without an explicit AppUserModelID, Windows taskbar groups the app
@@ -2899,7 +3043,7 @@ def main():
icon = _app_icon()
if not icon.isNull():
app.setWindowIcon(icon)
app.setStyleSheet(theme_stylesheet_for(app))
app.setStyleSheet(STYLESHEET)
win = MainWindow()
win.show()
sys.exit(app.exec())
+5 -1
View File
@@ -31,7 +31,11 @@ a = Analysis(
["bcc.py"],
pathex=[],
binaries=[],
datas=[("icons", "icons"), ("data/catalog.json", "data")],
datas=[
("icons", "icons"),
("data/catalog.json", "data"),
("data/catalog.json.sig", "data"),
],
hiddenimports=[],
hookspath=[],
hooksconfig={},
+252 -762
View File
File diff suppressed because it is too large Load Diff
+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,
or the current tip of `main`. The Console fetches the exact
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
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".
blob SHA for the rest of this review pass.
2. Review -- a semantic diff (catalog_review.diff_catalogs), one card per
changed entry, with risk annotations
(catalog_review.entry_risk_findings). A registry lookup for
@@ -30,34 +25,20 @@ Flow: Load -> Review -> Sign.
changed entry must be individually acknowledged (its
checkbox ticked) before Sign unlocks. There is no
"acknowledge all" -- see catalog_review.py.
3. Sign -- re-resolves the current blob SHA from the SAME ref that was
reviewed (never a hardcoded "main") and refuses to sign
unless it still matches the pinned SHA from step 1, the diff
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
3. Sign -- re-fetches the current blob SHA and refuses to sign unless
it still matches the pinned SHA from step 1 (TOCTOU fix:
catalog_review.can_sign). On success, writes
data/catalog.json + data/catalog.json.sig and commits BOTH
in a single commit, then pushes -- so main is never red
between a catalog merge and its signature. Signing uses the
CATALOG key ONLY -- see "Two signing keys" below.
between a catalog merge and its signature.
The signature must be the artefact of an actual review, not a step that
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
import argparse
import base64
import contextlib
import getpass
import html
@@ -88,28 +69,8 @@ SIG_PATH = "data/catalog.json.sig"
# 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
# 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_KINDS = ("catalog", "release")
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"
KEY_STORAGE_FILE = KEY_STORAGE_DIR / "signing_key.enc"
HTTP_TIMEOUT = 6.0
@@ -136,64 +97,51 @@ def _keyring_module():
_KEYRING_SERVICE = "bcc-catalog-console"
_KEYRING_USERNAME = "signing-key"
def _keyring_username(kind: str) -> 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:
def store_encrypted_key(blob: bytes) -> str:
"""Persist an already-encrypted key blob (see
catalog_review.encrypt_private_key) under the given `kind`
("catalog" or "release" -- see the KEY_STORAGE_DIR comment above; the
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
catalog_review.encrypt_private_key). 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."""
keyring = _keyring_module()
if keyring is not None:
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)"
except Exception:
pass # fall through to the file-based path
KEY_STORAGE_DIR.mkdir(parents=True, exist_ok=True)
key_file = _key_storage_file(kind)
key_file.write_bytes(blob)
KEY_STORAGE_FILE.write_bytes(blob)
with contextlib.suppress(OSError): # best-effort on platforms without POSIX perm bits
key_file.chmod(0o600)
return f"encrypted file at {key_file}"
KEY_STORAGE_FILE.chmod(0o600)
return f"encrypted file at {KEY_STORAGE_FILE}"
def load_encrypted_key(kind: str = "catalog") -> bytes:
"""Load the encrypted key blob of the given `kind` from wherever
store_encrypted_key() put it. Raises FileNotFoundError if no key of that
kind has been generated yet."""
def load_encrypted_key() -> bytes:
"""Load the encrypted key blob from wherever store_encrypted_key() put
it. Raises FileNotFoundError if no key has been generated yet."""
keyring = _keyring_module()
if keyring is not None:
try:
hex_blob = keyring.get_password(_KEYRING_SERVICE, _keyring_username(kind))
hex_blob = keyring.get_password(_KEYRING_SERVICE, _KEYRING_USERNAME)
if hex_blob:
return bytes.fromhex(hex_blob)
except Exception:
pass
key_file = _key_storage_file(kind)
if not key_file.exists():
flag = " --release" if kind == "release" else ""
if not KEY_STORAGE_FILE.exists():
raise FileNotFoundError(
f"No {kind} signing key found (checked the OS keychain and {key_file}). "
f"Run `python catalog_console.py keygen{flag}` first."
f"No signing key found (checked the OS keychain and {KEY_STORAGE_FILE}). "
"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:
"""Load + decrypt the signing key seed of the given `kind`. Raises
ValueError on a wrong passphrase, FileNotFoundError if no key of that
kind exists yet. Defaults to "catalog" because that's the key
ReviewWindow._on_sign uses -- the GUI never touches the release key."""
blob = load_encrypted_key(kind)
def unlock_signing_key(passphrase: str) -> bytes:
"""Load + decrypt the signing key seed. Raises ValueError on a wrong
passphrase, FileNotFoundError if no key exists yet."""
blob = load_encrypted_key()
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
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(
repo_dir: Path, raw_bytes: bytes, signature: bytes, *, branch: str = "main"
) -> str:
@@ -739,50 +644,28 @@ class ReviewWindow(QMainWindow):
row = self.source_list.currentRow()
try:
if row <= 0:
# source = main: diff against the last catalog a maintainer
# actually SIGNED (the bytes covered by the current
# 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": [],
}
)
commit = fetch_ref(self.repo_dir, "main")
old_commit = None # main vs itself has no "old" -- nothing to diff without a base
else:
pr = self._prs[row - 1]
loaded_ref = pr.head_ref
commit = fetch_ref(self.repo_dir, loaded_ref)
commit = fetch_ref(self.repo_dir, pr.head_ref)
old_commit = fetch_ref(self.repo_dir, "main")
new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
new_catalog = core.load_catalog(new_raw)
new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
new_catalog = core.load_catalog(new_raw)
if old_commit:
old_raw, _old_sha = read_catalog_at_commit(self.repo_dir, old_commit)
old_catalog = core.load_catalog(old_raw)
else:
old_catalog = new_catalog
except (GitError, ValueError) as e:
QMessageBox.critical(self, "Load failed", html.escape(str(e)))
return
self._new_raw = new_raw
# `loaded_ref` is pinned into the session (not just this method's
# 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.session = review.start_review(new_blob_sha, old_catalog, new_catalog)
self._render_cards()
def _render_cards(self):
@@ -823,43 +706,20 @@ class ReviewWindow(QMainWindow):
def _on_sign(self):
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:
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:
QMessageBox.critical(self, "Sign failed", html.escape(str(e)))
return
decision = review.can_sign(self.session, current_sha)
if not decision.ok:
QMessageBox.warning(self, "Cannot sign", html.escape(decision.reason or ""))
# Only the TOCTOU blob-SHA-mismatch reason should trigger a
# reload -- "mismatch" appears ONLY in that reason (deliberately
# 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()
if decision.reason and "changed" in decision.reason.lower():
self._on_load() # force a re-review against the new bytes
return
dialog = PassphraseDialog("Enter CATALOG signing key passphrase:", self)
dialog = PassphraseDialog("Enter signing key passphrase:", self)
if dialog.exec() != QDialog.DialogCode.Accepted:
return
try:
@@ -884,17 +744,9 @@ class ReviewWindow(QMainWindow):
# --------------------------------------------------------------------------- #
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"
def cmd_keygen(_args: argparse.Namespace) -> int:
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: ")
if passphrase != confirm:
print("error: passphrases did not match", file=sys.stderr)
@@ -904,67 +756,31 @@ def cmd_keygen(args: argparse.Namespace) -> int:
return 1
blob = review.encrypt_private_key(seed, passphrase)
where = store_encrypted_key(blob, kind=kind)
pubkey_b64 = base64.b64encode(pubkey).decode("ascii")
where = store_encrypted_key(blob)
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()
if kind == "catalog":
print(
"This is the CATALOG key. It is the root of trust for every catalog "
"entry BCC ships -- it must stay offline and Console-only. NEVER paste "
"it, its seed, or `show-seed-b64` output into CI, an env var, or a repo "
"secret. (If the key currently in bcc_core.CATALOG_PUBKEYS has ever been "
"pasted into a CI secret, treat it as burned for catalog use -- generate "
"a fresh one with this command and rotate.)"
)
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")
print("Public key (base64, paste into bcc_core.CATALOG_PUBKEYS):")
print(f" {pubkey_b64}")
print()
print(
"Also add it as the Gitea repo secret RELEASE_SIGNING_KEY (base64 of the "
"32-byte private seed) used by release.yml -- get that value with:"
)
print(" python catalog_console.py show-seed-b64 # careful: prints the raw key")
return 0
def cmd_show_seed_b64(args: argparse.Namespace) -> int:
# Deliberately requires --release: this command's whole purpose is to
# 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: ")
def cmd_show_seed_b64(_args: argparse.Namespace) -> int:
passphrase = getpass.getpass("Signing key passphrase: ")
try:
seed = unlock_signing_key(passphrase, kind="release")
seed = unlock_signing_key(passphrase)
except (FileNotFoundError, ValueError) as e:
print(f"error: {e}", file=sys.stderr)
return 1
import base64
print(base64.b64encode(seed).decode("ascii"))
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.set_defaults(func=cmd_gui)
p_keygen = sub.add_parser(
"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 = sub.add_parser("keygen", help="generate a new Ed25519 signing keypair")
p_keygen.set_defaults(func=cmd_keygen)
p_seed = sub.add_parser(
"show-seed-b64",
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",
"show-seed-b64", help="print the base64 private seed (for the RELEASE_SIGNING_KEY secret)"
)
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_ALLOWED_COMMANDS
from bcc_core import verify_catalog_signature as _verify_catalog_signature
# --------------------------------------------------------------------------- #
# Semantic diff
@@ -433,21 +432,11 @@ def has_blocking_risk(change: EntryChange) -> bool:
class ReviewSession:
"""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
can_sign()/sign_precondition().
`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).
"""
can_sign()."""
pinned_blob_sha: str
old_catalog: dict
new_catalog: dict
loaded_ref: str = "main"
changes: list[EntryChange] = field(default_factory=list)
acknowledged: set[str] = field(default_factory=set)
@@ -456,39 +445,12 @@ class ReviewSession:
self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
def start_review(
pinned_blob_sha: str,
old_catalog: dict,
new_catalog: dict,
loaded_ref: str = "main",
) -> ReviewSession:
def start_review(pinned_blob_sha: str, old_catalog: dict, new_catalog: dict) -> ReviewSession:
return ReviewSession(
pinned_blob_sha=pinned_blob_sha,
old_catalog=old_catalog,
new_catalog=new_catalog,
loaded_ref=loaded_ref,
pinned_blob_sha=pinned_blob_sha, old_catalog=old_catalog, new_catalog=new_catalog
)
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:
ids = {c.entry_id for c in session.changes}
if entry_id not in ids:
@@ -521,53 +483,22 @@ class SignDecision:
def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
"""Whether the Sign button may fire right now.
Four independent gates, all required, checked in this order:
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())
Two independent gates, both required:
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing)
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
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"
true rather than aspirational: the signature is bound to the exact
reviewed bytes, not to "whatever the file happens to be now".
2. No blocking risk finding may be outstanding on ANY changed entry, full
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.
2. 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:
return SignDecision(
False,
"The reviewed bytes changed since this review began (blob SHA "
"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):
pending = sorted({c.entry_id for c in session.changes} - session.acknowledged)
return SignDecision(
@@ -576,28 +507,6 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
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:
"""The exact bytes that get signed: bcc_core's domain-separation prefix
(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.
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
@@ -154,19 +135,6 @@ def public_key_b64_from_seed(seed_b64: str) -> str:
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
# --------------------------------------------------------------------------- #
+3 -143
View File
@@ -334,29 +334,13 @@ def test_acknowledge_gating_requires_every_entry():
assert r.all_entries_acknowledged(session) is True
def test_no_acknowledge_all_shortcut_and_gate_is_real():
"""Two things, both load-bearing (issue #68: the original version of
this test asserted ONLY the first half, and passed the entire time the
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".
"""
def test_no_acknowledge_all_function_exists():
"""Deliberate: there must be no shortcut to acknowledge every entry at
once. See the comment in catalog_review.py above SignDecision."""
names = [n for n in dir(r) if "acknowledge" in n.lower()]
assert "acknowledge_all" 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
@@ -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()
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
# --------------------------------------------------------------------------- #
+270 -679
View File
File diff suppressed because it is too large Load Diff