Skip to content

feat: define approval/idempotency/audit/rescan gate for agent-driven remediation #266

Closed

Description

@parthrohit22

What problem does this solve?

Any future agent-proposed remediation (running a playbooks/cli/*.sh script against a real
subscription) has real blast radius — a retry or a bad proposal can change customer
infrastructure. Phase one should be proposal-only, but the execution gate needs to be specified
now so no track starts building an execution loop without it already in place.

Describe the solution

  • Typed allowlist of playbooks eligible for any future automated execution (starts empty).
  • Mandatory human approval step before any execution.
  • Idempotency guarantee: a retried action must never execute twice.
  • Full audit log of proposal → approval → execution → rescan-verification.
  • Rescan-based verification: a remediation is "pending verification," not "done," until a rescan
    confirms the finding cleared; failed verification stays open, not silently closed.
  • Phase one ships proposal generation only — no execution path merges until this gate exists in
    code, not just as a design doc.

Alternatives considered

Build the proposal UI now and add the safety gate right before the first real execution ships —
rejected; it has to exist before any execution-adjacent code is written.

Additional context

Exit test: retries never execute twice; failed verification remains open.

Activity

  1. added
    enhancementNew feature or request
    coreCore team ownership not for students
    priority: highImportant, should be fixed in the current sprint
    on Aug 13, 2026
  2. dipeshrayg commented on Aug 19, 2026

    @dipeshrayg
    Contributor

    I would like to work on this.

  3. dipeshrayg commented on Aug 19, 2026

    @dipeshrayg
    Contributor

    I'd like to take this on. Here's the design for phase one, scoped exactly to what the issue asks: the gate itself, not execution.

    What exists today: GET /api/findings/<id>/playbook (api/routes/findings.py) only returns the CLI script as text for a human to read and run manually. There is no persisted proposal, no approval step, and no execution path anywhere in the codebase. Phase one builds the gate that any future execution work will have to go through.

    New table, remediation_actions (Alembic migration, matching the existing baseline/rate_limit_hits migration style):

    id, finding_id (FK findings), rule_id, idempotency_key (unique),
    status (PROPOSED | APPROVED | REJECTED | EXECUTING | PENDING_VERIFICATION | VERIFIED | VERIFICATION_FAILED),
    proposed_at, approved_by, approved_at, executed_at, verified_at
    

    New table, remediation_audit_log, append-only: every status transition recorded with actor, timestamp, and a note. Nothing in this system changes state without a row here.

    The allowlist gate lives in one new module, a plain set of rule IDs eligible for execution, empty by default. Any code path that would execute a playbook has to check membership first and refuse otherwise. Since the set starts empty, this repo ships with execution structurally impossible until someone deliberately adds a rule to it, which is the actual safety property the issue is asking for.

    Phase one API surface, propose and approve only:

    • POST /api/findings/<id>/remediation/propose creates a PROPOSED row. Idempotency is enforced by uniquing on (finding_id, non-terminal status), a retried propose call for the same finding returns the existing action instead of creating a second one.
    • POST /api/remediation/<action_id>/approve moves PROPOSED to APPROVED and logs the approver.
    • No endpoint moves anything to EXECUTING. The status enum includes EXECUTING / PENDING_VERIFICATION / VERIFIED / VERIFICATION_FAILED so the contract exists for whoever builds execution next, but nothing in this PR can reach those states, since the allowlist gate blocks it structurally, not just by omission.

    Test coverage proves the two guarantees the issue names as its exit test: proposing twice for the same finding never creates two rows, and no code path can move an action past APPROVED while the allowlist is empty.

    Scope boundary: actual playbook execution and rescan-based verification are explicitly out of this PR. They're real future work once someone wants to add a rule to the allowlist, but this issue's own text says phase one is proposal-only, so I'm not building toward execution speculatively.

  4. github-actions commented on Sep 19, 2026

    @github-actions
    Contributor

    This issue has been inactive for 30 days and will be closed in 7 days unless there is activity.

  5. babyblueviper1 commented on Sep 19, 2026

    @babyblueviper1

    The propose-side idempotency (unique on (finding_id, non-terminal status)) is solid. One thing worth naming explicitly in the design before this merges, since it's the exact class of bug that's bitten us in a live system: the approve side needs the same discipline. Two concurrent POST /remediation/<action_id>/approve calls hitting the same PROPOSED row is a real race if the status check-then-update isn't atomic (e.g. a compare-and-swap on status='PROPOSED' -> 'APPROVED' at the DB layer, not a read-then-write in application code) — otherwise both requests can succeed, double-log an approval, and (once execution exists) potentially both think they're authorized to trigger the playbook. We've hit this exact shape of bug (a check-then-act gap on a state transition, not the initial creation) in our own agent-approval pipeline more than once, and it's easy to miss because propose-side idempotency looks like it covers the whole flow when it only covers half of it. Worth a row-level lock or a conditional UPDATE ... WHERE status='PROPOSED' on the approve endpoint specifically, with the affected-rows count as the real signal (0 rows = already approved/rejected by someone else, not an error).

  6. self-assigned this
    on Sep 30, 2026
  7. parthrohit22 commented on Sep 30, 2026

    @parthrohit22
    CollaboratorAuthor

    Update on #266: I have opened #364 with the phase-one gate. It is database and gate logic only. It has no API routes and nothing executes a playbook.

    @dipeshrayg, thank you for the design you posted on Aug 19. I should have replied before starting. #364 follows the same shape you proposed: an action table, an append-only audit log, an allowlist that starts empty, and idempotent proposals. It does not include the propose and approve endpoints from your design. If you still want to build those on top of #364 once it is reviewed, please say so here. I would rather review your PR than duplicate it.

    @ritiksah141 @TFT444, #313 and #314 describe their own remediation state machine. #364 overlaps with that. Could you take a look and tell me whether it fits the Automation series, should be reshaped to match it, or should be dropped? I will adjust it to whatever you decide.

    Design notes are in docs/remediation-gate.md on the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

coreCore team ownership not for studentsenhancementNew feature or requestpriority: highImportant, should be fixed in the current sprint

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions