Adding exit codes

[Torxed]

Description

This PR will add exit codes on scan.
This will help automating and adding guarddog to runners or scripts, as there's no other way to determine if guarddog found issues or not in a machine friendly way (other than grep [1-9]+ or some other shenanigans).

This will use results['issues'] counter, 0 results will exit 0 and anything above will exit with the number of issues.

Usage

Assuming a runner similar to https://github.com/Torxed/archoffline/blob/797436de3999e73963af4b60364e0c9767afece9/.github/workflows/guarddog.yaml:

on: [ push, pull_request ]
name: guarddog security checkup
jobs:
    guarddog:
        runs-on: ubuntu-latest
        container:
            image: archlinux:latest
        steps:
            - uses: actions/checkout@v3
            - run: pacman --noconfirm -Syu git python python-setuptools python-pip python-build
            - run: python -m pip install --upgrade pip
            - run: pip install git+https://github.com/DataDog/guarddog.git
            - run: python --version
            - name: run build
              run: python -m build
            - name: run guarddog
              run: guarddog scan dist/*.tar.gz

Normally, this runner could never exit with a bad exit code, leaving a green tick on github:

When in fact, there might have been 1+ issues.
And this is due to github runners checking for non-zero exit codes (by default) to indicate if the runner had any complaints.

This should allow people to incorporate guarddog in their runners in a natural way :)

Footnote

Cool project and fun approach to grabbing the common suspects.

[christophetd]

Thanks for the contribution! Definitely something I wanted to tackle at some point.

A few things:

• We might not want this behavior by default. Technically, if GuardDog runs and finds some issues, it did run properly, so it sounds semantically wrong to exit with a non-zero status code. How about we add a dedicated flag instead? e.g. --exit-non-zero-on-finding

• To have a predictable status code, how about we exit with status code 1 if this flag is passed and at least 1 issue was found? Otherwise the status code depends on the number of issues identified

[Torxed]

I personally wouldn't object to adding --exit-non-zero-on-finding.
Exit codes is mostly used not just to indicate if the program ran or not. But to the outside world if the program did it's thing without anything to expect or if something happened that needs further investigation (1 being the default catch-all have a look at the result, >1 being custom codes which should be defined in the man pages). So it's more as a signaling feature if it found something or not. Not necessarily if the program behaved correctly which it can on != 0 codes too. But I get your point.

Some examples:

• blkid - https://github.com/util-linux/util-linux/blob/12786a901520d7b30ff75ee8727221bf8a3089cb/misc-utils/blkid.c#L29-L31

  #define BLKID_EXIT_NOTFOUND 2 /* token or device not found /
  #define BLKID_EXIT_OTHER 4 / bad usage or other error /
  #define BLKID_EXIT_AMBIVAL 8 / ambivalent low-level probing detected */

• clamav - https://linux.die.net/man/1/clamscan

  Return Codes
  0 : No virus found.

  1 : Virus(es) found.
  2 : Some error(s) occured.

But I'm open to modify the PR :)

[Torxed]

Changed to an exit code of maximum 1 based on potential malicious indicators.
I default to 1 if for some reason result['issues'] should be renamed to catch it.

[christophetd]

Thanks, would like to add a few things, can you tick "Allow maintainers to edit my pull request"?

[Torxed]

@christophetd That is already ticked :) I can add the things too if you're busy or want help. But by all means, go nuts! :)

[christophetd]

Weird, I can't seem to push it:

$ git pull --rebase pr-76 adding-exit-codes2
From github.com:Torxed/guarddog
 * branch            adding-exit-codes2 -> FETCH_HEAD
Current branch pr-76 is up to date

$ git push pr-76 pr-76/adding-exit-codes2
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
To github.com:Torxed/guarddog.git
 ! [remote rejected] pr-76/adding-exit-codes2 -> pr-76/adding-exit-codes2 (permission denied)
error: failed to push some refs to 'github.com:Torxed/guarddog.git'

Anyway, this was my suggested set of changes, care to apply it yourself? Then we can merge it.Let me know if that looks good!

commit fda55b962ad5f405b999f986f29256b61d1ca6db
Author: Christophe Tafani-Dereeper <christophe.tafanidereeper@datadoghq.com>
Date:   Mon Nov 28 09:59:08 2022 +0100

    Exit with non-zero status code when issues are identified and a specific CLI flag is provided

diff --git a/guarddog/cli.py b/guarddog/cli.py
index f068dba..56cd8a0 100644
--- a/guarddog/cli.py
+++ b/guarddog/cli.py
@@ -18,7 +18,7 @@ from .scanners.project_scanner import RequirementsScanner
 
 analyzer = Analyzer()
 ALL_RULES = analyzer.sourcecode_ruleset | analyzer.metadata_ruleset
-
+EXIT_CODE_ISSUES_FOUND = 1
 
 @click.group
 def cli():
@@ -29,7 +29,8 @@ def cli():
 @cli.command("verify")
 @click.argument("path")
 @click.option("--json", default=False, is_flag=True, help="Dump the output as JSON to standard out")
-def verify(path, json):
+@click.option("--exit-non-zero-on-finding", default=False, is_flag=True, help="Exit with a non-zero status code if at least one issue is identified")
+def verify(path, json, exit_non_zero_on_finding):
     """Verify a requirements.txt file
 
     Args:
@@ -46,6 +47,8 @@ def verify(path, json):
         import json as js
         print(js.dumps(results))
 
+    if exit_non_zero_on_finding:
+        exit_with_status_code(results)
 
 @cli.command("scan")
 @click.argument("identifier")
@@ -53,7 +56,8 @@ def verify(path, json):
 @click.option("-r", "--rules", multiple=True, type=click.Choice(ALL_RULES, case_sensitive=False))
 @click.option("-x", "--exclude-rules", multiple=True, type=click.Choice(ALL_RULES, case_sensitive=False))
 @click.option("--json", default=False, is_flag=True, help="Dump the output as JSON to standard out")
-def scan(identifier, version, rules, exclude_rules, json):
+@click.option("--exit-non-zero-on-finding", default=False, is_flag=True, help="Exit with a non-zero status code if at least one issue is identified")
+def scan(identifier, version, rules, exclude_rules, json, exit_non_zero_on_finding):
     """Scan a package
 
     Args:
@@ -81,7 +85,8 @@ def scan(identifier, version, rules, exclude_rules, json):
     else:
         print_scan_results(results, identifier)
 
-    exit(min(results.get('issues', 1), 1))
+    if exit_non_zero_on_finding:
+        exit_with_status_code(results)
 
 # Determines if the input passed to the 'scan' command is a local package name
 def is_local_package(input):
@@ -114,5 +119,13 @@ def print_scan_results(results, identifier):
                 print('  * ' + finding['message'] + ' at ' + finding['location'] + '\n    ' + format_code_line_for_output(finding['code']))
             print()
 
+
 def format_code_line_for_output(code):
-    return '    ' + colored(code.strip().replace('\n', '\n    ').replace('\t', '  '), None, 'on_red', attrs=['bold'])
\ No newline at end of file
+    return '    ' + colored(code.strip().replace('\n', '\n    ').replace('\t', '  '), None, 'on_red', attrs=['bold'])
+
+
+# Given the results, exit with the appropriate status code
+def exit_with_status_code(results):
+    num_issues = results.get('issues', 0)
+    if num_issues > 0:
+        exit(EXIT_CODE_ISSUES_FOUND)
\ No newline at end of file

[Torxed]

I've had better success in the past by git cloning the contributors repo, and working directly in it by doing:

$ git clone .. && cd
$ git checkout adding-exit-codes2
$ git add -A . && git commit ..

For whatever reason, that's what works for me.
Patch is applied now tho! :)

[christophetd]

Thanks!

[Torxed]

No worries :) Happy to help!