From f8179be0bf7a3c900c421bd5f0dd9b16e439907d Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 18:02:01 +0200 Subject: [PATCH 1/3] ci: run build and race tests on Windows as well as Linux Convert the single ubuntu-latest test job into a matrix over ubuntu-latest and windows-latest with fail-fast disabled, so a failure on one OS does not mask the other's result. GitHub's Windows runner images do not ship GNU make, so the Windows leg runs the equivalent Go commands directly (go build -trimpath -o grant.exe . and go test -race ./... -v) while Linux keeps the make targets. The Windows build drops -ldflags: they only inject version strings that are already exercised on Linux, and reimplementing the Makefile's git rev-parse/date shell-outs in PowerShell would buy nothing. -race is kept on Windows rather than degraded to plain go test: the runner image ships gcc and setup-go leaves CGO_ENABLED=1, so the race detector genuinely runs there. golangci-lint stays Linux-only. internal/config tests asserting POSIX permission bits and HOME-based home-directory resolution are skipped on Windows; neither behaves that way there. MERGE ORDER: fix/ui-test-data-race must merge before this branch. The internal/ui tests swap the package-level IsTerminalFunc while marked t.Parallel(), which is a real data race; go test -race ./... fails on it today on Linux and will fail on both matrix legs until that fix lands. The race fix is deliberately not carried here. --- .github/workflows/ci.yml | 18 +++++++++++++++++- CHANGELOG.md | 5 +++++ CLAUDE.md | 7 +++++++ internal/config/config_test.go | 7 +++++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd1039e..955fd01 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,11 @@ permissions: jobs: test: - runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - name: Checkout uses: actions/checkout@v4 @@ -22,12 +26,24 @@ jobs: go-version: "1.25" - name: Build + if: runner.os != 'Windows' run: make build - name: Test with race detector + if: runner.os != 'Windows' run: make test-race + # Windows runners have no GNU make, so run the equivalent go commands directly. + - name: Build (Windows) + if: runner.os == 'Windows' + run: go build -trimpath -o grant.exe . + + - name: Test with race detector (Windows) + if: runner.os == 'Windows' + run: go test -race ./... -v + - name: Lint + if: runner.os != 'Windows' uses: golangci/golangci-lint-action@v6 with: version: latest diff --git a/CHANGELOG.md b/CHANGELOG.md index af6023f..62e1bd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ All notable changes to this project will be documented in this file. ### Changed - Rebranded user-facing references from CyberArk to Idira (formerly CyberArk) in the README and CLI help/prompt text, following the Palo Alto Networks rebrand. No functional or API changes; SDK import paths (`github.com/cyberark/idsec-sdk-golang`), `*.cyberark.cloud` URLs, and environment variables are unchanged. +- CI now runs the build and race-enabled test suite on both `ubuntu-latest` and `windows-latest` (`fail-fast: false`). The Windows leg invokes `go build` / `go test` directly because GitHub's Windows runners have no GNU make; lint still runs on Linux only. + +### Fixed + +- Skipped two Unix-only assertions in `internal/config` tests (POSIX permission bits, `HOME`) on Windows. ## [0.7.0] - 2026-04-21 diff --git a/CLAUDE.md b/CLAUDE.md index c0d02d8..df379f0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,6 +156,13 @@ make clean # Clean build artifacts - `-trimpath` used in both `Makefile` and `.goreleaser.yaml` for reproducible builds - `.goreleaser.yaml` uses `CommitDate` (not build date) and `mod_timestamp` for reproducibility +## CI +- `.github/workflows/ci.yml` — `test` job runs as a matrix over `ubuntu-latest` and `windows-latest` with `fail-fast: false` +- Windows runners have no GNU make, so that leg runs the equivalent Go commands directly (`go build -trimpath -o grant.exe .`, `go test -race ./... -v`); Linux keeps `make build` / `make test-race`. Keep the two legs in sync when Makefile targets change +- `go test -race` works on windows/amd64 because the runner image ships gcc (the race detector needs cgo) +- Lint (`golangci-lint-action`) runs on Linux only — a second pass on Windows adds minutes and finds nothing new +- Tests must be OS-portable. Unix-only behaviour (POSIX file permissions, `HOME`) is skipped via `runtime.GOOS == "windows"` in `internal/config/config_test.go` + ## Release Process 1. Move `[Unreleased]` entries in `CHANGELOG.md` to a new `[X.Y.Z] - YYYY-MM-DD` section (leave `[Unreleased]` header empty) 2. Commit: `docs: prepare CHANGELOG for vX.Y.Z release` diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ca5c11d..bb2beba 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -176,6 +177,9 @@ func TestDefaultConfig(t *testing.T) { } func TestLoadConfig_PermissionError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("chmod 0000 does not make a file unreadable on Windows") + } dir := t.TempDir() path := filepath.Join(dir, "config.yaml") @@ -209,6 +213,9 @@ func TestConfigPath_Override(t *testing.T) { } func TestConfigDir_Error(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("os.UserHomeDir uses USERPROFILE on Windows, not HOME") + } // Override HOME to empty to force error t.Setenv("HOME", "") t.Setenv("XDG_CONFIG_HOME", "") From ad9500db826dc6432785cf071889e4eb09f4e0e6 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 21:51:26 +0200 Subject: [PATCH 2/3] test: make cmd and cache tests pass on Windows The first windows-latest CI run on this branch failed three packages. Two are portability bugs in the tests themselves; the third (internal/ui) is the pre-existing IsTerminalFunc data race fixed by #52 and is left alone here. cmd: TestConfigureCommand/config_save_error forced a write failure by pointing GRANT_CONFIG at /dev/null/config.yaml, relying on /dev/null being a file so config.Save's MkdirAll fails with ENOTDIR. On Windows that path is an ordinary writable location, MkdirAll happily creates D:\dev\null, the save succeeds and the test sees no error. Replaced with a temp regular file used as the parent component, which fails on both POSIX (ENOTDIR) and Windows (ERROR_DIRECTORY). This keeps the case running everywhere rather than skipping it on Windows. internal/cache: TestSet_FilePermissions asserts the cache file is 0600. Go reports ordinary writable Windows files as 0666 (os/types_windows.go) and os.Chmod there only toggles the read-only attribute, so the assertion is meaningless rather than wrong. Skipped on Windows; the 0600 assertion keeps its full strength on Linux and macOS. No shared skip helper: after this change only two tests skip for permission reasons (this one and TestLoadConfig_PermissionError), and an inline t.Skip with its own reason reads better than an indirection for two callers. A sweep of every _test.go for mode literals, Chmod, and Mode().Perm() turned up no other permission assertions. --- CHANGELOG.md | 2 +- CLAUDE.md | 3 ++- cmd/configure_test.go | 12 ++++++++++-- internal/cache/cache_test.go | 4 ++++ 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62e1bd6..8619f24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ All notable changes to this project will be documented in this file. ### Fixed -- Skipped two Unix-only assertions in `internal/config` tests (POSIX permission bits, `HOME`) on Windows. +- Made the test suite pass on Windows: skipped Unix-only assertions in `internal/config` (POSIX permission bits, `HOME`) and `internal/cache` (`0600` cache-file mode, which Go reports as `0666` on Windows), and reworked `cmd`'s `config save error` case to block the write with a path under an existing regular file instead of a hardcoded `/dev/null` path. ## [0.7.0] - 2026-04-21 diff --git a/CLAUDE.md b/CLAUDE.md index df379f0..c633834 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -161,7 +161,8 @@ make clean # Clean build artifacts - Windows runners have no GNU make, so that leg runs the equivalent Go commands directly (`go build -trimpath -o grant.exe .`, `go test -race ./... -v`); Linux keeps `make build` / `make test-race`. Keep the two legs in sync when Makefile targets change - `go test -race` works on windows/amd64 because the runner image ships gcc (the race detector needs cgo) - Lint (`golangci-lint-action`) runs on Linux only — a second pass on Windows adds minutes and finds nothing new -- Tests must be OS-portable. Unix-only behaviour (POSIX file permissions, `HOME`) is skipped via `runtime.GOOS == "windows"` in `internal/config/config_test.go` +- Tests must be OS-portable. Never assert POSIX permission bits without a `runtime.GOOS == "windows"` skip: Go synthesizes `0666`/`0777` for Windows files and `os.Chmod` there only toggles the read-only attribute. Current skips: `internal/config/config_test.go` (`TestLoadConfig_PermissionError`, `TestConfigDir_Error` — chmod 0000 and `HOME`) and `internal/cache/cache_test.go` (`TestSet_FilePermissions`) +- Prefer a portable construction over a skip where one exists. To force a write failure, point at a path whose parent component is an existing regular file (`MkdirAll` fails with ENOTDIR on POSIX and ERROR_DIRECTORY on Windows) rather than a hardcoded `/dev/null/...` path, which is an ordinary writable location on Windows ## Release Process 1. Move `[Unreleased]` entries in `CHANGELOG.md` to a new `[X.Y.Z] - YYYY-MM-DD` section (leave `[Unreleased]` header empty) diff --git a/cmd/configure_test.go b/cmd/configure_test.go index c73f7e8..dd3f1b6 100644 --- a/cmd/configure_test.go +++ b/cmd/configure_test.go @@ -181,8 +181,16 @@ func TestConfigureCommand(t *testing.T) { } }, setupConfigFn: func() (string, error) { - // Return a path in a non-existent directory with restricted permissions - return "/dev/null/config.yaml", nil + // Return a path whose parent directory component is an existing + // regular file, so config.Save's MkdirAll fails. This is portable: + // creating a directory under a file errors on POSIX (ENOTDIR) and + // on Windows (ERROR_DIRECTORY). A hardcoded /dev/null path does not + // work on Windows, where it is just an ordinary writable location. + blocker := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(blocker, []byte("x"), 0o600); err != nil { + return "", err + } + return filepath.Join(blocker, "config.yaml"), nil }, wantContain: []string{ "failed to save config", diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index 6866bef..8e3b243 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -3,6 +3,7 @@ package cache import ( "os" "path/filepath" + "runtime" "testing" "time" ) @@ -92,6 +93,9 @@ func TestSet_CreatesDirectory(t *testing.T) { func TestSet_FilePermissions(t *testing.T) { t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("Go synthesizes 0666/0444 for Windows files; POSIX permission bits are not modeled there") + } dir := t.TempDir() s := NewStore(dir, 4*time.Hour) From af1c135d4369d6301685a0cd4be5c3ee9615c31e Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 22:13:28 +0200 Subject: [PATCH 3/3] test: make internal/selfupdate tests pass on Windows Two Windows-only failures surfaced by the new windows-latest CI leg: - unwritable target directory: chmod 0500 does not deny writes to a directory on Windows, so applyWithOptions succeeded where the test expected a failure. Skip the case there. - rollback failure hint: recoveryHint renders paths with %q, which escapes the backslashes in a Windows path, so the raw-path strings.Contains assertion never matched. Compare against strconv.Quote of each path instead. --- CHANGELOG.md | 1 + internal/selfupdate/apply_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4323867..8fce34e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ All notable changes to this project will be documented in this file. - `grant configure` no longer reports a stale SDK profile location. The help text and the "Profile saved to" success message printed `~/.idsec_profiles/grant`, which has been wrong since the SDK upgrade in v0.7.0. The path is now resolved with the SDK's own `profiles.GetProfilesFolder()`, so it always matches the directory the profile loader reads (`~/.idsec/profiles/grant` by default, or `IDSEC_PROFILES_FOLDER` when set). - Made the test suite pass on Windows: skipped Unix-only assertions in `internal/config` (POSIX permission bits, `HOME`) and `internal/cache` (`0600` cache-file mode, which Go reports as `0666` on Windows), and reworked `cmd`'s `config save error` case to block the write with a path under an existing regular file instead of a hardcoded `/dev/null` path. +- Made `internal/selfupdate` pass on Windows: the `unwritable target directory` case is skipped (a `0500` chmod does not deny directory writes there), and the recovery-hint assertion now compares against the `%q`-quoted path, which escapes the backslashes in a Windows path. ## [0.7.0] - 2026-04-21 diff --git a/internal/selfupdate/apply_test.go b/internal/selfupdate/apply_test.go index c3af7b2..30083e2 100644 --- a/internal/selfupdate/apply_test.go +++ b/internal/selfupdate/apply_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "testing" @@ -127,6 +128,9 @@ func TestApplyBinaryFailsBeforeTouchingTarget(t *testing.T) { { name: "unwritable target directory", setup: func(t *testing.T, target string) { + if runtime.GOOS == "windows" { + t.Skip("chmod 0500 does not deny directory writes on Windows") + } if os.Geteuid() == 0 { t.Skip("running as root: directory permissions are not enforced") } @@ -246,7 +250,9 @@ func TestApplyBinaryRollbackFailureIsReported(t *testing.T) { if !interrupted { t.Fatal("InterruptedUpdate did not detect the interrupted state") } - if !strings.Contains(hint, oldPathFor(path)) || !strings.Contains(hint, path) { + // recoveryHint renders the paths with %q, so compare against the quoted + // form - on Windows the raw path contains backslashes that %q escapes. + if !strings.Contains(hint, strconv.Quote(oldPathFor(path))) || !strings.Contains(hint, strconv.Quote(path)) { t.Errorf("recovery hint does not name both paths: %q", hint) }