stdlib: remove query args and fragments from redirect URLs (#2598)

This will reduce the number of random redirect-to-oauth items, which
provide no value to the blast-radius. The trade-off is that we now might
be missing some valid redirect chains that would require the args to
pass through.

GitOrigin-RevId: c897427fe3cdbef7dfcac3647c0e39c8c52eb6f0

Author: DavidS-ovm

stdlib-source/adapters/http.go

@@ -7,13 +7,15 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/url"
 	"runtime"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/overmindtech/cli/sdp-go"
 	"github.com/overmindtech/cli/sdpcache"
+	"google.golang.org/protobuf/types/known/structpb"
 )
 
 const USER_AGENT_VERSION = "0.1"
@@ -271,19 +273,38 @@ func (s *HTTPAdapter) Get(ctx context.Context, scope string, query string, ignor
 	// Detect redirect and add a linked item for the redirect target
 	if res.StatusCode >= 300 && res.StatusCode < 400 {
 		if loc := res.Header.Get("Location"); loc != "" {
-			item.LinkedItemQueries = append(item.LinkedItemQueries, &sdp.LinkedItemQuery{
-				Query: &sdp.Query{
-					Type:   "http",
-					Method: sdp.QueryMethod_GET,
-					Query:  loc,
-					Scope:  scope,
-				},
-				BlastPropagation: &sdp.BlastPropagation{
-					// Redirects are tightly coupled
-					In:  true,
-					Out: true,
+			item.Attributes.AttrStruct.Fields["location"] = &structpb.Value{
+				Kind: &structpb.Value_StringValue{
+					StringValue: loc,
 				},
-			})
+			}
+			locU, err := url.Parse(loc)
+			if err != nil {
+				item.Attributes.AttrStruct.Fields["location-error"] = &structpb.Value{
+					Kind: &structpb.Value_StringValue{
+						StringValue: err.Error(),
+					},
+				}
+			} else {
+				// Don't include query string and fragment in the linked item.
+				// This leads to too many items, like auth redirect errors, that
+				// do not provide value.
+				locU.Fragment = ""
+				locU.RawQuery = ""
+				item.LinkedItemQueries = append(item.LinkedItemQueries, &sdp.LinkedItemQuery{
+					Query: &sdp.Query{
+						Type:   "http",
+						Method: sdp.QueryMethod_GET,
+						Query:  locU.String(),
+						Scope:  scope,
+					},
+					BlastPropagation: &sdp.BlastPropagation{
+						// Redirects are tightly coupled
+						In:  true,
+						Out: true,
+					},
+				})
+			}
 		}
 	}
 

stdlib-source/adapters/http_test.go

@@ -47,7 +47,7 @@ func NewTestServer() (*TestHTTPServer, error) {
 	}))
 
 	sm.Handle("/301", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Add("Location", "https://www.google.com")
+		w.Header().Add("Location", "https://www.google.com?foo=bar#baz")
 		w.WriteHeader(http.StatusMovedPermanently)
 	}))
 
@@ -232,8 +232,18 @@ func TestHTTPGet(t *testing.T) {
 			t.Errorf("expected status to be 301, got: %v", status)
 		}
 
-		if len(item.GetLinkedItemQueries()) == 0 {
-			t.Error("expected a linked item to redirected location, got none")
+		liqs := item.GetLinkedItemQueries()
+		if len(liqs) != 3 {
+			t.Errorf("expected linked items for redirected location, ip, and dns, got %v: %v", len(liqs), liqs)
+		}
+		for l := range liqs {
+			// Look for the linked item with the http query to the redirect
+			// location, check that the query and fragment have been stripped.
+			if liqs[l].GetQuery().GetType() == "http" {
+				if liqs[l].GetQuery().GetQuery() != "https://www.google.com" {
+					t.Errorf("expected linked item query to be https://www.google.com, got %v", liqs[l].GetQuery().GetQuery())
+				}
+			}
 		}
 
 		discovery.TestValidateItem(t, item)