diff --git a/docker-compose.nas.yml b/docker-compose.nas.yml index 3b67c247..3d3a1216 100644 --- a/docker-compose.nas.yml +++ b/docker-compose.nas.yml @@ -80,6 +80,11 @@ services: timeout: 10s retries: 3 start_period: 20s + deploy: + resources: + limits: + memory: 2G + cpus: "2.0" # ── SSH Git server ───────────────────────────────────────── ssh-git: @@ -90,6 +95,11 @@ services: PROCESS_TYPE: ssh ports: - "${SSH_PORT:-2222}:2222" + deploy: + resources: + limits: + memory: 512M + cpus: "1.0" # ── Background worker ────────────────────────────────────── worker: @@ -107,6 +117,11 @@ services: timeout: 10s retries: 3 start_period: 20s + deploy: + resources: + limits: + memory: 1G + cpus: "1.0" # ── PostgreSQL ───────────────────────────────────────────── postgres: @@ -126,6 +141,11 @@ services: interval: 10s timeout: 5s retries: 5 + deploy: + resources: + limits: + memory: 1G + cpus: "1.0" # ── Redis ────────────────────────────────────────────────── redis: @@ -151,6 +171,11 @@ services: interval: 10s timeout: 5s retries: 5 + deploy: + resources: + limits: + memory: 512M + cpus: "0.5" # ── Cloudflare Tunnel (optional) ─────────────────────────── cloudflared: diff --git a/docker-compose.production.yml b/docker-compose.production.yml index fad2fbe3..91ae07ce 100644 --- a/docker-compose.production.yml +++ b/docker-compose.production.yml @@ -57,7 +57,7 @@ services: - DATABASE_SSL=false - REDIS_URL=redis://:${REDIS_PASSWORD:?Set REDIS_PASSWORD in .env}@redis:6379 - JWT_SECRET=${JWT_SECRET:?Set JWT_SECRET in .env} - - SESSION_SECRET=${SESSION_SECRET:-${JWT_SECRET}} + - SESSION_SECRET=${SESSION_SECRET:?Set SESSION_SECRET in .env (must differ from JWT_SECRET)} - INTERNAL_HOOK_SECRET=${INTERNAL_HOOK_SECRET:?Set INTERNAL_HOOK_SECRET in .env} - CRON_SECRET=${CRON_SECRET:?Set CRON_SECRET in .env} - AI_CONFIG_ENCRYPTION_KEY=${AI_CONFIG_ENCRYPTION_KEY:?Set AI_CONFIG_ENCRYPTION_KEY in .env} diff --git a/docker-compose.yml b/docker-compose.yml index 24d66b93..34d18a21 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -68,13 +68,19 @@ services: redis: condition: service_healthy networks: - - och + - opencodehub-internal + - opencodehub-external healthcheck: test: ["CMD", "wget", "--no-verbose", "--tries=1", "--spider", "http://localhost:4321/api/health"] interval: 30s timeout: 10s retries: 3 start_period: 20s + deploy: + resources: + limits: + memory: 2G + cpus: "2.0" # ── SSH Git server ───────────────────────────────────────── ssh-git: @@ -108,7 +114,19 @@ services: redis: condition: service_healthy networks: - - och + - opencodehub-internal + - opencodehub-external + healthcheck: + test: ["CMD-SHELL", "nc -z localhost 2222 || exit 1"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 15s + deploy: + resources: + limits: + memory: 512M + cpus: "1.0" # ── Background worker (merge queue, webhooks, mirrors, digests) ── worker: @@ -143,13 +161,18 @@ services: redis: condition: service_healthy networks: - - och + - opencodehub-internal healthcheck: test: ["CMD", "wget", "--no-verbose", "--tries=1", "--spider", "http://localhost:9090/healthz"] interval: 30s timeout: 10s retries: 3 start_period: 20s + deploy: + resources: + limits: + memory: 1G + cpus: "1.0" # ── PostgreSQL ───────────────────────────────────────────── postgres: @@ -163,12 +186,17 @@ services: volumes: - postgres-data:/var/lib/postgresql/data networks: - - och + - opencodehub-internal healthcheck: test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-opencodehub} -d ${POSTGRES_DB:-opencodehub}"] interval: 10s timeout: 5s retries: 5 + deploy: + resources: + limits: + memory: 1G + cpus: "1.0" # ── Redis (sessions, caching, distributed locking, queues) ── redis: @@ -188,12 +216,17 @@ services: volumes: - redis-data:/data networks: - - och + - opencodehub-internal healthcheck: test: ["CMD", "redis-cli", "-a", "${REDIS_PASSWORD}", "ping"] interval: 10s timeout: 5s retries: 5 + deploy: + resources: + limits: + memory: 512M + cpus: "0.5" # ── CI/CD Runner (Docker-in-Docker, optional) ────────────── runner: @@ -216,7 +249,18 @@ services: depends_on: - app networks: - - och + - opencodehub-external + healthcheck: + test: ["CMD", "wget", "--no-verbose", "--tries=1", "--spider", "http://localhost:9090/healthz"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 20s + deploy: + resources: + limits: + memory: 2G + cpus: "2.0" profiles: - with-runner @@ -234,7 +278,7 @@ services: volumes: - minio-data:/data networks: - - och + - opencodehub-external healthcheck: test: ["CMD", "mc", "ready", "local"] interval: 30s @@ -264,5 +308,7 @@ volumes: driver: local networks: - och: + opencodehub-internal: + driver: bridge + opencodehub-external: driver: bridge diff --git a/scripts/worker.ts b/scripts/worker.ts index fbf631f3..246afabc 100644 --- a/scripts/worker.ts +++ b/scripts/worker.ts @@ -250,9 +250,11 @@ function setupGracefulShutdown(healthServer: ReturnType) { process.on("SIGTERM", () => shutdown("SIGTERM")); process.on("uncaughtException", (err) => { logger.error({ err }, "Uncaught exception in worker"); + process.exit(1); }); process.on("unhandledRejection", (reason) => { logger.error({ reason }, "Unhandled rejection in worker"); + process.exit(1); }); } diff --git a/src/db/index.ts b/src/db/index.ts index d400bb4d..9223c786 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -30,6 +30,7 @@ let db: | LibSQLDatabase | NodePgDatabase | null = null; +let pgPool: pg.Pool | null = null; /** * Infer database driver from connection URL when no explicit driver is set. @@ -113,6 +114,10 @@ export function getDatabase(): ssl: sslEnabled ? { rejectUnauthorized } : undefined, max: parseInt(process.env.DATABASE_POOL_SIZE || "10", 10), }); + pool.on('error', (err) => { + logger.error({ err }, 'Unexpected database pool error'); + }); + pgPool = pool; db = drizzlePg(pool, { schema }); logger.info( { @@ -179,9 +184,11 @@ export async function closeDatabase(): Promise { // @ts-ignore db.close(); } - // For PG pool, we might need to close the pool if we had access to it, - // but Drizzle doesn't expose it directly on the db instance easily without type casting. - // In serverless/long-running app, closing might not be strictly necessary unless ensuring graceful shutdown. + // Close PostgreSQL pool if available + if (pgPool) { + await pgPool.end(); + pgPool = null; + } logger.info("Database connection closed"); db = null; diff --git a/src/db/schema/ai-reviews.ts b/src/db/schema/ai-reviews.ts index 4c04db7c..f2db6043 100644 --- a/src/db/schema/ai-reviews.ts +++ b/src/db/schema/ai-reviews.ts @@ -5,6 +5,7 @@ import { relations } from "drizzle-orm"; import { boolean, integer, pgTable, text, timestamp } from "drizzle-orm/pg-core"; +import { index } from "drizzle-orm/pg-core"; import { pullRequests } from "./pull-requests"; import { users } from "./users"; @@ -48,7 +49,9 @@ export const aiReviews = pgTable("ai_reviews", { // Error errorMessage: text("error_message"), -}); +}, (table) => [ + index("ai_reviews_pull_request_id_idx").on(table.pullRequestId), +]); // AI Review Suggestions - individual findings export const aiReviewSuggestions = pgTable("ai_review_suggestions", { diff --git a/src/db/schema/automations.ts b/src/db/schema/automations.ts index 269b79b5..ecb0d954 100644 --- a/src/db/schema/automations.ts +++ b/src/db/schema/automations.ts @@ -5,6 +5,7 @@ import { relations } from "drizzle-orm"; import { boolean, integer, pgTable, text, timestamp } from "drizzle-orm/pg-core"; +import { index } from "drizzle-orm/pg-core"; import { repositories } from "./repositories"; import { users } from "./users"; @@ -31,7 +32,9 @@ export const automationRules = pgTable("automation_rules", { lastRunAt: timestamp("last_run_at"), createdAt: timestamp("created_at").notNull().defaultNow(), updatedAt: timestamp("updated_at").notNull().defaultNow(), -}); +}, (table) => [ + index("automation_rules_repository_id_idx").on(table.repositoryId), +]); /** * Trigger types: diff --git a/src/db/schema/branch-protection.ts b/src/db/schema/branch-protection.ts index 62c9c0e0..8775dc4a 100644 --- a/src/db/schema/branch-protection.ts +++ b/src/db/schema/branch-protection.ts @@ -5,6 +5,7 @@ import { relations } from "drizzle-orm"; import { boolean, integer, pgTable, text, timestamp } from "drizzle-orm/pg-core"; +import { index } from "drizzle-orm/pg-core"; import { repositories } from "./repositories"; import { users } from "./users"; @@ -29,7 +30,9 @@ export const branchProtection = pgTable("branch_protection", { createdAt: timestamp("created_at").notNull().defaultNow(), updatedAt: timestamp("updated_at").notNull().defaultNow(), createdById: text("created_by_id").references(() => users.id), -}); +}, (table) => [ + index("branch_protection_repository_id_idx").on(table.repositoryId), +]); export const branchProtectionRelations = relations(branchProtection, ({ one }) => ({ repository: one(repositories, { diff --git a/src/db/schema/deploy-keys.ts b/src/db/schema/deploy-keys.ts index 710d817a..15e9f797 100644 --- a/src/db/schema/deploy-keys.ts +++ b/src/db/schema/deploy-keys.ts @@ -5,6 +5,7 @@ import { relations } from "drizzle-orm"; import { boolean, pgTable, text, timestamp } from "drizzle-orm/pg-core"; +import { index } from "drizzle-orm/pg-core"; import { repositories } from "./repositories"; export const deployKeys = pgTable("deploy_keys", { @@ -18,7 +19,9 @@ export const deployKeys = pgTable("deploy_keys", { readOnly: boolean("read_only").default(true).notNull(), createdAt: timestamp("created_at").notNull().defaultNow(), lastUsedAt: timestamp("last_used_at"), -}); +}, (table) => [ + index("deploy_keys_repository_id_idx").on(table.repositoryId), +]); export const deployKeysRelations = relations( deployKeys, diff --git a/src/db/schema/merge-queue.ts b/src/db/schema/merge-queue.ts index eec62a06..9c1e90f6 100644 --- a/src/db/schema/merge-queue.ts +++ b/src/db/schema/merge-queue.ts @@ -1,5 +1,5 @@ import { relations } from "drizzle-orm"; -import { boolean, integer, pgTable, text, timestamp } from "drizzle-orm/pg-core"; +import { boolean, index, integer, pgTable, text, timestamp } from "drizzle-orm/pg-core"; import { pullRequests } from "./pull-requests"; import { repositories } from "./repositories"; import { users } from "./users"; @@ -38,7 +38,13 @@ export const mergeQueue = pgTable("merge_queue", { startedAt: timestamp("started_at"), completedAt: timestamp("completed_at"), failureReason: text("failure_reason"), -}); + }, + (t) => ({ + repoIdx: index("merge_queue_repo_idx").on(t.repositoryId), + statusIdx: index("merge_queue_status_idx").on(t.status), + prIdx: index("merge_queue_pr_idx").on(t.pullRequestId), + }), +); // Alias for backwards compatibility export const mergeQueueItems = mergeQueue; diff --git a/src/db/schema/pull-requests.ts b/src/db/schema/pull-requests.ts index 2d89a6a4..c88601b1 100644 --- a/src/db/schema/pull-requests.ts +++ b/src/db/schema/pull-requests.ts @@ -196,7 +196,9 @@ export const pullRequestReviewers = pgTable("pull_request_reviewers", { .references(() => users.id, { onDelete: "cascade" }), isRequired: boolean("is_required").default(false), requestedAt: timestamp("requested_at").notNull().defaultNow(), -}); +}, (t) => ({ + prIdx: index("pr_reviewers_pr_idx").on(t.pullRequestId), +})); export const pullRequestChecks = pgTable( "pull_request_checks", diff --git a/src/db/schema/stacked-prs.ts b/src/db/schema/stacked-prs.ts index e923c013..ebf4c7aa 100644 --- a/src/db/schema/stacked-prs.ts +++ b/src/db/schema/stacked-prs.ts @@ -4,7 +4,7 @@ */ import { relations } from "drizzle-orm"; -import { integer, pgTable, text, timestamp } from "drizzle-orm/pg-core"; +import { index, integer, pgTable, text, timestamp } from "drizzle-orm/pg-core"; import { pullRequests } from "./pull-requests"; import { repositories } from "./repositories"; import { users } from "./users"; @@ -23,7 +23,11 @@ export const prStacks = pgTable("pr_stacks", { .references(() => users.id), createdAt: timestamp("created_at").notNull().defaultNow(), updatedAt: timestamp("updated_at").notNull().defaultNow(), -}); + }, + (t) => ({ + repoIdx: index("pr_stacks_repo_idx").on(t.repositoryId), + }), +); // Stack entries - individual PRs in a stack with ordering export const prStackEntries = pgTable("pr_stack_entries", { @@ -37,7 +41,13 @@ export const prStackEntries = pgTable("pr_stack_entries", { stackOrder: integer("stack_order").notNull(), // Position in stack (1 = base) parentPrId: text("parent_pr_id").references(() => pullRequests.id), createdAt: timestamp("created_at").notNull().defaultNow(), -}); + }, + (t) => ({ + stackIdx: index("pr_stack_entries_stack_idx").on(t.stackId), + prIdx: index("pr_stack_entries_pr_idx").on(t.pullRequestId), + parentPrIdx: index("pr_stack_entries_parent_pr_idx").on(t.parentPrId), + }), +); // Relations export const prStacksRelations = relations(prStacks, ({ one, many }) => ({ diff --git a/src/env.d.ts b/src/env.d.ts index 48c6f0d4..6494e3da 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -4,5 +4,6 @@ declare namespace App { interface Locals { user: import("./db/schema").User | null; session: import("./db/schema").Session | null; + cspNonce?: string; } } diff --git a/src/lib/graceful-shutdown.ts b/src/lib/graceful-shutdown.ts index 0cfa4363..4f06d8ee 100644 --- a/src/lib/graceful-shutdown.ts +++ b/src/lib/graceful-shutdown.ts @@ -9,17 +9,21 @@ * - Worker queues stop accepting new jobs */ +import { closeDatabase } from "@/db"; import { logger } from "@/lib/logger"; let isShuttingDown = false; const SHUTDOWN_TIMEOUT_MS = 30_000; -function gracefulShutdown(signal: string) { +async function gracefulShutdown(signal: string) { if (isShuttingDown) return; isShuttingDown = true; logger.info({ signal }, "Received shutdown signal, draining..."); + // Set flag so health checks return "shutting down" + process.env.OPCODEHUB_SHUTTING_DOWN = "1"; + // Force exit after timeout const forceTimer = setTimeout(() => { logger.error("Forced shutdown after timeout"); @@ -27,33 +31,41 @@ function gracefulShutdown(signal: string) { }, SHUTDOWN_TIMEOUT_MS); forceTimer.unref(); - // Let the process exit naturally once all handles are drained. - // The Node.js event loop will empty when no more async work is pending. - // We set a flag so any health check returns "shutting down". - process.env.OPCODEHUB_SHUTTING_DOWN = "1"; + try { + // Close database connections + await closeDatabase(); + logger.info("Database connections closed"); + } catch (err) { + logger.error({ err }, "Error closing database connections"); + } + + try { + // Close Redis if available — import dynamically to avoid circular deps + const { closeRedis } = await import("@/lib/redis"); + await closeRedis(); + } catch { + // Redis not available, skip + } - logger.info("Shutdown signal processed, waiting for in-flight requests..."); + logger.info("Shutdown complete, exiting..."); + + // Let the process exit naturally once all handles are drained + setTimeout(() => process.exit(0), 1000); } // Handle signals process.on("SIGTERM", () => gracefulShutdown("SIGTERM")); process.on("SIGINT", () => gracefulShutdown("SIGINT")); -// Handle uncaught errors — log and continue (don't crash) +// Handle uncaught errors — log and exit to let process manager restart process.on("uncaughtException", (err) => { - logger.error({ err }, "Uncaught exception"); - // In production, don't crash — log and keep serving - if (process.env.NODE_ENV !== "production") { - process.exit(1); - } + logger.fatal({ err }, "Uncaught exception — exiting to prevent corrupted state"); + process.exit(1); }); process.on("unhandledRejection", (reason) => { - logger.error({ reason }, "Unhandled rejection"); - // In production, don't crash - if (process.env.NODE_ENV !== "production") { - process.exit(1); - } + logger.fatal({ reason }, "Unhandled rejection — exiting to let process manager restart"); + process.exit(1); }); // Memory pressure warning diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 04b0c8b8..036b3a38 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -68,6 +68,8 @@ class InMemoryRateLimiter implements RateLimiter { } class RedisRateLimiter implements RateLimiter { + private readonly fallback = new InMemoryRateLimiter(); + constructor(private readonly redis: Redis) {} async check( @@ -101,8 +103,8 @@ class RedisRateLimiter implements RateLimiter { return { allowed: count <= limit, remaining, resetTime }; } catch (error) { - logger.error({ error, identifier }, "Redis rate limit check failed, allowing request"); - return { allowed: true, remaining: limit, resetTime: now + windowMs }; + logger.error({ error, identifier }, "Redis rate limit check failed, falling back to in-memory limiter"); + return this.fallback.check(identifier, limit, windowMs); } } diff --git a/src/lib/redis.ts b/src/lib/redis.ts index 8d70aae2..ad797809 100644 --- a/src/lib/redis.ts +++ b/src/lib/redis.ts @@ -151,6 +151,18 @@ export async function deleteSession(sessionId: string): Promise { } } +export async function closeRedis(): Promise { + if (client) { + try { + await client.quit(); + logger.info("Redis connection closed"); + } catch { + // Already disconnected + } + client = null; + } +} + // log Redis configuration on module load only when actually used if (!shouldSkipRedis()) { logger.info({ redisUrl: safeUrl(resolveRedisUrl()) }, "Redis configured"); diff --git a/src/lib/ssh.ts b/src/lib/ssh.ts index 1baac80b..ad303b5e 100644 --- a/src/lib/ssh.ts +++ b/src/lib/ssh.ts @@ -11,6 +11,9 @@ import { dirname, join } from "path"; import ssh2 from "ssh2"; const { Server } = ssh2; +const GIT_PROCESS_TIMEOUT_MS = + parseInt(process.env.GIT_PROCESS_TIMEOUT_SECS || "3600", 10) * 1000; + /** * Per-IP SSH authentication rate limiter. * Tracks failed attempts and blocks IPs that exceed the threshold. @@ -243,6 +246,14 @@ export function createSSHServer( }, }); + // Kill git process if it runs too long + const gitTimer = setTimeout(() => { + logger.warn({ fullRepoPath, operation }, "SSH git process timed out, killing"); + gitProcess.kill("SIGKILL"); + }, GIT_PROCESS_TIMEOUT_MS); + gitProcess.on("close", () => clearTimeout(gitTimer)); + gitProcess.on("error", () => clearTimeout(gitTimer)); + // Track refs for push hook let receivedRefs: string[] = []; diff --git a/src/lib/validation.ts b/src/lib/validation.ts index 46806c86..4c90c459 100644 --- a/src/lib/validation.ts +++ b/src/lib/validation.ts @@ -1,5 +1,128 @@ import { z } from "zod"; import { sanitizeHtml } from "./sanitize"; +import { lookup } from "dns"; +import { promisify } from "util"; + +const dnsLookup = promisify(lookup); + +/** + * Check if an IPv4 address falls within a CIDR range + */ +function isIPv4InCidr(ip: string, cidr: string): boolean { + const [range, bits] = cidr.split("/"); + const mask = ~((1 << (32 - parseInt(bits))) - 1); + const ipNum = ipToNumber(ip); + const rangeNum = ipToNumber(range); + if (ipNum === null || rangeNum === null) return false; + return (ipNum & mask) === (rangeNum & mask); +} + +/** + * Check if an IPv6 address falls within a CIDR range + */ +function isIPv6InCidr(ip: string, cidr: string): boolean { + const [range, bits] = cidr.split("/"); + const prefixLen = parseInt(bits); + const ipBuf = ipv6ToBuffer(ip); + const rangeBuf = ipv6ToBuffer(range); + if (!ipBuf || !rangeBuf) return false; + const fullBytes = Math.floor(prefixLen / 8); + const remainingBits = prefixLen % 8; + for (let i = 0; i < fullBytes; i++) { + if (ipBuf[i] !== rangeBuf[i]) return false; + } + if (remainingBits > 0) { + const mask = ~((1 << (8 - remainingBits)) - 1) & 0xff; + if ((ipBuf[fullBytes] & mask) !== (rangeBuf[fullBytes] & mask)) return false; + } + return true; +} + +function ipToNumber(ip: string): number | null { + const parts = ip.split("."); + if (parts.length !== 4) return null; + return parts.reduce((acc, part) => (acc << 8) + parseInt(part), 0) >>> 0; +} + +function ipv6ToBuffer(ip: string): Buffer | null { + try { + // Expand compressed IPv6 + const sections = ip.split(":"); + if (sections.length > 8) return null; + // Handle :: expansion + const fullSections = []; + let foundDoubleColon = false; + for (const s of sections) { + if (s === "") { + if (foundDoubleColon) return null; + foundDoubleColon = true; + const remaining = 8 - sections.length + 1; + for (let i = 0; i < remaining; i++) fullSections.push("0000"); + } else { + fullSections.push(s.padStart(4, "0")); + } + } + while (fullSections.length < 8) fullSections.push("0000"); + const hex = fullSections.join(""); + return Buffer.from(hex, "hex"); + } catch { + return null; + } +} + +/** + * Resolve hostname and check if the resolved IP is private/reserved + */ +async function isPrivateOrReservedIP(hostname: string): Promise { + // Always block these hostnames before DNS resolution + const alwaysBlockedHostnames = [ + "localhost", + "127.0.0.1", + "::1", + "metadata.google.internal", + ]; + if (alwaysBlockedHostnames.includes(hostname.toLowerCase())) return true; + + try { + const { address } = await dnsLookup(hostname); + const ip = address.toLowerCase(); + + // Block 0.0.0.0 + if (ip === "0.0.0.0") return true; + + // Block 169.254.0.0/16 (link-local) + if (ip.startsWith("169.254.")) return true; + + // Block 10.0.0.0/8 (RFC 1918) + if (ip.startsWith("10.")) return true; + + // Block 172.16.0.0/12 (RFC 1918) + if (isIPv4InCidr(ip, "172.16.0.0/12")) return true; + + // Block 192.168.0.0/16 (RFC 1918) + if (ip.startsWith("192.168.")) return true; + + // Block 127.0.0.0/8 (loopback) + if (ip.startsWith("127.")) return true; + + // IPv6 private/reserved ranges + if (ip.includes(":")) { + // fc00::/7 (unique local addresses) + if (isIPv6InCidr(ip, "fc00::/7")) return true; + // fe80::/10 (link-local) + if (isIPv6InCidr(ip, "fe80::/10")) return true; + // ::1 (loopback) + if (ip === "::1") return true; + // ::0 (unspecified) + if (ip === "::") return true; + } + } catch { + // DNS resolution failed — reject to be safe + return true; + } + + return false; +} /** * Validation Schemas for OpenCodeHub APIs @@ -92,21 +215,13 @@ export const WebhookConfigSchema = z.object({ .string() .url() .refine( - (url) => { + async (url) => { try { const parsed = new URL(url); if (!["http:", "https:"].includes(parsed.protocol)) return false; const h = parsed.hostname.toLowerCase(); - const blocked = [ - "localhost", - "127.0.0.1", - "::1", - "0.0.0.0", - "169.254.169.254", - "metadata.google.internal", - ]; - if (blocked.includes(h)) return false; - return true; + // Check if the resolved IP is private/reserved via DNS lookup + return !(await isPrivateOrReservedIP(h)); } catch { return false; } diff --git a/src/middleware.ts b/src/middleware.ts index 89301a89..21449cc1 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -13,6 +13,7 @@ import { newRequestId, withRequestContext, } from "./lib/request-context"; +import { randomBytes } from "crypto"; // Define tiers for different routes const apiLimiter = createRateLimitMiddleware("api"); @@ -157,11 +158,13 @@ async function onRequestInner( // Add Server-Timing header for observability response.headers.set("Server-Timing", `total;dur=${durationMs.toFixed(1)}`); - // Add Content-Security-Policy header + // Add Content-Security-Policy header with nonce-based script-src if (!response.headers.has("Content-Security-Policy")) { + const cspNonce = randomBytes(16).toString("base64"); + context.locals.cspNonce = cspNonce; response.headers.set( "Content-Security-Policy", - "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' blob:; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' data: https://fonts.gstatic.com; img-src 'self' data: https: blob:; connect-src 'self' https:;", + `default-src 'self'; script-src 'self' 'nonce-${cspNonce}' blob:; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' data: https://fonts.gstatic.com; img-src 'self' data: https: blob:; connect-src 'self' https:;`, ); } diff --git a/src/middleware/csrf.ts b/src/middleware/csrf.ts index 7984004f..406301d8 100644 --- a/src/middleware/csrf.ts +++ b/src/middleware/csrf.ts @@ -5,6 +5,7 @@ */ import { nanoid } from "nanoid"; +import { timingSafeEqual as nodeTimingSafeEqual } from "crypto"; const CSRF_TOKEN_LENGTH = 32; const CSRF_COOKIE_NAME = "csrf_token"; @@ -104,19 +105,17 @@ export async function validateCsrfToken(request: Request): Promise { } /** - * Timing-safe string comparison + * Timing-safe string comparison using Node.js crypto + * Pads both strings to a fixed length to prevent length leakage */ -function timingSafeEqual(a: string, b: string): boolean { - if (a.length !== b.length) { - return false; - } +const COMPARISON_FIXED_LENGTH = 64; - let mismatch = 0; - for (let i = 0; i < a.length; i++) { - mismatch |= a.charCodeAt(i) ^ b.charCodeAt(i); - } - - return mismatch === 0; +function timingSafeEqual(a: string, b: string): boolean { + const paddedA = a.padEnd(COMPARISON_FIXED_LENGTH, "\0"); + const paddedB = b.padEnd(COMPARISON_FIXED_LENGTH, "\0"); + const bufA = Buffer.from(paddedA, "utf8"); + const bufB = Buffer.from(paddedB, "utf8"); + return nodeTimingSafeEqual(bufA, bufB); } /** diff --git a/src/middleware/rate-limit.ts b/src/middleware/rate-limit.ts index b2c20b6b..5505689f 100644 --- a/src/middleware/rate-limit.ts +++ b/src/middleware/rate-limit.ts @@ -66,12 +66,13 @@ export function createRateLimitMiddleware( return null; } - // Skip rate limiting in development if configured - if ( - process.env.NODE_ENV === "development" && + // Never skip rate limiting in production, even if RATE_LIMIT_SKIP_DEV is set + if (process.env.NODE_ENV !== "development") { + // Fall through to rate limiting + } else if ( process.env.RATE_LIMIT_SKIP_DEV === "true" ) { - return null; // Continue to next handler + return null; // Continue to next handler (development only) } const identifier = getClientIdentifier(request, context); diff --git a/src/pages/api/admin/stats.ts b/src/pages/api/admin/stats.ts index e7e1fcfe..7a4c6c74 100644 --- a/src/pages/api/admin/stats.ts +++ b/src/pages/api/admin/stats.ts @@ -1,10 +1,11 @@ import { getDatabase, schema } from "@/db"; -import { count, desc, eq, gte } from "drizzle-orm"; +import { count, desc, eq, gte, sql } from "drizzle-orm"; import type { APIRoute } from "astro"; import os from "node:os"; import { withErrorHandler } from "@/lib/errors"; import { success } from "@/lib/api"; +import { logger } from "@/lib/logger"; import type { NodePgDatabase } from "drizzle-orm/node-postgres"; export const GET: APIRoute = withErrorHandler(async ({ locals }) => { @@ -44,35 +45,51 @@ export const GET: APIRoute = withErrorHandler(async ({ locals }) => { } }); - // 4. Code Stats (Real Aggregation) - const allCommits = await db.select({ stats: schema.commits.stats }).from(schema.commits); + // 4. Code Stats — aggregate in SQL to avoid loading all rows into memory let added = 0; let deleted = 0; - allCommits.forEach(c => { - try { - const s = typeof c.stats === 'string' ? JSON.parse(c.stats) : c.stats; - if (s) { - // ... - } - } catch (e) { } - }); + try { + const statsRows = await db.execute(sql` + SELECT + COALESCE(SUM((stats::json->>'additions')::int), 0)::int AS total_added, + COALESCE(SUM((stats::json->>'deletions')::int), 0)::int AS total_deleted + FROM commits + `); + const row = (statsRows as any)?.rows?.[0] || (Array.isArray(statsRows) ? statsRows[0] : null); + if (row) { + added = Number(row.total_added) || 0; + deleted = Number(row.total_deleted) || 0; + } + } catch (e) { + // Fallback: if JSON extraction fails, return zeros (non-critical) + logger.warn({ e }, "Failed to aggregate commit stats via SQL"); + } - // 5. Languages Stats (Real Aggregation) - const allRepos = await db.select({ languages: schema.repositories.languages }).from(schema.repositories); + // 5. Languages Stats — aggregate in SQL to avoid loading all repos into memory const langMap: Record = {}; let totalLangUsage = 0; - - allRepos.forEach(r => { - try { - const l = typeof r.languages === 'string' ? JSON.parse(r.languages) : r.languages; - if (l) { - Object.entries(l).forEach(([key, val]) => { - langMap[key] = (langMap[key] || 0) + (val as number); - totalLangUsage += (val as number); - }); - } - } catch (e) { } - }); + try { + const langRows = await db.execute(sql` + SELECT + key AS lang, + SUM(val::bigint)::bigint AS total_bytes + FROM repositories, + jsonb_each_text(COALESCE(languages::jsonb, '{}'::jsonb)) AS kv(key, val) + GROUP BY key + ORDER BY total_bytes DESC + LIMIT 20 + `); + const rows = (langRows as any)?.rows || (Array.isArray(langRows) ? langRows : []); + for (const r of rows) { + const langName = String(r.lang); + const bytes = Number(r.total_bytes) || 0; + langMap[langName] = bytes; + totalLangUsage += bytes; + } + } catch (e) { + // Fallback: if JSON extraction fails, return empty (non-critical) + logger.warn({ e }, "Failed to aggregate language stats via SQL"); + } const languages = Object.entries(langMap) .map(([name, count]) => ({ diff --git a/src/pages/api/health.ts b/src/pages/api/health.ts index 7fdcbda6..3bfe6c1a 100644 --- a/src/pages/api/health.ts +++ b/src/pages/api/health.ts @@ -33,8 +33,13 @@ export const GET: APIRoute = withErrorHandler(async () => { if (process.env.REDIS_URL) { const redisStart = Date.now(); try { - // Would ping Redis here - checks.redis = { status: "ok", latency: Date.now() - redisStart }; + const { redis } = await import("@/lib/redis"); + const pong = await redis.ping(); + if (pong === "PONG" || pong === "OK") { + checks.redis = { status: "ok", latency: Date.now() - redisStart }; + } else { + checks.redis = { status: "error", message: `Unexpected ping response: ${pong}` }; + } } catch (error) { checks.redis = { status: "error", @@ -108,6 +113,7 @@ export const GET: APIRoute = withErrorHandler(async () => { status: isHealthy ? 200 : 503, headers: { "Content-Type": "application/json", + "Cache-Control": "no-store, no-cache, must-revalidate", }, } ); diff --git a/src/pages/api/oauth/authorize.ts b/src/pages/api/oauth/authorize.ts index 2a58dada..dafd8632 100644 --- a/src/pages/api/oauth/authorize.ts +++ b/src/pages/api/oauth/authorize.ts @@ -8,6 +8,7 @@ import { logger } from "@/lib/logger"; import type { NodePgDatabase } from "drizzle-orm/node-postgres"; import { createHash, randomBytes } from "crypto"; import { generateId } from "@/lib/utils"; +import { escapeHtml } from "@/lib/sanitize"; const CODE_TTL_MINUTES = 10; @@ -44,10 +45,17 @@ export const GET: APIRoute = withErrorHandler(async ({ request, url }) => { const appScopes: string[] = app.scopes ? JSON.parse(app.scopes) : []; const scopeOk = requestedScopes.every((s: string) => appScopes.includes(s) || appScopes.includes("admin")); + const safeAppName = escapeHtml(app.name || ""); + const safeClientId = escapeHtml(clientId); + const safeRedirectUri = escapeHtml(redirectUri); + const safeState = escapeHtml(state); + const safeUsername = escapeHtml(user.username); + const safeScopes = requestedScopes.map((s: string) => escapeHtml(s)); + return new Response( ` -Authorize ${app.name} +Authorize ${safeAppName}
- - - - -

Authorize ${app.name}

-

${app.name} (by ${user.username}) is requesting access to your OpenCodeHub account.

+ + + + +

Authorize ${safeAppName}

+

${safeAppName} (by ${safeUsername}) is requesting access to your OpenCodeHub account.

This app will be able to:

-
${requestedScopes.map((s: string) => `${s}`).join("") || 'basic profile'}
-

Redirect URI: ${redirectUri}

+
${safeScopes.map((s: string) => `${s}`).join("") || 'basic profile'}
+

Redirect URI: ${safeRedirectUri}

diff --git a/src/pages/api/repos/[owner]/[repo]/git/push.ts b/src/pages/api/repos/[owner]/[repo]/git/push.ts index e434c94b..6982ac43 100644 --- a/src/pages/api/repos/[owner]/[repo]/git/push.ts +++ b/src/pages/api/repos/[owner]/[repo]/git/push.ts @@ -17,7 +17,7 @@ import { parseStoragePath, ensureRepoInitialized } from "@/lib/git-storage"; -import { spawn, execSync } from "child_process"; +import { spawn } from "child_process"; import { existsSync, writeFileSync, unlinkSync } from "fs"; import { join } from "path"; @@ -146,10 +146,21 @@ export const POST: APIRoute = withErrorHandler(async ({ request, params }) => { // Fetch from the bundle const refs: string[] = []; - // Get list of refs in bundle - const listOutput = execSync(`git bundle list-heads "${tempBundlePath}"`, { - cwd: repoPath, - encoding: "utf-8", + // Get list of refs in bundle (using spawn to avoid command injection) + const listOutput = await new Promise((resolve, reject) => { + const child = spawn("git", ["bundle", "list-heads", tempBundlePath], { + cwd: repoPath, + stdio: ["pipe", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (data) => { stdout += data.toString(); }); + child.stderr.on("data", (data) => { stderr += data.toString(); }); + child.on("close", (code) => { + if (code !== 0) reject(new Error(`Failed to list bundle refs: ${stderr}`)); + else resolve(stdout); + }); + child.on("error", reject); }); const bundleRefs = listOutput @@ -161,22 +172,36 @@ export const POST: APIRoute = withErrorHandler(async ({ request, params }) => { return { sha, ref }; }); - // Fetch each ref from bundle + // Fetch each ref from bundle (using spawn to avoid command injection) for (const { ref } of bundleRefs) { try { // Fetch the ref - execSync(`git fetch "${tempBundlePath}" "${ref}:${ref}"`, { - cwd: repoPath, - stdio: "pipe", + await new Promise((resolve, reject) => { + const child = spawn("git", ["fetch", tempBundlePath, `${ref}:${ref}`], { + cwd: repoPath, + stdio: "pipe", + }); + child.on("close", (code) => { + if (code !== 0) reject(new Error(`git fetch exited with code ${code}`)); + else resolve(); + }); + child.on("error", reject); }); refs.push(ref); } catch (fetchError) { // Try force update if regular fetch fails if (force) { try { - execSync(`git fetch "${tempBundlePath}" "+${ref}:${ref}"`, { - cwd: repoPath, - stdio: "pipe", + await new Promise((resolve, reject) => { + const child = spawn("git", ["fetch", tempBundlePath, `+${ref}:${ref}`], { + cwd: repoPath, + stdio: "pipe", + }); + child.on("close", (code) => { + if (code !== 0) reject(new Error(`git fetch exited with code ${code}`)); + else resolve(); + }); + child.on("error", reject); }); refs.push(`${ref} (force)`); } catch { diff --git a/src/pages/api/repos/[owner]/[repo]/ide/fs.ts b/src/pages/api/repos/[owner]/[repo]/ide/fs.ts index 7aa04a0e..e1a94e88 100644 --- a/src/pages/api/repos/[owner]/[repo]/ide/fs.ts +++ b/src/pages/api/repos/[owner]/[repo]/ide/fs.ts @@ -18,6 +18,11 @@ export const GET: APIRoute = async ({ request, params, url }) => { const branch = url.searchParams.get("branch") || "main"; const filePath = url.searchParams.get("path") || ""; + // Prevent path traversal + if (filePath.includes("..")) { + return badRequest("Invalid path: path traversal detected"); + } + const db = getDatabase(); const ownerUser = await db.query.users.findFirst({ where: eq(schema.users.username, owner!) }); if (!ownerUser) return notFound("Owner not found"); @@ -84,7 +89,11 @@ export const POST: APIRoute = async ({ request, params }) => { // Write files for (const [filePath, content] of Object.entries(files)) { - const fullPath = path.join(tempPath, filePath); + const fullPath = path.resolve(path.join(tempPath, filePath)); + if (!fullPath.startsWith(path.resolve(tempPath))) { + await fs.rm(tempPath, { recursive: true, force: true }); + return badRequest("Invalid file path: path traversal detected"); + } await fs.mkdir(path.dirname(fullPath), { recursive: true }); await fs.writeFile(fullPath, content); await workGit.add(filePath); diff --git a/src/pages/api/repos/[owner]/[repo]/pulls/[number]/requested-reviewers.ts b/src/pages/api/repos/[owner]/[repo]/pulls/[number]/requested-reviewers.ts index 6c44a44f..8706d80c 100644 --- a/src/pages/api/repos/[owner]/[repo]/pulls/[number]/requested-reviewers.ts +++ b/src/pages/api/repos/[owner]/[repo]/pulls/[number]/requested-reviewers.ts @@ -63,14 +63,12 @@ export const POST: APIRoute = async ({ request, params }) => { if (!pr) return notFound("Pull request not found"); // Verify users exist and exclude the author - const valid: string[] = []; - for (const id of userIds) { - const u = await db.query.users.findFirst({ - where: eq(schema.users.id, id), - columns: { id: true }, - }); - if (u && u.id !== pr.authorId) valid.push(id); - } + const uniqueUserIds = [...new Set(userIds)]; + const validUsers = await db.query.users.findMany({ + where: inArray(schema.users.id, uniqueUserIds), + columns: { id: true }, + }); + const valid = validUsers.map(u => u.id).filter(id => id !== pr.authorId); const existing = await db.query.pullRequestReviewers.findMany({ where: and( diff --git a/src/pages/api/repos/[owner]/[repo]/pulls/reviewer-routing.ts b/src/pages/api/repos/[owner]/[repo]/pulls/reviewer-routing.ts index ed426757..6f5d5171 100644 --- a/src/pages/api/repos/[owner]/[repo]/pulls/reviewer-routing.ts +++ b/src/pages/api/repos/[owner]/[repo]/pulls/reviewer-routing.ts @@ -276,6 +276,10 @@ export const POST: APIRoute = withErrorHandler( let assigneesAdded = 0; let assigneesSkipped = 0; + // Batch collect reviewer and assignee inserts + const reviewerInserts: { id: string; pullRequestId: string; userId: string; isRequired: boolean; requestedAt: Date }[] = []; + const assigneeInserts: { id: string; pullRequestId: string; userId: string; assignedAt: Date }[] = []; + for (const pr of pullRequests) { for (const reviewerId of parsed.data.reviewerIds) { if (reviewerId === pr.authorId) { @@ -289,7 +293,7 @@ export const POST: APIRoute = withErrorHandler( continue; } - await db.insert(schema.pullRequestReviewers).values({ + reviewerInserts.push({ id: generateId(), pullRequestId: pr.id, userId: reviewerId, @@ -307,7 +311,7 @@ export const POST: APIRoute = withErrorHandler( continue; } - await db.insert(schema.pullRequestAssignees).values({ + assigneeInserts.push({ id: generateId(), pullRequestId: pr.id, userId: assigneeId, @@ -318,6 +322,14 @@ export const POST: APIRoute = withErrorHandler( } } + // Batch inserts instead of N+1 individual inserts + if (reviewerInserts.length > 0) { + await db.insert(schema.pullRequestReviewers).values(reviewerInserts); + } + if (assigneeInserts.length > 0) { + await db.insert(schema.pullRequestAssignees).values(assigneeInserts); + } + return success({ routedPrCount: pullRequests.length, summary: { diff --git a/tests/unit/validation.test.ts b/tests/unit/validation.test.ts index abf8738d..93674dda 100644 --- a/tests/unit/validation.test.ts +++ b/tests/unit/validation.test.ts @@ -203,8 +203,8 @@ describe("BranchProtectionSchema", () => { }); describe("WebhookConfigSchema", () => { - it("accepts valid webhook config", () => { - const result = WebhookConfigSchema.safeParse({ + it("accepts valid webhook config", async () => { + const result = await WebhookConfigSchema.safeParseAsync({ url: "https://example.com/webhook", events: ["push", "pull_request"], secret: "mysecret", @@ -213,15 +213,15 @@ describe("WebhookConfigSchema", () => { expect(result.success).toBe(true); }); - it("rejects missing url", () => { - const result = WebhookConfigSchema.safeParse({ + it("rejects missing url", async () => { + const result = await WebhookConfigSchema.safeParseAsync({ events: ["push"], }); expect(result.success).toBe(false); }); - it("rejects private IP urls (SSRF protection)", () => { - const result = WebhookConfigSchema.safeParse({ + it("rejects private IP urls (SSRF protection)", async () => { + const result = await WebhookConfigSchema.safeParseAsync({ url: "http://127.0.0.1:8080/hook", events: ["push"], }); @@ -229,8 +229,8 @@ describe("WebhookConfigSchema", () => { expect(result.success).toBe(false); }); - it("rejects localhost urls", () => { - const result = WebhookConfigSchema.safeParse({ + it("rejects localhost urls", async () => { + const result = await WebhookConfigSchema.safeParseAsync({ url: "http://localhost:3000/hook", events: ["push"], });