From 3d91d6f5165763af16f076185d7a72448414d4dd Mon Sep 17 00:00:00 2001 From: LyraShield Dev Agent Date: Sat, 1 Aug 2026 22:14:23 +0530 Subject: [PATCH 1/3] fix(security): close addScanEvent cross-tenant gap + harden worker/CLI/OG surfaces Review of direct-to-main commit 79d8369 surfaced one real cross-tenant bug and three low-priority hardening gaps. This PR fixes all four as a single security hardening batch. 1. addScanEvent cross-tenant guard (packages/db/src/scan-service.ts) The function already required an active workspace context and did a findUnique on the scan, but only checked EXISTENCE, not ownership. A caller holding a valid scanId from another workspace could inject a ScanEvent row into that scan. Now compares scan.workspaceId to the active workspace context and rejects on mismatch, before any event row is written. Adds a regression test (no-context / cross-tenant / missing / happy path) in scan-service-operations.test.ts. 2. Worker readTextFileBounded TOCTOU (apps/worker/src/engine/runner.ts) Previously lstat(path) then open(path): an attacker able to write into the engine run dir could swap the artifact for a symlink between the two calls. Now opens first and fstats the live handle, removing the lstat->open time window. (Note: fs/promises open() does not expose O_NOFOLLOW, so a final-component symlink at open time is still followed; the engine run dir is worker-controlled and validated upstream, so this is defense-in-depth.) 3. CLI atomicWrite durability + parent-symlink guard (packages/cli/src/installers/atomic-write.ts) - fsync the temp file before rename so content is durable on disk before it becomes visible at the final path. - Validate the destination directory with realpath and refuse to write through a symlinked parent (prevents redirecting the write outside the intended location, e.g. /tmp/link -> /etc). 4. OG scorecard CDN stale window (apps/web (public)/api/og/score/[slug]/route.tsx) Cache-Control was max-age=3600, s-maxage=86400, swr=86400, so a revoked or superseded scorecard could keep being served by the CDN for up to a day. Reduced to max-age=300, s-maxage=300, swr=60 so revoked/expired cards stop being served within minutes. No change to the allowlisted payload. Verification: repository has no node_modules in this environment (npm registry unreachable), so vitest/lint/typecheck could not be executed locally; the regression test and all four changes are designed to run under the existing CI gates (lint, typecheck, unit tests). No schema, API, or behaviour change beyond the security tightenings above. --- .../(public)/api/og/score/[slug]/route.tsx | 5 +- apps/worker/src/engine/runner.ts | 13 +++-- packages/cli/src/installers/atomic-write.ts | 39 ++++++++++--- .../db/src/scan-service-operations.test.ts | 56 ++++++++++++++++++- packages/db/src/scan-service.ts | 10 +++- 5 files changed, 105 insertions(+), 18 deletions(-) diff --git a/apps/web/src/app/(public)/api/og/score/[slug]/route.tsx b/apps/web/src/app/(public)/api/og/score/[slug]/route.tsx index 86dca41d..f2b4975d 100644 --- a/apps/web/src/app/(public)/api/og/score/[slug]/route.tsx +++ b/apps/web/src/app/(public)/api/og/score/[slug]/route.tsx @@ -148,7 +148,10 @@ export async function GET(request: Request, { params }: { params: Promise<{ slug width, height, headers: { - "Cache-Control": "public, max-age=3600, s-maxage=86400, stale-while-revalidate=86400", + // Scorecards can be revoked or superseded; keep the shared/CDN cache window + // short so a revoked card stops being served quickly. Browser cache stays at + // 5 minutes; CDN revalidates after 5 minutes rather than holding for a day. + "Cache-Control": "public, max-age=300, s-maxage=300, stale-while-revalidate=60", "Content-Disposition": `inline; filename="lyrashield-${variant}-${format}.png"`, }, } diff --git a/apps/worker/src/engine/runner.ts b/apps/worker/src/engine/runner.ts index 9ed38cb8..f81ed142 100644 --- a/apps/worker/src/engine/runner.ts +++ b/apps/worker/src/engine/runner.ts @@ -523,15 +523,16 @@ export async function findRunOutputDir(workDir: string): Promise async function readTextFileBounded(path: string, maxBytes: number): Promise { // The artifact location is selected only from a validated engine output directory. - // eslint-disable-next-line security/detect-non-literal-fs-filename - const fileStat = await lstat(path) - if (!fileStat.isFile()) { - throw new Error(`Engine artifact is not a regular file: ${path}`) - } - + // Open first, then fstat the live handle: this avoids the TOCTOU window where an + // attacker swaps the path for a symlink between a prior lstat and the open. // eslint-disable-next-line security/detect-non-literal-fs-filename const handle = await open(path, "r") try { + const fileStat = await handle.stat() + if (!fileStat.isFile()) { + throw new Error(`Engine artifact is not a regular file: ${path}`) + } + const buffer = Buffer.allocUnsafe(maxBytes + 1) let offset = 0 while (offset <= maxBytes) { diff --git a/packages/cli/src/installers/atomic-write.ts b/packages/cli/src/installers/atomic-write.ts index 29a478eb..17f72c0d 100644 --- a/packages/cli/src/installers/atomic-write.ts +++ b/packages/cli/src/installers/atomic-write.ts @@ -1,22 +1,47 @@ -import { lstat, rename, writeFile } from "node:fs/promises" +import { lstat, open, rename, realpath } from "node:fs/promises" import { randomUUID } from "node:crypto" -import { resolve } from "node:path" +import { dirname, resolve } from "node:path" /** * Atomically write a file with a temp-and-rename pattern. The temp file is - * created with O_EXCL so a pre-existing file or symlink cannot be hijacked, and - * the final path is re-validated with lstat after the rename to ensure it is a - * regular file and not a dangling or followed symlink. + * created with O_EXCL so a pre-existing file or symlink cannot be hijacked, + * fsynced before the rename for durability, and the final path is re-validated + * with lstat after the rename to ensure it is a regular file and not a dangling + * or followed symlink. The destination directory is resolved and checked so a + * parent-path symlink cannot redirect the write outside the intended location. */ export async function atomicWrite(filePath: string, content: string): Promise { const absolutePath = resolve(filePath) + + // Validate the parent directory chain: if any parent is a symlink, the write + // could land outside the intended target (e.g. /tmp/link -> /etc). Resolve the + // directory and confirm the real path matches the requested directory. + const dir = dirname(absolutePath) + try { + // eslint-disable-next-line security/detect-non-literal-fs-filename + const realDir = await realpath(dir) + if (realDir !== dir) { + throw new Error(`Refusing to write through a symlinked directory: ${dir}`) + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error + // Parent does not exist yet (callers mkdir recursively); nothing to validate. + } + const tmp = `${absolutePath}.${randomUUID()}.lyrashield-tmp` // O_EXCL: fail if the temp path already exists (including as a symlink). // This prevents a symlink attack where an attacker points the temp path at - // another file and we overwrite the target. + // another file and we overwrite the target. fsync before rename so the data is + // durable on disk before it becomes visible at the final path. // eslint-disable-next-line security/detect-non-literal-fs-filename - await writeFile(tmp, content, { encoding: "utf-8", flag: "wx" }) + const handle = await open(tmp, "wx") + try { + await handle.writeFile(content, "utf-8") + await handle.sync() + } finally { + await handle.close() + } // filePath is the resolved installer target path selected for this workspace. // eslint-disable-next-line security/detect-non-literal-fs-filename diff --git a/packages/db/src/scan-service-operations.test.ts b/packages/db/src/scan-service-operations.test.ts index 3d2337f2..f98bf3e8 100644 --- a/packages/db/src/scan-service-operations.test.ts +++ b/packages/db/src/scan-service-operations.test.ts @@ -18,8 +18,11 @@ vi.mock("./client", () => ({ vi.mock("@lyrashield/logger", () => ({ logger: { info: vi.fn() } })) +const mockGetWorkspaceContext = vi.fn() +vi.mock("./extension", () => ({ getWorkspaceContext: () => mockGetWorkspaceContext() })) + import { prisma } from "./client" -import { createScan, getScanWithEvents, listScans, updateScanStatus } from "./scan-service" +import { addScanEvent, createScan, getScanWithEvents, listScans, updateScanStatus } from "./scan-service" const mockPrisma = prisma as unknown as { $transaction: ReturnType @@ -64,6 +67,57 @@ describe("updateScanStatus", () => { }) }) +describe("addScanEvent — cross-tenant guard", () => { + beforeEach(() => { + vi.clearAllMocks() + mockPrisma.scanEvent.create.mockResolvedValue({ id: "event-1" }) + }) + + it("rejects when there is no workspace context", async () => { + mockGetWorkspaceContext.mockReturnValue(null) + await expect(addScanEvent("scan-1", "queued", "info", "msg")).rejects.toThrow( + "workspace context is required" + ) + expect(mockPrisma.scanEvent.create).not.toHaveBeenCalled() + }) + + it("rejects a scanId that belongs to a different workspace (cross-tenant injection)", async () => { + mockGetWorkspaceContext.mockReturnValue("ws-attacker") + mockPrisma.scan.findUnique.mockResolvedValue({ workspaceId: "ws-victim" }) + + await expect(addScanEvent("scan-victim", "queued", "info", "msg")).rejects.toThrow( + "Scan not found in workspace" + ) + // The ownership comparison must happen before any event row is written. + expect(mockPrisma.scan.findUnique).toHaveBeenCalledWith({ + where: { id: "scan-victim" }, + select: { workspaceId: true }, + }) + expect(mockPrisma.scanEvent.create).not.toHaveBeenCalled() + }) + + it("rejects a scanId that does not exist", async () => { + mockGetWorkspaceContext.mockReturnValue("ws-1") + mockPrisma.scan.findUnique.mockResolvedValue(null) + await expect(addScanEvent("scan-missing", "queued", "info", "msg")).rejects.toThrow( + "Scan not found in workspace" + ) + expect(mockPrisma.scanEvent.create).not.toHaveBeenCalled() + }) + + it("writes the event when the scan belongs to the active workspace", async () => { + mockGetWorkspaceContext.mockReturnValue("ws-1") + mockPrisma.scan.findUnique.mockResolvedValue({ workspaceId: "ws-1" }) + + await expect(addScanEvent("scan-1", "queued", "info", "msg")).resolves.toEqual({ + id: "event-1", + }) + expect(mockPrisma.scanEvent.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ scanId: "scan-1", stage: "queued", level: "info" }), + }) + }) +}) + describe("createScan", () => { beforeEach(() => { vi.clearAllMocks() diff --git a/packages/db/src/scan-service.ts b/packages/db/src/scan-service.ts index eb8629a3..a1cc5252 100644 --- a/packages/db/src/scan-service.ts +++ b/packages/db/src/scan-service.ts @@ -203,9 +203,13 @@ export async function addScanEvent( // Defense-in-depth: verify the scan belongs to the current workspace before // writing an event. ScanEvent is a child table without its own workspaceId, so // this prevents cross-tenant event injection if a caller has a valid scanId - // from another workspace. - const scan = await prisma.scan.findUnique({ where: { id: scanId } }) - if (!scan) { + // from another workspace. The existence check alone is not sufficient — we must + // compare the scan's workspaceId to the active workspace context. + const scan = await prisma.scan.findUnique({ + where: { id: scanId }, + select: { workspaceId: true }, + }) + if (!scan || scan.workspaceId !== workspaceId) { throw new Error(`Scan not found in workspace: ${scanId}`) } From 282e270a120e0d2005e1e11d587152b421bda939 Mon Sep 17 00:00:00 2001 From: LyraShield Dev Agent Date: Sat, 1 Aug 2026 23:08:14 +0530 Subject: [PATCH 2/3] style: prettier-format addScanEvent test import to pass CI format:check The multi-symbol import from ./scan-service exceeded Prettier's print width and failed the repo format:check gate (xargs exit 123). Split it onto multiple lines; no logic change. --- packages/db/src/scan-service-operations.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/db/src/scan-service-operations.test.ts b/packages/db/src/scan-service-operations.test.ts index f98bf3e8..a8747d65 100644 --- a/packages/db/src/scan-service-operations.test.ts +++ b/packages/db/src/scan-service-operations.test.ts @@ -22,7 +22,13 @@ const mockGetWorkspaceContext = vi.fn() vi.mock("./extension", () => ({ getWorkspaceContext: () => mockGetWorkspaceContext() })) import { prisma } from "./client" -import { addScanEvent, createScan, getScanWithEvents, listScans, updateScanStatus } from "./scan-service" +import { + addScanEvent, + createScan, + getScanWithEvents, + listScans, + updateScanStatus, +} from "./scan-service" const mockPrisma = prisma as unknown as { $transaction: ReturnType From f08cda3e264a7545ddf96f045f77c8e1640dc981 Mon Sep 17 00:00:00 2001 From: LyraShield Dev Agent Date: Sat, 1 Aug 2026 23:42:44 +0530 Subject: [PATCH 3/3] fix(marketing): stop mobile nav menu duplicating the desktop top navbar Founder report (screenshot): the right-side mobile nav panel was visible on a desktop-width landing page at the same time as the full top navbar, so the same links (Methodology / Free scan / Tools / Resources / Docs / Sign in / Get started) rendered twice. Root cause: the hamburger button and the mobile menu are hidden from md up, but nothing closed an already-open menu when the viewport crossed to desktop width. A menu opened at mobile width (or restored by a desktop browser from a small-window session) stayed open and duplicated the top nav. Fix (three coordinated layers): 1. Add md:hidden to the so it can never render on desktop, even if [open]. Moved the open layout from Tailwind's open:grid to a plain CSS #mobile-menu[open]{display:grid} rule so the md:hidden utility wins on md+. 2. JS: auto-close the menu when a matchMedia('(min-width: 48rem)') listener fires (viewport grows to desktop), and once on init to clear any restored open state. 3. Kept the menu mobile-only; the top navbar remains the single desktop nav. No change to links, routes, or the desktop navbar. a11y preserved: aria-modal, aria-expanded toggle, focus-visible rings, Escape/backdrop/link close all unchanged; the menu simply cannot persist into the desktop breakpoint. Verification: authoring environment has no node_modules (npm registry unreachable), so astro build/check could not be run locally; change is scoped to Header.astro and runs under the existing CI gates (astro check + build). --- apps/marketing/src/components/Header.astro | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/apps/marketing/src/components/Header.astro b/apps/marketing/src/components/Header.astro index 4d685f94..3dd970f5 100644 --- a/apps/marketing/src/components/Header.astro +++ b/apps/marketing/src/components/Header.astro @@ -74,7 +74,7 @@ const appUrl = (import.meta.env.PUBLIC_APP_URL as string | undefined)?.replace(/ id="mobile-menu" aria-label="Mobile navigation" aria-modal="true" - class="fixed m-0 w-56 max-w-[calc(100%-2rem)] grid gap-1 rounded-xl border border-border bg-bg-raised p-2 shadow-2xl shadow-black/20 open:grid" + class="fixed m-0 w-56 max-w-[calc(100%-2rem)] gap-1 rounded-xl border border-border bg-bg-raised p-2 shadow-2xl shadow-black/20 md:hidden" style="top: 4.5rem; right: 1rem; bottom: auto; left: auto;" > Methodology @@ -151,6 +151,17 @@ const appUrl = (import.meta.env.PUBLIC_APP_URL as string | undefined)?.replace(/ menu.querySelectorAll(".menu-link").forEach((link) => { link.addEventListener("click", () => menu.close()) }) + + // The top navbar is visible from md up, so the mobile menu must never stay open + // there. If the viewport crosses to desktop while the menu is open (e.g. the + // window is widened, or a desktop browser restores a small-window state), close + // it so the two navs never render at once. + const desktopMq = window.matchMedia("(min-width: 48rem)") // Tailwind md + const closeOnDesktop = () => { + if (desktopMq.matches && menu.open) menu.close() + } + desktopMq.addEventListener("change", closeOnDesktop) + closeOnDesktop() } } @@ -159,4 +170,9 @@ const appUrl = (import.meta.env.PUBLIC_APP_URL as string | undefined)?.replace(/ #mobile-menu::backdrop { background: transparent; } + /* The menu is mobile-only. When open (and only on small screens) lay it out as a + vertical grid; on md+ it is hidden entirely by the md:hidden utility above. */ + #mobile-menu[open] { + display: grid; + }