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 3f2dbe8..8fce34e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ 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. - Upgraded dependencies: - `github.com/cyberark/idsec-sdk-golang` v0.2.3 → v0.8.1 - `github.com/spf13/cobra` v1.9.1 → v1.10.2 @@ -44,6 +45,8 @@ All notable changes to this project will be documented in this file. ### Fixed - `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/CLAUDE.md b/CLAUDE.md index 150be8e..5079d81 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -181,6 +181,14 @@ 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. 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) 2. Commit: `docs: prepare CHANGELOG for vX.Y.Z release` diff --git a/cmd/configure_test.go b/cmd/configure_test.go index 1da67e9..aa60438 100644 --- a/cmd/configure_test.go +++ b/cmd/configure_test.go @@ -182,8 +182,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) 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", "") 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) }