From dace53ea8a0884799bfe84b2de6875c5799491cf Mon Sep 17 00:00:00 2001 From: James Pichardo Date: Wed, 12 Aug 2026 08:36:35 -0400 Subject: [PATCH 1/7] fix(core): locate .env. by search, not by the process CWD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every server called load_dotenv(".env.") with a bare relative path, which resolves against whatever working directory the MCP client happened to have. Launching opencode from a subdirectory of the checkout (its shipped wiring is `uv run --directory .`) therefore started all nine servers with no credentials at all, and each failed with "Missing required environment variables" — an error that points at the credentials rather than at the launch context, so the failure reads as misconfiguration rather than as a path bug. core/auth/env.py resolves the file by searching instead: $F0_SECTOOLS_ENV_DIR, then the working directory and its parents, then the installed package's checkout. python-dotenv's override=False semantics are preserved, so an exported variable still beats the file and container/systemd deployments that supply credentials without any file are unaffected. The five duplicated "missing variables" raises in auth/config.py now share one helper that also reports which of the two problems occurred — file present but missing a key, or no file found — with the search hint. Values are never included; a test asserts the error cannot leak one. Verified live: the read-only Sentinel smoke run from skills/ now reaches the workspace and all seven tools return findings. A drift guard fails if a future server reintroduces the bare relative load. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ --- CLAUDE.md | 1 + core/f0_sectools_core/auth/config.py | 49 +++++--- core/f0_sectools_core/auth/env.py | 88 +++++++++++++ core/tests/test_auth_env.py | 119 ++++++++++++++++++ docs/user-guide/runtimes/opencode.md | 3 +- docs/user-guide/troubleshooting.md | 18 ++- scripts/live_smoke_defender.py | 6 +- scripts/live_smoke_entra.py | 6 +- scripts/live_smoke_intune.py | 6 +- scripts/live_smoke_limacharlie.py | 6 +- scripts/live_smoke_projectachilles.py | 6 +- scripts/live_smoke_projectachilles_actions.py | 6 +- scripts/live_smoke_purview.py | 6 +- scripts/live_smoke_sentinel.py | 6 +- scripts/live_smoke_tenable.py | 6 +- scripts/report_gather.py | 38 +++--- .../defender-mcp/f0_defender_mcp/server.py | 6 +- servers/entra-mcp/f0_entra_mcp/server.py | 6 +- servers/intune-mcp/f0_intune_mcp/server.py | 4 +- .../f0_limacharlie_mcp/server.py | 4 +- .../f0_pa_actions_mcp/server.py | 4 +- .../f0_projectachilles_mcp/server.py | 4 +- servers/purview-mcp/f0_purview_mcp/server.py | 4 +- .../sentinel-mcp/f0_sentinel_mcp/server.py | 4 +- servers/tenable-mcp/f0_tenable_mcp/server.py | 4 +- 25 files changed, 324 insertions(+), 86 deletions(-) create mode 100644 core/f0_sectools_core/auth/env.py create mode 100644 core/tests/test_auth_env.py diff --git a/CLAUDE.md b/CLAUDE.md index ffa2665..e127c10 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -243,6 +243,7 @@ Each integration follows `.env.` and the thin-server pattern. Read too ## Secrets & Privacy - **Per-platform `.env`.** `.env.wazuh`, `.env.defender`, `.env.entra`, … Each server loads only its own. All `.env*` files are gitignored. +- **Credential files are located by search, never by CWD.** Servers and scripts call `core/auth/env.py`'s `load_platform_env("")`, which searches the working directory and its parents, then the installed package's checkout, with `$F0_SECTOOLS_ENV_DIR` as an explicit override. A bare `load_dotenv(".env.")` is a defect — it silently loads nothing whenever the MCP client is launched from anywhere but the repo root, and a test in `core/tests/test_auth_env.py` guards against reintroducing it. - **Nothing leaves the host.** No telemetry, no analytics, no external calls except to the operator's own configured security platforms. - **Secrets never reach the model.** Credentials live in `core/auth/`; they are used to make API calls and are never placed in tool output, prompts, or model context. - **Redaction is mandatory and centralized.** Every return path goes through `core/redaction/`, including error/exception paths. diff --git a/core/f0_sectools_core/auth/config.py b/core/f0_sectools_core/auth/config.py index 093419a..1efbd12 100644 --- a/core/f0_sectools_core/auth/config.py +++ b/core/f0_sectools_core/auth/config.py @@ -7,12 +7,39 @@ from __future__ import annotations import os -from collections.abc import Mapping +from collections.abc import Iterable, Mapping from dataclasses import dataclass, field +from .env import find_platform_env + _TRUE = {"1", "true", "yes", "on"} +def _require_env(prefix: str, names: Iterable[str], env: Mapping[str, str]) -> None: + """Raise if any required variable is unset, saying where credentials were looked for. + + The variable names alone are a dead end: the usual cause is not a missing + key but a credential file that was never located, which looks identical + from the caller's side. Naming the file and its search result turns a + guessing game into a one-line fix. Values are never included. + """ + missing = [name for name in names if not env.get(name)] + if not missing: + return + platform = prefix.lower() + found = find_platform_env(platform) + where = ( + f"found .env.{platform} in {found.parent} - add the missing keys there" + if found + else ( + f"no .env.{platform} was found in the working directory, its parents, " + "or the installed package's checkout - create one in the repo root " + "or point F0_SECTOOLS_ENV_DIR at it" + ) + ) + raise ValueError(f"Missing required environment variables: {', '.join(missing)} ({where})") + + @dataclass class PlatformConfig: tenant_id: str @@ -26,9 +53,7 @@ class PlatformConfig: def from_env(cls, prefix: str, env: Mapping[str, str] | None = None) -> PlatformConfig: env = env if env is not None else os.environ required = {k: f"{prefix}_{k.upper()}" for k in ("tenant_id", "client_id", "client_secret")} - missing = [name for name in required.values() if not env.get(name)] - if missing: - raise ValueError(f"Missing required environment variables: {', '.join(missing)}") + _require_env(prefix, required.values(), env) verify = env.get(f"{prefix}_VERIFY_TLS", "true").strip().lower() in _TRUE allow_write = env.get(f"{prefix}_ALLOW_WRITE", "false").strip().lower() in _TRUE return cls( @@ -58,9 +83,7 @@ def from_env( ) -> LimaCharlieConfig: env = env if env is not None else os.environ required = {"oid": f"{prefix}_OID", "api_key": f"{prefix}_API_KEY"} - missing = [name for name in required.values() if not env.get(name)] - if missing: - raise ValueError(f"Missing required environment variables: {', '.join(missing)}") + _require_env(prefix, required.values(), env) allow_write = env.get(f"{prefix}_ALLOW_WRITE", "false").strip().lower() in _TRUE return cls( oid=env[required["oid"]], @@ -90,9 +113,7 @@ def from_env( ) -> ProjectAchillesConfig: env = env if env is not None else os.environ required = {"base_url": f"{prefix}_BASE_URL", "api_key": f"{prefix}_API_KEY"} - missing = [name for name in required.values() if not env.get(name)] - if missing: - raise ValueError(f"Missing required environment variables: {', '.join(missing)}") + _require_env(prefix, required.values(), env) verify = env.get(f"{prefix}_VERIFY_TLS", "true").strip().lower() in _TRUE allow_write = env.get(f"{prefix}_ALLOW_WRITE", "false").strip().lower() in _TRUE confirm_mode = env.get(f"{prefix}_CONFIRM_MODE", "token").strip().lower() @@ -132,9 +153,7 @@ def from_env( "access_key": f"{prefix}_ACCESS_KEY", "secret_key": f"{prefix}_SECRET_KEY", } - missing = [name for name in required.values() if not env.get(name)] - if missing: - raise ValueError(f"Missing required environment variables: {', '.join(missing)}") + _require_env(prefix, required.values(), env) verify = env.get(f"{prefix}_VERIFY_TLS", "true").strip().lower() in _TRUE base_url = env.get(f"{prefix}_BASE_URL", "https://cloud.tenable.com").rstrip("/") return cls( @@ -182,9 +201,7 @@ def from_env( k: f"{prefix}_{k.upper()}" for k in ("tenant_id", "client_id", "client_secret", "workspace_id") } - missing = [name for name in required.values() if not env.get(name)] - if missing: - raise ValueError(f"Missing required environment variables: {', '.join(missing)}") + _require_env(prefix, required.values(), env) try: retention = int(env.get(f"{prefix}_RETENTION_DAYS", "30")) except ValueError: diff --git a/core/f0_sectools_core/auth/env.py b/core/f0_sectools_core/auth/env.py new file mode 100644 index 0000000..4473792 --- /dev/null +++ b/core/f0_sectools_core/auth/env.py @@ -0,0 +1,88 @@ +"""Locate a platform's ``.env`` file by searching, not by trusting the working directory. + +An MCP client starts a server with whatever working directory it happens to +have: opencode launched from a subdirectory of the checkout, a systemd unit +from ``/``, a desktop client from ``$HOME``. A bare +``load_dotenv(".env.defender")`` resolves against that directory, so it +silently loads nothing and the server fails later with an opaque "missing +environment variables" error that points at the credentials rather than at +the launch context. + +Resolving by search makes the credential location a property of the checkout +instead of a property of how the client was launched. + +Search order, highest precedence first: + +1. ``$F0_SECTOOLS_ENV_DIR`` -- an explicit operator override, for keeping + credentials outside the checkout entirely. +2. The working directory and each of its ancestors. +3. The installed package's directory and each of its ancestors -- reached + when the checkout is nowhere near the working directory at all. + +A file that is *not* found is not an error: supplying credentials as real +environment variables is a supported deployment, and ``python-dotenv`` never +overwrites a variable that is already set, so the surrounding environment +always wins over the file. + +Secrets read here enter this process's environment only. They are never +logged, never returned in tool output, and never placed in model context. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +from dotenv import load_dotenv + +__all__ = ["env_search_dirs", "find_platform_env", "load_platform_env"] + + +def env_search_dirs() -> list[Path]: + """Directories searched for a credential file, highest precedence first.""" + candidates: list[Path] = [] + + override = os.environ.get("F0_SECTOOLS_ENV_DIR") + if override: + candidates.append(Path(override).expanduser()) + + try: + cwd = Path.cwd() + except OSError: # working directory deleted out from under the process + cwd = None + if cwd is not None: + candidates.extend([cwd, *cwd.parents]) + + package = Path(__file__).resolve() + candidates.extend(package.parents) + + seen: set[Path] = set() + ordered: list[Path] = [] + for directory in candidates: + if directory not in seen: + seen.add(directory) + ordered.append(directory) + return ordered + + +def find_platform_env(platform: str) -> Path | None: + """Return the path to ``.env.``, or None if no checkout holds one.""" + filename = f".env.{platform}" + for directory in env_search_dirs(): + candidate = directory / filename + if candidate.is_file(): + return candidate + return None + + +def load_platform_env(platform: str) -> Path | None: + """Load ``.env.`` into the environment; return where it came from. + + Returns None when no file exists, leaving the surrounding environment as + the credential source. Callers use the return value for diagnostics only + -- never log or return the file's *contents*. + """ + path = find_platform_env(platform) + if path is not None: + load_dotenv(path) + return path diff --git a/core/tests/test_auth_env.py b/core/tests/test_auth_env.py new file mode 100644 index 0000000..74271a6 --- /dev/null +++ b/core/tests/test_auth_env.py @@ -0,0 +1,119 @@ +"""Credential files must be found by where the checkout is, not by how the client was launched.""" +import os +from pathlib import Path + +import pytest +from f0_sectools_core.auth.env import env_search_dirs, find_platform_env, load_platform_env + +# A platform name no real .env.* uses, so the developer's own checkout can +# never satisfy a test that asserts "nothing found". +FAKE = "acmecorp" + + +@pytest.fixture +def checkout(tmp_path, monkeypatch): + """A fake checkout with a credential file at its root and a nested subdir.""" + (tmp_path / f".env.{FAKE}").write_text(f"{FAKE.upper()}_TOKEN=from-file\n") + deep = tmp_path / "skills" / "sentinel" / "detection-coverage" + deep.mkdir(parents=True) + monkeypatch.delenv("F0_SECTOOLS_ENV_DIR", raising=False) + monkeypatch.delenv(f"{FAKE.upper()}_TOKEN", raising=False) + return tmp_path, deep + + +def test_finds_env_file_when_cwd_is_a_subdirectory(checkout, monkeypatch): + """The regression: opencode launched from skills/ left every server credential-less.""" + root, deep = checkout + monkeypatch.chdir(deep) + assert find_platform_env(FAKE) == root / f".env.{FAKE}" + + +def test_load_populates_environment_from_a_subdirectory(checkout, monkeypatch): + root, deep = checkout + monkeypatch.chdir(deep) + assert load_platform_env(FAKE) == root / f".env.{FAKE}" + assert os.environ[f"{FAKE.upper()}_TOKEN"] == "from-file" + + +def test_missing_file_is_not_an_error(tmp_path, monkeypatch): + """Credentials supplied as real env vars (container, systemd) is a supported deployment.""" + monkeypatch.delenv("F0_SECTOOLS_ENV_DIR", raising=False) + monkeypatch.chdir(tmp_path) + assert load_platform_env(FAKE) is None + + +def test_explicit_override_wins(checkout, monkeypatch, tmp_path_factory): + """F0_SECTOOLS_ENV_DIR relocates credentials off the checkout entirely.""" + root, deep = checkout + elsewhere = tmp_path_factory.mktemp("vault") + (elsewhere / f".env.{FAKE}").write_text(f"{FAKE.upper()}_TOKEN=from-vault\n") + monkeypatch.setenv("F0_SECTOOLS_ENV_DIR", str(elsewhere)) + monkeypatch.chdir(deep) + assert load_platform_env(FAKE) == elsewhere / f".env.{FAKE}" + assert os.environ[f"{FAKE.upper()}_TOKEN"] == "from-vault" + + +def test_real_environment_beats_the_file(checkout, monkeypatch): + """A var already exported must not be clobbered by the file (dotenv override=False).""" + root, deep = checkout + monkeypatch.chdir(deep) + monkeypatch.setenv(f"{FAKE.upper()}_TOKEN", "from-shell") + load_platform_env(FAKE) + assert os.environ[f"{FAKE.upper()}_TOKEN"] == "from-shell" + + +def test_search_dirs_are_ordered_and_deduplicated(checkout, monkeypatch): + root, deep = checkout + monkeypatch.chdir(deep) + dirs = env_search_dirs() + assert dirs[0] == deep, "the working directory is searched first" + assert len(dirs) == len(set(dirs)), "no directory is searched twice" + assert root in dirs + + +def test_no_server_loads_dotenv_by_a_bare_relative_path(): + """Drift guard: server #10 must not reintroduce the CWD dependency.""" + repo = Path(__file__).resolve().parents[2] + offenders = [ + f"{p.relative_to(repo)}:{n}" + for p in sorted(repo.glob("servers/*/*/server.py")) + for n, line in enumerate(p.read_text().splitlines(), 1) + if 'load_dotenv(".env.' in line + ] + assert offenders == [], f"use load_platform_env() from core: {offenders}" + + +def test_missing_vars_error_points_at_the_absent_credential_file(tmp_path, monkeypatch): + """The message that sent a local model into a debugging loop must name the file.""" + from f0_sectools_core.auth.config import PlatformConfig + + monkeypatch.delenv("F0_SECTOOLS_ENV_DIR", raising=False) + monkeypatch.chdir(tmp_path) + with pytest.raises(ValueError) as e: + PlatformConfig.from_env(FAKE.upper(), env={}) + message = str(e.value) + assert f"{FAKE.upper()}_TENANT_ID" in message, "still lists the variable names" + assert f"no .env.{FAKE} was found" in message + assert "F0_SECTOOLS_ENV_DIR" in message + + +def test_missing_vars_error_points_at_a_present_credential_file(checkout, monkeypatch): + from f0_sectools_core.auth.config import PlatformConfig + + root, deep = checkout + monkeypatch.chdir(deep) + with pytest.raises(ValueError) as e: + PlatformConfig.from_env(FAKE.upper(), env={}) + assert f"found .env.{FAKE} in {root}" in str(e.value) + + +def test_missing_vars_error_never_leaks_a_value(checkout, monkeypatch): + """Critical Rule 2: an error path is still an output path.""" + from f0_sectools_core.auth.config import PlatformConfig + + root, deep = checkout + (root / f".env.{FAKE}").write_text(f"{FAKE.upper()}_CLIENT_SECRET=super-secret-value\n") + monkeypatch.chdir(deep) + with pytest.raises(ValueError) as e: + PlatformConfig.from_env(FAKE.upper(), env={}) + assert "super-secret-value" not in str(e.value) diff --git a/docs/user-guide/runtimes/opencode.md b/docs/user-guide/runtimes/opencode.md index efa185d..2363541 100644 --- a/docs/user-guide/runtimes/opencode.md +++ b/docs/user-guide/runtimes/opencode.md @@ -92,7 +92,8 @@ Supervised sessions only. - **`opencode mcp list` shows a server failed** — run the server directly (`uv run --directory . f0-defender-mcp`) to see the startup error; usually a - missing `.env.` (servers resolve them from the repo root). + missing `.env.` (servers search the working directory upward, so + a checkout-root file is found even when you start opencode in a subdirectory). - **Skills missing from `opencode debug skill`** — that CLI snapshot races the skill scan and shows a partial list (observed on 1.18.4); a real session sees all skills. Trust an in-session check ("list your available skills"), diff --git a/docs/user-guide/troubleshooting.md b/docs/user-guide/troubleshooting.md index cac6812..3a76244 100644 --- a/docs/user-guide/troubleshooting.md +++ b/docs/user-guide/troubleshooting.md @@ -37,9 +37,21 @@ retry once** — don't hammer it, which refreshes the throttle window. ## "Missing required environment variables" -The server couldn't find its credentials. Ensure `.env.defender` / `.env.entra` -exist at the **repo root** and the runtime launches the server with -`uv run --directory ` (so it loads them). +The server couldn't find a credential it needs. The error names the missing +variables *and* where it looked, which tells you which of two problems you have: + +- **`found .env. in `** — the file exists but is missing a key. + Add it there. Compare against + `servers/-mcp/.env..example`. +- **`no .env. was found`** — there is no credential file. Create one + at the repo root, or set `F0_SECTOOLS_ENV_DIR` to the directory holding your + `.env.*` files (useful for keeping credentials outside the checkout). + +Servers locate `.env.` by searching the working directory and its +parents, then the installed package's checkout — so launching your runtime from +a subdirectory of the repo works. Variables already exported in the environment +always win over the file, which is how container and systemd deployments supply +credentials without a file at all. ## Tool not found / wrong name diff --git a/scripts/live_smoke_defender.py b/scripts/live_smoke_defender.py index a5ddc29..aed5e7e 100644 --- a/scripts/live_smoke_defender.py +++ b/scripts/live_smoke_defender.py @@ -1,6 +1,6 @@ """Live smoke test for the Defender MCP server against a real tenant. -Usage (from the repo root): +Usage: 1. Copy servers/defender-mcp/.env.defender.example to ./.env.defender and fill in DEFENDER_TENANT_ID / DEFENDER_CLIENT_ID / DEFENDER_CLIENT_SECRET. 2. uv run python scripts/live_smoke_defender.py @@ -15,14 +15,14 @@ import asyncio import json -from dotenv import load_dotenv from f0_defender_mcp import tools from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.renderers import Persona, render_findings -load_dotenv(".env.defender") +load_platform_env("defender") # A harmless, bounded hunting query to validate ThreatHunting.Read.All. SMOKE_KQL = "DeviceInfo | take 1" diff --git a/scripts/live_smoke_entra.py b/scripts/live_smoke_entra.py index b02d61b..2afc716 100644 --- a/scripts/live_smoke_entra.py +++ b/scripts/live_smoke_entra.py @@ -1,6 +1,6 @@ """Live smoke test for the Entra MCP server against a real tenant. -Usage (from the repo root): +Usage: 1. Copy servers/entra-mcp/.env.entra.example to ./.env.entra and fill in ENTRA_TENANT_ID / ENTRA_CLIENT_ID / ENTRA_CLIENT_SECRET. 2. uv run python scripts/live_smoke_entra.py @@ -14,13 +14,13 @@ import asyncio import json -from dotenv import load_dotenv from f0_entra_mcp import tools from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.redaction.redact import redact_finding -load_dotenv(".env.entra") +load_platform_env("entra") def _show(label: str, findings) -> None: diff --git a/scripts/live_smoke_intune.py b/scripts/live_smoke_intune.py index 54fc19a..4429ff5 100644 --- a/scripts/live_smoke_intune.py +++ b/scripts/live_smoke_intune.py @@ -1,6 +1,6 @@ """Live smoke test for the Intune MCP server against a real tenant. -Usage (from the repo root): +Usage: 1. Copy servers/intune-mcp/.env.intune.example to ./.env.intune and fill in INTUNE_TENANT_ID / INTUNE_CLIENT_ID / INTUNE_CLIENT_SECRET (an Entra app with DeviceManagementManagedDevices.Read.All + DeviceManagementConfiguration.Read.All). @@ -15,13 +15,13 @@ import asyncio import json -from dotenv import load_dotenv from f0_intune_mcp import tools from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.redaction.redact import redact_finding -load_dotenv(".env.intune") +load_platform_env("intune") def _show(label: str, findings) -> None: diff --git a/scripts/live_smoke_limacharlie.py b/scripts/live_smoke_limacharlie.py index 88299e4..70f55c9 100644 --- a/scripts/live_smoke_limacharlie.py +++ b/scripts/live_smoke_limacharlie.py @@ -1,6 +1,6 @@ """Live smoke test for the LimaCharlie MCP server against a real org. -Usage (from the repo root): +Usage: 1. Copy servers/limacharlie-mcp/.env.limacharlie.example to ./.env.limacharlie and fill in LIMACHARLIE_OID / LIMACHARLIE_API_KEY. 2. uv run python scripts/live_smoke_limacharlie.py @@ -14,13 +14,13 @@ import json import os -from dotenv import load_dotenv from f0_limacharlie_mcp import tools from f0_limacharlie_mcp.client import LimaCharlieClient from f0_sectools_core.auth.config import LimaCharlieConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding -load_dotenv(".env.limacharlie") +load_platform_env("limacharlie") # Optional: set LIMACHARLIE_SMOKE_HOSTNAME to a real sensor to exercise host-scoping. # Empty -> query_telemetry runs unscoped (all sensors), so the step still runs. diff --git a/scripts/live_smoke_projectachilles.py b/scripts/live_smoke_projectachilles.py index 65f09be..585593f 100644 --- a/scripts/live_smoke_projectachilles.py +++ b/scripts/live_smoke_projectachilles.py @@ -1,6 +1,6 @@ """Live smoke test for the ProjectAchilles MCP server against a real instance. -Usage (from the repo root): +Usage: 1. Copy servers/projectachilles-mcp/.env.projectachilles.example to ./.env.projectachilles and fill in PROJECTACHILLES_BASE_URL and a read-scope PROJECTACHILLES_API_KEY (pa_...). @@ -23,13 +23,13 @@ import asyncio import json -from dotenv import load_dotenv from f0_projectachilles_mcp import tools from f0_projectachilles_mcp.client import ProjectAchillesClient from f0_sectools_core.auth.config import ProjectAchillesConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding, redact_obj -load_dotenv(".env.projectachilles") +load_platform_env("projectachilles") def _show(label: str, findings) -> None: diff --git a/scripts/live_smoke_projectachilles_actions.py b/scripts/live_smoke_projectachilles_actions.py index 36ae1fe..45917a0 100644 --- a/scripts/live_smoke_projectachilles_actions.py +++ b/scripts/live_smoke_projectachilles_actions.py @@ -1,6 +1,6 @@ """Live smoke test for the ProjectAchilles ACTIONS server against a real instance. -Usage (from the repo root): +Usage: 1. Ensure ./.env.projectachilles has PROJECTACHILLES_BASE_URL and a READ-WRITE-scope PROJECTACHILLES_API_KEY (pa_...). Writes additionally need PROJECTACHILLES_ALLOW_WRITE=true. @@ -20,14 +20,14 @@ import asyncio import json -from dotenv import load_dotenv from f0_pa_actions_mcp import tools from f0_pa_actions_mcp.client import ProjectAchillesClient from f0_sectools_core.auth.config import ProjectAchillesConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.gating.actions import AuditLog, GatedAction, TokenStore from f0_sectools_core.redaction.redact import redact_finding -load_dotenv(".env.projectachilles") +load_platform_env("projectachilles") def _show(label: str, findings) -> None: diff --git a/scripts/live_smoke_purview.py b/scripts/live_smoke_purview.py index 181f75e..6797255 100644 --- a/scripts/live_smoke_purview.py +++ b/scripts/live_smoke_purview.py @@ -1,6 +1,6 @@ """Live smoke test for the Purview MCP server against a real tenant. -Usage (from the repo root): +Usage: 1. Copy servers/purview-mcp/.env.purview.example to ./.env.purview and fill in PURVIEW_TENANT_ID / PURVIEW_CLIENT_ID / PURVIEW_CLIENT_SECRET. Required app permissions: SecurityAlert.Read.All, AuditLogsQuery.Read.All, @@ -15,13 +15,13 @@ import asyncio import json -from dotenv import load_dotenv from f0_purview_mcp import tools from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.redaction.redact import redact_finding -load_dotenv(".env.purview") +load_platform_env("purview") def _show(label: str, findings) -> None: diff --git a/scripts/live_smoke_sentinel.py b/scripts/live_smoke_sentinel.py index c081216..122c1dd 100644 --- a/scripts/live_smoke_sentinel.py +++ b/scripts/live_smoke_sentinel.py @@ -1,6 +1,6 @@ """Live smoke test for the Sentinel MCP server against a real workspace. -Usage (from the repo root): +Usage: 1. Copy servers/sentinel-mcp/.env.sentinel.example to ./.env.sentinel and fill it in. Required Azure roles on the Log Analytics workspace resource: Log Analytics Reader (all telemetry tools) and, optionally, Microsoft @@ -34,8 +34,8 @@ import asyncio import json -from dotenv import load_dotenv from f0_sectools_core.auth.config import SentinelConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.renderers import Persona, render_findings from f0_sentinel_mcp import tools @@ -43,7 +43,7 @@ # Mirrors every other live_smoke_*.py script: reads local env vars into this # process only. Never printed, logged, or otherwise surfaced below. -load_dotenv(".env.sentinel") +load_platform_env("sentinel") def _show(label: str, findings, persona: str | None = None, show: int = 6) -> None: diff --git a/scripts/live_smoke_tenable.py b/scripts/live_smoke_tenable.py index 96bad8d..70ba67d 100644 --- a/scripts/live_smoke_tenable.py +++ b/scripts/live_smoke_tenable.py @@ -1,6 +1,6 @@ """Live smoke test for the Tenable MCP server against a real Tenable VM instance. -Usage (from the repo root): +Usage: 1. Copy servers/tenable-mcp/.env.tenable.example to ./.env.tenable and fill in TENABLE_ACCESS_KEY and TENABLE_SECRET_KEY. 2. uv run python scripts/live_smoke_tenable.py [--persona ciso] @@ -15,14 +15,14 @@ import asyncio import json -from dotenv import load_dotenv from f0_sectools_core.auth.config import TenableConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.renderers import Persona, render_findings from f0_tenable_mcp import tools from f0_tenable_mcp.client import TenableClient -load_dotenv(".env.tenable") +load_platform_env("tenable") def _show(label: str, findings, persona: str | None = None) -> None: diff --git a/scripts/report_gather.py b/scripts/report_gather.py index 739f1d9..7e79a59 100644 --- a/scripts/report_gather.py +++ b/scripts/report_gather.py @@ -9,7 +9,7 @@ import asyncio from collections.abc import Awaitable, Callable -from dotenv import load_dotenv +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding, redact_text from f0_sectools_core.reports.content import MetricCard, ScopeMeta from f0_sectools_core.reports.i18n import group_label @@ -60,7 +60,7 @@ async def _pillar_config_hardening(window_hours: int) -> list[Finding]: from f0_defender_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.defender") + load_platform_env("defender") cfg = PlatformConfig.from_env("DEFENDER") async with GraphClient(cfg) as gc: return await tools.get_secure_score(gc) @@ -70,7 +70,7 @@ async def _pillar_vuln_exposure(window_hours: int) -> list[Finding]: from f0_sectools_core.auth.config import TenableConfig from f0_tenable_mcp import tools from f0_tenable_mcp.client import TenableClient - load_dotenv(".env.tenable") + load_platform_env("tenable") async with TenableClient(TenableConfig.from_env()) as tio: return await tools.get_vulnerability_summary(tio) @@ -79,7 +79,7 @@ async def _pillar_device_compliance(window_hours: int) -> list[Finding]: from f0_intune_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.intune") + load_platform_env("intune") cfg = PlatformConfig.from_env("INTUNE") async with GraphClient(cfg) as gc: return await tools.get_compliance_summary(gc) @@ -89,7 +89,7 @@ async def _pillar_data_risk(window_hours: int) -> list[Finding]: from f0_purview_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.purview") + load_platform_env("purview") cfg = PlatformConfig.from_env("PURVIEW") async with GraphClient(cfg) as gc: return await tools.get_dlp_summary(gc, hours_back=window_hours) @@ -99,7 +99,7 @@ async def _pillar_attack_validation(window_hours: int) -> list[Finding]: from f0_projectachilles_mcp import tools from f0_projectachilles_mcp.client import ProjectAchillesClient from f0_sectools_core.auth.config import ProjectAchillesConfig - load_dotenv(".env.projectachilles") + load_platform_env("projectachilles") async with ProjectAchillesClient(ProjectAchillesConfig.from_env()) as pa: return await tools.get_defense_score(pa) @@ -108,7 +108,7 @@ async def _pillar_endpoint_coverage(window_hours: int) -> list[Finding]: from f0_limacharlie_mcp import tools from f0_limacharlie_mcp.client import LimaCharlieClient from f0_sectools_core.auth.config import LimaCharlieConfig - load_dotenv(".env.limacharlie") + load_platform_env("limacharlie") lc = LimaCharlieClient(LimaCharlieConfig.from_env()) return await asyncio.to_thread(tools.get_org_overview, lc) @@ -117,7 +117,7 @@ async def _pillar_detection_coverage(window_hours: int) -> list[Finding]: from f0_sectools_core.auth.config import SentinelConfig from f0_sentinel_mcp import tools from f0_sentinel_mcp.client import SentinelClient - load_dotenv(".env.sentinel") + load_platform_env("sentinel") cfg = SentinelConfig.from_env("SENTINEL") async with SentinelClient(cfg) as c: findings = await tools.get_detection_coverage(c) @@ -135,7 +135,7 @@ async def _defender_alerts(window_hours: int) -> list[Finding]: from f0_defender_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.defender") + load_platform_env("defender") cfg = PlatformConfig.from_env("DEFENDER") async with GraphClient(cfg) as gc: return _within_window( @@ -147,7 +147,7 @@ async def _defender_incidents(window_hours: int) -> list[Finding]: from f0_defender_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.defender") + load_platform_env("defender") cfg = PlatformConfig.from_env("DEFENDER") async with GraphClient(cfg) as gc: return _within_window( @@ -159,7 +159,7 @@ async def _lc_dr_rules(window_hours: int) -> list[Finding]: from f0_limacharlie_mcp import tools from f0_limacharlie_mcp.client import LimaCharlieClient from f0_sectools_core.auth.config import LimaCharlieConfig - load_dotenv(".env.limacharlie") + load_platform_env("limacharlie") lc = LimaCharlieClient(LimaCharlieConfig.from_env()) return await asyncio.to_thread(tools.list_dr_rules, lc, "general", 15) @@ -168,7 +168,7 @@ async def _lc_detections(window_hours: int) -> list[Finding]: from f0_limacharlie_mcp import tools from f0_limacharlie_mcp.client import LimaCharlieClient from f0_sectools_core.auth.config import LimaCharlieConfig - load_dotenv(".env.limacharlie") + load_platform_env("limacharlie") lc = LimaCharlieClient(LimaCharlieConfig.from_env()) return await asyncio.to_thread(tools.list_detections, lc, float(window_hours), 15) @@ -177,7 +177,7 @@ async def _sentinel_analytics_rules(window_hours: int) -> list[Finding]: from f0_sectools_core.auth.config import SentinelConfig from f0_sentinel_mcp import tools from f0_sentinel_mcp.client import SentinelClient - load_dotenv(".env.sentinel") + load_platform_env("sentinel") cfg = SentinelConfig.from_env("SENTINEL") async with SentinelClient(cfg) as c: # Full result, including the per-rule findings the CISO pillar above @@ -190,7 +190,7 @@ async def _pa_weak_techniques(window_hours: int) -> list[Finding]: from f0_projectachilles_mcp import tools from f0_projectachilles_mcp.client import ProjectAchillesClient from f0_sectools_core.auth.config import ProjectAchillesConfig - load_dotenv(".env.projectachilles") + load_platform_env("projectachilles") async with ProjectAchillesClient(ProjectAchillesConfig.from_env()) as pa: return await tools.get_weak_techniques(pa, days=max(1, window_hours // 24), limit=10) @@ -200,7 +200,7 @@ async def _entra_conditional_access(window_hours: int) -> list[Finding]: from f0_entra_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.entra") + load_platform_env("entra") cfg = PlatformConfig.from_env("ENTRA") async with GraphClient(cfg) as gc: # list_conditional_access_policies has no limit param (it pages unbounded), @@ -213,7 +213,7 @@ async def _entra_privileged_roles(window_hours: int) -> list[Finding]: from f0_entra_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.entra") + load_platform_env("entra") cfg = PlatformConfig.from_env("ENTRA") async with GraphClient(cfg) as gc: return await tools.list_privileged_role_assignments(gc, limit=10) @@ -223,7 +223,7 @@ async def _entra_risky_users(window_hours: int) -> list[Finding]: from f0_entra_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.entra") + load_platform_env("entra") cfg = PlatformConfig.from_env("ENTRA") async with GraphClient(cfg) as gc: return await tools.list_risky_users(gc, limit=10) @@ -233,7 +233,7 @@ async def _intune_stale_devices(window_hours: int) -> list[Finding]: from f0_intune_mcp import tools from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.graph import GraphClient - load_dotenv(".env.intune") + load_platform_env("intune") cfg = PlatformConfig.from_env("INTUNE") async with GraphClient(cfg) as gc: # Deliberately NOT window_hours: "stale" is defined by the tool's own @@ -245,7 +245,7 @@ async def _tenable_top_vulns(window_hours: int) -> list[Finding]: from f0_sectools_core.auth.config import TenableConfig from f0_tenable_mcp import tools from f0_tenable_mcp.client import TenableClient - load_dotenv(".env.tenable") + load_platform_env("tenable") async with TenableClient(TenableConfig.from_env()) as tio: return await tools.list_top_vulnerabilities(tio, limit=10) diff --git a/servers/defender-mcp/f0_defender_mcp/server.py b/servers/defender-mcp/f0_defender_mcp/server.py index 395130d..2fbac94 100644 --- a/servers/defender-mcp/f0_defender_mcp/server.py +++ b/servers/defender-mcp/f0_defender_mcp/server.py @@ -11,8 +11,8 @@ import os from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.gating.actions import AuditLog, GatedAction, TokenStore from f0_sectools_core.redaction.redact import redact_finding @@ -21,8 +21,8 @@ from . import tools -# Load .env.defender from the working directory if present (no-op otherwise). -load_dotenv(".env.defender") +# Locate .env.defender by searching upward from the working directory (no-op if absent). +load_platform_env("defender") mcp = MCPServer("f0-defender") diff --git a/servers/entra-mcp/f0_entra_mcp/server.py b/servers/entra-mcp/f0_entra_mcp/server.py index 2ed6e4c..530e433 100644 --- a/servers/entra-mcp/f0_entra_mcp/server.py +++ b/servers/entra-mcp/f0_entra_mcp/server.py @@ -8,8 +8,8 @@ from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding @@ -17,8 +17,8 @@ from . import tools -# Load .env.entra from the working directory if present (no-op otherwise). -load_dotenv(".env.entra") +# Locate .env.entra by searching upward from the working directory (no-op if absent). +load_platform_env("entra") mcp = MCPServer("f0-entra") diff --git a/servers/intune-mcp/f0_intune_mcp/server.py b/servers/intune-mcp/f0_intune_mcp/server.py index 02bb430..c25c078 100644 --- a/servers/intune-mcp/f0_intune_mcp/server.py +++ b/servers/intune-mcp/f0_intune_mcp/server.py @@ -8,8 +8,8 @@ from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding @@ -17,7 +17,7 @@ from . import tools -load_dotenv(".env.intune") +load_platform_env("intune") mcp = MCPServer("f0-intune") diff --git a/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py b/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py index c9bf793..c0ea122 100644 --- a/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py +++ b/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py @@ -8,8 +8,8 @@ import asyncio from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import LimaCharlieConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -17,7 +17,7 @@ from . import tools from .client import LimaCharlieClient -load_dotenv(".env.limacharlie") +load_platform_env("limacharlie") mcp = MCPServer("f0-limacharlie") diff --git a/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py b/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py index 30ce516..6d046b5 100644 --- a/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py +++ b/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py @@ -10,8 +10,8 @@ import os from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import ProjectAchillesConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.gating.actions import AuditLog, GatedAction, TokenStore from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding @@ -20,7 +20,7 @@ from . import tools from .client import ProjectAchillesClient -load_dotenv(".env.projectachilles") +load_platform_env("projectachilles") mcp = MCPServer("f0-pa-actions") diff --git a/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py b/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py index 38b1db8..8582ccf 100644 --- a/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py +++ b/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py @@ -6,8 +6,8 @@ from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import ProjectAchillesConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -15,7 +15,7 @@ from . import tools from .client import ProjectAchillesClient -load_dotenv(".env.projectachilles") +load_platform_env("projectachilles") mcp = MCPServer("f0-projectachilles") diff --git a/servers/purview-mcp/f0_purview_mcp/server.py b/servers/purview-mcp/f0_purview_mcp/server.py index 30ab328..e3f5265 100644 --- a/servers/purview-mcp/f0_purview_mcp/server.py +++ b/servers/purview-mcp/f0_purview_mcp/server.py @@ -8,8 +8,8 @@ from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import PlatformConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding @@ -17,7 +17,7 @@ from . import tools -load_dotenv(".env.purview") +load_platform_env("purview") mcp = MCPServer("f0-purview") diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/server.py b/servers/sentinel-mcp/f0_sentinel_mcp/server.py index d764a8f..2dcb3c0 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/server.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/server.py @@ -8,8 +8,8 @@ from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import SentinelConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -17,7 +17,7 @@ from . import tools from .client import SentinelClient -load_dotenv(".env.sentinel") +load_platform_env("sentinel") mcp = MCPServer("f0-sentinel") diff --git a/servers/tenable-mcp/f0_tenable_mcp/server.py b/servers/tenable-mcp/f0_tenable_mcp/server.py index 7ed1bc1..3b4f5f2 100644 --- a/servers/tenable-mcp/f0_tenable_mcp/server.py +++ b/servers/tenable-mcp/f0_tenable_mcp/server.py @@ -6,8 +6,8 @@ from typing import Any, Literal -from dotenv import load_dotenv from f0_sectools_core.auth.config import TenableConfig +from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -15,7 +15,7 @@ from . import tools from .client import TenableClient -load_dotenv(".env.tenable") +load_platform_env("tenable") mcp = MCPServer("f0-tenable") From 0a0a389f2c0a7831f5c761a0db9135b72216bdd3 Mon Sep 17 00:00:00 2001 From: James Pichardo Date: Wed, 12 Aug 2026 09:39:18 -0400 Subject: [PATCH 2/7] =?UTF-8?q?fix(sentinel):=20apply=20the=20read-tool=20?= =?UTF-8?q?audit=20to=20server=20#9=20=E2=80=94=20open-by-default=20queue,?= =?UTF-8?q?=20honest=20truncation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026-07-25 read-tool staleness audit answered its four questions for every read tool across the then-eight servers. Sentinel shipped as server #9 on 2026-08-11 and was never put through it. Two of the four answers were wrong. Q2 (can already-handled records come back as current?) — list_sentinel_incidents defaulted to status="any", so the tool described as "the SOC incident queue" returned closed incidents as current work. On the validation workspace the default call returned 23 Closed and 2 New while exactly 2 incidents were open: the queue was 92% already-handled work with the two that mattered buried in it. It now defaults to status="open", expressed as an exclusion (Status !~ "Closed") rather than an allow-list of ("New","Active"), so a Status value Sentinel adds later is treated as open work instead of silently disappearing. status="any" and status="closed" remain available. Q4 (is truncation disclosed?) — three of seven tools cut results silently: list_sentinel_incidents (25 of 55 deduped incidents dropped), search_office_activity, and run_kql. All three now emit core's truncation finding. The four tools that already disclosed used `len(rows) >= limit`, which cannot tell an exactly-full page from a truncated one and so over-reports; every row-returning tool now fetches limit + 1 and reports what it actually observed. run_kql discloses only when it added the bound itself — when the caller supplied their own `take`, neither answer is knowable. The detection-coverage skill now passes status="any" explicitly at the step that compares incident volume against rule count: that comparison is about the whole population a rule set produced, and the new default would have quietly reduced it to open work only. Q1 (server-side filtering) and Q3 (relevant page, not an arbitrary one) were already correct — severity and status filter inside the KQL before the bound, and every row query orders before it takes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ --- docs/reference/tools/sentinel.md | 7 +- .../sentinel-mcp/f0_sentinel_mcp/server.py | 9 +- servers/sentinel-mcp/f0_sentinel_mcp/tools.py | 105 +++++++++++++----- servers/sentinel-mcp/tests/test_tools.py | 70 +++++++++++- skills/sentinel/detection-coverage/SKILL.md | 7 +- 5 files changed, 162 insertions(+), 36 deletions(-) diff --git a/docs/reference/tools/sentinel.md b/docs/reference/tools/sentinel.md index 9462d75..6fd9d85 100644 --- a/docs/reference/tools/sentinel.md +++ b/docs/reference/tools/sentinel.md @@ -93,10 +93,15 @@ the Defender XDR-native incident view (with its own alert and device context) use f0-defender's list_incidents. Not an alert list — for individual alerts use f0-defender's list_alerts. +Returns open work by default (`status="open"` = everything not Closed), +because a queue of already-handled incidents is not a queue. Pass +`status="closed"` for handled work or `status="any"` for both. When more +incidents match than `limit`, a final "showing N" finding says so. + | Parameter | Type | Default | |---|---|---| | `severity_min` | `"informational"` \| `"low"` \| `"medium"` \| `"high"` | `"low"` | -| `status` | `"new"` \| `"active"` \| `"closed"` \| `"any"` | `"any"` | +| `status` | `"open"` \| `"new"` \| `"active"` \| `"closed"` \| `"any"` | `"open"` | | `hours_back` | `number` | `168` | | `limit` | `integer` | `25` | diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/server.py b/servers/sentinel-mcp/f0_sentinel_mcp/server.py index 2dcb3c0..3345209 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/server.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/server.py @@ -110,7 +110,7 @@ async def search_office_activity( @mcp.tool() async def list_sentinel_incidents( severity_min: Literal["informational", "low", "medium", "high"] = "low", - status: Literal["new", "active", "closed", "any"] = "any", + status: Literal["open", "new", "active", "closed", "any"] = "open", hours_back: float = 168, limit: int = 25, ) -> list[dict[str, Any]]: @@ -120,7 +120,12 @@ async def list_sentinel_incidents( or which ATT&CK tactics are showing up. This is the Sentinel-side view; for the Defender XDR-native incident view (with its own alert and device context) use f0-defender's list_incidents. Not an alert list — for - individual alerts use f0-defender's list_alerts.""" + individual alerts use f0-defender's list_alerts. + + Returns open work by default (`status="open"` = everything not Closed), + because a queue of already-handled incidents is not a queue. Pass + `status="closed"` for handled work or `status="any"` for both. When more + incidents match than `limit`, a final "showing N" finding says so.""" async with _client() as c: return _render( await tools.list_sentinel_incidents(c, severity_min, status, hours_back, limit) diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py index 2cd5449..d3c83ca 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py @@ -10,7 +10,11 @@ from typing import Any, Literal from f0_sectools_core.auth.graph import GraphError -from f0_sectools_core.paging import clamp_limit, more_available_finding +from f0_sectools_core.paging import ( + clamp_limit, + more_available_finding, + truncation_finding, +) from f0_sectools_core.schema.findings import ( Entity, EntityKind, @@ -172,6 +176,27 @@ async def list_data_sources(client: Any, limit: int = 25) -> list[Finding]: } +def _fetch_bound(limit: int) -> int: + """Ask the platform for one row more than we intend to show. + + `len(rows) >= limit` cannot tell "exactly limit rows exist" from "limit + rows and more", so it reports truncation that never happened. Fetching one + spare row turns the question into a fact for the cost of a single row. + """ + return limit + 1 + + +def _split_page(rows: list[dict[str, Any]], limit: int) -> tuple[list[dict[str, Any]], bool]: + """Split a `_fetch_bound` result into the rows to show and "was there more?".""" + return rows[:limit], len(rows) > limit + + +def _more(shown: int, has_more: bool, hint: str) -> list[Finding]: + """The core truncation note, or nothing when the page was complete.""" + f = truncation_finding("sentinel", shown=shown, fetched=shown, has_more=has_more, hint=hint) + return [f] if f else [] + + def _rows_to_findings(rows: list[dict[str, Any]], title_key: str, limit: int) -> list[Finding]: out: list[Finding] = [] for r in rows[:limit]: @@ -242,7 +267,7 @@ async def _run_surface( ] if indicator: parts.append(f"| project {', '.join(spec.project)}") - parts.append(f"| order by TimeGenerated desc | take {limit}") + parts.append(f"| order by TimeGenerated desc | take {_fetch_bound(limit)}") else: # No indicator -> aggregate. Never dump rows from a table this large. parts.append( @@ -273,15 +298,11 @@ async def _run_surface( ] title_key = spec.indicator_fields[0] if indicator else spec.action_field - findings = _rows_to_findings(rows, title_key, limit) - if len(rows) >= limit: - findings.append( - more_available_finding( - "sentinel", shown=len(findings), - hint="Narrow with an indicator or a shorter hours window.", - ) - ) - return findings + shown, has_more = _split_page(rows, limit) + findings = _rows_to_findings(shown, title_key, limit) + return findings + _more( + len(findings), has_more, "Narrow with an indicator or a shorter hours window." + ) async def hunt_firewall( @@ -371,11 +392,14 @@ async def search_office_activity( if operation: parts.append(f'| where Operation =~ "{operation}"') parts.append(f"| project {', '.join(_OA_PROJECT)}") - parts.append(f"| order by TimeGenerated desc | take {limit}") + parts.append(f"| order by TimeGenerated desc | take {_fetch_bound(limit)}") else: # Discovery mode: hand back the operation vocabulary so the model can # pick a real value rather than inventing one. - parts.append(f"| summarize Events=count() by Operation | top {limit} by Events desc") + parts.append( + f"| summarize Events=count() by Operation " + f"| top {_fetch_bound(limit)} by Events desc" + ) kql = " ".join(parts) try: @@ -399,6 +423,7 @@ async def search_office_activity( ), ) ] + shown_rows, has_more = _split_page(rows, limit) if not operation: return [ Finding( @@ -415,9 +440,14 @@ async def search_office_activity( f"operation=\"{r.get('Operation')}\" to see the events.", ), ) - for r in rows[:limit] - ] - return _rows_to_findings(rows, "Operation", limit) + for r in shown_rows + ] + _more( + len(shown_rows), has_more, + "Raise limit to see more operations, or pick one and call again with it.", + ) + return _rows_to_findings(shown_rows, "Operation", limit) + _more( + len(shown_rows), has_more, "Narrow with a shorter hours window or raise limit." + ) _SEV_ORDER = ("informational", "low", "medium", "high") @@ -429,6 +459,11 @@ async def search_office_activity( "medium": Severity.medium, "high": Severity.high, } _STATUS_VALUE = {"new": "New", "active": "Active", "closed": "Closed"} +# "open" is an exclusion, not a value: Sentinel's Status vocabulary can grow +# (an allow-list of ("New","Active") would silently drop a state added later), +# so open work is defined as "not Closed". Same reasoning as the Defender read +# tools, which prefer ne-exclusions of closed states over allow-lists. +_STATUS_CHOICES = ("open", "new", "active", "closed", "any") def _parse_json_object(raw: Any) -> dict[str, Any]: @@ -486,7 +521,7 @@ def _incident_tactics_techniques(raw: Any) -> tuple[str, str]: async def list_sentinel_incidents( client: Any, severity_min: str = "low", - status: str = "any", + status: str = "open", hours_back: float = 168, limit: int = 25, ) -> list[Finding]: @@ -494,8 +529,8 @@ async def list_sentinel_incidents( cap = "Sentinel incidents" if severity_min not in _SEV_ORDER: return [_bad_arg("severity_min", severity_min, ", ".join(_SEV_ORDER))] - if status != "any" and status not in _STATUS_VALUE: - return [_bad_arg("status", status, "new, active, closed, any")] + if status not in _STATUS_CHOICES: + return [_bad_arg("status", status, ", ".join(_STATUS_CHOICES))] missing = await _probe_or_finding(client, "SecurityIncident", "Sentinel incidents", cap) if missing: @@ -514,10 +549,12 @@ async def list_sentinel_incidents( "| summarize arg_max(TimeGenerated, *) by IncidentNumber", f"| where Severity in~ ({sev_list})", ] - if status != "any": + if status == "open": + parts.append('| where Status !~ "Closed"') + elif status != "any": parts.append(f'| where Status =~ "{_STATUS_VALUE[status]}"') parts.append("| order by TimeGenerated desc") - parts.append(f"| take {limit}") + parts.append(f"| take {_fetch_bound(limit)}") kql = " ".join(parts) try: @@ -534,12 +571,16 @@ async def list_sentinel_incidents( source="sentinel", finding_type=FindingType.posture, severity=Severity.info, - title=f"No Sentinel incidents at severity {severity_min}+ in the last {hours:g}h", + title=( + f"No {'open ' if status == 'open' else ''}Sentinel incidents at " + f"severity {severity_min}+ in the last {hours:g}h" + ), ) ] + shown_rows, has_more = _split_page(rows, limit) out: list[Finding] = [] - for r in rows[:limit]: + for r in shown_rows: num = str(r.get("IncidentNumber", "?")) tactics, techniques = _incident_tactics_techniques(r.get("AdditionalData")) out.append( @@ -559,7 +600,9 @@ async def list_sentinel_incidents( observed_at=str(r.get("TimeGenerated") or "") or None, ) ) - return out + return out + _more( + len(out), has_more, "Raise limit, shorten hours_back, or filter with severity_min." + ) # The MITRE tactics Sentinel analytics rules can carry. Used to name the GAP — @@ -901,7 +944,12 @@ async def run_kql( # take 25"` -- the whole bound silently swallowed by the comment, so # the query dispatches unbounded. A KQL line comment only extends to # the end of its own line, so a bound on the NEXT line always applies. - query = f"{query}\n| take {limit}" + query = f"{query}\n| take {_fetch_bound(limit)}" + bounded = True + else: + # The caller supplied their own bound; we cannot tell a complete result + # from a truncated one, so we must not claim either way. + bounded = False try: rows = await client.query(query, n.timespan(hours)) @@ -920,5 +968,8 @@ async def run_kql( title=f"Query returned no rows in the last {hours:g}h", ) ] - first_col = next(iter(rows[0].keys()), "result") - return _rows_to_findings(rows, first_col, limit) + shown, has_more = _split_page(rows, limit) if bounded else (rows[:limit], False) + first_col = next(iter(shown[0].keys()), "result") + return _rows_to_findings(shown, first_col, limit) + _more( + len(shown), has_more, "Add a filter to the query or raise limit." + ) diff --git a/servers/sentinel-mcp/tests/test_tools.py b/servers/sentinel-mcp/tests/test_tools.py index 8273219..d71054e 100644 --- a/servers/sentinel-mcp/tests/test_tools.py +++ b/servers/sentinel-mcp/tests/test_tools.py @@ -199,14 +199,18 @@ async def test_hunt_firewall_indicator_mode_kql_ends_with_bounded_take(fake): client = fake(rows={USAGE: _TABLES, CEF: []}) await tools.hunt_firewall(client, indicator="10.1.2.3", limit=7) kql = [q for q in client.queries if CEF in q][0] - assert kql.rstrip().endswith("| take 7") + # limit + 1: one spare row makes "was there more?" a fact rather than the + # `len(rows) >= limit` guess, which over-reports on an exactly-full page. + assert kql.rstrip().endswith("| take 8") async def test_hunt_firewall_indicator_mode_clamps_huge_limit(fake): client = fake(rows={USAGE: _TABLES, CEF: []}) await tools.hunt_firewall(client, indicator="10.1.2.3", limit=100000) kql = [q for q in client.queries if CEF in q][0] - assert kql.rstrip().endswith("| take 100") + # Clamped to MAX_LIMIT (100), then one spare row for truncation detection. + # The extra row is fetched, never shown -- the cap on returned findings holds. + assert kql.rstrip().endswith("| take 101") async def test_hunt_firewall_time_predicate_comes_first(fake): @@ -758,7 +762,7 @@ async def test_run_kql_passes_query_through(fake): async def test_run_kql_appends_bound_when_query_has_none(fake): client = fake(rows={"Heartbeat": []}) await tools.run_kql(client, "Heartbeat | project Computer", limit=10) - assert "| take 10" in client.queries[0] + assert "| take 11" in client.queries[0] # limit + 1 spare, as above async def test_run_kql_respects_an_existing_bound(fake): @@ -780,7 +784,7 @@ async def test_run_kql_force_bound_survives_a_trailing_line_comment(fake): lines = dispatched.split("\n") take_line = next(line for line in lines if "take" in line) assert "//" not in take_line - assert "| take 25" in take_line + assert "| take 26" in take_line async def test_run_kql_rejects_control_commands(fake): @@ -1096,3 +1100,61 @@ async def test_every_tool_returns_finding_not_exception_on_graph_error(fake, too out = await _ALL_SEVEN_TOOLS[tool_name](client) assert isinstance(out, list) and len(out) >= 1 assert all(f.finding_type.value == "posture" for f in out), (tool_name, status) + + +# --- Read-tool audit (2026-07-25 framework) applied to sentinel, server #9 --- +# Q2: can already-handled records come back as current? Q4: is truncation +# disclosed? Both were live-confirmed on the validation workspace: the default +# queue returned 23 Closed + 2 New while only 2 incidents were actually open, +# and 25 of 55 deduped incidents were dropped with no disclosure. + +def _incidents(n, status="New"): + return [dict(_INC[0], IncidentNumber=4000 + i, Status=status) for i in range(n)] + + +async def test_incident_queue_excludes_closed_by_default(fake): + """A SOC queue is open work. Closed incidents are not the analyst's queue.""" + client = fake(rows={USAGE: _TABLES, SI: _INC}) + await tools.list_sentinel_incidents(client) + kql = client.queries[-1] + assert 'Status !~ "Closed"' in kql + + +async def test_incident_queue_status_any_still_includes_closed(fake): + """The old behaviour stays reachable -- it just stops being the default.""" + client = fake(rows={USAGE: _TABLES, SI: _INC}) + await tools.list_sentinel_incidents(client, status="any") + assert "Closed" not in client.queries[-1] + + +async def test_incident_queue_status_closed_is_still_selectable(fake): + client = fake(rows={USAGE: _TABLES, SI: _INC}) + await tools.list_sentinel_incidents(client, status="closed") + assert 'Status =~ "Closed"' in client.queries[-1] + + +async def test_incident_queue_discloses_truncation(fake): + client = fake(rows={USAGE: _TABLES, SI: _incidents(6)}) + out = await tools.list_sentinel_incidents(client, limit=5) + assert sum(f.finding_type.value == "incident" for f in out) == 5 + assert any("more results available" in f.title for f in out) + + +async def test_incident_queue_silent_when_nothing_hidden(fake): + client = fake(rows={USAGE: _TABLES, SI: _incidents(5)}) + out = await tools.list_sentinel_incidents(client, limit=5) + assert not any("more results available" in f.title for f in out) + + +async def test_office_activity_discloses_truncation(fake): + rows = [{"Operation": f"Op{i}", "TimeGenerated": "2026-08-10T12:00:00Z"} for i in range(6)] + client = fake(rows={USAGE: _TABLES, OA: rows}) + out = await tools.search_office_activity(client, operation="FileDownloaded", limit=5) + assert any("more results available" in f.title for f in out) + + +async def test_run_kql_discloses_truncation(fake): + rows = [{"Computer": f"host{i}"} for i in range(6)] + client = fake(rows={USAGE: _TABLES, "Heartbeat": rows}) + out = await tools.run_kql(client, "Heartbeat", limit=5) + assert any("more results available" in f.title for f in out) diff --git a/skills/sentinel/detection-coverage/SKILL.md b/skills/sentinel/detection-coverage/SKILL.md index b353731..853145c 100644 --- a/skills/sentinel/detection-coverage/SKILL.md +++ b/skills/sentinel/detection-coverage/SKILL.md @@ -29,8 +29,11 @@ Base tool names: `get_detection_coverage`, `list_sentinel_incidents`, the custom figure — it is what the operator actually built. 2. **Only enabled rules count.** A disabled rule contributes to neither figure; do not credit coverage for a rule that is off. -3. Call `list_sentinel_incidents` and compare tactics against what - `get_detection_coverage` reports. A large incident volume against a small +3. Call `list_sentinel_incidents` with `status="any"` and compare tactics + against what `get_detection_coverage` reports. Pass `status="any"` + deliberately: the tool defaults to open work, but this comparison is about + the whole population a rule set produced, and closed incidents count toward + that just as much as open ones. A large incident volume against a small custom rule count means most incidents come from a connected product (e.g. Defender XDR mirroring into Sentinel) rather than Sentinel analytics — that is a real, commonly-missed finding, and it is invisible from the From 1823335d3ac77db87005241c002e7192e1ccf70d Mon Sep 17 00:00:00 2001 From: James Pichardo Date: Wed, 12 Aug 2026 09:46:33 -0400 Subject: [PATCH 3/7] fix(core): no exception leaves a tool unredacted or findingless MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each server maps the errors it expects — auth, permission, licensing, rate limit — into posture findings. Everything else propagated out of the tool, past `_render`, and reached the MCP client as a raw exception string. Reproduced on sentinel: an httpx.ConnectError surfaced its message verbatim, having passed through no redaction at all. That breaks Critical Rule 3 (redaction covers error paths) and Critical Rule 4 (every tool returns the findings schema), and it does so on all 9 servers and all 58 tools — which is why it is fixed once in core rather than per server. `core/redaction/boundary.py` adds `guarded_tool`, applied directly beneath `@mcp.tool()` on every registered tool. It is the only seam wide enough to catch a failure in client construction, in the tool body, and in the mapping code alike. An unclaimed exception becomes one posture finding carrying the exception type and its message — truncated to 300 characters, because an exception can carry an entire HTTP response body, and routed through the same redaction pass as any other output rather than discarded, since a caller with no detail cannot act. The title carries "temporarily unavailable", one of core/reports' DEGRADATION_MARKERS, so a generated report files an infrastructure failure under coverage instead of counting it as a security finding. `Exception`, deliberately not `BaseException`: cancellation and interrupts must keep propagating or a shutting-down server does not shut down. `functools.wraps` keeps the wrapped signature intact — verified by comparing the MCP-generated tool schema with and without the decorator, since a decorator visible to schema generation would silently rewrite all 58 tool contracts. An AST drift guard fails if any registered tool lacks the decorator, or if one server reports more than one source. CONTRIBUTING's recipe step 6 gains the requirement (and loses two stale references: FastMCP, redact_obj). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ --- CLAUDE.md | 2 +- CONTRIBUTING.md | 8 +- core/f0_sectools_core/redaction/boundary.py | 100 ++++++++++++++ core/tests/test_tool_boundary.py | 123 ++++++++++++++++++ .../defender-mcp/f0_defender_mcp/server.py | 8 ++ servers/entra-mcp/f0_entra_mcp/server.py | 5 + servers/intune-mcp/f0_intune_mcp/server.py | 7 + .../f0_limacharlie_mcp/server.py | 7 + .../f0_pa_actions_mcp/server.py | 8 ++ .../f0_projectachilles_mcp/server.py | 9 ++ servers/purview-mcp/f0_purview_mcp/server.py | 7 + .../sentinel-mcp/f0_sentinel_mcp/server.py | 8 ++ servers/sentinel-mcp/tests/test_server.py | 23 ++++ servers/tenable-mcp/f0_tenable_mcp/server.py | 8 ++ 14 files changed, 320 insertions(+), 3 deletions(-) create mode 100644 core/f0_sectools_core/redaction/boundary.py create mode 100644 core/tests/test_tool_boundary.py diff --git a/CLAUDE.md b/CLAUDE.md index e127c10..d7a2e1d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,7 +27,7 @@ This single constraint drives almost every design rule below. Small local models 1. **Read-only by default.** Every tool that queries a platform is read-only. Any tool that *changes state* on a live platform (isolate host, disable user, quarantine file, close incident) is a **gated write action** — see [Gated Write Actions](#gated-write-actions). It MUST require an explicit config flag AND per-action human confirmation, in one of two modes: **(a) forge-resistant** — a single-use confirmation token or a watcher approval delivered out-of-band on a channel the model cannot read; this is the default and the **only** permitted mode for destructive or irreversible actions. **(b) chat-confirm** — an opt-in, per-platform mode (off by default) where the operator's in-chat "approved" is the confirmation; it is convenient for supervised, reversible actions but is **not** forge-resistant (a misaligned model could fabricate it), so it is never used for destructive actions. 2. **Secrets never leave the host and never reach the model.** Credentials are loaded from per-platform `.env` files. They are **never logged, never included in tool output, never passed into a prompt or model context, never sent off-box.** -3. **Redact before returning.** All tool output passes through the core redaction layer before it is returned to the agent. Strip API keys, tokens, raw PII, and secrets from every payload — including error messages and stack traces. +3. **Redact before returning.** All tool output passes through the core redaction layer before it is returned to the agent. Strip API keys, tokens, raw PII, and secrets from every payload — including error messages and stack traces. Expected platform errors are mapped to findings by each server's `errors.py`; everything else — transport failures, unmapped statuses, bugs in our own mapping — is caught by `core/redaction/boundary.py`'s `guarded_tool`, applied beneath `@mcp.tool()` on every registered tool so no exception can reach the client unredacted or findingless. 4. **Every tool returns the structured findings schema.** No tool returns ad-hoc text. Output is normalized JSON (see [The Findings Schema](#the-findings-schema)) so agents — and small models especially — can parse and chain results predictably. 5. **Tools must be small-model-safe.** Flat argument schemas, short enums, few tools per server, no deeply nested objects, bounded/paginated output. See [Designing Tools for Small Models](#designing-tools-for-small-models). This is the repo's reason to exist — do not regress it for convenience. 6. **All safety logic lives in `core/`, never in a server.** Redaction, secret handling, the findings schema, and the gated-action machinery are implemented once, in the shared core, and imported by every server. A server must not re-implement or bypass them. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7c422f7..89209d5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,8 +46,12 @@ change. Do it in this order, TDD-ing each code step. (Background: 5. **Tools** (`tools.py`) — ≤ ~8 flat read tools returning `list[Finding]`; write the contract tests first (fake client) — live data validates real field names later. -6. **Server** (`server.py`) — `FastMCP`, one `@mcp.tool()` per tool, build the - client from config, **redact at the boundary** (`redact_obj(f.model_dump())`). +6. **Server** (`server.py`) — `MCPServer`, one `@mcp.tool()` per tool with + `@guarded_tool("")` directly beneath it, build the client from + config, **redact at the boundary** (`redact_finding(f).model_dump()`). + The guard is not optional: it turns any error your mapper did not claim + into one redacted finding instead of a raw exception string, and a test + (`core/tests/test_tool_boundary.py`) fails if a tool is missing it. 7. **Evals** — `evals//tasks.yaml` (≥1 task per tool) + add the server to `SERVERS` in `evals/test_eval_coverage.py` and `SERVER_MODULES` in `evals/run.py`. diff --git a/core/f0_sectools_core/redaction/boundary.py b/core/f0_sectools_core/redaction/boundary.py new file mode 100644 index 0000000..e9249a6 --- /dev/null +++ b/core/f0_sectools_core/redaction/boundary.py @@ -0,0 +1,100 @@ +"""The last line of defence on a tool's return path: nothing escapes unredacted. + +Every server maps the errors it *expects* -- auth, permission, licensing, rate +limit -- into posture findings. What is left over is the problem: a DNS or TLS +failure, an HTTP status no mapper claims, or a bug in our own mapping code. +Those propagate out of the tool, past the server's ``_render``, and reach the +MCP client as a raw exception string. That breaks two Critical Rules at once -- +the text never passes through redaction (Rule 3), and the tool returns no +finding at all (Rule 4). + +``guarded_tool`` closes that path once, here, so that no server has to remember +to. It is applied *beneath* ``@mcp.tool()`` on every registered tool, which is +the only place wide enough to catch a failure in client construction, in the +tool body, and in the mapping code alike. + +Deliberately catches ``Exception`` and not ``BaseException``: cancellation and +interrupts must keep propagating so a shutting-down server actually shuts down. +""" + +from __future__ import annotations + +import functools +from collections.abc import Awaitable, Callable +from typing import Any, TypeVar, cast + +from f0_sectools_core.redaction.redact import redact_finding +from f0_sectools_core.schema.findings import ( + Entity, + EntityKind, + Evidence, + Finding, + FindingType, + RecommendedAction, + Severity, +) + +__all__ = ["MAX_ERROR_CHARS", "guarded_tool", "unexpected_error_finding"] + +# An exception message can carry an entire HTTP response body. Bounded output is +# a repo rule, and an unbounded error is the worst kind of context flood -- +# it arrives exactly when the model is already off its intended path. +MAX_ERROR_CHARS = 300 + + +def unexpected_error_finding(source: str, capability: str, exc: BaseException) -> Finding: + """A posture finding standing in for an error no server mapper claimed. + + The title carries "temporarily unavailable" on purpose: that string is one + of ``core/reports``' DEGRADATION_MARKERS, so a generated report files this + under coverage ("not assessed") rather than counting an infrastructure + failure as a security finding. + + The message is included, truncated, and left to the standard redaction pass + -- Critical Rule 3 requires stripping secrets from error text, not + discarding the text, and a caller with no detail cannot act. + """ + detail = str(exc) + if len(detail) > MAX_ERROR_CHARS: + detail = detail[:MAX_ERROR_CHARS] + "…" + return Finding( + source=source, + finding_type=FindingType.posture, + severity=Severity.info, + title=f"{capability} temporarily unavailable — unexpected {type(exc).__name__}", + entity=Entity(kind=EntityKind.tenant, id=source), + evidence=[ + Evidence(key="error_type", value=type(exc).__name__), + Evidence(key="error", value=detail), + ], + recommended_action=RecommendedAction( + summary="Retry once. If it persists, check host connectivity and the " + "platform's service health before treating it as a data finding.", + ), + ) + + +F = TypeVar("F", bound=Callable[..., Awaitable[list[dict[str, Any]]]]) + + +def guarded_tool(source: str) -> Callable[[F], F]: + """Turn any unmapped exception into one redacted finding. + + ``functools.wraps`` keeps ``__name__``/``__doc__``/``__wrapped__`` intact so + the MCP layer still derives the same tool name, description and argument + schema from the wrapped function -- the decorator must be invisible to + schema generation, or it would silently change every tool's contract. + """ + + def decorate(fn: F) -> F: + @functools.wraps(fn) + async def wrapper(*args: Any, **kwargs: Any) -> list[dict[str, Any]]: + try: + return await fn(*args, **kwargs) + except Exception as exc: + finding = unexpected_error_finding(source, fn.__name__, exc) + return [redact_finding(finding).model_dump()] + + return cast(F, wrapper) + + return decorate diff --git a/core/tests/test_tool_boundary.py b/core/tests/test_tool_boundary.py new file mode 100644 index 0000000..d3c9721 --- /dev/null +++ b/core/tests/test_tool_boundary.py @@ -0,0 +1,123 @@ +"""No exception may leave a tool unredacted (Critical Rules 3 and 4).""" +import ast +import asyncio +import pathlib + +import pytest +from f0_sectools_core.redaction.boundary import ( + MAX_ERROR_CHARS, + guarded_tool, + unexpected_error_finding, +) +from f0_sectools_core.redaction.patterns import REDACTED +from f0_sectools_core.reports.sections import DEGRADATION_MARKERS + +REPO = pathlib.Path(__file__).resolve().parents[2] + + +async def test_unmapped_exception_becomes_a_finding_instead_of_raising(): + @guarded_tool("sentinel") + async def tool(): + raise ConnectionError("failed to resolve internal-host.example") + + out = await tool() + assert len(out) == 1 + assert out[0]["source"] == "sentinel" + assert out[0]["finding_type"] == "posture" + assert "ConnectionError" in out[0]["title"] + + +async def test_guard_is_transparent_when_nothing_raises(): + @guarded_tool("sentinel") + async def tool(): + return [{"ok": True}] + + assert await tool() == [{"ok": True}] + + +async def test_error_text_is_redacted(): + """An error path is an output path: Critical Rule 3 applies to it too.""" + + @guarded_tool("defender") + async def tool(): + raise RuntimeError("upstream rejected Bearer abcdefghijklmnop1234567890") + + out = await tool() + blob = str(out[0]) + assert "abcdefghijklmnop1234567890" not in blob + assert REDACTED in blob + + +async def test_long_error_is_truncated(): + """An exception can carry a whole HTTP body; bounded output is a repo rule.""" + + @guarded_tool("tenable") + async def tool(): + raise RuntimeError("x" * 5000) + + finding = (await tool())[0] + err = next(e for e in finding["evidence"] if e["key"] == "error") + assert len(err["value"]) <= MAX_ERROR_CHARS + 1 # +1 for the ellipsis + assert err["value"].endswith("…") + + +async def test_cancellation_still_propagates(): + """Catching BaseException would stop a shutting-down server from shutting down.""" + + @guarded_tool("entra") + async def tool(): + raise asyncio.CancelledError() + + with pytest.raises(asyncio.CancelledError): + await tool() + + +def test_title_is_a_report_degradation_marker(): + """A transport failure must render as 'not assessed', not as a security finding.""" + f = unexpected_error_finding("purview", "get_dlp_summary", TimeoutError("slow")) + assert any(m in f.title for m in DEGRADATION_MARKERS) + + +def test_guard_preserves_the_wrapped_identity(): + """MCP derives tool name/description from the function; the guard must be invisible.""" + + @guarded_tool("intune") + async def list_managed_devices(): + """Original docstring.""" + + assert list_managed_devices.__name__ == "list_managed_devices" + assert list_managed_devices.__doc__ == "Original docstring." + + +def _tool_decorators(path): + """(function name, [decorator nodes]) for every @mcp.tool() in a server module.""" + tree = ast.parse(path.read_text()) + for node in ast.walk(tree): + if isinstance(node, ast.AsyncFunctionDef) and any( + isinstance(d, ast.Call) and getattr(d.func, "attr", "") == "tool" + for d in node.decorator_list + ): + yield node.name, node.decorator_list + + +@pytest.mark.parametrize( + "server", sorted(REPO.glob("servers/*/*/server.py")), ids=lambda p: p.parts[-3] +) +def test_every_registered_tool_is_guarded(server): + """Drift guard: a new tool -- or a new server -- cannot silently skip the boundary.""" + sources = set() + unguarded = [] + for name, decorators in _tool_decorators(server): + guards = [ + d for d in decorators + if isinstance(d, ast.Call) and getattr(d.func, "id", "") == "guarded_tool" + ] + if not guards: + unguarded.append(name) + continue + sources.update( + a.value for g in guards for a in g.args if isinstance(a, ast.Constant) + ) + assert unguarded == [], f"tools missing @guarded_tool: {unguarded}" + assert len(sources) == 1, f"one server must report one source, got {sources}" + assert sources.pop(), "guarded_tool source must be a non-empty string" diff --git a/servers/defender-mcp/f0_defender_mcp/server.py b/servers/defender-mcp/f0_defender_mcp/server.py index 2fbac94..18e30e3 100644 --- a/servers/defender-mcp/f0_defender_mcp/server.py +++ b/servers/defender-mcp/f0_defender_mcp/server.py @@ -15,6 +15,7 @@ from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient from f0_sectools_core.gating.actions import AuditLog, GatedAction, TokenStore +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -53,6 +54,7 @@ def _gate(name: str, cfg: PlatformConfig) -> GatedAction: @mcp.tool() +@guarded_tool("defender") async def get_secure_score() -> list[dict[str, Any]]: """Get the Microsoft Secure Score — Microsoft 365 / Defender config-hardening posture (%). @@ -65,6 +67,7 @@ async def get_secure_score() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("defender") async def list_incidents( severity_min: Literal["info", "low", "medium", "high", "critical"] = "medium", limit: int = 25, @@ -87,6 +90,7 @@ async def list_incidents( @mcp.tool() +@guarded_tool("defender") async def list_alerts( severity_min: Literal["info", "low", "medium", "high", "critical"] = "high", limit: int = 25, @@ -104,6 +108,7 @@ async def list_alerts( @mcp.tool() +@guarded_tool("defender") async def run_hunting_query(kql: str) -> list[dict[str, Any]]: """Run a Microsoft Defender advanced hunting query (KQL) — a READ-ONLY search of M365 / Entra / device telemetry (30d). @@ -130,6 +135,7 @@ async def run_hunting_query(kql: str) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("defender") async def hunt( category: Literal["network", "process", "logon", "email"], indicator: str = "", @@ -156,6 +162,7 @@ async def hunt( @mcp.tool() +@guarded_tool("defender") async def isolate_host( device_id: str, comment: str, confirmation_token: str = "" ) -> list[dict[str, Any]]: @@ -182,6 +189,7 @@ async def isolate_host( @mcp.tool() +@guarded_tool("defender") async def release_host( device_id: str, comment: str, confirmation_token: str = "" ) -> list[dict[str, Any]]: diff --git a/servers/entra-mcp/f0_entra_mcp/server.py b/servers/entra-mcp/f0_entra_mcp/server.py index 530e433..d5074cf 100644 --- a/servers/entra-mcp/f0_entra_mcp/server.py +++ b/servers/entra-mcp/f0_entra_mcp/server.py @@ -11,6 +11,7 @@ from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -29,6 +30,7 @@ def _render(findings: list[Finding]) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("entra") async def list_risky_users( limit: int = 25, state: Literal["active", "all"] = "active" ) -> list[dict[str, Any]]: @@ -44,6 +46,7 @@ async def list_risky_users( @mcp.tool() +@guarded_tool("entra") async def list_risk_detections( limit: int = 25, state: Literal["active", "all"] = "active" ) -> list[dict[str, Any]]: @@ -59,6 +62,7 @@ async def list_risk_detections( @mcp.tool() +@guarded_tool("entra") async def list_conditional_access_policies() -> list[dict[str, Any]]: """List Conditional Access policies, flagging disabled and report-only ones.""" cfg = PlatformConfig.from_env("ENTRA") @@ -67,6 +71,7 @@ async def list_conditional_access_policies() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("entra") async def list_privileged_role_assignments(limit: int = 25) -> list[dict[str, Any]]: """List directory role assignments, highlighting critical privileged roles. diff --git a/servers/intune-mcp/f0_intune_mcp/server.py b/servers/intune-mcp/f0_intune_mcp/server.py index c25c078..d4c96de 100644 --- a/servers/intune-mcp/f0_intune_mcp/server.py +++ b/servers/intune-mcp/f0_intune_mcp/server.py @@ -11,6 +11,7 @@ from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -27,6 +28,7 @@ def _render(findings: list[Finding]) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("intune") async def list_managed_devices( compliance: Literal["all", "compliant", "noncompliant", "ingraceperiod", "unknown"] = "all", limit: int = 25, @@ -41,6 +43,7 @@ async def list_managed_devices( @mcp.tool() +@guarded_tool("intune") async def get_compliance_summary() -> list[dict[str, Any]]: """Intune device-compliance rollup: how many managed devices are compliant vs not.""" cfg = PlatformConfig.from_env("INTUNE") @@ -49,6 +52,7 @@ async def get_compliance_summary() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("intune") async def get_managed_device(device_name: str) -> list[dict[str, Any]]: """Get one Intune-managed device by its device name (compliance, encryption, owner, sync).""" cfg = PlatformConfig.from_env("INTUNE") @@ -57,6 +61,7 @@ async def get_managed_device(device_name: str) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("intune") async def list_stale_devices(days: int = 30, limit: int = 25) -> list[dict[str, Any]]: """List Intune devices not synced in the last `days` (coverage drift / abandoned).""" cfg = PlatformConfig.from_env("INTUNE") @@ -65,6 +70,7 @@ async def list_stale_devices(days: int = 30, limit: int = 25) -> list[dict[str, @mcp.tool() +@guarded_tool("intune") async def list_compliance_policies(limit: int = 25) -> list[dict[str, Any]]: """List Intune device COMPLIANCE POLICIES. @@ -76,6 +82,7 @@ async def list_compliance_policies(limit: int = 25) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("intune") async def list_configuration_profiles(limit: int = 25) -> list[dict[str, Any]]: """List Intune device CONFIGURATION PROFILES. diff --git a/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py b/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py index c0ea122..63bd369 100644 --- a/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py +++ b/servers/limacharlie-mcp/f0_limacharlie_mcp/server.py @@ -10,6 +10,7 @@ from f0_sectools_core.auth.config import LimaCharlieConfig from f0_sectools_core.auth.env import load_platform_env +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -31,6 +32,7 @@ def _client() -> LimaCharlieClient: @mcp.tool() +@guarded_tool("limacharlie") async def get_org_overview() -> list[dict[str, Any]]: """LimaCharlie EDR deployment posture: sensor counts, D&R rule count, recent detection volume. @@ -41,6 +43,7 @@ async def get_org_overview() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("limacharlie") async def list_sensors( online_only: bool = False, limit: int = 50, tag: str = "" ) -> list[dict[str, Any]]: @@ -54,6 +57,7 @@ async def list_sensors( @mcp.tool() +@guarded_tool("limacharlie") async def get_sensor(hostname: str) -> list[dict[str, Any]]: """Get LimaCharlie sensor detail by hostname (prefix match): platform, online status, sid, tags. @@ -62,12 +66,14 @@ async def get_sensor(hostname: str) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("limacharlie") async def list_dr_rules(namespace: str = "general", limit: int = 50) -> list[dict[str, Any]]: """List Detection & Response (D&R) rules in the org (coverage). namespace: general|managed.""" return _render(await asyncio.to_thread(tools.list_dr_rules, _client(), namespace, limit)) @mcp.tool() +@guarded_tool("limacharlie") async def list_detections( hours_back: float = 24, limit: int = 50, category: str | None = None ) -> list[dict[str, Any]]: @@ -80,6 +86,7 @@ async def list_detections( @mcp.tool() +@guarded_tool("limacharlie") async def query_telemetry( hunt: Literal[ "new_processes", "powershell_activity", "dns_requests", "network_connections", diff --git a/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py b/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py index 6d046b5..6cd217f 100644 --- a/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py +++ b/servers/projectachilles-actions-mcp/f0_pa_actions_mcp/server.py @@ -13,6 +13,7 @@ from f0_sectools_core.auth.config import ProjectAchillesConfig from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.gating.actions import AuditLog, GatedAction, TokenStore +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -49,6 +50,7 @@ def _gate(name: str, cfg: ProjectAchillesConfig) -> GatedAction: @mcp.tool() +@guarded_tool("projectachilles") async def run_test( test_id: str, hostname: str = "", tag: str = "", confirmation_token: str = "" ) -> list[dict[str, Any]]: @@ -75,6 +77,7 @@ async def run_test( @mcp.tool() +@guarded_tool("projectachilles") async def schedule_test( test_id: str, hostname: str = "", @@ -107,6 +110,7 @@ async def schedule_test( @mcp.tool() +@guarded_tool("projectachilles") async def set_schedule_status( schedule_id: str, status: Literal["active", "paused"], @@ -129,6 +133,7 @@ async def set_schedule_status( @mcp.tool() +@guarded_tool("projectachilles") async def cancel_tasks( task_id: str = "", status: Literal["pending", "assigned", "running", "completed", "failed", "expired"] = "pending", @@ -151,6 +156,7 @@ async def cancel_tasks( @mcp.tool() +@guarded_tool("projectachilles") async def list_schedules( status: Literal["", "active", "paused", "completed"] = "", ) -> list[dict[str, Any]]: @@ -165,6 +171,7 @@ async def list_schedules( @mcp.tool() +@guarded_tool("projectachilles") async def get_task_status(task_id: str) -> list[dict[str, Any]]: """One-shot status check for a ProjectAchilles test-run task (read-only). @@ -179,6 +186,7 @@ async def get_task_status(task_id: str) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("projectachilles") async def list_tasks( status: Literal["", "pending", "assigned", "running", "completed", "failed", "expired"] = "", search: str = "", diff --git a/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py b/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py index 8582ccf..075f12d 100644 --- a/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py +++ b/servers/projectachilles-mcp/f0_projectachilles_mcp/server.py @@ -8,6 +8,7 @@ from f0_sectools_core.auth.config import ProjectAchillesConfig from f0_sectools_core.auth.env import load_platform_env +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -29,6 +30,7 @@ def _client() -> ProjectAchillesClient: @mcp.tool() +@guarded_tool("projectachilles") async def get_defense_score( days: int = 30, over_time: bool = False, interval: str = "day" ) -> list[dict[str, Any]]: @@ -50,6 +52,7 @@ async def get_defense_score( @mcp.tool() +@guarded_tool("projectachilles") async def get_weak_techniques(days: int = 30, limit: int = 10) -> list[dict[str, Any]]: """Lowest-scoring MITRE techniques — where defenses most often fail.""" async with _client() as pa: @@ -57,6 +60,7 @@ async def get_weak_techniques(days: int = 30, limit: int = 10) -> list[dict[str, @mcp.tool() +@guarded_tool("projectachilles") async def list_test_executions( days: int = 7, limit: int = 25, test: str = "", tag: str = "", hostname: str = "", @@ -73,6 +77,7 @@ async def list_test_executions( @mcp.tool() +@guarded_tool("projectachilles") async def list_risk_acceptances( status: Literal["active", "revoked"] = "active", limit: int = 50 ) -> list[dict[str, Any]]: @@ -82,6 +87,7 @@ async def list_risk_acceptances( @mcp.tool() +@guarded_tool("projectachilles") async def list_agents( status: str | None = None, online_only: bool = False, limit: int = 50 ) -> list[dict[str, Any]]: @@ -91,6 +97,7 @@ async def list_agents( @mcp.tool() +@guarded_tool("projectachilles") async def get_fleet_health() -> list[dict[str, Any]]: """ProjectAchilles validation-agent fleet health: attack-simulation agents online/offline. @@ -102,6 +109,7 @@ async def get_fleet_health() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("projectachilles") async def find_tests( by: Literal["technique", "actor", "tactic", "category", "tag", "keyword"], value: str, @@ -116,6 +124,7 @@ async def find_tests( @mcp.tool() +@guarded_tool("projectachilles") async def get_test(test_id: str) -> list[dict[str, Any]]: """Full detail for ONE specific test — use for "what does test X cover / do", "details on the test". Returns description, OS/target, complexity, tactics, diff --git a/servers/purview-mcp/f0_purview_mcp/server.py b/servers/purview-mcp/f0_purview_mcp/server.py index e3f5265..b3c9536 100644 --- a/servers/purview-mcp/f0_purview_mcp/server.py +++ b/servers/purview-mcp/f0_purview_mcp/server.py @@ -11,6 +11,7 @@ from f0_sectools_core.auth.config import PlatformConfig from f0_sectools_core.auth.env import load_platform_env from f0_sectools_core.auth.graph import GraphClient +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -31,6 +32,7 @@ def _client() -> GraphClient: @mcp.tool() +@guarded_tool("purview") async def get_dlp_summary( hours_back: float = 168, state: Literal["open", "all"] = "open" ) -> list[dict[str, Any]]: @@ -45,6 +47,7 @@ async def get_dlp_summary( @mcp.tool() +@guarded_tool("purview") async def list_dlp_alerts( hours_back: float = 168, severity_min: Literal["low", "medium", "high"] = "low", @@ -60,6 +63,7 @@ async def list_dlp_alerts( @mcp.tool() +@guarded_tool("purview") async def list_insider_risk_alerts( hours_back: float = 168, limit: int = 25, state: Literal["open", "all"] = "open" ) -> list[dict[str, Any]]: @@ -72,6 +76,7 @@ async def list_insider_risk_alerts( @mcp.tool() +@guarded_tool("purview") async def list_sensitivity_labels() -> list[dict[str, Any]]: """List the organization's Purview sensitivity labels (classification inventory) — answers whether data classification is actually deployed.""" @@ -80,6 +85,7 @@ async def list_sensitivity_labels() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("purview") async def search_audit_log( activity: str = "", user: str = "", @@ -107,6 +113,7 @@ async def search_audit_log( @mcp.tool() +@guarded_tool("purview") async def get_audit_results(audit_query_id: str, limit: int = 25) -> list[dict[str, Any]]: """Fetch the results of a previously submitted audit search (the audit_query_id returned by search_audit_log when it was still running). diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/server.py b/servers/sentinel-mcp/f0_sentinel_mcp/server.py index 3345209..9e90888 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/server.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/server.py @@ -10,6 +10,7 @@ from f0_sectools_core.auth.config import SentinelConfig from f0_sectools_core.auth.env import load_platform_env +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -31,6 +32,7 @@ def _client() -> SentinelClient: @mcp.tool() +@guarded_tool("sentinel") async def list_data_sources(limit: int = 25) -> list[dict[str, Any]]: """List which security telemetry this Sentinel workspace actually ingests. @@ -45,6 +47,7 @@ async def list_data_sources(limit: int = 25) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("sentinel") async def hunt_firewall( action: Literal["allowed", "blocked", "detected", "any"] = "any", indicator: str = "", @@ -64,6 +67,7 @@ async def hunt_firewall( @mcp.tool() +@guarded_tool("sentinel") async def hunt_dns_web( surface: Literal["dns", "web", "vpn"] = "dns", action: Literal["allowed", "blocked", "detected", "any"] = "any", @@ -86,6 +90,7 @@ async def hunt_dns_web( @mcp.tool() +@guarded_tool("sentinel") async def search_office_activity( workload: Literal["sharepoint", "onedrive", "exchange", "teams", "any"] = "any", operation: str = "", @@ -108,6 +113,7 @@ async def search_office_activity( @mcp.tool() +@guarded_tool("sentinel") async def list_sentinel_incidents( severity_min: Literal["informational", "low", "medium", "high"] = "low", status: Literal["open", "new", "active", "closed", "any"] = "open", @@ -133,6 +139,7 @@ async def list_sentinel_incidents( @mcp.tool() +@guarded_tool("sentinel") async def get_detection_coverage() -> list[dict[str, Any]]: """Report Sentinel's analytics-rule inventory and which MITRE tactics are UNCOVERED. @@ -150,6 +157,7 @@ async def get_detection_coverage() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("sentinel") async def run_kql(kql: str, hours_back: float = 24, limit: int = 25) -> list[dict[str, Any]]: """Run a CUSTOM read-only KQL query against the Sentinel Log Analytics workspace. diff --git a/servers/sentinel-mcp/tests/test_server.py b/servers/sentinel-mcp/tests/test_server.py index d1fdbe2..9ad42e6 100644 --- a/servers/sentinel-mcp/tests/test_server.py +++ b/servers/sentinel-mcp/tests/test_server.py @@ -37,3 +37,26 @@ async def test_routing_docstrings_name_the_neighbouring_tool(): assert "run_hunting_query" in (tools["run_kql"].description or "") assert "list_incidents" in (tools["list_sentinel_incidents"].description or "") assert "search_audit_log" in (tools["search_office_activity"].description or "") + + +async def test_transport_failure_returns_a_finding_not_a_raw_exception(monkeypatch): + """End-to-end through the real registration path: a DNS/TLS failure used to + reach the MCP client as a bare ConnectError string, bypassing redaction.""" + class Boom: + retention_days = 30 + has_arm = True + workspace_id = "ws" + + async def __aenter__(self): + return self + + async def __aexit__(self, *a): + return False + + async def query(self, *a, **k): + raise ConnectionError("failed to resolve internal-collector.example") + + monkeypatch.setattr(server, "_client", lambda: Boom()) + out = await server.list_data_sources() + assert out[0]["finding_type"] == "posture" + assert "temporarily unavailable" in out[0]["title"] diff --git a/servers/tenable-mcp/f0_tenable_mcp/server.py b/servers/tenable-mcp/f0_tenable_mcp/server.py index 3b4f5f2..6bcdd16 100644 --- a/servers/tenable-mcp/f0_tenable_mcp/server.py +++ b/servers/tenable-mcp/f0_tenable_mcp/server.py @@ -8,6 +8,7 @@ from f0_sectools_core.auth.config import TenableConfig from f0_sectools_core.auth.env import load_platform_env +from f0_sectools_core.redaction.boundary import guarded_tool from f0_sectools_core.redaction.redact import redact_finding from f0_sectools_core.schema.findings import Finding from mcp.server import MCPServer @@ -29,6 +30,7 @@ def _client() -> TenableClient: @mcp.tool() +@guarded_tool("tenable") async def get_vulnerability_summary() -> list[dict[str, Any]]: """Tenable environment-wide vulnerability posture — counts by severity. @@ -40,6 +42,7 @@ async def get_vulnerability_summary() -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("tenable") async def list_top_vulnerabilities( severity_min: Literal["low", "medium", "high", "critical"] = "high", limit: int = 10 ) -> list[dict[str, Any]]: @@ -53,6 +56,7 @@ async def list_top_vulnerabilities( @mcp.tool() +@guarded_tool("tenable") async def list_assets(hostname: str = "", limit: int = 25) -> list[dict[str, Any]]: """Tenable asset inventory — hosts Tenable has scanned. @@ -64,6 +68,7 @@ async def list_assets(hostname: str = "", limit: int = 25) -> list[dict[str, Any @mcp.tool() +@guarded_tool("tenable") async def get_asset_vulnerabilities( asset: str, severity_min: Literal["low", "medium", "high", "critical"] = "high", @@ -80,6 +85,7 @@ async def get_asset_vulnerabilities( @mcp.tool() +@guarded_tool("tenable") async def get_vulnerability_info(plugin_id: str) -> list[dict[str, Any]]: """Tenable detail for one plugin/vulnerability: CVSS, VPR, description, remediation. @@ -90,6 +96,7 @@ async def get_vulnerability_info(plugin_id: str) -> list[dict[str, Any]]: @mcp.tool() +@guarded_tool("tenable") async def list_vulnerability_assets(plugin_id: str, limit: int = 25) -> list[dict[str, Any]]: """List the hosts affected by a specific Tenable vulnerability (plugin_id). @@ -100,6 +107,7 @@ async def list_vulnerability_assets(plugin_id: str, limit: int = 25) -> list[dic @mcp.tool() +@guarded_tool("tenable") async def list_scans(limit: int = 25) -> list[dict[str, Any]]: """Tenable scan inventory — each scan's status and last-run time (coverage freshness).""" async with _client() as tio: From 522faf83bd238777f01ddc74e748256dbdbde5d6 Mon Sep 17 00:00:00 2001 From: James Pichardo Date: Wed, 12 Aug 2026 09:57:38 -0400 Subject: [PATCH 4/7] feat(sentinel): make Umbrella identities and IPs searchable, not just returned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asked "which host or user is behind these IPs", a local model concluded the Umbrella logs carry no such field and spent six tool calls hunting the mapping across LimaCharlie, Tenable, Entra and Office 365. The conclusion was wrong. Identities_s is 100% populated on Cisco_Umbrella_dns_CL (1,245 distinct in 24h; identity types "AD Users" and "Anyconnect Roaming Client"), and the tool was already returning it — it simply could not be searched for, and nothing told the model it was there. The dns surface matched `indicator` against Domain_s alone, while its own help text advertised "a domain, URL fragment, or IP". An IP indicator passed validation and then matched nothing, so the tool answered "no activity" to a question it had never actually asked — the same advertised-but-not-honoured shape as the $orderby trap in the read-tool audit. Rather than add a `hostname` parameter, the existing `indicator` now covers identity and address: dns gains InternalIp_s, ExternalIp_s and Identities_s; web gains Internal_IP_s and Identities_s; vpn gains Device_ID_s. One argument that means "the thing you are looking for" beats a sixth argument on a tool that already has five — tool-selection and argument-filling accuracy is the whole premise of this repo. `has` needed no change: a token drawn from a row matched its own Identities_s on 359,304 of 359,304 rows. validate_indicator now also accepts UPN_RE, widening the charset by exactly one character ("@") so an AD user is usable, still with no quote, backslash or whitespace. Verified live: hunt_dns_web(surface="dns", indicator="tailscale") returns 8 distinct identities across 8 internal IPs in a single call — the whole six-call cross-platform hunt, answered from the row the tool already had. The skill gains the routing rule and a pitfall the same transcript earned: the model reported "IP to User Mapping Found" for an external IP it had itself just shown serving five internal hosts. That address is a site's NAT egress; naming one user behind it is a false attribution stated with confidence. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ --- docs/reference/tools/sentinel.md | 10 +++-- .../sentinel-mcp/f0_sentinel_mcp/normalize.py | 18 ++++++-- .../sentinel-mcp/f0_sentinel_mcp/server.py | 10 +++-- servers/sentinel-mcp/f0_sentinel_mcp/tools.py | 3 +- servers/sentinel-mcp/tests/test_normalize.py | 42 +++++++++++++++++++ .../sentinel/network-investigation/SKILL.md | 19 ++++++++- 6 files changed, 89 insertions(+), 13 deletions(-) diff --git a/docs/reference/tools/sentinel.md b/docs/reference/tools/sentinel.md index 6fd9d85..5e4cfb2 100644 --- a/docs/reference/tools/sentinel.md +++ b/docs/reference/tools/sentinel.md @@ -48,9 +48,13 @@ SEARCH DNS, web-proxy, or remote-access VPN activity (Cisco Umbrella). Choose surface by what you are looking for: dns — a domain was resolved or blocked (C2, newly-registered domains, blocked categories); web — a URL was fetched, a file downloaded, or a proxy verdict applied; vpn — remote-access -VPN sessions and failures. `indicator` is a domain, URL fragment or IP. -Without an indicator this returns an aggregate, not individual events. For -perimeter firewall connections by IP/port use hunt_firewall. +VPN sessions and failures. `indicator` is a domain, URL fragment, IP +address, or an identity — Umbrella names the AD user or roaming-client +machine behind each request, so pass a hostname or username to see what it +did, or pass an IP or domain to see who was behind it. Every returned row +carries that identity, so you do not need another platform to answer "who +was this?". Without an indicator this returns an aggregate, not individual +events. For perimeter firewall connections by IP/port use hunt_firewall. | Parameter | Type | Default | |---|---|---| diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py b/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py index 105238b..d44ece8 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py @@ -81,7 +81,12 @@ class Surface: table="Cisco_Umbrella_dns_CL", action_field="Action_s", action_map={"allowed": ("Allowed",), "blocked": ("Blocked",)}, - indicator_fields=("Domain_s",), + # Live fill rates (2026-08-12, 24h): Identities_s 100% / 1,245 distinct, + # identity types "AD Users" + "Anyconnect Roaming Client". The "who" + # behind a DNS query is in the row; these fields make it searchable, so + # "which host resolved X" and "what did host Y resolve" are one call + # each instead of a correlation hunt across other platforms. + indicator_fields=("Domain_s", "InternalIp_s", "ExternalIp_s", "Identities_s"), project=( "TimeGenerated", "Action_s", "Domain_s", "Categories_s", "InternalIp_s", "ExternalIp_s", "Identities_s", "QueryType_s", @@ -92,7 +97,8 @@ class Surface: table="Cisco_Umbrella_proxy_CL", action_field="Verdict_s", action_map={"allowed": ("ALLOWED",), "blocked": ("BLOCKED",)}, - indicator_fields=("URL_s", "Destination_IP_s"), + # Identities_s 100% / 1,114 distinct; Host_Name_s 100% (24h sample). + indicator_fields=("URL_s", "Destination_IP_s", "Internal_IP_s", "Identities_s"), project=( "TimeGenerated", "Verdict_s", "URL_s", "Categories_s", "Internal_IP_s", "Identities_s", "File_Name_s", "SHA_SHA256_s", @@ -106,7 +112,8 @@ class Surface: # plus Disconnected=240 (a session end, not an accept/deny outcome -- # deliberately left out of both buckets rather than guessed into one). action_map={"allowed": ("Connected",), "blocked": ("Failed",)}, - indicator_fields=("User_ID_s", "Public_IP_s", "Assigned_IP_s"), + # Device_ID_s is 100% populated (281 distinct, matching User_ID_s). + indicator_fields=("User_ID_s", "Public_IP_s", "Assigned_IP_s", "Device_ID_s"), project=( "TimeGenerated", "Event_Type_s", "User_ID_s", "Public_IP_s", "Assigned_IP_s", "VPN_Profile_s", "OS_Version_s", "Failed_Reasons_s", @@ -180,7 +187,10 @@ def validate_indicator(indicator: str, kind: str) -> bool: return True if kind == "net": return bool(IP_RE.fullmatch(indicator) or PORT_RE.fullmatch(indicator)) - return bool(DOMAIN_RE.fullmatch(indicator)) + # UPN_RE widens the charset by exactly one character, "@", so an Umbrella + # identity (an AD user) is a usable indicator. It stays inside the same + # injection boundary as DOMAIN_RE -- no quote, backslash or whitespace. + return bool(DOMAIN_RE.fullmatch(indicator) or UPN_RE.fullmatch(indicator)) def indicator_clause(spec: Surface, indicator: str) -> str: diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/server.py b/servers/sentinel-mcp/f0_sentinel_mcp/server.py index 9e90888..b0fa929 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/server.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/server.py @@ -80,9 +80,13 @@ async def hunt_dns_web( Choose surface by what you are looking for: dns — a domain was resolved or blocked (C2, newly-registered domains, blocked categories); web — a URL was fetched, a file downloaded, or a proxy verdict applied; vpn — remote-access - VPN sessions and failures. `indicator` is a domain, URL fragment or IP. - Without an indicator this returns an aggregate, not individual events. For - perimeter firewall connections by IP/port use hunt_firewall.""" + VPN sessions and failures. `indicator` is a domain, URL fragment, IP + address, or an identity — Umbrella names the AD user or roaming-client + machine behind each request, so pass a hostname or username to see what it + did, or pass an IP or domain to see who was behind it. Every returned row + carries that identity, so you do not need another platform to answer "who + was this?". Without an indicator this returns an aggregate, not individual + events. For perimeter firewall connections by IP/port use hunt_firewall.""" async with _client() as c: return _render( await tools.hunt_dns_web(c, surface, action, indicator, hours_back, limit) diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py index d3c83ca..d576d1a 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py @@ -172,7 +172,8 @@ async def list_data_sources(client: Any, limit: int = 25) -> list[Finding]: _INDICATOR_HELP = { "net": "an IP address or a port number (this table carries no URLs or " "usernames — for domains and URLs use hunt_dns_web)", - "domain": "a domain, URL fragment, or IP", + "domain": "a domain, URL fragment, IP address, or an Umbrella identity " + "(the AD user or roaming-client machine name that made the request)", } diff --git a/servers/sentinel-mcp/tests/test_normalize.py b/servers/sentinel-mcp/tests/test_normalize.py index 4642c6d..44706a2 100644 --- a/servers/sentinel-mcp/tests/test_normalize.py +++ b/servers/sentinel-mcp/tests/test_normalize.py @@ -130,3 +130,45 @@ def test_validate_indicator_empty_string_passes_for_every_kind(): # implicit. assert n.validate_indicator("", "net") is True assert n.validate_indicator("", "domain") is True + + +# --- Identity/IP search on the Umbrella surfaces ------------------------- +# Live-verified 2026-08-12 on the validation workspace: Identities_s is 100% +# populated on Cisco_Umbrella_dns_CL (1,245 distinct in 24h; identity types are +# "AD Users" and "Anyconnect Roaming Client"), and `has` matched a token drawn +# from the same row on 359,304 of 359,304 rows. The "who" was always in the row +# the tool already returned -- it just could not be searched for. + +def test_dns_indicator_searches_the_ip_columns(): + """`_INDICATOR_HELP` promised IP matching for dns; only Domain_s was searched.""" + spec = n.SURFACE_SPECS["dns"] + clause = n.indicator_clause(spec, "192.168.1.29") + assert "InternalIp_s has" in clause + assert "ExternalIp_s has" in clause + + +def test_dns_indicator_searches_identity(): + clause = n.indicator_clause(n.SURFACE_SPECS["dns"], "lt-tpl-l114") + assert "Identities_s has" in clause + + +def test_web_indicator_searches_identity(): + clause = n.indicator_clause(n.SURFACE_SPECS["web"], "lt-tpl-l114") + assert "Identities_s has" in clause + + +def test_a_upn_is_a_valid_indicator(): + """Umbrella identities are AD users, so an indicator must survive an '@'.""" + assert n.validate_indicator("rherrera@example.gob.do", "domain") is True + + +def test_upn_widening_still_rejects_a_kql_break_out(): + """The charset is the injection boundary; widening it must not open a quote.""" + for bad in ['a" or 1==1 //', "a\\b", "a'b", "a b"]: + assert n.validate_indicator(bad, "domain") is False + + +def test_identity_is_projected_on_the_umbrella_surfaces(): + """Returning the answer matters as much as being able to search for it.""" + for surface in ("dns", "web"): + assert "Identities_s" in n.SURFACE_SPECS[surface].project diff --git a/skills/sentinel/network-investigation/SKILL.md b/skills/sentinel/network-investigation/SKILL.md index 8379226..ec19cef 100644 --- a/skills/sentinel/network-investigation/SKILL.md +++ b/skills/sentinel/network-investigation/SKILL.md @@ -27,7 +27,14 @@ Base tool names: `list_data_sources`, `hunt_firewall`, `hunt_dns_web`, in this skill: - **Domain or URL** → `hunt_dns_web` (`surface="dns"` for resolutions, `surface="web"` for fetches and downloads). - - **IP address or port** → `hunt_firewall`. + - **IP address or port** → `hunt_firewall` for perimeter connections. An + *internal* IP also goes to `hunt_dns_web` — Umbrella rows name the AD + user or roaming-client machine behind the address, so that is how you + turn an IP into a who. + - **A user or a hostname** → `hunt_dns_web` (`surface="dns"` or `"web"`). + Umbrella identities are searchable, so "what did this host resolve" is + one call. Do not go hunting for an IP-to-user mapping in other platforms + before trying this: the identity is in the same row as the query. - **Never send a domain to `hunt_firewall`.** The firewall (CEF) table carries essentially no URL data — on a validated workspace, well under 1% of rows had anything in a URL field. A domain query against it comes @@ -36,7 +43,7 @@ Base tool names: `list_data_sources`, `hunt_firewall`, `hunt_dns_web`, 3. Start with `action="blocked"` to see what controls already caught, then `action="allowed"` to find what got through. What was allowed is usually the more urgent half. -4. For a user rather than an address, use `surface="vpn"` for remote-access +4. To widen from a single user: `surface="vpn"` for their remote-access sessions, or `search_office_activity` for what they touched in M365. `search_office_activity` reads the same Microsoft 365 audit data as Purview's `search_audit_log` but through Log Analytics — it answers in @@ -57,6 +64,14 @@ Base tool names: `list_data_sources`, `hunt_firewall`, `hunt_dns_web`, as substring matches, not exact ones. - Firewall data is Sentinel-only. For endpoint process/network telemetry use the Defender or LimaCharlie tools instead. +- **An external IP is not a person.** Umbrella's ExternalIp_s is the NAT + egress address of a whole site, routinely shared by dozens of internal + hosts; a single external IP mapping to many internal IPs is normal NAT, not + a finding. Attribute to the `Identities_s` on the row, never by matching an + external IP against a user seen on that same IP in another product — that + reasoning names one arbitrary user out of everyone behind the NAT, and it + reads as confident attribution. If you need a who, search the identity + fields directly. - **"No rows" and "no table" are different findings.** If `hunt_firewall` or `hunt_dns_web` returns a posture finding saying the table is absent, that is a visibility gap — report it as one, never as "checked, nothing suspicious". From 194aead78a77ddc2c711f5a3754cd784f8378729 Mon Sep 17 00:00:00 2001 From: James Pichardo Date: Wed, 12 Aug 2026 10:17:09 -0400 Subject: [PATCH 5/7] feat(sentinel): reach the Umbrella cloud firewall as a second hunt_firewall surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cisco_Umbrella_firewall_CL (10.6M rows/7d) was the last table in the workspace no tool could reach. It is not a duplicate of the CEF table behind hunt_firewall: those are on-prem appliances seeing traffic that crosses the office network, while this is Umbrella's cloud-delivered firewall seeing roaming and remote clients that never touch the perimeter at all. The measured difference is what makes it worth reaching. CommonSecurityLog carries 108M rows/7d with a named user on 0.14% of them; Cisco_Umbrella_firewall_CL carries 10.6M with one on 100% (Identity_Type_s "AD Users" on 3,629,629 of 3,630,796 rows in 24h, 286 distinct). Ten times smaller and fully attributed — it answers "which user opened this connection", which the perimeter firewall structurally cannot. SourceIP, destination IP/port and byte counts are 100% populated too, so volume questions become answerable. Added as a second surface rather than a new tool: it mirrors hunt_dns_web's shape, which the model already drives, and keeps the server at seven tools under the ~8 ceiling. The Surface dataclass already carried per-surface action maps, indicator fields and junk filters, so the differing vocabulary needed no new machinery. Identity leads indicator_fields, making it both the primary search field and the aggregate group-by, so a bare call answers "which users generated this traffic, allowed vs blocked". Ports are string-typed here, so unlike CEF's int DestinationPort they need no port_field special case. FQDNS_s (2.5% populated), Destination_Country_s (1%) and App_ID_s (0.8%) are deliberately absent from indicator_fields: searching them would answer "no such traffic" to questions that were really "that column is mostly empty" — the same defect this branch just removed from the dns surface. The connector's ingested CSV header row appears here as verdict_s == "Action" (~1,152/24h), filtered by the existing hygiene clause. Verified live: a bare cloud call returns the top users by verdict with no junk rows; action="blocked" finds the 9 BLOCK flows in 24h; the perimeter default is unchanged. Worth noting operationally — 3,629,635 ALLOW against 9 BLOCK means that firewall is effectively in monitor mode, which is a posture finding for the tenant rather than anything about this code. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ --- docs/reference/tools/sentinel.md | 22 +++++--- .../sentinel-mcp/f0_sentinel_mcp/normalize.py | 31 ++++++++++ .../sentinel-mcp/f0_sentinel_mcp/server.py | 26 ++++++--- servers/sentinel-mcp/f0_sentinel_mcp/tools.py | 17 +++++- servers/sentinel-mcp/tests/test_normalize.py | 56 +++++++++++++++++++ servers/sentinel-mcp/tests/test_tools.py | 39 +++++++++++++ .../sentinel/network-investigation/SKILL.md | 17 ++++-- 7 files changed, 183 insertions(+), 25 deletions(-) diff --git a/docs/reference/tools/sentinel.md b/docs/reference/tools/sentinel.md index 5e4cfb2..1890c87 100644 --- a/docs/reference/tools/sentinel.md +++ b/docs/reference/tools/sentinel.md @@ -23,17 +23,23 @@ Used by skills: [`data-source-coverage`](../../../skills/sentinel/data-source-co ## `hunt_firewall` -SEARCH firewall traffic (Check Point / Fortinet) for an IP or port. - -Use for questions about network connections, blocked traffic, or a -suspicious IP talking through the perimeter. `indicator` must be an IP -ADDRESS or PORT NUMBER — this table carries almost no URLs or usernames, so -a domain here finds nothing: for domains, URLs and web categories use -hunt_dns_web instead. Without an indicator this returns an aggregate -(top talkers by action), not individual events. +SEARCH firewall traffic for an IP, a port, or (cloud only) a user. + +Two different firewalls, so pick by where the traffic went. perimeter — +the on-prem CEF appliances (Check Point / Fortinet); use for connections +crossing the office network. Its `indicator` must be an IP ADDRESS or PORT +NUMBER: it carries almost no URLs or usernames. cloud — Cisco Umbrella's +cloud-delivered firewall, which sees roaming and remote clients that never +reach the perimeter at all; every one of its flows names the AD user, so +`indicator` may also be a username, and a bare call aggregates by user. +If you need to know WHO made a connection, use surface="cloud"; the +perimeter surface cannot answer it. For domains, URLs and web categories +use hunt_dns_web instead. Without an indicator this returns an aggregate, +not individual events. | Parameter | Type | Default | |---|---|---| +| `surface` | `"perimeter"` \| `"cloud"` | `"perimeter"` | | `action` | `"allowed"` \| `"blocked"` \| `"detected"` \| `"any"` | `"any"` | | `indicator` | `string` | `""` | | `hours_back` | `number` | `24` | diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py b/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py index d44ece8..6a0bb36 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/normalize.py @@ -20,6 +20,12 @@ ACTIONS = ("allowed", "blocked", "detected", "any") SURFACES = ("dns", "web", "vpn") +# The two firewalls are complements, not duplicates. The perimeter (CEF) +# appliances see traffic that crosses the office network; Umbrella's +# cloud-delivered firewall sees roaming clients that never touch it. Live +# 2026-08-12: CommonSecurityLog 108M rows/7d with a named user on 0.14% of +# them, Cisco_Umbrella_firewall_CL 10.6M rows/7d with one on 100%. +FIREWALL_SURFACES: dict[str, str] = {"perimeter": "firewall", "cloud": "cloud_firewall"} WORKLOADS = ("sharepoint", "onedrive", "exchange", "teams", "any") DEFAULT_HOURS = 24.0 @@ -77,6 +83,31 @@ class Surface: indicator_kind="net", port_field="DestinationPort", ), + "cloud_firewall": Surface( + table="Cisco_Umbrella_firewall_CL", + action_field="verdict_s", + # Live 24h: ALLOW 3,629,635 / BLOCK 9. There is no "detected" verdict + # on this surface, so the bucket is absent rather than guessed into. + action_map={"allowed": ("ALLOW",), "blocked": ("BLOCK",)}, + # Identity first: it is both the most useful thing to search and the + # aggregate group-by, so a bare call answers "which users generated + # this traffic, allowed vs blocked" -- the question the perimeter + # firewall cannot answer. Ports are string-typed here (unlike CEF's + # int DestinationPort), so they need no port_field special case. + indicator_fields=( + "Identity_s", "SourceIP", "destinationIp_s", "destinationPort_s", + ), + project=( + "TimeGenerated", "verdict_s", "Identity_s", "SourceIP", + "destinationIp_s", "destinationPort_s", "ipProtocol_s", + "Bytes_Sent_s", "Bytes_Received_s", + ), + # Live fill rates make FQDNS_s (2.5%), Destination_Country_s (1%) and + # App_ID_s (0.8%) unusable as search fields: a miss would read as "no + # such traffic" when it means "that column is mostly empty". + indicator_kind="flow", + junk=("Action",), + ), "dns": Surface( table="Cisco_Umbrella_dns_CL", action_field="Action_s", diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/server.py b/servers/sentinel-mcp/f0_sentinel_mcp/server.py index b0fa929..a549d1e 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/server.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/server.py @@ -49,21 +49,29 @@ async def list_data_sources(limit: int = 25) -> list[dict[str, Any]]: @mcp.tool() @guarded_tool("sentinel") async def hunt_firewall( + surface: Literal["perimeter", "cloud"] = "perimeter", action: Literal["allowed", "blocked", "detected", "any"] = "any", indicator: str = "", hours_back: float = 24, limit: int = 25, ) -> list[dict[str, Any]]: - """SEARCH firewall traffic (Check Point / Fortinet) for an IP or port. - - Use for questions about network connections, blocked traffic, or a - suspicious IP talking through the perimeter. `indicator` must be an IP - ADDRESS or PORT NUMBER — this table carries almost no URLs or usernames, so - a domain here finds nothing: for domains, URLs and web categories use - hunt_dns_web instead. Without an indicator this returns an aggregate - (top talkers by action), not individual events.""" + """SEARCH firewall traffic for an IP, a port, or (cloud only) a user. + + Two different firewalls, so pick by where the traffic went. perimeter — + the on-prem CEF appliances (Check Point / Fortinet); use for connections + crossing the office network. Its `indicator` must be an IP ADDRESS or PORT + NUMBER: it carries almost no URLs or usernames. cloud — Cisco Umbrella's + cloud-delivered firewall, which sees roaming and remote clients that never + reach the perimeter at all; every one of its flows names the AD user, so + `indicator` may also be a username, and a bare call aggregates by user. + If you need to know WHO made a connection, use surface="cloud"; the + perimeter surface cannot answer it. For domains, URLs and web categories + use hunt_dns_web instead. Without an indicator this returns an aggregate, + not individual events.""" async with _client() as c: - return _render(await tools.hunt_firewall(c, action, indicator, hours_back, limit)) + return _render( + await tools.hunt_firewall(c, surface, action, indicator, hours_back, limit) + ) @mcp.tool() diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py index d576d1a..c73b60f 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py @@ -174,6 +174,8 @@ async def list_data_sources(client: Any, limit: int = 25) -> list[Finding]: "usernames — for domains and URLs use hunt_dns_web)", "domain": "a domain, URL fragment, IP address, or an Umbrella identity " "(the AD user or roaming-client machine name that made the request)", + "flow": "an IP address, a port number, or the identity (AD user) behind " + "the flow — this table carries no usable URL, domain or country data", } @@ -306,17 +308,26 @@ async def _run_surface( ) +_FIREWALL_HUMAN = { + "perimeter": "perimeter firewall (CEF)", + "cloud": "cloud firewall (Cisco Umbrella)", +} + + async def hunt_firewall( client: Any, + surface: str = "perimeter", action: str = "any", indicator: str = "", hours_back: float = 24, limit: int = 25, ) -> list[Finding]: - """Firewall traffic from the CEF table (Check Point / Fortinet).""" + """Firewall traffic: on-prem CEF appliances, or Umbrella's cloud firewall.""" + if surface not in n.FIREWALL_SURFACES: + return [_bad_arg("surface", surface, ", ".join(n.FIREWALL_SURFACES))] return await _run_surface( - client, n.SURFACE_SPECS["firewall"], - cap="Sentinel firewall telemetry", human="firewall (CEF)", + client, n.SURFACE_SPECS[n.FIREWALL_SURFACES[surface]], + cap=f"Sentinel {surface} firewall telemetry", human=_FIREWALL_HUMAN[surface], action=action, indicator=indicator, hours_back=hours_back, limit=limit, ) diff --git a/servers/sentinel-mcp/tests/test_normalize.py b/servers/sentinel-mcp/tests/test_normalize.py index 44706a2..ae84040 100644 --- a/servers/sentinel-mcp/tests/test_normalize.py +++ b/servers/sentinel-mcp/tests/test_normalize.py @@ -172,3 +172,59 @@ def test_identity_is_projected_on_the_umbrella_surfaces(): """Returning the answer matters as much as being able to search for it.""" for surface in ("dns", "web"): assert "Identities_s" in n.SURFACE_SPECS[surface].project + + +# --- Umbrella cloud firewall (CDFW) ------------------------------------- +# Live-verified 2026-08-12 (24h, 3,630,796 rows): Identity_s 100% populated / +# 286 distinct, Identity_Type_s "AD Users" on 3,629,629 rows; SourceIP, +# destinationIp_s, destinationPort_s and Bytes_Sent_s all 100%. By contrast +# CommonSecurityLog carries SourceUserName on 0.14% of 108M rows/7d — the two +# firewalls are complements, not duplicates: the perimeter sees the traffic, +# the cloud sees who made it. + +def test_cloud_firewall_surface_targets_the_umbrella_table(): + assert n.SURFACE_SPECS["cloud_firewall"].table == "Cisco_Umbrella_firewall_CL" + + +def test_firewall_surface_names_map_to_specs(): + assert n.FIREWALL_SURFACES["perimeter"] == "firewall" + assert n.FIREWALL_SURFACES["cloud"] == "cloud_firewall" + + +def test_cloud_firewall_action_vocabulary_is_its_own(): + """Live values are ALLOW/BLOCK, not the CEF Accept/Drop/Detect.""" + spec = n.SURFACE_SPECS["cloud_firewall"] + assert "ALLOW" in n.action_clause(spec, "allowed") + assert "BLOCK" in n.action_clause(spec, "blocked") + + +def test_cloud_firewall_drops_the_ingested_csv_header_row(): + """verdict_s == "Action" is a header row ingested as data (~1,152/24h).""" + clause = n.hygiene_clause(n.SURFACE_SPECS["cloud_firewall"]) + assert "verdict_s !in~" in clause + assert '"Action"' in clause + + +def test_cloud_firewall_is_searchable_by_identity_ip_and_port(): + clause = n.indicator_clause(n.SURFACE_SPECS["cloud_firewall"], "10.1.2.3") + for f in ("Identity_s", "SourceIP", "destinationIp_s", "destinationPort_s"): + assert f"{f} has" in clause + + +def test_cloud_firewall_aggregates_by_identity(): + """Grouping by user is what this table adds over the perimeter firewall.""" + assert n.SURFACE_SPECS["cloud_firewall"].indicator_fields[0] == "Identity_s" + + +def test_cloud_firewall_does_not_advertise_its_empty_columns(): + """FQDNS 2.5%, Destination_Country 1%, App_ID 0.8% — searching them would + answer "nothing found" to questions that were never really asked.""" + spec = n.SURFACE_SPECS["cloud_firewall"] + for absent in ("FQDNS_s", "Destination_Country_s", "App_ID_s"): + assert absent not in spec.indicator_fields + + +def test_a_flow_indicator_accepts_an_identity_and_a_port(): + assert n.validate_indicator("someone@example.gob.do", "flow") is True + assert n.validate_indicator("443", "flow") is True + assert n.validate_indicator('x" or 1==1', "flow") is False diff --git a/servers/sentinel-mcp/tests/test_tools.py b/servers/sentinel-mcp/tests/test_tools.py index d71054e..bd8e21e 100644 --- a/servers/sentinel-mcp/tests/test_tools.py +++ b/servers/sentinel-mcp/tests/test_tools.py @@ -1158,3 +1158,42 @@ async def test_run_kql_discloses_truncation(fake): client = fake(rows={USAGE: _TABLES, "Heartbeat": rows}) out = await tools.run_kql(client, "Heartbeat", limit=5) assert any("more results available" in f.title for f in out) + + +# --- hunt_firewall: perimeter + cloud ------------------------------------ +UFW = "Cisco_Umbrella_firewall_CL" + + +async def test_hunt_firewall_still_defaults_to_the_perimeter_table(fake): + """Adding a surface must not move the default out from under existing callers.""" + client = fake(rows={USAGE: _TABLES, CEF: []}) + await tools.hunt_firewall(client) + assert any(CEF in q for q in client.queries) + assert not any(UFW in q for q in client.queries) + + +async def test_hunt_firewall_cloud_surface_queries_the_umbrella_table(fake): + client = fake(rows={USAGE: _TABLES + [{"DataType": UFW, "GB": 12.3}], UFW: []}) + await tools.hunt_firewall(client, surface="cloud") + assert any(UFW in q for q in client.queries) + + +async def test_hunt_firewall_rejects_an_unknown_surface(fake): + client = fake(rows={USAGE: _TABLES}) + out = await tools.hunt_firewall(client, surface="datacenter") + assert "surface" in out[0].title + assert not any(CEF in q or UFW in q for q in client.queries) + + +async def test_hunt_firewall_cloud_searches_identity(fake): + client = fake(rows={USAGE: _TABLES + [{"DataType": UFW, "GB": 12.3}], UFW: []}) + await tools.hunt_firewall(client, surface="cloud", indicator="someone@example.gob.do") + kql = [q for q in client.queries if UFW in q][0] + assert 'Identity_s has "someone@example.gob.do"' in kql + + +async def test_hunt_firewall_perimeter_still_rejects_an_identity_indicator(fake): + """The CEF table carries usernames on 0.14% of rows; it is IP/port only.""" + client = fake(rows={USAGE: _TABLES, CEF: []}) + out = await tools.hunt_firewall(client, indicator="someone@example.gob.do") + assert "indicator" in out[0].title diff --git a/skills/sentinel/network-investigation/SKILL.md b/skills/sentinel/network-investigation/SKILL.md index ec19cef..ad5a0e2 100644 --- a/skills/sentinel/network-investigation/SKILL.md +++ b/skills/sentinel/network-investigation/SKILL.md @@ -27,11 +27,18 @@ Base tool names: `list_data_sources`, `hunt_firewall`, `hunt_dns_web`, in this skill: - **Domain or URL** → `hunt_dns_web` (`surface="dns"` for resolutions, `surface="web"` for fetches and downloads). - - **IP address or port** → `hunt_firewall` for perimeter connections. An - *internal* IP also goes to `hunt_dns_web` — Umbrella rows name the AD - user or roaming-client machine behind the address, so that is how you - turn an IP into a who. - - **A user or a hostname** → `hunt_dns_web` (`surface="dns"` or `"web"`). + - **IP address or port** → `hunt_firewall`. Choose the surface by where + the traffic went: `surface="perimeter"` (default) for the on-prem CEF + appliances, `surface="cloud"` for Umbrella's cloud firewall, which sees + roaming and remote clients that never reach the perimeter. **Check both + before concluding a host had no network activity** — on a validated + workspace the perimeter carried a named user on 0.14% of rows and the + cloud firewall on 100%, so they answer different questions and neither + is a superset. An *internal* IP also goes to `hunt_dns_web`, whose rows + name the AD user behind the address. + - **A user or a hostname** → `hunt_dns_web` (`surface="dns"` or `"web"`) + for name resolution and web fetches, or `hunt_firewall` + (`surface="cloud"`) for their L3/L4 connections. Umbrella identities are searchable, so "what did this host resolve" is one call. Do not go hunting for an IP-to-user mapping in other platforms before trying this: the identity is in the same row as the query. From ce560773e573ca339a93dd198182b0b124e020f9 Mon Sep 17 00:00:00 2001 From: James Pichardo Date: Wed, 12 Aug 2026 10:27:11 -0400 Subject: [PATCH 6/7] fix: close three gaps found by the PR #104 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Aggregate mode never disclosed truncation.** `_run_surface`'s no-indicator branch asked for `| top {limit}`, which can never return more than `limit` rows, so `_split_page`'s `len(rows) > limit` was structurally always False. The row path was fixed in 0a0a389 and this branch was missed, making that commit's claim — "every row-returning tool now fetches limit + 1" — untrue for every hunt_* aggregate. It bites hardest on the surface added in 194aead: cloud_firewall aggregates by identity across 286 distinct users against a default limit of 25, so a bare call hid 261 of them silently. The first test written for this passed before the fix. The fake client returns its canned rows whatever the query says, so asserting on returned rows proved only that the fake ignores KQL. The assertion is now on the emitted query, which is the actual contract with the platform. **An audited write could report as though it had not happened.** A gated action runs the platform call and then records the audit entry; the write must come first, or the record could not describe its result. If that record throws, the new guarded_tool caught it and rendered "temporarily unavailable" — a degradation, which says the opposite of what occurred. In chat-confirm mode the confirmation is deliberately not single-use, so a model reading that and retrying would execute the action a second time. `AuditWriteFailed` now carries `action_executed = True`, and the boundary renders it as a high-severity action finding that says the action took effect and must not be retried. Duck-typed, so the redaction layer keeps no dependency on the gating layer. **A credential file was injected wholesale.** `load_platform_env` loaded every variable in the file it found. Critical Rule 7 is per-platform isolation, so a `.env.defender` was never meant to be able to set `ENTRA_CLIENT_SECRET` — and since the search added in dace53e walks parent directories, a file placed in a shared ancestor could also set process-wide knobs. `HTTPS_PROXY` is the sharp one: every client here builds `httpx.AsyncClient` with the default `trust_env=True`, so it would be honoured on calls carrying a live token. Only `_*` keys are injected now, via `setdefault` so an exported variable still wins. Every documented variable across all nine `.env.*.example` files is already prefixed, so nothing supported changes; the user guide notes it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ --- core/f0_sectools_core/auth/env.py | 20 +++++++++++-- core/f0_sectools_core/gating/actions.py | 28 +++++++++++++++++-- core/f0_sectools_core/redaction/boundary.py | 26 +++++++++++++++++ core/tests/test_auth_env.py | 18 ++++++++++++ core/tests/test_gating.py | 28 +++++++++++++++++++ core/tests/test_tool_boundary.py | 21 ++++++++++++++ docs/user-guide/troubleshooting.md | 7 +++++ servers/sentinel-mcp/f0_sentinel_mcp/tools.py | 5 +++- servers/sentinel-mcp/tests/test_tools.py | 22 +++++++++++++++ 9 files changed, 169 insertions(+), 6 deletions(-) diff --git a/core/f0_sectools_core/auth/env.py b/core/f0_sectools_core/auth/env.py index 4473792..30cae7d 100644 --- a/core/f0_sectools_core/auth/env.py +++ b/core/f0_sectools_core/auth/env.py @@ -19,6 +19,10 @@ 3. The installed package's directory and each of its ancestors -- reached when the checkout is nowhere near the working directory at all. +Only variables named ``_*`` are injected; anything else in the +file is ignored, so one platform's file can neither set another's +credential nor alter the process environment. + A file that is *not* found is not an error: supplying credentials as real environment variables is a supported deployment, and ``python-dotenv`` never overwrites a variable that is already set, so the surrounding environment @@ -33,7 +37,7 @@ import os from pathlib import Path -from dotenv import load_dotenv +from dotenv import dotenv_values __all__ = ["env_search_dirs", "find_platform_env", "load_platform_env"] @@ -83,6 +87,16 @@ def load_platform_env(platform: str) -> Path | None: -- never log or return the file's *contents*. """ path = find_platform_env(platform) - if path is not None: - load_dotenv(path) + if path is None: + return None + # Only this platform's own variables are injected. Two reasons, both + # Critical Rules: Rule 7 is per-platform credential isolation, and a file + # loaded wholesale could also set process-wide knobs -- HTTPS_PROXY is the + # sharp one, since httpx honours it (trust_env) on calls carrying a live + # token. `setdefault` keeps dotenv's override=False semantics: a variable + # already exported wins. + prefix = f"{platform.upper()}_" + for key, value in dotenv_values(path).items(): + if value is not None and key.startswith(prefix): + os.environ.setdefault(key, value) return path diff --git a/core/f0_sectools_core/gating/actions.py b/core/f0_sectools_core/gating/actions.py index 26f6921..69c8486 100644 --- a/core/f0_sectools_core/gating/actions.py +++ b/core/f0_sectools_core/gating/actions.py @@ -215,6 +215,21 @@ def consume(self, action: str, target: str) -> bool: return True +class AuditWriteFailed(RuntimeError): + """The platform write succeeded but recording it to the audit trail did not. + + Rule 8 requires every write to be audited, and the write necessarily runs + before the record can describe its result -- so this window cannot be + designed away, only reported honestly. ``action_executed`` is read + (duck-typed, no import) by ``core/redaction/boundary.py`` so the caller is + told the action TOOK EFFECT rather than being handed a generic failure it + might retry. That matters most in chat-confirm mode, where the token is not + single-use and a retry would execute the action a second time. + """ + + action_executed = True + + class GatedAction: def __init__( self, @@ -270,12 +285,21 @@ def _audit(self, target: str, actor: str, token: str | None, method: str) -> Non ) self.audit.record(self.name, target, actor, token or "", method=method, ref=ref) + def _audit_or_flag(self, target: str, actor: str, token: str | None, method: str) -> None: + """Audit the completed write, or fail in a way that says it completed.""" + try: + self._audit(target, actor, token, method) + except Exception as exc: + raise AuditWriteFailed( + f"{self.name} executed against {target} but the audit record failed" + ) from exc + def execute( self, *, target: str, actor: str, token: str | None, run: Callable[[], Any] ) -> Any: method = self._authorize(target, token) result = run() - self._audit(target, actor, token, method) + self._audit_or_flag(target, actor, token, method) return result async def execute_async( @@ -288,5 +312,5 @@ async def execute_async( ) -> Any: method = self._authorize(target, token) result = await run() - self._audit(target, actor, token, method) + self._audit_or_flag(target, actor, token, method) return result diff --git a/core/f0_sectools_core/redaction/boundary.py b/core/f0_sectools_core/redaction/boundary.py index e9249a6..2d29481 100644 --- a/core/f0_sectools_core/redaction/boundary.py +++ b/core/f0_sectools_core/redaction/boundary.py @@ -57,6 +57,32 @@ def unexpected_error_finding(source: str, capability: str, exc: BaseException) - detail = str(exc) if len(detail) > MAX_ERROR_CHARS: detail = detail[:MAX_ERROR_CHARS] + "…" + + # A gated write runs the platform call first and records the audit entry + # after it. If that record fails, the state change ALREADY HAPPENED, and + # reporting it as a degradation would say the opposite. Any exception may + # opt out of the degradation wording by carrying `action_executed = True`; + # duck-typed so this module keeps no dependency on core/gating. + if getattr(exc, "action_executed", False): + return Finding( + source=source, + finding_type=FindingType.action, + severity=Severity.high, + title=f"{capability} EXECUTED on the platform but was not audited " + f"({type(exc).__name__}) — do not retry", + entity=Entity(kind=EntityKind.tenant, id=source), + evidence=[ + Evidence(key="error_type", value=type(exc).__name__), + Evidence(key="error", value=detail), + Evidence(key="action_executed", value="true"), + ], + recommended_action=RecommendedAction( + summary="The action took effect. Do NOT retry — a retry would " + "repeat it. Record it manually, then fix the audit trail " + "(disk space and permissions on $F0_GATING_DIR).", + ), + ) + return Finding( source=source, finding_type=FindingType.posture, diff --git a/core/tests/test_auth_env.py b/core/tests/test_auth_env.py index 74271a6..db8d294 100644 --- a/core/tests/test_auth_env.py +++ b/core/tests/test_auth_env.py @@ -117,3 +117,21 @@ def test_missing_vars_error_never_leaks_a_value(checkout, monkeypatch): with pytest.raises(ValueError) as e: PlatformConfig.from_env(FAKE.upper(), env={}) assert "super-secret-value" not in str(e.value) + + +def test_only_the_platforms_own_variables_are_loaded(tmp_path, monkeypatch): + """Critical Rule 7 is per-platform isolation: a platform's file must not be + able to set another platform's credential, nor process-wide knobs like + HTTPS_PROXY, which httpx honours (trust_env) on calls carrying a token.""" + monkeypatch.delenv("F0_SECTOOLS_ENV_DIR", raising=False) + for var in (f"{FAKE.upper()}_TOKEN", "OTHERPLAT_CLIENT_SECRET", "HTTPS_PROXY"): + monkeypatch.delenv(var, raising=False) + (tmp_path / f".env.{FAKE}").write_text( + f"{FAKE.upper()}_TOKEN=mine\nOTHERPLAT_CLIENT_SECRET=stolen\n" + "HTTPS_PROXY=http://attacker.example:8080\n" + ) + monkeypatch.chdir(tmp_path) + load_platform_env(FAKE) + assert os.environ[f"{FAKE.upper()}_TOKEN"] == "mine" + assert "OTHERPLAT_CLIENT_SECRET" not in os.environ + assert "HTTPS_PROXY" not in os.environ diff --git a/core/tests/test_gating.py b/core/tests/test_gating.py index e1359fd..2eaf718 100644 --- a/core/tests/test_gating.py +++ b/core/tests/test_gating.py @@ -5,6 +5,7 @@ from f0_sectools_core.gating.actions import ( ApprovalStore, AuditLog, + AuditWriteFailed, GatedAction, GateDenied, TokenStore, @@ -382,3 +383,30 @@ def test_chat_mode_empty_target_and_token_denied(tmp_path): g = _gate(tmp_path, enabled=True, confirm_mode="chat") with pytest.raises(GateDenied): g.execute(target="", actor="james", token="", run=lambda: "ok") + + +# ── audit-write failure must not look like "the write did not happen" ── +def test_audit_failure_reports_that_the_action_already_executed(tmp_path): + """Rule 8. The platform call runs before the audit record can describe it, + so this window cannot be designed away — only reported honestly. A generic + failure here would invite a retry, and in chat-confirm mode the token is + not single-use, so the retry would execute the action a second time.""" + ran = [] + g = GatedAction( + "defender.isolate_host", + enabled=True, + audit=AuditLog(str(tmp_path / "a.log")), + token_store=TokenStore(str(tmp_path / "pending")), + ) + tok = g.token_store.issue("defender.isolate_host", "web-01") + + def boom(*a, **k): + raise OSError("read-only file system") + + g.audit.record = boom # type: ignore[method-assign] + with pytest.raises(AuditWriteFailed) as e: + g.execute(target="web-01", actor="james", token=tok, run=lambda: ran.append("x")) + + assert ran == ["x"], "the platform call really did run" + assert getattr(e.value, "action_executed", False) is True + assert "web-01" in str(e.value) diff --git a/core/tests/test_tool_boundary.py b/core/tests/test_tool_boundary.py index d3c9721..45251d7 100644 --- a/core/tests/test_tool_boundary.py +++ b/core/tests/test_tool_boundary.py @@ -121,3 +121,24 @@ def test_every_registered_tool_is_guarded(server): assert unguarded == [], f"tools missing @guarded_tool: {unguarded}" assert len(sources) == 1, f"one server must report one source, got {sources}" assert sources.pop(), "guarded_tool source must be a non-empty string" + + +async def test_a_write_that_executed_but_failed_to_audit_is_not_a_degradation(): + """Rule 8. The gated write runs, THEN the audit record is written. If the + audit throws, a generic "temporarily unavailable" finding says nothing + happened — while the live platform change already happened. In chat-confirm + mode the token is not single-use, so a model that retries re-executes.""" + + class Executed(RuntimeError): + action_executed = True + + @guarded_tool("projectachilles") + async def run_test(): + raise Executed("audit trail write failed") + + out = (await run_test())[0] + assert out["severity"] == "high" + assert "temporarily unavailable" not in out["title"], "must not read as a degradation" + assert not any(m in out["title"] for m in DEGRADATION_MARKERS) + assert "executed" in out["title"].lower() + assert "not retry" in str(out).lower() diff --git a/docs/user-guide/troubleshooting.md b/docs/user-guide/troubleshooting.md index 3a76244..cf19ed1 100644 --- a/docs/user-guide/troubleshooting.md +++ b/docs/user-guide/troubleshooting.md @@ -53,6 +53,13 @@ a subdirectory of the repo works. Variables already exported in the environment always win over the file, which is how container and systemd deployments supply credentials without a file at all. +**Only `_*` variables are read from the file.** Anything else in it is +ignored, so one platform's file cannot set another platform's credential or +change the process environment (a stray `HTTPS_PROXY` in a `.env` would +otherwise be honoured on outbound calls carrying a live token). If you need a +proxy or other process-wide setting, export it in the environment that launches +your runtime rather than putting it in a `.env.` file. + ## Tool not found / wrong name Runtimes prefix MCP tool names differently (Hermes diff --git a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py index c73b60f..b782cb4 100644 --- a/servers/sentinel-mcp/f0_sentinel_mcp/tools.py +++ b/servers/sentinel-mcp/f0_sentinel_mcp/tools.py @@ -276,7 +276,10 @@ async def _run_surface( parts.append( f"| summarize Events=count() by {spec.action_field}, {spec.indicator_fields[0]}" ) - parts.append(f"| top {limit} by Events desc") + # limit + 1 here as well: `top {limit}` can never return more than + # limit rows, so has_more was structurally always False and the + # aggregate silently hid every group past the cut. + parts.append(f"| top {_fetch_bound(limit)} by Events desc") kql = " ".join(p for p in parts if p) try: diff --git a/servers/sentinel-mcp/tests/test_tools.py b/servers/sentinel-mcp/tests/test_tools.py index bd8e21e..f8f10c2 100644 --- a/servers/sentinel-mcp/tests/test_tools.py +++ b/servers/sentinel-mcp/tests/test_tools.py @@ -1197,3 +1197,25 @@ async def test_hunt_firewall_perimeter_still_rejects_an_identity_indicator(fake) client = fake(rows={USAGE: _TABLES, CEF: []}) out = await tools.hunt_firewall(client, indicator="someone@example.gob.do") assert "indicator" in out[0].title + + +async def test_aggregate_mode_discloses_truncation(fake): + """`top {limit}` can never return more than limit, so has_more was always + False — the row path was fixed but the aggregate path kept truncating + silently. cloud_firewall has 286 distinct identities against a default + limit of 25, so a bare call hides 261 users without saying so.""" + client = fake(rows={USAGE: _TABLES + [{"DataType": UFW, "GB": 1.0}], UFW: []}) + await tools.hunt_firewall(client, surface="cloud", limit=5) + kql = [q for q in client.queries if UFW in q][0] + # Asserted on the emitted KQL, not on rows the fake hands back: the fake + # returns its canned list whatever the query says, so a row-count assertion + # here would pass against the very bug it is meant to catch. + assert "| top 6 by Events desc" in kql, "aggregate must fetch limit + 1 too" + + +async def test_aggregate_mode_silent_when_nothing_hidden(fake): + rows = [{"verdict_s": "ALLOW", "Identity_s": f"user{i}", "Events": 9 - i} for i in range(5)] + client = fake(rows={USAGE: _TABLES + [{"DataType": UFW, "GB": 1.0}], UFW: rows}) + out = await tools.hunt_firewall(client, surface="cloud", limit=5) + assert not any("more results available" in f.title for f in out) + assert sum(f.finding_type.value == "hunt_result" for f in out) == 5 From e9939057487e5040290a7d88560bf966e4aef7c6 Mon Sep 17 00:00:00 2001 From: James Pichardo Date: Wed, 12 Aug 2026 10:33:22 -0400 Subject: [PATCH 7/7] fix(docs): stop documenting F0_GATING_DIR in files that no longer load it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit claimed every documented variable across the .env.*.example files was platform-prefixed. That was wrong, and the check behind it was wrong: it matched `^[A-Z0-9_]+=` and so skipped commented lines, which in an example file are exactly the instructions operators copy. Both the defender and projectachilles examples carried `# F0_GATING_DIR=...`, deliberately un-prefixed because servers and scripts/confirm_action.py share one gating root. After the prefix filter that line is silently ignored, so an operator who uncommented it would get the default directory, no warning, and a watcher CLI looking somewhere else. Fixed by removing the line rather than allowlisting the variable. Allowlisting would let any discovered .env file relocate where approvals, tokens and the audit log live — and since these files are now found by walking parent directories, that is a strictly worse version of the vector the prefix filter just closed. Rule 8 wants the audit trail somewhere a credential file cannot move it. Both examples now say to export it in the environment instead, which was already the only fully-correct way to set it: the confirm CLI is a separate process that never reads these files. A test now enforces it, since the manual check is what failed: no .env.*.example may document an assignment — commented or not — that load_platform_env would not inject. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ --- core/tests/test_auth_env.py | 27 +++++++++++++++++++ docs/user-guide/gated-actions.md | 3 ++- servers/defender-mcp/.env.defender.example | 6 ++++- .../.env.projectachilles.example | 6 ++++- 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/core/tests/test_auth_env.py b/core/tests/test_auth_env.py index db8d294..fb11b25 100644 --- a/core/tests/test_auth_env.py +++ b/core/tests/test_auth_env.py @@ -1,5 +1,6 @@ """Credential files must be found by where the checkout is, not by how the client was launched.""" import os +import re from pathlib import Path import pytest @@ -135,3 +136,29 @@ def test_only_the_platforms_own_variables_are_loaded(tmp_path, monkeypatch): assert os.environ[f"{FAKE.upper()}_TOKEN"] == "mine" assert "OTHERPLAT_CLIENT_SECRET" not in os.environ assert "HTTPS_PROXY" not in os.environ + + +def test_env_examples_only_document_prefixed_variables(): + """A guard for the claim I got wrong by hand. + + `load_platform_env` injects only `_*` keys, so an example file + that documents anything else is telling operators to set something that is + silently ignored. Commented assignments count: they are copy-paste + instructions. Deliberately shared knobs like F0_GATING_DIR must be real + exported environment variables — the confirm CLI is a separate process and + never reads these files, and letting a discoverable file relocate the audit + trail would undo the isolation this loader exists to provide. + """ + repo = Path(__file__).resolve().parents[2] + assign = re.compile(r"^#?\s*([A-Z][A-Z0-9_]*)\s*=") + offenders = [] + for example in sorted(repo.glob("**/.env.*.example")): + if ".venv" in example.parts: + continue + platform = example.name.removeprefix(".env.").removesuffix(".example") + prefix = f"{platform.upper()}_" + for num, line in enumerate(example.read_text().splitlines(), 1): + m = assign.match(line) + if m and not m.group(1).startswith(prefix): + offenders.append(f"{example.relative_to(repo)}:{num} {m.group(1)}") + assert offenders == [], f"documented but never loaded: {offenders}" diff --git a/docs/user-guide/gated-actions.md b/docs/user-guide/gated-actions.md index 172cbee..33ddcce 100644 --- a/docs/user-guide/gated-actions.md +++ b/docs/user-guide/gated-actions.md @@ -74,7 +74,8 @@ actions (it is deliberately not wired to any). Details and the honest caveat: ## 5. Read the audit trail Every executed action appends a line to `~/.f0sectools/gating/audit.log` -(override the directory with `F0_GATING_DIR` — servers and the CLI must agree +(override the directory by **exporting** `F0_GATING_DIR` in the environment, +never in a `.env.` file — servers and the CLI must agree on it): ```bash diff --git a/servers/defender-mcp/.env.defender.example b/servers/defender-mcp/.env.defender.example index f41e194..9cbe441 100644 --- a/servers/defender-mcp/.env.defender.example +++ b/servers/defender-mcp/.env.defender.example @@ -28,7 +28,11 @@ DEFENDER_VERIFY_TLS=true # it in `python scripts/confirm_action.py --watch`; alternatively # scripts/confirm_action.py prints a one-shot token. DEFENDER_ALLOW_WRITE=false -# F0_GATING_DIR=~/.f0sectools/gating (shared gating-state dir override) +# Gating state (approvals, tokens, audit log) lives in $F0_GATING_DIR, +# default ~/.f0sectools/gating. Set it as a real exported environment +# variable, NOT here: this file is read only for its own _* +# keys, and scripts/confirm_action.py is a separate process that never +# reads it at all — the server and the CLI must agree on the directory. # Recorded as the actor in $F0_GATING_DIR/audit.log (default # ~/.f0sectools/gating/audit.log; application context has no signed-in user). # Optional. diff --git a/servers/projectachilles-mcp/.env.projectachilles.example b/servers/projectachilles-mcp/.env.projectachilles.example index 48e6b33..1815412 100644 --- a/servers/projectachilles-mcp/.env.projectachilles.example +++ b/servers/projectachilles-mcp/.env.projectachilles.example @@ -25,7 +25,11 @@ PROJECTACHILLES_VERIFY_TLS=true # scripts/confirm_action.py (--platform projectachilles) for a one-shot # token. Leave false unless you use the actions server. PROJECTACHILLES_ALLOW_WRITE=false -# F0_GATING_DIR=~/.f0sectools/gating (shared gating-state dir override) +# Gating state (approvals, tokens, audit log) lives in $F0_GATING_DIR, +# default ~/.f0sectools/gating. Set it as a real exported environment +# variable, NOT here: this file is read only for its own _* +# keys, and scripts/confirm_action.py is a separate process that never +# reads it at all — the server and the CLI must agree on the directory. # Confirmation mode for gated writes (projectachilles-actions server): # token (default) — forge-resistant: approve in `confirm_action.py --watch`