From e2a24d768bc336c1c1f45652105955bc62b50632 Mon Sep 17 00:00:00 2001 From: le king fu Date: Wed, 7 Oct 2026 20:25:26 -0400 Subject: [PATCH] fix(server): require a full ISO receivedAt on read, cover the temp-file cleanup readClaudeUsage() and readHostSnapshot() validated the stored receivedAt with new Date(), which accepts "1" or a bare date. Both now go through isIsoTimestamp(), exported by claude-usage.js and built on the sanitizer's own check, so no regex is duplicated. The existing 500 tests failed at mkdir, before the temp file existed, so removing the unlinkSync() cleanup of either writer left the suite green. Two new cases make the rename fail after the temp write (the target is a non-empty directory) and require that no temp file is left. Refs #54 Co-Authored-By: Claude Opus 5.5 --- __tests__/claude-usage.test.js | 22 ++++++++++++++++++++++ __tests__/hosts.test.js | 21 +++++++++++++++++++++ claude-usage.js | 8 ++++++++ index.js | 6 +++--- 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/__tests__/claude-usage.test.js b/__tests__/claude-usage.test.js index ae50546..ec08e58 100644 --- a/__tests__/claude-usage.test.js +++ b/__tests__/claude-usage.test.js @@ -736,6 +736,23 @@ describe("POST /claude-usage — payload", () => { expect(JSON.parse(fs.readFileSync(usagePath, "utf-8")).claudeCodeVersion).toBe("2.1.284"); }); + test("500 when the rename fails after the temp file is written, and the temp file is removed", async () => { + // The target is a non-empty directory: mkdir and the temp write succeed, + // renameSync fails. Only the cleanup in writeClaudeUsage() removes the + // temp file (#54: the mkdir case below never reaches it). + fs.mkdirSync(path.join(usagePath, "occupied"), { recursive: true }); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + const res = await push(); + expect(res.status).toBe(500); + expect(res.body.error).toBe("Internal error"); + } finally { + error.mockRestore(); + } + expect(fs.readdirSync(path.dirname(usagePath))).toEqual(["claude-usage.json"]); + expect(fs.readdirSync(usagePath)).toEqual(["occupied"]); + }); + test("500 when the snapshot cannot be written, and no temp file is left", async () => { // The parent of the file is a regular file: mkdir fails, nothing lands. const blocker = path.join(tmpDir, "blocker"); @@ -842,6 +859,11 @@ describe("GET /claude-usage", () => { ["another schemaVersion", JSON.stringify({ ...validUsage(), schemaVersion: 2, receivedAt: "2026-09-27T19:00:00.000Z" })], ["no receivedAt", JSON.stringify(validUsage())], ["an unusable receivedAt", JSON.stringify({ ...validUsage(), receivedAt: "never" })], + // new Date() reads all three; the server only ever stamps a full ISO + // timestamp with an offset, so anything else was not written by it (#54). + ["a receivedAt of \"1\"", JSON.stringify({ ...validUsage(), receivedAt: "1" })], + ["a date-only receivedAt", JSON.stringify({ ...validUsage(), receivedAt: "2026-10-07" })], + ["a receivedAt without an offset", JSON.stringify({ ...validUsage(), receivedAt: "2026-10-07T12:00:00" })], ])("on %s", async (_label, raw) => { writeStored(raw); const res = await read(); diff --git a/__tests__/hosts.test.js b/__tests__/hosts.test.js index 8ed3212..a0bf8c8 100644 --- a/__tests__/hosts.test.js +++ b/__tests__/hosts.test.js @@ -869,6 +869,19 @@ describe("POST /hosts/ — what actually lands on disk", () => { expect(status).toBe(500); expect(body.error).toBe("Internal error"); }); + + test("500 when the rename fails after the temp file is written, and the temp file is removed", async () => { + // The target is a non-empty directory: mkdir and the temp write succeed, + // renameSync fails. Only the cleanup in writeHostSnapshot() removes the + // temp file (#54: the case above fails at mkdir, before it exists). + fs.mkdirSync(path.join(hostsDir, "thinkpad.json", "occupied"), { recursive: true }); + + const { status, body } = await ingest("/hosts/thinkpad"); + expect(status).toBe(500); + expect(body.error).toBe("Internal error"); + expect(fs.readdirSync(hostsDir)).toEqual(["thinkpad.json"]); + expect(fs.readdirSync(path.join(hostsDir, "thinkpad.json"))).toEqual(["occupied"]); + }); }); describe("GET /hosts — freshness boundary", () => { @@ -1010,6 +1023,14 @@ describe("GET /hosts — listing semantics", () => { "a receivedAt that is not a date", JSON.stringify({ id: "thinkpad", ...validSnapshot(), receivedAt: "yesterday" }), ], + // new Date() reads the next three; the server only ever stamps a full ISO + // timestamp with an offset, so anything else was not written by it (#54). + ['a receivedAt of "1"', JSON.stringify({ id: "thinkpad", ...validSnapshot(), receivedAt: "1" })], + ["a date-only receivedAt", JSON.stringify({ id: "thinkpad", ...validSnapshot(), receivedAt: "2026-10-07" })], + [ + "a receivedAt without an offset", + JSON.stringify({ id: "thinkpad", ...validSnapshot(), receivedAt: "2026-10-07T12:00:00" }), + ], [ "a receivedAt that is not a string", JSON.stringify({ id: "thinkpad", ...validSnapshot(), receivedAt: 1700000000000 }), diff --git a/claude-usage.js b/claude-usage.js index 7097c7c..bdeb207 100644 --- a/claude-usage.js +++ b/claude-usage.js @@ -68,6 +68,13 @@ function cleanTimestamp(value) { return value; } +// Also used by index.js on the stored receivedAt of both snapshot files: the +// server only ever stamps Date#toISOString(), and new Date() alone would +// accept "1" or a bare date (#54). +function isIsoTimestamp(value) { + return cleanTimestamp(value) !== undefined; +} + // The calendar check catches what the regex lets through (2026-02-31). function cleanDayDate(value) { if (typeof value !== "string" || !DAY_DATE_RE.test(value)) return undefined; @@ -176,6 +183,7 @@ function sanitizeClaudeUsage(payload) { module.exports = { sanitizeClaudeUsage, + isIsoTimestamp, CLAUDE_USAGE_MAX_BODY_BYTES, SCHEMA_VERSION, MAX_DAYS, diff --git a/index.js b/index.js index 8fb7769..97c0715 100644 --- a/index.js +++ b/index.js @@ -13,7 +13,7 @@ const { const path = require("node:path"); const { createHash, timingSafeEqual } = require("node:crypto"); const { collectMetrics } = require("./metrics.js"); -const { sanitizeClaudeUsage, CLAUDE_USAGE_MAX_BODY_BYTES } = require("./claude-usage.js"); +const { sanitizeClaudeUsage, isIsoTimestamp, CLAUDE_USAGE_MAX_BODY_BYTES } = require("./claude-usage.js"); const PORT = parseInt(process.env.PORT || "3001", 10); const TOKEN = process.env.HEALTH_TOKEN; @@ -500,7 +500,7 @@ function readHostSnapshot(id) { } const receivedAt = typeof parsed.receivedAt === "string" ? parsed.receivedAt : null; - if (!receivedAt || Number.isNaN(new Date(receivedAt).getTime())) { + if (!isIsoTimestamp(receivedAt)) { console.error(`[hosts] snapshot for ${id} has no usable receivedAt`); return null; } @@ -554,7 +554,7 @@ function readClaudeUsage() { } const receivedAt = typeof parsed.receivedAt === "string" ? parsed.receivedAt : null; - if (!receivedAt || Number.isNaN(new Date(receivedAt).getTime())) { + if (!isIsoTimestamp(receivedAt)) { console.error("[claude-usage] snapshot has no usable receivedAt"); return null; }