fix(secret): 闭合 #4 secret 生命周期 — 账户停用全量清理 + 30天 purge + 显式轮换
承接 PR#13(撤销时删 KV secret)的剩余生命周期: - 账户停用/删除全量清理:新增 revokeUserResourceSecrets(userID),撤销该用户全部 带 secret_ref 的资源绑定并 best-effort 软删 KV 凭证材料(DB 标 revoked 为权威, KV 故障只记日志不阻塞)。接入 4 个账户路径:ManageUser disable/delete、 DeleteUser(管理员硬删)、DeleteSelf(自助删)。 - 30天 purge:新增 listDeletedSecrets(GET /deletedsecrets 分页)+ secretExpired 纯函数 + purgeExpiredVaultSecrets;StartSecretPurgeTask 每日(可配)purge 软删 ≥30天(可配)的 secret,master 节点执行,KV 未配置则 no-op,purge-protection 下安全 no-op(403 容错)。env:HEICODE_SECRET_PURGE_ENABLED/RETENTION_DAYS/ INTERVAL_HOURS/NAME_PREFIX。 - 轮换显式化:UpsertResourceSecret 已有 secret 时改用 rotateSecret(同名新版本) 并审计日志,而非每次 putSecret。 测试:parseDeletedSecretsPage / secretExpired / lastPathSegment 纯函数 + revokeUserResourceSecrets 在 KV 未配置下仍正确标记 revoked、不误伤他人/无密钥绑定。 Refs #2 (secret_store delete/rotate/purge 部分;Manager↔Swarm 契约属 Swarm 侧) Fixes #4 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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()))
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user