fix: Codex review round 2 — path traversal, shutdown order, races

High:
- embed.Extract: safePath validates all rendered paths stay under targetDir
- embed.path: reject .. traversal on arbitrary fs.FS
- ServiceShutdown: drain background tasks BEFORE stopping services

Medium:
- cli.Run: command lookup holds registry RLock (race fix)
- NewWithFactories: propagate factory/registration failures

Co-Authored-By: Virgil <virgil@lethean.io>

pkg/core/cli.go

@@ -50,7 +50,18 @@ func (cl *Cli) Run(args ...string) Result {
 
 	clean := FilterArgs(args)
 
-	if cl.core == nil || cl.core.commands == nil || len(cl.core.commands.commands) == 0 {
+	if cl.core == nil || cl.core.commands == nil {
+		if cl.banner != nil {
+			cl.Print(cl.banner(cl))
+		}
+		return Result{}
+	}
+
+	cl.core.commands.mu.RLock()
+	cmdCount := len(cl.core.commands.commands)
+	cl.core.commands.mu.RUnlock()
+
+	if cmdCount == 0 {
 		if cl.banner != nil {
 			cl.Print(cl.banner(cl))
 		}
@@ -61,6 +72,7 @@ func (cl *Cli) Run(args ...string) Result {
 	var cmd *Command
 	var remaining []string
 
+	cl.core.commands.mu.RLock()
 	for i := len(clean); i > 0; i-- {
 		path := JoinPath(clean[:i]...)
 		if c, ok := cl.core.commands.commands[path]; ok {
@@ -69,6 +81,7 @@ func (cl *Cli) Run(args ...string) Result {
 			break
 		}
 	}
+	cl.core.commands.mu.RUnlock()
 
 	if cmd == nil {
 		if cl.banner != nil {

pkg/core/embed.go

@@ -367,7 +367,12 @@ func MountEmbed(efs embed.FS, basedir string) Result {
 }
 
 func (s *Embed) path(name string) string {
-	return filepath.ToSlash(filepath.Join(s.basedir, name))
+	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 joined
 }
 
 // Open opens the named file for reading.
@@ -526,9 +531,24 @@ func Extract(fsys fs.FS, targetDir string, data any, opts ...ExtractOptions) Res
 		return Result{err, false}
 	}
 
+	// safePath ensures a rendered path stays under targetDir.
+	safePath := func(rendered string) (string, error) {
+		abs, err := filepath.Abs(rendered)
+		if err != nil {
+			return "", err
+		}
+		if !HasPrefix(abs, targetDir+string(filepath.Separator)) && abs != targetDir {
+			return "", E("embed.Extract", Concat("path escapes target: ", abs), nil)
+		}
+		return abs, nil
+	}
+
 	// Create directories (names may contain templates)
 	for _, dir := range dirs {
-		target := renderPath(filepath.Join(targetDir, dir), data)
+		target, err := safePath(renderPath(filepath.Join(targetDir, dir), data))
+		if err != nil {
+			return Result{err, false}
+		}
 		if err := os.MkdirAll(target, 0755); err != nil {
 			return Result{err, false}
 		}
@@ -552,7 +572,10 @@ func Extract(fsys fs.FS, targetDir string, data any, opts ...ExtractOptions) Res
 		if renamed := opt.RenameFiles[name]; renamed != "" {
 			name = renamed
 		}
-		targetFile = filepath.Join(dir, name)
+		targetFile, err = safePath(filepath.Join(dir, name))
+		if err != nil {
+			return Result{err, false}
+		}
 
 		f, err := os.Create(targetFile)
 		if err != nil {
@@ -572,7 +595,10 @@ func Extract(fsys fs.FS, targetDir string, data any, opts ...ExtractOptions) Res
 		if renamed := opt.RenameFiles[name]; renamed != "" {
 			targetPath = filepath.Join(filepath.Dir(path), renamed)
 		}
-		target := renderPath(filepath.Join(targetDir, targetPath), data)
+		target, err := safePath(renderPath(filepath.Join(targetDir, targetPath), data))
+		if err != nil {
+			return Result{err, false}
+		}
 		if err := copyFile(fsys, path, target); err != nil {
 			return Result{err, false}
 		}

pkg/core/runtime.go

@@ -49,10 +49,24 @@ func (c *Core) ServiceStartup(ctx context.Context, options any) Result {
 	return Result{OK: true}
 }
 
-// ServiceShutdown runs OnStop for all registered services that have one.
+// ServiceShutdown drains background tasks, then stops all registered services.
 func (c *Core) ServiceShutdown(ctx context.Context) Result {
 	c.shutdown.Store(true)
 	c.ACTION(ActionServiceShutdown{})
+
+	// Drain background tasks before stopping services
+	done := make(chan struct{})
+	go func() {
+		c.wg.Wait()
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-ctx.Done():
+		return Result{ctx.Err(), false}
+	}
+
+	// Stop services
 	var firstErr error
 	stoppables := c.Stoppables()
 	if stoppables.OK {
@@ -68,16 +82,6 @@ func (c *Core) ServiceShutdown(ctx context.Context) Result {
 			}
 		}
 	}
-	done := make(chan struct{})
-	go func() {
-		c.wg.Wait()
-		close(done)
-	}()
-	select {
-	case <-done:
-	case <-ctx.Done():
-		return Result{ctx.Err(), false}
-	}
 	if firstErr != nil {
 		return Result{firstErr, false}
 	}
@@ -108,10 +112,15 @@ func NewWithFactories(app any, factories map[string]ServiceFactory) Result {
 		}
 		r := factory()
 		if !r.OK {
-			continue
+			return Result{E("core.NewWithFactories", Concat("factory \"", name, "\" failed"), nil), false}
 		}
-		if svc, ok := r.Value.(Service); ok {
-			c.Service(name, svc)
+		svc, ok := r.Value.(Service)
+		if !ok {
+			return Result{E("core.NewWithFactories", Concat("factory \"", name, "\" returned non-Service type"), nil), false}
+		}
+		sr := c.Service(name, svc)
+		if !sr.OK {
+			return sr
 		}
 	}
 	return Result{&Runtime{app: app, Core: c}, true}