Skip to content

Commit 85919da

Browse files
committed
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
1 parent b879057 commit 85919da

9 files changed

Lines changed: 233 additions & 27 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
-- Issue #756: API key rotation with grace period and usage tracking
2+
3+
-- AlterTable
4+
ALTER TABLE "api_keys" ADD COLUMN "rotated_at" TIMESTAMP(3);
5+
ALTER TABLE "api_keys" ADD COLUMN "grace_period_ends_at" TIMESTAMP(3);
6+
ALTER TABLE "api_keys" ADD COLUMN "predecessor_key_id" TEXT;
7+
ALTER TABLE "api_keys" ADD COLUMN "successor_key_id" TEXT;

backend/prisma/schema.prisma

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,6 +1578,14 @@ model ApiKey {
15781578
expiresAt DateTime? @map("expires_at")
15791579
revokedAt DateTime? @map("revoked_at")
15801580
1581+
// Issue #756: rotation with grace period. When a key is rotated, the
1582+
// predecessor stays active until gracePeriodEndsAt so in-flight clients
1583+
// have time to switch to the new key before the old one stops working.
1584+
rotatedAt DateTime? @map("rotated_at")
1585+
gracePeriodEndsAt DateTime? @map("grace_period_ends_at")
1586+
predecessorKeyId String? @map("predecessor_key_id")
1587+
successorKeyId String? @map("successor_key_id")
1588+
15811589
usage ApiKeyUsage[]
15821590
quota ApiKeyQuota?
15831591

backend/src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ app.use(
172172
allowedHeaders: ['Content-Type', 'Authorization', 'X-Trace-Id', REQUEST_ID_HEADER],
173173
})
174174
);
175+
176+
// Content Security Policy & related security headers (XSS prevention)
177+
app.use(contentSecurityPolicy());
178+
175179
app.use(express.json());
176180

177181
app.use(

backend/src/middleware/security.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -197,25 +197,24 @@ export class SQLInjectionPrevention {
197197
}
198198

199199
/**
200-
* Create safe SQL query with parameterization
200+
* Create a safe, parameterized SQL query.
201+
*
202+
* Values are NEVER interpolated into the query string — `?` placeholders
203+
* are rewritten to positional bind parameters (`$1`, `$2`, ...) and the
204+
* caller must execute the returned `query`/`safeParams` pair through the
205+
* database driver's parameter-binding API (e.g. `pool.query(query, safeParams)`).
201206
*/
202207
public static createSafeQuery(template: string, params: any[]): { query: string; safeParams: any[] } {
203208
if (!this.validateQueryParams(params)) {
204209
throw new Error('Invalid SQL parameters detected');
205210
}
206211

207-
// Simple parameterization (in production, use proper ORM)
208-
let query = template;
209212
let paramIndex = 0;
213+
const query = template.replace(/\?/g, () => `$${++paramIndex}`);
210214

211-
// Replace placeholders with safe parameters
212-
query = query.replace(/\?/g, () => {
213-
if (paramIndex < params.length) {
214-
const param = params[paramIndex++];
215-
return typeof param === 'string' ? `'${param.replace(/'/g, "''")}'` : String(param);
216-
}
217-
return '?';
218-
});
215+
if (paramIndex !== params.length) {
216+
throw new Error('Parameter count does not match placeholder count');
217+
}
219218

220219
return { query, safeParams: params };
221220
}

backend/src/repositories/implementations/TimescaleRepository.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,29 @@ interface PgPool {
1212
query<R>(sql: string, params?: unknown[]): Promise<{ rows: R[] }>;
1313
}
1414

15+
/**
16+
* SQL identifiers (table/column names) can't be bound as query parameters,
17+
* so they must be validated against a strict allowlist pattern before being
18+
* interpolated into a query string. Only alphanumerics and underscores are
19+
* permitted, which rules out any injection via quotes, semicolons, or SQL
20+
* keywords riding along in a table/column name.
21+
*/
22+
const IDENTIFIER_PATTERN = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
23+
24+
function assertSafeIdentifier(identifier: string, kind: string): string {
25+
if (!IDENTIFIER_PATTERN.test(identifier)) {
26+
throw new Error(`Invalid ${kind} identifier: ${identifier}`);
27+
}
28+
return identifier;
29+
}
30+
1531
export class TimescaleRepository<T extends { id: string }> implements Repository<T> {
1632
constructor(
1733
private readonly pool: PgPool,
1834
private readonly table: string,
19-
) {}
35+
) {
36+
assertSafeIdentifier(table, 'table');
37+
}
2038

2139
async findById(id: string): Promise<T | null> {
2240
const { rows } = await this.pool.query<T>(`SELECT * FROM ${this.table} WHERE id = $1 LIMIT 1`, [id]);
@@ -30,14 +48,17 @@ export class TimescaleRepository<T extends { id: string }> implements Repository
3048

3149
if (options.where) {
3250
for (const [key, value] of Object.entries(options.where)) {
51+
assertSafeIdentifier(key, 'column');
3352
conditions.push(`${key} = $${idx++}`);
3453
params.push(value);
3554
}
3655
}
3756

3857
const where = conditions.length ? `WHERE ${conditions.join(' AND ')}` : '';
3958
const orderBy = options.orderBy
40-
? `ORDER BY ${String(options.orderBy.field)} ${options.orderBy.direction}`
59+
? `ORDER BY ${assertSafeIdentifier(String(options.orderBy.field), 'column')} ${
60+
options.orderBy.direction === 'asc' ? 'ASC' : 'DESC'
61+
}`
4162
: 'ORDER BY time DESC';
4263
const limit = options.limit ? `LIMIT $${idx++}` : '';
4364
const offset = options.offset ? `OFFSET $${idx++}` : '';
@@ -54,6 +75,7 @@ export class TimescaleRepository<T extends { id: string }> implements Repository
5475
const id = `ts_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`;
5576
const record = { ...data, id } as T;
5677
const keys = Object.keys(record);
78+
keys.forEach((key) => assertSafeIdentifier(key, 'column'));
5779
const placeholders = keys.map((_, i) => `$${i + 1}`).join(', ');
5880
const values = Object.values(record);
5981
const { rows } = await this.pool.query<T>(
@@ -66,6 +88,7 @@ export class TimescaleRepository<T extends { id: string }> implements Repository
6688
async update(id: string, data: Partial<T>): Promise<T | null> {
6789
const entries = Object.entries(data);
6890
if (entries.length === 0) return this.findById(id);
91+
entries.forEach(([k]) => assertSafeIdentifier(k, 'column'));
6992
const sets = entries.map(([k], i) => `${k} = $${i + 1}`).join(', ');
7093
const values = [...entries.map(([, v]) => v), id];
7194
const { rows } = await this.pool.query<T>(

backend/src/routes/api-keys.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { prisma } from '../lib/prisma.js';
33
import { asyncHandler } from '../middleware/errorHandler.js';
44
import { AppError } from '../middleware/errorHandler.js';
55
import { quotaManagerService } from '../services/keys/quota-manager.js';
6+
import { rotateApiKeyWithGracePeriod, settleGracePeriod } from '../services/keys/rotation.js';
67
import { randomBytes, createHash } from 'node:crypto';
78

89
export const apiKeysRouter = Router();
@@ -37,7 +38,8 @@ apiKeysRouter.get('/', asyncHandler(async (req, res) => {
3738
quota: true,
3839
},
3940
});
40-
res.json({ keys });
41+
const settled = await Promise.all(keys.map((key) => settleGracePeriod(key)));
42+
res.json({ keys: settled });
4143
}));
4244

4345
apiKeysRouter.get('/:keyId', asyncHandler(async (req, res) => {
@@ -47,7 +49,8 @@ apiKeysRouter.get('/:keyId', asyncHandler(async (req, res) => {
4749
include: { quota: true },
4850
});
4951
if (!key || key.tenantId !== tenantId) throw new AppError(404, 'API key not found', 'KEY_NOT_FOUND');
50-
res.json(key);
52+
const settled = await settleGracePeriod(key);
53+
res.json(settled);
5154
}));
5255

5356
apiKeysRouter.delete('/:keyId', asyncHandler(async (req, res) => {
@@ -62,8 +65,20 @@ apiKeysRouter.get('/:keyId/usage', asyncHandler(async (req, res) => {
6265
const tenantId = resolveTenant(req);
6366
const key = await prisma.apiKey.findUnique({ where: { keyId: req.params.keyId } });
6467
if (!key || key.tenantId !== tenantId) throw new AppError(404, 'API key not found', 'KEY_NOT_FOUND');
68+
const settled = await settleGracePeriod(key);
6569
const summary = await quotaManagerService.getUsageSummary(req.params.keyId);
66-
res.json(summary);
70+
res.json({
71+
...summary,
72+
rotation: {
73+
rotatedAt: settled.rotatedAt,
74+
gracePeriodEndsAt: settled.gracePeriodEndsAt,
75+
predecessorKeyId: settled.predecessorKeyId,
76+
successorKeyId: settled.successorKeyId,
77+
inGracePeriod: Boolean(
78+
settled.isActive && settled.gracePeriodEndsAt && settled.gracePeriodEndsAt.getTime() > Date.now(),
79+
),
80+
},
81+
});
6782
}));
6883

6984
apiKeysRouter.get('/:keyId/usage/daily', asyncHandler(async (req, res) => {
@@ -93,19 +108,24 @@ apiKeysRouter.post('/:keyId/rotate', asyncHandler(async (req, res) => {
93108
const tenantId = resolveTenant(req);
94109
const key = await prisma.apiKey.findUnique({ where: { keyId: req.params.keyId } });
95110
if (!key || key.tenantId !== tenantId) throw new AppError(404, 'API key not found', 'KEY_NOT_FOUND');
111+
if (!key.isActive) throw new AppError(409, 'API key is not active', 'KEY_INACTIVE');
96112

97-
await prisma.apiKey.update({ where: { keyId: req.params.keyId }, data: { isActive: false, revokedAt: new Date() } });
113+
const { gracePeriodHours } = req.body as { gracePeriodHours?: number };
114+
const { previousKey, newKey, gracePeriodEndsAt } = await rotateApiKeyWithGracePeriod({
115+
tenantId,
116+
keyId: key.keyId,
117+
gracePeriodHours,
118+
});
98119

99-
const newKeyId = `ak_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`;
100-
const newKey = await prisma.apiKey.create({
101-
data: {
102-
tenantId,
103-
keyId: newKeyId,
104-
description: key.description ? `${key.description} (rotated)` : 'Rotated key',
105-
expiresAt: key.expiresAt,
120+
res.status(201).json({
121+
keyId: newKey.keyId,
122+
description: newKey.description,
123+
rotatedFrom: previousKey.keyId,
124+
gracePeriod: {
125+
predecessorKeyId: previousKey.keyId,
126+
gracePeriodEndsAt,
106127
},
107128
});
108-
res.status(201).json({ keyId: newKey.keyId, description: newKey.description, rotatedFrom: key.keyId });
109129
}));
110130

111131
apiKeysRouter.post('/:keyId/revoke', asyncHandler(async (req, res) => {
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Issue #756: API key rotation with grace period and usage tracking
2+
//
3+
// Rotating a key used to revoke the old one immediately, which breaks any
4+
// in-flight client that hasn't picked up the new key yet. Instead, the
5+
// predecessor key is kept active for a configurable grace period so both
6+
// keys work during the handover window; usage against the predecessor is
7+
// still recorded (see api-usage-tracker.ts / ApiKeyUsage) so the grace
8+
// period's traffic is visible before the old key is retired.
9+
10+
import { prisma } from '../../lib/prisma.js';
11+
12+
const DEFAULT_GRACE_PERIOD_HOURS = 24;
13+
const MAX_GRACE_PERIOD_HOURS = 24 * 30; // 30 days
14+
15+
export function resolveGracePeriodHours(requested?: number): number {
16+
const envDefault = Number(process.env.API_KEY_ROTATION_GRACE_PERIOD_HOURS);
17+
const fallback = Number.isFinite(envDefault) && envDefault > 0 ? envDefault : DEFAULT_GRACE_PERIOD_HOURS;
18+
19+
if (requested === undefined || requested === null) return fallback;
20+
if (!Number.isFinite(requested) || requested < 0) return fallback;
21+
return Math.min(requested, MAX_GRACE_PERIOD_HOURS);
22+
}
23+
24+
/**
25+
* Deactivate this key's grace period if it has expired. Lazy expiry avoids
26+
* needing a scheduler: any read path that touches the key settles its state.
27+
*/
28+
export async function settleGracePeriod<T extends {
29+
keyId: string;
30+
isActive: boolean;
31+
gracePeriodEndsAt: Date | null;
32+
revokedAt: Date | null;
33+
}>(key: T): Promise<T> {
34+
if (!key.isActive || !key.gracePeriodEndsAt || key.gracePeriodEndsAt.getTime() > Date.now()) {
35+
return key;
36+
}
37+
38+
const updated = await prisma.apiKey.update({
39+
where: { keyId: key.keyId },
40+
data: { isActive: false, revokedAt: key.revokedAt ?? new Date() },
41+
});
42+
43+
return { ...key, isActive: updated.isActive, revokedAt: updated.revokedAt } as T;
44+
}
45+
46+
/**
47+
* Rotate an API key: the predecessor stays active (still authenticates
48+
* requests) until `gracePeriodEndsAt`, while a new key is issued to replace
49+
* it. Both keys' usage is tracked independently via ApiKeyUsage.
50+
*/
51+
export async function rotateApiKeyWithGracePeriod(opts: {
52+
tenantId: string;
53+
keyId: string;
54+
gracePeriodHours?: number;
55+
}) {
56+
const gracePeriodHours = resolveGracePeriodHours(opts.gracePeriodHours);
57+
const now = new Date();
58+
const gracePeriodEndsAt = new Date(now.getTime() + gracePeriodHours * 60 * 60 * 1000);
59+
60+
const newKeyId = `ak_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`;
61+
62+
const [previousKey, newKey] = await prisma.$transaction(async (tx) => {
63+
const existing = await tx.apiKey.findUnique({ where: { keyId: opts.keyId } });
64+
if (!existing || existing.tenantId !== opts.tenantId) {
65+
throw new Error('API key not found');
66+
}
67+
68+
const created = await tx.apiKey.create({
69+
data: {
70+
tenantId: opts.tenantId,
71+
keyId: newKeyId,
72+
description: existing.description ? `${existing.description} (rotated)` : 'Rotated key',
73+
expiresAt: existing.expiresAt,
74+
predecessorKeyId: existing.keyId,
75+
},
76+
});
77+
78+
const previous = await tx.apiKey.update({
79+
where: { keyId: existing.keyId },
80+
data: {
81+
rotatedAt: now,
82+
gracePeriodEndsAt,
83+
successorKeyId: created.keyId,
84+
// isActive stays true — the predecessor remains usable through the grace window.
85+
},
86+
});
87+
88+
return [previous, created];
89+
});
90+
91+
return { previousKey, newKey, gracePeriodEndsAt };
92+
}

frontend/next.config.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,39 @@ const nextConfig: NextConfig = {
225225
key: "Critical-CH",
226226
value: "sec-ch-prefers-color-scheme, sec-ch-viewport-width",
227227
},
228+
{
229+
key: "Content-Security-Policy",
230+
value: [
231+
"default-src 'self'",
232+
"script-src 'self' 'unsafe-inline' 'unsafe-eval' https://vercel.live",
233+
"style-src 'self' 'unsafe-inline' https://fonts.googleapis.com",
234+
"font-src 'self' https://fonts.gstatic.com",
235+
"img-src 'self' data: https: blob:",
236+
"connect-src 'self' https://api.stellar.org https://horizon-testnet.stellar.org",
237+
"frame-src 'none'",
238+
"object-src 'none'",
239+
"base-uri 'self'",
240+
"form-action 'self'",
241+
"frame-ancestors 'none'",
242+
"upgrade-insecure-requests",
243+
].join("; "),
244+
},
245+
{
246+
key: "X-Content-Type-Options",
247+
value: "nosniff",
248+
},
249+
{
250+
key: "X-Frame-Options",
251+
value: "DENY",
252+
},
253+
{
254+
key: "Referrer-Policy",
255+
value: "strict-origin-when-cross-origin",
256+
},
257+
{
258+
key: "Permissions-Policy",
259+
value: "geolocation=(), microphone=(), camera=()",
260+
},
228261
],
229262
},
230263
{

0 commit comments

Comments
 (0)