fix(domains,routes): address footgun review findings (async/correctness/perf/security) - #188
Merged
Merged
Conversation
…ss/perf/security) Domain/routes subsystem review follow-ups: - routes.js: await-safe monitor.checkRouteById() in create+update (no unhandled rejection); clamp ?limit lower bound on /:id/trace (negative splice bug); async (non-blocking) tail read of the trace log; central isPrivateOrReservedIpv4() SSRF guard incl. 100.64/10 CGNAT (POST+PUT); /check-dns now uses domains.getServerPublicIp() as the single server-IP source (drops the divergent ipify/getServerIp cache + isPublicIp helper). - domains.js: canonicalize IPs (ipaddr.js) before compare so a non-canonical IPv6 server-IP override still verifies; upsert() uses RETURNING * (no second SELECT). - domainBoot.js: verifyAndReflag runs verifications concurrently and prepares its UPDATE statements once; seed loop prepares its INSERT once. - ipaddr.js promoted to a direct dependency (pinned 1.9.1, already deduped via proxy-addr). - Tests: IPv6 non-canonical verify; CGNAT SSRF reject + 100.128/9 allow boundary.
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.
footgun-Review-Fixes — Domain/Routes-Subsystem
Behebt alle 13 Funde der footgun-Analyse des Domain/Routes-Subsystems (2 major, 6 minor, 5 nit). Keine Merge-Blocker, aber risikoarme Verbesserungen + 2 echte Korrekturen.
routes.js
monitor.checkRouteById()in Create+Update war fire-and-forget → eine async-Rejection wurde zur unhandled rejection. Jetzt.catch(() => {})./:id/trace:?limithatte keine Untergrenze → negativer Wert führte beientries.splice(limit)zum Löschen vom Ende. JetztMath.max(…, 1).isPrivateOrReservedIpv4()-Helper inkl. 100.64.0.0/10 (CGNAT), einheitlich in POST+PUT./check-dnsnutzt jetztdomains.getServerPublicIp()als einzige Server-IP-Quelle; die divergentegetServerIp()/ipify-Cache- +isPublicIp()-Logik (mit Race + irreführendem Namen) wurde entfernt.domains.js
verify()kanonisiert beide Seiten (ipaddr.js) → ein nicht-kanonischer IPv6-Server-IP-Override verifiziert jetzt korrekt.upsert()nutztRETURNING *statt eines zweiten SELECT.domainBoot.js
verifyAndReflagführt die unabhängigen Verifikationen parallel aus und bereitet seine UPDATE-Statements einmal vor (statt pro Zeile).Sonstiges
ipaddr.jsals direkte Dependency deklariert (gepinnt 1.9.1, bereits via proxy-addr dedupliziert).Tests (lokal grün, CI ist das Gate)
Domain-Suites 23/23 · Routes-API 46/46 · api/sync/grouping 71/71 · neue/aktualisierte 13/13.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Nfanfu8v8xvSMCbuAmd8Sq