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

Preserve directory traversal failure phases

Browse Source
This commit is contained in:
Eric Rakestraw
2026-10-25 21:33:36 +00:00
parent 877c497325
commit 8aba43da38
6 changed files with 287 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
docs/roadmap/implementation.md
View File

@@ -1,6 +1,6 @@
# Audit Remediation Implementation Plan
Status: ready for implementation; all stages pending.
Status: in progress; completion is recorded per stage below.
Prepared: 2026-10-25.
Planning baseline: `4a5316b7fc2da57dcced33190cb6499c9a1beb5c`.
Audience: a Luna coding agent implementing the stages sequentially.
@@ -229,6 +229,12 @@ Finding: **C01**, including its integrated test improvements.
Owners: `internal/engine/filesystem_access.go`, `import_selection.go`, `glob.go`,
`native_filesystem_test.go`, and `resource_context_test.go`.
Status: **complete** (C01). Prior completed commits: `2736ddf`, `deb9aa5`,
`ae3408a`, `ea27f17`, `08a8c25`, `877c497`. Validation passed:
`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Directory|Glob|Filesystem|Resource' -count=1`,
`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`,
and `git diff --check`.
1. Return private directory failure information distinguishing unsuccessful
initial open from enumeration/close failures. Preserve error unwrapping for
classification; do not expose backing error text.

33
internal/engine/filesystem_access.go
View File

@@ -141,40 +141,59 @@ func (a *filesystemAccess) open(name string) (fs.File, error) {
return a.root.Open(relative)
}
type directoryReadError struct {
opened bool
err error
}
func (e *directoryReadError) Error() string { return "directory could not be read" }
func (e *directoryReadError) Unwrap() error { return e.err }
func missingDirectoryOpen(err error) bool {
var failure *directoryReadError
return errors.As(err, &failure) && !failure.opened && errors.Is(err, fs.ErrNotExist)
}
func (a *filesystemAccess) readDirectory(name string, visit func(fs.DirEntry) bool) error {
file, err := a.open(name)
if err != nil {
return err
return &directoryReadError{err: err}
}
failure := func(err error) error {
if err == nil {
return nil
}
return &directoryReadError{opened: true, err: err}
}
if reader, ok := file.(fs.ReadDirFile); ok {
for {
entries, readErr := reader.ReadDir(globReadBatch)
for _, entry := range entries {
if !visit(entry) {
return file.Close()
return failure(file.Close())
}
}
if errors.Is(readErr, io.EOF) {
return file.Close()
return failure(file.Close())
}
if readErr != nil {
_ = file.Close()
return readErr
return failure(readErr)
}
if len(entries) == 0 {
_ = file.Close()
return io.ErrNoProgress
return failure(io.ErrNoProgress)
}
}
}
if err := file.Close(); err != nil {
return err
return failure(err)
}
// fs.ReadDir provides the standard fallback for implementations whose opened
// files do not implement fs.ReadDirFile. It may allocate all entries itself.
entries, err := fs.ReadDir(a.native, name)
if err != nil {
return err
return failure(err)
}
for _, entry := range entries {
if !visit(entry) {

2
internal/engine/glob.go
View File

@@ -191,7 +191,7 @@ func readGlobEntries(access *filesystemAccess, directory string, location Locati
if errors.Is(err, errImportDirectoryEntryLimit) {
return nil, false, []Diagnostic{importDiagnostic(CodeImportLimitExceeded, "glob traversal exceeds the directory-entry visit limit", &location)}
}
if errors.Is(err, fs.ErrNotExist) {
if missingDirectoryOpen(err) {
return nil, true, nil
}
if err != nil {

2
internal/engine/import_selection.go
View File

@@ -299,7 +299,7 @@ func importDirectoryEntries(access *filesystemAccess, directory string, site imp
if errors.Is(err, errImportDirectoryEntryLimit) {
return nil, false, importSelectionDiagnostic(site, CodeImportLimitExceeded, "directory search exceeds the directory-entry visit limit")
}
if errors.Is(err, fs.ErrNotExist) {
if missingDirectoryOpen(err) {
return nil, true, nil
}
if err != nil {

77
internal/engine/native_filesystem_test.go
View File

@@ -246,22 +246,36 @@ func TestNativeFilesystemDetectsRepeatedAndCyclicImports(t *testing.T) {
}
type observedFilesystem struct {
files fstest.MapFS
opened int
closed int
fallback bool
failRead string
failStat string
failDir string
files fstest.MapFS
opened int
closed int
fallback bool
failOpen string
failRead string
failStat string
failDir string
failReadDir string
readDirErr error
failNoProgress string
failClose string
closeErr error
failFallbackDir string
fallbackErr error
fallbackBeforeClose bool
handles []*observedFile
}
func (f *observedFilesystem) Open(name string) (fs.File, error) {
if name == f.failOpen {
return nil, fs.ErrNotExist
}
inner, err := f.files.Open(name)
if err != nil {
return nil, err
}
f.opened++
file := &observedFile{File: inner, owner: f, name: name}
f.handles = append(f.handles, file)
if directory, ok := inner.(fs.ReadDirFile); ok && !f.fallback {
return &observedDirectoryFile{observedFile: file, directory: directory}, nil
}
@@ -272,20 +286,36 @@ func (f *observedFilesystem) ReadDir(name string) ([]fs.DirEntry, error) {
if name == f.failDir {
return nil, errors.New("tracking failure canary")
}
return fs.ReadDir(f.files, name)
entries, err := fs.ReadDir(f.files, name)
if name == f.failFallbackDir {
for _, handle := range f.handles {
if handle.name == name && !handle.closed {
f.fallbackBeforeClose = true
}
}
return entries, f.fallbackErr
}
return entries, err
}
func (f *observedFilesystem) assertClosed(t *testing.T) {
t.Helper()
if f.opened == 0 || f.opened != f.closed {
t.Fatalf("filesystem opens=%d closes=%d", f.opened, f.closed)
if f.opened != f.closed || len(f.handles) != f.opened {
t.Fatalf("filesystem opens=%d closes=%d handles=%d", f.opened, f.closed, len(f.handles))
}
for _, handle := range f.handles {
if !handle.closed || handle.closeCalls != 1 {
t.Errorf("filesystem handle %q closed=%t close calls=%d", handle.name, handle.closed, handle.closeCalls)
}
}
}
type observedFile struct {
fs.File
owner *observedFilesystem
name string
owner *observedFilesystem
name string
closed bool
closeCalls int
}
func (f *observedFile) Read(data []byte) (int, error) {
@@ -304,7 +334,16 @@ func (f *observedFile) Stat() (fs.FileInfo, error) {
func (f *observedFile) Close() error {
f.owner.closed++
return f.File.Close()
f.closeCalls++
f.closed = true
err := f.File.Close()
if f.name == f.owner.failClose {
if f.owner.closeErr != nil {
return f.owner.closeErr
}
return fs.ErrNotExist
}
return err
}
type observedDirectoryFile struct {
@@ -316,5 +355,15 @@ func (f *observedDirectoryFile) ReadDir(count int) ([]fs.DirEntry, error) {
if f.name == f.owner.failDir {
return nil, errors.New("tracking failure canary")
}
return f.directory.ReadDir(count)
if f.name == f.owner.failNoProgress {
return nil, nil
}
entries, err := f.directory.ReadDir(count)
if f.name == f.owner.failReadDir {
if f.owner.readDirErr != nil {
return entries, f.owner.readDirErr
}
return entries, fs.ErrNotExist
}
return entries, err
}

189
internal/engine/resource_context_test.go
View File

@@ -6,6 +6,7 @@ import (
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"testing/fstest"
)
@@ -181,3 +182,191 @@ func TestLoadingClosesNativeFilesAcrossPipelineOutcomes(t *testing.T) {
})
}
}
func TestDirectorySelectorsFailOnPostOpenNotExistErrors(t *testing.T) {
const canary = "DIRECTORY_FAILURE_CANARY"
readFailure := errors.Join(fs.ErrNotExist, errors.New(canary))
closeFailure := errors.Join(fs.ErrNotExist, errors.New(canary))
for _, test := range []struct {
name string
selector string
fallback bool
readError bool
noProgress bool
closeError bool
fallbackErr bool
}{
{name: "directory batched read with entries", selector: "directory", readError: true},
{name: "directory close", selector: "directory", closeError: true},
{name: "directory fallback with entries", selector: "directory", fallback: true, fallbackErr: true},
{name: "directory no progress", selector: "directory", noProgress: true},
{name: "glob batched read with entries", selector: "glob", readError: true},
{name: "glob close", selector: "glob", closeError: true},
{name: "glob fallback with entries", selector: "glob", fallback: true, fallbackErr: true},
{name: "glob no progress", selector: "glob", noProgress: true},
} {
t.Run(test.name, func(t *testing.T) {
main := "configkit:\n imports:\n"
if test.selector == "directory" {
main += " - dir: [broken, good]\n pattern: '*.yml'\n"
} else {
main += " - glob: 'broken/*.yml'\n - glob: 'good/*.yml'\n"
}
filesystem := &observedFilesystem{
files: fstest.MapFS{
"main.yml": &fstest.MapFile{Data: []byte(main)},
"broken": &fstest.MapFile{Mode: fs.ModeDir | 0o700},
"broken/bad.yml": &fstest.MapFile{Data: []byte("value: bad\n")},
"good": &fstest.MapFile{Mode: fs.ModeDir | 0o700},
"good/good.yml": &fstest.MapFile{Data: []byte("value: good\n")},
},
fallback: test.fallback,
failReadDir: "broken",
readDirErr: readFailure,
failNoProgress: "broken",
failClose: "broken",
closeErr: closeFailure,
failFallbackDir: "broken",
fallbackErr: readFailure,
}
if !test.readError {
filesystem.failReadDir = ""
}
if !test.noProgress {
filesystem.failNoProgress = ""
}
if !test.closeError {
filesystem.failClose = ""
}
if !test.fallbackErr {
filesystem.failFallbackDir = ""
}
result, err := testLoad[map[string]any](filesystemFile(newNativeNamespace("bundle", filesystem), "main.yml"), Options[map[string]any]{Imports: true})
if result != nil {
t.Fatalf("load returned a result after directory failure: %#v", result)
}
var loadError *LoadError
if !errors.As(err, &loadError) {
t.Fatalf("load error = %#v, want structured LoadError", err)
}
diagnostics := loadError.Diagnostics()
if len(diagnostics) != 1 {
t.Fatalf("diagnostics = %#v, want one directory failure", diagnostics)
}
diagnostic := diagnostics[0]
wantCode := CodeImportInvalid
if test.selector == "glob" {
wantCode = CodeImportRead
}
if diagnostic.Code != wantCode || diagnostic.Primary == nil || diagnostic.Primary.SourceID != "source-1" {
t.Fatalf("directory diagnostic = %#v, want %q at source-1", diagnostic, wantCode)
}
var rendered strings.Builder
if err := WriteDiagnostics(&rendered, diagnostics); err != nil {
t.Fatal(err)
}
if strings.Contains(err.Error()+diagnostic.Message+rendered.String(), canary) {
t.Fatalf("backing error canary escaped: error=%q diagnostic=%#v rendered=%q", err, diagnostic, rendered.String())
}
if filesystem.fallbackBeforeClose {
t.Fatal("fallback directory enumeration began before the opened handle was closed")
}
filesystem.assertClosed(t)
})
}
}
func TestReadDirectoryPreservesReadErrorOverCloseError(t *testing.T) {
readFailure := errors.New("read failure")
closeFailure := errors.New("close failure")
filesystem := &observedFilesystem{
files: fstest.MapFS{
"broken": &fstest.MapFile{Mode: fs.ModeDir | 0o700},
"broken/entry": &fstest.MapFile{Data: []byte("entry")},
},
failReadDir: "broken",
readDirErr: readFailure,
failClose: "broken",
closeErr: closeFailure,
}
access := newNativeAccess(filesystem, ".", &namespaceToken{})
visited := 0
err := access.readDirectory("broken", func(fs.DirEntry) bool {
visited++
return true
})
if !errors.Is(err, readFailure) || errors.Is(err, closeFailure) || visited != 1 {
t.Fatalf("directory read = %v, visited %d; want entries plus primary read failure", err, visited)
}
if missingDirectoryOpen(err) {
t.Fatal("post-open read failure was classified as missing-on-open")
}
filesystem.assertClosed(t)
}
func TestReadDirectoryVisitStopCloseNotExistIsPostOpen(t *testing.T) {
filesystem := &observedFilesystem{
files: fstest.MapFS{
"broken": &fstest.MapFile{Mode: fs.ModeDir | 0o700},
"broken/entry": &fstest.MapFile{Data: []byte("entry")},
},
failClose: "broken",
}
access := newNativeAccess(filesystem, ".", &namespaceToken{})
err := access.readDirectory("broken", func(fs.DirEntry) bool { return false })
if !errors.Is(err, fs.ErrNotExist) || missingDirectoryOpen(err) {
t.Fatalf("visit-stop close error = %v, missing-on-open=%t", err, missingDirectoryOpen(err))
}
filesystem.assertClosed(t)
}
func TestDirectorySelectorsKeepMissingRootsOptional(t *testing.T) {
for _, selector := range []string{"directory", "glob"} {
t.Run(selector, func(t *testing.T) {
main := "configkit:\n imports:\n"
if selector == "directory" {
main += " - dir: [missing, good]\n pattern: '*.yml'\n required: true\n"
} else {
main += " - glob: 'missing/*.yml'\n - glob: 'good/*.yml'\n required: true\n"
}
filesystem := &observedFilesystem{files: fstest.MapFS{
"main.yml": &fstest.MapFile{Data: []byte(main)},
"good": &fstest.MapFile{Mode: fs.ModeDir | 0o700},
"good/good.yml": &fstest.MapFile{Data: []byte("value: good\n")},
}}
result, err := testLoad[map[string]any](filesystemFile(newNativeNamespace("bundle", filesystem), "main.yml"), Options[map[string]any]{Imports: true})
if err != nil || result == nil {
t.Fatalf("load with missing search root = %#v, %v", result, err)
}
filesystem.assertClosed(t)
})
}
}
func TestDirectorySelectorsTreatMissingOnOpenAsOptional(t *testing.T) {
for _, selector := range []string{"directory", "glob"} {
t.Run(selector, func(t *testing.T) {
main := "configkit:\n imports:\n"
if selector == "directory" {
main += " - dir: [broken, good]\n pattern: '*.yml'\n required: true\n"
} else {
main += " - glob: 'broken/*.yml'\n - glob: 'good/*.yml'\n required: true\n"
}
filesystem := &observedFilesystem{
files: fstest.MapFS{
"main.yml": &fstest.MapFile{Data: []byte(main)},
"broken": &fstest.MapFile{Mode: fs.ModeDir | 0o700},
"good": &fstest.MapFile{Mode: fs.ModeDir | 0o700},
"good/good.yml": &fstest.MapFile{Data: []byte("value: good\n")},
},
failOpen: "broken",
}
result, err := testLoad[map[string]any](filesystemFile(newNativeNamespace("bundle", filesystem), "main.yml"), Options[map[string]any]{Imports: true})
if err != nil || result == nil {
t.Fatalf("load after missing-on-open root = %#v, %v", result, err)
}
filesystem.assertClosed(t)
})
}
}