fix: stock PE hash in manifest; document config XML limits

Prefer profile/bak stock SHA-1 for HashSHA1 and manifest PESHA1 after
re-plan/re-apply (not patched live PE). Document encoding/xml fidelity
limits (comments, PIs, whitespace) on vpilotconfig package.
This commit is contained in:
Reese Norris
2026-07-28 23:08:39 -04:00
parent ee816d67fb
commit c4d93489a8
3 changed files with 125 additions and 7 deletions

View File

@@ -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 {

View File

@@ -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 {

View File

@@ -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 (