From 231fa5e952a8e3f021ffa3e55d98d5006bb6db86 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 02:48:45 +0000 Subject: [PATCH 1/2] fix: fall back to the default when PECANS_CACHE_MAX_AGE is not a number parseInt of a non-numeric value produced NaN, which flowed into the backend's cache-age comparison where every check is false - the release cache was fetched once and never refreshed again. Parse the env var through parseCacheMaxAge, which falls back to the 2 hour default for missing or non-numeric values. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01AtqCsX7ptP36t5rdyaJJhM --- src/index.ts | 17 ++++++++++++++--- test/unit/index.spec.ts | 24 +++++++++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/index.ts b/src/index.ts index c134adc..8b93d55 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,12 +15,23 @@ export * from "./pecans.js"; export * from "./service.js"; export * from "./utils/index.js"; +const DEFAULT_CACHE_MAX_AGE = 60 * 60 * 2; // 2 hours in seconds + +/** + * Parse the PECANS_CACHE_MAX_AGE env var (seconds). A missing or + * non-numeric value falls back to the default: a NaN would flow into the + * backend's cache-age comparison, where every check comes out false and + * the cache is never refreshed after the initial fetch. + */ +export function parseCacheMaxAge(value: string | undefined): number { + const parsed = Number.parseInt(value ?? "", 10); + return Number.isNaN(parsed) ? DEFAULT_CACHE_MAX_AGE : parsed; +} + export function configure() { const PECANS_BACKEND = process.env.PECANS_BACKEND || "PecansGithubBackend"; const basePath = process.env.PECANS_BASE_PATH || ""; - const cacheMaxAge = process.env.PECANS_CACHE_MAX_AGE - ? parseInt(process.env.PECANS_CACHE_MAX_AGE) - : 60 * 60 * 2; // Default 2 hours + const cacheMaxAge = parseCacheMaxAge(process.env.PECANS_CACHE_MAX_AGE); // enables POST /webhook/refresh (see README); without it the cache-bust // endpoint stays disabled const refreshSecret = process.env.PECANS_REFRESH_SECRET; diff --git a/test/unit/index.spec.ts b/test/unit/index.spec.ts index 92a33dc..b6b4c9f 100644 --- a/test/unit/index.spec.ts +++ b/test/unit/index.spec.ts @@ -1,6 +1,10 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { PecansGitHubBackend } from "../../src/backends/index.js"; -import { configure, parseTrustProxy } from "../../src/index.js"; +import { + configure, + parseCacheMaxAge, + parseTrustProxy, +} from "../../src/index.js"; describe("Index", () => { let originalEnv: NodeJS.ProcessEnv; @@ -106,6 +110,24 @@ describe("Index", () => { }); }); + describe("parseCacheMaxAge", () => { + it("parses numeric values as seconds", () => { + expect(parseCacheMaxAge("3600")).toBe(3600); + expect(parseCacheMaxAge("0")).toBe(0); + }); + + it("falls back to the 2 hour default when unset", () => { + expect(parseCacheMaxAge(undefined)).toBe(7200); + expect(parseCacheMaxAge("")).toBe(7200); + }); + + it("falls back to the 2 hour default for non-numeric values", () => { + // NaN would disable cache refreshes entirely: the backend's + // cacheAge > cacheMaxAgeMs check is always false against NaN + expect(parseCacheMaxAge("garbage")).toBe(7200); + }); + }); + describe("parseTrustProxy", () => { it("parses JSON scalars and arrays", () => { expect(parseTrustProxy("true")).toBe(true); From a08fe307ceb6801e4d2c861434b6ffe6a6021e59 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 02:58:49 +0000 Subject: [PATCH 2/2] fix: strict non-negative integer parsing for PECANS_CACHE_MAX_AGE Copilot review follow-up: parseInt-permissive values like '3600ms' or '-1' now fall back to the default instead of silently changing caching behavior (a negative age refreshes on every request), and the 2 hour default is shared from the backend module instead of duplicated. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01AtqCsX7ptP36t5rdyaJJhM --- src/backends/backend.ts | 2 +- src/index.ts | 21 ++++++++++++--------- test/unit/index.spec.ts | 8 ++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/backends/backend.ts b/src/backends/backend.ts index 69228f2..df762b3 100644 --- a/src/backends/backend.ts +++ b/src/backends/backend.ts @@ -7,7 +7,7 @@ import { ForbiddenError } from "../errors.js"; import type { PecansReleases } from "../models/index.js"; import type { PecansAssetDTO } from "../models/PecansAsset.js"; -const DEFAULT_CACHE_MAX_AGE = 60 * 60 * 2; // 2 hours in seconds +export const DEFAULT_CACHE_MAX_AGE = 60 * 60 * 2; // 2 hours in seconds export interface BackendOpts { refreshSecret?: string; diff --git a/src/index.ts b/src/index.ts index 8b93d55..f3ca8e3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,7 +5,10 @@ import express, { type Response, } from "express"; import { errorHandler } from "./errors.js"; -import { PecansGitHubBackend } from "./backends/index.js"; +import { + DEFAULT_CACHE_MAX_AGE, + PecansGitHubBackend, +} from "./backends/index.js"; import { Pecans, type PecansOptions } from "./pecans.js"; export * from "./backends/index.js"; @@ -15,17 +18,17 @@ export * from "./pecans.js"; export * from "./service.js"; export * from "./utils/index.js"; -const DEFAULT_CACHE_MAX_AGE = 60 * 60 * 2; // 2 hours in seconds - /** - * Parse the PECANS_CACHE_MAX_AGE env var (seconds). A missing or - * non-numeric value falls back to the default: a NaN would flow into the - * backend's cache-age comparison, where every check comes out false and - * the cache is never refreshed after the initial fetch. + * Parse the PECANS_CACHE_MAX_AGE env var (seconds). Anything but a + * non-negative integer falls back to the backend's default: a NaN would + * flow into the backend's cache-age comparison, where every check comes + * out false and the cache is never refreshed after the initial fetch, + * while parseInt-style permissive parsing would silently accept values + * like "3600ms" or "-1" (a negative age refreshes on every request). */ export function parseCacheMaxAge(value: string | undefined): number { - const parsed = Number.parseInt(value ?? "", 10); - return Number.isNaN(parsed) ? DEFAULT_CACHE_MAX_AGE : parsed; + if (!value || !/^\d+$/.test(value)) return DEFAULT_CACHE_MAX_AGE; + return Number.parseInt(value, 10); } export function configure() { diff --git a/test/unit/index.spec.ts b/test/unit/index.spec.ts index b6b4c9f..db335c3 100644 --- a/test/unit/index.spec.ts +++ b/test/unit/index.spec.ts @@ -126,6 +126,14 @@ describe("Index", () => { // cacheAge > cacheMaxAgeMs check is always false against NaN expect(parseCacheMaxAge("garbage")).toBe(7200); }); + + it("falls back for partially-numeric and negative values", () => { + // parseInt would read "3600ms" as 3600 and "-1" as -1; a negative + // age makes the cache look expired on every request + expect(parseCacheMaxAge("3600ms")).toBe(7200); + expect(parseCacheMaxAge("-1")).toBe(7200); + expect(parseCacheMaxAge("1.5")).toBe(7200); + }); }); describe("parseTrustProxy", () => {