From 8e2670ff1dedddbce81a1ced4c028445c292e963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 25 Aug 2026 15:48:59 +0000 Subject: [PATCH] todo: a plugin declares its secret purposes and the build checks it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit encryptSecret/decryptSecret take the purpose as a plain string the caller types, and nothing checks who is asking — vault passes 'vault', invoices passes 'invoiceshelf', and either could pass 'wallet'. the per-purpose split is real; what is missing is anything stopping a plugin naming a purpose that is not its own. it cannot be enforced at the call while plugins run in the platform's own process: a function call carries no identity the callee can trust. the two designs that would fix it properly — make the plugin a separate process, or never hand it the key and do the operation on its behalf — were both declined on 2026-08-25 as too large for now. keep the system, catch it in review. so this is only the part that makes the review mechanical: declare the purposes in the manifest, have docs:check fail on an undeclared one. records what it does not buy, which is protection from a plugin nobody read. backburner, low. explicitly not urgent. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- ...in-declares-its-secret-purposes-and-the.md | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 todo/plugin-ecosystem/a-plugin-declares-its-secret-purposes-and-the.md diff --git a/todo/plugin-ecosystem/a-plugin-declares-its-secret-purposes-and-the.md b/todo/plugin-ecosystem/a-plugin-declares-its-secret-purposes-and-the.md new file mode 100644 index 00000000..9e027f40 --- /dev/null +++ b/todo/plugin-ecosystem/a-plugin-declares-its-secret-purposes-and-the.md @@ -0,0 +1,69 @@ +--- +id: plugin-ecosystem/a-plugin-declares-its-secret-purposes-and-the +title: 'A plugin declares its secret purposes, and the build checks it only uses those' +theme: plugin-ecosystem +status: backburner +priority: low +created: 2026-08-25 +date_source: stated +order: 4 +--- + +**A plugin declares its secret purposes in its manifest, and `docs:check` fails if it encrypts or +decrypts under one it did not declare.** Wanted 2026-08-25, explicitly **not urgent** — the owner's +call was to keep the current system and catch this in plugin review instead. This is what makes that +review mechanical rather than remembered. + +## What is true today + +`encryptSecret(purpose: string, plaintext)` and `decryptSecret(purpose: string, blob)` in +`src/databases/officer_db/src/crypto.ts` take the purpose as **a plain string the caller types**. +Nothing checks who is asking. The vault plugin passes `'vault'`, invoices passes `'invoiceshelf'`, +proxy-manager passes `'proxy-manager'` — and any of them could pass `'wallet'` and get a decrypted +seed envelope back, if it can read the row. + +The per-purpose split in the secret store is real and worth having: one leaked key opens one plugin's +columns rather than all of them (`docs/secret-store.md`). What is missing is anything stopping a plugin +from naming a purpose that is not its own. **The isolation is a naming convention that everybody +happens to follow.** + +## Why it cannot simply be enforced at the call + +In-process plugins run inside the platform's own process. Asking for a secret is a function call, and a +function call carries no identity the callee can trust. Two designs would fix it properly and both were +considered and declined on 2026-08-25 as too large for now: + +- **The plugin becomes its own process.** Sidecars already register over a socket and the platform can + tell those apart. Not worth converting an in-process plugin for this alone. +- **The plugin never receives the key.** It asks the platform to do the sensitive operation — "issue a + certificate for mydomain.com" — and the platform does it, key in hand. Better, and a much bigger + change than it sounds, because every existing secret-touching plugin would have to be inverted. + +So the decision stands: keep the current system, enforce by review. This item only removes the part of +that which depends on a human noticing a wrong string in a diff. + +## What to build + +1. **`secretPurposes?: string[]` on `PluginManifest`** (`src/servers/plugins/manifest.ts`), beside + `permissions`. Document it as the exhaustive list — a plugin with no secrets omits it and is then + asserted to use none. + +2. **A check in `scripts/docs-check.ts`** that walks each plugin directory, finds every + `encryptSecret('…'` / `decryptSecret('…'` literal, and fails naming the plugin, the file and the + undeclared purpose. + +3. **Backfill the declarations** for the plugins that already use one: `vault` → `vault`, `invoices` → + `invoiceshelf`, `kiosk` → `kiosk`, `proxy-manager` → `proxy-manager`, plus whatever a sweep turns + up. Note the names do **not** match the directories, which is exactly why the declaration is needed + rather than a directory-name rule. + +## What it does and does not buy + +It catches the accident and the careless copy-paste, which is most of what review is for. It does +**not** stop a deliberately hostile plugin: the string could be computed at runtime, and a plugin that +wanted the wallet's key could just call `key()` itself. That is fine and worth saying out loud — +the threat model here is a plugin we reviewed, not one we did not. Third-party plugins that nobody +reads are a different item in this theme, and they need one of the two declined designs above. + +Belongs with `[[third-party-plugin-sidecars-are-unconfined]]` — same theme, same shape: a plugin gets +more of the host than its job needs, and today nothing but attention stands in the way.