Add go support #89
Add go support #89
Conversation
ikretz
left a comment
ikretz
left a comment
There was a problem hiding this comment.
Looks great! Thank for for making this PR so clean and high-quality: it was very easy to review. I just had a couple questions here and there but am ready to approve.
The main branch has gotten a little ahead of this one, but it shouldn't be too bad to resynchronize. The main thing we'll need to do is implement the new list_installed_packages() method of PackageManager for the Go class. This was added to support the new audit feature in bcedd7d. This is the only reason why I selected "Request changes" instead of "Approve", since all package managers have to support both modes.
I am happy to help with resolving conflicts or anything else related to getting this merged. Thanks again, @SirGFM!
| if platform.system() == "Windows": | ||
| separator = ";" |
There was a problem hiding this comment.
We currently do not support Windows so we can remove this for now.
Good to know for down the road, though!
| if self._original_dir is None: | ||
| raise GoModNotFoundError("Failed to find a 'go.mod' file to operate on") | ||
|
|
||
| mod_file = self._original_dir / "go.mod" |
There was a problem hiding this comment.
I see on line 260 that you're checking whether the go.sum file exists. Do we need to do a similar check here for go.mod or is it safe to assume it exists?
| if len(command) > 2 and command[1] == "mod" and not command[2] in INSPECTED_MOD_COMMANDS: | ||
| return [] |
There was a problem hiding this comment.
Is there the potential to have optional command-line arguments that could change the indices for these tokens? It doesn't look like it based on the help messages but I thought I would confirm.
| is_tidy = len(command) > 2 and command[1] == "mod" and command[2] == "tidy" | ||
| for param in command[2 if not is_tidy else 3:]: |
There was a problem hiding this comment.
Same question about indices and optional command-line arguments as above.
| if is_tidy: | ||
| tmp.duplicate_go_mod() | ||
| tmp.run(["mod", "tidy"], True) | ||
|
|
||
| if is_tidy or len(local_packages) > 0: | ||
| dry_run = tmp.run(["list", "-m", "all"], True) | ||
| packages.update(set(filter(None, map(line_to_package, dry_run.stdout.split('\n'))))) |
There was a problem hiding this comment.
Would it be possible/desirable to always duplicate the current project into the temporary directory and then run the given command in there? I think this wouldn't have any unintended effects on the "real" project, right?
|
I thought of one more thing we'll want to do: add matrix tests in CI for Go across all supported versions of the package manager. |
SirGFM
force-pushed
the
sirgfm/add-go-support
branch
from
6765c3f to
dc61de6
Compare
SirGFM
force-pushed
the
sirgfm/add-go-support
branch
from
dc61de6 to
5af9e8b
Compare
2 participants
This PR adds to SCFW the ability to verify go packages:
Although go has some commands that explicitly install dependencies (e.g.,
go get <pkg>andgo mod download), go also automatically downloads dependencies whenever it deems necessary. For example, if you've just cloned a repository and try to execute it (e.g.,go run ./main.go), it would automatically download any missing dependency. Additionally,go mod tidymay be used to both updatego.modandgo.sumbased on the imports being used in the project as well as to download those dependencies.Considering all that, this implementation creates a temporary directory where it initially downloads anything that would be required by command. For
go getcommands, it only downloads the requested package(s) and its dependencies, if any. For other commands (go install <pkg>,go run <pkg>,go mod *, etc), verifies every dependency used by the package. This temporary directory is deleted when the analysis of the packages that will be installed finishes, alongside any cache used by go.An important detail is that since
go mod tidymay add more dependencies togo.mod, it must run on the project itself. Therefore, for that specific command bothgo.modandgo.sumare saved in a temporarily directory and the changes are applied directly to the current project, restoring those files when the analysis finishes.Currently this only checks for vulnerabilities, as the database would need to be updated with information about malicious go packages.
I'm opening this mainly as as suggestion, as I know that this implementation doesn't follow the SCFW's principles of not installing anything until it has been verified.