From 8fa43a3e4734057123022de1b45bf155e6c4bc89 Mon Sep 17 00:00:00 2001 From: D4rkst3r Date: Sat, 1 Aug 2026 04:48:52 +0200 Subject: [PATCH] Code-Review: drei Fehler behoben MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session-Cookie ohne secure-Flag (src/web/auth.js) Das Anmelde-Cookie war httpOnly, sameSite und signiert — aber ohne secure. Beim ersten Aufruf ueber http, bevor der Proxy auf https umleitet, geht es damit im Klartext mit. Auf einem fremden WLAN ist das die Sitzung. Das Flag laesst sich nicht aus request.protocol ableiten: hinter dem Reverse-Proxy kommt jede Anfrage als http an, trustProxy ist nicht gesetzt. Stattdessen aus der konfigurierten Adresse des Hosts, mit derselben Zuordnung, die der OAuth-Redirect schon benutzt. Lokal ueber http://localhost bleibt es aus, sonst kaeme man beim Entwickeln nicht rein. Geprueft gegen sechs Hosts: hub und bot ergeben secure, localhost nicht. Gilt auch fuer das OAuth-State- und das SSO-Ruecksprung-Cookie. Unbehandelte Rejection beim Start (src/bot/client.js) registerCommands() faengt nichts ab, der Aufrufer im ClientReady-Handler auch nicht. Seit Node 15 beendet eine unbehandelte Rejection den Prozess — ein Rate-Limit von Discord beim Start haette den Bot also abgeschossen und mit restart: unless-stopped in eine Neustart-Schleife geschickt. Dabei stehen die zuvor registrierten Befehle bei Discord weiter, und alles andere koennte laufen. Jetzt wird der Fehler geloggt und der Bot bleibt an. Number('') ist 0 (src/bot/anti-raid.js) Beim Aufheben der Raid-Sperre wird die gemerkte Verifizierungsstufe zurueckgesetzt. Fehlt die Notiz, ergab Number('') die 0 — und 0 ist eine gueltige Stufe, naemlich "keine". Der Server waere also auf offen gestellt worden statt so gelassen, wie er war. Jetzt wird nur zurueckgesetzt, was auch wirklich als Zahl notiert ist. Ohne Befund geprueft: alle 168 SQL-Abfragen gegen das echte Schema inklusive Migrationen, alle 109 Routen auf Rechtepruefung (53 von 54 schreibenden haben eine, die 54. haengt am timing-safe verglichenen Secret), Farbwerte vor dem Einbetten in ausgeliefertes CSS, das Escaping der OG-Meta-Tags, Timer-Cleanup im Frontend und die uebrigen async-Handler. Co-Authored-By: Claude Fable 5 --- src/bot/anti-raid.js | 8 ++++++-- src/bot/client.js | 11 ++++++++++- src/web/auth.js | 24 ++++++++++++++++++++++-- src/web/sso.js | 3 ++- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/bot/anti-raid.js b/src/bot/anti-raid.js index 0c68a51..aa5c922 100644 --- a/src/bot/anti-raid.js +++ b/src/bot/anti-raid.js @@ -131,11 +131,15 @@ export async function entsperren(client, { melde = true, durch = null } = {}) { if (!getSetting(BIS)) return false; const guild = client.guilds.cache.first(); - const zurueck = Number(getSetting(VORHER)); + // Number('') ist 0 — und 0 ist eine gültige Verifizierungsstufe („keine"). + // Ohne die Leerprüfung würde eine fehlende Notiz den Server auf offen + // stellen, statt ihn zu lassen, wie er war. Lieber nichts anfassen. + const notiert = String(getSetting(VORHER) ?? '').trim(); + const zurueck = /^\d+$/.test(notiert) ? Number(notiert) : null; const getan = []; if (guild) { - if (Number.isInteger(zurueck) && zurueck !== guild.verificationLevel) { + if (zurueck !== null && zurueck !== guild.verificationLevel) { const ok = await guild.setVerificationLevel(zurueck, 'Anti-Raid: Sperre aufgehoben') .then(() => true).catch(() => false); if (ok) getan.push('Verifizierung zurückgesetzt'); diff --git a/src/bot/client.js b/src/bot/client.js index c8ee40f..b6c8c99 100644 --- a/src/bot/client.js +++ b/src/bot/client.js @@ -76,7 +76,16 @@ export async function startBot() { client.once(Events.ClientReady, async (readyClient) => { console.log(`[bot] Eingeloggt als ${readyClient.user.tag}`); - await registerCommands(); + // Nicht mitreißen lassen: seit Node 15 beendet eine unbehandelte + // Rejection den Prozess. Ein Rate-Limit oder Netz-Schluckauf beim + // Registrieren würde den Bot sonst in eine Neustart-Schleife schicken, + // obwohl die zuvor registrierten Befehle bei Discord weiter stehen und + // alles andere — Devlogs, Monitor, Moderation — laufen könnte. + try { + await registerCommands(); + } catch (error) { + console.error('[bot] Slash-Commands konnten nicht registriert werden:', error.message); + } }); // Live-Archivierung: neue Devlogs (Webhook-Posts im Devlog-Kanal) sofort sichern diff --git a/src/web/auth.js b/src/web/auth.js index 8dce2a2..9eaac02 100644 --- a/src/web/auth.js +++ b/src/web/auth.js @@ -16,12 +16,28 @@ const STATE_COOKIE = 'd4rkbot_oauth_state'; * * Jede hier mögliche Adresse muss im Discord-Portal als Redirect eingetragen sein. */ -function redirectUri(request) { +function passendeBasis(request) { const host = String(request?.headers?.host ?? '').toLowerCase().split(':')[0]; const known = [hubUrl(), botUrl(), publicUrl()].find((u) => { try { return new URL(u).hostname === host; } catch { return false; } }); - return `${known ?? publicUrl()}/auth/callback`; + return known ?? publicUrl(); +} + +function redirectUri(request) { + return `${passendeBasis(request)}/auth/callback`; +} + +/** + * Darf das Cookie nur über HTTPS zurückkommen? + * + * Abgeleitet aus der konfigurierten Adresse und nicht aus request.protocol: + * hinter dem Reverse-Proxy kommt die Anfrage als HTTP an, das Flag wäre dann + * nie gesetzt. Lokal über http://localhost bleibt es aus, sonst käme man + * beim Entwickeln nicht mehr rein. + */ +export function nurUeberHttps(request) { + return String(passendeBasis(request)).startsWith('https://'); } /** Eingeloggten User aus dem signierten Session-Cookie lesen (null wenn nicht eingeloggt) */ @@ -68,6 +84,7 @@ export function registerAuthRoutes(app, client) { return reply .setCookie(STATE_COOKIE, state, { path: '/auth', httpOnly: true, sameSite: 'lax', maxAge: 600, signed: true, + secure: nurUeberHttps(request), }) .redirect(`https://discord.com/oauth2/authorize?${params}`); }); @@ -136,6 +153,9 @@ export function registerAuthRoutes(app, client) { return reply .setCookie(SESSION_COOKIE, JSON.stringify(session), { path: '/', httpOnly: true, sameSite: 'lax', maxAge: 7 * 24 * 3600, signed: true, + // Ohne secure geht das Cookie beim ersten Aufruf über http mit + // — noch bevor der Proxy auf https umleitet. + secure: nurUeberHttps(request), ...(domain ? { domain } : {}), }) // Der Callback läuft auf derselben Domain wie der Login — ein diff --git a/src/web/sso.js b/src/web/sso.js index 3b0f12d..452ca57 100644 --- a/src/web/sso.js +++ b/src/web/sso.js @@ -12,7 +12,7 @@ import crypto from 'node:crypto'; import { config } from '../config.js'; import { getSsoApp, touchSsoApp, webAdminScopes } from '../db.js'; import { discordGuildId } from '../runtime-settings.js'; -import { getSessionUser, isAdmin, isGuildMember } from './auth.js'; +import { getSessionUser, isAdmin, isGuildMember, nurUeberHttps } from './auth.js'; const TOKEN_TTL_MS = 60_000; const RETURN_COOKIE = 'd4rkbot_return'; @@ -85,6 +85,7 @@ export function registerSsoRoutes(app, client) { return reply .setCookie(RETURN_COOKIE, request.url, { path: '/', httpOnly: true, sameSite: 'lax', maxAge: 600, signed: true, + secure: nurUeberHttps(request), }) .redirect('/auth/login'); }