fix(security): scrub startup secrets before any child can spawn

The scrub ran in main after server.New returned, but New resumes every
non-terminal task before returning, and those loops start workers that run
shells. Move the scrub into New, right after both environment readers, and
start MCP discovery after it as well: both paths spawn child processes.

New takes an option rather than scrubbing unconditionally. The test suite
builds several servers in one process and still needs ARTEX_PG_DSN on later
calls, so only the binary opts in.

cmd/artex/main.go

@@ -93,8 +93,7 @@ func run() int {
 		skillDir = abs
 	}
 	log.Printf("[config] skill directory: %s", skillDir)
-	srv := server.New(ctx, mgr, skillDir, *dataDir, config.BaseDir())
-	scrubSecretEnv()
+	srv := server.New(ctx, mgr, skillDir, *dataDir, config.BaseDir(), server.WithSecretScrub())
 	httpSrv := &http.Server{
 		Addr:              *addr,
 		Handler:           srv.Handler(),
@@ -118,19 +117,6 @@ func run() int {
 	return 0
 }
 
-// secretEnv lists the credentials read from the environment only at startup:
-// ARTEX_PG_DSN by server.NewManager, the API keys by agent.FromEnv in server.New.
-var secretEnv = []string{"ARTEX_PG_DSN", "ANTHROPIC_API_KEY", "OPENAI_API_KEY"}
-
-// scrubSecretEnv removes secretEnv from the process environment once startup has
-// read it. Child processes (agent shells, custom tools, MCP stdio servers) copy
-// os.Environ() when they are spawned, so they no longer inherit these values.
-func scrubSecretEnv() {
-	for _, k := range secretEnv {
-		_ = os.Unsetenv(k)
-	}
-}
-
 // shutdownContext deliberately does not derive from signalCtx. If it did, the
 // parent's plain context.Canceled could win the race before AbortShutdown was
 // attached to the child, losing the diagnostic cause in every running Agent.

cmd/artex/main_test.go

@@ -2,42 +2,13 @@ package main
 
 import (
 	"context"
-	"os/exec"
-	"strings"
 	"testing"
 
 	"github.com/Autumn-27/artex/agent"
 )
 
-// A child process spawned after startup must not see the startup secrets, while
-// other variables still pass through.
-func TestScrubSecretEnvHidesSecretsFromChildren(t *testing.T) {
-	sh, err := exec.LookPath("sh")
-	if err != nil {
-		t.Skip("no sh in PATH")
-	}
-	for _, k := range secretEnv {
-		t.Setenv(k, "leak-"+k)
-	}
-	t.Setenv("ARTEX_SCRUB_KEEP", "kept")
-
-	scrubSecretEnv()
-
-	out, err := exec.Command(sh, "-c", "env").Output()
-	if err != nil {
-		t.Fatal(err)
-	}
-	env := "\n" + string(out)
-	for _, k := range secretEnv {
-		if strings.Contains(env, "\n"+k+"=") {
-			t.Errorf("%s still reaches child processes", k)
-		}
-	}
-	if !strings.Contains(env, "\nARTEX_SCRUB_KEEP=kept\n") {
-		t.Errorf("non-secret variable did not reach the child process")
-	}
-}
-
+// The shutdown cause must survive the parent context being cancelled first;
+// losing it would drop the diagnostic on every graceful stop.
 func TestShutdownContextPreservesNamedCause(t *testing.T) {
 	signalCtx, signalCancel := context.WithCancel(context.Background())
 	ctx, shutdown := shutdownContext(signalCtx)

server/env_scrub.go

@@ -0,0 +1,30 @@
+package server
+
+import "os"
+
+// secretEnv lists the credentials read from the environment only at startup:
+// ARTEX_PG_DSN by NewManager, the API keys by agent.FromEnv inside New.
+var secretEnv = []string{"ARTEX_PG_DSN", "ANTHROPIC_API_KEY", "OPENAI_API_KEY"}
+
+// ScrubSecretEnv removes secretEnv from the process environment once startup has
+// read it. Child processes (agent shells, custom tools, MCP stdio servers) copy
+// os.Environ() when they are spawned, so they no longer inherit these values.
+//
+// It runs inside New, after both readers and before the MCP discovery goroutine
+// and the task resume loop: both can spawn children, and resuming a task starts
+// workers that run shells. Scrubbing in main after New returned left that window
+// open.
+func ScrubSecretEnv() {
+	for _, k := range secretEnv {
+		_ = os.Unsetenv(k)
+	}
+}
+
+// Option configures New. Options keep the test call sites unchanged: only the
+// binary opts into scrubbing, because tests create several servers in one
+// process and still need ARTEX_PG_DSN on later calls.
+type Option func(*Server)
+
+// WithSecretScrub makes New drop the startup secrets after reading them, before
+// the MCP discovery goroutine and the task resume loop can spawn children.
+func WithSecretScrub() Option { return func(s *Server) { s.scrubSecrets = true } }

server/env_scrub_test.go

@@ -0,0 +1,36 @@
+package server
+
+import (
+	"os/exec"
+	"strings"
+	"testing"
+)
+
+// A child process spawned after startup must not see the startup secrets, while
+// other variables still pass through.
+func TestScrubSecretEnvHidesSecretsFromChildren(t *testing.T) {
+	sh, err := exec.LookPath("sh")
+	if err != nil {
+		t.Skip("no sh in PATH")
+	}
+	for _, k := range secretEnv {
+		t.Setenv(k, "leak-"+k)
+	}
+	t.Setenv("ARTEX_SCRUB_KEEP", "kept")
+
+	ScrubSecretEnv()
+
+	out, err := exec.Command(sh, "-c", "env").Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+	env := "\n" + string(out)
+	for _, k := range secretEnv {
+		if strings.Contains(env, "\n"+k+"=") {
+			t.Errorf("%s still reaches child processes", k)
+		}
+	}
+	if !strings.Contains(env, "\nARTEX_SCRUB_KEEP=kept\n") {
+		t.Errorf("non-secret variable did not reach the child process")
+	}
+}

server/server.go

@@ -46,6 +46,8 @@ type Server struct {
 	skillDir string // root directory for skill subdirectories
 	jwtKey   []byte // HS256 signing key loaded from / generated into dataDir/jwt.key
 
+	scrubSecrets bool // drop ARTEX_PG_DSN and the API keys from the environment once read (binary only)
+
 	// concMu serializes concurrency-cap decisions (admission + reconcile) so a
 	// scheduler tick and an HTTP settings change / task creation can't both count
 	// free slots off the same snapshot and over-promote past the limit.
@@ -140,7 +142,7 @@ type triggeredRun struct {
 	mergeable bool   // true for finding/goal event triggers (merge by taskID)
 }
 
-func New(ctx context.Context, m *Manager, skillDir string, dataDir string, keyDir string) *Server {
+func New(ctx context.Context, m *Manager, skillDir string, dataDir string, keyDir string, opts ...Option) *Server {
 	key, err := loadOrCreateJWTKey(keyDir, dataDir)
 	if err != nil {
 		log.Fatalf("[auth] JWT key: %v", err)
@@ -232,9 +234,6 @@ func New(ctx context.Context, m *Manager, skillDir string, dataDir string, keyDi
 		// Vulnerability IM push delivery engine. Sits beside the Scheduler but independent: push latency demands (3s)
 		// differ from the triggers' business cadence, and neither's failures drag the other down — a stuck push must not affect agent triggers.
 		go newNotifier(s).Run(s.ctx)
-		// Fill the tool cache for any enabled MCP that has none yet (notably the
-		// seeded browser MCP on first run). Async so it never blocks startup.
-		go s.discoverEmptyMCPsOnStartup()
 		logSink.SetDB(ctx, m.pg) // restore last 100 log rows and enable async persistence
 	}
 	// precedence: persisted DB config > env.
@@ -253,6 +252,16 @@ func New(ctx context.Context, m *Manager, skillDir string, dataDir string, keyDi
 	} else {
 		log.Printf("[engine] no LLM provider configured — engine idle until set via /api/llm or env")
 	}
+	// Both env readers have now run (NewManager for the DSN, FromEnv above for the
+	// keys). Drop the secrets before anything below can spawn a child process:
+	// restoring runtimes resumes workers that run shells, and MCP discovery starts
+	// stdio servers.
+	if s.scrubSecrets {
+		ScrubSecretEnv()
+	}
+	// Fill the tool cache for any enabled MCP that has none yet (notably the
+	// seeded browser MCP on first run). Async so it never blocks startup.
+	go s.discoverEmptyMCPsOnStartup()
 	s.restoreTaskRuntimes()
 	go s.reconcileConcurrency()
 	s.startTaskArchiveWorker()