Skip to content

Commit cccfacf

Browse files
committed
Add path-containment validation for resolved skill directories
Apply filepath.Clean at path resolution time and validate against traversal segments before any filesystem operation.
1 parent 7435745 commit cccfacf

1 file changed

Lines changed: 6 additions & 6 deletions

File tree

pkg/skills/skillsvc/skillsvc.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,7 @@ func (s *service) resolveAndValidateClients(
12471247
}
12481248
return nil, nil, fmt.Errorf("resolving skill path for client %q: %w", ct, err)
12491249
}
1250+
dir = filepath.Clean(dir)
12501251
if err := validateResolvedDir(dir); err != nil {
12511252
return nil, nil, fmt.Errorf("resolved path for client %q is unsafe: %w", ct, err)
12521253
}
@@ -1276,6 +1277,7 @@ func (s *service) expandToExistingClients(
12761277
if err != nil {
12771278
return nil, nil, fmt.Errorf("resolving skill path for existing client %q: %w", ct, err)
12781279
}
1280+
dir = filepath.Clean(dir)
12791281
if err := validateResolvedDir(dir); err != nil {
12801282
return nil, nil, fmt.Errorf("resolved path for client %q is unsafe: %w", ct, err)
12811283
}
@@ -1284,15 +1286,13 @@ func (s *service) expandToExistingClients(
12841286
return allClients, allDirs, nil
12851287
}
12861288

1287-
// validateResolvedDir ensures a directory path returned by the PathResolver is
1288-
// clean, absolute, and free of path-traversal segments. This provides
1289-
// defense-in-depth against tainted paths reaching filesystem operations.
1289+
// validateResolvedDir ensures a directory path is absolute and free of
1290+
// path-traversal segments. Callers must pass a filepath.Clean'd value.
12901291
func validateResolvedDir(dir string) error {
1291-
cleaned := filepath.Clean(dir)
1292-
if !filepath.IsAbs(cleaned) {
1292+
if !filepath.IsAbs(dir) {
12931293
return fmt.Errorf("path must be absolute, got %q", dir)
12941294
}
1295-
for _, seg := range strings.Split(filepath.ToSlash(cleaned), "/") {
1295+
for _, seg := range strings.Split(filepath.ToSlash(dir), "/") {
12961296
if seg == ".." {
12971297
return fmt.Errorf("path contains traversal segment: %q", dir)
12981298
}

0 commit comments

Comments
 (0)