Skip to content

Commit e8c2db6

Browse files
jmooringclaude
andcommitted
security: Fix overly permissive file modes and cache directory boundary
Four issues identified during code review and resolved: 1. World-writable directory creation during archive extraction — pkg/archive/extract_targz.go, pkg/archive/extract_zip.go Directories unpacked from archives were created with mode 0o777 (world-writable). Changed to 0o755 to match the principle of least privilege applied elsewhere in the codebase. 2. World-writable file creation during zip extraction — pkg/archive/extract_zip.go Files extracted from .zip archives were created with a hardcoded mode of 0o777. The .tar.gz extractor already did the right thing by using os.FileMode(th.Mode) to preserve the permission bits from the archive header. The .zip extractor now does the same via z.Mode(). Hugo release zips built by goreleaser carry Unix permission metadata, so the Hugo binary emerges executable (0o755) rather than world-writable. 3. World-writable parent directory creation in helpers — pkg/helpers/helpers.go CopyFile used 0o777 when creating parent directories for the destination. Changed to 0o755. 4. Incorrect directory boundary in cache size calculation — pkg/cache/cache.go, pkg/cache/cache_test.go Size() excluded files whose path had excludeDir as a string prefix, meaning a directory named e.g. 'default123' would be silently excluded alongside 'default'. Changed to an exact match check: path != excludeDir && !strings.HasPrefix(path, excludeDir+"/") The accompanying test was asserting the buggy behaviour (with a comment claiming it was intentional); it now asserts the correct behaviour. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9c8b192 commit e8c2db6

5 files changed

Lines changed: 11 additions & 10 deletions

File tree

pkg/archive/extract_targz.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func extractTarGZ(src, dst string) error {
6666
switch th.Typeflag {
6767
case tar.TypeDir:
6868
if _, err := os.Stat(target); err != nil {
69-
if err := os.MkdirAll(target, 0o777); err != nil {
69+
if err := os.MkdirAll(target, 0o755); err != nil {
7070
return err
7171
}
7272
}

pkg/archive/extract_zip.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ func extractZip(src, dst string) error {
4141
}
4242

4343
if f.FileInfo().IsDir() {
44-
err = os.MkdirAll(target, 0o777)
44+
err = os.MkdirAll(target, 0o755)
4545
if err != nil {
4646
return err
4747
}
4848
continue
4949
}
5050

51-
err = os.MkdirAll(filepath.Dir(target), 0o777)
51+
err = os.MkdirAll(filepath.Dir(target), 0o755)
5252
if err != nil {
5353
return err
5454
}
@@ -70,7 +70,7 @@ func copyFileFromZip(z *zip.File, dst string) (retErr error) {
7070
}
7171
defer zrc.Close()
7272

73-
df, err := os.OpenFile(dst, os.O_CREATE|os.O_RDWR, 0o777)
73+
df, err := os.OpenFile(dst, os.O_CREATE|os.O_RDWR, z.Mode())
7474
if err != nil {
7575
return err
7676
}

pkg/cache/cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func Size(cachePath, excludeDir string) (int64, error) {
105105
if err != nil {
106106
return err
107107
}
108-
if !d.IsDir() && !strings.HasPrefix(path, excludeDir) && path != SchemaFileName && path != TagListFileName {
108+
if !d.IsDir() && path != excludeDir && !strings.HasPrefix(path, excludeDir+"/") && path != SchemaFileName && path != TagListFileName {
109109
fi, err := d.Info()
110110
if err != nil {
111111
return err

pkg/cache/cache_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,18 @@ func TestSize(t *testing.T) {
4848
write(t, filepath.Join(base, "default", "hugo"), 10)
4949
write(t, filepath.Join(base, "default", "nested", "file"), 7)
5050

51-
// Path that starts with exclude prefix should be excluded
51+
// Path whose name starts with the exclude prefix but is a different
52+
// directory should be included, not excluded.
5253
write(t, filepath.Join(base, "default123", "file"), 9)
5354

5455
size, err := Size(base, "default")
5556
if err != nil {
5657
t.Fatalf("Size() error: %v", err)
5758
}
5859

59-
// Included: 20 + 5 + 3 = 28
60-
// Excluded: 10 + 7 + 9
61-
if want := int64(28); size != want {
60+
// Included: 20 + 5 + 3 + 9 = 37
61+
// Excluded: 10 + 7
62+
if want := int64(37); size != want {
6263
t.Fatalf("Size(): want %d got %d", want, size)
6364
}
6465
}

pkg/helpers/helpers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func CopyFile(src, dst string) (retErr error) {
6464
}
6565
defer s.Close()
6666

67-
err = os.MkdirAll(filepath.Dir(dst), 0o777)
67+
err = os.MkdirAll(filepath.Dir(dst), 0o755)
6868
if err != nil {
6969
return err
7070
}

0 commit comments

Comments
 (0)