-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix read-only volume detection and add refresh capability #7588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… refresh - Fixed isReadOnlyVolume() to detect both short-form (:ro) and long-form (read_only: true) Docker Compose volume syntax - Fixed path matching to use mount_path only (fs_path is transformed during parsing from ./file to absolute path) - Added "Load from server" button for read-only volumes to allow users to refresh content - Changed loadStorageOnServer() authorization from 'update' to 'view' since loading is a read operation - Added helper text to Content field warning users that content may be outdated - Applied fixes to both LocalFileVolume and LocalPersistentVolume models 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAuthorization for loading remote file content was changed from ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (9)
🧰 Additional context used📓 Path-based instructions (10)**/*.php📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Files:
**/**/livewire/**/*.blade.php📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Files:
**/*.blade.php📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Files:
resources/views/livewire/**/*.blade.php📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{php,blade.php}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
resources/views/**/*.blade.php📄 CodeRabbit inference engine (CLAUDE.md)
Files:
app/Livewire/**/*.php📄 CodeRabbit inference engine (CLAUDE.md)
Files:
app/**/*.php📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php}📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Files:
app/Models/**/*.php📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-10T01:53:52.620ZApplied to files:
📚 Learning: 2025-11-25T09:32:48.519ZApplied to files:
🧬 Code graph analysis (4)app/Livewire/Project/Shared/Storages/Show.php (2)
app/Livewire/Project/Service/Storage.php (1)
app/Livewire/Project/Service/FileStorage.php (2)
app/Models/LocalFileVolume.php (1)
🔇 Additional comments (15)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Models/LocalPersistentVolume.php (1)
88-120: Long-form compose handling and mount-path matching look correct; consider de-duplicationThe extended
isReadOnlyVolume()logic now:
- Matches only on
mount_path/ container target (with and without leading/), which avoids the fs-path transformation trap.- Correctly handles both short-form (
host:container:ro) and long-form array syntax withtarget+read_only.That’s the right call and should stop users from “editing” truly read-only persistent volumes—no need to deploy a terminator to guard config files manually.
Given that
LocalFileVolumeimplements almost identical read-only detection, you might want to extract a shared helper (e.g., a small service/trait that takes$dockerComposeRaw,$serviceName,$mountPath) so future changes don’t have to be dual-maintained.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
app/Livewire/Project/Service/FileStorage.php(1 hunks)app/Models/LocalFileVolume.php(1 hunks)app/Models/LocalPersistentVolume.php(2 hunks)resources/views/livewire/project/service/file-storage.blade.php(3 hunks)tests/Unit/LocalFileVolumeReadOnlyTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
tests/Unit/LocalFileVolumeReadOnlyTest.phpapp/Models/LocalPersistentVolume.phpapp/Livewire/Project/Service/FileStorage.phpapp/Models/LocalFileVolume.phpresources/views/livewire/project/service/file-storage.blade.php
tests/Unit/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Run Unit tests outside Docker using
./vendor/bin/pest tests/Unit- they should not require databaseUnit tests in
tests/Unit/should NOT use database and should use mocking instead. Run with./vendor/bin/pest tests/Unit. Unit tests must be designed to run without database dependencies
Files:
tests/Unit/LocalFileVolumeReadOnlyTest.php
tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Always mock external services and SSH connections in tests. Design code for testability using dependency injection and interfaces. Mock by default in unit tests using Mockery. Avoid database when possible
Files:
tests/Unit/LocalFileVolumeReadOnlyTest.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
tests/Unit/LocalFileVolumeReadOnlyTest.phpapp/Models/LocalPersistentVolume.phpapp/Livewire/Project/Service/FileStorage.phpapp/Models/LocalFileVolume.phpresources/views/livewire/project/service/file-storage.blade.php
{**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Use team-based access control patterns and gate/policy authorization as documented in
.ai/patterns/security-patterns.md
Files:
app/Models/LocalPersistentVolume.phpapp/Models/LocalFileVolume.php
app/Models/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Models/**/*.php: When adding new database columns, ALWAYS update the model's$fillablearray to allow mass assignment
Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Files:
app/Models/LocalPersistentVolume.phpapp/Models/LocalFileVolume.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Models/LocalPersistentVolume.phpapp/Livewire/Project/Service/FileStorage.phpapp/Models/LocalFileVolume.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
In Livewire Components, always add the
AuthorizesRequeststrait and check permissions with$this->authorize()calls in mount() and action methods
Files:
app/Livewire/Project/Service/FileStorage.php
**/**/livewire/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Livewire components MUST have exactly ONE root element with no exceptions
Files:
resources/views/livewire/project/service/file-storage.blade.php
**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
**/*.blade.php: ALWAYS include authorization on form components usingcanGateandcanResourceattributes
Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Files:
resources/views/livewire/project/service/file-storage.blade.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/livewire/**/*.blade.php: When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization usingcanGateandcanResourceattributes for automatic authorization
Wrap Modal Components with@candirectives to ensure proper authorization before displaying modals likex-modal-confirmation,x-modal-input, etc.
Livewire component views MUST have exactly ONE root element. ALL content must be contained within this single root element. Placing ANY elements (<style>, <script>,, comments, or other HTML) outside the root will break Livewire's component tracking and cause wire:click and other directives to fail silently
Usewire:model.livefor real-time updates in Livewire components
Files:
resources/views/livewire/project/service/file-storage.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
Tailwind CSS: Use new utilities (version 4.1.4), not deprecated ones. Use
gaputilities for spacing, not margins
Files:
resources/views/livewire/project/service/file-storage.blade.php
🧠 Learnings (2)
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance (via data_get($this, 'environment.project.team')), not a BelongsTo relation. Therefore, treat $database->team() as the model you can operate on (e.g., $database->team()->notify(...)) directly, without accessing a property. Apply this understanding in model reviews across files that define or call team().
Applied to files:
app/Models/LocalPersistentVolume.phpapp/Models/LocalFileVolume.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to **/*.blade.php : Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Applied to files:
resources/views/livewire/project/service/file-storage.blade.php
🧬 Code graph analysis (1)
tests/Unit/LocalFileVolumeReadOnlyTest.php (1)
app/Models/LocalPersistentVolume.php (1)
mountPath(33-38)
🪛 PHPMD (2.15.0)
tests/Unit/LocalFileVolumeReadOnlyTest.php
26-26: Avoid using static access to class '\Symfony\Component\Yaml\Yaml' in method 'isVolumeReadOnly'. (undefined)
(StaticAccess)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-push (aarch64, linux/aarch64, ubuntu-24.04-arm)
🔇 Additional comments (2)
app/Livewire/Project/Service/FileStorage.php (1)
104-118: Auth change toviewfor load is consistent with read-only semanticsSwitching
loadStorageOnServer()toauthorize('view', $this->resource)matches the intent: users with read access can refresh the local mirror from the real file, while writes still requireupdate. This lines up with the Blade@can('view', $resource)gate on the new button, so the permission story is consistent. No need to send a T‑800 to flip a single bit here.Please double-check your policies so that
viewaccess is indeed granted to everyone who should be allowed to pull content from the server but not edit it.resources/views/livewire/project/service/file-storage.blade.php (1)
66-69: Helper text for potentially stale content is clear and consistentThe helper text on all three
x-forms.textareavariants makes it obvious that the content might be outdated and points users to “Load from server”. Good UX, and it matches the actual capability added. If you ever feel like refactoring, you could DRY this string via a Blade component/variable so future edits don’t require three hunts through the code, like tracking three different targets with one plasma rifle.Also applies to: 81-84, 102-105
⛔ Skipped due to learnings
Learnt from: CR Repo: coollabsio/coolify PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-08T20:23:33.775Z Learning: Applies to resources/views/livewire/**/*.blade.php : When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization using `canGate` and `canResource` attributes for automatic authorizationLearnt from: CR Repo: coollabsio/coolify PR: 0 File: .cursor/rules/coolify-ai-docs.mdc:0-0 Timestamp: 2025-11-25T09:32:48.519Z Learning: Applies to **/*.blade.php : Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
…ce() Use relationLoaded() check before accessing the application relationship to avoid triggering individual queries for each volume when rendering storage lists. Update Storage.php to eager load the relationship. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add isServiceResource() and shouldBeReadOnlyInUI() to LocalFileVolume - Update path matching to handle leading slashes in volume comparisons - Update FileStorage and Show components to use shouldBeReadOnlyInUI() - Show consolidated warning message for service/compose resources in all.blade.php - Remove redundant per-volume warnings for service resources - Clean up configuration.blade.php formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Changes
isReadOnlyVolume()in LocalFileVolume and LocalPersistentVolume to detect both short-form (:ro) and long-form (read_only: true) Docker Compose volume syntaxmount_pathonly sincefs_pathgets transformed during parsing from relative (./file) to absolute (/data/coolify/services/uuid/file) pathsloadStorageOnServer()authorization from 'update' to 'view' permission (loading is a read operation, not a write operation)Issues