From c1a40d98a8ef5acf48c0cb944ad492625673cf1a Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:50:23 +0000 Subject: [PATCH 01/10] build: add make vulncheck target To be able to detect known vulnerabilities reachable from the repository's Go code, add a vulncheck target that runs govulncheck over all Go packages. The tool is pinned (currently v1.9.0) and downloaded to ./bin/ by the existing go-install-tool machinery, so the version stays out of go.mod. The scan fetches the Go vulnerability database on demand and therefore needs network access. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 2e34693172337d45db30dcb798532872ab5a47b7) --- Makefile | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Makefile b/Makefile index e222078c..e28a9960 100644 --- a/Makefile +++ b/Makefile @@ -98,6 +98,10 @@ go-lines: get-golines go-lines-fix: get-golines "$(GOLINES_BIN)" -w . +.PHONY: vulncheck +vulncheck: govulncheck ## Check Go code for known vulnerabilities (requires network access to the Go vulnerability database). + "$(GOVULNCHECK)" ./... + .PHONY: e2e e2e: ## Run e2e tests (requires: make deploy-bink). V=1 for verbose. RUN= to filter. # NB: we `cd` here instead of passing a package path to `go test` so that `-v` @@ -288,11 +292,13 @@ CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen ENVTEST ?= $(LOCALBIN)/setup-envtest GOLANGCI_LINT = $(LOCALBIN)/golangci-lint GOLINES_BIN = $(LOCALBIN)/golines +GOVULNCHECK = $(LOCALBIN)/govulncheck YQ ?= $(LOCALBIN)/yq KUSTOMIZE_VERSION ?= v5.8.1 CONTROLLER_TOOLS_VERSION ?= v0.20.1 GOLANGCI_LINT_VERSION ?= v2.11.4 +GOVULNCHECK_VERSION ?= v1.9.0 YQ_VERSION ?= v4.53.2 # ENVTEST_VERSION is the version of controller-runtime release branch to fetch the envtest setup script (i.e. release-0.20) @@ -332,6 +338,11 @@ golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. $(GOLANGCI_LINT): $(LOCALBIN) $(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint,$(GOLANGCI_LINT_VERSION)) +.PHONY: govulncheck +govulncheck: $(GOVULNCHECK) ## Download govulncheck locally if necessary. +$(GOVULNCHECK): $(LOCALBIN) + $(call go-install-tool,$(GOVULNCHECK),golang.org/x/vuln/cmd/govulncheck,$(GOVULNCHECK_VERSION)) + .PHONY: get-golines get-golines: $(GOLINES_BIN) ## Download golines locally if necessary. $(GOLINES_BIN): $(LOCALBIN) From 25a13c950f4a2b6fd093ccd6c6869d9b83b30c82 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:51:01 +0000 Subject: [PATCH 02/10] chore(deps): update golang.org/x/net to v0.60.0 govulncheck reports five vulnerabilities reachable from this repository's code, all in golang.org/x/net v0.59.0 and fixed in v0.60.0: GO-2026-6603, GO-2026-6610, GO-2026-6611, GO-2026-6612 and GO-2026-6617 (HTTP/2 flow control, HPACK and header handling issues). Bump to the minimum version that fixes them so the vulnerability check can be enforced in CI without any existing finding to waive. Two further findings in x/net/html (GO-2026-6860 and GO-2026-6866) are not reachable from this code and stay informational; they do not fail the check. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 0d424cb2eb43cefac7eeaf6115c70411320a3c60) # Conflicts: # go.mod # go.sum --- go.mod | 9 +++++++++ go.sum | 13 +++++++++++++ 2 files changed, 22 insertions(+) diff --git a/go.mod b/go.mod index bc27b942..9cbbef73 100644 --- a/go.mod +++ b/go.mod @@ -95,12 +95,21 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect +<<<<<<< HEAD golang.org/x/net v0.57.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/term v0.45.0 // indirect golang.org/x/text v0.40.0 // indirect +======= + golang.org/x/net v0.60.0 // indirect + golang.org/x/oauth2 v0.37.0 // indirect + golang.org/x/sync v0.23.0 // indirect + golang.org/x/sys v0.48.0 // indirect + golang.org/x/term v0.46.0 // indirect + golang.org/x/text v0.42.0 // indirect +>>>>>>> 0d424cb (chore(deps): update golang.org/x/net to v0.60.0) golang.org/x/time v0.15.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af // indirect diff --git a/go.sum b/go.sum index eee757bd..5510ee0b 100644 --- a/go.sum +++ b/go.sum @@ -207,6 +207,7 @@ go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= +<<<<<<< HEAD golang.org/x/mod v0.39.0 h1:UF5zwQdCRRUpHfyPwr7d4UrGiVeldIsogtzWVnczL74= golang.org/x/mod v0.39.0/go.mod h1:bvIbwjQ0HUFFf5AKukeeYQG4ZBUG9yxQbR9aEweIwYY= golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= @@ -215,6 +216,18 @@ golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +======= +golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c= +golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o= +golang.org/x/net v0.59.0 h1:5zfYln+w5XCxwrnMMJPufRgNoXEaGxl0wo5GqPXyues= +golang.org/x/net v0.59.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= +golang.org/x/net v0.60.0 h1:79p50tfZlm0J9YfoDsSi639qSXNGVwEzOPLCxM2FsYU= +golang.org/x/net v0.60.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= +golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98= +golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58= +golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= +golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= +>>>>>>> 0d424cb (chore(deps): update golang.org/x/net to v0.60.0) golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= From a08893103d4aad9486789a697553da1e3bf6b0c3 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:51:06 +0000 Subject: [PATCH 03/10] ci: run vulnerability check in unit job Run make vulncheck as a dedicated step in the unit job so pull requests fail when a known vulnerability is reachable from the repository's Go code. It is a separate step rather than part of make lint so the two checks can be run and diagnosed independently, and it reuses the Makefile target instead of duplicating the invocation. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 2985ad1eeca33cf7285d60137cbd00cda6f2a6a6) --- .github/workflows/ci.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d68dd1fc..18ad6190 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -48,6 +48,9 @@ jobs: - name: Lint run: make lint + - name: Vulnerability check + run: make vulncheck + build-bink: uses: $/.github/workflows/build-bink.yaml with: From 2d241d3e4cc911fb4a42a398b693c79d60eab8a9 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:51:11 +0000 Subject: [PATCH 04/10] docs: document make vulncheck Describe the new target in CONTRIBUTING.md: what it scans, that it downloads both the govulncheck binary and the Go vulnerability database and so needs network access, and how to interpret its output. Only findings in functions the code actually calls fail the check; findings in imported packages or required modules that are not called are informational. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit cbb242ecbb778b870eb52733c13933603deea776) --- CONTRIBUTING.md | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6e4ee6a8..452c526f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,9 +34,9 @@ For end-to-end testing you will also need: backed by bootc nodes - [skopeo](https://github.com/containers/skopeo) — container image inspection -Tool dependencies like `controller-gen`, `kustomize`, `golangci-lint`, and -`setup-envtest` are downloaded automatically to `./bin/` by the Makefile when -needed. +Tool dependencies like `controller-gen`, `kustomize`, `golangci-lint`, +`govulncheck`, and `setup-envtest` are downloaded automatically to `./bin/` by +the Makefile when needed. ### Development Environment @@ -106,6 +106,26 @@ make unit V=1 # Verbose output make unit RUN=Foo # Run tests matching "Foo" ``` +### Checking for Vulnerabilities + +```shell +make vulncheck +``` + +This runs [govulncheck](https://go.dev/blog/vuln) against all Go packages in +the repository, looking for known vulnerabilities from the +[Go vulnerability database](https://vuln.go.dev/). Both the `govulncheck` +binary and the database are downloaded on demand, so the target requires +network access. + +Only reachable findings — vulnerabilities in functions that the code actually +calls — fail the check. Fix them by upgrading the affected dependency or +changing the affected code. The scan may also report informational findings +in packages you import or modules you require whose vulnerable functions are +not called; these do not fail the check. + +CI runs this check in the `unit` job. + ### Running End-to-End Tests E2E tests live in `test/e2e/` and run against a real @@ -247,6 +267,7 @@ Follow a commit style similar to the Linux kernel: make vet make lint make unit + make vulncheck ``` 4. Push your branch and open a pull request against `main`. From 62db3ea2bdbc9458ddabbb6a19c470d4917a736c Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:37 +0000 Subject: [PATCH 05/10] internal: add package doc comments Every internal package lacked a package comment, which staticcheck's ST1000 check flags. Add a short one to each so the full staticcheck suite can be enabled. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 1caecb62d8d1948fa212d49e746a4686e018f9db) --- internal/bootc/executor.go | 1 + internal/controller/bootcnodepool_controller.go | 2 ++ internal/daemon/reconciler.go | 2 ++ internal/image/digest.go | 2 ++ internal/registry/resolver.go | 1 + internal/version/version.go | 1 + 6 files changed, 9 insertions(+) diff --git a/internal/bootc/executor.go b/internal/bootc/executor.go index 6d7a0a7c..4554966a 100644 --- a/internal/bootc/executor.go +++ b/internal/bootc/executor.go @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +// Package bootc runs bootc commands on the host and parses their output. package bootc import ( diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 7d334476..6f9dee32 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -1,5 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 +// Package controller implements the BootcNodePool controller, which +// resolves a pool's target image and rolls it out across member nodes. package controller import ( diff --git a/internal/daemon/reconciler.go b/internal/daemon/reconciler.go index 6bf422b3..afb3e11a 100644 --- a/internal/daemon/reconciler.go +++ b/internal/daemon/reconciler.go @@ -1,5 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 +// Package daemon implements the per-node daemon that watches bootc +// status and reconciles the node's BootcNode. package daemon import ( diff --git a/internal/image/digest.go b/internal/image/digest.go index 82a9c545..f46b5fd4 100644 --- a/internal/image/digest.go +++ b/internal/image/digest.go @@ -1,5 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 +// Package image provides helpers for matching container image +// references against digests. package image import ( diff --git a/internal/registry/resolver.go b/internal/registry/resolver.go index b9ee71e6..0c8631ee 100644 --- a/internal/registry/resolver.go +++ b/internal/registry/resolver.go @@ -1,3 +1,4 @@ +// Package registry resolves container image tags to digests. package registry import ( diff --git a/internal/version/version.go b/internal/version/version.go index 07251d1a..de51aa6f 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +// Package version holds build metadata set via linker flags. package version var ( From ea6ccd5963ff3cfaeaeddd7d835b066bdd7878a6 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:37 +0000 Subject: [PATCH 06/10] controller: saturate status counts instead of wrapping The pool status counts are int32 in the API but computed from int slice lengths, and the bare conversion would silently wrap on overflow (gosec G115). Node counts can't realistically get there, but a saturating conversion makes the intent explicit and keeps the linter quiet without a suppression. The counts can never be negative, but they stay int32 rather than uint32: the Kubernetes API conventions advise against unsigned integers because client support across languages is uneven, and recommend validating that the value is non-negative instead. Mark the counts with a minimum of 0 in the CRD schema, and clamp the conversion to [0, math.MaxInt32] so the controller never writes a value the schema would reject. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 971a76abc85b376d8072652ab853221ef012a15c) --- api/v1alpha1/bootcnodepool_types.go | 4 ++++ .../bases/node.bootc.dev_bootcnodepools.yaml | 4 ++++ internal/controller/status.go | 21 +++++++++++++++---- internal/controller/status_test.go | 13 ++++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/bootcnodepool_types.go b/api/v1alpha1/bootcnodepool_types.go index 2277cbbc..559c691a 100644 --- a/api/v1alpha1/bootcnodepool_types.go +++ b/api/v1alpha1/bootcnodepool_types.go @@ -200,19 +200,23 @@ type BootcNodePoolStatus struct { // nodeCount is the total number of nodes in this pool. // +optional + // +kubebuilder:validation:Minimum=0 NodeCount int32 `json:"nodeCount,omitempty"` // updatedCount is the number of nodes running the target digest. // +optional + // +kubebuilder:validation:Minimum=0 UpdatedCount int32 `json:"updatedCount,omitempty"` // updatingCount is the number of nodes actively being updated // (staging, staged, or rebooting). // +optional + // +kubebuilder:validation:Minimum=0 UpdatingCount int32 `json:"updatingCount,omitempty"` // degradedCount is the number of nodes in a degraded state. // +optional + // +kubebuilder:validation:Minimum=0 DegradedCount int32 `json:"degradedCount,omitempty"` // conditions represent the current state of the pool. diff --git a/config/crd/bases/node.bootc.dev_bootcnodepools.yaml b/config/crd/bases/node.bootc.dev_bootcnodepools.yaml index db056742..2332791a 100644 --- a/config/crd/bases/node.bootc.dev_bootcnodepools.yaml +++ b/config/crd/bases/node.bootc.dev_bootcnodepools.yaml @@ -263,6 +263,7 @@ spec: degradedCount: description: degradedCount is the number of nodes in a degraded state. format: int32 + minimum: 0 type: integer deployedDigest: description: |- @@ -278,6 +279,7 @@ spec: nodeCount: description: nodeCount is the total number of nodes in this pool. format: int32 + minimum: 0 type: integer observedGeneration: description: |- @@ -300,12 +302,14 @@ spec: description: updatedCount is the number of nodes running the target digest. format: int32 + minimum: 0 type: integer updatingCount: description: |- updatingCount is the number of nodes actively being updated (staging, staged, or rebooting). format: int32 + minimum: 0 type: integer type: object required: diff --git a/internal/controller/status.go b/internal/controller/status.go index 83fc2fee..266d0ac4 100644 --- a/internal/controller/status.go +++ b/internal/controller/status.go @@ -4,6 +4,7 @@ package controller import ( "fmt" + "math" "strings" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -19,12 +20,12 @@ func syncPoolStatus(pool *bootcv1alpha1.BootcNodePool, rs *rolloutState) { return } pool.Status.ObservedGeneration = pool.Generation - pool.Status.NodeCount = int32(rs.nodeCount()) - pool.Status.UpdatedCount = int32(len(rs.upToDate)) - pool.Status.UpdatingCount = int32( + pool.Status.NodeCount = clampCount(rs.nodeCount()) + pool.Status.UpdatedCount = clampCount(len(rs.upToDate)) + pool.Status.UpdatingCount = clampCount( len(rs.pending) + len(rs.staging) + len(rs.staged) + len(rs.rebooting), ) - pool.Status.DegradedCount = int32(len(rs.degraded)) + pool.Status.DegradedCount = clampCount(len(rs.degraded)) if pool.Status.NodeCount == pool.Status.UpdatedCount { pool.Status.DeployedDigest = pool.Status.TargetDigest @@ -34,6 +35,18 @@ func syncPoolStatus(pool *bootcv1alpha1.BootcNodePool, rs *rolloutState) { syncUpToDateCondition(pool, rs) } +// clampCount converts a count to int32 for the status API, saturating to +// [0, math.MaxInt32] to match the schema instead of wrapping. +func clampCount(n int) int32 { + if n < 0 { + return 0 + } + if n > math.MaxInt32 { + return math.MaxInt32 + } + return int32(n) +} + func syncUpToDateCondition(pool *bootcv1alpha1.BootcNodePool, rs *rolloutState) { if pool == nil || rs == nil { return diff --git a/internal/controller/status_test.go b/internal/controller/status_test.go index 7568b51b..20dcc0df 100644 --- a/internal/controller/status_test.go +++ b/internal/controller/status_test.go @@ -3,6 +3,7 @@ package controller import ( + "math" "testing" . "github.com/onsi/gomega" @@ -170,3 +171,15 @@ func TestSyncPoolStatusDeployedDigestPreserved(t *testing.T) { g.Expect(pool.Status.DeployedDigest).To(Equal(testDigestA)) g.Expect(pool.Status.UpdateAvailable).To(BeTrue()) } + +// Verifies that clampCount passes in-range counts through and saturates +// instead of wrapping when a count does not fit in int32. +func TestClampCount(t *testing.T) { + g := NewWithT(t) + + g.Expect(clampCount(0)).To(Equal(int32(0))) + g.Expect(clampCount(42)).To(Equal(int32(42))) + g.Expect(clampCount(math.MaxInt32)).To(Equal(int32(math.MaxInt32))) + g.Expect(clampCount(math.MaxInt32 + 1)).To(Equal(int32(math.MaxInt32))) + g.Expect(clampCount(-1)).To(Equal(int32(0))) +} From b900415b28cf27ec2bd37a81a3121bed379614f1 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:37:14 +0000 Subject: [PATCH 07/10] controller: release drain context once the drain is collected collectDrainResults dropped finished drains from r.drains without calling their cancel func, which is only invoked when a node leaves the pool. Each drain context derives from the reconcile context, which has no deadline, so every completed or failed drain stayed registered with the long-lived parent until the process exited. Cancel the context when the result is collected, after checking whether it was cancelled so the existing cancellation handling still works. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit c4107895bd3cbe6579160bac5590e947f6d105e2) --- internal/controller/rollout.go | 7 +++++-- internal/controller/rollout_test.go | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/internal/controller/rollout.go b/internal/controller/rollout.go index f1c3d069..461d8f18 100644 --- a/internal/controller/rollout.go +++ b/internal/controller/rollout.go @@ -371,11 +371,14 @@ func (r *BootcNodePoolReconciler) collectDrainResults( continue } - // Drain finished; remove from the map. + // Drain finished; remove from the map and release its context, + // which would otherwise stay registered with the parent. delete(r.drains, nodeName) + cancelled := ds.ctx.Err() != nil + ds.cancel() if drainErr != nil { - if ds.ctx.Err() != nil { + if cancelled { // Context was cancelled (node left pool or controller // shutdown). The canceller is responsible for cleanup. log.V(1).Info("Drain cancelled", "node", nodeName) diff --git a/internal/controller/rollout_test.go b/internal/controller/rollout_test.go index 18ffd244..b35b039f 100644 --- a/internal/controller/rollout_test.go +++ b/internal/controller/rollout_test.go @@ -3,6 +3,8 @@ package controller import ( + "context" + "errors" "testing" "time" @@ -522,3 +524,23 @@ func TestFindUnhealthySlots(t *testing.T) { g.Expect(findUnhealthySlots(rs, targetDigest)).To(BeEmpty()) }) } + +// Verifies that collecting a finished drain cancels its context, so it +// does not stay registered with the long-lived parent context. +func TestCollectDrainResultsReleasesContext(t *testing.T) { + g := NewWithT(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + drainCtx, drainCancel := context.WithCancel(ctx) + defer drainCancel() + ds := &drainStatus{result: make(chan error, 1), ctx: drainCtx, cancel: drainCancel} + ds.result <- errors.New("drain failed") + + r := &BootcNodePoolReconciler{drains: map[string]*drainStatus{"n1": ds}} + pool := testutil.NewPool("drain-release", testImageDigestRefA, testutil.WithWorkerSelector()) + + g.Expect(r.collectDrainResults(ctx, pool, nil)).To(Succeed()) + g.Expect(r.drains).To(BeEmpty()) + g.Expect(drainCtx.Err()).To(MatchError(context.Canceled)) +} From ad9472c49f1b29291c76ff83280a9c9378b91c68 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:37 +0000 Subject: [PATCH 08/10] internal: address gosec findings Prepare for enabling gosec by resolving its findings outside the e2e suite: - Tighten directory permissions in watcher tests to 0750 (G301). - Annotate the remaining reports as false positives, each with the reason: - G204: nsenter callers pass fixed program names, and the only external input is a validated, digested image reference; nothing goes through a shell. - G118 (drain): the cancel func is stored in r.drains and called when the drain is collected. - G118 (staging): staging intentionally outlives the reconcile, and its parentless context cannot leak. - G301: /run/ostree on the host keeps the conventional 0755 mode. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 0bedf3d56044b5e4030a50e618c39b3c268f856c) --- internal/bootc/executor.go | 5 ++++- internal/controller/rollout.go | 3 ++- internal/daemon/reconciler.go | 16 ++++++++++++++-- internal/daemon/watcher_test.go | 10 +++++----- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/internal/bootc/executor.go b/internal/bootc/executor.go index 4554966a..eaaa9bf7 100644 --- a/internal/bootc/executor.go +++ b/internal/bootc/executor.go @@ -61,7 +61,10 @@ func (e *HostExecutor) nsenterCmd(ctx context.Context, args ...string) *exec.Cmd "--setuid", "0", "--setgid", "0", "--env", "--", } - return exec.CommandContext(ctx, "nsenter", append(base, args...)...) + // Callers in this package pass fixed program names; the only external + // input is a validated, digested image reference. Arguments are not + // passed through a shell. + return exec.CommandContext(ctx, "nsenter", append(base, args...)...) //nolint:gosec // G204 } func (e *HostExecutor) Status(ctx context.Context) ([]byte, error) { diff --git a/internal/controller/rollout.go b/internal/controller/rollout.go index 461d8f18..212cb1cc 100644 --- a/internal/controller/rollout.go +++ b/internal/controller/rollout.go @@ -307,7 +307,8 @@ func (r *BootcNodePoolReconciler) ensureDrain( log.Info("Starting drain", "node", bn.Name) - drainCtx, cancel := context.WithCancel(ctx) + // cancel is stored in r.drains and called when the drain is collected. + drainCtx, cancel := context.WithCancel(ctx) //nolint:gosec // G118 status := &drainStatus{ result: make(chan error, 1), ctx: drainCtx, diff --git a/internal/daemon/reconciler.go b/internal/daemon/reconciler.go index afb3e11a..7a873336 100644 --- a/internal/daemon/reconciler.go +++ b/internal/daemon/reconciler.go @@ -236,7 +236,17 @@ func (r *BootcNodeReconciler) reconcileBootcNode( switchCtx, cancel := context.WithCancel(context.Background()) r.inflight.acquire(log, desiredImage, cancel) log.Info("Starting staging", "image", desiredImage) - go r.inflight.run(switchCtx, r.NodeName, desiredImage, r.Executor, r.stageDone) + // Staging must outlive this reconcile, so it is not derived from + // the request context. The context has no parent, so it cannot + // leak; acquire cancels it when a newer stage replaces it. + //nolint:gosec // G118 + go r.inflight.run( + switchCtx, + r.NodeName, + desiredImage, + r.Executor, + r.stageDone, + ) case actionAwaitStage: reason = bootcv1alpha1.NodeReasonStaging @@ -440,7 +450,9 @@ func (r *BootcNodeReconciler) writeAuthFile(data []byte) error { } dir := filepath.Join(hostRoot, "run", "ostree") - if err := os.MkdirAll(dir, 0755); err != nil { + // Standard mode for a host system directory under /run; it only + // applies if the host has not created the directory already. + if err := os.MkdirAll(dir, 0755); err != nil { //nolint:gosec // G301 return fmt.Errorf("creating auth dir: %w", err) } diff --git a/internal/daemon/watcher_test.go b/internal/daemon/watcher_test.go index ccf59a6c..9fe13f57 100644 --- a/internal/daemon/watcher_test.go +++ b/internal/daemon/watcher_test.go @@ -78,12 +78,12 @@ func TestWatcherEvents(t *testing.T) { composefsPath := filepath.Join(dir, "deploy") if tt.mkOstree { - if err := os.Mkdir(ostreePath, 0o755); err != nil { + if err := os.Mkdir(ostreePath, 0o750); err != nil { t.Fatal(err) } } if tt.mkComposefs { - if err := os.Mkdir(composefsPath, 0o755); err != nil { + if err := os.Mkdir(composefsPath, 0o750); err != nil { t.Fatal(err) } } @@ -100,7 +100,7 @@ func TestWatcherEvents(t *testing.T) { t.Fatal(err) } case triggerCreate: - if err := os.Mkdir(filepath.Join(composefsPath, "new-deploy"), 0o755); err != nil { + if err := os.Mkdir(filepath.Join(composefsPath, "new-deploy"), 0o750); err != nil { t.Fatal(err) } } @@ -204,7 +204,7 @@ func TestWatcherGetStatusColdCache(t *testing.T) { func TestWatcherShutdown(t *testing.T) { dir := t.TempDir() watchDir := filepath.Join(dir, "bootc") - if err := os.Mkdir(watchDir, 0o755); err != nil { + if err := os.Mkdir(watchDir, 0o750); err != nil { t.Fatal(err) } @@ -226,7 +226,7 @@ func TestWatcherShutdown(t *testing.T) { func TestWatcherNoPollWhenFsnotifyHealthy(t *testing.T) { dir := t.TempDir() ostreePath := filepath.Join(dir, "bootc") - if err := os.Mkdir(ostreePath, 0o755); err != nil { + if err := os.Mkdir(ostreePath, 0o750); err != nil { t.Fatal(err) } From 0165d79e10054a6e961778a1e7ee428518485871 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:49 +0000 Subject: [PATCH 09/10] lint: enable gosec and full staticcheck suite Without a config file golangci-lint runs only its standard linters, and staticcheck runs without the stylistic checks golangci-lint disables by default (package comments, naming, receiver and doc comment consistency). Add .golangci.yml to keep the standard set, enable all staticcheck checks, and add gosec for security issues. gosec is excluded from test/e2e, which intentionally drives kubectl, helm and bink with computed arguments. Because make lint already runs in CI, this needs no workflow change. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 2aa6aec456cf90dc49554049cfca385f4f22a498) --- .golangci.yml | 19 +++++++++++++++++++ CONTRIBUTING.md | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..0286eabe --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,19 @@ +version: "2" + +linters: + default: standard + enable: + - gosec + settings: + staticcheck: + # golangci-lint disables some stylistic checks by default; enable + # the full staticcheck suite. + checks: + - all + exclusions: + rules: + # The e2e suite drives external tools (kubectl, helm, bink) with + # computed arguments by design. + - path: ^test/e2e/ + linters: + - gosec diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 452c526f..779e1365 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -216,7 +216,10 @@ matching the structure described in [Finding Test Logs](#finding-test-logs). - Run `make fmt` and `make vet` before submitting. - Run `make lint` to check with [golangci-lint](https://golangci-lint.run/). Use `make lint-fix` to apply - automatic fixes. + automatic fixes. The configuration in `.golangci.yml` enables the full + staticcheck suite and gosec (except for the e2e tests). If a gosec finding + is a false positive, suppress it with `//nolint:gosec // ` and a + comment explaining why. - All Go files must include the SPDX license header: ```go From be3b09b090028d1e5e62195dd9818e19ff6548e2 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 15:20:35 +0000 Subject: [PATCH 10/10] build: require Go 1.26.10 CI installs the Go version named by the go directive in go.mod, so the vulnerability check ran against Go 1.26.0 and reported ten standard library vulnerabilities reachable from this repository's code, in net/http, net/textproto, crypto/tls and crypto/x509 (GO-2026-6603 through GO-2026-6809). They are fixed in Go 1.26.9 and 1.26.10. Raise the minimum to 1.26.10, the first release that fixes all of them, so CI builds and scans with a patched toolchain. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 67487724d966b1b02211447b401cfc8e2a2a1bec) --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 9cbbef73..16dab49c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/bootc-dev/bootc-operator -go 1.26.0 +go 1.26.10 require ( github.com/aws/aws-sdk-go-v2 v1.47.1