bug fixes.

This commit is contained in:
KingKDot
2026-05-18 23:07:11 -05:00
parent 59a2622b47
commit 423e41854f
14 changed files with 110 additions and 69 deletions
+20 -2
View File
@@ -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);
}
+6 -7
View File
@@ -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,
});
-1
View File
@@ -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"],
-4
View File
@@ -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");
@@ -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<void> {
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;
}
@@ -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;
@@ -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") {
@@ -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;
@@ -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 });
}
}
@@ -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 });
}
@@ -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();
@@ -65,6 +65,7 @@ type CreateDeps = {
pluginState: { enabled: Record<string, boolean>; lastError: Record<string, string> };
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),
@@ -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");
-6
View File
@@ -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";