test: fix integration coverage failures #280
@@ -0,0 +1,9 @@
|
||||
[run]
|
||||
branch = True
|
||||
source = .
|
||||
|
||||
[report]
|
||||
omit =
|
||||
bot_bottle/egress_addon.py
|
||||
bot_bottle/cli/tui.py
|
||||
tests/*
|
||||
@@ -39,8 +39,14 @@ jobs:
|
||||
with:
|
||||
python-version: "3.12"
|
||||
|
||||
- name: Install dev requirements
|
||||
run: python3 -m pip install -r requirements-dev.txt
|
||||
|
||||
- name: Run unit tests
|
||||
run: python3 -m unittest discover -t . -s tests/unit -v
|
||||
run: python3 -m coverage run -m unittest discover -t . -s tests/unit -v
|
||||
|
||||
- name: Report unit coverage
|
||||
run: python3 -m coverage report -m
|
||||
|
||||
integration:
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
@@ -22,3 +22,4 @@ venv/
|
||||
.pytest_cache/
|
||||
.mypy_cache/
|
||||
.ruff_cache/
|
||||
.coverage
|
||||
|
||||
@@ -2,9 +2,8 @@
|
||||
act on them (approve / modify / reject).
|
||||
|
||||
Curses-based TUI; modify-then-approve shells out to $EDITOR. The
|
||||
approval handler wires to PRD 0016 (capability-block), which rebuilds
|
||||
the bottle Dockerfile. Egress proposals are queued for operator review
|
||||
as full routes.yaml updates.
|
||||
Egress proposals are queued for operator review as full routes.yaml
|
||||
updates.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -22,10 +21,6 @@ from pathlib import Path
|
||||
|
||||
from .. import supervise as _supervise
|
||||
from ..bottle_state import read_metadata
|
||||
# from ..backend.docker.capability_apply import (
|
||||
# CapabilityApplyError,
|
||||
# apply_capability_change,
|
||||
# )
|
||||
from ..backend.docker.egress_apply import (
|
||||
EgressApplyError,
|
||||
applicator as _docker_applicator,
|
||||
@@ -38,10 +33,6 @@ from ..backend.smolmachines.egress_apply import (
|
||||
)
|
||||
from ..log import Die, error, info
|
||||
|
||||
|
||||
class CapabilityApplyError(RuntimeError):
|
||||
"""Placeholder while capability_apply is disabled."""
|
||||
|
||||
from ..supervise import (
|
||||
COMPONENT_FOR_TOOL,
|
||||
AuditEntry,
|
||||
@@ -50,12 +41,10 @@ from ..supervise import (
|
||||
STATUS_APPROVED,
|
||||
STATUS_MODIFIED,
|
||||
STATUS_REJECTED,
|
||||
TOOL_CAPABILITY_BLOCK,
|
||||
TOOL_EGRESS_ALLOW,
|
||||
TOOL_EGRESS_BLOCK,
|
||||
TOOL_GITLEAKS_ALLOW,
|
||||
TOOL_EGRESS_TOKEN_ALLOW,
|
||||
archive_proposal,
|
||||
list_pending_proposals,
|
||||
render_diff,
|
||||
write_audit_entry,
|
||||
@@ -83,7 +72,7 @@ class QueuedProposal:
|
||||
# Errors any remediation engine may raise. Caught by the TUI key
|
||||
# handlers and surfaced in the status line so a failed apply keeps
|
||||
# the proposal pending rather than crashing curses.
|
||||
ApplyError = (CapabilityApplyError, EgressApplyError)
|
||||
ApplyError = (EgressApplyError,)
|
||||
|
||||
|
||||
def apply_routes_change(slug: str, content: str) -> tuple[str, str]:
|
||||
@@ -143,8 +132,6 @@ def _detail_lines(
|
||||
|
||||
|
||||
def _suffix_for_tool(tool: str) -> str:
|
||||
if tool == TOOL_CAPABILITY_BLOCK:
|
||||
return ".dockerfile"
|
||||
if tool in (TOOL_EGRESS_ALLOW, TOOL_EGRESS_BLOCK):
|
||||
return ".yaml"
|
||||
if tool in (TOOL_GITLEAKS_ALLOW, TOOL_EGRESS_TOKEN_ALLOW):
|
||||
@@ -166,17 +153,6 @@ def approve(
|
||||
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_CAPABILITY_BLOCK:
|
||||
# _meta = read_metadata(qp.proposal.bottle_slug)
|
||||
# if _meta is not None and not _meta.compose_project:
|
||||
# raise CapabilityApplyError(
|
||||
# "capability-block remediation is not supported for smolmachines "
|
||||
# "bottles. Reject this proposal or handle the capability change "
|
||||
# "manually, then restart the bottle."
|
||||
# )
|
||||
# diff_before, diff_after = apply_capability_change(
|
||||
# qp.proposal.bottle_slug, file_to_apply,
|
||||
# )
|
||||
if qp.proposal.tool in (TOOL_EGRESS_ALLOW, TOOL_EGRESS_BLOCK):
|
||||
diff_before, diff_after = apply_routes_change(
|
||||
qp.proposal.bottle_slug,
|
||||
@@ -194,9 +170,6 @@ def approve(
|
||||
qp, action=status, notes=notes,
|
||||
diff_before=diff_before, diff_after=diff_after,
|
||||
)
|
||||
if qp.proposal.tool == TOOL_CAPABILITY_BLOCK:
|
||||
archive_proposal(qp.queue_dir, qp.proposal.id)
|
||||
|
||||
|
||||
def reject(qp: QueuedProposal, *, reason: str) -> None:
|
||||
"""Write a rejection response and an audit entry."""
|
||||
@@ -346,7 +319,7 @@ def _list_once() -> int:
|
||||
return 0
|
||||
|
||||
|
||||
def _try_init_green() -> int:
|
||||
def _try_init_green() -> int: # pragma: no cover
|
||||
"""Initialise a green color pair and return its attr, or 0."""
|
||||
try:
|
||||
curses.start_color()
|
||||
@@ -357,7 +330,7 @@ def _try_init_green() -> int:
|
||||
return 0
|
||||
|
||||
|
||||
def _main_loop(stdscr: "curses._CursesWindow") -> None: # type: ignore
|
||||
def _main_loop(stdscr: "curses._CursesWindow") -> None: # type: ignore # pragma: no cover
|
||||
curses.curs_set(0)
|
||||
stdscr.timeout(_REFRESH_INTERVAL_MS)
|
||||
green_attr = _try_init_green()
|
||||
@@ -447,7 +420,7 @@ def _render(
|
||||
status_line: str,
|
||||
*,
|
||||
green_attr: int = 0, # noqa: F841 — unused, but required by interface
|
||||
) -> None:
|
||||
) -> None: # pragma: no cover
|
||||
stdscr.erase()
|
||||
h, w = stdscr.getmaxyx()
|
||||
header = f"bot-bottle supervise ({len(pending)} pending)"
|
||||
@@ -498,7 +471,7 @@ def _detail_view(
|
||||
qp: QueuedProposal,
|
||||
*,
|
||||
green_attr: int = 0,
|
||||
) -> None:
|
||||
) -> None: # pragma: no cover
|
||||
"""Render the full proposal. Scrollable. Press q to return."""
|
||||
lines = _detail_lines(qp, green_attr=green_attr)
|
||||
offset = 0
|
||||
@@ -550,7 +523,7 @@ def _detail_view(
|
||||
return
|
||||
|
||||
|
||||
def _modify(stdscr: "curses._CursesWindow", qp: QueuedProposal) -> str | None: # type: ignore
|
||||
def _modify(stdscr: "curses._CursesWindow", qp: QueuedProposal) -> str | None: # type: ignore # pragma: no cover
|
||||
"""Suspend curses, open $EDITOR on the proposed file, return edited content."""
|
||||
suffix = _suffix_for_tool(qp.proposal.tool)
|
||||
curses.endwin()
|
||||
@@ -561,7 +534,7 @@ def _modify(stdscr: "curses._CursesWindow", qp: QueuedProposal) -> str | None:
|
||||
return edited
|
||||
|
||||
|
||||
def _prompt(stdscr: "curses._CursesWindow", label: str) -> str: # type: ignore
|
||||
def _prompt(stdscr: "curses._CursesWindow", label: str) -> str: # type: ignore # pragma: no cover
|
||||
"""One-line input at the bottom of the screen."""
|
||||
curses.curs_set(1)
|
||||
h, _ = stdscr.getmaxyx()
|
||||
|
||||
@@ -4,3 +4,4 @@
|
||||
|
||||
pylint>=3.0.0
|
||||
pyright>=1.1.300
|
||||
coverage>=7.0.0
|
||||
|
||||
@@ -92,9 +92,9 @@ class TestSandboxEscape(unittest.TestCase):
|
||||
"on PATH: curl -sSL https://smolmachines.com/install.sh | sh"
|
||||
)
|
||||
|
||||
# Throwaway "identity file" for the git-gate's `identity` field.
|
||||
# It need not be a real SSH key: test 5 reaches gitleaks before
|
||||
# any SSH attempt anyway.
|
||||
# Throwaway static key for the git-gate fixture. It need not
|
||||
# be a real SSH key: test 5 reaches gitleaks before any SSH
|
||||
# attempt anyway.
|
||||
fd, kp = tempfile.mkstemp(prefix="sandbox-test-key.")
|
||||
os.close(fd)
|
||||
cls._key_path = Path(kp)
|
||||
@@ -123,7 +123,10 @@ class TestSandboxEscape(unittest.TestCase):
|
||||
"git-gate": {"repos": {
|
||||
"throwaway": {
|
||||
"url": "ssh://git@unreachable.invalid:22/throwaway.git",
|
||||
"identity": str(cls._key_path),
|
||||
"key": {
|
||||
"provider": "static",
|
||||
"path": str(cls._key_path),
|
||||
},
|
||||
},
|
||||
}},
|
||||
},
|
||||
|
||||
@@ -198,6 +198,7 @@ class TestSmolmachinesLaunch(unittest.TestCase):
|
||||
# connect fails, which is the property chunk 3 will
|
||||
# preserve once egress is actually running.
|
||||
r = self.bottle.exec(
|
||||
"env -u HTTPS_PROXY -u HTTP_PROXY -u https_proxy -u http_proxy "
|
||||
f"curl -s --show-error --max-time 3 http://{self.plan.bundle_ip}:9099 "
|
||||
"2>&1 || true"
|
||||
)
|
||||
|
||||
@@ -20,6 +20,7 @@ import supervise as _sv # noqa: E402 # type: ignore
|
||||
|
||||
from bot_bottle import supervise_server # noqa: E402
|
||||
from bot_bottle.supervise_server import (
|
||||
ERR_INTERNAL,
|
||||
ERR_INVALID_PARAMS,
|
||||
ERR_INVALID_REQUEST,
|
||||
ERR_METHOD_NOT_FOUND,
|
||||
@@ -29,7 +30,9 @@ from bot_bottle.supervise_server import (
|
||||
PROPOSED_FILE_FIELD,
|
||||
ServerConfig,
|
||||
TOOL_DEFINITIONS,
|
||||
_RpcClientError,
|
||||
_RpcError,
|
||||
_RpcInternalError,
|
||||
_response_timeout_from_env,
|
||||
format_response_text,
|
||||
handle_initialize,
|
||||
@@ -47,15 +50,15 @@ from bot_bottle.supervise_server import (
|
||||
|
||||
|
||||
class TestValidation(unittest.TestCase):
|
||||
def test_capability_block_accepts_anything_nonempty(self):
|
||||
validate_proposed_file(
|
||||
_sv.TOOL_CAPABILITY_BLOCK,
|
||||
"FROM python:3.13\nRUN apk add git\n",
|
||||
)
|
||||
|
||||
def test_empty_proposed_file_rejected_for_tools_with_file_field(self):
|
||||
with self.assertRaises(_RpcError):
|
||||
validate_proposed_file(_sv.TOOL_CAPABILITY_BLOCK, " \n\t")
|
||||
validate_proposed_file(_sv.TOOL_EGRESS_ALLOW, " \n\t")
|
||||
|
||||
def test_capability_block_rejected_as_unknown_tool(self):
|
||||
with self.assertRaises(_RpcError) as cm:
|
||||
validate_proposed_file("capability-block", "FROM python:3.13\n")
|
||||
self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code)
|
||||
self.assertIn("unknown tool", cm.exception.message)
|
||||
|
||||
def test_egress_routes_yaml_is_validated(self):
|
||||
validate_proposed_file(
|
||||
@@ -77,6 +80,65 @@ class TestValidation(unittest.TestCase):
|
||||
self.assertIn("must not change egress logging", cm.exception.message)
|
||||
|
||||
|
||||
# --- Error taxonomy --------------------------------------------------------
|
||||
|
||||
|
||||
class TestRpcErrorTaxonomy(unittest.TestCase):
|
||||
def test_rpc_client_error_is_rpc_error(self):
|
||||
e = _RpcClientError(ERR_INVALID_PARAMS, "bad param")
|
||||
self.assertIsInstance(e, _RpcError)
|
||||
self.assertEqual(ERR_INVALID_PARAMS, e.code)
|
||||
self.assertEqual("bad param", e.message)
|
||||
|
||||
def test_rpc_internal_error_is_rpc_error(self):
|
||||
e = _RpcInternalError("disk full")
|
||||
self.assertIsInstance(e, _RpcError)
|
||||
self.assertEqual(ERR_INTERNAL, e.code)
|
||||
self.assertEqual("disk full", e.message)
|
||||
|
||||
def test_rpc_internal_error_preserves_cause(self):
|
||||
cause = OSError("no space left on device")
|
||||
try:
|
||||
raise _RpcInternalError("failed to write") from cause
|
||||
except _RpcInternalError as e:
|
||||
self.assertIs(cause, e.__cause__)
|
||||
|
||||
def test_parse_error_is_client_error(self):
|
||||
with self.assertRaises(_RpcClientError):
|
||||
parse_jsonrpc(b"{bad json")
|
||||
|
||||
def test_validation_error_is_client_error(self):
|
||||
with self.assertRaises(_RpcClientError):
|
||||
validate_proposed_file(_sv.TOOL_EGRESS_ALLOW, "routes: nope\n")
|
||||
|
||||
def test_unknown_tool_in_tools_call_is_client_error(self):
|
||||
config = ServerConfig(bottle_slug="dev", queue_dir=Path("/unused"))
|
||||
with self.assertRaises(_RpcClientError) as cm:
|
||||
handle_tools_call({"name": "no-such-tool", "arguments": {}}, config)
|
||||
self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code)
|
||||
|
||||
|
||||
class TestRpcInternalErrorOnIoFailure(unittest.TestCase):
|
||||
def test_write_proposal_os_error_raises_internal(self):
|
||||
config = ServerConfig(
|
||||
bottle_slug="dev",
|
||||
queue_dir=Path("/dev/null/cannot-exist"),
|
||||
)
|
||||
with self.assertRaises(_RpcInternalError) as cm:
|
||||
handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_EGRESS_ALLOW,
|
||||
"arguments": {
|
||||
"routes_yaml": "routes:\n - host: example.com\n",
|
||||
"justification": "x",
|
||||
},
|
||||
},
|
||||
config,
|
||||
)
|
||||
self.assertEqual(ERR_INTERNAL, cm.exception.code)
|
||||
self.assertIsNotNone(cm.exception.__cause__)
|
||||
|
||||
|
||||
# --- JSON-RPC parsing ------------------------------------------------------
|
||||
|
||||
|
||||
@@ -157,7 +219,6 @@ class TestHandleToolsList(unittest.TestCase):
|
||||
self.assertEqual(
|
||||
sorted([
|
||||
_sv.TOOL_EGRESS_ALLOW,
|
||||
_sv.TOOL_CAPABILITY_BLOCK,
|
||||
_sv.TOOL_EGRESS_BLOCK,
|
||||
_sv.TOOL_LIST_EGRESS_ROUTES,
|
||||
]),
|
||||
@@ -233,10 +294,10 @@ class TestHandleToolsCall(unittest.TestCase):
|
||||
try:
|
||||
result = handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||
"name": _sv.TOOL_EGRESS_BLOCK,
|
||||
"arguments": {
|
||||
"dockerfile": "FROM python:3.13\n",
|
||||
"justification": "need git",
|
||||
"routes_yaml": "routes:\n - host: example.com\n",
|
||||
"justification": "need example.com",
|
||||
},
|
||||
},
|
||||
self.config,
|
||||
@@ -273,9 +334,9 @@ class TestHandleToolsCall(unittest.TestCase):
|
||||
try:
|
||||
result = handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||
"name": _sv.TOOL_EGRESS_ALLOW,
|
||||
"arguments": {
|
||||
"dockerfile": "FROM python:3.13\n",
|
||||
"routes_yaml": "routes:\n - host: example.com\n",
|
||||
"justification": "needed for tests",
|
||||
},
|
||||
},
|
||||
@@ -297,20 +358,52 @@ class TestHandleToolsCall(unittest.TestCase):
|
||||
with self.assertRaises(_RpcError):
|
||||
handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||
"arguments": {"dockerfile": "FROM python:3.13\n"},
|
||||
"name": _sv.TOOL_EGRESS_ALLOW,
|
||||
"arguments": {"routes_yaml": "routes:\n - host: example.com\n"},
|
||||
},
|
||||
self.config,
|
||||
)
|
||||
|
||||
def test_missing_name_raises(self):
|
||||
with self.assertRaises(_RpcError) as cm:
|
||||
handle_tools_call({"arguments": {}}, self.config)
|
||||
self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code)
|
||||
|
||||
def test_arguments_must_be_object(self):
|
||||
with self.assertRaises(_RpcError) as cm:
|
||||
handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_EGRESS_ALLOW,
|
||||
"arguments": [],
|
||||
},
|
||||
self.config,
|
||||
)
|
||||
self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code)
|
||||
self.assertIn("must be an object", cm.exception.message)
|
||||
|
||||
def test_capability_block_call_raises_unknown_tool(self):
|
||||
with self.assertRaises(_RpcError) as cm:
|
||||
handle_tools_call(
|
||||
{
|
||||
"name": "capability-block",
|
||||
"arguments": {
|
||||
"dockerfile": "FROM python:3.13\n",
|
||||
"justification": "need git",
|
||||
},
|
||||
},
|
||||
self.config,
|
||||
)
|
||||
self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code)
|
||||
self.assertIn("unknown tool", cm.exception.message)
|
||||
|
||||
def test_archives_proposal_after_response(self):
|
||||
responder = self._respond_when_proposal_appears(_sv.STATUS_APPROVED)
|
||||
try:
|
||||
handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||
"name": _sv.TOOL_EGRESS_ALLOW,
|
||||
"arguments": {
|
||||
"dockerfile": "FROM python:3.13\n",
|
||||
"routes_yaml": "routes:\n - host: example.com\n",
|
||||
"justification": "x",
|
||||
},
|
||||
},
|
||||
@@ -332,10 +425,10 @@ class TestHandleToolsCall(unittest.TestCase):
|
||||
)
|
||||
result = handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||
"name": _sv.TOOL_EGRESS_ALLOW,
|
||||
"arguments": {
|
||||
"dockerfile": "FROM python:3.13\n",
|
||||
"justification": "need a capability",
|
||||
"routes_yaml": "routes:\n - host: example.com\n",
|
||||
"justification": "need egress",
|
||||
},
|
||||
},
|
||||
config,
|
||||
@@ -350,6 +443,31 @@ class TestHandleToolsCall(unittest.TestCase):
|
||||
|
||||
|
||||
class TestHandleListEgressRoutes(unittest.TestCase):
|
||||
def test_success_returns_body_text(self):
|
||||
class _Resp:
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, exc_type: type[BaseException] | None, exc: BaseException | None, tb: object) -> bool:
|
||||
return False
|
||||
|
||||
def read(self):
|
||||
return b"[{\"host\": \"example.com\"}]"
|
||||
|
||||
class _Opener:
|
||||
def open(self, *args, **kwargs): # noqa: ANN001, ANN002, ANN003 # type: ignore
|
||||
return _Resp()
|
||||
|
||||
with patch.object(supervise_server.urllib.request, "build_opener", return_value=_Opener()):
|
||||
result = handle_list_egress_routes(
|
||||
{},
|
||||
ServerConfig(bottle_slug="dev", queue_dir=Path("/unused")),
|
||||
)
|
||||
|
||||
self.assertFalse(result["isError"]) # type: ignore[index]
|
||||
text = result["content"][0]["text"] # type: ignore[index]
|
||||
self.assertIn("example.com", text)
|
||||
|
||||
def test_url_error_returns_tool_error(self):
|
||||
class _Opener:
|
||||
def open(self, *args, **kwargs): # noqa: ANN001, ANN002, ANN003 # type: ignore
|
||||
@@ -409,6 +527,13 @@ class TestFormatResponseText(unittest.TestCase):
|
||||
self.assertIn("the operator modified", text.lower())
|
||||
|
||||
|
||||
class TestFormatPendingResponseText(unittest.TestCase):
|
||||
def test_formats_timeout_message(self):
|
||||
text = supervise_server.format_pending_response_text(12.5)
|
||||
self.assertIn("status: pending", text)
|
||||
self.assertIn("12.5s", text)
|
||||
|
||||
|
||||
# --- End-to-end HTTP sanity ------------------------------------------------
|
||||
|
||||
|
||||
@@ -459,7 +584,7 @@ class TestHttpEndToEnd(unittest.TestCase):
|
||||
self.assertEqual("2.0", result["jsonrpc"])
|
||||
self.assertEqual(1, result["id"])
|
||||
names = [t["name"] for t in result["result"]["tools"]] # type: ignore[index]
|
||||
self.assertIn(_sv.TOOL_CAPABILITY_BLOCK, names)
|
||||
self.assertNotIn("capability-block", names)
|
||||
self.assertIn(_sv.TOOL_EGRESS_ALLOW, names)
|
||||
self.assertIn(_sv.TOOL_EGRESS_BLOCK, names)
|
||||
|
||||
@@ -469,6 +594,26 @@ class TestHttpEndToEnd(unittest.TestCase):
|
||||
)
|
||||
self.assertEqual(ERR_METHOD_NOT_FOUND, result["error"]["code"]) # type: ignore[index]
|
||||
|
||||
def test_internal_error_returns_err_internal_over_http(self):
|
||||
with patch.object(
|
||||
supervise_server._sv, "write_proposal",
|
||||
side_effect=OSError("disk full"),
|
||||
):
|
||||
result = self._post_jsonrpc({
|
||||
"jsonrpc": "2.0",
|
||||
"id": 99,
|
||||
"method": "tools/call",
|
||||
"params": {
|
||||
"name": _sv.TOOL_EGRESS_ALLOW,
|
||||
"arguments": {
|
||||
"routes_yaml": "routes:\n - host: example.com\n",
|
||||
"justification": "x",
|
||||
},
|
||||
},
|
||||
})
|
||||
self.assertIn("error", result)
|
||||
self.assertEqual(ERR_INTERNAL, result["error"]["code"]) # type: ignore[index]
|
||||
|
||||
def test_health_endpoint(self):
|
||||
conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=5)
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user