detached 2b89483

fix(review): harden subprocess and log file guards

commit
2b8948310caa9cd29db0a87534a1b91654297888
author
brice · · 2026-04-19 09:45 UTC
parent
521a0b3
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +81 -11
Unified
UnifiedSplit
M cmd/dumber/main.go +5 -1
@@ -734,7 +734,11 @@ func createUseCases(repos *repositories, cfg *config.Config) *useCases {
734734 func isCEFSubprocess(args []string) bool {
735735 for i, arg := range args {
736736 if strings.HasPrefix(arg, "--type=") {
737- return true
737+ value := strings.TrimPrefix(arg, "--type=")
738+ if value != "" && !strings.HasPrefix(value, "-") {
739+ return true
740+ }
741+ continue
738742 }
739743 if arg == "--type" && i+1 < len(args) {
740744 next := args[i+1]
@@ -734,7 +734,11 @@ func createUseCases(repos *repositories, cfg *config.Config) *useCases {
734 func isCEFSubprocess(args []string) bool {734 func isCEFSubprocess(args []string) bool {
735 for i, arg := range args {735 for i, arg := range args {
736 if strings.HasPrefix(arg, "--type=") {736 if strings.HasPrefix(arg, "--type=") {
737- return true737+ value := strings.TrimPrefix(arg, "--type=")
738+ if value != "" && !strings.HasPrefix(value, "-") {
739+ return true
740+ }
741+ continue
738 }742 }
739 if arg == "--type" && i+1 < len(args) {743 if arg == "--type" && i+1 < len(args) {
740 next := args[i+1]744 next := args[i+1]
M cmd/dumber/main_test.go +5 -0
@@ -88,6 +88,11 @@ func TestIsCEFSubprocess(t *testing.T) {
8888 args: []string{"dumber", "--type", "renderer"},
8989 want: true,
9090 },
91+ {
92+ name: "equals form with empty value",
93+ args: []string{"dumber", "--type="},
94+ want: false,
95+ },
9196 {
9297 name: "bare type",
9398 args: []string{"dumber", "--type"},
@@ -88,6 +88,11 @@ func TestIsCEFSubprocess(t *testing.T) {
88 args: []string{"dumber", "--type", "renderer"},88 args: []string{"dumber", "--type", "renderer"},
89 want: true,89 want: true,
90 },90 },
91+ {
92+ name: "equals form with empty value",
93+ args: []string{"dumber", "--type="},
94+ want: false,
95+ },
91 {96 {
92 name: "bare type",97 name: "bare type",
93 args: []string{"dumber", "--type"},98 args: []string{"dumber", "--type"},
M internal/infrastructure/cef/engine_init.go +45 -10
@@ -420,22 +420,57 @@ func prepareCEFLogFile(defaultLogFile, logFile string) (string, error) {
420420 if err := os.MkdirAll(filepath.Dir(runtimeLogFile), dirPerm); err != nil {
421421 return "", fmt.Errorf("mkdir %s: %w", filepath.Dir(runtimeLogFile), err)
422422 }
423- if _, err := os.Stat(runtimeLogFile); err != nil {
423+
424+ file, err := openRegularLogFile(runtimeLogFile)
425+ if err != nil {
426+ return "", err
427+ }
428+ defer func() { _ = file.Close() }()
429+
430+ info, err := file.Stat()
431+ if err != nil {
432+ return "", fmt.Errorf("stat %s: %w", runtimeLogFile, err)
433+ }
434+ if !info.Mode().IsRegular() {
435+ return "", fmt.Errorf("log path %s is not a regular file", runtimeLogFile)
436+ }
437+ if err := file.Chmod(filePerm); err != nil {
438+ return "", fmt.Errorf("chmod %s: %w", runtimeLogFile, err)
439+ }
440+ return runtimeLogFile, nil
441+}
442+
443+func openRegularLogFile(path string) (*os.File, error) {
444+ info, err := os.Lstat(path)
445+ if err != nil {
424446 if !errors.Is(err, os.ErrNotExist) {
425- return "", fmt.Errorf("stat %s: %w", runtimeLogFile, err)
447+ return nil, fmt.Errorf("lstat %s: %w", path, err)
426448 }
427- file, createErr := os.OpenFile(runtimeLogFile, os.O_CREATE|os.O_WRONLY, filePerm)
449+ file, createErr := openFileNoFollow(path, syscall.O_CREAT|syscall.O_EXCL|syscall.O_WRONLY, filePerm)
428450 if createErr != nil {
429- return "", fmt.Errorf("create %s: %w", runtimeLogFile, createErr)
430- }
431- if closeErr := file.Close(); closeErr != nil {
432- return "", fmt.Errorf("close %s: %w", runtimeLogFile, closeErr)
451+ return nil, fmt.Errorf("create %s: %w", path, createErr)
433452 }
453+ return file, nil
434454 }
435- if err := os.Chmod(runtimeLogFile, filePerm); err != nil {
436- return "", fmt.Errorf("chmod %s: %w", runtimeLogFile, err)
455+ if info.Mode()&os.ModeSymlink != 0 {
456+ return nil, fmt.Errorf("log path %s must not be a symlink", path)
437457 }
438- return runtimeLogFile, nil
458+ if !info.Mode().IsRegular() {
459+ return nil, fmt.Errorf("log path %s is not a regular file", path)
460+ }
461+ file, openErr := openFileNoFollow(path, syscall.O_WRONLY, 0)
462+ if openErr != nil {
463+ return nil, fmt.Errorf("open %s: %w", path, openErr)
464+ }
465+ return file, nil
466+}
467+
468+func openFileNoFollow(path string, flags int, perm os.FileMode) (*os.File, error) {
469+ fd, err := syscall.Open(path, flags|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, uint32(perm))
470+ if err != nil {
471+ return nil, err
472+ }
473+ return os.NewFile(uintptr(fd), path), nil
439474 }
440475
441476 func prepareCEFInitTraceFile(defaultLogFile, logFile string) (string, error) {
@@ -420,22 +420,57 @@ func prepareCEFLogFile(defaultLogFile, logFile string) (string, error) {
420 if err := os.MkdirAll(filepath.Dir(runtimeLogFile), dirPerm); err != nil {420 if err := os.MkdirAll(filepath.Dir(runtimeLogFile), dirPerm); err != nil {
421 return "", fmt.Errorf("mkdir %s: %w", filepath.Dir(runtimeLogFile), err)421 return "", fmt.Errorf("mkdir %s: %w", filepath.Dir(runtimeLogFile), err)
422 }422 }
423- if _, err := os.Stat(runtimeLogFile); err != nil {423+
424+ file, err := openRegularLogFile(runtimeLogFile)
425+ if err != nil {
426+ return "", err
427+ }
428+ defer func() { _ = file.Close() }()
429+
430+ info, err := file.Stat()
431+ if err != nil {
432+ return "", fmt.Errorf("stat %s: %w", runtimeLogFile, err)
433+ }
434+ if !info.Mode().IsRegular() {
435+ return "", fmt.Errorf("log path %s is not a regular file", runtimeLogFile)
436+ }
437+ if err := file.Chmod(filePerm); err != nil {
438+ return "", fmt.Errorf("chmod %s: %w", runtimeLogFile, err)
439+ }
440+ return runtimeLogFile, nil
441+}
442+
443+func openRegularLogFile(path string) (*os.File, error) {
444+ info, err := os.Lstat(path)
445+ if err != nil {
424 if !errors.Is(err, os.ErrNotExist) {446 if !errors.Is(err, os.ErrNotExist) {
425- return "", fmt.Errorf("stat %s: %w", runtimeLogFile, err)447+ return nil, fmt.Errorf("lstat %s: %w", path, err)
426 }448 }
427- file, createErr := os.OpenFile(runtimeLogFile, os.O_CREATE|os.O_WRONLY, filePerm)449+ file, createErr := openFileNoFollow(path, syscall.O_CREAT|syscall.O_EXCL|syscall.O_WRONLY, filePerm)
428 if createErr != nil {450 if createErr != nil {
429- return "", fmt.Errorf("create %s: %w", runtimeLogFile, createErr)451+ return nil, fmt.Errorf("create %s: %w", path, createErr)
430- }
431- if closeErr := file.Close(); closeErr != nil {
432- return "", fmt.Errorf("close %s: %w", runtimeLogFile, closeErr)
433 }452 }
453+ return file, nil
434 }454 }
435- if err := os.Chmod(runtimeLogFile, filePerm); err != nil {455+ if info.Mode()&os.ModeSymlink != 0 {
436- return "", fmt.Errorf("chmod %s: %w", runtimeLogFile, err)456+ return nil, fmt.Errorf("log path %s must not be a symlink", path)
437 }457 }
438- return runtimeLogFile, nil458+ if !info.Mode().IsRegular() {
459+ return nil, fmt.Errorf("log path %s is not a regular file", path)
460+ }
461+ file, openErr := openFileNoFollow(path, syscall.O_WRONLY, 0)
462+ if openErr != nil {
463+ return nil, fmt.Errorf("open %s: %w", path, openErr)
464+ }
465+ return file, nil
466+}
467+
468+func openFileNoFollow(path string, flags int, perm os.FileMode) (*os.File, error) {
469+ fd, err := syscall.Open(path, flags|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, uint32(perm))
470+ if err != nil {
471+ return nil, err
472+ }
473+ return os.NewFile(uintptr(fd), path), nil
439 }474 }
440 475
441 func prepareCEFInitTraceFile(defaultLogFile, logFile string) (string, error) {476 func prepareCEFInitTraceFile(defaultLogFile, logFile string) (string, error) {
M internal/infrastructure/cef/engine_init_test.go +26 -0
@@ -98,6 +98,32 @@ func TestPrepareCEFLogFile_TightensExistingFilePermissions(t *testing.T) {
9898 require.Equal(t, os.FileMode(filePerm), fileInfo.Mode().Perm())
9999 }
100100
101+func TestPrepareCEFLogFile_RejectsSymlink(t *testing.T) {
102+ target := filepath.Join(t.TempDir(), "target.log")
103+ require.NoError(t, os.WriteFile(target, []byte("existing"), 0o644))
104+
105+ logFile := filepath.Join(t.TempDir(), "private", "cef_runtime.log")
106+ require.NoError(t, os.MkdirAll(filepath.Dir(logFile), dirPerm))
107+ require.NoError(t, os.Symlink(target, logFile))
108+
109+ path, err := prepareCEFLogFile("", logFile)
110+ require.ErrorContains(t, err, "must not be a symlink")
111+ require.Empty(t, path)
112+
113+ targetInfo, statErr := os.Stat(target)
114+ require.NoError(t, statErr)
115+ require.Equal(t, os.FileMode(0o644), targetInfo.Mode().Perm())
116+}
117+
118+func TestPrepareCEFLogFile_RejectsDirectoryPath(t *testing.T) {
119+ logFile := filepath.Join(t.TempDir(), "private", "cef_runtime.log")
120+ require.NoError(t, os.MkdirAll(logFile, dirPerm))
121+
122+ path, err := prepareCEFLogFile("", logFile)
123+ require.ErrorContains(t, err, "not a regular file")
124+ require.Empty(t, path)
125+}
126+
101127 func TestPrepareCEFInitTraceFile_EnabledViaExplicitLogFile(t *testing.T) {
102128 t.Setenv(puregoCEFInitTraceEnvVar, "1")
103129 logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log")
@@ -98,6 +98,32 @@ func TestPrepareCEFLogFile_TightensExistingFilePermissions(t *testing.T) {
98 require.Equal(t, os.FileMode(filePerm), fileInfo.Mode().Perm())98 require.Equal(t, os.FileMode(filePerm), fileInfo.Mode().Perm())
99 }99 }
100 100
101+func TestPrepareCEFLogFile_RejectsSymlink(t *testing.T) {
102+ target := filepath.Join(t.TempDir(), "target.log")
103+ require.NoError(t, os.WriteFile(target, []byte("existing"), 0o644))
104+
105+ logFile := filepath.Join(t.TempDir(), "private", "cef_runtime.log")
106+ require.NoError(t, os.MkdirAll(filepath.Dir(logFile), dirPerm))
107+ require.NoError(t, os.Symlink(target, logFile))
108+
109+ path, err := prepareCEFLogFile("", logFile)
110+ require.ErrorContains(t, err, "must not be a symlink")
111+ require.Empty(t, path)
112+
113+ targetInfo, statErr := os.Stat(target)
114+ require.NoError(t, statErr)
115+ require.Equal(t, os.FileMode(0o644), targetInfo.Mode().Perm())
116+}
117+
118+func TestPrepareCEFLogFile_RejectsDirectoryPath(t *testing.T) {
119+ logFile := filepath.Join(t.TempDir(), "private", "cef_runtime.log")
120+ require.NoError(t, os.MkdirAll(logFile, dirPerm))
121+
122+ path, err := prepareCEFLogFile("", logFile)
123+ require.ErrorContains(t, err, "not a regular file")
124+ require.Empty(t, path)
125+}
126+
101 func TestPrepareCEFInitTraceFile_EnabledViaExplicitLogFile(t *testing.T) {127 func TestPrepareCEFInitTraceFile_EnabledViaExplicitLogFile(t *testing.T) {
102 t.Setenv(puregoCEFInitTraceEnvVar, "1")128 t.Setenv(puregoCEFInitTraceEnvVar, "1")
103 logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log")129 logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log")

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close