diff --git a/docs/internal/overview.md b/docs/internal/overview.md index a4f5653..01c6e7d 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -6,7 +6,8 @@ module and package documentation boundary. Its implemented public primitives are - immutable semantic `Path` construction and strict JSON Pointer parsing; - detached provenance and diagnostic record types with empty `Metadata` queries; - canonical, control-safe diagnostic rendering and structured `LoadError` access; -- opaque file, byte, reader, and no-input source construction and bounded reads; +- opaque file, byte, reader, and no-input source construction in `source.go`, + with shared regular-file checks and bounded read mechanics in `source_file.go`; - a parser-independent ordered tree in `internal/document`; - the sole upstream YAML integration in `internal/yaml`, which translates validated input into that owned tree and exposes no parser types outside its boundary; diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index a80707a..214ff98 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -120,7 +120,14 @@ small cross-entry regression only if a source path lacks equivalent coverage. ## Stage 3 — Share bounded regular-file read mechanics -Status: pending. +Status: complete. + +Completion: `source_file.go` owns regular-file preflight/open/recheck/close and +bounded reads, with private failure kinds and stat/open-only optional-missing +classification. Callers retain containment, metadata, budgets, accounting, and +limit diagnostics. Existing filesystem, FIFO, budget, and reader tests pass; +a private regression covers nondeterministic filesystem failure phases. Full +offline baseline and race suites passed with Go 1.26.7 on Linux/amd64. Evidence: `readSource`, `importTraversal.readImport`, and `readProfileFile` repeat preflight stat, open, post-open stat, regular-file checks, size checks, and bounded diff --git a/imports.go b/imports.go index 0c65425..f29e1a2 100644 --- a/imports.go +++ b/imports.go @@ -3,7 +3,6 @@ package configkit import ( "bytes" "errors" - "io" "io/fs" "os" "path" @@ -382,47 +381,12 @@ func (s *importTraversal) readImport(relative, sourceID string, optional bool) ( if limit < 0 { limit = 0 } - preflight, err := s.root.Stat(relative) - if err != nil { - if optional && errors.Is(err, fs.ErrNotExist) { + data, failure := readRegularFile(relative, s.root.Stat, s.root.Open, limit) + if failure != nil { + if optional && failure.missingTarget { return nil, true, nil } - value := classifySourceError(err, sourceID) - return nil, false, &value - } - if !preflight.Mode().IsRegular() { - value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file") - return nil, false, &value - } - file, err := s.root.Open(relative) - if err != nil { - if optional && errors.Is(err, fs.ErrNotExist) { - return nil, true, nil - } - value := classifySourceError(err, sourceID) - return nil, false, &value - } - defer func() { _ = file.Close() }() - info, err := file.Stat() - if err != nil { - value := classifySourceError(err, sourceID) - return nil, false, &value - } - if !info.Mode().IsRegular() { - value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file") - return nil, false, &value - } - if info.Size() > limit { - value := importDiagnostic(CodeImportLimitExceeded, "configuration exceeds a source or aggregate byte limit", &Location{SourceID: sourceID}) - return nil, false, &value - } - data, err := io.ReadAll(io.LimitReader(file, limit+1)) - if err != nil { - value := classifySourceError(err, sourceID) - return nil, false, &value - } - if int64(len(data)) > limit { - value := importDiagnostic(CodeImportLimitExceeded, "configuration exceeds a source or aggregate byte limit", &Location{SourceID: sourceID}) + value := sourceReadDiagnostic(failure, sourceID, CodeImportLimitExceeded, "configuration exceeds a source or aggregate byte limit") return nil, false, &value } s.usedBytes += int64(len(data)) diff --git a/profile.go b/profile.go index bb29c49..75becba 100644 --- a/profile.go +++ b/profile.go @@ -3,7 +3,6 @@ package configkit import ( "bytes" "errors" - "io" "io/fs" "os" "path/filepath" @@ -122,41 +121,9 @@ func readProfileFile(root *os.Root, relative, sourceID string, remaining int64) if limit < 0 { limit = 0 } - preflight, err := root.Stat(relative) - if err != nil { - value := classifySourceError(err, sourceID) - return nil, &value - } - if !preflight.Mode().IsRegular() { - value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file") - return nil, &value - } - file, err := root.Open(relative) - if err != nil { - value := classifySourceError(err, sourceID) - return nil, &value - } - defer func() { _ = file.Close() }() - info, err := file.Stat() - if err != nil { - value := classifySourceError(err, sourceID) - return nil, &value - } - if !info.Mode().IsRegular() { - value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file") - return nil, &value - } - if info.Size() > limit { - value := importDiagnostic(CodeProfileLimitExceeded, "selected profile exceeds a source or aggregate byte limit", &Location{SourceID: sourceID}) - return nil, &value - } - data, err := io.ReadAll(io.LimitReader(file, limit+1)) - if err != nil { - value := classifySourceError(err, sourceID) - return nil, &value - } - if int64(len(data)) > limit { - value := importDiagnostic(CodeProfileLimitExceeded, "selected profile exceeds a source or aggregate byte limit", &Location{SourceID: sourceID}) + data, failure := readRegularFile(relative, root.Stat, root.Open, limit) + if failure != nil { + value := sourceReadDiagnostic(failure, sourceID, CodeProfileLimitExceeded, "selected profile exceeds a source or aggregate byte limit") return nil, &value } return data, nil diff --git a/source.go b/source.go index 8b0eecc..5914679 100644 --- a/source.go +++ b/source.go @@ -125,10 +125,7 @@ func readSource(source Source, sourceID string, limits sourceLimits) (sourceRead } info := &SourceInfo{ID: sourceID, Label: source.label, Layer: LayerBase} - var ( - input io.Reader - close func() error - ) + var input io.Reader switch source.kind { case sourceFile: absolute, err := filepath.Abs(source.path) @@ -141,32 +138,11 @@ func readSource(source Source, sourceID string, limits sourceLimits) (sourceRead return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)} } info.Path = canonical - preflight, err := os.Stat(canonical) - if err != nil { - return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)} + data, failure := readRegularFile(canonical, os.Stat, os.Open, limit) + if failure != nil { + return sourceRead{info: info}, []Diagnostic{sourceReadDiagnostic(failure, sourceID, CodeSourceLimitExceeded, "source exceeds the configured byte limit")} } - if !preflight.Mode().IsRegular() { - return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")} - } - file, err := os.Open(canonical) - if err != nil { - return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)} - } - close = file.Close - stat, err := file.Stat() - if err != nil { - _ = file.Close() - return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)} - } - if !stat.Mode().IsRegular() { - _ = file.Close() - return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")} - } - if stat.Size() > limit { - _ = file.Close() - return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceLimitExceeded, sourceID, "source exceeds the configured byte limit")} - } - input = file + return sourceRead{data: bytes.Clone(data), info: info}, nil case sourceBytes: input = bytes.NewReader(source.data) case sourceReader: @@ -177,16 +153,9 @@ func readSource(source Source, sourceID string, limits sourceLimits) (sourceRead default: return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceInvalid, sourceID, "source is invalid")} } - if close != nil { - defer func() { _ = close() }() - } - - data, err := io.ReadAll(io.LimitReader(input, limit+1)) - if err != nil { - return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)} - } - if int64(len(data)) > limit { - return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceLimitExceeded, sourceID, "source exceeds the configured byte limit")} + data, failure := readBoundedSource(input, limit) + if failure != nil { + return sourceRead{info: info}, []Diagnostic{sourceReadDiagnostic(failure, sourceID, CodeSourceLimitExceeded, "source exceeds the configured byte limit")} } return sourceRead{data: bytes.Clone(data), info: info}, nil } diff --git a/source_file.go b/source_file.go new file mode 100644 index 0000000..852ada2 --- /dev/null +++ b/source_file.go @@ -0,0 +1,75 @@ +package configkit + +import ( + "errors" + "io" + "io/fs" + "os" +) + +type sourceReadFailureKind uint8 + +const ( + sourceReadIO sourceReadFailureKind = iota + sourceReadNotRegular + sourceReadLimit +) + +// sourceReadFailure never escapes the private source boundary. missingTarget is +// set only for preflight/open failures; a later missing-file error is not optional. +type sourceReadFailure struct { + kind sourceReadFailureKind + cause error + missingTarget bool +} + +// readRegularFile borrows stat/open operations so root-bound callers retain their +// containment policy. Preflight rejects FIFOs before open can block. The opened +// handle is checked again and always closed; read-only close errors are ignored. +func readRegularFile(name string, stat func(string) (fs.FileInfo, error), open func(string) (*os.File, error), limit int64) ([]byte, *sourceReadFailure) { + info, err := stat(name) + if err != nil { + return nil, &sourceReadFailure{cause: err, missingTarget: errors.Is(err, fs.ErrNotExist)} + } + if !info.Mode().IsRegular() { + return nil, &sourceReadFailure{kind: sourceReadNotRegular} + } + file, err := open(name) + if err != nil { + return nil, &sourceReadFailure{cause: err, missingTarget: errors.Is(err, fs.ErrNotExist)} + } + defer func() { _ = file.Close() }() + info, err = file.Stat() + if err != nil { + return nil, &sourceReadFailure{cause: err} + } + if !info.Mode().IsRegular() { + return nil, &sourceReadFailure{kind: sourceReadNotRegular} + } + if info.Size() > limit { + return nil, &sourceReadFailure{kind: sourceReadLimit} + } + return readBoundedSource(file, limit) +} + +func readBoundedSource(input io.Reader, limit int64) ([]byte, *sourceReadFailure) { + data, err := io.ReadAll(io.LimitReader(input, limit+1)) + if err != nil { + return nil, &sourceReadFailure{cause: err} + } + if int64(len(data)) > limit { + return nil, &sourceReadFailure{kind: sourceReadLimit} + } + return data, nil +} + +func sourceReadDiagnostic(failure *sourceReadFailure, sourceID string, limitCode Code, limitMessage string) Diagnostic { + switch failure.kind { + case sourceReadNotRegular: + return sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file") + case sourceReadLimit: + return sourceDiagnostic(limitCode, sourceID, limitMessage) + default: + return classifySourceError(failure.cause, sourceID) + } +} diff --git a/source_file_test.go b/source_file_test.go new file mode 100644 index 0000000..ec73a87 --- /dev/null +++ b/source_file_test.go @@ -0,0 +1,48 @@ +package configkit + +import ( + "io/fs" + "os" + "path/filepath" + "testing" +) + +// Files disappearing between filesystem operations are not deterministic with +// real concurrent deletion. Exercise the private seam that controls optionality. +func TestRegularFileMissingFailurePhases(t *testing.T) { + name := filepath.Join(t.TempDir(), "input.yml") + if err := os.WriteFile(name, []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + for _, test := range []struct { + name string + stat func(string) (fs.FileInfo, error) + open func(string) (*os.File, error) + missing bool + }{ + {"preflight", func(string) (fs.FileInfo, error) { return nil, fs.ErrNotExist }, os.Open, true}, + {"open", os.Stat, func(string) (*os.File, error) { return nil, fs.ErrNotExist }, true}, + {"post-open stat", os.Stat, func(name string) (*os.File, error) { + file, err := os.Open(name) + if err == nil { + err = file.Close() + } + return file, err + }, false}, + } { + t.Run(test.name, func(t *testing.T) { + data, failure := readRegularFile(name, test.stat, test.open, 16) + if data != nil || failure == nil || failure.kind != sourceReadIO || failure.missingTarget != test.missing { + t.Fatalf("data = %v, failure = %#v", data, failure) + } + }) + } + _, failure := readBoundedSource(missingDuringRead{}, 16) + if failure == nil || failure.kind != sourceReadIO || failure.missingTarget { + t.Fatalf("late read failure = %#v", failure) + } +} + +type missingDuringRead struct{} + +func (missingDuringRead) Read([]byte) (int, error) { return 0, fs.ErrNotExist }