diff --git a/README.md b/README.md index 28b30a7..9ccd02b 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Your Markdown workspace for real projects, docs, and personal knowledge bases, b Official website: [notebranch.app](https://notebranch.app) -**Version**: 2.9.1 +**Version**: 2.9.2 **License**: MIT ## Built for daily note workflows diff --git a/app/desktop/integration-tests/git/find-in-file.integration.spec.ts b/app/desktop/integration-tests/git/find-in-file.integration.spec.ts new file mode 100644 index 0000000..25a48f9 --- /dev/null +++ b/app/desktop/integration-tests/git/find-in-file.integration.spec.ts @@ -0,0 +1,63 @@ +import { expect, test } from "@playwright/test"; +import type { ElectronApplication, Page } from "@playwright/test"; +import { + appendToCurrentEditor, + cleanupUserDataDir, + closeAppIfOpen, + connectGitRepo, + createIsolatedUserDataDir, + createMarkdownFile, + launchIntegrationApp, +} from "../helpers/gitIntegration"; + +const getModKey = () => (process.platform === "darwin" ? "Meta" : "Control"); + +test("(git) in-file find uses one working search bar", async ({ + request: _request, +}, testInfo) => { + const userDataDir = await createIsolatedUserDataDir(testInfo); + let app: ElectronApplication | null = null; + try { + const launched = await launchAndSetup(userDataDir); + app = launched.app; + const page = launched.page; + + const editor = page.locator(".cm-content").first(); + await expect(editor).toBeVisible(); + await editor.click(); + + await page.keyboard.press(`${getModKey()}+F`); + await expect(page.getByTestId("find-replace-bar")).toBeVisible(); + await expect(page.locator(".cm-search")).toHaveCount(0); + + await page.keyboard.press(`${getModKey()}+F`); + await expect(page.getByTestId("find-replace-bar")).toHaveCount(1); + await expect(page.locator(".cm-search")).toHaveCount(0); + + const queryInput = page.getByTestId("find-replace-query-input"); + await expect(queryInput).toBeVisible(); + await queryInput.fill("beta"); + await queryInput.press("Enter"); + + await expect(queryInput).toBeFocused(); + await expect(page.getByText("1/2", { exact: true })).toBeVisible(); + + await queryInput.press("Enter"); + await expect(queryInput).toBeFocused(); + await expect(page.getByText("2/2", { exact: true })).toBeVisible(); + } finally { + await closeAppIfOpen(app); + await cleanupUserDataDir(userDataDir); + } +}); + +const launchAndSetup = async ( + userDataDir: string, +): Promise<{ app: ElectronApplication; page: Page }> => { + const launched = await launchIntegrationApp(userDataDir); + const page = launched.page; + await connectGitRepo(page); + await createMarkdownFile(page, "find-chaos.txt"); + await appendToCurrentEditor(page, "\nalpha beta alpha beta alpha\n"); + return launched; +}; diff --git a/app/desktop/integration-tests/helpers/gitIntegration.ts b/app/desktop/integration-tests/helpers/gitIntegration.ts index 8fce3e8..9ef478c 100644 --- a/app/desktop/integration-tests/helpers/gitIntegration.ts +++ b/app/desktop/integration-tests/helpers/gitIntegration.ts @@ -122,7 +122,34 @@ export const closeAppIfOpen = async ( if (!app) { return; } - await app.close(); + + const forceExit = async () => { + try { + await app.evaluate(({ app: electronApp }) => { + electronApp.exit(0); + }); + } catch { + // Best-effort fallback during teardown. + } + }; + + try { + const windows = app.windows(); + await Promise.all( + windows.map(async (window) => { + if (!window.isClosed()) { + await window.close({ runBeforeUnload: false }); + } + }), + ); + await app.close(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (!message.includes("Page.handleJavaScriptDialog")) { + throw error; + } + await forceExit(); + } }; export const connectGitRepo = async ( @@ -452,7 +479,7 @@ export const apiCreateFile = async ( ): Promise => { const response = await page.evaluate( async ({ parentPath: p, name: n }) => { - return await window.NoteBranchApi.files.create(p, n); + return await window.NoteBranchApi.files.createFile(p, n); }, { parentPath, name }, ); diff --git a/app/desktop/package.json b/app/desktop/package.json index 4a332aa..5dde97e 100644 --- a/app/desktop/package.json +++ b/app/desktop/package.json @@ -1,6 +1,6 @@ { "name": "NoteBranch", - "version": "2.9.1", + "version": "2.9.2", "packageManager": "pnpm@10.28.0", "description": "A Git-backed Markdown note-taking desktop application", "homepage": "https://github.com/scabir/notebranch", diff --git a/app/desktop/src/backend/handlers/configHandlers.ts b/app/desktop/src/backend/handlers/configHandlers.ts index 058c849..6a1a5a7 100644 --- a/app/desktop/src/backend/handlers/configHandlers.ts +++ b/app/desktop/src/backend/handlers/configHandlers.ts @@ -6,16 +6,220 @@ import { RepoSettings, Profile, ApiErrorCode, + REPO_PROVIDERS, + AuthMethod, } from "../../shared/types"; import { ConfigService } from "../services/ConfigService"; import { RepoService } from "../services/RepoService"; import { GitAdapter } from "../adapters/GitAdapter"; +import type { FilesService } from "../services/FilesService"; +import type { SearchService } from "../services/SearchService"; import { logger } from "../utils/logger"; import { BackendTranslate, createFallbackBackendTranslator, } from "../i18n/backendTranslator"; import { localizeApiError } from "../i18n/localizeApiError"; +import { + assertBoolean, + assertInteger, + assertOneOf, + assertPlainObject, + assertString, + assertStringArray, +} from "../utils/inputValidation"; + +interface ProfileSwitchServices { + filesService?: Pick; + searchService?: Pick; +} + +const MAX_IPC_PROFILE_NAME_LENGTH = 120; +const MAX_IPC_PROFILE_ID_LENGTH = 256; +const MAX_IPC_PATH_LENGTH = 4096; +const MAX_IPC_URL_LENGTH = 4096; +const MAX_IPC_TOKEN_LENGTH = 10000; +const MAX_IPC_REGION_LENGTH = 128; +const MAX_IPC_BUCKET_LENGTH = 256; +const MAX_IPC_PREFIX_LENGTH = 1024; +const MAX_IPC_FAVORITES_COUNT = 5000; + +const REPO_PROVIDER_VALUES = Object.values(REPO_PROVIDERS); +const AUTH_METHOD_VALUES = Object.values(AuthMethod); +const THEME_VALUES = ["light", "dark", "system"] as const; + +const validateRepoSettingsInput = ( + value: unknown, + field: string, + options: { requireProvider?: boolean } = {}, +): Partial => { + const settings = assertPlainObject(value, field); + const { requireProvider = false } = options; + + if (requireProvider || settings.provider !== undefined) { + assertOneOf(settings.provider, `${field}.provider`, REPO_PROVIDER_VALUES, { + allowEmpty: false, + }); + } + + if (settings.localPath !== undefined) { + assertString(settings.localPath, `${field}.localPath`, { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + } + + if (settings.remoteUrl !== undefined) { + assertString(settings.remoteUrl, `${field}.remoteUrl`, { + allowEmpty: false, + maxLength: MAX_IPC_URL_LENGTH, + }); + } + + if (settings.branch !== undefined) { + assertString(settings.branch, `${field}.branch`, { + allowEmpty: false, + maxLength: 256, + }); + } + + if (settings.pat !== undefined) { + assertString(settings.pat, `${field}.pat`, { + maxLength: MAX_IPC_TOKEN_LENGTH, + }); + } + + if (settings.authMethod !== undefined) { + assertOneOf( + settings.authMethod, + `${field}.authMethod`, + AUTH_METHOD_VALUES, + { + allowEmpty: false, + }, + ); + } + + if (settings.bucket !== undefined) { + assertString(settings.bucket, `${field}.bucket`, { + allowEmpty: false, + maxLength: MAX_IPC_BUCKET_LENGTH, + }); + } + + if (settings.region !== undefined) { + assertString(settings.region, `${field}.region`, { + allowEmpty: false, + maxLength: MAX_IPC_REGION_LENGTH, + }); + } + + if (settings.prefix !== undefined) { + assertString(settings.prefix, `${field}.prefix`, { + maxLength: MAX_IPC_PREFIX_LENGTH, + }); + } + + if (settings.accessKeyId !== undefined) { + assertString(settings.accessKeyId, `${field}.accessKeyId`, { + maxLength: MAX_IPC_TOKEN_LENGTH, + }); + } + + if (settings.secretAccessKey !== undefined) { + assertString(settings.secretAccessKey, `${field}.secretAccessKey`, { + maxLength: MAX_IPC_TOKEN_LENGTH, + }); + } + + if (settings.sessionToken !== undefined) { + assertString(settings.sessionToken, `${field}.sessionToken`, { + maxLength: MAX_IPC_TOKEN_LENGTH, + }); + } + + return settings as Partial; +}; + +const validateAppSettingsInput = (value: unknown): Partial => { + const settings = assertPlainObject(value, "settings"); + + if (settings.language !== undefined) { + assertString(settings.language, "settings.language", { + allowEmpty: false, + maxLength: 32, + }); + } + + if (settings.autoSaveEnabled !== undefined) { + assertBoolean(settings.autoSaveEnabled, "settings.autoSaveEnabled"); + } + + if (settings.autoSaveIntervalSec !== undefined) { + assertInteger( + settings.autoSaveIntervalSec, + "settings.autoSaveIntervalSec", + { + min: 1, + max: 86400, + }, + ); + } + + if (settings.s3AutoSyncEnabled !== undefined) { + assertBoolean(settings.s3AutoSyncEnabled, "settings.s3AutoSyncEnabled"); + } + + if (settings.s3AutoSyncIntervalSec !== undefined) { + assertInteger( + settings.s3AutoSyncIntervalSec, + "settings.s3AutoSyncIntervalSec", + { + min: 1, + max: 86400, + }, + ); + } + + if (settings.theme !== undefined) { + assertOneOf(settings.theme, "settings.theme", THEME_VALUES, { + allowEmpty: false, + }); + } + + if (settings.editorPrefs !== undefined) { + const editorPrefs = assertPlainObject( + settings.editorPrefs, + "settings.editorPrefs", + ); + if (editorPrefs.fontSize !== undefined) { + assertInteger(editorPrefs.fontSize, "settings.editorPrefs.fontSize", { + min: 8, + max: 72, + }); + } + if (editorPrefs.lineNumbers !== undefined) { + assertBoolean( + editorPrefs.lineNumbers, + "settings.editorPrefs.lineNumbers", + ); + } + if (editorPrefs.tabSize !== undefined) { + assertInteger(editorPrefs.tabSize, "settings.editorPrefs.tabSize", { + min: 1, + max: 12, + }); + } + if (editorPrefs.showPreview !== undefined) { + assertBoolean( + editorPrefs.showPreview, + "settings.editorPrefs.showPreview", + ); + } + } + + return settings as Partial; +}; export function registerConfigHandlers( ipcMain: IpcMain, @@ -23,6 +227,7 @@ export function registerConfigHandlers( repoService: RepoService, gitAdapter: GitAdapter, translate: BackendTranslate = createFallbackBackendTranslator(), + profileSwitchServices: ProfileSwitchServices = {}, ): void { const t = ( key: string, @@ -63,7 +268,8 @@ export function registerConfigHandlers( settings: Partial, ): Promise> => { try { - await configService.updateAppSettings(settings); + const validatedSettings = validateAppSettingsInput(settings); + await configService.updateAppSettings(validatedSettings); try { await repoService.refreshAutoSyncSettings(); } catch (error) { @@ -76,14 +282,16 @@ export function registerConfigHandlers( logger.error("Failed to update app settings", { error }); return { ok: false, - error: { - code: ApiErrorCode.UNKNOWN_ERROR, - message: await t( - "config.errors.failedUpdateAppSettings", - "Failed to update app settings", - ), - details: error, - }, + error: error.code + ? await localizeApiError(error, translate) + : { + code: ApiErrorCode.UNKNOWN_ERROR, + message: await t( + "config.errors.failedUpdateAppSettings", + "Failed to update app settings", + ), + details: error, + }, }; } }, @@ -93,7 +301,12 @@ export function registerConfigHandlers( "config:updateRepoSettings", async (_event, settings: RepoSettings): Promise> => { try { - await configService.updateRepoSettings(settings); + const validatedSettings = validateRepoSettingsInput( + settings, + "settings", + { requireProvider: true }, + ) as RepoSettings; + await configService.updateRepoSettings(validatedSettings); return { ok: true, }; @@ -191,7 +404,11 @@ export function registerConfigHandlers( "config:updateFavorites", async (_event, favorites: string[]): Promise> => { try { - await configService.updateFavorites(favorites); + const validatedFavorites = assertStringArray(favorites, "favorites", { + maxItems: MAX_IPC_FAVORITES_COUNT, + itemMaxLength: MAX_IPC_PATH_LENGTH, + }); + await configService.updateFavorites(validatedFavorites); return { ok: true, }; @@ -199,14 +416,16 @@ export function registerConfigHandlers( logger.error("Failed to update favorites", { error }); return { ok: false, - error: { - code: ApiErrorCode.UNKNOWN_ERROR, - message: await t( - "config.errors.failedSaveFavorites", - "Failed to save favorites", - ), - details: error, - }, + error: error.code + ? await localizeApiError(error, translate) + : { + code: ApiErrorCode.UNKNOWN_ERROR, + message: await t( + "config.errors.failedSaveFavorites", + "Failed to save favorites", + ), + details: error, + }, }; } }, @@ -272,7 +491,19 @@ export function registerConfigHandlers( repoSettings: Partial, ): Promise> => { try { - const profile = await configService.createProfile(name, repoSettings); + const validatedName = assertString(name, "name", { + allowEmpty: false, + maxLength: MAX_IPC_PROFILE_NAME_LENGTH, + }); + const validatedRepoSettings = validateRepoSettingsInput( + repoSettings, + "repoSettings", + ); + + const profile = await configService.createProfile( + validatedName, + validatedRepoSettings, + ); logger.info("Profile created, preparing repository", { profileId: profile.id, @@ -338,7 +569,11 @@ export function registerConfigHandlers( "config:deleteProfile", async (_event, profileId: string): Promise> => { try { - await configService.deleteProfile(profileId); + const validatedProfileId = assertString(profileId, "profileId", { + allowEmpty: false, + maxLength: MAX_IPC_PROFILE_ID_LENGTH, + }); + await configService.deleteProfile(validatedProfileId); return { ok: true, }; @@ -346,14 +581,16 @@ export function registerConfigHandlers( logger.error("Failed to delete profile", { error }); return { ok: false, - error: { - code: ApiErrorCode.UNKNOWN_ERROR, - message: await t( - "config.errors.failedDeleteProfile", - "Failed to delete profile", - ), - details: error, - }, + error: error.code + ? await localizeApiError(error, translate) + : { + code: ApiErrorCode.UNKNOWN_ERROR, + message: await t( + "config.errors.failedDeleteProfile", + "Failed to delete profile", + ), + details: error, + }, }; } }, @@ -363,7 +600,32 @@ export function registerConfigHandlers( "config:setActiveProfile", async (_event, profileId: string): Promise> => { try { - await configService.setActiveProfileId(profileId); + const validatedProfileId = assertString(profileId, "profileId", { + allowEmpty: false, + maxLength: MAX_IPC_PROFILE_ID_LENGTH, + }); + await configService.setActiveProfileId(validatedProfileId); + repoService.resetActiveRepo?.(); + profileSwitchServices.filesService?.reset(); + profileSwitchServices.searchService?.reset(); + + try { + const repoSettings = await configService.getRepoSettings(); + if (repoSettings?.localPath) { + profileSwitchServices.searchService?.setRepoPath( + repoSettings.localPath, + ); + } + } catch (error) { + logger.warn( + "Failed to refresh search repo path after profile switch", + { + error, + profileId: validatedProfileId, + }, + ); + } + return { ok: true, }; @@ -371,14 +633,16 @@ export function registerConfigHandlers( logger.error("Failed to set active profile", { error }); return { ok: false, - error: { - code: ApiErrorCode.UNKNOWN_ERROR, - message: await t( - "config.errors.failedSetActiveProfile", - "Failed to set active profile", - ), - details: error, - }, + error: error.code + ? await localizeApiError(error, translate) + : { + code: ApiErrorCode.UNKNOWN_ERROR, + message: await t( + "config.errors.failedSetActiveProfile", + "Failed to set active profile", + ), + details: error, + }, }; } }, diff --git a/app/desktop/src/backend/handlers/filesHandlers.ts b/app/desktop/src/backend/handlers/filesHandlers.ts index 5672774..ad286c2 100644 --- a/app/desktop/src/backend/handlers/filesHandlers.ts +++ b/app/desktop/src/backend/handlers/filesHandlers.ts @@ -16,6 +16,12 @@ import { createFallbackBackendTranslator, } from "../i18n/backendTranslator"; import { localizeApiError } from "../i18n/localizeApiError"; +import { assertBoolean, assertString } from "../utils/inputValidation"; + +const MAX_IPC_PATH_LENGTH = 4096; +const MAX_IPC_NAME_LENGTH = 255; +const MAX_IPC_COMMIT_MESSAGE_LENGTH = 1000; +const MAX_IPC_CONTENT_LENGTH = 5_000_000; export function registerFilesHandlers( ipcMain: IpcMain, @@ -58,7 +64,11 @@ export function registerFilesHandlers( "files:read", async (_event, path: string): Promise> => { try { - const content = await filesService.readFile(path); + const validatedPath = assertString(path, "path", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const content = await filesService.readFile(validatedPath); return { ok: true, data: content, @@ -87,9 +97,20 @@ export function registerFilesHandlers( content: string, ): Promise> => { try { - await filesService.saveFile(path, content); - void repoService.queueS3Upload(path).catch((error) => { - logger.warn("Failed to queue S3 upload operation", { path, error }); + const validatedPath = assertString(path, "path", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedContent = assertString(content, "content", { + maxLength: MAX_IPC_CONTENT_LENGTH, + }); + + await filesService.saveFile(validatedPath, validatedContent); + void repoService.queueS3Upload(validatedPath).catch((error) => { + logger.warn("Failed to queue S3 upload operation", { + path: validatedPath, + error, + }); }); return { ok: true, @@ -125,10 +146,19 @@ export function registerFilesHandlers( }> > => { try { + const validatedPath = assertString(path, "path", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedContent = assertString(content, "content", { + maxLength: MAX_IPC_CONTENT_LENGTH, + }); + const validatedIsAutosave = assertBoolean(isAutosave, "isAutosave"); + const result = await filesService.saveWithGitWorkflow( - path, - content, - isAutosave, + validatedPath, + validatedContent, + validatedIsAutosave, ); return { ok: true, @@ -163,7 +193,16 @@ export function registerFilesHandlers( message: string, ): Promise> => { try { - await filesService.commitFile(path, message); + const validatedPath = assertString(path, "path", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedMessage = assertString(message, "message", { + allowEmpty: false, + maxLength: MAX_IPC_COMMIT_MESSAGE_LENGTH, + }); + + await filesService.commitFile(validatedPath, validatedMessage); return { ok: true, }; @@ -187,7 +226,11 @@ export function registerFilesHandlers( "files:commitAll", async (_event, message: string): Promise> => { try { - await filesService.commitAll(message); + const validatedMessage = assertString(message, "message", { + allowEmpty: false, + maxLength: MAX_IPC_COMMIT_MESSAGE_LENGTH, + }); + await filesService.commitAll(validatedMessage); return { ok: true, }; @@ -344,7 +387,15 @@ export function registerFilesHandlers( name: string, ): Promise> => { try { - await filesService.createFile(parentPath, name); + const validatedParentPath = assertString(parentPath, "parentPath", { + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedName = assertString(name, "name", { + allowEmpty: false, + maxLength: MAX_IPC_NAME_LENGTH, + }); + + await filesService.createFile(validatedParentPath, validatedName); return { ok: true, }; @@ -372,7 +423,15 @@ export function registerFilesHandlers( name: string, ): Promise> => { try { - await filesService.createFolder(parentPath, name); + const validatedParentPath = assertString(parentPath, "parentPath", { + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedName = assertString(name, "name", { + allowEmpty: false, + maxLength: MAX_IPC_NAME_LENGTH, + }); + + await filesService.createFolder(validatedParentPath, validatedName); return { ok: true, }; @@ -401,9 +460,17 @@ export function registerFilesHandlers( "files:delete", async (_event, path: string): Promise> => { try { - await filesService.deletePath(path); - void repoService.queueS3Delete(path).catch((error) => { - logger.warn("Failed to queue S3 delete operation", { path, error }); + const validatedPath = assertString(path, "path", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + + await filesService.deletePath(validatedPath); + void repoService.queueS3Delete(validatedPath).catch((error) => { + logger.warn("Failed to queue S3 delete operation", { + path: validatedPath, + error, + }); }); return { ok: true, @@ -432,14 +499,25 @@ export function registerFilesHandlers( newPath: string, ): Promise> => { try { - await filesService.renamePath(oldPath, newPath); - void repoService.queueS3Move(oldPath, newPath).catch((error) => { - logger.warn("Failed to queue S3 move operation", { - oldPath, - newPath, - error, - }); + const validatedOldPath = assertString(oldPath, "oldPath", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedNewPath = assertString(newPath, "newPath", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, }); + + await filesService.renamePath(validatedOldPath, validatedNewPath); + void repoService + .queueS3Move(validatedOldPath, validatedNewPath) + .catch((error) => { + logger.warn("Failed to queue S3 move operation", { + oldPath: validatedOldPath, + newPath: validatedNewPath, + error, + }); + }); return { ok: true, }; @@ -467,7 +545,16 @@ export function registerFilesHandlers( destPath: string, ): Promise> => { try { - await filesService.saveFileAs(repoPath, destPath); + const validatedRepoPath = assertString(repoPath, "repoPath", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedDestPath = assertString(destPath, "destPath", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + + await filesService.saveFileAs(validatedRepoPath, validatedDestPath); return { ok: true, }; @@ -491,7 +578,12 @@ export function registerFilesHandlers( "files:duplicate", async (_event, repoPath: string): Promise> => { try { - const duplicatedPath = await filesService.duplicateFile(repoPath); + const validatedRepoPath = assertString(repoPath, "repoPath", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const duplicatedPath = + await filesService.duplicateFile(validatedRepoPath); return { ok: true, data: duplicatedPath, @@ -525,7 +617,16 @@ export function registerFilesHandlers( targetPath: string, ): Promise> => { try { - await filesService.importFile(sourcePath, targetPath); + const validatedSourcePath = assertString(sourcePath, "sourcePath", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + const validatedTargetPath = assertString(targetPath, "targetPath", { + allowEmpty: false, + maxLength: MAX_IPC_PATH_LENGTH, + }); + + await filesService.importFile(validatedSourcePath, validatedTargetPath); return { ok: true, }; diff --git a/app/desktop/src/backend/handlers/searchHandlers.ts b/app/desktop/src/backend/handlers/searchHandlers.ts index ac8c72d..0398861 100644 --- a/app/desktop/src/backend/handlers/searchHandlers.ts +++ b/app/desktop/src/backend/handlers/searchHandlers.ts @@ -11,6 +11,19 @@ import { createFallbackBackendTranslator, } from "../i18n/backendTranslator"; import { localizeApiError } from "../i18n/localizeApiError"; +import { + assertOptionalBoolean, + assertOptionalInteger, + assertOptionalPlainObject, + assertOptionalStringArray, + assertString, +} from "../utils/inputValidation"; + +const MAX_IPC_SEARCH_QUERY_LENGTH = 5000; +const MAX_IPC_SEARCH_REPLACEMENT_LENGTH = 100000; +const MAX_IPC_SEARCH_FILE_PATH_LENGTH = 4096; +const MAX_IPC_SEARCH_REPLACE_PATH_COUNT = 5000; +const MAX_IPC_SEARCH_RESULTS_LIMIT = 500; export function registerSearchHandlers( ipcMain: IpcMain, @@ -31,7 +44,19 @@ export function registerSearchHandlers( options?: { maxResults?: number }, ): Promise> => { try { - const results = await searchService.search(query, options); + const validatedQuery = assertString(query, "query", { + maxLength: MAX_IPC_SEARCH_QUERY_LENGTH, + }); + const rawOptions = assertOptionalPlainObject(options, "options"); + const validatedMaxResults = assertOptionalInteger( + rawOptions?.maxResults, + "options.maxResults", + { min: 1, max: MAX_IPC_SEARCH_RESULTS_LIMIT }, + ); + + const results = await searchService.search(validatedQuery, { + maxResults: validatedMaxResults, + }); return { ok: true, data: results, @@ -61,7 +86,23 @@ export function registerSearchHandlers( options?: { caseSensitive?: boolean; useRegex?: boolean }, ): Promise> => { try { - const results = await searchService.searchRepoWide(query, options); + const validatedQuery = assertString(query, "query", { + maxLength: MAX_IPC_SEARCH_QUERY_LENGTH, + }); + const rawOptions = assertOptionalPlainObject(options, "options"); + const caseSensitive = assertOptionalBoolean( + rawOptions?.caseSensitive, + "options.caseSensitive", + ); + const useRegex = assertOptionalBoolean( + rawOptions?.useRegex, + "options.useRegex", + ); + + const results = await searchService.searchRepoWide(validatedQuery, { + caseSensitive, + useRegex, + }); return { ok: true, data: results }; } catch (error: any) { logger.error("Failed to perform repo-wide search", { query, error }); @@ -97,10 +138,38 @@ export function registerSearchHandlers( }, ): Promise> => { try { + const validatedQuery = assertString(query, "query", { + maxLength: MAX_IPC_SEARCH_QUERY_LENGTH, + }); + const validatedReplacement = assertString(replacement, "replacement", { + maxLength: MAX_IPC_SEARCH_REPLACEMENT_LENGTH, + }); + const rawOptions = assertOptionalPlainObject(options, "options"); + const caseSensitive = assertOptionalBoolean( + rawOptions?.caseSensitive, + "options.caseSensitive", + ); + const useRegex = assertOptionalBoolean( + rawOptions?.useRegex, + "options.useRegex", + ); + const filePaths = assertOptionalStringArray( + rawOptions?.filePaths, + "options.filePaths", + { + maxItems: MAX_IPC_SEARCH_REPLACE_PATH_COUNT, + itemMaxLength: MAX_IPC_SEARCH_FILE_PATH_LENGTH, + }, + ); + const result = await searchService.replaceInRepo( - query, - replacement, - options, + validatedQuery, + validatedReplacement, + { + caseSensitive, + useRegex, + filePaths, + }, ); return { ok: true, data: result }; } catch (error: any) { diff --git a/app/desktop/src/backend/index.ts b/app/desktop/src/backend/index.ts index 6cfe283..b61310d 100644 --- a/app/desktop/src/backend/index.ts +++ b/app/desktop/src/backend/index.ts @@ -109,11 +109,18 @@ export async function createBackend(ipcMain: IpcMain): Promise { }); } - configService.getFull().then((config) => { - if (config?.repoSettings?.localPath) { - searchService.setRepoPath(config.repoSettings.localPath); - } - }); + configService + .getFull() + .then((config) => { + if (config?.repoSettings?.localPath) { + searchService.setRepoPath(config.repoSettings.localPath); + } + }) + .catch((error) => { + logger.warn("Failed to pre-initialize search service from config", { + error, + }); + }); registerConfigHandlers( ipcMain, @@ -121,6 +128,7 @@ export async function createBackend(ipcMain: IpcMain): Promise { repoService, gitAdapter, translate, + { filesService, searchService }, ); registerRepoHandlers(ipcMain, repoService, translate); registerFilesHandlers(ipcMain, filesService, repoService, translate); diff --git a/app/desktop/src/backend/providers/GitRepoProvider.ts b/app/desktop/src/backend/providers/GitRepoProvider.ts index 3254d0e..f501072 100644 --- a/app/desktop/src/backend/providers/GitRepoProvider.ts +++ b/app/desktop/src/backend/providers/GitRepoProvider.ts @@ -161,14 +161,24 @@ export class GitRepoProvider implements RepoProvider { await this.performPullThenPush(); } - startAutoSync(_intervalMs?: number): void { + startAutoSync(intervalMs?: number): void { + const effectiveIntervalMs = + typeof intervalMs === "number" && + Number.isFinite(intervalMs) && + intervalMs > 0 + ? intervalMs + : this.AUTO_SYNC_INTERVAL; + if (this.autoSyncTimer) { - logger.debug("Git auto-sync timer already running"); - return; + logger.debug("Restarting git auto-sync timer", { + intervalMs: effectiveIntervalMs, + }); + clearInterval(this.autoSyncTimer); + this.autoSyncTimer = null; } logger.info("Starting git auto-sync timer", { - intervalMs: this.AUTO_SYNC_INTERVAL, + intervalMs: effectiveIntervalMs, }); this.autoSyncTimer = setInterval(async () => { @@ -177,7 +187,7 @@ export class GitRepoProvider implements RepoProvider { } catch (error) { logger.debug("Git auto-sync attempt failed, will retry", { error }); } - }, this.AUTO_SYNC_INTERVAL); + }, effectiveIntervalMs); } stopAutoSync(): void { diff --git a/app/desktop/src/backend/providers/S3RepoProvider.ts b/app/desktop/src/backend/providers/S3RepoProvider.ts index be3aee7..fb91e15 100644 --- a/app/desktop/src/backend/providers/S3RepoProvider.ts +++ b/app/desktop/src/backend/providers/S3RepoProvider.ts @@ -46,6 +46,12 @@ type RemoteFileInfo = { lastModifiedMs: number; }; +type PendingSyncRequest = { + mode: SyncMode; + resolve: () => void; + reject: (error: unknown) => void; +}; + export class S3RepoProvider implements RepoProvider { readonly type = REPO_PROVIDERS.s3; private readonly conflictSuffix = "s3-conflict"; @@ -59,8 +65,7 @@ export class S3RepoProvider implements RepoProvider { private lastSyncTime: Date | null = null; private syncInProgress = false; private pendingSyncTimer: NodeJS.Timeout | null = null; - private pendingSyncRequested = false; - private pendingSyncMode: SyncMode | null = null; + private pendingSyncRequests: PendingSyncRequest[] = []; private syncCompletionPromise: Promise | null = null; private resolveSyncCompletion: (() => void) | null = null; private rejectSyncCompletion: ((error: unknown) => void) | null = null; @@ -204,14 +209,14 @@ export class S3RepoProvider implements RepoProvider { private async sync(mode: SyncMode = "sync"): Promise { if (this.syncInProgress) { - this.queuePendingSync(mode); - await this.waitForSyncCycleCompletion(); + await this.queuePendingSync(mode); return; } this.syncInProgress = true; this.beginSyncCycle(); let currentMode: SyncMode = mode; + let activeQueuedRequest: PendingSyncRequest | null = null; let syncError: unknown = null; try { @@ -219,21 +224,26 @@ export class S3RepoProvider implements RepoProvider { while (continueSync) { await this.performSync(currentMode); - if (!this.pendingSyncRequested) { + if (activeQueuedRequest) { + activeQueuedRequest.resolve(); + activeQueuedRequest = null; + } + + const nextRequest = this.pendingSyncRequests.shift(); + if (!nextRequest) { continueSync = false; continue; } - currentMode = this.pendingSyncMode || currentMode; - this.pendingSyncRequested = false; - this.pendingSyncMode = null; + activeQueuedRequest = nextRequest; + currentMode = nextRequest.mode; } } catch (error) { syncError = error; + activeQueuedRequest?.reject(error); + this.rejectPendingSyncRequests(error); throw error; } finally { - this.pendingSyncRequested = false; - this.pendingSyncMode = null; this.syncInProgress = false; this.endSyncCycle(syncError); } @@ -268,10 +278,17 @@ export class S3RepoProvider implements RepoProvider { logger.info(`S3 ${mode} completed`, { updatedAt: this.lastSyncTime }); } - private queuePendingSync(mode: SyncMode): void { - this.pendingSyncRequested = true; - this.pendingSyncMode = - this.pendingSyncMode === "sync" || mode === "sync" ? "sync" : "pull"; + private queuePendingSync(mode: SyncMode): Promise { + return new Promise((resolve, reject) => { + this.pendingSyncRequests.push({ mode, resolve, reject }); + }); + } + + private rejectPendingSyncRequests(error: unknown): void { + const pendingRequests = this.pendingSyncRequests.splice(0); + for (const request of pendingRequests) { + request.reject(error); + } } private beginSyncCycle(): void { @@ -298,10 +315,11 @@ export class S3RepoProvider implements RepoProvider { } private async waitForSyncCycleCompletion(): Promise { - if (!this.syncCompletionPromise) { + const promise = this.syncCompletionPromise; + if (!promise) { return; } - await this.syncCompletionPromise; + await promise; } private async requestSync(): Promise { diff --git a/app/desktop/src/backend/services/ExportService.ts b/app/desktop/src/backend/services/ExportService.ts index b223111..c04d160 100644 --- a/app/desktop/src/backend/services/ExportService.ts +++ b/app/desktop/src/backend/services/ExportService.ts @@ -16,6 +16,15 @@ import { createFallbackBackendTranslator, } from "../i18n/backendTranslator"; +const ZIP_EXPORT_IGNORED_PATHS = [ + ".git/**", + ".git", + ".NoteBranch/**", + ".NoteBranch", + "node_modules/**", + "node_modules", +]; + export class ExportService { private repoPath: string | null = null; private translate: BackendTranslate; @@ -205,7 +214,7 @@ export class ExportService { archive.glob("**/*", { cwd: sourcePath, - ignore: [".git/**", "node_modules/**"], + ignore: ZIP_EXPORT_IGNORED_PATHS, dot: true, }); diff --git a/app/desktop/src/backend/services/FilesService.ts b/app/desktop/src/backend/services/FilesService.ts index eb89501..95485c7 100644 --- a/app/desktop/src/backend/services/FilesService.ts +++ b/app/desktop/src/backend/services/FilesService.ts @@ -15,6 +15,8 @@ import { import { logger } from "../utils/logger"; import { validateRepoPath } from "../utils/pathValidation"; +const MAX_DUPLICATE_FILE_ATTEMPTS = 1000; + export class FilesService { private repoPath: string | null = null; private gitAdapter: GitAdapter | null = null; @@ -29,6 +31,11 @@ export class FilesService { this.gitAdapter = gitAdapter; } + reset(): void { + this.repoPath = null; + this.repoProvider = null; + } + async init(): Promise { const repoSettings = await this.configService.getRepoSettings(); if (repoSettings?.localPath) { @@ -391,6 +398,16 @@ export class FilesService { let fullNewPath = this.resolveValidatedRepoPath(newRelativePath); while (await this.fsAdapter.exists(fullNewPath)) { + if (counter >= MAX_DUPLICATE_FILE_ATTEMPTS) { + throw this.createError( + ApiErrorCode.UNKNOWN_ERROR, + "Too many duplicate filename attempts", + { + source: normalizedPath, + maxAttempts: MAX_DUPLICATE_FILE_ATTEMPTS, + }, + ); + } counter += 1; newRelativePath = path.join(dir, `${name}(${counter})${ext}`); fullNewPath = this.resolveValidatedRepoPath(newRelativePath); diff --git a/app/desktop/src/backend/services/RepoService.ts b/app/desktop/src/backend/services/RepoService.ts index eba8183..a346970 100644 --- a/app/desktop/src/backend/services/RepoService.ts +++ b/app/desktop/src/backend/services/RepoService.ts @@ -49,6 +49,21 @@ export class RepoService { this.filesService = filesService; } + resetActiveRepo(): void { + if (this.activeProvider) { + try { + this.activeProvider.stopAutoSync(); + } catch (error) { + logger.warn("Failed to stop auto-sync while resetting repository", { + error, + }); + } + } + + this.activeProvider = null; + this.activeSettings = null; + } + async openOrClone(settings: RepoSettings): Promise { return await this.openRepo(settings, { updateConfig: true, @@ -344,7 +359,7 @@ export class RepoService { } destroy(): void { - this.stopAutoPush(); + this.resetActiveRepo(); } async refreshAutoSyncSettings(): Promise { diff --git a/app/desktop/src/backend/services/SearchService.ts b/app/desktop/src/backend/services/SearchService.ts index 80f82a0..b363152 100644 --- a/app/desktop/src/backend/services/SearchService.ts +++ b/app/desktop/src/backend/services/SearchService.ts @@ -30,6 +30,11 @@ export class SearchService { logger.info("Search service repo path set", { repoPath }); } + reset(): void { + this.repoPath = null; + logger.info("Search service repo path reset"); + } + private createError( code: ApiErrorCode, message: string, @@ -216,6 +221,7 @@ export class SearchService { const results: RepoWideSearchResult[] = []; const caseSensitive = options?.caseSensitive || false; const useRegex = options?.useRegex || false; + const regex = useRegex ? this.createSafeRegex(query, caseSensitive) : null; try { const files = await this.getAllMarkdownFiles(this.repoPath!); @@ -235,6 +241,7 @@ export class SearchService { query, caseSensitive, useRegex, + regex, ); for (const match of matches) { @@ -276,6 +283,9 @@ export class SearchService { return results; } catch (error: any) { + if (error?.code === ApiErrorCode.VALIDATION_ERROR) { + throw error; + } logger.error("Repo-wide search failed", { error }); throw this.createError( ApiErrorCode.UNKNOWN_ERROR, @@ -304,6 +314,7 @@ export class SearchService { const caseSensitive = options?.caseSensitive || false; const useRegex = options?.useRegex || false; + const regex = useRegex ? this.createSafeRegex(query, caseSensitive) : null; let filesToProcess: string[] = []; try { @@ -333,6 +344,7 @@ export class SearchService { replacement, caseSensitive, useRegex, + regex, ); if (newContent !== content) { @@ -344,6 +356,7 @@ export class SearchService { query, caseSensitive, useRegex, + regex, ); result.totalReplacements += matches.length; @@ -449,23 +462,22 @@ export class SearchService { query: string, caseSensitive: boolean, useRegex: boolean, + regex: RegExp | null = null, ): { start: number; end: number }[] { const matches: { start: number; end: number }[] = []; if (useRegex) { - try { - const flags = caseSensitive ? "g" : "gi"; - const regex = new RegExp(query, flags); - let match; - - while ((match = regex.exec(line)) !== null) { - matches.push({ - start: match.index, - end: match.index + match[0].length, - }); - } - } catch (error) { - return this.findMatchesInLine(line, query, caseSensitive, false); + if (!regex) { + return matches; + } + regex.lastIndex = 0; + let match: RegExpExecArray | null; + + while ((match = regex.exec(line)) !== null) { + matches.push({ + start: match.index, + end: match.index + match[0].length, + }); } } else { const searchLine = caseSensitive ? line : line.toLowerCase(); @@ -489,23 +501,22 @@ export class SearchService { query: string, caseSensitive: boolean, useRegex: boolean, + regex: RegExp | null = null, ): { start: number; end: number }[] { const matches: { start: number; end: number }[] = []; if (useRegex) { - try { - const flags = caseSensitive ? "g" : "gi"; - const regex = new RegExp(query, flags); - let match; - - while ((match = regex.exec(content)) !== null) { - matches.push({ - start: match.index, - end: match.index + match[0].length, - }); - } - } catch (error) { - return this.findMatchesInContent(content, query, caseSensitive, false); + if (!regex) { + return matches; + } + regex.lastIndex = 0; + let match: RegExpExecArray | null; + + while ((match = regex.exec(content)) !== null) { + matches.push({ + start: match.index, + end: match.index + match[0].length, + }); } } else { const searchContent = caseSensitive ? content : content.toLowerCase(); @@ -530,21 +541,14 @@ export class SearchService { replacement: string, caseSensitive: boolean, useRegex: boolean, + regex: RegExp | null = null, ): string { if (useRegex) { - try { - const flags = caseSensitive ? "g" : "gi"; - const regex = new RegExp(query, flags); - return content.replace(regex, replacement); - } catch (error) { - return this.replaceInContent( - content, - query, - replacement, - caseSensitive, - false, - ); + if (!regex) { + return content; } + regex.lastIndex = 0; + return content.replace(regex, replacement); } else { if (caseSensitive) { return content.split(query).join(replacement); @@ -557,6 +561,90 @@ export class SearchService { } } } + + private createSafeRegex(query: string, caseSensitive: boolean): RegExp { + if (this.hasNestedQuantifiedGroup(query)) { + throw this.createError( + ApiErrorCode.VALIDATION_ERROR, + "Unsafe regex pattern rejected", + { query, reason: "nestedQuantifiedGroup" }, + ); + } + + const flags = caseSensitive ? "g" : "gi"; + try { + return new RegExp(query, flags); + } catch (error: any) { + throw this.createError( + ApiErrorCode.VALIDATION_ERROR, + `Invalid regex pattern: ${error?.message || query}`, + { query, error }, + ); + } + } + + private hasNestedQuantifiedGroup(pattern: string): boolean { + const quantifierChars = new Set(["+", "*", "?", "{"]); + const stack: Array<{ hasInnerQuantifier: boolean }> = []; + let escaped = false; + let inCharClass = false; + + for (let i = 0; i < pattern.length; i++) { + const char = pattern[i]; + + if (escaped) { + escaped = false; + continue; + } + + if (char === "\\") { + escaped = true; + continue; + } + + if (inCharClass) { + if (char === "]") { + inCharClass = false; + } + continue; + } + + if (char === "[") { + inCharClass = true; + continue; + } + + if (char === "(") { + stack.push({ hasInnerQuantifier: false }); + continue; + } + + if (char === ")") { + const currentGroup = stack.pop(); + if (!currentGroup) { + continue; + } + + const next = pattern[i + 1]; + const groupIsQuantified = + typeof next === "string" && quantifierChars.has(next); + if (currentGroup.hasInnerQuantifier && groupIsQuantified) { + return true; + } + + if (stack.length > 0 && groupIsQuantified) { + stack[stack.length - 1].hasInnerQuantifier = true; + } + continue; + } + + if (quantifierChars.has(char) && stack.length > 0) { + stack[stack.length - 1].hasInnerQuantifier = true; + } + } + + return false; + } } export interface RepoWideSearchResult { diff --git a/app/desktop/src/backend/utils/inputValidation.ts b/app/desktop/src/backend/utils/inputValidation.ts new file mode 100644 index 0000000..2623c68 --- /dev/null +++ b/app/desktop/src/backend/utils/inputValidation.ts @@ -0,0 +1,192 @@ +import { ApiErrorCode } from "../../shared/types"; + +export interface ValidationOptions { + allowEmpty?: boolean; + minLength?: number; + maxLength?: number; +} + +const isPlainObject = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +const createValidationError = (message: string, field: string) => ({ + code: ApiErrorCode.VALIDATION_ERROR, + message, + details: { field }, +}); + +export const assertString = ( + value: unknown, + field: string, + options: ValidationOptions = {}, +): string => { + if (typeof value !== "string") { + throw createValidationError(`${field} must be a string`, field); + } + + const { allowEmpty = true, minLength, maxLength } = options; + + if (!allowEmpty && value.trim().length === 0) { + throw createValidationError(`${field} cannot be empty`, field); + } + + if (typeof minLength === "number" && value.length < minLength) { + throw createValidationError( + `${field} must be at least ${minLength} characters`, + field, + ); + } + + if (typeof maxLength === "number" && value.length > maxLength) { + throw createValidationError( + `${field} exceeds maximum length of ${maxLength}`, + field, + ); + } + + return value; +}; + +export const assertBoolean = (value: unknown, field: string): boolean => { + if (typeof value !== "boolean") { + throw createValidationError(`${field} must be a boolean`, field); + } + return value; +}; + +export const assertOptionalBoolean = ( + value: unknown, + field: string, +): boolean | undefined => { + if (value === undefined) { + return undefined; + } + return assertBoolean(value, field); +}; + +export const assertInteger = ( + value: unknown, + field: string, + options: { min?: number; max?: number } = {}, +): number => { + if (typeof value !== "number" || !Number.isFinite(value)) { + throw createValidationError(`${field} must be a number`, field); + } + if (!Number.isInteger(value)) { + throw createValidationError(`${field} must be an integer`, field); + } + + if (typeof options.min === "number" && value < options.min) { + throw createValidationError(`${field} must be >= ${options.min}`, field); + } + + if (typeof options.max === "number" && value > options.max) { + throw createValidationError(`${field} must be <= ${options.max}`, field); + } + + return value; +}; + +export const assertOptionalInteger = ( + value: unknown, + field: string, + options: { min?: number; max?: number } = {}, +): number | undefined => { + if (value === undefined) { + return undefined; + } + return assertInteger(value, field, options); +}; + +export const assertPlainObject = ( + value: unknown, + field: string, +): Record => { + if (!isPlainObject(value)) { + throw createValidationError(`${field} must be an object`, field); + } + return value; +}; + +export const assertOptionalPlainObject = ( + value: unknown, + field: string, +): Record | undefined => { + if (value === undefined) { + return undefined; + } + return assertPlainObject(value, field); +}; + +export const assertStringArray = ( + value: unknown, + field: string, + options: { + allowEmptyItems?: boolean; + maxItems?: number; + itemMaxLength?: number; + } = {}, +): string[] => { + if (!Array.isArray(value)) { + throw createValidationError(`${field} must be an array`, field); + } + + const { allowEmptyItems = true, maxItems, itemMaxLength } = options; + + if (typeof maxItems === "number" && value.length > maxItems) { + throw createValidationError( + `${field} exceeds maximum item count of ${maxItems}`, + field, + ); + } + + return value.map((item, index) => + assertString(item, `${field}[${index}]`, { + allowEmpty: allowEmptyItems, + maxLength: itemMaxLength, + }), + ); +}; + +export const assertOptionalStringArray = ( + value: unknown, + field: string, + options: { + allowEmptyItems?: boolean; + maxItems?: number; + itemMaxLength?: number; + } = {}, +): string[] | undefined => { + if (value === undefined) { + return undefined; + } + return assertStringArray(value, field, options); +}; + +export const assertOneOf = ( + value: unknown, + field: string, + allowed: readonly T[], + options: ValidationOptions = {}, +): T => { + const normalized = assertString(value, field, options); + if (!allowed.includes(normalized as T)) { + throw createValidationError( + `${field} must be one of: ${allowed.join(", ")}`, + field, + ); + } + return normalized as T; +}; + +export const assertOptionalOneOf = ( + value: unknown, + field: string, + allowed: readonly T[], + options: ValidationOptions = {}, +): T | undefined => { + if (value === undefined) { + return undefined; + } + return assertOneOf(value, field, allowed, options); +}; diff --git a/app/desktop/src/backend/utils/resolveTutorialsRootDir.ts b/app/desktop/src/backend/utils/resolveTutorialsRootDir.ts index 86b6999..2cd56c2 100644 --- a/app/desktop/src/backend/utils/resolveTutorialsRootDir.ts +++ b/app/desktop/src/backend/utils/resolveTutorialsRootDir.ts @@ -3,7 +3,7 @@ import * as path from "path"; const REQUIRED_TUTORIAL_SENTINELS = [ path.join( "scenarios", - "create-repo-on-NoteBranch", + "create-repo-on-notegit", "images", "step-01-welcome-screen.png", ), diff --git a/app/desktop/src/electron/main.ts b/app/desktop/src/electron/main.ts index ee8e858..7c90f4d 100644 --- a/app/desktop/src/electron/main.ts +++ b/app/desktop/src/electron/main.ts @@ -20,6 +20,7 @@ const buildWindowTitle = () => `NoteBranch - ${app.getVersion()}`; const SOURCE_CODE_URL = "https://github.com/scabir/notebranch"; const USER_GUIDE_URL = `${SOURCE_CODE_URL}/blob/main/docs/USER_GUIDE.md`; const isIntegrationTestMode = process.env.NOTEBRANCH_INTEGRATION_TEST === "1"; +const SAVE_BEFORE_CLOSE_TIMEOUT_MS = 5000; const configureIntegrationUserDataPath = () => { if (!isIntegrationTestMode) { @@ -44,7 +45,7 @@ const configureIntegrationUserDataPath = () => { configureIntegrationUserDataPath(); const sendMenuCommand = ( - channel: "menu:open-shortcuts" | "menu:open-about", + channel: "menu:open-shortcuts" | "menu:open-about" | "menu:open-find-in-file", ) => { mainWindow?.webContents.send(channel); }; @@ -92,6 +93,7 @@ const buildAppMenu = (): MenuItemConstructorOptions[] => { function createWindow() { const isDevelopment = process.env.NODE_ENV === "development"; const rendererEntryUrl = getRendererEntryUrl(isDevelopment, __dirname); + let isClosingAfterSave = false; mainWindow = new BrowserWindow({ width: 1200, @@ -144,7 +146,57 @@ function createWindow() { mainWindow?.setTitle(buildWindowTitle()); }); + mainWindow.webContents.on("before-input-event", (event, input) => { + const key = input.key.toLowerCase(); + const isFindShortcut = + (input.meta || input.control) && !input.shift && key === "f"; + if (!isFindShortcut) { + return; + } + + event.preventDefault(); + sendMenuCommand("menu:open-find-in-file"); + }); + + mainWindow.on("close", (event) => { + if (isClosingAfterSave) { + return; + } + + event.preventDefault(); + const windowToClose = mainWindow; + if (!windowToClose || windowToClose.isDestroyed()) { + return; + } + + void (async () => { + try { + const saveBeforeCloseTask = windowToClose.webContents.executeJavaScript( + "window.__NOTE_BRANCH_SAVE_BEFORE_CLOSE__ ? window.__NOTE_BRANCH_SAVE_BEFORE_CLOSE__() : Promise.resolve(false)", + true, + ); + await Promise.race([ + saveBeforeCloseTask, + new Promise((resolve) => + setTimeout(resolve, SAVE_BEFORE_CLOSE_TIMEOUT_MS), + ), + ]); + } catch (error) { + console.error( + "Failed to run save-before-close handler in renderer", + error, + ); + } finally { + if (!windowToClose.isDestroyed()) { + isClosingAfterSave = true; + windowToClose.close(); + } + } + })(); + }); + mainWindow.on("closed", () => { + isClosingAfterSave = false; mainWindow = null; }); } diff --git a/app/desktop/src/electron/preload.ts b/app/desktop/src/electron/preload.ts index 94541e9..9b95861 100644 --- a/app/desktop/src/electron/preload.ts +++ b/app/desktop/src/electron/preload.ts @@ -3,6 +3,7 @@ import type { NoteBranchApi } from "../shared/types/api"; const openShortcutsListeners = new Set<() => void>(); const openAboutListeners = new Set<() => void>(); +const openFindInFileListeners = new Set<() => void>(); ipcRenderer.on("menu:open-shortcuts", () => { openShortcutsListeners.forEach((listener) => listener()); @@ -12,6 +13,10 @@ ipcRenderer.on("menu:open-about", () => { openAboutListeners.forEach((listener) => listener()); }); +ipcRenderer.on("menu:open-find-in-file", () => { + openFindInFileListeners.forEach((listener) => listener()); +}); + const api: NoteBranchApi = { menu: { onOpenShortcuts: (listener) => { @@ -26,6 +31,12 @@ const api: NoteBranchApi = { openAboutListeners.delete(listener); }; }, + onOpenFindInFile: (listener) => { + openFindInFileListeners.add(listener); + return () => { + openFindInFileListeners.delete(listener); + }; + }, }, config: { getFull: () => ipcRenderer.invoke("config:getFull"), @@ -71,8 +82,6 @@ const api: NoteBranchApi = { ipcRenderer.invoke("files:commit", path, message), commitAll: (message) => ipcRenderer.invoke("files:commitAll", message), commitAndPushAll: () => ipcRenderer.invoke("files:commitAndPushAll"), - create: (parentPath, name) => - ipcRenderer.invoke("files:create", parentPath, name), createFile: (parentPath, name) => ipcRenderer.invoke("files:create", parentPath, name), createFolder: (parentPath, name) => diff --git a/app/desktop/src/frontend/components/EditorShell/constants.ts b/app/desktop/src/frontend/components/EditorShell/constants.ts index 4999d00..fb254e5 100644 --- a/app/desktop/src/frontend/components/EditorShell/constants.ts +++ b/app/desktop/src/frontend/components/EditorShell/constants.ts @@ -1,4 +1,5 @@ export const PROFILE_NAME_LIMIT = 20; +export const DEFAULT_AUTOSAVE_INTERVAL_SEC = 30; export const SIDEBAR_DEFAULT_WIDTH = 300; export const SIDEBAR_COLLAPSED_WIDTH = 0; export const SIDEBAR_MIN_WIDTH = 200; diff --git a/app/desktop/src/frontend/components/EditorShell/index.tsx b/app/desktop/src/frontend/components/EditorShell/index.tsx index 8f6c1fe..d4a5b16 100644 --- a/app/desktop/src/frontend/components/EditorShell/index.tsx +++ b/app/desktop/src/frontend/components/EditorShell/index.tsx @@ -34,6 +34,7 @@ import { editorPaneSx, } from "./styles"; import { + DEFAULT_AUTOSAVE_INTERVAL_SEC, SIDEBAR_COLLAPSED_WIDTH, SIDEBAR_DEFAULT_WIDTH, SIDEBAR_MAX_WIDTH, @@ -42,6 +43,12 @@ import { import { buildHeaderTitle } from "./utils"; import type { EditorShellProps } from "./types"; +declare global { + interface Window { + __NOTE_BRANCH_SAVE_BEFORE_CLOSE__?: () => Promise; + } +} + const MAX_NAV_HISTORY = 100; type TreePanelState = "open" | "closed"; type TreePanelAction = "toggle"; @@ -106,6 +113,7 @@ export function EditorShell({ onThemeChange }: EditorShellProps) { const lastExpandedSidebarWidthRef = React.useRef(SIDEBAR_DEFAULT_WIDTH); const fileContentCacheRef = React.useRef>(new Map()); + const openRequestIdRef = React.useRef(0); const shortcutHelperRef = React.useRef(null); const navigationEntriesRef = React.useRef([]); const navigationIndexRef = React.useRef(-1); @@ -376,6 +384,13 @@ export function EditorShell({ onThemeChange }: EditorShellProps) { }, []); useEffect(() => { + const configuredInterval = appSettings?.autoSaveIntervalSec; + const autoSaveIntervalSec = + typeof configuredInterval === "number" && configuredInterval > 0 + ? configuredInterval + : DEFAULT_AUTOSAVE_INTERVAL_SEC; + const autoSaveDelayMs = autoSaveIntervalSec * 1000; + if (hasUnsavedChanges && selectedFile && editorContent) { if (autosaveTimerRef.current) { clearTimeout(autosaveTimerRef.current); @@ -383,7 +398,7 @@ export function EditorShell({ onThemeChange }: EditorShellProps) { autosaveTimerRef.current = setTimeout(() => { handleSaveFile(editorContent, true); - }, 300000); // 5 minutes + }, autoSaveDelayMs); unrefTimeout(autosaveTimerRef.current); } @@ -392,22 +407,44 @@ export function EditorShell({ onThemeChange }: EditorShellProps) { clearTimeout(autosaveTimerRef.current); } }; - }, [hasUnsavedChanges, selectedFile, editorContent, handleSaveFile]); + }, [ + hasUnsavedChanges, + selectedFile, + editorContent, + handleSaveFile, + appSettings?.autoSaveIntervalSec, + ]); + + const saveUnsavedChangesForClose = + React.useCallback(async (): Promise => { + if (!hasUnsavedChanges || !selectedFile || !editorContent) { + return false; + } + await handleSaveFile(editorContent, true); + return true; + }, [hasUnsavedChanges, selectedFile, editorContent, handleSaveFile]); // Save on app close useEffect(() => { - const handleBeforeUnload = async (e: BeforeUnloadEvent) => { - if (hasUnsavedChanges && selectedFile && editorContent) { - await handleSaveFile(editorContent, true); - - e.preventDefault(); - e.returnValue = ""; - } + const handleBeforeUnload = () => { + // Keep beforeunload path best-effort; BrowserWindow close orchestration is handled in main. + void saveUnsavedChangesForClose(); }; window.addEventListener("beforeunload", handleBeforeUnload); return () => window.removeEventListener("beforeunload", handleBeforeUnload); - }, [hasUnsavedChanges, selectedFile, editorContent, handleSaveFile]); + }, [saveUnsavedChangesForClose]); + + useEffect(() => { + window.__NOTE_BRANCH_SAVE_BEFORE_CLOSE__ = saveUnsavedChangesForClose; + return () => { + if ( + window.__NOTE_BRANCH_SAVE_BEFORE_CLOSE__ === saveUnsavedChangesForClose + ) { + delete window.__NOTE_BRANCH_SAVE_BEFORE_CLOSE__; + } + }; + }, [saveUnsavedChangesForClose]); // loadWorkspace moved to useCallback above @@ -429,36 +466,41 @@ export function EditorShell({ onThemeChange }: EditorShellProps) { fileContentCacheRef.current.set(selectedFile, editorContent); } + const requestId = ++openRequestIdRef.current; setSelectedFile(path); try { const cachedContent = fileContentCacheRef.current.get(path); - if (cachedContent !== undefined) { - const response = await window.NoteBranchApi.files.read(path); - if (response.ok && response.data) { + const response = await window.NoteBranchApi.files.read(path); + if (requestId !== openRequestIdRef.current) { + return; + } + + if (response.ok && response.data) { + if (cachedContent !== undefined) { setFileContent({ ...response.data, content: cachedContent, }); setEditorContent(cachedContent); - setHasUnsavedChanges(true); - } - } else { - const response = await window.NoteBranchApi.files.read(path); - if (response.ok && response.data) { + setHasUnsavedChanges(cachedContent !== response.data.content); + } else { setFileContent(response.data); setEditorContent(response.data.content); setHasUnsavedChanges(false); - } else { - setTransientStatus( - "error", - response.error?.message || message("failedReadFile"), - 5000, - ); } + } else { + setTransientStatus( + "error", + response.error?.message || message("failedReadFile"), + 5000, + ); } } catch (error) { + if (requestId !== openRequestIdRef.current) { + return; + } setTransientStatus("error", message("failedReadFile"), 5000); } }, @@ -630,7 +672,7 @@ export function EditorShell({ onThemeChange }: EditorShellProps) { }; const handleCreateFile = async (parentPath: string, fileName: string) => { - const response = await window.NoteBranchApi.files.create( + const response = await window.NoteBranchApi.files.createFile( parentPath, fileName, ); diff --git a/app/desktop/src/frontend/components/FindReplaceBar/index.tsx b/app/desktop/src/frontend/components/FindReplaceBar/index.tsx index 8b1494e..e1f8684 100644 --- a/app/desktop/src/frontend/components/FindReplaceBar/index.tsx +++ b/app/desktop/src/frontend/components/FindReplaceBar/index.tsx @@ -52,12 +52,14 @@ export function FindReplaceBar({ const handleFindNext = () => { if (findQuery.trim()) { onFindNext(findQuery); + findInputRef.current?.focus(); } }; const handleFindPrevious = () => { if (findQuery.trim()) { onFindPrevious(findQuery); + findInputRef.current?.focus(); } }; @@ -86,7 +88,7 @@ export function FindReplaceBar({ }; return ( - + setFindQuery(e.target.value)} onKeyDown={handleKeyDown} sx={findInputSx} + inputProps={{ "data-testid": "find-replace-query-input" }} InputProps={{ endAdornment: matchInfo && matchInfo.total > 0 && ( @@ -126,6 +130,7 @@ export function FindReplaceBar({ size="small" onClick={handleFindNext} disabled={!findQuery.trim()} + data-testid="find-replace-next-button" > diff --git a/app/desktop/src/frontend/components/MarkdownEditorPane/index.tsx b/app/desktop/src/frontend/components/MarkdownEditorPane/index.tsx index f11c9f7..e027b9f 100644 --- a/app/desktop/src/frontend/components/MarkdownEditorPane/index.tsx +++ b/app/desktop/src/frontend/components/MarkdownEditorPane/index.tsx @@ -33,6 +33,7 @@ export function MarkdownEditorPane({ highlightActiveLineGutter: true, highlightActiveLine: true, foldGutter: true, + searchKeymap: false, }} /> diff --git a/app/desktop/src/frontend/components/MermaidDiagram/index.tsx b/app/desktop/src/frontend/components/MermaidDiagram/index.tsx index ecce43d..33029be 100644 --- a/app/desktop/src/frontend/components/MermaidDiagram/index.tsx +++ b/app/desktop/src/frontend/components/MermaidDiagram/index.tsx @@ -1,5 +1,6 @@ import React, { useEffect, useRef, useState } from "react"; import { Box, Typography } from "@mui/material"; +import DOMPurify from "dompurify"; import { useI18n } from "../../i18n"; import { buildMarkdownEditorMessages } from "../MarkdownEditor/constants"; @@ -25,6 +26,7 @@ export function MermaidDiagram({ code, isDark }: MermaidDiagramProps) { const { default: mermaid } = await import("mermaid"); mermaid.initialize({ startOnLoad: false, + securityLevel: "strict", theme: isDark ? "dark" : "default", }); @@ -34,7 +36,11 @@ export function MermaidDiagram({ code, isDark }: MermaidDiagramProps) { ); if (!active || !containerRef.current) return; - containerRef.current.innerHTML = svg; + const sanitizedSvg = DOMPurify.sanitize(svg, { + USE_PROFILES: { svg: true, svgFilters: true }, + ADD_TAGS: ["foreignObject"], + }); + containerRef.current.innerHTML = sanitizedSvg; bindFunctions?.(containerRef.current); setError(null); } catch (err) { diff --git a/app/desktop/src/frontend/components/TextEditor/index.tsx b/app/desktop/src/frontend/components/TextEditor/index.tsx index 4150fd5..0a22baa 100644 --- a/app/desktop/src/frontend/components/TextEditor/index.tsx +++ b/app/desktop/src/frontend/components/TextEditor/index.tsx @@ -99,10 +99,8 @@ export function TextEditor({ }, [content, hasChanges, onChange]); useEffect(() => { - if (editorRef.current && editorRef.current.view) { - editorViewRef.current = editorRef.current.view; - } - }, []); + editorViewRef.current = editorRef.current?.view ?? null; + }); const handleSave = useCallback(() => { if (file && hasChanges) { @@ -262,7 +260,7 @@ export function TextEditor({ crosshairCursor: true, highlightSelectionMatches: true, closeBracketsKeymap: true, - searchKeymap: true, + searchKeymap: false, foldKeymap: true, completionKeymap: true, lintKeymap: true, diff --git a/app/desktop/src/frontend/utils/editorHooks.ts b/app/desktop/src/frontend/utils/editorHooks.ts index cfa8912..b0d1570 100644 --- a/app/desktop/src/frontend/utils/editorHooks.ts +++ b/app/desktop/src/frontend/utils/editorHooks.ts @@ -41,12 +41,12 @@ export const useEditorFindReplace = ({ >([]); const [currentMatchIndex, setCurrentMatchIndex] = useState(-1); - const findMatches = useCallback( - (query: string): { start: number; end: number }[] => { + const findMatchesInText = useCallback( + (source: string, query: string): { start: number; end: number }[] => { if (!query) return []; const matches: { start: number; end: number }[] = []; - const lowerContent = content.toLowerCase(); + const lowerContent = source.toLowerCase(); const lowerQuery = query.toLowerCase(); let index = 0; @@ -57,19 +57,25 @@ export const useEditorFindReplace = ({ return matches; }, - [content], + [], + ); + + const findMatches = useCallback( + (query: string): { start: number; end: number }[] => + findMatchesInText(content, query), + [content, findMatchesInText], ); const highlightMatch = useCallback( - (matchIndex: number) => { + (matches: { start: number; end: number }[], matchIndex: number) => { if ( !editorViewRef.current || matchIndex < 0 || - matchIndex >= searchMatches.length + matchIndex >= matches.length ) return; - const match = searchMatches[matchIndex]; + const match = matches[matchIndex]; const view = editorViewRef.current; view.dispatch({ @@ -79,7 +85,7 @@ export const useEditorFindReplace = ({ view.focus(); }, - [editorViewRef, searchMatches], + [editorViewRef], ); const resetFindState = useCallback(() => { @@ -111,7 +117,9 @@ export const useEditorFindReplace = ({ setSearchMatches(matches); if (matches.length > 0) { setCurrentMatchIndex(0); - setTimeout(() => highlightMatch(0), 0); + setTimeout(() => highlightMatch(matches, 0), 0); + } else { + setCurrentMatchIndex(-1); } } }, [editorViewRef, findMatches, highlightMatch]); @@ -129,7 +137,7 @@ export const useEditorFindReplace = ({ const nextIndex = currentMatchIndex < matches.length - 1 ? currentMatchIndex + 1 : 0; setCurrentMatchIndex(nextIndex); - highlightMatch(nextIndex); + highlightMatch(matches, nextIndex); }, [findMatches, highlightMatch, currentMatchIndex], ); @@ -147,7 +155,7 @@ export const useEditorFindReplace = ({ const prevIndex = currentMatchIndex > 0 ? currentMatchIndex - 1 : matches.length - 1; setCurrentMatchIndex(prevIndex); - highlightMatch(prevIndex); + highlightMatch(matches, prevIndex); }, [findMatches, highlightMatch, currentMatchIndex], ); @@ -169,13 +177,12 @@ export const useEditorFindReplace = ({ } setTimeout(() => { - const matches = findMatches(query); + const matches = findMatchesInText(newContent, query); setSearchMatches(matches); if (matches.length > 0) { - const nextIndex = - currentMatchIndex < matches.length ? currentMatchIndex : 0; + const nextIndex = Math.min(currentMatchIndex, matches.length - 1); setCurrentMatchIndex(nextIndex); - highlightMatch(nextIndex); + highlightMatch(matches, nextIndex); } else { setCurrentMatchIndex(-1); } @@ -185,7 +192,7 @@ export const useEditorFindReplace = ({ content, searchMatches, currentMatchIndex, - findMatches, + findMatchesInText, highlightMatch, setContent, onContentModified, @@ -243,10 +250,12 @@ export const useEditorGlobalShortcuts = ({ return; } - if (enableSaveShortcut && (e.metaKey || e.ctrlKey) && e.key === "s") { + const key = e.key.toLowerCase(); + + if (enableSaveShortcut && (e.metaKey || e.ctrlKey) && key === "s") { e.preventDefault(); onSave?.(); - } else if ((e.metaKey || e.ctrlKey) && e.key === "f" && !e.shiftKey) { + } else if ((e.metaKey || e.ctrlKey) && key === "f" && !e.shiftKey) { e.preventDefault(); onOpenFind?.(); } @@ -255,6 +264,20 @@ export const useEditorGlobalShortcuts = ({ window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); }, [enableSaveShortcut, onSave, onOpenFind]); + + useEffect(() => { + const offOpenFindInFile = window.NoteBranchApi?.menu?.onOpenFindInFile?.( + () => { + onOpenFind?.(); + }, + ); + + return () => { + if (typeof offOpenFindInFile === "function") { + offOpenFindInFile(); + } + }; + }, [onOpenFind]); }; export const useEditorKeymap = (onSave: () => void) => diff --git a/app/desktop/src/shared/types/api.ts b/app/desktop/src/shared/types/api.ts index 7565143..32f46bf 100644 --- a/app/desktop/src/shared/types/api.ts +++ b/app/desktop/src/shared/types/api.ts @@ -56,6 +56,7 @@ export interface NoteBranchApi { menu: { onOpenShortcuts: (listener: () => void) => () => void; onOpenAbout: (listener: () => void) => () => void; + onOpenFindInFile: (listener: () => void) => () => void; }; config: { getFull: () => Promise>; @@ -105,7 +106,6 @@ export interface NoteBranchApi { commit: (path: string, message: string) => Promise>; commitAll: (message: string) => Promise>; commitAndPushAll: () => Promise>; - create: (parentPath: string, name: string) => Promise>; createFile: ( parentPath: string, name: string, diff --git a/app/desktop/src/unit-tests/backend/handlers/configHandlers.test.ts b/app/desktop/src/unit-tests/backend/handlers/configHandlers.test.ts index dd4012f..b60e885 100644 --- a/app/desktop/src/unit-tests/backend/handlers/configHandlers.test.ts +++ b/app/desktop/src/unit-tests/backend/handlers/configHandlers.test.ts @@ -1,5 +1,5 @@ import { registerConfigHandlers } from "../../../backend/handlers/configHandlers"; -import { REPO_PROVIDERS } from "../../../shared/types"; +import { ApiErrorCode, REPO_PROVIDERS } from "../../../shared/types"; describe("configHandlers", () => { const createIpcMain = () => { @@ -75,6 +75,28 @@ describe("configHandlers", () => { expect(gitAdapter.checkGitInstalled).toHaveBeenCalled(); }); + it("rejects non-object app settings payloads", async () => { + const { ipcMain, handlers } = createIpcMain(); + const configService = { + updateAppSettings: jest.fn(), + } as any; + const repoService = { + refreshAutoSyncSettings: jest.fn(), + } as any; + const gitAdapter = {} as any; + + registerConfigHandlers(ipcMain, configService, repoService, gitAdapter); + + const response = await handlers["config:updateAppSettings"]( + null, + "invalid", + ); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(configService.updateAppSettings).not.toHaveBeenCalled(); + }); + it("returns false when checkGitInstalled fails", async () => { const { ipcMain, handlers } = createIpcMain(); const configService = {} as any; @@ -140,6 +162,26 @@ describe("configHandlers", () => { expect(configService.updateRepoSettings).toHaveBeenCalled(); }); + it("rejects invalid repo provider in updateRepoSettings", async () => { + const { ipcMain, handlers } = createIpcMain(); + const configService = { + updateRepoSettings: jest.fn(), + } as any; + const repoService = {} as any; + const gitAdapter = {} as any; + + registerConfigHandlers(ipcMain, configService, repoService, gitAdapter); + + const response = await handlers["config:updateRepoSettings"](null, { + provider: "invalid-provider", + localPath: "/repo", + }); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(configService.updateRepoSettings).not.toHaveBeenCalled(); + }); + it("returns error when profile preparation fails", async () => { const { ipcMain, handlers } = createIpcMain(); const createdProfile = { @@ -363,6 +405,50 @@ describe("configHandlers", () => { expect(configService.setActiveProfileId).toHaveBeenCalledWith("p1"); }); + it("resets stale services and refreshes search path after setActiveProfile", async () => { + const { ipcMain, handlers } = createIpcMain(); + const filesService = { + reset: jest.fn(), + }; + const searchService = { + reset: jest.fn(), + setRepoPath: jest.fn(), + }; + const configService = { + setActiveProfileId: jest.fn().mockResolvedValue(undefined), + getRepoSettings: jest.fn().mockResolvedValue({ + provider: REPO_PROVIDERS.git, + remoteUrl: "url", + branch: "main", + localPath: "/profiles/new", + pat: "token", + authMethod: "pat", + }), + } as any; + const repoService = { + resetActiveRepo: jest.fn(), + } as any; + const gitAdapter = {} as any; + + registerConfigHandlers( + ipcMain, + configService, + repoService, + gitAdapter, + undefined, + { filesService, searchService }, + ); + + const response = await handlers["config:setActiveProfile"](null, "p2"); + + expect(response.ok).toBe(true); + expect(configService.setActiveProfileId).toHaveBeenCalledWith("p2"); + expect(repoService.resetActiveRepo).toHaveBeenCalled(); + expect(filesService.reset).toHaveBeenCalled(); + expect(searchService.reset).toHaveBeenCalled(); + expect(searchService.setRepoPath).toHaveBeenCalledWith("/profiles/new"); + }); + it("returns error when deleteProfile fails", async () => { const { ipcMain, handlers } = createIpcMain(); const configService = { @@ -381,18 +467,58 @@ describe("configHandlers", () => { it("returns error when setActiveProfile fails", async () => { const { ipcMain, handlers } = createIpcMain(); + const filesService = { + reset: jest.fn(), + }; + const searchService = { + reset: jest.fn(), + setRepoPath: jest.fn(), + }; const configService = { setActiveProfileId: jest.fn().mockRejectedValue(new Error("set failed")), } as any; - const repoService = {} as any; + const repoService = { + resetActiveRepo: jest.fn(), + } as any; const gitAdapter = {} as any; - registerConfigHandlers(ipcMain, configService, repoService, gitAdapter); + registerConfigHandlers( + ipcMain, + configService, + repoService, + gitAdapter, + undefined, + { filesService, searchService }, + ); const response = await handlers["config:setActiveProfile"](null, "p1"); expect(response.ok).toBe(false); expect(response.error?.message).toBe("Failed to set active profile"); + expect(repoService.resetActiveRepo).not.toHaveBeenCalled(); + expect(filesService.reset).not.toHaveBeenCalled(); + expect(searchService.reset).not.toHaveBeenCalled(); + expect(searchService.setRepoPath).not.toHaveBeenCalled(); + }); + + it("rejects invalid profileId values for setActiveProfile", async () => { + const { ipcMain, handlers } = createIpcMain(); + const configService = { + setActiveProfileId: jest.fn(), + } as any; + const repoService = { + resetActiveRepo: jest.fn(), + } as any; + const gitAdapter = {} as any; + + registerConfigHandlers(ipcMain, configService, repoService, gitAdapter); + + const response = await handlers["config:setActiveProfile"](null, 42); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(configService.setActiveProfileId).not.toHaveBeenCalled(); + expect(repoService.resetActiveRepo).not.toHaveBeenCalled(); }); it("returns error for updateAppSettings failure", async () => { @@ -514,4 +640,21 @@ describe("configHandlers", () => { expect(response.ok).toBe(false); expect(response.error?.message).toBe("Failed to save favorites"); }); + + it("rejects invalid favorites payloads", async () => { + const { ipcMain, handlers } = createIpcMain(); + const configService = { + updateFavorites: jest.fn(), + } as any; + const repoService = {} as any; + const gitAdapter = {} as any; + + registerConfigHandlers(ipcMain, configService, repoService, gitAdapter); + + const response = await handlers["config:updateFavorites"](null, "note.md"); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(configService.updateFavorites).not.toHaveBeenCalled(); + }); }); diff --git a/app/desktop/src/unit-tests/backend/handlers/filesHandlers.test.ts b/app/desktop/src/unit-tests/backend/handlers/filesHandlers.test.ts index c214d76..2b69612 100644 --- a/app/desktop/src/unit-tests/backend/handlers/filesHandlers.test.ts +++ b/app/desktop/src/unit-tests/backend/handlers/filesHandlers.test.ts @@ -1,5 +1,9 @@ import { registerFilesHandlers } from "../../../backend/handlers/filesHandlers"; -import { COMMIT_AND_PUSH_RESULTS, REPO_PROVIDERS } from "../../../shared/types"; +import { + ApiErrorCode, + COMMIT_AND_PUSH_RESULTS, + REPO_PROVIDERS, +} from "../../../shared/types"; describe("filesHandlers", () => { const createIpcMain = () => { @@ -31,6 +35,59 @@ describe("filesHandlers", () => { expect(repoService.queueS3Delete).toHaveBeenCalledWith("note.md"); }); + it("rejects non-string path input for files:read", async () => { + const { ipcMain, handlers } = createIpcMain(); + const filesService = { + readFile: jest.fn(), + } as any; + const repoService = {} as any; + + registerFilesHandlers(ipcMain, filesService, repoService); + + const response = await handlers["files:read"](null, 123); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(filesService.readFile).not.toHaveBeenCalled(); + }); + + it("rejects non-boolean isAutosave for files:saveWithGitWorkflow", async () => { + const { ipcMain, handlers } = createIpcMain(); + const filesService = { + saveWithGitWorkflow: jest.fn(), + } as any; + const repoService = {} as any; + + registerFilesHandlers(ipcMain, filesService, repoService); + + const response = await handlers["files:saveWithGitWorkflow"]( + null, + "note.md", + "body", + "yes", + ); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(filesService.saveWithGitWorkflow).not.toHaveBeenCalled(); + }); + + it("rejects overly long file names for files:create", async () => { + const { ipcMain, handlers } = createIpcMain(); + const filesService = { + createFile: jest.fn(), + } as any; + const repoService = {} as any; + + registerFilesHandlers(ipcMain, filesService, repoService); + + const response = await handlers["files:create"](null, "", "a".repeat(256)); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(filesService.createFile).not.toHaveBeenCalled(); + }); + it("returns ok for rename even when s3 queueing fails", async () => { const { ipcMain, handlers } = createIpcMain(); const filesService = { diff --git a/app/desktop/src/unit-tests/backend/handlers/searchHandlers.test.ts b/app/desktop/src/unit-tests/backend/handlers/searchHandlers.test.ts index 244444b..3befe30 100644 --- a/app/desktop/src/unit-tests/backend/handlers/searchHandlers.test.ts +++ b/app/desktop/src/unit-tests/backend/handlers/searchHandlers.test.ts @@ -31,6 +31,42 @@ describe("searchHandlers", () => { expect(response.data).toEqual([{ filePath: "notes/a.md" }]); }); + it("rejects non-string query input", async () => { + const searchService = { + search: jest.fn(), + searchRepoWide: jest.fn(), + replaceInRepo: jest.fn(), + } as any; + + const { ipcMain, handlers } = createIpcMain(); + registerSearchHandlers(ipcMain, searchService); + + const response = await handlers["search:query"](null, 123); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(searchService.search).not.toHaveBeenCalled(); + }); + + it("rejects invalid maxResults input", async () => { + const searchService = { + search: jest.fn(), + searchRepoWide: jest.fn(), + replaceInRepo: jest.fn(), + } as any; + + const { ipcMain, handlers } = createIpcMain(); + registerSearchHandlers(ipcMain, searchService); + + const response = await handlers["search:query"](null, "test", { + maxResults: 0, + }); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(searchService.search).not.toHaveBeenCalled(); + }); + it("returns repo-wide search results", async () => { const searchService = { search: jest.fn(), @@ -74,6 +110,25 @@ describe("searchHandlers", () => { expect(response.data?.filesProcessed).toBe(1); }); + it("rejects non-string file paths in replace options", async () => { + const searchService = { + search: jest.fn(), + searchRepoWide: jest.fn(), + replaceInRepo: jest.fn(), + } as any; + + const { ipcMain, handlers } = createIpcMain(); + registerSearchHandlers(ipcMain, searchService); + + const response = await handlers["search:replaceInRepo"](null, "a", "b", { + filePaths: ["ok.md", 123], + }); + + expect(response.ok).toBe(false); + expect(response.error?.code).toBe(ApiErrorCode.VALIDATION_ERROR); + expect(searchService.replaceInRepo).not.toHaveBeenCalled(); + }); + it("returns error when search fails", async () => { const searchService = { search: jest.fn().mockRejectedValue(new Error("fail")), diff --git a/app/desktop/src/unit-tests/backend/providers/GitRepoProvider.test.ts b/app/desktop/src/unit-tests/backend/providers/GitRepoProvider.test.ts index 3bef26d..6da369f 100644 --- a/app/desktop/src/unit-tests/backend/providers/GitRepoProvider.test.ts +++ b/app/desktop/src/unit-tests/backend/providers/GitRepoProvider.test.ts @@ -196,7 +196,7 @@ describe("GitRepoProvider", () => { expect(gitAdapter.push).toHaveBeenCalledWith(baseSettings.pat); }); - it("starts and stops the auto-sync timer", () => { + it("restarts the auto-sync timer when startAutoSync is called again", () => { jest.useFakeTimers(); const { provider } = createProvider(); @@ -204,19 +204,36 @@ describe("GitRepoProvider", () => { const clearIntervalSpy = jest.spyOn(global, "clearInterval"); provider.startAutoSync(); + const firstTimer = (provider as any).autoSyncTimer; provider.startAutoSync(); - expect(setIntervalSpy).toHaveBeenCalledTimes(1); + expect(setIntervalSpy).toHaveBeenCalledTimes(2); + expect(clearIntervalSpy).toHaveBeenCalledWith(firstTimer); + expect((provider as any).autoSyncTimer).not.toBe(firstTimer); provider.stopAutoSync(); - expect(clearIntervalSpy).toHaveBeenCalled(); + expect(clearIntervalSpy).toHaveBeenCalledTimes(2); setIntervalSpy.mockRestore(); clearIntervalSpy.mockRestore(); jest.useRealTimers(); }); + it("uses the provided interval when starting auto-sync", () => { + jest.useFakeTimers(); + + const { provider } = createProvider(); + const setIntervalSpy = jest.spyOn(global, "setInterval"); + + provider.startAutoSync(45000); + + expect(setIntervalSpy).toHaveBeenCalledWith(expect.any(Function), 45000); + + setIntervalSpy.mockRestore(); + jest.useRealTimers(); + }); + it("fetches from remote and returns status", async () => { const { provider, gitAdapter } = createProvider(); provider.configure(baseSettings); diff --git a/app/desktop/src/unit-tests/backend/providers/S3RepoProvider.test.ts b/app/desktop/src/unit-tests/backend/providers/S3RepoProvider.test.ts index ac61fca..e196935 100644 --- a/app/desktop/src/unit-tests/backend/providers/S3RepoProvider.test.ts +++ b/app/desktop/src/unit-tests/backend/providers/S3RepoProvider.test.ts @@ -788,6 +788,116 @@ describe("S3RepoProvider", () => { expect(callModes).toEqual(["pull", "sync"]); }); + it("resolves queued sync callers only after their requested mode runs", async () => { + const provider = new S3RepoProvider(createAdapter() as any); + provider.configure({ ...baseSettings, localPath: "/tmp/NoteBranch-s3" }); + const callModes: Array<"pull" | "sync"> = []; + let hasReleaseFirstSync = false; + let hasReleaseThirdSync = false; + let releaseFirstSync: () => void = () => { + throw new Error("Expected first sync release callback"); + }; + let releaseThirdSync: () => void = () => { + throw new Error("Expected third queued sync to start"); + }; + + jest + .spyOn(provider as any, "performSync") + .mockImplementation(async (mode: unknown) => { + callModes.push(mode as "pull" | "sync"); + if (callModes.length === 1) { + await new Promise((resolve) => { + releaseFirstSync = resolve; + hasReleaseFirstSync = true; + }); + return; + } + if (callModes.length === 3) { + await new Promise((resolve) => { + releaseThirdSync = resolve; + hasReleaseThirdSync = true; + }); + } + }); + + const firstSync = (provider as any).sync("pull"); + + while (!hasReleaseFirstSync) { + await Promise.resolve(); + } + + const secondSync = (provider as any).sync("sync"); + const thirdSync = (provider as any).sync("pull"); + + let secondResolved = false; + let thirdResolved = false; + void secondSync.then(() => { + secondResolved = true; + }); + void thirdSync.then(() => { + thirdResolved = true; + }); + + if (!hasReleaseFirstSync) { + throw new Error("Expected first sync release callback"); + } + releaseFirstSync(); + await secondSync; + + expect(secondResolved).toBe(true); + expect(thirdResolved).toBe(false); + expect(callModes).toEqual(["pull", "sync", "pull"]); + + if (!hasReleaseThirdSync) { + throw new Error("Expected third queued sync to start"); + } + releaseThirdSync(); + + await thirdSync; + await firstSync; + expect(thirdResolved).toBe(true); + }); + + it("waits on the captured sync completion promise reference", async () => { + const provider = new S3RepoProvider(createAdapter() as any); + provider.configure({ ...baseSettings, localPath: "/tmp/NoteBranch-s3" }); + let hasResolveCompletion = false; + let resolveCompletion: () => void = () => { + throw new Error("Expected sync completion promise resolver"); + }; + const completionPromise = new Promise((resolve) => { + resolveCompletion = resolve; + hasResolveCompletion = true; + }); + let readCount = 0; + + Object.defineProperty(provider as any, "syncCompletionPromise", { + configurable: true, + get: () => { + readCount += 1; + return readCount === 1 ? completionPromise : null; + }, + set: jest.fn(), + }); + + let waitResolved = false; + const waitPromise = (provider as any) + .waitForSyncCycleCompletion() + .then(() => { + waitResolved = true; + }); + + await Promise.resolve(); + expect(waitResolved).toBe(false); + + if (!hasResolveCompletion) { + throw new Error("Expected sync completion promise resolver"); + } + resolveCompletion(); + await waitPromise; + expect(waitResolved).toBe(true); + }); + it("collects remote info while ignoring folders and metadata paths", async () => { const s3Adapter = createAdapter({ listObjects: jest.fn().mockResolvedValue([ diff --git a/app/desktop/src/unit-tests/backend/services/ExportService.zip-security.test.ts b/app/desktop/src/unit-tests/backend/services/ExportService.zip-security.test.ts new file mode 100644 index 0000000..8bf7570 --- /dev/null +++ b/app/desktop/src/unit-tests/backend/services/ExportService.zip-security.test.ts @@ -0,0 +1,74 @@ +import * as path from "path"; +import * as os from "os"; +import * as fs from "fs/promises"; +import AdmZip from "adm-zip"; +import { ExportService } from "../../../backend/services/ExportService"; +import { FsAdapter } from "../../../backend/adapters/FsAdapter"; +import { ConfigService } from "../../../backend/services/ConfigService"; + +describe("ExportService zip security", () => { + it("excludes .git and .NoteBranch internal metadata from exported zip", async () => { + const tempRoot = await fs.mkdtemp( + path.join(os.tmpdir(), "notebranch-zip-"), + ); + const sourcePath = path.join(tempRoot, "repo"); + const zipPath = path.join(tempRoot, "repo-export.zip"); + const exportService = new ExportService( + {} as FsAdapter, + { getRepoSettings: jest.fn() } as unknown as ConfigService, + ); + + try { + await fs.mkdir(path.join(sourcePath, "notes"), { recursive: true }); + await fs.mkdir(path.join(sourcePath, ".git"), { recursive: true }); + await fs.mkdir(path.join(sourcePath, ".NoteBranch"), { + recursive: true, + }); + await fs.mkdir(path.join(sourcePath, "node_modules", "pkg"), { + recursive: true, + }); + + await fs.writeFile(path.join(sourcePath, "notes", "note.md"), "# note"); + await fs.writeFile(path.join(sourcePath, ".gitignore"), "node_modules"); + await fs.writeFile( + path.join(sourcePath, ".git", "config"), + '[remote "origin"]\nurl=https://token@example.com/repo.git', + ); + await fs.writeFile( + path.join(sourcePath, ".NoteBranch", "s3-sync.json"), + '{"bucket":"private"}', + ); + await fs.writeFile( + path.join(sourcePath, "node_modules", "pkg", "index.js"), + "module.exports = {};", + ); + + await (exportService as any).createZipArchive(sourcePath, zipPath); + + const zip = new AdmZip(zipPath); + const entryNames = zip.getEntries().map((entry) => entry.entryName); + + expect(entryNames).toContain("notes/note.md"); + expect(entryNames).toContain(".gitignore"); + expect( + entryNames.some( + (entry) => entry === ".git" || entry.startsWith(".git/"), + ), + ).toBe(false); + expect( + entryNames.some( + (entry) => + entry === ".NoteBranch" || entry.startsWith(".NoteBranch/"), + ), + ).toBe(false); + expect( + entryNames.some( + (entry) => + entry === "node_modules" || entry.startsWith("node_modules/"), + ), + ).toBe(false); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } + }); +}); diff --git a/app/desktop/src/unit-tests/backend/services/FilesService.test.ts b/app/desktop/src/unit-tests/backend/services/FilesService.test.ts index 9b216a9..1029147 100644 --- a/app/desktop/src/unit-tests/backend/services/FilesService.test.ts +++ b/app/desktop/src/unit-tests/backend/services/FilesService.test.ts @@ -56,6 +56,49 @@ describe("FilesService", () => { }); }); + describe("reset", () => { + it("re-initializes with the latest repo path after reset", async () => { + mockConfigService.getRepoSettings + .mockResolvedValueOnce({ + provider: REPO_PROVIDERS.git, + localPath: "/repo-one", + remoteUrl: "url", + branch: "main", + pat: "token", + authMethod: AuthMethod.PAT, + }) + .mockResolvedValueOnce({ + provider: REPO_PROVIDERS.git, + localPath: "/repo-two", + remoteUrl: "url", + branch: "main", + pat: "token", + authMethod: AuthMethod.PAT, + }); + + mockFsAdapter.readFile.mockResolvedValue("# note"); + mockFsAdapter.stat.mockResolvedValue({ + size: 100, + mtime: new Date(), + isDirectory: () => false, + isFile: () => true, + } as Stats); + + await filesService.readFile("notes/test.md"); + filesService.reset(); + await filesService.readFile("notes/test.md"); + + expect(mockFsAdapter.readFile).toHaveBeenNthCalledWith( + 1, + path.join("/repo-one", "notes/test.md"), + ); + expect(mockFsAdapter.readFile).toHaveBeenNthCalledWith( + 2, + path.join("/repo-two", "notes/test.md"), + ); + }); + }); + describe("getFileType", () => { it("should identify markdown files", () => { expect(filesService.getFileType("note.md")).toBe(FileType.MARKDOWN); @@ -1060,6 +1103,21 @@ describe("FilesService", () => { expect(result).toBe(path.join("note(2).md")); }); + it("throws after too many duplicate filename attempts", async () => { + mockFsAdapter.stat.mockResolvedValue({ isFile: () => true } as any); + mockFsAdapter.exists.mockResolvedValue(true); + + await expect(filesService.duplicateFile("note.md")).rejects.toMatchObject( + { + code: ApiErrorCode.UNKNOWN_ERROR, + message: "Too many duplicate filename attempts", + }, + ); + + expect(mockFsAdapter.exists).toHaveBeenCalledTimes(1000); + expect(mockFsAdapter.copyFile).not.toHaveBeenCalled(); + }); + it("throws when source is not a file", async () => { mockFsAdapter.stat.mockResolvedValue({ isFile: () => false } as any); await expect(filesService.duplicateFile("folder")).rejects.toBeTruthy(); diff --git a/app/desktop/src/unit-tests/backend/services/RepoService.test.ts b/app/desktop/src/unit-tests/backend/services/RepoService.test.ts index df61c82..a475104 100644 --- a/app/desktop/src/unit-tests/backend/services/RepoService.test.ts +++ b/app/desktop/src/unit-tests/backend/services/RepoService.test.ts @@ -513,6 +513,43 @@ describe("RepoService", () => { ).resolves.toBeUndefined(); }); + it("resetActiveRepo clears cached provider and reloads from current profile settings", async () => { + const { repoService, gitProvider, s3Provider, mockConfigService } = + createRepoService(); + + await repoService.openOrClone({ + provider: REPO_PROVIDERS.git, + remoteUrl: "https://github.com/user/repo.git", + branch: "main", + localPath: "/repo", + pat: "token", + authMethod: "pat", + } as any); + + mockConfigService.getRepoSettings = jest.fn().mockResolvedValue({ + provider: REPO_PROVIDERS.s3, + localPath: "/repo-s3", + bucket: "notes-bucket", + region: "us-east-1", + prefix: "", + accessKeyId: "access-key", + secretAccessKey: "secret-key", + sessionToken: "", + }); + + repoService.resetActiveRepo(); + await repoService.getStatus(); + + expect(gitProvider.stopAutoSync).toHaveBeenCalled(); + expect(s3Provider.configure).toHaveBeenCalledWith( + expect.objectContaining({ + provider: REPO_PROVIDERS.s3, + localPath: "/repo-s3", + }), + ); + expect(s3Provider.getStatus).toHaveBeenCalled(); + }); + it("stops auto sync when destroyed", async () => { const { repoService, gitProvider } = createRepoService(); diff --git a/app/desktop/src/unit-tests/backend/services/SearchService.test.ts b/app/desktop/src/unit-tests/backend/services/SearchService.test.ts index 60ea713..4aa3559 100644 --- a/app/desktop/src/unit-tests/backend/services/SearchService.test.ts +++ b/app/desktop/src/unit-tests/backend/services/SearchService.test.ts @@ -84,9 +84,9 @@ describe("SearchService", () => { expect(results).toEqual([]); }); - it("supports regex fallback when regex is invalid", async () => { + it("supports safe regex patterns when regex mode is enabled", async () => { const files: Record = { - "/test/repo/notes/a.md": "Has [ bracket", + "/test/repo/notes/a.md": "hello hillo hullo", }; const dirs: Record = { "/test/repo": ["notes"], @@ -106,12 +106,55 @@ describe("SearchService", () => { async (filePath: string) => files[filePath], ); - const results = await searchService.searchRepoWide("[", { + const results = await searchService.searchRepoWide("h.llo", { useRegex: true, }); expect(results).toHaveLength(1); - expect(results[0].matches[0].lineContent).toContain("["); + expect(results[0].matches).toHaveLength(3); + }); + + it("rejects invalid regex patterns when regex mode is enabled", async () => { + const files: Record = { + "/test/repo/notes/a.md": "Has [ bracket", + }; + const dirs: Record = { + "/test/repo": ["notes"], + "/test/repo/notes": ["a.md"], + }; + + mockFsAdapter.readdir.mockImplementation( + async (dir: string) => dirs[dir] || [], + ); + mockFsAdapter.stat.mockImplementation( + async (fullPath: string) => + ({ + isDirectory: () => Boolean(dirs[fullPath]), + }) as any, + ); + mockFsAdapter.readFile.mockImplementation( + async (filePath: string) => files[filePath], + ); + + await expect( + searchService.searchRepoWide("[", { + useRegex: true, + }), + ).rejects.toMatchObject({ + code: ApiErrorCode.VALIDATION_ERROR, + }); + expect(mockFsAdapter.readFile).not.toHaveBeenCalled(); + }); + + it("rejects unsafe regex patterns that can trigger catastrophic backtracking", async () => { + await expect( + searchService.searchRepoWide("(a+)+$", { + useRegex: true, + }), + ).rejects.toMatchObject({ + code: ApiErrorCode.VALIDATION_ERROR, + }); + expect(mockFsAdapter.readFile).not.toHaveBeenCalled(); }); it("continues when individual files cannot be read", async () => { @@ -230,20 +273,19 @@ describe("SearchService", () => { expect(mockFsAdapter.writeFile).toHaveBeenCalledTimes(2); }); - it("falls back to plain text replacement when the regex is invalid", async () => { + it("rejects invalid regex replacement patterns when regex mode is enabled", async () => { mockFsAdapter.readFile.mockResolvedValue("a[b a[b"); mockFsAdapter.writeFile.mockResolvedValue(); - const result = await searchService.replaceInRepo("[b", "X", { - useRegex: true, - filePaths: ["test.md"], + await expect( + searchService.replaceInRepo("[b", "X", { + useRegex: true, + filePaths: ["test.md"], + }), + ).rejects.toMatchObject({ + code: ApiErrorCode.VALIDATION_ERROR, }); - - expect(result.totalReplacements).toBe(2); - expect(mockFsAdapter.writeFile).toHaveBeenCalledWith( - "/test/repo/test.md", - "aX aX", - ); + expect(mockFsAdapter.writeFile).not.toHaveBeenCalled(); }); it("throws when markdown file discovery fails before replacement starts", async () => { @@ -278,6 +320,15 @@ describe("SearchService", () => { code: ApiErrorCode.REPO_NOT_INITIALIZED, }); }); + + it("throws after reset until a new repo path is configured", async () => { + searchService.setRepoPath("/test/repo"); + searchService.reset(); + + await expect(searchService.search("test")).rejects.toMatchObject({ + code: ApiErrorCode.REPO_NOT_INITIALIZED, + }); + }); }); // Note: Additional SearchService tests would require complex file system mocking diff --git a/app/desktop/src/unit-tests/backend/utils/resolveTutorialsRootDir.test.ts b/app/desktop/src/unit-tests/backend/utils/resolveTutorialsRootDir.test.ts index badb554..d760c67 100644 --- a/app/desktop/src/unit-tests/backend/utils/resolveTutorialsRootDir.test.ts +++ b/app/desktop/src/unit-tests/backend/utils/resolveTutorialsRootDir.test.ts @@ -1,7 +1,7 @@ import { resolveTutorialsRootDir } from "../../../backend/utils/resolveTutorialsRootDir"; const TUTORIAL_SENTINELS = [ - "scenarios/create-repo-on-NoteBranch/images/step-01-welcome-screen.png", + "scenarios/create-repo-on-notegit/images/step-01-welcome-screen.png", "scenarios/connect-s3-bucket-with-prefix/images/step-05-verify-s3-connected.png", ]; diff --git a/app/desktop/src/unit-tests/frontend/components/EditorShell.test.ts b/app/desktop/src/unit-tests/frontend/components/EditorShell.test.ts index a70a0b2..debc189 100644 --- a/app/desktop/src/unit-tests/frontend/components/EditorShell.test.ts +++ b/app/desktop/src/unit-tests/frontend/components/EditorShell.test.ts @@ -426,7 +426,7 @@ describe("EditorShell", () => { return buildFileResponse(FileType.MARKDOWN, filePath); }), save: jest.fn().mockResolvedValue({ ok: true }), - create: jest.fn().mockResolvedValue({ ok: true }), + createFile: jest.fn().mockResolvedValue({ ok: true }), createFolder: jest.fn().mockResolvedValue({ ok: true }), delete: jest.fn().mockResolvedValue({ ok: true }), rename: jest.fn().mockResolvedValue({ ok: true }), @@ -669,7 +669,7 @@ describe("EditorShell", () => { }); expect( - (global as any).window.NoteBranchApi.files.create, + (global as any).window.NoteBranchApi.files.createFile, ).toHaveBeenCalledWith("notes", "new.md"); expect( (global as any).window.NoteBranchApi.files.createFolder, @@ -768,7 +768,7 @@ describe("EditorShell", () => { }); await act(async () => { - jest.advanceTimersByTime(300000); + jest.advanceTimersByTime(30000); await flushPromises(); }); expect( @@ -798,6 +798,234 @@ describe("EditorShell", () => { ).toContain("updated content"); }); + it("does not mark reopened cached files as dirty when cache matches disk", async () => { + const fileStore: Record = { + "notes/doc.md": "notes/doc.md content", + "notes/text.txt": "notes/text.txt content", + }; + + (global as any).window.NoteBranchApi.files.read = jest + .fn() + .mockImplementation(async (filePath: string) => { + if (filePath.endsWith(".txt")) { + return { + ok: true, + data: { + path: filePath, + content: fileStore[filePath], + type: FileType.TEXT, + size: 10, + lastModified: new Date("2024-01-01T00:00:00Z"), + }, + }; + } + + return { + ok: true, + data: { + path: filePath, + content: fileStore[filePath], + type: FileType.MARKDOWN, + size: 10, + lastModified: new Date("2024-01-01T00:00:00Z"), + }, + }; + }); + (global as any).window.NoteBranchApi.files.save = jest + .fn() + .mockImplementation(async (filePath: string, content: string) => { + fileStore[filePath] = content; + return { ok: true }; + }); + + const renderer = await renderEditorShell(); + + await act(async () => { + findButton(renderer!, "SelectMarkdown").props.onClick(); + await flushPromises(); + }); + + act(() => { + findButton(renderer!, "MarkdownChange").props.onClick(); + }); + + await act(async () => { + jest.advanceTimersByTime(30000); + await flushPromises(); + }); + expect( + (global as any).window.NoteBranchApi.files.save, + ).toHaveBeenCalledTimes(1); + + await act(async () => { + findButton(renderer!, "SelectText").props.onClick(); + await flushPromises(); + }); + await act(async () => { + findButton(renderer!, "SelectMarkdown").props.onClick(); + await flushPromises(); + }); + + await act(async () => { + jest.advanceTimersByTime(30000); + await flushPromises(); + }); + + expect( + (global as any).window.NoteBranchApi.files.save, + ).toHaveBeenCalledTimes(1); + expect( + renderer!.root.findByProps({ "data-testid": "markdown-editor" }).children, + ).toContain("updated content"); + }); + + it("marks reopened cached files as dirty when cache differs from disk", async () => { + const renderer = await renderEditorShell(); + + await act(async () => { + findButton(renderer!, "SelectMarkdown").props.onClick(); + await flushPromises(); + }); + + act(() => { + findButton(renderer!, "MarkdownChange").props.onClick(); + }); + + await act(async () => { + findButton(renderer!, "SelectText").props.onClick(); + await flushPromises(); + }); + await act(async () => { + findButton(renderer!, "SelectMarkdown").props.onClick(); + await flushPromises(); + }); + + await act(async () => { + jest.advanceTimersByTime(30000); + await flushPromises(); + }); + + expect( + (global as any).window.NoteBranchApi.files.save, + ).toHaveBeenCalledWith("notes/doc.md", "updated content"); + }); + + it("triggers save on beforeunload without awaiting completion", async () => { + let resolveSave: (value: { ok: boolean }) => void = () => {}; + let capturedSaveResolver = false; + (global as any).window.NoteBranchApi.files.save = jest + .fn() + .mockImplementation( + () => + new Promise<{ ok: boolean }>((resolve) => { + resolveSave = resolve; + capturedSaveResolver = true; + }), + ); + + const renderer = await renderEditorShell(); + + await act(async () => { + findButton(renderer!, "SelectMarkdown").props.onClick(); + await flushPromises(); + }); + + act(() => { + findButton(renderer!, "MarkdownChange").props.onClick(); + }); + + const beforeUnloadEvent: any = { + type: "beforeunload", + preventDefault: jest.fn(), + returnValue: undefined, + }; + + act(() => { + window.dispatchEvent(beforeUnloadEvent); + }); + + expect( + (global as any).window.NoteBranchApi.files.save, + ).toHaveBeenCalledWith("notes/doc.md", "updated content"); + + expect(capturedSaveResolver).toBe(true); + + await act(async () => { + resolveSave({ ok: true }); + await flushPromises(); + }); + }); + + it("exposes save-before-close hook on window", async () => { + (global as any).window.NoteBranchApi.files.save = jest + .fn() + .mockResolvedValue({ ok: true }); + + const renderer = await renderEditorShell(); + + await act(async () => { + findButton(renderer!, "SelectMarkdown").props.onClick(); + await flushPromises(); + }); + + act(() => { + findButton(renderer!, "MarkdownChange").props.onClick(); + }); + + await act(async () => { + await (window as any).__NOTE_BRANCH_SAVE_BEFORE_CLOSE__?.(); + await flushPromises(); + }); + + expect( + (global as any).window.NoteBranchApi.files.save, + ).toHaveBeenCalledWith("notes/doc.md", "updated content"); + }); + + it("uses configured autosave interval from app settings", async () => { + (global as any).window.NoteBranchApi.config.getFull = jest + .fn() + .mockResolvedValue({ + ok: true, + data: { + ...baseConfig, + appSettings: { + ...baseConfig.appSettings, + autoSaveIntervalSec: 7, + }, + }, + }); + + const renderer = await renderEditorShell(); + + await act(async () => { + findButton(renderer!, "SelectMarkdown").props.onClick(); + await flushPromises(); + }); + + act(() => { + findButton(renderer!, "MarkdownChange").props.onClick(); + }); + + await act(async () => { + jest.advanceTimersByTime(6999); + await flushPromises(); + }); + + expect( + (global as any).window.NoteBranchApi.files.save, + ).not.toHaveBeenCalledWith("notes/doc.md", "updated content"); + + await act(async () => { + jest.advanceTimersByTime(1); + await flushPromises(); + }); + + expect( + (global as any).window.NoteBranchApi.files.save, + ).toHaveBeenCalledWith("notes/doc.md", "updated content"); + }); + it("handles git commit-and-push edge cases", async () => { (global as any).window.NoteBranchApi.files.commitAndPushAll = jest .fn() @@ -1061,6 +1289,53 @@ describe("EditorShell", () => { ).toContain("notes/text.txt content"); }); + it("keeps the latest file selection when reads resolve out of order", async () => { + let resolveMarkdownRead: ((value: any) => void) | null = null; + let resolveTextRead: ((value: any) => void) | null = null; + + (global as any).window.NoteBranchApi.files.read = jest + .fn() + .mockImplementation((filePath: string) => { + return new Promise((resolve) => { + if (filePath === "notes/doc.md") { + resolveMarkdownRead = resolve; + return; + } + + if (filePath === "notes/text.txt") { + resolveTextRead = resolve; + return; + } + + resolve(buildFileResponse(FileType.MARKDOWN, filePath)); + }); + }); + + const renderer = await renderEditorShell(); + + await act(async () => { + findButton(renderer, "SelectMarkdown").props.onClick(); + findButton(renderer, "SelectText").props.onClick(); + await flushPromises(); + }); + + await act(async () => { + resolveTextRead?.(buildFileResponse(FileType.TEXT, "notes/text.txt")); + await flushPromises(); + }); + + await act(async () => { + resolveMarkdownRead?.( + buildFileResponse(FileType.MARKDOWN, "notes/doc.md"), + ); + await flushPromises(); + }); + + expect( + renderer.root.findByProps({ "data-testid": "text-editor" }).children, + ).toContain("notes/text.txt content"); + }); + it("handles commit callbacks and dialog close handlers", async () => { const renderer = await renderEditorShell(); diff --git a/app/desktop/src/unit-tests/frontend/components/MarkdownEditor.test.ts b/app/desktop/src/unit-tests/frontend/components/MarkdownEditor.test.ts index 65aa80d..51a9633 100644 --- a/app/desktop/src/unit-tests/frontend/components/MarkdownEditor.test.ts +++ b/app/desktop/src/unit-tests/frontend/components/MarkdownEditor.test.ts @@ -542,8 +542,19 @@ describe("MarkdownEditor task list formatting", () => { const findBar = renderer.root.findByType( require("../../../frontend/components/FindReplaceBar").FindReplaceBar, ); + mockView.dispatch.mockClear(); + mockView.focus.mockClear(); + await act(async () => { findBar.props.onFindNext("hello"); + }); + + expect(mockView.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ scrollIntoView: true }), + ); + expect(mockView.focus).toHaveBeenCalled(); + + await act(async () => { findBar.props.onFindPrevious("hello"); }); @@ -552,13 +563,13 @@ describe("MarkdownEditor task list formatting", () => { jest.runAllTimers(); }); - expect(onChange).toHaveBeenCalledWith("hello hi", true); + expect(onChange).toHaveBeenCalledWith("hi hello", true); await act(async () => { findBar.props.onReplaceAll("hello", "hey"); }); - expect(onChange).toHaveBeenCalledWith("hey hi", true); + expect(onChange).toHaveBeenCalledWith("hi hey", true); jest.useRealTimers(); }); diff --git a/app/desktop/src/unit-tests/frontend/components/MermaidDiagram.test.ts b/app/desktop/src/unit-tests/frontend/components/MermaidDiagram.test.ts index b013d44..88bcf7c 100644 --- a/app/desktop/src/unit-tests/frontend/components/MermaidDiagram.test.ts +++ b/app/desktop/src/unit-tests/frontend/components/MermaidDiagram.test.ts @@ -1,6 +1,7 @@ import React from "react"; import { act, create } from "react-test-renderer"; import mermaid from "mermaid"; +import DOMPurify from "dompurify"; import { MermaidDiagram } from "../../../frontend/components/MermaidDiagram"; jest.mock("mermaid", () => ({ @@ -11,10 +12,20 @@ jest.mock("mermaid", () => ({ }, })); +jest.mock("dompurify", () => ({ + __esModule: true, + default: { + sanitize: jest.fn((value: string) => value), + }, +})); + const mockedMermaid = mermaid as unknown as { initialize: jest.Mock; render: jest.Mock; }; +const mockedDOMPurify = DOMPurify as unknown as { + sanitize: jest.Mock; +}; const flushPromises = () => new Promise((resolve) => setImmediate(resolve)); @@ -65,6 +76,8 @@ describe("MermaidDiagram", () => { svg: "", bindFunctions: jest.fn(), }); + mockedDOMPurify.sanitize.mockClear(); + mockedDOMPurify.sanitize.mockImplementation((value: string) => value); }); it("initializes mermaid with light theme and renders diagram", async () => { @@ -74,6 +87,7 @@ describe("MermaidDiagram", () => { expect(mockedMermaid.initialize).toHaveBeenCalledWith({ startOnLoad: false, + securityLevel: "strict", theme: "default", }); expect(mockedMermaid.render).toHaveBeenCalledWith(expect.any(String), code); @@ -84,6 +98,7 @@ describe("MermaidDiagram", () => { expect(mockedMermaid.initialize).toHaveBeenCalledWith({ startOnLoad: false, + securityLevel: "strict", theme: "dark", }); }); @@ -138,4 +153,36 @@ describe("MermaidDiagram", () => { useRefSpy.mockRestore(); }); + + it("sanitizes rendered svg before inserting into innerHTML", async () => { + const bindFunctions = jest.fn(); + const useRefSpy = jest + .spyOn(React, "useRef") + .mockReturnValueOnce({ current: { innerHTML: "" } } as any) + .mockReturnValueOnce({ current: "mermaid-fixed" } as any); + mockedMermaid.render.mockResolvedValueOnce({ + svg: "", + bindFunctions, + }); + mockedDOMPurify.sanitize.mockImplementationOnce((value: string) => + value.replace(//gi, ""), + ); + + await renderDiagram("graph TD; A-->B", false); + + expect(mockedDOMPurify.sanitize).toHaveBeenCalledWith( + "", + { + USE_PROFILES: { svg: true, svgFilters: true }, + ADD_TAGS: ["foreignObject"], + }, + ); + expect(bindFunctions).toHaveBeenCalledWith( + expect.objectContaining({ + innerHTML: "", + }), + ); + + useRefSpy.mockRestore(); + }); }); diff --git a/app/desktop/src/unit-tests/frontend/components/TextEditor.test.ts b/app/desktop/src/unit-tests/frontend/components/TextEditor.test.ts index 477e87e..033aea3 100644 --- a/app/desktop/src/unit-tests/frontend/components/TextEditor.test.ts +++ b/app/desktop/src/unit-tests/frontend/components/TextEditor.test.ts @@ -346,6 +346,14 @@ describe("TextEditor", () => { const findBar = renderer.root.findByType(FindReplaceBar); await act(async () => { findBar.props.onFindNext("hello"); + }); + + expect(mockView.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ scrollIntoView: true }), + ); + expect(mockView.focus).toHaveBeenCalled(); + + await act(async () => { findBar.props.onFindPrevious("hello"); }); diff --git a/app/desktop/src/unit-tests/frontend/components/Workspace.test.ts b/app/desktop/src/unit-tests/frontend/components/Workspace.test.ts index 71733ff..644687d 100644 --- a/app/desktop/src/unit-tests/frontend/components/Workspace.test.ts +++ b/app/desktop/src/unit-tests/frontend/components/Workspace.test.ts @@ -3,7 +3,10 @@ import TestRenderer, { act } from "react-test-renderer"; import { EditorShell } from "../../../frontend/components/EditorShell"; import type { RepoStatus } from "../../../shared/types"; import { FileType, REPO_PROVIDERS } from "../../../shared/types"; -import { SIDEBAR_COLLAPSED_WIDTH } from "../../../frontend/components/EditorShell/constants"; +import { + DEFAULT_AUTOSAVE_INTERVAL_SEC, + SIDEBAR_COLLAPSED_WIDTH, +} from "../../../frontend/components/EditorShell/constants"; const FileTreeViewMock = jest.fn((_props: any) => null); const MarkdownEditorMock = jest.fn((_props: any) => null); @@ -143,7 +146,7 @@ describe("EditorShell", () => { }, }), save: jest.fn().mockResolvedValue({ ok: true }), - create: jest.fn(), + createFile: jest.fn(), createFolder: jest.fn(), delete: jest.fn(), rename: jest.fn(), @@ -254,7 +257,7 @@ describe("EditorShell", () => { }); it("creates and deletes files via file tree actions", async () => { - (global as any).window.NoteBranchApi.files.create = jest + (global as any).window.NoteBranchApi.files.createFile = jest .fn() .mockResolvedValue({ ok: true }); (global as any).window.NoteBranchApi.files.delete = jest @@ -306,7 +309,7 @@ describe("EditorShell", () => { }); expect( - (global as any).window.NoteBranchApi.files.create, + (global as any).window.NoteBranchApi.files.createFile, ).toHaveBeenCalledWith("", "new.md"); expect( (global as any).window.NoteBranchApi.files.delete, @@ -910,7 +913,7 @@ describe("EditorShell", () => { markdownProps.onChange("autosave", true); }); act(() => { - jest.advanceTimersByTime(300000); + jest.advanceTimersByTime(DEFAULT_AUTOSAVE_INTERVAL_SEC * 1000); }); const beforeUnloadHandler = ( diff --git a/app/desktop/version.json b/app/desktop/version.json index 302d4b9..d3e987b 100644 --- a/app/desktop/version.json +++ b/app/desktop/version.json @@ -1,3 +1,3 @@ { - "version": "2.9.1" + "version": "2.9.2" } diff --git a/app/website/src/data/siteContent.ts b/app/website/src/data/siteContent.ts index 8f6e219..406a9eb 100644 --- a/app/website/src/data/siteContent.ts +++ b/app/website/src/data/siteContent.ts @@ -231,7 +231,7 @@ export const latestRelease = { apiUrl: githubLatestReleaseApi }; -export const desktopReleaseVersion = "2.9.1"; +export const desktopReleaseVersion = "2.9.2"; export const releasesPageUrl = `${githubBase}/releases`; diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 5ce3153..fc029af 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -2,7 +2,7 @@ How to use NoteBranch as an end user. -**Version**: 2.9.1 +**Version**: 2.9.2 **Last Updated**: March 7, 2026 ## Table of Contents