mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 12:21:31 +00:00
fix(deploy): Hotfix #16 — split unixOwnerFromStat per-OS build tags (closes Windows CI matrix)
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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
Reference in New Issue
Block a user