Improve validation helpers

Still, there are a lot of bugs in the shroom module to be fixed later.

hyphae/files.go

@@ -6,7 +6,6 @@ import (
 	"path/filepath"
 
 	"github.com/bouncepaw/mycorrhiza/mimetype"
-	"github.com/bouncepaw/mycorrhiza/util"
 )
 
 // Index finds all hypha files in the full `path` and saves them to the hypha storage.
@@ -20,7 +19,7 @@ func Index(path string) {
 	}(ch)
 
 	for h := range ch {
-		// At this time it is safe to ignore the mutex, because there is only one worker.
+		// It's safe to ignore the mutex because there is a single worker right now.
 		if oh := ByName(h.Name); oh.Exists {
 			oh.MergeIn(h)
 		} else {
@@ -32,7 +31,9 @@ func Index(path string) {
 	log.Println("Indexed", Count(), "hyphae")
 }
 
-// indexHelper finds all hypha files in the full `path` and sends them to the channel. Handling of duplicate entries and attachment and counting them is up to the caller.
+// indexHelper finds all hypha files in the full `path` and sends them to the
+// channel. Handling of duplicate entries and attachment and counting them is
+// up to the caller.
 func indexHelper(path string, nestLevel uint, ch chan *Hypha) {
 	nodes, err := os.ReadDir(path)
 	if err != nil {
@@ -40,10 +41,10 @@ func indexHelper(path string, nestLevel uint, ch chan *Hypha) {
 	}
 
 	for _, node := range nodes {
-		// If this hypha looks like it can be a hypha path, go deeper. Do not touch the .git and static folders for they have an administrative importance!
+		// If this hypha looks like it can be a hypha path, go deeper. Do not
-		if node.IsDir() &&
+		// touch the .git and static folders for they have an administrative
-			util.IsCanonicalName(node.Name()) &&
+		// importance!
-			node.Name() != ".git" &&
+		if node.IsDir() && IsValidName(node.Name()) && node.Name() != ".git" &&
 			!(nestLevel == 0 && node.Name() == "static") {
 			indexHelper(filepath.Join(path, node.Name()), nestLevel+1, ch)
 			continue

hyphae/hyphae.go

@@ -2,16 +2,31 @@
 package hyphae
 
 import (
-	"github.com/bouncepaw/mycorrhiza/files"
 	"log"
 	"path/filepath"
 	"regexp"
+	"strings"
 	"sync"
+
+	"github.com/bouncepaw/mycorrhiza/files"
 )
 
-// HyphaPattern is a pattern which all hyphae must match.
+// HyphaPattern is a pattern which all hyphae names must match.
 var HyphaPattern = regexp.MustCompile(`[^?!:#@><*|"'&%{}]+`)
 
+// IsValidName checks for invalid characters and path traversals.
+func IsValidName(hyphaName string) bool {
+	if !HyphaPattern.MatchString(hyphaName) {
+		return false
+	}
+	for _, segment := range strings.Split(hyphaName, "/") {
+		if segment == ".git" || segment == ".." {
+			return false
+		}
+	}
+	return true
+}
+
 // Hypha keeps vital information about a hypha
 type Hypha struct {
 	sync.RWMutex

shroom/rename.go

@@ -23,7 +23,7 @@ func canRenameThisToThat(oh *hyphae.Hypha, nh *hyphae.Hypha, u *user.User, lc *l
 		return lc.Get("ui.rename_noname"), errors.New(lc.Get("ui.rename_noname_tip"))
 	}
 
-	if !hyphae.HyphaPattern.MatchString(nh.Name) {
+	if !hyphae.IsValidName(nh.Name) {
 		rejectRenameLog(oh, u, fmt.Sprintf("new name ‘%s’ invalid", nh.Name))
 		return lc.Get("ui.rename_badname"), errors.New(lc.Get("ui.rename_badname_tip", &l18n.Replacements{"chars": "<code>^?!:#@&gt;&lt;*|\"\\'&amp;%</code>"}))
 	}

shroom/upload.go

@@ -72,7 +72,7 @@ func uploadHelp(h *hyphae.Hypha, hop *history.Op, ext string, data []byte, u *us
 		originalFullPath = &h.TextPath
 		originalText     = "" // for backlink update
 	)
-	if isBadPath(fullPath) {
+	if !isValidPath(fullPath) || !hyphae.IsValidName(h.Name) {
 		err := errors.New("bad path")
 		return hop.WithErrAbort(err), err.Error()
 	}
@@ -110,8 +110,6 @@ func uploadHelp(h *hyphae.Hypha, hop *history.Op, ext string, data []byte, u *us
 	return hop.WithFiles(fullPath).WithUser(u).Apply(), ""
 }
 
-func isBadPath(pathname string) bool {
+func isValidPath(pathname string) bool {
-	return !strings.HasPrefix(pathname, files.HyphaeDir()) ||
+	return strings.HasPrefix(pathname, files.HyphaeDir())
-		strings.Contains(pathname, "..") ||
-		strings.Contains(pathname, "/.git/")
 }

tools.go

@@ -1,3 +1,4 @@
+//go:build tools
 // +build tools
 
 package tools

tree/tree.go

@@ -127,7 +127,7 @@ type child struct {
 func figureOutChildren(hyphaName string, exists bool) child {
 	var (
 		descPrefix = hyphaName + "/"
-		child = child{hyphaName, true, make([]child, 0)}
+		child      = child{hyphaName, true, make([]child, 0)}
 	)
 
 	for desc := range hyphae.YieldExistingHyphae() {
@@ -153,9 +153,9 @@ func addHyphaToChild(hyphaName, subPath string, child *child) {
 	} else {
 		var (
 			firstSlash = strings.IndexRune(subPath, '/')
-			firstDir = subPath[:firstSlash]
+			firstDir   = subPath[:firstSlash]
-			restOfPath = subPath[firstSlash + 1:]
+			restOfPath = subPath[firstSlash+1:]
-			subchild = findOrCreateSubchild(firstDir, child)
+			subchild   = findOrCreateSubchild(firstDir, child)
 		)
 		addHyphaToChild(hyphaName, restOfPath, subchild)
 	}
@@ -172,7 +172,7 @@ func findOrCreateSubchild(name string, baseChild *child) *child {
 		}
 	}
 	baseChild.children = append(baseChild.children, child{fullName, false, make([]child, 0)})
-	return &baseChild.children[len(baseChild.children) - 1]
+	return &baseChild.children[len(baseChild.children)-1]
 }
 
 type sibling struct {

user/net.go

@@ -44,7 +44,7 @@ func Register(username, password, group, source string, force bool) error {
 	username = util.CanonicalName(username)
 
 	switch {
-	case !util.IsPossibleUsername(username):
+	case !IsValidUsername(username):
 		return fmt.Errorf("illegal username ‘%s’", username)
 	case !ValidGroup(group):
 		return fmt.Errorf("invalid group ‘%s’", group)

user/user.go

@@ -2,6 +2,8 @@ package user
 
 import (
 	"net/http"
+	"regexp"
+	"strings"
 	"sync"
 	"time"
 
@@ -9,7 +11,9 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
-// User is a user (duh).
+var usernamePattern = regexp.MustCompile(`[^?!:#@><*|"'&%{}/]+`)
+
+// User contains information about a given user required for identification.
 type User struct {
 	// Name is a username. It must follow hypha naming rules.
 	Name         string    `json:"name"`
@@ -117,3 +121,22 @@ func (user *User) ShowLockMaybe(w http.ResponseWriter, rq *http.Request) bool {
 	}
 	return false
 }
+
+// IsValidUsername checks if the given username is valid.
+func IsValidUsername(username string) bool {
+	return username != "anon" && username != "wikimind" &&
+		usernamePattern.MatchString(strings.TrimSpace(username)) &&
+		usernameIsWhiteListed(username)
+}
+
+func usernameIsWhiteListed(username string) bool {
+	if !cfg.UseWhiteList {
+		return true
+	}
+	for _, allowedUsername := range cfg.WhiteList {
+		if allowedUsername == username {
+			return true
+		}
+	}
+	return false
+}

util/util.go

@@ -6,7 +6,6 @@ import (
 	"github.com/bouncepaw/mycorrhiza/files"
 	"log"
 	"net/http"
-	"regexp"
 	"strings"
 
 	"github.com/bouncepaw/mycomarkup/v2/util"
@@ -65,33 +64,6 @@ func CanonicalName(name string) string {
 	return util.CanonicalName(name)
 }
 
-// hyphaPattern is a pattern which all hypha names must match.
-var hyphaPattern = regexp.MustCompile(`[^?!:#@><*|"'&%{}]+`)
-
-var usernamePattern = regexp.MustCompile(`[^?!:#@><*|"'&%{}/]+`)
-
-// IsCanonicalName checks if the `name` is canonical.
-func IsCanonicalName(name string) bool {
-	return hyphaPattern.MatchString(name)
-}
-
-// IsPossibleUsername is true if the given username is ok. Same as IsCanonicalName, but cannot have / in it and cannot be equal to "anon" or "wikimind"
-func IsPossibleUsername(username string) bool {
-	return username != "anon" && username != "wikimind" && usernameIsWhiteListed(username) && usernamePattern.MatchString(strings.TrimSpace(username))
-}
-
-func usernameIsWhiteListed(username string) bool {
-	if !cfg.UseWhiteList {
-		return true
-	}
-	for _, allowedUsername := range cfg.WhiteList {
-		if allowedUsername == username {
-			return true
-		}
-	}
-	return false
-}
-
 // HyphaNameFromRq extracts hypha name from http request. You have to also pass the action which is embedded in the url or several actions. For url /hypha/hypha, the action would be "hypha".
 func HyphaNameFromRq(rq *http.Request, actions ...string) string {
 	p := rq.URL.Path