Skip to content

Commit f43934e

Browse files
josh-padnickjosh-padnick
committed
Add --output-path flag to CLI
Users can now specify a custom output folder by adding the --output-path folder when calling runbooks. This will generate files in the given folder. Also adds security protections for outputPaths specified by Runbook authors to no longer permit absolute paths or directory traversal (i.e. "../").
1 parent 48c6c54 commit f43934e

File tree

13 files changed

+348
-56
lines changed

13 files changed

+348
-56
lines changed

‎api/boilerplate_render.go‎

Lines changed: 97 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"os"
88
"path/filepath"
9+
"strings"
910

1011
"github.com/gin-gonic/gin"
1112
bpConfig "github.com/gruntwork-io/boilerplate/config"
@@ -14,10 +15,96 @@ import (
1415
bpVariables "github.com/gruntwork-io/boilerplate/variables"
1516
)
1617

18+
// validateOutputPath validates an output path from an API request to prevent security issues.
19+
// It ensures the path is:
20+
// - Not absolute (to prevent writing to arbitrary filesystem locations)
21+
// - Does not contain ".." (to prevent directory traversal attacks)
22+
// Returns an error if validation fails.
23+
func validateOutputPath(path string) error {
24+
// Empty path is allowed (will use default)
25+
if path == "" {
26+
return nil
27+
}
28+
29+
// Check for absolute paths (Unix-style and Windows-style)
30+
if filepath.IsAbs(path) {
31+
return fmt.Errorf("absolute paths are not allowed in API requests: %s", path)
32+
}
33+
34+
// Additional check for Windows paths on Unix systems
35+
if len(path) >= 2 && path[1] == ':' {
36+
return fmt.Errorf("absolute paths are not allowed in API requests: %s", path)
37+
}
38+
39+
// Check for directory traversal attempts
40+
if containsDirectoryTraversal(path) {
41+
return fmt.Errorf("directory traversal is not allowed: %s", path)
42+
}
43+
44+
return nil
45+
}
46+
47+
// containsDirectoryTraversal checks if a path contains ".." components
48+
func containsDirectoryTraversal(path string) bool {
49+
// Normalize path separators
50+
normalizedPath := filepath.ToSlash(path)
51+
52+
// Check for ".." as a path component
53+
// This handles: "..", "../foo", "foo/../bar", "foo/.."
54+
parts := strings.Split(normalizedPath, "/")
55+
for _, part := range parts {
56+
if part == ".." {
57+
return true
58+
}
59+
}
60+
61+
return false
62+
}
63+
64+
// determineOutputDirectory determines the final output directory based on CLI config and API request.
65+
// CLI output path is the path set via the --output-path CLI flag and is trusted (specified by end user)
66+
// API request output path is the path specified in a component prop in the Runbook and is untrusted (specified by runbook author).
67+
// If apiRequestOutputPath is provided, it's validated and treated as a subdirectory within the CLI path.
68+
// Returns the absolute output directory path or an error.
69+
func determineOutputDirectory(cliOutputPath string, apiRequestOutputPath *string) (string, error) {
70+
var outputDir string
71+
72+
if apiRequestOutputPath != nil && *apiRequestOutputPath != "" {
73+
// Validate the API-provided output path for security
74+
if err := validateOutputPath(*apiRequestOutputPath); err != nil {
75+
return "", fmt.Errorf("invalid output path: %w", err)
76+
}
77+
78+
// Treat the validated path as a subdirectory within the CLI output path
79+
if filepath.IsAbs(cliOutputPath) {
80+
outputDir = filepath.Join(cliOutputPath, *apiRequestOutputPath)
81+
} else {
82+
currentDir, err := os.Getwd()
83+
if err != nil {
84+
return "", fmt.Errorf("failed to get current working directory: %w", err)
85+
}
86+
outputDir = filepath.Join(currentDir, cliOutputPath, *apiRequestOutputPath)
87+
}
88+
} else {
89+
// Use the CLI output path
90+
if filepath.IsAbs(cliOutputPath) {
91+
outputDir = cliOutputPath
92+
} else {
93+
currentDir, err := os.Getwd()
94+
if err != nil {
95+
return "", fmt.Errorf("failed to get current working directory: %w", err)
96+
}
97+
outputDir = filepath.Join(currentDir, cliOutputPath)
98+
}
99+
}
100+
101+
return outputDir, nil
102+
}
103+
17104
// This handler renders a boilerplate template with the provided variables.
18105

19106
// HandleBoilerplateRender renders a boilerplate template with the provided variables
20-
func HandleBoilerplateRender(runbookPath string) gin.HandlerFunc {
107+
func HandleBoilerplateRender(runbookPath string, cliOutputPath string) gin.HandlerFunc {
21108
return func(c *gin.Context) {
22109
var req RenderRequest
23110
if err := c.ShouldBindJSON(&req); err != nil {
@@ -54,22 +141,14 @@ func HandleBoilerplateRender(runbookPath string) gin.HandlerFunc {
54141
}
55142

56143
// Determine the output directory
57-
var outputDir string
58-
if req.OutputPath != nil && *req.OutputPath != "" {
59-
// Use the provided output path (can be relative or absolute)
60-
outputDir = *req.OutputPath
61-
} else {
62-
// Default to "generated" subfolder in current working directory
63-
currentDir, err := os.Getwd()
64-
if err != nil {
65-
slog.Error("Failed to get current working directory", "error", err)
66-
c.JSON(http.StatusInternalServerError, gin.H{
67-
"error": "Failed to get current working directory",
68-
"details": err.Error(),
69-
})
70-
return
71-
}
72-
outputDir = filepath.Join(currentDir, "generated")
144+
outputDir, err := determineOutputDirectory(cliOutputPath, req.OutputPath)
145+
if err != nil {
146+
slog.Error("Failed to determine output directory", "error", err)
147+
c.JSON(http.StatusBadRequest, gin.H{
148+
"error": "Invalid output path",
149+
"details": err.Error(),
150+
})
151+
return
73152
}
74153

75154
// Create the output directory if it doesn't exist
@@ -85,7 +164,7 @@ func HandleBoilerplateRender(runbookPath string) gin.HandlerFunc {
85164
slog.Info("Rendering template to output directory", "outputDir", outputDir)
86165

87166
// Render the template using the boilerplate package
88-
err := renderBoilerplateTemplate(fullTemplatePath, outputDir, req.Variables)
167+
err = renderBoilerplateTemplate(fullTemplatePath, outputDir, req.Variables)
89168
if err != nil {
90169
slog.Error("Failed to render boilerplate template", "error", err)
91170
c.JSON(http.StatusInternalServerError, gin.H{

‎api/boilerplate_render_security_test.go‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
package api
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestValidateOutputPath(t *testing.T) {
8+
tests := []struct {
9+
name string
10+
path string
11+
expectError bool
12+
description string
13+
}{
14+
{
15+
name: "valid relative path",
16+
path: "prod",
17+
expectError: false,
18+
description: "Simple subdirectory name should be allowed",
19+
},
20+
{
21+
name: "valid nested relative path",
22+
path: "environments/prod",
23+
expectError: false,
24+
description: "Nested subdirectories should be allowed",
25+
},
26+
{
27+
name: "reject absolute path - unix",
28+
path: "/etc/passwd",
29+
expectError: true,
30+
description: "Absolute Unix paths should be rejected",
31+
},
32+
{
33+
name: "reject absolute path - windows",
34+
path: "C:\\Windows\\System32",
35+
expectError: true,
36+
description: "Absolute Windows paths should be rejected",
37+
},
38+
{
39+
name: "reject directory traversal - simple",
40+
path: "../secrets",
41+
expectError: true,
42+
description: "Simple directory traversal should be rejected",
43+
},
44+
{
45+
name: "reject directory traversal - nested",
46+
path: "../../etc/passwd",
47+
expectError: true,
48+
description: "Nested directory traversal should be rejected",
49+
},
50+
{
51+
name: "reject directory traversal - middle",
52+
path: "foo/../bar",
53+
expectError: true,
54+
description: "Directory traversal in the middle should be rejected",
55+
},
56+
{
57+
name: "reject directory traversal - end",
58+
path: "foo/bar/..",
59+
expectError: true,
60+
description: "Directory traversal at the end should be rejected",
61+
},
62+
{
63+
name: "valid path with dots in name",
64+
path: "my.folder/sub.dir",
65+
expectError: false,
66+
description: "Dots in folder names (not ..) should be allowed",
67+
},
68+
{
69+
name: "empty path",
70+
path: "",
71+
expectError: false,
72+
description: "Empty path should be allowed (will use default)",
73+
},
74+
}
75+
76+
for _, tt := range tests {
77+
t.Run(tt.name, func(t *testing.T) {
78+
err := validateOutputPath(tt.path)
79+
80+
if tt.expectError && err == nil {
81+
t.Errorf("Expected error for path %q but got none. %s", tt.path, tt.description)
82+
}
83+
84+
if !tt.expectError && err != nil {
85+
t.Errorf("Expected no error for path %q but got: %v. %s", tt.path, err, tt.description)
86+
}
87+
})
88+
}
89+
}
90+
91+
func TestContainsDirectoryTraversal(t *testing.T) {
92+
tests := []struct {
93+
name string
94+
path string
95+
expected bool
96+
}{
97+
{
98+
name: "no traversal - simple path",
99+
path: "foo/bar",
100+
expected: false,
101+
},
102+
{
103+
name: "no traversal - single dot",
104+
path: "./foo",
105+
expected: false,
106+
},
107+
{
108+
name: "traversal - double dot at start",
109+
path: "../foo",
110+
expected: true,
111+
},
112+
{
113+
name: "traversal - double dot in middle",
114+
path: "foo/../bar",
115+
expected: true,
116+
},
117+
{
118+
name: "traversal - double dot at end",
119+
path: "foo/..",
120+
expected: true,
121+
},
122+
{
123+
name: "traversal - just double dot",
124+
path: "..",
125+
expected: true,
126+
},
127+
{
128+
name: "no traversal - dots in filename",
129+
path: "my.file.txt",
130+
expected: false,
131+
},
132+
{
133+
name: "no traversal - dotdot in filename",
134+
path: "foo/..bar",
135+
expected: false,
136+
},
137+
}
138+
139+
for _, tt := range tests {
140+
t.Run(tt.name, func(t *testing.T) {
141+
result := containsDirectoryTraversal(tt.path)
142+
if result != tt.expected {
143+
t.Errorf("containsDirectoryTraversal(%q) = %v, want %v", tt.path, result, tt.expected)
144+
}
145+
})
146+
}
147+
}
148+
149+
func TestDetermineOutputDirectory(t *testing.T) {
150+
tests := []struct {
151+
name string
152+
cliOutputPath string
153+
apiRequestOutputPath *string
154+
expectError bool
155+
expectedPath string
156+
}{
157+
{
158+
name: "absolute CLI path only",
159+
cliOutputPath: "/tmp/output",
160+
apiRequestOutputPath: nil,
161+
expectError: false,
162+
expectedPath: "/tmp/output",
163+
},
164+
{
165+
name: "absolute CLI path with valid API subdirectory",
166+
cliOutputPath: "/tmp/output",
167+
apiRequestOutputPath: strPtr("prod"),
168+
expectError: false,
169+
expectedPath: "/tmp/output/prod",
170+
},
171+
{
172+
name: "reject API path with directory traversal",
173+
cliOutputPath: "/tmp/output",
174+
apiRequestOutputPath: strPtr("../secrets"),
175+
expectError: true,
176+
},
177+
}
178+
179+
for _, tt := range tests {
180+
t.Run(tt.name, func(t *testing.T) {
181+
result, err := determineOutputDirectory(tt.cliOutputPath, tt.apiRequestOutputPath)
182+
183+
if tt.expectError {
184+
if err == nil {
185+
t.Errorf("Expected error but got none")
186+
}
187+
} else {
188+
if err != nil {
189+
t.Errorf("Unexpected error: %v", err)
190+
}
191+
if tt.expectedPath != "" && result != tt.expectedPath {
192+
t.Errorf("Expected result to be %q, got %q", tt.expectedPath, result)
193+
}
194+
}
195+
})
196+
}
197+
}
198+
199+
// Helper function to create string pointer
200+
func strPtr(s string) *string {
201+
return &s
202+
}
203+

0 commit comments

Comments
 (0)