fix(install): unblock Windows 10 downloads, and make the activation refusal actionable (#1856) - #2188
Merged
Merged
Conversation
install.ps1 set its protocol bitmask to Tls12 -bor Tls13 unconditionally. Windows 10's schannel has no TLS 1.3 -- it arrives with Windows 11 and Server 2022 (build 20348) -- but .NET Framework 4.8 still DEFINES the enum value, so the assignment succeeds and nothing warns. The first HTTPS request then dies with "The request was aborted: Could not create SSL/TLS secure channel", because an unsupported flag in this bitmask is a hard failure rather than a downgrade. Every Windows 10 user was blocked before a single byte downloaded, on the documented `irm ... | iex` path, with an error that points at the network rather than at the installer. Compute the set instead: start at Tls12 and add Tls13 only on build 20348+. The enum-name probe alongside the build gate keeps the script parsing on .NET Framework 4.7, where the member does not exist at all. Verified on the Windows VM (build 26200) with the shipped parser: PARSE_OK, pure ASCII preserved, the guard computes and assigns "Tls12, Tls13" there, forcing build 19045 through the same expression yields "Tls12" alone, and an HTTPS HEAD to github.com under the assigned set returns 200. The contract test fails on the previous installer with all four assertions -- unconditional bit, missing build gate, missing enum probe, literal bitmask -- and passes with this change. Reported with a full ProcMon trace and a decoded SID by zaferavci1. Refs #1856 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A reporter brought a full ProcMon trace to #1856 and still could not say which check refused their install. That is not their failing: the binary discards the answer before printing it. cli_activation_production_context_init() validates the cache, rendezvous and log directories -- it is the emitter MOST likely to hold a useful detail -- and it printed CLI_ACTIVATION_REFUSED_MESSAGE bare, through the raw sink rather than cli_activation_diagnostic(). So the reader got "Check the errors above" with nothing above. That is the exact dead end #1416 and #1537 already fixed; the property was repaired on the paths that had tests and left broken on this sibling, which had none. Route it through the attributing helper: the refusal now names the transaction refusal note or cbm_daemon_ipc_validation_detail(). The remedy was also wrong for the reported shape. `icacls <dir> /remove:g <sid>` cannot remove an INHERITED ACE, and the stock C:\ grant for Authenticated Users reaches every new child directory exactly that way -- so a reporter following our advice watches the command succeed and the refusal persist. The refusal now says whether the ACE was inherited, and the advice names `icacls <dir> /inheritance:r /grant:r "%USERNAME%":(OI)(CI)F` for that case. No safety check is relaxed. An install directory writable by other accounts is still refused; it is now refused in terms the reader can act on. The new contract test guards the CLASS rather than this one call site: any emitter of a refusal constant that bypasses the attributing helper fails it, which is what would have caught this. It fails on the previous tree naming src/cli/cli.c:750, plus the missing inherited-ACE report and the missing /inheritance:r remedy. Local: cli 314 passed; activation_transaction daemon_ipc daemon_bootstrap daemon_version 110 passed, 1 skipped (user namespaces are Linux-only, runs on the Linux leg); make -f Makefile.cbm lint-ci clean. Reported by zaferavci1. Refs #1856 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added a commit
that referenced
this pull request
Sep 12, 2026
The macos-15-intel leg killed the cli suite at its 900s wall clock while reporting 7537 passed, 0 failed. Two separate things were wrong. The suite was misclassified. cli spends 497s of the 900s default on macos-14, the FASTEST macOS runner, while macos-15-intel in the same matrix is 2.4-3.6x slower on comparable suites (daemon_runtime 842s vs 349s, stack_overflow_b 277s vs 76s). 497s at that ratio cannot fit. daemon_runtime, at 842s on that same runner, passes only because it was already in SLOW_SUITES. cli belongs there too -- it is deterministic and simply large, which is the only thing that list is allowed to mean. The drain test was also waiting on a clock instead of on the system. Profiling the suite per test (315 tests, 300s wall / 166s cpu) put cli_install_into_host_namespace_still_drains_host_cohort at 42s wall for 19s of cpu -- 23s idle, the largest single idle block, where the next worst was 11s. The existing cli_install_force_quiesces_active_cohort_before_replacing_binary drains a cohort too and idles 0.5s, which is what pointed at the difference. The cost was the host_serving probe. Its generous budget exists for the POSITIVE question -- is this daemon still up? -- where a slow reply on a loaded runner must not be misread as drained; that fixed a real flake and is kept. Asked in the negative it inverts: no reply is coming, so the full 15s is spent establishing silence and ASSERT_FALSE is decided by the timeout expiring rather than by the daemon's behaviour. The drain is already proven positively in that test -- install returns 0 only after the activation completed, and the host child exits CLI_SCOPE_HOST_DRAINED -- so the probe only has to confirm it. Also shortens the drained child's teardown budget, which retried service_free / lease_release / manager_free against 10s on a path where the activation had already torn the service down. Behaviour at the deadline is unchanged; it is reached sooner when nothing is wedged. Measured at only -3s on its own, kept because it is correct and free. Local, same machine, ASan+UBSan: the drain test 42.0s -> 25.8s (-38%), the suite 300s -> 279s, 315 passed before and after -- no test removed, no assertion weakened. PR #2188, which runs the same leg without these two tests, passed macos-15-intel in 37m34s, so the suite was not already over budget on main. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
enabled auto-merge
September 12, 2026 16:04
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.
Two independent Windows install blockers from #1856, whose reporter did the work to find both.
1. The installer hard-failed on every Windows 10 machine
install.ps1setSecurityProtocol = Tls12 -bor Tls13unconditionally. Windows 10's schannel has no TLS 1.3 — it arrives with Windows 11 and Server 2022 (build 20348) — but .NET Framework 4.8 still defines the enum value, so the assignment succeeds and nothing warns. The first HTTPS request then dies withThe request was aborted: Could not create SSL/TLS secure channel, because an unsupported flag in this bitmask is a hard failure, not a downgrade.The protocol set is now computed:
Tls12, plusTls13only on build 20348+. The enum-name probe alongside the build gate keeps the script parsing on .NET Framework 4.7, where the member does not exist at all.Verified on a real Windows box (build 26200) with the shipped parser:
2. The refusal named no check, and its remedy could not remove the grant
The reporter captured a full ProcMon trace and still could not say which check refused. That is not their failing — the binary discards the answer before printing it.
cli_activation_production_context_init()validates the cache, rendezvous and log directories, so it is the emitter most likely to hold a useful detail. It printedCLI_ACTIVATION_REFUSED_MESSAGEbare through the raw sink instead ofcli_activation_diagnostic(), so the reader got "Check the errors above" with nothing above. That is the exact dead end #1416 and #1537 already fixed: the property was repaired on the paths that had tests and left broken on this sibling, which had none.The advice was wrong for the reported shape too.
icacls <dir> /remove:g <sid>cannot remove an inherited ACE, and the stockC:\grant for Authenticated Users reaches every new child directory exactly that way — so a reporter following our advice watches the command succeed and the refusal persist. The refusal now says whether the ACE was inherited, and namesicacls <dir> /inheritance:r /grant:r "%USERNAME%":(OI)(CI)Ffor that case.No safety check is relaxed. An install directory writable by other accounts is still refused — it is now refused in terms the reader can act on.
On the tolerance that was considered and rejected
#1856 also asks us to accept the stock
Authenticated UsersACE so that installs underC:\succeed. A design review rejected that, and the source backs it up:install.ps1copies itself into the install directory with inherited ACLs, andcbm updatetells the user to run that exact file.cbm installpersists the install directory intoHKCU\Environment\Path.The binary's own DACL is owner-only and
SE_DACL_PROTECTED, so the binary is safe — but the tolerance would have been on the directory, handing any authenticated local account a planted DLL beside the .exe, an editableinstall.ps1, and PATH shadowing. Refusing with an actionable message is the better trade, and it is what this PR does.A separate, real asymmetry stays open for its own change: Windows ancestors are never DACL-checked in
activation_transaction.c, while POSIX ancestors and the daemon's ancestors both are.Tests
Both halves are RED-proven before the fix:
tests/test_windows_bundle_contract.sh— four assertions fail on the previous installer (unconditional bit, missing build gate, missing enum probe, literal bitmask).tests/test_activation_diagnostic_contract.sh(new, wired intoscripts/test.shas Step 0e2) — fails namingsrc/cli/cli.c:750, plus the missing inherited-ACE report and missing/inheritance:rremedy. It guards the class: any emitter of a refusal constant that bypasses the attributing helper fails it, which is what would have caught the original miss.Local:
cli314 passed ·activation_transaction daemon_ipc daemon_bootstrap daemon_version110 passed, 1 skipped (user namespaces are Linux-only; runs on the Linux leg) ·make -f Makefile.cbm lint-ciclean.Thanks to @zaferavci1 — the ProcMon trace, the decoded
S-1-5-11, and the TLS bisect are why both of these are fixable at all.Refs #1856