carlos / platform Public

zdd-4 final review: live link chains are followed to their end (F3, R14), ensureStaged's doc stops promising a restage (R15)

ResolveLive's StagedPath is the end of the live link's leaf symlink chain,
and BoxPins pins every hop of every live entry's chain, so a live link that
reaches a staged copy through another link still protects that copy from a
restage. Both walk the chain with one helper, linkChain: relative hops
resolve against the holding link's directory, parents are not resolved, a
missing hop ends the chain and is pinned, and a loop, more than 40 hops or
an unreadable hop is an error (no restage).

ensureStaged's doc now says a refused copy stays refused until a pass that
adopts that version restages it, or the release ships under a new name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 2 weeks ago f2037fa456963fc74ccd89d8343335b72a00161e parent acc6de7

6 files changed, +164 −24

internal/release/adopt.go +3 −2
internal/release/pins.go +9 −9
internal/release/pins_test.go +51 −0
internal/release/stage_reuse_test.go +14 −0
internal/release/staged.go +61 −13
internal/release/staged_test.go +26 −0

diff --git a/internal/release/adopt.go b/internal/release/adopt.go

index 89bec3ec0..b2224db81 100644

--- a/internal/release/adopt.go

+++ b/internal/release/adopt.go

@@ -1738,8 +1738,9 @@ type stageCheck struct {

// ensureStaged is ensureStagedChecked with no reference check. It has no

// registry in hand, so it cannot know what refers to a staged copy and never

-// restages one: a copy that differs from its release is refused, and a full

-// pass restages it.

+// restages one: a copy that differs from its release is refused. It stays

+// refused until a pass that adopts that version restages it, or the release

+// is shipped under a new name.

func ensureStaged(ctx context.Context, s Store, optDir, acct, app, ver, host string) (*Manifest, error) {

return ensureStagedChecked(ctx, s, optDir, acct, app, ver, host, stageCheck{})

}

diff --git a/internal/release/pins.go b/internal/release/pins.go

index 6401f082d..0e37c1883 100644

--- a/internal/release/pins.go

+++ b/internal/release/pins.go

@@ -39,9 +39,11 @@ func (f PinSourceFunc) StagedPins(ctx context.Context, optDir string) (StagedPat

return f(ctx, optDir)

}

-// BoxPins is the part of the pin set this package reads itself: the target

-// of every entry under <opt>/live (a crashed swap's <host>.tmp included, as

-// LivePins includes it), and the staged path of every row's Version and

+// 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

+// 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.

@@ -55,15 +57,13 @@ func BoxPins(optDir string, routes []registry.Route) (StagedPaths, error) {

return nil, fmt.Errorf("read %s: %w", dir, err)

}

for _, e := range entries {

- link := filepath.Join(dir, e.Name())

- target, err := os.Readlink(link)

+ _, hops, err := linkChain(filepath.Join(dir, e.Name()))

if err != nil {

- return nil, fmt.Errorf("read live link %s: %w", link, err)

+ return nil, err

}

- if !filepath.IsAbs(target) {

- target = filepath.Join(dir, target)

+ for _, h := range hops {

+ pins.add(h)

}

- pins.add(target)

}

for _, rt := range routes {

if rt.App == "" {

diff --git a/internal/release/pins_test.go b/internal/release/pins_test.go

index 337c00620..40c18135d 100644

--- a/internal/release/pins_test.go

+++ b/internal/release/pins_test.go

@@ -192,3 +192,54 @@ func TestGenerationPinsFailClosed(t *testing.T) {

})

}

+// A live link that names another link (an operator's own "current" pointer)

+// refers to every hop of the chain, the staged copy at its end included.

+func TestBoxPinsPinsEveryHopOfAChainedLink(t *testing.T) {

+ opt := t.TempDir()

+ if err := StageArtifact(opt, "bab", "v1", "km", []byte("v1")); err != nil {

+ t.Fatal(err)

+ }

+ current := filepath.Join(t.TempDir(), "operator-current")

+ if err := os.Symlink(VersionPath(opt, "bab", "v1", "km"), current); err != nil {

+ t.Fatal(err)

+ }

+ 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)

+ }

+ pins, err := BoxPins(opt, nil)

+ if err != nil {

+ t.Fatalf("BoxPins: %v", err)

+ }

+ for _, p := range []string{current, VersionPath(opt, "bab", "v1", "km")} {

+ if !pins.Has(p) {

+ t.Errorf("BoxPins = %v, missing %s", pins, p)

+ }

+// A chain that cannot be followed to its end is an unknown reference: no

+// answer, so no restage.

+func TestBoxPinsFailsOnALinkLoop(t *testing.T) {

+ opt := t.TempDir()

+ dir := t.TempDir()

+ a, b := filepath.Join(dir, "a"), filepath.Join(dir, "b")

+ if err := os.Symlink(b, a); err != nil {

+ t.Fatal(err)

+ }

+ if err := os.Symlink(a, b); err != nil {

+ t.Fatal(err)

+ }

+ if err := os.MkdirAll(filepath.Join(opt, "live"), 0o755); err != nil {

+ t.Fatal(err)

+ }

+ if err := os.Symlink(a, LiveLink(opt, "h.example.com")); err != nil {

+ t.Fatal(err)

+ }

+ if pins, err := BoxPins(opt, nil); err == nil {

+ t.Fatalf("BoxPins = %v, nil; want an error for a link loop", pins)

+ }

diff --git a/internal/release/stage_reuse_test.go b/internal/release/stage_reuse_test.go

index 47493e848..7a370bf1a 100644

--- a/internal/release/stage_reuse_test.go

+++ b/internal/release/stage_reuse_test.go

@@ -134,6 +134,20 @@ func TestStaleCopyALiveLinkNamesIsNotOverwritten(t *testing.T) {

f.assertRefused(t, f.pass(t), ErrVersionReused.Error())

}

+// A live link that reaches the stale copy through another link still refers

+// to it: every hop of the chain is a reference.

+func TestStaleCopyAChainedLiveLinkNamesIsNotOverwritten(t *testing.T) {

+ f := newReuseFixture(t)

+ current := filepath.Join(t.TempDir(), "operator-current")

+ if err := os.Symlink(VersionPath(f.opt, "bab", "v2", "myapp"), current); err != nil {

+ t.Fatal(err)

+ }

+ if err := os.Symlink(current, LiveLink(f.opt, "h2.example.com")); err != nil {

+ t.Fatal(err)

+ }

+ f.assertRefused(t, f.pass(t), ErrVersionReused.Error())

// GoodVersion and live generations arrive through a PinSource (plan 5).

func TestStaleCopyAPinSourceNamesIsNotOverwritten(t *testing.T) {

f := newReuseFixture(t)

diff --git a/internal/release/staged.go b/internal/release/staged.go

index d42b12f2d..223e29717 100644

--- a/internal/release/staged.go

+++ b/internal/release/staged.go

@@ -35,22 +35,70 @@ var ErrStagedCopyChanged = errors.New("staged copy no longer matches the digest

// overwritten. The text is the spec's.

var ErrVersionReused = errors.New("version name reused with different content, ship under a new name")

-// ResolveLive reads host's live link ONCE and reports the file it names and

-// that file's sha256. Version is the link's version as LiveVersion reads it,

-// "" for a link 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) {

- link := LiveLink(optDir, host)

- raw, err := os.Readlink(link)

+// maxLinkHops caps a live link's symlink chain, as Linux caps the symlinks

+// one path lookup follows.

+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) {

+ raw, err = os.Readlink(link)

if err != nil {

- return StagedIdentity{}, fmt.Errorf("read live link %s: %w", link, err)

+ return "", nil, fmt.Errorf("read live link %s: %w", link, err)

}

- path := raw

- if !filepath.IsAbs(path) {

- path = filepath.Join(filepath.Dir(link), path)

+ 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)

+ }

+ if !filepath.IsAbs(target) {

+ target = filepath.Join(filepath.Dir(holder), target)

+ }

+ target = filepath.Clean(target)

+ if seen[target] {

+ return "", 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

+ }

+ if err != nil {

+ return "", nil, fmt.Errorf("live link %s: %w", link, err)

+ }

+ if fi.Mode()&os.ModeSymlink == 0 {

+ return raw, hops, nil

+ }

+ next, err := os.Readlink(target)

+ if err != nil {

+ return "", nil, fmt.Errorf("live link %s: %w", link, err)

+ }

+ holder, target = target, next

+ }

+// ResolveLive reads host's live link ONCE and reports the file at the end of

+// its symlink chain (linkChain) and that file's sha256. Version is the link's

+// version as LiveVersion reads it, from the link's own target, "" for a link

+// 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))

+ if err != nil {

+ return StagedIdentity{}, err

}

- path = filepath.Clean(path)

+ path := hops[len(hops)-1]

sum, err := FileSHA256(path)

if err != nil {

return StagedIdentity{}, fmt.Errorf("hash %s: %w", path, err)

diff --git a/internal/release/staged_test.go b/internal/release/staged_test.go

index 5f9b2cd36..4da781349 100644

--- a/internal/release/staged_test.go

+++ b/internal/release/staged_test.go

@@ -112,3 +112,29 @@ func TestVerifyStaged(t *testing.T) {

t.Fatalf("VerifyStaged with no retained path or digest = %v, want ErrStagedCopyChanged", err)

}

+// 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) {

+ opt := t.TempDir()

+ if err := StageArtifact(opt, "bab", "v1", "km", []byte("build one")); err != nil {

+ t.Fatal(err)

+ }

+ current := filepath.Join(t.TempDir(), "operator-current")

+ if err := os.Symlink(VersionPath(opt, "bab", "v1", "km"), current); err != nil {

+ t.Fatal(err)

+ }

+ 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)

+ }

+ id, err := ResolveLive(opt, "h.example.com")

+ if err != nil {

+ t.Fatalf("ResolveLive: %v", err)

+ }

+ if id.StagedPath != VersionPath(opt, "bab", "v1", "km") || id.Digest != etagOf([]byte("build one")) {

+ t.Fatalf("ResolveLive = %+v; want path %s and the file's digest", id, VersionPath(opt, "bab", "v1", "km"))

+ }