mount: honor a claimed transform as a chain suffix

A claimed transform was recorded but never actually left unapplied:
every parsed transformer in a mount's chain was applied in full
regardless of what the caller had claimed, so a shim advertising it
performs "mkdir" in "format/mkdir/overlay" never got the chance to,
since the manager silently applied it anyway. This predates the shim
capability protocol — it was reachable before through the "format/*"
annotation convention — but this series turns it into a documented,
supported way for a shim to claim a transform, so a shim relying on it
(at least one is: Kata's EROFS snapshotter shim, per its annotation
usage) would find its claim silently ignored.

A claimed transform can only be honored as a suffix of the chain: each
transform's input is the previous one's output, so an outer, unclaimed
transform must still be applied even when an inner one is claimed,
since the inner one cannot run without it. planActivation now tracks,
per mount, how much of its chain — starting from the beginning — the
manager must apply itself: through the last unclaimed transform, which
by construction covers every transform before it too. Applying only
that prefix and returning the true, contiguous claimed suffix to the
caller is what actually makes the claim meaningful. This is only
observable for mounts[firstSystemMount], the one mount partially
transformed by the manager and returned to the caller to finish; every
earlier mount is fully internal to the manager and was, and remains,
always fully transformed regardless of any claim.

While changing this expression, also guard it against the case where
firstSystemMount reaches len(mounts) — every mount consumed inside the
manager, with no system mount left to transform — which indexes out of
range today. This is a distinct, pre-existing latent panic (for
example, reachable via WithTemporary combined with a transformed mount
type), tracked separately from the claim-handling fix above; guarding
it here only prevents a crash newly reachable by touching this line,
it does not address the mount.ActivationInfo.System non-empty
invariant that scenario also violates.

Signed-off-by: Derek McGowan <derek@mcg.dev>

core/mount/manager/manager.go

@@ -408,9 +408,17 @@ func (mm *mountManager) Activate(ctx context.Context, name string, mounts []moun
 		mounted = append(mounted, active)
 	}
 
-	// If first system mount is converted, fill in the format
-	if mountConv != nil {
-		for _, tr := range mountConv[firstSystemMount] {
+	// If the first system mount has transforms, apply the ones the caller
+	// has not claimed. A claim can only be honored as a suffix of the
+	// chain: applyCount always covers at least every transform up to and
+	// including the last unclaimed one, since each depends on the last's
+	// output; see planActivation.
+	//
+	// firstSystemMount can reach len(mounts) when every mount is handled
+	// inside the manager (for example, every mount claimed via Temporary);
+	// there is then no system mount left to transform.
+	if mountConv != nil && firstSystemMount < len(mounts) {
+		for _, tr := range mountConv[firstSystemMount][:plan.applyCount[firstSystemMount]] {
 			newM, err := tr.Transform(ctx, mounts[firstSystemMount], mounted)
 			if err != nil {
 				return mount.ActivationInfo{}, err

core/mount/manager/plan.go

@@ -42,6 +42,18 @@ type activationPlan struct {
 	// prefix.
 	transforms [][]mount.Transformer
 
+	// applyCount[i] is how many of transforms[i], from the start, the
+	// manager must apply itself: through the last transform in the chain the
+	// caller did not claim. Meaningful only for mounts[firstSystemMount],
+	// the one mount returned to the caller with some of its own transforms
+	// possibly still unapplied; every earlier mount's transforms are always
+	// applied in full; they are internal to the manager and never exposed
+	// to the caller. A transform, once applied, resolves the input the next
+	// one in the chain depends on, so a run of claimed transforms can only
+	// be honored as a suffix: transforms strictly after the last unclaimed
+	// one in the chain.
+	applyCount []int
+
 	// handlers[i] is the Handler to use for mounts[i], or nil to perform a
 	// plain system mount.
 	handlers []mount.Handler
@@ -51,6 +63,16 @@ type activationPlan struct {
 // must apply and whether the manager or the caller performs the mount
 // itself, honoring the mount types and transforms config.AllowMountTypes and
 // config.AllowTransforms claim.
+//
+// A claimed transform is honored only for mounts[firstSystemMount] in the
+// result, and only as a suffix of its transform chain: transforms apply
+// outside-in, so an inner transform's input depends on an outer, unclaimed
+// transform's output, and the manager must still apply that outer transform
+// even though the caller performs the suffix itself. For example, in
+// "format/mkdir/overlay", claiming "mkdir" causes the manager to apply
+// "format" and hand the caller "mkdir/overlay" to finish; claiming "format"
+// alone does nothing, since "mkdir" cannot run without it having already
+// run.
 func planActivation(ctx context.Context, mounts []mount.Mount, config mount.ActivateOptions, transforms map[string]mount.Transformer, handlers map[string]mount.Handler) activationPlan {
 	shouldTransform := func(p string, t string) bool {
 		return !slices.Contains(config.AllowTransforms, p) && !slices.Contains(config.AllowMountTypes, t)
@@ -73,7 +95,10 @@ func planActivation(ctx context.Context, mounts []mount.Mount, config mount.Acti
 				break
 			}
 
-			if shouldTransform(transformType, mounts[i].Type) { //nolint:gosec // G602: i is bounded by range mounts
+			// mustApply reports whether the manager itself must apply this
+			// transform, as opposed to the caller having claimed it.
+			mustApply := shouldTransform(transformType, mounts[i].Type) //nolint:gosec // G602: i is bounded by range mounts
+			if mustApply {
 				// At least everything before this must be mounted
 				// by the mount manager
 				plan.firstSystemMount = i
@@ -85,11 +110,21 @@ func planActivation(ctx context.Context, mounts []mount.Mount, config mount.Acti
 			if plan.transforms == nil {
 				plan.transforms = make([][]mount.Transformer, len(mounts))
 			}
+			if plan.applyCount == nil {
+				plan.applyCount = make([]int, len(mounts))
+			}
 
 			plan.transforms[i] = append(plan.transforms[i], typeTransformer{
 				Transformer: tr,
 				mountType:   mt,
 			})
+			if mustApply {
+				// Every transform up to and including this one must be
+				// applied by the manager: this one because it is unclaimed,
+				// and every one before it because this one's input depends
+				// on their output.
+				plan.applyCount[i] = len(plan.transforms[i])
+			}
 
 			mountType = mt
 		}

core/mount/manager/plan_test.go

@@ -65,30 +65,47 @@ func TestPlanActivationTransformChain(t *testing.T) {
 		name              string
 		opts              []mount.ActivateOpt
 		expectFirstSystem int
+		expectApplyCount  int
 	}{
 		{
-			name:              "nothing claimed requires the manager",
+			// Nothing claimed: the manager must apply the whole chain, the
+			// same as it always has.
+			name:              "nothing claimed applies the whole chain",
 			expectFirstSystem: 0,
+			expectApplyCount:  2,
 		},
 		{
-			name:              "a claimed transform still requires the manager for now",
+			// Claiming the innermost transform lets the manager stop after
+			// the one before it: mkdir is left for the caller.
+			name:              "innermost transform claimed is honored",
 			opts:              []mount.ActivateOpt{mount.WithAllowTransform("mkdir")},
 			expectFirstSystem: 0,
+			expectApplyCount:  1,
 		},
 		{
-			name: "every transform claimed no longer requires the manager",
+			// Claiming the outermost transform alone cannot be honored:
+			// mkdir's input depends on format having already run.
+			name:              "outermost transform claimed alone is not honored",
+			opts:              []mount.ActivateOpt{mount.WithAllowTransform("format")},
+			expectFirstSystem: 0,
+			expectApplyCount:  2,
+		},
+		{
+			name: "both transforms claimed applies neither",
 			opts: []mount.ActivateOpt{
 				mount.WithAllowTransform("format"),
 				mount.WithAllowTransform("mkdir"),
 			},
 			expectFirstSystem: -1,
+			expectApplyCount:  0,
 		},
 		{
 			// Claiming the full literal mount type is equivalent to
 			// claiming every transform in its chain.
-			name:              "full mount type claimed no longer requires the manager",
+			name:              "full mount type claimed applies neither",
 			opts:              []mount.ActivateOpt{mount.WithAllowMountType("format/mkdir/overlay")},
 			expectFirstSystem: -1,
+			expectApplyCount:  0,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -99,11 +116,33 @@ func TestPlanActivationTransformChain(t *testing.T) {
 
 			plan := planActivation(context.Background(), mounts, config, testTransforms, nil)
 			assert.Equal(t, tc.expectFirstSystem, plan.firstSystemMount)
-			require.Len(t, plan.transforms[0], 2)
+			if tc.expectFirstSystem >= 0 {
+				require.NotNil(t, plan.applyCount)
+				assert.Equal(t, tc.expectApplyCount, plan.applyCount[0])
+				require.Len(t, plan.transforms[0], 2)
+			}
 		})
 	}
 }
 
+// TestPlanActivationClaimedGapInMiddle covers a three transform chain where
+// an unclaimed transform sits between two claimed ones: the middle one still
+// forces the manager to apply everything up to and including it, regardless
+// of the outer claim, and only the true suffix after it is left claimed.
+func TestPlanActivationClaimedGapInMiddle(t *testing.T) {
+	mounts := []mount.Mount{{Type: "format/mkfs/mkdir/overlay"}}
+	config := mount.ActivateOptions{}
+	mount.WithAllowTransform("format")(&config)
+	mount.WithAllowTransform("mkdir")(&config)
+	// mkfs is left unclaimed.
+
+	plan := planActivation(context.Background(), mounts, config, testTransforms, nil)
+	require.Equal(t, 0, plan.firstSystemMount)
+	require.Len(t, plan.transforms[0], 3)
+	// format(0) and mkfs(1) must be applied; mkdir(2) is left to the caller.
+	assert.Equal(t, 2, plan.applyCount[0])
+}
+
 func TestPlanActivationUnknownTransform(t *testing.T) {
 	mounts := []mount.Mount{{Type: "unknown/overlay"}}
 	plan := planActivation(context.Background(), mounts, mount.ActivateOptions{}, testTransforms, nil)
@@ -173,3 +212,19 @@ func TestPlanActivationTemporary(t *testing.T) {
 	require.Len(t, plan.handlers, 1)
 	assert.Equal(t, 1, plan.firstSystemMount)
 }
+
+func TestPlanActivationTemporaryIgnoresTransformClaims(t *testing.T) {
+	mounts := []mount.Mount{{Type: "format/mkdir/overlay"}}
+	config := mount.ActivateOptions{}
+	mount.WithTemporary(&config)
+	mount.WithAllowTransform("format")(&config)
+	mount.WithAllowTransform("mkdir")(&config)
+
+	plan := planActivation(context.Background(), mounts, config, testTransforms, nil)
+	// Temporary forces this, the only mount, to be handled, advancing
+	// firstSystemMount past it. It does not change how much of a transform
+	// chain is required, which is governed solely by what is claimed: both
+	// transforms here are claimed, so applyCount stays at 0.
+	assert.Equal(t, 1, plan.firstSystemMount)
+	assert.Equal(t, 0, plan.applyCount[0])
+}