-
Notifications
You must be signed in to change notification settings - Fork 578
fix: resolve data directory persistence between Electron and Web modes #573
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
Changes from all commits
4cd84a4
7eae021
4186b80
b8875f7
e10cb83
b0b4976
fdad82b
a7f7898
174c02c
2a8706e
b66efae
9137f0e
7b7ac72
484d4c6
f378122
2e57553
505a2b1
980006d
7795d81
f68aee6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -91,6 +91,9 @@ const PORT = parseInt(process.env.PORT || '3008', 10); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const HOST = process.env.HOST || '0.0.0.0'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const HOSTNAME = process.env.HOSTNAME || 'localhost'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const DATA_DIR = process.env.DATA_DIR || './data'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('[SERVER_STARTUP] process.env.DATA_DIR:', process.env.DATA_DIR); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('[SERVER_STARTUP] Resolved DATA_DIR:', DATA_DIR); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('[SERVER_STARTUP] process.cwd():', process.cwd()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const ENABLE_REQUEST_LOGGING_DEFAULT = process.env.ENABLE_REQUEST_LOGGING !== 'false'; // Default to true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Runtime-configurable request logging flag (can be changed via settings) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -175,14 +178,25 @@ app.use( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // For local development, allow localhost origins | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| origin.startsWith('http://localhost:') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| origin.startsWith('http://127.0.0.1:') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| origin.startsWith('http://[::1]:') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| callback(null, origin); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // For local development, allow all localhost/loopback origins (any port) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const url = new URL(origin); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const hostname = url.hostname; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname === 'localhost' || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname === '127.0.0.1' || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname === '::1' || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname === '0.0.0.0' || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname.startsWith('192.168.') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname.startsWith('10.') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname.startsWith('172.') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| callback(null, origin); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ignore URL parsing errors | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+181
to
200
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restrict 172. CORS allowance to RFC1918 only.* 🔧 Proposed fix try {
const url = new URL(origin);
const hostname = url.hostname;
+ const isPrivate172 = /^172\.(1[6-9]|2\d|3[0-1])\./.test(hostname);
if (
hostname === 'localhost' ||
hostname === '127.0.0.1' ||
hostname === '::1' ||
hostname === '0.0.0.0' ||
hostname.startsWith('192.168.') ||
hostname.startsWith('10.') ||
- hostname.startsWith('172.')
+ isPrivate172
) {
callback(null, origin);
return;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Reject other origins by default for security | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,18 +45,24 @@ export function createUpdateGlobalHandler(settingsService: SettingsService) { | |
| } | ||
|
|
||
| // Minimal debug logging to help diagnose accidental wipes. | ||
| if ('projects' in updates || 'theme' in updates || 'localStorageMigrated' in updates) { | ||
| const projectsLen = Array.isArray((updates as any).projects) | ||
| ? (updates as any).projects.length | ||
| : undefined; | ||
| logger.info( | ||
| `Update global settings request: projects=${projectsLen ?? 'n/a'}, theme=${ | ||
| (updates as any).theme ?? 'n/a' | ||
| }, localStorageMigrated=${(updates as any).localStorageMigrated ?? 'n/a'}` | ||
| ); | ||
| } | ||
| const projectsLen = Array.isArray((updates as any).projects) | ||
| ? (updates as any).projects.length | ||
| : undefined; | ||
| const trashedLen = Array.isArray((updates as any).trashedProjects) | ||
| ? (updates as any).trashedProjects.length | ||
| : undefined; | ||
| logger.info( | ||
| `[SERVER_SETTINGS_UPDATE] Request received: projects=${projectsLen ?? 'n/a'}, trashedProjects=${trashedLen ?? 'n/a'}, theme=${ | ||
| (updates as any).theme ?? 'n/a' | ||
| }, localStorageMigrated=${(updates as any).localStorageMigrated ?? 'n/a'}` | ||
| ); | ||
|
|
||
| logger.info('[SERVER_SETTINGS_UPDATE] Calling updateGlobalSettings...'); | ||
| const settings = await settingsService.updateGlobalSettings(updates); | ||
| logger.info( | ||
| '[SERVER_SETTINGS_UPDATE] Update complete, projects count:', | ||
| settings.projects?.length ?? 0 | ||
| ); | ||
|
Comment on lines
+54
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| // Apply server log level if it was updated | ||
| if ('serverLogLevel' in updates && updates.serverLogLevel) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,13 +273,39 @@ export class SettingsService { | |
| }; | ||
|
|
||
| const currentProjectsLen = Array.isArray(current.projects) ? current.projects.length : 0; | ||
| // Check if this is a legitimate project removal (moved to trash) vs accidental wipe | ||
| const newTrashedProjectsLen = Array.isArray(sanitizedUpdates.trashedProjects) | ||
| ? sanitizedUpdates.trashedProjects.length | ||
| : Array.isArray(current.trashedProjects) | ||
| ? current.trashedProjects.length | ||
| : 0; | ||
|
|
||
| if ( | ||
| Array.isArray(sanitizedUpdates.projects) && | ||
| sanitizedUpdates.projects.length === 0 && | ||
| currentProjectsLen > 0 | ||
| ) { | ||
| attemptedProjectWipe = true; | ||
| delete sanitizedUpdates.projects; | ||
| // Only treat as accidental wipe if trashedProjects is also empty | ||
| // (If projects are moved to trash, they appear in trashedProjects) | ||
| if (newTrashedProjectsLen === 0) { | ||
| logger.warn( | ||
| '[WIPE_PROTECTION] Attempted to set projects to empty array with no trash! Ignoring update.', | ||
| { | ||
| currentProjectsLen, | ||
| newProjectsLen: 0, | ||
| newTrashedProjectsLen, | ||
| currentProjects: current.projects?.map((p) => p.name), | ||
| } | ||
| ); | ||
| attemptedProjectWipe = true; | ||
| delete sanitizedUpdates.projects; | ||
| } else { | ||
| logger.info('[LEGITIMATE_REMOVAL] Removing all projects to trash', { | ||
| currentProjectsLen, | ||
| newProjectsLen: 0, | ||
| movedToTrash: newTrashedProjectsLen, | ||
| }); | ||
| } | ||
| } | ||
|
Comment on lines
+277
to
309
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic for A safer approach is to consider a removal legitimate only if the number of trashed projects increases in the same update. const currentTrashedProjectsLen = current.trashedProjects?.length ?? 0;
if (
Array.isArray(sanitizedUpdates.projects) &&
sanitizedUpdates.projects.length === 0 &&
currentProjectsLen > 0
) {
// A legitimate removal of all projects should be accompanied by an increase in trashed projects.
// If `trashedProjects` is not in the update or is not increasing, it's likely an accidental wipe.
if (
!Array.isArray(sanitizedUpdates.trashedProjects) ||
sanitizedUpdates.trashedProjects.length <= currentTrashedProjectsLen
) {
logger.warn(
'[WIPE_PROTECTION] Attempted to set projects to empty array without moving items to trash! Ignoring update.',
{
currentProjectsLen,
newProjectsLen: 0,
currentTrashedProjectsLen,
newTrashedProjectsLen: sanitizedUpdates.trashedProjects?.length ?? 'n/a',
currentProjects: current.projects?.map((p) => p.name),
}
);
attemptedProjectWipe = true;
delete sanitizedUpdates.projects;
} else {
logger.info('[LEGITIMATE_REMOVAL] Removing all projects to trash', {
currentProjectsLen,
newProjectsLen: 0,
movedToTrash: sanitizedUpdates.trashedProjects.length - currentTrashedProjectsLen,
});
}
}
Comment on lines
275
to
309
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wipe protection can still allow accidental wipes when 🔧 Proposed fix- const newTrashedProjectsLen = Array.isArray(sanitizedUpdates.trashedProjects)
- ? sanitizedUpdates.trashedProjects.length
- : Array.isArray(current.trashedProjects)
- ? current.trashedProjects.length
- : 0;
+ const hasTrashedProjectsUpdate = Array.isArray(sanitizedUpdates.trashedProjects);
+ const newTrashedProjectsLen = hasTrashedProjectsUpdate
+ ? sanitizedUpdates.trashedProjects.length
+ : 0;
if (
Array.isArray(sanitizedUpdates.projects) &&
sanitizedUpdates.projects.length === 0 &&
currentProjectsLen > 0
) {
// Only treat as accidental wipe if trashedProjects is also empty
// (If projects are moved to trash, they appear in trashedProjects)
- if (newTrashedProjectsLen === 0) {
+ if (!hasTrashedProjectsUpdate || newTrashedProjectsLen === 0) {
logger.warn(
'[WIPE_PROTECTION] Attempted to set projects to empty array with no trash! Ignoring update.',
{
currentProjectsLen,
newProjectsLen: 0,🤖 Prompt for AI Agents |
||
|
|
||
| ignoreEmptyArrayOverwrite('trashedProjects'); | ||
|
|
||
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.
These startup logs are helpful for debugging but might be too verbose for production environments. Consider changing them to
logger.debugor removing them if they are temporary.