From 8e84527ba2b6ef49b4f4ceef7dfe74ca32c53171 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Thu, 14 May 2026 20:04:25 +0000 Subject: [PATCH] =?UTF-8?q?fix(deploy):=20Hotfix=20#16=20=E2=80=94=20split?= =?UTF-8?q?=20unixOwnerFromStat=20per-OS=20build=20tags=20(closes=20Window?= =?UTF-8?q?s=20CI=20matrix)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's cross-platform-build (windows-latest) job has been red for several runs: internal/deploy/ownership.go:205 — undefined: syscall.Stat_t Root cause: `syscall.Stat_t` is the Unix-specific POSIX stat-struct shape (linux / darwin / freebsd / openbsd / netbsd / dragonfly / solaris all expose it). On Windows GOOS, the syscall package defines `syscall.Win32FileAttributeData` instead, which carries no uid/gid fields. Any production tsx that names `syscall.Stat_t` unconditionally fails to compile on GOOS=windows. The function was added pre-cross-platform-matrix and never had to compile for Windows; CI's `cross-platform-build` job (added by Phase 3 TEST-H2) is what surfaced it. The ubuntu / macos matrix runs stayed green because both GOOSes expose the type. Fix (standard Go per-platform build-tag split): Move `unixOwnerFromStat(fi os.FileInfo) (uid, gid int, ok bool)` out of ownership.go into per-OS sibling files: internal/deploy/ownership_unix.go //go:build unix internal/deploy/ownership_windows.go //go:build windows ownership_unix.go: same impl as before. Uses `syscall.Stat_t`. Covers every Unix-y GOOS via Go 1.19+'s `unix` build constraint (linux + darwin + freebsd + openbsd + netbsd + dragonfly + solaris). ownership_windows.go: stub that returns (-1, -1, false). Windows has no native uid/gid; file ownership is expressed via SIDs + ACLs (`syscall.Win32FileAttributeData`), which the deploy package's call sites can't translate into uid/gid anyway. All four callers — applyOwnership (ownership.go:75), preserveSourceOwner (atomic.go:237), and two test sites — ALREADY handle ok=false by falling back to Plan.Defaults / runtime umask. Stub returning false is the correct platform contract. ownership.go: drop the `syscall` import (no longer needed there) + replace the function body with a doc comment pointing to the per-OS files so future readers know where the impl lives. Note: the agent binary still compiles + runs on Windows; the chown/chmod codepaths in the deploy package gate on `runningAsRoot()` (os.Geteuid() == 0) which is also Unix-only in practice — Windows agents run as a service under a SID that doesn't translate to a uid anyway, so ownership operations on Windows naturally no-op. Verification (Go toolchain wired in sandbox, sub-platform builds ran locally): • gofmt -l on all three touched files — clean • GOOS=linux GOARCH=amd64 go build ./internal/deploy/... — exit 0 • GOOS=darwin GOARCH=amd64 go build ./internal/deploy/... — exit 0 • GOOS=windows GOARCH=amd64 go build ./internal/deploy/... — exit 0 • GOOS=windows GOARCH=amd64 go build ./cmd/{server,agent,cli,mcp-server}/... — exit 0 (all four CI matrix targets) • go vet ./internal/deploy/... — exit 0 • staticcheck ./internal/deploy/... — zero findings • go test -short -count=1 ./internal/deploy/... — ok 0.216s (the four callers' tests all still pass on Linux) Ground-truth: origin/master tip 622c19c (TEST-H3 just pushed) verified via GitHub API BEFORE commit. Falsifiable proof for the next CI run: the windows-latest leg of cross-platform-build should turn green. The ubuntu-latest and macos-latest legs were already green; this fix doesn't touch their build path. --- internal/deploy/ownership.go | 20 ++++++++-------- internal/deploy/ownership_unix.go | 33 ++++++++++++++++++++++++++ internal/deploy/ownership_windows.go | 35 ++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 internal/deploy/ownership_unix.go create mode 100644 internal/deploy/ownership_windows.go diff --git a/internal/deploy/ownership.go b/internal/deploy/ownership.go index f54e218..ce55293 100644 --- a/internal/deploy/ownership.go +++ b/internal/deploy/ownership.go @@ -9,7 +9,6 @@ import ( "os" "os/user" "strconv" - "syscall" ) // runningAsRoot reports whether the current process has uid 0. @@ -198,12 +197,13 @@ func lookupGID(groupname string) (int, error) { // unixOwnerFromStat extracts (uid, gid) from a Unix-style FileInfo. // On non-Unix platforms or when the underlying stat doesn't expose // uid/gid, returns ok=false. -func unixOwnerFromStat(fi os.FileInfo) (uid int, gid int, ok bool) { - if fi == nil { - return -1, -1, false - } - if sysStat, isUnix := fi.Sys().(*syscall.Stat_t); isUnix { - return int(sysStat.Uid), int(sysStat.Gid), true - } - return -1, -1, false -} +// +// Platform-specific implementations live in: +// - ownership_unix.go (//go:build unix — uses *syscall.Stat_t) +// - ownership_windows.go (//go:build windows — stub returns false) +// +// The split exists because syscall.Stat_t is Unix-only — Windows +// has no equivalent shape, so any production tsx that names it +// fails to compile on GOOS=windows. The cross-platform-build CI +// matrix caught this at Hotfix #16; the function was originally +// in this file pre-split. diff --git a/internal/deploy/ownership_unix.go b/internal/deploy/ownership_unix.go new file mode 100644 index 0000000..e4eb210 --- /dev/null +++ b/internal/deploy/ownership_unix.go @@ -0,0 +1,33 @@ +// Copyright 2026 certctl LLC. All rights reserved. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build unix + +// Unix-side implementation of unixOwnerFromStat. The `unix` build +// constraint (Go 1.19+) covers linux / darwin / freebsd / openbsd / +// netbsd / dragonfly / solaris — every GOOS where *syscall.Stat_t +// is a valid type assertion target for os.FileInfo.Sys(). +// +// Hotfix #16 (2026-05-14): pre-split, this function lived inline in +// ownership.go with an unconditional `syscall.Stat_t` reference. That +// failed `GOOS=windows go build` because the type is undefined on +// that platform. The split is the standard Go pattern — the same +// function name + signature is satisfied by either build of the +// package, callers don't know or care which. + +package deploy + +import ( + "os" + "syscall" +) + +func unixOwnerFromStat(fi os.FileInfo) (uid int, gid int, ok bool) { + if fi == nil { + return -1, -1, false + } + if sysStat, isUnix := fi.Sys().(*syscall.Stat_t); isUnix { + return int(sysStat.Uid), int(sysStat.Gid), true + } + return -1, -1, false +} diff --git a/internal/deploy/ownership_windows.go b/internal/deploy/ownership_windows.go new file mode 100644 index 0000000..de98777 --- /dev/null +++ b/internal/deploy/ownership_windows.go @@ -0,0 +1,35 @@ +// Copyright 2026 certctl LLC. All rights reserved. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build windows + +// Windows stub for unixOwnerFromStat. Windows has no uid/gid concept +// the way Unix does — file ownership is expressed via SIDs (Security +// Identifiers) and ACLs (Access Control Lists), and os.FileInfo.Sys() +// returns *syscall.Win32FileAttributeData which carries no +// ownership data the deploy package's existing call sites can use. +// +// All four callers — applyOwnership at ownership.go:75, +// preserveSourceOwner at atomic.go:237, and two test sites — already +// handle the ok=false return path by falling back to Plan.Defaults +// or the runtime's umask. Returning false here is the correct +// platform contract: "no native ownership available on this +// platform; use the supplied defaults." +// +// Hotfix #16 (2026-05-14): created to unblock the +// cross-platform-build Windows matrix in CI, which had been +// red since the agent's deploy package gained ownership- +// preservation semantics. The agent binary still compiles for +// Windows; ownership operations on Windows are no-ops (which +// matches operator expectations — the certctl-agent's +// chown/chmod codepaths gate on `runningAsRoot()` and Windows +// runs the agent as a service under a SID that doesn't +// translate to a uid anyway). + +package deploy + +import "os" + +func unixOwnerFromStat(_ os.FileInfo) (uid int, gid int, ok bool) { + return -1, -1, false +}