Fix vaultdb DSN escaping and lease cleanup

Author: cursoragent

internal/connectors/vaultdb/connector.go

@@ -113,10 +113,14 @@ func (c *Connector) Issue(ctx context.Context, req core.IssueRequest) (*core.Iss
 	if err != nil {
 		return nil, err
 	}
-	lease, leaseExpiresAt, err := c.extendLeaseForGrant(ctx, lease, req.Grant.ExpiresAt)
+	extendedLease, leaseExpiresAt, err := c.extendLeaseForGrant(ctx, lease, req.Grant.ExpiresAt)
 	if err != nil {
+		if lease != nil && lease.LeaseID != "" {
+			_ = c.client.RevokeLease(ctx, lease.LeaseID)
+		}
 		return nil, err
 	}
+	lease = extendedLease
 	dsn, err := renderDSN(c.roleDSNs[req.Resource.Name], lease)
 	if err != nil {
 		return nil, err
@@ -192,8 +196,8 @@ func renderDSN(renderPattern string, lease *LeaseCredentials) (string, error) {
 	}
 	var builder strings.Builder
 	if err := tpl.Execute(&builder, map[string]string{
-		"username": url.QueryEscape(lease.Username),
-		"password": url.QueryEscape(lease.Password),
+		"username": url.User(lease.Username).String(),
+		"password": strings.TrimPrefix(url.UserPassword("", lease.Password).String(), ":"),
 	}); err != nil {
 		return "", fmt.Errorf("%w: render dsn template: %v", core.ErrInvalidRequest, err)
 	}

internal/connectors/vaultdb/connector_test.go

@@ -15,8 +15,8 @@ func TestConnector_IssueAndRevokeDynamicCredentials(t *testing.T) {
 
 	client := &fakeVaultClient{
 		lease: &vaultdb.LeaseCredentials{
-			Username:      "v-token-user",
-			Password:      "secret:/?#[]@",
+			Username:      "v token user",
+			Password:      "secret with spaces:/?#[]@",
 			LeaseID:       "database/creds/analytics_ro/123",
 			LeaseDuration: 10 * time.Minute,
 			Renewable:     true,
@@ -56,9 +56,15 @@ func TestConnector_IssueAndRevokeDynamicCredentials(t *testing.T) {
 	if issued.Metadata["lease_expires_at"] == "" {
 		t.Fatalf("issued artifact = %#v, want lease expiry metadata", issued)
 	}
-	if strings.Contains(issued.SecretData["dsn"], "secret:/?#[]@") {
+	if strings.Contains(issued.SecretData["dsn"], "v token user") || strings.Contains(issued.SecretData["dsn"], "secret with spaces:/?#[]@") {
 		t.Fatalf("dsn = %q, want escaped credentials", issued.SecretData["dsn"])
 	}
+	if strings.Contains(issued.SecretData["dsn"], "+") {
+		t.Fatalf("dsn = %q, want spaces percent-encoded in userinfo", issued.SecretData["dsn"])
+	}
+	if want := "postgres://v%20token%20user:secret%20with%20spaces%3A%2F%3F%23%5B%5D%40@db.internal:5432/analytics?sslmode=require"; issued.SecretData["dsn"] != want {
+		t.Fatalf("dsn = %q, want %q", issued.SecretData["dsn"], want)
+	}
 
 	if err := connector.Revoke(context.Background(), core.RevokeRequest{
 		Session: &core.Session{ID: "sess_db", TenantID: "t_acme"},
@@ -187,15 +193,16 @@ func TestConnector_IssueRenewsLeaseToMatchGrantTTL(t *testing.T) {
 func TestConnector_IssueFailsWhenGrantTTLExceedsNonRenewableLease(t *testing.T) {
 	t.Parallel()
 
-	connector, err := vaultdb.NewConnector(vaultdb.Config{
-		Client: &fakeVaultClient{
-			lease: &vaultdb.LeaseCredentials{
-				Username:      "dyn-user",
-				Password:      "dyn-pass",
-				LeaseID:       "database/creds/analytics_ro/123",
-				LeaseDuration: time.Minute,
-			},
+	client := &fakeVaultClient{
+		lease: &vaultdb.LeaseCredentials{
+			Username:      "dyn-user",
+			Password:      "dyn-pass",
+			LeaseID:       "database/creds/analytics_ro/123",
+			LeaseDuration: time.Minute,
 		},
+	}
+	connector, err := vaultdb.NewConnector(vaultdb.Config{
+		Client: client,
 		RoleDSNs: map[string]string{
 			"analytics_ro": "postgres://{{username}}:{{password}}@db.internal:5432/analytics?sslmode=require",
 		},
@@ -219,6 +226,9 @@ func TestConnector_IssueFailsWhenGrantTTLExceedsNonRenewableLease(t *testing.T)
 	if err == nil || !strings.Contains(err.Error(), "not renewable") {
 		t.Fatalf("Issue() error = %v, want renewal failure", err)
 	}
+	if client.revokedLeaseID != "database/creds/analytics_ro/123" {
+		t.Fatalf("revoked lease = %q, want original lease revoked on failure", client.revokedLeaseID)
+	}
 }
 
 type fakeVaultClient struct {