amadan
zdd-4 second fix wave: a relative link hop resolves against its link's real parent, as the OS does (N3, R17)
linkChain, the hop walk ResolveLive and BoxPins share, resolved a relative target lexically against the holding link's directory. Through a symlinked directory that names the wrong file: live/h -> <tmp>/alias/current, alias -> <version dir>/links, current -> ../app reaches <version dir>/app on the OS but <tmp>/app lexically, so a worker failed to start and the real staged copy went unpinned. A relative target now joins filepath.EvalSymlinks of the holding link's parent, as the OS does. Absolute targets stay as written. A parent that does not exist keeps the lexical join (its hop does not exist either); any other error resolving a parent is an error, so the pin set fails closed. Every hop is still pinned, at the resolved path for relative hops. linkChain also returns the lexical join of each relative hop whose real path differs, and BoxPins pins those too, so a box whose opt dir is itself reached through a symlink keeps the pin it had under the opt dir's own name, which is how VersionPath names a staged copy. Tests: ResolveLive on Astra's layout returns the OS's answer (filepath.EvalSymlinks of the live link) and the file's digest; BoxPins pins the intermediate link and the staged file; a relative live link under a symlinked opt dir is pinned under both names. Existing tests unchanged and passing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+148
−22
internal/release/pins.go+7 −2internal/release/pins_test.go+45 −0internal/release/staged.go+42 −20internal/release/staged_test.go+54 −0
diff --git a/internal/release/pins.go b/internal/release/pins.go| index 598bcafb6..8f1591d6e 100644 |
| --- a/internal/release/pins.go |
| +++ b/internal/release/pins.go |
| @@ -42,7 +42,9 @@ func (f PinSourceFunc) StagedPins(ctx context.Context, optDir string) (StagedPat |
| // BoxPins is the part of the pin set this package reads itself: every hop of |
| // the symlink chain of every entry under <opt>/live (linkChain; a crashed |
| // swap's <host>.tmp included, as LivePins includes it), so a live link that |
| -// reaches a staged copy through another link still pins that copy, and the |
| +// reaches a staged copy through another link still pins that copy (a |
| +// relative hop pinned at the real path the OS reaches, and at its lexical |
| +// join too when the two differ), and the |
| // staged path of every row's Version and |
| // PrevVersion. routes is every registry row, unfiltered: a hibernating route, |
| // an expired canary and a mid-provision row all still refer to their builds. |
| @@ -57,13 +59,16 @@ func BoxPins(optDir string, routes []registry.Route) (StagedPaths, error) { |
| return nil, fmt.Errorf("read %s: %w", dir, err) |
| } |
| for _, e := range entries { |
| - _, hops, err := linkChain(filepath.Join(dir, e.Name())) |
| + _, hops, aliases, err := linkChain(filepath.Join(dir, e.Name())) |
| if err != nil { |
| return nil, err |
| } |
| for _, h := range hops { |
| pins.add(h) |
| } |
| + for _, h := range aliases { |
| + pins.add(h) |
| + } |
| } |
| for _, rt := range routes { |
| if rt.App == "" { |
diff --git a/internal/release/pins_test.go b/internal/release/pins_test.go| index 40c18135d..0fc767741 100644 |
| --- a/internal/release/pins_test.go |
| +++ b/internal/release/pins_test.go |
| @@ -243,3 +243,48 @@ func TestBoxPinsFailsOnALinkLoop(t *testing.T) { |
| t.Fatalf("BoxPins = %v, nil; want an error for a link loop", pins) |
| } |
| } |
| + |
| +// A relative hop under a symlinked directory pins what the OS reaches, the |
| +// staged file, along with the hop as written (Astra r2 N3, ruling R17). |
| +func TestBoxPinsPinsWhatARelativeHopUnderASymlinkedDirReaches(t *testing.T) { |
| + opt := t.TempDir() |
| + staged, current := aliasedChain(t, opt) |
| + pins, err := BoxPins(opt, nil) |
| + if err != nil { |
| + t.Fatalf("BoxPins: %v", err) |
| + } |
| + for _, p := range []string{current, staged} { |
| + if !pins.Has(p) { |
| + t.Errorf("BoxPins = %v, missing %s", pins, p) |
| + } |
| + } |
| +} |
| + |
| +// Resolving a relative hop's parent must not lose the pin a box with a |
| +// symlinked opt dir had before: the copy is pinned under the opt dir's own |
| +// name, which is how VersionPath names it, as well as under its real path. |
| +func TestBoxPinsRelativeLinkUnderASymlinkedOptDir(t *testing.T) { |
| + real := t.TempDir() |
| + opt := filepath.Join(t.TempDir(), "opt") |
| + if err := os.Symlink(real, opt); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := StageArtifact(opt, "bab", "v1", "km", []byte("v1")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.MkdirAll(filepath.Join(opt, "live"), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(filepath.Join("..", "versions", "v1", "km"), LiveLink(opt, "h.example.com")); err != nil { |
| + t.Fatal(err) |
| + } |
| + pins, err := BoxPins(opt, nil) |
| + if err != nil { |
| + t.Fatalf("BoxPins: %v", err) |
| + } |
| + for _, p := range []string{VersionPath(opt, "bab", "v1", "km"), VersionPath(real, "bab", "v1", "km")} { |
| + if !pins.Has(p) { |
| + t.Errorf("BoxPins = %v, missing %s", pins, p) |
| + } |
| + } |
| +} |
diff --git a/internal/release/staged.go b/internal/release/staged.go| index 223e29717..5c2760aef 100644 |
| --- a/internal/release/staged.go |
| +++ b/internal/release/staged.go |
| @@ -40,49 +40,71 @@ var ErrVersionReused = errors.New("version name reused with different content, s |
| const maxLinkHops = 40 |
| // linkChain follows link's leaf symlink chain one hop at a time: link's own |
| -// target, then that target's target for as long as it is a symlink. A |
| -// relative target is resolved against the directory of the link that holds |
| -// it, and parent directories are never resolved, so every hop is a cleaned |
| -// path as written. It returns link's raw first target and every hop in order; |
| -// the last hop is where the chain ends. A hop that does not exist ends the |
| -// chain and is included. A loop, more than maxLinkHops hops, or any other |
| -// error reading a hop is an error: a chain that cannot be followed to its end |
| -// names nothing for certain. ResolveLive and BoxPins both walk links with it, |
| -// so the build a worker execs and the copies the pin set protects cannot |
| -// disagree. |
| -func linkChain(link string) (raw string, hops []string, err error) { |
| +// target, then that target's target for as long as it is a symlink. An |
| +// absolute target is a hop as written (cleaned). A relative target is |
| +// resolved as the OS resolves it: against the REAL directory of the link |
| +// that holds it (filepath.EvalSymlinks of that link's parent), so a link |
| +// reached through a symlinked directory reaches what the OS reaches (Astra r2 |
| +// N3, ruling R17). A parent that does not exist leaves the lexical join, |
| +// whose hop then does not exist either; any other error resolving a parent |
| +// is an error. |
| +// |
| +// It returns link's raw first target, every hop in order (the last is where |
| +// the chain ends), and aliases: the lexical join of each relative hop whose |
| +// real path differs from it. BoxPins pins those too, so a box whose opt dir |
| +// is itself reached through a symlink keeps the pin it had under the opt |
| +// dir's own name, which is how VersionPath names a staged copy. A hop that |
| +// does not exist ends the chain and is included. A loop, more than |
| +// maxLinkHops hops, or any other error reading a hop is an error: a chain |
| +// that cannot be followed to its end names nothing for certain. ResolveLive |
| +// and BoxPins both walk links with it, so the build a worker execs and the |
| +// copies the pin set protects cannot disagree. |
| +func linkChain(link string) (raw string, hops, aliases []string, err error) { |
| raw, err = os.Readlink(link) |
| if err != nil { |
| - return "", nil, fmt.Errorf("read live link %s: %w", link, err) |
| + return "", nil, nil, fmt.Errorf("read live link %s: %w", link, err) |
| } |
| seen := map[string]bool{filepath.Clean(link): true} |
| holder, target := link, raw |
| for { |
| if len(hops) == maxLinkHops { |
| - return "", nil, fmt.Errorf("live link %s: more than %d symlink hops", link, maxLinkHops) |
| + return "", nil, nil, fmt.Errorf("live link %s: more than %d symlink hops", link, maxLinkHops) |
| } |
| if !filepath.IsAbs(target) { |
| - target = filepath.Join(filepath.Dir(holder), target) |
| + parent := filepath.Dir(holder) |
| + lexical := filepath.Join(parent, target) |
| + realParent, err := filepath.EvalSymlinks(parent) |
| + switch { |
| + case errors.Is(err, os.ErrNotExist): |
| + target = lexical |
| + case err != nil: |
| + return "", nil, nil, fmt.Errorf("live link %s: resolve %s: %w", link, parent, err) |
| + default: |
| + target = filepath.Join(realParent, target) |
| + if target != lexical { |
| + aliases = append(aliases, lexical) |
| + } |
| + } |
| } |
| target = filepath.Clean(target) |
| if seen[target] { |
| - return "", nil, fmt.Errorf("live link %s: symlink loop at %s", link, target) |
| + return "", nil, nil, fmt.Errorf("live link %s: symlink loop at %s", link, target) |
| } |
| seen[target] = true |
| hops = append(hops, target) |
| fi, err := os.Lstat(target) |
| if errors.Is(err, os.ErrNotExist) { |
| - return raw, hops, nil |
| + return raw, hops, aliases, nil |
| } |
| if err != nil { |
| - return "", nil, fmt.Errorf("live link %s: %w", link, err) |
| + return "", nil, nil, fmt.Errorf("live link %s: %w", link, err) |
| } |
| if fi.Mode()&os.ModeSymlink == 0 { |
| - return raw, hops, nil |
| + return raw, hops, aliases, nil |
| } |
| next, err := os.Readlink(target) |
| if err != nil { |
| - return "", nil, fmt.Errorf("live link %s: %w", link, err) |
| + return "", nil, nil, fmt.Errorf("live link %s: %w", link, err) |
| } |
| holder, target = target, next |
| } |
| @@ -94,7 +116,7 @@ func linkChain(link string) (raw string, hops []string, err error) { |
| // outside the version store; the path and digest identify the bytes either |
| // way. Demand is left empty: only CanonicalDemand can vouch for it. |
| func ResolveLive(optDir, host string) (StagedIdentity, error) { |
| - raw, hops, err := linkChain(LiveLink(optDir, host)) |
| + raw, hops, _, err := linkChain(LiveLink(optDir, host)) |
| if err != nil { |
| return StagedIdentity{}, err |
| } |
diff --git a/internal/release/staged_test.go b/internal/release/staged_test.go| index 4da781349..6588c993a 100644 |
| --- a/internal/release/staged_test.go |
| +++ b/internal/release/staged_test.go |
| @@ -113,6 +113,60 @@ func TestVerifyStaged(t *testing.T) { |
| } |
| } |
| +// aliasedChain builds Astra's r2 N3 layout: live/h -> <tmp>/alias/current, |
| +// where <tmp>/alias is a symlink to the staged version's "links" directory |
| +// and current -> ../km. The OS resolves current's relative target against |
| +// its REAL parent, the links directory, so the chain ends at the staged |
| +// file. A lexical join would end at <tmp>/km instead. It returns the staged |
| +// file's path and the intermediate link as written. |
| +func aliasedChain(t *testing.T, opt string) (staged, current string) { |
| + t.Helper() |
| + if err := StageArtifact(opt, "bab", "v1", "km", []byte("build one")); err != nil { |
| + t.Fatal(err) |
| + } |
| + staged = VersionPath(opt, "bab", "v1", "km") |
| + links := filepath.Join(filepath.Dir(staged), "links") |
| + if err := os.MkdirAll(links, 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(filepath.Join("..", "km"), filepath.Join(links, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + alias := filepath.Join(t.TempDir(), "alias") |
| + if err := os.Symlink(links, alias); err != nil { |
| + t.Fatal(err) |
| + } |
| + current = filepath.Join(alias, "current") |
| + if err := os.MkdirAll(filepath.Join(opt, "live"), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(current, LiveLink(opt, "h.example.com")); err != nil { |
| + t.Fatal(err) |
| + } |
| + return staged, current |
| +} |
| + |
| +// A relative hop resolves against the real directory of the link that holds |
| +// it, as the OS resolves it (Astra r2 N3, ruling R17). |
| +func TestResolveLiveResolvesARelativeHopAgainstItsRealParent(t *testing.T) { |
| + opt := t.TempDir() |
| + staged, _ := aliasedChain(t, opt) |
| + osAnswer, err := filepath.EvalSymlinks(LiveLink(opt, "h.example.com")) |
| + if err != nil { |
| + t.Fatalf("the OS cannot resolve the chain: %v", err) |
| + } |
| + if osAnswer != staged { |
| + t.Fatalf("setup: the OS resolves the chain to %s, want %s", osAnswer, staged) |
| + } |
| + id, err := ResolveLive(opt, "h.example.com") |
| + if err != nil { |
| + t.Fatalf("ResolveLive: %v", err) |
| + } |
| + if id.StagedPath != osAnswer || id.Digest != etagOf([]byte("build one")) { |
| + t.Fatalf("ResolveLive = %+v; want the OS's answer %s and the file's digest", id, osAnswer) |
| + } |
| +} |
| + |
| // A live link that names another link: the identity is the file at the end |
| // of the chain, not the intermediate link, which can move. |
| func TestResolveLiveFollowsAChainedLinkToTheFile(t *testing.T) { |