Delete Windows sub-agent API-key temp file after use

writeKeyToUnlinkedFile now returns a cleanup callback that removes the
0600 temp file on Windows after the child has exited and the parent's
handle is closed. POSIX behaviour is unchanged: the file is unlinked at
creation and cleanup is a no-op.

Author: jkyberneees

cmd/odek/subagent_key.go

@@ -14,28 +14,39 @@ import (
 // cmd.ExtraFiles without ever leaving a readable file on disk.
 //
 // On Windows we cannot unlink an open file, so we fall back to a
-// 0600 file in the user's TempDir; the caller deletes it after Start.
-// (Windows is a degraded path; the env-passing concern is largely a
-// Unix /proc problem anyway.)
-func writeKeyToUnlinkedFile(key string) (*os.File, error) {
+// 0600 file in the user's TempDir. The returned cleanup function must be
+// called after the child has exited and the file has been closed; on
+// Windows it deletes the temp file, and on POSIX it is a no-op (the file
+// was already unlinked).
+func writeKeyToUnlinkedFile(key string) (*os.File, func(), error) {
 	f, err := os.CreateTemp("", "odek-key-*")
 	if err != nil {
-		return nil, fmt.Errorf("create temp: %w", err)
+		return nil, nil, fmt.Errorf("create temp: %w", err)
 	}
+
+	cleanup := func() {
+		// POSIX: file was already unlinked, nothing to do.
+		// Windows: delete the temp file after the child has exited and
+		// the parent's handle has been closed.
+		if runtime.GOOS == "windows" {
+			_ = os.Remove(f.Name())
+		}
+	}
+
 	if err := f.Chmod(0600); err != nil {
 		f.Close()
 		os.Remove(f.Name())
-		return nil, fmt.Errorf("chmod: %w", err)
+		return nil, nil, fmt.Errorf("chmod: %w", err)
 	}
 	if _, err := f.WriteString(key); err != nil {
 		f.Close()
 		os.Remove(f.Name())
-		return nil, fmt.Errorf("write: %w", err)
+		return nil, nil, fmt.Errorf("write: %w", err)
 	}
 	if _, err := f.Seek(0, io.SeekStart); err != nil {
 		f.Close()
 		os.Remove(f.Name())
-		return nil, fmt.Errorf("seek: %w", err)
+		return nil, nil, fmt.Errorf("seek: %w", err)
 	}
 	// Unlink immediately on POSIX. The open FD keeps the inode alive
 	// for the parent and the soon-to-be-forked child.
@@ -46,7 +57,7 @@ func writeKeyToUnlinkedFile(key string) (*os.File, error) {
 			fmt.Fprintf(os.Stderr, "odek: warning: could not unlink key tempfile: %v\n", err)
 		}
 	}
-	return f, nil
+	return f, cleanup, nil
 }
 
 // keyFDEnvVar is the signal env var the parent sets when it passes an

cmd/odek/subagent_key_branches_test.go

@@ -53,14 +53,18 @@ func TestReadKeyFromInheritedFD_UnsetsEnvVarBeforeRead(t *testing.T) {
 // degenerate but valid case of an empty key — the function should still
 // hand back a readable FD (containing zero bytes) and unlink the file.
 func TestWriteKeyToUnlinkedFile_EmptyKeyStillProducesValidFD(t *testing.T) {
-	f, err := writeKeyToUnlinkedFile("")
+	f, cleanup, err := writeKeyToUnlinkedFile("")
 	if err != nil {
 		t.Fatalf("writeKeyToUnlinkedFile(\"\"): %v", err)
 	}
-	defer f.Close()
 	buf := make([]byte, 16)
 	n, _ := f.Read(buf)
 	if n != 0 {
+		f.Close()
+		cleanup()
 		t.Errorf("expected 0 bytes for empty key, got %d (%q)", n, string(buf[:n]))
+		return
 	}
+	f.Close()
+	cleanup()
 }

cmd/odek/subagent_key_test.go

@@ -14,25 +14,43 @@ import (
 // the unlink behaviour.
 func TestWriteKeyToUnlinkedFile_RoundtripsThroughFD(t *testing.T) {
 	const want = "sk-test-1234567890"
-	f, err := writeKeyToUnlinkedFile(want)
+	f, cleanup, err := writeKeyToUnlinkedFile(want)
 	if err != nil {
 		t.Fatalf("writeKeyToUnlinkedFile: %v", err)
 	}
-	defer f.Close()
 
 	got, err := io.ReadAll(f)
 	if err != nil {
+		f.Close()
+		cleanup()
 		t.Fatalf("read: %v", err)
 	}
 	if string(got) != want {
+		f.Close()
+		cleanup()
 		t.Errorf("read = %q, want %q", string(got), want)
+		return
 	}
 
 	// On POSIX, the file must already be unlinked — the inode lives on
 	// only because we hold the FD. Verify by stat'ing the path.
 	if runtime.GOOS != "windows" {
 		if _, err := os.Stat(f.Name()); !os.IsNotExist(err) {
+			f.Close()
+			cleanup()
 			t.Errorf("expected file to be unlinked on disk, stat err = %v", err)
+			return
+		}
+	}
+
+	// Cleanup must be called after close. On POSIX it is a no-op; on
+	// Windows it deletes the temp file.
+	f.Close()
+	cleanup()
+
+	if runtime.GOOS == "windows" {
+		if _, err := os.Stat(f.Name()); !os.IsNotExist(err) {
+			t.Errorf("expected temp file to be deleted on Windows, stat err = %v", err)
 		}
 	}
 }
@@ -73,11 +91,14 @@ else
   echo "env_leak=false"
 fi
 `
-	keyFile, err := writeKeyToUnlinkedFile(want)
+	keyFile, cleanup, err := writeKeyToUnlinkedFile(want)
 	if err != nil {
 		t.Fatalf("writeKeyToUnlinkedFile: %v", err)
 	}
-	defer keyFile.Close()
+	defer func() {
+		_ = keyFile.Close()
+		cleanup()
+	}()
 
 	cmd := exec.Command("/bin/sh", "-c", script)
 	// Strip any inherited key env vars so the test cannot pass by

cmd/odek/subagent_tool.go

@@ -235,16 +235,23 @@ func (t *delegateTasksTool) runTask(taskIdx int, goal, taskContext, guidance, tr
 	// as it has read the key.
 	cmd.Env = os.Environ() // parent already stripped *_API_KEY in LoadConfig
 	var keyFile *os.File
+	var keyCleanup func()
 	if t.apiKey != "" {
-		f, err := writeKeyToUnlinkedFile(t.apiKey)
+		f, cleanup, err := writeKeyToUnlinkedFile(t.apiKey)
 		if err != nil {
 			return fmt.Sprintf(`{"error":"key handoff: %v"}`, err)
 		}
 		keyFile = f
+		keyCleanup = cleanup
 		cmd.ExtraFiles = []*os.File{keyFile}
 		// FD 3 in the child = the first ExtraFiles entry.
 		cmd.Env = append(cmd.Env, keyFDEnvVar+"=3")
-		defer keyFile.Close()
+		defer func() {
+			_ = keyFile.Close()
+			if keyCleanup != nil {
+				keyCleanup()
+			}
+		}()
 	}
 
 	if err := cmd.Start(); err != nil {