fix(core): bracket IPv6 bind addresses in forward arg and access URL#2449
fix(core): bracket IPv6 bind addresses in forward arg and access URL#2449andrewwhitecdw wants to merge 1 commit into
Conversation
ForwardSpec::parse supports IPv6 binds (e.g. '::1:8080'), but ssh_forward_arg emitted '::1:8080:127.0.0.1:8080', which OpenSSH misparses, and access_url printed the invalid URL 'http://::1:8080/'. Bracket IPv6 literals as OpenSSH and RFC 3986 require, matching the bracketing logic already used by format_gateway_url, and extend the existing forward spec tests with IPv6 cases. Signed-off-by: Andrew White <andrewh@cdw.com>
|
All contributors have signed the DCO ✍️ ✅ |
|
I have read the DCO document and I hereby sign the DCO. |
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This is a project-valid, concentrated core bug fix. It restores valid OpenSSH and URL formatting for an IPv6 bind address; PR #2409 was the same author's earlier submission and was closed only by the vouch gate.
Head SHA: bc87935b91607195c7c6b5e281a78e8b55b7cbc8
Review findings:
- The focused implementation and regression tests are sound; no correctness or security defect was found.
- The new helper leaves identical IPv6-bracketing logic in
format_gateway_url(); the inline suggestion asks to consolidate that logic so the two paths cannot drift. - This changes direct user-visible forwarding behavior and printed access URLs. Please add an IPv6 example to the existing Port Forwarding section in
docs/sandboxes/manage-sandboxes.mdx, including the bracketedhttp://[::1]:8000/output. Existing navigation already covers that page, sodocs/index.ymldoes not need to change.
Docs: missing for this direct UX change.
E2E/pipeline: deferred until review feedback is resolved.
Next state: gator:in-review
|
|
||
| /// Bracket an IPv6 literal for use in `host:port` contexts (SSH `-L` | ||
| /// arguments and URLs); leave other addresses unchanged. | ||
| fn bracket_ipv6(addr: &str) -> String { |
There was a problem hiding this comment.
gator-agent
Suggestion: format_gateway_url() still duplicates this exact IPv6-bracketing logic. Please have it call bracket_ipv6(host) as well so URL and forwarding behavior cannot drift. Its existing tests can remain unchanged.
johntmyers
added
the
gator:in-review
2 participants
Summary
ForwardSpec::parsesupports IPv6 bind addresses (e.g.openshell forward ::1:8080, covered by an existing parse test), but two formatters emitted unbracketed output:ssh_forward_arg()produced::1:8080:127.0.0.1:8080, which OpenSSH misparses — the-Lforward failsaccess_url()producedhttp://::1:8080/, an invalid URL (RFC 3986 requires[::1]) printed verbatim to usersThis PR supersedes #2409, which was auto-closed by the vouch-check workflow before I was vouched.
Related Issue
N/A — small fix found during code review. The same file already brackets IPv6 literals correctly in
format_gateway_url; this applies the same logic to the forward spec.Changes
bracket_ipv6helper (same condition asformat_gateway_url) and used it inssh_forward_arg()andaccess_url()forward_spec_ssh_forward_argandforward_spec_access_urltests with::1casesTesting
mise run pre-commitpasses (mise unavailable in this environment; ran equivalentcargo fmt+cargo clippy -p openshell-core --all-targets— clean)cargo test -p openshell-core forward_spec— 9 passed)Checklist