Keep subpackage metadata static

Reject build argument references in supplemental package names and descriptions while preserving substitution for dependency constraints.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Author: cpuguy83

docs/spec.schema.json

@@ -2230,14 +2230,14 @@
 					"type": [
 						"string"
 					],
-					"description": "Description is the package description. This is required — both RPM and\nDebian require a description/summary for every subpackage."
+					"description": "Description is the package description. This is required — both RPM and\nDebian require a description/summary for every subpackage.\nBuild arguments are not supported in this field."
 				},
 				"name": {
 					"type": [
 						"string",
 						"null"
 					],
-					"description": "Name overrides the default package name.\nBy default, the package name is \"\u003cparent\u003e-\u003ckey\u003e\" where \u003ckey\u003e is the map\nkey under which this SubPackage is defined. Set this to use a fully custom\npackage name instead."
+					"description": "Name overrides the default package name.\nBy default, the package name is \"\u003cparent\u003e-\u003ckey\u003e\" where \u003ckey\u003e is the map\nkey under which this SubPackage is defined. Set this to use a fully custom\npackage name instead. Build arguments are not supported in this field."
 				},
 				"provides": {
 					"$ref": "#/$defs/PackageDependencyList",

subpackage.go

@@ -3,6 +3,8 @@ package dalec
 import (
 	goerrors "errors"
 	"fmt"
+	"slices"
+	"unicode"
 
 	"github.com/moby/buildkit/frontend/dockerfile/shell"
 	"github.com/pkg/errors"
@@ -77,11 +79,12 @@ type SubPackage struct {
 	// Name overrides the default package name.
 	// By default, the package name is "<parent>-<key>" where <key> is the map
 	// key under which this SubPackage is defined. Set this to use a fully custom
-	// package name instead.
+	// package name instead. Build arguments are not supported in this field.
 	Name string `yaml:"name,omitempty" json:"name,omitempty"`
 
 	// Description is the package description. This is required — both RPM and
 	// Debian require a description/summary for every subpackage.
+	// Build arguments are not supported in this field.
 	Description string `yaml:"description" json:"description" jsonschema:"required"`
 
 	// Artifacts specifies which build outputs go into this supplemental package.
@@ -120,6 +123,14 @@ func (s *SubPackage) validate() error {
 		errs = append(errs, fmt.Errorf("description is required"))
 	}
 
+	if err := validateNoBuildArgReferences(s.Name); err != nil {
+		errs = append(errs, errors.Wrap(err, "name"))
+	}
+
+	if err := validateNoBuildArgReferences(s.Description); err != nil {
+		errs = append(errs, errors.Wrap(err, "description"))
+	}
+
 	if s.Artifacts != nil {
 		if err := s.Artifacts.validate(); err != nil {
 			errs = append(errs, errors.Wrap(err, "artifacts"))
@@ -132,22 +143,12 @@ func (s *SubPackage) validate() error {
 func (s *SubPackage) processBuildArgs(lex *shell.Lex, args map[string]string, allowArg func(string) bool) error {
 	var errs []error
 
-	if s.Name != "" {
-		updated, err := expandArgs(lex, s.Name, args, allowArg)
-		if err != nil {
-			errs = append(errs, errors.Wrap(err, "name"))
-		} else {
-			s.Name = updated
-		}
+	if err := validateNoBuildArgReferences(s.Name); err != nil {
+		errs = append(errs, errors.Wrap(err, "name"))
 	}
 
-	if s.Description != "" {
-		updated, err := expandArgs(lex, s.Description, args, allowArg)
-		if err != nil {
-			errs = append(errs, errors.Wrap(err, "description"))
-		} else {
-			s.Description = updated
-		}
+	if err := validateNoBuildArgReferences(s.Description); err != nil {
+		errs = append(errs, errors.Wrap(err, "description"))
 	}
 
 	if err := s.Dependencies.processBuildArgs(lex, args, allowArg); err != nil {
@@ -190,6 +191,37 @@ func (s *SubPackage) processBuildArgs(lex *shell.Lex, args map[string]string, al
 	return goerrors.Join(errs...)
 }
 
+func validateNoBuildArgReferences(value string) error {
+	lex := shell.NewLex('\\')
+	lex.SkipProcessQuotes = true
+
+	result, _ := lex.ProcessWordWithMatches(value, envGetterMap(nil))
+	refs := make([]string, 0, len(result.Unmatched))
+	for ref := range result.Unmatched {
+		if isBuildArgName(ref) {
+			refs = append(refs, ref)
+		}
+	}
+	if len(refs) == 0 {
+		return nil
+	}
+
+	slices.Sort(refs)
+	return fmt.Errorf("build arguments are not supported (found %q)", refs)
+}
+
+func isBuildArgName(value string) bool {
+	for i, r := range value {
+		if i == 0 && r != '_' && !unicode.IsLetter(r) {
+			return false
+		}
+		if i > 0 && r != '_' && !unicode.IsLetter(r) && !unicode.IsDigit(r) {
+			return false
+		}
+	}
+	return value != ""
+}
+
 func (s *SubPackage) fillDefaults() {
 }
 

subpackage_test.go

@@ -115,6 +115,93 @@ func TestSubPackageValidation(t *testing.T) {
 	}
 }
 
+func TestSubPackageMetadataIsStatic(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name      string
+		pkg       SubPackage
+		errSubstr string
+	}{
+		{
+			name: "A name containing a braced build argument is rejected",
+			pkg: SubPackage{
+				Name:        "tools-${PACKAGE_SUFFIX}",
+				Description: "Tools package",
+			},
+			errSubstr: `name: build arguments are not supported (found ["PACKAGE_SUFFIX"])`,
+		},
+		{
+			name: "A description containing an unbraced build argument is rejected",
+			pkg: SubPackage{
+				Name:        "tools",
+				Description: "Tools for $TARGETARCH",
+			},
+			errSubstr: `description: build arguments are not supported (found ["TARGETARCH"])`,
+		},
+		{
+			name: "Literal dollar signs do not make static metadata invalid",
+			pkg: SubPackage{
+				Name:        "tools-$",
+				Description: "Tools costing $5",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			err := tc.pkg.validate()
+
+			if tc.errSubstr == "" {
+				assert.NilError(t, err)
+				return
+			}
+			assert.ErrorContains(t, err, tc.errSubstr)
+		})
+	}
+}
+
+func TestSubPackageBuildArgumentSubstitution(t *testing.T) {
+	t.Parallel()
+
+	t.Run("A runtime dependency version containing a build argument is substituted", func(t *testing.T) {
+		t.Parallel()
+
+		spec := Spec{
+			Args: map[string]string{
+				"PACKAGE_VERSION": "",
+			},
+			Targets: map[string]Target{
+				"linux": {
+					Packages: map[string]SubPackage{
+						"tools": {
+							Name:        "tools",
+							Description: "Tools package",
+							Dependencies: &SubPackageDependencies{
+								Runtime: PackageDependencyList{
+									"runtime": {
+										Version: []string{"=${PACKAGE_VERSION}"},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		err := spec.SubstituteArgs(map[string]string{"PACKAGE_VERSION": "1.2.3"})
+
+		assert.NilError(t, err)
+		pkg := spec.Targets["linux"].Packages["tools"]
+		assert.Equal(t, pkg.Name, "tools")
+		assert.Equal(t, pkg.Description, "Tools package")
+		assert.DeepEqual(t, pkg.Dependencies.Runtime["runtime"].Version, []string{"=1.2.3"})
+	})
+}
+
 func TestValidateSubPackageNames(t *testing.T) {
 	t.Parallel()