feat: add preserve scripts option for Postman import/export#8654
feat: add preserve scripts option for Postman import/export#8654gopu-bruno wants to merge 6 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis change adds optional script preservation to Postman↔Bruno conversion, exposes the setting in import and export advanced options, and replaces direct Postman downloads with a validated modal export flow backed by Redux and Electron IPC. ChangesPostman preservation and export
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ShareCollection
participant ExportToPostman
participant ReduxStore
participant ElectronIPC
participant FileSystem
User->>ShareCollection: select Postman export
ShareCollection->>ExportToPostman: open modal
User->>ExportToPostman: configure name, location, and scripts
ExportToPostman->>ReduxStore: dispatch exportCollectionToPostman
ReduxStore->>ElectronIPC: invoke export handler
ElectronIPC->>FileSystem: validate and write JSON
FileSystem-->>ElectronIPC: success or existing-file error
ElectronIPC-->>ExportToPostman: return result
ExportToPostman-->>ShareCollection: close after completion
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
tests/import/postman/import-preserve-scripts.spec.ts (1)
23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the new import-option locators.
Both specs bypass
buildCommonLocators()for controls introduced by this change.
tests/import/postman/import-preserve-scripts.spec.ts#L23-L31: add and use shared locators for the advanced-options toggle and preserve-scripts checkbox.tests/import/postman/import-file-format-options.spec.ts#L34-L54: use the same shared advanced-options locator instead of the raw test ID.As per path instructions, “Don’t inline raw page selectors in specs—use locator/action builders from tests/utils/page/* via buildCommonLocators.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/import/postman/import-preserve-scripts.spec.ts` around lines 23 - 31, Centralize the new import-option selectors through buildCommonLocators: in tests/import/postman/import-preserve-scripts.spec.ts lines 23-31, add and use shared locators for show-advanced-options-toggle and preserve-scripts-toggle; in tests/import/postman/import-file-format-options.spec.ts lines 34-54, replace the raw advanced-options test ID with that shared locator.Source: Path instructions
tests/import/postman/import-file-format-options.spec.ts (1)
34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared import locator for the advanced-options toggle.
Add this control to
buildCommonLocators()and consume it here instead of repeating the raw test ID.As per path instructions, “Don’t inline raw page selectors in specs—use locator/action builders from tests/utils/page/* via buildCommonLocators.”
Also applies to: 52-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/import/postman/import-file-format-options.spec.ts` at line 34, Move the advanced-options toggle locator into buildCommonLocators() and expose it through the shared import locators. Update the test’s toggle interactions around the referenced lines to use that shared locator/action builder instead of the raw show-advanced-options-toggle test ID.Source: Path instructions
packages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.js (1)
113-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHandle the dropdown ref explicitly.
Replace the optional chain with an explicit guard before calling
hide().- optionsDropdownTippyRef?.current?.hide(); + if (optionsDropdownTippyRef.current) { + optionsDropdownTippyRef.current.hide(); + }As per coding guidelines, “Avoid optional chaining unless the null case is handled immediately through a fallback, early return, or guard.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.js` around lines 113 - 116, Update the onClick handler in the ExportToPostman component to explicitly check optionsDropdownTippyRef.current before invoking hide(), while preserving the existing setShowAdvancedOptions toggle behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-app/src/components/ShareCollection/index.js`:
- Around line 211-217: Update the ShareCollection render condition around
ExportToPostman so it is displayed exclusively when showPostmanExportModal is
true, while the Share Collection dialog is unmounted or hidden during that
state. Preserve the existing handlePostmanModalClose and handlePostmanExported
callbacks so closing or completing the export returns to Share Collection
without competing focus traps or Escape handlers.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.js`:
- Around line 45-50: Update the ExportToPostman export flow around handleExport
to track an isExporting state, set it while the async export is in progress, and
clear it when the operation finishes. Pass isExporting to the confirmation
control’s confirmDisabled prop so repeated clicks or Enter presses are disabled
until export completion.
- Around line 32-38: Update the validationSchema filename rule in
ExportToPostman to reuse the existing Create Collection filename validation,
including cross-platform filesystem restrictions such as trimming whitespace,
rejecting separators, and disallowing reserved names. Ensure the submitted
filename is trimmed before export while preserving the current required,
minimum, and maximum length behavior.
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 2540-2552: Update the renderer:export-collection-postman handler
to validate that the resolved file path for fileName remains inside dirPath
before checking existence or writing. Resolve both the export directory and
target path, reject traversal or escape cases, then continue the existing
overwrite and writeFile flow only for valid paths.
In `@tests/export/postman/export-preserve-scripts.spec.ts`:
- Around line 9-56: The export spec currently embeds raw selectors and modal
navigation instead of using page modules. Add semantic locators for the Postman
export modal, options, preserve-scripts toggle, location, and export controls to
buildCommonLocators in locators.ts; move openExportToPostmanModal and
exportPostmanCollection into reusable actions in actions.ts, then update the
spec to call those actions and retain only assertions.
- Around line 68-71: Change the test hook in “Export Postman Collection -
Preserve scripts option” from afterAll to afterEach so closeAllCollections runs
after every test, preserving deterministic isolation even when an individual
test fails.
---
Nitpick comments:
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.js`:
- Around line 113-116: Update the onClick handler in the ExportToPostman
component to explicitly check optionsDropdownTippyRef.current before invoking
hide(), while preserving the existing setShowAdvancedOptions toggle behavior.
In `@tests/import/postman/import-file-format-options.spec.ts`:
- Line 34: Move the advanced-options toggle locator into buildCommonLocators()
and expose it through the shared import locators. Update the test’s toggle
interactions around the referenced lines to use that shared locator/action
builder instead of the raw show-advanced-options-toggle test ID.
In `@tests/import/postman/import-preserve-scripts.spec.ts`:
- Around line 23-31: Centralize the new import-option selectors through
buildCommonLocators: in tests/import/postman/import-preserve-scripts.spec.ts
lines 23-31, add and use shared locators for show-advanced-options-toggle and
preserve-scripts-toggle; in
tests/import/postman/import-file-format-options.spec.ts lines 34-54, replace the
raw advanced-options test ID with that shared locator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b5f8794-7921-4472-a8da-93c78fbfebec
📒 Files selected for processing (22)
packages/bruno-app/src/components/ShareCollection/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.jspackages/bruno-app/src/components/Sidebar/CreateCollection/index.jspackages/bruno-app/src/components/Sidebar/ImportCollectionLocation/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ImportCollectionLocation/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/utils/exporters/postman-collection.jspackages/bruno-app/src/utils/importers/postman-collection.jspackages/bruno-converters/src/postman/bruno-to-postman.jspackages/bruno-converters/src/postman/postman-to-bruno.jspackages/bruno-converters/tests/postman/preserve-scripts.spec.jspackages/bruno-electron/src/ipc/collection.jstests/export/postman/export-preserve-scripts.spec.tstests/export/postman/fixtures/collection/bruno.jsontests/export/postman/fixtures/collection/login.brutests/export/postman/init-user-data/preferences.jsontests/import/postman/fixtures/postman-with-scripts.jsontests/import/postman/import-file-format-options.spec.tstests/import/postman/import-preserve-scripts.spec.tstests/utils/page/actions.tstests/utils/page/locators.ts
fc85d5d to
43e05a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-electron/src/ipc/collection.js (1)
1599-1609: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate that the
appitem's path is strictly within the collection directory.As indicated by static analysis, the
item.pathnameis provided by the renderer and read directly from the filesystem without validation. To prevent path traversal attacks or accidental corruption of files outside the collection, validate thatitem.pathnameis strictly within the expected collection directory before reading or writing.You can use
validatePathIsInsideCollection(item.pathname)to enforce this boundary, similar to other IPC handlers in this file.🛡️ Proposed fix
} else if (item?.type === 'app') { + validatePathIsInsideCollection(item.pathname); if (fs.existsSync(item.pathname)) { const existingContent = fs.readFileSync(item.pathname, 'utf8');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/ipc/collection.js` around lines 1599 - 1609, In the app-item branch, validate item.pathname with validatePathIsInsideCollection before any filesystem access, including fs.existsSync, fs.readFileSync, and writeFile. Preserve the existing sequence comparison and update behavior only for paths confirmed to be inside the collection directory.Source: Linters/SAST tools
♻️ Duplicate comments (1)
packages/bruno-electron/src/ipc/collection.js (1)
2552-2576: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject filenames that escape the export directory.
fileNameis renderer-controlled, so values such as../settings.jsonmakepath.join()write outside the selected directory. Validate the resolved parent before checking or writing the file.While the added
startsWithcheck provides some protection against directory traversal, it still allowsfileNameto be empty (which attempts to overwrite the directory itself) and doesn't verify ifdirPathis a valid directory. Usepath.dirname()for stricter boundary checking.🛡️ Proposed fix to resolve the remaining boundary flaws
- if (!dirPath || !fs.existsSync(dirPath)) { + if (!dirPath || !fs.existsSync(dirPath) || !fs.statSync(dirPath).isDirectory()) { throw new Error('Export location does not exist'); } - // 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) { + if (!fileName || path.dirname(filePath) !== resolvedDir) { throw new Error('Invalid file name'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/ipc/collection.js` around lines 2552 - 2576, Harden the `renderer:export-collection-postman` handler by validating that `dirPath` exists and is a directory, rejecting an empty `fileName`, and resolving the target parent with `path.dirname(filePath)`. Require that parent to remain within `resolvedDir` before checking overwrite status or calling `writeFile`; preserve the existing success and rejection behavior.
🧹 Nitpick comments (1)
tests/export/postman/export-preserve-scripts.spec.ts (1)
100-104: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the Windows path separator too.
This case can pass while
foo\barremains accepted on Windows. Parameterize the validation test with both/and\.As per path instructions, “Bruno is a cross-platform Electron desktop app that runs on macOS, Windows, and Linux.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/export/postman/export-preserve-scripts.spec.ts` around lines 100 - 104, Update the invalid-name validation test around the export name input to run for both path separators, “/” and “\”, using the test framework’s parameterization support. For each input, preserve the existing export attempt and assertion that the special-characters validation message is visible.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/export/postman/export-preserve-scripts.spec.ts`:
- Around line 32-35: Update the assertions in the export-preservation test to
compare the complete prerequest.script.exec and testEvent.script.exec arrays
with the corresponding fixture script arrays. Replace the substring and
pm.-absence checks while preserving the existing exact script content,
formatting, comments, ordering, and statements.
In `@tests/utils/page/actions.ts`:
- Around line 513-521: Centralize the advanced-options and preserve-scripts
test-id locators in the import namespace of buildCommonLocators within
locators.ts, mirroring the existing export namespace definitions. Update the
import flow in the preserveScripts branch to consume those centralized locators
instead of calling page.getByTestId inline, while keeping the existing click,
check, and assertion behavior unchanged.
---
Outside diff comments:
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 1599-1609: In the app-item branch, validate item.pathname with
validatePathIsInsideCollection before any filesystem access, including
fs.existsSync, fs.readFileSync, and writeFile. Preserve the existing sequence
comparison and update behavior only for paths confirmed to be inside the
collection directory.
---
Duplicate comments:
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 2552-2576: Harden the `renderer:export-collection-postman` handler
by validating that `dirPath` exists and is a directory, rejecting an empty
`fileName`, and resolving the target parent with `path.dirname(filePath)`.
Require that parent to remain within `resolvedDir` before checking overwrite
status or calling `writeFile`; preserve the existing success and rejection
behavior.
---
Nitpick comments:
In `@tests/export/postman/export-preserve-scripts.spec.ts`:
- Around line 100-104: Update the invalid-name validation test around the export
name input to run for both path separators, “/” and “\”, using the test
framework’s parameterization support. For each input, preserve the existing
export attempt and assertion that the special-characters validation message is
visible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c66ac9da-1f18-4283-9fbd-95b897075f11
📒 Files selected for processing (22)
packages/bruno-app/src/components/ShareCollection/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.jspackages/bruno-app/src/components/Sidebar/CreateCollection/index.jspackages/bruno-app/src/components/Sidebar/ImportCollectionLocation/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ImportCollectionLocation/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/utils/exporters/postman-collection.jspackages/bruno-app/src/utils/importers/postman-collection.jspackages/bruno-converters/src/postman/bruno-to-postman.jspackages/bruno-converters/src/postman/postman-to-bruno.jspackages/bruno-converters/tests/postman/preserve-scripts.spec.jspackages/bruno-electron/src/ipc/collection.jstests/export/postman/export-preserve-scripts.spec.tstests/export/postman/fixtures/collection/bruno.jsontests/export/postman/fixtures/collection/login.brutests/export/postman/init-user-data/preferences.jsontests/import/postman/fixtures/postman-with-scripts.jsontests/import/postman/import-file-format-options.spec.tstests/import/postman/import-preserve-scripts.spec.tstests/utils/page/actions.tstests/utils/page/locators.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- tests/export/postman/fixtures/collection/login.bru
- tests/export/postman/fixtures/collection/bruno.json
- packages/bruno-app/src/components/Sidebar/ImportCollectionLocation/StyledWrapper.js
- packages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/StyledWrapper.js
- tests/import/postman/import-preserve-scripts.spec.ts
- tests/export/postman/init-user-data/preferences.json
- tests/import/postman/fixtures/postman-with-scripts.json
- packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
- packages/bruno-app/src/utils/exporters/postman-collection.js
- packages/bruno-app/src/components/Sidebar/Collections/Collection/ExportCollection/ExportToPostman/index.js
- tests/import/postman/import-file-format-options.spec.ts
- packages/bruno-app/src/components/Sidebar/ImportCollectionLocation/index.js
- packages/bruno-converters/tests/postman/preserve-scripts.spec.js
- packages/bruno-app/src/components/Sidebar/CreateCollection/index.js
- packages/bruno-converters/src/postman/postman-to-bruno.js
- packages/bruno-app/src/components/ShareCollection/index.js
| // Convert raw data to Bruno collection format | ||
| // Returns { collection, issues } where issues tracks items that were skipped or degraded | ||
| const convertCollection = async (format, rawData, groupingType, collectionFormat) => { | ||
| const convertCollection = async (format, rawData, groupingType, collectionFormat, preserveScripts) => { |
There was a problem hiding this comment.
Consider using an options object to improve readability and avoid parameter ordering issues.
| }); | ||
| }; | ||
|
|
||
| export const exportCollectionToPostman = (location, fileName, content, overwrite = false) => (_dispatch, _getState) => { |
There was a problem hiding this comment.
same like above Consider using an options object instead of multiple parameters.
| }; | ||
|
|
||
| const importPostmanV2CollectionItem = (brunoParent, item, { useWorkers = false } = {}, scriptMap, issues = [], parentPath = '') => { | ||
| const importPostmanV2CollectionItem = (brunoParent, item, { useWorkers = false, preserveScripts = false } = {}, scriptMap, issues = [], parentPath = '') => { |
There was a problem hiding this comment.
same options like above for more params, but check before its use in other files also.
| const handleProceed = async () => { | ||
| if (isCollectionLoading || isExporting) return; | ||
|
|
||
| if (selectedFormat === EXPORT_FORMATS.POSTMAN) { |
There was a problem hiding this comment.
why to take this condition out from switch?
Description
Adds support for preserving Postman scripts during import and export instead of translating them into Bruno scripts.
Introduce an optional preserve scripts mode that skips script translation while continuing to convert the rest of the collection collection structure, requests, variables, and settings.
Ref: BRU-2573
When enabled:
Screen Recording
Screen.Recording.2026-07-17.at.1.10.38.PM.mov
Screen.Recording.2026-07-17.at.1.11.45.PM.mov
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests