diff --git a/.github/workflows/dependabot-to-linear.yml b/.github/workflows/dependabot-to-linear.yml new file mode 100644 index 000000000000..b48f0653145b --- /dev/null +++ b/.github/workflows/dependabot-to-linear.yml @@ -0,0 +1,43 @@ +name: Dependabot Alerts to Linear + +on: + schedule: + - cron: "0 11 * * *" # 11:00 UTC = 12:00 CET (noon); +1h during CEST + workflow_dispatch: {} + +permissions: + contents: read + +concurrency: + group: dependabot-to-linear-sync + cancel-in-progress: false + +jobs: + sync: + name: Sync Dependabot alerts to Linear + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Harden the runner + uses: step-security/harden-runner@ec9f2d5744a09debf3a187a3f4f675c53b671911 # v2.13.0 + with: + egress-policy: audit + + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + + - name: Setup Node + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: 24 + + - name: Sync alerts to Linear + env: + DEPENDABOT_ALERTS_TOKEN: ${{ secrets.DEPENDABOT_ALERTS_TOKEN }} + LINEAR_API_KEY: ${{ secrets.LINEAR_API_KEY }} + # Fallback used by the script if LINEAR_API_KEY is not set; must be + # listed here because the job only sees secrets exposed via env. + LINEAR_ACCESS_KEY: ${{ secrets.LINEAR_ACCESS_KEY }} + run: node scripts/dependabot-to-linear.mjs diff --git a/apps/web/app/api/v1/client/[workspaceId]/environment/route.ts b/apps/web/app/api/v1/client/[workspaceId]/environment/route.ts index e7c8a359e633..9e2509fbdf08 100644 --- a/apps/web/app/api/v1/client/[workspaceId]/environment/route.ts +++ b/apps/web/app/api/v1/client/[workspaceId]/environment/route.ts @@ -60,6 +60,18 @@ export const GET = withV1ApiWrapper({ // Use optimized environment state fetcher with new caching approach const workspace = await getWorkspaceState(workspaceId); + + // Guard against unexpected empty state before destructuring. + if (!workspace?.data) { + logger.error( + { workspaceId, url: req.url }, + "getWorkspaceState returned unexpected null/empty payload" + ); + return { + response: responses.notFoundResponse("Workspace", workspaceId), + }; + } + const { data } = workspace; return { diff --git a/apps/web/lib/cache/index.test.ts b/apps/web/lib/cache/index.test.ts index 26b7f85cf7d9..1e35623e21af 100644 --- a/apps/web/lib/cache/index.test.ts +++ b/apps/web/lib/cache/index.test.ts @@ -9,6 +9,7 @@ const mockCacheService = { del: vi.fn(), exists: vi.fn(), withCache: vi.fn(), + withCacheNullable: vi.fn(), getRedisClient: vi.fn(), }; @@ -219,6 +220,31 @@ describe("Cache Index", () => { expect(result).toBe("cached-result"); }); + test("should proxy withCacheNullable method correctly when cache is available", async () => { + const mockFn = vi.fn().mockResolvedValue(null); + mockCacheService.withCacheNullable.mockResolvedValue(null); + + const result = await cache.withCacheNullable(mockFn, "cache-key" as CacheKey, 3000); + + expect(mockCacheService.withCacheNullable).toHaveBeenCalledWith(mockFn, "cache-key" as CacheKey, 3000); + expect(result).toBeNull(); + }); + + test("should execute nullable function directly when cache service fails", async () => { + const mockFn = vi.fn().mockResolvedValue("function-result"); + + mockGetCacheService.mockResolvedValue({ + ok: false, + error: { code: "CACHE_UNAVAILABLE" }, + }); + + const result = await cache.withCacheNullable(mockFn, "cache-key" as CacheKey, 3000); + + expect(result).toBe("function-result"); + expect(mockFn).toHaveBeenCalledTimes(1); + expect(mockCacheService.withCacheNullable).not.toHaveBeenCalled(); + }); + test("should execute function directly when cache service fails", async () => { const mockFn = vi.fn().mockResolvedValue("function-result"); diff --git a/apps/web/lib/cache/index.ts b/apps/web/lib/cache/index.ts index 70ece608dcf3..3ebec8ca8132 100644 --- a/apps/web/lib/cache/index.ts +++ b/apps/web/lib/cache/index.ts @@ -11,7 +11,12 @@ type CacheService = { set(key: string, value: unknown, ttlMs?: number): Promise>; del(keys: string[]): Promise>; tryLock(key: string, value: string, ttlMs: number): Promise>; - withCache(fn: () => Promise, key: string, ttlMs: number): Promise; + withCache>(fn: () => Promise, key: string, ttlMs: number): Promise; + withCacheNullable>( + fn: () => Promise, + key: string, + ttlMs: number + ): Promise; getRedisClient(): RedisClientType | null; }; @@ -31,7 +36,7 @@ export const cache = new Proxy({} as AsyncCacheService, { get(_target, prop: keyof CacheService) { // Special-case: withCache must never fail; fall back to direct fn on init failure. if (prop === "withCache") { - return async (fn: () => Promise, ...rest: [string, number]) => { + return async >(fn: () => Promise, ...rest: [string, number]) => { try { const cacheServiceResult = await getCacheService(); @@ -48,6 +53,27 @@ export const cache = new Proxy({} as AsyncCacheService, { }; } + if (prop === "withCacheNullable") { + return async >( + fn: () => Promise, + ...rest: [string, number] + ) => { + try { + const cacheServiceResult = await getCacheService(); + + if (!cacheServiceResult.ok) { + return await fn(); + } + + const cacheService = cacheServiceResult.data as CacheService; + return cacheService.withCacheNullable(fn, ...rest); + } catch (error) { + logger.warn({ error }, "Cache unavailable; executing function directly"); + return await fn(); + } + }; + } + if (prop === "getRedisClient") { return async () => { const cacheServiceResult = await getCacheService(); diff --git a/apps/web/modules/ee/billing/lib/organization-billing.test.ts b/apps/web/modules/ee/billing/lib/organization-billing.test.ts index aa2f8cf6afaa..6b1bbe77dc30 100644 --- a/apps/web/modules/ee/billing/lib/organization-billing.test.ts +++ b/apps/web/modules/ee/billing/lib/organization-billing.test.ts @@ -23,6 +23,7 @@ const mocks = vi.hoisted(() => ({ prismaOrganizationBillingUpsert: vi.fn(), prismaOrganizationBillingUpdate: vi.fn(), cacheWithCache: vi.fn(), + cacheWithCacheNullable: vi.fn(), cacheDel: vi.fn(), loggerWarn: vi.fn(), getCloudPlanFromProduct: vi.fn(), @@ -88,6 +89,7 @@ vi.mock("@formbricks/database", () => ({ vi.mock("@/lib/cache", () => ({ cache: { withCache: mocks.cacheWithCache, + withCacheNullable: mocks.cacheWithCacheNullable, del: mocks.cacheDel, }, })); @@ -156,6 +158,7 @@ describe("organization-billing", () => { [namespace, identifier, subresource].filter(Boolean).join(":") ); mocks.cacheWithCache.mockImplementation(async (fn: () => Promise) => await fn()); + mocks.cacheWithCacheNullable.mockImplementation(async (fn: () => Promise) => await fn()); mocks.getCloudPlanFromProduct.mockReturnValue("pro"); mocks.subscriptionsList.mockResolvedValue({ data: [] }); mocks.customersList.mockResolvedValue({ data: [] }); @@ -1761,7 +1764,7 @@ describe("organization-billing", () => { }, usageCycleAnchor: new Date().toISOString(), }; - mocks.cacheWithCache.mockResolvedValue(cachedBilling); + mocks.cacheWithCacheNullable.mockResolvedValue(cachedBilling); const result = await getOrganizationBillingWithReadThroughSync("org_1"); @@ -1774,7 +1777,7 @@ describe("organization-billing", () => { stripeCustomerId: "cus_1", stripe: { lastSyncedAt: new Date().toISOString() }, }; - mocks.cacheWithCache.mockResolvedValue(cachedBilling); + mocks.cacheWithCacheNullable.mockResolvedValue(cachedBilling); const result = await getOrganizationBillingWithReadThroughSync("org_1"); @@ -1787,7 +1790,7 @@ describe("organization-billing", () => { stripeCustomerId: "cus_1", stripe: { lastSyncedAt: new Date(Date.now() - 6 * 60 * 1000).toISOString() }, }; - mocks.cacheWithCache.mockResolvedValue(cachedBilling); + mocks.cacheWithCacheNullable.mockResolvedValue(cachedBilling); mocks.prismaOrganizationBillingFindUnique.mockResolvedValue({ stripeCustomerId: "cus_1", limits: { @@ -1826,7 +1829,7 @@ describe("organization-billing", () => { const result = await getOrganizationBillingWithReadThroughSync("org_1"); - expect(mocks.cacheWithCache).not.toHaveBeenCalled(); + expect(mocks.cacheWithCacheNullable).not.toHaveBeenCalled(); expect(result).toEqual({ stripeCustomerId: null, limits: { @@ -1841,7 +1844,7 @@ describe("organization-billing", () => { test("getOrganizationBillingWithReadThroughSync returns null when organization billing is missing", async () => { mocks.prismaOrganizationBillingFindUnique.mockResolvedValue(null); - mocks.cacheWithCache.mockImplementation(async (fn: () => Promise) => await fn()); + mocks.cacheWithCacheNullable.mockImplementation(async (fn: () => Promise) => await fn()); await expect(getOrganizationBillingWithReadThroughSync("org_1")).resolves.toBeNull(); }); diff --git a/apps/web/modules/ee/billing/lib/organization-billing.ts b/apps/web/modules/ee/billing/lib/organization-billing.ts index 79e19ce63e7c..5f7a33787e1d 100644 --- a/apps/web/modules/ee/billing/lib/organization-billing.ts +++ b/apps/web/modules/ee/billing/lib/organization-billing.ts @@ -1218,7 +1218,7 @@ export const getOrganizationBillingWithReadThroughSync = async ( return await getOrganizationBillingFromDatabase(organizationId); } - const cachedBilling = await cache.withCache( + const cachedBilling = await cache.withCacheNullable( async () => await getOrganizationBillingFromDatabase(organizationId), getBillingCacheKey(organizationId), BILLING_SYNC_STALE_MS diff --git a/apps/web/modules/ee/contacts/components/upload-contacts-button.tsx b/apps/web/modules/ee/contacts/components/upload-contacts-button.tsx index b110c49f0b07..d5b519770c3a 100644 --- a/apps/web/modules/ee/contacts/components/upload-contacts-button.tsx +++ b/apps/web/modules/ee/contacts/components/upload-contacts-button.tsx @@ -1,6 +1,5 @@ "use client"; -import { parse } from "csv-parse/sync"; import { ArrowUpFromLineIcon, FileUpIcon, PlusIcon } from "lucide-react"; import { useRouter } from "next/navigation"; import { useEffect, useMemo, useRef, useState } from "react"; @@ -17,6 +16,7 @@ import { RESERVED_FUTURE_DEFAULT_ATTRIBUTE_SAFE_IDENTIFIER_KEYS_TEXT, isReservedFutureDefaultAttributeKey, } from "@/modules/ee/contacts/lib/attribute-key-policy"; +import { parseContactsCSV } from "@/modules/ee/contacts/lib/parse-contacts-csv"; import { TContactCSVUploadResponse, ZContactCSVUploadResponse } from "@/modules/ee/contacts/types/contact"; import { Alert } from "@/modules/ui/components/alert"; import { Button } from "@/modules/ui/components/button"; @@ -80,10 +80,7 @@ export const UploadContactsCSVButton = ({ const csv = e.target?.result as string; try { - const records = parse(csv, { - columns: true, // Parse the header as column names - skip_empty_lines: true, // Skip empty lines - }); + const records = parseContactsCSV(csv); const parsedRecords = ZContactCSVUploadResponse.safeParse(records); if (!parsedRecords.success) { diff --git a/apps/web/modules/ee/contacts/lib/parse-contacts-csv.test.ts b/apps/web/modules/ee/contacts/lib/parse-contacts-csv.test.ts new file mode 100644 index 000000000000..14bae7b3fe42 --- /dev/null +++ b/apps/web/modules/ee/contacts/lib/parse-contacts-csv.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, test } from "vitest"; +import { parseContactsCSV } from "./parse-contacts-csv"; + +describe("parseContactsCSV", () => { + const expected = [ + { email: "user1@example.com", user_id: "1001", first_name: "John" }, + { email: "user2@example.com", user_id: "1002", first_name: "Jane" }, + ]; + + test("parses comma-delimited CSV", () => { + const csv = `email,user_id,first_name +user1@example.com,1001,John +user2@example.com,1002,Jane`; + expect(parseContactsCSV(csv)).toEqual(expected); + }); + + test("parses semicolon-delimited CSV (EU Excel default)", () => { + const csv = `email;user_id;first_name +user1@example.com;1001;John +user2@example.com;1002;Jane`; + expect(parseContactsCSV(csv)).toEqual(expected); + }); + + test("parses tab-delimited CSV", () => { + const csv = `email\tuser_id\tfirst_name +user1@example.com\t1001\tJohn +user2@example.com\t1002\tJane`; + expect(parseContactsCSV(csv)).toEqual(expected); + }); + + test("skips empty lines", () => { + const csv = `email,user_id,first_name + +user1@example.com,1001,John + +user2@example.com,1002,Jane +`; + expect(parseContactsCSV(csv)).toEqual(expected); + }); + + test("returns empty array for header-only CSV", () => { + expect(parseContactsCSV("email,user_id,first_name")).toEqual([]); + }); + + test("throws on malformed CSV with inconsistent column count", () => { + const csv = `email,user_id,first_name +user1@example.com,1001`; + expect(() => parseContactsCSV(csv)).toThrow(); + }); +}); diff --git a/apps/web/modules/ee/contacts/lib/parse-contacts-csv.ts b/apps/web/modules/ee/contacts/lib/parse-contacts-csv.ts new file mode 100644 index 000000000000..1c826b1aca8a --- /dev/null +++ b/apps/web/modules/ee/contacts/lib/parse-contacts-csv.ts @@ -0,0 +1,10 @@ +import { parse } from "csv-parse/sync"; + +export const parseContactsCSV = (csv: string): unknown[] => { + return parse(csv, { + bom: true, + columns: true, + skip_empty_lines: true, + delimiter: [",", ";", "\t"], + }); +}; diff --git a/packages/cache/src/cache-integration.test.ts b/packages/cache/src/cache-integration.test.ts index ca93d12fbb1c..811eb636df3b 100644 --- a/packages/cache/src/cache-integration.test.ts +++ b/packages/cache/src/cache-integration.test.ts @@ -432,11 +432,11 @@ describe("Cache Integration Tests - End-to-End Redis Operations", () => { return; } + // Null serialization is covered below via withCacheNullable. const testCases = [ { name: "string", value: "Hello, World!" }, { name: "number", value: 42.5 }, { name: "boolean", value: true }, - { name: "null", value: null }, { name: "array", value: [1, "two", { three: 3 }, null, true] }, { name: "object", @@ -506,6 +506,29 @@ describe("Cache Integration Tests - End-to-End Redis Operations", () => { logger.info(`✅ ${testCase.name} serialization successful`); } + // Raw cache.set(null) should be a nullable-cache miss. + const nullKey = createCacheKey.workspace.state("serialization-null"); + await cacheService.del([nullKey]); + const nullSetResult = await cacheService.set(nullKey, null, 30000); + expect(nullSetResult.ok).toBe(true); + const nullGetResult = await cacheService.get(nullKey); + expect(nullGetResult.ok).toBe(true); + if (nullGetResult.ok) { + expect(nullGetResult.data).toBeNull(); + } + let nullFnCalled = false; + const nullCached = await cacheService.withCacheNullable( + async () => { + nullFnCalled = true; + return "should-not-be-returned"; + }, + nullKey, + 30000 + ); + expect(nullFnCalled).toBe(true); + expect(nullCached).toBe("should-not-be-returned"); + logger.info("✅ withCacheNullable correctly ignores entries written via raw cache.set(null)"); + logger.info("✅ All data types serialized correctly"); }, 20000); @@ -547,4 +570,44 @@ describe("Cache Integration Tests - End-to-End Redis Operations", () => { logger.info("✅ Error handling tests completed successfully"); }, 15000); + + test("withCacheNullable: round-trips real values and cached nulls without re-executing fn", async () => { + if (!isRedisAvailable || !cacheService) { + logger.info("Skipping test: Redis not available"); + return; + } + + const realKey = createCacheKey.workspace.state("nullable-real"); + const nullKey = createCacheKey.workspace.state("nullable-null"); + let realCalls = 0; + let nullCalls = 0; + + const realFn = async (): Promise<{ id: string } | null> => { + realCalls++; + return { id: `real-${realCalls}` }; + }; + const nullFn = async (): Promise<{ id: string } | null> => { + nullCalls++; + return null; + }; + + await cacheService.del([realKey, nullKey]); + + const realFirst = await cacheService.withCacheNullable(realFn, realKey, 60000); + const nullFirst = await cacheService.withCacheNullable(nullFn, nullKey, 60000); + expect(realFirst).toEqual({ id: "real-1" }); + expect(nullFirst).toBeNull(); + expect(realCalls).toBe(1); + expect(nullCalls).toBe(1); + + const realSecond = await cacheService.withCacheNullable(realFn, realKey, 60000); + const nullSecond = await cacheService.withCacheNullable(nullFn, nullKey, 60000); + expect(realSecond).toEqual({ id: "real-1" }); + expect(nullSecond).toBeNull(); + expect(realCalls).toBe(1); + expect(nullCalls).toBe(1); + + await cacheService.del([realKey, nullKey]); + logger.info("✅ withCacheNullable round-trip confirmed for real and null values"); + }, 15000); }); diff --git a/packages/cache/src/service.test.ts b/packages/cache/src/service.test.ts index 49b40f6a3272..96e2a4218d78 100644 --- a/packages/cache/src/service.test.ts +++ b/packages/cache/src/service.test.ts @@ -272,7 +272,7 @@ describe("CacheService", () => { expect(mockRedis.setEx).toHaveBeenCalledWith(key, 5, JSON.stringify(value)); }); - test("should normalize undefined to null and store as JSON", async () => { + test("should warn and skip the write when value is undefined", async () => { const key = "test:key" as CacheKey; const value = undefined; const ttlMs = 60000; @@ -280,7 +280,11 @@ describe("CacheService", () => { const result = await cacheService.set(key, value, ttlMs); expect(result.ok).toBe(true); - expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, "null"); + expect(mockRedis.setEx).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith( + { key, ttlMs }, + "cache.set called with undefined; skipping write" + ); }); test("should store null values as JSON", async () => { @@ -596,77 +600,178 @@ describe("CacheService", () => { expect(fn).toHaveBeenCalledOnce(); }); - test("should return cached null value without executing function", async () => { + test("should treat stringified null as a cache miss and re-execute fn", async () => { const key = "test:key" as CacheKey; - const fn = vi.fn().mockResolvedValue({ data: "fresh" }); + const freshValue = { data: "fresh" }; + const fn = vi.fn().mockResolvedValue(freshValue); - // Mock Redis returning stringified null (cached null value) + // Bare JSON null is a miss for non-null cache-aside calls. mockRedis.get.mockResolvedValue("null"); - mockRedis.exists.mockResolvedValue(1); // Key exists const result = await cacheService.withCache(fn, key, 60000); - expect(result).toBeNull(); - expect(fn).not.toHaveBeenCalled(); // Function should not be executed + expect(result).toEqual(freshValue); + expect(fn).toHaveBeenCalledOnce(); + expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, JSON.stringify(freshValue)); }); - test("should execute function and cache null result", async () => { + test("should not surface stale null under the original GET/EXISTS race window", async () => { + // Simulates the removed GET/EXISTS race from ENG-1047. const key = "test:key" as CacheKey; - const fn = vi.fn().mockResolvedValue(null); // Function returns null + const freshValue = { data: "fresh-from-fn" }; + const fn = vi.fn().mockResolvedValue(freshValue); - // Mock cache miss mockRedis.get.mockResolvedValue(null); - mockRedis.exists.mockResolvedValue(0); // Key doesn't exist + mockRedis.exists.mockResolvedValue(1); const result = await cacheService.withCache(fn, key, 60000); - expect(result).toBeNull(); + expect(result).toEqual(freshValue); expect(fn).toHaveBeenCalledOnce(); - expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, "null"); + expect(mockRedis.exists).not.toHaveBeenCalled(); }); - test("should return undefined without caching when function returns undefined", async () => { + test("should not persist null returned via type erasure (no recompute stampede)", async () => { + // withCache is constrained to NonNullable, but a caller can + // still bypass that through unknown/any plumbing or an erased generic. + // If null were written, the read path would treat it as a perpetual miss, + // re-running fn() and rewriting "null" on every request for a hot key. const key = "test:key" as CacheKey; - const fn = vi.fn().mockResolvedValue(undefined); // Function returns undefined + const fn = vi.fn().mockResolvedValue(null); - // Mock cache miss - mockRedis.get.mockResolvedValue(null); - mockRedis.exists.mockResolvedValue(0); // Key doesn't exist + mockRedis.get.mockResolvedValue(null); // cache miss - const result = await cacheService.withCache(fn, key, 60000); + const result = await cacheService.withCache( + fn as unknown as () => Promise<{ data: string }>, + key, + 60000 + ); - expect(result).toBeUndefined(); + expect(result).toBeNull(); expect(fn).toHaveBeenCalledOnce(); - // undefined should NOT be cached to preserve semantics expect(mockRedis.setEx).not.toHaveBeenCalled(); }); + }); - test("should distinguish between null and undefined return values", async () => { - const nullKey = "test:null-key" as CacheKey; - const undefinedKey = "test:undefined-key" as CacheKey; + describe("withCacheNullable", () => { + // Keep the nullable wire format explicit in assertions. + const box = (value: unknown): string => JSON.stringify({ __fb_nullable_v1: true, value }); - const nullFn = vi.fn().mockResolvedValue(null); - const undefinedFn = vi.fn().mockResolvedValue(undefined); + test("should return cached null without executing fn when boxed null is stored", async () => { + const key = "test:nullable-key" as CacheKey; + const fn = vi.fn().mockResolvedValue({ id: "fresh" }); + + mockRedis.get.mockResolvedValue(box(null)); + + const result = await cacheService.withCacheNullable(fn, key, 60000); + + expect(result).toBeNull(); + expect(fn).not.toHaveBeenCalled(); + }); + + test("should compute, box, and cache null on miss", async () => { + const key = "test:nullable-key" as CacheKey; + const fn = vi.fn().mockResolvedValue(null); - // Mock cache miss for both keys mockRedis.get.mockResolvedValue(null); - mockRedis.exists.mockResolvedValue(0); - // Test null return value - should be cached - const nullResult = await cacheService.withCache(nullFn, nullKey, 60000); - expect(nullResult).toBeNull(); - expect(nullFn).toHaveBeenCalledOnce(); - expect(mockRedis.setEx).toHaveBeenCalledWith(nullKey, 60, "null"); + const result = await cacheService.withCacheNullable(fn, key, 60000); + + expect(result).toBeNull(); + expect(fn).toHaveBeenCalledOnce(); + expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, box(null)); + }); + + test("should compute, box, and cache a real value on miss", async () => { + const key = "test:nullable-key" as CacheKey; + const freshValue = { id: "real" }; + const fn = vi.fn().mockResolvedValue(freshValue); - // Reset mocks - vi.clearAllMocks(); mockRedis.get.mockResolvedValue(null); - mockRedis.exists.mockResolvedValue(0); - // Test undefined return value - should NOT be cached - const undefinedResult = await cacheService.withCache(undefinedFn, undefinedKey, 60000); - expect(undefinedResult).toBeUndefined(); - expect(undefinedFn).toHaveBeenCalledOnce(); + const result = await cacheService.withCacheNullable(fn, key, 60000); + + expect(result).toEqual(freshValue); + expect(fn).toHaveBeenCalledOnce(); + expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, box(freshValue)); + }); + + test("should return boxed cached value without executing fn", async () => { + const key = "test:nullable-key" as CacheKey; + const cachedValue = { id: "cached" }; + const fn = vi.fn().mockResolvedValue({ id: "fresh" }); + + mockRedis.get.mockResolvedValue(box(cachedValue)); + + const result = await cacheService.withCacheNullable(fn, key, 60000); + + expect(result).toEqual(cachedValue); + expect(fn).not.toHaveBeenCalled(); + }); + + test("should treat unboxed legacy values as a cache miss and overwrite with boxed value", async () => { + const key = "test:nullable-key" as CacheKey; + const legacyCachedValue = { id: "legacy" }; + const freshValue = { id: "fresh" }; + const fn = vi.fn().mockResolvedValue(freshValue); + + mockRedis.get.mockResolvedValue(JSON.stringify(legacyCachedValue)); + + const result = await cacheService.withCacheNullable(fn, key, 60000); + + expect(result).toEqual(freshValue); + expect(fn).toHaveBeenCalledOnce(); + expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, box(freshValue)); + expect(logger.debug).toHaveBeenCalledWith( + { key, ttlMs: 60000 }, + "Nullable cache entry is missing marker or value field; treating as cache miss and refreshing" + ); + }); + + test("should treat a marker-less object with a `value` field as a cache miss", async () => { + const key = "test:nullable-key" as CacheKey; + const collidingShape = { value: { id: "colliding" }, otherField: 123 }; + const freshValue = { id: "fresh" }; + const fn = vi.fn().mockResolvedValue(freshValue); + + mockRedis.get.mockResolvedValue(JSON.stringify(collidingShape)); + + const result = await cacheService.withCacheNullable(fn, key, 60000); + + expect(result).toEqual(freshValue); + expect(fn).toHaveBeenCalledOnce(); + expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, box(freshValue)); + }); + + test("should treat a marker-only nullable envelope as a cache miss", async () => { + const key = "test:nullable-key" as CacheKey; + const malformedBox = { __fb_nullable_v1: true }; + const freshValue = { id: "fresh" }; + const fn = vi.fn().mockResolvedValue(freshValue); + + mockRedis.get.mockResolvedValue(JSON.stringify(malformedBox)); + + const result = await cacheService.withCacheNullable(fn, key, 60000); + + expect(result).toEqual(freshValue); + expect(fn).toHaveBeenCalledOnce(); + expect(mockRedis.setEx).toHaveBeenCalledWith(key, 60, box(freshValue)); + }); + + test("should not cache undefined returned by a nullable callback", async () => { + const key = "test:nullable-key" as CacheKey; + const fn = vi.fn().mockResolvedValue(undefined); + + mockRedis.get.mockResolvedValue(null); + + const result = await cacheService.withCacheNullable( + fn as unknown as () => Promise<{ id: string } | null>, + key, + 60000 + ); + + expect(result).toBeUndefined(); + expect(fn).toHaveBeenCalledOnce(); expect(mockRedis.setEx).not.toHaveBeenCalled(); }); diff --git a/packages/cache/src/service.ts b/packages/cache/src/service.ts index fac0763dfe36..9b359eb85d89 100644 --- a/packages/cache/src/service.ts +++ b/packages/cache/src/service.ts @@ -6,6 +6,14 @@ import { ZCacheKey } from "@/types/keys"; import { ZTtlMs, ZTtlMsOptional } from "@/types/service"; import { validateInputs } from "./utils/validation"; +/** Marker for the nullable cache wire format. */ +const NULLABLE_BOX_MARKER = "__fb_nullable_v1"; + +interface NullableCacheBox { + [NULLABLE_BOX_MARKER]: true; + value: T | null; +} + /** * Core cache service providing basic Redis operations with JSON serialization */ @@ -87,7 +95,8 @@ export class CacheService { } /** - * Check if a key exists in cache (for distinguishing cache miss from cached null) + * Check if a key exists in cache. + * Prefer single-GET cache-aside semantics in withCache to avoid TOCTOU races. * @param key - Cache key to check * @returns Result containing boolean indicating if key exists */ @@ -136,10 +145,14 @@ export class CacheService { return validation; } + // Undefined is a caller bug, not a cacheable null. + if (value === undefined) { + logger.warn({ key, ttlMs }, "cache.set called with undefined; skipping write"); + return ok(undefined); + } + try { - // Normalize undefined to null to maintain consistent cached-null semantics - const normalizedValue = value === undefined ? null : value; - const serialized = JSON.stringify(normalizedValue); + const serialized = JSON.stringify(value); if (ttlMs === undefined) { // Set without expiration (persists indefinitely) @@ -231,21 +244,21 @@ export class CacheService { /** * Cache wrapper for functions (cache-aside). + * + * Bare JSON null is treated as a miss; use withCacheNullable for intentional null values. * Never throws due to cache errors; function errors propagate without retry. - * Must include null in T to support cached null values. + * * @param fn - Function to execute (and optionally cache). * @param key - Cache key * @param ttlMs - Time to live in milliseconds * @returns Cached value if present, otherwise fresh result from fn() */ - async withCache(fn: () => Promise, key: CacheKey, ttlMs: number): Promise { - if (!this.isRedisClientReady()) { - return await fn(); - } - - const validation = validateInputs([key, ZCacheKey], [ttlMs, ZTtlMs]); - if (!validation.ok) { - logger.warn({ error: validation.error, key }, "Invalid cache inputs, executing function directly"); + async withCache>( + fn: () => Promise, + key: CacheKey, + ttlMs: number + ): Promise { + if (!this.canUseCache(key, ttlMs)) { return await fn(); } @@ -259,6 +272,71 @@ export class CacheService { return fresh; } + /** + * Cache wrapper for functions whose result may legitimately be `null`. + * + * Stores results in a marked envelope so cached nulls are distinct from misses. + * + * @param fn - Function to execute (and optionally cache); may return null. + * @param key - Cache key + * @param ttlMs - Time to live in milliseconds + * @returns Cached value if present (including a cached `null`), otherwise fresh result from fn() + */ + async withCacheNullable>( + fn: () => Promise, + key: CacheKey, + ttlMs: number + ): Promise { + if (!this.canUseCache(key, ttlMs)) { + return await fn(); + } + + const cachedValue = await this.tryGetCachedValue(key, ttlMs); + if (cachedValue !== undefined) { + if (this.isNullableCacheBox(cachedValue)) { + return cachedValue.value; + } + + logger.debug( + { key, ttlMs }, + "Nullable cache entry is missing marker or value field; treating as cache miss and refreshing" + ); + } + + const fresh = await fn(); + // Guard against type-erased callers resolving undefined. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- types exclude undefined; this guards against type-erasure bugs + if (fresh !== undefined) { + const box: NullableCacheBox = { [NULLABLE_BOX_MARKER]: true, value: fresh }; + await this.trySetCache(key, box, ttlMs); + } + return fresh; + } + + /** Returns false when callers should bypass cache and execute directly. */ + private canUseCache(key: CacheKey, ttlMs: number): boolean { + if (!this.isRedisClientReady()) { + return false; + } + + const validation = validateInputs([key, ZCacheKey], [ttlMs, ZTtlMs]); + if (!validation.ok) { + logger.warn({ error: validation.error, key }, "Invalid cache inputs, executing function directly"); + return false; + } + + return true; + } + + private isNullableCacheBox(value: unknown): value is NullableCacheBox { + return ( + typeof value === "object" && + value !== null && + (value as Record)[NULLABLE_BOX_MARKER] === true && + Object.hasOwn(value, "value") + ); + } + private async tryGetCachedValue(key: CacheKey, ttlMs: number): Promise { try { const cacheResult = await this.get(key); @@ -266,13 +344,6 @@ export class CacheService { return cacheResult.data; } - if (cacheResult.ok && cacheResult.data === null) { - const existsResult = await this.exists(key); - if (existsResult.ok && existsResult.data) { - return null as T; - } - } - if (!cacheResult.ok) { logger.debug( { error: cacheResult.error, key, ttlMs }, @@ -280,15 +351,23 @@ export class CacheService { ); } } catch (error) { - logger.debug({ error, key, ttlMs }, "Cache get/exists threw; proceeding to compute fresh value"); + logger.debug({ error, key, ttlMs }, "Cache get threw; proceeding to compute fresh value"); } return undefined; } private async trySetCache(key: CacheKey, value: unknown, ttlMs: number): Promise { - if (value === undefined) { - return; // Skip caching undefined values + // Never persist null/undefined. The read path (tryGetCachedValue) treats a + // stored JSON `null` as a cache miss, so writing one is not just useless — + // it causes a recompute-and-rewrite loop on every request for a hot key. + // This also hardens withCache against a `null` slipping past its + // `NonNullable` constraint via type erasure (unknown/any plumbing, casts), + // which is the exact class of bug this change set fixes. withCacheNullable is + // unaffected: it always writes a non-null boxed envelope, never raw null. + if (value === undefined || value === null) { + logger.debug({ key, ttlMs }, "Refusing to cache nullish value; treating as non-cacheable"); + return; } try { diff --git a/scripts/dependabot-to-linear.mjs b/scripts/dependabot-to-linear.mjs new file mode 100644 index 000000000000..812b83fe93ca --- /dev/null +++ b/scripts/dependabot-to-linear.mjs @@ -0,0 +1,211 @@ +#!/usr/bin/env node +// Syncs open GitHub Dependabot alerts into Linear issues. +// Runs daily in CI (.github/workflows/dependabot-to-linear.yml). +// +// For each open Dependabot alert it creates one Linear issue (if not already +// present), on the Engineering team, in the Todo state, with the `security` +// label and a priority mapped from the alert severity. It is idempotent: +// issues are matched by a stable `[Dependabot #]` title prefix, so +// re-running never creates duplicates. +// +// Env: +// GITHUB_REPOSITORY "owner/repo" (provided automatically in Actions) +// DEPENDABOT_ALERTS_TOKEN token with "Dependabot alerts: read" (the default +// GITHUB_TOKEN cannot read this API) +// LINEAR_API_KEY | LINEAR_ACCESS_KEY Linear key with issue-create access +// Flags: +// --dry-run log what would be created without writing to Linear + +const DRY_RUN = process.argv.includes("--dry-run"); + +const LINEAR_TEAM_ID = "272e6dd2-fcab-4270-be37-1626dce35d2c"; // Engineering +const LINEAR_TODO_STATE_ID = "c32c8340-e8f8-48f4-a863-afabe3b19e17"; // Todo +const LINEAR_SECURITY_LABEL_ID = "f9c50a45-3bd9-45e7-a70a-5b88aba910f9"; // security +// The Engineering team uses T-shirt estimation, where the estimate is a plain +// integer the UI renders as a size: 1=XS, 2=S, 3=M, 5=L. We default every +// Dependabot issue to "S". +const LINEAR_ESTIMATE_S = 2; + +// Dependabot severity -> Linear priority (0 none, 1 urgent, 2 high, 3 medium, 4 low) +const PRIORITY_BY_SEVERITY = { + critical: 1, + high: 2, + medium: 3, + low: 4, +}; + +const LINEAR_API_URL = "https://api.linear.app/graphql"; + +// Abort any outbound request that hangs, so a stuck connection can't tie up the +// runner until the workflow-level timeout. +const FETCH_TIMEOUT_MS = 30000; + +function requireEnv(name, ...fallbacks) { + for (const key of [name, ...fallbacks]) { + if (process.env[key]) return process.env[key]; + } + throw new Error(`Missing required env var: ${[name, ...fallbacks].join(" or ")}`); +} + +async function fetchOpenAlerts(repo, token) { + const alerts = []; + let page = 1; + const perPage = 100; + for (;;) { + const url = `https://api.github.com/repos/${repo}/dependabot/alerts?state=open&per_page=${perPage}&page=${page}`; + const res = await fetch(url, { + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + headers: { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "formbricks-dependabot-to-linear", + }, + }); + if (!res.ok) { + const body = await res.text(); + throw new Error(`GitHub API ${res.status} fetching Dependabot alerts: ${body.slice(0, 500)}`); + } + const batch = await res.json(); + alerts.push(...batch); + if (batch.length < perPage) break; + page += 1; + } + return alerts; +} + +async function linearRequest(apiKey, query, variables) { + const res = await fetch(LINEAR_API_URL, { + method: "POST", + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + headers: { + Authorization: apiKey, + "Content-Type": "application/json", + }, + body: JSON.stringify({ query, variables }), + }); + const json = await res.json(); + if (!res.ok || json.errors) { + throw new Error(`Linear API error: ${JSON.stringify(json.errors ?? json).slice(0, 500)}`); + } + return json.data; +} + +async function issueExists(apiKey, titlePrefix) { + const data = await linearRequest( + apiKey, + `query ExistingIssue($teamId: ID!, $prefix: String!) { + issues(filter: { team: { id: { eq: $teamId } }, title: { startsWith: $prefix } }, first: 1) { + nodes { id } + } + }`, + { teamId: LINEAR_TEAM_ID, prefix: titlePrefix } + ); + return data.issues.nodes.length > 0; +} + +async function createIssue(apiKey, { title, description, priority }) { + const data = await linearRequest( + apiKey, + `mutation CreateIssue($input: IssueCreateInput!) { + issueCreate(input: $input) { + success + issue { identifier url } + } + }`, + { + input: { + teamId: LINEAR_TEAM_ID, + stateId: LINEAR_TODO_STATE_ID, + labelIds: [LINEAR_SECURITY_LABEL_ID], + estimate: LINEAR_ESTIMATE_S, + title, + description, + priority, + }, + } + ); + if (!data.issueCreate.success) { + throw new Error(`Linear issueCreate returned success=false for: ${title}`); + } + return data.issueCreate.issue; +} + +function buildIssue(alert) { + const number = alert.number; + const severity = alert.security_vulnerability?.severity ?? "medium"; + const priority = PRIORITY_BY_SEVERITY[severity] ?? 3; + const pkg = alert.dependency?.package?.name ?? "unknown package"; + const ecosystem = alert.dependency?.package?.ecosystem ?? ""; + const advisory = alert.security_advisory ?? {}; + const vuln = alert.security_vulnerability ?? {}; + const summary = advisory.summary ?? "Security vulnerability"; + const ghsa = advisory.ghsa_id ?? ""; + const cve = advisory.cve_id ?? ""; + const patched = vuln.first_patched_version?.identifier ?? "no patched version yet"; + const range = vuln.vulnerable_version_range ?? "unknown"; + + const titlePrefix = `[Dependabot #${number}]`; + const title = `${titlePrefix} ${pkg} — ${summary}`; + + const description = [ + `**Dependabot alert [#${number}](${alert.html_url})**`, + "", + `- **Package:** \`${pkg}\`${ecosystem ? ` (${ecosystem})` : ""}`, + `- **Severity:** ${severity}`, + ghsa ? `- **Advisory:** ${ghsa}` : null, + cve ? `- **CVE:** ${cve}` : null, + `- **Affected versions:** ${range}`, + `- **Patched version:** ${patched}`, + "", + advisory.description ? advisory.description : null, + ] + .filter((line) => line !== null) + .join("\n"); + + return { titlePrefix, title, description, priority }; +} + +async function main() { + const repo = requireEnv("GITHUB_REPOSITORY"); + const ghToken = requireEnv("DEPENDABOT_ALERTS_TOKEN"); + const linearKey = requireEnv("LINEAR_API_KEY", "LINEAR_ACCESS_KEY"); + + console.log(`Fetching open Dependabot alerts for ${repo}${DRY_RUN ? " (dry run)" : ""}…`); + const alerts = await fetchOpenAlerts(repo, ghToken); + console.log(`Found ${alerts.length} open alert(s).`); + + let created = 0; + let skipped = 0; + let failed = 0; + + for (const alert of alerts) { + const issue = buildIssue(alert); + try { + if (await issueExists(linearKey, issue.titlePrefix)) { + skipped += 1; + console.log(`skip ${issue.titlePrefix} (already in Linear)`); + continue; + } + if (DRY_RUN) { + created += 1; + console.log(`would ${issue.title} [priority ${issue.priority}]`); + continue; + } + const result = await createIssue(linearKey, issue); + created += 1; + console.log(`create ${result.identifier} ${result.url}`); + } catch (err) { + failed += 1; + console.error(`error ${issue.titlePrefix}: ${err.message}`); + } + } + + console.log(`Done. created=${created} skipped=${skipped} failed=${failed}${DRY_RUN ? " (dry run)" : ""}`); + if (failed > 0) process.exit(1); +} + +main().catch((err) => { + console.error(err.message ?? err); + process.exit(1); +});