feat: Add bounded graph expansion API BP-2366 - #3089
Open
justin-prime1 wants to merge 2 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesThe PR adds Graph expansion API
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ExpandGraph
participant GraphDatabase
participant ETACFiltering
Client->>ExpandGraph: POST graph expansion payload
ExpandGraph->>GraphDatabase: Execute bounded Cypher query
GraphDatabase-->>ExpandGraph: Return graph results
ExpandGraph->>ETACFiltering: Filter graph results
ETACFiltering-->>ExpandGraph: Return filtered results
ExpandGraph-->>Client: Return nodes, edges, limit, and truncation
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
justin-prime1
marked this pull request as ready for review
August 3, 2026 17:59
coderabbitai
Bot
added
api
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
cmd/api/src/api/v2/graphexpansion.go (2)
80-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGroup declarations into a
var (...)block.
filteredRelationshipKinds,seenRelationshipKinds, andrelationshipMatchare declared at different points in the function instead of being grouped near the top.♻️ Proposed refactor to hoist declarations
func buildGraphExpansionQuery(nodeID int64, direction string, relationshipKinds []string, limit int) (string, error) { - filteredRelationshipKinds := make([]string, 0, len(relationshipKinds)+1) - seenRelationshipKinds := map[string]struct{}{} + var ( + filteredRelationshipKinds = make([]string, 0, len(relationshipKinds)+1) + seenRelationshipKinds = map[string]struct{}{} + relationshipMatch string + ) for _, relationshipKind := range append([]string{graphExpansionBuiltInKinds}, relationshipKinds...) { if !graphExpansionRelationshipKindPattern.MatchString(relationshipKind) { return "", fmt.Errorf("invalid relationship kind: %s", relationshipKind) } if _, seen := seenRelationshipKinds[relationshipKind]; seen { continue } seenRelationshipKinds[relationshipKind] = struct{}{} filteredRelationshipKinds = append(filteredRelationshipKinds, relationshipKind) } - relationshipMatch := "" relationshipFilter := strings.Join(filteredRelationshipKinds, "|")As per coding guidelines, "When possible, group variable initializations in a
var (...)block and hoist them to the top of the function."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/api/src/api/v2/graphexpansion.go` around lines 80 - 98, Update buildGraphExpansionQuery to group the initial declarations of filteredRelationshipKinds, seenRelationshipKinds, and relationshipMatch in a single var block near the top of the function, while preserving their existing initialization values and subsequent behavior.Source: Coding guidelines
259-259: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPreserve properties required by ETAC filtering.
filterETACGraphreadsnode.Propertiesto determine environment access. IfRawCypherQueryreceivesfalsewhile ETAC filtering is active, all nodes become hidden. If optimization is required, request properties whenpayload.IncludeProperties || filterForETACand strip them from the response whenpayload.IncludePropertiesis false.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/api/src/api/v2/graphexpansion.go` at line 259, Update the RawCypherQuery call in the graph expansion handler to request properties whenever payload.IncludeProperties or filterForETAC is true, so filterETACGraph can evaluate node environment access; when properties were requested only for ETAC filtering and payload.IncludeProperties is false, remove them before returning the response.cmd/api/src/api/v2/graphexpansion_test.go (1)
45-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the ETAC-filtered path.
This test uses
model.User{AllEnvironments: true}, soShouldFilterForETACtakes the non-filtering branch andfilterETACGraphis never exercised. Add a test case with a user that doesn't haveAllEnvironmentsto confirmfilterETACGraphcorrectly restricts nodes and edges before the response is pruned and returned.Do you want me to generate this additional test case?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/api/src/api/v2/graphexpansion_test.go` around lines 45 - 122, Add a separate ExpandGraph test case using a user without AllEnvironments so ShouldFilterForETAC enters the filtering branch and filterETACGraph is exercised. Configure the graph fixture with ETAC-relevant nodes and edges, then assert the response contains only permitted nodes and their corresponding edges before the existing limit-based pruning. Keep the current all-environments test unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/go/openapi/src/paths/graph.graphs.expand.yaml`:
- Around line 59-62: Require data in the 200 response schema by adding it to the
response object's required fields beside properties in
packages/go/openapi/src/paths/graph.graphs.expand.yaml:59-62. Regenerate
packages/go/openapi/doc/openapi.json:7653-7659 from the corrected YAML source;
do not edit the generated artifact directly.
- Around line 37-39: Update node_id in
packages/go/openapi/src/paths/graph.graphs.expand.yaml (lines 37-39) to use a
JavaScript-safe representation by constraining it to the safe integer range or
changing it to a decimal string, while preserving compatibility with the Go
int64 handler. Regenerate packages/go/openapi/doc/openapi.json (lines 7621-7624)
so the generated schema reflects the same representation.
---
Nitpick comments:
In `@cmd/api/src/api/v2/graphexpansion_test.go`:
- Around line 45-122: Add a separate ExpandGraph test case using a user without
AllEnvironments so ShouldFilterForETAC enters the filtering branch and
filterETACGraph is exercised. Configure the graph fixture with ETAC-relevant
nodes and edges, then assert the response contains only permitted nodes and
their corresponding edges before the existing limit-based pruning. Keep the
current all-environments test unchanged.
In `@cmd/api/src/api/v2/graphexpansion.go`:
- Around line 80-98: Update buildGraphExpansionQuery to group the initial
declarations of filteredRelationshipKinds, seenRelationshipKinds, and
relationshipMatch in a single var block near the top of the function, while
preserving their existing initialization values and subsequent behavior.
- Line 259: Update the RawCypherQuery call in the graph expansion handler to
request properties whenever payload.IncludeProperties or filterForETAC is true,
so filterETACGraph can evaluate node environment access; when properties were
requested only for ETAC filtering and payload.IncludeProperties is false, remove
them before returning the response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: fc183c0a-f6d2-4db7-8132-a08d953f835d
📒 Files selected for processing (12)
cmd/api/src/api/registration/v2.gocmd/api/src/api/v2/graphexpansion.gocmd/api/src/api/v2/graphexpansion_internal_test.gocmd/api/src/api/v2/graphexpansion_test.gopackages/go/openapi/doc/openapi.jsonpackages/go/openapi/src/openapi.yamlpackages/go/openapi/src/paths/graph.graphs.expand.yamlpackages/go/openapi/src/schemas/model.unified-graph.expansion.yamlpackages/javascript/js-client-library/src/client.tspackages/javascript/js-client-library/src/requests.tspackages/javascript/js-client-library/src/responses.tspackages/javascript/js-client-library/src/types.ts
Comment on lines
+37
to
+39
| node_id: | ||
| type: integer | ||
| format: int64 |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'export interface GraphExpansionRequest|node_id' \
packages/javascript/js-client-library/src/requests.ts
rg -n -C 6 '\bExpandGraph\b|node_id|int64|uint64|MaxSafeInteger|9007199254740991' \
cmd/api/src packages/go/openapi/srcRepository: SpecterOps/BloodHound
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate request definitions ---'
rg -n -C 5 'GraphExpansionRequest|node_id' packages/javascript packages/go/openapi/src/paths/graph.graphs.expand.yaml 2>/dev/null | head -n 240
printf '%s\n' '--- graph expansion implementation and models ---'
rg -n -C 6 'GraphExpansionPayload|NodeID|ExpandGraph|graphs/expand' cmd/api/src packages/go/openapi/src --glob '*.go' --glob '*.yaml' | head -n 320
printf '%s\n' '--- contract snippets ---'
sed -n '25,75p' packages/go/openapi/src/paths/graph.graphs.expand.yaml
sed -n '7610,7670p' packages/go/openapi/doc/openapi.jsonRepository: SpecterOps/BloodHound
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- expansion resource and payload declarations ---'
find cmd/api/src -type f -name '*.go' -print0 |
xargs -0 rg -l 'func .*ExpandGraph|type GraphExpansionPayload|GraphExpansionPayload' |
sort
printf '%s\n' '--- relevant declarations and handlers ---'
find cmd/api/src -type f -name '*.go' -print0 |
xargs -0 rg -n -C 12 'func .*ExpandGraph|type GraphExpansionPayload|NodeID.*int64|node_id is required' |
head -n 360
printf '%s\n' '--- graph ID type definitions ---'
find . -type f \( -name '*.go' -o -name '*.ts' \) -print0 |
xargs -0 rg -n -C 3 'type ID (uint|int)|type ID =' |
head -n 160Repository: SpecterOps/BloodHound
Length of output: 12374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const safe = 9007199254740991;
const firstUnsafe = 9007199254740993;
const parsed = JSON.parse(`{"node_id":${firstUnsafe}}`).node_id;
console.log(JSON.stringify({
maxSafeInteger: Number.MAX_SAFE_INTEGER,
firstUnsafe,
parsed,
rounded: parsed !== firstUnsafe,
adjacentIntegersEqual: Number(firstUnsafe) === Number(firstUnsafe + 1),
}));
JS
printf '%s\n' '--- storage and sequence declarations for node_id ---'
rg -n -C 4 'node_id_seq|node_id.*(bigserial|bigint)|bigserial.*node_id|CREATE TABLE.*nodes' . \
--glob '*.sql' --glob '*.go' --glob '*.yaml' --glob '*.yml' |
head -n 240
printf '%s\n' '--- source and generated node_id schemas ---'
python3 - <<'PY'
import json
from pathlib import Path
doc = json.loads(Path("packages/go/openapi/doc/openapi.json").read_text())
hits = []
def walk(value, path=()):
if isinstance(value, dict):
if "node_id" in value and isinstance(value["node_id"], dict):
hits.append((path + ("node_id",), value["node_id"]))
for key, child in value.items():
walk(child, path + (key,))
elif isinstance(value, list):
for i, child in enumerate(value):
walk(child, path + (str(i),))
walk(doc)
for path, schema in hits:
if schema.get("type") == "integer" and schema.get("format") == "int64":
print(".".join(path), schema)
PYRepository: SpecterOps/BloodHound
Length of output: 6029
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const jsonValue = "9007199254740993";
const parsed = JSON.parse(`{"node_id":${jsonValue}}`).node_id;
const exact = BigInt(jsonValue);
console.log(JSON.stringify({
maxSafeInteger: Number.MAX_SAFE_INTEGER,
wireValue: jsonValue,
parsedAsNumber: parsed,
parsedAsBigInt: BigInt(parsed),
losesPrecision: BigInt(parsed) !== exact,
adjacentNumbersEqual: Number("9007199254740992") === Number(jsonValue),
}));
JS
sed -n '42,52p' cmd/api/src/api/v2/graphexpansion.go
sed -n '34,40p' packages/javascript/js-client-library/src/requests.tsRepository: SpecterOps/BloodHound
Length of output: 769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const jsonValue = "9007199254740993";
const parsed = JSON.parse(`{"node_id":${jsonValue}}`).node_id;
const exact = BigInt(jsonValue);
console.log({
maxSafeInteger: Number.MAX_SAFE_INTEGER,
wireValue: jsonValue,
parsedAsNumber: String(parsed),
parsedAsBigInt: String(BigInt(parsed)),
losesPrecision: BigInt(parsed) !== exact,
adjacentNumbersEqual: Number("9007199254740992") === Number(jsonValue),
});
JS
sed -n '42,52p' cmd/api/src/api/v2/graphexpansion.go
sed -n '34,40p' packages/javascript/js-client-library/src/requests.tsRepository: SpecterOps/BloodHound
Length of output: 970
Use a JavaScript-safe representation for node_id. The Go handler accepts int64, while GraphExpansionRequest.node_id uses number; values above 9007199254740991 can be rounded and expand the wrong node. Constrain the OpenAPI schema to the safe range or use a decimal string, then regenerate packages/go/openapi/doc/openapi.json.
📍 Affects 2 files
packages/go/openapi/src/paths/graph.graphs.expand.yaml#L37-L39(this comment)packages/go/openapi/doc/openapi.json#L7621-L7624
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/go/openapi/src/paths/graph.graphs.expand.yaml` around lines 37 - 39,
Update node_id in packages/go/openapi/src/paths/graph.graphs.expand.yaml (lines
37-39) to use a JavaScript-safe representation by constraining it to the safe
integer range or changing it to a decimal string, while preserving compatibility
with the Go int64 handler. Regenerate packages/go/openapi/doc/openapi.json
(lines 7621-7624) so the generated schema reflects the same representation.
Comment on lines
+59
to
+62
| type: object | ||
| properties: | ||
| data: | ||
| $ref: './../schemas/model.unified-graph.expansion.yaml' |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require data in the 200 response.
The response schema permits {} because it does not list data as required. The required limit and truncated fields do not apply when data is absent. Require data at the response-object level.
packages/go/openapi/src/paths/graph.graphs.expand.yaml#L59-L62: addrequired: [data]besideproperties.packages/go/openapi/doc/openapi.json#L7653-L7659: regenerate this artifact from the corrected YAML.
Based on learnings, regenerate packages/go/openapi/doc/openapi.json from the YAML source rather than editing it directly.
Proposed fix
schema:
type: object
+ required:
+ - data
properties:
data:
$ref: './../schemas/model.unified-graph.expansion.yaml'📝 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.
Suggested change
| type: object | |
| properties: | |
| data: | |
| $ref: './../schemas/model.unified-graph.expansion.yaml' | |
| type: object | |
| required: | |
| - data | |
| properties: | |
| data: | |
| $ref: './../schemas/model.unified-graph.expansion.yaml' |
📍 Affects 2 files
packages/go/openapi/src/paths/graph.graphs.expand.yaml#L59-L62(this comment)packages/go/openapi/doc/openapi.json#L7653-L7659
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/go/openapi/src/paths/graph.graphs.expand.yaml` around lines 59 - 62,
Require data in the 200 response schema by adding it to the response object's
required fields beside properties in
packages/go/openapi/src/paths/graph.graphs.expand.yaml:59-62. Regenerate
packages/go/openapi/doc/openapi.json:7653-7659 from the corrected YAML source;
do not edit the generated artifact directly.
Source: Learnings
justin-prime1
force-pushed
the
bp-2366-bounded-graph-expansion-api
branch
from
August 3, 2026 18:19
af47d75 to
9fb0070
Compare
coderabbitai
Bot
added
the
dependencies
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Motivation
This supports the BHE Explore directional expansion work while also giving API consumers a bounded one-hop expansion endpoint without relying on UI-generated Cypher.
Testing
Companion
Resolves: BP-2366
Summary by CodeRabbit
New Features
Documentation
Tests