Skip to content

Commit 70a41a3

Browse files
committed
feedback: common name validate func, count cancel, better error
1 parent 440f89c commit 70a41a3

File tree

3 files changed

+39
-21
lines changed

3 files changed

+39
-21
lines changed

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

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ func RenderComponents(env *azldev.Env, options *RenderOptions) ([]*RenderResult,
126126
// Check early to avoid wasting work on all three phases.
127127
if options.ComponentFilter.IncludeAllComponents && !options.Force {
128128
return nil, errors.New(
129-
"rendering all components (-a) requires --force to enable cleanup of stale output directories")
129+
"rendering all components (-a) requires --force to enable cleanup of stale output directories" +
130+
" (auto-set when rendered-specs-dir is configured)")
130131
}
131132

132133
resolver := components.NewResolver(env)
@@ -208,21 +209,31 @@ func sortRenderResults(results []*RenderResult) {
208209
})
209210
}
210211

211-
// checkRenderErrors counts error results and returns an error if FailOnError is set.
212+
// checkRenderErrors counts error and cancelled results and returns an error if FailOnError is set.
212213
func checkRenderErrors(results []*RenderResult, failOnError bool) error {
213-
errCount := 0
214+
var errCount, cancelledCount int
214215

215216
for _, result := range results {
216-
if result != nil && result.Status == renderStatusError {
217+
if result == nil {
218+
continue
219+
}
220+
221+
switch result.Status {
222+
case renderStatusError:
217223
errCount++
224+
case renderStatusCancelled:
225+
cancelledCount++
218226
}
219227
}
220228

221-
if errCount > 0 {
222-
slog.Error("Some components failed to render", "errorCount", errCount)
229+
failCount := errCount + cancelledCount
230+
231+
if failCount > 0 {
232+
slog.Error("Some components failed to render",
233+
"errorCount", errCount, "cancelledCount", cancelledCount)
223234

224235
if failOnError {
225-
return fmt.Errorf("%d component(s) failed to render", errCount)
236+
return fmt.Errorf("%d component(s) failed to render", failCount)
226237
}
227238
}
228239

@@ -323,12 +334,7 @@ func prepWithSemaphore(
323334
compOutputDir := filepath.Join(outputDir, componentName)
324335

325336
// Validate component name before any filesystem work to prevent path traversal.
326-
// This mirrors validateComponentInput in mockprocessor.go but runs earlier
327-
// (before staging dir creation and RemoveAll calls).
328-
if componentName == "" || componentName == "." ||
329-
strings.ContainsAny(componentName, "/\\") ||
330-
strings.Contains(componentName, "..") ||
331-
strings.ContainsRune(componentName, 0) {
337+
if !sources.IsSimpleName(componentName) {
332338
return prepResult{index: index, result: &RenderResult{
333339
Component: componentName,
334340
OutputDir: "(invalid)",
@@ -620,7 +626,7 @@ func finishComponentRender(
620626
mockResult, hasMockResult := mockResultMap[componentName]
621627
if !hasMockResult {
622628
return fmt.Errorf(
623-
"no mock result for %#q (batch processing may have failed)", componentName)
629+
"no mock result for %#q (batch mock processing failed; see earlier errors)", componentName)
624630
}
625631

626632
if mockResult.Error != nil {
@@ -759,7 +765,15 @@ func findSpecFile(fs opctx.FS, dir, componentName string) (string, error) {
759765

760766
for _, entry := range entries {
761767
if !entry.IsDir() && filepath.Ext(entry.Name()) == ".spec" {
762-
return filepath.Join(dir, entry.Name()), nil
768+
foundPath := filepath.Join(dir, entry.Name())
769+
770+
slog.Warn("Spec filename does not match component name; using fallback",
771+
"component", componentName,
772+
"expected", componentName+".spec",
773+
"found", entry.Name(),
774+
)
775+
776+
return foundPath, nil
763777
}
764778
}
765779

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,24 +80,26 @@ func validateInputs(inputs []ComponentInput) error {
8080
return nil
8181
}
8282

83-
// isSimpleName returns true if s is a non-empty, single-component filename
84-
// without path separators, traversal sequences, or null bytes.
85-
func isSimpleName(s string) bool {
83+
// IsSimpleName returns true if s is a non-empty, single-component filename
84+
// without path separators, traversal sequences, whitespace, or null bytes.
85+
// Use this to validate component names or filenames before using them in
86+
// filesystem paths.
87+
func IsSimpleName(s string) bool {
8688
return s != "" && s != "." && s != ".." &&
87-
!strings.ContainsAny(s, "/\\") &&
89+
!strings.ContainsAny(s, "/\\ \t\n\r") &&
8890
!strings.Contains(s, "..") &&
8991
!strings.ContainsRune(s, 0)
9092
}
9193

9294
// validateComponentInput rejects component inputs that could cause path traversal
9395
// or other safety issues when used to construct paths inside the mock chroot.
9496
func validateComponentInput(input ComponentInput) error {
95-
if !isSimpleName(input.Name) {
97+
if !IsSimpleName(input.Name) {
9698
return fmt.Errorf(
9799
"invalid component name %#q: must be a simple name without path separators or traversal sequences", input.Name)
98100
}
99101

100-
if !isSimpleName(input.SpecFilename) {
102+
if !IsSimpleName(input.SpecFilename) {
101103
return fmt.Errorf("invalid spec filename %#q for component %#q", input.SpecFilename, input.Name)
102104
}
103105

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ func TestValidateInputs(t *testing.T) {
119119
{"dotdot in name", []ComponentInput{{Name: "..", SpecFilename: "a.spec"}}, true, "invalid component name"},
120120
{"absolute name", []ComponentInput{{Name: "/tmp/evil", SpecFilename: "a.spec"}}, true, "invalid component name"},
121121
{"null in name", []ComponentInput{{Name: "has\x00null", SpecFilename: "a.spec"}}, true, "invalid component name"},
122+
{"space in name", []ComponentInput{{Name: "has space", SpecFilename: "a.spec"}}, true, "invalid component name"},
123+
{"tab in name", []ComponentInput{{Name: "has\ttab", SpecFilename: "a.spec"}}, true, "invalid component name"},
122124
{"empty spec", []ComponentInput{{Name: "curl", SpecFilename: ""}}, true, "invalid spec filename"},
123125
{"dot spec", []ComponentInput{{Name: "curl", SpecFilename: "."}}, true, "invalid spec filename"},
124126
{"dotdot spec", []ComponentInput{{Name: "curl", SpecFilename: ".."}}, true, "invalid spec filename"},

0 commit comments

Comments
 (0)