fix(import): address coderabbit comments

Author: gopu-bruno

packages/bruno-app/src/components/ShareCollection/index.js

@@ -65,10 +65,6 @@ const ShareCollection = ({ onClose, collectionUid }) => {
 
   const handlePostmanModalClose = () => {
     setShowPostmanExportModal(false);
-  };
-
-  const handlePostmanExported = () => {
-    setShowPostmanExportModal(false);
     onClose();
   };
 
@@ -100,6 +96,16 @@ const ShareCollection = ({ onClose, collectionUid }) => {
 
   const isDisabled = isCollectionLoading || isExporting;
 
+  if (showPostmanExportModal) {
+    return (
+      <ExportToPostman
+        collection={collection}
+        onClose={handlePostmanModalClose}
+        onExported={handlePostmanModalClose}
+      />
+    );
+  }
+
   return (
     <>
       <Modal size="lg" title="Share Collection" handleCancel={onClose} hideFooter>
@@ -207,14 +213,6 @@ const ShareCollection = ({ onClose, collectionUid }) => {
           </div>
         </StyledWrapper>
       </Modal>
-
-      {showPostmanExportModal && (
-        <ExportToPostman
-          collection={collection}
-          onClose={handlePostmanModalClose}
-          onExported={handlePostmanExported}
-        />
-      )}
     </>
   );
 };

packages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.js

@@ -5,7 +5,7 @@ import * as Yup from 'yup';
 import { cloneDeep } from 'lodash';
 import { IconCaretDown } from '@tabler/icons';
 import toast from 'react-hot-toast';
-import { sanitizeName } from 'utils/common/regex';
+import { sanitizeName, validateName, validateNameError } from 'utils/common/regex';
 import Portal from 'components/Portal';
 import Modal from 'components/Modal';
 import Dropdown from 'components/Dropdown';
@@ -20,6 +20,7 @@ const ExportToPostman = ({ onClose, onExported, collection }) => {
   const inputRef = useRef();
   const [preserveScripts, setPreserveScripts] = useState(false);
   const [showAdvancedOptions, setShowAdvancedOptions] = useState(false);
+  const [isExporting, setIsExporting] = useState(false);
   const optionsDropdownTippyRef = useRef();
   const onOptionsDropdownCreate = (ref) => (optionsDropdownTippyRef.current = ref);
 
@@ -31,8 +32,13 @@ const ExportToPostman = ({ onClose, onExported, collection }) => {
     },
     validationSchema: Yup.object({
       fileName: Yup.string()
+        .trim()
         .min(1, 'must be at least 1 character')
         .max(255, 'must be 255 characters or less')
+        .test('is-valid-name', function (value) {
+          const isValid = validateName(value);
+          return isValid ? true : this.createError({ message: validateNameError(value) });
+        })
         .required('Name is required'),
       location: Yup.string().min(1, 'Location is required').required('Location is required')
     }),
@@ -43,9 +49,12 @@ const ExportToPostman = ({ onClose, onExported, collection }) => {
   const fileExists = formik.errors.fileName === FILE_EXISTS_ERROR;
 
   async function handleExport(values, overwrite) {
+    if (isExporting) return;
+
+    setIsExporting(true);
     try {
       const content = exportPostmanCollection(cloneDeep(collection), { preserveScripts });
-      await dispatch(exportCollectionToPostman(values.location, `${values.fileName}.json`, content, overwrite));
+      await dispatch(exportCollectionToPostman(values.location, `${values.fileName.trim()}.json`, content, overwrite));
       toast.success('Collection exported successfully');
       onExported();
     } catch (error) {
@@ -57,6 +66,8 @@ const ExportToPostman = ({ onClose, onExported, collection }) => {
         return;
       }
       toast.error('Failed to export collection: ' + message);
+    } finally {
+      setIsExporting(false);
     }
   }
 
@@ -102,6 +113,7 @@ const ExportToPostman = ({ onClose, onExported, collection }) => {
           dataTestId="export-to-postman-modal"
           confirmText={fileExists ? 'Replace' : 'Export'}
           confirmButtonColor={fileExists ? 'danger' : 'primary'}
+          confirmDisabled={isExporting}
           handleConfirm={() => (fileExists ? handleReplace() : formik.handleSubmit())}
           handleCancel={onClose}
           footerLeft={(
@@ -182,7 +194,7 @@ const ExportToPostman = ({ onClose, onExported, collection }) => {
                 <div>
                   <span className="preserve-scripts-label">Preserve scripts</span>
                   <p className="preserve-scripts-description">
-                    Exports bru.* scripts as-is, without translating them to pm.*.
+                    Export Bruno scripts without translating them.
                   </p>
                 </div>
               </label>

packages/bruno-app/src/components/Sidebar/ImportCollectionLocation/index.js

@@ -373,7 +373,7 @@ const ImportCollectionLocation = ({ onClose, handleSubmit, rawData, format, sour
                   <div>
                     <span className="checkbox-option-label">Preserve scripts</span>
                     <p className="checkbox-option-description">
-                      Imports pm.* scripts as-is, without translating them to bru.*.
+                      Import Postman scripts without translating them.
                     </p>
                   </div>
                 </label>

packages/bruno-electron/src/ipc/collection.js

@@ -2555,7 +2555,12 @@ const registerRendererEventHandlers = (mainWindow, watcher) => {
         throw new Error('Export location does not exist');
       }
 
-      const filePath = path.join(dirPath, fileName);
+      // ensure the resolved path is inside the export directory
+      const resolvedDir = path.resolve(dirPath);
+      const filePath = path.resolve(resolvedDir, fileName);
+      if (!filePath.startsWith(resolvedDir + path.sep) && filePath !== resolvedDir) {
+        throw new Error('Invalid file name');
+      }
 
       if (!overwrite && fs.existsSync(filePath)) {
         throw new Error(`path: ${filePath} already exists`);

tests/export/postman/export-preserve-scripts.spec.ts

@@ -1,62 +1,16 @@
-import { test, expect, Page } from '../../../playwright';
-import { closeAllCollections, openCollection } from '../../utils/page';
-import { buildCommonLocators } from '../../utils/page/locators';
+import { test, expect } from '../../../playwright';
+import {
+  closeAllCollections,
+  buildCommonLocators,
+  openExportToPostmanModal,
+  closeExportToPostmanModal,
+  exportCollectionToPostman
+} from '../../utils/page';
 import * as fs from 'fs';
 import * as nodePath from 'path';
 
 const COLLECTION_NAME = 'Export Scripts Collection';
 
-const openExportToPostmanModal = async (page: Page) => {
-  const locators = buildCommonLocators(page);
-
-  await openCollection(page, COLLECTION_NAME);
-
-  const collectionAction = locators.actions.collectionActions(COLLECTION_NAME);
-  await locators.sidebar.collection(COLLECTION_NAME).hover();
-  await expect(collectionAction).toBeVisible({ timeout: 2000 });
-  await collectionAction.click();
-  await locators.dropdown.item('Share').click();
-  await expect(locators.modal.title('Share Collection')).toBeVisible();
-
-  await page.getByTestId('export-format-postman').click();
-  await locators.modal.button('Proceed').click();
-  await expect(locators.modal.title('Export to Postman')).toBeVisible();
-};
-
-// export a collection via Share -> Export to Postman into outputDir and read the file
-const exportPostmanCollection = async (
-  page: Page,
-  outputDir: string,
-  { preserveScripts }: { preserveScripts: boolean }
-) => {
-  const locators = buildCommonLocators(page);
-
-  await openExportToPostmanModal(page);
-
-  await test.step('Set the export location', async () => {
-    await page.getByLabel('Location', { exact: true }).fill(outputDir);
-  });
-
-  await test.step('Configure preserve scripts', async () => {
-    if (preserveScripts) {
-      // 'Preserve scripts' lives under the modal footer's advanced options
-      await page.getByRole('button', { name: 'Options' }).click();
-      await page.getByTestId('show-advanced-options-toggle').click();
-      const checkbox = page.getByTestId('preserve-scripts-toggle');
-      await expect(checkbox).toBeVisible();
-      await checkbox.check();
-      await expect(checkbox).toBeChecked();
-    }
-  });
-
-  return await test.step('Export and read the written file', async () => {
-    await locators.modal.button('Export').click();
-    const filePath = nodePath.join(outputDir, `${COLLECTION_NAME}.json`);
-    await expect.poll(() => fs.existsSync(filePath), { timeout: 5000 }).toBe(true);
-    return JSON.parse(fs.readFileSync(filePath, 'utf-8'));
-  });
-};
-
 const findLoginEvents = (exported: any) => {
   const login = exported.item.find((i: any) => i.name === 'Login');
   return {
@@ -66,13 +20,13 @@ const findLoginEvents = (exported: any) => {
 };
 
 test.describe('Export Postman Collection - Preserve scripts option', () => {
-  test.afterAll(async ({ pageWithUserData: page }) => {
+  test.afterEach(async ({ pageWithUserData: page }) => {
     await closeAllCollections(page);
   });
 
   test('should export scripts as is when preserve scripts is enabled', async ({ pageWithUserData: page, createTmpDir }) => {
     const outputDir = await createTmpDir('postman-export');
-    const exported = await exportPostmanCollection(page, outputDir, { preserveScripts: true });
+    const exported = await exportCollectionToPostman(page, COLLECTION_NAME, outputDir, { preserveScripts: true });
     const { prerequest, test: testEvent } = findLoginEvents(exported);
 
     expect(prerequest.script.exec.join('\n')).toContain('bru.setEnvVar(\'token\', \'abc\')');
@@ -83,7 +37,7 @@ test.describe('Export Postman Collection - Preserve scripts option', () => {
 
   test('should translate scripts to pm.* by default when preserve scripts is disabled', async ({ pageWithUserData: page, createTmpDir }) => {
     const outputDir = await createTmpDir('postman-export');
-    const exported = await exportPostmanCollection(page, outputDir, { preserveScripts: false });
+    const exported = await exportCollectionToPostman(page, COLLECTION_NAME, outputDir, { preserveScripts: false });
     const { prerequest } = findLoginEvents(exported);
 
     expect(prerequest.script.exec.join('\n')).toContain('pm.environment.set');
@@ -98,10 +52,10 @@ test.describe('Export Postman Collection - Preserve scripts option', () => {
     // Pre seed a conflicting file so the export hits the "already exists" path
     fs.writeFileSync(filePath, '{"stale":true}', 'utf-8');
 
-    await openExportToPostmanModal(page);
+    await openExportToPostmanModal(page, COLLECTION_NAME);
 
     await test.step('Exporting over an existing file prompts to replace', async () => {
-      await page.getByLabel('Location', { exact: true }).fill(outputDir);
+      await locators.export.locationInput().fill(outputDir);
       await locators.modal.button('Export').click();
       await expect(page.getByText('Name already exists in this location')).toBeVisible();
       await expect(locators.modal.button('Replace')).toBeVisible();
@@ -123,10 +77,10 @@ test.describe('Export Postman Collection - Preserve scripts option', () => {
   test('should require a name and location before exporting', async ({ pageWithUserData: page }) => {
     const locators = buildCommonLocators(page);
 
-    await openExportToPostmanModal(page);
+    await openExportToPostmanModal(page, COLLECTION_NAME);
 
     await test.step('Clearing the name and location blocks export with validation errors', async () => {
-      await page.getByLabel('Name', { exact: true }).fill('');
+      await locators.export.nameInput().fill('');
       await locators.modal.button('Export').click();
 
       await expect(page.getByText('Name is required')).toBeVisible();
@@ -135,13 +89,24 @@ test.describe('Export Postman Collection - Preserve scripts option', () => {
       await expect(locators.modal.title('Export to Postman')).toBeVisible();
     });
 
-    // Close both modals to cleanup the app state for the next test
-    await test.step('Close the export and share modals', async () => {
-      await locators.export.postmanModal().getByTestId('modal-close-button').click();
-      await expect(locators.modal.title('Export to Postman')).toBeHidden();
-      await locators.modal.byTitle('Share Collection').getByTestId('modal-close-button').click();
-      await expect(locators.modal.title('Share Collection')).toBeHidden();
+    await closeExportToPostmanModal(page);
+  });
+
+  test('should block export when the name has invalid characters', async ({ pageWithUserData: page }) => {
+    const locators = buildCommonLocators(page);
+
+    await openExportToPostmanModal(page, COLLECTION_NAME);
+
+    await test.step('An invalid name shows a validation error and blocks export', async () => {
+      await locators.export.nameInput().fill('foo/bar');
+      await locators.modal.button('Export').click();
+
+      await expect(page.getByText('Special characters aren\'t allowed in the name')).toBeVisible();
+      // The modal stays open and the export did not proceed
+      await expect(locators.modal.title('Export to Postman')).toBeVisible();
     });
+
+    await closeExportToPostmanModal(page);
   });
 
   test('should let the user rename to avoid overwriting an existing file', async ({ pageWithUserData: page, createTmpDir }) => {
@@ -151,17 +116,17 @@ test.describe('Export Postman Collection - Preserve scripts option', () => {
     // Pre-seed a conflicting file for the default (collection) name
     fs.writeFileSync(nodePath.join(outputDir, `${COLLECTION_NAME}.json`), '{"stale":true}', 'utf-8');
 
-    await openExportToPostmanModal(page);
+    await openExportToPostmanModal(page, COLLECTION_NAME);
 
     await test.step('Exporting over an existing file prompts to replace', async () => {
-      await page.getByLabel('Location', { exact: true }).fill(outputDir);
+      await locators.export.locationInput().fill(outputDir);
       await locators.modal.button('Export').click();
       await expect(page.getByText('Name already exists in this location')).toBeVisible();
       await expect(locators.modal.button('Replace')).toBeVisible();
     });
 
     await test.step('Renaming clears the conflict and restores Export', async () => {
-      await page.getByLabel('Name', { exact: true }).fill('Renamed Export');
+      await locators.export.nameInput().fill('Renamed Export');
       await expect(page.getByText('Name already exists in this location')).toBeHidden();
       await expect(locators.modal.button('Export')).toBeVisible();
     });

tests/utils/page/actions.ts

@@ -1,6 +1,7 @@
 import { test, expect, Page, Locator, ElectronApplication, waitForReadyPage as waitForReadyPageImpl } from '../../../playwright';
 import process from 'node:process';
 import * as path from 'path';
+import * as fs from 'fs';
 import { buildCommonLocators, buildScriptErrorLocators, buildGrpcCommonLocators, buildWebsocketCommonLocators } from './locators';
 import { waitForCollectionMount } from './mounting';
 
@@ -2145,6 +2146,68 @@ const generateCollectionDocs = async (
   });
 };
 
+const openExportToPostmanModal = async (page: Page, collectionName: string) => {
+  await test.step(`Open Export to Postman for "${collectionName}"`, async () => {
+    const locators = buildCommonLocators(page);
+
+    await openCollection(page, collectionName);
+
+    const collectionAction = locators.actions.collectionActions(collectionName);
+    await locators.sidebar.collection(collectionName).hover();
+    await expect(collectionAction).toBeVisible({ timeout: 2000 });
+    await collectionAction.click();
+    await locators.dropdown.item('Share').click();
+    await expect(locators.modal.title('Share Collection')).toBeVisible();
+
+    await locators.export.postmanFormatCard().click();
+    await locators.modal.button('Proceed').click();
+    await expect(locators.modal.title('Export to Postman')).toBeVisible();
+  });
+};
+
+const closeExportToPostmanModal = async (page: Page) => {
+  await test.step('Close the export modal', async () => {
+    const locators = buildCommonLocators(page);
+
+    await locators.export.postmanModal().getByTestId('modal-close-button').click();
+    await expect(locators.modal.title('Export to Postman')).toBeHidden();
+    await expect(locators.modal.title('Share Collection')).toBeHidden();
+  });
+};
+
+const exportCollectionToPostman = async (
+  page: Page,
+  collectionName: string,
+  outputDir: string,
+  { preserveScripts = false }: { preserveScripts?: boolean } = {}
+) => {
+  const locators = buildCommonLocators(page);
+
+  await openExportToPostmanModal(page, collectionName);
+
+  await test.step('Set the export location', async () => {
+    await locators.export.locationInput().fill(outputDir);
+  });
+
+  await test.step('Configure preserve scripts', async () => {
+    if (preserveScripts) {
+      await locators.export.optionsButton().click();
+      await locators.export.advancedOptionsToggle().click();
+      const checkbox = locators.export.preserveScriptsToggle();
+      await expect(checkbox).toBeVisible();
+      await checkbox.check();
+      await expect(checkbox).toBeChecked();
+    }
+  });
+
+  return await test.step('Export and read the written file', async () => {
+    await locators.modal.button('Export').click();
+    const filePath = path.join(outputDir, `${collectionName}.json`);
+    await expect.poll(() => fs.existsSync(filePath), { timeout: 5000 }).toBe(true);
+    return JSON.parse(fs.readFileSync(filePath, 'utf-8'));
+  });
+};
+
 /**
  * Toggle the "Enable App" request setting idempotently (Settings tab).
  * Enabling exposes the App tab and the Request/App/File view-mode toggle.
@@ -2482,6 +2545,9 @@ export {
   openRequestInFolder,
   setUrlEncoding,
   generateCollectionDocs,
+  openExportToPostmanModal,
+  closeExportToPostmanModal,
+  exportCollectionToPostman,
   openFolderSettings,
   setTableRowDescriptionValue,
   setAppCode,