From 1f7712415f465032185b360e8907c4fafbbc5c6b Mon Sep 17 00:00:00 2001 From: Kyle Date: Sun, 18 Oct 2026 17:54:50 -0400 Subject: [PATCH] chore(sync): add id and path constraints for synced data, aligned with backend --- .../__tests__/blob-path-containment.test.ts | 82 ++++ .../branch-name-path-containment.test.ts | 73 +++ .../store-key-id-containment.test.ts | 130 ++++++ packages/insomnia-vcs/src/backend-projects.ts | 12 + packages/insomnia-vcs/src/store-keys.ts | 45 ++ .../file-system-driver-containment.test.ts | 77 ++++ .../src/store/drivers/file-system-driver.ts | 47 ++ packages/insomnia-vcs/src/vcs.ts | 58 ++- .../src/common/synced-doc-validation.ts | 32 ++ ...backup-restore-version-containment.test.ts | 70 +++ ...lushes-change-buffer-on-early-exit.test.ts | 106 +++++ ...one-legacy-workspace-id-validation.test.ts | 52 +++ ...acy-insomnia-folder-doc-validation.test.ts | 93 ++++ .../src/main/api.protocol-handler.test.ts | 174 +++++++ .../insomnia/src/main/api.protocol.test.ts | 70 +++ packages/insomnia/src/main/api.protocol.ts | 48 +- packages/insomnia/src/main/backup.ts | 23 +- ...ull-backend-project-doc-validation.test.ts | 76 +++ .../main/cloud-sync/pull-backend-project.ts | 19 +- packages/insomnia/src/main/git-service.ts | 434 +++++++++--------- ...e-proto-file-synced-id-containment.test.ts | 76 +++ .../src/network/grpc/write-proto-file.node.ts | 16 +- .../fs-client-path-containment.test.ts | 96 ++++ .../fs-client-symlink-handling.test.ts | 131 ++++++ .../ne-db-client-doc-validation.test.ts | 37 ++ ...-watcher-symlink-write-containment.test.ts | 93 ++++ ...file-watcher-synced-doc-validation.test.ts | 71 +++ .../routable-fs-client-symlink-target.test.ts | 55 +++ ...utable-fs-client-traversal-routing.test.ts | 75 +++ .../insomnia/src/sync/git/contained-path.ts | 65 +++ packages/insomnia/src/sync/git/fs-client.ts | 28 +- .../insomnia/src/sync/git/ne-db-client.ts | 4 + .../sync/git/project-routable-fs-client.ts | 13 + .../src/sync/git/repo-file-watcher.ts | 45 ++ .../src/sync/git/routable-fs-client.ts | 13 + 35 files changed, 2288 insertions(+), 251 deletions(-) create mode 100644 packages/insomnia-vcs/src/__tests__/blob-path-containment.test.ts create mode 100644 packages/insomnia-vcs/src/__tests__/branch-name-path-containment.test.ts create mode 100644 packages/insomnia-vcs/src/__tests__/store-key-id-containment.test.ts create mode 100644 packages/insomnia-vcs/src/store-keys.ts create mode 100644 packages/insomnia-vcs/src/store/drivers/__tests__/file-system-driver-containment.test.ts create mode 100644 packages/insomnia/src/common/synced-doc-validation.ts create mode 100644 packages/insomnia/src/main/__tests__/backup-restore-version-containment.test.ts create mode 100644 packages/insomnia/src/main/__tests__/git-service-clone-flushes-change-buffer-on-early-exit.test.ts create mode 100644 packages/insomnia/src/main/__tests__/git-service-clone-legacy-workspace-id-validation.test.ts create mode 100644 packages/insomnia/src/main/__tests__/git-service-import-legacy-insomnia-folder-doc-validation.test.ts create mode 100644 packages/insomnia/src/main/api.protocol-handler.test.ts create mode 100644 packages/insomnia/src/main/api.protocol.test.ts create mode 100644 packages/insomnia/src/main/cloud-sync/pull-backend-project-doc-validation.test.ts create mode 100644 packages/insomnia/src/network/grpc/__tests__/write-proto-file-synced-id-containment.test.ts create mode 100644 packages/insomnia/src/sync/git/__tests__/fs-client-path-containment.test.ts create mode 100644 packages/insomnia/src/sync/git/__tests__/fs-client-symlink-handling.test.ts create mode 100644 packages/insomnia/src/sync/git/__tests__/ne-db-client-doc-validation.test.ts create mode 100644 packages/insomnia/src/sync/git/__tests__/repo-file-watcher-symlink-write-containment.test.ts create mode 100644 packages/insomnia/src/sync/git/__tests__/repo-file-watcher-synced-doc-validation.test.ts create mode 100644 packages/insomnia/src/sync/git/__tests__/routable-fs-client-symlink-target.test.ts create mode 100644 packages/insomnia/src/sync/git/__tests__/routable-fs-client-traversal-routing.test.ts create mode 100644 packages/insomnia/src/sync/git/contained-path.ts diff --git a/packages/insomnia-vcs/src/__tests__/blob-path-containment.test.ts b/packages/insomnia-vcs/src/__tests__/blob-path-containment.test.ts new file mode 100644 index 000000000000..4b09577cfd06 --- /dev/null +++ b/packages/insomnia-vcs/src/__tests__/blob-path-containment.test.ts @@ -0,0 +1,82 @@ +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { configureStore } from '../store/current-store'; +import FileSystemDriver from '../store/drivers/file-system-driver'; +import { VCS } from '../vcs'; + +const VALID_BLOB_ID = 'a'.repeat(40); + +// Ids that a malicious sync server could return. None may cause a write outside the store. +const maliciousBlobIds: Record = { + 'parent traversal': 'zz/../../../../../../ESCAPED_PROOF.txt', + 'parent traversal without prefix': '../../../../../../ESCAPED_PROOF.txt', + 'absolute path': '/tmp/ESCAPED_PROOF.txt', + 'backslash traversal': 'zz\\..\\..\\..\\..\\..\\..\\ESCAPED_PROOF.txt', + 'NUL byte': 'ab\0/../../../../../../ESCAPED_PROOF.txt', + 'non-hex characters': 'z'.repeat(40), + 'too short': 'abcd', + 'too long': 'a'.repeat(41), + 'empty': '', +}; + +describe('cloud sync blob id path containment', () => { + let root: string; + let storeDir: string; + let vcs: VCS; + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'vcs-blob-containment-')); + const dataDir = path.join(root, 'data'); + storeDir = path.join(dataDir, 'version-control'); + configureStore(FileSystemDriver.create(dataDir)); + vcs = new VCS({ workspaceId: 'workspace_1' }); + await vcs.switchAndCreateBackendProjectIfNotExist('workspace_1', 'Test Workspace'); + }); + + afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + // Every file under `root` must live inside the store directory. + async function filesOutsideStore() { + const entries = await fs.readdir(root, { recursive: true, withFileTypes: true }); + return entries + .filter(e => e.isFile()) + .map(e => path.join(e.parentPath, e.name)) + .filter(p => !p.startsWith(storeDir + path.sep)); + } + + it('stores a well-formed blob id inside the store', async () => { + await vcs._storeBlobsBuffer({ [VALID_BLOB_ID]: Buffer.from('X') }); + expect(await vcs._hasBlob(VALID_BLOB_ID)).toBe(true); + expect((await vcs._getBlobRaw(VALID_BLOB_ID))?.toString()).toBe('X'); + expect(await filesOutsideStore()).toEqual([]); + }); + + describe.each(Object.entries(maliciousBlobIds))('%s', (_label, blobId) => { + it('_storeBlobsBuffer rejects and writes nothing outside the store', async () => { + await expect(vcs._storeBlobsBuffer({ [blobId]: Buffer.from('X') })).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('_storeBlobs rejects and writes nothing outside the store', async () => { + await expect(vcs._storeBlobs({ [blobId]: 'X' })).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('_storeBlob rejects and writes nothing outside the store', async () => { + await expect(vcs._storeBlob(blobId, { a: 1 })).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('read helpers reject', async () => { + await expect(vcs._getBlobRaw(blobId)).rejects.toThrow(); + await expect(vcs._hasBlob(blobId)).rejects.toThrow(); + await expect(vcs._getBlob(blobId)).rejects.toThrow(); + }); + }); +}); diff --git a/packages/insomnia-vcs/src/__tests__/branch-name-path-containment.test.ts b/packages/insomnia-vcs/src/__tests__/branch-name-path-containment.test.ts new file mode 100644 index 000000000000..3d90e4350995 --- /dev/null +++ b/packages/insomnia-vcs/src/__tests__/branch-name-path-containment.test.ts @@ -0,0 +1,73 @@ +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { configureStore } from '../store/current-store'; +import FileSystemDriver from '../store/drivers/file-system-driver'; +import { VCS } from '../vcs'; + +// Branch names can arrive from the sync server. Writes are already restricted by +// VCS.validateBranchName; lookups and removals must be just as strict. +const hostileBranchNames: Record = { + 'parent traversal': '../../../../ESCAPED_PROOF', + 'backslash traversal': '..\\..\\..\\..\\ESCAPED_PROOF', + 'NUL byte': 'main\0/../../ESCAPED_PROOF', + 'bare dot-dot': '..', + 'dot-dot segment': 'feature/../../ESCAPED_PROOF', + 'bare backslash': 'a\\b', +}; + +const legitBranchNames = ['main', 'feature/login', 'fix/a/b/c', 'v1.2.3', 'my_branch-2']; + +describe('cloud sync branch name path containment', () => { + let root: string; + let storeDir: string; + let vcs: VCS; + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'vcs-branch-containment-')); + const dataDir = path.join(root, 'data'); + storeDir = path.join(dataDir, 'version-control'); + configureStore(FileSystemDriver.create(dataDir)); + vcs = new VCS({ workspaceId: 'workspace_1' }); + await vcs.switchAndCreateBackendProjectIfNotExist('workspace_1', 'Test Workspace'); + }); + + afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + describe.each(legitBranchNames)('legit branch name %j', name => { + it('round-trips through the store and can be removed', async () => { + const now = new Date(); + await vcs._storeBranch({ name, created: now, modified: now, snapshots: [] }); + expect((await vcs._getBranch(name.toLowerCase()))?.name).toBe(name); + await vcs._removeBranch({ name: name.toLowerCase(), created: now, modified: now, snapshots: [] }); + expect(await vcs._getBranch(name)).toBeNull(); + }); + }); + + describe.each(Object.entries(hostileBranchNames))('%s', (_label, name) => { + const branch = { name, created: new Date(), modified: new Date(), snapshots: [] }; + + it('_storeBranch rejects and writes nothing outside the store', async () => { + await expect(vcs._storeBranch({ ...branch })).rejects.toThrow(); + const entries = await fs.readdir(root, { recursive: true, withFileTypes: true }); + const outside = entries + .filter(e => e.isFile()) + .map(e => path.join(e.parentPath, e.name)) + .filter(p => !p.startsWith(storeDir + path.sep)); + expect(outside).toEqual([]); + }); + + it('_getBranch rejects instead of reading a path derived from the name', async () => { + await expect(vcs._getBranch(name)).rejects.toThrow(); + }); + + it('_removeBranch rejects instead of removing a path derived from the name', async () => { + await expect(vcs._removeBranch({ ...branch })).rejects.toThrow(); + }); + }); +}); diff --git a/packages/insomnia-vcs/src/__tests__/store-key-id-containment.test.ts b/packages/insomnia-vcs/src/__tests__/store-key-id-containment.test.ts new file mode 100644 index 000000000000..cb2d7cf5f1cb --- /dev/null +++ b/packages/insomnia-vcs/src/__tests__/store-key-id-containment.test.ts @@ -0,0 +1,130 @@ +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { getBackendProjectById, removeProject, storeBackendProject } from '../backend-projects'; +import { configureStore } from '../store/current-store'; +import FileSystemDriver from '../store/drivers/file-system-driver'; +import { VCS } from '../vcs'; + +vi.mock('../session', () => ({ + assertSession: vi.fn(), + runGraphQL: vi.fn(async () => { + throw new Error('network must not be reached for an invalid id'); + }), +})); + +const VALID_SNAPSHOT_ID = 'b'.repeat(40); + +const maliciousSnapshotIds: Record = { + 'parent traversal': '../../../../../../ESCAPED_PROOF', + 'absolute path': '/tmp/ESCAPED_PROOF', + 'backslash traversal': '..\\..\\..\\..\\..\\ESCAPED_PROOF', + 'NUL byte': 'ab\0/../../../../../ESCAPED_PROOF', + 'non-hex characters': 'z'.repeat(40), + 'wrong length': 'abcd', +}; + +const maliciousProjectIds: Record = { + 'parent traversal': '../../../../../ESCAPED_PROOF', + 'nested path': 'prj_a/../../ESCAPED_PROOF', + 'absolute path': '/tmp/ESCAPED_PROOF', + 'backslash traversal': '..\\..\\..\\ESCAPED_PROOF', + 'NUL byte': 'prj_a\0/../../ESCAPED_PROOF', + 'empty': '', + 'dot-dot': '..', +}; + +describe('cloud sync store key id containment', () => { + let root: string; + let storeDir: string; + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'vcs-id-containment-')); + const dataDir = path.join(root, 'data'); + storeDir = path.join(dataDir, 'version-control'); + configureStore(FileSystemDriver.create(dataDir)); + }); + + afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + async function filesOutsideStore() { + const entries = await fs.readdir(root, { recursive: true, withFileTypes: true }); + return entries + .filter(e => e.isFile()) + .map(e => path.join(e.parentPath, e.name)) + .filter(p => !p.startsWith(storeDir + path.sep)); + } + + describe('snapshot ids', () => { + let vcs: VCS; + + beforeEach(async () => { + vcs = new VCS({ workspaceId: 'workspace_1' }); + await vcs.switchAndCreateBackendProjectIfNotExist('workspace_1', 'Test Workspace'); + }); + + it('stores a well-formed snapshot id', async () => { + const snapshot: any = { id: VALID_SNAPSHOT_ID, created: new Date(), parent: '', state: [] }; + await vcs._storeSnapshots([snapshot]); + expect((await vcs._getSnapshot(VALID_SNAPSHOT_ID))?.id).toBe(VALID_SNAPSHOT_ID); + expect(await filesOutsideStore()).toEqual([]); + }); + + describe.each(Object.entries(maliciousSnapshotIds))('%s', (_label, id) => { + const snapshot: any = { created: new Date(), parent: '', state: [] }; + + it('_storeSnapshots rejects and writes nothing outside the store', async () => { + await expect(vcs._storeSnapshots([{ ...snapshot, id }])).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('_storeSnapshot rejects and writes nothing outside the store', async () => { + await expect(vcs._storeSnapshot({ ...snapshot, id })).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('_assertSnapshot rejects', async () => { + await expect(vcs._assertSnapshot(id)).rejects.toThrow(); + }); + + it('_getSnapshot does not look the id up', async () => { + expect(await vcs._getSnapshot(id)).toBeNull(); + }); + }); + }); + + describe('backend project ids', () => { + it('stores a well-formed project id', async () => { + const project = { id: 'prj_0123456789abcdef0123456789abcdef', name: 'n', rootDocumentId: 'wrk_1' }; + await storeBackendProject(project); + expect(await getBackendProjectById(project.id)).toEqual(project); + await removeProject(project); + }); + + describe.each(Object.entries(maliciousProjectIds))('%s', (_label, id) => { + const project = { id, name: 'n', rootDocumentId: 'wrk_1' }; + + it('storeBackendProject rejects and writes nothing outside the store', async () => { + await expect(storeBackendProject(project)).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('setBackendProject rejects and writes nothing outside the store', async () => { + const vcs = new VCS({ workspaceId: 'workspace_1' }); + await expect(vcs.setBackendProject(project)).rejects.toThrow(); + expect(vcs.hasBackendProject()).toBe(false); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('getBackendProjectById / removeProject reject', async () => { + await expect(getBackendProjectById(id)).rejects.toThrow(); + await expect(removeProject(project)).rejects.toThrow(); + }); + }); + }); +}); diff --git a/packages/insomnia-vcs/src/backend-projects.ts b/packages/insomnia-vcs/src/backend-projects.ts index df9e36d93025..20cc12e5ea76 100644 --- a/packages/insomnia-vcs/src/backend-projects.ts +++ b/packages/insomnia-vcs/src/backend-projects.ts @@ -4,6 +4,7 @@ import { deterministicStringify, generateId } from 'insomnia-data/common'; import { runGraphQL } from './session'; import { getStore } from './store/current-store'; +import { assertValidBackendProjectId, isValidBackendProjectId } from './store-keys'; import type { BackendProject, BackendProjectWithTeams, BackendProjectWithTeamsAndTeamProjectId, Branch } from './types'; // These helpers never read a VCS instance's active `_backendProject` — they operate on an @@ -12,10 +13,12 @@ import type { BackendProject, BackendProjectWithTeams, BackendProjectWithTeamsAn // activating a workspace-scoped VCS instance. export async function getBackendProjectById(projectId: string): Promise { + assertValidBackendProjectId(projectId); return getStore().getItem(`/projects/${projectId}/meta.json`); } export async function storeBackendProject(project: BackendProject) { + assertValidBackendProjectId(project.id); let existingProject: BackendProject | null = null; try { existingProject = await getBackendProjectById(project.id); @@ -32,6 +35,7 @@ export async function storeBackendProject(project: BackendProject) { } export async function removeProject(project: BackendProject) { + assertValidBackendProjectId(project.id); console.log(`[sync] Remove local project ${project.id}`); return getStore().removeItem(`/projects/${project.id}/meta.json`); } @@ -42,6 +46,12 @@ export async function localBackendProjects(): Promise { for (const key of keys) { const id = path.basename(key); + + // Ignore anything in the store that isn't a well-formed project id + if (!isValidBackendProjectId(id)) { + continue; + } + const p = await getBackendProjectById(id); if (p === null) { @@ -56,6 +66,7 @@ export async function localBackendProjects(): Promise { } export async function getBranches(backendProjectId: string): Promise { + assertValidBackendProjectId(backendProjectId); const branches: Branch[] = []; for (const p of await getStore().keys(`/projects/${backendProjectId}/branches/`)) { @@ -143,6 +154,7 @@ export async function removeBackendProjectsForRoot(rootDocumentId: string) { // Archives a remote backend project by id, without requiring it to be pulled locally first. export async function archiveBackendProject(backendProjectId: string) { + assertValidBackendProjectId(backendProjectId); await runGraphQL( ` mutation ($id: ID!) { diff --git a/packages/insomnia-vcs/src/store-keys.ts b/packages/insomnia-vcs/src/store-keys.ts new file mode 100644 index 000000000000..a57a781506e4 --- /dev/null +++ b/packages/insomnia-vcs/src/store-keys.ts @@ -0,0 +1,45 @@ +// Identifiers that originate from (or can be influenced by) the sync server end up as path +// segments in store keys. Every such identifier must be validated here before use so that a +// hostile server can never steer a read/write outside the intended directory. + +// Blob ids are the sha1 hex digest of the document content (see `hash` in util.ts). +const BLOB_ID_PATTERN = /^[0-9a-f]{40}$/; + +export function assertValidBlobId(blobId: unknown): asserts blobId is string { + if (typeof blobId !== 'string' || !BLOB_ID_PATTERN.test(blobId)) { + throw new Error('Invalid blob id'); + } +} + +/** Store key for a blob, sharded by the first two hex characters of its id. */ +export function blobKey(backendProjectId: string, blobId: string) { + assertValidBlobId(blobId); + return `/projects/${backendProjectId}/blobs/${blobId.slice(0, 2)}/${blobId.slice(2)}`; +} + +// Snapshot ids are the sha1 hex digest produced by `_generateSnapshotID` in vcs.ts. +const SNAPSHOT_ID_PATTERN = /^[0-9a-f]{40}$/; + +export function isValidSnapshotId(id: unknown): id is string { + return typeof id === 'string' && SNAPSHOT_ID_PATTERN.test(id); +} + +export function assertValidSnapshotId(id: unknown): asserts id is string { + if (!isValidSnapshotId(id)) { + throw new Error('Invalid snapshot id'); + } +} + +// Backend project ids are generated locally as `prj_<32 hex>` (generateId), but projects created +// elsewhere are issued by the server, so only the character set and length are constrained. +const BACKEND_PROJECT_ID_PATTERN = /^[A-Za-z0-9_-]{1,128}$/; + +export function isValidBackendProjectId(id: unknown): id is string { + return typeof id === 'string' && BACKEND_PROJECT_ID_PATTERN.test(id); +} + +export function assertValidBackendProjectId(id: unknown): asserts id is string { + if (!isValidBackendProjectId(id)) { + throw new Error('Invalid backend project id'); + } +} diff --git a/packages/insomnia-vcs/src/store/drivers/__tests__/file-system-driver-containment.test.ts b/packages/insomnia-vcs/src/store/drivers/__tests__/file-system-driver-containment.test.ts new file mode 100644 index 000000000000..baa027e894a1 --- /dev/null +++ b/packages/insomnia-vcs/src/store/drivers/__tests__/file-system-driver-containment.test.ts @@ -0,0 +1,77 @@ +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import FileSystemDriver from '../file-system-driver'; + +const maliciousKeys: Record = { + 'parent traversal': '/projects/p/blobs/zz/../../../../../ESCAPED_PROOF.txt', + 'leading parent traversal': '../ESCAPED_PROOF.txt', + 'bare parent': '..', + 'NUL byte': '/projects/p\0/../../../ESCAPED_PROOF.txt', +}; + +describe('FileSystemDriver path containment', () => { + let root: string; + let storeDir: string; + let driver: FileSystemDriver; + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'fs-driver-containment-')); + const dataDir = path.join(root, 'data'); + storeDir = path.join(dataDir, 'version-control'); + driver = FileSystemDriver.create(dataDir); + }); + + afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + async function filesOutsideStore() { + const entries = await fs.readdir(root, { recursive: true, withFileTypes: true }); + return entries + .filter(e => e.isFile()) + .map(e => path.join(e.parentPath, e.name)) + .filter(p => !p.startsWith(storeDir + path.sep)); + } + + it('round-trips a normal key', async () => { + await driver.setItem('/projects/p/meta.json', Buffer.from('ok')); + expect(await driver.hasItem('/projects/p/meta.json')).toBe(true); + expect((await driver.getItem('/projects/p/meta.json'))?.toString()).toBe('ok'); + expect(await driver.keys('/projects/', true)).toEqual(['/projects/p/meta.json']); + await driver.removeItem('/projects/p/meta.json'); + expect(await driver.hasItem('/projects/p/meta.json')).toBe(false); + }); + + describe.each(Object.entries(maliciousKeys))('%s', (_label, key) => { + it('setItem rejects and writes nothing outside the store', async () => { + await expect(driver.setItem(key, Buffer.from('X'))).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); + + it('getItem/hasItem/removeItem/keys reject', async () => { + await expect(driver.getItem(key)).rejects.toThrow(); + await expect(driver.hasItem(key)).rejects.toThrow(); + await expect(driver.removeItem(key)).rejects.toThrow(); + await expect(driver.keys(key, true)).rejects.toThrow(); + }); + }); + + it('does not create directories outside the store for a rejected key', async () => { + await expect(driver.setItem('/../../escaped-dir/x/y.txt', Buffer.from('X'))).rejects.toThrow(); + await expect(fs.stat(path.join(root, 'escaped-dir'))).rejects.toThrow(); + }); + + it('does not write through a symlinked directory that points outside the store', async () => { + const outside = path.join(root, 'outside'); + await fs.mkdir(outside, { recursive: true }); + await fs.mkdir(path.join(storeDir, 'projects'), { recursive: true }); + await fs.symlink(outside, path.join(storeDir, 'projects', 'link')); + + await expect(driver.setItem('/projects/link/pwned.txt', Buffer.from('X'))).rejects.toThrow(); + expect(await filesOutsideStore()).toEqual([]); + }); +}); diff --git a/packages/insomnia-vcs/src/store/drivers/file-system-driver.ts b/packages/insomnia-vcs/src/store/drivers/file-system-driver.ts index dc4a4b7f87c1..7e092167397b 100644 --- a/packages/insomnia-vcs/src/store/drivers/file-system-driver.ts +++ b/packages/insomnia-vcs/src/store/drivers/file-system-driver.ts @@ -123,10 +123,57 @@ export default class FileSystemDriver implements BaseDriver { return keys; } + /** + * Resolve a store key to an absolute path, guaranteeing the result stays inside this._directory. + * Keys can embed server-supplied identifiers, so they are treated as untrusted. + */ async _getKeyPath(key: string) { + if (typeof key !== 'string' || key.includes('\0')) { + throw new Error('Invalid store key'); + } + const p = path.join(this._directory, key); + this._assertLexicallyInside(p, key); + + // Make sure the base directory exists so that it can be canonicalized + await fs.mkdir(this._directory, { recursive: true }); + const realBase = await fs.realpath(this._directory); + + // Existing ancestors may be symlinks that point outside the store. Check the nearest + // existing ancestor BEFORE creating any directories. + let ancestor = path.dirname(p); + for (;;) { + try { + const realAncestor = await fs.realpath(ancestor); + this._assertInside(realBase, realAncestor, key); + break; + } catch (err) { + if (!(err && typeof err === 'object' && 'code' in err && err.code === 'ENOENT')) { + throw err; + } + + const parent = path.dirname(ancestor); + if (parent === ancestor) { + throw err; + } + ancestor = parent; + } + } + // Create base directory await fs.mkdir(path.dirname(p), { recursive: true }); return p; } + + _assertLexicallyInside(resolvedPath: string, key: string) { + this._assertInside(path.resolve(this._directory), path.resolve(resolvedPath), key); + } + + _assertInside(base: string, target: string, key: string) { + const rel = path.relative(base, target); + + if (rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) { + throw new Error(`Store key escapes the store directory: ${JSON.stringify(key)}`); + } + } } diff --git a/packages/insomnia-vcs/src/vcs.ts b/packages/insomnia-vcs/src/vcs.ts index 737137a9e467..c00fb741bd53 100644 --- a/packages/insomnia-vcs/src/vcs.ts +++ b/packages/insomnia-vcs/src/vcs.ts @@ -16,6 +16,13 @@ import { import * as crypt from './crypt'; import { assertSession, runGraphQL } from './session'; import { getStore } from './store/current-store'; +import { + assertValidBackendProjectId, + assertValidBlobId, + assertValidSnapshotId, + blobKey, + isValidSnapshotId, +} from './store-keys'; import type { BackendProject, Branch, @@ -120,6 +127,7 @@ export class VCS { } async setBackendProject(backendProject: BackendProject) { + assertValidBackendProjectId(backendProject.id); this._backendProject = backendProject; console.debug(`[sync] Activated project ${backendProject.id}`); // Store it because it might not be yet @@ -1013,6 +1021,7 @@ export class VCS { ); for (const blob of blobs) { + assertValidBlobId(blob.id); const encryptedResult = JSON.parse(blob.content); result[blob.id] = crypt.decryptAESToBuffer(symmetricKey, encryptedResult); } @@ -1322,6 +1331,7 @@ export class VCS { } async _assertSnapshot(id: string) { + assertValidSnapshotId(id); const snapshot: Snapshot | null = await getStore().getItem( `/projects/${this._backendProjectId()}/snapshots/${id}.json`, ); @@ -1338,6 +1348,11 @@ export class VCS { } async _getSnapshot(id: string) { + // Not a valid snapshot id (e.g. the 'n/a' sentinel), so it can't exist in the store + if (!isValidSnapshotId(id)) { + return null; + } + const snapshot: Snapshot | null = await getStore().getItem( `/projects/${this._backendProjectId()}/snapshots/${id}.json`, ); @@ -1357,10 +1372,13 @@ export class VCS { } async _storeSnapshot(snapshot: Snapshot) { + assertValidSnapshotId(snapshot.id); return getStore().setItem(`/projects/${this._backendProjectId()}/snapshots/${snapshot.id}.json`, snapshot); } async _storeSnapshots(snapshots: Snapshot[]) { + // Validate every id up front so a bad id can't result in a partial write + snapshots.forEach(s => assertValidSnapshotId(s.id)); const promises: Promise[] = []; for (const snapshot of snapshots) { @@ -1398,10 +1416,8 @@ export class VCS { await getStore().setItem(`/projects/${this._backendProjectId()}/head.json`, head); } - _getBlob(blobId: string) { - return getStore().getItem( - `/projects/${this._backendProjectId()}/blobs/${blobId.slice(0, 2)}/${blobId.slice(2)}`, - ) as Promise; + async _getBlob(blobId: string) { + return getStore().getItem(blobKey(this._backendProjectId(), blobId)) as Promise; } async _getBlobs(ids: string[]) { @@ -1415,51 +1431,40 @@ export class VCS { } async _storeBlob(blobId: string, content: Record | null) { - return getStore().setItem( - `/projects/${this._backendProjectId()}/blobs/${blobId.slice(0, 2)}/${blobId.slice(2)}`, - content, - ); + return getStore().setItem(blobKey(this._backendProjectId(), blobId), content); } async _storeBlobs(blobsToStore: Record) { + // Validate every id up front so a bad id can't result in a partial write + Object.keys(blobsToStore).forEach(assertValidBlobId); const promises: Promise[] = []; for (const blobId of Object.keys(blobsToStore)) { const buff = Buffer.from(blobsToStore[blobId], 'utf8'); - promises.push( - getStore().setItem( - `/projects/${this._backendProjectId()}/blobs/${blobId.slice(0, 2)}/${blobId.slice(2)}`, - buff, - ), - ); + promises.push(getStore().setItem(blobKey(this._backendProjectId(), blobId), buff)); } await Promise.all(promises); } async _storeBlobsBuffer(decryptedBlobs: Record) { + // Validate every id up front so a bad id can't result in a partial write + Object.keys(decryptedBlobs).forEach(assertValidBlobId); const promises: Promise[] = []; for (const blobId of Object.keys(decryptedBlobs)) { - promises.push( - getStore().setItemRaw( - `/projects/${this._backendProjectId()}/blobs/${blobId.slice(0, 2)}/${blobId.slice(2)}`, - decryptedBlobs[blobId], - ), - ); + promises.push(getStore().setItemRaw(blobKey(this._backendProjectId(), blobId), decryptedBlobs[blobId])); } await Promise.all(promises); } async _getBlobRaw(blobId: string) { - return getStore().getItemRaw( - `/projects/${this._backendProjectId()}/blobs/${blobId.slice(0, 2)}/${blobId.slice(2)}`, - ); + return getStore().getItemRaw(blobKey(this._backendProjectId(), blobId)); } async _hasBlob(blobId: string) { - return getStore().hasItem(`/projects/${this._backendProjectId()}/blobs/${blobId.slice(0, 2)}/${blobId.slice(2)}`); + return getStore().hasItem(blobKey(this._backendProjectId(), blobId)); } getActiveBackendProject() { @@ -1480,5 +1485,10 @@ function _generateSnapshotID(parentId: string, backendProjectId: string, state: } function encodeBranchName(branchName: string) { + // Branch names can come from the sync server; never let one influence the path beyond its own file name + if (/[\\\0]/.test(branchName) || branchName.split('/').some(part => part === '..' || part === '.')) { + throw new Error('Invalid branch name'); + } + return branchName.replaceAll('/', '~'); } diff --git a/packages/insomnia/src/common/synced-doc-validation.ts b/packages/insomnia/src/common/synced-doc-validation.ts new file mode 100644 index 000000000000..b1102fe75a17 --- /dev/null +++ b/packages/insomnia/src/common/synced-doc-validation.ts @@ -0,0 +1,32 @@ +// `_id` and `parentId` of docs received from remote sources (cloud sync pulls, git repos) are later +// interpolated into on-disk names (proto temp files, response/timeline paths, git yaml names), so they +// must be a single safe path segment: no separators, dots, NUL or other special characters. +// +// The charset matches what generateId() produces ("_") plus the fixed ids used by +// imports and legacy data (e.g. "__WORKSPACE_ID__", "__BASE_ENVIRONMENT_ID__", prefixes containing '-'). +// The model prefix is deliberately NOT enforced: legacy and imported docs have ids that do not carry it. +const SAFE_SYNCED_ID_PATTERN = /^[A-Za-z0-9_-]{1,255}$/; + +export const isSafeSyncedId = (value: unknown): value is string => + typeof value === 'string' && SAFE_SYNCED_ID_PATTERN.test(value); + +/** + * Throws when a doc received from a remote source has an unsafe `_id`, an unsafe `parentId` + * (null/undefined/'' are allowed), or a non-string `type`. + */ +export const assertSafeSyncedDoc = (doc: unknown): void => { + const candidate = doc as { _id?: unknown; type?: unknown; parentId?: unknown } | null | undefined; + if (!candidate || typeof candidate !== 'object') { + throw new Error('Refusing synced document: not an object'); + } + if (typeof candidate.type !== 'string') { + throw new TypeError(`Refusing synced document "${String(candidate._id)}": invalid type`); + } + if (!isSafeSyncedId(candidate._id)) { + throw new Error(`Refusing synced ${candidate.type} document: unsafe _id "${String(candidate._id)}"`); + } + const { parentId } = candidate; + if (parentId !== null && parentId !== undefined && parentId !== '' && !isSafeSyncedId(parentId)) { + throw new Error(`Refusing synced ${candidate.type} document "${candidate._id}": unsafe parentId "${String(parentId)}"`); + } +}; diff --git a/packages/insomnia/src/main/__tests__/backup-restore-version-containment.test.ts b/packages/insomnia/src/main/__tests__/backup-restore-version-containment.test.ts new file mode 100644 index 000000000000..aee10f4facb9 --- /dev/null +++ b/packages/insomnia/src/main/__tests__/backup-restore-version-containment.test.ts @@ -0,0 +1,70 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const relaunch = vi.fn(); +const exit = vi.fn(); + +vi.mock('electron', () => ({ + default: { app: { relaunch: (...a: any[]) => relaunch(...a), exit: (...a: any[]) => exit(...a), getPath: () => '' }, net: { fetch: vi.fn() } }, +})); +vi.mock('insomnia-data', () => ({ services: { settings: { get: vi.fn() } } })); +vi.mock('~/main/updates', () => ({ getUpdateUrl: vi.fn() })); + +import { restoreBackup } from '../backup'; + +describe('restoreBackup version containment', () => { + let root: string; + let dataPath: string; + const originalEnv = process.env['INSOMNIA_DATA_PATH']; + + beforeEach(async () => { + root = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'insomnia-backup-')); + dataPath = path.join(root, 'data'); + await fs.promises.mkdir(path.join(dataPath, 'backups', '1.0.0'), { recursive: true }); + await fs.promises.writeFile(path.join(dataPath, 'backups', '1.0.0', 'good.db'), 'good'); + // a directory outside backups/ holding an attacker controlled database + await fs.promises.mkdir(path.join(root, 'evil')); + await fs.promises.writeFile(path.join(root, 'evil', 'evil.db'), 'evil'); + await fs.promises.mkdir(path.join(dataPath, 'sibling')); + await fs.promises.writeFile(path.join(dataPath, 'sibling', 'sib.db'), 'sib'); + process.env['INSOMNIA_DATA_PATH'] = dataPath; + relaunch.mockClear(); + exit.mockClear(); + }); + + afterEach(async () => { + if (originalEnv === undefined) { + delete process.env['INSOMNIA_DATA_PATH']; + } else { + process.env['INSOMNIA_DATA_PATH'] = originalEnv; + } + await fs.promises.rm(root, { recursive: true, force: true }); + }); + + it('restores a legitimate version', async () => { + await restoreBackup('1.0.0'); + await vi.waitFor(() => expect(fs.readFileSync(path.join(dataPath, 'good.db'), 'utf8')).toBe('good')); + expect(relaunch).toHaveBeenCalled(); + }); + + it.each(['../../evil', '../../../evil', '..', '.', '', '../sibling', '1.0.0/../../sibling', path.join(os.tmpdir(), 'x'), '1.0.0\0'])( + 'rejects version %j and copies nothing into the data dir', + async version => { + await expect(restoreBackup(version)).rejects.toThrow(); + await new Promise(resolve => setTimeout(resolve, 50)); + expect(fs.existsSync(path.join(dataPath, 'evil.db'))).toBe(false); + expect(fs.existsSync(path.join(dataPath, 'sib.db'))).toBe(false); + expect(fs.existsSync(path.join(dataPath, 'backups', 'evil.db'))).toBe(false); + expect(relaunch).not.toHaveBeenCalled(); + }, + ); + + it('rejects an absolute path to a directory outside backups', async () => { + await expect(restoreBackup(path.join(root, 'evil'))).rejects.toThrow(); + await new Promise(resolve => setTimeout(resolve, 50)); + expect(fs.existsSync(path.join(dataPath, 'evil.db'))).toBe(false); + }); +}); diff --git a/packages/insomnia/src/main/__tests__/git-service-clone-flushes-change-buffer-on-early-exit.test.ts b/packages/insomnia/src/main/__tests__/git-service-clone-flushes-change-buffer-on-early-exit.test.ts new file mode 100644 index 000000000000..0b08ba33360c --- /dev/null +++ b/packages/insomnia/src/main/__tests__/git-service-clone-flushes-change-buffer-on-early-exit.test.ts @@ -0,0 +1,106 @@ +import { services } from 'insomnia-data'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import YAML from 'yaml'; + +import { database as db } from '~/common/database'; + +vi.mock('~/main/analytics', async () => { + const { AnalyticsEvent } = await import('insomnia-analytics'); + return { + AnalyticsEvent, + trackAnalyticsEvent: vi.fn(), + setCurrentOrganizationId: vi.fn(), + trackPageView: vi.fn(), + }; +}); + +const { legacyWorkspaceIds } = vi.hoisted(() => ({ legacyWorkspaceIds: { value: [] as string[] } })); + +// Pretend the remote repo contains one legacy `.insomnia/Workspace/.yaml` per configured id. +vi.mock('~/sync/git/shallow-clone', () => ({ + shallowClone: vi.fn(async ({ fsClient }: { fsClient: any }) => { + if (legacyWorkspaceIds.value.length === 0) { + return; + } + await fsClient.promises.mkdir('.insomnia'); + await fsClient.promises.mkdir('.insomnia/Workspace'); + for (const [index, id] of legacyWorkspaceIds.value.entries()) { + await fsClient.promises.writeFile( + `.insomnia/Workspace/wrk_legacy_${index}.yaml`, + YAML.stringify({ type: 'Workspace', _id: id, name: 'x', scope: 'collection' }), + ); + } + }), +})); + +const { cloneGitRepoAction } = await import('~/main/git-service'); + +describe('cloneGitRepoAction change buffer', () => { + beforeEach(async () => { + await db.init({ inMemoryOnly: true }, true); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + const spyOnBuffer = () => { + const bufferSpy = vi.spyOn(db, 'bufferChanges'); + const flushSpy = vi.spyOn(db, 'flushChanges'); + return { bufferSpy, flushSpy }; + }; + + const expectFlushedOnce = async ({ bufferSpy, flushSpy }: ReturnType) => { + expect(bufferSpy).toHaveBeenCalledTimes(1); + const bufferId = await bufferSpy.mock.results[0].value; + expect(flushSpy).toHaveBeenCalledTimes(1); + expect(flushSpy).toHaveBeenCalledWith(bufferId); + }; + + it('flushes the change buffer when a legacy workspace with an unsafe _id is refused', async () => { + legacyWorkspaceIds.value = ['wrk_../../../x']; + const project = await services.project.create({ name: 'p', parentId: 'org_test' }); + const spies = spyOnBuffer(); + + const result = await cloneGitRepoAction({ + organizationId: 'org_test', + projectId: project._id, + credentialsId: null, + uri: 'https://example.com/repo.git', + }); + + expect(result.errors?.[0]).toMatch(/unsafe _id/); + await expectFlushedOnce(spies); + }); + + it('flushes the change buffer when the repository contains multiple workspaces', async () => { + legacyWorkspaceIds.value = ['wrk_one', 'wrk_two']; + const project = await services.project.create({ name: 'p', parentId: 'org_test' }); + const spies = spyOnBuffer(); + + const result = await cloneGitRepoAction({ + organizationId: 'org_test', + projectId: project._id, + credentialsId: null, + uri: 'https://example.com/repo.git', + }); + + expect(result.errors?.[0]).toMatch(/Multiple workspaces/); + await expectFlushedOnce(spies); + }); + + it('flushes the change buffer when a full clone into a missing project throws', async () => { + legacyWorkspaceIds.value = []; + const spies = spyOnBuffer(); + + const result = await cloneGitRepoAction({ + organizationId: 'org_test', + cloneIntoProjectId: 'proj_missing', + credentialsId: null, + uri: 'https://example.com/repo.git', + }); + + expect(result.errors?.[0]).toMatch(/Project not found/); + await expectFlushedOnce(spies); + }); +}); diff --git a/packages/insomnia/src/main/__tests__/git-service-clone-legacy-workspace-id-validation.test.ts b/packages/insomnia/src/main/__tests__/git-service-clone-legacy-workspace-id-validation.test.ts new file mode 100644 index 000000000000..de2e5da54044 --- /dev/null +++ b/packages/insomnia/src/main/__tests__/git-service-clone-legacy-workspace-id-validation.test.ts @@ -0,0 +1,52 @@ +import { services } from 'insomnia-data'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import YAML from 'yaml'; + +import { database as db } from '~/common/database'; + +vi.mock('~/main/analytics', async () => { + const { AnalyticsEvent } = await import('insomnia-analytics'); + return { + AnalyticsEvent, + trackAnalyticsEvent: vi.fn(), + setCurrentOrganizationId: vi.fn(), + trackPageView: vi.fn(), + }; +}); + +const { legacyWorkspaceId } = vi.hoisted(() => ({ legacyWorkspaceId: { value: '' } })); + +// Pretend the remote repo contains a legacy `.insomnia/Workspace/.yaml` whose `_id` is attacker controlled. +vi.mock('~/sync/git/shallow-clone', () => ({ + shallowClone: vi.fn(async ({ fsClient }: { fsClient: any }) => { + await fsClient.promises.mkdir('.insomnia'); + await fsClient.promises.mkdir('.insomnia/Workspace'); + await fsClient.promises.writeFile( + '.insomnia/Workspace/wrk_legacy.yaml', + YAML.stringify({ type: 'Workspace', _id: legacyWorkspaceId.value, name: 'x', scope: 'collection' }), + ); + }), +})); + +const { cloneGitRepoAction } = await import('~/main/git-service'); + +describe('cloneGitRepoAction legacy workspace id validation', () => { + beforeEach(async () => { + await db.init({ inMemoryOnly: true }, true); + }); + + it('refuses a legacy workspace with an unsafe _id before creating any metadata for it', async () => { + legacyWorkspaceId.value = 'wrk_../../../x'; + const project = await services.project.create({ name: 'p', parentId: 'org_test' }); + + const result = await cloneGitRepoAction({ + organizationId: 'org_test', + projectId: project._id, + credentialsId: null, + uri: 'https://example.com/repo.git', + }); + + expect(await services.workspaceMeta.getByParentId('wrk_../../../x')).toBeFalsy(); + expect(result.errors?.[0]).toMatch(/unsafe _id/); + }); +}); diff --git a/packages/insomnia/src/main/__tests__/git-service-import-legacy-insomnia-folder-doc-validation.test.ts b/packages/insomnia/src/main/__tests__/git-service-import-legacy-insomnia-folder-doc-validation.test.ts new file mode 100644 index 000000000000..ca69d4916465 --- /dev/null +++ b/packages/insomnia/src/main/__tests__/git-service-import-legacy-insomnia-folder-doc-validation.test.ts @@ -0,0 +1,93 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { services } from 'insomnia-data'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import YAML from 'yaml'; + +import { database as db } from '~/common/database'; +import { fsClient } from '~/sync/git/fs-client'; + +vi.mock('~/main/analytics', async () => { + const { AnalyticsEvent } = await import('insomnia-analytics'); + return { + AnalyticsEvent, + trackAnalyticsEvent: vi.fn(), + setCurrentOrganizationId: vi.fn(), + trackPageView: vi.fn(), + }; +}); + +const { importLegacyInsomniaFolder } = await import('~/main/git-service'); + +describe('importLegacyInsomniaFolder() doc id validation', () => { + let repoDir: string; + + beforeEach(async () => { + await db.init({ inMemoryOnly: true }, true); + repoDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'insomnia-legacy-validation-')); + }); + + afterEach(async () => { + await fs.promises.rm(repoDir, { recursive: true, force: true }); + }); + + // The file name is independent from the `_id` inside the file, which is attacker controlled. + const writeRaw = (type: string, fileName: string, doc: Record) => { + const typeDir = path.join(repoDir, '.insomnia', type); + fs.mkdirSync(typeDir, { recursive: true }); + fs.writeFileSync(path.join(typeDir, fileName), YAML.stringify({ type, ...doc })); + }; + + it('refuses a workspace whose _id passes the file path substring check but is not a safe segment', async () => { + // '.insomnia/Workspace/wrk_x.yaml' includes 'Workspace/wrk_x', so the old check let this through + writeRaw('Workspace', 'wrk_x.yaml', { _id: 'Workspace/wrk_x', name: 'evil', scope: 'collection' }); + + const result = await importLegacyInsomniaFolder({ fsClient: fsClient(repoDir), projectId: 'proj_test' }); + + expect(result.errors?.[0]).toMatch(/unsafe _id/); + expect(await db.findOne('Workspace', { _id: 'Workspace/wrk_x' })).toBeUndefined(); + expect(await services.workspaceMeta.getByParentId('Workspace/wrk_x')).toBeUndefined(); + expect(result.changes).toBeUndefined(); + }); + + it('refuses an empty _id, which is a substring of every file path', async () => { + writeRaw('Request', 'req_x.yaml', { _id: '', parentId: 'wrk_a' }); + const result = await importLegacyInsomniaFolder({ fsClient: fsClient(repoDir), projectId: 'proj_test' }); + expect(result.errors?.[0]).toMatch(/unsafe _id/); + }); + + it('refuses a doc with an unsafe parentId', async () => { + writeRaw('Request', 'req_x.yaml', { _id: 'req_x', parentId: '../../etc' }); + const result = await importLegacyInsomniaFolder({ fsClient: fsClient(repoDir), projectId: 'proj_test' }); + expect(result.errors?.[0]).toMatch(/unsafe parentId/); + expect(await db.findOne('Request', { _id: 'req_x' })).toBeUndefined(); + }); + + it('applies nothing when a later doc is hostile (all-or-nothing)', async () => { + writeRaw('Request', 'req_a.yaml', { _id: 'req_a', parentId: 'wrk_a', name: 'benign' }); + writeRaw('Workspace', 'wrk_a.yaml', { _id: 'wrk_a', name: 'benign', scope: 'collection' }); + writeRaw('Workspace', 'wrk_z.yaml', { _id: 'Workspace/wrk_z', name: 'evil', scope: 'collection' }); + + const result = await importLegacyInsomniaFolder({ fsClient: fsClient(repoDir), projectId: 'proj_test' }); + + expect(result.errors).toBeDefined(); + expect(await db.findOne('Request', { _id: 'req_a' })).toBeUndefined(); + expect(await db.findOne('Workspace', { _id: 'wrk_a' })).toBeUndefined(); + expect(await services.workspaceMeta.getByParentId('wrk_a')).toBeUndefined(); + expect(fs.existsSync(path.join(repoDir, '.insomnia'))).toBe(true); + }); + + it('applies nothing when a later file has a merge conflict', async () => { + writeRaw('Request', 'req_a.yaml', { _id: 'req_a', parentId: 'wrk_a' }); + const dir = path.join(repoDir, '.insomnia', 'Workspace'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'wrk_a.yaml'), 'a: 1\n=======\nb: 2\n'); + + const result = await importLegacyInsomniaFolder({ fsClient: fsClient(repoDir), projectId: 'proj_test' }); + + expect(result.errors?.[0]).toMatch(/merge conflict/); + expect(await db.findOne('Request', { _id: 'req_a' })).toBeUndefined(); + }); +}); diff --git a/packages/insomnia/src/main/api.protocol-handler.test.ts b/packages/insomnia/src/main/api.protocol-handler.test.ts new file mode 100644 index 000000000000..d17d0bf82c10 --- /dev/null +++ b/packages/insomnia/src/main/api.protocol-handler.test.ts @@ -0,0 +1,174 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; + +const electron = vi.hoisted(() => ({ + handlers: new Map Promise>(), + netFetch: vi.fn(async (_input: unknown, _init?: unknown) => new Response('fetched', { status: 200 })), +})); + +vi.mock('electron', () => ({ + app: { whenReady: vi.fn(async () => {}) }, + net: { fetch: electron.netFetch }, + protocol: { + registerSchemesAsPrivileged: vi.fn(), + isProtocolHandled: vi.fn(() => false), + handle: vi.fn((scheme: string, handler: (request: Request) => Promise) => { + electron.handlers.set(scheme, handler); + }), + }, + session: {}, +})); +vi.mock('@getinsomnia/node-libcurl', () => ({ Curl: vi.fn(), CurlAuth: {}, CurlFeature: {}, CurlSslOpt: {} })); +vi.mock('insomnia-data', () => ({ services: {} })); +vi.mock('insomnia-data/common', () => ({ ProxyScopes: {} })); +vi.mock('./network/libcurl-promise', () => ({})); +vi.mock('./templating-worker-database', () => ({ resolveDbByKey: vi.fn() })); + +import { registerInsomniaProtocols } from './api.protocol'; + +// The handler resolves its root as path.resolve(__dirname, 'client'). The real directory does not exist in the +// source tree, so we redirect only that exact call to a per-test tmp directory. Everything else runs for real. +const mainSrcDir = path.dirname(fileURLToPath(import.meta.url)); +const originalResolve = path.resolve.bind(path); + +let base: string; +let clientRoot: string; +let assetsDir: string; +let outsideDir: string; + +const httpsHandler = () => { + const handler = electron.handlers.get('https'); + if (!handler) { + throw new Error('https protocol handler was not registered'); + } + return handler; +}; + +const request = (requestPath: string) => httpsHandler()(new Request(`https://insomnia-app.local${requestPath}`)); + +const expectNotFound = async (requestPath: string) => { + const response = await request(requestPath); + expect(response.status).toBe(404); + expect(electron.netFetch).not.toHaveBeenCalled(); +}; + +const expectFetchedFile = async (requestPath: string, file: string) => { + await request(requestPath); + expect(electron.netFetch).toHaveBeenCalledTimes(1); + expect(electron.netFetch).toHaveBeenCalledWith(pathToFileURL(file).href, { bypassCustomProtocolHandlers: true }); +}; + +beforeAll(async () => { + vi.spyOn(path, 'resolve').mockImplementation((...segments: string[]) => { + if (segments[0] === mainSrcDir && segments[1] === 'client') { + return clientRoot; + } + return originalResolve(...segments); + }); + vi.spyOn(console, 'log').mockImplementation(() => {}); + await registerInsomniaProtocols(); +}); + +afterAll(() => { + vi.restoreAllMocks(); +}); + +beforeEach(() => { + base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'api-protocol-handler-'))); + clientRoot = path.join(base, 'client'); + assetsDir = path.join(clientRoot, 'assets'); + outsideDir = path.join(base, 'outside'); + fs.mkdirSync(assetsDir, { recursive: true }); + fs.mkdirSync(outsideDir, { recursive: true }); + fs.writeFileSync(path.join(clientRoot, 'index.html'), ''); + fs.writeFileSync(path.join(assetsDir, 'app.js'), 'console.log(1);'); + fs.writeFileSync(path.join(outsideDir, 'secret.txt'), 'top secret'); + electron.netFetch.mockClear(); +}); + +afterEach(() => { + fs.rmSync(base, { recursive: true, force: true }); +}); + +describe('insomnia-app.local handler', () => { + it('fetches a legitimate asset from rootDir/assets', async () => { + await expectFetchedFile('/assets/app.js', path.join(assetsDir, 'app.js')); + }); + + it('returns 404 without fetching for an encoded-slash traversal', async () => { + await expectNotFound('/assets/..%2f..%2foutside/secret.txt'); + }); + + it('returns 404 without fetching for an encoded-backslash traversal', async () => { + await expectNotFound('/assets/..%5c..%5coutside%5csecret.txt'); + }); + + it('returns 404 without fetching for a NUL byte in the path', async () => { + await expectNotFound('/assets/app.js%00.txt'); + }); + + it('never serves outside rootDir for a raw backslash in the path', async () => { + // The WHATWG URL parser rewrites raw backslashes to "/" for https and collapses the dot segments, + // so this becomes /outside/secret.txt, a non-asset path that falls back to index.html. + await expectFetchedFile('/assets/..\\..\\outside\\secret.txt', path.join(clientRoot, 'index.html')); + }); + + it('returns 404 without fetching for malformed percent-encoding', async () => { + await expectNotFound('/assets/%E0%A4%A'); + }); + + it('returns 404 without fetching when an in-tree symlink points outside rootDir', async () => { + fs.symlinkSync(path.join(outsideDir, 'secret.txt'), path.join(assetsDir, 'leak.js')); + await expectNotFound('/assets/leak.js'); + }); + + it('returns 404 without fetching when a symlinked directory points outside rootDir', async () => { + fs.symlinkSync(outsideDir, path.join(assetsDir, 'linkdir')); + await expectNotFound('/assets/linkdir/secret.txt'); + }); + + it('fetches an in-tree symlink whose target is also inside rootDir', async () => { + const alias = path.join(assetsDir, 'alias.js'); + fs.symlinkSync(path.join(assetsDir, 'app.js'), alias); + await expectFetchedFile('/assets/alias.js', alias); + }); + + it('falls through to fetch for a non-existent asset (realpath ENOENT is swallowed)', async () => { + const missing = path.join(assetsDir, 'missing.js'); + await expectFetchedFile('/assets/missing.js', missing); + }); + + it('serves index.html for non-asset paths', async () => { + await expectFetchedFile('/some/route', path.join(clientRoot, 'index.html')); + }); + + it('does not apply the asset containment logic to non-asset paths', async () => { + // Traversal segments are normalised by the URL parser before reaching the handler, so this lands on /secret. + await expectFetchedFile('/assets/../../outside/secret.txt', path.join(clientRoot, 'index.html')); + }); +}); + +describe('other origins', () => { + it('passes other https hosts straight to net.fetch with the original request', async () => { + const incoming = new Request('https://example.com/api?x=1'); + await httpsHandler()(incoming); + expect(electron.netFetch).toHaveBeenCalledWith(incoming, { bypassCustomProtocolHandlers: true }); + }); + + it('passes Google Fonts hosts straight to net.fetch by URL string', async () => { + await httpsHandler()(new Request('https://fonts.googleapis.com/css?family=Inter')); + expect(electron.netFetch).toHaveBeenCalledWith('https://fonts.googleapis.com/css?family=Inter', { + bypassCustomProtocolHandlers: true, + }); + }); + + it('does not treat a lookalike host as the app origin', async () => { + const incoming = new Request('https://insomnia-app.local.example.com/assets/app.js'); + await httpsHandler()(incoming); + expect(electron.netFetch).toHaveBeenCalledWith(incoming, { bypassCustomProtocolHandlers: true }); + }); +}); diff --git a/packages/insomnia/src/main/api.protocol.test.ts b/packages/insomnia/src/main/api.protocol.test.ts new file mode 100644 index 000000000000..372182c2f7a1 --- /dev/null +++ b/packages/insomnia/src/main/api.protocol.test.ts @@ -0,0 +1,70 @@ +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('electron', () => ({ app: {}, net: {}, protocol: {}, session: {} })); +vi.mock('@getinsomnia/node-libcurl', () => ({ Curl: vi.fn(), CurlAuth: {}, CurlFeature: {}, CurlSslOpt: {} })); +vi.mock('insomnia-data', () => ({ services: {} })); +vi.mock('insomnia-data/common', () => ({ ProxyScopes: {} })); +vi.mock('./network/libcurl-promise', () => ({})); +vi.mock('./templating-worker-database', () => ({})); + +import { resolveClientFilePath } from './api.protocol'; + +const rootDir = path.resolve('/app/resources/client'); + +// Mirrors what the real handler does with request.url, then what a file:// fetch does (decoding the file URL) +const effectiveFile = (requestPath: string) => { + const url = new URL(`https://insomnia-app.local${requestPath}`); + const resolved = resolveClientFilePath(rootDir, url.pathname); + return resolved === null ? null : fileURLToPath(pathToFileURL(resolved).href); +}; + +const isInside = (file: string) => { + const rel = path.relative(rootDir, file); + return !!rel && !rel.startsWith('..') && !path.isAbsolute(rel); +}; + +describe('resolveClientFilePath', () => { + it.each([ + '/assets/../../x', + '/assets/..%2f..%2fx', + '/assets/..%2F..%2Fx', + '/assets/%2e%2e/%2e%2e/x', + '/assets/%2e%2e%2f%2e%2e%2fx', + '/assets/%252e%252e%252f%252e%252e%252fx', + '/assets/..%5c..%5cx', + '/assets/..\\..\\x', + '/assets%2f..%2f..%2fx', + '/assets/x%00.js', + '/assets/%00', + '/assets//etc/passwd', + '/assets/%2fetc%2fpasswd', + '/assets/..', + '/assets/%E0%A4%A', + ])('never resolves outside rootDir for %s', requestPath => { + const file = effectiveFile(requestPath); + if (file !== null) { + expect(isInside(file)).toBe(true); + } + }); + + it('rejects encoded traversal', () => { + expect(effectiveFile('/assets/..%2f..%2fx')).toBeNull(); + expect(effectiveFile('/assets/..%5c..%5cx')).toBeNull(); + expect(effectiveFile('/assets/x%00.js')).toBeNull(); + }); + + it('serves legitimate assets, including encoded characters', () => { + expect(resolveClientFilePath(rootDir, '/assets/index-abc123.js')).toBe(path.join(rootDir, 'assets', 'index-abc123.js')); + expect(resolveClientFilePath(rootDir, '/assets/sub/font%20name.woff2')).toBe( + path.join(rootDir, 'assets', 'sub', 'font name.woff2'), + ); + }); + + it('serves index.html for non-asset paths', () => { + expect(resolveClientFilePath(rootDir, '/some/route')).toBe(path.join(rootDir, 'index.html')); + expect(resolveClientFilePath(rootDir, '/')).toBe(path.join(rootDir, 'index.html')); + }); +}); diff --git a/packages/insomnia/src/main/api.protocol.ts b/packages/insomnia/src/main/api.protocol.ts index f413a786e25b..5fa516a55419 100644 --- a/packages/insomnia/src/main/api.protocol.ts +++ b/packages/insomnia/src/main/api.protocol.ts @@ -1,6 +1,7 @@ +import fs from 'node:fs'; import path from 'node:path'; import { Readable } from 'node:stream'; -import { parse as urlParse } from 'node:url'; +import { parse as urlParse,pathToFileURL } from 'node:url'; import { Curl, CurlAuth, CurlFeature, CurlSslOpt, type HeaderInfo } from '@getinsomnia/node-libcurl'; import { app, net, protocol, session } from 'electron'; @@ -11,6 +12,33 @@ import { getApiBaseURL } from '../common/constants'; import { parseResolvedProxy, setDefaultProtocol, shouldBypassProxyForHost } from './network/libcurl-promise'; import { resolveDbByKey } from './templating-worker-database'; +/** + * Resolve the on-disk file served for a request path of the app's custom origin. + * The pathname is decoded here (the later file:// fetch would otherwise decode %2f / %5c + * after path.join has already run) and the result must stay inside rootDir. + * Returns null when the path is malformed or escapes rootDir. + */ +export function resolveClientFilePath(rootDir: string, pathname: string): string | null { + if (!pathname.startsWith('/assets')) { + return path.join(rootDir, 'index.html'); + } + let decoded: string; + try { + decoded = decodeURIComponent(pathname); + } catch { + return null; + } + if (decoded.includes('\0') || decoded.includes('\\')) { + return null; + } + const resolved = path.resolve(rootDir, `.${decoded}`); + const relative = path.relative(rootDir, resolved); + if (!relative || relative.startsWith('..') || path.isAbsolute(relative)) { + return null; + } + return resolved; +} + export interface RegisterProtocolOptions { scheme: string; } @@ -140,10 +168,24 @@ export async function registerInsomniaProtocols() { const url = new URL(request.url); if (url.hostname === 'insomnia-app.local') { const rootDir = path.resolve(__dirname, 'client'); - const filePath = path.join(rootDir, url.pathname.startsWith('/assets') ? url.pathname : 'index.html'); + const filePath = resolveClientFilePath(rootDir, url.pathname); + if (!filePath) { + return new Response('Not found', { status: 404 }); + } + // Symlinks must not let an in-tree path point outside rootDir + try { + const realRoot = fs.realpathSync(rootDir); + const realFile = fs.realpathSync(filePath); + const rel = path.relative(realRoot, realFile); + if (rel.startsWith('..') || path.isAbsolute(rel)) { + return new Response('Not found', { status: 404 }); + } + } catch { + // file does not exist: let the fetch below produce its normal error + } console.log(`Loading index for: ${url.pathname} from: ${filePath}`); - return await net.fetch(`file://${filePath}`, { bypassCustomProtocolHandlers: true }); + return await net.fetch(pathToFileURL(filePath).href, { bypassCustomProtocolHandlers: true }); } // Allow Google Fonts to bypass the custom https protocol handler. diff --git a/packages/insomnia/src/main/backup.ts b/packages/insomnia/src/main/backup.ts index 222cd352d0b7..056378a3622f 100644 --- a/packages/insomnia/src/main/backup.ts +++ b/packages/insomnia/src/main/backup.ts @@ -8,6 +8,7 @@ import { getUpdateUrl } from '~/main/updates'; import { version } from '../../package.json'; import { getClientString } from '../common/constants'; +import { isPathInsideDir } from './ipc/path-guard'; export async function backupIfNewerVersionAvailable() { try { @@ -59,10 +60,28 @@ export async function backup() { } } +// `version` comes from the renderer; it must name a single directory directly under `backups/`. +const resolveBackupVersionPath = (dataPath: string, version: string): string => { + const backupsDir = path.join(dataPath, 'backups'); + const versionPath = path.resolve(backupsDir, version); + if ( + typeof version !== 'string' || + !version || + version === '.' || + version === '..' || + /[/\\\0]/.test(version) || + path.dirname(versionPath) !== path.resolve(backupsDir) || + !isPathInsideDir(versionPath, backupsDir) + ) { + throw new Error('restoreBackup: invalid backup version'); + } + return versionPath; +}; + export async function restoreBackup(version: string) { + const dataPath = process.env['INSOMNIA_DATA_PATH'] || electron.app.getPath('userData'); + const versionPath = resolveBackupVersionPath(dataPath, version); try { - const dataPath = process.env['INSOMNIA_DATA_PATH'] || electron.app.getPath('userData'); - const versionPath = path.join(dataPath, 'backups', version); const files = await readdir(versionPath); if (!files.length) { console.log('[main] No backup found at:', versionPath); diff --git a/packages/insomnia/src/main/cloud-sync/pull-backend-project-doc-validation.test.ts b/packages/insomnia/src/main/cloud-sync/pull-backend-project-doc-validation.test.ts new file mode 100644 index 000000000000..8c85033b10d3 --- /dev/null +++ b/packages/insomnia/src/main/cloud-sync/pull-backend-project-doc-validation.test.ts @@ -0,0 +1,76 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { updateMock, upsertMock, bufferChangesMock, flushChangesMock } = vi.hoisted(() => ({ + updateMock: vi.fn(async (doc: any) => doc), + upsertMock: vi.fn(async (doc: any) => doc), + bufferChangesMock: vi.fn(async () => 1), + flushChangesMock: vi.fn(async () => {}), +})); + +vi.mock('insomnia-data', async () => ({ + ...(await vi.importActual>('insomnia-data')), + database: { update: updateMock, bufferChanges: bufferChangesMock, flushChanges: flushChangesMock }, + services: { workspace: { upsert: upsertMock } }, +})); +vi.mock('~/sync/access-error', () => ({ interceptAccessError: ({ callback }: any) => callback() })); + +import { pullBackendProject } from './pull-backend-project'; + +const makeVcs = (docs: any[], branches = ['master']) => + ({ + setBackendProject: vi.fn(async () => {}), + checkout: vi.fn(async () => {}), + getRemoteBranchNames: vi.fn(async () => branches), + pull: vi.fn(async () => {}), + allDocuments: vi.fn(async () => docs), + }) as any; + +const backendProject = { id: 'bp_1', name: 'bp', rootDocumentId: 'wrk_1' } as any; +const remoteProject = { _id: 'proj_1', parentId: 'org_1' } as any; +const now = Date.now(); +const doc = (type: string, _id: string, parentId = 'wrk_1') => ({ type, _id, parentId, created: now, modified: now }); + +describe('pullBackendProject doc validation', () => { + beforeEach(() => vi.clearAllMocks()); + + it('writes docs with well-formed ids', async () => { + const docs = [ + doc('Workspace', 'wrk_0123456789abcdef0123456789abcdef', 'proj_x'), + doc('Request', 'req_0123456789abcdef0123456789abcdef'), + doc('Environment', '__BASE_ENVIRONMENT_ID__'), + doc('ProtoFile', 'pf_abc-DEF_123'), + ]; + await pullBackendProject({ vcs: makeVcs(docs), backendProject, remoteProject }); + expect(updateMock).toHaveBeenCalledTimes(4); + }); + + it.each([ + ['_id with traversal', doc('ProtoFile', 'pf_../../../x')], + ['_id with backslash', doc('Request', 'req_..\\..\\x')], + ['_id with NUL', doc('Request', 'req_a\0b')], + ['_id absolute', doc('Request', '/etc/passwd')], + ['_id empty', doc('Request', '')], + ['parentId with traversal', doc('Request', 'req_ok', '../../x')], + ])('rejects a doc with %s before any database write', async (_name, bad) => { + const docs = [doc('Request', 'req_good'), bad]; + await expect(pullBackendProject({ vcs: makeVcs(docs), backendProject, remoteProject })).rejects.toThrow(/Refusing/); + expect(updateMock).not.toHaveBeenCalled(); + }); + + it('rejects a non-string type', async () => { + const docs = [{ ...doc('Request', 'req_ok'), type: { a: 1 } }]; + await expect(pullBackendProject({ vcs: makeVcs(docs), backendProject, remoteProject })).rejects.toThrow(/Refusing/); + expect(updateMock).not.toHaveBeenCalled(); + }); + + it('rejects an unsafe backend project root document id when creating the workspace', async () => { + await expect( + pullBackendProject({ + vcs: makeVcs([], []), + backendProject: { ...backendProject, rootDocumentId: 'wrk_../../x' }, + remoteProject, + }), + ).rejects.toThrow(/Refusing/); + expect(upsertMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/insomnia/src/main/cloud-sync/pull-backend-project.ts b/packages/insomnia/src/main/cloud-sync/pull-backend-project.ts index 576cf31af6e4..334cbd71ba47 100644 --- a/packages/insomnia/src/main/cloud-sync/pull-backend-project.ts +++ b/packages/insomnia/src/main/cloud-sync/pull-backend-project.ts @@ -1,10 +1,11 @@ -import type { RemoteProject } from 'insomnia-data'; +import type { BaseModel, RemoteProject } from 'insomnia-data'; import { database, models, services } from 'insomnia-data'; import type { BackendProjectWithTeam, VCS } from 'insomnia-vcs'; import { interceptAccessError } from '~/sync/access-error'; import { DEFAULT_BRANCH_NAME } from '../../common/constants'; +import { assertSafeSyncedDoc } from '../../common/synced-doc-validation'; interface Options { vcs: VCS; @@ -27,6 +28,11 @@ export const pullBackendProject = async ({ vcs, backendProject, remoteProject }: // @TODO Revisit the UX for this. What should happen if there are other branches? // The default branch does not exist, so we create it and the workspace locally if (defaultBranchMissing) { + assertSafeSyncedDoc({ + _id: backendProject.rootDocumentId, + type: models.workspace.type, + parentId: remoteProject._id, + }); const workspace = await services.workspace.upsert({ ...models.workspace.init(), _id: backendProject.rootDocumentId, @@ -49,10 +55,17 @@ export const pullBackendProject = async ({ vcs, backendProject, remoteProject }: projectId: remoteProject._id, }); // There won't be any existing docs since it's a new pull + // @ts-expect-error -- TSCONVERSION + const pulledDocs: BaseModel[] = (await vcs.allDocuments()) || []; + // Remote docs are teammate-controlled and their ids later feed filesystem paths. Validate every doc + // up front so a malformed one is rejected before anything is written (no partial apply). + for (const doc of pulledDocs) { + assertSafeSyncedDoc(doc); + } + const flushId = await database.bufferChanges(); let workspaceId; - // @ts-expect-error -- TSCONVERSION - for (const doc of (await vcs.allDocuments()) || []) { + for (const doc of pulledDocs) { // When we pull a BackendProject we need to update the parent ID of the workspace so that it appears inside. // There can't be more than one workspace. if (models.workspace.isWorkspace(doc)) { diff --git a/packages/insomnia/src/main/git-service.ts b/packages/insomnia/src/main/git-service.ts index ef1b3e5be6fc..93e4c36fbab5 100644 --- a/packages/insomnia/src/main/git-service.ts +++ b/packages/insomnia/src/main/git-service.ts @@ -33,6 +33,7 @@ import type { AutoResolvedConflict, MergeConflict } from 'insomnia-vcs'; import { Errors, type PromiseFsClient } from 'isomorphic-git'; import YAML, { parse } from 'yaml'; +import { assertSafeSyncedDoc } from '~/common/synced-doc-validation'; import { invariant } from '~/common/utils/invariant'; import { GitVCSOperationErrors } from '~/sync/git/git-vcs-operation-errors'; import { @@ -51,6 +52,7 @@ import { insomniaSchemaTypeToScope } from '../common/insomnia-v5'; import { checkStrictSchemaMatch } from '../common/zod/check-schema-mismatch'; import { findUnknownFieldRanges, type UnknownFieldRange } from '../common/zod/find-unknown-field-ranges'; import { isUndiscardableChange } from '../sync/git/canonical-file'; +import { assertRealPathContained } from '../sync/git/contained-path'; import { fsClient } from '../sync/git/fs-client'; import { CURRENT_MIGRATION_VERSION, migrateRepoStructureIfNeeded } from '../sync/git/git-repo-migration'; import GitVCS, { @@ -1193,7 +1195,9 @@ export async function importLegacyInsomniaFolder({ return {}; } - // Process each legacy file and migrate it to the database + // Phase 1: read and validate every file before anything is written, so a hostile or broken + // file cannot leave the database partially migrated. + const legacyDocs: { filePath: string; doc: BaseModel }[] = []; for (const legacyInsomniaFile of legacyInsomniaFiles) { const fileContents = await fsClient.promises.readFile(legacyInsomniaFile.filePath, 'utf8'); @@ -1209,6 +1213,9 @@ export async function importLegacyInsomniaFolder({ // Parse the YAML file to get the document const doc: BaseModel = YAML.parse(fileContents); + // The repo is untrusted: _id/parentId end up in on-disk names (e.g. the gitFilePath below) + assertSafeSyncedDoc(doc); + // Validate that the document ID matches the file path if (!legacyInsomniaFile.filePath.includes(doc._id)) { throw new Error(`Doc _id does not match file path [${doc._id} - ${legacyInsomniaFile.filePath}]`); @@ -1228,6 +1235,11 @@ export async function importLegacyInsomniaFolder({ continue; } + legacyDocs.push({ filePath: legacyInsomniaFile.filePath, doc }); + } + + // Phase 2: write + for (const { filePath, doc } of legacyDocs) { // Special handling for workspaces: ensure they're associated with the correct project if (models.workspace.isWorkspace(doc)) { console.log('[git] setting workspace parent to be that of the active project', { @@ -1257,7 +1269,7 @@ export async function importLegacyInsomniaFolder({ // Track the removal of the legacy file changes.push({ - path: legacyInsomniaFile.filePath, + path: filePath, // It existed and was removed from the git repository status: [1, 0, 1], // Deleted from working directory }); @@ -1527,120 +1539,125 @@ export const cloneGitRepoAction = async ({ } const bufferId = await database.bufferChanges(); - let gitRepository = await services.gitRepository.create(repoSettingsPatch); + try { + let gitRepository = await services.gitRepository.create(repoSettingsPatch); - async function getProject() { - if (cloneIntoProjectId) { - const project = await services.project.getById(cloneIntoProjectId); - invariant(project, 'Project not found'); + async function getProject() { + if (cloneIntoProjectId) { + const project = await services.project.getById(cloneIntoProjectId); + invariant(project, 'Project not found'); + + await services.project.update(project, { + remoteId: null, + gitRepositoryId: models.project.toProtectedRepoId(gitRepository._id), + }); + + return project; + } - await services.project.update(project, { - remoteId: null, + const project = await services.project.create({ + name: name || gitRepository.uri.split('/').pop() || 'New Git Project', + parentId: organizationId, gitRepositoryId: models.project.toProtectedRepoId(gitRepository._id), }); return project; } - const project = await services.project.create({ - name: name || gitRepository.uri.split('/').pop() || 'New Git Project', - parentId: organizationId, - gitRepositoryId: models.project.toProtectedRepoId(gitRepository._id), - }); - - return project; - } + const project = await getProject(); - const project = await getProject(); - - // Give the app-managed folder a readable name derived from the project's - // name up front, instead of leaving it as the bare-id `git_` folder - // until the next app-startup backfill pass (see `backfillManagedFolderSlug`). - // Irrelevant when the user picked a `directory` — that folder name is theirs. - if (!gitRepository.directory) { - const slug = slugify(project.name); - if (slug) { - gitRepository = await services.gitRepository.update(gitRepository, { folderSlug: slug }); + // Give the app-managed folder a readable name derived from the project's + // name up front, instead of leaving it as the bare-id `git_` folder + // until the next app-startup backfill pass (see `backfillManagedFolderSlug`). + // Irrelevant when the user picked a `directory` — that folder name is theirs. + if (!gitRepository.directory) { + const slug = slugify(project.name); + if (slug) { + gitRepository = await services.gitRepository.update(gitRepository, { folderSlug: slug }); + } } - } - const fsClient = await getGitFSClient({ - projectId: project._id, - gitRepositoryId: gitRepository._id, - directory: gitRepository.directory, - folderSlug: gitRepository.folderSlug, - }); - - const repoBaseDir = await getRepoBaseDir(gitRepository._id, gitRepository.directory, gitRepository.folderSlug); - - if (gitRepository.needsFullClone) { - await GitVCS.initFromClone({ - repoId: gitRepository._id, - url: uri, - credentialsId: gitRepository.credentialsId, - directory: GIT_CLONE_DIR, - fs: fsClient, - gitDirectory: GIT_INTERNAL_DIR, - ref, - repoPath: repoBaseDir, + const fsClient = await getGitFSClient({ + projectId: project._id, + gitRepositoryId: gitRepository._id, + directory: gitRepository.directory, + folderSlug: gitRepository.folderSlug, }); - await services.gitRepository.update(gitRepository, { - needsFullClone: false, - }); - } else { - await GitVCS.init({ - repoId: gitRepository._id, - uri, - directory: GIT_CLONE_DIR, - fs: fsClient, - gitDirectory: GIT_INTERNAL_DIR, - credentialsId: gitRepository.credentialsId, - repoPath: repoBaseDir, - }); - } + const repoBaseDir = await getRepoBaseDir(gitRepository._id, gitRepository.directory, gitRepository.folderSlug); + + if (gitRepository.needsFullClone) { + await GitVCS.initFromClone({ + repoId: gitRepository._id, + url: uri, + credentialsId: gitRepository.credentialsId, + directory: GIT_CLONE_DIR, + fs: fsClient, + gitDirectory: GIT_INTERNAL_DIR, + ref, + repoPath: repoBaseDir, + }); - await GitVCS.setAuthor(); - await GitVCS.addRemote(uri); + await services.gitRepository.update(gitRepository, { + needsFullClone: false, + }); + } else { + await GitVCS.init({ + repoId: gitRepository._id, + uri, + directory: GIT_CLONE_DIR, + fs: fsClient, + gitDirectory: GIT_INTERNAL_DIR, + credentialsId: gitRepository.credentialsId, + repoPath: repoBaseDir, + }); + } - const hasLegacyInsomniaDir = await containsLegacyInsomniaDir({ fsClient: inMemoryFsClient }); - if (hasLegacyInsomniaDir) { - await migrateLegacyInsomniaFolderToFile({ projectId: project._id }); - } + await GitVCS.setAuthor(); + await GitVCS.addRemote(uri); - // Start watcher — it automatically imports all YAML files during creation - const cloneBaseDir = repoBaseDir; + const hasLegacyInsomniaDir = await containsLegacyInsomniaDir({ fsClient: inMemoryFsClient }); + if (hasLegacyInsomniaDir) { + await migrateLegacyInsomniaFolderToFile({ projectId: project._id }); + } - // If the project already has a ruleset in the DB (e.g. cloud → git migration), - // write it to disk now so its mtime is newer than the cloned file. This ensures - // the cloud ruleset is preserved and it shows up as a diff in the commit modal rather than being silently replaced by the repo's file. - const existingRuleset = await services.projectLintRuleset.getByParentId(project._id); - if (existingRuleset) { - const rulesetPath = path.join(cloneBaseDir, '.spectral.yaml'); - await fs.promises.writeFile(rulesetPath, existingRuleset.rulesetContent, 'utf8'); - } + // Start watcher — it automatically imports all YAML files during creation + const cloneBaseDir = repoBaseDir; + + // If the project already has a ruleset in the DB (e.g. cloud → git migration), + // write it to disk now so its mtime is newer than the cloned file. This ensures + // the cloud ruleset is preserved and it shows up as a diff in the commit modal rather than being silently replaced by the repo's file. + const existingRuleset = await services.projectLintRuleset.getByParentId(project._id); + if (existingRuleset) { + const rulesetPath = path.join(cloneBaseDir, '.spectral.yaml'); + // The cloned repo is untrusted: do not write through a symlink that leaves the clone folder + await assertRealPathContained(cloneBaseDir, rulesetPath, true); + await fs.promises.writeFile(rulesetPath, existingRuleset.rulesetContent, 'utf8'); + } - await repoFileWatcherRegistry.startWatcher(gitRepository._id, cloneBaseDir, project._id); + await repoFileWatcherRegistry.startWatcher(gitRepository._id, cloneBaseDir, project._id); - const updateRepository = await services.gitRepository.getById(gitRepository._id); - invariant(updateRepository, 'Git Repository not found'); + const updateRepository = await services.gitRepository.getById(gitRepository._id); + invariant(updateRepository, 'Git Repository not found'); - await services.gitRepository.update(updateRepository, { - cachedGitLastCommitTime: Date.now(), - cachedGitRepositoryBranch: await GitVCS.getCurrentBranch(), - }); + await services.gitRepository.update(updateRepository, { + cachedGitLastCommitTime: Date.now(), + cachedGitRepositoryBranch: await GitVCS.getCurrentBranch(), + }); - await database.flushChanges(bufferId); - trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { - ...vcsEventProperties('git', 'clone'), - providerName, - repoId: repoSettingsPatch._id, - }); + trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { + ...vcsEventProperties('git', 'clone'), + providerName, + repoId: repoSettingsPatch._id, + }); - return { - organizationId, - projectId: project._id, - }; + return { + organizationId, + projectId: project._id, + }; + } finally { + await database.flushChanges(bufferId); + } } const project = await services.project.getById(projectId); @@ -1703,141 +1720,146 @@ export const cloneGitRepoAction = async ({ const bufferId = await database.bufferChanges(); let workspaceId = ''; let scope: 'design' | 'collection' = models.workspace.WorkspaceScopeKeys.design; - // If no workspace exists we create a new one - if (!(await containsInsomniaWorkspaceDir(inMemoryFsClient))) { - // Create a new workspace - - const workspace = await services.workspace.create({ - name: repoSettingsPatch.uri?.split('/').pop(), - scope: scope, - parentId: project._id, - description: `Insomnia Workspace for ${repoSettingsPatch.uri}}`, - }); - await services.apiSpec.getOrCreateForParentId(workspace._id); - - trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { - ...vcsEventProperties('git', 'clone', 'no directory found'), - providerName: provider, - repoId: repoSettingsPatch._id, - }); - - workspaceId = workspace._id; - - const newRepo = await services.gitRepository.create(repoSettingsPatch); - const meta = await services.workspaceMeta.getOrCreateByParentId(workspaceId); - await services.workspaceMeta.update(meta, { - gitRepositoryId: newRepo._id, - }); - } else { - // Clone all entities from the repository - const workspaceBase = path.join(GIT_INSOMNIA_DIR, models.workspace.type); - const workspaces = await inMemoryFsClient.promises.readdir(workspaceBase); + try { + // If no workspace exists we create a new one + if (!(await containsInsomniaWorkspaceDir(inMemoryFsClient))) { + // Create a new workspace + + const workspace = await services.workspace.create({ + name: repoSettingsPatch.uri?.split('/').pop(), + scope: scope, + parentId: project._id, + description: `Insomnia Workspace for ${repoSettingsPatch.uri}}`, + }); + await services.apiSpec.getOrCreateForParentId(workspace._id); - if (workspaces.length === 0) { trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { - ...vcsEventProperties('git', 'clone', 'no workspaces found'), + ...vcsEventProperties('git', 'clone', 'no directory found'), providerName: provider, repoId: repoSettingsPatch._id, }); - return { - errors: ['No workspaces found in repository'], - }; - } + workspaceId = workspace._id; - if (workspaces.length > 1) { - trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { - ...vcsEventProperties('git', 'clone', 'multiple workspaces found'), - providerName: provider, - repoId: repoSettingsPatch._id, + const newRepo = await services.gitRepository.create(repoSettingsPatch); + const meta = await services.workspaceMeta.getOrCreateByParentId(workspaceId); + await services.workspaceMeta.update(meta, { + gitRepositoryId: newRepo._id, }); + } else { + // Clone all entities from the repository + const workspaceBase = path.join(GIT_INSOMNIA_DIR, models.workspace.type); + const workspaces = await inMemoryFsClient.promises.readdir(workspaceBase); + + if (workspaces.length === 0) { + trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { + ...vcsEventProperties('git', 'clone', 'no workspaces found'), + providerName: provider, + repoId: repoSettingsPatch._id, + }); - return { - errors: ['Multiple workspaces found in repository. Expected one.'], - }; - } + return { + errors: ['No workspaces found in repository'], + }; + } + + if (workspaces.length > 1) { + trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { + ...vcsEventProperties('git', 'clone', 'multiple workspaces found'), + providerName: provider, + repoId: repoSettingsPatch._id, + }); - // Only one workspace - const workspacePath = path.join(workspaceBase, workspaces[0]); - const workspaceJson = await inMemoryFsClient.promises.readFile(workspacePath); - const workspace = YAML.parse(workspaceJson.toString()); - workspaceId = workspace._id; - scope = - workspace.scope === models.workspace.WorkspaceScopeKeys.collection - ? models.workspace.WorkspaceScopeKeys.collection - : models.workspace.WorkspaceScopeKeys.design; - // Check if the workspace already exists - const existingWorkspace = await services.workspace.getById(workspace._id); - - if (existingWorkspace) { - const project = await services.project.getById(existingWorkspace.parentId); - if (!project) { return { - errors: [ - 'It seems that the repository being cloned is connected to an orphaned workspace. Please move that workspace to a project and try again.', - ], + errors: ['Multiple workspaces found in repository. Expected one.'], }; } - const organizationId = project?.parentId; + // Only one workspace + const workspacePath = path.join(workspaceBase, workspaces[0]); + const workspaceJson = await inMemoryFsClient.promises.readFile(workspacePath); + const workspace = YAML.parse(workspaceJson.toString()); + // The repo is untrusted: refuse an unsafe _id before it is used to create any metadata + assertSafeSyncedDoc({ ...workspace, type: models.workspace.type, parentId: null }); + workspaceId = workspace._id; + scope = + workspace.scope === models.workspace.WorkspaceScopeKeys.collection + ? models.workspace.WorkspaceScopeKeys.collection + : models.workspace.WorkspaceScopeKeys.design; + // Check if the workspace already exists + const existingWorkspace = await services.workspace.getById(workspace._id); + + if (existingWorkspace) { + const project = await services.project.getById(existingWorkspace.parentId); + if (!project) { + return { + errors: [ + 'It seems that the repository being cloned is connected to an orphaned workspace. Please move that workspace to a project and try again.', + ], + }; + } - return { - existingWorkspace: { - organizationId, - workspaceId: existingWorkspace._id, - projectId: project._id, - }, - }; - } + const organizationId = project?.parentId; - // Store GitRepository settings and set it as active - const gitRepository = await services.gitRepository.create(repoSettingsPatch); - const meta = await services.workspaceMeta.getOrCreateByParentId(workspaceId); - await services.workspaceMeta.update(meta, { - gitRepositoryId: gitRepository._id, - }); + return { + existingWorkspace: { + organizationId, + workspaceId: existingWorkspace._id, + projectId: project._id, + }, + }; + } - const routableFS = await getGitFSClient({ - projectId, - workspaceId, - gitRepositoryId: gitRepository._id, - }); - const wsRepoBaseDir = await getRepoBaseDir(gitRepository._id); - - // Configure basic info - if (gitRepository.needsFullClone) { - await GitVCS.initFromClone({ - repoId: gitRepository._id, - url: uri, - credentialsId: gitRepository.credentialsId, - directory: GIT_CLONE_DIR, - fs: routableFS, - gitDirectory: GIT_INTERNAL_DIR, - repoPath: wsRepoBaseDir, + // Store GitRepository settings and set it as active + const gitRepository = await services.gitRepository.create(repoSettingsPatch); + const meta = await services.workspaceMeta.getOrCreateByParentId(workspaceId); + await services.workspaceMeta.update(meta, { + gitRepositoryId: gitRepository._id, }); - await services.gitRepository.update(gitRepository, { - needsFullClone: false, - }); - } else { - await GitVCS.init({ - repoId: gitRepository._id, - uri, - directory: GIT_CLONE_DIR, - fs: routableFS, - gitDirectory: GIT_INTERNAL_DIR, - credentialsId: gitRepository.credentialsId, - legacyDiff: true, - repoPath: wsRepoBaseDir, + const routableFS = await getGitFSClient({ + projectId, + workspaceId, + gitRepositoryId: gitRepository._id, }); + const wsRepoBaseDir = await getRepoBaseDir(gitRepository._id); + + // Configure basic info + if (gitRepository.needsFullClone) { + await GitVCS.initFromClone({ + repoId: gitRepository._id, + url: uri, + credentialsId: gitRepository.credentialsId, + directory: GIT_CLONE_DIR, + fs: routableFS, + gitDirectory: GIT_INTERNAL_DIR, + repoPath: wsRepoBaseDir, + }); + + await services.gitRepository.update(gitRepository, { + needsFullClone: false, + }); + } else { + await GitVCS.init({ + repoId: gitRepository._id, + uri, + directory: GIT_CLONE_DIR, + fs: routableFS, + gitDirectory: GIT_INTERNAL_DIR, + credentialsId: gitRepository.credentialsId, + legacyDiff: true, + repoPath: wsRepoBaseDir, + }); + } + + await GitVCS.setAuthor(); + await GitVCS.addRemote(uri); } - await GitVCS.setAuthor(); - await GitVCS.addRemote(uri); + } finally { + // Flush DB changes, also when exiting early + await database.flushChanges(bufferId); } - - // Flush DB changes - await database.flushChanges(bufferId); trackAnalyticsEvent(AnalyticsEvent.vcsSyncComplete, { ...vcsEventProperties('git', 'clone'), providerName: provider, diff --git a/packages/insomnia/src/network/grpc/__tests__/write-proto-file-synced-id-containment.test.ts b/packages/insomnia/src/network/grpc/__tests__/write-proto-file-synced-id-containment.test.ts new file mode 100644 index 000000000000..ffc719367306 --- /dev/null +++ b/packages/insomnia/src/network/grpc/__tests__/write-proto-file-synced-id-containment.test.ts @@ -0,0 +1,76 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { services } from 'insomnia-data'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { writeProtoFile } from '../write-proto-file.node'; + +// ProtoFile/ProtoDirectory docs are cloud-synced, so a teammate (or anyone able to push to the +// project) controls their `_id` and `modified`. Those values are interpolated into on-disk names. +describe('writeProtoFile with attacker-controlled synced ids', () => { + let sandbox: string; + let tmpRoot: string; + + beforeEach(() => { + sandbox = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'proto-id-containment-'))); + tmpRoot = path.join(sandbox, 'tmp'); + fs.mkdirSync(tmpRoot); + vi.spyOn(os, 'tmpdir').mockReturnValue(tmpRoot); + }); + + afterEach(() => { + vi.restoreAllMocks(); + fs.rmSync(sandbox, { recursive: true, force: true }); + }); + + const listAll = (dir: string): string[] => + fs.readdirSync(dir, { recursive: true }).map(String); + + it('does not write a single proto file outside insomnia-grpc when _id contains ../', async () => { + const w = await services.workspace.create(); + const pf = await services.protoFile.create({ + _id: 'pf_../../../escaped', + parentId: w._id, + protoText: 'syntax = "proto3";', + }); + + await expect(writeProtoFile(pf)).rejects.toThrow(); + + expect(listAll(sandbox).filter(f => f.endsWith('.proto'))).toEqual([]); + }); + + it('does not write outside insomnia-grpc when modified contains ../', async () => { + const w = await services.workspace.create(); + const pf = await services.protoFile.create({ parentId: w._id, protoText: 'x' }); + + await expect(writeProtoFile({ ...pf, modified: '/../../escaped' as unknown as number })).rejects.toThrow(); + + expect(listAll(sandbox).filter(f => f.endsWith('.proto'))).toEqual([]); + }); + + it('does not create directories outside insomnia-grpc when root directory _id contains ../', async () => { + const w = await services.workspace.create(); + const pd = await services.protoDirectory.create({ + _id: 'pd_../../../escaped-dir', + parentId: w._id, + name: 'dir', + }); + const pf = await services.protoFile.create({ parentId: pd._id, name: 'a.proto', protoText: 'x' }); + + await expect(writeProtoFile(pf)).rejects.toThrow(); + + expect(fs.existsSync(path.join(tmpRoot, 'escaped-dir.' + pd.modified))).toBe(false); + expect(listAll(sandbox).filter(f => f.includes('escaped'))).toEqual([]); + }); + + it('still writes a normally-id\'d proto file', async () => { + const w = await services.workspace.create(); + const pf = await services.protoFile.create({ parentId: w._id, protoText: 'syntax = "proto3";' }); + + const result = await writeProtoFile(pf); + + expect(fs.readFileSync(path.join(tmpRoot, 'insomnia-grpc', result.filePath), 'utf8')).toBe(pf.protoText); + }); +}); diff --git a/packages/insomnia/src/network/grpc/write-proto-file.node.ts b/packages/insomnia/src/network/grpc/write-proto-file.node.ts index c1d8aa660479..f7e6d3e1a0f6 100644 --- a/packages/insomnia/src/network/grpc/write-proto-file.node.ts +++ b/packages/insomnia/src/network/grpc/write-proto-file.node.ts @@ -27,6 +27,17 @@ const assertWithinTempRoot = (tempRoot: string, candidatePath: string): void => } }; +// `_id` and `modified` come from synced docs and are interpolated into on-disk names, so they must +// be a single safe path segment (no separators, no `..`). +const SAFE_ID_PATTERN = /^[A-Za-z0-9_-]+$/; +const SAFE_MODIFIED_PATTERN = /^[0-9]+$/; +const toVersionedSegment = (doc: Pick): string => { + if (!SAFE_ID_PATTERN.test(String(doc._id)) || !SAFE_MODIFIED_PATTERN.test(String(doc.modified))) { + throw new Error(`Unsafe proto document id or modified timestamp: "${doc._id}.${doc.modified}"`); + } + return `${doc._id}.${doc.modified}`; +}; + const recursiveWriteProtoDirectory = async ( dir: ProtoDirectory, descendants: BaseModel[], @@ -91,7 +102,7 @@ export const writeProtoFile = async (protoFile: ProtoFile): Promise const tempRoot = path.join( os.tmpdir(), 'insomnia-grpc', - `${rootAncestorProtoDirectory._id}.${rootAncestorProtoDirectory.modified}`, + toVersionedSegment(rootAncestorProtoDirectory), ); const treeRootDirs = await recursiveWriteProtoDirectory( rootAncestorProtoDirectory, @@ -115,13 +126,14 @@ export const writeProtoFile = async (protoFile: ProtoFile): Promise const rootDir = path.join(os.tmpdir(), 'insomnia-grpc'); fs.mkdirSync(rootDir, { recursive: true }); - const filePath = `${protoFile._id}.${protoFile.modified}.proto`; + const filePath = `${toVersionedSegment(protoFile)}.proto`; const result = { filePath, dirs: [rootDir], }; // Check if file already exists const fullPath = path.join(rootDir, filePath); + assertWithinTempRoot(rootDir, fullPath); if (fs.existsSync(fullPath)) { return result; } diff --git a/packages/insomnia/src/sync/git/__tests__/fs-client-path-containment.test.ts b/packages/insomnia/src/sync/git/__tests__/fs-client-path-containment.test.ts new file mode 100644 index 000000000000..925b29b705b4 --- /dev/null +++ b/packages/insomnia/src/sync/git/__tests__/fs-client-path-containment.test.ts @@ -0,0 +1,96 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import git from 'isomorphic-git'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { fsClient } from '../fs-client'; + +describe('fsClient path containment', () => { + let root: string; + let base: string; + let outside: string; + + beforeEach(async () => { + root = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'insomnia-fsclient-')); + base = path.join(root, 'base'); + outside = path.join(root, 'outside'); + await fs.promises.mkdir(outside); + }); + + afterEach(async () => { + await fs.promises.rm(root, { recursive: true, force: true }); + }); + + const escapingPaths = ['../escape.txt', './../escape.txt', 'a/../../escape.txt', '../outside/escape.txt', 'sub/../../outside/escape.txt']; + + it.each(escapingPaths)('rejects writeFile to %s and creates nothing outside base', async p => { + const client = fsClient(base); + await expect(client.promises.writeFile(p, 'pwn')).rejects.toThrow(); + expect(fs.existsSync(path.join(root, 'escape.txt'))).toBe(false); + expect(fs.existsSync(path.join(outside, 'escape.txt'))).toBe(false); + }); + + it.each(['../newdir', './../newdir'])('rejects mkdir of %s', async p => { + const client = fsClient(base); + await expect(client.promises.mkdir(p)).rejects.toThrow(); + expect(fs.existsSync(path.join(root, 'newdir'))).toBe(false); + }); + + it('rejects unlink, rmdir, readFile, readdir, stat, lstat and readlink outside base', async () => { + await fs.promises.writeFile(path.join(root, 'secret.txt'), 's'); + const client = fsClient(base); + await expect(client.promises.unlink('../secret.txt')).rejects.toThrow(); + expect(fs.existsSync(path.join(root, 'secret.txt'))).toBe(true); + await expect(client.promises.rmdir('../outside')).rejects.toThrow(); + expect(fs.existsSync(outside)).toBe(true); + await expect(client.promises.readFile('../secret.txt')).rejects.toThrow(); + await expect(client.promises.readdir('..')).rejects.toThrow(); + await expect(client.promises.stat('../secret.txt')).rejects.toThrow(); + await expect(client.promises.lstat('../secret.txt')).rejects.toThrow(); + await expect(client.promises.readlink('../secret.txt')).rejects.toThrow(); + }); + + it('still allows normal nested paths, dot paths and inner dot-dot segments', async () => { + const client = fsClient(base); + await client.promises.mkdir('a'); + await client.promises.writeFile('./a/f.txt', 'ok'); + await client.promises.writeFile('a/../g.txt', 'ok2'); + await client.promises.writeFile('/h.txt', 'ok3'); + expect(await client.promises.readFile('a/f.txt', 'utf8')).toBe('ok'); + expect(fs.readFileSync(path.join(base, 'g.txt'), 'utf8')).toBe('ok2'); + expect(fs.readFileSync(path.join(base, 'h.txt'), 'utf8')).toBe('ok3'); + expect(await client.promises.readdir('.')).toContain('a'); + }); + + // End to end: isomorphic-git does not validate tree entry names, so a tree + // entry named `..` produces `./../x` write paths on checkout. + it('isomorphic-git checkout of a tree containing a ".." entry does not write outside base', async () => { + const client = fsClient(base); + const dir = '.'; + await git.init({ fs: client, dir, defaultBranch: 'main' }); + const blob = await git.writeBlob({ fs: client, dir, blob: Buffer.from('pwned') }); + const inner = await git.writeTree({ fs: client, dir, tree: [{ mode: '100644', path: 'pwn.txt', oid: blob, type: 'blob' }] }); + const rootTree = await git.writeTree({ fs: client, dir, tree: [{ mode: '040000', path: '..', oid: inner, type: 'tree' }] }); + const commit = await git.writeCommit({ + fs: client, + dir, + commit: { + message: 'evil', + tree: rootTree, + parent: [], + author: { name: 'a', email: 'a@a', timestamp: 1, timezoneOffset: 0 }, + committer: { name: 'a', email: 'a@a', timestamp: 1, timezoneOffset: 0 }, + }, + }); + await git.writeRef({ fs: client, dir, ref: 'refs/heads/main', value: commit, force: true }); + try { + await git.checkout({ fs: client, dir, ref: 'main', force: true }); + } catch { + // checkout is expected to fail once the escaping entry is refused + } + expect(fs.existsSync(path.join(root, 'pwn.txt'))).toBe(false); + expect(fs.existsSync(path.join(base, '..', 'pwn.txt'))).toBe(false); + }); +}); diff --git a/packages/insomnia/src/sync/git/__tests__/fs-client-symlink-handling.test.ts b/packages/insomnia/src/sync/git/__tests__/fs-client-symlink-handling.test.ts new file mode 100644 index 000000000000..2aff491b26c8 --- /dev/null +++ b/packages/insomnia/src/sync/git/__tests__/fs-client-symlink-handling.test.ts @@ -0,0 +1,131 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import git from 'isomorphic-git'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { fsClient } from '../fs-client'; + +describe('fsClient symlink handling', () => { + let root: string; + let base: string; + let outside: string; + + beforeEach(async () => { + root = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'insomnia-fsclient-link-')); + base = path.join(root, 'base'); + outside = path.join(root, 'outside'); + await fs.promises.mkdir(outside); + }); + + afterEach(async () => { + await fs.promises.rm(root, { recursive: true, force: true }); + }); + + // isomorphic-git calls symlink(target, filepath) with the stored blob text as target. + it('creates the link at filepath with the target text stored verbatim', async () => { + const client = fsClient(base); + await client.promises.symlink('../shared/x', 'link'); + expect(fs.readlinkSync(path.join(base, 'link'))).toBe('../shared/x'); + expect(await client.promises.readlink('link')).toBe('../shared/x'); + await client.promises.symlink('inner/file.txt', 'rel'); + expect(fs.readlinkSync(path.join(base, 'rel'))).toBe('inner/file.txt'); + }); + + it('does not create a link location outside base', async () => { + const client = fsClient(base); + await expect(client.promises.symlink('anything', '../evil-link')).rejects.toThrow(); + expect(fs.existsSync(path.join(root, 'evil-link'))).toBe(false); + expect(fs.lstatSync(base).isDirectory()).toBe(true); + }); + + it('refuses writes, mkdir and reads through a symlinked directory that points outside base', async () => { + const client = fsClient(base); + await client.promises.symlink(outside, 'esc'); + await expect(client.promises.writeFile('esc/pwn.txt', 'x')).rejects.toThrow(); + await expect(client.promises.mkdir('esc/sub')).rejects.toThrow(); + expect(fs.readdirSync(outside)).toEqual([]); + fs.writeFileSync(path.join(outside, 'secret.txt'), 's'); + await expect(client.promises.readFile('esc/secret.txt')).rejects.toThrow(); + await expect(client.promises.unlink('esc/secret.txt')).rejects.toThrow(); + expect(fs.existsSync(path.join(outside, 'secret.txt'))).toBe(true); + }); + + it('refuses writing through a symlinked leaf, including a dangling one, pointing outside base', async () => { + const client = fsClient(base); + fs.writeFileSync(path.join(outside, 'target.txt'), 'orig'); + await client.promises.symlink(path.join(outside, 'target.txt'), 'leaf'); + await expect(client.promises.writeFile('leaf', 'pwn')).rejects.toThrow(); + expect(fs.readFileSync(path.join(outside, 'target.txt'), 'utf8')).toBe('orig'); + await client.promises.symlink(path.join(outside, 'new.txt'), 'dangling'); + await expect(client.promises.writeFile('dangling', 'pwn')).rejects.toThrow(); + expect(fs.existsSync(path.join(outside, 'new.txt'))).toBe(false); + }); + + it('allows an outward-pointing link to be created, inspected and removed without following it', async () => { + const client = fsClient(base); + await client.promises.symlink(outside, 'esc'); + expect((await client.promises.lstat('esc')).isSymbolicLink()).toBe(true); + expect(await client.promises.readlink('esc')).toBe(outside); + await client.promises.unlink('esc'); + expect(fs.existsSync(path.join(base, 'esc'))).toBe(false); + expect(fs.existsSync(outside)).toBe(true); + }); + + it('allows writes through symlinks that stay inside base', async () => { + const client = fsClient(base); + await client.promises.mkdir('real'); + await client.promises.symlink('real', 'alias'); + await client.promises.writeFile('alias/ok.txt', 'ok'); + expect(fs.readFileSync(path.join(base, 'real', 'ok.txt'), 'utf8')).toBe('ok'); + }); + + it('works when basePath itself is reached through a symlink', async () => { + fs.mkdirSync(base); + const linkedBase = path.join(root, 'linked-base'); + fs.symlinkSync(base, linkedBase); + const client = fsClient(linkedBase); + await client.promises.writeFile('ok.txt', 'ok'); + expect(fs.readFileSync(path.join(base, 'ok.txt'), 'utf8')).toBe('ok'); + }); + + it('isomorphic-git checkout of a symlink entry followed by a file beneath it does not write outside base', async () => { + const client = fsClient(base); + const dir = '.'; + await git.init({ fs: client, dir, defaultBranch: 'main' }); + const linkBlob = await git.writeBlob({ fs: client, dir, blob: Buffer.from(outside) }); + const fileBlob = await git.writeBlob({ fs: client, dir, blob: Buffer.from('pwned') }); + const inner = await git.writeTree({ fs: client, dir, tree: [{ mode: '100644', path: 'pwn.txt', oid: fileBlob, type: 'blob' }] }); + // Git tree names are unique, so use a second directory entry that is processed after the link. + const rootTree = await git.writeTree({ + fs: client, + dir, + tree: [ + { mode: '120000', path: 'esc', oid: linkBlob, type: 'blob' }, + { mode: '100644', path: 'esc2', oid: fileBlob, type: 'blob' }, + { mode: '040000', path: 'esc-dir', oid: inner, type: 'tree' }, + ], + }); + const commit = await git.writeCommit({ + fs: client, + dir, + commit: { + message: 'evil', + tree: rootTree, + parent: [], + author: { name: 'a', email: 'a@a', timestamp: 1, timezoneOffset: 0 }, + committer: { name: 'a', email: 'a@a', timestamp: 1, timezoneOffset: 0 }, + }, + }); + await git.writeRef({ fs: client, dir, ref: 'refs/heads/main', value: commit, force: true }); + try { + await git.checkout({ fs: client, dir, ref: 'main', force: true }); + } catch { + // checkout is expected to fail once the escaping entry is refused + } + expect(fs.readdirSync(outside)).toEqual([]); + // the link itself must have been written verbatim in base + expect(fs.readlinkSync(path.join(base, 'esc'))).toBe(outside); + }); +}); diff --git a/packages/insomnia/src/sync/git/__tests__/ne-db-client-doc-validation.test.ts b/packages/insomnia/src/sync/git/__tests__/ne-db-client-doc-validation.test.ts new file mode 100644 index 000000000000..3fa7cbaa96c5 --- /dev/null +++ b/packages/insomnia/src/sync/git/__tests__/ne-db-client-doc-validation.test.ts @@ -0,0 +1,37 @@ +import path from 'node:path'; + +import { models } from 'insomnia-data'; +import { beforeEach, describe, expect, it } from 'vitest'; +import YAML from 'yaml'; + +import { database as db } from '../../../common/database'; +import { GIT_INSOMNIA_DIR } from '../git-vcs'; +import { NeDBClient } from '../ne-db-client'; + +describe('NeDBClient.writeFile() doc validation', () => { + beforeEach(async () => { + await db.init({ inMemoryOnly: true }, true); + }); + + const write = (fileId: string, doc: Record) => + new NeDBClient('wrk_test', 'proj_test').writeFile( + path.join(GIT_INSOMNIA_DIR, models.request.type, `${fileId}.yml`), + YAML.stringify({ type: models.request.type, name: 'r', url: 'http://example.com', ...doc }), + ); + + it('writes a doc with well-formed ids', async () => { + await write('req_ok', { _id: 'req_ok', parentId: 'wrk_test' }); + expect(await db.findOne(models.request.type, { _id: 'req_ok' })).toBeTruthy(); + }); + + it('refuses a doc whose _id matches its file name but is not a safe segment', async () => { + // a backslash is a legal file name character on POSIX and a separator on Windows + await expect(write('req_..\\..\\x', { _id: 'req_..\\..\\x', parentId: 'wrk_test' })).rejects.toThrow(/unsafe _id/); + expect(await db.findOne(models.request.type, { _id: 'req_..\\..\\x' })).toBeFalsy(); + }); + + it('refuses a doc with an unsafe parentId', async () => { + await expect(write('req_evil', { _id: 'req_evil', parentId: '../../x' })).rejects.toThrow(/unsafe parentId/); + expect(await db.findOne(models.request.type, { _id: 'req_evil' })).toBeFalsy(); + }); +}); diff --git a/packages/insomnia/src/sync/git/__tests__/repo-file-watcher-symlink-write-containment.test.ts b/packages/insomnia/src/sync/git/__tests__/repo-file-watcher-symlink-write-containment.test.ts new file mode 100644 index 000000000000..5bcb32156dd5 --- /dev/null +++ b/packages/insomnia/src/sync/git/__tests__/repo-file-watcher-symlink-write-containment.test.ts @@ -0,0 +1,93 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { services } from 'insomnia-data'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { database as db } from '../../../common/database'; +import { RepoFileWatcherRegistry } from '../repo-file-watcher'; + +vi.mock('../../../common/insomnia-v5', () => ({ + getInsomniaV5DataExport: vi.fn().mockResolvedValue('type: collection.insomnia.rest/5.0\nname: Exported\n'), + tryImportV5Data: vi.fn().mockReturnValue({ data: undefined, error: undefined }), +})); + +const REPO_ID = 'git_repo_test'; +const PROJECT_ID = 'proj_test'; +const VICTIM_CONTENT = 'do not overwrite\n'; + +// A cloned/pulled repo is attacker controlled and may contain symlinks. The watcher's DB -> disk flush +// must never write through one that leaves the repo, even though the lexical path stays inside it. +describe('RepoFileWatcher disk writes through symlinks', () => { + let repoDir: string; + let outsideDir: string; + let registry: RepoFileWatcherRegistry; + + beforeEach(async () => { + await db.init({ inMemoryOnly: true }, true); + const root = await fs.promises.realpath(await fs.promises.mkdtemp(path.join(os.tmpdir(), 'insomnia-watcher-symlink-'))); + repoDir = path.join(root, 'repo'); + outsideDir = path.join(root, 'outside'); + await fs.promises.mkdir(repoDir); + await fs.promises.mkdir(outsideDir); + registry = new RepoFileWatcherRegistry({ onDbSynced: vi.fn(), onProblemsChanged: vi.fn() }); + }); + + afterEach(async () => { + registry.stopAll(); + await fs.promises.rm(path.dirname(repoDir), { recursive: true, force: true }); + vi.clearAllMocks(); + }); + + const createWorkspaceWithMeta = async (gitFilePath: string) => { + const workspace = await services.workspace.create({ name: 'WS', scope: 'collection', parentId: PROJECT_ID }); + const meta = await services.workspaceMeta.getOrCreateByParentId(workspace._id); + await services.workspaceMeta.update(meta, { gitFilePath, gitFileLastSyncTime: null }); + }; + + it('does not write a workspace file through a symlinked file that points outside the repo', async () => { + const victim = path.join(outsideDir, 'victim.txt'); + await fs.promises.writeFile(victim, VICTIM_CONTENT); + await fs.promises.symlink(victim, path.join(repoDir, 'insomnia.wrk.yaml')); + await createWorkspaceWithMeta('insomnia.wrk.yaml'); + + await registry.startWatcher(REPO_ID, repoDir, PROJECT_ID); + await registry.flushNow(REPO_ID); + + expect(await fs.promises.readFile(victim, 'utf8')).toBe(VICTIM_CONTENT); + }); + + it('does not write a workspace file through a symlinked folder that points outside the repo', async () => { + await fs.promises.symlink(outsideDir, path.join(repoDir, 'sub')); + await createWorkspaceWithMeta('sub/ws.yaml'); + + await registry.startWatcher(REPO_ID, repoDir, PROJECT_ID); + await registry.flushNow(REPO_ID); + + expect(fs.existsSync(path.join(outsideDir, 'ws.yaml'))).toBe(false); + }); + + it('does not write the project lint ruleset through a symlinked .spectral.yaml that points outside the repo', async () => { + const victim = path.join(outsideDir, 'victim.txt'); + await fs.promises.writeFile(victim, VICTIM_CONTENT); + await fs.promises.symlink(victim, path.join(repoDir, '.spectral.yaml')); + await services.projectLintRuleset.upsert(PROJECT_ID, { rulesetContent: 'extends: []\nrules: {}\n' }); + + await registry.startWatcher(REPO_ID, repoDir, PROJECT_ID); + await registry.flushNow(REPO_ID); + + expect(await fs.promises.readFile(victim, 'utf8')).toBe(VICTIM_CONTENT); + }); + + it('still writes a workspace file through a symlinked folder that stays inside the repo', async () => { + await fs.promises.mkdir(path.join(repoDir, 'real')); + await fs.promises.symlink(path.join(repoDir, 'real'), path.join(repoDir, 'sub')); + await createWorkspaceWithMeta('sub/ws.yaml'); + + await registry.startWatcher(REPO_ID, repoDir, PROJECT_ID); + await registry.flushNow(REPO_ID); + + expect(fs.existsSync(path.join(repoDir, 'real', 'ws.yaml'))).toBe(true); + }); +}); diff --git a/packages/insomnia/src/sync/git/__tests__/repo-file-watcher-synced-doc-validation.test.ts b/packages/insomnia/src/sync/git/__tests__/repo-file-watcher-synced-doc-validation.test.ts new file mode 100644 index 000000000000..efb064658a99 --- /dev/null +++ b/packages/insomnia/src/sync/git/__tests__/repo-file-watcher-synced-doc-validation.test.ts @@ -0,0 +1,71 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { services } from 'insomnia-data'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { database as db } from '../../../common/database'; +import { getInsomniaV5DataExport } from '../../../common/insomnia-v5'; +import { RepoFileWatcherRegistry } from '../repo-file-watcher'; + +const REPO_ID = 'git_repo_test'; +const PROJECT_ID = 'proj_test'; + +// Builds a real exported collection (so the fixture always matches the current schema), then swaps in the given ids. +const collectionYaml = async ({ workspaceId, requestId, folderId }: { workspaceId: string; requestId: string; folderId: string }) => { + const workspace = await services.workspace.create({ name: 'Imported', scope: 'collection', parentId: PROJECT_ID }); + await services.environment.getOrCreateForParentId(workspace._id); + const folder = await services.requestGroup.create({ name: 'Folder', parentId: workspace._id }); + const request = await services.request.create({ name: 'Request', url: 'http://example.com', parentId: folder._id }); + const exported = (await getInsomniaV5DataExport({ workspaceId: workspace._id, includePrivateEnvironments: false })) as string; + await db.init({ inMemoryOnly: true }, true); + return exported + .replaceAll(workspace._id, workspaceId) + .replaceAll(folder._id, folderId) + .replaceAll(request._id, requestId); +}; + +describe('RepoFileWatcher synced doc id validation', () => { + let repoDir: string; + let registry: RepoFileWatcherRegistry; + + beforeEach(async () => { + await db.init({ inMemoryOnly: true }, true); + repoDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'insomnia-repo-watcher-validation-')); + registry = new RepoFileWatcherRegistry({ onDbSynced: vi.fn(), onProblemsChanged: vi.fn() }); + }); + + afterEach(async () => { + registry.stopAll(); + await fs.promises.rm(repoDir, { recursive: true, force: true }); + vi.clearAllMocks(); + }); + + const importYaml = async (yaml: string) => { + await fs.promises.writeFile(path.join(repoDir, 'insomnia.collection.yaml'), yaml, 'utf8'); + await registry.startWatcher(REPO_ID, repoDir, PROJECT_ID); + }; + + it('imports a file with well-formed ids', async () => { + await importYaml(await collectionYaml({ workspaceId: 'wrk_abc123', requestId: 'req_abc123', folderId: 'fld_abc123' })); + + expect(await services.workspace.getById('wrk_abc123')).toBeTruthy(); + expect(await db.findOne('Request', { _id: 'req_abc123' })).toBeTruthy(); + expect(registry.getProblems(REPO_ID)).toEqual([]); + }); + + it.each([ + ['workspace id', { workspaceId: 'wrk_../../../x', requestId: 'req_ok', folderId: 'fld_ok' }], + ['request id', { workspaceId: 'wrk_ok', requestId: 'req_../../../x', folderId: 'fld_ok' }], + ['folder id, which becomes the request parentId', { workspaceId: 'wrk_ok', requestId: 'req_ok', folderId: 'fld_../../x' }], + ])('refuses the whole file and reports a problem for an unsafe %s', async (_label, ids) => { + await importYaml(await collectionYaml(ids)); + + expect(await services.workspace.getById('wrk_ok')).toBeFalsy(); + expect(await db.findOne('Request', { _id: 'req_ok' })).toBeFalsy(); + expect(await db.findOne('Request', { _id: 'req_../../../x' })).toBeFalsy(); + expect(await services.workspaceMeta.getByParentId('wrk_../../../x')).toBeFalsy(); + expect(registry.getProblems(REPO_ID)).toMatchObject([{ kind: 'parse-error', relPath: 'insomnia.collection.yaml' }]); + }); +}); diff --git a/packages/insomnia/src/sync/git/__tests__/routable-fs-client-symlink-target.test.ts b/packages/insomnia/src/sync/git/__tests__/routable-fs-client-symlink-target.test.ts new file mode 100644 index 000000000000..b14e1f900f50 --- /dev/null +++ b/packages/insomnia/src/sync/git/__tests__/routable-fs-client-symlink-target.test.ts @@ -0,0 +1,55 @@ +import path from 'node:path'; + +import type { PromiseFsClient } from 'isomorphic-git'; +import { describe, expect, it, vi } from 'vitest'; + +import { projectRoutableFSClient } from '../project-routable-fs-client'; +import { routableFSClient } from '../routable-fs-client'; + +const makeSpyClient = () => { + const promises: any = {}; + const calls = { + symlink: vi.fn(async (..._args: any[]) => {}), + other: vi.fn(async (..._args: any[]) => {}), + }; + promises.symlink = calls.symlink; + promises.readFile = calls.other; + promises.writeFile = calls.other; + promises.mkdir = calls.other; + return { client: { promises } as PromiseFsClient, calls }; +}; + +const factories = { + routableFSClient, + projectRoutableFSClient, +}; + +describe.each(Object.entries(factories))('%s symlink routing', (_name, factory) => { + const targets = ['.git/x', '.insomnia/x', '../shared/x', './a/../b', 'a//b/']; + + it.each(targets)('passes symlink target %s verbatim to the client owning the link location', async target => { + const def = makeSpyClient(); + const git = makeSpyClient(); + const ins = makeSpyClient(); + const routable = factory(def.client, { '.git': git.client, '.insomnia': ins.client }).promises; + + await routable.symlink!(target, 'docs/link'); + + expect(def.calls.symlink).toHaveBeenCalledTimes(1); + expect(def.calls.symlink.mock.calls[0][0]).toBe(target); + expect(def.calls.symlink.mock.calls[0][1]).toBe(path.normalize('docs/link')); + expect(git.calls.symlink).not.toHaveBeenCalled(); + expect(ins.calls.symlink).not.toHaveBeenCalled(); + }); + + it('routes by the link location, not the target', async () => { + const def = makeSpyClient(); + const git = makeSpyClient(); + const routable = factory(def.client, { '.git': git.client }).promises; + + await routable.symlink!('../shared/x', './.git/link'); + + expect(def.calls.symlink).not.toHaveBeenCalled(); + expect(git.calls.symlink).toHaveBeenCalledWith('../shared/x', path.normalize('.git/link')); + }); +}); diff --git a/packages/insomnia/src/sync/git/__tests__/routable-fs-client-traversal-routing.test.ts b/packages/insomnia/src/sync/git/__tests__/routable-fs-client-traversal-routing.test.ts new file mode 100644 index 000000000000..f95382a153aa --- /dev/null +++ b/packages/insomnia/src/sync/git/__tests__/routable-fs-client-traversal-routing.test.ts @@ -0,0 +1,75 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import type { PromiseFsClient } from 'isomorphic-git'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { fsClient } from '../fs-client'; +import { projectRoutableFSClient } from '../project-routable-fs-client'; +import { routableFSClient } from '../routable-fs-client'; + +const makeSpyClient = () => { + const promises: any = {}; + const calls = { + symlink: vi.fn(async (..._args: any[]) => {}), + other: vi.fn(async (..._args: any[]) => {}), + }; + promises.symlink = calls.symlink; + promises.readFile = calls.other; + promises.writeFile = calls.other; + promises.mkdir = calls.other; + return { client: { promises } as PromiseFsClient, calls }; +}; + +const factories = { + routableFSClient, + projectRoutableFSClient, +}; + +describe.each(Object.entries(factories))('%s traversal in routing decisions', (_name, factory) => { + let root: string; + let base: string; + + beforeEach(async () => { + root = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'insomnia-routable-')); + base = path.join(root, 'base'); + }); + + afterEach(async () => { + await fs.promises.rm(root, { recursive: true, force: true }); + }); + + const smuggling = [ + '.insomnia/../../escape.txt', + './.git/../../escape.txt', + '.git/../../escape.txt', + '.git/a/../../../escape.txt', + '/.git/../../escape.txt', + ]; + + it.each(smuggling)('cannot write outside the base dir via %s', async p => { + // Mirrors main/git-service.ts: each routed prefix maps to a contained fsClient. + const routable = factory(fsClient(path.join(base, 'other')), { + '.git': fsClient(base), + '.insomnia': fsClient(path.join(base, 'ins')), + }).promises; + + await routable.writeFile(p, 'pwn').catch(() => {}); + + expect(fs.existsSync(path.join(root, 'escape.txt'))).toBe(false); + expect(fs.existsSync(path.join(base, 'escape.txt'))).toBe(false); + expect(fs.existsSync(path.join(base, '..', 'escape.txt'))).toBe(false); + }); + + it('normalizes before prefix matching so a traversal path is not handed to the prefix client', async () => { + const def = makeSpyClient(); + const ins = makeSpyClient(); + const routable = factory(def.client, { '.insomnia': ins.client }).promises; + + await routable.writeFile('.insomnia/../../x', 'a'); + + expect(ins.calls.other).not.toHaveBeenCalled(); + expect(def.calls.other).toHaveBeenCalledWith(path.normalize('../x'), 'a'); + }); +}); diff --git a/packages/insomnia/src/sync/git/contained-path.ts b/packages/insomnia/src/sync/git/contained-path.ts new file mode 100644 index 000000000000..3b223f50c8bd --- /dev/null +++ b/packages/insomnia/src/sync/git/contained-path.ts @@ -0,0 +1,65 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +import { isPathInsideDir } from '../../main/ipc/path-guard'; + +const escapeError = (filePath: string) => + Object.assign(new Error(`Path escapes the git working directory: ${filePath}`), { code: 'EACCES' }); + +/** + * Resolves a path handed to the isomorphic-git fs client against `basePath`, + * throwing if it would land outside it. isomorphic-git builds paths from git + * tree entry names without validating them, so `..` segments can arrive here. + * Lexical only; see {@link assertRealPathContained} for symlinks. + */ +export const resolveContainedPath = (basePath: string, filePath: string): string => { + const resolved = path.join(basePath, path.normalize(filePath)); + if (!isPathInsideDir(resolved, basePath)) { + throw escapeError(filePath); + } + return resolved; +}; + +const MAX_LINK_HOPS = 40; + +/** realpath() that tolerates a non-existent tail: resolves the deepest existing ancestor and re-appends the rest. Dangling symlinks are followed, as a write through them would create their target. */ +const realpathOfDeepestExisting = async (p: string, hops = 0): Promise => { + if (hops > MAX_LINK_HOPS) { + throw Object.assign(new Error('Too many symbolic links'), { code: 'ELOOP' }); + } + try { + return await fs.promises.realpath(p); + } catch (err: any) { + if (err?.code !== 'ENOENT' && err?.code !== 'ENOTDIR') { + throw err; + } + } + try { + const stat = await fs.promises.lstat(p); + if (stat.isSymbolicLink()) { + const target = await fs.promises.readlink(p); + return realpathOfDeepestExisting(path.resolve(path.dirname(p), target), hops + 1); + } + } catch { + // does not exist at all, fall through to the parent + } + const parent = path.dirname(p); + if (parent === p) { + return p; + } + return path.join(await realpathOfDeepestExisting(parent, hops), path.basename(p)); +}; + +/** + * Ensures `resolvedPath` (already lexically contained) does not reach outside `basePath` through + * symlinks. With `followLeaf` false the final component is not followed (lstat/readlink/unlink/the + * location of a link being created), only its ancestors are. + */ +export const assertRealPathContained = async (basePath: string, resolvedPath: string, followLeaf: boolean) => { + const realBase = await fs.promises.realpath(basePath); + const candidate = followLeaf || resolvedPath === path.resolve(basePath) ? resolvedPath : path.dirname(resolvedPath); + const realCandidate = await realpathOfDeepestExisting(candidate); + if (!isPathInsideDir(realCandidate, realBase)) { + throw escapeError(resolvedPath); + } +}; diff --git a/packages/insomnia/src/sync/git/fs-client.ts b/packages/insomnia/src/sync/git/fs-client.ts index 0010681755f3..415faa2322e9 100644 --- a/packages/insomnia/src/sync/git/fs-client.ts +++ b/packages/insomnia/src/sync/git/fs-client.ts @@ -1,5 +1,6 @@ import fs from 'node:fs'; -import path from 'node:path'; + +import { assertRealPathContained, resolveContainedPath } from './contained-path'; type FSWraps = | typeof fs.promises.readFile @@ -18,35 +19,40 @@ export const fsClient = (basePath: string) => { console.log(`[fsClient] Created in ${basePath}`); fs.mkdirSync(basePath, { recursive: true }); + // `followLeaf: false` is for operations that act on a symlink itself rather than what it points to. const wrap = - (fn: FSWraps) => + (fn: FSWraps, followLeaf = true) => async (filePath: string, ...args: any[]) => { - const modifiedPath = path.join(basePath, path.normalize(filePath)); + const modifiedPath = resolveContainedPath(basePath, filePath); + await assertRealPathContained(basePath, modifiedPath, followLeaf); // @ts-expect-error -- TSCONVERSION return fn(modifiedPath, ...args); }; + // isomorphic-git calls `symlink(target, filepath)`. The target is the stored link text and must be + // kept verbatim (it may legitimately be relative or point outside the repo); only the location of + // the link is confined. Writes through a link that resolves outside basePath are refused by `wrap`. const wrapSymlink = (fn: typeof fs.promises.symlink) => - async (filePath: string, target: string, ...args: any[]) => { - const modifiedPath = path.join(basePath, path.normalize(filePath)); - const modifiedTarget = path.join(basePath, path.normalize(target)); + async (target: string, filePath: string, ...args: any[]) => { + const linkPath = resolveContainedPath(basePath, filePath); + await assertRealPathContained(basePath, linkPath, false); - return fn(modifiedPath, modifiedTarget, ...args); + return fn(target, linkPath, ...args); }; return { promises: { readFile: wrap(fs.promises.readFile), writeFile: wrap(fs.promises.writeFile), - unlink: wrap(fs.promises.unlink), + unlink: wrap(fs.promises.unlink, false), readdir: wrap(fs.promises.readdir), mkdir: wrap(fs.promises.mkdir), - rmdir: wrap(fs.promises.rmdir), + rmdir: wrap(fs.promises.rmdir, false), stat: wrap(fs.promises.stat), - lstat: wrap(fs.promises.lstat), - readlink: wrap(fs.promises.readlink), + lstat: wrap(fs.promises.lstat, false), + readlink: wrap(fs.promises.readlink, false), symlink: wrapSymlink(fs.promises.symlink), }, }; diff --git a/packages/insomnia/src/sync/git/ne-db-client.ts b/packages/insomnia/src/sync/git/ne-db-client.ts index 60ad67694b37..06762d968994 100644 --- a/packages/insomnia/src/sync/git/ne-db-client.ts +++ b/packages/insomnia/src/sync/git/ne-db-client.ts @@ -21,6 +21,7 @@ import type { PromiseFsClient } from 'isomorphic-git'; import YAML from 'yaml'; import { database as db } from '../../common/database'; +import { assertSafeSyncedDoc } from '../../common/synced-doc-validation'; import { GIT_INSOMNIA_DIR_NAME } from './git-vcs'; import parseGitPath from './parse-git-path'; import Stat from './stat'; @@ -151,6 +152,9 @@ export class NeDBClient { return; } + // The repo is untrusted: _id/parentId end up in on-disk names later, so they must be a single safe segment + assertSafeSyncedDoc(doc); + // Special handling for workspaces: ensure they stay in the correct project if (models.workspace.isWorkspace(doc)) { console.log('[git] setting workspace parent to be that of the active project', { diff --git a/packages/insomnia/src/sync/git/project-routable-fs-client.ts b/packages/insomnia/src/sync/git/project-routable-fs-client.ts index 7e55f4bb8794..b19648f9c901 100644 --- a/packages/insomnia/src/sync/git/project-routable-fs-client.ts +++ b/packages/insomnia/src/sync/git/project-routable-fs-client.ts @@ -32,6 +32,19 @@ export function projectRoutableFSClient(defaultFS: git.PromiseFsClient, otherFS: let writeFileMap: WriteFileMap | null = null; const execMethod = async (method: Methods, filePath: string, ...args: any[]) => { + // isomorphic-git calls symlink(target, filepath): the first argument is the stored link text, + // not a path. Keep it verbatim and route on the link location instead. + if (method === 'symlink') { + const [linkLocation, ...rest] = args; + const location = path.normalize(linkLocation); + for (const prefix of Object.keys(otherFS)) { + if (location.indexOf(path.normalize(prefix)) === 0) { + return otherFS[prefix].promises.symlink!(filePath, location, ...rest); + } + } + return defaultFS.promises.symlink!(filePath, location, ...rest); + } + filePath = path.normalize(filePath); // 1) Prefix routing: forward into any registered special FS (e.g. '.git') diff --git a/packages/insomnia/src/sync/git/repo-file-watcher.ts b/packages/insomnia/src/sync/git/repo-file-watcher.ts index 2dc6a611614e..534fa047c371 100644 --- a/packages/insomnia/src/sync/git/repo-file-watcher.ts +++ b/packages/insomnia/src/sync/git/repo-file-watcher.ts @@ -50,6 +50,8 @@ import { database as db } from '../../common/database'; import { InsomniaFileTypeValues } from '../../common/import-v5-parser'; import { getInsomniaV5DataExport, tryImportV5Data } from '../../common/insomnia-v5'; import { validateSpectralRuleset } from '../../common/spectral-ruleset-validator'; +import { assertSafeSyncedDoc } from '../../common/synced-doc-validation'; +import { assertRealPathContained } from './contained-path'; import { SyncQueue } from './sync-queue'; const POLL_INTERVAL_MS = 10_000; @@ -280,6 +282,8 @@ class RepoFileWatcher { // Path-traversal guard if (isPathOutsideRepo(this.repoDir, absPath)) return; + // Symlink guard: the lexical check above does not see links inside the repo that point out of it + if (!(await this.isRealPathInsideRepo(absPath))) return; // Get the most recently modified DB document in this workspace\u2019s tree const allDocs = await db.getWithDescendants(workspace); @@ -512,6 +516,11 @@ class RepoFileWatcher { continue; } + if (!(await this.isRealPathInsideRepo(absPath))) { + console.warn('[repo-file-watcher] Refusing to write workspace file through a symlink leaving the repo:', gitFilePath); + continue; + } + if (this.hasProblem(absPath)) { continue; } @@ -604,6 +613,11 @@ class RepoFileWatcher { return; } + if (!(await this.isRealPathInsideRepo(absPath))) { + console.warn('[repo-file-watcher] Refusing to write the lint ruleset through a symlink leaving the repo'); + return; + } + await fs.promises.writeFile(absPath, ruleset.rulesetContent, 'utf8'); this.lastWrittenHash.set(absPath, hash); const stat = await fs.promises.stat(absPath); @@ -863,6 +877,20 @@ class RepoFileWatcher { return null; } + // The repo is untrusted and the imported ids are preserved: they later end up in on-disk names. + // Validate every doc before anything is deleted or written so a hostile file cannot partially apply. + try { + docs.forEach(assertSafeSyncedDoc); + } catch (err) { + this.addProblem(normalised, { + filePath: absPath, + relPath: this.toPosixRelPath(absPath), + kind: 'parse-error', + message: err instanceof Error ? err.message : String(err), + }); + return null; + } + this.clearProblem(normalised); return docs; } @@ -978,7 +1006,21 @@ class RepoFileWatcher { this.clearProblem(normalisedPath); } + /** Whether `absPath` still resolves inside the repo once symlinks are followed (a leaf symlink is followed too). */ + private async isRealPathInsideRepo(absPath: string, followLeaf = true): Promise { + try { + await assertRealPathContained(this.repoDir, absPath, followLeaf); + return true; + } catch { + return false; + } + } + private async removeWorkspaceFileFromDisk(workspaceId: string, normalisedPath: string): Promise { + if (!(await this.isRealPathInsideRepo(normalisedPath, false))) { + console.warn('[repo-file-watcher] Refusing to remove a file through a symlink leaving the repo:', normalisedPath); + return; + } try { await fs.promises.unlink(normalisedPath); console.log('[repo-file-watcher] Removed workspace file from disk:', workspaceId, normalisedPath); @@ -1131,6 +1173,9 @@ class RepoFileWatcher { if (rel.startsWith('..') || path.isAbsolute(rel)) { return; } + if (!(await this.isRealPathInsideRepo(absPath))) { + return; + } try { const yamlContent = await getInsomniaV5DataExport({ diff --git a/packages/insomnia/src/sync/git/routable-fs-client.ts b/packages/insomnia/src/sync/git/routable-fs-client.ts index 808e227aa8ee..9edcf703f591 100644 --- a/packages/insomnia/src/sync/git/routable-fs-client.ts +++ b/packages/insomnia/src/sync/git/routable-fs-client.ts @@ -23,6 +23,19 @@ type Methods = */ export function routableFSClient(defaultFS: git.PromiseFsClient, otherFS: Record) { const execMethod = async (method: Methods, filePath: string, ...args: any[]) => { + // isomorphic-git calls symlink(target, filepath): the first argument is the stored link text, + // not a path. Keep it verbatim and route on the link location instead. + if (method === 'symlink') { + const [linkLocation, ...rest] = args; + const location = path.normalize(linkLocation); + for (const prefix of Object.keys(otherFS)) { + if (location.indexOf(path.normalize(prefix)) === 0) { + return otherFS[prefix].promises.symlink!(filePath, location, ...rest); + } + } + return defaultFS.promises.symlink!(filePath, location, ...rest); + } + filePath = path.normalize(filePath); // Route by path prefix: if a prefix matches (e.g. '.insomnia' or '.git')