Skip to content

fix(preflight): correct 4 proxy/TLS-inspection detection bugs (client#589) - #600

Merged
LukasWodka merged 1 commit into
developfrom
fix/1427-preflight-proxy-tls
Aug 5, 2026
Merged

fix(preflight): correct 4 proxy/TLS-inspection detection bugs (client#589)#600
LukasWodka merged 1 commit into
developfrom
fix/1427-preflight-proxy-tls

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Addresses the 4 Bugbot findings on the staging mirror (client#589), all in the corporate-proxy / TLS-inspection probe from #590/#582:

sevfilefix
Highscripts/lib/preflight.sh:755|| issuer="" wiped an already-captured issuer (s_client exits non-zero via SIGPIPE after x509 even on a good handshake) → MITM networks read 'unknown'. Now || true; a genuinely empty capture is still caught by the [[ -z ]] below.
Mediumpreflight.sh:697_pf_urldecode ran the whole string through printf '%b', expanding literal \ in passwords. Escape backslashes first → percent-only, matching the PS peer's UnescapeDataString.
Mediumpreflight.sh:734openssl -help | grep -q under pipefail dropped -proxy_user (grep -q SIGPIPEs openssl). Capture then match.
Mediuminstall-k8s.ps1:4018[System.Uri] rejected schemeless proxy.corp:8080; prepend a scheme like the display path already does.

@shujaatTracebloc — your area (preflight/proxy). Once this merges, client rejoins the next staging hop. Regenerated the R8 manifest for the install-k8s.ps1 change.


Note

Low Risk
Changes are confined to best-effort preflight network probing and installer manifest hashes; no auth, cluster, or data-path behavior.

Overview
Corporate proxy and TLS-inspection preflight was misclassifying some real MITM/proxy setups as unknown instead of detecting inspection. This change tightens parity between the bash and PowerShell paths and avoids wiping good probe results.

On bash (preflight.sh): issuer capture no longer clears a valid issuer when openssl s_client exits non-zero after SIGPIPE; _pf_urldecode escapes backslashes before printf '%b' so proxy passwords match PowerShell percent-only decoding; and OpenSSL -proxy_user support is detected via captured help text instead of openssl -help | grep -q, which failed under pipefail.

On Windows (install-k8s.ps1), schemeless proxy env values like proxy.corp:8080 get an http:// prefix before [System.Uri] parsing so the HEAD probe can connect through the proxy.

scripts/manifest.sha256 is updated for the touched install scripts.

Reviewed by Cursor Bugbot for commit e0646a2. Bugbot is set up for automated code reviews on this repo. Configure here.

…#589)
Bugbot on the staging mirror flagged 4 issues in the corporate-proxy / TLS-
inspection probe (from #590/#582):
- HIGH: the TLS probe cleared an already-captured issuer via `|| issuer=""`;
openssl s_client often exits non-zero (SIGPIPE after x509) even on a good
handshake, so MITM networks read as 'unknown'. Use `|| true` (empty capture is
still caught below).
- proxy password url-decode ran the whole string through printf '%b', mangling
literal backslashes; escape them first so only percent-escapes expand (PS parity).
- `openssl -help | grep -q` under pipefail dropped -proxy_user on authenticated
proxies (grep -q closes the pipe, openssl gets SIGPIPE); capture then match.
- PS Get-TlsInspectionState: [System.Uri] rejected schemeless proxy.corp:8080;
prepend a scheme like the display path already does.
Regenerated the R8 manifest for the install-k8s.ps1 change.

@shujaatTraceblocshujaatTracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved — reviewed carefully as this is my area (preflight/proxy). All 4 fixes are correct, and I independently verified them against the surrounding code (the function runs under set -euo pipefail, which is what makes the two SIGPIPE bugs real):

  • High — preflight.sh:763|| issuer=""|| true. Correct. Under pipefail the substitution inherits s_client's SIGPIPE exit even though x509 already printed a valid issuer, so the old idiom wiped a good capture and reported unknown on real MITM networks. || true preserves it; a genuinely empty capture is still caught by the [[ -z ]] guard on 766.
  • Medium — _pf_urldecode backslash escaping. Correct. Doubling before `printf %b` stops literal// in a password from being interpreted, so decoding is percent-only, matching the PS peer's UnescapeDataString. Verified order-of-operations: %5C→literal `` still works, literal A is preserved.
  • Medium — capture-then-match for -proxy_user. Correct. openssl -help | grep -q under pipefail fails the pipeline when grep closes the pipe early (openssl SIGPIPEs), dropping the credential flags on exactly the authenticated proxies this handles. The captured-var pattern match avoids the pipe.
  • Medium — install-k8s.ps1 schemeless proxy. Correct. The scheme regex is RFC-3986-shaped and prepends http:// only when no scheme:// is present, so [System.Uri] parses proxy.corp:8080; existing https://… values pass through untouched.

Manifest: recomputed sha256 of both touched files locally — both match scripts/manifest.sha256 exactly. Full CI green (incl. bats, Pester, Source-of-truth drift) and Cursor Bugbot passed clean.

Non-blocking nit (fine as a follow-up — not worth a manifest-regen + full CI re-run to block the staging hop): the block comment on preflight.sh:758 still reads "|| issuer="" keeps a failed/timed-out probe from aborting" but the code beneath it is now || true. The new inline comment on 763-765 is accurate; just the older reference on 758 is stale.

@LukasWodka
LukasWodka merged commit c7aed81 into developAug 5, 2026
49 checks passed
@LukasWodka
LukasWodka deleted the fix/1427-preflight-proxy-tls branch August 14, 2026 13:53
Sign up for freeto 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

@LukasWodka@shujaatTracebloc