feat(bloodhound): add Active Directory attack-path injector (#316) by SamuelHassine · Pull Request #326 · OpenAEV-Platform/injectors

SamuelHassine · 2026-07-14T09:34:07Z

Summary

Add the Active Directory attack-path injector wrapping BloodHound.py (SharpHound-compatible), surfacing users, computers and privilege-escalation paths (Kerberoastable, AS-REP roastable) as OpenAEV findings.
Wire CI: add bloodhound to the CircleCI build_docker_images, publish_images and test matrices.

Notes

Per-inject AD credentials; the password argument is redacted by position whenever the collector command is logged.
The collector fails loudly: a non-zero bloodhound-python exit returns an ERROR result with a trimmed, password-scrubbed stderr/stdout snippet instead of a false SUCCESS with empty outputs. The generic handler also logs the exception via injector_logger before the ERROR callback.
Missing icon asset raises a clear, actionable error at startup naming the expected path and the icon standard (Connector icons should be square with a solid background (netexec, teams, ...) #305) rather than an opaque FileNotFoundError. The icon path is read from the configurable injector_icon_filepath (falling back to the default), and that same resolved path is used in the error message.
Parsing is deterministic and de-duplicated: _names and _privileged iterate over a sorted glob, de-duplicate via a set, and share a single name -> samaccountname fallback. Parsing also tolerates a present-but-null Properties object without crashing.
The collector runs bloodhound-python -c All (without --zip) so the loose per-object JSON files stay on disk for parse_collection to glob; with --zip the tool compresses then deletes them, which would make every run report zero users/computers.
VULNERABILITY and DETECTION expectations. Domain: Endpoint (identity).
README documents the OpenAEV and base injector environment variables (Configuration variables section), consistent with the other injectors.

Known follow-up (non-CI blocker)

The genuine 512x512 opaque BloodHound icon PNG is intentionally not committed yet (placeholder tracked by Connector icons should be square with a solid background (netexec, teams, ...) #305). It must be dropped into bloodhound_injector/img/icon-bloodhound.png (or the path set via injector_icon_filepath) before the injector can start in a real deployment; until then startup raises the actionable error above. CI is unaffected because the tests mock the icon read.

Test plan

isort (--profile black) / black / flake8 clean
python -m unittest / pytest (contracts + collection parser + injector lifecycle, incl. non-zero exit path, de-dup/fallback parsing, and null-Properties tolerance) - 12 tests
docker build via the CircleCI build_docker_images-bloodhound matrix

Closes #316

Runs the BloodHound.py AD collector and surfaces users, computers and privilege-escalation attack paths as findings. Wires the injector into the CircleCI build, publish and test matrices.

Copilot

Pull request overview

This PR introduces a new bloodhound/ injector module that wraps BloodHound.py to collect Active Directory data and emit OpenAEV findings (users, computers, and privilege-escalation “attack path” indicators), and wires the new injector into CircleCI build/test/publish matrices.

Changes:

Add the BloodHound injector implementation (contracts, config loader, executor, entrypoint) plus unit tests for contract shape and JSON parsing.
Add containerization + runtime scaffolding for the new injector (Dockerfile, compose, samples, manifest metadata, docs).
Extend CircleCI matrices to include bloodhound for tests and image build/publish.

Reviewed changes

Copilot reviewed 17 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
bloodhound/test/test_bloodhound.py	Adds unit tests for contract outputs and collection JSON parsing.
bloodhound/test/init.py	Test package marker.
bloodhound/README.md	Documents injector purpose, contract, dev workflow, and icon requirements.
bloodhound/pyproject.toml	Defines the new injector package metadata and dependencies.
bloodhound/manifest-metadata.json	Adds catalog/manager metadata for the new injector image.
bloodhound/Dockerfile	Builds and runs the injector container image.
bloodhound/docker-compose.yml	Provides a local compose service definition.
bloodhound/config.yml.sample	Adds sample runtime configuration.
bloodhound/bloodhound_injector/openaev_bloodhound.py	Implements the injector entrypoint and execution callback handling.
bloodhound/bloodhound_injector/img/README.md	Documents required icon asset placement and standards.
bloodhound/bloodhound_injector/helpers/bloodhound_executor.py	Runs bloodhound-python and parses emitted JSON into outputs.
bloodhound/bloodhound_injector/helpers/init.py	Helpers package marker.
bloodhound/bloodhound_injector/contracts_bloodhound.py	Defines the injector contract inputs/outputs and expectations.
bloodhound/bloodhound_injector/configuration/injector_config_override.py	Declares injector config overrides (id/name/icon path).
bloodhound/bloodhound_injector/configuration/config_loader.py	Wires config hints and contracts into the OpenAEV configuration object.
bloodhound/bloodhound_injector/configuration/init.py	Configuration package marker.
bloodhound/bloodhound_injector/init.py	Declares module version metadata.
bloodhound/.env.sample	Adds sample environment variables.
bloodhound/.dockerignore	Excludes config and caches from Docker build context.
.circleci/config.yml	Adds `bloodhound` to the test/build/publish matrices.

Adds coverage for __init__, process_message, start and the BloodHound collection runner to satisfy the codecov patch gate.

#316) Report a failure when bloodhound-python exits non-zero instead of returning success=True with empty outputs, including a redacted stderr/stdout snippet for troubleshooting. Redact the password argument by position when logging the command. Raise an actionable error when the injector icon asset is missing so the startup failure is self-explanatory. Add a test for the non-zero exit path asserting the password never leaks into the failure message.

SamuelHassine · 2026-07-14T10:47:15Z

Independent review + fix pass complete. Read the full changed files (not just the diff) and addressed all findings in 911facd:

issue (blocking) - correctness: run_collection ignored the collector exit status and returned SUCCESS with empty outputs on failure (bad creds, network error). Now returns an ERROR result on a non-zero exit with a trimmed, password-scrubbed stderr/stdout snippet.
issue (blocking) - security/robustness: password redaction in command logging was value-based and could mask the wrong argument. Now positional and explicit via a small _redact helper.
issue (blocking) - startup crash: the unconditional icon read raised an opaque FileNotFoundError. Now raises a clear, actionable error naming the expected path and the icon standard (Connector icons should be square with a solid background (netexec, teams, ...) #305).
test: added test_run_collection_nonzero_exit_reports_error, which also asserts the password never leaks into the failure message.

Verification: local isort (--profile black) / black / flake8 clean; full unit-test suite passes (10 tests); all CI checks green (GitHub Actions test matrix, CircleCI test + build_docker_images, codecov, signed-commits, linked-issue, PR-title).

Resolved all 3 review threads (Copilot).

Non-CI follow-up: the genuine 512x512 opaque BloodHound icon PNG still needs to be committed at bloodhound_injector/img/icon-bloodhound.png before the injector can start in a real deployment (tracked by #305). CI is unaffected because the tests mock the icon read.

Note: I could not submit a formal Approve review - GitHub blocks approving your own pull request, and the PR author is the reviewing account. A separate reviewer approval is still required to clear REVIEW_REQUIRED / unblock merge.

Copilot

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 4 comments.

- Dockerfile: use a POSIX `[ -n "..." ]` test for the PYOAEV_GIT_BRANCH_OVERRIDE block so it works under Alpine /bin/sh (the Bash-only `[[ ... ]]` never evaluated true). - Dockerfile: copy /usr/local/bin from the builder into the runner so the `bloodhound-python` console script is actually present at runtime; copying site-packages alone left it behind and every collection would have failed with "bloodhound-python not found in the image". - openaev_bloodhound: read the icon from the configurable `injector_icon_filepath` (fallback to the default path) and use that same resolved path in the missing-icon error message. - bloodhound_executor: de-duplicate parsed names with a set and sort the glob for deterministic output; make `_privileged` use the same name/samaccountname fallback as `_names`, sort the glob, and de-duplicate findings. - test: cover de-duplication across multiple *_users.json files and the samaccountname fallback.

SamuelHassine · 2026-07-14T11:31:06Z

Second review-and-fix pass complete (commit 653f22d).

New review comments addressed (all 4 threads resolved):

Dockerfile: replaced the Bash-only [[ ... ]] test with POSIX [ -n "${PYOAEV_GIT_BRANCH_OVERRIDE}" ] so the client-python override actually runs under Alpine /bin/sh.
openaev_bloodhound: icon is now read from the configurable injector_icon_filepath (fallback to the default), and the missing-icon error message reports that same resolved path.
bloodhound_executor _names(): de-duplicate via a seen set and sort the glob for deterministic output.
bloodhound_executor _privileged(): shared name -> samaccountname -> unknown fallback, sorted glob, and de-duplicated findings across multiple *_users.json files.

Additional finding from the independent re-review (also fixed in 653f22d):

The runner image copied only site-packages, so the bloodhound-python console script was missing and every collection would have failed with "bloodhound-python not found in the image". Added COPY --from=builder /usr/local/bin /usr/local/bin.

Verification:

Local isort (--profile black, repo-root config) / black / flake8 all clean.
Full unit suite passes (11 tests), including the new de-dup / samaccountname-fallback parsing test.
All CI checks green: GitHub Actions test matrix, CircleCI test + build_docker_images (incl. build_docker_images-bloodhound and test-bloodhound), codecov, signed-commits, linked-issue, PR-title. CLA not required (organization member).

Non-CI follow-up (unchanged): the genuine 512x512 opaque BloodHound icon PNG still needs to be committed at bloodhound_injector/img/icon-bloodhound.png (or the path set via injector_icon_filepath) before the injector can start in a real deployment (tracked by #305). CI is unaffected because the tests mock the icon read.

Note: I could not submit a formal Approve review - GitHub blocks approving your own pull request and the reviewing account is the PR author. A second maintainer approval is still required to clear REVIEW_REQUIRED and unblock merge.

Copilot

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 1 comment.

Harden the collector parser against a present-but-null "Properties" object in the BloodHound JSON (valid JSON that previously crashed parse_collection with an AttributeError) by treating it as an empty mapping in both _names() and _privileged(). Extract a shared _name_or_none() helper so the name -> samaccountname fallback ordering lives in one place, keeping the deliberate difference that enumeration lists skip nameless entries while privileged findings label them "unknown". Add a regression test for the null-Properties case.

SamuelHassine · 2026-07-15T16:05:45Z

Review-and-fix pass complete (commit 3427c95).

Independent senior review of the full changed files (not just the diff), plus all prior reviewer threads addressed. LGTM.

New finding fixed this pass:

Parsing crash on null Properties: _names() and _privileged() did entry.get("Properties", {}), which returns None (not {}) when a JSON entry has "Properties": null, then raised AttributeError in parse_collection. Both now use entry.get("Properties") or {}. Also extracted a shared _name_or_none() helper so the name -> samaccountname fallback ordering lives in one place (_entry_name() builds on it), resolving the last open Copilot thread. _names() still intentionally skips nameless entries (no synthetic "unknown" in the users/computers enumeration findings), while _privileged() keeps the "unknown" fallback so a Kerberoastable / AS-REP-roastable account is always flagged.

Verification:

Local isort (--profile black) / black / flake8 clean.
Full unit suite passes (12 tests), including the new test_parse_collection_tolerates_null_properties.
All CI checks green: GitHub Actions test matrix (incl. Test bloodhound), CircleCI test + build_docker_images (incl. test-bloodhound and build_docker_images-bloodhound), codecov, check-signed-commits, linked-issue, PR-title. CLA not required (organization member).

All 8 review threads are resolved. PR is MERGEABLE; mergeStateStatus is BLOCKED only on REVIEW_REQUIRED.

Note: I could not submit a formal Approve review - GitHub blocks approving your own pull request and the reviewing account is the PR author. Consider this comment an explicit LGTM; a second maintainer approval is still required to clear REVIEW_REQUIRED and unblock merge.

Non-CI follow-up (unchanged): the genuine 512x512 opaque BloodHound icon PNG still needs to be committed at bloodhound_injector/img/icon-bloodhound.png (or the path set via injector_icon_filepath) before the injector can start in a real deployment (tracked by #305). CI is unaffected because the tests mock the icon read.

Copilot

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 2 comments.

…#316) - Drop the `--zip` flag from the bloodhound-python command. With `--zip` the collector compresses the per-object JSON files into an archive and then deletes the loose files, so parse_collection (which globs the loose *_users.json / *_computers.json in the workdir) always found zero objects and every successful run reported "Collected 0 users and 0 computers". Without `--zip` the loose JSON stays on disk for parsing. - Log the exception via injector_logger before sending the ERROR callback in process_message, matching the injector implementation guide and easing operational troubleshooting. - Document the OpenAEV and base injector environment variables in the README (Configuration variables section), consistent with the other injectors.

SamuelHassine · 2026-07-17T11:33:03Z

Review-and-fix pass complete (commit bd9938c). Independent senior review of the full changed files (not just the diff), plus both remaining Copilot threads addressed and resolved. LGTM.

New findings fixed this pass:

issue (blocking) - correctness: the collector ran bloodhound-python ... --zip, which compresses the per-object JSON files into an archive and then deletes the loose files. parse_collection globs the loose *_users.json / *_computers.json in the workdir, so every successful run would have parsed zero objects and reported "Collected 0 users and 0 computers" with no findings. Dropped --zip so the loose JSON stays on disk for parsing.
issue (should fix): the generic exception handler in process_message reported an ERROR callback but never logged the exception. It now logs via injector_logger.error(...) before the callback, matching the injector implementation guide (Copilot thread).
issue (should fix): the README did not document the required OpenAEV / base injector environment variables. Added a "Configuration variables" section in the same table format as the other injectors (Copilot thread).

Verification:

Local isort (--profile black) / black / flake8 clean.
Full unit suite passes (12 tests).
All CI checks green: GitHub Actions test matrix (incl. Test bloodhound), CircleCI test + build_docker_images (incl. test-bloodhound and build_docker_images-bloodhound), codecov (patch + project), check-signed-commits, linked-issue, PR-title. CLA not required (organization member).

All review threads are resolved. PR is MERGEABLE; mergeStateStatus is BLOCKED only on REVIEW_REQUIRED.

Note: I could not submit a formal Approve review - GitHub blocks approving your own pull request and the reviewing account is the PR author. Consider this comment an explicit LGTM; a second maintainer approval is still required to clear REVIEW_REQUIRED and unblock merge.

Non-CI follow-up (unchanged): the genuine 512x512 opaque BloodHound icon PNG still needs to be committed at bloodhound_injector/img/icon-bloodhound.png (or the path set via injector_icon_filepath) before the injector can start in a real deployment (tracked by #305). CI is unaffected because the tests mock the icon read.

Copilot

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 2 comments.

+from bloodhound_injector.configuration.config_loader import ConfigLoader
+from bloodhound_injector.helpers.bloodhound_executor import BloodhoundExecutor
+from pyoaev.helpers import OpenAEVConfigHelper, OpenAEVInjectorHelper
+
+from injector_common.data_helpers import DataHelpers
+from injector_common.dump_config import intercept_dump_argument


+from bloodhound_injector.configuration.injector_config_override import (
+    InjectorConfigOverride,
+)
+from bloodhound_injector.contracts_bloodhound import BloodhoundContracts
+from pydantic import Field
+from pyoaev.configuration import ConfigLoaderOAEV, Configuration, SettingsLoader


feat(bloodhound): add Active Directory attack-path injector (#316)

4c004b1

Runs the BloodHound.py AD collector and surfaces users, computers and privilege-escalation attack paths as findings. Wires the injector into the CircleCI build, publish and test matrices.

Copilot AI review requested due to automatic review settings July 14, 2026 09:34

Copilot started reviewing on behalf of SamuelHassine July 14, 2026 09:34 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread bloodhound/bloodhound_injector/helpers/bloodhound_executor.py

Comment thread bloodhound/bloodhound_injector/helpers/bloodhound_executor.py Outdated

Comment thread bloodhound/bloodhound_injector/openaev_bloodhound.py Outdated

SamuelHassine added 3 commits July 14, 2026 11:41

style(bloodhound): sort imports for repo-root isort check (#316)

7900442

test(bloodhound): cover injector lifecycle and collector (#316)

752b9a4

Adds coverage for __init__, process_message, start and the BloodHound collection runner to satisfy the codecov patch gate.

SamuelHassine requested a review from Copilot July 14, 2026 10:57

Copilot started reviewing on behalf of SamuelHassine July 14, 2026 10:58 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread bloodhound/Dockerfile Outdated

Comment thread bloodhound/bloodhound_injector/openaev_bloodhound.py

Comment thread bloodhound/bloodhound_injector/helpers/bloodhound_executor.py

Comment thread bloodhound/bloodhound_injector/helpers/bloodhound_executor.py

SamuelHassine requested a review from Copilot July 15, 2026 15:04

Copilot started reviewing on behalf of SamuelHassine July 15, 2026 15:05 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread bloodhound/bloodhound_injector/helpers/bloodhound_executor.py Outdated

SamuelHassine requested a review from Copilot July 15, 2026 20:59

Copilot started reviewing on behalf of SamuelHassine July 15, 2026 20:59 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread bloodhound/bloodhound_injector/openaev_bloodhound.py

Comment thread bloodhound/README.md

SamuelHassine requested a review from Copilot July 18, 2026 04:20

Copilot started reviewing on behalf of SamuelHassine July 18, 2026 04:20 View session

Copilot AI reviewed Jul 18, 2026

View reviewed changes

Uh oh!

Conversation

SamuelHassine commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Known follow-up (non-CI blocker)

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SamuelHassine commented Jul 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SamuelHassine commented Jul 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

SamuelHassine commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

SamuelHassine commented Jul 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SamuelHassine commented Jul 14, 2026 •

edited

Loading