refactor(security): enforce account-scoped pairing APIs

This commit is contained in:
Peter Steinberger
2026-02-26 21:57:10 +01:00
parent a0c5e28f3b
commit bce643a0bd
27 changed files with 331 additions and 94 deletions

View File

@@ -4,12 +4,15 @@ import os from "node:os";
import path from "node:path";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import { resolveOAuthDir } from "../config/paths.js";
import { DEFAULT_ACCOUNT_ID } from "../routing/session-key.js";
import { withEnvAsync } from "../test-utils/env.js";
import {
addChannelAllowFromStoreEntry,
approveChannelPairingCode,
listChannelPairingRequests,
readChannelAllowFromStore,
readLegacyChannelAllowFromStore,
readLegacyChannelAllowFromStoreSync,
readChannelAllowFromStoreSync,
removeChannelAllowFromStoreEntry,
upsertChannelPairingRequest,
@@ -69,10 +72,12 @@ describe("pairing store", () => {
const first = await upsertChannelPairingRequest({
channel: "discord",
id: "u1",
accountId: DEFAULT_ACCOUNT_ID,
});
const second = await upsertChannelPairingRequest({
channel: "discord",
id: "u1",
accountId: DEFAULT_ACCOUNT_ID,
});
expect(first.created).toBe(true);
expect(second.created).toBe(false);
@@ -89,6 +94,7 @@ describe("pairing store", () => {
const created = await upsertChannelPairingRequest({
channel: "signal",
id: "+15550001111",
accountId: DEFAULT_ACCOUNT_ID,
});
expect(created.created).toBe(true);
@@ -111,6 +117,7 @@ describe("pairing store", () => {
const next = await upsertChannelPairingRequest({
channel: "signal",
id: "+15550001111",
accountId: DEFAULT_ACCOUNT_ID,
});
expect(next.created).toBe(true);
});
@@ -128,6 +135,7 @@ describe("pairing store", () => {
const first = await upsertChannelPairingRequest({
channel: "telegram",
id: "123",
accountId: DEFAULT_ACCOUNT_ID,
});
expect(first.code).toBe("AAAAAAAA");
@@ -137,6 +145,7 @@ describe("pairing store", () => {
const second = await upsertChannelPairingRequest({
channel: "telegram",
id: "456",
accountId: DEFAULT_ACCOUNT_ID,
});
expect(second.code).toBe("BBBBBBBB");
} finally {
@@ -152,6 +161,7 @@ describe("pairing store", () => {
const created = await upsertChannelPairingRequest({
channel: "whatsapp",
id,
accountId: DEFAULT_ACCOUNT_ID,
});
expect(created.created).toBe(true);
}
@@ -159,6 +169,7 @@ describe("pairing store", () => {
const blocked = await upsertChannelPairingRequest({
channel: "whatsapp",
id: "+15550000004",
accountId: DEFAULT_ACCOUNT_ID,
});
expect(blocked.created).toBe(false);
@@ -181,7 +192,7 @@ describe("pairing store", () => {
});
const accountScoped = await readChannelAllowFromStore("telegram", process.env, "yy");
const channelScoped = await readChannelAllowFromStore("telegram");
const channelScoped = await readLegacyChannelAllowFromStore("telegram");
expect(accountScoped).toContain("12345");
expect(channelScoped).not.toContain("12345");
});
@@ -203,7 +214,7 @@ describe("pairing store", () => {
expect(approved?.id).toBe("12345");
const accountScoped = await readChannelAllowFromStore("telegram", process.env, "yy");
const channelScoped = await readChannelAllowFromStore("telegram");
const channelScoped = await readLegacyChannelAllowFromStore("telegram");
expect(accountScoped).toContain("12345");
expect(channelScoped).not.toContain("12345");
});
@@ -278,7 +289,7 @@ describe("pairing store", () => {
});
const scoped = readChannelAllowFromStoreSync("telegram", process.env, "yy");
const channelScoped = readChannelAllowFromStoreSync("telegram");
const channelScoped = readLegacyChannelAllowFromStoreSync("telegram");
expect(scoped).toEqual(["1002", "1001"]);
expect(channelScoped).toEqual(["1001"]);
});
@@ -380,7 +391,7 @@ describe("pairing store", () => {
allowFrom: ["1002"],
});
const scoped = await readChannelAllowFromStore("telegram", process.env, "default");
const scoped = await readChannelAllowFromStore("telegram", process.env, DEFAULT_ACCOUNT_ID);
expect(scoped).toEqual(["1002", "1001"]);
});
});

View File

@@ -8,6 +8,7 @@ import { resolveOAuthDir, resolveStateDir } from "../config/paths.js";
import { withFileLock as withPathLock } from "../infra/file-lock.js";
import { resolveRequiredHomeDir } from "../infra/home-dir.js";
import { readJsonFileWithFallback, writeJsonFileAtomically } from "../plugin-sdk/json-store.js";
import { DEFAULT_ACCOUNT_ID } from "../routing/session-key.js";
const PAIRING_CODE_LENGTH = 8;
const PAIRING_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789";
@@ -221,7 +222,7 @@ function requestMatchesAccountId(entry: PairingRequest, normalizedAccountId: str
function shouldIncludeLegacyAllowFromEntries(normalizedAccountId: string): boolean {
// Keep backward compatibility for legacy channel-scoped allowFrom only on default account.
// Non-default accounts should remain isolated to avoid cross-account implicit approvals.
return !normalizedAccountId || normalizedAccountId === "default";
return !normalizedAccountId || normalizedAccountId === DEFAULT_ACCOUNT_ID;
}
function normalizeId(value: string | number): string {
@@ -383,25 +384,30 @@ async function updateAllowFromStoreEntry(params: {
);
}
export async function readLegacyChannelAllowFromStore(
channel: PairingChannel,
env: NodeJS.ProcessEnv = process.env,
): Promise<string[]> {
const filePath = resolveAllowFromPath(channel, env);
return await readAllowFromStateForPath(channel, filePath);
}
export async function readChannelAllowFromStore(
channel: PairingChannel,
env: NodeJS.ProcessEnv = process.env,
accountId?: string,
accountId: string,
): Promise<string[]> {
const normalizedAccountId = accountId?.trim().toLowerCase() ?? "";
if (!normalizedAccountId) {
const filePath = resolveAllowFromPath(channel, env);
return await readAllowFromStateForPath(channel, filePath);
}
const normalizedAccountId = accountId.trim().toLowerCase();
const resolvedAccountId = normalizedAccountId || DEFAULT_ACCOUNT_ID;
if (!shouldIncludeLegacyAllowFromEntries(normalizedAccountId)) {
if (!shouldIncludeLegacyAllowFromEntries(resolvedAccountId)) {
return await readNonDefaultAccountAllowFrom({
channel,
env,
accountId: normalizedAccountId,
accountId: resolvedAccountId,
});
}
const scopedPath = resolveAllowFromPath(channel, env, accountId);
const scopedPath = resolveAllowFromPath(channel, env, resolvedAccountId);
const scopedEntries = await readAllowFromStateForPath(channel, scopedPath);
// Backward compatibility: legacy channel-level allowFrom store was unscoped.
// Keep honoring it for default account to prevent re-pair prompts after upgrades.
@@ -410,25 +416,30 @@ export async function readChannelAllowFromStore(
return dedupePreserveOrder([...scopedEntries, ...legacyEntries]);
}
export function readLegacyChannelAllowFromStoreSync(
channel: PairingChannel,
env: NodeJS.ProcessEnv = process.env,
): string[] {
const filePath = resolveAllowFromPath(channel, env);
return readAllowFromStateForPathSync(channel, filePath);
}
export function readChannelAllowFromStoreSync(
channel: PairingChannel,
env: NodeJS.ProcessEnv = process.env,
accountId?: string,
accountId: string,
): string[] {
const normalizedAccountId = accountId?.trim().toLowerCase() ?? "";
if (!normalizedAccountId) {
const filePath = resolveAllowFromPath(channel, env);
return readAllowFromStateForPathSync(channel, filePath);
}
const normalizedAccountId = accountId.trim().toLowerCase();
const resolvedAccountId = normalizedAccountId || DEFAULT_ACCOUNT_ID;
if (!shouldIncludeLegacyAllowFromEntries(normalizedAccountId)) {
if (!shouldIncludeLegacyAllowFromEntries(resolvedAccountId)) {
return readNonDefaultAccountAllowFromSync({
channel,
env,
accountId: normalizedAccountId,
accountId: resolvedAccountId,
});
}
const scopedPath = resolveAllowFromPath(channel, env, accountId);
const scopedPath = resolveAllowFromPath(channel, env, resolvedAccountId);
const scopedEntries = readAllowFromStateForPathSync(channel, scopedPath);
const legacyPath = resolveAllowFromPath(channel, env);
const legacyEntries = readAllowFromStateForPathSync(channel, legacyPath);
@@ -537,7 +548,7 @@ export async function listChannelPairingRequests(
export async function upsertChannelPairingRequest(params: {
channel: PairingChannel;
id: string | number;
accountId?: string;
accountId: string;
meta?: Record<string, string | undefined | null>;
env?: NodeJS.ProcessEnv;
/** Extension channels can pass their adapter directly to bypass registry lookup. */
@@ -552,7 +563,7 @@ export async function upsertChannelPairingRequest(params: {
const now = new Date().toISOString();
const nowMs = Date.now();
const id = normalizeId(params.id);
const normalizedAccountId = params.accountId?.trim();
const normalizedAccountId = normalizePairingAccountId(params.accountId) || DEFAULT_ACCOUNT_ID;
const baseMeta =
params.meta && typeof params.meta === "object"
? Object.fromEntries(
@@ -561,7 +572,7 @@ export async function upsertChannelPairingRequest(params: {
.filter(([_, v]) => Boolean(v)),
)
: undefined;
const meta = normalizedAccountId ? { ...baseMeta, accountId: normalizedAccountId } : baseMeta;
const meta = { ...baseMeta, accountId: normalizedAccountId };
let reqs = await readPairingRequests(filePath);
const { requests: prunedExpired, removed: expiredRemoved } = pruneExpiredRequests(
@@ -569,7 +580,7 @@ export async function upsertChannelPairingRequest(params: {
nowMs,
);
reqs = prunedExpired;
const normalizedMatchingAccountId = normalizePairingAccountId(normalizedAccountId);
const normalizedMatchingAccountId = normalizedAccountId;
const existingIdx = reqs.findIndex((r) => {
if (r.id !== id) {
return false;