fix: prevent EnsurePath panic on non-ENOENT stat errors#1588
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash in pkg/utils.EnsurePath by correctly handling os.Stat failures that are not ENOENT (e.g., ENOTDIR, permission errors), returning the Stat error instead of dereferencing a nil FileInfo. It also adds a regression test to ensure the non-ENOENT error path returns an error and does not panic.
Changes:
- Update
EnsurePathto return immediately on non-os.IsNotExisterrors fromos.Stat. - Restrict
os.MkdirAllto theENOENTcase and return its result directly. - Add a unit test case covering a non-
ENOENTos.Staterror scenario (file/child=>ENOTDIR-style error).
Show a summary per file
| File | Description |
|---|---|
pkg/utils/utils.go |
Prevents EnsurePath panic by returning non-ENOENT os.Stat errors early and only creating directories on ENOENT. |
pkg/utils/utils_test.go |
Adds regression coverage to ensure the non-ENOENT stat error path returns an error (and would fail if a panic regressed). |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1588 +/- ##
==========================================
+ Coverage 38.80% 41.18% +2.37%
==========================================
Files 57 58 +1
Lines 9561 10114 +553
==========================================
+ Hits 3710 4165 +455
- Misses 5581 5653 +72
- Partials 270 296 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
omercnet
left a comment
There was a problem hiding this comment.
Fix looks correct. The lint failure appears to be CI infrastructure, not this diff: golangci-lint config schema fetch timed out. Please rename StatErrorNotExist to StatErrorNonENOENT since path under a file exercises ENOTDIR.
Motivation
EnsurePathcould dereference a nilFileInfowhenos.Statreturned an error that was notENOENT, which caused a panic for callers (e.g.,ENOTDIRor permission errors).ENOENTos.Staterrors as immediate failures while preserving the existing behavior of creating missing directories.Description
pkg/utils/utils.goupdatedEnsurePathto immediately return non-ENOENTerrors fromos.Statinstead of dereferencingst, and to only callos.MkdirAllwhen the error isENOENT.st.IsDir()and permission bits whenos.Statsucceeds.StatErrorNotExistinpkg/utils/utils_test.gothat exercises a non-ENOENTstat error path (e.g.,path.Join(emptyFile, "child")) to ensure the function returns an error instead of panicking.Testing
StatErrorNotExistinpkg/utilsalongside existingTestEnsurePathtests and validated locally that the test exercises the non-ENOENTpath (regression coverage added).go test ./pkg/utilsin this environment but the run failed due to module downloads fromproxy.golang.orgbeing forbidden, so tests could not complete here.Codex Task