diff --git a/.changeset/channel-privacy-audit-job.md b/.changeset/channel-privacy-audit-job.md new file mode 100644 index 0000000000..25a9dd4ab6 --- /dev/null +++ b/.changeset/channel-privacy-audit-job.md @@ -0,0 +1,8 @@ +--- +--- + +Close #2849: daily audit backstop for admin-settings channel privacy. #2735 catches drift at send time — a channel that has flipped private → public stops receiving sensitive posts on the first send after the drift. Channels that sit idle between writes could go undetected. This job runs once a day, checks each configured channel (billing / escalation / admin / prospect / error / editorial) against Slack, and emits a structured `channel_privacy_drift_audit` warn/info log on any drift or unverifiable state. + +When drift is found, a summary is posted to the `admin_slack_channel` — unless that channel itself is the drifted one, in which case the summary is suppressed and the structured log is the only signal (log aggregation alerting should key on the event). Non-destructive by design: the audit does NOT auto-null the drifted setting. The send-time gate from #2735 already refuses to post sensitive content; this job is pure observability. + +Registered with the existing `jobScheduler`, 24-hour interval, 10-minute initial delay. Uses the `runChannelPrivacyAudit()` export as its runner. 9 unit-test scenarios cover the orchestration logic (unconfigured channels skipped, admin-channel self-drift suppresses the summary, throws collapse to 'unknown', non-destructive behavior pinned). diff --git a/server/src/addie/jobs/channel-privacy-audit.ts b/server/src/addie/jobs/channel-privacy-audit.ts new file mode 100644 index 0000000000..e39331ac03 --- /dev/null +++ b/server/src/addie/jobs/channel-privacy-audit.ts @@ -0,0 +1,203 @@ +/** + * Daily audit of the six admin-settings notification channels. + * + * #2735 added a send-time recheck (`sendChannelMessage` gates on + * `verifyChannelStillPrivate`), so a channel that flipped private → + * public stops receiving sensitive posts on the first send after the + * drift. That's the hot path. The gap: channels that sit idle for + * days aren't written to, so the drift sits undetected. + * + * This job runs once per day, checks each configured channel's + * current privacy state against Slack, and emits structured warnings + * on any confirmed drift. It also posts a summary to the + * `admin_slack_channel` — unless that's the drifted one, in which + * case the structured log is the only signal and log aggregation + * alerting should pick it up. (Filed #2849 acceptance: "notifies a + * human without using the drifted channel as the notification + * surface.") + * + * This is non-destructive: we do NOT auto-null the setting. The + * hot-path recheck already refuses to post sensitive content to a + * drifted channel; enforcement is the send-time gate's job. This + * audit is pure observability. + */ + +import { createLogger } from '../../logger.js'; +import { + getBillingChannel, + getEscalationChannel, + getAdminChannel, + getProspectChannel, + getErrorChannel, + getEditorialChannel, +} from '../../db/system-settings-db.js'; +import { + verifyChannelStillPrivate, + sendChannelMessage, + type ChannelPrivacyState, +} from '../../slack/client.js'; + +const logger = createLogger('channel-privacy-audit'); + +/** + * One configured admin channel: the setting name (for log context) + * and the channel id we'd post sensitive content to. + */ +interface AdminChannelConfig { + settingName: + | 'billing_slack_channel' + | 'escalation_slack_channel' + | 'admin_slack_channel' + | 'prospect_slack_channel' + | 'error_slack_channel' + | 'editorial_slack_channel'; + channelId: string; + channelName: string | null; +} + +export interface ChannelPrivacyAuditResult { + /** Number of configured channels we inspected. */ + checked: number; + /** Channels that came back `'public'`. */ + drifted: Array<{ settingName: AdminChannelConfig['settingName']; channelId: string; channelName: string | null }>; + /** Channels whose state could not be verified (transient Slack API failure, permission issues, etc.). */ + unknown: Array<{ settingName: AdminChannelConfig['settingName']; channelId: string }>; + /** Whether we managed to post a summary to the admin channel (false when skipped for drift or when not configured). */ + summaryPosted: boolean; +} + +async function gatherConfiguredChannels(): Promise { + const [billing, escalation, admin, prospect, error, editorial] = await Promise.all([ + getBillingChannel(), + getEscalationChannel(), + getAdminChannel(), + getProspectChannel(), + getErrorChannel(), + getEditorialChannel(), + ]); + + const configured: AdminChannelConfig[] = []; + if (billing.channel_id) { + configured.push({ settingName: 'billing_slack_channel', channelId: billing.channel_id, channelName: billing.channel_name }); + } + if (escalation.channel_id) { + configured.push({ settingName: 'escalation_slack_channel', channelId: escalation.channel_id, channelName: escalation.channel_name }); + } + if (admin.channel_id) { + configured.push({ settingName: 'admin_slack_channel', channelId: admin.channel_id, channelName: admin.channel_name }); + } + if (prospect.channel_id) { + configured.push({ settingName: 'prospect_slack_channel', channelId: prospect.channel_id, channelName: prospect.channel_name }); + } + if (error.channel_id) { + configured.push({ settingName: 'error_slack_channel', channelId: error.channel_id, channelName: error.channel_name }); + } + if (editorial.channel_id) { + configured.push({ settingName: 'editorial_slack_channel', channelId: editorial.channel_id, channelName: editorial.channel_name }); + } + return configured; +} + +/** + * Run one audit pass across all six configured admin channels. + * Returns a structured result so callers (tests, observability) can + * assert on outcomes without parsing log output. + */ +export async function runChannelPrivacyAudit(): Promise { + const configured = await gatherConfiguredChannels(); + + const drifted: ChannelPrivacyAuditResult['drifted'] = []; + const unknown: ChannelPrivacyAuditResult['unknown'] = []; + + // Check each in series — we want any rate-limit backoffs from the + // Slack client to apply cleanly, and the volume is tiny (≤6). + for (const cfg of configured) { + let state: ChannelPrivacyState; + try { + state = await verifyChannelStillPrivate(cfg.channelId); + } catch (err) { + // Narrow the error shape (pg/net errors can carry diagnostic + // fields pino's default err-serializer would emit — mirror the + // sanitization pattern from #2830's brand_json_drift log). + const errMessage = err instanceof Error ? err.message : String(err); + logger.warn( + { error: errMessage, settingName: cfg.settingName, channelId: cfg.channelId }, + 'Channel privacy audit: verify threw', + ); + state = 'unknown'; + } + if (state === 'public') { + drifted.push({ settingName: cfg.settingName, channelId: cfg.channelId, channelName: cfg.channelName }); + } else if (state === 'unknown') { + unknown.push({ settingName: cfg.settingName, channelId: cfg.channelId }); + } + } + + // Structured audit record. This is the source-of-truth alert — log + // aggregation rules should key on `event: 'channel_privacy_drift_audit'` + // so the drift surfaces even when the Slack summary can't be sent. + logger.info( + { + event: 'channel_privacy_drift_audit', + checked: configured.length, + driftedCount: drifted.length, + unknownCount: unknown.length, + driftedSettings: drifted.map((d) => d.settingName), + unknownSettings: unknown.map((u) => u.settingName), + }, + `Channel privacy audit: ${configured.length} checked, ${drifted.length} drifted, ${unknown.length} unverifiable`, + ); + + // Post a summary to the admin channel — but only when: + // (a) it's configured, AND + // (b) the admin channel's OWN privacy state is confirmed `'private'` + // in this audit pass. The #2849 acceptance calls out "notifies + // a human without using the drifted channel as the notification + // surface" — we extend that to `'unknown'` too: if the admin + // channel's state couldn't be verified, there's a narrow window + // where it's actually public and we'd leak drift details about + // the other channels into a workspace-visible thread. Log + // aggregation alerting on `event: 'channel_privacy_drift_audit'` + // is the documented backstop for this case. + let summaryPosted = false; + if (drifted.length > 0 || unknown.length > 0) { + const adminSetting = configured.find((c) => c.settingName === 'admin_slack_channel'); + const adminDrifted = drifted.some((d) => d.settingName === 'admin_slack_channel'); + const adminUnknown = unknown.some((u) => u.settingName === 'admin_slack_channel'); + if (adminSetting && !adminDrifted && !adminUnknown) { + const lines: string[] = [ + `:mag: *Channel privacy audit* — ${configured.length} channels checked`, + ]; + if (drifted.length > 0) { + lines.push('', `*Drifted to public* (posts blocked by send-time gate — admin action needed):`); + for (const d of drifted) { + lines.push(`• \`${d.settingName}\` → <#${d.channelId}|${d.channelName ?? d.channelId}>`); + } + } + if (unknown.length > 0) { + lines.push('', `*Unverifiable* (transient Slack error — may resolve on next run):`); + for (const u of unknown) { + lines.push(`• \`${u.settingName}\``); + } + } + lines.push('', 'Re-privatize the listed channels or update the settings at /admin/settings.'); + // We already confirmed the admin channel is `'private'` in this + // audit pass. `requirePrivate: true` (strict) belts-and-braces + // the send-time gate against a drift that raced between the + // audit loop and this send. + const result = await sendChannelMessage( + adminSetting.channelId, + { text: lines.join('\n') }, + { requirePrivate: true }, + ); + summaryPosted = result.ok; + } + } + + return { + checked: configured.length, + drifted, + unknown, + summaryPosted, + }; +} diff --git a/server/src/addie/jobs/job-definitions.ts b/server/src/addie/jobs/job-definitions.ts index d17fd3a20a..c3c82443b2 100644 --- a/server/src/addie/jobs/job-definitions.ts +++ b/server/src/addie/jobs/job-definitions.ts @@ -46,6 +46,7 @@ import { runEventRecapNudgeJob } from './event-recap-nudge.js'; import { runMeetingPrepNudgeJob } from './meeting-prep-nudge.js'; import { runProfileCompletionNudgeJob } from './profile-completion-nudge.js'; import { runSpecInsightPostJob } from './spec-insight-post.js'; +import { runChannelPrivacyAudit, type ChannelPrivacyAuditResult } from './channel-privacy-audit.js'; import { NotificationDatabase } from '../../db/notification-db.js'; import { notifyUser } from '../../notifications/notification-service.js'; import { logger } from '../../logger.js'; @@ -145,6 +146,20 @@ export function registerAllJobs(): void { shouldLogResult: (r) => r.summariesGenerated > 0, }); + // Channel privacy audit (#2849) — daily backstop for the send-time + // recheck in #2735. Catches drift on admin-settings channels that + // sit idle between writes so the drift doesn't linger until + // someone tries to post. + jobScheduler.register({ + name: 'channel-privacy-audit', + description: 'Channel privacy audit', + interval: { value: 24, unit: 'hours' }, + initialDelay: { value: 10, unit: 'minutes' }, + runner: runChannelPrivacyAudit, + shouldLogResult: (r: ChannelPrivacyAuditResult) => + r.drifted.length > 0 || r.unknown.length > 0, + }); + // Relationship orchestrator - continues member relationships across channels jobScheduler.register({ name: 'relationship-orchestrator', diff --git a/server/tests/unit/channel-privacy-audit.test.ts b/server/tests/unit/channel-privacy-audit.test.ts new file mode 100644 index 0000000000..a0123cdb37 --- /dev/null +++ b/server/tests/unit/channel-privacy-audit.test.ts @@ -0,0 +1,252 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +/** + * #2849 — daily audit job for admin-channel privacy drift. + * + * Mocks at the module seam (`system-settings-db` + `slack/client`) + * rather than driving real Slack calls — the audit's logic lives in + * the orchestration (which channels get checked, what happens to + * the summary when admin itself drifts), not in Slack plumbing. The + * send-time recheck helpers already have their own integration-ish + * tests in `slack-channel-privacy.test.ts`. + */ + +const { mockChannels, mockVerify, mockSend, mockLoggerInfo, mockLoggerWarn } = vi.hoisted(() => ({ + mockChannels: { + getBillingChannel: vi.fn(), + getEscalationChannel: vi.fn(), + getAdminChannel: vi.fn(), + getProspectChannel: vi.fn(), + getErrorChannel: vi.fn(), + getEditorialChannel: vi.fn(), + }, + mockVerify: vi.fn(), + mockSend: vi.fn(), + mockLoggerInfo: vi.fn(), + mockLoggerWarn: vi.fn(), +})); + +vi.mock('../../src/db/system-settings-db.js', () => mockChannels); + +vi.mock('../../src/slack/client.js', () => ({ + verifyChannelStillPrivate: mockVerify, + sendChannelMessage: mockSend, +})); + +// Spy on the logger so we can assert the structured audit record +// fires even when the summary send is suppressed — the log is the +// primary alert signal per the #2849 acceptance. +vi.mock('../../src/logger.js', () => ({ + createLogger: () => ({ + info: mockLoggerInfo, + warn: mockLoggerWarn, + error: vi.fn(), + debug: vi.fn(), + }), + logger: { child: () => ({ info: mockLoggerInfo, warn: mockLoggerWarn, error: vi.fn(), debug: vi.fn() }) }, +})); + +import { runChannelPrivacyAudit } from '../../src/addie/jobs/channel-privacy-audit.js'; + +/** Default: every channel is configured and private (clean run). */ +function seedAllPrivate() { + mockChannels.getBillingChannel.mockResolvedValue({ channel_id: 'C_billing', channel_name: 'billing' }); + mockChannels.getEscalationChannel.mockResolvedValue({ channel_id: 'C_escalation', channel_name: 'escalation' }); + mockChannels.getAdminChannel.mockResolvedValue({ channel_id: 'C_admin', channel_name: 'admin' }); + mockChannels.getProspectChannel.mockResolvedValue({ channel_id: 'C_prospect', channel_name: 'prospect' }); + mockChannels.getErrorChannel.mockResolvedValue({ channel_id: 'C_error', channel_name: 'error' }); + mockChannels.getEditorialChannel.mockResolvedValue({ channel_id: 'C_editorial', channel_name: 'editorial' }); + mockVerify.mockResolvedValue('private'); + mockSend.mockResolvedValue({ ok: true, ts: '1.1' }); +} + +/** Pull the `driftedSettings` / `unknownSettings` off the audit's structured log call. */ +function auditLogPayload(): Record | undefined { + const call = mockLoggerInfo.mock.calls.find( + (args: unknown[]) => + typeof args[0] === 'object' && + args[0] !== null && + (args[0] as Record).event === 'channel_privacy_drift_audit', + ); + return call ? (call[0] as Record) : undefined; +} + +beforeEach(() => { + vi.clearAllMocks(); + seedAllPrivate(); +}); + +describe('runChannelPrivacyAudit', () => { + it('checks every configured channel', async () => { + const result = await runChannelPrivacyAudit(); + expect(result.checked).toBe(6); + expect(result.drifted).toEqual([]); + expect(result.unknown).toEqual([]); + expect(mockVerify).toHaveBeenCalledTimes(6); + }); + + it('skips unconfigured channels (empty channel_id)', async () => { + mockChannels.getBillingChannel.mockResolvedValue({ channel_id: null, channel_name: null }); + mockChannels.getProspectChannel.mockResolvedValue({ channel_id: '', channel_name: null }); + + const result = await runChannelPrivacyAudit(); + expect(result.checked).toBe(4); + expect(mockVerify).toHaveBeenCalledTimes(4); + const verifiedIds = mockVerify.mock.calls.map((c: unknown[]) => c[0]); + expect(verifiedIds).not.toContain('C_billing'); + expect(verifiedIds).not.toContain('C_prospect'); + }); + + it('does not post a summary when everything is private', async () => { + const result = await runChannelPrivacyAudit(); + expect(result.summaryPosted).toBe(false); + expect(mockSend).not.toHaveBeenCalled(); + }); + + it('posts a summary to the admin channel when a non-admin channel drifts', async () => { + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_billing') return 'public'; + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.drifted).toHaveLength(1); + expect(result.drifted[0].settingName).toBe('billing_slack_channel'); + expect(result.summaryPosted).toBe(true); + expect(mockSend).toHaveBeenCalledTimes(1); + const [channel, message] = mockSend.mock.calls[0]; + expect(channel).toBe('C_admin'); + expect(message.text).toContain('billing_slack_channel'); + }); + + it('groups every drifted setting into a single summary call (#2849 fan-out)', async () => { + // billing + editorial both drifted → one summary mentioning both; + // guards against a future refactor that sends per-channel posts. + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_billing' || channelId === 'C_editorial') return 'public'; + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.drifted.map((d) => d.settingName).sort()).toEqual([ + 'billing_slack_channel', + 'editorial_slack_channel', + ]); + expect(mockSend).toHaveBeenCalledTimes(1); + const text = mockSend.mock.calls[0][1].text; + expect(text).toContain('billing_slack_channel'); + expect(text).toContain('editorial_slack_channel'); + }); + + it('suppresses the summary when admin_slack_channel is drifted AND logs the drift in the structured audit record (#2849 acceptance)', async () => { + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_admin') return 'public'; + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.drifted.map((d) => d.settingName)).toContain('admin_slack_channel'); + expect(result.summaryPosted).toBe(false); + expect(mockSend).not.toHaveBeenCalled(); + + // The structured log is the only signal in this case — assert it + // fired with the drift information intact so log aggregation + // alerting can pick it up. + const payload = auditLogPayload(); + expect(payload).toBeDefined(); + expect(payload!.driftedSettings).toContain('admin_slack_channel'); + }); + + it('suppresses the summary when admin is in the unknown bucket (narrow leak window)', async () => { + // If we can't prove admin is private and another channel is + // confirmed public, posting the drift summary into admin risks + // leaking details to a channel whose privacy flipped between the + // last audit and this one (with Slack's info endpoint failing + // exactly during this run). Conservative: suppress, rely on log. + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_admin') return 'unknown'; + if (channelId === 'C_billing') return 'public'; + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.drifted.map((d) => d.settingName)).toEqual(['billing_slack_channel']); + expect(result.unknown.map((u) => u.settingName)).toEqual(['admin_slack_channel']); + expect(result.summaryPosted).toBe(false); + expect(mockSend).not.toHaveBeenCalled(); + }); + + it('suppresses the summary when both admin AND another channel are drifted (admin drop does not accidentally drop the billing record)', async () => { + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_admin' || channelId === 'C_billing') return 'public'; + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.drifted.map((d) => d.settingName).sort()).toEqual([ + 'admin_slack_channel', + 'billing_slack_channel', + ]); + expect(result.summaryPosted).toBe(false); + expect(mockSend).not.toHaveBeenCalled(); + + // Both settings must be in the structured log, even though the + // summary was suppressed. + const payload = auditLogPayload(); + expect(payload!.driftedSettings).toContain('admin_slack_channel'); + expect(payload!.driftedSettings).toContain('billing_slack_channel'); + }); + + it('records unknown states separately from drift', async () => { + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_error') return 'unknown'; + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.drifted).toEqual([]); + expect(result.unknown).toHaveLength(1); + expect(result.unknown[0].settingName).toBe('error_slack_channel'); + }); + + it('posts a summary mentioning unknown channels even when nothing is drifted', async () => { + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_error') return 'unknown'; + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.summaryPosted).toBe(true); + const text = mockSend.mock.calls[0][1].text; + expect(text).toContain('error_slack_channel'); + expect(text).toMatch(/Unverifiable|unknown/i); + }); + + it('treats a throw from verifyChannelStillPrivate as unknown, not drift', async () => { + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_escalation') throw new Error('network flake'); + return 'private'; + }); + + const result = await runChannelPrivacyAudit(); + expect(result.drifted).toEqual([]); + expect(result.unknown.map((u) => u.settingName)).toContain('escalation_slack_channel'); + }); + + it('does not write to the drifted setting — audit is pure observability (non-destructive invariant)', async () => { + // The real guarantee here is structural: the audit module imports + // no setting-mutation helpers. If a future refactor "helpfully" + // auto-nulls a drifted setting, the new import will require the + // test to expand — catching the drift at review time. + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_billing') return 'public'; + return 'private'; + }); + + await runChannelPrivacyAudit(); + // sendChannelMessage is the only mocked write; confirm it's not + // called against the drifted channel (only the admin channel). + const sendTargets = mockSend.mock.calls.map((c: unknown[]) => c[0]); + expect(sendTargets).not.toContain('C_billing'); + }); +});