Skip to content

Commit 72a49c8

Browse files
retranretran
committed
refactor: add type-safe BSON helpers in writer_security
1 parent 1d67cb5 commit 72a49c8

2 files changed

Lines changed: 136 additions & 31 deletions

File tree

‎sdk/mpr/writer_security.go‎

Lines changed: 58 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package mpr
44

55
import (
66
"fmt"
7+
"log"
78
"strings"
89

910
"github.com/mendixlabs/mxcli/model"
@@ -12,6 +13,32 @@ import (
1213
"go.mongodb.org/mongo-driver/bson/primitive"
1314
)
1415

16+
// bsonString extracts a string from an interface value, logging unexpected types.
17+
func bsonString(v any, field string) string {
18+
if s, ok := v.(string); ok {
19+
return s
20+
}
21+
if v == nil {
22+
log.Printf("warning: writer_security: expected string for %q, got <nil>", field)
23+
} else {
24+
log.Printf("warning: writer_security: expected string for %q, got %T", field, v)
25+
}
26+
return ""
27+
}
28+
29+
// bsonBool extracts a bool from an interface value, logging unexpected types.
30+
func bsonBool(v any, field string) bool {
31+
if b, ok := v.(bool); ok {
32+
return b
33+
}
34+
if v == nil {
35+
log.Printf("warning: writer_security: expected bool for %q, got <nil>", field)
36+
} else {
37+
log.Printf("warning: writer_security: expected bool for %q, got %T", field, v)
38+
}
39+
return false
40+
}
41+
1542
// GetRawUnitBytes reads the raw BSON bytes for a unit by ID.
1643
// This returns unprocessed bytes suitable for raw BSON patching.
1744
func (r *Reader) GetRawUnitBytes(id model.ID) ([]byte, error) {
@@ -251,7 +278,7 @@ func (w *Writer) RemoveModuleRole(unitID model.ID, roleName string) error {
251278
name := ""
252279
for _, f := range roleDoc {
253280
if f.Key == "Name" {
254-
name, _ = f.Value.(string)
281+
name = bsonString(f.Value, "Name")
255282
break
256283
}
257284
}
@@ -325,7 +352,7 @@ func (w *Writer) AlterUserRoleModuleRoles(unitID model.ID, userRoleName string,
325352
name := ""
326353
for _, f := range roleDoc {
327354
if f.Key == "Name" {
328-
name, _ = f.Value.(string)
355+
name = bsonString(f.Value, "Name")
329356
break
330357
}
331358
}
@@ -476,7 +503,7 @@ func (w *Writer) RemoveUserRole(unitID model.ID, name string) error {
476503
roleName := ""
477504
for _, f := range roleDoc {
478505
if f.Key == "Name" {
479-
roleName, _ = f.Value.(string)
506+
roleName = bsonString(f.Value, "Name")
480507
break
481508
}
482509
}
@@ -530,7 +557,7 @@ func (w *Writer) RemoveDemoUser(unitID model.ID, userName string) error {
530557
name := ""
531558
for _, f := range userDoc {
532559
if f.Key == "UserName" {
533-
name, _ = f.Value.(string)
560+
name = bsonString(f.Value, "UserName")
534561
break
535562
}
536563
}
@@ -582,7 +609,7 @@ func (w *Writer) AddEntityAccessRule(unitID model.ID, entityName string, roleNam
582609
name := ""
583610
for _, f := range entityDoc {
584611
if f.Key == "Name" {
585-
name, _ = f.Value.(string)
612+
name = bsonString(f.Value, "Name")
586613
break
587614
}
588615
}
@@ -767,11 +794,11 @@ func mergeAccessRule(existing, newRule bson.D) bson.D {
767794
for _, mf := range maDoc {
768795
switch mf.Key {
769796
case "Attribute":
770-
ref = mf.Value.(string)
797+
ref = bsonString(mf.Value, "Attribute")
771798
case "Association":
772-
ref = mf.Value.(string)
799+
ref = bsonString(mf.Value, "Association")
773800
case "AccessRights":
774-
rights, _ = mf.Value.(string)
801+
rights = bsonString(mf.Value, "AccessRights")
775802
}
776803
}
777804
if ref != "" {
@@ -786,32 +813,32 @@ func mergeAccessRule(existing, newRule bson.D) bson.D {
786813
for _, f := range existing {
787814
switch f.Key {
788815
case "AllowCreate":
789-
existCreate, _ = f.Value.(bool)
816+
existCreate = bsonBool(f.Value, "AllowCreate")
790817
case "AllowDelete":
791-
existDelete, _ = f.Value.(bool)
818+
existDelete = bsonBool(f.Value, "AllowDelete")
792819
case "DefaultMemberAccessRights":
793-
existDefault, _ = f.Value.(string)
820+
existDefault = bsonString(f.Value, "DefaultMemberAccessRights")
794821
case "XPathConstraint":
795-
existXPath, _ = f.Value.(string)
822+
existXPath = bsonString(f.Value, "XPathConstraint")
796823
}
797824
}
798825

799826
// Merge into newRule
800827
for i, f := range newRule {
801828
switch f.Key {
802829
case "AllowCreate":
803-
newVal, _ := f.Value.(bool)
830+
newVal := bsonBool(f.Value, "AllowCreate")
804831
newRule[i].Value = newVal || existCreate
805832
case "AllowDelete":
806-
newVal, _ := f.Value.(bool)
833+
newVal := bsonBool(f.Value, "AllowDelete")
807834
newRule[i].Value = newVal || existDelete
808835
case "DefaultMemberAccessRights":
809-
newVal, _ := f.Value.(string)
836+
newVal := bsonString(f.Value, "DefaultMemberAccessRights")
810837
if accessRightsLevel(existDefault) > accessRightsLevel(newVal) {
811838
newRule[i].Value = existDefault
812839
}
813840
case "XPathConstraint":
814-
newVal, _ := f.Value.(string)
841+
newVal := bsonString(f.Value, "XPathConstraint")
815842
if newVal == "" && existXPath != "" {
816843
newRule[i].Value = existXPath
817844
}
@@ -829,11 +856,11 @@ func mergeAccessRule(existing, newRule bson.D) bson.D {
829856
for _, mf := range maDoc {
830857
switch mf.Key {
831858
case "Attribute":
832-
ref = mf.Value.(string)
859+
ref = bsonString(mf.Value, "Attribute")
833860
case "Association":
834-
ref = mf.Value.(string)
861+
ref = bsonString(mf.Value, "Association")
835862
case "AccessRights":
836-
newRights, _ = mf.Value.(string)
863+
newRights = bsonString(mf.Value, "AccessRights")
837864
}
838865
}
839866
if ref == "" {
@@ -885,7 +912,7 @@ func (w *Writer) RemoveEntityAccessRule(unitID model.ID, entityName string, role
885912
name := ""
886913
for _, f := range entityDoc {
887914
if f.Key == "Name" {
888-
name, _ = f.Value.(string)
915+
name = bsonString(f.Value, "Name")
889916
break
890917
}
891918
}
@@ -1016,7 +1043,7 @@ func (w *Writer) RevokeEntityMemberAccess(unitID model.ID, entityName string, ro
10161043
name := ""
10171044
for _, f := range entityDoc {
10181045
if f.Key == "Name" {
1019-
name, _ = f.Value.(string)
1046+
name = bsonString(f.Value, "Name")
10201047
break
10211048
}
10221049
}
@@ -1073,7 +1100,7 @@ func (w *Writer) RevokeEntityMemberAccess(unitID model.ID, entityName string, ro
10731100
ruleDoc[k].Value = "None"
10741101
ruleModified = true
10751102
} else if revocation.RevokeWriteAll {
1076-
cur, _ := rf.Value.(string)
1103+
cur := bsonString(rf.Value, "DefaultMemberAccessRights")
10771104
if cur == "ReadWrite" {
10781105
ruleDoc[k].Value = "ReadOnly"
10791106
ruleModified = true
@@ -1093,11 +1120,11 @@ func (w *Writer) RevokeEntityMemberAccess(unitID model.ID, entityName string, ro
10931120
for _, mf := range maDoc {
10941121
switch mf.Key {
10951122
case "Attribute":
1096-
ref = mf.Value.(string)
1123+
ref = bsonString(mf.Value, "Attribute")
10971124
case "Association":
1098-
ref = mf.Value.(string)
1125+
ref = bsonString(mf.Value, "Association")
10991126
case "AccessRights":
1100-
rights, _ = mf.Value.(string)
1127+
rights = bsonString(mf.Value, "AccessRights")
11011128
}
11021129
}
11031130
if ref == "" {
@@ -1267,7 +1294,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
12671294
entityName := ""
12681295
for _, f := range entityDoc {
12691296
if f.Key == "Name" {
1270-
entityName, _ = f.Value.(string)
1297+
entityName = bsonString(f.Value, "Name")
12711298
break
12721299
}
12731300
}
@@ -1288,7 +1315,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
12881315
isCalculated := false
12891316
for _, f := range attrDoc {
12901317
if f.Key == "Name" {
1291-
attrName, _ = f.Value.(string)
1318+
attrName = bsonString(f.Value, "Name")
12921319
}
12931320
if f.Key == "Value" {
12941321
if valueDoc, ok := f.Value.(bson.D); ok {
@@ -1363,7 +1390,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
13631390
case "ParentPointer":
13641391
aParentID = extractBsonIDValue(f.Value)
13651392
case "Name":
1366-
aName, _ = f.Value.(string)
1393+
aName = bsonString(f.Value, "Name")
13671394
}
13681395
}
13691396
if aParentID == entityID && aName != "" {
@@ -1382,7 +1409,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
13821409
parentID = extractBsonIDValue(f.Value)
13831410
}
13841411
if f.Key == "Name" {
1385-
caName, _ = f.Value.(string)
1412+
caName = bsonString(f.Value, "Name")
13861413
}
13871414
}
13881415
if parentID == entityID && caName != "" {
@@ -1461,10 +1488,10 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
14611488
assocRef := ""
14621489
for _, mf := range maDoc {
14631490
if mf.Key == "Attribute" {
1464-
attrRef, _ = mf.Value.(string)
1491+
attrRef = bsonString(mf.Value, "Attribute")
14651492
}
14661493
if mf.Key == "Association" {
1467-
assocRef, _ = mf.Value.(string)
1494+
assocRef = bsonString(mf.Value, "Association")
14681495
}
14691496
}
14701497

‎sdk/mpr/writer_security_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
package mpr
44

55
import (
6+
"bytes"
7+
"io"
8+
"log"
9+
"strings"
610
"testing"
711

812
"go.mongodb.org/mongo-driver/bson"
@@ -124,3 +128,77 @@ func TestRemoveRolesFromAccessRule_ThreeRoles_RemoveMiddle(t *testing.T) {
124128
t.Errorf("expected [Mod.A, Mod.C], got %v", names)
125129
}
126130
}
131+
132+
// =============================================================================
133+
// bsonString / bsonBool — unexpected types must not panic
134+
// =============================================================================
135+
136+
func TestBsonHelpers_UnexpectedTypes_NoPanic(t *testing.T) {
137+
var buf bytes.Buffer
138+
origOutput := log.Writer()
139+
log.SetOutput(&buf)
140+
defer log.SetOutput(origOutput)
141+
142+
// bsonString with non-string values
143+
if s := bsonString(42, "test"); s != "" {
144+
t.Errorf("expected empty string, got %q", s)
145+
}
146+
if s := bsonString(nil, "test"); s != "" {
147+
t.Errorf("expected empty string for nil, got %q", s)
148+
}
149+
if s := bsonString(true, "test"); s != "" {
150+
t.Errorf("expected empty string for bool, got %q", s)
151+
}
152+
153+
// bsonBool with non-bool values
154+
if b := bsonBool("true", "test"); b {
155+
t.Error("expected false for string input")
156+
}
157+
if b := bsonBool(nil, "test"); b {
158+
t.Error("expected false for nil")
159+
}
160+
if b := bsonBool(42, "test"); b {
161+
t.Error("expected false for int input")
162+
}
163+
164+
// Verify diagnostic warnings were emitted
165+
logged := buf.String()
166+
expectedWarnings := []string{
167+
`expected string for "test", got int`,
168+
`expected string for "test", got <nil>`,
169+
`expected string for "test", got bool`,
170+
`expected bool for "test", got string`,
171+
`expected bool for "test", got <nil>`,
172+
`expected bool for "test", got int`,
173+
}
174+
for _, w := range expectedWarnings {
175+
if !strings.Contains(logged, w) {
176+
t.Errorf("expected warning containing %q in log output", w)
177+
}
178+
}
179+
}
180+
181+
func TestMergeAccessRule_UnexpectedTypes_NoPanic(t *testing.T) {
182+
origOutput := log.Writer()
183+
log.SetOutput(io.Discard)
184+
defer log.SetOutput(origOutput)
185+
186+
existing := bson.D{
187+
{Key: "$Type", Value: "DomainModels$AccessRule"},
188+
{Key: "AllowCreate", Value: 42}, // wrong type: int instead of bool
189+
{Key: "AllowDelete", Value: "not-a-bool"}, // wrong type: string instead of bool
190+
{Key: "DefaultMemberAccessRights", Value: 99},
191+
}
192+
newRule := bson.D{
193+
{Key: "$Type", Value: "DomainModels$AccessRule"},
194+
{Key: "AllowCreate", Value: true},
195+
{Key: "AllowDelete", Value: false},
196+
{Key: "DefaultMemberAccessRights", Value: "ReadWrite"},
197+
}
198+
199+
// Must not panic
200+
result := mergeAccessRule(existing, newRule)
201+
if result == nil {
202+
t.Error("expected non-nil result")
203+
}
204+
}

0 commit comments

Comments
 (0)