Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 7 additions & 7 deletions .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
FROM golang:1.23-bullseye
FROM golang:1.25-bookworm

RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
&& apt-get -y install --no-install-recommends\
gettext-base=0.21-4 \
iptables=1.8.7-1 \
libgl1-mesa-dev=20.3.5-1 \
xorg-dev=1:7.7+22 \
libayatana-appindicator3-dev=0.5.5-2+deb11u2 \
gettext-base=0.21-12 \
iptables=1.8.9-2 \
libgl1-mesa-dev=22.3.6-1+deb12u1 \
xorg-dev=1:7.7+23 \
libayatana-appindicator3-dev=0.5.92-1 \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* \
&& go install -v golang.org/x/tools/gopls@v0.18.1
&& go install -v golang.org/x/tools/gopls@latest


WORKDIR /app
2 changes: 1 addition & 1 deletion .github/workflows/golang-test-freebsd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
release: "14.2"
prepare: |
pkg install -y curl pkgconf xorg
GO_TARBALL="go1.24.10.freebsd-amd64.tar.gz"
GO_TARBALL="go1.25.3.freebsd-amd64.tar.gz"
GO_URL="https://go.dev/dl/$GO_TARBALL"
curl -vLO "$GO_URL"
tar -C /usr/local -vxzf "$GO_TARBALL"
Comment on lines +28 to 31
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find and examine the workflow files mentioned
find .github/workflows -name "*.yml" -o -name "*.yaml" | head -20

Repository: netbirdio/netbird

Length of output: 707


🏁 Script executed:

# Check the specific file mentioned in the review
cat -n .github/workflows/golang-test-freebsd.yml | sed -n '25,35p'

Repository: netbirdio/netbird

Length of output: 536


🏁 Script executed:

# Search for release.yml and check its Go version
find .github -name "*release*" -type f

Repository: netbirdio/netbird

Length of output: 90


🏁 Script executed:

# Search for all Go version references in workflow files
rg "go[0-9]\.[0-9]+\.[0-9]+" .github/workflows/ -o

Repository: netbirdio/netbird

Length of output: 150


🏁 Script executed:

# Check release.yml for Go version context
cat -n .github/workflows/release.yml | grep -B5 -A5 "1.25.5"

Repository: netbirdio/netbird

Length of output: 532


🏁 Script executed:

# Check all workflow files for Go version references more comprehensively
rg "go1\.25\." .github/workflows/ -B2 -A2

Repository: netbirdio/netbird

Length of output: 803


🌐 Web query:

Go 1.25.3 Go 1.25.5 release available download

💡 Result:

Yes — both Go 1.25.3 and Go 1.25.5 binary and source downloads are available on the official Go downloads page. See the download listings for go1.25.5 (and other 1.25.x releases) on the Go site. [1][2]

Sources:

  • Official Go downloads page (lists go1.25.5 and 1.25.x artifacts). [1]
  • Go release history / release notes (go1.25.3 and go1.25.5 entries). [2]

(If you want direct download links or checksums for a specific OS/arch, tell me which one.)


Align Go versions across workflows: golang-test-freebsd.yml uses go1.25.3 while release.yml uses go1.25.5.

Both versions exist and are available, but this inconsistency should be resolved. Either align both workflows to use the same version (likely go1.25.5 as the more recent patch) or document why the versions differ.

🤖 Prompt for AI Agents
In @.github/workflows/golang-test-freebsd.yml around lines 28 - 31, The workflow
sets GO_TARBALL/GO_URL to "go1.25.3.freebsd-amd64.tar.gz"/"$GO_TARBALL" which
mismatches release.yml's go1.25.5; update the GO_TARBALL and any references to
use go1.25.5 (e.g., "go1.25.5.freebsd-amd64.tar.gz" and corresponding GO_URL) so
both workflows use the same Go patch release, or add a comment/documentation
near the GO_TARBALL/GO_URL variables explaining and justifying the intentional
version difference.

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/golang-test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ jobs:
-e GOCACHE=${CONTAINER_GOCACHE} \
-e GOMODCACHE=${CONTAINER_GOMODCACHE} \
-e CONTAINER=${CONTAINER} \
golang:1.24-alpine \
golang:1.25-alpine \
sh -c ' \
apk update; apk add --no-cache \
ca-certificates iptables ip6tables dbus dbus-dev libpcap-dev build-base; \
Expand Down Expand Up @@ -259,7 +259,7 @@ jobs:
CGO_ENABLED=1 GOARCH=${{ matrix.arch }} \
go test ${{ matrix.raceFlag }} \
-exec 'sudo' \
-timeout 10m ./relay/... ./shared/relay/...
-timeout 10m -p 1 ./relay/... ./shared/relay/...

test_signal:
name: "Signal / Unit"
Expand Down
7 changes: 5 additions & 2 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ jobs:
if: matrix.os == 'ubuntu-latest'
run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev libpcap-dev
- name: golangci-lint
uses: golangci/golangci-lint-action@v4
uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0
with:
version: latest
args: --timeout=12m --out-format colored-line-number
skip-cache: true
skip-save-cache: true
cache-invalidation-interval: 0
args: --timeout=12m
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
pkg install -y git curl portlint go

# Install Go for building
GO_TARBALL="go1.24.10.freebsd-amd64.tar.gz"
GO_TARBALL="go1.25.5.freebsd-amd64.tar.gz"
GO_URL="https://go.dev/dl/$GO_TARBALL"
curl -LO "$GO_URL"
tar -C /usr/local -xzf "$GO_TARBALL"
Expand Down
13 changes: 7 additions & 6 deletions .github/workflows/wasm-build-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ jobs:
js_lint:
name: "JS / Lint"
runs-on: ubuntu-latest
env:
GOOS: js
GOARCH: wasm
steps:
- name: Checkout repository
uses: actions/checkout@v4
Expand All @@ -24,16 +27,14 @@ jobs:
- name: Install dependencies
run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev libpcap-dev
- name: Install golangci-lint
uses: golangci/golangci-lint-action@d6238b002a20823d52840fda27e2d4891c5952dc
uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the commit hash for v8.0.0 tag
cd /tmp && rm -rf golangci-lint-action
git clone --depth 100 https://github.com/golangci/golangci-lint-action.git 2>&1 | grep -E "(Cloning|done)"
cd golangci-lint-action
git fetch origin tag v8.0.0 --no-tags 2>&1 | grep -E "(Fetching|error|fatal)"
echo "=== v8.0.0 tag commit hash ==="
git rev-parse v8.0.0
echo "=== Expected commit hash ==="
echo "4afd733a84b1f43292c63897423277bb7f4313a9"
echo "=== Match result ==="
if [ "$(git rev-parse v8.0.0)" = "4afd733a84b1f43292c63897423277bb7f4313a9" ]; then
  echo "✓ MATCH: Hash corresponds to v8.0.0"
else
  echo "✗ MISMATCH: Hash does not correspond to v8.0.0"
fi

Repository: netbirdio/netbird

Length of output: 309


🏁 Script executed:

#!/bin/bash
# Find what tag/version the actual commit hash belongs to
cd /tmp/golangci-lint-action
echo "=== Tags for commit 4afd733a84b1f43292c63897423277bb7f4313a9 ==="
git tag --contains 4afd733a84b1f43292c63897423277bb7f4313a9 2>/dev/null | head -20
echo ""
echo "=== Commit date and message ==="
git log -1 --format="%H %ai %s" 4afd733a84b1f43292c63897423277bb7f4313a9
echo ""
echo "=== All tags in repo (last 20) ==="
git tag --sort=-version:refname | head -20

Repository: netbirdio/netbird

Length of output: 352


Update golangci-lint-action to pin to the actual v8.0.0 commit or correct the version comment.

The commit hash 4afd733a84b1f43292c63897423277bb7f4313a9 does not correspond to the v8.0.0 release. The actual v8.0.0 tag points to commit 25e2cdc5eb1d7a04fdc45ff538f1a00e960ae128. The pinned commit predates the v8.0.0 release and should either be updated to the correct v8.0.0 commit or the version comment should be corrected to reflect the actual version being used.

🤖 Prompt for AI Agents
In @.github/workflows/wasm-build-validation.yml at line 30, The workflow pins
golangci-lint-action to commit 4afd733a84b1f43292c63897423277bb7f4313a9 while
commenting v8.0.0; update the uses entry to point to the actual v8.0.0 commit
hash 25e2cdc5eb1d7a04fdc45ff538f1a00e960ae128 (i.e., replace the existing commit
in the uses value) or, if you intentionally want a different commit, change the
trailing comment to the correct version tag to avoid the mismatch.

with:
version: latest
install-mode: binary
skip-cache: true
skip-pkg-cache: true
skip-build-cache: true
- name: Run golangci-lint for WASM
run: |
GOOS=js GOARCH=wasm golangci-lint run --timeout=12m --out-format colored-line-number ./client/...
skip-save-cache: true
cache-invalidation-interval: 0
working-directory: ./client
continue-on-error: true

js_build:
Expand Down
255 changes: 120 additions & 135 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -1,139 +1,124 @@
run:
# Timeout for analysis, e.g. 30s, 5m.
# Default: 1m
timeout: 6m

# This file contains only configs which differ from defaults.
# All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml
linters-settings:
errcheck:
# Report about not checking of errors in type assertions: `a := b.(MyStruct)`.
# Such cases aren't reported by default.
# Default: false
check-type-assertions: false

gosec:
includes:
- G101 # Look for hard coded credentials
#- G102 # Bind to all interfaces
- G103 # Audit the use of unsafe block
- G104 # Audit errors not checked
- G106 # Audit the use of ssh.InsecureIgnoreHostKey
#- G107 # Url provided to HTTP request as taint input
- G108 # Profiling endpoint automatically exposed on /debug/pprof
- G109 # Potential Integer overflow made by strconv.Atoi result conversion to int16/32
- G110 # Potential DoS vulnerability via decompression bomb
- G111 # Potential directory traversal
#- G112 # Potential slowloris attack
- G113 # Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772)
#- G114 # Use of net/http serve function that has no support for setting timeouts
- G201 # SQL query construction using format string
- G202 # SQL query construction using string concatenation
- G203 # Use of unescaped data in HTML templates
#- G204 # Audit use of command execution
- G301 # Poor file permissions used when creating a directory
- G302 # Poor file permissions used with chmod
- G303 # Creating tempfile using a predictable path
- G304 # File path provided as taint input
- G305 # File traversal when extracting zip/tar archive
- G306 # Poor file permissions used when writing to a new file
- G307 # Poor file permissions used when creating a file with os.Create
#- G401 # Detect the usage of DES, RC4, MD5 or SHA1
#- G402 # Look for bad TLS connection settings
- G403 # Ensure minimum RSA key length of 2048 bits
#- G404 # Insecure random number source (rand)
#- G501 # Import blocklist: crypto/md5
- G502 # Import blocklist: crypto/des
- G503 # Import blocklist: crypto/rc4
- G504 # Import blocklist: net/http/cgi
#- G505 # Import blocklist: crypto/sha1
- G601 # Implicit memory aliasing of items from a range statement
- G602 # Slice access out of bounds

gocritic:
disabled-checks:
- commentFormatting
- captLocal
- deprecatedComment

govet:
# Enable all analyzers.
# Default: false
enable-all: false
enable:
- nilness

revive:
rules:
- name: exported
severity: warning
disabled: false
arguments:
- "checkPrivateReceivers"
- "sayRepetitiveInsteadOfStutters"
tenv:
# The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures.
# Otherwise, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked.
# Default: false
all: true

version: "2"
linters:
disable-all: true
default: none
enable:
## enabled by default
- errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases
- gosimple # specializes in simplifying a code
- govet # reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
- ineffassign # detects when assignments to existing variables are not used
- staticcheck # is a go vet on steroids, applying a ton of static analysis checks
- tenv # Tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17.
- typecheck # like the front-end of a Go compiler, parses and type-checks Go code
- unused # checks for unused constants, variables, functions and types
## disable by default but the have interesting results so lets add them
- bodyclose # checks whether HTTP response body is closed successfully
- dupword # dupword checks for duplicate words in the source code
- durationcheck # durationcheck checks for two durations multiplied together
- forbidigo # forbidigo forbids identifiers
- gocritic # provides diagnostics that check for bugs, performance and style issues
- gosec # inspects source code for security problems
- mirror # mirror reports wrong mirror patterns of bytes/strings usage
- misspell # misspess finds commonly misspelled English words in comments
- nilerr # finds the code that returns nil even if it checks that the error is not nil
- nilnil # checks that there is no simultaneous return of nil error and an invalid value
- predeclared # predeclared finds code that shadows one of Go's predeclared identifiers
- revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint.
- sqlclosecheck # checks that sql.Rows and sql.Stmt are closed
# - thelper # thelper detects Go test helpers without t.Helper() call and checks the consistency of test helpers.
- wastedassign # wastedassign finds wasted assignment statements
- bodyclose
- dupword
- durationcheck
- errcheck
- forbidigo
- gocritic
- gosec
- govet
- ineffassign
- mirror
- misspell
- nilerr
- nilnil
- predeclared
- revive
- sqlclosecheck
- staticcheck
- unused
- wastedassign
settings:
errcheck:
check-type-assertions: false
gocritic:
disabled-checks:
- commentFormatting
- captLocal
- deprecatedComment
gosec:
includes:
- G101
- G103
- G104
- G106
- G108
- G109
- G110
- G111
- G201
- G202
- G203
- G301
- G302
- G303
- G304
- G305
- G306
- G307
- G403
- G502
- G503
- G504
- G601
- G602
govet:
enable:
- nilness
enable-all: false
revive:
rules:
- name: exported
arguments:
- checkPrivateReceivers
- sayRepetitiveInsteadOfStutters
severity: warning
disabled: false
exclusions:
generated: lax
presets:
- comments
- common-false-positives
- legacy
- std-error-handling
rules:
- linters:
- forbidigo
path: management/cmd/root\.go
- linters:
- forbidigo
path: signal/cmd/root\.go
- linters:
- unused
path: sharedsock/filter\.go
- linters:
- unused
path: client/firewall/iptables/rule\.go
- linters:
- gosec
- mirror
path: test\.go
- linters:
- nilnil
path: mock\.go
- linters:
- staticcheck
text: grpc.DialContext is deprecated
- linters:
- staticcheck
text: grpc.WithBlock is deprecated
- linters:
- staticcheck
text: "QF1001"
- linters:
- staticcheck
text: "QF1008"
- linters:
- staticcheck
text: "QF1012"
paths:
- third_party$
- builtin$
- examples$
issues:
# Maximum count of issues with the same text.
# Set to 0 to disable.
# Default: 3
max-same-issues: 5

exclude-rules:
# allow fmt
- path: management/cmd/root\.go
linters: forbidigo
- path: signal/cmd/root\.go
linters: forbidigo
- path: sharedsock/filter\.go
linters:
- unused
- path: client/firewall/iptables/rule\.go
linters:
- unused
- path: test\.go
linters:
- mirror
- gosec
- path: mock\.go
linters:
- nilnil
# Exclude specific deprecation warnings for grpc methods
- linters:
- staticcheck
text: "grpc.DialContext is deprecated"
- linters:
- staticcheck
text: "grpc.WithBlock is deprecated"
formatters:
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$
1 change: 1 addition & 0 deletions client/cmd/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ func setLogLevel(cmd *cobra.Command, args []string) error {
client := proto.NewDaemonServiceClient(conn)
level := server.ParseLogLevel(args[0])
if level == proto.LogLevel_UNKNOWN {
//nolint
return fmt.Errorf("unknown log level: %s. Available levels are: panic, fatal, error, warn, info, debug, trace\n", args[0])
}

Expand Down
2 changes: 2 additions & 0 deletions client/cmd/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ var loginCmd = &cobra.Command{
func doDaemonLogin(ctx context.Context, cmd *cobra.Command, providedSetupKey string, activeProf *profilemanager.Profile, username string, pm *profilemanager.ProfileManager) error {
conn, err := DialClientGRPCServer(ctx, daemonAddr)
if err != nil {
//nolint
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n client/cmd/login.go | sed -n '75,95p'

Repository: netbirdio/netbird

Length of output: 886


🏁 Script executed:

rg -n "//nolint" --type=go -C 2 | head -100

Repository: netbirdio/netbird

Length of output: 6128


🏁 Script executed:

fd -e "golangci.yml" -e ".golangci.yml" -e ".golangci.yaml" -e "golangci.yaml" -e ".lintignore"

Repository: netbirdio/netbird

Length of output: 43


🏁 Script executed:

rg -n "fmt\.Errorf" --type=go -C 1 | grep -A 2 -B 2 "nolint" | head -50

Repository: netbirdio/netbird

Length of output: 1432


Use a specific nolint directive to clarify the suppressed rule.

The generic //nolint directive suppresses all lint warnings on this line. Consider replacing it with a more specific directive like //nolint:lll (for line length) to make the intent clear and prevent accidental suppression of other violations. This pattern appears throughout the codebase (e.g., client/cmd/root.go:393, client/cmd/up.go:219), so standardizing on a specific rule would improve code clarity.

🤖 Prompt for AI Agents
In @client/cmd/login.go at line 84, Replace the generic `//nolint` on the line
containing the `//nolint` token with a rule-specific nolint (e.g.,
`//nolint:lll` for long lines) so only the intended linter check is suppressed;
identify the exact lint being avoided by running your linter or checking similar
instances (e.g., `client/cmd/root.go` and `client/cmd/up.go` use `//nolint:lll`)
and change the comment to `//nolint:<rule>` to narrowly scope the suppression.

return fmt.Errorf("failed to connect to daemon error: %v\n"+
"If the daemon is not running please run: "+
"\nnetbird service install \nnetbird service start\n", err)
Expand Down Expand Up @@ -206,6 +207,7 @@ func switchProfileOnDaemon(ctx context.Context, pm *profilemanager.ProfileManage
func switchProfile(ctx context.Context, profileName string, username string) error {
conn, err := DialClientGRPCServer(ctx, daemonAddr)
if err != nil {
//nolint
return fmt.Errorf("failed to connect to daemon error: %v\n"+
"If the daemon is not running please run: "+
"\nnetbird service install \nnetbird service start\n", err)
Expand Down
1 change: 0 additions & 1 deletion client/cmd/pprof.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build pprof
// +build pprof

package cmd

Expand Down
Loading
Loading