docs(20): capture phase context
This commit is contained in:
parent
d0285645e9
commit
03a641efa0
2 changed files with 300 additions and 0 deletions
|
|
@ -0,0 +1,221 @@
|
|||
# Phase 20: Remediation, Approval & Audit Safety - Context
|
||||
|
||||
**Gathered:** 2026-07-16
|
||||
**Status:** Ready for planning
|
||||
|
||||
<domain>
|
||||
## Phase Boundary
|
||||
|
||||
Turns a campaign's classification (Phase 19 output) into governed action:
|
||||
recommended remediation actions are persisted as `proposed`; an operator with
|
||||
elevated permission can approve specific action(s) with exact params
|
||||
(recording approver/timestamp); `POST /remediate` proceeds only for approved
|
||||
actions and is idempotent on re-run; an operator can mark a campaign false
|
||||
positive (blocked once remediation is already approved/completed); every
|
||||
state-changing call (classify/approve/remediate/mark-false-positive) writes
|
||||
an `audit_events` row.
|
||||
|
||||
Does NOT cover actually wiring a real external remediation provider (Mimecast
|
||||
block, MS Graph forwarding-rule toggle, RMM isolate, password reset) — those
|
||||
are `REMEDEXEC-01..05`, explicitly deferred to v2. Does NOT cover the
|
||||
Autotask triage note (Phase 21) or the LiveLink approval UI (Phase 22) — this
|
||||
phase only builds the APIs those consume.
|
||||
|
||||
</domain>
|
||||
|
||||
<decisions>
|
||||
## Implementation Decisions
|
||||
|
||||
### Remediate Execution Model
|
||||
- **D-01:** Since no external provider (Mimecast block/purge, MS Graph
|
||||
forwarding-rule toggle, RMM isolate, password reset) is wired in this
|
||||
milestone — confirmed by direct inspection: `mimecast-client.ts` has no
|
||||
block/purge method, `msgraph-client.ts` has no forwarding-rule method, no
|
||||
RMM isolate/quarantine method exists anywhere in `lib/services/` —
|
||||
`POST /remediate` uses a **simulated internal effect** for ALL 7 action
|
||||
types uniformly (no provider distinction yet): it transitions
|
||||
`remediation_actions.status` to `completed`, writes the `audit_events` row,
|
||||
and returns success. No real external API call is made for any action
|
||||
type in this phase. This is the "effect" that REMED-04's idempotency test
|
||||
asserts does not duplicate (calling remediate twice on the same approved
|
||||
action must produce exactly one `completed` transition / one audit entry,
|
||||
not `not_implemented` twice). REMEDEXEC-01..05 (v2) is where a real
|
||||
provider call would replace this simulated transition per action type.
|
||||
|
||||
### Permission Grants
|
||||
- **D-02:** `approve` and `remediate` (declared but ungranted in Phase 18
|
||||
D-05) are granted to **both `superAdminRole` and `adminRole`** in
|
||||
`lib/permissions.ts` — parity with how `analyze` is already granted to
|
||||
both. `userRole` still gets only `phishing: ["read"]`. Confirmed via direct
|
||||
read of `lib/permissions.ts`: both roles currently have
|
||||
`phishing: ["read", "analyze"]` — Phase 20 adds `"approve", "remediate"` to
|
||||
both role definitions (the `statement` already has the full vocabulary from
|
||||
Phase 18, so this is a role-grant change only, not a new statement key).
|
||||
`mark-false-positive` uses the same elevated tier (`approve`-equivalent) —
|
||||
see D-04 for why it can't be looser than `approve`.
|
||||
|
||||
### Approval Granularity
|
||||
- **D-03:** `POST /approve` request body specifies WHICH action(s) from the
|
||||
classification's `recommended_actions` to approve, and MAY override/narrow
|
||||
their params (e.g. `block_sender` scoped to one specific address rather
|
||||
than the classifier's broader suggestion). This is NOT a bare
|
||||
"approve everything the classifier suggested" call — REMED-02's "recording
|
||||
... the exact approved action parameters" implies operator input, not an
|
||||
echo of the classifier's proposal. Each approved action becomes its own
|
||||
`remediation_actions` row (`status: 'approved'`, `approved_by`,
|
||||
`approved_at`, `params` = the operator-supplied/overridden params) —
|
||||
unapproved recommended actions from the classification are simply never
|
||||
materialized as rows.
|
||||
|
||||
### Mark-False-Positive State Guard
|
||||
- **D-04:** `POST /mark-false-positive` is **blocked** (explicit
|
||||
conflict/error response, not silent success) if the campaign already has
|
||||
any `remediation_actions` row with `status IN ('approved', 'completed')`.
|
||||
Rationale: prevents a contradictory audit trail where a campaign is both
|
||||
"remediated" and "false positive." If no approved/completed remediation
|
||||
exists yet, mark-false-positive proceeds freely (this doesn't block
|
||||
re-classification or a later un-false-positive correction — that's not in
|
||||
scope here; planner's call whether false-positive is reversible).
|
||||
Requires the same elevated permission tier as `approve` (D-02) — marking
|
||||
false positive is a state-changing security decision, not a read action.
|
||||
|
||||
### Claude's Discretion (explicitly deferred to research + planner)
|
||||
- Exact idempotency detection mechanism for `POST /remediate` re-runs —
|
||||
whether it's a status check (`status = 'completed'` short-circuits with
|
||||
the same response) or a dedupe key/constraint — as long as calling
|
||||
remediate twice on the same approved action produces exactly one
|
||||
`completed` transition and one audit entry (D-01's contract).
|
||||
- Exact `audit_events.actor` value format — likely session email, matching
|
||||
the `disabled_by` convention already used for admin actions elsewhere in
|
||||
the codebase (per CLAUDE.md's integration-disable section) — planner's
|
||||
call, not re-litigated here.
|
||||
- Whether `POST /approve` allows approving a subset now and a different
|
||||
subset later (multiple approve calls against the same classification), or
|
||||
is a single one-shot call — planner's call, no strong preference surfaced.
|
||||
Must not conflict with D-04's block-after-approved guard.
|
||||
- Exact JSON request/response shapes for approve/remediate/mark-false-positive
|
||||
— follow the project's camelCase API convention; `remediation_actions` and
|
||||
`audit_events` are already JSONB-backed tables from migration 097.
|
||||
- Whether `not_implemented` still has any code path in this phase (e.g. as a
|
||||
defensive fallback if a future action type is added without wiring D-01's
|
||||
simulated-effect handling) — planner's call; REMED-03's exact wording is
|
||||
satisfied by D-01's simulated-effect model since it always "takes or logs
|
||||
an action" rather than silently succeeding.
|
||||
|
||||
</decisions>
|
||||
|
||||
<canonical_refs>
|
||||
## Canonical References
|
||||
|
||||
**Downstream agents MUST read these before planning or implementing.**
|
||||
|
||||
### Schema (this phase writes)
|
||||
- `migrations/097_phishing_triage_schema.sql` — `remediation_actions` table
|
||||
(`action_type`, `status` DEFAULT `'proposed'`, `params` JSONB,
|
||||
`approved_by`, `approved_at`) and `audit_events` table (`actor`,
|
||||
`event_type`, `payload` JSONB) — both currently stubs, this phase is their
|
||||
first real writer.
|
||||
|
||||
### Action vocabulary (locked upstream, not re-decided)
|
||||
- `.planning/phases/19-classification-engine/19-CONTEXT.md` D-08 — the 7
|
||||
action types (`no_action`, `warn_user`, `disable_forwarding_rule` /
|
||||
`block_sender`, `purge_message`, `reset_password`, `isolate_endpoint`) and
|
||||
the OR'd `requires_approval` invariant this phase's approve logic must
|
||||
respect.
|
||||
- `lib/services/campaign-classifier.ts` — `classifyCampaign()` output shape
|
||||
(`recommendedActions`, `requiresApproval`) that `POST /approve` reads from
|
||||
the most recent `classifications` row for the campaign.
|
||||
|
||||
### Auth/permissions (this phase grants; Phase 18 declared the vocabulary)
|
||||
- `lib/permissions.ts` — `phishing` statement already has
|
||||
`["read", "analyze", "approve", "remediate"]` (Phase 18 D-05); this phase
|
||||
adds `"approve", "remediate"` to `superAdminRole` and `adminRole` (D-02).
|
||||
- `lib/auth-utils.ts` — `requirePermission(resource, action)` — same
|
||||
call-and-early-return pattern as every other `/api/phishing/*` route
|
||||
(Phase 18 D-06 convention).
|
||||
- `app/api/phishing/campaigns/[id]/classify/route.ts` (Phase 19) — the exact
|
||||
route-shape precedent (UUID guard + `requirePermission` + delegation) this
|
||||
phase's three new routes should copy.
|
||||
|
||||
### Provider capability audit (confirms D-01's "no real provider" premise)
|
||||
- `lib/services/mimecast-client.ts` — no block/purge/quarantine method
|
||||
exists (`getHeldMessages`/`releaseHeldMessage` are read/release only, not
|
||||
a send-time block).
|
||||
- `lib/services/msgraph-client.ts` — no mailbox forwarding-rule or password-
|
||||
reset method exists.
|
||||
- No RMM isolate/quarantine method exists anywhere in `lib/services/`.
|
||||
|
||||
### Prior phase decisions (for consistency)
|
||||
- `.planning/phases/19-classification-engine/19-CONTEXT.md` (D-08 action
|
||||
vocabulary; D-02 append-only classification history this phase reads from)
|
||||
- `.planning/phases/18-campaign-grouping-phishing-analysis-api/18-CONTEXT.md`
|
||||
(D-05 permission statement vocabulary; D-06 per-route `requirePermission`
|
||||
convention)
|
||||
|
||||
</canonical_refs>
|
||||
|
||||
<code_context>
|
||||
## Existing Code Insights
|
||||
|
||||
### Reusable Assets
|
||||
- `app/api/phishing/campaigns/[id]/classify/route.ts` — exact structural
|
||||
precedent (UUID param guard, `requirePermission`, service delegation,
|
||||
error shape) for the three new routes in this phase.
|
||||
- `lib/permissions.ts`'s existing per-role object literals — this phase adds
|
||||
two array entries (`"approve"`, `"remediate"`) to two existing role
|
||||
objects; no new statement keys needed (Phase 18 D-05 already declared
|
||||
them).
|
||||
|
||||
### Established Patterns
|
||||
- Append-only history tables with `campaign_id` FK + `created_at` (same
|
||||
shape `classifications` used in Phase 19) — `remediation_actions` and
|
||||
`audit_events` should follow the same insert-only shape, no upsert.
|
||||
- `requirePermission(resource, action)` early-return pattern for every
|
||||
`/api/phishing/*` route (Phase 18 D-06) — this phase's three routes follow
|
||||
the same shape with `'approve'` / `'remediate'` actions respectively.
|
||||
|
||||
### Integration Points
|
||||
- `POST /approve` reads the campaign's most recent `classifications` row
|
||||
(Phase 19) for `recommendedActions`/`requiresApproval`, writes one or more
|
||||
`remediation_actions` rows.
|
||||
- `POST /remediate` reads approved `remediation_actions` rows for the
|
||||
campaign, transitions eligible ones to `completed` (D-01), writes
|
||||
`audit_events`.
|
||||
- `POST /mark-false-positive` checks `remediation_actions` state (D-04) before
|
||||
proceeding, writes `audit_events`.
|
||||
- Phase 21 (Autotask triage note) and Phase 22 (LiveLink approval UI) both
|
||||
read `remediation_actions`/`audit_events` written by this phase — the
|
||||
status values and audit event shape are a direct contract with both.
|
||||
|
||||
</code_context>
|
||||
|
||||
<specifics>
|
||||
## Specific Ideas
|
||||
|
||||
No specific UI/reference examples surfaced — this is a pure API/backend
|
||||
phase (`UI hint: no` per ROADMAP.md). Discussion focused entirely on the
|
||||
approve/remediate/audit contract shape.
|
||||
|
||||
</specifics>
|
||||
|
||||
<deferred>
|
||||
## Deferred Ideas
|
||||
|
||||
- **Real remediation provider wiring** (Mimecast block, Graph forwarding-rule
|
||||
toggle, RMM isolate, password reset) — `REMEDEXEC-01..05`, explicitly v2.
|
||||
D-01's simulated-effect model is the placeholder until a v2 phase picks one
|
||||
provider at a time.
|
||||
- **False-positive reversibility** — whether an operator can un-mark a false
|
||||
positive later — not discussed, no signal it's needed for this phase;
|
||||
planner should treat it as out of scope unless it blocks a success
|
||||
criterion.
|
||||
- **Multi-round approval** (approving additional actions after an initial
|
||||
approve call) — noted as Claude's discretion (see `<decisions>`), not a
|
||||
locked requirement either way.
|
||||
|
||||
</deferred>
|
||||
|
||||
---
|
||||
|
||||
*Phase: 20-remediation-approval-audit-safety*
|
||||
*Context gathered: 2026-07-16*
|
||||
|
|
@ -0,0 +1,79 @@
|
|||
# Phase 20: Remediation, Approval & Audit Safety - Discussion Log
|
||||
|
||||
> **Audit trail only.** Do not use as input to planning, research, or execution agents.
|
||||
> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered.
|
||||
|
||||
**Date:** 2026-07-16
|
||||
**Phase:** 20-remediation-approval-audit-safety
|
||||
**Areas discussed:** Remediate scope, Permission grant, Approval scope, FP state guard
|
||||
|
||||
---
|
||||
|
||||
## Remediate scope
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Simulated internal effect | No real external API call for any action type. Remediate transitions `remediation_actions.status` to `completed` internally, writes the audit event, and returns success — the "effect" REMED-04's idempotency test asserts doesn't duplicate. All 7 action types behave the same way. | ✓ |
|
||||
| Always not_implemented | Every remediate call returns not_implemented/501 for every action type, since no provider is configured. Pure safety-contract stub. | |
|
||||
| Wire one real action for real | Actually implement `disable_forwarding_rule` via MS Graph — real effect for one action, not_implemented for the other 6. More scope/risk. | |
|
||||
|
||||
**User's choice:** Simulated internal effect (Recommended)
|
||||
**Notes:** Confirmed no provider client (Mimecast, MS Graph, RMM) has a block/purge/forwarding-rule/isolate method today, before presenting this question.
|
||||
|
||||
---
|
||||
|
||||
## Permission grant
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Admin + Super-Admin | Same tier as `analyze` today — parity with how this milestone has treated admin/super-admin identically so far. | ✓ |
|
||||
| Super-Admin only | Stricter tier — approve/remediate can affect real security posture, so only super-admin gets it. | |
|
||||
|
||||
**User's choice:** Admin + Super-Admin (Recommended)
|
||||
**Notes:** Confirmed via direct read of `lib/permissions.ts` that both roles currently have only `phishing: ["read", "analyze"]` before asking.
|
||||
|
||||
---
|
||||
|
||||
## Approval scope
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Operator selects a subset | Request body includes which action(s) to approve and can override params (e.g. narrowing `block_sender` to a specific address). Matches REMED-02's "exact approved action parameters" wording. | ✓ |
|
||||
| Approve the full set verbatim | One approve call locks in ALL recommended_actions exactly as proposed — simpler, less operator control. | |
|
||||
|
||||
**User's choice:** Operator selects a subset (Recommended)
|
||||
**Notes:** None.
|
||||
|
||||
---
|
||||
|
||||
## FP state guard
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Block after approve/remediate | mark-false-positive returns an explicit conflict/error if the campaign already has an approved or completed remediation_actions row. | ✓ |
|
||||
| Always allow, just label it | mark-false-positive is a pure label change regardless of remediation state — simpler but permits a contradictory audit trail. | |
|
||||
|
||||
**User's choice:** Block after approve/remediate (Recommended)
|
||||
**Notes:** None.
|
||||
|
||||
---
|
||||
|
||||
## Wrap-up check
|
||||
|
||||
Asked whether any further gray areas needed discussion before writing context.
|
||||
|
||||
**User's choice:** I'm ready for context (Recommended)
|
||||
|
||||
## Claude's Discretion
|
||||
|
||||
- Exact idempotency detection mechanism for `POST /remediate` re-runs.
|
||||
- Exact `audit_events.actor` value format (likely session email).
|
||||
- Whether `POST /approve` allows multiple approve calls over time vs. one-shot.
|
||||
- Exact JSON request/response shapes for the three new routes.
|
||||
- Whether a `not_implemented` code path still exists as a defensive fallback.
|
||||
|
||||
## Deferred Ideas
|
||||
|
||||
- Real remediation provider wiring (Mimecast block, Graph forwarding-rule, RMM isolate, password reset) — `REMEDEXEC-01..05`, v2.
|
||||
- False-positive reversibility — not discussed, no signal it's needed.
|
||||
- Multi-round approval — noted as Claude's discretion, not locked.
|
||||
Loading…
Add table
Add a link
Reference in a new issue