Merged
Conversation
…ks (#409) * Fix alert severity inflation, schedule resilience, and add source links - Exclude system-category issues from audit.completed severity counts so fingerprint DB unavailability doesn't inflate to Error severity - Isolate DB update failures in ScheduleService so a successful task isn't mislabeled as failed when UpdateRunStatusAsync throws - Reconnect to UniFi console before scheduled audits to prevent stale session failures on fingerprint cache refresh - Enrich schedule summary with finding counts (e.g., "Score: 100 - 0 critical, 3 recommended") - Add SourceUrl property to AlertEvent and AlertHistoryEntry with fragment-based deep links (e.g., /speedtest#result-123, /audit) - Render View links in Active Alerts, Acknowledged, and History tabs - Include SourceUrl in all notification channels (ntfy, Discord, Teams, Slack, Email, Webhook) * Use tab query params for data usage and schedule source links Data usage alerts link to /alerts?tab=data-usage and schedule alerts link to /alerts?tab=schedule instead of bare /alerts. * Render source URL as plain text in email template Relative paths like /audit are broken as <a href> in email clients since there's no base URL context. Render as plain text instead. * Resolve SourceUrl to absolute URLs for notification channels Use the same hostname priority as the canonical redirect in Program.cs (REVERSE_PROXIED_HOST_NAME > HOST_NAME > HOST_IP) to build absolute URLs in AlertProcessingService. Email template restored to clickable "View in app" link now that URLs are absolute. * Fall back to auto-detected IP for alert source URLs HOST_IP is only set if the user configures it. Use DetectLocalIpFromInterfaces() as the final fallback so source URLs are always absolute when possible. * Fix delivery channels receiving relative instead of absolute source URLs Delivery channels were using alertEvent.SourceUrl (the original relative path like "/audit") instead of the resolved absolute URL from historyEntry.SourceUrl. Uses record with-expression to pass the resolved URL to DeliverAsync.
* Fix legacy firewall rule mapping regression from #407 (#251) PR #407 added "ANY" matching targets for empty source/dest on legacy rules, which caused infrastructure rules like "Allow Established/Related" and "Drop Invalid State" to eclipse the real inter-VLAN block rule. - Guard "ANY" fallback with isStateless check so stateful rules get null matching targets (invisible to network-pair evaluator) - Fix evaluator to skip non-NEW block rules when forNewConnections=true - Fix CheckForProblematicAllowRules to use forNewConnections=true - Skip non-NEW allow rules in DetectPermissiveRules - Check BlocksNewConnections in VlanAnalyzer.IsIsolatedViaFirewall - Add 17 new tests covering legacy infrastructure rules and integration * Add comprehensive rule engine tests for legacy RFC1918 block posture Multi-network integration test verifying the full evaluation chain: infrastructure rules (EST/REL, Drop Invalid) invisible, RFC1918 block satisfies isolation for all pairs, and DetectPermissiveRules doesn't flag infrastructure rules as permissive ANY->ANY.
Media networks (entertainment, streaming, theater, A/V) were previously classified as IoT, causing false-positive IsolationBypassed criticals when intentional allow rules exist (e.g., "Allow Guest to Media"). Media is semi-trusted: peers with IoT (no isolation between them), accessible from Guest, but cannot initiate to trusted networks (Corporate, Home, Gaming, Server). Gaming networks (consoles, Xbox, PlayStation) were previously classified as Home. Now a distinct type with the same trust level as Home - including UPnP support.
* Accept Media and Gaming networks for device VLAN placement Streaming devices, smart TVs, media players on a Media network and game consoles on a Gaming network are now correctly placed (no audit finding). Also remove dead IoT/Home word boundary arrays and fix AlertHistoryEntry.SourceUrl doc comment. * Only accept entertainment devices on Media network Media networks allow Guest access, so security devices (smart locks, cameras, hubs) should NOT be on Media. Only entertainment devices (streaming, TVs, media players, speakers, consoles) are correctly placed there. * Include Media in DNS shared servers and routing checks Media is semi-trusted (Guest can access it), so it should be checked for shared DNS servers with corporate and for routing being enabled, same as IoT and Guest networks.
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.
Summary