From df211f85fea93feae64c59b7ea02540ce260ed64 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 23 Apr 2026 07:11:49 -0400 Subject: [PATCH 1/2] feat(slack): daily channel-privacy audit backstop (#2849) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2735 added a send-time recheck so a channel that drifted private → public stops receiving sensitive posts on the first send after the drift. Channels that sit idle between writes could linger in a drifted state until someone tries to post, so the fix only handled the hot path. Adds a daily audit that proactively checks each of the six admin-settings channels (billing, escalation, admin, prospect, error, editorial) against Slack and emits a structured `channel_privacy_drift_audit` log on drift or unverifiable states. When drift is found, posts a summary to `admin_slack_channel` — unless that channel itself drifted, in which case the summary is suppressed so we don't leak drift details into the now-public channel. Non-destructive: the audit does NOT auto-null settings. The send-time gate (#2735) is the enforcement path; this job is pure observability. Log aggregation alerting should key on `event: 'channel_privacy_drift_audit'`. Registered with the existing jobScheduler at 24h interval / 10min initial delay. Reuses `verifyChannelStillPrivate` + the `sendChannelMessage({ requirePrivate: 'strict-public-only' })` mode from #2861. 9 unit tests cover the orchestration logic: unconfigured channels skipped, admin-channel self-drift suppresses the summary (the core #2849 acceptance), throws collapse to 'unknown', non-destructive behavior pinned against future refactors. Typecheck clean, 1923 server unit tests pass, 631 root unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/channel-privacy-audit-job.md | 8 + .../src/addie/jobs/channel-privacy-audit.ts | 192 ++++++++++++++++++ server/src/addie/jobs/job-definitions.ts | 15 ++ .../tests/unit/channel-privacy-audit.test.ts | 163 +++++++++++++++ 4 files changed, 378 insertions(+) create mode 100644 .changeset/channel-privacy-audit-job.md create mode 100644 server/src/addie/jobs/channel-privacy-audit.ts create mode 100644 server/tests/unit/channel-privacy-audit.test.ts 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..00233cfea9 --- /dev/null +++ b/server/src/addie/jobs/channel-privacy-audit.ts @@ -0,0 +1,192 @@ +/** + * 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) { + logger.warn( + { err, 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 itself is NOT on the drifted list (posting + // sensitive content to a now-public channel is what this whole + // audit is trying to prevent). + 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'); + if (adminSetting && !adminDrifted) { + 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.'); + const result = await sendChannelMessage( + adminSetting.channelId, + { text: lines.join('\n') }, + // 'strict-public-only': we already confirmed admin_slack_channel + // itself is NOT drifted (checked above). If Slack returns + // 'unknown' here, send anyway — the summary is the notification + // and losing it silently would defeat the point of the audit. + { requirePrivate: 'strict-public-only' }, + ); + 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..cc4f10e273 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 } 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: { drifted: unknown[]; unknown: unknown[] }) => + 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..12ca2e714d --- /dev/null +++ b/server/tests/unit/channel-privacy-audit.test.ts @@ -0,0 +1,163 @@ +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 } = 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(), +})); + +vi.mock('../../src/db/system-settings-db.js', () => mockChannels); + +vi.mock('../../src/slack/client.js', () => ({ + verifyChannelStillPrivate: mockVerify, + sendChannelMessage: mockSend, +})); + +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' }); +} + +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 () => { + // billing is drifted; admin is still private so we can notify there. + 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('refuses to post the summary to the admin channel when the admin channel itself drifted (#2849 acceptance)', async () => { + // Don't post sensitive drift info into the drifted channel. + 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(); + }); + + 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 auto-null a drifted setting (non-destructive)', async () => { + // The PR description explicitly leaves auto-null out of scope — + // enforcement is the send-time gate's job. This test pins that + // decision so a future refactor that "helpfully" clears the + // setting flips this red. + mockVerify.mockImplementation(async (channelId: string) => { + if (channelId === 'C_billing') return 'public'; + return 'private'; + }); + + await runChannelPrivacyAudit(); + // No settings-update helpers are imported by the audit module; the + // only mock that writes is `sendChannelMessage` (summary) and even + // that goes to the admin channel, not the billing setting. + const sendTargets = mockSend.mock.calls.map((c: unknown[]) => c[0]); + expect(sendTargets).not.toContain('C_billing'); + }); +}); From cf24c2038a5e55418a51278c09a9dd9212b4cfc3 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 23 Apr 2026 07:24:23 -0400 Subject: [PATCH 2/2] fix(slack): expert-review follow-ups on #2924 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All Should-Fix items from the code / security / testing reviewers: - Suppress the admin summary when admin_slack_channel is 'unknown', not just 'public'. Security reviewer correctly flagged the 'strict-public-only' mode lets 'unknown' through — narrow window where the admin channel could actually be public while Slack info returns unverifiable. The #2849 spec explicitly prioritizes "not using the drifted channel as the notification surface" and structured-log aggregation is the documented backstop. Now the summary is attempted only when admin is confirmed 'private' AND sent with the strict `requirePrivate: true` gate (belt-and-braces against a drift that raced between the audit loop and the send). (security Should-Fix) - Sanitize the `err` payload in the verify-threw log — use `err instanceof Error ? .message : String(err)` so pg / library errors don't spill `.query` / `.parameters` through pino's default err serializer. Mirrors the pattern from #2830. (code Should-Fix) - Tighten `shouldLogResult` type in job-definitions.ts by importing the exported `ChannelPrivacyAuditResult` instead of the inline structural shape. (code Should-Fix) - Spy on the logger in tests and assert the structured `channel_privacy_drift_audit` record fires even when the summary is suppressed — it's the only remaining alert signal in those cases and wasn't being verified. (testing Should-Fix) - Pin the admin-'unknown' + billing-drift branch with an explicit test: confirms the summary is suppressed, both bucket assignments are correct, and the structured log captures the drift. (testing Should-Fix) - New test for multi-drift fan-out (billing + editorial both drifted → single summary mentioning both) to guard against a future refactor that sends per-channel posts. (testing Nice-to-Have) - New test for admin + billing both drifted → summary suppressed, structured log still carries BOTH settings. Distinct from the admin-only case because it proves suppression doesn't accidentally drop billing from the drifted array. (testing Nice-to-Have) - Renamed the non-destructive test for clarity: "does not write to the drifted setting — audit is pure observability". (testing Nice-to-Have) 12 scenarios total (up from 9), 1932 server + 631 root unit tests pass, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/addie/jobs/channel-privacy-audit.ts | 31 +++-- server/src/addie/jobs/job-definitions.ts | 4 +- .../tests/unit/channel-privacy-audit.test.ts | 113 ++++++++++++++++-- 3 files changed, 124 insertions(+), 24 deletions(-) diff --git a/server/src/addie/jobs/channel-privacy-audit.ts b/server/src/addie/jobs/channel-privacy-audit.ts index 00233cfea9..e39331ac03 100644 --- a/server/src/addie/jobs/channel-privacy-audit.ts +++ b/server/src/addie/jobs/channel-privacy-audit.ts @@ -116,8 +116,12 @@ export async function runChannelPrivacyAudit(): Promise 0 || unknown.length > 0) { const adminSetting = configured.find((c) => c.settingName === 'admin_slack_channel'); const adminDrifted = drifted.some((d) => d.settingName === 'admin_slack_channel'); - if (adminSetting && !adminDrifted) { + 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`, ]; @@ -170,14 +181,14 @@ export async function runChannelPrivacyAudit(): Promise + shouldLogResult: (r: ChannelPrivacyAuditResult) => r.drifted.length > 0 || r.unknown.length > 0, }); diff --git a/server/tests/unit/channel-privacy-audit.test.ts b/server/tests/unit/channel-privacy-audit.test.ts index 12ca2e714d..a0123cdb37 100644 --- a/server/tests/unit/channel-privacy-audit.test.ts +++ b/server/tests/unit/channel-privacy-audit.test.ts @@ -11,7 +11,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; * tests in `slack-channel-privacy.test.ts`. */ -const { mockChannels, mockVerify, mockSend } = vi.hoisted(() => ({ +const { mockChannels, mockVerify, mockSend, mockLoggerInfo, mockLoggerWarn } = vi.hoisted(() => ({ mockChannels: { getBillingChannel: vi.fn(), getEscalationChannel: vi.fn(), @@ -22,6 +22,8 @@ const { mockChannels, mockVerify, mockSend } = vi.hoisted(() => ({ }, mockVerify: vi.fn(), mockSend: vi.fn(), + mockLoggerInfo: vi.fn(), + mockLoggerWarn: vi.fn(), })); vi.mock('../../src/db/system-settings-db.js', () => mockChannels); @@ -31,6 +33,19 @@ vi.mock('../../src/slack/client.js', () => ({ 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). */ @@ -45,6 +60,17 @@ function seedAllPrivate() { 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(); @@ -78,7 +104,6 @@ describe('runChannelPrivacyAudit', () => { }); it('posts a summary to the admin channel when a non-admin channel drifts', async () => { - // billing is drifted; admin is still private so we can notify there. mockVerify.mockImplementation(async (channelId: string) => { if (channelId === 'C_billing') return 'public'; return 'private'; @@ -94,8 +119,26 @@ describe('runChannelPrivacyAudit', () => { expect(message.text).toContain('billing_slack_channel'); }); - it('refuses to post the summary to the admin channel when the admin channel itself drifted (#2849 acceptance)', async () => { - // Don't post sensitive drift info into the drifted 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'; @@ -105,6 +148,53 @@ describe('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 () => { @@ -143,20 +233,19 @@ describe('runChannelPrivacyAudit', () => { expect(result.unknown.map((u) => u.settingName)).toContain('escalation_slack_channel'); }); - it('does not auto-null a drifted setting (non-destructive)', async () => { - // The PR description explicitly leaves auto-null out of scope — - // enforcement is the send-time gate's job. This test pins that - // decision so a future refactor that "helpfully" clears the - // setting flips this red. + 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(); - // No settings-update helpers are imported by the audit module; the - // only mock that writes is `sendChannelMessage` (summary) and even - // that goes to the admin channel, not the billing setting. + // 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'); });