refactor(supervise): hash the proposed file inside Proposal.new; sha256_hex → root util
tracker-policy-pr / check-pr (pull_request) Successful in 16s
test / integration-docker (pull_request) Successful in 39s
test / unit (pull_request) Successful in 53s
lint / lint (push) Failing after 1m3s
test / integration-firecracker (pull_request) Successful in 3m35s
test / coverage (pull_request) Successful in 19s
test / publish-infra (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Successful in 16s
test / integration-docker (pull_request) Successful in 39s
test / unit (pull_request) Successful in 53s
lint / lint (push) Failing after 1m3s
test / integration-firecracker (pull_request) Successful in 3m35s
test / coverage (pull_request) Successful in 19s
test / publish-infra (pull_request) Has been skipped
Move sha256_hex out of orchestrator/supervisor/util.py to the root bot_bottle.util (it's a generic string hash, not supervise-specific), and have Proposal.new take the proposed file contents and compute current_file_hash itself instead of every caller passing sha256_hex(proposed_file). Every call site set current_file_hash to the hash of the proposed file, and nothing reads the field except the DB round-trip (schema + _row_to_proposal + from_dict, all untouched), so folding the hash into the factory is behavior-preserving and removes the boilerplate. Callers now pass just the file; the orchestrator no longer imports sha256_hex at all. Full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -41,7 +41,6 @@ from .supervisor import (
|
||||
TOOL_EGRESS_BLOCK,
|
||||
Supervisor,
|
||||
render_diff,
|
||||
sha256_hex,
|
||||
)
|
||||
|
||||
|
||||
@@ -216,7 +215,6 @@ class Orchestrator:
|
||||
tool=tool,
|
||||
proposed_file=proposed_file,
|
||||
justification=justification,
|
||||
current_file_hash=sha256_hex(proposed_file),
|
||||
)
|
||||
self._supervisor.write_proposal(proposal)
|
||||
return proposal.id
|
||||
|
||||
@@ -8,8 +8,8 @@ dependency explicit and injectable in tests.
|
||||
|
||||
For convenience this package also re-exports the neutral vocabulary
|
||||
(`bot_bottle.supervisor`'s types + `SupervisePlan`) and the pure `render_diff`
|
||||
/ `sha256_hex` helpers (`util`), so orchestrator-side callers import from one
|
||||
place. The data plane must NOT import this package — it imports
|
||||
helper (`util`), so orchestrator-side callers import from one place. The data
|
||||
plane must NOT import this package — it imports
|
||||
`bot_bottle.supervisor` (neutral) directly and reaches the queue over the
|
||||
control-plane RPC.
|
||||
"""
|
||||
@@ -24,7 +24,7 @@ from ..store.store_manager import StoreManager
|
||||
from ..store.queue_store import QueueStore
|
||||
from ...store.audit_store import AuditStore
|
||||
from ...supervisor.types import AuditEntry, Proposal, Response
|
||||
from .util import render_diff, sha256_hex # noqa: F401 — re-exported for callers
|
||||
from .util import render_diff # noqa: F401 — re-exported for callers
|
||||
|
||||
|
||||
class Supervisor:
|
||||
|
||||
@@ -1,14 +1,13 @@
|
||||
"""Pure supervise helpers — diff rendering and content hashing.
|
||||
"""Pure supervise helpers — diff rendering.
|
||||
|
||||
Stateless utilities used alongside the `Supervisor` service. Kept as plain
|
||||
functions (not methods) because they carry no state and are also handy to
|
||||
callers that only need to hash or diff.
|
||||
Stateless utility used alongside the `Supervisor` service. Kept as a plain
|
||||
function (not a method) because it carries no state. (Content hashing is a
|
||||
generic helper — `bot_bottle.util.sha256_hex`.)
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import difflib
|
||||
import hashlib
|
||||
|
||||
|
||||
def render_diff(before: str, after: str, *, label: str = "config") -> str:
|
||||
@@ -25,7 +24,3 @@ def render_diff(before: str, after: str, *, label: str = "config") -> str:
|
||||
if not parts:
|
||||
return ""
|
||||
return "".join(p if p.endswith("\n") else p + "\n" for p in parts).rstrip("\n")
|
||||
|
||||
|
||||
def sha256_hex(content: str) -> str:
|
||||
return hashlib.sha256(content.encode("utf-8")).hexdigest()
|
||||
|
||||
@@ -15,6 +15,8 @@ import uuid
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from ..util import sha256_hex
|
||||
|
||||
TOOL_EGRESS_BLOCK = "egress-block"
|
||||
TOOL_EGRESS_ALLOW = "egress-allow"
|
||||
TOOL_GITLEAKS_ALLOW = "gitleaks-allow"
|
||||
@@ -94,7 +96,6 @@ class Proposal:
|
||||
tool: str,
|
||||
proposed_file: str,
|
||||
justification: str,
|
||||
current_file_hash: str,
|
||||
now: datetime | None = None,
|
||||
) -> "Proposal":
|
||||
ts = (now or datetime.now(timezone.utc)).isoformat()
|
||||
@@ -105,7 +106,7 @@ class Proposal:
|
||||
proposed_file=proposed_file,
|
||||
justification=justification,
|
||||
arrival_timestamp=ts,
|
||||
current_file_hash=current_file_hash,
|
||||
current_file_hash=sha256_hex(proposed_file),
|
||||
)
|
||||
|
||||
def to_dict(self) -> dict[str, object]:
|
||||
|
||||
@@ -5,11 +5,17 @@ level deeper, under their backend package."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import ipaddress
|
||||
import os
|
||||
import sys
|
||||
|
||||
|
||||
def sha256_hex(content: str) -> str:
|
||||
"""Hex SHA-256 of a UTF-8 string."""
|
||||
return hashlib.sha256(content.encode("utf-8")).hexdigest()
|
||||
|
||||
|
||||
def is_ip_literal(value: str) -> bool:
|
||||
try:
|
||||
ipaddress.ip_address(value)
|
||||
|
||||
@@ -28,7 +28,6 @@ from bot_bottle.orchestrator.store.store_manager import StoreManager
|
||||
from bot_bottle.orchestrator.supervisor import (
|
||||
Proposal,
|
||||
TOOL_EGRESS_ALLOW,
|
||||
sha256_hex,
|
||||
Supervisor,
|
||||
)
|
||||
|
||||
@@ -420,7 +419,7 @@ class TestDispatchSupervise(unittest.TestCase):
|
||||
"10.243.0.1", metadata=json.dumps({"slug": slug}), policy="routes: []\n")
|
||||
p = Proposal.new(
|
||||
bottle_slug=slug, tool=TOOL_EGRESS_ALLOW, proposed_file=proposed,
|
||||
justification="need it", current_file_hash=sha256_hex(proposed))
|
||||
justification="need it")
|
||||
Supervisor().write_proposal(p)
|
||||
return p.id
|
||||
|
||||
|
||||
@@ -20,7 +20,6 @@ from bot_bottle.orchestrator.supervisor import (
|
||||
Proposal,
|
||||
STATUS_APPROVED,
|
||||
TOOL_EGRESS_ALLOW,
|
||||
sha256_hex,
|
||||
Supervisor,
|
||||
)
|
||||
|
||||
@@ -183,7 +182,7 @@ class TestOrchestratorSupervise(unittest.TestCase):
|
||||
def _queue(self, slug: str, proposed: str) -> str:
|
||||
p = Proposal.new(
|
||||
bottle_slug=slug, tool=TOOL_EGRESS_ALLOW, proposed_file=proposed,
|
||||
justification="need it", current_file_hash=sha256_hex(proposed))
|
||||
justification="need it")
|
||||
Supervisor().write_proposal(p)
|
||||
return p.id
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ from pathlib import Path
|
||||
|
||||
from bot_bottle.orchestrator import supervisor as supervise
|
||||
from bot_bottle.paths import host_db_path
|
||||
from bot_bottle.util import sha256_hex
|
||||
from tests.unit import use_bottle_root
|
||||
from bot_bottle.store.audit_store import AuditStore
|
||||
from bot_bottle.orchestrator.store.queue_store import QueueStore
|
||||
@@ -21,7 +22,6 @@ from bot_bottle.orchestrator.supervisor import (
|
||||
TOOL_EGRESS_ALLOW,
|
||||
TOOL_GITLEAKS_ALLOW,
|
||||
render_diff,
|
||||
sha256_hex,
|
||||
)
|
||||
|
||||
_SV = Supervisor()
|
||||
@@ -40,7 +40,6 @@ def _proposal(
|
||||
tool=tool,
|
||||
proposed_file=proposed,
|
||||
justification=justification,
|
||||
current_file_hash=sha256_hex(proposed),
|
||||
now=FIXED_TS,
|
||||
)
|
||||
|
||||
@@ -145,13 +144,11 @@ class TestQueueIO(unittest.TestCase):
|
||||
a = Proposal.new(
|
||||
bottle_slug="dev", tool=TOOL_EGRESS_ALLOW,
|
||||
proposed_file="routes:\n - host: early.example.com\n", justification="early",
|
||||
current_file_hash="x",
|
||||
now=datetime(2026, 5, 25, 10, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
b = Proposal.new(
|
||||
bottle_slug="dev", tool=TOOL_EGRESS_ALLOW,
|
||||
proposed_file="routes:\n - host: late.example.com\n", justification="late",
|
||||
current_file_hash="x",
|
||||
now=datetime(2026, 5, 25, 14, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
# Write in reverse order.
|
||||
@@ -288,7 +285,6 @@ class TestToolConstants(unittest.TestCase):
|
||||
tool=supervise.TOOL_EGRESS_TOKEN_ALLOW,
|
||||
proposed_file="host: api.example.com\n",
|
||||
justification="false positive",
|
||||
current_file_hash="h",
|
||||
)
|
||||
self.assertEqual(p, Proposal.from_dict(p.to_dict()))
|
||||
|
||||
|
||||
@@ -19,7 +19,6 @@ from bot_bottle.orchestrator.supervisor import (
|
||||
TOOL_EGRESS_BLOCK,
|
||||
TOOL_GITLEAKS_ALLOW,
|
||||
TOOL_EGRESS_TOKEN_ALLOW,
|
||||
sha256_hex,
|
||||
)
|
||||
|
||||
|
||||
@@ -37,7 +36,7 @@ def _proposal(slug: str = "dev", tool: str = TOOL_EGRESS_ALLOW,
|
||||
payload = payloads.get(tool, "")
|
||||
return Proposal.new(
|
||||
bottle_slug=slug, tool=tool, proposed_file=payload,
|
||||
justification=f"needed for {slug}", current_file_hash=sha256_hex(payload),
|
||||
justification=f"needed for {slug}",
|
||||
now=now,
|
||||
)
|
||||
|
||||
|
||||
@@ -30,7 +30,6 @@ def _proposal() -> Proposal:
|
||||
tool=TOOL_EGRESS_ALLOW,
|
||||
proposed_file="x",
|
||||
justification="j",
|
||||
current_file_hash="h",
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -92,7 +92,7 @@ class _FakeSuperviseResolver:
|
||||
return None
|
||||
proposal = _sv.Proposal.new(
|
||||
bottle_slug=self.bottle_id, tool=tool, proposed_file=proposed_file,
|
||||
justification=justification, current_file_hash=_sv.sha256_hex(proposed_file),
|
||||
justification=justification,
|
||||
)
|
||||
_SV.write_proposal(proposal)
|
||||
return proposal.id
|
||||
@@ -750,7 +750,6 @@ class TestNonBlockingSupervise(unittest.TestCase):
|
||||
tool=_sv.TOOL_EGRESS_ALLOW,
|
||||
proposed_file=self._ROUTES,
|
||||
justification="need example.com",
|
||||
current_file_hash=_sv.sha256_hex(self._ROUTES),
|
||||
)
|
||||
_SV.write_proposal(p)
|
||||
return p
|
||||
|
||||
Reference in New Issue
Block a user