Skip to content

feat: add linting to go backend #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
steveiliop56 wants to merge 2 commits into from
+48 −30
Closed

feat: add linting to go backend #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3da9a5b
feat: add golangci-lint for go linting
steveiliop56 May 9, 2026
5649aea
chore: add linter to ci
steveiliop56 May 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ jobs:
run: |
cp -r frontend/dist internal/assets/dist

- name: Lint backend
uses: golangci/golangci-lint-action@v9
with:
version: v2.12

- name: Run tests
run: go test -coverprofile=coverage.txt -v ./...

Expand Down
17 changes: 17 additions & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
version: "2"
linters:
settings:
errcheck:
exclude-functions:
- (http.ResponseWriter).Write
- (http.ResponseWriter).WriteString
- (github.com/gin-gonic/gin.ResponseWriter).Write
- (github.com/gin-gonic/gin.ResponseWriter).WriteString
exclusions:
rules:
- linters:
- errcheck
text: "//nolint:errcheck"
- linters:
- staticcheck
text: "//nolint:staticcheck"
Comment on lines +10 to +17
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the correct golangci-lint v2 configuration format for exclusions

echo "Checking golangci-lint documentation for v2 configuration..."

# Check if the exclusions.rules.text field is meant for error messages or source patterns
rg -i "exclusions.*rules.*text" --type yaml -C 5

Repository: tinyauthapp/tinyauth

Length of output: 125

🌐 Web query:

golangci-lint v2 configuration format exclusions rules documentation

💡 Result:

golangci-lint v2 uses YAML, TOML, or JSON configuration files (.golangci.yml,.golangci.yaml,.golangci.toml,.golangci.json) with version: "2" at the top.[1][2] Exclusions are configured under the top-level linters.exclusions section.[1][3][4] Key exclusion options: - generated: Controls exclusion of generated files (strict, lax, auto, disable). Default: lax.[1] - warn-unused: Log warning if exclusion rule unused. Default: false.[1] - presets: Predefined exclusion sets like comments, std-error-handling, common-false-positives, legacy.[1][5] - rules: Array of fine-grained exclusion rules matching by path, path-except, text, source, and/or linters (regex patterns).[1][3][4] Example excluding specific linters on test files: linters: exclusions: rules: - path: _test\\.go linters: - gocyclo - errcheck - dupl - gosec [1][4] Example excluding by text and source: - linters: [lll] source: "^//go:generate " [1][4] Example path-except (exclude everywhere except tests): - path-except: _test\\.go linters: [forbidigo] [1] - paths: File/directory paths to exclude issues from (regex, OS separator normalized). Default: [].[1] - paths-except: Paths not to exclude.[1] Patterns use regex (not globs); escape dots like \. in _test\.go. Paths can use placeholders like ${base-path}, ${config-path}.[6][1] Separate exclusions exist for formatters.formatters.exclusions.[5] See official docs for full details: https://golangci-lint.run/docs/configuration/file/ [1][2], false positives: https://golangci-lint.run/docs/linters/false-positives/ [3][4], migration from v1: https://golangci-lint.run/docs/product/migration-guide/ [7]. Reference config: https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml [8].

Citations:

🌐 Web query:

golangci-lint //nolint directive native support how it works

💡 Result:

golangci-lint has native support for //nolint directives. It works by parsing “directive comments” in Go source files and then filtering/removing reported issues that match the directive’s scope (line, block, or file) and the specified linter list. 1) The supported //nolint syntaxes and scopes - Directives are not arbitrary comments; golangci-lint treats them as machine-readable “directive comments” matching the Go directive/comment syntax [1]. - Use exact machine syntax with no spaces in the critical positions, e.g. //nolint:xxx (not // nolint:xxx) [1]. - Excluding all linters: //nolint:all. If used inline (on the same line as the code), it suppresses issues only for that line [1]. Example: var bad_name int //nolint:all [1] - Excluding specific linters: //nolint:, (comma-separated). Example: //nolint:wsl,unused [1]. - Excluding a whole block: place the directive at the beginning of a line, e.g. at the start of a function or before a var block [1]. Example: //nolint:all func... {... } [1] and //nolint:govet var (...) [1] - Excluding whole files: place //nolint: at the beginning of the file, e.g. before the package declaration [1]. Example: //nolint:unparam package pkg [1] 2) How golangci-lint applies the directive to reported issues (how it “works”) - golangci-lint includes a dedicated processing stage for nolint directives (a “nolint filter”), which removes issues from the final results when an issue’s FromLinter and position match a nolint directive in scope. This is tested in the nolint filter unit tests, which construct a nolint filter and assert that issues are removed from results when a directive applies [2]. 3) Validation / correctness of directives (nolintlint) - There is also a “nolintlint” linter that checks that //nolint directives are well-formed and (optionally) properly explained. - The nolintlint package README describes its purpose: ensuring nolint exceptions have explanations and enforcing machine-readable formatting like //nolint:all and specifying which linter is suppressed (e.g. //nolint:gosec) [3]. Practical takeaway - If your goal is to suppress a specific set of warnings, prefer //nolint: (or //nolint:all) rather than disabling via configuration; and place it inline for single-line suppression, at the beginning of a line for block/file suppression [1]. If you want, paste a small code example (where the directive is placed) and I’ll explain exactly which issues it should suppress according to these rules.

Citations:

🏁 Script executed:

cat -n .golangci.yml

Repository: tinyauthapp/tinyauth

Length of output: 635

Remove the exclusions section—it's misconfigured and won't work.

The exclusions.rules configuration uses the text field to match against lint error message text, not source code comments. The literal string "//nolint:errcheck" will never appear in actual lint error messages, so these rules won't exclude anything.

More importantly, golangci-lint natively supports //nolint:<linter> directives in your source code, which already suppress warnings without requiring exclusion rules. Delete the entire exclusions section (lines 10–17).

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.golangci.yml around lines 10 - 17, Remove the misconfigured exclusions
block in the golangci-lint config: delete the entire "exclusions" -> "rules"
entries that use the "text" field with literal "//nolint:errcheck" and
"//nolint:staticcheck", because golangci-lint matches source nolint directives
directly and the "text" matcher targets lint messages, not source comments;
simply remove the "exclusions" section (keys: exclusions, rules, linters, text)
so the linter uses native //nolint:<linter> handling.

5 changes: 1 addition & 4 deletions cmd/tinyauth/generate_totp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ func generateTotpCmd() *cli.Command {
return fmt.Errorf("failed to parse user: %w", err)
}

docker := false
if strings.Contains(tCfg.User, "$$") {
docker = true
}
docker := strings.Contains(tCfg.User, "$$")

if user.TOTPSecret != "" {
return fmt.Errorf("user already has a TOTP secret")
Expand Down
6 changes: 3 additions & 3 deletions cmd/tinyauth/healthcheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"os"
"time"

"github.com/tinyauthapp/tinyauth/internal/utils/tlog"
"github.com/tinyauthapp/paerser/cli"
"github.com/tinyauthapp/tinyauth/internal/utils/tlog"
)

type healthzResponse struct {
Expand Down Expand Up @@ -45,7 +45,7 @@ func healthcheckCmd() *cli.Command {
}

if appUrl == "" {
return errors.New("Could not determine app URL")
return errors.New("could not determine app url")
}

tlog.App.Info().Str("app_url", appUrl).Msg("Performing health check")
Expand All @@ -70,7 +70,7 @@ func healthcheckCmd() *cli.Command {
return fmt.Errorf("service is not healthy, got: %s", resp.Status)
}

defer resp.Body.Close()
defer resp.Body.Close() //nolint:errcheck

var healthResp healthzResponse

Expand Down
2 changes: 1 addition & 1 deletion internal/bootstrap/app_bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ func (app *BootstrapApp) heartbeatRoutine() {
continue
}

res.Body.Close()
res.Body.Close() //nolint:errcheck
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: Missing defer keyword on response body close.

The defer keyword is missing from the res.Body.Close() call. Without defer, the body closes immediately on line 295, before the status code is checked on line 297. While reading StatusCode doesn't strictly require an open body, this breaks the standard pattern and could cause issues if the code is modified later. Response bodies should always be deferred closed to ensure proper cleanup across all execution paths.

🐛 Proposed fix 
-		res.Body.Close() //nolint:errcheck
+		defer res.Body.Close() //nolint:errcheck
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
res.Body.Close() //nolint:errcheck
defer res.Body.Close() //nolint:errcheck
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/bootstrap/app_bootstrap.go` at line 295, The response body is being
closed immediately with res.Body.Close() instead of deferring it; change the
call to defer res.Body.Close() (keeping the //nolint:errcheck if desired) so the
body is closed at function return and not before subsequent checks—locate the
res variable and the res.Body.Close() call in the bootstrap logic and replace it
with deferred close.


if res.StatusCode != 200 && res.StatusCode != 201 {
tlog.App.Debug().Str("status", res.Status).Msg("Heartbeat returned non-200/201 status")
Expand Down
2 changes: 1 addition & 1 deletion internal/bootstrap/service_bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (app *BootstrapApp) initServices(queries *repository.Queries) (Services, er

if err != nil {
tlog.App.Warn().Err(err).Msg("Failed to setup LDAP service, starting without it")
ldapService.Unconfigure()
ldapService.Unconfigure() //nolint:errcheck
}

services.ldapService = ldapService
Expand Down
6 changes: 6 additions & 0 deletions internal/controller/oauth_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {

user, err := controller.auth.GetOAuthUserinfo(sessionIdCookie)

if err != nil {
tlog.App.Error().Err(err).Msg("Failed to get user info from OAuth provider")
c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
return
}

if user.Email == "" {
tlog.App.Error().Msg("OAuth provider did not return an email")
c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/error", controller.config.AppURL))
Expand Down
12 changes: 6 additions & 6 deletions internal/controller/user_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func TestUserController(t *testing.T) {
{
description: "Should rate limit on 3 invalid attempts",
middlewares: []gin.HandlerFunc{},
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { //nolint:staticcheck
loginReq := controller.LoginRequest{
Username: "testuser",
Password: "wrongpassword",
Expand All @@ -201,7 +201,7 @@ func TestUserController(t *testing.T) {
}

// 4th attempt should be rate limited
recorder = httptest.NewRecorder()
recorder = httptest.NewRecorder() //nolint:staticcheck
req := httptest.NewRequest("POST", "/api/user/login", strings.NewReader(string(loginReqBody)))
req.Header.Set("Content-Type", "application/json")

Expand Down Expand Up @@ -293,7 +293,7 @@ func TestUserController(t *testing.T) {
middlewares: []gin.HandlerFunc{
totpCtx,
},
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { //nolint:staticcheck
_, err := queries.CreateSession(context.TODO(), repository.CreateSessionParams{
UUID: "test-totp-login-uuid",
Username: "test",
Expand All @@ -316,7 +316,7 @@ func TestUserController(t *testing.T) {
totpReqBody, err := json.Marshal(totpReq)
assert.NoError(t, err)

recorder = httptest.NewRecorder()
recorder = httptest.NewRecorder() //nolint:staticcheck
req := httptest.NewRequest("POST", "/api/user/totp", strings.NewReader(string(totpReqBody)))
req.Header.Set("Content-Type", "application/json")
req.AddCookie(&http.Cookie{
Expand Down Expand Up @@ -345,7 +345,7 @@ func TestUserController(t *testing.T) {
middlewares: []gin.HandlerFunc{
totpCtx,
},
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { //nolint:staticcheck
for range 3 {
totpReq := controller.TotpRequest{
Code: "000000", // invalid code
Expand All @@ -354,7 +354,7 @@ func TestUserController(t *testing.T) {
totpReqBody, err := json.Marshal(totpReq)
assert.NoError(t, err)

recorder = httptest.NewRecorder()
recorder = httptest.NewRecorder() //nolint:staticcheck
req := httptest.NewRequest("POST", "/api/user/totp", strings.NewReader(string(totpReqBody)))
req.Header.Set("Content-Type", "application/json")

Expand Down
2 changes: 1 addition & 1 deletion internal/middleware/context_middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (m *ContextMiddleware) cookieAuth(ctx context.Context, uuid string) (*model
}

if !m.auth.IsEmailWhitelisted(userContext.OAuth.Email) {
m.auth.DeleteSession(ctx, uuid)
m.auth.DeleteSession(ctx, uuid) //nolint:errcheck
return nil, nil, fmt.Errorf("email from session cookie not whitelisted: %s", userContext.OAuth.Email)
}
}
Expand Down
7 changes: 0 additions & 7 deletions internal/service/auth_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,10 +845,3 @@ func (auth *AuthService) ClearRateLimitsTestingOnly() {
}
auth.loginMutex.Unlock()
}

func (auth *AuthService) getCookieDomain() string {
if auth.config.SubdomainsEnabled {
return "." + auth.config.CookieDomain
}
return auth.config.CookieDomain
}
2 changes: 1 addition & 1 deletion internal/service/ldap_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func (ldap *LdapService) reconnect() error {
exp.Reset()

operation := func() (*ldapgo.Conn, error) {
ldap.conn.Close()
ldap.conn.Close() //nolint:errcheck
conn, err := ldap.connect()
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion internal/service/oauth_extractors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func simpleReq[T any](client *http.Client, url string, headers map[string]string
if err != nil {
return nil, err
}
defer res.Body.Close()
defer res.Body.Close() //nolint:errcheck

if res.StatusCode < 200 || res.StatusCode >= 300 {
return nil, fmt.Errorf("request failed with status: %s", res.Status)
Expand Down
2 changes: 1 addition & 1 deletion internal/utils/fs_utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestReadFile(t *testing.T) {

err = file.Close()
require.NoError(t, err)
defer os.Remove("/tmp/tinyauth_test_file")
defer os.Remove("/tmp/tinyauth_test_file") //nolint:errcheck
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid fixed /tmp filename in tests.

Line 21 uses cleanup for a hardcoded path, which can conflict with parallel test execution. Use t.TempDir() for isolation and keep the same deferred cleanup pattern.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/utils/fs_utils_test.go` at line 21, Replace the hardcoded
"/tmp/tinyauth_test_file" cleanup with a test-scoped temp directory from
t.TempDir(); create the test file path via filepath.Join(t.TempDir(),
"tinyauth_test_file") and update the deferred cleanup to defer
os.Remove(tempFilePath) (or drop the defer since t.TempDir() auto-cleans),
ensuring you add an import for "path/filepath" if needed and update any
references to the old hardcoded path (the os.Remove("/tmp/tinyauth_test_file")
call) accordingly.


// Normal case
content, err := ReadFile("/tmp/tinyauth_test_file")
Expand Down
2 changes: 1 addition & 1 deletion internal/utils/security_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func FilterIP(filter string, ip string) (bool, error) {
return false, errors.New("invalid IP address")
}

filter = strings.Replace(filter, "-", "/", -1)
filter = strings.ReplaceAll(filter, "-", "/")

if strings.Contains(filter, "/") {
_, cidr, err := net.ParseCIDR(filter)
Expand Down
2 changes: 1 addition & 1 deletion internal/utils/security_utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestGetSecret(t *testing.T) {

err = file.Close()
require.NoError(t, err)
defer os.Remove("/tmp/tinyauth_test_secret")
defer os.Remove("/tmp/tinyauth_test_secret") //nolint:errcheck
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use test-scoped temp files instead of a fixed /tmp path.

Line 22 still cleans up a globally fixed file path, which can collide across concurrent test runs and cause flakes. Prefer t.TempDir() + a unique path per test.

Suggested change 
- file, err := os.Create("/tmp/tinyauth_test_secret")
+ tmpDir := t.TempDir()
+ secretPath := tmpDir + "/tinyauth_test_secret"
+ file, err := os.Create(secretPath)
  require.NoError(t, err)

  // ...
- defer os.Remove("/tmp/tinyauth_test_secret") //nolint:errcheck
+ defer os.Remove(secretPath) //nolint:errcheck

- assert.Equal(t, "secret", utils.GetSecret("", "/tmp/tinyauth_test_secret"))
+ assert.Equal(t, "secret", utils.GetSecret("", secretPath))
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/utils/security_utils_test.go` at line 22, Replace the hard-coded
"/tmp/tinyauth_test_secret" cleanup with a test-scoped temp path: call
t.TempDir() in the test, construct the secret path via filepath.Join(tmpDir,
"tinyauth_test_secret"), use that path wherever the test currently references
the fixed string, and remove the explicit os.Remove defer (or change it to defer
os.Remove(secretPath) if you still want explicit cleanup); also add an import
for "path/filepath" if missing. This ensures the test uses an isolated temp
directory rather than a global /tmp path.


// Get from config
assert.Equal(t, "mysecret", utils.GetSecret("mysecret", ""))
Expand Down
2 changes: 1 addition & 1 deletion internal/utils/string_utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestGetStringList(t *testing.T) {

err = file.Close()
assert.NoError(t, err)
defer os.Remove("/tmp/tinyauth_list_test_file")
defer os.Remove("/tmp/tinyauth_list_test_file") //nolint:errcheck

values, err := utils.GetStringList([]string{" first@example.com ", "", "second@example.com"}, "/tmp/tinyauth_list_test_file")
assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/utils/user_utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestGetUsers(t *testing.T) {

err = file.Close()
require.NoError(t, err)
defer os.Remove(tmpDir + "/tinyauth_users_test.txt")
defer os.Remove(tmpDir + "/tinyauth_users_test.txt") //nolint:errcheck

noAttrs := map[string]model.UserAttributes{}

Expand Down