fix(hints): write debug dumps privately
- commit
- 7bfc830e1aedd840fd8fadd6673796bf7d38e16f
- author
- brice · · 2026-10-19 15:28 UTC
- parent
- ce28c36
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +79 -6
M internal/adapters/hints/dump.go +39 -6
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "path/filepath" | |
| 12 | 12 | "sort" | |
| 13 | 13 | "strings" | |
| 14 | + | "syscall" | |
| 14 | 15 | "time" | |
| 15 | 16 | ||
| 16 | 17 | "github.com/bnema/neferwl/internal/ports" | |
| @@ -101,7 +102,7 @@ func ms(d time.Duration) float64 { return float64(d.Microseconds()) / 1000 } | |||
| 101 | 102 | ||
| 102 | 103 | // writeDump writes the three files of j into dir, then prunes old groups. | |
| 103 | 104 | func writeDump(dir string, j *dumpJob) error { | |
| 104 | - | if err := os.MkdirAll(dir, 0o700); err != nil { | |
| 105 | + | if err := secureDir(dir); err != nil { | |
| 105 | 106 | return err | |
| 106 | 107 | } | |
| 107 | 108 | prefix := filepath.Join(dir, j.when.Format("20060102-150405.000")+"-"+safeName(j.req.Output)) | |
| @@ -184,16 +185,48 @@ func writePNG(path string, img image.Image) error { | |||
| 184 | 185 | }) | |
| 185 | 186 | } | |
| 186 | 187 | ||
| 187 | - | func writeFile(path string, fill func(*os.File) error) error { | |
| 188 | - | f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) | |
| 188 | + | // secureDir makes sure dir is a real directory owned by the current user and | |
| 189 | + | // private to it, whatever it was before. | |
| 190 | + | func secureDir(dir string) error { | |
| 191 | + | if err := os.MkdirAll(dir, 0o700); err != nil { | |
| 192 | + | return err | |
| 193 | + | } | |
| 194 | + | fi, err := os.Lstat(dir) | |
| 189 | 195 | if err != nil { | |
| 190 | 196 | return err | |
| 191 | 197 | } | |
| 192 | - | if err := fill(f); err != nil { | |
| 193 | - | _ = f.Close() | |
| 198 | + | if !fi.IsDir() { | |
| 199 | + | return fmt.Errorf("dump dir %s is not a directory", dir) | |
| 200 | + | } | |
| 201 | + | if st, ok := fi.Sys().(*syscall.Stat_t); ok && int(st.Uid) != os.Getuid() { | |
| 202 | + | return fmt.Errorf("dump dir %s is not owned by the current user", dir) | |
| 203 | + | } | |
| 204 | + | return os.Chmod(dir, 0o700) | |
| 205 | + | } | |
| 206 | + | ||
| 207 | + | // writeFile fills a fresh private temporary file in the directory of path and | |
| 208 | + | // renames it over path: an existing file or symlink there is replaced, never | |
| 209 | + | // followed. | |
| 210 | + | func writeFile(path string, fill func(*os.File) error) error { | |
| 211 | + | f, err := os.CreateTemp(filepath.Dir(path), ".dump-*") | |
| 212 | + | if err != nil { | |
| 194 | 213 | return err | |
| 195 | 214 | } | |
| 196 | - | return f.Close() | |
| 215 | + | tmp := f.Name() | |
| 216 | + | err = f.Chmod(0o600) | |
| 217 | + | if err == nil { | |
| 218 | + | err = fill(f) | |
| 219 | + | } | |
| 220 | + | if cerr := f.Close(); err == nil { | |
| 221 | + | err = cerr | |
| 222 | + | } | |
| 223 | + | if err == nil { | |
| 224 | + | err = os.Rename(tmp, path) | |
| 225 | + | } | |
| 226 | + | if err != nil { | |
| 227 | + | _ = os.Remove(tmp) | |
| 228 | + | } | |
| 229 | + | return err | |
| 197 | 230 | } | |
| 198 | 231 | ||
| 199 | 232 | // outline draws a 2 px frame inside r, clipped to img. | |
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | "path/filepath" | 11 | "path/filepath" | ||
| 12 | "sort" | 12 | "sort" | ||
| 13 | "strings" | 13 | "strings" | ||
| 14 | + | "syscall" | |||
| 14 | "time" | 15 | "time" | ||
| 15 | 16 | ||||
| 16 | "github.com/bnema/neferwl/internal/ports" | 17 | "github.com/bnema/neferwl/internal/ports" | ||
| @@ -101,7 +102,7 @@ func ms(d time.Duration) float64 { return float64(d.Microseconds()) / 1000 } | |||||
| 101 | 102 | ||||
| 102 | // writeDump writes the three files of j into dir, then prunes old groups. | 103 | // writeDump writes the three files of j into dir, then prunes old groups. | ||
| 103 | func writeDump(dir string, j *dumpJob) error { | 104 | func writeDump(dir string, j *dumpJob) error { | ||
| 104 | - | if err := os.MkdirAll(dir, 0o700); err != nil { | 105 | + | if err := secureDir(dir); err != nil { |
| 105 | return err | 106 | return err | ||
| 106 | } | 107 | } | ||
| 107 | prefix := filepath.Join(dir, j.when.Format("20060102-150405.000")+"-"+safeName(j.req.Output)) | 108 | prefix := filepath.Join(dir, j.when.Format("20060102-150405.000")+"-"+safeName(j.req.Output)) | ||
| @@ -184,16 +185,48 @@ func writePNG(path string, img image.Image) error { | |||||
| 184 | }) | 185 | }) | ||
| 185 | } | 186 | } | ||
| 186 | 187 | ||||
| 187 | - | func writeFile(path string, fill func(*os.File) error) error { | 188 | + | // secureDir makes sure dir is a real directory owned by the current user and |
| 188 | - | f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) | 189 | + | // private to it, whatever it was before. |
| 190 | + | func secureDir(dir string) error { | |||
| 191 | + | if err := os.MkdirAll(dir, 0o700); err != nil { | |||
| 192 | + | return err | |||
| 193 | + | } | |||
| 194 | + | fi, err := os.Lstat(dir) | |||
| 189 | if err != nil { | 195 | if err != nil { | ||
| 190 | return err | 196 | return err | ||
| 191 | } | 197 | } | ||
| 192 | - | if err := fill(f); err != nil { | 198 | + | if !fi.IsDir() { |
| 193 | - | _ = f.Close() | 199 | + | return fmt.Errorf("dump dir %s is not a directory", dir) |
| 200 | + | } | |||
| 201 | + | if st, ok := fi.Sys().(*syscall.Stat_t); ok && int(st.Uid) != os.Getuid() { | |||
| 202 | + | return fmt.Errorf("dump dir %s is not owned by the current user", dir) | |||
| 203 | + | } | |||
| 204 | + | return os.Chmod(dir, 0o700) | |||
| 205 | + | } | |||
| 206 | + | ||||
| 207 | + | // writeFile fills a fresh private temporary file in the directory of path and | |||
| 208 | + | // renames it over path: an existing file or symlink there is replaced, never | |||
| 209 | + | // followed. | |||
| 210 | + | func writeFile(path string, fill func(*os.File) error) error { | |||
| 211 | + | f, err := os.CreateTemp(filepath.Dir(path), ".dump-*") | |||
| 212 | + | if err != nil { | |||
| 194 | return err | 213 | return err | ||
| 195 | } | 214 | } | ||
| 196 | - | return f.Close() | 215 | + | tmp := f.Name() |
| 216 | + | err = f.Chmod(0o600) | |||
| 217 | + | if err == nil { | |||
| 218 | + | err = fill(f) | |||
| 219 | + | } | |||
| 220 | + | if cerr := f.Close(); err == nil { | |||
| 221 | + | err = cerr | |||
| 222 | + | } | |||
| 223 | + | if err == nil { | |||
| 224 | + | err = os.Rename(tmp, path) | |||
| 225 | + | } | |||
| 226 | + | if err != nil { | |||
| 227 | + | _ = os.Remove(tmp) | |||
| 228 | + | } | |||
| 229 | + | return err | |||
| 197 | } | 230 | } | ||
| 198 | 231 | ||||
| 199 | // outline draws a 2 px frame inside r, clipped to img. | 232 | // outline draws a 2 px frame inside r, clipped to img. | ||
A internal/adapters/hints/dump_test.go +40 -0
| @@ -0,0 +1,40 @@ | |||
| 1 | + | package hints | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "os" | |
| 5 | + | "path/filepath" | |
| 6 | + | "testing" | |
| 7 | + | "time" | |
| 8 | + | ||
| 9 | + | "github.com/stretchr/testify/require" | |
| 10 | + | ||
| 11 | + | "github.com/bnema/neferwl/internal/ports" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // An existing dump directory is made private, and a symlink at a file name is | |
| 15 | + | // replaced instead of followed. | |
| 16 | + | func TestWriteDumpPrivate(t *testing.T) { | |
| 17 | + | base := t.TempDir() | |
| 18 | + | dir := filepath.Join(base, "hints") | |
| 19 | + | require.NoError(t, os.Mkdir(dir, 0o755)) | |
| 20 | + | require.NoError(t, os.Chmod(dir, 0o755)) | |
| 21 | + | victim := filepath.Join(base, "victim") | |
| 22 | + | require.NoError(t, os.WriteFile(victim, []byte("keep"), 0o644)) | |
| 23 | + | when := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) | |
| 24 | + | prefix := filepath.Join(dir, when.Format("20060102-150405.000")+"-o") | |
| 25 | + | require.NoError(t, os.Symlink(victim, prefix+".json")) | |
| 26 | + | pix := make([]byte, 4*4*4) | |
| 27 | + | j := &dumpJob{when: when, req: ports.HintScanRequest{Output: "o", Width: 4, Height: 4, Scale: 1}, pix: pix, stride: 16, m: &mapping{}} | |
| 28 | + | require.NoError(t, writeDump(dir, j)) | |
| 29 | + | ||
| 30 | + | fi, err := os.Stat(dir) | |
| 31 | + | require.NoError(t, err) | |
| 32 | + | require.Equal(t, os.FileMode(0o700), fi.Mode().Perm()) | |
| 33 | + | got, err := os.ReadFile(victim) | |
| 34 | + | require.NoError(t, err) | |
| 35 | + | require.Equal(t, "keep", string(got)) | |
| 36 | + | li, err := os.Lstat(prefix + ".json") | |
| 37 | + | require.NoError(t, err) | |
| 38 | + | require.True(t, li.Mode().IsRegular()) | |
| 39 | + | require.Equal(t, os.FileMode(0o600), li.Mode().Perm()) | |
| 40 | + | } | |
| @@ -0,0 +1,40 @@ | |||||
| 1 | + | package hints | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "os" | |||
| 5 | + | "path/filepath" | |||
| 6 | + | "testing" | |||
| 7 | + | "time" | |||
| 8 | + | ||||
| 9 | + | "github.com/stretchr/testify/require" | |||
| 10 | + | ||||
| 11 | + | "github.com/bnema/neferwl/internal/ports" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // An existing dump directory is made private, and a symlink at a file name is | |||
| 15 | + | // replaced instead of followed. | |||
| 16 | + | func TestWriteDumpPrivate(t *testing.T) { | |||
| 17 | + | base := t.TempDir() | |||
| 18 | + | dir := filepath.Join(base, "hints") | |||
| 19 | + | require.NoError(t, os.Mkdir(dir, 0o755)) | |||
| 20 | + | require.NoError(t, os.Chmod(dir, 0o755)) | |||
| 21 | + | victim := filepath.Join(base, "victim") | |||
| 22 | + | require.NoError(t, os.WriteFile(victim, []byte("keep"), 0o644)) | |||
| 23 | + | when := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) | |||
| 24 | + | prefix := filepath.Join(dir, when.Format("20060102-150405.000")+"-o") | |||
| 25 | + | require.NoError(t, os.Symlink(victim, prefix+".json")) | |||
| 26 | + | pix := make([]byte, 4*4*4) | |||
| 27 | + | j := &dumpJob{when: when, req: ports.HintScanRequest{Output: "o", Width: 4, Height: 4, Scale: 1}, pix: pix, stride: 16, m: &mapping{}} | |||
| 28 | + | require.NoError(t, writeDump(dir, j)) | |||
| 29 | + | ||||
| 30 | + | fi, err := os.Stat(dir) | |||
| 31 | + | require.NoError(t, err) | |||
| 32 | + | require.Equal(t, os.FileMode(0o700), fi.Mode().Perm()) | |||
| 33 | + | got, err := os.ReadFile(victim) | |||
| 34 | + | require.NoError(t, err) | |||
| 35 | + | require.Equal(t, "keep", string(got)) | |||
| 36 | + | li, err := os.Lstat(prefix + ".json") | |||
| 37 | + | require.NoError(t, err) | |||
| 38 | + | require.True(t, li.Mode().IsRegular()) | |||
| 39 | + | require.Equal(t, os.FileMode(0o600), li.Mode().Perm()) | |||
| 40 | + | } | |||