-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Add NAT localnet support with custom ports 6060/6061 #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
17ef3c6
23d7249
5a6821a
1fd8257
ebe2599
93fc0b7
b4b659c
11d0463
6f22ae7
9375c4c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
|
|
||
| # Don't index SpecStory auto-save files, but allow explicit context inclusion via @ references | ||
| .specstory/** |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # SpecStory project identity file | ||
| /.project.json | ||
| # SpecStory explanation file | ||
| /.what-is-this.md |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -412,6 +412,8 @@ route[WITHINDLG] { | |||||
| route(DLGURI); | ||||||
| if ( is_method("ACK") ) { | ||||||
| # ACK is forwarded statelessly | ||||||
| # Ensure correct socket is used (especially for INTERNAL_NETWORK) | ||||||
|
||||||
| # Ensure correct socket is used (especially for INTERNAL_NETWORK) | |
| # Ensure correct socket is used for ACKs across all relevant network types |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "uses PRIVATE_IP for advertised address" but the actual implementation now sets the socket to PRIVATE_IP:6060 instead of using set_advertised_address(). The comment should be updated to accurately reflect that the socket is being set to a specific IP:port combination, not just using an advertised address.
| # - LOCALNETWORKS: uses PRIVATE_IP for advertised address | |
| # - LOCALNETWORKS: sets the socket to PRIVATE_IP:6060 (specific IP:port) |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new logic to extract destination_ip from $du when present (lines 1071-1075) is a good improvement. However, the comparison $du != 0 is checking if $du is the numeric value 0, which may not be meaningful for a URI string. Consider simplifying to just check $du != $null && $du != "" for clarity.
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded port ":6060" is used here for LOCALNETWORKS traffic, but this differs from the standard port :5060 used elsewhere. While this appears intentional based on the PR description, there's an inconsistency: the listener setup in bootstrap.sh adds port 6060 only for NAT scenarios, but this code will try to use it regardless of NAT configuration. This could cause issues when not behind NAT.
| $fs = PRIVATE_IP + ":6060"; | |
| $fs = PRIVATE_IP; |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The INTERNAL_NETWORK check at line 1106 can override the LOCALNETWORKS socket setting from line 1092. If a destination IP is in both LOCALNETWORKS and INTERNAL_NETWORK, the socket will first be set to PRIVATE_IP:6060 (line 1092), but then immediately overridden to SERVICE_IP:5060 (line 1110). Consider restructuring this logic with else-if to ensure only one socket configuration is applied, or document why both checks should run sequentially.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The firewall configuration opens port 6061/tcp but not 6061/udp, while port 5061 is opened for both TCP and UDP (lines 13-14). The listener configuration in bootstrap.sh uses TLS on port 6061 (line 39), which typically only needs TCP. However, for consistency with how port 5061 is configured, consider whether 6061/udp should also be opened, or document why the asymmetry is intentional.