diff --git a/Overlord-Server/src/db.ts b/Overlord-Server/src/db.ts index 5ad75ab..7197155 100644 --- a/Overlord-Server/src/db.ts +++ b/Overlord-Server/src/db.ts @@ -1634,7 +1634,15 @@ export function saveSavedScript( name: string, content: string, scriptType: string, -): SavedScriptRecord { +): SavedScriptRecord | null { + const owner = db + .query<{ user_id: number }, [string]>( + `SELECT user_id FROM saved_scripts WHERE id = ?`, + ) + .get(id); + if (owner && owner.user_id !== userId) { + return null; + } const now = Date.now(); db.run( `INSERT INTO saved_scripts (id, user_id, name, content, script_type, created_at, updated_at) @@ -1643,7 +1651,8 @@ export function saveSavedScript( name = excluded.name, content = excluded.content, script_type = excluded.script_type, - updated_at = excluded.updated_at`, + updated_at = excluded.updated_at + WHERE saved_scripts.user_id = excluded.user_id`, id, userId, name, @@ -2058,6 +2067,15 @@ export function deletePushSubscription(endpoint: string): void { db.run(`DELETE FROM push_subscriptions WHERE endpoint=?`, endpoint); } +export function deletePushSubscriptionForUser(userId: number, endpoint: string): boolean { + const result = db.run( + `DELETE FROM push_subscriptions WHERE endpoint=? AND user_id=?`, + endpoint, + userId, + ); + return ((result as any)?.changes || 0) > 0; +} + export function deletePushSubscriptionsByUser(userId: number): void { db.run(`DELETE FROM push_subscriptions WHERE user_id=?`, userId); } diff --git a/Overlord-Server/src/main-server.ts b/Overlord-Server/src/main-server.ts index 5fe2546..a1cfc6a 100644 --- a/Overlord-Server/src/main-server.ts +++ b/Overlord-Server/src/main-server.ts @@ -13,7 +13,7 @@ import { v4 as uuidv4 } from "uuid"; import { authenticateRequest } from "./auth"; import { loadConfig, getConfig } from "./config"; import { flushAuditLogsSync } from "./auditLog"; -import { getUserById, getUsersForNotificationDelivery, isClientOwnedByUser, canUserAccessClient, setUserClientAccessRule, setUserClientAccessScope, getUserClientAccessScope, hasPermission } from "./users"; +import { getUserById, getUsersForNotificationDelivery, getUsersForNotificationDeliveryByClientOwnership, isClientOwnedByUser, canUserAccessClient, setUserClientAccessRule, setUserClientAccessScope, getUserClientAccessScope, hasPermission } from "./users"; import { requireAuth, requirePermission } from "./rbac"; import { metrics } from "./metrics"; import { ensureDataDir } from "./paths"; @@ -342,6 +342,7 @@ const notificationPluginHandlers = createNotificationPluginHandlers({ pluginState, getNotificationConfig, canUserAccessClient: canUserAccessClient as (userId: number, userRole: string, clientId: string) => boolean, + isClientOwnedByUser, getUserRole: (userId: number) => getUserById(userId)?.role, isClientNotificationsMuted, storeNotificationScreenshot: storeNotificationScreenshotForPending, @@ -349,14 +350,12 @@ const notificationPluginHandlers = createNotificationPluginHandlers({ forwardPluginEventToRuntime: (clientId, pluginId, event, payload) => pluginRuntime.dispatchClientEvent(clientId, pluginId, event, payload), getDeliveryTargetsForClientEvent: (event: string, clientId: string): UserDeliveryTarget[] => { - const all = getUsersForNotificationDelivery(); - let filtered: typeof all; if (event === "client_purgatory") { - filtered = all.filter((u) => u.role === "admin" || u.role === "operator"); - } else { - filtered = all.filter((u) => canUserAccessClient(u.id, u.role, clientId)); + return getUsersForNotificationDeliveryByClientOwnership(clientId).map(toDeliveryTarget); } - return filtered.map(toDeliveryTarget); + return getUsersForNotificationDelivery() + .filter((u) => canUserAccessClient(u.id, u.role, clientId)) + .map(toDeliveryTarget); }, savePluginState, }); diff --git a/Overlord-Server/src/rbac.test.ts b/Overlord-Server/src/rbac.test.ts index 2ff7f30..29ee606 100644 --- a/Overlord-Server/src/rbac.test.ts +++ b/Overlord-Server/src/rbac.test.ts @@ -178,7 +178,6 @@ describe("getPermissionDescription", () => { const cases: [Permission, string][] = [ ["users:manage", "Manage users and roles"], ["clients:control", "Control clients (execute commands, desktop, console, files)"], - ["clients:view", "View connected clients"], ["clients:build", "Build client binaries"], ["clients:enroll", "Manage client enrollment approvals"], ["audit:view", "View audit logs"], diff --git a/Overlord-Server/src/rbac.ts b/Overlord-Server/src/rbac.ts index 6e05283..281cc47 100644 --- a/Overlord-Server/src/rbac.ts +++ b/Overlord-Server/src/rbac.ts @@ -4,7 +4,6 @@ import { hasPermission, type UserRole } from "./users"; export type Permission = | "users:manage" | "clients:control" - | "clients:view" | "clients:build" | "clients:enroll" | "audit:view" @@ -73,8 +72,6 @@ export function getPermissionDescription(permission: Permission): string { return "Manage users and roles"; case "clients:control": return "Control clients (execute commands, desktop, console, files)"; - case "clients:view": - return "View connected clients"; case "clients:build": return "Build client binaries"; case "clients:enroll": @@ -107,7 +104,6 @@ export function getRolePermissions(role: UserRole): Permission[] { if (hasPermission(role, "users:manage")) permissions.push("users:manage"); if (hasPermission(role, "clients:control")) permissions.push("clients:control"); - if (hasPermission(role, "clients:view")) permissions.push("clients:view"); if (hasPermission(role, "clients:build")) permissions.push("clients:build"); if (hasPermission(role, "clients:enroll")) permissions.push("clients:enroll"); if (hasPermission(role, "audit:view")) permissions.push("audit:view"); diff --git a/Overlord-Server/src/server/notification-delivery.ts b/Overlord-Server/src/server/notification-delivery.ts index d4d6200..65f72de 100644 --- a/Overlord-Server/src/server/notification-delivery.ts +++ b/Overlord-Server/src/server/notification-delivery.ts @@ -58,6 +58,20 @@ export const DEFAULT_TELEGRAM_TEMPLATE = const NOTIFICATION_SCREENSHOT_WAIT_MS = 5_000; const NOTIFICATION_SCREENSHOT_POLL_MS = 250; +export function isPrivateOrInternalHostname(hostname: string): boolean { + const h = hostname.toLowerCase(); + const BLOCKED_HOSTS = ["localhost", "metadata.google.internal", "169.254.169.254"]; + return ( + BLOCKED_HOSTS.includes(h) || + h.endsWith(".internal") || + h.startsWith("127.") || + h === "[::1]" || + /^(10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.)/.test(h) || + h.startsWith("169.254.") || + h.startsWith("0.") + ); +} + function getScreenshotMeta(format: string | undefined): { contentType: string; ext: string } { const normalized = (format || "jpeg").toLowerCase(); if (normalized === "png") return { contentType: "image/png", ext: "png" }; @@ -175,16 +189,7 @@ async function deliverToUserWebhook( } const hostname = parsed.hostname.toLowerCase(); - const BLOCKED_HOSTS = ["localhost", "metadata.google.internal", "169.254.169.254"]; - if ( - BLOCKED_HOSTS.includes(hostname) || - hostname.endsWith(".internal") || - hostname.startsWith("127.") || - hostname === "[::1]" || - /^(10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.)/.test(hostname) || - hostname.startsWith("169.254.") || - hostname.startsWith("0.") - ) { + if (isPrivateOrInternalHostname(hostname)) { logger.warn(`[notify] blocked webhook to private/internal address: ${hostname}`); return; } @@ -359,6 +364,7 @@ export async function deliverWebPushClientEvent( canUserAccessClient: (userId: number, userRole: string, clientId: string) => boolean, getUserRole: (userId: number) => string | undefined, isClientEventPushEnabled?: (userId: number) => boolean, + isClientOwnedByUser?: (userId: number, clientId: string) => boolean, ): Promise { const subs = getAllPushSubscriptions(); if (subs.length === 0) return; @@ -393,7 +399,14 @@ export async function deliverWebPushClientEvent( if (!role) return; if (isClientEventPushEnabled && !isClientEventPushEnabled(sub.userId)) return; if (event === "client_purgatory") { - if (role !== "admin" && role !== "operator") return; + if (role === "admin") { + } else if (role === "operator") { + if (!isClientOwnedByUser || !isClientOwnedByUser(sub.userId, info.id)) { + return; + } + } else { + return; + } } else if (role !== "admin") { if (!canUserAccessClient(sub.userId, role, info.id)) return; } diff --git a/Overlord-Server/src/server/routes/chat-routes.ts b/Overlord-Server/src/server/routes/chat-routes.ts index d6ed20d..8dc18e4 100644 --- a/Overlord-Server/src/server/routes/chat-routes.ts +++ b/Overlord-Server/src/server/routes/chat-routes.ts @@ -16,6 +16,10 @@ export async function handleChatRoutes( }); } + if (!hasPermission(user.role, "chat:write", user.userId)) { + return new Response("Forbidden: Chat access denied", { status: 403 }); + } + const beforeParam = url.searchParams.get("before"); const limitParam = url.searchParams.get("limit"); const before = beforeParam ? Number(beforeParam) : undefined; diff --git a/Overlord-Server/src/server/routes/client-routes.ts b/Overlord-Server/src/server/routes/client-routes.ts index a86695e..186f227 100644 --- a/Overlord-Server/src/server/routes/client-routes.ts +++ b/Overlord-Server/src/server/routes/client-routes.ts @@ -158,11 +158,8 @@ export async function handleClientRoutes( const user = await authenticateRequest(req); if (!user) return new Response("Unauthorized", { status: 401 }); - try { - requirePermission(user, "clients:control"); - } catch (error) { - if (error instanceof Response) return error; - return new Response("Forbidden", { status: 403 }); + if (user.role !== "admin") { + return new Response("Forbidden: Admin access required", { status: 403 }); } if (req.method === "GET") { diff --git a/Overlord-Server/src/server/routes/enrollment-routes.ts b/Overlord-Server/src/server/routes/enrollment-routes.ts index 574ab80..4adb569 100644 --- a/Overlord-Server/src/server/routes/enrollment-routes.ts +++ b/Overlord-Server/src/server/routes/enrollment-routes.ts @@ -346,11 +346,8 @@ export async function handleEnrollmentRoutes( if (req.method === "GET" && url.pathname === "/api/enrollment/banned-ips") { const user = await authenticateRequest(req); if (!user) return new Response("Unauthorized", { status: 401 }); - try { - requirePermission(user, "clients:enroll"); - } catch (error) { - if (error instanceof Response) return error; - return new Response("Forbidden", { status: 403 }); + if (user.role !== "admin") { + return new Response("Forbidden: Admin access required", { status: 403 }); } return Response.json({ items: listBannedIps() }); @@ -360,11 +357,8 @@ export async function handleEnrollmentRoutes( if (req.method === "DELETE" && url.pathname === "/api/enrollment/banned-ips") { const user = await authenticateRequest(req); if (!user) return new Response("Unauthorized", { status: 401 }); - try { - requirePermission(user, "clients:enroll"); - } catch (error) { - if (error instanceof Response) return error; - return new Response("Forbidden", { status: 403 }); + if (user.role !== "admin") { + return new Response("Forbidden: Admin access required", { status: 403 }); } const ipToUnban = (url.searchParams.get("ip") || "").trim(); @@ -390,11 +384,8 @@ export async function handleEnrollmentRoutes( if (req.method === "POST" && url.pathname === "/api/enrollment/ban-ip") { const user = await authenticateRequest(req); if (!user) return new Response("Unauthorized", { status: 401 }); - try { - requirePermission(user, "clients:enroll"); - } catch (error) { - if (error instanceof Response) return error; - return new Response("Forbidden", { status: 403 }); + if (user.role !== "admin") { + return new Response("Forbidden: Admin access required", { status: 403 }); } let body: any; diff --git a/Overlord-Server/src/server/routes/notifications-config-routes.ts b/Overlord-Server/src/server/routes/notifications-config-routes.ts index d9ac054..4029d4a 100644 --- a/Overlord-Server/src/server/routes/notifications-config-routes.ts +++ b/Overlord-Server/src/server/routes/notifications-config-routes.ts @@ -2,15 +2,17 @@ import { authenticateRequest } from "../../auth"; import { AuditAction, logAudit } from "../../auditLog"; import * as clientManager from "../../clientManager"; import { getConfig, updateNotificationsConfig } from "../../config"; -import { savePushSubscription, deletePushSubscription, getPushSubscriptionsByUser } from "../../db"; +import { savePushSubscription, deletePushSubscriptionForUser, getPushSubscriptionsByUser } from "../../db"; import { encodeMessage } from "../../protocol"; import { + canUserAccessClient, getUserNotificationSettings, updateUserNotificationSettings, } from "../../users"; import { DEFAULT_WEBHOOK_TEMPLATE, DEFAULT_TELEGRAM_TEMPLATE, + isPrivateOrInternalHostname, renderNotificationTemplate, } from "../notification-delivery"; import { getVapidPublicKey } from "../web-push"; @@ -20,7 +22,7 @@ type RequestIpProvider = { }; type NotificationsRouteDeps = { - getNotificationScreenshot: (notificationId: string) => { format?: string; bytes: Uint8Array } | null; + getNotificationScreenshot: (notificationId: string) => { format?: string; bytes: Uint8Array; clientId?: string } | null; secureHeaders: (contentType?: string) => HeadersInit; }; @@ -45,6 +47,13 @@ export async function handleNotificationsConfigRoutes( return new Response("Not found", { status: 404 }); } + if ( + screenshot.clientId && + !canUserAccessClient(user.userId, user.role, screenshot.clientId) + ) { + return new Response("Not found", { status: 404 }); + } + const format = (screenshot.format || "jpeg").toLowerCase(); const contentType = format === "jpg" || format === "jpeg" ? "image/jpeg" @@ -67,8 +76,8 @@ export async function handleNotificationsConfigRoutes( headers: { "Content-Type": "application/json" }, }); } - if (user.role === "viewer") { - return new Response("Forbidden: Viewer access denied", { status: 403 }); + if (user.role !== "admin") { + return new Response("Forbidden: Admin access required", { status: 403 }); } return Response.json({ notifications: getConfig().notifications }); } @@ -348,6 +357,13 @@ export async function handleNotificationsConfigRoutes( return Response.json({ error: "Invalid webhook URL" }, { status: 400 }); } + if (isPrivateOrInternalHostname(parsed.hostname.toLowerCase())) { + return Response.json( + { error: "Webhook URL points at a private/internal address" }, + { status: 400 }, + ); + } + const sampleRecord = { id: "preview-notification", clientId: "abc123def456", @@ -489,7 +505,7 @@ export async function handleNotificationsConfigRoutes( return Response.json({ error: "Missing endpoint" }, { status: 400 }); } - deletePushSubscription(endpoint); + deletePushSubscriptionForUser(user.userId, endpoint); return Response.json({ ok: true }); } } diff --git a/Overlord-Server/src/server/routes/saved-scripts-routes.ts b/Overlord-Server/src/server/routes/saved-scripts-routes.ts index 7530c26..55e82c0 100644 --- a/Overlord-Server/src/server/routes/saved-scripts-routes.ts +++ b/Overlord-Server/src/server/routes/saved-scripts-routes.ts @@ -56,6 +56,9 @@ export async function handleSavedScriptsRoutes( : "powershell"; const item = saveSavedScript(id, user.userId, name, content, scriptType); + if (!item) { + return Response.json({ error: "Script id already in use" }, { status: 409 }); + } return Response.json({ ok: true, item }); } diff --git a/Overlord-Server/src/server/routes/ws-upgrade-routes.ts b/Overlord-Server/src/server/routes/ws-upgrade-routes.ts index ef41f9c..cccc7f9 100644 --- a/Overlord-Server/src/server/routes/ws-upgrade-routes.ts +++ b/Overlord-Server/src/server/routes/ws-upgrade-routes.ts @@ -1,7 +1,7 @@ import { authenticateRequest } from "../../auth"; import { logger } from "../../logger"; import { isIpBanned } from "../../db"; -import { canUserAccessClient, canUserAccessFeature } from "../../users"; +import { canUserAccessClient, canUserAccessFeature, hasPermission } from "../../users"; import type { FeatureName } from "../../users"; type RequestServer = { @@ -296,6 +296,9 @@ export async function handleWsUpgradeRoutes( if (!user) { return new Response("Unauthorized", { status: 401 }); } + if (!hasPermission(user.role, "chat:write", user.userId)) { + return new Response("Forbidden: Chat access denied", { status: 403 }); + } const ip = server.requestIP(req)?.address || ""; if (server.upgrade(req, { data: { role: "chat_viewer", clientId: "", ip, userRole: user.role, userId: user.userId } })) { return new Response(); diff --git a/Overlord-Server/src/server/ws-notifications-plugin.ts b/Overlord-Server/src/server/ws-notifications-plugin.ts index 847447f..4e5016f 100644 --- a/Overlord-Server/src/server/ws-notifications-plugin.ts +++ b/Overlord-Server/src/server/ws-notifications-plugin.ts @@ -65,6 +65,7 @@ type CreateDeps = { pluginState: { enabled: Record; lastError: Record }; getNotificationConfig: () => NotificationConfigShape; canUserAccessClient: (userId: number, userRole: string, clientId: string) => boolean; + isClientOwnedByUser: (userId: number, clientId: string) => boolean; getUserRole: (userId: number) => string | undefined; isClientNotificationsMuted: (clientId: string) => boolean; storeNotificationScreenshot: ( @@ -459,7 +460,14 @@ export function createNotificationPluginHandlers(deps: CreateDeps) { const sRole = session.userRole ?? session.viewer.data.userRole ?? ""; const sUserId = session.userId ?? session.viewer.data.userId; if (event === "client_purgatory") { - if (sRole !== "admin" && sRole !== "operator") continue; + if (sRole === "admin") { + } else if (sRole === "operator") { + if (sUserId === undefined || !deps.isClientOwnedByUser(sUserId, info.id)) { + continue; + } + } else { + continue; + } } else { if (sRole !== "admin") { if (sUserId === undefined || !deps.canUserAccessClient(sUserId, sRole, info.id)) { @@ -473,10 +481,17 @@ export function createNotificationPluginHandlers(deps: CreateDeps) { const externalTargets = deps.getDeliveryTargetsForClientEvent(event, info.id); const pushEnabledByUser = new Map(externalTargets.map((t) => [t.userId, t.clientEventPush])); - deliverWebPushClientEvent(event, info, deps.canUserAccessClient, deps.getUserRole, (userId) => { - const enabled = pushEnabledByUser.get(userId); - return enabled !== undefined ? enabled : true; - }).catch((err) => logger.warn("[notify] web push client event delivery failed", err)); + deliverWebPushClientEvent( + event, + info, + deps.canUserAccessClient, + deps.getUserRole, + (userId) => { + const enabled = pushEnabledByUser.get(userId); + return enabled !== undefined ? enabled : true; + }, + deps.isClientOwnedByUser, + ).catch((err) => logger.warn("[notify] web push client event delivery failed", err)); deliverClientEventToExternalChannels(event, info, externalTargets).catch((err) => logger.warn("[notify] external channel client event delivery failed", err), diff --git a/Overlord-Server/src/users.client-access.test.ts b/Overlord-Server/src/users.client-access.test.ts index b959310..dbc4116 100644 --- a/Overlord-Server/src/users.client-access.test.ts +++ b/Overlord-Server/src/users.client-access.test.ts @@ -1,7 +1,6 @@ import { describe, expect, test } from "bun:test"; import { canUserAccessClient, - canViewClients, createUser, deleteUser, getUserByUsername, @@ -34,12 +33,6 @@ function cleanupCreatedUsers() { } describe("users client access RBAC", () => { - test("clients:view permission is admin-only by default", () => { - expect(canViewClients("admin")).toBe(true); - expect(canViewClients("operator")).toBe(false); - expect(canViewClients("viewer")).toBe(false); - }); - test("new non-admin users default to no client visibility", async () => { try { const viewer = await createTempUser("viewer"); diff --git a/Overlord-Server/src/users.ts b/Overlord-Server/src/users.ts index b08c9aa..4fa519b 100644 --- a/Overlord-Server/src/users.ts +++ b/Overlord-Server/src/users.ts @@ -973,10 +973,6 @@ export function canControlClients(role: UserRole): boolean { return role === "admin" || role === "operator"; } -export function canViewClients(role: UserRole): boolean { - return role === "admin"; -} - export function canBuildClients(userId: number, role: UserRole): boolean { if (role === "admin") return true; const user = getUserById(userId); @@ -997,8 +993,6 @@ export function hasPermission(role: UserRole, permission: string, userId?: numbe return canManageUsers(role); case "clients:control": return canControlClients(role); - case "clients:view": - return canViewClients(role); case "clients:build": if (userId !== undefined) return canBuildClients(userId, role); return role === "admin" || role === "operator";