SVI/netbird
1
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-08-04 19:55:09 -04:00
Code Issues 3.0k Packages Projects Releases 10 Wiki Activity

[PR #4173] [management] Handle missing extra settings in updateAccount #4125

New Issue
Open
opened 2025-11-20 08:07:46 -05:00 by saavagebueno · 0 comments
saavagebueno commented 2025-11-20 08:07:46 -05:00
Owner
Copy Link

Original Pull Request: https://github.com/netbirdio/netbird/pull/4173

State: open
Merged: No

Describe your changes

If you send an "update an account" via the REST API without the required field json object "extra", a panic that crashes the API service will happen. See the stacktrace below.
This can be problematic for backwards-compatibility reasons, since in older versions of the REST-API, this field was optional. Having the management API service crash is not acceptable imo.

This fix makes the REST API throw a "400 BadRequest" when the required JSON field is missing.
An alternative fix could be to set default values for the extra required fields.

Issue ticket number and link

Stack

2025/07/04 09:45:00 http: panic serving 172.18.0.1:52232: runtime error: invalid memory address or nil pointer dereference
goroutine 166 [running]:
net/http.(*conn).serve.func1()
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:1947 +0xbe
panic({0x1aa7ac0?, 0x2d91c00?})
        /opt/hostedtoolcache/go/1.23.10/x64/src/runtime/panic.go:791 +0x132
gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func17({0x20001b0, 0x2e67a20}, {0x1d31360?, 0xc000894540?, 0xc0007fc600?}, {0x1ad6000?, 0xc0002b81c0?})
        /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/schema/field.go:964 +0x112
gorm.io/gorm/callbacks.ConvertToAssignments.func2(0x1a86920?, {0x1ad6000?, 0xc0002b81c0?})
        /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks/update.go:149 +0x55
gorm.io/gorm/callbacks.ConvertToAssignments(0xc000894380)
        /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks/update.go:288 +0xd66
gorm.io/gorm/callbacks.RegisterDefaultCallbacks.Update.func7(0xc000866ae0)
        /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks/update.go:74 +0x1ac
gorm.io/gorm.(*processor).Execute(0xc00029cfa0, 0xc0007a9cb0?)
        /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks.go:130 +0x3cb
gorm.io/gorm.(*DB).Updates(0xc00042f662?, {0x194b3e0, 0xc00014e020})
        /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/finisher_api.go:411 +0x92
github.com/netbirdio/netbird/management/server/store.(*SqlStore).SaveAccountSettings(0xc0001196e0, {0x20000d0, 0xc000816150}, {0x1d87b4a?, 0xc000816150?}, {0xc00042f662, 0x14}, 0xc000807200)
        /home/runner/work/netbird/netbird/management/server/store/sql_store.go:2171 +0x218
github.com/netbirdio/netbird/management/server.(*DefaultAccountManager).UpdateAccountSettings.func1({0x2024518, 0xc0001196e0})
        /home/runner/work/netbird/netbird/management/server/account.go:330 +0x394
github.com/netbirdio/netbird/management/server/store.(*SqlStore).ExecuteInTransaction(0xc000501920, {0x20000d0, 0xc000816150}, 0xc0002c79d0)
        /home/runner/work/netbird/netbird/management/server/store/sql_store.go:1598 +0xe7
github.com/netbirdio/netbird/management/server.(*DefaultAccountManager).UpdateAccountSettings(0xc0001a6f20, {0x20000d0, 0xc000816150}, {0xc00042f662, 0x14}, {0xc0007abd20, 0x1e}, 0xc000807200)
        /home/runner/work/netbird/netbird/management/server/account.go:299 +0x36f
github.com/netbirdio/netbird/management/server/http/handlers/accounts.(*handler).updateAccount(0xc0002ef800, {0x1ffce00, 0xc0001122a0}, 0xc0002a5180)
        /home/runner/work/netbird/netbird/management/server/http/handlers/accounts/accounts_handler.go:129 +0x54c
net/http.HandlerFunc.ServeHTTP(0xc0005a6570?, {0x1ffce00?, 0xc0001122a0?}, 0xc00098a890?)
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29
github.com/netbirdio/netbird/management/server/http/middleware.(*AuthMiddleware).Handler-fm.(*AuthMiddleware).Handler.func1({0x1ffce00, 0xc0001122a0}, 0xc0002a4640)
        /home/runner/work/netbird/netbird/management/server/http/middleware/auth_middleware.go:74 +0x348
net/http.HandlerFunc.ServeHTTP(0xc0004be210?, {0x1ffce00?, 0xc0001122a0?}, 0xc0002a4640?)
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29
github.com/rs/cors.(*Cors).Handler-fm.(*Cors).Handler.func1({0x1ffce00, 0xc0001122a0}, 0xc0002a4640)
        /home/runner/go/pkg/mod/github.com/rs/cors@v1.8.0/cors.go:219 +0x17e
net/http.HandlerFunc.ServeHTTP(0xc0002a4500?, {0x1ffce00?, 0xc0001122a0?}, 0x1?)
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29
github.com/netbirdio/netbird/management/server/telemetry.(*HTTPMiddleware).Handler-fm.(*HTTPMiddleware).Handler.func1({0x1ffc6d0, 0xc0001a8540}, 0xc0002a4500)
        /home/runner/work/netbird/netbird/management/server/telemetry/http_api_metrics.go:186 +0x515
net/http.HandlerFunc.ServeHTTP(0xc0002a43c0?, {0x1ffc6d0?, 0xc0001a8540?}, 0x47acb2?)
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29
github.com/gorilla/mux.(*Router).ServeHTTP(0xc000000fc0, {0x1ffc6d0, 0xc0001a8540}, 0xc0002a4280)
        /home/runner/go/pkg/mod/github.com/gorilla/mux@v1.8.0/mux.go:210 +0x1e2
github.com/netbirdio/netbird/management/cmd.init.func3.handlerFunc.11({0x1ffc6d0, 0xc0001a8540}, 0xc0002a4280)
        /home/runner/work/netbird/netbird/management/cmd/management.go:497 +0xdc
net/http.HandlerFunc.ServeHTTP(0xc0006fe4b0?, {0x1ffc6d0?, 0xc0001a8540?}, 0xc00079494d?)
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29
golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP({{0x1fee480?, 0xc000982d60?}, 0xc0002c7490?}, {0x1ffc6d0, 0xc0001a8540}, 0xc0002a4280)
        /home/runner/go/pkg/mod/golang.org/x/net@v0.39.0/http2/h2c/h2c.go:125 +0x673
net/http.serverHandler.ServeHTTP({0x1ff7378?}, {0x1ffc6d0?, 0xc0001a8540?}, 0x6?)
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:3210 +0x8e
net/http.(*conn).serve(0xc00065f710, {0x20000d0, 0xc0008668a0})
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2092 +0x5d0
created by net/http.(*Server).Serve in goroutine 133
        /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:3360 +0x485

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs not required, since the field is already tagged as required in the API doc

**Original Pull Request:** https://github.com/netbirdio/netbird/pull/4173 **State:** open **Merged:** No --- ## Describe your changes If you send an ["update an account"](https://docs.netbird.io/api/resources/accounts#update-an-account) via the REST API without the required field json object "extra", a panic that crashes the API service will happen. See the stacktrace below. This can be problematic for backwards-compatibility reasons, since in older versions of the REST-API, this field was [optional](https://web.archive.org/web/20250117034542/https://docs.netbird.io/api/resources/accounts#update-an-account). Having the management API service crash is not acceptable imo. This fix makes the REST API throw a "400 BadRequest" when the required JSON field is missing. An alternative fix could be to set default values for the extra required fields. ## Issue ticket number and link ## Stack ``` 2025/07/04 09:45:00 http: panic serving 172.18.0.1:52232: runtime error: invalid memory address or nil pointer dereference goroutine 166 [running]: net/http.(*conn).serve.func1() /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:1947 +0xbe panic({0x1aa7ac0?, 0x2d91c00?}) /opt/hostedtoolcache/go/1.23.10/x64/src/runtime/panic.go:791 +0x132 gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func17({0x20001b0, 0x2e67a20}, {0x1d31360?, 0xc000894540?, 0xc0007fc600?}, {0x1ad6000?, 0xc0002b81c0?}) /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/schema/field.go:964 +0x112 gorm.io/gorm/callbacks.ConvertToAssignments.func2(0x1a86920?, {0x1ad6000?, 0xc0002b81c0?}) /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks/update.go:149 +0x55 gorm.io/gorm/callbacks.ConvertToAssignments(0xc000894380) /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks/update.go:288 +0xd66 gorm.io/gorm/callbacks.RegisterDefaultCallbacks.Update.func7(0xc000866ae0) /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks/update.go:74 +0x1ac gorm.io/gorm.(*processor).Execute(0xc00029cfa0, 0xc0007a9cb0?) /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/callbacks.go:130 +0x3cb gorm.io/gorm.(*DB).Updates(0xc00042f662?, {0x194b3e0, 0xc00014e020}) /home/runner/go/pkg/mod/gorm.io/gorm@v1.25.12/finisher_api.go:411 +0x92 github.com/netbirdio/netbird/management/server/store.(*SqlStore).SaveAccountSettings(0xc0001196e0, {0x20000d0, 0xc000816150}, {0x1d87b4a?, 0xc000816150?}, {0xc00042f662, 0x14}, 0xc000807200) /home/runner/work/netbird/netbird/management/server/store/sql_store.go:2171 +0x218 github.com/netbirdio/netbird/management/server.(*DefaultAccountManager).UpdateAccountSettings.func1({0x2024518, 0xc0001196e0}) /home/runner/work/netbird/netbird/management/server/account.go:330 +0x394 github.com/netbirdio/netbird/management/server/store.(*SqlStore).ExecuteInTransaction(0xc000501920, {0x20000d0, 0xc000816150}, 0xc0002c79d0) /home/runner/work/netbird/netbird/management/server/store/sql_store.go:1598 +0xe7 github.com/netbirdio/netbird/management/server.(*DefaultAccountManager).UpdateAccountSettings(0xc0001a6f20, {0x20000d0, 0xc000816150}, {0xc00042f662, 0x14}, {0xc0007abd20, 0x1e}, 0xc000807200) /home/runner/work/netbird/netbird/management/server/account.go:299 +0x36f github.com/netbirdio/netbird/management/server/http/handlers/accounts.(*handler).updateAccount(0xc0002ef800, {0x1ffce00, 0xc0001122a0}, 0xc0002a5180) /home/runner/work/netbird/netbird/management/server/http/handlers/accounts/accounts_handler.go:129 +0x54c net/http.HandlerFunc.ServeHTTP(0xc0005a6570?, {0x1ffce00?, 0xc0001122a0?}, 0xc00098a890?) /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29 github.com/netbirdio/netbird/management/server/http/middleware.(*AuthMiddleware).Handler-fm.(*AuthMiddleware).Handler.func1({0x1ffce00, 0xc0001122a0}, 0xc0002a4640) /home/runner/work/netbird/netbird/management/server/http/middleware/auth_middleware.go:74 +0x348 net/http.HandlerFunc.ServeHTTP(0xc0004be210?, {0x1ffce00?, 0xc0001122a0?}, 0xc0002a4640?) /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29 github.com/rs/cors.(*Cors).Handler-fm.(*Cors).Handler.func1({0x1ffce00, 0xc0001122a0}, 0xc0002a4640) /home/runner/go/pkg/mod/github.com/rs/cors@v1.8.0/cors.go:219 +0x17e net/http.HandlerFunc.ServeHTTP(0xc0002a4500?, {0x1ffce00?, 0xc0001122a0?}, 0x1?) /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29 github.com/netbirdio/netbird/management/server/telemetry.(*HTTPMiddleware).Handler-fm.(*HTTPMiddleware).Handler.func1({0x1ffc6d0, 0xc0001a8540}, 0xc0002a4500) /home/runner/work/netbird/netbird/management/server/telemetry/http_api_metrics.go:186 +0x515 net/http.HandlerFunc.ServeHTTP(0xc0002a43c0?, {0x1ffc6d0?, 0xc0001a8540?}, 0x47acb2?) /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29 github.com/gorilla/mux.(*Router).ServeHTTP(0xc000000fc0, {0x1ffc6d0, 0xc0001a8540}, 0xc0002a4280) /home/runner/go/pkg/mod/github.com/gorilla/mux@v1.8.0/mux.go:210 +0x1e2 github.com/netbirdio/netbird/management/cmd.init.func3.handlerFunc.11({0x1ffc6d0, 0xc0001a8540}, 0xc0002a4280) /home/runner/work/netbird/netbird/management/cmd/management.go:497 +0xdc net/http.HandlerFunc.ServeHTTP(0xc0006fe4b0?, {0x1ffc6d0?, 0xc0001a8540?}, 0xc00079494d?) /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x29 golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP({{0x1fee480?, 0xc000982d60?}, 0xc0002c7490?}, {0x1ffc6d0, 0xc0001a8540}, 0xc0002a4280) /home/runner/go/pkg/mod/golang.org/x/net@v0.39.0/http2/h2c/h2c.go:125 +0x673 net/http.serverHandler.ServeHTTP({0x1ff7378?}, {0x1ffc6d0?, 0xc0001a8540?}, 0x6?) /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:3210 +0x8e net/http.(*conn).serve(0xc00065f710, {0x20000d0, 0xc0008668a0}) /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2092 +0x5d0 created by net/http.(*Server).Serve in goroutine 133 /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:3360 +0x485 ``` ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] Extended the README / documentation, if necessary > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is not needed for this change (explain why) Docs not required, since the field is already tagged as required in the API doc
saavagebueno added the pull-request label 2025-11-20 08:07:46 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
2021 Q4
2022 Q1
2022 Q1
accessibility
acl
agent
agent
Android
Android
api
authentik
automation
azure
battery-usage
bug
cache
client
client-ui
cloud
cloud-only
cloudflare
community
compatibility
config-idp
config-issue
connection
contribution
coturn
cross-vpn
dashboard
data-usage
distribution
dns
docker
documentation
duplicate
enhancement
enhancement
event-stream
feature-request
freebsd
getting-started
go
good first issue
gui
help wanted
home-assistant
idp
inconsistency
integration
integrations
ios
ipv6
jwt
k8s
keycloak
linux
login
macos
management-service
missing-docs
mobile
moved-internal
needs-review
netbird-ui
networking
new-platform
nginx
notification
okta
openwrt
packaging
peer-management
peer-management
peer-management
performance
postgres
posture-checks
psk
pull-request
Mirrored from GitHub Pull Request
 question
refactor
relay
release
rfc
routes
security
security-related
self-hosting
server
signal
sleep-issue
ssh
ssl
status
store
synology
system-compatibility-issue
test-suite
third-party-integration
triage
triage-needed
troubleshooting
UX
waiting-feedback
windows
wontfix
zitadel
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
saavagebueno
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: SVI/netbird#4125
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?