From 2b69dbec7dfff36ecbabe7dc726207e12af74d6b Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 13:53:06 +0530 Subject: [PATCH 01/12] fix(harness): keep customers' credential files from the model on every reader - One list of globs (files.SECRET_GLOBS), read as gitignore reads them, builds both the regex the kit's tools, the scan's tools and the gate use and the Read deny rules every Claude Code session now carries. It adds .kube/config, kubeconfig files, .docker/config.json and contexts, .dockercfg, .pgpass, .htpasswd, .git-credentials, .my.cnf, .s3cfg, .boto, .vault-token, SSH key names, *.tfstate and backups, *.tfvars, *-key.json, *_key.json, *.pfx, *.jks and *.keystore, each exact name also as .bak. - Claude Code's own Grep and Glob searched hidden files below any folder the gate allowed; the root-anchored deny rules now make them skip listed files, in workers too. - The gate resolves a relative path in the session's folder (the hook's cwd). - The contract tool's prompt copy and the model citation refuse a listed file, judged where it resolves inside the source, before anything is opened. A .env file may still cite a model, only on a line whose setting names one, and never with its line count. - CI pins the Claude Agent SDK to uv.lock's version and runs the new tests and the model citation tests. TH-9042 Signed-off-by: khushalsonawat --- .github/workflows/sdk-smoke.yml | 9 ++ src/fi/alk/harness/backends/claude.py | 9 +- src/fi/alk/harness/backends/files.py | 120 ++++++++++++++++++++++++-- src/fi/alk/harness/config.py | 13 +-- src/fi/alk/harness/model_citation.py | 27 ++++++ src/fi/alk/harness/tools.py | 9 ++ 6 files changed, 175 insertions(+), 12 deletions(-) diff --git a/.github/workflows/sdk-smoke.yml b/.github/workflows/sdk-smoke.yml index 8589e0e0d..217d28188 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 from uv.lock, so the tests run the Claude Code CLI that ships + # (test_secret_paths.py drives it; TH-9042). + 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, @@ -81,6 +88,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..ec1f3fed8 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -39,13 +39,121 @@ 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*", + "**/.claude", + "**/.ssh", + "**/.aws", + "**/.config/gcloud", + "**/.authoring-credentials", + "**/.netrc", + "**/.npmrc", + "**/.pypirc", + "**/*credential*.json", + "**/*secret*.json", + "**/*token*.json", + "**/*service-account*.json", + "**/*service_account*.json", + "**/*serviceaccount*.json", + "**/*.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", ) +# Exact names whose `.bak` copy is a credential too. `.kube/config.*` already covers its own. +_WITH_BACKUP: tuple[str, ...] = ( + "**/kubeconfig", + "**/kubeconfig.yaml", + "**/kubeconfig.yml", + "**/*.kubeconfig", + "**/.docker/config.json", + "**/.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", +) +ALL_SECRET_GLOBS: tuple[str, ...] = SECRET_GLOBS + tuple(f"{glob}.bak" for glob in _WITH_BACKUP) +_ENV_GLOB = "**/.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 `.env` files: what a model citation may not name (`model_citation`). +_SECRET_BUT_ENV = _compiled(tuple(glob for glob in ALL_SECRET_GLOBS if glob != _ENV_GLOB)) + + +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/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 fe274a647..f5291738e 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 @@ -516,6 +517,14 @@ def _reads_as_a_setting(name: str) -> bool: return "." in name and not has_digit +def _env_key(line: str) -> str: + """The setting a `.env` line sets: what comes before its first `=`, without `export`.""" + key = line.split("=", 1)[0].strip() + if key.startswith("export ") or key.startswith("export\t"): + key = key[len("export") :].strip() + return key + + 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 @@ -562,6 +571,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. @@ -578,7 +598,14 @@ 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"] + if env_file and "MODEL" not in _env_key(lines[model.line - 1]).upper(): + return [ + f"model: in a .env file, cite the line whose setting names the model (such as " + f"OPENAI_MODEL); {shown_path} line {model.line} sets another setting" + ] 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/tools.py b/src/fi/alk/harness/tools.py index 48d734c16..8d5f86bd3 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, @@ -467,6 +468,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) From fcb1de7b59f99cf73651e22133a3ad5e3445dde3 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 13:53:06 +0530 Subject: [PATCH 02/12] test(harness): hold the credential list on every reader, with the real CLI test_secret_paths.py holds the glob list against a literal table, its near misses, the rules on every session, and runs the bundled Claude Code CLI against a local stand-in for the Messages API (no model, no key) in both session layouts and in a worker. The gate is tested through its hook with the payload's cwd; the prompt copy and the model citation get their refusal cases. Two model citation cases change answer by design: a .env line whose setting isn't a model's is now refused. TH-9042 Signed-off-by: khushalsonawat --- tests/harness/test_builder_chat.py | 37 ++ tests/harness/test_contract_model.py | 93 ++++- .../harness/test_contract_tools_subfolder.py | 59 +++ tests/harness/test_scan_ai.py | 6 + tests/harness/test_secret_paths.py | 391 ++++++++++++++++++ 5 files changed, 584 insertions(+), 2 deletions(-) create mode 100644 tests/harness/test_secret_paths.py diff --git a/tests/harness/test_builder_chat.py b/tests/harness/test_builder_chat.py index 703539a07..b3f4b31df 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): diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index 6ba8e7dc8..224924f73 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -75,6 +75,20 @@ 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); {path} line {line} sets another setting" + ) + + +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", @@ -2070,7 +2084,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( @@ -3122,7 +3137,6 @@ 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"), @@ -3463,3 +3477,78 @@ 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 == [] diff --git a/tests/harness/test_contract_tools_subfolder.py b/tests/harness/test_contract_tools_subfolder.py index 67c93ea88..5914e9ce6 100644 --- a/tests/harness/test_contract_tools_subfolder.py +++ b/tests/harness/test_contract_tools_subfolder.py @@ -2099,3 +2099,62 @@ 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, + ) + + +def test_a_prompt_named_in_a_credential_file_is_refused_and_never_copied(tmp_path, repo): + (repo / ".pgpass").write_text("db:5432:app:admin:fake-pass-123\n") + + for start, end in ((1, 1), (1, 40)): + result = _copy(tmp_path, repo, ".pgpass", 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_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..74495b7c3 --- /dev/null +++ b/tests/harness/test_secret_paths.py @@ -0,0 +1,391 @@ +"""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*", ".env.local"), + ("**/.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"), + ("**/*.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"), +) +# 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", +) # 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) + assert len(backups) == len(BACKED_UP) + + +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") + + +@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.""" + 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": + found[block["tool_use_id"]] = (json.dumps(block.get("content")), 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 == "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()) + 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 = {mark for path, mark in near.items() if any(scan_ai._secret_name(part) for part in path.split("/"))} + 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 + assert any(r["role"] == "worker" and "Task" not in r["tools"] for r in fake.requests) + assert set(_MARK.findall(text)) == _expected(near, ""), _versions() From 51b853443ea7788902a0ce059096fd4da98e42b1 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 14:33:06 +0530 Subject: [PATCH 03/12] fix(harness): answer the change review of the credential list Both reviewers cleared round 1 (A: 2 Medium, 7 Low; B: 2 Medium, 6 Low). - The source-data review lists no file that resolves to a listed one, so a link named like code (settings_local.py -> .pgpass) can't be read. - A .env model citation is accepted when its setting's words name a model (MODEL, DEPLOYMENT, ENGINE or LLM) and none marks a secret (KEY, TOKEN, SECRET, PASSWORD and the like), and a line that sets nothing is refused. This keeps Azure's AZURE_OPENAI_DEPLOYMENT and the like, which #268 accepted, and refuses MODEL_API_KEY. - Tests: .env in the prompt copy, a .bak copy in a model citation, the worker's own tool list, and no error behind the end-to-end Glob cases. - The CI comment says only the SDK is pinned. TH-9042 Signed-off-by: khushalsonawat --- .github/workflows/sdk-smoke.yml | 4 +- src/fi/alk/harness/model_citation.py | 23 +++++++--- src/fi/alk/harness/source_data_invariants.py | 3 ++ tests/harness/test_contract_model.py | 45 +++++++++++++++++++ .../harness/test_contract_tools_subfolder.py | 8 ++-- tests/harness/test_secret_paths.py | 18 ++++++-- .../test_source_data_invariants_folder.py | 13 ++++++ 7 files changed, 99 insertions(+), 15 deletions(-) diff --git a/.github/workflows/sdk-smoke.yml b/.github/workflows/sdk-smoke.yml index 217d28188..67a551283 100644 --- a/.github/workflows/sdk-smoke.yml +++ b/.github/workflows/sdk-smoke.yml @@ -45,8 +45,8 @@ jobs: '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 from uv.lock, so the tests run the Claude Code CLI that ships - # (test_secret_paths.py drives it; TH-9042). + # 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'] diff --git a/src/fi/alk/harness/model_citation.py b/src/fi/alk/harness/model_citation.py index f5291738e..ef1f45f55 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -517,12 +517,21 @@ def _reads_as_a_setting(name: str) -> bool: return "." in name and not has_digit -def _env_key(line: str) -> str: - """The setting a `.env` line sets: what comes before its first `=`, without `export`.""" - key = line.split("=", 1)[0].strip() - if key.startswith("export ") or key.startswith("export\t"): - key = key[len("export") :].strip() - return key +_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, and one that holds a secret even so. +_MODEL_WORDS = frozenset({"MODEL", "DEPLOYMENT", "ENGINE", "LLM"}) +_SECRET_WORDS = frozenset({"KEY", "TOKEN", "SECRET", "PASSWORD", "PASS", "PWD", "AUTH", "CREDENTIAL", "CREDENTIALS"}) + + +def _names_a_model(line: str) -> bool: + """Whether a `.env` line sets a model: a setting whose words include MODEL, DEPLOYMENT, ENGINE + or LLM (OPENAI_MODEL, AZURE_OPENAI_DEPLOYMENT, LLM) and none that marks a secret (MODEL_API_KEY). + A line that sets nothing (no `=` or `:` after a name) doesn't.""" + found = _ENV_SETTING.match(line) + if found is None: + return False + words = set(re.split(r"[_.-]+", found.group(1).upper())) + return bool(words & _MODEL_WORDS) and not words & _SECRET_WORDS def model_problems(contract: AgentContract, source_root: Path) -> list[str]: @@ -601,7 +610,7 @@ def model_problems(contract: AgentContract, source_root: Path) -> list[str]: 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"] - if env_file and "MODEL" not in _env_key(lines[model.line - 1]).upper(): + if env_file and not _names_a_model(lines[model.line - 1]): return [ f"model: in a .env file, cite the line whose setting names the model (such as " f"OPENAI_MODEL); {shown_path} line {model.line} sets another setting" 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/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index 224924f73..b69e2d71b 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -3552,3 +3552,48 @@ def test_a_source_under_a_listed_folder_still_cites_its_model(tmp_path, monkeypa 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("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"), + ], +) +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( + "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.yaml", "DB_PASSWORD: MODELfake123", "MODELfake123", id="colon-setting"), + pytest.param(".env", "MODELfake123", "MODELfake123", id="no-setting"), + ], +) +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 5914e9ce6..92610cbe8 100644 --- a/tests/harness/test_contract_tools_subfolder.py +++ b/tests/harness/test_contract_tools_subfolder.py @@ -2117,11 +2117,13 @@ def _copy(tmp_path: Path, root: Path, path: str, start: int = 1, end: int = 1) - ) -def test_a_prompt_named_in_a_credential_file_is_refused_and_never_copied(tmp_path, repo): - (repo / ".pgpass").write_text("db:5432:app:admin:fake-pass-123\n") +@pytest.mark.parametrize("name", [".pgpass", ".env"]) +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).write_text("db:5432:app:admin:fake-pass-123\n") for start, end in ((1, 1), (1, 40)): - result = _copy(tmp_path, repo, ".pgpass", start, end) + 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 diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py index 74495b7c3..5498a1a62 100644 --- a/tests/harness/test_secret_paths.py +++ b/tests/harness/test_secret_paths.py @@ -252,7 +252,8 @@ 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.""" + """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 []: @@ -260,7 +261,10 @@ def results(self) -> dict[str, tuple[str, str]]: continue for block in message["content"]: if block.get("type") == "tool_result": - found[block["tool_use_id"]] = (json.dumps(block.get("content")), request["role"]) + text = json.dumps(block.get("content")) + if block.get("is_error"): + text = "ERROR:" + text + found[block["tool_use_id"]] = (text, request["role"]) return found @@ -364,10 +368,14 @@ def test_claude_code_and_the_kit_hide_the_same_files( 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) @@ -387,5 +395,9 @@ def test_claude_code_and_the_kit_hide_the_same_files( _run(ClaudeBackend()._options(spec)) text, role = fake.results()["toolu_worker"] assert role == "worker", fake.served - assert any(r["role"] == "worker" and "Task" not in r["tools"] for r in fake.requests) + 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"} From c35bd7e0599fe854d605dd4e205142995407a96b Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 18:21:21 +0530 Subject: [PATCH 04/12] fix(harness): answer Dileep's review of the credential list Add prod.env-style settings files, secrets and credentials YAML and TOML, and *-sa.json keys to the one list, so every reader hides them, not just the scan. The end-to-end test now names by hand what only the scan hides. In a .env file, a setting holding KEY, TOKEN, SECRET, PASS, PWD, AUTH, CREDENTIAL, URL, URI or DSN anywhere in its name is never a model setting, and ENGINE counts only beside OPENAI or AZURE. Every kind of .env file takes that rule. The model check's time bounds are loosened to 2 s (5 s under tracemalloc), so a busy CI runner can't fail them. TH-9042 Signed-off-by: khushalsonawat --- src/fi/alk/harness/backends/files.py | 16 ++++++-- src/fi/alk/harness/model_citation.py | 22 +++++++---- src/fi/alk/harness/scan_ai.py | 6 ++- tests/harness/test_contract_model.py | 56 ++++++++++++++++++++++------ tests/harness/test_secret_paths.py | 24 +++++++++++- 5 files changed, 98 insertions(+), 26 deletions(-) diff --git a/src/fi/alk/harness/backends/files.py b/src/fi/alk/harness/backends/files.py index ec1f3fed8..12573146e 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -49,6 +49,8 @@ def _resolved(raw: str, cwd: Path) -> Path: # the SDK splits its rules on commas and spaces. SECRET_GLOBS: tuple[str, ...] = ( "**/.env*", + "**/*.env", + "**/*.env.*", "**/.claude", "**/.ssh", "**/.aws", @@ -63,6 +65,12 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/*service-account*.json", "**/*service_account*.json", "**/*serviceaccount*.json", + "**/*secret*.yaml", + "**/*secret*.yml", + "**/*secret*.toml", + "**/*credential*.yaml", + "**/*credential*.yml", + "**/*credential*.toml", "**/*.pem", "**/*.key", "**/*.p12", @@ -98,6 +106,7 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/*.tfvars.json", "**/*-key.json", "**/*_key.json", + "**/*-sa.json", ) # Exact names whose `.bak` copy is a credential too. `.kube/config.*` already covers its own. _WITH_BACKUP: tuple[str, ...] = ( @@ -122,7 +131,8 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/id_ed25519_sk", ) ALL_SECRET_GLOBS: tuple[str, ...] = SECRET_GLOBS + tuple(f"{glob}.bak" for glob in _WITH_BACKUP) -_ENV_GLOB = "**/.env*" +# 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(/|$)" @@ -138,8 +148,8 @@ def _compiled(globs: tuple[str, ...]) -> re.Pattern[str]: _SECRET_PATHS = _compiled(ALL_SECRET_GLOBS) -# The same, less `.env` files: what a model citation may not name (`model_citation`). -_SECRET_BUT_ENV = _compiled(tuple(glob for glob in ALL_SECRET_GLOBS if glob != _ENV_GLOB)) +# 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: diff --git a/src/fi/alk/harness/model_citation.py b/src/fi/alk/harness/model_citation.py index ef1f45f55..9924a9114 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -518,20 +518,28 @@ def _reads_as_a_setting(name: str) -> bool: _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, and one that holds a secret even so. -_MODEL_WORDS = frozenset({"MODEL", "DEPLOYMENT", "ENGINE", "LLM"}) -_SECRET_WORDS = frozenset({"KEY", "TOKEN", "SECRET", "PASSWORD", "PASS", "PWD", "AUTH", "CREDENTIAL", "CREDENTIALS"}) +# 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 whose words include MODEL, DEPLOYMENT, ENGINE - or LLM (OPENAI_MODEL, AZURE_OPENAI_DEPLOYMENT, LLM) and none that marks a secret (MODEL_API_KEY). + """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 - words = set(re.split(r"[_.-]+", found.group(1).upper())) - return bool(words & _MODEL_WORDS) and not words & _SECRET_WORDS + 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)) def model_problems(contract: AgentContract, source_root: Path) -> list[str]: diff --git a/src/fi/alk/harness/scan_ai.py b/src/fi/alk/harness/scan_ai.py index bdd22d9b2..b83963aa6 100644 --- a/src/fi/alk/harness/scan_ai.py +++ b/src/fi/alk/harness/scan_ai.py @@ -85,8 +85,10 @@ _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. Some of +# them are on the shared list too; the rest hide more than a build may: any name with "secret" or +# "credential" in it, such as `secret_manager.py`, and an SSH key's `.pub`. The scan only looks +# for agents, so that's the price of never showing a settings file under an unusual name. _SECRET_NAMES = ( "*.env", "*.env.*", diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index 6dfe86bbb..f3118ecd2 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -24,6 +24,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 = 2.0 +QUICK_SECONDS_TRACED = 5.0 FIXTURES = Path(__file__).resolve().parents[1] / "providers" / "fixtures" / "authored" EMPTY_NAME = ( @@ -304,7 +309,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 @@ -325,7 +330,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")]) @@ -424,7 +429,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 @@ -626,7 +631,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 @@ -1026,7 +1031,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 @@ -1626,7 +1631,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 @@ -1754,7 +1759,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): @@ -2061,7 +2066,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 @@ -2137,7 +2142,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): @@ -2148,7 +2153,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 @@ -2222,7 +2227,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): @@ -3142,7 +3147,8 @@ def test_a_pipe_is_logged_as_not_a_regular_file(tmp_path, caplog): 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 its line must set a model (since TH-9042's second round). + pytest.param("prod.env", "MODEL_NOTE=a#b MODEL=gpt-4o", "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"), @@ -3560,6 +3566,7 @@ def test_a_source_under_a_listed_folder_still_cites_its_model(tmp_path, monkeypa 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"), @@ -3573,12 +3580,37 @@ def test_a_env_setting_named_for_a_model_by_any_of_its_words_is_accepted(root, w assert problems == [] +@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", "MODEL_URL=https://admin:fake@llm.local/v1", "https://admin:fake@llm.local/v1", id="model-url"), + 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.yaml", "DB_PASSWORD: MODELfake123", "MODELfake123", id="colon-setting"), pytest.param(".env", "MODELfake123", "MODELfake123", id="no-setting"), ], diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py index 5498a1a62..e45ea019e 100644 --- a/tests/harness/test_secret_paths.py +++ b/tests/harness/test_secret_paths.py @@ -32,7 +32,9 @@ # 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*", ".env.local"), + ("**/.env*", ".envrc"), + ("**/*.env", "deploy/prod.env"), + ("**/*.env.*", "deploy/prod.env.example"), ("**/.claude", ".claude/settings.json"), ("**/.ssh", ".ssh/config"), ("**/.aws", ".aws/credentials"), @@ -47,6 +49,12 @@ ("**/*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"), ("**/*.pem", "certs/server.pem"), ("**/*.key", "certs/server.key"), ("**/*.p12", "certs/a.p12"), @@ -84,6 +92,7 @@ ("**/*.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 = ( @@ -103,6 +112,17 @@ "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", +) # fmt: skip + + +# What the scan hides beyond the shared list, written out by hand: the scan hides every name with +# "secret" or "credential" in it, and every SSH key's .pub, where a build must read those. 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", ) # fmt: skip @@ -384,7 +404,7 @@ def test_claude_code_and_the_kit_hide_the_same_files( 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 = {mark for path, mark in near.items() if any(scan_ai._secret_name(part) for part in path.split("/"))} + 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. From ce8ab4fd13dc526591fe392a6d86cbdcbbee4c09 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 18:40:26 +0530 Subject: [PATCH 05/12] fix(harness): answer the security check of Dileep's fixes In a .env file the cited name must be the value of the one setting that names the model, so a guess is never checked against a second setting on the same line (a compose default, ${MODEL:-gpt-4o}, counts as the value). The list also hides Firebase Admin SDK keys and secrets or credentials files in INI, CFG, CONF, properties and XML form. The prompt-copy test covers Dileep's prod.env and config/secrets.yaml. TH-9042 Signed-off-by: khushalsonawat --- src/fi/alk/harness/backends/files.py | 11 +++++++ src/fi/alk/harness/model_citation.py | 31 +++++++++++++++++-- tests/harness/test_contract_model.py | 23 +++++++++++--- .../harness/test_contract_tools_subfolder.py | 5 ++- tests/harness/test_secret_paths.py | 15 ++++++++- 5 files changed, 77 insertions(+), 8 deletions(-) diff --git a/src/fi/alk/harness/backends/files.py b/src/fi/alk/harness/backends/files.py index 12573146e..e163841d8 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -71,6 +71,17 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/*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", "**/*.pem", "**/*.key", "**/*.p12", diff --git a/src/fi/alk/harness/model_citation.py b/src/fi/alk/harness/model_citation.py index 9924a9114..6a7b12b9b 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -542,6 +542,28 @@ def _names_a_model(line: str) -> bool: 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. + default = _ENV_DEFAULT.fullmatch(value) + return default.group(1) if default else 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 @@ -618,10 +640,15 @@ def model_problems(contract: AgentContract, source_root: Path) -> list[str]: 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"] - if env_file and not _names_a_model(lines[model.line - 1]): + # 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 + ): return [ f"model: in a .env file, cite the line whose setting names the model (such as " - f"OPENAI_MODEL); {shown_path} line {model.line} sets another setting" + 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 diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index f3118ecd2..b41689d38 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -83,7 +83,8 @@ 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); {path} line {line} sets another setting" + f"OPENAI_MODEL), with the name as its value; {path} line {line} sets another setting " + "or another value" ) @@ -3147,8 +3148,9 @@ def test_a_pipe_is_logged_as_not_a_regular_file(tmp_path, caplog): 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"), - # prod.env is a .env file, so its line must set a model (since TH-9042's second round). - pytest.param("prod.env", "MODEL_NOTE=a#b 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"), @@ -3178,7 +3180,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"), @@ -3570,6 +3571,10 @@ def test_a_source_under_a_listed_folder_still_cites_its_model(tmp_path, monkeypa 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"), ], ) def test_a_env_setting_named_for_a_model_by_any_of_its_words_is_accepted(root, written, name): @@ -3611,6 +3616,16 @@ def test_every_kind_of_env_file_takes_the_same_rule(root, file): ), 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"), ], diff --git a/tests/harness/test_contract_tools_subfolder.py b/tests/harness/test_contract_tools_subfolder.py index e1c24bcc0..6554c2a35 100644 --- a/tests/harness/test_contract_tools_subfolder.py +++ b/tests/harness/test_contract_tools_subfolder.py @@ -2117,9 +2117,12 @@ def _copy(tmp_path: Path, root: Path, path: str, start: int = 1, end: int = 1) - ) -@pytest.mark.parametrize("name", [".pgpass", ".env"]) +@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)): diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py index e45ea019e..e1be69290 100644 --- a/tests/harness/test_secret_paths.py +++ b/tests/harness/test_secret_paths.py @@ -55,6 +55,17 @@ ("**/*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"), ("**/*.pem", "certs/server.pem"), ("**/*.key", "certs/server.key"), ("**/*.p12", "certs/a.p12"), @@ -114,6 +125,7 @@ "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", ) # fmt: skip @@ -122,7 +134,8 @@ # 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", + "credentials_helper.go", "secrets.yaml.example", "secrets.py", "secrets.sh", "id_ed25519.pub", + "secrets.ini.example", ) # fmt: skip From 6e11bdb66a17e54b7a677db8e599b423136b4fef Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 18:52:53 +0530 Subject: [PATCH 06/12] fix(harness): answer the second reviewer of Dileep's fixes The list also hides credentials CSV downloads, secrets and credentials text files, a secrets folder's text files, .secrets, and the settings files tools keep keys in (.dev.vars, local.settings.json, key.properties, keystore.properties, AuthKey_*.p8, *.ppk, gh's hosts.yml, .databrickscfg, .terraformrc, Cargo's and RubyGems' credentials). The scan's own list drops the three names the shared list now covers. The model check's untimed bound is 1 s again, close enough to catch a search that resumes one character on; and every secret word in a .env setting's name has its own case. TH-9042 Signed-off-by: khushalsonawat --- src/fi/alk/harness/backends/files.py | 17 +++++++++++++++++ src/fi/alk/harness/scan_ai.py | 11 ++++------- tests/harness/test_contract_model.py | 12 +++++++++++- tests/harness/test_secret_paths.py | 26 ++++++++++++++++++++++---- 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/fi/alk/harness/backends/files.py b/src/fi/alk/harness/backends/files.py index e163841d8..4fb3e8d6c 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -82,6 +82,23 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/*credential*.properties", "**/*credential*.xml", "**/*firebase-adminsdk*.json", + "**/*credential*.csv", + "**/*accesskeys*.csv", + "**/*secret*.txt", + "**/*credential*.txt", + "**/secrets/*.txt", + "**/.secrets", + "**/.dev.vars", + "**/local.settings.json", + "**/key.properties", + "**/keystore.properties", + "**/AuthKey_*.p8", + "**/*.ppk", + "**/.config/gh/hosts.yml", + "**/.databrickscfg", + "**/.terraformrc", + "**/.cargo/credentials", + "**/.gem/credentials", "**/*.pem", "**/*.key", "**/*.p12", diff --git a/src/fi/alk/harness/scan_ai.py b/src/fi/alk/harness/scan_ai.py index b83963aa6..38503d54a 100644 --- a/src/fi/alk/harness/scan_ai.py +++ b/src/fi/alk/harness/scan_ai.py @@ -85,19 +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." -# The scan applies the shared list (`files.SECRET_GLOBS`) and hides these names as well. Some of -# them are on the shared list too; the rest hide more than a build may: any name with "secret" or -# "credential" in it, such as `secret_manager.py`, and an SSH key's `.pub`. The scan only looks -# for agents, so 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/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index b41689d38..e947e1d52 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -27,7 +27,7 @@ # 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 = 2.0 +QUICK_SECONDS = 1.0 QUICK_SECONDS_TRACED = 5.0 FIXTURES = Path(__file__).resolve().parents[1] / "providers" / "fixtures" / "authored" @@ -3607,6 +3607,16 @@ def test_every_kind_of_env_file_takes_the_same_rule(root, file): 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", diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py index e1be69290..f372c3130 100644 --- a/tests/harness/test_secret_paths.py +++ b/tests/harness/test_secret_paths.py @@ -66,6 +66,23 @@ ("**/*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", "rootkey-accessKeys.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"), + ("**/local.settings.json", "functions/local.settings.json"), + ("**/key.properties", "android/key.properties"), + ("**/keystore.properties", "android/keystore.properties"), + ("**/AuthKey_*.p8", "AuthKey_ABC123.p8"), + ("**/*.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"), @@ -126,16 +143,17 @@ "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: the scan hides every name with -# "secret" or "credential" in it, and every SSH key's .pub, where a build must read those. A name the -# scan starts or stops hiding fails the end-to-end test. +# 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.ini.example", "secrets/manager.go", "credentials/README.txt", ) # fmt: skip From b9a1a67ab68daf1ca0bb90a6eae309049c641c31 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 19:08:04 +0530 Subject: [PATCH 07/12] test(harness): hold the base's new block-comment check to the shared bound The base's merged test kept a 0.5 s bound, which CI's runner passed at 0.68 s. It now uses the file's QUICK_SECONDS like the others. TH-9042 Signed-off-by: khushalsonawat --- tests/harness/test_contract_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index a17a1d7e4..f9a07cff9 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -2145,7 +2145,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( From 9e1666bb1b69fb11a370e7742d077e5941f80fe8 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 11:12:42 +0530 Subject: [PATCH 08/12] fix(harness): hide .dev.vars., rootkey.csv and every .p8, and take a compose default only from the setting itself From the #294 check: - .dev.vars. (Wrangler's per-environment form), rootkey.csv (the file AWS names a root access-key download) and any .p8 (always a PKCS#8 private key, not just Apple's AuthKey_) reached every reader. All three are on the list now; *.p8 replaces AuthKey_*.p8. - A .env line like OPENAI_MODEL=${DB_PASSWORD:-guess} confirmed the guess, because the default of any braced variable counted as the setting's value. It counts now only when the braced variable is the setting itself. - The a-file-of-openers timing case ran close to its 1 s bound; its input is halved. The planted slow path still fails it. TH-9042 Signed-off-by: khushalsonawat --- src/fi/alk/harness/backends/files.py | 4 +++- src/fi/alk/harness/model_citation.py | 9 ++++++--- tests/harness/test_contract_model.py | 4 +++- tests/harness/test_secret_paths.py | 6 ++++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/fi/alk/harness/backends/files.py b/src/fi/alk/harness/backends/files.py index 4fb3e8d6c..0fb23e5cd 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -84,15 +84,17 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/*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", - "**/AuthKey_*.p8", + "**/*.p8", "**/*.ppk", "**/.config/gh/hosts.yml", "**/.databrickscfg", diff --git a/src/fi/alk/harness/model_citation.py b/src/fi/alk/harness/model_citation.py index ae1fe772e..f7042b0da 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -596,7 +596,7 @@ def _names_a_model(line: str) -> bool: 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_]*:?-([^{}$]*)\}") +_ENV_DEFAULT = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*):?-([^{}$]*)\}") def _env_value(line: str) -> str | None: @@ -613,9 +613,12 @@ def _env_value(line: str) -> str | 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. + # Compose's default, ${MODEL:-gpt-4o}: the default is what the setting holds when unset. Only + # when the braced variable is the setting itself: ${DB_PASSWORD:-guess} is another setting's. default = _ENV_DEFAULT.fullmatch(value) - return default.group(1) if default else value + if default and default.group(1) == found.group(1): + return default.group(2) + return value def model_problems(contract: AgentContract, source_root: Path) -> list[str]: diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index f9a07cff9..79913946f 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -2123,7 +2123,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( @@ -3951,6 +3951,8 @@ def test_every_kind_of_env_file_takes_the_same_rule(root, file): 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"), ], ) def test_a_env_line_that_holds_a_secret_or_sets_nothing_is_refused(root, file, written, name): diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py index f372c3130..e39bde79e 100644 --- a/tests/harness/test_secret_paths.py +++ b/tests/harness/test_secret_paths.py @@ -67,16 +67,18 @@ ("**/*credential*.xml", "jenkins/credentials.xml"), ("**/*firebase-adminsdk*.json", "app/myproj-firebase-adminsdk-ab12c-0123456789.json"), ("**/*credential*.csv", "aws/my_credentials.csv"), - ("**/*accesskeys*.csv", "rootkey-accessKeys.csv"), + ("**/*accesskeys*.csv", "aws/accessKeys.csv"), + ("**/rootkey.csv", "rootkey.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"), - ("**/AuthKey_*.p8", "AuthKey_ABC123.p8"), + ("**/*.p8", "keys/SubscriptionKey_ABC123.p8"), ("**/*.ppk", "deploy/server.ppk"), ("**/.config/gh/hosts.yml", ".config/gh/hosts.yml"), ("**/.databrickscfg", ".databrickscfg"), From 47c6d647bf1913c9e0d933e5aecc502a600917d6 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 11:25:38 +0530 Subject: [PATCH 09/12] fix(harness): hide .pk8 keys and rootkey copies, and accept a default from another model setting From the 9e1666bb checks: - Android's PKCS#8 signing keys (platform.pk8, releasekey.pk8) reached every reader, for the same reason .p8 did. **/*.pk8 is on the list. - A second download of AWS's root key file is named "rootkey (1).csv". rootkey.csv becomes rootkey*.csv. - 9e1666bb refused CHAT_MODEL=${DEFAULT_MODEL:-gpt-4o-mini}, which was accepted before: a model setting defaulting from another model setting. The default counts again when the braced variable names a model and has no secret part; another secret's default still doesn't. 9e1666bb's message says the halved a-file-of-openers case still fails "the planted slow path". That case guards the block scan's last-closer cache, not the named_on resume plant, which the crafted_model_file cases catch. TH-9042 Signed-off-by: khushalsonawat --- src/fi/alk/harness/backends/files.py | 3 ++- src/fi/alk/harness/model_citation.py | 7 +++++-- tests/harness/test_contract_model.py | 3 +++ tests/harness/test_secret_paths.py | 3 ++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/fi/alk/harness/backends/files.py b/src/fi/alk/harness/backends/files.py index 0fb23e5cd..dc536d208 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -84,7 +84,7 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/*firebase-adminsdk*.json", "**/*credential*.csv", "**/*accesskeys*.csv", - "**/rootkey.csv", + "**/rootkey*.csv", "**/*secret*.txt", "**/*credential*.txt", "**/secrets/*.txt", @@ -95,6 +95,7 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/key.properties", "**/keystore.properties", "**/*.p8", + "**/*.pk8", "**/*.ppk", "**/.config/gh/hosts.yml", "**/.databrickscfg", diff --git a/src/fi/alk/harness/model_citation.py b/src/fi/alk/harness/model_citation.py index f7042b0da..1cae5469f 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -614,9 +614,12 @@ def _env_value(line: str) -> str | None: 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: ${DB_PASSWORD:-guess} is another setting's. + # 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): + if default and ( + default.group(1) == found.group(1) or _names_a_model(f"{default.group(1)}=") + ): return default.group(2) return value diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index 79913946f..ebe8818c5 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -3888,6 +3888,9 @@ def test_a_source_under_a_listed_folder_still_cites_its_model(tmp_path, monkeypa 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): diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py index e39bde79e..0c4438c07 100644 --- a/tests/harness/test_secret_paths.py +++ b/tests/harness/test_secret_paths.py @@ -68,7 +68,7 @@ ("**/*firebase-adminsdk*.json", "app/myproj-firebase-adminsdk-ab12c-0123456789.json"), ("**/*credential*.csv", "aws/my_credentials.csv"), ("**/*accesskeys*.csv", "aws/accessKeys.csv"), - ("**/rootkey.csv", "rootkey.csv"), + ("**/rootkey*.csv", "Downloads/rootkey (1).csv"), ("**/*secret*.txt", "api-secret.txt"), ("**/*credential*.txt", "gcp-credential.txt"), ("**/secrets/*.txt", "deploy/secrets/db_password.txt"), @@ -79,6 +79,7 @@ ("**/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"), From b62feaa3769770e3f91567b4fc86d735b9838646 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 11:32:37 +0530 Subject: [PATCH 10/12] test(harness): a model-named braced default that holds a secret is still refused Both 47c6d647 checks found no test pinned the secret-part check on the braced name: a check by model word alone passed every test while OPENAI_MODEL=${MODEL_API_KEY:-guess} confirmed the guess. Two refused cases now cover it, one in upper and one in lower case. TH-9042 Signed-off-by: khushalsonawat --- tests/harness/test_contract_model.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index ebe8818c5..5b093ef81 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -3956,6 +3956,9 @@ def test_every_kind_of_env_file_takes_the_same_rule(root, file): 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): From e6efff1af85d8a9c8dc8622e180654f60303fb11 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 12:20:05 +0530 Subject: [PATCH 11/12] fix(harness): answer Dileep's second review: two more readers, .bak copies, URL values, timing bounds - declare_tool refused nothing: it answered whether a quote appears in any source file, so a guess could be grown letter by letter against .env. It now gives a listed file (or a link to one, or a missing one) the same refusal as the kit's Read, before reading. The understand stage's briefing no longer names listed files. - Every exact name on the list now has its .bak form, derived from the list rather than kept by hand (.netrc.bak, .npmrc.bak, .pypirc.bak and the rest). The CLI deny rules follow. - A .env value that is a locator (any "://", a fullwidth at sign, or an "@" other than a Vertex version suffix like @20240620 or @latest) is refused, so MODEL_SERVER=postgres://admin:pw@db/app can't confirm a guess. - The four remaining 0.25 s timing asserts use QUICK_SECONDS. TH-9042 Signed-off-by: khushalsonawat --- src/fi/alk/harness/backends/files.py | 28 ++++-------- src/fi/alk/harness/builder.py | 12 +++++- src/fi/alk/harness/model_citation.py | 18 +++++++- src/fi/alk/harness/sources.py | 4 ++ tests/harness/test_builder_chat.py | 64 ++++++++++++++++++++++++++++ tests/harness/test_contract_model.py | 37 ++++++++++++++-- tests/harness/test_secret_paths.py | 32 +++++++++++++- tests/test_harness.py | 16 +++++++ 8 files changed, 184 insertions(+), 27 deletions(-) diff --git a/src/fi/alk/harness/backends/files.py b/src/fi/alk/harness/backends/files.py index dc536d208..a6b2176cc 100644 --- a/src/fi/alk/harness/backends/files.py +++ b/src/fi/alk/harness/backends/files.py @@ -139,29 +139,17 @@ def _resolved(raw: str, cwd: Path) -> Path: "**/*_key.json", "**/*-sa.json", ) -# Exact names whose `.bak` copy is a credential too. `.kube/config.*` already covers its own. +# 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, ...] = ( - "**/kubeconfig", - "**/kubeconfig.yaml", - "**/kubeconfig.yml", + *(glob for glob in SECRET_GLOBS if "*" not in glob.removeprefix("**/")), "**/*.kubeconfig", - "**/.docker/config.json", - "**/.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", ) -ALL_SECRET_GLOBS: tuple[str, ...] = SECRET_GLOBS + tuple(f"{glob}.bak" for glob in _WITH_BACKUP) +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. 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/model_citation.py b/src/fi/alk/harness/model_citation.py index 1cae5469f..640ac22fe 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -624,6 +624,20 @@ def _env_value(line: str) -> str | None: 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{8}|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 @@ -703,7 +717,9 @@ def model_problems(contract: AgentContract, source_root: Path) -> list[str]: # 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 + 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 " 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/tests/harness/test_builder_chat.py b/tests/harness/test_builder_chat.py index b3f4b31df..6895545bb 100644 --- a/tests/harness/test_builder_chat.py +++ b/tests/harness/test_builder_chat.py @@ -2101,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 5b093ef81..0a906ea30 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -2680,7 +2680,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 @@ -2812,7 +2812,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( @@ -2954,7 +2954,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): @@ -3187,7 +3187,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( @@ -3901,6 +3901,29 @@ def test_a_env_setting_named_for_a_model_by_any_of_its_words_is_accepted(root, w 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("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) @@ -3934,6 +3957,12 @@ def test_every_kind_of_env_file_takes_the_same_rule(root, file): 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", diff --git a/tests/harness/test_secret_paths.py b/tests/harness/test_secret_paths.py index 0c4438c07..36824522c 100644 --- a/tests/harness/test_secret_paths.py +++ b/tests/harness/test_secret_paths.py @@ -167,7 +167,14 @@ def test_every_glob_is_one_the_cli_takes_as_written() -> None: 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) - assert len(backups) == len(BACKED_UP) + # 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: @@ -193,6 +200,29 @@ def test_every_listed_name_is_refused_however_it_is_written( 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) 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"}] From 6da229c3a50c2a01336b9e300d5840026763446b Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 12:35:22 +0530 Subject: [PATCH 12/12] fix(harness): keep listed files out of runtime-repair evidence, and accept short Vertex versions From the e6efff1a checks: - The runtime repair stage took any evidence path in the repository and then checked whether the command's first word appeared in it, so a guess at a key in .env could be grown letter by letter. A listed file, a link to one and a missing one now get one refusal before anything is read. - e6efff1a accepted an "@" in a .env model name only before 8 digits or "latest", refusing real Vertex names like mistral-large@2407 and text-bison@001. Any digits now count. TH-9042 Signed-off-by: khushalsonawat --- src/fi/alk/harness/model_citation.py | 2 +- src/fi/alk/harness/runtime_repair.py | 6 ++++- tests/harness/test_contract_model.py | 2 ++ tests/harness/test_runtime_repair.py | 33 ++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/fi/alk/harness/model_citation.py b/src/fi/alk/harness/model_citation.py index 640ac22fe..bc821421a 100644 --- a/src/fi/alk/harness/model_citation.py +++ b/src/fi/alk/harness/model_citation.py @@ -626,7 +626,7 @@ def _env_value(line: str) -> str | None: # 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{8}|latest)", re.IGNORECASE) +_VERSIONED_NAME = re.compile(r"[^@:]+@(?:\d+|latest)", re.IGNORECASE) def _env_value_is_a_locator(value: str) -> bool: 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/tests/harness/test_contract_model.py b/tests/harness/test_contract_model.py index cd983b12b..91d1934ff 100644 --- a/tests/harness/test_contract_model.py +++ b/tests/harness/test_contract_model.py @@ -3921,6 +3921,8 @@ def test_a_env_setting_named_for_a_model_by_any_of_its_words_is_accepted(root, w [ 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"), ], ) 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"