eric/weatherreporter
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Harden report output publication

Browse Source
This commit is contained in:
Eric Rakestraw
2026-08-13 02:40:29 +00:00
parent f4e3a6f26c
commit 04b8358965
9 changed files with 253 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
internal/app/generation_test.go
View File

@@ -26,6 +26,20 @@ type generationCollector struct {
beforeRun func()
}
type publicationGateContext struct {
context.Context
err error
checks int
}
func (c *publicationGateContext) Err() error {
c.checks++
if c.checks >= 2 {
return c.err
}
return nil
}
func (c *generationCollector) Run(context.Context, collect.Request) (*collect.Result, error) {
if c.beforeRun != nil {
c.beforeRun()
@@ -385,6 +399,41 @@ func TestGenerateDetailedPreservesDestinationWhenContextDeadlineExpiresBeforePub
}
}
func TestGenerateDetailedPreservesDestinationWhenContextChangesDuringPublication(t *testing.T) {
for _, tt := range []struct {
name string
err error
category promptexec.ErrorCategory
}{
{name: "canceled", err: context.Canceled, category: promptexec.Canceled},
{name: "deadline", err: context.DeadlineExceeded, category: promptexec.DeadlineExceeded},
} {
t.Run(tt.name, func(t *testing.T) {
outputPath := filepath.Join(t.TempDir(), "daily.md")
const previousReport = "previous report"
if err := os.WriteFile(outputPath, []byte(previousReport), 0o600); err != nil {
t.Fatal(err)
}
ctx := &publicationGateContext{Context: context.Background(), err: tt.err}
cfg := generationConfig()
cfg.Notify.Distributor.Enabled = true
cfg.Notify.Distributor.PipelineIDTemplate = "weather"
bundle := generationBundle(t)
notifier := &generationNotifier{}
result, err := GenerateDetailed(ctx, GenerateRequest{
Config: cfg, Report: ReportDaily,
Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"),
WorkingDir: t.TempDir(), OutputPath: outputPath, Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: notifier,
})
data, readErr := os.ReadFile(outputPath)
matches, globErr := filepath.Glob(filepath.Join(filepath.Dir(outputPath), ".weatherreporter-*.tmp"))
if !errors.Is(err, tt.err) || promptexec.CategoryOf(err) != tt.category || result == nil || result.OutputPath != "" || notifier.calls != 0 || readErr != nil || string(data) != previousReport || globErr != nil || len(matches) != 0 {
t.Fatalf("GenerateDetailed() result/error/output/notification/temp = %#v/%v/%q/%#v/%v/%v", result, err, data, notifier, matches, globErr)
}
})
}
}
func TestGenerateDetailedRetainsPublishedOutputWhenNotificationFails(t *testing.T) {
cfg := generationConfig()
cfg.Notify.Distributor.Enabled = true
@@ -407,10 +456,35 @@ func TestGenerateDetailedDoesNotReplaceDirectoryOutput(t *testing.T) {
if err := os.Mkdir(outputPath, 0o700); err != nil {
t.Fatal(err)
}
result, err := GenerateDetailed(context.Background(), GenerateRequest{Config: generationConfig(), Report: ReportDaily, Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputPath: outputPath, Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}})
collector := &generationCollector{bundle: &bundle}
executor := &generationExecutor{}
notifier := &generationNotifier{}
result, err := GenerateDetailed(context.Background(), GenerateRequest{Config: generationConfig(), Report: ReportDaily, Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputPath: outputPath, Collector: collector, Executor: executor, Notifier: notifier})
info, statErr := os.Stat(outputPath)
if err == nil || result == nil || statErr != nil || !info.IsDir() {
t.Fatalf("GenerateDetailed() result/error/output-info = %#v/%v/%#v (%v)", result, err, info, statErr)
if err == nil || result == nil || statErr != nil || !info.IsDir() || collector.called || executor.promptInspections != 0 || executor.called || notifier.calls != 0 {
t.Fatalf("GenerateDetailed() result/error/output-info/collector/executor/notifier = %#v/%v/%#v (%v)/%t/%#v/%#v", result, err, info, statErr, collector.called, executor, notifier)
}
}
func TestGenerateDetailedDoesNotReplaceSymbolicLinkOutput(t *testing.T) {
dir := t.TempDir()
backing := filepath.Join(dir, "backing.md")
if err := os.WriteFile(backing, []byte("previous report"), 0o600); err != nil {
t.Fatal(err)
}
outputPath := filepath.Join(dir, "daily.md")
if err := os.Symlink(backing, outputPath); err != nil {
t.Fatal(err)
}
bundle := generationBundle(t)
collector := &generationCollector{bundle: &bundle}
executor := &generationExecutor{}
notifier := &generationNotifier{}
result, err := GenerateDetailed(context.Background(), GenerateRequest{Config: generationConfig(), Report: ReportDaily, Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputPath: outputPath, Collector: collector, Executor: executor, Notifier: notifier})
info, statErr := os.Lstat(outputPath)
data, readErr := os.ReadFile(backing)
if err == nil || result == nil || statErr != nil || info.Mode()&os.ModeSymlink == 0 || readErr != nil || string(data) != "previous report" || collector.called || executor.promptInspections != 0 || executor.called || notifier.calls != 0 {
t.Fatalf("GenerateDetailed() result/error/output/backing/collector/executor/notifier = %#v/%v/%#v (%v)/%q (%v)/%t/%#v/%#v", result, err, info, statErr, data, readErr, collector.called, executor, notifier)
}
}

5
internal/app/output.go
View File

@@ -188,10 +188,5 @@ func validateOutputPath(path string) (string, error) {
if err := fileutil.ValidateAtomicPath(path); err != nil {
return "", fmt.Errorf("validate final output path %q: %w", path, err)
}
if info, err := os.Stat(path); err == nil && info.IsDir() {
return "", fmt.Errorf("final output path %q is a directory", path)
} else if err != nil && !os.IsNotExist(err) {
return "", fmt.Errorf("inspect final output path %q: %w", path, err)
}
return path, nil
}

59
internal/app/output_linux_test.go Normal file
View File

@@ -0,0 +1,59 @@
//go:build linux
package app
import (
"context"
"net"
"os"
"path/filepath"
"syscall"
"testing"
)
func TestGenerateDetailedRejectsSpecialOutputBeforeWork(t *testing.T) {
for _, tt := range []struct {
name string
setup func(t *testing.T, path string)
}{
{
name: "named pipe",
setup: func(t *testing.T, path string) {
t.Helper()
if err := syscall.Mkfifo(path, 0o600); err != nil {
t.Fatal(err)
}
},
},
{
name: "socket",
setup: func(t *testing.T, path string) {
t.Helper()
listener, err := net.ListenUnix("unix", &net.UnixAddr{Name: path, Net: "unix"})
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = listener.Close() })
},
},
} {
t.Run(tt.name, func(t *testing.T) {
outputPath := filepath.Join(t.TempDir(), "daily.md")
tt.setup(t, outputPath)
bundle := generationBundle(t)
collector := &generationCollector{bundle: &bundle}
executor := &generationExecutor{}
notifier := &generationNotifier{}
result, err := GenerateDetailed(context.Background(), GenerateRequest{
Config: generationConfig(), Report: ReportDaily,
Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"),
WorkingDir: t.TempDir(), OutputPath: outputPath, Collector: collector, Executor: executor, Notifier: notifier,
})
info, statErr := os.Lstat(outputPath)
if err == nil || result == nil || statErr != nil || info.Mode().IsRegular() || collector.called || executor.promptInspections != 0 || executor.called || notifier.calls != 0 {
t.Fatalf("GenerateDetailed() result/error/output/collector/executor/notifier = %#v/%v/%#v (%v)/%t/%#v/%#v", result, err, info, statErr, collector.called, executor, notifier)
}
})
}
}

5
internal/app/prompt_generate.go
View File

@@ -86,7 +86,10 @@ func publishPromptReport(ctx context.Context, req promptPublicationRequest) (*Re
if err := publicationContextError(ctx); err != nil {
return req.Result, generatedReportError(req.Resolved, req.Result.RunID, "publish report", err)
}
if err := fileutil.WriteFileAtomic(req.OutputPath, req.Markdown); err != nil {
if err := fileutil.WriteFileAtomicContext(ctx, req.OutputPath, req.Markdown); err != nil {
if contextErr := publicationContextError(ctx); contextErr != nil {
return req.Result, generatedReportError(req.Resolved, req.Result.RunID, "publish report", contextErr)
}
return req.Result, err
}
req.Result.OutputPath = req.OutputPath

29
internal/fileutil/fileutil.go
View File

@@ -2,6 +2,7 @@
package fileutil
import (
"context"
"encoding/json"
"fmt"
"os"
@@ -14,15 +15,35 @@ const (
)
// ValidateAtomicPath verifies that the final path can be safely used with this
// package's same-directory atomic-write implementation.
// package's same-directory atomic-write implementation. It accepts only an
// absent path or an existing regular file.
func ValidateAtomicPath(path string) error {
if len(filepath.Base(path)) > maxFileNameBytes {
return fmt.Errorf("final file name exceeds the %d-byte limit", maxFileNameBytes)
}
info, err := os.Lstat(path)
if os.IsNotExist(err) {
return nil
}
if err != nil {
return fmt.Errorf("inspect final destination %q: %w", path, err)
}
if !info.Mode().IsRegular() {
return fmt.Errorf("final destination %q must be absent or a regular file", path)
}
return nil
}
func WriteFileAtomic(path string, data []byte) error {
return WriteFileAtomicContext(context.Background(), path, data)
}
// WriteFileAtomicContext writes data through a same-directory temporary file.
// It checks cancellation immediately before the rename that publishes the file.
func WriteFileAtomicContext(ctx context.Context, path string, data []byte) error {
if ctx == nil {
ctx = context.Background()
}
if err := ValidateAtomicPath(path); err != nil {
return err
}
@@ -42,6 +63,12 @@ func WriteFileAtomic(path string, data []byte) error {
if err := tmp.Close(); err != nil {
return fmt.Errorf("close temporary file for %q: %w", path, err)
}
if err := ValidateAtomicPath(path); err != nil {
return err
}
if err := ctx.Err(); err != nil {
return err
}
if err := os.Rename(tmpName, path); err != nil {
return fmt.Errorf("save %q: %w", path, err)
}

89
internal/fileutil/fileutil_test.go
View File

@@ -1,6 +1,8 @@
package fileutil
import (
"context"
"errors"
"os"
"path/filepath"
"strings"
@@ -21,6 +23,10 @@ func TestWriteFileAtomicCreatesParentDirectory(t *testing.T) {
if string(data) != "artifact" {
t.Fatalf("data = %q, want artifact", data)
}
info, err := os.Stat(path)
if err != nil || info.Mode().Perm() != 0o600 {
t.Fatalf("output mode/error = %o/%v", info.Mode().Perm(), err)
}
}
func TestWriteFileAtomicOverwritesTarget(t *testing.T) {
@@ -67,26 +73,81 @@ func TestWriteFileAtomicSupportsLongestFileName(t *testing.T) {
}
}
func TestWriteFileAtomicCleansTemporaryFileAfterRenameError(t *testing.T) {
func TestWriteFileAtomicRejectsUnsafeFinalDestinations(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "target")
if err := os.Mkdir(target, 0o755); err != nil {
t.Fatalf("Mkdir() error = %v", err)
backing := filepath.Join(dir, "backing.md")
if err := os.WriteFile(backing, []byte("old"), 0o600); err != nil {
t.Fatal(err)
}
for _, tt := range []struct {
name string
setup func(t *testing.T, path string)
}{
{
name: "directory",
setup: func(t *testing.T, path string) {
t.Helper()
if err := os.Mkdir(path, 0o755); err != nil {
t.Fatal(err)
}
},
},
{
name: "symbolic link",
setup: func(t *testing.T, path string) {
t.Helper()
if err := os.Symlink(backing, path); err != nil {
t.Fatal(err)
}
},
},
} {
t.Run(tt.name, func(t *testing.T) {
target := filepath.Join(dir, tt.name)
tt.setup(t, target)
before, err := os.Lstat(target)
if err != nil {
t.Fatal(err)
}
err := WriteFileAtomic(target, []byte("data"))
if err == nil {
t.Fatal("WriteFileAtomic() error = nil, want rename error")
if err := WriteFileAtomic(target, []byte("new")); err == nil {
t.Fatal("WriteFileAtomic() error = nil")
}
after, err := os.Lstat(target)
if err != nil || after.Mode() != before.Mode() {
t.Fatalf("target mode/error = %v/%v, want %v", after.Mode(), err, before.Mode())
}
matches, err := filepath.Glob(filepath.Join(dir, ".weatherreporter-*.tmp"))
if err != nil || len(matches) != 0 {
t.Fatalf("temporary files/error = %v/%v", matches, err)
}
})
}
if !strings.Contains(err.Error(), "save") {
t.Fatalf("error = %q, want save context", err.Error())
data, err := os.ReadFile(backing)
if err != nil || string(data) != "old" {
t.Fatalf("symbolic link target/error = %q/%v", data, err)
}
matches, err := filepath.Glob(filepath.Join(dir, ".weatherreporter-*.tmp"))
if err != nil {
t.Fatalf("Glob() error = %v", err)
}
func TestWriteFileAtomicContextPreservesDestinationWhenCanceledAtPublication(t *testing.T) {
path := filepath.Join(t.TempDir(), "artifact.txt")
if err := os.WriteFile(path, []byte("old"), 0o600); err != nil {
t.Fatal(err)
}
if len(matches) != 0 {
t.Fatalf("temporary files = %v, want none", matches)
ctx, cancel := context.WithCancel(context.Background())
cancel()
err := WriteFileAtomicContext(ctx, path, []byte("new"))
if !errors.Is(err, context.Canceled) {
t.Fatalf("WriteFileAtomicContext() error = %v, want context cancellation", err)
}
data, readErr := os.ReadFile(path)
if readErr != nil || string(data) != "old" {
t.Fatalf("output/error = %q/%v", data, readErr)
}
matches, globErr := filepath.Glob(filepath.Join(filepath.Dir(path), ".weatherreporter-*.tmp"))
if globErr != nil || len(matches) != 0 {
t.Fatalf("temporary files/error = %v/%v", matches, globErr)
}
}