#61 makes the remote catalog trustworthy by requiring a maintainer signature. But if signing is a CLI script (python scripts/sign_catalog.py), the security property quietly erodes: the path of least resistance becomes "merge PR, run script, push" without ever really reading the diff. A signing step you perform reflexively is not an approval — it's a rubber stamp with a key attached.
The signature should be the artefact of an actual review, not a step that follows one. So: a small maintainer-only desktop tool where reviewing and signing are the same action.
What it is
bcc-catalog-console — a separate PySide6 app in this repo (never shipped to users, never in the release bundle; maintainer runs it from source).
Flow:
Pull — lists open PRs touching data/catalog.json (Gitea API), plus the working state of main.
Review — renders a semantic diff, not a text diff. Per entry: added / removed / changed, with the dangerous fields called out loudly:
command and args changes highlighted hard (this is the field that executes)
env_required gaining a non-empty value → blocking error (values must always be empty)
a command outside the allowlist → blocking error
new/changed homepage, docs_url, source → shown with the domain broken out, so a swap from github.com/foo/bar to a lookalike domain is visible
a brand-new entry whose repo is young / low-star → flagged for extra scrutiny
Validate — runs the exact same validate_catalog() the app uses (imported from bcc_core, not reimplemented — one source of truth, per the recurring lesson from the masking work).
Sign — enabled only after every entry in the diff has been explicitly acknowledged. Unlocks the private key (passphrase-protected, stored outside the repo / in the OS keychain), writes data/catalog.json.sig, commits and pushes.
Deliberate friction: the Sign button stays disabled until each changed entry is individually acknowledged. That is the whole point of the tool. Resist any urge to add a "sign all" shortcut later.
Tasks
Semantic catalog differ as a pure function — diff_catalogs(old, new) -> list[EntryChange] — in bcc_core (or a shared module), unit-tested. The GUI stays thin.
Risk annotations as pure predicates (non-empty env value, command not in allowlist, domain change on any URL field, new entry) — testable, and reusable by the CI gate.
Key storage: passphrase-protected private key outside the repo; OS keychain if straightforward, encrypted file if not. Never a plaintext key on disk, never an env var, never in CI.
Exclude from bcc.spec / the release bundle. Add a test that asserts it is not in the packaged datas — a maintainer signing tool shipping to end users would be an own-goal.
Non-goals
Not a general PR review client. It only understands data/catalog.json.
Not shipped to users. Ever.
Not a merge tool — merging still happens in Gitea; this signs what is (or is about to be) on main.
## Why
#61 makes the remote catalog trustworthy by requiring a maintainer signature. But if signing is a CLI script (`python scripts/sign_catalog.py`), the security property quietly erodes: the path of least resistance becomes "merge PR, run script, push" without ever really *reading* the diff. A signing step you perform reflexively is not an approval — it's a rubber stamp with a key attached.
**The signature should be the artefact of an actual review, not a step that follows one.** So: a small maintainer-only desktop tool where reviewing and signing are the same action.
## What it is
`bcc-catalog-console` — a separate PySide6 app in this repo (never shipped to users, never in the release bundle; maintainer runs it from source).
**Flow:**
1. **Pull** — lists open PRs touching `data/catalog.json` (Gitea API), plus the working state of `main`.
2. **Review** — renders a *semantic* diff, not a text diff. Per entry: added / removed / changed, with the dangerous fields called out loudly:
- `command` and `args` changes highlighted hard (this is the field that executes)
- `env_required` gaining a non-empty value → **blocking error** (values must always be empty)
- a `command` outside the allowlist → **blocking error**
- new/changed `homepage`, `docs_url`, `source` → shown with the domain broken out, so a swap from `github.com/foo/bar` to a lookalike domain is visible
- a brand-new entry whose repo is young / low-star → flagged for extra scrutiny
3. **Validate** — runs the exact same `validate_catalog()` the app uses (imported from `bcc_core`, not reimplemented — one source of truth, per the recurring lesson from the masking work).
4. **Sign** — enabled only after every entry in the diff has been explicitly acknowledged. Unlocks the private key (passphrase-protected, stored outside the repo / in the OS keychain), writes `data/catalog.json.sig`, commits and pushes.
**Deliberate friction:** the Sign button stays disabled until each changed entry is individually acknowledged. That is the whole point of the tool. Resist any urge to add a "sign all" shortcut later.
## Tasks
- [ ] Semantic catalog differ as a **pure function** — `diff_catalogs(old, new) -> list[EntryChange]` — in `bcc_core` (or a shared module), unit-tested. The GUI stays thin.
- [ ] Risk annotations as pure predicates (non-empty env value, command not in allowlist, domain change on any URL field, new entry) — testable, and reusable by the CI gate.
- [ ] Console UI: PR list → diff view → per-entry acknowledge → Sign.
- [ ] Key storage: passphrase-protected private key outside the repo; OS keychain if straightforward, encrypted file if not. **Never** a plaintext key on disk, never an env var, never in CI.
- [ ] Sign + commit + push `catalog.json.sig` (reuse the existing tokened-remote flow).
- [ ] Exclude from `bcc.spec` / the release bundle. Add a test that asserts it is not in the packaged datas — a maintainer signing tool shipping to end users would be an own-goal.
## Non-goals
- Not a general PR review client. It only understands `data/catalog.json`.
- Not shipped to users. Ever.
- Not a merge tool — merging still happens in Gitea; this signs what is (or is about to be) on `main`.
Related: #61 (verification side), #10 (catalog).
the_og
added the P1 label 2026-07-12 17:11:49 -04:00
Adversarial review (Fable) — amendments to the Console
🔴 Review/sign TOCTOU
The Console reviews PR diffs but signs a file. Between "maintainer acknowledged every entry in PR #12" and "Sign pressed," the bytes on main can change — a second commit pushed to the same PR after review started, a force-push, a different PR merged in between. The signature must be cryptographically bound to the reviewed bytes, not to "whatever data/catalog.json happens to be now."
Fix: pin the git blob SHA when review begins; sign exactly those bytes; refuse to sign if the current blob differs (re-review required). Cheap, and it's what makes "signing is the approval act" actually true rather than aspirational.
🔴 Emit payload + signature as ONE commit
Per the review on #61: if signing happens after merge, main is red between every catalog merge and the signing commit, and "red main" becomes routine noise — which is precisely the alarm-fatigue failure this whole design exists to prevent. The Console's output must be a single commit containing catalog.json + catalog.json.sig together, or published state lives on a published branch while main carries proposals. Unsigned catalog changes must not be able to land on the published surface at all.
🟠 The Console renders attacker-controlled strings
description, notes, homepage, docs_url, package names in args — all attacker-supplied, all rendered in the diff view.
Qt's QLabelauto-interprets HTML. Set Qt.PlainText everywhere and escape everything.
Homoglyph / RTL-override Unicode can make @modelcontextprotocol/server-filesystem and a typosquat visually identical in a diff — defeating the entire point of the review tool. Render command, args and package names with non-ASCII code points explicitly flagged (highlight, or show escaped). A review tool that can be fooled by a lookalike character is worse than no review tool, because it manufactures confidence.
🟠 Add the one check a human genuinely can't do
The dominant real attack is package-name substitution (typosquat / dependency confusion) in args — it passes signature, schema, allowlist, and a tired human. Humans are bad at this and tools are good at it:
Resolve each package name against the live registry (npm/PyPI) and show, next to the diff: publisher, package age, weekly downloads, and whether the name is a near-neighbour of an existing entry. Also enforce exact version pinning (@scope/pkg@1.2.3) so a later-compromised package can't auto-upgrade into every user.
This is where the actual security of the whole feature lives — not in the allowlist, not in "the user sees the command."
Updated task list
Pin git blob SHA at review start; sign those exact bytes; refuse to sign a changed blob
Emit payload + sig as a single commit (or publish to a published branch)
Qt.PlainText + full escaping on every attacker-controlled string
Flag non-ASCII code points in command / args / package names (homoglyph defence)
Live registry lookup per package: publisher, age, downloads, near-neighbour warning
Enforce exact version pins in args
Import validate_catalog() from bcc_core — never reimplement (one source of truth)
Sign button disabled until every changed entry is individually acknowledged (no "sign all", ever)
## Adversarial review (Fable) — amendments to the Console
### 🔴 Review/sign TOCTOU
The Console reviews **PR diffs** but signs **a file**. Between "maintainer acknowledged every entry in PR #12" and "Sign pressed," the bytes on main can change — a second commit pushed to the same PR after review started, a force-push, a different PR merged in between. The signature must be cryptographically bound to the *reviewed bytes*, not to "whatever `data/catalog.json` happens to be now."
**Fix:** pin the git **blob SHA** when review begins; sign exactly those bytes; **refuse to sign if the current blob differs** (re-review required). Cheap, and it's what makes "signing is the approval act" actually true rather than aspirational.
### 🔴 Emit payload + signature as ONE commit
Per the review on #61: if signing happens after merge, main is red between every catalog merge and the signing commit, and "red main" becomes routine noise — which is precisely the alarm-fatigue failure this whole design exists to prevent. The Console's output must be a **single commit containing `catalog.json` + `catalog.json.sig` together**, or published state lives on a `published` branch while main carries proposals. Unsigned catalog changes must not be able to land on the published surface at all.
### 🟠 The Console renders attacker-controlled strings
`description`, `notes`, `homepage`, `docs_url`, package names in `args` — all attacker-supplied, all rendered in the diff view.
- Qt's `QLabel` **auto-interprets HTML**. Set `Qt.PlainText` everywhere and escape everything.
- **Homoglyph / RTL-override Unicode** can make `@modelcontextprotocol/server-filesystem` and a typosquat *visually identical* in a diff — defeating the entire point of the review tool. Render `command`, `args` and package names with **non-ASCII code points explicitly flagged** (highlight, or show escaped). A review tool that can be fooled by a lookalike character is worse than no review tool, because it manufactures confidence.
### 🟠 Add the one check a human genuinely can't do
The dominant real attack is **package-name substitution** (typosquat / dependency confusion) in `args` — it passes signature, schema, allowlist, and a tired human. Humans are bad at this and tools are good at it:
**Resolve each package name against the live registry (npm/PyPI) and show, next to the diff: publisher, package age, weekly downloads, and whether the name is a near-neighbour of an existing entry.** Also enforce **exact version pinning** (`@scope/pkg@1.2.3`) so a later-compromised package can't auto-upgrade into every user.
This is where the actual security of the whole feature lives — not in the allowlist, not in "the user sees the command."
### Updated task list
- [ ] Pin git blob SHA at review start; sign those exact bytes; refuse to sign a changed blob
- [ ] Emit payload + sig as a single commit (or publish to a `published` branch)
- [ ] `Qt.PlainText` + full escaping on every attacker-controlled string
- [ ] Flag non-ASCII code points in `command` / `args` / package names (homoglyph defence)
- [ ] Live registry lookup per package: publisher, age, downloads, near-neighbour warning
- [ ] Enforce exact version pins in `args`
- [ ] Import `validate_catalog()` from `bcc_core` — never reimplement (one source of truth)
- [ ] Sign button disabled until every changed entry is individually acknowledged (no "sign all", ever)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Why
#61 makes the remote catalog trustworthy by requiring a maintainer signature. But if signing is a CLI script (
python scripts/sign_catalog.py), the security property quietly erodes: the path of least resistance becomes "merge PR, run script, push" without ever really reading the diff. A signing step you perform reflexively is not an approval — it's a rubber stamp with a key attached.The signature should be the artefact of an actual review, not a step that follows one. So: a small maintainer-only desktop tool where reviewing and signing are the same action.
What it is
bcc-catalog-console— a separate PySide6 app in this repo (never shipped to users, never in the release bundle; maintainer runs it from source).Flow:
data/catalog.json(Gitea API), plus the working state ofmain.commandandargschanges highlighted hard (this is the field that executes)env_requiredgaining a non-empty value → blocking error (values must always be empty)commandoutside the allowlist → blocking errorhomepage,docs_url,source→ shown with the domain broken out, so a swap fromgithub.com/foo/barto a lookalike domain is visiblevalidate_catalog()the app uses (imported frombcc_core, not reimplemented — one source of truth, per the recurring lesson from the masking work).data/catalog.json.sig, commits and pushes.Deliberate friction: the Sign button stays disabled until each changed entry is individually acknowledged. That is the whole point of the tool. Resist any urge to add a "sign all" shortcut later.
Tasks
diff_catalogs(old, new) -> list[EntryChange]— inbcc_core(or a shared module), unit-tested. The GUI stays thin.catalog.json.sig(reuse the existing tokened-remote flow).bcc.spec/ the release bundle. Add a test that asserts it is not in the packaged datas — a maintainer signing tool shipping to end users would be an own-goal.Non-goals
data/catalog.json.main.Related: #61 (verification side), #10 (catalog).
Adversarial review (Fable) — amendments to the Console
🔴 Review/sign TOCTOU
The Console reviews PR diffs but signs a file. Between "maintainer acknowledged every entry in PR #12" and "Sign pressed," the bytes on main can change — a second commit pushed to the same PR after review started, a force-push, a different PR merged in between. The signature must be cryptographically bound to the reviewed bytes, not to "whatever
data/catalog.jsonhappens to be now."Fix: pin the git blob SHA when review begins; sign exactly those bytes; refuse to sign if the current blob differs (re-review required). Cheap, and it's what makes "signing is the approval act" actually true rather than aspirational.
🔴 Emit payload + signature as ONE commit
Per the review on #61: if signing happens after merge, main is red between every catalog merge and the signing commit, and "red main" becomes routine noise — which is precisely the alarm-fatigue failure this whole design exists to prevent. The Console's output must be a single commit containing
catalog.json+catalog.json.sigtogether, or published state lives on apublishedbranch while main carries proposals. Unsigned catalog changes must not be able to land on the published surface at all.🟠 The Console renders attacker-controlled strings
description,notes,homepage,docs_url, package names inargs— all attacker-supplied, all rendered in the diff view.QLabelauto-interprets HTML. SetQt.PlainTexteverywhere and escape everything.@modelcontextprotocol/server-filesystemand a typosquat visually identical in a diff — defeating the entire point of the review tool. Rendercommand,argsand package names with non-ASCII code points explicitly flagged (highlight, or show escaped). A review tool that can be fooled by a lookalike character is worse than no review tool, because it manufactures confidence.🟠 Add the one check a human genuinely can't do
The dominant real attack is package-name substitution (typosquat / dependency confusion) in
args— it passes signature, schema, allowlist, and a tired human. Humans are bad at this and tools are good at it:Resolve each package name against the live registry (npm/PyPI) and show, next to the diff: publisher, package age, weekly downloads, and whether the name is a near-neighbour of an existing entry. Also enforce exact version pinning (
@scope/pkg@1.2.3) so a later-compromised package can't auto-upgrade into every user.This is where the actual security of the whole feature lives — not in the allowlist, not in "the user sees the command."
Updated task list
publishedbranch)Qt.PlainText+ full escaping on every attacker-controlled stringcommand/args/ package names (homoglyph defence)argsvalidate_catalog()frombcc_core— never reimplement (one source of truth)