Repository navigation
Feature: Pre-populate the file system of containers to support SAST scanners #707
Description
➹ New Feature implementation request
Pulling out the "pre-populate file system" requirement out of the semgrep issue (#595) so it can be discussed separately, as I would consider this a feature for SecureCodeBox on its own.
Is your feature request related to a problem?
To recap the discussion from #595: As a scanner developer, I want to pre-populate the file system of a scanner job so that I can run static analysis scanners on the resulting data. In #595, we are mostly talking about cloning a Git repository to run a SAST scanner on it, but the same feature can also be useful in other contexts, for example, downloading up-to-date word lists for ncrack from the Internet. Thus, this should be a modular setup that can be extended.
Describe the solution you'd like
After discussions with the team, we propose to use init containers internally for this purpose. The syntax proposed in #595 is a good start, however, I would propose a small alteration: make the data sources a list so that multiple downloaders can be chained / combined (i.e., download a git repository for scanning, and another Git repository containing a set of custom semgrep rules that are used by the scanner). So, the proposed syntax would look like this:
apiVersion: "execution.securecodebox.io/v1"
kind: Scan
metadata:
name: "semgrep-juiceshop"
spec:
scanType: "semgrep"
parameters:
- "--config"
- "p/ci"
- "/etc/repo"
prepopulateFilesystemFrom:
- git:
mountPoint: /etc/repo
repoUrl: "https://github.com/bkimminich/juice-shop.git"
secretRef:
name: https-credentials
- git:
mountPoint: /etc/rules
repoUrl: "https://github.com/my-organization/semgrep-rules.git"
secretRef:
name: https-credentials(The exact arguments are up for debate and would have to be decided for each downloader separately, it's more of a general layout).
Describe alternatives you've considered
Extending each SAST scanner with their own functionality to download repositories, rules and other things from the Internet. Would result in redundant code.
Additional Considerations
We would also need to figure out what downloaders we would want to support. So far, I have come up with the following list:
- VCS (Git, SVN, ...)
- HTTP(S) (curl, curl+unzip, curl+untar, ...)
- ... Your ideas? Possibilities may include S3, and it could also be an alternative way for mounting content from k8s secrets as in the ncrack scanner documentation if that is desired.
Looking for feedback on this proposal, especially on how the YAML syntax should be structured to be both intuitive and flexible enough to support different authentication schemes for Git etc.
Activity
malexmave commentedon Oct 6, 2021
After a bit of discussion with @J12934 about possible edge cases and how conflicts should be handled, we have arrived at the following updated proposal for the YAML:
(config key names subject to change)
The idea is that we want to declare for each prepopulate-Job in which folder it will be operating (this is the folder that will be mounted as a k8s volume). Any jobs that declare the same mountPoint will see the same content (i.e., if I run a git job on /etc/repo and then follow that with a HTTP job on /etc/repo, the HTTP job will see the results of the Git job and add its own changes on top). This allows additive changes to the same directory structure.
Advantages of this approach: You can first Git clone a repository and then HTTP fetch additional resources into the repository if this is necessary. It is also intuitive that multiple changes to the same directory are additive. Finally, it avoids potential clashes with attempting to mount multiple k8s volumes into the same location in the FS.
Disadvantages: It is a bit of work for deduplicating the paths to create the shared volumes (but that should be rather straightforward).
More importantly, we will have to figure out if and how we want to handle a situation where one job operates on /etc/repo, and another declares a mountPoint of /etc/repo/some/subdirectory.
At the moment I am tending towards the prefix matching option because it feels like the most intuitive solution to me, even though it may be a bit of additional work during the implementation. Opinions are very much welcome, on this and on the rest of the design (e.g., the approach of pulling out the declaration of the base folder onto a higher hierarchy level in the yaml file).
malexmave commentedon Oct 7, 2021
After further discussions with @J12934, we see two alternatives on how to implement this.
Modular with CRDs
One option is to go the fully modular route and design the downloaders similarly to the scanners: Have a common infrastructure that is managing them, and then have each of them as a standalone module that can be separately managed and installed. This would require setting up a new class of CRDs similar to ScanType, and implement support for that inside the controller.
A potential config with this solution may look like this:
The mountPoint would be managed by the controller (for creation and deduplication of volumes, as discussed in the previous comment), while the rest would be passed to the container itself.
Advantages:
Disadvantages:
@J12934 made a counterproposal, which I will describe below.
Semi-static with options for power users
This option would stick closer to the syntax of the previous comments, with downloader-specific parameters (repoUrl, accessToken, ... for Git, url and headers for curl, ...). This requires writing specific code for each downloader in the controller, and makes it harder to extend the feature with new downloaders. However, we could also have a "power user" option of providing a path to a docker image of your choice, which you can freely parameterize. The volumes would still be managed by the controller (no good way around that).
A potential configuration file may look like this:
Advantages:
Disadvantages:
Conclusion
@J12934 tends towards option two, and while I began as a proponent of option 1, I am slowly being convinced as well. However, @rseedorff was arguing for option 1 earlier, so we wanted to document our thoughts here and get an opinion from the rest of the team before proceeding with the implementation. Any opinions (and discussions of further drawbacks / advantages) are welcome. We will also take this to the next sprint meeting.
rfelber commentedon Oct 7, 2021
Hi @malexmave,
thx for your proposal and detailed discussion 🎉! It would be great if you prepare a new ADR (Architectural Decision Record) in our already existing list: https://github.com/secureCodeBox/secureCodeBox/tree/main/docs/adr
There are already some good ADR examples, like this one: https://github.com/secureCodeBox/secureCodeBox/blob/main/docs/adr/adr_0002.md
A markdown based ADR makes it easier (in my opinion) to work on this proposal (with different alternatives, pros and cons) together, because every team member is able to edit / extend the spec 🙂