diff --git a/docs/internal/validation.md b/docs/internal/validation.md index 81e4d45..ad3cf93 100644 --- a/docs/internal/validation.md +++ b/docs/internal/validation.md @@ -107,6 +107,7 @@ 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. +Candidate and delivery release-note reads use the same local object boundary. Public validation errors convert their diagnostic fields to the internal failure model for shared formatting. Converted failures retain their original diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 53b1564..46ad372 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Remediation: module release implementation review -Status: open; issues 1–13 are complete, and issues 14–15 require remediation. +Status: open; issues 1–14 are complete, and issue 15 requires remediation. The initial review examined `90559fe`, compared with planning baseline `dbfae9e`. The follow-up repository audit examined `6b0b8ce`, including those corrections. @@ -504,7 +504,16 @@ local path without timing thresholds or additional source guarantees. ## Issue 14: Keep committed release-note inspection within the local object boundary (P2) -Status: open. Category: incomplete earlier offline-inspection correction. +Status: complete. Category: incomplete earlier offline-inspection correction. + +**Completion:** Committed-file reads now use the shared local-only Git object +runner. Filtered-clone regressions cover direct reads plus public check and +delivery with an unavailable note, preserving the note path, Git cause, and +captured identity while making no hosted request, transport attempt, or object +database change. Both regressions exposed transport before the correction. +The Git/candidate suites and focused source, page-delivery, and explicit tagged +checkout preparation cases pass offline; ordinary note bytes and cancellation +handling retain their existing command boundary. **Evidence:** [ReadCommitFile](../../internal/git/source.go), used by candidate capture and delivery for release notes, executes `git show` through the ordinary diff --git a/internal/git/source.go b/internal/git/source.go index 9b297de..102c313 100644 --- a/internal/git/source.go +++ b/internal/git/source.go @@ -116,11 +116,11 @@ func ReadCommitFile(ctx context.Context, repositoryDir, commit, name string) ([] if !validObjectID(commit) || name == "" || path.IsAbs(name) || path.Clean(name) != name || strings.HasPrefix(name, "../") || name == ".." || strings.Contains(name, "\\") { return nil, fmt.Errorf("invalid committed file identity %q at %q", name, commit) } - content, err := run(ctx, repositoryDir, nil, "show", commit+":"+name) + content, err := readLocalObjects(ctx, repositoryDir, nil, "show", commit+":"+name) if err != nil { return nil, fmt.Errorf("read committed file %q: %w", name, err) } - return []byte(content), nil + return content, nil } // CommittedPaths resolves individual paths from captured objects. It requests diff --git a/internal/git/source_test.go b/internal/git/source_test.go index 0099c0d..0f71e95 100644 --- a/internal/git/source_test.go +++ b/internal/git/source_test.go @@ -88,6 +88,11 @@ func TestCandidateObjectReadsStayLocalInPartialClones(t *testing.T) { if _, err := ReadTree(t.Context(), repository, fixture.commit); err == nil { t.Fatal("tree inspection succeeded with unavailable objects") } + _, fileErr := ReadCommitFile(t.Context(), repository, fixture.commit, "source.txt") + var fileExitError *exec.ExitError + if !errors.As(fileErr, &fileExitError) { + t.Fatalf("committed file error %v lost its Git process cause", fileErr) + } _, err := ReadBlob(t.Context(), repository, blob) var exitError *exec.ExitError if !errors.As(err, &exitError) { diff --git a/source_inspection_test.go b/source_inspection_test.go new file mode 100644 index 0000000..2aca7f8 --- /dev/null +++ b/source_inspection_test.go @@ -0,0 +1,87 @@ +package releasekit_test + +import ( + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + releasekit "gitea.maximumdirect.net/eric/ReleaseKit" +) + +func TestSourceOperationsRejectUnavailableNoteWithoutTransport(t *testing.T) { + source := newPublishFixture(t) + repository := newFilteredSparseCheckout(t, source.work, "/go.mod", "/fixture_test.go") + commit := gitText(t, repository, "rev-parse", "HEAD") + noteObject := gitText(t, repository, "rev-parse", "HEAD:docs/releases/v1.2.3.md") + before := gitText(t, repository, "cat-file", "--batch-all-objects", "--batch-check=%(objectname)") + if strings.Contains(before, noteObject) { + t.Fatal("fixture already contains the release note blob") + } + buildGit(t, repository, "tag", "v1.2.3") + attempts := forbidInspectionTransport(t, repository) + checked, checkErr := releasekit.Check(t.Context(), releasekit.ReleaseRequest{ + RepositoryDir: repository, Product: "Sample App", Version: "v1.2.3", + }) + server, remote := newFacadeDeliveryServer(t, repository) + defer server.Close() + delivered, deliveryErr := releasekit.Deliver(t.Context(), releasekit.DeliveryRequest{ + RepositoryDir: repository, Product: "Sample App", Version: "v1.2.3", + Destination: facadeDeliveryDestination(server.URL), + }) + for _, err := range []error{checkErr, deliveryErr} { + var exitError *exec.ExitError + if !errors.As(err, &exitError) || !strings.Contains(err.Error(), "docs/releases/v1.2.3.md") { + t.Errorf("note inspection error = %v; want note path and Git cause", err) + } + } + if checked.Commit != commit || checked.Tag != "v1.2.3" || delivered.CandidateCommit != commit || delivered.Tag != checked.Tag || delivered.State != releasekit.DeliveryNotAttempted { + t.Fatalf("missing-note evidence: check=%#v, delivery=%#v", checked, delivered) + } + if remote.requests != 0 { + t.Fatalf("missing note caused %d hosted requests", remote.requests) + } + assertInspectionStayedLocal(t, repository, attempts, before) +} + +func newFilteredSparseCheckout(t *testing.T, source string, paths ...string) string { + t.Helper() + gitRun(t, source, "config", "uploadpack.allowFilter", "true") + repository := filepath.Join(t.TempDir(), "partial") + gitRun(t, source, "clone", "--filter=blob:none", "--no-checkout", "--no-tags", "file://"+filepath.ToSlash(source), repository) + gitRun(t, repository, "sparse-checkout", "init", "--no-cone") + gitRun(t, repository, append([]string{"sparse-checkout", "set"}, paths...)...) + gitRun(t, repository, "checkout", "main") + if status := gitText(t, repository, "status", "--porcelain"); status != "" { + t.Fatalf("sparse checkout is dirty: %s", status) + } + return repository +} + +func forbidInspectionTransport(t *testing.T, repository string) string { + t.Helper() + gitRun(t, repository, "config", "remote.origin.url", "ssh://releasekit-test.invalid/source") + directory := t.TempDir() + transport := filepath.Join(directory, "ssh") + attempts := filepath.Join(directory, "attempts") + 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") + return attempts +} + +func assertInspectionStayedLocal(t *testing.T, repository, attempts, before string) { + t.Helper() + if data, err := os.ReadFile(attempts); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("inspection invoked remote transport: %q, error = %v", data, err) + } + if after := gitText(t, repository, "cat-file", "--batch-all-objects", "--batch-check=%(objectname)"); after != before { + t.Fatal("inspection changed the local object database") + } +}