-
-
Notifications
You must be signed in to change notification settings - Fork 17.6k
bloodhound-ce: init at 8.3.1 #461387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bloodhound-ce: init at 8.3.1 #461387
Conversation
|
|
Ran into #415328 while testing a Darwin build with full sandboxing. Trying with relaxed sandboxing now; if this still doesn't work, I'm going to mark this package as Linux-exclusive. |
|
36a6c2a to
1f348d8
Compare
| # cmd/ui/package.json includes "git@github.com:BloodHoundAD/dagre.git" | ||
| # which is fetched using SSH, and that doesn't work in the Nix sandbox | ||
| # so we just replace it with a standard HTTPS URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # cmd/ui/package.json includes "git@github.com:BloodHoundAD/dagre.git" | |
| # which is fetched using SSH, and that doesn't work in the Nix sandbox | |
| # so we just replace it with a standard HTTPS URL | |
| # cmd/ui/package.json includes "git@github.com:BloodHoundAD/dagre.git" | |
| # which requires credential |
Even if we would provide ssh, then we would still need to authenticate.
Please try to convince upstream to switch to https git cloning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if we would provide ssh, then we would still need to authenticate.
That's what I meant with "doesn't work", but yeah I could've worded that better.
Please try to convince upstream to switch to https git cloning.
See SpecterOps/BloodHound#2096 & SpecterOps/BloodHound#2097.
Can we still merge this PR in its current state? If upstream even accepts a change like this (which imo is only really relevant for Nix and thus fits https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#vendoring-patches), it'll still take a while for a new stable release to include the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can merge it without this but this is not only applicable to nix. Any CI like process would require credentials to clone the repo which is a big anti pattern.
c3c1260 to
0dfed49
Compare
This PR adds BloodHound Community Edition, the successor to the legacy version currently packaged in nixpkgs.
As for deciding how to package this, I have tried to mostly mirror the upstream Dockerfile, including the way the data collectors/ingestors are bundled as well as how sources are filtered in the front- and backend builds.
Since
lib.filesetsadly only works with local paths and I couldn't for the life for me figure out how to do this cleanly withlib.sources, I've opted to write my own little filter function. Hope that's okay.When this gets merged, I'm planning to open some followup PRs for Neo4j 4.4 (since BloodHound-CE fails with 5.x) and RustHound-CE, a third-party Active Directory ingestor written in Rust.
If I've made some glaring mistakes in this PR, feel free to scream at me.
This is my first time contributing to nixpkgs, so I'm open for any criticism!
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.