Allow editing .env files as text and add update validation tests - #20
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 25 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds ChangesText file editing enhancements
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over Client,DB: Text file update with revisioning
end
participant Client
participant Handler as text-file update handler
participant updateFileWithRevision
participant DB as DB transaction
participant Disk as filesystem
Client->>Handler: PUT /api/file-tree/text-file {content: "..."}
Handler->>Handler: decode into TextFileUpdateAPIData (*string)
Handler->>updateFileWithRevision: path, buildContent(*req.Content), writeCallback
updateFileWithRevision->>Disk: acquire OS lock, read existing bytes
updateFileWithRevision->>updateFileWithRevision: hash old vs new bytes
alt content unchanged
updateFileWithRevision-->>Handler: no-op, return early
end
updateFileWithRevision->>DB: begin tx, insert revision record
updateFileWithRevision->>Disk: write previous content to revision dir
updateFileWithRevision->>DB: update revision with snapshot path
updateFileWithRevision->>Disk: execute write callback (new content)
alt write succeeds
updateFileWithRevision->>DB: update revision status = completed, commit
updateFileWithRevision-->>Handler: revision_id
Handler-->>Client: HTTP 200
else write fails
updateFileWithRevision->>DB: rollback
updateFileWithRevision->>Disk: restoreFileAfterFailedWrite (best-effort)
Handler-->>Client: HTTP 500
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 unit tests (beta)
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/server/file_system_routes.go (1)
779-792: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRevision snapshots are stored with overly permissive filesystem modes.
Line 779 creates revision directories as
0755, and Line 792 writes prior-content snapshots as0644. With.envsupport, revision copies can contain secrets and become readable by other local users.🔧 Proposed fix
- if err = s.fileEditor.MkdirAll(revisionDir, 0755); err != nil { + if err = s.fileEditor.MkdirAll(revisionDir, 0700); err != nil { ... } - if err = s.fileEditor.WriteFile(revisionPath, previousData, 0644); err != nil { + if err = s.fileEditor.WriteFile(revisionPath, previousData, ctx.info.Mode().Perm()); err != nil { ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/server/file_system_routes.go` around lines 779 - 792, The revision directory and file permissions are overly permissive, allowing other local users to read sensitive data. Change the permissions in the s.fileEditor.MkdirAll call from 0755 to 0700 to restrict directory access to the owner only, and change the permissions in the s.fileEditor.WriteFile call from 0644 to 0600 to restrict file access to the owner only. This ensures revision snapshots containing secrets are not readable by other local users.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/server/file_system_routes.go`:
- Around line 1010-1012: The WriteTextFileData method call within the
updateFileWithRevision closure on line 1012 writes files with fixed 0644
permissions, which unintentionally relaxes permissions on previously restricted
files like .env files with 0600 permissions. To fix this, before calling
WriteTextFileData, retrieve the current file's permission metadata from the
existing file at ctx.cleanPath, and then pass those original permissions to
WriteTextFileData (or a similar method that accepts permission parameters) to
ensure the updated file retains its original permission settings.
---
Outside diff comments:
In `@internal/server/file_system_routes.go`:
- Around line 779-792: The revision directory and file permissions are overly
permissive, allowing other local users to read sensitive data. Change the
permissions in the s.fileEditor.MkdirAll call from 0755 to 0700 to restrict
directory access to the owner only, and change the permissions in the
s.fileEditor.WriteFile call from 0644 to 0600 to restrict file access to the
owner only. This ensures revision snapshots containing secrets are not readable
by other local users.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2aebe3dc-8fba-464c-9d25-2e7780bcaca9
📒 Files selected for processing (4)
internal/server/file_system_routes.gointernal/server/file_system_text_test.gointernal/services/file_editor_service.gointernal/services/file_editor_service_item_test.go
Summary
.envand.env.*files as editable text files in the file editor servicecontentpayloads.envdetection and text file update behaviorTesting
.envfile type detection and matching rulesSummary by CodeRabbit
Release Notes
New Features
Improvements
Tests