Skip to content

Commit ab603ff

Browse files
authored
feat: include rendered spec location in component list output (#85)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent f1257d3 commit ab603ff

File tree

9 files changed

+220
-31
lines changed

9 files changed

+220
-31
lines changed

internal/app/azldev/cmds/component/list_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package component_test
55

66
import (
7+
"path/filepath"
78
"testing"
89

910
"github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/cmds/component"
@@ -61,4 +62,66 @@ func TestListComponents_OneComponent(t *testing.T) {
6162
result := results[0]
6263
assert.Equal(t, testComponentName, result.Name)
6364
assert.Equal(t, testSpecPath, result.Spec.Path)
65+
assert.Empty(t, result.RenderedSpecDir, "RenderedSpecDir should be empty when rendered-specs-dir is not configured")
66+
}
67+
68+
func TestListComponents_WithRenderedSpecsDir(t *testing.T) {
69+
const (
70+
testComponentName = "vim"
71+
testSpecPath = "/path/to/spec"
72+
testRenderedDir = "/path/to/repo/specs"
73+
)
74+
75+
testEnv := testutils.NewTestEnv(t)
76+
testEnv.Config.Project.RenderedSpecsDir = testRenderedDir
77+
testEnv.Config.Components[testComponentName] = projectconfig.ComponentConfig{
78+
Name: testComponentName,
79+
Spec: projectconfig.SpecSource{
80+
Path: testSpecPath,
81+
},
82+
}
83+
84+
options := component.ListComponentOptions{
85+
ComponentFilter: components.ComponentFilter{
86+
ComponentNamePatterns: []string{testComponentName},
87+
},
88+
}
89+
90+
results, err := component.ListComponentConfigs(testEnv.Env, &options)
91+
require.NoError(t, err)
92+
require.Len(t, results, 1)
93+
94+
result := results[0]
95+
assert.Equal(t, testComponentName, result.Name)
96+
assert.Equal(t, filepath.Join(testRenderedDir, testComponentName), result.RenderedSpecDir)
97+
}
98+
99+
func TestListComponents_MultipleWithRenderedSpecsDir(t *testing.T) {
100+
const testRenderedDir = "/rendered/specs"
101+
102+
testEnv := testutils.NewTestEnv(t)
103+
testEnv.Config.Project.RenderedSpecsDir = testRenderedDir
104+
105+
testEnv.Config.Components["curl"] = projectconfig.ComponentConfig{
106+
Name: "curl",
107+
Spec: projectconfig.SpecSource{Path: "/specs/curl.spec"},
108+
}
109+
testEnv.Config.Components["vim"] = projectconfig.ComponentConfig{
110+
Name: "vim",
111+
Spec: projectconfig.SpecSource{Path: "/specs/vim.spec"},
112+
}
113+
114+
options := component.ListComponentOptions{
115+
ComponentFilter: components.ComponentFilter{
116+
IncludeAllComponents: true,
117+
},
118+
}
119+
120+
results, err := component.ListComponentConfigs(testEnv.Env, &options)
121+
require.NoError(t, err)
122+
require.Len(t, results, 2)
123+
124+
for _, result := range results {
125+
assert.Equal(t, filepath.Join(testRenderedDir, result.Name), result.RenderedSpecDir)
126+
}
64127
}

internal/app/azldev/cmds/component/render.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,10 @@ func checkRenderErrors(results []*RenderResult, failOnError bool) error {
251251
// preparedComponent holds the intermediate state after source preparation,
252252
// before mock processing.
253253
type preparedComponent struct {
254-
index int
255-
comp components.Component
256-
specFilename string // e.g., "curl.spec"
254+
index int
255+
comp components.Component
256+
specFilename string // e.g., "curl.spec"
257+
compOutputDir string // validated output path computed in phase 1
257258
}
258259

259260
// prepResult pairs a prepared component (on success) or a render result (on error).
@@ -334,17 +335,15 @@ func prepWithSemaphore(
334335
allowOverwrite bool,
335336
) prepResult {
336337
componentName := comp.GetName()
337-
compOutputDir := filepath.Join(outputDir, componentName)
338338

339-
// Validate component name before any filesystem work to prevent path traversal.
340-
if !sources.IsSimpleName(componentName) {
339+
// Validate component name and compute output directory.
340+
compOutputDir, nameErr := components.RenderedSpecDir(outputDir, componentName)
341+
if nameErr != nil {
341342
return prepResult{index: index, result: &RenderResult{
342343
Component: componentName,
343344
OutputDir: "(invalid)",
344345
Status: renderStatusError,
345-
Error: fmt.Sprintf(
346-
"component name %#q is invalid or contains path separators/traversal sequences",
347-
componentName),
346+
Error: nameErr.Error(),
348347
}}
349348
}
350349

@@ -385,6 +384,7 @@ func prepWithSemaphore(
385384
}
386385

387386
prep.index = index
387+
prep.compOutputDir = compOutputDir
388388

389389
return prepResult{index: index, prepared: prep}
390390
}
@@ -572,7 +572,7 @@ func finishOneComponent(
572572
allowOverwrite bool,
573573
) *RenderResult {
574574
componentName := prep.comp.GetName()
575-
compOutputDir := filepath.Join(outputDir, componentName)
575+
compOutputDir := prep.compOutputDir
576576

577577
// Context-aware semaphore acquisition.
578578
select {
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
package components
5+
6+
import (
7+
"fmt"
8+
"path/filepath"
9+
10+
"github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils"
11+
)
12+
13+
// RenderedSpecDir returns the rendered spec output directory for a given component.
14+
// The path is computed as {renderedSpecsDir}/{componentName}.
15+
// Returns an empty string if renderedSpecsDir is not configured (empty).
16+
// Returns an error if componentName is unsafe (absolute, contains path separators
17+
// or traversal sequences).
18+
func RenderedSpecDir(renderedSpecsDir, componentName string) (string, error) {
19+
if err := fileutils.ValidateFilename(componentName); err != nil {
20+
return "", fmt.Errorf("invalid component name for rendered spec dir:\n%w", err)
21+
}
22+
23+
if renderedSpecsDir == "" {
24+
return "", nil
25+
}
26+
27+
return filepath.Join(renderedSpecsDir, componentName), nil
28+
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
package components_test
5+
6+
import (
7+
"testing"
8+
9+
"github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestRenderedSpecDir(t *testing.T) {
15+
t.Run("ReturnsPathWhenConfigured", func(t *testing.T) {
16+
result, err := components.RenderedSpecDir("/path/to/specs", "vim")
17+
require.NoError(t, err)
18+
assert.Equal(t, "/path/to/specs/vim", result)
19+
})
20+
21+
t.Run("ReturnsEmptyWhenNotConfigured", func(t *testing.T) {
22+
result, err := components.RenderedSpecDir("", "vim")
23+
require.NoError(t, err)
24+
assert.Empty(t, result)
25+
})
26+
27+
t.Run("HandlesComponentNameWithDashes", func(t *testing.T) {
28+
result, err := components.RenderedSpecDir("/rendered", "my-component")
29+
require.NoError(t, err)
30+
assert.Equal(t, "/rendered/my-component", result)
31+
})
32+
33+
t.Run("RejectsAbsoluteComponentName", func(t *testing.T) {
34+
_, err := components.RenderedSpecDir("/rendered", "/tmp")
35+
assert.Error(t, err)
36+
})
37+
38+
t.Run("RejectsTraversalInComponentName", func(t *testing.T) {
39+
_, err := components.RenderedSpecDir("/rendered", "../escape")
40+
assert.Error(t, err)
41+
})
42+
43+
t.Run("RejectsPathSeparatorInComponentName", func(t *testing.T) {
44+
_, err := components.RenderedSpecDir("/rendered", "sub/dir")
45+
assert.Error(t, err)
46+
})
47+
48+
t.Run("RejectsEmptyComponentName", func(t *testing.T) {
49+
_, err := components.RenderedSpecDir("/rendered", "")
50+
assert.Error(t, err)
51+
})
52+
53+
t.Run("RejectsDotComponentName", func(t *testing.T) {
54+
_, err := components.RenderedSpecDir("/rendered", ".")
55+
assert.Error(t, err)
56+
})
57+
58+
t.Run("RejectsDotDotComponentName", func(t *testing.T) {
59+
_, err := components.RenderedSpecDir("/rendered", "..")
60+
assert.Error(t, err)
61+
})
62+
63+
t.Run("ValidatesEvenWhenNotConfigured", func(t *testing.T) {
64+
// Component name is validated even when renderedSpecsDir is empty,
65+
// so invalid names are caught early regardless of configuration.
66+
_, err := components.RenderedSpecDir("", "../escape")
67+
assert.Error(t, err)
68+
})
69+
}

internal/app/azldev/core/components/resolver.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,12 @@ func (r *Resolver) FindAllComponents() (components *ComponentSet, err error) {
124124
}
125125

126126
// ...and add it to the set.
127-
components.Add(r.createComponentFromConfig(updatedComponentConfig))
127+
comp, createErr := r.createComponentFromConfig(updatedComponentConfig)
128+
if createErr != nil {
129+
return components, createErr
130+
}
131+
132+
components.Add(comp)
128133
}
129134

130135
return components, nil
@@ -444,14 +449,24 @@ func (r *Resolver) getComponentFromNameAndSpecPath(name, specPath string) (compo
444449
}
445450
}
446451

447-
return r.createComponentFromConfig(updatedComponentConfig), nil
452+
return r.createComponentFromConfig(updatedComponentConfig)
448453
}
449454

450-
func (r *Resolver) createComponentFromConfig(componentConfig *projectconfig.ComponentConfig) Component {
455+
func (r *Resolver) createComponentFromConfig(componentConfig *projectconfig.ComponentConfig) (Component, error) {
456+
var err error
457+
458+
componentConfig.RenderedSpecDir, err = RenderedSpecDir(
459+
r.env.Config().Project.RenderedSpecsDir, componentConfig.Name,
460+
)
461+
if err != nil {
462+
return nil, fmt.Errorf("failed to resolve rendered spec dir for component %#q:\n%w",
463+
componentConfig.Name, err)
464+
}
465+
451466
return &resolvedComponent{
452467
env: r.env,
453468
config: *componentConfig,
454-
}
469+
}, nil
455470
}
456471

457472
// Given an explicit component config, apply all inherited defaults.

internal/app/azldev/core/sources/mockprocessor.go

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"strconv"
1515
"strings"
1616
"sync"
17-
"unicode"
1817

1918
"github.com/microsoft/azure-linux-dev-tools/internal/global/opctx"
2019
"github.com/microsoft/azure-linux-dev-tools/internal/rpm/mock"
@@ -81,27 +80,15 @@ func validateInputs(inputs []ComponentInput) error {
8180
return nil
8281
}
8382

84-
// IsSimpleName returns true if s is a non-empty, single-component filename
85-
// without path separators, traversal sequences, whitespace, or null bytes.
86-
// Use this to validate component names or filenames before using them in
87-
// filesystem paths.
88-
func IsSimpleName(s string) bool {
89-
return s != "" && s != "." && s != ".." &&
90-
!strings.ContainsAny(s, "/\\") &&
91-
!strings.ContainsFunc(s, unicode.IsSpace) &&
92-
!strings.ContainsRune(s, 0)
93-
}
94-
9583
// validateComponentInput rejects component inputs that could cause path traversal
9684
// or other safety issues when used to construct paths inside the mock chroot.
9785
func validateComponentInput(input ComponentInput) error {
98-
if !IsSimpleName(input.Name) {
99-
return fmt.Errorf(
100-
"invalid component name %#q: must be a simple name without path separators or traversal sequences", input.Name)
86+
if err := fileutils.ValidateFilename(input.Name); err != nil {
87+
return fmt.Errorf("invalid component name %#q:\n%w", input.Name, err)
10188
}
10289

103-
if !IsSimpleName(input.SpecFilename) {
104-
return fmt.Errorf("invalid spec filename %#q for component %#q", input.SpecFilename, input.Name)
90+
if err := fileutils.ValidateFilename(input.SpecFilename); err != nil {
91+
return fmt.Errorf("invalid spec filename %#q for component %#q:\n%w", input.SpecFilename, input.Name, err)
10592
}
10693

10794
return nil

internal/projectconfig/component.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ type ComponentConfig struct {
117117
// in serialized files.
118118
SourceConfigFile *ConfigFile `toml:"-" json:"-" table:"-"`
119119

120+
// RenderedSpecDir is the output directory for this component's rendered spec files.
121+
// Derived at resolve time from the project's rendered-specs-dir setting; not present
122+
// in serialized files. Empty when rendered-specs-dir is not configured.
123+
RenderedSpecDir string `toml:"-" json:"renderedSpecDir,omitempty" table:"-"`
124+
120125
// Where to get its spec and adjacent files from.
121126
Spec SpecSource `toml:"spec,omitempty" json:"spec,omitempty" jsonschema:"title=Spec,description=Identifies where to find the spec for this component"`
122127

@@ -167,6 +172,7 @@ func (c *ComponentConfig) WithAbsolutePaths(referenceDir string) *ComponentConfi
167172
result := &ComponentConfig{
168173
Name: c.Name,
169174
SourceConfigFile: c.SourceConfigFile,
175+
RenderedSpecDir: c.RenderedSpecDir,
170176
Spec: deep.MustCopy(c.Spec),
171177
Build: deep.MustCopy(c.Build),
172178
SourceFiles: deep.MustCopy(c.SourceFiles),

internal/utils/fileutils/file.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"fmt"
99
"os"
1010
"path/filepath"
11+
"strings"
12+
"unicode"
1113

1214
"github.com/bmatcuk/doublestar/v4"
1315
"github.com/microsoft/azure-linux-dev-tools/internal/global/opctx"
@@ -79,5 +81,20 @@ func ValidateFilename(filename string) error {
7981
return fmt.Errorf("filename %#q must be a simple filename without directory components", filename)
8082
}
8183

84+
if strings.ContainsFunc(filename, unicode.IsSpace) {
85+
return fmt.Errorf("filename %#q must not contain whitespace", filename)
86+
}
87+
88+
if strings.ContainsRune(filename, 0) {
89+
return fmt.Errorf("filename %#q must not contain null bytes", filename)
90+
}
91+
92+
// Reject backslashes even on Linux where they are technically valid in
93+
// filenames. Component names travel across platform boundaries (e.g.,
94+
// mock chroots, JSON output) where backslashes act as path separators.
95+
if strings.ContainsRune(filename, '\\') {
96+
return fmt.Errorf("filename %#q must not contain backslashes", filename)
97+
}
98+
8299
return nil
83100
}

internal/utils/fileutils/file_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ func TestValidateFilename(t *testing.T) {
8888
{name: "path traversal", filename: "../escape.tar.gz", expectedError: "without directory components"},
8989
{name: "directory component", filename: "subdir/file.tar.gz", expectedError: "without directory components"},
9090
{name: "dot prefix traversal", filename: "./file.tar.gz", expectedError: "path traversal"},
91+
{name: "whitespace in name", filename: "has space.tar.gz", expectedError: "must not contain whitespace"},
92+
{name: "tab in name", filename: "has\ttab.tar.gz", expectedError: "must not contain whitespace"},
93+
{name: "null byte in name", filename: "has\x00null.tar.gz", expectedError: "must not contain null bytes"},
94+
{name: "backslash in name", filename: "foo\\bar.tar.gz", expectedError: "must not contain backslashes"},
9195
}
9296

9397
for _, tc := range tests {

0 commit comments

Comments
 (0)