Toolstack security hardening

admin/README.md

@@ -123,10 +123,21 @@ neither process liveness nor health. `DELETE /api/tools/{id}` returns
 `deleted: true` for managed-folder deletion. Missing deletion metadata leaves the
 filesystem outcome unknown; other duplicate sources may still discover the tool.
 
-OpenAPI `auth_headers` are caller-envelope hints, not stored automatic injection.
-Preserve `allowed_headers` and secret declarations; callers supply
-`{{secret:NAME}}` placeholders in request headers. Never enter actual secret values
-in the operator UI.
+OpenAPI imports store authentication in each operation's operator-owned `headers`
+and `allowed_secrets`. The tool editor exposes those fields and an optional
+`body_template` for a fully operator-owned text body. References such as
+`{{secret:api_token}}` are permitted only in these templates and only for that
+operation's declared secret allowlist. Never enter actual secret values in the UI.
+Caller header values and bodies containing `{{secret:` are rejected with HTTP 400.
+`allowed_headers` is only for caller-provided headers and must exclude static
+header names. Caller bodies cannot override a body template.
+
+To migrate older tools, move placeholder-bearing headers/body into the operator
+templates, fill `allowed_secrets`, remove the static names from `allowed_headers`,
+and uncheck legacy caller substitution. Explicit `body_substitution = true` now
+fails validation; the default is false. Update caller envelopes to omit the
+operator-owned fields. See [REST forwarder](../docs/rest-forwarder.md).
+Forwarder refresh operations return status only, without names or value lengths.
 
 At source 855da6e (broker a9319b9 / release059dd13), see [integrated security contracts](../docs/security-contracts.md)
 for disclosure and remaining gates. Optional operation approval_fields is preserved
@@ -157,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

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)):
@@ -295,12 +303,15 @@ async def api_add_tool(request: Request, user: str = Depends(require_user)):
             if repo:
                 tool = await work.run(tool_sources.add_from_github,
                     repo, config.tools_root, existing,
-                    subdir=data.get("subdir", ""), ref=data.get("ref", ""))
+                    subdir=data.get("subdir", ""), ref=data.get("ref", ""),
+                    confirm_secret_items=data.get("confirm_secret_items") is True)
             elif manifest is not None:
                 # author-from-code-only: copy the folder + write the supplied manifest (no toolyard.toml needed)
-                tool = await work.run(tool_sources.add_with_manifest, source, config.tools_root, existing, manifest)
+                tool = await work.run(tool_sources.add_with_manifest, source, config.tools_root, existing, manifest,
+                                      confirm_secret_items=data.get("confirm_secret_items") is True)
             else:
-                tool = await work.run(tool_sources.add_from_path, source, config.tools_root, existing)
+                tool = await work.run(tool_sources.add_from_path, source, config.tools_root, existing,
+                                      confirm_secret_items=data.get("confirm_secret_items") is True)
         except tool_sources.NoManifest:
             raise HTTPException(status_code=422, detail="no toolyard.toml at that location; author one to add it")
         except ValueError as exc:
@@ -354,11 +365,12 @@ async def api_edit_tool(tool_id: str, request: Request, user: str = Depends(requ
                 raise HTTPException(status_code=400, detail="secrets must be a list")
             tool["secrets"] = data["secrets"]  # replace: sending `secrets` overwrites all declarations; omit to keep
         tool = tool_authoring.normalize(tool)
-        errors = tool_authoring.validate(tool)
+        errors = tool_authoring.validate(tool, confirm_secret_items=data.get("confirm_secret_items") is True)
         if errors:
             raise HTTPException(status_code=400, detail="; ".join(errors))
         try:
-            tool_authoring.write(dir_path, tool)
+            tool_authoring.write(dir_path, tool, confirm_secret_items=data.get("confirm_secret_items") is True)
+            toolyard_ops.record_edited_declarations(tool)
         except ValueError as exc:
             raise HTTPException(status_code=400, detail=str(exc))
         with open_store(config) as store:

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(

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:

admin/reconciler.py

@@ -58,7 +58,7 @@ def reconcile_running_tools(config, link, *, wait_timeout: float = 60.0) -> int:
                     continue
                 try:
                     toolyard_ops.start(tool_id, config.tools_root, config.tool_dirs,
-                                       backend=running.backend)
+                                       backend=running.backend, recovery=True)
                     started += 1
                     log.info("reconcile: restarted %s", tool_id)
                 except LookupError:

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
@@ -608,7 +626,7 @@ async def create_tool(request: Request):
             return HTMLResponse(views.tool_editor_view(
                 user=user, csrf=csrf_for(request), backend=settings.sps_info(), mode="new", tool=tool,
                 dir_value=dir_path, error="Invalid CSRF token."))
-        errors = list(tool_authoring.validate(tool))
+        errors = list(tool_authoring.validate(tool, confirm_secret_items=data.get("confirm_secret_items") == "1"))
         if not os.path.isabs(dir_path):
             errors.append("tool directory must be an absolute path")
         elif not Path(dir_path).is_dir():
@@ -622,7 +640,7 @@ async def create_tool(request: Request):
             return HTMLResponse(views.tool_editor_view(
                 user=user, csrf=csrf_for(request), backend=settings.sps_info(), mode="new", tool=tool,
                 dir_value=dir_path, error="; ".join(errors)))
-        tool_authoring.write(dir_path, tool)
+        tool_authoring.write(dir_path, tool, confirm_secret_items=data.get("confirm_secret_items") == "1")
         norm_dir = str(Path(dir_path))  # normalize so the later removable check matches
         if norm_dir not in config.tool_dirs:
             previous = replace(config)
@@ -675,12 +693,13 @@ async def save_tool(request: Request, tool_id: str):
             return HTMLResponse(views.tool_editor_view(
                 user=user, csrf=csrf_for(request), backend=settings.sps_info(), mode="edit", tool=tool,
                 dir_value=dir_path, error="Invalid CSRF token."))
-        errors = tool_authoring.validate(tool)
+        errors = tool_authoring.validate(tool, confirm_secret_items=data.get("confirm_secret_items") == "1")
         if errors:
             return HTMLResponse(views.tool_editor_view(
                 user=user, csrf=csrf_for(request), backend=settings.sps_info(), mode="edit", tool=tool,
                 dir_value=dir_path, error="; ".join(errors)))
-        tool_authoring.write(dir_path, tool)
+        tool_authoring.write(dir_path, tool, confirm_secret_items=data.get("confirm_secret_items") == "1")
+        toolyard_ops.record_edited_declarations(tool)
         with open_store(config) as store:
             operations.record_admin_event(store, user, "tool_edited", {"tool": tool["id"], "dir": dir_path})
         return render_dashboard(request, user, banner=(
@@ -735,9 +754,11 @@ def again(error: str) -> HTMLResponse:
             if data.get("kind") == "github":
                 tool = await work.run(tool_sources.add_from_github,
                     repo_val, config.tools_root, existing_ids,
-                    subdir=(data.get("subdir") or "").strip(), ref=(data.get("ref") or "").strip())
+                    subdir=(data.get("subdir") or "").strip(), ref=(data.get("ref") or "").strip(),
+                    confirm_secret_items=data.get("confirm_secret_items") == "1")
             else:
-                tool = await work.run(tool_sources.add_from_path, src_val, config.tools_root, existing_ids)
+                tool = await work.run(tool_sources.add_from_path, src_val, config.tools_root, existing_ids,
+                                      confirm_secret_items=data.get("confirm_secret_items") == "1")
         except tool_sources.NoManifest:
             return again("No toolyard.toml at that location; use “Author a tool” to create one.")
         except ValueError as exc:
@@ -775,7 +796,8 @@ async def update_tool_source(request: Request, tool_id: str):
             operations.record_admin_event(store, user, "tool_updated", {"tool": tool_id, "dir": tool["path"]})
         return render_dashboard(request, user, banner=(
             f"Updated tool {views.esc(tool_id)} from its source. Restart the broker if its entrypoint "
-            "or operations changed."))
+            "or operations changed." + (" Upstream secret declarations differ; local declarations were kept. Review them in Edit."
+                                       if tool.get("secret_declarations_changed") else "")))
 
     # JSON operator API (bearer-token auth) for native / automation clients: same ops,
     # JSON face. See admin/api.py (T-029).

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()

admin/sps_link.py

@@ -20,18 +20,13 @@
 import time
 
 from sps.client import SPSError, SPSClient
-from sps.config import load_config, parse_bool
+from toolyard.sps_config import load_registration_config as load_config, parse_verify
 
 log = logging.getLogger(__name__)
 
 _DEFAULT_SPS_ENV = "/etc/toolstack/sps.env"
 
 
-def parse_verify(value: str) -> bool:
-    """Use the integrated shared TLS verification contract."""
-    return parse_bool(value, name="TOOLSTACK_SPS_VERIFY")
-
-
 class SPSLink:
     def __init__(self, host: str, port: int, ca_file: str | None,
                  verify: bool = True) -> None:

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.