From ee9e715243c2cd5e886ef1c6ae7e771c877eaa58 Mon Sep 17 00:00:00 2001 From: Snider Date: Fri, 20 Mar 2026 17:46:47 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20Codex=20review=20round=203=20=E2=80=94?= =?UTF-8?q?=205=20remaining=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Command: allow overwriting auto-created parent placeholders - NewWithFactories: wrap original factory error cause - I18n.SetTranslator: reapply saved locale to new translator - Options/Drive: copy slices on intake (prevent aliasing) - Embed.path: returns Result, rejects traversal with error Co-Authored-By: Virgil --- pkg/core/command.go | 2 +- pkg/core/contract.go | 6 ++++-- pkg/core/drive.go | 4 +++- pkg/core/embed.go | 31 +++++++++++++++++++++++-------- pkg/core/i18n.go | 4 ++++ pkg/core/runtime.go | 3 ++- 6 files changed, 37 insertions(+), 13 deletions(-) diff --git a/pkg/core/command.go b/pkg/core/command.go index 8253b15..da5598f 100644 --- a/pkg/core/command.go +++ b/pkg/core/command.go @@ -144,7 +144,7 @@ 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 { + if existing, exists := c.commands.commands[path]; exists && existing.Action != nil { return Result{E("core.Command", Concat("command \"", path, "\" already registered"), nil), false} } diff --git a/pkg/core/contract.go b/pkg/core/contract.go index dad7f02..2f52e6b 100644 --- a/pkg/core/contract.go +++ b/pkg/core/contract.go @@ -88,8 +88,10 @@ func New(opts ...Options) *Core { } if len(opts) > 0 { - c.options = &opts[0] - name := opts[0].String("name") + cp := make(Options, len(opts[0])) + copy(cp, opts[0]) + c.options = &cp + name := cp.String("name") if name != "" { c.app.Name = name } diff --git a/pkg/core/drive.go b/pkg/core/drive.go index cbe9ac6..e6988c4 100644 --- a/pkg/core/drive.go +++ b/pkg/core/drive.go @@ -62,10 +62,12 @@ func (d *Drive) New(opts Options) Result { d.handles = make(map[string]*DriveHandle) } + cp := make(Options, len(opts)) + copy(cp, opts) handle := &DriveHandle{ Name: name, Transport: transport, - Options: opts, + Options: cp, } d.handles[name] = handle diff --git a/pkg/core/embed.go b/pkg/core/embed.go index 86cb0ba..d960c25 100644 --- a/pkg/core/embed.go +++ b/pkg/core/embed.go @@ -366,13 +366,12 @@ func MountEmbed(efs embed.FS, basedir string) Result { return Mount(efs, basedir) } -func (s *Embed) path(name string) string { +func (s *Embed) path(name string) Result { joined := filepath.ToSlash(filepath.Join(s.basedir, name)) - // Reject traversal outside the base directory if HasPrefix(joined, "..") || Contains(joined, "/../") || HasSuffix(joined, "/..") { - return s.basedir + return Result{E("embed.path", Concat("path traversal rejected: ", name), nil), false} } - return joined + return Result{joined, true} } // Open opens the named file for reading. @@ -380,7 +379,11 @@ func (s *Embed) path(name string) string { // r := emb.Open("test.txt") // if r.OK { file := r.Value.(fs.File) } func (s *Embed) Open(name string) Result { - f, err := s.fsys.Open(s.path(name)) + r := s.path(name) + if !r.OK { + return r + } + f, err := s.fsys.Open(r.Value.(string)) if err != nil { return Result{err, false} } @@ -389,7 +392,11 @@ func (s *Embed) Open(name string) Result { // ReadDir reads the named directory. func (s *Embed) ReadDir(name string) Result { - return Result{}.Result(fs.ReadDir(s.fsys, s.path(name))) + r := s.path(name) + if !r.OK { + return r + } + return Result{}.Result(fs.ReadDir(s.fsys, r.Value.(string))) } // ReadFile reads the named file. @@ -397,7 +404,11 @@ func (s *Embed) ReadDir(name string) Result { // r := emb.ReadFile("test.txt") // if r.OK { data := r.Value.([]byte) } func (s *Embed) ReadFile(name string) Result { - data, err := fs.ReadFile(s.fsys, s.path(name)) + r := s.path(name) + if !r.OK { + return r + } + data, err := fs.ReadFile(s.fsys, r.Value.(string)) if err != nil { return Result{err, false} } @@ -421,7 +432,11 @@ func (s *Embed) ReadString(name string) Result { // r := emb.Sub("testdata") // if r.OK { sub := r.Value.(*Embed) } func (s *Embed) Sub(subDir string) Result { - sub, err := fs.Sub(s.fsys, s.path(subDir)) + r := s.path(subDir) + if !r.OK { + return r + } + sub, err := fs.Sub(s.fsys, r.Value.(string)) if err != nil { return Result{err, false} } diff --git a/pkg/core/i18n.go b/pkg/core/i18n.go index c2a88ed..7061ce8 100644 --- a/pkg/core/i18n.go +++ b/pkg/core/i18n.go @@ -69,7 +69,11 @@ func (i *I18n) Locales() Result { func (i *I18n) SetTranslator(t Translator) { i.mu.Lock() i.translator = t + locale := i.locale i.mu.Unlock() + if t != nil && locale != "" { + _ = t.SetLanguage(locale) + } } // Translator returns the registered translation implementation, or nil. diff --git a/pkg/core/runtime.go b/pkg/core/runtime.go index 9ce010e..fa2d054 100644 --- a/pkg/core/runtime.go +++ b/pkg/core/runtime.go @@ -112,7 +112,8 @@ func NewWithFactories(app any, factories map[string]ServiceFactory) Result { } r := factory() if !r.OK { - return Result{E("core.NewWithFactories", Concat("factory \"", name, "\" failed"), nil), false} + cause, _ := r.Value.(error) + return Result{E("core.NewWithFactories", Concat("factory \"", name, "\" failed"), cause), false} } svc, ok := r.Value.(Service) if !ok {