feat(23-01): wire acknowledge_user manual-path real note post into remediateApprovedActions

- remediateApprovedActions now captures the transaction's RemediateResult,
  then post-commit checks whether an approved acknowledge_user row was
  transitioned this pass (alreadyCompleted === false) and, if so, calls
  generateAndPostAcknowledgment(campaignId) exactly once
- Call happens outside the DB transaction (network I/O hazard) and is
  wrapped in its own try/catch that logs and swallows failures -- the DB
  transition has already committed
- Every other action type (block_sender, purge_message, warn_user,
  reset_password, isolate_endpoint, disable_forwarding_rule, quarantine)
  remains a simulated status-only transition, unchanged
- Updated top-of-file D-01 doc comment to record the narrow D-04 carve-out
- Tests: acknowledge_user IS posted once when remediated, NOT called for
  block_sender/warn_user-only remediation, NOT called on idempotent re-run
  of an already-completed acknowledge_user row, and a post rejection does
  not propagate out of remediateApprovedActions
This commit is contained in:
lorentz 2026-07-16 19:39:13 -04:00
parent 50e241592c
commit 6224e44219
2 changed files with 83 additions and 2 deletions

View file

@ -15,6 +15,11 @@ vi.mock('./phishing-audit', () => ({
writeAuditEvent: (...args: unknown[]) => writeAuditEventMock(...args),
}));
const generateAndPostAcknowledgmentMock = vi.fn();
vi.mock('./triage-note-service', () => ({
generateAndPostAcknowledgment: (...args: unknown[]) => generateAndPostAcknowledgmentMock(...args),
}));
// eslint-disable-next-line import/first -- imported after vi.mock hoisting
import {
approveRemediationActions,
@ -82,6 +87,8 @@ beforeEach(() => {
transactionMock.mockReset();
writeAuditEventMock.mockReset();
writeAuditEventMock.mockResolvedValue('audit-id');
generateAndPostAcknowledgmentMock.mockReset();
generateAndPostAcknowledgmentMock.mockResolvedValue({ noteText: 'thanks', tickets: [] });
clientCalls = [];
});
@ -208,6 +215,51 @@ describe('remediateApprovedActions', () => {
);
expect(writeAuditEventMock).not.toHaveBeenCalled();
});
it('calls generateAndPostAcknowledgment exactly once with the campaignId when an approved acknowledge_user row is remediated', async () => {
stage({
remediationRows: [{ id: 'action-1', action_type: 'acknowledge_user', status: 'approved' }],
});
await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(generateAndPostAcknowledgmentMock).toHaveBeenCalledTimes(1);
expect(generateAndPostAcknowledgmentMock).toHaveBeenCalledWith('campaign-1');
});
it('does NOT call generateAndPostAcknowledgment for a block_sender/warn_user-only remediation', async () => {
stage({
remediationRows: [
{ id: 'action-1', action_type: 'block_sender', status: 'approved' },
{ id: 'action-2', action_type: 'warn_user', status: 'approved' },
],
});
await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(generateAndPostAcknowledgmentMock).not.toHaveBeenCalled();
});
it('does NOT call generateAndPostAcknowledgment when the acknowledge_user row staged is already completed (idempotent re-run)', async () => {
stage({
remediationRows: [{ id: 'action-1', action_type: 'acknowledge_user', status: 'completed' }],
});
await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(generateAndPostAcknowledgmentMock).not.toHaveBeenCalled();
});
it('does not propagate a generateAndPostAcknowledgment rejection out of remediateApprovedActions (already-committed transition)', async () => {
stage({
remediationRows: [{ id: 'action-1', action_type: 'acknowledge_user', status: 'approved' }],
});
generateAndPostAcknowledgmentMock.mockRejectedValueOnce(new Error('Autotask unavailable'));
const result = await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(result.actions[0]).toMatchObject({ actionType: 'acknowledge_user', status: 'completed' });
});
});
// =============================================================================

View file

@ -13,13 +13,22 @@
* D-01: this file never has an unimplemented/no-op code path for the
* remediate step. Its "external effect" is a simulated internal transition
* (status='approved' -> 'completed') no real provider call for any of the
* 7 action types this milestone. The only explicit-failure branch is the
* 7 original action types. The only explicit-failure branch is the
* zero-approved-actions case (REMED-03) remediating with nothing approved
* always throws, never silently no-ops as a success.
*
* Phase 23 carve-out (D-04): the ONE exception is the post-commit
* `acknowledge_user` customer-note post below a non-destructive thank-you
* message, not a security action. It runs AFTER the transaction commits
* (never inside it the Autotask write is network I/O and must not hold a
* DB transaction open or risk a post-then-rollback), and its failure is
* caught/logged, never propagated (the DB transition already succeeded).
* Every other action type remains a simulated status-only transition.
*/
import { postgresClient } from './postgres-client';
import { writeAuditEvent } from './phishing-audit';
import { generateAndPostAcknowledgment } from './triage-note-service';
export class RemediationValidationError extends Error {
constructor(message: string) {
@ -171,7 +180,7 @@ export interface RemediateResult {
* success for "nothing to do".
*/
export async function remediateApprovedActions(campaignId: string, actor: string | null): Promise<RemediateResult> {
return postgresClient.transaction(async (client) => {
const result = await postgresClient.transaction(async (client) => {
const rowsRes = await client.query<RemediationActionRow>(
`SELECT id::text, action_type, status FROM remediation_actions WHERE campaign_id = $1 FOR UPDATE`,
[campaignId]
@ -204,6 +213,26 @@ export async function remediateApprovedActions(campaignId: string, actor: string
return { campaignId, actions };
});
// Phase 23 D-04 carve-out: post the real customer-visible acknowledgment
// note AFTER the transaction has committed — only when an approved
// acknowledge_user row was actually transitioned this pass (never on the
// already-completed idempotent re-run). Never held inside the DB
// transaction (network I/O), and never allowed to propagate (the DB
// transition already succeeded; generateAndPostAcknowledgment already
// isolates per-ticket failures internally).
const shouldPostAcknowledgment = result.actions.some(
(action) => action.actionType === 'acknowledge_user' && action.alreadyCompleted === false
);
if (shouldPostAcknowledgment) {
try {
await generateAndPostAcknowledgment(campaignId);
} catch (err) {
console.error('[REMEDIATE] acknowledge_user note post failed', campaignId, err);
}
}
return result;
}
// =============================================================================