From d2b13290957a568831c48787ad160809612b6e4b Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 22:17:59 +0000 Subject: [PATCH] Check the exact publication record filename before writing --- docs/roadmap/remediation.md | 5 ++++ internal/publish/record.go | 23 +++++++++--------- internal/publish/record_test.go | 41 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 3a50c18..bb155e0 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -60,6 +60,11 @@ successful publication. ## R02: Validate the filename that will actually be written — P2 +**Completed:** Each generated filename is checked before that exact path is +created, including collision retries. Deterministic selective-ignore tests passed +and confirm that rejected writes leave the candidate clean and existing records +unchanged. + **Relevant code:** `internal/publish/record.go:68`, `record.go:85`, and `record.go:134`; the pre-push save in `internal/publish/publish.go`. diff --git a/internal/publish/record.go b/internal/publish/record.go index 0f545df..bdf486b 100644 --- a/internal/publish/record.go +++ b/internal/publish/record.go @@ -59,21 +59,16 @@ func SaveEvidence(repositoryDir, override string, evidence Evidence) (string, er } func saveEvidenceInDir(repositoryDir, root, override, recordDir string, evidence Evidence) (string, error) { + return saveEvidenceNamed(repositoryDir, root, override, recordDir, evidence, proposedRecordPath) +} + +func saveEvidenceNamed(repositoryDir, root, override, recordDir string, evidence Evidence, nextPath func(string) (string, error)) (string, error) { if evidence.RecordedAt == "" { evidence.RecordedAt = time.Now().UTC().Format(time.RFC3339Nano) } if err := os.MkdirAll(recordDir, 0o700); err != nil { return "", &RecoveryError{Kind: "record_write", cause: fmt.Errorf("create record directory %q: %w", recordDir, err)} } - if override != "" && root != "" { - path, pathErr := proposedRecordPath(recordDir) - if pathErr != nil { - return "", &RecoveryError{Kind: "record_write", cause: pathErr} - } - if err := rejectUnignoredWorktreePath(repositoryDir, root, path); err != nil { - return "", &RecoveryError{Kind: "record_write", cause: err} - } - } evidenceJSON, err := json.Marshal(evidence) if err != nil { return "", &RecoveryError{Kind: "record_write", cause: fmt.Errorf("encode publication evidence: %w", err)} @@ -83,11 +78,15 @@ func saveEvidenceInDir(repositoryDir, root, override, recordDir string, evidence return "", &RecoveryError{Kind: "record_write", cause: fmt.Errorf("encode publication evidence: %w", err)} } for attempt := 0; attempt < 5; attempt++ { - var nonce [16]byte - if _, err := rand.Read(nonce[:]); err != nil { + path, err := nextPath(recordDir) + if err != nil { return "", &RecoveryError{Kind: "record_write", cause: fmt.Errorf("name publication record: %w", err)} } - path := filepath.Join(recordDir, "attempt-"+hex.EncodeToString(nonce[:])+".json") + if override != "" && root != "" { + if err := rejectUnignoredWorktreePath(repositoryDir, root, path); err != nil { + return "", &RecoveryError{Kind: "record_write", cause: err} + } + } file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) if errors.Is(err, os.ErrExist) { continue diff --git a/internal/publish/record_test.go b/internal/publish/record_test.go index 2c3d02d..9620114 100644 --- a/internal/publish/record_test.go +++ b/internal/publish/record_test.go @@ -157,6 +157,47 @@ func TestSaveEvidenceRejectsUnignoredOverrideInsideWorktree(t *testing.T) { } } +func TestSaveEvidenceChecksEachActualFilename(t *testing.T) { + repo := recordRepo(t) + if err := os.WriteFile(filepath.Join(repo, ".gitignore"), []byte("selective/attempt-[0-7]*.json\n"), 0600); err != nil { + t.Fatal(err) + } + recordGit(t, repo, "add", ".gitignore") + recordGit(t, repo, "commit", "-qm", "Selective ignore") + dir := filepath.Join(repo, "selective") + next := func(name string) func(string) (string, error) { + return func(directory string) (string, error) { return filepath.Join(directory, name), nil } + } + path, err := saveEvidenceNamed(repo, repo, dir, dir, Evidence{Tag: "v1.0.0"}, next("attempt-0.json")) + if err != nil { + t.Fatal(err) + } + recordGit(t, repo, "check-ignore", "--", path) + // A collision must validate the replacement name as well. + calls := 0 + _, err = saveEvidenceNamed(repo, repo, dir, dir, Evidence{Tag: "v2.0.0"}, func(directory string) (string, error) { + calls++ + name := "attempt-0.json" + if calls > 1 { + name = "attempt-c.json" + } + return filepath.Join(directory, name), nil + }) + if err == nil || calls != 2 { + t.Fatalf("unignored replacement: calls=%d error=%v", calls, err) + } + if _, err := os.Stat(filepath.Join(dir, "attempt-c.json")); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("unignored file was created: %v", err) + } + if status := recordGit(t, repo, "status", "--porcelain"); status != "" { + t.Fatalf("recording dirtied candidate: %s", status) + } + loaded, err := LoadEvidence(path) + if err != nil || loaded.Tag != "v1.0.0" { + t.Fatalf("collision changed original record: %#v, %v", loaded, err) + } +} + func TestListAttemptsReturnsReadableRecordsAndDiagnosticsInPathOrder(t *testing.T) { repo := recordRepo(t) directory := filepath.Join(t.TempDir(), "attempts")