4.1 KiB
SmartProxy Module Problems
Based on test analysis, the following potential issues have been identified in the SmartProxy module:
1. HttpProxy Route Configuration Issue
Location: ts/proxies/http-proxy/http-proxy.ts:380
Problem: The HttpProxy is trying to read the 'type' property of an undefined object when updating route configurations.
Evidence: test.http-forwarding-fix.ts
fails with:
TypeError: Cannot read properties of undefined (reading 'type')
at HttpProxy.updateRouteConfigs (/mnt/data/lossless/push.rocks/smartproxy/ts/proxies/http-proxy/http-proxy.ts:380:24)
Impact: Routes with useHttpProxy
configuration may not work properly.
2. Connection Forwarding Issues
Problem: Basic TCP forwarding appears to not be working correctly after the simplification to just 'forward' and 'socket-handler' action types. Evidence: Multiple forwarding tests timeout waiting for data to be forwarded:
test.forwarding-fix-verification.ts
- times out waiting for forwarded datatest.connection-forwarding.ts
- times out on SNI-based forwarding Impact: The 'forward' action type may not be properly forwarding connections to target servers.
3. Missing Certificate Manager Methods
Problem: Tests expect provisionAllCertificates
method on certificate manager but it may not exist or may not be properly initialized.
Evidence: Multiple tests fail with "this.certManager.provisionAllCertificates is not a function"
Impact: Certificate provisioning may not work as expected.
4. Route Update Mechanism
Problem: The route update mechanism may have issues preserving certificate manager callbacks and other state. Evidence: Tests specifically designed to verify callback preservation after route updates. Impact: Dynamic route updates might break certificate management functionality.
5. Route-Specific Security Not Fully Implemented
Problem: While the route definitions support security configurations (ipAllowList, ipBlockList, authentication), these are not being enforced at the route level. Evidence:
- SecurityManager has methods like
isIPAuthorized
for route-specific security - Route connection handler only checks global IP validation, not route-specific security rules
- No evidence of route.action.security being checked when handling connections Impact: Route-specific security rules defined in configuration are not enforced, potentially allowing unauthorized access. Status: ✅ FIXED - Route-specific IP allow/block lists are now enforced when a route is matched. Authentication is logged as not enforceable for non-terminated connections. Additional Fix: Removed security checks from route matching logic - security is now properly enforced AFTER a route is matched, not during matching.
6. Security Property Location Consolidation
Problem: Security was defined in two places - route.security and route.action.security - causing confusion. Status: ✅ FIXED - Consolidated to only route.security. Removed action.security from types and updated all references.
Recommendations
-
Verify Forward Action Implementation: Check that the 'forward' action type properly establishes bidirectional data flow between client and target server. ✅ FIXED - Basic forwarding now works correctly.
-
Fix HttpProxy Route Handling: Ensure that route objects passed to HttpProxy.updateRouteConfigs have the expected structure with all required properties. ✅ FIXED - Routes now preserve their structure.
-
Review Certificate Manager API: Ensure all expected methods exist and are properly documented.
-
Add Integration Tests: Many unit tests are testing internal implementation details. Consider adding more integration tests that test the public API.
-
Implement Route-Specific Security: Add security checks when a route is matched to enforce route-specific IP allow/block lists and authentication rules. ✅ FIXED - IP allow/block lists are now enforced at the route level.
-
Fix TLS Detection Logic: The connection handler was treating all connections as TLS. This has been partially fixed but needs proper testing for all TLS modes.