You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: define approval/idempotency/audit/rescan gate for agent-driven remediation #266
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.
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):
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.
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).
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.
Activity
dipeshrayg commentedon Aug 19, 2026
I would like to work on this.
dipeshrayg commentedon Aug 19, 2026
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):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/proposecreates aPROPOSEDrow. 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>/approvemovesPROPOSEDtoAPPROVEDand logs the approver.EXECUTING. The status enum includesEXECUTING/PENDING_VERIFICATION/VERIFIED/VERIFICATION_FAILEDso 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
APPROVEDwhile 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.
github-actions commentedon Sep 19, 2026
This issue has been inactive for 30 days and will be closed in 7 days unless there is activity.
babyblueviper1 commentedon Sep 19, 2026
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 concurrentPOST /remediation/<action_id>/approvecalls hitting the samePROPOSEDrow is a real race if the status check-then-update isn't atomic (e.g. a compare-and-swap onstatus='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).fix(api): satisfy Bandit and Semgrep on the remediation gate (OWASP#266)
feat(api): add approval, idempotency, audit and rescan gate for remed…
fix(api): satisfy Bandit and Semgrep on the remediation gate (OWASP#266)
parthrohit22 commentedon Sep 30, 2026
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.mdon the PR.feat(api): add approval, idempotency, audit and rescan gate for remed…
fix(api): satisfy Bandit and Semgrep on the remediation gate (OWASP#266)
feat(api): add approval, idempotency, audit and rescan gate for remed…