Skip to content

Commit 44bc04e

Browse files
fix(security): viewer RBAC isolation + tenant-scoped key enumeration (#3382)
Closes #3361, closes #3359. Viewer blocked from 6 admin-only endpoints. Rate-limits tenant-scoped for non-system viewers.
1 parent b4bedf2 commit 44bc04e

3 files changed

Lines changed: 60 additions & 4 deletions

File tree

src/routes/analytics.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010

1111
import type { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify';
1212
import type { RouteContext } from './context.js';
13-
import { requireRole, registerWithLegacy } from './context.js';
13+
import { requireRole, registerWithLegacy, getRequestRole } from './context.js';
14+
import { SYSTEM_TENANT } from '../config.js';
1415
import type {
1516
RateLimitKeyUsage,
1617
RateLimitForecast,
@@ -29,6 +30,12 @@ export function registerAnalyticsRoutes(app: FastifyInstance, ctx: RouteContext)
2930
config: { rateLimit: { max: 60, timeWindow: '1 minute' } },
3031
handler: async (req: FastifyRequest, reply: FastifyReply) => {
3132
if (!requireRole(auth, req, reply, 'admin', 'operator', 'viewer')) return;
33+
// #3361: Viewer on non-system tenant blocked — summary is global operational data
34+
const role = getRequestRole(auth, req);
35+
if (role === 'viewer' && req.tenantId && req.tenantId !== SYSTEM_TENANT) {
36+
reply.status(403).send({ error: 'Forbidden: insufficient role' });
37+
return;
38+
}
3239
return metricsCache.getMetrics();
3340
},
3441
});
@@ -38,6 +45,12 @@ export function registerAnalyticsRoutes(app: FastifyInstance, ctx: RouteContext)
3845
config: { rateLimit: { max: 60, timeWindow: '1 minute' } },
3946
handler: async (req: FastifyRequest, reply: FastifyReply) => {
4047
if (!requireRole(auth, req, reply, 'admin', 'operator', 'viewer')) return;
48+
// #3361: Viewer on non-system tenant blocked — costs are global operational data
49+
const role = getRequestRole(auth, req);
50+
if (role === 'viewer' && req.tenantId && req.tenantId !== SYSTEM_TENANT) {
51+
reply.status(403).send({ error: 'Forbidden: insufficient role' });
52+
return;
53+
}
4154

4255
const metrics = metricsCache.getMetrics();
4356
const totalCostUsd = metrics.costTrends.reduce((sum, d) => sum + d.cost, 0);
@@ -78,6 +91,12 @@ export function registerAnalyticsRoutes(app: FastifyInstance, ctx: RouteContext)
7891
config: { rateLimit: { max: 60, timeWindow: '1 minute' } },
7992
handler: async (req: FastifyRequest, reply: FastifyReply) => {
8093
if (!requireRole(auth, req, reply, 'admin', 'operator', 'viewer')) return;
94+
// #3361: Viewer on non-system tenant blocked — tokens are global operational data
95+
const role = getRequestRole(auth, req);
96+
if (role === 'viewer' && req.tenantId && req.tenantId !== SYSTEM_TENANT) {
97+
reply.status(403).send({ error: 'Forbidden: insufficient role' });
98+
return;
99+
}
81100

82101
const query = req.query as { from?: string; to?: string };
83102
const metrics = metricsCache.getMetrics();
@@ -125,6 +144,40 @@ export function registerAnalyticsRoutes(app: FastifyInstance, ctx: RouteContext)
125144
handler: async (req: FastifyRequest, reply: FastifyReply) => {
126145
if (!requireRole(auth, req, reply, 'admin', 'operator', 'viewer')) return;
127146

147+
// #3359: Tenant-scope key enumeration — viewer sees only own tenant's keys
148+
const role = getRequestRole(auth, req);
149+
const callerTenantId = req.tenantId;
150+
const isSystemTenant = !callerTenantId || callerTenantId === SYSTEM_TENANT;
151+
if (role === 'viewer' && !isSystemTenant) {
152+
const allKeys = auth.listKeys();
153+
const keys = allKeys.filter(k => k.tenantId === callerTenantId);
154+
const allSessions = sessions.listSessions();
155+
const perKey: RateLimitKeyUsage[] = keys.map((key) => {
156+
const owned = allSessions.filter((s) => s.ownerKeyId === key.id);
157+
const usage = quotas.getUsage(key as unknown as ApiKey, owned.length);
158+
return {
159+
keyId: key.id,
160+
keyName: key.name,
161+
activeSessions: usage.activeSessions,
162+
maxSessions: usage.maxSessions,
163+
tokensInWindow: usage.tokensInWindow,
164+
maxTokens: usage.maxTokens,
165+
spendInWindowUsd: usage.spendInWindow,
166+
maxSpendUsd: usage.maxSpend,
167+
windowMs: usage.windowMs,
168+
};
169+
});
170+
const forecast = computeForecast(perKey);
171+
return {
172+
global: { ...GLOBAL_RATE_LIMIT },
173+
perKey,
174+
forecast,
175+
generatedAt: new Date().toISOString(),
176+
};
177+
}
178+
179+
// Admin/operator/system: show all keys
180+
128181
const keys = auth.listKeys();
129182
const allSessions = sessions.listSessions();
130183

src/routes/audit.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,19 +231,21 @@ export function registerAuditRoutes(app: FastifyInstance, ctx: RouteContext): vo
231231

232232
// Global metrics (Issue #40)
233233
// Note: cannot use registerWithLegacy because legacy path /metrics is used by Prometheus
234+
// #3361: Restricted to admin/operator — operational metrics are not viewer data
234235
app.get('/v1/metrics', {
235236
config: { rateLimit: { max: 120, timeWindow: '1 minute' } },
236237
handler: async (req: FastifyRequest, reply: FastifyReply) => {
237-
if (!requireRole(auth, req, reply, 'admin', 'operator', 'viewer')) return;
238+
if (!requireRole(auth, req, reply, 'admin', 'operator')) return;
238239
return metrics.getGlobalMetrics(sessions.listSessions().length);
239240
},
240241
});
241242

242243
// Bounded no-PII diagnostics channel (Issue #881)
244+
// #3361: Restricted to admin/operator — diagnostics are operational data
243245
registerWithLegacy(app, 'get', '/v1/diagnostics', {
244246
config: { rateLimit: { max: 120, timeWindow: '1 minute' } },
245247
handler: async (req: FastifyRequest, reply: FastifyReply) => {
246-
if (!requireRole(auth, req, reply, 'admin', 'operator', 'viewer')) return;
248+
if (!requireRole(auth, req, reply, 'admin', 'operator')) return;
247249
const parsed = diagnosticsQuerySchema.safeParse(req.query ?? {});
248250
if (!parsed.success) {
249251
return reply.status(400).send({

src/routes/health.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,9 @@ export function registerHealthRoutes(app: FastifyInstance, ctx: RouteContext): v
178178
});
179179

180180
// Issue #89 L15: Per-channel health reporting
181+
// #3361: Restricted to admin/operator — channel config is operational data
181182
registerWithLegacy(app, 'get', '/v1/channels/health', async (req: FastifyRequest, reply: FastifyReply) => {
182-
if (!requireRole(auth, req, reply, 'admin', 'operator', 'viewer')) return;
183+
if (!requireRole(auth, req, reply, 'admin', 'operator')) return;
183184
return channels.getChannels().map(ch => {
184185
const health = ch.getHealth?.();
185186
if (health) return health;

0 commit comments

Comments
 (0)