-
Notifications
You must be signed in to change notification settings - Fork 57
refactor: update partials resolution to support relative paths and improve error handling #458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ import ( | |||||||||||||||||
| "log" | ||||||||||||||||||
| "net/http" | ||||||||||||||||||
| "os" | ||||||||||||||||||
| "path" | ||||||||||||||||||
| "path/filepath" | ||||||||||||||||||
| "strings" | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -26,16 +27,81 @@ type Engine struct { | |||||||||||||||||
| type fileSystemPartialProvider struct { | ||||||||||||||||||
| fileSystem http.FileSystem | ||||||||||||||||||
| extension string | ||||||||||||||||||
| baseDir string | ||||||||||||||||||
| verbose bool | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func (p fileSystemPartialProvider) Get(path string) (string, error) { | ||||||||||||||||||
| buf, err := core.ReadFile(path+p.extension, p.fileSystem) | ||||||||||||||||||
| return string(buf), err | ||||||||||||||||||
| func (p fileSystemPartialProvider) Get(partial string) (string, error) { | ||||||||||||||||||
| candidates := p.lookupCandidates(partial) | ||||||||||||||||||
| var firstErr error | ||||||||||||||||||
| for _, candidate := range candidates { | ||||||||||||||||||
| buf, err := core.ReadFile(candidate, p.fileSystem) | ||||||||||||||||||
| if err == nil { | ||||||||||||||||||
| return string(buf), nil | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if firstErr == nil { | ||||||||||||||||||
| firstErr = err | ||||||||||||||||||
| } | ||||||||||||||||||
| if p.verbose { | ||||||||||||||||||
| log.Printf("views: partial lookup failed: partial=%q candidate=%q err=%v", partial, candidate, err) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if p.verbose { | ||||||||||||||||||
| log.Printf("views: partial not found: partial=%q candidates=%v", partial, candidates) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if firstErr == nil { | ||||||||||||||||||
| firstErr = fmt.Errorf("no partial candidates generated") | ||||||||||||||||||
| } | ||||||||||||||||||
| return "", fmt.Errorf("render: partial %q does not exist (tried: %s): %w", partial, strings.Join(candidates, ", "), firstErr) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+55
to
+59
|
||||||||||||||||||
|
|
||||||||||||||||||
| func (p fileSystemPartialProvider) lookupCandidates(partial string) []string { | ||||||||||||||||||
| addCandidate := func(candidates []string, candidate string) []string { | ||||||||||||||||||
| for _, existing := range candidates { | ||||||||||||||||||
| if existing == candidate { | ||||||||||||||||||
| return candidates | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| return append(candidates, candidate) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| addExtension := func(raw string) string { | ||||||||||||||||||
| if strings.HasSuffix(raw, p.extension) { | ||||||||||||||||||
| return raw | ||||||||||||||||||
| } | ||||||||||||||||||
| return raw + p.extension | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| base := filepath.ToSlash(strings.TrimSpace(p.baseDir)) | ||||||||||||||||||
| base = strings.TrimSuffix(base, "/") | ||||||||||||||||||
| if base == "." || base == "/" { | ||||||||||||||||||
| base = "" | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| clean := filepath.ToSlash(strings.TrimSpace(partial)) | ||||||||||||||||||
| clean = strings.TrimPrefix(clean, "./") | ||||||||||||||||||
|
|
||||||||||||||||||
| candidates := make([]string, 0, 2) | ||||||||||||||||||
| if clean != "" { | ||||||||||||||||||
| candidates = addCandidate(candidates, addExtension(clean)) | ||||||||||||||||||
| if base != "" { | ||||||||||||||||||
| candidates = addCandidate(candidates, addExtension(path.Join(base, clean))) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+89
to
+92
|
||||||||||||||||||
| candidates = addCandidate(candidates, addExtension(clean)) | |
| if base != "" { | |
| candidates = addCandidate(candidates, addExtension(path.Join(base, clean))) | |
| } | |
| if base != "" { | |
| candidates = addCandidate(candidates, addExtension(path.Join(base, clean))) | |
| } | |
| candidates = addCandidate(candidates, addExtension(clean)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lookupCandidates method does not sanitize the partial input, leading to a Path Traversal vulnerability. When the engine is initialized using mustache.New(), it uses the local filesystem directly (p.fileSystem is nil). A malicious template could use ../ sequences in a partial tag (e.g., {{> ../../../etc/passwd }}) to read arbitrary files from the server, escaping the intended template directory. More robust sanitization is required to prevent access outside the intended template directory. This involves cleaning the path and then checking for directory traversal attempts or absolute paths to ensure the resolved path remains within the baseDir.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| {{> views/partials/header }} | ||
| {{> partials/header }} | ||
| <h1>{{Title}}</h1> | ||
| {{> views/partials/footer }} | ||
| {{> partials/footer }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| {{> partials/header }} | ||
| <h1>{{Title}}</h1> | ||
| {{> partials/footer }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The README example shows
http.Dir(...)in the (commented) embedded/filesystem snippet but the import block in the example doesn’t includenet/http. Add the import so users can copy/paste and uncomment the snippet without compile errors.