diff --git a/internal/clientinject/engine.go b/internal/clientinject/engine.go index d845984..a09f33c 100644 --- a/internal/clientinject/engine.go +++ b/internal/clientinject/engine.go @@ -129,6 +129,35 @@ func fileSHA1(w FileWriter, path string) (string, error) { return hex.EncodeToString(sum[:]), nil } +// stockPESHA1 returns the stock primary-PE SHA-1 for forensics / manifest PESHA1. +// +// Order: +// 1. profile PrimaryBinary.SHA1 (authoritative stock fingerprint) +// 2. sibling .openfsd-bak SHA-1 (post-Apply; live PE is patched) +// 3. live PE SHA-1 (first-time / no bak — expected to equal stock) +// +// Never prefer a patched live hash when profile stock or bak is available. +func stockPESHA1(w FileWriter, pe string, profile *Profile) string { + if profile != nil { + if s := strings.ToLower(strings.TrimSpace(profile.PrimaryBinary.SHA1)); s != "" { + return s + } + } + if pe == "" { + return "" + } + if w == nil { + w = OSFileWriter{} + } + if sum, err := fileSHA1(w, BackupPath(pe)); err == nil && sum != "" { + return sum + } + if sum, err := fileSHA1(w, pe); err == nil { + return sum + } + return "" +} + // normalizeInstall makes PrimaryPE absolute under RootDir when relative. func normalizeInstall(install Install) Install { if pe := AbsPrimaryPE(install); pe != "" { @@ -153,8 +182,10 @@ func (e *Engine) Plan(ctx context.Context, install Install, ep Endpoints) (*Plan if install.ProfileID == "" { install.ProfileID = profile.ProfileID } - if install.HashSHA1 == "" && install.PrimaryPE != "" { - if sum, err := fileSHA1(e.writer(), install.PrimaryPE); err == nil { + // HashSHA1 / manifest PESHA1 must be the stock PE fingerprint, not a + // post-patch live digest (re-Plan after Apply leaves bak + patched PE). + if install.HashSHA1 == "" { + if sum := stockPESHA1(e.writer(), install.PrimaryPE, profile); sum != "" { install.HashSHA1 = sum } } @@ -214,6 +245,8 @@ func (e *Engine) Apply(ctx context.Context, plan *Plan) (*ApplyResult, error) { // On first apply (no stock bak for PE yet), PE must match profile stock SHA. // Re-apply after a prior successful inject keeps stock in .openfsd-bak while // the live PE may already be patched — skip live-hash stock check then. + // HashSHA1 / manifest PESHA1 always record stock identity (profile or bak), + // never a patched live digest. if plan.Install.PrimaryPE != "" { want := strings.ToLower(strings.TrimSpace(profile.PrimaryBinary.SHA1)) bak := BackupPath(plan.Install.PrimaryPE) @@ -226,11 +259,9 @@ func (e *Engine) Apply(ctx context.Context, plan *Plan) (*ApplyResult, error) { return nil, fmt.Errorf("clientinject: PE sha1 %s does not match profile %s stock %s", sum, profile.ProfileID, want) } - plan.Install.HashSHA1 = sum - } else if plan.Install.HashSHA1 == "" { - if sum, err := fileSHA1(w, plan.Install.PrimaryPE); err == nil { - plan.Install.HashSHA1 = sum - } + } + if stock := stockPESHA1(w, plan.Install.PrimaryPE, profile); stock != "" { + plan.Install.HashSHA1 = stock } } if err := a.Verify(plan.Install, profile); err != nil { diff --git a/internal/clientinject/engine_test.go b/internal/clientinject/engine_test.go index 83ebd8f..0167a62 100644 --- a/internal/clientinject/engine_test.go +++ b/internal/clientinject/engine_test.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" ) @@ -767,6 +768,78 @@ func TestEngine_PlanFillsHash(t *testing.T) { } } +// After Apply the live PE may be patched; re-Plan with empty HashSHA1 must still +// record stock (profile/bak) for HashSHA1, and re-Apply manifest PESHA1 must match stock. +func TestEngine_RePlanHashSHA1IsStockNotLive(t *testing.T) { + root := t.TempDir() + pe := filepath.Join(root, "app.exe") + payload := filepath.Join(root, "payload.bin") + stockPE := []byte("MZ-fake-pe") + patchedPE := []byte("MZ-PATCHED-PE-CONTENT") + if err := os.WriteFile(pe, stockPE, 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(payload, []byte("STOCK-PAYLOAD"), 0o644); err != nil { + t.Fatal(err) + } + stockSum := sha1hex(stockPE) + store := NewProfileStore() + _ = store.Add(&Profile{ + SchemaVersion: 2, ProfileID: "fake-1", ClientID: "fake", + PrimaryBinary: PrimaryBinarySpec{RelativePath: "app.exe", SHA1: stockSum}, + }) + fake := &FakeAdapter{ID: "fake", NewData: []byte("OPENFSD-PATCHED")} + eng := NewEngine(store, fake) + install := Install{ + ClientID: "fake", RootDir: root, PrimaryPE: pe, ProfileID: "fake-1", + } + plan, err := eng.Plan(context.Background(), install, Endpoints{WebBaseURL: "https://a/"}) + if err != nil { + t.Fatal(err) + } + if _, err := eng.Apply(context.Background(), plan); err != nil { + t.Fatal(err) + } + // Simulate PE body mutation (real adapters patch the PE; FakeAdapter only + // rewrites payload.bin — write a patched PE while bak holds stock). + if err := os.WriteFile(pe, patchedPE, 0o644); err != nil { + t.Fatal(err) + } + // Bak must still be stock from first Apply. + bak, err := os.ReadFile(pe + BackupSuffix) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(bak, stockPE) { + t.Fatalf("bak=%q", bak) + } + + // Re-Plan without HashSHA1 / with only RootDir+PE — must fill stock, not live. + plan2, err := eng.Plan(context.Background(), Install{ + ClientID: "fake", RootDir: root, PrimaryPE: pe, ProfileID: "fake-1", + }, Endpoints{WebBaseURL: "https://a/"}) + if err != nil { + t.Fatal(err) + } + if plan2.Install.HashSHA1 != stockSum { + t.Fatalf("re-plan HashSHA1=%q want stock %q (not live %q)", + plan2.Install.HashSHA1, stockSum, sha1hex(patchedPE)) + } + + // Re-Apply: manifest PESHA1 must be stock. + plan2.Mutations = plan.Mutations // reuse write mutation + if _, err := eng.Apply(context.Background(), plan2); err != nil { + t.Fatal(err) + } + m, err := ReadManifest(OSFileWriter{}, root) + if err != nil { + t.Fatal(err) + } + if !strings.EqualFold(m.PESHA1, stockSum) { + t.Fatalf("manifest PESHA1=%q want stock %q", m.PESHA1, stockSum) + } +} + func TestNewEngine_NilAdapterSkipped(t *testing.T) { eng := NewEngine(NewProfileStore(), nil, &FakeAdapter{ID: "x"}) if eng.Adapters["x"] == nil { diff --git a/internal/clientinject/vpilotconfig/vpilotconfig.go b/internal/clientinject/vpilotconfig/vpilotconfig.go index 5b691d5..a11fe8a 100644 --- a/internal/clientinject/vpilotconfig/vpilotconfig.go +++ b/internal/clientinject/vpilotconfig/vpilotconfig.go @@ -14,6 +14,20 @@ // Prefer Rewrite (or ParseDocument + Document.Format) when updating an existing // install config: unknown elements and attributes are preserved. Format alone // emits a minimal four-field document and is for synthetic fixtures only. +// +// # encoding/xml fidelity limits +// +// Round-trip uses encoding/xml with a generic element tree (xml:",any"). That +// preserves unknown elements and their attributes for normal vPilot configs, +// but does **not** preserve: +// - XML comments () +// - processing instructions +// - exact original whitespace, indentation, or attribute order +// - document type declarations / entity expansions beyond the stdlib decoder +// +// Acceptable for Phase 0 (vPilot writes machine-generated field values). Operators +// who hand-edit vPilotConfig.xml with comments should expect comments to be +// dropped on Apply; re-add comments after inject if needed. package vpilotconfig import (