docs: BloodHound RFC 4: OpenGraph Extension Fundamental Requirements BED-7034 #2169
Conversation
OpenGraph Extension Fundamental Requirements
WalkthroughAdds a new RFC ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
superlinkx
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
rfc/bh-rfc-4.md (4)
24-32: Clarify hybrid path cross‑extension semantics and tighten wordingThe intent around hybrid paths and intentional cross‑extension interactions is good, but the normative behavior is a bit underspecified:
- Line 25/28: “must be able to define hybrid paths… via referencing other extension's kinds” and “implicitly through references” doesn’t spell out how those references are expressed (symbol name only vs.
namespace + symbol, extension identifier, or something else). It would help implementers if this section explicitly defined the referencing mechanism and any stability guarantees (e.g., what happens if an extension is renamed or removed).- Lines 56‑59 (“Mitigation” bullets): this is where the cross‑namespace story is re‑emphasized; might be a good place to state that hybrid paths MUST reference foreign kinds by their fully‑qualified, namespaced symbol (or whatever the canonical form is), rather than relying on ambiguous short names.
Minor text/lint nits you can pick up while touching this section:
- Line 28: “extension defined identifiers” → “extension‑defined identifiers”.
- Line 28: add the period to “etc.”.
- Lines 58‑59: markdownlint is right that the sub‑bullets are indented more than the configured style (4 vs expected 2 spaces); adjusting here will quiet MD007.
Also applies to: 54-60
37-41: Tighten description of legacy validation bypass and migration expectationsThe note about SharpHound/AzureHound initially bypassing namespace validation is important, but currently leaves room for differing interpretations:
- It would help to be explicit about where the bypass lives (collector side vs. server side vs. both) and what “bypass” means from the perspective of the OpenGraph extension loader (e.g., “core AD/Azure extensions are treated as grandfathered and not subject to namespace checks until migration completes”).
- Consider adding a rough deprecation/migration statement, e.g., “New extensions MUST pass namespace validation immediately; the legacy AD/Azure extensions MAY bypass validation until the migration tool has been run and a minimum platform version has been reached.”
- The bullet “No longer bundle these extensions but allow for easy installation” could use a bit more color: e.g., is the expectation that they ship as first‑party extensions in a default registry, or that operators are responsible for installing them separately?
Clarifying these points in the RFC will make it easier for implementers and operators to align on the transition plan.
121-175: Clarify junction table requirements and lifecycle semantics in Kinds handlingThe ERD and the statement that extensions “should use junction tables when creating relationships with tables owned by DAWGS” are solid, but a few extra details would make this section more actionable:
- If using junction tables is intended to be mandatory for 3rd‑party extensions, consider using RFC language like “MUST use junction tables” instead of “should”, and reserve exceptions for core, internal migrations.
- It may be worth specifying expected delete/update behavior:
- What happens in
schema_node_kinds/schema_relationship_kinds/schema_environments(_principal_kinds)when an extension is uninstalled?- Are FKs
ON DELETE CASCADEfromschema_extensionsto these junction tables, and how is cleanup fromkindshandled?- You might also want to call out uniqueness expectations (e.g.,
(extension_id, kind_id)uniqueness inschema_node_kindsandschema_relationship_kinds) if those are considered part of the contract; that will inform how implementers design their schemas and migrations.These additions would make the “Kinds Table Handling” requirements less open to interpretation for both DB schema designers and extension authors.
195-199: Tighten environment validation rules and align creation/reactivation behaviorThe three environment validation rules are clear at a high level, but there are a couple of subtle points worth tightening:
- Rule 2 explicitly mentions “create if it doesn't, reactivate if it does” for
sourceKind, but Rules 1 and 3 only say “Ensure … exists” without specifying whether automatic creation/reactivation is allowed or whether failure should be treated as a hard validation error.- If the intended behavior is different (e.g.,
environmentKindandprincipalKindsmust already exist and failure is an error, butsourceKindcan be created on the fly), it would help to be explicit about that distinction and the rationale.- If, instead, the same create/reactivate behavior is intended for all three, consider aligning the wording across rules so implementers don’t accidentally special‑case
sourceKindonly.While you’re here, you could also slightly vary the phrasing (“Validate that…”, “Confirm that…”) to avoid the repeated “Ensure …” if you care about the stylistic lint warning, but the semantics are the more important part.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rfc/bh-rfc-4.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 1820
File: packages/go/stbernard/command/audit/audit.go:133-136
Timestamp: 2025-08-26T21:25:33.480Z
Learning: superlinkx prefers to receive feedback and identify issues as early as possible in the review process rather than later.
🪛 LanguageTool
rfc/bh-rfc-4.md
[grammar] ~28-~28: Use a hyphen to join words.
Context: ...aggressive namespacing for any extension defined identifiers (node kinds, edge ki...
(QB_NEW_EN_HYPHEN)
[style] ~28-~28: In American English, abbreviations like “etc.” require a period.
Context: ...rs (node kinds, edge kinds, properties, etc) - Avoid Attribute Collisions - Pr...
(ETC_PERIOD)
[style] ~199-~199: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... it doesn't, reactivate if it does). 3. Ensure all principalKinds exist.
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
rfc/bh-rfc-4.md
58-58: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ 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). (3)
- GitHub Check: run-analysis
- GitHub Check: run-tests
- GitHub Check: build-ui
|
|
||
| The above schema is invalid because `User` should be prefixed (e.g., `GH_User`). | ||
|
|
||
| ## 7. Handling Customization |
There was a problem hiding this comment.
this section is vague. what kind of customization are we talking about? node icons and colors?
There was a problem hiding this comment.
It's somewhat intentionally vague (this RFC is meant to cover the underlying philosophy of extensions, including future additions we haven't explored yet. But I can add the icon/color examples in, that will help give better guidance than the way this ended up
and updates to clarity in a couple sections
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rfc/bh-rfc-4.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 1820
File: packages/go/stbernard/command/audit/audit.go:133-136
Timestamp: 2025-08-26T21:25:33.480Z
Learning: superlinkx prefers to receive feedback and identify issues as early as possible in the review process rather than later.
🪛 LanguageTool
rfc/bh-rfc-4.md
[grammar] ~28-~28: Use a hyphen to join words.
Context: ...aggressive namespacing for any extension defined identifiers (node kinds, edge ki...
(QB_NEW_EN_HYPHEN)
[style] ~28-~28: In American English, abbreviations like “etc.” require a period.
Context: ...rs (node kinds, edge kinds, properties, etc) - Avoid Attribute Collisions - Pr...
(ETC_PERIOD)
[style] ~201-~201: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... it doesn't, reactivate if it does). 3. Ensure all principalKinds exist.
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
rfc/bh-rfc-4.md
58-58: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ 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). (3)
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
| Extensions must be able to define hybrid paths, despite their namespacing requirements. This requirement should be maintainable via referencing other extension's kinds when defining hybrid paths. | ||
|
|
||
| ### 2.3 Intentional Interactions vs Unintentional Collisions | ||
| BloodHound needs to support intentional interactions (such as hybrid paths) implicitly through references, and must avoid unintentional collisions through aggressive namespacing for any extension defined identifiers (node kinds, edge kinds, properties, etc) |
There was a problem hiding this comment.
Fix grammar: compound adjective and abbreviation period.
Line 28 should read extension-defined (with hyphen) and end properties, etc.) (with period before closing paren).
-BloodHound needs to support intentional interactions (such as hybrid paths) implicitly through references, and must avoid unintentional collisions through aggressive namespacing for any extension defined identifiers (node kinds, edge kinds, properties, etc)
+BloodHound needs to support intentional interactions (such as hybrid paths) implicitly through references, and must avoid unintentional collisions through aggressive namespacing for any extension-defined identifiers (node kinds, edge kinds, properties, etc.)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BloodHound needs to support intentional interactions (such as hybrid paths) implicitly through references, and must avoid unintentional collisions through aggressive namespacing for any extension defined identifiers (node kinds, edge kinds, properties, etc) | |
| BloodHound needs to support intentional interactions (such as hybrid paths) implicitly through references, and must avoid unintentional collisions through aggressive namespacing for any extension-defined identifiers (node kinds, edge kinds, properties, etc.) |
🧰 Tools
🪛 LanguageTool
[grammar] ~28-~28: Use a hyphen to join words.
Context: ...aggressive namespacing for any extension defined identifiers (node kinds, edge ki...
(QB_NEW_EN_HYPHEN)
[style] ~28-~28: In American English, abbreviations like “etc.” require a period.
Context: ...rs (node kinds, edge kinds, properties, etc) - Avoid Attribute Collisions - Pr...
(ETC_PERIOD)
🤖 Prompt for AI Agents
In rfc/bh-rfc-4.md around line 28, fix the grammar by changing "extension
defined identifiers" to the hyphenated compound adjective "extension-defined
identifiers" and ensure the parenthetical ends with a period before the closing
parenthesis so the phrase reads "properties, etc.)." Replace the text on that
line accordingly.
| - Extensions should be focused on their own domain. | ||
| - Namespacing prevents conflicts. For hybrid paths between technologies, references will be allowed to types in other namespaces. |
There was a problem hiding this comment.
Adjust list indentation to match Markdown convention.
Lines 58–59 use 4 spaces but should use 2 spaces for nested bullets under the mitigation list.
- **Mitigation**:
- - Extensions should be focused on their own domain.
- - Namespacing prevents conflicts. For hybrid paths between technologies, references will be allowed to types in other namespaces.
+ - Extensions should be focused on their own domain.
+ - Namespacing prevents conflicts. For hybrid paths between technologies, references will be allowed to types in other namespaces.🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
58-58: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In rfc/bh-rfc-4.md around lines 58 to 59, the nested list items under the
mitigation list use 4-space indentation instead of the 2-space indentation
preferred for Markdown lists; update those two lines to use 2 spaces for the
nested bullets so they render consistently as sub-items under the parent bullet.
| "environments": [ | ||
| { | ||
| "environmentKind": "GH_Organization", | ||
| "sourceKind": "GHBase", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n rfc/bh-rfc-4.md | sed -n '75,100p'Repository: SpecterOps/BloodHound
Length of output: 876
🏁 Script executed:
cat -n rfc/bh-rfc-4.md | sed -n '180,210p'Repository: SpecterOps/BloodHound
Length of output: 843
🏁 Script executed:
cat -n rfc/bh-rfc-4.md | sed -n '1,50p'Repository: SpecterOps/BloodHound
Length of output: 2686
🏁 Script executed:
cat -n rfc/bh-rfc-4.md | sed -n '88,120p'Repository: SpecterOps/BloodHound
Length of output: 1469
Clarify whether sourceKind is restricted to core kinds or can be extension-defined.
The RFC has unresolved ambiguity around the treatment of sourceKind:
-
Section 5 (line 94) lists
GHBaseas exempt from prefixing, citing it as "a source kind" not defined by the extension. However, this doesn't explicitly state whethersourceKinditself is restricted to core kinds. -
Section 10 (line 200) states that
sourceKindshould be created if it doesn't exist, implying extensions can define their own source kinds—yet no guidance is given on whether extension-defined source kinds must follow the naming convention (e.g.,GH_Basevs.GHBase). -
The manifest examples (lines 83, 190) show
sourceKindasGHBase(unprefixed), whileenvironmentKindandprincipalKindsare consistently prefixed (GH_Organization,GH_User).
Recommendation: Explicitly document in Section 5 whether sourceKind is exclusively core-owned (and thus exempt from prefixing) or whether extensions may define source kinds (in which case the naming rule should apply). Align the rule with the examples and the validation logic in Section 10.
| 1. Ensure the specified `environmentKind` exists. | ||
| 2. Ensure the specified `sourceKind` exists (create if it doesn't, reactivate if it does). | ||
| 3. Ensure all `principalKinds` exist. |
There was a problem hiding this comment.
Reduce repetitive sentence structure in validation rules.
Lines 199–201 have three consecutive sentences starting with "Ensure". Vary the structure for clarity.
1. Ensure the specified `environmentKind` exists.
-2. Ensure the specified `sourceKind` exists (create if it doesn't, reactivate if it does).
-3. Ensure all `principalKinds` exist.
+2. The specified `sourceKind` must exist (or be created and reactivated as needed).
+3. All `principalKinds` must be defined.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. Ensure the specified `environmentKind` exists. | |
| 2. Ensure the specified `sourceKind` exists (create if it doesn't, reactivate if it does). | |
| 3. Ensure all `principalKinds` exist. | |
| 1. Ensure the specified `environmentKind` exists. | |
| 2. The specified `sourceKind` must exist (or be created and reactivated as needed). | |
| 3. All `principalKinds` must be defined. |
🧰 Tools
🪛 LanguageTool
[style] ~201-~201: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... it doesn't, reactivate if it does). 3. Ensure all principalKinds exist.
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
rfc/bh-rfc-4.md around lines 199 to 201: the three validation rules each begin
with the word "Ensure", producing repetitive sentence structure; rewrite these
three lines to vary sentence starters and improve clarity while preserving
meaning (for example: "Verify the specified `environmentKind` exists.", "If the
specified `sourceKind` does not exist, create it; if it exists but is inactive,
reactivate it.", "Confirm that all `principalKinds` are present."), keeping the
same order and intent.
Description
This RFC proposal covers the design space and requirements for how BloodHound will need to handle the upcoming OpenGraph extensions feature. This PR is meant to be a place for the OpenGraph teams to discuss and refine this RFC as design completes the solidification process. Once all technical leaders of the OpenGraph teams have approved this RFC, it will be considered final and merged for our records.
Motivation and Context
Resolves BED-7034
Time for a more mature design process for our most complex projects
How Has This Been Tested?
Documentation only, no testing only approvals
Types of changes
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.