Skip to content

Commit 474eec1

Browse files
pd93pd93
committed
feat: redact credentials in remote urls
1 parent fc17343 commit 474eec1

File tree

6 files changed

+117
-51
lines changed

6 files changed

+117
-51
lines changed

‎task_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,7 @@ func TestIncludesRemote(t *testing.T) {
702702
enableExperimentForTest(t, &experiments.RemoteTaskfiles, 1)
703703

704704
dir := "testdata/includes_remote"
705+
os.RemoveAll(filepath.Join(dir, ".task", "remote"))
705706

706707
srv := httptest.NewServer(http.FileServer(http.Dir(dir)))
707708
defer srv.Close()
@@ -777,8 +778,8 @@ func TestIncludesRemote(t *testing.T) {
777778
},
778779
}
779780

780-
for j, e := range executors {
781-
t.Run(fmt.Sprint(j), func(t *testing.T) {
781+
for _, e := range executors {
782+
t.Run(e.name, func(t *testing.T) {
782783
require.NoError(t, e.executor.Setup())
783784

784785
for k, taskCall := range taskCalls {

‎taskfile/node_git.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,10 @@ func NewGitNode(
4040
return nil, err
4141
}
4242

43-
basePath, path := func() (string, string) {
44-
x := strings.Split(u.Path, "//")
45-
return x[0], x[1]
46-
}()
43+
basePath, path := splitPath(u)
4744
ref := u.Query().Get("ref")
4845

49-
rawUrl := u.String()
46+
rawUrl := u.Redacted()
5047

5148
u.RawQuery = ""
5249
u.Path = basePath
@@ -127,11 +124,23 @@ func (node *GitNode) ResolveDir(dir string) (string, error) {
127124

128125
func (node *GitNode) CacheKey() string {
129126
checksum := strings.TrimRight(checksum([]byte(node.Location())), "=")
130-
prefix := filepath.Base(filepath.Dir(node.path))
131-
lastDir := filepath.Base(node.path)
127+
lastDir := filepath.Base(filepath.Dir(node.path))
128+
prefix := filepath.Base(node.path)
132129
// Means it's not "", nor "." nor "/", so it's a valid directory
133130
if len(lastDir) > 1 {
134-
prefix = fmt.Sprintf("%s-%s", lastDir, prefix)
131+
prefix = fmt.Sprintf("%s.%s", lastDir, prefix)
132+
}
133+
return fmt.Sprintf("git.%s.%s.%s", node.URL.Host, prefix, checksum)
134+
}
135+
136+
func splitPath(u *url.URL) (string, string) {
137+
x := strings.Split(u.Path, "//")
138+
switch len(x) {
139+
case 0:
140+
return "", ""
141+
case 1:
142+
return x[0], ""
143+
default:
144+
return x[0], x[1]
135145
}
136-
return fmt.Sprintf("%s.%s", prefix, checksum)
137146
}

‎taskfile/node_git_test.go‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55

66
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
78
)
89

910
func TestGitNode_ssh(t *testing.T) {
@@ -13,7 +14,7 @@ func TestGitNode_ssh(t *testing.T) {
1314
assert.NoError(t, err)
1415
assert.Equal(t, "main", node.ref)
1516
assert.Equal(t, "Taskfile.yml", node.path)
16-
assert.Equal(t, "ssh://[email protected]/foo/bar.git//Taskfile.yml?ref=main", node.rawUrl)
17+
assert.Equal(t, "ssh://[email protected]/foo/bar.git//Taskfile.yml?ref=main", node.Location())
1718
assert.Equal(t, "ssh://[email protected]/foo/bar.git", node.URL.String())
1819
entrypoint, err := node.ResolveEntrypoint("common.yml")
1920
assert.NoError(t, err)
@@ -27,7 +28,7 @@ func TestGitNode_sshWithDir(t *testing.T) {
2728
assert.NoError(t, err)
2829
assert.Equal(t, "main", node.ref)
2930
assert.Equal(t, "directory/Taskfile.yml", node.path)
30-
assert.Equal(t, "ssh://[email protected]/foo/bar.git//directory/Taskfile.yml?ref=main", node.rawUrl)
31+
assert.Equal(t, "ssh://[email protected]/foo/bar.git//directory/Taskfile.yml?ref=main", node.Location())
3132
assert.Equal(t, "ssh://[email protected]/foo/bar.git", node.URL.String())
3233
entrypoint, err := node.ResolveEntrypoint("common.yml")
3334
assert.NoError(t, err)
@@ -41,7 +42,7 @@ func TestGitNode_https(t *testing.T) {
4142
assert.NoError(t, err)
4243
assert.Equal(t, "main", node.ref)
4344
assert.Equal(t, "Taskfile.yml", node.path)
44-
assert.Equal(t, "https://github.com/foo/bar.git//Taskfile.yml?ref=main", node.rawUrl)
45+
assert.Equal(t, "https://github.com/foo/bar.git//Taskfile.yml?ref=main", node.Location())
4546
assert.Equal(t, "https://github.com/foo/bar.git", node.URL.String())
4647
entrypoint, err := node.ResolveEntrypoint("common.yml")
4748
assert.NoError(t, err)
@@ -55,7 +56,7 @@ func TestGitNode_httpsWithDir(t *testing.T) {
5556
assert.NoError(t, err)
5657
assert.Equal(t, "main", node.ref)
5758
assert.Equal(t, "directory/Taskfile.yml", node.path)
58-
assert.Equal(t, "https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main", node.rawUrl)
59+
assert.Equal(t, "https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main", node.Location())
5960
assert.Equal(t, "https://github.com/foo/bar.git", node.URL.String())
6061
entrypoint, err := node.ResolveEntrypoint("common.yml")
6162
assert.NoError(t, err)
@@ -65,18 +66,28 @@ func TestGitNode_httpsWithDir(t *testing.T) {
6566
func TestGitNode_CacheKey(t *testing.T) {
6667
t.Parallel()
6768

68-
node, err := NewGitNode("https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main", "", false)
69-
assert.NoError(t, err)
70-
key := node.CacheKey()
71-
assert.Equal(t, "Taskfile.yml-directory.f1ddddac425a538870230a3e38fc0cded4ec5da250797b6cab62c82477718fbb", key)
69+
tests := []struct {
70+
entrypoint string
71+
expectedKey string
72+
}{
73+
{
74+
entrypoint: "https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main",
75+
expectedKey: "git.github.com.directory.Taskfile.yml.f1ddddac425a538870230a3e38fc0cded4ec5da250797b6cab62c82477718fbb",
76+
},
77+
{
78+
entrypoint: "https://github.com/foo/bar.git//Taskfile.yml?ref=main",
79+
expectedKey: "git.github.com.Taskfile.yml.39d28c1ff36f973705ae188b991258bbabaffd6d60bcdde9693d157d00d5e3a4",
80+
},
81+
{
82+
entrypoint: "https://github.com/foo/bar.git//multiple/directory/Taskfile.yml?ref=main",
83+
expectedKey: "git.github.com.directory.Taskfile.yml.1b6d145e01406dcc6c0aa572e5a5d1333be1ccf2cae96d18296d725d86197d31",
84+
},
85+
}
7286

73-
node, err = NewGitNode("https://github.com/foo/bar.git//Taskfile.yml?ref=main", "", false)
74-
assert.NoError(t, err)
75-
key = node.CacheKey()
76-
assert.Equal(t, "Taskfile.yml-..39d28c1ff36f973705ae188b991258bbabaffd6d60bcdde9693d157d00d5e3a4", key)
77-
78-
node, err = NewGitNode("https://github.com/foo/bar.git//multiple/directory/Taskfile.yml?ref=main", "", false)
79-
assert.NoError(t, err)
80-
key = node.CacheKey()
81-
assert.Equal(t, "Taskfile.yml-directory.1b6d145e01406dcc6c0aa572e5a5d1333be1ccf2cae96d18296d725d86197d31", key)
87+
for _, tt := range tests {
88+
node, err := NewGitNode(tt.entrypoint, "", false)
89+
require.NoError(t, err)
90+
key := node.CacheKey()
91+
assert.Equal(t, tt.expectedKey, key)
92+
}
8293
}

‎taskfile/node_http.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ import (
1717
// An HTTPNode is a node that reads a Taskfile from a remote location via HTTP.
1818
type HTTPNode struct {
1919
*BaseNode
20-
URL *url.URL // stores url pointing actual remote file. (e.g. with Taskfile.yml)
21-
entrypoint string // stores entrypoint url. used for building graph vertices.
20+
URL *url.URL // stores url pointing actual remote file. (e.g. with Taskfile.yml)
2221
}
2322

2423
func NewHTTPNode(
@@ -33,46 +32,43 @@ func NewHTTPNode(
3332
return nil, err
3433
}
3534
if url.Scheme == "http" && !insecure {
36-
return nil, &errors.TaskfileNotSecureError{URI: entrypoint}
35+
return nil, &errors.TaskfileNotSecureError{URI: url.Redacted()}
3736
}
38-
3937
return &HTTPNode{
40-
BaseNode: base,
41-
URL: url,
42-
entrypoint: entrypoint,
38+
BaseNode: base,
39+
URL: url,
4340
}, nil
4441
}
4542

4643
func (node *HTTPNode) Location() string {
47-
return node.entrypoint
44+
return node.URL.Redacted()
4845
}
4946

5047
func (node *HTTPNode) Read() ([]byte, error) {
5148
return node.ReadContext(context.Background())
5249
}
5350

5451
func (node *HTTPNode) ReadContext(ctx context.Context) ([]byte, error) {
55-
url, err := RemoteExists(ctx, node.URL)
52+
url, err := RemoteExists(ctx, *node.URL)
5653
if err != nil {
5754
return nil, err
5855
}
59-
node.URL = url
60-
req, err := http.NewRequest("GET", node.URL.String(), nil)
56+
req, err := http.NewRequest("GET", url.String(), nil)
6157
if err != nil {
62-
return nil, errors.TaskfileFetchFailedError{URI: node.URL.String()}
58+
return nil, errors.TaskfileFetchFailedError{URI: node.Location()}
6359
}
6460

6561
resp, err := http.DefaultClient.Do(req.WithContext(ctx))
6662
if err != nil {
6763
if ctx.Err() != nil {
6864
return nil, err
6965
}
70-
return nil, errors.TaskfileFetchFailedError{URI: node.URL.String()}
66+
return nil, errors.TaskfileFetchFailedError{URI: node.Location()}
7167
}
7268
defer resp.Body.Close()
7369
if resp.StatusCode != http.StatusOK {
7470
return nil, errors.TaskfileFetchFailedError{
75-
URI: node.URL.String(),
71+
URI: node.Location(),
7672
HTTPStatusCode: resp.StatusCode,
7773
}
7874
}
@@ -116,12 +112,12 @@ func (node *HTTPNode) ResolveDir(dir string) (string, error) {
116112

117113
func (node *HTTPNode) CacheKey() string {
118114
checksum := strings.TrimRight(checksum([]byte(node.Location())), "=")
119-
dir, filename := filepath.Split(node.entrypoint)
115+
dir, filename := filepath.Split(node.URL.Path)
120116
lastDir := filepath.Base(dir)
121117
prefix := filename
122118
// Means it's not "", nor "." nor "/", so it's a valid directory
123119
if len(lastDir) > 1 {
124-
prefix = fmt.Sprintf("%s-%s", lastDir, filename)
120+
prefix = fmt.Sprintf("%s.%s", lastDir, filename)
125121
}
126-
return fmt.Sprintf("%s.%s", prefix, checksum)
122+
return fmt.Sprintf("http.%s.%s.%s", node.URL.Host, prefix, checksum)
127123
}

‎taskfile/node_http_test.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package taskfile
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestHTTPNode_CacheKey(t *testing.T) {
11+
t.Parallel()
12+
13+
tests := []struct {
14+
entrypoint string
15+
expectedKey string
16+
}{
17+
{
18+
entrypoint: "https://github.com",
19+
expectedKey: "http.github.com..996e1f714b08e971ec79e3bea686287e66441f043177999a13dbc546d8fe402a",
20+
},
21+
{
22+
entrypoint: "https://github.com/Taskfile.yml",
23+
expectedKey: "http.github.com.Taskfile.yml.85b3c3ad71b78dc74e404c7b4390fc13672925cb644a4d26c21b9f97c17b5fc0",
24+
},
25+
{
26+
entrypoint: "https://github.com/foo",
27+
expectedKey: "http.github.com.foo.df3158dafc823e6847d9bcaf79328446c4877405e79b100723fa6fd545ed3e2b",
28+
},
29+
{
30+
entrypoint: "https://github.com/foo/Taskfile.yml",
31+
expectedKey: "http.github.com.foo.Taskfile.yml.aea946ea7eb6f6bb4e159e8b840b6b50975927778b2e666df988c03bbf10c4c4",
32+
},
33+
{
34+
entrypoint: "https://github.com/foo/bar",
35+
expectedKey: "http.github.com.foo.bar.d3514ad1d4daedf9cc2825225070b49ebc8db47fa5177951b2a5b9994597570c",
36+
},
37+
{
38+
entrypoint: "https://github.com/foo/bar/Taskfile.yml",
39+
expectedKey: "http.github.com.bar.Taskfile.yml.b9cf01e01e47c0e96ea536e1a8bd7b3a6f6c1f1881bad438990d2bfd4ccd0ac0",
40+
},
41+
}
42+
43+
for _, tt := range tests {
44+
node, err := NewHTTPNode(tt.entrypoint, "", false)
45+
require.NoError(t, err)
46+
key := node.CacheKey()
47+
assert.Equal(t, tt.expectedKey, key)
48+
}
49+
}

‎taskfile/taskfile.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ var (
3636
// at the given URL with any of the default Taskfile files names. If any of
3737
// these match a file, the first matching path will be returned. If no files are
3838
// found, an error will be returned.
39-
func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) {
39+
func RemoteExists(ctx context.Context, u url.URL) (*url.URL, error) {
4040
// Create a new HEAD request for the given URL to check if the resource exists
4141
req, err := http.NewRequestWithContext(ctx, "HEAD", u.String(), nil)
4242
if err != nil {
43-
return nil, errors.TaskfileFetchFailedError{URI: u.String()}
43+
return nil, errors.TaskfileFetchFailedError{URI: u.Redacted()}
4444
}
4545

4646
// Request the given URL
@@ -49,7 +49,7 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) {
4949
if ctx.Err() != nil {
5050
return nil, fmt.Errorf("checking remote file: %w", ctx.Err())
5151
}
52-
return nil, errors.TaskfileFetchFailedError{URI: u.String()}
52+
return nil, errors.TaskfileFetchFailedError{URI: u.Redacted()}
5353
}
5454
defer resp.Body.Close()
5555

@@ -61,7 +61,7 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) {
6161
if resp.StatusCode == http.StatusOK && slices.ContainsFunc(allowedContentTypes, func(s string) bool {
6262
return strings.Contains(contentType, s)
6363
}) {
64-
return u, nil
64+
return &u, nil
6565
}
6666

6767
// If the request was not successful, append the default Taskfile names to
@@ -78,7 +78,7 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) {
7878
// Try the alternative URL
7979
resp, err = http.DefaultClient.Do(req)
8080
if err != nil {
81-
return nil, errors.TaskfileFetchFailedError{URI: u.String()}
81+
return nil, errors.TaskfileFetchFailedError{URI: u.Redacted()}
8282
}
8383
defer resp.Body.Close()
8484

@@ -88,5 +88,5 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) {
8888
}
8989
}
9090

91-
return nil, errors.TaskfileNotFoundError{URI: u.String(), Walk: false}
91+
return nil, errors.TaskfileNotFoundError{URI: u.Redacted(), Walk: false}
9292
}

0 commit comments

Comments
 (0)