BED-5725 Additional Integration Tests for Remaining Fixture Files #1665
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TODO: burn it all with fire
WalkthroughThis update introduces a comprehensive set of new JSON fixture files for versions 5 and 6 of the graphify ingestion service, covering entities such as computers, users, groups, domains, GPOs, and certificate authorities. It also adds extensive integration tests for these ingestion scenarios, improves assertion error messaging in graph comparison utilities, and applies minor code and formatting corrections. Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant IngestionService
participant Database
participant FixtureLoader
TestSuite->>FixtureLoader: Load fixture files (JSON/ZIP)
TestSuite->>IngestionService: Ingest fixture data
IngestionService->>Database: Insert/Update entities (computers, users, etc.)
TestSuite->>Database: Retrieve graph state
TestSuite->>FixtureLoader: Load expected graph
TestSuite->>TestSuite: Assert actual vs expected graph
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/rootcas.json (1)
44-54: Duplicate ACE entries inflate fixture size without adding semantic valueThe same
PrincipalSID(...-519) with identicalRightName"GenericAll"appears twice – once withIsInherited=false(L44-48) and once withIsInherited=true(L50-54). Unless the ingestion logic explicitly needs both variants, consider removing one of them to keep the fixture concise and avoid confusion when counting effective rights.cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/domains.json (1)
56-118: Several ACE rows repeat the same SID / right combinationMultiple
PrincipalSID/RightNamepairs (e.g. domain admins WriteDacl, WriteOwner, GetChanges) are duplicated within the same ACL block. If duplicated ACEs are not required for a specific edge-case test, trimming them will reduce noise and filesize.cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/ntauthstores.json (1)
40-56: Redundant ACE repeatsThe fixture lists the same “GenericAll” right for SID
…-519twice (once inherited, once not). Unless inheritance behaviour is under test, one entry should suffice.cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/gpos.json (1)
22-56: Duplicate permission triplets for domain adminsFor SID
…-512the trio WriteDacl, WriteOwner, GenericWrite is listed twice (L22-38 and L40-56). If duplicate ACEs are unintended, pruning them will streamline the test data.cmd/api/src/api/v2/apiclient/analysisrequest.go (1)
23-23: Remove trailing space from import line.There's an unnecessary trailing space at the end of the import line that should be removed for consistent formatting.
- "github.com/specterops/bloodhound/cmd/api/src/model" + "github.com/specterops/bloodhound/cmd/api/src/model"cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/ous.json (1)
30-67: ACE list duplicates inherited rights – optional size optimisationSeveral ACEs differ only in
IsInherited. While functionally correct, trimming duplicates where the inherited entry provides the same right can cut fixture size and ingest time for large suites. Not required, just a potential clean-up.cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/computers.json (1)
67-84: Inconsistent key nameLocalNamevs.LocalNamesAll other fixtures (v5 and v6) – and the SharpHound v5 schema docs – use the plural
LocalNames.
Please confirm the parser really expects the singular form here; otherwise rename for consistency.Also applies to: 95-110
cmd/api/src/services/graphify/tasks_integration_test.go (1)
102-180: Consider refactoring to reduce code duplication.The
TestVersion6AllJSONandTestVersion6ADCSJSONfunctions are nearly identical, differing only in their fixture paths. Consider extracting the common test logic into a helper function to improve maintainability.Example refactor:
+func testVersion6JSONIngestion(t *testing.T, fixturePath string) { + t.Helper() + var ( + ctx = context.Background() + testSuite = setupIntegrationTestSuite(t, fixturePath) + files = []string{ + // ... file list + } + ) + defer teardownIntegrationTestSuite(t, &testSuite) + // ... rest of test logic +} func TestVersion6AllJSON(t *testing.T) { t.Parallel() - var ( - ctx = context.Background() - fixturesPath = path.Join("fixtures", t.Name(), "ingest") - // ... rest of implementation - ) + testVersion6JSONIngestion(t, path.Join("fixtures", t.Name(), "ingest")) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
cmd/api/src/services/graphify/fixtures/TestVersion5IngestZIP/ingest/archive.zipis excluded by!**/*.zipcmd/api/src/services/graphify/fixtures/TestVersion6ADCSZIP/ingest/archive.zipis excluded by!**/*.zipcmd/api/src/services/graphify/fixtures/TestVersion6AllZIP/ingest/archive.zipis excluded by!**/*.zipcmd/api/src/services/graphify/fixtures/TestVersion6IngestZIP/ingest/archive.zipis excluded by!**/*.zip
📒 Files selected for processing (33)
cmd/api/src/api/v2/apiclient/analysisrequest.go(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/computers.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/containers.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/domains.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/gpos.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/groups.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/ous.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/sessions.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/users.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/aiacas.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/computers.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/domains.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/enterprisecas.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/gpos.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/issuancepolicies.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/ntauthstores.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/ous.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/rootcas.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/users.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/aiacas.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/computers.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/domains.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/enterprisecas.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/gpos.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/issuancepolicies.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/ntauthstores.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/ous.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/rootcas.json(1 hunks)cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/users.json(1 hunks)cmd/api/src/services/graphify/graphify_integration_test.go(2 hunks)cmd/api/src/services/graphify/tasks_integration_test.go(3 hunks)cmd/api/src/test/fixtures/fixtures/v6/all/ous.json(1 hunks)packages/go/lab/generic/graph.go(1 hunks)
🧰 Additional context used
🧠 Learnings (32)
📓 Common learnings
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/rootcas.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/computers.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/aiacas.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/graphify_integration_test.go (2)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: superlinkx
PR: SpecterOps/BloodHound#1503
File: cmd/api/src/services/job/jobs_test.go:19-143
Timestamp: 2025-05-27T16:58:33.295Z
Learning: Tests in cmd/api/src/services/job/jobs_test.go have been found to be flaky in the past and are due for rewrite. They should be skipped with t.Skip() until they can be properly rewritten.
cmd/api/src/api/v2/apiclient/analysisrequest.go (1)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/rootcas.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/test/fixtures/fixtures/v6/all/ous.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/ous.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/domains.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/ntauthstores.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/ntauthstores.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/domains.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/issuancepolicies.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/issuancepolicies.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/gpos.json (3)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:78-85
Timestamp: 2025-06-18T08:29:51.923Z
Learning: In Active Directory environments, GPOs are very rarely applied to child objects on multiple levels, so optimizations for preventing duplication in GPO inheritance processing may not be worth the added code complexity.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/computers.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/containers.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/users.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/computers.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/enterprisecas.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/gpos.json (3)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:78-85
Timestamp: 2025-06-18T08:29:51.923Z
Learning: In Active Directory environments, GPOs are very rarely applied to child objects on multiple levels, so optimizations for preventing duplication in GPO inheritance processing may not be worth the added code complexity.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/enterprisecas.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/aiacas.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/gpos.json (3)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:78-85
Timestamp: 2025-06-18T08:29:51.923Z
Learning: In Active Directory environments, GPOs are very rarely applied to child objects on multiple levels, so optimizations for preventing duplication in GPO inheritance processing may not be worth the added code complexity.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/sessions.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/domains.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/groups.json (3)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/ous.json (2)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/users.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/services/graphify/tasks_integration_test.go (5)
Learnt from: superlinkx
PR: SpecterOps/BloodHound#1503
File: cmd/api/src/services/job/jobs_test.go:19-143
Timestamp: 2025-05-27T16:58:33.295Z
Learning: Tests in cmd/api/src/services/job/jobs_test.go have been found to be flaky in the past and are due for rewrite. They should be skipped with t.Skip() until they can be properly rewritten.
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: LawsonWillard
PR: SpecterOps/BloodHound#1595
File: cmd/api/src/api/v2/saved_queries_test.go:2594-2594
Timestamp: 2025-06-17T22:37:36.389Z
Learning: In Go table-driven tests, there's a distinction between main test function parallelism and subtest parallelism. Main test functions can safely use t.Parallel() for performance benefits, but individual subtests within table-driven tests may need to run sequentially to avoid race conditions with mocks, deferred functions, or shared resources.
Learnt from: LawsonWillard
PR: SpecterOps/BloodHound#1595
File: cmd/api/src/api/v2/saved_queries_test.go:2197-2197
Timestamp: 2025-06-17T22:37:50.192Z
Learning: In Go tests using table-driven patterns, t.Parallel() at the function level allows test functions to run in parallel with other test functions, which is generally beneficial. However, when subtests contain deferred goroutines, the individual t.Run() calls should not use t.Parallel() to avoid flakiness. The comment "Tests will be flaky if run in parallel due to use of go routine in deferred function" specifically refers to subtest parallelization, not function-level parallelization.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/users.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
🧬 Code Graph Analysis (1)
cmd/api/src/services/graphify/graphify_integration_test.go (1)
cmd/api/src/database/db.go (1)
Database(64-174)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
🔇 Additional comments (25)
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/users.json (1)
1-1461: Well-structured JSON fixture file for comprehensive user testing.This fixture file provides extensive user data with realistic Active Directory attributes, ACEs, and metadata. The structure is consistent across all 14 user objects, making it suitable for thorough ingestion testing.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/gpos.json (1)
22-56: Potential duplicate ACE entries in GPO data.The ACE entries for the first GPO contain duplicates - the same permissions (WriteDacl, WriteOwner, GenericWrite) are listed multiple times for the same principals (S-1-5-21-909015691-3030120388-2582151266-512). This pattern is repeated in the second GPO as well.
This duplication might be intentional for testing edge cases, but please verify if this accurately represents the expected test data structure.
packages/go/lab/generic/graph.go (1)
165-165: Excellent improvement to assertion error messaging.Adding the descriptive error message
"expected node%s"significantly improves debugging when tests fail by clearly indicating which specific node was expected but missing.cmd/api/src/test/fixtures/fixtures/v6/all/ous.json (1)
2-81: Good JSON formatting improvements for better readability.The reformatting corrects indentation and nesting throughout the file, making it more readable and maintainable while preserving all the original data values.
cmd/api/src/services/graphify/graphify_integration_test.go (2)
122-126: Robust connection string parsing implementation.The changes improve parsing robustness by using
strings.Fields()for whitespace splitting and adding validation to prevent panics on malformed input. This is a solid defensive programming approach.
158-165: No further action needed: error handling is consistent
GraphDB.Close(ctx)returns anerror, which your teardown logs.BloodhoundDB.Close(ctx)returns nothing and already logs any errors internally.- Both nil checks are in place to prevent panics during test cleanup.
The current teardown implementation is appropriate and requires no changes.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/sessions.json (1)
1-15: Ingest fixtures intentionally usecount = 0regardless of data length
The TestVersion5IngestJSON (and TestVersion6IngestJSON) fixtures always setmeta.countto 0 by design, even when sample items are present. This pattern is consistent across all ingest fixtures and is meant to test the ingestion logic rather than reflect actual record counts—no changes needed.cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/aiacas.json (1)
1-79: Well-structured CA fixture data.The JSON structure is valid and contains comprehensive certificate authority data including properties, ACEs, and metadata. The count field correctly matches the data array length.
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/ntauthstores.json (1)
1-73: Comprehensive NT authentication store fixture data.The JSON structure is valid and contains detailed NT authentication store data including properties, certificate thumbprints, ACEs, and metadata. The count field correctly matches the data array length.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/rootcas.json (1)
1-78: Well-structured root CA fixture data.The JSON structure is valid and contains comprehensive root certificate authority data including properties, certificate details, ACEs, and metadata. The count field correctly matches the data array length.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/issuancepolicies.json (2)
8-14: Duplicate ACL-protection flags – pick one canonical casing/value pairEach object exposes the flag twice:
•Properties.isaclprotected(lower-case)
• top-levelIsACLProtected(upper-case)If the ingestion schema only maps one of these, the other silently becomes dead weight or—worse—causes per-object ambiguity when the two values diverge. Please confirm the schema expects both fields; otherwise remove the redundant one and stick to the canonical casing.
Also applies to: 58-60
65-80:GroupLink.ObjectIdentifierisnullwhileObjectTypeis"Base"Down-stream resolvers may interpret a
nullidentifier as “link absent”; mixing that with a concreteObjectTypecan break pointer dereferencing/edge construction. Verify this combination is valid for theMediumandHighassurance policies; if the link is intentionally absent, consider omittingGroupLinkentirely to avoid confusion.cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/aiacas.json (1)
19-68: LGTM – schema fields and ACE structure look consistentThe CA fixture conforms to the existing v6 schema; the ACE array mirrors other fixtures and the meta block matches conventions. No issues spotted.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/domains.json (1)
1-20: First domain object lacks essentialProperties.domainmetadataEntry at index 0 provides neither a
domainnornameinsideProperties, unlike the second object. If this is intended to represent a “placeholder/forest root”, confirm that the graphify loader handles missing fields gracefully; otherwise populate the standard domain attributes to avoid nil-pointer or validation errors.cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/ous.json (1)
1-82: Fixture looks internally consistent – no action needed
meta.countmatches the single OU entry and the JSON structure is valid.cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/computers.json (1)
1-550:meta.countcorrectly reflects the two-computer payloadThe metadata and payload sizes line up; nothing blocking here.
cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/users.json (1)
1-1460: Looks good – data count & schema align with v6 spec
meta.count(14) matches the number of user objects and the field naming is consistent with the existing v6 fixtures.
No issues spotted in this fixture.cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/computers.json (1)
1-548: Fixture is internally consistent
meta.countcorrectly reflects 2 computer objects and field names follow the v6 convention (LocalGroups.LocalNames, etc.).
No action required.cmd/api/src/services/graphify/fixtures/TestVersion6ADCSJSON/ingest/enterprisecas.json (1)
1-348: CA fixture validatedSingle-object CA fixture;
meta.countis correct and matches the companion v6 schema used elsewhere.
No concerns.cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/enterprisecas.json (1)
1-348: Duplicate of v6 ADCS CA fixture is acceptableThe content intentionally mirrors the ADCS fixture for the “AllJSON” test suite.
No discrepancies detected.cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/users.json (1)
1-1467: Comprehensive version 5 user fixture data looks good.This JSON fixture provides thorough test coverage for version 5 user ingestion with comprehensive Active Directory user properties, security descriptors, ACEs, SID history, and delegation permissions. The structure is well-formed and appropriate for integration testing.
cmd/api/src/services/graphify/tasks_integration_test.go (4)
32-65: Well-structured integration test for version 5 JSON ingestion.The test follows good patterns with proper setup, comprehensive assertions, and graph verification. The use of
t.Parallel()is appropriate for function-level parallelism.
182-208: Version 5 ZIP ingestion test is well-implemented.The test correctly validates ZIP file processing with appropriate total count assertion (8 items) and proper graph verification.
238-292: ZIP ingestion tests follow consistent patterns.The
TestVersion6AllZIPandTestVersion6ADCSZIPfunctions are well-structured and consistent with the established test patterns. The total count assertions (13 items) are appropriate for the comprehensive fixture sets.
72-72: Fixture path update is consistent.The change from "v6ingest" to "ingest" maintains consistency across all test functions and aligns with the new fixture directory structure.
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/ous.json
Show resolved
Hide resolved
cmd/api/src/services/graphify/fixtures/TestVersion6AllJSON/ingest/issuancepolicies.json
Show resolved
Hide resolved
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/domains.json
Show resolved
Hide resolved
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/containers.json
Show resolved
Hide resolved
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/gpos.json
Show resolved
Hide resolved
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/computers.json
Show resolved
Hide resolved
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/groups.json
Show resolved
Hide resolved
cmd/api/src/services/graphify/fixtures/TestVersion5IngestJSON/ingest/groups.json
Show resolved
Hide resolved
cweidenkeller
approved these changes
Jul 9, 2025
|
LGTM please squash commits tho =) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Replacement of previous integration tests to test ingest. This PR covers the remaining fixture files to test ingest post-PR.
Motivation and Context
Builds upon steel thread for BED-5725
Additional integration tests were created to test ingestion.
Asserts the remaining json data files with graph fixture
Asserts the remaining zip files (containing the json data files) with graph fixture
Flow
a. Reuse payload files from IngestJob Test as input
b. Set up database with a consistent fake job that appropriately points to the input files
c. Validate graph matches some graph fixture
How Has This Been Tested?
Passing integration tests
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit