diff --git a/.github/workflows/sdk-smoke.yml b/.github/workflows/sdk-smoke.yml index a7767a96e..eba1f8efb 100644 --- a/.github/workflows/sdk-smoke.yml +++ b/.github/workflows/sdk-smoke.yml @@ -44,6 +44,13 @@ jobs: 'livekit-plugins-google==1.8.3' 'livekit-plugins-openai==1.8.3' 'livekit-plugins-silero==1.8.3' + - name: Pin the Claude Agent SDK to the lock's version + # The image installs the SDK at uv.lock's version, so the tests run the Claude Code CLI + # that ships (test_secret_paths.py drives it; TH-9042). Only the SDK is pinned here. + run: >- + python -m pip install "claude-agent-sdk==$(python -c "import tomllib; + print(next(p['version'] for p in tomllib.load(open('uv.lock', 'rb'))['package'] + if p['name'] == 'claude-agent-sdk'))")" - name: Run engine + dispatch + acceptance tests # Scoped to the suites this PR adds/relies on. The broader tests/ tree has # pre-existing failures unrelated to the voice engine (CLI golden-path, @@ -91,6 +98,8 @@ jobs: tests/harness/test_authoring_runtime_validation.py tests/harness/test_failure_file.py tests/harness/test_contract_tools_subfolder.py + tests/harness/test_contract_model.py + tests/harness/test_secret_paths.py tests/harness/test_import_roots.py tests/harness/test_source_data_invariants_folder.py tests/harness/test_hosted_authoring_entrypoint.py diff --git a/src/fi/alk/harness/backends/claude.py b/src/fi/alk/harness/backends/claude.py index 0759766f5..604269e6c 100644 --- a/src/fi/alk/harness/backends/claude.py +++ b/src/fi/alk/harness/backends/claude.py @@ -48,6 +48,7 @@ WorkerSpec, tool_input_schema, ) +from .files import secret_read_rules # The loop is Claude Code; the model it bills is not. DEFAULT_MODEL = "gemini-3.7-flash" @@ -475,11 +476,17 @@ def _options(self, spec: SessionSpec) -> ClaudeAgentOptions: # is consulted, so a stage could rewrite an artifact by hand and skip the tool whose # whole job is to validate that change. options.permission_mode = "default" - options.disallowed_tools = [name for name in UNWANTED if name not in spec.builtins] options.hooks = gate_hooks(allowed) options.can_use_tool = spec.permission_override or permission_gate( spec.ask, allowed ) + # Every session, gated or not, carries the secret list as Read deny rules: Claude Code's own + # Grep and Glob turn them into exclusions, which the gate (it sees only a call's + # arguments) can't do for a search that starts above a credential file. + options.disallowed_tools = [ + *([name for name in UNWANTED if name not in spec.builtins] if spec.gated else []), + *secret_read_rules(), + ] if spec.thinking: options.thinking = thinking_config() return options diff --git a/src/fi/alk/harness/backends/files.py b/src/fi/alk/harness/backends/files.py index f1b9f8641..a6b2176cc 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -39,13 +39,150 @@ def _resolved(raw: str, cwd: Path) -> Path: # Where credentials live in a sandbox or a checkout, and what reads them. A session never needs # any of them: the platform hands secrets to processes as files and environment, not to a model. -_SECRET_PATHS = re.compile( - r"(^|/)(\.env[^/]*|\.claude|\.ssh|\.aws|\.config/gcloud|\.netrc|\.npmrc|\.pypirc" - r"|\.authoring-credentials)(/|$)" - r"|^/proc(/|$)|^/run/futureagi(/|$)|^/run/secrets(/|$)" - r"|(credential|secret|token|service[-_]?account)[^/]*\.json$|\.(pem|key|p12)$", - re.IGNORECASE, +# +# One list of globs, read the way gitignore and ripgrep read them: a glob matches a path when it +# matches the path or any folder above it, `*` stands for any run of characters within one name, +# `**/` at the start means "in any folder", and case is ignored. The kit's own tools, the scan's +# tools and the gate match paths against the regex built from it (`_SECRET_PATHS`); Claude Code +# gets the same list as Read deny rules (`secret_read_rules`), which its Grep and Glob turn into +# exclusions. A glob uses only letters, digits and `.-_*/`: the CLI skips a rule holding `[`, and +# the SDK splits its rules on commas and spaces. +SECRET_GLOBS: tuple[str, ...] = ( + "**/.env*", + "**/*.env", + "**/*.env.*", + "**/.claude", + "**/.ssh", + "**/.aws", + "**/.config/gcloud", + "**/.authoring-credentials", + "**/.netrc", + "**/.npmrc", + "**/.pypirc", + "**/*credential*.json", + "**/*secret*.json", + "**/*token*.json", + "**/*service-account*.json", + "**/*service_account*.json", + "**/*serviceaccount*.json", + "**/*secret*.yaml", + "**/*secret*.yml", + "**/*secret*.toml", + "**/*credential*.yaml", + "**/*credential*.yml", + "**/*credential*.toml", + "**/*secret*.ini", + "**/*secret*.cfg", + "**/*secret*.conf", + "**/*secret*.properties", + "**/*secret*.xml", + "**/*credential*.ini", + "**/*credential*.cfg", + "**/*credential*.conf", + "**/*credential*.properties", + "**/*credential*.xml", + "**/*firebase-adminsdk*.json", + "**/*credential*.csv", + "**/*accesskeys*.csv", + "**/rootkey*.csv", + "**/*secret*.txt", + "**/*credential*.txt", + "**/secrets/*.txt", + "**/.secrets", + "**/.dev.vars", + "**/.dev.vars.*", + "**/local.settings.json", + "**/key.properties", + "**/keystore.properties", + "**/*.p8", + "**/*.pk8", + "**/*.ppk", + "**/.config/gh/hosts.yml", + "**/.databrickscfg", + "**/.terraformrc", + "**/.cargo/credentials", + "**/.gem/credentials", + "**/*.pem", + "**/*.key", + "**/*.p12", + "**/*.pfx", + "**/*.jks", + "**/*.keystore", + "**/.kube/config", + "**/.kube/config.*", + "**/kubeconfig", + "**/kubeconfig.yaml", + "**/kubeconfig.yml", + "**/*.kubeconfig", + "**/.docker/config.json", + "**/.docker/contexts", + "**/.dockercfg", + "**/.pgpass", + "**/.htpasswd", + "**/.git-credentials", + "**/.my.cnf", + "**/.s3cfg", + "**/.boto", + "**/.vault-token", + "**/id_rsa", + "**/id_dsa", + "**/id_ecdsa", + "**/id_ed25519", + "**/id_ecdsa_sk", + "**/id_ed25519_sk", + "**/*.tfstate", + "**/*.tfstate.backup", + "**/*.tfstate.*.backup", + "**/*.tfvars", + "**/*.tfvars.json", + "**/*-key.json", + "**/*_key.json", + "**/*-sa.json", ) +# Every glob with no `*` after its `**/` names one exact file or folder, and its `.bak` copy is a +# credential too (`.netrc.bak`), so those forms are derived from the list rather than written +# out: a name added to SECRET_GLOBS gets its backup form with it. `*.kubeconfig` has a `*` but is +# kept as well. `.kube/config.*` already covers its own. +_WITH_BACKUP: tuple[str, ...] = ( + *(glob for glob in SECRET_GLOBS if "*" not in glob.removeprefix("**/")), + "**/*.kubeconfig", +) +ALL_SECRET_GLOBS: tuple[str, ...] = SECRET_GLOBS + tuple( + dict.fromkeys(f"{glob}.bak" for glob in _WITH_BACKUP) +) +# The settings files a model citation may name, under its own rule (`model_citation`). +_ENV_GLOBS = ("**/.env*", "**/*.env", "**/*.env.*") +# The platform's own roots, which are paths, not names. +_ROOTS = r"^/proc(/|$)|^/run/futureagi(/|$)|^/run/secrets(/|$)" + + +def _glob_pattern(glob: str) -> str: + """The regex for one `**/` glob: the name parts at any depth, as the path or a folder above it.""" + body = glob.removeprefix("**/") + return "(^|/)" + "".join("[^/]*" if char == "*" else re.escape(char) for char in body) + "(/|$)" + + +def _compiled(globs: tuple[str, ...]) -> re.Pattern[str]: + return re.compile("|".join([*(_glob_pattern(glob) for glob in globs), _ROOTS]), re.IGNORECASE) + + +_SECRET_PATHS = _compiled(ALL_SECRET_GLOBS) +# The same, less those settings files: what a model citation may not name. +_SECRET_BUT_ENV = _compiled(tuple(glob for glob in ALL_SECRET_GLOBS if glob not in _ENV_GLOBS)) + + +def secret_text(text: str, *, env_allowed: bool = False) -> bool: + """Whether a path, as given and never resolved, names a place on the list. Callers pass a + path relative to the source, so the folders the source sits in don't count.""" + return bool((_SECRET_BUT_ENV if env_allowed else _SECRET_PATHS).search(text)) + + +def secret_read_rules() -> list[str]: + """Claude Code's Read deny rules for the list, anchored at the file system's root (`//`), so + they hold wherever a search starts and whatever the session's folder is.""" + return [f"Read(//{glob})" for glob in ALL_SECRET_GLOBS] + + # A search rooted here would reach into where credentials live. _SECRET_ROOTS = re.compile(r"^/(proc|run/futureagi|run/secrets)/") SECRET_REFUSAL = ( diff --git a/src/fi/alk/harness/builder.py b/src/fi/alk/harness/builder.py index 55d579355..dac10b348 100644 --- a/src/fi/alk/harness/builder.py +++ b/src/fi/alk/harness/builder.py @@ -279,6 +279,7 @@ async def change_world(args: dict[str, Any]) -> dict[str, Any]: ) async def declare_tool(args: dict[str, Any]) -> dict[str, Any]: from .amend import _persist + from .backends.files import secret_text from .contract import ToolSpec, validate_contract from .source_folder import agent_scope, in_agent_scope @@ -290,7 +291,16 @@ async def declare_tool(args: dict[str, Any]) -> dict[str, Any]: quote = str(args.get("evidence_quote") or "").strip() root = source_root or Path("/nonexistent") target = (root / str(args.get("evidence_file") or "")).resolve() - if source_root is None or root.resolve() not in target.parents or not target.is_file(): + if source_root is None or root.resolve() not in target.parents: + return _err("evidence_file must be a file inside the agent's source") + if secret_text(target.relative_to(root.resolve()).as_posix()): + # Judged where it resolves, before the file is opened: a listed file, a link to one + # and one that is missing get this same answer, and a guess is never checked. + return _err( + f"{args.get('evidence_file')} is kept out of this session because it holds " + "credentials; name a file in the agent's code or configuration" + ) + if not target.is_file(): return _err("evidence_file must be a file inside the agent's source") folder = contract_folder(contract) scope = agent_scope(root, folder) diff --git a/src/fi/alk/harness/config.py b/src/fi/alk/harness/config.py index f1149c893..af076f4a2 100644 --- a/src/fi/alk/harness/config.py +++ b/src/fi/alk/harness/config.py @@ -293,8 +293,10 @@ def read_only_session( ) -def secret_refusal(tool: str, arguments: dict[str, Any]) -> str: - """Why this call would expose a secret or skip a proof, or empty when it would not.""" +def secret_refusal(tool: str, arguments: dict[str, Any], cwd: str | None = None) -> str: + """Why this call would expose a secret or skip a proof, or empty when it would not. ``cwd`` is + the session's own folder, from the hook's payload: a relative path resolves there, as the + tool itself resolves it, not in the harness's folder.""" if tool in ("Write", "Edit") and _PROVED_FILES.search(str(arguments.get("file_path") or "")): return ( "That file is part of the proved environment and only changes through the tool that " @@ -318,9 +320,10 @@ def secret_refusal(tool: str, arguments: dict[str, Any]) -> str: if arguments.get(key) ] pattern = str(arguments.get("pattern") or "") if tool == "Glob" else "" - if any(secret_path(one) for one in [*paths, pattern] if one): + base = Path(cwd) if cwd else None + if any(secret_path(one, base) for one in [*paths, pattern] if one): return SECRET_REFUSAL - if tool in ("Grep", "Glob") and any(above_secrets(one) for one in paths): + if tool in ("Grep", "Glob") and any(above_secrets(one, base) for one in paths): return SECRET_REFUSAL return "" @@ -360,7 +363,7 @@ async def refuse( name = str(payload.get("tool_name") or "") if name in permitted: arguments = payload.get("tool_input") or {} - refusal = secret_refusal(name, arguments) + refusal = secret_refusal(name, arguments, payload.get("cwd")) if refusal: return { "hookSpecificOutput": { diff --git a/src/fi/alk/harness/model_citation.py b/src/fi/alk/harness/model_citation.py index 755e5315e..bc821421a 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -20,6 +20,7 @@ import unicodedata from pathlib import Path +from .backends.files import secret_text from .contract import AgentContract from .runtime_selection import charged_path from .source_folder import is_within @@ -570,6 +571,73 @@ def _reads_as_a_setting(name: str) -> bool: return "." in name and not has_digit +_ENV_SETTING = re.compile(r"\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_.-]*)\s*[=:]") +# A setting that names a model, by its words. ENGINE alone also names databases (DB_ENGINE), so it +# counts only beside a provider's word (OPENAI_ENGINE, AZURE_OPENAI_ENGINE). +_MODEL_WORDS = frozenset({"MODEL", "DEPLOYMENT", "LLM"}) +_ENGINE_PROVIDERS = frozenset({"OPENAI", "AZURE"}) +# What marks a setting that holds a secret, anywhere in its name, joined words included +# (LLM_APIKEY). A URL can carry a password, and a model's name is never one. +_SECRET_PARTS = ("KEY", "TOKEN", "SECRET", "PASS", "PWD", "AUTH", "CREDENTIAL", "URL", "URI", "DSN") + + +def _names_a_model(line: str) -> bool: + """Whether a `.env` line sets a model: a setting with the word MODEL, DEPLOYMENT or LLM, or + ENGINE beside OPENAI or AZURE (OPENAI_MODEL, AZURE_OPENAI_DEPLOYMENT, LLM, OPENAI_ENGINE), and + nothing anywhere in its name that marks a secret (MODEL_API_KEY, LLM_APIKEY, DB_ENGINE_URL). + A line that sets nothing (no `=` or `:` after a name) doesn't.""" + found = _ENV_SETTING.match(line) + if found is None: + return False + setting = found.group(1).upper() + if any(part in setting for part in _SECRET_PARTS): + return False + words = set(re.split(r"[_.-]+", setting)) + return bool(words & _MODEL_WORDS) or ("ENGINE" in words and bool(words & _ENGINE_PROVIDERS)) + + +_ENV_DEFAULT = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*):?-([^{}$]*)\}") + + +def _env_value(line: str) -> str | None: + """The value a `.env` line gives its setting: quoted, or up to the first space, `;` or `,`. + None when the line sets nothing.""" + found = _ENV_SETTING.match(line) + if found is None: + return None + rest = line[found.end():].lstrip() + if rest[:1] in ("'", '"'): + end = rest.find(rest[0], 1) + if end < 0: + return None + value = rest[1:end] + else: + value = re.split(r"[\s;,]", rest, maxsplit=1)[0] + # Compose's default, ${MODEL:-gpt-4o}: the default is what the setting holds when unset. Only + # when the braced variable is the setting itself or another model setting + # (${DEFAULT_MODEL:-gpt-4o}): ${DB_PASSWORD:-guess} is a secret's. + default = _ENV_DEFAULT.fullmatch(value) + if default and ( + default.group(1) == found.group(1) or _names_a_model(f"{default.group(1)}=") + ): + return default.group(2) + return value + + +# A Vertex model name carries its version after an @ (claude-3-5-sonnet@20240620); any other @ in +# a value is a login (user@host, user:password@host). +_VERSIONED_NAME = re.compile(r"[^@:]+@(?:\d+|latest)", re.IGNORECASE) + + +def _env_value_is_a_locator(value: str) -> bool: + """A .env value that is a URL, or holds a login: MODEL_SERVER=postgres://admin:pw@db/app is + where a model is served from, with its credentials, not which model. `:` alone is a model + name's (anthropic.claude-3-5-sonnet-20240620-v1:0), and so is a version after an @.""" + if "://" in value or "\uff20" in value: + return True + return "@" in value and not _VERSIONED_NAME.fullmatch(value) + + def model_problems(contract: AgentContract, source_root: Path) -> list[str]: """Problems with contract.model, checked against the cited line. On success, strips name and provider and stores path as the resolved file relative to the root, in POSIX form. Resolves @@ -616,6 +684,17 @@ def model_problems(contract: AgentContract, source_root: Path) -> list[str]: resolved = resolved_in_source(root, path) if resolved is None: return [f"model: {shown_path} is not a file in the source"] + # Judged where it resolves, inside the source, before anything is opened, so a file that + # exists and one that doesn't get the same answer. A `.env` file is the one listed place a + # model may be cited from (it is where OPENAI_MODEL and the like are set), but only on a + # line whose setting names a model, and never with its length told. + relative = resolved.relative_to(root).as_posix() + if secret_text(relative, env_allowed=True): + return [ + f"model: {shown_path} is kept out of this session because it holds credentials; cite " + "the line in the agent's code or configuration the model is set on, or leave model out" + ] + env_file = secret_text(relative) if is_prose_file(resolved): # A README, a text file or a prompt template says which model is used; the code or its # configuration sets it. @@ -632,7 +711,21 @@ def model_problems(contract: AgentContract, source_root: Path) -> list[str]: ] lines = source_lines(raw) if model.line > len(lines): + if env_file: + return [f"model: line {model.line} is outside {shown_path}"] return [f"model: line {model.line} is outside {shown_path}, which has {len(lines)} lines"] + # In a .env file the name must be the value of the one setting that names the model, so a + # guess is never checked against another setting on the same line. + if env_file and ( + not _names_a_model(lines[model.line - 1]) + or _env_value(lines[model.line - 1]) != name + or _env_value_is_a_locator(name) + ): + return [ + f"model: in a .env file, cite the line whose setting names the model (such as " + f"OPENAI_MODEL), with the name as its value; {shown_path} line {model.line} sets " + "another setting or another value" + ] suffix = kind_suffix(resolved) dashes, ini = suffix in _DASH_COMMENT_SUFFIXES, suffix in _INI_SUFFIXES triple = suffix in _TRIPLE_SUFFIXES diff --git a/src/fi/alk/harness/runtime_repair.py b/src/fi/alk/harness/runtime_repair.py index d22b4eed4..0b043817f 100644 --- a/src/fi/alk/harness/runtime_repair.py +++ b/src/fi/alk/harness/runtime_repair.py @@ -18,7 +18,7 @@ from .backends import SessionSpec, tool, tool_server from .backends.base import ToolSpec -from .backends.files import file_tools +from .backends.files import file_tools, secret_text from .contract import ( AgentContract, Dependency, @@ -1043,6 +1043,10 @@ def validate_runtime_plan_patch( evidence.relative_to(root) except ValueError as exc: raise ValueError("runtime_repair_evidence_escapes_repository") from exc + if secret_text(evidence.relative_to(root).as_posix()): + # Judged where it resolves, before it is opened or checked for: a listed file, a link + # to one and one that is missing get this same answer, so no guess is ever checked. + raise ValueError(f"runtime_repair_evidence_holds_credentials: {relative}") if not evidence.exists(): raise ValueError(f"runtime_repair_evidence_missing: {relative}") if evidence.is_file(): diff --git a/src/fi/alk/harness/scan_ai.py b/src/fi/alk/harness/scan_ai.py index bdd22d9b2..38503d54a 100644 --- a/src/fi/alk/harness/scan_ai.py +++ b/src/fi/alk/harness/scan_ai.py @@ -85,17 +85,16 @@ _TOO_SLOW = "That call took too long and was stopped. Narrow the path." _FAILED = "That call failed. Try a different path or argument." -# Names the kit's secret filter doesn't cover. A source file named like `secret_manager.py` is -# hidden too; that's the price of never showing a settings file under an unusual name. +# The scan applies the shared list (`files.SECRET_GLOBS`) and hides these names as well: any name +# with "secret" or "credential" in it, such as `secret_manager.py` or a `secrets/` source folder, +# and an SSH key's `.pub`. Builds read some of those, so the shared list takes only their settings +# and key shapes; the scan only looks for agents, so it hides the rest too. _SECRET_NAMES = ( - "*.env", - "*.env.*", "*secret*", "*credential*", "id_rsa*", "id_ed25519*", "id_ecdsa*", - "*.tfvars", ) _OPENING = ( diff --git a/src/fi/alk/harness/source_data_invariants.py b/src/fi/alk/harness/source_data_invariants.py index f07f84d71..002abc8e3 100644 --- a/src/fi/alk/harness/source_data_invariants.py +++ b/src/fi/alk/harness/source_data_invariants.py @@ -18,6 +18,7 @@ from urllib.parse import urlsplit from .backends import SessionSpec, tool, tool_server +from .backends.files import secret_text from .config import chosen_model from .contract import AgentContract, is_data_free_conversation from .session import Stage @@ -43,6 +44,8 @@ def source_files(root: Path, folder: str = "") -> dict[str, Path]: for part in path.relative_to(root).parts ) and path.resolve().is_relative_to(root) + # A link named like code can lead to a credential file; judged where it resolves (TH-9042). + and not secret_text(path.resolve().relative_to(root).as_posix()) and ( scope is None or in_agent_scope(PurePosixPath(path.relative_to(root).as_posix()), scope) diff --git a/src/fi/alk/harness/sources.py b/src/fi/alk/harness/sources.py index 10c82c535..f7f5a3610 100644 --- a/src/fi/alk/harness/sources.py +++ b/src/fi/alk/harness/sources.py @@ -59,6 +59,7 @@ def servers(self) -> dict[str, Any]: return {} def briefing(self) -> str: + from .backends.files import secret_text from .source_folder import SubfolderNotFound, agent_folder ignored = { @@ -83,6 +84,9 @@ def briefing(self) -> str: continue if any(part in ignored for part in relative.parts) or not path.is_file(): continue + if secret_text(relative.as_posix()): + # A listed file is not named to the model, in the list or anywhere else. + continue indexed.append(relative.as_posix()) if len(indexed) >= 240: break diff --git a/src/fi/alk/harness/tools.py b/src/fi/alk/harness/tools.py index b467fe55a..00df7140c 100644 --- a/src/fi/alk/harness/tools.py +++ b/src/fi/alk/harness/tools.py @@ -32,6 +32,7 @@ ) from .backends import qualified as qualified # noqa: F401 (re-export; callers import it here) from .backends import tool, tool_server +from .backends.files import secret_text from .contract import ( prompt_quote, @@ -474,6 +475,14 @@ def _copy_prompt(contract: AgentContract, source_root: Path) -> list[str]: # target; each is charged to the save's budget. found_at[where.path] = resolved_in_source(root, where.path) path = found_at[where.path] + if path is not None and secret_text(path.relative_to(root).as_posix()): + # Judged where it resolves, inside the source, before anything is opened: a file + # that exists, one that doesn't and a link to either get this same answer. + problems.append( + f"system_prompt_from[{index}]: {where.path} is kept out of this session because " + "it holds credentials; name the file the prompt is written in" + ) + continue raw = None if path is not None and path not in lines_of and path not in too_large: raw = read_source(path, _LARGEST_PROMPT_FILE) diff --git a/tests/harness/test_builder_chat.py b/tests/harness/test_builder_chat.py index 703539a07..6895545bb 100644 --- a/tests/harness/test_builder_chat.py +++ b/tests/harness/test_builder_chat.py @@ -955,6 +955,11 @@ def test_file_tools_on_any_backend_never_return_credentials(tmp_path): (work / "scenarios" / "a.json").write_text("SENTINEL-free") (work / ".env").write_text("SENTINEL=1") (work / "service_account.json").write_text('{"SENTINEL": 1}') + # The names TH-9042 added, each holding the same marker. + added = (".pgpass", ".kube/config", ".docker/config.json", "infra/terraform.tfstate", "gcp-key.json", "id_ed25519") + for name in added: + (work / name).parent.mkdir(parents=True, exist_ok=True) + (work / name).write_text("SENTINEL=2") tools = {spec.name: _spec_tool(spec.name, spec) for spec in file_tools(str(work / "scenarios"))} def run(name, **args): @@ -968,6 +973,38 @@ def run(name, **args): assert ".env" not in run("Glob", pattern="*", path="..") assert "a.json" in run("Glob", pattern="*.json") assert "kept out of this session" in run("Grep", pattern="x", path="/") + assert "SENTINEL=2" not in run("Grep", pattern="SENTINEL", path="..") + for name in added: + assert "SENTINEL=2" not in run("Read", file_path=f"../{name}"), name + assert name.rsplit("/", 1)[-1] not in run("Glob", pattern="**/*", path=".."), name + + +def test_the_gate_judges_a_call_where_the_session_resolves_it(tmp_path): + """Through the hook itself, with the payload's cwd: a relative path resolves in the session's + folder, not the harness's (TH-9042).""" + from fi.alk.harness.config import gate_hooks + + session = tmp_path / "session" + (session / "k").mkdir(parents=True) + (session / ".pgpass").write_text("x") + (session / "notes.txt").symlink_to(".pgpass") + refuse = gate_hooks(["Read", "Glob", "Grep"])["PreToolUse"][0].hooks[0] + + def denied(tool, arguments, cwd=str(session)): + payload = {"tool_name": tool, "tool_input": arguments, "cwd": cwd} + answer = asyncio.run(refuse(payload, None, None)) + return answer.get("hookSpecificOutput", {}).get("permissionDecision") == "deny" + + assert denied("Grep", {"pattern": "x", "path": "notes.txt"}) + assert denied("Read", {"file_path": "notes.txt"}) + assert denied("Grep", {"pattern": "x", "path": "."}, cwd="/run") + assert denied("Glob", {"pattern": "**/.kube/config"}) + assert denied("Glob", {"pattern": "**/*.tfstate"}) + for name in (".pgpass", ".kube/config", ".docker/config.json", "x.tfstate", "gcp-key.json", "id_ecdsa_sk"): + assert denied("Read", {"file_path": str(session / name)}), name + assert not denied("Read", {"file_path": str(session / ".kube" / "configmap.yaml")}) + assert not denied("Grep", {"pattern": "x", "path": "k"}) + assert not denied("Grep", {"pattern": "x", "path": "notes.txt"}, cwd=str(tmp_path)) class RebuildClient(BuilderClient): @@ -2064,6 +2101,70 @@ def declared(path: str, name: str) -> dict: assert not own.get("is_error"), own +def _declare_from(tmp_path, evidence_file: str, quote: str): + root, source, contract, kept = _two_agent_world(tmp_path, "") + server = builder.suite_server(root, kept, contract, source) + declare = next(one for one in server.tools if one.name == "declare_tool") + return source, asyncio.run( + declare.handler( + { + "name": "cancel_a", + "description": "cancel", + "evidence_file": evidence_file, + "evidence_quote": quote, + "why": "the agent has it", + } + ) + ) + + +def test_declared_tool_evidence_never_checks_a_guess_against_a_listed_file(tmp_path): + answers = [] + for case, make in { + "listed": lambda src: (src / ".env").write_text("API_KEY=canary-value\n"), + "link": lambda src: ( + (src / ".env").write_text("API_KEY=canary-value\n"), + (src / "notes.txt").symlink_to(src / ".env"), + ), + "missing": lambda src: None, + }.items(): + world = tmp_path / case + world.mkdir() + root, source, contract, kept = _two_agent_world(world, "") + make(source) + server = builder.suite_server(root, kept, contract, source) + declare = next(one for one in server.tools if one.name == "declare_tool") + path = "notes.txt" if case == "link" else ".env" + for quote in ("API_KEY=canary-value", "API_KEY=c", "API_KEY=zzz"): + answers.append( + asyncio.run( + declare.handler( + { + "name": "cancel_a", + "description": "cancel", + "evidence_file": path, + "evidence_quote": quote, + "why": "x", + } + ) + ) + ) + + texts = {one["content"][0]["text"] for one in answers} + # A link is named differently from the file it points at, so its text differs only by name. + assert all(one["is_error"] for one in answers) + assert {t.replace("notes.txt", ".env") for t in texts} == { + ".env is kept out of this session because it holds credentials; " + "name a file in the agent's code or configuration" + } + + +def test_declared_tool_evidence_from_a_normal_file_still_works(tmp_path): + _, result = _declare_from(tmp_path, "agents/a/agent.py", "def cancel_a():") + + assert not result.get("is_error"), result + + # -- Change review r2, A-H1: what the chat sends the platform loses URL credentials ------------------ _SECRET_URL = "https://bot:hunter2@hooks.example/v1?key=sk-1#frag" diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index 3b3e775ce..91d1934ff 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -25,6 +25,11 @@ from fi.alk.harness.tools import accept_contract, contract_tools MIB = 1024 * 1024 +# How long a check this file holds to "quick" may take: it takes well under a tenth of a second +# alone, and the bound leaves room for a shared CI runner under load. A check under tracemalloc, +# which slows Python down several times over, gets its own. +QUICK_SECONDS = 1.0 +QUICK_SECONDS_TRACED = 5.0 FIXTURES = Path(__file__).resolve().parents[1] / "providers" / "fixtures" / "authored" @@ -90,6 +95,21 @@ def not_named(name: str, line: int, path: str) -> str: ) +def not_a_model_line(path: str, line: int) -> str: + return ( + f"model: in a .env file, cite the line whose setting names the model (such as " + f"OPENAI_MODEL), with the name as its value; {path} line {line} sets another setting " + "or another value" + ) + + +def kept_out(path: str) -> str: + return ( + f"model: {path} is kept out of this session because it holds credentials; cite the line " + "in the agent's code or configuration the model is set on, or leave model out" + ) + + def payload(**fields): return { "agent": "pair", @@ -305,7 +325,7 @@ def test_model_path_outside_the_source_is_refused(root, cited): problems, _, took = checked(root, {"name": "gpt-4o", "path": path, "line": 2}) assert problems == [not_a_file(path)] - assert took < 0.5 + assert took < QUICK_SECONDS # The save's one budget @@ -326,7 +346,7 @@ def test_model_path_spending_the_save_budget_is_refused(tmp_path, root): assert result.get("is_error") assert text_of(result) == REFUSED + str(PathLookupsSpent()) assert not (destination / "contract.json").exists() - assert took < 0.5 + assert took < QUICK_SECONDS @pytest.mark.parametrize("links", [pytest.param(11, id="lookup-budget-real")]) @@ -425,7 +445,7 @@ def test_model_file_that_cannot_be_read_is_refused(root, cited): problems, _, took = answer[0] assert problems == [unreadable(path)] - assert took < 0.5 + assert took < QUICK_SECONDS # The cited line must be in the file @@ -627,7 +647,7 @@ def test_a_crafted_model_file_does_not_hold_up_the_save(root, written, line, nam problems, _, took = checked(root, {"name": name, "path": "crafted.py", "line": line}) assert problems == ([not_named(name, line, "crafted.py")] if refused else []) - assert took < 0.5 + assert took < QUICK_SECONDS # Asking once for the prompt's lines @@ -1027,7 +1047,7 @@ def test_a_crafted_configuration_line_does_not_hold_up_the_check(root): problems, _, took = checked(root, {"name": "a", "path": "crafted.yaml", "line": 1}) assert problems == [not_named("a", 1, "crafted.yaml")] - assert took < 0.5 + assert took < QUICK_SECONDS # Names a setting goes by are not models, written in any case @@ -1627,7 +1647,7 @@ def test_a_quote_of_many_pieces_does_not_hold_up_the_check(root): took = time.perf_counter() - started assert answer is not None - assert took < 0.5 + assert took < QUICK_SECONDS # A prompt written with Python string prefixes holds its quote @@ -1755,7 +1775,7 @@ def test_a_crafted_file_of_triple_quotes_does_not_hold_up_the_check(root, writte _, _, took = checked(root, {"name": "gpt-4o", "path": "crafted.py", "line": line}) - assert took < 0.5 + assert took < QUICK_SECONDS def test_leaving_the_quote_out_is_not_asked_about(tmp_path, root): @@ -2118,7 +2138,7 @@ def test_a_value_outside_a_block_comment_is_the_model(root, file, written, line) "written", [ pytest.param( - "/*\n" * (MIB // 3 - 20) + 'const model = "gpt-4o";\n', + "/*\n" * (MIB // 6 - 20) + 'const model = "gpt-4o";\n', id="a-file-of-openers", ), pytest.param( @@ -2140,7 +2160,7 @@ def test_a_crafted_file_of_block_comments_does_not_hold_up_the_check(root, writt _, _, took = checked(root, {"name": "gpt-4o", "path": "crafted.js", "line": line}) - assert took < 0.5 + assert took < QUICK_SECONDS def test_lines_inside_a_block_comment_without_its_closer_are_not_scanned( @@ -2369,7 +2389,7 @@ def test_a_quote_that_grows_when_normalised_costs_little(root): finally: tracemalloc.stop() - assert took < 0.5 + assert took < QUICK_SECONDS_TRACED assert peak < 32 * MIB @@ -2392,7 +2412,8 @@ def test_an_apostrophe_does_not_hide_a_comment(root, file, written, name): problems, _, _ = checked(root, {"name": name, "path": file, "line": 2}) - assert problems == [not_named(name, 2, file)] + # In a .env file a line whose setting isn't a model's is refused before it is read (TH-9042). + assert problems == [not_a_model_line(file, 2) if file == ".env" else not_named(name, 2, file)] @pytest.mark.parametrize( @@ -2444,7 +2465,7 @@ def test_a_crafted_line_of_generic_calls_does_not_hold_up_the_check(root): _, _, took = checked(root, {"name": "a", "path": "crafted.ts", "line": 1}) - assert took < 0.5 + assert took < QUICK_SECONDS def test_a_crafted_line_of_apostrophes_does_not_hold_up_the_check(root): @@ -2455,7 +2476,7 @@ def test_a_crafted_line_of_apostrophes_does_not_hold_up_the_check(root): problems, _, took = checked(root, {"name": "a", "path": "crafted.py", "line": 1}) assert problems == [not_named("a", 1, "crafted.py")] - assert took < 0.5 + assert took < QUICK_SECONDS # Lines sent alone after a refusal are asked about once more before either side is given up @@ -2529,7 +2550,7 @@ def test_a_quote_whose_start_matches_everywhere_does_not_hold_up_the_check(root) took = time.perf_counter() - started assert answer is not None - assert took < 0.5 + assert took < QUICK_SECONDS def test_a_first_word_found_early_is_tried_again(tmp_path, root): @@ -2674,7 +2695,7 @@ def test_a_crafted_long_word_costs_little(root): took = time.perf_counter() - started assert answer is not None - assert took < 0.25 + assert took < QUICK_SECONDS # Scripts written without spaces: each character is a word @@ -2806,7 +2827,7 @@ def test_a_crafted_file_of_one_byte_lines_and_triple_quotes_costs_little(root): _, _, took = checked(root, {"name": "gpt-4o", "path": "crafted.py", "line": line}) - assert took < 0.25 + assert took < QUICK_SECONDS @pytest.mark.parametrize( @@ -2948,7 +2969,7 @@ def test_a_long_first_word_is_charged_on_every_try(root): took = time.perf_counter() - started assert answer is not None - assert took < 0.25 + assert took < QUICK_SECONDS def test_a_short_unspaced_quote_is_still_searched_with_gaps(tmp_path, root): @@ -3181,7 +3202,7 @@ def test_an_unclosed_string_of_escapes_costs_one_pass(root, quote): _, _, took = checked(root, {"name": "gpt-4o", "path": "crafted.ts", "line": 1}) - assert took < 0.25 + assert took < QUICK_SECONDS @pytest.mark.parametrize( @@ -3449,13 +3470,14 @@ def test_a_pipe_is_logged_as_not_a_regular_file(tmp_path, caplog): pytest.param("init.lua", 'local n = a // 2; model = "gpt-4o"', "gpt-4o", id="lua-floor-division"), pytest.param("Makefile", "\techo a#b; MODEL=gpt-4o", "gpt-4o", id="makefile-hash-in-a-word"), pytest.param(".flaskenv", "URL=http://x/#a MODEL=gpt-4o", "gpt-4o", id="flaskenv-hash-in-a-word"), - pytest.param(".env.local", "URL=http://x/#a MODEL=gpt-4o", "gpt-4o", id="env-name-hash-in-a-word"), pytest.param("Dockerfile.dev", "ENV A=a#b MODEL=gpt-4o", "gpt-4o", id="dockerfile-name-hash-in-a-word"), pytest.param("run.bash", "echo a#b; MODEL=gpt-4o", "gpt-4o", id="bash-hash-in-a-word"), pytest.param("run.zsh", "echo a#b; MODEL=gpt-4o", "gpt-4o", id="zsh-hash-in-a-word"), pytest.param("rules.mk", "\techo a#b; MODEL=gpt-4o", "gpt-4o", id="mk-hash-in-a-word"), pytest.param("app.dockerfile", "ENV A=a#b MODEL=gpt-4o", "gpt-4o", id="dockerfile-suffix-hash-in-a-word"), - pytest.param("prod.env", "URL=http://x/#a MODEL=gpt-4o", "gpt-4o", id="env-suffix-hash-in-a-word"), + # prod.env is a .env file, so the name must be its line's own model setting's value (since + # TH-9042's second round). + pytest.param("prod.env", "MODEL=gpt-4o # see a#b", "gpt-4o", id="env-suffix-hash-in-a-word"), pytest.param("agent.mts", 'class A { #model = "gpt-4o"; }', "gpt-4o", id="mts-private-field"), pytest.param("agent.cts", 'this.#model = "gpt-4o";', "gpt-4o", id="cts-private-field"), pytest.param("agent.sc", 'val t: A#B = x; val model = "gpt-4o"', "gpt-4o", id="scala-script-type-projection"), @@ -3485,7 +3507,6 @@ def test_more_honest_lines_after_the_settled_round(root, file, written, name): pytest.param("agent.json", '"model": "gpt-4o", // "gpt-4-turbo" is slower', "gpt-4-turbo", id="json-comment-after-a-value"), pytest.param("agent.json", '* "model": "gpt-4",', "gpt-4", id="json-block-comment-middle-line"), pytest.param("dockerfile_gen.py", 'MODEL = DEFAULT#"gpt-4"', "gpt-4", id="python-named-like-a-dockerfile"), - pytest.param(".env_loader.py", 'MODEL = DEFAULT#"gpt-4"', "gpt-4", id="python-named-like-an-env-file"), pytest.param("cfg.yaml", "note: the cart's total # model: 'gpt-4'", "gpt-4", id="a-word-ending-like-a-prefix"), pytest.param("cfg.yaml", "note: it_t's ok # model: 'gpt-4'", "gpt-4", id="an-underscore-before-a-prefix"), pytest.param("env.rb", "MODEL = gpt-4o", "gpt-4o", id="env-rb-is-code"), @@ -3791,3 +3812,212 @@ def test_words_that_say_nothing_do_not_replace_the_held_quote(tmp_path, root, no ) assert text_of(result) == REFUSED + lines_alone("agent.py 1-4", _BOOKING_QUOTE) + + +# TH-9042: a model citation never reads a file on the secret list, apart from a .env file's +# line whose setting names a model; and it never tells such a file's length or whether it exists. + + +@pytest.mark.parametrize( + "file, written", + [ + pytest.param(".pgpass", "db:5432:app:admin:changeme", id="pgpass"), + pytest.param("infra/prod.tfvars", 'db_password = "changeme"', id="tfvars"), + pytest.param("kubeconfig.yaml", "token: changeme", id="kubeconfig"), + pytest.param("gcp-key.json", '{"private_key": "changeme"}', id="key-json"), + ], +) +def test_a_model_citation_never_reads_a_listed_file(root, file, written): + target = root / file + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(f"{written}\n", encoding="utf-8") + + present, contract, _ = checked(root, {"name": "changeme", "path": file, "line": 1}) + past_the_end, _, _ = checked(root, {"name": "changeme", "path": file, "line": 9}) + target.unlink() + missing, _, _ = checked(root, {"name": "changeme", "path": file, "line": 1}) + + assert present == past_the_end == missing == [kept_out(file)] + assert contract.model is not None and contract.model.name == "changeme" + + +def test_a_model_citation_through_a_link_to_a_listed_file_is_refused(root): + (root / ".pgpass").write_text("db:5432:app:admin:changeme\n", encoding="utf-8") + (root / "settings.txt").symlink_to(".pgpass") + (root / "gone.yaml").symlink_to("kubeconfig.yaml") + + linked, _, _ = checked(root, {"name": "changeme", "path": "settings.txt", "line": 1}) + dangling, _, _ = checked(root, {"name": "changeme", "path": "gone.yaml", "line": 1}) + + assert linked == [kept_out("settings.txt")] + assert dangling == [kept_out("gone.yaml")] + + +def test_a_env_file_cites_a_model_only_on_a_line_that_sets_one(root): + (root / ".env").write_text( + "DB_PASSWORD=changeme\nOPENAI_MODEL=gpt-4o\nexport LLM_MODEL=gpt-4o-mini\n", encoding="utf-8" + ) + + password, _, _ = checked(root, {"name": "changeme", "path": ".env", "line": 1}) + model, contract, _ = checked(root, {"name": "gpt-4o", "path": ".env", "line": 2}) + exported, _, _ = checked(root, {"name": "gpt-4o-mini", "path": ".env", "line": 3}) + past_the_end, _, _ = checked(root, {"name": "gpt-4o", "path": ".env", "line": 40}) + + assert password == [not_a_model_line(".env", 1)] + assert model == [] and contract.model is not None and contract.model.path == ".env" + assert exported == [] + assert past_the_end == ["model: line 40 is outside .env"] + + +def test_a_env_line_whose_setting_is_not_a_model_is_refused_even_with_a_model_on_it(root): + (root / ".env.local").write_text("\nURL=http://x/#a MODEL=gpt-4o\n", encoding="utf-8") + + problems, _, _ = checked(root, {"name": "gpt-4o", "path": ".env.local", "line": 2}) + + assert problems == [not_a_model_line(".env.local", 2)] + + +def test_a_source_under_a_listed_folder_still_cites_its_model(tmp_path, monkeypatch): + source = tmp_path / ".claude" / "worktrees" / "repo" + source.mkdir(parents=True) + (source / "agent.py").write_text('MODEL = "gpt-4o"\n', encoding="utf-8") + monkeypatch.chdir(tmp_path / ".claude") + + relative, _, _ = checked(source, {"name": "gpt-4o", "path": "agent.py", "line": 1}) + absolute, _, _ = checked(source, {"name": "gpt-4o", "path": str(source / "agent.py"), "line": 1}) + + assert relative == absolute == [] + + +@pytest.mark.parametrize( + "written, name", + [ + pytest.param("AZURE_OPENAI_DEPLOYMENT=gpt4o-prod", "gpt4o-prod", id="azure-deployment"), + pytest.param("AZURE_OPENAI_DEPLOYMENT_NAME=gpt4o-prod", "gpt4o-prod", id="azure-deployment-name"), + pytest.param("OPENAI_ENGINE=gpt-4o", "gpt-4o", id="engine"), + pytest.param("AZURE_OPENAI_ENGINE=gpt4o-prod", "gpt4o-prod", id="azure-engine"), + pytest.param("LLM=gpt-4o", "gpt-4o", id="llm"), + pytest.param("DEFAULT_LLM=gpt-4o", "gpt-4o", id="default-llm"), + pytest.param("export LLM_MODEL=gpt-4o", "gpt-4o", id="exported"), + pytest.param('OPENAI_MODEL="gpt-4o"', "gpt-4o", id="double-quoted"), + pytest.param("export OPENAI_MODEL='gpt-4o' # pinned", "gpt-4o", id="single-quoted-exported"), + pytest.param("OPENAI_MODEL=gpt-4o # the default", "gpt-4o", id="comment-after"), + pytest.param('OPENAI_MODEL="${OPENAI_MODEL:-gpt-4o}"', "gpt-4o", id="quoted-compose-default"), + pytest.param( + "CHAT_MODEL=${DEFAULT_MODEL:-gpt-4o-mini}", "gpt-4o-mini", id="another-model-setting-default" + ), + ], +) +def test_a_env_setting_named_for_a_model_by_any_of_its_words_is_accepted(root, written, name): + (root / ".env").write_text(f"{written}\n", encoding="utf-8") + + problems, _, _ = checked(root, {"name": name, "path": ".env", "line": 1}) + + assert problems == [] + + +@pytest.mark.parametrize( + "written, name", + [ + pytest.param("VERTEX_MODEL=claude-3-5-sonnet@20240620", "claude-3-5-sonnet@20240620", id="vertex-version"), + pytest.param("VERTEX_MODEL=claude-3-5-sonnet-v2@latest", "claude-3-5-sonnet-v2@latest", id="vertex-latest"), + pytest.param("VERTEX_MODEL=mistral-large@2407", "mistral-large@2407", id="vertex-short-version"), + pytest.param("VERTEX_MODEL=text-bison@001", "text-bison@001", id="vertex-three-digit-version"), + pytest.param("BEDROCK_MODEL=anthropic.claude-3-5-sonnet-20240620-v1:0", "anthropic.claude-3-5-sonnet-20240620-v1:0", id="bedrock-colon"), + ], +) +def test_a_env_model_name_with_an_at_or_a_colon_of_its_own_is_still_accepted(root, written, name): + (root / ".env").write_text(f"{written}\n", encoding="utf-8") + + problems, _, _ = checked(root, {"name": name, "path": ".env", "line": 1}) + + assert problems == [] + + +def test_a_fullwidth_at_in_a_env_value_is_refused(): + from fi.alk.harness.model_citation import _env_value_is_a_locator + + assert _env_value_is_a_locator("admin\uff20llm.internal") + assert not _env_value_is_a_locator("claude-3-5-sonnet@20240620") + + +@pytest.mark.parametrize("file", ["prod.env", "deploy/app.env", ".env.production"]) +def test_every_kind_of_env_file_takes_the_same_rule(root, file): + (root / file).parent.mkdir(parents=True, exist_ok=True) + (root / file).write_text("DB_PASSWORD=fake-guess\nOPENAI_MODEL=gpt-4o\n", encoding="utf-8") + + accepted, _, _ = checked(root, {"name": "gpt-4o", "path": file, "line": 2}) + refused, _, _ = checked(root, {"name": "fake-guess", "path": file, "line": 1}) + + assert accepted == [] + assert refused == [not_a_model_line(file, 1)] + + +@pytest.mark.parametrize( + "file, written, name", + [ + pytest.param(".env", "MODEL_API_KEY=sk-fake-guess", "sk-fake-guess", id="model-key"), + pytest.param(".env", "LLM_TOKEN=fake-guess", "fake-guess", id="llm-token"), + pytest.param(".env", "OPENAI_MODEL_PASSWORD=fake-guess", "fake-guess", id="model-password"), + pytest.param(".env", "LLM_APIKEY=sk-fake-guess", "sk-fake-guess", id="joined-apikey"), + pytest.param(".env", "MODEL_ACCESSKEY=fake-guess", "fake-guess", id="joined-accesskey"), + pytest.param(".env", "OPENAI_MODEL_APIKEY=sk-fake-guess", "sk-fake-guess", id="model-joined-apikey"), + pytest.param(".env", "LLM_PASSWD=fake-guess", "fake-guess", id="joined-passwd"), + pytest.param(".env", "LLM_KEY=fake-guess", "fake-guess", id="part-key"), + pytest.param(".env", "MODEL_TOKEN=fake-guess", "fake-guess", id="part-token"), + pytest.param(".env", "MODEL_SECRET=fake-guess", "fake-guess", id="part-secret"), + pytest.param(".env", "LLM_PASS=fake-guess", "fake-guess", id="part-pass"), + pytest.param(".env", "LLM_PWD=fake-guess", "fake-guess", id="part-pwd"), + pytest.param(".env", "LLM_AUTH=fake-guess", "fake-guess", id="part-auth"), + pytest.param(".env", "MODEL_CREDENTIAL=fake-guess", "fake-guess", id="part-credential"), + pytest.param(".env", "LLM_URL=fake-guess", "fake-guess", id="part-url"), + pytest.param(".env", "MODEL_URI=fake-guess", "fake-guess", id="part-uri"), + pytest.param(".env", "LLM_DSN=fake-guess", "fake-guess", id="part-dsn"), + pytest.param(".env", "MODEL_URL=https://admin:fake@llm.local/v1", "https://admin:fake@llm.local/v1", id="model-url"), + pytest.param(".env", "MODEL_SERVER=postgres://admin:hunter2@db/app", "postgres://admin:hunter2@db/app", id="model-server-url-with-login"), + pytest.param(".env", "LLM_ENDPOINT=https://admin:pw@llm.internal/v1", "https://admin:pw@llm.internal/v1", id="llm-endpoint-url-with-login"), + pytest.param(".env", "MODEL_HOST=https://llm.internal/v1", "https://llm.internal/v1", id="url-without-login"), + pytest.param(".env", "MODEL_SERVER=admin:hunter2@db", "admin:hunter2@db", id="bare-login"), + pytest.param(".env", "LLM_ENDPOINT=gpt-4o@llm.internal", "gpt-4o@llm.internal", id="user-at-host"), + pytest.param(".env", "MODEL_ID=gpt-4o@evil.example", "gpt-4o@evil.example", id="model-then-host"), + pytest.param( + ".env", + "DB_ENGINE_URL=postgresql://admin:fake@db/app", + "postgresql://admin:fake@db/app", + id="database-engine-url", + ), + pytest.param(".env", "DB_ENGINE=postgresql", "postgresql", id="database-engine"), + pytest.param("prod.env", "URL=http://x/#a MODEL=gpt-4o", "gpt-4o", id="env-suffix-url-first"), + pytest.param(".env", "OPENAI_MODEL=gpt-4o OPENAI_API_KEY=sk-fakeguess1", "sk-fakeguess1", id="second-setting"), + pytest.param(".env", "OPENAI_MODEL=gpt-4o;OPENAI_API_KEY=sk-fakeguess1", "sk-fakeguess1", id="second-setting-semicolon"), + pytest.param(".env", 'OPENAI_MODEL="gpt-4o" DB_PASSWORD="fakeguess1"', "fakeguess1", id="second-setting-quoted"), + pytest.param(".envrc", "export LLM_MODEL=gpt-4o; export DB_PASSWORD=fakeguess1", "fakeguess1", id="envrc-second-export"), + pytest.param("prod.env", "MODEL_NOTE=a#b OPENAI_API_KEY=sk-fakeguess1", "sk-fakeguess1", id="hash-then-key"), + pytest.param(".env", 'LLM_CONFIG={"model":"gpt-4o","api_key":"sk-fakeguess1"}', "sk-fakeguess1", id="json-value"), + pytest.param(".env.yaml", "model: {name: gpt-4o, api_key: sk-fakeguess1}", "sk-fakeguess1", id="flow-mapping"), + pytest.param(".env", "MODEL_NOTE=a#b MODEL=gpt-4o", "gpt-4o", id="model-as-a-second-setting"), + # Named like a .env file, so it takes the .env rule (since TH-9042's second round). + pytest.param(".env_loader.py", 'MODEL = DEFAULT#"gpt-4"', "gpt-4", id="python-named-like-an-env-file"), + pytest.param(".env.yaml", "DB_PASSWORD: MODELfake123", "MODELfake123", id="colon-setting"), + pytest.param(".env", "MODELfake123", "MODELfake123", id="no-setting"), + pytest.param(".env", "OPENAI_MODEL=${DB_PASSWORD:-fakeguess1}", "fakeguess1", id="other-setting-compose-default"), + pytest.param(".env", 'OPENAI_MODEL="${OPENAI_API_KEY:-sk-fakeguess1}"', "sk-fakeguess1", id="quoted-other-setting-compose-default"), + # A braced name with a model word but a secret part is still a secret's default. + pytest.param(".env", "OPENAI_MODEL=${MODEL_API_KEY:-sk-fakeguess1}", "sk-fakeguess1", id="model-secret-compose-default"), + pytest.param(".env", "CHAT_MODEL=${llm_token:-fakeguess1}", "fakeguess1", id="lowercase-model-secret-compose-default"), + ], +) +def test_a_env_line_that_holds_a_secret_or_sets_nothing_is_refused(root, file, written, name): + (root / file).write_text(f"{written}\n", encoding="utf-8") + + problems, _, _ = checked(root, {"name": name, "path": file, "line": 1}) + + assert problems == [not_a_model_line(file, 1)] + + +def test_a_backup_copy_of_a_credential_file_is_refused(root): + (root / ".pgpass.bak").write_text("db:5432:app:admin:changeme\n", encoding="utf-8") + + problems, _, _ = checked(root, {"name": "changeme", "path": ".pgpass.bak", "line": 1}) + + assert problems == [kept_out(".pgpass.bak")] diff --git a/tests/harness/test_contract_tools_subfolder.py b/tests/harness/test_contract_tools_subfolder.py index 60e1df84a..22d09f978 100644 --- a/tests/harness/test_contract_tools_subfolder.py +++ b/tests/harness/test_contract_tools_subfolder.py @@ -2133,3 +2133,67 @@ def test_a_path_past_twice_the_system_limit_is_refused_before_any_lookup(monkeyp with pytest.raises(ValueError): runtime_selection.charged_path(Path("/" + "../" * 3000 + "x")) assert looked == [] + + +# TH-9042: the prompt copy never opens a file on the secret list, and never says whether one exists. + +_KEPT_OUT = "is kept out of this session because it holds credentials" + + +def _copy(tmp_path: Path, root: Path, path: str, start: int = 1, end: int = 1) -> dict: + return accept_contract( + payload( + runtime={"language": "python"}, + system_prompt_from=[{"path": path, "start_line": start, "end_line": end}], + ), + tmp_path / "out", + source_root=root, + ) + + +@pytest.mark.parametrize( + "name", [".pgpass", ".env", "prod.env", "config/secrets.yaml", "secrets.properties"] +) +def test_a_prompt_named_in_a_credential_file_is_refused_and_never_copied(tmp_path, repo, name): + # A .env file too: only a model citation may name one, never the prompt copy. + (repo / name).parent.mkdir(parents=True, exist_ok=True) + (repo / name).write_text("db:5432:app:admin:fake-pass-123\n") + + for start, end in ((1, 1), (1, 40)): + result = _copy(tmp_path, repo, name, start, end) + text = result["content"][0]["text"] + assert result.get("is_error") and _KEPT_OUT in text, result + assert "fake-pass-123" not in text and "which has" not in text + assert not (tmp_path / "out" / "contract.json").exists() + + +@pytest.mark.parametrize("how", ["link", "missing", "missing-absolute", "dangling-link"]) +def test_a_credential_file_gets_one_answer_however_it_is_reached(tmp_path, repo, how): + if how == "link": + (repo / ".pgpass").write_text("db:5432:app:admin:fake-pass-123\n") + (repo / "prompt.md").symlink_to(".pgpass") + path = "prompt.md" + elif how == "dangling-link": + (repo / "prompt.md").symlink_to(".pgpass") + path = "prompt.md" + elif how == "missing-absolute": + path = str(repo / ".pgpass") + else: + path = ".pgpass" + + result = _copy(tmp_path, repo, path) + + assert result.get("is_error") and _KEPT_OUT in result["content"][0]["text"], result + + +def test_a_source_under_a_listed_folder_still_copies_its_prompt(tmp_path, monkeypatch): + source = tmp_path / ".claude" / "worktrees" / "repo" + (source / "prompts").mkdir(parents=True) + (source / "agent.py").write_text("x = 1\n") + (source / "prompts" / "system.md").write_text("Be brief.\n") + monkeypatch.chdir(tmp_path / ".claude") + + for path in ("prompts/system.md", str(source / "prompts" / "system.md")): + result = _copy(tmp_path, source, path) + assert not result.get("is_error"), (path, result) + assert _saved(tmp_path / "out")["system_prompt_excerpt"] == "Be brief." diff --git a/tests/harness/test_runtime_repair.py b/tests/harness/test_runtime_repair.py index 313ebf3da..d44d6e858 100644 --- a/tests/harness/test_runtime_repair.py +++ b/tests/harness/test_runtime_repair.py @@ -140,6 +140,39 @@ def test_runtime_patch_rejects_missing_or_outside_evidence(tmp_path) -> None: ) +@pytest.mark.parametrize("form", ["present", "link", "missing"]) +@pytest.mark.parametrize("name", [".env", "service/.env.production", ".aws/credentials"]) +def test_runtime_patch_never_checks_a_command_against_a_listed_file(tmp_path, form, name) -> None: + # A listed file, a link to one and a missing one get one answer before anything is read, so + # the command check can't be used to grow a guess at a key letter by letter. + source = tmp_path / "source" + (source / "service").mkdir(parents=True) + cited = name + if form != "missing": + target = source / name + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("OPENAI_API_KEY=canaryfakeguess1\n", encoding="utf-8") + if form == "link": + cited = "service/settings_local.py" + (source / cited).symlink_to(source / name) + contract = AgentContract(agent="agent", runtime=Runtime(language="python")) + patch = RuntimePlanPatch( + base_contract_hash=_contract_hash(contract), + runtime=Runtime(language="python", workdir="service", start="canaryfakeguess1 run"), + evidence_paths=(cited,), + diagnostic_codes=("generated_runtime_plan_invalid",), + summary="Use service.", + ) + + with pytest.raises(ValueError, match="runtime_repair_evidence_holds_credentials"): + validate_runtime_plan_patch( + source, + contract, + patch, + allowed_diagnostic_codes={"generated_runtime_plan_invalid"}, + ) + + def test_runtime_patch_rejects_stale_contract(tmp_path) -> None: source = tmp_path / "source" service = source / "service" diff --git a/tests/harness/test_scan_ai.py b/tests/harness/test_scan_ai.py index e53825f6a..fc1e0b1bf 100644 --- a/tests/harness/test_scan_ai.py +++ b/tests/harness/test_scan_ai.py @@ -613,6 +613,12 @@ def test_the_ai_cannot_read_outside_the_source(tmp_path): "id_rsa", "terraform.tfvars", "secrets/keys.py", + # Names only the shared list covers, not the scan's own (TH-9042). + ".pgpass", + ".kube/config", + "infra/x.tfstate", + "gcp-key.json", + ".docker/config.json", ) diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py new file mode 100644 index 000000000..36824522c --- /dev/null +++ b/tests/harness/test_secret_paths.py @@ -0,0 +1,487 @@ +"""TH-9042: one list of credential files, kept from the model on every reader. + +The list is `files.SECRET_GLOBS`. The kit's own tools, the scan's tools and the gate match paths +against the regex built from it; Claude Code gets it as Read deny rules, which its own Grep and +Glob turn into exclusions. The last test runs the real bundled CLI against a local stand-in for +the Messages API (no model, no key) and checks that it hides exactly what the kit hides. +""" + +from __future__ import annotations + +import asyncio +import json +import re +import threading +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from typing import Any + +import pytest + +from fi.alk.harness.backends import files +from fi.alk.harness.backends.base import SessionSpec, WorkerSpec +from fi.alk.harness.backends.files import ( + ALL_SECRET_GLOBS, + SECRET_GLOBS, + file_tools, + secret_path, + secret_read_rules, + secret_text, +) + +# Written out by hand, never built from the module's list: removing a glob there must fail here. +# Each sample is matched by its own glob and no other. +TABLE: tuple[tuple[str, str], ...] = ( + ("**/.env*", ".envrc"), + ("**/*.env", "deploy/prod.env"), + ("**/*.env.*", "deploy/prod.env.example"), + ("**/.claude", ".claude/settings.json"), + ("**/.ssh", ".ssh/config"), + ("**/.aws", ".aws/credentials"), + ("**/.config/gcloud", ".config/gcloud/adc.json"), + ("**/.authoring-credentials", ".authoring-credentials/g.json"), + ("**/.netrc", ".netrc"), + ("**/.npmrc", "web/.npmrc"), + ("**/.pypirc", ".pypirc"), + ("**/*credential*.json", "my-credentials.json"), + ("**/*secret*.json", "client_secret.json"), + ("**/*token*.json", "token.json"), + ("**/*service-account*.json", "gcp-service-account.json"), + ("**/*service_account*.json", "service_account.json"), + ("**/*serviceaccount*.json", "serviceaccount.json"), + ("**/*secret*.yaml", "config/secrets.yaml"), + ("**/*secret*.yml", "k8s/db-secret.yml"), + ("**/*secret*.toml", ".streamlit/secrets.toml"), + ("**/*credential*.yaml", "credentials.yaml"), + ("**/*credential*.yml", "config/credentials.yml"), + ("**/*credential*.toml", "app-credentials.toml"), + ("**/*secret*.ini", "config/secrets.ini"), + ("**/*secret*.cfg", "secrets.cfg"), + ("**/*secret*.conf", "etc/app-secrets.conf"), + ("**/*secret*.properties", "secrets.properties"), + ("**/*secret*.xml", "secrets.xml"), + ("**/*credential*.ini", "credentials.ini"), + ("**/*credential*.cfg", "conf/credentials.cfg"), + ("**/*credential*.conf", "credentials.conf"), + ("**/*credential*.properties", "src/main/resources/credentials.properties"), + ("**/*credential*.xml", "jenkins/credentials.xml"), + ("**/*firebase-adminsdk*.json", "app/myproj-firebase-adminsdk-ab12c-0123456789.json"), + ("**/*credential*.csv", "aws/my_credentials.csv"), + ("**/*accesskeys*.csv", "aws/accessKeys.csv"), + ("**/rootkey*.csv", "Downloads/rootkey (1).csv"), + ("**/*secret*.txt", "api-secret.txt"), + ("**/*credential*.txt", "gcp-credential.txt"), + ("**/secrets/*.txt", "deploy/secrets/db_password.txt"), + ("**/.secrets", ".secrets"), + ("**/.dev.vars", "worker/.dev.vars"), + ("**/.dev.vars.*", "worker/.dev.vars.staging"), + ("**/local.settings.json", "functions/local.settings.json"), + ("**/key.properties", "android/key.properties"), + ("**/keystore.properties", "android/keystore.properties"), + ("**/*.p8", "keys/SubscriptionKey_ABC123.p8"), + ("**/*.pk8", "build/target/product/security/platform.pk8"), + ("**/*.ppk", "deploy/server.ppk"), + ("**/.config/gh/hosts.yml", ".config/gh/hosts.yml"), + ("**/.databrickscfg", ".databrickscfg"), + ("**/.terraformrc", ".terraformrc"), + ("**/.cargo/credentials", ".cargo/credentials"), + ("**/.gem/credentials", ".gem/credentials"), + ("**/*.pem", "certs/server.pem"), + ("**/*.key", "certs/server.key"), + ("**/*.p12", "certs/a.p12"), + ("**/*.pfx", "certs/a.pfx"), + ("**/*.jks", "android/release.jks"), + ("**/*.keystore", "android/debug.keystore"), + # The ticket's six: .kube/config, .docker/config.json, .pgpass, .htpasswd, terraform.tfstate + # and *-key.json. + ("**/.kube/config", ".kube/config"), + ("**/.kube/config.*", ".kube/config.prod"), + ("**/kubeconfig", "deploy/kubeconfig"), + ("**/kubeconfig.yaml", "kubeconfig.yaml"), + ("**/kubeconfig.yml", "kubeconfig.yml"), + ("**/*.kubeconfig", "admin.kubeconfig"), + ("**/.docker/config.json", ".docker/config.json"), + ("**/.docker/contexts", ".docker/contexts/meta/m.json"), + ("**/.dockercfg", ".dockercfg"), + ("**/.pgpass", ".pgpass"), + ("**/.htpasswd", "web/.htpasswd"), + ("**/.git-credentials", ".git-credentials"), + ("**/.my.cnf", ".my.cnf"), + ("**/.s3cfg", ".s3cfg"), + ("**/.boto", ".boto"), + ("**/.vault-token", ".vault-token"), + ("**/id_rsa", "deploy/id_rsa"), + ("**/id_dsa", "id_dsa"), + ("**/id_ecdsa", "id_ecdsa"), + ("**/id_ed25519", "keys/id_ed25519"), + ("**/id_ecdsa_sk", "id_ecdsa_sk"), + ("**/id_ed25519_sk", "id_ed25519_sk"), + ("**/*.tfstate", "infra/terraform.tfstate"), + ("**/*.tfstate.backup", "infra/terraform.tfstate.backup"), + ("**/*.tfstate.*.backup", "infra/terraform.tfstate.1700000000.backup"), + ("**/*.tfvars", "infra/prod.tfvars"), + ("**/*.tfvars.json", "infra/prod.tfvars.json"), + ("**/*-key.json", "gcp-key.json"), + ("**/*_key.json", "api_key.json"), + ("**/*-sa.json", "deploy/gcp-sa.json"), +) +# Exact names whose .bak copy is listed too. +BACKED_UP = ( + "deploy/kubeconfig", "kubeconfig.yaml", "kubeconfig.yml", "admin.kubeconfig", + ".docker/config.json", ".dockercfg", ".pgpass", "web/.htpasswd", ".git-credentials", + ".my.cnf", ".s3cfg", ".boto", ".vault-token", "deploy/id_rsa", "id_dsa", "id_ecdsa", + "keys/id_ed25519", "id_ecdsa_sk", "id_ed25519_sk", +) # fmt: skip +NEAR_MISSES = ( + ".dockerignore", ".kubeignore", ".docker/Dockerfile", ".docker/compose.yml", + ".docker/contexts-notes.md", ".docker/config.json.example", "x.docker/config.json", + ".kube/configmap.yaml", ".kube/configs/a.yaml", ".kube/cache/x", "x.kube/config", + "kubeconfig.go", "xkubeconfig", "my-kubeconfig-docs.md", "x.kubeconfig.md", + ".dockercfg.example", "x.dockercfg", "valid_rsa", "id_rsa.pub", "my.pgpass", + ".htpasswd.example", "my.boto", ".boto.cfg", ".vault-token.md", + "infra/terraform.tfstate.lock.info", "infra/terraform.tfstate.backup.old", + "infra/terraform.tfvars.example", "debug.keystore.md", "key.json", "gcp-key.json.example", + ".azure/pipelines.yml", "secrets/app.json", "tokens/list.json", "credentials/README.json", + "infra/terraform.tfstate.d/default.backup", "agent.py", + "src/env.mjs", "next-env.d.ts", "vite-env.d.ts", "env.example", "dotenv.py", + "secret_manager.py", "credentials_helper.go", "secrets.yaml.example", "visa.json", "sa.json", + "secrets.py", "secrets.sh", "firebase.json", "id_ed25519.pub", "secrets.ini.example", + "secrets/manager.go", "credentials/README.txt", ".cargo/config.toml", "gh/hosts.yml", "AuthKey.md", +) # fmt: skip + + +# What the scan hides beyond the shared list, written out by hand: every name with "secret" or +# "credential" in it, such as source files and source folders a build reads, and every SSH key's +# .pub. A name the scan starts or stops hiding fails the end-to-end test. +SCAN_ONLY = ( + "id_rsa.pub", "secrets/app.json", "credentials/README.json", "secret_manager.py", + "credentials_helper.go", "secrets.yaml.example", "secrets.py", "secrets.sh", "id_ed25519.pub", + "secrets.ini.example", "secrets/manager.go", "credentials/README.txt", +) # fmt: skip + + +def test_every_glob_is_one_the_cli_takes_as_written() -> None: + assert SECRET_GLOBS == tuple(glob for glob, _ in TABLE) + for glob in ALL_SECRET_GLOBS: + assert glob.startswith("**/"), glob + assert re.fullmatch(r"[A-Za-z0-9._*/-]+", glob), glob + backups = ALL_SECRET_GLOBS[len(SECRET_GLOBS) :] + assert all(glob.endswith(".bak") for glob in backups) + # The backup forms are derived: one for each glob with no `*` after `**/`, and `*.kubeconfig`. + # Built here from the hand-written TABLE, not from the module. + exact = [glob for glob, _ in TABLE if "*" not in glob.removeprefix("**/")] + assert set(backups) == {f"{glob}.bak" for glob in [*exact, "**/*.kubeconfig"]} + assert len(set(backups)) == len(backups) + # The CLI's deny rules come from the same derived list. + assert secret_read_rules() == [f"Read(//{glob})" for glob in ALL_SECRET_GLOBS] + assert all(f"Read(//{glob})" in secret_read_rules() for glob in backups) + + +def test_a_glob_means_the_path_or_any_folder_above_it() -> None: + assert files._glob_pattern("**/.pgpass") == r"(^|/)\.pgpass(/|$)" + assert files._glob_pattern("**/*.tfstate") == r"(^|/)[^/]*\.tfstate(/|$)" + assert files._glob_pattern("**/.kube/config.*") == r"(^|/)\.kube/config\.[^/]*(/|$)" + + +@pytest.mark.parametrize("glob, sample", TABLE, ids=[glob for glob, _ in TABLE]) +def test_every_listed_name_is_refused_however_it_is_written( + tmp_path: Path, glob: str, sample: str +) -> None: + for written in (sample, f"a/b/{sample}", sample.upper(), f"/abs/x/{sample}", f"{sample}/inner"): + assert secret_text(written), written + others = [other for other in ALL_SECRET_GLOBS if other != glob] + assert not re.search("|".join(files._glob_pattern(g) for g in others), sample, re.I), sample + target = tmp_path / sample + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("x") + (tmp_path / "plain.txt").symlink_to(target) + (tmp_path / "plainfolder").symlink_to(target.parent) + assert secret_path("plain.txt", tmp_path) + assert secret_path(f"plainfolder/{target.name}", tmp_path) + if sample in BACKED_UP: + assert secret_text(f"{sample}.bak") + if "*" not in glob.removeprefix("**/"): + # The exact name's own backup, however it is written, and the backup glob is needed for it + # (it is the one glob among the rest that matches this form). + name = f"{glob.removeprefix('**/')}.bak" + for written in (name, f"a/b/{name}", name.upper(), f"/abs/x/{name}"): + assert secret_text(written), written + lone = [other for other in ALL_SECRET_GLOBS if other != f"{glob}.bak"] + covered = re.search("|".join(files._glob_pattern(g) for g in lone), name, re.I) + # `.kube/config.*` and `.dev.vars.*` already say these two; no other name is covered twice. + assert (covered is None) == (glob not in ("**/.kube/config", "**/.dev.vars")), name + + +@pytest.mark.parametrize( + "name", + [ + ".netrc.bak", ".npmrc.bak", ".pypirc.bak", ".databrickscfg.bak", ".terraformrc.bak", + ".cargo/credentials.bak", ".gem/credentials.bak", "functions/local.settings.json.bak", + ".secrets.bak", ".authoring-credentials.bak", "android/key.properties.bak", + ".config/gh/hosts.yml.bak", ".ssh.bak", + ], +) # fmt: skip +def test_the_backup_copy_of_every_exact_name_is_refused(name: str) -> None: + assert secret_text(name) and secret_text(f"a/b/{name}") + + +@pytest.mark.parametrize("name", NEAR_MISSES) +def test_near_misses_are_not_refused(name: str) -> None: + assert not secret_text(name) + assert not secret_text(f"a/{name}") + + +def test_every_claude_session_carries_the_rules() -> None: + from fi.alk.harness.backends.claude import ClaudeBackend + + rules = [f"Read(//{glob})" for glob in ALL_SECRET_GLOBS] + assert secret_read_rules() == rules + for gated in (True, False): + spec = SessionSpec( + system_prompt="x", builtins=("Read", "Bash"), model="gemini-test", gated=gated + ) + disallowed = ClaudeBackend()._options(spec).disallowed_tools + assert disallowed[len(disallowed) - len(rules) :] == rules, gated + assert "Bash" not in disallowed # granted, so not disallowed + if gated: + assert "Write" in disallowed + + +# ---- The end-to-end test, with the real bundled CLI ------------------------------------------- + + +class _FakeMessages: + """A local stand-in for the Messages API. Each request is answered with the next scripted + tool call for its session (one that offers `Task` is the parent; one that doesn't is the + worker), then a plain end. Every request is kept, so the tool results can be read after.""" + + def __init__(self, parent: list[dict], worker: list[dict], *, split: bool = True) -> None: + self.split = split + self.queues = {"parent": list(parent), "worker": list(worker)} + self.served = {"parent": 0, "worker": 0} + self.requests: list[dict] = [] + self.lock = threading.Lock() + outer = self + + class Handler(BaseHTTPRequestHandler): + def log_message(self, *args: Any) -> None: + pass + + def do_GET(self) -> None: + self._json(b"{}") + + def _json(self, body: bytes) -> None: + self.send_response(200) + self.send_header("content-type", "application/json") + self.end_headers() + self.wfile.write(body) + + def do_POST(self) -> None: + size = int(self.headers.get("content-length") or 0) + body = json.loads(self.rfile.read(size) or b"{}") + if "count_tokens" in self.path: + self._json(b'{"input_tokens": 10}') + return + tools = {tool.get("name") for tool in body.get("tools") or []} + role = "parent" if outer.split and tools & {"Task", "Agent"} else "worker" + with outer.lock: + outer.requests.append({"role": role, "tools": sorted(tools), "body": body}) + call = None + if tools & {"Grep", "Glob", "Read", "Task", "Agent"} and outer.queues[role]: + call = outer.queues[role].pop(0) + outer.served[role] += 1 + self.send_response(200) + self.send_header("content-type", "text/event-stream") + self.end_headers() + self._events(call, body.get("model", "fake")) + + def _events(self, call: dict | None, model: str) -> None: + def send(event: str, data: dict) -> None: + self.wfile.write(f"event: {event}\ndata: {json.dumps(data)}\n\n".encode()) + + message = { + "id": "msg_fake", "type": "message", "role": "assistant", "model": model, + "content": [], "stop_reason": None, "stop_sequence": None, + "usage": {"input_tokens": 10, "output_tokens": 1}, + } # fmt: skip + send("message_start", {"type": "message_start", "message": message}) + if call is not None: + block = {"type": "tool_use", "id": call["id"], "name": call["name"], "input": {}} + delta = {"type": "input_json_delta", "partial_json": json.dumps(call["input"])} + stop = "tool_use" + else: + block = {"type": "text", "text": ""} + delta = {"type": "text_delta", "text": "done"} + stop = "end_turn" + send("content_block_start", {"type": "content_block_start", "index": 0, "content_block": block}) + send("content_block_delta", {"type": "content_block_delta", "index": 0, "delta": delta}) + send("content_block_stop", {"type": "content_block_stop", "index": 0}) + send("message_delta", {"type": "message_delta", "delta": {"stop_reason": stop, "stop_sequence": None}, "usage": {"output_tokens": 5}}) + send("message_stop", {"type": "message_stop"}) + self.wfile.flush() + + self.server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + self.thread = threading.Thread(target=self.server.serve_forever, daemon=True) + + def __enter__(self) -> "_FakeMessages": + self.thread.start() + return self + + def __exit__(self, *exc: Any) -> None: + self.server.shutdown() + self.server.server_close() + + @property + def url(self) -> str: + return f"http://127.0.0.1:{self.server.server_address[1]}" + + def results(self) -> dict[str, tuple[str, str]]: + """Each tool call's result text, and the role of the session that sent it back. An error + result's text starts with "ERROR:", so a case can't pass on a call that failed.""" + found: dict[str, tuple[str, str]] = {} + for request in self.requests: + for message in request["body"].get("messages") or []: + if message.get("role") != "user" or not isinstance(message.get("content"), list): + continue + for block in message["content"]: + if block.get("type") == "tool_result": + text = json.dumps(block.get("content")) + if block.get("is_error"): + text = "ERROR:" + text + found[block["tool_use_id"]] = (text, request["role"]) + return found + + +_MARK = re.compile(r"MARK-[PN]\d+") + + +def _fixture(source: Path) -> tuple[dict[str, str], dict[str, str]]: + """One fake file for each listed sample and each near miss, each with its own marker.""" + positives = {sample: f"MARK-P{n}" for n, (_, sample) in enumerate(TABLE)} + positives.update({f"{sample}.bak": f"MARK-P{100 + n}" for n, sample in enumerate(BACKED_UP)}) + near = {name: f"MARK-N{n}" for n, name in enumerate(NEAR_MISSES)} + for path, mark in {**positives, **near}.items(): + target = source / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(f"{mark} password\n") + (source / "k").symlink_to(".kube") + return positives, near + + +def _expected(near: dict[str, str], root: str) -> set[str]: + if root and secret_text(root): + return set() + return {mark for path, mark in near.items() if not root or path.startswith(f"{root}/")} + + +def _calls(source: Path, prefix: str) -> list[tuple[dict, str]]: + """Each call and the folder (relative to the source) whose near misses it should show.""" + where = (lambda rel: str(source / rel) if rel else str(source)) if prefix else (lambda rel: rel or ".") + calls = [ + ({"name": "Grep", "input": {"pattern": "password", "path": where(""), "output_mode": "content"}}, ""), + ({"name": "Grep", "input": {"pattern": "password", "path": where("k"), "output_mode": "content"}}, ".kube"), + ({"name": "Grep", "input": {"pattern": "password", "path": where(""), "glob": "**/.pgpass", "output_mode": "content"}}, "-"), + ({"name": "Glob", "input": {"pattern": "**/*", "path": where("")}}, "glob"), + ] + for rel in (".kube", ".docker", ".docker/contexts", ".config", "infra"): + calls.append(({"name": "Grep", "input": {"pattern": "password", "path": where(rel), "output_mode": "content"}}, rel)) + if not prefix: + calls.append(({"name": "Grep", "input": {"pattern": "password", "output_mode": "content"}}, "")) + return calls + + +def _run(options: Any) -> None: + from claude_agent_sdk import query + + async def go() -> None: + async for _ in query(prompt="go", options=options): + pass + + asyncio.run(asyncio.wait_for(go(), 180)) + + +def _versions() -> str: + import claude_agent_sdk + + from claude_agent_sdk import _cli_version + + return f"sdk {claude_agent_sdk.__version__}, cli {getattr(_cli_version, '__cli_version__', '?')}" + + +@pytest.mark.parametrize("layout", ["source", "output"]) +def test_claude_code_and_the_kit_hide_the_same_files( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, layout: str +) -> None: + import claude_agent_sdk + + from fi.alk.harness import scan_ai + from fi.alk.harness.backends.claude import ClaudeBackend + + bundled = Path(claude_agent_sdk.__file__).parent / "_bundled" / "claude" + assert bundled.exists(), f"the bundled Claude Code CLI is missing ({_versions()})" + source = tmp_path / "src" + positives, near = _fixture(source) + out = tmp_path / "out" + out.mkdir() + cwd = source if layout == "source" else out + calls = _calls(source, "" if layout == "source" else "abs") + for index, (call, _) in enumerate(calls): + call["id"] = f"toolu_fake{index}" + worker_call = dict(calls[0][0], id="toolu_worker") + parent = [{"id": "toolu_task", "name": "Task", "input": {"subagent_type": "looker", "description": "look", "prompt": "go"}}] + for name, value in { + "HOME": str(tmp_path / "home"), + "CLAUDE_CONFIG_DIR": str(tmp_path / "cfg"), + "CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC": "1", + "ALK_CLAUDE_GATEWAY_API_KEY": "local-fake-not-a-key", + }.items(): + monkeypatch.setenv(name, value) + (tmp_path / "home").mkdir() + (tmp_path / "cfg").mkdir() + + with _FakeMessages(parent=[], worker=[c for c, _ in calls], split=False) as fake: + monkeypatch.setenv("ALK_CLAUDE_GATEWAY_URL", fake.url) + spec = SessionSpec(system_prompt="test", builtins=("Read", "Glob", "Grep"), cwd=str(cwd), model="gemini-test", max_turns=40) + _run(ClaudeBackend()._options(spec)) + assert fake.served["worker"] == len(calls), (fake.served, _versions()) + results = fake.results() + + kit = {spec.name: spec.handler for spec in file_tools(str(source))} + scan = {spec.name: spec.handler for spec in scan_ai.rooted_file_tools(source).tools} + for call, root in calls: + text, _ = results[call["id"]] + seen = set(_MARK.findall(text)) + assert not {m for m in seen if m.startswith("MARK-P")}, (call, seen, _versions()) + if root in ("glob", "-"): + assert not text.startswith("ERROR:"), (call, text[:300], _versions()) + if root == "glob": + listed = {line.strip().strip('"') for line in text.replace("\\n", "\n").splitlines()} + for path in positives: + assert not any(item.endswith(f"/{path}") or item == path for item in listed), (path, _versions()) + for path in (".kube/configmap.yaml", ".docker/Dockerfile", "agent.py"): + assert any(item.endswith(path) for item in listed), (path, text[:300], _versions()) + continue + if root == "-": + assert not seen, (call, seen) + continue + assert seen == _expected(near, root), (call, seen ^ _expected(near, root), _versions()) + kit_seen = set(_MARK.findall(json.dumps(asyncio.run(kit["Grep"]({"pattern": "password"}))))) + scan_seen = set(_MARK.findall(json.dumps(asyncio.run(scan["grep"]({"text": "password"}))))) + assert kit_seen == _expected(near, ""), kit_seen ^ _expected(near, "") + scan_own = {near[path] for path in SCAN_ONLY} + assert scan_seen == _expected(near, "") - scan_own, scan_seen ^ (_expected(near, "") - scan_own) + + # The same search inside a worker the session starts with Task keeps to the same rules. + worker = {"looker": WorkerSpec(description="looks", instructions="look", builtins=("Read", "Glob", "Grep"))} + with _FakeMessages(parent=parent, worker=[worker_call]) as fake: + monkeypatch.setenv("ALK_CLAUDE_GATEWAY_URL", fake.url) + spec = SessionSpec(system_prompt="test", builtins=("Read", "Glob", "Grep"), cwd=str(cwd), model="gemini-test", workers=worker, max_turns=40) + _run(ClaudeBackend()._options(spec)) + text, role = fake.results()["toolu_worker"] + assert role == "worker", fake.served + worker_tools = [set(r["tools"]) for r in fake.requests if r["role"] == "worker"] + parent_tools = [set(r["tools"]) for r in fake.requests if r["role"] == "parent"] + assert worker_tools and parent_tools, fake.served + # The worker's own session: the three file tools it was given, and none of the parent's others. + assert all({"Read", "Glob", "Grep"} <= tools and len(tools) < len(parent_tools[0]) for tools in worker_tools) + assert set(_MARK.findall(text)) == _expected(near, ""), _versions() diff --git a/tests/harness/test_source_data_invariants_folder.py b/tests/harness/test_source_data_invariants_folder.py index bdbb3f7c2..831a790be 100644 --- a/tests/harness/test_source_data_invariants_folder.py +++ b/tests/harness/test_source_data_invariants_folder.py @@ -94,3 +94,16 @@ def test_a_linked_folder_is_listed_by_what_it_points_at(repo): def test_a_folder_that_cannot_be_resolved_is_taken_as_written(repo): assert set(invariants.source_files(repo, "agents/gone")) == {"db.py", "agents/shared.py"} + + +def test_a_link_named_like_code_to_a_credential_file_is_not_listed(tmp_path): + # TH-9042: the review session reads any listed file whole, so one is judged where it resolves. + (tmp_path / "app.py").write_text("x = 1\n") + (tmp_path / ".pgpass").write_text("db:5432:app:admin:fake-pass-123\n") + (tmp_path / "prod.tfvars").write_text('db_password = "fake-pass-123"\n') + (tmp_path / "settings_local.py").symlink_to(".pgpass") + (tmp_path / "vars.sql").symlink_to("prod.tfvars") + + listed = invariants.source_files(tmp_path) + + assert set(listed) == {"app.py"} diff --git a/tests/test_harness.py b/tests/test_harness.py index 7ef935fc6..a3c73f68a 100644 --- a/tests/test_harness.py +++ b/tests/test_harness.py @@ -1337,6 +1337,22 @@ def test_github_source_reads_like_a_repository(tmp_path): assert "https://github.com/acme/agent" in source.briefing() +def test_the_briefing_file_list_leaves_out_credential_files(tmp_path): + listed = ( + ".env.production", ".aws/credentials", "deploy/gcp-sa.json", ".kube/config", + "infra/prod.tfvars", ".netrc.bak", + ) # fmt: skip + for name in (*listed, "agent.py", "deploy/app.yaml"): + (tmp_path / name).parent.mkdir(parents=True, exist_ok=True) + (tmp_path / name).write_text("canary\n") + + briefing = RepoSource(name="a", root=tmp_path).briefing() + + assert "- agent.py" in briefing and "- deploy/app.yaml" in briefing + for name in listed: + assert f"- {name}" not in briefing, name + + def test_spec_source_gets_no_file_tools_because_there_is_nothing_to_read(): source = SpecSource( name="a", system_prompt="you are a bot", tool_schema=[{"name": "t"}]