[4.x] Prevent mkdir() race conditions in FilesystemTenancyBootstrapper#1453
[4.x] Prevent mkdir() race conditions in FilesystemTenancyBootstrapper#1453
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDirectory creation in FilesystemTenancyBootstrapper now uses error-suppressed mkdir calls ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Bootstrappers/FilesystemTenancyBootstrapper.php`:
- Line 79: In FilesystemTenancyBootstrapper (the bootstrap routine that calls
`@mkdir`), keep the `@mkdir` for race tolerance but immediately check that the
target directory actually exists after the call; if it does not, throw a generic
\Exception with a clear message including the path so bootstrap fails fast.
Apply the same change to the other occurrence of `@mkdir` in this class (the
second directory-creation site) so neither silent mkdir failure is swallowed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9736031c-344f-4e02-b25a-181d6bda6379
📒 Files selected for processing (1)
src/Bootstrappers/FilesystemTenancyBootstrapper.php
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1453 +/- ##
============================================
- Coverage 86.08% 86.03% -0.05%
- Complexity 1154 1156 +2
============================================
Files 184 184
Lines 3377 3381 +4
============================================
+ Hits 2907 2909 +2
- Misses 470 472 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This prevents race conditions that may occur if there are two concurrent processes trying to create the storage path for the tenant. The storagePath() method runs during bootstrap() which can easily happen in two places at once. The race condition specifically occurs in between the is_dir() check and the mkdir() call, the latter producing an exception if the dir already exist. We simply ignore any error coming out of mkdir() and then check for success separately. We could omit that success check since failure is unlikely and would only occur due to a server misconfiguration that would manifest itself in other ways as well, but this way the simple TOC/TOU race condition is prevented while other errors are still reported. We apply the same change to the mkdir() in scopeSessions() as the logic is similar. Resolves #1452
fcd4d82 to
c344e3d
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Prevent mkdir() race conditions in FilesystemTenancyBootstrapper
This prevents race conditions that may occur if there are two concurrent
processes trying to create the storage path for the tenant. The
storagePath() method runs during bootstrap() which can easily happen
in two places at once. The race condition specifically occurs in between
the is_dir() check and the mkdir() call, the latter producing an
exception if the dir already exist. We simply ignore any error coming
out of mkdir() and then check for success separately.
We could omit that success check since failure is unlikely and would
only occur due to a server misconfiguration that would manifest itself
in other ways as well, but this way the simple TOC/TOU race condition
is prevented while other errors are still reported.
We apply the same change to the mkdir() in scopeSessions() as the logic
is similar.
Resolves #1452
Summary by CodeRabbit