From 497222094f0c92b3de159d1001319d61693b17c5 Mon Sep 17 00:00:00 2001 From: Kit OC5 Date: Thu, 24 Sep 2026 03:04:13 -0400 Subject: [PATCH] secret-guard: triaged allow list (fakes by hash, test PEMs by path) Private-key matches are only the BEGIN line (same hash everywhere), so they are allowed by path+kind; everything else by hash. Real revoked tokens (1354fc9b, d49dc2ba) are pinned by a test to never be allowed. Co-Authored-By: Claude Opus 5.5 --- api/tests/test_secret_guard.py | 30 ++++++++++++++++++--- ci/secret-guard-allow.yml | 49 +++++++++++++++++++++++++++++++--- scripts/secret_guard.py | 34 ++++++++++++++++------- 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/api/tests/test_secret_guard.py b/api/tests/test_secret_guard.py index 294d4f7..8009f64 100644 --- a/api/tests/test_secret_guard.py +++ b/api/tests/test_secret_guard.py @@ -61,13 +61,35 @@ def test_allow_is_by_hash_only(): F = sg.cg.Finding fake = F("tests/t.py", 3, "telegram bot token", f"telegram bot token #{ss.h8(TG)}") real = F("tests/t.py", 9, "telegram bot token", "telegram bot token #deadbeef") - kept = sg._drop_allowed("windy-chat", [fake, real], {"windy-chat": {ss.h8(TG)}}) - assert kept == [real] - assert sg._drop_allowed("windy-mail", [fake], {"windy-chat": {ss.h8(TG)}}) == [fake] + allow = {"windy-chat": {"hashes": {ss.h8(TG)}, "paths": []}} + assert sg._drop_allowed("windy-chat", [fake, real], allow) == [real] + assert sg._drop_allowed("windy-mail", [fake], allow) == [fake] + + +def test_private_key_blocks_are_allowed_by_path_never_by_hash(): + F = sg.cg.Finding + hdr = "private key block #" + ss.h8("-----BEGIN PRIVATE KEY-----") + test_key = F("tests/keys/test.pem", 1, "private key block", hdr) + prod_key = F("deploy/prod.pem", 1, "private key block", hdr) + allow = {"r": {"hashes": {hdr.rsplit("#", 1)[1]}, "paths": [("tests/keys/*", {"private key block"})]}} + assert sg._drop_allowed("r", [test_key, prod_key], allow) == [prod_key] + + +def test_path_allow_cannot_cover_real_token_kinds(tmp_path): + bad = tmp_path / "a.yml" + bad.write_text("allow:\n - repo: r\n paths: [tests/*]\n kinds: [telegram bot token]\n reason: no\n") + with pytest.raises(ValueError): + sg.load_allow(bad) + + +def test_shipped_allow_file_never_excuses_the_real_leaked_tokens(): + a = sg.load_allow() + every = set().union(*(v["hashes"] for v in a.values())) if a else set() + assert not {"1354fc9b", "d49dc2ba"} & every # real (now revoked) credentials: remove, never allow def test_allow_file_loads_and_needs_reasons(tmp_path): - assert sg.load_allow() == {} or isinstance(sg.load_allow(), dict) + assert isinstance(sg.load_allow(), dict) bad = tmp_path / "a.yml" bad.write_text("allow:\n - repo: r\n hashes: [abcd1234]\n") with pytest.raises(ValueError): diff --git a/ci/secret-guard-allow.yml b/ci/secret-guard-allow.yml index cf7d18b..99ec5df 100644 --- a/ci/secret-guard-allow.yml +++ b/ci/secret-guard-allow.yml @@ -1,5 +1,46 @@ # Secret guard allow-list: KNOWN FAKE values that look like secrets (test -# fixtures, docs). Allowed BY HASH (sha256[:8] of the value), never by path, so a -# real secret in the same file still flags. Every entry MUST say why. -# Owner: Windy Git lane (13); changes go through the orchestrator. -allow: [] +# fixtures, docs). Allowed BY HASH (sha256[:8] of the value), so a real secret +# in the same file still flags. Private-key blocks (the match is only the BEGIN +# line, same hash everywhere) are allowed by PATH + kind instead. +# Every entry MUST say why. Owner: Windy Git lane (13); changes via the orchestrator. +# Triage 09-24 (values never printed): each hash checked against every version of +# the lockbox; fakes judged by impossible length for the kind (real Anthropic keys +# ~108 chars, OpenAI 51 or 160+), fake-words, or identity with upstream public +# fixtures. NOT allowed, remove instead: 1354fc9b (old @Windy_0_bot token) and +# d49dc2ba (old Anthropic key), both real and revoked, in public windy-agent. +allow: + - repo: windy-code + hashes: [ac9265e5, 46eb1235] + reason: "Upstream microsoft/vscode terminalEnvironment.test.ts fixtures (identical hash upstream; public)." + - repo: windy-code + paths: ["build/azure-pipelines/common/publish.ts"] + kinds: [private key block] + reason: "Upstream VS Code build script (PEM header string in code, not a key)." + - repo: windy-agent + hashes: [a9235a6d, dd6a2baa, 02c362d8, a6f6ff79, f7503b21, d0c94833, 4ab092e3, e3aa1eb8, 833382ee] + reason: "Redaction/sanitizer test fixtures; lengths impossible for real Anthropic/OpenAI keys; never in the lockbox." + - repo: windy-agent + paths: ["tests/test_agent_keys.py"] + kinds: [private key block] + reason: "Test-generated key material for agent-key tests." + - repo: windy-mind + hashes: [f8a630b2] + reason: "Provider test fixture (27 chars; a real Anthropic key is ~108)." + - repo: windy-pro + hashes: [756de8d8, 7828319d, 1a5d44a2] + reason: ".env.production.example placeholder + crash-summary test fixtures (AWS doc EXAMPLE key shape, short fake Slack token)." + - repo: windy-pro + paths: ["account-server/docs/oauth-providers.md"] + kinds: [private key block] + reason: "Docs show the PEM header format; no key material." + - repo: windytalk + hashes: [baf8656a] + reason: "Diagnostics redaction test fixture (fake-word in value)." + - repo: eternitas + paths: ["tests/golden_vectors/**", "tests/test_soul_vault_key_separation.py"] + kinds: [private key block] + reason: "Test vectors and throwaway keys for signature/vault tests." + - repo: windy-drops + paths: ["tools/conformance/test-keys/*"] + kinds: [private key block] + reason: "Conformance-suite test keys (named test-private.pem)." diff --git a/scripts/secret_guard.py b/scripts/secret_guard.py index cffffb2..d658111 100644 --- a/scripts/secret_guard.py +++ b/scripts/secret_guard.py @@ -11,6 +11,7 @@ BY HASH in ci/secret-guard-allow.yml (repo + hashes + reason). from __future__ import annotations +import fnmatch import hashlib import os import re @@ -33,14 +34,24 @@ def path_ok(path: str) -> bool: return not NEVER.search(path) -def load_allow(path: Path = ALLOW_FILE) -> dict[str, set[str]]: - """{repo: {hash8, ...}}; every entry needs a reason.""" +# A private-key match is only its BEGIN line, so its hash is the same everywhere: +# those are allowed by PATH (entries with `paths` + `kinds`), everything else by HASH. +PATH_ONLY_KINDS = {"private key block"} + + +def load_allow(path: Path = ALLOW_FILE) -> dict[str, dict]: + """{repo: {"hashes": {hash8}, "paths": [(glob, {kind})]}}; every entry needs a reason.""" data = yaml.safe_load(path.read_text()) if path.exists() else {} - out: dict[str, set[str]] = {} + out: dict[str, dict] = {} for e in (data or {}).get("allow") or []: - if not (e.get("repo") and e.get("hashes") and str(e.get("reason", "")).strip()): - raise ValueError(f"allow entry needs repo, hashes and a reason: {e}") - out.setdefault(e["repo"], set()).update(str(h) for h in e["hashes"]) + if not (e.get("repo") and (e.get("hashes") or (e.get("paths") and e.get("kinds"))) + and str(e.get("reason", "")).strip()): + raise ValueError(f"allow entry needs repo, hashes (or paths + kinds) and a reason: {e}") + if e.get("paths") and not set(e["kinds"]) <= PATH_ONLY_KINDS: + raise ValueError(f"path allows are only for {sorted(PATH_ONLY_KINDS)}: {e}") + r = out.setdefault(e["repo"], {"hashes": set(), "paths": []}) + r["hashes"].update(str(h) for h in e.get("hashes") or []) + r["paths"] += [(g, set(e["kinds"])) for g in e.get("paths") or []] return out @@ -48,9 +59,14 @@ def scan_line(path: str, text: str) -> list[tuple[str, str]]: return [(kind, f"{kind} #{h}") for kind, h in ss.find(text)] -def _drop_allowed(repo: str, findings, allow: dict[str, set[str]]): - ok = allow.get(repo, set()) - return [f for f in findings if f.match.rsplit("#", 1)[-1] not in ok] +def _drop_allowed(repo: str, findings, allow: dict[str, dict]): + a = allow.get(repo) or {"hashes": set(), "paths": []} + + def ok(f) -> bool: + if f.kind in PATH_ONLY_KINDS: + return any(f.kind in kinds and fnmatch.fnmatch(f.path, g) for g, kinds in a["paths"]) + return f.match.rsplit("#", 1)[-1] in a["hashes"] + return [f for f in findings if not ok(f)] def check(repo: str, sha: str, default_branch: str, is_default_head: bool):