[client/management/signal/relay] Update go.mod to use Go 1.24.10 and upgrade x/crypto dependencies #4828
Conversation
WalkthroughUpgrade Go toolchain and golang.org/x/* deps to 1.24.10, standardize GitHub Actions to derive Go version from go.mod and adjust checkout ordering, raise WASM size limit to 55 MB, update FreeBSD tarball and gomobile refs, fix a few format-string/logging calls, treat usernames ending with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant IsSys as IsSystemAccount
Note over IsSys: Determine if username is system/computer account
Caller->>IsSys: IsSystemAccount(username)
alt username ends with "$"
IsSys-->>Caller: true
else case-insensitive match in known-list (e.g., "SYSTEM", "NT AUTHORITY\\SYSTEM")
IsSys-->>Caller: true
else
IsSys-->>Caller: false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mlsmaycon
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/golang-test-freebsd.yml (1)
28-28: Consider extracting Go version from go.mod to avoid drift.While the version bump to 1.24.10 is correct, this workflow still hardcodes the version in the tarball filename, unlike other workflows that now use
go-version-file: "go.mod". If go.mod is updated in the future, this hardcoded version could drift out of sync.Consider extracting the version from go.mod dynamically:
prepare: | pkg install -y curl pkgconf xorg - GO_TARBALL="go1.24.10.freebsd-amd64.tar.gz" + GO_VERSION=$(grep -E '^go [0-9]+\.[0-9]+' go.mod | awk '{print $2}') + GO_TARBALL="go${GO_VERSION}.freebsd-amd64.tar.gz" GO_URL="https://go.dev/dl/$GO_TARBALL" curl -vLO "$GO_URL" tar -C /usr/local -vxzf "$GO_TARBALL"Note: This assumes the FreeBSD VM environment has already checked out the repository, which appears to be the case based on the workflow structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
.github/workflows/golang-test-darwin.yml(1 hunks).github/workflows/golang-test-freebsd.yml(1 hunks).github/workflows/golang-test-linux.yml(10 hunks).github/workflows/golang-test-windows.yml(1 hunks).github/workflows/golangci-lint.yml(1 hunks).github/workflows/mobile-build-validation.yml(2 hunks).github/workflows/release.yml(4 hunks).github/workflows/test-infrastructure-files.yml(1 hunks).github/workflows/wasm-build-validation.yml(3 hunks)client/internal/dns/upstream.go(1 hunks)go.mod(4 hunks)management/server/http/handlers/policies/posture_checks_handler_test.go(1 hunks)management/server/posture_checks.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
management/server/http/handlers/policies/posture_checks_handler_test.go (1)
shared/management/status/error.go (3)
Errorf(70-75)InvalidArgument(27-27)Error(54-57)
management/server/posture_checks.go (1)
shared/management/status/error.go (3)
Errorf(70-75)InvalidArgument(27-27)Error(54-57)
🪛 actionlint (1.7.8)
.github/workflows/release.yml
23-23: label "ubuntu-latest-m" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🔇 Additional comments (15)
client/internal/dns/upstream.go (1)
200-200: LGTM - Correct logging call.The change from
WarnftoWarnis correct sincetimeoutMsgis already fully formatted viafmt.Sprintfon lines 195-199. This avoids unnecessary format string parsing..github/workflows/mobile-build-validation.yml (1)
23-23: LGTM - Centralizes Go version management.Switching to
go-version-file: "go.mod"ensures the Go version is consistently derived from the repository's go.mod file across all workflows. The checkout steps are already in place (lines 18-19 and 54-55), so the setup-go action will have access to go.mod.Also applies to: 59-59
.github/workflows/golang-test-windows.yml (1)
27-27: LGTM - Consistent Go version resolution.The change to
go-version-file: "go.mod"aligns with the repository-wide standardization. The checkout step (lines 20-21) ensures go.mod is available before setup-go executes..github/workflows/golangci-lint.yml (1)
49-49: LGTM - Standardizes version resolution.The change to
go-version-file: "go.mod"is consistent with the repository-wide standardization effort. The checkout step (lines 40-41) ensures go.mod is available..github/workflows/golang-test-darwin.yml (2)
18-20: LGTM - Adds necessary checkout step.Adding the checkout step before Install Go is required when using
go-version-file: "go.mod"(line 24), as the setup-go action needs access to go.mod to determine the version.
24-24: LGTM - Aligns with standardized version resolution.The change to
go-version-file: "go.mod"is consistent with other workflows. The checkout step now precedes this (lines 18-20), ensuring go.mod is available..github/workflows/release.yml (1)
43-43: LGTM - Standardizes Go version management.The switch to
go-version-file: "go.mod"across all three release jobs (release, release_ui, release_ui_darwin) centralizes version management and aligns with the repository-wide standardization.Also applies to: 139-139, 203-203
.github/workflows/wasm-build-validation.yml (2)
23-23: LGTM - Consistent version resolution.The switch to
go-version-file: "go.mod"aligns with the repository-wide standardization of Go version management.Also applies to: 48-48
63-64: Threshold increase verified and acceptable.The WASM binary currently measures 51 MB, comfortably within the new 55 MB threshold. The 10% increase (from 50 MB to 55 MB) corresponds to the Go 1.24.10 upgrade and provides a reasonable 4 MB safety margin for future growth. Commit 0f0548e made this change; no production risks identified.
.github/workflows/test-infrastructure-files.yml (1)
70-76: LGTM! Proper checkout sequencing and go.mod-based versioning.The changes ensure code is checked out before Go installation and use go.mod as the source of truth for the Go version, which improves consistency across the project.
.github/workflows/golang-test-linux.yml (1)
109-116: LGTM! Consistent checkout sequencing and go.mod-based versioning.The changes consistently add explicit checkout steps before Go setup and use go.mod as the version source across all jobs, improving workflow reliability and consistency.
Also applies to: 154-161, 223-230, 273-280, 324-331, 412-419, 501-508, 565-572
management/server/posture_checks.go (1)
161-161: LGTM! Fixes potential format string vulnerability.The change from
err.Error()as the format string to using"%s"witherr.Error()as an argument prevents potential format string vulnerabilities if the error message contains format specifiers like %s, %d, etc.management/server/http/handlers/policies/posture_checks_handler_test.go (1)
49-49: LGTM! Test mock aligns with production code.The change maintains consistency with the error formatting fix in management/server/posture_checks.go, ensuring the test mock behavior matches production code.
go.mod (2)
3-3: Go version 1.24.10 is valid and legitimately released.The verification confirms that
go1.24.10is an official Go release available from the Go download API, so the version specification ingo.modis correct and requires no changes.
20-21: ---golang.org/x dependencies verified as valid and secure.
Verification confirms these versions are appropriate: golang.org/x/crypto v0.45.0 is a security patch addressing CVE-2025-58181 and CVE-2025-47914; golang.org/x/net v0.47.0, golang.org/x/sys v0.38.0, and golang.org/x/term v0.37.0 have no known new advisories.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
client/ssh/testutil/user_helpers.go (1)
76-78: Correct detection of Windows computer accounts.The addition of the
$suffix check properly identifies Windows computer/machine accounts as system accounts. This is a well-established Windows naming convention for computer accounts in Active Directory and local domains.However, note that this functional enhancement appears unrelated to the PR's stated objective of updating Go to 1.24.10 and upgrading x/crypto dependencies.
Consider adding a brief comment explaining the Windows computer account convention:
+ // Computer/machine accounts on Windows end with $ if strings.HasSuffix(username, "$") { return true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/ssh/testutil/user_helpers.go(1 hunks)client/ssh/testutil/user_helpers_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
client/ssh/testutil/user_helpers_test.go (1)
client/ssh/testutil/user_helpers.go (1)
IsSystemAccount(62-81)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Client / Unit
- GitHub Check: release_ui_darwin
- GitHub Check: release
- GitHub Check: release_ui
- GitHub Check: JS / Lint
- GitHub Check: test-docker-compose (mysql)
- GitHub Check: Build Cache
- GitHub Check: test-docker-compose (sqlite)
- GitHub Check: test-docker-compose (postgres)
- GitHub Check: Client / Unit
- GitHub Check: test-getting-started-script
- GitHub Check: Linux
- GitHub Check: Darwin
- GitHub Check: Windows
- GitHub Check: Android / Build
🔇 Additional comments (2)
client/ssh/testutil/user_helpers_test.go (2)
17-92: Well-structured and comprehensive test coverage.The test properly validates Windows-specific user identity behavior with:
- Appropriate platform-specific skip
- Use of
requirefor setup andassertfor validation- Clear subtests for SYSTEM account behavior and IsSystemAccount validation
- Comprehensive table-driven test cases covering NT AUTHORITY forms, computer accounts, and regular users
- Good documentation explaining the expected behavior when running as SYSTEM
95-115: Good coverage of computer account formats.The test validates various computer account naming patterns including simple names, domain-qualified, workgroup-qualified, and FQDN formats. All test cases appropriately expect
truefromIsSystemAccount, correctly reflecting that computer accounts ending with$are system accounts.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
.github/workflows/mobile-build-validation.yml(3 hunks)go.mod(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: JS / Lint
- GitHub Check: test-docker-compose (postgres)
- GitHub Check: Darwin
- GitHub Check: iOS / Build
- GitHub Check: test-docker-compose (sqlite)
- GitHub Check: Linux
- GitHub Check: test-docker-compose (mysql)
- GitHub Check: release_ui
- GitHub Check: release_ui_darwin
- GitHub Check: test-getting-started-script
- GitHub Check: Windows
- GitHub Check: Android / Build
- GitHub Check: release
- GitHub Check: Client / Unit
- GitHub Check: Check External GPL/AGPL Licenses
- GitHub Check: Client / Unit
- GitHub Check: Build Cache
🔇 Additional comments (2)
.github/workflows/mobile-build-validation.yml (1)
23-23: Good: standardized Go version resolution via go.mod.Both Android (line 23) and iOS (line 59) jobs now use
go-version-file: "go.mod"instead of explicit version strings. This ensures the CI environment uses the exact Go version specified in the module file, eliminating version drift between local development and CI.Also applies to: 59-59
go.mod (1)
3-3: Solid dependency updates with critical security fixes.The update to Go 1.24.10 (released 2025-11-05) and golang.org/x/crypto v0.45.0 which fixes vulnerabilities in the ssh and ssh/agent packages is well-justified. Go 1.24 maintains the Go 1 promise of compatibility, with almost all Go programs expected to continue compiling and running as before. The consistent updates across all golang.org/x packages are good practice and should improve compatibility.
Please verify that:
- The codebase compiles and tests pass with Go 1.24.10 and the new dependencies.
- No breaking changes from the x/crypto API updates affect the SSH-related code (if any).
Also applies to: 20-20, 21-21, 108-108, 109-109, 110-110, 112-112, 113-113, 254-254, 255-255, 256-256
|
Describe your changes
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Summary by CodeRabbit
Chores
Style / Build
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.