feat(catalog-console): a keys status command, and complete rotation without a red main (#62, #68)
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 12s
CI / Tests (py3.12 / windows-latest) (pull_request) Successful in 25s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 11s
CI / Catalog signature (pull_request) Failing after 6s

Two problems from issue #68's follow-up review:

1. The maintainer -- the only person who will ever use this tool -- cannot
   reliably tell which of the two signing keys is which or what state
   either is in. He already pasted a private key into a chat window
   because a prompt was ambiguous. That's a defect in this tool, not user
   error.

2. PR #71 rotates bcc_core.CATALOG_PUBKEYS, which makes
   data/catalog.json.sig (signed by the retired key) stop verifying and
   the CI catalog-signature job go red. The Console could previously only
   load/sign against `main`, so the only way through was to merge a red
   PR and fix main afterwards -- normalizing exactly the alarm fatigue
   this whole design exists to prevent.

Task 1 -- `python catalog_console.py keys`:
  A plain-English-first status report for BOTH keys: purpose, where the
  private half lives, whether it exists locally, its fingerprint, whether
  that fingerprint matches every place its public half is expected to be
  committed (bcc_core.CATALOG_PUBKEYS, ci.yml's trust anchor, and
  scripts/sign_checksums.RELEASE_PUBKEYS -- checked independently, since
  issue #68 finding 4 was exactly bcc_core.py and ci.yml silently
  drifting apart), and whether data/catalog.json.sig currently verifies --
  ending with the exact command to run next. Needs no passphrase and never
  touches private key bytes: a plaintext public-key cache
  (store_public_key/load_public_key) is written alongside the existing
  encrypted private blob at keygen time, precisely so this command can
  report a fingerprint without decrypting anything.

  The status/report logic (key_status, render_key_status_report,
  recommend_next_steps, fingerprint_pubkey, extract_pubkey_list_literal,
  extract_ci_trust_anchor_pubkey) is pure and lives in catalog_review.py;
  cmd_keys in catalog_console.py is a thin printer over it, per the
  project's existing pure-core/thin-GUI split.

Task 2 -- rotation completable without a red main:
  ReviewWindow now offers a "current branch" source (auto-detected via
  `current_branch()`, or --ref to name one explicitly) alongside "main"
  and open PRs. Loading it runs the exact same diff-against-last-signed /
  rotation-detection logic "main" always used (_load_own_ref, extracted
  from the old hardcoded-to-main _on_load), just parameterized on the
  ref. Signing now pushes to session.loaded_ref, never a hardcoded "main"
  (commit_and_push_signed_catalog's branch param was already there --
  only the call site was wrong). The ref-list computation itself is a
  pure function (compute_own_refs) so this seam is unit-testable without
  git or Qt. None of can_sign()'s guards (empty-diff, acknowledge-all,
  blocking-risk, TOCTOU) were touched.

  This lets a rotation branch be reviewed, re-attested (every entry,
  since the new key never vouched for any of them -- issue #68 finding 5
  follow-up), signed, and pushed to ITS OWN branch before it's ever
  merged.

Task 3 -- label the keys everywhere:
  PassphraseDialog now shows which key (CATALOG vs RELEASE) and its
  fingerprint before the passphrase field, both in its window title and
  its prompt text -- the exact ambiguity that led to a private key being
  pasted into a chat window. cmd_keygen's stored-key confirmation now
  reads "CATALOG private key encrypted..." / "RELEASE private key
  encrypted..." instead of a capitalized-lowercase kind. The reattest
  banner now says "CATALOG signing key" / "CATALOG key" throughout
  instead of "the key".

PySide6's import is now guarded (try/except -> _PYSIDE6_AVAILABLE) and
every GUI class definition that depends on it moved under
`if _PYSIDE6_AVAILABLE:`. `keygen`, `show-seed-b64`, and the new `keys`
command have no GUI dependency and now work (and are testable) in an
environment without PySide6 -- which is exactly this repo's own `test`
CI job (pytest + cryptography only, no PySide6). `gui` fails with a clear
message instead of an ImportError stack trace if it's missing.

Tests: 26 new pure-function tests in tests/test_catalog_review.py
(fingerprint_pubkey, extract_pubkey_list_literal,
extract_ci_trust_anchor_pubkey, key_status, recommend_next_steps,
render_key_status_report) and a new tests/test_catalog_console_git.py
(14 tests) covering compute_own_refs, current_branch,
commit_and_push_signed_catalog's branch targeting, and
catalog_sig_status_on_disk against real local git repos -- importing
catalog_console.py directly, proving it works without PySide6. 400
passed, 1 skipped (pre-existing). ruff check / ruff format --check clean.
This commit is contained in:
BCC Agent
2026-07-13 13:10:36 -04:00
parent 4836c6cb48
commit aa40f8e139
5 changed files with 1513 additions and 422 deletions
+215
View File
@@ -0,0 +1,215 @@
"""Tests for catalog_console.py's non-Qt git plumbing and ref-resolution
seam (issue #68 rotation-completability fix).
catalog_console.py is importable here WITHOUT PySide6 -- its Qt import is
guarded (`_PYSIDE6_AVAILABLE`) precisely so `keygen`, `show-seed-b64`,
`keys`, and this git plumbing stay usable (and testable) wherever PySide6
isn't installed, including this CI test job, which never installs it. If
PySide6 genuinely isn't importable in this environment, that itself
exercises the guard path -- see test_module_imports_without_pyside6.
"""
from __future__ import annotations
import subprocess
import sys
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import catalog_console as cc
import catalog_review as review
_SEED_CATALOG = b'{"schema": 1, "version": 1, "servers": []}'
_SEED_SIG = b"\x00" * 64
def _run(*args: str, cwd: Path) -> None:
subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True)
def _init_bare_and_clone(tmp_path: Path) -> tuple[Path, Path]:
"""A bare "origin" repo with `main` and `rotation-branch` both seeded
with a catalog + (dummy) signature, plus a working clone with `origin`
already configured -- mirroring the tokened-remote clone
catalog_console.py's git plumbing is always run against."""
origin = tmp_path / "origin.git"
_run("init", "--bare", str(origin), cwd=tmp_path)
seed = tmp_path / "seed"
_run("clone", str(origin), str(seed), cwd=tmp_path)
_run("config", "user.email", "test@example.com", cwd=seed)
_run("config", "user.name", "Test", cwd=seed)
(seed / "data").mkdir()
(seed / "data" / "catalog.json").write_bytes(_SEED_CATALOG)
(seed / "data" / "catalog.json.sig").write_bytes(_SEED_SIG)
_run("add", "-A", cwd=seed)
_run("commit", "-m", "seed", cwd=seed)
_run("push", "origin", "HEAD:refs/heads/main", cwd=seed)
_run("checkout", "-b", "rotation-branch", cwd=seed)
_run("push", "origin", "HEAD:refs/heads/rotation-branch", cwd=seed)
clone = tmp_path / "work"
_run("clone", str(origin), str(clone), cwd=tmp_path)
_run("config", "user.email", "test@example.com", cwd=clone)
_run("config", "user.name", "Test", cwd=clone)
return origin, clone
# --------------------------------------------------------------------------- #
# The module must stay importable without PySide6 -- this IS the fix that
# lets `keys`/`keygen`/`show-seed-b64` (and this whole test file) run
# somewhere PySide6 isn't installed.
# --------------------------------------------------------------------------- #
def test_module_imports_without_pyside6():
assert hasattr(cc, "_PYSIDE6_AVAILABLE")
# This CI test job never installs PySide6 (see .github/workflows/ci.yml
# "Install test dependencies": pytest + cryptography only) -- so on CI,
# this assertion is itself proof the guard is doing its job. Locally,
# where a maintainer's env DOES have PySide6, it's fine either way; the
# only real assertion this test needs is "importing the module never
# raises", which happened just by getting this far.
assert cc._PYSIDE6_AVAILABLE in (True, False)
def test_cmd_gui_fails_soft_without_pyside6(monkeypatch, capsys):
if cc._PYSIDE6_AVAILABLE:
return # nothing to prove where PySide6 IS available
import argparse
args = argparse.Namespace(repo=".", ref=None)
assert cc.cmd_gui(args) == 1
assert "PySide6" in capsys.readouterr().err
# --------------------------------------------------------------------------- #
# compute_own_refs: the PURE ref-resolution seam. No git, no Qt.
# --------------------------------------------------------------------------- #
def test_compute_own_refs_defaults_to_main_only():
assert cc.compute_own_refs(None, None) == ["main"]
def test_compute_own_refs_adds_detected_branch():
assert cc.compute_own_refs(None, "chore/68-key-rotation") == [
"main",
"chore/68-key-rotation",
]
def test_compute_own_refs_explicit_ref_overrides_detected_branch():
assert cc.compute_own_refs("explicit-branch", "detected-branch") == [
"main",
"explicit-branch",
]
def test_compute_own_refs_does_not_duplicate_main():
assert cc.compute_own_refs(None, "main") == ["main"]
assert cc.compute_own_refs("main", "some-other-branch") == ["main"]
# --------------------------------------------------------------------------- #
# current_branch: git plumbing, no Qt.
# --------------------------------------------------------------------------- #
def test_current_branch_detects_checked_out_branch(tmp_path):
_origin, clone = _init_bare_and_clone(tmp_path)
_run("fetch", "origin", "rotation-branch", cwd=clone)
_run("checkout", "-B", "rotation-branch", "origin/rotation-branch", cwd=clone)
assert cc.current_branch(clone) == "rotation-branch"
def test_current_branch_none_on_detached_head(tmp_path):
_origin, clone = _init_bare_and_clone(tmp_path)
commit = cc.fetch_ref(clone, "main")
_run("checkout", commit, cwd=clone)
assert cc.current_branch(clone) is None
# --------------------------------------------------------------------------- #
# commit_and_push_signed_catalog: MUST target the given branch, never a
# hardcoded "main" -- issue #68's completability fix. This is exactly the
# bug that, before the fix, would have made ReviewWindow._on_sign push a
# PR/branch review's signature straight to main regardless of what was
# actually reviewed.
# --------------------------------------------------------------------------- #
def test_commit_and_push_signed_catalog_targets_the_given_branch_not_main(tmp_path):
_origin, clone = _init_bare_and_clone(tmp_path)
new_raw = b'{"schema": 1, "version": 2, "servers": []}'
new_sig = b"\x01" * 64
cc.commit_and_push_signed_catalog(clone, new_raw, new_sig, branch="rotation-branch")
rotation_commit = cc.fetch_ref(clone, "rotation-branch")
rotation_raw, _sha = cc.read_catalog_at_commit(clone, rotation_commit)
assert rotation_raw == new_raw
# main on the shared origin must be COMPLETELY untouched by a sign that
# was reviewed and pushed against rotation-branch.
main_commit = cc.fetch_ref(clone, "main")
main_raw, _sha = cc.read_catalog_at_commit(clone, main_commit)
assert main_raw == _SEED_CATALOG
def test_commit_and_push_signed_catalog_still_defaults_to_main(tmp_path):
"""Backward-compatible default: callers that don't pass `branch` (there
are none left in catalog_console.py itself, but the signature keeps the
default for any other caller / test fixture) still push to main."""
_origin, clone = _init_bare_and_clone(tmp_path)
new_raw = b'{"schema": 1, "version": 2, "servers": []}'
new_sig = b"\x01" * 64
cc.commit_and_push_signed_catalog(clone, new_raw, new_sig)
main_commit = cc.fetch_ref(clone, "main")
main_raw, _sha = cc.read_catalog_at_commit(clone, main_commit)
assert main_raw == new_raw
rotation_commit = cc.fetch_ref(clone, "rotation-branch")
rotation_raw, _sha = cc.read_catalog_at_commit(clone, rotation_commit)
assert rotation_raw == _SEED_CATALOG # untouched
# --------------------------------------------------------------------------- #
# catalog_sig_status_on_disk: the check behind `keys`' "does catalog.json.sig
# currently verify?" line -- this is precisely the check that would have
# caught the current chore/68-key-rotation state (bcc_core.CATALOG_PUBKEYS
# rotated, data/catalog.json.sig still signed by the retired key).
# --------------------------------------------------------------------------- #
def test_catalog_sig_status_on_disk_valid(tmp_path):
seed, pub = review.generate_keypair()
raw = b'{"schema": 1, "version": 1, "servers": []}'
sig = review.sign_catalog_bytes(raw, seed)
(tmp_path / "data").mkdir()
(tmp_path / "data" / "catalog.json").write_bytes(raw)
(tmp_path / "data" / "catalog.json.sig").write_bytes(sig)
assert cc.catalog_sig_status_on_disk(tmp_path, [pub]) == "valid"
def test_catalog_sig_status_on_disk_invalid_when_pubkey_rotated(tmp_path):
"""The exact chore/68-key-rotation scenario: signed by an OLD key, but
the committed pubkey list now only has the NEW key."""
old_seed, _old_pub = review.generate_keypair()
_new_seed, new_pub = review.generate_keypair()
raw = b'{"schema": 1, "version": 1, "servers": []}'
sig = review.sign_catalog_bytes(raw, old_seed)
(tmp_path / "data").mkdir()
(tmp_path / "data" / "catalog.json").write_bytes(raw)
(tmp_path / "data" / "catalog.json.sig").write_bytes(sig)
assert cc.catalog_sig_status_on_disk(tmp_path, [new_pub]) == "invalid"
def test_catalog_sig_status_on_disk_missing_when_no_sig_file(tmp_path):
(tmp_path / "data").mkdir()
(tmp_path / "data" / "catalog.json").write_bytes(b"{}")
assert cc.catalog_sig_status_on_disk(tmp_path, []) == "missing"
def test_catalog_sig_status_on_disk_missing_when_no_catalog_file(tmp_path):
(tmp_path / "data").mkdir()
(tmp_path / "data" / "catalog.json.sig").write_bytes(b"\x00" * 64)
assert cc.catalog_sig_status_on_disk(tmp_path, []) == "missing"
+242
View File
@@ -785,3 +785,245 @@ def test_contains_non_ascii_true():
def test_contains_non_ascii_false():
assert r.contains_non_ascii("package") is False
# --------------------------------------------------------------------------- #
# fingerprint_pubkey
# --------------------------------------------------------------------------- #
def test_fingerprint_pubkey_is_deterministic():
pub = b"\x01" * 32
assert r.fingerprint_pubkey(pub) == r.fingerprint_pubkey(pub)
def test_fingerprint_pubkey_differs_for_different_keys():
assert r.fingerprint_pubkey(b"\x01" * 32) != r.fingerprint_pubkey(b"\x02" * 32)
def test_fingerprint_pubkey_never_contains_the_key_bytes_themselves():
pub = b"\x42" * 32
fp = r.fingerprint_pubkey(pub)
assert pub.hex() not in fp.lower().replace(" ", "")
# --------------------------------------------------------------------------- #
# extract_pubkey_list_literal / extract_ci_trust_anchor_pubkey: text parsing
# for `catalog_console.py keys`, exercised here with no file I/O.
# --------------------------------------------------------------------------- #
def test_extract_pubkey_list_literal_single_key():
_seed, pub = r.generate_keypair()
import base64
text = (
"CATALOG_PUBKEYS: list[bytes] = [\n"
f' base64.b64decode("{base64.b64encode(pub).decode()}"),\n'
"]\n"
)
assert r.extract_pubkey_list_literal(text, "CATALOG_PUBKEYS") == [pub]
def test_extract_pubkey_list_literal_multiple_keys():
import base64
pubs = [r.generate_keypair()[1] for _ in range(2)]
body = ",\n".join(f' base64.b64decode("{base64.b64encode(p).decode()}")' for p in pubs)
text = f"RELEASE_PUBKEYS: list[bytes] = [\n{body},\n]\n"
assert r.extract_pubkey_list_literal(text, "RELEASE_PUBKEYS") == pubs
def test_extract_pubkey_list_literal_missing_variable_returns_empty():
assert r.extract_pubkey_list_literal("some unrelated text", "CATALOG_PUBKEYS") == []
def test_extract_pubkey_list_literal_does_not_match_a_different_variable():
import base64
_seed, pub = r.generate_keypair()
text = f'OTHER_PUBKEYS: list[bytes] = [base64.b64decode("{base64.b64encode(pub).decode()}")]\n'
assert r.extract_pubkey_list_literal(text, "CATALOG_PUBKEYS") == []
def test_extract_ci_trust_anchor_pubkey_found():
import base64
_seed, pub = r.generate_keypair()
text = f' EXPECTED_CATALOG_PUBKEY_B64: "{base64.b64encode(pub).decode()}"\n'
assert r.extract_ci_trust_anchor_pubkey(text) == pub
def test_extract_ci_trust_anchor_pubkey_missing_returns_none():
assert r.extract_ci_trust_anchor_pubkey("no anchor here") is None
# --------------------------------------------------------------------------- #
# key_status / render_key_status_report / recommend_next_steps
# --------------------------------------------------------------------------- #
def _kw(**overrides):
base = dict(
kind="catalog",
display_name="CATALOG",
purpose="Signs the catalog.",
private_key_location="on this machine",
local_exists=True,
local_pubkey=b"\x01" * 32,
locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x01" * 32])],
catalog_sig_status="valid",
)
base.update(overrides)
return base
def test_key_status_reports_match_when_local_pubkey_in_committed_list():
status = r.key_status(**_kw())
assert status.locations[0].status == "match"
def test_key_status_reports_mismatch_when_local_pubkey_not_in_committed_list():
status = r.key_status(**_kw(locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x02" * 32])]))
assert status.locations[0].status == "mismatch"
def test_key_status_reports_unknown_when_no_local_pubkey():
status = r.key_status(**_kw(local_pubkey=None, local_exists=False))
assert status.locations[0].status == "unknown"
assert status.local_fingerprint is None
def test_key_status_never_carries_a_local_fingerprint_when_key_absent():
status = r.key_status(**_kw(local_pubkey=None, local_exists=False))
assert status.local_exists is False
assert status.local_fingerprint is None
def test_key_status_fingerprint_matches_fingerprint_pubkey_helper():
pub = b"\x03" * 32
status = r.key_status(**_kw(local_pubkey=pub, locations=[("x", [pub])]))
assert status.local_fingerprint == r.fingerprint_pubkey(pub)
def test_key_status_checks_multiple_locations_independently():
"""A key can match one committed location and mismatch another -- this
is exactly the drift issue #68 finding 4 was about (bcc_core.py and
ci.yml silently disagreeing on the trust anchor)."""
pub = b"\x04" * 32
other = b"\x05" * 32
status = r.key_status(
**_kw(
local_pubkey=pub,
locations=[
("bcc_core.CATALOG_PUBKEYS", [pub]),
("ci.yml trust anchor", [other]),
],
)
)
assert status.locations[0].status == "match"
assert status.locations[1].status == "mismatch"
def test_recommend_next_steps_flags_never_generated_key():
status = r.key_status(**_kw(local_exists=False, local_pubkey=None, locations=[]))
steps = r.recommend_next_steps([status])
assert any("keygen" in s and "CATALOG" in s for s in steps)
def test_recommend_next_steps_release_key_uses_release_flag():
status = r.key_status(
kind="release",
display_name="RELEASE",
purpose="Signs checksums.",
private_key_location="not generated yet",
local_exists=False,
local_pubkey=None,
locations=[],
)
steps = r.recommend_next_steps([status])
assert any("keygen --release" in s for s in steps)
def test_recommend_next_steps_flags_mismatch_by_location_name():
status = r.key_status(**_kw(locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x99" * 32])]))
steps = r.recommend_next_steps([status])
assert any("bcc_core.CATALOG_PUBKEYS" in s and "CATALOG" in s for s in steps)
def test_recommend_next_steps_flags_invalid_catalog_signature():
status = r.key_status(**_kw(catalog_sig_status="invalid"))
steps = r.recommend_next_steps([status])
assert any("re-signing" in s for s in steps)
def test_recommend_next_steps_flags_missing_catalog_signature():
status = r.key_status(**_kw(catalog_sig_status="missing"))
steps = r.recommend_next_steps([status])
assert any("re-signing" in s for s in steps)
def test_recommend_next_steps_all_clear_when_nothing_wrong():
status = r.key_status(**_kw())
steps = r.recommend_next_steps([status])
assert steps == ["Everything is consistent -- no action needed."]
def test_recommend_next_steps_release_key_has_no_catalog_signature_advice():
"""A mismatched RELEASE key must never trigger catalog-signing advice --
the two keys' remediation paths must not bleed into each other."""
status = r.key_status(
kind="release",
display_name="RELEASE",
purpose="Signs checksums.",
private_key_location="on this machine",
local_exists=True,
local_pubkey=b"\x06" * 32,
locations=[("scripts/sign_checksums.py RELEASE_PUBKEYS", [b"\x07" * 32])],
catalog_sig_status=None,
)
steps = r.recommend_next_steps([status])
assert not any("re-signing" in s for s in steps)
assert any("RELEASE" in s for s in steps)
def test_render_key_status_report_never_prints_private_key_material():
"""The report string must be built ONLY from public inputs. Sanity
check: no field on KeyStatus/PubkeyLocationCheck is capable of holding
private key bytes in the first place (there's no such field to leak),
and the render function only touches fields that exist -- this test
guards against a future field addition reintroducing that risk."""
status = r.key_status(**_kw())
text = r.render_key_status_report([status])
assert "CATALOG" in text
assert (
"purpose" not in text.lower() or "Signs the catalog." in text
) # sanity, not a real secret
# No 64-hex-char (or longer) run anywhere -- a raw 32-byte seed/sig
# would show up as one if it were ever accidentally interpolated in.
import re as _re
assert not _re.search(r"[0-9a-fA-F]{64,}", text)
def test_render_key_status_report_names_the_specific_key_not_generic_the_key():
status = r.key_status(**_kw())
text = r.render_key_status_report([status])
assert "CATALOG KEY" in text
assert "the key" not in text.lower()
def test_render_key_status_report_includes_catalog_signature_line_only_for_catalog():
catalog_status = r.key_status(**_kw())
release_status = r.key_status(
kind="release",
display_name="RELEASE",
purpose="Signs checksums.",
private_key_location="on this machine",
local_exists=True,
local_pubkey=b"\x08" * 32,
locations=[("scripts/sign_checksums.py RELEASE_PUBKEYS", [b"\x08" * 32])],
catalog_sig_status=None,
)
text = r.render_key_status_report([catalog_status, release_status])
assert text.count("Catalog signature:") == 1
def test_render_key_status_report_ends_with_what_to_do_next_section():
status = r.key_status(**_kw())
text = r.render_key_status_report([status])
assert "What to do next:" in text