From 9dbb75adb37c74a85c80776ba5466e13577776ca Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Tue, 20 Oct 2026 03:12:13 +0000 Subject: [PATCH] apiserver: persist API keys across restarts via state file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit API keys were a pure in-memory map (store/apikey.go) — every apiserver restart silently revoked every service-account key (advisor, MCP server) until re-issued, with only an inexplicable 401 to show for it. Keys now persist to /state/apikeys.json: hash → metadata only (never the raw key, preserving the existing "a dump cannot leak usable credentials" property), written atomically (temp file + fsync + rename, 0600) on each issuance, loaded at InitStores. The load also restores the user→role mapping, without which a reloaded key would authenticate but be denied every permission check. A missing file is a normal fresh install; a corrupt one fails fast at startup instead of quietly starting with zero keys. A failed save fails the issuance request — the raw key is shown exactly once, so we must not hand out a key we could not persist. The memory-only path (no SetAPIKeyStatePath) is kept for unit tests. Cluster-verified on t1-t3: issue key → restart apiserver → key still authenticates (hosts OK, advisor:report POST published 1) — previously impossible. Verify job submitted, ran, exited; all daemons healthy. Co-Authored-By: Claude Code --- src/docs/advisor-demo-cases.md | 4 +- src/docs/advisor-design.md | 3 +- src/frontend/apiserver/cmd/main.go | 6 +- src/frontend/apiserver/controllers/apikey.go | 16 +- .../apiserver/controllers/apikey_test.go | 48 ++++++ src/frontend/apiserver/store/apikey.go | 100 ++++++++++++- src/frontend/apiserver/store/apikey_test.go | 141 ++++++++++++++++++ src/frontend/apiserver/store/stores.go | 3 + 8 files changed, 312 insertions(+), 9 deletions(-) diff --git a/src/docs/advisor-demo-cases.md b/src/docs/advisor-demo-cases.md index aa8641a2..50fd354b 100644 --- a/src/docs/advisor-demo-cases.md +++ b/src/docs/advisor-demo-cases.md @@ -24,7 +24,7 @@ curl -s -H "Authorization: Bearer $KEY" http://172.31.1.30:8080/xce/v1/hosts \ # 4. LLM 路由可用（用例 2 需要；不通则 advisor 自动降级 echo，演示仍可做但亮点变少） curl -s -m 10 http://127.0.0.1:6789/v1/models -H "Authorization: Bearer sk-litellm-router" | head -c 120 -# 5. API key 是否有效（apiserver 重启过就失效，见"坑"一节） +# 5. API key 是否有效（2026-10-20 起重启不再失效；若 401 见"坑"一节） curl -s -H "Authorization: Bearer $KEY" "http://172.31.1.30:8080/xce/v1/jobs?limit=1" | head -c 80 ``` @@ -196,7 +196,7 @@ EOF | 坑 | 影响 | 预案 | |---|---|---| -| **apiserver 重启使所有 API key 失效**（key 是纯内存 map，无持久化） | 所有 curl/xce 401 | 重新发：logon 拿 admin token → `POST /xce/v1/apikeys`（role_name=advisor_agent，username=advisor-svc）→ 存回 `/tmp/advisor_apikey_v2` → **重启 advisor** | +| ~~apiserver 重启使所有 API key 失效~~（2026-10-20 起已修：key 持久化到 `/opt/xce/state/apikeys.json`，存 hash+metadata，重启自动恢复） | — | 仅当 state 文件丢失/损坏时才需重发：logon 拿 admin token → `POST /xce/v1/apikeys`（role_name=advisor_agent，username=advisor-svc）→ 存回 `/tmp/advisor_apikey_v2` → **重启 advisor** | | glm 路由超时（偶发） | 用例 2 降级为 echo（degraded=true） | 顺势讲兜底设计；或演示前 `curl /v1/models` 预检；**直接换 deepseek-v4-flash**（快且稳，见用例 2） | | SSE 监听用 `curl -m 180` 会在 5min 阈值前断 | 以为推送没到 | 监听窗口用 `-m 600` 以上，或用 MCP 工具事后查 | | stall 阈值 5 分钟 | 用例 1 有 5-6 分钟干等 | 先讲架构图/MCP 工具填这段时间；或把演示作业提前 5 分钟提交 | diff --git a/src/docs/advisor-design.md b/src/docs/advisor-design.md index d928e49d..541d95e1 100644 --- a/src/docs/advisor-design.md +++ b/src/docs/advisor-design.md @@ -119,7 +119,8 @@ tokens / findings / raw / state(+digest) / degraded`。JSONL、按大小轮转 | 阶段 | 内容 | |---|---| -| P1 ✅ | propose 档：findings 推送（SSE 广播 + MCP 工具，已实现；CLI 直出未做）；评估 echo vs LLM 的对照数据 | +| P1 ✅ | propose 档：findings 推送（SSE 广播 + MCP 工具，已实现；CLI 直出未做）；评估 echo vs LLM 的对照数据（`advisor compare` 子命令，d4bd504f） | +| — ✅ | 依赖项修复：apiserver API key 持久化（advisor/MCP 的服务账号 key 原先纯内存，apiserver 重启即全部失效；现落盘 `/state/apikeys.json`，hash+metadata，签发即原子写、启动即加载，重启 key 存活） | | P2 | act 档（仅当引入带校准置信度的 provider——见附录 A）：allowlist + 爆炸半径上限 + 审计 | | — | 自洽性伪置信度（k 次采样一致率）作为 act 档的非 Jev 替代——5 倍成本，只在关键判断用 | diff --git a/src/frontend/apiserver/cmd/main.go b/src/frontend/apiserver/cmd/main.go index f85b8998..7cfc5070 100644 --- a/src/frontend/apiserver/cmd/main.go +++ b/src/frontend/apiserver/cmd/main.go @@ -110,7 +110,11 @@ func main() { } logger.Info("********** Initialize configuration files done!") - // Initialize in-memory stores + // Initialize in-memory stores. API keys are issued at runtime, so they + // get a state file under the instance dir and are reloaded across + // restarts — without this every apiserver restart silently revoked every + // service-account key (advisor, MCP server) until it was re-issued. + store.SetAPIKeyStatePath(utils.Global_TopDir + "/state/apikeys.json") store.InitStores() logger.Info("********** Initialize in-memory stores done!") diff --git a/src/frontend/apiserver/controllers/apikey.go b/src/frontend/apiserver/controllers/apikey.go index cc1e6548..dcf875c6 100644 --- a/src/frontend/apiserver/controllers/apikey.go +++ b/src/frontend/apiserver/controllers/apikey.go @@ -8,8 +8,10 @@ import ( "apiserver/common" "apiserver/models" "apiserver/store" + logger "apiserver/utils" "github.com/gin-gonic/gin" + "go.uber.org/zap" ) func generateAPIKey() (string, error) { @@ -53,14 +55,22 @@ func (a *APIKeyController) GenerateAPIKey(ctx *gin.Context) { createTime := time.Now() expiryTime := createTime.AddDate(0, 0, expiryDays) - // Persist the key (by hash) so it can actually be used to authenticate. - store.AddAPIKey(apiKey, store.APIKeyEntry{ + // Persist the key (by hash) so it can actually be used to authenticate — + // both in memory and, when a state file is configured, on disk so it + // survives a restart. A failed disk write must fail the request: the raw + // key is only ever shown once, so a key we cannot persist is a key the + // operator would be handing out on false pretenses. + if err := store.AddAPIKey(apiKey, store.APIKeyEntry{ Username: req.Username, RoleName: req.RoleName, Description: req.Description, CreateTime: createTime, ExpiryTime: expiryTime, - }) + }); err != nil { + logger.Error("Failed to persist API key state", zap.Error(err)) + ReturnError(ctx, common.ERROR_CODE, "Failed to persist API key: "+err.Error(), nil) + return + } // RequirePermission resolves permissions via store.GetUserRoleNames, not // via the APIKeyEntry directly — register the RBAC user→role mapping too, diff --git a/src/frontend/apiserver/controllers/apikey_test.go b/src/frontend/apiserver/controllers/apikey_test.go index fe60a329..ed568fcb 100644 --- a/src/frontend/apiserver/controllers/apikey_test.go +++ b/src/frontend/apiserver/controllers/apikey_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "path/filepath" "testing" "time" @@ -174,3 +175,50 @@ func TestGenerateAPIKey_RequiresAuthorizationHeader(t *testing.T) { t.Errorf("status = %d, want 401", w.Code) } } + +// TestGenerateAPIKey_StateFileWritten verifies that when a state file is +// configured, a key issued through the HTTP endpoint lands on disk and still +// validates after a simulated restart (fresh stores over the same path). +func TestGenerateAPIKey_StateFileWritten(t *testing.T) { + resetAPIKeyControllerState() + + path := filepath.Join(t.TempDir(), "apikeys.json") + store.SetAPIKeyStatePath(path) + defer store.SetAPIKeyStatePath("") + + gin.SetMode(gin.TestMode) + router := gin.New() + handler := &APIKeyController{} + router.POST("/xce/v1/apikeys", handler.GenerateAPIKey) + + body := `{"username":"advisor-svc","role_name":"advisor_agent","expiry_days":90}` + req := httptest.NewRequest(http.MethodPost, "/xce/v1/apikeys", bytes.NewBufferString(body)) + req.Header.Set("Authorization", "Bearer some-admin-token") + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String()) + } + + var resp struct { + Data models.APIKeyResponse `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + + // Simulate restart: reinitialize the stores (fresh memory) and verify + // the issued key still authenticates. + store.InitStores() + entry, ok := store.ValidateAPIKey(resp.Data.APIKey) + if !ok { + t.Fatal("issued key did not survive a simulated restart") + } + if entry.RoleName != "advisor_agent" { + t.Errorf("reloaded role = %q, want advisor_agent", entry.RoleName) + } + if roles := store.GetUserRoleNames("advisor-svc"); len(roles) == 0 { + t.Error("user→role mapping not restored across restart") + } +} diff --git a/src/frontend/apiserver/store/apikey.go b/src/frontend/apiserver/store/apikey.go index 55c7fb9d..faccc3f6 100644 --- a/src/frontend/apiserver/store/apikey.go +++ b/src/frontend/apiserver/store/apikey.go @@ -3,6 +3,10 @@ package store import ( "crypto/sha256" "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" "time" ) @@ -20,6 +24,15 @@ type APIKeyEntry struct { ExpiryTime time.Time } +// persistedAPIKeyState is the on-disk shape of the API key store. API keys +// are issued at runtime (POST /xce/v1/apikeys), not declared in rbac.yaml, +// so they must survive an apiserver restart the way the in-memory store +// never could. Only hashes are written — the raw key exists solely in the +// HTTP response issued once at creation time. +type persistedAPIKeyState struct { + Keys map[string]APIKeyEntry `json:"keys"` // hash → metadata +} + // HashAPIKey returns the hex-encoded SHA-256 digest of a raw API key. // Used both when persisting a newly generated key and when validating one // presented by a client — the raw value is never compared or stored directly. @@ -28,9 +41,13 @@ func HashAPIKey(rawKey string) string { return hex.EncodeToString(sum[:]) } -// AddAPIKey persists a new API key by its hash. -func AddAPIKey(rawKey string, entry APIKeyEntry) { +// AddAPIKey persists a new API key by its hash. If a state file is in use +// (SetAPIKeyStatePath called before InitStores), the updated store is +// atomically rewritten; a write failure is returned so the issuer can +// surface it rather than silently lose the key on next restart. +func AddAPIKey(rawKey string, entry APIKeyEntry) error { APIKeys.Put(HashAPIKey(rawKey), entry) + return saveAPIKeyState() } // ValidateAPIKey looks up a raw API key by its hash and returns its metadata @@ -45,3 +62,82 @@ func ValidateAPIKey(rawKey string) (APIKeyEntry, bool) { } return entry, true } + +// apiKeyStatePath is empty until SetAPIKeyStatePath is called; an empty path +// keeps the store memory-only (unit tests rely on this). +var apiKeyStatePath string + +// SetAPIKeyStatePath selects where the API key store persists. Must be +// called before InitStores, which performs the initial load. +func SetAPIKeyStatePath(path string) { apiKeyStatePath = path } + +// loadAPIKeyState restores API keys (and their user→role mappings) from the +// state file selected by SetAPIKeyStatePath. A missing file is normal on a +// fresh install — keys will appear as they are issued. A corrupt or +// unreadable file is an error: silently starting with zero keys would turn +// every service account into a mysterious 401. +func loadAPIKeyState() error { + if apiKeyStatePath == "" { + return nil + } + data, err := os.ReadFile(apiKeyStatePath) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("read api key state: %w", err) + } + var state persistedAPIKeyState + if err := json.Unmarshal(data, &state); err != nil { + return fmt.Errorf("parse %s: %w", apiKeyStatePath, err) + } + for hash, entry := range state.Keys { + APIKeys.Put(hash, entry) + if entry.RoleName != "" { + AddRoleForUser(entry.Username, entry.RoleName) + } + } + return nil +} + +// saveAPIKeyState atomically rewrites the state file: write to a temp file +// in the same directory, fsync, then rename over the target. +func saveAPIKeyState() error { + if apiKeyStatePath == "" { + return nil + } + state := persistedAPIKeyState{Keys: make(map[string]APIKeyEntry, APIKeys.Count())} + APIKeys.Range(func(hash string, entry APIKeyEntry) bool { + state.Keys[hash] = entry + return true + }) + data, err := json.MarshalIndent(&state, "", " ") + if err != nil { + return err + } + dir := filepath.Dir(apiKeyStatePath) + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } + tmp, err := os.CreateTemp(dir, ".apikey-*.tmp") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) // no-op after a successful rename + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := os.Chmod(tmpName, 0o600); err != nil { + return err + } + return os.Rename(tmpName, apiKeyStatePath) +} diff --git a/src/frontend/apiserver/store/apikey_test.go b/src/frontend/apiserver/store/apikey_test.go index b1993be4..301c51f5 100644 --- a/src/frontend/apiserver/store/apikey_test.go +++ b/src/frontend/apiserver/store/apikey_test.go @@ -1,11 +1,15 @@ package store import ( + "encoding/json" + "os" + "path/filepath" "testing" "time" ) func resetAPIKeyState() { + SetAPIKeyStatePath("") InitStores() } @@ -92,3 +96,140 @@ func TestValidateAPIKey_StoredByHashNotPlaintext(t *testing.T) { t.Error("store should contain the hashed key") } } + +// ========== Persistence across restarts ========== + +// TestAPIKeyPersistence_ReloadSurvivesRestart simulates an apiserver restart: +// issue keys against one store instance with a state file, then re-run +// InitStores (fresh memory) and verify every key still validates and its +// user→role mapping was restored — the exact failure mode that made every +// apiserver restart silently revoke all service-account keys before. +func TestAPIKeyPersistence_ReloadSurvivesRestart(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "apikeys.json") + SetAPIKeyStatePath(path) + defer resetAPIKeyState() + + InitStores() + raw := "xce_ak_survives-restart" + AddAPIKey(raw, APIKeyEntry{ + Username: "advisor-svc", + RoleName: "advisor_agent", + CreateTime: time.Now(), + ExpiryTime: time.Now().Add(90 * 24 * time.Hour), + }) + + // Restart: brand-new store over the same state path. + InitStores() + + entry, ok := ValidateAPIKey(raw) + if !ok { + t.Fatal("key lost across restart — state file was not reloaded") + } + if entry.Username != "advisor-svc" || entry.RoleName != "advisor_agent" { + t.Errorf("entry = %+v, want advisor-svc/advisor_agent", entry) + } + // Role mapping must be restored too, or the key authenticates but is + // denied every permission check after restart. + roles := GetUserRoleNames("advisor-svc") + if len(roles) != 1 || roles[0] != "advisor_agent" { + t.Errorf("GetUserRoleNames(advisor-svc) = %v, want [advisor_agent]", roles) + } +} + +// TestAPIKeyPersistence_FileNeverContainsRawKey verifies the state file holds +// only hashes — reading it must not leak usable credentials. +func TestAPIKeyPersistence_FileNeverContainsRawKey(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "apikeys.json") + SetAPIKeyStatePath(path) + defer resetAPIKeyState() + + InitStores() + raw := "xce_ak_secret-never-on-disk" + AddAPIKey(raw, APIKeyEntry{Username: "mcp-bot", ExpiryTime: time.Now().Add(time.Hour)}) + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("state file missing: %v", err) + } + if string(data) != "" && containsPlain(data, raw) { + t.Error("state file contains the raw key — must store hashes only") + } + var state persistedAPIKeyState + if err := json.Unmarshal(data, &state); err != nil { + t.Fatalf("state file is not valid JSON: %v", err) + } + if len(state.Keys) != 1 { + t.Fatalf("state.Keys = %d entries, want 1", len(state.Keys)) + } + for hash := range state.Keys { + if hash != HashAPIKey(raw) { + t.Errorf("stored key %q is not the expected SHA-256 hash", hash) + } + } +} + +// TestAPIKeyPersistence_CorruptFileFailsFast verifies a corrupt state file +// makes InitStores panic instead of silently starting with zero keys (which +// would turn every service account into a mystery 401). +func TestAPIKeyPersistence_CorruptFileFailsFast(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "apikeys.json") + SetAPIKeyStatePath(path) + + if err := os.WriteFile(path, []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + defer resetAPIKeyState() + + defer func() { + if recover() == nil { + t.Error("InitStores did not fail on a corrupt state file") + } + }() + InitStores() +} + +// TestAPIKeyPersistence_MissingFileIsFreshInstall verifies a missing state +// file is the normal fresh-install case, not an error. +func TestAPIKeyPersistence_MissingFileIsFreshInstall(t *testing.T) { + SetAPIKeyStatePath(filepath.Join(t.TempDir(), "does-not-exist.json")) + defer resetAPIKeyState() + + InitStores() // must not panic +} + +// TestAPIKeyPersistence_SaveFailurePropagates verifies a state file that +// cannot be written makes AddAPIKey return an error (the controller turns +// this into a failed request rather than issuing a key that dies on restart). +func TestAPIKeyPersistence_SaveFailurePropagates(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "apikeys.json") + SetAPIKeyStatePath(path) + defer resetAPIKeyState() + + InitStores() + // A directory where the state file lives makes saveAPIKeyState's rename + // fail — os.Rename(file, dir) fails with EISDIR/ENOTDIR on Linux. + if err := os.Mkdir(path, 0o700); err != nil { + t.Fatal(err) + } + err := AddAPIKey("xce_ak_unwritable", APIKeyEntry{Username: "x"}) + if err == nil { + t.Error("AddAPIKey did not report the failed state save") + } +} + +func containsPlain(data []byte, needle string) bool { + return len(needle) > 0 && string(data) != "" && jsonContains(data, []byte(needle)) +} + +func jsonContains(haystack, needle []byte) bool { + for i := 0; i+len(needle) <= len(haystack); i++ { + if string(haystack[i:i+len(needle)]) == string(needle) { + return true + } + } + return false +} diff --git a/src/frontend/apiserver/store/stores.go b/src/frontend/apiserver/store/stores.go index 0fd9c2bb..603fc69f 100644 --- a/src/frontend/apiserver/store/stores.go +++ b/src/frontend/apiserver/store/stores.go @@ -44,6 +44,9 @@ func InitStores() { Roles = NewDataStore[string, RoleEntry]() UserRoles = NewDataStore[string, []string]() APIKeys = NewDataStore[string, APIKeyEntry]() + if err := loadAPIKeyState(); err != nil { + panic(err) // caller chooses to fail fast over issuing unusable credentials + } } // Helper functions for RBAC.