diff --git a/docs/feature/self-update-recovery-fixes.md b/docs/feature/self-update-recovery-fixes.md index 7d623df8..8a218a55 100644 --- a/docs/feature/self-update-recovery-fixes.md +++ b/docs/feature/self-update-recovery-fixes.md @@ -34,3 +34,11 @@ Compatibility: properly written current journals remain readable, legacy plain-t 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. 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..85bce887 --- /dev/null +++ b/internal/handler/external_mcp_activation_test.go @@ -0,0 +1,110 @@ +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") + } + 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/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 {