diff --git a/internal/app/azldev/cmds/component/list_test.go b/internal/app/azldev/cmds/component/list_test.go index 90ce47b6..baba511c 100644 --- a/internal/app/azldev/cmds/component/list_test.go +++ b/internal/app/azldev/cmds/component/list_test.go @@ -4,6 +4,7 @@ package component_test import ( + "path/filepath" "testing" "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/cmds/component" @@ -61,4 +62,66 @@ func TestListComponents_OneComponent(t *testing.T) { result := results[0] assert.Equal(t, testComponentName, result.Name) assert.Equal(t, testSpecPath, result.Spec.Path) + assert.Empty(t, result.RenderedSpecDir, "RenderedSpecDir should be empty when rendered-specs-dir is not configured") +} + +func TestListComponents_WithRenderedSpecsDir(t *testing.T) { + const ( + testComponentName = "vim" + testSpecPath = "/path/to/spec" + testRenderedDir = "/path/to/repo/specs" + ) + + testEnv := testutils.NewTestEnv(t) + testEnv.Config.Project.RenderedSpecsDir = testRenderedDir + testEnv.Config.Components[testComponentName] = projectconfig.ComponentConfig{ + Name: testComponentName, + Spec: projectconfig.SpecSource{ + Path: testSpecPath, + }, + } + + options := component.ListComponentOptions{ + ComponentFilter: components.ComponentFilter{ + ComponentNamePatterns: []string{testComponentName}, + }, + } + + results, err := component.ListComponentConfigs(testEnv.Env, &options) + require.NoError(t, err) + require.Len(t, results, 1) + + result := results[0] + assert.Equal(t, testComponentName, result.Name) + assert.Equal(t, filepath.Join(testRenderedDir, testComponentName), result.RenderedSpecDir) +} + +func TestListComponents_MultipleWithRenderedSpecsDir(t *testing.T) { + const testRenderedDir = "/rendered/specs" + + testEnv := testutils.NewTestEnv(t) + testEnv.Config.Project.RenderedSpecsDir = testRenderedDir + + testEnv.Config.Components["curl"] = projectconfig.ComponentConfig{ + Name: "curl", + Spec: projectconfig.SpecSource{Path: "/specs/curl.spec"}, + } + testEnv.Config.Components["vim"] = projectconfig.ComponentConfig{ + Name: "vim", + Spec: projectconfig.SpecSource{Path: "/specs/vim.spec"}, + } + + options := component.ListComponentOptions{ + ComponentFilter: components.ComponentFilter{ + IncludeAllComponents: true, + }, + } + + results, err := component.ListComponentConfigs(testEnv.Env, &options) + require.NoError(t, err) + require.Len(t, results, 2) + + for _, result := range results { + assert.Equal(t, filepath.Join(testRenderedDir, result.Name), result.RenderedSpecDir) + } } diff --git a/internal/app/azldev/cmds/component/render.go b/internal/app/azldev/cmds/component/render.go index 052dd2ea..2819b94d 100644 --- a/internal/app/azldev/cmds/component/render.go +++ b/internal/app/azldev/cmds/component/render.go @@ -251,9 +251,10 @@ func checkRenderErrors(results []*RenderResult, failOnError bool) error { // preparedComponent holds the intermediate state after source preparation, // before mock processing. type preparedComponent struct { - index int - comp components.Component - specFilename string // e.g., "curl.spec" + index int + comp components.Component + specFilename string // e.g., "curl.spec" + compOutputDir string // validated output path computed in phase 1 } // prepResult pairs a prepared component (on success) or a render result (on error). @@ -334,17 +335,15 @@ func prepWithSemaphore( allowOverwrite bool, ) prepResult { componentName := comp.GetName() - compOutputDir := filepath.Join(outputDir, componentName) - // Validate component name before any filesystem work to prevent path traversal. - if !sources.IsSimpleName(componentName) { + // Validate component name and compute output directory. + compOutputDir, nameErr := components.RenderedSpecDir(outputDir, componentName) + if nameErr != nil { return prepResult{index: index, result: &RenderResult{ Component: componentName, OutputDir: "(invalid)", Status: renderStatusError, - Error: fmt.Sprintf( - "component name %#q is invalid or contains path separators/traversal sequences", - componentName), + Error: nameErr.Error(), }} } @@ -385,6 +384,7 @@ func prepWithSemaphore( } prep.index = index + prep.compOutputDir = compOutputDir return prepResult{index: index, prepared: prep} } @@ -572,7 +572,7 @@ func finishOneComponent( allowOverwrite bool, ) *RenderResult { componentName := prep.comp.GetName() - compOutputDir := filepath.Join(outputDir, componentName) + compOutputDir := prep.compOutputDir // Context-aware semaphore acquisition. select { diff --git a/internal/app/azldev/core/components/renderedspecdir.go b/internal/app/azldev/core/components/renderedspecdir.go new file mode 100644 index 00000000..0b6501e3 --- /dev/null +++ b/internal/app/azldev/core/components/renderedspecdir.go @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package components + +import ( + "fmt" + "path/filepath" + + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" +) + +// RenderedSpecDir returns the rendered spec output directory for a given component. +// The path is computed as {renderedSpecsDir}/{componentName}. +// Returns an empty string if renderedSpecsDir is not configured (empty). +// Returns an error if componentName is unsafe (absolute, contains path separators +// or traversal sequences). +func RenderedSpecDir(renderedSpecsDir, componentName string) (string, error) { + if err := fileutils.ValidateFilename(componentName); err != nil { + return "", fmt.Errorf("invalid component name for rendered spec dir:\n%w", err) + } + + if renderedSpecsDir == "" { + return "", nil + } + + return filepath.Join(renderedSpecsDir, componentName), nil +} diff --git a/internal/app/azldev/core/components/renderedspecdir_test.go b/internal/app/azldev/core/components/renderedspecdir_test.go new file mode 100644 index 00000000..690baec1 --- /dev/null +++ b/internal/app/azldev/core/components/renderedspecdir_test.go @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package components_test + +import ( + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRenderedSpecDir(t *testing.T) { + t.Run("ReturnsPathWhenConfigured", func(t *testing.T) { + result, err := components.RenderedSpecDir("/path/to/specs", "vim") + require.NoError(t, err) + assert.Equal(t, "/path/to/specs/vim", result) + }) + + t.Run("ReturnsEmptyWhenNotConfigured", func(t *testing.T) { + result, err := components.RenderedSpecDir("", "vim") + require.NoError(t, err) + assert.Empty(t, result) + }) + + t.Run("HandlesComponentNameWithDashes", func(t *testing.T) { + result, err := components.RenderedSpecDir("/rendered", "my-component") + require.NoError(t, err) + assert.Equal(t, "/rendered/my-component", result) + }) + + t.Run("RejectsAbsoluteComponentName", func(t *testing.T) { + _, err := components.RenderedSpecDir("/rendered", "/tmp") + assert.Error(t, err) + }) + + t.Run("RejectsTraversalInComponentName", func(t *testing.T) { + _, err := components.RenderedSpecDir("/rendered", "../escape") + assert.Error(t, err) + }) + + t.Run("RejectsPathSeparatorInComponentName", func(t *testing.T) { + _, err := components.RenderedSpecDir("/rendered", "sub/dir") + assert.Error(t, err) + }) + + t.Run("RejectsEmptyComponentName", func(t *testing.T) { + _, err := components.RenderedSpecDir("/rendered", "") + assert.Error(t, err) + }) + + t.Run("RejectsDotComponentName", func(t *testing.T) { + _, err := components.RenderedSpecDir("/rendered", ".") + assert.Error(t, err) + }) + + t.Run("RejectsDotDotComponentName", func(t *testing.T) { + _, err := components.RenderedSpecDir("/rendered", "..") + assert.Error(t, err) + }) + + t.Run("ValidatesEvenWhenNotConfigured", func(t *testing.T) { + // Component name is validated even when renderedSpecsDir is empty, + // so invalid names are caught early regardless of configuration. + _, err := components.RenderedSpecDir("", "../escape") + assert.Error(t, err) + }) +} diff --git a/internal/app/azldev/core/components/resolver.go b/internal/app/azldev/core/components/resolver.go index 654053c5..4d66d59e 100644 --- a/internal/app/azldev/core/components/resolver.go +++ b/internal/app/azldev/core/components/resolver.go @@ -124,7 +124,12 @@ func (r *Resolver) FindAllComponents() (components *ComponentSet, err error) { } // ...and add it to the set. - components.Add(r.createComponentFromConfig(updatedComponentConfig)) + comp, createErr := r.createComponentFromConfig(updatedComponentConfig) + if createErr != nil { + return components, createErr + } + + components.Add(comp) } return components, nil @@ -444,14 +449,24 @@ func (r *Resolver) getComponentFromNameAndSpecPath(name, specPath string) (compo } } - return r.createComponentFromConfig(updatedComponentConfig), nil + return r.createComponentFromConfig(updatedComponentConfig) } -func (r *Resolver) createComponentFromConfig(componentConfig *projectconfig.ComponentConfig) Component { +func (r *Resolver) createComponentFromConfig(componentConfig *projectconfig.ComponentConfig) (Component, error) { + var err error + + componentConfig.RenderedSpecDir, err = RenderedSpecDir( + r.env.Config().Project.RenderedSpecsDir, componentConfig.Name, + ) + if err != nil { + return nil, fmt.Errorf("failed to resolve rendered spec dir for component %#q:\n%w", + componentConfig.Name, err) + } + return &resolvedComponent{ env: r.env, config: *componentConfig, - } + }, nil } // Given an explicit component config, apply all inherited defaults. diff --git a/internal/app/azldev/core/sources/mockprocessor.go b/internal/app/azldev/core/sources/mockprocessor.go index 9a9d6bd8..e7bfe468 100644 --- a/internal/app/azldev/core/sources/mockprocessor.go +++ b/internal/app/azldev/core/sources/mockprocessor.go @@ -14,7 +14,6 @@ import ( "strconv" "strings" "sync" - "unicode" "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" "github.com/microsoft/azure-linux-dev-tools/internal/rpm/mock" @@ -81,27 +80,15 @@ func validateInputs(inputs []ComponentInput) error { return nil } -// IsSimpleName returns true if s is a non-empty, single-component filename -// without path separators, traversal sequences, whitespace, or null bytes. -// Use this to validate component names or filenames before using them in -// filesystem paths. -func IsSimpleName(s string) bool { - return s != "" && s != "." && s != ".." && - !strings.ContainsAny(s, "/\\") && - !strings.ContainsFunc(s, unicode.IsSpace) && - !strings.ContainsRune(s, 0) -} - // validateComponentInput rejects component inputs that could cause path traversal // or other safety issues when used to construct paths inside the mock chroot. func validateComponentInput(input ComponentInput) error { - if !IsSimpleName(input.Name) { - return fmt.Errorf( - "invalid component name %#q: must be a simple name without path separators or traversal sequences", input.Name) + if err := fileutils.ValidateFilename(input.Name); err != nil { + return fmt.Errorf("invalid component name %#q:\n%w", input.Name, err) } - if !IsSimpleName(input.SpecFilename) { - return fmt.Errorf("invalid spec filename %#q for component %#q", input.SpecFilename, input.Name) + if err := fileutils.ValidateFilename(input.SpecFilename); err != nil { + return fmt.Errorf("invalid spec filename %#q for component %#q:\n%w", input.SpecFilename, input.Name, err) } return nil diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index d59c679a..e8f77225 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -117,6 +117,11 @@ type ComponentConfig struct { // in serialized files. SourceConfigFile *ConfigFile `toml:"-" json:"-" table:"-"` + // RenderedSpecDir is the output directory for this component's rendered spec files. + // Derived at resolve time from the project's rendered-specs-dir setting; not present + // in serialized files. Empty when rendered-specs-dir is not configured. + RenderedSpecDir string `toml:"-" json:"renderedSpecDir,omitempty" table:"-"` + // Where to get its spec and adjacent files from. Spec SpecSource `toml:"spec,omitempty" json:"spec,omitempty" jsonschema:"title=Spec,description=Identifies where to find the spec for this component"` @@ -167,6 +172,7 @@ func (c *ComponentConfig) WithAbsolutePaths(referenceDir string) *ComponentConfi result := &ComponentConfig{ Name: c.Name, SourceConfigFile: c.SourceConfigFile, + RenderedSpecDir: c.RenderedSpecDir, Spec: deep.MustCopy(c.Spec), Build: deep.MustCopy(c.Build), SourceFiles: deep.MustCopy(c.SourceFiles), diff --git a/internal/utils/fileutils/file.go b/internal/utils/fileutils/file.go index 0522c6c4..53fa6828 100644 --- a/internal/utils/fileutils/file.go +++ b/internal/utils/fileutils/file.go @@ -8,6 +8,8 @@ import ( "fmt" "os" "path/filepath" + "strings" + "unicode" "github.com/bmatcuk/doublestar/v4" "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" @@ -79,5 +81,20 @@ func ValidateFilename(filename string) error { return fmt.Errorf("filename %#q must be a simple filename without directory components", filename) } + if strings.ContainsFunc(filename, unicode.IsSpace) { + return fmt.Errorf("filename %#q must not contain whitespace", filename) + } + + if strings.ContainsRune(filename, 0) { + return fmt.Errorf("filename %#q must not contain null bytes", filename) + } + + // Reject backslashes even on Linux where they are technically valid in + // filenames. Component names travel across platform boundaries (e.g., + // mock chroots, JSON output) where backslashes act as path separators. + if strings.ContainsRune(filename, '\\') { + return fmt.Errorf("filename %#q must not contain backslashes", filename) + } + return nil } diff --git a/internal/utils/fileutils/file_test.go b/internal/utils/fileutils/file_test.go index ee21f79c..56412295 100644 --- a/internal/utils/fileutils/file_test.go +++ b/internal/utils/fileutils/file_test.go @@ -88,6 +88,10 @@ func TestValidateFilename(t *testing.T) { {name: "path traversal", filename: "../escape.tar.gz", expectedError: "without directory components"}, {name: "directory component", filename: "subdir/file.tar.gz", expectedError: "without directory components"}, {name: "dot prefix traversal", filename: "./file.tar.gz", expectedError: "path traversal"}, + {name: "whitespace in name", filename: "has space.tar.gz", expectedError: "must not contain whitespace"}, + {name: "tab in name", filename: "has\ttab.tar.gz", expectedError: "must not contain whitespace"}, + {name: "null byte in name", filename: "has\x00null.tar.gz", expectedError: "must not contain null bytes"}, + {name: "backslash in name", filename: "foo\\bar.tar.gz", expectedError: "must not contain backslashes"}, } for _, tc := range tests {