chore(remote): trim the remote.auth comments

internal/flags/flags.go

@@ -166,8 +166,7 @@ func init() {
 	pflag.StringVar(&CACert, "cacert", getConfig(config, "REMOTE_CACERT", func() *string { return config.Remote.CACert }, ""), "Path to a custom CA certificate for HTTPS connections.")
 	pflag.StringVar(&Cert, "cert", getConfig(config, "REMOTE_CERT", func() *string { return config.Remote.Cert }, ""), "Path to a client certificate for HTTPS connections.")
 	pflag.StringVar(&CertKey, "cert-key", getConfig(config, "REMOTE_CERT_KEY", func() *string { return config.Remote.CertKey }, ""), "Path to a client certificate key for HTTPS connections.")
-	// Configurable through the configuration file only: a token given on the
+	// No flag: a token on the command line is visible to any process listing it.
-	// command line would be visible to any process listing it.
 	RemoteAuth = remoteAuth(config)
 
 	// Gentle force experiment will override the force flag and add a new force-all flag
@@ -316,9 +315,8 @@ func (o *flagsOption) ApplyToExecutor(e *task.Executor) {
 	)
 }
 
-// remoteAuth flattens the configured authentication entries into a lookup by
+// remoteAuth flattens the configured entries into a lookup by host, the last
-// host. A host declared twice in the same file keeps its last entry, which is
+// entry winning as it does when configuration files are merged.
-// the rule the configuration files themselves follow when they are merged.
 func remoteAuth(config *taskrcast.TaskRC) map[string]map[string]string {
 	if config == nil || len(config.Remote.Auth) == 0 {
 		return nil

taskfile/node_base.go

@@ -77,8 +77,7 @@ func WithCertKey(certKey string) NodeOption {
 	}
 }
 
-// WithAuthHeaders sets the HTTP headers to send when the node's host matches
+// WithAuthHeaders sets the HTTP headers to send, keyed by host.
-// one of the configured ones.
 func WithAuthHeaders(authHeaders HostHeaders) NodeOption {
 	return func(node *baseNode) {
 		node.authHeaders = authHeaders

taskfile/node_http_auth.go

@@ -11,11 +11,9 @@ import (
 )
 
 // HostHeaders maps a host to the HTTP headers to send when fetching a remote
-// Taskfile from it. Values may reference environment variables using the
+// Taskfile from it. Values may reference environment variables.
-// `${VAR}` or `$VAR` syntax.
 type HostHeaders map[string]map[string]string
 
-// authTransport adds the configured headers to every request made to host.
 type authTransport struct {
 	base    http.RoundTripper
 	host    string
@@ -23,15 +21,11 @@ type authTransport struct {
 }
 
 func (t *authTransport) RoundTrip(req *http.Request) (*http.Response, error) {
-	// The headers are scoped to a single host. Checking here rather than once
+	// Re-checked per request: a redirect goes through this same transport, and
-	// at build time is what keeps a redirect from carrying the credentials
+	// Go only strips Authorization, WWW-Authenticate and Cookie on its own.
-	// somewhere else: the client sends the redirected request through this same
-	// transport, and Go only strips Authorization, WWW-Authenticate and Cookie
-	// on its own.
 	if !hostMatches(t.host, req.URL.Host) {
 		return t.base.RoundTrip(req)
 	}
-	// A RoundTripper must not modify the request it is given.
 	req = req.Clone(req.Context())
 	for name, value := range t.headers {
 		req.Header.Set(name, value)
@@ -39,10 +33,8 @@ func (t *authTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	return t.base.RoundTrip(req)
 }
 
-// authenticatedClient returns the node's client, wrapped so that it sends the
+// authenticatedClient resolves the headers on each read, not when the node is
-// configured headers. The environment variables the headers reference are read
+// built, so that a run served from the cache needs no credentials.
-// here rather than when the node is built, so that a run served from the cache
-// does not require credentials it will never send.
 func (node *HTTPNode) authenticatedClient() (*http.Client, error) {
 	headers, err := resolveAuthHeaders(node.authHeaders, node.url.Host)
 	if err != nil {
@@ -54,8 +46,7 @@ func (node *HTTPNode) authenticatedClient() (*http.Client, error) {
 	return withAuthHeaders(node.client, node.url.Host, headers), nil
 }
 
-// withAuthHeaders returns a copy of client that sends headers to host. The
+// withAuthHeaders copies rather than mutates: buildHTTPClient returns the
-// client is copied rather than mutated because buildHTTPClient returns the
 // shared http.DefaultClient when no TLS option is set.
 func withAuthHeaders(client *http.Client, host string, headers map[string]string) *http.Client {
 	authenticated := *client
@@ -67,9 +58,8 @@ func withAuthHeaders(client *http.Client, host string, headers map[string]string
 	return &authenticated
 }
 
-// resolveAuthHeaders returns the headers configured for host, with their
+// resolveAuthHeaders returns the expanded headers configured for host, or nil
-// environment variable references expanded. It returns nil when no entry
+// when no entry matches.
-// matches, leaving the request unauthenticated.
 func resolveAuthHeaders(hostHeaders HostHeaders, host string) (map[string]string, error) {
 	var headers map[string]string
 	for pattern, patternHeaders := range hostHeaders {
@@ -96,10 +86,9 @@ func resolveAuthHeaders(hostHeaders HostHeaders, host string) (map[string]string
 	return resolved, nil
 }
 
-// expandEnv replaces ${VAR} and $VAR references with the value of the
+// expandEnv replaces ${VAR} and $VAR references; `$$` is a literal dollar
-// environment variable. An undefined variable is an error rather than an empty
+// sign. An undefined variable is an error, not an empty header that would only
-// header, which would only surface later as an opaque 401. A literal dollar
+// surface as an opaque 401.
-// sign is written `$$`.
 func expandEnv(value string) (string, error) {
 	var missing []string
 	expanded := os.Expand(value, func(name string) string {
@@ -119,9 +108,8 @@ func expandEnv(value string) (string, error) {
 	return expanded, nil
 }
 
-// validateHeaderName rejects names that http.Header.Set would silently accept
+// validateHeaderName reports the offending header by name, where the transport
-// but the transport would later refuse, so that the error names the offending
+// would only refuse the request.
-// header instead of the request.
 func validateHeaderName(name string) error {
 	if name == "" {
 		return fmt.Errorf("header name cannot be empty")
@@ -134,8 +122,7 @@ func validateHeaderName(name string) error {
 	return nil
 }
 
-// hostMatches reports whether a host matches a configured pattern. The
+// hostMatches compares exactly, port included, as trusted hosts do.
-// comparison is exact and includes the port, as it does for trusted hosts.
 func hostMatches(pattern, host string) bool {
 	return pattern == host
 }

taskfile/node_http_auth_test.go

@@ -128,9 +128,8 @@ func TestWithAuthHeadersDoesNotMutateTheDefaultClient(t *testing.T) {
 	assert.IsType(t, &authTransport{}, client.Transport)
 }
 
-// TestHTTPNodeAuthHeaders covers the whole download: RemoteExists probes the
+// Both requests must carry the headers: RemoteExists probes with HEAD before
-// URL with a HEAD request before ReadContext issues the GET, and both must
+// ReadContext issues the GET.
-// carry the headers.
 func TestHTTPNodeAuthHeaders(t *testing.T) { //nolint:paralleltest // t.Setenv cannot be used in parallel tests
 	var methods []string
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -158,8 +157,7 @@ func TestHTTPNodeAuthHeaders(t *testing.T) { //nolint:paralleltest // t.Setenv c
 	assert.Equal(t, []string{"HEAD", "GET"}, methods)
 }
 
-// TestHTTPNodeAuthHeadersNotSentOnRedirect guards the credentials against a
+// A server bouncing the request must not get the credentials forwarded to it.
-// server that bounces the request to a host they were never meant for.
 func TestHTTPNodeAuthHeadersNotSentOnRedirect(t *testing.T) {
 	t.Parallel()
 
@@ -191,9 +189,8 @@ func TestHTTPNodeAuthHeadersNotSentOnRedirect(t *testing.T) {
 	}
 }
 
-// TestHTTPNodeAuthHeadersResolvedLazily makes sure a node can be built without
+// A node must build without the credentials it would need to download, so that
-// the credentials it would need to download: a run served from the cache, or an
+// cached and offline runs do not require them.
-// offline one, never sends them.
 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{

taskrc/ast/taskrc.go

@@ -83,10 +83,9 @@ func (t *TaskRC) Merge(other *TaskRC) {
 	t.TempDir = cmp.Or(other.TempDir, t.TempDir)
 }
 
-// mergeAuth unions two lists of [RemoteAuth] by host. An entry from other
+// mergeAuth unions both lists by host. An entry from other replaces the one
-// replaces the entry for the same host as a whole, so that a closer
+// for the same host as a whole, so a closer file can drop a header rather than
-// configuration file can redefine the headers of a host without inheriting the
+// inherit it.
-// ones it chose to drop.
 func mergeAuth(base, other []RemoteAuth) []RemoteAuth {
 	if len(other) == 0 {
 		return base