fix: fix up tests and linting

9 files changed, 81 insertions, 49 deletions

diff --git a/internal/admin/middleware.go b/internal/admin/middleware.go
index 56d4797..60274ad 100644
--- a/internal/admin/middleware.go
+++ b/internal/admin/middleware.go
@@ -5,8 +5,8 @@ import (
 	"net/http"
 )
 
-// Admin middleware to handle requests to the admin repo.
-func AdminHooks(adminSvc *Servicer, next http.Handler) http.Handler {
+// Hooks middleware to handle requests to the admin repo.
+func Hooks(adminSvc *Servicer, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		log.Printf("stuffs about to reload %s", "now")
 		next.ServeHTTP(rw, req)
diff --git a/internal/admin/model.go b/internal/admin/model.go
index 5a7f984..2841e0a 100644
--- a/internal/admin/model.go
+++ b/internal/admin/model.go
@@ -15,9 +15,7 @@ import (
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/storage/filesystem"
 	"github.com/go-git/go-git/v5/storage/memory"
-
 	"gopkg.in/ini.v1"
-
 	"sigs.k8s.io/yaml"
 )
 
@@ -30,7 +28,7 @@ const (
 	Admin = 2
 	// GitExportMagic magic file name for daemon export
 	GitExportMagic = "git-daemon-export-ok"
-	// GitWebExportMagic
+	// GitWebExportMagic magic filename for web repos
 	GitWebExportMagic = "git-web-export-ok"
 )
 
@@ -71,11 +69,11 @@ type ServerRepos struct {
 	BasePath string     `json:"basepath"`
 }
 
-func loadFromGit(gitUrl, filePath string) ([]byte, error) {
+func loadFromGit(gitURL, filePath string) ([]byte, error) {
 	fs := memfs.New()
 	storer := memory.NewStorage()
 	_, err := git.Clone(storer, fs, &git.CloneOptions{
-		URL: gitUrl,
+		URL: gitURL,
 	})
 	if err != nil {
 		// log.error
@@ -97,6 +95,7 @@ func loadLocalFile(path string) ([]byte, error) {
 		log.Printf("config file not opened %s", path)
 		return []byte{}, err
 	}
+	defer file.Close()
 	configBytes, err := io.ReadAll(file)
 	if err != nil {
 		log.Print("config file not read")
@@ -105,9 +104,12 @@ func loadLocalFile(path string) ([]byte, error) {
 	return configBytes, nil
 }
 
+// loadServerConfig configPath should be the absolutepath to the configmap if it's not in a repo
 func loadServerConfig(mgmtRepo bool, baseDir, configPath string) (*ServerRepos, error) {
-	configBytes := []byte{}
-	var err error
+	var (
+		configBytes []byte
+		err         error
+	)
 	if mgmtRepo {
 		repoURI := filepath.Join("file:///", baseDir, "mgmt.git")
 		configBytes, err = loadFromGit(repoURI, configPath)
@@ -117,17 +119,17 @@ func loadServerConfig(mgmtRepo bool, baseDir, configPath string) (*ServerRepos,
 			return &ServerRepos{}, err
 		}
 	} else {
-		configBytes, err = loadLocalFile(filepath.Join(baseDir, configPath))
+		configBytes, err = loadLocalFile(configPath)
 		if err != nil {
 			// log.error
-			log.Print("Failed to load config file from git")
+			log.Print("Failed to load config file from file system")
 			return &ServerRepos{}, err
 		}
 	}
 	config := &ServerRepos{}
 	err = yaml.Unmarshal(configBytes, &config)
 	if err != nil {
-		return &ServerRepos{}, errors.New("Could not parse gitserver config")
+		return &ServerRepos{}, errors.New("could not parse gitserver config")
 	}
 	return config, nil
 }
@@ -189,8 +191,8 @@ func (r *GitRepo) ReconcileRepo(basePath string) {
 	_, err := os.Stat(repoBase)
 	if errors.Is(err, fs.ErrNotExist) {
 		// if no exist -> init bare
-		fs := osfs.New(repoBase)
-		strg := filesystem.NewStorage(fs, nil)
+		repoFs := osfs.New(repoBase)
+		strg := filesystem.NewStorage(repoFs, nil)
 		_, _ = git.Init(strg, nil)
 	}
 	// set export file for git-http-backend
@@ -231,7 +233,9 @@ func (r *GitWeb) ReconcileGitConf(repoBase string) {
 	if (GitWeb{} == *r) {
 		if cfg.HasSection("gitweb") {
 			cfg.DeleteSection("gitweb")
-			cfg.SaveTo(confPath)
+			if err := cfg.SaveTo(confPath); err != nil {
+				log.Fatalf("Coudln't save gitconfig %s", err)
+			}
 		}
 		return
 	}
@@ -240,5 +244,7 @@ func (r *GitWeb) ReconcileGitConf(repoBase string) {
 	section.Key("owner").SetValue(r.Owner)
 	section.Key("url").SetValue(r.URL)
 	section.Key("category").SetValue(r.Category)
-	cfg.SaveTo(confPath)
+	if err := cfg.SaveTo(confPath); err != nil {
+		log.Fatalf("Coudln't save gitconfig %s", err)
+	}
 }
diff --git a/internal/admin/model_test.go b/internal/admin/model_test.go
index 7f816f5..70ec738 100644
--- a/internal/admin/model_test.go
+++ b/internal/admin/model_test.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -18,6 +17,7 @@ import (
 	"gopkg.in/ini.v1"
 )
 
+//nolint:cyclop
 func TestCasbinPolicies(t *testing.T) {
 	roleName := "myrole"
 	repoName := "myrepo"
@@ -86,7 +86,7 @@ func TestLoadServerConfig(t *testing.T) {
 		localDir := t.TempDir()
 		// TODO Refactor next touch
 		localFile := filepath.Join(localDir, "stuff.yaml")
-		srcFile, err := os.Open("../../gitserver.yaml")
+		srcFile, err := os.Open(filepath.Clean("../../gitserver.yaml"))
 		if err != nil {
 			t.Fatalf("Error opening base config %s", err)
 		}
@@ -105,7 +105,7 @@ func TestLoadServerConfig(t *testing.T) {
 		}
 
 		// end copy file
-		loadedFile, err := loadServerConfig(false, localDir, "stuff.yaml")
+		loadedFile, err := loadServerConfig(false, localDir, filepath.Join(localDir, "stuff.yaml"))
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -114,7 +114,7 @@ func TestLoadServerConfig(t *testing.T) {
 		}
 	})
 
-	t.Run("testing server config from git", func(t *testing.T) {
+	t.Run("testing server config from git", func(_ *testing.T) {
 
 	})
 }
@@ -122,7 +122,10 @@ func TestLoadServerConfig(t *testing.T) {
 func TestLocalFile(t *testing.T) {
 	localDir := t.TempDir()
 	localFile := filepath.Join(localDir, "stuff.yaml")
-	os.WriteFile(localFile, []byte("stuff"), 0750)
+	//nolint:gosec
+	if err := os.WriteFile(localFile, []byte("stuff"), 0500); err != nil {
+		t.Fatal(err)
+	}
 	loadedFile, err := loadLocalFile(localFile)
 	if err != nil {
 		t.Fatal(err)
@@ -137,6 +140,7 @@ func TestLocalFile(t *testing.T) {
 	}
 }
 
+//nolint:cyclop
 func TestMgmtGitConfig(t *testing.T) {
 	// setup tempdir
 	gitDir := t.TempDir()
@@ -173,7 +177,9 @@ func TestMgmtGitConfig(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	wt.Add(fileToCommit)
+	if _, err := wt.Add(fileToCommit); err != nil {
+		t.Fatal(err)
+	}
 	_, err = wt.Commit(fileToCommit, &git.CommitOptions{})
 	if err != nil {
 		t.Fatalf("Error creating commit %s", err)
@@ -203,13 +209,16 @@ func TestMgmtGitConfig(t *testing.T) {
 	// TODO run via serverLoadConfig
 }
 
+//nolint:cyclop
 func TestConfigReconcile(t *testing.T) {
 	tempDir := t.TempDir()
 	defer os.RemoveAll(tempDir)
 	// make "fake" repo
 	testRepo := filepath.Join(tempDir, "testrepo.git")
 	testConf := filepath.Join(testRepo, "config")
-	os.Mkdir(testRepo, 0750)
+	if err := os.Mkdir(testRepo, 0750); err != nil {
+		t.Fatal(err)
+	}
 	f, err := os.Create(filepath.Join(testRepo, "config"))
 	if err != nil {
 		t.Fatalf("couldn't create testdir, %s", err)
@@ -247,6 +256,9 @@ func TestConfigReconcile(t *testing.T) {
 		emptyGitWeb := &GitWeb{}
 		emptyGitWeb.ReconcileGitConf(testRepo)
 		emptyCfg, err := ini.Load(testConf)
+		if err != nil {
+			t.Fatalf("error couldn't load %s", testConf)
+		}
 		if emptyCfg.HasSection("gitweb") {
 			t.Fatalf("reconciler conf didn't remove section `gitweb`")
 		}
@@ -283,15 +295,18 @@ func TestRepoReconcile(t *testing.T) {
 		t.Fatal("expected repo to be created, but does not exist")
 	}
 	defaultFile := []byte(`
-[core]      
-bare = true 
+[core]
+bare = true
 `)
 	// write the base config to repo
 	tempConfigFile := filepath.Join(repoPath, "config")
-	ioutil.WriteFile(tempConfigFile, defaultFile, 0644)
+	//nolint:gosec
+	if err := os.WriteFile(tempConfigFile, defaultFile, 0500); err != nil {
+		t.Fatal(err)
+	}
 	// re-reconcile
 	repo.ReconcileRepo(tempDir)
-	content, err := ioutil.ReadFile(tempConfigFile)
+	content, err := os.ReadFile(tempConfigFile)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -303,5 +318,4 @@ bare = true
 	if _, err := os.Stat(gitExportMagicPath); errors.Is(err, fs.ErrNotExist) {
 		t.Fatal("expected git export magic to be created, but does not exist")
 	}
-
 }
diff --git a/internal/admin/service.go b/internal/admin/service.go
index 84547fa..1b5662d 100644
--- a/internal/admin/service.go
+++ b/internal/admin/service.go
@@ -41,7 +41,7 @@ func (s *Servicer) InitServer() {
 			continue
 		}
 		if added {
-			numAdded += 1
+			numAdded++
 		}
 	}
 	log.Printf("policies added %d", numAdded)
diff --git a/internal/admin/service_test.go b/internal/admin/service_test.go
index fdd3aa6..e13d28c 100644
--- a/internal/admin/service_test.go
+++ b/internal/admin/service_test.go
@@ -10,7 +10,7 @@ import (
 )
 
 var (
-	updatedServerConfig []byte = []byte(`
+	updatedServerConfig = []byte(`
 ---
 name: "go-git-server"
 version: "v1alpha1"
@@ -42,7 +42,6 @@ repos:
 )
 
 func copyFile(t *testing.T, srcFilePath, destPath string) {
-
 	srcFile, err := os.Open(srcFilePath)
 	if err != nil {
 		t.Fatalf("Error opening base config %s", err)
@@ -87,10 +86,11 @@ func TestInitServer(t *testing.T) {
 	t.Run("test reload config success", func(t *testing.T) {
 		svc := NewService(destModelFile,
 			destPolicyFile,
-			"gitserver.yaml",
+			filepath.Join(tempRepoDir, "gitserver.yaml"),
 			tempRepoDir,
 			false)
-		err := os.WriteFile(destConfigFile, updatedServerConfig, 0755)
+		//nolint:gosec
+		err := os.WriteFile(destConfigFile, updatedServerConfig, 0500)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -104,16 +104,17 @@ func TestInitServer(t *testing.T) {
 		if !strings.Contains(string(data), "thisismynewrepo") {
 			t.Fatal("expected to find test new repo but didn't")
 		}
-
 	})
 	t.Run("test reload config err", func(t *testing.T) {
 		svc := NewService(destModelFile,
 			destPolicyFile,
-			"gitserver.yaml",
+			// TODO set abs path
+			filepath.Join(tempRepoDir, "gitserver.yaml"),
 			tempRepoDir,
 			false)
 		notAGoodConfig := []byte("this is not valid yaml")
-		err := os.WriteFile(destConfigFile, notAGoodConfig, 0755)
+		//nolint:gosec
+		err := os.WriteFile(destConfigFile, notAGoodConfig, 0500)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -127,6 +128,5 @@ func TestInitServer(t *testing.T) {
 		if !strings.Contains(string(data), "mgmt") {
 			log.Fatal("expected to mgmt repo but didn't in policy")
 		}
-
 	})
 }
diff --git a/internal/authz/middleware.go b/internal/authz/middleware.go
index a35b6b4..6763323 100644
--- a/internal/authz/middleware.go
+++ b/internal/authz/middleware.go
@@ -1,3 +1,4 @@
+// authentication and authorization module
 package authz
 
 import (
@@ -11,6 +12,13 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
+// AuthzContextKey key used to store urn of user in context
+type AuthzContextKey string
+
+var (
+	AuthzUrnKey AuthzContextKey = "goGitAuthzUrn"
+)
+
 func Authentication(authMap TokenMap, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		u, p, ok := req.BasicAuth()
@@ -34,7 +42,7 @@ func Authentication(authMap TokenMap, next http.Handler) http.Handler {
 			http.Error(rw, "Bad Request", http.StatusForbidden)
 			return
 		}
-		ctx := context.WithValue(req.Context(), "urn", urn)
+		ctx := context.WithValue(req.Context(), AuthzUrnKey, urn)
 		next.ServeHTTP(rw, req.WithContext(ctx))
 	})
 }
@@ -43,7 +51,10 @@ func Authentication(authMap TokenMap, next http.Handler) http.Handler {
 func Authorization(adminSvc *admin.Servicer, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		ctx := req.Context()
-		urn := ctx.Value("urn").(string)
+		urn, ok := ctx.Value(AuthzUrnKey).(string)
+		if !ok || urn == "" {
+			http.Error(rw, "Bad Request", http.StatusBadRequest)
+		}
 		repo := req.URL.Path
 		action := req.Method
 		ok, err := adminSvc.Enforce(urn, repo, action)
diff --git a/internal/authz/middleware_test.go b/internal/authz/middleware_test.go
index cc3f6d1..9ed9081 100644
--- a/internal/authz/middleware_test.go
+++ b/internal/authz/middleware_test.go
@@ -40,11 +40,10 @@ func TestAuthentication(t *testing.T) {
 			description: "Good Login",
 			handler: func(rw http.ResponseWriter, req *http.Request) {
 				ctx := req.Context()
-				uid := ctx.Value("urn")
+				uid := ctx.Value(AuthzUrnKey)
 				if uid != fmt.Sprintf("uid:%s", okUserName) {
 					t.Fatal("Context UID not set")
 				}
-
 			},
 		},
 		{
@@ -72,6 +71,7 @@ func TestAuthentication(t *testing.T) {
 		recorder := httptest.NewRecorder()
 		authHandler.ServeHTTP(recorder, req)
 		result := recorder.Result()
+		defer result.Body.Close()
 		if result.StatusCode != tc.statusCode {
 			t.Fatalf("Test Case %s failed Expected: %d Found: %d",
 				tc.description, tc.statusCode, result.StatusCode)
@@ -94,13 +94,13 @@ func TestAuthorization(t *testing.T) {
 			url:            fmt.Sprintf("%s/%s", baseURL, "repo/url"),
 			user:           "uid:jack",
 			expectedStatus: 200,
-			description:    "an autorized action should yield a 200",
+			description:    "an authorized action should yield a 200",
 		},
 		{
 			url:            fmt.Sprintf("%s/%s", baseURL, "repo/url/bar"),
 			user:           "uid:chumba",
 			expectedStatus: 403,
-			description:    "an unautorized action should yield a 403",
+			description:    "an unauthorized action should yield a 403",
 		},
 	}
 	svcr := admin.NewService(
@@ -115,12 +115,13 @@ func TestAuthorization(t *testing.T) {
 		recorder := httptest.NewRecorder()
 		req := httptest.NewRequest(http.MethodGet, tc.url, nil)
 		ctx := req.Context()
-		ctx = context.WithValue(ctx, "urn", tc.user)
+		ctx = context.WithValue(ctx, AuthzUrnKey, tc.user)
 		req = req.WithContext(ctx)
 		authHandler.ServeHTTP(recorder, req)
 		result := recorder.Result()
+		defer result.Body.Close()
 		if result.StatusCode != tc.expectedStatus {
-			t.Fatalf("Test Case failed Expected: %d Found: %d", tc.expectedStatus, result.StatusCode)
+			t.Fatalf("Test Case %s failed Expected: %d Found: %d", tc.description, tc.expectedStatus, result.StatusCode)
 		}
 	}
 }
diff --git a/internal/authz/model.go b/internal/authz/model.go
index cf9c952..efa78f7 100644
--- a/internal/authz/model.go
+++ b/internal/authz/model.go
@@ -19,7 +19,7 @@ func NewTokenMap() TokenMap {
 // TokenMap a map of username,hash
 type TokenMap map[string]string
 
-// LoadTokens load tokens from a csv into a map
+// LoadTokensFromFile load tokens from a csv into a map
 func (tm TokenMap) LoadTokensFromFile(path string) error {
 	// TODO this should be configurable
 	contents, err := os.Open(path)
@@ -45,8 +45,8 @@ func (tm TokenMap) LoadTokensFromFile(path string) error {
 func GenerateNewToken() (string, string, error) {
 	tokenBytes := make([]byte, 28)
 	for i := range tokenBytes {
-		max := big.NewInt(int64(255))
-		randInt, err := rand.Int(rand.Reader, max)
+		maxInt := big.NewInt(int64(255))
+		randInt, err := rand.Int(rand.Reader, maxInt)
 		if err != nil {
 			return "", "", err
 		}
diff --git a/internal/git/handler.go b/internal/git/handler.go
index e90ab5f..6dc4584 100644
--- a/internal/git/handler.go
+++ b/internal/git/handler.go
@@ -3,7 +3,7 @@ package git
 import (
 	"fmt"
 	"net/http"
-	"net/http/cgi"
+	"net/http/cgi" //nolint:gosec
 )
 
 // GitHttpBackendHandler a handler for git cgi