diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index 52ab34d..f0068f8 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -4,11 +4,11 @@ # # Source repository: https://github.com/actions/dependency-review-action # Public documentation: https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-dependency-review#dependency-review-enforcement -name: 'Dependency Review' +name: "Dependency Review" -on: +on: pull_request: - branches: + branches: - main permissions: @@ -19,12 +19,12 @@ jobs: dependency-review: runs-on: ubuntu-latest steps: - - name: 'Checkout Repository' + - name: "Checkout Repository" uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd - - name: 'Dependency Review' + - name: "Dependency Review" uses: actions/dependency-review-action@2031cfc080254a8a887f58cffee85186f0e49e48 with: base-ref: ${{ github.event.pull_request.base.sha || 'main' }} head-ref: ${{ github.event.pull_request.head.sha || github.ref }} - comment-summary-in-pr: on-failure \ No newline at end of file + comment-summary-in-pr: on-failure diff --git a/src/app/index.ts b/src/app/index.ts index 6f483e3..61848f6 100644 --- a/src/app/index.ts +++ b/src/app/index.ts @@ -6,6 +6,7 @@ import versionsV1 from "../services/versions/v1"; import statsV1 from "../services/stats/v1"; import { platformMiddleware } from "../lib/middleware"; import { createCloudflareBindings } from "../lib/adapters/cloudflare"; +import { logError } from "../lib/logger"; const app = new Hono<{ Bindings: CloudflareBindings }>(); @@ -25,11 +26,14 @@ app.onError((err, c) => { if (err instanceof HTTPException) { return err.getResponse(); } + logError("app", "Unhandled request error", { + message: err instanceof Error ? err.message : String(err) + }); return c.json( { result: null, error: { - message: err.message || "Internal Server Error", + message: "Internal Server Error", code: "INTERNAL_SERVER_ERROR" } }, diff --git a/src/lib/cache.ts b/src/lib/cache.ts new file mode 100644 index 0000000..ebc1707 --- /dev/null +++ b/src/lib/cache.ts @@ -0,0 +1,12 @@ +import type { Context } from "hono"; + +export function normalizePublicCacheKey(url: string): string { + const cacheUrl = new URL(url); + cacheUrl.search = ""; + cacheUrl.hash = ""; + return cacheUrl.toString(); +} + +export function publicCacheKey(c: Context): string { + return normalizePublicCacheKey(c.req.url); +} diff --git a/src/services/central-alerts/v1/index.ts b/src/services/central-alerts/v1/index.ts index 421a9cc..1b2f687 100644 --- a/src/services/central-alerts/v1/index.ts +++ b/src/services/central-alerts/v1/index.ts @@ -3,6 +3,7 @@ import { cors } from "hono/cors"; import { trimTrailingSlash } from "hono/trailing-slash"; import { CentralAlertsDatabase } from "./database"; import { getPlatform } from "../../../lib/middleware"; +import { logError } from "../../../lib/logger"; const centralAlertsV1 = new Hono<{ Bindings: CloudflareBindings }>(); @@ -16,11 +17,15 @@ centralAlertsV1.get("/list", async (c) => { const { data, error } = await db.getAllAlerts(); if (error) { + logError("central-alerts", "Failed to list central alerts", { + message: error.message, + code: error.code + }); return c.json( { result: null, error: { - message: error.message, + message: "Unable to load central alerts", code: error.code || "DATABASE_ERROR" } }, diff --git a/src/services/stats/v1/index.ts b/src/services/stats/v1/index.ts index aba9686..9dd9073 100644 --- a/src/services/stats/v1/index.ts +++ b/src/services/stats/v1/index.ts @@ -10,6 +10,7 @@ import { Releases } from "../../versions/v1/interfaces"; import { StatsData, ReleasesPerYearData } from "./interfaces"; import { getPlatform } from "../../../lib/middleware"; import { ICache } from "../../../lib/interfaces"; +import { publicCacheKey } from "../../../lib/cache"; import { logError, logInfo } from "../../../lib/logger"; import { GitHubError } from "../../../lib/github-errors"; @@ -38,7 +39,8 @@ function registerCachedRoute

( path, cache({ cacheName: STATS_CACHE_NAME, - cacheControl: STATS_CACHE_CONTROL + cacheControl: STATS_CACHE_CONTROL, + keyGenerator: publicCacheKey }), etag(), prettyJSON(), diff --git a/src/services/versions/v1/index.ts b/src/services/versions/v1/index.ts index 691e2b5..25a831c 100644 --- a/src/services/versions/v1/index.ts +++ b/src/services/versions/v1/index.ts @@ -15,6 +15,7 @@ import { import { Releases, ReleaseDetails } from "./interfaces"; import { getPlatform } from "../../../lib/middleware"; import { ICache } from "../../../lib/interfaces"; +import { publicCacheKey } from "../../../lib/cache"; import { logError, logWarn, logInfo } from "../../../lib/logger"; import { GitHubError, @@ -34,8 +35,9 @@ const RELEASES_URL = "https://api.github.com/repos/FOSSBilling/FOSSBilling/releases"; type VersionsEnv = { Bindings: CloudflareBindings }; -// Cache for UPDATE_TOKEN to avoid repeated KV lookups -let updateTokenCache: string | null = null; +const UPDATE_TOKEN_CACHE_TTL_MS = 60_000; + +let updateTokenCache: { token: string; expiresAt: number } | null = null; const versionsV1 = new Hono(); @@ -48,8 +50,8 @@ versionsV1.use( ); async function getUpdateToken(cache: ICache): Promise { - if (updateTokenCache) { - return updateTokenCache; + if (updateTokenCache && updateTokenCache.expiresAt > Date.now()) { + return updateTokenCache.token; } const token = await cache.get("UPDATE_TOKEN"); @@ -58,7 +60,10 @@ async function getUpdateToken(cache: ICache): Promise { throw new Error("UPDATE_TOKEN not found in AUTH_KV storage"); } - updateTokenCache = token; + updateTokenCache = { + token, + expiresAt: Date.now() + UPDATE_TOKEN_CACHE_TTL_MS + }; return token; } @@ -71,7 +76,8 @@ function registerCachedRoute

( path, cache({ cacheName: RELEASES_CACHE_NAME, - cacheControl: RELEASES_CACHE_CONTROL + cacheControl: RELEASES_CACHE_CONTROL, + keyGenerator: publicCacheKey }), etag(), prettyJSON(), diff --git a/test/app/index.test.ts b/test/app/index.test.ts index bc9782e..966ae02 100644 --- a/test/app/index.test.ts +++ b/test/app/index.test.ts @@ -27,6 +27,7 @@ vi.mock("@octokit/request", () => ({ })); import { request as ghRequest } from "@octokit/request"; +import { resetUpdateTokenCache } from "../../src/services/versions/v1/index"; const mockGitHubReleases = [ { @@ -57,6 +58,7 @@ describe("FOSSBilling API Worker - Main App", () => { beforeEach(async () => { // Clear KV cache before each test await env.CACHE_KV.delete("gh-fossbilling-releases"); + resetUpdateTokenCache(); // Set up UPDATE_TOKEN in AUTH_KV storage for tests const testUpdateToken = "test-update-token-12345"; diff --git a/test/integration/app.test.ts b/test/integration/app.test.ts index 8302039..e16c012 100644 --- a/test/integration/app.test.ts +++ b/test/integration/app.test.ts @@ -20,10 +20,12 @@ vi.mock("@octokit/request", () => ({ })); import { request as ghRequest } from "@octokit/request"; +import { resetUpdateTokenCache } from "../../src/services/versions/v1/index"; describe("FOSSBilling API Worker - Full App Integration", () => { beforeEach(async () => { await env.CACHE_KV.delete("gh-fossbilling-releases"); + resetUpdateTokenCache(); await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); env.DB_CENTRAL_ALERTS = mockD1Database; diff --git a/test/integration/versions/index.test.ts b/test/integration/versions/index.test.ts index fc5e4bd..34d325f 100644 --- a/test/integration/versions/index.test.ts +++ b/test/integration/versions/index.test.ts @@ -21,10 +21,12 @@ vi.mock("@octokit/request", () => ({ })); import { request as ghRequest } from "@octokit/request"; +import { resetUpdateTokenCache } from "../../../src/services/versions/v1/index"; describe("Versions API v1 - Integration Tests", () => { beforeEach(async () => { await env.CACHE_KV.delete("gh-fossbilling-releases"); + resetUpdateTokenCache(); await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); vi.resetAllMocks(); diff --git a/test/lib/cache.test.ts b/test/lib/cache.test.ts new file mode 100644 index 0000000..6d02af5 --- /dev/null +++ b/test/lib/cache.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from "vitest"; +import { normalizePublicCacheKey } from "../../src/lib/cache"; + +describe("cache helpers", () => { + it("normalizes public cache keys by removing query strings and fragments", () => { + expect( + normalizePublicCacheKey( + "https://api.fossbilling.net/versions/v1/latest?cacheBust=1#section" + ) + ).toBe("https://api.fossbilling.net/versions/v1/latest"); + }); + + it("keeps different public paths isolated", () => { + expect( + normalizePublicCacheKey("https://api.fossbilling.net/versions/v1") + ).not.toBe( + normalizePublicCacheKey("https://api.fossbilling.net/versions/v1/count") + ); + }); +}); diff --git a/test/services/central-alerts/v1/index.test.ts b/test/services/central-alerts/v1/index.test.ts index a9bf832..df0d872 100644 --- a/test/services/central-alerts/v1/index.test.ts +++ b/test/services/central-alerts/v1/index.test.ts @@ -141,6 +141,30 @@ describe("Central Alerts API v1", () => { }); describe("Error Cases", () => { + it("should not expose database exception details", async () => { + env.DB_CENTRAL_ALERTS = { + prepare() { + throw new Error("secret schema detail"); + } + } as unknown as D1Database; + + const ctx = createExecutionContext(); + const response = await app.request( + "/central-alerts/v1/list", + {}, + env, + ctx + ); + await waitOnExecutionContext(ctx); + + expect(response.status).toBe(500); + const data = (await response.json()) as { + error: { message: string; code: string }; + }; + expect(data.error.message).toBe("Unable to load central alerts"); + expect(data.error.message).not.toContain("secret schema detail"); + }); + it("should return 404 for unknown routes", async () => { const ctx = createExecutionContext(); const response = await app.request( diff --git a/test/services/versions/v1/errors.test.ts b/test/services/versions/v1/errors.test.ts index 161726f..8f659d2 100644 --- a/test/services/versions/v1/errors.test.ts +++ b/test/services/versions/v1/errors.test.ts @@ -22,6 +22,7 @@ vi.mock("@octokit/request", () => ({ })); import { request as ghRequest } from "@octokit/request"; +import { resetUpdateTokenCache } from "../../../../src/services/versions/v1/index"; let restoreConsole: (() => void) | null = null; @@ -29,6 +30,7 @@ describe("Versions API v1 - Error Handling", () => { beforeEach(async () => { restoreConsole = suppressConsole(); await env.CACHE_KV.delete("gh-fossbilling-releases"); + resetUpdateTokenCache(); await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); vi.resetAllMocks(); @@ -71,6 +73,11 @@ describe("Versions API v1 - Error Handling", () => { await waitOnExecutionContext(ctx); expect(response.status).toBe(500); + const data = (await response.json()) as { + error: { message: string; code: string }; + }; + expect(data.error.message).toBe("Internal Server Error"); + expect(data.error.message).not.toContain("UPDATE_TOKEN"); }); it("should reject malformed Authorization headers", async () => { diff --git a/test/services/versions/v1/index.test.ts b/test/services/versions/v1/index.test.ts index 2781308..f90947d 100644 --- a/test/services/versions/v1/index.test.ts +++ b/test/services/versions/v1/index.test.ts @@ -334,6 +334,75 @@ describe("Versions API v1", () => { expect(response.status).toBe(401); }); + + it("should stop accepting a rotated token after the token cache TTL", async () => { + const now = Date.now(); + const dateNow = vi.spyOn(Date, "now").mockReturnValue(now); + + try { + const ctx1 = createExecutionContext(); + const response1 = await app.request( + "/versions/v1/update", + { + headers: { + Authorization: "Bearer test-update-token-12345" + } + }, + env, + ctx1 + ); + await waitOnExecutionContext(ctx1); + expect(response1.status).toBe(200); + + await env.AUTH_KV.put("UPDATE_TOKEN", "rotated-update-token-12345"); + + const ctx2 = createExecutionContext(); + const response2 = await app.request( + "/versions/v1/update", + { + headers: { + Authorization: "Bearer test-update-token-12345" + } + }, + env, + ctx2 + ); + await waitOnExecutionContext(ctx2); + expect(response2.status).toBe(200); + + dateNow.mockReturnValue(now + 60_001); + + const ctx3 = createExecutionContext(); + const response3 = await app.request( + "/versions/v1/update", + { + headers: { + Authorization: "Bearer test-update-token-12345" + } + }, + env, + ctx3 + ); + await waitOnExecutionContext(ctx3); + expect(response3.status).toBe(401); + + const ctx4 = createExecutionContext(); + const response4 = await app.request( + "/versions/v1/update", + { + headers: { + Authorization: "Bearer rotated-update-token-12345" + } + }, + env, + ctx4 + ); + await waitOnExecutionContext(ctx4); + expect(response4.status).toBe(200); + } finally { + dateNow.mockRestore(); + } + }); }); describe("Error Handling", () => { diff --git a/test/services/versions/v1/middleware.test.ts b/test/services/versions/v1/middleware.test.ts index 330b19d..2acfb71 100644 --- a/test/services/versions/v1/middleware.test.ts +++ b/test/services/versions/v1/middleware.test.ts @@ -20,6 +20,7 @@ vi.mock("@octokit/request", () => ({ })); import { request as ghRequest } from "@octokit/request"; +import { resetUpdateTokenCache } from "../../../../src/services/versions/v1/index"; let restoreConsole: (() => void) | null = null; @@ -27,6 +28,7 @@ describe("Versions API v1 - Middleware", () => { beforeEach(async () => { restoreConsole = suppressConsole(); await env.CACHE_KV.delete("gh-fossbilling-releases"); + resetUpdateTokenCache(); await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); vi.clearAllMocks();