From 36b5e770eb565c22c76483065a259bd2bbcb2279 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Wed, 22 Jul 2026 18:40:11 +0100 Subject: [PATCH] fix(detect): stop the sensitive-file filter from dropping topic docs and real source (#2106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The heuristic over-matched and silently dropped legitimate files: - prose `.md`/`.rst` whose topic slug ends in a keyword (privacy-tokens.md, token-economics.md) — only code was exempt, not prose; - the unbounded Stage-2 `service.account` substring (regex `.` wildcard) matched real source (google/oauth2/service_account.py) and prose slugs. It also MISSED real secrets (.npmrc, .pypirc, secring, .git-credentials, and case variants on case-insensitive filesystems), which were being indexed. Fix: move service_account/aws_credentials to the boundary-checked keyword path (so real source is spared, downloaded key files still drop), add a prose-note carve-out (multi-word slugs indexed, bare `secrets.md`/`token.md` still dropped), tighten id_rsa with a left boundary, add the missed secret dotfiles + secring, lowercase the dir/segment comparisons, and count multi-dot slugs as multi-word. Net effect is stricter on real secrets and stops the false-positive data loss. Traceability: `graphify extract` now names the files skipped as sensitive (not just a count), so a wrongly-flagged file is visible. --- graphify/cli.py | 13 +++++++ graphify/detect.py | 58 ++++++++++++++++++++++++----- tests/test_detect.py | 52 ++++++++++++++++++++++++-- tests/test_extract_code_only_cli.py | 14 +++++++ 4 files changed, 124 insertions(+), 13 deletions(-) diff --git a/graphify/cli.py b/graphify/cli.py index c631366c..91df0967 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -2817,6 +2817,19 @@ def dispatch_command(cmd: str) -> None: f"[graphify extract] {len(_unclassified)} file(s) not classified " f"(no supported extension or shebang), skipped: {_names}{_more}" ) + # Name the files dropped by the sensitive-file filter so a wrongly-flagged + # source/doc is visible, not just a count (#2106). Operational skips + # (symlink/office/Workspace) carry a " [reason]" suffix; exclude those here + # so this line reports only the security-heuristic drops. + _sensitive = detection.get("skipped_sensitive", []) if isinstance(detection, dict) else [] + _sec = [s for s in _sensitive if " [" not in s] + if _sec: + _snames = ", ".join(sorted({Path(p).name for p in _sec})[:6]) + _smore = f" (+{len(_sec) - 6} more)" if len(_sec) > 6 else "" + print( + f"[graphify extract] {len(_sec)} file(s) skipped as potentially sensitive " + f"(rename or move if wrongly flagged): {_snames}{_smore}" + ) stages.mark("detect") # Resolve the LLM backend only now that we know whether the corpus diff --git a/graphify/detect.py b/graphify/detect.py index 75c2c29c..fe7c653f 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -116,9 +116,17 @@ _AMBIGUOUS_SENSITIVE_DIRS = frozenset({ _SENSITIVE_PATTERNS = [ re.compile(r'(^|[\\/])\.(env|envrc)(\.|$)', re.IGNORECASE), re.compile(r'\.(pem|key|p12|pfx|cert|crt|der|p8)$', re.IGNORECASE), - re.compile(r'(id_rsa|id_dsa|id_ecdsa|id_ed25519)(\.pub)?$'), - re.compile(r'(\.netrc|\.pgpass|\.htpasswd)$', re.IGNORECASE), - re.compile(r'(aws_credentials|gcloud_credentials|service.account)', re.IGNORECASE), + # SSH/GPG private keys. Left boundary + IGNORECASE so `grid_rsa` (alpha before + # `id_rsa`) and `ID_RSA` are handled correctly, not matched as a substring. + re.compile(r'(^|[^A-Za-z0-9])(id_rsa|id_dsa|id_ecdsa|id_ed25519)(\.pub)?$', re.IGNORECASE), + re.compile(r'^secring(\.(gpg|pgp))?$', re.IGNORECASE), # GPG private keyring + # Auth/credential dotfiles that routinely hold tokens (#2106: .npmrc/.pypirc/ + # .git-credentials/.boto were silently indexed before). + re.compile(r'(\.netrc|\.pgpass|\.htpasswd|\.npmrc|\.pypirc|\.git-credentials|\.boto)$', re.IGNORECASE), + # NOTE: aws_credentials/gcloud_credentials/service_account moved to the + # boundary-checked Stage 3 keyword path (#2106). The old unbounded + # `service.account` substring (regex `.` wildcard) matched real source like + # google/oauth2/service_account.py and prose like aws_credentials_rotation.md. ] # Generic keyword patterns - these only count when the keyword is LOAD-BEARING @@ -134,8 +142,21 @@ _SENSITIVE_PATTERNS = [ _GENERIC_KEYWORD_PATTERNS = [ re.compile(r'(? bool: + """A prose/note file (.md/.rst/...) whose stem is a multi-word topic slug is + exempt from the generic-keyword drop (#2106). A stem that IS exactly a bare + keyword (secrets / token / passwords) is NOT exempt — that still reads as a + credential dump.""" + if path.suffix.lower() not in _PROSE_EXTS: + return False + stem = Path(path.name).stem.lstrip('.') or Path(path.name).stem + return not any(p.fullmatch(stem) for p in _GENERIC_KEYWORD_PATTERNS) def _generic_keyword_hit(name: str) -> bool: @@ -167,9 +199,11 @@ def _generic_keyword_hit(name: str) -> bool: ("token-economics-of-recall.md", "password-policy-discussion.md") and must not cause the file to be silently dropped from the graph (#436, #718). """ - # Stem = name up to the first dot, ignoring leading dots so dotfiles like - # ".token" keep their keyword ("" stems would never match). - stem = name.lstrip('.').split('.')[0] + # Stem = name minus only the FINAL extension (not up to the first dot), so a + # multi-dot topic slug like `token.economics.notes.md` keeps all its words and + # doesn't collapse to a bare `token` (#2106). Leading dots stripped so + # dotfiles like `.token` keep their keyword. + stem = Path(name).stem.lstrip('.') or Path(name).stem for pat in _GENERIC_KEYWORD_PATTERNS: hit = False for m in pat.finditer(stem): @@ -218,9 +252,11 @@ def _is_sensitive(path: Path) -> bool: # (secrets/, credentials/) spare genuine source (#1943), which still falls # through so Stages 2-3 screen its filename like anywhere else. parents = path.parts[:-1] - if any(part in _CREDENTIAL_STORE_DIRS for part in parents): + # Lowercase the segment comparison so `Secrets/`/`SECRETS/` (real on + # case-insensitive macOS/Windows filesystems) are still caught (#2106). + if any(part.lower() in _CREDENTIAL_STORE_DIRS for part in parents): return True - if any(part in _AMBIGUOUS_SENSITIVE_DIRS for part in parents) and not _is_graphable_source(path): + if any(part.lower() in _AMBIGUOUS_SENSITIVE_DIRS for part in parents) and not _is_graphable_source(path): return True # Stage 2: filename pattern match name = path.name @@ -235,7 +271,9 @@ def _is_sensitive(path: Path) -> bool: # secret stores this stage must catch. The specific Stage 2 patterns (.env, .pem, # id_rsa, ...) still apply to everything regardless of extension. if _generic_keyword_hit(name): - return not _is_graphable_source(path) + # Genuine source AND multi-word prose notes are exempt; a bare-keyword + # name (secrets.md, token.txt) still drops (#1666, #2106). + return not (_is_graphable_source(path) or _is_prose_note(path)) return False diff --git a/tests/test_detect.py b/tests/test_detect.py index f6f1f322..0298bea7 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -1,5 +1,6 @@ import os import unicodedata +import pytest from pathlib import Path from graphify.detect import classify_file, count_words, detect, detect_incremental, save_manifest, FileType, _looks_like_paper, _is_ignored, _load_graphifyignore, _is_sensitive from graphify import detect as detect_mod @@ -1165,10 +1166,15 @@ def test_sensitive_flags_everything_under_credential_store_dirs(): assert _is_sensitive(Path("backup/.gcloud/sync.sh")) def test_sensitive_dir_carveout_does_not_bypass_name_screens(): - # #1943: rescued source still falls through to Stages 2-3, so a file whose - # NAME is sensitive stays dropped even though its dir carve-out applied. - assert _is_sensitive(Path("secrets/service_account.py")) # Stage 2 pattern + # #1943: rescued source still falls through to Stages 2-3, so a NON-source + # file whose name/extension is sensitive stays dropped even though the dir + # carve-out spared genuine source beside it. assert _is_sensitive(Path("credentials/id_rsa")) # extensionless key + assert _is_sensitive(Path("secrets/deploy.pem")) # Stage 2 extension + # #2106: `service_account.py` is real source (e.g. Google's oauth2 lib), not a + # secret. The old unbounded `service.account` substring wrongly dropped it; + # it is now indexed. A downloaded `service-account.json` key still drops. + assert not _is_sensitive(Path("secrets/service_account.py")) def test_sensitive_dir_carveout_still_drops_tfvars_values_store(): @@ -2288,3 +2294,43 @@ def test_detect_incremental_exclusion_stable_across_runs(tmp_path): inc2 = detect_incremental(tmp_path, manifest_path, extra_excludes=["b.py"]) assert inc2["deleted_files"] == [] assert inc2["excluded_files"] == [] + + +# ── #2106: sensitive-filter over-match (prose/source rescued, real secrets kept) ── + +@pytest.mark.parametrize("path", [ + "wiki/privacy-tokens.md", # reporter's own hub node + "wiki/ai-token-economics.md", + "wiki/chain-of-hope-tokenomics.md", + "tokenizer.py", + "secretary.py", + "google/oauth2/service_account.py", # real Google auth source + "docs/service-account-setup.md", + "wiki/aws_credentials_rotation_guide.md", + "token.economics.notes.md", # multi-dot topic slug + "password-reset/design.md", +]) +def test_sensitive_filter_indexes_topic_prose_and_source(path): + from graphify.detect import _is_sensitive + assert not _is_sensitive(Path(path)), f"{path} is a topic doc / real source, must be indexed (#2106)" + + +@pytest.mark.parametrize("path", [ + ".env", "id_rsa", "credentials.json", "server.pem", "certs/server.key", + "secrets.md", "passwords.md", "token.md", "token.txt", "api_token.json", + "service-account.json", # a downloaded GCP key file + ".npmrc", ".pypirc", "secring.gpg", ".git-credentials", # #2106 newly-caught + "Secrets/creds.json", "SECRETS/db.json", "ID_RSA", # #2106 case variants + "secrets/prod.tfvars", "credentials/id_rsa", +]) +def test_sensitive_filter_still_excludes_real_secrets(path): + from graphify.detect import _is_sensitive + assert _is_sensitive(Path(path)), f"{path} is a real secret, must stay excluded (#2106)" + + +def test_sensitive_bare_keyword_prose_still_dropped(): + """A prose file whose stem IS exactly a bare keyword still reads as a dump.""" + from graphify.detect import _is_sensitive + assert _is_sensitive(Path("secrets.md")) + assert _is_sensitive(Path("token.rst")) + assert not _is_sensitive(Path("token-lifecycle.md")) # multi-word slug indexed diff --git a/tests/test_extract_code_only_cli.py b/tests/test_extract_code_only_cli.py index 437f5bdb..93fec48d 100644 --- a/tests/test_extract_code_only_cli.py +++ b/tests/test_extract_code_only_cli.py @@ -244,3 +244,17 @@ def test_explicit_exclude_replaces_persisted_setting_with_custom_out(tmp_path): assert json.loads((graph_out / ".graphify_build.json").read_text()) == { "excludes": ["generated"] } + + +def test_extract_names_skipped_sensitive_files(tmp_path): + """#2106 traceability: a file dropped by the sensitive-file filter is reported + by NAME (not just a count), so a wrongly-flagged file is visible.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "app.py").write_text("def hello():\n return 1\n") + (repo / "github_token.txt").write_text("ghp_secretvalue\n") # real secret -> skipped + r = _run(repo, "--code-only", "--no-cluster") + assert r.returncode == 0, r.stderr + out = r.stdout + r.stderr + assert "skipped as potentially sensitive" in out + assert "github_token.txt" in out, "the skipped filename must be surfaced (#2106)"