Full document, spreadsheet, slideshow, and diagram tooling
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge pull request 'fix: server hardening + formula edge cases batch 3' (#243) from fix/hardening-batch3 into main

scott 71847b60 e72900a7

+104 -17
+19 -4
server/index.ts
··· 326 326 deleteBlobsForDoc: db.prepare('DELETE FROM blobs WHERE document_id = ?'), 327 327 }; 328 328 329 + /** Validate document/blob ID: UUID format or reasonable alphanumeric */ 330 + function isValidDocId(id: string): boolean { 331 + return typeof id === 'string' && id.length > 0 && id.length <= 100 && /^[a-zA-Z0-9_-]+$/.test(id); 332 + } 333 + 329 334 // --- Express --- 330 335 const app = express(); 331 336 app.use(compression()); ··· 495 500 496 501 // Accept both PUT (normal save) and POST (sendBeacon — which can only POST) 497 502 const snapshotHandler = (req: Request<{ id: string }>, res: Response): void => { 503 + if (!isValidDocId(req.params.id)) { 504 + res.status(400).json({ error: 'Invalid document ID' }); 505 + return; 506 + } 498 507 if (!req.body || !Buffer.isBuffer(req.body) || req.body.length === 0) { 499 508 res.status(400).json({ error: 'Empty or missing snapshot body' }); 500 509 return; ··· 534 543 535 544 // --- Sharing --- 536 545 app.put('/api/documents/:id/share', (req: Request<{ id: string }, unknown, UpdateShareBody>, res: Response) => { 537 - const doc = stmts.getOne.get(req.params.id) as DocumentRow | undefined; 546 + const doc = stmts.getOne.get(req.params.id) as DocumentListRow | undefined; 538 547 if (!doc) { res.status(404).json({ error: 'Not found' }); return; } 539 548 549 + // Only the document owner can change sharing settings 550 + if (doc.owner && req.tsUser?.login !== doc.owner) { 551 + res.status(403).json({ error: 'Only the document owner can change sharing settings' }); 552 + return; 553 + } 554 + 540 555 const { share_mode, expires_at } = req.body; 541 556 542 - // Validate share_mode 543 - if (share_mode && !['edit', 'view'].includes(share_mode)) { 557 + // Validate share_mode (also reject empty string) 558 + if (share_mode !== undefined && share_mode !== null && !['edit', 'view'].includes(share_mode)) { 544 559 res.status(400).json({ error: 'share_mode must be "edit" or "view"' }); 545 560 return; 546 561 } ··· 876 891 } 877 892 wss.handleUpgrade(request, socket, head, (ws: WebSocket) => { 878 893 const room = url.searchParams.get('room'); 879 - if (!room) { ws.close(); return; } 894 + if (!room || room.length > 200 || !/^[a-zA-Z0-9_-]+$/.test(room)) { ws.close(); return; } 880 895 881 896 // Extract Tailscale identity from the upgrade request headers 882 897 const wsUserLogin = request.headers['tailscale-user-login'] as string | undefined;
+7 -3
src/sheets/formulas.ts
··· 771 771 772 772 switch (name) { 773 773 case 'SUM': return nums(args).reduce((a, b) => a + b, 0); 774 - case 'AVERAGE': { const n = nums(args); return n.length ? n.reduce((a, b) => a + b, 0) / n.length : 0; } 774 + case 'AVERAGE': { const n = nums(args); return n.length ? n.reduce((a, b) => a + b, 0) / n.length : '#DIV/0!'; } 775 775 case 'COUNT': return nums(args).length; 776 776 case 'COUNTA': return flat(args).length; 777 777 case 'MIN': { const n = nums(args); return n.length ? Math.min(...n) : 0; } ··· 957 957 for (let i = 0; i < range.length; i++) { 958 958 if (matchCriteria(range[i], criteria)) vals.push(toNum(avgRange[i] ?? 0)); 959 959 } 960 - return vals.length ? vals.reduce((a, b) => a + b, 0) / vals.length : 0; 960 + return vals.length ? vals.reduce((a, b) => a + b, 0) / vals.length : '#DIV/0!'; 961 961 } 962 962 963 963 case 'MEDIAN': { ··· 969 969 970 970 case 'STDEV': { 971 971 const n = nums(args); 972 - if (n.length < 2) return 0; 972 + if (n.length < 2) return '#DIV/0!'; 973 973 const mean = n.reduce((a, b) => a + b, 0) / n.length; 974 974 const variance = n.reduce((a, b) => a + (b - mean) ** 2, 0) / (n.length - 1); 975 975 return Math.sqrt(variance); ··· 1308 1308 case 'NETWORKDAYS': { 1309 1309 const nwStart = new Date(args[0] as string | number | Date); 1310 1310 const nwEnd = new Date(args[1] as string | number | Date); 1311 + if (isNaN(nwStart.getTime()) || isNaN(nwEnd.getTime())) return '#VALUE!'; 1312 + // Safety: cap at ~200 years to prevent infinite/very-long loops 1313 + const nwDaySpan = Math.abs(nwEnd.getTime() - nwStart.getTime()) / 86400000; 1314 + if (nwDaySpan > 73000) return '#VALUE!'; 1311 1315 let nwCount = 0; 1312 1316 const nwStep = nwStart <= nwEnd ? 1 : -1; 1313 1317 const nwCur = new Date(nwStart);
+70 -2
tests/formulas-edge-cases.test.ts
··· 53 53 expect(result).toBe('#DIV/0!'); 54 54 }); 55 55 56 - it('AVERAGE of range with zero count still returns 0', () => { 56 + it('AVERAGE of empty range returns #DIV/0!', () => { 57 57 const result = evalWith('AVERAGE(A1:A5)', {}); 58 - expect(result).toBe(0); 58 + expect(result).toBe('#DIV/0!'); 59 59 }); 60 60 61 61 it('nested division by zero: IF guards prevent error', () => { ··· 1195 1195 expect(evalWith('1.5e2')).toBe(150); 1196 1196 }); 1197 1197 }); 1198 + 1199 + // ============================================================ 1200 + // AVERAGE/AVERAGEIF/STDEV edge cases (#394, #395, #396) 1201 + // ============================================================ 1202 + 1203 + describe('AVERAGE/AVERAGEIF/STDEV — empty/single edge cases', () => { 1204 + it('AVERAGE of empty range returns #DIV/0!', () => { 1205 + // AVERAGE with no numeric values should be a division error, not 0 1206 + expect(evalWith('AVERAGE("")')).toBe('#DIV/0!'); 1207 + }); 1208 + 1209 + it('AVERAGEIF with no matching values returns #DIV/0!', () => { 1210 + const cells: Record<string, unknown> = { A1: 1, A2: 2, A3: 3 }; 1211 + expect(evaluate('AVERAGEIF(A1:A3,">10")', (ref) => cells[ref] ?? '')).toBe('#DIV/0!'); 1212 + }); 1213 + 1214 + it('STDEV of single element returns #DIV/0!', () => { 1215 + expect(evalWith('STDEV(5)')).toBe('#DIV/0!'); 1216 + }); 1217 + 1218 + it('STDEV of two elements works', () => { 1219 + const result = evalWith('STDEV(2,4)'); 1220 + expect(typeof result).toBe('number'); 1221 + expect(result).toBeCloseTo(Math.SQRT2, 5); 1222 + }); 1223 + }); 1224 + 1225 + // ============================================================ 1226 + // NETWORKDAYS safety guard (#414) 1227 + // ============================================================ 1228 + 1229 + describe('NETWORKDAYS — safety guards', () => { 1230 + it('returns #VALUE! for invalid start date', () => { 1231 + expect(evalWith('NETWORKDAYS("not-a-date","2024-01-15")')).toBe('#VALUE!'); 1232 + }); 1233 + 1234 + it('returns #VALUE! for invalid end date', () => { 1235 + expect(evalWith('NETWORKDAYS("2024-01-01","not-a-date")')).toBe('#VALUE!'); 1236 + }); 1237 + 1238 + it('handles reasonable date range', () => { 1239 + // Jan 1 (Sun) to Jan 5 (Thu): Mon-Thu = 4 workdays 1240 + expect(evalWith('NETWORKDAYS("2024-01-01","2024-01-05")')).toBe(4); 1241 + }); 1242 + 1243 + it('caps excessively large date ranges', () => { 1244 + // 100+ year span should be guarded 1245 + const result = evalWith('NETWORKDAYS("1900-01-01","2100-01-01")'); 1246 + // Should either return a value or an error, but not hang 1247 + expect(result !== undefined).toBe(true); 1248 + }); 1249 + }); 1250 + 1251 + // ============================================================ 1252 + // matchCriteria '<>' coercion (#400) 1253 + // ============================================================ 1254 + 1255 + describe('matchCriteria — <> operator coercion', () => { 1256 + it('COUNTIF with <> should use type-aware comparison', () => { 1257 + const cells: Record<string, unknown> = { A1: 1, A2: 2, A3: 1, A4: 3 }; 1258 + expect(evaluate('COUNTIF(A1:A4,"<>1")', (ref) => cells[ref] ?? '')).toBe(2); 1259 + }); 1260 + 1261 + it('COUNTIF with <> on strings', () => { 1262 + const cells: Record<string, unknown> = { A1: 'a', A2: 'b', A3: 'a' }; 1263 + expect(evaluate('COUNTIF(A1:A3,"<>a")', (ref) => cells[ref] ?? '')).toBe(1); 1264 + }); 1265 + });
+2 -2
tests/formulas-extended.test.ts
··· 209 209 expect(evalWith('AVERAGEIF(A1:A3,">15",B1:B3)', cells)).toBe(250); // (200+300)/2 210 210 }); 211 211 212 - it('returns 0 for no matching criteria', () => { 212 + it('returns #DIV/0! for no matching criteria', () => { 213 213 const cells = { A1: 1, A2: 2, A3: 3 }; 214 - expect(evalWith('AVERAGEIF(A1:A3,">100")', cells)).toBe(0); 214 + expect(evalWith('AVERAGEIF(A1:A3,">100")', cells)).toBe('#DIV/0!'); 215 215 }); 216 216 }); 217 217
+6 -6
tests/formulas.test.ts
··· 379 379 expect(result).toBe(0); 380 380 }); 381 381 382 - it('AVERAGE of empty range returns 0', () => { 382 + it('AVERAGE of empty range returns #DIV/0!', () => { 383 383 const result = evalWith('AVERAGE(A1:A5)', {}); 384 - expect(result).toBe(0); 384 + expect(result).toBe('#DIV/0!'); 385 385 }); 386 386 387 387 it('MIN of empty range returns 0', () => { ··· 648 648 expect(result).toBeCloseTo(2.138, 2); 649 649 }); 650 650 651 - it('returns 0 for single element', () => { 651 + it('returns #DIV/0! for single element', () => { 652 652 const result = evalWith('STDEV(5)'); 653 - expect(result).toBe(0); 653 + expect(result).toBe('#DIV/0!'); 654 654 }); 655 655 656 - it('returns 0 for empty range', () => { 656 + it('returns #DIV/0! for empty range', () => { 657 657 const result = evalWith('STDEV(A1:A3)', {}); 658 - expect(result).toBe(0); 658 + expect(result).toBe('#DIV/0!'); 659 659 }); 660 660 661 661 it('computes STDEV of two identical values as 0', () => {