6 Commits

Author SHA1 Message Date
BCC Agent aa40f8e139 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.
2026-07-13 13:10:36 -04:00
BCC Agent 4836c6cb48 chore(#68): rotate signing keys, key-rotation re-attestation mode, harden show-seed-b64
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.12 / windows-latest) (pull_request) Successful in 35s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Failing after 6s
Wires the maintainer's freshly-rotated signing keys into BCC (issue #68
finding 5: the old key was shared between catalog+release and had been
exposed to CI), adds a key-rotation re-attestation mode to the Catalog
Console so the Console can actually re-sign under the new key, and
hardens the show-seed-b64 CLI prompt that led to a private key being
pasted into a chat.

## New keys (#68 finding 5)

- bcc_core.CATALOG_PUBKEYS -> new CATALOG pubkey
  (0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k=). Signs data/catalog.json,
  Console-only, offline.
- .github/workflows/ci.yml EXPECTED_CATALOG_PUBKEY_B64 -> same new
  catalog pubkey (the CI trust anchor added for #68 finding 4).
- scripts/sign_checksums.py RELEASE_PUBKEYS -> new RELEASE pubkey
  (6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA=). Signs SHA256SUMS only,
  CI-resident.
- README.md 'Verifying your download' / 'Signing keys' sections filled in
  with both pubkeys, explicit about which key is which.

The old key 082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4= is retired (it
was shared and exposed to CI) and is deliberately NOT retained in either
trust list -- keeping a burned key in CATALOG_PUBKEYS would defeat the
point of rotating it.

## Key-rotation re-attestation mode (Catalog Console)

Problem: after the key swap, the existing data/catalog.json.sig (signed
with the OLD key) no longer verifies under the NEW CATALOG_PUBKEYS, but
catalog content is unchanged, so diff_catalogs(last_signed, current) is
empty -- and can_sign()'s empty-diff guard (load-bearing, #68 finding 1)
correctly refuses to sign an empty changeset. Without a rotation-aware
path, the Console could never re-sign and CI would stay red forever.

Fix: treat rotation as a full re-attestation, not a diff.

- catalog_review.py: ReviewSession/start_review gain reattest: bool =
  False. When set, changes is built via diff_catalogs(None, new_catalog)
  -- every entry presented as if newly added, requiring a fresh
  acknowledgement -- instead of diffing against old_catalog. can_sign()
  is UNCHANGED: it still refuses a genuinely-empty changeset and still
  enforces the TOCTOU blob-SHA pin and the blocking-risk check, because
  reattest sessions simply never produce an empty changeset (unless the
  catalog itself is empty).
- catalog_console.py: adds catalog_signature_valid_at(repo_dir, commit,
  raw), which calls bcc_core.verify_catalog_signature directly (never
  reimplemented) to detect whether the committed .sig verifies under the
  CURRENT CATALOG_PUBKEYS. ReviewWindow._on_load's source=main path uses
  this to decide reattest=True/False, and shows a loud, explicit red
  banner ("KEY ROTATION IN PROGRESS...") whenever reattest mode is
  entered -- never silent. Status text and Sign-button gating flow
  through the same can_sign()/all_entries_acknowledged() path as normal
  review.
- tests/test_catalog_review.py: 6 new tests covering re-attest mode
  (one change-entry per server, gating until all acknowledged, then
  permits), confirming normal mode still refuses an empty diff (rotation
  path is not a general bypass), and confirming reattest mode still
  enforces the blocking-risk check and the TOCTOU pin.

## show-seed-b64 hardening (Task 3)

The maintainer ran show-seed-b64 --release, saw an ambiguous prompt,
and pasted the printed PRIVATE seed into a chat believing it was public.

- cmd_show_seed_b64: passphrase prompt is now explicit ("Passphrase for
  the release signing key (the one YOU chose when generating it)"). A
  loud three-line warning banner prints to STDERR immediately before the
  seed ("!!! PRIVATE KEY BELOW..."); the seed itself stays alone on
  STDOUT so piping into pbcopy or a CI secret field still works cleanly.
- cmd_keygen: labels for both key kinds now say PUBLIC/PRIVATE explicitly
  and state safety properties inline (safe to commit vs. never commit),
  so the printed output can't be mistaken for the other key's.

## Verification

- ruff check . / ruff format --check .: clean.
- pytest: full suite green (360 passed, 1 skipped).
- Delete-the-check-and-watch-it-fail: each of can_sign()'s four gates
  (empty-diff, TOCTOU pin, blocking-risk, acknowledge-all) and the
  reattest branch itself were individually removed and confirmed to turn
  a test red, then restored -- see PR description for the exact
  failures.

Not touched: data/catalog.json (content-signed, maintainer's job via the
Console). No private key generated, requested, or committed. bcc.py
untouched (open PR #67).
2026-07-13 12:24:28 -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
9 changed files with 2403 additions and 437 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: "0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="
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
+35 -3
View File
@@ -90,6 +90,21 @@ key, because they protect different things and live in different places:
| Generated with | `python catalog_console.py keygen` | `python catalog_console.py keygen --release` | | 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` | | Exported for CI with | *(never — there is no supported way to export this key)* | `python catalog_console.py show-seed-b64 --release` |
**Confused about which key is which, or what state either is in?** Run:
```bash
python catalog_console.py keys
```
It needs no passphrase (it never touches private key bytes) and prints a
plain-English report for both keys: where each private half lives, whether
it's present on this machine, its fingerprint, whether that fingerprint
matches what's actually committed in `bcc_core.py`, `ci.yml`'s trust
anchor, and `scripts/sign_checksums.py`, and whether
`data/catalog.json.sig` currently verifies — ending with the exact command
to run next for whatever state it finds. This is the check that would have
caught [issue #68](../../issues/68)'s finding 5 incident before it happened.
**Why two keys:** the catalog key is the root of trust for what BCC **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 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 the shipped catalog is only there because this key signed it. If that key
@@ -110,12 +125,29 @@ refuses to run without `--release` specifically so the catalog seed can't
be exported by habit or muscle memory. be exported by habit or muscle memory.
**Release signing public key** (Ed25519, base64, raw 32 bytes) — this is **Release signing public key** (Ed25519, base64, raw 32 bytes) — this is
the RELEASE key, not the catalog key: the RELEASE key, which signs `SHA256SUMS` (release checksums). It does
**not** sign `data/catalog.json` and is not the key `bcc_core.CATALOG_PUBKEYS`
trusts:
``` ```
<PLACEHOLDER — AJ: paste the release public key from `catalog_console.py keygen --release` here> 6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA=
``` ```
The catalog public key (Ed25519, base64, raw 32 bytes) — this is the key
that signs `data/catalog.json` and is trusted via `bcc_core.CATALOG_PUBKEYS`
and the CI trust anchor in `.github/workflows/ci.yml`. It is listed here
for completeness, not because you need it to verify a download — use the
*release* key above for that:
```
0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k=
```
Both keys above were rotated 2026-07 — see [issue #68](../../issues/68)
finding 5. The prior (shared) key is retired and is deliberately **not**
kept in either trust list; retaining a burned key would defeat the point
of rotating it.
## Run from source ## Run from source
```bash ```bash
@@ -190,7 +222,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). - `catalog_console.py` / `catalog_review.py`**maintainer-only**, never shipped to users (excluded from `bcc.spec`; see `tests/test_catalog_console_packaging.py`). The Catalog Console: review + sign `data/catalog.json` (against `main`, an open PR, or the branch you have checked out — `--ref <branch>` to be explicit, e.g. mid key-rotation, so a rotation can be signed and pushed to its own branch *before* it's merged, never forcing a red `main`), generate/manage both signing keys (`keygen`, `keygen --release`), and report on their status (`keys`, no passphrase needed) — see [Signing keys](#signing-keys).
## Building from source ## Building from source
+300 -41
View File
@@ -2164,12 +2164,36 @@ def restart_claude_desktop() -> RestartResult:
# is rejected by validate_catalog() regardless of how plausible it looks. # is rejected by validate_catalog() regardless of how plausible it looks.
CATALOG_ALLOWED_COMMANDS = frozenset({"npx", "uvx", "docker", "node", "python", "python3"}) CATALOG_ALLOWED_COMMANDS = frozenset({"npx", "uvx", "docker", "node", "python", "python3"})
# Env var keys a catalog entry's config.env must never set. Every one of
# these is a loader/interpreter override that lets a value walk straight
# past CATALOG_ALLOWED_COMMANDS and the -e/--eval/-c deny-rule below: e.g.
# NODE_OPTIONS="--require /tmp/x.js" turns an allowlisted `npx` entry into
# arbitrary code execution without ever touching config.args, which is the
# only field the allowlist/deny-rules/ASCII/secret checks used to cover.
# Matched case-insensitively -- env keys are case-sensitive on POSIX, but a
# `node_options` lookalike is exactly the kind of thing this must catch.
CATALOG_DENIED_ENV_KEYS = frozenset(
{
"NODE_OPTIONS",
"PYTHONSTARTUP",
"PYTHONPATH",
"PYTHONHOME",
"LD_PRELOAD",
"LD_LIBRARY_PATH",
"DYLD_INSERT_LIBRARIES",
"DYLD_LIBRARY_PATH",
"BROWSER",
"PATH",
"NODE_REPL_EXTERNAL_MODULE",
}
)
# Ed25519 public keys allowed to sign a catalog, raw 32-byte form. A LIST # Ed25519 public keys allowed to sign a catalog, raw 32-byte form. A LIST
# (not a single key) so keys can be rotated without bricking installs that # (not a single key) so keys can be rotated without bricking installs that
# still trust an older key: verify_catalog_signature() accepts a match # still trust an older key: verify_catalog_signature() accepts a match
# against ANY key in this list. # against ANY key in this list.
CATALOG_PUBKEYS: list[bytes] = [ CATALOG_PUBKEYS: list[bytes] = [
base64.b64decode("082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="), base64.b64decode("0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="),
] ]
# Domain-separation prefix for the signed message. The signature covers # Domain-separation prefix for the signed message. The signature covers
@@ -2189,6 +2213,36 @@ _CATALOG_SECRET_ARG_RE = re.compile(r"(?i)--api[-_]?key=|--token=|--password=")
# <PLACEHOLDER>-style tokens the GUI must have the user fill in before Save. # <PLACEHOLDER>-style tokens the GUI must have the user fill in before Save.
_PLACEHOLDER_RE = re.compile(r"<[^<>\s]+>") _PLACEHOLDER_RE = re.compile(r"<[^<>\s]+>")
# A catalog entry's id becomes an mcpServers JSON key AND is interpolated
# into Qt.AutoText widgets (status bar, QMessageBox) -- an id like
# "<b>Verified</b>" renders as markup there. Not RCE, but UI spoofing, so
# ids are constrained to a plain lowercase slug.
_CATALOG_ID_RE = re.compile(r"^[a-z0-9][a-z0-9._-]{0,63}$")
# Docker flags that consume the next arg as a value (so that value must not
# be mistaken for the image reference when locating it in config.args).
_DOCKER_VALUE_FLAGS = frozenset(
{
"-e",
"--env",
"-v",
"--volume",
"-p",
"--publish",
"--name",
"-w",
"--workdir",
"-u",
"--user",
"--entrypoint",
"--network",
"--platform",
"--add-host",
"-l",
"--label",
}
)
# How many versions a single accepted catalog jump may leap in one go. Bounds # How many versions a single accepted catalog jump may leap in one go. Bounds
# a "freeze" attack: a compromised/leaked signing key claiming an absurd # a "freeze" attack: a compromised/leaked signing key claiming an absurd
# future version would otherwise permanently outrank every legitimate # future version would otherwise permanently outrank every legitimate
@@ -2263,6 +2317,121 @@ def _docker_arg_violations(tag: str, args: list[str]) -> list[str]:
return problems return problems
def _catalog_package_spec_version(spec: str) -> str | None:
"""
Extract the version pin from an npm-style package spec, or None if the
spec carries no pin.
Handles unscoped "name@version" and scoped "@scope/name@version" --
scoped names have a leading "@" that is NOT the version separator, so a
naive split on the first/only "@" misparses "@scope/pkg" (no version)
as pinned to "scope/pkg". Splitting from the right side instead is safe
for both forms because a package name may contain "@" only as the
scope's leading character.
"""
if spec.startswith("@"):
rest = spec[1:]
if "@" not in rest:
return None
_, _, version = rest.rpartition("@")
return version or None
if "@" not in spec:
return None
_, _, version = spec.rpartition("@")
return version or None
def _catalog_package_spec_pinned(spec: str) -> bool:
"""
True if `spec` carries an exact version pin. Covers npm's "name@version"
/ "@scope/name@version" and uv's documented PyPI pin forms
"name@version" and "name==version".
"""
if "==" in spec:
_, _, version = spec.partition("==")
return bool(version)
return bool(_catalog_package_spec_version(spec))
def _first_catalog_package_spec(args: list[str]) -> str | None:
"""
The first arg that could plausibly BE a package spec: skip flags
(leading "-") and <PLACEHOLDER> tokens (which can't be validated and
are filled in by the user later, never shipped by the catalog as the
package name itself). Everything after the first hit is ignored --
trailing flags, paths, and placeholders are not package specs.
"""
for a in args:
if a.startswith("-"):
continue
if _PLACEHOLDER_RE.fullmatch(a):
continue
return a
return None
def _catalog_pin_violations(tag: str, command: str, args: list[str]) -> list[str]:
"""
Version-pinning enforcement (finding #3): a catalog PR can otherwise
ship `npx -y @scope/pkg` or `docker run img:latest` and the *next*
resolve of that package/image is whatever the registry serves that day
-- outside review, outside the signature's meaning. This is the only
place that enforces pinning at runtime; catalog_review.py's
risk_unpinned_package() is a maintainer-facing hint, not a gate.
"""
if command in ("npx", "uvx"):
spec = _first_catalog_package_spec(args)
if spec is None:
return [f"{tag}: config.args must include a package spec to pin (e.g. name@1.2.3)."]
if not _catalog_package_spec_pinned(spec):
return [
f"{tag}: config.args package {spec!r} is not version-pinned; use "
"name@version, @scope/name@version, or name==version."
]
return []
if command == "docker":
image = _docker_image_ref(args)
if image is None:
return [f"{tag}: config.args docker command has no image reference to pin."]
_, sep, image_tag = image.rpartition(":")
if not sep or "/" in image_tag:
return [
f"{tag}: config.args docker image {image!r} has no explicit tag; "
"pin an exact version (not 'latest', not untagged)."
]
if image_tag == "latest":
return [
f"{tag}: config.args docker image {image!r} uses the 'latest' tag, "
"which is not allowed; pin an exact version."
]
return []
return []
def _docker_image_ref(args: list[str]) -> str | None:
"""
Locate the image reference in a `docker run ...` args list: skip the
"run" subcommand and any flags, including ones that consume the next
token as a value (-e, -v, --name, ...) so that value isn't mistaken for
the image. The first remaining positional token is the image.
"""
i = 0
if i < len(args) and args[i] == "run":
i += 1
while i < len(args):
a = args[i]
if a.startswith("-"):
if a in _DOCKER_VALUE_FLAGS and "=" not in a:
i += 2
else:
i += 1
continue
return a
return None
def _validate_catalog_config(tag: str, config) -> list[str]: def _validate_catalog_config(tag: str, config) -> list[str]:
"""Validate the `config` block of a basic-tier catalog entry.""" """Validate the `config` block of a basic-tier catalog entry."""
if not isinstance(config, dict): if not isinstance(config, dict):
@@ -2283,10 +2452,11 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
f"({', '.join(sorted(CATALOG_ALLOWED_COMMANDS))})." f"({', '.join(sorted(CATALOG_ALLOWED_COMMANDS))})."
) )
args = config.get("args") raw_args = config.get("args")
if not isinstance(args, list) or not all(isinstance(a, str) for a in args): args_ok = isinstance(raw_args, list) and all(isinstance(a, str) for a in raw_args)
args = raw_args if args_ok else []
if not args_ok:
problems.append(f"{tag}: config.args must be a list of strings.") problems.append(f"{tag}: config.args must be a list of strings.")
args = []
for a in args: for a in args:
if not a.isascii(): if not a.isascii():
@@ -2305,11 +2475,55 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
if command == "docker": if command == "docker":
problems.extend(_docker_arg_violations(tag, args)) problems.extend(_docker_arg_violations(tag, args))
# Version pinning (finding #3) -- only meaningful once command/args are
# actually well-formed; a malformed args list already got its own
# problem above and has nothing left to pin-check.
if args_ok and command in ("npx", "uvx", "docker"):
problems.extend(_catalog_pin_violations(tag, command, args))
env = config.get("env") env = config.get("env")
if env is not None and ( if env is not None:
not isinstance(env, dict) or any(not isinstance(v, str) for v in env.values()) env_ok = isinstance(env, dict) and all(
): isinstance(k, str) and isinstance(v, str) for k, v in env.items()
problems.append(f"{tag}: config.env must be an object of string values.") )
if not env_ok:
problems.append(f"{tag}: config.env must be an object of string values.")
else:
# config.env (finding #2): unlike args, env was previously
# type-checked ONLY -- no allowlist, no deny-rule, no ASCII
# check, no secret check. That made it the single easiest way
# to smuggle a payload past every other guard in this
# function: an allowlisted `command: npx` plus
# NODE_OPTIONS=--require /tmp/x.js in env walks straight past
# the command allowlist AND the -e/--eval/-c deny-rule above,
# because neither of those ever looks at env.
for key, value in env.items():
if not key.isascii():
problems.append(
f"{tag}: config.env key {key!r} must be ASCII "
"(non-ASCII code points rejected)."
)
if key.upper() in CATALOG_DENIED_ENV_KEYS:
problems.append(
f"{tag}: config.env key {key!r} is on the catalog deny-list "
"(interpreter/loader override) and is not allowed."
)
if not value.isascii():
problems.append(
f"{tag}: config.env value for {key!r} must be ASCII "
"(non-ASCII code points rejected)."
)
if _is_secret_value(value):
problems.append(
f"{tag}: config.env[{key!r}] looks like a real secret value; "
"catalog entries must never ship secret values."
)
if value != "" and not _PLACEHOLDER_RE.fullmatch(value):
problems.append(
f"{tag}: config.env[{key!r}] must be an empty string or a "
"single <PLACEHOLDER> token -- the catalog declares which env "
"vars a server needs, it never supplies their values."
)
return problems return problems
@@ -2329,6 +2543,12 @@ def _validate_catalog_entry(idx: int, entry, seen_ids: set[str]) -> list[str]:
tag = f"servers[{idx}] ({entry_id!r})" tag = f"servers[{idx}] ({entry_id!r})"
if not entry_id.isascii(): if not entry_id.isascii():
problems.append(f"{tag}: 'id' must be ASCII (non-ASCII code points rejected).") problems.append(f"{tag}: 'id' must be ASCII (non-ASCII code points rejected).")
elif not _CATALOG_ID_RE.match(entry_id):
problems.append(
f"{tag}: 'id' must match ^[a-z0-9][a-z0-9._-]{{0,63}}$ "
"(it becomes an mcpServers JSON key and is interpolated into "
"Qt.AutoText widgets)."
)
if entry_id in seen_ids: if entry_id in seen_ids:
problems.append(f"{tag}: duplicate id.") problems.append(f"{tag}: duplicate id.")
seen_ids.add(entry_id) seen_ids.add(entry_id)
@@ -2442,15 +2662,32 @@ def verify_catalog_signature(raw: bytes, sig: bytes, pubkeys: list[bytes]) -> bo
return False return False
def _verify_catalog_candidate(candidate: tuple[bytes, bytes] | None) -> tuple[dict | None, int]:
"""Verify+load+validate one (raw, sig) candidate. Returns (None, -1) on any failure."""
if not candidate:
return None, -1
raw, sig = candidate
if not verify_catalog_signature(raw, sig, CATALOG_PUBKEYS):
return None, -1
try:
data = load_catalog(raw)
except (ValueError, TypeError):
return None, -1
if validate_catalog(data):
return None, -1
return data, catalog_version(data)
def resolve_catalog( def resolve_catalog(
bundled: tuple[bytes, bytes] | None, bundled: tuple[bytes, bytes] | None,
cached: tuple[bytes, bytes] | None, cached: tuple[bytes, bytes] | None,
remote: tuple[bytes, bytes] | None, remote: tuple[bytes, bytes] | None,
floor: int = 0,
) -> dict: ) -> dict:
""" """
Pick the highest-version catalog among bundled/cached/remote. Each Pick the highest-version catalog among bundled/cached/remote. Each of
argument is either None (unavailable) or an (raw_bytes, signature_bytes) bundled/cached/remote is either None (unavailable) or an (raw_bytes,
pair. signature_bytes) pair.
🔴 SECURITY: every candidate — including `bundled`, the copy frozen into 🔴 SECURITY: every candidate — including `bundled`, the copy frozen into
this binary — is verified against CATALOG_PUBKEYS and re-validated from this binary — is verified against CATALOG_PUBKEYS and re-validated from
@@ -2461,46 +2698,68 @@ def resolve_catalog(
by virtue of being local. Signing (and checking the signature at by virtue of being local. Signing (and checking the signature at
runtime, every time) closes that. runtime, every time) closes that.
Anti-rollback: a candidate's version is never accepted if it's lower `floor` is a pure, caller-supplied lower bound (e.g. a persisted
than the best verified candidate already found in this same resolution "last accepted version" the GUI can load from disk and pass in) — this
pass — an attacker replaying an old, since-superseded signed catalog function does no storage of its own.
can't downgrade you.
Anti-freeze: a candidate whose version leaps more than Anti-rollback / anti-freeze, and WHY they apply to every candidate
_CATALOG_MAX_VERSION_JUMP past the current best is also rejected. A including the first one evaluated: the previous version of this
compromised/leaked signing key claiming an absurd future version would function only ran these checks `if best_version >= 0`, i.e. once a
otherwise permanently outrank every legitimate release from then on, candidate had already been accepted in this pass. That let the FIRST
since the resolver always prefers the highest verified version — this verified candidate through unconditionally — a signed catalog claiming
caps how far a single accepted jump can go. version=999999999 sailed straight past both guards if it happened to be
evaluated first, and rollback protection reset on every call anyway
(nothing persisted across restarts). Now both guards are anchored to
something that doesn't depend on iteration order:
- The anti-freeze cap is measured against the BUNDLED catalog's version
(verified independently, once), not against "whatever was accepted
so far in this loop." Bundled ships inside the binary, so it's the
one candidate that isn't attacker-supplied at resolve time — the
natural trust anchor. If bundled itself doesn't verify, `floor` is
the anchor instead.
- The anti-rollback floor is max(floor, bundled's version), so a
caller that persists `floor` across restarts gets real rollback
protection; a caller that doesn't still gets "never below bundled."
On a version TIE, the bundled candidate wins over cached/remote (it
previously lost ties to whichever candidate happened to be evaluated
last, silently preferring remote over bundled at equal version).
Returns the winning catalog dict, or {} if nothing verified and Returns the winning catalog dict, or {} if nothing verified and
validated. validated.
""" """
bundled_data, bundled_version = _verify_catalog_candidate(bundled)
anchor = bundled_version if bundled_version >= 0 else floor
min_accepted = max(floor, bundled_version if bundled_version >= 0 else 0)
candidates = (
("bundled", bundled_data, bundled_version),
("cached", *_verify_catalog_candidate(cached)),
("remote", *_verify_catalog_candidate(remote)),
)
best: dict = {} best: dict = {}
best_version = -1 best_version = -1
best_is_bundled = False
for candidate in (bundled, cached, remote): for source, data, version in candidates:
if not candidate: if data is None:
continue
raw, sig = candidate
if not verify_catalog_signature(raw, sig, CATALOG_PUBKEYS):
continue
try:
data = load_catalog(raw)
except (ValueError, TypeError):
continue
if validate_catalog(data):
continue continue
if version < min_accepted:
continue # anti-rollback / below the persisted floor
if version > anchor + _CATALOG_MAX_VERSION_JUMP:
continue # anti-freeze, capped against the bundled trust anchor
version = catalog_version(data) is_bundled = source == "bundled"
if best_version >= 0: better = version > best_version or (
if version < best_version: version == best_version and is_bundled and not best_is_bundled
continue # anti-rollback )
if version > best_version + _CATALOG_MAX_VERSION_JUMP: if better:
continue # anti-freeze best = data
best_version = version
best = data best_is_bundled = is_bundled
best_version = version
return best return best
+869 -380
View File
File diff suppressed because it is too large Load Diff
+278 -1
View File
@@ -16,6 +16,8 @@ new surface" recurring-bug lesson).
from __future__ import annotations from __future__ import annotations
import base64
import hashlib
import os import os
import re import re
from collections.abc import Callable from collections.abc import Callable
@@ -442,18 +444,43 @@ class ReviewSession:
from the reviewed ref on every PR review (the bug that made the PR path 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 unable to sign at all, and forced everyone onto the vacuous
main-vs-itself path instead). main-vs-itself path instead).
`reattest` marks a KEY-ROTATION re-attestation pass (issue #68 finding 5
follow-up): the currently-trusted `bcc_core.CATALOG_PUBKEYS` key changed
and the existing `data/catalog.json.sig` no longer verifies under it.
Content-wise nothing may have changed -- `diff_catalogs(old, new)` can be
genuinely empty -- but the NEW key has never vouched for any of this
catalog before, so every entry needs a first-time attestation under the
new key, not a diff against the old one. When `reattest` is set,
`changes` is built as "every entry in `new_catalog`, presented as if
newly added" (via `diff_catalogs(None, new_catalog)`) instead of a
diff against `old_catalog`, so the acknowledge-gate in `can_sign()`
requires re-reviewing everything the new key will sign -- which is the
intended cost of a key rotation, not a bypass of the empty-diff guard.
""" """
pinned_blob_sha: str pinned_blob_sha: str
old_catalog: dict old_catalog: dict
new_catalog: dict new_catalog: dict
loaded_ref: str = "main" loaded_ref: str = "main"
reattest: bool = False
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)
def __post_init__(self) -> None: def __post_init__(self) -> None:
if not self.changes: if not self.changes:
self.changes = diff_catalogs(self.old_catalog, self.new_catalog) if self.reattest:
# Every entry in the new catalog is treated as though it
# were newly added -- because, under the NEW signing key,
# it is: nothing this key signs was ever attested by it
# before. Reusing diff_catalogs(None, new) (rather than a
# bespoke code path) means the same "added" risk predicate
# (risk_new_entry) and the same EntryChange shape the rest
# of this module and the GUI already know how to render
# apply here unmodified.
self.changes = diff_catalogs(None, self.new_catalog)
else:
self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
def start_review( def start_review(
@@ -461,12 +488,14 @@ def start_review(
old_catalog: dict, old_catalog: dict,
new_catalog: dict, new_catalog: dict,
loaded_ref: str = "main", loaded_ref: str = "main",
reattest: bool = False,
) -> ReviewSession: ) -> ReviewSession:
return ReviewSession( return ReviewSession(
pinned_blob_sha=pinned_blob_sha, pinned_blob_sha=pinned_blob_sha,
old_catalog=old_catalog, old_catalog=old_catalog,
new_catalog=new_catalog, new_catalog=new_catalog,
loaded_ref=loaded_ref, loaded_ref=loaded_ref,
reattest=reattest,
) )
@@ -531,6 +560,14 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
mean "nothing to sign", never "sign unlocked". (Issue #68 finding 1; 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 this is what let commit b08cf21 sign all 19 entries with zero of them
ever reviewed.) ever reviewed.)
This gate is unaffected by `session.reattest`: a key-rotation
re-attestation session's `changes` is built from
`diff_catalogs(None, new_catalog)` (see ReviewSession), which is
empty ONLY if the catalog itself has zero entries -- a genuinely
empty catalog either way. Rotation never manufactures a non-empty
changeset out of an empty one; it just changes *what* "non-empty"
is computed against.
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing, 1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing,
from the ref that was actually reviewed -- see sign_precondition()) 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
@@ -816,3 +853,243 @@ _NON_ASCII_RE = re.compile(r"[^\x00-\x7f]")
def contains_non_ascii(s: str) -> bool: def contains_non_ascii(s: str) -> bool:
return bool(_NON_ASCII_RE.search(s)) return bool(_NON_ASCII_RE.search(s))
# --------------------------------------------------------------------------- #
# Key status reporting (issue #62/#68 follow-up: "make key handling
# comprehensible"). Pure functions only -- `catalog_console.py cmd_keys` is a
# thin printer that gathers inputs (local key caches, source-file text, the
# catalog + its .sig) and hands them here. NEVER touches private key bytes:
# every input/output here is a public key, a fingerprint, or a status string.
# --------------------------------------------------------------------------- #
def fingerprint_pubkey(pubkey: bytes) -> str:
"""Short, human-comparable fingerprint of a raw Ed25519 public key: the
first 16 hex chars of its SHA-256 digest, grouped in 4s (e.g. "3F2A 9C1B
44DE 08AA") so two fingerprints can be eyeballed for a mismatch the way a
PGP fingerprint is. Deliberately NOT the raw base64 pubkey itself in the
default short form (that's available via the full committed value in the
report) -- a fixed-width grouped hex string is easier to compare at a
glance and to read aloud/type over chat if needed. Never derived from,
and never printed alongside, any private key material.
"""
digest = hashlib.sha256(pubkey).hexdigest().upper()[:16]
return " ".join(digest[i : i + 4] for i in range(0, len(digest), 4))
_PUBKEY_LIST_B64_RE = re.compile(r'base64\.b64decode\(\s*"([^"]+)"\s*\)')
def extract_pubkey_list_literal(source_text: str, var_name: str) -> list[bytes]:
"""Best-effort extraction of a `<var_name>: list[bytes] = [...]` literal
(each entry a `base64.b64decode("...")` call, matching the exact style
bcc_core.CATALOG_PUBKEYS and scripts.sign_checksums.RELEASE_PUBKEYS are
both written in) straight out of Python source TEXT.
Deliberately a regex over text, not an import: `catalog_console.py keys`
must report on whatever ref/branch is checked out at the inspected repo
path, which may not be (and need not be) importable from the running
process's own sys.path. Returns [] if the variable isn't found in this
exact shape -- callers treat that as "nothing committed here", not an
error, since a report that can't parse a file should say so plainly
rather than crash the whole `keys` command over one malformed file.
"""
match = re.search(
rf"{re.escape(var_name)}\s*:\s*list\[bytes\]\s*=\s*\[(.*?)\]", source_text, re.DOTALL
)
if not match:
return []
keys: list[bytes] = []
for b64 in _PUBKEY_LIST_B64_RE.findall(match.group(1)):
try:
keys.append(base64.b64decode(b64))
except ValueError:
continue
return keys
_CI_TRUST_ANCHOR_RE = re.compile(r'EXPECTED_CATALOG_PUBKEY_B64:\s*"([^"]+)"')
def extract_ci_trust_anchor_pubkey(ci_yml_text: str) -> bytes | None:
"""Best-effort extraction of ci.yml's `EXPECTED_CATALOG_PUBKEY_B64` trust
anchor (issue #68 finding 4) from the workflow file's TEXT. Returns None
if the constant isn't found -- the `keys` report shows that plainly
("not found in ci.yml") rather than raising.
"""
match = _CI_TRUST_ANCHOR_RE.search(ci_yml_text)
if not match:
return None
try:
return base64.b64decode(match.group(1))
except ValueError:
return None
@dataclass(frozen=True)
class PubkeyLocationCheck:
"""One place in the source tree a key's public half is expected to be
committed, and whether the fingerprint(s) found there match the key
stored locally."""
location: str
committed_fingerprints: tuple[str, ...]
status: str # "match" | "mismatch" | "unknown" (no local key to compare against)
@dataclass(frozen=True)
class KeyStatus:
"""Everything `catalog_console.py keys` reports about ONE signing key.
Built by key_status() below; rendered by render_key_status_report().
Never carries private key material -- every field here is safe to print.
"""
kind: str # "catalog" | "release"
display_name: str # "CATALOG" | "RELEASE"
purpose: str # one-line plain-English purpose
private_key_location: str # human-readable, e.g. "on this machine, in the OS keychain"
local_exists: bool
local_fingerprint: str | None
locations: tuple[PubkeyLocationCheck, ...]
catalog_sig_status: str | None = None # "valid" | "invalid" | "missing" | None (n/a)
def key_status(
kind: str,
*,
display_name: str,
purpose: str,
private_key_location: str,
local_exists: bool,
local_pubkey: bytes | None,
locations: list[tuple[str, list[bytes]]],
catalog_sig_status: str | None = None,
) -> KeyStatus:
"""Pure assembly of a KeyStatus from already-resolved inputs (no file or
git I/O here -- that's catalog_console.py's job). `locations` is a list
of (label, committed_pubkeys) pairs, e.g.
[("bcc_core.CATALOG_PUBKEYS", [...]), ("ci.yml trust anchor", [...])],
so a key can be checked against every place its public half is expected
to be committed, independently -- this is the check that would have
caught bcc_core.CATALOG_PUBKEYS and ci.yml's trust anchor silently
drifting apart (issue #68 finding 4 was exactly that kind of drift).
"""
checks: list[PubkeyLocationCheck] = []
for label, committed_pubkeys in locations:
fps = tuple(fingerprint_pubkey(pk) for pk in committed_pubkeys)
if local_pubkey is None:
status = "unknown"
elif local_pubkey in committed_pubkeys:
status = "match"
else:
status = "mismatch"
checks.append(
PubkeyLocationCheck(location=label, committed_fingerprints=fps, status=status)
)
return KeyStatus(
kind=kind,
display_name=display_name,
purpose=purpose,
private_key_location=private_key_location,
local_exists=local_exists,
local_fingerprint=fingerprint_pubkey(local_pubkey) if local_pubkey is not None else None,
locations=tuple(checks),
catalog_sig_status=catalog_sig_status,
)
_LOCATION_STATUS_ICON = {"match": "", "mismatch": "", "unknown": "⚠️"}
_LOCATION_STATUS_VERDICT = {
"match": "MATCHES the local private key",
"mismatch": "DOES NOT MATCH the local private key",
"unknown": "cannot compare -- no local key to check against",
}
_CATALOG_SIG_STATUS_LINE = {
"valid": "✅ data/catalog.json.sig verifies under the committed CATALOG_PUBKEYS.",
"invalid": (
"❌ data/catalog.json.sig does NOT verify under the committed CATALOG_PUBKEYS -- "
"the catalog needs re-signing (Load → acknowledge all → Sign)."
),
"missing": (
"⚠️ data/catalog.json.sig is missing entirely -- the catalog has never been signed."
),
}
def recommend_next_steps(statuses: list[KeyStatus]) -> list[str]:
"""The pure "what to do next" logic behind the keys report's closing
section -- one concrete, runnable-looking instruction per problem found,
naming the exact key involved (never just "the key"). Returns a single
reassuring line if nothing needs attention."""
steps: list[str] = []
for s in statuses:
if not s.local_exists:
flag = " --release" if s.kind == "release" else ""
steps.append(
f"{s.display_name} key has never been generated on this machine -- run "
f"`python catalog_console.py keygen{flag}`."
)
continue
for loc in s.locations:
if loc.status == "mismatch":
steps.append(
f"{s.display_name} key's local fingerprint does not match "
f"{loc.location} -- update {loc.location} to the fingerprint shown "
"above (or, if this is unexpected, treat the committed key as "
"untrusted and investigate before doing anything else)."
)
elif loc.status == "unknown":
steps.append(
f"{s.display_name} key's local fingerprint could not be checked against "
f"{loc.location} -- re-run keygen (or, for an older install, unlock the "
"key once) so its public half is cached locally."
)
if s.kind == "catalog" and s.catalog_sig_status in ("invalid", "missing"):
steps.append(
"The catalog needs re-signing: run `python catalog_console.py gui --repo .` "
"and Load → acknowledge every entry → Sign. If main is red because "
"of a key rotation, load the branch with the rotation instead of main "
"(current-branch / --ref source) so the fix lands before merge."
)
if not steps:
steps.append("Everything is consistent -- no action needed.")
return steps
def render_key_status_report(statuses: list[KeyStatus]) -> str:
"""Render a full, plain-English-first key status report as one string.
`catalog_console.py cmd_keys` prints this verbatim -- the CLI is a thin
printer over this pure function, which is what makes the report's
content (not just its plumbing) unit-testable."""
lines: list[str] = []
for s in statuses:
lines.append(f"=== {s.display_name} KEY ===")
lines.append(s.purpose)
lines.append(f"Private half lives: {s.private_key_location}")
if s.local_exists and s.local_fingerprint:
lines.append(f"Exists locally: yes (fingerprint {s.local_fingerprint})")
elif s.local_exists:
lines.append("Exists locally: yes (fingerprint unknown -- re-run keygen to cache it)")
else:
lines.append("Exists locally: no")
for loc in s.locations:
icon = _LOCATION_STATUS_ICON.get(loc.status, "?")
fps = (
", ".join(loc.committed_fingerprints)
if loc.committed_fingerprints
else "(nothing committed here)"
)
verdict = _LOCATION_STATUS_VERDICT.get(loc.status, loc.status)
lines.append(f" {icon} {loc.location}: {fps} -- {verdict}")
if s.catalog_sig_status is not None:
lines.append(
f"Catalog signature: {_CATALOG_SIG_STATUS_LINE.get(s.catalog_sig_status, s.catalog_sig_status)}"
)
lines.append("")
lines.append("What to do next:")
for step in recommend_next_steps(statuses):
lines.append(f" - {step}")
return "\n".join(lines)
+10 -6
View File
@@ -53,13 +53,17 @@ DOMAIN_PREFIX = b"bcc-release-v1|"
# the signature on every past release: verification accepts a match against # the signature on every past release: verification accepts a match against
# ANY key here. # ANY key here.
# #
# Empty until the maintainer generates the release keypair (separately from # Populated by the maintainer via:
# the catalog keypair) and pastes the public half in:
# python catalog_console.py keygen --release # python catalog_console.py keygen --release
# This is intentionally NOT pre-populated with a placeholder that looks # Rotated 2026-07 (issue #68 finding 5 / #68 CI-exposure incident): the
# like a real key -- release.yml's signing-smoke-test fails closed (loudly) # original key was shared with the catalog key and had been exposed to CI,
# on an empty list rather than silently verifying against nothing. # so both keypairs were regenerated as separate, disjoint keys. This list
RELEASE_PUBKEYS: list[bytes] = [] # holds only the current release key -- if release.yml's signing-smoke-test
# ever sees this list empty, it fails closed (loudly) rather than silently
# verifying against nothing.
RELEASE_PUBKEYS: list[bytes] = [
base64.b64decode("6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA="),
]
CHUNK_SIZE = 1024 * 1024 CHUNK_SIZE = 1024 * 1024
+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"
+334
View File
@@ -491,6 +491,98 @@ def test_sign_precondition_defaults_to_main_when_loaded_ref_unset():
assert decision.ok is True assert decision.ok is True
# --------------------------------------------------------------------------- #
# ReviewSession(reattest=True): key-rotation re-attestation (issue #68
# finding 5 follow-up). After rotating bcc_core.CATALOG_PUBKEYS, the
# existing data/catalog.json.sig no longer verifies under the new key even
# though catalog CONTENT is unchanged -- diff_catalogs(old, new) would be
# empty, and an empty changeset must never unlock Sign (can_sign gate 0).
# Re-attestation mode sidesteps that correctly: instead of diffing against
# the (now-untrustworthy) last-signed content, it treats every entry as
# requiring a fresh acknowledgement, exactly like a brand-new catalog.
# --------------------------------------------------------------------------- #
def test_reattest_mode_with_unchanged_content_yields_one_change_per_entry():
same = _catalog(_entry(id="a"), _entry(id="b"), _entry(id="c"))
session = r.start_review("sha1", same, same, reattest=True)
assert len(session.changes) == 3
assert {c_.entry_id for c_ in session.changes} == {"a", "b", "c"}
# Presented "as if newly added" -- old_catalog plays no role here.
assert all(c_.status == "added" for c_ in session.changes)
assert all(c_.old is None for c_ in session.changes)
def test_reattest_mode_can_sign_refuses_until_all_acknowledged_then_permits():
same = _catalog(_entry(id="a"), _entry(id="b"), _entry(id="c"))
session = r.start_review("sha1", same, same, reattest=True)
decision = r.can_sign(session, "sha1")
assert decision.ok is False
assert "acknowledged" in decision.reason.lower()
r.acknowledge_entry(session, "a")
r.acknowledge_entry(session, "b")
decision = r.can_sign(session, "sha1")
assert decision.ok is False # "c" still outstanding
assert "acknowledged" in decision.reason.lower()
r.acknowledge_entry(session, "c")
decision = r.can_sign(session, "sha1")
assert decision.ok is True
assert decision.reason is None
def test_normal_mode_with_unchanged_content_still_refuses_empty_diff():
"""The rotation path must NOT become a general bypass of the empty-diff
guard: reattest=False (the default) against identical old/new catalogs
must behave exactly as before -- can_sign refuses with 'nothing to
sign', full stop."""
same = _catalog(_entry(id="a"), _entry(id="b"))
session = r.start_review("sha1", same, same) # reattest defaults False
assert session.changes == []
decision = r.can_sign(session, "sha1")
assert decision.ok is False
assert "nothing to sign" in decision.reason.lower()
def test_reattest_mode_still_enforces_blocking_risk_check():
"""Re-attestation must not relax the blocking-risk gate: a
disallowed-command entry blocks Sign even with every entry
acknowledged."""
cat = _catalog(
_entry(id="a"),
_entry(id="evil", config={"command": "bash", "args": ["-c", "rm -rf /"]}),
)
session = r.start_review("sha1", cat, cat, reattest=True)
assert len(session.changes) == 2
r.acknowledge_entry(session, "a")
r.acknowledge_entry(session, "evil")
assert r.all_entries_acknowledged(session) is True
decision = r.can_sign(session, "sha1")
assert decision.ok is False
assert "blocking" in decision.reason.lower()
def test_reattest_mode_still_enforces_toctou_pin():
"""Re-attestation must not relax the TOCTOU blob-SHA pin: acknowledging
everything is not enough if the bytes moved underneath the review."""
same = _catalog(_entry(id="a"))
session = r.start_review("sha1", same, same, reattest=True)
r.acknowledge_entry(session, "a")
decision = r.can_sign(session, "sha1")
assert decision.ok is True # sanity: matches when blob is unchanged
decision = r.can_sign(session, "sha2-a-new-commit-landed")
assert decision.ok is False
assert "mismatch" in decision.reason.lower() or "changed" in decision.reason.lower()
def test_reattest_defaults_to_false():
"""start_review()'s reattest parameter defaults to False -- normal
(diff-based) review is the default behaviour, never silently entered."""
session = r.start_review("sha1", _catalog(), _catalog(_entry()))
assert session.reattest is False
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# find_last_signed_catalog_raw: what source=main diffs against # find_last_signed_catalog_raw: what source=main diffs against
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
@@ -693,3 +785,245 @@ def test_contains_non_ascii_true():
def test_contains_non_ascii_false(): def test_contains_non_ascii_false():
assert r.contains_non_ascii("package") is 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
+312 -4
View File
@@ -1690,8 +1690,12 @@ def _minimal_catalog(version: int = 1) -> dict:
"official": True, "official": True,
"setup": "basic", "setup": "basic",
"config": { "config": {
# Pinned on purpose (issue #68 finding 3): an earlier
# version of this fixture used an unpinned package and
# asserted it validated clean, which enshrined the bug
# instead of catching it.
"command": "npx", "command": "npx",
"args": ["-y", "widget-mcp"], "args": ["-y", "widget-mcp@1.0.0"],
}, },
"placeholders": {}, "placeholders": {},
"env_required": {}, "env_required": {},
@@ -1952,6 +1956,213 @@ def test_validate_catalog_rejects_duplicate_ids():
assert any("duplicate id" in p for p in problems) assert any("duplicate id" in p for p in problems)
# --- validate_catalog: config.env (issue #68 finding 2) -------------------- #
def test_validate_catalog_rejects_each_denied_env_key():
for key in sorted(c.CATALOG_DENIED_ENV_KEYS):
data = _catalog_with(
{"config": {"command": "npx", "args": ["-y", "widget-mcp@1.0.0"], "env": {key: ""}}}
)
problems = c.validate_catalog(data)
assert any("deny-list" in p for p in problems), (key, problems)
def test_validate_catalog_rejects_denied_env_key_case_insensitively():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"node_options": ""},
}
}
)
problems = c.validate_catalog(data)
assert any("deny-list" in p for p in problems)
def test_validate_catalog_rejects_nonempty_nonplaceholder_env_value():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FOO_URL": "https://example.com"},
}
}
)
problems = c.validate_catalog(data)
assert any("empty string or a single <PLACEHOLDER>" in p for p in problems)
def test_validate_catalog_accepts_empty_and_placeholder_env_values():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FOO": "", "BAR_URL": "<BAR_URL>"},
}
}
)
assert c.validate_catalog(data) == []
def test_validate_catalog_rejects_non_ascii_env_key():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FÖO": ""},
}
}
)
problems = c.validate_catalog(data)
assert any("config.env key" in p and "ASCII" in p for p in problems)
def test_validate_catalog_rejects_non_ascii_env_value():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"FOO": "<Bäd>"},
}
}
)
problems = c.validate_catalog(data)
assert any("config.env value" in p and "ASCII" in p for p in problems)
def test_validate_catalog_rejects_secret_looking_env_value():
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"SOME_TOKEN": "ghp_abcdef1234567890"},
}
}
)
problems = c.validate_catalog(data)
assert any("real secret value" in p for p in problems)
def test_validate_catalog_rejects_node_options_env_walking_past_allowlist():
# The exact reproduction from issue #68 finding 2: an allowlisted
# `npx` command carrying NODE_OPTIONS in env, which previously passed
# validation and would have flowed straight into the executed
# subprocess via catalog_entry_to_paste_json().
data = _catalog_with(
{
"config": {
"command": "npx",
"args": ["-y", "widget-mcp@1.0.0"],
"env": {"NODE_OPTIONS": "--require /tmp/payload.js"},
}
}
)
problems = c.validate_catalog(data)
assert problems != []
# --- validate_catalog: version pinning (issue #68 finding 3) --------------- #
def test_validate_catalog_rejects_unpinned_npx_package():
data = _catalog_with({"config": {"command": "npx", "args": ["-y", "widget-mcp"]}})
problems = c.validate_catalog(data)
assert any("not version-pinned" in p for p in problems)
def test_validate_catalog_rejects_unpinned_scoped_npx_package():
data = _catalog_with({"config": {"command": "npx", "args": ["-y", "@scope/pkg"]}})
problems = c.validate_catalog(data)
assert any("not version-pinned" in p for p in problems)
def test_validate_catalog_accepts_pinned_scoped_npx_package():
data = _catalog_with({"config": {"command": "npx", "args": ["-y", "@scope/pkg@1.2.3"]}})
assert c.validate_catalog(data) == []
def test_validate_catalog_rejects_unpinned_uvx_package():
data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool"]}})
problems = c.validate_catalog(data)
assert any("not version-pinned" in p for p in problems)
def test_validate_catalog_accepts_uvx_at_version_pin():
data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool@1.0.0"]}})
assert c.validate_catalog(data) == []
def test_validate_catalog_accepts_uvx_double_equals_pin():
data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool==1.0.0"]}})
assert c.validate_catalog(data) == []
def test_validate_catalog_rejects_docker_latest_tag():
data = _catalog_with({"config": {"command": "docker", "args": ["run", "some/image:latest"]}})
problems = c.validate_catalog(data)
assert any("'latest'" in p for p in problems)
def test_validate_catalog_rejects_docker_untagged_image():
data = _catalog_with({"config": {"command": "docker", "args": ["run", "some/image"]}})
problems = c.validate_catalog(data)
assert any("no explicit tag" in p for p in problems)
def test_validate_catalog_accepts_pinned_docker_image_with_flags():
data = _catalog_with(
{
"config": {
"command": "docker",
"args": ["run", "-i", "--rm", "-e", "SOME_TOKEN", "some/image:1.2.3"],
}
}
)
assert c.validate_catalog(data) == []
def test_validate_catalog_does_not_pin_check_placeholders_flags_or_subcommand():
# A pinned uvx spec followed by flags and a <PLACEHOLDER> positional
# must not itself get mistaken for an unpinned package.
data = _catalog_with(
{
"config": {
"command": "uvx",
"args": ["mcp-server-git@2026.7.10", "--repository", "<REPO_PATH>"],
}
}
)
assert c.validate_catalog(data) == []
# --- validate_catalog: id constraint (issue #68 finding 7) ----------------- #
def test_validate_catalog_rejects_id_with_markup():
data = _catalog_with({"id": "<b>Verified</b>"})
problems = c.validate_catalog(data)
assert any("must match" in p for p in problems)
def test_validate_catalog_rejects_id_with_uppercase():
data = _catalog_with({"id": "Widget"})
problems = c.validate_catalog(data)
assert any("must match" in p for p in problems)
def test_validate_catalog_rejects_id_starting_with_dash():
data = _catalog_with({"id": "-widget"})
problems = c.validate_catalog(data)
assert any("must match" in p for p in problems)
def test_validate_catalog_accepts_valid_slug_id():
data = _catalog_with({"id": "widget-2.thing-ok"})
assert c.validate_catalog(data) == []
# --- resolve_catalog -------------------------------------------------------- # # --- resolve_catalog -------------------------------------------------------- #
def test_resolve_catalog_nothing_available_returns_empty_dict(): def test_resolve_catalog_nothing_available_returns_empty_dict():
assert c.resolve_catalog(None, None, None) == {} assert c.resolve_catalog(None, None, None) == {}
@@ -2003,13 +2214,93 @@ def test_resolve_catalog_rejects_absurd_version_jump(monkeypatch):
pub = priv.public_key().public_bytes_raw() pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub]) monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
cached = _signed(_minimal_catalog(version=5), priv) # The freeze attempt goes FIRST (as `cached`), the legitimate catalog
# SECOND (as `remote`) -- on purpose. Putting the good catalog first
# (as an earlier version of this test did) never exercises the
# vulnerable path: the old implementation only guarded a candidate
# against "the best accepted so far," so whichever candidate was
# evaluated FIRST got in unconditionally, uncapped. Ordering the freeze
# attempt first is what actually proves the cap holds regardless of
# evaluation order.
freeze_attempt = _signed(_minimal_catalog(version=999999), priv) freeze_attempt = _signed(_minimal_catalog(version=999999), priv)
good = _signed(_minimal_catalog(version=5), priv)
result = c.resolve_catalog(None, cached, freeze_attempt) result = c.resolve_catalog(None, freeze_attempt, good)
assert c.catalog_version(result) == 5 assert c.catalog_version(result) == 5
def test_resolve_catalog_caps_first_and_only_candidate(monkeypatch):
# issue #68 finding 6: with no bundled catalog to anchor against, a
# signed catalog claiming an absurd version must still be capped even
# when it is the ONLY candidate resolve_catalog() ever sees -- there is
# no "best so far" for it to be compared against, so the cap has to
# apply unconditionally, not "once something else has already landed."
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
freeze_attempt = _signed(_minimal_catalog(version=999999999), priv)
result = c.resolve_catalog(None, None, freeze_attempt)
assert result == {}
def test_resolve_catalog_anchors_cap_to_bundled_not_a_chained_best(monkeypatch):
# Anti-freeze must be measured against the BUNDLED version specifically,
# not against "whatever the best-so-far happens to be after each
# candidate is accepted" -- a chained anchor lets each accepted
# candidate ratchet the allowed ceiling upward, so a legitimate
# moderate bump (cached) plus a second, much larger jump (remote) can
# each individually look "within _CATALOG_MAX_VERSION_JUMP of the
# previous one" while remote is nowhere near bundled's version.
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
bundled = _signed(_minimal_catalog(version=2), priv)
cached = _signed(_minimal_catalog(version=1000), priv) # within 1000 of bundled
remote = _signed(_minimal_catalog(version=1900), priv) # within 1000 of cached,
# NOT of bundled
result = c.resolve_catalog(bundled, cached, remote)
assert c.catalog_version(result) == 1000
def test_resolve_catalog_prefers_bundled_on_version_tie(monkeypatch):
# issue #68 finding 6: on a tie the LAST candidate evaluated used to
# win, so remote silently beat bundled at equal version. Bundled --
# the copy frozen into the binary -- must win ties.
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
bundled_data = _minimal_catalog(version=5)
bundled = _signed(bundled_data, priv)
remote_data = _minimal_catalog(version=5)
remote_data["servers"][0]["display"] = "Remote Impostor"
remote = _signed(remote_data, priv)
result = c.resolve_catalog(bundled, None, remote)
assert c.catalog_version(result) == 5
assert result["servers"][0]["display"] == "Widget"
def test_resolve_catalog_floor_rejects_below_persisted_version(monkeypatch):
# `floor` is a pure parameter: the caller (eventually the GUI, from
# persisted storage) can pass a previously-accepted version, and
# nothing below it may be accepted even with no bundled catalog to
# anchor against.
priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw()
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
stale = _signed(_minimal_catalog(version=3), priv)
result = c.resolve_catalog(None, None, stale, floor=10)
assert result == {}
def test_resolve_catalog_malformed_candidate_does_not_raise(monkeypatch): def test_resolve_catalog_malformed_candidate_does_not_raise(monkeypatch):
priv = Ed25519PrivateKey.generate() priv = Ed25519PrivateKey.generate()
pub = priv.public_key().public_bytes_raw() pub = priv.public_key().public_bytes_raw()
@@ -2039,15 +2330,32 @@ def test_resolve_catalog_invalid_but_signed_candidate_is_skipped(monkeypatch):
def test_catalog_entry_to_paste_json_basic_shape(): def test_catalog_entry_to_paste_json_basic_shape():
entry = _minimal_catalog()["servers"][0] entry = _minimal_catalog()["servers"][0]
result = c.catalog_entry_to_paste_json(entry) result = c.catalog_entry_to_paste_json(entry)
assert result == {"widget": {"command": "npx", "args": ["-y", "widget-mcp"]}} assert result == {"widget": {"command": "npx", "args": ["-y", "widget-mcp@1.0.0"]}}
def test_catalog_entry_to_paste_json_includes_env_when_present(): def test_catalog_entry_to_paste_json_includes_env_when_present():
# NOTE: catalog_entry_to_paste_json() is a pure shape-converter for an
# entry that has ALREADY passed validate_catalog() -- it is correct for
# it to carry env through verbatim. The bug (issue #68 finding 2) was
# never in this function; it was that validate_catalog() let entries
# with dangerous/non-placeholder env values reach this function in the
# first place. This test now proves that boundary explicitly: a
# validation-legal env value (a <PLACEHOLDER> token) survives the
# conversion, and a value validate_catalog() would have rejected is
# confirmed rejected before it ever gets here.
entry = _minimal_catalog()["servers"][0] entry = _minimal_catalog()["servers"][0]
entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"} entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
result = c.catalog_entry_to_paste_json(entry) result = c.catalog_entry_to_paste_json(entry)
assert result["widget"]["env"] == {"GRAFANA_URL": "<GRAFANA_URL>"} assert result["widget"]["env"] == {"GRAFANA_URL": "<GRAFANA_URL>"}
catalog = _minimal_catalog()
catalog["servers"][0]["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
assert c.validate_catalog(catalog) == []
malicious = _minimal_catalog()
malicious["servers"][0]["config"]["env"] = {"NODE_OPTIONS": "--require /tmp/payload.js"}
assert c.validate_catalog(malicious) != []
def test_config_has_unfilled_placeholders_true_for_token(): def test_config_has_unfilled_placeholders_true_for_token():
cfg = {"command": "npx", "args": ["-y", "server", "<ALLOWED_DIR>"]} cfg = {"command": "npx", "args": ["-y", "server", "<ALLOWED_DIR>"]}