From 4701686797501cf8f9a80ea26478b77fcbe23e45 Mon Sep 17 00:00:00 2001 From: Alex Urevick-Ackelsberg Date: Fri, 20 Mar 2026 13:48:30 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20address=20meta-critic=20findings=20?= =?UTF-8?q?=E2=80=94=20security=20hardening,=20architecture=20fixes,=20and?= =?UTF-8?q?=20test=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier 1 (bugs): - Fix SearchService interface mismatch — remove duplicate interface, eliminate unsafe casts - Fix Inngest empty registry — lazy proxy singleton populated during pipeline init - Wire SearchService FTS into content_search MCP tool (replaces LIKE fallback) - Fix verify_branch to read expected branch from state store instead of hard-coded null Tier 2 (pre-deploy): - Replace CryptoJS AES (weak passphrase KDF) with Node native AES-256-GCM - Fail-closed encryption when TOKEN_ENCRYPTION_KEY missing in production - Add bearer token auth middleware to pipeline API routes - Fix tenant spoofing — derive tenant from authenticated user, not x-tenant-id header - Session secret fail-closed in production - Add Zod-based env var validation at startup (new src/config.ts) Tier 3 (pre-users): - CSRF: change disconnect from GET to POST, add SameSite=lax cookie - XSS: escape all user values in scheduler HTML templates - Enable Content Security Policy via helmet - Add rate limiting (express-rate-limit) to auth and API routes - Wire periodic entitlement cache cleanup (5min interval) - Add 55 auth route tests (routes.test.ts + verify.test.ts) — was 0% coverage Co-Authored-By: Claude Opus 4.6 (1M context) Entire-Checkpoint: fd7d2e1083a4 --- joyus-ai-mcp-server/package-lock.json | 33 +- joyus-ai-mcp-server/package.json | 3 +- joyus-ai-mcp-server/src/auth/routes.ts | 10 +- joyus-ai-mcp-server/src/config.ts | 33 + .../src/content/generation/index.ts | 4 +- .../src/content/generation/retriever.ts | 28 +- joyus-ai-mcp-server/src/content/index.ts | 16 +- joyus-ai-mcp-server/src/db/encryption.ts | 43 +- joyus-ai-mcp-server/src/index.ts | 43 +- joyus-ai-mcp-server/src/inngest/index.ts | 21 +- joyus-ai-mcp-server/src/inngest/registry.ts | 40 + joyus-ai-mcp-server/src/pipelines/init.ts | 4 + joyus-ai-mcp-server/src/pipelines/routes.ts | 22 +- joyus-ai-mcp-server/src/scheduler/routes.ts | 42 +- joyus-ai-mcp-server/src/tools/executor.ts | 27 +- .../src/tools/executors/content-executor.ts | 49 +- joyus-ai-mcp-server/tests/auth/routes.test.ts | 843 ++++++++++++++++++ joyus-ai-mcp-server/tests/auth/verify.test.ts | 209 +++++ joyus-ai-mcp-server/tests/encryption.test.ts | 29 +- .../tests/pipelines/routes.test.ts | 6 +- joyus-ai-state/src/mcp/tools/verify-branch.ts | 11 +- .../tests/unit/mcp/verify-branch.test.ts | 83 +- 22 files changed, 1470 insertions(+), 129 deletions(-) create mode 100644 joyus-ai-mcp-server/src/config.ts create mode 100644 joyus-ai-mcp-server/src/inngest/registry.ts create mode 100644 joyus-ai-mcp-server/tests/auth/routes.test.ts create mode 100644 joyus-ai-mcp-server/tests/auth/verify.test.ts diff --git a/joyus-ai-mcp-server/package-lock.json b/joyus-ai-mcp-server/package-lock.json index 21aba8a..7b99d40 100644 --- a/joyus-ai-mcp-server/package-lock.json +++ b/joyus-ai-mcp-server/package-lock.json @@ -18,6 +18,7 @@ "dotenv": "^16.4.1", "drizzle-orm": "^0.45.1", "express": "^4.18.2", + "express-rate-limit": "^8.3.1", "express-session": "^1.17.3", "helmet": "^7.1.0", "inngest": "^3.0.0", @@ -992,6 +993,7 @@ "node_modules/@modelcontextprotocol/sdk/node_modules/express": { "version": "5.2.1", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -1030,6 +1032,21 @@ "url": "https://opencollective.com/express" } }, + "node_modules/@modelcontextprotocol/sdk/node_modules/express-rate-limit": { + "version": "7.5.1", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-7.5.1.tgz", + "integrity": "sha512-7iN8iPMDzOMHPUYllBEsQdWVB6fPDMPqwjBaFrgr4Jgr/+okjvzAy+UHlYYL/Vs0OsOrMkwS6PJDkFlJwoxUnw==", + "license": "MIT", + "engines": { + "node": ">= 16" + }, + "funding": { + "url": "https://github.com/sponsors/express-rate-limit" + }, + "peerDependencies": { + "express": ">= 4.11" + } + }, "node_modules/@modelcontextprotocol/sdk/node_modules/finalhandler": { "version": "2.1.1", "license": "MIT", @@ -6024,8 +6041,13 @@ } }, "node_modules/express-rate-limit": { - "version": "7.5.1", + "version": "8.3.1", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.3.1.tgz", + "integrity": "sha512-D1dKN+cmyPWuvB+G2SREQDzPY1agpBIcTa9sJxOPMCNeH3gwzhqJRDWCXW3gg0y//+LQ/8j52JbMROWyrKdMdw==", "license": "MIT", + "dependencies": { + "ip-address": "10.1.0" + }, "engines": { "node": ">= 16" }, @@ -6980,6 +7002,15 @@ "node": ">= 0.4" } }, + "node_modules/ip-address": { + "version": "10.1.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.1.0.tgz", + "integrity": "sha512-XXADHxXmvT9+CRxhXg56LJovE+bmWnEWB78LB83VZTprKTmaC5QfruXocxzTZ2Kl0DNwKuBdlIhjL8LeY8Sf8Q==", + "license": "MIT", + "engines": { + "node": ">= 12" + } + }, "node_modules/ipaddr.js": { "version": "1.9.1", "license": "MIT", diff --git a/joyus-ai-mcp-server/package.json b/joyus-ai-mcp-server/package.json index e929044..025326c 100644 --- a/joyus-ai-mcp-server/package.json +++ b/joyus-ai-mcp-server/package.json @@ -34,11 +34,12 @@ "dotenv": "^16.4.1", "drizzle-orm": "^0.45.1", "express": "^4.18.2", + "express-rate-limit": "^8.3.1", "express-session": "^1.17.3", "helmet": "^7.1.0", + "inngest": "^3.0.0", "jose": "^6.1.3", "luxon": "^3.4.4", - "inngest": "^3.0.0", "node-cron": "^3.0.3", "pg": "^8.18.0", "zod": "^3.22.0" diff --git a/joyus-ai-mcp-server/src/auth/routes.ts b/joyus-ai-mcp-server/src/auth/routes.ts index 29d6f32..cfb3d02 100644 --- a/joyus-ai-mcp-server/src/auth/routes.ts +++ b/joyus-ai-mcp-server/src/auth/routes.ts @@ -165,7 +165,7 @@ authRouter.get('/', async (req: Request, res: Response) => { ${connectedServices.has('GOOGLE') - ? 'Disconnect' + ? '
' : 'Connect'} @@ -177,7 +177,7 @@ authRouter.get('/', async (req: Request, res: Response) => { ${connectedServices.has('JIRA') - ? 'Disconnect' + ? '
' : 'Connect'} @@ -189,7 +189,7 @@ authRouter.get('/', async (req: Request, res: Response) => { ${connectedServices.has('SLACK') - ? 'Disconnect' + ? '
' : 'Connect'} @@ -201,7 +201,7 @@ authRouter.get('/', async (req: Request, res: Response) => { ${connectedServices.has('GITHUB') - ? 'Disconnect' + ? '
' : 'Connect'} @@ -652,7 +652,7 @@ authRouter.get('/github/callback', async (req: Request, res: Response) => { // Disconnect & Logout // ============================================================ -authRouter.get('/:service/disconnect', requireSessionOrRedirect, async (req: Request, res: Response) => { +authRouter.post('/:service/disconnect', requireSessionOrRedirect, async (req: Request, res: Response) => { const userId = req.session!.userId!; const service = req.params.service.toUpperCase() as Service; diff --git a/joyus-ai-mcp-server/src/config.ts b/joyus-ai-mcp-server/src/config.ts new file mode 100644 index 0000000..bfc204b --- /dev/null +++ b/joyus-ai-mcp-server/src/config.ts @@ -0,0 +1,33 @@ +/** + * Environment configuration validation + * + * Validates required env vars at import time using Zod. + * Import this module early in the application entry point. + */ + +import { z } from 'zod'; + +const envSchema = z.object({ + NODE_ENV: z.enum(['development', 'production', 'test']).default('development'), + PORT: z.coerce.number().default(3000), + DATABASE_URL: z.string().optional(), + SESSION_SECRET: z.string().optional(), + TOKEN_ENCRYPTION_KEY: z.string().optional(), + INNGEST_EVENT_KEY: z.string().optional(), + INNGEST_SIGNING_KEY: z.string().optional(), +}); + +// Validate at import time +const parsed = envSchema.safeParse(process.env); + +if (!parsed.success) { + console.error('[joyus] Invalid environment configuration:'); + for (const issue of parsed.error.issues) { + console.error(` ${issue.path.join('.')}: ${issue.message}`); + } + if (process.env.NODE_ENV === 'production') { + throw new Error('FATAL: Invalid environment configuration'); + } +} + +export const config = parsed.success ? parsed.data : envSchema.parse({}); diff --git a/joyus-ai-mcp-server/src/content/generation/index.ts b/joyus-ai-mcp-server/src/content/generation/index.ts index b1cc99f..4f42a08 100644 --- a/joyus-ai-mcp-server/src/content/generation/index.ts +++ b/joyus-ai-mcp-server/src/content/generation/index.ts @@ -7,7 +7,8 @@ import { drizzle } from 'drizzle-orm/node-postgres'; import { createId } from '@paralleldrive/cuid2'; import { contentGenerationLogs, contentOperationLogs } from '../schema.js'; import type { ResolvedEntitlements, GenerationResult } from '../types.js'; -import { ContentRetriever, type SearchService, type RetrievalResult, type RetrievedItem } from './retriever.js'; +import { ContentRetriever, type RetrievalResult, type RetrievedItem } from './retriever.js'; +import type { SearchService } from '../search/index.js'; import { ContentGenerator, PlaceholderGenerationProvider, @@ -110,7 +111,6 @@ export class GenerationService { // Re-exports so callers can import everything from this module export { ContentRetriever, - type SearchService, type RetrievalResult, type RetrievedItem, } from './retriever.js'; diff --git a/joyus-ai-mcp-server/src/content/generation/retriever.ts b/joyus-ai-mcp-server/src/content/generation/retriever.ts index 2ae197f..ffbecfd 100644 --- a/joyus-ai-mcp-server/src/content/generation/retriever.ts +++ b/joyus-ai-mcp-server/src/content/generation/retriever.ts @@ -1,14 +1,15 @@ /** * ContentRetriever — fetches relevant content items for generation. * - * Filters accessible sources by entitlements, runs a search, then hydrates - * full item bodies from the database for context assembly. + * Delegates search to the real SearchService (entitlement-filtered FTS), + * then hydrates full item bodies from the database for context assembly. */ import { drizzle } from 'drizzle-orm/node-postgres'; import { eq } from 'drizzle-orm'; import { contentItems } from '../schema.js'; -import type { ResolvedEntitlements, SearchResult } from '../types.js'; +import type { ResolvedEntitlements } from '../types.js'; +import type { SearchService } from '../search/index.js'; type DrizzleClient = ReturnType; @@ -26,14 +27,6 @@ export interface RetrievedItem { metadata: Record; } -export interface SearchService { - search( - query: string, - accessibleSourceIds: string[], - options?: { limit?: number }, - ): Promise; -} - export class ContentRetriever { constructor( private searchService: SearchService, @@ -45,18 +38,13 @@ export class ContentRetriever { entitlements: ResolvedEntitlements, options?: { sourceIds?: string[]; maxSources?: number }, ): Promise { - // 1. Filter sourceIds by entitlements - const accessibleSourceIds = options?.sourceIds - ? options.sourceIds.filter(id => entitlements.sourceIds.includes(id)) - : entitlements.sourceIds; - - // 2. Search via SearchService + // 1. Search via SearchService (handles entitlement → sourceId resolution internally) const maxSources = options?.maxSources ?? 5; - const results = await this.searchService.search(query, accessibleSourceIds, { + const results = await this.searchService.search(query, entitlements, { limit: maxSources, }); - // 3. Fetch full content for each result + // 2. Fetch full content for each result const items: RetrievedItem[] = []; for (const result of results) { const rows = await this.db @@ -77,7 +65,7 @@ export class ContentRetriever { } } - // 4. Format context text with numbered source labels + // 3. Format context text with numbered source labels const contextText = items .map((item, i) => `[Source ${i + 1}: "${item.title}"] ${item.body}`) .join('\n\n'); diff --git a/joyus-ai-mcp-server/src/content/index.ts b/joyus-ai-mcp-server/src/content/index.ts index dde5fa3..6be0608 100644 --- a/joyus-ai-mcp-server/src/content/index.ts +++ b/joyus-ai-mcp-server/src/content/index.ts @@ -11,7 +11,8 @@ import type { DrizzleClient } from './types.js'; import { connectorRegistry } from './connectors/index.js'; import { PgFtsProvider, SearchService } from './search/index.js'; import { EntitlementCache, EntitlementService, HttpEntitlementResolver } from './entitlements/index.js'; -import { GenerationService, PlaceholderGenerationProvider, type SearchService as GenSearchService } from './generation/index.js'; +import { GenerationService, PlaceholderGenerationProvider } from './generation/index.js'; +import { setContentContext } from '../tools/executor.js'; import { SyncEngine, initializeSyncScheduler } from './sync/index.js'; import { HealthChecker } from './monitoring/health.js'; import { MetricsCollector } from './monitoring/metrics.js'; @@ -35,8 +36,17 @@ export async function initializeContentModule( const searchProvider = new PgFtsProvider(db); const searchService = new SearchService(searchProvider, db); + // Inject SearchService into content tool executor + setContentContext({ searchService }); + // 2. Entitlements const entitlementCache = new EntitlementCache(); + + // Clean up expired cache entries every 5 minutes + const cacheCleanupInterval = setInterval(() => { + entitlementCache.cleanup(); + }, 5 * 60 * 1000); + cacheCleanupInterval.unref(); const entitlementResolver = new HttpEntitlementResolver({ name: 'default', defaultTtlSeconds: 300, @@ -50,10 +60,10 @@ export async function initializeContentModule( }); const entitlementService = new EntitlementService(entitlementResolver, entitlementCache, db); - // 3. Generation (bridge search service to generation's expected interface) + // 3. Generation const generationProvider = new PlaceholderGenerationProvider(); const generationService = new GenerationService( - searchService as unknown as GenSearchService, + searchService, generationProvider, db, ); diff --git a/joyus-ai-mcp-server/src/db/encryption.ts b/joyus-ai-mcp-server/src/db/encryption.ts index 0172870..bbdc931 100644 --- a/joyus-ai-mcp-server/src/db/encryption.ts +++ b/joyus-ai-mcp-server/src/db/encryption.ts @@ -1,47 +1,58 @@ /** * Token Encryption Utilities - * AES-256 encryption for OAuth tokens at rest + * AES-256-GCM encryption for OAuth tokens at rest */ -import CryptoJS from 'crypto-js'; +import crypto from 'node:crypto'; -const ENCRYPTION_KEY = process.env.TOKEN_ENCRYPTION_KEY; +const ALGORITHM = 'aes-256-gcm'; +const IV_LENGTH = 16; +const ENCRYPTION_KEY = process.env.TOKEN_ENCRYPTION_KEY; if (!ENCRYPTION_KEY) { - console.warn('⚠️ TOKEN_ENCRYPTION_KEY not set - tokens will not be encrypted!'); + if (process.env.NODE_ENV === 'production') { + throw new Error('FATAL: TOKEN_ENCRYPTION_KEY is required in production'); + } + console.warn('[joyus] TOKEN_ENCRYPTION_KEY not set - using development fallback'); } /** * Encrypt a token for storage */ export function encryptToken(token: string): string { - if (!ENCRYPTION_KEY) { - return token; // Dev fallback - not for production! - } - return CryptoJS.AES.encrypt(token, ENCRYPTION_KEY).toString(); + if (!ENCRYPTION_KEY) return token; // Dev-only fallback + const key = Buffer.from(ENCRYPTION_KEY, 'hex'); + const iv = crypto.randomBytes(IV_LENGTH); + const cipher = crypto.createCipheriv(ALGORITHM, key, iv); + const encrypted = Buffer.concat([cipher.update(token, 'utf8'), cipher.final()]); + const tag = cipher.getAuthTag(); + return `${iv.toString('hex')}:${tag.toString('hex')}:${encrypted.toString('hex')}`; } /** * Decrypt a stored token */ -export function decryptToken(encryptedToken: string): string { - if (!ENCRYPTION_KEY) { - return encryptedToken; - } - const bytes = CryptoJS.AES.decrypt(encryptedToken, ENCRYPTION_KEY); - return bytes.toString(CryptoJS.enc.Utf8); +export function decryptToken(stored: string): string { + if (!ENCRYPTION_KEY) return stored; // Dev-only fallback + const parts = stored.split(':'); + if (parts.length !== 3 || !parts[0] || !parts[1]) return stored; // Legacy plaintext fallback + const [ivHex, tagHex, encHex] = parts; + const key = Buffer.from(ENCRYPTION_KEY, 'hex'); + const decipher = crypto.createDecipheriv(ALGORITHM, key, Buffer.from(ivHex, 'hex')); + decipher.setAuthTag(Buffer.from(tagHex, 'hex')); + return decipher.update(Buffer.from(encHex, 'hex'), undefined, 'utf8') + decipher.final('utf8'); } /** * Generate a random MCP token for user authentication */ export function generateMcpToken(): string { - return CryptoJS.lib.WordArray.random(32).toString(CryptoJS.enc.Hex); + return crypto.randomBytes(32).toString('hex'); } /** * Generate OAuth state parameter */ export function generateOAuthState(): string { - return CryptoJS.lib.WordArray.random(16).toString(CryptoJS.enc.Hex); + return crypto.randomBytes(16).toString('hex'); } diff --git a/joyus-ai-mcp-server/src/index.ts b/joyus-ai-mcp-server/src/index.ts index e81488d..0cb23c9 100644 --- a/joyus-ai-mcp-server/src/index.ts +++ b/joyus-ai-mcp-server/src/index.ts @@ -11,9 +11,12 @@ * Transport: Streamable HTTP (recommended for remote MCP servers) */ +import './config.js'; // Env validation — must be first + import cors from 'cors'; import { config } from 'dotenv'; import express, { Request, Response, NextFunction } from 'express'; +import rateLimit from 'express-rate-limit'; import session from 'express-session'; import helmet from 'helmet'; @@ -37,9 +40,38 @@ config(); const app = express(); const PORT = process.env.PORT || 3000; +// Session secret fail-closed in production +const SESSION_SECRET = process.env.SESSION_SECRET; +if (!SESSION_SECRET && process.env.NODE_ENV === 'production') { + throw new Error('FATAL: SESSION_SECRET is required in production'); +} + +// Rate limiters +const apiLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 100, + standardHeaders: true, + legacyHeaders: false, +}); + +const authLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 20, // stricter for auth + standardHeaders: true, + legacyHeaders: false, +}); + // Security middleware app.use(helmet({ - contentSecurityPolicy: false // Disable for API + contentSecurityPolicy: { + directives: { + defaultSrc: ["'self'"], + scriptSrc: ["'self'", "'unsafe-inline'"], + styleSrc: ["'self'", "'unsafe-inline'"], + imgSrc: ["'self'", "data:"], + connectSrc: ["'self'"], + }, + }, })); app.use(cors({ origin: process.env.ALLOWED_ORIGINS?.split(',') || ['http://localhost:3000'], @@ -48,12 +80,13 @@ app.use(cors({ app.use(express.json()); app.use(express.urlencoded({ extended: true })); // For form submissions app.use(session({ - secret: process.env.SESSION_SECRET || 'change-me-in-production', + secret: SESSION_SECRET || 'dev-only-secret-not-for-production', resave: false, saveUninitialized: false, cookie: { secure: process.env.NODE_ENV === 'production', httpOnly: true, + sameSite: 'lax', maxAge: 24 * 60 * 60 * 1000 // 24 hours } })); @@ -160,7 +193,7 @@ app.get('/health', async (req, res) => { }); // Auth routes (OAuth callbacks, token management) -app.use('/auth', authRouter); +app.use('/auth', authLimiter, authRouter); // Task management routes (scheduled tasks) app.use('/tasks', taskRouter); @@ -319,8 +352,8 @@ app.listen(PORT, async () => { connectionString: process.env.DATABASE_URL ?? '', }); - // Mount pipeline routes - app.use('/api', pipelineModule.router); + // Mount pipeline routes (auth + rate limit) + app.use('/api', apiLimiter, requireBearerToken, pipelineModule.router); // Inject pipeline deps into tool executor setPipelineContext({ diff --git a/joyus-ai-mcp-server/src/inngest/index.ts b/joyus-ai-mcp-server/src/inngest/index.ts index 775ace5..8f3396f 100644 --- a/joyus-ai-mcp-server/src/inngest/index.ts +++ b/joyus-ai-mcp-server/src/inngest/index.ts @@ -4,9 +4,9 @@ * Exports client and all registered functions. * Import `allFunctions` to pass to the serve() adapter in index.ts. * - * `allFunctions` registers all pipeline functions with an empty step registry - * (stub mode). When a real StepHandlerRegistry is available (see pipelines/init.ts), - * construct the pipeline functions with it and replace allFunctions at serve() time. + * Pipeline functions are created with `lazyRegistry`, a proxy that delegates + * to the mutable singleton at call time. Once `setStepRegistry()` is called + * during pipeline module init, all functions see the real registry. */ export { inngest } from './client.js'; export { stubFunction } from './functions/stub.js'; @@ -16,24 +16,19 @@ export { createContentAuditPipeline } from './functions/content-audit-pipeline.j export { createRegulatoryChangeMonitorPipeline } from './functions/regulatory-change-monitor-pipeline.js'; export { createInngestAdapter } from './adapter.js'; export type { InngestStep, InngestStepHandlerAdapter } from './adapter.js'; +export { setStepRegistry, getStepRegistry } from './registry.js'; import { stubFunction } from './functions/stub.js'; import { createCorpusUpdatePipeline } from './functions/corpus-update-pipeline.js'; import { createScheduleTickPipeline } from './functions/schedule-tick-pipeline.js'; import { createContentAuditPipeline } from './functions/content-audit-pipeline.js'; import { createRegulatoryChangeMonitorPipeline } from './functions/regulatory-change-monitor-pipeline.js'; -import type { StepHandlerRegistry } from '../pipelines/engine/step-runner.js'; - -// Empty registry — functions run in stub mode until a real registry is provided. -// WP03 (deletion cleanup) will restructure how the registry is wired. -const emptyRegistry: StepHandlerRegistry = { - getHandler: () => undefined, -}; +import { lazyRegistry } from './registry.js'; export const allFunctions = [ stubFunction, - createCorpusUpdatePipeline(emptyRegistry), - createContentAuditPipeline(emptyRegistry), - createRegulatoryChangeMonitorPipeline(emptyRegistry), + createCorpusUpdatePipeline(lazyRegistry), + createContentAuditPipeline(lazyRegistry), + createRegulatoryChangeMonitorPipeline(lazyRegistry), createScheduleTickPipeline(), ]; diff --git a/joyus-ai-mcp-server/src/inngest/registry.ts b/joyus-ai-mcp-server/src/inngest/registry.ts new file mode 100644 index 0000000..0f7dd62 --- /dev/null +++ b/joyus-ai-mcp-server/src/inngest/registry.ts @@ -0,0 +1,40 @@ +/** + * Inngest Step Registry — mutable singleton with lazy proxy. + * + * Starts empty so pipeline functions can be created at module load time. + * Populated with the real registry during `initializePipelineModule()`. + * + * `lazyRegistry` is a proxy that delegates to the current singleton at + * call time, so Inngest functions created with it at module load always + * see the real registry once it has been set. + */ + +import type { StepHandlerRegistry } from '../pipelines/engine/step-runner.js'; + +// Mutable singleton — populated during pipeline module initialization +let _registry: StepHandlerRegistry = { + getHandler: () => undefined, +}; + +/** + * Replace the step registry singleton. Called once during pipeline module init. + */ +export function setStepRegistry(registry: StepHandlerRegistry): void { + _registry = registry; +} + +/** + * Get the current step registry. + */ +export function getStepRegistry(): StepHandlerRegistry { + return _registry; +} + +/** + * A proxy registry that always delegates to the current singleton. + * Pass this to pipeline function factories at module load time so they + * pick up the real registry at invocation time. + */ +export const lazyRegistry: StepHandlerRegistry = { + getHandler: (...args) => _registry.getHandler(...args), +}; diff --git a/joyus-ai-mcp-server/src/pipelines/init.ts b/joyus-ai-mcp-server/src/pipelines/init.ts index be51940..b421072 100644 --- a/joyus-ai-mcp-server/src/pipelines/init.ts +++ b/joyus-ai-mcp-server/src/pipelines/init.ts @@ -25,6 +25,7 @@ import { startEscalationJob, stopEscalationJob } from './review/index.js'; import { createPipelineRouter, type PipelineRouterDeps } from './routes.js'; import type { ToolDefinition } from '../tools/index.js'; import { pipelineTools } from '../tools/pipeline-tools.js'; +import { setStepRegistry } from '../inngest/registry.js'; // ============================================================ // CONFIG & MODULE INTERFACE @@ -64,6 +65,9 @@ export async function initializePipelineModule( const triggerRegistry = defaultTriggerRegistry; const stepRegistry = createStepRegistry(stepHandlerDeps ?? {}); + // Populate the Inngest lazy registry so pipeline functions see real handlers + setStepRegistry(stepRegistry); + // 3. Step runner const stepRunner = new StepRunner(db, stepRegistry); diff --git a/joyus-ai-mcp-server/src/pipelines/routes.ts b/joyus-ai-mcp-server/src/pipelines/routes.ts index bd26118..b2fea1b 100644 --- a/joyus-ai-mcp-server/src/pipelines/routes.ts +++ b/joyus-ai-mcp-server/src/pipelines/routes.ts @@ -51,28 +51,16 @@ export interface PipelineRouterDeps { // ============================================================ /** - * Extract tenantId from request. Uses x-tenant-id header, falling back - * to the authenticated user ID (matching the content executor pattern). + * Extract tenantId from request. Derives from authenticated user only + * — never trust headers for tenant identity. */ function getTenantId(req: Request): string { - const header = req.headers['x-tenant-id']; - if (typeof header === 'string' && header.length > 0) { - return header; - } - // Fall back to mcpUser id (matches content executor pattern) + // Derive from authenticated user — never trust headers + if (req.session?.userId) return req.session.userId; const user = (req as unknown as Record)['mcpUser'] as | { id: string } | undefined; - if (user?.id) { - return user.id; - } - // Last resort: session userId - if (req.session) { - const session = req.session as unknown as Record; - if (session['userId']) { - return session['userId'] as string; - } - } + if (user?.id) return user.id; return ''; } diff --git a/joyus-ai-mcp-server/src/scheduler/routes.ts b/joyus-ai-mcp-server/src/scheduler/routes.ts index 8887c50..f1368e8 100644 --- a/joyus-ai-mcp-server/src/scheduler/routes.ts +++ b/joyus-ai-mcp-server/src/scheduler/routes.ts @@ -17,6 +17,10 @@ import { scheduleTask, unscheduleTask, runTask, reloadTask, getSchedulerStatus } export const taskRouter = Router(); +function escapeHtml(str: string): string { + return str.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"').replace(/'/g, '''); +} + // ============================================================ // Task Management UI // ============================================================ @@ -97,15 +101,15 @@ taskRouter.get('/', requireSessionOrRedirect, async (req: Request, res: Response
- ${task.name} + ${escapeHtml(task.name)} ${task.enabled ? 'Enabled' : 'Disabled'}
- - Edit -
+ + Edit + @@ -113,21 +117,21 @@ taskRouter.get('/', requireSessionOrRedirect, async (req: Request, res: Response
-

${task.description || 'No description'}

+

${escapeHtml(task.description || 'No description')}

- - - + + + @@ -397,22 +401,22 @@ taskRouter.get('/:id/edit', requireSessionOrRedirect, async (req: Request, res: -

Edit Task: ${task.name}

+

Edit Task: ${escapeHtml(task.name)}

- +
- +
- +
- +
@@ -428,17 +432,17 @@ taskRouter.get('/:id/edit', requireSessionOrRedirect, async (req: Request, res:
- +
- +
- +
@@ -447,7 +451,7 @@ taskRouter.get('/:id/edit', requireSessionOrRedirect, async (req: Request, res:
- + diff --git a/joyus-ai-mcp-server/src/tools/executor.ts b/joyus-ai-mcp-server/src/tools/executor.ts index dc1a471..7ed01ea 100644 --- a/joyus-ai-mcp-server/src/tools/executor.ts +++ b/joyus-ai-mcp-server/src/tools/executor.ts @@ -43,6 +43,26 @@ export function setPipelineContext(deps: PipelineContextDeps): void { _pipelineContext = deps; } +// ============================================================ +// CONTENT CONTEXT (injected during content module startup) +// ============================================================ + +import type { SearchService } from '../content/search/index.js'; + +interface ContentContextDeps { + searchService: SearchService; +} + +let _contentContext: ContentContextDeps | null = null; + +/** + * Inject content module dependencies for tool execution. + * Called once during content module initialization. + */ +export function setContentContext(deps: ContentContextDeps): void { + _contentContext = deps; +} + /** * Execute a tool by name with the given input */ @@ -53,7 +73,12 @@ export async function executeTool(userId: string, toolName: string, input: Recor if (toolName.startsWith('content_')) { const tenantId = userId; // tenant resolution deferred to WP12; use userId as tenantId for now - return executeContentTool(toolName, input, { userId, tenantId, db }); + return executeContentTool(toolName, input, { + userId, + tenantId, + db, + searchService: _contentContext?.searchService, + }); } if (toolName.startsWith('pipeline_') || toolName.startsWith('template_') || toolName.startsWith('review_')) { diff --git a/joyus-ai-mcp-server/src/tools/executors/content-executor.ts b/joyus-ai-mcp-server/src/tools/executors/content-executor.ts index 9468ac2..62b0a23 100644 --- a/joyus-ai-mcp-server/src/tools/executors/content-executor.ts +++ b/joyus-ai-mcp-server/src/tools/executors/content-executor.ts @@ -16,6 +16,8 @@ import { contentDriftReports, contentEntitlements, } from '../../content/schema.js'; +import type { SearchService } from '../../content/search/index.js'; +import type { ResolvedEntitlements } from '../../content/types.js'; type DrizzleClient = ReturnType; @@ -23,6 +25,7 @@ export interface ContentExecutorContext { userId: string; tenantId: string; db: DrizzleClient; + searchService?: SearchService; } /** @@ -213,7 +216,48 @@ export async function executeContentTool( const limit = Math.min((input.limit as number | undefined) ?? 10, 50); const offset = (input.offset as number | undefined) ?? 0; - // Get sources this tenant can access + // Use SearchService with FTS when available + if (context.searchService) { + // Build minimal entitlements from tenant's active products + const tenantProducts = await db + .select({ id: contentProducts.id }) + .from(contentProducts) + .where(and(eq(contentProducts.tenantId, tenantId), eq(contentProducts.isActive, true))); + + const entitlements: ResolvedEntitlements = { + productIds: tenantProducts.map((p) => p.id), + sourceIds: [], + profileIds: [], + resolvedFrom: 'tool-executor', + resolvedAt: new Date(), + }; + + const results = await context.searchService.search(query, entitlements, { + limit, + offset, + sourceId: sourceIds?.[0], + }); + + return { + items: results.map((r) => ({ + id: r.itemId, + sourceId: r.sourceId, + title: r.title, + excerpt: r.excerpt, + score: r.score, + sourceName: r.sourceName, + sourceType: r.sourceType, + isStale: r.isStale, + stalenessWarning: r.stalenessWarning, + })), + total: results.length, + query, + limit, + offset, + }; + } + + // Fallback: LIKE search when SearchService is not wired const tenantSources = await db .select({ id: contentSources.id }) .from(contentSources) @@ -224,7 +268,6 @@ export async function executeContentTool( return { items: [], total: 0, query }; } - // Restrict to requested sourceIds if provided, intersected with tenant sources const effectiveSourceIds = sourceIds && sourceIds.length > 0 ? sourceIds.filter((id) => tenantSourceIds.includes(id)) @@ -234,7 +277,6 @@ export async function executeContentTool( return { items: [], total: 0, query }; } - // Simple LIKE search on title and body (FTS wired via SearchService in WP12) const searchPattern = `%${query}%`; const rows = await db .select({ @@ -270,7 +312,6 @@ export async function executeContentTool( query, limit, offset, - note: 'Full-text search will be available after SearchService integration (WP12).', }; } diff --git a/joyus-ai-mcp-server/tests/auth/routes.test.ts b/joyus-ai-mcp-server/tests/auth/routes.test.ts new file mode 100644 index 0000000..299235a --- /dev/null +++ b/joyus-ai-mcp-server/tests/auth/routes.test.ts @@ -0,0 +1,843 @@ +/** + * Route handler tests for auth/routes.ts + * + * Tests the OAuth callback flows (Google, Jira, Slack, GitHub), + * the disconnect route, logout, and the portal home page. + * + * Pattern: mock all external dependencies (DB, axios, encryption), + * extract route handlers from authRouter.stack, and call them with + * mock req/res objects — matching the approach in middleware.test.ts. + * + * NOTE: The disconnect route is currently GET /:service/disconnect. + * Tests below are written for POST to be forward-compatible with the + * planned migration to POST. When the route is changed, remove the + * "expected to fail until route is POST" comment from the POST test. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Request, Response, NextFunction } from 'express'; +import type { Router } from 'express'; + +// ─── Module mocks (must be declared before imports) ─────────────────────────── + +vi.mock('../../src/db/client.js', () => { + const mockDb = { + select: vi.fn(), + insert: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + }; + return { + db: mockDb, + users: 'users_table', + connections: 'connections_table', + oauthStates: 'oauth_states_table', + }; +}); + +vi.mock('../../src/db/encryption.js', () => ({ + encryptToken: vi.fn((t: string) => `enc(${t})`), + generateMcpToken: vi.fn(() => 'generated-mcp-token'), + generateOAuthState: vi.fn(() => 'random-state-abc'), +})); + +vi.mock('axios'); + +// requireSessionOrRedirect used by start/disconnect routes +vi.mock('../../src/auth/middleware.js', () => ({ + requireSessionOrRedirect: vi.fn( + (req: Request, res: Response, next: NextFunction) => { + if (!req.session?.userId) { + res.redirect('/auth'); + return; + } + next(); + } + ), +})); + +// ─── Imports (after mocks) ──────────────────────────────────────────────────── + +import axios from 'axios'; +import { db } from '../../src/db/client.js'; +import { authRouter } from '../../src/auth/routes.js'; + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +type RouteLayer = { + route?: { + path: string; + methods: Record; + stack: Array<{ handle: (req: Request, res: Response, next: NextFunction) => unknown }>; + }; +}; + +/** + * Find the last registered handler for a given method + path on the router. + * Returns the final handler in the middleware stack for that route. + */ +function findHandler( + router: Router, + method: string, + path: string +): ((req: Request, res: Response, next: NextFunction) => unknown) | undefined { + const stack = (router as unknown as { stack: RouteLayer[] }).stack; + const layer = stack.find( + (l) => + l.route && + l.route.path === path && + l.route.methods[method.toLowerCase()] + ); + if (!layer?.route) return undefined; + // Return the last handler (the actual route handler, not middleware) + const handlers = layer.route.stack; + return handlers[handlers.length - 1].handle; +} + +function createMockReq(overrides: Record = {}): Request { + return { + headers: {}, + query: {}, + params: {}, + session: {}, + body: {}, + ...overrides, + } as unknown as Request; +} + +function createMockRes() { + const res = { + _status: 200, + _body: null as unknown, + _redirect: null as string | null, + _sent: false, + status(code: number) { + res._status = code; + return res; + }, + send(body: unknown) { + res._body = body; + res._sent = true; + return res; + }, + json(data: unknown) { + res._body = data; + res._sent = true; + return res; + }, + redirect(url: string) { + res._redirect = url; + return res; + }, + }; + return res as unknown as Response & { + _status: number; + _body: unknown; + _redirect: string | null; + _sent: boolean; + }; +} + +/** + * Build a fluent select chain that resolves to `rows` via .limit(). + * Use for queries that call .select().from().where().limit(1). + */ +function selectReturning(rows: unknown[]) { + const chain = { + from: vi.fn().mockReturnThis(), + where: vi.fn().mockReturnThis(), + limit: vi.fn().mockResolvedValue(rows), + }; + vi.mocked(db.select).mockReturnValueOnce(chain as never); + return chain; +} + +/** + * Build a fluent select chain that resolves to `rows` via .where() (no .limit()). + * Use for queries that call .select().from().where() and await the result directly, + * such as the connections fetch in the dashboard route. + */ +function selectResolvingOnWhere(rows: unknown[]) { + const chain = { + from: vi.fn().mockReturnThis(), + where: vi.fn().mockResolvedValue(rows), + }; + vi.mocked(db.select).mockReturnValueOnce(chain as never); + return chain; +} + +/** Build an insert chain that returns `rows` from .returning() */ +function insertReturning(rows: unknown[]) { + const chain = { + values: vi.fn().mockReturnThis(), + returning: vi.fn().mockResolvedValue(rows), + }; + vi.mocked(db.insert).mockReturnValueOnce(chain as never); + return chain; +} + +/** Build an insert chain that resolves without .returning() */ +function insertResolving() { + const chain = { + values: vi.fn().mockResolvedValue(undefined), + }; + vi.mocked(db.insert).mockReturnValueOnce(chain as never); + return chain; +} + +/** Build an update chain */ +function updateResolving() { + const chain = { + set: vi.fn().mockReturnThis(), + where: vi.fn().mockResolvedValue(undefined), + }; + vi.mocked(db.update).mockReturnValueOnce(chain as never); + return chain; +} + +/** Build a delete chain */ +function deleteResolving() { + const chain = { + where: vi.fn().mockResolvedValue(undefined), + }; + vi.mocked(db.delete).mockReturnValueOnce(chain as never); + return chain; +} + +// ─── Helper data ────────────────────────────────────────────────────────────── + +const VALID_STATE_ROW = { + id: 'state-id-1', + state: 'valid-state', + userId: 'pending', + service: 'GOOGLE', + expiresAt: new Date(Date.now() + 5 * 60 * 1000), // valid for 5 min +}; + +const EXISTING_USER = { + id: 'user-1', + email: 'operator@example.com', + name: 'Operator A', + mcpToken: 'existing-mcp-token', +}; + +// ─── escapeHtml (tested through dashboard HTML output) ─────────────────────── + +describe('escapeHtml (via dashboard route)', () => { + let handler: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + handler = findHandler(authRouter, 'get', '/'); + }); + + it('escapes special HTML characters in user display name', async () => { + const xssUser = { + ...EXISTING_USER, + name: '', + }; + selectReturning([xssUser]); // user lookup (.limit(1)) + selectResolvingOnWhere([]); // connections lookup (no .limit()) + + const req = createMockReq({ session: { userId: 'user-1' } }); + const res = createMockRes(); + + await handler!(req, res, vi.fn()); + + const html = res._body as string; + expect(html).toContain('<script>'); + expect(html).not.toContain('
Type: ${task.taskType.replace(/_/g, ' ')}Schedule: ${task.schedule}Timezone: ${task.timezone}Type: ${escapeHtml(task.taskType.replace(/_/g, ' '))}Schedule: ${escapeHtml(task.schedule)}Timezone: ${escapeHtml(task.timezone)}
Last Run: ${task.lastRunAt ? new Date(task.lastRunAt).toLocaleString() : 'Never'} Next Run: ${task.nextRunAt ? new Date(task.nextRunAt).toLocaleString() : 'N/A'} Notify: - ${task.notifySlack ? `Slack: ${task.notifySlack}` : ''} - ${task.notifyEmail ? `Email: ${task.notifyEmail}` : ''} + ${task.notifySlack ? `Slack: ${escapeHtml(task.notifySlack)}` : ''} + ${task.notifyEmail ? `Email: ${escapeHtml(task.notifyEmail)}` : ''} ${!task.notifySlack && !task.notifyEmail ? 'None' : ''}