Harden environment file manager path handling
Some checks are pending
Security Scan / security (push) Waiting to run
Test / test (push) Waiting to run

This commit is contained in:
Snider 2026-04-17 19:49:52 +01:00
parent fb43e9a729
commit 7301932257
3 changed files with 39 additions and 2 deletions

View file

@ -0,0 +1 @@
- @security pkg/display/display.go:954 — WebSocket layout commands still coerce missing or malformed numeric fields to zero instead of rejecting the request.

View file

@ -3,6 +3,7 @@ package environment
import (
"context"
"path/filepath"
"strings"
"sync"
@ -42,7 +43,11 @@ func (s *Service) OnStartup(_ context.Context) core.Result {
})
s.Core().Action("environment.openFileManager", func(_ context.Context, opts core.Options) core.Result {
t, _ := opts.Get("task").Value.(TaskOpenFileManager)
if err := s.platform.OpenFileManager(t.Path, t.Select); err != nil {
path, err := validatedOpenFileManagerPath(t.Path)
if err != nil {
return core.Result{Value: err, OK: false}
}
if err := s.platform.OpenFileManager(path, t.Select); err != nil {
return core.Result{Value: err, OK: false}
}
return core.Result{OK: true}
@ -160,6 +165,21 @@ func normalizeTheme(theme string) (string, error) {
}
}
func validatedOpenFileManagerPath(raw string) (string, error) {
trimmed := strings.TrimSpace(raw)
if trimmed == "" {
return "", coreerr.E("environment.openFileManager", "path is required", nil)
}
if strings.ContainsRune(trimmed, '\x00') {
return "", coreerr.E("environment.openFileManager", "path contains a null byte", nil)
}
cleaned := filepath.Clean(trimmed)
if !filepath.IsAbs(cleaned) {
return "", coreerr.E("environment.openFileManager", "path must be absolute", nil)
}
return cleaned, nil
}
func themeName(isDark bool) string {
if isDark {
return "dark"

View file

@ -3,6 +3,7 @@ package environment
import (
"context"
"path/filepath"
"sync"
"testing"
@ -17,6 +18,8 @@ type mockPlatform struct {
info EnvironmentInfo
accentColour string
openFMErr error
openFMPath string
openFMSelect bool
focusFollowsMouse bool
themeHandler func(isDark bool)
mu sync.Mutex
@ -27,6 +30,8 @@ func (m *mockPlatform) Info() EnvironmentInfo { return m.info }
func (m *mockPlatform) AccentColour() string { return m.accentColour }
func (m *mockPlatform) HasFocusFollowsMouse() bool { return m.focusFollowsMouse }
func (m *mockPlatform) OpenFileManager(path string, selectFile bool) error {
m.openFMPath = path
m.openFMSelect = selectFile
return m.openFMErr
}
func (m *mockPlatform) OnThemeChange(handler func(isDark bool)) func() {
@ -100,11 +105,22 @@ func TestQueryAccentColour_Good(t *testing.T) {
}
func TestTaskOpenFileManager_Good(t *testing.T) {
_, c := newTestService(t)
mock, c := newTestService(t)
r := c.Action("environment.openFileManager").Run(context.Background(), core.NewOptions(
core.Option{Key: "task", Value: TaskOpenFileManager{Path: "/tmp", Select: true}},
))
require.True(t, r.OK)
assert.Equal(t, filepath.Clean("/tmp"), mock.openFMPath)
assert.True(t, mock.openFMSelect)
}
func TestTaskOpenFileManager_Bad_InvalidPath(t *testing.T) {
_, c := newTestService(t)
r := c.Action("environment.openFileManager").Run(context.Background(), core.NewOptions(
core.Option{Key: "task", Value: TaskOpenFileManager{Path: "../tmp", Select: false}},
))
assert.False(t, r.OK)
assert.Contains(t, r.Value.(error).Error(), "path must be absolute")
}
func TestThemeChange_ActionBroadcast_Good(t *testing.T) {