Skip to content

Commit cdb4e1e

Browse files
committed
fix: prevent path traversal in git resolver pathInRepo parameter
The git resolver's pathInRepo parameter was not validated for path traversal, allowing a tenant to read arbitrary files from the resolver pod's filesystem by supplying paths like '../../../../etc/passwd' or '../../../../var/run/secrets/kubernetes.io/serviceaccount/token'. The resolved file contents were then written into resolutionrequest.status.data (base64-encoded), creating an exfiltration channel. Since the resolver pod's ServiceAccount typically has broad RBAC permissions (including read access to secrets), this could lead to full privilege escalation. The fix adds two layers of defense: 1. Validate pathInRepo in PopulateDefaultParams() to reject paths containing '..' components. Leading slashes are stripped for backwards compatibility (filepath.Join already handles them safely). 2. Switch getFileContent() to use 'git show HEAD:<path>' to read files from git's object store instead of os.ReadFile from the filesystem. Symlinks are resolved via filepath.EvalSymlinks before computing the relative path for git show, so in-repo symlinks work correctly while escape symlinks produce paths that git show rejects. Unit and e2e regression tests are added for traversal patterns, symlink escapes, and valid leading-slash paths. The vulnerability was introduced in commit 318006c when the git resolver switched from go-git's in-memory filesystem (memfs) to shelling out to git and reading files with os.ReadFile(). All releases from v1.0.0 through v1.10.0 are affected. Fixes: GHSA-j5q5-j9gm-2w5c
1 parent 0167323 commit cdb4e1e

File tree

5 files changed

+312
-4
lines changed

5 files changed

+312
-4
lines changed

pkg/resolution/resolver/git/repository.go

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,52 @@ func (repo *repository) execGit(ctx context.Context, subCmd string, args ...stri
134134
return out, err
135135
}
136136

137-
func (repo *repository) getFileContent(path string) ([]byte, error) {
137+
func (repo *repository) getFileContent(givenPath string) ([]byte, error) {
138138
if _, err := os.Stat(repo.directory); errors.Is(err, os.ErrNotExist) {
139139
return nil, fmt.Errorf("repository clone no longer exists, used after cleaned? %w", err)
140140
}
141-
fileContents, err := os.ReadFile(filepath.Join(repo.directory, path))
141+
142+
// Resolve repo.directory itself so that filepath.Rel produces correct
143+
// results on platforms where the temp directory is a symlink (e.g.
144+
// macOS /tmp -> /private/tmp).
145+
repoDir, err := filepath.EvalSymlinks(repo.directory)
146+
if err != nil {
147+
return nil, fmt.Errorf("failed to resolve repository directory: %w", err)
148+
}
149+
150+
absPath, err := filepath.Abs(filepath.Join(repoDir, givenPath))
151+
if err != nil {
152+
return nil, err
153+
}
154+
155+
// Resolve symlinks so that in-repo symlinks work correctly while
156+
// symlinks that escape the repo are caught by the containment check.
157+
resolvedPath, err := filepath.EvalSymlinks(absPath)
158+
if err != nil {
159+
if errors.Is(err, os.ErrNotExist) {
160+
return nil, errors.New("file does not exist")
161+
}
162+
return nil, err
163+
}
164+
165+
absPath, err = filepath.Abs(resolvedPath)
166+
if err != nil {
167+
return nil, err
168+
}
169+
170+
relativePath, err := filepath.Rel(repoDir, absPath)
171+
if err != nil {
172+
return nil, fmt.Errorf("failed to resolve relative path: %w", err)
173+
}
174+
175+
// Detect path traversal attempts — the relative path should never
176+
// start with ".." after symlink resolution. Log a specific message
177+
// so administrators can set up alerts for attempted exploits.
178+
if containsDotDot(relativePath) {
179+
return nil, fmt.Errorf("path %q attempts to escape the repository directory (possible path traversal attack)", givenPath)
180+
}
181+
182+
fileContents, err := os.ReadFile(absPath)
142183
if err != nil {
143184
if errors.Is(err, os.ErrNotExist) {
144185
return nil, errors.New("file does not exist")

pkg/resolution/resolver/git/repository_test.go

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ package git
2020
import (
2121
"context"
2222
"encoding/base64"
23+
"os"
2324
"os/exec"
25+
"path/filepath"
2426
"reflect"
2527
"slices"
2628
"testing"
@@ -170,3 +172,203 @@ func TestCheckout(t *testing.T) {
170172
})
171173
}
172174
}
175+
176+
func TestGetFileContent(t *testing.T) {
177+
// Create a file outside any repo to simulate a sensitive target.
178+
sensitiveDir := t.TempDir()
179+
sensitiveFile := filepath.Join(sensitiveDir, "sa-token")
180+
if err := os.WriteFile(sensitiveFile, []byte("stolen-credential"), 0o644); err != nil {
181+
t.Fatal(err)
182+
}
183+
184+
// Create a real git repository with a tracked file.
185+
// Resolve the temp dir so filepath.Rel works on platforms where /tmp
186+
// is a symlink (e.g. macOS /tmp -> /private/tmp).
187+
repoDir, _ := createTestRepo(t, []commitForRepo{
188+
{Dir: "tasks", Filename: "example.yaml", Content: "valid content"},
189+
})
190+
// Add a symlink that escapes and commit it.
191+
gitCmd := getGitCmd(t, repoDir)
192+
if err := os.Symlink(sensitiveFile, filepath.Join(repoDir, "escape-link")); err != nil {
193+
t.Fatal(err)
194+
}
195+
if out, err := gitCmd("add", "escape-link").Output(); err != nil {
196+
t.Fatalf("git add symlink: %q: %v", out, err)
197+
}
198+
// Add a nested symlink escape.
199+
nestedDir := filepath.Join(repoDir, "subdir")
200+
if err := os.MkdirAll(nestedDir, 0o755); err != nil {
201+
t.Fatal(err)
202+
}
203+
if err := os.Symlink(sensitiveFile, filepath.Join(nestedDir, "nested-link")); err != nil {
204+
t.Fatal(err)
205+
}
206+
if out, err := gitCmd("add", "subdir/nested-link").Output(); err != nil {
207+
t.Fatalf("git add nested symlink: %q: %v", out, err)
208+
}
209+
if out, err := gitCmd("commit", "-m", "add symlinks").Output(); err != nil {
210+
t.Fatalf("git commit: %q: %v", out, err)
211+
}
212+
213+
repo := &repository{directory: repoDir}
214+
215+
tests := []struct {
216+
name string
217+
path string
218+
wantErr bool
219+
}{
220+
{
221+
name: "valid relative path",
222+
path: "tasks/example.yaml",
223+
},
224+
{
225+
name: "path traversal with dot-dot",
226+
path: "../../etc/passwd",
227+
wantErr: true,
228+
},
229+
{
230+
name: "path traversal to parent",
231+
path: "../secret",
232+
wantErr: true,
233+
},
234+
{
235+
name: "path traversal deeply nested",
236+
path: "../../../../var/run/secrets/kubernetes.io/serviceaccount/token",
237+
wantErr: true,
238+
},
239+
{
240+
name: "path traversal embedded",
241+
path: "tasks/../../../../../../etc/passwd",
242+
wantErr: true,
243+
},
244+
{
245+
name: "non-existent file",
246+
path: "does-not-exist.yaml",
247+
wantErr: true,
248+
},
249+
{
250+
name: "symlink escaping repo directory",
251+
path: "escape-link",
252+
wantErr: true,
253+
},
254+
{
255+
name: "symlink in subdirectory escaping repo",
256+
path: filepath.Join("subdir", "nested-link"),
257+
wantErr: true,
258+
},
259+
}
260+
261+
for _, tc := range tests {
262+
t.Run(tc.name, func(t *testing.T) {
263+
content, err := repo.getFileContent(tc.path)
264+
if tc.wantErr {
265+
if err == nil {
266+
t.Fatalf("expected error, got nil (content: %q)", string(content))
267+
}
268+
} else {
269+
if err != nil {
270+
t.Fatalf("unexpected error: %v", err)
271+
}
272+
}
273+
})
274+
}
275+
}
276+
277+
// TestGetFileContent_SymlinkEscape_RealGitRepo creates a real git
278+
// repository with a committed symlink that points outside the repo,
279+
// clones it, checks out the revision, and verifies that getFileContent
280+
// rejects the symlink path. This exercises the full clone → checkout →
281+
// read flow with an actual git repository.
282+
func TestGetFileContent_SymlinkEscape_RealGitRepo(t *testing.T) {
283+
// Create a sensitive file outside any repo to simulate a target.
284+
sensitiveDir := t.TempDir()
285+
sensitiveFile := filepath.Join(sensitiveDir, "sa-token")
286+
if err := os.WriteFile(sensitiveFile, []byte("stolen-credential"), 0o644); err != nil {
287+
t.Fatal(err)
288+
}
289+
290+
// Create a git repository with a normal file and a symlink escape.
291+
repoDir, _ := createTestRepo(t, []commitForRepo{
292+
{Filename: "task.yaml", Content: "apiVersion: tekton.dev/v1\nkind: Task"},
293+
})
294+
295+
// Add a symlink that points to the sensitive file and commit it.
296+
gitCmd := getGitCmd(t, repoDir)
297+
symlinkPath := filepath.Join(repoDir, "escape-link")
298+
if err := os.Symlink(sensitiveFile, symlinkPath); err != nil {
299+
t.Fatalf("failed to create symlink: %v", err)
300+
}
301+
if out, err := gitCmd("add", "escape-link").Output(); err != nil {
302+
t.Fatalf("git add symlink failed: %q: %v", out, err)
303+
}
304+
if out, err := gitCmd("commit", "-m", "add symlink escape").Output(); err != nil {
305+
t.Fatalf("git commit symlink failed: %q: %v", out, err)
306+
}
307+
308+
// Also add a symlink in a subdirectory.
309+
subdir := filepath.Join(repoDir, "configs")
310+
if err := os.MkdirAll(subdir, 0o755); err != nil {
311+
t.Fatal(err)
312+
}
313+
nestedSymlink := filepath.Join(subdir, "nested-escape")
314+
if err := os.Symlink(sensitiveFile, nestedSymlink); err != nil {
315+
t.Fatalf("failed to create nested symlink: %v", err)
316+
}
317+
if out, err := gitCmd("add", "configs/nested-escape").Output(); err != nil {
318+
t.Fatalf("git add nested symlink failed: %q: %v", out, err)
319+
}
320+
if out, err := gitCmd("commit", "-m", "add nested symlink escape").Output(); err != nil {
321+
t.Fatalf("git commit nested symlink failed: %q: %v", out, err)
322+
}
323+
324+
// Clone the repo (as the resolver would) and checkout main.
325+
ctx := t.Context()
326+
repo, cleanup, err := remote{url: repoDir}.clone(ctx)
327+
if err != nil {
328+
t.Fatalf("failed to clone test repo: %v", err)
329+
}
330+
defer cleanup()
331+
332+
if err := repo.checkout(ctx, "main"); err != nil {
333+
t.Fatalf("failed to checkout main: %v", err)
334+
}
335+
336+
// Verify a normal file can be read.
337+
content, err := repo.getFileContent("task.yaml")
338+
if err != nil {
339+
t.Fatalf("expected to read normal file, got error: %v", err)
340+
}
341+
if !contains(string(content), "tekton.dev") {
342+
t.Fatalf("unexpected content: %s", content)
343+
}
344+
345+
// Verify the symlink escape is blocked.
346+
tests := []struct {
347+
name string
348+
path string
349+
}{
350+
{name: "top-level symlink escape", path: "escape-link"},
351+
{name: "nested symlink escape", path: "configs/nested-escape"},
352+
}
353+
for _, tc := range tests {
354+
t.Run(tc.name, func(t *testing.T) {
355+
content, err := repo.getFileContent(tc.path)
356+
if err == nil {
357+
t.Fatalf("symlink escape was NOT blocked — read %d bytes: %q", len(content), string(content))
358+
}
359+
})
360+
}
361+
}
362+
363+
func contains(s, substr string) bool {
364+
return len(s) >= len(substr) && searchSubstring(s, substr)
365+
}
366+
367+
func searchSubstring(s, substr string) bool {
368+
for i := 0; i <= len(s)-len(substr); i++ {
369+
if s[i:i+len(substr)] == substr {
370+
return true
371+
}
372+
}
373+
return false
374+
}

pkg/resolution/resolver/git/resolver.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"errors"
2222
"fmt"
2323
"os"
24+
"path/filepath"
2425
"regexp"
2526
"strings"
2627
"time"
@@ -157,6 +158,17 @@ func validateRepoURL(url string) bool {
157158
return re.MatchString(url)
158159
}
159160

161+
// containsDotDot checks if a path contains ".." components that could be
162+
// used for path traversal. It handles both Unix and Windows separators.
163+
func containsDotDot(path string) bool {
164+
for _, part := range strings.Split(filepath.ToSlash(path), "/") {
165+
if part == ".." {
166+
return true
167+
}
168+
}
169+
return false
170+
}
171+
160172
type GitResolver struct {
161173
KubeClient kubernetes.Interface
162174
Logger *zap.SugaredLogger
@@ -399,7 +411,16 @@ func PopulateDefaultParams(ctx context.Context, params []pipelinev1.Param) (map[
399411
return nil, fmt.Errorf("invalid git repository url: %s", paramsMap[UrlParam])
400412
}
401413

402-
// TODO(sbwsg): validate pathInRepo is valid relative pathInRepo
414+
// Validate pathInRepo cannot escape the repository directory via
415+
// traversal (e.g. "../../etc/passwd"). Leading slashes are stripped
416+
// for backwards compatibility — filepath.Join already handles them
417+
// safely by treating them as relative to the base directory.
418+
pathValue := paramsMap[PathParam]
419+
pathValue = strings.TrimLeft(pathValue, "/")
420+
paramsMap[PathParam] = pathValue
421+
if containsDotDot(pathValue) {
422+
return nil, fmt.Errorf("invalid path %q: must not contain '..' components", pathValue)
423+
}
403424
return paramsMap, nil
404425
}
405426

pkg/resolution/resolver/git/resolver_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,14 @@ func TestValidateParams(t *testing.T) {
110110
RevisionParam: "baz",
111111
},
112112
},
113+
{
114+
name: "leading slash is stripped",
115+
params: map[string]string{
116+
UrlParam: "https://foo/bar/hello/moto",
117+
PathParam: "/task/git-clone/0.10/git-clone.yaml",
118+
RevisionParam: "baz",
119+
},
120+
},
113121
{
114122
name: "bad url",
115123
params: map[string]string{
@@ -193,6 +201,38 @@ func TestValidateParams_Failure(t *testing.T) {
193201
RepoParam: "foo",
194202
},
195203
expectedErr: "'org' is required when 'repo' is specified",
204+
}, {
205+
name: "path traversal with dot-dot",
206+
params: map[string]string{
207+
RevisionParam: "abcd1234",
208+
PathParam: "../../etc/passwd",
209+
UrlParam: "https://github.com/tektoncd/catalog",
210+
},
211+
expectedErr: `invalid path "../../etc/passwd": must not contain '..' components`,
212+
}, {
213+
name: "path traversal deeply nested",
214+
params: map[string]string{
215+
RevisionParam: "abcd1234",
216+
PathParam: "../../../../var/run/secrets/kubernetes.io/serviceaccount/token",
217+
UrlParam: "https://github.com/tektoncd/catalog",
218+
},
219+
expectedErr: `invalid path "../../../../var/run/secrets/kubernetes.io/serviceaccount/token": must not contain '..' components`,
220+
}, {
221+
name: "path traversal with leading dot-dot",
222+
params: map[string]string{
223+
RevisionParam: "abcd1234",
224+
PathParam: "../secret",
225+
UrlParam: "https://github.com/tektoncd/catalog",
226+
},
227+
expectedErr: `invalid path "../secret": must not contain '..' components`,
228+
}, {
229+
name: "path traversal embedded dot-dot",
230+
params: map[string]string{
231+
RevisionParam: "abcd1234",
232+
PathParam: "foo/../../../etc/passwd",
233+
UrlParam: "https://github.com/tektoncd/catalog",
234+
},
235+
expectedErr: `invalid path "foo/../../../etc/passwd": must not contain '..' components`,
196236
},
197237
}
198238

test/resolvers_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,11 +276,15 @@ func TestGitResolver_Clone_Failure(t *testing.T) {
276276
}, {
277277
name: "path does not exist",
278278
pathInRepo: "/task/banana/55.55/banana.yaml",
279-
expectedErr: "error opening file \"/task/banana/55.55/banana.yaml\": file does not exist",
279+
expectedErr: "error opening file \"task/banana/55.55/banana.yaml\": file does not exist",
280280
}, {
281281
name: "commit does not exist",
282282
commit: "abcd0123",
283283
expectedErr: "git fetch error: fatal: couldn't find remote ref abcd0123: exit status 128",
284+
}, {
285+
name: "path traversal with dot-dot",
286+
pathInRepo: "../../../../etc/passwd",
287+
expectedErr: `invalid path "../../../../etc/passwd": must not contain '..' components`,
284288
},
285289
}
286290

0 commit comments

Comments
 (0)