Skip to content

Commit 07064df

Browse files
fix(scripts): surface git-diff failure in selfhost pre-deploy summary (#7787)
The sensitive-path scan read `git diff --name-only` via a process substitution (`done < <(git diff ...)`), whose exit status is invisible to `set -euo pipefail`. A diff failure (bad ref, shallow clone, detached history) left the loop with zero lines, so the script reported "no historically-sensitive paths touched" -- a false all-clear on a deploy-safety advisory. Capture the diff via a checked command substitution and error out loudly if it fails. Closes #7775 Co-authored-by: jaytbarimbao-collab <300663773+jaytbarimbao-collab@users.noreply.github.com>
1 parent e8577cb commit 07064df

2 files changed

Lines changed: 34 additions & 1 deletion

File tree

scripts/selfhost-pre-deploy-summary.sh

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,19 @@ is_sensitive() {
116116
}
117117

118118
sensitive_files=()
119+
# #7775: capture the diff via a CHECKED command substitution rather than `done < <(git diff ...)`. A
120+
# process-substitution failure (bad ref, shallow clone, detached history) is invisible to `set -e`, so the
121+
# loop would just see no lines and the script would report "no historically-sensitive paths touched" -- a
122+
# false all-clear on a deploy-safety advisory. Surface it as a real error instead.
123+
if ! diff_output="$(git diff --name-only "$range")"; then
124+
echo "error: 'git diff --name-only $range' failed; cannot assess historically-sensitive paths" >&2
125+
exit 1
126+
fi
119127
while IFS= read -r file; do
120128
if [ -n "$file" ] && is_sensitive "$file"; then
121129
sensitive_files+=("$file")
122130
fi
123-
done < <(git diff --name-only "$range")
131+
done <<< "$diff_output"
124132

125133
echo ""
126134
if [ "${#sensitive_files[@]}" -eq 0 ]; then

test/unit/selfhost-pre-deploy-summary-script.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,4 +224,29 @@ describe("selfhost-pre-deploy-summary.sh", () => {
224224
expect(result.stderr).toContain("is not an ancestor");
225225
expect(result.stdout).toContain("upstream commit");
226226
});
227+
228+
it("surfaces an error instead of a false all-clear when git diff --name-only fails (#7775)", () => {
229+
const { base, seedDir, checkoutDir } = createSandbox();
230+
// Give origin an incoming commit so the script proceeds past "up to date" to the sensitive-path diff.
231+
commitFile(seedDir, "src/incoming.ts", "export const incoming = 1;\n", "incoming commit");
232+
git(["push", "-q", "origin", "main"], seedDir);
233+
234+
// Shadow `git` with a fake that fails ONLY `git diff --name-only ...` (the sensitive-path scan),
235+
// delegating every other command -- including fetch and the `git diff --stat` summary -- to real git.
236+
const realGit = spawnSync("bash", ["-c", "command -v git"], { encoding: "utf8" }).stdout.trim();
237+
const fakeBin = join(base, "fakebin");
238+
mkdirSync(fakeBin, { recursive: true });
239+
writeFileSync(
240+
join(fakeBin, "git"),
241+
`#!/usr/bin/env bash\nif [ "$1" = "diff" ] && [ "$2" = "--name-only" ]; then\n echo "forced diff failure" >&2\n exit 1\nfi\nexec ${realGit} "$@"\n`,
242+
{ mode: 0o755 },
243+
);
244+
245+
const result = run(checkoutDir, { PATH: `${fakeBin}:${process.env.PATH ?? ""}` });
246+
247+
// A failed diff must be reported as a failure, not silently presented as "no sensitive paths touched".
248+
expect(result.status, result.stdout + result.stderr).not.toBe(0);
249+
expect(result.stderr).toMatch(/git diff.*failed|cannot assess historically-sensitive/i);
250+
expect(result.stdout).not.toContain("no historically-sensitive paths touched");
251+
});
227252
});

0 commit comments

Comments
 (0)