mirror of
https://github.com/go-task/task.git
synced 2026-09-01 19:50:16 +02:00
fix(remote): report a 401 instead of a missing Taskfile
RemoteExists treated every non-200 as an absent file, so a server refusing the credentials ended up as "No Taskfile found", sending the user to check the URL rather than the token. A 401 now stops the search and reports the status code; the default names need the same credentials, so trying them would only add rejected requests. A 403 is left alone: it is also what a server without directory listing answers for a readable directory. That message being correct, the expansion no longer needs to refuse an undefined variable: os.ExpandEnv is inlined and expandEnv is gone. The `$$` escape goes with it, so a literal value can no longer hold a `$` followed by a name; a secret carried in an environment variable is unaffected, as os.Expand never rescans what it substituted. Header names are validated with httpguts.ValidHeaderFieldName, the table net/http itself uses, rather than a denylist that let X-Foo(bar) through. golang.org/x/net was already in the module graph, so tidy only moves it to the direct block. Finally, node_http_auth.go becomes http_auth.go: the node_ prefix is for files defining a Node type, and this one holds the auth concern of HTTPNode plus hostMatches, which reader.go uses for trusted hosts.
This commit is contained in:
@@ -10,6 +10,13 @@
|
||||
where it would leak into error messages and the confirmation prompt (#2329 by
|
||||
@vmaerten).
|
||||
|
||||
### 🐛 Fixes
|
||||
|
||||
- Fixed a remote Taskfile whose server refuses the credentials being reported as
|
||||
a missing Taskfile. A `401` now stops the search and reports the status code,
|
||||
instead of retrying every default Taskfile name and concluding that no
|
||||
Taskfile exists (#2329 by @vmaerten).
|
||||
|
||||
### 📦 Package API
|
||||
|
||||
- Bumped the minimum Go version to 1.26. Task follows Go's two-latest support
|
||||
|
||||
2
go.mod
2
go.mod
@@ -28,6 +28,7 @@ require (
|
||||
github.com/stretchr/testify v1.11.1
|
||||
github.com/zeebo/xxh3 v1.1.0
|
||||
go.yaml.in/yaml/v3 v3.0.4
|
||||
golang.org/x/net v0.58.0
|
||||
golang.org/x/sync v0.22.0
|
||||
golang.org/x/term v0.45.0
|
||||
mvdan.cc/sh/moreinterp v0.0.0-20260817215856-d6550df7ed8d
|
||||
@@ -121,7 +122,6 @@ require (
|
||||
go.opentelemetry.io/otel/trace v1.45.0 // indirect
|
||||
golang.org/x/crypto v0.55.0 // indirect
|
||||
golang.org/x/exp v0.0.0-20260718201538-764159d718ef // indirect
|
||||
golang.org/x/net v0.58.0 // indirect
|
||||
golang.org/x/oauth2 v0.36.0 // indirect
|
||||
golang.org/x/sys v0.47.0 // indirect
|
||||
golang.org/x/text v0.41.0 // indirect
|
||||
|
||||
@@ -7,7 +7,8 @@ import (
|
||||
"net/http"
|
||||
"os"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/net/http/httpguts"
|
||||
)
|
||||
|
||||
// HostHeaders maps a host to the HTTP headers to send when fetching a remote
|
||||
@@ -33,8 +34,8 @@ func (t *authTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
return t.base.RoundTrip(req)
|
||||
}
|
||||
|
||||
// authenticatedClient resolves the headers on each read, not when the node is
|
||||
// built, so that a run served from the cache needs no credentials.
|
||||
// authenticatedClient resolves on each read, not at build time, so a cached
|
||||
// run needs no credentials.
|
||||
func (node *HTTPNode) authenticatedClient() (*http.Client, error) {
|
||||
headers, err := resolveAuthHeaders(node.authHeaders, node.url.Host)
|
||||
if err != nil {
|
||||
@@ -58,8 +59,7 @@ func withAuthHeaders(client *http.Client, host string, headers map[string]string
|
||||
return &authenticated
|
||||
}
|
||||
|
||||
// resolveAuthHeaders returns the expanded headers configured for host, or nil
|
||||
// when no entry matches.
|
||||
// resolveAuthHeaders returns the expanded headers for host, or nil if none.
|
||||
func resolveAuthHeaders(hostHeaders HostHeaders, host string) (map[string]string, error) {
|
||||
var headers map[string]string
|
||||
for pattern, patternHeaders := range hostHeaders {
|
||||
@@ -77,47 +77,16 @@ func resolveAuthHeaders(hostHeaders HostHeaders, host string) (map[string]string
|
||||
if err := validateHeaderName(name); err != nil {
|
||||
return nil, fmt.Errorf(`remote auth for host %q: %w`, host, err)
|
||||
}
|
||||
value, err := expandEnv(headers[name])
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf(`remote auth for host %q: header %q: %w`, host, name, err)
|
||||
}
|
||||
resolved[name] = value
|
||||
resolved[name] = os.ExpandEnv(headers[name])
|
||||
}
|
||||
return resolved, nil
|
||||
}
|
||||
|
||||
// expandEnv replaces ${VAR} and $VAR references; `$$` is a literal dollar
|
||||
// sign. An undefined variable is an error, not an empty header that would only
|
||||
// surface as an opaque 401.
|
||||
func expandEnv(value string) (string, error) {
|
||||
var missing []string
|
||||
expanded := os.Expand(value, func(name string) string {
|
||||
if name == "$" {
|
||||
return "$"
|
||||
}
|
||||
v, ok := os.LookupEnv(name)
|
||||
if !ok {
|
||||
missing = append(missing, name)
|
||||
return ""
|
||||
}
|
||||
return v
|
||||
})
|
||||
if len(missing) > 0 {
|
||||
return "", fmt.Errorf("environment variable $%s is not set", strings.Join(missing, ", $"))
|
||||
}
|
||||
return expanded, nil
|
||||
}
|
||||
|
||||
// validateHeaderName reports the offending header by name, where the transport
|
||||
// would only refuse the request.
|
||||
// validateHeaderName names the offending header; ReadContext discards the
|
||||
// transport's own error.
|
||||
func validateHeaderName(name string) error {
|
||||
if name == "" {
|
||||
return fmt.Errorf("header name cannot be empty")
|
||||
}
|
||||
if strings.ContainsFunc(name, func(r rune) bool {
|
||||
return r <= ' ' || r == ':' || r == 0x7f
|
||||
}) {
|
||||
return fmt.Errorf("header name %q contains invalid characters", name)
|
||||
if !httpguts.ValidHeaderFieldName(name) {
|
||||
return fmt.Errorf("invalid header name %q", name)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -55,22 +55,28 @@ func TestResolveAuthHeaders(t *testing.T) { //nolint:paralleltest // t.Setenv ca
|
||||
want: map[string]string{"Authorization": "Bearer s3cret"},
|
||||
},
|
||||
{
|
||||
name: "escaped dollar sign",
|
||||
hostHeaders: HostHeaders{"gitlab.com": {"PRIVATE-TOKEN": "lit$$eral"}},
|
||||
host: "gitlab.com",
|
||||
want: map[string]string{"PRIVATE-TOKEN": "lit$eral"},
|
||||
},
|
||||
{
|
||||
name: "undefined environment variable",
|
||||
name: "undefined environment variable expands to nothing",
|
||||
hostHeaders: HostHeaders{"gitlab.com": {"PRIVATE-TOKEN": "${TASK_TEST_UNSET}"}}, //nolint:gosec // an env var reference, not a credential
|
||||
host: "gitlab.com",
|
||||
wantErr: `remote auth for host "gitlab.com": header "PRIVATE-TOKEN": environment variable $TASK_TEST_UNSET is not set`,
|
||||
want: map[string]string{"PRIVATE-TOKEN": ""},
|
||||
},
|
||||
{
|
||||
name: "invalid header name",
|
||||
name: "header name with a space",
|
||||
hostHeaders: HostHeaders{"gitlab.com": {"PRIVATE TOKEN": "token"}},
|
||||
host: "gitlab.com",
|
||||
wantErr: `remote auth for host "gitlab.com": header name "PRIVATE TOKEN" contains invalid characters`,
|
||||
wantErr: `remote auth for host "gitlab.com": invalid header name "PRIVATE TOKEN"`,
|
||||
},
|
||||
{
|
||||
name: "header name outside the HTTP token grammar",
|
||||
hostHeaders: HostHeaders{"gitlab.com": {"X-Foo(bar)": "token"}},
|
||||
host: "gitlab.com",
|
||||
wantErr: `remote auth for host "gitlab.com": invalid header name "X-Foo(bar)"`,
|
||||
},
|
||||
{
|
||||
name: "empty header name",
|
||||
hostHeaders: HostHeaders{"gitlab.com": {"": "token"}},
|
||||
host: "gitlab.com",
|
||||
wantErr: `remote auth for host "gitlab.com": invalid header name ""`,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -194,18 +200,17 @@ func TestHTTPNodeAuthHeadersNotSentOnRedirect(t *testing.T) {
|
||||
func TestHTTPNodeAuthHeadersResolvedLazily(t *testing.T) { //nolint:paralleltest // t.Setenv cannot be used in parallel tests
|
||||
node, err := NewHTTPNode("https://gitlab.com/Taskfile.yml", "", false,
|
||||
WithAuthHeaders(HostHeaders{
|
||||
"gitlab.com": {"PRIVATE-TOKEN": "${TASK_TEST_UNSET}"}, //nolint:gosec // an env var reference, not a credential
|
||||
"gitlab.com": {"PRIVATE-TOKEN": "${TASK_TEST_LAZY}"}, //nolint:gosec // an env var reference, not a credential
|
||||
}),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = node.authenticatedClient()
|
||||
require.EqualError(t, err, `remote auth for host "gitlab.com": header "PRIVATE-TOKEN": environment variable $TASK_TEST_UNSET is not set`)
|
||||
// Defined only after the node was built: the value must still be picked up.
|
||||
t.Setenv("TASK_TEST_LAZY", "s3cret")
|
||||
|
||||
t.Setenv("TASK_TEST_UNSET", "s3cret")
|
||||
client, err := node.authenticatedClient()
|
||||
headers, err := resolveAuthHeaders(node.authHeaders, node.url.Host)
|
||||
require.NoError(t, err)
|
||||
assert.IsType(t, &authTransport{}, client.Transport)
|
||||
assert.Equal(t, map[string]string{"PRIVATE-TOKEN": "s3cret"}, headers)
|
||||
}
|
||||
|
||||
type roundTripperFunc func(*http.Request) (*http.Response, error)
|
||||
@@ -66,6 +66,13 @@ func RemoteExists(ctx context.Context, u url.URL, client *http.Client) (*url.URL
|
||||
return &u, nil
|
||||
}
|
||||
|
||||
// The default names need the same credentials, so trying them would only
|
||||
// add rejected requests. A 403 is left alone: it is also what a server
|
||||
// without directory listing answers for a readable directory.
|
||||
if resp.StatusCode == http.StatusUnauthorized {
|
||||
return nil, errors.TaskfileFetchFailedError{URI: u.Redacted(), HTTPStatusCode: resp.StatusCode}
|
||||
}
|
||||
|
||||
// If the request was not successful, append the default Taskfile names to
|
||||
// the URL and return the URL of the first successful request
|
||||
for _, taskfile := range DefaultTaskfiles {
|
||||
|
||||
84
taskfile/taskfile_test.go
Normal file
84
taskfile/taskfile_test.go
Normal file
@@ -0,0 +1,84 @@
|
||||
package taskfile
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/go-task/task/v3/errors"
|
||||
)
|
||||
|
||||
// alwaysStatus answers every request with the given status.
|
||||
func alwaysStatus(t *testing.T, status int) (*url.URL, *int) {
|
||||
t.Helper()
|
||||
var requests int
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
requests++
|
||||
w.WriteHeader(status)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
return mustParse(t, srv.URL), &requests
|
||||
}
|
||||
|
||||
func TestRemoteExistsUnauthorized(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
u, requests := alwaysStatus(t, http.StatusUnauthorized)
|
||||
_, err := RemoteExists(t.Context(), *u, http.DefaultClient)
|
||||
|
||||
var fetchErr errors.TaskfileFetchFailedError
|
||||
require.ErrorAs(t, err, &fetchErr)
|
||||
assert.Equal(t, http.StatusUnauthorized, fetchErr.HTTPStatusCode)
|
||||
assert.Equal(t, 1, *requests)
|
||||
}
|
||||
|
||||
// A 403 is ambiguous, so it keeps the existing behaviour.
|
||||
func TestRemoteExistsForbiddenEverywhere(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
u, requests := alwaysStatus(t, http.StatusForbidden)
|
||||
_, err := RemoteExists(t.Context(), *u, http.DefaultClient)
|
||||
|
||||
var notFoundErr errors.TaskfileNotFoundError
|
||||
assert.ErrorAs(t, err, ¬FoundErr)
|
||||
assert.Greater(t, *requests, 1)
|
||||
}
|
||||
|
||||
func TestRemoteExistsForbiddenDirectoryWithReadableTaskfile(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/Taskfile.yml" {
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "text/yaml")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
found, err := RemoteExists(t.Context(), *mustParse(t, srv.URL), http.DefaultClient)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "/Taskfile.yml", found.Path)
|
||||
}
|
||||
|
||||
func TestRemoteExistsNotFound(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
u, _ := alwaysStatus(t, http.StatusNotFound)
|
||||
_, err := RemoteExists(t.Context(), *u, http.DefaultClient)
|
||||
|
||||
var notFoundErr errors.TaskfileNotFoundError
|
||||
assert.ErrorAs(t, err, ¬FoundErr)
|
||||
}
|
||||
|
||||
func mustParse(t *testing.T, rawURL string) *url.URL {
|
||||
t.Helper()
|
||||
parsed, err := url.Parse(rawURL)
|
||||
require.NoError(t, err)
|
||||
return parsed
|
||||
}
|
||||
@@ -326,9 +326,10 @@ commit.
|
||||
Each entry applies to a single host, matched exactly and including the port if
|
||||
the URL has one — the same rule as
|
||||
[`remote.trusted-hosts`](#remote-trusted-hosts). Header values may reference
|
||||
environment variables with `${VAR}` or `$VAR`; write `$$` for a literal dollar
|
||||
sign. A variable is only read when Task actually contacts the host, and an
|
||||
undefined one is reported as an error instead of being sent as an empty header.
|
||||
environment variables with `${VAR}` or `$VAR`, read when Task contacts the host.
|
||||
An undefined variable expands to nothing, so the header is sent empty and the
|
||||
server rejects it — prefer an environment variable over a literal value, which
|
||||
cannot contain a `$` followed by a name.
|
||||
|
||||
The header your server expects depends on the service:
|
||||
|
||||
|
||||
Reference in New Issue
Block a user