14 Commits

Author SHA1 Message Date
the_og a73f2e3883 feat: author ${VAR} references, gated on whether the client expands them (#76)
CI / Lint (ruff) (pull_request) Successful in 9s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 12s
CI / Catalog signature (pull_request) Successful in 7s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
The blocker on this issue was whether BCC or the client does the expanding.
Answer, from Anthropic's docs: Claude Code expands ${VAR} and
${VAR:-default} itself, in command, args, env, url and headers, for both
project .mcp.json and user-scope ~/.claude.json. Claude Desktop has no
documented support.

So this is a per-client capability, not a global one, and BCC does NOT
expand on write: resolving a reference into the file would put the secret
back on disk -- the whole thing the user is avoiding -- and would defeat a
feature the client already implements correctly. BCC authors, validates and
warns; expand_env_refs exists to preview what the client will do.

Semantics mirror the documented ones exactly, including the unusual bit:
an unset variable with no default is left as literal ${VAR} text rather
than blanked, because that is what Claude Code passes through.

Gating uses the existing profile_targets_claude_desktop(), so a config that
is correct under Claude Code and broken under Desktop is reported against
whichever profile is actually loaded. The two warnings are worded
differently on purpose -- 'this client will never expand these' is a
different problem from 'this variable looks unset here'.

Two existing behaviours were backwards for this feature and are fixed:

- Secret masking hid placeholders. is_secret_key('API_KEY') is true, so
  ${API_KEY} rendered as dots -- making a reference indistinguishable from
  a stored credential, which is the one distinction that makes the feature
  worth adopting. should_mask_value() now skips references, in the table
  delegate, _redact_server_data and redact_args alike.
- args_secret_warning fired on placeholders. Moving a token into ${VAR} is
  the recommended fix for that warning; continuing to warn punished the
  fix. It now skips references while still flagging a real secret that
  follows one.

Real secrets are still masked everywhere they were before -- asserted, not
assumed.

Refs #76
2026-07-20 13:46:14 -04:00
the_og 7ff4f6e5c0 Merge PR #81: make the update checker visible — persistent banner + Help menu item (#78, #79)
CI / Lint (ruff) (push) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 10s
CI / Catalog signature (push) Successful in 6s
CI / Tests (py3.12 / windows-latest) (push) Has been cancelled
2026-07-20 12:52:48 -04:00
the_og 7517e16b15 Merge branch 'main' into fix/78-79-update-visibility
CI / Lint (ruff) (pull_request) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 6s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Three conflicts, two of them semantic rather than textual:

- bcc.py QSS: this branch added the noticeBanner rules using the old
  module-level constants ({MUTED}, {ACCENT}); main had since moved the
  stylesheet onto palette slots ({p.muted}). Took main's form and
  translated the notice rules into it -- picking either side wholesale
  would have either dropped the banner styling or reintroduced globals
  that test_stylesheet_builder_has_no_hardcoded_colours now forbids.
- bcc.py methods: both sides appended to MainWindow (update-notice
  handlers vs theme handlers). Additive, kept both.
- tests/test_core.py: the usual EOF append. Kept both blocks.

_build_menu_bar auto-merged cleanly (View menu above, Help menu below);
verified both are present with their menu roles intact.

Verified: 272 test functions = 265 (main) + 7 (this branch), no
duplicates; 421 passed, ruff clean.
2026-07-20 12:51:55 -04:00
the_og 9a0433225e Merge PR #80: light theme + system-following, dark preserved exactly (#75)
CI / Lint (ruff) (push) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 10s
CI / Catalog signature (push) Successful in 7s
CI / Tests (py3.12 / windows-latest) (push) Has been cancelled
2026-07-20 12:51:01 -04:00
the_og fa82d30087 Merge branch 'main' into feat/75-theming
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 7s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Union conflict at the end of tests/test_core.py -- both branches appended a
test block. Kept both, main's #72/#73 block first. Verified: 265 test
functions = 238 baseline + 15 (#77) + 12 (theming), no duplicates.
2026-07-20 12:50:09 -04:00
the_og 05b00a40c0 Merge PR #77: tolerate non-object server values; keep named sets across a merge (#72, #73)
CI / Lint (ruff) (push) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 11s
CI / Catalog signature (push) Successful in 7s
CI / Tests (py3.12 / windows-latest) (push) Has been cancelled
2026-07-20 12:49:23 -04:00
the_og 3068e74e5c fix: make the update checker visible -- persistent banner + a menu item
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 6s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Reported from the field: running v1.2 against a repo with v1.3.0 published
gave no prompt, and there appeared to be no way to check manually. The
checker itself works; it was invisible, for two reasons.

#78 -- the notice was written to the shared status label, which 21 other
call sites rewrite. The check runs off-thread and lands a second or two
after launch, right as the user starts clicking, so the next selection or
refresh wiped it. Exactly the bug fixed for the MSIX warning in #35, which
got a persistent banner; that fix was never carried to the update notice.

Adds NoticeBanner: a persistent, dismissible notice carrying its own action
button. It's a shared widget rather than a second bespoke banner, so the
next thing needing the user's attention doesn't reach for the status bar
again. (The MSIX banner still uses its own QLabel -- migrating it is a
follow-up, deliberately not bundled with a bug fix.)

#79 -- the only 'Check for updates' affordance was a button inside the
About dialog, which is not where anyone looks. Worse, the About action was
created without a menu role, and Qt auto-assigns AboutRole to actions whose
text begins with 'About', relocating it into the macOS application menu --
so the notice's own hint, 'Help > About to view it', pointed at a menu that
on macOS doesn't contain the item.

Help now has its own 'Check for updates...' item with an explicit
ApplicationSpecificRole, and the About action states its AboutRole rather
than inheriting it invisibly. The menu-driven check is never throttled and
always reports back -- the user asked, so silence would read as broken.

The decision and the wording live in core.update_notice() because the test
suite has no PySide6 (CI installs pytest + cryptography only), so anything
in bcc.py is untestable. A test asserts the notice text names no menu path,
which is what went stale here in the first place.

Closes #78
Closes #79
2026-07-20 12:29:37 -04:00
the_og febd617c56 feat: light theme + system-following, with the dark theme preserved exactly
CI / Lint (ruff) (pull_request) Successful in 8s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 12s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 11s
CI / Catalog signature (pull_request) Successful in 6s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
BCC has always been dark-only -- BG #1b1d23, hardcoded at import time, with
no light option and no awareness of the desktop's appearance. On a light
desktop it matches nothing else on screen and there was no way to change it.

Adds a Palette value type in bcc_core with DARK (byte-identical to the
colours v1.3.0 shipped) and a new LIGHT, plus resolve_theme(setting,
system_is_dark) so the decision is testable without a Qt app. View > Theme
offers Match system / Light / Dark, persisted in QSettings under ui/theme,
defaulting to following the system.

The light palette's semantic colours are deliberately not the dark ones
lightened: #4ade80 sits near 1.7:1 against white. They are darkened to clear
WCAG AA, and a contrast test enforces >= 4.5:1 for every text colour against
its surface in both palettes so nobody harmonises them back later.

Three near-black literals were baked into the stylesheet (#1a1205 on-accent
text, #202229 disabled table, #16181d diagnostics pane). Fine with one theme,
invisible breakage with two -- each now has a palette slot, and a test
asserts build_stylesheet contains no hex literals at all.

The ~20 inline setStyleSheet(f"color: {MUTED}") call sites are left alone:
apply_palette rebinds the module-level colour names, and an f-string resolves
its names when it runs, so each call site picks up the new colour on its next
render. Switching theme reapplies the global QSS and re-renders the
inline-styled widgets, so nothing is left dark-on-light.

Refs #75
2026-07-20 12:22:38 -04:00
the_og da20eb2fdb fix: tolerate non-object server values on load; keep named sets across a merge
CI / Lint (ruff) (pull_request) Successful in 13s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 12s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 12s
CI / Catalog signature (pull_request) Successful in 8s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Two silent-failure bugs found auditing the v1.3.0 features.

#72 -- extract_servers called dict() on every server value, so a config
that was valid JSON but held a non-object server ("foo": "oops", a
number, a list, null) raised on load. The strict parse succeeded, so the
repair path never saw it, and the call sat outside the load try/except:
an unhandled traceback with the window half-swapped to the new profile.
It also made the #54 schema lint unreachable for the most likely
hand-edit mistake -- the load died before the linter ran.

Malformed values are now preserved verbatim on ServerEntry.raw (behind a
NO_RAW sentinel, since a literal JSON null is itself a malformed entry
worth keeping) and written back untouched on Save, so nothing is silently
deleted. lint_servers names the offending entry instead.

#73 -- the stale-file "Merge & save" path reloaded the file from disk and
re-applied the user's servers, but apply_servers only writes mcpServers
and _disabledMcpServers. Named server sets live under _bccServerSets in
the same file, so a set saved that session was dropped from disk and then
from memory, with no warning, on the path the user picks because it
sounds like the safe one.

BCC-owned keys are now declared in BCC_OWNED_KEYS and carried across by
carry_owned_keys, which reports genuinely contested keys so the status
line can say so. Deliberately one-directional: a key absent locally is
left alone on disk, because 'user deleted their last set' and 'another
machine just added sets' are indistinguishable and deleting someone
else's data is the worse failure.

Closes #72
Closes #73
2026-07-20 12:11:49 -04:00
the_og cd2ac2f6f8 Merge PR #69: make the review gate load-bearing, split the keys (#68)
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.12 / windows-latest) (push) Successful in 24s
CI / Lint (ruff) (push) Successful in 6s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 11s
CI / Catalog signature (push) Successful in 7s
Finding 1: can_sign() returned True on an empty changeset, and _on_sign
compared the reviewed blob against a hardcoded "main" rather than the ref
actually reviewed — so the PR path could never sign, and the main-vs-main
path unlocked Sign with zero entries acknowledged. That is how commit b08cf21
signed 19 entries nobody reviewed. can_sign now refuses an empty diff, checks
has_blocking_risk itself instead of trusting a GUI checkbox, and resolves the
TOCTOU pin from the reviewed ref via a pure, testable sign_precondition().

Finding 5: the catalog key and the release key are now separate. The release
key lives in CI and signs checksums; the catalog key stays offline and signs
what users execute. A CI compromise gets the former, not the latter.
2026-07-12 21:30:13 -04:00
the_og 26c66b7db1 Merge branch 'main' into fix/68-console-gate
CI / Tests (py3.12 / windows-latest) (pull_request) Successful in 24s
CI / Lint (ruff) (pull_request) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 6s
2026-07-12 21:28:27 -04:00
the_og 86139100eb Merge PR #70: enforce the checks we said we had (#68)
CI / Tests (py3.12 / windows-latest) (push) Successful in 23s
CI / Lint (ruff) (push) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 10s
CI / Catalog signature (push) Successful in 6s
Findings 2/3/4/6/7. config.env now gets the deny-list, ASCII, secret and
empty-or-placeholder checks that args always had; version pinning is enforced
at runtime, not only in the maintainer tool; the CI gate pins the expected
pubkey instead of trusting the one in the PR it is reviewing; resolve_catalog
anchors its cap to the bundled version and prefers bundled on ties; catalog
ids are constrained.

Tests rewritten: the old fixtures asserted the unpinned form validates clean
and that config.env passes through verbatim — they enshrined two of the bugs.
2026-07-12 21:28:24 -04:00
BCC Agent 38f14deeff fix(core): validate config.env, enforce version pinning, fix CI trust anchor and resolve_catalog guards (#68)
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / windows-latest) (pull_request) Successful in 23s
CI / Catalog signature (pull_request) Successful in 7s
Fixes findings 2, 3, 4, 6, 7 from the issue #68 adversarial review.

- Finding 2: config.env was type-checked only. Add CATALOG_DENIED_ENV_KEYS
  (case-insensitive) for interpreter/loader-override keys (NODE_OPTIONS,
  PYTHONPATH, LD_PRELOAD, ...), apply the ASCII check and the existing
  secret-value check to env keys/values, and require env values to be
  empty or a single <PLACEHOLDER> token.

- Finding 3: version pinning was only checked by catalog_review.py (which
  never runs on the signing path per finding 1). Move enforcement into
  _validate_catalog_config: npm/uvx specs must carry @version or ==version
  (scoped names handled), docker images must have an explicit non-latest
  tag. Only the first plausible package-spec token is checked, so flags,
  <PLACEHOLDER>s, and docker subcommands/flags don't trip it. All 19 real
  catalog entries still validate clean.

- Finding 4: the CI catalog-signature gate imported bcc_core from the PR
  branch and trusted whatever CATALOG_PUBKEYS said there, so a PR changing
  both catalog.json and CATALOG_PUBKEYS (with a matching signature) went
  green. ci.yml now hardcodes the expected base64 pubkey and asserts
  bcc_core.CATALOG_PUBKEYS matches it before verifying the signature.
  NOTE: the maintainer is planning to rotate this key -- update
  EXPECTED_CATALOG_PUBKEY_B64 in ci.yml as its own reviewed change when
  that happens, never bundled with a catalog content change.

- Finding 6: resolve_catalog's anti-rollback/anti-freeze guards sat behind
  `if best_version >= 0`, so the first verified candidate was accepted
  unconditionally and the anti-freeze anchor drifted with each accepted
  candidate instead of staying fixed. The cap is now measured against the
  bundled catalog's version specifically (the trust anchor baked into the
  binary), regardless of evaluation order; bundled wins version ties; and
  a new pure `floor` parameter lets a future caller pass a persisted
  accepted-version floor.

- Finding 7: catalog id is now constrained to ^[a-z0-9][a-z0-9._-]{0,63}$.

Tests: fixed _minimal_catalog to use a pinned package (was enshrining
finding 3), rewrote the env-passthrough test to prove the validation
boundary instead of asserting env passes through unchecked, and
reordered test_resolve_catalog_rejects_absurd_version_jump so it
actually exercises the first-candidate path. Added positive/negative
tests for every new rule. Manually verified each new check by commenting
it out and confirming the guarding test goes red, then restoring it.
2026-07-12 21:27:08 -04:00
BCC Fix Agent 82483e693d fix(catalog-console): close the vacuous review gate; split catalog/release signing keys
CI / Lint (ruff) (pull_request) Successful in 9s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / windows-latest) (pull_request) Successful in 34s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 9s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 6s
Fixes #68 findings 1 and 5.

Finding 1 -- the review gate signed without reviewing anything:
- ReviewWindow._on_sign hardcoded "main" as the TOCTOU comparison ref, so
  any PR review (where _on_load pins the PR head's blob SHA) could never
  sign; the only working path was main-vs-itself, whose empty diff made
  can_sign() vacuously True (set() <= set()). Commit b08cf21 signed 19
  entries through exactly that path with zero of them reviewed.
- can_sign() now refuses an empty changeset outright, and itself checks
  has_blocking_risk() across every changed entry rather than trusting the
  GUI to have disabled a checkbox.
- ReviewSession now carries loaded_ref (the exact ref reviewed); a new pure
  sign_precondition(session, resolve_blob_sha) resolves the TOCTOU SHA from
  that ref, never a hardcoded "main". _on_load's retry path re-diffs
  instead of re-pinning the same stale SHA, so a blob-mismatch refusal
  can't loop forever.
- source="main" now diffs against the last catalog a maintainer actually
  SIGNED (walking catalog.json's git history until a version verifies
  against the current .sig), not against itself.
- Replaced the theatre-only test_no_acknowledge_all_function_exists (only
  asserted no function was *named* acknowledge_all) with a test that also
  exercises the real gate. Added can_sign/sign_precondition coverage for
  the empty-diff, blocking-risk, and ref-resolution seams -- each verified
  to fail when its guard is removed.

Finding 5 -- the catalog key and release key were the same CI-resident key:
- scripts/sign_checksums.py gets its own RELEASE_PUBKEYS (separate from
  bcc_core.CATALOG_PUBKEYS) and a verify_checksums_against_any() helper.
- release.yml's signing-smoke-test now verifies RELEASE_SIGNING_KEY against
  RELEASE_PUBKEYS only -- it no longer imports bcc_core/CATALOG_PUBKEYS at
  all, so this workflow can never compare a CI secret against the
  catalog's root of trust.
- catalog_console.py: keygen/show-seed-b64 gain --release, with separate
  keychain/file storage per key kind. show-seed-b64 refuses to run without
  --release, so the catalog seed can't be exported to a CI secret by habit.
- README documents both keys' trust properties and the asymmetry: a CI
  compromise burns the release key, never the catalog key.

The maintainer must rotate the catalog key (it was CI-resident, so treat it
as burned for catalog use) and generate a fresh release key -- see the PR
description for the exact steps. No key is generated or committed here.
2026-07-12 21:25:34 -04:00
11 changed files with 2412 additions and 1063 deletions
+50 -2
View File
@@ -95,12 +95,39 @@ jobs:
- name: Install dependencies - name: Install dependencies
run: pip install cryptography run: pip install cryptography
# 🔴 TRUST ANCHOR — issue #68 finding 4.
#
# This step used to do `import bcc_core as c` FROM THE CHECKED-OUT PR
# BRANCH and verify the catalog against c.CATALOG_PUBKEYS — i.e. it
# trusted the public key shipped in the very diff it was reviewing. A
# PR that changed data/catalog.json AND bcc_core.CATALOG_PUBKEYS (to
# an attacker key, with a matching signature produced by the attacker's
# matching private key) went green, because there was nothing outside
# the PR's own content to check the key against. The gate's whole
# point is catching a friendly-looking PR the maintainer merges
# without really reading it — and that hole made it a two-file diff.
#
# EXPECTED_CATALOG_PUBKEY_B64 below is hardcoded HERE, in the workflow
# file, independent of whatever bcc_core.py says on the PR branch. It
# is intentionally the only line in this step that matters for
# security review: changing it changes what this gate is willing to
# trust. THIS CONSTANT IS A TRUST ANCHOR. A PR that changes this line
# in the same diff as a catalog change is exactly the attack this gate
# exists to prevent — review a change to this line on its own,
# never bundled with a catalog update.
#
# NOTE for the next key rotation: update EXPECTED_CATALOG_PUBKEY_B64
# below to the new key's base64 form, as its own reviewed change.
- name: Verify data/catalog.json.sig - name: Verify data/catalog.json.sig
env:
EXPECTED_CATALOG_PUBKEY_B64: "082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="
run: | run: |
python - <<'PY' python - <<'PY'
import pathlib, sys import base64, os, pathlib, sys
import bcc_core as c import bcc_core as c
expected_pubkey_b64 = os.environ["EXPECTED_CATALOG_PUBKEY_B64"]
raw = pathlib.Path("data/catalog.json").read_bytes() raw = pathlib.Path("data/catalog.json").read_bytes()
sig_path = pathlib.Path("data/catalog.json.sig") sig_path = pathlib.Path("data/catalog.json.sig")
@@ -111,6 +138,26 @@ jobs:
if b"\x00" * 32 in c.CATALOG_PUBKEYS: if b"\x00" * 32 in c.CATALOG_PUBKEYS:
sys.exit("FAIL: CATALOG_PUBKEYS still holds the placeholder key.") sys.exit("FAIL: CATALOG_PUBKEYS still holds the placeholder key.")
# Trust anchor check FIRST, before verifying anything against
# bcc_core.CATALOG_PUBKEYS: a PR is not allowed to bring its own
# key. CATALOG_PUBKEYS on the checked-out branch must be EXACTLY
# the key(s) this workflow file itself expects -- no more, no
# fewer, no substitutions.
actual_pubkeys_b64 = [base64.b64encode(k).decode() for k in c.CATALOG_PUBKEYS]
if actual_pubkeys_b64 != [expected_pubkey_b64]:
sys.exit(
"FAIL: bcc_core.CATALOG_PUBKEYS on this branch does not match the "
"trust anchor hardcoded in .github/workflows/ci.yml.\n"
f" expected: {[expected_pubkey_b64]}\n"
f" actual: {actual_pubkeys_b64}\n"
"\n"
"This PR is changing (or has changed) the catalog signing key. That "
"change must be reviewed on its own, separately from any catalog "
"content change, and the workflow's EXPECTED_CATALOG_PUBKEY_B64 "
"updated deliberately -- not accepted because it happened to match "
"whatever bcc_core.py says on this branch."
)
if not c.verify_catalog_signature(raw, sig_path.read_bytes(), c.CATALOG_PUBKEYS): if not c.verify_catalog_signature(raw, sig_path.read_bytes(), c.CATALOG_PUBKEYS):
sys.exit( sys.exit(
"FAIL: data/catalog.json does NOT match its signature.\n" "FAIL: data/catalog.json does NOT match its signature.\n"
@@ -124,5 +171,6 @@ jobs:
if problems: if problems:
sys.exit("FAIL: catalog failed validation:\n " + "\n ".join(problems)) sys.exit("FAIL: catalog failed validation:\n " + "\n ".join(problems))
print("OK: catalog signature verifies and the catalog validates clean.") print("OK: catalog signature verifies, the pubkey matches the CI trust anchor, "
"and the catalog validates clean.")
PY PY
+49 -22
View File
@@ -101,9 +101,20 @@ jobs:
# signing step — which means a wrong/missing RELEASE_SIGNING_KEY secret # signing step — which means a wrong/missing RELEASE_SIGNING_KEY secret
# would only be discovered at the worst possible moment: during a real # would only be discovered at the worst possible moment: during a real
# release. This job signs a throwaway manifest with the secret and verifies # release. This job signs a throwaway manifest with the secret and verifies
# the result against the PUBLIC key already compiled into bcc_core. # the result against scripts/sign_checksums.RELEASE_PUBKEYS.
# #
# It proves the two halves of the keypair actually match, without # IMPORTANT (issue #68 finding 5): this must verify against the RELEASE
# public key, never bcc_core.CATALOG_PUBKEYS. The catalog key is the
# offline, maintainer-held root of trust for what BCC executes; it must
# NEVER be compared against a value that lives in a CI secret, because
# that comparison is itself a way to smuggle a catalog-trusted key through
# CI review ("does this repo secret match the catalog key" is a question
# this workflow must never even ask). The release key is a SEPARATE
# keypair, generated via `catalog_console.py keygen --release`, that only
# ever signs release SHA256SUMS manifests -- a CI/secret compromise burns
# this key, not the catalog key.
#
# It proves the two halves of the RELEASE keypair actually match, without
# publishing anything. Run it from the Actions tab after setting or # publishing anything. Run it from the Actions tab after setting or
# rotating the secret. # rotating the secret.
signing-smoke-test: signing-smoke-test:
@@ -120,42 +131,54 @@ jobs:
- name: Install dependencies - name: Install dependencies
run: pip install cryptography run: pip install cryptography
- name: Sign a throwaway manifest and verify against the shipped pubkey - name: Sign a throwaway manifest and verify against the RELEASE pubkey
env: env:
RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }} RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }}
run: | run: |
if [ -z "$RELEASE_SIGNING_KEY" ]; then if [ -z "$RELEASE_SIGNING_KEY" ]; then
echo "FAIL: RELEASE_SIGNING_KEY secret is not set." echo "FAIL: RELEASE_SIGNING_KEY secret is not set."
echo "Generate it with: python catalog_console.py show-seed-b64" echo "Generate the RELEASE key (NOT the catalog key) with:"
echo "then add it under Settings -> Actions -> Secrets." echo " python catalog_console.py keygen --release"
echo "then add its seed under Settings -> Actions -> Secrets, via:"
echo " python catalog_console.py show-seed-b64 --release"
exit 1 exit 1
fi fi
mkdir -p smoke && echo "smoke test payload" > smoke/hello.txt mkdir -p smoke && echo "smoke test payload" > smoke/hello.txt
python3 scripts/sign_checksums.py generate smoke --out smoke/SHA256SUMS python3 scripts/sign_checksums.py generate smoke --out smoke/SHA256SUMS
python3 scripts/sign_checksums.py sign --sums smoke/SHA256SUMS --out smoke/SHA256SUMS.sig python3 scripts/sign_checksums.py sign --sums smoke/SHA256SUMS --out smoke/SHA256SUMS.sig
python - <<'PY' python - <<'PY'
import base64, pathlib, sys import pathlib, sys
import bcc_core as c from scripts.sign_checksums import RELEASE_PUBKEYS, verify_checksums_against_any
from scripts.sign_checksums import verify_checksums
# The public half that ships inside the binary. If the secret is a # Deliberately does NOT import bcc_core / CATALOG_PUBKEYS at all --
# DIFFERENT key than the one users' copies trust, this fails here -- # this smoke test must never be able to compare the CI secret
# which is the entire point of the job. # against the catalog's root of trust (issue #68 finding 5). Only
pub_b64 = base64.b64encode(c.CATALOG_PUBKEYS[0]).decode() # RELEASE_PUBKEYS (scripts/sign_checksums.py) is a legitimate
# target for a CI-resident key.
if not RELEASE_PUBKEYS:
sys.exit(
"FAIL: scripts/sign_checksums.RELEASE_PUBKEYS is empty.\n"
"\n"
"Generate the release keypair with:\n"
" python catalog_console.py keygen --release\n"
"then paste the printed public key into RELEASE_PUBKEYS in\n"
"scripts/sign_checksums.py and commit that change."
)
sums = pathlib.Path("smoke/SHA256SUMS").read_text() sums = pathlib.Path("smoke/SHA256SUMS").read_text()
sig = pathlib.Path("smoke/SHA256SUMS.sig").read_bytes() sig = pathlib.Path("smoke/SHA256SUMS.sig").read_bytes()
if not verify_checksums(pub_b64, sums, sig): if not verify_checksums_against_any(RELEASE_PUBKEYS, sums, sig):
sys.exit( sys.exit(
"FAIL: the signature produced by RELEASE_SIGNING_KEY does NOT verify\n" "FAIL: the signature produced by RELEASE_SIGNING_KEY does NOT verify\n"
"against the public key in bcc_core.CATALOG_PUBKEYS.\n" "against any key in scripts/sign_checksums.RELEASE_PUBKEYS.\n"
"\n" "\n"
"The secret and the shipped public key are different keypairs. Users\n" "The secret and the shipped release public key are different keypairs.\n"
"would reject every signature this CI produces. Re-copy the seed from\n" "Downloaders would reject every signature this CI produces. Re-copy the\n"
"`catalog_console.py show-seed-b64`, or update CATALOG_PUBKEYS." "seed from `catalog_console.py show-seed-b64 --release`, or update\n"
"RELEASE_PUBKEYS with the matching public key."
) )
print("OK: RELEASE_SIGNING_KEY matches the public key shipped in bcc_core.") print("OK: RELEASE_SIGNING_KEY matches a key in RELEASE_PUBKEYS.")
PY PY
# ── Create GitHub Release with all three artifacts ────────────────────── # ── Create GitHub Release with all three artifacts ──────────────────────
@@ -206,9 +229,13 @@ jobs:
# checks. It does NOT remove Gatekeeper/SmartScreen warnings. # checks. It does NOT remove Gatekeeper/SmartScreen warnings.
# #
# The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw # The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw
# Ed25519 seed) generated via the Catalog Console (#62). If it's not # Ed25519 seed) for the RELEASE key -- a SEPARATE keypair from the
# set, we still publish the release — just without a .sig — rather # catalog key, generated via `python catalog_console.py keygen
# than fail the release outright. # --release` (issue #68 finding 5; #62). This key is intentionally
# CI-resident and signs ONLY this checksum manifest; it is never
# trusted to sign data/catalog.json. If it's not set, we still
# publish the release — just without a .sig — rather than fail the
# release outright.
- name: Check for signing key - name: Check for signing key
id: signing id: signing
run: | run: |
@@ -234,7 +261,7 @@ jobs:
- name: Warn — release will be unsigned - name: Warn — release will be unsigned
if: steps.signing.outputs.has_key != 'true' if: steps.signing.outputs.has_key != 'true'
run: | run: |
echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Add the secret (base64 raw Ed25519 seed, generated via the Catalog Console, #62) before the next tag." echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Generate the RELEASE key (python catalog_console.py keygen --release) and add its seed (python catalog_console.py show-seed-b64 --release) as this secret before the next tag."
- name: Create GitHub Release - name: Create GitHub Release
uses: softprops/action-gh-release@v2 uses: softprops/action-gh-release@v2
+45 -6
View File
@@ -36,11 +36,10 @@ are only suppressed by a paid OS-vendor certificate, which this project
doesn't have. Verifying checksums is about detecting tampering in transit or doesn't have. Verifying checksums is about detecting tampering in transit or
on a mirror, not about vouching for the software. on a mirror, not about vouching for the software.
**Release signing public key** (Ed25519, base64, raw 32 bytes): This manifest is signed with BCC's **release key**, which is a different
key from the one that signs the MCP server catalog — see
``` [Signing keys](#signing-keys) below for why, and for the public key value
<PLACEHOLDER — AJ: paste the public key from the Catalog Console (#62) here> to use with `--pubkey-b64` below.
```
### macOS / Linux ### macOS / Linux
@@ -59,7 +58,7 @@ To also verify the manifest's signature (optional, requires Python +
```bash ```bash
python3 scripts/sign_checksums.py verify \ python3 scripts/sign_checksums.py verify \
--sums SHA256SUMS --sig SHA256SUMS.sig \ --sums SHA256SUMS --sig SHA256SUMS.sig \
--pubkey-b64 "<the public key above>" --pubkey-b64 "<the release public key from Signing keys, below>"
``` ```
### Windows (PowerShell) ### Windows (PowerShell)
@@ -78,6 +77,45 @@ release is missing the `.sig` file, the checksums themselves are still
valid and safe to check against — the release workflow only skips signing, valid and safe to check against — the release workflow only skips signing,
never checksum generation. never checksum generation.
## Signing keys
BCC uses **two separate Ed25519 keypairs**, deliberately never the same
key, because they protect different things and live in different places:
| | Catalog key | Release key |
|---|---|---|
| Signs | `data/catalog.json` (the MCP server catalog every user's app trusts) | `SHA256SUMS` (the checksum manifest for release binaries) |
| Verified by | `bcc_core.CATALOG_PUBKEYS` | `scripts/sign_checksums.RELEASE_PUBKEYS` |
| Lives | Offline, passphrase-encrypted, maintainer's machine only (OS keychain or an encrypted file outside the repo — see the [Catalog Console](#files), issue #62) | A Gitea Actions repo secret, `RELEASE_SIGNING_KEY`**intentionally CI-resident** |
| Generated with | `python catalog_console.py keygen` | `python catalog_console.py keygen --release` |
| Exported for CI with | *(never — there is no supported way to export this key)* | `python catalog_console.py show-seed-b64 --release` |
**Why two keys:** the catalog key is the root of trust for what BCC
actually *executes* on a user's machine — every `command`/`args` pair in
the shipped catalog is only there because this key signed it. If that key
and the release-checksum key were the same (as they briefly were — see
[issue #68](../../issues/68)), then anything that can exfiltrate a Gitea
Actions secret (a malicious workflow-file PR, a compromised runner, a leaky
log) could sign a catalog every user's copy of BCC would trust, not just a
checksum manifest. Splitting them means **a CI/secret compromise burns the
release key, never the catalog key** — checksums for a future release could
be forged, which is bad, but no attacker gains the ability to make BCC run
arbitrary commands on installs that trust the catalog. That asymmetry is
the entire point of having two keys instead of one.
The catalog key is **never** meant to leave the maintainer's machine: it's
generated, stored, unlocked, and used to sign entirely inside the Catalog
Console (`catalog_console.py`), and `catalog_console.py show-seed-b64`
refuses to run without `--release` specifically so the catalog seed can't
be exported by habit or muscle memory.
**Release signing public key** (Ed25519, base64, raw 32 bytes) — this is
the RELEASE key, not the catalog key:
```
<PLACEHOLDER — AJ: paste the release public key from `catalog_console.py keygen --release` here>
```
## Run from source ## Run from source
```bash ```bash
@@ -152,6 +190,7 @@ file is also listed, marked *legacy*, so you can copy them over.
- `bcc.spec` — PyInstaller build spec (cross-platform). - `bcc.spec` — PyInstaller build spec (cross-platform).
- `scripts/build_icons.py` — regenerates `icons/app.icns` and `icons/app.ico` from source PNGs. - `scripts/build_icons.py` — regenerates `icons/app.icns` and `icons/app.ico` from source PNGs.
- `scripts/sign_checksums.py` — generates and Ed25519-signs the release `SHA256SUMS` manifest (see [Verifying your download](#verifying-your-download)). - `scripts/sign_checksums.py` — generates and Ed25519-signs the release `SHA256SUMS` manifest (see [Verifying your download](#verifying-your-download)).
- `catalog_console.py` / `catalog_review.py`**maintainer-only**, never shipped to users (excluded from `bcc.spec`; see `tests/test_catalog_console_packaging.py`). The Catalog Console: review + sign `data/catalog.json`, and generate/manage both signing keys (`keygen`, `keygen --release`) — see [Signing keys](#signing-keys).
## Building from source ## Building from source
+292 -436
View File
@@ -10,7 +10,7 @@ Run: python mcp_manager.py
from __future__ import annotations from __future__ import annotations
import html import contextlib
import sys import sys
import time import time
from pathlib import Path from pathlib import Path
@@ -19,6 +19,7 @@ from typing import ClassVar
from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal
from PySide6.QtGui import ( from PySide6.QtGui import (
QAction, QAction,
QActionGroup,
QColor, QColor,
QCursor, QCursor,
QDesktopServices, QDesktopServices,
@@ -26,13 +27,12 @@ from PySide6.QtGui import (
QIcon, QIcon,
QKeySequence, QKeySequence,
QPainter, QPainter,
QPalette,
QPixmap, QPixmap,
QTextCursor,
) )
from PySide6.QtWidgets import ( from PySide6.QtWidgets import (
QAbstractItemView, QAbstractItemView,
QApplication, QApplication,
QButtonGroup,
QCheckBox, QCheckBox,
QComboBox, QComboBox,
QDialog, QDialog,
@@ -68,105 +68,122 @@ import bcc_core as core
# thread during drag-and-drop import, so skip anything larger than this. # thread during drag-and-drop import, so skip anything larger than this.
MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024 # 5 MB MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024 # 5 MB
# --- Theming (issue #75) -------------------------------------------------- #
# The palette lives in bcc_core (testable without a Qt app); these module-level
# names are rebound by `apply_palette()` whenever the theme changes.
#
# Why globals rather than passing a palette around: ~20 inline
# `setStyleSheet(f"color: {MUTED}")` calls are scattered through this file, and
# an f-string resolves its names when it runs, not when it's compiled. Rebinding
# the globals means every one of those call sites picks up the new colour on its
# next render, with no change to the call sites themselves.
PALETTE = core.DARK_PALETTE
ACCENT = ACCENT_DIM = BG = PANEL = PANEL_2 = TEXT = MUTED = BORDER = ""
GOOD = BAD = WARN = REMOTE = ON_ACCENT = DISABLED_BG = MONO_BG = SEL_TEXT = ""
STATUS_COLORS: dict[str, str] = {}
HEALTH_COLORS: dict[str, str] = {}
def plain_label(text: object) -> QLabel: STATUS_GLYPH = {
"""A QLabel guaranteed to render `text` as plain text, never HTML. "ok": "\u25cf",
"missing": "\u25cf",
Qt's QLabel auto-interprets HTML by default (Qt.AutoText). Every catalog "warn": "\u25b2",
entry field (description, notes, display name, urls -- and especially "remote": "\u25c6",
args) is attacker-influenceable: catalog.json accepts community PRs, and "unknown": "\u25cb",
only a valid Ed25519 signature stands between a PR and what a user sees }
here. A `<b>` or `<img onerror=...>` in a description must render as
visible text, not markup -- exactly the same reasoning catalog_console.py
documents for its own plain_label(). Every catalog-derived string shown
by the Browse dialog MUST go through this helper (or an inherently
plain-text widget like QPlainTextEdit) rather than a bare QLabel(...).
"""
label = QLabel(html.escape(str(text)))
label.setTextFormat(Qt.TextFormat.PlainText)
label.setWordWrap(True)
return label
# --- One-line rebrand: change this to recolor the whole app --------------- #
ACCENT = "#f97316" # warm orange
ACCENT_DIM = "#c2570b"
BG = "#1b1d23"
PANEL = "#23262e"
PANEL_2 = "#2b2f39"
TEXT = "#e7e9ee"
MUTED = "#9aa0ad"
BORDER = "#3a3f4b"
GOOD = "#4ade80"
BAD = "#f87171"
WARN = "#fbbf24"
STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
STATUS_GLYPH = {"ok": "", "missing": "", "warn": "", "remote": "", "unknown": ""}
# Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the # Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the
# server tables' "Health" column -- distinct from the PATH-dependency Status # server tables' "Health" column -- distinct from the PATH-dependency Status
# column above. # column above.
HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED} HEALTH_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"}
HEALTH_GLYPH = {"ok": "", "failed": "", "untested": ""}
STYLESHEET = f"""
def build_stylesheet(p: core.Palette) -> str:
"""Render the global QSS for a palette."""
return f"""
/* No font-family here on purpose: Qt already uses the native system UI font /* No font-family here on purpose: Qt already uses the native system UI font
on every platform (San Francisco / Segoe UI / desktop default). Naming on every platform (San Francisco / Segoe UI / desktop default). Naming
web-CSS aliases like -apple-system forces a costly font-alias scan. */ web-CSS aliases like -apple-system forces a costly font-alias scan. */
* {{ font-size: 13px; color: {TEXT}; }} * {{ font-size: 13px; color: {p.text}; }}
QMainWindow, QDialog {{ background: {BG}; }} QMainWindow, QDialog {{ background: {p.bg}; }}
QLabel#h1 {{ font-size: 15px; font-weight: 600; }} QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
QLabel#muted {{ color: {MUTED}; }} QLabel#muted {{ color: {p.muted}; }}
QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }} QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }}
QLineEdit, QPlainTextEdit, QComboBox {{ QLineEdit, QPlainTextEdit, QComboBox {{
background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px; background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205; 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::drop-down {{ border: none; width: 22px; }}
QComboBox QAbstractItemView {{ background: {PANEL_2}; border: 1px solid {BORDER}; QComboBox QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border};
selection-background-color: {ACCENT}; outline: none; }} selection-background-color: {p.accent}; outline: none; }}
QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px; QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
padding: 7px 13px; }} padding: 7px 13px; }}
QPushButton:hover {{ border: 1px solid {ACCENT}; }} QPushButton:hover {{ border: 1px solid {p.accent}; }}
QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }} QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }}
QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }} QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }}
QPushButton#primary:hover {{ background: {ACCENT_DIM}; }} QPushButton#primary:hover {{ background: {p.accent_dim}; }}
QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }} QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }}
QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }} QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }}
QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; QTableWidget {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px;
gridline-color: transparent; outline: none; }} gridline-color: transparent; outline: none; }}
QTableWidget::item {{ padding: 6px 8px; border: none; }} QTableWidget::item {{ padding: 6px 8px; border: none; }}
QTableWidget::item:selected {{ background: {ACCENT}; color: #1a1205; }} QTableWidget::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
/* Inline cell editors: the global QLineEdit padding/radius clips the text /* Inline cell editors: the global QLineEdit padding/radius clips the text
inside a table row, so give editors a compact, flat style instead. */ inside a table row, so give editors a compact, flat style instead. */
QTableWidget QLineEdit {{ QTableWidget QLineEdit {{
background: {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; 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; QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none;
border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }} border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }}
QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }} QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }}
QScrollBar::handle:vertical {{ background: {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; }} QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }} QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }} QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }} QFrame#noticeBanner {{ background: {p.panel_2}; border: 1px solid {p.accent}; border-radius: 8px; }}
QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }} QLabel#noticeText {{ color: {p.text}; }}
QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }} QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }}
QTableWidget#disabledTable {{ background: #202229; }} QPushButton#noticeClose:hover {{ color: {p.text}; }}
QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }} 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; 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; }} QFrame#diagCard {{ background: transparent; border: none; }}
QSplitter::handle {{ background: transparent; }} QSplitter::handle {{ background: transparent; }}
QSplitter::handle:hover {{ background: {BORDER}; border-radius: 4px; }} QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }}
QSplitter::handle:pressed {{ background: {ACCENT}; 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) # Background reachability tester (keeps the UI responsive during the request)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
@@ -251,7 +268,7 @@ class _SecretMaskDelegate(QStyledItemDelegate):
if self.revealed or not option.text: if self.revealed or not option.text:
return return
key_item = self._table.item(index.row(), 0) key_item = self._table.item(index.row(), 0)
if key_item and core.is_secret_key(key_item.text()): if key_item and core.should_mask_value(key_item.text(), option.text):
option.text = core.MASK option.text = core.MASK
@@ -699,39 +716,6 @@ class ServerEditor(QFrame):
def current_name(self) -> str: def current_name(self) -> str:
return self.name.text().strip() return self.name.text().strip()
def focus_target(self, target: tuple[str, int | str] | None):
"""
Focus the field a catalog Add left unfilled -- `target` is whatever
core.first_unfilled_focus_target() returned: ("args", line_index),
("env", var_name), or None (nothing to fill, so do nothing).
Only meaningful on the stdio page, which is the only page a catalog
entry ever populates (link-only entries never reach dump_data()).
"""
if not target or self.type.currentIndex() != 0:
return
kind, value = target
if kind == "args":
self.args.setFocus()
cursor = self.args.textCursor()
cursor.movePosition(QTextCursor.MoveOperation.Start)
cursor.movePosition(
QTextCursor.MoveOperation.Down, QTextCursor.MoveMode.MoveAnchor, int(value)
)
cursor.movePosition(
QTextCursor.MoveOperation.EndOfLine, QTextCursor.MoveMode.KeepAnchor
)
self.args.setTextCursor(cursor)
elif kind == "env":
for r in range(self.env.table.rowCount()):
key_item = self.env.table.item(r, 0)
if key_item and key_item.text() == value:
self.env.table.setCurrentCell(r, 1)
val_item = self.env.table.item(r, 1)
if val_item:
self.env.table.editItem(val_item)
break
def _type_switched(self): def _type_switched(self):
self.stack.setCurrentIndex(self.type.currentIndex()) self.stack.setCurrentIndex(self.type.currentIndex())
self._emit() self._emit()
@@ -1260,262 +1244,6 @@ class PasteDialog(QDialog):
self.err.setText(str(e)) self.err.setText(str(e))
# --------------------------------------------------------------------------- #
# Browse catalog dialog (issue #10 phase 2): search/filter the signed,
# bundled server catalog and add a "basic" entry through the existing
# paste/import path, or send a "link-only" entry to its setup docs.
#
# Every widget here that shows catalog-derived text uses plain_label() or an
# inherently-plain widget (QPlainTextEdit) -- see plain_label()'s docstring.
# The dialog itself does no signature/schema work: MainWindow hands it an
# already-verified `entries` list (bcc_core.load_bundled_catalog_entries()),
# and an empty list here means "show the empty state", never "fall back to
# something less trusted".
# --------------------------------------------------------------------------- #
class BrowseCatalogDialog(QDialog):
def __init__(self, parent, entries: list[dict]):
super().__init__(parent)
self.setWindowTitle("Browse catalog")
self.resize(880, 560)
self.entries = entries or []
self.result_entry: dict | None = None
self._current_entry: dict | None = None
self._current_homepage: str | None = None
self._current_docs_url: str | None = None
self._current_group = "All"
outer = QVBoxLayout(self)
if not self.entries:
# Signature verification failed, or nothing was bundled -- never
# show a half-trusted list, and never explain WHY beyond this;
# a stale/tampered catalog isn't the user's problem to diagnose.
msg = plain_label("Catalog unavailable.")
msg.setObjectName("placeholder")
msg.setAlignment(Qt.AlignmentFlag.AlignCenter)
outer.addWidget(msg, 1)
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
btns.rejected.connect(self.reject)
outer.addWidget(btns)
return
search_row = QHBoxLayout()
self.search_box = QLineEdit()
self.search_box.setPlaceholderText("Search by name, description, or category…")
self.search_box.setClearButtonEnabled(True)
self.search_box.textChanged.connect(self._refresh_list)
search_row.addWidget(self.search_box, 1)
outer.addLayout(search_row)
chip_row = QHBoxLayout()
self._chip_group = QButtonGroup(self)
self._chip_group.setExclusive(True)
for label in core.CATALOG_CATEGORY_CHIPS:
btn = QPushButton(label)
btn.setCheckable(True)
btn.setChecked(label == "All")
btn.clicked.connect(lambda _checked=False, g=label: self._set_group(g))
self._chip_group.addButton(btn)
chip_row.addWidget(btn)
chip_row.addStretch()
outer.addLayout(chip_row)
splitter = QSplitter(Qt.Orientation.Horizontal)
left = QWidget()
lv = QVBoxLayout(left)
lv.setContentsMargins(0, 0, 0, 0)
self.list = QListWidget()
self.list.currentItemChanged.connect(self._on_selected)
lv.addWidget(self.list, 1)
splitter.addWidget(left)
right = QFrame()
right.setObjectName("card")
rv = QVBoxLayout(right)
self.detail_title = plain_label("")
self.detail_title.setObjectName("h1")
rv.addWidget(self.detail_title)
self.detail_meta = plain_label("")
self.detail_meta.setObjectName("muted")
rv.addWidget(self.detail_meta)
self.detail_freshness = plain_label("")
self.detail_freshness.setObjectName("muted")
rv.addWidget(self.detail_freshness)
self.detail_desc = plain_label("")
rv.addWidget(self.detail_desc)
self.detail_notes = plain_label("")
self.detail_notes.setObjectName("muted")
rv.addWidget(self.detail_notes)
self.detail_homepage_btn = QPushButton("Open homepage")
self.detail_homepage_btn.clicked.connect(self._open_homepage)
rv.addWidget(self.detail_homepage_btn)
rv.addWidget(plain_label("Exact command this will add:"))
self.detail_command_preview = QPlainTextEdit()
self.detail_command_preview.setObjectName("diag")
self.detail_command_preview.setReadOnly(True)
# Read-only QPlainTextEdit never interprets HTML, regardless of what
# a compromised/careless catalog entry's command/args contain -- this
# is the field that renders "the exact bytes that will be written".
rv.addWidget(self.detail_command_preview, 1)
self.detail_action_btn = QPushButton("")
self.detail_action_btn.setObjectName("primary")
self.detail_action_btn.clicked.connect(self._on_action)
rv.addWidget(self.detail_action_btn)
splitter.addWidget(right)
splitter.setSizes([360, 480])
outer.addWidget(splitter, 1)
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
btns.rejected.connect(self.reject)
outer.addWidget(btns)
self._refresh_list()
# --- list / filtering -------------------------------------------------- #
def _set_group(self, group: str):
self._current_group = group
self._refresh_list()
def _visible_entries(self) -> list[dict]:
filtered = core.filter_catalog_entries(self.entries, self.search_box.text())
return core.catalog_entries_in_group(filtered, self._current_group)
def _format_row(self, entry: dict) -> str:
display = str(entry.get("display") or entry.get("id") or "")
official = "" if entry.get("official") else ""
stars = entry.get("stars")
star_txt = f"{stars:,}" if isinstance(stars, int) else ""
group = core.catalog_category_group(entry.get("category", ""))
desc = str(entry.get("description") or "")
if len(desc) > 88:
desc = desc[:87] + ""
# QListWidgetItem text is always rendered literally by Qt (no HTML
# interpretation), so no escaping is needed here -- unlike QLabel.
return f"{official}{display}{star_txt}\n{desc} · {group}"
def _refresh_list(self):
self.list.blockSignals(True)
self.list.clear()
for entry in self._visible_entries():
item = QListWidgetItem(self._format_row(entry))
item.setData(Qt.ItemDataRole.UserRole, entry)
# Tooltips DO auto-detect rich text in Qt, so escape defensively
# even though descriptions are already shown, unescaped-but-safe,
# in the QListWidgetItem text above.
item.setToolTip(html.escape(str(entry.get("description", ""))))
self.list.addItem(item)
self.list.blockSignals(False)
if self.list.count():
self.list.setCurrentRow(0)
else:
self._on_selected(None, None)
# --- detail pane -------------------------------------------------------- #
def _on_selected(self, current, _previous=None):
if current is None:
self._current_entry = None
self._current_homepage = None
self._current_docs_url = None
self.detail_title.setText("")
self.detail_meta.setText("")
self.detail_freshness.setText("")
self.detail_desc.setText("No matching servers." if self.entries else "")
self.detail_notes.setText("")
self.detail_homepage_btn.setVisible(False)
self.detail_command_preview.setPlainText("")
self.detail_action_btn.setEnabled(False)
self.detail_action_btn.setText("Add")
return
entry = current.data(Qt.ItemDataRole.UserRole)
self._current_entry = entry
self.detail_title.setText(str(entry.get("display") or entry.get("id") or ""))
official = "✓ Official" if entry.get("official") else ""
stars = entry.get("stars")
star_txt = f"{stars:,}" if isinstance(stars, int) else ""
group = core.catalog_category_group(entry.get("category", ""))
meta_bits = [b for b in (official, star_txt, group) if b]
self.detail_meta.setText(" · ".join(meta_bits))
freshness = core.format_freshness_hint(entry.get("last_release"))
self.detail_freshness.setText(freshness)
self.detail_freshness.setVisible(bool(freshness))
self.detail_desc.setText(str(entry.get("description") or ""))
notes = entry.get("notes") or ""
self.detail_notes.setText(notes)
self.detail_notes.setVisible(bool(notes))
homepage = entry.get("homepage")
self._current_homepage = homepage if isinstance(homepage, str) else None
self.detail_homepage_btn.setVisible(bool(self._current_homepage))
self._current_docs_url = (
entry.get("docs_url") if isinstance(entry.get("docs_url"), str) else None
)
self.detail_command_preview.setPlainText(self._render_command_preview(entry))
if entry.get("setup") == "basic":
self.detail_action_btn.setText("Add")
self.detail_action_btn.setEnabled(True)
else:
self.detail_action_btn.setText("Open setup docs")
self.detail_action_btn.setEnabled(bool(self._current_docs_url))
def _render_command_preview(self, entry: dict) -> str:
"""
The exact command that will be written, rendered verbatim. Every
value here comes straight from the (signature-verified) catalog
entry with no interpretation beyond str() -- this must never be the
place a markup-laced description sneaks back in as "helpful"
formatting.
"""
if entry.get("setup") != "basic":
docs = entry.get("docs_url") or "(none provided)"
return (
"This is a hosted/managed integration -- there is no local "
"command to add.\n\nSetup docs:\n " + str(docs)
)
config = entry.get("config") or {}
lines = [f"command: {config.get('command', '')}"]
args = config.get("args") or []
if args:
lines.append("args:")
lines.extend(f" {a}" for a in args)
env = config.get("env") or {}
env_required = entry.get("env_required") or {}
env_keys = list(env.keys()) + [k for k in env_required if k not in env]
if env_keys:
lines.append("env (names only -- you provide the values):")
lines.extend(f" {k}" for k in env_keys)
return "\n".join(lines)
def _open_homepage(self):
url = self._current_homepage
if url and url.startswith("https://"):
QDesktopServices.openUrl(QUrl(url))
def _on_action(self):
entry = self._current_entry
if not entry:
return
if entry.get("setup") == "basic":
self.result_entry = entry
self.accept()
else:
url = self._current_docs_url
if url and url.startswith("https://"):
QDesktopServices.openUrl(QUrl(url))
# link-only never auto-adds and never closes the dialog -- the
# user can keep browsing after opening the docs in their browser.
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Log viewer dialog (issue #6): a read-only, auto-tailing view of a single # Log viewer dialog (issue #6): a read-only, auto-tailing view of a single
# server's MCP log file. Polls on a QTimer instead of watching the filesystem # server's MCP log file. Polls on a QTimer instead of watching the filesystem
@@ -1805,6 +1533,54 @@ class AboutDialog(QDialog):
QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL)) QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL))
class NoticeBanner(QFrame):
"""A persistent, dismissible notice with an optional action button.
The status bar is the wrong home for anything the user needs to act on --
21 call sites rewrite it, so a message posted there is gone by the next
click. That wiped the MSIX warning (#35) and then the update notice (#78).
This is the shared mechanism so it doesn't happen a third time.
"""
def __init__(self, parent=None):
super().__init__(parent)
self.setObjectName("noticeBanner")
row = QHBoxLayout(self)
row.setContentsMargins(10, 8, 8, 8)
row.setSpacing(8)
self._label = QLabel("")
self._label.setObjectName("noticeText")
self._label.setWordWrap(True)
row.addWidget(self._label, 1)
self._action_btn = QPushButton("")
self._action_btn.setCursor(Qt.CursorShape.PointingHandCursor)
self._action_btn.hide()
row.addWidget(self._action_btn)
self._close_btn = QPushButton("\u2715")
self._close_btn.setObjectName("noticeClose")
self._close_btn.setCursor(Qt.CursorShape.PointingHandCursor)
self._close_btn.setFixedWidth(26)
self._close_btn.setToolTip("Dismiss")
self._close_btn.clicked.connect(self.hide)
row.addWidget(self._close_btn)
self.hide()
def show_notice(self, text: str, action_label: str = "", on_action=None):
self._label.setText(text)
self._label.setToolTip(text)
# Reconnect cleanly: a banner reused for a second notice would
# otherwise fire the previous notice's action too.
with contextlib.suppress(RuntimeError, TypeError):
self._action_btn.clicked.disconnect()
if action_label and on_action is not None:
self._action_btn.setText(action_label)
self._action_btn.clicked.connect(lambda _=False: on_action())
self._action_btn.show()
else:
self._action_btn.hide()
self.show()
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS # Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS
# waiting for the old instance to exit, so it must run off the UI thread. # waiting for the old instance to exit, so it must run off the UI thread.
@@ -1862,6 +1638,11 @@ class MainWindow(QMainWindow):
self.warn_banner.hide() self.warn_banner.hide()
root.addWidget(self.warn_banner) root.addWidget(self.warn_banner)
# Update availability gets its own persistent banner rather than a
# status-line write, which the next UI action overwrites (#78).
self.update_banner = NoticeBanner(self)
root.addWidget(self.update_banner)
# User-draggable divider between the server list and the editor. # User-draggable divider between the server list and the editor.
split = QSplitter(Qt.Orientation.Horizontal) split = QSplitter(Qt.Orientation.Horizontal)
split.setChildrenCollapsible(False) split.setChildrenCollapsible(False)
@@ -1897,11 +1678,96 @@ class MainWindow(QMainWindow):
# --- menu bar ---------------------------------------------------------- # # --- menu bar ---------------------------------------------------------- #
def _build_menu_bar(self): def _build_menu_bar(self):
view_menu = self.menuBar().addMenu("&View")
theme_menu = view_menu.addMenu("Theme")
self._theme_group = QActionGroup(self)
self._theme_group.setExclusive(True)
current = stored_theme_setting()
for setting, label in (
(core.THEME_SYSTEM, "Match system"),
(core.THEME_LIGHT, "Light"),
(core.THEME_DARK, "Dark"),
):
act = QAction(label, self, checkable=True)
act.setChecked(setting == current)
act.triggered.connect(lambda _checked=False, s=setting: self._set_theme(s))
self._theme_group.addAction(act)
theme_menu.addAction(act)
help_menu = self.menuBar().addMenu("&Help") help_menu = self.menuBar().addMenu("&Help")
# "Check for updates" used to exist only as a button inside the About
# dialog, which is not somewhere anyone looks for it (#79).
update_action = QAction("Check for updates…", self)
# Explicit role: macOS relocates actions it recognises by text, and
# some Qt versions treat "update" as application-menu material. Pin it
# so the item stays where the menu says it is on every platform.
update_action.setMenuRole(QAction.MenuRole.ApplicationSpecificRole)
update_action.triggered.connect(self.check_for_updates)
help_menu.addAction(update_action)
help_menu.addSeparator()
about_action = QAction("About Better Claude Config…", self) about_action = QAction("About Better Claude Config…", self)
# Qt auto-assigns AboutRole to actions whose text starts with "About",
# which moves this into the application menu on macOS. That is the
# right home there -- state it explicitly rather than inheriting it by
# accident, since the behaviour is invisible from this call site.
about_action.setMenuRole(QAction.MenuRole.AboutRole)
about_action.triggered.connect(self._show_about) about_action.triggered.connect(self._show_about)
help_menu.addAction(about_action) help_menu.addAction(about_action)
def _show_update_notice(self, notice: dict):
"""Surface an available update where it survives the next click."""
url = notice["url"]
self.update_banner.show_notice(
notice["text"],
action_label="Open releases page",
on_action=lambda: QDesktopServices.openUrl(QUrl(url)),
)
def check_for_updates(self):
"""Menu-driven check. Unlike the startup check this is never throttled
and always reports back -- the user asked, so silence would read as a
broken button."""
self.status.setText("Checking for updates…")
self._menu_update_worker = UpdateCheckWorker()
self._menu_update_worker.done.connect(self._on_menu_update_checked)
self._menu_update_worker.start()
def _on_menu_update_checked(self, release: dict | None):
self._menu_update_worker = None
if release is None:
self.status.setText("Couldn't check for updates (offline?).")
return
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
notice = core.update_notice(core.__version__, release)
if notice:
self._show_update_notice(notice)
self.status.setText(f"Update available: {notice['version']}")
else:
self.update_banner.hide()
self.status.setText(f"You're up to date ({core.__version__}).")
def _set_theme(self, setting: str):
"""Persist the theme choice and repaint the running window."""
QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting)
app = QApplication.instance()
if app is None: # pragma: no cover - only in a headless test harness
return
app.setStyleSheet(theme_stylesheet_for(app, setting))
# The global stylesheet covers most of the UI, but the inline
# setStyleSheet calls (status dots, warning labels, update banner) only
# pick up the new palette when their widget next renders -- so re-render
# them now rather than leaving dark-on-light text behind.
self._repaint_themed_widgets()
def _repaint_themed_widgets(self):
"""Re-run the inline-styled bits after a palette change."""
self.status.setStyleSheet(f"color: {MUTED};")
idx = self._current_index()
self._refresh_tables(select_index=idx if idx >= 0 else -1)
self._update_status(saved=False)
def _show_about(self): def _show_about(self):
AboutDialog(self).exec() AboutDialog(self).exec()
@@ -1922,10 +1788,9 @@ class MainWindow(QMainWindow):
if release is None: if release is None:
return # offline/failed check: don't advance lastCheck, allow retry return # offline/failed check: don't advance lastCheck, allow retry
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time()) QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
if core.is_newer_version(core.__version__, release["version"]): notice = core.update_notice(core.__version__, release)
self.status.setText( if notice:
f"Update available: {release['version']} · Help ▸ About to view it." self._show_update_notice(notice)
)
# --- layout persistence ---------------------------------------------- # # --- layout persistence ---------------------------------------------- #
def _restore_layout(self): def _restore_layout(self):
@@ -2092,10 +1957,6 @@ class MainWindow(QMainWindow):
self.del_btn = QPushButton("Delete") self.del_btn = QPushButton("Delete")
self.del_btn.setObjectName("danger") self.del_btn.setObjectName("danger")
self.paste_btn = QPushButton("Paste JSON...") self.paste_btn = QPushButton("Paste JSON...")
self.browse_catalog_btn = QPushButton("Browse catalog…")
self.browse_catalog_btn.setToolTip(
"Add a popular MCP server from the curated, signed catalog"
)
self.copy_btn = QPushButton("Copy to ▸") self.copy_btn = QPushButton("Copy to ▸")
self.undo_btn = QPushButton("Undo") self.undo_btn = QPushButton("Undo")
self.undo_btn.setEnabled(False) self.undo_btn.setEnabled(False)
@@ -2108,7 +1969,6 @@ class MainWindow(QMainWindow):
self.dup_btn.clicked.connect(self.duplicate_server) self.dup_btn.clicked.connect(self.duplicate_server)
self.del_btn.clicked.connect(self.delete_server) self.del_btn.clicked.connect(self.delete_server)
self.paste_btn.clicked.connect(self.paste_json) self.paste_btn.clicked.connect(self.paste_json)
self.browse_catalog_btn.clicked.connect(self.browse_catalog)
self.copy_btn.clicked.connect(self.copy_to_menu) self.copy_btn.clicked.connect(self.copy_to_menu)
self.undo_btn.clicked.connect(self._undo) self.undo_btn.clicked.connect(self._undo)
self.test_all_btn.clicked.connect(self._test_all_servers) self.test_all_btn.clicked.connect(self._test_all_servers)
@@ -2117,7 +1977,6 @@ class MainWindow(QMainWindow):
self.dup_btn, self.dup_btn,
self.del_btn, self.del_btn,
self.paste_btn, self.paste_btn,
self.browse_catalog_btn,
self.copy_btn, self.copy_btn,
self.undo_btn, self.undo_btn,
self.test_all_btn, self.test_all_btn,
@@ -2257,9 +2116,21 @@ class MainWindow(QMainWindow):
return return
self.full_config = cfg self.full_config = cfg
repaired = True repaired = True
# extract_servers tolerates malformed entries rather than raising (#72),
# but keep it inside the guard: a load failure must leave the previously
# loaded profile intact instead of half-swapping the window's state.
try:
servers = core.extract_servers(self.full_config)
except Exception as exc: # pragma: no cover - defence in depth
QMessageBox.critical(
self,
"Could not read config",
f"{profile.path}\n\nThe server list couldn't be read: {exc}",
)
return
self._loaded_stat = core.config_fingerprint(profile.path) self._loaded_stat = core.config_fingerprint(profile.path)
self.current_profile = profile self.current_profile = profile
self.servers = core.extract_servers(self.full_config) self.servers = servers
self.dirty = False self.dirty = False
self.restart_btn.hide() self.restart_btn.hide()
self._undo_stack.clear() self._undo_stack.clear()
@@ -2582,7 +2453,7 @@ class MainWindow(QMainWindow):
entry = self.servers[idx] entry = self.servers[idx]
old_name = entry.name old_name = entry.name
entry.name = self.editor.current_name() entry.name = self.editor.current_name()
entry.data = self.editor.dump_data() entry.set_data(self.editor.dump_data())
# The server stays in its section (enable state unchanged), so update # The server stays in its section (enable state unchanged), so update
# its existing row in place rather than re-rendering. # its existing row in place rather than re-rendering.
# An edit invalidates any cached "Test all" result -- the server that # An edit invalidates any cached "Test all" result -- the server that
@@ -2676,7 +2547,7 @@ class MainWindow(QMainWindow):
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
) )
if ans == QMessageBox.StandardButton.Yes: if ans == QMessageBox.StandardButton.Yes:
self.servers[existing[name]].data = data self.servers[existing[name]].set_data(data)
return False, True return False, True
name = core.resolve_name_collision(name, {s.name for s in self.servers}) name = core.resolve_name_collision(name, {s.name for s in self.servers})
self.servers.append(core.ServerEntry(name, data, True)) self.servers.append(core.ServerEntry(name, data, True))
@@ -2696,41 +2567,6 @@ class MainWindow(QMainWindow):
self._mark_dirty() self._mark_dirty()
self.status.setText(f"Imported {added} added, {replaced} replaced. Review and Save.") self.status.setText(f"Imported {added} added, {replaced} replaced. Review and Save.")
def browse_catalog(self):
"""
Open the Browse-catalog dialog (issue #10 phase 2). The catalog is
loaded and signature-verified fresh every time the dialog opens --
never cached across app runs at this phase (remote fetch/cache is
#61, not yet built) -- so a bundled-catalog swap only takes effect
on next dialog open, never mid-session in a stale way.
"""
entries = core.load_bundled_catalog_entries(
_asset_dir() / "data" / "catalog.json", _asset_dir() / "data" / "catalog.json.sig"
)
dlg = BrowseCatalogDialog(self, entries)
if dlg.exec() != QDialog.DialogCode.Accepted or not dlg.result_entry:
return
entry = dlg.result_entry
paste = core.catalog_entry_to_paste_json(entry)
name, data = next(iter(paste.items()))
existing_before = {s.name: i for i, s in enumerate(self.servers)}
self._push_undo()
_added, replaced = self._import_server(name, data)
idx = (
existing_before.get(name, len(self.servers) - 1) if replaced else len(self.servers) - 1
)
self._refresh_tables(select_index=idx)
self._mark_dirty()
target = core.first_unfilled_focus_target(data)
self.editor.focus_target(target)
verb = "Replaced" if replaced else "Added"
self.status.setText(
f"{verb}{name}” from the catalog. Fill in the highlighted field and Save."
)
def copy_to_menu(self): def copy_to_menu(self):
idx = self._current_index() idx = self._current_index()
if not (0 <= idx < len(self.servers)): if not (0 <= idx < len(self.servers)):
@@ -2786,6 +2622,12 @@ class MainWindow(QMainWindow):
self.save_btn.setEnabled(False) self.save_btn.setEnabled(False)
return False return False
lint_warnings = core.lint_servers(self.servers) lint_warnings = core.lint_servers(self.servers)
# ${VAR} references are only meaningful if the target client expands
# them -- Claude Desktop doesn't, so the same config is fine in one
# profile and broken in another (#76). Report against the loaded one.
for entry in self.servers:
for warning in core.env_ref_warnings(entry.data, self.current_profile):
lint_warnings.append(f"'{entry.name}': {warning}")
if lint_warnings: if lint_warnings:
self.validation_lbl.setText(f"{lint_warnings[0]}") self.validation_lbl.setText(f"{lint_warnings[0]}")
self.validation_lbl.setStyleSheet(f"color: {WARN};") self.validation_lbl.setStyleSheet(f"color: {WARN};")
@@ -2809,29 +2651,6 @@ class MainWindow(QMainWindow):
QMessageBox.warning(self, "Can't save yet", "Fix the highlighted problem first.") QMessageBox.warning(self, "Can't save yet", "Fix the highlighted problem first.")
return return
# Placeholder guard (issue #10): a catalog Add can leave a
# <PLACEHOLDER>-style token in args/env until the user fills it in.
# This warns, it does not block -- the user may be deliberately
# saving a stub to finish later -- but it must never save silently,
# since a server launched with a literal "<PLACEHOLDER>" argument
# just fails in a confusing way at spawn time.
placeholder_names = [
s.name for s in self.servers if core.config_has_unfilled_placeholders(s.data)
]
if placeholder_names:
names = ", ".join(f"{n}" for n in placeholder_names)
ans = QMessageBox.warning(
self,
"Unfilled placeholder",
f"{names} still has a <PLACEHOLDER> value that hasn't been "
"replaced with a real value. Claude won't be able to use "
"it as-is.\n\nSave anyway?",
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
QMessageBox.StandardButton.No,
)
if ans != QMessageBox.StandardButton.Yes:
return
# Stale-file check: if the file changed on disk since we loaded it, prompt. # Stale-file check: if the file changed on disk since we loaded it, prompt.
# Compare mtime AND size (not mtime alone) so a concurrent external write # Compare mtime AND size (not mtime alone) so a concurrent external write
# that lands within the mtime resolution window, or that restores the # that lands within the mtime resolution window, or that restores the
@@ -2854,6 +2673,11 @@ class MainWindow(QMainWindow):
except Exception as e: except Exception as e:
QMessageBox.critical(self, "Reload failed", str(e)) QMessageBox.critical(self, "Reload failed", str(e))
return return
# The reload above is the on-disk truth for everything the user
# didn't touch -- but it also wipes BCC-authored keys the user
# changed in this session (named sets), which apply_servers
# doesn't write. Carry them over before saving (#73).
contested = core.carry_owned_keys(self.full_config, fresh)
core.apply_servers(fresh, self.servers) core.apply_servers(fresh, self.servers)
try: try:
backup = core.write_config(self.current_profile.path, fresh) backup = core.write_config(self.current_profile.path, fresh)
@@ -2866,8 +2690,13 @@ class MainWindow(QMainWindow):
self.dirty = False self.dirty = False
self.save_btn.setEnabled(False) self.save_btn.setEnabled(False)
bnote = f" · backup: {backup.name}" if backup else " · (new file)" bnote = f" · backup: {backup.name}" if backup else " · (new file)"
cnote = (
f" · kept your {', '.join(contested)} (the file on disk had a different copy)"
if contested
else ""
)
self.status.setText( self.status.setText(
f"Merged & saved {self.current_profile.path}{bnote}" f"Merged & saved {self.current_profile.path}{bnote}{cnote}"
f" · Restart {self.current_profile.label} to apply." f" · Restart {self.current_profile.label} to apply."
) )
self._offer_restart_button() self._offer_restart_button()
@@ -3025,6 +2854,33 @@ class MainWindow(QMainWindow):
e.accept() e.accept()
def system_is_dark(app: QApplication) -> bool:
"""Whether the desktop is currently using a dark appearance.
Read from the style's own window colour rather than per-platform APIs --
Qt has already resolved the OS appearance by the time it builds the
default palette, so this works the same on all three platforms.
"""
try:
return app.palette().color(QPalette.ColorRole.Window).lightness() < 128
except Exception: # pragma: no cover - defensive; never block startup on theming
return True
def stored_theme_setting() -> str:
"""The user's theme choice, defaulting to following the system."""
value = QSettings("BCC", "BetterClaudeConfig").value("ui/theme", core.THEME_SYSTEM)
return value if value in core.THEME_CHOICES else core.THEME_SYSTEM
def theme_stylesheet_for(app: QApplication, setting: str | None = None) -> str:
"""Resolve setting + OS appearance into a palette, apply it, return the QSS."""
if setting is None:
setting = stored_theme_setting()
theme = core.resolve_theme(setting, system_is_dark(app))
return apply_palette(core.palette_for(theme))
def main(): def main():
if sys.platform == "win32": if sys.platform == "win32":
# Without an explicit AppUserModelID, Windows taskbar groups the app # Without an explicit AppUserModelID, Windows taskbar groups the app
@@ -3043,7 +2899,7 @@ def main():
icon = _app_icon() icon = _app_icon()
if not icon.isNull(): if not icon.isNull():
app.setWindowIcon(icon) app.setWindowIcon(icon)
app.setStyleSheet(STYLESHEET) app.setStyleSheet(theme_stylesheet_for(app))
win = MainWindow() win = MainWindow()
win.show() win.show()
sys.exit(app.exec()) sys.exit(app.exec())
+1 -5
View File
@@ -31,11 +31,7 @@ a = Analysis(
["bcc.py"], ["bcc.py"],
pathex=[], pathex=[],
binaries=[], binaries=[],
datas=[ datas=[("icons", "icons"), ("data/catalog.json", "data")],
("icons", "icons"),
("data/catalog.json", "data"),
("data/catalog.json.sig", "data"),
],
hiddenimports=[], hiddenimports=[],
hookspath=[], hookspath=[],
hooksconfig={}, hooksconfig={},
+762 -252
View File
File diff suppressed because it is too large Load Diff
+262 -61
View File
@@ -12,7 +12,12 @@ Flow: Load -> Review -> Sign.
1. Load -- pick a source: an open Gitea PR touching data/catalog.json, 1. Load -- pick a source: an open Gitea PR touching data/catalog.json,
or the current tip of `main`. The Console fetches the exact or the current tip of `main`. The Console fetches the exact
git blob (via a local clone's git plumbing) and PINS its git blob (via a local clone's git plumbing) and PINS its
blob SHA for the rest of this review pass. blob SHA *and the ref it came from* for the rest of this
review pass. For a PR, the diff is against `main`; for
`main`, the diff is against the last catalog a maintainer
actually SIGNED (the bytes covered by the current
data/catalog.json.sig), never against itself -- an empty
diff must mean "nothing to sign", never "sign unlocked".
2. Review -- a semantic diff (catalog_review.diff_catalogs), one card per 2. Review -- a semantic diff (catalog_review.diff_catalogs), one card per
changed entry, with risk annotations changed entry, with risk annotations
(catalog_review.entry_risk_findings). A registry lookup for (catalog_review.entry_risk_findings). A registry lookup for
@@ -25,20 +30,34 @@ Flow: Load -> Review -> Sign.
changed entry must be individually acknowledged (its changed entry must be individually acknowledged (its
checkbox ticked) before Sign unlocks. There is no checkbox ticked) before Sign unlocks. There is no
"acknowledge all" -- see catalog_review.py. "acknowledge all" -- see catalog_review.py.
3. Sign -- re-fetches the current blob SHA and refuses to sign unless 3. Sign -- re-resolves the current blob SHA from the SAME ref that was
it still matches the pinned SHA from step 1 (TOCTOU fix: reviewed (never a hardcoded "main") and refuses to sign
catalog_review.can_sign). On success, writes unless it still matches the pinned SHA from step 1, the diff
is non-empty, and no changed entry has an outstanding
blocking risk finding (catalog_review.sign_precondition /
can_sign -- the TOCTOU fix). On success, writes
data/catalog.json + data/catalog.json.sig and commits BOTH data/catalog.json + data/catalog.json.sig and commits BOTH
in a single commit, then pushes -- so main is never red in a single commit, then pushes -- so main is never red
between a catalog merge and its signature. between a catalog merge and its signature. Signing uses the
CATALOG key ONLY -- see "Two signing keys" below.
The signature must be the artefact of an actual review, not a step that The signature must be the artefact of an actual review, not a step that
follows one. Signing IS the approval act. follows one. Signing IS the approval act.
Two signing keys (issue #68 finding 5): the CATALOG key (offline,
Console-only, `keygen` / `keygen --release` picks which) is the root of
trust for what BCC executes and must never touch CI. The RELEASE key is
CI-resident and signs ONLY the release SHA256SUMS manifest
(scripts/sign_checksums.py) -- `show-seed-b64 --release` is the only
supported way to get a seed out of this tool, and it refuses to run without
`--release` so the catalog seed can never be exported by habit. See the
README's "Signing keys" section.
""" """
from __future__ import annotations from __future__ import annotations
import argparse import argparse
import base64
import contextlib import contextlib
import getpass import getpass
import html import html
@@ -69,8 +88,28 @@ SIG_PATH = "data/catalog.json.sig"
# maintainer-only tool, so a dotfile under $HOME is an acceptable fallback # maintainer-only tool, so a dotfile under $HOME is an acceptable fallback
# when the OS keychain isn't available -- the blob stored there is always # when the OS keychain isn't available -- the blob stored there is always
# passphrase-encrypted (see catalog_review.encrypt_private_key), never raw. # passphrase-encrypted (see catalog_review.encrypt_private_key), never raw.
#
# Two SEPARATE keys are stored here, never conflated (issue #68 finding 5):
# "catalog" -- offline, Console-only. Verified by bcc_core.CATALOG_PUBKEYS.
# Roots of trust for every catalog entry BCC ships. Must
# NEVER leave this machine, never touch CI, never become an
# env var or a repo secret.
# "release" -- CI-resident. Verified by scripts.sign_checksums.
# RELEASE_PUBKEYS. Signs ONLY the release SHA256SUMS
# manifest. Its private seed is deliberately meant to be
# pasted into the RELEASE_SIGNING_KEY Gitea Actions secret
# (via `show-seed-b64 --release`) -- that is its normal,
# intended flow. A CI compromise burns this key, not the
# catalog key: that asymmetry is the whole point of having
# two keys instead of one.
KEY_STORAGE_DIR = Path.home() / ".bcc-catalog-console" KEY_STORAGE_DIR = Path.home() / ".bcc-catalog-console"
KEY_STORAGE_FILE = KEY_STORAGE_DIR / "signing_key.enc" _KEY_KINDS = ("catalog", "release")
def _key_storage_file(kind: str) -> Path:
assert kind in _KEY_KINDS, f"unknown key kind {kind!r}, expected one of {_KEY_KINDS}"
return KEY_STORAGE_DIR / f"signing_key_{kind}.enc"
HTTP_TIMEOUT = 6.0 HTTP_TIMEOUT = 6.0
@@ -97,51 +136,64 @@ def _keyring_module():
_KEYRING_SERVICE = "bcc-catalog-console" _KEYRING_SERVICE = "bcc-catalog-console"
_KEYRING_USERNAME = "signing-key"
def store_encrypted_key(blob: bytes) -> str: def _keyring_username(kind: str) -> str:
assert kind in _KEY_KINDS, f"unknown key kind {kind!r}, expected one of {_KEY_KINDS}"
return f"signing-key-{kind}"
def store_encrypted_key(blob: bytes, kind: str = "catalog") -> str:
"""Persist an already-encrypted key blob (see """Persist an already-encrypted key blob (see
catalog_review.encrypt_private_key). Prefers the OS keychain; falls back catalog_review.encrypt_private_key) under the given `kind`
to a file under KEY_STORAGE_DIR (outside the repo) with restrictive ("catalog" or "release" -- see the KEY_STORAGE_DIR comment above; the
two are stored under different keychain entries / filenames so they can
never be loaded interchangeably). Prefers the OS keychain; falls back to
a file under KEY_STORAGE_DIR (outside the repo) with restrictive
permissions. Returns a human-readable description of where it went.""" permissions. Returns a human-readable description of where it went."""
keyring = _keyring_module() keyring = _keyring_module()
if keyring is not None: if keyring is not None:
try: try:
keyring.set_password(_KEYRING_SERVICE, _KEYRING_USERNAME, blob.hex()) keyring.set_password(_KEYRING_SERVICE, _keyring_username(kind), blob.hex())
return "OS keychain (via the `keyring` package)" return "OS keychain (via the `keyring` package)"
except Exception: except Exception:
pass # fall through to the file-based path pass # fall through to the file-based path
KEY_STORAGE_DIR.mkdir(parents=True, exist_ok=True) KEY_STORAGE_DIR.mkdir(parents=True, exist_ok=True)
KEY_STORAGE_FILE.write_bytes(blob) key_file = _key_storage_file(kind)
key_file.write_bytes(blob)
with contextlib.suppress(OSError): # best-effort on platforms without POSIX perm bits with contextlib.suppress(OSError): # best-effort on platforms without POSIX perm bits
KEY_STORAGE_FILE.chmod(0o600) key_file.chmod(0o600)
return f"encrypted file at {KEY_STORAGE_FILE}" return f"encrypted file at {key_file}"
def load_encrypted_key() -> bytes: def load_encrypted_key(kind: str = "catalog") -> bytes:
"""Load the encrypted key blob from wherever store_encrypted_key() put """Load the encrypted key blob of the given `kind` from wherever
it. Raises FileNotFoundError if no key has been generated yet.""" store_encrypted_key() put it. Raises FileNotFoundError if no key of that
kind has been generated yet."""
keyring = _keyring_module() keyring = _keyring_module()
if keyring is not None: if keyring is not None:
try: try:
hex_blob = keyring.get_password(_KEYRING_SERVICE, _KEYRING_USERNAME) hex_blob = keyring.get_password(_KEYRING_SERVICE, _keyring_username(kind))
if hex_blob: if hex_blob:
return bytes.fromhex(hex_blob) return bytes.fromhex(hex_blob)
except Exception: except Exception:
pass pass
if not KEY_STORAGE_FILE.exists(): key_file = _key_storage_file(kind)
if not key_file.exists():
flag = " --release" if kind == "release" else ""
raise FileNotFoundError( raise FileNotFoundError(
f"No signing key found (checked the OS keychain and {KEY_STORAGE_FILE}). " f"No {kind} signing key found (checked the OS keychain and {key_file}). "
"Run `python catalog_console.py keygen` first." f"Run `python catalog_console.py keygen{flag}` first."
) )
return KEY_STORAGE_FILE.read_bytes() return key_file.read_bytes()
def unlock_signing_key(passphrase: str) -> bytes: def unlock_signing_key(passphrase: str, kind: str = "catalog") -> bytes:
"""Load + decrypt the signing key seed. Raises ValueError on a wrong """Load + decrypt the signing key seed of the given `kind`. Raises
passphrase, FileNotFoundError if no key exists yet.""" ValueError on a wrong passphrase, FileNotFoundError if no key of that
blob = load_encrypted_key() kind exists yet. Defaults to "catalog" because that's the key
ReviewWindow._on_sign uses -- the GUI never touches the release key."""
blob = load_encrypted_key(kind)
return review.decrypt_private_key(blob, passphrase) return review.decrypt_private_key(blob, passphrase)
@@ -188,6 +240,49 @@ def read_catalog_at_commit(repo_dir: Path, commit: str) -> tuple[bytes, str]:
return blob_bytes(repo_dir, sha), sha return blob_bytes(repo_dir, sha), sha
def catalog_blob_history(repo_dir: Path, ref: str, limit: int = 200) -> list[str]:
"""Blob SHAs of CATALOG_PATH at each commit that touched it, walking
back from `ref`, most-recent-first. Used by last_signed_catalog_raw() to
find "the version of the catalog the current signature actually covers"
without assuming it's the tip commit."""
log_out = _git(repo_dir, "log", f"--max-count={limit}", "--format=%H", ref, "--", CATALOG_PATH)
commits = [line for line in log_out.splitlines() if line]
shas: list[str] = []
for commit in commits:
try:
shas.append(blob_sha_at(repo_dir, commit, CATALOG_PATH))
except GitError:
continue
return shas
def last_signed_catalog_raw(repo_dir: Path, commit: str) -> bytes | None:
"""The raw data/catalog.json bytes that verify against
data/catalog.json.sig as of `commit` -- i.e. "the last catalog a
maintainer actually signed", found by walking the catalog's git history
on that ref until a version verifies against the *current* signature.
This is what source="main (current tip)" diffs against (issue #68
finding 1): if main's tip catalog.json already matches its .sig, this
returns that same content and the diff is correctly empty (nothing new
to sign). If someone merged a catalog change to main without running it
through the Console -- the exact bypass that produced commit b08cf21 --
the signature still covers the OLDER content, so this returns that older
version and the diff surfaces exactly what was never actually reviewed.
Returns None if there's no signature yet, or none of the recent history
verifies against it (caller should treat this as "diff against nothing
ever signed", i.e. every current entry shows as newly added).
"""
try:
sig_sha = blob_sha_at(repo_dir, commit, SIG_PATH)
except GitError:
return None
sig_bytes = blob_bytes(repo_dir, sig_sha)
candidates = [blob_bytes(repo_dir, sha) for sha in catalog_blob_history(repo_dir, commit)]
return review.find_last_signed_catalog_raw(candidates, sig_bytes, core.CATALOG_PUBKEYS)
def commit_and_push_signed_catalog( def commit_and_push_signed_catalog(
repo_dir: Path, raw_bytes: bytes, signature: bytes, *, branch: str = "main" repo_dir: Path, raw_bytes: bytes, signature: bytes, *, branch: str = "main"
) -> str: ) -> str:
@@ -644,28 +739,50 @@ class ReviewWindow(QMainWindow):
row = self.source_list.currentRow() row = self.source_list.currentRow()
try: try:
if row <= 0: if row <= 0:
commit = fetch_ref(self.repo_dir, "main") # source = main: diff against the last catalog a maintainer
old_commit = None # main vs itself has no "old" -- nothing to diff without a base # actually SIGNED (the bytes covered by the current
# data/catalog.json.sig), never against itself. Diffing
# main-vs-main is what made an empty diff -> instantly
# "signable" in the first place (issue #68 finding 1) --
# this is the only path that reaches sign_catalog_bytes(),
# so if it can't be trusted nothing can.
loaded_ref = "main"
commit = fetch_ref(self.repo_dir, loaded_ref)
new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
new_catalog = core.load_catalog(new_raw)
old_raw = last_signed_catalog_raw(self.repo_dir, commit)
old_catalog = (
core.load_catalog(old_raw)
if old_raw is not None
else {
"schema": 1,
"version": 0,
"servers": [],
}
)
else: else:
pr = self._prs[row - 1] pr = self._prs[row - 1]
commit = fetch_ref(self.repo_dir, pr.head_ref) loaded_ref = pr.head_ref
commit = fetch_ref(self.repo_dir, loaded_ref)
old_commit = fetch_ref(self.repo_dir, "main") old_commit = fetch_ref(self.repo_dir, "main")
new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit) new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
new_catalog = core.load_catalog(new_raw) new_catalog = core.load_catalog(new_raw)
if old_commit:
old_raw, _old_sha = read_catalog_at_commit(self.repo_dir, old_commit) old_raw, _old_sha = read_catalog_at_commit(self.repo_dir, old_commit)
old_catalog = core.load_catalog(old_raw) old_catalog = core.load_catalog(old_raw)
else:
old_catalog = new_catalog
except (GitError, ValueError) as e: except (GitError, ValueError) as e:
QMessageBox.critical(self, "Load failed", html.escape(str(e))) QMessageBox.critical(self, "Load failed", html.escape(str(e)))
return return
self._new_raw = new_raw self._new_raw = new_raw
self.session = review.start_review(new_blob_sha, old_catalog, new_catalog) # `loaded_ref` is pinned into the session (not just this method's
# local variable) so _on_sign can re-resolve the TOCTOU blob SHA
# from the SAME ref that was reviewed, instead of a hardcoded
# "main" -- see review.sign_precondition and issue #68 finding 1.
self.session = review.start_review(
new_blob_sha, old_catalog, new_catalog, loaded_ref=loaded_ref
)
self._render_cards() self._render_cards()
def _render_cards(self): def _render_cards(self):
@@ -706,20 +823,43 @@ class ReviewWindow(QMainWindow):
def _on_sign(self): def _on_sign(self):
assert self.session is not None assert self.session is not None
def _resolve_blob_sha(ref: str) -> str:
# Re-fetch fresh, immediately before signing, from the SAME ref
# that was reviewed (session.loaded_ref) -- NEVER hardcode
# "main" here. Hardcoding "main" is the bug that made the PR
# review path unable to sign at all: _on_load() pins the PR
# head's blob SHA, so comparing against main's SHA differs by
# definition for any PR that actually changes the catalog, and
# this refused every PR review permanently (see issue #68
# finding 1 and review.sign_precondition's docstring).
return blob_sha_at(self.repo_dir, fetch_ref(self.repo_dir, ref), CATALOG_PATH)
try: try:
current_sha = blob_sha_at(self.repo_dir, fetch_ref(self.repo_dir, "main"), CATALOG_PATH) decision = review.sign_precondition(self.session, _resolve_blob_sha)
except GitError as e: except GitError as e:
QMessageBox.critical(self, "Sign failed", html.escape(str(e))) QMessageBox.critical(self, "Sign failed", html.escape(str(e)))
return return
decision = review.can_sign(self.session, current_sha)
if not decision.ok: if not decision.ok:
QMessageBox.warning(self, "Cannot sign", html.escape(decision.reason or "")) QMessageBox.warning(self, "Cannot sign", html.escape(decision.reason or ""))
if decision.reason and "changed" in decision.reason.lower(): # Only the TOCTOU blob-SHA-mismatch reason should trigger a
self._on_load() # force a re-review against the new bytes # reload -- "mismatch" appears ONLY in that reason (deliberately
# checked instead of the broader "changed", which also matches
# "Not every CHANGED entry has been acknowledged yet" and would
# wrongly force a reload -- and with it a fresh review.py
# ReviewSession -- every time a reviewer pauses partway through
# ticking checkboxes).
if decision.reason and "mismatch" in decision.reason.lower():
# Force a genuine re-review against the new bytes -- this
# must call _on_load() (which re-fetches and re-diffs), NOT
# re-invoke the same stale resolver, or this becomes the
# infinite loop described in issue #68 finding 1: re-pinning
# the same wrong SHA forever instead of ever converging.
self._on_load()
return return
dialog = PassphraseDialog("Enter signing key passphrase:", self) dialog = PassphraseDialog("Enter CATALOG signing key passphrase:", self)
if dialog.exec() != QDialog.DialogCode.Accepted: if dialog.exec() != QDialog.DialogCode.Accepted:
return return
try: try:
@@ -744,9 +884,17 @@ class ReviewWindow(QMainWindow):
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
def cmd_keygen(_args: argparse.Namespace) -> int: def cmd_keygen(args: argparse.Namespace) -> int:
# Two SEPARATE keypairs, never conflated (issue #68 finding 5): the
# catalog key is the offline root of trust for what BCC executes and
# must never touch CI; the release key is CI-resident and signs ONLY
# the release SHA256SUMS manifest. Which one this run generates is
# explicit via --release, and the printed instructions differ sharply
# so it's obvious which key is safe to paste into a CI secret (release)
# and which one never is (catalog).
kind = "release" if getattr(args, "release", False) else "catalog"
seed, pubkey = review.generate_keypair() seed, pubkey = review.generate_keypair()
passphrase = getpass.getpass("Choose a passphrase to encrypt the new signing key: ") passphrase = getpass.getpass(f"Choose a passphrase to encrypt the new {kind} signing key: ")
confirm = getpass.getpass("Confirm passphrase: ") confirm = getpass.getpass("Confirm passphrase: ")
if passphrase != confirm: if passphrase != confirm:
print("error: passphrases did not match", file=sys.stderr) print("error: passphrases did not match", file=sys.stderr)
@@ -756,31 +904,67 @@ def cmd_keygen(_args: argparse.Namespace) -> int:
return 1 return 1
blob = review.encrypt_private_key(seed, passphrase) blob = review.encrypt_private_key(seed, passphrase)
where = store_encrypted_key(blob) where = store_encrypted_key(blob, kind=kind)
pubkey_b64 = __import__("base64").b64encode(pubkey).decode("ascii") pubkey_b64 = base64.b64encode(pubkey).decode("ascii")
print(f"Private key encrypted and stored in: {where}") print(f"{kind.capitalize()} private key encrypted and stored in: {where}")
print() print()
print("Public key (base64, paste into bcc_core.CATALOG_PUBKEYS):") if kind == "catalog":
print(f" {pubkey_b64}") print(
print() "This is the CATALOG key. It is the root of trust for every catalog "
print( "entry BCC ships -- it must stay offline and Console-only. NEVER paste "
"Also add it as the Gitea repo secret RELEASE_SIGNING_KEY (base64 of the " "it, its seed, or `show-seed-b64` output into CI, an env var, or a repo "
"32-byte private seed) used by release.yml -- get that value with:" "secret. (If the key currently in bcc_core.CATALOG_PUBKEYS has ever been "
) "pasted into a CI secret, treat it as burned for catalog use -- generate "
print(" python catalog_console.py show-seed-b64 # careful: prints the raw key") "a fresh one with this command and rotate.)"
)
print()
print(
"Public key (base64) -- hand this to whoever maintains bcc_core.py so "
"they can add it to CATALOG_PUBKEYS (this tool does not edit that file):"
)
print(f" {pubkey_b64}")
else:
print(
"This is the RELEASE key. It signs ONLY the release SHA256SUMS "
"manifest in CI -- it is intentionally CI-resident and is NOT trusted "
"to sign the catalog (bcc_core.CATALOG_PUBKEYS does not and must not "
"contain it)."
)
print()
print("1. Public key (base64) -- paste into scripts/sign_checksums.py RELEASE_PUBKEYS:")
print(f" {pubkey_b64}")
print()
print(
"2. Private key -- add it as the Gitea repo secret RELEASE_SIGNING_KEY "
"(base64 of the 32-byte private seed). Get that value with:"
)
print(" python catalog_console.py show-seed-b64 --release # prints the raw key")
return 0 return 0
def cmd_show_seed_b64(_args: argparse.Namespace) -> int: def cmd_show_seed_b64(args: argparse.Namespace) -> int:
passphrase = getpass.getpass("Signing key passphrase: ") # Deliberately requires --release: this command's whole purpose is to
# produce a value that gets pasted into a CI secret, and the catalog key
# must NEVER be pasted into CI (issue #68 finding 5 -- that is exactly
# how the catalog key ended up burned in the first place). Refusing to
# run without --release makes "export the catalog seed for CI" a
# structurally different, more deliberate action than a typo away.
if not getattr(args, "release", False):
print(
"error: show-seed-b64 only ever exports the RELEASE key -- it is the "
"only key allowed to leave this machine, for the RELEASE_SIGNING_KEY CI "
"secret. Re-run as `show-seed-b64 --release`. The catalog key must never "
"be exported this way; see issue #68 finding 5.",
file=sys.stderr,
)
return 1
passphrase = getpass.getpass("Release signing key passphrase: ")
try: try:
seed = unlock_signing_key(passphrase) seed = unlock_signing_key(passphrase, kind="release")
except (FileNotFoundError, ValueError) as e: except (FileNotFoundError, ValueError) as e:
print(f"error: {e}", file=sys.stderr) print(f"error: {e}", file=sys.stderr)
return 1 return 1
import base64
print(base64.b64encode(seed).decode("ascii")) print(base64.b64encode(seed).decode("ascii"))
return 0 return 0
@@ -810,11 +994,28 @@ def build_parser() -> argparse.ArgumentParser:
p_gui.add_argument("--repo", default=".", help="path to a BCC git checkout (default: cwd)") p_gui.add_argument("--repo", default=".", help="path to a BCC git checkout (default: cwd)")
p_gui.set_defaults(func=cmd_gui) p_gui.set_defaults(func=cmd_gui)
p_keygen = sub.add_parser("keygen", help="generate a new Ed25519 signing keypair") p_keygen = sub.add_parser(
"keygen", help="generate a new Ed25519 signing keypair (catalog key by default)"
)
p_keygen.add_argument(
"--release",
action="store_true",
help=(
"generate the RELEASE key (CI-resident, signs SHA256SUMS only) instead "
"of the CATALOG key (offline, Console-only, signs data/catalog.json -- "
"see issue #68 finding 5)"
),
)
p_keygen.set_defaults(func=cmd_keygen) p_keygen.set_defaults(func=cmd_keygen)
p_seed = sub.add_parser( p_seed = sub.add_parser(
"show-seed-b64", help="print the base64 private seed (for the RELEASE_SIGNING_KEY secret)" "show-seed-b64",
help="print a base64 private seed for a CI secret -- RELEASE key only",
)
p_seed.add_argument(
"--release",
action="store_true",
help="required: only the release key may ever be exported this way",
) )
p_seed.set_defaults(func=cmd_show_seed_b64) p_seed.set_defaults(func=cmd_show_seed_b64)
+97 -6
View File
@@ -24,6 +24,7 @@ from urllib.parse import urlsplit
from bcc_core import _CATALOG_SIG_DOMAIN as CATALOG_SIG_DOMAIN from bcc_core import _CATALOG_SIG_DOMAIN as CATALOG_SIG_DOMAIN
from bcc_core import CATALOG_ALLOWED_COMMANDS from bcc_core import CATALOG_ALLOWED_COMMANDS
from bcc_core import verify_catalog_signature as _verify_catalog_signature
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Semantic diff # Semantic diff
@@ -432,11 +433,21 @@ def has_blocking_risk(change: EntryChange) -> bool:
class ReviewSession: class ReviewSession:
"""State for one review pass. `pinned_blob_sha` is the git blob SHA of """State for one review pass. `pinned_blob_sha` is the git blob SHA of
data/catalog.json as it existed the moment review began -- see data/catalog.json as it existed the moment review began -- see
can_sign().""" can_sign()/sign_precondition().
`loaded_ref` is the exact ref this review was loaded from ("main", or a
PR's `refs/pull/<n>/head`) -- see issue #68 finding 1. It exists so the
Sign path can re-resolve the TOCTOU blob SHA from *the ref that was
actually reviewed*, instead of a hardcoded "main" that silently diverges
from the reviewed ref on every PR review (the bug that made the PR path
unable to sign at all, and forced everyone onto the vacuous
main-vs-itself path instead).
"""
pinned_blob_sha: str pinned_blob_sha: str
old_catalog: dict old_catalog: dict
new_catalog: dict new_catalog: dict
loaded_ref: str = "main"
changes: list[EntryChange] = field(default_factory=list) changes: list[EntryChange] = field(default_factory=list)
acknowledged: set[str] = field(default_factory=set) acknowledged: set[str] = field(default_factory=set)
@@ -445,12 +456,39 @@ class ReviewSession:
self.changes = diff_catalogs(self.old_catalog, self.new_catalog) self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
def start_review(pinned_blob_sha: str, old_catalog: dict, new_catalog: dict) -> ReviewSession: def start_review(
pinned_blob_sha: str,
old_catalog: dict,
new_catalog: dict,
loaded_ref: str = "main",
) -> ReviewSession:
return ReviewSession( return ReviewSession(
pinned_blob_sha=pinned_blob_sha, old_catalog=old_catalog, new_catalog=new_catalog pinned_blob_sha=pinned_blob_sha,
old_catalog=old_catalog,
new_catalog=new_catalog,
loaded_ref=loaded_ref,
) )
def find_last_signed_catalog_raw(
candidates: list[bytes], sig: bytes, pubkeys: list[bytes]
) -> bytes | None:
"""Given `candidates` (candidate raw catalog.json byte-strings -- e.g.
successive historical versions from git log, most-recent-first),
return the first one whose signature verifies against `sig`/`pubkeys`,
or None if none do.
This is how source="main" review diffs against "the last catalog a
maintainer actually signed" instead of against itself (issue #68
finding 1): `catalog_console.last_signed_catalog_raw` walks
data/catalog.json's git history on main and hands the candidates here.
"""
for raw in candidates:
if _verify_catalog_signature(raw, sig, pubkeys):
return raw
return None
def acknowledge_entry(session: ReviewSession, entry_id: str) -> None: def acknowledge_entry(session: ReviewSession, entry_id: str) -> None:
ids = {c.entry_id for c in session.changes} ids = {c.entry_id for c in session.changes}
if entry_id not in ids: if entry_id not in ids:
@@ -483,22 +521,53 @@ class SignDecision:
def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision: def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
"""Whether the Sign button may fire right now. """Whether the Sign button may fire right now.
Two independent gates, both required: Four independent gates, all required, checked in this order:
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing)
0. The diff must be non-empty. An empty diff historically meant "Sign
unlocks instantly" (`set() <= set()` is vacuously True), which is
exactly backwards: a vacuously-satisfied gate is worse than no gate
at all, because it *manufactures confidence* -- the signature looks
identical to one produced by a real review. "Nothing changed" must
mean "nothing to sign", never "sign unlocked". (Issue #68 finding 1;
this is what let commit b08cf21 sign all 19 entries with zero of them
ever reviewed.)
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing,
from the ref that was actually reviewed -- see sign_precondition())
must match the blob SHA pinned when review began. If the bytes on the must match the blob SHA pinned when review began. If the bytes on the
remote changed since -- a new commit pushed to the same PR, a remote changed since -- a new commit pushed to the same PR, a
force-push, another PR merged in between -- signing is refused and a force-push, another PR merged in between -- signing is refused and a
re-review is forced. This is what makes "signing is the approval act" re-review is forced. This is what makes "signing is the approval act"
true rather than aspirational: the signature is bound to the exact true rather than aspirational: the signature is bound to the exact
reviewed bytes, not to "whatever the file happens to be now". reviewed bytes, not to "whatever the file happens to be now".
2. Every changed entry in the diff must be individually acknowledged. 2. No blocking risk finding may be outstanding on ANY changed entry, full
stop -- checked here, not just in the GUI. The GUI additionally
disables the acknowledge checkbox for a blocking entry, but that is a
UI nicety, not the enforcement point: if this pure gate didn't also
check it, a blocking risk would only be stopped by the GUI happening
to have wired the checkbox correctly, and nothing would catch a
regression in that wiring. The GUI must not be the only thing
standing between a blocking risk and a signature.
3. Every changed entry in the diff must be individually acknowledged.
""" """
if not session.changes:
return SignDecision(
False,
"Nothing to sign: this review's diff is empty. If you expected "
"changes here, you may be diffing the wrong source/ref.",
)
if current_blob_sha != session.pinned_blob_sha: if current_blob_sha != session.pinned_blob_sha:
return SignDecision( return SignDecision(
False, False,
"The reviewed bytes changed since this review began (blob SHA " "The reviewed bytes changed since this review began (blob SHA "
"mismatch) -- re-review required before signing.", "mismatch) -- re-review required before signing.",
) )
blocking_ids = sorted({c.entry_id for c in session.changes if has_blocking_risk(c)})
if blocking_ids:
return SignDecision(
False,
"Blocking risk finding(s) outstanding on: "
f"{', '.join(blocking_ids)} -- fix the underlying change, do not sign around it.",
)
if not all_entries_acknowledged(session): if not all_entries_acknowledged(session):
pending = sorted({c.entry_id for c in session.changes} - session.acknowledged) pending = sorted({c.entry_id for c in session.changes} - session.acknowledged)
return SignDecision( return SignDecision(
@@ -507,6 +576,28 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
return SignDecision(True, None) return SignDecision(True, None)
def sign_precondition(
session: ReviewSession, resolve_blob_sha: Callable[[str], str]
) -> SignDecision:
"""The real Sign-button gate: resolves the current TOCTOU blob SHA from
*the ref this session was actually loaded from* (`session.loaded_ref`),
never a hardcoded "main", then delegates to can_sign().
`resolve_blob_sha` is injected so this stays testable without git/Qt --
catalog_console.ReviewWindow._on_sign passes a real resolver
(fetch_ref + blob_sha_at against self.repo_dir); tests pass a fake
dict-backed lookup. This is the fix for issue #68 finding 1's first bug:
`_on_sign` used to hardcode `fetch_ref(self.repo_dir, "main")` as the
comparison ref, so for any PR review (where `loaded_ref` is the PR's
head, not main) the SHAs differed by definition and Sign could never
fire -- and the retry path re-called the same hardcoded resolver, so it
re-pinned the same wrong value and looped forever instead of forcing a
genuine re-review.
"""
current_blob_sha = resolve_blob_sha(session.loaded_ref)
return can_sign(session, current_blob_sha)
def catalog_signing_message(raw_bytes: bytes) -> bytes: def catalog_signing_message(raw_bytes: bytes) -> bytes:
"""The exact bytes that get signed: bcc_core's domain-separation prefix """The exact bytes that get signed: bcc_core's domain-separation prefix
(imported, never retyped) + the raw catalog bytes. Using this function (imported, never retyped) + the raw catalog bytes. Using this function
+32
View File
@@ -42,6 +42,25 @@ from pathlib import Path
# message signed by the same key. # message signed by the same key.
DOMAIN_PREFIX = b"bcc-release-v1|" DOMAIN_PREFIX = b"bcc-release-v1|"
# Public half of the RELEASE signing key(s) -- a SEPARATE keypair from
# bcc_core.CATALOG_PUBKEYS (issue #68 finding 5). The catalog key is the
# offline, Console-only root of trust for what BCC executes; this key is
# CI-resident and signs ONLY the release SHA256SUMS manifest, never the
# catalog. Keeping them apart means a CI/repo-secret compromise burns the
# release key -- annoying, but it never lets an attacker sign a catalog a
# user's binary would trust. A LIST (not a single key), mirroring
# CATALOG_PUBKEYS, so the release key can be rotated without invalidating
# the signature on every past release: verification accepts a match against
# ANY key here.
#
# Empty until the maintainer generates the release keypair (separately from
# the catalog keypair) and pastes the public half in:
# python catalog_console.py keygen --release
# This is intentionally NOT pre-populated with a placeholder that looks
# like a real key -- release.yml's signing-smoke-test fails closed (loudly)
# on an empty list rather than silently verifying against nothing.
RELEASE_PUBKEYS: list[bytes] = []
CHUNK_SIZE = 1024 * 1024 CHUNK_SIZE = 1024 * 1024
@@ -135,6 +154,19 @@ def public_key_b64_from_seed(seed_b64: str) -> str:
return base64.b64encode(raw).decode("ascii") return base64.b64encode(raw).decode("ascii")
def verify_checksums_against_any(pubkeys: list[bytes], sums_text: str, signature: bytes) -> bool:
"""Verify `signature` against ANY key in `pubkeys` (each a raw 32-byte
Ed25519 public key). Mirrors bcc_core.verify_catalog_signature's
rotation-friendly "any currently-trusted key" semantics, applied to
RELEASE_PUBKEYS instead of the catalog's key list. Returns False (never
raises) for an empty `pubkeys` list -- fails closed rather than
vacuously verifying against nothing."""
return any(
verify_checksums(base64.b64encode(pk).decode("ascii"), sums_text, signature)
for pk in pubkeys
)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# CLI # CLI
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
+143 -3
View File
@@ -334,13 +334,29 @@ def test_acknowledge_gating_requires_every_entry():
assert r.all_entries_acknowledged(session) is True assert r.all_entries_acknowledged(session) is True
def test_no_acknowledge_all_function_exists(): def test_no_acknowledge_all_shortcut_and_gate_is_real():
"""Deliberate: there must be no shortcut to acknowledge every entry at """Two things, both load-bearing (issue #68: the original version of
once. See the comment in catalog_review.py above SignDecision.""" this test asserted ONLY the first half, and passed the entire time the
gate below it was vacuously satisfiable -- 'no function named
acknowledge_all' is worthless if signing doesn't actually require
acknowledgement in practice).
1. No bulk-acknowledge shortcut exists (see the comment in
catalog_review.py above SignDecision -- deliberate friction).
2. The gate that friction protects is actually enforced: with entries
still unacknowledged, can_sign() must refuse, not just "some GUI
checkbox happens to be unticked".
"""
names = [n for n in dir(r) if "acknowledge" in n.lower()] names = [n for n in dir(r) if "acknowledge" in n.lower()]
assert "acknowledge_all" not in names assert "acknowledge_all" not in names
assert "acknowledge_all_entries" not in names assert "acknowledge_all_entries" not in names
session = r.start_review("sha1", _catalog(), _catalog(_entry(id="a"), _entry(id="b")))
r.acknowledge_entry(session, "a") # only one of two -- not a bulk call
decision = r.can_sign(session, "sha1")
assert decision.ok is False
assert "acknowledged" in decision.reason.lower()
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# can_sign: TOCTOU blob pinning + acknowledge gating combined # can_sign: TOCTOU blob pinning + acknowledge gating combined
@@ -377,6 +393,130 @@ def test_can_sign_blob_mismatch_takes_priority_message():
assert "blob" in decision.reason.lower() or "changed" in decision.reason.lower() assert "blob" in decision.reason.lower() or "changed" in decision.reason.lower()
def test_can_sign_false_on_empty_changeset():
"""The exact bug behind issue #68 finding 1: 'main' loaded against
itself diffs to [], and an empty changeset used to leave can_sign()
with nothing to refuse on (set() <= set() is vacuously True). Commit
b08cf21 signed 19 entries through precisely this path -- zero of them
were ever reviewed. An empty diff must mean 'nothing to sign', never
'sign unlocked'."""
same_catalog = _catalog(_entry())
session = r.start_review("sha1", same_catalog, same_catalog)
assert session.changes == [] # diff_catalogs(x, x) -> []
assert r.all_entries_acknowledged(session) is True # vacuously -- this is the trap
decision = r.can_sign(session, "sha1") # blob matches, "everything" acknowledged
assert decision.ok is False
assert "nothing to sign" in decision.reason.lower()
def test_can_sign_false_with_outstanding_blocking_risk_even_if_acknowledged():
"""can_sign() must itself refuse a blocking risk finding -- today a
blocking finding only disables the GUI checkbox, so the pure gate must
not simply trust that the caller never acknowledged a blocking entry.
Acknowledge it directly here (bypassing any GUI checkbox-disable logic
entirely) to prove the gate catches it independently of the GUI."""
session = r.start_review(
"sha1",
_catalog(),
_catalog(_entry(config={"command": "bash", "args": ["-c", "evil"]})),
)
r.acknowledge_entry(session, "filesystem")
assert r.all_entries_acknowledged(session) is True
decision = r.can_sign(session, "sha1")
assert decision.ok is False
assert "blocking" in decision.reason.lower()
# --------------------------------------------------------------------------- #
# sign_precondition: the ref-resolution seam that used to hardcode "main"
# --------------------------------------------------------------------------- #
def test_sign_precondition_resolves_against_loaded_ref_not_hardcoded_main():
"""The regression test for issue #68 finding 1's first bug:
ReviewWindow._on_sign used to hardcode fetch_ref(repo, "main") as the
TOCTOU comparison ref. For a PR review, _on_load pins the PR HEAD's
blob SHA, so comparing against main's SHA differs by definition and
Sign could never fire on the PR path.
The fake resolver below returns a DIFFERENT (deliberately wrong) SHA for
"main" than for the PR ref that was actually loaded. If
sign_precondition ever resolves against "main" instead of
session.loaded_ref, this test fails -- both via the recorded `calls`
list and via decision.ok flipping to False.
"""
pr_ref = "refs/pull/42/head"
session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
r.acknowledge_entry(session, "filesystem")
calls: list[str] = []
def fake_resolver(ref: str) -> str:
calls.append(ref)
return {"main": "main-blob-sha-WRONG", pr_ref: "pr-blob-sha"}[ref]
decision = r.sign_precondition(session, fake_resolver)
assert calls == [pr_ref] # never asked the resolver for "main"
assert decision.ok is True
assert decision.reason is None
def test_sign_precondition_refuses_when_loaded_ref_blob_moved():
"""Same seam, the negative case: if the loaded ref's blob SHA has moved
since review began (a new commit landed on the reviewed PR/branch), the
resolver reflects that and sign_precondition must refuse -- proving this
isn't just a hardcoded pass-through."""
pr_ref = "refs/pull/42/head"
session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
r.acknowledge_entry(session, "filesystem")
def fake_resolver(_ref: str) -> str:
return "pr-blob-sha-AFTER-A-NEW-PUSH"
decision = r.sign_precondition(session, fake_resolver)
assert decision.ok is False
assert "mismatch" in decision.reason.lower() or "changed" in decision.reason.lower()
def test_sign_precondition_defaults_to_main_when_loaded_ref_unset():
"""start_review()'s loaded_ref defaults to 'main' for source=main
reviews (and backward-compat with callers that don't pass it)."""
session = r.start_review("sha1", _catalog(), _catalog(_entry()))
assert session.loaded_ref == "main"
r.acknowledge_entry(session, "filesystem")
def fake_resolver(ref: str) -> str:
assert ref == "main"
return "sha1"
decision = r.sign_precondition(session, fake_resolver)
assert decision.ok is True
# --------------------------------------------------------------------------- #
# find_last_signed_catalog_raw: what source=main diffs against
# --------------------------------------------------------------------------- #
def test_find_last_signed_catalog_raw_returns_matching_candidate():
"""Simulates walking catalog.json's git history: the CURRENT signature
covers an OLDER version of the bytes (a later commit changed
catalog.json without re-signing -- the exact bypass that produced
commit b08cf21). The first candidate that verifies against that
signature is 'the last catalog a maintainer actually signed'."""
seed, pubkey = r.generate_keypair()
old_raw = b'{"schema":1,"version":1,"servers":[]}'
new_raw = b'{"schema":1,"version":2,"servers":[]}'
sig = r.sign_catalog_bytes(old_raw, seed) # signature covers the OLD bytes
found = r.find_last_signed_catalog_raw([new_raw, old_raw], sig, [pubkey])
assert found == old_raw
def test_find_last_signed_catalog_raw_none_when_nothing_verifies():
seed, _pubkey = r.generate_keypair()
_other_seed, other_pubkey = r.generate_keypair()
raw = b'{"schema":1,"version":1,"servers":[]}'
sig = r.sign_catalog_bytes(raw, seed)
# Check against a pubkey list that does NOT include the signer's key.
assert r.find_last_signed_catalog_raw([raw], sig, [other_pubkey]) is None
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# catalog_signing_message: domain separation must match bcc_core exactly # catalog_signing_message: domain separation must match bcc_core exactly
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
+679 -270
View File
File diff suppressed because it is too large Load Diff