chore(sync): add id and path constraints for synced data

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<string, string> = {
+  '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();
+    });
+  });
+});

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<string, string> = {
+  '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();
+    });
+  });
+});

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<string, string> = {
+  '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<string, string> = {
+  '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();
+      });
+    });
+  });
+});

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<BackendProject | null> {
+  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<BackendProject[]> {
 
   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<BackendProject[]> {
 }
 
 export async function getBranches(backendProjectId: string): Promise<Branch[]> {
+  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!) {

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');
+  }
+}