TODO.md

@@ -18,6 +18,12 @@ Tag v1.0.0.
 
 # Completed Steps
 
+- 2026-09-23: Pinned three guards reviewers found untested (issue 89). The
+  download idle deadline's timer is unref'd, so it can never keep the process
+  alive, and a test checks no timer is left after a download completes or fails.
+  A test covers the rejection of `#` in a request path. The EXIF scan compares
+  the `Exif` header only in an APP1 segment of length 8 or more, so it never
+  reads the next segment's bytes, with a test for a short one.
 - 2026-09-23: Made the download deadline an idle deadline (issue 24).
   `downloadTimeoutMs` now aborts a file or thumbnail download only after no
   bytes have arrived for that long, default 60 seconds, instead of bounding the

src/api/client.ts

@@ -49,8 +49,8 @@ export interface StreamOptions {
 // An abort signal that fires once `ms` pass without a call to `restart`. It
 // aborts with a `TimeoutError`, the same reason `AbortSignal.timeout()` gives,
 // so the retry classifier treats an idle download exactly as it treats any
-// other deadline. `stop` must be called when the download ends, or the timer
-// keeps the process alive until it fires.
+// other deadline. `stop` must be called when the download ends. The timer is
+// unref'd, so even one left running never keeps the process alive.
 const idleDeadline = (ms: number) => {
     const controller = new AbortController();
     let timer: ReturnType<typeof setTimeout> | undefined;
@@ -65,6 +65,7 @@ const idleDeadline = (ms: number) => {
                 ),
             );
         }, ms);
+        timer.unref();
     };
     restart();
     return { signal: controller.signal, restart, stop };

src/metadata-backup.ts

@@ -45,8 +45,11 @@ export const extractExifFromJpeg = (
                 error: `segment length ${len} at byte ${offset} runs past the end of the file`,
             };
         if (marker === 0xe1) {
-            // APP1 — check for "Exif\0\0" header
+            // APP1 — check for "Exif\0\0" header. A length under 8 cannot hold
+            // the six-byte header, so the segment is not EXIF; below 6 the
+            // bytes compared would also lie past the segment.
             if (
+                len >= 8 &&
                 buf[offset + 4] === 0x45 &&
                 buf[offset + 5] === 0x78 &&
                 buf[offset + 6] === 0x69 &&

test/api/client.test.ts

@@ -474,6 +474,16 @@ describe("ApiClient request URLs", () => {
         );
         expect(calls).toHaveLength(0);
     });
+
+    it("rejects a path that carries a fragment", async () => {
+        const { fetch, calls } = recordingFetch();
+        const client = new ApiClient({ fetch });
+
+        await expect(client.getJSON("/diff#top")).rejects.toThrow(
+            /must not contain "\?" or "#"/,
+        );
+        expect(calls).toHaveLength(0);
+    });
 });
 
 describe("ApiError", () => {
@@ -980,6 +990,52 @@ describe("ApiClient timeouts", () => {
         }
         expect(joined).toEqual(payload);
     });
+
+    it("leaves no timer pending after a download completes or fails", async () => {
+        vi.useFakeTimers();
+        try {
+            const { fetch } = scriptedFetch(
+                streamResponse(new Uint8Array([1, 2, 3])),
+                textResponse("gone", 404),
+            );
+            const client = new ApiClient({
+                fetch,
+                retry: { ...noWait, attempts: 1 },
+            });
+
+            expect(await readAll(await client.getFileStream(1))).toBe(3);
+            expect(vi.getTimerCount()).toBe(0);
+
+            await expect(client.getFileStream(2)).rejects.toBeInstanceOf(
+                ApiError,
+            );
+            expect(vi.getTimerCount()).toBe(0);
+        } finally {
+            vi.useRealTimers();
+        }
+    });
+
+    it("never lets the download timer keep the process alive", async () => {
+        const spy = vi.spyOn(globalThis, "setTimeout");
+        try {
+            const { fetch } = scriptedFetch(
+                streamResponse(new Uint8Array([1])),
+            );
+            const client = new ApiClient({
+                fetch,
+                downloadTimeoutMs: 12_345,
+                retry: noWait,
+            });
+
+            const stream = await client.getFileStream(1);
+            const i = spy.mock.calls.findIndex((call) => call[1] === 12_345);
+            const timer = spy.mock.results[i]!.value as NodeJS.Timeout;
+            expect(timer.hasRef()).toBe(false);
+            await stream.cancel();
+        } finally {
+            spy.mockRestore();
+        }
+    });
 });
 
 describe("ApiClient error typing", () => {

test/cli/metadata-exif.test.ts

@@ -51,6 +51,20 @@ describe("extractExifFromJpeg", () => {
         expect(extractExifFromJpeg(bytes(SOI, app0, SOS))).toEqual({});
     });
 
+    it("ignores an APP1 segment too short to hold the Exif header", () => {
+        // A length under 8 cannot hold the six-byte "Exif\0\0" header, so the
+        // segment is not EXIF. This one has length 7 and holds only "Exif\0",
+        // which the old code, lacking the length check, returned as EXIF.
+        const short = app1(EXIF_HEADER.slice(0, 5));
+        expect(extractExifFromJpeg(bytes(SOI, short, SOS))).toEqual({});
+    });
+
+    it("accepts an APP1 segment of length 8 holding just the Exif header", () => {
+        const scan = extractExifFromJpeg(bytes(SOI, app1(EXIF_HEADER), SOS));
+        expect(scan.error).toBeUndefined();
+        expect([...scan.exif!]).toEqual(EXIF_HEADER);
+    });
+
     it("reports a JPEG truncated inside a segment header", () => {
         const scan = extractExifFromJpeg(bytes(SOI, [0xff, 0xe1, 0x00]));
         expect(scan.exif).toBeUndefined();