Compare commits
8 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 7ff4f6e5c0 | |||
| 7517e16b15 | |||
| 9a0433225e | |||
| fa82d30087 | |||
| 05b00a40c0 | |||
| 3068e74e5c | |||
| febd617c56 | |||
| da20eb2fdb |
@@ -120,7 +120,7 @@ jobs:
|
||||
# below to the new key's base64 form, as its own reviewed change.
|
||||
- name: Verify data/catalog.json.sig
|
||||
env:
|
||||
EXPECTED_CATALOG_PUBKEY_B64: "0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="
|
||||
EXPECTED_CATALOG_PUBKEY_B64: "082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="
|
||||
run: |
|
||||
python - <<'PY'
|
||||
import base64, os, pathlib, sys
|
||||
|
||||
@@ -90,21 +90,6 @@ key, because they protect different things and live in different places:
|
||||
| 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` |
|
||||
|
||||
**Confused about which key is which, or what state either is in?** Run:
|
||||
|
||||
```bash
|
||||
python catalog_console.py keys
|
||||
```
|
||||
|
||||
It needs no passphrase (it never touches private key bytes) and prints a
|
||||
plain-English report for both keys: where each private half lives, whether
|
||||
it's present on this machine, its fingerprint, whether that fingerprint
|
||||
matches what's actually committed in `bcc_core.py`, `ci.yml`'s trust
|
||||
anchor, and `scripts/sign_checksums.py`, and whether
|
||||
`data/catalog.json.sig` currently verifies — ending with the exact command
|
||||
to run next for whatever state it finds. This is the check that would have
|
||||
caught [issue #68](../../issues/68)'s finding 5 incident before it happened.
|
||||
|
||||
**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
|
||||
@@ -125,29 +110,12 @@ 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, which signs `SHA256SUMS` (release checksums). It does
|
||||
**not** sign `data/catalog.json` and is not the key `bcc_core.CATALOG_PUBKEYS`
|
||||
trusts:
|
||||
the RELEASE key, not the catalog key:
|
||||
|
||||
```
|
||||
6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA=
|
||||
<PLACEHOLDER — AJ: paste the release public key from `catalog_console.py keygen --release` here>
|
||||
```
|
||||
|
||||
The catalog public key (Ed25519, base64, raw 32 bytes) — this is the key
|
||||
that signs `data/catalog.json` and is trusted via `bcc_core.CATALOG_PUBKEYS`
|
||||
and the CI trust anchor in `.github/workflows/ci.yml`. It is listed here
|
||||
for completeness, not because you need it to verify a download — use the
|
||||
*release* key above for that:
|
||||
|
||||
```
|
||||
0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k=
|
||||
```
|
||||
|
||||
Both keys above were rotated 2026-07 — see [issue #68](../../issues/68)
|
||||
finding 5. The prior (shared) key is retired and is deliberately **not**
|
||||
kept in either trust list; retaining a burned key would defeat the point
|
||||
of rotating it.
|
||||
|
||||
## Run from source
|
||||
|
||||
```bash
|
||||
@@ -222,7 +190,7 @@ 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` (against `main`, an open PR, or the branch you have checked out — `--ref <branch>` to be explicit, e.g. mid key-rotation, so a rotation can be signed and pushed to its own branch *before* it's merged, never forcing a red `main`), generate/manage both signing keys (`keygen`, `keygen --release`), and report on their status (`keys`, no passphrase needed) — see [Signing keys](#signing-keys).
|
||||
- `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,6 +10,7 @@ Run: python mcp_manager.py
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
@@ -18,6 +19,7 @@ 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,
|
||||
@@ -25,6 +27,7 @@ from PySide6.QtGui import (
|
||||
QIcon,
|
||||
QKeySequence,
|
||||
QPainter,
|
||||
QPalette,
|
||||
QPixmap,
|
||||
)
|
||||
from PySide6.QtWidgets import (
|
||||
@@ -65,85 +68,122 @@ 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
|
||||
|
||||
# --- 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"
|
||||
# --- 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_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
|
||||
STATUS_GLYPH = {"ok": "●", "missing": "●", "warn": "▲", "remote": "◆", "unknown": "○"}
|
||||
STATUS_GLYPH = {
|
||||
"ok": "\u25cf",
|
||||
"missing": "\u25cf",
|
||||
"warn": "\u25b2",
|
||||
"remote": "\u25c6",
|
||||
"unknown": "\u25cb",
|
||||
}
|
||||
|
||||
# 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_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
|
||||
HEALTH_GLYPH = {"ok": "●", "failed": "●", "untested": "○"}
|
||||
HEALTH_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"}
|
||||
|
||||
STYLESHEET = f"""
|
||||
|
||||
def build_stylesheet(p: core.Palette) -> str:
|
||||
"""Render the global QSS for a palette."""
|
||||
return f"""
|
||||
/* No font-family here on purpose: Qt already uses the native system UI font
|
||||
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: {TEXT}; }}
|
||||
QMainWindow, QDialog {{ background: {BG}; }}
|
||||
* {{ font-size: 13px; color: {p.text}; }}
|
||||
QMainWindow, QDialog {{ background: {p.bg}; }}
|
||||
QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
|
||||
QLabel#muted {{ color: {MUTED}; }}
|
||||
QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }}
|
||||
QLabel#muted {{ color: {p.muted}; }}
|
||||
QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }}
|
||||
QLineEdit, QPlainTextEdit, QComboBox {{
|
||||
background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
|
||||
padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205;
|
||||
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};
|
||||
}}
|
||||
QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {ACCENT}; }}
|
||||
QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {p.accent}; }}
|
||||
QComboBox::drop-down {{ border: none; width: 22px; }}
|
||||
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;
|
||||
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;
|
||||
padding: 7px 13px; }}
|
||||
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;
|
||||
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;
|
||||
gridline-color: transparent; outline: none; }}
|
||||
QTableWidget::item {{ padding: 6px 8px; border: none; }}
|
||||
QTableWidget::item:selected {{ background: {ACCENT}; color: #1a1205; }}
|
||||
QTableWidget::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
|
||||
/* 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: {PANEL_2}; color: {TEXT}; border: 1px solid {ACCENT};
|
||||
background: {p.panel_2}; color: {p.text}; border: 1px solid {p.accent};
|
||||
border-radius: 3px; padding: 0px 4px; margin: 0px;
|
||||
selection-background-color: {ACCENT_DIM}; selection-color: #ffffff;
|
||||
selection-background-color: {p.accent_dim}; selection-color: {p.selection_text};
|
||||
}}
|
||||
QHeaderView::section {{ background: {PANEL}; color: {MUTED}; border: none;
|
||||
border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }}
|
||||
QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none;
|
||||
border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }}
|
||||
QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }}
|
||||
QScrollBar::handle:vertical {{ background: {BORDER}; border-radius: 5px; min-height: 24px; }}
|
||||
QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-height: 24px; }}
|
||||
QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
|
||||
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; }}
|
||||
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}; }}
|
||||
QPlainTextEdit#diag {{ font-family: "Menlo", "Cascadia Code", "Consolas", "DejaVu Sans Mono", monospace;
|
||||
font-size: 12px; background: #16181d; border: 1px solid {BORDER}; border-radius: 8px; }}
|
||||
font-size: 12px; background: {p.mono_bg}; border: 1px solid {p.border}; border-radius: 8px; }}
|
||||
QFrame#diagCard {{ background: transparent; border: none; }}
|
||||
QSplitter::handle {{ background: transparent; }}
|
||||
QSplitter::handle:hover {{ background: {BORDER}; border-radius: 4px; }}
|
||||
QSplitter::handle:pressed {{ background: {ACCENT}; border-radius: 4px; }}
|
||||
QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }}
|
||||
QSplitter::handle:pressed {{ background: {p.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)
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -1493,6 +1533,54 @@ 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.
|
||||
@@ -1550,6 +1638,11 @@ 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)
|
||||
@@ -1585,11 +1678,96 @@ 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()
|
||||
|
||||
@@ -1610,10 +1788,9 @@ 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())
|
||||
if core.is_newer_version(core.__version__, release["version"]):
|
||||
self.status.setText(
|
||||
f"Update available: {release['version']} · Help ▸ About to view it."
|
||||
)
|
||||
notice = core.update_notice(core.__version__, release)
|
||||
if notice:
|
||||
self._show_update_notice(notice)
|
||||
|
||||
# --- layout persistence ---------------------------------------------- #
|
||||
def _restore_layout(self):
|
||||
@@ -1939,9 +2116,21 @@ 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 = core.extract_servers(self.full_config)
|
||||
self.servers = servers
|
||||
self.dirty = False
|
||||
self.restart_btn.hide()
|
||||
self._undo_stack.clear()
|
||||
@@ -2264,7 +2453,7 @@ class MainWindow(QMainWindow):
|
||||
entry = self.servers[idx]
|
||||
old_name = entry.name
|
||||
entry.name = self.editor.current_name()
|
||||
entry.data = self.editor.dump_data()
|
||||
entry.set_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
|
||||
@@ -2358,7 +2547,7 @@ class MainWindow(QMainWindow):
|
||||
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
|
||||
)
|
||||
if ans == QMessageBox.StandardButton.Yes:
|
||||
self.servers[existing[name]].data = data
|
||||
self.servers[existing[name]].set_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))
|
||||
@@ -2478,6 +2667,11 @@ 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)
|
||||
@@ -2490,8 +2684,13 @@ 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}"
|
||||
f"Merged & saved {self.current_profile.path}{bnote}{cnote}"
|
||||
f" · Restart {self.current_profile.label} to apply."
|
||||
)
|
||||
self._offer_restart_button()
|
||||
@@ -2649,6 +2848,33 @@ 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
|
||||
@@ -2667,7 +2893,7 @@ def main():
|
||||
icon = _app_icon()
|
||||
if not icon.isNull():
|
||||
app.setWindowIcon(icon)
|
||||
app.setStyleSheet(STYLESHEET)
|
||||
app.setStyleSheet(theme_stylesheet_for(app))
|
||||
win = MainWindow()
|
||||
win.show()
|
||||
sys.exit(app.exec())
|
||||
|
||||
+283
-7
@@ -15,6 +15,7 @@ from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import contextlib
|
||||
import copy
|
||||
import difflib
|
||||
import functools
|
||||
import glob
|
||||
@@ -49,6 +50,12 @@ 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
|
||||
|
||||
@@ -163,6 +170,39 @@ 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
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -178,16 +218,195 @@ 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
|
||||
@@ -452,13 +671,30 @@ 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."""
|
||||
"""Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers.
|
||||
|
||||
Never raises on a structurally-odd config -- malformed entries come back as
|
||||
empty-data entries carrying their original value (see `_server_entry`).
|
||||
"""
|
||||
out: list[ServerEntry] = []
|
||||
for name, data in (cfg.get("mcpServers") or {}).items():
|
||||
out.append(ServerEntry(name=name, data=dict(data), enabled=True))
|
||||
out.append(_server_entry(name, data, True))
|
||||
for name, data in (cfg.get(DISABLED_KEY) or {}).items():
|
||||
out.append(ServerEntry(name=name, data=dict(data), enabled=False))
|
||||
out.append(_server_entry(name, data, False))
|
||||
return out
|
||||
|
||||
|
||||
@@ -550,8 +786,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.data for s in servers if s.enabled}
|
||||
disabled = {s.name: s.data for s in servers if not s.enabled}
|
||||
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}
|
||||
|
||||
cfg["mcpServers"] = enabled # replaces value if key existed; appends otherwise
|
||||
if disabled:
|
||||
@@ -564,6 +800,34 @@ 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)
|
||||
@@ -1380,9 +1644,21 @@ 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."""
|
||||
"""Concatenate lint_server warnings across every entry, in order.
|
||||
|
||||
Entries whose config value wasn't a JSON object at all are reported here
|
||||
rather than in lint_server, which takes an already-dict `data` (#72).
|
||||
"""
|
||||
out: list[str] = []
|
||||
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
|
||||
|
||||
@@ -2193,7 +2469,7 @@ CATALOG_DENIED_ENV_KEYS = frozenset(
|
||||
# still trust an older key: verify_catalog_signature() accepts a match
|
||||
# against ANY key in this list.
|
||||
CATALOG_PUBKEYS: list[bytes] = [
|
||||
base64.b64decode("0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="),
|
||||
base64.b64decode("082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="),
|
||||
]
|
||||
|
||||
# Domain-separation prefix for the signed message. The signature covers
|
||||
|
||||
+380
-869
File diff suppressed because it is too large
Load Diff
+1
-278
@@ -16,8 +16,6 @@ new surface" recurring-bug lesson).
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import hashlib
|
||||
import os
|
||||
import re
|
||||
from collections.abc import Callable
|
||||
@@ -444,43 +442,18 @@ class ReviewSession:
|
||||
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).
|
||||
|
||||
`reattest` marks a KEY-ROTATION re-attestation pass (issue #68 finding 5
|
||||
follow-up): the currently-trusted `bcc_core.CATALOG_PUBKEYS` key changed
|
||||
and the existing `data/catalog.json.sig` no longer verifies under it.
|
||||
Content-wise nothing may have changed -- `diff_catalogs(old, new)` can be
|
||||
genuinely empty -- but the NEW key has never vouched for any of this
|
||||
catalog before, so every entry needs a first-time attestation under the
|
||||
new key, not a diff against the old one. When `reattest` is set,
|
||||
`changes` is built as "every entry in `new_catalog`, presented as if
|
||||
newly added" (via `diff_catalogs(None, new_catalog)`) instead of a
|
||||
diff against `old_catalog`, so the acknowledge-gate in `can_sign()`
|
||||
requires re-reviewing everything the new key will sign -- which is the
|
||||
intended cost of a key rotation, not a bypass of the empty-diff guard.
|
||||
"""
|
||||
|
||||
pinned_blob_sha: str
|
||||
old_catalog: dict
|
||||
new_catalog: dict
|
||||
loaded_ref: str = "main"
|
||||
reattest: bool = False
|
||||
changes: list[EntryChange] = field(default_factory=list)
|
||||
acknowledged: set[str] = field(default_factory=set)
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
if not self.changes:
|
||||
if self.reattest:
|
||||
# Every entry in the new catalog is treated as though it
|
||||
# were newly added -- because, under the NEW signing key,
|
||||
# it is: nothing this key signs was ever attested by it
|
||||
# before. Reusing diff_catalogs(None, new) (rather than a
|
||||
# bespoke code path) means the same "added" risk predicate
|
||||
# (risk_new_entry) and the same EntryChange shape the rest
|
||||
# of this module and the GUI already know how to render
|
||||
# apply here unmodified.
|
||||
self.changes = diff_catalogs(None, self.new_catalog)
|
||||
else:
|
||||
self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
|
||||
self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
|
||||
|
||||
|
||||
def start_review(
|
||||
@@ -488,14 +461,12 @@ def start_review(
|
||||
old_catalog: dict,
|
||||
new_catalog: dict,
|
||||
loaded_ref: str = "main",
|
||||
reattest: bool = False,
|
||||
) -> ReviewSession:
|
||||
return ReviewSession(
|
||||
pinned_blob_sha=pinned_blob_sha,
|
||||
old_catalog=old_catalog,
|
||||
new_catalog=new_catalog,
|
||||
loaded_ref=loaded_ref,
|
||||
reattest=reattest,
|
||||
)
|
||||
|
||||
|
||||
@@ -560,14 +531,6 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
|
||||
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.)
|
||||
|
||||
This gate is unaffected by `session.reattest`: a key-rotation
|
||||
re-attestation session's `changes` is built from
|
||||
`diff_catalogs(None, new_catalog)` (see ReviewSession), which is
|
||||
empty ONLY if the catalog itself has zero entries -- a genuinely
|
||||
empty catalog either way. Rotation never manufactures a non-empty
|
||||
changeset out of an empty one; it just changes *what* "non-empty"
|
||||
is computed against.
|
||||
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing,
|
||||
from the ref that was actually reviewed -- see sign_precondition())
|
||||
must match the blob SHA pinned when review began. If the bytes on the
|
||||
@@ -853,243 +816,3 @@ _NON_ASCII_RE = re.compile(r"[^\x00-\x7f]")
|
||||
|
||||
def contains_non_ascii(s: str) -> bool:
|
||||
return bool(_NON_ASCII_RE.search(s))
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Key status reporting (issue #62/#68 follow-up: "make key handling
|
||||
# comprehensible"). Pure functions only -- `catalog_console.py cmd_keys` is a
|
||||
# thin printer that gathers inputs (local key caches, source-file text, the
|
||||
# catalog + its .sig) and hands them here. NEVER touches private key bytes:
|
||||
# every input/output here is a public key, a fingerprint, or a status string.
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
|
||||
def fingerprint_pubkey(pubkey: bytes) -> str:
|
||||
"""Short, human-comparable fingerprint of a raw Ed25519 public key: the
|
||||
first 16 hex chars of its SHA-256 digest, grouped in 4s (e.g. "3F2A 9C1B
|
||||
44DE 08AA") so two fingerprints can be eyeballed for a mismatch the way a
|
||||
PGP fingerprint is. Deliberately NOT the raw base64 pubkey itself in the
|
||||
default short form (that's available via the full committed value in the
|
||||
report) -- a fixed-width grouped hex string is easier to compare at a
|
||||
glance and to read aloud/type over chat if needed. Never derived from,
|
||||
and never printed alongside, any private key material.
|
||||
"""
|
||||
digest = hashlib.sha256(pubkey).hexdigest().upper()[:16]
|
||||
return " ".join(digest[i : i + 4] for i in range(0, len(digest), 4))
|
||||
|
||||
|
||||
_PUBKEY_LIST_B64_RE = re.compile(r'base64\.b64decode\(\s*"([^"]+)"\s*\)')
|
||||
|
||||
|
||||
def extract_pubkey_list_literal(source_text: str, var_name: str) -> list[bytes]:
|
||||
"""Best-effort extraction of a `<var_name>: list[bytes] = [...]` literal
|
||||
(each entry a `base64.b64decode("...")` call, matching the exact style
|
||||
bcc_core.CATALOG_PUBKEYS and scripts.sign_checksums.RELEASE_PUBKEYS are
|
||||
both written in) straight out of Python source TEXT.
|
||||
|
||||
Deliberately a regex over text, not an import: `catalog_console.py keys`
|
||||
must report on whatever ref/branch is checked out at the inspected repo
|
||||
path, which may not be (and need not be) importable from the running
|
||||
process's own sys.path. Returns [] if the variable isn't found in this
|
||||
exact shape -- callers treat that as "nothing committed here", not an
|
||||
error, since a report that can't parse a file should say so plainly
|
||||
rather than crash the whole `keys` command over one malformed file.
|
||||
"""
|
||||
match = re.search(
|
||||
rf"{re.escape(var_name)}\s*:\s*list\[bytes\]\s*=\s*\[(.*?)\]", source_text, re.DOTALL
|
||||
)
|
||||
if not match:
|
||||
return []
|
||||
keys: list[bytes] = []
|
||||
for b64 in _PUBKEY_LIST_B64_RE.findall(match.group(1)):
|
||||
try:
|
||||
keys.append(base64.b64decode(b64))
|
||||
except ValueError:
|
||||
continue
|
||||
return keys
|
||||
|
||||
|
||||
_CI_TRUST_ANCHOR_RE = re.compile(r'EXPECTED_CATALOG_PUBKEY_B64:\s*"([^"]+)"')
|
||||
|
||||
|
||||
def extract_ci_trust_anchor_pubkey(ci_yml_text: str) -> bytes | None:
|
||||
"""Best-effort extraction of ci.yml's `EXPECTED_CATALOG_PUBKEY_B64` trust
|
||||
anchor (issue #68 finding 4) from the workflow file's TEXT. Returns None
|
||||
if the constant isn't found -- the `keys` report shows that plainly
|
||||
("not found in ci.yml") rather than raising.
|
||||
"""
|
||||
match = _CI_TRUST_ANCHOR_RE.search(ci_yml_text)
|
||||
if not match:
|
||||
return None
|
||||
try:
|
||||
return base64.b64decode(match.group(1))
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class PubkeyLocationCheck:
|
||||
"""One place in the source tree a key's public half is expected to be
|
||||
committed, and whether the fingerprint(s) found there match the key
|
||||
stored locally."""
|
||||
|
||||
location: str
|
||||
committed_fingerprints: tuple[str, ...]
|
||||
status: str # "match" | "mismatch" | "unknown" (no local key to compare against)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class KeyStatus:
|
||||
"""Everything `catalog_console.py keys` reports about ONE signing key.
|
||||
Built by key_status() below; rendered by render_key_status_report().
|
||||
Never carries private key material -- every field here is safe to print.
|
||||
"""
|
||||
|
||||
kind: str # "catalog" | "release"
|
||||
display_name: str # "CATALOG" | "RELEASE"
|
||||
purpose: str # one-line plain-English purpose
|
||||
private_key_location: str # human-readable, e.g. "on this machine, in the OS keychain"
|
||||
local_exists: bool
|
||||
local_fingerprint: str | None
|
||||
locations: tuple[PubkeyLocationCheck, ...]
|
||||
catalog_sig_status: str | None = None # "valid" | "invalid" | "missing" | None (n/a)
|
||||
|
||||
|
||||
def key_status(
|
||||
kind: str,
|
||||
*,
|
||||
display_name: str,
|
||||
purpose: str,
|
||||
private_key_location: str,
|
||||
local_exists: bool,
|
||||
local_pubkey: bytes | None,
|
||||
locations: list[tuple[str, list[bytes]]],
|
||||
catalog_sig_status: str | None = None,
|
||||
) -> KeyStatus:
|
||||
"""Pure assembly of a KeyStatus from already-resolved inputs (no file or
|
||||
git I/O here -- that's catalog_console.py's job). `locations` is a list
|
||||
of (label, committed_pubkeys) pairs, e.g.
|
||||
[("bcc_core.CATALOG_PUBKEYS", [...]), ("ci.yml trust anchor", [...])],
|
||||
so a key can be checked against every place its public half is expected
|
||||
to be committed, independently -- this is the check that would have
|
||||
caught bcc_core.CATALOG_PUBKEYS and ci.yml's trust anchor silently
|
||||
drifting apart (issue #68 finding 4 was exactly that kind of drift).
|
||||
"""
|
||||
checks: list[PubkeyLocationCheck] = []
|
||||
for label, committed_pubkeys in locations:
|
||||
fps = tuple(fingerprint_pubkey(pk) for pk in committed_pubkeys)
|
||||
if local_pubkey is None:
|
||||
status = "unknown"
|
||||
elif local_pubkey in committed_pubkeys:
|
||||
status = "match"
|
||||
else:
|
||||
status = "mismatch"
|
||||
checks.append(
|
||||
PubkeyLocationCheck(location=label, committed_fingerprints=fps, status=status)
|
||||
)
|
||||
|
||||
return KeyStatus(
|
||||
kind=kind,
|
||||
display_name=display_name,
|
||||
purpose=purpose,
|
||||
private_key_location=private_key_location,
|
||||
local_exists=local_exists,
|
||||
local_fingerprint=fingerprint_pubkey(local_pubkey) if local_pubkey is not None else None,
|
||||
locations=tuple(checks),
|
||||
catalog_sig_status=catalog_sig_status,
|
||||
)
|
||||
|
||||
|
||||
_LOCATION_STATUS_ICON = {"match": "✅", "mismatch": "❌", "unknown": "⚠️"}
|
||||
_LOCATION_STATUS_VERDICT = {
|
||||
"match": "MATCHES the local private key",
|
||||
"mismatch": "DOES NOT MATCH the local private key",
|
||||
"unknown": "cannot compare -- no local key to check against",
|
||||
}
|
||||
_CATALOG_SIG_STATUS_LINE = {
|
||||
"valid": "✅ data/catalog.json.sig verifies under the committed CATALOG_PUBKEYS.",
|
||||
"invalid": (
|
||||
"❌ data/catalog.json.sig does NOT verify under the committed CATALOG_PUBKEYS -- "
|
||||
"the catalog needs re-signing (Load → acknowledge all → Sign)."
|
||||
),
|
||||
"missing": (
|
||||
"⚠️ data/catalog.json.sig is missing entirely -- the catalog has never been signed."
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
def recommend_next_steps(statuses: list[KeyStatus]) -> list[str]:
|
||||
"""The pure "what to do next" logic behind the keys report's closing
|
||||
section -- one concrete, runnable-looking instruction per problem found,
|
||||
naming the exact key involved (never just "the key"). Returns a single
|
||||
reassuring line if nothing needs attention."""
|
||||
steps: list[str] = []
|
||||
for s in statuses:
|
||||
if not s.local_exists:
|
||||
flag = " --release" if s.kind == "release" else ""
|
||||
steps.append(
|
||||
f"{s.display_name} key has never been generated on this machine -- run "
|
||||
f"`python catalog_console.py keygen{flag}`."
|
||||
)
|
||||
continue
|
||||
for loc in s.locations:
|
||||
if loc.status == "mismatch":
|
||||
steps.append(
|
||||
f"{s.display_name} key's local fingerprint does not match "
|
||||
f"{loc.location} -- update {loc.location} to the fingerprint shown "
|
||||
"above (or, if this is unexpected, treat the committed key as "
|
||||
"untrusted and investigate before doing anything else)."
|
||||
)
|
||||
elif loc.status == "unknown":
|
||||
steps.append(
|
||||
f"{s.display_name} key's local fingerprint could not be checked against "
|
||||
f"{loc.location} -- re-run keygen (or, for an older install, unlock the "
|
||||
"key once) so its public half is cached locally."
|
||||
)
|
||||
if s.kind == "catalog" and s.catalog_sig_status in ("invalid", "missing"):
|
||||
steps.append(
|
||||
"The catalog needs re-signing: run `python catalog_console.py gui --repo .` "
|
||||
"and Load → acknowledge every entry → Sign. If main is red because "
|
||||
"of a key rotation, load the branch with the rotation instead of main "
|
||||
"(current-branch / --ref source) so the fix lands before merge."
|
||||
)
|
||||
if not steps:
|
||||
steps.append("Everything is consistent -- no action needed.")
|
||||
return steps
|
||||
|
||||
|
||||
def render_key_status_report(statuses: list[KeyStatus]) -> str:
|
||||
"""Render a full, plain-English-first key status report as one string.
|
||||
`catalog_console.py cmd_keys` prints this verbatim -- the CLI is a thin
|
||||
printer over this pure function, which is what makes the report's
|
||||
content (not just its plumbing) unit-testable."""
|
||||
lines: list[str] = []
|
||||
for s in statuses:
|
||||
lines.append(f"=== {s.display_name} KEY ===")
|
||||
lines.append(s.purpose)
|
||||
lines.append(f"Private half lives: {s.private_key_location}")
|
||||
if s.local_exists and s.local_fingerprint:
|
||||
lines.append(f"Exists locally: yes (fingerprint {s.local_fingerprint})")
|
||||
elif s.local_exists:
|
||||
lines.append("Exists locally: yes (fingerprint unknown -- re-run keygen to cache it)")
|
||||
else:
|
||||
lines.append("Exists locally: no")
|
||||
for loc in s.locations:
|
||||
icon = _LOCATION_STATUS_ICON.get(loc.status, "?")
|
||||
fps = (
|
||||
", ".join(loc.committed_fingerprints)
|
||||
if loc.committed_fingerprints
|
||||
else "(nothing committed here)"
|
||||
)
|
||||
verdict = _LOCATION_STATUS_VERDICT.get(loc.status, loc.status)
|
||||
lines.append(f" {icon} {loc.location}: {fps} -- {verdict}")
|
||||
if s.catalog_sig_status is not None:
|
||||
lines.append(
|
||||
f"Catalog signature: {_CATALOG_SIG_STATUS_LINE.get(s.catalog_sig_status, s.catalog_sig_status)}"
|
||||
)
|
||||
lines.append("")
|
||||
|
||||
lines.append("What to do next:")
|
||||
for step in recommend_next_steps(statuses):
|
||||
lines.append(f" - {step}")
|
||||
return "\n".join(lines)
|
||||
|
||||
@@ -53,17 +53,13 @@ DOMAIN_PREFIX = b"bcc-release-v1|"
|
||||
# the signature on every past release: verification accepts a match against
|
||||
# ANY key here.
|
||||
#
|
||||
# Populated by the maintainer via:
|
||||
# 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
|
||||
# Rotated 2026-07 (issue #68 finding 5 / #68 CI-exposure incident): the
|
||||
# original key was shared with the catalog key and had been exposed to CI,
|
||||
# so both keypairs were regenerated as separate, disjoint keys. This list
|
||||
# holds only the current release key -- if release.yml's signing-smoke-test
|
||||
# ever sees this list empty, it fails closed (loudly) rather than silently
|
||||
# verifying against nothing.
|
||||
RELEASE_PUBKEYS: list[bytes] = [
|
||||
base64.b64decode("6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA="),
|
||||
]
|
||||
# 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
|
||||
|
||||
|
||||
@@ -1,215 +0,0 @@
|
||||
"""Tests for catalog_console.py's non-Qt git plumbing and ref-resolution
|
||||
seam (issue #68 rotation-completability fix).
|
||||
|
||||
catalog_console.py is importable here WITHOUT PySide6 -- its Qt import is
|
||||
guarded (`_PYSIDE6_AVAILABLE`) precisely so `keygen`, `show-seed-b64`,
|
||||
`keys`, and this git plumbing stay usable (and testable) wherever PySide6
|
||||
isn't installed, including this CI test job, which never installs it. If
|
||||
PySide6 genuinely isn't importable in this environment, that itself
|
||||
exercises the guard path -- see test_module_imports_without_pyside6.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
import catalog_console as cc
|
||||
import catalog_review as review
|
||||
|
||||
_SEED_CATALOG = b'{"schema": 1, "version": 1, "servers": []}'
|
||||
_SEED_SIG = b"\x00" * 64
|
||||
|
||||
|
||||
def _run(*args: str, cwd: Path) -> None:
|
||||
subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True)
|
||||
|
||||
|
||||
def _init_bare_and_clone(tmp_path: Path) -> tuple[Path, Path]:
|
||||
"""A bare "origin" repo with `main` and `rotation-branch` both seeded
|
||||
with a catalog + (dummy) signature, plus a working clone with `origin`
|
||||
already configured -- mirroring the tokened-remote clone
|
||||
catalog_console.py's git plumbing is always run against."""
|
||||
origin = tmp_path / "origin.git"
|
||||
_run("init", "--bare", str(origin), cwd=tmp_path)
|
||||
|
||||
seed = tmp_path / "seed"
|
||||
_run("clone", str(origin), str(seed), cwd=tmp_path)
|
||||
_run("config", "user.email", "test@example.com", cwd=seed)
|
||||
_run("config", "user.name", "Test", cwd=seed)
|
||||
|
||||
(seed / "data").mkdir()
|
||||
(seed / "data" / "catalog.json").write_bytes(_SEED_CATALOG)
|
||||
(seed / "data" / "catalog.json.sig").write_bytes(_SEED_SIG)
|
||||
_run("add", "-A", cwd=seed)
|
||||
_run("commit", "-m", "seed", cwd=seed)
|
||||
_run("push", "origin", "HEAD:refs/heads/main", cwd=seed)
|
||||
_run("checkout", "-b", "rotation-branch", cwd=seed)
|
||||
_run("push", "origin", "HEAD:refs/heads/rotation-branch", cwd=seed)
|
||||
|
||||
clone = tmp_path / "work"
|
||||
_run("clone", str(origin), str(clone), cwd=tmp_path)
|
||||
_run("config", "user.email", "test@example.com", cwd=clone)
|
||||
_run("config", "user.name", "Test", cwd=clone)
|
||||
return origin, clone
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# The module must stay importable without PySide6 -- this IS the fix that
|
||||
# lets `keys`/`keygen`/`show-seed-b64` (and this whole test file) run
|
||||
# somewhere PySide6 isn't installed.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_module_imports_without_pyside6():
|
||||
assert hasattr(cc, "_PYSIDE6_AVAILABLE")
|
||||
# This CI test job never installs PySide6 (see .github/workflows/ci.yml
|
||||
# "Install test dependencies": pytest + cryptography only) -- so on CI,
|
||||
# this assertion is itself proof the guard is doing its job. Locally,
|
||||
# where a maintainer's env DOES have PySide6, it's fine either way; the
|
||||
# only real assertion this test needs is "importing the module never
|
||||
# raises", which happened just by getting this far.
|
||||
assert cc._PYSIDE6_AVAILABLE in (True, False)
|
||||
|
||||
|
||||
def test_cmd_gui_fails_soft_without_pyside6(monkeypatch, capsys):
|
||||
if cc._PYSIDE6_AVAILABLE:
|
||||
return # nothing to prove where PySide6 IS available
|
||||
import argparse
|
||||
|
||||
args = argparse.Namespace(repo=".", ref=None)
|
||||
assert cc.cmd_gui(args) == 1
|
||||
assert "PySide6" in capsys.readouterr().err
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# compute_own_refs: the PURE ref-resolution seam. No git, no Qt.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_compute_own_refs_defaults_to_main_only():
|
||||
assert cc.compute_own_refs(None, None) == ["main"]
|
||||
|
||||
|
||||
def test_compute_own_refs_adds_detected_branch():
|
||||
assert cc.compute_own_refs(None, "chore/68-key-rotation") == [
|
||||
"main",
|
||||
"chore/68-key-rotation",
|
||||
]
|
||||
|
||||
|
||||
def test_compute_own_refs_explicit_ref_overrides_detected_branch():
|
||||
assert cc.compute_own_refs("explicit-branch", "detected-branch") == [
|
||||
"main",
|
||||
"explicit-branch",
|
||||
]
|
||||
|
||||
|
||||
def test_compute_own_refs_does_not_duplicate_main():
|
||||
assert cc.compute_own_refs(None, "main") == ["main"]
|
||||
assert cc.compute_own_refs("main", "some-other-branch") == ["main"]
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# current_branch: git plumbing, no Qt.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_current_branch_detects_checked_out_branch(tmp_path):
|
||||
_origin, clone = _init_bare_and_clone(tmp_path)
|
||||
_run("fetch", "origin", "rotation-branch", cwd=clone)
|
||||
_run("checkout", "-B", "rotation-branch", "origin/rotation-branch", cwd=clone)
|
||||
assert cc.current_branch(clone) == "rotation-branch"
|
||||
|
||||
|
||||
def test_current_branch_none_on_detached_head(tmp_path):
|
||||
_origin, clone = _init_bare_and_clone(tmp_path)
|
||||
commit = cc.fetch_ref(clone, "main")
|
||||
_run("checkout", commit, cwd=clone)
|
||||
assert cc.current_branch(clone) is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# commit_and_push_signed_catalog: MUST target the given branch, never a
|
||||
# hardcoded "main" -- issue #68's completability fix. This is exactly the
|
||||
# bug that, before the fix, would have made ReviewWindow._on_sign push a
|
||||
# PR/branch review's signature straight to main regardless of what was
|
||||
# actually reviewed.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_commit_and_push_signed_catalog_targets_the_given_branch_not_main(tmp_path):
|
||||
_origin, clone = _init_bare_and_clone(tmp_path)
|
||||
|
||||
new_raw = b'{"schema": 1, "version": 2, "servers": []}'
|
||||
new_sig = b"\x01" * 64
|
||||
cc.commit_and_push_signed_catalog(clone, new_raw, new_sig, branch="rotation-branch")
|
||||
|
||||
rotation_commit = cc.fetch_ref(clone, "rotation-branch")
|
||||
rotation_raw, _sha = cc.read_catalog_at_commit(clone, rotation_commit)
|
||||
assert rotation_raw == new_raw
|
||||
|
||||
# main on the shared origin must be COMPLETELY untouched by a sign that
|
||||
# was reviewed and pushed against rotation-branch.
|
||||
main_commit = cc.fetch_ref(clone, "main")
|
||||
main_raw, _sha = cc.read_catalog_at_commit(clone, main_commit)
|
||||
assert main_raw == _SEED_CATALOG
|
||||
|
||||
|
||||
def test_commit_and_push_signed_catalog_still_defaults_to_main(tmp_path):
|
||||
"""Backward-compatible default: callers that don't pass `branch` (there
|
||||
are none left in catalog_console.py itself, but the signature keeps the
|
||||
default for any other caller / test fixture) still push to main."""
|
||||
_origin, clone = _init_bare_and_clone(tmp_path)
|
||||
|
||||
new_raw = b'{"schema": 1, "version": 2, "servers": []}'
|
||||
new_sig = b"\x01" * 64
|
||||
cc.commit_and_push_signed_catalog(clone, new_raw, new_sig)
|
||||
|
||||
main_commit = cc.fetch_ref(clone, "main")
|
||||
main_raw, _sha = cc.read_catalog_at_commit(clone, main_commit)
|
||||
assert main_raw == new_raw
|
||||
|
||||
rotation_commit = cc.fetch_ref(clone, "rotation-branch")
|
||||
rotation_raw, _sha = cc.read_catalog_at_commit(clone, rotation_commit)
|
||||
assert rotation_raw == _SEED_CATALOG # untouched
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# catalog_sig_status_on_disk: the check behind `keys`' "does catalog.json.sig
|
||||
# currently verify?" line -- this is precisely the check that would have
|
||||
# caught the current chore/68-key-rotation state (bcc_core.CATALOG_PUBKEYS
|
||||
# rotated, data/catalog.json.sig still signed by the retired key).
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_catalog_sig_status_on_disk_valid(tmp_path):
|
||||
seed, pub = review.generate_keypair()
|
||||
raw = b'{"schema": 1, "version": 1, "servers": []}'
|
||||
sig = review.sign_catalog_bytes(raw, seed)
|
||||
|
||||
(tmp_path / "data").mkdir()
|
||||
(tmp_path / "data" / "catalog.json").write_bytes(raw)
|
||||
(tmp_path / "data" / "catalog.json.sig").write_bytes(sig)
|
||||
|
||||
assert cc.catalog_sig_status_on_disk(tmp_path, [pub]) == "valid"
|
||||
|
||||
|
||||
def test_catalog_sig_status_on_disk_invalid_when_pubkey_rotated(tmp_path):
|
||||
"""The exact chore/68-key-rotation scenario: signed by an OLD key, but
|
||||
the committed pubkey list now only has the NEW key."""
|
||||
old_seed, _old_pub = review.generate_keypair()
|
||||
_new_seed, new_pub = review.generate_keypair()
|
||||
raw = b'{"schema": 1, "version": 1, "servers": []}'
|
||||
sig = review.sign_catalog_bytes(raw, old_seed)
|
||||
|
||||
(tmp_path / "data").mkdir()
|
||||
(tmp_path / "data" / "catalog.json").write_bytes(raw)
|
||||
(tmp_path / "data" / "catalog.json.sig").write_bytes(sig)
|
||||
|
||||
assert cc.catalog_sig_status_on_disk(tmp_path, [new_pub]) == "invalid"
|
||||
|
||||
|
||||
def test_catalog_sig_status_on_disk_missing_when_no_sig_file(tmp_path):
|
||||
(tmp_path / "data").mkdir()
|
||||
(tmp_path / "data" / "catalog.json").write_bytes(b"{}")
|
||||
assert cc.catalog_sig_status_on_disk(tmp_path, []) == "missing"
|
||||
|
||||
|
||||
def test_catalog_sig_status_on_disk_missing_when_no_catalog_file(tmp_path):
|
||||
(tmp_path / "data").mkdir()
|
||||
(tmp_path / "data" / "catalog.json.sig").write_bytes(b"\x00" * 64)
|
||||
assert cc.catalog_sig_status_on_disk(tmp_path, []) == "missing"
|
||||
@@ -491,98 +491,6 @@ def test_sign_precondition_defaults_to_main_when_loaded_ref_unset():
|
||||
assert decision.ok is True
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# ReviewSession(reattest=True): key-rotation re-attestation (issue #68
|
||||
# finding 5 follow-up). After rotating bcc_core.CATALOG_PUBKEYS, the
|
||||
# existing data/catalog.json.sig no longer verifies under the new key even
|
||||
# though catalog CONTENT is unchanged -- diff_catalogs(old, new) would be
|
||||
# empty, and an empty changeset must never unlock Sign (can_sign gate 0).
|
||||
# Re-attestation mode sidesteps that correctly: instead of diffing against
|
||||
# the (now-untrustworthy) last-signed content, it treats every entry as
|
||||
# requiring a fresh acknowledgement, exactly like a brand-new catalog.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_reattest_mode_with_unchanged_content_yields_one_change_per_entry():
|
||||
same = _catalog(_entry(id="a"), _entry(id="b"), _entry(id="c"))
|
||||
session = r.start_review("sha1", same, same, reattest=True)
|
||||
assert len(session.changes) == 3
|
||||
assert {c_.entry_id for c_ in session.changes} == {"a", "b", "c"}
|
||||
# Presented "as if newly added" -- old_catalog plays no role here.
|
||||
assert all(c_.status == "added" for c_ in session.changes)
|
||||
assert all(c_.old is None for c_ in session.changes)
|
||||
|
||||
|
||||
def test_reattest_mode_can_sign_refuses_until_all_acknowledged_then_permits():
|
||||
same = _catalog(_entry(id="a"), _entry(id="b"), _entry(id="c"))
|
||||
session = r.start_review("sha1", same, same, reattest=True)
|
||||
|
||||
decision = r.can_sign(session, "sha1")
|
||||
assert decision.ok is False
|
||||
assert "acknowledged" in decision.reason.lower()
|
||||
|
||||
r.acknowledge_entry(session, "a")
|
||||
r.acknowledge_entry(session, "b")
|
||||
decision = r.can_sign(session, "sha1")
|
||||
assert decision.ok is False # "c" still outstanding
|
||||
assert "acknowledged" in decision.reason.lower()
|
||||
|
||||
r.acknowledge_entry(session, "c")
|
||||
decision = r.can_sign(session, "sha1")
|
||||
assert decision.ok is True
|
||||
assert decision.reason is None
|
||||
|
||||
|
||||
def test_normal_mode_with_unchanged_content_still_refuses_empty_diff():
|
||||
"""The rotation path must NOT become a general bypass of the empty-diff
|
||||
guard: reattest=False (the default) against identical old/new catalogs
|
||||
must behave exactly as before -- can_sign refuses with 'nothing to
|
||||
sign', full stop."""
|
||||
same = _catalog(_entry(id="a"), _entry(id="b"))
|
||||
session = r.start_review("sha1", same, same) # reattest defaults False
|
||||
assert session.changes == []
|
||||
decision = r.can_sign(session, "sha1")
|
||||
assert decision.ok is False
|
||||
assert "nothing to sign" in decision.reason.lower()
|
||||
|
||||
|
||||
def test_reattest_mode_still_enforces_blocking_risk_check():
|
||||
"""Re-attestation must not relax the blocking-risk gate: a
|
||||
disallowed-command entry blocks Sign even with every entry
|
||||
acknowledged."""
|
||||
cat = _catalog(
|
||||
_entry(id="a"),
|
||||
_entry(id="evil", config={"command": "bash", "args": ["-c", "rm -rf /"]}),
|
||||
)
|
||||
session = r.start_review("sha1", cat, cat, reattest=True)
|
||||
assert len(session.changes) == 2
|
||||
r.acknowledge_entry(session, "a")
|
||||
r.acknowledge_entry(session, "evil")
|
||||
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()
|
||||
|
||||
|
||||
def test_reattest_mode_still_enforces_toctou_pin():
|
||||
"""Re-attestation must not relax the TOCTOU blob-SHA pin: acknowledging
|
||||
everything is not enough if the bytes moved underneath the review."""
|
||||
same = _catalog(_entry(id="a"))
|
||||
session = r.start_review("sha1", same, same, reattest=True)
|
||||
r.acknowledge_entry(session, "a")
|
||||
decision = r.can_sign(session, "sha1")
|
||||
assert decision.ok is True # sanity: matches when blob is unchanged
|
||||
|
||||
decision = r.can_sign(session, "sha2-a-new-commit-landed")
|
||||
assert decision.ok is False
|
||||
assert "mismatch" in decision.reason.lower() or "changed" in decision.reason.lower()
|
||||
|
||||
|
||||
def test_reattest_defaults_to_false():
|
||||
"""start_review()'s reattest parameter defaults to False -- normal
|
||||
(diff-based) review is the default behaviour, never silently entered."""
|
||||
session = r.start_review("sha1", _catalog(), _catalog(_entry()))
|
||||
assert session.reattest is False
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# find_last_signed_catalog_raw: what source=main diffs against
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -785,245 +693,3 @@ def test_contains_non_ascii_true():
|
||||
|
||||
def test_contains_non_ascii_false():
|
||||
assert r.contains_non_ascii("package") is False
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# fingerprint_pubkey
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_fingerprint_pubkey_is_deterministic():
|
||||
pub = b"\x01" * 32
|
||||
assert r.fingerprint_pubkey(pub) == r.fingerprint_pubkey(pub)
|
||||
|
||||
|
||||
def test_fingerprint_pubkey_differs_for_different_keys():
|
||||
assert r.fingerprint_pubkey(b"\x01" * 32) != r.fingerprint_pubkey(b"\x02" * 32)
|
||||
|
||||
|
||||
def test_fingerprint_pubkey_never_contains_the_key_bytes_themselves():
|
||||
pub = b"\x42" * 32
|
||||
fp = r.fingerprint_pubkey(pub)
|
||||
assert pub.hex() not in fp.lower().replace(" ", "")
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# extract_pubkey_list_literal / extract_ci_trust_anchor_pubkey: text parsing
|
||||
# for `catalog_console.py keys`, exercised here with no file I/O.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_extract_pubkey_list_literal_single_key():
|
||||
_seed, pub = r.generate_keypair()
|
||||
import base64
|
||||
|
||||
text = (
|
||||
"CATALOG_PUBKEYS: list[bytes] = [\n"
|
||||
f' base64.b64decode("{base64.b64encode(pub).decode()}"),\n'
|
||||
"]\n"
|
||||
)
|
||||
assert r.extract_pubkey_list_literal(text, "CATALOG_PUBKEYS") == [pub]
|
||||
|
||||
|
||||
def test_extract_pubkey_list_literal_multiple_keys():
|
||||
import base64
|
||||
|
||||
pubs = [r.generate_keypair()[1] for _ in range(2)]
|
||||
body = ",\n".join(f' base64.b64decode("{base64.b64encode(p).decode()}")' for p in pubs)
|
||||
text = f"RELEASE_PUBKEYS: list[bytes] = [\n{body},\n]\n"
|
||||
assert r.extract_pubkey_list_literal(text, "RELEASE_PUBKEYS") == pubs
|
||||
|
||||
|
||||
def test_extract_pubkey_list_literal_missing_variable_returns_empty():
|
||||
assert r.extract_pubkey_list_literal("some unrelated text", "CATALOG_PUBKEYS") == []
|
||||
|
||||
|
||||
def test_extract_pubkey_list_literal_does_not_match_a_different_variable():
|
||||
import base64
|
||||
|
||||
_seed, pub = r.generate_keypair()
|
||||
text = f'OTHER_PUBKEYS: list[bytes] = [base64.b64decode("{base64.b64encode(pub).decode()}")]\n'
|
||||
assert r.extract_pubkey_list_literal(text, "CATALOG_PUBKEYS") == []
|
||||
|
||||
|
||||
def test_extract_ci_trust_anchor_pubkey_found():
|
||||
import base64
|
||||
|
||||
_seed, pub = r.generate_keypair()
|
||||
text = f' EXPECTED_CATALOG_PUBKEY_B64: "{base64.b64encode(pub).decode()}"\n'
|
||||
assert r.extract_ci_trust_anchor_pubkey(text) == pub
|
||||
|
||||
|
||||
def test_extract_ci_trust_anchor_pubkey_missing_returns_none():
|
||||
assert r.extract_ci_trust_anchor_pubkey("no anchor here") is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# key_status / render_key_status_report / recommend_next_steps
|
||||
# --------------------------------------------------------------------------- #
|
||||
def _kw(**overrides):
|
||||
base = dict(
|
||||
kind="catalog",
|
||||
display_name="CATALOG",
|
||||
purpose="Signs the catalog.",
|
||||
private_key_location="on this machine",
|
||||
local_exists=True,
|
||||
local_pubkey=b"\x01" * 32,
|
||||
locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x01" * 32])],
|
||||
catalog_sig_status="valid",
|
||||
)
|
||||
base.update(overrides)
|
||||
return base
|
||||
|
||||
|
||||
def test_key_status_reports_match_when_local_pubkey_in_committed_list():
|
||||
status = r.key_status(**_kw())
|
||||
assert status.locations[0].status == "match"
|
||||
|
||||
|
||||
def test_key_status_reports_mismatch_when_local_pubkey_not_in_committed_list():
|
||||
status = r.key_status(**_kw(locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x02" * 32])]))
|
||||
assert status.locations[0].status == "mismatch"
|
||||
|
||||
|
||||
def test_key_status_reports_unknown_when_no_local_pubkey():
|
||||
status = r.key_status(**_kw(local_pubkey=None, local_exists=False))
|
||||
assert status.locations[0].status == "unknown"
|
||||
assert status.local_fingerprint is None
|
||||
|
||||
|
||||
def test_key_status_never_carries_a_local_fingerprint_when_key_absent():
|
||||
status = r.key_status(**_kw(local_pubkey=None, local_exists=False))
|
||||
assert status.local_exists is False
|
||||
assert status.local_fingerprint is None
|
||||
|
||||
|
||||
def test_key_status_fingerprint_matches_fingerprint_pubkey_helper():
|
||||
pub = b"\x03" * 32
|
||||
status = r.key_status(**_kw(local_pubkey=pub, locations=[("x", [pub])]))
|
||||
assert status.local_fingerprint == r.fingerprint_pubkey(pub)
|
||||
|
||||
|
||||
def test_key_status_checks_multiple_locations_independently():
|
||||
"""A key can match one committed location and mismatch another -- this
|
||||
is exactly the drift issue #68 finding 4 was about (bcc_core.py and
|
||||
ci.yml silently disagreeing on the trust anchor)."""
|
||||
pub = b"\x04" * 32
|
||||
other = b"\x05" * 32
|
||||
status = r.key_status(
|
||||
**_kw(
|
||||
local_pubkey=pub,
|
||||
locations=[
|
||||
("bcc_core.CATALOG_PUBKEYS", [pub]),
|
||||
("ci.yml trust anchor", [other]),
|
||||
],
|
||||
)
|
||||
)
|
||||
assert status.locations[0].status == "match"
|
||||
assert status.locations[1].status == "mismatch"
|
||||
|
||||
|
||||
def test_recommend_next_steps_flags_never_generated_key():
|
||||
status = r.key_status(**_kw(local_exists=False, local_pubkey=None, locations=[]))
|
||||
steps = r.recommend_next_steps([status])
|
||||
assert any("keygen" in s and "CATALOG" in s for s in steps)
|
||||
|
||||
|
||||
def test_recommend_next_steps_release_key_uses_release_flag():
|
||||
status = r.key_status(
|
||||
kind="release",
|
||||
display_name="RELEASE",
|
||||
purpose="Signs checksums.",
|
||||
private_key_location="not generated yet",
|
||||
local_exists=False,
|
||||
local_pubkey=None,
|
||||
locations=[],
|
||||
)
|
||||
steps = r.recommend_next_steps([status])
|
||||
assert any("keygen --release" in s for s in steps)
|
||||
|
||||
|
||||
def test_recommend_next_steps_flags_mismatch_by_location_name():
|
||||
status = r.key_status(**_kw(locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x99" * 32])]))
|
||||
steps = r.recommend_next_steps([status])
|
||||
assert any("bcc_core.CATALOG_PUBKEYS" in s and "CATALOG" in s for s in steps)
|
||||
|
||||
|
||||
def test_recommend_next_steps_flags_invalid_catalog_signature():
|
||||
status = r.key_status(**_kw(catalog_sig_status="invalid"))
|
||||
steps = r.recommend_next_steps([status])
|
||||
assert any("re-signing" in s for s in steps)
|
||||
|
||||
|
||||
def test_recommend_next_steps_flags_missing_catalog_signature():
|
||||
status = r.key_status(**_kw(catalog_sig_status="missing"))
|
||||
steps = r.recommend_next_steps([status])
|
||||
assert any("re-signing" in s for s in steps)
|
||||
|
||||
|
||||
def test_recommend_next_steps_all_clear_when_nothing_wrong():
|
||||
status = r.key_status(**_kw())
|
||||
steps = r.recommend_next_steps([status])
|
||||
assert steps == ["Everything is consistent -- no action needed."]
|
||||
|
||||
|
||||
def test_recommend_next_steps_release_key_has_no_catalog_signature_advice():
|
||||
"""A mismatched RELEASE key must never trigger catalog-signing advice --
|
||||
the two keys' remediation paths must not bleed into each other."""
|
||||
status = r.key_status(
|
||||
kind="release",
|
||||
display_name="RELEASE",
|
||||
purpose="Signs checksums.",
|
||||
private_key_location="on this machine",
|
||||
local_exists=True,
|
||||
local_pubkey=b"\x06" * 32,
|
||||
locations=[("scripts/sign_checksums.py RELEASE_PUBKEYS", [b"\x07" * 32])],
|
||||
catalog_sig_status=None,
|
||||
)
|
||||
steps = r.recommend_next_steps([status])
|
||||
assert not any("re-signing" in s for s in steps)
|
||||
assert any("RELEASE" in s for s in steps)
|
||||
|
||||
|
||||
def test_render_key_status_report_never_prints_private_key_material():
|
||||
"""The report string must be built ONLY from public inputs. Sanity
|
||||
check: no field on KeyStatus/PubkeyLocationCheck is capable of holding
|
||||
private key bytes in the first place (there's no such field to leak),
|
||||
and the render function only touches fields that exist -- this test
|
||||
guards against a future field addition reintroducing that risk."""
|
||||
status = r.key_status(**_kw())
|
||||
text = r.render_key_status_report([status])
|
||||
assert "CATALOG" in text
|
||||
assert (
|
||||
"purpose" not in text.lower() or "Signs the catalog." in text
|
||||
) # sanity, not a real secret
|
||||
# No 64-hex-char (or longer) run anywhere -- a raw 32-byte seed/sig
|
||||
# would show up as one if it were ever accidentally interpolated in.
|
||||
import re as _re
|
||||
|
||||
assert not _re.search(r"[0-9a-fA-F]{64,}", text)
|
||||
|
||||
|
||||
def test_render_key_status_report_names_the_specific_key_not_generic_the_key():
|
||||
status = r.key_status(**_kw())
|
||||
text = r.render_key_status_report([status])
|
||||
assert "CATALOG KEY" in text
|
||||
assert "the key" not in text.lower()
|
||||
|
||||
|
||||
def test_render_key_status_report_includes_catalog_signature_line_only_for_catalog():
|
||||
catalog_status = r.key_status(**_kw())
|
||||
release_status = r.key_status(
|
||||
kind="release",
|
||||
display_name="RELEASE",
|
||||
purpose="Signs checksums.",
|
||||
private_key_location="on this machine",
|
||||
local_exists=True,
|
||||
local_pubkey=b"\x08" * 32,
|
||||
locations=[("scripts/sign_checksums.py RELEASE_PUBKEYS", [b"\x08" * 32])],
|
||||
catalog_sig_status=None,
|
||||
)
|
||||
text = r.render_key_status_report([catalog_status, release_status])
|
||||
assert text.count("Catalog signature:") == 1
|
||||
|
||||
|
||||
def test_render_key_status_report_ends_with_what_to_do_next_section():
|
||||
status = r.key_status(**_kw())
|
||||
text = r.render_key_status_report([status])
|
||||
assert "What to do next:" in text
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""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
|
||||
@@ -2370,3 +2371,312 @@ def test_config_has_unfilled_placeholders_false_after_fill():
|
||||
def test_config_has_unfilled_placeholders_checks_env_too():
|
||||
cfg = {"command": "uvx", "args": ["mcp-grafana"], "env": {"GRAFANA_URL": "<GRAFANA_URL>"}}
|
||||
assert c.config_has_unfilled_placeholders(cfg) is True
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #72 -- a server value that isn't a JSON object must not take the load down
|
||||
# --------------------------------------------------------------------------- #
|
||||
@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
|
||||
# --------------------------------------------------------------------------- #
|
||||
@pytest.mark.parametrize(
|
||||
"setting,system_dark,expected",
|
||||
[
|
||||
(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"),
|
||||
],
|
||||
)
|
||||
def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected):
|
||||
assert c.resolve_theme(setting, system_dark) == expected
|
||||
|
||||
|
||||
@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_palette_for_known_names():
|
||||
assert c.palette_for("dark") is c.DARK_PALETTE
|
||||
assert c.palette_for("light") is c.LIGHT_PALETTE
|
||||
|
||||
|
||||
def test_palette_for_unknown_name_falls_back_to_dark():
|
||||
assert c.palette_for("chartreuse") is c.DARK_PALETTE
|
||||
|
||||
|
||||
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")
|
||||
|
||||
|
||||
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}"
|
||||
|
||||
|
||||
@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
|
||||
|
||||
|
||||
@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_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_relative_luminance_extremes():
|
||||
assert c.relative_luminance("#000000") == pytest.approx(0.0)
|
||||
assert c.relative_luminance("#ffffff") == pytest.approx(1.0)
|
||||
|
||||
|
||||
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_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"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #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_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
|
||||
|
||||
|
||||
@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_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_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_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_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"
|
||||
|
||||
Reference in New Issue
Block a user