fix(cli): never inject undecryptable ciphertext into a session (bug2)
A sealed DM (nonce + senderPubkey) that no key opened fell through to a base64 "plaintext" fallback. Buffer#toString never throws — invalid bytes become U+FFFD — so the raw ciphertext reached Claude as «21e425db: ��a.�tH�/…» (flexicar report 2026-09-26). - daemon/inbound: the base64 fallback only applies to unsealed v1 broadcasts, decoded with a fatal TextDecoder + renderable-text check. An undecryptable topic post renders an honest placeholder pointing at `topic tail`; anything else is dropped: logged `inbound_decrypt_failed`, acked (stops redelivery), no inbox row, no bus event. - mcp/server: second guard — a body with U+FFFD or C0 controls is never emitted as a channel notification. - utils/text: isRenderableText + decodeBase64Utf8. Test fails 3/6 on the old inbound.ts, passes on the new one. Spec: .artifacts/specs/2026-09-26-session-identity-and-routing.md §1 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
76
.artifacts/specs/2026-09-26-session-identity-and-routing.md
Normal file
76
.artifacts/specs/2026-09-26-session-identity-and-routing.md
Normal file
@@ -0,0 +1,76 @@
|
||||
# Session identity & routing hardening (CLI 1.38.0)
|
||||
|
||||
Source: flexicar-orchestrator bug report 2026-09-26 (9 bugs, CLI 1.37.0) + user report
|
||||
"peers spread messages to the wrong peers, creates noise in context". Investigated at `59610b5`.
|
||||
|
||||
## Root thread
|
||||
The member key is used where a session key belongs, at three points that feed each other:
|
||||
1. a session that lost its daemon registration silently signs sends with the **member** key;
|
||||
2. the channel notification exposes `from_pubkey` = member key, so the receiver replies to the member;
|
||||
3. the broker delivers a member-key DM to *any/every* connection of that member.
|
||||
Result: replies fan out to sibling sessions → noise. Fix all three; each alone is insufficient.
|
||||
|
||||
## Decisions (per bug, ordered by blast radius)
|
||||
|
||||
### §1 bug2 — never inject undecryptable bytes
|
||||
- `daemon/inbound.ts`: base64 "plaintext" fallback only when the push is **not** a sealed DM
|
||||
(no `nonce`+`senderPubkey`), decoded with `TextDecoder("utf-8",{fatal:true})`.
|
||||
- A sealed payload that fails every key → body `null`. If `topic` is known → render placeholder
|
||||
`[encrypted #<topic> message — claudemesh topic tail <topic>]`; otherwise **drop**: log
|
||||
`inbound_decrypt_failed {broker_message_id, sender}`, ack (stop redelivery), no inbox row, no bus event.
|
||||
- MCP (`mcp/server.ts`) second guard: never emit a channel body containing U+FFFD or C0 controls
|
||||
(except \t\n\r).
|
||||
- Broker drained push (`index.ts maybePushQueuedMessages`) carries `senderName`,
|
||||
`senderMemberPubkey`, `topic`, `subtype` like the live push already does.
|
||||
- Deferred (own feature, not effort): daemon-side v2 topic decrypt needs a REST key mint per topic
|
||||
in the daemon hot path; placeholder is honest until then.
|
||||
|
||||
### §2 bug1 — no silent member-key identity
|
||||
- `ipc/server.ts`: a request carrying a session header that doesn't resolve → **401 session_unknown**
|
||||
(not a silent `session=null`).
|
||||
- `send`: when the process is inside a launched session (`CLAUDEMESH_SESSION_ID` set) but the
|
||||
daemon doesn't know the session → error with fix hint, unless `--as-member`.
|
||||
- Re-attach: `POST /v1/sessions/reattach {session_id, mesh, display_name, pid}` — daemon loads the
|
||||
persisted keypair (never mints), signs the attestation (same as boot reload), registers with the
|
||||
caller's pid. Called by the MCP server at startup when its token is missing/unknown.
|
||||
- Token lives in a stable per-session path (`~/.claudemesh/sessions/<mesh>/<uuid>.token`), not the
|
||||
launch tmpdir, so a resumed claude finds it.
|
||||
- The 24 h absolute cap is removed for entries whose pid+start-time still match.
|
||||
- Tokenless MCP subscribes to **no** message events (today: all of them).
|
||||
|
||||
### §3 wrong-peer delivery
|
||||
- Broker: a DM whose target is a member pubkey of a member with ≥1 live session is rejected
|
||||
`member_target_requires_fanout` unless the envelope sets `fanout:true`. Rollout: log-only via
|
||||
env `CLAUDEMESH_MEMBER_DM_POLICY=warn` (default), `reject` after one release.
|
||||
- CLI `send`: refuses a member-key target (other member) unless `--fanout`.
|
||||
- Channel meta: `from_pubkey`/`from_id` = sender **session** pubkey; new `from_member_pubkey`.
|
||||
Skill text updated: reply to `from_pubkey`.
|
||||
- Name / prefix resolution (daemon + cold path): >1 match → error listing candidates; control-plane
|
||||
rows and member keys excluded.
|
||||
- `--self`: requires a registered session; scoped to the current mesh.
|
||||
- Inbox dedupe key = (client_message_id, recipient_pubkey).
|
||||
|
||||
### §4 bug3 — keys don't change behind a session's back
|
||||
- `launch`: mint the UUID first; the same id drives keypair + registration; no throwaway keypair
|
||||
when a session id exists (non-UUID ids persisted under sha256).
|
||||
- Boot reload: load-only; a missing key file → skip + log, never mint.
|
||||
- Supervisor: a session client that ends `replaced`/closed while its registry entry is alive is rebuilt.
|
||||
|
||||
### §5 bug4 — shipped in `dd351a7` (1.37.1, unpublished) — releases with 1.38.0.
|
||||
|
||||
### §6 the rest
|
||||
- bug5 `topic history` decrypts via the same topic-key path as `tail` (shared `decryptForRender`).
|
||||
- bug6 author label: history/tail return the session pubkey + a `sender_display_name` captured at write.
|
||||
- bug7 non-TTY confirm → exit non-zero with `--yes` hint; `[-y]` in kick/ban/disconnect help.
|
||||
- bug8 `message status` matches broker id OR `client_message_id` (hyphens ignored).
|
||||
- bug9 `send` to a control-plane row says so (not "ephemeral"); `peer list --all` tags control-plane.
|
||||
|
||||
## Smoke tests
|
||||
| case | expected |
|
||||
|---|---|
|
||||
| push with sealed DM + wrong key | no inbox row, no channel event, `inbound_decrypt_failed` logged, acked |
|
||||
| broadcast base64 of invalid UTF-8 | dropped, not rendered |
|
||||
| resumed session (`claude --resume`) | same session pubkey, sends signed by session key |
|
||||
| DM to a member pubkey (policy reject) | broker error `member_target_requires_fanout` |
|
||||
| reply via channel `from_pubkey` | lands in exactly one session |
|
||||
| `message status <8 chars printed by send>` | resolves |
|
||||
@@ -8,6 +8,7 @@ import type { SqliteDb } from "./db/sqlite.js";
|
||||
import { insertIfNew } from "./db/inbox.js";
|
||||
import type { EventBus } from "./events.js";
|
||||
import { decryptDirect } from "~/services/crypto/facade.js";
|
||||
import { decodeBase64Utf8 } from "~/utils/text.js";
|
||||
|
||||
export interface InboundContext {
|
||||
db: SqliteDb;
|
||||
@@ -96,9 +97,26 @@ export async function handleBrokerPush(msg: Record<string, unknown>, ctx: Inboun
|
||||
const subtype = stringOrNull(msg.subtype);
|
||||
// Forward-compat: Sprint 7 brokers will send client_message_id alongside.
|
||||
const clientMessageId = stringOrNull(msg.client_message_id) ?? brokerMessageId ?? randomUUID();
|
||||
const body = await decryptOrFallback({
|
||||
const decrypted = await decryptOrFallback({
|
||||
ciphertext, nonce, senderPubkey, ctx,
|
||||
});
|
||||
// 1.38.0 (spec 2026-09-26 §1): an undecryptable payload is never
|
||||
// rendered. Topic posts we can't open yet get an honest placeholder;
|
||||
// anything else is dropped (acked so the broker stops redelivering).
|
||||
let body: string | null = decrypted;
|
||||
if (decrypted === null && ciphertext) {
|
||||
if (topic) {
|
||||
body = `[encrypted #${topic} message — read it with: claudemesh topic tail ${topic}]`;
|
||||
} else {
|
||||
ctx.log?.("warn", "inbound_decrypt_failed", {
|
||||
broker_message_id: brokerMessageId,
|
||||
sender: senderPubkey.slice(0, 16),
|
||||
has_nonce: Boolean(nonce),
|
||||
});
|
||||
ctx.ackClientMessage?.(clientMessageId, brokerMessageId);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const id = randomUUID();
|
||||
const inserted = insertIfNew(ctx.db, {
|
||||
@@ -178,11 +196,11 @@ async function decryptOrFallback(args: {
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback: broadcast/topic posts are base64 plaintext (existing CLI
|
||||
// pre-encryption convention for `*` and `@topic`). Sprint 7+ adds per-
|
||||
// topic symmetric keys.
|
||||
try { return Buffer.from(ciphertext, "base64").toString("utf8"); }
|
||||
catch (e) { ctx.log?.("warn", "inbound_b64_decode_failed", { err: String(e) }); return null; }
|
||||
// A sealed payload (nonce + sender) that no key opened is NOT base64
|
||||
// plaintext — decoding it produced the raw-binary injection of the
|
||||
// 2026-09-26 report. Only unsealed v1 broadcasts take the fallback.
|
||||
if (nonce && senderPubkey) return null;
|
||||
return decodeBase64Utf8(ciphertext);
|
||||
}
|
||||
|
||||
function stringOrNull(v: unknown): string | null {
|
||||
|
||||
@@ -36,6 +36,7 @@ import { join } from "node:path";
|
||||
|
||||
import { DAEMON_PATHS } from "~/daemon/paths.js";
|
||||
import { VERSION } from "~/constants/urls.js";
|
||||
import { isRenderableText } from "~/utils/text.js";
|
||||
import { readConfig } from "~/services/config/facade.js";
|
||||
import { BrokerClient } from "~/services/broker/facade.js";
|
||||
|
||||
@@ -439,7 +440,14 @@ export async function startMcpServer(): Promise<void> {
|
||||
const d = ev.data;
|
||||
const fromName = String(d.sender_name ?? "unknown");
|
||||
const fromMember = String(d.sender_member_pubkey ?? d.sender_pubkey ?? "");
|
||||
const body = String(d.body ?? "(decrypt failed)");
|
||||
const rawBody = typeof d.body === "string" ? d.body : null;
|
||||
// 1.38.0 (spec 2026-09-26 §1): second guard — never hand Claude
|
||||
// binary garbage, whatever the daemon's inbox holds.
|
||||
if (rawBody === null || !isRenderableText(rawBody)) {
|
||||
mcpLog("channel_dropped_unrenderable", { mesh: String(d.mesh ?? ""), id: String(d.id ?? "") });
|
||||
return;
|
||||
}
|
||||
const body = rawBody;
|
||||
const priority = String(d.priority ?? "next");
|
||||
const prioBadge = priority === "now" ? "[URGENT] " : priority === "low" ? "[low] " : "";
|
||||
const topicTag = d.topic ? ` (#${d.topic})` : "";
|
||||
|
||||
@@ -4,3 +4,4 @@ export { isInviteUrl, extractInviteCode } from "./url.js";
|
||||
export { formatBytes, formatDuration } from "./format.js";
|
||||
export { isNewer } from "./semver.js";
|
||||
export { retry } from "./retry.js";
|
||||
export { isRenderableText, decodeBase64Utf8 } from "./text.js";
|
||||
|
||||
19
apps/cli/src/utils/text.ts
Normal file
19
apps/cli/src/utils/text.ts
Normal file
@@ -0,0 +1,19 @@
|
||||
/** No U+FFFD replacement chars, no C0/DEL controls other than tab, newline, CR.
|
||||
* Used to refuse rendering undecryptable payloads as text (spec 2026-09-26 §1). */
|
||||
export function isRenderableText(text: string): boolean {
|
||||
// eslint-disable-next-line no-control-regex
|
||||
return !/[\uFFFD\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/.test(text);
|
||||
}
|
||||
|
||||
/** Strict base64 → UTF-8. Buffer#toString never throws (invalid bytes
|
||||
* become U+FFFD), so validate with a fatal TextDecoder instead. */
|
||||
export function decodeBase64Utf8(b64: string): string | null {
|
||||
try {
|
||||
const bytes = Buffer.from(b64, "base64");
|
||||
if (bytes.length === 0) return null;
|
||||
const text = new TextDecoder("utf-8", { fatal: true }).decode(bytes);
|
||||
return isRenderableText(text) ? text : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
109
apps/cli/tests/unit/inbound-undecryptable.test.ts
Normal file
109
apps/cli/tests/unit/inbound-undecryptable.test.ts
Normal file
@@ -0,0 +1,109 @@
|
||||
/**
|
||||
* Spec 2026-09-26 §1 (bug2): an undecryptable push must never reach a
|
||||
* Claude session as text. Before 1.38.0 a sealed DM that no key opened
|
||||
* was base64-decoded and rendered as raw binary («21e425db: <20><>a.<2E>tH<74>/…»).
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { randomBytes } from "node:crypto";
|
||||
|
||||
import { openSqlite, type SqliteDb } from "~/daemon/db/sqlite.js";
|
||||
import { migrateInbox } from "~/daemon/db/inbox.js";
|
||||
import { EventBus } from "~/daemon/events.js";
|
||||
import { handleBrokerPush, type InboundContext } from "~/daemon/inbound.js";
|
||||
import { encryptDirect, generateKeypair } from "~/services/crypto/facade.js";
|
||||
import { decodeBase64Utf8, isRenderableText } from "~/utils/text.js";
|
||||
|
||||
describe("inbound: undecryptable payloads", () => {
|
||||
let dir: string;
|
||||
let db: SqliteDb;
|
||||
let events: Array<Record<string, unknown>>;
|
||||
let logs: string[];
|
||||
let acks: string[];
|
||||
let ctx: InboundContext;
|
||||
|
||||
beforeEach(async () => {
|
||||
dir = mkdtempSync(join(tmpdir(), "cmh-inbound-"));
|
||||
db = await openSqlite(join(dir, "inbox.db"));
|
||||
migrateInbox(db);
|
||||
events = [];
|
||||
logs = [];
|
||||
acks = [];
|
||||
const bus = new EventBus();
|
||||
bus.subscribe((e) => { if (e.kind === "message") events.push(e.data); });
|
||||
const me = await generateKeypair();
|
||||
ctx = {
|
||||
db, bus, meshSlug: "test",
|
||||
recipientSecretKeyHex: me.secretKey,
|
||||
ackClientMessage: (cid) => acks.push(cid),
|
||||
log: (_l, msg) => logs.push(msg),
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
try { db.close(); } catch { /* ignore */ }
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("drops a sealed DM no key opens: no event, no row, logged, acked", async () => {
|
||||
const sender = await generateKeypair();
|
||||
const stranger = await generateKeypair();
|
||||
// Sealed for somebody else — our key can't open it.
|
||||
const env = await encryptDirect("secret", stranger.publicKey, sender.secretKey);
|
||||
await handleBrokerPush({
|
||||
type: "push", messageId: "m1", senderPubkey: sender.publicKey,
|
||||
nonce: env.nonce, ciphertext: env.ciphertext,
|
||||
}, ctx);
|
||||
expect(events).toHaveLength(0);
|
||||
expect(db.prepare("SELECT COUNT(*) AS n FROM inbox").get()).toEqual({ n: 0 });
|
||||
expect(logs).toContain("inbound_decrypt_failed");
|
||||
expect(acks).toEqual(["m1"]);
|
||||
});
|
||||
|
||||
it("renders a placeholder for a topic post it can't open", async () => {
|
||||
const sender = await generateKeypair();
|
||||
await handleBrokerPush({
|
||||
type: "push", messageId: "m2", senderPubkey: sender.publicKey, topic: "compras-orq",
|
||||
nonce: Buffer.from(randomBytes(24)).toString("base64"),
|
||||
ciphertext: Buffer.from(randomBytes(64)).toString("base64"),
|
||||
}, ctx);
|
||||
expect(events).toHaveLength(1);
|
||||
expect(String(events[0]!.body)).toContain("claudemesh topic tail compras-orq");
|
||||
});
|
||||
|
||||
it("still decrypts a DM sealed for us", async () => {
|
||||
const sender = await generateKeypair();
|
||||
const me = await generateKeypair();
|
||||
ctx.recipientSecretKeyHex = me.secretKey;
|
||||
const env = await encryptDirect("hola", me.publicKey, sender.secretKey);
|
||||
await handleBrokerPush({
|
||||
type: "push", messageId: "m3", senderPubkey: sender.publicKey,
|
||||
nonce: env.nonce, ciphertext: env.ciphertext,
|
||||
}, ctx);
|
||||
expect(events.map((e) => e.body)).toEqual(["hola"]);
|
||||
});
|
||||
|
||||
it("keeps the v1 base64 broadcast path for valid text", async () => {
|
||||
await handleBrokerPush({
|
||||
type: "push", messageId: "m4", ciphertext: Buffer.from("hello mesh").toString("base64"),
|
||||
}, ctx);
|
||||
expect(events.map((e) => e.body)).toEqual(["hello mesh"]);
|
||||
});
|
||||
|
||||
it("drops a v1 broadcast whose bytes aren't text", async () => {
|
||||
await handleBrokerPush({
|
||||
type: "push", messageId: "m5", ciphertext: Buffer.from([0xff, 0xfe, 0x00, 0x9c]).toString("base64"),
|
||||
}, ctx);
|
||||
expect(events).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("text helpers", () => {
|
||||
expect(isRenderableText("línea\ncon\ttab")).toBe(true);
|
||||
expect(isRenderableText("bad <20>")).toBe(false);
|
||||
expect(isRenderableText("bell \u0007")).toBe(false);
|
||||
expect(decodeBase64Utf8(Buffer.from([0xc3, 0x28]).toString("base64"))).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user