Skip to content

fix: ensure supportedVersions cache is created with secure permissions (0600)#3148

Open
Nitinref wants to merge 1 commit intoRocketChat:masterfrom
Nitinref:fix-electronstore-permissions
Open

fix: ensure supportedVersions cache is created with secure permissions (0600)#3148
Nitinref wants to merge 1 commit intoRocketChat:masterfrom
Nitinref:fix-electronstore-permissions

Conversation

@Nitinref
Copy link

@Nitinref Nitinref commented Dec 1, 2025

This PR fixes incorrect permission mode for the supportedVersions ElectronStore cache file.

The file was created with default permissions (~644), which caused permission errors when reading the cache in some environments.
This fix sets fileMode: 0o600 so the cache is readable and writable only by the current user.

Tested on Windows and WSL — the cache file is now created with 600 permissions.

Screenshot:

Closes #3147

Summary by CodeRabbit

  • Bug Fixes

    • Improved error messaging and diagnostic logging to help users identify and troubleshoot issues more effectively.
  • Chores

    • Internal configuration optimizations and refined error handling mechanisms for enhanced security and reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

@CLAassistant
Copy link

CLAassistant commented Dec 1, 2025

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 1, 2025

Walkthrough

Modifies ElectronStore initialization to set file permissions to 0o600 (read/write only for owner) instead of the default permissive mode, addressing a security issue. Also restructures error logging in the request handler with explicit branches for different error types.

Changes

Cohort / File(s) Summary
ElectronStore Configuration & Error Logging
src/servers/supportedVersions/main.ts
Introduces MyStoreOptions interface and sets fileMode: 0o600 on ElectronStore initialization for restrictive file permissions. Refactors logRequestError with explicit branching: logs error.message for AxiosErrors without response, and updates non-AxiosError logging pattern with literal error description string.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Single-file change with straightforward configuration and logging updates
  • No complex logic, control flow changes, or multi-component interactions
  • Security fix follows standard permission-setting pattern
  • Error logging improvements are incremental and isolated

Poem

🐰 A rabbit hops through the config, so fine,
Setting permissions to 0o600 in a line,
No more world-writable woes,
Secure file modes the whole tunnel knows! 🔐✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes modifications to error handling in logRequestError function that are unrelated to the permission fix objective from issue #3147. Remove the error handling changes to logRequestError; keep only the supportedVersions cache permission fix required to resolve issue #3147.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing the supportedVersions cache file permissions to use secure mode (0600).
Linked Issues check ✅ Passed The PR changes set fileMode to 0o600 for supportedVersions ElectronStore, directly addressing issue #3147's requirement for secure file permissions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Nitinref
Copy link
Author

Nitinref commented Dec 1, 2025

I have created a pull request to fix this issue: #3148.
The PR adds secure file permissions (0600) to the supportedVersions cache so it is only readable/writable by the current user.
Tested on Windows and WSL.
Please review when possible. 🙂

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4663dcc and 9c77cd0.

📒 Files selected for processing (1)
  • src/servers/supportedVersions/main.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions

Files:

  • src/servers/supportedVersions/main.ts
🧬 Code graph analysis (1)
src/servers/supportedVersions/main.ts (1)
src/servers/supportedVersions/types.ts (1)
  • SupportedVersions (33-45)
🪛 ESLint
src/servers/supportedVersions/main.ts

[error] 51-51: Insert {

(prettier/prettier)


[error] 52-52: Replace ·{·[key:·string]:·SupportedVersions·} with ·[key:·string]:·SupportedVersions;

(prettier/prettier)


[error] 53-54: Replace >(⏎·· with }>(

(prettier/prettier)


[error] 55-55: Delete ··

(prettier/prettier)


[error] 56-56: Delete ··

(prettier/prettier)


[error] 57-59: Replace ··}·as·unknown·as·MyStoreOptions⏎);⏎ with }·as·unknown·as·MyStoreOptions);

(prettier/prettier)


[error] 81-81: Delete ··

(prettier/prettier)


[error] 82-82: Replace ······ with ····

(prettier/prettier)


[error] 83-83: Delete ··

(prettier/prettier)


[error] 84-84: Delete ··

(prettier/prettier)


[error] 85-85: Delete ··

(prettier/prettier)


[error] 86-86: Delete ··

(prettier/prettier)


[error] 87-87: Delete ··

(prettier/prettier)


[error] 88-88: Delete ··

(prettier/prettier)


[error] 89-89: Delete ··

(prettier/prettier)


[error] 90-90: Delete ··

(prettier/prettier)


[error] 91-91: Delete ··

(prettier/prettier)


[error] 92-92: Delete ··

(prettier/prettier)


[error] 93-93: Delete ··

(prettier/prettier)


[error] 94-94: Delete ··

(prettier/prettier)


[error] 95-95: Delete ··

(prettier/prettier)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rocket.Chat Electron creates config files with overly permissive file permissions (0666)

2 participants