spindle/secrets: rework openbao manager to use bao proxy

Signed-off-by: Anirudh Oppiliappan <[e-mail]>

spindle/config/config.go

···
 }
 
 type OpenBaoConfig struct {
-	Addr     string `env:"ADDR"`
-	RoleID   string `env:"ROLE_ID"`
-	SecretID string `env:"SECRET_ID"`
-	Mount    string `env:"MOUNT, default=spindle"`
+	ProxyAddr string `env:"PROXY_ADDR, default=http://127.0.0.1:8200"`
+	Mount     string `env:"MOUNT, default=spindle"`
 }
 
 type Pipelines struct {

spindle/secrets/openbao.go

···
 	"log/slog"
 	"path"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/bluesky-social/indigo/atproto/syntax"
···
 type OpenBaoManager struct {
 	client    *vault.Client
 	mountPath string
-	roleID    string
-	secretID  string
-	stopCh    chan struct{}
-	tokenMu   sync.RWMutex
 	logger    *slog.Logger
 }
 
···
 	}
 }
 
-func NewOpenBaoManager(address, roleID, secretID string, logger *slog.Logger, opts ...OpenBaoManagerOpt) (*OpenBaoManager, error) {
-	if address == "" {
-		return nil, fmt.Errorf("address cannot be empty")
-	}
-	if roleID == "" {
-		return nil, fmt.Errorf("role_id cannot be empty")
-	}
-	if secretID == "" {
-		return nil, fmt.Errorf("secret_id cannot be empty")
+// NewOpenBaoManager creates a new OpenBao manager that connects to a Bao Proxy
+// The proxyAddress should point to the local Bao Proxy (e.g., "http://127.0.0.1:8200")
+// The proxy handles all authentication automatically via Auto-Auth
+func NewOpenBaoManager(proxyAddress string, logger *slog.Logger, opts ...OpenBaoManagerOpt) (*OpenBaoManager, error) {
+	if proxyAddress == "" {
+		return nil, fmt.Errorf("proxy address cannot be empty")
 	}
 
 	config := vault.DefaultConfig()
-	config.Address = address
+	config.Address = proxyAddress
 
 	client, err := vault.NewClient(config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create openbao client: %w", err)
 	}
 
-	// Authenticate using AppRole
-	err = authenticateAppRole(client, roleID, secretID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to authenticate with AppRole: %w", err)
-	}
-
 	manager := &OpenBaoManager{
 		client:    client,
 		mountPath: "spindle", // default KV v2 mount path
-		roleID:    roleID,
-		secretID:  secretID,
-		stopCh:    make(chan struct{}),
 		logger:    logger,
 	}
 
···
 		opt(manager)
 	}
 
-	go manager.tokenRenewalLoop()
-
-	return manager, nil
-}
-
-// authenticateAppRole authenticates the client using AppRole method
-func authenticateAppRole(client *vault.Client, roleID, secretID string) error {
-	authData := map[string]interface{}{
-		"role_id":   roleID,
-		"secret_id": secretID,
-	}
-
-	resp, err := client.Logical().Write("auth/approle/login", authData)
-	if err != nil {
-		return fmt.Errorf("failed to login with AppRole: %w", err)
-	}
-
-	if resp == nil || resp.Auth == nil {
-		return fmt.Errorf("no auth info returned from AppRole login")
-	}
-
-	client.SetToken(resp.Auth.ClientToken)
-	return nil
-}
-
-// stop stops the token renewal goroutine
-func (v *OpenBaoManager) Stop() {
-	close(v.stopCh)
-}
-
-// tokenRenewalLoop runs in a background goroutine to automatically renew or re-authenticate tokens
-func (v *OpenBaoManager) tokenRenewalLoop() {
-	ticker := time.NewTicker(30 * time.Second) // Check every 30 seconds
-	defer ticker.Stop()
-
-	for {
-		select {
-		case <-v.stopCh:
-			return
-		case <-ticker.C:
-			ctx := context.Background()
-			if err := v.ensureValidToken(ctx); err != nil {
-				v.logger.Error("openbao token renewal failed", "error", err)
-			}
-		}
-	}
-}
-
-// ensureValidToken checks if the current token is valid and renews or re-authenticates if needed
-func (v *OpenBaoManager) ensureValidToken(ctx context.Context) error {
-	v.tokenMu.Lock()
-	defer v.tokenMu.Unlock()
-
-	// check current token info
-	tokenInfo, err := v.client.Auth().Token().LookupSelf()
-	if err != nil {
-		// token is invalid, need to re-authenticate
-		v.logger.Warn("token lookup failed, re-authenticating", "error", err)
-		return v.reAuthenticate()
-	}
-
-	if tokenInfo == nil || tokenInfo.Data == nil {
-		return v.reAuthenticate()
-	}
-
-	// check TTL
-	ttlRaw, ok := tokenInfo.Data["ttl"]
-	if !ok {
-		return v.reAuthenticate()
-	}
-
-	var ttl int64
-	switch t := ttlRaw.(type) {
-	case int64:
-		ttl = t
-	case float64:
-		ttl = int64(t)
-	case int:
-		ttl = int64(t)
-	default:
-		return v.reAuthenticate()
+	if err := manager.testConnection(); err != nil {
+		return nil, fmt.Errorf("failed to connect to bao proxy: %w", err)
 	}
 
-	// if TTL is less than 5 minutes, try to renew
-	if ttl < 300 {
-		v.logger.Info("token ttl low, attempting renewal", "ttl_seconds", ttl)
-
-		renewResp, err := v.client.Auth().Token().RenewSelf(3600) // 1h
-		if err != nil {
-			v.logger.Warn("token renewal failed, re-authenticating", "error", err)
-			return v.reAuthenticate()
-		}
-
-		if renewResp == nil || renewResp.Auth == nil {
-			v.logger.Warn("token renewal returned no auth info, re-authenticating")
-			return v.reAuthenticate()
-		}
-
-		v.logger.Info("token renewed successfully", "new_ttl_seconds", renewResp.Auth.LeaseDuration)
-	}
-
-	return nil
+	logger.Info("successfully connected to bao proxy", "address", proxyAddress)
+	return manager, nil
 }
 
-// reAuthenticate performs a fresh authentication using AppRole
-func (v *OpenBaoManager) reAuthenticate() error {
-	v.logger.Info("re-authenticating with approle")
+// testConnection verifies that we can connect to the proxy
+func (v *OpenBaoManager) testConnection() error {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
 
-	err := authenticateAppRole(v.client, v.roleID, v.secretID)
+	// try token self-lookup as a quick way to verify proxy works
+	// and is authenticated
+	_, err := v.client.Auth().Token().LookupSelfWithContext(ctx)
 	if err != nil {
-		return fmt.Errorf("re-authentication failed: %w", err)
+		return fmt.Errorf("proxy connection test failed: %w", err)
 	}
 
-	v.logger.Info("re-authentication successful")
 	return nil
 }
 
 func (v *OpenBaoManager) AddSecret(ctx context.Context, secret UnlockedSecret) error {
-	v.tokenMu.RLock()
-	defer v.tokenMu.RUnlock()
 	if err := ValidateKey(secret.Key); err != nil {
 		return err
 	}
 
 	secretPath := v.buildSecretPath(secret.Repo, secret.Key)
-
-	fmt.Println(v.mountPath, secretPath)
+	v.logger.Debug("adding secret", "repo", secret.Repo, "key", secret.Key, "path", secretPath)
 
+	// Check if secret already exists
 	existing, err := v.client.KVv2(v.mountPath).Get(ctx, secretPath)
 	if err == nil && existing != nil {
+		v.logger.Debug("secret already exists", "path", secretPath)
 		return ErrKeyAlreadyPresent
 	}
 
···
 		"created_by": secret.CreatedBy.String(),
 	}
 
-	_, err = v.client.KVv2(v.mountPath).Put(ctx, secretPath, secretData)
+	v.logger.Debug("writing secret to openbao", "path", secretPath, "mount", v.mountPath)
+	resp, err := v.client.KVv2(v.mountPath).Put(ctx, secretPath, secretData)
 	if err != nil {
+		v.logger.Error("failed to write secret", "path", secretPath, "error", err)
 		return fmt.Errorf("failed to store secret in openbao: %w", err)
 	}
 
+	v.logger.Debug("secret write response", "version", resp.VersionMetadata.Version, "created_time", resp.VersionMetadata.CreatedTime)
+
+	v.logger.Debug("verifying secret was written", "path", secretPath)
+	readBack, err := v.client.KVv2(v.mountPath).Get(ctx, secretPath)
+	if err != nil {
+		v.logger.Error("failed to verify secret after write", "path", secretPath, "error", err)
+		return fmt.Errorf("secret not found after writing to %s/%s: %w", v.mountPath, secretPath, err)
+	}
+
+	if readBack == nil || readBack.Data == nil {
+		v.logger.Error("secret verification returned empty data", "path", secretPath)
+		return fmt.Errorf("secret verification failed: empty data returned for %s/%s", v.mountPath, secretPath)
+	}
+
+	v.logger.Info("secret added and verified successfully", "repo", secret.Repo, "key", secret.Key, "version", readBack.VersionMetadata.Version)
 	return nil
 }
 
 func (v *OpenBaoManager) RemoveSecret(ctx context.Context, secret Secret[any]) error {
-	v.tokenMu.RLock()
-	defer v.tokenMu.RUnlock()
 	secretPath := v.buildSecretPath(secret.Repo, secret.Key)
 
+	// check if secret exists
 	existing, err := v.client.KVv2(v.mountPath).Get(ctx, secretPath)
 	if err != nil || existing == nil {
 		return ErrKeyNotFound
···
 		return fmt.Errorf("failed to delete secret from openbao: %w", err)
 	}
 
+	v.logger.Debug("secret removed successfully", "repo", secret.Repo, "key", secret.Key)
 	return nil
 }
 
 func (v *OpenBaoManager) GetSecretsLocked(ctx context.Context, repo DidSlashRepo) ([]LockedSecret, error) {
-	v.tokenMu.RLock()
-	defer v.tokenMu.RUnlock()
 	repoPath := v.buildRepoPath(repo)
 
-	secretsList, err := v.client.Logical().List(fmt.Sprintf("%s/metadata/%s", v.mountPath, repoPath))
+	secretsList, err := v.client.Logical().ListWithContext(ctx, fmt.Sprintf("%s/metadata/%s", v.mountPath, repoPath))
 	if err != nil {
 		if strings.Contains(err.Error(), "no secret found") || strings.Contains(err.Error(), "no handler for route") {
 			return []LockedSecret{}, nil
···
 			continue
 		}
 
-		secretPath := path.Join(repoPath, key)
+		secretPath := fmt.Sprintf("%s/%s", repoPath, key)
 		secretData, err := v.client.KVv2(v.mountPath).Get(ctx, secretPath)
 		if err != nil {
-			continue // Skip secrets we can't read
+			v.logger.Warn("failed to read secret metadata", "path", secretPath, "error", err)
+			continue
 		}
 
 		if secretData == nil || secretData.Data == nil {
···
 		secrets = append(secrets, secret)
 	}
 
+	v.logger.Debug("retrieved locked secrets", "repo", repo, "count", len(secrets))
 	return secrets, nil
 }
 
 func (v *OpenBaoManager) GetSecretsUnlocked(ctx context.Context, repo DidSlashRepo) ([]UnlockedSecret, error) {
-	v.tokenMu.RLock()
-	defer v.tokenMu.RUnlock()
 	repoPath := v.buildRepoPath(repo)
 
-	secretsList, err := v.client.Logical().List(fmt.Sprintf("%s/metadata/%s", v.mountPath, repoPath))
+	secretsList, err := v.client.Logical().ListWithContext(ctx, fmt.Sprintf("%s/metadata/%s", v.mountPath, repoPath))
 	if err != nil {
 		if strings.Contains(err.Error(), "no secret found") || strings.Contains(err.Error(), "no handler for route") {
 			return []UnlockedSecret{}, nil
···
 			continue
 		}
 
-		secretPath := path.Join(repoPath, key)
+		secretPath := fmt.Sprintf("%s/%s", repoPath, key)
 		secretData, err := v.client.KVv2(v.mountPath).Get(ctx, secretPath)
 		if err != nil {
+			v.logger.Warn("failed to read secret", "path", secretPath, "error", err)
 			continue
 		}
 
···
 
 		valueStr, ok := data["value"].(string)
 		if !ok {
-			continue // skip secrets without values
+			v.logger.Warn("secret missing value", "path", secretPath)
+			continue
 		}
 
 		createdAtStr, ok := data["created_at"].(string)
···
 		secrets = append(secrets, secret)
 	}
 
+	v.logger.Debug("retrieved unlocked secrets", "repo", repo, "count", len(secrets))
 	return secrets, nil
 }
 
-// buildRepoPath creates an OpenBao path for a repository
+// buildRepoPath creates a safe path for a repository
 func (v *OpenBaoManager) buildRepoPath(repo DidSlashRepo) string {
 	// convert DidSlashRepo to a safe path by replacing special characters
 	repoPath := strings.ReplaceAll(string(repo), "/", "_")
···
 	return fmt.Sprintf("repos/%s", repoPath)
 }
 
-// buildSecretPath creates an OpenBao path for a specific secret
+// buildSecretPath creates a path for a specific secret
 func (v *OpenBaoManager) buildSecretPath(repo DidSlashRepo, key string) string {
 	return path.Join(v.buildRepoPath(repo), key)
 }

spindle/secrets/openbao_test.go

···
 	secrets       map[string]UnlockedSecret // key: repo_key format
 	shouldError   bool
 	errorToReturn error
-	stopped       bool
 }
 
 func NewMockOpenBaoManager() *MockOpenBaoManager {
···
 func (m *MockOpenBaoManager) ClearError() {
 	m.shouldError = false
 	m.errorToReturn = nil
-}
-
-func (m *MockOpenBaoManager) Stop() {
-	m.stopped = true
-}
-
-func (m *MockOpenBaoManager) IsStopped() bool {
-	return m.stopped
 }
 
 func (m *MockOpenBaoManager) buildKey(repo DidSlashRepo, key string) string {
···
 	}
 }
 
+// Test MockOpenBaoManager interface compliance
+func TestMockOpenBaoManagerInterface(t *testing.T) {
+	var _ Manager = (*MockOpenBaoManager)(nil)
+}
+
 func TestOpenBaoManagerInterface(t *testing.T) {
 	var _ Manager = (*OpenBaoManager)(nil)
 }
···
 func TestNewOpenBaoManager(t *testing.T) {
 	tests := []struct {
 		name          string
-		address       string
-		roleID        string
-		secretID      string
+		proxyAddr     string
 		opts          []OpenBaoManagerOpt
 		expectError   bool
 		errorContains string
 	}{
 		{
-			name:          "empty address",
-			address:       "",
-			roleID:        "test-role-id",
-			secretID:      "test-secret-id",
+			name:          "empty proxy address",
+			proxyAddr:     "",
 			opts:          nil,
 			expectError:   true,
-			errorContains: "address cannot be empty",
+			errorContains: "proxy address cannot be empty",
 		},
 		{
-			name:          "empty role_id",
-			address:       "http://localhost:8200",
-			roleID:        "",
-			secretID:      "test-secret-id",
+			name:          "valid proxy address",
+			proxyAddr:     "http://localhost:8200",
 			opts:          nil,
-			expectError:   true,
-			errorContains: "role_id cannot be empty",
+			expectError:   true, // Will fail because no real proxy is running
+			errorContains: "failed to connect to bao proxy",
 		},
 		{
-			name:          "empty secret_id",
-			address:       "http://localhost:8200",
-			roleID:        "test-role-id",
-			secretID:      "",
-			opts:          nil,
-			expectError:   true,
-			errorContains: "secret_id cannot be empty",
+			name:          "with mount path option",
+			proxyAddr:     "http://localhost:8200",
+			opts:          []OpenBaoManagerOpt{WithMountPath("custom-mount")},
+			expectError:   true, // Will fail because no real proxy is running
+			errorContains: "failed to connect to bao proxy",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			logger := slog.New(slog.NewTextHandler(os.Stderr, nil))
-			manager, err := NewOpenBaoManager(tt.address, tt.roleID, tt.secretID, logger, tt.opts...)
+			manager, err := NewOpenBaoManager(tt.proxyAddr, logger, tt.opts...)
 
 			if tt.expectError {
 				assert.Error(t, err)
 				assert.Nil(t, manager)
 				assert.Contains(t, err.Error(), tt.errorContains)
 			} else {
-				// For valid configurations, we expect an error during authentication
-				// since we're not connecting to a real OpenBao server
-				assert.Error(t, err)
-				assert.Nil(t, manager)
+				assert.NoError(t, err)
+				assert.NotNil(t, manager)
 			}
 		})
 	}
···
 	assert.Equal(t, "custom-mount", manager.mountPath)
 }
 
-func TestOpenBaoManager_Stop(t *testing.T) {
-	// Create a manager with minimal setup
-	manager := &OpenBaoManager{
-		mountPath: "test",
-		stopCh:    make(chan struct{}),
-	}
-
-	// Verify the manager implements Stopper interface
-	var stopper Stopper = manager
-	assert.NotNil(t, stopper)
-
-	// Call Stop and verify it doesn't panic
-	assert.NotPanics(t, func() {
-		manager.Stop()
-	})
-
-	// Verify the channel was closed
-	select {
-	case <-manager.stopCh:
-		// Channel was closed as expected
-	default:
-		t.Error("Expected stop channel to be closed after Stop()")
-	}
-}
-
-func TestOpenBaoManager_StopperInterface(t *testing.T) {
-	manager := &OpenBaoManager{}
-
-	// Verify that OpenBaoManager implements the Stopper interface
-	_, ok := interface{}(manager).(Stopper)
-	assert.True(t, ok, "OpenBaoManager should implement Stopper interface")
-}
-
-// Test MockOpenBaoManager interface compliance
-func TestMockOpenBaoManagerInterface(t *testing.T) {
-	var _ Manager = (*MockOpenBaoManager)(nil)
-	var _ Stopper = (*MockOpenBaoManager)(nil)
-}
-
 func TestMockOpenBaoManager_AddSecret(t *testing.T) {
 	tests := []struct {
 		name        string
···
 	assert.NoError(t, err)
 }
 
-func TestMockOpenBaoManager_Stop(t *testing.T) {
-	mock := NewMockOpenBaoManager()
-
-	assert.False(t, mock.IsStopped())
-
-	mock.Stop()
-
-	assert.True(t, mock.IsStopped())
-}
-
 func TestMockOpenBaoManager_Integration(t *testing.T) {
 	tests := []struct {
 		name     string
···
 		})
 	}
 }
+
+func TestOpenBaoManager_ProxyConfiguration(t *testing.T) {
+	tests := []struct {
+		name        string
+		proxyAddr   string
+		description string
+	}{
+		{
+			name:        "default_localhost",
+			proxyAddr:   "http://127.0.0.1:8200",
+			description: "Should connect to default localhost proxy",
+		},
+		{
+			name:        "custom_host",
+			proxyAddr:   "http://bao-proxy:8200",
+			description: "Should connect to custom proxy host",
+		},
+		{
+			name:        "https_proxy",
+			proxyAddr:   "https://127.0.0.1:8200",
+			description: "Should connect to HTTPS proxy",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Log("Testing scenario:", tt.description)
+			logger := slog.New(slog.NewTextHandler(os.Stderr, nil))
+
+			// All these will fail because no real proxy is running
+			// but we can test that the configuration is properly accepted
+			manager, err := NewOpenBaoManager(tt.proxyAddr, logger)
+			assert.Error(t, err) // Expected because no real proxy
+			assert.Nil(t, manager)
+			assert.Contains(t, err.Error(), "failed to connect to bao proxy")
+		})
+	}
+}

spindle/secrets/policy.hcl

···
-# KV v2 data operations  
-path "spindle/data/*" {
+# Allow full access to the spindle KV mount
+path "spindle/*" {
   capabilities = ["create", "read", "update", "delete", "list"]
 }
 
-# KV v2 metadata operations (needed for listing)
+path "spindle/data/*" {
+  capabilities = ["create", "read", "update", "delete"]
+}
+
 path "spindle/metadata/*" {
   capabilities = ["list", "read", "delete"]
 }
 
-# Root path access (needed for mount-level operations)
-path "spindle/*" {
-  capabilities = ["list"]
+# Allow listing mounts (for connection testing)
+path "sys/mounts" {
+  capabilities = ["read"]
 }
 
+# Allow token self-lookup (for health checks)
+path "auth/token/lookup-self" {
+  capabilities = ["read"]
+}

spindle/server.go

···
 	var vault secrets.Manager
 	switch cfg.Server.Secrets.Provider {
 	case "openbao":
-		if cfg.Server.Secrets.OpenBao.Addr == "" {
-			return fmt.Errorf("openbao address is required when using openbao secrets provider")
-		}
-		if cfg.Server.Secrets.OpenBao.RoleID == "" {
-			return fmt.Errorf("openbao role_id is required when using openbao secrets provider")
-		}
-		if cfg.Server.Secrets.OpenBao.SecretID == "" {
-			return fmt.Errorf("openbao secret_id is required when using openbao secrets provider")
+		if cfg.Server.Secrets.OpenBao.ProxyAddr == "" {
+			return fmt.Errorf("openbao proxy address is required when using openbao secrets provider")
 		}
 		vault, err = secrets.NewOpenBaoManager(
-			cfg.Server.Secrets.OpenBao.Addr,
-			cfg.Server.Secrets.OpenBao.RoleID,
-			cfg.Server.Secrets.OpenBao.SecretID,
+			cfg.Server.Secrets.OpenBao.ProxyAddr,
 			logger,
 			secrets.WithMountPath(cfg.Server.Secrets.OpenBao.Mount),
 		)
 		if err != nil {
 			return fmt.Errorf("failed to setup openbao secrets provider: %w", err)
 		}
-		logger.Info("using openbao secrets provider", "address", cfg.Server.Secrets.OpenBao.Addr, "mount", cfg.Server.Secrets.OpenBao.Mount)
+		logger.Info("using openbao secrets provider", "proxy_address", cfg.Server.Secrets.OpenBao.ProxyAddr, "mount", cfg.Server.Secrets.OpenBao.Mount)
 	case "sqlite", "":
 		vault, err = secrets.NewSQLiteManager(cfg.Server.DBPath, secrets.WithTableName("secrets"))
 		if err != nil {