feat: Add option to delete HasSession edges #853 #2110
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
WalkthroughAdds a flexible relationship-deletion option to the clear-database flow: UI checkbox state, API payload field Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as DatabaseManagement UI
participant API as Backend API
participant Graph as Graph DB
participant Audit as Audit Log
User->>UI: Check "HasSession" and submit clear-database
UI->>API: DELETE /api/v2/clear-database { ..., deleteRelationships: ["HasSession"] }
alt deleteRelationships contains names
API->>API: validate request, create audit entry
API->>Graph: Query relationships matching each name (by edge type/name)
Graph-->>API: Return matching relationship ids
API->>Graph: Batch-delete returned relationships
Graph-->>API: Deletion results per edge-name
API->>Audit: Record success/failure for each edge-name
end
API-->>UI: Return wipe result (including any edge-delete failures)
UI->>User: Show confirmation/errors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
I have read the CLA Document and I hereby sign the CLA |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cmd/api/src/api/v2/database_wipe.go (1)
251-276: Consider using a streaming pattern for better scalability.The current implementation uses
ops.FetchRelationshipswhich loads all HasSession relationships into memory at once. For environments with many session edges, this could cause high memory consumption or OOM issues.The existing
DeleteCollectedGraphDatafunction (referenced in relevant snippets) uses a more scalable streaming approach withops.StartNewOperation, readers, writers, and channels to process IDs incrementally without loading everything into memory.Consider refactoring to use the streaming pattern:
func (s Resources) deleteHasSessionEdges(ctx context.Context, auditEntry *model.AuditEntry) (failure bool) { - // Use the graph batch API to find and delete all HasSession relationships - if err := s.Graph.BatchOperation(ctx, func(batch graph.Batch) error { - targetCriteria := query.Kind(query.Relationship(), ad.HasSession) - - rels, err := ops.FetchRelationships(batch.Relationships().Filter(targetCriteria)) - if err != nil { - return err - } - - for _, rel := range rels { - if err := batch.DeleteRelationship(rel.ID); err != nil { - return err - } - } - - return nil - }); err != nil { + operation := ops.StartNewOperation[graph.ID](ops.OperationContext{ + Parent: ctx, + DB: s.Graph, + NumReaders: 1, + NumWriters: 1, + }) + + operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- graph.ID) error { + return tx.Relationships().Filter( + query.Kind(query.Relationship(), ad.HasSession), + ).FetchIDs(func(cursor graph.Cursor[graph.ID]) error { + for id := range cursor.Chan() { + select { + case <-ctx.Done(): + return ctx.Err() + case outC <- id: + } + } + return cursor.Error() + }) + }) + + operation.SubmitWriter(func(ctx context.Context, batch graph.Batch, inC <-chan graph.ID) error { + for { + select { + case <-ctx.Done(): + return ctx.Err() + case id, ok := <-inC: + if !ok { + return nil + } + if err := batch.DeleteRelationship(id); err != nil { + return err + } + } + } + }) + + if err := operation.Done(); err != nil { slog.ErrorContext(ctx, fmt.Sprintf("%s: %s", "there was an error deleting HasSession edges", err.Error())) s.handleAuditLogForDatabaseWipe(ctx, auditEntry, false, "HasSession edges") return true } else { s.handleAuditLogForDatabaseWipe(ctx, auditEntry, true, "HasSession edges") return false } }packages/go/openapi/doc/openapi.json (1)
13319-13319: Clarify description and align terminology with schema.
- The description says “custom high value selectors,” but the request body uses
deleteAssetGroupSelectors. Consider replacing with “asset group selectors” for consistency and to avoid ambiguity. Also, briefly state thatdeleteHasSessionEdgesdeletes only HasSession relationships, not nodes. Adding a small request example would help.Example (description tweak only):
- Wipes your BloodHound data permanently. Specify the data to delete in the request body. Possible data includes collected graph data, HasSession relationship edges, custom high value selectors, file ingest history, and data quality history. + Wipes your BloodHound data permanently. Specify the data to delete in the request body. Possible data includes collected graph data, HasSession relationship edges, asset group selectors, file ingest history, and data quality history.Optionally add a request example under
requestBody.content.application/json.example:{ "deleteHasSessionEdges": true }Based on learnings, this JSON file is generated; please ensure the change is applied to the YAML under
packages/go/openapi/src/schemasand the spec is regenerated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmd/api/src/api/v2/database_wipe.go(4 hunks)cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx(8 hunks)packages/go/openapi/doc/openapi.json(2 hunks)packages/go/openapi/src/paths/data-quality.clear-database.yaml(2 hunks)packages/javascript/js-client-library/src/requests.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-08T19:01:53.112Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:108-148
Timestamp: 2025-09-08T19:01:53.112Z
Learning: In BloodHound's CypherSearch component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the sharing state reset for sharedIds and isPublic after the two-step permissions update is handled elsewhere in the codebase, so additional state reset callbacks in the updateQueryPermissions function are not needed.
Applied to files:
cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx
📚 Learning: 2025-11-25T22:11:53.509Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.509Z
Learning: In cmd/api/src/database/graphschema.go, the CreateSchemaEdgeKind method intentionally does not use AuditableTransaction or audit logging because it would create too much noise in the audit log, unlike CreateGraphSchemaExtension which does use auditing.
Applied to files:
cmd/api/src/api/v2/database_wipe.go
📚 Learning: 2025-08-28T16:43:43.961Z
Learnt from: mvlipka
Repo: SpecterOps/BloodHound PR: 1784
File: packages/go/openapi/doc/openapi.json:18008-18029
Timestamp: 2025-08-28T16:43:43.961Z
Learning: In SpecterOps/BloodHound, packages/go/openapi/doc/openapi.json is generated from YAML under packages/go/openapi/src/schemas; edits must be made to the YAML and then the spec regenerated.
Applied to files:
packages/go/openapi/doc/openapi.json
📚 Learning: 2025-11-25T18:07:04.522Z
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 2083
File: cmd/api/src/model/graphschema.go:42-53
Timestamp: 2025-11-25T18:07:04.522Z
Learning: In the BloodHound codebase (SpecterOps/BloodHound), database models are moving toward a newer architecture where API validation is handled via `jsonschema` at the handler level, which obviates the need for json validation tags on database models themselves.
Applied to files:
packages/go/openapi/doc/openapi.json
🧬 Code graph analysis (1)
cmd/api/src/api/v2/database_wipe.go (2)
cmd/api/src/daemons/datapipe/delete.go (1)
DeleteCollectedGraphData(33-113)packages/go/graphschema/ad/ad.go (2)
HasSession(53-53)Relationships(1149-1151)
🔇 Additional comments (5)
packages/go/openapi/src/paths/data-quality.clear-database.yaml (1)
24-25: LGTM! Schema changes are correct and consistent.The OpenAPI schema updates properly document and expose the new HasSession edge deletion capability. The field naming and structure follow the existing patterns for other deletion options.
Also applies to: 38-39
cmd/api/src/api/v2/database_wipe.go (1)
32-35: LGTM! Integration and validation changes are correct.The struct field addition, imports, empty-request validation, and main workflow integration all follow the established patterns used by other deletion options in this file.
Also applies to: 41-41, 70-70, 195-200
packages/javascript/js-client-library/src/requests.ts (1)
241-241: LGTM! TypeScript interface update is correct.The optional field addition properly mirrors the backend API and OpenAPI schema, maintaining type consistency across the stack.
cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx (1)
42-42: LGTM! UI implementation is correct and consistent.The React component changes properly integrate the new HasSession edges deletion option:
- State management follows the established patterns for other checkboxes
- The reducer correctly handles initialization, selection, reset, and validation
- The UI checkbox respects permissions and is appropriately labeled
- The mutation payload correctly includes the new field
All changes align with the existing implementation patterns in this component.
Also applies to: 59-59, 107-107, 137-137, 177-177, 224-224, 263-263, 371-381
packages/go/openapi/doc/openapi.json (1)
13334-13336: Based on my verification, I have sufficient information to assess the review comment. The key findings are:
Context confirmed: The review is for proposed changes adding
deleteHasSessionEdgesto the clear-database endpoint—this field does not yet exist in the codebase.YAML source verified: The source file at
packages/go/openapi/src/paths/data-quality.clear-database.yamlcurrently defines 4 properties without individual descriptions.Current properties: deleteCollectedGraphData, deleteFileIngestHistory, deleteDataQualityHistory, deleteAssetGroupSelectors
Review concerns are valid:
- Naming consistency across layers (Go, YAML, UI, JS client) is a legitimate concern
- Input validation (ensuring at least one flag) is a reasonable schema pattern
- Adding descriptions to properties follows API documentation best practices
- Regenerating from YAML (not hand-editing JSON) is the correct process
The review comment is technically sound. However, I can streamline it by removing the overly complex verification script (which searches for typos that don't exist yet) and focusing on the core actionable items.
Verify the new
deleteHasSessionEdgesfield is consistently named across all implementation layers and properly validated in the schema.
- Field name follows existing camelCase pattern (good). Ensure it matches exactly in: Go request struct/handler, YAML schema, UI form key, and JS client.
- Add a short description for this property in the YAML schema (other properties currently lack individual descriptions; consider adding for consistency).
- Consider adding schema validation (e.g.,
anyOfconstraint) to require at least one delete flag to prevent no-op requests.- Critical: Update
packages/go/openapi/src/paths/data-quality.clear-database.yamlwith this field, then regeneratepackages/go/openapi/doc/openapi.jsonper project standards. Do not edit the JSON file directly.
|
This is a great PR. Going to take some time to review it and go through our workflow. Thank you! |
|
recheck |
urangel
changed the title
|
There is a failing test in the MR. Could you patch that up so it fixes the UI test run? |
|
Also we require commit signing as part of our PR process. Can you set up commit signing and then use a squash to sign all of your commits into a single signed one? The process for squashing is here: https://www.freecodecamp.org/news/git-squash-commits/ And the process for setting up commit signing is here: https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits |
rtpt-romankarwacik
force-pushed
the
main
branch
from
44783e1 to
0ffaec4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/api/src/api/v2/database_wipe.go (1)
41-41: DeleteHasSessionEdges flag wiring looks sound; clarify interaction with full graph deletion and feature flagThe new
DeleteHasSessionEdgesfield is correctly:
- Exposed on the
DatabaseWipepayload with a safe default (false), so existing clients remain compatible.- Included in the “empty request” check, allowing a request that only deletes HasSession edges.
- Plumbed into the handler with consistent error aggregation and audit logging via
deleteHasSessionEdges.Two behavioral points you may want to tighten up:
Combination with full graph deletion
Right now, a request can set bothdeleteCollectedGraphData(ordeleteSourceKinds) anddeleteHasSessionEdges. Functionally this means:
- You enqueue a full collected-graph deletion job, and
- You synchronously wipe HasSession edges in the same request.
That’s redundant at best and could be confusing from an API/UX perspective. Consider either:- Explicitly rejecting this combination (similar to the existing
deleteCollectedGraphDatavsdeleteSourceKindsguard), or- Clearly documenting that
DeleteHasSessionEdgesis ignored when a full graph delete is requested.Feature flag coverage
TheFeatureClearGraphDataflag currently gates onlyDeleteCollectedGraphData/DeleteSourceKinds, notDeleteHasSessionEdges. If that feature flag is intended as a general “no destructive graph operations” kill switch, you may want to includeDeleteHasSessionEdgesunder the same guard; otherwise, this is a deliberate carve‑out that should be documented.These are semantics/UX clarifications rather than correctness issues, but addressing them now will prevent surprising behavior for API clients and operators.
Also applies to: 69-71, 195-200
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmd/api/src/api/v2/database_wipe.go(4 hunks)cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx(8 hunks)packages/go/openapi/doc/openapi.json(2 hunks)packages/go/openapi/src/paths/data-quality.clear-database.yaml(2 hunks)packages/javascript/js-client-library/src/requests.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/javascript/js-client-library/src/requests.ts
- packages/go/openapi/src/paths/data-quality.clear-database.yaml
- cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx
- packages/go/openapi/doc/openapi.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-25T22:11:53.518Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.518Z
Learning: In cmd/api/src/database/graphschema.go, the CreateSchemaEdgeKind method intentionally does not use AuditableTransaction or audit logging because it would create too much noise in the audit log, unlike CreateGraphSchemaExtension which does use auditing.
Applied to files:
cmd/api/src/api/v2/database_wipe.go
🧬 Code graph analysis (1)
cmd/api/src/api/v2/database_wipe.go (2)
cmd/api/src/daemons/datapipe/delete.go (1)
DeleteCollectedGraphData(33-113)packages/go/graphschema/ad/ad.go (1)
HasSession(53-53)
🔇 Additional comments (2)
cmd/api/src/api/v2/database_wipe.go (2)
32-36: New graph-related imports line up with the HasSession deletion helperThe added imports for
adgraphschema,ops, andqueryare consistent with the newdeleteHasSessionEdgesimplementation and appear correctly scoped and used; no issues here.
251-276: Implementation follows established patterns and is functionally correctThe core logic is correct and consistent with how
HasSessionrelationships are deleted elsewhere in the codebase. The use ofops.FetchRelationshipswithins.Graph.BatchOperationis the standard pattern throughout BloodHound's migration code and batch relationship operations.While DAWGS does provide cursor-based streaming via
FetchIDsandCursor[T]for incrementally processing results, the codebase consistently uses theFetchRelationshipspattern for relationship batch deletions. The streaming/pipelined approach referenced in the comment is primarily used byDeleteCollectedGraphDatafor mass node deletions, not targeted relationship deletions.There is no evidence of practical scalability issues with this pattern in the codebase, and the implementation aligns with established practices. This can be deferred as a future optimization if real-world performance data indicates a problem.
rtpt-romankarwacik
force-pushed
the
main
branch
from
0ffaec4 to
2e765f3
Compare
Done 👍
Done 👍 Thank you for your review 😊 |
rvazarkar
left a comment
rvazarkar
left a comment
There was a problem hiding this comment.
This is fantastic, thank you for the extra effort. We're going to go through our final review process here and hopefully get this merged in soon!
mistahj67
left a comment
mistahj67
left a comment
There was a problem hiding this comment.
Code LGTM. Pulled and ran locally as well to confirm edges properly deleted and audit log was correctly inserted. 🚀
rtpt-romankarwacik
force-pushed
the
main
branch
3 times, most recently
from
ddfda34 to
70ff963
Compare
Updated OpenAPI for new HasSession property Generalized deleting of relatioships
rtpt-romankarwacik
force-pushed
the
main
branch
from
70ff963 to
677e98f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx (1)
371-381: Consider a more user-friendly label.The label "HasSession edges" uses technical graph terminology that may not be intuitive for all users. Consider using more descriptive text like "User session data" or adding a tooltip to explain what HasSession edges represent.
Example with improved label:
<FormControlLabel - label="HasSession edges" + label="User session data (HasSession edges)" control={ <Checkbox checked={deleteHasSessionEdges} onChange={handleCheckbox} name='deleteHasSessionEdges' disabled={!hasPermission} /> } />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/api/src/api/v2/database_wipe.go(4 hunks)cmd/ui/src/views/DatabaseManagement/DatabaseManagement.test.tsx(1 hunks)cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx(8 hunks)packages/go/openapi/doc/openapi.json(2 hunks)packages/go/openapi/src/paths/data-quality.clear-database.yaml(2 hunks)packages/javascript/js-client-library/src/requests.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/ui/src/views/DatabaseManagement/DatabaseManagement.test.tsx
- packages/go/openapi/doc/openapi.json
- packages/go/openapi/src/paths/data-quality.clear-database.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-25T22:11:53.518Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.518Z
Learning: In cmd/api/src/database/graphschema.go, the CreateSchemaEdgeKind method intentionally does not use AuditableTransaction or audit logging because it would create too much noise in the audit log, unlike CreateGraphSchemaExtension which does use auditing.
Applied to files:
cmd/api/src/api/v2/database_wipe.go
📚 Learning: 2025-09-08T19:01:53.112Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:108-148
Timestamp: 2025-09-08T19:01:53.112Z
Learning: In BloodHound's CypherSearch component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the sharing state reset for sharedIds and isPublic after the two-step permissions update is handled elsewhere in the codebase, so additional state reset callbacks in the updateQueryPermissions function are not needed.
Applied to files:
cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx
🔇 Additional comments (11)
packages/javascript/js-client-library/src/requests.ts (1)
241-241: LGTM!The addition of the optional
deleteRelationshipsfield to theClearDatabaseRequestinterface is correct and aligns with the backend payload structure.cmd/api/src/api/v2/database_wipe.go (4)
33-34: LGTM!The new imports for
opsandquerypackages are necessary for the edge deletion functionality.
38-40: LGTM!The
DeleteRelationshipsfield properly extends theDatabaseWipestruct to support selective edge deletion by name.
69-69: LGTM!The empty request validation correctly includes the new
DeleteRelationshipsfield.
194-199: LGTM!The conditional block properly integrates edge deletion into the database wipe flow with appropriate error handling and audit logging.
cmd/ui/src/views/DatabaseManagement/DatabaseManagement.tsx (6)
42-42: LGTM!The new
deleteHasSessionEdgesstate is properly initialized in the initial state.
59-59: LGTM!The
deleteHasSessionEdgesfield is correctly added to theStatetype.
107-107: LGTM!The state is properly reset to
falseafter successful mutation, consistent with other checkbox states.
137-137: LGTM!The
deleteHasSessionEdgesstate is correctly included in the no-selection validation check.
177-177: LGTM!The state is properly destructured for use in the component.
224-224: LGTM!The mapping from
deleteHasSessionEdgestodeleteRelationships: ['HasSession']correctly translates the UI state to the expected API payload format.
| func (s Resources) deleteEdges(ctx context.Context, auditEntry *model.AuditEntry, edgeNames []string) (failure bool) { | ||
| // Use the graph batch API to find and delete relationships matching the provided edge names | ||
| if err := s.Graph.BatchOperation(ctx, func(batch graph.Batch) error { | ||
| for _, name := range edgeNames { | ||
| targetCriteria := query.Kind(query.Relationship(), graph.StringKind(name)) | ||
|
|
||
| rels, err := ops.FetchRelationships(batch.Relationships().Filter(targetCriteria)) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| for _, rel := range rels { | ||
| if err := batch.DeleteRelationship(rel.ID); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| }); err != nil { | ||
| slog.ErrorContext(ctx, fmt.Sprintf("%s: %s", "there was an error deleting graph edges", err.Error())) | ||
| s.handleAuditLogForDatabaseWipe(ctx, auditEntry, false, strings.Join(edgeNames, ", ")) | ||
| return true | ||
| } else { | ||
| s.handleAuditLogForDatabaseWipe(ctx, auditEntry, true, strings.Join(edgeNames, ", ")) | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for documentation or examples of batch relationship deletion patterns
# to understand if there are memory-efficient alternatives
# Look for other uses of ops.FetchRelationships to see if pagination is used
rg -n -C3 -A5 'ops\.FetchRelationships' --type=go
# Look for batch deletion patterns in the codebase
rg -n -C3 'batch\.DeleteRelationship' --type=go
# Check if there are examples of validating relationship kinds
rg -n -C3 'FetchKinds.*Relationship' --type=goRepository: SpecterOps/BloodHound
Length of output: 23132
🏁 Script executed:
# Also look for how edgeNames parameter might be validated or constrained
rg -n -C3 'deleteEdges' --type=go
# Look for streaming or pagination patterns in batch operations
rg -n -C3 'batch.*Stream\|batch.*Pagina' --type=go
# Check the Graph.BatchOperation signature and documentation
rg -n -C5 'BatchOperation' --type=go | head -50Repository: SpecterOps/BloodHound
Length of output: 4650
🏁 Script executed:
# Search for validation of relationship/edge types in the codebase
rg -n -C3 'Relationship.*Kind\|Kind.*Relationship' --type=go
# Look for any TODO/FIXME comments related to memory or validation
rg -n 'TODO|FIXME' cmd/api/src/api/v2/database_wipe.goRepository: SpecterOps/BloodHound
Length of output: 47
🏁 Script executed:
# Find where payload.DeleteRelationships is defined
rg -n -A5 'DeleteRelationships' --type=go cmd/api/src/api/v2/database_wipe.go
# Look for the payload structure definition
rg -n -B5 -A10 'type.*[Pp]ayload.*struct' cmd/api/src/api/v2/database_wipe.go
# Check for any validation of edge names in the function or nearby
rg -n -B10 'deleteEdges' cmd/api/src/api/v2/database_wipe.go | head -50Repository: SpecterOps/BloodHound
Length of output: 2093
🏁 Script executed:
# Look for relationship kind definitions to understand valid edge types
fd -n '\.go$' packages/go/analysis | xargs rg -l 'Relationship.*=.*string' | head -5
# Check if there are constants defining valid relationship kinds
rg -n -B2 -A2 'const.*Relationship' packages/go/analysis/ad/ | head -50Repository: SpecterOps/BloodHound
Length of output: 492
🏁 Script executed:
# Inspect the complete deleteEdges function and surrounding context
sed -n '190,280p' cmd/api/src/api/v2/database_wipe.goRepository: SpecterOps/BloodHound
Length of output: 3505
🏁 Script executed:
# Check if there's a list of valid relationship types that could be validated against
fd -type f '\.go$' packages/go/graphschema | head -10
# Look for relationship kind constants or definitions
rg -n 'const.*=' packages/go/graphschema/ad/identifiers.go | grep -i 'relationship\|raw\|owns' | head -20Repository: SpecterOps/BloodHound
Length of output: 317
🏁 Script executed:
# Check if there's any API documentation or swagger definition for this endpoint
fd -name '*database*wipe*' --type f
# Look for test cases to understand expected behavior
rg -n 'deleteEdges\|DeleteRelationships' --type=go -A10 -B5 | grep -i testRepository: SpecterOps/BloodHound
Length of output: 295
🏁 Script executed:
# Find graphschema files
fd 'identifiers\.go' packages/go/
# Look for relationship definitions
rg 'const.*=' packages/go/graphschema/ | grep -i 'own\|raw' | head -20Repository: SpecterOps/BloodHound
Length of output: 109
🏁 Script executed:
# Search for test cases related to deleteEdges or DeleteRelationships
rg 'deleteEdges|DeleteRelationships' --type=go -B3 -A8 | grep -A8 -B3 'Test\|test' | head -50Repository: SpecterOps/BloodHound
Length of output: 47
🏁 Script executed:
# Look at the complete payload structure in database_wipe.go to understand all context
cat cmd/api/src/api/v2/database_wipe.go | grep -A50 'type.*Payload.*struct'Repository: SpecterOps/BloodHound
Length of output: 47
🏁 Script executed:
# Get the payload struct definition
sed -n '30,50p' cmd/api/src/api/v2/database_wipe.go
# Look for any validation logic before deleteEdges is called
sed -n '50,200p' cmd/api/src/api/v2/database_wipe.go | head -80Repository: SpecterOps/BloodHound
Length of output: 3537
🏁 Script executed:
# Search for where relationship kind constants might be defined
fd identifiers.go
# Look at the graphschema structure
ls -la packages/go/graphschema/Repository: SpecterOps/BloodHound
Length of output: 643
🏁 Script executed:
# Check if there are any known valid relationship types or validation logic
rg 'valid.*Relationship|Relationship.*valid|KindIn' cmd/api/src/api/v2/ -A3 -B3Repository: SpecterOps/BloodHound
Length of output: 8272
Add input validation for edge names against known relationship types and consider memory efficiency for large deletions.
The deleteEdges function has two issues:
-
Missing input validation: Edge names bypass the validation pattern established elsewhere in the codebase (e.g.,
pathfinding.govalidates againstgraph.Kinds(ad.Relationships()).Concatenate(azure.Relationships())). Invalid edge names will silently result in zero deleted relationships without alerting the user. -
Memory inefficiency: The function loads all matching relationships into memory via
ops.FetchRelationshipsbefore deletion. This pattern is consistent throughout the codebase but lacks pagination or streaming alternatives. For large relationship sets, this creates unnecessary memory pressure.
Consider validating edge names against valid relationship types and exploring whether the batch API supports more efficient deletion patterns.
4 participants
Description
This pull requests adds an option for database management to delete HasSession edges. The pull request has been created using GitHub Copilot.
#853
Motivation and Context
When analyzing sessions the current state is very important. Currently, when ingesting new sessions, the old ones are not deleted. This resolves #853
How Has This Been Tested?
I tested it with dummy data from the UI, where i queried all sessions after insertion, and after deletion.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.