diff --git a/.claude/hooks/check-migration-guard.sh b/.claude/hooks/check-migration-guard.sh new file mode 100755 index 000000000..1b9e88b5e --- /dev/null +++ b/.claude/hooks/check-migration-guard.sh @@ -0,0 +1,30 @@ +#!/bin/bash +set -euo pipefail +# Hook: Prevent editing existing migration files (forward-only migrations) +# Rule: "Forward-only. Idempotent." — CLAUDE.md +INPUT=$(cat) +FILE_PATH=$(echo "$INPUT" | jq -er '.tool_input.file_path // .tool_input.filePath // empty') || { + echo "BLOCKED: invalid hook payload (missing/invalid tool_input.file_path)" >&2 + exit 2 +} +[ -z "$FILE_PATH" ] && exit 0 + +# Only check migration files +case "$FILE_PATH" in + *migrations/*.sql|*migrations/*.ts) + # Allow creating NEW migration files (Write tool with no existing file) + TOOL_NAME=$(echo "$INPUT" | jq -er '.tool_name // empty') || TOOL_NAME="" + if [ "$TOOL_NAME" = "Write" ] && [ ! -f "$FILE_PATH" ]; then + exit 0 + fi + # Block editing existing migration files + if [ -f "$FILE_PATH" ]; then + echo "BLOCKED: Cannot edit existing migration file: $(basename "$FILE_PATH")" >&2 + echo "Rule: Migrations are forward-only. Create a new migration instead." >&2 + echo "Use: npm run db:generate" >&2 + exit 2 + fi + ;; +esac + +exit 0 diff --git a/.env.example b/.env.example new file mode 100644 index 000000000..1d0d4891c --- /dev/null +++ b/.env.example @@ -0,0 +1,71 @@ +# NeoBoard — Environment Variables +# Copy to app/.env.local and fill in values. +# For dev setup, scripts/setup.sh generates these automatically. + +# PostgreSQL connection (required) +DATABASE_URL=postgresql://neoboard:neoboard@localhost:5432/neoboard + +# 32-byte hex key for AES-256-GCM credential encryption (required) +# Generate: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))" +# WARNING: losing this key makes all stored credentials unrecoverable. +ENCRYPTION_KEY= + +# Previous encryption key — set this when rotating ENCRYPTION_KEY (optional) +# Rotation flow: 1) copy current ENCRYPTION_KEY to ENCRYPTION_KEY_OLD, +# 2) generate and set a new ENCRYPTION_KEY, 3) restart the app, +# 4) call POST /api/admin/rotate-key (admin only) to re-encrypt all credentials, +# 5) remove ENCRYPTION_KEY_OLD after successful rotation. +# ENCRYPTION_KEY_OLD= + +# Auth.js session secret (required) +# Generate: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))" +NEXTAUTH_SECRET= + +# Application URL (required) +NEXTAUTH_URL=http://localhost:3000 + +# One-time token for creating the first admin account via /signup (optional, dev only) +# Generate: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))" +ADMIN_BOOTSTRAP_TOKEN= + +# HMAC secret for API key hashing — required if using API keys (optional) +# Generate: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))" +API_KEY_HMAC_SECRET= + +# Self-registration toggle — set to "false" to disable /signup (optional, default: true) +# REGISTRATION_ENABLED=true + +# Tenant ID — defaults to "default" if unset (optional) +# TENANT_ID=default + +# Session max age in seconds — defaults to 28800 (8 hours) if unset (optional) +# SESSION_MAX_AGE=28800 + +# Log level — one of: fatal, error, warn, info, debug, trace (optional, default: info) +# LOG_LEVEL=info + +# Per-user query rate limit — max queries per minute per user (optional, default: 60) +# QUERY_RATE_LIMIT=60 + +# ── SSO / OIDC (optional) ──────────────────────────────────────────────────── +# Set all four required vars to enable a single OIDC provider via env. +# Requires NEOBOARD_EDITION=enterprise. +# For multiple providers, use the Admin UI (Settings > Authentication). + +# Required (all four must be set to activate SSO) +# NEOBOARD_EDITION=enterprise +# OIDC_ISSUER=https://myorg.okta.com +# OIDC_CLIENT_ID=neoboard +# OIDC_CLIENT_SECRET=your-client-secret + +# Optional +# OIDC_DISPLAY_NAME=Company SSO +# OIDC_SCOPES=openid profile email +# OIDC_CLAIM_KEY=groups +# OIDC_ADMIN_VALUE=neoboard-admins +# OIDC_CREATOR_VALUE=neoboard-editors +# OIDC_READER_VALUE=neoboard-viewers +# OIDC_AUTO_PROVISION=true +# OIDC_DEFAULT_ROLE=creator +# OIDC_ENFORCE_SSO=false + diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba187b086..1366ea2e0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ on: - 'package.json' - 'package-lock.json' pull_request: - branches: [main, dev, 'release/*'] + branches: [main, dev, 'release/*', 'feat/*'] paths: - 'app/**' - 'component/**' diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 000000000..0222a7414 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "enterprise"] + path = enterprise + url = https://github.com/alfredo1996/neoboard-enterprise.git diff --git a/app/e2e/widget-lab.spec.ts b/app/e2e/widget-lab.spec.ts index beae10edd..7bfc89991 100644 --- a/app/e2e/widget-lab.spec.ts +++ b/app/e2e/widget-lab.spec.ts @@ -733,6 +733,13 @@ test.describe("Widget Lab", () => { // ── Widget Lab consumption: duplicate, filter, search ─────────────── test.describe("Widget Lab consumption", () => { + // Tests in this block share the same template names ("Neo4j Bar Template", + // "PostgreSQL Table Template") in their beforeEach. With fullyParallel and + // 2 CI workers, two tests' beforeEach can race → two templates with the + // same name → strict-mode locator violation. Force serial execution so + // each test's beforeEach/afterEach owns the templates exclusively. + test.describe.configure({ mode: "serial" }); + let templateIds: string[] = []; test.beforeEach(async ({ page }) => { diff --git a/app/src/app/api/query/write/__tests__/route.test.ts b/app/src/app/api/query/write/__tests__/route.test.ts index cb529d5b0..4ddb5d13a 100644 --- a/app/src/app/api/query/write/__tests__/route.test.ts +++ b/app/src/app/api/query/write/__tests__/route.test.ts @@ -242,7 +242,7 @@ describe("POST /api/query/write", () => { ); }); - it("returns 500 when executeQuery throws", async () => { + it("returns 500 with sanitized message when executeQuery throws (no driver leak)", async () => { mockRequireSession.mockResolvedValue(writerSession); mockConnectionAndDashboard(); mockDecryptJson.mockReturnValue({ @@ -250,19 +250,24 @@ describe("POST /api/query/write", () => { username: "neo4j", password: "pass", }); - mockExecuteQuery.mockRejectedValue(new Error("Driver error")); + // Driver errors echo user-supplied SQL — must never bleed into the + // response body (security/PII consideration). + mockExecuteQuery.mockRejectedValue( + new Error('syntax error at or near "THIS"'), + ); const res = await POST( makeRequest({ connectionId: "c1", - query: "CREATE (n:Test)", + query: "THIS IS NOT VALID SQL", widgetId: "w1", dashboardId: "d1", }), ); expect(res.status).toBe(500); const body = await res.json(); - expect(body.error.message).toBe("Driver error"); + expect(body.error.message).toBe("Write query execution failed"); + expect(body.error.message).not.toMatch(/syntax error/i); }); it("returns 404 when connection belongs to another user", async () => { diff --git a/app/src/app/api/query/write/route.ts b/app/src/app/api/query/write/route.ts index c47916b13..034990553 100644 --- a/app/src/app/api/query/write/route.ts +++ b/app/src/app/api/query/write/route.ts @@ -157,6 +157,9 @@ async function handleWriteQuery(request: Request): Promise { }, "write_query_failed", ); - return handleRouteError(error, "Write query execution failed"); + // safeMessage: write queries echo user SQL in driver errors — never leak. + return handleRouteError(error, "Write query execution failed", { + safeMessage: true, + }); } } diff --git a/app/src/components/__tests__/dashboard-container-branches.test.tsx b/app/src/components/__tests__/dashboard-container-branches.test.tsx index 7e1ece8f4..4dd626214 100644 --- a/app/src/components/__tests__/dashboard-container-branches.test.tsx +++ b/app/src/components/__tests__/dashboard-container-branches.test.tsx @@ -70,9 +70,23 @@ vi.mock("@neoboard/components", () => ({ DashboardGrid: ({ children }: { children: React.ReactNode }) => (
{children}
), - Dialog: ({ children, open }: { children: React.ReactNode; open: boolean }) => + Dialog: ({ + children, + open, + onOpenChange, + }: { + children: React.ReactNode; + open: boolean; + onOpenChange?: (open: boolean) => void; + }) => open ? (
+ {children}
) : null, @@ -542,6 +556,88 @@ describe("DashboardContainer — refresh + fullscreen + sync dialogs", () => { ); }); + it("closes the fullscreen dialog and clears the pending ready timer", () => { + vi.useFakeTimers(); + try { + renderWithProviders(); + const fullscreenBtn = screen.getByText("Fullscreen").closest("button"); + act(() => { + fireEvent.click(fullscreenBtn!); + }); + expect(screen.getByTestId("fullscreen-dialog")).toBeDefined(); + // Close before the 250ms ready-timer fires — exercises closeFullscreen's + // clearTimeout branch. + act(() => { + fireEvent.click(screen.getByTestId("fullscreen-dialog-close")); + }); + expect(screen.queryByTestId("fullscreen-dialog")).toBeNull(); + // Advance past the ready-timer; if it weren't cleared it would attempt a + // setState on the now-closed dialog. No throw = success. + act(() => { + vi.advanceTimersByTime(500); + }); + } finally { + vi.useRealTimers(); + } + }); + + it("re-arming fullscreen clears the previous ready timer", () => { + vi.useFakeTimers(); + try { + // Two widgets so we can open fullscreen on each in succession. + renderWithProviders( + , + ); + const buttons = screen.getAllByText("Fullscreen"); + act(() => { + fireEvent.click(buttons[0].closest("button")!); + }); + expect(screen.getByTestId("fullscreen-title").textContent).toBe( + "Widget One", + ); + // Re-arm before the first 250ms timer fires — exercises openFullscreen's + // clearTimeout branch (line: clear previous ref before setting new). + act(() => { + fireEvent.click(buttons[1].closest("button")!); + }); + expect(screen.getByTestId("fullscreen-title").textContent).toBe( + "Widget Two", + ); + act(() => { + vi.advanceTimersByTime(500); + }); + } finally { + vi.useRealTimers(); + } + }); + + it("unmounting with a pending fullscreen-ready timer clears it cleanly", () => { + vi.useFakeTimers(); + try { + const { unmount } = renderWithProviders( + , + ); + const fullscreenBtn = screen.getByText("Fullscreen").closest("button"); + act(() => { + fireEvent.click(fullscreenBtn!); + }); + // Unmount before the 250ms timer fires — exercises the useEffect cleanup + // branch. Without it, the timer would call setState on a torn-down tree. + unmount(); + act(() => { + vi.advanceTimersByTime(500); + }); + // No unhandled "window is not defined" / "setState on unmounted" = pass. + } finally { + vi.useRealTimers(); + } + }); + it("renders the template-outdated RefreshCw header extra and opens sync dialog on click", () => { mockIsTemplateOutdated.mockReturnValue(true); const onSyncWidget = vi.fn(); diff --git a/app/src/components/dashboard-container.tsx b/app/src/components/dashboard-container.tsx index 4d538f205..1ca50daac 100644 --- a/app/src/components/dashboard-container.tsx +++ b/app/src/components/dashboard-container.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useMemo, useCallback } from "react"; +import { useState, useMemo, useCallback, useEffect, useRef } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { CardContainer } from "./card-container"; import { @@ -109,15 +109,41 @@ export function DashboardContainer({ // settles. Without this, NVL reads the canvas dimensions mid-animation // (at ~95% of final size) and the hit-test coordinates are permanently offset. const [fullscreenReady, setFullscreenReady] = useState(false); + // Track the deferred-ready timer so we can clear it on unmount or when the + // dialog is closed before the animation settles. Without this, the timer + // can fire after the component unmounts and call setState on a torn-down + // tree (jsdom: "window is not defined"; browser: React unmounted-update + // warning). + const fullscreenReadyTimerRef = useRef | null>( + null, + ); const openFullscreen = useCallback((w: DashboardWidget) => { setFullscreenReady(false); setFullscreenWidget(w); - setTimeout(() => setFullscreenReady(true), 250); + if (fullscreenReadyTimerRef.current !== null) { + clearTimeout(fullscreenReadyTimerRef.current); + } + fullscreenReadyTimerRef.current = setTimeout(() => { + fullscreenReadyTimerRef.current = null; + setFullscreenReady(true); + }, 250); }, []); const closeFullscreen = useCallback(() => { + if (fullscreenReadyTimerRef.current !== null) { + clearTimeout(fullscreenReadyTimerRef.current); + fullscreenReadyTimerRef.current = null; + } setFullscreenWidget(null); setFullscreenReady(false); }, []); + useEffect(() => { + return () => { + if (fullscreenReadyTimerRef.current !== null) { + clearTimeout(fullscreenReadyTimerRef.current); + fullscreenReadyTimerRef.current = null; + } + }; + }, []); const [pendingSyncWidget, setPendingSyncWidget] = useState(null); const parameters = useParameterStore((s) => s.parameters); @@ -282,14 +308,26 @@ export function DashboardContainer({ onRefresh={ showRefresh ? () => { - // Invalidate all TanStack Query entries matching this widget's - // connection + query combo. This triggers a refetch. + // Invalidate the TanStack Query entry for this widget so + // it refetches. We must mirror the prefix shape used by + // useWidgetQuery exactly: + // ["widget-query", connectionId, database, query, params, staleTime] + // Earlier we omitted `database`, which made position 2 + // mismatch (null vs query string), so invalidation never + // matched and the refresh button silently no-op'd. + // + // We intentionally stop the prefix at `query` — the hook + // merges $param_xxx values into `params` at call time, so + // `widget.params` here is not deep-equal to the hook's + // mergedParams when parameters are referenced. Stopping + // at `query` guarantees prefix match for both the + // parameterless and parameterised cases. void queryClient.invalidateQueries({ queryKey: [ "widget-query", widget.connectionId, + widget.database ?? null, widget.query, - widget.params, ], }); } diff --git a/app/src/lib/__tests__/api/api-utils.test.ts b/app/src/lib/__tests__/api/api-utils.test.ts index 35b6a5431..2650e22f9 100644 --- a/app/src/lib/__tests__/api/api-utils.test.ts +++ b/app/src/lib/__tests__/api/api-utils.test.ts @@ -134,6 +134,39 @@ describe("handleRouteError", () => { expect(body.error.code).toBe("REQUEST_TIMEOUT"); expect(res.headers.get("Retry-After")).toBe("5"); }); + + describe("safeMessage option", () => { + it("collapses raw driver errors to fallback when safeMessage=true", async () => { + const res = handleRouteError( + new Error('syntax error at or near "THIS"'), + "Write query execution failed", + { safeMessage: true }, + ); + expect(res.status).toBe(500); + const body = await res.json(); + expect(body.error.message).toBe("Write query execution failed"); + expect(body.error.message).not.toMatch(/syntax error/i); + }); + + it("still returns raw driver errors when safeMessage is omitted", async () => { + const res = handleRouteError( + new Error('syntax error at or near "THIS"'), + "Write query execution failed", + ); + const body = await res.json(); + expect(body.error.message).toBe('syntax error at or near "THIS"'); + }); + + it("safeMessage does not bypass typed app errors (Queue/Auth/etc.)", async () => { + const { QueueTimeoutError } = await import("@/lib/query/scheduler"); + const res = handleRouteError(new QueueTimeoutError(), "Write failed", { + safeMessage: true, + }); + // QueueTimeoutError still gets its specific 408 + Retry-After handling. + expect(res.status).toBe(408); + expect(res.headers.get("Retry-After")).toBe("5"); + }); + }); }); describe("validateBody", () => { diff --git a/app/src/lib/api/api-utils.ts b/app/src/lib/api/api-utils.ts index bc4fc4468..103a1758b 100644 --- a/app/src/lib/api/api-utils.ts +++ b/app/src/lib/api/api-utils.ts @@ -87,6 +87,16 @@ export function validateBody( export function handleRouteError( error: unknown, fallbackMsg = "Internal server error", + options?: { + /** + * When true, untyped errors (e.g. raw driver/query errors) collapse to + * `fallbackMsg` instead of being passed through `sanitizeErrorMessage`. + * Use this on routes where the underlying error message could leak schema + * details, query structure, or other sensitive shape — most notably the + * write-query route where pg syntax errors echo the user-supplied SQL. + */ + safeMessage?: boolean; + }, ): ReturnType { if (error instanceof EnterpriseRequiredError) { return apiError("ENTERPRISE_REQUIRED", error.message); @@ -130,5 +140,10 @@ export function handleRouteError( // Return a sanitized error message to the client. Raw driver/DB errors // can leak query structure and schema details, so sanitizeErrorMessage // strips bundler internals while preserving meaningful messages. + // Routes that opt into `safeMessage` collapse to the fallback unconditionally + // — used by the write route to keep pg/Cypher syntax errors out of responses. + if (options?.safeMessage) { + return serverError(fallbackMsg); + } return serverError(sanitizeErrorMessage(message, fallbackMsg)); } diff --git a/component/src/components/composed/__tests__/data-grid-dynamic-pagination.test.tsx b/component/src/components/composed/__tests__/data-grid-dynamic-pagination.test.tsx index 8c6bae28a..a2a6ef917 100644 --- a/component/src/components/composed/__tests__/data-grid-dynamic-pagination.test.tsx +++ b/component/src/components/composed/__tests__/data-grid-dynamic-pagination.test.tsx @@ -363,7 +363,7 @@ describe("DataGrid — enablePagination", () => { await user.click(checkboxes[1]); expect(screen.getByText(/1 of 30 row\(s\) selected\./)).toBeInTheDocument(); - }); + }, 15000); }); // --------------------------------------------------------------------------- diff --git a/connection/jest.config.js b/connection/jest.config.js index db9761b9b..23a7cc8cd 100644 --- a/connection/jest.config.js +++ b/connection/jest.config.js @@ -2,9 +2,16 @@ module.exports = { testEnvironment: "node", transform: { - "^.+.tsx?$": ["ts-jest", { diagnostics: false }], + "^.+\\.tsx?$": ["ts-jest", { diagnostics: false }], + "^.+\\.m?js$": ["ts-jest", { diagnostics: false, useESM: false }], }, - testPathIgnorePatterns: ["utils"], + // uuid v14+ ships ESM only; transform it (and any future ESM-only deps in + // the testcontainers→dockerode chain) so Jest's CJS runtime can require them. + transformIgnorePatterns: ["/node_modules/(?!(uuid)/)"], + // Skip the built `dist/` output — adding the JS transform above means jest + // would otherwise pick up compiled `.test.js` and `.test.d.ts` files from + // a previous `tsc -p tsconfig.build.json` and double-run them. + testPathIgnorePatterns: ["utils", "/dist/"], globalSetup: "./__tests__/utils/setup.ts", globalTeardown: "./__tests__/utils/teardown.ts", // Integration tests hit a live Neo4j/PostgreSQL testcontainer. diff --git a/connection/src/generalized/__tests__/errors.test.ts b/connection/src/generalized/__tests__/errors.test.ts index 0217fb220..6bb2eff6d 100644 --- a/connection/src/generalized/__tests__/errors.test.ts +++ b/connection/src/generalized/__tests__/errors.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect } from "@jest/globals"; import { ConnectionError, QueryError, diff --git a/docker/docker-compose.prod-full.yml b/docker/docker-compose.prod-full.yml new file mode 100644 index 000000000..8047267d2 --- /dev/null +++ b/docker/docker-compose.prod-full.yml @@ -0,0 +1,72 @@ +# Full-stack NeoBoard: app + PostgreSQL in a single compose. +# For single-server deployments where you don't have an external PostgreSQL. +# +# Usage: +# cp ../.env.example .env +# # Fill in ENCRYPTION_KEY, NEXTAUTH_SECRET, NEXTAUTH_URL +# docker compose -f docker/docker-compose.prod-full.yml up -d +services: + postgres: + image: postgres:16-alpine + environment: + POSTGRES_USER: ${POSTGRES_USER:-neoboard} + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-neoboard} + POSTGRES_DB: ${POSTGRES_DB:-neoboard} + volumes: + - pgdata:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-neoboard}"] + interval: 10s + timeout: 5s + retries: 5 + deploy: + resources: + limits: + cpus: '1.0' + memory: 512M + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" + restart: unless-stopped + + neoboard: + build: + context: .. + dockerfile: Dockerfile + ports: + - "${PORT:-3000}:3000" + environment: + DATABASE_URL: postgresql://${POSTGRES_USER:-neoboard}:${POSTGRES_PASSWORD:-neoboard}@postgres:5432/${POSTGRES_DB:-neoboard} + ENCRYPTION_KEY: ${ENCRYPTION_KEY} + NEXTAUTH_SECRET: ${NEXTAUTH_SECRET} + NEXTAUTH_URL: ${NEXTAUTH_URL:-http://localhost:3000} + FORCE_HTTPS: ${FORCE_HTTPS:-false} + API_KEY_HMAC_SECRET: ${API_KEY_HMAC_SECRET:-} + depends_on: + postgres: + condition: service_healthy + deploy: + resources: + limits: + cpus: '2.0' + memory: 1G + reservations: + cpus: '0.5' + memory: 256M + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" + healthcheck: + test: ["CMD", "wget", "-q", "-O-", "http://localhost:3000/api/health"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 30s + restart: unless-stopped + +volumes: + pgdata: diff --git a/enterprise b/enterprise new file mode 160000 index 000000000..9461e0395 --- /dev/null +++ b/enterprise @@ -0,0 +1 @@ +Subproject commit 9461e03950b94d3f449d65b76a9ddef59e5ca333