From 650dbdf3392f3396d576d8f669d15d58c7b90322 Mon Sep 17 00:00:00 2001 From: samzong Date: Fri, 26 Jun 2026 08:37:11 -0400 Subject: [PATCH] fix(sourceconfig): reject unsafe source paths Signed-off-by: samzong --- internal/sourceconfig/sources.go | 47 +++++++++++++++ internal/sourceconfig/sources_test.go | 85 +++++++++++++++++++++++++++ internal/specsync/graphql.go | 11 +++- internal/specsync/openapi3.go | 11 +++- internal/specsync/path.go | 28 +++++++++ internal/specsync/proto.go | 21 +++++-- internal/specsync/swagger.go | 10 +++- 7 files changed, 201 insertions(+), 12 deletions(-) create mode 100644 internal/specsync/path.go diff --git a/internal/sourceconfig/sources.go b/internal/sourceconfig/sources.go index 8b61238..35b6472 100644 --- a/internal/sourceconfig/sources.go +++ b/internal/sourceconfig/sources.go @@ -157,6 +157,9 @@ func validate(s *Source, baseDir string) error { if s.Swagger == nil || len(s.Swagger.Files) == 0 { return fmt.Errorf("backend=swagger requires non-empty swagger.files") } + if err := validateRelPathList("swagger.files", s.Swagger.Files); err != nil { + return err + } case BackendProto: if s.Proto == nil || len(s.Proto.Entries) == 0 { return fmt.Errorf("backend=proto requires non-empty proto.entries") @@ -164,14 +167,34 @@ func validate(s *Source, baseDir string) error { if len(s.Proto.Staging) == 0 { return fmt.Errorf("backend=proto requires non-empty proto.staging") } + for _, st := range s.Proto.Staging { + if err := ValidateRelPath("proto.staging.from", st.From); err != nil { + return err + } + if err := ValidateRelPath("proto.staging.to", st.To); err != nil { + return err + } + } + if err := validateRelPathList("proto.entries", s.Proto.Entries); err != nil { + return err + } + if err := validateRelPathList("proto.import_roots", s.Proto.ImportRoots); err != nil { + return err + } case BackendOpenAPI3: if s.OpenAPI3 == nil || len(s.OpenAPI3.Files) == 0 { return fmt.Errorf("backend=openapi3 requires non-empty openapi3.files") } + if err := validateRelPathList("openapi3.files", s.OpenAPI3.Files); err != nil { + return err + } case BackendGraphQL: if s.GraphQL == nil || s.GraphQL.Schema == "" { return fmt.Errorf("backend=graphql requires graphql.schema") } + if err := ValidateRelPath("graphql.schema", s.GraphQL.Schema); err != nil { + return err + } if s.GraphQL.Expose == nil || (len(s.GraphQL.Expose.Queries) == 0 && len(s.GraphQL.Expose.Mutations) == 0) { return fmt.Errorf("backend=graphql requires an explicit graphql.expose policy (queries and/or mutations); refusing to expose the whole schema") } @@ -186,6 +209,30 @@ func validate(s *Source, baseDir string) error { return rejectForeignBlocks(s) } +func validateRelPathList(field string, paths []string) error { + for _, p := range paths { + if err := ValidateRelPath(field, p); err != nil { + return err + } + } + return nil +} + +func ValidateRelPath(field, value string) error { + if value == "" { + return fmt.Errorf("unsafe path %s: empty path", field) + } + if filepath.IsAbs(value) || !filepath.IsLocal(value) { + return fmt.Errorf("unsafe path %s: %q", field, value) + } + for _, part := range strings.Split(strings.ReplaceAll(value, "\\", "/"), "/") { + if part == "" || part == ".." { + return fmt.Errorf("unsafe path %s: %q", field, value) + } + } + return nil +} + func resolveLocalPath(baseDir, raw string) (string, error) { if raw == "" { return "", fmt.Errorf("local_path must not be empty") diff --git a/internal/sourceconfig/sources_test.go b/internal/sourceconfig/sources_test.go index d7718c4..d9bbb73 100644 --- a/internal/sourceconfig/sources_test.go +++ b/internal/sourceconfig/sources_test.go @@ -241,6 +241,91 @@ func TestLoad_RejectsOpenAPI3WithoutFiles(t *testing.T) { } } +func TestValidate_RejectsUnsafeSourcePaths(t *testing.T) { + remote := func(src Source) Source { + src.RepoURL = "https://example.com/repo.git" + src.PinnedTag = "v2.0.0" + return src + } + cases := []struct { + name string + src Source + }{ + { + name: "swagger absolute file", + src: remote(Source{Backend: BackendSwagger, Swagger: &SwaggerConfig{Files: []string{"/tmp/api.json"}}}), + }, + { + name: "openapi empty segment", + src: remote(Source{Backend: BackendOpenAPI3, OpenAPI3: &OpenAPI3Config{Files: []string{"api//openapi.yaml"}}}), + }, + { + name: "proto staging from traversal", + src: remote(Source{ + Backend: BackendProto, + Proto: &ProtoConfig{ + Staging: []StagingEntry{{From: "../proto", To: "proto"}}, + Entries: []string{"api/v1/service.proto"}, + }, + }), + }, + { + name: "proto staging to absolute", + src: remote(Source{ + Backend: BackendProto, + Proto: &ProtoConfig{ + Staging: []StagingEntry{{From: "proto", To: "/tmp/proto"}}, + Entries: []string{"api/v1/service.proto"}, + }, + }), + }, + { + name: "proto entry traversal", + src: remote(Source{ + Backend: BackendProto, + Proto: &ProtoConfig{ + Staging: []StagingEntry{{From: "proto", To: "proto"}}, + Entries: []string{"../api/v1/service.proto"}, + }, + }), + }, + { + name: "proto import root traversal", + src: remote(Source{ + Backend: BackendProto, + Proto: &ProtoConfig{ + Staging: []StagingEntry{{From: "proto", To: "proto"}}, + Entries: []string{"api/v1/service.proto"}, + ImportRoots: []string{"../includes"}, + }, + }), + }, + { + name: "graphql schema traversal", + src: remote(Source{ + Backend: BackendGraphQL, + GraphQL: &GraphQLConfig{ + Schema: "../schema.graphql", + Expose: &GraphQLExpose{ + Queries: []string{"viewer"}, + }, + }, + }), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validate(&tc.src, t.TempDir()) + if err == nil { + t.Fatal("validate accepted unsafe source path; want rejection") + } + if !strings.Contains(err.Error(), "unsafe path") { + t.Errorf("error = %v, want to mention unsafe path", err) + } + }) + } +} + func TestLoad_RejectsOpenAPI3WithSwaggerBlock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "sources.yaml") diff --git a/internal/specsync/graphql.go b/internal/specsync/graphql.go index c081418..097d30c 100644 --- a/internal/specsync/graphql.go +++ b/internal/specsync/graphql.go @@ -3,15 +3,20 @@ package specsync import ( "fmt" "os" - "path/filepath" "github.com/lathe-cli/lathe/internal/sourceconfig" ) func syncGraphQL(src *sourceconfig.Source, workDir, syncDir string) error { rel := src.GraphQL.Schema - srcPath := filepath.Join(workDir, rel) - dstPath := filepath.Join(syncDir, rel) + srcPath, err := safeJoin(workDir, rel) + if err != nil { + return err + } + dstPath, err := safeJoin(syncDir, rel) + if err != nil { + return err + } if _, err := os.Stat(srcPath); err != nil { return fmt.Errorf("missing %s in %s@%s", rel, src.Name, src.PinnedTag) } diff --git a/internal/specsync/openapi3.go b/internal/specsync/openapi3.go index d1d549e..d177e97 100644 --- a/internal/specsync/openapi3.go +++ b/internal/specsync/openapi3.go @@ -3,15 +3,20 @@ package specsync import ( "fmt" "os" - "path/filepath" "github.com/lathe-cli/lathe/internal/sourceconfig" ) func syncOpenAPI3(src *sourceconfig.Source, workDir, syncDir string) error { for i, rel := range src.OpenAPI3.Files { - srcPath := filepath.Join(workDir, rel) - dstPath := filepath.Join(syncDir, rel) + srcPath, err := safeJoin(workDir, rel) + if err != nil { + return err + } + dstPath, err := safeJoin(syncDir, rel) + if err != nil { + return err + } if _, err := os.Stat(srcPath); err != nil { return fmt.Errorf("missing %s in %s@%s", rel, src.Name, src.PinnedTag) } diff --git a/internal/specsync/path.go b/internal/specsync/path.go new file mode 100644 index 0000000..fe083ad --- /dev/null +++ b/internal/specsync/path.go @@ -0,0 +1,28 @@ +package specsync + +import ( + "fmt" + "path/filepath" + "strings" + + "github.com/lathe-cli/lathe/internal/sourceconfig" +) + +func safeJoin(root, rel string) (string, error) { + if err := sourceconfig.ValidateRelPath("source path", rel); err != nil { + return "", err + } + rootAbs, err := filepath.Abs(root) + if err != nil { + return "", fmt.Errorf("resolve root path: %w", err) + } + full := filepath.Join(rootAbs, rel) + inside, err := filepath.Rel(rootAbs, full) + if err != nil { + return "", fmt.Errorf("resolve source path %q: %w", rel, err) + } + if inside == ".." || strings.HasPrefix(inside, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("unsafe path source path: %q", rel) + } + return full, nil +} diff --git a/internal/specsync/proto.go b/internal/specsync/proto.go index 97f1d6d..caffbec 100644 --- a/internal/specsync/proto.go +++ b/internal/specsync/proto.go @@ -12,8 +12,14 @@ import ( func syncProto(src *sourceconfig.Source, workDir, syncDir string) error { for _, st := range src.Proto.Staging { - from := filepath.Join(workDir, st.From) - to := filepath.Join(syncDir, st.To) + from, err := safeJoin(workDir, st.From) + if err != nil { + return err + } + to, err := safeJoin(syncDir, st.To) + if err != nil { + return err + } if _, err := os.Stat(from); err != nil { return fmt.Errorf("staging %s: source missing: %w", st.From, err) } @@ -24,7 +30,10 @@ func syncProto(src *sourceconfig.Source, workDir, syncDir string) error { } entries := make([]string, 0, len(src.Proto.Entries)) for _, e := range src.Proto.Entries { - full := filepath.Join(syncDir, e) + full, err := safeJoin(syncDir, e) + if err != nil { + return err + } if _, err := os.Stat(full); err != nil { return fmt.Errorf("entry %s not found in staged tree: %w", e, err) } @@ -38,7 +47,11 @@ func syncProto(src *sourceconfig.Source, workDir, syncDir string) error { "--descriptor_set_out=" + descOut, } for _, r := range src.Proto.ImportRoots { - args = append(args, "-I", filepath.Join(syncDir, r)) + full, err := safeJoin(syncDir, r) + if err != nil { + return err + } + args = append(args, "-I", full) } args = append(args, entries...) cmd := exec.Command("protoc", args...) diff --git a/internal/specsync/swagger.go b/internal/specsync/swagger.go index a537798..651af79 100644 --- a/internal/specsync/swagger.go +++ b/internal/specsync/swagger.go @@ -11,8 +11,14 @@ import ( func syncSwagger(src *sourceconfig.Source, workDir, syncDir string) error { for i, rel := range src.Swagger.Files { - srcPath := filepath.Join(workDir, rel) - dstPath := filepath.Join(syncDir, rel) + srcPath, err := safeJoin(workDir, rel) + if err != nil { + return err + } + dstPath, err := safeJoin(syncDir, rel) + if err != nil { + return err + } if _, err := os.Stat(srcPath); err != nil { return fmt.Errorf("missing %s in %s@%s", rel, src.Name, src.PinnedTag) }