fix: harden pairing flow
This commit is contained in:
Peter Steinberger
109
src/pairing/pairing-store.test.ts
Normal file
109
src/pairing/pairing-store.test.ts
Normal file
@@ -0,0 +1,109 @@
|
||||
import crypto from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { resolveOAuthDir } from "../config/paths.js";
|
||||
import {
|
||||
listProviderPairingRequests,
|
||||
upsertProviderPairingRequest,
|
||||
} from "./pairing-store.js";
|
||||
|
||||
async function withTempStateDir<T>(fn: (stateDir: string) => Promise<T>) {
|
||||
const previous = process.env.CLAWDBOT_STATE_DIR;
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-pairing-"));
|
||||
process.env.CLAWDBOT_STATE_DIR = dir;
|
||||
try {
|
||||
return await fn(dir);
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env.CLAWDBOT_STATE_DIR;
|
||||
else process.env.CLAWDBOT_STATE_DIR = previous;
|
||||
await fs.rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe("pairing store", () => {
|
||||
it("reuses pending code and reports created=false", async () => {
|
||||
await withTempStateDir(async () => {
|
||||
const first = await upsertProviderPairingRequest({
|
||||
provider: "discord",
|
||||
id: "u1",
|
||||
});
|
||||
const second = await upsertProviderPairingRequest({
|
||||
provider: "discord",
|
||||
id: "u1",
|
||||
});
|
||||
expect(first.created).toBe(true);
|
||||
expect(second.created).toBe(false);
|
||||
expect(second.code).toBe(first.code);
|
||||
|
||||
const list = await listProviderPairingRequests("discord");
|
||||
expect(list).toHaveLength(1);
|
||||
expect(list[0]?.code).toBe(first.code);
|
||||
});
|
||||
});
|
||||
|
||||
it("expires pending requests after TTL", async () => {
|
||||
await withTempStateDir(async (stateDir) => {
|
||||
const created = await upsertProviderPairingRequest({
|
||||
provider: "signal",
|
||||
id: "+15550001111",
|
||||
});
|
||||
expect(created.created).toBe(true);
|
||||
|
||||
const oauthDir = resolveOAuthDir(process.env, stateDir);
|
||||
const filePath = path.join(oauthDir, "signal-pairing.json");
|
||||
const raw = await fs.readFile(filePath, "utf8");
|
||||
const parsed = JSON.parse(raw) as {
|
||||
requests?: Array<Record<string, unknown>>;
|
||||
};
|
||||
const expiredAt = new Date(Date.now() - 2 * 60 * 60 * 1000).toISOString();
|
||||
const requests = (parsed.requests ?? []).map((entry) => ({
|
||||
...entry,
|
||||
createdAt: expiredAt,
|
||||
lastSeenAt: expiredAt,
|
||||
}));
|
||||
await fs.writeFile(
|
||||
filePath,
|
||||
`${JSON.stringify({ version: 1, requests }, null, 2)}\n`,
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const list = await listProviderPairingRequests("signal");
|
||||
expect(list).toHaveLength(0);
|
||||
|
||||
const next = await upsertProviderPairingRequest({
|
||||
provider: "signal",
|
||||
id: "+15550001111",
|
||||
});
|
||||
expect(next.created).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it("regenerates when a generated code collides", async () => {
|
||||
await withTempStateDir(async () => {
|
||||
const spy = vi.spyOn(crypto, "randomInt");
|
||||
try {
|
||||
spy.mockReturnValue(0);
|
||||
const first = await upsertProviderPairingRequest({
|
||||
provider: "telegram",
|
||||
id: "123",
|
||||
});
|
||||
expect(first.code).toBe("AAAAAAAA");
|
||||
|
||||
const sequence = Array(8).fill(0).concat(Array(8).fill(1));
|
||||
let idx = 0;
|
||||
spy.mockImplementation(() => sequence[idx++] ?? 1);
|
||||
const second = await upsertProviderPairingRequest({
|
||||
provider: "telegram",
|
||||
id: "456",
|
||||
});
|
||||
expect(second.code).toBe("BBBBBBBB");
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,9 +1,26 @@
|
||||
import crypto from "node:crypto";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import lockfile from "proper-lockfile";
|
||||
|
||||
import { resolveOAuthDir, resolveStateDir } from "../config/paths.js";
|
||||
|
||||
const PAIRING_CODE_LENGTH = 8;
|
||||
const PAIRING_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789";
|
||||
const PAIRING_PENDING_TTL_MS = 60 * 60 * 1000;
|
||||
const PAIRING_STORE_LOCK_OPTIONS = {
|
||||
retries: {
|
||||
retries: 10,
|
||||
factor: 2,
|
||||
minTimeout: 100,
|
||||
maxTimeout: 10_000,
|
||||
randomize: true,
|
||||
},
|
||||
stale: 30_000,
|
||||
} as const;
|
||||
|
||||
export type PairingProvider =
|
||||
| "telegram"
|
||||
| "signal"
|
||||
@@ -74,24 +91,92 @@ async function readJsonFile<T>(
|
||||
}
|
||||
|
||||
async function writeJsonFile(filePath: string, value: unknown): Promise<void> {
|
||||
await fs.promises.mkdir(path.dirname(filePath), { recursive: true });
|
||||
await fs.promises.writeFile(
|
||||
filePath,
|
||||
`${JSON.stringify(value, null, 2)}\n`,
|
||||
"utf-8",
|
||||
const dir = path.dirname(filePath);
|
||||
await fs.promises.mkdir(dir, { recursive: true, mode: 0o700 });
|
||||
const tmp = path.join(
|
||||
dir,
|
||||
`${path.basename(filePath)}.${crypto.randomUUID()}.tmp`,
|
||||
);
|
||||
await fs.promises.writeFile(tmp, `${JSON.stringify(value, null, 2)}\n`, {
|
||||
encoding: "utf-8",
|
||||
});
|
||||
await fs.promises.chmod(tmp, 0o600);
|
||||
await fs.promises.rename(tmp, filePath);
|
||||
}
|
||||
|
||||
async function ensureJsonFile(filePath: string, fallback: unknown) {
|
||||
try {
|
||||
await fs.promises.access(filePath);
|
||||
} catch {
|
||||
await writeJsonFile(filePath, fallback);
|
||||
}
|
||||
}
|
||||
|
||||
async function withFileLock<T>(
|
||||
filePath: string,
|
||||
fallback: unknown,
|
||||
fn: () => Promise<T>,
|
||||
): Promise<T> {
|
||||
await ensureJsonFile(filePath, fallback);
|
||||
let release: (() => Promise<void>) | undefined;
|
||||
try {
|
||||
release = await lockfile.lock(filePath, PAIRING_STORE_LOCK_OPTIONS);
|
||||
return await fn();
|
||||
} finally {
|
||||
if (release) {
|
||||
try {
|
||||
await release();
|
||||
} catch {
|
||||
// ignore unlock errors
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function parseTimestamp(value: string | undefined): number | null {
|
||||
if (!value) return null;
|
||||
const parsed = Date.parse(value);
|
||||
if (!Number.isFinite(parsed)) return null;
|
||||
return parsed;
|
||||
}
|
||||
|
||||
function isExpired(entry: PairingRequest, nowMs: number): boolean {
|
||||
const createdAt = parseTimestamp(entry.createdAt);
|
||||
if (!createdAt) return true;
|
||||
return nowMs - createdAt > PAIRING_PENDING_TTL_MS;
|
||||
}
|
||||
|
||||
function pruneExpiredRequests(reqs: PairingRequest[], nowMs: number) {
|
||||
const kept: PairingRequest[] = [];
|
||||
let removed = false;
|
||||
for (const req of reqs) {
|
||||
if (isExpired(req, nowMs)) {
|
||||
removed = true;
|
||||
continue;
|
||||
}
|
||||
kept.push(req);
|
||||
}
|
||||
return { requests: kept, removed };
|
||||
}
|
||||
|
||||
function randomCode(): string {
|
||||
// Human-friendly: 8 chars, upper, no ambiguous chars (0O1I).
|
||||
const alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789";
|
||||
let out = "";
|
||||
for (let i = 0; i < 8; i++) {
|
||||
out += alphabet[Math.floor(Math.random() * alphabet.length)];
|
||||
for (let i = 0; i < PAIRING_CODE_LENGTH; i++) {
|
||||
const idx = crypto.randomInt(0, PAIRING_CODE_ALPHABET.length);
|
||||
out += PAIRING_CODE_ALPHABET[idx];
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function generateUniqueCode(existing: Set<string>): string {
|
||||
for (let attempt = 0; attempt < 500; attempt += 1) {
|
||||
const code = randomCode();
|
||||
if (!existing.has(code)) return code;
|
||||
}
|
||||
throw new Error("failed to generate unique pairing code");
|
||||
}
|
||||
|
||||
function normalizeId(value: string | number): string {
|
||||
return String(value).trim();
|
||||
}
|
||||
@@ -129,26 +214,32 @@ export async function addProviderAllowFromStoreEntry(params: {
|
||||
}): Promise<{ changed: boolean; allowFrom: string[] }> {
|
||||
const env = params.env ?? process.env;
|
||||
const filePath = resolveAllowFromPath(params.provider, env);
|
||||
const { value } = await readJsonFile<AllowFromStore>(filePath, {
|
||||
version: 1,
|
||||
allowFrom: [],
|
||||
});
|
||||
const current = (Array.isArray(value.allowFrom) ? value.allowFrom : [])
|
||||
.map((v) => normalizeAllowEntry(params.provider, String(v)))
|
||||
.filter(Boolean);
|
||||
const normalized = normalizeAllowEntry(
|
||||
params.provider,
|
||||
normalizeId(params.entry),
|
||||
return await withFileLock(
|
||||
filePath,
|
||||
{ version: 1, allowFrom: [] } satisfies AllowFromStore,
|
||||
async () => {
|
||||
const { value } = await readJsonFile<AllowFromStore>(filePath, {
|
||||
version: 1,
|
||||
allowFrom: [],
|
||||
});
|
||||
const current = (Array.isArray(value.allowFrom) ? value.allowFrom : [])
|
||||
.map((v) => normalizeAllowEntry(params.provider, String(v)))
|
||||
.filter(Boolean);
|
||||
const normalized = normalizeAllowEntry(
|
||||
params.provider,
|
||||
normalizeId(params.entry),
|
||||
);
|
||||
if (!normalized) return { changed: false, allowFrom: current };
|
||||
if (current.includes(normalized))
|
||||
return { changed: false, allowFrom: current };
|
||||
const next = [...current, normalized];
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
allowFrom: next,
|
||||
} satisfies AllowFromStore);
|
||||
return { changed: true, allowFrom: next };
|
||||
},
|
||||
);
|
||||
if (!normalized) return { changed: false, allowFrom: current };
|
||||
if (current.includes(normalized))
|
||||
return { changed: false, allowFrom: current };
|
||||
const next = [...current, normalized];
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
allowFrom: next,
|
||||
} satisfies AllowFromStore);
|
||||
return { changed: true, allowFrom: next };
|
||||
}
|
||||
|
||||
export async function listProviderPairingRequests(
|
||||
@@ -156,21 +247,35 @@ export async function listProviderPairingRequests(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): Promise<PairingRequest[]> {
|
||||
const filePath = resolvePairingPath(provider, env);
|
||||
const { value } = await readJsonFile<PairingStore>(filePath, {
|
||||
version: 1,
|
||||
requests: [],
|
||||
});
|
||||
const reqs = Array.isArray(value.requests) ? value.requests : [];
|
||||
return reqs
|
||||
.filter(
|
||||
(r) =>
|
||||
r &&
|
||||
typeof r.id === "string" &&
|
||||
typeof r.code === "string" &&
|
||||
typeof r.createdAt === "string",
|
||||
)
|
||||
.slice()
|
||||
.sort((a, b) => a.createdAt.localeCompare(b.createdAt));
|
||||
return await withFileLock(
|
||||
filePath,
|
||||
{ version: 1, requests: [] } satisfies PairingStore,
|
||||
async () => {
|
||||
const { value } = await readJsonFile<PairingStore>(filePath, {
|
||||
version: 1,
|
||||
requests: [],
|
||||
});
|
||||
const reqs = Array.isArray(value.requests) ? value.requests : [];
|
||||
const nowMs = Date.now();
|
||||
const { requests: pruned, removed } = pruneExpiredRequests(reqs, nowMs);
|
||||
if (removed) {
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: pruned,
|
||||
} satisfies PairingStore);
|
||||
}
|
||||
return pruned
|
||||
.filter(
|
||||
(r) =>
|
||||
r &&
|
||||
typeof r.id === "string" &&
|
||||
typeof r.code === "string" &&
|
||||
typeof r.createdAt === "string",
|
||||
)
|
||||
.slice()
|
||||
.sort((a, b) => a.createdAt.localeCompare(b.createdAt));
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
export async function upsertProviderPairingRequest(params: {
|
||||
@@ -181,56 +286,75 @@ export async function upsertProviderPairingRequest(params: {
|
||||
}): Promise<{ code: string; created: boolean }> {
|
||||
const env = params.env ?? process.env;
|
||||
const filePath = resolvePairingPath(params.provider, env);
|
||||
const { value } = await readJsonFile<PairingStore>(filePath, {
|
||||
version: 1,
|
||||
requests: [],
|
||||
});
|
||||
const now = new Date().toISOString();
|
||||
const id = normalizeId(params.id);
|
||||
const meta =
|
||||
params.meta && typeof params.meta === "object"
|
||||
? Object.fromEntries(
|
||||
Object.entries(params.meta)
|
||||
.map(([k, v]) => [k, String(v ?? "").trim()] as const)
|
||||
.filter(([_, v]) => Boolean(v)),
|
||||
)
|
||||
: undefined;
|
||||
return await withFileLock(
|
||||
filePath,
|
||||
{ version: 1, requests: [] } satisfies PairingStore,
|
||||
async () => {
|
||||
const { value } = await readJsonFile<PairingStore>(filePath, {
|
||||
version: 1,
|
||||
requests: [],
|
||||
});
|
||||
const now = new Date().toISOString();
|
||||
const nowMs = Date.now();
|
||||
const id = normalizeId(params.id);
|
||||
const meta =
|
||||
params.meta && typeof params.meta === "object"
|
||||
? Object.fromEntries(
|
||||
Object.entries(params.meta)
|
||||
.map(([k, v]) => [k, String(v ?? "").trim()] as const)
|
||||
.filter(([_, v]) => Boolean(v)),
|
||||
)
|
||||
: undefined;
|
||||
|
||||
const reqs = Array.isArray(value.requests) ? value.requests : [];
|
||||
const existingIdx = reqs.findIndex((r) => r.id === id);
|
||||
if (existingIdx >= 0) {
|
||||
const existing = reqs[existingIdx];
|
||||
const existingCode =
|
||||
existing && typeof existing.code === "string" ? existing.code.trim() : "";
|
||||
const code = existingCode || randomCode();
|
||||
const next: PairingRequest = {
|
||||
id,
|
||||
code,
|
||||
createdAt: existing?.createdAt ?? now,
|
||||
lastSeenAt: now,
|
||||
meta: meta ?? existing?.meta,
|
||||
};
|
||||
reqs[existingIdx] = next;
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: reqs,
|
||||
} satisfies PairingStore);
|
||||
return { code, created: false };
|
||||
}
|
||||
let reqs = Array.isArray(value.requests) ? value.requests : [];
|
||||
const { requests: pruned } = pruneExpiredRequests(reqs, nowMs);
|
||||
reqs = pruned;
|
||||
const existingIdx = reqs.findIndex((r) => r.id === id);
|
||||
const existingCodes = new Set(
|
||||
reqs.map((req) =>
|
||||
String(req.code ?? "")
|
||||
.trim()
|
||||
.toUpperCase(),
|
||||
),
|
||||
);
|
||||
|
||||
const code = randomCode();
|
||||
const next: PairingRequest = {
|
||||
id,
|
||||
code,
|
||||
createdAt: now,
|
||||
lastSeenAt: now,
|
||||
...(meta ? { meta } : {}),
|
||||
};
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: [...reqs, next],
|
||||
} satisfies PairingStore);
|
||||
return { code, created: true };
|
||||
if (existingIdx >= 0) {
|
||||
const existing = reqs[existingIdx];
|
||||
const existingCode =
|
||||
existing && typeof existing.code === "string"
|
||||
? existing.code.trim()
|
||||
: "";
|
||||
const code = existingCode || generateUniqueCode(existingCodes);
|
||||
const next: PairingRequest = {
|
||||
id,
|
||||
code,
|
||||
createdAt: existing?.createdAt ?? now,
|
||||
lastSeenAt: now,
|
||||
meta: meta ?? existing?.meta,
|
||||
};
|
||||
reqs[existingIdx] = next;
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: reqs,
|
||||
} satisfies PairingStore);
|
||||
return { code, created: false };
|
||||
}
|
||||
|
||||
const code = generateUniqueCode(existingCodes);
|
||||
const next: PairingRequest = {
|
||||
id,
|
||||
code,
|
||||
createdAt: now,
|
||||
lastSeenAt: now,
|
||||
...(meta ? { meta } : {}),
|
||||
};
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: [...reqs, next],
|
||||
} satisfies PairingStore);
|
||||
return { code, created: true };
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
export async function approveProviderPairingCode(params: {
|
||||
@@ -243,26 +367,42 @@ export async function approveProviderPairingCode(params: {
|
||||
if (!code) return null;
|
||||
|
||||
const filePath = resolvePairingPath(params.provider, env);
|
||||
const { value } = await readJsonFile<PairingStore>(filePath, {
|
||||
version: 1,
|
||||
requests: [],
|
||||
});
|
||||
const reqs = Array.isArray(value.requests) ? value.requests : [];
|
||||
const idx = reqs.findIndex(
|
||||
(r) => String(r.code ?? "").toUpperCase() === code,
|
||||
return await withFileLock(
|
||||
filePath,
|
||||
{ version: 1, requests: [] } satisfies PairingStore,
|
||||
async () => {
|
||||
const { value } = await readJsonFile<PairingStore>(filePath, {
|
||||
version: 1,
|
||||
requests: [],
|
||||
});
|
||||
const reqs = Array.isArray(value.requests) ? value.requests : [];
|
||||
const nowMs = Date.now();
|
||||
const { requests: pruned, removed } = pruneExpiredRequests(reqs, nowMs);
|
||||
const idx = pruned.findIndex(
|
||||
(r) => String(r.code ?? "").toUpperCase() === code,
|
||||
);
|
||||
if (idx < 0) {
|
||||
if (removed) {
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: pruned,
|
||||
} satisfies PairingStore);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
const entry = pruned[idx];
|
||||
if (!entry) return null;
|
||||
pruned.splice(idx, 1);
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: pruned,
|
||||
} satisfies PairingStore);
|
||||
await addProviderAllowFromStoreEntry({
|
||||
provider: params.provider,
|
||||
entry: entry.id,
|
||||
env,
|
||||
});
|
||||
return { id: entry.id, entry };
|
||||
},
|
||||
);
|
||||
if (idx < 0) return null;
|
||||
const entry = reqs[idx];
|
||||
if (!entry) return null;
|
||||
reqs.splice(idx, 1);
|
||||
await writeJsonFile(filePath, {
|
||||
version: 1,
|
||||
requests: reqs,
|
||||
} satisfies PairingStore);
|
||||
await addProviderAllowFromStoreEntry({
|
||||
provider: params.provider,
|
||||
entry: entry.id,
|
||||
env,
|
||||
});
|
||||
return { id: entry.id, entry };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user