PRD 0014: cred-proxy block remediation #20
@@ -22,6 +22,10 @@ from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
|
||||
from .. import supervise as _supervise
|
||||
from ..backend.docker.cred_proxy_apply import (
|
||||
CredProxyApplyError,
|
||||
apply_routes_change,
|
||||
)
|
||||
from ..log import info
|
||||
from ..supervise import (
|
||||
ACTION_OPERATOR_EDIT,
|
||||
@@ -33,6 +37,7 @@ from ..supervise import (
|
||||
STATUS_MODIFIED,
|
||||
STATUS_REJECTED,
|
||||
TOOL_CAPABILITY_BLOCK,
|
||||
TOOL_CRED_PROXY_BLOCK,
|
||||
list_pending_proposals,
|
||||
render_diff,
|
||||
write_audit_entry,
|
||||
@@ -78,9 +83,28 @@ def approve(
|
||||
notes: str = "",
|
||||
final_file: str | None = None,
|
||||
) -> None:
|
||||
"""Write an approval response and an audit entry. If `final_file`
|
||||
is provided the status is `modified`; otherwise `approved`."""
|
||||
"""Apply the proposal to the running sidecar, write the response
|
||||
file the agent's tool call is waiting on, and append an audit
|
||||
entry. If `final_file` is provided the status is `modified`;
|
||||
otherwise `approved`.
|
||||
|
||||
Raises CredProxyApplyError if the cred-proxy-block apply fails
|
||||
(sidecar down, invalid JSON survived the operator's modify).
|
||||
On failure no response is written and no audit entry is
|
||||
appended — the proposal stays pending so the operator can fix
|
||||
the input and retry."""
|
||||
status = STATUS_MODIFIED if final_file is not None else STATUS_APPROVED
|
||||
file_to_apply = final_file if final_file is not None else qp.proposal.proposed_file
|
||||
|
||||
diff_before, diff_after = "", ""
|
||||
if qp.proposal.tool == TOOL_CRED_PROXY_BLOCK:
|
||||
diff_before, diff_after = apply_routes_change(
|
||||
qp.proposal.bottle_slug, file_to_apply,
|
||||
)
|
||||
# pipelock-block + capability-block remediation lands in PRDs
|
||||
# 0015 + 0016; for 0014 they remain no-op approvals and the
|
||||
# audit diff stays empty.
|
||||
|
||||
response = Response(
|
||||
proposal_id=qp.proposal.id,
|
||||
status=status,
|
||||
@@ -88,11 +112,16 @@ def approve(
|
||||
final_file=final_file,
|
||||
)
|
||||
write_response(qp.queue_dir, response)
|
||||
_write_audit(qp, action=status, notes=notes, final_file=final_file)
|
||||
_write_audit(
|
||||
qp, action=status, notes=notes,
|
||||
diff_before=diff_before, diff_after=diff_after,
|
||||
)
|
||||
|
||||
|
||||
def reject(qp: QueuedProposal, *, reason: str) -> None:
|
||||
"""Write a rejection response and an audit entry."""
|
||||
"""Write a rejection response and an audit entry. No remediation
|
||||
apply happens on reject — the agent sees the rejection and
|
||||
decides whether to retry / give up."""
|
||||
response = Response(
|
||||
proposal_id=qp.proposal.id,
|
||||
status=STATUS_REJECTED,
|
||||
@@ -100,7 +129,7 @@ def reject(qp: QueuedProposal, *, reason: str) -> None:
|
||||
final_file=None,
|
||||
)
|
||||
write_response(qp.queue_dir, response)
|
||||
_write_audit(qp, action=STATUS_REJECTED, notes=reason, final_file=None)
|
||||
_write_audit(qp, action=STATUS_REJECTED, notes=reason, diff_before="", diff_after="")
|
||||
|
||||
|
||||
def _write_audit(
|
||||
@@ -108,19 +137,21 @@ def _write_audit(
|
||||
*,
|
||||
action: str,
|
||||
notes: str,
|
||||
final_file: str | None,
|
||||
diff_before: str,
|
||||
diff_after: str,
|
||||
) -> None:
|
||||
"""Audit log for cred-proxy / pipelock tools. capability-block has
|
||||
no audit log (its changes are captured by the bottle's rebuild
|
||||
record + git history per PRD 0016)."""
|
||||
record + git history per PRD 0016).
|
||||
|
||||
For cred-proxy-block approvals the (before, after) come from the
|
||||
apply_routes_change return — a real fetched-from-sidecar diff.
|
||||
For rejections, or for tools whose remediation hasn't landed yet
|
||||
(pipelock in 0014, capability anywhere), both are empty strings
|
||||
and the audit diff renders as empty."""
|
||||
component = COMPONENT_FOR_TOOL.get(qp.proposal.tool)
|
||||
if component is None:
|
||||
# capability-block: skip audit log; 0016 records via rebuild.
|
||||
return
|
||||
# v1 audit diff is empty: 0013's no-op handler doesn't have the
|
||||
# actual current-on-disk file to diff against, only the agent's
|
||||
# proposed file. 0014 / 0015 fill in the real diff against the
|
||||
# live routes.json / allowlist after writing the change.
|
||||
write_audit_entry(AuditEntry(
|
||||
timestamp=datetime.now(timezone.utc).isoformat(),
|
||||
bottle_slug=qp.proposal.bottle_slug,
|
||||
@@ -128,11 +159,7 @@ def _write_audit(
|
||||
operator_action=action,
|
||||
operator_notes=notes,
|
||||
justification=qp.proposal.justification,
|
||||
diff=render_diff(
|
||||
"",
|
||||
final_file if final_file is not None else qp.proposal.proposed_file,
|
||||
label=component,
|
||||
),
|
||||
diff=render_diff(diff_before, diff_after, label=component),
|
||||
))
|
||||
|
||||
|
||||
@@ -228,15 +255,21 @@ def _main_loop(stdscr: "curses._CursesWindow") -> None:
|
||||
elif key in (curses.KEY_ENTER, 10, 13, ord("v")):
|
||||
_detail_view(stdscr, qp)
|
||||
elif key == ord("a"):
|
||||
approve(qp)
|
||||
status_line = f"approved {qp.proposal.tool} for [{qp.proposal.bottle_slug}]"
|
||||
try:
|
||||
approve(qp)
|
||||
status_line = f"approved {qp.proposal.tool} for [{qp.proposal.bottle_slug}]"
|
||||
except CredProxyApplyError as e:
|
||||
status_line = f"apply failed: {e}"
|
||||
elif key == ord("m"):
|
||||
edited = _modify(stdscr, qp)
|
||||
if edited is None:
|
||||
status_line = "modify aborted (no change)"
|
||||
else:
|
||||
approve(qp, final_file=edited, notes="operator modified before approving")
|
||||
status_line = f"modified+approved {qp.proposal.tool} for [{qp.proposal.bottle_slug}]"
|
||||
try:
|
||||
approve(qp, final_file=edited, notes="operator modified before approving")
|
||||
status_line = f"modified+approved {qp.proposal.tool} for [{qp.proposal.bottle_slug}]"
|
||||
except CredProxyApplyError as e:
|
||||
status_line = f"apply failed: {e}"
|
||||
elif key == ord("r"):
|
||||
reason = _prompt(stdscr, "reject reason: ")
|
||||
if reason:
|
||||
@@ -316,12 +349,18 @@ def _detail_view(stdscr: "curses._CursesWindow", qp: QueuedProposal) -> None:
|
||||
elif key == ord("G"):
|
||||
offset = max(0, len(lines) - 1)
|
||||
elif key == ord("a"):
|
||||
approve(qp)
|
||||
try:
|
||||
approve(qp)
|
||||
except CredProxyApplyError:
|
||||
pass # Status surfaces back in the list view's render.
|
||||
return
|
||||
elif key == ord("m"):
|
||||
edited = _modify(stdscr, qp)
|
||||
if edited is not None:
|
||||
approve(qp, final_file=edited, notes="operator modified before approving")
|
||||
try:
|
||||
approve(qp, final_file=edited, notes="operator modified before approving")
|
||||
except CredProxyApplyError:
|
||||
pass
|
||||
return
|
||||
elif key == ord("r"):
|
||||
reason = _prompt(stdscr, "reject reason: ")
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
"""Unit: dashboard headless paths (PRD 0013 phase 4).
|
||||
"""Unit: dashboard headless paths (PRD 0013 phase 4, PRD 0014).
|
||||
|
||||
The curses TUI itself isn't exercised here — these tests cover the
|
||||
discovery + approve/reject + audit-write paths that the TUI's key
|
||||
handlers call into.
|
||||
|
||||
apply_routes_change is stubbed at the dashboard module level so the
|
||||
tests don't need a running cred-proxy sidecar; the real docker
|
||||
exec/cp/SIGHUP plumbing is covered by the integration test.
|
||||
"""
|
||||
|
||||
import os
|
||||
@@ -12,6 +16,7 @@ from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
|
||||
from claude_bottle import supervise
|
||||
from claude_bottle.backend.docker.cred_proxy_apply import CredProxyApplyError
|
||||
from claude_bottle.cli import dashboard
|
||||
from claude_bottle.supervise import (
|
||||
Proposal,
|
||||
@@ -110,8 +115,15 @@ class TestDiscoverPending(_FakeHomeMixin, unittest.TestCase):
|
||||
class TestApproveReject(_FakeHomeMixin, unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._setup_fake_home()
|
||||
self._original_apply = dashboard.apply_routes_change
|
||||
# Default stub: succeed with deterministic before/after so the
|
||||
# audit log shows a non-empty diff.
|
||||
dashboard.apply_routes_change = lambda slug, content: (
|
||||
'{"routes": []}\n', content,
|
||||
)
|
||||
|
||||
def tearDown(self):
|
||||
dashboard.apply_routes_change = self._original_apply
|
||||
self._teardown_fake_home()
|
||||
|
||||
def _enqueue(self, tool: str = TOOL_CRED_PROXY_BLOCK):
|
||||
@@ -166,6 +178,96 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase):
|
||||
self.assertEqual(0, len(read_audit_entries("cred-proxy", "dev")))
|
||||
|
||||
|
||||
class TestCredProxyApplyWiring(_FakeHomeMixin, unittest.TestCase):
|
||||
"""PRD 0014 Phase 3: approve() on a cred-proxy-block proposal
|
||||
must call apply_routes_change with the right args and surface
|
||||
its failures."""
|
||||
|
||||
def setUp(self):
|
||||
self._setup_fake_home()
|
||||
self._original_apply = dashboard.apply_routes_change
|
||||
|
||||
def tearDown(self):
|
||||
dashboard.apply_routes_change = self._original_apply
|
||||
self._teardown_fake_home()
|
||||
|
||||
def _enqueue_cred_proxy(self, proposed: str = '{"routes": []}\n'):
|
||||
p = Proposal.new(
|
||||
bottle_slug="dev", tool=TOOL_CRED_PROXY_BLOCK,
|
||||
proposed_file=proposed,
|
||||
justification="need a route",
|
||||
current_file_hash=sha256_hex(proposed),
|
||||
now=FIXED,
|
||||
)
|
||||
qdir = supervise.queue_dir_for_slug("dev")
|
||||
qdir.mkdir(parents=True, exist_ok=True)
|
||||
supervise.write_proposal(qdir, p)
|
||||
return dashboard.QueuedProposal(proposal=p, queue_dir=qdir)
|
||||
|
||||
def test_cred_proxy_block_calls_apply_with_proposed_file(self):
|
||||
calls = []
|
||||
dashboard.apply_routes_change = lambda slug, content: (
|
||||
calls.append((slug, content)) or ("before", content)
|
||||
)
|
||||
qp = self._enqueue_cred_proxy(proposed='{"routes": [{"path": "/new/"}]}\n')
|
||||
dashboard.approve(qp)
|
||||
self.assertEqual(1, len(calls))
|
||||
slug, content = calls[0]
|
||||
self.assertEqual("dev", slug)
|
||||
self.assertEqual('{"routes": [{"path": "/new/"}]}\n', content)
|
||||
|
||||
def test_modify_passes_final_file_to_apply(self):
|
||||
calls = []
|
||||
dashboard.apply_routes_change = lambda slug, content: (
|
||||
calls.append(content) or ("before", content)
|
||||
)
|
||||
qp = self._enqueue_cred_proxy()
|
||||
dashboard.approve(qp, final_file='{"routes": [{"path": "/edited/"}]}\n', notes="tweaked")
|
||||
self.assertEqual(['{"routes": [{"path": "/edited/"}]}\n'], calls)
|
||||
|
||||
def test_apply_failure_blocks_response_and_audit(self):
|
||||
dashboard.apply_routes_change = lambda slug, content: (_ for _ in ()).throw(
|
||||
CredProxyApplyError("docker exec failed")
|
||||
)
|
||||
qp = self._enqueue_cred_proxy()
|
||||
with self.assertRaises(CredProxyApplyError):
|
||||
dashboard.approve(qp)
|
||||
# No response file (proposal stays pending).
|
||||
self.assertEqual(
|
||||
[qp.proposal.id],
|
||||
[p.id for p in supervise.list_pending_proposals(qp.queue_dir)],
|
||||
)
|
||||
# No audit entry.
|
||||
self.assertEqual([], read_audit_entries("cred-proxy", "dev"))
|
||||
|
||||
def test_real_diff_lands_in_audit(self):
|
||||
dashboard.apply_routes_change = lambda slug, content: (
|
||||
'{"routes": []}\n', # before
|
||||
'{"routes": [{"path": "/new/"}]}\n', # after
|
||||
)
|
||||
qp = self._enqueue_cred_proxy(proposed='{"routes": [{"path": "/new/"}]}\n')
|
||||
dashboard.approve(qp)
|
||||
entries = read_audit_entries("cred-proxy", "dev")
|
||||
self.assertEqual(1, len(entries))
|
||||
self.assertIn('+{"routes": [{"path": "/new/"}]}', entries[0].diff)
|
||||
self.assertIn('-{"routes": []}', entries[0].diff)
|
||||
|
||||
def test_reject_does_not_call_apply(self):
|
||||
called = []
|
||||
dashboard.apply_routes_change = lambda slug, content: (
|
||||
called.append(True) or ("", content)
|
||||
)
|
||||
qp = self._enqueue_cred_proxy()
|
||||
dashboard.reject(qp, reason="no thanks")
|
||||
self.assertEqual([], called)
|
||||
# Reject still writes a response + audit entry with empty diff.
|
||||
resp = read_response(qp.queue_dir, qp.proposal.id)
|
||||
self.assertEqual(STATUS_REJECTED, resp.status)
|
||||
entries = read_audit_entries("cred-proxy", "dev")
|
||||
self.assertEqual(1, len(entries))
|
||||
self.assertEqual("", entries[0].diff)
|
||||
|
||||
|
||||
class TestEditInEditor(unittest.TestCase):
|
||||
def test_runs_editor_returns_edited_content(self):
|
||||
# Fake "editor" is /bin/sh -c 'cat <<EOF > $1 ... EOF'
|
||||
|
||||
Reference in New Issue
Block a user