[codex] Fix KiwiSDR proxy redirect transport#3850
Merged
ten9876 merged 2 commits intoJun 27, 2026
Merged
Conversation
Follow validated KiwiSDR /status redirects and use the final trusted status endpoint for WebSocket transport. Allow same-host redirects and the matching Kiwi proxy-to-proxy2 alias only; reject unrelated domains before opening WebSockets.
There was a problem hiding this comment.
Pull request overview
Fixes KiwiSDR proxy redirect handling during /status preflight so that AetherSDR can connect through Kiwi’s proxy→proxy2 migration while validating redirects before using the resulting host/port as the WebSocket transport authority (boundary input validation).
Changes:
- Adds manual redirect handling for
/statuspreflight, with allowlisted redirect validation (same-host, plus proxy↔proxy2 receiver-alias migration). - Uses the final validated
/statusURL to derive the WebSocket scheme/host/port for subsequent WebSocket connections. - Tracks redirect counts and persists a resolved WebSocket port separately from the original endpoint port.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/core/KiwiSdrClient.h | Adjusts preflight API and adds state for resolved WebSocket port + redirect tracking. |
| src/core/KiwiSdrClient.cpp | Implements manual redirect following/validation and derives WebSocket transport from the final validated /status URL. |
Comment on lines
+334
to
+395
| QString canonicalRedirectHost(QString host) | ||
| { | ||
| host = host.trimmed().toLower(); | ||
| while (host.endsWith(QLatin1Char('.'))) { | ||
| host.chop(1); | ||
| } | ||
| return host; | ||
| } | ||
|
|
||
| QString kiwiProxyReceiverAlias(const QString& host) | ||
| { | ||
| constexpr QLatin1StringView kProxySuffix{".proxy.kiwisdr.com"}; | ||
| constexpr QLatin1StringView kProxy2Suffix{".proxy2.kiwisdr.com"}; | ||
| if (host.endsWith(kProxySuffix) && host.size() > kProxySuffix.size()) { | ||
| return host.left(host.size() - kProxySuffix.size()); | ||
| } | ||
| if (host.endsWith(kProxy2Suffix) && host.size() > kProxy2Suffix.size()) { | ||
| return host.left(host.size() - kProxy2Suffix.size()); | ||
| } | ||
| return {}; | ||
| } | ||
|
|
||
| bool isAllowedKiwiStatusRedirect(const QUrl& from, | ||
| const QUrl& to, | ||
| QString* detail) | ||
| { | ||
| const QString scheme = to.scheme().toLower(); | ||
| if (scheme != QStringLiteral("http") && scheme != QStringLiteral("https")) { | ||
| if (detail) { | ||
| *detail = QStringLiteral("unsupported scheme %1").arg(to.scheme()); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| const QString fromHost = canonicalRedirectHost(from.host()); | ||
| const QString toHost = canonicalRedirectHost(to.host()); | ||
| if (fromHost.isEmpty() || toHost.isEmpty()) { | ||
| if (detail) { | ||
| *detail = QStringLiteral("missing redirect host"); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| if (fromHost == toHost) { | ||
| return true; | ||
| } | ||
|
|
||
| // Kiwi's public proxy can migrate one receiver alias from proxy to proxy2. | ||
| // Keep that exception exact so a status page cannot move the WebSocket | ||
| // target to an unrelated host after the policy preflight succeeds. | ||
| const QString fromAlias = kiwiProxyReceiverAlias(fromHost); | ||
| const QString toAlias = kiwiProxyReceiverAlias(toHost); | ||
| if (!fromAlias.isEmpty() && fromAlias == toAlias) { | ||
| return true; | ||
| } | ||
|
|
||
| if (detail) { | ||
| *detail = QStringLiteral("cross-domain redirect from %1 to %2") | ||
| .arg(fromHost, toHost); | ||
| } | ||
| return false; | ||
| } |
Comment on lines
+634
to
+640
| if (redirectUrl.isEmpty()) { | ||
| setState( | ||
| State::Error, | ||
| tr("This KiwiSDR's status page redirected without a target, so AetherSDR won't connect.")); | ||
| cleanupSockets(); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Thanks @rfoust — this is a clean, well-scoped fix. The manual redirect handling is fail-closed by default, the proxy→proxy2 alias exception is narrow and exact, and extracting the policy into KiwiSdrRedirectPolicy with a dedicated unit test is exactly the right call. The transport-resolution logic correctly threads the final validated /status host/port/scheme into m_webSocketPort so the WebSocket opens against the right endpoint. Nice work on the boundary validation (Principle VII).
A few notes, all minor / optional:
Reviewed and confirmed correct:
- Reply lifecycle is leak-free — each
startStatusPreflightassigns a freshm_statusReplyand the prior reply isdeleteLater()'d; the redirect-count reset on the https retry is handled. - The 3xx block is correctly placed before
if (ok), which is necessary sinceManualRedirectPolicymakes a 3xx finish withNoError. - Per-hop validation (
requestUrl→nextUrl) chains correctly through theproxy:8073 → proxy2 → proxy2:8073reproduction, andproxyReceiverAliascorrectly rejects a bareproxy.kiwisdr.com(no receiver prefix) via thesize() >guard — good that the test pins this.
On the two Copilot findings — both appear already addressed in the final diff:
- The helpers were extracted into a testable
KiwiSdrRedirectPolicyunit (kiwi_sdr_redirect_policy_test.cpp) covering same-host, proxy↔proxy2, and rejected cross-domain/scheme cases. Resolved. - The 3xx-without-target message now reads "unsupported HTTP %1 without a redirect target" and includes the status code, so the 304 wording concern is handled. Resolved.
Optional polish (non-blocking):
- The rejection path collapses three distinct reasons (cross-domain, unsupported scheme, too many redirects) into one user-facing string — "redirected outside its trusted KiwiSDR proxy host." For the
>= kStatusPreflightMaxRedirectscase that message is slightly inaccurate (the host may be fine, the chain is just too long). The precise reason is already in theqCWarninglog, so this is cosmetic, but a distinct message for the redirect-limit case would read better. isAllowedStatusRedirectpermits a same-host https→http downgrade. Given the trust is already scoped to the Kiwi proxy host this isn't a real exposure, but if you wanted to be strict you could forbid downgrading the scheme on same-host hops. Entirely optional.
No blockers from me. LGTM.
🤖 aethersdr-agent · cost: $2.1574 · model: claude-opus-4-8
ten9876
approved these changes
Jun 27, 2026
ten9876
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Independently traced the redirect/SSRF surface and it's genuinely fail-closed (Principle VII):
- Policy is suffix-anchored —
21042.proxy.kiwisdr.com.evil.comand bareproxy.kiwisdr.comboth reject; the cross-host exception can only ever target<same-id>.proxy[2].kiwisdr.com, so a status page can't repoint the WebSocket to an unrelated domain. Non-http(s) schemes rejected up front. - Enforcement is correct —
ManualRedirectPolicymakes the policy authoritative; per-hop validation onrequestUrl → resolved(redirectUrl); the alias is preserved across hops so a multi-hop chain stays confined; max-redirect cap (8) kills the proxy↔proxy2 bounce; the WS authority is taken from the policy-validated final URL; 3xx handled beforeif (ok)(correct for manual-redirect NoError finishes). - Well-tested — canonical normalization, same-host, proxy↔proxy2 both ways, cross-domain reject, mismatched-id reject, scheme reject.
Two cosmetic nits (generic "too many redirects" message; permitted same-host https→http downgrade) are non-blocking. Thanks @rfoust — clean, security-conscious fix for the proxy split.
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
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
Fixes #3790. KiwiSDR
/statuspreflight now follows validated redirects and uses the final validated status endpoint for WebSocket transport, so proxied receivers such as21042.proxy.kiwisdr.com:8073can connect after Kiwi'sproxy->proxy2split.Redirect handling is manual and fail-closed: same-host redirects are allowed, and the only cross-host exception is the matching Kiwi proxy receiver alias, such as
21042.proxy.kiwisdr.com->21042.proxy2.kiwisdr.com. Redirects to unrelated domains or mismatched proxy aliases are rejected before WebSockets open.Constitution principle honored
Principle VII — Boundary Input Validation. The redirected
/statusURL is validated before it can become the WebSocket authority.Test plan
cmake --build build -j10ctest --test-dir build -R 'kiwi_sdr_protocol_test|kiwi_sdr_trace_math_test|kiwi_public_directory_test' --output-on-failurecurl -sSIL --max-time 10 http://21042.proxy.kiwisdr.com:8073/status21042.proxy.kiwisdr.com:8073->21042.proxy2.kiwisdr.com->21042.proxy2.kiwisdr.com:8073->200 OKext_api=3git diff --checkChecklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key (Principle V)MeterSmoother(not applicable; no meter UI touched)