fix: ensure supportedVersions cache is created with secure permissions (0600) #3148
Conversation
…event permission errors
|
|
WalkthroughModifies Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
I have created a pull request to fix this issue: #3148. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
📒 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)
| interface MyStoreOptions { | ||
| name: string; | ||
| fileMode?: number; | ||
| } | ||
|
|
||
| const supportedVersionsStore = new ElectronStore< | ||
| { [key: string]: SupportedVersions } | ||
| >( | ||
| { | ||
| name: 'supportedVersions', | ||
| fileMode: 0o600, | ||
| } as unknown as MyStoreOptions | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
electron-store fileMode option documentation
💡 Result:
Short answer: electron-store has no documented fileMode option. The official electron-store README/options list does not include a fileMode setting. To control file permissions, change them yourself (e.g., fs.chmod / fs.chmodSync) on store.path after writing. See electron-store docs and Node's fs.chmod. [1][2]
Sources
- electron-store README / options list. [1]
- Node.js fs.chmod documentation (how to change file mode). [2]
🏁 Script executed:
cat -n src/servers/supportedVersions/main.ts | head -100Repository: RocketChat/Rocket.Chat.Electron
Length of output: 206
Remove fileMode approach; use fs.chmodSync instead for file permissions
The fileMode: 0o600 option does not work with electron-store—the library does not support this option in its documented API. To set file permissions on the store file, use fs.chmodSync() on the store path after initialization:
-interface MyStoreOptions {
- name: string;
- fileMode?: number;
-}
-
-const supportedVersionsStore = new ElectronStore<
- { [key: string]: SupportedVersions }
->(
- {
- name: 'supportedVersions',
- fileMode: 0o600,
- } as unknown as MyStoreOptions
-);
+const supportedVersionsStore = new ElectronStore<{
+ [key: string]: SupportedVersions;
+}>({
+ name: 'supportedVersions',
+});
+
+fs.chmodSync(supportedVersionsStore.path, 0o600);This removes the unnecessary MyStoreOptions interface and the incorrect type cast. Apply fs.chmodSync() immediately after store creation to enforce 0o600 permissions on the config file.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[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)
🤖 Prompt for AI Agents
In src/servers/supportedVersions/main.ts around lines 46 to 58, remove the
custom MyStoreOptions interface, drop the fileMode: 0o600 option and the
incorrect as unknown as MyStoreOptions cast when constructing the ElectronStore,
create the store with only the supported options (e.g., name:
'supportedVersions'), then immediately after store creation call
fs.chmodSync(store.path, 0o600) (import fs at top) to enforce file permissions;
handle or log any chmod errors as appropriate.
| const logRequestError = | ||
| (description: string) => | ||
| (error: unknown): undefined => { | ||
| if (axios.isAxiosError(error)) { | ||
| if (error.response) { | ||
| console.error( | ||
| `Couldn't load ${description}: ${error.response.status} ${error.response.data}` | ||
| ); | ||
| (error: unknown): undefined => { | ||
| if (axios.isAxiosError(error)) { | ||
| if (error.response) { | ||
| console.error( | ||
| `Couldn't load ${description}: ${error.response.status} ${error.response.data}` | ||
| ); | ||
| } else { | ||
| // Something happened in setting up the request that triggered an Error | ||
| console.error(`Couldn't load ${description}: ${error.message}`); | ||
| } | ||
| } else { | ||
| // Something happened in setting up the request that triggered an Error | ||
| console.error(`Couldn't load ${description}: ${error.message}`); | ||
| console.error('Fetching ${description} error:', error); | ||
| } | ||
| } else { | ||
| console.error('Fetching ${description} error:', error); | ||
| } | ||
| return undefined; | ||
| }; | ||
| return undefined; | ||
| }; |
There was a problem hiding this comment.
Fix non‑axios error log interpolation and align with Prettier formatting
The new branching for Axios vs non‑Axios errors is clearer, but there’s a small bug and some style issues:
- Line 92 uses single quotes, so
${description}is not interpolated and will log literally. - The arrow function indentation is what Prettier is flagging.
A minimal fix that addresses both:
-const logRequestError =
- (description: string) =>
- (error: unknown): undefined => {
- if (axios.isAxiosError(error)) {
- if (error.response) {
- console.error(
- `Couldn't load ${description}: ${error.response.status} ${error.response.data}`
- );
- } else {
- // Something happened in setting up the request that triggered an Error
- console.error(`Couldn't load ${description}: ${error.message}`);
- }
- } else {
- console.error('Fetching ${description} error:', error);
- }
- return undefined;
- };
+const logRequestError =
+ (description: string) =>
+ (error: unknown): undefined => {
+ if (axios.isAxiosError(error)) {
+ if (error.response) {
+ console.error(
+ `Couldn't load ${description}: ${error.response.status} ${error.response.data}`
+ );
+ } else {
+ // Something happened in setting up the request that triggered an Error
+ console.error(`Couldn't load ${description}: ${error.message}`);
+ }
+ } else {
+ console.error(`Fetching ${description} error:`, error);
+ }
+ return undefined;
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const logRequestError = | |
| (description: string) => | |
| (error: unknown): undefined => { | |
| if (axios.isAxiosError(error)) { | |
| if (error.response) { | |
| console.error( | |
| `Couldn't load ${description}: ${error.response.status} ${error.response.data}` | |
| ); | |
| (error: unknown): undefined => { | |
| if (axios.isAxiosError(error)) { | |
| if (error.response) { | |
| console.error( | |
| `Couldn't load ${description}: ${error.response.status} ${error.response.data}` | |
| ); | |
| } else { | |
| // Something happened in setting up the request that triggered an Error | |
| console.error(`Couldn't load ${description}: ${error.message}`); | |
| } | |
| } else { | |
| // Something happened in setting up the request that triggered an Error | |
| console.error(`Couldn't load ${description}: ${error.message}`); | |
| console.error('Fetching ${description} error:', error); | |
| } | |
| } else { | |
| console.error('Fetching ${description} error:', error); | |
| } | |
| return undefined; | |
| }; | |
| return undefined; | |
| }; | |
| const logRequestError = | |
| (description: string) => | |
| (error: unknown): undefined => { | |
| if (axios.isAxiosError(error)) { | |
| if (error.response) { | |
| console.error( | |
| `Couldn't load ${description}: ${error.response.status} ${error.response.data}` | |
| ); | |
| } else { | |
| // Something happened in setting up the request that triggered an Error | |
| console.error(`Couldn't load ${description}: ${error.message}`); | |
| } | |
| } else { | |
| console.error(`Fetching ${description} error:`, error); | |
| } | |
| return undefined; | |
| }; |
🧰 Tools
🪛 ESLint
[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)
🤖 Prompt for AI Agents
In src/servers/supportedVersions/main.ts around lines 79 to 95, the non-Axios
error branch uses single quotes so `${description}` is not interpolated and the
arrow function indentation violates Prettier; change that console.error to use
template literals (backticks) so description is interpolated and reformat the
nested arrow function indentation to match project Prettier rules (consistent
two-space indentation for the returned function body and its inner blocks).
2 participants
This PR fixes incorrect permission mode for the
supportedVersionsElectronStore 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: 0o600so 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
Chores
✏️ Tip: You can customize this high-level summary in your review settings.