Merge pull request #43 from crashappsec/nettrino/nilderefs

fix nil pointer derefs on bad permissions and add contributing.md

CONTRIBUTING.md

@@ -0,0 +1,53 @@
+<!-- START doctoc generated TOC please keep comment here to allow auto update -->
+<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
+
+- [Contributing to GitHub Analyzer](#contributing-to-github-analyzer)
+  - [Code of Conduct](#code-of-conduct)
+  - [Questions](#questions)
+  - [Filing a bug or feature](#filing-a-bug-or-feature)
+  - [Submitting changes](#submitting-changes)
+    - [Sample Workflow](#sample-workflow)
+
+<!-- END doctoc generated TOC please keep comment here to allow auto update -->
+
+# Contributing to GitHub Analyzer
+
+Thank you for contributing to GitHub Analyzer! Below you can find some core
+guidelines for contributing to the project.
+
+## Code of Conduct
+
+Be kind and respectful to the members of the community. Take time to educate
+others who are seeking help. Harassment of any kind will not be tolerated.
+
+## Questions
+
+If you have questions or ideas that are not a good fit for an issue
+(i.e., not a feature request or bug) feel free to use the repository's [discussions](https://github.com/crashappsec/github-analyzer/discussions) to get feedback from the community and maintainers.
+
+## Filing a bug or feature
+
+1. When filing a bug or request, please check existing issues to see if there
+   exist some issue capturing the same topic already.
+
+1. When filing bugs, please provide as detailed steps of reproduction as possible.
+
+## Submitting changes
+
+1. You can submit changes via pull requests (PRs). PRs will only be getting reviewed
+   once all lint and unit test steps are passing. We will do our best to reply
+   to requests for reviews in a timely manner, however we cannot provide an SLA
+   for reviews.
+
+1. The current license will remain, and we might introduce a requirement for
+   contributors to sign a CLA once the project matures.
+
+### Sample Workflow
+
+1. Ensure you have [pre-commit](https://pre-commit.com/) installed as it will be used for formatting and linting during PRs
+1. Fork the project in your account
+1. Create your feature branch (`git checkout -b your_handle/your-feature`)
+1. Make changes and add them to staging (`git add .`)
+1. Commit your changes (`git commit -m 'a short description of your feature'`)
+1. Push to your branch (`git push origin your_handle/your-feature`)
+1. Create new pull request

cmd/github-analyzer/main.go

@@ -101,6 +101,14 @@ func runCmd() {
 		}
 	}
 
+	errors := map[issue.IssueID]string{}
+	for k, v := range checkStatuses {
+		if v == nil {
+			errors[k] = ""
+			continue
+		}
+		errors[k] = fmt.Sprintf("%v", v)
+	}
 	issuesPath := filepath.Join(futils.IssuesDir, "issues.json")
 	auditStatsPath := filepath.Join(futils.StatsDir, "auditStats.json")
 	execStatusPath := filepath.Join(futils.MetadataDir, "execStatus.json")
@@ -110,7 +118,7 @@ func runCmd() {
 
 	futils.SerializeFile(issues, issuesPath)
 	futils.SerializeFile(stats, auditStatsPath)
-	futils.SerializeFile(checkStatuses, execStatusPath)
+	futils.SerializeFile(errors, execStatusPath)
 
 	html.Serve(
 		config.ViperEnv.Organization,

cmd/github-analyzer/version.txt

@@ -1 +1 @@
-0.1-pre-alpha-6-g40b79dc
+v0.1.2-pre-alpha-6-g5c5e9c9

pkg/futils/futils.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 
 	"github.com/crashappsec/github-analyzer/pkg/config"
+	"github.com/crashappsec/github-analyzer/pkg/log"
 )
 
 var IssuesDir, StatsDir, MetadataDir, HtmlDir string
@@ -25,13 +26,20 @@ func Init() {
 
 func CreateDir(path string) {
 	if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
-		_ = os.Mkdir(path, os.ModePerm)
+		err = os.Mkdir(path, os.ModePerm)
+		if err != nil {
+			log.Logger.Fatal(
+				"Could not create directories in %s. Please ensure you have write permissions for this directory",
+				path,
+			)
+		}
 	}
 }
 
 func SerializeFile(raw interface{}, writeLoc string) error {
 	output, err := json.MarshalIndent(raw, "", " ")
 	if err != nil {
+		log.Logger.Error(err)
 		return err
 	}
 	return ioutil.WriteFile(writeLoc, output, 0644)

pkg/github/org/org.go

@@ -381,6 +381,11 @@ func (org *Organization) Audit2FA(
 	execStatus := map[issue.IssueID]error{}
 
 	log.Logger.Debug("Checking if 2FA is required at org-level")
+	if org.CoreStats.TwoFactorRequirementEnabled == nil {
+		execStatus[issue.AUTH_2FA_ORG_DISABLED] = utils.PermissionsError
+		execStatus[issue.AUTH_2FA_USER_DISABLED] = utils.PermissionsError
+		return issues, execStatus, utils.PermissionsError
+	}
 	execStatus[issue.AUTH_2FA_ORG_DISABLED] = nil
 	execStatus[issue.AUTH_2FA_USER_DISABLED] = nil
 	if !*org.CoreStats.TwoFactorRequirementEnabled {
@@ -485,16 +490,26 @@ func (org *Organization) AuditCoreStats(
 	var issues []issue.Issue
 	execStatus := make(map[issue.IssueID]error, 2)
 
-	execStatus[issue.TOOLING_ADVANCED_SECURITY_DISABLED] = nil
-	if !*org.CoreStats.AdvancedSecurityEnabledForNewRepos {
+	if org.CoreStats == nil {
+		log.Logger.Fatalf(
+			"It appears you don't have permissions to query this org",
+		)
+	}
+
+	if org.CoreStats.AdvancedSecurityEnabledForNewRepos == nil {
+		execStatus[issue.TOOLING_ADVANCED_SECURITY_DISABLED] = utils.PermissionsError
+	} else if !*org.CoreStats.AdvancedSecurityEnabledForNewRepos {
+		execStatus[issue.TOOLING_ADVANCED_SECURITY_DISABLED] = nil
 		issues = append(
 			issues,
 			issue.OrgAdvancedSecurityDisabled(*org.info.Login),
 		)
 	}
 
-	execStatus[issue.INF_DISC_SECRET_SCANNING_DISABLED] = nil
-	if !*org.CoreStats.SecretScanningEnabledForNewRepos {
+	if org.CoreStats.SecretScanningEnabledForNewRepos == nil {
+		execStatus[issue.INF_DISC_SECRET_SCANNING_DISABLED] = utils.PermissionsError
+	} else if !*org.CoreStats.SecretScanningEnabledForNewRepos {
+		execStatus[issue.INF_DISC_SECRET_SCANNING_DISABLED] = nil
 		issues = append(
 			issues,
 			issue.OrgSecretScanningDisabledForNewRepos(*org.info.Login),

pkg/github/utils/utils.go

@@ -2,6 +2,9 @@ package utils
 
 import (
 	"context"
+	"fmt"
+	"reflect"
+	"runtime"
 	"time"
 
 	"github.com/crashappsec/github-analyzer/pkg/github/types"
@@ -10,6 +13,8 @@ import (
 	"github.com/jpillora/backoff"
 )
 
+var PermissionsError = fmt.Errorf("Permissions error")
+
 func RunnersAggregator(runners *github.Runners) []types.Runner {
 	var orgRunners []types.Runner
 	for _, runner := range runners.Runners {
@@ -104,8 +109,13 @@ func GetPaginatedResult[T any, K any](
 
 		if err != nil {
 			if resp.StatusCode == 403 {
-				log.Logger.Infoln(
-					"It appears the token being used doesn't have access to this information",
+				log.Logger.Debugf(
+					"It appears the token being used doesn't have access to call %v",
+					runtime.FuncForPC(reflect.ValueOf(githubCall).Pointer()).
+						Name(),
+				)
+				log.Logger.Errorf(
+					"The token used does not have premissions to make this API call",
 				)
 			} else {
 				log.Logger.Error(err)

pkg/output/html/html.go

@@ -192,12 +192,12 @@ func parsePermissions(
 
 func parseIssues(
 	execStatusPath, issuesPath string,
-) ([]WrappedIssue, []string, error) {
-	var checks map[issue.IssueID]error
+) ([]WrappedIssue, []string, []string, error) {
+	var checks map[issue.IssueID]string
 	jsonFile, err := os.Open(execStatusPath)
 	if err != nil {
 		jsonFile.Close()
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	jsonBytes, _ := ioutil.ReadAll(jsonFile)
 	json.Unmarshal(jsonBytes, &checks)
@@ -208,7 +208,7 @@ func parseIssues(
 	jsonFile, err = os.Open(issuesPath)
 	if err != nil {
 		jsonFile.Close()
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	jsonBytes, _ = ioutil.ReadAll(jsonFile)
 	json.Unmarshal(jsonBytes, &issues)
@@ -247,14 +247,20 @@ func parseIssues(
 	}
 
 	var passed []string
-	for ch := range checks {
+	var failed []string
+	for ch, hadError := range checks {
 		if strings.HasPrefix(string(ch), "STATS") {
 			continue
 		}
-		passed = append(passed, issue.AvailableChecks[ch])
+		if hadError != "" {
+			failed = append(failed, issue.AvailableChecks[ch])
+		} else {
+			passed = append(passed, issue.AvailableChecks[ch])
+		}
 	}
 	sort.Strings(passed)
-	return wrappedIssues, passed, nil
+	sort.Strings(failed)
+	return wrappedIssues, passed, failed, nil
 }
 
 func Serve(
@@ -267,7 +273,10 @@ func Serve(
 		log.Logger.Error(err)
 	}
 
-	wrappedIssues, checksPassed, err := parseIssues(execStatusPath, issuesPath)
+	wrappedIssues, checksPassed, checksFailed, err := parseIssues(
+		execStatusPath,
+		issuesPath,
+	)
 	if err != nil {
 		log.Logger.Error(err)
 	}
@@ -296,6 +305,7 @@ func Serve(
 		Issues             []WrappedIssue
 		Installations      []InstallationInfo
 		ChecksPassed       []string
+		ChecksFailed       []string
 		Permissions        []string
 		Users              []string
 		PermissionSummary  map[string]map[string]string
@@ -320,6 +330,7 @@ func Serve(
 				Issues:             wrappedIssues,
 				Installations:      wrappedInstallations,
 				ChecksPassed:       checksPassed,
+				ChecksFailed:       checksFailed,
 				Permissions:        perms,
 				Users:              users,
 				PermissionSummary:  permissionSummary,

pkg/output/html/static/css/plain.css

@@ -68,6 +68,9 @@
 .issuetitle {
   color: #a2e504;
 }
+.failedissuetitle {
+  color: #990000;
+}
 .full_abstract {
   display: none;
 }

pkg/output/html/templates/index.html.tmpl

@@ -2,6 +2,7 @@
 {{ $installations := .Installations }}
 {{ $issues := .Issues }}
 {{ $passed := .ChecksPassed }}
+{{ $failed := .ChecksFailed }}
 {{ $apps := .Apps }}
 {{ $users := .Users }}
 {{ $permissions := .Permissions }}
@@ -92,6 +93,29 @@
     </div>
   {{end}}
 
+  {{if $failed}}
+    <div class="page-header">
+      <div class="row">
+        <div class="col-lg-12">
+          Checks that did not successfully complete
+        </div>
+      </div>
+    </div>
+    <div>
+      The following checks had errors - does token you used have the appropriate permissions?
+      <ul>
+        {{range $issue := $failed}}
+        <li>
+          <div class="issue">
+            <div class="failedissuetitle">
+              [X] {{$issue}}
+            </div>
+        </li>
+        {{end}}
+      </ul>
+    </div>
+  {{end}}
+
   <div class="sectiontitle">
     {{.Org}} Misc Stats
   </div>