Skip to content

Conversation

@nfebe
Copy link
Contributor

@nfebe nfebe commented Dec 24, 2025

…tion

  • Whitelist table with default internal IPs/paths prevents self-blocking
  • CRUD API for whitelist management (/api/security/whitelist)
  • RejectUnknownDomains config drops connections to unconfigured hosts
  • Real client IP extracted from CF-Connecting-IP/X-Forwarded-For headers
  • Docker gateway auto-whitelisted on startup
  • /stats endpoint includes networks and ports counts

- Whitelist table with default internal IPs/paths prevents self-blocking
- CRUD API for whitelist management (/api/security/whitelist)
- RejectUnknownDomains config drops connections to unconfigured hosts
- Real client IP extracted from CF-Connecting-IP/X-Forwarded-For headers
- Docker gateway auto-whitelisted on startup
- /stats endpoint includes networks and ports counts

Signed-off-by: nfebe <fenn25.fn@gmail.com>
Copy link

@sourceant sourceant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. No specific code suggestions were generated. See the overview comment for a summary.

@nfebe nfebe force-pushed the feat/configurable-detection-thresholds branch from d3d6f04 to 5b413c3 Compare December 24, 2025 00:55
Copy link

@sourceant sourceant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. No specific code suggestions were generated. See the overview comment for a summary.

Add API endpoint to track requests to domains not matching any
configured deployment, useful for detecting reconnaissance attempts
and misconfigured DNS.

Signed-off-by: nfebe <fenn25.fn@gmail.com>
@sourceant
Copy link

sourceant bot commented Dec 24, 2025

Code Review Summary

✨ This pull request significantly enhances the agent's security capabilities by introducing an IP/CIDR/path whitelisting mechanism, both managed by the agent and integrated into Nginx via Lua scripts. It also adds functionality for tracking unknown domain traffic statistics and improves Nginx configuration flexibility. The changes introduce several valuable security and monitoring features.

🚀 Key Improvements

  • Introduction of a robust whitelisting feature for IPs, CIDRs, and request paths, with caching in Nginx Lua for performance.
  • Enhanced Nginx configuration management to dynamically apply settings like RejectUnknownDomains based on agent config.
  • Improved client IP detection in Nginx Lua scripts, correctly handling X-Forwarded-For and CF-Connecting-IP headers.
  • New internal Nginx API endpoints to dynamically manage blocked IPs and whitelist entries, allowing the agent to update Nginx rules in real-time.
  • Added unknown domain traffic statistics to help identify suspicious probing or misconfigurations.

💡 Minor Suggestions

  • Refactor internal/api/server.go:getSystemStats to remove redundant calculation of network and port counts.
  • Review the ON CONFLICT logic in internal/security/db.go:AddWhitelistEntry to ensure that internal whitelist entries are not unintentionally degraded by external updates.
  • Consider adding more inline comments to the complex IPv6 CIDR matching logic in templates/infra/nginx/lua/security.lua for better future maintainability.

🚨 Critical Issues

  • Compilation Failure: Several files in internal/security (db.go, manager.go, models.go) have incorrect package declarations (package traffic instead of package security), which will prevent the agent from building successfully.
  • Unauthorized Internal API Access: The Nginx Lua handlers for internal security operations (handle_block_ip_request, handle_unblock_ip_request, handle_refresh_request) currently lack X-Internal-Token validation. This allows any authenticated or unauthenticated client (within the Docker network) to call these critical endpoints, posing a severe security risk.

Copy link

@sourceant sourceant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. See the overview comment for a summary.

- Extend is_ip_in_cidr to handle both IPv4 and IPv6 addresses
- Replace certificate-based reject with ssl_reject_handshake directive
- Remove ensureDefaultSSLCert function and related crypto imports

Signed-off-by: nfebe <fenn25.fn@gmail.com>
Copy link

@sourceant sourceant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. See the overview comment for a summary.

@nfebe nfebe merged commit eecaa0b into main Dec 24, 2025
5 checks passed
@nfebe nfebe deleted the feat/configurable-detection-thresholds branch December 24, 2025 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants