fix: validate Default and roll back SetPath on later failure

Three review-driven hardenings rolled into one commit:

1. config.EnsureExists now Validate()s Default() before writing.
   TestDefaultPassesValidate already locks the happy path, but a
   future regression should fail EnsureExists rather than persist
   garbage that Load would later reject.

2. simulator.New captures the package-level path with config.Path()
   before the SetPath, then defers a rollback that fires unless a
   'committed' flag flips at the end. Now Load() and rebuildAuthChain()
   failures restore the previous global path instead of leaving the
   constructor's mutation behind.

3. Tests:
   - newTestSimulator captures config.Path() into priorPath and the
     cleanup restores that value (instead of unconditionally setting
     '') so a parent test's path is preserved across nested fixtures.
   - TestNewDoesNotLeakActivePathOnEnsureFailure now installs a
     non-empty sentinel as the prior value so the assertion exercises
     the preserve-existing-path behaviour, not just preserve-empty.
   - New TestNewDoesNotLeakActivePathOnLoadFailure covers the second
     leak window — EnsureExists succeeds (file exists) but Load
     fails because the existing file is invalid JSON. The deferred
     rollback in New must restore the sentinel.

Author: GyeongHoKim

internal/config/config.go

@@ -961,6 +961,14 @@ func EnsureExists(p string) (bool, error) {
 		return false, fmt.Errorf("config: stat %s: %w", p, err)
 	}
 	cfg := Default()
+	// Belt-and-braces: Validate Default() before persisting so a future
+	// regression in the baseline can never write a file that Load would
+	// immediately reject. TestDefaultPassesValidate locks the happy path,
+	// but this guard turns any drift into an EnsureExists error instead
+	// of a corrupted on-disk state.
+	if err := Validate(&cfg); err != nil {
+		return false, fmt.Errorf("config: default config failed validation: %w", err)
+	}
 	tmpPath, err := writeTempFile(p, &cfg)
 	if err != nil {
 		return false, err

internal/simulator/simulator.go

@@ -159,7 +159,17 @@ func New(opts Options) (*Simulator, error) {
 	if _, ensureErr := config.EnsureExists(cfgPath); ensureErr != nil {
 		return nil, fmt.Errorf("simulator: ensure config: %w", ensureErr)
 	}
+	prevPath := config.Path()
 	config.SetPath(cfgPath)
+	// Roll the global active path back if any subsequent step (Load,
+	// rebuildAuthChain, …) errors out. The flag flips to true at the
+	// very end of New, after every fallible step has succeeded.
+	committed := false
+	defer func() {
+		if !committed {
+			config.SetPath(prevPath)
+		}
+	}()
 
 	cfg, err := config.Load()
 	if err != nil {
@@ -192,6 +202,7 @@ func New(opts Options) (*Simulator, error) {
 	if err := sim.rebuildAuthChain(&cfg); err != nil {
 		return nil, fmt.Errorf("simulator: build auth chain: %w", err)
 	}
+	committed = true
 
 	sim.devHandler = devicesvc.NewHandler(sim.deviceProv, devicesvc.WithAuthHook(
 		devicesvc.AuthFunc(sim.deviceAuthHook),

internal/simulator/simulator_test.go

@@ -73,7 +73,10 @@ func newTestSimulator(t *testing.T) (sim *Simulator, cleanup func()) {
 		t.Fatalf("write config: %v", writeErr)
 	}
 
-	cleanup = func() { config.SetPath("") }
+	// Capture the active path before construction so cleanup restores
+	// whatever was set globally (instead of clobbering it with "").
+	priorPath := config.Path()
+	cleanup = func() { config.SetPath(priorPath) }
 
 	s, newErr := New(Options{EventBufferSize: 16, ConfigPath: cfgPath})
 	if newErr != nil {
@@ -109,18 +112,48 @@ func TestNewDoesNotLeakActivePathOnEnsureFailure(t *testing.T) {
 	// path under a regular file → MkdirAll inside writeTempFile fails.
 	bogus := filepath.Join(blocker, config.FileName)
 
-	// Reset to a clean baseline before the test, snapshot it, and
-	// restore on cleanup so test ordering doesn't matter.
-	config.SetPath("")
-	t.Cleanup(func() { config.SetPath("") })
-	prior := config.Path()
+	// Snapshot whatever was set before the test, install a non-empty
+	// sentinel as the "prior" we expect to be preserved, and restore
+	// the original value on cleanup so test ordering does not matter.
+	original := config.Path()
+	t.Cleanup(func() { config.SetPath(original) })
+	const sentinel = "/some/sentinel/onvif-simulator.json"
+	config.SetPath(sentinel)
 
 	_, err := New(Options{ConfigPath: bogus})
 	if err == nil {
 		t.Fatal("expected New to fail with bogus path")
 	}
-	if got := config.Path(); got != prior {
-		t.Errorf("config.Path leaked after failed New: got %q, want %q", got, prior)
+	if got := config.Path(); got != sentinel {
+		t.Errorf("config.Path leaked after failed New: got %q, want %q", got, sentinel)
+	}
+}
+
+// TestNewDoesNotLeakActivePathOnLoadFailure complements the
+// EnsureExists case: even after EnsureExists succeeds and SetPath has
+// fired, a Load failure (e.g. a corrupted config file written by a
+// concurrent editor) must roll the global path back to its prior
+// value via the deferred restore in New.
+func TestNewDoesNotLeakActivePathOnLoadFailure(t *testing.T) {
+	dir := t.TempDir()
+	cfgPath := filepath.Join(dir, config.FileName)
+	// Seed a file that EnsureExists treats as already-present (no-op)
+	// but Load rejects because it is not valid JSON.
+	if err := os.WriteFile(cfgPath, []byte("not json"), 0o600); err != nil {
+		t.Fatalf("seed corrupted config: %v", err)
+	}
+
+	original := config.Path()
+	t.Cleanup(func() { config.SetPath(original) })
+	const sentinel = "/some/sentinel/onvif-simulator.json"
+	config.SetPath(sentinel)
+
+	_, err := New(Options{ConfigPath: cfgPath})
+	if err == nil {
+		t.Fatal("expected New to fail when config file is corrupt")
+	}
+	if got := config.Path(); got != sentinel {
+		t.Errorf("config.Path leaked after Load failure: got %q, want %q", got, sentinel)
 	}
 }