fix(bed-5436): Add support for existing nodes in LinkWellKnownGroups

This commit fixes an issue where the LinkWellKnownGroups function was
encountering graph DB constraint violations during ingestion. The errors occurred
when attempting to create well-known group nodes with SIDs that already existed
in the database but with different node types.

The root cause was in the getOrCreateWellKnownGroup function, which was
filtering nodes by both ObjectID and the Group kind. When a node with the
matching ObjectID existed but didn't have the Group kind, the function would
try to create a new node with the same ObjectID, causing a constraint violation.

The fix modifies the getOrCreateWellKnownGroup function to:
1. Only filter by ObjectID when looking up nodes, not by kind
2. Add the Group kind to existing nodes if it's missing
3. Update the node to ensure it has the correct properties

This approach ensures that:
- We don't attempt to create duplicate nodes with the same ObjectID
- Existing nodes are properly enhanced with the Group kind when needed
- We avoid constraint violations by working with existing nodes

Additional changes:
- Added comprehensive tests to verify the behavior with various node scenarios
- Fixed formatting in FixWellKnownNodeTypes

This change ensures that SID linking will no longer error when it encounters
nodes with different types than expected, addressing the reported constraint
violation errors during ingestion.

Fixes: [BED-5436](https://specterops.atlassian.net/browse/BED-5436)

packages/go/analysis/ad/ad.go

@@ -122,7 +122,8 @@ func FetchWellKnownTierZeroEntities(ctx context.Context, db graph.Database, doma
 func FixWellKnownNodeTypes(ctx context.Context, db graph.Database) error {
 	defer measure.ContextMeasure(ctx, slog.LevelInfo, "Fix well known node types")()
 
-	groupSuffixes := []string{EnterpriseKeyAdminsGroupSIDSuffix,
+	groupSuffixes := []string{
+		EnterpriseKeyAdminsGroupSIDSuffix,
 		KeyAdminsGroupSIDSuffix,
 		EnterpriseDomainControllersGroupSIDSuffix,
 		DomainAdminsGroupSIDSuffix,
@@ -190,7 +191,6 @@ func RunDomainAssociations(ctx context.Context, db graph.Database) error {
 		// TODO: Reimplement unassociated node pruning if we decide that FOSS needs unassociated node pruning
 		return nil
 	})
-
 }
 
 func grabDomainInformation(tx graph.Transaction) (map[string]string, error) {
@@ -274,14 +274,13 @@ func LinkWellKnownGroups(ctx context.Context, db graph.Database) error {
 }
 
 func getOrCreateWellKnownGroup(tx graph.Transaction, wellKnownSid string, domainSid, domainName, nodeName string) (*graph.Node, error) {
+	// Only filter by ObjectID, not by kind
 	if wellKnownNode, err := tx.Nodes().Filterf(func() graph.Criteria {
-		return query.And(
-			query.Equals(query.NodeProperty(common.ObjectID.String()), wellKnownSid),
-			query.Kind(query.Node(), ad.Group),
-		)
+		return query.Equals(query.NodeProperty(common.ObjectID.String()), wellKnownSid)
 	}).First(); err != nil && !graph.IsErrNotFound(err) {
 		return nil, err
 	} else if graph.IsErrNotFound(err) {
+		// Only create a new node if no node with this ObjectID exists
 		properties := graph.AsProperties(graph.PropertyMap{
 			common.Name:     nodeName,
 			common.ObjectID: wellKnownSid,
@@ -291,6 +290,15 @@ func getOrCreateWellKnownGroup(tx graph.Transaction, wellKnownSid string, domain
 		})
 		return tx.CreateNode(properties, ad.Entity, ad.Group)
 	} else {
+		// If a node with this ObjectID exists (regardless of its kind), return it
+		// Optionally, we could add the ad.Group kind if it's missing
+		if !wellKnownNode.Kinds.ContainsOneOf(ad.Group) {
+			// Add the ad.Group kind if it's missing
+			wellKnownNode.AddKinds(ad.Group)
+			if err := tx.UpdateNode(wellKnownNode); err != nil {
+				return nil, fmt.Errorf("failed to update node with Group kind: %w", err)
+			}
+		}
 		return wellKnownNode, nil
 	}
 }
@@ -342,7 +350,6 @@ func CalculateCrossProductNodeSets(tx graph.Transaction, groupExpansions impact.
 
 	// Get the IDs of the Auth. Users and Everyone groups
 	specialGroups, err := FetchAuthUsersAndEveryoneGroups(tx)
-
 	if err != nil {
 		slog.Error(fmt.Sprintf("Could not fetch groups: %s", err.Error()))
 	}

packages/go/analysis/ad/ad_test.go

@@ -0,0 +1,276 @@
+package ad_test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/specterops/bloodhound/dawgs/graph"
+	"github.com/specterops/bloodhound/dawgs/query"
+	"github.com/stretchr/testify/require"
+
+	adAnalysis "github.com/specterops/bloodhound/analysis/ad"
+
+	"github.com/specterops/bloodhound/graphschema/ad"
+	"github.com/specterops/bloodhound/graphschema/common"
+)
+
+func TestLinkWellKnownGroups(t *testing.T) {
+	// Skip if running short tests
+	if testing.Short() {
+		t.Skip("Skipping test in short mode")
+	}
+
+	// Define database drivers to test
+	dbDrivers := []struct {
+		name   string
+		driver string
+		initFn func(t *testing.T, ctx context.Context) (graph.Database, func())
+	}{
+		{
+			name:   "Neo4j",
+			driver: "neo4j",
+			initFn: initNeo4jDatabase,
+		},
+		{
+			name:   "PostgreSQL",
+			driver: "pg",
+			initFn: initPostgresDatabase,
+		},
+	}
+
+	// Define test cases
+	testCases := []struct {
+		name                string
+		setupFunc           func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error
+		expectedGroupExists map[string]bool         // Map of well-known SID suffix to expected existence
+		expectedGroupKinds  map[string][]graph.Kind // Map of well-known SID suffix to expected kinds
+	}{
+		{
+			name: "Normal scenario - no pre-existing nodes",
+			setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error {
+				return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error {
+					// Just create the domain node
+					domainProperties := graph.AsProperties(graph.PropertyMap{
+						common.Name:     domainName,
+						common.ObjectID: domainSid,
+					})
+					_, err := tx.CreateNode(domainProperties, ad.Domain)
+					return err
+				})
+			},
+			expectedGroupExists: map[string]bool{
+				"-512": true, // Domain Admins
+				"-513": true, // Domain Users
+				"-515": true, // Domain Computers
+				"-516": true, // Domain Controllers
+			},
+			expectedGroupKinds: map[string][]graph.Kind{
+				"-512": {ad.Entity, ad.Group},
+				"-513": {ad.Entity, ad.Group},
+				"-515": {ad.Entity, ad.Group},
+				"-516": {ad.Entity, ad.Group},
+			},
+		},
+		{
+			name: "Pre-existing node without Group kind",
+			setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error {
+				return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error {
+					// Create the domain node
+					domainProperties := graph.AsProperties(graph.PropertyMap{
+						common.Name:     domainName,
+						common.ObjectID: domainSid,
+						ad.DomainSID:    domainSid,
+					})
+					_, err := tx.CreateNode(domainProperties, ad.Domain)
+					if err != nil {
+						return err
+					}
+
+					// Create a node for Domain Users with the same SID but without Group kind
+					domainUsersSid := domainSid + "-513"
+					userProperties := graph.AsProperties(graph.PropertyMap{
+						common.Name:     "Domain Users",
+						common.ObjectID: domainUsersSid,
+					})
+					_, err = tx.CreateNode(userProperties, ad.Entity) // Note: No ad.Group kind
+					return err
+				})
+			},
+			expectedGroupExists: map[string]bool{
+				"-512": true, // Domain Admins
+				"-513": true, // Domain Users (pre-existing)
+				"-515": true, // Domain Computers
+				"-516": true, // Domain Controllers
+			},
+			expectedGroupKinds: map[string][]graph.Kind{
+				"-512": {ad.Entity, ad.Group},
+				"-513": {ad.Entity, ad.Group}, // Should have Group kind added
+				"-515": {ad.Entity, ad.Group},
+				"-516": {ad.Entity, ad.Group},
+			},
+		},
+		{
+			name: "Pre-existing node with different kind (constraint violation scenario)",
+			setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error {
+				return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error {
+					// Create the domain node
+					domainProperties := graph.AsProperties(graph.PropertyMap{
+						common.Name:     domainName,
+						common.ObjectID: domainSid,
+						ad.DomainSID:    domainSid,
+					})
+					_, err := tx.CreateNode(domainProperties, ad.Domain)
+					if err != nil {
+						return err
+					}
+
+					// Create a node with the same SID as Domain Computers but with a different kind
+					domainComputersSid := domainSid + "-515"
+					computerProperties := graph.AsProperties(graph.PropertyMap{
+						common.Name:     "Some Other Entity",
+						common.ObjectID: domainComputersSid,
+					})
+					_, err = tx.CreateNode(computerProperties, ad.Entity, ad.User) // Not a Group
+					return err
+				})
+			},
+			expectedGroupExists: map[string]bool{
+				"-512": true, // Domain Admins
+				"-513": true, // Domain Users
+				"-515": true, // Domain Computers (pre-existing with different kind)
+				"-516": true, // Domain Controllers
+			},
+			expectedGroupKinds: map[string][]graph.Kind{
+				"-512": {ad.Entity, ad.Group},
+				"-513": {ad.Entity, ad.Group},
+				"-515": {ad.Entity, ad.Group, ad.User}, // Should have both User and Group kinds
+				"-516": {ad.Entity, ad.Group},
+			},
+		},
+		{
+			name: "Pre-existing node with Group kind",
+			setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error {
+				return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error {
+					// Create the domain node
+					domainProperties := graph.AsProperties(graph.PropertyMap{
+						common.Name:     domainName,
+						common.ObjectID: domainSid,
+						ad.DomainSID:    domainSid,
+					})
+					_, err := tx.CreateNode(domainProperties, ad.Domain)
+					if err != nil {
+						return err
+					}
+
+					// Create a node for Domain Admins with Group kind already
+					domainAdminsSid := domainSid + "-512"
+					adminProperties := graph.AsProperties(graph.PropertyMap{
+						common.Name:     "Domain Admins",
+						common.ObjectID: domainAdminsSid,
+					})
+					_, err = tx.CreateNode(adminProperties, ad.Entity, ad.Group)
+					return err
+				})
+			},
+			expectedGroupExists: map[string]bool{
+				"-512": true, // Domain Admins (pre-existing with Group kind)
+				"-513": true, // Domain Users
+				"-515": true, // Domain Computers
+				"-516": true, // Domain Controllers
+			},
+			expectedGroupKinds: map[string][]graph.Kind{
+				"-512": {ad.Entity, ad.Group},
+				"-513": {ad.Entity, ad.Group},
+				"-515": {ad.Entity, ad.Group},
+				"-516": {ad.Entity, ad.Group},
+			},
+		},
+	}
+
+	// Run tests for each database driver
+	for _, dbDriver := range dbDrivers {
+		t.Run(dbDriver.name, func(t *testing.T) {
+			ctx := context.Background()
+
+			// Initialize the database
+			graphDB, cleanup := dbDriver.initFn(t, ctx)
+			defer cleanup()
+
+			for _, tc := range testCases {
+				t.Run(tc.name, func(t *testing.T) {
+					// Clean the database before each test
+					cleanDatabase(t, ctx, graphDB)
+
+					// Set up the test case
+					domainSid := "S-1-5-21-123456789-123456789-123456789"
+					domainName := "TESTDOMAIN.LOCAL"
+					err := tc.setupFunc(ctx, graphDB, domainSid, domainName)
+					require.NoError(t, err)
+
+					// Make sure the domain node has the required properties for LinkWellKnownGroups
+					err = graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error {
+						domain, err := tx.Nodes().Filterf(func() graph.Criteria {
+							return query.Kind(query.Node(), ad.Domain)
+						}).First()
+						if err != nil {
+							return err
+						}
+
+						// Ensure domain has DomainSID property
+						if _, err := domain.Properties.Get(ad.DomainSID.String()).String(); err != nil {
+							domain.Properties.Set(ad.DomainSID.String(), domainSid)
+							if err := tx.UpdateNode(domain); err != nil {
+								return err
+							}
+						}
+						return nil
+					})
+					require.NoError(t, err)
+					require.NoError(t, err)
+
+					// Run LinkWellKnownGroups
+					err = adAnalysis.LinkWellKnownGroups(ctx, graphDB)
+					require.NoError(t, err)
+
+					// Verify the results in a read transaction
+					err = graphDB.ReadTransaction(ctx, func(tx graph.Transaction) error {
+						for sidSuffix, shouldExist := range tc.expectedGroupExists {
+							wellKnownSid := domainSid + sidSuffix
+							node, err := tx.Nodes().Filterf(func() graph.Criteria {
+								return query.Equals(query.NodeProperty(common.ObjectID.String()), wellKnownSid)
+							}).First()
+
+							if shouldExist {
+								if err != nil {
+									return fmt.Errorf("node with SID %s should exist: %w", wellKnownSid, err)
+								}
+								if node == nil {
+									return fmt.Errorf("node with SID %s should not be nil", wellKnownSid)
+								}
+
+								// Verify the kinds
+								expectedKinds := tc.expectedGroupKinds[sidSuffix]
+								for _, kind := range expectedKinds {
+									if !node.Kinds.ContainsOneOf(kind) {
+										return fmt.Errorf(
+											"node with SID %s should have kind %s",
+											wellKnownSid,
+											kind,
+										)
+									}
+								}
+							} else {
+								if err == nil || !graph.IsErrNotFound(err) {
+									return fmt.Errorf("node with SID %s should not exist", wellKnownSid)
+								}
+							}
+						}
+						return nil
+					})
+					require.NoError(t, err)
+				})
+			}
+		})
+	}
+}