`` and write the authored manifest into the copy. Returns
``{id, type, description, path}``. Raises ``ValueError`` on a bad source / invalid manifest / id
@@ -245,7 +248,7 @@ def add_with_manifest(source: str, tools_root: str, existing_ids, manifest: dict
if src.resolve() == root.resolve() or root.resolve() in src.resolve().parents:
raise ValueError("source folder is inside the tools dir; choose a folder outside it")
tool = tool_authoring.normalize(manifest)
- errors = list(tool_authoring.validate(tool)) # also bounds the id to a safe single path component
+ errors = list(tool_authoring.validate(tool, confirm_secret_items=confirm_secret_items))
ep = tool_authoring.entrypoint_error(tool, src)
if ep:
errors.append(ep)
@@ -255,7 +258,7 @@ def add_with_manifest(source: str, tools_root: str, existing_ids, manifest: dict
if tool_id in set(existing_ids):
raise ValueError(f"a tool named '{tool_id}' already exists")
dest = _copy_tool_dir(src, root, tool_id)
- tool_authoring.write(dest, tool) # write the authored toolyard.toml into the copied code
+ tool_authoring.write(dest, tool, confirm_secret_items=confirm_secret_items)
td = load_tool(dest / "toolyard.toml")
return {"id": td.id, "type": td.type, "description": td.description, "path": str(dest)}
diff --git a/admin/toolyard_ops.py b/admin/toolyard_ops.py
index 17393b0..4ce68b3 100644
--- a/admin/toolyard_ops.py
+++ b/admin/toolyard_ops.py
@@ -14,13 +14,13 @@
from __future__ import annotations
import shutil
-from dataclasses import asdict
+from dataclasses import asdict, replace
from pathlib import Path
# Reuse the toolyard's single state file so the panel and the CLI never disagree
# about which tools are running.
from toolyard.cli import _load_state, _save_state
-from toolyard.config import discover
+from toolyard.config import discover, SecretSpec
from toolyard.config import load as load_tool
from toolyard.runner import RunningTool, RunnerStartCleanupError, get_runner
@@ -95,12 +95,13 @@ def list_tools(tools_root: str, tool_dirs=()) -> list[dict]:
"state": "unknown" if alive is None else "running" if alive else "crashed" if record else "stopped",
"health": "unknown" if alive or alive is None else "stopped",
"backend": record["backend"] if record else None,
+ "secrets": [asdict(s) for s in d.secrets],
"removable": str(Path(d.path).resolve()) in dir_set,
})
return tools
-def _start(tool_id: str, tools_root: str, tool_dirs, backend: str = "process") -> None:
+def _start(tool_id: str, tools_root: str, tool_dirs, backend: str = "process", *, recovery=False) -> None:
"""Start the tool via the runner. Secrets are resolved by the tool at
boot (Phase 5: SPS pull), so the panel hands the runner nothing but
the tool definition."""
@@ -108,6 +109,13 @@ def _start(tool_id: str, tools_root: str, tool_dirs, backend: str = "process") -
if tool_id not in defs:
raise LookupError(f"unknown tool: {tool_id}")
state = _load_state()
+ if recovery:
+ rec = state.get(tool_id, {})
+ if rec.get("secret_declarations") is None or not rec.get("manifest_digest"):
+ raise ValueError("recorded declarations missing; operator Start required")
+ defs[tool_id] = replace(defs[tool_id], secrets=tuple(
+ SecretSpec(**s) for s in rec["secret_declarations"]),
+ manifest_digest=rec["manifest_digest"])
if tool_id in state:
# The state file survives the process, VM, and host. Treat it as running
# intent, not proof that the recorded runtime still exists.
@@ -157,9 +165,22 @@ def restart(tool_id: str, tools_root: str, tool_dirs, backend: str = "process")
_start(tool_id, tools_root, tool_dirs, backend)
-def start(tool_id: str, tools_root: str, tool_dirs, backend: str = "process") -> None:
+def start(tool_id: str, tools_root: str, tool_dirs, backend: str = "process", *, recovery=False) -> None:
with state_lock():
- _start(tool_id, tools_root, tool_dirs, backend)
+ _start(tool_id, tools_root, tool_dirs, backend, recovery=recovery)
+
+
+def record_edited_declarations(tool):
+ """An explicit operator edit may change the next recovery registration."""
+ import hashlib
+ from .tool_authoring import to_toml
+ digest = hashlib.sha256(to_toml(tool).encode("utf-8")).hexdigest()
+ with state_lock():
+ state = _load_state()
+ if tool["id"] in state:
+ state[tool["id"]]["secret_declarations"] = [dict(s) for s in tool["secrets"]]
+ state[tool["id"]]["manifest_digest"] = digest
+ _save_state(state)
def stop(tool_id: str) -> None:
diff --git a/admin/views/tools.py b/admin/views/tools.py
index fbd0e59..6af784f 100644
--- a/admin/views/tools.py
+++ b/admin/views/tools.py
@@ -28,8 +28,11 @@ def _secret_backend_note(sps: dict | None) -> str:
def tools_view(*, user, csrf, tools, tools_root, banner=None, error=None, catalog=None, broker_port=None, focused=False, mutation_outcome=None) -> str:
def action(tool_id, act, label, disabled, primary=False):
d = " disabled" if disabled else ""
- confirmation = (f" data-confirm='{esc('Restart ' + tool_id + '? Stops then starts trusted tool code; interrupts active calls. Requests are not replayed.')}'"
- if act == "restart" else "")
+ warning = " This tool runs with this host's credentials. Only start tools you trust."
+ if act == "restart":
+ warning += " Stops then starts tool code; interrupts active calls. Requests are not replayed."
+ confirmation = (f" data-confirm='{esc(('Restart ' if act == 'restart' else 'Start ') + tool_id + '?' + warning)}'"
+ if act in ("start", "restart") else "")
return (f"")
@@ -80,6 +83,10 @@ def action(tool_id, act, label, disabled, primary=False):
f"| {esc(state)} {registration} | "
f"{esc(t.get('path', ''))} | "
""
+ + " Secrets: " + (", ".join(
+ esc(s["name"]) + " (item: " + esc(s.get("item") or t["id"]) + ")"
+ for s in t.get("secrets", [])) or "none") + " "
+ +
f"Edit"
f"{action(t['id'], 'start', 'Start', alive, focused and not running)}"
f"{action(t['id'], 'stop', 'Stop', not running)}"
@@ -125,6 +132,8 @@ def tool_add_view(*, user, csrf, source_value="", repo_value="", error=None) ->
""
""
+ ""
""
"From a Git repository"
@@ -136,6 +145,8 @@ def tool_add_view(*, user, csrf, source_value="", repo_value="", error=None) ->
""
""
+ ""
""
"Cloned code is copied in, never executed by the panel. Restart the broker to "
"register the new tool. "
@@ -211,6 +222,8 @@ def tool_editor_view(*, user, csrf, mode, tool, dir_value="", backend=None, erro
"Path (Infisical)"
"Writable"
""
+ ""
""
""
diff --git a/docs/secret-declarations.md b/docs/secret-declarations.md
new file mode 100644
index 0000000..4ddb8a6
--- /dev/null
+++ b/docs/secret-declarations.md
@@ -0,0 +1,31 @@
+# Operator-approved secret declarations
+
+The tools page shows secret names and backend items beside Start, never values.
+Start and Restart ask for confirmation: a started tool runs with this host's
+credentials, so only start tools you trust. Cancel is the default web action.
+
+Source updates always retain the local secret declarations, including an empty
+list. The update result includes `secret_declarations_changed` and a
+`secret_declarations` object containing `kept` and `upstream` lists. The web
+update banner warns when they differ. Review and save an explicit Edit to adopt
+changes; upstream declarations never become grants merely by updating code.
+
+Authoring and import reject a nonempty secret `item` different from the tool id
+unless the operator checks the explicit confirmation. JSON clients send
+`confirm_secret_items: true` alongside the request, outside the manifest.
+An empty item retains the default of the tool id.
+
+Each process Start records declarations and the SHA-256 manifest digest in
+private toolyard state. SPS recovery and automatic process reconciliation use
+that snapshot even if the manifest changes on disk. An explicit operator Edit
+updates the recorded declarations; Start/Restart adopts the current manifest.
+Older running records without snapshots require an operator stop/start rather
+than having grants inferred from disk during recovery.
+
+Admin and runner read only `SP_HOST`, `SP_PORT`, `SP_TLS_CA`, and `SP_SECRET`
+from the top-level SPS TOML env file, which must remain a regular mode-0600 file.
+Provider tables are not decoded. A registration-only env file can therefore be
+used via `TOOLSTACK_SPS_ENV`; provider configuration remains SPS's responsibility.
+TLS verification is enabled by default. `TOOLSTACK_SPS_VERIFY=0` is refused
+unless `TOOLSTACK_SPS_DEV=1` is also set explicitly for trusted development.
+This does not change `TOOLSTACK_SPS_SKIP=1`, which skips SPS entirely.
diff --git a/toolyard/config.py b/toolyard/config.py
index 655eb42..67e4d4a 100644
--- a/toolyard/config.py
+++ b/toolyard/config.py
@@ -8,6 +8,7 @@
from __future__ import annotations
import re
+import hashlib
import tomllib
from dataclasses import dataclass
from pathlib import Path
@@ -46,12 +47,14 @@ class ToolDef:
path: Path # directory containing toolyard.toml (and the tool's files)
description: str = "" # optional tool-level summary (the broker registry ignores it)
egress: tuple[str, ...] = () # [sandbox] egress: hosts a sandboxed tool may reach outbound
+ manifest_digest: str | None = None
def load(toml_path: str | Path) -> ToolDef:
path = Path(toml_path)
with open(path, "rb") as f:
- data = tomllib.load(f)
+ raw = f.read()
+ data = tomllib.loads(raw.decode("utf-8"))
tool_id = data.get("id")
# Reject a missing/invalid id at load, naming the file + id; mirrors the broker registry's
# check (broker/registry.py). A dotted id would slip through here and only misroute policy
@@ -107,6 +110,7 @@ def load(toml_path: str | Path) -> ToolDef:
image=entry.get("image"),
secrets=secrets,
path=path.parent,
+ manifest_digest=hashlib.sha256(raw).hexdigest(),
description=data.get("description", ""),
egress=tuple(egress),
)
diff --git a/toolyard/runner.py b/toolyard/runner.py
index 0a3b9e7..27a04dc 100644
--- a/toolyard/runner.py
+++ b/toolyard/runner.py
@@ -21,7 +21,7 @@
import subprocess
import sys
import time
-from dataclasses import dataclass, field, replace
+from dataclasses import asdict, dataclass, field, replace
from pathlib import Path
from typing import Protocol
@@ -186,21 +186,11 @@ def _reregister_results(*, tools_root: str, tool_dirs, sps_env_path: str) -> dic
log.warning("SPS re-register: sps.env not found at %s; skipping", sps_env_path)
raise RunnerConfigError("SPS configuration missing; recovery must retry")
from .cli import _load_state
- from .config import discover, load as load_tool
+ from types import SimpleNamespace
+ from .config import SecretSpec
state = _load_state()
if not state:
return {}
- defs: dict[str, ToolDef] = {}
- if tools_root:
- for d in discover(tools_root):
- defs[d.id] = d
- for path in tool_dirs or ():
- toml_path = Path(path) / "toolyard.toml"
- if toml_path.exists():
- try:
- defs[load_tool(toml_path).id] = load_tool(toml_path)
- except Exception as exc:
- log.warning("SPS re-register: cannot load %s: %s", toml_path, exc)
results = {}
for tool_id, rec in state.items():
if not isinstance(rec, dict):
@@ -219,12 +209,13 @@ def _reregister_results(*, tools_root: str, tool_dirs, sps_env_path: str) -> dic
alive = True
if not alive:
continue # positively dead; reconcile restarts it
- tool_def = defs.get(tool_id)
- if tool_def is None:
- log.warning("SPS re-register: tool %s has e_secret but no tool_def", tool_id)
- results[tool_id] = "tool definition missing"
+ if rec.get("secret_declarations") is None or not rec.get("manifest_digest"):
+ log.warning("SPS re-register: tool %s needs an operator Start to record declarations", tool_id)
+ results[tool_id] = "recorded declarations missing; operator Start required"
continue
try:
+ tool_def = SimpleNamespace(id=tool_id, secrets=tuple(
+ SecretSpec(**s) for s in rec["secret_declarations"]))
_sps_register(tool_def, e_secret, sps_env_path)
results[tool_id] = None
log.info("SPS re-registered %s", tool_id)
@@ -246,9 +237,9 @@ class RunnerConfigError(ValueError):
def strict_bool(value: str) -> bool:
- from sps.config import parse_bool
+ from .sps_config import parse_verify
try:
- return parse_bool(value, name="TOOLSTACK_SPS_VERIFY")
+ return parse_verify(value)
except ValueError as exc:
raise RunnerConfigError(str(exc)) from exc
@@ -263,7 +254,7 @@ class ResolvedSPS:
def _resolve_sps(path):
- from sps.config import load_config
+ from .sps_config import load_registration_config as load_config
try:
cfg = load_config(path)
return ResolvedSPS(os.environ.get("TOOLSTACK_SPS_HOST", cfg.sp_host),
@@ -276,7 +267,7 @@ def _resolve_sps(path):
def _check_sps_env(path: str) -> str:
"""Compatibility validation entry point; start resolves/validates once instead."""
- from sps.config import ConfigModeError, load_config
+ from .sps_config import ConfigModeError, load_registration_config as load_config
try:
load_config(path)
except ConfigModeError as exc:
@@ -311,7 +302,6 @@ def _sps_unregister(tool_id: str, sps_env_path: str) -> None:
"""Best-effort: a failed unregister should not turn a clean stop into a
failure. Logged as a warning, never raised to the caller."""
from sps.client import SPSClient
- from sps.config import load_config
try:
cfg = _resolve_sps(sps_env_path)
except Exception as exc:
@@ -467,6 +457,8 @@ class RunningTool:
process_exited: bool = False
log_exited: bool = False
egress_exited: bool = False
+ secret_declarations: list[dict] | None = None
+ manifest_digest: str | None = None
class RunnerStartCleanupError(RuntimeError):
@@ -619,7 +611,9 @@ def start(self, tool_def: ToolDef) -> RunningTool:
log_path = _tool_log_path(tool_def)
running = RunningTool(tool_def.id, tool_def.port, self.backend, "",
log_path=str(log_path), boot_id=_boot_id(),
- e_secret=e_secret, channel_secret=channel_secret)
+ e_secret=e_secret, channel_secret=channel_secret,
+ secret_declarations=[asdict(s) for s in tool_def.secrets],
+ manifest_digest=tool_def.manifest_digest)
def mark_reaped():
nonlocal running
diff --git a/toolyard/sps_config.py b/toolyard/sps_config.py
new file mode 100644
index 0000000..a0c278d
--- /dev/null
+++ b/toolyard/sps_config.py
@@ -0,0 +1,95 @@
+"""Read only the registration client's top-level SPS settings."""
+from __future__ import annotations
+
+import os
+import stat
+import tomllib
+from dataclasses import dataclass, field
+
+
+class ConfigModeError(ValueError):
+ pass
+
+
+@dataclass(frozen=True)
+class RegistrationConfig:
+ sp_host: str
+ sp_port: int
+ sp_secret: str = field(repr=False)
+ sp_tls_ca: str = ""
+
+
+def _statements(stream):
+ """Frame TOML statements without decoding values we do not need."""
+ buf = ""
+ quote = None
+ depth = 0
+ for line in stream:
+ buf += line
+ i = 0
+ while i < len(line):
+ c = line[i]
+ if quote:
+ if quote.startswith('"') and c == "\\":
+ i += 2
+ continue
+ if line.startswith(quote, i):
+ i += len(quote)
+ quote = None
+ continue
+ elif c == "#":
+ break
+ elif c in "\"'":
+ quote = c * 3 if line.startswith(c * 3, i) else c
+ i += len(quote)
+ continue
+ elif c in "[{":
+ depth += 1
+ elif c in "]}":
+ depth -= 1
+ i += 1
+ if quote is None and depth == 0:
+ yield buf
+ buf = ""
+ if buf.strip():
+ raise ValueError("incomplete SPS configuration statement")
+
+
+def load_registration_config(path):
+ allowed = {"SP_HOST", "SP_PORT", "SP_TLS_CA", "SP_SECRET"}
+ data = {}
+ with open(path, "r", encoding="utf-8") as stream:
+ st = os.fstat(stream.fileno())
+ if not stat.S_ISREG(st.st_mode) or stat.S_IMODE(st.st_mode) != 0o600:
+ raise ConfigModeError("SPS configuration must be a regular mode 0600 file")
+ for statement in _statements(stream):
+ text = statement.strip()
+ if text.startswith("["):
+ break
+ if not text or text.startswith("#"):
+ continue
+ key = text.split("=", 1)[0].strip().strip("\"'")
+ if key not in allowed:
+ continue
+ try:
+ value = tomllib.loads(text)
+ except ValueError:
+ raise ValueError("invalid SPS registration setting") from None
+ if set(value) != {key} or key in data:
+ raise ValueError("invalid or duplicate SPS registration setting")
+ data.update(value)
+ for key in ("SP_HOST", "SP_PORT", "SP_SECRET"):
+ if not isinstance(data.get(key), str) or not data[key]:
+ raise ValueError(f"missing SPS registration setting {key}")
+ if not isinstance(data.get("SP_TLS_CA", ""), str):
+ raise ValueError("invalid SP_TLS_CA")
+ return RegistrationConfig(data["SP_HOST"], int(data["SP_PORT"]),
+ data["SP_SECRET"], data.get("SP_TLS_CA", ""))
+
+
+def parse_verify(value):
+ from sps.config import parse_bool
+ verify = parse_bool(value, name="TOOLSTACK_SPS_VERIFY")
+ if not verify and os.environ.get("TOOLSTACK_SPS_DEV") != "1":
+ raise ValueError("TOOLSTACK_SPS_VERIFY=0 requires TOOLSTACK_SPS_DEV=1")
+ return verify
diff --git a/toolyard/tests/test_partial_cleanup.py b/toolyard/tests/test_partial_cleanup.py
index 62d3a64..71ed33c 100644
--- a/toolyard/tests/test_partial_cleanup.py
+++ b/toolyard/tests/test_partial_cleanup.py
@@ -132,7 +132,7 @@ def test_positive_receipt_skips_reused_pid_on_retry(self):
def start_fixture(self):
directory = self.stack.enter_context(tempfile.TemporaryDirectory())
- tool = SimpleNamespace(id="fake", port=123, path=Path(directory).resolve(), command="fake", secrets=[])
+ tool = SimpleNamespace(id="fake", port=123, path=Path(directory).resolve(), command="fake", secrets=[], manifest_digest="recorded")
sps_env = tool.path / "fake.env"
sps_env.touch()
self.stack.enter_context(patch.dict(os.environ, {"TOOLSTACK_SPS_SKIP": "0", "TOOLSTACK_SPS_ENV": str(sps_env)}))
diff --git a/toolyard/tests/test_registration_config.py b/toolyard/tests/test_registration_config.py
new file mode 100644
index 0000000..55f7f8a
--- /dev/null
+++ b/toolyard/tests/test_registration_config.py
@@ -0,0 +1,57 @@
+import os
+from pathlib import Path
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from toolyard import runner, sps_config
+from admin.sps_link import SPSLink
+
+
+class RegistrationConfiguration(unittest.TestCase):
+ def setUp(self):
+ self.tmp = tempfile.TemporaryDirectory()
+ self.addCleanup(self.tmp.cleanup)
+ self.path = Path(self.tmp.name) / "sps.env"
+ self.path.write_text('SP_HOST="localhost"\nSP_PORT="8743"\nSP_SECRET="fixture"\n'
+ 'SP_TLS_CA="ca.pem"\nSP_PLUGIN="unused"\n'
+ '[vault]\nTOKEN="provider-fixture"\n')
+ self.path.chmod(0o600)
+
+ def test_only_registration_values_are_decoded(self):
+ original = sps_config.tomllib.loads
+ with patch.object(sps_config.tomllib, "loads", wraps=original) as decode:
+ cfg = sps_config.load_registration_config(self.path)
+ self.assertEqual(cfg.sp_host, "localhost")
+ self.assertEqual(cfg.sp_port, 8743)
+ self.assertNotIn("fixture", repr(cfg))
+ parsed = "\n".join(call.args[0] for call in decode.call_args_list)
+ self.assertNotIn("provider-fixture", parsed)
+ self.assertNotIn("SP_PLUGIN", parsed)
+
+ def test_ignored_multiline_value_cannot_spoof_registration(self):
+ self.path.write_text("IGNORED='''\nSP_HOST=\"wrong\"\n'''\n"
+ 'SP_HOST="right"\nSP_PORT="8743"\nSP_SECRET="fixture"\n')
+ self.assertEqual(sps_config.load_registration_config(self.path).sp_host, "right")
+
+ def test_quoted_keys_comments_and_multiline_strings(self):
+ self.path.write_text('# comment\n"SP_HOST" = "localhost" # comment\n'
+ "SP_PORT='8743'\nSP_SECRET='''fixture\nvalue'''\n")
+ self.assertEqual(sps_config.load_registration_config(self.path).sp_secret, "fixture\nvalue")
+
+ def test_private_mode_required(self):
+ self.path.chmod(0o644)
+ with self.assertRaises(sps_config.ConfigModeError):
+ sps_config.load_registration_config(self.path)
+
+ def test_both_clients_use_registration_loader_and_dev_gate(self):
+ with patch.dict(os.environ, {}, clear=True), patch("sps.config.load_config", side_effect=AssertionError):
+ self.assertTrue(runner._resolve_sps(str(self.path)).verify)
+ self.assertTrue(SPSLink.from_env(str(self.path)).verify)
+ with patch.dict(os.environ, {"TOOLSTACK_SPS_VERIFY": "0"}, clear=True):
+ with self.assertRaises(runner.RunnerConfigError):
+ runner._resolve_sps(str(self.path))
+ self.assertIsNone(SPSLink.from_env(str(self.path)))
+ with patch.dict(os.environ, {"TOOLSTACK_SPS_VERIFY": "0", "TOOLSTACK_SPS_DEV": "1"}, clear=True):
+ self.assertFalse(runner._resolve_sps(str(self.path)).verify)
+ self.assertFalse(SPSLink.from_env(str(self.path)).verify)
diff --git a/toolyard/tests/test_runner.py b/toolyard/tests/test_runner.py
index e9f6c89..a28a429 100644
--- a/toolyard/tests/test_runner.py
+++ b/toolyard/tests/test_runner.py
@@ -1055,7 +1055,8 @@ def setUp(self):
'id = "echo"\ntype = "api"\n[entrypoint]\ncommand = "true"\nport = 4601\n',
encoding="utf-8")
_save_state({"echo": asdict(RunningTool("echo", 4601, "process", "123",
- e_secret="e" * 32, boot_id="b"))})
+ e_secret="e" * 32, boot_id="b",
+ secret_declarations=[], manifest_digest="recorded"))})
self.env = Path(self.tmp) / "sps.env"
self.env.write_text('SP_HOST="h"\nSP_PORT="1"\nSP_SECRET="s"\n'
'SP_PLUGIN="localfile"\n', encoding="utf-8")
diff --git a/toolyard/tests/test_uplift.py b/toolyard/tests/test_uplift.py
index 7de1926..f9a9ac0 100644
--- a/toolyard/tests/test_uplift.py
+++ b/toolyard/tests/test_uplift.py
@@ -114,7 +114,7 @@ def test_child_environment_drops_master_and_provider_credentials(self):
def test_resolved_verify_normalized_once(self):
cfg = SimpleNamespace(sp_host="fake", sp_port=123, sp_secret="fake", sp_tls_ca="fake-ca")
- with patch("sps.config.load_config", return_value=cfg), patch.dict(os.environ, {"TOOLSTACK_SPS_VERIFY": "YeS"}, clear=True):
+ with patch("toolyard.sps_config.load_registration_config", return_value=cfg), patch.dict(os.environ, {"TOOLSTACK_SPS_VERIFY": "YeS"}, clear=True):
resolved = runner._resolve_sps("fake.env")
self.assertIs(resolved.verify, True)
with patch.dict(os.environ, {"TOOLSTACK_SPS_VERIFY": "invalid"}, clear=True), patch("sps.client.SPSClient") as client:
@@ -194,7 +194,8 @@ def test_strict_bool(self):
for value in ["1", "TRUE", "yes"]:
self.assertTrue(runner.strict_bool(value))
for value in ["0", "false", "NO"]:
- self.assertFalse(runner.strict_bool(value))
+ with patch.dict(os.environ, {"TOOLSTACK_SPS_DEV": "1"}):
+ self.assertFalse(runner.strict_bool(value))
with self.assertRaises(ValueError):
runner.strict_bool("typo")
@@ -250,7 +251,7 @@ def test_unrecordable_start_rolled_back(self):
def test_sps_resolution_reused_and_channel_independent(self):
cfg = SimpleNamespace(sp_host="configured.invalid", sp_port=4444, sp_tls_ca="fake-ca", verify=False)
- tool = SimpleNamespace(id="fake", port=123, path=Path("."), command="fake", secrets=[])
+ tool = SimpleNamespace(id="fake", port=123, path=Path("."), command="fake", secrets=[], manifest_digest="recorded")
with patch.object(runner, "_check_port_free"), patch.object(runner.os.path, "exists", return_value=True), patch.object(runner, "_resolve_sps", return_value=cfg) as resolve, patch.object(runner, "_sps_register") as register, patch.object(runner, "_tool_log_path", return_value=Path(os.devnull)), patch.object(runner.os, "fchmod"), patch.object(runner.os, "posix_spawn", return_value=999) as spawn, patch.object(runner, "_process_identity", return_value="start"), patch.object(runner.ProcessRunner, "is_alive", return_value=True), patch.object(runner.time, "sleep"), patch.dict(os.environ, {"TOOLSTACK_SPS_SKIP": "0", "TOOLSTACK_SPS_ENV": "fake.env"}):
record = runner.ProcessRunner().start(tool)
resolve.assert_called_once_with("fake.env")
From eb3fe3f3a01cfe908f3f9b524f1d865d2befc8bb Mon Sep 17 00:00:00 2001
From: batteryshark <1574871+batteryshark@users.noreply.github.com>
Date: Sun, 25 Oct 2026 15:01:58 -0400
Subject: [PATCH 3/4] Harden request handling, operator sessions, and private
runtime files
---
admin/README.md | 11 +++
admin/api.py | 10 ++-
admin/auth.py | 51 +++++++++++++-
admin/broker_config.py | 4 +-
admin/server.py | 18 +++++
admin/settings.py | 11 ++-
admin/supervisor.py | 4 +-
admin/tests/test_api.py | 38 +++++++++-
admin/tests/test_app.py | 2 +-
admin/tests/test_auth.py | 30 +++++++-
admin/tests/test_dialog_logout.py | 2 +
admin/tests/test_private_writes.py | 40 +++++++++++
admin/views/assets.py | 2 +-
broker/gateway.py | 12 +++-
broker/mcp.py | 3 +-
broker/ratelimit.py | 2 +-
broker/request_lifecycle.py | 4 +-
broker/server.py | 7 ++
broker/tests/test_request_guards.py | 69 +++++++++++++++++++
client/tests/test_proxy_policy.py | 14 ++++
client/toolstack.py | 2 +-
deploy/docker/README.md | 4 ++
deploy/install.sh | 2 +-
macapp/Sources/ToolstackApp/AppModel.swift | 9 ++-
.../Sources/ToolstackApp/ToolstackApp.swift | 2 +-
macapp/Sources/ToolstackKit/ApiClient.swift | 5 ++
.../ActiveRefreshTests.swift | 13 ++--
.../NativeContractTests.swift | 6 +-
.../StalePublicationTests.swift | 2 +-
toolstack_forwarder/outbound.py | 2 +-
.../tests/test_proxy_policy.py | 14 ++++
toolyard/egress_proxy.py | 20 ++++--
toolyard/runner.py | 21 ++++--
toolyard/tests/test_egress_proxy.py | 7 +-
toolyard/tests/test_log_guards.py | 63 +++++++++++++++++
toolyard/tests/test_partial_cleanup.py | 3 +
toolyard/tests/test_runner.py | 22 +++---
37 files changed, 479 insertions(+), 52 deletions(-)
create mode 100644 admin/tests/test_private_writes.py
create mode 100644 broker/tests/test_request_guards.py
create mode 100644 client/tests/test_proxy_policy.py
create mode 100644 toolstack_forwarder/tests/test_proxy_policy.py
create mode 100644 toolyard/tests/test_log_guards.py
diff --git a/admin/README.md b/admin/README.md
index d040927..d41d04c 100644
--- a/admin/README.md
+++ b/admin/README.md
@@ -168,6 +168,17 @@ Password reset rotates the session generation atomically. Running admin instance
reject old browser cookies and bearer tokens on their next request, without a
restart. An already-authorized in-flight action is not cancelled by reset.
+Browser sign-out and `POST /api/logout` revoke the presented session on the server.
+Sessions have independent random identifiers; revocations persist until expiry.
+At the bounded revocation capacity, the signing generation rotates to invalidate
+all sessions. Upgrading from the older session format requires signing in again.
+The native app calls logout before discarding its client token; an unreachable
+server cannot confirm revocation. Accepted HTTP hosts are localhost, loopback
+addresses, and the configured admin host.
+
+`GET /api/audit` includes request metadata only, never stored action arguments or
+result payloads. Correlation identifiers remain available to native clients.
+
Configuration audit records changed schema field names, never values, tokens,
paths or destinations. An intent audit must succeed before saving. If completion
auditing fails after saving, the operation fails with an explicit already-saved
diff --git a/admin/api.py b/admin/api.py
index 9c3fd29..b1f3059 100644
--- a/admin/api.py
+++ b/admin/api.py
@@ -111,6 +111,11 @@ async def api_login(request: Request):
token = auth.sign_session(username, request.state.session_secret, settings.session_ttl_seconds())
return {"token": token, "username": username}
+ @app.post("/api/logout")
+ async def api_logout(request: Request, user: str = Depends(require_user)):
+ auth.revoke_session(request.headers["authorization"][7:].strip(), request.state.session_secret)
+ return {"ok": True}
+
@app.get("/api/broker")
async def api_broker_status(user: str = Depends(require_user)):
return supervisor.status()
@@ -224,7 +229,10 @@ async def api_audit(user: str = Depends(require_user), limit: int = 50):
limit = max(1, min(limit, 500))
with open_store(broker_config.load()) as store:
return {"audit": store.recent_audit(limit=limit),
- "requests": _rows(store.list_requests(limit=limit))}
+ "requests": [{key: row[key] for key in (
+ "id", "correlation_id", "caller_id", "tool", "op", "status", "error",
+ "created_at", "updated_at", "idempotency_key", "pruned_at")}
+ for row in store.list_requests(limit=limit)]}
@app.delete("/api/audit")
async def api_clear_audit(user: str = Depends(require_user)):
diff --git a/admin/auth.py b/admin/auth.py
index 41af716..0b6e289 100644
--- a/admin/auth.py
+++ b/admin/auth.py
@@ -19,6 +19,11 @@
import hmac
import secrets
import time
+import json
+import fcntl
+import os
+
+from . import settings
def hash_password(password: str) -> str:
@@ -58,7 +63,7 @@ def verify_password(password: str, encoded: str) -> bool:
def sign_session(username: str, secret: str, ttl_seconds: int) -> str:
expires_at = int(time.time()) + ttl_seconds
- message = f"{username}|{expires_at}"
+ message = f"{username}|{expires_at}|{secrets.token_hex(16)}"
signature = hmac.new(secret.encode("utf-8"), ("session|" + message).encode("utf-8"), hashlib.sha256).hexdigest()
raw = f"{message}|{signature}".encode("utf-8")
return base64.urlsafe_b64encode(raw).decode("ascii")
@@ -69,19 +74,59 @@ def verify_session(cookie: str | None, secret: str) -> str | None:
return None
try:
decoded = base64.urlsafe_b64decode(cookie.encode("ascii")).decode("utf-8")
- username, raw_expires, signature = decoded.split("|", 2)
+ username, raw_expires, session_id, signature = decoded.split("|")
expires_at = int(raw_expires)
+ if len(session_id) != 32 or any(c not in "0123456789abcdef" for c in session_id):
+ return None
except Exception:
return None
if expires_at < int(time.time()):
return None
- message = f"{username}|{expires_at}"
+ message = f"{username}|{expires_at}|{session_id}"
expected = hmac.new(secret.encode("utf-8"), ("session|" + message).encode("utf-8"), hashlib.sha256).hexdigest()
if not signature.isascii() or not hmac.compare_digest(signature, expected):
return None
+ try:
+ if session_id in _revoked_sessions():
+ return None
+ except (OSError, ValueError, TypeError):
+ return None
return username
+def _revoked_sessions() -> dict:
+ path = settings.config_dir() / "revoked-sessions.json"
+ try:
+ data = json.loads(path.read_text(encoding="utf-8"))
+ except FileNotFoundError:
+ return {}
+ if not isinstance(data, dict) or any(not isinstance(v, int) for v in data.values()):
+ raise ValueError("invalid session revocations")
+ return data
+
+
+def revoke_session(token: str, secret: str) -> None:
+ """Persist revocation until expiry, serializing writers across admin processes."""
+ directory = settings.config_dir()
+ settings._private_directory(directory)
+ fd = os.open(directory / "revoked-sessions.lock", os.O_CREAT | os.O_RDWR | os.O_NOFOLLOW, 0o600)
+ try:
+ fcntl.flock(fd, fcntl.LOCK_EX)
+ if verify_session(token, secret) is None:
+ return
+ _, expiry, session_id, _ = base64.urlsafe_b64decode(token).decode().split("|")
+ revoked = {key: value for key, value in _revoked_sessions().items() if value >= int(time.time())}
+ if len(revoked) >= 4096:
+ # Bound state without making any revoked token valid again.
+ settings.rotate_session_secret()
+ revoked = {}
+ else:
+ revoked[session_id] = int(expiry)
+ settings._write_private(directory / "revoked-sessions.json", json.dumps(revoked))
+ finally:
+ os.close(fd)
+
+
def csrf_token(session_value: str, secret: str) -> str:
"""A CSRF token bound to one session, derived via HMAC over the session cookie."""
return hmac.new(
diff --git a/admin/broker_config.py b/admin/broker_config.py
index 49ebc4a..10d6e23 100644
--- a/admin/broker_config.py
+++ b/admin/broker_config.py
@@ -190,9 +190,7 @@ def _warn_if_suspect(cfg: BrokerRunConfig) -> None:
def save(config: BrokerRunConfig) -> None:
path = config_file()
- path.parent.mkdir(parents=True, exist_ok=True)
- path.write_text(_to_toml(asdict(config)), encoding="utf-8")
- path.chmod(0o600) # holds the nod token
+ settings._write_private(path, _to_toml(asdict(config)))
def _toml_str(value) -> str:
diff --git a/admin/server.py b/admin/server.py
index 442a8d2..38d74ce 100644
--- a/admin/server.py
+++ b/admin/server.py
@@ -47,6 +47,23 @@ def create_app() -> FastAPI:
guard = loginguard.LoginGuard() # shared by /login and /api/login
app = FastAPI(title="Toolstack Admin", docs_url=None, redoc_url=None, openapi_url=None)
+ @app.middleware("http")
+ async def trusted_host(request: Request, call_next):
+ # Parse IPv6 authorities correctly (including brackets and optional port).
+ from urllib.parse import urlsplit
+ try:
+ authority = urlsplit("//" + request.headers.get("host", ""))
+ host = authority.hostname
+ if (len(request.headers.getlist("host")) != 1 or authority.username is not None
+ or authority.password is not None or authority.path or authority.query or authority.fragment):
+ host = None
+ except ValueError:
+ host = None
+ allowed = {"localhost", "127.0.0.1", "::1", settings.admin_host().lower()}
+ if host not in allowed:
+ return JSONResponse({"detail": "Invalid host"}, status_code=400)
+ return await call_next(request)
+
@app.middleware("http")
async def session_generation(request: Request, call_next):
# Snapshot once per request; never use the startup key after reset.
@@ -227,6 +244,7 @@ async def logout(request: Request):
data = await read_form(request)
if not csrf_ok(request, data):
return redirect("/")
+ auth.revoke_session(request.cookies.get(SESSION_COOKIE, ""), request.state.session_secret)
resp = redirect("/login")
resp.delete_cookie(SESSION_COOKIE)
return resp
diff --git a/admin/settings.py b/admin/settings.py
index 7ab6e48..922db98 100644
--- a/admin/settings.py
+++ b/admin/settings.py
@@ -109,7 +109,7 @@ def sps_info() -> dict:
def _write_private(path: Path, data: str) -> None:
- path.parent.mkdir(parents=True, exist_ok=True)
+ _private_directory(path.parent)
fd, name = tempfile.mkstemp(prefix=".admin-", dir=path.parent)
try:
with os.fdopen(fd, "w", encoding="utf-8") as stream:
@@ -122,6 +122,15 @@ def _write_private(path: Path, data: str) -> None:
os.unlink(name)
+def _private_directory(path: Path) -> None:
+ """Create missing ancestors privately; reject an unsafe immediate state parent."""
+ if not path.exists():
+ _private_directory(path.parent)
+ path.mkdir(mode=0o700, exist_ok=True)
+ if path.is_symlink() or not path.is_dir() or path.stat().st_mode & 0o022:
+ raise PermissionError("admin state directory must not be group/world writable")
+
+
def read_password_hash() -> str | None:
"""The stored admin password hash, or None if no password has been set."""
path = password_hash_file()
diff --git a/admin/supervisor.py b/admin/supervisor.py
index 713719e..aae7d8f 100644
--- a/admin/supervisor.py
+++ b/admin/supervisor.py
@@ -74,7 +74,7 @@ def _read_state() -> dict | None:
def _write_state(state: dict) -> None:
path = _state_file()
- path.parent.mkdir(parents=True, exist_ok=True)
+ settings._private_directory(path.parent)
path.write_text(json.dumps(state), encoding="utf-8")
@@ -141,7 +141,7 @@ def start(config: BrokerRunConfig) -> dict:
if current["running"]:
return current
state_dir = settings.state_dir()
- state_dir.mkdir(parents=True, exist_ok=True)
+ settings._private_directory(state_dir)
env = {**os.environ, **config.to_env()}
# os.open/os.close (not open()) so the parent leaves no Python file object to
# warn about; the child keeps its own dup'd copies of fds 1 and 2.
diff --git a/admin/tests/test_api.py b/admin/tests/test_api.py
index 9aa18d6..61d7d16 100644
--- a/admin/tests/test_api.py
+++ b/admin/tests/test_api.py
@@ -9,6 +9,7 @@
import subprocess
import tempfile
import unittest
+from contextlib import closing
from pathlib import Path
from unittest import mock
@@ -47,7 +48,7 @@ def setUp(self):
encoding="utf-8")
self.db_path = str(Path(self.tmp, "broker.sqlite3"))
broker_config.save(BrokerRunConfig(db_path=self.db_path, tools_root=str(Path(self.tmp, "tools"))))
- self.client = TestClient(create_app())
+ self.client = TestClient(create_app(), base_url="http://127.0.0.1")
def _restore_env(self):
for key, prev in self._env.items():
@@ -56,6 +57,41 @@ def _restore_env(self):
else:
os.environ[key] = prev
+ def test_logout_revokes_only_presented_session_and_survives_app_recreation(self):
+ first, second = self._auth(), self._auth()
+ self.assertNotEqual(first, second)
+ self.assertEqual(self.client.post("/api/logout", headers=first).status_code, 200)
+ fresh = TestClient(create_app(), base_url="http://127.0.0.1")
+ self.assertEqual(fresh.get("/api/config", headers=first).status_code, 401)
+ self.assertEqual(fresh.get("/api/config", headers=second).status_code, 200)
+ self.assertEqual((settings.config_dir() / "revoked-sessions.json").stat().st_mode & 0o777, 0o600)
+
+ def test_host_is_checked_before_login(self):
+ for host in ("attacker.invalid", "localhost.attacker.invalid", "127.0.0.1.attacker.invalid"):
+ self.assertEqual(self.client.post("/api/login", headers={"Host": host}, json={"password": PASSWORD}).status_code, 400)
+ for host in ("localhost:8780", "127.0.0.1:8780", "[::1]:8780"):
+ self.assertEqual(self.client.post("/api/login", headers={"Host": host}, json={"password": PASSWORD}).status_code, 200)
+ with mock.patch.dict(os.environ, {"TOOLSTACK_ADMIN_HOST": "configured.invalid", "TOOLSTACK_ADMIN_ALLOW_NONLOOPBACK": "1"}):
+ self.assertEqual(self.client.post("/api/login", headers={"Host": "configured.invalid"}, json={"password": PASSWORD}).status_code, 200)
+
+ def test_audit_requests_have_only_metadata(self):
+ from broker.tests.support import seed_caller, make_registry, FakeRuntime
+ from broker.context import BrokerContext
+ from broker.audit import AuditLog
+ from broker.identity import authenticate
+ from broker.request_lifecycle import submit
+ with closing(Store(self.db_path)) as store:
+ token = seed_caller(store, allow=["echo.say"])
+ ctx = BrokerContext(store=store, registry=make_registry({"echo": {"say": "read"}}), runtime=FakeRuntime(), audit=AuditLog(store, sink=None))
+ submit(ctx, authenticate(store, f"Bearer {token}"), "echo", "say", {"private": "fake-payload"}, "fake-correlation")
+ response = self.client.get("/api/audit", headers=self._auth())
+ self.assertEqual(response.status_code, 200)
+ row = response.json()["requests"][0]
+ self.assertEqual(row["correlation_id"], "fake-correlation")
+ self.assertEqual(row["status"], "completed")
+ self.assertFalse({"arguments_json", "action_json", "result_json"} & row.keys())
+ self.assertNotIn("fake-payload", response.text)
+
def test_reset_invalidates_live_bearer_without_restart(self):
authz = self._auth()
self.assertEqual(self.client.get("/api/config", headers=authz).status_code, 200)
diff --git a/admin/tests/test_app.py b/admin/tests/test_app.py
index 4ed286b..af539c8 100644
--- a/admin/tests/test_app.py
+++ b/admin/tests/test_app.py
@@ -71,7 +71,7 @@ def setUp(self):
self.db_path = str(Path(self.tmp, "broker.sqlite3"))
broker_config.save(BrokerRunConfig(db_path=self.db_path, tools_root=str(Path(self.tmp, "tools"))))
- self.client = TestClient(create_app())
+ self.client = TestClient(create_app(), base_url="http://127.0.0.1")
def _restore_env(self):
for key, prev in self._env.items():
diff --git a/admin/tests/test_auth.py b/admin/tests/test_auth.py
index f5f54e6..4f1698e 100644
--- a/admin/tests/test_auth.py
+++ b/admin/tests/test_auth.py
@@ -6,8 +6,12 @@
import hmac
import time
import unittest
+import json
+import os
+import tempfile
+from unittest.mock import patch
-from admin import auth
+from admin import auth, settings
class Password(unittest.TestCase):
@@ -26,6 +30,30 @@ def test_verify_rejects_garbage(self):
class Session(unittest.TestCase):
SECRET = "session-secret"
+ def test_revocation_prunes_expired_entries_and_corrupt_state_fails_closed(self):
+ with tempfile.TemporaryDirectory() as root, patch.dict(os.environ, {"XDG_CONFIG_HOME": root}):
+ path = settings.config_dir() / "revoked-sessions.json"
+ settings._write_private(path, json.dumps({"expired": 1}))
+ token = auth.sign_session("admin", self.SECRET, 60)
+ auth.revoke_session(token, self.SECRET)
+ self.assertNotIn("expired", json.loads(path.read_text()))
+ self.assertIsNone(auth.verify_session(token, self.SECRET))
+ fresh = auth.sign_session("admin", self.SECRET, 60)
+ settings._write_private(path, "invalid json")
+ self.assertIsNone(auth.verify_session(fresh, self.SECRET))
+
+ def test_revocation_capacity_rotates_authority_instead_of_resurrecting_tokens(self):
+ with tempfile.TemporaryDirectory() as root, patch.dict(os.environ, {"XDG_CONFIG_HOME": root}):
+ key = settings.load_or_create_session_secret()
+ token = auth.sign_session("admin", key, 60)
+ path = settings.config_dir() / "revoked-sessions.json"
+ settings._write_private(path, json.dumps({str(i): int(time.time()) + 60 for i in range(4096)}))
+ auth.revoke_session(token, key)
+ new_key = settings.load_or_create_session_secret()
+ self.assertNotEqual(new_key, key)
+ self.assertIsNone(auth.verify_session(token, new_key))
+ self.assertLessEqual(len(json.loads(path.read_text())), 4096)
+
def test_roundtrip(self):
cookie = auth.sign_session("admin", self.SECRET, ttl_seconds=60)
self.assertEqual(auth.verify_session(cookie, self.SECRET), "admin")
diff --git a/admin/tests/test_dialog_logout.py b/admin/tests/test_dialog_logout.py
index c93db3f..531ed6d 100644
--- a/admin/tests/test_dialog_logout.py
+++ b/admin/tests/test_dialog_logout.py
@@ -49,6 +49,8 @@ def test_signout_form_clears_session_and_get_logout_is_not_an_action(self):
self.assertEqual(response.headers['location'], '/login')
self.assertIn('Max-Age=0', response.headers['set-cookie'])
self.assertIsNone(self.client.cookies.get(SESSION_COOKIE))
+ from admin import auth, settings
+ self.assertIsNone(auth.verify_session(cookie, settings.load_or_create_session_secret()))
response = self.client.get('/', follow_redirects=False)
self.assertEqual(response.status_code, 303)
self.assertEqual(response.headers['location'], '/login')
diff --git a/admin/tests/test_private_writes.py b/admin/tests/test_private_writes.py
new file mode 100644
index 0000000..d0d259b
--- /dev/null
+++ b/admin/tests/test_private_writes.py
@@ -0,0 +1,40 @@
+"""Private admin files are private from creation, independently of umask."""
+import os
+import tempfile
+import unittest
+from pathlib import Path
+from unittest.mock import patch
+from admin import settings, broker_config
+
+
+class PrivateWrites(unittest.TestCase):
+ def test_config_and_ancestors_private_with_permissive_umask(self):
+ with tempfile.TemporaryDirectory() as root, patch.dict(os.environ, {"XDG_CONFIG_HOME": root + "/config"}):
+ previous = os.umask(0o002)
+ try:
+ broker_config.save(broker_config.BrokerRunConfig(nod_token="fake-token"))
+ finally:
+ os.umask(previous)
+ path = broker_config.config_file()
+ self.assertEqual(path.stat().st_mode & 0o777, 0o600)
+ self.assertEqual(path.parent.stat().st_mode & 0o777, 0o700)
+ self.assertEqual(Path(root, "config").stat().st_mode & 0o777, 0o700)
+
+ def test_existing_writable_parent_is_refused(self):
+ with tempfile.TemporaryDirectory() as root:
+ parent = Path(root, "unsafe")
+ parent.mkdir()
+ parent.chmod(0o770)
+ with self.assertRaises(PermissionError):
+ settings._write_private(parent / "state", "fake")
+ self.assertFalse((parent / "state").exists())
+
+ def test_tool_editor_uses_text_nodes_for_option_labels(self):
+ from admin.views import assets
+ source = Path(assets.__file__).read_text()
+ self.assertIn("o.textContent=v", source)
+ self.assertNotIn("+'>'+v+''", source)
+
+ def test_installer_log_directory_is_owner_only(self):
+ source = (Path(__file__).resolve().parents[2] / "deploy/install.sh").read_text()
+ self.assertIn('-m 700 /var/log/toolstack', source)
diff --git a/admin/views/assets.py b/admin/views/assets.py
index da22637..a1bc215 100644
--- a/admin/views/assets.py
+++ b/admin/views/assets.py
@@ -289,7 +289,7 @@
var REST_BODY_KINDS = ["none","text","binary"];
var REST_RULE_RESPONSE_TYPES = ["json","xml","form","plaintext"];
function mk(html){var t=document.createElement('template');t.innerHTML=html.trim();return t.content.firstChild;}
- function opts(values, sel){return values.map(function(v){return ''+v+'';}).join('');}
+ function opts(values, sel){return values.map(function(v){var o=document.createElement('option');o.textContent=v;if(v===sel)o.setAttribute('selected','');return o.outerHTML;}).join('');}
function riskOpts(sel){return RISK_CHOICES.indexOf(sel)<0 ? [sel].concat(RISK_CHOICES) : RISK_CHOICES;}
function currentSecretNames(){
return [].map.call(document.querySelectorAll('#secrets .sec-name'), function(n){return n.value.trim();}).filter(Boolean);
diff --git a/broker/gateway.py b/broker/gateway.py
index e421941..6f03d65 100644
--- a/broker/gateway.py
+++ b/broker/gateway.py
@@ -21,6 +21,7 @@
from . import request_lifecycle as lifecycle
import re
from .identity import authenticate, token_fingerprint
+from .registry import _ID_RE
# lifecycle terminal status -> (request.* audit event_type, audit outcome word). A single
# terminal request.* event per request makes "what was the outcome" directly queryable,
@@ -90,9 +91,14 @@ def handle(method: str, path: str, headers: dict, body, ctx) -> Response:
audited = not (method == "GET" and path == HEALTH_PATH)
if audited:
+ audit_path = path
+ if path.startswith(ACTIONS_PREFIX):
+ parts = path[len(ACTIONS_PREFIX):].split(".")
+ if len(parts) != 2 or not all(_ID_RE.fullmatch(part) for part in parts):
+ audit_path = ACTIONS_PREFIX + ""
ctx.audit.record(
"gateway", "request_received", "accepted", correlation_id,
- details={"method": method, "path": path, "has_bearer": "authorization" in headers},
+ details={"method": method, "path": audit_path, "has_bearer": "authorization" in headers},
)
response = _route(method, path, headers, body, ctx, correlation_id)
if audited:
@@ -145,6 +151,8 @@ def _route(method, path, headers, body, ctx, correlation_id) -> Response:
return _describe_tool(path, ctx, caller, correlation_id)
if method == "GET" and path.startswith(REQUESTS_PREFIX):
+ if ctx.rate_limiter is not None and not ctx.rate_limiter.allow(caller.id):
+ return Response(429, {"error": "rate_limited"}, correlation_id)
return _request_status(path, ctx, caller, correlation_id)
return Response(404, {"error": "not_found"}, correlation_id)
@@ -206,7 +214,7 @@ def _request_status(path, ctx, caller, correlation_id) -> Response:
def _action(path, body, ctx, caller, correlation_id, idempotency_key=None) -> Response:
spec = path[len(ACTIONS_PREFIX):]
parts = spec.split(".")
- if len(parts) != 2 or not all(parts):
+ if len(parts) != 2 or not all(_ID_RE.fullmatch(p) for p in parts):
return Response(400, {"error": "invalid",
"detail": "expected /v1/actions/."}, correlation_id)
tool, op = parts
diff --git a/broker/mcp.py b/broker/mcp.py
index 0af2bb0..dafcd18 100644
--- a/broker/mcp.py
+++ b/broker/mcp.py
@@ -128,7 +128,8 @@ def _resolve_name(ctx, name: str):
if name in registered:
return registered[name]
tool, sep, op = name.partition("__")
- if sep and tool and op:
+ from .registry import _ID_RE
+ if sep and _ID_RE.fullmatch(tool) and _ID_RE.fullmatch(op):
return tool, op
return None, None
diff --git a/broker/ratelimit.py b/broker/ratelimit.py
index 9e04322..6afdef9 100644
--- a/broker/ratelimit.py
+++ b/broker/ratelimit.py
@@ -1,6 +1,6 @@
"""Per-caller fixed-window rate limiter (in-memory, single process).
-Bounds how many action submissions a caller can make per minute. Disabled when
+Bounds action submissions and request-status polls per caller per minute. Disabled when
the limit is <= 0. `now` is injectable for testing. Single-threaded dev server, so
a plain dict is fine; revisit with locking under concurrent serving.
"""
diff --git a/broker/request_lifecycle.py b/broker/request_lifecycle.py
index bf5ad0c..fbb4da6 100644
--- a/broker/request_lifecycle.py
+++ b/broker/request_lifecycle.py
@@ -24,6 +24,7 @@
from . import policy as policy_rules
from .approval_summary import summarize
from .runtime import NotDispatched
+from .redaction import redact
OK = "ok"
PENDING = "pending_approval"
@@ -287,6 +288,7 @@ def resolve_request(ctx, request_id, now=None, caller=None) -> Outcome:
return _outcome_from_request(ctx.store.request(request_id), ctx.store.approval_for_request(request_id))
state = ctx.surface.poll(approval_row["surface_ref"])
+ state = replace(state, note=redact(state.note))
after_poll = max(now, time.time())
current_req = ctx.store.request(request_id)
current_approval = ctx.store.approval_for_request(request_id)
@@ -351,7 +353,7 @@ def _outcome_from_request(req, approval_row=None) -> Outcome:
}
reason = "approval_rejected" if req["error"] == "approval_rejected" else None
approver = approval_row["approver"] if approval_row else None
- note = approval_row["note"] if approval_row else None
+ note = redact(approval_row["note"]) if approval_row else None
decided_at = approval_row["decided_at"] if approval_row else None
return Outcome(mapping.get(status, UNKNOWN), request_id=req["id"],
result=result, error=req["error"], reason=reason,
diff --git a/broker/server.py b/broker/server.py
index b541882..ac51b0b 100644
--- a/broker/server.py
+++ b/broker/server.py
@@ -30,6 +30,7 @@
from .audit import AuditLog, stderr_sink
from .context import BrokerContext
from .gateway import handle
+from .identity import authenticate
from .ratelimit import RateLimiter
from .registry import Registry
from .runtime import HttpRuntime
@@ -242,6 +243,8 @@ def _read_body(self):
cap = self._body_cap()
if length > cap:
return _TooLarge(cap)
+ if length > MAX_BODY_BYTES and authenticate(self._ctx().store, self.headers.get("Authorization")) is None:
+ raise PermissionError("unauthorized")
raw = self.rfile.read(length)
if len(raw) != length:
raise ValueError("incomplete body")
@@ -255,6 +258,10 @@ def _read_body(self):
def _respond(self) -> None:
try:
body = self._read_body()
+ except PermissionError:
+ self.close_connection = True
+ self._send_json(401, b'{"error":"unauthorized"}', uuid.uuid4().hex)
+ return
except ValueError:
self.send_error(400)
return
diff --git a/broker/tests/test_request_guards.py b/broker/tests/test_request_guards.py
new file mode 100644
index 0000000..a7c4c40
--- /dev/null
+++ b/broker/tests/test_request_guards.py
@@ -0,0 +1,69 @@
+"""Input admission and approval receipt regressions with fake transports."""
+import io
+from email.message import Message
+from types import SimpleNamespace
+from unittest.mock import Mock
+
+from broker import approval, mcp
+from broker.gateway import handle
+from broker.ratelimit import RateLimiter
+from broker.server import _Handler, MAX_BODY_BYTES
+from .support import BrokerTestCase, seed_caller
+
+
+class RequestGuards(BrokerTestCase):
+ def test_large_body_authentication_precedes_read(self):
+ ctx = self.make_ctx()
+ token = seed_caller(ctx.store)
+ handler = object.__new__(_Handler)
+ handler.server = SimpleNamespace(ctx=ctx)
+ handler.path = "/v1/actions/echo.say"
+ handler.headers = Message()
+ handler.headers["Content-Length"] = str(MAX_BODY_BYTES + 1)
+ handler.rfile = Mock()
+ with self.assertRaises(PermissionError):
+ handler._read_body()
+ handler.rfile.read.assert_not_called()
+ handler.headers["Authorization"] = f"Bearer {token}"
+ handler.rfile = io.BytesIO(b" " * (MAX_BODY_BYTES - 1) + b"{}")
+ self.assertEqual(handler._read_body(), {})
+
+ def test_invalid_names_never_reserve_or_lookup(self):
+ ctx = self.make_ctx()
+ token = seed_caller(ctx.store)
+ headers = {"Authorization": f"Bearer {token}", "Idempotency-Key": "fake-key"}
+ for name in ("echo.bad/name", "echo.bad%00", "-echo.say", "echo.\n"):
+ response = handle("POST", "/v1/actions/" + name, headers, {}, ctx)
+ self.assertEqual(response.status, 400)
+ for name in ("echo__bad/name", "-echo__say", "echo__bad%00"):
+ self.assertEqual(mcp._resolve_name(ctx, name), (None, None))
+ self.assertEqual(ctx.store.list_requests(), [])
+ self.assertFalse(any(e["event_type"] == "tool_lookup_failed" for e in ctx.audit.events()))
+ self.assertNotIn("bad/name", str(ctx.audit.events()))
+
+ def test_status_polls_are_metered_before_surface_contact(self):
+ ctx = self.make_ctx(catalog={"echo": {"say": "read"}})
+ token = seed_caller(ctx.store, review=["echo.say"])
+ headers = {"Authorization": f"Bearer {token}"}
+ response = handle("POST", "/v1/actions/echo.say", headers, {}, ctx)
+ path = f'/v1/requests/{response.body["request_id"]}'
+ ctx.rate_limiter = RateLimiter(1)
+ ctx.surface.poll = Mock(wraps=ctx.surface.poll)
+ self.assertEqual(handle("GET", path, headers, {}, ctx).status, 200)
+ self.assertEqual(handle("GET", path, headers, {}, ctx).status, 429)
+ ctx.surface.poll.assert_called_once()
+
+ def test_approval_note_is_masked_in_storage_and_receipt(self):
+ for outcome in (approval.APPROVED, approval.REJECTED):
+ ctx = self.make_ctx(catalog={"echo": {"say": "read"}})
+ token = seed_caller(ctx.store, review=["echo.say"])
+ headers = {"Authorization": f"Bearer {token}"}
+ response = handle("POST", "/v1/actions/echo.say", headers, {}, ctx)
+ rid = response.body["request_id"]
+ ctx.surface.set(outcome, note="Bearer fake-secret " + "words " * 200)
+ receipt = handle("GET", f"/v1/requests/{rid}", headers, {}, ctx)
+ note = ctx.store.approval_for_request(rid)["note"]
+ self.assertNotIn("fake-secret", note)
+ self.assertLessEqual(len(note), 283)
+ self.assertIn("[redacted]", note)
+ self.assertNotIn("fake-secret", str(receipt.body))
diff --git a/client/tests/test_proxy_policy.py b/client/tests/test_proxy_policy.py
new file mode 100644
index 0000000..488093c
--- /dev/null
+++ b/client/tests/test_proxy_policy.py
@@ -0,0 +1,14 @@
+import importlib
+import os
+import unittest
+import urllib.request
+from unittest.mock import patch
+from client import toolstack
+
+
+class ProxyPolicy(unittest.TestCase):
+ def test_ambient_proxies_are_not_installed(self):
+ with patch.dict(os.environ, {"HTTP_PROXY": "http://fake.invalid:1", "HTTPS_PROXY": "http://fake.invalid:1"}):
+ importlib.reload(toolstack)
+ self.assertFalse(any(isinstance(h, urllib.request.ProxyHandler) and h.proxies
+ for h in toolstack._OPENER.handlers))
diff --git a/client/toolstack.py b/client/toolstack.py
index b2b5af2..a82df8b 100755
--- a/client/toolstack.py
+++ b/client/toolstack.py
@@ -48,7 +48,7 @@ def redirect_request(self, *args, **kwargs):
return None
-_OPENER = urllib.request.build_opener(_NoRedirect)
+_OPENER = urllib.request.build_opener(urllib.request.ProxyHandler({}), _NoRedirect)
def _base() -> str:
diff --git a/deploy/docker/README.md b/deploy/docker/README.md
index d8e68c3..248c8a5 100644
--- a/deploy/docker/README.md
+++ b/deploy/docker/README.md
@@ -9,6 +9,10 @@ does not provide proven runner registration/recovery. Do not use it for the supp
release. See [runtime matrix](../../docs/runtime-matrix.md) and native deployment.
The image's process runner does not isolate hostile tools.
+The one-box service must be the only container on its Docker network. Admin and
+broker bind all container interfaces: loopback-only published ports do not prevent
+another container on the same network from connecting directly.
+
Before any future Docker/SPS support claim, prove: SPS lifecycle, TLS server name/CA
matching the reachable service, explicit runner config and passphrase wiring, private
state/log volumes, offline vault provisioning, separate broker channel credentials,
diff --git a/deploy/install.sh b/deploy/install.sh
index d595e65..fa3e313 100755
--- a/deploy/install.sh
+++ b/deploy/install.sh
@@ -51,7 +51,7 @@ sudo -u "$SERVICE_USER" sh -c "cd '$REPO_ROOT' && '$VENV_PIP' install --quiet '.
# 4. State + log dirs (the unit's StateDirectory/ReadWritePaths keep them; pre-create
# so set-password + the SPS audit logger land right instead of crashing on first write)
install -d -o "$SERVICE_USER" -g "$SERVICE_GROUP" -m 700 /var/lib/toolstack
-install -d -o "$SERVICE_USER" -g "$SERVICE_GROUP" -m 750 /var/log/toolstack
+install -d -o "$SERVICE_USER" -g "$SERVICE_GROUP" -m 700 /var/log/toolstack
install -d -o root -g "$SERVICE_GROUP" -m 750 /etc/toolstack
# 5. Admin password (the panel fails closed without one)
diff --git a/macapp/Sources/ToolstackApp/AppModel.swift b/macapp/Sources/ToolstackApp/AppModel.swift
index 4da0fcd..76853ef 100644
--- a/macapp/Sources/ToolstackApp/AppModel.swift
+++ b/macapp/Sources/ToolstackApp/AppModel.swift
@@ -185,7 +185,7 @@ final class AppModel: ObservableObject {
clearAdminState()
restoring = false
tokenStorage.delete(account)
- await oldClient.setToken(nil)
+ try? await oldClient.logout()
}
func refreshAll() async {
@@ -241,9 +241,12 @@ final class AppModel: ObservableObject {
/// Start / stop / restart a tool, then refresh so its row reflects the new run state.
func toolAction(id: String, action: String, confirmed: Bool = false) async {
guard uncertainMutation == nil, !effectInFlight else { return }
- if action == "restart", !confirmed {
+ if (action == "start" || action == "restart"), !confirmed {
let epoch = generation
- confirmation = Confirmation(title: "Restart \(id)?", effect: "Stops and starts \(id), interrupting active calls. Requests will not be replayed.") {
+ let title = action == "start" ? "Start \(id)?" : "Restart \(id)?"
+ let effect = "A started tool runs with this host's credentials. Only start tools you trust."
+ + (action == "restart" ? " Stops and starts \(id), interrupting active calls. Requests will not be replayed." : "")
+ confirmation = Confirmation(title: title, effect: effect) {
guard epoch == self.generation else { return }
await self.toolAction(id: id, action: action, confirmed: true)
}
diff --git a/macapp/Sources/ToolstackApp/ToolstackApp.swift b/macapp/Sources/ToolstackApp/ToolstackApp.swift
index a5c2da5..a5add35 100644
--- a/macapp/Sources/ToolstackApp/ToolstackApp.swift
+++ b/macapp/Sources/ToolstackApp/ToolstackApp.swift
@@ -116,7 +116,7 @@ struct ContentView: View {
.alert(model.confirmation?.title ?? "Confirm", isPresented: Binding(
get: { model.confirmation != nil }, set: { if !$0 { model.confirmation = nil } }
), presenting: model.confirmation) { confirmation in
- Button("Cancel", role: .cancel) {}
+ Button("Cancel", role: .cancel) {}.keyboardShortcut(.defaultAction)
Button(confirmation.title.replacingOccurrences(of: "?", with: ""), role: .destructive) {
Task { await confirmation.action() }
}
diff --git a/macapp/Sources/ToolstackKit/ApiClient.swift b/macapp/Sources/ToolstackKit/ApiClient.swift
index 5185b00..9fec447 100644
--- a/macapp/Sources/ToolstackKit/ApiClient.swift
+++ b/macapp/Sources/ToolstackKit/ApiClient.swift
@@ -48,6 +48,11 @@ public actor ApiClient {
try await send("GET", "/api/broker")
}
+ public func logout() async throws {
+ defer { token = nil }
+ let _: [String: Bool] = try await send("POST", "/api/logout", body: [:])
+ }
+
public func catalogObservation() async throws -> CatalogObservation {
guard let token, !token.isEmpty else { throw ApiError.unauthorized }
return try await send("GET", "/api/catalog")
diff --git a/macapp/Tests/ToolstackKitTests/ActiveRefreshTests.swift b/macapp/Tests/ToolstackKitTests/ActiveRefreshTests.swift
index e7d9efc..8a58f71 100644
--- a/macapp/Tests/ToolstackKitTests/ActiveRefreshTests.swift
+++ b/macapp/Tests/ToolstackKitTests/ActiveRefreshTests.swift
@@ -17,6 +17,7 @@ final class ActiveRefreshTests: XCTestCase {
case "/api/broker": json = #"{"running":true}"#
case "/api/callers": json = #"{"callers":[{"id":1,"name":"demo","revoked_at":123}]}"#
case "/api/tools": json = #"{"tools":[]}"#
+ case "/api/logout": json = #"{"ok":true}"#
default: XCTFail("Unexpected request: \(request)"); json = "{}"
}
return (200, [:], Data(json.utf8))
@@ -80,7 +81,7 @@ final class ActiveRefreshTests: XCTestCase {
await first.value; await second.value
XCTAssertEqual(PendingTransport.count, 3)
}
- @MainActor func testSignOutDeletesStoredTokenClearsClientAndReturnsToLoginWithoutServerLogout() async throws {
+ @MainActor func testSignOutRevokesSessionDeletesStoredTokenAndReturnsToLogin() async throws {
stub()
let api = client()
var tokens = [account: "FAKE-TEST-TOKEN"]
@@ -94,6 +95,8 @@ final class ActiveRefreshTests: XCTestCase {
XCTAssertEqual(StubURLProtocol.lastRequest?.value(forHTTPHeaderField: "Authorization"), "Bearer FAKE-TEST-TOKEN")
let count = StubURLProtocol.requestCount
await model.logout()
+ XCTAssertEqual(StubURLProtocol.lastRequest?.url?.path, "/api/logout")
+ XCTAssertEqual(StubURLProtocol.lastRequest?.value(forHTTPHeaderField: "Authorization"), "Bearer FAKE-TEST-TOKEN")
XCTAssertEqual(deleted, [account]); XCTAssertNil(storage.load(account))
let authorized = await api.isAuthenticated
XCTAssertFalse(authorized)
@@ -101,7 +104,7 @@ final class ActiveRefreshTests: XCTestCase {
XCTAssertTrue(model.callers.isEmpty); XCTAssertNil(model.broker)
await model.refreshWhileActive(true)
await model.refreshAll()
- XCTAssertEqual(StubURLProtocol.requestCount, count, "Sign-out is client-side only; no logout endpoint or signed-out refresh")
+ XCTAssertEqual(StubURLProtocol.requestCount, count + 1, "Sign-out revokes once; no signed-out refresh")
_ = try await api.brokerStatus()
XCTAssertNil(StubURLProtocol.lastRequest?.value(forHTTPHeaderField: "Authorization"))
}
@@ -114,11 +117,13 @@ final class ActiveRefreshTests: XCTestCase {
PendingTransport.started = { started.fulfill() }
let task = Task { await model.refreshWhileActive(true) }
await fulfillment(of: [started], timeout: 2)
+ let held = PendingTransport.pending
+ PendingTransport.started = { PendingTransport.pending?.respond(200, #"{"ok":true}"#) }
await model.logout()
- PendingTransport.pending?.respond(200, #"{"running":true}"#)
+ held?.respond(200, #"{"running":true}"#)
await task.value
await model.refreshWhileActive(true)
- XCTAssertEqual(PendingTransport.count, 1)
+ XCTAssertEqual(PendingTransport.count, 2)
XCTAssertNil(model.broker); XCTAssertTrue(model.callers.isEmpty)
}
override func tearDown() {
diff --git a/macapp/Tests/ToolstackKitTests/NativeContractTests.swift b/macapp/Tests/ToolstackKitTests/NativeContractTests.swift
index 7b9f3e7..c46426f 100644
--- a/macapp/Tests/ToolstackKitTests/NativeContractTests.swift
+++ b/macapp/Tests/ToolstackKitTests/NativeContractTests.swift
@@ -138,7 +138,7 @@ final class NativeContractTests: XCTestCase {
let model = AppModel(client: client(), serverURL: "http://fake.invalid")
_ = await model.refreshTools()
XCTAssertNotNil(model.toolsError)
- await model.toolAction(id: "demo", action: "start")
+ await model.toolAction(id: "demo", action: "start", confirmed: true)
XCTAssertNotNil(model.uncertainMutation)
XCTAssertNotNil(model.toolsError)
listingWorks = true
@@ -170,6 +170,10 @@ final class NativeContractTests: XCTestCase {
XCTAssertEqual(effects, ["/api/tools"])
XCTAssertFalse(model.registrationObserved(for: "demo"))
await model.toolAction(id: "demo", action: "start")
+ XCTAssertEqual(effects.count, 1)
+ XCTAssertEqual(model.confirmation?.title, "Start demo?")
+ XCTAssertTrue(model.confirmation?.effect.contains("this host's credentials") == true)
+ await model.confirmation?.action()
XCTAssertEqual(effects.count, 2)
XCTAssertFalse(model.registrationObserved(for: "demo"))
await model.brokerAction("restart")
diff --git a/macapp/Tests/ToolstackKitTests/StalePublicationTests.swift b/macapp/Tests/ToolstackKitTests/StalePublicationTests.swift
index 04b6dff..c471301 100644
--- a/macapp/Tests/ToolstackKitTests/StalePublicationTests.swift
+++ b/macapp/Tests/ToolstackKitTests/StalePublicationTests.swift
@@ -168,7 +168,7 @@ final class StalePublicationTests: XCTestCase {
let oldTask = Task { await model.refreshTools() }
await fulfillment(of: [first], timeout: 2)
let old = try XCTUnwrap(PendingTransport.pending)
- let actionTask = Task { await model.toolAction(id: "demo", action: "start") }
+ let actionTask = Task { await model.toolAction(id: "demo", action: "start", confirmed: true) }
await fulfillment(of: [action], timeout: 2)
PendingTransport.pending?.respond(200, #"{"id":"demo"}"#)
await fulfillment(of: [listing], timeout: 2)
diff --git a/toolstack_forwarder/outbound.py b/toolstack_forwarder/outbound.py
index b81d44d..319535d 100644
--- a/toolstack_forwarder/outbound.py
+++ b/toolstack_forwarder/outbound.py
@@ -14,7 +14,7 @@ def redirect_request(self, *args, **kwargs):
return None
-_OPENER = urllib.request.build_opener(_NoRedirect)
+_OPENER = urllib.request.build_opener(urllib.request.ProxyHandler({}), _NoRedirect)
_HOP_BY_HOP = {
"connection", "keep-alive", "proxy-authenticate", "proxy-authorization",
"te", "trailer", "transfer-encoding", "upgrade",
diff --git a/toolstack_forwarder/tests/test_proxy_policy.py b/toolstack_forwarder/tests/test_proxy_policy.py
new file mode 100644
index 0000000..40e61d3
--- /dev/null
+++ b/toolstack_forwarder/tests/test_proxy_policy.py
@@ -0,0 +1,14 @@
+import importlib
+import os
+import unittest
+import urllib.request
+from unittest.mock import patch
+from toolstack_forwarder import outbound
+
+
+class ProxyPolicy(unittest.TestCase):
+ def test_ambient_proxies_are_not_installed(self):
+ with patch.dict(os.environ, {"HTTP_PROXY": "http://fake.invalid:1", "HTTPS_PROXY": "http://fake.invalid:1"}):
+ importlib.reload(outbound)
+ self.assertFalse(any(isinstance(h, urllib.request.ProxyHandler) and h.proxies
+ for h in outbound._OPENER.handlers))
diff --git a/toolyard/egress_proxy.py b/toolyard/egress_proxy.py
index 41b7f0a..a0d6ac7 100644
--- a/toolyard/egress_proxy.py
+++ b/toolyard/egress_proxy.py
@@ -12,7 +12,9 @@
HTTP: the tool sends an absolute-form request (`GET http://host/..`); the proxy checks
the host and forwards it in origin form.
-Host matching is exact. Run detached by the runner; serves until killed on tool stop:
+Host matching is exact. CONNECT permits port 443 for a bare host entry; other
+ports require an explicit host:port entry (IPv6: [host]:port).
+Run detached by the runner; serves until killed on tool stop:
python3 -m toolyard.egress_proxy --port --allow [--allow ...]
"""
@@ -144,11 +146,19 @@ def _deny(self, host: str) -> None:
def do_CONNECT(self) -> None:
self.close_connection = True
- host, _, port = self.path.partition(":")
- if host not in allow:
- return self._deny(host)
try:
- upstream = self._connect((host, int(port or 443)))
+ target = urlsplit("//" + self.path)
+ host = target.hostname
+ port = 443 if target.port is None else target.port
+ if not host or not 1 <= port <= 65535 or target.username is not None or target.password is not None or target.path or target.query or target.fragment:
+ return self._deny("invalid destination")
+ except ValueError:
+ return self._deny("invalid destination")
+ authority = f"[{host}]:{port}" if ":" in host else f"{host}:{port}"
+ if not (authority in allow or (port == 443 and host in allow)):
+ return self._deny(authority)
+ try:
+ upstream = self._connect((host, port))
except (OSError, ValueError):
return self.send_error(502, "upstream failed")
try:
diff --git a/toolyard/runner.py b/toolyard/runner.py
index 27a04dc..aa2444f 100644
--- a/toolyard/runner.py
+++ b/toolyard/runner.py
@@ -61,7 +61,7 @@ def _child_environment() -> dict[str, str]:
"""
allowed = {"PATH", "HOME", "TMPDIR", "TMP", "TEMP", "LANG", "TZ",
"PYTHONPATH", "VIRTUAL_ENV", "PYTHONUNBUFFERED", "SSL_CERT_FILE",
- "SSL_CERT_DIR", "TOOLSTACK_SPS_FAKE"}
+ "SSL_CERT_DIR"}
return {key: value for key, value in os.environ.items()
if key in allowed or key.startswith("LC_")}
@@ -71,10 +71,23 @@ def _tool_log_path(tool_def: ToolDef) -> Path:
the tool, not an opaque global state directory."""
d = tool_def.path / "logs"
d.mkdir(parents=True, exist_ok=True, mode=0o700)
- os.chmod(d, 0o700)
+ fd = os.open(d, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW)
+ try:
+ os.fchmod(fd, 0o700)
+ finally:
+ os.close(fd)
return d / "tool.log"
+def _open_tool_log(path: Path) -> int:
+ directory = os.open(path.parent, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW)
+ try:
+ return os.open(path.name, os.O_WRONLY | os.O_CREAT | os.O_APPEND | os.O_NOFOLLOW,
+ 0o600, dir_fd=directory)
+ finally:
+ os.close(directory)
+
+
def _check_port_free(port: int) -> None:
"""Fail early + clearly if the tool's loopback port is already taken; otherwise the tool
binds, exits, and only surfaces as a 502 at call time."""
@@ -505,7 +518,7 @@ def _stop_egress_proxy(running: RunningTool) -> None:
def _start_docker_log_follower(container: str, log_path: Path, *, _on_spawn=None) -> str:
- fd = os.open(log_path, os.O_WRONLY | os.O_CREAT | os.O_APPEND | os.O_NOFOLLOW, 0o600)
+ fd = _open_tool_log(log_path)
os.fchmod(fd, 0o600)
try:
pid = os.posix_spawn(
@@ -670,7 +683,7 @@ def mark_reaped():
# warn about; setpgroup=0 gives it its own group so stop() can killpg it.
inner_script = f"cd {shlex.quote(str(tool_def.path))} && exec {_bind_interpreter(tool_def.command)}"
executable, argv = self._spawn_argv(tool_def, inner_script, None, egress_port)
- log_fd = os.open(log_path, os.O_WRONLY | os.O_CREAT | os.O_APPEND | os.O_NOFOLLOW, 0o600)
+ log_fd = _open_tool_log(log_path)
os.fchmod(log_fd, 0o600)
try:
pid = os.posix_spawn(
diff --git a/toolyard/tests/test_egress_proxy.py b/toolyard/tests/test_egress_proxy.py
index 9000be8..e3642a5 100644
--- a/toolyard/tests/test_egress_proxy.py
+++ b/toolyard/tests/test_egress_proxy.py
@@ -71,7 +71,7 @@ def setUp(self):
echo_thread.start()
self.addCleanup(self._stop_echo, echo_thread)
- self.proxy = serve(_free_port(), ["127.0.0.1"])
+ self.proxy = serve(_free_port(), ["127.0.0.1", f"127.0.0.1:{self.echo_port}"])
self.proxy.daemon_threads = False
proxy_thread = threading.Thread(target=self.proxy.serve_forever)
proxy_thread.start()
@@ -248,6 +248,11 @@ def test_connect_to_denied_host_is_403(self):
self.addCleanup(s.close)
self.assertIn(b"403", status)
+ def test_connect_to_unlisted_port_is_403(self):
+ s, status = self._connect(f"127.0.0.1:{self.origin_port}")
+ self.addCleanup(s.close)
+ self.assertIn(b"403", status)
+
if __name__ == "__main__":
unittest.main()
diff --git a/toolyard/tests/test_log_guards.py b/toolyard/tests/test_log_guards.py
new file mode 100644
index 0000000..e0e176f
--- /dev/null
+++ b/toolyard/tests/test_log_guards.py
@@ -0,0 +1,63 @@
+"""Tool environment and log paths remain constrained before launch."""
+import os
+import tempfile
+import unittest
+from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import patch
+from unittest.mock import Mock
+
+from toolyard import runner
+
+
+class LogGuards(unittest.TestCase):
+ def test_tunnel_default_and_explicit_ports_checked_before_connect(self):
+ from toolyard.egress_proxy import _handler
+ for authority, allow, permitted in (
+ ("fake.invalid:443", {"fake.invalid"}, True),
+ ("fake.invalid:8443", {"fake.invalid"}, False),
+ ("fake.invalid:8443", {"fake.invalid:8443"}, True),
+ ("fake.invalid:0", {"fake.invalid"}, False),
+ ("[::1]:8443", {"[::1]:8443"}, True),
+ ("user@fake.invalid:443", {"fake.invalid"}, False),
+ ):
+ handler = object.__new__(_handler(frozenset(allow)))
+ handler.path = authority
+ handler._connect = Mock(side_effect=OSError)
+ handler.send_error = Mock()
+ handler._deny = Mock()
+ handler.do_CONNECT()
+ self.assertEqual(handler._connect.called, permitted, authority)
+ self.assertEqual(handler._deny.called, not permitted, authority)
+
+ def test_fake_secret_switch_not_inherited(self):
+ with patch.dict(os.environ, {"TOOLSTACK_SPS_FAKE": "1", "PATH": "/bin"}, clear=True):
+ self.assertEqual(runner._child_environment(), {"PATH": "/bin"})
+
+ def test_log_directory_symlink_is_refused_without_chmod(self):
+ with tempfile.TemporaryDirectory() as directory:
+ root = Path(directory)
+ target = root / "target"
+ target.mkdir(mode=0o755)
+ before = target.stat().st_mode
+ (root / "logs").symlink_to(target, target_is_directory=True)
+ with self.assertRaises(OSError):
+ runner._tool_log_path(SimpleNamespace(path=root))
+ self.assertEqual(target.stat().st_mode, before)
+ self.assertFalse((target / "tool.log").exists())
+
+ def test_log_final_component_and_swapped_parent_refuse_links(self):
+ with tempfile.TemporaryDirectory() as directory:
+ root = Path(directory)
+ path = runner._tool_log_path(SimpleNamespace(path=root))
+ target = root / "target"
+ target.write_text("unchanged")
+ path.symlink_to(target)
+ with self.assertRaises(OSError):
+ runner._open_tool_log(path)
+ path.unlink()
+ path.parent.rmdir()
+ path.parent.symlink_to(root, target_is_directory=True)
+ with self.assertRaises(OSError):
+ runner._open_tool_log(path)
+ self.assertEqual(target.read_text(), "unchanged")
diff --git a/toolyard/tests/test_partial_cleanup.py b/toolyard/tests/test_partial_cleanup.py
index 71ed33c..6d47a70 100644
--- a/toolyard/tests/test_partial_cleanup.py
+++ b/toolyard/tests/test_partial_cleanup.py
@@ -261,6 +261,9 @@ def test_docker_removal_failure_keeps_full_helper_record(self):
def test_docker_log_close_failure_still_retains_spawned_helper(self):
tool, _ = self.start_fixture()
tool.type, tool.image = "rest", "fake-image"
+ log_path = runner._tool_log_path(tool)
+ self.enterContext(patch.object(runner, "_tool_log_path", return_value=log_path))
+ self.enterContext(patch.object(runner, "_open_tool_log", return_value=99))
with patch.dict(os.environ, {"TOOLSTACK_SPS_SKIP": "1", "TOOLSTACK_E_SECRET": ""}), patch.object(runner.DockerRunner, "_docker"), patch.object(runner.os, "open", return_value=99), patch.object(runner.os, "fchmod"), patch.object(runner.os, "close", side_effect=OSError("close failed")), patch.object(runner.subprocess, "run", return_value=SimpleNamespace(returncode=0)), patch.object(runner, "_terminate", side_effect=RuntimeError()) as terminate:
with self.assertRaises(runner.RunnerStartCleanupError) as error:
runner.DockerRunner().start(tool)
diff --git a/toolyard/tests/test_runner.py b/toolyard/tests/test_runner.py
index a28a429..f672568 100644
--- a/toolyard/tests/test_runner.py
+++ b/toolyard/tests/test_runner.py
@@ -30,7 +30,7 @@
from broker.registry import Registry, ToolOp
from broker.runtime import HttpRuntime
from broker.server import build_server
-from toolyard.config import load
+from toolyard.config import load as _load
from toolyard.egress_proxy import serve as _serve_egress_proxy
from toolyard.runner import (BwrapRunner, DockerRunner, ProcessRunner, RunningTool, SeatbeltRunner, RunnerConfigError,
_SANDBOX_EXEC, _netguard_argv, _seatbelt_profile)
@@ -47,7 +47,7 @@ def setUpModule():
_boot_id() # Resolve before any subprocess/spawn mocks can pollute the cache.
_fixture_dir = tempfile.TemporaryDirectory()
_fixture_env = mock.patch.dict(os.environ, {
- "TOOLSTACK_SPS_SKIP": "1", "TOOLSTACK_SPS_FAKE": "1",
+ "TOOLSTACK_SPS_SKIP": "1",
"PYTHONPATH": str(REPO),
"TOOLSTACK_SPS_ENV": str(Path(_fixture_dir.name) / "absent.env"),
"TOOLSTACK_TOOLYARD_STATE": str(Path(_fixture_dir.name) / "state.json"),
@@ -56,14 +56,8 @@ def setUpModule():
_fixture_env.start()
# Foreign SPS credentials must never become channel credentials.
os.environ.setdefault('TOOLSTACK_E_SECRET', 'foreign-sps-key')
- # Phase 5: tool boot pulls secrets from SPS. The 12 tool-starting tests
- # don't exercise the SPS wire (that's covered in sps/tests); they only
- # need the tool subprocess to come up. TOOLSTACK_SPS_FAKE=1 makes the
- # SDK's from_env() return a fake client whose cache is empty and whose
- # _cli is None, so the tool boots without ever dialing SPS. The SpsRegistration
- # tests reset os.environ under their own mock.patch.dict, so this flag never
- # leaks into assertions there. Production never sets this env var.
- os.environ.setdefault('TOOLSTACK_SPS_FAKE', '1')
+ # Echo subprocess fixtures explicitly enable their fake SDK in the test
+ # command below; no test-only switch is inherited through the runner.
# Channel fixtures use only runner-minted independent keys.
@@ -83,6 +77,14 @@ def _remember_running(running):
TOOL_MCP_TOML = REPO / "tools" / "echo_mcp" / "toolyard.toml"
SECRET = "dev-secret-123"
+
+def load(path):
+ tool = _load(path)
+ if path in (TOOL_TOML, TOOL_MCP_TOML):
+ bootstrap = "import os, runpy; os.environ['TOOLSTACK_SPS_FAKE']='1'; runpy.run_path('app.py', run_name='__main__')"
+ tool = dataclasses.replace(tool, command=f"{shlex.quote(sys.executable)} -B -c {shlex.quote(bootstrap)}")
+ return tool
+
# A pid value used as a mock-return value in tests that simulate `posix_spawn`. Why
# this high: if a future refactor accidentally lets the value reach `os.killpg` / `os.kill`
# without the test's mock being in scope, the OS will report `ProcessLookupError` (no
From 99473e4195c6846c96b0b12e94b62d13da82cc0e Mon Sep 17 00:00:00 2001
From: batteryshark <1574871+batteryshark@users.noreply.github.com>
Date: Sun, 25 Oct 2026 15:08:25 -0400
Subject: [PATCH 4/4] Accept the Start confirmation in offscreen recovery
fixtures
---
admin/tests/webkit_fixtures.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/admin/tests/webkit_fixtures.py b/admin/tests/webkit_fixtures.py
index 7407cf1..f1c0e35 100644
--- a/admin/tests/webkit_fixtures.py
+++ b/admin/tests/webkit_fixtures.py
@@ -69,11 +69,13 @@
UNKNOWN_HTML = r"""
const form=document.querySelector('form[data-mutation]');let posts=0;
window.fetch=async()=>{posts++;return {ok:true,status:200,url:form.action,text:async()=>window.RETURNED_PAGE};};
-form.requestSubmit();await wait(150);
+// Start now confirms first (host-credential warning); accept it to send the one mutation.
+form.requestSubmit();document.querySelector('dialog[open]').querySelectorAll('button')[1].click();await wait(150);
check(document.body.textContent.includes('outcome unknown')||document.body.textContent.includes('Outcome unknown'),'returned unknown stage remains visible');
const retry=document.querySelector('form[data-mutation]');
check(!!retry&&retry.querySelector('button').disabled,'200 unknown stage cannot immediately retry without observation');
if(retry)retry.requestSubmit();await wait();check(posts===1,'returned unknown stage sends no duplicate mutation');
+document.querySelectorAll('dialog[open]').forEach(d=>d.close());
const alternate=document.querySelector('form[action$="/restart"]');
alternate.requestSubmit();const dialog=document.querySelector('dialog[open]');
if(dialog)dialog.querySelectorAll('button')[1].click();await wait();
@@ -84,7 +86,7 @@
REPLACEMENT = r"""
let posts=0;
window.fetch=async()=>{posts++;return {ok:true,status:200,url:'https://fixture.invalid/tools?tool=notes-demo',text:async()=>window.RETURNED_PAGE};};
-document.querySelector('form[data-mutation]').requestSubmit();await wait(150);
+document.querySelector('form[data-mutation]').requestSubmit();document.querySelector('dialog[open]').querySelectorAll('button')[1].click();await wait(150);
const form=document.querySelector('form[data-confirm]');
check(!!form,'replacement keeps focused recovery form');
form.requestSubmit();check(!!document.querySelector('dialog[open]'),'document.write replacement initializes production scripts');
|