Add datadog certifier #2366
Add datadog certifier #2366
Conversation
|
As a general comment, I wonder if we want to call it something more specific than "DataDog"? "DataDog Malicious Packages DataSet" is unwieldy, but I'm concerned that there might be some future thing that pulls from DataDog proper and the name is already taken. I don't have any great ideas and this may not be a concern worth worrying about right now, but I wanted to raise it. |
yeah, that is a solid point, if DataDog eventually spin out other datasets, I can see how that might cause some confusion. The data itself mostly comes from GuardDog but I think not exclusively. Maybe we can go with something like |
robert-cronin
force-pushed
the
feat/datadog-certifier
branch
from
cd71306 to
6be472b
Compare
Signed-off-by: robert-cronin <robert.owen.cronin@gmail.com>
robert-cronin
force-pushed
the
feat/datadog-certifier
branch
from
6be472b to
d2f86e2
Compare
|
Thanks @robert-cronin! Sorry for the delay. We will review this soon! |
No problems, thanks @pxp928! |
There was a problem hiding this comment.
This is a super cool addition. I wasn't aware of this dataset but this was a really cool implementation and its such a good example on how to add another data source easily (or at least you made it look easy! - any feedback on how to make this easier would be super great as well, or any particular frictions you had). Thanks so much for yet another great contribution! 🙌
| opt(d) | ||
| } | ||
|
|
||
| if err := d.fetchManifests(); err != nil { |
There was a problem hiding this comment.
it looks like manifests are fetched once on initialization. Given the database will be updated regularly - it would be helpful to refresh the manifests based on some frequency. Is this feasible?
| if pkgInput.Namespace != nil && *pkgInput.Namespace != "" { | ||
| namespace := strings.TrimPrefix(*pkgInput.Namespace, "@") | ||
| namespace = strings.TrimPrefix(namespace, "%40") | ||
| fullName = "@" + namespace + "/" + pkgInput.Name |
There was a problem hiding this comment.
it looks like it isn't always the case that the packages start with "@" in the dataset, could we add a check here after the trim to see if the namespace had the prefix? and add the "@" only if there was a prefix trim?
| } | ||
|
|
||
| // NewDatadogMalwareCertifier initializes the Datadog Malicious Software Packages certifier | ||
| func NewDatadogMalwareCertifier(ctx context.Context, assemblerFunc assemblerFuncType, opts ...CertifierOption) (certifier.Certifier, error) { |
There was a problem hiding this comment.
Can you add a bit of documentation here on what the datadog malicious software packages are, for those that are not familiar.
In addition could you add some details on:
- The added predicates on the graph
- The recommended interval times (considering that the current certifier will generate a certifyBad each time indefinitely).
- Any caveats: see comment on periodic fetching of manifest.
Description of the PR
Fixes #2345
I am not sure if there is a need for a parser or attestation since were just ingesting CertifyBad for a particular pURL, but if there is a need to represent the source information in a predicate, I'd be happy to try and figure out how to add that in.
PR Checklist
-sflag togit commit.make generatehas been runmake generatehas been runmake generatehas been runcollectsubprotobuf has been changed,make protohas been run