From 85919da2c9105b4c4789835e5ebc98b8fb2736a7 Mon Sep 17 00:00:00 2001 From: hardcordev Date: Fri, 28 Aug 2026 06:59:42 -0700 Subject: [PATCH] security: hardening for api keys, encryption, csp, sql injection - rotate API keys with a grace period instead of immediate revocation, and surface grace-period/rotation status in usage tracking - add customer-managed KMS key and wire it into RDS, secrets manager, and backup vault encryption at rest - apply the existing CSP middleware/headers to the backend and frontend - parameterize the SQL helper and validate dynamic SQL identifiers used by the timescale repository --- .../migration.sql | 7 ++ backend/prisma/schema.prisma | 8 ++ backend/src/index.ts | 4 + backend/src/middleware/security.ts | 21 ++--- .../implementations/TimescaleRepository.ts | 27 +++++- backend/src/routes/api-keys.ts | 44 ++++++--- backend/src/services/keys/rotation.ts | 92 +++++++++++++++++++ frontend/next.config.ts | 33 +++++++ infra/main.tf | 24 ++++- 9 files changed, 233 insertions(+), 27 deletions(-) create mode 100644 backend/prisma/migrations/20260828000000_api_key_rotation_grace_period/migration.sql create mode 100644 backend/src/services/keys/rotation.ts diff --git a/backend/prisma/migrations/20260828000000_api_key_rotation_grace_period/migration.sql b/backend/prisma/migrations/20260828000000_api_key_rotation_grace_period/migration.sql new file mode 100644 index 00000000..867f9a34 --- /dev/null +++ b/backend/prisma/migrations/20260828000000_api_key_rotation_grace_period/migration.sql @@ -0,0 +1,7 @@ +-- Issue #756: API key rotation with grace period and usage tracking + +-- AlterTable +ALTER TABLE "api_keys" ADD COLUMN "rotated_at" TIMESTAMP(3); +ALTER TABLE "api_keys" ADD COLUMN "grace_period_ends_at" TIMESTAMP(3); +ALTER TABLE "api_keys" ADD COLUMN "predecessor_key_id" TEXT; +ALTER TABLE "api_keys" ADD COLUMN "successor_key_id" TEXT; diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index ed9ea66c..7b241454 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -1578,6 +1578,14 @@ model ApiKey { expiresAt DateTime? @map("expires_at") revokedAt DateTime? @map("revoked_at") + // Issue #756: rotation with grace period. When a key is rotated, the + // predecessor stays active until gracePeriodEndsAt so in-flight clients + // have time to switch to the new key before the old one stops working. + rotatedAt DateTime? @map("rotated_at") + gracePeriodEndsAt DateTime? @map("grace_period_ends_at") + predecessorKeyId String? @map("predecessor_key_id") + successorKeyId String? @map("successor_key_id") + usage ApiKeyUsage[] quota ApiKeyQuota? diff --git a/backend/src/index.ts b/backend/src/index.ts index f3138ca6..d3576b79 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -172,6 +172,10 @@ app.use( allowedHeaders: ['Content-Type', 'Authorization', 'X-Trace-Id', REQUEST_ID_HEADER], }) ); + +// Content Security Policy & related security headers (XSS prevention) +app.use(contentSecurityPolicy()); + app.use(express.json()); app.use( diff --git a/backend/src/middleware/security.ts b/backend/src/middleware/security.ts index 74c017e6..f4c44901 100644 --- a/backend/src/middleware/security.ts +++ b/backend/src/middleware/security.ts @@ -197,25 +197,24 @@ export class SQLInjectionPrevention { } /** - * Create safe SQL query with parameterization + * Create a safe, parameterized SQL query. + * + * Values are NEVER interpolated into the query string — `?` placeholders + * are rewritten to positional bind parameters (`$1`, `$2`, ...) and the + * caller must execute the returned `query`/`safeParams` pair through the + * database driver's parameter-binding API (e.g. `pool.query(query, safeParams)`). */ public static createSafeQuery(template: string, params: any[]): { query: string; safeParams: any[] } { if (!this.validateQueryParams(params)) { throw new Error('Invalid SQL parameters detected'); } - // Simple parameterization (in production, use proper ORM) - let query = template; let paramIndex = 0; + const query = template.replace(/\?/g, () => `$${++paramIndex}`); - // Replace placeholders with safe parameters - query = query.replace(/\?/g, () => { - if (paramIndex < params.length) { - const param = params[paramIndex++]; - return typeof param === 'string' ? `'${param.replace(/'/g, "''")}'` : String(param); - } - return '?'; - }); + if (paramIndex !== params.length) { + throw new Error('Parameter count does not match placeholder count'); + } return { query, safeParams: params }; } diff --git a/backend/src/repositories/implementations/TimescaleRepository.ts b/backend/src/repositories/implementations/TimescaleRepository.ts index 2ece48c5..de6cb775 100644 --- a/backend/src/repositories/implementations/TimescaleRepository.ts +++ b/backend/src/repositories/implementations/TimescaleRepository.ts @@ -12,11 +12,29 @@ interface PgPool { query(sql: string, params?: unknown[]): Promise<{ rows: R[] }>; } +/** + * SQL identifiers (table/column names) can't be bound as query parameters, + * so they must be validated against a strict allowlist pattern before being + * interpolated into a query string. Only alphanumerics and underscores are + * permitted, which rules out any injection via quotes, semicolons, or SQL + * keywords riding along in a table/column name. + */ +const IDENTIFIER_PATTERN = /^[a-zA-Z_][a-zA-Z0-9_]*$/; + +function assertSafeIdentifier(identifier: string, kind: string): string { + if (!IDENTIFIER_PATTERN.test(identifier)) { + throw new Error(`Invalid ${kind} identifier: ${identifier}`); + } + return identifier; +} + export class TimescaleRepository implements Repository { constructor( private readonly pool: PgPool, private readonly table: string, - ) {} + ) { + assertSafeIdentifier(table, 'table'); + } async findById(id: string): Promise { const { rows } = await this.pool.query(`SELECT * FROM ${this.table} WHERE id = $1 LIMIT 1`, [id]); @@ -30,6 +48,7 @@ export class TimescaleRepository implements Repository if (options.where) { for (const [key, value] of Object.entries(options.where)) { + assertSafeIdentifier(key, 'column'); conditions.push(`${key} = $${idx++}`); params.push(value); } @@ -37,7 +56,9 @@ export class TimescaleRepository implements Repository const where = conditions.length ? `WHERE ${conditions.join(' AND ')}` : ''; const orderBy = options.orderBy - ? `ORDER BY ${String(options.orderBy.field)} ${options.orderBy.direction}` + ? `ORDER BY ${assertSafeIdentifier(String(options.orderBy.field), 'column')} ${ + options.orderBy.direction === 'asc' ? 'ASC' : 'DESC' + }` : 'ORDER BY time DESC'; const limit = options.limit ? `LIMIT $${idx++}` : ''; const offset = options.offset ? `OFFSET $${idx++}` : ''; @@ -54,6 +75,7 @@ export class TimescaleRepository implements Repository const id = `ts_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; const record = { ...data, id } as T; const keys = Object.keys(record); + keys.forEach((key) => assertSafeIdentifier(key, 'column')); const placeholders = keys.map((_, i) => `$${i + 1}`).join(', '); const values = Object.values(record); const { rows } = await this.pool.query( @@ -66,6 +88,7 @@ export class TimescaleRepository implements Repository async update(id: string, data: Partial): Promise { const entries = Object.entries(data); if (entries.length === 0) return this.findById(id); + entries.forEach(([k]) => assertSafeIdentifier(k, 'column')); const sets = entries.map(([k], i) => `${k} = $${i + 1}`).join(', '); const values = [...entries.map(([, v]) => v), id]; const { rows } = await this.pool.query( diff --git a/backend/src/routes/api-keys.ts b/backend/src/routes/api-keys.ts index ff8bb95c..e2bb8596 100644 --- a/backend/src/routes/api-keys.ts +++ b/backend/src/routes/api-keys.ts @@ -3,6 +3,7 @@ import { prisma } from '../lib/prisma.js'; import { asyncHandler } from '../middleware/errorHandler.js'; import { AppError } from '../middleware/errorHandler.js'; import { quotaManagerService } from '../services/keys/quota-manager.js'; +import { rotateApiKeyWithGracePeriod, settleGracePeriod } from '../services/keys/rotation.js'; import { randomBytes, createHash } from 'node:crypto'; export const apiKeysRouter = Router(); @@ -37,7 +38,8 @@ apiKeysRouter.get('/', asyncHandler(async (req, res) => { quota: true, }, }); - res.json({ keys }); + const settled = await Promise.all(keys.map((key) => settleGracePeriod(key))); + res.json({ keys: settled }); })); apiKeysRouter.get('/:keyId', asyncHandler(async (req, res) => { @@ -47,7 +49,8 @@ apiKeysRouter.get('/:keyId', asyncHandler(async (req, res) => { include: { quota: true }, }); if (!key || key.tenantId !== tenantId) throw new AppError(404, 'API key not found', 'KEY_NOT_FOUND'); - res.json(key); + const settled = await settleGracePeriod(key); + res.json(settled); })); apiKeysRouter.delete('/:keyId', asyncHandler(async (req, res) => { @@ -62,8 +65,20 @@ apiKeysRouter.get('/:keyId/usage', asyncHandler(async (req, res) => { const tenantId = resolveTenant(req); const key = await prisma.apiKey.findUnique({ where: { keyId: req.params.keyId } }); if (!key || key.tenantId !== tenantId) throw new AppError(404, 'API key not found', 'KEY_NOT_FOUND'); + const settled = await settleGracePeriod(key); const summary = await quotaManagerService.getUsageSummary(req.params.keyId); - res.json(summary); + res.json({ + ...summary, + rotation: { + rotatedAt: settled.rotatedAt, + gracePeriodEndsAt: settled.gracePeriodEndsAt, + predecessorKeyId: settled.predecessorKeyId, + successorKeyId: settled.successorKeyId, + inGracePeriod: Boolean( + settled.isActive && settled.gracePeriodEndsAt && settled.gracePeriodEndsAt.getTime() > Date.now(), + ), + }, + }); })); apiKeysRouter.get('/:keyId/usage/daily', asyncHandler(async (req, res) => { @@ -93,19 +108,24 @@ apiKeysRouter.post('/:keyId/rotate', asyncHandler(async (req, res) => { const tenantId = resolveTenant(req); const key = await prisma.apiKey.findUnique({ where: { keyId: req.params.keyId } }); if (!key || key.tenantId !== tenantId) throw new AppError(404, 'API key not found', 'KEY_NOT_FOUND'); + if (!key.isActive) throw new AppError(409, 'API key is not active', 'KEY_INACTIVE'); - await prisma.apiKey.update({ where: { keyId: req.params.keyId }, data: { isActive: false, revokedAt: new Date() } }); + const { gracePeriodHours } = req.body as { gracePeriodHours?: number }; + const { previousKey, newKey, gracePeriodEndsAt } = await rotateApiKeyWithGracePeriod({ + tenantId, + keyId: key.keyId, + gracePeriodHours, + }); - const newKeyId = `ak_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`; - const newKey = await prisma.apiKey.create({ - data: { - tenantId, - keyId: newKeyId, - description: key.description ? `${key.description} (rotated)` : 'Rotated key', - expiresAt: key.expiresAt, + res.status(201).json({ + keyId: newKey.keyId, + description: newKey.description, + rotatedFrom: previousKey.keyId, + gracePeriod: { + predecessorKeyId: previousKey.keyId, + gracePeriodEndsAt, }, }); - res.status(201).json({ keyId: newKey.keyId, description: newKey.description, rotatedFrom: key.keyId }); })); apiKeysRouter.post('/:keyId/revoke', asyncHandler(async (req, res) => { diff --git a/backend/src/services/keys/rotation.ts b/backend/src/services/keys/rotation.ts new file mode 100644 index 00000000..4376bc9b --- /dev/null +++ b/backend/src/services/keys/rotation.ts @@ -0,0 +1,92 @@ +// Issue #756: API key rotation with grace period and usage tracking +// +// Rotating a key used to revoke the old one immediately, which breaks any +// in-flight client that hasn't picked up the new key yet. Instead, the +// predecessor key is kept active for a configurable grace period so both +// keys work during the handover window; usage against the predecessor is +// still recorded (see api-usage-tracker.ts / ApiKeyUsage) so the grace +// period's traffic is visible before the old key is retired. + +import { prisma } from '../../lib/prisma.js'; + +const DEFAULT_GRACE_PERIOD_HOURS = 24; +const MAX_GRACE_PERIOD_HOURS = 24 * 30; // 30 days + +export function resolveGracePeriodHours(requested?: number): number { + const envDefault = Number(process.env.API_KEY_ROTATION_GRACE_PERIOD_HOURS); + const fallback = Number.isFinite(envDefault) && envDefault > 0 ? envDefault : DEFAULT_GRACE_PERIOD_HOURS; + + if (requested === undefined || requested === null) return fallback; + if (!Number.isFinite(requested) || requested < 0) return fallback; + return Math.min(requested, MAX_GRACE_PERIOD_HOURS); +} + +/** + * Deactivate this key's grace period if it has expired. Lazy expiry avoids + * needing a scheduler: any read path that touches the key settles its state. + */ +export async function settleGracePeriod(key: T): Promise { + if (!key.isActive || !key.gracePeriodEndsAt || key.gracePeriodEndsAt.getTime() > Date.now()) { + return key; + } + + const updated = await prisma.apiKey.update({ + where: { keyId: key.keyId }, + data: { isActive: false, revokedAt: key.revokedAt ?? new Date() }, + }); + + return { ...key, isActive: updated.isActive, revokedAt: updated.revokedAt } as T; +} + +/** + * Rotate an API key: the predecessor stays active (still authenticates + * requests) until `gracePeriodEndsAt`, while a new key is issued to replace + * it. Both keys' usage is tracked independently via ApiKeyUsage. + */ +export async function rotateApiKeyWithGracePeriod(opts: { + tenantId: string; + keyId: string; + gracePeriodHours?: number; +}) { + const gracePeriodHours = resolveGracePeriodHours(opts.gracePeriodHours); + const now = new Date(); + const gracePeriodEndsAt = new Date(now.getTime() + gracePeriodHours * 60 * 60 * 1000); + + const newKeyId = `ak_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`; + + const [previousKey, newKey] = await prisma.$transaction(async (tx) => { + const existing = await tx.apiKey.findUnique({ where: { keyId: opts.keyId } }); + if (!existing || existing.tenantId !== opts.tenantId) { + throw new Error('API key not found'); + } + + const created = await tx.apiKey.create({ + data: { + tenantId: opts.tenantId, + keyId: newKeyId, + description: existing.description ? `${existing.description} (rotated)` : 'Rotated key', + expiresAt: existing.expiresAt, + predecessorKeyId: existing.keyId, + }, + }); + + const previous = await tx.apiKey.update({ + where: { keyId: existing.keyId }, + data: { + rotatedAt: now, + gracePeriodEndsAt, + successorKeyId: created.keyId, + // isActive stays true — the predecessor remains usable through the grace window. + }, + }); + + return [previous, created]; + }); + + return { previousKey, newKey, gracePeriodEndsAt }; +} diff --git a/frontend/next.config.ts b/frontend/next.config.ts index 94a09a15..f586161d 100644 --- a/frontend/next.config.ts +++ b/frontend/next.config.ts @@ -225,6 +225,39 @@ const nextConfig: NextConfig = { key: "Critical-CH", value: "sec-ch-prefers-color-scheme, sec-ch-viewport-width", }, + { + key: "Content-Security-Policy", + value: [ + "default-src 'self'", + "script-src 'self' 'unsafe-inline' 'unsafe-eval' https://vercel.live", + "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com", + "font-src 'self' https://fonts.gstatic.com", + "img-src 'self' data: https: blob:", + "connect-src 'self' https://api.stellar.org https://horizon-testnet.stellar.org", + "frame-src 'none'", + "object-src 'none'", + "base-uri 'self'", + "form-action 'self'", + "frame-ancestors 'none'", + "upgrade-insecure-requests", + ].join("; "), + }, + { + key: "X-Content-Type-Options", + value: "nosniff", + }, + { + key: "X-Frame-Options", + value: "DENY", + }, + { + key: "Referrer-Policy", + value: "strict-origin-when-cross-origin", + }, + { + key: "Permissions-Policy", + value: "geolocation=(), microphone=(), camera=()", + }, ], }, { diff --git a/infra/main.tf b/infra/main.tf index 3f9bd362..b6f8c383 100644 --- a/infra/main.tf +++ b/infra/main.tf @@ -53,6 +53,21 @@ module "vpc" { create_database_internet_gateway_route = false } +# ------------------------------------------------------------------------------ +# ENCRYPTION AT REST (customer-managed KMS key for sensitive data fields) +# ------------------------------------------------------------------------------ + +resource "aws_kms_key" "data_at_rest" { + description = "Customer-managed key for agenticpay-${var.environment} encryption at rest (RDS, Secrets Manager, backups)" + deletion_window_in_days = 30 + enable_key_rotation = true +} + +resource "aws_kms_alias" "data_at_rest" { + name = "alias/agenticpay-${var.environment}-data-at-rest" + target_key_id = aws_kms_key.data_at_rest.key_id +} + # ------------------------------------------------------------------------------ # DATABASE RESOURCES (PostgreSQL + PgBouncer via RDS Proxy) # ------------------------------------------------------------------------------ @@ -101,6 +116,7 @@ resource "aws_db_instance" "postgres" { max_allocated_storage = var.db_max_allocated_storage storage_type = "gp3" storage_encrypted = true + kms_key_id = aws_kms_key.data_at_rest.arn backup_retention_period = var.environment == "prod" ? 30 : 7 backup_window = "03:00-04:00" @@ -114,6 +130,7 @@ resource "aws_db_instance" "postgres" { performance_insights_enabled = var.environment == "prod" performance_insights_retention_period = var.environment == "prod" ? 7 : 0 + performance_insights_kms_key_id = var.environment == "prod" ? aws_kms_key.data_at_rest.arn : null enabled_cloudwatch_logs_exports = ["postgresql"] @@ -195,7 +212,8 @@ resource "aws_db_proxy_target" "main" { # Secrets Manager for database credentials resource "aws_secretsmanager_secret" "db_credentials" { - name = "agenticpay-${var.environment}-db-credentials" + name = "agenticpay-${var.environment}-db-credentials" + kms_key_id = aws_kms_key.data_at_rest.arn } # Secrets Manager for application-level secrets (Stripe, OpenAI, VAPID keys, etc). @@ -204,7 +222,8 @@ resource "aws_secretsmanager_secret" "db_credentials" { resource "aws_secretsmanager_secret" "app_secrets" { count = var.environment == "dev" ? 0 : 1 - name = "agenticpay-${var.environment}-app-secrets" + name = "agenticpay-${var.environment}-app-secrets" + kms_key_id = aws_kms_key.data_at_rest.arn } resource "aws_iam_policy" "app_secrets_read" { @@ -276,6 +295,7 @@ resource "aws_iam_role_policy" "rds_proxy_secrets" { resource "aws_backup_vault" "db_backup_vault" { name = "agenticpay-${var.environment}-db-backup-vault" + kms_key_arn = aws_kms_key.data_at_rest.arn } resource "aws_backup_plan" "db_backup_plan" {