Skip to content

Commit 440f89c

Browse files
committed
feedback: validate name, better errors
1 parent a608129 commit 440f89c

File tree

2 files changed

+33
-10
lines changed

2 files changed

+33
-10
lines changed

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ sidecar files are included. Multiple components can be rendered at once.`,
8585
"exit with error if any component fails to render (useful for CI)")
8686

8787
cmd.Flags().BoolVarP(&options.Force, "force", "f", false,
88-
"delete and recreate existing rendered component directories")
88+
"allow deletion of existing rendered output directories; required for -a and when output dirs already exist")
8989

9090
return cmd
9191
}
@@ -122,6 +122,13 @@ func RenderComponents(env *azldev.Env, options *RenderOptions) ([]*RenderResult,
122122
return nil, err
123123
}
124124

125+
// Rendering all components requires --force for stale directory cleanup.
126+
// Check early to avoid wasting work on all three phases.
127+
if options.ComponentFilter.IncludeAllComponents && !options.Force {
128+
return nil, errors.New(
129+
"rendering all components (-a) requires --force to enable cleanup of stale output directories")
130+
}
131+
125132
resolver := components.NewResolver(env)
126133

127134
comps, err := resolver.FindComponents(&options.ComponentFilter)
@@ -172,13 +179,7 @@ func RenderComponents(env *azldev.Env, options *RenderOptions) ([]*RenderResult,
172179
options.Force)
173180

174181
// Clean up stale rendered directories when rendering all components.
175-
// Requires --force (auto-set for configured dirs) to prevent accidental deletion.
176182
if options.ComponentFilter.IncludeAllComponents {
177-
if !options.Force {
178-
return nil, errors.New(
179-
"rendering all components (-a) requires --force to enable cleanup of stale output directories")
180-
}
181-
182183
if cleanupErr := cleanupStaleRenders(env.FS(), comps, options.OutputDir); cleanupErr != nil {
183184
return results, fmt.Errorf("cleaning up stale rendered output:\n%w", cleanupErr)
184185
}
@@ -321,6 +322,23 @@ func prepWithSemaphore(
321322
componentName := comp.GetName()
322323
compOutputDir := filepath.Join(outputDir, componentName)
323324

325+
// 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) {
332+
return prepResult{index: index, result: &RenderResult{
333+
Component: componentName,
334+
OutputDir: "(invalid)",
335+
Status: renderStatusError,
336+
Error: fmt.Sprintf(
337+
"component name %#q is invalid or contains path separators/traversal sequences",
338+
componentName),
339+
}}
340+
}
341+
324342
// Context-aware semaphore acquisition.
325343
select {
326344
case semaphore <- struct{}{}:
@@ -684,7 +702,7 @@ func copyRenderedOutput(env *azldev.Env, tempDir, baseOutputDir, componentName s
684702
func removeUnreferencedFiles(fs opctx.FS, tempDir, specPath string, specFiles []string, componentName string) error {
685703
keepSet := make(map[string]bool, len(specFiles))
686704
keepSet[filepath.Base(specPath)] = true
687-
keepSet["sources"] = true
705+
keepSet["sources"] = true // lookaside hashes/signatures; always preserved
688706

689707
for _, f := range specFiles {
690708
// Extract the first path component so subdirectory entries are preserved.
@@ -801,6 +819,10 @@ const renderErrorMarkerFile = "RENDER_FAILED"
801819
// writeRenderErrorMarker writes a static marker file to the component's output directory
802820
// indicating that rendering failed. The content is intentionally static (no error details)
803821
// so the file is deterministic across runs and safe to check in.
822+
//
823+
// This is always written on failure, even without --force. The --force flag controls
824+
// deletion of existing directories (RemoveAll), not creation of new files. Writing a
825+
// small marker into an existing directory is safe and provides visible git diff feedback.
804826
func writeRenderErrorMarker(fs opctx.FS, componentOutputDir string) {
805827
if mkdirErr := fileutils.MkdirAll(fs, componentOutputDir); mkdirErr != nil {
806828
slog.Debug("Failed to create directory for error marker", "path", componentOutputDir, "error", mkdirErr)

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,13 @@ func TestRenderCmd_NoComponents(t *testing.T) {
4646
testEnv := testutils.NewTestEnv(t)
4747

4848
cmd := componentcmds.NewRenderCmd()
49-
cmd.SetArgs([]string{"nonexistent-component"})
49+
cmd.SetArgs([]string{"-o", "SPECS", "nonexistent-component"})
5050

5151
err := cmd.ExecuteContext(testEnv.Env)
5252

53-
// We expect an error because no components match.
53+
// We expect an error because no components match (not the output-dir error).
5454
require.Error(t, err)
55+
assert.Contains(t, err.Error(), "component not found")
5556
}
5657

5758
func TestRenderCmd_NoOutputDir(t *testing.T) {

0 commit comments

Comments
 (0)