Merge pull request #20 from xmindlab-heicode/fix/secret-lifecycle-purge-rotate-cleanup

fix(secret): 闭合 #4 secret 生命周期 — 账户停用全量清理 + 30天 purge + 显式轮换

heicode/controller/resource.go

@@ -627,12 +627,23 @@ func UpsertResourceSecret(c *gin.Context) {
 		common.ApiError(c, err)
 		return
 	}
-	secretName := resourceSecretName(resource)
-	secretRef, err := client.putSecret(secretName, payload.Data)
+	// Issue #4 rotation: if this binding already has a secret, rotate it (Azure
+	// stores a new version under the same ref/name); otherwise create it. Either
+	// way the plaintext credential only lives in Key Vault, never in our DB.
+	var secretRef string
+	rotated := strings.TrimSpace(resource.SecretRef) != ""
+	if rotated {
+		secretRef, err = client.rotateSecret(resource.SecretRef, payload.Data)
+	} else {
+		secretRef, err = client.putSecret(resourceSecretName(resource), payload.Data)
+	}
 	if err != nil {
 		common.ApiError(c, err)
 		return
 	}
+	if rotated {
+		common.SysLog(fmt.Sprintf("UpsertResourceSecret: rotated KV secret for binding %d (user %d) — new version", resource.Id, userId))
+	}
 	resource.SecretRef = secretRef
 	if err := model.DB.Save(&resource).Error; err != nil {
 		common.ApiError(c, err)

heicode/controller/secret_lifecycle.go

@@ -0,0 +1,257 @@
+package controller
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"net/http"
+	"net/url"
+	"os"
+	"strings"
+	"sync"
+	"time"
+
+	"github.com/heicode/manager/common"
+	"github.com/heicode/manager/model"
+)
+
+// Secret lifecycle (issue #4). Beyond the revoke-time delete already shipped in
+// DeleteResource, this file closes the remaining lifecycle:
+//   1. revokeUserResourceSecrets — on account disable/delete, soft-delete ALL of
+//      the user's resource-bound Key Vault secrets (full cleanup, not just DB).
+//   2. StartSecretPurgeTask — daily job that PERMANENTLY purges vault secrets
+//      soft-deleted ≥ retention (default 30 days): the "30-day delete / render
+//      unrecoverable" requirement.
+//   3. rotation — UpsertResourceSecret re-PUTs a credential, which Azure stores
+//      as a new version (rotateSecret == putSecret); see resource.go.
+
+// revokeUserResourceSecrets revokes every still-active resource binding owned by
+// userID and best-effort soft-deletes its Key Vault material, so disabling or
+// deleting an account never leaves live credentials in the vault. The DB revoke
+// is authoritative and committed first; a Key Vault outage is logged but never
+// blocks account management. Returns the number of secrets soft-deleted.
+func revokeUserResourceSecrets(userID int) int {
+	if userID <= 0 || model.DB == nil {
+		return 0
+	}
+	var bindings []model.ResourceBinding
+	if err := model.DB.
+		Where("user_id = ? AND secret_ref <> '' AND status <> ?", userID, "revoked").
+		Find(&bindings).Error; err != nil {
+		common.SysLog(fmt.Sprintf("revokeUserResourceSecrets: list bindings for user %d failed: %s", userID, err.Error()))
+		return 0
+	}
+	if len(bindings) == 0 {
+		return 0
+	}
+	ids := make([]int, 0, len(bindings))
+	for _, b := range bindings {
+		ids = append(ids, b.Id)
+	}
+	if err := model.DB.Model(&model.ResourceBinding{}).Where("id IN ?", ids).
+		Update("status", "revoked").Error; err != nil {
+		common.SysLog(fmt.Sprintf("revokeUserResourceSecrets: mark revoked for user %d failed: %s", userID, err.Error()))
+	}
+	store, sErr := newSecretStoreClientFromEnv()
+	if sErr != nil {
+		common.SysLog(fmt.Sprintf("revokeUserResourceSecrets: secret store unavailable, %d KV secrets NOT deleted for user %d: %s", len(bindings), userID, sErr.Error()))
+		return 0
+	}
+	deleted := 0
+	for _, b := range bindings {
+		if dErr := store.deleteSecret(b.SecretRef); dErr != nil {
+			common.SysLog(fmt.Sprintf("revokeUserResourceSecrets: KV delete failed for binding %d (user %d): %s", b.Id, userID, dErr.Error()))
+			continue
+		}
+		deleted++
+	}
+	common.SysLog(fmt.Sprintf("revokeUserResourceSecrets: user %d — %d bindings revoked, %d KV secrets soft-deleted", userID, len(bindings), deleted))
+	return deleted
+}
+
+// ── 30-day purge ────────────────────────────────────────────────────────────
+
+type deletedSecretInfo struct {
+	name        string
+	deletedDate int64 // unix seconds, from Azure KV
+}
+
+// parseDeletedSecretsPage parses one page of Azure KV GET /deletedsecrets and
+// returns the secrets + the nextLink (empty when no more pages). Pure (testable).
+func parseDeletedSecretsPage(body []byte) ([]deletedSecretInfo, string, error) {
+	var page struct {
+		Value []struct {
+			RecoveryID  string `json:"recoveryId"`
+			ID          string `json:"id"`
+			DeletedDate int64  `json:"deletedDate"`
+		} `json:"value"`
+		NextLink string `json:"nextLink"`
+	}
+	if err := common.Unmarshal(body, &page); err != nil {
+		return nil, "", err
+	}
+	items := make([]deletedSecretInfo, 0, len(page.Value))
+	for _, v := range page.Value {
+		name := lastPathSegment(v.RecoveryID)
+		if name == "" {
+			name = lastPathSegment(v.ID)
+		}
+		if name == "" {
+			continue
+		}
+		items = append(items, deletedSecretInfo{name: name, deletedDate: v.DeletedDate})
+	}
+	return items, strings.TrimSpace(page.NextLink), nil
+}
+
+func lastPathSegment(raw string) string {
+	raw = strings.TrimSpace(raw)
+	if raw == "" {
+		return ""
+	}
+	if u, err := url.Parse(raw); err == nil && u.Path != "" {
+		raw = u.Path
+	}
+	raw = strings.Trim(raw, "/")
+	if i := strings.LastIndex(raw, "/"); i >= 0 {
+		raw = raw[i+1:]
+	}
+	return raw
+}
+
+// secretExpired reports whether a secret soft-deleted at deletedDate (unix sec)
+// is at or past retentionDays old relative to nowUnix. Pure (testable).
+func secretExpired(deletedDate int64, retentionDays int, nowUnix int64) bool {
+	if deletedDate <= 0 || retentionDays <= 0 {
+		return false
+	}
+	return nowUnix-deletedDate >= int64(retentionDays)*86400
+}
+
+// listDeletedSecrets returns every soft-deleted secret in the vault (paginated).
+func (s secretStoreClient) listDeletedSecrets() ([]deletedSecretInfo, error) {
+	token, err := s.accessToken()
+	if err != nil {
+		return nil, err
+	}
+	endpoint := fmt.Sprintf("%s/deletedsecrets?api-version=7.4", s.vaultURL)
+	var out []deletedSecretInfo
+	for endpoint != "" {
+		req, rErr := http.NewRequest(http.MethodGet, endpoint, nil)
+		if rErr != nil {
+			return nil, rErr
+		}
+		req.Header.Set("Authorization", "Bearer "+token)
+		resp, dErr := s.client.Do(req)
+		if dErr != nil {
+			return nil, dErr
+		}
+		body, _ := io.ReadAll(io.LimitReader(resp.Body, 4<<20))
+		resp.Body.Close()
+		if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
+			message := readSecretStoreError(bytes.NewReader(body))
+			if message == "" {
+				message = resp.Status
+			}
+			return nil, fmt.Errorf("Azure Key Vault list deleted secrets failed: %s", message)
+		}
+		items, next, pErr := parseDeletedSecretsPage(body)
+		if pErr != nil {
+			return nil, pErr
+		}
+		out = append(out, items...)
+		endpoint = next
+	}
+	return out, nil
+}
+
+// purgeExpiredVaultSecrets permanently purges every soft-deleted vault secret
+// older than retentionDays. An optional namePrefix restricts purging to secrets
+// HM manages (empty = all soft-deleted secrets in the vault). Returns (purged,
+// scanned). Purge-protected vaults safely no-op (purgeSecret treats 403 as ok).
+func purgeExpiredVaultSecrets(retentionDays int, namePrefix string, nowUnix int64) (purged int, scanned int, err error) {
+	store, err := newSecretStoreClientFromEnv()
+	if err != nil {
+		return 0, 0, err
+	}
+	items, err := store.listDeletedSecrets()
+	if err != nil {
+		return 0, 0, err
+	}
+	prefix := strings.TrimSpace(namePrefix)
+	for _, it := range items {
+		if prefix != "" && !strings.HasPrefix(it.name, prefix) {
+			continue
+		}
+		scanned++
+		if !secretExpired(it.deletedDate, retentionDays, nowUnix) {
+			continue
+		}
+		if pErr := store.purgeSecret(store.secretRef(it.name)); pErr != nil {
+			common.SysLog(fmt.Sprintf("purgeExpiredVaultSecrets: purge %q failed: %s", it.name, pErr.Error()))
+			continue
+		}
+		common.SysLog(fmt.Sprintf("purgeExpiredVaultSecrets: purged %q (soft-deleted %d days ago)", it.name, (nowUnix-it.deletedDate)/86400))
+		purged++
+	}
+	return purged, scanned, nil
+}
+
+var secretPurgeTaskOnce sync.Once
+
+// StartSecretPurgeTask launches the daily background purge of vault secrets that
+// have been soft-deleted ≥ HEICODE_SECRET_PURGE_RETENTION_DAYS (default 30) ago,
+// satisfying issue #4's "render unrecoverable within 30 days" requirement.
+// Disabled by HEICODE_SECRET_PURGE_ENABLED=false; no-ops when no vault is
+// configured. HEICODE_SECRET_PURGE_NAME_PREFIX scopes purging to HM-managed
+// secrets when the vault is shared.
+func StartSecretPurgeTask() {
+	secretPurgeTaskOnce.Do(func() {
+		if !common.GetEnvOrDefaultBool("HEICODE_SECRET_PURGE_ENABLED", true) {
+			common.SysLog("secret purge task disabled (HEICODE_SECRET_PURGE_ENABLED=false)")
+			return
+		}
+		if strings.TrimSpace(os.Getenv("AZURE_KEY_VAULT_URL")) == "" {
+			return // no vault configured — nothing to purge
+		}
+		intervalHours := common.GetEnvOrDefault("HEICODE_SECRET_PURGE_INTERVAL_HOURS", 24)
+		if intervalHours < 1 {
+			intervalHours = 24
+		}
+		go func() {
+			time.Sleep(5 * time.Minute) // avoid startup churn
+			runSecretPurgeOnce()
+			ticker := time.NewTicker(time.Duration(intervalHours) * time.Hour)
+			defer ticker.Stop()
+			for range ticker.C {
+				runSecretPurgeOnce()
+			}
+		}()
+		common.SysLog(fmt.Sprintf("secret purge task started: retention=%dd interval=%dh", secretPurgeRetentionDays(), intervalHours))
+	})
+}
+
+func secretPurgeRetentionDays() int {
+	d := common.GetEnvOrDefault("HEICODE_SECRET_PURGE_RETENTION_DAYS", 30)
+	if d < 1 {
+		d = 30
+	}
+	return d
+}
+
+func runSecretPurgeOnce() {
+	defer func() {
+		if r := recover(); r != nil {
+			common.SysLog(fmt.Sprintf("secret purge task panic recovered: %v", r))
+		}
+	}()
+	prefix := common.GetEnvOrDefaultString("HEICODE_SECRET_PURGE_NAME_PREFIX", "")
+	purged, scanned, err := purgeExpiredVaultSecrets(secretPurgeRetentionDays(), prefix, time.Now().Unix())
+	if err != nil {
+		common.SysLog("secret purge task: " + err.Error())
+		return
+	}
+	if purged > 0 {
+		common.SysLog(fmt.Sprintf("secret purge task: purged %d of %d scanned deleted secrets past %d-day retention", purged, scanned, secretPurgeRetentionDays()))
+	}
+}

heicode/controller/secret_lifecycle_test.go

@@ -0,0 +1,74 @@
+package controller
+
+import (
+	"testing"
+
+	"github.com/heicode/manager/model"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseDeletedSecretsPage(t *testing.T) {
+	body := []byte(`{
+		"value": [
+			{"recoveryId":"https://v.vault.azure.net/deletedsecrets/users-7-bindings-abc","id":"https://v.vault.azure.net/secrets/users-7-bindings-abc","deletedDate":1700000000},
+			{"id":"https://v.vault.azure.net/secrets/foo","deletedDate":1700001000}
+		],
+		"nextLink":"https://v.vault.azure.net/deletedsecrets?api-version=7.4&$skiptoken=xyz"
+	}`)
+	items, next, err := parseDeletedSecretsPage(body)
+	require.NoError(t, err)
+	require.Len(t, items, 2)
+	require.Equal(t, "users-7-bindings-abc", items[0].name) // from recoveryId
+	require.EqualValues(t, 1700000000, items[0].deletedDate)
+	require.Equal(t, "foo", items[1].name) // recoveryId absent -> falls back to id
+	require.EqualValues(t, 1700001000, items[1].deletedDate)
+	require.NotEmpty(t, next)
+}
+
+func TestSecretExpired(t *testing.T) {
+	const now int64 = 1_000_000_000
+	const day int64 = 86400
+	require.True(t, secretExpired(now-31*day, 30, now), "31d old past 30d retention")
+	require.True(t, secretExpired(now-30*day, 30, now), "exactly 30d is past retention")
+	require.False(t, secretExpired(now-29*day, 30, now), "29d old still within retention")
+	require.False(t, secretExpired(0, 30, now), "unknown deletedDate never expires")
+	require.False(t, secretExpired(now-100*day, 0, now), "retention<=0 disables purge")
+}
+
+func TestLastPathSegment(t *testing.T) {
+	require.Equal(t, "abc", lastPathSegment("https://v.vault.azure.net/deletedsecrets/abc"))
+	require.Equal(t, "x", lastPathSegment("https://v.vault.azure.net/secrets/x/"))
+	require.Equal(t, "plain", lastPathSegment("plain"))
+	require.Equal(t, "", lastPathSegment(""))
+}
+
+// With no Key Vault configured, revokeUserResourceSecrets must still flip the
+// user's secret-bearing bindings to revoked (DB is authoritative) and leave
+// other users / secret-less bindings untouched.
+func TestRevokeUserResourceSecrets_MarksRevoked(t *testing.T) {
+	setupResourceControllerTestDB(t)
+	require.NoError(t, model.DB.AutoMigrate(&model.ResourceBinding{}))
+	t.Setenv("AZURE_KEY_VAULT_URL", "") // force secret store unconfigured -> KV skipped
+
+	mk := func(uid int, secretRef string) model.ResourceBinding {
+		b := model.ResourceBinding{UserId: uid, Name: "n", ResourceType: "git", SecretRef: secretRef, Status: "active"}
+		require.NoError(t, model.DB.Create(&b).Error)
+		return b
+	}
+	withSecretA := mk(7777, "azkv://v.vault.azure.net/secrets/users-7777-a")
+	withSecretB := mk(7777, "azkv://v.vault.azure.net/secrets/users-7777-b")
+	noSecret := mk(7777, "")          // not selected (secret_ref empty) -> stays active
+	otherUser := mk(8888, "azkv://v.vault.azure.net/secrets/users-8888-a")
+
+	revokeUserResourceSecrets(7777)
+
+	get := func(id int) string {
+		var b model.ResourceBinding
+		require.NoError(t, model.DB.Where("id = ?", id).First(&b).Error)
+		return b.Status
+	}
+	require.Equal(t, "revoked", get(withSecretA.Id))
+	require.Equal(t, "revoked", get(withSecretB.Id))
+	require.Equal(t, "active", get(noSecret.Id), "secret-less binding must not be revoked")
+	require.Equal(t, "active", get(otherUser.Id), "another user's binding must be untouched")
+}

heicode/controller/user.go

@@ -780,6 +780,9 @@ func DeleteUser(c *gin.Context) {
 		})
 		return
 	}
+	// Hard-deleting an account also revokes + deletes its resource-bound Key
+	// Vault credential material (issue #4). Best-effort.
+	revokeUserResourceSecrets(id)
 }
 
 func DeleteSelf(c *gin.Context) {
@@ -796,6 +799,9 @@ func DeleteSelf(c *gin.Context) {
 		common.ApiError(c, err)
 		return
 	}
+	// Self-deletion also revokes + deletes the account's resource-bound Key
+	// Vault credential material (issue #4). Best-effort.
+	revokeUserResourceSecrets(id)
 	c.JSON(http.StatusOK, gin.H{
 		"success": true,
 		"message": "",
@@ -898,6 +904,10 @@ func ManageUser(c *gin.Context) {
 		if err := model.InvalidateUserTokensCache(user.Id); err != nil {
 			common.SysLog(fmt.Sprintf("failed to invalidate tokens cache for user %d: %s", user.Id, err.Error()))
 		}
+		// Account deletion must also revoke + delete the user's resource-bound
+		// Key Vault credential material (issue #4): account gone, secrets gone.
+		// Best-effort; never blocks the delete.
+		revokeUserResourceSecrets(user.Id)
 	case "promote":
 		if myRole != common.RoleRootUser {
 			common.ApiErrorI18n(c, i18n.MsgUserAdminCannotPromote)
@@ -983,6 +993,11 @@ func ManageUser(c *gin.Context) {
 			common.SysLog(fmt.Sprintf("failed to invalidate tokens cache for user %d: %s", user.Id, err.Error()))
 		}
 	}
+	if req.Action == "disable" {
+		// Disabling an account also revokes its resource-bound KV secrets (issue
+		// #4), so a disabled user's live credentials don't linger in the vault.
+		revokeUserResourceSecrets(user.Id)
+	}
 	clearUser := model.User{
 		Role:   user.Role,
 		Status: user.Status,

heicode/main.go

@@ -130,6 +130,12 @@ func main() {
 	// Channel upstream model update check task
 	controller.StartChannelUpstreamModelUpdateTask()
 
+	// Secret lifecycle: daily purge of vault secrets soft-deleted past the
+	// retention window (issue #4). Master-only so multiple nodes don't all purge.
+	if common.IsMasterNode {
+		controller.StartSecretPurgeTask()
+	}
+
 	if common.IsMasterNode && constant.UpdateTask {
 		gopool.Go(func() {
 			controller.UpdateMidjourneyTaskBulk()