diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c09c14d..415334cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,4 +4,7 @@ ### Fixed +- 自更新恢复在改写文件前严格验证恢复记录、State、提交和二进制配对;拒绝缺失、重复或矛盾证据,保留可恢复现场。 +- 编辑已停止的外部 MCP 不再意外启动连接;启动/停止、配置编辑和应用重启保持一致的启停状态。 + - 登录密码显隐按钮改为输入框内的眼睛图标,统一输入框宽高,支持键盘焦点、44px 触控区域、中英俄文无障碍标签及浅色/深色主题。 diff --git a/docs/en-US/developer-guide.md b/docs/en-US/developer-guide.md index f503e9ce..96081f18 100644 --- a/docs/en-US/developer-guide.md +++ b/docs/en-US/developer-guide.md @@ -250,3 +250,9 @@ High-value tests: - One-click update: `internal/update/` (status, apply, rollback), HTTP surface in `internal/handler/update.go`, CLI switches in `cmd/server/update_cli.go`, thin shell in `upgrade.sh` + +### Recovery evidence and MCP activation edits + +Self-update recovery validates JSON field names/types, duplicate keys, commit/hash relationships and the actual binary files before resuming filesystem changes. The plain-text legacy build marker remains supported, but any accompanying State is validated too. An ambiguous, incomplete or unsupported record is refused with the files and recovery evidence preserved. Do not delete a pending record just to bypass the refusal: first reconcile the source commit, live binary and previous binary/backup. + +Editing an existing external MCP without activation fields preserves its current enabled/stopped state. Explicit `disabled` or `external_mcp_enable` fields change activation; `disabled: true` takes precedence. A new server defaults to enabled. Start/Stop also persist the corresponding disabled state, so editing or restarting the application does not silently enable a stopped server. diff --git a/docs/feature/self-update-recovery-fixes.md b/docs/feature/self-update-recovery-fixes.md new file mode 100644 index 00000000..72e5c3a7 --- /dev/null +++ b/docs/feature/self-update-recovery-fixes.md @@ -0,0 +1,63 @@ +# Self-update recovery and MCP edit fixes + +## Intake (F0) + +PR #346 was merged at 72cd8d62e36e536a6b18e9e10ea73d8d16294044 on explicit user direction to merge first and repair the reviewed defects afterwards. This is corrective work on branch `codex/self-update-recovery-fixes`; the checkout was clean. No release or deployment is requested. + +Workflow gate: P10 feedback/iteration, implementation allowed. The frozen review at a63070d and executable PRR-002/PRR-007 counterexamples define the upstream contract; there are no missing product decisions. Acceptance requires preserved MCP stop state and refusal of ambiguous recovery before filesystem mutation. + +Maintainability gate: `apply.go` has 1099 lines and combines recovery orchestration and filesystem IO; `external_mcp.go` has 508 lines. Risk is high for recovery. Allowed slice: narrow fix with a separate pure JSON/evidence validation module, small call-site changes, and focused regression tests. Broad refactoring is not required. Existing update/handler tests and reviewer reproductions cover the affected boundary. + +## Requirements (F1, confirmed by the user's repair request) + +- R1 / PRR-007: editing an existing MCP without an explicit enable/disable field preserves its runtime activation state. Explicit activation fields and Start/Stop remain supported; a new server defaults to enabled. Environment expansion remains isolated from persisted raw values. +- R2 / PRR-002: incomplete, null, duplicate-key, malformed, or contradictory recovery evidence cannot authorize a second swap, wrong rollback binary, or deletion of recovery records. Valid current records and the documented legacy marker retain safe recovery. +- Non-goals: redesigning update UX, changing release policy, or unrelated refactoring. The two nonblocking review observations are outside this corrective slice unless required by the evidence validation boundary. + +## Design (F2) + +Recovery remains an internal on-disk contract, with no new HTTP fields. Decode exact JSON field names and types, reject duplicate keys at every depth and null scalars, and reject unknown fields rather than interpreting a newer format as current evidence. The one documented plain-text legacy build marker remains supported by its existing conservative path. + +For JSON build records require a target commit and exactly one proof of the old binary (SHA256 or explicit absence), validate hash/commit shapes, and reconcile the record with any persisted State for the same update or its immediate predecessor. Before restoring protected content or building, verify the live/previous files match the indicated pre-swap or post-swap phase. A successful State cannot be reinterpreted as an owed build. + +Rollback records require an explicit restore_binary decision; reconcile it, the live/previous hashes and the nested State, then compare with the persisted State when it still exists. Missing persisted State is allowed after the final cleanup crash window, but the self-contained record must remain coherent. Invalid evidence returns an existing state_unreadable/bad_state error and preserves recovery files. + +MCP request decoding retains field presence separately from bool values. For existing entries, omission preserves the manager's activation state; explicit disabled or external_mcp_enable fields determine it. New entries default to enabled. Keep raw configuration persistence and environment-expanded connection snapshots separate as today. + +Compatibility: properly written current journals remain readable, legacy plain-text recovery remains guarded; hand-edited or unsupported JSON requires manual correction instead of guessed mutation. Rollback of this patch restores old behavior and is not recommended while invalid journals remain. + +## Implementation plan (F3) + +1. Add recovery evidence parser/validators in a separate internal/update module; add narrow calls in build/rollback readers and pre-mutation paths. Keep producer formats unchanged. +2. Preserve explicit field presence in the MCP request without changing its JSON shape; normalize enable state against the existing manager configuration. +3. Promote the two failing review reproductions into portable regression tests; add duplicate/null/type/contradiction matrices and valid retry controls. Assert no build/swap, unchanged binaries and preserved journals for refusals. +4. Run update/handler targeted suites and race checks, full Go tests, build, JS and shell checks; update changelog and verification record. Open a corrective PR and integrate the verified patch as part of the authorized merge-and-fix task, respecting repository checks. + +Docs: this phase record plus concise behavior notes in deployment/developer documentation if needed. No screenshots or production external services are required; tests use local Git and harmless command/HTTP markers. No version bump, tag or release publication. + +## Implementation (F4) + +Implemented strict recovery decoding in recovery_evidence.go, with exact field names, nested duplicate/type/null checks, commit/hash validation and State reconciliation. Build recovery verifies pre-move versus post-move binary evidence before content restore and rechecks files after compilation. Fresh rollback and legacy-marker recovery also read State strictly. Self-contained rollback journals validate absence/hash coherence even after State cleanup, and actual absence is checked before any cleanup. + +MCP edits distinguish omitted activation fields from explicit booleans. Omitted fields preserve the manager state; Start/Stop and persisted disabled are synchronized so restart retains the requested state. Existing environment snapshot expansion is preserved. + +Tests added for malformed build/rollback journals, all independent review counterexamples, valid partial binary rename and final cleanup retries, and MCP stop/edit/start plus config reload. Targeted reproductions pass; original full Go, build, five-package race, and 277 JS checks passed before final evidence guard refinements. Final exact-patch validation follows in F5. No production external service was contacted. + +### F4 follow-up: activation round-trip consistency + +Final GET-to-edit inspection found that manager Start/Stop retained a stale disabled flag even after changing ExternalMCPEnable. Synchronize these two existing fields in the manager as well as persistence, and test the actual GET payload after Start followed by the UI-shaped edit. Maintainability assessment: external_manager.go is 1665 lines (high risk); this narrow fix adds only three assignments beside existing activation assignments, without new control flow or broad refactoring. The additional handler/MCP race suites validate this consistency change. + +## Verification and user documentation (F5) + +- Recovery code at fab7e603: `go test -race ./internal/update ./internal/handler` passed (84.580s / 16.692s). This includes malformed/null/duplicate/contradictory records, legacy and fresh State entry points, actual absence, pre-merge phase rejection and valid crash controls. +- Final activation consistency change at ff50af02: `go test -race ./internal/handler ./internal/mcp -count=1` passed (18.658s / 3.281s), including Stop/edit/reload/Start/GET/edit behavior. Independent targeted handler tests also passed. +- Full Go suite, `go build ./...`, 277 JS tests and shell syntax passed during implementation. A frozen-code full Go/build rerun and exact-head GitHub CI are tracked in F6 before integration. +- Independent fresh-context code review at ff50af02 found no remaining blockers. Six independent recovery counterexamples were promoted to permanent tests and passed. No acceptance relies solely on test status: refusals assert retained journals/binary data, and valid controls prove continued recovery. +- English and Chinese developer guides describe refusal/reconciliation behavior and MCP activation field precedence. No public request shape or release version changed. +- Limits: local validation is macOS arm64; Windows/Linux results come from CI. No production restart/deployment smoke or exhaustive disk fault testing was performed. + +## Review and integration record (F6) + +Corrective PR: https://github.com/AIPentest/CyberStrikeAI/pull/347 . Code snapshot ff50af028f87465954911533ac07b420c92767b8 passed independent review; subsequent changes are documentation/changelog only. PRR-002 and PRR-007 are closed by the recorded implementation and regression evidence. The Unreleased changelog records both user-visible fixes. Final integration requires exact-head cross-platform CI success and unchanged reviewed production code; no tag or deployment is part of this task. + +The earlier nonblocking version-display crash window and recovery-path symlink observation from PR #346 remain follow-up items, outside the two requested blocker fixes. No broader reliability guarantee is inferred from these tests. diff --git a/docs/zh-CN/developer-guide.md b/docs/zh-CN/developer-guide.md index cb46a58d..c9d752bf 100644 --- a/docs/zh-CN/developer-guide.md +++ b/docs/zh-CN/developer-guide.md @@ -290,3 +290,9 @@ i18n 规范见 [前端国际化方案](frontend-i18n.md)。 - SQLite 迁移兼容。 这些地方比普通 getter/setter 更容易出现真实用户故障。 + +### 恢复证据与 MCP 编辑启停状态 + +自更新恢复在改写文件前验证 JSON 字段名称/类型、重复键、提交与哈希配对及实际二进制文件。旧版纯文本待编译标记仍受支持,但关联 State 也必须通过验证。记录不完整、矛盾或格式不受支持时会拒绝恢复并保留文件与恢复证据。不要仅为绕过报错而删除标记,应先核对源码提交、当前二进制和旧二进制/备份的配对。 + +编辑已有外部 MCP 时,省略启停字段会保留当前启用/停止状态。显式 `disabled` 或 `external_mcp_enable` 可改变启停状态,`disabled: true` 优先;新建 MCP 默认启用。启动/停止按钮同时持久化对应的 disabled 状态,避免编辑或应用重启悄悄启用已停止的服务。 diff --git a/internal/handler/external_mcp.go b/internal/handler/external_mcp.go index 0da18210..42cfb41b 100644 --- a/internal/handler/external_mcp.go +++ b/internal/handler/external_mcp.go @@ -179,13 +179,10 @@ func (h *ExternalMCPHandler) AddOrUpdateExternalMCP(c *gin.Context) { cfg := req.Config - // 官方 disabled 字段 → ExternalMCPEnable 取反 - if cfg.Disabled { - cfg.ExternalMCPEnable = false - } else if !cfg.ExternalMCPEnable { - // 用户未显式设置 external_mcp_enable,官方配置默认就是启用的 - cfg.ExternalMCPEnable = true - } + // Editing metadata does not change activation unless the request says so. + previous, exists := h.manager.GetConfigs()[name] + cfg.ExternalMCPEnable = req.activation(previous.ExternalMCPEnable, exists) + cfg.Disabled = !cfg.ExternalMCPEnable // 展开 ${VAR} 环境变量 config.ExpandConfigEnv(&cfg) @@ -272,6 +269,7 @@ func (h *ExternalMCPHandler) StartExternalMCP(c *gin.Context) { } cfg := h.config.ExternalMCP.Servers[name] cfg.ExternalMCPEnable = true + cfg.Disabled = false h.config.ExternalMCP.Servers[name] = cfg // 保存到配置文件 @@ -326,6 +324,7 @@ func (h *ExternalMCPHandler) StopExternalMCP(c *gin.Context) { } cfg := h.config.ExternalMCP.Servers[name] cfg.ExternalMCPEnable = false + cfg.Disabled = true h.config.ExternalMCP.Servers[name] = cfg // 保存到配置文件 @@ -496,7 +495,9 @@ func setStringArrayInMap(mapNode *yaml.Node, key string, values []string) { // AddOrUpdateExternalMCPRequest 添加或更新外部MCP请求 type AddOrUpdateExternalMCPRequest struct { - Config config.ExternalMCPServerConfig `json:"config"` + Config config.ExternalMCPServerConfig `json:"config"` + enable *bool + disabled *bool } // ExternalMCPResponse 外部MCP响应 diff --git a/internal/handler/external_mcp_activation_test.go b/internal/handler/external_mcp_activation_test.go new file mode 100644 index 00000000..21ff100f --- /dev/null +++ b/internal/handler/external_mcp_activation_test.go @@ -0,0 +1,129 @@ +package handler + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "cyberstrike-ai/internal/config" +) + +func TestMCPEditPreservesStoppedState(t *testing.T) { + router, h, path := setupTestRouter() + defer cleanupTestConfig(path) + defer h.manager.StopAll() + var calls atomic.Int64 + called := make(chan struct{}, 16) + endpoint := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + select { + case called <- struct{}{}: + default: + } + http.Error(w, "test endpoint", 503) + })) + defer endpoint.Close() + cfg := config.ExternalMCPServerConfig{Type: "http", URL: endpoint.URL, Description: "before"} + h.config.ExternalMCP.Servers["stopped"] = cfg + if err := h.manager.AddOrUpdateConfig("stopped", cfg); err != nil { + t.Fatal(err) + } + request := func(method, url string, body []byte) *httptest.ResponseRecorder { + t.Helper() + w := httptest.NewRecorder() + r := httptest.NewRequest(method, url, bytes.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, r) + if w.Code != 200 { + t.Fatalf("%s %s: %d %s", method, url, w.Code, w.Body) + } + return w + } + request("POST", "/api/external-mcp/stopped/stop", nil) + // Shape emitted by the edit UI: enable omitted, descriptive metadata changed. + body, _ := json.Marshal(map[string]any{"config": map[string]any{"type": "http", "url": endpoint.URL, "description": "after"}}) + request("PUT", "/api/external-mcp/stopped", body) + if h.manager.GetConfigs()["stopped"].ExternalMCPEnable { + t.Fatal("metadata edit enabled a stopped MCP") + } + persisted, err := config.Load(path) + if err != nil { + t.Fatal(err) + } + if persisted.ExternalMCP.Servers["stopped"].ExternalMCPEnable { + t.Fatal("stop state lost on config reload") + } + select { + case <-called: + t.Fatal("metadata edit initiated a connection") + case <-time.After(100 * time.Millisecond): + } + if calls.Load() != 0 { + t.Fatal("stopped edit started external work") + } + request("POST", "/api/external-mcp/stopped/start", nil) + select { + case <-called: + case <-time.After(3 * time.Second): + t.Fatal("explicit Start did not connect") + } + if !h.manager.GetConfigs()["stopped"].ExternalMCPEnable { + t.Fatal("explicit Start stayed disabled") + } + response := request("GET", "/api/external-mcp/stopped", nil) + var server ExternalMCPResponse + if err := json.Unmarshal(response.Body.Bytes(), &server); err != nil { + t.Fatal(err) + } + if server.Config.Disabled { + t.Fatal("Start left a stale disabled field in the edit response") + } + // Round-trip the actual GET payload just as the UI does, removing enable. + encoded, _ := json.Marshal(server.Config) + var edit map[string]any + _ = json.Unmarshal(encoded, &edit) + delete(edit, "external_mcp_enable") + edit["description"] = "after explicit start" + body, _ = json.Marshal(map[string]any{"config": edit}) + request("PUT", "/api/external-mcp/stopped", body) + if !h.manager.GetConfigs()["stopped"].ExternalMCPEnable { + t.Fatal("edit after Start stopped the MCP") + } + persisted, err = config.Load(path) + if err != nil { + t.Fatal(err) + } + if !persisted.ExternalMCP.Servers["stopped"].ExternalMCPEnable { + t.Fatal("explicit Start not persisted") + } +} + +func TestMCPRequestActivationPresence(t *testing.T) { + for _, tc := range []struct { + name, fields string + previous, exists, want bool + }{ + {"new default", "", false, false, true}, + {"stopped edit", "", false, true, false}, + {"running edit", "", true, true, true}, + {"explicit enable", `,"external_mcp_enable":true`, false, true, true}, + {"explicit stop", `,"external_mcp_enable":false`, true, true, false}, + {"explicit disabled", `,"disabled":true`, true, true, false}, + {"explicit enabled official", `,"disabled":false`, false, true, true}, + {"disabled wins", `,"disabled":true,"external_mcp_enable":true`, true, true, false}, + } { + t.Run(tc.name, func(t *testing.T) { + var r AddOrUpdateExternalMCPRequest + if err := json.Unmarshal([]byte(`{"config":{"description":"edit"`+tc.fields+`}}`), &r); err != nil { + t.Fatal(err) + } + if got := r.activation(tc.previous, tc.exists); got != tc.want { + t.Fatalf("activation=%v want=%v", got, tc.want) + } + }) + } +} diff --git a/internal/handler/external_mcp_request.go b/internal/handler/external_mcp_request.go new file mode 100644 index 00000000..deebae8d --- /dev/null +++ b/internal/handler/external_mcp_request.go @@ -0,0 +1,41 @@ +package handler + +import "encoding/json" + +// Keep omission distinct from false without changing the public request shape. +func (r *AddOrUpdateExternalMCPRequest) UnmarshalJSON(data []byte) error { + type plain AddOrUpdateExternalMCPRequest + var decoded plain + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + var presence struct { + Config struct { + Enable *bool `json:"external_mcp_enable"` + Disabled *bool `json:"disabled"` + } `json:"config"` + } + if err := json.Unmarshal(data, &presence); err != nil { + return err + } + *r = AddOrUpdateExternalMCPRequest(decoded) + r.enable = presence.Config.Enable + r.disabled = presence.Config.Disabled + return nil +} + +func (r AddOrUpdateExternalMCPRequest) activation(previous, exists bool) bool { + if r.disabled != nil && *r.disabled { + return false + } + if r.enable != nil { + return *r.enable + } + if r.disabled != nil { + return !*r.disabled + } + if exists { + return previous + } + return true +} diff --git a/internal/mcp/external_manager.go b/internal/mcp/external_manager.go index f708b3d1..b1d8f850 100644 --- a/internal/mcp/external_manager.go +++ b/internal/mcp/external_manager.go @@ -337,6 +337,7 @@ func (m *ExternalMCPManager) startClient(name string, autoReconnect bool) error if !autoReconnect { m.mu.Lock() serverCfg.ExternalMCPEnable = true + serverCfg.Disabled = false m.configs[name] = serverCfg m.mu.Unlock() } @@ -362,6 +363,7 @@ func (m *ExternalMCPManager) startClient(name string, autoReconnect bool) error // 更新配置为启用 m.mu.Lock() serverCfg.ExternalMCPEnable = true + serverCfg.Disabled = false m.configs[name] = serverCfg // 清除之前的错误信息(重新启动时) delete(m.errors, name) @@ -443,6 +445,7 @@ func (m *ExternalMCPManager) StopClient(name string) error { // 更新配置为禁用 serverCfg.ExternalMCPEnable = false + serverCfg.Disabled = true m.configs[name] = serverCfg m.clearReconnectState(name) diff --git a/internal/update/apply.go b/internal/update/apply.go index 8b7f2ddf..8d30f52e 100644 --- a/internal/update/apply.go +++ b/internal/update/apply.go @@ -73,12 +73,18 @@ func readBuildPending(root string) (buildPendingState, bool, error) { if err != nil { return st, true, err } - if err := json.Unmarshal(data, &st); err != nil { + if err := decodeRecoveryJSON(data, &st); err != nil { if strings.TrimSpace(string(data)) == "source is ahead of the binary" { + if _, _, stateErr := recoveryState(root); stateErr != nil { + return st, true, stateErr + } return buildPendingState{Legacy: true}, true, nil } return buildPendingState{}, true, fmt.Errorf("待编译状态格式损坏: %w", err) } + if err := validateBuildEvidence(root, st); err != nil { + return st, true, err + } return st, true, nil } @@ -476,6 +482,14 @@ func finishPendingBuild(ctx context.Context, snap *Snapshot, opts Options, res * if err != nil { return &Error{Reason: "bad_state", Message: "待编译状态里的提交不存在:" + pending.Commit} } + fileEvidence := pending + if target != head { + // Before the source move only the original binary/absence is valid. + fileEvidence.BinarySHA256 = "" + } + if err := validateBuildFiles(bin, fileEvidence); err != nil { + return &Error{Reason: "bad_state", Message: "恢复证据不足,已保留二进制和记录:" + err.Error()} + } if target != head { if pending.PreviousCommit != "" { previous, previousErr := gitCmd(ctx, snap.Root, "rev-parse", pending.PreviousCommit) @@ -585,6 +599,9 @@ func buildAndSwap(ctx context.Context, root string, opts Options, res *Result, s return &Error{Reason: "build_failed", Message: "无法校验新二进制:" + err.Error()} } bin := filepath.Join(root, opts.binaryName()) + if err := validateBuildFiles(bin, pending); err != nil { + return &Error{Reason: "binary_changed", Message: "编译后恢复证据不再匹配,已保留现场:" + err.Error()} + } if pending.Legacy && fileExists(bin) && fileExists(bin+".prev") { currentHash, currentErr := fileSHA256(bin) previousHash, previousErr := fileSHA256(bin + ".prev") @@ -917,7 +934,10 @@ func Rollback(ctx context.Context, opts Options) (*Result, error) { return nil, &Error{Reason: "moved_since_update", Message: "回滚中断后 HEAD 已经变了,拒绝覆盖后续提交"} } } - st, ok := readState(root) + st, ok, stateErr := recoveryState(root) + if stateErr != nil { + return nil, &Error{Reason: "state_unreadable", Message: "无法读取回滚状态:" + stateErr.Error()} + } if !ok { return nil, &Error{Reason: "no_state", Message: "没有可回滚的更新记录"} } diff --git a/internal/update/recovery_evidence.go b/internal/update/recovery_evidence.go new file mode 100644 index 00000000..58b0368a --- /dev/null +++ b/internal/update/recovery_evidence.go @@ -0,0 +1,260 @@ +package update + +import ( + "bytes" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "os" + "reflect" + "strings" +) + +// Recovery JSON authorizes filesystem mutations. Unlike encoding/json's default +// behavior, repeated keys, case aliases, unknown fields and null scalars must not +// silently replace or erase evidence. Slice nulls are emitted by our own writer. +func decodeRecoveryJSON(data []byte, dst any) error { + dec := json.NewDecoder(bytes.NewReader(data)) + if err := checkRecoveryValue(dec, reflect.TypeOf(dst).Elem()); err != nil { + return err + } + if _, err := dec.Token(); err != io.EOF { + return fmt.Errorf("trailing recovery data: %v", err) + } + return json.Unmarshal(data, dst) +} + +func checkRecoveryValue(dec *json.Decoder, typ reflect.Type) error { + token, err := dec.Token() + if err != nil { + return err + } + if token == nil { + if typ.Kind() == reflect.Slice { + return nil + } + return fmt.Errorf("null recovery %s", typ) + } + switch typ.Kind() { + case reflect.Struct: + if token != json.Delim('{') { + return fmt.Errorf("expected recovery object") + } + fields := map[string]reflect.Type{} + for i := 0; i < typ.NumField(); i++ { + f := typ.Field(i) + name := strings.Split(f.Tag.Get("json"), ",")[0] + if name != "" && name != "-" { + fields[name] = f.Type + } + } + seen := map[string]bool{} + for dec.More() { + key, err := dec.Token() + if err != nil { + return err + } + name, ok := key.(string) + if !ok { + return fmt.Errorf("invalid recovery key") + } + ft, known := fields[name] + if !known || seen[name] { + return fmt.Errorf("unknown or duplicate recovery field %q", name) + } + seen[name] = true + if err := checkRecoveryValue(dec, ft); err != nil { + return fmt.Errorf("%s: %w", name, err) + } + } + _, err = dec.Token() + return err + case reflect.Slice: + if token != json.Delim('[') { + return fmt.Errorf("expected recovery array") + } + for dec.More() { + if err := checkRecoveryValue(dec, typ.Elem()); err != nil { + return err + } + } + _, err = dec.Token() + return err + case reflect.String: + if _, ok := token.(string); !ok { + return fmt.Errorf("expected recovery string") + } + case reflect.Bool: + if _, ok := token.(bool); !ok { + return fmt.Errorf("expected recovery boolean") + } + default: + return fmt.Errorf("unsupported recovery type %s", typ) + } + return nil +} + +func validDigest(value string, sizes ...int) bool { + for _, size := range sizes { + if len(value) == size { + _, err := hex.DecodeString(value) + return err == nil && value == strings.ToLower(value) + } + } + return false +} + +// A State from the previous update may coexist with a newly written build +// marker. Read it strictly, and distinguish absent from damaged evidence. +func recoveryState(root string) (State, bool, error) { + var st State + data, err := os.ReadFile(statePath(root)) + if os.IsNotExist(err) { + return st, false, nil + } + if err != nil { + return st, false, err + } + if err := decodeRecoveryJSON(data, &st); err != nil { + return st, true, err + } + return st, true, validateRecoveryState(st) +} + +func validateRecoveryState(st State) error { + if !validDigest(st.PreviousCommit, 40, 64) || !validDigest(st.UpdatedCommit, 40, 64) { + return fmt.Errorf("invalid state commits") + } + for _, hash := range []string{st.BinarySHA256, st.PreviousBinarySHA256} { + if hash != "" && !validDigest(hash, 64) { + return fmt.Errorf("invalid state binary hash") + } + } + if st.PreviousBinaryAbsent && st.BinaryCommit == st.PreviousCommit && st.BinarySHA256 != "" { + return fmt.Errorf("absent previous binary has a live hash") + } + if st.PreviousBinaryAbsent && st.PreviousBinarySHA256 != "" { + return fmt.Errorf("state contradicts binary absence") + } + return nil +} + +func validateBuildEvidence(root string, p buildPendingState) error { + if !validDigest(p.Commit, 40, 64) || (p.PreviousCommit != "" && (!validDigest(p.PreviousCommit, 40, 64) || p.PreviousCommit == p.Commit)) { + return fmt.Errorf("invalid build commits") + } + if p.PreviousBinaryAbsent == (p.PreviousBinarySHA256 != "") { + return fmt.Errorf("build record must prove previous binary hash or absence") + } + if p.PreviousBinarySHA256 != "" && !validDigest(p.PreviousBinarySHA256, 64) { + return fmt.Errorf("invalid previous binary hash") + } + if p.BinarySHA256 != "" && !validDigest(p.BinarySHA256, 64) { + return fmt.Errorf("invalid built binary hash") + } + st, present, err := recoveryState(root) + if err != nil || !present { + return err + } + if st.BinaryCommit != "" && st.BinaryCommit != st.PreviousCommit && st.BinaryCommit != st.UpdatedCommit { + return fmt.Errorf("state binary commit is unrelated") + } + switch { + case st.UpdatedCommit == p.Commit: + if st.PreviousCommit != p.PreviousCommit || st.PreviousBinarySHA256 != p.PreviousBinarySHA256 || st.PreviousBinaryAbsent != p.PreviousBinaryAbsent { + return fmt.Errorf("build record contradicts rollback state") + } + if st.BinaryCommit == p.PreviousCommit && st.BinarySHA256 != p.PreviousBinarySHA256 { + return fmt.Errorf("pre-build state contradicts previous binary hash") + } + if st.BinaryCommit == p.Commit && (p.BinarySHA256 == "" || st.BinarySHA256 != p.BinarySHA256) { + return fmt.Errorf("completed build lacks matching recovery hash") + } + case st.UpdatedCommit == p.PreviousCommit: + if st.BinaryCommit != st.UpdatedCommit || st.BinarySHA256 == "" || st.BinarySHA256 != p.PreviousBinarySHA256 || p.PreviousBinaryAbsent { + return fmt.Errorf("previous update does not prove the current binary") + } + default: + return fmt.Errorf("build record belongs to a different update") + } + return nil +} + +func validateBuildFiles(bin string, p buildPendingState) error { + if p.Legacy { + return nil + } // The legacy path separately proves identity from State/VCS. + live, err := fileSHA256(bin) + if err == nil && p.BinarySHA256 != "" && live == p.BinarySHA256 { + if !p.PreviousBinaryAbsent { + previous, e := fileSHA256(bin + ".prev") + if e != nil || previous != p.PreviousBinarySHA256 { + return fmt.Errorf("completed swap has no matching rollback binary") + } + } + return nil + } + if p.PreviousBinaryAbsent { + if !os.IsNotExist(err) { + return fmt.Errorf("binary exists despite recorded absence") + } + return nil + } + if err == nil && live == p.PreviousBinarySHA256 { + return nil + } + // installBinary can crash between moving live to .prev and installing staged. + if os.IsNotExist(err) && p.BinarySHA256 != "" { + previous, e := fileSHA256(bin + ".prev") + if e == nil && previous == p.PreviousBinarySHA256 { + return nil + } + } + return fmt.Errorf("live binary does not match build recovery evidence") +} + +func validateRollbackEvidence(root string, p rollbackPendingState) error { + st := p.State + if err := validateRecoveryState(st); err != nil { + return err + } + if !validDigest(st.PreviousCommit, 40, 64) || !validDigest(st.UpdatedCommit, 40, 64) || st.PreviousCommit == st.UpdatedCommit { + return fmt.Errorf("invalid rollback commits") + } + if p.LiveSHA256 != "" && !validDigest(p.LiveSHA256, 64) { + return fmt.Errorf("invalid live hash") + } + if p.RestoreBinary { + if !validDigest(p.LiveSHA256, 64) || !validDigest(p.PreviousSHA256, 64) || st.PreviousBinaryAbsent { + return fmt.Errorf("rollback lacks binary evidence") + } + if st.BinaryCommit != "" && st.BinaryCommit != st.UpdatedCommit { + return fmt.Errorf("rollback binary decision contradicts state") + } + if st.PreviousBinarySHA256 != "" && p.PreviousSHA256 != st.PreviousBinarySHA256 { + return fmt.Errorf("rollback previous hash contradicts state") + } + } else { + if st.PreviousBinaryAbsent && p.LiveSHA256 != "" { + return fmt.Errorf("rollback absence contradicts live hash") + } + if p.LiveSHA256 == "" && !st.PreviousBinaryAbsent { + return fmt.Errorf("rollback lacks unchanged binary evidence") + } + if p.PreviousSHA256 != "" || st.BinaryCommit != st.PreviousCommit { + return fmt.Errorf("rollback cannot skip binary restoration") + } + } + if st.BinarySHA256 != "" && p.LiveSHA256 != st.BinarySHA256 { + return fmt.Errorf("rollback live hash contradicts state") + } + disk, present, err := recoveryState(root) + if err != nil { + return err + } + if present && disk != st { + return fmt.Errorf("rollback journal contradicts persisted state") + } + return nil +} diff --git a/internal/update/recovery_json_test.go b/internal/update/recovery_json_test.go new file mode 100644 index 00000000..152fbd4c --- /dev/null +++ b/internal/update/recovery_json_test.go @@ -0,0 +1,74 @@ +package update + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestRecoveryJSONRefusesAmbiguousFields(t *testing.T) { + for _, data := range []string{ + `null`, `[]`, `{} {}`, `{"commit":null}`, `{"commit":123}`, + `{"commit":"a","commit":"a"}`, `{"commit":"a","commit":"b"}`, + `{"Commit":"a"}`, `{"unknown":true}`, `{"previous_binary_absent":"true"}`, + `{"previous_binary_absent":null}`, `{"kept_content":[null]}`, + } { + t.Run(data, func(t *testing.T) { + var p buildPendingState + if err := decodeRecoveryJSON([]byte(data), &p); err == nil { + t.Fatal("ambiguous JSON accepted") + } + }) + } + // Duplicate nested state fields must not overwrite the evidence either. + var rollback rollbackPendingState + if err := decodeRecoveryJSON([]byte(`{"state":{"binary_sha256":"a","binary_sha256":"b"}}`), &rollback); err == nil { + t.Fatal("duplicate nested evidence accepted") + } +} + +func TestBuildEvidenceRequiresOldBinaryProof(t *testing.T) { + for _, p := range []buildPendingState{ + {Commit: strings.Repeat("a", 40)}, + {Commit: strings.Repeat("a", 40), PreviousBinarySHA256: ""}, + {Commit: strings.Repeat("a", 40), PreviousBinarySHA256: "bad"}, + {Commit: strings.Repeat("a", 40), PreviousBinarySHA256: strings.Repeat("b", 64), PreviousBinaryAbsent: true}, + {Commit: strings.Repeat("a", 40), PreviousBinaryAbsent: true, BinarySHA256: "bad"}, + } { + if err := validateBuildEvidence(t.TempDir(), p); err == nil { + t.Fatalf("incomplete evidence accepted: %+v", p) + } + } + for _, p := range []buildPendingState{ + {Commit: strings.Repeat("a", 40), PreviousBinaryAbsent: true}, + {Commit: strings.Repeat("a", 40), PreviousBinarySHA256: strings.Repeat("b", 64)}, + } { + data, _ := json.Marshal(p) + var decoded buildPendingState + if err := decodeRecoveryJSON(data, &decoded); err != nil { + t.Fatal(err) + } + if err := validateBuildEvidence(t.TempDir(), decoded); err != nil { + t.Fatal(err) + } + } +} + +func TestRollbackEvidenceWithoutDiskState(t *testing.T) { + st := State{PreviousCommit: strings.Repeat("a", 40), UpdatedCommit: strings.Repeat("b", 40), BinaryCommit: strings.Repeat("a", 40), PreviousBinaryAbsent: true} + p := rollbackPendingState{State: st} + if err := validateRollbackEvidence(t.TempDir(), p); err != nil { + t.Fatal(err) + } + for _, hash := range []string{"bad", strings.Repeat("c", 64)} { + p.State.PreviousBinarySHA256 = hash + if err := validateRollbackEvidence(t.TempDir(), p); err == nil { + t.Fatal("missing disk state bypassed embedded state validation") + } + } + p.State = st + p.State.PreviousBinaryAbsent = false + if err := validateRollbackEvidence(t.TempDir(), p); err == nil { + t.Fatal("missing live evidence accepted without binary absence") + } +} diff --git a/internal/update/recovery_phase_test.go b/internal/update/recovery_phase_test.go new file mode 100644 index 00000000..899ccb2c --- /dev/null +++ b/internal/update/recovery_phase_test.go @@ -0,0 +1,143 @@ +package update + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" +) + +func TestRollbackRecoveryRequiresActualBinaryAbsence(t *testing.T) { + requireGit(t) + tr := newTree(t) + before := mustGit(t, tr.install, "rev-parse", "HEAD") + tr.upstreamCommit(t, "next", map[string]string{"review.go": "package review\n"}) + mustGit(t, tr.install, "fetch", "origin") + target := mustGit(t, tr.install, "rev-parse", "origin/main") + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "unrelated live binary") + p := rollbackPendingState{State: State{PreviousCommit: before, UpdatedCommit: target, BinaryCommit: before, PreviousBinaryAbsent: true}, RestoreBinary: false} + if err := writeRollbackPending(tr.install, p); err != nil { + t.Fatal(err) + } + _, err := Rollback(context.Background(), buildOpts(tr)) + t.Logf("error=%v live=%q marker_exists=%v", err, readFile(t, bin), fileExists(filepath.Join(tr.install, rollbackPendingFile))) + if err == nil || !fileExists(filepath.Join(tr.install, rollbackPendingFile)) { + t.Fatal("contradictory absence evidence authorized cleanup") + } +} +func TestBuildRecoveryBeforeMergeRequiresFileProof(t *testing.T) { + requireGit(t) + tr := newTree(t) + before := mustGit(t, tr.install, "rev-parse", "HEAD") + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "old binary") + oldHash, _ := fileSHA256(bin) + tr.upstreamCommit(t, "next", map[string]string{"review.go": "package review\n"}) + mustGit(t, tr.install, "fetch", "origin") + target := mustGit(t, tr.install, "rev-parse", "origin/main") + p := buildPendingState{Commit: target, PreviousCommit: before, PreviousBinarySHA256: oldHash} + if err := writeBuildPending(tr.install, p); err != nil { + t.Fatal(err) + } + writeFile(t, bin, "unrelated live binary") + err := finishPendingBuild(context.Background(), &Snapshot{Root: tr.install}, buildOpts(tr), &Result{}, func(string, string) {}) + _, statErr := os.Stat(buildPendingPath(tr.install)) + t.Logf("error=%v marker_stat=%v", err, statErr) + if err == nil || os.IsNotExist(statErr) { + t.Fatal("mismatching binary evidence authorized marker deletion") + } +} + +func TestLegacyRecoveryRejectsMalformedState(t *testing.T) { + requireGit(t) + tr := newTree(t) + scriptFakeGo(t) + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "old binary") + tr.upstreamCommit(t, "next", map[string]string{"review.go": "package review\n"}) + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + state := readFile(t, statePath(tr.install)) + state = state[:len(state)-2] + ",\"binary_sha256\":null}" + writeFile(t, statePath(tr.install), state) + writeFile(t, buildPendingPath(tr.install), "source is ahead of the binary") + builds := 0 + _, err := Apply(context.Background(), buildOpts(tr), func(s Step) { + if s.Phase == "build" { + builds++ + } + }) + t.Logf("error=%v builds=%d marker_exists=%v", err, builds, fileExists(buildPendingPath(tr.install))) + if err == nil || builds != 0 || !fileExists(buildPendingPath(tr.install)) { + t.Fatal("legacy marker authorized mutation with malformed state") + } +} + +func TestFreshRollbackRejectsMalformedState(t *testing.T) { + requireGit(t) + tr := newTree(t) + scriptFakeGo(t) + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "old binary") + tr.upstreamCommit(t, "next", map[string]string{"review.go": "package review\n"}) + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + writeFile(t, bin+".prev", "unrelated backup") + wrong, _ := fileSHA256(bin + ".prev") + state := readFile(t, statePath(tr.install)) + state = state[:len(state)-2] + fmt.Sprintf(",\"previous_binary_sha256\":%q}", wrong) + writeFile(t, statePath(tr.install), state) + _, err := Rollback(context.Background(), buildOpts(tr)) + t.Logf("error=%v binary=%q state_exists=%v", err, readFile(t, bin), fileExists(statePath(tr.install))) + if err == nil || !fileExists(statePath(tr.install)) { + t.Fatal("malformed state authorized wrong rollback") + } +} + +func TestRollbackAbsenceCannotHaveLiveHash(t *testing.T) { + requireGit(t) + tr := newTree(t) + before := mustGit(t, tr.install, "rev-parse", "HEAD") + tr.upstreamCommit(t, "next", map[string]string{"review.go": "package review\n"}) + mustGit(t, tr.install, "fetch", "origin") + target := mustGit(t, tr.install, "rev-parse", "origin/main") + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "unrelated live binary") + hash, _ := fileSHA256(bin) + p := rollbackPendingState{State: State{PreviousCommit: before, UpdatedCommit: target, BinaryCommit: before, PreviousBinaryAbsent: true, BinarySHA256: hash}, LiveSHA256: hash, RestoreBinary: false} + if err := writeRollbackPending(tr.install, p); err != nil { + t.Fatal(err) + } + _, err := Rollback(context.Background(), buildOpts(tr)) + t.Logf("error=%v marker_exists=%v", err, fileExists(filepath.Join(tr.install, rollbackPendingFile))) + if err == nil || !fileExists(filepath.Join(tr.install, rollbackPendingFile)) { + t.Fatal("absent binary with a hash authorized cleanup") + } +} + +func TestBuildBeforeMergeRejectsPostSwap(t *testing.T) { + requireGit(t) + tr := newTree(t) + scriptFakeGo(t) + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "old binary") + tr.upstreamCommit(t, "next", map[string]string{"review.go": "package review\n"}) + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + st, _ := readState(tr.install) + p := buildPendingState{Commit: st.UpdatedCommit, PreviousCommit: st.PreviousCommit, BinarySHA256: st.BinarySHA256, PreviousBinarySHA256: st.PreviousBinarySHA256} + if err := writeBuildPending(tr.install, p); err != nil { + t.Fatal(err) + } + mustGit(t, tr.install, "reset", "--hard", st.PreviousCommit) + err := finishPendingBuild(context.Background(), &Snapshot{Root: tr.install}, buildOpts(tr), &Result{}, func(string, string) {}) + t.Logf("error=%v marker_exists=%v", err, fileExists(buildPendingPath(tr.install))) + if err == nil || !fileExists(buildPendingPath(tr.install)) { + t.Fatal("post-swap files with pre-merge HEAD authorized cleanup") + } +} diff --git a/internal/update/recovery_regression_test.go b/internal/update/recovery_regression_test.go new file mode 100644 index 00000000..cb7a783b --- /dev/null +++ b/internal/update/recovery_regression_test.go @@ -0,0 +1,109 @@ +package update + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" +) + +func TestBuildRecoveryRejectsAmbiguousEvidence(t *testing.T) { + for _, kind := range []string{"valid_residual", "missing_hashes", "null_hashes", "conflicting_duplicate", "contradictory_prior_hash"} { + t.Run(kind, func(t *testing.T) { + requireGit(t) + tr := newTree(t) + scriptFakeGo(t) + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "original binary") + before := mustGit(t, tr.install, "rev-parse", "HEAD") + tr.upstreamCommit(t, "release", map[string]string{"internal_service.go": "package service\nconst Version = \"2\"\n"}) + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + target := mustGit(t, tr.install, "rev-parse", "HEAD") + st, _ := readState(tr.install) + var marker string + switch kind { + case "valid_residual": + marker = fmt.Sprintf(`{"commit":%q,"previous_commit":%q,"binary_sha256":%q,"previous_binary_sha256":%q}`, target, before, st.BinarySHA256, st.PreviousBinarySHA256) + case "missing_hashes": + marker = fmt.Sprintf(`{"commit":%q}`, target) + case "null_hashes": + marker = fmt.Sprintf(`{"commit":%q,"binary_sha256":null,"previous_binary_sha256":null}`, target) + case "conflicting_duplicate": + marker = fmt.Sprintf(`{"commit":%q,"previous_commit":%q,"previous_binary_sha256":%q,"previous_binary_sha256":%q}`, target, before, st.PreviousBinarySHA256, st.BinarySHA256) + case "contradictory_prior_hash": + marker = fmt.Sprintf(`{"commit":%q,"previous_commit":%q,"previous_binary_sha256":%q}`, target, before, st.BinarySHA256) + } + if err := os.WriteFile(buildPendingPath(tr.install), []byte(marker), 0644); err != nil { + t.Fatal(err) + } + liveBefore := readFile(t, bin) + stateBefore := readFile(t, statePath(tr.install)) + builds := 0 + res, err := Apply(context.Background(), buildOpts(tr), func(s Step) { + if s.Phase == "build" { + builds++ + } + }) + t.Logf("recovery builds=%d error=%v result=%+v", builds, err, res) + gotPrev := readFile(t, bin+".prev") + if kind == "valid_residual" { + if builds != 0 || gotPrev != "original binary" || err != nil { + t.Fatalf("valid residual replay: builds=%d prev=%q err=%v", builds, gotPrev, err) + } + return + } + if err != nil && gotPrev == "original binary" && builds == 0 { + if readFile(t, bin) != liveBefore || readFile(t, statePath(tr.install)) != stateBefore || readFile(t, buildPendingPath(tr.install)) != marker { + t.Fatal("refusal changed live binary or recovery evidence") + } + return + } + if _, rbErr := Rollback(context.Background(), buildOpts(tr)); rbErr != nil { + t.Logf("rollback error=%v", rbErr) + } else { + t.Logf("rollback HEAD=%s binary=%q", mustGit(t, tr.install, "rev-parse", "HEAD"), readFile(t, bin)) + } + t.Fatalf("unsafe malformed recovery: builds=%d error=%v previous binary=%q (wanted refusal before build/swap; original binary preserved)", builds, err, gotPrev) + }) + } +} + +func TestBuildRecoveryBetweenBinaryRenames(t *testing.T) { + requireGit(t) + tr := newTree(t) + scriptFakeGo(t) + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "original binary") + tr.upstreamCommit(t, "second", map[string]string{"run.sh": "new script\n"}) + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + st, _ := readState(tr.install) + pending := buildPendingState{Commit: st.UpdatedCommit, PreviousCommit: st.PreviousCommit, BinarySHA256: st.BinarySHA256, PreviousBinarySHA256: st.PreviousBinarySHA256} + st.BinaryCommit = st.PreviousCommit + st.BinarySHA256 = st.PreviousBinarySHA256 + if err := writeState(tr.install, st); err != nil { + t.Fatal(err) + } + if err := writeBuildPending(tr.install, pending); err != nil { + t.Fatal(err) + } + if err := os.Remove(bin); err != nil { + t.Fatal(err) + } + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + if readFile(t, bin+".prev") != "original binary" { + t.Fatal("interrupted swap lost previous binary") + } + if _, err := Rollback(context.Background(), buildOpts(tr)); err != nil { + t.Fatal(err) + } + if readFile(t, bin) != "original binary" { + t.Fatal("rollback no longer restores original binary") + } +} diff --git a/internal/update/rollback_evidence_test.go b/internal/update/rollback_evidence_test.go new file mode 100644 index 00000000..9f8df2ef --- /dev/null +++ b/internal/update/rollback_evidence_test.go @@ -0,0 +1,103 @@ +package update + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestRollbackRecoveryRejectsAmbiguousEvidence(t *testing.T) { + for _, variant := range []string{"valid", "missing_restore_binary", "missing_live_hash", "contradictory_previous_hash", "duplicate_restore_binary"} { + t.Run(variant, func(t *testing.T) { + requireGit(t) + tr := newTree(t) + scriptFakeGo(t) + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "original binary") + tr.upstreamCommit(t, "second", map[string]string{"run.sh": "new script\n"}) + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + st, _ := readState(tr.install) + live := readFile(t, bin) + liveHash, _ := fileSHA256(bin) + previousHash, _ := fileSHA256(bin + ".prev") + pending := rollbackPendingState{State: st, RestoreBinary: true, LiveSHA256: liveHash, PreviousSHA256: previousHash} + encoded, _ := json.Marshal(pending) + var fields map[string]interface{} + _ = json.Unmarshal(encoded, &fields) + switch variant { + case "missing_restore_binary": + delete(fields, "restore_binary") + case "missing_live_hash": + delete(fields, "live_sha256") + case "contradictory_previous_hash": + writeFile(t, bin+".prev", "unrelated binary") + wrongHash, _ := fileSHA256(bin + ".prev") + fields["previous_sha256"] = wrongHash + } + encoded, _ = json.Marshal(fields) + if variant == "duplicate_restore_binary" { + encoded = append(encoded[:len(encoded)-1], []byte(",\"restore_binary\":false}")...) + } + if err := os.WriteFile(filepath.Join(tr.install, rollbackPendingFile), encoded, 0600); err != nil { + t.Fatal(err) + } + mustGit(t, tr.install, "reset", "--hard", st.PreviousCommit) + _, err := Rollback(context.Background(), buildOpts(tr)) + if variant == "valid" { + if err != nil { + t.Fatal(err) + } + if got := readFile(t, bin); got != "original binary" { + t.Fatalf("valid rollback did not restore binary: %q", got) + } + return + } + if err == nil { + t.Errorf("malformed/contradictory marker authorized recovery, live before=%q after=%q; state exists=%v marker exists=%v", live, readFile(t, bin), fileExists(statePath(tr.install)), fileExists(filepath.Join(tr.install, rollbackPendingFile))) + } + if got := readFile(t, bin); got != live { + t.Errorf("invalid evidence mutated binary: %q => %q", live, got) + } + if !fileExists(statePath(tr.install)) || !fileExists(filepath.Join(tr.install, rollbackPendingFile)) { + t.Error("invalid evidence deleted recovery records") + } + }) + } +} + +func TestRollbackRecoveryAfterStateCleanup(t *testing.T) { + requireGit(t) + tr := newTree(t) + scriptFakeGo(t) + bin := filepath.Join(tr.install, Binary) + writeFile(t, bin, "original binary") + tr.upstreamCommit(t, "second", map[string]string{"run.sh": "new script\n"}) + if _, err := Apply(context.Background(), buildOpts(tr), nil); err != nil { + t.Fatal(err) + } + st, _ := readState(tr.install) + pending := rollbackPendingState{State: st, RestoreBinary: true, LiveSHA256: st.BinarySHA256, PreviousSHA256: st.PreviousBinarySHA256} + if err := writeRollbackPending(tr.install, pending); err != nil { + t.Fatal(err) + } + mustGit(t, tr.install, "reset", "--hard", st.PreviousCommit) + if err := restorePrevBinary(bin); err != nil { + t.Fatal(err) + } + if err := os.Remove(statePath(tr.install)); err != nil { + t.Fatal(err) + } + if _, err := Rollback(context.Background(), buildOpts(tr)); err != nil { + t.Fatal(err) + } + if readFile(t, bin) != "original binary" { + t.Fatal("cleanup retry changed restored binary") + } + if fileExists(filepath.Join(tr.install, rollbackPendingFile)) { + t.Fatal("cleanup retry left journal") + } +} diff --git a/internal/update/rollback_pending.go b/internal/update/rollback_pending.go index 228d89c8..656893e2 100644 --- a/internal/update/rollback_pending.go +++ b/internal/update/rollback_pending.go @@ -29,11 +29,16 @@ func readRollbackPending(root string) (*rollbackPendingState, error) { return nil, err } var pending rollbackPendingState - if err := json.Unmarshal(data, &pending); err != nil { + if err := decodeRecoveryJSON(data, &pending); err != nil { return nil, err } - if pending.State.PreviousCommit == "" || pending.State.UpdatedCommit == "" { - return nil, fmt.Errorf("回滚恢复记录缺少提交") + var fields map[string]json.RawMessage + _ = json.Unmarshal(data, &fields) + if _, ok := fields["restore_binary"]; !ok { + return nil, fmt.Errorf("回滚恢复记录缺少二进制恢复决策") + } + if err := validateRollbackEvidence(root, pending); err != nil { + return nil, err } return &pending, nil } @@ -58,6 +63,11 @@ func finishRollback(ctx context.Context, opts Options, pending rollbackPendingSt return nil, err } bin := filepath.Join(root, opts.binaryName()) + if !pending.RestoreBinary && pending.LiveSHA256 == "" { + if _, err := os.Lstat(bin); !os.IsNotExist(err) { + return nil, &Error{Reason: "binary_changed", Message: "恢复记录要求二进制不存在,但现场不匹配;已保留记录"} + } + } if pending.RestoreBinary { current, currentErr := fileSHA256(bin) if currentErr != nil || current != pending.PreviousSHA256 {