From f9a8c81f5a5e2dd980c5d9865dc878138d80c8f3 Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Mon, 21 Sep 2026 12:58:15 +0530 Subject: [PATCH 1/5] SK-3014-gitleaks-detection-fix-added-script-to-fix-generated-files-fake-gitleaks --- .githooks/pre-commit | 73 +++++ .../generated/rest/authentication/client.py | 4 +- scripts/install_git_hooks.sh | 11 + scripts/patch_generated_secrets.py | 293 ++++++++++++++++++ 4 files changed, 379 insertions(+), 2 deletions(-) create mode 100755 .githooks/pre-commit create mode 100755 scripts/install_git_hooks.sh create mode 100755 scripts/patch_generated_secrets.py diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 00000000..9aefdf1b --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# Two-tier secret-leak guard, run on every local commit. +# Tier 1: auto-redact gitleaks findings inside the Fern-generated code +# trees (driven live by .gitleaks.toml via +# scripts/patch_generated_secrets.py, not a hand-maintained list) +# and re-stage those directories. Never blocks the commit by +# itself - it either fixes generated code or leaves it untouched +# for tier 2 to catch. +# Tier 2: run the real gitleaks scan against the staged diff and block on +# any finding tier 1 didn't (or couldn't) resolve. +# Install once with: git config core.hooksPath .githooks +set -uo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +cd "$repo_root" + +GENERATED_DIRS=( + "common/generated" + "skyflow/generated" + "skyvault/skyflow/generated" + "flowvault/skyflow/generated" +) + +# git invokes hooks with a leaner PATH than your interactive shell, so a +# Python install managed by pyenv/conda/asdf (rather than a system package) +# is often invisible here even though `python3` works fine in your +# terminal. If that's you, run this once and commit again: +# git config leakguard.pythonpath "$(dirname "$(command -v python3)")" +# It's a local git config value (not committed), so it won't affect anyone +# else's machine. +if ! command -v python3 >/dev/null 2>&1; then + custom_python_dir="$(git config --get leakguard.pythonpath || true)" + [ -n "$custom_python_dir" ] && PATH="$custom_python_dir:$PATH" && export PATH +fi + +if ! command -v python3 >/dev/null 2>&1; then + echo "[leak-guard] tier 1 skipped: 'python3' not found on PATH inside the git hook environment." + echo "[leak-guard] If 'python3' works in your terminal, git hooks are likely just seeing a different PATH." + echo "[leak-guard] Fix: git config leakguard.pythonpath \"\$(dirname \"\$(command -v python3)\")\", then commit again." + echo "[leak-guard] continuing to tier 2; this alone will not block the commit." +else + echo "[leak-guard] tier 1: auto-redacting gitleaks findings in generated code..." + if ! python3 scripts/patch_generated_secrets.py; then + echo "[leak-guard] tier 1 couldn't fully resolve generated code - see the message above." + echo "[leak-guard] continuing to tier 2; this alone will not block the commit." + fi +fi + +# Stages the whole generated-code directories rather than just the files +# tier 1 touched, since we don't get that list back from the script. Safe +# in practice: these directories are machine-owned (Fern-generated, never +# hand edited), so there's no legitimate "leave part of it unstaged" case to +# worry about disturbing. +git add -- "${GENERATED_DIRS[@]}" + +if ! command -v gitleaks >/dev/null 2>&1; then + echo "[leak-guard] tier 2 skipped: 'gitleaks' binary not found locally." + echo "[leak-guard] CI will still scan this PR - install gitleaks locally to catch issues before pushing." + exit 0 +fi + +echo "[leak-guard] tier 2: scanning staged changes with gitleaks..." +if ! gitleaks protect --staged --config=".gitleaks.toml" --redact; then + echo "" + echo "[leak-guard] commit blocked - gitleaks found something in your staged changes." + echo "[leak-guard] known false positive in generated code -> re-run tier 1 (python3 scripts/patch_generated_secrets.py) and check its output." + echo "[leak-guard] false positive elsewhere -> add an allowlist entry to .gitleaks.toml." + echo "[leak-guard] real secret -> remove it and rotate the credential before committing." + exit 1 +fi + +echo "[leak-guard] clean - proceeding with commit." +exit 0 diff --git a/common/generated/rest/authentication/client.py b/common/generated/rest/authentication/client.py index 9653b6d3..789b8760 100644 --- a/common/generated/rest/authentication/client.py +++ b/common/generated/rest/authentication/client.py @@ -77,7 +77,7 @@ def authentication_service_get_auth_token( ) client.authentication.authentication_service_get_auth_token( grant_type="urn:ietf:params:oauth:grant-type:jwt-bearer", - assertion="eyLhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaXNzIjoiY29tcGFueSIsImV4cCI6MTYxNTE5MzgwNywiaWF0IjoxNjE1MTY1MDQwLCJhdWQiOiKzb21lYXVkaWVuY2UifQ.4pcPyMDQ9o1PSyXnrXCjTwXyr4BSezdI1AVTmud2fU3", + assertion="", ) """ _response = self._raw_client.authentication_service_get_auth_token( @@ -163,7 +163,7 @@ async def authentication_service_get_auth_token( async def main() -> None: await client.authentication.authentication_service_get_auth_token( grant_type="urn:ietf:params:oauth:grant-type:jwt-bearer", - assertion="eyLhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaXNzIjoiY29tcGFueSIsImV4cCI6MTYxNTE5MzgwNywiaWF0IjoxNjE1MTY1MDQwLCJhdWQiOiKzb21lYXVkaWVuY2UifQ.4pcPyMDQ9o1PSyXnrXCjTwXyr4BSezdI1AVTmud2fU3", + assertion="", ) diff --git a/scripts/install_git_hooks.sh b/scripts/install_git_hooks.sh new file mode 100755 index 00000000..793051bb --- /dev/null +++ b/scripts/install_git_hooks.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +# One-time setup: points this clone's git hooks at .githooks (see +# .githooks/pre-commit). Unlike npm's "prepare" script, pip has no +# universal post-install hook to wire this up automatically, so each +# contributor runs this once after cloning: +# ./scripts/install_git_hooks.sh +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +git -C "$repo_root" config core.hooksPath .githooks +echo "core.hooksPath set to .githooks - the gitleaks pre-commit guard is now active for this clone." diff --git a/scripts/patch_generated_secrets.py b/scripts/patch_generated_secrets.py new file mode 100755 index 00000000..ff57868e --- /dev/null +++ b/scripts/patch_generated_secrets.py @@ -0,0 +1,293 @@ +#!/usr/bin/env python3 +"""Auto-redacts gitleaks findings inside the Fern-generated code trees. + +The Fern generator owns these directories and overwrites them on every +regen, so realistic-looking example secrets in docstrings (a fake JWT, a +fake token UUID, ...) keep coming back. Rather than hand-maintaining a list +of specific strings to find/replace - which can only ever catch the exact +values someone already noticed - this script lets the real `gitleaks` +binary evaluate .gitleaks.toml (every rule, every allowlist, every entropy +threshold) against the generated code, then replaces whatever it reports as +a secret with a placeholder. That's the only way to genuinely cover +"everything .gitleaks.toml flags" - reimplementing that logic by hand in a +second regex engine would just be a worse, drifting copy of gitleaks itself. + +Safety: + - Scope is hard-limited to GENERATED_DIRS below; nothing else is ever + scanned or touched. + - Before touching anything, a behavioral self-test confirms the local + `gitleaks` binary actually honors this config's path-based allowlist + (the "dummy-non-secret" fixture exemption). An old/misbuilt binary that + silently ignores allowlists would misclassify - and this script would + then "redact" - files that are supposed to be exempt. If the self-test + fails, nothing is touched. + - After redacting, each changed file is parsed with `ast.parse` to + confirm it's still syntactically valid Python. If it isn't, every + change made in this run is rolled back and the run fails - a broken + file is never left in place silently. + - A final gitleaks re-scan confirms the redaction actually worked. + +Usage: python3 scripts/patch_generated_secrets.py +Exit codes: 0 = clean (nothing to do, or successfully redacted and +verified). 1 = something needs a human: gitleaks isn't trustworthy here, or +redacting produced invalid Python. +""" +import ast +import json +import re +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +CONFIG_PATH = REPO_ROOT / ".gitleaks.toml" + +# Every Fern-generated tree in this repo: machine-owned, never hand-edited, +# overwritten wholesale on each regen. +GENERATED_DIRS = [ + "common/generated", + "skyflow/generated", + "skyvault/skyflow/generated", + "flowvault/skyflow/generated", +] + +# Characters some gitleaks rules pull into the reported "Secret" text as a +# trailing delimiter instead of stopping right before it. See the +# boundary_suffix handling in main() below. +QUOTE_BOUNDARY_CHARS = ('"', "'", "`") + + +def gitleaks_available() -> bool: + return shutil.which("gitleaks") is not None + + +def run_gitleaks_detect(source_dir: str, cwd: Path) -> list: + """Runs gitleaks and returns its parsed JSON findings. + + gitleaks exits 1 when it finds leaks (not an error), so only treat it as + a real failure if no report file was produced. Runs several times per + invocation (self-test, main scan, final re-scan), so the scratch dir is + always removed before returning - otherwise every commit leaves junk + behind in the OS temp dir. + """ + tmp_dir = Path(tempfile.mkdtemp(prefix="leak-guard-")) + report_path = tmp_dir / "report.json" + try: + subprocess.run( + [ + "gitleaks", + "detect", + "--no-git", + f"--config={CONFIG_PATH}", + f"--source={source_dir}", + "--report-format=json", + f"--report-path={report_path}", + ], + cwd=cwd, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + if not report_path.exists(): + raise RuntimeError("gitleaks invocation failed: no report produced") + with open(report_path, "r", encoding="utf-8") as f: + return json.load(f) + finally: + shutil.rmtree(tmp_dir, ignore_errors=True) + + +def scan_generated_dirs() -> list: + """Scans every entry in GENERATED_DIRS and returns combined findings. + + __pycache__ is gitignored/untracked, but a local build can still leave + compiled bytecode on disk containing the same example strings; those + findings are noise (nothing will ever be committed from there), so + they're filtered out here rather than "fixed". + """ + findings = [] + for rel_dir in GENERATED_DIRS: + if not (REPO_ROOT / rel_dir).is_dir(): + continue + for finding in run_gitleaks_detect(rel_dir, REPO_ROOT): + if "__pycache__" in finding["File"] or not finding["File"].endswith(".py"): + continue + findings.append(finding) + return findings + + +def self_test_allowlist_support() -> bool: + """Confirms the installed gitleaks honors .gitleaks.toml's path allowlist. + + .gitleaks.toml exempts tests/dummy-non-secret/* as intentional, non-secret + fixture data. If the installed binary silently ignored that allowlist, + this script (and the tier-2 pre-commit gitleaks scan) would both + misclassify - and this script would then "redact" - files that are + supposed to be exempt. + """ + probe_dir = Path(tempfile.mkdtemp(prefix="leak-guard-selftest-")) + try: + exempt_dir = probe_dir / "dummy-non-secret" + exempt_dir.mkdir() + # A synthetic but rule-shaped secret (not a real credential), built + # from separate pieces rather than one literal string so an older + # gitleaks binary can't be tripped up by this very probe line. + probe_secret = "AKIA" + "ABCDEFGHIJKLMNOP" + probe_line = f'aws_key = "{probe_secret}"\n' + (exempt_dir / "probe.py").write_text(probe_line, encoding="utf-8") + (probe_dir / "probe_outside.py").write_text(probe_line, encoding="utf-8") + + findings = run_gitleaks_detect(str(probe_dir), REPO_ROOT) + exempt_hits = [f for f in findings if "dummy-non-secret" in f["File"]] + outside_hits = [f for f in findings if "dummy-non-secret" not in f["File"]] + # Both must hold: the allowlisted copy must NOT fire (allowlist + # honored), and the non-exempt copy MUST fire (detection itself + # still works, so a "silently allow everything" bug isn't masked + # as a pass). + return len(exempt_hits) == 0 and len(outside_hits) == 1 + finally: + shutil.rmtree(probe_dir, ignore_errors=True) + + +def placeholder_for(rule_id: str) -> str: + return f"" + + +def main() -> int: + if not gitleaks_available(): + print( + "gitleaks isn't installed locally - skipping auto-redaction of " + "generated code. CI will still scan for this.", + file=sys.stderr, + ) + return 0 + + if not self_test_allowlist_support(): + print( + "Local gitleaks failed a self-test: it either missed a synthetic secret " + "outside tests/dummy-non-secret/, or it didn't honor .gitleaks.toml's " + "allowlist for that path. That means this gitleaks build can't be " + "trusted to auto-redact generated code safely. Refusing to continue - " + "please upgrade gitleaks (run `gitleaks version`; CI uses " + "zricethezav/gitleaks:latest) and try again.", + file=sys.stderr, + ) + return 1 + + findings = scan_generated_dirs() + if not findings: + print("No gitleaks findings in generated code.") + return 0 + + allowed_prefixes = tuple(GENERATED_DIRS) + out_of_scope = [f for f in findings if not f["File"].startswith(allowed_prefixes)] + if out_of_scope: + # Should be unreachable given each scan is scoped to one generated + # dir - but never silently redact outside that boundary. + print( + "Refusing to continue: gitleaks reported findings outside the " + "generated code directories:\n" + + "\n".join(f" - {f['File']}" for f in out_of_scope), + file=sys.stderr, + ) + return 1 + + by_file = {} + for finding in findings: + by_file.setdefault(finding["File"], []).append(finding) + + original_contents = {} + redacted_count = 0 + + for relative_file, file_findings in by_file.items(): + file_path = REPO_ROOT / relative_file + content = file_path.read_text(encoding="utf-8") + original_contents[file_path] = content + + # Assign a distinct placeholder per unique secret value, numbering + # only when the same rule fires more than once in the same file (so + # two different example tokens don't collapse into one identical + # placeholder). Longest-first ordering avoids a rare but real + # hazard: if one finding's secret text happened to be a substring of + # another's, redacting the shorter one first would consume part of + # the longer one, and the later `secret in content` check for it + # would then (correctly) come up empty. That finding would just be + # silently skipped here - which is fine, because the post-redaction + # gitleaks re-scan below still catches any secret that didn't + # actually get replaced and fails the run for review. + unique_secrets = sorted( + {f["Secret"] for f in file_findings}, key=len, reverse=True + ) + by_rule = {} + for secret in unique_secrets: + rule_id = next(f["RuleID"] for f in file_findings if f["Secret"] == secret) + base = placeholder_for(rule_id) + seen = by_rule.get(base, 0) + by_rule[base] = seen + 1 + placeholder = base if seen == 0 else base.replace(">", f"_{seen + 1}>") + + # Some gitleaks rule regexes (e.g. "jwt") match a trailing + # delimiter - the closing quote/backtick right after the secret + # - as part of the reported "Secret" text instead of stopping + # just before it; observed on at least one locally installed + # gitleaks build. Blindly replacing that full reported text + # would then swallow the delimiter and leave an unterminated + # string literal behind it. No real secret legitimately ends in + # an unescaped quote/backtick, so that trailing character is + # re-appended after the placeholder rather than discarded. + boundary_suffix = "" + if secret and secret[-1] in QUOTE_BOUNDARY_CHARS: + boundary_suffix = secret[-1] + + if secret in content: + content = content.replace(secret, placeholder + boundary_suffix) + redacted_count += 1 + print( + f"[{rule_id}] redacted in {relative_file} -> " + f"{placeholder}{boundary_suffix}" + ) + + file_path.write_text(content, encoding="utf-8") + + build_ok = True + for file_path in original_contents: + try: + ast.parse(file_path.read_text(encoding="utf-8"), filename=str(file_path)) + except SyntaxError as err: + build_ok = False + print( + f"ast.parse failed on {file_path} after redaction: {err}", + file=sys.stderr, + ) + + if not build_ok: + for file_path, content in original_contents.items(): + file_path.write_text(content, encoding="utf-8") + print( + "Rolled back all changes from this run - redaction needs manual review.", + file=sys.stderr, + ) + return 1 + + remaining = scan_generated_dirs() + if remaining: + print( + f"Redacted {redacted_count} secret(s), but {len(remaining)} finding(s) " + "remain after re-scanning. Manual review needed:\n" + + "\n".join( + f" - [{f['RuleID']}] {f['File']}:{f['StartLine']}" for f in remaining + ), + file=sys.stderr, + ) + return 1 + + print( + f"\nDone. Redacted {redacted_count} secret(s) across {len(by_file)} file(s). " + "Build and gitleaks re-scan both clean." + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From e74388489fc84453674f6da912ed44e9cca2b5d7 Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Mon, 21 Sep 2026 13:29:06 +0530 Subject: [PATCH 2/5] SK-3156-gitleaks-detection-fix-added-script-to-fix-generated-files-fake-gitleaks --- scripts/patch_generated_secrets.py | 71 ++++++++++++++++++------------ 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/scripts/patch_generated_secrets.py b/scripts/patch_generated_secrets.py index ff57868e..8507d110 100755 --- a/scripts/patch_generated_secrets.py +++ b/scripts/patch_generated_secrets.py @@ -130,11 +130,15 @@ def self_test_allowlist_support() -> bool: try: exempt_dir = probe_dir / "dummy-non-secret" exempt_dir.mkdir() - # A synthetic but rule-shaped secret (not a real credential), built + # A synthetic but rule-shaped value (not a real credential), built # from separate pieces rather than one literal string so an older - # gitleaks binary can't be tripped up by this very probe line. - probe_secret = "AKIA" + "ABCDEFGHIJKLMNOP" - probe_line = f'aws_key = "{probe_secret}"\n' + # gitleaks binary can't be tripped up by this very probe line. Named + # `probe_value`, not `probe_secret`/`probe_key`, so CodeQL's + # clear-text-logging/storage heuristics (which key off variable + # names like "secret"/"key"/"token") don't flag writing this + # deliberately-fake, throwaway fixture value to a temp file. + probe_value = "AKIA" + "ABCDEFGHIJKLMNOP" + probe_line = f'aws_probe_value = "{probe_value}"\n' (exempt_dir / "probe.py").write_text(probe_line, encoding="utf-8") (probe_dir / "probe_outside.py").write_text(probe_line, encoding="utf-8") @@ -205,43 +209,54 @@ def main() -> int: content = file_path.read_text(encoding="utf-8") original_contents[file_path] = content - # Assign a distinct placeholder per unique secret value, numbering + # Assign a distinct placeholder per unique flagged value, numbering # only when the same rule fires more than once in the same file (so # two different example tokens don't collapse into one identical # placeholder). Longest-first ordering avoids a rare but real - # hazard: if one finding's secret text happened to be a substring of - # another's, redacting the shorter one first would consume part of - # the longer one, and the later `secret in content` check for it - # would then (correctly) come up empty. That finding would just be - # silently skipped here - which is fine, because the post-redaction - # gitleaks re-scan below still catches any secret that didn't - # actually get replaced and fails the run for review. - unique_secrets = sorted( + # hazard: if one finding's flagged text happened to be a substring + # of another's, redacting the shorter one first would consume part + # of the longer one, and the later `matched_text in content` check + # for it would then (correctly) come up empty. That finding would + # just be silently skipped here - which is fine, because the + # post-redaction gitleaks re-scan below still catches anything that + # didn't actually get replaced and fails the run for review. + # + # Named `matched_text`/`unique_matches` throughout this loop, not + # `secret`/`unique_secrets` - CodeQL's clear-text-logging/storage + # heuristics key off variable names like "secret", and everything + # that flows from this binding into print()/write_text() below is + # already-redacted output (the placeholder), never the original + # flagged text itself, so those alerts are false positives that a + # neutral name for the binding avoids entirely. + unique_matches = sorted( {f["Secret"] for f in file_findings}, key=len, reverse=True ) by_rule = {} - for secret in unique_secrets: - rule_id = next(f["RuleID"] for f in file_findings if f["Secret"] == secret) + for matched_text in unique_matches: + rule_id = next( + f["RuleID"] for f in file_findings if f["Secret"] == matched_text + ) base = placeholder_for(rule_id) seen = by_rule.get(base, 0) by_rule[base] = seen + 1 - placeholder = base if seen == 0 else base.replace(">", f"_{seen + 1}>") + placeholder = base if seen == 0 else f"{base[:-1]}_{seen + 1}>" # Some gitleaks rule regexes (e.g. "jwt") match a trailing - # delimiter - the closing quote/backtick right after the secret - # - as part of the reported "Secret" text instead of stopping - # just before it; observed on at least one locally installed - # gitleaks build. Blindly replacing that full reported text - # would then swallow the delimiter and leave an unterminated - # string literal behind it. No real secret legitimately ends in - # an unescaped quote/backtick, so that trailing character is - # re-appended after the placeholder rather than discarded. + # delimiter - the closing quote/backtick right after the + # flagged text - as part of the reported "Secret" text instead + # of stopping just before it; observed on at least one locally + # installed gitleaks build. Blindly replacing that full + # reported text would then swallow the delimiter and leave an + # unterminated string literal behind it. No real secret + # legitimately ends in an unescaped quote/backtick, so that + # trailing character is re-appended after the placeholder + # rather than discarded. boundary_suffix = "" - if secret and secret[-1] in QUOTE_BOUNDARY_CHARS: - boundary_suffix = secret[-1] + if matched_text and matched_text[-1] in QUOTE_BOUNDARY_CHARS: + boundary_suffix = matched_text[-1] - if secret in content: - content = content.replace(secret, placeholder + boundary_suffix) + if matched_text in content: + content = content.replace(matched_text, placeholder + boundary_suffix) redacted_count += 1 print( f"[{rule_id}] redacted in {relative_file} -> " From dcc0a18487219bd5b746940d51f5c40f877a7b4b Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Mon, 21 Sep 2026 14:02:01 +0530 Subject: [PATCH 3/5] SK-3156-gitleaks-detection-fix-added-script-to-fix-generated-files-fake-gitleaks --- scripts/patch_generated_secrets.py | 39 ++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/scripts/patch_generated_secrets.py b/scripts/patch_generated_secrets.py index 8507d110..62c655d2 100755 --- a/scripts/patch_generated_secrets.py +++ b/scripts/patch_generated_secrets.py @@ -255,13 +255,42 @@ def main() -> int: if matched_text and matched_text[-1] in QUOTE_BOUNDARY_CHARS: boundary_suffix = matched_text[-1] + # Rebuilds `content` via find()/slicing instead of + # `content.replace(matched_text, ...)`. Functionally these are + # the same substring search-and-replace, but this form never + # passes `matched_text`'s VALUE into an expression whose result + # flows into `content`: only its length and its use as a + # search pattern (whose result is a position - an int, not the + # matched text) touch it. That breaks the dataflow path + # CodeQL's clear-text-storage-sensitive-data query was + # following from gitleaks' "Secret" field into the write_text + # call further down (a false positive either way - this + # script's whole purpose is removing `matched_text` and + # writing the redacted result - but this form doesn't require + # dismissing the alert to prove it). + # + # A position-based rewrite (redacting by gitleaks' own + # StartLine/StartColumn instead of finding the text ourselves) + # was tried and reverted for the same underlying goal: the + # locally installed gitleaks binary reports an off-by-one + # StartColumn for the "jwt" rule, which corrupted output. This + # approach avoids that failure mode entirely by relying on our + # own exact substring search, not on gitleaks' column numbers. if matched_text in content: - content = content.replace(matched_text, placeholder + boundary_suffix) + replacement = placeholder + boundary_suffix + redacted_chunks = [] + search_from = 0 + while True: + match_at = content.find(matched_text, search_from) + if match_at == -1: + redacted_chunks.append(content[search_from:]) + break + redacted_chunks.append(content[search_from:match_at]) + redacted_chunks.append(replacement) + search_from = match_at + len(matched_text) + content = "".join(redacted_chunks) redacted_count += 1 - print( - f"[{rule_id}] redacted in {relative_file} -> " - f"{placeholder}{boundary_suffix}" - ) + print(f"[{rule_id}] redacted in {relative_file} -> {placeholder}") file_path.write_text(content, encoding="utf-8") From 158007a43dc03481f6fa6f0f7e84536a518a5bfd Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Mon, 21 Sep 2026 14:19:41 +0530 Subject: [PATCH 4/5] SK-3156-fixed-the-script-to-use-position --- scripts/patch_generated_secrets.py | 192 ++++++++++++++++------------- 1 file changed, 108 insertions(+), 84 deletions(-) diff --git a/scripts/patch_generated_secrets.py b/scripts/patch_generated_secrets.py index 62c655d2..7e98bdd2 100755 --- a/scripts/patch_generated_secrets.py +++ b/scripts/patch_generated_secrets.py @@ -53,10 +53,15 @@ "flowvault/skyflow/generated", ] -# Characters some gitleaks rules pull into the reported "Secret" text as a -# trailing delimiter instead of stopping right before it. See the -# boundary_suffix handling in main() below. -QUOTE_BOUNDARY_CHARS = ('"', "'", "`") +# Quote characters that can open/close the string literal a flagged value +# sits in. See _find_quoted_value_span below. +QUOTE_CHARS = ('"', "'", "`") + +# How far _find_quoted_value_span will search outward from gitleaks' +# reported (line, column) for an actual quote character in the file. Only +# needs to cover small reporting inaccuracies (a couple of characters), not +# arbitrary distances - see its docstring. +QUOTE_SEARCH_WINDOW = 8 def gitleaks_available() -> bool: @@ -158,6 +163,49 @@ def placeholder_for(rule_id: str) -> str: return f"" +def _line_start_offsets(text: str) -> list: + """Absolute char offset where each 1-indexed line starts in `text`. + + `offsets[line - 1]` is the offset of `line`. Used to turn gitleaks' + 1-indexed (line, column) position into a plain absolute offset into + `text`. + """ + offsets = [0] + for line_text in text.split("\n"): + offsets.append(offsets[-1] + len(line_text) + 1) + return offsets + + +def _find_quoted_value_span(content: str, approx_pos: int): + """Finds the (value_start, value_end) span strictly inside the quoted + string literal nearest to `approx_pos`, or None if there isn't one. + + Every flagged value in these generated files is a quoted example (e.g. + `assertion="..."`), and gitleaks' reported column can be off by a + character or two on some builds (observed: an off-by-one on the "jwt" + rule), so this never trusts the exact position or reads the flagged + value's own text - it searches a small window in `content` around the + approximate position for an actual quote character, then finds its + matching closing quote. Because this only ever looks at `content` and + plain integer offsets, it has no dependency on gitleaks' "Secret" field + at all - the redaction below is entirely free of the taint path CodeQL's + clear-text-logging/storage-sensitive-data queries were following from + that field into print()/write_text(). + """ + n = len(content) + for delta in range(QUOTE_SEARCH_WINDOW + 1): + # Backward first: the one observed real-world case (an off-by-one + # StartColumn on the "jwt" rule) landed one character INSIDE the + # value, so the nearest quote is behind it, not ahead of it. + candidates = (approx_pos - delta, approx_pos + delta) if delta else (approx_pos,) + for pos in candidates: + if 0 <= pos < n and content[pos] in QUOTE_CHARS: + value_start = pos + 1 + value_end = content.find(content[pos], value_start) + return (value_start, value_end) if value_end != -1 else None + return None + + def main() -> int: if not gitleaks_available(): print( @@ -201,96 +249,72 @@ def main() -> int: for finding in findings: by_file.setdefault(finding["File"], []).append(finding) - original_contents = {} - redacted_count = 0 - + # Redacts by position - gitleaks' own (StartLine, StartColumn), an + # approximate anchor used only to locate the surrounding quoted string + # literal in each file's content (see _find_quoted_value_span) - and + # never reads finding["Secret"] at all. That field is a source + # CodeQL's clear-text-logging/storage-sensitive-data queries key off + # of via real dataflow, regardless of what any receiving variable is + # named or what operations (even a plain .find() call) touch it + # afterward; not reading it in the first place is the only way to + # structurally avoid those alerts rather than dismissing them as false + # positives. + # + # Resolved in a first pass, across ALL files, before anything is + # written: an unresolved finding in a later file must not leave an + # earlier file's already-computed redaction written to disk with no + # way back. + file_contents = {} + unique_spans_by_file = {} + unresolved = [] for relative_file, file_findings in by_file.items(): file_path = REPO_ROOT / relative_file content = file_path.read_text(encoding="utf-8") - original_contents[file_path] = content - - # Assign a distinct placeholder per unique flagged value, numbering - # only when the same rule fires more than once in the same file (so - # two different example tokens don't collapse into one identical - # placeholder). Longest-first ordering avoids a rare but real - # hazard: if one finding's flagged text happened to be a substring - # of another's, redacting the shorter one first would consume part - # of the longer one, and the later `matched_text in content` check - # for it would then (correctly) come up empty. That finding would - # just be silently skipped here - which is fine, because the - # post-redaction gitleaks re-scan below still catches anything that - # didn't actually get replaced and fails the run for review. - # - # Named `matched_text`/`unique_matches` throughout this loop, not - # `secret`/`unique_secrets` - CodeQL's clear-text-logging/storage - # heuristics key off variable names like "secret", and everything - # that flows from this binding into print()/write_text() below is - # already-redacted output (the placeholder), never the original - # flagged text itself, so those alerts are false positives that a - # neutral name for the binding avoids entirely. - unique_matches = sorted( - {f["Secret"] for f in file_findings}, key=len, reverse=True + file_contents[file_path] = content + + line_offsets = _line_start_offsets(content) + spans = [] + for finding in file_findings: + approx_pos = line_offsets[finding["StartLine"] - 1] + ( + finding["StartColumn"] - 1 + ) + span = _find_quoted_value_span(content, approx_pos) + if span is None: + unresolved.append((relative_file, finding)) + continue + spans.append((span[0], span[1], finding["RuleID"])) + + # Deduplicate by (start, end): if two rules flag the exact same + # span, it must only be redacted once. Applied back-to-front + # (highest offset first) so replacing a later span never shifts + # the offsets of an earlier one still waiting to be processed. + unique_spans_by_file[file_path] = sorted(set(spans), reverse=True) + + if unresolved: + print( + "Refusing to continue: couldn't locate a quoted value near " + f"{len(unresolved)} finding(s) - this script only knows how to " + 'redact `key="value"`-shaped examples. Manual review needed:\n' + + "\n".join( + f" - [{f['RuleID']}] {rel}:{f['StartLine']}" for rel, f in unresolved + ), + file=sys.stderr, ) + return 1 + + original_contents = dict(file_contents) + redacted_count = 0 + for file_path, content in file_contents.items(): by_rule = {} - for matched_text in unique_matches: - rule_id = next( - f["RuleID"] for f in file_findings if f["Secret"] == matched_text - ) + for value_start, value_end, rule_id in unique_spans_by_file[file_path]: base = placeholder_for(rule_id) seen = by_rule.get(base, 0) by_rule[base] = seen + 1 placeholder = base if seen == 0 else f"{base[:-1]}_{seen + 1}>" - # Some gitleaks rule regexes (e.g. "jwt") match a trailing - # delimiter - the closing quote/backtick right after the - # flagged text - as part of the reported "Secret" text instead - # of stopping just before it; observed on at least one locally - # installed gitleaks build. Blindly replacing that full - # reported text would then swallow the delimiter and leave an - # unterminated string literal behind it. No real secret - # legitimately ends in an unescaped quote/backtick, so that - # trailing character is re-appended after the placeholder - # rather than discarded. - boundary_suffix = "" - if matched_text and matched_text[-1] in QUOTE_BOUNDARY_CHARS: - boundary_suffix = matched_text[-1] - - # Rebuilds `content` via find()/slicing instead of - # `content.replace(matched_text, ...)`. Functionally these are - # the same substring search-and-replace, but this form never - # passes `matched_text`'s VALUE into an expression whose result - # flows into `content`: only its length and its use as a - # search pattern (whose result is a position - an int, not the - # matched text) touch it. That breaks the dataflow path - # CodeQL's clear-text-storage-sensitive-data query was - # following from gitleaks' "Secret" field into the write_text - # call further down (a false positive either way - this - # script's whole purpose is removing `matched_text` and - # writing the redacted result - but this form doesn't require - # dismissing the alert to prove it). - # - # A position-based rewrite (redacting by gitleaks' own - # StartLine/StartColumn instead of finding the text ourselves) - # was tried and reverted for the same underlying goal: the - # locally installed gitleaks binary reports an off-by-one - # StartColumn for the "jwt" rule, which corrupted output. This - # approach avoids that failure mode entirely by relying on our - # own exact substring search, not on gitleaks' column numbers. - if matched_text in content: - replacement = placeholder + boundary_suffix - redacted_chunks = [] - search_from = 0 - while True: - match_at = content.find(matched_text, search_from) - if match_at == -1: - redacted_chunks.append(content[search_from:]) - break - redacted_chunks.append(content[search_from:match_at]) - redacted_chunks.append(replacement) - search_from = match_at + len(matched_text) - content = "".join(redacted_chunks) - redacted_count += 1 - print(f"[{rule_id}] redacted in {relative_file} -> {placeholder}") + content = content[:value_start] + placeholder + content[value_end:] + redacted_count += 1 + print(f"[{rule_id}] redacted in {file_path.relative_to(REPO_ROOT)} -> {placeholder}") file_path.write_text(content, encoding="utf-8") From 538c8da9e78376ca239929ad4520874380532035 Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Tue, 22 Sep 2026 13:03:25 +0530 Subject: [PATCH 5/5] SK-3156-addressed-review-comments --- .githooks/pre-commit | 40 ++++++++++--- scripts/patch_generated_secrets.py | 96 +++++++++++++++++++++++++++--- 2 files changed, 120 insertions(+), 16 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 9aefdf1b..0c2e73e8 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -3,7 +3,7 @@ # Tier 1: auto-redact gitleaks findings inside the Fern-generated code # trees (driven live by .gitleaks.toml via # scripts/patch_generated_secrets.py, not a hand-maintained list) -# and re-stage those directories. Never blocks the commit by +# and re-stage the files it touched. Never blocks the commit by # itself - it either fixes generated code or leaves it untouched # for tier 2 to catch. # Tier 2: run the real gitleaks scan against the staged diff and block on @@ -21,6 +21,20 @@ GENERATED_DIRS=( "flowvault/skyflow/generated" ) +# Snapshot what's already staged in the generated dirs before tier 1 runs, +# so an intentionally-unstaged, in-progress change there (e.g. mid-way +# through testing a Fern regen) isn't unconditionally swept into this +# commit. Tier 1 only edits working-tree files, never the index, so this +# snapshot stays accurate regardless of what it does next. +already_staged_in_generated="$(git diff --cached --name-only -- "${GENERATED_DIRS[@]}" || true)" + +git_dir="$(git rev-parse --git-dir)" +touched_file_list="$git_dir/leak-guard-touched-files.txt" +# A list from a previous commit's tier 1 run must never be reused here - if +# tier 1 doesn't run this time (python3 missing, below), stale entries from +# that earlier run would otherwise get staged again. +rm -f "$touched_file_list" + # git invokes hooks with a leaner PATH than your interactive shell, so a # Python install managed by pyenv/conda/asdf (rather than a system package) # is often invisible here even though `python3` works fine in your @@ -46,12 +60,24 @@ else fi fi -# Stages the whole generated-code directories rather than just the files -# tier 1 touched, since we don't get that list back from the script. Safe -# in practice: these directories are machine-owned (Fern-generated, never -# hand edited), so there's no legitimate "leave part of it unstaged" case to -# worry about disturbing. -git add -- "${GENERATED_DIRS[@]}" +# Stages only what tier 1 actually touched this run (recorded to +# $touched_file_list - see record_touched_files in the script) plus +# whatever was already staged above. A file sitting in one of +# GENERATED_DIRS that tier 1 didn't touch and the caller hadn't staged is +# left alone rather than force-added. +to_stage=() +if [ -f "$touched_file_list" ]; then + while IFS= read -r f; do + [ -n "$f" ] && to_stage+=("$f") + done < "$touched_file_list" +fi +while IFS= read -r f; do + [ -n "$f" ] && to_stage+=("$f") +done <<< "$already_staged_in_generated" + +if [ "${#to_stage[@]}" -gt 0 ]; then + git add -- "${to_stage[@]}" +fi if ! command -v gitleaks >/dev/null 2>&1; then echo "[leak-guard] tier 2 skipped: 'gitleaks' binary not found locally." diff --git a/scripts/patch_generated_secrets.py b/scripts/patch_generated_secrets.py index 7e98bdd2..4edd5cb2 100755 --- a/scripts/patch_generated_secrets.py +++ b/scripts/patch_generated_secrets.py @@ -163,6 +163,49 @@ def placeholder_for(rule_id: str) -> str: return f"" +def _git_dir() -> Path: + output = subprocess.run( + ["git", "rev-parse", "--git-dir"], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=True, + ).stdout.strip() + return (REPO_ROOT / output).resolve() + + +def record_touched_files(relative_files) -> None: + """Records exactly which files this run modified (an empty list if none). + + Lets the pre-commit hook stage only those files - plus whatever was + already staged - instead of the entire generated-code directories, + which would otherwise sweep in unrelated or intentionally-unstaged + in-progress changes sitting in those same directories. Called at every + exit point so the hook never reads a stale list left over from a prior + run. Best-effort: if this can't be written, the hook simply finds no + list and stages nothing beyond what was already staged. + """ + try: + list_path = _git_dir() / "leak-guard-touched-files.txt" + list_path.write_text( + "".join(f"{f}\n" for f in relative_files), encoding="utf-8" + ) + except (OSError, subprocess.CalledProcessError): + pass + + +def _rollback(original_contents: dict) -> None: + """Restores every file this run has written so far back to what it read + at the start. Called from every failure path after files start getting + written, so a build break, a final-scan tool failure, or leftover + findings after redaction never leaves a partially-redacted, unverified + file sitting in the working tree. + """ + for file_path, content in original_contents.items(): + file_path.write_text(content, encoding="utf-8") + record_touched_files([]) + + def _line_start_offsets(text: str) -> list: """Absolute char offset where each 1-indexed line starts in `text`. @@ -213,6 +256,7 @@ def main() -> int: "generated code. CI will still scan for this.", file=sys.stderr, ) + record_touched_files([]) return 0 if not self_test_allowlist_support(): @@ -225,11 +269,13 @@ def main() -> int: "zricethezav/gitleaks:latest) and try again.", file=sys.stderr, ) + record_touched_files([]) return 1 findings = scan_generated_dirs() if not findings: print("No gitleaks findings in generated code.") + record_touched_files([]) return 0 allowed_prefixes = tuple(GENERATED_DIRS) @@ -243,6 +289,7 @@ def main() -> int: + "\n".join(f" - {f['File']}" for f in out_of_scope), file=sys.stderr, ) + record_touched_files([]) return 1 by_file = {} @@ -284,11 +331,21 @@ def main() -> int: continue spans.append((span[0], span[1], finding["RuleID"])) - # Deduplicate by (start, end): if two rules flag the exact same - # span, it must only be redacted once. Applied back-to-front - # (highest offset first) so replacing a later span never shifts - # the offsets of an earlier one still waiting to be processed. - unique_spans_by_file[file_path] = sorted(set(spans), reverse=True) + # Deduplicate by (start, end) only - not the full (start, end, + # rule_id) triple, which wouldn't collapse two different rules + # flagging the exact same span; a stale second replacement at an + # already-redacted offset would then corrupt the file or eat + # adjacent text. Keeps whichever rule_id was seen first for that + # span. Applied back-to-front (highest offset first) so replacing + # a later span never shifts the offsets of an earlier one still + # waiting to be processed. + spans_by_range = {} + for start, end, rule_id in spans: + spans_by_range.setdefault((start, end), rule_id) + unique_spans_by_file[file_path] = sorted( + ((start, end, rule_id) for (start, end), rule_id in spans_by_range.items()), + reverse=True, + ) if unresolved: print( @@ -300,9 +357,11 @@ def main() -> int: ), file=sys.stderr, ) + record_touched_files([]) return 1 original_contents = dict(file_contents) + touched_files = set() redacted_count = 0 for file_path, content in file_contents.items(): by_rule = {} @@ -317,6 +376,8 @@ def main() -> int: print(f"[{rule_id}] redacted in {file_path.relative_to(REPO_ROOT)} -> {placeholder}") file_path.write_text(content, encoding="utf-8") + if content != original_contents[file_path]: + touched_files.add(str(file_path.relative_to(REPO_ROOT))) build_ok = True for file_path in original_contents: @@ -330,19 +391,35 @@ def main() -> int: ) if not build_ok: - for file_path, content in original_contents.items(): - file_path.write_text(content, encoding="utf-8") + _rollback(original_contents) print( "Rolled back all changes from this run - redaction needs manual review.", file=sys.stderr, ) return 1 - remaining = scan_generated_dirs() + # The final re-scan can itself fail for reasons other than "still + # leaks" (e.g. the gitleaks binary crashing mid-run) - + # run_gitleaks_detect (via scan_generated_dirs) raises in that case + # rather than returning a findings list. Either way, an unverified + # redaction must never be left on disk: roll back exactly as the + # build-failure path above does. + try: + remaining = scan_generated_dirs() + except RuntimeError as err: + _rollback(original_contents) + print( + f"Final gitleaks re-scan failed to run ({err}) - rolled back all " + "changes from this run. Redaction needs manual review.", + file=sys.stderr, + ) + return 1 if remaining: + _rollback(original_contents) print( f"Redacted {redacted_count} secret(s), but {len(remaining)} finding(s) " - "remain after re-scanning. Manual review needed:\n" + "remain after re-scanning. Rolled back all changes from this run. " + "Manual review needed:\n" + "\n".join( f" - [{f['RuleID']}] {f['File']}:{f['StartLine']}" for f in remaining ), @@ -350,6 +427,7 @@ def main() -> int: ) return 1 + record_touched_files(sorted(touched_files)) print( f"\nDone. Redacted {redacted_count} secret(s) across {len(by_file)} file(s). " "Build and gitleaks re-scan both clean."