Keep committed release note reads offline
This commit is contained in:
@@ -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;
|
for those child commands. Missing local objects produce inspection errors;
|
||||||
source preparation retains its separate network-capable operations. See Git's
|
source preparation retains its separate network-capable operations. See Git's
|
||||||
[lazy-fetch controls](https://git-scm.com/docs/git) for the command mechanism.
|
[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
|
Public validation errors convert their diagnostic fields to the internal
|
||||||
failure model for shared formatting. Converted failures retain their original
|
failure model for shared formatting. Converted failures retain their original
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# Remediation: module release implementation review
|
# 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 initial review examined `90559fe`, compared with planning baseline `dbfae9e`.
|
||||||
The follow-up repository audit examined `6b0b8ce`, including those corrections.
|
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)
|
## 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
|
**Evidence:** [ReadCommitFile](../../internal/git/source.go), used by candidate
|
||||||
capture and delivery for release notes, executes `git show` through the ordinary
|
capture and delivery for release notes, executes `git show` through the ordinary
|
||||||
|
|||||||
@@ -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, "\\") {
|
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)
|
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 {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("read committed file %q: %w", name, err)
|
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
|
// CommittedPaths resolves individual paths from captured objects. It requests
|
||||||
|
|||||||
@@ -88,6 +88,11 @@ func TestCandidateObjectReadsStayLocalInPartialClones(t *testing.T) {
|
|||||||
if _, err := ReadTree(t.Context(), repository, fixture.commit); err == nil {
|
if _, err := ReadTree(t.Context(), repository, fixture.commit); err == nil {
|
||||||
t.Fatal("tree inspection succeeded with unavailable objects")
|
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)
|
_, err := ReadBlob(t.Context(), repository, blob)
|
||||||
var exitError *exec.ExitError
|
var exitError *exec.ExitError
|
||||||
if !errors.As(err, &exitError) {
|
if !errors.As(err, &exitError) {
|
||||||
|
|||||||
87
source_inspection_test.go
Normal file
87
source_inspection_test.go
Normal file
@@ -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")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user