Skip to content

Commit 54964f5

Browse files
Handle truncated git review diff previews safely
- Drop unterminated paths from truncated NUL-separated git stdout - Preserve truncation state when building untracked review diffs
1 parent 665c0c6 commit 54964f5

2 files changed

Lines changed: 35 additions & 4 deletions

File tree

apps/server/src/vcs/GitVcsDriverCore.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as Scope from "effect/Scope";
99

1010
import { GitCommandError } from "@t3tools/contracts";
1111
import { ServerConfig } from "../config.ts";
12+
import { splitNullSeparatedGitStdoutPaths } from "./GitVcsDriverCore.ts";
1213
import * as GitVcsDriver from "./GitVcsDriver.ts";
1314

1415
const ServerConfigLayer = ServerConfig.layerTest(process.cwd(), {
@@ -77,6 +78,30 @@ const initRepoWithCommit = (
7778
});
7879

7980
it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
81+
describe("review diff previews", () => {
82+
it.effect("drops an unterminated path from truncated NUL-separated git output", () =>
83+
Effect.sync(() => {
84+
const paths = splitNullSeparatedGitStdoutPaths({
85+
stdout: "complete.txt\0partial",
86+
stdoutTruncated: true,
87+
});
88+
89+
assert.deepStrictEqual(paths, ["complete.txt"]);
90+
}),
91+
);
92+
93+
it.effect("keeps the final path when NUL-separated git output is complete", () =>
94+
Effect.sync(() => {
95+
const paths = splitNullSeparatedGitStdoutPaths({
96+
stdout: "complete.txt\0final.txt",
97+
stdoutTruncated: false,
98+
});
99+
100+
assert.deepStrictEqual(paths, ["complete.txt", "final.txt"]);
101+
}),
102+
);
103+
});
104+
80105
describe("repository status", () => {
81106
it.effect("reports non-repository directories without failing", () =>
82107
Effect.gen(function* () {

apps/server/src/vcs/GitVcsDriverCore.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,12 @@ function splitNullSeparatedPaths(input: string, truncated: boolean): string[] {
208208
return parts.filter((value) => value.length > 0);
209209
}
210210

211+
export function splitNullSeparatedGitStdoutPaths(
212+
result: Pick<GitVcsDriver.ExecuteGitResult, "stdout" | "stdoutTruncated">,
213+
): string[] {
214+
return splitNullSeparatedPaths(result.stdout, result.stdoutTruncated);
215+
}
216+
211217
function sanitizeRemoteName(value: string): string {
212218
const sanitized = value
213219
.trim()
@@ -1654,7 +1660,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
16541660
});
16551661

16561662
const readUntrackedReviewDiffs = Effect.fn("readUntrackedReviewDiffs")(function* (cwd: string) {
1657-
const untrackedOutput = yield* runGitStdoutWithOptions(
1663+
const untrackedResult = yield* executeGit(
16581664
"GitVcsDriver.readUntrackedReviewDiffs.list",
16591665
cwd,
16601666
["ls-files", "--others", "--exclude-standard", "-z"],
@@ -1663,9 +1669,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
16631669
truncateOutputAtMaxBytes: true,
16641670
},
16651671
);
1666-
const untrackedPaths = splitNullSeparatedPaths(untrackedOutput, false);
1672+
const untrackedPaths = splitNullSeparatedGitStdoutPaths(untrackedResult);
16671673
if (untrackedPaths.length === 0) {
1668-
return { diff: "", truncated: false };
1674+
return { diff: "", truncated: untrackedResult.stdoutTruncated };
16691675
}
16701676

16711677
const diffs = yield* Effect.forEach(
@@ -1689,7 +1695,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
16891695
.map((result) => result.stdout)
16901696
.filter((diff) => diff.trim().length > 0)
16911697
.join("\n"),
1692-
truncated: diffs.some((result) => result.stdoutTruncated),
1698+
truncated: untrackedResult.stdoutTruncated || diffs.some((result) => result.stdoutTruncated),
16931699
};
16941700
});
16951701

0 commit comments

Comments
 (0)