diff --git a/packages/insomnia-data/common-src/misc.test.ts b/packages/insomnia-data/common-src/misc.test.ts new file mode 100644 index 00000000000..2342d184f4e --- /dev/null +++ b/packages/insomnia-data/common-src/misc.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest'; + +import { slugify } from './misc'; + +describe('slugify', () => { + it('lowercases and joins words with hyphens', () => { + expect(slugify('Cams Project!')).toBe('cams-project'); + }); + + it('strips accents', () => { + expect(slugify('CafΓ© MΓ©xico')).toBe('cafe-mexico'); + }); + + it('collapses runs of non-alphanumeric characters', () => { + expect(slugify(' --Weird__Name-- ')).toBe('weird-name'); + }); + + it('returns an empty string when nothing usable remains', () => { + expect(slugify('πŸš€πŸš€πŸš€')).toBe(''); + }); + + it('truncates to maxLength without leaving a trailing hyphen', () => { + const longName = 'a-very-long-project-name-that-goes-on-and-on-and-on'; + const slug = slugify(longName, 20); + + expect(slug.length).toBeLessThanOrEqual(20); + expect(slug.endsWith('-')).toBe(false); + }); +}); diff --git a/packages/insomnia-data/common-src/misc.ts b/packages/insomnia-data/common-src/misc.ts index 0872996c616..65d2c410cbd 100644 --- a/packages/insomnia-data/common-src/misc.ts +++ b/packages/insomnia-data/common-src/misc.ts @@ -13,3 +13,25 @@ export function generateId(prefix?: string) { } return id; } + +/** + * Turns arbitrary text into a filesystem-safe slug: lowercased, accents + * stripped, runs of non-alphanumeric characters collapsed to a single `-`, + * leading/trailing `-` trimmed, and truncated to `maxLength`. + * + * Returns `''` when nothing usable remains (e.g. all-emoji or all-punctuation + * input) β€” callers should treat that as "no slug available". + */ +export function slugify(input: string, maxLength = 40) { + const slug = input + .normalize('NFKD') + .replace(/[\u0300-\u036F]/g, '') // strip accents (combining diacritical marks) + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, ''); + + if (slug.length <= maxLength) { + return slug; + } + return slug.slice(0, maxLength).replace(/-+$/g, ''); +} diff --git a/packages/insomnia-data/src/models/git-repository.test.ts b/packages/insomnia-data/src/models/git-repository.test.ts new file mode 100644 index 00000000000..1604321e7a0 --- /dev/null +++ b/packages/insomnia-data/src/models/git-repository.test.ts @@ -0,0 +1,40 @@ +import { models } from 'insomnia-data'; +import { describe, expect, it } from 'vitest'; + +describe('getGitRepoFolderName', () => { + it('returns the bare id when there is no folderSlug', () => { + const folderName = models.gitRepository.getGitRepoFolderName({ + _id: 'git_57d071a646b34393929036c34dd0915e', + folderSlug: null, + }); + + expect(folderName).toBe('git_57d071a646b34393929036c34dd0915e'); + }); + + it('embeds the slug between the "git" prefix and the hex id when set', () => { + const folderName = models.gitRepository.getGitRepoFolderName({ + _id: 'git_57d071a646b34393929036c34dd0915e', + folderSlug: 'cams-project', + }); + + expect(folderName).toBe('git_cams-project_57d071a646b34393929036c34dd0915e'); + }); + + it('falls back to the whole id if it somehow lacks the "git_" prefix', () => { + const folderName = models.gitRepository.getGitRepoFolderName({ + _id: '57d071a646b34393929036c34dd0915e', + folderSlug: 'cams-project', + }); + + expect(folderName).toBe('git_cams-project_57d071a646b34393929036c34dd0915e'); + }); + + it('falls back to the bare id when folderSlug contains anything outside [a-z0-9-] (e.g. a corrupted or path-traversal value)', () => { + const folderName = models.gitRepository.getGitRepoFolderName({ + _id: 'git_57d071a646b34393929036c34dd0915e', + folderSlug: '../../etc', + }); + + expect(folderName).toBe('git_57d071a646b34393929036c34dd0915e'); + }); +}); diff --git a/packages/insomnia-data/src/models/git-repository.ts b/packages/insomnia-data/src/models/git-repository.ts index 7e53110eade..e83e33a91bf 100644 --- a/packages/insomnia-data/src/models/git-repository.ts +++ b/packages/insomnia-data/src/models/git-repository.ts @@ -33,6 +33,7 @@ export function init(): BaseGitRepository { uriNeedsMigration: true, repoMigrationVersion: 0, directory: null, + folderSlug: null, }; } @@ -75,15 +76,50 @@ export interface BaseGitRepository { * repository's working tree and .git directory live. * * `null` (the default) means the repository is stored in the app-managed - * location: `{INSOMNIA_DATA_PATH || userData}/version-control/git/{_id}`. - * Insomnia owns that managed folder. When `directory` is set, the user owns - * the folder and Insomnia must not delete it on project removal. + * location: `{INSOMNIA_DATA_PATH || userData}/version-control/git/{folder}`, + * where `folder` is computed by {@link getGitRepoFolderName}. Insomnia owns + * that managed folder. When `directory` is set, the user owns the folder and + * Insomnia must not delete it on project removal. */ directory: string | null; + /** + * A filesystem-safe slug derived from the owning project's name at the time + * the app-managed folder was created (or, for repos that predate this field, + * backfilled by a one-time best-effort startup pass before the window is + * created). It is baked into the managed folder name for readability (see + * {@link getGitRepoFolderName}) and is intentionally NOT kept in sync with + * later project renames β€” renaming the folder on every project rename would + * risk moving a directory out from under an open editor, terminal, or + * native git process. + * + * `null` means either the repo uses a user-chosen `directory` (irrelevant), + * or the folder still uses its legacy bare-id name. + */ + folderSlug: string | null; } export const isGitRepository = (model: Pick): model is GitRepository => model.type === type; +// `folderSlug` is only ever written via `slugify()`, which already restricts +// its output to this charset β€” this is a second, independent check at the +// point the value gets baked into a filesystem path, so a corrupted or +// otherwise unsanitized `folderSlug` can never introduce a path separator or +// a `..` traversal segment into the computed folder name. +const SAFE_FOLDER_SLUG_PATTERN = /^[a-z0-9-]+$/; + +/** + * Computes the on-disk folder name for a Git repository's app-managed + * location: `git__` when a `folderSlug` snapshot is available, + * otherwise the bare `_id` (e.g. pre-existing repos not yet backfilled). + */ +export function getGitRepoFolderName(repo: Pick): string { + if (!repo.folderSlug || !SAFE_FOLDER_SLUG_PATTERN.test(repo.folderSlug)) { + return repo._id; + } + const hex = repo._id.startsWith(`${prefix}_`) ? repo._id.slice(prefix.length + 1) : repo._id; + return `${prefix}_${repo.folderSlug}_${hex}`; +} + export interface GitAuthor { name: string; email: string; diff --git a/packages/insomnia-smoke-test/playwright/pages/project/index.ts b/packages/insomnia-smoke-test/playwright/pages/project/index.ts index 01ab8fa479e..0546a14e6e9 100644 --- a/packages/insomnia-smoke-test/playwright/pages/project/index.ts +++ b/packages/insomnia-smoke-test/playwright/pages/project/index.ts @@ -251,6 +251,9 @@ export class ProjectPage extends BasePage { if (await projectModalCloseButton.isVisible()) { await projectModalCloseButton.click(); } + // The modal's backdrop still intercepts clicks for a moment after closing + // (exit animation / unmount), which flakily blocks the click below. + await this.page.getByRole('dialog').waitFor({ state: 'hidden' }); await this.page.getByRole('button', { name: 'Personal workspace Organizations' }).click(); await this.page.getByRole('option', { name: /Magic/ }).click(); await this.page.getByRole('button', { name: /Magic/ }).click(); diff --git a/packages/insomnia/src/entry.main.ts b/packages/insomnia/src/entry.main.ts index 7af48ab5956..321f4d39772 100644 --- a/packages/insomnia/src/entry.main.ts +++ b/packages/insomnia/src/entry.main.ts @@ -29,7 +29,7 @@ import { AnalyticsEvent, trackAnalyticsEvent } from './main/analytics'; import { registerInsomniaProtocols } from './main/api.protocol'; import { backupIfNewerVersionAvailable } from './main/backup'; import { registerSyncHandlers } from './main/cloud-sync/ipc'; -import { registerGitServiceAPI } from './main/git-service'; +import { backfillAllManagedGitFolderSlugs, registerGitServiceAPI } from './main/git-service'; import { registerCookieHandlers } from './main/ipc/cookies'; import { ipcMainOn, ipcMainOnce, registerElectronHandlers } from './main/ipc/electron'; import { registerElectronStorageHandlers } from './main/ipc/electron-storage'; @@ -150,6 +150,12 @@ app.on('ready', async () => { sentryWatchAnalyticsEnabled(); await runGitCredentialsMigration(); + // Must run β€” and finish β€” before the window/renderer exists: it's the only + // point in the app's lifecycle where nothing else (a file watcher, a git + // operation kicked off by a route loader) can be concurrently reading or + // writing these same folders, so the rename can never race another reader + // into resurrecting the old path (see backfillManagedFolderSlug). + await backfillAllManagedGitFolderSlugs(); await _launchApp(); diff --git a/packages/insomnia/src/main/git-service.ts b/packages/insomnia/src/main/git-service.ts index 9904d123889..84030ec7dcc 100644 --- a/packages/insomnia/src/main/git-service.ts +++ b/packages/insomnia/src/main/git-service.ts @@ -28,6 +28,7 @@ import type { WorkspaceScope, } from 'insomnia-data'; import { models, services } from 'insomnia-data'; +import { slugify } from 'insomnia-data/common'; import { Errors, type PromiseFsClient } from 'isomorphic-git'; import YAML, { parse } from 'yaml'; @@ -386,6 +387,29 @@ async function assertBranchOnOrigin(context: string): Promise { } } +function getManagedGitRoot(): string { + return path.join(process.env['INSOMNIA_DATA_PATH'] || app.getPath('userData'), 'version-control', 'git'); +} + +/** + * Joins `folderName` onto `gitRoot` and refuses to return a path that would + * escape it, returning `null` instead. `folderName` comes from + * `models.gitRepository.getGitRepoFolderName`, which already restricts + * `folderSlug` to `[a-z0-9-]` β€” this is a second, independent guard at the + * point the path is actually used on disk, in case that invariant is ever + * broken upstream. (`path.join`/`path.normalize` alone would silently collapse + * a `..` segment instead of rejecting it.) + */ +function resolveWithinGitRoot(gitRoot: string, folderName: string): string | null { + const resolvedGitRoot = path.resolve(gitRoot); + const resolvedPath = path.resolve(gitRoot, folderName); + const relative = path.relative(resolvedGitRoot, resolvedPath); + if (relative.startsWith('..') || path.isAbsolute(relative)) { + return null; + } + return resolvedPath; +} + /** * Resolves the absolute base directory where a Git repository's working tree and * .git directory live on disk. @@ -393,27 +417,37 @@ async function assertBranchOnOrigin(context: string): Promise { * - When the repository has a user-chosen `directory`, that path is used as-is * (the user owns it; Insomnia must not delete it on project removal). * - Otherwise the repository lives in the app-managed location - * `{INSOMNIA_DATA_PATH || userData}/version-control/git/{id}`. + * `{INSOMNIA_DATA_PATH || userData}/version-control/git/{folder}`, where + * `folder` is computed by `models.gitRepository.getGitRepoFolderName` (the + * bare id, or `git__` once a `folderSlug` has been set). * * This is the single source of truth for repo paths. Callers that already have - * the GitRepository document should pass `directory` to avoid a DB lookup; - * otherwise the directory is resolved from the database by id. + * the GitRepository document should pass `directory`/`folderSlug` to avoid a DB + * lookup; otherwise both are resolved from the database by id. */ -async function getRepoBaseDir(gitRepositoryId: string, directory?: string | null): Promise { +async function getRepoBaseDir(gitRepositoryId: string, directory?: string | null, folderSlug?: string | null): Promise { let dir = directory; + let slug = folderSlug; if (dir === undefined) { const repo = await services.gitRepository.getById(gitRepositoryId); dir = repo?.directory ?? null; + slug = repo?.folderSlug ?? null; } if (dir) { return dir; } - return path.join( - process.env['INSOMNIA_DATA_PATH'] || app.getPath('userData'), - `version-control/git/${gitRepositoryId}`, - ); + const gitRoot = getManagedGitRoot(); + const folderName = models.gitRepository.getGitRepoFolderName({ _id: gitRepositoryId, folderSlug: slug ?? null }); + const resolved = resolveWithinGitRoot(gitRoot, folderName); + if (!resolved) { + // Should be unreachable β€” getGitRepoFolderName already validates folderSlug β€” but + // fall back to the safe bare-id path rather than ever returning one outside gitRoot. + console.warn('[git] Computed managed repo folder path escaped the git root, falling back to bare id:', folderName); + return path.join(gitRoot, gitRepositoryId); + } + return resolved; } /** @@ -423,13 +457,13 @@ async function getRepoBaseDir(gitRepositoryId: string, directory?: string | null * `directory` belongs to the user and must never be deleted when the project is * removed. Failures are logged, not thrown β€” losing a project record should not be blocked by a stale folder. */ -async function deleteManagedRepoFolderIfOwned(repo: Pick) { +async function deleteManagedRepoFolderIfOwned(repo: Pick) { if (repo.directory) { // User-owned folder β€” leave it on disk. return; } - const baseDir = await getRepoBaseDir(repo._id, null); + const baseDir = await getRepoBaseDir(repo._id, null, repo.folderSlug); try { await fs.promises.rm(baseDir, { recursive: true, force: true }); } catch (e) { @@ -437,6 +471,88 @@ async function deleteManagedRepoFolderIfOwned(repo: Pick>(); + +/** + * One-time, best-effort backfill: gives an app-managed repo folder a + * human-readable name derived from its owning project's name. + * + * MUST only be called from `backfillAllManagedGitFolderSlugs` at main-process + * startup, before the app window/renderer exists. Renaming a folder that any + * other code might concurrently read (a file watcher, a route loader's git + * call) is unsafe: a reader holding the old path can recreate it via the FS + * client's auto-mkdir-on-write behavior microseconds after the rename, + * silently forking the repo's files across both directories. Before startup + * completes, nothing else can be touching these folders yet, so no such + * reader exists. + * + * No-ops (returns `repo` unchanged) when there's nothing on disk yet to rename, + * the target name is already taken, the project has no usable name, or any I/O + * error occurs β€” callers keep working against the legacy (unslugged) folder + * name via `getRepoBaseDir`'s fallback. Never throws. + */ +async function backfillManagedFolderSlug(repo: GitRepository, projectId: string): Promise { + const inFlight = foldersBeingBackfilled.get(repo._id); + if (inFlight) { + return inFlight; + } + + const promise = (async () => { + try { + const project = await services.project.getById(projectId); + const slug = project ? slugify(project.name) : ''; + if (!slug) { + return repo; + } + + const gitRoot = getManagedGitRoot(); + const oldDir = resolveWithinGitRoot(gitRoot, models.gitRepository.getGitRepoFolderName(repo)); + const newDir = resolveWithinGitRoot(gitRoot, models.gitRepository.getGitRepoFolderName({ _id: repo._id, folderSlug: slug })); + if (!oldDir || !newDir) { + // Should be unreachable β€” getGitRepoFolderName already validates folderSlug β€” but + // never rename into/out of a path outside gitRoot. + console.warn('[git] Computed managed repo folder path escaped the git root, skipping backfill for', repo._id); + return repo; + } + + const oldDirExists = await fs.promises + .stat(oldDir) + .then(stat => stat.isDirectory()) + .catch(() => false); + if (!oldDirExists) { + // Nothing on disk yet β€” just record the slug so it's used from now on. + return await services.gitRepository.update(repo, { folderSlug: slug }); + } + + const newDirExists = await fs.promises + .access(newDir) + .then(() => true) + .catch(() => false); + if (newDirExists) { + console.warn('[git] Skipping managed repo folder rename β€” target already exists:', newDir); + return repo; + } + + await fs.promises.rename(oldDir, newDir); + return await services.gitRepository.update(repo, { folderSlug: slug }); + } catch (e) { + console.warn('[git] Failed to give managed repo folder a readable name', e); + return repo; + } + })(); + + foldersBeingBackfilled.set(repo._id, promise); + try { + return await promise; + } finally { + foldersBeingBackfilled.delete(repo._id); + } +} + /** * Creates a file system client for Git operations * Returns different clients based on whether we're working with a workspace or project @@ -445,6 +561,7 @@ async function deleteManagedRepoFolderIfOwned(repo: Pick { + try { + const allProjects = await services.project.list(); + const gitProjects = allProjects.filter((p): p is GitProject => models.project.isConnectedGitProject(p)); + if (gitProjects.length === 0) { + return; + } + + const repoIds = gitProjects.map(p => models.project.getEffectiveRepoId(p)).filter(Boolean) as string[]; + const gitRepositories = await database.find(models.gitRepository.type, { + _id: { $in: repoIds }, + }); + const repoById = new Map(gitRepositories.map(r => [r._id, r])); + + await Promise.all( + gitProjects.map(async project => { + const repoId = models.project.getEffectiveRepoId(project); + const repo = repoId ? repoById.get(repoId) : undefined; + if (!repo || repo.directory || repo.folderSlug || GitVCS.isInitializedForRepo(repo._id)) { + return; + } + await backfillManagedFolderSlug(repo, project._id); + }), + ); + } catch (e) { + console.warn('[git] Failed to backfill managed repo folder names on startup', e); + } +} + export interface MigrationSummary { logs: string[]; failedProjects: { id: string; name: string }[]; diff --git a/packages/insomnia/src/ui/utils/git-repo-path.ts b/packages/insomnia/src/ui/utils/git-repo-path.ts index b3357f9d161..04dd933e969 100644 --- a/packages/insomnia/src/ui/utils/git-repo-path.ts +++ b/packages/insomnia/src/ui/utils/git-repo-path.ts @@ -1,14 +1,16 @@ -import type { GitRepository } from 'insomnia-data'; +import { type GitRepository, models } from 'insomnia-data'; /** * Resolve a Git repository's on-disk base directory in the renderer. * * Mirrors the main-process `getRepoBaseDir` (git-service.ts): a user-chosen - * `directory` when set, else the app-managed location. + * `directory` when set, else the app-managed location (named via + * `models.gitRepository.getGitRepoFolderName`). */ -export const resolveGitRepoBaseDir = (gitRepository: Pick): string => { +export const resolveGitRepoBaseDir = (gitRepository: Pick): string => { if (gitRepository.directory) { return gitRepository.directory; } - return window.path.join(window.app.getPath('userData'), `version-control/git/${gitRepository._id}`); + const folderName = models.gitRepository.getGitRepoFolderName(gitRepository); + return window.path.join(window.app.getPath('userData'), `version-control/git/${folderName}`); };