From 02050016c16aa558afd163bd97425ef4bb0f5f15 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 7 Oct 2026 20:18:13 +0000 Subject: [PATCH] Keep candidate object inspection offline --- docs/internal/validation.md | 5 ++++ docs/roadmap/remediation.md | 9 ++++++- internal/git/source.go | 49 ++++++++++++++++++++++++------------- internal/git/source_test.go | 45 ++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 18 deletions(-) diff --git a/docs/internal/validation.md b/docs/internal/validation.md index 118c9b8..fdaba14 100644 --- a/docs/internal/validation.md +++ b/docs/internal/validation.md @@ -16,6 +16,11 @@ candidate contents; the default Go-only path does not enumerate the tree. lazily, and resolves destinations without consulting the worktree or neighboring files. +Committed-tree metadata and blob reads disable lazy fetching and Git transports +for those child commands. Missing local objects produce inspection errors; +source preparation retains its separate network-capable operations. See Git's +[lazy-fetch controls](https://git-scm.com/docs/git) for the command mechanism. + ## Adding a shared check When an optional check is accepted for implementation: diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 627804c..317a810 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Shared validation foundation remediation -Status: Remediation in progress; findings 1–2 addressed, findings 3–6 pending. +Status: Remediation in progress; findings 1–3 addressed, findings 4–6 pending. Reviewed committed implementation `f53354b`, covering the eight foundation commits after `0c7129a`, against the [implementation plan](implementation.md), @@ -95,6 +95,13 @@ boundary classifications independently of neighboring filesystem contents. ## 3. P2: Committed-file inspection can fetch from a promisor remote +**Resolution:** Addressed. Candidate tree, size, and blob reads share a local +Git command helper that disables lazy fetching and transport access per child +process. Missing blobs report local unavailability, and Git process causes +remain wrapped. Partial-clone tests cover missing blobs and missing trees, +instrument transport attempts, and verify the object database stays unchanged. +`go test ./internal/git ./internal/validation` passed. + **Owner:** [Git source readers](../../internal/git/source.go), `ReadTree`, `populateBlobSizes`, and `ReadBlob`. diff --git a/internal/git/source.go b/internal/git/source.go index 00a4d45..ac698bd 100644 --- a/internal/git/source.go +++ b/internal/git/source.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io" "path" "strconv" "strings" @@ -95,11 +96,11 @@ func ReadTree(ctx context.Context, repositoryDir, commit string) ([]TreeEntry, e if !validObjectID(commit) { return nil, fmt.Errorf("invalid commit object ID %q", commit) } - output, err := run(ctx, repositoryDir, nil, "ls-tree", "-r", "-z", "--full-tree", commit) + output, err := readLocalObjects(ctx, repositoryDir, nil, "ls-tree", "-r", "-z", "--full-tree", commit) if err != nil { return nil, fmt.Errorf("enumerate committed tree %s: %w", commit, err) } - entries, err := parseTreeEntries([]byte(output)) + entries, err := parseTreeEntries(output) if err != nil { return nil, fmt.Errorf("parse committed tree %s: %w", commit, err) } @@ -115,11 +116,11 @@ func ReadBlob(ctx context.Context, repositoryDir, objectID string) ([]byte, erro if !validObjectID(objectID) { return nil, fmt.Errorf("invalid blob object ID %q", objectID) } - content, err := run(ctx, repositoryDir, nil, "cat-file", "blob", objectID) + content, err := readLocalObjects(ctx, repositoryDir, nil, "cat-file", "blob", objectID) if err != nil { return nil, fmt.Errorf("read blob %s: %w", objectID, err) } - return []byte(content), nil + return content, nil } func parseTreeEntries(output []byte) ([]TreeEntry, error) { @@ -163,28 +164,21 @@ func populateBlobSizes(ctx context.Context, repositoryDir string, entries []Tree if len(blobIndexes) == 0 { return nil } - var stdout, stderr bytes.Buffer - err := process.Run(ctx, process.Invocation{ - Args: []string{"git", "cat-file", "--batch-check=%(objectname) %(objecttype) %(objectsize)"}, - Dir: repositoryDir, - Stdin: strings.NewReader(input.String()), - Stdout: &stdout, - Stderr: &stderr, - }) + output, err := readLocalObjects(ctx, repositoryDir, strings.NewReader(input.String()), + "cat-file", "--batch-check=%(objectname) %(objecttype) %(objectsize)") if err != nil { - message := strings.TrimSpace(stderr.String()) - if message != "" { - return fmt.Errorf("git cat-file --batch-check: %s: %w", message, err) - } return err } - lines := strings.Split(strings.TrimSuffix(stdout.String(), "\n"), "\n") + lines := strings.Split(strings.TrimSuffix(string(output), "\n"), "\n") if len(lines) != len(blobIndexes) { return fmt.Errorf("git returned %d blob sizes for %d blobs", len(lines), len(blobIndexes)) } for index, line := range lines { fields := strings.Fields(line) entry := &entries[blobIndexes[index]] + if len(fields) == 2 && fields[0] == entry.ObjectID && fields[1] == "missing" { + return fmt.Errorf("committed blob %s is unavailable locally", entry.ObjectID) + } if len(fields) != 3 || fields[0] != entry.ObjectID || fields[1] != "blob" { return fmt.Errorf("unexpected blob size record %q", line) } @@ -197,3 +191,24 @@ func populateBlobSizes(ctx context.Context, repositoryDir string, entries []Tree } return nil } + +// readLocalObjects keeps source inspection separate from network preparation. +func readLocalObjects(ctx context.Context, repositoryDir string, input io.Reader, args ...string) ([]byte, error) { + var stdout, stderr bytes.Buffer + err := process.Run(ctx, process.Invocation{ + Args: append([]string{"git"}, args...), + Dir: repositoryDir, + // Disable transports as well for Git versions predating lazy-fetch control. + Env: map[string]string{"GIT_NO_LAZY_FETCH": "1", "GIT_ALLOW_PROTOCOL": ""}, + Stdin: input, + Stdout: &stdout, + Stderr: &stderr, + }) + if err != nil { + if message := strings.TrimSpace(stderr.String()); message != "" { + return nil, fmt.Errorf("read local Git objects: %s: %w", message, err) + } + return nil, err + } + return stdout.Bytes(), nil +} diff --git a/internal/git/source_test.go b/internal/git/source_test.go index 0f1e5fd..0d2fd46 100644 --- a/internal/git/source_test.go +++ b/internal/git/source_test.go @@ -2,6 +2,10 @@ package git import ( "context" + "errors" + "os" + "os/exec" + "path/filepath" "strings" "testing" ) @@ -28,6 +32,47 @@ func TestInspectTaggedSourceSupportsLightweightAndAnnotatedTags(t *testing.T) { } } +func TestCandidateObjectReadsStayLocalInPartialClones(t *testing.T) { + fixture := newRemoteFixture(t) + gitRun(t, fixture.work, "config", "uploadpack.allowFilter", "true") + blob := strings.TrimSpace(gitText(t, fixture.work, "rev-parse", "HEAD:source.txt")) + for _, filter := range []string{"blob:none", "tree:0"} { + t.Run(filter, func(t *testing.T) { + repository := filepath.Join(t.TempDir(), "partial") + gitRun(t, fixture.work, "clone", "--filter="+filter, "--no-checkout", "file://"+filepath.ToSlash(fixture.work), repository) + before := gitText(t, repository, "cat-file", "--batch-all-objects", "--batch-check=%(objectname)") + if strings.Contains(before, blob) { + t.Fatal("fixture contains the filtered blob") + } + gitRun(t, repository, "config", "remote.origin.url", "ssh://releasekit-test.invalid/source") + attempts := filepath.Join(t.TempDir(), "attempts") + transport := filepath.Join(t.TempDir(), "ssh") + if err := os.WriteFile(transport, []byte("#!/bin/sh\nprintf 'remote accessed\\n' >> \"$RELEASEKIT_GIT_ATTEMPTS\"\nexit 1\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("GIT_SSH_COMMAND", transport) + t.Setenv("RELEASEKIT_GIT_ATTEMPTS", attempts) + t.Setenv("GIT_NO_LAZY_FETCH", "0") + t.Setenv("GIT_ALLOW_PROTOCOL", "ssh") + if _, err := ReadTree(t.Context(), repository, fixture.commit); err == nil { + t.Fatal("tree inspection succeeded with unavailable objects") + } + _, err := ReadBlob(t.Context(), repository, blob) + var exitError *exec.ExitError + if !errors.As(err, &exitError) { + t.Fatalf("blob error %v lost its Git process cause", err) + } + if data, err := os.ReadFile(attempts); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("object inspection invoked remote transport: %q, %v", data, err) + } + after := gitText(t, repository, "cat-file", "--batch-all-objects", "--batch-check=%(objectname)") + if before != after { + t.Fatal("object inspection populated the object database") + } + }) + } +} + func TestInspectTaggedSourceReportsDirtyCheckoutAndReadCommitFile(t *testing.T) { fixture := newRemoteFixture(t) gitRun(t, fixture.work, "tag", "v1.2.3")