refactor(cachetest): build fixtures via ldap.NewObject instead of unsafe (#633)

simple-ldap-go v1.14.0 adds `NewObject(cn, dn) Object`. Until then
`Object.cn` / `Object.dn` were written only by the package-internal
`objectFromEntry`, so a consumer that wanted a fixture with a known DN
had no public way to build one — `internal/ldap_cache/cachetest` reached
the fields through `reflect` + `unsafe.Pointer(...UnsafeAddr())`, which
gosec flags as G103. See netresearch/simple-ldap-go#191.

The three fixture constructors (`NewUserWithDN`, `NewGroupWithDN`,
`NewComputerWithDN`) now set the embedded `Object` via
`ldap.NewObject(cn, dn)` in a plain composite literal. That removes
`setObjectFields`, the `reflect` and `unsafe` imports, and the two
`#nosec G103` annotations together with the comment block that justified
them — a suppression that no longer suppresses anything.

`cachetest` stays an ordinary (non-`_test.go`) package: it is imported
from another package's tests, which `_test.go` files cannot be.

Dependency bumped to v1.14.0; `go mod tidy` also dropped stale `go.sum`
lines for `gofiber/storage/bbolt` v2.1.8, `gofiber/utils` v2.1.0 and
`shamaton/msgpack` v3.1.2, none of which were the selected versions.

Not covered here: `internal/ldap_cache/test_helpers_test.go` carries a
second copy of the same reflect+unsafe writer for that package's own
tests. It is a `_test.go` file, outside gosec's default scan set, and
out of scope for this change.

Verification: `go build ./...`, `go vet ./...`, `go test ./... -count=1`
and `golangci-lint run ./...` on the branch; `gosec ./...` reports
`Issues: 0`. The fixture tests assert on DN/CN — swapping the
`NewObject` arguments locally made them fail, so they do pin the values
the unsafe writer used to produce.

Author: CybotTM

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/gofiber/storage/memory/v2 v2.1.2
 	github.com/joho/godotenv v1.5.1
 	github.com/mxschmitt/playwright-go v0.6100.0
-	github.com/netresearch/simple-ldap-go v1.13.0
+	github.com/netresearch/simple-ldap-go v1.14.0
 	github.com/rs/zerolog v1.35.1
 	github.com/stretchr/testify v1.11.1
 	github.com/testcontainers/testcontainers-go v0.43.0

go.sum

@@ -235,14 +235,10 @@ github.com/godoc-lint/godoc-lint v0.11.2 h1:Bp0FkJWoSdNsBikdNgIcgtaoo+xz6I/Y9s5W
 github.com/godoc-lint/godoc-lint v0.11.2/go.mod h1:iVpGdL1JCikNH2gGeAn3Hh+AgN5Gx/I/cxV+91L41jo=
 github.com/gofiber/fiber/v2 v2.52.14 h1:Of3L+9qVFaQNwPlcmEdl5IIodHz8BSE0j37R7rWu4pE=
 github.com/gofiber/fiber/v2 v2.52.14/go.mod h1:YEcBbO/FB+5M1IZNBP9FO3J9281zgPAreiI1oqg8nDw=
-github.com/gofiber/storage/bbolt/v2 v2.1.8 h1:GBxG2lEtsbBN2zhwwlL5x8YB8jqX4994q3HMC7GCok4=
-github.com/gofiber/storage/bbolt/v2 v2.1.8/go.mod h1:afyeCkYB+HTFMzq0TMqYTbNG4fJi+TIW22B93/oEadc=
 github.com/gofiber/storage/bbolt/v2 v2.1.9 h1:lO9ZeF8qAF6p/Chf3wc8eiTzkWCFly1XdYIldSk2zS0=
 github.com/gofiber/storage/bbolt/v2 v2.1.9/go.mod h1:fJ7Xm6mQbfxxCYliaVxCU+EUzr+17YdODoIhbtQF33w=
 github.com/gofiber/storage/memory/v2 v2.1.2 h1:IGOMSgnlCv/Dh1rkXA9G69kHIAywUqPjaGFUR5XATE0=
 github.com/gofiber/storage/memory/v2 v2.1.2/go.mod h1:R3dBncEPAGt83cQleHFXdT/t7Pep6+5ED0rBfJSqjJg=
-github.com/gofiber/utils/v2 v2.1.0 h1:WSu4COJhJw9moNfJu2nQvaM9AFvAQ/nZbigjhHqKgOQ=
-github.com/gofiber/utils/v2 v2.1.0/go.mod h1:DdOgEVwQTi8cou/AKWPqhXOR4fHGRVhA/rEWL3IXG7Q=
 github.com/gofiber/utils/v2 v2.2.0 h1:YSSmCzQponq/f9uSOg2HtXC5qK1Dmor0o6DqaQVz8GE=
 github.com/gofiber/utils/v2 v2.2.0/go.mod h1:Ieopk6sQh7rbhQ12aBNCJtJuG0gxAg0nz63sFCrrOmE=
 github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw=
@@ -420,8 +416,8 @@ github.com/nakabonne/nestif v0.3.1 h1:wm28nZjhQY5HyYPx+weN3Q65k6ilSBxDb8v5S81B81
 github.com/nakabonne/nestif v0.3.1/go.mod h1:9EtoZochLn5iUprVDmDjqGKPofoUEBL8U4Ngq6aY7OE=
 github.com/natefinch/atomic v1.0.1 h1:ZPYKxkqQOx3KZ+RsbnP/YsgvxWQPGxjC0oBt2AhwV0A=
 github.com/natefinch/atomic v1.0.1/go.mod h1:N/D/ELrljoqDyT3rZrsUmtsuzvHkeB/wWjHV22AZRbM=
-github.com/netresearch/simple-ldap-go v1.13.0 h1:hP3zlKCsB+0me6TqV41RoPTanRaqDp4u2hcpDPuGoiY=
-github.com/netresearch/simple-ldap-go v1.13.0/go.mod h1:PHBAklheFIQUQRfoarVSMwlfcB9YhRhbaL8CuQq9awU=
+github.com/netresearch/simple-ldap-go v1.14.0 h1:H9ERR1FcEWOzzxj5tI2LjnTlo3FCNyeynCnQ9MS3X14=
+github.com/netresearch/simple-ldap-go v1.14.0/go.mod h1:+Wz+HjRgA49ClNG9CB3poT82N7FxDFP4szggt3n720I=
 github.com/nishanths/exhaustive v0.12.0 h1:vIY9sALmw6T/yxiASewa4TQcFsVYZQQRUQJhKRf3Swg=
 github.com/nishanths/exhaustive v0.12.0/go.mod h1:mEZ95wPIZW+x8kC4TgC+9YCUgiST7ecevsVDTgc2obs=
 github.com/nishanths/predeclared v0.2.2 h1:V2EPdZPliZymNAn79T8RkNApBjMmVKh5XRpLm/w98Vk=
@@ -498,8 +494,8 @@ github.com/securego/gosec/v2 v2.26.1 h1:gdkttGhQFVehqRJ8grKH4DrpqM/QlPKNHBnl8Qgc
 github.com/securego/gosec/v2 v2.26.1/go.mod h1:57UW4p0uoP3kxoTkhoo3axLdVAi+OWrLg/Ax/kdqtPE=
 github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
 github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
-github.com/shamaton/msgpack/v3 v3.1.2 h1:d5gWAIyMU4M0WgDjz6IFSCuXJUA2dFwRHBpDclE8CLw=
-github.com/shamaton/msgpack/v3 v3.1.2/go.mod h1:DcQG8jrdrQCIxr3HlMYkiXdMhK+KfN2CitkyzsQV4uc=
+github.com/shamaton/msgpack/v3 v3.2.0 h1:1q2Ms+MWmuRju+PuDMSFDB7p7621npeX4zprJN5Zck8=
+github.com/shamaton/msgpack/v3 v3.2.0/go.mod h1:sgBYvEiyz8JR1NC3yGRoPVME9xXovpnh3l/plW1nfRo=
 github.com/shirou/gopsutil/v4 v4.26.5 h1:RPcBXkpz7kOj9PqGFQOlBPZHsyaPvPVQc098y9RmCNM=
 github.com/shirou/gopsutil/v4 v4.26.5/go.mod h1:LZ6ewCSkBqUpvSOf+LsTGnRinC6iaNUNMGBtDkJBaLQ=
 github.com/sirupsen/logrus v1.9.4 h1:TsZE7l11zFCLZnZ+teH4Umoq5BhEIfIzfRDZ1Uzql2w=

internal/ldap_cache/cachetest/helpers.go

@@ -1,41 +1,40 @@
 // Package cachetest provides exported test helpers for seeding the
-// ldap_cache.Manager from external test packages. It uses reflection +
-// unsafe to set the unexported dn/cn fields on simple-ldap-go's Object
-// type. DO NOT import from production code — this exists for tests only.
+// ldap_cache.Manager from external test packages. DO NOT import from
+// production code — this exists for tests only.
 package cachetest
 
 import (
-	"reflect"
-	"unsafe"
-
 	ldap "github.com/netresearch/simple-ldap-go"
 
 	"github.com/netresearch/ldap-manager/internal/ldap_cache"
 )
 
-// NewUserWithDN builds a ldap.User with the unexported dn/cn fields set
-// via reflection. Use only in tests.
+// NewUserWithDN builds a ldap.User with dn/cn populated. Use only in tests.
 func NewUserWithDN(dn, cn, sam string, enabled bool, groups []string) ldap.User {
-	u := ldap.User{SAMAccountName: sam, Enabled: enabled, Groups: groups}
-	setObjectFields(reflect.ValueOf(&u).Elem().FieldByName("Object"), dn, cn)
-
-	return u
+	return ldap.User{
+		Object:         ldap.NewObject(cn, dn),
+		SAMAccountName: sam,
+		Enabled:        enabled,
+		Groups:         groups,
+	}
 }
 
-// NewGroupWithDN builds a ldap.Group with unexported dn/cn populated.
+// NewGroupWithDN builds a ldap.Group with dn/cn populated.
 func NewGroupWithDN(dn, cn string, members []string) ldap.Group {
-	g := ldap.Group{Members: members}
-	setObjectFields(reflect.ValueOf(&g).Elem().FieldByName("Object"), dn, cn)
-
-	return g
+	return ldap.Group{
+		Object:  ldap.NewObject(cn, dn),
+		Members: members,
+	}
 }
 
-// NewComputerWithDN builds a ldap.Computer with unexported dn/cn populated.
+// NewComputerWithDN builds a ldap.Computer with dn/cn populated.
 func NewComputerWithDN(dn, cn, sam string, enabled bool, groups []string) ldap.Computer {
-	c := ldap.Computer{SAMAccountName: sam, Enabled: enabled, Groups: groups}
-	setObjectFields(reflect.ValueOf(&c).Elem().FieldByName("Object"), dn, cn)
-
-	return c
+	return ldap.Computer{
+		Object:         ldap.NewObject(cn, dn),
+		SAMAccountName: sam,
+		Enabled:        enabled,
+		Groups:         groups,
+	}
 }
 
 // Seed replaces all three caches in one call. Pass nil for any kind you
@@ -51,20 +50,3 @@ func Seed(m *ldap_cache.Manager, users []ldap.User, groups []ldap.Group, compute
 		m.SetComputersForTesting(computers)
 	}
 }
-
-func setObjectFields(obj reflect.Value, dn, cn string) {
-	dnField := obj.FieldByName("dn")
-	cnField := obj.FieldByName("cn")
-	// Test-only writers for unexported simple-ldap-go fields. This package is
-	// imported by the tests of two other packages (internal/web and
-	// internal/ldap_cache), and Go cannot share helpers declared in _test.go
-	// files across package boundaries — so it has to be an ordinary package and
-	// is compiled into the binary, unreachable from any production call path.
-	// The unsafe write exists because simple-ldap-go sets Object.dn/cn only in
-	// objectFromEntry when decoding a directory response and exposes no
-	// constructor taking them; see netresearch/simple-ldap-go#191.
-	dnPtr := unsafe.Pointer(dnField.UnsafeAddr()) // #nosec G103 -- test fixture writer, see comment above
-	cnPtr := unsafe.Pointer(cnField.UnsafeAddr()) // #nosec G103 -- test fixture writer, see comment above
-	reflect.NewAt(dnField.Type(), dnPtr).Elem().SetString(dn)
-	reflect.NewAt(cnField.Type(), cnPtr).Elem().SetString(cn)
-}

internal/ldap_cache/test_helpers_test.go

@@ -4,9 +4,7 @@ package ldap_cache
 
 import (
 	"errors"
-	"reflect"
 	"sync"
-	"unsafe"
 
 	ldap "github.com/netresearch/simple-ldap-go"
 )
@@ -104,40 +102,34 @@ func NewMockComputer(_, samAccountName string, enabled bool, groups []string) ld
 	}
 }
 
-// Test-only helpers: seed ldap.User / Group / Computer with a real DN by
-// poking simple-ldap-go's unexported Object fields. Production code builds
-// these via objectFromEntry internal to that package.
+// Test-only helpers: seed ldap.User / Group / Computer with a real DN.
+// simple-ldap-go's Object carries dn and cn unexported; ldap.NewObject is the
+// constructor it exposes for building fixtures.
 
-// newUserWithDN creates a ldap.User with the DN and CN fields populated via reflection.
+// newUserWithDN creates a ldap.User with the DN and CN fields populated.
 func newUserWithDN(dn, cn, sam string, enabled bool, groups []string) ldap.User {
-	u := ldap.User{SAMAccountName: sam, Enabled: enabled, Groups: groups}
-	setObjectFields(reflect.ValueOf(&u).Elem().FieldByName("Object"), dn, cn)
-
-	return u
+	return ldap.User{
+		Object:         ldap.NewObject(cn, dn),
+		SAMAccountName: sam,
+		Enabled:        enabled,
+		Groups:         groups,
+	}
 }
 
-// newGroupWithDN creates a ldap.Group with the DN and CN fields populated via reflection.
+// newGroupWithDN creates a ldap.Group with the DN and CN fields populated.
 func newGroupWithDN(dn, cn string, members []string) ldap.Group {
-	g := ldap.Group{Members: members}
-	setObjectFields(reflect.ValueOf(&g).Elem().FieldByName("Object"), dn, cn)
-
-	return g
+	return ldap.Group{
+		Object:  ldap.NewObject(cn, dn),
+		Members: members,
+	}
 }
 
-// newComputerWithDN creates a ldap.Computer with the DN and CN fields populated via reflection.
+// newComputerWithDN creates a ldap.Computer with the DN and CN fields populated.
 func newComputerWithDN(dn, cn, sam string, enabled bool, groups []string) ldap.Computer { //nolint:unused
-	c := ldap.Computer{SAMAccountName: sam, Enabled: enabled, Groups: groups}
-	setObjectFields(reflect.ValueOf(&c).Elem().FieldByName("Object"), dn, cn)
-
-	return c
-}
-
-// setObjectFields uses unsafe pointer arithmetic to write the unexported dn and cn
-// fields of simple-ldap-go's Object struct. This is only valid in test code where
-// we need to seed deterministic DNs without going through LDAP entry parsing.
-func setObjectFields(obj reflect.Value, dn, cn string) {
-	dnField := obj.FieldByName("dn")
-	cnField := obj.FieldByName("cn")
-	reflect.NewAt(dnField.Type(), unsafe.Pointer(dnField.UnsafeAddr())).Elem().SetString(dn) //nolint:gosec
-	reflect.NewAt(cnField.Type(), unsafe.Pointer(cnField.UnsafeAddr())).Elem().SetString(cn) //nolint:gosec
+	return ldap.Computer{
+		Object:         ldap.NewObject(cn, dn),
+		SAMAccountName: sam,
+		Enabled:        enabled,
+		Groups:         groups,
+	}
 }