Add option to delete HasSession edges #2110
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
WalkthroughThis PR adds a new optional feature to the database wipe endpoint that allows selective deletion of HasSession relationship edges. It extends the API schema, backend deletion logic, React UI, and JavaScript client library with a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as DatabaseManagement UI
participant API as Backend API
participant Graph as Graph Database
participant Audit as Audit Log
User->>UI: Checks "Delete HasSession" checkbox
User->>UI: Submits database wipe request
UI->>API: POST /api/v2/clear-database<br/>{deleteHasSessionEdges: true, ...}
rect rgb(200, 220, 255)
Note over API,Graph: HasSession Deletion Flow (if enabled)
API->>Graph: Query all (Computer)-[HasSession]->(User)
Graph-->>API: Return HasSession edges
API->>Graph: Batch delete HasSession edges
Graph-->>API: Deletion complete
API->>Audit: Log HasSession deletion success/failure
end
API-->>UI: Success response
UI->>User: Display confirmation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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://cdn.statically.io/img/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.
1 participant
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
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.