Skip to content

Commit a14a32b

Browse files
committed
Fix validation, file permissions, and temp file handling
- Reject negative timeout values in ParseTimeout - Fix ParseRedirects error message to say "non-negative" - Restrict session and update cache directories to 0700 - Restrict update lock file to 0600 - Use os.CreateTemp for atomic metadata writes
1 parent 196d9c7 commit a14a32b

3 files changed

Lines changed: 22 additions & 9 deletions

File tree

internal/config/config.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ func (c *Config) ParseQuery(value string) error {
479479
func (c *Config) ParseRedirects(value string) error {
480480
n, err := strconv.Atoi(value)
481481
if err != nil || n < 0 {
482-
const usage = "must be a positive integer"
482+
const usage = "must be a non-negative integer"
483483
return core.NewValueError("redirects", value, usage, c.isFile)
484484
}
485485
c.Redirects = &n
@@ -525,8 +525,8 @@ func (c *Config) ParseSilent(value string) error {
525525

526526
func (c *Config) ParseTimeout(value string) error {
527527
secs, err := strconv.ParseFloat(value, 64)
528-
if err != nil {
529-
return core.NewValueError("timeout", value, "must be a valid number", c.isFile)
528+
if err != nil || secs < 0 {
529+
return core.NewValueError("timeout", value, "must be a non-negative number", c.isFile)
530530
}
531531
c.Timeout = new(time.Duration(float64(time.Second) * secs))
532532
return nil

internal/session/session.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func (j *sessionJar) Cookies(u *url.URL) []*http.Cookie {
214214
func getSessionsDir() (string, error) {
215215
// Allow override for testing.
216216
if dir := os.Getenv("FETCH_INTERNAL_SESSIONS_DIR"); dir != "" {
217-
err := os.MkdirAll(dir, 0755)
217+
err := os.MkdirAll(dir, 0700)
218218
if err != nil {
219219
return "", err
220220
}
@@ -227,7 +227,7 @@ func getSessionsDir() (string, error) {
227227
}
228228

229229
path := filepath.Join(dir, "fetch", "sessions")
230-
err = os.MkdirAll(path, 0755)
230+
err = os.MkdirAll(path, 0700)
231231
if err != nil {
232232
return "", err
233233
}

internal/update/update.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ func getCacheDir() (string, error) {
468468
}
469469

470470
path := filepath.Join(dir, "fetch")
471-
err = os.MkdirAll(path, 0755)
471+
err = os.MkdirAll(path, 0700)
472472
if err != nil {
473473
return "", err
474474
}
@@ -483,8 +483,21 @@ func updateLastAttemptTime(dir string, now time.Time) error {
483483
}
484484

485485
path := filepath.Join(dir, "metadata.json")
486-
tempPath := path + ".__temp"
487-
err = os.WriteFile(tempPath, data, 0666)
486+
f, err := os.CreateTemp(dir, ".metadata-*.tmp")
487+
if err != nil {
488+
return err
489+
}
490+
tempPath := f.Name()
491+
defer func() {
492+
// Clean up temp file on error.
493+
if err != nil {
494+
os.Remove(tempPath)
495+
}
496+
}()
497+
_, err = f.Write(data)
498+
if err2 := f.Close(); err == nil {
499+
err = err2
500+
}
488501
if err != nil {
489502
return err
490503
}
@@ -494,7 +507,7 @@ func updateLastAttemptTime(dir string, now time.Time) error {
494507

495508
func acquireLock(ctx context.Context, p *core.Printer, dir string, block bool) (func(), error) {
496509
path := filepath.Join(dir, ".update-lock")
497-
f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0666)
510+
f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0600)
498511
if err != nil {
499512
return nil, err
500513
}

0 commit comments

Comments
 (0)