fix(installer): harden the Windows install→first-experiment path — checksum-driven downloads, reliable cluster-create, writable ingest & training volumes, local-chart support (#611) - #612
Merged
Conversation
…ated tool binary self-heals (#611) Field follow-up to #607/#608. On a Windows machine behind a filtering proxy, the k3d download kept failing at "System tool checksum verification failed" even with #608's multi-transport download — because #608 validated a download by SIZE FLOOR + magic bytes only. A binary truncated mid-transfer to somewhere between the 10 MB floor and the real 25.8 MB still passed (it's >10 MB and starts with 'MZ'), so the fallbacks never fired and it dead-ended at the separate, no-retry checksum step. (Proven on the box: a manual download produced the correct hash at 25,805,312 bytes, while the installer's copy failed the checksum in ~2s with no fallback.) Fix — the checksum is the authoritative completeness test: - Get-VerifiedDownload gains -Sha256: after a transport lands a size/magic-valid file, its SHA-256 must equal the expected hash or the transport is treated as failed and the NEXT one (curl.exe -> BITS) is tried. A truncated/altered copy now self-heals instead of dead-ending. - Get-VerifiedDownload gains -MustContain for the small checksum-list files, so a proxy error page lacking the expected asset line is retried too. - k3d / kubectl / helm now fetch their checksum FIRST (resiliently) and pass the extracted, 64-hex-validated hash as the download gate. helm gains checksum verification on the PS path for the first time (parity with the bash path). Tests: Pester source-guards for the -Sha256/-MustContain gates, the mismatch->retry path, and per-tool checksum-first wiring; full suite green (444). Manifest regenerated. Contributes to #578. Closes #611. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shujaatTracebloc
force-pushed
the
fix/609-checksum-driven-download
branch
from
August 5, 2026 14:20
4032a15 to
436ae6e
Compare
shujaatTracebloc
marked this pull request as draft
August 5, 2026 14:21
…ries transports (Bugbot)
The kubectl .sha256 is a bare 64-hex hash with no fixed substring, so it used
-MinBytes 1 with no content gate -- a proxy error page satisfied the floor, the
first transport 'succeeded', curl.exe/BITS never ran, and the later hex check
aborted. Add -MatchPattern (a regex content gate) to Get-VerifiedDownload and use
'[0-9a-fA-F]{64}' for the kubectl checksum fetch, matching how k3d/helm use
-MustContain. Pester guards updated; manifest regenerated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ed (#611) Field report (same Windows box, past the k3d download fix): Step 3 aborted with "Failed to create compute environment" even though k3d printed "Cluster 'tracebloc' created successfully!" with EMPTY stderr and the cluster was actually up. Cause: Wait-ProcessWithDeadline polled HasExited but never called WaitForExit(), and Start-Process -RedirectStandardOutput can leave $proc.ExitCode $null in that window -- so `$null -ne 0` misread an exit-0 success as a failure. Not machine-specific; a latent race any Windows user can hit. - Wait-ProcessWithDeadline now calls $Process.WaitForExit() before returning success, so the redirected streams drain and ExitCode is reliable for EVERY caller (cluster create, partial delete, tracked installs). - Cluster-create adds defense-in-depth: a still-null exit code falls back to k3d's own "created successfully" marker rather than failing a cluster that is up. - Pester source-guards for both. Windows-only change (install-k8s.ps1); Linux/mac paths untouched and their suites remain green. Contributes to #578. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shujaatTracebloc
marked this pull request as ready for review
August 6, 2026 06:45
…ms in the exit fallback (Bugbot) Two Bugbot findings on the checksum-driven download work: 1. (High) The checksum-LIST fetch gates were fail-open, so a proxy error page "succeeded" on the first transport and skipped the curl.exe/BITS retry — exactly the case #611 exists to survive. Helm's -MustContain substring (helm-<ver>-windows-<arch>.zip) also appears in the request URL a proxy page can echo; kubectl's -MatchPattern was unanchored so any page with a 64-hex run passed; k3d gated on the bare asset name. Fix: drop the weak -MustContain entirely and gate every checksum-list fetch on the hash STRUCTURE — k3d/helm require a 64-hex hash adjacent to the asset, kubectl requires the hash at the start of the body. A proxy/HTML error page can't satisfy that, so it retries transports as intended. 2. (Medium) The null-exit-code cluster-create fallback only scanned $k3dStdout, but k3d logs its "Cluster created successfully!" line via logrus to STDERR — so a real success could be misread as failure. Fix: check both $k3dStdout and $k3dStderr. Pester source-guards updated: kubectl gate is start-anchored, k3d/helm gates are hash-anchored, no -MustContain remains, and the fallback inspects both streams. Full suite green (447). Manifest regenerated. Contributes to #578. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Path installs `tb data ingest` failed at the copy step with `mkdir: can't create directory '/data/shared/.tracebloc-staging/': Permission denied`. On hostPath installs (the Windows/WSL2 + bare-metal default) kubelet does not apply fsGroup to hostPath volumes (kubernetes/kubernetes#138411), so /data/shared (client-pvc) is created root-owned and the non-root ingest-staging pod can't write to it. mysql-data has a privileged init-chown for exactly this reason; the shared data volume had none. - jobs-manager gains fsGroup: 1000 (CSI clusters apply it to the shared volume). - On hostPath, a privileged init-shared-data container (root, CHOWN+FOWNER only) chowns /data/shared to 1000:1000 and chmod 2777. World-writable, unlike mysql-data's single-UID chown, because the shared volume has multiple non-root writers whose UIDs this chart doesn't control -- jobs-manager, the training/ ingestor pods it spawns, and the CLI's ingest-staging pod. setgid keeps new files in GID 1000; the init is gated on hostPath (CSI relies on fsGroup). - helm-unittest: init present + world-writable on hostPath; absent (fsGroup kept) on CSI. Chart bumped 1.9.15 -> 1.9.16. Client-side companion to the installer fixes on this PR (requested to land here). Contributes to #578. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… parity with bash) The Windows installer could only ever install the PUBLISHED chart (helm repo), so a branch-only chart change (e.g. the #611 /data/shared fix, or #585's global.imageRegistry) was impossible to test from a Windows install. The bash installer already supports a local chart via TRACEBLOC_CHART_PATH (_resolve_chart_ref); this brings Windows to parity. - When $env:TRACEBLOC_CHART_PATH is set, install-k8s.ps1 installs from that local chart directory (validated) and skips `helm repo add`; otherwise it uses the published repo as before. Applied to both the fresh-install and adopt/reconcile helm upgrades. - Pester source-guards for the local-chart ref, the repo-add skip, and the not-a-directory error. Manifest regenerated. Full suite green (451). Enables a from-scratch Windows test of the branch chart. Contributes to #578. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
reviewed
Aug 6, 2026
saadqbal
left a comment
Contributor
There was a problem hiding this comment.
Careful, well-tested PR — the checksum-as-completeness-test and the both-streams k3d exit fallback are the right calls. A few non-blocking things to weigh, mostly on the chart side (left inline).
#611) hostPath ignores fsGroup (kubernetes/kubernetes#138411), so /data/logs was created root-owned and non-root training/inference pods hit `PermissionError [Errno 13]` creating their per-run log dir (`os.makedirs('/data/logs/<run>')`). The #611 init-container chowned /data/shared but not /data/logs — the same class of bug on the logs volume. Extend the init (renamed init-shared-data -> init-writable-data) to chown+chmod BOTH hostPath volumes and mount both. Training and inference pods share one spec (job.yaml), so this covers both; ingestion already covered by the /data/shared chmod. Bump chart 1.9.16 -> 1.9.17. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…en-reuse parity Two reviewer follow-ups on #612: - chart: chmod the writable hostPath dirs 3777 (was 2777) — add the sticky bit so one writer can't unlink/rename another writer's files in /data/shared // /data/logs (/tmp semantics). setgid is retained. Safe given the uid topology (dir owned by 1000; training pods run as 1000; the ingestor writes its own subtrees as a stable uid) and no cross-uid filesystem deletes exist in client-runtime. Chart 1.9.17 -> 1.9.18. - install-k8s.ps1: the adopt/reconcile helm upgrade now prefers --reset-then-reuse-values when `helm upgrade --help` advertises it (Helm >= 3.14), falling back to --reuse-values otherwise — so NEW chart defaults reach adopted Windows edges on auto-upgrade (bash parity with install-client-helm.sh). manifest.sha256 regenerated. Tests: helm-unittest updated (3777); new Pester test asserts the reset-then-reuse preference; full Pester suite 452 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…bot) init-writable-data chowns the shared/logs hostPath dirs to GID 1000 then chmods 3777. With caps dropped to CHOWN+FOWNER only, the kernel silently strips S_ISGID on the chmod — after the chown the dir's group no longer matches the process (fsgid 0), and a root process without CAP_FSETID can't set setgid on it — so the mount landed at 1777 and new files did NOT inherit GID 1000 as documented. Add FSETID to the cap set; setgid now sticks. helm-unittest asserts FSETID present. Chart 1.9.18 -> 1.9.19. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 15e0f37. Configure here.
… (Bugbot) Two Bugbot findings on the writable-volume fix: - HIGH — remove `fsGroup: 1000` / `fsGroupChangePolicy` from jobs-manager. It is a no-op on hostPath (kubelet ignores fsGroup — the init does the work) and on CSI it only grants jobs-manager's OWN processes GID 1000 while its OnRootMismatch relabel flips the shared/logs volumes to group 1000 — stripping the group-0 access the spawned training pods (UID 1001 / OpenShift arbitrary UID, GID 0) and the host-UID ingestion pods rely on (docs/SECURITY.md §5.3). It never reaches those spawned writers, so it was all regression risk and no gain. Those pods keep their own documented posture; CSI is untouched (matches develop). - MEDIUM — the init now fixes each dir INDEPENDENTLY and best-effort: `for d in /data/shared /data/logs; do chown && chmod || echo <warn>; done`. A chown that can't complete (e.g. /data/shared on an NFS root_squash export) no longer aborts the chain and skips /data/logs — the other dir is still repaired and jobs-manager still starts; a truly unwritable mount surfaces as a clear error at the writer pod instead of wedging the edge in Init. helm-unittest updated (no fsGroup on either path; per-dir loop; CSI skips init). Chart 1.9.19 -> 1.9.20. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Aug 6, 2026
divyasinghds
added a commit
that referenced
this pull request
Aug 6, 2026
After the rebase merged our port-6550 Pester tests with #612, Test-Preflight's network-reachability block calls Has for kubectl/helm/k3d. The "port 6550 in use by our running cluster -> ok" case passes ownership, so it now continues into that block, where a k3d-only -ParameterFilter left those later Has calls with no matching mock ("No mock for command 'Has' matched"). Use a plain default mock (k3d + tools present -> only always-critical hosts probed) so every Has call is covered and the case does not throw. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

What & why
Hardens the Windows install → first-dataset-ingest → first-experiment path end to end, driven by real field failures on a Windows machine behind a filtering corporate/AV network. Follow-on to the resilient multi-transport download landed in #607/#608. Each fix below is a place a real user got stuck; together they take the flow from "dies at step 2" to "installs, provisions, ingests, and runs an experiment" on a hostPath (Windows/WSL2) install.
Closes #611. Contributes to #578.
Fixes
1. Checksum-driven tool download (k3d / kubectl / helm) —
install-k8s.ps1#607's multi-transport download validated by size + magic bytes only, so a binary truncated mid-transfer to between the size floor and the real size still passed and dead-ended at a no-retry checksum step. Now the checksum is the completeness test:
Get-VerifiedDownload -Sha256— after a transport lands a size/magic-valid file, its SHA-256 must match or the next transport (curl.exe → BITS) is tried. A truncated/altered copy self-heals.checksums.txt,*.sha256) are fetched first and gated on a hash-anchored-MatchPattern(a 64-hex hash adjacent to the asset / at the start of the body), so a proxy error page echoing the URL can't fail the gate open (Bugbot).System tool checksum verification failedon a correct-but-truncated k3d download.2. Reliable cluster-create —
install-k8s.ps1Wait-ProcessWithDeadlinepolledHasExitedbut never calledWaitForExit(), so withStart-Process -RedirectStandardOutputthe k3d process'sExitCodecame back null and$null -ne 0misread a successful create as a failure. Now it callsWaitForExit()before returning (reliable exit code for every caller), plus a defense-in-depth fallback that trusts k3d'screated successfullymarker — checking both stdout and stderr (k3d logs success via logrus to stderr) (Bugbot).Failed to create compute environmenteven though the cluster was up.3. Writable hostPath volumes for ingest and training/inference — chart (
jobs-manager-deployment.yaml)Two permission failures, same root cause. On hostPath installs kubelet doesn't apply
fsGroup(kubernetes/kubernetes#138411), so a hostPath dir is created root-owned and the non-root pods that mount it can't write:tb data ingestfailed at the copy step —mkdir: can't create directory '/data/shared/.tracebloc-staging/': Permission denied.PermissionError [Errno 13]: '/data/logs/<run>'while creating its per-run log dir (os.makedirs).mysql-dataalready had a privileged init-chown; the shared and logs volumes had none. Now, on hostPath only, jobs-manager runs a privilegedinit-writable-data(root, capsCHOWN+FOWNER+FSETID) that makes both/data/sharedand/data/logschmod 3777— world-writable (multiple non-root writers whose UIDs the chart doesn't control: jobs-manager, the spawned training/inference pods, the CLI ingest-staging pod) + setgid (GID-1000 inheritance;FSETIDis required or the kernel strips setgid after the chown) + sticky (one writer can't unlink another's files). The spawned pods mount the same hostPath dirs, so fixing them once reaches them too. Training and inference share one pod spec (client-runtimejob.yaml), so/data/logscovers both; every other write path there is anemptyDiror read-only. Each dir is fixed independently and best-effort, so a chown that can't complete (NFSroot_squash) doesn't wedge the edge. NofsGroupis set — it's a no-op on hostPath and would strip the spawned pods' group-0 access on CSI; CSI is untouched (matchesdevelop) and its pods keep their documented posture (docs/SECURITY.md §5.3).4. Windows local-chart support (
TRACEBLOC_CHART_PATH) —install-k8s.ps1The Windows installer could only install the published chart, so branch-only chart fixes (like #3 above, or #585's
global.imageRegistry) were untestable from a Windows install. AddedTRACEBLOC_CHART_PATH— parity with the bash installer's_resolve_chart_ref— so a local chart directory can be installed (skippinghelm repo add); the published repo remains the default.Tests
WaitForExit, both-stream fallback, local-chart ref). Full suite green (451).fsGroupon either path, and asserts the init has capsCHOWN+FOWNER+FSETID, runs the per-dir best-effort loop withchmod 3777, and mounts both/data/sharedand/data/logs(mounting both is required or the chown is a no-op). Chart bumped 1.9.15 → 1.9.20 across the chart-affecting fixes (chart-content guard).manifest.sha256regenerated. bats/macOS suites unaffected (Linux/macOS paths untouched except the already-merged Installer tool acquisition must survive a proxy/AV-truncated binary download (k3d/kubectl/helm) #607 shared-bash bits, which a full cross-OS audit confirmed safe).Scope note
This PR spans the installer scripts and the chart because they're the two halves of the same Windows install→ingest→experiment flow being validated together (kept in one PR by request).
The chart change is confined to the hostPath path: the privileged
init-writable-datacontainer is gated to hostPath and sets nofsGroup, so managed (EKS/AKS/OC) installs are genuinely untouched — CSI matchesdevelop, and its spawned pods keep their documented posture (docs/SECURITY.md §5.3). (An earlier revision setfsGroup: 1000on jobs-manager; @saadqbal and Bugbot correctly flagged that it never reaches the spawned writers and its relabel would strip their group-0 access on CSI, so it was removed.)CI note
If
pii-gate / pii-checkis red, it is failing closed because the orgPII_DENYLISTsecret is unavailable — an org-wide infra issue affecting every PR, not this PR's content (tracked separately; thepii-gate-overridelabel doesn't currently bypass it — tracebloc/.github#165). All other checks and Cursor Bugbot are green.🤖 Generated with Claude Code