diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88d93577..2fc0b5c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -182,8 +182,9 @@ jobs: VER=2.14.0 TARBALL="golangci-lint-${VER}-linux-amd64.tar.gz" BASE="https://github.com/golangci/golangci-lint/releases/download/v${VER}" - curl -sSfL "${BASE}/${TARBALL}" -o "${TARBALL}" - curl -sSfL "${BASE}/golangci-lint-${VER}-checksums.txt" -o checksums.txt + # --proto '=https' keeps a redirect from downgrading to plain HTTP. + curl -sSfL --proto '=https' --tlsv1.2 "${BASE}/${TARBALL}" -o "${TARBALL}" + curl -sSfL --proto '=https' --tlsv1.2 "${BASE}/golangci-lint-${VER}-checksums.txt" -o checksums.txt # sha256sum -c matches by the filename recorded in checksums.txt, so the # tarball must be on disk under its canonical name. grep " ${TARBALL}\$" checksums.txt | sha256sum -c - diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml new file mode 100644 index 00000000..1889b3ef --- /dev/null +++ b/.github/workflows/sonar.yml @@ -0,0 +1,103 @@ +name: sonar + +# SonarQube Cloud analysis. NETWORK-ALLOWED (the scanner talks to sonarcloud.io), +# so it lives in its own workflow and is never a dependency of the offline jobs +# in ci.yml — the same boundary supply-chain.yml and functional.yml keep. The +# tests that feed it still run offline (-mod=vendor, GOPROXY=off); only the +# upload needs the network. +# +# Setup (once, by a maintainer — see AGENTS.md "SonarQube Cloud"): import the +# repository on sonarcloud.io, turn its Automatic Analysis OFF (it conflicts with +# a CI-based scan), set the new-code definition to "Previous version", and store +# the analysis token as the repository secret SONAR_TOKEN. Until the secret +# exists the scan job is skipped, not failed. + +# Pushes to claude/** are scanned too, like ci.yml: an agent reads the result of +# its push before moving on (AGENTS.md "Where checks run"), and it is how a +# change to this workflow is proved before it reaches master, where +# workflow_dispatch first becomes available. +on: + push: + branches: + - master + - "claude/**" + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: sonar-${{ github.ref }} + cancel-in-progress: true + +jobs: + # A job-level `if` cannot read secrets, so this job turns "is SONAR_TOKEN + # set?" into an output. It is empty on fork and Dependabot PRs (GitHub does + # not hand them repository secrets) and before the secret is configured; + # either way the scan is skipped instead of going red on every push. + token: + runs-on: ubuntu-latest + outputs: + present: ${{ steps.check.outputs.present }} + steps: + - id: check + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + run: | + if [ -n "$SONAR_TOKEN" ]; then + echo "present=true" >> "$GITHUB_OUTPUT" + else + echo "::notice::SONAR_TOKEN is not available to this run; skipping the SonarQube Cloud scan." + echo "present=false" >> "$GITHUB_OUTPUT" + fi + + scan: + needs: token + if: needs.token.outputs.present == 'true' + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + # Full history and tags: the scanner needs blame for new-code detection on + # a pull request, and `git describe` below needs the latest v* tag. A + # shallow clone silently loses both. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + + # Same toolchain as ci.yml, for the same reasons (see the comment there). + - name: Set up Go + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: "1.27.x" + check-latest: true + cache: false + + # Generated here, against this exact checkout, so it can never be stale — + # the failure that read 25% new-code coverage against an 83% tree. + - name: Coverage profile (offline, from vendor/) + env: + GOFLAGS: -mod=vendor + GOPROXY: "off" + CGO_ENABLED: "0" + run: make cover + + # The new-code period is "Previous version", so the version string is what + # makes "new code" mean "since the last release". An empty one makes every + # scan analyse the same version and freezes the baseline, so fail loudly. + - name: Resolve the release the new-code period starts from + id: version + run: | + v=$(git describe --tags --abbrev=0 --match 'v*') || { + echo "::error::no v* tag is reachable from HEAD; refusing to scan without sonar.projectVersion" + exit 1 + } + echo "version=$v" >> "$GITHUB_OUTPUT" + + - name: SonarQube Cloud scan + uses: SonarSource/sonarqube-scan-action@d209202bc7d53ff1cc128f7f907dac145c9d6ae9 # v8.3.0 + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + with: + args: > + -Dsonar.projectVersion=${{ steps.version.outputs.version }} diff --git a/AGENTS.md b/AGENTS.md index 6c1782d9..5bcdf8c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,8 +92,9 @@ nil pointer meaning "leave this field unset". A sentinel error would make every call site worse. The reasoning is recorded in `.golangci.yml` next to the disabled entry; re-check it there before enabling. -Gate before committing: `gofmt` clean, `go vet` clean, `golangci-lint` **0 -issues**, `go test ./...` green, the offline static build succeeds, and — if the +Gate before a change is done (which parts an agent runs itself and which it +leaves to GitHub is in "Where checks run (agent sessions)"): `gofmt` clean, +`go vet` clean, `golangci-lint` **0 issues**, `go test ./...` green, the offline static build succeeds, and — if the commit changes the command surface — `docs/coverage.md` is updated (see "Coverage tracking") and every new leaf has a functional test (`go test -tags functional ./test/functional/` runs `TestEveryLeafIsCovered`, offline; see "Testing"). @@ -111,67 +112,110 @@ the golangci-lint pin ever moves to a build made with a newer Go, re-run can also outrun the pin: v2.13.2 could not read go1.27.2's export data ("export data version 5"), and v2.14.0 can — move the pin when that happens. -## SonarQube (out-of-band static analysis) +## Where checks run (agent sessions) -SonarQube analysis is **not** part of the per-commit gate above and not part of -CI — it needs network and a server token, so it lives outside the air-gap -invariant on purpose (see "Releases & CI" for why that boundary is load-bearing). -It is run manually, typically after a merge to `master` has already gone green -offline. `sonar-project.properties` is committed; the server address and the -token are not. +An agent session — a cloud container or a laptop — runs the **cheap, fast** +checks for the code it touched; GitHub runs the **whole matrix** on every push +to `claude/**`. Measured in a 4-core cloud container on 2026-10-21: -Point the scanner at the internal SonarQube server through the environment — the -host name and token are private data and must never be committed (see "Private -data never leaves the org"): - -```sh -export SONAR_HOST_URL=https:// # ask a maintainer / LCM -export SONAR_TOKEN= # sqp_… ; never commit -``` - -Then scan with **`make sonar`**, from the repo root: - -```sh -make sonar -``` - -**Use the target, not the two steps by hand.** It refreshes `coverage.out` and -passes `sonar.projectVersion`, and both were forgotten by hand on every scan -between 2026-09-01 and 2026-09-22 — neither failure is loud: - -- A **stale `coverage.out`** does not error. The scanner uploads it happily, and - every line written since the profile was generated reports as uncovered. The - 2026-09-22 scan read 25% new-code coverage against a profile from 2026-08-31; - the tree's actual figure for the same period was 83%. -- A **missing `projectVersion`** makes every scan analyse the same version, so - `PREVIOUS_VERSION` never advances. The baseline froze at v0.26.0 across seven - releases, "new code" grew to 24.5k lines, and long-standing findings kept - counting as new. - -`make sonar` needs network, podman and the two environment variables; it is the -one target outside the offline gate. The image is `linux/amd64`, so on Apple -silicon podman emulates it and prints a platform warning that is expected and -harmless. Prefer `podman run --env-file ` over the -target's `-e` if you would rather the token stay out of the process list. +| Check | Where | Cost | +| --- | --- | --- | +| `go vet ./...` | session | ~5 s | +| `go test .//...` for the packages you changed | session | ~1 s each | +| `go test ./...` (full unit suite) | session, before pushing a multi-package change | ~45 s, ~40 lines of output | +| `go test -tags functional ./test/functional/` (no cloud; includes `TestEveryLeafIsCovered`) | session, when the command surface changed | seconds | +| `gofmt` (Go **1.27**, see below) | session, on the files you changed | seconds | +| `make lint` (golangci-lint **v2.14.0**) | GitHub `ci.yml` → `lint` | — | +| `make race`, `make crossbuild` | GitHub `ci.yml` | — | +| `make cover` + SonarQube Cloud | GitHub `sonar.yml` | ~1–5 min; never run it by hand | +| vendor integrity, govulncheck, importer, api-compat | GitHub `supply-chain.yml` | — | +| devstack functional run | GitHub `functional.yml` (nightly) | 75 min | + +Rules that keep this cheap and correct: + +- **Filter test output.** `go test ./... 2>&1 | grep -v '^ok'` prints only + failures; a verbose `-v` run of the whole tree is thousands of lines that + nobody reads. Use `-run` to rerun one failing test. +- **Format with Go 1.27's gofmt even when the session has an older Go.** The + cloud image ships an older toolchain, and its gofmt reverts 1.27's formatting + (see "Build / test / lint"). The go command fetches the right one through the + module proxy and caches it: + `"$(GOTOOLCHAIN=go1.27.2 go env GOROOT)/bin/gofmt" -l ` (`-w` to fix). + Never run `make fmt` on an older toolchain. +- **Do not trust a local golangci-lint that is not v2.14.0.** The cloud image + carries an older one, which enables different checks; a clean run there proves + nothing. Push and read the `lint` job instead. +- **After pushing, read the result.** Check the runs for the pushed commit on + GitHub (`ci`, `supply-chain`, `sonar`) and fix a red job before moving on — + the push is not done until they are green. + +## SonarQube Cloud (out-of-band static analysis) + +SonarQube Cloud (sonarcloud.io) analyses the project from +**`.github/workflows/sonar.yml`**, on every push to `master` and `claude/**`, on +every pull request from this repository, and on `workflow_dispatch`. Nobody runs a scan by +hand any more. It is **not** part of the per-commit offline gate: it needs +network and a token, so it lives in its own network-allowed workflow, outside +the air-gap invariant on purpose (see "Releases & CI" for why that boundary is +load-bearing). The tests that produce its coverage profile still run offline. +The code is public, so analysing it on an external service exposes nothing new; +the token is the only secret. + +One-time setup, by a maintainer: + +1. Import the repository on sonarcloud.io. If it assigns a key or organization + other than `ftarasenko_go-openstackclient` / `ftarasenko`, change + `sonar-project.properties` to match. +2. Turn **Automatic Analysis off** for the project (Administration → Analysis + Method). It conflicts with a CI-based scan, and it cannot import coverage. +3. Set the **new-code definition to "Previous version"**. +4. Store the project's analysis token as the repository secret **`SONAR_TOKEN`**. + +Until the secret exists, and on pull requests from forks or Dependabot (GitHub +does not give those runs repository secrets), the `scan` job is skipped with a +notice rather than failed. + +The workflow exists because the manual scan failed silently, twice over, on every +scan between 2026-09-01 and 2026-09-22. It now does both steps every time: + +- It **generates `coverage.out` on the checkout it scans** (`make cover`). A + stale profile does not error. The scanner uploads it, and every line written + since the profile was generated reports as uncovered. The 2026-09-22 scan read + 25% new-code coverage against a profile from 2026-08-31; the tree's actual + figure for the same period was 83%. +- It **passes `sonar.projectVersion`** from `git describe --tags --abbrev=0 + --match 'v*'`, and fails the job if no tag is reachable. A missing version + makes every scan analyse the same version, so `PREVIOUS_VERSION` never + advances. The baseline froze at v0.26.0 across seven releases, "new code" grew + to 24.5k lines, and long-standing findings kept counting as new. The checkout + is full-depth (`fetch-depth: 0`) for the same reason. A shallow clone has no + tags and no blame, and the scan degrades without saying so. **Coverage is generated by `make cover`, not a bare `go test -coverprofile`.** That target passes `-coverpkg=./...`, and the flag is load-bearing here: by default a test binary instruments only its own package, but `internal/cli`'s root tests build the *entire* command tree via `NewRootCommand`, so every service's flag registration already executes and would otherwise not be attributed to it. Measuring without -`-coverpkg` under-reports the project by roughly 11 points (54% vs 65%) — the -tests are the same either way. +`-coverpkg` under-reported the project by roughly 11 points (54% vs 65% when +measured). The figure with it was 76.3% on 2026-10-21. The tests are the same +either way. Two consequences of the flag that the target handles: + +- Every test binary reports every block, so the raw profile repeats each block + once per package (about 36× and 800k lines). The target merges the repeats, + keeping the highest count, as `go tool cover` does. The total is unchanged, and + the upload is about 23k lines. +- A package with no test file is reported through `go tool covdata`. Toolchains + fetched by `GOTOOLCHAIN` do not ship that tool, so a single test-less package + failed the whole run with `no such tool "covdata"`. `cmd/koc` has a test for + that reason. Keep every package that holds Go code tested. `/coverage.*` is gitignored (root-anchored, so `docs/coverage.md` survives), and -`coverage.out` is in `sonar.exclusions` so the profile is not analysed as source. - -There is no scanner binary to install — the target runs the container image, -which is also what keeps the Go plugin version matched to the server. +`coverage.*` is in `sonar.exclusions` so the profile is not analysed as source. **Why `sonar.projectVersion` matters.** The project's new-code period is `PREVIOUS_VERSION`, so the version string is what makes "new code" mean *since -the last release* instead of *since the previous scan* — otherwise a mechanical -refactor re-attributes long-standing untested lines to new code and the gate +the last release* instead of *since the previous scan*. Otherwise a mechanical +refactor re-attributes long-standing untested lines to new code, and the gate fails on work nobody just wrote. The same note sits next to the setting in `sonar-project.properties`. @@ -187,7 +231,7 @@ Two known, accepted divergences — neither is a regression to chase: through, so it wins; do not split a function only to satisfy S3776, and do not re-enable the rule without raising its threshold to 25 on a custom profile first. -- **First scan of a fresh server.** With no prior analysis there is no previous +- **First scan of a fresh project.** With no prior analysis there is no previous version to diff against, so the whole tree lands in "new code" and the gate can fail on pre-existing findings. Re-check against the second scan before treating anything as new. @@ -591,7 +635,7 @@ Every commit follows [Conventional Commits 1.0.0](https://www.conventionalcommit The checks are split along the network boundary, and the split is load-bearing: everything in `ci.yml` is **offline** (`-mod=vendor`, `GOPROXY=off`), everything that needs network lives in its own workflow -(`supply-chain.yml`, `functional.yml`), and **no network-allowed job is ever a +(`supply-chain.yml`, `functional.yml`, `sonar.yml`), and **no network-allowed job is ever a dependency of an offline job**. That is what keeps the air-gap invariant from being quietly weakened by a check that happens to need a proxy. @@ -659,6 +703,13 @@ being quietly weakened by a check that happens to need a proxy. journal, `functional.env` and `openrc.env` are excluded at upload. A red cell with a failed "Stack devstack" step is devstack or the runner image; a red "Smoke" step with a `koc …` FAIL line, or a red "Functional tests" step, is koc. +- `.github/workflows/sonar.yml` — the **SonarQube Cloud scan**, network-allowed: + push to `master`/`claude/**`, pull requests, `workflow_dispatch`. A `token` job turns + "is `SONAR_TOKEN` available?" into an output (a job-level `if` cannot read + secrets), so fork/Dependabot PRs and an unconfigured repository skip the + `scan` job instead of failing it. `scan` checks out full depth, runs `make + cover` offline, and passes `sonar.projectVersion` from the latest `v*` tag. It + refuses to scan without one. See "SonarQube Cloud". - `.github/dependabot.yml` — weekly `gomod` and `github-actions` updates, each grouped into a single PR. Commit-message prefixes are set to `build` (gomod) and `ci` (actions) so Dependabot speaks Conventional Commits and diff --git a/CLAUDE.md b/CLAUDE.md index 43d5da0f..b94052ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,6 +17,11 @@ below so there is a single source of truth. - **Every leaf command has a functional test** (`test/functional`), a new one included, in the same commit. `TestEveryLeafIsCovered` fails otherwise; `uncovered.txt` is old debt that only shrinks. See AGENTS.md → "Testing". +- Run the cheap checks here (vet, the changed packages' tests, the full unit + suite at ~45 s, gofmt from Go 1.27) and leave lint, race, crossbuild, coverage + and SonarQube to GitHub — AGENTS.md → "Where checks run (agent sessions)". Pipe + test output through `grep -v '^ok'`; after a push, read the CI result and fix + red before moving on. - Prefer the dedicated file/search tools over shell `grep`/`find`. - This repo is developed on a feature branch — never push to `main` without explicit permission. Commit and push only when asked. diff --git a/Makefile b/Makefile index b6ba5b30..2a6256e0 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ export GOFLAGS := -mod=vendor # CI cross-compile matrix stay in sync with the release matrix. PLATFORMS := linux/amd64 linux/arm64 darwin/amd64 darwin/arm64 windows/amd64 windows/arm64 -.PHONY: all build static test functional race cover sonar crossbuild vet lint fmt tidy vendor completions size clean +.PHONY: all build static test functional race cover crossbuild vet lint fmt tidy vendor completions size clean all: build @@ -40,34 +40,24 @@ functional: race: CGO_ENABLED=1 GOPROXY=off go test -race ./... -## cover: write coverage.out for SonarQube (see AGENTS.md "SonarQube"). +## cover: write coverage.out for SonarQube Cloud (see AGENTS.md "SonarQube Cloud"; +## .github/workflows/sonar.yml runs this, so there is no local scan to remember). ## -coverpkg=./... is load-bearing: without it each test binary only instruments ## its own package, so the whole command tree that internal/cli's root tests ## build is executed but never attributed. It is worth ~11 points. +## +## -coverpkg also makes EVERY test binary report EVERY block, so the raw profile +## repeats each block once per package (~36x, ~800k lines). The awk pass merges +## the repeats, keeping the highest count; `go tool cover` merges the same way, +## so the total is unchanged and the upload is the size of the tree. cover: - GOPROXY=off go test -coverpkg=./... -coverprofile=coverage.out ./... + GOPROXY=off go test -coverpkg=./... -coverprofile=coverage.raw.out ./... + @awk 'NR == 1 { print; next } \ + { k = $$1 " " $$2; if (!(k in c)) { o[++n] = k; c[k] = $$3 } else if ($$3 + 0 > c[k] + 0) c[k] = $$3 } \ + END { for (i = 1; i <= n; i++) print o[i], c[o[i]] }' coverage.raw.out > coverage.out + @rm -f coverage.raw.out @go tool cover -func=coverage.out | tail -1 -## sonar: run the SonarQube scan (NETWORK + a server token; deliberately outside -## the offline per-commit gate — see AGENTS.md "SonarQube"). -## -## The two prerequisites below are the whole point of this target, because both -## were forgotten by hand and both fail silently: -## - `cover` first, so the profile matches the tree being scanned. A stale -## coverage.out does not error; it reports the new code as untested. -## - -Dsonar.projectVersion, so the new-code period means "since the last -## release". Omit it and every scan analyses the same version, the baseline -## freezes at whatever it was, and old findings keep counting as new. -## Host and token come from the environment and must never be committed. -sonar: cover - @test -n "$(SONAR_HOST_URL)" || { echo "SONAR_HOST_URL is unset" >&2; exit 1; } - @test -n "$(SONAR_TOKEN)" || { echo "SONAR_TOKEN is unset" >&2; exit 1; } - podman run --rm \ - -e SONAR_HOST_URL -e SONAR_TOKEN \ - -v "$(CURDIR):/usr/src" \ - docker.io/sonarsource/sonar-scanner-cli:latest \ - -Dsonar.projectVersion="$$(git describe --tags --abbrev=0)" - ## crossbuild: compile every release target (build-only, offline) so a build-tag ## or syscall mistake is caught here instead of at release time. crossbuild: diff --git a/cmd/koc/main_test.go b/cmd/koc/main_test.go new file mode 100644 index 00000000..c51aa5c8 --- /dev/null +++ b/cmd/koc/main_test.go @@ -0,0 +1,19 @@ +package main + +import ( + "os" + "testing" +) + +// TestMainVersion runs main end to end on the one invocation that exits 0 +// without a cloud. It also gives this package a test file: under +// `go test -coverpkg=./...` (make cover) a package without one is reported via +// `go tool covdata`, which toolchains fetched by GOTOOLCHAIN do not ship, and +// the whole run then exits 1. +func TestMainVersion(t *testing.T) { + saved := os.Args + t.Cleanup(func() { os.Args = saved }) + os.Args = []string{"koc", "--version"} + + main() +} diff --git a/docs/shell.md b/docs/shell.md index a6692235..5e61fdcb 100644 --- a/docs/shell.md +++ b/docs/shell.md @@ -86,7 +86,11 @@ directory, `$HOME`, `XDG_CONFIG_HOME`, `XDG_CONFIG_DIRS`, `/etc/openstack`), operator names explicitly — `OS_CLIENT_CONFIG_FILE`, `VAULT_TOKEN_FILE`, `KUBECONFIG`, `--kubeconfig` — still wins in the default profile. In the embedded profile `KOC_HOME` defaults to `$TMPDIR/koc-home`, explicit config -paths are ignored, and `$HOME`, `XDG_*` and `/etc` are never read. +paths are ignored, and `$HOME`, `XDG_*` and `/etc` are never read. koc creates +that default directory itself, `0700`, and refuses to run when it is a symlink, +belongs to another user, or is writable by group or others, since on a shared +host another user could otherwise plant the scope file or redirect the audit +log; set `KOC_HOME` to use another directory. ### History diff --git a/internal/cli/embedded.go b/internal/cli/embedded.go index 5af1e99d..68610895 100644 --- a/internal/cli/embedded.go +++ b/internal/cli/embedded.go @@ -203,6 +203,9 @@ func checkEmbedded(cmd *cobra.Command) error { case cobra.ShellCompRequestCmd, cobra.ShellCompNoDescRequestCmd, "help", "completion": return nil } + if err := kochome.CheckEmbeddedHome(); err != nil { + return err + } for _, r := range embeddedCommandRefusals { if path == r.path || strings.HasPrefix(path, r.path+" ") { return &RefusedError{Reason: r.reason, Msg: r.msg} diff --git a/internal/cli/embedded_test.go b/internal/cli/embedded_test.go index 8b5d1154..a7a99f0d 100644 --- a/internal/cli/embedded_test.go +++ b/internal/cli/embedded_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "os" + "path/filepath" "regexp" + "runtime" "strings" "testing" @@ -264,3 +267,31 @@ func TestDefaultProfileRefusesNothing(t *testing.T) { t.Errorf("--%s exists outside the embedded profile", flagPrintPrivateKey) } } + +// An unsafe default home stops the command before anything reads from it. +func TestEmbeddedRefusesAnUnsafeDefaultHome(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("permission bits do not describe who may write on Windows") + } + clearOSEnv(t) + t.Setenv("OS_REGION_NAME", "RegionOne") + t.Setenv("OS_PROJECT_ID", "p1") + t.Setenv(kochome.EnvProfile, kochome.ProfileEmbedded) + t.Setenv(kochome.EnvHome, "") + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + home := filepath.Join(tmp, "koc-home") + if err := os.Mkdir(home, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Chmod(home, 0o777); err != nil { + t.Fatal(err) + } + _, err := Run(context.Background(), "test", []string{"server", "list"}, func(root *cobra.Command, _ []string) { + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + }) + if err == nil || !strings.Contains(err.Error(), "writable by group or others") { + t.Errorf("server list with a 0777 default home: err = %v, want the home refused", err) + } +} diff --git a/internal/kochome/kochome.go b/internal/kochome/kochome.go index 2831606f..2952bcce 100644 --- a/internal/kochome/kochome.go +++ b/internal/kochome/kochome.go @@ -19,6 +19,9 @@ package kochome import ( + "errors" + "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -64,6 +67,35 @@ func Dir() string { return "" } +// CheckEmbeddedHome makes the embedded profile's default directory safe to use, +// and is a no-op otherwise (default profile, or KOC_HOME named by the host). +// +// That default is a fixed name under $TMPDIR, which on a shared host is +// world-writable: another user could create it first and plant the scope file +// koc reads, or a symlink where the audit log is appended. So koc creates it +// itself, 0700, and refuses one that is a symlink, belongs to someone else, or +// is writable by group or others. Set KOC_HOME to choose another directory. +func CheckEmbeddedHome() error { + if !Embedded() || strings.TrimSpace(os.Getenv(EnvHome)) != "" { + return nil + } + dir := Dir() + if err := os.Mkdir(dir, 0o700); err != nil && !errors.Is(err, fs.ErrExist) { + return fmt.Errorf("creating %s: %w", dir, err) + } + fi, err := os.Lstat(dir) + if err != nil { + return fmt.Errorf("checking %s: %w", dir, err) + } + if !fi.IsDir() { + return fmt.Errorf("%s is not a directory koc created; set %s to another directory", dir, EnvHome) + } + if why := unsafeDir(fi); why != "" { + return fmt.Errorf("%s %s; set %s to another directory", dir, why, EnvHome) + } + return nil +} + // Confined reports whether every implicit config location is replaced by Dir. func Confined() bool { return Dir() != "" } diff --git a/internal/kochome/kochome_test.go b/internal/kochome/kochome_test.go index a02eb598..f758300d 100644 --- a/internal/kochome/kochome_test.go +++ b/internal/kochome/kochome_test.go @@ -3,6 +3,8 @@ package kochome import ( "os" "path/filepath" + "runtime" + "strings" "testing" ) @@ -62,3 +64,103 @@ func TestOpenHook(t *testing.T) { t.Errorf("hook saw %q, want exactly one %q", seen, p) } } + +func TestCheckEmbeddedHome(t *testing.T) { + embedded := func(t *testing.T) string { + t.Helper() + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + t.Setenv(EnvProfile, ProfileEmbedded) + t.Setenv(EnvHome, "") + return filepath.Join(tmp, embeddedHomeName) + } + + t.Run("default profile is a no-op", func(t *testing.T) { + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + t.Setenv(EnvProfile, "") + t.Setenv(EnvHome, "") + if err := CheckEmbeddedHome(); err != nil { + t.Fatal(err) + } + if _, err := os.Lstat(filepath.Join(tmp, embeddedHomeName)); !os.IsNotExist(err) { + t.Errorf("default profile created the embedded home: %v", err) + } + }) + + t.Run("KOC_HOME named by the host is trusted", func(t *testing.T) { + dir := embedded(t) + t.Setenv(EnvHome, t.TempDir()) + if err := CheckEmbeddedHome(); err != nil { + t.Fatal(err) + } + if _, err := os.Lstat(dir); !os.IsNotExist(err) { + t.Errorf("created %s although KOC_HOME is set: %v", dir, err) + } + }) + + t.Run("missing is created 0700", func(t *testing.T) { + dir := embedded(t) + if err := CheckEmbeddedHome(); err != nil { + t.Fatal(err) + } + fi, err := os.Lstat(dir) + if err != nil { + t.Fatal(err) + } + if !fi.IsDir() { + t.Fatalf("%s is not a directory", dir) + } + if runtime.GOOS != "windows" && fi.Mode().Perm() != 0o700 { + t.Errorf("mode = %v, want 0700", fi.Mode().Perm()) + } + if err := CheckEmbeddedHome(); err != nil { + t.Errorf("second call on the directory it created: %v", err) + } + }) + + t.Run("a symlink is refused", func(t *testing.T) { + dir := embedded(t) + if err := os.Symlink(t.TempDir(), dir); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + if err := CheckEmbeddedHome(); err == nil || !strings.Contains(err.Error(), EnvHome) { + t.Errorf("symlinked home: err = %v, want a refusal naming %s", err, EnvHome) + } + }) + + t.Run("an uncreatable home is an error", func(t *testing.T) { + embedded(t) + t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "missing")) + if err := CheckEmbeddedHome(); err == nil || !strings.Contains(err.Error(), "creating") { + t.Errorf("home under a missing TMPDIR: err = %v, want a creation error", err) + } + }) + + t.Run("a file is refused", func(t *testing.T) { + dir := embedded(t) + if err := os.WriteFile(dir, nil, 0o600); err != nil { + t.Fatal(err) + } + if err := CheckEmbeddedHome(); err == nil { + t.Error("a regular file was accepted as the home") + } + }) + + t.Run("group or world writable is refused", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("permission bits do not describe who may write on Windows") + } + dir := embedded(t) + if err := os.Mkdir(dir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Chmod(dir, 0o777); err != nil { + t.Fatal(err) + } + err := CheckEmbeddedHome() + if err == nil || !strings.Contains(err.Error(), "writable by group or others") { + t.Errorf("0777 home: err = %v, want a refusal", err) + } + }) +} diff --git a/internal/kochome/unsafe_other.go b/internal/kochome/unsafe_other.go new file mode 100644 index 00000000..122d3139 --- /dev/null +++ b/internal/kochome/unsafe_other.go @@ -0,0 +1,10 @@ +//go:build !unix + +package kochome + +import "os" + +// unsafeDir accepts every directory: outside Unix, the temporary directory is +// already per-user, and the permission bits Go reports do not describe who may +// write to it. +func unsafeDir(os.FileInfo) string { return "" } diff --git a/internal/kochome/unsafe_unix.go b/internal/kochome/unsafe_unix.go new file mode 100644 index 00000000..fc7b626b --- /dev/null +++ b/internal/kochome/unsafe_unix.go @@ -0,0 +1,20 @@ +//go:build unix + +package kochome + +import ( + "os" + "syscall" +) + +// unsafeDir says why a directory another local user could control is unsafe to +// use as the embedded home, or "" when it is not. +func unsafeDir(fi os.FileInfo) string { + if st, ok := fi.Sys().(*syscall.Stat_t); ok && int64(st.Uid) != int64(os.Geteuid()) { + return "belongs to another user" + } + if fi.Mode().Perm()&0o022 != 0 { + return "is writable by group or others" + } + return "" +} diff --git a/internal/kochome/unsafe_unix_test.go b/internal/kochome/unsafe_unix_test.go new file mode 100644 index 00000000..d72cfea9 --- /dev/null +++ b/internal/kochome/unsafe_unix_test.go @@ -0,0 +1,43 @@ +//go:build unix + +package kochome + +import ( + "io/fs" + "os" + "syscall" + "testing" + "time" +) + +// dirInfo is a directory's FileInfo with the owner and mode a test chooses. +type dirInfo struct { + mode fs.FileMode + uid uint32 +} + +func (d dirInfo) Name() string { return "koc-home" } +func (d dirInfo) Size() int64 { return 0 } +func (d dirInfo) Mode() fs.FileMode { return fs.ModeDir | d.mode } +func (d dirInfo) ModTime() time.Time { return time.Time{} } +func (d dirInfo) IsDir() bool { return true } +func (d dirInfo) Sys() any { return &syscall.Stat_t{Uid: d.uid} } + +func TestUnsafeDir(t *testing.T) { + me := uint32(os.Geteuid()) + for _, tc := range []struct { + name string + fi dirInfo + want string + }{ + {"mine, 0700", dirInfo{0o700, me}, ""}, + {"mine, 0755", dirInfo{0o755, me}, ""}, + {"another user's", dirInfo{0o700, me + 1}, "belongs to another user"}, + {"mine, group-writable", dirInfo{0o770, me}, "is writable by group or others"}, + {"mine, world-writable", dirInfo{0o707, me}, "is writable by group or others"}, + } { + if got := unsafeDir(tc.fi); got != tc.want { + t.Errorf("%s: unsafeDir = %q, want %q", tc.name, got, tc.want) + } + } +} diff --git a/internal/s3/retry_test.go b/internal/s3/retry_test.go index f1538e36..011e7e02 100644 --- a/internal/s3/retry_test.go +++ b/internal/s3/retry_test.go @@ -184,6 +184,7 @@ func TestDoDoesNotRetryASinkFailure(t *testing.T) { // A cancelled context ends the loop instead of sleeping out the budget. func TestDoStopsOnContextCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) + defer cancel() var calls atomic.Int32 c := retryClient(t, 5, func(w http.ResponseWriter, _ *http.Request) { calls.Add(1) diff --git a/internal/watch/watch_test.go b/internal/watch/watch_test.go index 41315ddd..bf9f0dd7 100644 --- a/internal/watch/watch_test.go +++ b/internal/watch/watch_test.go @@ -126,6 +126,7 @@ func TestRunStopsAtCount(t *testing.T) { func TestRunStopsOnContextCancel(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) + defer cancel() h := newHarness() h.render = func(n int, out, _ io.Writer) error { if n == 2 { @@ -336,6 +337,7 @@ func TestBackoffWidensTheIntervalWhenRefreshesAreSlow(t *testing.T) { func TestRepaintEntersAndLeavesTheAlternateScreenExactlyOnce(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) + defer cancel() h := newHarness() h.render = func(n int, out, _ io.Writer) error { if n == 3 { diff --git a/scripts/api-compat.sh b/scripts/api-compat.sh index 5da491b1..51be8c43 100755 --- a/scripts/api-compat.sh +++ b/scripts/api-compat.sh @@ -37,11 +37,11 @@ echo "Public API changes, $base → HEAD:" apidiff -m "$work/base.export" "$work/head.export" 2>&1 | quiet incompatible=$(apidiff -m -incompatible "$work/base.export" "$work/head.export" 2>&1 | quiet) -if [ -z "$incompatible" ]; then +if [[ -z "$incompatible" ]]; then echo "No incompatible change." exit 0 fi -if [ ! -d "$work/base/pkg" ]; then +if [[ ! -d "$work/base/pkg" ]]; then echo "::notice::$base has no pkg/ yet; reporting only until the first release that carries the public API." exit 0 fi diff --git a/scripts/devstack/up.sh b/scripts/devstack/up.sh index f66f1f17..0871ca2d 100755 --- a/scripts/devstack/up.sh +++ b/scripts/devstack/up.sh @@ -308,7 +308,7 @@ fi # from gophercloud/devstack-action (see the header); harmless on a fresh VM but # not needed there, so only on --gha. gha_workarounds() { - python3 -m pip install --upgrade pip + python3 -m pip install --upgrade --only-binary :all: pip sudo apt-get update sudo apt-get purge -y 'mysql-*' || true # A leading "-e " in /etc/hosts (actions/runner-images#12192) and a diff --git a/scripts/release-notes.sh b/scripts/release-notes.sh index cc645ac2..616b918b 100755 --- a/scripts/release-notes.sh +++ b/scripts/release-notes.sh @@ -22,7 +22,7 @@ PREV="$(git tag --sort=v:refname | awk -v t="$TAG" '$0==t{exit} {print}' | tail # A symmetric range with --cherry-pick --right-only drops a commit whose patch # already shipped under the previous tag with another hash, as a rebase merge # re-creates every commit of a tagged branch on master. -if [ -n "$PREV" ]; then REV="${PREV}...${END}"; else REV="$END"; fi +if [[ -n "$PREV" ]]; then REV="${PREV}...${END}"; else REV="$END"; fi # Collect one bullet per commit into its type's bucket. feat=(); fix=(); perf=(); refactor=(); docs=(); tests=(); tooling=(); other=(); breaking=(); deprecated=() @@ -34,7 +34,7 @@ feat=(); fix=(); perf=(); refactor=(); docs=(); tests=(); tooling=(); other=(); while IFS= read -r sha; do subject="$(git log -1 --format='%s' "$sha")" body="$(git log -1 --format='%b' "$sha")" - [ -z "$subject" ] && continue + [[ -z "$subject" ]] && continue # Match: type(scope)!: description if [[ "$subject" =~ ^([a-zA-Z]+)(\(([^\)]*)\))?(!)?:[[:space:]]*(.*)$ ]]; then type="${BASH_REMATCH[1],,}" @@ -44,18 +44,18 @@ while IFS= read -r sha; do else type="_"; scope=""; bang=""; desc="$subject" fi - if [ -n "$scope" ]; then prefix="- **${scope}:** "; else prefix="- "; fi + if [[ -n "$scope" ]]; then prefix="- **${scope}:** "; else prefix="- "; fi bullet="${prefix}${desc}" # A "Deprecated: …" footer (the public Go API's deprecation # policy, pkg/openstack/doc.go) adds a bullet under its own heading; the # commit is still listed under its type. One footer per identifier. while IFS= read -r note; do - [ -n "$note" ] && deprecated+=("${prefix}${note}") + [[ -n "$note" ]] && deprecated+=("${prefix}${note}") done < <(printf '%s\n' "$body" | sed -n 's/^Deprecated:[[:space:]]*//p') # A breaking change is listed once, under its own heading, keeping its scope. # Listing it again under "Bug fixes"/"Features" reads as a duplicated bullet # and buries the one line a consumer most needs to see. - if [ -n "$bang" ] || printf '%s\n' "$body" | grep -qE '^BREAKING[ -]CHANGE:'; then + if [[ -n "$bang" ]] || printf '%s\n' "$body" | grep -qE '^BREAKING[ -]CHANGE:'; then breaking+=("$bullet") continue fi @@ -74,7 +74,7 @@ done < <(git log --no-merges --cherry-pick --right-only --format='%H' "$REV") emit() { # heading, array-name local heading="$1"; shift local arr=("$@") - [ "${#arr[@]}" -eq 0 ] && return 0 + [[ "${#arr[@]}" -eq 0 ]] && return 0 printf '%s\n' "$heading" printf '%s\n' "${arr[@]}" printf '\n' @@ -93,7 +93,7 @@ emit() { # heading, array-name emit "### Tests" ${tests[@]+"${tests[@]}"} emit "### Build & tooling" ${tooling[@]+"${tooling[@]}"} emit "### Other" ${other[@]+"${other[@]}"} - if [ -n "$PREV" ]; then + if [[ -n "$PREV" ]]; then printf '**Full Changelog**: https://github.com/%s/compare/%s...%s\n' "$REPO" "$PREV" "$TAG" fi } diff --git a/sonar-project.properties b/sonar-project.properties index 31a06c47..b662a640 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -1,11 +1,16 @@ -sonar.projectKey=go-openstackclient +# SonarQube Cloud (.github/workflows/sonar.yml). The key and organization are +# the ones sonarcloud.io assigns when it imports the repository; if the import +# picks different ones, change them here, not in the workflow. +sonar.organization=ftarasenko +sonar.projectKey=ftarasenko_go-openstackclient sonar.projectName=go-openstackclient (koc) sonar.sourceEncoding=UTF-8 sonar.scm.provider=git -# Pass the release tag at scan time so the "new code" period means "since the -# last release" rather than "since the previous scan" — otherwise a mechanical -# refactor re-attributes long-standing untested lines to new code: -# -Dsonar.projectVersion=$(git describe --tags --abbrev=0) +# The workflow passes the release tag at scan time so the "new code" period +# means "since the last release" rather than "since the previous scan" — +# otherwise a mechanical refactor re-attributes long-standing untested lines to +# new code: +# -Dsonar.projectVersion=$(git describe --tags --abbrev=0 --match 'v*') # --- Sources --- # Analyse the whole repo, minus vendored/generated/build artifacts and test code @@ -20,8 +25,7 @@ sonar.exclusions=\ koc-darwin-*,\ koc-windows-*,\ koc-*/**,\ - coverage.out,\ - coverage.html,\ + coverage.*,\ **/*_test.go,\ **/testdata/** sonar.coverage.exclusions=\ @@ -37,7 +41,7 @@ sonar.test.exclusions=\ vendor/**,\ dist/** -# --- Coverage (optional; `go test -coverprofile=coverage.out ./...`) --- +# --- Coverage (`make cover`, run by the workflow on the scanned checkout) --- sonar.go.coverage.reportPaths=coverage.out # --- Rule scoping ---