[INT-718] Add TargetNotFoundError for targeted scan targets missing from the source (#5123)

* Add TargetNotFoundError for targeted scan targets missing from the source * Check download response status in exact-path retry
2026-07-10 15:17:52 -07:00
parent 09c9107861
commit 8feb77b20a
4 changed files with 247 additions and 1 deletions
@@ -74,3 +74,22 @@ func (t TargetedScanError) Error() string {
 func (t TargetedScanError) Unwrap() error {
 	return t.Err
 }
+
+// TargetNotFoundError indicates a targeted scan failed because the requested
+// target is definitively absent from the source (e.g. the file, commit, or
+// message it pointed to was deleted), as opposed to inaccessible due to
+// authorization or transient failures. Consumers can treat this as terminal:
+// retrying cannot succeed until a fresh scan updates the stored location.
+type TargetNotFoundError struct {
+	Err error
+}
+
+var _ error = (*TargetNotFoundError)(nil)
+
+func (t TargetNotFoundError) Error() string {
+	return t.Err.Error()
+}
+
+func (t TargetNotFoundError) Unwrap() error {
+	return t.Err
+}
@@ -159,3 +159,19 @@ func TestScanErrorsString(t *testing.T) {
 		t.Errorf("got %q, want %q", got, want)
 	}
 }
+
+func TestTargetNotFoundErrorUnwrapsThroughJobChain(t *testing.T) {
+	inner := &TargetNotFoundError{Err: fmt.Errorf("could not download file for scan: 404")}
+	// The chain a targeted scan failure travels before a consumer sees it:
+	// scanTargets wraps in TargetedScanError, the source manager wraps in
+	// Fatal, and JobProgressMetrics.FatalErrors joins the results.
+	chain := errors.Join(Fatal{&TargetedScanError{Err: inner, SecretID: 42}})
+
+	var tnf *TargetNotFoundError
+	if !errors.As(chain, &tnf) {
+		t.Fatal("TargetNotFoundError not found in wrapped chain")
+	}
+	if tnf.Error() != inner.Error() {
+		t.Fatalf("unexpected message: %q", tnf.Error())
+	}
+}
@@ -2214,7 +2214,30 @@ func (s *Source) scanTarget(ctx context.Context, target sources.ChunkingTarget,
 		defer func() { _ = resp.Body.Close() }()
 	}
 	if err != nil {
-		return fmt.Errorf("could not download file for scan: %w", err)
+		// DownloadContents locates the file by listing its parent directory,
+		// and the contents API caps listings at 1000 entries, so it can fail
+		// for files that exist. Retry with an exact-path lookup, which also
+		// serves as the existence probe when it fails.
+		retryCloser, retryResp, retryErr := s.downloadContentsByPath(
+			ctx, apiClient, segments[1], segments[2], meta.GetFile(), meta.GetCommit())
+		if retryResp != nil && retryResp.Response != nil && retryResp.Body != nil {
+			defer func() { _ = retryResp.Body.Close() }()
+		}
+		if retryErr != nil {
+			wrapped := fmt.Errorf("could not download file for scan: %w", retryErr)
+			// The exact-path lookup 404ing is authoritative for the path, but
+			// GitHub also returns 404 for existing resources the credentials
+			// cannot see, so only classify the target as gone when the
+			// repository itself is still reachable with the same client.
+			if retryResp != nil && retryResp.Response != nil &&
+				retryResp.StatusCode == http.StatusNotFound &&
+				s.repoReachable(ctx, apiClient, segments[1], segments[2]) {
+				return &sources.TargetNotFoundError{Err: wrapped}
+			}
+			return wrapped
+		}
+		fileCtx := context.WithValues(ctx, "path", meta.GetFile())
+		return handlers.HandleFile(fileCtx, retryCloser, &chunkSkel, reporter)
 	}
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("unexpected HTTP response status when trying to download file for scan: %v", resp.Status)
@@ -2224,6 +2247,44 @@ func (s *Source) scanTarget(ctx context.Context, target sources.ChunkingTarget,
 	return handlers.HandleFile(fileCtx, readCloser, &chunkSkel, reporter)
 }

+// downloadContentsByPath fetches a file the same way DownloadContents does,
+// but resolves it with an exact-path contents lookup instead of searching a
+// directory listing, so it is immune to the 1000-entry listing cap. On
+// failure the returned response is always the exact-path lookup's, whose 404
+// is an authoritative statement about the path at that ref; a failure of the
+// download itself must not be mistaken for the target being gone, because
+// the lookup just proved it exists.
+func (s *Source) downloadContentsByPath(ctx context.Context, apiClient *github.Client, owner, repo, filePath, ref string) (io.ReadCloser, *github.Response, error) {
+	fileContent, _, resp, err := apiClient.Repositories.GetContents(
+		ctx, owner, repo, filePath, &github.RepositoryContentGetOptions{Ref: ref})
+	if err != nil {
+		return nil, resp, err
+	}
+	if fileContent == nil || fileContent.GetDownloadURL() == "" {
+		return nil, resp, fmt.Errorf("no download link found for %s", filePath)
+	}
+	dlReq, err := http.NewRequestWithContext(ctx, http.MethodGet, fileContent.GetDownloadURL(), nil)
+	if err != nil {
+		return nil, resp, err
+	}
+	dlResp, err := apiClient.Client().Do(dlReq)
+	if err != nil {
+		return nil, resp, err
+	}
+	if dlResp.StatusCode != http.StatusOK {
+		_ = dlResp.Body.Close()
+		return nil, resp, fmt.Errorf("unexpected HTTP response status when trying to download file for scan: %v", dlResp.Status)
+	}
+	return dlResp.Body, &github.Response{Response: dlResp}, nil
+}
+
+// repoReachable reports whether the repository is still visible with the
+// same client.
+func (s *Source) repoReachable(ctx context.Context, apiClient *github.Client, owner, repo string) bool {
+	_, resp, err := apiClient.Repositories.Get(ctx, owner, repo)
+	return err == nil && resp != nil && resp.StatusCode == http.StatusOK
+}
+
 func (s *Source) scanWikiTarget(ctx context.Context, wikiURL string, meta *source_metadatapb.Github, chunkSkel *sources.Chunk, reporter sources.ChunkReporter) error {
 	if meta.GetCommit() == "" {
 		return fmt.Errorf("wiki target has no commit")
@@ -0,0 +1,150 @@
+package github
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/google/go-github/v67/github"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/trufflesecurity/trufflehog/v3/pkg/context"
+)
+
+func testClientForServer(t *testing.T, serverURL string) *github.Client {
+	t.Helper()
+	client := github.NewClient(nil)
+	base, err := url.Parse(serverURL + "/")
+	assert.NoError(t, err)
+	client.BaseURL = base
+	return client
+}
+
+// downloadByPathServer answers the exact-path contents lookup, the raw
+// download it points at, and the repository lookup repoReachable makes.
+func downloadByPathServer(t *testing.T, fileStatus, rawStatus, repoStatus int) *httptest.Server {
+	t.Helper()
+	var server *httptest.Server
+	server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/repos/o/r/contents/dir/f.txt":
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(fileStatus)
+			if fileStatus == http.StatusOK {
+				_, _ = fmt.Fprintf(w, `{"type":"file","name":"f.txt","path":"dir/f.txt","download_url":"%s/raw/dir/f.txt"}`, server.URL)
+				return
+			}
+			_, _ = w.Write([]byte(`{"message":"Not Found"}`))
+		case "/raw/dir/f.txt":
+			w.WriteHeader(rawStatus)
+			if rawStatus == http.StatusOK {
+				_, _ = w.Write([]byte("file body"))
+				return
+			}
+			_, _ = w.Write([]byte("raw host error page"))
+		case "/repos/o/r":
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(repoStatus)
+			if repoStatus == http.StatusOK {
+				_, _ = w.Write([]byte(`{"id":1,"name":"r","full_name":"o/r"}`))
+				return
+			}
+			_, _ = w.Write([]byte(`{"message":"Not Found"}`))
+		default:
+			t.Errorf("unexpected request: %s", r.URL.Path)
+			w.WriteHeader(http.StatusInternalServerError)
+		}
+	}))
+	return server
+}
+
+func TestDownloadContentsByPathFetchesFileBody(t *testing.T) {
+	server := downloadByPathServer(t, http.StatusOK, http.StatusOK, http.StatusOK)
+	defer server.Close()
+
+	s := &Source{}
+	client := testClientForServer(t, server.URL)
+	rc, resp, err := s.downloadContentsByPath(context.Background(), client, "o", "r", "dir/f.txt", "abc123")
+	assert.NoError(t, err)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	body, err := io.ReadAll(rc)
+	assert.NoError(t, err)
+	assert.NoError(t, rc.Close())
+	assert.Equal(t, "file body", string(body))
+}
+
+func TestDownloadContentsByPathReturns404Response(t *testing.T) {
+	server := downloadByPathServer(t, http.StatusNotFound, http.StatusOK, http.StatusOK)
+	defer server.Close()
+
+	s := &Source{}
+	client := testClientForServer(t, server.URL)
+	rc, resp, err := s.downloadContentsByPath(context.Background(), client, "o", "r", "dir/f.txt", "abc123")
+	assert.Error(t, err)
+	assert.Nil(t, rc)
+	// The 404 is the exact-path lookup's, which the caller combines with
+	// repoReachable to classify the target as gone.
+	assert.NotNil(t, resp)
+	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
+}
+
+func TestDownloadContentsByPathNon404Failure(t *testing.T) {
+	server := downloadByPathServer(t, http.StatusForbidden, http.StatusOK, http.StatusOK)
+	defer server.Close()
+
+	s := &Source{}
+	client := testClientForServer(t, server.URL)
+	_, resp, err := s.downloadContentsByPath(context.Background(), client, "o", "r", "dir/f.txt", "abc123")
+	assert.Error(t, err)
+	assert.NotNil(t, resp)
+	assert.Equal(t, http.StatusForbidden, resp.StatusCode)
+}
+
+func TestRepoReachable(t *testing.T) {
+	server := downloadByPathServer(t, http.StatusNotFound, http.StatusOK, http.StatusOK)
+	defer server.Close()
+
+	s := &Source{}
+	client := testClientForServer(t, server.URL)
+	assert.True(t, s.repoReachable(context.Background(), client, "o", "r"))
+}
+
+func TestRepoNotReachable(t *testing.T) {
+	server := downloadByPathServer(t, http.StatusNotFound, http.StatusOK, http.StatusNotFound)
+	defer server.Close()
+
+	s := &Source{}
+	client := testClientForServer(t, server.URL)
+	// The repo 404s too: could be lost access rather than deletion, so the
+	// caller must not classify the target as gone.
+	assert.False(t, s.repoReachable(context.Background(), client, "o", "r"))
+}
+
+func TestRepoReachableServerDown(t *testing.T) {
+	server := downloadByPathServer(t, http.StatusNotFound, http.StatusOK, http.StatusOK)
+	server.Close()
+
+	s := &Source{}
+	client := testClientForServer(t, server.URL)
+	assert.False(t, s.repoReachable(context.Background(), client, "o", "r"))
+}
+
+func TestDownloadContentsByPathNon200Download(t *testing.T) {
+	server := downloadByPathServer(t, http.StatusOK, http.StatusInternalServerError, http.StatusOK)
+	defer server.Close()
+
+	s := &Source{}
+	client := testClientForServer(t, server.URL)
+	rc, resp, err := s.downloadContentsByPath(context.Background(), client, "o", "r", "dir/f.txt", "abc123")
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "unexpected HTTP response status")
+	assert.Nil(t, rc)
+	// The returned response is the exact-path lookup's (200), not the failed
+	// download's, so the caller cannot mistake a download failure for the
+	// target being gone.
+	assert.NotNil(t, resp)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+}