fix: Codex review — medium/low severity issues resolved

Medium:
- Config zero-value safe (nil ConfigOptions guards on all mutators)
- ServiceShutdown collects and returns first OnStop error
- Default logger uses atomic.Pointer (race fix)
- Command registration rejects duplicates (like Service)

Low:
- Array.AsSlice returns copy, not backing slice
- fs.validatePath constructs error on sandbox escape (was nil)

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-20 17:25:12 +00:00
parent f1bd36db2e
commit 4c3a671b48
8 changed files with 59 additions and 18 deletions

View file

@ -90,7 +90,12 @@ func (s *Array[T]) Clear() {
s.items = nil
}
// AsSlice returns the underlying slice.
// AsSlice returns a copy of the underlying slice.
func (s *Array[T]) AsSlice() []T {
return s.items
if s.items == nil {
return nil
}
out := make([]T, len(s.items))
copy(out, s.items)
return out
}

View file

@ -144,6 +144,10 @@ func (c *Core) Command(path string, command ...Command) Result {
c.commands.mu.Lock()
defer c.commands.mu.Unlock()
if _, exists := c.commands.commands[path]; exists {
return Result{E("core.Command", Concat("command \"", path, "\" already registered"), nil), false}
}
cmd := &command[0]
cmd.Name = pathName(path)
cmd.Path = path

View file

@ -51,6 +51,9 @@ type Config struct {
// Set stores a configuration value by key.
func (e *Config) Set(key string, val any) {
e.mu.Lock()
if e.ConfigOptions == nil {
e.ConfigOptions = &ConfigOptions{}
}
e.ConfigOptions.init()
e.Settings[key] = val
e.mu.Unlock()
@ -89,6 +92,9 @@ func ConfigGet[T any](e *Config, key string) T {
func (e *Config) Enable(feature string) {
e.mu.Lock()
if e.ConfigOptions == nil {
e.ConfigOptions = &ConfigOptions{}
}
e.ConfigOptions.init()
e.Features[feature] = true
e.mu.Unlock()
@ -96,6 +102,9 @@ func (e *Config) Enable(feature string) {
func (e *Config) Disable(feature string) {
e.mu.Lock()
if e.ConfigOptions == nil {
e.ConfigOptions = &ConfigOptions{}
}
e.ConfigOptions.init()
e.Features[feature] = false
e.mu.Unlock()
@ -103,14 +112,19 @@ func (e *Config) Disable(feature string) {
func (e *Config) Enabled(feature string) bool {
e.mu.RLock()
v := e.Features[feature]
e.mu.RUnlock()
return v
defer e.mu.RUnlock()
if e.ConfigOptions == nil || e.Features == nil {
return false
}
return e.Features[feature]
}
func (e *Config) EnabledFeatures() []string {
e.mu.RLock()
defer e.mu.RUnlock()
if e.ConfigOptions == nil || e.Features == nil {
return nil
}
var result []string
for k, v := range e.Features {
if v {

View file

@ -79,7 +79,7 @@ func New(opts ...Options) *Core {
fs: &Fs{root: "/"},
config: &Config{ConfigOptions: &ConfigOptions{}},
error: &ErrorPanic{},
log: &ErrorLog{log: defaultLog},
log: &ErrorLog{log: Default()},
lock: &Lock{},
ipc: &Ipc{},
i18n: &I18n{},

View file

@ -244,7 +244,7 @@ func (el *ErrorLog) logger() *Log {
if el.log != nil {
return el.log
}
return defaultLog
return Default()
}
// Error logs at Error level and returns a Result with the wrapped error.

View file

@ -79,6 +79,9 @@ func (m *Fs) validatePath(p string) Result {
}
Print(os.Stderr, "[%s] SECURITY sandbox escape detected root=%s path=%s attempted=%s user=%s",
time.Now().Format(time.RFC3339), m.root, p, realNext, username)
if err == nil {
err = E("fs.validatePath", Concat("sandbox escape: ", p, " resolves outside ", m.root), nil)
}
return Result{err, false}
}
current = realNext

View file

@ -11,6 +11,7 @@ import (
"os/user"
"slices"
"sync"
"sync/atomic"
"time"
)
@ -296,51 +297,56 @@ func Username() string {
// --- Default logger ---
var defaultLog = NewLog(LogOptions{Level: LevelInfo})
var defaultLogPtr atomic.Pointer[Log]
func init() {
l := NewLog(LogOptions{Level: LevelInfo})
defaultLogPtr.Store(l)
}
// Default returns the default logger.
func Default() *Log {
return defaultLog
return defaultLogPtr.Load()
}
// SetDefault sets the default logger.
func SetDefault(l *Log) {
defaultLog = l
defaultLogPtr.Store(l)
}
// SetLevel sets the default logger's level.
func SetLevel(level Level) {
defaultLog.SetLevel(level)
Default().SetLevel(level)
}
// SetRedactKeys sets the default logger's redaction keys.
func SetRedactKeys(keys ...string) {
defaultLog.SetRedactKeys(keys...)
Default().SetRedactKeys(keys...)
}
// Debug logs to the default logger.
func Debug(msg string, keyvals ...any) {
defaultLog.Debug(msg, keyvals...)
Default().Debug(msg, keyvals...)
}
// Info logs to the default logger.
func Info(msg string, keyvals ...any) {
defaultLog.Info(msg, keyvals...)
Default().Info(msg, keyvals...)
}
// Warn logs to the default logger.
func Warn(msg string, keyvals ...any) {
defaultLog.Warn(msg, keyvals...)
Default().Warn(msg, keyvals...)
}
// Error logs to the default logger.
func Error(msg string, keyvals ...any) {
defaultLog.Error(msg, keyvals...)
Default().Error(msg, keyvals...)
}
// Security logs to the default logger.
func Security(msg string, keyvals ...any) {
defaultLog.Security(msg, keyvals...)
Default().Security(msg, keyvals...)
}
// --- LogErr: Error-Aware Logger ---

View file

@ -53,13 +53,19 @@ func (c *Core) ServiceStartup(ctx context.Context, options any) Result {
func (c *Core) ServiceShutdown(ctx context.Context) Result {
c.shutdown.Store(true)
c.ACTION(ActionServiceShutdown{})
var firstErr error
stoppables := c.Stoppables()
if stoppables.OK {
for _, s := range stoppables.Value.([]*Service) {
if err := ctx.Err(); err != nil {
return Result{err, false}
}
s.OnStop()
r := s.OnStop()
if !r.OK && firstErr == nil {
if e, ok := r.Value.(error); ok {
firstErr = e
}
}
}
}
done := make(chan struct{})
@ -72,6 +78,9 @@ func (c *Core) ServiceShutdown(ctx context.Context) Result {
case <-ctx.Done():
return Result{ctx.Err(), false}
}
if firstErr != nil {
return Result{firstErr, false}
}
return Result{OK: true}
}