From 0e6dd33bd4b01fbe79c6b853b1fb21b18d001b4c Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Sat, 4 Jul 2026 23:00:27 +0200 Subject: [PATCH 1/4] fix(env): verify up-to-date fast path against source, not just the lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "(up to date)" fast path trusted the lock as ground truth: at an unchanged commit it compared on-disk hashes against the lock's recorded shas (#174) and skipped when they matched. A lock whose commit pointer was advanced without its content — e.g. written by an older buggy version — reads as in-sync forever (lock == disk, both stale relative to the pinned commit), so `b update` kept printing "(up to date)" while the tree stayed at old content, with no heal path short of manually re-extracting the files. `b verify` (disk-vs-lock) is equally blind to this state. The fast path now verifies three layers, and any mismatch falls through to a full re-sync (which reconciles via the per-file strategy + safety gate and re-records the lock): 1. managed set — the CURRENT config's glob match against the tree at the pinned commit must equal the lock's file set. Changed globs/ignore/dest now re-sync immediately instead of silently waiting for the next upstream commit (the old comment suggested `--force`, which never existed for envs). 2. lock — on-disk bytes must hash to the recorded sha (unchanged, #174). 3. source — for files whose bytes are supposed to equal the raw upstream blob (replace strategy, no select filter, no local b.pin annotations), the disk content's git blob OID must match the ls-tree OID at the pinned commit. Computed in-process (gitcache.BlobOID) against a single `git ls-tree`; the pinned commit is read from the cache and fetched at most once per machine; on failure the check degrades to layer 2 instead of failing the update. client/merge strategies and select/pinned files intentionally diverge from upstream, so for those the recorded lock hash remains the contract (guards prevent re-sync churn; covered by tests). Semantics change worth noting: under replace, an interactive "keep" now survives only until the next update (source is authoritative) instead of being silently absorbed until the next upstream commit. Durable local divergence belongs to strategy client/merge, select scopes, or b.pin — documented in env-sync.mdx, which also loses its stale `b update --force` tip. gitcache: TreeEntry gains Type+OID (ls-tree already emits them), plus HasCommit and BlobOID helpers. Tests (each fails without the fix): HealsStaleLockAtBumpedCommit (the reported "lock advanced but tree didn't" state), ConfigChangeResyncsAtUnchangedCommit, ReplaceKeepIsOneShot; guards covered by PinnedFileFastPathStaysSkippable, ClientStrategyFastPathUnaffected, SelectFileFastPathSkips; gitcache HasCommit + BlobOID vs `git hash-object`. Co-Authored-By: Claude Opus 4.8 --- docs/env-sync.mdx | 3 +- pkg/env/env.go | 192 ++++++++++++---- pkg/env/sync_source_verify_test.go | 331 ++++++++++++++++++++++++++++ pkg/gitcache/gitcache.go | 39 +++- pkg/gitcache/gitcache_extra_test.go | 86 ++++++++ 5 files changed, 607 insertions(+), 44 deletions(-) create mode 100644 pkg/env/sync_source_verify_test.go diff --git a/docs/env-sync.mdx b/docs/env-sync.mdx index ac06bee..a83fe2c 100644 --- a/docs/env-sync.mdx +++ b/docs/env-sync.mdx @@ -707,5 +707,6 @@ Uses shallow bare clones (`--depth 1`) to minimize disk usage. - **Verify**: `b verify` checks all artifacts against `b.lock` checksums. - **Conflicts**: If two env entries write to the same path, **b** warns before syncing. - **Auth**: Private repos need `GITHUB_TOKEN`, `GITLAB_TOKEN`, or `GITEA_TOKEN`. See [Authentication](/authentication). -- **Force**: `b update --force` re-syncs even when up-to-date. +- **Self-healing**: at an unchanged commit, `b update` verifies the on-disk files against the lock **and** the actual source content at the pinned commit (for plainly-synced files under `replace`). Any drift — local edits, a stale/corrupted lock, or a changed `files:` glob config — triggers a re-sync that reconciles via your strategy and re-records the lock. No `--force` needed. +- **Keeps are one-shot**: under `replace`, an interactive "keep" survives only until the next update — source is authoritative. For durable local divergence use `strategy: client`/`merge`, a `select` scope, or a `b.pin` annotation. - **Unchanged**: Files identical to upstream are automatically skipped. diff --git a/pkg/env/env.go b/pkg/env/env.go index 8fbaca5..1876d59 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -2,6 +2,7 @@ package env import ( + "bytes" "crypto/sha256" "fmt" "io" @@ -107,50 +108,23 @@ func SyncEnv(cfg EnvConfig, projectRoot, cacheRoot string, lockEntry *lock.EnvEn // Check if up-to-date (skip when forcing a specific commit). // - // Skip only when the commit is unchanged AND every locked file is still - // byte-identical to its recorded hash. A missing OR drifted file — local - // edits, or a lock written with stale/wrong hashes (e.g. by an older buggy - // version) — falls through to a full re-sync, which reconciles via the - // per-file strategy + safety gate and re-records the lock. Without the hash - // check, a stale lock at an unchanged commit was unhealable: status/verify - // reported drift forever while update kept printing "(up to date)". + // Skip only when the commit is unchanged AND the on-disk state still + // matches both the lock and — for plainly-written files — the actual + // source content at that commit. The lock alone is not trusted as ground + // truth: a lock whose commit pointer was advanced without its content + // (e.g. by an older buggy version) would otherwise read as "up to date" + // forever while the tree stayed stale. Any mismatch — missing file, local + // edit, changed glob config, or lock+disk both stale relative to source — + // falls through to a full re-sync, which reconciles via the per-file + // strategy + safety gate and re-records the lock. // - // This trades a local hash of each synced file (no network) on the - // commit-unchanged fast path for self-healing + drift detection — a - // deliberate UX-over-compute choice; the network fetch still happens only - // when drift is actually found. + // This is a deliberate UX-over-compute choice: the fast path costs a local + // read+hash per synced file and one `git ls-tree` (against the local + // cache; the pinned commit is fetched at most once per machine). if cfg.ForceCommit == "" && lockEntry != nil && lockEntry.Commit == commit { - inSync := true - for _, f := range lockEntry.Files { - dest := filepath.Join(projectRoot, f.Dest) - if err := ValidatePathUnderRoot(projectRoot, dest); err != nil { - return nil, fmt.Errorf("lock entry has invalid dest %q: %w", f.Dest, err) - } - // A legacy/partial lock entry with no recorded hash can't be compared; - // fall back to an existence check (and don't read the file, so a - // present-but-unreadable file stays skippable as before). - if f.SHA256 == "" { - if _, err := os.Stat(dest); err != nil { - if os.IsNotExist(err) { - inSync = false // missing → re-sync - break - } - return nil, fmt.Errorf("checking synced env file %q: %w", dest, err) - } - continue - } - localHash, err := lock.SHA256File(dest) - if err != nil { - if os.IsNotExist(err) { - inSync = false // missing → re-sync - break - } - return nil, fmt.Errorf("checking synced env file %q: %w", dest, err) - } - if localHash != f.SHA256 { - inSync = false // drifted → re-sync (heals a poisoned lock) - break - } + inSync, err := lockedStateInSync(cfg, resolved, projectRoot, cacheRoot, baseRef, commit, strategy, lockEntry) + if err != nil { + return nil, err } if inSync { return &SyncResult{ @@ -879,6 +853,140 @@ func ValidatePathUnderRoot(root, destPath string) error { return nil } +// lockedStateInSync reports whether the on-disk state still matches the lock +// AND, where verifiable, the source content at the pinned commit — the +// precondition for SyncEnv's "(up to date)" fast path. +// +// Three layers, strongest available wins: +// +// 1. Managed-set check: the glob match of the CURRENT config against the tree +// at `commit` must equal the lock's file set. A changed glob/ignore/dest +// config alters what belongs on disk even at an unchanged commit; +// previously this skipped silently (the old comment said "use --force", +// which never existed for envs). +// 2. Lock check (per file): on-disk bytes must hash to the recorded SHA-256 — +// catches local edits and deletions. Legacy entries with no recorded hash +// fall back to an existence check (no read, so a present-but-unreadable +// file stays skippable as before). +// 3. Source check (per file): for files whose on-disk bytes are supposed to +// equal the raw upstream blob — replace strategy, no select filter, no +// local `b.pin` annotations — the disk content's git blob OID must match +// the tree entry's OID. This catches a lock whose commit pointer was +// advanced without its content (lock == disk == stale): indistinguishable +// from "up to date" by layers 1–2, yet the tree is stale relative to the +// pinned commit. client/merge strategies and select/pinned files +// intentionally diverge from upstream, so for those the recorded lock +// hash remains the contract. +// +// Source context (layers 1 and 3) is best-effort: the pinned commit is used +// from the cache when present, fetched once when not; if it cannot be +// obtained (offline against an evicted cache, listing failure) the check +// degrades to layer 2 rather than failing the update. +func lockedStateInSync(cfg EnvConfig, resolved gitcache.ResolvedRef, projectRoot, cacheRoot, baseRef, commit, strategy string, lockEntry *lock.EnvEntry) (bool, error) { + srcOID := make(map[string]string) // source path → blob OID at commit + selective := make(map[string]bool) // path\x00dest → has select filter + sourceOK := false + + repoDir := "" + if resolved.IsLocal { + repoDir = resolved.URL + } else if gitcache.HasCommit(cacheRoot, baseRef, commit) { + repoDir = gitcache.CacheDir(cacheRoot, baseRef) + } else if gitcache.EnsureCloneAuth(cacheRoot, baseRef, resolved.URL, resolved.AuthHeader) == nil && + gitcache.FetchAuth(cacheRoot, baseRef, commit, resolved.AuthHeader) == nil { + // Commit not cached (fresh machine / evicted cache): fetch it so the + // fast path can verify against source. On failure, degrade gracefully. + repoDir = gitcache.CacheDir(cacheRoot, baseRef) + } + if repoDir != "" { + if entries, err := gitcache.ListTreeWithModesDir(repoDir, commit); err == nil { + paths := make([]string, 0, len(entries)) + for _, e := range entries { + paths = append(paths, e.Path) + if e.Type == "" || e.Type == "blob" { + srcOID[e.Path] = e.OID + } + } + matched := envmatch.MatchGlobs(paths, cfg.Files, cfg.Ignore) + if len(matched) != len(lockEntry.Files) { + return false, nil // managed set changed → re-sync + } + want := make(map[string]int, len(matched)) + for _, m := range matched { + k := m.SourcePath + "\x00" + m.DestPath + want[k]++ + if len(m.Select) > 0 { + selective[k] = true + } + } + for _, f := range lockEntry.Files { + k := f.Path + "\x00" + f.Dest + if want[k] == 0 { + return false, nil // managed set changed → re-sync + } + want[k]-- + } + sourceOK = true + } + } + + for _, f := range lockEntry.Files { + dest := filepath.Join(projectRoot, f.Dest) + if err := ValidatePathUnderRoot(projectRoot, dest); err != nil { + return false, fmt.Errorf("lock entry has invalid dest %q: %w", f.Dest, err) + } + // Legacy/partial lock entry with no recorded hash: existence only. + if f.SHA256 == "" { + if _, err := os.Stat(dest); err != nil { + if os.IsNotExist(err) { + return false, nil // missing → re-sync + } + return false, fmt.Errorf("checking synced env file %q: %w", dest, err) + } + continue + } + data, err := os.ReadFile(dest) + if err != nil { + if os.IsNotExist(err) { + return false, nil // missing → re-sync + } + return false, fmt.Errorf("checking synced env file %q: %w", dest, err) + } + if fmt.Sprintf("%x", sha256.Sum256(data)) != f.SHA256 { + return false, nil // local drift → re-sync (heals a poisoned lock) + } + // Layer 3: source-authoritative check for plainly-written files. + if !sourceOK || strategy != StrategyReplace { + continue + } + k := f.Path + "\x00" + f.Dest + if selective[k] || mayCarryPins(f.Path, data) { + continue + } + oid, ok := srcOID[f.Path] + if !ok { + return false, nil // no longer a blob in the tree → re-sync + } + if gitcache.BlobOID(data, len(oid)) != oid { + return false, nil // lock+disk stale vs source → re-sync heals both + } + } + return true, nil +} + +// mayCarryPins reports whether the on-disk content may carry `b.pin` +// annotations that make it legitimately diverge from the raw upstream blob. +// Mirrors applyPinsYAML's cheap pre-check: pinning is YAML-only and requires +// the annotation substring. False positives (a key literally named "b.pin") +// merely downgrade that file to the lock-hash comparison — safe. +func mayCarryPins(sourcePath string, data []byte) bool { + ext := strings.ToLower(filepath.Ext(sourcePath)) + if ext != ".yaml" && ext != ".yml" { + return false + } + return bytes.Contains(data, []byte(PinAnnotation)) +} + // safeShort returns the first 12 chars of s, or s itself if shorter. func safeShort(s string) string { if len(s) > 12 { diff --git a/pkg/env/sync_source_verify_test.go b/pkg/env/sync_source_verify_test.go new file mode 100644 index 0000000..d75a507 --- /dev/null +++ b/pkg/env/sync_source_verify_test.go @@ -0,0 +1,331 @@ +package env + +import ( + "crypto/sha256" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/fentas/b/pkg/envmatch" + "github.com/fentas/b/pkg/lock" +) + +// sourceVerifyRepo builds a work repo + bare mirror with cfg/a.yaml=v1 (and +// any extra files), committed as commit A. The returned commitB func amends +// upstream (modify cfg/a.yaml to v2 and/or add files) and pushes commit B into +// the bare, returning its sha. +func sourceVerifyRepo(t *testing.T, extra map[string]string) (bare string, commitB func(changes map[string]string) string) { + t.Helper() + tmp := t.TempDir() + work := filepath.Join(tmp, "work") + bare = filepath.Join(tmp, "bare.git") + run := func(args ...string) string { + t.Helper() + out, err := exec.Command(args[0], args[1:]...).CombinedOutput() + if err != nil { + t.Fatalf("%v: %v\n%s", args, err, out) + } + return string(out) + } + run("git", "init", "-q", "-b", "main", work) + run("git", "-C", work, "config", "user.email", "t@t.com") + run("git", "-C", work, "config", "user.name", "T") + files := map[string]string{"cfg/a.yaml": "v: 1\n"} + for p, c := range extra { + files[p] = c + } + for p, c := range files { + full := filepath.Join(work, p) + if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(c), 0644); err != nil { + t.Fatal(err) + } + } + run("git", "-C", work, "add", "-A") + run("git", "-C", work, "commit", "-q", "-m", "A", "--no-gpg-sign") + run("git", "clone", "--bare", "-q", work, bare) + + commitB = func(changes map[string]string) string { + t.Helper() + for p, c := range changes { + full := filepath.Join(work, p) + if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(c), 0644); err != nil { + t.Fatal(err) + } + } + run("git", "-C", work, "add", "-A") + run("git", "-C", work, "commit", "-q", "-m", "B", "--no-gpg-sign") + run("git", "-C", work, "push", "-q", bare, "main") + out, err := exec.Command("git", "-C", bare, "rev-parse", "HEAD").Output() + if err != nil { + t.Fatal(err) + } + return string(out[:40]) + } + return bare, commitB +} + +func lockFromResult(res *SyncResult) *lock.EnvEntry { + le := &lock.EnvEntry{Commit: res.Commit, Files: make([]lock.LockFile, len(res.Files))} + for i, f := range res.Files { + le.Files[i] = lock.LockFile{Path: f.Path, Dest: f.Dest, SHA256: f.SHA256} + } + return le +} + +// TestSyncEnv_HealsStaleLockAtBumpedCommit is the regression for the +// "lock advanced but tree didn't" state: lock.Commit points at the new commit +// while its per-file hashes AND the on-disk files still hold the old content +// (a lock corrupted by an older version). Layers 1–2 of the fast path see +// disk==lock and would skip forever; the source check (layer 3) must detect +// that lock+disk are stale relative to the pinned commit and re-sync. +func TestSyncEnv_HealsStaleLockAtBumpedCommit(t *testing.T) { + bare, commitB := sourceVerifyRepo(t, nil) + project := t.TempDir() + cfg := EnvConfig{Ref: bare, Strategy: StrategyReplace, + Files: map[string]envmatch.GlobConfig{"cfg/*.yaml": {Dest: "configs"}}} + + res, err := SyncEnv(cfg, project, t.TempDir(), nil) + if err != nil { + t.Fatalf("sync A: %v", err) + } + le := lockFromResult(res) + + // Upstream moves: same file set, changed content (isolates the blob check + // from the managed-set check). + newCommit := commitB(map[string]string{"cfg/a.yaml": "v: 2\n"}) + + // Poison: commit pointer advanced, shas + disk still at A. + le.Commit = newCommit + + res2, err := SyncEnv(cfg, project, t.TempDir(), le) + if err != nil { + t.Fatalf("sync B: %v", err) + } + if res2.Skipped { + t.Fatal("must not skip: lock+disk are stale relative to the pinned commit") + } + data, err := os.ReadFile(filepath.Join(project, "configs", "a.yaml")) + if err != nil { + t.Fatal(err) + } + if string(data) != "v: 2\n" { + t.Errorf("disk = %q, want healed to v: 2", data) + } + want := fmt.Sprintf("%x", sha256.Sum256([]byte("v: 2\n"))) + for _, f := range res2.Files { + if f.Dest == "configs/a.yaml" && f.SHA256 != want { + t.Errorf("re-recorded sha = %s, want sha of new content", f.SHA256) + } + } +} + +// TestSyncEnv_ConfigChangeResyncsAtUnchangedCommit: adding a glob to b.yaml +// changes what belongs on disk even when the commit didn't move. Previously +// the fast path skipped silently (the old comment suggested a --force that +// never existed for envs). +func TestSyncEnv_ConfigChangeResyncsAtUnchangedCommit(t *testing.T) { + bare, _ := sourceVerifyRepo(t, map[string]string{"extra/x.yaml": "x: 1\n"}) + project := t.TempDir() + cfg := EnvConfig{Ref: bare, Strategy: StrategyReplace, + Files: map[string]envmatch.GlobConfig{"cfg/*.yaml": {Dest: "configs"}}} + + res, err := SyncEnv(cfg, project, t.TempDir(), nil) + if err != nil { + t.Fatalf("sync: %v", err) + } + le := lockFromResult(res) + + // Same commit, wider config: extra/* now included. + cfg.Files["extra/*.yaml"] = envmatch.GlobConfig{Dest: "extras"} + res2, err := SyncEnv(cfg, project, t.TempDir(), le) + if err != nil { + t.Fatalf("re-sync: %v", err) + } + if res2.Skipped { + t.Fatal("must not skip when the glob config changed") + } + if _, err := os.Stat(filepath.Join(project, "extras", "x.yaml")); err != nil { + t.Errorf("newly-matched file not written: %v", err) + } +} + +// TestSyncEnv_PinnedFileFastPathStaysSkippable: a file whose local copy +// carries `b.pin` annotations legitimately diverges from the upstream blob. +// Once the sync has stabilized (lock records the pinned target), the fast +// path must keep skipping — the source check defers to the lock for +// pin-carrying files instead of re-syncing on every run. +func TestSyncEnv_PinnedFileFastPathStaysSkippable(t *testing.T) { + bare, _ := sourceVerifyRepo(t, map[string]string{"cfg/app.yaml": "app:\n image: upstream\n"}) + project := t.TempDir() + cfg := EnvConfig{Ref: bare, Strategy: StrategyReplace, + Files: map[string]envmatch.GlobConfig{"cfg/*.yaml": {Dest: "configs"}}} + + res, err := SyncEnv(cfg, project, t.TempDir(), nil) + if err != nil { + t.Fatalf("sync: %v", err) + } + le := lockFromResult(res) + + // Consumer pins the app map with a local override. + pinned := "app:\n b.pin: true\n image: custom\n" + if err := os.WriteFile(filepath.Join(project, "configs", "app.yaml"), []byte(pinned), 0644); err != nil { + t.Fatal(err) + } + // Re-sync: local drift is detected (disk != lock), pins are honored, and + // the lock re-records the pinned target. + res2, err := SyncEnv(cfg, project, t.TempDir(), le) + if err != nil { + t.Fatalf("re-sync: %v", err) + } + if res2.Skipped { + t.Fatal("pin edit should trigger one reconciling sync") + } + data, err := os.ReadFile(filepath.Join(project, "configs", "app.yaml")) + if err != nil { + t.Fatal(err) + } + // Pin restoration re-marshals the YAML (indentation may normalize), so + // assert semantically: the pinned override survived, upstream didn't win. + if !strings.Contains(string(data), "b.pin: true") || !strings.Contains(string(data), "image: custom") { + t.Fatalf("pinned content not preserved: %q", data) + } + if strings.Contains(string(data), "image: upstream") { + t.Fatalf("upstream overwrote the pinned key: %q", data) + } + + // Stabilized: subsequent runs at the same commit must skip, even though + // the pinned file's bytes differ from the upstream blob. + le2 := lockFromResult(res2) + res3, err := SyncEnv(cfg, project, t.TempDir(), le2) + if err != nil { + t.Fatalf("third sync: %v", err) + } + if !res3.Skipped { + t.Error("pinned file must not defeat the up-to-date fast path") + } +} + +// TestSyncEnv_ClientStrategyFastPathUnaffected: under strategy client, local +// divergence from upstream is the contract — the source check must not apply, +// or every update would re-sync (and re-keep) forever. +func TestSyncEnv_ClientStrategyFastPathUnaffected(t *testing.T) { + bare, _ := sourceVerifyRepo(t, nil) + project := t.TempDir() + cfg := EnvConfig{Ref: bare, Strategy: StrategyClient, + Files: map[string]envmatch.GlobConfig{"cfg/*.yaml": {Dest: "configs"}}} + + res, err := SyncEnv(cfg, project, t.TempDir(), nil) + if err != nil { + t.Fatalf("sync: %v", err) + } + le := lockFromResult(res) + + // Local edit; client keeps it and records the local hash. + if err := os.WriteFile(filepath.Join(project, "configs", "a.yaml"), []byte("local: edit\n"), 0644); err != nil { + t.Fatal(err) + } + res2, err := SyncEnv(cfg, project, t.TempDir(), le) + if err != nil { + t.Fatalf("re-sync: %v", err) + } + if res2.Skipped { + t.Fatal("local edit should trigger one reconciling sync") + } + + // Stabilized: disk matches the lock (kept hash) and diverges from source — + // client envs must still hit the fast path. + le2 := lockFromResult(res2) + res3, err := SyncEnv(cfg, project, t.TempDir(), le2) + if err != nil { + t.Fatalf("third sync: %v", err) + } + if !res3.Skipped { + t.Error("client-strategy env must keep skipping when disk matches the lock") + } + if got, _ := os.ReadFile(filepath.Join(project, "configs", "a.yaml")); string(got) != "local: edit\n" { + t.Errorf("client strategy must preserve local content, got %q", got) + } +} + +// TestSyncEnv_SelectFileFastPathSkips: a select-filtered file's on-disk bytes +// are a scoped slice, never the raw upstream blob — the source check must +// defer to the lock hash for it, keeping the fast path stable. +func TestSyncEnv_SelectFileFastPathSkips(t *testing.T) { + bare, _ := sourceVerifyRepo(t, map[string]string{"cfg/multi.yaml": "keep: 1\nother: 2\n"}) + project := t.TempDir() + cfg := EnvConfig{Ref: bare, Strategy: StrategyReplace, + Files: map[string]envmatch.GlobConfig{ + "cfg/a.yaml": {Dest: "configs"}, + "cfg/multi.yaml": {Dest: "configs", Select: []string{".keep"}}, + }} + + res, err := SyncEnv(cfg, project, t.TempDir(), nil) + if err != nil { + t.Fatalf("sync: %v", err) + } + le := lockFromResult(res) + + res2, err := SyncEnv(cfg, project, t.TempDir(), le) + if err != nil { + t.Fatalf("re-sync: %v", err) + } + if !res2.Skipped { + t.Error("in-sync env with a select-filtered file must skip") + } +} + +// TestSyncEnv_ReplaceKeepIsOneShot documents the sharpened `replace` +// semantics: an interactive "keep" decision survives only until the next +// update. Under replace, source is authoritative — the source check re-detects +// the divergence at the same commit and the file is re-synced (previously the +// keep was silently absorbed until the next upstream commit). Durable local +// divergence belongs to strategy client/merge, select, or b.pin. +func TestSyncEnv_ReplaceKeepIsOneShot(t *testing.T) { + bare, _ := sourceVerifyRepo(t, nil) + project := t.TempDir() + cfg := EnvConfig{Ref: bare, Strategy: StrategyReplace, + Files: map[string]envmatch.GlobConfig{"cfg/*.yaml": {Dest: "configs"}}} + + res, err := SyncEnv(cfg, project, t.TempDir(), nil) + if err != nil { + t.Fatalf("sync: %v", err) + } + le := lockFromResult(res) + + // Local edit, then an interactive "keep" during re-sync. + if err := os.WriteFile(filepath.Join(project, "configs", "a.yaml"), []byte("mine: 1\n"), 0644); err != nil { + t.Fatal(err) + } + cfg.ResolveConflict = func(sourcePath, destPath string) string { return StrategyClient } + res2, err := SyncEnv(cfg, project, t.TempDir(), le) + if err != nil { + t.Fatalf("keep sync: %v", err) + } + if got, _ := os.ReadFile(filepath.Join(project, "configs", "a.yaml")); string(got) != "mine: 1\n" { + t.Fatalf("keep should preserve local content, got %q", got) + } + + // Next update at the same commit: source check sees disk≠source and + // re-syncs; without a resolver, replace restores upstream. + cfg.ResolveConflict = nil + le2 := lockFromResult(res2) + res3, err := SyncEnv(cfg, project, t.TempDir(), le2) + if err != nil { + t.Fatalf("third sync: %v", err) + } + if res3.Skipped { + t.Fatal("kept divergence under replace must resurface, not be absorbed") + } + if got, _ := os.ReadFile(filepath.Join(project, "configs", "a.yaml")); string(got) != "v: 1\n" { + t.Errorf("replace should restore upstream content, got %q", got) + } +} diff --git a/pkg/gitcache/gitcache.go b/pkg/gitcache/gitcache.go index 7d20e96..367ecd0 100644 --- a/pkg/gitcache/gitcache.go +++ b/pkg/gitcache/gitcache.go @@ -4,7 +4,9 @@ package gitcache import ( "bytes" + "crypto/sha1" "crypto/sha256" + "encoding/hex" "fmt" "os" "os/exec" @@ -62,6 +64,15 @@ func FetchAuth(root, ref, commitOrTag, authHeader string) error { return nil } +// HasCommit reports whether the given commit object is already present in the +// cache for ref, so callers can skip a redundant fetch (and the network probe +// it entails) on the up-to-date fast path. +func HasCommit(root, ref, commit string) bool { + dir := CacheDir(root, ref) + cmd := exec.Command("git", "-C", dir, "cat-file", "-e", commit+"^{commit}") + return cmd.Run() == nil +} + // redactWrap wraps an error with a redacted message while preserving the error chain. func redactWrap(err error, authHeader string) error { if authHeader == "" { @@ -130,6 +141,8 @@ func ResolveRefAuth(url, version, authHeader string) (string, error) { type TreeEntry struct { Path string Mode string // git mode, e.g. "100644", "100755" + Type string // git object type, e.g. "blob", "commit" (submodule) + OID string // git object id of the entry (blob hash for files) } // ListTree returns all file paths in the repo at the given commit. @@ -181,15 +194,39 @@ func ListTreeWithModesDir(dir, commit string) ([]TreeEntry, error) { } path := line[tabIdx+1:] fields := strings.Fields(line[:tabIdx]) + // ls-tree line shape: " \t". mode := "100644" if len(fields) >= 1 { mode = fields[0] } - entries = append(entries, TreeEntry{Path: path, Mode: mode}) + entry := TreeEntry{Path: path, Mode: mode} + if len(fields) >= 3 { + entry.Type = fields[1] + entry.OID = fields[2] + } + entries = append(entries, entry) } return entries, nil } +// BlobOID computes the git object id of a blob with the given content, using +// the hash implied by hexLen (40 → SHA-1, git's default object format; 64 → +// SHA-256 for sha256 repositories). This lets callers compare on-disk bytes +// against `git ls-tree` output without spawning git once per file. +func BlobOID(content []byte, hexLen int) string { + header := fmt.Sprintf("blob %d\x00", len(content)) + if hexLen == 64 { + h := sha256.New() + h.Write([]byte(header)) + h.Write(content) + return hex.EncodeToString(h.Sum(nil)) + } + h := sha1.New() // matches git's default (sha1) object format + h.Write([]byte(header)) + h.Write(content) + return hex.EncodeToString(h.Sum(nil)) +} + // runAuth executes a git command with optional auth env vars. func runAuth(ac AuthCmd) error { cmd := exec.Command(ac.Args[0], ac.Args[1:]...) diff --git a/pkg/gitcache/gitcache_extra_test.go b/pkg/gitcache/gitcache_extra_test.go index 43328d4..1534e25 100644 --- a/pkg/gitcache/gitcache_extra_test.go +++ b/pkg/gitcache/gitcache_extra_test.go @@ -2,7 +2,9 @@ package gitcache import ( "os" + "os/exec" "path/filepath" + "strings" "testing" ) @@ -106,6 +108,90 @@ func TestRepoPath(t *testing.T) { } } +func TestHasCommit(t *testing.T) { + tmp := t.TempDir() + work := filepath.Join(tmp, "work") + bare := filepath.Join(tmp, "bare.git") + cacheRoot := filepath.Join(tmp, "cache") + run := func(args ...string) { + t.Helper() + if out, err := exec.Command(args[0], args[1:]...).CombinedOutput(); err != nil { + t.Fatalf("%v: %v\n%s", args, err, out) + } + } + run("git", "init", "-q", "-b", "main", work) + run("git", "-C", work, "config", "user.email", "t@t.com") + run("git", "-C", work, "config", "user.name", "T") + if err := os.WriteFile(filepath.Join(work, "f"), []byte("x"), 0644); err != nil { + t.Fatal(err) + } + run("git", "-C", work, "add", "-A") + run("git", "-C", work, "commit", "-q", "-m", "c", "--no-gpg-sign") + run("git", "clone", "--bare", "-q", work, bare) + + // Cache dir doesn't exist yet → false, no error. + if HasCommit(cacheRoot, "r", "0000000000000000000000000000000000000000") { + t.Error("HasCommit on missing cache dir should be false") + } + if err := EnsureClone(cacheRoot, "r", bare); err != nil { + t.Fatal(err) + } + commit, err := ResolveRef(bare, "") + if err != nil { + t.Fatal(err) + } + if err := Fetch(cacheRoot, "r", commit); err != nil { + t.Fatal(err) + } + if !HasCommit(cacheRoot, "r", commit) { + t.Error("HasCommit should be true for a fetched commit") + } + if HasCommit(cacheRoot, "r", "1111111111111111111111111111111111111111") { + t.Error("HasCommit should be false for an unknown sha") + } +} + +func TestBlobOIDAndTreeOIDs(t *testing.T) { + tmp := t.TempDir() + work := filepath.Join(tmp, "work") + run := func(args ...string) string { + t.Helper() + out, err := exec.Command(args[0], args[1:]...).CombinedOutput() + if err != nil { + t.Fatalf("%v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + run("git", "init", "-q", "-b", "main", work) + run("git", "-C", work, "config", "user.email", "t@t.com") + run("git", "-C", work, "config", "user.name", "T") + content := []byte("hello blob\n") + if err := os.WriteFile(filepath.Join(work, "f.txt"), content, 0644); err != nil { + t.Fatal(err) + } + run("git", "-C", work, "add", "-A") + run("git", "-C", work, "commit", "-q", "-m", "c", "--no-gpg-sign") + + // BlobOID must equal what git itself computes. + gitOID := run("git", "-C", work, "hash-object", filepath.Join(work, "f.txt")) + if got := BlobOID(content, len(gitOID)); got != gitOID { + t.Errorf("BlobOID = %s, want %s (git hash-object)", got, gitOID) + } + + // ListTreeWithModesDir must surface type + OID per entry. + commit := run("git", "-C", work, "rev-parse", "HEAD") + entries, err := ListTreeWithModesDir(work, commit) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 { + t.Fatalf("entries = %d, want 1", len(entries)) + } + if entries[0].Type != "blob" || entries[0].OID != gitOID { + t.Errorf("entry = %+v, want type=blob oid=%s", entries[0], gitOID) + } +} + func TestRefLabel_EdgeCases(t *testing.T) { tests := []struct { ref string From 762700176364b4a8259c433286f9e9adf211891c Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Sat, 4 Jul 2026 23:11:49 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(env):=20address=20review=20round=201=20?= =?UTF-8?q?=E2=80=94=20stream=20lock-only=20hashes,=20shared=20pin=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot: - test helper returned rev-parse output truncated to 40 chars, invalid for sha256-object-format repos → TrimSpace instead. - lockedStateInSync slurped every file even when only the lock comparison (layer 2) would run. Files outside layer 3's scope (non-replace strategy, no source context, select-filtered) now use the streaming lock.SHA256File; the full read happens only where the bytes are needed anyway (blob OID + pin detection). - env-sync.mdx self-healing tip now states the fallback: source verification needs the pinned commit in the local cache (auto-fetched when missing) and degrades to lock-only verification for that run when unavailable. Subagent: - mayCarryPins moved to pin.go as the shared gate used by both applyPinsYAML and the fast path (was a drift-prone mirror of the same ext+substring check). - the fast-path comments no longer overclaim "fetched at most once per machine": a failed fetch degrades for the run and is retried next update (transient failures self-heal; persistent ones cost one attempt per run). Co-Authored-By: Claude Opus 4.8 --- docs/env-sync.mdx | 2 +- pkg/env/env.go | 49 ++++++++++++++++-------------- pkg/env/pin.go | 39 ++++++++++++++---------- pkg/env/sync_source_verify_test.go | 3 +- 4 files changed, 53 insertions(+), 40 deletions(-) diff --git a/docs/env-sync.mdx b/docs/env-sync.mdx index a83fe2c..35e673d 100644 --- a/docs/env-sync.mdx +++ b/docs/env-sync.mdx @@ -707,6 +707,6 @@ Uses shallow bare clones (`--depth 1`) to minimize disk usage. - **Verify**: `b verify` checks all artifacts against `b.lock` checksums. - **Conflicts**: If two env entries write to the same path, **b** warns before syncing. - **Auth**: Private repos need `GITHUB_TOKEN`, `GITLAB_TOKEN`, or `GITEA_TOKEN`. See [Authentication](/authentication). -- **Self-healing**: at an unchanged commit, `b update` verifies the on-disk files against the lock **and** the actual source content at the pinned commit (for plainly-synced files under `replace`). Any drift — local edits, a stale/corrupted lock, or a changed `files:` glob config — triggers a re-sync that reconciles via your strategy and re-records the lock. No `--force` needed. +- **Self-healing**: at an unchanged commit, `b update` verifies the on-disk files against the lock **and** the actual source content at the pinned commit (for plainly-synced files under `replace`). Any drift — local edits, a stale/corrupted lock, or a changed `files:` glob config — triggers a re-sync that reconciles via your strategy and re-records the lock. No `--force` needed. Source verification needs the pinned commit in the local git cache (fetched automatically when missing); if it can't be obtained — e.g. offline — that run falls back to lock-only verification. - **Keeps are one-shot**: under `replace`, an interactive "keep" survives only until the next update — source is authoritative. For durable local divergence use `strategy: client`/`merge`, a `select` scope, or a `b.pin` annotation. - **Unchanged**: Files identical to upstream are automatically skipped. diff --git a/pkg/env/env.go b/pkg/env/env.go index 1876d59..0c52b97 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -2,7 +2,6 @@ package env import ( - "bytes" "crypto/sha256" "fmt" "io" @@ -119,8 +118,9 @@ func SyncEnv(cfg EnvConfig, projectRoot, cacheRoot string, lockEntry *lock.EnvEn // strategy + safety gate and re-records the lock. // // This is a deliberate UX-over-compute choice: the fast path costs a local - // read+hash per synced file and one `git ls-tree` (against the local - // cache; the pinned commit is fetched at most once per machine). + // read+hash per synced file and one `git ls-tree` against the local cache + // (the pinned commit is fetched when missing; a failed fetch degrades + // source verification to the lock comparison for that run). if cfg.ForceCommit == "" && lockEntry != nil && lockEntry.Commit == commit { inSync, err := lockedStateInSync(cfg, resolved, projectRoot, cacheRoot, baseRef, commit, strategy, lockEntry) if err != nil { @@ -895,7 +895,10 @@ func lockedStateInSync(cfg EnvConfig, resolved gitcache.ResolvedRef, projectRoot } else if gitcache.EnsureCloneAuth(cacheRoot, baseRef, resolved.URL, resolved.AuthHeader) == nil && gitcache.FetchAuth(cacheRoot, baseRef, commit, resolved.AuthHeader) == nil { // Commit not cached (fresh machine / evicted cache): fetch it so the - // fast path can verify against source. On failure, degrade gracefully. + // fast path can verify against source. On failure, source verification + // degrades to the lock comparison for this run and the fetch is simply + // retried on the next update — transient failures self-heal, a + // persistently failing fetch costs one attempt per run. repoDir = gitcache.CacheDir(cacheRoot, baseRef) } if repoDir != "" { @@ -930,6 +933,10 @@ func lockedStateInSync(cfg EnvConfig, resolved gitcache.ResolvedRef, projectRoot } } + // Layer 3 needs the full file bytes (blob OID + pin detection); files it + // won't apply to get the cheaper streaming hash instead. + needSource := sourceOK && strategy == StrategyReplace + for _, f := range lockEntry.Files { dest := filepath.Join(projectRoot, f.Dest) if err := ValidatePathUnderRoot(projectRoot, dest); err != nil { @@ -945,6 +952,21 @@ func lockedStateInSync(cfg EnvConfig, resolved gitcache.ResolvedRef, projectRoot } continue } + k := f.Path + "\x00" + f.Dest + if !needSource || selective[k] { + // Lock-only comparison (layer 2): stream the hash, no full read. + localHash, err := lock.SHA256File(dest) + if err != nil { + if os.IsNotExist(err) { + return false, nil // missing → re-sync + } + return false, fmt.Errorf("checking synced env file %q: %w", dest, err) + } + if localHash != f.SHA256 { + return false, nil // local drift → re-sync (heals a poisoned lock) + } + continue + } data, err := os.ReadFile(dest) if err != nil { if os.IsNotExist(err) { @@ -956,11 +978,7 @@ func lockedStateInSync(cfg EnvConfig, resolved gitcache.ResolvedRef, projectRoot return false, nil // local drift → re-sync (heals a poisoned lock) } // Layer 3: source-authoritative check for plainly-written files. - if !sourceOK || strategy != StrategyReplace { - continue - } - k := f.Path + "\x00" + f.Dest - if selective[k] || mayCarryPins(f.Path, data) { + if mayCarryPins(f.Path, data) { continue } oid, ok := srcOID[f.Path] @@ -974,19 +992,6 @@ func lockedStateInSync(cfg EnvConfig, resolved gitcache.ResolvedRef, projectRoot return true, nil } -// mayCarryPins reports whether the on-disk content may carry `b.pin` -// annotations that make it legitimately diverge from the raw upstream blob. -// Mirrors applyPinsYAML's cheap pre-check: pinning is YAML-only and requires -// the annotation substring. False positives (a key literally named "b.pin") -// merely downgrade that file to the lock-hash comparison — safe. -func mayCarryPins(sourcePath string, data []byte) bool { - ext := strings.ToLower(filepath.Ext(sourcePath)) - if ext != ".yaml" && ext != ".yml" { - return false - } - return bytes.Contains(data, []byte(PinAnnotation)) -} - // safeShort returns the first 12 chars of s, or s itself if shorter. func safeShort(s string) string { if len(s) > 12 { diff --git a/pkg/env/pin.go b/pkg/env/pin.go index 0b31237..8feb000 100644 --- a/pkg/env/pin.go +++ b/pkg/env/pin.go @@ -55,6 +55,14 @@ const PinAnnotation = "b.pin" // for that one file. applyPinsYAML returns local verbatim when it // sees a root pin, so the splice's bytes are preserved exactly. // +// mayCarryPins is the shared cheap pre-check: pinning is YAML-only and +// requires the `b.pin` annotation substring, so a file failing either test +// can't possibly carry a pin. Substring matches can produce false positives +// (e.g. a key literally named "b.pin" elsewhere); callers only use this as a +// gate — applyPinsYAML falls through to the structural collectPinnedPaths +// (the source of truth), and the up-to-date fast path merely downgrades such +// a file to the lock-hash comparison. Both directions are safe. +// // Formatting caveat: when pin restoration actually substitutes a // subtree, the file is round-tripped through the yaml.v3 encoder, // so comments and whitespace on the affected file are NOT @@ -64,22 +72,9 @@ const PinAnnotation = "b.pin" // splice's bytes verbatim — so the common no-drift case keeps // splice's byte-preservation guarantees. func applyPinsYAML(local, pending []byte, filePath string) ([]byte, error) { - ext := strings.ToLower(filepath.Ext(filePath)) - if ext != ".yaml" && ext != ".yml" { - return pending, nil - } - if len(local) == 0 { - // No local file → nothing pinned to honor. - return pending, nil - } - // Cheap pre-check: a file with no `b.pin` substring anywhere - // can't possibly carry a pin annotation, and we'd spend the - // yaml.Unmarshal cost on every sync of every YAML file just to - // learn that. Substring matches can produce false positives - // (e.g. a key literally named "b.pin" elsewhere), but those - // just trigger the slow path — the structural collectPinnedPaths - // is the source of truth. - if !bytes.Contains(local, []byte(PinAnnotation)) { + // mayCarryPins covers the ext + annotation-substring gate (an empty + // local trivially has no annotation → nothing pinned to honor). + if !mayCarryPins(filePath, local) { return pending, nil } @@ -390,3 +385,15 @@ func addPath(doc *yaml.Node, path []string, value *yaml.Node) { n = found } } + +// mayCarryPins reports whether content may carry `b.pin` annotations that make +// it legitimately diverge from the raw upstream blob. Shared gate for +// applyPinsYAML and the up-to-date fast path — see the applyPinsYAML doc +// comment for the false-positive contract. +func mayCarryPins(sourcePath string, data []byte) bool { + ext := strings.ToLower(filepath.Ext(sourcePath)) + if ext != ".yaml" && ext != ".yml" { + return false + } + return bytes.Contains(data, []byte(PinAnnotation)) +} diff --git a/pkg/env/sync_source_verify_test.go b/pkg/env/sync_source_verify_test.go index d75a507..02278db 100644 --- a/pkg/env/sync_source_verify_test.go +++ b/pkg/env/sync_source_verify_test.go @@ -68,7 +68,8 @@ func sourceVerifyRepo(t *testing.T, extra map[string]string) (bare string, commi if err != nil { t.Fatal(err) } - return string(out[:40]) + // TrimSpace, not [:40]: sha256-object-format repos emit 64-char ids. + return strings.TrimSpace(string(out)) } return bare, commitB } From 7750925219418d143856403b2acea6eb1c2f2950 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Sat, 4 Jul 2026 23:18:17 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix(env):=20address=20review=20round=202=20?= =?UTF-8?q?=E2=80=94=20confirm=20pins=20structurally=20before=20exemption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot: mayCarryPins is a substring gate and can false-positive on a file that merely MENTIONS "b.pin" (comment/value). The fast path took that hit as grounds to skip the layer-3 source check, silently downgrading such a file to lock-only verification — reintroducing the undetectable stale-lock state this PR exists to close, for exactly that file. The exemption now requires structural confirmation: hasActivePins (pin.go) parses the content and checks collectPinnedPaths only when the cheap substring gate matches. No real pin → the file stays source-verified. Unparseable content cannot carry effective pins (applyPinsYAML leaves such files alone), so it also stays source-verified — which keeps templated .yaml files (invalid YAML) under source verification too. Test: TestSyncEnv_PinMentionDoesNotExemptFromSourceCheck — a stale file whose content mentions b.pin (no structural pin) at a poisoned lock must re-sync and heal; fails with the substring-only exemption. PinnedFileFastPathStaysSkippable still green (real pins keep skipping). Co-Authored-By: Claude Opus 4.8 --- pkg/env/env.go | 7 ++++-- pkg/env/pin.go | 16 +++++++++++++ pkg/env/sync_source_verify_test.go | 37 ++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/pkg/env/env.go b/pkg/env/env.go index 0c52b97..50ff673 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -977,8 +977,11 @@ func lockedStateInSync(cfg EnvConfig, resolved gitcache.ResolvedRef, projectRoot if fmt.Sprintf("%x", sha256.Sum256(data)) != f.SHA256 { return false, nil // local drift → re-sync (heals a poisoned lock) } - // Layer 3: source-authoritative check for plainly-written files. - if mayCarryPins(f.Path, data) { + // Layer 3: source-authoritative check for plainly-written files. A + // substring hit alone is not enough to exempt a file — confirm the + // pins structurally so a mere mention of "b.pin" (comment, value) + // doesn't silently exclude the file from source verification. + if mayCarryPins(f.Path, data) && hasActivePins(data) { continue } oid, ok := srcOID[f.Path] diff --git a/pkg/env/pin.go b/pkg/env/pin.go index 8feb000..540f660 100644 --- a/pkg/env/pin.go +++ b/pkg/env/pin.go @@ -397,3 +397,19 @@ func mayCarryPins(sourcePath string, data []byte) bool { } return bytes.Contains(data, []byte(PinAnnotation)) } + +// hasActivePins reports whether the content structurally carries at least one +// `b.pin` annotation. The fast path uses it to confirm a mayCarryPins substring +// hit before excluding a file from source verification — a substring false +// positive (e.g. a comment mentioning "b.pin") would otherwise silently +// downgrade that file to the lock-only comparison, reintroducing the +// undetectable-stale state this check exists to prevent. Unparseable content +// cannot carry effective pins (applyPinsYAML leaves such files alone), so it +// reports false and the file stays source-verified. +func hasActivePins(data []byte) bool { + var doc yaml.Node + if err := yaml.Unmarshal(data, &doc); err != nil { + return false + } + return len(collectPinnedPaths(&doc, nil)) > 0 +} diff --git a/pkg/env/sync_source_verify_test.go b/pkg/env/sync_source_verify_test.go index 02278db..cea2f20 100644 --- a/pkg/env/sync_source_verify_test.go +++ b/pkg/env/sync_source_verify_test.go @@ -215,6 +215,43 @@ func TestSyncEnv_PinnedFileFastPathStaysSkippable(t *testing.T) { } } +// TestSyncEnv_PinMentionDoesNotExemptFromSourceCheck: a file that merely +// MENTIONS "b.pin" (comment/value) without a structural pin annotation must +// still be source-verified — a substring false positive must not reintroduce +// the undetectable stale-lock state for that file (Copilot round-2). +func TestSyncEnv_PinMentionDoesNotExemptFromSourceCheck(t *testing.T) { + mention := "# docs: use b.pin to pin keys\nv: 1\n" + bare, commitB := sourceVerifyRepo(t, map[string]string{"cfg/doc.yaml": mention}) + project := t.TempDir() + cfg := EnvConfig{Ref: bare, Strategy: StrategyReplace, + Files: map[string]envmatch.GlobConfig{"cfg/*.yaml": {Dest: "configs"}}} + + res, err := SyncEnv(cfg, project, t.TempDir(), nil) + if err != nil { + t.Fatalf("sync A: %v", err) + } + le := lockFromResult(res) + + // Upstream changes the mentioning file; poison the lock like the S3 state. + newMention := "# docs: use b.pin to pin keys\nv: 2\n" + le.Commit = commitB(map[string]string{"cfg/doc.yaml": newMention}) + + res2, err := SyncEnv(cfg, project, t.TempDir(), le) + if err != nil { + t.Fatalf("sync B: %v", err) + } + if res2.Skipped { + t.Fatal("a b.pin MENTION (no structural pin) must not exempt the file from source verification") + } + data, err := os.ReadFile(filepath.Join(project, "configs", "doc.yaml")) + if err != nil { + t.Fatal(err) + } + if string(data) != newMention { + t.Errorf("disk = %q, want healed to the new content", data) + } +} + // TestSyncEnv_ClientStrategyFastPathUnaffected: under strategy client, local // divergence from upstream is the contract — the source check must not apply, // or every update would re-sync (and re-keep) forever. From 88b1ae8fc22023dd5bef3910695e694ddc6d5ac6 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Sat, 4 Jul 2026 23:24:18 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix(env):=20address=20final=20review=20?= =?UTF-8?q?=E2=80=94=20fail=20fast=20on=20malformed=20ls-tree,=20docs=20ca?= =?UTF-8?q?veat?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot verification pass: - ListTreeWithModesDir now errors on ls-tree lines without the full " " prefix instead of admitting entries with empty Type/OID, which source verification would read as perpetual drift (silent re-syncs) rather than a loud parse problem. git always emits three fields; anything else is corrupt output. (Also flagged by the round-2 subagent.) - env-sync.mdx: the offline/cache-miss fallback disables changed-glob detection too, not just source drift — say so. Co-Authored-By: Claude Opus 4.8 --- docs/env-sync.mdx | 2 +- pkg/gitcache/gitcache.go | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/docs/env-sync.mdx b/docs/env-sync.mdx index 35e673d..5f1e9c7 100644 --- a/docs/env-sync.mdx +++ b/docs/env-sync.mdx @@ -707,6 +707,6 @@ Uses shallow bare clones (`--depth 1`) to minimize disk usage. - **Verify**: `b verify` checks all artifacts against `b.lock` checksums. - **Conflicts**: If two env entries write to the same path, **b** warns before syncing. - **Auth**: Private repos need `GITHUB_TOKEN`, `GITLAB_TOKEN`, or `GITEA_TOKEN`. See [Authentication](/authentication). -- **Self-healing**: at an unchanged commit, `b update` verifies the on-disk files against the lock **and** the actual source content at the pinned commit (for plainly-synced files under `replace`). Any drift — local edits, a stale/corrupted lock, or a changed `files:` glob config — triggers a re-sync that reconciles via your strategy and re-records the lock. No `--force` needed. Source verification needs the pinned commit in the local git cache (fetched automatically when missing); if it can't be obtained — e.g. offline — that run falls back to lock-only verification. +- **Self-healing**: at an unchanged commit, `b update` verifies the on-disk files against the lock **and** the actual source content at the pinned commit (for plainly-synced files under `replace`). Any drift — local edits, a stale/corrupted lock, or a changed `files:` glob config — triggers a re-sync that reconciles via your strategy and re-records the lock. No `--force` needed. Source verification needs the pinned commit in the local git cache (fetched automatically when missing); if it can't be obtained — e.g. offline — that run falls back to lock-only verification, and neither source drift nor changed-glob detection runs until the commit is available again. - **Keeps are one-shot**: under `replace`, an interactive "keep" survives only until the next update — source is authoritative. For durable local divergence use `strategy: client`/`merge`, a `select` scope, or a `b.pin` annotation. - **Unchanged**: Files identical to upstream are automatically skipped. diff --git a/pkg/gitcache/gitcache.go b/pkg/gitcache/gitcache.go index 367ecd0..f98fae2 100644 --- a/pkg/gitcache/gitcache.go +++ b/pkg/gitcache/gitcache.go @@ -194,17 +194,14 @@ func ListTreeWithModesDir(dir, commit string) ([]TreeEntry, error) { } path := line[tabIdx+1:] fields := strings.Fields(line[:tabIdx]) - // ls-tree line shape: " \t". - mode := "100644" - if len(fields) >= 1 { - mode = fields[0] - } - entry := TreeEntry{Path: path, Mode: mode} - if len(fields) >= 3 { - entry.Type = fields[1] - entry.OID = fields[2] + // ls-tree line shape: " \t". Fail fast on + // anything else — silently defaulting would flow empty Type/OID into + // source verification, reading as perpetual drift instead of a loud + // parse problem. + if len(fields) < 3 { + return nil, fmt.Errorf("git ls-tree: unexpected line format: %q", line) } - entries = append(entries, entry) + entries = append(entries, TreeEntry{Path: path, Mode: fields[0], Type: fields[1], OID: fields[2]}) } return entries, nil }