There is no point in sending to a minion the same file over and over.
The overlord can simply remember some facts about a file (say, metadata-backed hashes) and decide not to send over to a minion the file for analysis, simply returning the data.

Design notes:

• What is the minimum DNA required here? hashing will take care of "data" (file contents) but clearly metadata might play a role in the findings (e.g. executable bit, file position in the disk etc).
• Metadata-only files might still benefit from caching (e.g. malware-scanner minions looking for specific hashes).
• Down the line, we might want to have minions specify their own "caching rules" telling the overlord what they can cache.

This is just some comments from an initial general review

Package structure

Packages should be named after their functions but the directory tree should reflect dependencies. The overlord/interests package is used by packages outside overlord. Since it is a shared util it might be worth considering moving it one level up to highlight what is shared across packages.

Overlord

Naming convention

File: ./overlord/overlord_test.go: underscores in names are discouraged.

Implement a deadline

Line: ./overlord/overlord.go:116: // TODO(paradoxengine): most likely, a deadline here?

A dummy code (untested) to do it

	var interests []*mappedInterest
	for name, m := range minions {
		// TODO(paradoxengine): most likely, a deadline here?
		var intResp *pb.ListInitialInterestsResponse
		var err error
		done := make(chan struct{})
		go func(){
			intResp, err = m.ListInitialInterests(ctx, &mpb.ListInitialInterestsRequest{})
			close(done)
		}()
		select {
			case <- time.After(5*time.Second):
				// handle timeout
				continue
			case <- done:
		}
		if err != nil {
			return nil, err
		}
		for _, v := range intResp.GetInterests() {
			interests = append(interests, &mappedInterest{
				interest: v,
				minion:   name,
			})
		}
	}
	return interests, nil

copy is a shallow copy

Line: ./overlord/overlord.go:143: copy(scanState.interests, s.initialInterests)

Beware this is a shallow copy and might behave weirdly if the copied slice elements are then modified

Potential race condition

grpc.Serve might spawn several goroutines at the same time, so it might be worth to protect some calls with a mutex (add a --gotsan or --race flag during tests to enable the race detector)

For example the following field is both written and read after server initialization:
Line: ./overlord/overlord.go:34: scans map[string]state

Reads:
overlord/overlord.go:161: scan, ok := s.scans[req.GetScanId()]
overlord/overlord.go:176: scan, ok := s.scans[req.GetScanId()]

Write:
overlord/overlord.go:150: s.scans[scan.ScanId] = scanState

Native go maps are not concurrent safe, maybe protect it with a sync.RWMutex or use a sync.Map instead.

Nits:

Line:./overlord/overlord_test.go:90:// TOOD(paradoxengine): the overlord still needs plenty of unit tests typo on TODO.
Line:./overlord/overlord_test.go:90:// TOOD(paradoxengine): the overlord still needs plenty of unit tests maybe take a look at the cover tool.

Hey-ho!
We've faced some kind of issues with projects using our API.
Like trying to make a 1000rps to our backend using our official Python API :)
So I've made auto-throttling mechanics using two HTTP headers at the backend and some kind of semaphore in the Python API acting on them.
In response from the backend you can see:
X-Vulners-Ratelimit-Reqlimit = This headers tells client about ratelimit. Float.
X-Vulners-Ratelimit-Rate = This headers tells client his current rate. Float.
So avoiding banhammer is quite easy - just respect this values and throttle down if the end is near :)

I've added throttling functionality in this commit.
May I suggest you to follow it in Go lib? It's not urgent, but a definitely good stuff :)

Extracting installed npms should be relatively simple, and then they can be checked for vulnerabilities.
package.json is the obvious starting point,

npm list -g --depth=0 is the canonical way to get the view from a command line, but also subdirs of node_modules should work.

We'd have to figure out how to identify vulnerable ones, but there is a fairly large ecosystem for that as well.

Seems like a configuration scanner would be a good idea, and redis might be a good start.
We could check for:

• world readable config file (since it stores the password)
• requirepass (maybe in conjunction with being bound to something other than localhost)
• presence of rename command for CONFIG (though, this seems like hardening?)

Keyless requests end up in the anonymous bucket which has all sort of rate limiting. Keep supporting that but also add support for a key, adding apiKey to the json requests.

Turns out that Berkeley DB (which is what the RPM db is stored in) from Go is surprisingly hard.
It seems reasonable in Java instead (last famous words).
It's a good excuse to showcase using different languages for minions that interop with each other, so it might be worth building a Java minion to support that, though it means re-implementing the vulners apis etc, but since I've done it already it should be reasonably easy.

Elastic comes with quite a few security-related settings which seem easy enough to check for

https://www.elastic.co/guide/en/elasticsearch/reference/current/security-settings.html

Apparently you can't be a respectable project without at least the following:

Roadmap.md
Code-of-conduct.md
bill-of-materials.json
lgtm config
travis config

At a minimum:

• ASCII art with architecture
• Goals and non goals
• Why this matters and use cases
• How can you help

Sending binaries to virus total seems like a reasonable thing to do.

The public API is documented here: https://www.virustotal.com/es/documentation/public-api/
The first scan is bound to take a long while as the minion builds a cache of "harmless files" with all the binaries found in a healthy OS, but after that it should be reasonably fast.
I wonder if there is a way to optimize this entirely by having some form of pre-check on the hashes of well known good binaries - it seems a trivial enough idea that I'm sure it must exist already - otherwise, maybe it would be an interesting project to just download all the rpms/apts and generate hashes of those binaries?

Of course, this plays very nicely with the idea of controlling the scope of goblins re the binaries they serve.

Jenkins pipelines seem to be a good first use case for an actually useful Goblin.
It could be used to scan a Docker image for example

In modern ubuntu systems it's fairly common to have multiple os-release files in snaps.
These can be selected by goblins, so either we need to lock down the regexp or we need to handle multiple submissions of os-release (possibly by making multiple calls with the various values, if they disagree)

There are a number of things that can be checked for at the password level.
Complexity, for sure, but also whether there is a password, what's the hashing, presence of salting etc.

An endless classic, PHP has all sort of potential configuration woes

Allow remote opens, globals (though this has luckily gone away) and an array of other things - we can probably check a hardening guide to make sure we're not missing anything major.

It's important to keep in mind we should only flag things that have a reasonable chance of being a real problem, not just far-fetched hardening improvements.

Since minions wants to be used over a network, SSL authentication (and encryption, of course) is very much required.

Issue

When running bazel test with the --batch flag, bazel outputs deprecation warning messages. These warning messages can be seen here, in a travis build of master

Following what is stated on the Bazel User Manual, shutdown should be used to explicitly shut down the Bazel server.

I have tried to build an overlord:
$ bazel build //go/overlord/runner
then, I got the ERROR:

ERROR: /home/user/goworkspace/src/github.com/google/minions/src/proto/overlord/BUILD.bazel:4:1: every rule of type proto_library implicitly depends upon the target '@com_google_protobuf//:protoc', but this target could not be found because of: no such package '@com_google_protobuf//': java.io.IOException: thread interrupted
ERROR: Analysis of target '//go/overlord/runner:runner' failed; build aborted: no such package '@com_google_protobuf//': java.io.IOException: thread interrupted
INFO: Elapsed time: 677.133s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (42 packages loaded)

The minimum usable system goes through a goblin capable of handling container images.
I might want to start from raw dockers or something integrated, like spinnaker. TBD.

Goblins are currently pretty dumb when it comes to symlinks, and we need a good strategy to handle that.
In particular I'm thinking about the "fake" chroot that using the "rootdir" directive provides, which is trivially broken by symlinks.

There is little point in not doing this since day zero, so we might as well migrate to LogRus while we're at it.
https://github.com/sirupsen/logrus

I've looked briefly at other structured logging libraries (it's a bit sad there is no clear standard in Go yet) and this seems the best option

The local goblin currently searches the entire hard drive, but really it should look at prefixes of the regexp for the interests and bail out if the prefix (e.g. the root directory) does not match.

In addition it should only parse the filesystem once, looking at all the interests it knows about at a given point - after all, the overlord will take care of routing the files.