diff --git a/bin/gstack-redact-prepush b/bin/gstack-redact-prepush index 14bac37c5..6880cf11d 100755 --- a/bin/gstack-redact-prepush +++ b/bin/gstack-redact-prepush @@ -146,7 +146,8 @@ function pushRemote(): string | null { * "already on the remote" — so rev-list reported only the harmless commit as * new, the range anchored at its parent, and the credential shipped with exit 0. * A URL we were handed is not described by ANY remote-tracking ref, so there is - * nothing legitimately excludable and everything reachable must be scanned. + * nothing legitimately excludable: every commit reachable from the pushed tip + * counts as new, and the range is anchored below all of them. * * An UNKNOWN target (no argv, stdin/CLI invocation) keeps the historical bare * form: we were told nothing, so we cannot single out a namespace. @@ -356,11 +357,19 @@ function addedLinesFor(localSha: string, remoteSha: string): string { // the credential with exit 0. A guess that scanned something is still a // guess about the wrong thing. // - // So this shape gets no local narrowing at all: scan everything reachable - // from the pushed tip. That is affordable now only because every scan is - // sliced under the engine's cap (see scanAddedLines), so a whole-history - // range can no longer degrade into engine.input_too_large — which is - // exactly why the older code avoided this path. + // So this shape gets no local narrowing at all: the range is + // emptyTree..localSha, i.e. the full CONTENT of the pushed tip is scanned. + // That is affordable now only because every scan is sliced under the + // engine's cap (see scanAddedLines), so a whole-tree range can no longer + // degrade into engine.input_too_large — which is exactly why the older + // code avoided this path. + // + // Note what this is not: a `git diff` range is a NET diff, so it scans the + // final tree, not each commit. A credential added in one commit and + // removed in a later one within the same push is not in that diff at all, + // on this path and on every other range this hook computes. Closing that + // means scanning each commit's own diff — a separate change with its own + // cost profile, not part of this one. // // A NEW BRANCH (zero remote sha) is deliberately NOT treated this way: // there git named no tip, our tracking refs are not contradicted, and @@ -586,38 +595,45 @@ function scanAddedLines(added: string, opts: Parameters[1]): Findin sliceBytes = 0; return; } - // Re-seed the next slice with the trailing lines worth up to one overlap. + // Re-seed the next slice with up to one overlap's worth of trailing content. + // + // ⚠ FILL THE OVERLAP, DO NOT MERELY COLLECT WHOLE LINES. + // + // Walk back from the seam taking whole lines while they fit, and when one + // does not, take as much of ITS END as the remaining budget allows. Carrying + // whole lines only lets a line longer than the overlap contribute nothing, so + // a label at its end never met the secret after the seam. Carrying that + // suffix only when the tail was otherwise empty was not enough either: a + // short line between the long label line and the secret landed in the tail + // first, the tail was no longer empty, and the label was dropped again + // (measured, gate rows 19 and 21). Budget is exact: a character is added only + // if it still fits, so `overlap + unit <= SCAN_CHUNK_BYTES` keeps holding. const tail: string[] = []; let tailBytes = 0; for (let i = slice.length - 1; i >= 0; i--) { - const b = Buffer.byteLength(slice[i]!, "utf8") + 1; - if (tailBytes + b > SCAN_OVERLAP_BYTES) { - // ⚠ A LINE BIGGER THAN THE OVERLAP MUST STILL CONTRIBUTE ITS END. - // - // Carrying whole lines only means a single line longer than the whole - // overlap budget contributes NOTHING, and the seam it sits on gets no - // context at all. Measured: one ~770 KB ASCII line ending in - // `aws_secret_access_key =`, followed by a line holding the key, put the - // label and the secret in different slices with an empty overlap between - // them, so aws.secret_key never fired. Carry the line's SUFFIX instead — - // the last overlap-worth of it is exactly the part a proximity pattern - // needs. - if (tail.length === 0) { - const line = slice[i]!; - let back = 0; - let backBytes = 0; - while (back < line.length && backBytes < SCAN_OVERLAP_BYTES) { - backBytes += Buffer.byteLength(line[line.length - back - 1]!, "utf8"); - back++; - } - const suffix = line.slice(safeCut(line, line.length - back)); - tail.unshift(suffix); - tailBytes += Buffer.byteLength(suffix, "utf8") + 1; - } - break; + const line = slice[i]!; + const b = Buffer.byteLength(line, "utf8") + 1; + if (tailBytes + b <= SCAN_OVERLAP_BYTES) { + tail.unshift(line); + tailBytes += b; + continue; } - tail.unshift(slice[i]!); - tailBytes += b; + // One byte of the remaining room is the newline that rejoins the piece. + const room = SCAN_OVERLAP_BYTES - tailBytes - 1; + let back = 0; + let backBytes = 0; + while (back < line.length) { + const ch = Buffer.byteLength(line[line.length - back - 1]!, "utf8"); + if (backBytes + ch > room) break; + backBytes += ch; + back++; + } + const suffix = line.slice(safeCut(line, line.length - back)); + if (suffix) { + tail.unshift(suffix); + tailBytes += Buffer.byteLength(suffix, "utf8") + 1; + } + break; } slice = tail; sliceBytes = tailBytes; diff --git a/test/redact-prepush-fail-open.sh b/test/redact-prepush-fail-open.sh index f5ed452ec..fac9845d5 100755 --- a/test/redact-prepush-fail-open.sh +++ b/test/redact-prepush-fail-open.sh @@ -397,6 +397,40 @@ git add -A >/dev/null; git commit -qm bigline >/dev/null S19=$(run_probe "$R/s19") row "19 label at the end of an over-overlap line" "$S19" "BLOCK(aws.secret_key)" +# 21: same seam, but a SHORT line sits between the long line and the secret. +# Carrying a long line's suffix only when the tail is otherwise empty is not +# enough: the short line lands in the tail first, the tail is no longer empty, +# and the long line's end — where the label is — is dropped again. +mkrepo s21 +SEC="$SEC" python3 -c " +import os +sec=os.environ['SEC'] +filler='x'*770020 +with open('big.txt','w') as f: + f.write(filler+' aws_secret_access_key =\n') + f.write('\n') + f.write(sec+' '+'y'*20000+'\n')" +git add -A >/dev/null; git commit -qm bigline-gap >/dev/null +S21=$(run_probe "$R/s21") +row "21 short line between the long label line and the secret" "$S21" "BLOCK(aws.secret_key)" + +# 22: the general form of 19/21. Several short lines between the long label +# line and the secret, still well inside the pattern's proximity window. The +# property being pinned is that the LAST overlap-worth of text before a seam is +# always carried, whatever mix of long and short lines it is made of. +mkrepo s22 +SEC="$SEC" python3 -c " +import os +sec=os.environ['SEC'] +filler='x'*770000 +with open('big.txt','w') as f: + f.write(filler+' aws_secret_access_key =\n') + for _ in range(20): f.write('\n') + f.write(sec+' '+'y'*20000+'\n')" +git add -A >/dev/null; git commit -qm bigline-gaps >/dev/null +S22=$(run_probe "$R/s22") +row "22 several short lines between long label line and secret" "$S22" "BLOCK(aws.secret_key)" + # E6: long-line slicer survives multi-byte text mkrepo e6; KEY="$KEY" python3 -c " import os