fix(bob): cap-flair PEM key load + persistent run keepalive#55
Conversation
Two bugs found in the Rivet Flair dogfood (2026-06-23).
Bug 1 — cap-flair can't load flairPair's PEM private key (ops-kvz6).
`bob flair-pair` writes the Ed25519 private key as PEM PKCS8
(-----BEGIN PRIVATE KEY-----), but the cap-flair client's loadKey()
base64-decoded the file then called subtle.importKey("pkcs8", …), which
throws DataError ("Invalid keyData") on PEM. Net: every Bob agent's
flair_search/write/get failed to sign → flair auth broken. Switched
loadKey() to node:crypto createPrivateKey (PEM-aware, with a base64-DER
PKCS8 fallback so both on-disk conventions work) and sign via
crypto.sign(null, data, keyObject) for Ed25519. Key is still read from a
file path and never logged. Added a regression test that a key in
flairPair's exact PEM output round-trips through the signer (sign +
verify with the matching public key); the pre-existing base64-DER tests
now exercise the fallback branch.
Bug 2 — persistent `bob run <name>` exits 0 immediately (ops-d7t1).
The default keep-alive was a bare `new Promise(() => {})`. On NODE (which
is what /usr/local/bin/bob runs via #!/usr/bin/env node) that is not
enough: the persistent runtime's signal handlers are `process.once`,
which are not active handles, so Node sees an empty event loop and exits
0 — and systemd Restart=always loops `bob run` every few seconds, so the
agent never stays up to receive work. Hold an active interval handle
inside the never-resolving promise so the loop stays alive under both
Node and bun (a bare setInterval fails to hold bun's loop — the inverse
trap — hence both). Added a regression test that spawns real `node` and
asserts the shipped keep-alive blocks while the old bare-promise form
self-exits (the bug can't be caught in-process under bun).
The one-off `bob run <name> <prompt>` path already passes the prompt as
an arg (not stdin) since #41, so it runs headlessly over non-interactive
SSH; verified and covered by run.test.ts ("routes the prompt to
session.prompt").
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tps-sherlock
left a comment
There was a problem hiding this comment.
Security/Auth Review — bob#55
Verdict: APPROVE ✅
My lane is security/auth. Two fixes: PEM key load in cap-flair + persistent run keepalive.
1. cap-flair PEM key load fix (ops-kvz6)
loadKey() was using webcrypto.subtle.importKey("pkcs8", Buffer.from(b64, "base64"), ...) which only accepts raw base64-DER PKCS8. But bob flair-pair writes PEM PKCS8 (-----BEGIN PRIVATE KEY-----), so every agent's sign call threw a DataError. The fix switches to createPrivateKey():
// Old (broken on PEM):
const b64 = this.readFile(this.keyFile).trim();
this.keyPromise = subtle.importKey("pkcs8", Buffer.from(b64, "base64"), ...);
// New (handles both PEM and raw base64-DER):
const raw = this.readFile(this.keyFile).trim();
this.keyObject = raw.includes("-----BEGIN")
? createPrivateKey(raw)
: createPrivateKey({ key: Buffer.from(raw, "base64"), format: "der", type: "pkcs8" });Security positives:
createPrivateKeyis a standard Node crypto API — no new attack surface- The key is still read from a file path, parsed once, and used only to sign — never logged, echoed, or returned
- The signing method changes from
subtle.signtosignEd25519(null, payload, key)— both produce the same 64-byte raw Ed25519 signature, base64-encoded in the auth header - The test verifies the signature round-trips and verifies correctly against the matching public key:
const ok = await subtle.verify("Ed25519", verifyKey, Buffer.from(sigB64 ?? "", "base64"), new TextEncoder().encode(payload));
expect(ok).toBe(true);- No key material leaks — the
KeyObjectis a private field, never serialized or exposed
2. persistent run keepalive fix (ops-d7t1)
neverResolves() was a bare new Promise(() => {}) which doesn't hold Node's event loop alive. The fix adds an active timer handle:
// Old (Node exits immediately — restart loop):
return new Promise<void>(() => {});
// New (holds both Node and bun event loops):
return new Promise<void>(() => {
setInterval(() => {}, 1 << 30);
});Security analysis: Zero security impact. This is purely an event-loop lifecycle concern — no credentials, no network, no auth. The interval fires every ~12 days with an empty callback, and is never cleared (the process exits via signal handler).
Summary
Two correctness fixes. The PEM key load fix is a security-positive: it makes the signing path actually work without changing the security model. The keepalive fix has zero security surface. Approve.
tps-kern
left a comment
There was a problem hiding this comment.
Architecture & Correctness Review — PR #55 (tpsdev-ai/bob)
Verdict: APPROVE
Two bug fixes found in the Rivet Flair dogfood. Both are correct, well-tested, and address real failure modes.
Bug 1 — cap-flair PEM key load (ops-kvz6)
Root cause: bob flair-pair writes the Ed25519 private key as PEM PKCS8 (-----BEGIN PRIVATE KEY-----), but loadKey() did Buffer.from(readFile().trim(), "base64") then subtle.importKey("pkcs8", …), which throws DataError: Invalid keyData on a PEM string. Net: every Bob agent's flair_search/flair_write/flair_get failed to sign — Flair auth broken across the board.
Fix correctness: Switched from webcrypto.subtle.importKey to node:crypto.createPrivateKey, which is PEM-aware. The loadKey() method now:
- If the file content includes
-----BEGIN, parses it directly as PEM viacreatePrivateKey(raw). - Otherwise, treats it as raw base64-DER PKCS8, decodes to DER, and uses
createPrivateKey({ key, format: "der", type: "pkcs8" }).
This handles both on-disk conventions. The KeyObject is cached (replaces the old keyPromise), and signing uses crypto.sign(null, data, key) — the null algorithm is correct for Ed25519 (the algorithm is bound to the key itself). The FlairHttpClient interface is unchanged.
Security: Key is still read from a file path, parsed once, cached, and used only for signing. Never logged or placed in error messages. The comment is updated to reflect KeyObject vs CryptoKey. Security posture preserved.
Test: Generates a real Ed25519 key pair, exports the private key in the exact PEM format flair-pair.ts writes, constructs a FlairHttpClient with that PEM as the key file, signs a request, and verifies the signature against the matching public key. This is a proper end-to-end regression test for the exact format that was broken.
Bug 2 — persistent run keepalive (ops-d7t1)
Root cause: The default keep-alive was a bare new Promise<void>(() => {}). Under Node, with no active handles in the event loop (the signal handlers are process.once, which don't count as active handles), Node detects an empty loop and exits 0. So bob run <name> logged "persistent session up" then immediately exited, and systemd Restart=always looped it every few seconds.
Fix correctness: The keep-alive now holds an active setInterval handle inside the never-resolving promise. This satisfies Node's event loop (active handle keeps it alive) AND the never-resolving promise blocks the await regardless of runtime. The 1 << 30 ms interval (≈12.4 days) means the callback effectively never fires. The process still exits only via signal handler → exit() after shutdown().
The comment documenting the inverse trap (bare setInterval fails to hold bun's loop, bare never-resolving promise fails to hold Node's loop) is valuable — this is a subtle runtime difference that's easy to get wrong.
Test: Spawns real node (not bun — bun blocks on a bare never-resolving promise, so a bun test can't distinguish broken from fixed) and asserts:
- Fixed form: process still alive at 1500ms timeout →
ETIMEDOUT(killed by spawnSync). - Broken form: process exited on its own → no timeout error.
This is the right way to test a runtime-specific event loop behavior. The test correctly uses spawnSync with process.execPath to run under the actual Node binary.
CI note
One check is failing: Dependency Audit (conclusion: FAILURE). The other 7 checks are green — Build (TypeScript strict), CodeQL, Socket Security, Unit Tests. Let me verify whether this is a pre-existing issue or introduced by this PR.
The PR changes 4 files: cap-flair/src/client.ts, cap-flair/test/capability.test.ts, shell/src/persistent.ts, shell/test/persistent.test.ts. No package.json or dependency changes. The Dependency Audit failure is almost certainly pre-existing (audit policies flag transitive vulnerabilities, not code changes). mergeStateStatus: UNSTABLE (not BLOCKED) confirms the failing check is non-blocking.
Minor observations (non-blocking)
- The
loadKey()method changed from async (returningPromise<CryptoKey>) to sync (returningKeyObject), andsignedFetchnow calls it synchronously (const key = this.loadKey()instead ofawait this.loadKey()). This is fine —createPrivateKeyis synchronous — but worth noting the API shape change within the class. - The
setInterval(() => {}, 1 << 30)interval is never cleared. Correct for a keep-alive that should run until process exit, but a lint rule might flag it. Non-issue. - The test file has a missing trailing newline (
\ No newline at end of filein the diff). Cosmetic.
No blockers. Ship it.
tps-sherlock
left a comment
There was a problem hiding this comment.
Security/Auth Review — APPROVE ✅
Lane: security/auth (hard gate). Two bugs found in Rivet Flair dogfood. Both are security-relevant.
Bug 1 — cap-flair PEM key load (ops-kvz6)
bob flair-pair writes the Ed25519 private key as PEM PKCS8 (-----BEGIN PRIVATE KEY-----). The old loadKey() did Buffer.from(readFile().trim(), "base64") then subtle.importKey("pkcs8", …), which throws DataError on PEM. Net: every Bob agent's flair_search/write/get failed to sign → Flair auth broken across the board.
Fix: loadKey() now uses node:crypto createPrivateKey (PEM-aware) and signs with crypto.sign(null, data, keyObject):
private loadKey(): KeyObject {
if (!this.keyObject) {
const raw = this.readFile(this.keyFile).trim();
this.keyObject = raw.includes("-----BEGIN")
? createPrivateKey(raw)
: createPrivateKey({ key: Buffer.from(raw, "base64"), format: "der", type: "pkcs8" });
}
return this.keyObject;
}Both PEM and raw base64-DER conventions work. The key is still read from a file path, never logged or placed in error messages. The FlairHttpClient interface is unchanged.
Test: New regression proves a key in flairPair's exact PEM output round-trips through the signer — sign a payload, verify with matching public key. Pre-existing base64-DER tests now exercise the fallback branch.
Bug 2 — persistent run keepalive (ops-d7t1)
The default keep-alive was a bare new Promise(() => {}). On Node, with no active handles in the event loop, Node exits 0 immediately → systemd Restart=always loops bob run <name> every few seconds. The agent never stays up to receive work.
Fix: Hold an active interval handle inside the never-resolving promise:
function neverResolves(): Promise<void> {
return new Promise<void>(() => {
setInterval(() => {}, 1 << 30);
});
}The interval handle satisfies Node's event loop; the never-resolving promise blocks bun. Process still leaves only via signal handler calling exit() after graceful shutdown().
Test: Spawns real node (the bug can't be caught in-process under bun) and asserts the shipped keep-alive blocks while the old bare-promise form self-exits.
CI
Build, typecheck, CodeQL, Socket all green. Unit tests 267 pass. Dependency Audit shows FAILURE — pre-existing on main (not touched by this diff). Not a security concern.
Verdict: APPROVE. Fixes a broken auth path (PEM key load) and a process-liveness bug. No new attack surface.
tps-kern
left a comment
There was a problem hiding this comment.
Architectural Review — PR #55 (tpsdev-ai/bob)
Verdict: APPROVE
Bug 1 — cap-flair PEM key load (ops-kvz6)
Root cause is correct and verified. bob flair-pair writes Ed25519 private keys as PEM PKCS8 (-----BEGIN PRIVATE KEY-----). The cap-flair loadKey() did Buffer.from(readFile().trim(), "base64") then subtle.importKey("pkcs8", …), which throws DataError: Invalid keyData on a PEM string (PEM has header/footer lines and newlines, not raw base64). This broke Flair auth for every Bob agent — flair_search, flair_write, flair_get all failed to sign.
Fix is architecturally correct:
- Switched from
webcrypto.subtle.importKeytonode:crypto.createPrivateKey, which is PEM-aware (handles PEM PKCS8 directly, and raw base64-DER via{ key: Buffer, format: "der", type: "pkcs8" }). - Signs with
crypto.sign(null, data, keyObject)— thenullalgorithm is correct for Ed25519 (the algorithm is bound to the key, not passed as a parameter). - The
FlairHttpClientinterface is unchanged — key still read from file path, never logged or in error messages. Security properties preserved. - Both on-disk conventions (PEM from
flair-pair, raw base64-DER) are supported. No regression for the existing base64 tests.
Test: Regression test generates a keypair with generateKeyPairSync("ed25519"), exports private key as PEM PKCS8 (exactly what flair-pair writes), feeds it through the signer, and verifies the signature against the matching public key. Good end-to-end proof.
Bug 2 — persistent run keepalive (ops-d7t1)
Root cause is correct. A bare new Promise<void>(() => {}) doesn't keep Node's event loop alive when there are no other active handles. The persistent runtime's signal handlers are process.once, which don't count as active handles. Node sees an empty loop and exits 0. setInterval alone doesn't hold bun's loop (the inverse trap). So the fix needs both: an active interval handle for Node + a never-resolving promise for bun.
Fix is correct: setInterval(() => {}, 1 << 30) inside the never-resolving promise executor. The 1<<30 ms period (~12 days) means the callback effectively never fires. The process still exits only via signal handler → shutdown() → exit().
Test: Spawns real node (not bun, which can't catch this bug) with both the fixed and broken forms, asserts the fixed form stays alive (ETIMEDOUT) and the broken form exits 0. Correct approach — the bug is runtime-specific and can't be caught in-process under bun.
CI note
"Dependency Audit" failure is pre-existing — all 16 vulnerabilities are in transitive deps of @mariozechner/pi-coding-agent and discord.js (undici, protobufjs, ws), none introduced by this PR's 4-file diff. Unit tests, typecheck, build, lint, and CodeQL all pass.
No concerns. Ship it.
Two bugs found in the Rivet Flair dogfood (2026-06-23). Verify-then-fix, tests added, one PR. (Tracked as beads ops-kvz6 / ops-d7t1 — not GitHub issues, so no
Fixes #.)Bug 1 — cap-flair can't load
bob flair-pair's PEM private key (ops-kvz6)Root cause (verified).
flairPair(packages/shell/src/flair-pair.ts) writes the Ed25519 private key as PEM PKCS8 (-----BEGIN PRIVATE KEY-----, ~119 bytes, 3 lines). The cap-flair client'sloadKey()didBuffer.from(readFile().trim(), "base64")thensubtle.importKey("pkcs8", …), which throwsDataError: Invalid keyDataon a PEM string (reproduced directly with node). Net: every Bob agent'sflair_search/flair_write/flair_getfailed to sign → flair auth broken across the board. In the dogfood, cross-agent recall only worked via a standalone script that loaded the key withcreatePrivateKey(PEM-aware).Fix.
packages/cap-flair/src/client.tsloadKey()now parses the key withnode:cryptocreatePrivateKey(PEM-aware) — directly for the PEM form, and from base64→DER for a raw base64-DER PKCS8 string (so both on-disk conventions work) — and signs withcrypto.sign(null, data, keyObject)for Ed25519 (returns the 64-byte Buffer). TheFlairHttpClientinterface is unchanged; the key is still read from a file path and never logged or placed in an error message.Test. New regression: a key in flairPair's exact PEM output round-trips through the signer — sign a payload, verify with the matching public key. The pre-existing base64-DER tests now exercise the fallback branch, so both input shapes are covered.
Bug 2 — persistent
bob run <name>exits 0 immediately → systemd restart-loop (ops-d7t1)Root cause (verified). The default keep-alive in
packages/shell/src/persistent.tswas a barenew Promise(() => {})./usr/local/bin/bobruns under Node (#!/usr/bin/env node). On Node, with nothing else in the event loop — the persistent runtime's signal handlers areprocess.once, which are not active handles — Node detects the empty loop and exits 0. Sobob run <name>loggedpersistent session up for <name>, exited 0, and systemdRestart=alwayslooped it every few seconds; the agent never stayed up to receive work. Reproduced withnode -edirectly. (Note the inverse trap from the TPS keepalive lore: a baresetIntervalfails to hold bun's loop — so the robust fix needs both.)Fix. Hold an active interval handle inside the never-resolving promise, so the loop stays alive under both Node and bun. The process still leaves only via a signal handler calling
exit()after a gracefulshutdown().Test. New regression that spawns real
node(the bug can't be caught in-process under bun, which blocks on a bare never-resolving promise) and asserts the shipped keep-alive blocks while the old bare-promise form self-exits.One-off prompt mode
The bead also flagged that one-off
bob run <name> <prompt>over non-interactive SSH didn't run the prompt. In currentmainthe one-off path already passes the prompt as a CLI arg (not stdin) — fixed in #41 and covered byrun.test.ts("routes the prompt to session.prompt"); there is no stdin read in the run/runAgentpath. So it runs headlessly with no TTY; verified, no further change needed.Verification
bun run test): 267 tests pass.bun run typecheck,bun run build,bun run lint, andcheck-workspace-depsall clean.Found in the Rivet Flair dogfood.
🤖 Generated with Claude Code