Merge pull request #434 from ryanfowler/fixes

Fix validation, file permissions, and temp file handling

Author: ryanfowler

internal/config/config.go

@@ -479,7 +479,7 @@ func (c *Config) ParseQuery(value string) error {
 func (c *Config) ParseRedirects(value string) error {
 	n, err := strconv.Atoi(value)
 	if err != nil || n < 0 {
-		const usage = "must be a positive integer"
+		const usage = "must be a non-negative integer"
 		return core.NewValueError("redirects", value, usage, c.isFile)
 	}
 	c.Redirects = &n
@@ -525,8 +525,8 @@ func (c *Config) ParseSilent(value string) error {
 
 func (c *Config) ParseTimeout(value string) error {
 	secs, err := strconv.ParseFloat(value, 64)
-	if err != nil {
-		return core.NewValueError("timeout", value, "must be a valid number", c.isFile)
+	if err != nil || secs < 0 {
+		return core.NewValueError("timeout", value, "must be a non-negative number", c.isFile)
 	}
 	c.Timeout = new(time.Duration(float64(time.Second) * secs))
 	return nil

internal/session/session.go

@@ -214,7 +214,7 @@ func (j *sessionJar) Cookies(u *url.URL) []*http.Cookie {
 func getSessionsDir() (string, error) {
 	// Allow override for testing.
 	if dir := os.Getenv("FETCH_INTERNAL_SESSIONS_DIR"); dir != "" {
-		err := os.MkdirAll(dir, 0755)
+		err := os.MkdirAll(dir, 0700)
 		if err != nil {
 			return "", err
 		}
@@ -227,7 +227,7 @@ func getSessionsDir() (string, error) {
 	}
 
 	path := filepath.Join(dir, "fetch", "sessions")
-	err = os.MkdirAll(path, 0755)
+	err = os.MkdirAll(path, 0700)
 	if err != nil {
 		return "", err
 	}

internal/update/update.go

@@ -468,7 +468,7 @@ func getCacheDir() (string, error) {
 	}
 
 	path := filepath.Join(dir, "fetch")
-	err = os.MkdirAll(path, 0755)
+	err = os.MkdirAll(path, 0700)
 	if err != nil {
 		return "", err
 	}
@@ -483,18 +483,32 @@ func updateLastAttemptTime(dir string, now time.Time) error {
 	}
 
 	path := filepath.Join(dir, "metadata.json")
-	tempPath := path + ".__temp"
-	err = os.WriteFile(tempPath, data, 0666)
+	f, err := os.CreateTemp(dir, ".metadata-*.tmp")
+	if err != nil {
+		return err
+	}
+	tempPath := f.Name()
+	defer func() {
+		// Clean up temp file on error.
+		if err != nil {
+			os.Remove(tempPath)
+		}
+	}()
+	_, err = f.Write(data)
+	if err2 := f.Close(); err == nil {
+		err = err2
+	}
 	if err != nil {
 		return err
 	}
 
-	return os.Rename(tempPath, path)
+	err = os.Rename(tempPath, path)
+	return err
 }
 
 func acquireLock(ctx context.Context, p *core.Printer, dir string, block bool) (func(), error) {
 	path := filepath.Join(dir, ".update-lock")
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0666)
+	f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0600)
 	if err != nil {
 		return nil, err
 	}