Compare commits
1 Commits
main
..
dffa0e152f
| Author | SHA1 | Date | |
|---|---|---|---|
 Cowork Supervisor
|
dffa0e152f |
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -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)):
|
||||
@@ -2645,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
|
||||
@@ -2667,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)
|
||||
@@ -2684,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()
|
||||
@@ -2848,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
|
||||
@@ -2893,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())
|
||||
|
||||
@@ -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={},
|
||||
|
||||
+246
-582
@@ -15,7 +15,6 @@ from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import contextlib
|
||||
import copy
|
||||
import difflib
|
||||
import functools
|
||||
import glob
|
||||
@@ -30,6 +29,7 @@ import tempfile
|
||||
import threading
|
||||
import time
|
||||
from dataclasses import dataclass
|
||||
from datetime import date
|
||||
from pathlib import Path
|
||||
from typing import NamedTuple
|
||||
from urllib.parse import urlparse
|
||||
@@ -50,12 +50,6 @@ DISABLED_KEY = "_disabledMcpServers"
|
||||
# parks the rest under DISABLED_KEY.
|
||||
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"
|
||||
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}
|
||||
|
||||
|
||||
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
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -218,195 +179,16 @@ class Profile:
|
||||
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
|
||||
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
|
||||
data: dict
|
||||
enabled: bool = True
|
||||
raw: object = NO_RAW
|
||||
|
||||
@property
|
||||
def kind(self) -> str:
|
||||
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
|
||||
@@ -671,30 +453,13 @@ def repair_config_file(path: str | os.PathLike) -> tuple[dict, list[str], str]:
|
||||
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]:
|
||||
"""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`).
|
||||
"""
|
||||
"""Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers."""
|
||||
out: list[ServerEntry] = []
|
||||
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():
|
||||
out.append(_server_entry(name, data, False))
|
||||
out.append(ServerEntry(name=name, data=dict(data), enabled=False))
|
||||
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
|
||||
and the position of `mcpServers`. Returns the same dict for convenience.
|
||||
"""
|
||||
enabled = {s.name: s.config_value() for s in servers if s.enabled}
|
||||
disabled = {s.name: s.config_value() for s in servers if not s.enabled}
|
||||
enabled = {s.name: s.data for s in servers if 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
|
||||
if disabled:
|
||||
@@ -800,34 +565,6 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
|
||||
# --------------------------------------------------------------------------- #
|
||||
# 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:
|
||||
bdir = path.parent / BACKUP_DIRNAME
|
||||
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]:
|
||||
"""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).
|
||||
"""
|
||||
"""Concatenate lint_server warnings across every entry, in order."""
|
||||
out: list[str] = []
|
||||
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))
|
||||
return out
|
||||
|
||||
@@ -2440,30 +2165,6 @@ def restart_claude_desktop() -> RestartResult:
|
||||
# is rejected by validate_catalog() regardless of how plausible it looks.
|
||||
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
|
||||
# (not a single key) so keys can be rotated without bricking installs that
|
||||
# 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_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
|
||||
# a "freeze" attack: a compromised/leaked signing key claiming an absurd
|
||||
# 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
|
||||
|
||||
|
||||
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]:
|
||||
"""Validate the `config` block of a basic-tier catalog entry."""
|
||||
if not isinstance(config, dict):
|
||||
@@ -2728,11 +2284,10 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
|
||||
f"({', '.join(sorted(CATALOG_ALLOWED_COMMANDS))})."
|
||||
)
|
||||
|
||||
raw_args = config.get("args")
|
||||
args_ok = isinstance(raw_args, list) and all(isinstance(a, str) for a in raw_args)
|
||||
args = raw_args if args_ok else []
|
||||
if not args_ok:
|
||||
args = config.get("args")
|
||||
if not isinstance(args, list) or not all(isinstance(a, str) for a in args):
|
||||
problems.append(f"{tag}: config.args must be a list of strings.")
|
||||
args = []
|
||||
|
||||
for a in args:
|
||||
if not a.isascii():
|
||||
@@ -2751,55 +2306,11 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
|
||||
if command == "docker":
|
||||
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")
|
||||
if env is not None:
|
||||
env_ok = isinstance(env, dict) and all(
|
||||
isinstance(k, str) and isinstance(v, str) for k, v in env.items()
|
||||
)
|
||||
if not env_ok:
|
||||
if env is not None and (
|
||||
not isinstance(env, dict) or any(not isinstance(v, str) for v in env.values())
|
||||
):
|
||||
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
|
||||
|
||||
@@ -2819,12 +2330,6 @@ def _validate_catalog_entry(idx: int, entry, seen_ids: set[str]) -> list[str]:
|
||||
tag = f"servers[{idx}] ({entry_id!r})"
|
||||
if not entry_id.isascii():
|
||||
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:
|
||||
problems.append(f"{tag}: duplicate id.")
|
||||
seen_ids.add(entry_id)
|
||||
@@ -2938,32 +2443,15 @@ def verify_catalog_signature(raw: bytes, sig: bytes, pubkeys: list[bytes]) -> bo
|
||||
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(
|
||||
bundled: tuple[bytes, bytes] | None,
|
||||
cached: tuple[bytes, bytes] | None,
|
||||
remote: tuple[bytes, bytes] | None,
|
||||
floor: int = 0,
|
||||
) -> dict:
|
||||
"""
|
||||
Pick the highest-version catalog among bundled/cached/remote. Each of
|
||||
bundled/cached/remote is either None (unavailable) or an (raw_bytes,
|
||||
signature_bytes) pair.
|
||||
Pick the highest-version catalog among bundled/cached/remote. Each
|
||||
argument is either None (unavailable) or an (raw_bytes, signature_bytes)
|
||||
pair.
|
||||
|
||||
🔴 SECURITY: every candidate — including `bundled`, the copy frozen into
|
||||
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
|
||||
runtime, every time) closes that.
|
||||
|
||||
`floor` is a pure, caller-supplied lower bound (e.g. a persisted
|
||||
"last accepted version" the GUI can load from disk and pass in) — this
|
||||
function does no storage of its own.
|
||||
Anti-rollback: a candidate's version is never accepted if it's lower
|
||||
than the best verified candidate already found in this same resolution
|
||||
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
|
||||
including the first one evaluated: the previous version of this
|
||||
function only ran these checks `if best_version >= 0`, i.e. once a
|
||||
candidate had already been accepted in this pass. That let the FIRST
|
||||
verified candidate through unconditionally — a signed catalog claiming
|
||||
version=999999999 sailed straight past both guards if it happened to be
|
||||
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).
|
||||
Anti-freeze: a candidate whose version leaps more than
|
||||
_CATALOG_MAX_VERSION_JUMP past the current best is also rejected. A
|
||||
compromised/leaked signing key claiming an absurd future version would
|
||||
otherwise permanently outrank every legitimate release from then on,
|
||||
since the resolver always prefers the highest verified version — this
|
||||
caps how far a single accepted jump can go.
|
||||
|
||||
Returns the winning catalog dict, or {} if nothing verified and
|
||||
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_version = -1
|
||||
best_is_bundled = False
|
||||
|
||||
for source, data, version in candidates:
|
||||
if data is None:
|
||||
for candidate in (bundled, cached, remote):
|
||||
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
|
||||
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"
|
||||
better = version > best_version or (
|
||||
version == best_version and is_bundled and not best_is_bundled
|
||||
)
|
||||
if better:
|
||||
version = catalog_version(data)
|
||||
if best_version >= 0:
|
||||
if version < best_version:
|
||||
continue # anti-rollback
|
||||
if version > best_version + _CATALOG_MAX_VERSION_JUMP:
|
||||
continue # anti-freeze
|
||||
|
||||
best = data
|
||||
best_version = version
|
||||
best_is_bundled = is_bundled
|
||||
|
||||
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}}
|
||||
shape parse_pasted_json()/_import_server() already understand, so the
|
||||
(future) catalog picker dialog can feed a selection straight into the
|
||||
existing paste-import path instead of growing a parallel one.
|
||||
Browse-catalog dialog can feed a selection straight into the existing
|
||||
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 {}
|
||||
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", ""),
|
||||
"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:
|
||||
data["env"] = dict(env)
|
||||
data["env"] = env
|
||||
return {str(name): data}
|
||||
|
||||
|
||||
@@ -3075,3 +2552,190 @@ def config_has_unfilled_placeholders(cfg: dict) -> bool:
|
||||
if isinstance(env, dict):
|
||||
values.extend(v for v in env.values() if isinstance(v, str))
|
||||
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 []
|
||||
|
||||
+55
-256
@@ -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)
|
||||
|
||||
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("Public key (base64, paste into bcc_core.CATALOG_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:"
|
||||
"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 --release # prints the raw key")
|
||||
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
@@ -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
|
||||
|
||||
@@ -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
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
@@ -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
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
+300
-574
@@ -1,7 +1,6 @@
|
||||
"""Pytest port of the original test_core.py script (same 23 behaviours, now
|
||||
proper test functions with tmp_path/monkeypatch fixtures)."""
|
||||
|
||||
import dataclasses
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
@@ -1691,12 +1690,8 @@ def _minimal_catalog(version: int = 1) -> dict:
|
||||
"official": True,
|
||||
"setup": "basic",
|
||||
"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",
|
||||
"args": ["-y", "widget-mcp@1.0.0"],
|
||||
"args": ["-y", "widget-mcp"],
|
||||
},
|
||||
"placeholders": {},
|
||||
"env_required": {},
|
||||
@@ -1957,213 +1952,6 @@ def test_validate_catalog_rejects_duplicate_ids():
|
||||
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 -------------------------------------------------------- #
|
||||
def test_resolve_catalog_nothing_available_returns_empty_dict():
|
||||
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()
|
||||
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
|
||||
|
||||
# The freeze attempt goes FIRST (as `cached`), the legitimate catalog
|
||||
# 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.
|
||||
cached = _signed(_minimal_catalog(version=5), 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
|
||||
|
||||
|
||||
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):
|
||||
priv = Ed25519PrivateKey.generate()
|
||||
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():
|
||||
entry = _minimal_catalog()["servers"][0]
|
||||
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():
|
||||
# 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["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
|
||||
result = c.catalog_entry_to_paste_json(entry)
|
||||
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()
|
||||
malicious["servers"][0]["config"]["env"] = {"NODE_OPTIONS": "--require /tmp/payload.js"}
|
||||
assert c.validate_catalog(malicious) != []
|
||||
def test_catalog_entry_to_paste_json_seeds_env_required_keys():
|
||||
# Regression: env_required is where the seed data actually keeps its
|
||||
# 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():
|
||||
@@ -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():
|
||||
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
|
||||
# --------------------------------------------------------------------------- #
|
||||
# --- catalog_category_group / CATALOG_CATEGORY_GROUPS --------------------- #
|
||||
@pytest.mark.parametrize(
|
||||
"setting,system_dark,expected",
|
||||
"category,expected_group",
|
||||
[
|
||||
(c.THEME_DARK, False, "dark"),
|
||||
(c.THEME_DARK, True, "dark"),
|
||||
(c.THEME_LIGHT, False, "light"),
|
||||
(c.THEME_LIGHT, True, "light"),
|
||||
(c.THEME_SYSTEM, True, "dark"),
|
||||
(c.THEME_SYSTEM, False, "light"),
|
||||
("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"),
|
||||
],
|
||||
)
|
||||
def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected):
|
||||
assert c.resolve_theme(setting, system_dark) == expected
|
||||
def test_catalog_category_group_maps_every_taxonomy_value(category, expected_group):
|
||||
assert c.catalog_category_group(category) == expected_group
|
||||
|
||||
|
||||
@pytest.mark.parametrize("junk", ["", "solarized", None, "DARK", 3])
|
||||
def test_resolve_theme_falls_back_to_following_the_system(junk):
|
||||
"""A hand-edited or future QSettings value should follow the desktop,
|
||||
not pin a fixed theme."""
|
||||
assert c.resolve_theme(junk, True) == "dark"
|
||||
assert c.resolve_theme(junk, False) == "light"
|
||||
def test_catalog_category_group_unknown_falls_back_to_other():
|
||||
assert c.catalog_category_group("some-future-category-nobody-has-seen-yet") == "Other"
|
||||
assert c.catalog_category_group("") == "Other"
|
||||
assert c.catalog_category_group(None) == "Other"
|
||||
|
||||
|
||||
def test_palette_for_known_names():
|
||||
assert c.palette_for("dark") is c.DARK_PALETTE
|
||||
assert c.palette_for("light") is c.LIGHT_PALETTE
|
||||
def test_catalog_category_group_is_case_insensitive():
|
||||
assert c.catalog_category_group("Files") == "Files & Dev"
|
||||
assert c.catalog_category_group("DATABASE") == "Data"
|
||||
|
||||
|
||||
def test_palette_for_unknown_name_falls_back_to_dark():
|
||||
assert c.palette_for("chartreuse") is c.DARK_PALETTE
|
||||
def test_shipped_catalog_categories_all_have_a_known_group():
|
||||
"""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():
|
||||
"""v1.3.0 shipped these exact colours; adding a light theme must not
|
||||
quietly restyle the dark one."""
|
||||
p = c.DARK_PALETTE
|
||||
assert (p.accent, p.bg, p.panel, p.panel_2) == ("#f97316", "#1b1d23", "#23262e", "#2b2f39")
|
||||
assert (p.text, p.muted, p.border) == ("#e7e9ee", "#9aa0ad", "#3a3f4b")
|
||||
assert (p.good, p.bad, p.warn, p.remote) == ("#4ade80", "#f87171", "#fbbf24", "#60a5fa")
|
||||
assert (p.on_accent, p.disabled_bg, p.mono_bg) == ("#1a1205", "#202229", "#16181d")
|
||||
# --- catalog_entry_matches_query / filter_catalog_entries ------------------ #
|
||||
def _catalog_entries():
|
||||
return [
|
||||
{
|
||||
"id": "filesystem",
|
||||
"display": "Filesystem",
|
||||
"description": "Read/write access to local directories you choose.",
|
||||
"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():
|
||||
"""A missing slot should fail here rather than render a broken window."""
|
||||
for pal in (c.DARK_PALETTE, c.LIGHT_PALETTE):
|
||||
for f in dataclasses.fields(c.Palette):
|
||||
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}"
|
||||
def test_catalog_entry_matches_query_empty_matches_everything():
|
||||
entries = _catalog_entries()
|
||||
assert c.filter_catalog_entries(entries, "") == entries
|
||||
assert c.filter_catalog_entries(entries, " ") == entries
|
||||
|
||||
|
||||
@pytest.mark.parametrize("pal_name", ["dark", "light"])
|
||||
@pytest.mark.parametrize("slot", ["text", "muted", "good", "bad", "warn", "remote", "accent"])
|
||||
def test_palette_meets_contrast_on_panel(pal_name, slot):
|
||||
"""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
|
||||
def test_catalog_entry_matches_query_matches_id():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "postgres")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("pal_name", ["dark", "light"])
|
||||
def test_on_accent_is_legible_against_the_accent_fill(pal_name):
|
||||
"""Primary buttons and selected rows draw on_accent on top of accent."""
|
||||
pal = c.palette_for(pal_name)
|
||||
assert c.contrast_ratio(pal.on_accent, pal.accent) >= 4.5
|
||||
def test_catalog_entry_matches_query_matches_display_case_insensitive():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "SLACK")
|
||||
assert [e["id"] for e in result] == ["slack"]
|
||||
|
||||
|
||||
def test_contrast_ratio_endpoints():
|
||||
assert c.contrast_ratio("#000000", "#ffffff") == pytest.approx(21.0, abs=0.01)
|
||||
assert c.contrast_ratio("#123456", "#123456") == pytest.approx(1.0, abs=0.001)
|
||||
assert c.contrast_ratio("#ffffff", "#000000") == pytest.approx(21.0, abs=0.01)
|
||||
def test_catalog_entry_matches_query_matches_description():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "PostgreSQL database")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
|
||||
def test_relative_luminance_extremes():
|
||||
assert c.relative_luminance("#000000") == pytest.approx(0.0)
|
||||
assert c.relative_luminance("#ffffff") == pytest.approx(1.0)
|
||||
def test_catalog_entry_matches_query_matches_category():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "database")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
|
||||
def test_stylesheet_builder_has_no_hardcoded_colours():
|
||||
"""Every colour in the QSS must come from the palette.
|
||||
|
||||
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_catalog_entry_matches_query_no_match_returns_empty():
|
||||
assert c.filter_catalog_entries(_catalog_entries(), "kubernetes") == []
|
||||
|
||||
|
||||
def test_every_palette_slot_is_consumed():
|
||||
"""A slot added to Palette but never wired up is dead weight.
|
||||
|
||||
Checks for `p.<slot>` anywhere in bcc.py, which covers both the QSS and
|
||||
apply_palette's global bindings -- not every slot belongs in the
|
||||
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_all_returns_everything():
|
||||
entries = _catalog_entries()
|
||||
assert c.catalog_entries_in_group(entries, "All") == entries
|
||||
assert c.catalog_entries_in_group(entries, "") == entries
|
||||
assert c.catalog_entries_in_group(entries, None) == entries
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #78/#79 -- update notice: when to show it, and what it says
|
||||
# --------------------------------------------------------------------------- #
|
||||
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"})
|
||||
assert n is not None
|
||||
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_catalog_entries_in_group_filters_by_collapsed_category():
|
||||
result = c.catalog_entries_in_group(_catalog_entries(), "Data")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
result = c.catalog_entries_in_group(_catalog_entries(), "Work")
|
||||
assert [e["id"] for e in result] == ["slack"]
|
||||
|
||||
|
||||
def test_update_notice_is_silent_when_current():
|
||||
assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
|
||||
assert c.update_notice("1.4.0", {"version": "v1.3.0"}) is None
|
||||
# --- format_freshness_hint -------------------------------------------------- #
|
||||
def test_format_freshness_hint_none_returns_empty_string():
|
||||
assert c.format_freshness_hint(None) == ""
|
||||
assert c.format_freshness_hint("") == ""
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", [None, {}, {"version": ""}, {"version": None}, {"version": 3}, []])
|
||||
def test_update_notice_is_silent_on_a_failed_or_malformed_check(bad):
|
||||
"""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_format_freshness_hint_unparseable_returns_empty_string():
|
||||
assert c.format_freshness_hint("not-a-date") == ""
|
||||
|
||||
|
||||
def test_update_notice_falls_back_to_the_releases_page_without_a_url():
|
||||
n = c.update_notice("1.0.0", {"version": "v2.0.0"})
|
||||
assert n["url"] == c.RELEASES_URL
|
||||
def test_format_freshness_hint_this_month():
|
||||
assert (
|
||||
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():
|
||||
"""The old status-line text said 'Help > About to view it', which is wrong
|
||||
on macOS -- Qt moves the About action into the application menu (#79). The
|
||||
notice carries its own action, so it must not describe a menu path."""
|
||||
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_format_freshness_hint_one_month_singular():
|
||||
assert (
|
||||
c.format_freshness_hint("2026-06-01", today=c.date(2026, 7, 12))
|
||||
== "Last updated 1 month ago"
|
||||
)
|
||||
|
||||
|
||||
def test_update_notice_handles_the_v_prefix_consistently():
|
||||
assert c.update_notice("1.2.0", {"version": "1.3.0"}) is not None
|
||||
assert c.update_notice("v1.2.0", {"version": "v1.3.0"}) is not None
|
||||
assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
|
||||
def test_format_freshness_hint_months_ago():
|
||||
# Exactly 14 full months elapsed, no day-of-month remainder to round off.
|
||||
assert (
|
||||
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():
|
||||
"""Tags carry a 'v' prefix, __version__ doesn't -- don't show both forms
|
||||
in one sentence."""
|
||||
n = c.update_notice("1.2.0", {"version": "v1.3.0"})
|
||||
assert "v1.3.0" not in n["text"]
|
||||
assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
|
||||
# the machine-readable field keeps the real tag
|
||||
assert n["version"] == "v1.3.0"
|
||||
def test_format_freshness_hint_rounds_down_partial_month():
|
||||
# 2025-05-16 -> 2026-07-12 is 13 full months, not 14: the 14th month
|
||||
# would only complete on 2026-07-16.
|
||||
assert (
|
||||
c.format_freshness_hint("2025-05-16", today=c.date(2026, 7, 12))
|
||||
== "Last updated 13 months ago"
|
||||
)
|
||||
|
||||
|
||||
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"}
|
||||
|
||||
Reference in New Issue
Block a user