Security/review 2 #8

Merged
iamdoubz merged 4 commits from security/review-2 into main 2026-10-28 16:59:55 -05:00
34 changed files with 654 additions and 79 deletions

No files matched your search

+8 -4
View File
@@ -18,6 +18,7 @@ import {
import { todayIn } from '../generator/load.ts'; import { todayIn } from '../generator/load.ts';
import { getSetting } from '../services/settings.ts'; import { getSetting } from '../services/settings.ts';
import { ContrackError, contrackBook, contrackInteraction, contrackSettings } from './contrack.ts'; import { ContrackError, contrackBook, contrackInteraction, contrackSettings } from './contrack.ts';
import { ON_THE_CALL } from '../services/cdr.ts';
/** A recorded call waits this long for its summary before it's logged without one. */ /** A recorded call waits this long for its summary before it's logged without one. */
const SUMMARY_WAIT_MS = 30 * 60_000; const SUMMARY_WAIT_MS = 30 * 60_000;
@@ -141,11 +142,13 @@ export async function logCallsToContrack(d: {
.limit(500); .limit(500);
// One entry per call and person (a person can be in a call as caller, rang and answered). // One entry per call and person (a person can be in a call as caller, rang and answered).
const todo = new Map<string, (typeof rows)[number] & { out: boolean }>(); // `heard`: made or answered it; a phone that only rang gets no recording or summary (08 §7 #3).
const todo = new Map<string, (typeof rows)[number] & { out: boolean; heard: boolean }>();
for (const r of rows) { for (const r of rows) {
const k = `${r.call.id}|${r.userId}`; const k = `${r.call.id}|${r.userId}`;
const out = r.role === 'caller' || todo.get(k)?.out === true; const out = r.role === 'caller' || todo.get(k)?.out === true;
todo.set(k, { ...r, out }); const heard = ON_THE_CALL.includes(r.role) || todo.get(k)?.heard === true;
todo.set(k, { ...r, out, heard });
} }
const skipUser = new Set<string>(); // Contrack down or token refused: try again next run const skipUser = new Set<string>(); // Contrack down or token refused: try again next run
let logged = 0; let logged = 0;
@@ -169,7 +172,8 @@ export async function logCallsToContrack(d: {
await record(null); await record(null);
continue; continue;
} }
const wantSummary = r.withSummary && summariesOn && r.call.recordingId !== null; const recorded = r.heard && r.call.recordingId !== null;
const wantSummary = r.withSummary && summariesOn && recorded;
if (wantSummary && !r.aiDoneAt && now.getTime() - r.call.endedAt.getTime() < SUMMARY_WAIT_MS) if (wantSummary && !r.aiDoneAt && now.getTime() - r.call.endedAt.getTime() < SUMMARY_WAIT_MS)
continue; // the transcript job hasn't summarized it yet continue; // the transcript job hasn't summarized it yet
const e = timelineEntry( const e = timelineEntry(
@@ -180,7 +184,7 @@ export async function logCallsToContrack(d: {
toNumber: r.call.toNumber, toNumber: r.call.toNumber,
talkSeconds: r.call.talkSeconds, talkSeconds: r.call.talkSeconds,
answeredBy: r.answeredById && r.answeredById !== r.extId ? r.answeredBy : null, answeredBy: r.answeredById && r.answeredById !== r.extId ? r.answeredBy : null,
recorded: r.call.recordingId !== null, recorded,
}, },
wantSummary ? r.summary : null, wantSummary ? r.summary : null,
); );
+3
View File
@@ -29,6 +29,7 @@ import {
} from '../db/schema.ts'; } from '../db/schema.ts';
import { contactNames } from '../contacts/book.ts'; import { contactNames } from '../contacts/book.ts';
import { badRequest, conflict, notFound } from '../errors.ts'; import { badRequest, conflict, notFound } from '../errors.ts';
import { ON_THE_CALL } from '../services/cdr.ts';
import { getSetting } from '../services/settings.ts'; import { getSetting } from '../services/settings.ts';
const answerer = alias(extensions, 'answerer'); const answerer = alias(extensions, 'answerer');
@@ -228,6 +229,8 @@ export async function callRoutes(app: FastifyInstance) {
const out = mine.includes('caller'); const out = mine.includes('caller');
return { return {
...c, ...c,
// Only rang (a ring group): someone else's conversation (08 §7 #3).
recordingId: mine.some((r) => ON_THE_CALL.includes(r)) ? c.recordingId : null,
myRole: out ? 'out' : 'in', myRole: out ? 'out' : 'in',
missedByMe: !out && (MISSED as readonly string[]).includes(c.status), missedByMe: !out && (MISSED as readonly string[]).includes(c.status),
}; };
+3 -1
View File
@@ -270,8 +270,10 @@ export async function contactRoutes(app: FastifyInstance) {
.object({ url: z.string().trim().min(1), companyToken: z.string().trim().optional() }) .object({ url: z.string().trim().min(1), companyToken: z.string().trim().optional() })
.parse(req.body); .parse(req.body);
const saved = await getSetting(db, 'contacts.contrack'); const saved = await getSetting(db, 'contacts.contrack');
// The saved token only for the saved address: another one gets none (08 §7 #7).
const same = saved?.url === b.url.replace(/\/+$/, '');
const token = const token =
b.companyToken || (saved?.companyTokenEnc ? decrypt(saved.companyTokenEnc) : null); b.companyToken || (same && saved.companyTokenEnc ? decrypt(saved.companyTokenEnc) : null);
try { try {
return { ok: true, detail: await contrackCheck(b.url, token) }; return { ok: true, detail: await contrackCheck(b.url, token) };
} catch (err) { } catch (err) {
+21 -8
View File
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// Sign in with OpenID Connect, connect and disconnect an OpenID account (12 §3.2, §3.3). // Sign in with OpenID Connect, connect and disconnect an OpenID account (12 §3.2, §3.3).
import { and, eq, gt, isNull, notExists } from 'drizzle-orm'; import { and, eq, gt, isNull, ne, notExists } from 'drizzle-orm';
import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'; import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
import { z } from 'zod'; import { z } from 'zod';
import { decrypt, encrypt } from '../crypto/secrets.ts'; import { decrypt, encrypt } from '../crypto/secrets.ts';
@@ -15,7 +15,7 @@ import {
} from '../errors.ts'; } from '../errors.ts';
import { requireAdmin, requireAuth } from '../auth/guards.ts'; import { requireAdmin, requireAuth } from '../auth/guards.ts';
import { isLoginBlocked, recordLoginFailure } from '../auth/login-limiter.ts'; import { isLoginBlocked, recordLoginFailure } from '../auth/login-limiter.ts';
import { createSession, setSessionCookie } from '../auth/sessions.ts'; import { COOKIE, createSession, getSessionUser, setSessionCookie } from '../auth/sessions.ts';
import { audit } from '../services/audit.ts'; import { audit } from '../services/audit.ts';
import { getSetting } from '../services/settings.ts'; import { getSetting } from '../services/settings.ts';
import { decide, verifiedEmail } from '../sso/claims.ts'; import { decide, verifiedEmail } from '../sso/claims.ts';
@@ -146,7 +146,13 @@ export async function oidcRoutes(app: FastifyInstance) {
.where(and(eq(userIdentities.kind, 'oidc'), eq(userIdentities.subject, r.subject))); .where(and(eq(userIdentities.kind, 'oidc'), eq(userIdentities.subject, r.subject)));
if (connecting) { if (connecting) {
// Connect (12 §3.3): to the user of the session that asked, if it's still signed in. // Connect (12 §3.3): to the user of the session that asked, if it's still signed in, and
// only in that browser: a planted sp_oidc can't connect someone else's account (08 §7 #8).
const here = req.cookies[COOKIE]
? await getSessionUser(db, req.cookies[COOKIE], false)
: null;
if (here?.sessionId !== connecting)
return fail('expired', 'connect finished in another browser than the one that asked');
const [sess] = await db const [sess] = await db
.select({ userId: sessions.userId, email: users.email }) .select({ userId: sessions.userId, email: users.email })
.from(sessions) .from(sessions)
@@ -186,7 +192,8 @@ export async function oidcRoutes(app: FastifyInstance) {
let userId = owner?.userId; let userId = owner?.userId;
if (!userId) { if (!userId) {
// A user without a password or an OpenID account yet, by verified email (12 §3.3 step 2). // A user without a password or any connected account yet, by verified email (12 §3.3
// step 2). Never an admin: a provider account with their email isn't them (08 §7 #2).
const v = verifiedEmail(r.claims, s.trustEmail); const v = verifiedEmail(r.claims, s.trustEmail);
const [u] = v const [u] = v
? await db ? await db
@@ -196,13 +203,12 @@ export async function oidcRoutes(app: FastifyInstance) {
and( and(
eq(users.email, v), eq(users.email, v),
isNull(users.passwordHash), isNull(users.passwordHash),
ne(users.role, 'admin'),
notExists( notExists(
db db
.select({ id: userIdentities.id }) .select({ id: userIdentities.id })
.from(userIdentities) .from(userIdentities)
.where( .where(eq(userIdentities.userId, users.id)),
and(eq(userIdentities.userId, users.id), eq(userIdentities.kind, 'oidc')),
),
), ),
), ),
) )
@@ -322,8 +328,15 @@ export async function oidcRoutes(app: FastifyInstance) {
recordLoginFailure(req.ip, login); recordLoginFailure(req.ip, login);
throw badRequest('The directory refused that username or password'); throw badRequest('The directory refused that username or password');
} }
if ((await connectDirectory(db, req.log, req.user!.id, entry, req.ip)) === 'taken') const r = await connectDirectory(db, req.log, req.user!.id, entry, req.ip);
if (r === 'taken')
throw new HttpError(409, 'taken', 'That directory account belongs to another user'); throw new HttpError(409, 'taken', 'That directory account belongs to another user');
if (r === 'connected_other')
throw new HttpError(
409,
'connected_other',
'Another directory account is connected. Ask an admin to disconnect it first',
);
return { ok: true }; return { ok: true };
}); });
app.delete('/api/users/:id/identities/:kind', { preHandler: requireAdmin }, (req, reply) => { app.delete('/api/users/:id/identities/:kind', { preHandler: requireAdmin }, (req, reply) => {
+12 -2
View File
@@ -13,6 +13,7 @@ import { callExtensions, calls, extensions, recordings } from '../db/schema.ts';
import { badRequest, forbidden, notFound } from '../errors.ts'; import { badRequest, forbidden, notFound } from '../errors.ts';
import { audit } from '../services/audit.ts'; import { audit } from '../services/audit.ts';
import { sideFiles } from '../ai/calls.ts'; import { sideFiles } from '../ai/calls.ts';
import { ON_THE_CALL } from '../services/cdr.ts';
/** The recording and, when call transcripts left them, its two side files. */ /** The recording and, when call transcripts left them, its two side files. */
const removeFiles = async (dir: string, filePath: string) => { const removeFiles = async (dir: string, filePath: string) => {
@@ -109,7 +110,10 @@ export async function recordingRoutes(app: FastifyInstance) {
}, },
); );
/** Admins, or a participant: their extension is on the call this recording belongs to. */ /**
* Admins, or who was on the call: their extension made or answered it. Not a phone that only
* rang (a ring group, a queue): that's a coworker's conversation (08 §7 #3).
*/
async function mayHear(user: { id: string; role: string }, recordingId: string) { async function mayHear(user: { id: string; role: string }, recordingId: string) {
if (user.role === 'admin') return; if (user.role === 'admin') return;
const [mine] = await db const [mine] = await db
@@ -117,7 +121,13 @@ export async function recordingRoutes(app: FastifyInstance) {
.from(calls) .from(calls)
.innerJoin(callExtensions, eq(callExtensions.callId, calls.id)) .innerJoin(callExtensions, eq(callExtensions.callId, calls.id))
.innerJoin(extensions, eq(extensions.id, callExtensions.extensionId)) .innerJoin(extensions, eq(extensions.id, callExtensions.extensionId))
.where(and(eq(calls.recordingId, recordingId), eq(extensions.userId, user.id))) .where(
and(
eq(calls.recordingId, recordingId),
eq(extensions.userId, user.id),
inArray(callExtensions.role, ON_THE_CALL),
),
)
.limit(1); .limit(1);
if (!mine) throw forbidden('Not your call'); if (!mine) throw forbidden('Not your call');
} }
+42 -3
View File
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// /api/settings (04 §7.3) and the dashboard warnings. Admin only. // /api/settings (04 §7.3) and the dashboard warnings. Admin only.
import { eq, inArray } from 'drizzle-orm'; import { randomBytes } from 'node:crypto';
import { eq, inArray, isNotNull } from 'drizzle-orm';
import type { FastifyInstance } from 'fastify'; import type { FastifyInstance } from 'fastify';
import { z } from 'zod'; import { z } from 'zod';
import { import {
@@ -26,7 +27,7 @@ import {
type Warning, type Warning,
} from '@selfphoneng/shared'; } from '@selfphoneng/shared';
import type { Db } from '../db/client.ts'; import type { Db } from '../db/client.ts';
import { dids, extensions, trunks } from '../db/schema.ts'; import { dids, extensions, trunks, users } from '../db/schema.ts';
import { HttpError, badRequest } from '../errors.ts'; import { HttpError, badRequest } from '../errors.ts';
import { SLOW, requireAdmin } from '../auth/guards.ts'; import { SLOW, requireAdmin } from '../auth/guards.ts';
import { decrypt, encrypt } from '../crypto/secrets.ts'; import { decrypt, encrypt } from '../crypto/secrets.ts';
@@ -63,6 +64,7 @@ const READ_ONLY = new Set<string>([
'spam.state', 'spam.state',
'sms.webhookToken', 'sms.webhookToken',
'setup.completed', 'setup.completed',
'network.probeKeyEnc',
]); ]);
async function toDto(db: Db): Promise<SettingsDto> { async function toDto(db: Db): Promise<SettingsDto> {
@@ -72,10 +74,12 @@ async function toDto(db: Db): Promise<SettingsDto> {
'auth.oidc': oidc, 'auth.oidc': oidc,
'auth.ldap': ldap, 'auth.ldap': ldap,
spam, spam,
'network.probeKeyEnc': probeKey,
...rest ...rest
} = await getAllSettings(db); } = await getAllSettings(db);
return { return {
...rest, ...rest,
'network.hasProbeKey': probeKey !== null,
spam: { spam: {
...spam, ...spam,
ipqs: spam.ipqs && { hasKey: true }, ipqs: spam.ipqs && { hasKey: true },
@@ -257,6 +261,7 @@ export async function settingsRoutes(app: FastifyInstance) {
app.patch('/api/settings', async (req) => { app.patch('/api/settings', async (req) => {
const body = z.record(z.string(), z.unknown()).parse(req.body); const body = z.record(z.string(), z.unknown()).parse(req.body);
const values: Record<string, unknown> = {}; const values: Record<string, unknown> = {};
let contrackMoved = false;
for (const [key, raw] of Object.entries(body)) { for (const [key, raw] of Object.entries(body)) {
if (!(key in SettingsSchemas) || READ_ONLY.has(key)) if (!(key in SettingsSchemas) || READ_ONLY.has(key))
throw badRequest(`${key} can't be changed here`); throw badRequest(`${key} can't be changed here`);
@@ -287,8 +292,13 @@ export async function settingsRoutes(app: FastifyInstance) {
} catch (err) { } catch (err) {
throw badRequest(`Contrack address: ${(err as Error).message}`); throw badRequest(`Contrack address: ${(err as Error).message}`);
} }
// The saved tokens only go back to the address they were typed for (08 §7 #7).
const url = input?.url.replace(/\/+$/, '');
const moved = !!old && !!input && old.url !== url;
if (moved && old.companyTokenEnc && !input.companyToken && !input.clearCompanyToken)
throw badRequest('The Contrack address changed: enter the company token again');
values[key] = input && { values[key] = input && {
url: input.url.replace(/\/+$/, ''), url: url!,
publicUrl: input.publicUrl, publicUrl: input.publicUrl,
companyTokenEnc: input.companyToken companyTokenEnc: input.companyToken
? encrypt(input.companyToken) ? encrypt(input.companyToken)
@@ -296,6 +306,7 @@ export async function settingsRoutes(app: FastifyInstance) {
? '' ? ''
: (old?.companyTokenEnc ?? ''), : (old?.companyTokenEnc ?? ''),
}; };
if (moved) contrackMoved = true;
forgetBook(null); forgetBook(null);
} else if (key === 'auth.ldap') { } else if (key === 'auth.ldap') {
values[key] = await ldapFromInput(raw); values[key] = await ldapFromInput(raw);
@@ -349,8 +360,17 @@ export async function settingsRoutes(app: FastifyInstance) {
const cert = await readTlsCert(app.config.TLS_DIR); const cert = await readTlsCert(app.config.TLS_DIR);
if (!cert.ok) throw badRequest(`Can't turn on TLS. ${cert.error}`); if (!cert.ok) throw badRequest(`Can't turn on TLS. ${cert.error}`);
} }
// The probe's key was made for its address: another probe doesn't get it (08 §7 #9).
if ('network.probeUrl' in values && values['network.probeUrl'] !== before['network.probeUrl'])
values['network.probeKeyEnc'] = null;
for (const [key, value] of Object.entries(values)) for (const [key, value] of Object.entries(values))
await setSetting(db, key as SettingKey, value as never); await setSetting(db, key as SettingKey, value as never);
// People's own Contrack tokens were for the old address: they connect again (08 §7 #7).
if (contrackMoved)
await db
.update(users)
.set({ contrackTokenEnc: null })
.where(isNotNull(users.contrackTokenEnc));
// A new IPQualityScore key may have credits: don't wait for spamQuotaCheck (14 §3). // A new IPQualityScore key may have credits: don't wait for spamQuotaCheck (14 §3).
if ((body.spam as SpamInput | undefined)?.ipqs?.key) await resumeIpqs(db, 'a new key'); if ((body.spam as SpamInput | undefined)?.ipqs?.key) await resumeIpqs(db, 'a new key');
const changed = Object.keys(values).filter( const changed = Object.keys(values).filter(
@@ -474,6 +494,25 @@ export async function settingsRoutes(app: FastifyInstance) {
}, },
); );
/**
* A new key for the network test probe (08 §7 #9): shown this once, for its PROBE_KEY. Made for
* the probe address set now; the old key stops working on the next test.
*/
app.post('/api/settings/network/probe-key', async (req) => {
if (!(await getSetting(db, 'network.probeUrl')))
throw badRequest('Set the probe address first');
const key = randomBytes(32).toString('base64url');
await setSetting(db, 'network.probeKeyEnc', encrypt(key));
await audit(db, {
actorUserId: req.user!.id,
actorIp: req.ip,
action: 'update',
entityType: 'settings',
summary: 'Made a new network test probe key',
});
return { key };
});
/** The server's public IPv4, as an outside service sees it (only sent when asked). */ /** The server's public IPv4, as an outside service sees it (only sent when asked). */
app.post('/api/settings/network/detect-public-ip', { config: SLOW }, async () => { app.post('/api/settings/network/detect-public-ip', { config: SLOW }, async () => {
const ip = await detectPublicIp(); const ip = await detectPublicIp();
+6 -3
View File
@@ -138,6 +138,9 @@ export async function sendCallReports(d: {
aiDoneAt: recordings.aiDoneAt, aiDoneAt: recordings.aiDoneAt,
out: sql<boolean>`exists (select 1 from ${callExtensions} where ${callExtensions.callId} = ${calls.id} out: sql<boolean>`exists (select 1 from ${callExtensions} where ${callExtensions.callId} = ${calls.id}
and ${callExtensions.extensionId} = ${e.id} and ${callExtensions.role} = 'caller')`, and ${callExtensions.extensionId} = ${e.id} and ${callExtensions.role} = 'caller')`,
// Made or answered it: only rang means a coworker's conversation (08 §7 #3).
heard: sql<boolean>`exists (select 1 from ${callExtensions} where ${callExtensions.callId} = ${calls.id}
and ${callExtensions.extensionId} = ${e.id} and ${callExtensions.role} in ('caller', 'answered'))`,
}) })
.from(calls) .from(calls)
.leftJoin(answerer, eq(answerer.id, calls.answeredByExtensionId)) .leftJoin(answerer, eq(answerer.id, calls.answeredByExtensionId))
@@ -177,9 +180,9 @@ export async function sendCallReports(d: {
toNumber: r.c.toNumber, toNumber: r.c.toNumber,
talkSeconds: r.c.talkSeconds, talkSeconds: r.c.talkSeconds,
answeredBy: r.answeredById && r.answeredById !== e.id ? r.answeredBy : null, answeredBy: r.answeredById && r.answeredById !== e.id ? r.answeredBy : null,
recorded: r.c.recordingId !== null, recorded: r.heard && r.c.recordingId !== null,
summary: r.summary, summary: r.heard ? r.summary : null,
summaryPending: r.c.recordingId !== null && r.aiDoneAt === null, summaryPending: r.heard && r.c.recordingId !== null && r.aiDoneAt === null,
})); }));
const withSummaries = const withSummaries =
e.callReportSummaries && e.callReportSummaries &&
+6
View File
@@ -14,6 +14,12 @@ import { callExtensions, calls, cdr, dids, extensions, recordings } from '../db/
import { durationMs } from '../media/audio.ts'; import { durationMs } from '../media/audio.ts';
type CdrRow = typeof cdr.$inferSelect; type CdrRow = typeof cdr.$inferSelect;
/**
* The call_extensions roles of someone who was on the call, not just rung: they may hear its
* recording and read its transcript and summary (08 §7 #3).
*/
export const ON_THE_CALL = ['caller', 'answered'];
type Tx = Parameters<Parameters<Db['transaction']>[0]>[0]; type Tx = Parameters<Parameters<Db['transaction']>[0]>[0];
/** cdr_manager.conf has usegmtime = yes: "2026-10-18 19:28:37" is UTC. */ /** cdr_manager.conf has usegmtime = yes: "2026-10-18 19:28:37" is UTC. */
+23 -6
View File
@@ -9,6 +9,7 @@ import { BlockList, isIPv4 } from 'node:net';
import type { DiagnosticCheck, NetworkTestDto } from '@selfphoneng/shared'; import type { DiagnosticCheck, NetworkTestDto } from '@selfphoneng/shared';
import type { AmiClient } from '../ami/client.ts'; import type { AmiClient } from '../ami/client.ts';
import type { Config } from '../config.ts'; import type { Config } from '../config.ts';
import { decrypt } from '../crypto/secrets.ts';
import type { Db } from '../db/client.ts'; import type { Db } from '../db/client.ts';
import { detectPublicIp } from '../jobs.ts'; import { detectPublicIp } from '../jobs.ts';
import { getAllSettings } from './settings.ts'; import { getAllSettings } from './settings.ts';
@@ -221,15 +222,23 @@ export async function insideChecks(d: DiagDeps): Promise<DiagnosticCheck[]> {
); );
out.push( out.push(
s['network.probeUrl'] !s['network.probeUrl']
? check('probe', 'Network test probe', 'ok', s['network.probeUrl']) ? check(
: check(
'probe', 'probe',
'Network test probe', 'Network test probe',
'skip', 'skip',
'None set, so nothing is tested from outside your network.', 'None set, so nothing is tested from outside your network.',
'Run apps/probe on a machine outside this network (deploy/probe), then put its URL in Settings → Network.', 'Run apps/probe on a machine outside this network (deploy/probe), then put its URL in Settings → Network.',
), )
: s['network.probeKeyEnc']
? check('probe', 'Network test probe', 'ok', `${s['network.probeUrl']}, with its key`)
: check(
'probe',
'Network test probe',
'warn',
`${s['network.probeUrl']} has no key: anyone who finds it can use it.`,
'Settings → Network → Generate key, then set it as PROBE_KEY on the probe and restart it (strongly recommended).',
),
); );
return out; return out;
} }
@@ -289,14 +298,20 @@ export async function networkTest(
let ip: string | null = null; let ip: string | null = null;
let results: ProbeResult[] = []; let results: ProbeResult[] = [];
let probeError: string | null = null; let probeError: string | null = null;
let keyRefused = false;
const key = s['network.probeKeyEnc'];
try { try {
const r = await (d.fetchProbe ?? fetch)(new URL('/v1/check', probe), { const r = await (d.fetchProbe ?? fetch)(new URL('/v1/check', probe), {
method: 'POST', method: 'POST',
headers: { 'content-type': 'application/json' }, headers: {
'content-type': 'application/json',
...(key && { authorization: `Bearer ${decrypt(key)}` }),
},
body: JSON.stringify({ host, tests }), body: JSON.stringify({ host, tests }),
signal: AbortSignal.timeout(30_000), signal: AbortSignal.timeout(30_000),
}); });
const body = (await r.json()) as { ip?: string; results?: ProbeResult[]; error?: string }; const body = (await r.json()) as { ip?: string; results?: ProbeResult[]; error?: string };
keyRefused = r.status === 401;
if (!r.ok) probeError = body.error ?? `HTTP ${r.status}`; if (!r.ok) probeError = body.error ?? `HTTP ${r.status}`;
else ({ ip = null, results = [] } = body as { ip: string; results: ProbeResult[] }); else ({ ip = null, results = [] } = body as { ip: string; results: ProbeResult[] });
// Datagrams sent last arrive just after the answer. // Datagrams sent last arrive just after the answer.
@@ -316,7 +331,9 @@ export async function networkTest(
'Network test probe', 'Network test probe',
'fail', 'fail',
`${probe} didn't answer: ${probeError}`, `${probe} didn't answer: ${probeError}`,
'Check the probe runs and is reachable from this server, then try again.', keyRefused
? "The probe's PROBE_KEY isn't this server's key. Settings → Network → Generate key, set it as PROBE_KEY on the probe and restart it."
: 'Check the probe runs and is reachable from this server, then try again.',
), ),
], ],
}; };
+6 -5
View File
@@ -121,11 +121,12 @@ async function transcribeMessage(d: VoicemailDeps, row: Row): Promise<string | n
async function announce(d: VoicemailDeps, box: Box, found: Row) { async function announce(d: VoicemailDeps, box: Box, found: Row) {
const transcript = await transcribeMessage(d, found); const transcript = await transcribeMessage(d, found);
const row = { ...found, transcript }; const row = { ...found, transcript };
// Every mailbox, with or without a user (the live message is only for the user's own). // Every mailbox, with or without a user (the live message is only for the user's own). Not
void enqueueWebhook(d.db, 'voicemail.new', { // the transcript: webhooks go off this server, and transcripts never do (10 §1, 08 §7 #6).
...toVoicemailDto(row), const { transcript: _words, ...hook } = toVoicemailDto(row);
extension: box.number, void enqueueWebhook(d.db, 'voicemail.new', { ...hook, extension: box.number }).catch(
}).catch((err: unknown) => d.log.error({ err }, 'webhook enqueue failed')); (err: unknown) => d.log.error({ err }, 'webhook enqueue failed'),
);
if (box.userId) if (box.userId)
d.live.emit('message', { d.live.emit('message', {
type: 'voicemail.new', type: 'voicemail.new',
+27 -8
View File
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// A directory entry → its SelfPhoneNG user (12 §4.3): connected before, or by the directory's // A directory entry → its SelfPhoneNG user (12 §4.3): connected before, or by the directory's
// email. Never a new user (owner, 2026-10-25). // email. Never a new user (owner, 2026-10-25).
import { and, eq } from 'drizzle-orm'; import { and, eq, notExists } from 'drizzle-orm';
import { deleteUserSessions } from '../auth/sessions.ts'; import { deleteUserSessions } from '../auth/sessions.ts';
import type { Db } from '../db/client.ts'; import type { Db } from '../db/client.ts';
import { userIdentities, users } from '../db/schema.ts'; import { userIdentities, users } from '../db/schema.ts';
@@ -22,7 +22,21 @@ export async function directoryUserFor(db: Db, e: DirectoryEntry): Promise<UserR
.where(and(eq(userIdentities.kind, 'ldap'), eq(userIdentities.subject, e.id))); .where(and(eq(userIdentities.kind, 'ldap'), eq(userIdentities.subject, e.id)));
if (byId) return byId.u; if (byId) return byId.u;
if (!e.email) return null; if (!e.email) return null;
const [byEmail] = await db.select().from(users).where(eq(users.email, e.email)); // Nobody with another way in either: an OpenID user's account is theirs to connect (08 §7 #1).
const [byEmail] = await db
.select()
.from(users)
.where(
and(
eq(users.email, e.email),
notExists(
db
.select({ id: userIdentities.id })
.from(userIdentities)
.where(eq(userIdentities.userId, users.id)),
),
),
);
return byEmail && byEmailOk(byEmail) ? byEmail : null; return byEmail && byEmailOk(byEmail) ? byEmail : null;
} }
@@ -35,19 +49,27 @@ export async function directoryUserFor(db: Db, e: DirectoryEntry): Promise<UserR
const byEmailOk = (u: UserRow) => const byEmailOk = (u: UserRow) =>
u.role !== 'admin' && (u.passwordHash === null || u.mustChangePassword); u.role !== 'admin' && (u.passwordHash === null || u.mustChangePassword);
/** Connect a directory account the person proved (My settings, 12 §4.3). */ /**
* Connect a directory account the person proved (My settings, 12 §4.3). Not another one in place
* of the one they have: the leaver check follows it, so swapping is an admin's job (08 §7 #5).
*/
export async function connectDirectory( export async function connectDirectory(
db: Db, db: Db,
log: { info(o: object, m: string): void }, log: { info(o: object, m: string): void },
userId: string, userId: string,
e: DirectoryEntry, e: DirectoryEntry,
ip: string, ip: string,
): Promise<'connected' | 'taken'> { ): Promise<'connected' | 'taken' | 'connected_other'> {
const [owner] = await db const [owner] = await db
.select({ userId: userIdentities.userId }) .select({ userId: userIdentities.userId })
.from(userIdentities) .from(userIdentities)
.where(and(eq(userIdentities.kind, 'ldap'), eq(userIdentities.subject, e.id))); .where(and(eq(userIdentities.kind, 'ldap'), eq(userIdentities.subject, e.id)));
if (owner && owner.userId !== userId) return 'taken'; if (owner && owner.userId !== userId) return 'taken';
const [mine] = await db
.select({ subject: userIdentities.subject })
.from(userIdentities)
.where(and(eq(userIdentities.userId, userId), eq(userIdentities.kind, 'ldap')));
if (mine && mine.subject !== e.id) return 'connected_other';
await db.transaction(async (tx) => { await db.transaction(async (tx) => {
await tx await tx
.delete(userIdentities) .delete(userIdentities)
@@ -101,11 +123,8 @@ export async function directorySignIn(
entityId: u.id, entityId: u.id,
summary: `${u.email} connected directory account ${e.dn}`, summary: `${u.email} connected directory account ${e.dn}`,
}); });
} else if (mine.subject !== e.id) {
// Their email matches, but another directory account is already theirs: not this one.
log.info({ userId: u.id }, `directory entry ${e.dn} matches by email but isn't theirs`);
return null;
} else } else
// Found by this account's id (directoryUserFor never matches by email someone connected).
await db await db
.update(userIdentities) .update(userIdentities)
.set({ email: e.email, lastUsedAt: new Date() }) .set({ email: e.email, lastUsedAt: new Date() })
+39
View File
@@ -181,4 +181,43 @@ describe.skipIf(!hasTestDb)('call report email', () => {
await run('2026-10-23T13:06:00Z'); await run('2026-10-23T13:06:00Z');
expect(t.mails[1]!.text).not.toContain('quote'); expect(t.mails[1]!.text).not.toContain('quote');
}); });
it('no recording or summary of a call that only rang my phone (08 §7 #3)', async () => {
await dan('PATCH', '/api/me', { callReportSummaries: true });
const [rec] = await t.db
.insert(recordings)
.values({
linkedid: 'r4',
filePath: '2026/10/24/r4.wav',
startedAt: new Date('2026-10-24T15:00:00Z'),
summary: 'Secret pricing for Acme',
aiDoneAt: new Date(),
})
.returning();
const [c] = await t.db
.insert(calls)
.values({
linkedid: 'c4', // the ring group rang Dan; a coworker answered and talked
direction: 'inbound',
status: 'answered',
startedAt: new Date('2026-10-24T15:00:00Z'),
endedAt: new Date('2026-10-24T15:05:00Z'),
fromNumber: '13125557777',
talkSeconds: 300,
recordingId: rec!.id,
})
.returning();
await t.db.insert(callExtensions).values({ callId: c!.id, extensionId: extId, role: 'rang' });
await t.db
.update(extensions)
.set({ callReportSentAt: new Date('2026-10-24T13:00:00Z') })
.where(eq(extensions.id, extId));
t.mails.length = 0;
expect(await run('2026-10-25T13:06:00Z')).toBe(1);
const text = t.mails[0]!.text;
expect(text).toContain('Incoming from (312) 555-7777');
expect(text).not.toContain('Secret pricing');
expect(text).not.toContain('recorded');
expect(text).not.toContain('Summary not ready');
});
}); });
+44
View File
@@ -460,6 +460,19 @@ describe.skipIf(!hasTestDb)('contacts in Contrack', () => {
fake.state.readOnly.clear(); fake.state.readOnly.clear();
expect(await run()).toBe(0); expect(await run()).toBe(0);
expect(jane).toHaveLength(8); expect(jane).toHaveLength(8);
// Only rang Dan (a coworker took it): on his timeline, but not its recording or summary.
const [k12] = await t.db
.insert(calls)
.values(call('k12', 1.5, { recordingId: done!.id }))
.returning();
await t.db
.insert(callExtensions)
.values({ callId: k12!.id, extensionId: ext!.id, role: 'rang' });
forgetBook(danId);
expect(await run()).toBe(1);
expect(jane.at(-1)!.content).not.toMatch(/recorded|Acme/);
expect(jane.at(-1)).not.toHaveProperty('actionItem');
}); });
it('contacts from before Contrack are copied once, the builtin ones kept (C6)', async () => { it('contacts from before Contrack are copied once, the builtin ones kept (C6)', async () => {
@@ -521,4 +534,35 @@ describe.skipIf(!hasTestDb)('contacts in Contrack', () => {
]); ]);
expect((await dan('GET', '/api/me')).json().contrack).toBeNull(); expect((await dan('GET', '/api/me')).json().contrack).toBeNull();
}); });
it('the saved tokens only go to the address they were saved for (08 §7 #7)', async () => {
// Another machine on the network that answers like Contrack, and notes what it's sent.
const heard: string[] = [];
const other = createServer((req, res) => {
heard.push(String(req.headers.authorization ?? ''));
res.setHeader('content-type', 'application/json');
res.end('[]');
});
await new Promise<void>((ok) => other.listen(0, '127.0.0.1', ok));
const otherUrl = `http://127.0.0.1:${(other.address() as AddressInfo).port}`;
try {
await admin('POST', '/api/contacts/contrack/test', { url: otherUrl });
expect(heard.join(' ')).not.toContain('ctk_company');
// A new address needs its token typed again…
const kept = await admin('PATCH', '/api/settings', {
'contacts.contrack': { url: otherUrl },
});
expect(kept.statusCode).toBe(400);
expect(kept.json().error.message).toMatch(/token again/);
// …and people's own tokens are for the old one: they connect again.
const moved = await admin('PATCH', '/api/settings', {
'contacts.contrack': { url: otherUrl, companyToken: 'ctk_new_place' },
});
expect(moved.statusCode, moved.body).toBe(200);
const [d] = await t.db.select().from(users).where(eq(users.id, danId));
expect(d!.contrackTokenEnc).toBeNull();
} finally {
other.close();
}
});
}); });
+51
View File
@@ -154,4 +154,55 @@ describe.skipIf(!hasTestDb)('diagnostics', () => {
401, 401,
); );
}); });
it('the probe key: made here, shown once, sent to the probe, never in settings (08 §7 #9)', async () => {
const cookie = await t.login(ADMIN.email, ADMIN.password);
const as = (method: string, url: string, payload?: object) =>
t.app.inject({ method: method as 'GET', url, payload, headers: { ...H, cookie } });
await setSetting(t.db, 'network.probeUrl', 'https://probe.test');
// No key yet: the inside checks say to make one.
expect(byId(await insideChecks(deps)).probe).toMatchObject({ status: 'warn' });
const g = await as('POST', '/api/settings/network/probe-key');
expect(g.statusCode, g.body).toBe(200);
const { key } = g.json() as { key: string };
expect(key).toMatch(/^[A-Za-z0-9_-]{43}$/);
const s = await as('GET', '/api/settings');
expect(s.json()['network.hasProbeKey']).toBe(true);
expect(s.body).not.toContain(key);
expect(s.json()).not.toHaveProperty('network.probeKeyEnc');
expect((await as('PATCH', '/api/settings', { 'network.probeKeyEnc': 'x' })).statusCode).toBe(
400,
);
expect(byId(await insideChecks(deps)).probe).toMatchObject({ status: 'ok' });
let auth: string | null = null;
await networkTest(
{
...deps,
fetchProbe: (async (_u: URL, init: RequestInit) => {
auth = new Headers(init.headers).get('authorization');
return new Response(JSON.stringify({ ip: '203.0.113.10', results: [] }));
}) as unknown as typeof fetch,
},
'quick',
);
expect(auth).toBe(`Bearer ${key}`);
// The probe refusing it says what to do.
const refused = await networkTest(
{
...deps,
fetchProbe: (async () =>
new Response(JSON.stringify({ error: 'a key is needed' }), {
status: 401,
})) as unknown as typeof fetch,
},
'quick',
);
expect(byId(refused.checks).probe!.fix).toContain('PROBE_KEY');
// Another address: the key was for the old probe, so it's gone.
await as('PATCH', '/api/settings', { 'network.probeUrl': 'https://other-probe.test' });
expect((await as('GET', '/api/settings')).json()['network.hasProbeKey']).toBe(false);
});
}); });
+35 -2
View File
@@ -1,12 +1,13 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// 12 §4.3: who a directory entry may connect to by email, who may disconnect it (§4.5), and a // 12 §4.3: who a directory entry may connect to by email, who may disconnect it (§4.5), and a
// directory that's down counting as a failed sign-in (§4.2). // directory that's down counting as a failed sign-in (§4.2). 08 §7: no OpenID user by email, no
// swapping the connected account.
import { eq } from 'drizzle-orm'; import { eq } from 'drizzle-orm';
import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { resetLoginLimiter } from '../src/auth/login-limiter.ts'; import { resetLoginLimiter } from '../src/auth/login-limiter.ts';
import { userIdentities, users } from '../src/db/schema.ts'; import { userIdentities, users } from '../src/db/schema.ts';
import { setSetting } from '../src/services/settings.ts'; import { setSetting } from '../src/services/settings.ts';
import { directorySignIn } from '../src/sso/directory.ts'; import { connectDirectory, directorySignIn } from '../src/sso/directory.ts';
import type { DirectoryEntry } from '../src/sso/ldap.ts'; import type { DirectoryEntry } from '../src/sso/ldap.ts';
import { hasTestDb } from './db.ts'; import { hasTestDb } from './db.ts';
import { ADMIN, H, createTestApp } from './app.ts'; import { ADMIN, H, createTestApp } from './app.ts';
@@ -52,6 +53,38 @@ describe.skipIf(!hasTestDb)('directory accounts by email (12 §4.3)', () => {
} }
}); });
it('never connects by email someone who signs in with OpenID (08 §7 #1)', async () => {
// A directory that lets people edit their own mail: Mallory's entry, the victim's email.
const victim = await t.addUser('oidc-only@example.com', 'unused-pw-2');
await t.db.update(users).set({ passwordHash: null }).where(eq(users.id, victim.id));
await t.db
.insert(userIdentities)
.values({ userId: victim.id, kind: 'oidc', subject: 'https://idp victim-sub' });
expect(await directorySignIn(t.db, log, entry(victim.email), '127.0.0.1')).toBeNull();
expect(await connected(victim.id)).toBe(1); // the OpenID account only
});
it('a connected directory account is only swapped by an admin (08 §7 #5)', async () => {
const u = await t.addUser('swapper@example.com', 'swapper-password-1');
expect(await connectDirectory(t.db, log, u.id, entry(u.email), '127.0.0.1')).toBe('connected');
const [first] = await t.db.select().from(userIdentities).where(eq(userIdentities.userId, u.id));
// The same account again is fine; another one (a shared mailbox) is not.
expect(
await connectDirectory(
t.db,
log,
u.id,
{ ...entry(u.email), id: first!.subject },
'127.0.0.1',
),
).toBe('connected');
expect(await connectDirectory(t.db, log, u.id, entry('shared@example.com'), '127.0.0.1')).toBe(
'connected_other',
);
const now = await t.db.select().from(userIdentities).where(eq(userIdentities.userId, u.id));
expect(now.map((i) => i.subject)).toEqual([first!.subject]);
});
it('only an admin can disconnect a directory account (a leaver must stay checked)', async () => { it('only an admin can disconnect a directory account (a leaver must stay checked)', async () => {
const u = await t.addUser('leaver@example.com', 'leaver-password-1'); const u = await t.addUser('leaver@example.com', 'leaver-password-1');
await t.db.insert(userIdentities).values({ userId: u.id, kind: 'ldap', subject: 'leaver-id' }); await t.db.insert(userIdentities).values({ userId: u.id, kind: 'ldap', subject: 'leaver-id' });
+50
View File
@@ -207,6 +207,56 @@ for (const alg of ['RS256', 'ES256'] as const)
expect(ids).toMatchObject([{ kind: 'oidc', subject: `${idp.issuer} u-cara` }]); expect(ids).toMatchObject([{ kind: 'oidc', subject: `${idp.issuer} u-cara` }]);
}); });
it('never matches by email an admin, or someone with a directory account (08 §7 #2)', async () => {
const [boss, dir] = await t.db
.insert(users)
.values([
{ email: 'diradmin@example.com', displayName: 'Boss', passwordHash: null, role: 'admin' },
{ email: 'diruser@example.com', displayName: 'Dir', passwordHash: null },
])
.returning();
await t.db
.insert(userIdentities)
.values({ userId: dir!.id, kind: 'ldap', subject: 'dir-user-id' });
for (const u of [boss!, dir!]) {
idp.st.user = { sub: `u-${u.email}`, email: u.email, email_verified: true };
const { callback } = await flow();
expect(errorOf(callback!)).toBe('not_connected');
expect(session(callback!)).toBeNull();
}
});
it('connect finishes only in the browser whose session asked (08 §7 #8)', async () => {
// Mallory starts Connect, then plants her sp_oidc cookie in Vic's browser and sends him on.
await t.addUser('mallory@example.com', 'mallory-password-1');
const [m] = await t.db.select().from(users).where(eq(users.email, 'mallory@example.com'));
await t.addUser('vic@example.com', 'vic-password-123');
const mallory = await t.login('mallory@example.com', 'mallory-password-1');
const vic = await t.login('vic@example.com', 'vic-password-123');
idp.st.user = { sub: 'u-vic', email: 'vic@example.com', email_verified: true };
for (const theirs of [vic, '']) {
const start = await t.app.inject({
method: 'POST',
url: '/api/auth/oidc/connect',
headers: { ...H, cookie: mallory },
payload: {},
});
const pending = start.cookies.find((c) => c.name === 'sp_oidc')!;
const atIdp = await fetch((start.json() as { url: string }).url, { redirect: 'manual' });
const back = new URL(atIdp.headers.get('location')!);
const callback = await t.app.inject({
url: back.pathname + back.search,
headers: { cookie: [`sp_oidc=${pending.value}`, theirs].filter(Boolean).join('; ') },
});
expect(errorOf(callback)).toBe('expired');
const ids = await t.db
.select()
.from(userIdentities)
.where(eq(userIdentities.userId, m!.id));
expect(ids).toEqual([]);
}
});
it('trustEmail matches an unverified email', async () => { it('trustEmail matches an unverified email', async () => {
await t.db await t.db
.insert(users) .insert(users)
+24 -2
View File
@@ -1,13 +1,13 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// M7-T3: a recorded call (sp_recording) is linked to its calls row and playable by admins and the // M7-T3: a recorded call (sp_recording) is linked to its calls row and playable by admins and the
// people on the call; delete and retention remove file and row. // people on the call (not a phone that only rang, 08 §7 #3); delete and retention remove file and row.
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs';
import { join } from 'node:path'; import { join } from 'node:path';
import { eq } from 'drizzle-orm'; import { eq } from 'drizzle-orm';
import type { LightMyRequestResponse } from 'fastify'; import type { LightMyRequestResponse } from 'fastify';
import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import type { AmiMessage } from '../src/ami/events.ts'; import type { AmiMessage } from '../src/ami/events.ts';
import { calls, cdr, extensions, recordings } from '../src/db/schema.ts'; import { callExtensions, calls, cdr, extensions, recordings, users } from '../src/db/schema.ts';
import { encrypt } from '../src/crypto/secrets.ts'; import { encrypt } from '../src/crypto/secrets.ts';
import { cleanupRecordings } from '../src/routes/recordings.ts'; import { cleanupRecordings } from '../src/routes/recordings.ts';
import { aggregateCall, toCdrRow } from '../src/services/cdr.ts'; import { aggregateCall, toCdrRow } from '../src/services/cdr.ts';
@@ -56,6 +56,8 @@ describe.skipIf(!hasTestDb)('call recordings', () => {
}; };
}); });
afterAll(() => t?.close()); afterAll(() => t?.close());
const bobId = async () =>
(await t.db.select().from(users).where(eq(users.email, 'bob@example.com')))[0]!.id;
it('the recording is linked to its call, with size and length', async () => { it('the recording is linked to its call, with size and length', async () => {
const [c] = await t.db.select().from(calls); const [c] = await t.db.select().from(calls);
@@ -91,6 +93,26 @@ describe.skipIf(!hasTestDb)('call recordings', () => {
expect((await as.alice('GET', '/api/recordings')).statusCode).toBe(403); expect((await as.alice('GET', '/api/recordings')).statusCode).toBe(403);
}); });
it('a phone that only rang (ring group, queue) gets neither audio nor transcript (08 §7 #3)', async () => {
const [bob] = await t.db
.insert(extensions)
.values({
number: '102',
name: 'Bob',
userId: await bobId(),
sipPasswordEnc: encrypt('x'),
voicemailPinEnc: encrypt('4826'),
})
.returning();
const [c] = await t.db.select().from(calls);
await t.db.insert(callExtensions).values({ callId: c!.id, extensionId: bob!.id, role: 'rang' });
expect((await as.bob('GET', `/api/recordings/${recId}/audio`)).statusCode).toBe(403);
expect((await as.bob('GET', `/api/recordings/${recId}/transcript`)).statusCode).toBe(403);
const mine = (await as.bob('GET', '/api/me/calls')).json();
expect(mine.items).toEqual([expect.objectContaining({ id: c!.id, recordingId: null })]);
expect((await as.alice('GET', `/api/recordings/${recId}/transcript`)).statusCode).toBe(200);
});
it('retention deletes recordings past their days; delete removes file and link', async () => { it('retention deletes recordings past their days; delete removes file and link', async () => {
expect(await cleanupRecordings(t.db, dir(), 3650)).toBe(0); expect(await cleanupRecordings(t.db, dir(), 3650)).toBe(0);
expect((await as.admin('DELETE', `/api/recordings/${recId}`)).statusCode).toBe(200); expect((await as.admin('DELETE', `/api/recordings/${recId}`)).statusCode).toBe(200);
+11 -1
View File
@@ -7,7 +7,7 @@ import { join } from 'node:path';
import { eq } from 'drizzle-orm'; import { eq } from 'drizzle-orm';
import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import type { LiveMessage } from '@selfphoneng/shared'; import type { LiveMessage } from '@selfphoneng/shared';
import { extensions, voicemailMessages } from '../src/db/schema.ts'; import { extensions, voicemailMessages, webhookDeliveries, webhooks } from '../src/db/schema.ts';
import { parseCallerId, type watchVoicemail } from '../src/services/voicemail.ts'; import { parseCallerId, type watchVoicemail } from '../src/services/voicemail.ts';
import { setSetting } from '../src/services/settings.ts'; import { setSetting } from '../src/services/settings.ts';
import { hasTestDb } from './db.ts'; import { hasTestDb } from './db.ts';
@@ -175,11 +175,21 @@ describe.skipIf(!hasTestDb)('voicemail watcher', () => {
const url = `http://127.0.0.1:${(stt.address() as AddressInfo).port}/v1`; const url = `http://127.0.0.1:${(stt.address() as AddressInfo).port}/v1`;
await setSetting(t.db, 'ai.stt', { url, model: 'whisper' }); await setSetting(t.db, 'ai.stt', { url, model: 'whisper' });
await setSetting(t.db, 'ai.voicemailTranscripts', true); await setSetting(t.db, 'ai.voicemailTranscripts', true);
await t.db
.insert(webhooks)
.values({ url: 'https://hooks.example.com/x', secretEnc: 'x', events: ['voicemail.new'] });
try { try {
leave('INBOX', 3, '1792300700-00000004', '<104>'); leave('INBOX', 3, '1792300700-00000004', '<104>');
await w.scan('101'); await w.scan('101');
const r = (await rows()).find((x) => x.msgId === '1792300700-00000004')!; const r = (await rows()).find((x) => x.msgId === '1792300700-00000004')!;
expect(r.transcript).toBe('Please call me.'); expect(r.transcript).toBe('Please call me.');
// A webhook goes off this server: the words stay here (10 §1, 08 §7 #6).
const [hook] = await t.db
.select()
.from(webhookDeliveries)
.where(eq(webhookDeliveries.event, 'voicemail.new'));
expect((hook!.payload as { data: object }).data).toMatchObject({ id: r.id });
expect((hook!.payload as { data: object }).data).not.toHaveProperty('transcript');
expect(t.mails.at(-1)!.text).toContain( expect(t.mails.at(-1)!.text).toContain(
'What they said (automatic, may have mistakes):\nPlease call me.', 'What they said (automatic, may have mistakes):\nPlease call me.',
); );
+14 -3
View File
@@ -1,10 +1,21 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// The network test probe (08 §2.6): run it outside the network SelfPhoneNG is in, and put its URL // The network test probe (08 §2.6): run it outside the network SelfPhoneNG is in, and put its URL
// in Settings → Network. `node src/index.ts` (Node 24 runs TypeScript as is). // in Settings → Network. `node src/index.ts` (Node 24 runs TypeScript as is).
// PORT (default 8080), TRUST_PROXY=true behind a reverse proxy that sets X-Forwarded-For. // PORT (default 8080), TRUST_PROXY=true behind a reverse proxy on this machine that sets
// X-Forwarded-For. HOST: where to listen; with TRUST_PROXY=true 127.0.0.1 (only the proxy may
// reach it), else every address. PROBE_KEY: the key from SelfPhoneNG's Settings → Network;
// without one anyone who finds the probe can use it.
import { createProbe } from './server.ts'; import { createProbe } from './server.ts';
const port = Number(process.env.PORT ?? 8080); const port = Number(process.env.PORT ?? 8080);
const probe = createProbe({ trustProxy: process.env.TRUST_PROXY === 'true' }); const trustProxy = process.env.TRUST_PROXY === 'true';
probe.listen(port, () => console.log(`selfphoneng-probe listening on ${port}`)); const host = process.env.HOST ?? (trustProxy ? '127.0.0.1' : '::');
const key = process.env.PROBE_KEY || undefined;
const probe = createProbe({ trustProxy, key });
probe.listen(port, host, () => console.log(`selfphoneng-probe listening on ${host} ${port}`));
if (!key)
console.warn(
'No PROBE_KEY: anyone who finds this probe can use it. Make one in SelfPhoneNG ' +
'(Settings → Network → Generate key) and set PROBE_KEY.',
);
for (const sig of ['SIGINT', 'SIGTERM']) process.on(sig, () => probe.close(() => process.exit(0))); for (const sig of ['SIGINT', 'SIGTERM']) process.on(sig, () => probe.close(() => process.exit(0)));
+44 -3
View File
@@ -1,12 +1,18 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// The probe against stand-ins on 127.0.0.1: TCP, HTTP and a WebSocket upgrade, SIP over UDP and // The probe against stand-ins on 127.0.0.1: TCP, HTTP and a WebSocket upgrade, SIP over UDP and
// TCP, raw UDP; and the server: it only tests the address that asked, validates, rate-limits. // TCP, raw UDP; and the server: it only tests the address that asked, validates, rate-limits,
// believes X-Forwarded-For only from a proxy on this machine (08 §7 #4), and with PROBE_KEY only
// answers who sends it (#9).
import { createSocket, type Socket as UdpSocket } from 'node:dgram'; import { createSocket, type Socket as UdpSocket } from 'node:dgram';
import { createServer as httpServer, type Server as HttpServer } from 'node:http'; import {
createServer as httpServer,
type IncomingMessage,
type Server as HttpServer,
} from 'node:http';
import { createServer as tcpServer, type AddressInfo, type Server } from 'node:net'; import { createServer as tcpServer, type AddressInfo, type Server } from 'node:net';
import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { runTest } from './checks.ts'; import { runTest } from './checks.ts';
import { createProbe, parseCheck } from './server.ts'; import { clientIp, createProbe, parseCheck } from './server.ts';
const port = (s: Server | HttpServer) => (s.address() as AddressInfo).port; const port = (s: Server | HttpServer) => (s.address() as AddressInfo).port;
const listen = <T extends Server | HttpServer>(s: T) => const listen = <T extends Server | HttpServer>(s: T) =>
@@ -165,3 +171,38 @@ describe('probe server', () => {
target.close(); target.close();
}); });
}); });
describe('the probe key', () => {
it('with a key, only a request that sends it gets tested (08 §7 #9)', async () => {
const key = 'k3y'.repeat(15);
const probe = await listen(createProbe({ key }));
const post = (authorization?: string) =>
fetch(`http://127.0.0.1:${port(probe)}/v1/check`, {
method: 'POST',
headers: { 'content-type': 'application/json', ...(authorization && { authorization }) },
body: JSON.stringify({ host: 'pbx.test', tests: [{ id: 'a', kind: 'tcp', port: 1 }] }),
});
expect((await post()).status).toBe(401);
expect((await post('Bearer wrong')).status).toBe(401);
expect((await post(`Bearer ${key}x`)).status).toBe(401);
expect((await post(`Bearer ${key}`)).status).toBe(200);
expect((await fetch(`http://127.0.0.1:${port(probe)}/healthz`)).status).toBe(200);
probe.close();
});
});
describe('the address that asked', () => {
const req = (peer: string, xff?: string) =>
({
headers: xff ? { 'x-forwarded-for': xff } : {},
socket: { remoteAddress: peer },
}) as unknown as IncomingMessage;
it('takes X-Forwarded-For only from the local proxy (08 §7 #4)', () => {
// Reached straight from the internet: the header is whatever the caller wrote.
expect(clientIp(req('203.0.113.5', '198.51.100.9'), true)).toBe('203.0.113.5');
for (const proxy of ['127.0.0.1', '::ffff:127.0.0.1', '::1'])
expect(clientIp(req(proxy, '10.0.0.1, 198.51.100.9'), true)).toBe('198.51.100.9');
expect(clientIp(req('::ffff:203.0.113.5', '198.51.100.9'), false)).toBe('203.0.113.5');
});
});
+26 -8
View File
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
// POST /v1/check {host, tests}: runs the tests against the address the request came from and // POST /v1/check {host, tests}: runs the tests against the address the request came from and
// answers {ip, results}. GET /healthz. No other target can be given, which is what keeps the probe // answers {ip, results}. GET /healthz. No other target can be given, which is what keeps the probe
// from being aimed at someone else; each address gets a few checks per 10 minutes. // from being aimed at someone else; each address gets a few checks per 10 minutes. With a key
// (PROBE_KEY, made in SelfPhoneNG's Settings → Network), only who sends it is tested (08 §7 #9).
import { createHash, timingSafeEqual } from 'node:crypto';
import { createServer, type IncomingMessage, type Server } from 'node:http'; import { createServer, type IncomingMessage, type Server } from 'node:http';
import { isIP } from 'node:net'; import { isIP } from 'node:net';
import { runTest, type Result, type Test } from './checks.ts'; import { runTest, type Result, type Test } from './checks.ts';
@@ -9,6 +11,8 @@ import { runTest, type Result, type Test } from './checks.ts';
export type ProbeOptions = { export type ProbeOptions = {
/** Behind a reverse proxy: the address is the last X-Forwarded-For entry (the proxy adds it). */ /** Behind a reverse proxy: the address is the last X-Forwarded-For entry (the proxy adds it). */
trustProxy?: boolean; trustProxy?: boolean;
/** Only requests with `Authorization: Bearer <key>` are tested. None: anyone may ask. */
key?: string;
/** Checks per address per window. */ /** Checks per address per window. */
maxChecks?: number; maxChecks?: number;
windowMs?: number; windowMs?: number;
@@ -48,15 +52,27 @@ export function parseCheck(body: unknown): { host: string; tests: Test[] } | str
return { host: b.host, tests }; return { host: b.host, tests };
} }
const clientIp = (req: IncomingMessage, trustProxy: boolean) => { /**
* The address to test. X-Forwarded-For only counts when the request came from a proxy on this
* machine: reached directly, the header is whatever the caller wrote (08 §7 #4).
*/
export const clientIp = (req: IncomingMessage, trustProxy: boolean) => {
const peer = req.socket.remoteAddress!.replace(/^::ffff:/, '');
const xff = req.headers['x-forwarded-for']; const xff = req.headers['x-forwarded-for'];
const raw = const local = peer === '127.0.0.1' || peer === '::1';
trustProxy && typeof xff === 'string' return trustProxy && local && typeof xff === 'string'
? xff.split(',').at(-1)!.trim() ? xff
: req.socket.remoteAddress!; .split(',')
return raw.replace(/^::ffff:/, ''); .at(-1)!
.trim()
.replace(/^::ffff:/, '')
: peer;
}; };
/** Equal, in the same time whatever was sent (hashes, so lengths don't show either). */
const sameKey = (a: string, b: string) =>
timingSafeEqual(createHash('sha256').update(a).digest(), createHash('sha256').update(b).digest());
function readJson(req: IncomingMessage): Promise<unknown> { function readJson(req: IncomingMessage): Promise<unknown> {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
let size = 0; let size = 0;
@@ -78,7 +94,7 @@ function readJson(req: IncomingMessage): Promise<unknown> {
} }
export function createProbe(opts: ProbeOptions = {}): Server { export function createProbe(opts: ProbeOptions = {}): Server {
const { trustProxy = false, maxChecks = 10, windowMs = 10 * 60_000 } = opts; const { trustProxy = false, key, maxChecks = 10, windowMs = 10 * 60_000 } = opts;
// ponytail: in-process memory; addresses past the window are swept on each check. // ponytail: in-process memory; addresses past the window are swept on each check.
const seen = new Map<string, number[]>(); const seen = new Map<string, number[]>();
const allow = (ip: string, now = Date.now()) => { const allow = (ip: string, now = Date.now()) => {
@@ -95,6 +111,8 @@ export function createProbe(opts: ProbeOptions = {}): Server {
if (req.method === 'GET' && req.url === '/healthz') return send(res, 200, { ok: true }); if (req.method === 'GET' && req.url === '/healthz') return send(res, 200, { ok: true });
if (req.method !== 'POST' || req.url !== '/v1/check') if (req.method !== 'POST' || req.url !== '/v1/check')
return send(res, 404, { error: 'not found' }); return send(res, 404, { error: 'not found' });
if (key && !sameKey(req.headers.authorization ?? '', `Bearer ${key}`))
return send(res, 401, { error: 'this probe needs its key' });
const ip = clientIp(req, trustProxy); const ip = clientIp(req, trustProxy);
void readJson(req) void readJson(req)
.then(async (body) => { .then(async (body) => {
+3
View File
@@ -295,6 +295,9 @@ export const systemApi = {
api<{ ok: true }>('POST', `/api/system/asterisk/restart?now=${now}`), api<{ ok: true }>('POST', `/api/system/asterisk/restart?now=${now}`),
detectPublicIp: () => detectPublicIp: () =>
api<{ ip: string }>('POST', '/api/settings/network/detect-public-ip').then((r) => r.ip), api<{ ip: string }>('POST', '/api/settings/network/detect-public-ip').then((r) => r.ip),
/** A new key for the network test probe, shown this once (08 §7 #9). */
probeKey: () =>
api<{ key: string }>('POST', '/api/settings/network/probe-key').then((r) => r.key),
tls: () => api<TlsCert>('GET', '/api/system/tls'), tls: () => api<TlsCert>('GET', '/api/system/tls'),
diagnostics: () => api<DiagnosticsDto>('GET', '/api/system/diagnostics'), diagnostics: () => api<DiagnosticsDto>('GET', '/api/system/diagnostics'),
networkTest: (mode: 'quick' | 'complete') => networkTest: (mode: 'quick' | 'complete') =>
+54 -1
View File
@@ -505,6 +505,11 @@ function NetworkTab({ s }: { s: SettingsDto }) {
const [watch, setWatch] = useState(s['network.publicIpWatch']); const [watch, setWatch] = useState(s['network.publicIpWatch']);
const [probe, setProbe] = useState(s['network.probeUrl'] ?? ''); const [probe, setProbe] = useState(s['network.probeUrl'] ?? '');
const detect = useMutation({ mutationFn: systemApi.detectPublicIp, onSuccess: setIp }); const detect = useMutation({ mutationFn: systemApi.detectPublicIp, onSuccess: setIp });
const qc = useQueryClient();
const makeKey = useMutation({
mutationFn: systemApi.probeKey,
onSuccess: () => qc.invalidateQueries({ queryKey: ['settings'] }),
});
return ( return (
<form <form
className="flex flex-col gap-4" className="flex flex-col gap-4"
@@ -555,13 +560,61 @@ function NetworkTab({ s }: { s: SettingsDto }) {
/> />
)} )}
</Field> </Field>
{s['network.probeUrl'] && (
<section
aria-labelledby="probe-key-h"
className="flex flex-col gap-2 rounded-lg border border-line p-3 text-sm"
>
<h3 id="probe-key-h" className="font-medium">
Probe key
</h3>
{s['network.hasProbeKey'] ? (
<p className="text-muted">Set: the probe only answers this server.</p>
) : (
<p className="text-amber-800 dark:text-amber-200">
Not set: anyone who finds your probe can use it. Strongly recommended: generate a key
and set it on the probe.
</p>
)}
{makeKey.data ? (
<>
<p>
Set this on the probe (its compose file or service), then restart it. It&rsquo;s
shown only now.
</p>
<div className="flex flex-wrap items-center gap-2">
<code className="min-w-0 flex-1 rounded bg-subtle p-2 font-mono text-xs break-all select-all">
PROBE_KEY={makeKey.data}
</code>
<Button
type="button"
onClick={() => void navigator.clipboard.writeText(makeKey.data)}
>
Copy
</Button>
</div>
</>
) : (
<div>
<Button type="button" onClick={() => makeKey.mutate()} disabled={makeKey.isPending}>
{s['network.hasProbeKey'] ? 'Generate a new key' : 'Generate key'}
</Button>
</div>
)}
<p className="text-xs text-muted">
A new probe address drops the key: generate one for the new probe.
</p>
</section>
)}
<div className="flex items-center gap-3"> <div className="flex items-center gap-3">
<Button type="submit" variant="primary" disabled={save.isPending}> <Button type="submit" variant="primary" disabled={save.isPending}>
Save Save
</Button> </Button>
{save.isSuccess && <span className="text-sm text-muted">Saved.</span>} {save.isSuccess && <span className="text-sm text-muted">Saved.</span>}
</div> </div>
{(save.error ?? detect.error) && <Alert>{(save.error ?? detect.error)!.message}</Alert>} {(save.error ?? detect.error ?? makeKey.error) && (
<Alert>{(save.error ?? detect.error ?? makeKey.error)!.message}</Alert>
)}
</form> </form>
); );
} }
+6 -2
View File
@@ -6,8 +6,9 @@
# docker compose -f deploy/probe/compose.yml up -d → http://<this machine>:8080 # docker compose -f deploy/probe/compose.yml up -d → http://<this machine>:8080
# #
# Host networking, so it sees the real address that asks (Docker's published ports can hide it). # Host networking, so it sees the real address that asks (Docker's published ports can hide it).
# Behind your own HTTPS reverse proxy instead: set PORT to a local port, TRUST_PROXY=true, and have # Behind your own HTTPS reverse proxy on this machine instead: set PORT to a local port,
# the proxy set X-Forwarded-For. The SIP checks connect out from here, so no other ports are needed. # TRUST_PROXY=true (it then listens on 127.0.0.1 only; HOST changes that), and have the proxy set
# X-Forwarded-For. The SIP checks connect out from here, so no other ports are needed.
services: services:
probe: probe:
# Built by each release, like the api and asterisk images (pick your registry and version). # Built by each release, like the api and asterisk images (pick your registry and version).
@@ -17,6 +18,9 @@ services:
environment: environment:
PORT: '8080' PORT: '8080'
TRUST_PROXY: 'false' TRUST_PROXY: 'false'
# Strongly recommended: SelfPhoneNG → Settings → Network → Generate key, paste it here, and
# the probe only answers your server. Empty: anyone who finds the probe can use it.
PROBE_KEY: ''
logging: logging:
driver: json-file driver: json-file
options: { max-size: 10m, max-file: '3' } options: { max-size: 10m, max-file: '3' }
+5 -1
View File
@@ -12,7 +12,11 @@ Wants=network-online.target
[Service] [Service]
ExecStart=/usr/local/bin/selfphoneng-probe ExecStart=/usr/local/bin/selfphoneng-probe
Environment=PORT=8080 Environment=PORT=8080
# Behind your own HTTPS reverse proxy that sets X-Forwarded-For: PORT to a local one, and # Strongly recommended: SelfPhoneNG → Settings → Network → Generate key, then uncomment and paste
# it; the probe then only answers your server.
#Environment=PROBE_KEY=
# Behind your own HTTPS reverse proxy on this machine that sets X-Forwarded-For: PORT to a local
# one, and TRUST_PROXY=true (it then listens on 127.0.0.1 only; HOST changes that).
#Environment=TRUST_PROXY=true #Environment=TRUST_PROXY=true
Restart=always Restart=always
RestartSec=5 RestartSec=5
+4 -2
View File
@@ -72,8 +72,10 @@ on, each recorded call is also recorded one side at a time, and a minute or two
- the two one-side files are deleted. The normal recording stays. - the two one-side files are deleted. The normal recording stays.
On the **Recordings** page each call shows its summary, **Show transcript**, and the search box On the **Recordings** page each call shows its summary, **Show transcript**, and the search box
finds calls by what was said. People who were on a call can read its transcript too, like they can finds calls by what was said. People who made or answered a call can read its transcript too, like
play its recording, and get its summary in their call report email (My settings → Call report). they can play its recording, and get its summary in their call report email (My settings → Call
report). Someone whose phone only rang (a ring group or queue a coworker answered) can't: it's
someone else's conversation.
- Only calls that are recorded are written out (user record modes, phone numbers, queues and - Only calls that are recorded are written out (user record modes, phone numbers, queues and
conference rooms set to record), and only those recorded while it's on. conference rooms set to record), and only those recorded while it's on.
+10 -1
View File
@@ -103,7 +103,8 @@ sudo ufw allow 8080/tcp
(On GitHub, the downloads are at `https://github.com/iamdoubz/selfphoneng/releases/download/$v/…`.) (On GitHub, the downloads are at `https://github.com/iamdoubz/selfphoneng/releases/download/$v/…`.)
To try it first without the service, run `./selfphoneng-probe-linux-x64` and open To try it first without the service, run `./selfphoneng-probe-linux-x64` and open
`http://<machine>:8080/healthz`. `PORT` changes the port; behind your own HTTPS reverse proxy `http://<machine>:8080/healthz`. `PORT` changes the port; behind your own HTTPS reverse proxy
that sets X-Forwarded-For, also set `TRUST_PROXY=true`. on the same machine that sets X-Forwarded-For, also set `TRUST_PROXY=true`. The probe then only
listens on 127.0.0.1 (`HOST` changes it) and only believes X-Forwarded-For from there.
**With Docker:** **With Docker:**
@@ -117,6 +118,14 @@ against someone else, and each address gets a few tests per 10 minutes. Its SIP
OPTIONS request; if you run fail2ban or CrowdSec, add the probe's address to their allow list so a OPTIONS request; if you run fail2ban or CrowdSec, add the probe's address to their allow list so a
test never counts toward a ban. test never counts toward a ban.
**Give the probe a key (strongly recommended).** Without one, anyone who finds your probe can use
it to test their own address, and spend its few tests per 10 minutes. After saving the address,
click **Generate key** under it, copy the `PROBE_KEY=…` line, and add it to the probe: under
`environment:` in `deploy/probe/compose.yml` (`PROBE_KEY: '…'`), or as an `Environment=PROBE_KEY=…`
line in the service file (`sudo systemctl edit selfphoneng-probe`). Restart the probe. From then on
it only answers this server, and System → Diagnostics stops warning about it. The key is shown only
once; a new probe address drops it, so generate one for the new probe.
## Firewall ## Firewall
Allow only the ports in the [port table](README.md#ports). All SelfPhoneNG containers use the Allow only the ports in the [port table](README.md#ports). All SelfPhoneNG containers use the
+6 -2
View File
@@ -286,7 +286,9 @@ As built (M7-T1):
outside); otherwise `src`. An outbound call has no `answered_by` (no extension answered). outside); otherwise `src`. An outbound call has no `answered_by` (no extension answered).
- **recording** (M7-T3): `sp-sub-record` sets `CDR(SP_REC)`; the file (relative, under - **recording** (M7-T3): `sp-sub-record` sets `CDR(SP_REC)`; the file (relative, under
`RECORDINGS_DIR`) becomes a `recordings` row (size, ffprobe length) linked to the call. `RECORDINGS_DIR`) becomes a `recordings` row (size, ffprobe length) linked to the call.
`/api/recordings/:id/audio` serves it to admins and to users whose extension is on the call. `/api/recordings/:id/audio` serves it to admins and to users whose extension made or answered
the call (`call_extensions` role `caller` or `answered`), not to a phone that only rang (08 §7
#3); `/api/me/calls` shows such a call without its `recordingId`.
Retention runs once a day (and at start) in the same day-change check as the nightly regenerate. Retention runs once a day (and at start) in the same day-change check as the nightly regenerate.
- Aggregation runs 5 s after a linkedid's last Cdr event; on every AMI connect, linkedids older - Aggregation runs 5 s after a linkedid's last Cdr event; on every AMI connect, linkedids older
than a minute without a `calls` row are aggregated (instead of the nightly job). than a minute without a `calls` row are aggregated (instead of the nightly job).
@@ -443,6 +445,7 @@ As built (M11-T6): `routes/reports.ts`. `from`/`to` are company-time-zone days,
| GET | `/api/system/warnings` | `{items: [{code, message, link, dismissable?}]}` for the dashboard banner: `restart_required`, `e911_cid_missing` (a trunk exists, no default E911 caller ID), `e911_address_unconfirmed` (that DID has no `e911_address`), `smtp_missing`, `tls_cert` (TLS on and the cert missing or expiring within 14 days, M11-T2), `backup_old` (backups on and today's not made yet though the job had its chance, M11-T3), `directory_check` (12 §4.2, §4.5: the directory unreachable at a sign-in or by `directoryCheck`, or that job refusing to disable most people), `spam_quota` (14 §3, dismissable). "Trunk down" comes from live state in the browser | | GET | `/api/system/warnings` | `{items: [{code, message, link, dismissable?}]}` for the dashboard banner: `restart_required`, `e911_cid_missing` (a trunk exists, no default E911 caller ID), `e911_address_unconfirmed` (that DID has no `e911_address`), `smtp_missing`, `tls_cert` (TLS on and the cert missing or expiring within 14 days, M11-T2), `backup_old` (backups on and today's not made yet though the job had its chance, M11-T3), `directory_check` (12 §4.2, §4.5: the directory unreachable at a sign-in or by `directoryCheck`, or that job refusing to disable most people), `spam_quota` (14 §3, dismissable). "Trunk down" comes from live state in the browser |
| POST | `/api/system/warnings/spam_quota/dismiss` | hides `spam_quota` until IPQualityScore's credits run out again (14 §3) | | POST | `/api/system/warnings/spam_quota/dismiss` | hides `spam_quota` until IPQualityScore's credits run out again (14 §3) |
| POST | `/api/settings/network/detect-public-ip` | | | POST | `/api/settings/network/detect-public-ip` | |
| POST | `/api/settings/network/probe-key` | a new key for the network test probe, `{ key }` shown this once, stored encrypted in `network.probeKeyEnc` (never in GET: `network.hasProbeKey`); 400 without a probe address. Diagnostics sends it as `Authorization: Bearer`; a new `network.probeUrl` drops it (08 §7 #9) |
| GET | `/api/system/status` | `{ db: ok, ami: connected, asteriskVersion, uptime, trunks: [...], disk: {free, total}, restartRequired, lastGeneratorRun }` | | GET | `/api/system/status` | `{ db: ok, ami: connected, asteriskVersion, uptime, trunks: [...], disk: {free, total}, restartRequired, lastGeneratorRun }` |
| GET | `/api/system/tls` | the SIP TLS certificate in `TLS_DIR`: `{ok: true, subject, names, notAfter}` or `{ok: false, error}` (M11-T2) | | GET | `/api/system/tls` | the SIP TLS certificate in `TLS_DIR`: `{ok: true, subject, names, notAfter}` or `{ok: false, error}` (M11-T2) |
| GET | `/api/system/generated-files` | names + contents of generated files, **passwords masked** (debug view) | | GET | `/api/system/generated-files` | names + contents of generated files, **passwords masked** (debug view) |
@@ -533,7 +536,8 @@ Events: `call.started`, `call.answered`, `call.ended`, `voicemail.new`, `sms.rec
`sms.sent`, `emergency.call`, `trunk.registration_changed`, `extension.registration_changed`. `sms.sent`, `emergency.call`, `trunk.registration_changed`, `extension.registration_changed`.
- Payload: `{ "id": "<uuid>", "event": "call.ended", "createdAt": "...", "data": {...} }`. - Payload: `{ "id": "<uuid>", "event": "call.ended", "createdAt": "...", "data": {...} }`.
`data` uses the same shapes as the REST API. `data` uses the same shapes as the REST API, except `voicemail.new` has no `transcript`: a
webhook leaves the server and transcripts don't (10 §1, 08 §7 #6).
- Headers: `Content-Type: application/json`, `X-SelfPhoneNG-Event`, - Headers: `Content-Type: application/json`, `X-SelfPhoneNG-Event`,
`X-SelfPhoneNG-Timestamp` (unix seconds), `X-SelfPhoneNG-Timestamp` (unix seconds),
`X-SelfPhoneNG-Signature: sha256=<hex HMAC-SHA256(secret, timestamp + "." + rawBody)>`. `X-SelfPhoneNG-Signature: sha256=<hex HMAC-SHA256(secret, timestamp + "." + rawBody)>`.
+38 -1
View File
@@ -117,8 +117,12 @@ how to fix it.
works on Gitea's runner, where the job's checkout can't be bind-mounted). works on Gitea's runner, where the job's checkout can't be bind-mounted).
`deploy/probe/selfphoneng-probe.service` runs it under systemd with `DynamicUser` and a `deploy/probe/selfphoneng-probe.service` runs it under systemd with `DynamicUser` and a
read-only system. About 45 MB of memory. read-only system. About 45 MB of memory.
- With `PROBE_KEY` set the probe only answers `Authorization: Bearer <key>` (401 otherwise,
compared in constant time); without it, it answers anyone and logs a warning at start. The key
is made in Settings → Network and sent by the api only, never the browser (08 §7 #9).
- `POST /v1/check {host, tests}`: the probe tests **only the address the request came from** (the - `POST /v1/check {host, tests}`: the probe tests **only the address the request came from** (the
last X-Forwarded-For entry with `TRUST_PROXY=true`), so it can't be aimed at anyone else; 20 last X-Forwarded-For entry with `TRUST_PROXY=true`, believed only from a proxy on 127.0.0.1
or ::1, where the probe then listens by default; 08 §7 #4), so it can't be aimed at anyone else; 20
tests at most, 10 checks per address per 10 minutes, 4 s per test. tests at most, 10 checks per address per 10 minutes, 4 s per test.
- Quick: HTTP 80, HTTPS 443 `/healthz` (status and certificate), SIP OPTIONS over UDP (and TCP - Quick: HTTP 80, HTTPS 443 `/healthz` (status and certificate), SIP OPTIONS over UDP (and TCP
when on) to `sip.udpPort`. Complete adds SIP over TLS (when on), a WebSocket upgrade on `/ws` when on) to `sip.udpPort`. Complete adds SIP over TLS (when on), a WebSocket upgrade on `/ws`
@@ -237,3 +241,36 @@ Fixed after the review (owner, 2026-10-21):
exist; with credentials it counts as a failure (`devices.test.ts`). exist; with credentials it counts as a failure (`devices.test.ts`).
- The login limiter forgets keys past every window, swept at most once a minute - The login limiter forgets keys past every window, swept at most once a minute
(`login-limiter.test.ts`). (`login-limiter.test.ts`).
## 7. Security review 2 (2026-10-28)
Everything since §6 (`8d5475c..`): contacts and Contrack, single sign-on, call reports, local AI,
spam check, the mobile app's pairing, my reports and spam reports. Two passes again: one against
the code by the author, and an independent review agent (read-only, told to prove each finding
end to end). Each fix came with a test that failed before it and passes now. Nothing Critical or
High.
| # | Severity | Finding | Fix | Proof |
|---|---|---|---|---|
| 1 | Medium | A directory sign-in matched by email a user who signs in with OpenID only. Where people can edit their own `mail`, Mallory set hers to the victim's and her own directory password opened the victim's account | the email match also needs no connected account of any kind (12 §4.3 step 2) | `directory-signin.test.ts` |
| 2 | Medium | An OpenID sign-in matched by verified email an admin, or a user who signs in through the directory (no password, no OpenID account yet): a provider account with that email got their session, admin included | not an admin, and no connected account of any kind (12 §3.3 step 2) | `oidc.test.ts`, both key types |
| 3 | Medium, privacy | Everyone whose phone **rang** counted as on the call: a ring group or queue member who didn't answer could play a coworker's recording, read its transcript, and got its summary in their call report email and on their Contrack timeline. The audio part predates §6 | `ON_THE_CALL` (`services/cdr.ts`): only the `caller` and `answered` roles, in `mayHear`, `/api/me/calls` (no `recordingId`), the call report and the timeline | `recordings.test.ts`, `call-reports.test.ts`, `contrack.test.ts` |
| 4 | Low | The probe with `TRUST_PROXY=true` believed any X-Forwarded-For and listened on every address: reached directly, anyone could have it test any address and ports | X-Forwarded-For only from 127.0.0.1 or ::1; `HOST` defaults to 127.0.0.1 with `TRUST_PROXY=true` (§2.6) | `probe.test.ts` |
| 5 | Low | A user could swap their directory account for another one they know the password of (a shared mailbox): the leaver check then followed the wrong account | `POST /api/me/identities/ldap` → 409 `connected_other` when a different one is connected; an admin disconnects first (12 §4.3) | `directory-signin.test.ts` |
| 6 | Low, privacy | The `voicemail.new` webhook carried the transcript, though webhooks leave the server and 10 §1 says transcripts don't | the webhook has no `transcript` (04 §9) | `voicemail.test.ts` |
| 7 | Low | Contrack's saved company token went to any address: Test with another URL, or saving a new URL, sent it there (SMTP, LDAP and OpenID already refused) | the saved token only for the saved address; a new address needs it typed again and clears everyone's own token (11 §4.3) | `contrack.test.ts` |
| 8 | Low | The OpenID connect callback wasn't tied to the browser that asked: a planted `sp_oidc` (a sibling subdomain can set it) connected the victim's provider account to the attacker's user | the callback's own session must be the one in `sp_oidc`, else `expired` (12 §3.3) | `oidc.test.ts`, both key types |
| 9 | Low | Anyone who found a probe could use it (owner's idea after #4): to test their own address, and spend its per-address limits | an optional key: Settings → Network → Generate key, set as the probe's `PROBE_KEY`; the api sends it as a Bearer token, encrypted at rest, dropped when the probe address changes. Without a key the probe stays open (owner, 2026-10-28) and Diagnostics warns, strongly recommending one (§2.6) | `probe.test.ts`, `diagnostics.test.ts` |
The independent agent found 1, 2, 4, 5 and 8, the author 3; both found 6 and 7; 9 was the
owner's idea. The agent had
checked the recording rule and called it sound: it's the same rule as the audio, which §6 also
passed.
Checked and sound (both passes): every new route's guard and ownership check (contacts and
sharing, spam reports, my reports, caller check, mobile devices); a paired phone is never an admin
and can't make pairing codes, and codes are single-use; OpenID PKCE, state, nonce, `safeNext` and
encrypted provider tokens; LDAP filter escaping and no empty binds; the caller-name lookup only
for Asterisk on this machine with its token; phonebook XML and timeline HTML escaped, report
emails plain text; AI and Contrack addresses literal local IPs with redirects refused; the spam
provider key kept out of errors; every new raw SQL parameterized; no secret in any GET.
+1 -1
View File
@@ -47,7 +47,7 @@ is logged and the message goes out without it.
or timing out: the run stops and the call is retried next minute. or timing out: the run stops and the call is retried next minute.
- API: `GET /api/recordings` has `summary`, `hasTranscript`, `aiError`, and `?q=` (ILIKE on the - API: `GET /api/recordings` has `summary`, `hasTranscript`, `aiError`, and `?q=` (ILIKE on the
transcript and summary, `%`/`_` taken literally). `GET /api/recordings/:id/transcript`: admins transcript and summary, `%`/`_` taken literally). `GET /api/recordings/:id/transcript`: admins
and the people on the call, like the audio. Deleting a recording, and retention, delete its side and the people on the call (made or answered it, not only rang), like the audio. Deleting a recording, and retention, delete its side
files too. files too.
## 4. Next (in order) ## 4. Next (in order)
+3
View File
@@ -190,6 +190,9 @@ against the pinned version's `openapi.json`.
checked by reading contacts). checked by reading contacts).
- Admin, same page: the **company Contrack account's token** (read-only is enough; read-write lets - Admin, same page: the **company Contrack account's token** (read-only is enough; read-write lets
"Add to company contacts" work), write-only like the SMTP password (`hasCompanyToken` in GET). "Add to company contacts" work), write-only like the SMTP password (`hasCompanyToken` in GET).
Like it, the saved token only goes to the address it was saved for: Test with another address
sends none, and saving a new address needs the token typed again (400). A new address also
clears everyone's own token: they connect again (08 §7 #7).
The admin makes that account in Contrack (e.g. `company@…`) and fills it like any other. The admin makes that account in Contrack (e.g. `company@…`) and fills it like any other.
- Contrack tokens expire in 90 days unless made with **Never**: the guide and the forms say to pick - Contrack tokens expire in 90 days unless made with **Never**: the guide and the forms say to pick
Never (or a year). Never (or a year).
+14 -8
View File
@@ -199,11 +199,12 @@ Failed callbacks count in the IP login limiter (04 §2.2).
Signing in, in this order: Signing in, in this order:
1. A `user_identities` row (`oidc`, `"<iss> <sub>"`) → its user. 1. A `user_identities` row (`oidc`, `"<iss> <sub>"`) → its user.
2. Else, a user with that email (case-insensitive) who has **no password and no OpenID account 2. Else, a user with that email (case-insensitive) who is **not an admin** and has **no password
yet** (made by an admin in `oidc` mode, or whose password an admin removed), **and the email is and no connected account of any kind yet** (made by an admin in `oidc` mode, or whose password
verified** (`email_verified === true`, or `trustEmail`) → connect it now (step 3's row) and sign an admin removed), **and the email is verified** (`email_verified === true`, or `trustEmail`)
in. Users who have a password are never matched by email: they connect from My account, so a → connect it now (step 3's row) and sign in. Users who have a password, an admin, or someone
provider where people can set their own email can't hand over an account. who signs in through the directory are never matched by email: they connect from My account,
so a provider where people can set their own email can't hand over an account (08 §7 #2).
3. Else `not_connected`. 3. Else `not_connected`.
Connecting (from My account, already signed in, any mode but `password`): Connecting (from My account, already signed in, any mode but `password`):
@@ -211,6 +212,9 @@ Connecting (from My account, already signed in, any mode but `password`):
- After the callback, the session's user gets the `user_identities` row (email and groups as - After the callback, the session's user gets the `user_identities` row (email and groups as
sent). Refused (`taken`) when that `iss sub` belongs to another user; a user who already has one sent). Refused (`taken`) when that `iss sub` belongs to another user; a user who already has one
replaces it (confirm in the UI). Other sessions stay. Audit `sso_connected`. replaces it (confirm in the UI). Other sessions stay. Audit `sso_connected`.
- The callback must arrive in the browser whose session asked: its `sp_session` is the session in
`sp_oidc`, else `expired`. A planted `sp_oidc` (from a sibling subdomain) can't connect the
victim's provider account to someone else's user (08 §7 #8).
- **Disconnect** (`DELETE /api/me/identities/oidc`; admins also on Users, - **Disconnect** (`DELETE /api/me/identities/oidc`; admins also on Users,
`DELETE /api/users/:id/identities/oidc`): ends that user's `oidc` sessions. Refused (409 `DELETE /api/users/:id/identities/oidc`): ends that user's `oidc` sessions. Refused (409
`last_way_in`) when it would leave them no way to sign in in the current mode (no password, or `last_way_in`) when it would leave them no way to sign in in the current mode (no password, or
@@ -376,11 +380,13 @@ A user can have both a SelfPhoneNG password and a directory account: either pass
1. A `user_identities` row (`ldap`, id) → its user. Their email here follows the directory's when 1. A `user_identities` row (`ldap`, id) → its user. Their email here follows the directory's when
it changes (unless another user has it: kept, logged). it changes (unless another user has it: kept, logged).
2. Else, a user with the directory email who is **not an admin** and has **no password or only the 2. Else, a user with the directory email who is **not an admin**, has **no password or only the
temporary one** from Users (`must_change_password`) → connect it (add the row) and sign in, temporary one** from Users (`must_change_password`) and **no connected account of any kind**
(an OpenID user's account is theirs, 08 §7 #1) → connect it (add the row) and sign in,
audit `ldap_connected`. Anyone else connects on My settings → Sign-in → **Connect directory audit `ldap_connected`. Anyone else connects on My settings → Sign-in → **Connect directory
account** (`POST /api/me/identities/ldap`, `{ login, password }`, checked by the directory and account** (`POST /api/me/identities/ldap`, `{ login, password }`, checked by the directory and
counted by the login limiter; 409 `taken` when another user has it). Security audit, counted by the login limiter; 409 `taken` when another user has it, 409 `connected_other` when
they already have a different one: an admin disconnects it first, 08 §7 #5). Security audit,
2026-10-26: a directory where people can edit their own `mail` (common OpenLDAP access rules) 2026-10-26: a directory where people can edit their own `mail` (common OpenLDAP access rules)
must not hand out the admin's account. must not hand out the admin's account.
3. Else → 401 "Wrong email or password" (no account is made, §1), with `login_failed` audited as 3. Else → 401 "Wrong email or password" (no account is made, §1), with `login_failed` audited as
+2
View File
@@ -8,6 +8,8 @@ Tick a box only with proof: a date and what was seen.
- [x] Milestones M0–M11 built and tested (simulated phones and a simulated voip.ms). - [x] Milestones M0–M11 built and tested (simulated phones and a simulated voip.ms).
- [x] Security review (M11-T8), with an independent review (08 §6). - [x] Security review (M11-T8), with an independent review (08 §6).
- [x] Second security review of everything after it, with an independent review; all eight
findings fixed with tests (08 §7, 2026-10-28).
## Real voip.ms account (M3) ## Real voip.ms account (M3)
+13 -1
View File
@@ -42,6 +42,11 @@ export const SettingsSchemas = {
.url({ protocol: /^https?$/ }) .url({ protocol: /^https?$/ })
.nullable() .nullable()
.default(null), .default(null),
/**
* The probe's key (its PROBE_KEY), encrypted: made by POST /api/settings/network/probe-key,
* sent as a Bearer token, dropped when the address changes (08 §7 #9). Never in GET.
*/
'network.probeKeyEnc': z.string().nullable().default(null),
'network.stunServer': z 'network.stunServer': z
.string() .string()
.regex(/^[A-Za-z0-9.-]+(:\d{1,5})?$/) .regex(/^[A-Za-z0-9.-]+(:\d{1,5})?$/)
@@ -327,10 +332,17 @@ export type SettingsDto = {
[ [
K in Exclude< K in Exclude<
SettingKey, SettingKey,
'smtp' | 'sms.webhookToken' | 'contacts.contrack' | 'auth.oidc' | 'auth.ldap' | 'spam' | 'smtp'
| 'sms.webhookToken'
| 'contacts.contrack'
| 'auth.oidc'
| 'auth.ldap'
| 'spam'
| 'network.probeKeyEnc'
> >
]: SettingValue<K>; ]: SettingValue<K>;
} & { } & {
'network.hasProbeKey': boolean;
smtp: (Omit<SmtpInput, 'password'> & { hasPassword: boolean }) | null; smtp: (Omit<SmtpInput, 'password'> & { hasPassword: boolean }) | null;
'contacts.contrack': { url: string; publicUrl: string | null; hasCompanyToken: boolean } | null; 'contacts.contrack': { url: string; publicUrl: string | null; hasCompanyToken: boolean } | null;
spam: SpamSettingsDto; spam: SpamSettingsDto;