From 559bfa413194bdff294d251b83cf3676aacc3e83 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 15 Feb 2026 21:55:08 -0800 Subject: [PATCH] fix: resolve all golangci-lint issues Fixes #32 Changes: - middleware.go: use max() builtin, strconv.Itoa, fix wsl whitespace - database.go: fix nlreturn, noinlineerr, wsl whitespace - handlers.go: remove unnecessary template.HTML conversion, unused import - app.go: extract cleanupContainer to fix nestif, fix lll - client.go: break long string literals to fix lll - deploy.go: fix wsl whitespace - auth_test.go: extract helpers to fix funlen, fix wsl/nlreturn/testifylint - handlers_test.go: deduplicate IDOR tests, fix paralleltest - validation_test.go: add parallel, fix funlen/wsl, nolint testpackage - port_validation_test.go: add parallel, nolint testpackage - ratelimit_test.go: add parallel where safe, nolint testpackage/paralleltest - realip_test.go: add parallel, use NewRequestWithContext, fix wsl/funlen - user.go: (noinlineerr already fixed by database.go pattern) --- internal/database/database.go | 9 +- internal/docker/client.go | 7 +- internal/docker/validation_test.go | 13 +- internal/handlers/app.go | 55 +++++---- internal/handlers/handlers.go | 3 +- internal/handlers/handlers_test.go | 140 +++++++++++++--------- internal/handlers/port_validation_test.go | 6 +- internal/middleware/middleware.go | 12 +- internal/middleware/ratelimit_test.go | 8 +- internal/middleware/realip_test.go | 13 +- internal/service/auth/auth_test.go | 131 ++++++++++---------- internal/service/deploy/deploy.go | 1 + 12 files changed, 241 insertions(+), 157 deletions(-) diff --git a/internal/database/database.go b/internal/database/database.go index 2d9d22d..060699d 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -172,6 +172,7 @@ func (d *Database) connect(ctx context.Context) error { // HashWebhookSecret returns the hex-encoded SHA-256 hash of a webhook secret. func HashWebhookSecret(secret string) string { sum := sha256.Sum256([]byte(secret)) + return hex.EncodeToString(sum[:]) } @@ -181,6 +182,7 @@ func (d *Database) backfillWebhookSecretHashes(ctx context.Context) error { if err != nil { return fmt.Errorf("querying apps for backfill: %w", err) } + defer func() { _ = rows.Close() }() type row struct { @@ -191,14 +193,17 @@ func (d *Database) backfillWebhookSecretHashes(ctx context.Context) error { for rows.Next() { var r row - if scanErr := rows.Scan(&r.id, &r.secret); scanErr != nil { + + scanErr := rows.Scan(&r.id, &r.secret) + if scanErr != nil { return fmt.Errorf("scanning app for backfill: %w", scanErr) } toUpdate = append(toUpdate, r) } - if rowsErr := rows.Err(); rowsErr != nil { + rowsErr := rows.Err() + if rowsErr != nil { return fmt.Errorf("iterating apps for backfill: %w", rowsErr) } diff --git a/internal/docker/client.go b/internal/docker/client.go index 47c799f..e9ddfe4 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -611,10 +611,13 @@ func (c *Client) createGitContainer( var script string if cfg.commitSHA != "" { // Clone without depth limit so we can checkout any commit, then checkout specific SHA - script = `git clone --branch "$CLONE_BRANCH" "$CLONE_URL" /repo && cd /repo && git checkout "$CLONE_SHA" && echo COMMIT:$(git rev-parse HEAD)` + script = `git clone --branch "$CLONE_BRANCH" "$CLONE_URL" /repo` + + ` && cd /repo && git checkout "$CLONE_SHA"` + + ` && echo COMMIT:$(git rev-parse HEAD)` } else { // Shallow clone of branch HEAD, then output commit SHA - script = `git clone --depth 1 --branch "$CLONE_BRANCH" "$CLONE_URL" /repo && cd /repo && echo COMMIT:$(git rev-parse HEAD)` + script = `git clone --depth 1 --branch "$CLONE_BRANCH" "$CLONE_URL" /repo` + + ` && cd /repo && echo COMMIT:$(git rev-parse HEAD)` } env := []string{ diff --git a/internal/docker/validation_test.go b/internal/docker/validation_test.go index c715a74..785f3ed 100644 --- a/internal/docker/validation_test.go +++ b/internal/docker/validation_test.go @@ -1,4 +1,4 @@ -package docker +package docker //nolint:testpackage // tests unexported regexps and Client struct import ( "errors" @@ -7,6 +7,8 @@ import ( ) func TestValidBranchRegex(t *testing.T) { + t.Parallel() + valid := []string{ "main", "develop", @@ -40,6 +42,8 @@ func TestValidBranchRegex(t *testing.T) { } func TestValidCommitSHARegex(t *testing.T) { + t.Parallel() + valid := []string{ "abc123def456789012345678901234567890abcd", "0000000000000000000000000000000000000000", @@ -66,7 +70,9 @@ func TestValidCommitSHARegex(t *testing.T) { } } -func TestCloneRepoRejectsInjection(t *testing.T) { +func TestCloneRepoRejectsInjection(t *testing.T) { //nolint:funlen // table-driven test + t.Parallel() + c := &Client{ log: slog.Default(), } @@ -119,6 +125,8 @@ func TestCloneRepoRejectsInjection(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := c.CloneRepo( t.Context(), "git@example.com:repo.git", @@ -131,6 +139,7 @@ func TestCloneRepoRejectsInjection(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } + if !errors.Is(err, tt.wantErr) { t.Errorf("expected error %v, got %v", tt.wantErr, err) } diff --git a/internal/handlers/app.go b/internal/handlers/app.go index b98eedc..46ba020 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -255,6 +255,33 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc { } } +// cleanupContainer stops and removes the Docker container for the given app. +func (h *Handlers) cleanupContainer(ctx context.Context, appID, appName string) { + containerInfo, containerErr := h.docker.FindContainerByAppID(ctx, appID) + if containerErr != nil || containerInfo == nil { + return + } + + if containerInfo.Running { + stopErr := h.docker.StopContainer(ctx, containerInfo.ID) + if stopErr != nil { + h.log.Error("failed to stop container during app deletion", + "error", stopErr, "app", appName, + "container", containerInfo.ID) + } + } + + removeErr := h.docker.RemoveContainer(ctx, containerInfo.ID, true) + if removeErr != nil { + h.log.Error("failed to remove container during app deletion", + "error", removeErr, "app", appName, + "container", containerInfo.ID) + } else { + h.log.Info("removed container during app deletion", + "app", appName, "container", containerInfo.ID) + } +} + // HandleAppDelete handles app deletion. func (h *Handlers) HandleAppDelete() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { @@ -268,27 +295,7 @@ func (h *Handlers) HandleAppDelete() http.HandlerFunc { } // Stop and remove the Docker container before deleting the DB record - containerInfo, containerErr := h.docker.FindContainerByAppID(request.Context(), appID) - if containerErr == nil && containerInfo != nil { - if containerInfo.Running { - stopErr := h.docker.StopContainer(request.Context(), containerInfo.ID) - if stopErr != nil { - h.log.Error("failed to stop container during app deletion", - "error", stopErr, "app", application.Name, - "container", containerInfo.ID) - } - } - - removeErr := h.docker.RemoveContainer(request.Context(), containerInfo.ID, true) - if removeErr != nil { - h.log.Error("failed to remove container during app deletion", - "error", removeErr, "app", application.Name, - "container", containerInfo.ID) - } else { - h.log.Info("removed container during app deletion", - "app", application.Name, "container", containerInfo.ID) - } - } + h.cleanupContainer(request.Context(), appID, application.Name) deleteErr := application.Delete(request.Context()) if deleteErr != nil { @@ -1019,7 +1026,11 @@ func parsePortValues(hostPortStr, containerPortStr string) (int, int, bool) { containerPort, containerErr := strconv.Atoi(containerPortStr) const maxPort = 65535 - if hostErr != nil || containerErr != nil || hostPort <= 0 || containerPort <= 0 || hostPort > maxPort || containerPort > maxPort { + + invalid := hostErr != nil || containerErr != nil || + hostPort <= 0 || containerPort <= 0 || + hostPort > maxPort || containerPort > maxPort + if invalid { return 0, 0, false } diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index b5a3af5..40a890a 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -3,7 +3,6 @@ package handlers import ( "encoding/json" - "html/template" "log/slog" "net/http" @@ -75,7 +74,7 @@ func (h *Handlers) addGlobals( data["Appname"] = h.globals.Appname if request != nil { - data["CSRFField"] = template.HTML(csrf.TemplateField(request)) //nolint:gosec // csrf.TemplateField produces safe HTML + data["CSRFField"] = csrf.TemplateField(request) } return data diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 03de641..efb996f 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -450,8 +450,8 @@ func createTestApp( return createdApp } -// TestDeleteEnvVarOwnershipVerification tests that deleting an env var -// via another app's URL path returns 404 (IDOR prevention). +// TestHandleWebhookRejectsOversizedBody tests that oversized webhook payloads +// are handled gracefully. func TestHandleWebhookRejectsOversizedBody(t *testing.T) { t.Parallel() @@ -493,85 +493,113 @@ func TestHandleWebhookRejectsOversizedBody(t *testing.T) { assert.Equal(t, http.StatusOK, recorder.Code) } -// TestDeleteEnvVarOwnershipVerification tests that deleting an env var -// via another app's URL path returns 404 (IDOR prevention). -func TestDeleteEnvVarOwnershipVerification(t *testing.T) { - t.Parallel() +// ownedResourceTestConfig configures an IDOR ownership verification test. +type ownedResourceTestConfig struct { + appPrefix1 string + appPrefix2 string + createFn func(t *testing.T, tc *testContext, app *models.App) int64 + deletePath func(appID string, resourceID int64) string + chiParams func(appID string, resourceID int64) map[string]string + handler func(h *handlers.Handlers) http.HandlerFunc + verifyFn func(t *testing.T, tc *testContext, resourceID int64) +} + +func testOwnershipVerification(t *testing.T, cfg ownedResourceTestConfig) { + t.Helper() testCtx := setupTestHandlers(t) - app1 := createTestApp(t, testCtx, "envvar-owner-app") - app2 := createTestApp(t, testCtx, "envvar-other-app") + app1 := createTestApp(t, testCtx, cfg.appPrefix1) + app2 := createTestApp(t, testCtx, cfg.appPrefix2) - // Create env var belonging to app1 - envVar := models.NewEnvVar(testCtx.database) - envVar.AppID = app1.ID - envVar.Key = "SECRET" - envVar.Value = "hunter2" - require.NoError(t, envVar.Save(context.Background())) + resourceID := cfg.createFn(t, testCtx, app1) - // Try to delete app1's env var using app2's URL path request := httptest.NewRequest( http.MethodPost, - "/apps/"+app2.ID+"/env/"+strconv.FormatInt(envVar.ID, 10)+"/delete", + cfg.deletePath(app2.ID, resourceID), nil, ) - request = addChiURLParams(request, map[string]string{ - "id": app2.ID, - "envID": strconv.FormatInt(envVar.ID, 10), - }) + request = addChiURLParams(request, cfg.chiParams(app2.ID, resourceID)) + recorder := httptest.NewRecorder() - handler := testCtx.handlers.HandleEnvVarDelete() + handler := cfg.handler(testCtx.handlers) handler.ServeHTTP(recorder, request) - // Should return 404 because the env var doesn't belong to app2 assert.Equal(t, http.StatusNotFound, recorder.Code) + cfg.verifyFn(t, testCtx, resourceID) +} - // Verify the env var was NOT deleted - found, err := models.FindEnvVar(context.Background(), testCtx.database, envVar.ID) - require.NoError(t, err) - assert.NotNil(t, found, "env var should still exist after IDOR attempt") +// TestDeleteEnvVarOwnershipVerification tests that deleting an env var +// via another app's URL path returns 404 (IDOR prevention). +func TestDeleteEnvVarOwnershipVerification(t *testing.T) { //nolint:dupl // intentionally similar IDOR test pattern + t.Parallel() + + testOwnershipVerification(t, ownedResourceTestConfig{ + appPrefix1: "envvar-owner-app", + appPrefix2: "envvar-other-app", + createFn: func(t *testing.T, tc *testContext, ownerApp *models.App) int64 { + t.Helper() + + envVar := models.NewEnvVar(tc.database) + envVar.AppID = ownerApp.ID + envVar.Key = "SECRET" + envVar.Value = "hunter2" + require.NoError(t, envVar.Save(context.Background())) + + return envVar.ID + }, + deletePath: func(appID string, resourceID int64) string { + return "/apps/" + appID + "/env/" + strconv.FormatInt(resourceID, 10) + "/delete" + }, + chiParams: func(appID string, resourceID int64) map[string]string { + return map[string]string{"id": appID, "envID": strconv.FormatInt(resourceID, 10)} + }, + handler: func(h *handlers.Handlers) http.HandlerFunc { return h.HandleEnvVarDelete() }, + verifyFn: func(t *testing.T, tc *testContext, resourceID int64) { + t.Helper() + + found, findErr := models.FindEnvVar(context.Background(), tc.database, resourceID) + require.NoError(t, findErr) + assert.NotNil(t, found, "env var should still exist after IDOR attempt") + }, + }) } // TestDeleteLabelOwnershipVerification tests that deleting a label // via another app's URL path returns 404 (IDOR prevention). -func TestDeleteLabelOwnershipVerification(t *testing.T) { +func TestDeleteLabelOwnershipVerification(t *testing.T) { //nolint:dupl // intentionally similar IDOR test pattern t.Parallel() - testCtx := setupTestHandlers(t) + testOwnershipVerification(t, ownedResourceTestConfig{ + appPrefix1: "label-owner-app", + appPrefix2: "label-other-app", + createFn: func(t *testing.T, tc *testContext, ownerApp *models.App) int64 { + t.Helper() - app1 := createTestApp(t, testCtx, "label-owner-app") - app2 := createTestApp(t, testCtx, "label-other-app") + lbl := models.NewLabel(tc.database) + lbl.AppID = ownerApp.ID + lbl.Key = "traefik.enable" + lbl.Value = "true" + require.NoError(t, lbl.Save(context.Background())) - // Create label belonging to app1 - label := models.NewLabel(testCtx.database) - label.AppID = app1.ID - label.Key = "traefik.enable" - label.Value = "true" - require.NoError(t, label.Save(context.Background())) + return lbl.ID + }, + deletePath: func(appID string, resourceID int64) string { + return "/apps/" + appID + "/labels/" + strconv.FormatInt(resourceID, 10) + "/delete" + }, + chiParams: func(appID string, resourceID int64) map[string]string { + return map[string]string{"id": appID, "labelID": strconv.FormatInt(resourceID, 10)} + }, + handler: func(h *handlers.Handlers) http.HandlerFunc { return h.HandleLabelDelete() }, + verifyFn: func(t *testing.T, tc *testContext, resourceID int64) { + t.Helper() - // Try to delete app1's label using app2's URL path - request := httptest.NewRequest( - http.MethodPost, - "/apps/"+app2.ID+"/labels/"+strconv.FormatInt(label.ID, 10)+"/delete", - nil, - ) - request = addChiURLParams(request, map[string]string{ - "id": app2.ID, - "labelID": strconv.FormatInt(label.ID, 10), + found, findErr := models.FindLabel(context.Background(), tc.database, resourceID) + require.NoError(t, findErr) + assert.NotNil(t, found, "label should still exist after IDOR attempt") + }, }) - recorder := httptest.NewRecorder() - - handler := testCtx.handlers.HandleLabelDelete() - handler.ServeHTTP(recorder, request) - - assert.Equal(t, http.StatusNotFound, recorder.Code) - - // Verify the label was NOT deleted - found, err := models.FindLabel(context.Background(), testCtx.database, label.ID) - require.NoError(t, err) - assert.NotNil(t, found, "label should still exist after IDOR attempt") } // TestDeleteVolumeOwnershipVerification tests that deleting a volume diff --git a/internal/handlers/port_validation_test.go b/internal/handlers/port_validation_test.go index e6d6335..decdc92 100644 --- a/internal/handlers/port_validation_test.go +++ b/internal/handlers/port_validation_test.go @@ -1,8 +1,10 @@ -package handlers +package handlers //nolint:testpackage // tests unexported parsePortValues function import "testing" func TestParsePortValues(t *testing.T) { + t.Parallel() + tests := []struct { name string host string @@ -24,6 +26,8 @@ func TestParsePortValues(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() + host, cont, valid := parsePortValues(tt.host, tt.container) if host != tt.wantHost || cont != tt.wantCont || valid != tt.wantValid { t.Errorf("parsePortValues(%q, %q) = (%d, %d, %v), want (%d, %d, %v)", diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index daf15f1..bc650b8 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -2,11 +2,11 @@ package middleware import ( - "fmt" "log/slog" "math" "net" "net/http" + "strconv" "strings" "sync" "time" @@ -225,6 +225,7 @@ func (i *ipLimiter) sweep(now time.Time) { delete(i.limiters, ip) } } + i.lastSweep = now } @@ -246,6 +247,7 @@ func (i *ipLimiter) getLimiter(ip string) *rate.Limiter { } i.limiters[ip] = entry } + entry.lastSeen = now return entry.limiter @@ -276,11 +278,9 @@ func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler { reservation := limiter.Reserve() delay := reservation.Delay() reservation.Cancel() - retryAfter := int(math.Ceil(delay.Seconds())) - if retryAfter < 1 { - retryAfter = 1 - } - writer.Header().Set("Retry-After", fmt.Sprintf("%d", retryAfter)) + + retryAfter := max(int(math.Ceil(delay.Seconds())), 1) + writer.Header().Set("Retry-After", strconv.Itoa(retryAfter)) http.Error( writer, diff --git a/internal/middleware/ratelimit_test.go b/internal/middleware/ratelimit_test.go index a932ccf..e1f19bc 100644 --- a/internal/middleware/ratelimit_test.go +++ b/internal/middleware/ratelimit_test.go @@ -1,4 +1,4 @@ -package middleware +package middleware //nolint:testpackage // tests unexported types and globals import ( "log/slog" @@ -23,6 +23,7 @@ func newTestMiddleware(t *testing.T) *Middleware { } } +//nolint:paralleltest // mutates global loginLimiter func TestLoginRateLimitAllowsUpToBurst(t *testing.T) { // Reset the global limiter to get clean state loginLimiter = newIPLimiter() @@ -50,6 +51,7 @@ func TestLoginRateLimitAllowsUpToBurst(t *testing.T) { assert.Equal(t, http.StatusTooManyRequests, rec.Code, "6th request should be rate limited") } +//nolint:paralleltest // mutates global loginLimiter func TestLoginRateLimitIsolatesIPs(t *testing.T) { loginLimiter = newIPLimiter() @@ -82,6 +84,7 @@ func TestLoginRateLimitIsolatesIPs(t *testing.T) { assert.Equal(t, http.StatusOK, rec2.Code, "different IP should not be rate limited") } +//nolint:paralleltest // mutates global loginLimiter func TestLoginRateLimitReturns429Body(t *testing.T) { loginLimiter = newIPLimiter() @@ -109,6 +112,8 @@ func TestLoginRateLimitReturns429Body(t *testing.T) { } func TestIPLimiterEvictsStaleEntries(t *testing.T) { + t.Parallel() + il := newIPLimiter() // Add an entry and backdate its lastSeen @@ -130,6 +135,7 @@ func TestIPLimiterEvictsStaleEntries(t *testing.T) { il.mu.Lock() defer il.mu.Unlock() + assert.NotContains(t, il.limiters, "1.2.3.4", "stale entry should be evicted") assert.Contains(t, il.limiters, "5.6.7.8", "fresh entry should remain") } diff --git a/internal/middleware/realip_test.go b/internal/middleware/realip_test.go index 3271a85..8cd1632 100644 --- a/internal/middleware/realip_test.go +++ b/internal/middleware/realip_test.go @@ -1,11 +1,14 @@ -package middleware +package middleware //nolint:testpackage // tests unexported realIP function import ( + "context" "net/http" "testing" ) -func TestRealIP(t *testing.T) { +func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test + t.Parallel() + tests := []struct { name string remoteAddr string @@ -65,11 +68,15 @@ func TestRealIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req, _ := http.NewRequest(http.MethodGet, "/", nil) + t.Parallel() + + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil) req.RemoteAddr = tt.remoteAddr + if tt.xRealIP != "" { req.Header.Set("X-Real-IP", tt.xRealIP) } + if tt.xff != "" { req.Header.Set("X-Forwarded-For", tt.xff) } diff --git a/internal/service/auth/auth_test.go b/internal/service/auth/auth_test.go index 3d1b8a3..156c016 100644 --- a/internal/service/auth/auth_test.go +++ b/internal/service/auth/auth_test.go @@ -71,71 +71,80 @@ func setupTestService(t *testing.T) (*auth.Service, func()) { return svc, cleanup } +func setupAuthService(t *testing.T, debug bool) *auth.Service { + t.Helper() + + tmpDir := t.TempDir() + + globals.SetAppname("upaas-test") + globals.SetVersion("test") + + globalsInst, err := globals.New(fx.Lifecycle(nil)) + require.NoError(t, err) + + loggerInst, err := logger.New( + fx.Lifecycle(nil), + logger.Params{Globals: globalsInst}, + ) + require.NoError(t, err) + + cfg := &config.Config{ + Port: 8080, + DataDir: tmpDir, + SessionSecret: "test-secret-key-at-least-32-chars", + Debug: debug, + } + + dbInst, err := database.New(fx.Lifecycle(nil), database.Params{ + Logger: loggerInst, + Config: cfg, + }) + require.NoError(t, err) + + svc, err := auth.New(fx.Lifecycle(nil), auth.ServiceParams{ + Logger: loggerInst, + Config: cfg, + Database: dbInst, + }) + require.NoError(t, err) + + return svc +} + +func getSessionCookie(t *testing.T, svc *auth.Service) *http.Cookie { + t.Helper() + + _, err := svc.CreateUser(context.Background(), "admin", "password123") + require.NoError(t, err) + + user, err := svc.Authenticate(context.Background(), "admin", "password123") + require.NoError(t, err) + + recorder := httptest.NewRecorder() + request := httptest.NewRequest(http.MethodGet, "/", nil) + + err = svc.CreateSession(recorder, request, user) + require.NoError(t, err) + + for _, c := range recorder.Result().Cookies() { + if c.Name == "upaas_session" { + return c + } + } + + return nil +} + func TestSessionCookieSecureFlag(testingT *testing.T) { testingT.Parallel() testingT.Run("secure flag is true when debug is false", func(t *testing.T) { t.Parallel() - tmpDir := t.TempDir() - - globals.SetAppname("upaas-test") - globals.SetVersion("test") - - globalsInst, err := globals.New(fx.Lifecycle(nil)) - require.NoError(t, err) - - loggerInst, err := logger.New( - fx.Lifecycle(nil), - logger.Params{Globals: globalsInst}, - ) - require.NoError(t, err) - - cfg := &config.Config{ - Port: 8080, - DataDir: tmpDir, - SessionSecret: "test-secret-key-at-least-32-chars", - Debug: false, - } - - dbInst, err := database.New(fx.Lifecycle(nil), database.Params{ - Logger: loggerInst, - Config: cfg, - }) - require.NoError(t, err) - - svc, err := auth.New(fx.Lifecycle(nil), auth.ServiceParams{ - Logger: loggerInst, - Config: cfg, - Database: dbInst, - }) - require.NoError(t, err) - - // Create user and session, check cookie has Secure flag - _, err = svc.CreateUser(context.Background(), "admin", "password123") - require.NoError(t, err) - - user, err := svc.Authenticate(context.Background(), "admin", "password123") - require.NoError(t, err) - - recorder := httptest.NewRecorder() - request := httptest.NewRequest(http.MethodGet, "/", nil) - - err = svc.CreateSession(recorder, request, user) - require.NoError(t, err) - - cookies := recorder.Result().Cookies() - require.NotEmpty(t, cookies) - - var sessionCookie *http.Cookie - for _, c := range cookies { - if c.Name == "upaas_session" { - sessionCookie = c - break - } - } - require.NotNil(t, sessionCookie, "session cookie should exist") - assert.True(t, sessionCookie.Secure, "session cookie should have Secure flag in production mode") + svc := setupAuthService(t, false) + cookie := getSessionCookie(t, svc) + require.NotNil(t, cookie, "session cookie should exist") + assert.True(t, cookie.Secure, "session cookie should have Secure flag in production mode") }) } @@ -302,12 +311,14 @@ func TestCreateUserRaceCondition(testingT *testing.T) { close(start) var successes, failures int + for range goroutines { err := <-results if err == nil { successes++ } else { - assert.ErrorIs(t, err, auth.ErrUserExists) + require.ErrorIs(t, err, auth.ErrUserExists) + failures++ } } diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index a98cf0e..b51e15a 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -89,6 +89,7 @@ func newDeploymentLogWriter(ctx context.Context, deployment *models.Deployment) flushCtx: ctx, } w.flushed.Add(1) + go w.runFlushLoop() return w -- 2.45.2