Skip to content

Commit 15adb8f

Browse files
authored
golangci-lint upgrade to v2.11.4, related fixes (#549)
* exclude node_modules from golangci-lint * bump golangci-lint in CI The branch updates `.golangci.yaml` and is being linted locally with `golangci-lint` 2.11.4, but CI was still pinned to 2.9.0. That leaves local and CI checks evaluating different rule sets and can hide or invent failures depending on where the lint runs. Update the workflow pin to 2.11.4 so the GitHub Action fetches the same linter version used locally. Keeping the versions aligned makes the branch's lint results reproducible across environments. * replace httptest.NewRequest in tests Recent `golangci-lint` releases enable the `noctx` checks that now flag plain `httptest.NewRequest` calls in our tests and shared test helpers. Those requests were still valid, but they no longer match the context- aware API the linter expects. Switch the affected tests and helper code to `httptest.NewRequestWithContext`, reusing each test's existing context where possible and falling back to `context.Background()` in the small helper that has no test context available. This keeps the request setup behavior the same while satisfying the stricter lint rule. * confine packager writes to output root The packager writes `.mod`, `.zip`, and `.info` artifacts under a caller-provided output directory. Those writes were assembled with `filepath.Join` and then passed straight to file creation helpers, which is exactly the pattern `gosec` now flags for path traversal. Open the version output directory as an `os.Root` and perform the file writes through that root instead. This keeps the artifact generation behavior the same, but confines every write to the intended output subtree even if a filename is malformed or hostile. * suppress gosec on -healthcheck probe `gosec` flagged the `riverui -healthcheck=...` code path because it constructs an outbound HTTP request using deployment configuration. That probe is an operator-invoked check of the running River UI server's own health endpoint, and we don't treat steering it via env as a meaningful security boundary in this context. Keep the existing behavior and add a narrow `//nolint:gosec` suppression on the request construction and execution lines with an explanation of why the warning is not actionable here.
1 parent 3b76850 commit 15adb8f

11 files changed

Lines changed: 36 additions & 22 deletions

File tree

.github/workflows/ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ jobs:
9090
name: Go lint
9191
runs-on: ubuntu-latest
9292
env:
93-
GOLANGCI_LINT_VERSION: v2.9.0
93+
GOLANGCI_LINT_VERSION: v2.11.4
9494
GOPROXY: https://proxy.golang.org,https://u:${{ secrets.RIVERPRO_GO_MOD_CREDENTIAL }}@riverqueue.com/goproxy,direct
9595
permissions:
9696
contents: read

.golangci.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ linters:
9292

9393
exclusions:
9494
generated: lax
95+
paths:
96+
- node_modules/
9597
presets:
9698
- comments
9799
- common-false-positives
@@ -108,6 +110,10 @@ formatters:
108110
- gofumpt
109111
- goimports
110112

113+
exclusions:
114+
paths:
115+
- node_modules/
116+
111117
settings:
112118
gci:
113119
sections:

handler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func TestMountStaticFiles(t *testing.T) {
128128

129129
var (
130130
recorder = httptest.NewRecorder()
131-
req = httptest.NewRequest(http.MethodGet, "/robots.txt", nil)
131+
req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/robots.txt", nil)
132132
)
133133

134134
mux.ServeHTTP(recorder, req)

internal/authmiddleware/auth_middleware_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func TestBasicAuth_Middleware(t *testing.T) {
7070
t.Run(tt.name, func(t *testing.T) {
7171
t.Parallel()
7272

73-
req := httptest.NewRequest(http.MethodGet, "/", nil)
73+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
7474
tt.setupRequest(req)
7575

7676
rec := httptest.NewRecorder()
@@ -112,7 +112,7 @@ func Test_isReqAuthorized(t *testing.T) {
112112
t.Run(tt.name, func(t *testing.T) {
113113
t.Parallel()
114114

115-
req := httptest.NewRequest(http.MethodGet, "/", nil)
115+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
116116
if tt.hasAuth {
117117
req.SetBasicAuth(tt.reqUser, tt.reqPass)
118118
}

internal/handlertest/handlertest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func RunIntegrationTest[TClient any](t *testing.T, createClient func(ctx context
4848
body = bytes.NewBuffer(payload)
4949
}
5050

51-
req := httptest.NewRequest(method, path, body)
51+
req := httptest.NewRequestWithContext(ctx, method, path, body)
5252
recorder := httptest.NewRecorder()
5353

5454
t.Logf("--> %s %s", method, path)

internal/riveruicmd/auth_middleware_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestAuthMiddleware(t *testing.T) { //nolint:tparallel
5858
t.Parallel()
5959

6060
handler := setup(t, "/")
61-
req := httptest.NewRequest(http.MethodGet, "/api/jobs", nil)
61+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/jobs", nil)
6262
recorder := httptest.NewRecorder()
6363

6464
handler.ServeHTTP(recorder, req)
@@ -70,7 +70,7 @@ func TestAuthMiddleware(t *testing.T) { //nolint:tparallel
7070
t.Parallel()
7171

7272
handler := setup(t, "/")
73-
req := httptest.NewRequest(http.MethodGet, "/api/jobs", nil)
73+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/jobs", nil)
7474
req.SetBasicAuth(basicAuthUser, basicAuthPassword)
7575

7676
recorder := httptest.NewRecorder()
@@ -84,7 +84,7 @@ func TestAuthMiddleware(t *testing.T) { //nolint:tparallel
8484
t.Parallel()
8585

8686
handler := setup(t, "/")
87-
req := httptest.NewRequest(http.MethodGet, "/api/health-checks/complete", nil)
87+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/health-checks/complete", nil)
8888
recorder := httptest.NewRecorder()
8989

9090
handler.ServeHTTP(recorder, req)
@@ -96,7 +96,7 @@ func TestAuthMiddleware(t *testing.T) { //nolint:tparallel
9696
t.Parallel()
9797

9898
handler := setup(t, "/test-prefix")
99-
req := httptest.NewRequest(http.MethodGet, "/test-prefix/api/health-checks/complete", nil)
99+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/test-prefix/api/health-checks/complete", nil)
100100
recorder := httptest.NewRecorder()
101101

102102
handler.ServeHTTP(recorder, req)

internal/riveruicmd/riveruicmd.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,12 @@ func checkHealth(ctx context.Context, pathPrefix string, healthCheckName string)
7777
hostname := net.JoinHostPort(host, port)
7878
url := fmt.Sprintf("http://%s%s/api/health-checks/%s", hostname, pathPrefix, healthCheckName)
7979

80+
//nolint:gosec // `-healthcheck` is an operator-invoked probe of the running River UI server's own HTTP health endpoint.
8081
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
8182
if err != nil {
8283
return fmt.Errorf("error constructing request to health endpoint: %w", err)
8384
}
85+
//nolint:gosec // `-healthcheck` intentionally reuses the configured River UI endpoint and isn't treated as a security boundary here.
8486
response, err := http.DefaultClient.Do(req)
8587
if err != nil {
8688
return fmt.Errorf("error requesting health endpoint: %w", err)

internal/riveruicmd/riveruicmd_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func TestInitServer(t *testing.T) { //nolint:tparallel
8989
t.Parallel()
9090

9191
initRes, _ := setup(t)
92-
req := httptest.NewRequest(http.MethodGet, "/api/features", nil)
92+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/features", nil)
9393
recorder := httptest.NewRecorder()
9494
initRes.uiHandler.ServeHTTP(recorder, req)
9595

@@ -106,7 +106,7 @@ func TestInitServer(t *testing.T) { //nolint:tparallel
106106
// Cannot be parallelized because of Setenv calls.
107107
t.Setenv("RIVER_JOB_LIST_HIDE_ARGS_BY_DEFAULT", "true")
108108
initRes, _ := setup(t)
109-
req := httptest.NewRequest(http.MethodGet, "/api/features", nil)
109+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/features", nil)
110110
recorder := httptest.NewRecorder()
111111
initRes.uiHandler.ServeHTTP(recorder, req)
112112

@@ -123,7 +123,7 @@ func TestInitServer(t *testing.T) { //nolint:tparallel
123123
// Cannot be parallelized because of Setenv calls.
124124
t.Setenv("RIVER_JOB_LIST_HIDE_ARGS_BY_DEFAULT", "1")
125125
initRes, _ := setup(t)
126-
req := httptest.NewRequest(http.MethodGet, "/api/features", nil)
126+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/features", nil)
127127
recorder := httptest.NewRecorder()
128128
initRes.uiHandler.ServeHTTP(recorder, req)
129129

@@ -198,12 +198,12 @@ func TestSilentHealthchecks_SuppressesLogs(t *testing.T) {
198198
initRes := makeServer(t, "/", true)
199199

200200
recorder := httptest.NewRecorder()
201-
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/api/health-checks/minimal", nil))
201+
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/health-checks/minimal", nil))
202202
require.Equal(t, http.StatusOK, recorder.Code)
203203
require.Empty(t, memoryHandler.records)
204204

205205
recorder = httptest.NewRecorder()
206-
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/api/features", nil))
206+
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/features", nil))
207207
require.Equal(t, http.StatusOK, recorder.Code)
208208
require.NotEmpty(t, memoryHandler.records)
209209

@@ -212,7 +212,7 @@ func TestSilentHealthchecks_SuppressesLogs(t *testing.T) {
212212
initRes = makeServer(t, "/pfx", true)
213213

214214
recorder = httptest.NewRecorder()
215-
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/pfx/api/health-checks/minimal", nil))
215+
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/pfx/api/health-checks/minimal", nil))
216216
require.Equal(t, http.StatusOK, recorder.Code)
217217
require.Empty(t, memoryHandler.records)
218218

@@ -221,7 +221,7 @@ func TestSilentHealthchecks_SuppressesLogs(t *testing.T) {
221221
initRes = makeServer(t, "/pfx/", true)
222222

223223
recorder = httptest.NewRecorder()
224-
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/pfx/api/health-checks/minimal", nil))
224+
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/pfx/api/health-checks/minimal", nil))
225225
require.Equal(t, http.StatusOK, recorder.Code)
226226
require.Empty(t, memoryHandler.records)
227227

@@ -230,7 +230,7 @@ func TestSilentHealthchecks_SuppressesLogs(t *testing.T) {
230230
initRes = makeServer(t, "/", false)
231231

232232
recorder = httptest.NewRecorder()
233-
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/api/health-checks/minimal", nil))
233+
initRes.httpServer.Handler.ServeHTTP(recorder, httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/health-checks/minimal", nil))
234234
require.Equal(t, http.StatusOK, recorder.Code)
235235
require.NotEmpty(t, memoryHandler.records)
236236
}

packager/packager.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ func createBundle() error {
6565
if err := os.MkdirAll(vOutputDir, 0o700); err != nil {
6666
return err
6767
}
68+
outputRoot, err := os.OpenRoot(vOutputDir)
69+
if err != nil {
70+
return err
71+
}
72+
defer outputRoot.Close()
6873

6974
version := module.Version{
7075
Path: mod,
@@ -79,11 +84,11 @@ func createBundle() error {
7984
return err
8085
}
8186

82-
if err := os.WriteFile(filepath.Join(vOutputDir, modFilename), modFileContents, 0o600); err != nil {
87+
if err := outputRoot.WriteFile(modFilename, modFileContents, 0o600); err != nil {
8388
return err
8489
}
8590

86-
f, err := os.OpenFile(filepath.Join(vOutputDir, zipFilename), os.O_CREATE|os.O_WRONLY, 0o600)
91+
f, err := outputRoot.OpenFile(zipFilename, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600)
8792
if err != nil {
8893
return err
8994
}
@@ -98,7 +103,7 @@ func createBundle() error {
98103
Time: timestamp,
99104
}
100105

101-
infoFile, err := os.Create(filepath.Join(vOutputDir, version.Version+".info"))
106+
infoFile, err := outputRoot.Create(version.Version + ".info")
102107
if err != nil {
103108
return err
104109
}

riverproui/pro_handler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func TestProFeaturesEndpointResponse(t *testing.T) {
132132
}()
133133

134134
recorder := httptest.NewRecorder()
135-
req := httptest.NewRequest(http.MethodGet, "/api/features", nil)
135+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/features", nil)
136136

137137
handler.ServeHTTP(recorder, req)
138138

0 commit comments

Comments
 (0)