From 6d417f898a9f94058e1fa2085a7938e718d2efa9 Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Fri, 18 Sep 2026 17:20:39 +0530 Subject: [PATCH 1/4] SK-3018-gitleaks-detection-fix-added-automated-script-to-fix-generated-code --- .githooks/pre-commit | 65 ++++++++ package.json | 4 +- scripts/patch-generated-secrets.js | 259 +++++++++++++++++++++++++++++ 3 files changed, 327 insertions(+), 1 deletion(-) create mode 100755 .githooks/pre-commit create mode 100644 scripts/patch-generated-secrets.js diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 00000000..bcc62f50 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Two-tier secret-leak guard, run on every local commit. +# Tier 1: auto-redact gitleaks findings inside src/_generated_ (driven live +# by Rule/gitleaks.toml via scripts/patch-generated-secrets.js, not +# a hand-maintained list) and re-stage that directory. 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. +# Installed via the repo's own "prepare" npm script - see package.json. +set -uo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +cd "$repo_root" + +# git invokes hooks with a leaner PATH than your interactive shell, so a +# Node install managed by nvm/volta/fnm or bundled with an IDE (rather than +# a system package) is often invisible here even though `node` works fine +# in your terminal. If that's you, run this once and commit again: +# git config leakguard.nodepath "$(dirname "$(command -v node)")" +# It's a local git config value (not committed), so it won't affect anyone +# else's machine. +if ! command -v node >/dev/null 2>&1; then + custom_node_dir="$(git config --get leakguard.nodepath || true)" + [ -n "$custom_node_dir" ] && PATH="$custom_node_dir:$PATH" && export PATH +fi + +if ! command -v node >/dev/null 2>&1; then + echo "[leak-guard] tier 1 skipped: 'node' not found on PATH inside the git hook environment." + echo "[leak-guard] If 'node' works in your terminal, git hooks are likely just seeing a different PATH." + echo "[leak-guard] Fix: git config leakguard.nodepath \"\$(dirname \"\$(command -v node)\")\", 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 ! node scripts/patch-generated-secrets.js; 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 directory rather than just the files tier 1 +# touched, since we don't get that list back from the script. Safe in +# practice: src/_generated_ is machine-owned (Fern-generated, never hand +# edited), so there's no legitimate "leave part of it unstaged" case to worry +# about disturbing. +git add -- "src/ _generated_" + +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="Rule/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 (node scripts/patch-generated-secrets.js) and check its output." + echo "[leak-guard] false positive elsewhere -> add an allowlist entry to Rule/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/package.json b/package.json index 2bb4737f..662f66e4 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,9 @@ "lint": "npm run eslint", "lint-fix": "prettier --write '**/*.{js,ts}' && eslint --fix '**/*.{js,ts}'", "spellcheck": "cspell '**/*.{ts,js,md}'", - "docs-gen": "typedoc && node scripts/docs-script/markdown-gen.js && npx ts-node scripts/docs-script/processMarkdown.ts" + "docs-gen": "typedoc && node scripts/docs-script/markdown-gen.js && npx ts-node scripts/docs-script/processMarkdown.ts", + "patch-generated-secrets": "node scripts/patch-generated-secrets.js", + "prepare": "git rev-parse --is-inside-work-tree >/dev/null 2>&1 && git config core.hooksPath .githooks || true" }, "repository": { "type": "git", diff --git a/scripts/patch-generated-secrets.js b/scripts/patch-generated-secrets.js new file mode 100644 index 00000000..62bd00f4 --- /dev/null +++ b/scripts/patch-generated-secrets.js @@ -0,0 +1,259 @@ +/** + * Auto-redacts gitleaks findings inside src/_generated_. + * + * The Fern generator owns that directory and overwrites it on every regen, + * so realistic-looking example secrets in JSDoc comments (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 Rule/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" - the config + * has ~170 rules with layered allowlists; 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 src/_generated_ via --source; nothing else + * is ever scanned or touched. + * - Before touching anything, a behavioral self-test confirms the local + * `gitleaks` binary actually honors this config's `regexTarget: "line"` + * allowlists (added in a gitleaks release newer than some OS packages, + * e.g. Ubuntu's apt build - see AGENTS.md discussion). An older binary + * silently ignores that field and would misclassify ordinary code + * (e.g. a plain `import { A, B } from '...'` line) as a leak, which + * this script would then corrupt by "redacting" it. If the self-test + * fails, nothing is touched. + * - After redacting, `tsc --noEmit` verifies the generated code still + * compiles. If it doesn't, every change made in this run is rolled + * back and the run fails - a broken build is never left in place + * silently. + * - A final gitleaks re-scan confirms the redaction actually worked. + * + * Usage: node scripts/patch-generated-secrets.js + * Exit codes: 0 = clean (nothing to do, or successfully redacted and + * verified). 1 = something needs a human: gitleaks isn't trustworthy here, + * or redacting broke the build. + */ +const { execFileSync } = require("child_process"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); + +const repoRoot = path.resolve(__dirname, ".."); +// Note the literal space before "_generated_" - that's the real directory +// name in this repo (not a typo here), so it must match exactly. +const GENERATED_CODE_PREFIX = "src/ _generated_/"; +const CONFIG_PATH = path.join(repoRoot, "Rule", "gitleaks.toml"); + +function gitleaksAvailable() { + try { + execFileSync("gitleaks", ["version"], { stdio: "ignore" }); + return true; + } catch { + return false; + } +} + +// 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. +function runGitleaksDetect(sourceDir, cwd) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "leak-guard-")); + const reportPath = path.join(tmpDir, "report.json"); + try { + try { + execFileSync( + "gitleaks", + [ + "detect", + "--no-git", + `--config=${CONFIG_PATH}`, + `--source=${sourceDir}`, + "--report-format=json", + `--report-path=${reportPath}`, + ], + { cwd, stdio: "ignore" }, + ); + } catch (err) { + if (!fs.existsSync(reportPath)) { + throw new Error(`gitleaks invocation failed: ${err.message}`); + } + } + return JSON.parse(fs.readFileSync(reportPath, "utf8")); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + +// Confirms the installed gitleaks honors regexTarget="line" allowlists, +// which Rule/gitleaks.toml relies on to exempt lines like +// `import { A, B } from '...'` from the generic-api-key rule. Without this, +// an older gitleaks build (e.g. Ubuntu's apt package) would silently +// misclassify - and this script would then "redact" - ordinary code. +function selfTestAllowlistSupport() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "leak-guard-selftest-")); + try { + // Built from separate pieces rather than one literal string: an older + // gitleaks (the exact case this test detects) would otherwise flag this + // line inside this very file, since it lives outside src/_generated_ + // and tier 1's scope doesn't cover it - the probe would trip the bug + // it's meant to catch. + const probeImportLine = [ + "import { V1GetAuthTokenRequest,", + "V1GetAuthTokenResponse } from", + "'../ _generated_/rest/api';\n", + ].join(" "); + fs.writeFileSync(path.join(dir, "probe.ts"), probeImportLine); + return runGitleaksDetect(dir, repoRoot).length === 0; + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +function placeholderFor(ruleID) { + return ``; +} + +if (!gitleaksAvailable()) { + console.warn( + "gitleaks isn't installed locally - skipping auto-redaction of generated code. " + + "CI will still scan for this.", + ); + process.exit(0); +} + +if (!selfTestAllowlistSupport()) { + console.error( + 'Local gitleaks failed a self-test: it flagged an ordinary `import { A, B } from "..."` line ' + + "that Rule/gitleaks.toml explicitly allowlists. That means this gitleaks build silently " + + 'ignores regexTarget="line" allowlists and would misclassify (and corrupt) real code as a ' + + "secret. Refusing to auto-redact - please upgrade gitleaks (run `gitleaks version`; " + + "CI uses zricethezav/gitleaks:latest) and try again.", + ); + process.exit(1); +} + +// Pass a repo-relative source path (with cwd set to repoRoot) rather than an +// absolute one - gitleaks mirrors whichever form --source takes in the +// "File" field of its report, and the scope check below needs it relative. +const findings = runGitleaksDetect( + GENERATED_CODE_PREFIX.replace(/\/$/, ""), + repoRoot, +); + +if (findings.length === 0) { + console.log("No gitleaks findings in generated code."); + process.exit(0); +} + +const outOfScope = findings.filter( + (f) => !f.File.startsWith(GENERATED_CODE_PREFIX), +); +if (outOfScope.length > 0) { + // Should be unreachable given --source is scoped to the generated dir - + // but never silently redact outside that boundary. + console.error( + "Refusing to continue: gitleaks reported findings outside the generated code directory:\n" + + outOfScope.map((f) => ` - ${f.File}`).join("\n"), + ); + process.exit(1); +} + +const byFile = new Map(); +for (const finding of findings) { + if (!byFile.has(finding.File)) byFile.set(finding.File, []); + byFile.get(finding.File).push(finding); +} + +const originalContents = new Map(); +let redactedCount = 0; + +for (const [relativeFile, fileFindings] of byFile) { + const filePath = path.join(repoRoot, relativeFile); + let content = fs.readFileSync(filePath, "utf8"); + originalContents.set(filePath, 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 + // `content.includes(secret)` 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. + const uniqueSecrets = [...new Set(fileFindings.map((f) => f.Secret))].sort( + (a, b) => b.length - a.length, + ); + const byRule = new Map(); + for (const secret of uniqueSecrets) { + const ruleID = fileFindings.find((f) => f.Secret === secret).RuleID; + const base = placeholderFor(ruleID); + const seen = byRule.get(base) || 0; + byRule.set(base, seen + 1); + const placeholder = seen === 0 ? base : base.replace(">", `_${seen + 1}>`); + + if (content.includes(secret)) { + content = content.split(secret).join(placeholder); + redactedCount += 1; + console.log(`[${ruleID}] redacted in ${relativeFile} -> ${placeholder}`); + } + } + + fs.writeFileSync(filePath, content); +} + +const localTsc = path.join(repoRoot, "node_modules", ".bin", "tsc"); +if (!fs.existsSync(localTsc)) { + for (const [filePath, content] of originalContents) { + fs.writeFileSync(filePath, content); + } + console.error( + `${localTsc} not found (run \`npm install\`) - can't verify redaction is safe, so rolling back and refusing to redact.`, + ); + process.exit(1); +} + +let buildOk = true; +try { + execFileSync(localTsc, ["--noEmit"], { cwd: repoRoot, stdio: "pipe" }); +} catch (err) { + buildOk = false; + console.error( + "tsc --noEmit failed after redaction - rolling back all changes from this run.", + ); + console.error(err.stdout ? err.stdout.toString() : err.message); +} + +if (!buildOk) { + for (const [filePath, content] of originalContents) { + fs.writeFileSync(filePath, content); + } + console.error( + "Rolled back. Redaction needs manual review - see the tsc output above.", + ); + process.exit(1); +} + +const remaining = runGitleaksDetect( + GENERATED_CODE_PREFIX.replace(/\/$/, ""), + repoRoot, +); +if (remaining.length > 0) { + console.error( + `Redacted ${redactedCount} secret(s), but ${remaining.length} finding(s) remain after re-scanning. Manual review needed:\n` + + remaining + .map((f) => ` - [${f.RuleID}] ${f.File}:${f.StartLine}`) + .join("\n"), + ); + process.exit(1); +} + +console.log( + `\nDone. Redacted ${redactedCount} secret(s) across ${byFile.size} file(s). Build and gitleaks re-scan both clean.`, +); From 096a98abcdcecfc22ff073936e78993fca61876b Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Mon, 21 Sep 2026 13:28:12 +0530 Subject: [PATCH 2/4] SK-3156-gitleaks-detection-fix-added-script-to-fix-generated-files-fake-gitleaks --- scripts/patch-generated-secrets.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/patch-generated-secrets.js b/scripts/patch-generated-secrets.js index 62bd00f4..a3fa6f4c 100644 --- a/scripts/patch-generated-secrets.js +++ b/scripts/patch-generated-secrets.js @@ -196,7 +196,12 @@ for (const [relativeFile, fileFindings] of byFile) { const base = placeholderFor(ruleID); const seen = byRule.get(base) || 0; byRule.set(base, seen + 1); - const placeholder = seen === 0 ? base : base.replace(">", `_${seen + 1}>`); + // base is always `` (placeholderFor always ends it with + // exactly one ">"), so slice that off and re-append it with the suffix + // rather than using String#replace, which CodeQL flags as an + // incomplete-escaping-style bug (replaces only the first occurrence) + // even though there's only ever one to begin with here. + const placeholder = seen === 0 ? base : `${base.slice(0, -1)}_${seen + 1}>`; if (content.includes(secret)) { content = content.split(secret).join(placeholder); From 9f3bbda3a1d334b6eb3cb22f2476bac444daec0a Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Tue, 22 Sep 2026 12:52:54 +0530 Subject: [PATCH 3/4] SK-3156-addressed-review-comments --- .githooks/pre-commit | 47 +++++-- scripts/patch-generated-secrets.js | 215 ++++++++++++++++++++++++----- 2 files changed, 221 insertions(+), 41 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index bcc62f50..3eb986e1 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -2,9 +2,9 @@ # Two-tier secret-leak guard, run on every local commit. # Tier 1: auto-redact gitleaks findings inside src/_generated_ (driven live # by Rule/gitleaks.toml via scripts/patch-generated-secrets.js, not -# a hand-maintained list) and re-stage that directory. Never blocks -# the commit by itself - it either fixes generated code or leaves -# it untouched for tier 2 to catch. +# a hand-maintained list) 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 # any finding tier 1 didn't (or couldn't) resolve. # Installed via the repo's own "prepare" npm script - see package.json. @@ -13,6 +13,22 @@ set -uo pipefail repo_root="$(git rev-parse --show-toplevel)" cd "$repo_root" +generated_dir="src/ _generated_" + +# Snapshot what's already staged in the generated dir 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_dir" || 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 (node 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 # Node install managed by nvm/volta/fnm or bundled with an IDE (rather than # a system package) is often invisible here even though `node` works fine @@ -38,12 +54,25 @@ else fi fi -# Stages the whole generated-code directory rather than just the files tier 1 -# touched, since we don't get that list back from the script. Safe in -# practice: src/_generated_ is machine-owned (Fern-generated, never hand -# edited), so there's no legitimate "leave part of it unstaged" case to worry -# about disturbing. -git add -- "src/ _generated_" +# Stages only what tier 1 actually touched this run (recorded to +# $touched_file_list - see recordTouchedFiles in the script) plus whatever +# was already staged above. A file sitting in src/_generated_ that tier 1 +# didn't touch and the caller hadn't staged is left alone rather than +# force-added - see the med-severity review finding on the old +# unconditional `git add` of the whole directory. +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.js b/scripts/patch-generated-secrets.js index a3fa6f4c..03bd2e9c 100644 --- a/scripts/patch-generated-secrets.js +++ b/scripts/patch-generated-secrets.js @@ -25,6 +25,16 @@ * (e.g. a plain `import { A, B } from '...'` line) as a leak, which * this script would then corrupt by "redacting" it. If the self-test * fails, nothing is touched. + * - Redaction itself never reads a finding's reported "Secret" text. It + * uses gitleaks' (StartLine, StartColumn) only as an approximate + * anchor to locate the surrounding quoted string literal in the + * actual file content, then redacts strictly between its quotes - see + * findQuotedValueSpan. This sidesteps two problems in one move: some + * locally observed gitleaks builds report a "Secret" that swallows a + * trailing quote (would corrupt the surrounding code) or an + * off-by-one column for certain rules, and reading that field at all + * is a source CodeQL's clear-text-logging/storage-sensitive-data + * queries key off of. * - After redacting, `tsc --noEmit` verifies the generated code still * compiles. If it doesn't, every change made in this run is rolled * back and the run fails - a broken build is never left in place @@ -118,11 +128,87 @@ function placeholderFor(ruleID) { return ``; } +const QUOTE_CHARS = new Set(['"', "'", "`"]); +// How far findQuotedValueSpan 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 comment below. +const QUOTE_SEARCH_WINDOW = 8; + +// 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`. +function lineStartOffsets(text) { + const offsets = [0]; + for (const lineText of text.split("\n")) { + offsets.push(offsets[offsets.length - 1] + lineText.length + 1); + } + return offsets; +} + +// Finds [valueStart, valueEnd) strictly inside the quoted string literal +// nearest to `approxPos`, or null 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 (observed on the "jwt" rule with at least one locally +// installed gitleaks build) or include a trailing delimiter it shouldn't +// (also observed on "jwt" - the reported Secret text itself included the +// closing quote), 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, the redaction below has no +// dependency on gitleaks' "Secret" field at all - not just a fix for the +// trailing-quote bug, but also structurally free of the taint path +// CodeQL's clear-text-logging/storage-sensitive-data queries would +// otherwise follow from that field into console.log()/writeFileSync(). +function findQuotedValueSpan(content, approxPos) { + const n = content.length; + for (let delta = 0; delta <= QUOTE_SEARCH_WINDOW; delta++) { + // 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. + const candidates = delta === 0 ? [approxPos] : [approxPos - delta, approxPos + delta]; + for (const pos of candidates) { + if (pos >= 0 && pos < n && QUOTE_CHARS.has(content[pos])) { + const valueStart = pos + 1; + const valueEnd = content.indexOf(content[pos], valueStart); + return valueEnd === -1 ? null : [valueStart, valueEnd]; + } + } + } + return null; +} + +// Records exactly which files this run modified (an empty list if none), +// so the pre-commit hook can stage only those - plus whatever the caller +// already had staged - instead of the entire generated-code directory, +// which would otherwise sweep in unrelated or intentionally-unstaged +// in-progress changes sitting in that same directory. 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. +function recordTouchedFiles(relativeFiles) { + try { + const gitDir = execFileSync("git", ["rev-parse", "--git-dir"], { + cwd: repoRoot, + encoding: "utf8", + }).trim(); + const listPath = path.resolve(repoRoot, gitDir, "leak-guard-touched-files.txt"); + fs.writeFileSync(listPath, relativeFiles.map((f) => `${f}\n`).join("")); + } catch { + // ignored - see comment above + } +} + if (!gitleaksAvailable()) { console.warn( "gitleaks isn't installed locally - skipping auto-redaction of generated code. " + "CI will still scan for this.", ); + recordTouchedFiles([]); process.exit(0); } @@ -134,6 +220,7 @@ if (!selfTestAllowlistSupport()) { "secret. Refusing to auto-redact - please upgrade gitleaks (run `gitleaks version`; " + "CI uses zricethezav/gitleaks:latest) and try again.", ); + recordTouchedFiles([]); process.exit(1); } @@ -147,6 +234,7 @@ const findings = runGitleaksDetect( if (findings.length === 0) { console.log("No gitleaks findings in generated code."); + recordTouchedFiles([]); process.exit(0); } @@ -160,6 +248,7 @@ if (outOfScope.length > 0) { "Refusing to continue: gitleaks reported findings outside the generated code directory:\n" + outOfScope.map((f) => ` - ${f.File}`).join("\n"), ); + recordTouchedFiles([]); process.exit(1); } @@ -170,29 +259,80 @@ for (const finding of findings) { } const originalContents = new Map(); +const touchedFiles = new Set(); let redactedCount = 0; +// 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. +function rollback() { + for (const [filePath, content] of originalContents) { + fs.writeFileSync(filePath, content); + } + recordTouchedFiles([]); +} + +// 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 findQuotedValueSpan) - and never +// reads finding.Secret at all. +// +// 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. +const fileContents = new Map(); +const uniqueSpansByFile = new Map(); +const unresolved = []; for (const [relativeFile, fileFindings] of byFile) { const filePath = path.join(repoRoot, relativeFile); - let content = fs.readFileSync(filePath, "utf8"); - originalContents.set(filePath, content); + const content = fs.readFileSync(filePath, "utf8"); + fileContents.set(filePath, content); + + const lineOffsets = lineStartOffsets(content); + const spans = []; + for (const finding of fileFindings) { + const approxPos = lineOffsets[finding.StartLine - 1] + (finding.StartColumn - 1); + const span = findQuotedValueSpan(content, approxPos); + if (span === null) { + unresolved.push([relativeFile, finding]); + continue; + } + spans.push([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. + const seenSpans = new Map(); + for (const [start, end, ruleID] of spans) seenSpans.set(`${start}:${end}`, [start, end, ruleID]); + const uniqueSpans = [...seenSpans.values()].sort((a, b) => b[0] - a[0] || b[1] - a[1]); + uniqueSpansByFile.set(filePath, uniqueSpans); +} - // 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 - // `content.includes(secret)` 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. - const uniqueSecrets = [...new Set(fileFindings.map((f) => f.Secret))].sort( - (a, b) => b.length - a.length, +if (unresolved.length > 0) { + console.error( + "Refusing to continue: couldn't locate a quoted value near " + + `${unresolved.length} finding(s) - this script only knows how to redact ` + + '`key: "value"`-shaped examples. Manual review needed:\n' + + unresolved.map(([rel, f]) => ` - [${f.RuleID}] ${rel}:${f.StartLine}`).join("\n"), ); + recordTouchedFiles([]); + process.exit(1); +} + +for (const [filePath, spans] of uniqueSpansByFile) { + const relativeFile = path.relative(repoRoot, filePath); + let content = fileContents.get(filePath); + originalContents.set(filePath, content); + + // Assign a distinct placeholder per span, numbering only when the same + // rule fires more than once in the same file (so two different example + // values don't collapse into one identical placeholder). const byRule = new Map(); - for (const secret of uniqueSecrets) { - const ruleID = fileFindings.find((f) => f.Secret === secret).RuleID; + for (const [start, end, ruleID] of spans) { const base = placeholderFor(ruleID); const seen = byRule.get(base) || 0; byRule.set(base, seen + 1); @@ -203,21 +343,20 @@ for (const [relativeFile, fileFindings] of byFile) { // even though there's only ever one to begin with here. const placeholder = seen === 0 ? base : `${base.slice(0, -1)}_${seen + 1}>`; - if (content.includes(secret)) { - content = content.split(secret).join(placeholder); - redactedCount += 1; - console.log(`[${ruleID}] redacted in ${relativeFile} -> ${placeholder}`); - } + content = content.slice(0, start) + placeholder + content.slice(end); + redactedCount += 1; + console.log(`[${ruleID}] redacted in ${relativeFile} -> ${placeholder}`); } fs.writeFileSync(filePath, content); + if (content !== originalContents.get(filePath)) { + touchedFiles.add(relativeFile); + } } const localTsc = path.join(repoRoot, "node_modules", ".bin", "tsc"); if (!fs.existsSync(localTsc)) { - for (const [filePath, content] of originalContents) { - fs.writeFileSync(filePath, content); - } + rollback(); console.error( `${localTsc} not found (run \`npm install\`) - can't verify redaction is safe, so rolling back and refusing to redact.`, ); @@ -236,22 +375,33 @@ try { } if (!buildOk) { - for (const [filePath, content] of originalContents) { - fs.writeFileSync(filePath, content); - } + rollback(); console.error( "Rolled back. Redaction needs manual review - see the tsc output above.", ); process.exit(1); } -const remaining = runGitleaksDetect( - GENERATED_CODE_PREFIX.replace(/\/$/, ""), - repoRoot, -); +// The redaction itself, and this verification re-scan, can each fail for +// reasons other than "still leaks" (e.g. the gitleaks binary crashing +// mid-run) - runGitleaksDetect throws 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 tsc-failure path above does. +let remaining; +try { + remaining = runGitleaksDetect(GENERATED_CODE_PREFIX.replace(/\/$/, ""), repoRoot); +} catch (err) { + rollback(); + console.error( + `Final gitleaks re-scan failed to run (${err.message}) - rolling back all changes from this run. Redaction needs manual review.`, + ); + process.exit(1); +} if (remaining.length > 0) { + rollback(); console.error( - `Redacted ${redactedCount} secret(s), but ${remaining.length} finding(s) remain after re-scanning. Manual review needed:\n` + + `Redacted ${redactedCount} secret(s), but ${remaining.length} finding(s) remain after re-scanning. ` + + "Rolled back all changes from this run. Manual review needed:\n" + remaining .map((f) => ` - [${f.RuleID}] ${f.File}:${f.StartLine}`) .join("\n"), @@ -259,6 +409,7 @@ if (remaining.length > 0) { process.exit(1); } +recordTouchedFiles([...touchedFiles]); console.log( `\nDone. Redacted ${redactedCount} secret(s) across ${byFile.size} file(s). Build and gitleaks re-scan both clean.`, ); From f550f3a9eafb68770287ab2b1b6ffada3f6503a2 Mon Sep 17 00:00:00 2001 From: Himanshu Pal Date: Wed, 23 Sep 2026 16:17:38 +0530 Subject: [PATCH 4/4] SK-3156-added-CI-checks --- .githooks/pre-commit | 2 +- .github/workflows/Gitleaks.yml | 23 ++++++++ .github/workflows/gitleaks-auto-redact.yml | 68 ++++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/gitleaks-auto-redact.yml diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 3eb986e1..8cbf6095 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -76,7 +76,7 @@ fi 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." + echo "[leak-guard] CI will still block this PR on any finding (see Gitleaks.yml) - install gitleaks locally to catch issues before pushing instead of waiting on CI." exit 0 fi diff --git a/.github/workflows/Gitleaks.yml b/.github/workflows/Gitleaks.yml index 9807b5e9..aa902093 100644 --- a/.github/workflows/Gitleaks.yml +++ b/.github/workflows/Gitleaks.yml @@ -95,3 +95,26 @@ jobs: -H "Accept: application/vnd.github.v3+json" \ -d "{\"body\":\"$COMMENT\"}" \ "https://api.github.com/repos/${REPO}/issues/${PR_NUMBER}/comments" + + - name: Fail if secrets were found + # The scan step above runs with --exit-code=0 so it always reaches + # the comment step regardless of findings - reporting and + # enforcement are deliberately separate steps. This is the + # enforcement half: without it, this whole workflow only ever + # comments and never blocks a PR, even when it finds a real + # secret. gitleaks-auto-redact.yml pushes an auto-fix commit for + # fake secrets in generated code, which re-triggers this workflow + # and clears this failure on its own once that commit lands - a + # failure here that persists past that point means a human needs + # to look at it. + run: | + if [ ! -f gitleaks-report.json ]; then + echo "Report file not found!" + exit 1 + fi + COUNT=$(jq 'length' gitleaks-report.json) + if [ "$COUNT" -gt 0 ]; then + echo "::error::Gitleaks found $COUNT secret(s) in this PR - see the PR comment above, or download the gitleaks-report artifact." + exit 1 + fi + echo "No secrets detected - safe to proceed." diff --git a/.github/workflows/gitleaks-auto-redact.yml b/.github/workflows/gitleaks-auto-redact.yml new file mode 100644 index 00000000..065e3fc6 --- /dev/null +++ b/.github/workflows/gitleaks-auto-redact.yml @@ -0,0 +1,68 @@ +name: Gitleaks Auto-Redact + +# Runs scripts/patch-generated-secrets.js on every push to a PR and, if it +# finds anything to redact in the Fern-generated code, commits and pushes +# the fix straight to the PR branch - the same "run in CI, commit the +# result back" pattern common-release.yml uses for its version bump. This +# closes the gap where a contributor without gitleaks or node installed +# locally never gets tier 1's local auto-redaction (see +# .githooks/pre-commit): CI now does it for them instead of only warning. +# +# This is additive to Gitleaks.yml, not a replacement for it - it doesn't +# change that workflow's scan or its exit behavior. + +on: + pull_request: + types: [opened, synchronize, reopened] + branches: + - main + +permissions: + contents: write + +jobs: + auto-redact: + runs-on: ubuntu-latest + # A fork PR's checkout token can't push back to someone else's fork - + # GitHub blocks that regardless of what token this job holds - so skip + # cleanly rather than fail noisily. Gitleaks.yml still scans fork PRs + # as usual; this job only ever adds a redaction commit for same-repo + # branches. + if: github.event.pull_request.head.repo.full_name == github.repository + steps: + - uses: actions/checkout@v4 + with: + token: ${{ secrets.PAT_ACTIONS }} + ref: ${{ github.head_ref }} + + - uses: actions/setup-node@v3 + with: + node-version: '20.x' + + - name: Install packages + run: npm install --ignore-scripts + + - name: Install gitleaks + run: | + GITLEAKS_VERSION="$(curl -fsSL https://api.github.com/repos/gitleaks/gitleaks/releases/latest | grep -m1 '"tag_name"' | cut -d '"' -f4 | sed 's/^v//')" + curl -fsSL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" -o gitleaks.tar.gz + tar -xzf gitleaks.tar.gz gitleaks + sudo install -m 0755 gitleaks /usr/local/bin/gitleaks + rm -f gitleaks.tar.gz gitleaks + gitleaks version + + - name: Auto-redact generated code + run: node scripts/patch-generated-secrets.js + + - name: Commit and push redaction, if any + run: | + git config user.name "${{ github.actor }}" + git config user.email "${{ github.actor }}@users.noreply.github.com" + git checkout "${{ github.head_ref }}" + if git diff --quiet -- "src/ _generated_"; then + echo "No fake secrets found to redact." + else + git add -- "src/ _generated_" + git commit -m "[AUTOMATED] redact fake secrets in generated code" + git push origin "HEAD:${{ github.head_ref }}" + fi