todo: a plugin declares its secret purposes and the build checks it
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 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -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.
|
||||
Reference in New Issue
Block a user