From e1d05e7b12eea68c0690ab9cefd157fdee80ce84 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:49:41 +0100 Subject: [PATCH] Make deploy enrollment credentials atomic --- internal/api/deploy_handlers.go | 62 +++++++++--------- internal/api/deploy_handlers_test.go | 96 ++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 30 deletions(-) diff --git a/internal/api/deploy_handlers.go b/internal/api/deploy_handlers.go index d29a359a0..e39dea15f 100644 --- a/internal/api/deploy_handlers.go +++ b/internal/api/deploy_handlers.go @@ -760,16 +760,18 @@ func (h *DeployHandlers) MintBootstrapTokenForTarget(req deploy.BootstrapTokenRe } config.Mu.Lock() + previousTokens := append([]config.APITokenRecord(nil), h.config.APITokens...) h.config.UpsertAPIToken(*record) - tokens := make([]config.APITokenRecord, len(h.config.APITokens)) - copy(tokens, h.config.APITokens) - config.Mu.Unlock() - if h.persistence != nil { - if err := h.persistence.SaveAPITokens(tokens); err != nil { - log.Warn().Err(err).Msg("Failed to persist bootstrap token") + if err := h.persistence.SaveAPITokens(h.config.APITokens); err != nil { + h.config.APITokens = previousTokens + h.config.SortAPITokens() + config.Mu.Unlock() + log.Error().Err(err).Msg("Failed to persist bootstrap token") + return "", "", fmt.Errorf("persist bootstrap token: %w", err) } } + config.Mu.Unlock() return raw, record.ID, nil } @@ -865,25 +867,9 @@ func (h *DeployHandlers) HandleEnroll(w http.ResponseWriter, r *http.Request) { return } - // 7. Invalidate bootstrap token (single-use) BEFORE minting runtime token. - // Check return value to prevent concurrent replay. - config.Mu.Lock() - removed := h.config.RemoveAPIToken(bootstrapToken.ID) - tokensAfterRemove := make([]config.APITokenRecord, len(h.config.APITokens)) - copy(tokensAfterRemove, h.config.APITokens) - config.Mu.Unlock() - if removed == nil { - writeErrorResponse(w, http.StatusConflict, "token_already_consumed", - "Bootstrap token has already been used", nil) - return - } - if h.persistence != nil { - if err := h.persistence.SaveAPITokens(tokensAfterRemove); err != nil { - log.Warn().Err(err).Msg("Failed to persist token removal during enroll") - } - } - - // 8. Mint runtime token (long-lived, host-bound). + // 7. Prepare the long-lived, host-bound runtime token before consuming the + // bootstrap token. The credential is not admitted to live state until the + // atomic replacement below succeeds. runtimeRaw, err := auth.GenerateAPIToken() if err != nil { log.Error().Err(err).Msg("Failed to generate runtime token during enroll") @@ -914,16 +900,32 @@ func (h *DeployHandlers) HandleEnroll(w http.ResponseWriter, r *http.Request) { } setAPITokenOwnerUserID(runtimeRecord, apiTokenOwnerUserID(*bootstrapToken)) + // 8. Replace the single-use bootstrap token with the runtime token in one + // persisted transition. Holding the config lock through persistence prevents + // concurrent enrollment from consuming the same token and prevents unrelated + // token mutations from being omitted from the saved snapshot. config.Mu.Lock() + previousTokens := append([]config.APITokenRecord(nil), h.config.APITokens...) + removed := h.config.RemoveAPIToken(bootstrapToken.ID) + if removed == nil { + config.Mu.Unlock() + writeErrorResponse(w, http.StatusConflict, "token_already_consumed", + "Bootstrap token has already been used", nil) + return + } h.config.UpsertAPIToken(*runtimeRecord) - tokensAfterMint := make([]config.APITokenRecord, len(h.config.APITokens)) - copy(tokensAfterMint, h.config.APITokens) - config.Mu.Unlock() if h.persistence != nil { - if err := h.persistence.SaveAPITokens(tokensAfterMint); err != nil { - log.Warn().Err(err).Msg("Failed to persist runtime token during enroll") + if err := h.persistence.SaveAPITokens(h.config.APITokens); err != nil { + h.config.APITokens = previousTokens + h.config.SortAPITokens() + config.Mu.Unlock() + log.Error().Err(err).Msg("Failed to persist deploy enrollment token replacement") + writeErrorResponse(w, http.StatusInternalServerError, "token_persistence_error", + "Failed to persist enrollment credentials", nil) + return } } + config.Mu.Unlock() // 9. Update target status to VERIFYING. _ = h.store.UpdateTargetStatus(ctx, targetID, deploy.TargetVerifying, "") diff --git a/internal/api/deploy_handlers_test.go b/internal/api/deploy_handlers_test.go index a4e4cd95a..78e888cd4 100644 --- a/internal/api/deploy_handlers_test.go +++ b/internal/api/deploy_handlers_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" "testing" @@ -522,6 +523,10 @@ func TestHandleEnroll_Success(t *testing.T) { jobID, targetID := seedEnrollJobAndTarget(t, store, deploy.TargetEnrolling) rec := mintTestBootstrapToken(t, h.config, jobID, targetID, "pve-node2") rec.Metadata[apiTokenMetadataOwnerUserID] = "alice" + h.persistence = config.NewConfigPersistence(h.config.DataPath) + if err := h.persistence.SaveAPITokens(h.config.APITokens); err != nil { + t.Fatalf("persist bootstrap token: %v", err) + } req := httptest.NewRequest(http.MethodPost, "/api/agents/agent/enroll", enrollJSON(t, "pve-node2")) attachAPITokenRecord(req, rec) @@ -563,6 +568,13 @@ func TestHandleEnroll_Success(t *testing.T) { if got := runtimeRecord.Metadata[apiTokenMetadataOwnerUserID]; got != "alice" { t.Fatalf("runtime token owner_user_id = %q, want alice", got) } + persistedTokens, err := h.persistence.LoadAPITokens() + if err != nil { + t.Fatalf("load persisted tokens: %v", err) + } + if len(persistedTokens) != 1 || persistedTokens[0].ID != runtimeTokenID { + t.Fatalf("persisted enrollment transition = %+v, want only runtime token %q", persistedTokens, runtimeTokenID) + } // Target should now be verifying. target, err := store.GetTarget(context.Background(), targetID) @@ -600,6 +612,49 @@ func TestHandleEnroll_InstallingState(t *testing.T) { } } +func TestHandleEnroll_RollsBackBootstrapConsumptionWhenPersistenceFails(t *testing.T) { + h, store := newEnrollTestHandlers(t) + jobID, targetID := seedEnrollJobAndTarget(t, store, deploy.TargetEnrolling) + bootstrap := mintTestBootstrapToken(t, h.config, jobID, targetID, "pve-node2") + + statePath := filepath.Join(t.TempDir(), "blocked-state") + h.persistence = config.NewConfigPersistence(statePath) + if err := os.RemoveAll(statePath); err != nil { + t.Fatalf("remove persistence directory: %v", err) + } + if err := os.WriteFile(statePath, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("create persistence blocker: %v", err) + } + + req := httptest.NewRequest(http.MethodPost, "/api/agents/agent/enroll", enrollJSON(t, "pve-node2")) + attachAPITokenRecord(req, bootstrap) + rr := httptest.NewRecorder() + h.HandleEnroll(rr, req) + + if rr.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want %d (body=%q)", rr.Code, http.StatusInternalServerError, rr.Body.String()) + } + + config.Mu.Lock() + tokens := append([]config.APITokenRecord(nil), h.config.APITokens...) + primaryToken := h.config.APIToken + config.Mu.Unlock() + if len(tokens) != 1 || tokens[0].ID != bootstrap.ID { + t.Fatalf("bootstrap token was not restored exactly: %+v", tokens) + } + if primaryToken != bootstrap.Hash { + t.Fatalf("legacy primary token = %q, want restored bootstrap hash", primaryToken) + } + + target, err := store.GetTarget(context.Background(), targetID) + if err != nil { + t.Fatalf("get target: %v", err) + } + if target.Status != deploy.TargetEnrolling { + t.Fatalf("target status = %q, want %q", target.Status, deploy.TargetEnrolling) + } +} + func TestHandleEnroll_MissingHostname(t *testing.T) { h, _ := newEnrollTestHandlers(t) @@ -788,6 +843,47 @@ func TestMintBootstrapTokenForTarget(t *testing.T) { } } +func TestMintBootstrapTokenForTarget_RollsBackWhenPersistenceFails(t *testing.T) { + statePath := filepath.Join(t.TempDir(), "blocked-state") + persistence := config.NewConfigPersistence(statePath) + if err := os.RemoveAll(statePath); err != nil { + t.Fatalf("remove persistence directory: %v", err) + } + if err := os.WriteFile(statePath, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("create persistence blocker: %v", err) + } + + existing := config.APITokenRecord{ + ID: "existing", Name: "existing", Hash: "existing-hash", + CreatedAt: time.Now().UTC(), Scopes: []string{config.ScopeWildcard}, + } + cfg := &config.Config{APITokens: []config.APITokenRecord{existing}} + cfg.SortAPITokens() + h := &DeployHandlers{ + config: cfg, + persistence: persistence, + sseSubs: make(map[string]*deploySSESub), + } + + raw, tokenID, err := h.MintBootstrapTokenForTarget(deploy.BootstrapTokenRequest{ + ClusterID: "c1", NodeID: "n1", ExpectedNode: "pve-3", + JobID: "job-m1", TargetID: "tgt-m1", SourceAgentID: "agent-src", + OrgID: "test-org", TTL: 15 * time.Minute, + }, "alice") + if err == nil { + t.Fatal("expected persistence failure") + } + if raw != "" || tokenID != "" { + t.Fatalf("failed mint returned credential material: raw=%q id=%q", raw, tokenID) + } + if len(cfg.APITokens) != 1 || cfg.APITokens[0].ID != existing.ID { + t.Fatalf("live tokens were not rolled back: %+v", cfg.APITokens) + } + if cfg.APIToken != existing.Hash { + t.Fatalf("legacy primary token = %q, want %q", cfg.APIToken, existing.Hash) + } +} + func TestMintBootstrapTokenForTarget_InvalidTTL(t *testing.T) { cfg := &config.Config{DataPath: t.TempDir()} h := &DeployHandlers{