harden(portal): Pi-hole-Portal footgun-Review-Befunde (v5-Isolation + Härtung) - #194
Merged
Merged
Conversation
…-safe lookup FIX 1 (piholeSync): catch() on getTopClients(true) so Pi-hole v5 instances degrade topClientsBlocked to [] instead of marking the instance disconnected. FIX 2 (portalOwner): add explanatory comment documenting the intentional Kiosk trade-off on the device-trust branch (no behaviour change). FIX 3 (api/portal): extract piholeUnavailable(cache) helper; replaces the duplicated 3-clause guard in all three pihole handlers. FIX 4 (public/portal.js): split hydratePiholeScope into two named inner functions (renderPiholeReason, renderPiholeStats); test-safe via option (a). FIX 5 (public/portal.js): whitelist scope before PI_ENDPOINTS lookup to prevent proto-poisoning via DOM-supplied scope value. FIX 8 (api/portal): expand packed aggregation lines in /pihole/owner handler for readability; logic identical. FIX 9 (public/portal.js): boot call uses piScopeActive instead of literal 'device' (same runtime value, forward-compatible). New test: tests/pihole_sync_v5_degrade.test.js (2 cases, all pass). Full suite: 42/42 green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Härtung: Pi-hole-Portal (footgun-Review-Befunde)
Nachbesserung der von einer mehrstufigen JS-Review (footgun) gefundenen Punkte am Pi-hole-Portal (TP2a+TP2b). Verhaltens-erhaltend; keine Gate-/Präzedenz-/Leak-Semantik geändter.
portalIdentityunangetastet.Behoben
piholeSync.js— Pi-hole-v5-Isolation:client.getTopClients(true)lag imPromise.all; auf v5 (FTL ohne den v6-Endpoint) warf der Call und riss die ganze Instanz aufconnected:false(alle übrigen Daten verloren). Jetzt.catch(() => [])→ nurtopClientsBlockeddegradiert still zu[], die Instanz bleibt voll funktional. Neuer Testpihole_sync_v5_degrade.test.js(2 Fälle).portal.jsroute — Guard-Dedup: der dreifach kopierte License/Cache-Guard in einen HelperpiholeUnavailable(cache)extrahiert (3 Handler).portal.jswidget —hydratePiholeScopeentflochten: Render in benannte (test-sichere innere) FunktionenrenderPiholeReason/renderPiholeStatsaufgeteilt; Verhalten + DOM-Sicherheit + Leak-Guard identisch.portal.jswidget — Proto-sicherer Scope-Lookup: Whitelist stattPI_ENDPOINTS[scope](verhindert__proto__/constructor-TypeError).portal.jsroute — Aggregations-Pfad lesbar: gepackte Zeile + inline-for-Bodies entzerrt (security-sensitiver Pfad auditierbar).portal.jswidget — Initial-Scope überpiScopeActivestatt hartkodiertem'device'.portalOwner.js— Kiosk-Trade-off dokumentiert: der Shared-Peer-Fall des Trust-Schalters ist bewusstes Design (Default aus + Admin-Opt-in + Pflicht-Help-Text, Design §4.6); im Code als Kommentar festgehalten — kein Verhaltensbruch (das wäre eine Feature-Aushebelung).Bewusst NICHT umgesetzt (begründet)
getDb()legt das DB-Handle lazy an undclose()nullt es (Test-teardown()/Prod-Reconnect → neues Handle); ein gecachtes better-sqlite3-Statement liefe dann auf einem geschlossenen Handle → Use-after-close.getDb().prepare()pro Aufruf ist die repo-weite, sichere Konvention. Der vorgeschlagene „Fix" wäre selbst ein Bug.Tests
Volle TP2a+TP2b-Regression grün (mit
NODE_ENV=testfür die piholeSync-Tests, CI-Standard); neuer v5-Degrade-Test grün;portalIdentityper git-diff unangetastet. Fokussiertes Review: Ready to merge (alle 7 Punkte verhaltens-erhaltend verifiziert).🤖 Generated with Claude Code
https://claude.ai/code/session_01PrxALUszC9wFkYv1fedKyd