Compare commits
10 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| aa40f8e139 | |||
| 4836c6cb48 | |||
| cd2ac2f6f8 | |||
| 26c66b7db1 | |||
| 86139100eb | |||
| 38f14deeff | |||
| 82483e693d | |||
| 6fce19cc67 | |||
| 37b3c8f5d0 | |||
| b08cf2112b |
@@ -68,3 +68,109 @@ jobs:
|
|||||||
|
|
||||||
- name: Run tests
|
- name: Run tests
|
||||||
run: python -m pytest -v
|
run: python -m pytest -v
|
||||||
|
|
||||||
|
# ── Catalog signature gate (#61) ─────────────────────────────────────────
|
||||||
|
#
|
||||||
|
# data/catalog.json is a list of command+args entries that BCC writes into
|
||||||
|
# the user's Claude config, which Claude then EXECUTES. The catalog is only
|
||||||
|
# trusted if it carries a valid Ed25519 signature from the maintainer key.
|
||||||
|
#
|
||||||
|
# The threat this gate exists for is NOT an outsider pushing to the repo —
|
||||||
|
# it is the maintainer merging a friendly-looking PR without really reading
|
||||||
|
# it. A contributor can change catalog.json but cannot produce a matching
|
||||||
|
# signature, so a blindly-merged PR lands here as a RED BUILD within a
|
||||||
|
# minute, instead of quietly riding into the next release.
|
||||||
|
#
|
||||||
|
# Public-key verification only. No secret is used or needed.
|
||||||
|
catalog-signature:
|
||||||
|
name: Catalog signature
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
|
||||||
|
- uses: actions/setup-python@v5
|
||||||
|
with:
|
||||||
|
python-version: "3.12"
|
||||||
|
|
||||||
|
- name: Install dependencies
|
||||||
|
run: pip install cryptography
|
||||||
|
|
||||||
|
# 🔴 TRUST ANCHOR — issue #68 finding 4.
|
||||||
|
#
|
||||||
|
# This step used to do `import bcc_core as c` FROM THE CHECKED-OUT PR
|
||||||
|
# BRANCH and verify the catalog against c.CATALOG_PUBKEYS — i.e. it
|
||||||
|
# trusted the public key shipped in the very diff it was reviewing. A
|
||||||
|
# PR that changed data/catalog.json AND bcc_core.CATALOG_PUBKEYS (to
|
||||||
|
# an attacker key, with a matching signature produced by the attacker's
|
||||||
|
# matching private key) went green, because there was nothing outside
|
||||||
|
# the PR's own content to check the key against. The gate's whole
|
||||||
|
# point is catching a friendly-looking PR the maintainer merges
|
||||||
|
# without really reading it — and that hole made it a two-file diff.
|
||||||
|
#
|
||||||
|
# EXPECTED_CATALOG_PUBKEY_B64 below is hardcoded HERE, in the workflow
|
||||||
|
# file, independent of whatever bcc_core.py says on the PR branch. It
|
||||||
|
# is intentionally the only line in this step that matters for
|
||||||
|
# security review: changing it changes what this gate is willing to
|
||||||
|
# trust. THIS CONSTANT IS A TRUST ANCHOR. A PR that changes this line
|
||||||
|
# in the same diff as a catalog change is exactly the attack this gate
|
||||||
|
# exists to prevent — review a change to this line on its own,
|
||||||
|
# never bundled with a catalog update.
|
||||||
|
#
|
||||||
|
# NOTE for the next key rotation: update EXPECTED_CATALOG_PUBKEY_B64
|
||||||
|
# below to the new key's base64 form, as its own reviewed change.
|
||||||
|
- name: Verify data/catalog.json.sig
|
||||||
|
env:
|
||||||
|
EXPECTED_CATALOG_PUBKEY_B64: "0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="
|
||||||
|
run: |
|
||||||
|
python - <<'PY'
|
||||||
|
import base64, os, pathlib, sys
|
||||||
|
import bcc_core as c
|
||||||
|
|
||||||
|
expected_pubkey_b64 = os.environ["EXPECTED_CATALOG_PUBKEY_B64"]
|
||||||
|
|
||||||
|
raw = pathlib.Path("data/catalog.json").read_bytes()
|
||||||
|
sig_path = pathlib.Path("data/catalog.json.sig")
|
||||||
|
|
||||||
|
if not sig_path.exists():
|
||||||
|
sys.exit("FAIL: data/catalog.json.sig is missing. The catalog must be "
|
||||||
|
"signed via the Catalog Console (#62) before it can land.")
|
||||||
|
|
||||||
|
if b"\x00" * 32 in c.CATALOG_PUBKEYS:
|
||||||
|
sys.exit("FAIL: CATALOG_PUBKEYS still holds the placeholder key.")
|
||||||
|
|
||||||
|
# Trust anchor check FIRST, before verifying anything against
|
||||||
|
# bcc_core.CATALOG_PUBKEYS: a PR is not allowed to bring its own
|
||||||
|
# key. CATALOG_PUBKEYS on the checked-out branch must be EXACTLY
|
||||||
|
# the key(s) this workflow file itself expects -- no more, no
|
||||||
|
# fewer, no substitutions.
|
||||||
|
actual_pubkeys_b64 = [base64.b64encode(k).decode() for k in c.CATALOG_PUBKEYS]
|
||||||
|
if actual_pubkeys_b64 != [expected_pubkey_b64]:
|
||||||
|
sys.exit(
|
||||||
|
"FAIL: bcc_core.CATALOG_PUBKEYS on this branch does not match the "
|
||||||
|
"trust anchor hardcoded in .github/workflows/ci.yml.\n"
|
||||||
|
f" expected: {[expected_pubkey_b64]}\n"
|
||||||
|
f" actual: {actual_pubkeys_b64}\n"
|
||||||
|
"\n"
|
||||||
|
"This PR is changing (or has changed) the catalog signing key. That "
|
||||||
|
"change must be reviewed on its own, separately from any catalog "
|
||||||
|
"content change, and the workflow's EXPECTED_CATALOG_PUBKEY_B64 "
|
||||||
|
"updated deliberately -- not accepted because it happened to match "
|
||||||
|
"whatever bcc_core.py says on this branch."
|
||||||
|
)
|
||||||
|
|
||||||
|
if not c.verify_catalog_signature(raw, sig_path.read_bytes(), c.CATALOG_PUBKEYS):
|
||||||
|
sys.exit(
|
||||||
|
"FAIL: data/catalog.json does NOT match its signature.\n"
|
||||||
|
"\n"
|
||||||
|
"The catalog changed without being re-signed. Either someone edited\n"
|
||||||
|
"it directly (a PR you merged?), or a signing pass was forgotten.\n"
|
||||||
|
"Re-review and re-sign with the Catalog Console — do not bypass this."
|
||||||
|
)
|
||||||
|
|
||||||
|
problems = c.validate_catalog(c.load_catalog(raw))
|
||||||
|
if problems:
|
||||||
|
sys.exit("FAIL: catalog failed validation:\n " + "\n ".join(problems))
|
||||||
|
|
||||||
|
print("OK: catalog signature verifies, the pubkey matches the CI trust anchor, "
|
||||||
|
"and the catalog validates clean.")
|
||||||
|
PY
|
||||||
|
|||||||
@@ -95,6 +95,92 @@ jobs:
|
|||||||
name: ${{ matrix.artifact }}
|
name: ${{ matrix.artifact }}
|
||||||
path: ${{ matrix.artifact }}
|
path: ${{ matrix.artifact }}
|
||||||
|
|
||||||
|
# ── Signing-key smoke test (workflow_dispatch only) ─────────────────────
|
||||||
|
#
|
||||||
|
# The Publish job is gated on a tag, so a manual run never exercises the
|
||||||
|
# signing step — which means a wrong/missing RELEASE_SIGNING_KEY secret
|
||||||
|
# would only be discovered at the worst possible moment: during a real
|
||||||
|
# release. This job signs a throwaway manifest with the secret and verifies
|
||||||
|
# the result against scripts/sign_checksums.RELEASE_PUBKEYS.
|
||||||
|
#
|
||||||
|
# IMPORTANT (issue #68 finding 5): this must verify against the RELEASE
|
||||||
|
# public key, never bcc_core.CATALOG_PUBKEYS. The catalog key is the
|
||||||
|
# offline, maintainer-held root of trust for what BCC executes; it must
|
||||||
|
# NEVER be compared against a value that lives in a CI secret, because
|
||||||
|
# that comparison is itself a way to smuggle a catalog-trusted key through
|
||||||
|
# CI review ("does this repo secret match the catalog key" is a question
|
||||||
|
# this workflow must never even ask). The release key is a SEPARATE
|
||||||
|
# keypair, generated via `catalog_console.py keygen --release`, that only
|
||||||
|
# ever signs release SHA256SUMS manifests -- a CI/secret compromise burns
|
||||||
|
# this key, not the catalog key.
|
||||||
|
#
|
||||||
|
# It proves the two halves of the RELEASE keypair actually match, without
|
||||||
|
# publishing anything. Run it from the Actions tab after setting or
|
||||||
|
# rotating the secret.
|
||||||
|
signing-smoke-test:
|
||||||
|
name: Signing key smoke test
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
if: github.event_name == 'workflow_dispatch'
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
|
||||||
|
- uses: actions/setup-python@v5
|
||||||
|
with:
|
||||||
|
python-version: "3.12"
|
||||||
|
|
||||||
|
- name: Install dependencies
|
||||||
|
run: pip install cryptography
|
||||||
|
|
||||||
|
- name: Sign a throwaway manifest and verify against the RELEASE pubkey
|
||||||
|
env:
|
||||||
|
RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }}
|
||||||
|
run: |
|
||||||
|
if [ -z "$RELEASE_SIGNING_KEY" ]; then
|
||||||
|
echo "FAIL: RELEASE_SIGNING_KEY secret is not set."
|
||||||
|
echo "Generate the RELEASE key (NOT the catalog key) with:"
|
||||||
|
echo " python catalog_console.py keygen --release"
|
||||||
|
echo "then add its seed under Settings -> Actions -> Secrets, via:"
|
||||||
|
echo " python catalog_console.py show-seed-b64 --release"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
mkdir -p smoke && echo "smoke test payload" > smoke/hello.txt
|
||||||
|
python3 scripts/sign_checksums.py generate smoke --out smoke/SHA256SUMS
|
||||||
|
python3 scripts/sign_checksums.py sign --sums smoke/SHA256SUMS --out smoke/SHA256SUMS.sig
|
||||||
|
python - <<'PY'
|
||||||
|
import pathlib, sys
|
||||||
|
from scripts.sign_checksums import RELEASE_PUBKEYS, verify_checksums_against_any
|
||||||
|
|
||||||
|
# Deliberately does NOT import bcc_core / CATALOG_PUBKEYS at all --
|
||||||
|
# this smoke test must never be able to compare the CI secret
|
||||||
|
# against the catalog's root of trust (issue #68 finding 5). Only
|
||||||
|
# RELEASE_PUBKEYS (scripts/sign_checksums.py) is a legitimate
|
||||||
|
# target for a CI-resident key.
|
||||||
|
if not RELEASE_PUBKEYS:
|
||||||
|
sys.exit(
|
||||||
|
"FAIL: scripts/sign_checksums.RELEASE_PUBKEYS is empty.\n"
|
||||||
|
"\n"
|
||||||
|
"Generate the release keypair with:\n"
|
||||||
|
" python catalog_console.py keygen --release\n"
|
||||||
|
"then paste the printed public key into RELEASE_PUBKEYS in\n"
|
||||||
|
"scripts/sign_checksums.py and commit that change."
|
||||||
|
)
|
||||||
|
|
||||||
|
sums = pathlib.Path("smoke/SHA256SUMS").read_text()
|
||||||
|
sig = pathlib.Path("smoke/SHA256SUMS.sig").read_bytes()
|
||||||
|
|
||||||
|
if not verify_checksums_against_any(RELEASE_PUBKEYS, sums, sig):
|
||||||
|
sys.exit(
|
||||||
|
"FAIL: the signature produced by RELEASE_SIGNING_KEY does NOT verify\n"
|
||||||
|
"against any key in scripts/sign_checksums.RELEASE_PUBKEYS.\n"
|
||||||
|
"\n"
|
||||||
|
"The secret and the shipped release public key are different keypairs.\n"
|
||||||
|
"Downloaders would reject every signature this CI produces. Re-copy the\n"
|
||||||
|
"seed from `catalog_console.py show-seed-b64 --release`, or update\n"
|
||||||
|
"RELEASE_PUBKEYS with the matching public key."
|
||||||
|
)
|
||||||
|
print("OK: RELEASE_SIGNING_KEY matches a key in RELEASE_PUBKEYS.")
|
||||||
|
PY
|
||||||
|
|
||||||
# ── Create GitHub Release with all three artifacts ──────────────────────
|
# ── Create GitHub Release with all three artifacts ──────────────────────
|
||||||
|
|
||||||
release:
|
release:
|
||||||
@@ -143,9 +229,13 @@ jobs:
|
|||||||
# checks. It does NOT remove Gatekeeper/SmartScreen warnings.
|
# checks. It does NOT remove Gatekeeper/SmartScreen warnings.
|
||||||
#
|
#
|
||||||
# The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw
|
# The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw
|
||||||
# Ed25519 seed) generated via the Catalog Console (#62). If it's not
|
# Ed25519 seed) for the RELEASE key -- a SEPARATE keypair from the
|
||||||
# set, we still publish the release — just without a .sig — rather
|
# catalog key, generated via `python catalog_console.py keygen
|
||||||
# than fail the release outright.
|
# --release` (issue #68 finding 5; #62). This key is intentionally
|
||||||
|
# CI-resident and signs ONLY this checksum manifest; it is never
|
||||||
|
# trusted to sign data/catalog.json. If it's not set, we still
|
||||||
|
# publish the release — just without a .sig — rather than fail the
|
||||||
|
# release outright.
|
||||||
- name: Check for signing key
|
- name: Check for signing key
|
||||||
id: signing
|
id: signing
|
||||||
run: |
|
run: |
|
||||||
@@ -171,7 +261,7 @@ jobs:
|
|||||||
- name: Warn — release will be unsigned
|
- name: Warn — release will be unsigned
|
||||||
if: steps.signing.outputs.has_key != 'true'
|
if: steps.signing.outputs.has_key != 'true'
|
||||||
run: |
|
run: |
|
||||||
echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Add the secret (base64 raw Ed25519 seed, generated via the Catalog Console, #62) before the next tag."
|
echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Generate the RELEASE key (python catalog_console.py keygen --release) and add its seed (python catalog_console.py show-seed-b64 --release) as this secret before the next tag."
|
||||||
|
|
||||||
- name: Create GitHub Release
|
- name: Create GitHub Release
|
||||||
uses: softprops/action-gh-release@v2
|
uses: softprops/action-gh-release@v2
|
||||||
|
|||||||
@@ -36,11 +36,10 @@ are only suppressed by a paid OS-vendor certificate, which this project
|
|||||||
doesn't have. Verifying checksums is about detecting tampering in transit or
|
doesn't have. Verifying checksums is about detecting tampering in transit or
|
||||||
on a mirror, not about vouching for the software.
|
on a mirror, not about vouching for the software.
|
||||||
|
|
||||||
**Release signing public key** (Ed25519, base64, raw 32 bytes):
|
This manifest is signed with BCC's **release key**, which is a different
|
||||||
|
key from the one that signs the MCP server catalog — see
|
||||||
```
|
[Signing keys](#signing-keys) below for why, and for the public key value
|
||||||
<PLACEHOLDER — AJ: paste the public key from the Catalog Console (#62) here>
|
to use with `--pubkey-b64` below.
|
||||||
```
|
|
||||||
|
|
||||||
### macOS / Linux
|
### macOS / Linux
|
||||||
|
|
||||||
@@ -59,7 +58,7 @@ To also verify the manifest's signature (optional, requires Python +
|
|||||||
```bash
|
```bash
|
||||||
python3 scripts/sign_checksums.py verify \
|
python3 scripts/sign_checksums.py verify \
|
||||||
--sums SHA256SUMS --sig SHA256SUMS.sig \
|
--sums SHA256SUMS --sig SHA256SUMS.sig \
|
||||||
--pubkey-b64 "<the public key above>"
|
--pubkey-b64 "<the release public key from Signing keys, below>"
|
||||||
```
|
```
|
||||||
|
|
||||||
### Windows (PowerShell)
|
### Windows (PowerShell)
|
||||||
@@ -78,6 +77,77 @@ release is missing the `.sig` file, the checksums themselves are still
|
|||||||
valid and safe to check against — the release workflow only skips signing,
|
valid and safe to check against — the release workflow only skips signing,
|
||||||
never checksum generation.
|
never checksum generation.
|
||||||
|
|
||||||
|
## Signing keys
|
||||||
|
|
||||||
|
BCC uses **two separate Ed25519 keypairs**, deliberately never the same
|
||||||
|
key, because they protect different things and live in different places:
|
||||||
|
|
||||||
|
| | Catalog key | Release key |
|
||||||
|
|---|---|---|
|
||||||
|
| Signs | `data/catalog.json` (the MCP server catalog every user's app trusts) | `SHA256SUMS` (the checksum manifest for release binaries) |
|
||||||
|
| Verified by | `bcc_core.CATALOG_PUBKEYS` | `scripts/sign_checksums.RELEASE_PUBKEYS` |
|
||||||
|
| Lives | Offline, passphrase-encrypted, maintainer's machine only (OS keychain or an encrypted file outside the repo — see the [Catalog Console](#files), issue #62) | A Gitea Actions repo secret, `RELEASE_SIGNING_KEY` — **intentionally CI-resident** |
|
||||||
|
| Generated with | `python catalog_console.py keygen` | `python catalog_console.py keygen --release` |
|
||||||
|
| Exported for CI with | *(never — there is no supported way to export this key)* | `python catalog_console.py show-seed-b64 --release` |
|
||||||
|
|
||||||
|
**Confused about which key is which, or what state either is in?** Run:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
python catalog_console.py keys
|
||||||
|
```
|
||||||
|
|
||||||
|
It needs no passphrase (it never touches private key bytes) and prints a
|
||||||
|
plain-English report for both keys: where each private half lives, whether
|
||||||
|
it's present on this machine, its fingerprint, whether that fingerprint
|
||||||
|
matches what's actually committed in `bcc_core.py`, `ci.yml`'s trust
|
||||||
|
anchor, and `scripts/sign_checksums.py`, and whether
|
||||||
|
`data/catalog.json.sig` currently verifies — ending with the exact command
|
||||||
|
to run next for whatever state it finds. This is the check that would have
|
||||||
|
caught [issue #68](../../issues/68)'s finding 5 incident before it happened.
|
||||||
|
|
||||||
|
**Why two keys:** the catalog key is the root of trust for what BCC
|
||||||
|
actually *executes* on a user's machine — every `command`/`args` pair in
|
||||||
|
the shipped catalog is only there because this key signed it. If that key
|
||||||
|
and the release-checksum key were the same (as they briefly were — see
|
||||||
|
[issue #68](../../issues/68)), then anything that can exfiltrate a Gitea
|
||||||
|
Actions secret (a malicious workflow-file PR, a compromised runner, a leaky
|
||||||
|
log) could sign a catalog every user's copy of BCC would trust, not just a
|
||||||
|
checksum manifest. Splitting them means **a CI/secret compromise burns the
|
||||||
|
release key, never the catalog key** — checksums for a future release could
|
||||||
|
be forged, which is bad, but no attacker gains the ability to make BCC run
|
||||||
|
arbitrary commands on installs that trust the catalog. That asymmetry is
|
||||||
|
the entire point of having two keys instead of one.
|
||||||
|
|
||||||
|
The catalog key is **never** meant to leave the maintainer's machine: it's
|
||||||
|
generated, stored, unlocked, and used to sign entirely inside the Catalog
|
||||||
|
Console (`catalog_console.py`), and `catalog_console.py show-seed-b64`
|
||||||
|
refuses to run without `--release` specifically so the catalog seed can't
|
||||||
|
be exported by habit or muscle memory.
|
||||||
|
|
||||||
|
**Release signing public key** (Ed25519, base64, raw 32 bytes) — this is
|
||||||
|
the RELEASE key, which signs `SHA256SUMS` (release checksums). It does
|
||||||
|
**not** sign `data/catalog.json` and is not the key `bcc_core.CATALOG_PUBKEYS`
|
||||||
|
trusts:
|
||||||
|
|
||||||
|
```
|
||||||
|
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
|
||||||
@@ -152,6 +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` (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
|
||||||
|
|
||||||
|
|||||||
+298
-38
@@ -13,6 +13,7 @@ in its original position.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import base64
|
||||||
import contextlib
|
import contextlib
|
||||||
import difflib
|
import difflib
|
||||||
import functools
|
import functools
|
||||||
@@ -2163,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] = [
|
||||||
b"\x00" * 32, # TODO: real key from Catalog Console (#62)
|
base64.b64decode("0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="),
|
||||||
]
|
]
|
||||||
|
|
||||||
# Domain-separation prefix for the signed message. The signature covers
|
# Domain-separation prefix for the signed message. The signature covers
|
||||||
@@ -2188,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
|
||||||
@@ -2262,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):
|
||||||
@@ -2282,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():
|
||||||
@@ -2304,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()
|
||||||
|
)
|
||||||
|
if not env_ok:
|
||||||
problems.append(f"{tag}: config.env must be an object of string values.")
|
problems.append(f"{tag}: config.env must be an object of string values.")
|
||||||
|
else:
|
||||||
|
# config.env (finding #2): unlike args, env was previously
|
||||||
|
# type-checked ONLY -- no allowlist, no deny-rule, no ASCII
|
||||||
|
# check, no secret check. That made it the single easiest way
|
||||||
|
# to smuggle a payload past every other guard in this
|
||||||
|
# function: an allowlisted `command: npx` plus
|
||||||
|
# NODE_OPTIONS=--require /tmp/x.js in env walks straight past
|
||||||
|
# the command allowlist AND the -e/--eval/-c deny-rule above,
|
||||||
|
# because neither of those ever looks at env.
|
||||||
|
for key, value in env.items():
|
||||||
|
if not key.isascii():
|
||||||
|
problems.append(
|
||||||
|
f"{tag}: config.env key {key!r} must be ASCII "
|
||||||
|
"(non-ASCII code points rejected)."
|
||||||
|
)
|
||||||
|
if key.upper() in CATALOG_DENIED_ENV_KEYS:
|
||||||
|
problems.append(
|
||||||
|
f"{tag}: config.env key {key!r} is on the catalog deny-list "
|
||||||
|
"(interpreter/loader override) and is not allowed."
|
||||||
|
)
|
||||||
|
if not value.isascii():
|
||||||
|
problems.append(
|
||||||
|
f"{tag}: config.env value for {key!r} must be ASCII "
|
||||||
|
"(non-ASCII code points rejected)."
|
||||||
|
)
|
||||||
|
if _is_secret_value(value):
|
||||||
|
problems.append(
|
||||||
|
f"{tag}: config.env[{key!r}] looks like a real secret value; "
|
||||||
|
"catalog entries must never ship secret values."
|
||||||
|
)
|
||||||
|
if value != "" and not _PLACEHOLDER_RE.fullmatch(value):
|
||||||
|
problems.append(
|
||||||
|
f"{tag}: config.env[{key!r}] must be an empty string or a "
|
||||||
|
"single <PLACEHOLDER> token -- the catalog declares which env "
|
||||||
|
"vars a server needs, it never supplies their values."
|
||||||
|
)
|
||||||
|
|
||||||
return problems
|
return problems
|
||||||
|
|
||||||
@@ -2328,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)
|
||||||
@@ -2441,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
|
||||||
@@ -2460,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 = data
|
||||||
best_version = version
|
best_version = version
|
||||||
|
best_is_bundled = is_bundled
|
||||||
|
|
||||||
return best
|
return best
|
||||||
|
|
||||||
|
|||||||
+773
-83
File diff suppressed because it is too large
Load Diff
+374
-6
@@ -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
|
||||||
@@ -24,6 +26,7 @@ from urllib.parse import urlsplit
|
|||||||
|
|
||||||
from bcc_core import _CATALOG_SIG_DOMAIN as CATALOG_SIG_DOMAIN
|
from bcc_core import _CATALOG_SIG_DOMAIN as CATALOG_SIG_DOMAIN
|
||||||
from bcc_core import CATALOG_ALLOWED_COMMANDS
|
from bcc_core import CATALOG_ALLOWED_COMMANDS
|
||||||
|
from bcc_core import verify_catalog_signature as _verify_catalog_signature
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Semantic diff
|
# Semantic diff
|
||||||
@@ -432,25 +435,89 @@ def has_blocking_risk(change: EntryChange) -> bool:
|
|||||||
class ReviewSession:
|
class ReviewSession:
|
||||||
"""State for one review pass. `pinned_blob_sha` is the git blob SHA of
|
"""State for one review pass. `pinned_blob_sha` is the git blob SHA of
|
||||||
data/catalog.json as it existed the moment review began -- see
|
data/catalog.json as it existed the moment review began -- see
|
||||||
can_sign()."""
|
can_sign()/sign_precondition().
|
||||||
|
|
||||||
|
`loaded_ref` is the exact ref this review was loaded from ("main", or a
|
||||||
|
PR's `refs/pull/<n>/head`) -- see issue #68 finding 1. It exists so the
|
||||||
|
Sign path can re-resolve the TOCTOU blob SHA from *the ref that was
|
||||||
|
actually reviewed*, instead of a hardcoded "main" that silently diverges
|
||||||
|
from the reviewed ref on every PR review (the bug that made the PR path
|
||||||
|
unable to sign at all, and forced everyone onto the vacuous
|
||||||
|
main-vs-itself path instead).
|
||||||
|
|
||||||
|
`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"
|
||||||
|
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:
|
||||||
|
if self.reattest:
|
||||||
|
# Every entry in the new catalog is treated as though it
|
||||||
|
# were newly added -- because, under the NEW signing key,
|
||||||
|
# it is: nothing this key signs was ever attested by it
|
||||||
|
# before. Reusing diff_catalogs(None, new) (rather than a
|
||||||
|
# bespoke code path) means the same "added" risk predicate
|
||||||
|
# (risk_new_entry) and the same EntryChange shape the rest
|
||||||
|
# of this module and the GUI already know how to render
|
||||||
|
# apply here unmodified.
|
||||||
|
self.changes = diff_catalogs(None, self.new_catalog)
|
||||||
|
else:
|
||||||
self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
|
self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
|
||||||
|
|
||||||
|
|
||||||
def start_review(pinned_blob_sha: str, old_catalog: dict, new_catalog: dict) -> ReviewSession:
|
def start_review(
|
||||||
|
pinned_blob_sha: str,
|
||||||
|
old_catalog: dict,
|
||||||
|
new_catalog: dict,
|
||||||
|
loaded_ref: str = "main",
|
||||||
|
reattest: bool = False,
|
||||||
|
) -> ReviewSession:
|
||||||
return ReviewSession(
|
return ReviewSession(
|
||||||
pinned_blob_sha=pinned_blob_sha, old_catalog=old_catalog, new_catalog=new_catalog
|
pinned_blob_sha=pinned_blob_sha,
|
||||||
|
old_catalog=old_catalog,
|
||||||
|
new_catalog=new_catalog,
|
||||||
|
loaded_ref=loaded_ref,
|
||||||
|
reattest=reattest,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def find_last_signed_catalog_raw(
|
||||||
|
candidates: list[bytes], sig: bytes, pubkeys: list[bytes]
|
||||||
|
) -> bytes | None:
|
||||||
|
"""Given `candidates` (candidate raw catalog.json byte-strings -- e.g.
|
||||||
|
successive historical versions from git log, most-recent-first),
|
||||||
|
return the first one whose signature verifies against `sig`/`pubkeys`,
|
||||||
|
or None if none do.
|
||||||
|
|
||||||
|
This is how source="main" review diffs against "the last catalog a
|
||||||
|
maintainer actually signed" instead of against itself (issue #68
|
||||||
|
finding 1): `catalog_console.last_signed_catalog_raw` walks
|
||||||
|
data/catalog.json's git history on main and hands the candidates here.
|
||||||
|
"""
|
||||||
|
for raw in candidates:
|
||||||
|
if _verify_catalog_signature(raw, sig, pubkeys):
|
||||||
|
return raw
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def acknowledge_entry(session: ReviewSession, entry_id: str) -> None:
|
def acknowledge_entry(session: ReviewSession, entry_id: str) -> None:
|
||||||
ids = {c.entry_id for c in session.changes}
|
ids = {c.entry_id for c in session.changes}
|
||||||
if entry_id not in ids:
|
if entry_id not in ids:
|
||||||
@@ -483,22 +550,61 @@ class SignDecision:
|
|||||||
def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
|
def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
|
||||||
"""Whether the Sign button may fire right now.
|
"""Whether the Sign button may fire right now.
|
||||||
|
|
||||||
Two independent gates, both required:
|
Four independent gates, all required, checked in this order:
|
||||||
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing)
|
|
||||||
|
0. The diff must be non-empty. An empty diff historically meant "Sign
|
||||||
|
unlocks instantly" (`set() <= set()` is vacuously True), which is
|
||||||
|
exactly backwards: a vacuously-satisfied gate is worse than no gate
|
||||||
|
at all, because it *manufactures confidence* -- the signature looks
|
||||||
|
identical to one produced by a real review. "Nothing changed" must
|
||||||
|
mean "nothing to sign", never "sign unlocked". (Issue #68 finding 1;
|
||||||
|
this is what let commit b08cf21 sign all 19 entries with zero of them
|
||||||
|
ever reviewed.)
|
||||||
|
|
||||||
|
This gate is unaffected by `session.reattest`: a key-rotation
|
||||||
|
re-attestation session's `changes` is built from
|
||||||
|
`diff_catalogs(None, new_catalog)` (see ReviewSession), which is
|
||||||
|
empty ONLY if the catalog itself has zero entries -- a genuinely
|
||||||
|
empty catalog either way. Rotation never manufactures a non-empty
|
||||||
|
changeset out of an empty one; it just changes *what* "non-empty"
|
||||||
|
is computed against.
|
||||||
|
1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing,
|
||||||
|
from the ref that was actually reviewed -- see sign_precondition())
|
||||||
must match the blob SHA pinned when review began. If the bytes on the
|
must match the blob SHA pinned when review began. If the bytes on the
|
||||||
remote changed since -- a new commit pushed to the same PR, a
|
remote changed since -- a new commit pushed to the same PR, a
|
||||||
force-push, another PR merged in between -- signing is refused and a
|
force-push, another PR merged in between -- signing is refused and a
|
||||||
re-review is forced. This is what makes "signing is the approval act"
|
re-review is forced. This is what makes "signing is the approval act"
|
||||||
true rather than aspirational: the signature is bound to the exact
|
true rather than aspirational: the signature is bound to the exact
|
||||||
reviewed bytes, not to "whatever the file happens to be now".
|
reviewed bytes, not to "whatever the file happens to be now".
|
||||||
2. Every changed entry in the diff must be individually acknowledged.
|
2. No blocking risk finding may be outstanding on ANY changed entry, full
|
||||||
|
stop -- checked here, not just in the GUI. The GUI additionally
|
||||||
|
disables the acknowledge checkbox for a blocking entry, but that is a
|
||||||
|
UI nicety, not the enforcement point: if this pure gate didn't also
|
||||||
|
check it, a blocking risk would only be stopped by the GUI happening
|
||||||
|
to have wired the checkbox correctly, and nothing would catch a
|
||||||
|
regression in that wiring. The GUI must not be the only thing
|
||||||
|
standing between a blocking risk and a signature.
|
||||||
|
3. Every changed entry in the diff must be individually acknowledged.
|
||||||
"""
|
"""
|
||||||
|
if not session.changes:
|
||||||
|
return SignDecision(
|
||||||
|
False,
|
||||||
|
"Nothing to sign: this review's diff is empty. If you expected "
|
||||||
|
"changes here, you may be diffing the wrong source/ref.",
|
||||||
|
)
|
||||||
if current_blob_sha != session.pinned_blob_sha:
|
if current_blob_sha != session.pinned_blob_sha:
|
||||||
return SignDecision(
|
return SignDecision(
|
||||||
False,
|
False,
|
||||||
"The reviewed bytes changed since this review began (blob SHA "
|
"The reviewed bytes changed since this review began (blob SHA "
|
||||||
"mismatch) -- re-review required before signing.",
|
"mismatch) -- re-review required before signing.",
|
||||||
)
|
)
|
||||||
|
blocking_ids = sorted({c.entry_id for c in session.changes if has_blocking_risk(c)})
|
||||||
|
if blocking_ids:
|
||||||
|
return SignDecision(
|
||||||
|
False,
|
||||||
|
"Blocking risk finding(s) outstanding on: "
|
||||||
|
f"{', '.join(blocking_ids)} -- fix the underlying change, do not sign around it.",
|
||||||
|
)
|
||||||
if not all_entries_acknowledged(session):
|
if not all_entries_acknowledged(session):
|
||||||
pending = sorted({c.entry_id for c in session.changes} - session.acknowledged)
|
pending = sorted({c.entry_id for c in session.changes} - session.acknowledged)
|
||||||
return SignDecision(
|
return SignDecision(
|
||||||
@@ -507,6 +613,28 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
|
|||||||
return SignDecision(True, None)
|
return SignDecision(True, None)
|
||||||
|
|
||||||
|
|
||||||
|
def sign_precondition(
|
||||||
|
session: ReviewSession, resolve_blob_sha: Callable[[str], str]
|
||||||
|
) -> SignDecision:
|
||||||
|
"""The real Sign-button gate: resolves the current TOCTOU blob SHA from
|
||||||
|
*the ref this session was actually loaded from* (`session.loaded_ref`),
|
||||||
|
never a hardcoded "main", then delegates to can_sign().
|
||||||
|
|
||||||
|
`resolve_blob_sha` is injected so this stays testable without git/Qt --
|
||||||
|
catalog_console.ReviewWindow._on_sign passes a real resolver
|
||||||
|
(fetch_ref + blob_sha_at against self.repo_dir); tests pass a fake
|
||||||
|
dict-backed lookup. This is the fix for issue #68 finding 1's first bug:
|
||||||
|
`_on_sign` used to hardcode `fetch_ref(self.repo_dir, "main")` as the
|
||||||
|
comparison ref, so for any PR review (where `loaded_ref` is the PR's
|
||||||
|
head, not main) the SHAs differed by definition and Sign could never
|
||||||
|
fire -- and the retry path re-called the same hardcoded resolver, so it
|
||||||
|
re-pinned the same wrong value and looped forever instead of forcing a
|
||||||
|
genuine re-review.
|
||||||
|
"""
|
||||||
|
current_blob_sha = resolve_blob_sha(session.loaded_ref)
|
||||||
|
return can_sign(session, current_blob_sha)
|
||||||
|
|
||||||
|
|
||||||
def catalog_signing_message(raw_bytes: bytes) -> bytes:
|
def catalog_signing_message(raw_bytes: bytes) -> bytes:
|
||||||
"""The exact bytes that get signed: bcc_core's domain-separation prefix
|
"""The exact bytes that get signed: bcc_core's domain-separation prefix
|
||||||
(imported, never retyped) + the raw catalog bytes. Using this function
|
(imported, never retyped) + the raw catalog bytes. Using this function
|
||||||
@@ -725,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)
|
||||||
|
|||||||
@@ -0,0 +1,2 @@
|
|||||||
|
ы<8¶ђt2ішл„»‰/НЕ0Тjcw&`
|
||||||
|
тrH«MўК›єrBL,0AS€!Х2–иже.SТ°ч–'Agm
|
||||||
@@ -42,6 +42,29 @@ from pathlib import Path
|
|||||||
# message signed by the same key.
|
# message signed by the same key.
|
||||||
DOMAIN_PREFIX = b"bcc-release-v1|"
|
DOMAIN_PREFIX = b"bcc-release-v1|"
|
||||||
|
|
||||||
|
# Public half of the RELEASE signing key(s) -- a SEPARATE keypair from
|
||||||
|
# bcc_core.CATALOG_PUBKEYS (issue #68 finding 5). The catalog key is the
|
||||||
|
# offline, Console-only root of trust for what BCC executes; this key is
|
||||||
|
# CI-resident and signs ONLY the release SHA256SUMS manifest, never the
|
||||||
|
# catalog. Keeping them apart means a CI/repo-secret compromise burns the
|
||||||
|
# release key -- annoying, but it never lets an attacker sign a catalog a
|
||||||
|
# user's binary would trust. A LIST (not a single key), mirroring
|
||||||
|
# CATALOG_PUBKEYS, so the release key can be rotated without invalidating
|
||||||
|
# the signature on every past release: verification accepts a match against
|
||||||
|
# ANY key here.
|
||||||
|
#
|
||||||
|
# Populated by the maintainer via:
|
||||||
|
# python catalog_console.py keygen --release
|
||||||
|
# Rotated 2026-07 (issue #68 finding 5 / #68 CI-exposure incident): the
|
||||||
|
# original key was shared with the catalog key and had been exposed to CI,
|
||||||
|
# so both keypairs were regenerated as separate, disjoint keys. This list
|
||||||
|
# holds only the current release key -- if release.yml's signing-smoke-test
|
||||||
|
# ever sees this list empty, it fails closed (loudly) rather than silently
|
||||||
|
# verifying against nothing.
|
||||||
|
RELEASE_PUBKEYS: list[bytes] = [
|
||||||
|
base64.b64decode("6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA="),
|
||||||
|
]
|
||||||
|
|
||||||
CHUNK_SIZE = 1024 * 1024
|
CHUNK_SIZE = 1024 * 1024
|
||||||
|
|
||||||
|
|
||||||
@@ -135,6 +158,19 @@ def public_key_b64_from_seed(seed_b64: str) -> str:
|
|||||||
return base64.b64encode(raw).decode("ascii")
|
return base64.b64encode(raw).decode("ascii")
|
||||||
|
|
||||||
|
|
||||||
|
def verify_checksums_against_any(pubkeys: list[bytes], sums_text: str, signature: bytes) -> bool:
|
||||||
|
"""Verify `signature` against ANY key in `pubkeys` (each a raw 32-byte
|
||||||
|
Ed25519 public key). Mirrors bcc_core.verify_catalog_signature's
|
||||||
|
rotation-friendly "any currently-trusted key" semantics, applied to
|
||||||
|
RELEASE_PUBKEYS instead of the catalog's key list. Returns False (never
|
||||||
|
raises) for an empty `pubkeys` list -- fails closed rather than
|
||||||
|
vacuously verifying against nothing."""
|
||||||
|
return any(
|
||||||
|
verify_checksums(base64.b64encode(pk).decode("ascii"), sums_text, signature)
|
||||||
|
for pk in pubkeys
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# CLI
|
# CLI
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
|
|||||||
@@ -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,13 +334,29 @@ def test_acknowledge_gating_requires_every_entry():
|
|||||||
assert r.all_entries_acknowledged(session) is True
|
assert r.all_entries_acknowledged(session) is True
|
||||||
|
|
||||||
|
|
||||||
def test_no_acknowledge_all_function_exists():
|
def test_no_acknowledge_all_shortcut_and_gate_is_real():
|
||||||
"""Deliberate: there must be no shortcut to acknowledge every entry at
|
"""Two things, both load-bearing (issue #68: the original version of
|
||||||
once. See the comment in catalog_review.py above SignDecision."""
|
this test asserted ONLY the first half, and passed the entire time the
|
||||||
|
gate below it was vacuously satisfiable -- 'no function named
|
||||||
|
acknowledge_all' is worthless if signing doesn't actually require
|
||||||
|
acknowledgement in practice).
|
||||||
|
|
||||||
|
1. No bulk-acknowledge shortcut exists (see the comment in
|
||||||
|
catalog_review.py above SignDecision -- deliberate friction).
|
||||||
|
2. The gate that friction protects is actually enforced: with entries
|
||||||
|
still unacknowledged, can_sign() must refuse, not just "some GUI
|
||||||
|
checkbox happens to be unticked".
|
||||||
|
"""
|
||||||
names = [n for n in dir(r) if "acknowledge" in n.lower()]
|
names = [n for n in dir(r) if "acknowledge" in n.lower()]
|
||||||
assert "acknowledge_all" not in names
|
assert "acknowledge_all" not in names
|
||||||
assert "acknowledge_all_entries" not in names
|
assert "acknowledge_all_entries" not in names
|
||||||
|
|
||||||
|
session = r.start_review("sha1", _catalog(), _catalog(_entry(id="a"), _entry(id="b")))
|
||||||
|
r.acknowledge_entry(session, "a") # only one of two -- not a bulk call
|
||||||
|
decision = r.can_sign(session, "sha1")
|
||||||
|
assert decision.ok is False
|
||||||
|
assert "acknowledged" in decision.reason.lower()
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# can_sign: TOCTOU blob pinning + acknowledge gating combined
|
# can_sign: TOCTOU blob pinning + acknowledge gating combined
|
||||||
@@ -377,6 +393,222 @@ def test_can_sign_blob_mismatch_takes_priority_message():
|
|||||||
assert "blob" in decision.reason.lower() or "changed" in decision.reason.lower()
|
assert "blob" in decision.reason.lower() or "changed" in decision.reason.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_can_sign_false_on_empty_changeset():
|
||||||
|
"""The exact bug behind issue #68 finding 1: 'main' loaded against
|
||||||
|
itself diffs to [], and an empty changeset used to leave can_sign()
|
||||||
|
with nothing to refuse on (set() <= set() is vacuously True). Commit
|
||||||
|
b08cf21 signed 19 entries through precisely this path -- zero of them
|
||||||
|
were ever reviewed. An empty diff must mean 'nothing to sign', never
|
||||||
|
'sign unlocked'."""
|
||||||
|
same_catalog = _catalog(_entry())
|
||||||
|
session = r.start_review("sha1", same_catalog, same_catalog)
|
||||||
|
assert session.changes == [] # diff_catalogs(x, x) -> []
|
||||||
|
assert r.all_entries_acknowledged(session) is True # vacuously -- this is the trap
|
||||||
|
decision = r.can_sign(session, "sha1") # blob matches, "everything" acknowledged
|
||||||
|
assert decision.ok is False
|
||||||
|
assert "nothing to sign" in decision.reason.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_can_sign_false_with_outstanding_blocking_risk_even_if_acknowledged():
|
||||||
|
"""can_sign() must itself refuse a blocking risk finding -- today a
|
||||||
|
blocking finding only disables the GUI checkbox, so the pure gate must
|
||||||
|
not simply trust that the caller never acknowledged a blocking entry.
|
||||||
|
Acknowledge it directly here (bypassing any GUI checkbox-disable logic
|
||||||
|
entirely) to prove the gate catches it independently of the GUI."""
|
||||||
|
session = r.start_review(
|
||||||
|
"sha1",
|
||||||
|
_catalog(),
|
||||||
|
_catalog(_entry(config={"command": "bash", "args": ["-c", "evil"]})),
|
||||||
|
)
|
||||||
|
r.acknowledge_entry(session, "filesystem")
|
||||||
|
assert r.all_entries_acknowledged(session) is True
|
||||||
|
decision = r.can_sign(session, "sha1")
|
||||||
|
assert decision.ok is False
|
||||||
|
assert "blocking" in decision.reason.lower()
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# sign_precondition: the ref-resolution seam that used to hardcode "main"
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_sign_precondition_resolves_against_loaded_ref_not_hardcoded_main():
|
||||||
|
"""The regression test for issue #68 finding 1's first bug:
|
||||||
|
ReviewWindow._on_sign used to hardcode fetch_ref(repo, "main") as the
|
||||||
|
TOCTOU comparison ref. For a PR review, _on_load pins the PR HEAD's
|
||||||
|
blob SHA, so comparing against main's SHA differs by definition and
|
||||||
|
Sign could never fire on the PR path.
|
||||||
|
|
||||||
|
The fake resolver below returns a DIFFERENT (deliberately wrong) SHA for
|
||||||
|
"main" than for the PR ref that was actually loaded. If
|
||||||
|
sign_precondition ever resolves against "main" instead of
|
||||||
|
session.loaded_ref, this test fails -- both via the recorded `calls`
|
||||||
|
list and via decision.ok flipping to False.
|
||||||
|
"""
|
||||||
|
pr_ref = "refs/pull/42/head"
|
||||||
|
session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
|
||||||
|
r.acknowledge_entry(session, "filesystem")
|
||||||
|
|
||||||
|
calls: list[str] = []
|
||||||
|
|
||||||
|
def fake_resolver(ref: str) -> str:
|
||||||
|
calls.append(ref)
|
||||||
|
return {"main": "main-blob-sha-WRONG", pr_ref: "pr-blob-sha"}[ref]
|
||||||
|
|
||||||
|
decision = r.sign_precondition(session, fake_resolver)
|
||||||
|
assert calls == [pr_ref] # never asked the resolver for "main"
|
||||||
|
assert decision.ok is True
|
||||||
|
assert decision.reason is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_sign_precondition_refuses_when_loaded_ref_blob_moved():
|
||||||
|
"""Same seam, the negative case: if the loaded ref's blob SHA has moved
|
||||||
|
since review began (a new commit landed on the reviewed PR/branch), the
|
||||||
|
resolver reflects that and sign_precondition must refuse -- proving this
|
||||||
|
isn't just a hardcoded pass-through."""
|
||||||
|
pr_ref = "refs/pull/42/head"
|
||||||
|
session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
|
||||||
|
r.acknowledge_entry(session, "filesystem")
|
||||||
|
|
||||||
|
def fake_resolver(_ref: str) -> str:
|
||||||
|
return "pr-blob-sha-AFTER-A-NEW-PUSH"
|
||||||
|
|
||||||
|
decision = r.sign_precondition(session, fake_resolver)
|
||||||
|
assert decision.ok is False
|
||||||
|
assert "mismatch" in decision.reason.lower() or "changed" in decision.reason.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_sign_precondition_defaults_to_main_when_loaded_ref_unset():
|
||||||
|
"""start_review()'s loaded_ref defaults to 'main' for source=main
|
||||||
|
reviews (and backward-compat with callers that don't pass it)."""
|
||||||
|
session = r.start_review("sha1", _catalog(), _catalog(_entry()))
|
||||||
|
assert session.loaded_ref == "main"
|
||||||
|
r.acknowledge_entry(session, "filesystem")
|
||||||
|
|
||||||
|
def fake_resolver(ref: str) -> str:
|
||||||
|
assert ref == "main"
|
||||||
|
return "sha1"
|
||||||
|
|
||||||
|
decision = r.sign_precondition(session, fake_resolver)
|
||||||
|
assert decision.ok is True
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# 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
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_find_last_signed_catalog_raw_returns_matching_candidate():
|
||||||
|
"""Simulates walking catalog.json's git history: the CURRENT signature
|
||||||
|
covers an OLDER version of the bytes (a later commit changed
|
||||||
|
catalog.json without re-signing -- the exact bypass that produced
|
||||||
|
commit b08cf21). The first candidate that verifies against that
|
||||||
|
signature is 'the last catalog a maintainer actually signed'."""
|
||||||
|
seed, pubkey = r.generate_keypair()
|
||||||
|
old_raw = b'{"schema":1,"version":1,"servers":[]}'
|
||||||
|
new_raw = b'{"schema":1,"version":2,"servers":[]}'
|
||||||
|
sig = r.sign_catalog_bytes(old_raw, seed) # signature covers the OLD bytes
|
||||||
|
found = r.find_last_signed_catalog_raw([new_raw, old_raw], sig, [pubkey])
|
||||||
|
assert found == old_raw
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_last_signed_catalog_raw_none_when_nothing_verifies():
|
||||||
|
seed, _pubkey = r.generate_keypair()
|
||||||
|
_other_seed, other_pubkey = r.generate_keypair()
|
||||||
|
raw = b'{"schema":1,"version":1,"servers":[]}'
|
||||||
|
sig = r.sign_catalog_bytes(raw, seed)
|
||||||
|
# Check against a pubkey list that does NOT include the signer's key.
|
||||||
|
assert r.find_last_signed_catalog_raw([raw], sig, [other_pubkey]) is None
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# catalog_signing_message: domain separation must match bcc_core exactly
|
# catalog_signing_message: domain separation must match bcc_core exactly
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
@@ -553,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
@@ -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>"]}
|
||||||
|
|||||||
Reference in New Issue
Block a user