Skip to content

Commit 68fac24

Browse files
authored
fix(review): hash a JSON payload in linkedIssueSatisfactionCacheInputFingerprint (#5976)
The fingerprint "|"-joined free-form GitHub text fields (issueText, prTitle, prBody, diff), so an unescaped "|" inside one field could shift a boundary and make two genuinely different inputs serialize identically and collide on the same cache key. Build the payload with JSON.stringify, matching the sibling aiSlopCacheInputFingerprint, so distinct inputs always produce distinct fingerprints. Closes #5939
1 parent ac83483 commit 68fac24

2 files changed

Lines changed: 25 additions & 11 deletions

File tree

src/review/linked-issue-satisfaction-cache-input.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,19 @@ export type LinkedIssueSatisfactionCacheInput = {
1818
};
1919

2020
export async function linkedIssueSatisfactionCacheInputFingerprint(input: LinkedIssueSatisfactionCacheInput): Promise<string> {
21-
const payload = [
22-
LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION,
23-
input.byok ? "1" : "0",
24-
input.provider ?? "",
25-
input.model ?? "",
26-
input.issueText ?? "",
27-
input.prTitle ?? "",
28-
input.prBody ?? "",
29-
input.diff ?? "",
30-
].join("|");
31-
return `${LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION}:${await sha256Hex(payload)}`;
21+
// Structurally-delimited payload (mirrors ai-slop-cache-input.ts): a bare "|"-join of free-form
22+
// GitHub text let an unescaped "|" inside one field shift a field boundary, so two genuinely different
23+
// inputs could serialize identically and collide on the same fingerprint. JSON.stringify escapes the
24+
// field values, so distinct inputs always produce distinct payloads.
25+
const payload = {
26+
version: LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION,
27+
byok: input.byok,
28+
provider: input.provider ?? "",
29+
model: input.model ?? "",
30+
issueText: input.issueText ?? "",
31+
prTitle: input.prTitle ?? "",
32+
prBody: input.prBody ?? "",
33+
diff: input.diff ?? "",
34+
};
35+
return `${LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION}:${await sha256Hex(JSON.stringify(payload))}`;
3236
}

test/unit/linked-issue-satisfaction-cache.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,16 @@ describe("linked-issue satisfaction cache (#1961/#3906)", () => {
4747
expect(await getCachedLinkedIssueSatisfaction(env, "o/r", 9, "sha1", 1, freeFingerprint)).toEqual({ status: "ok", result: { status: "partial", rationale: "r", confidence: 0.7 }, estimatedNeurons: 6 });
4848
});
4949

50+
it("does not collide when a '|' inside one text field would shift a delimiter boundary (#5939)", async () => {
51+
// A bare "|"-join let an unescaped "|" move a field boundary: {issueText: "foo|bar", prTitle: "baz"}
52+
// and {issueText: "foo", prTitle: "bar|baz"} (other fields equal) serialized identically and hashed
53+
// to the same fingerprint. JSON.stringify escapes the field values, so the two stay distinct.
54+
const base = { byok: false, provider: null, model: null } as const;
55+
const a = await linkedIssueSatisfactionCacheInputFingerprint({ ...base, issueText: "foo|bar", prTitle: "baz" });
56+
const b = await linkedIssueSatisfactionCacheInputFingerprint({ ...base, issueText: "foo", prTitle: "bar|baz" });
57+
expect(a).not.toBe(b);
58+
});
59+
5060
it("upserts — a re-run at the same key replaces the stored assessment", async () => {
5161
const env = createTestEnv();
5262
const fingerprint = await fp();

0 commit comments

Comments
 (0)