Skip to content

Conversation

@naman-bruno
Copy link
Collaborator

@naman-bruno naman-bruno commented Feb 2, 2026

Description

Contribution Checklist:

  • I've used AI significantly to create this pull request
  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

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

  • New Features
    • Multi-format collection export: ZIP, YAML (OpenCollection), and Postman.
    • Redesigned Share Collection dialog with selectable format cards and clearer feature descriptions.
    • ZIP export now supports user save flow and shows success/cancellation feedback.
    • Asynchronous export flow with toast notifications and format-specific warnings for non-exportable request types.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 2, 2026

Walkthrough

Refactors ShareCollection into a multi-format export UI (ZIP, YAML, Postman) with card selection and async export handlers; adds an Electron IPC handler to create ZIP archives from a collection path.

Changes

Cohort / File(s) Summary
ShareCollection UI Styling
packages/bruno-app/src/components/ShareCollection/StyledWrapper.js
Replaces beta-badge styling with section title, link, two-column grids, and extensive card styles (.format-card, .other-format-card, badges, feature lists, modal-footer). Presentation-only changes.
ShareCollection Component Logic
packages/bruno-app/src/components/ShareCollection/index.js
Refactors component to add local state (selectedFormat, isExporting), EXPORT_FORMATS enum, per-format async handlers (ZIP via IPC, YAML via OpenCollection export, POSTMAN export), centralized proceed flow, toast notifications, and card-based modal UI with warnings for non-exportable requests.
Electron IPC Collection Handler
packages/bruno-electron/src/ipc/collection.js
Adds renderer:export-collection-zip IPC handler that prompts save dialog, streams a ZIP archive (using archiver) of the collection while excluding node_modules/.git, and returns success/cancel or error. Note: handler appears duplicated in file.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as ShareCollection (React)
    participant IPC as IPC Renderer
    participant Main as Electron Main
    participant Archiver as Archiver
    participant FS as Disk

    User->>UI: Click Proceed (selected format)
    activate UI
    UI->>UI: validate & set exporting state
    UI->>IPC: invoke('renderer:export-collection-zip', collectionPath, collectionName)
    deactivate UI

    IPC->>Main: forward request
    activate Main
    Main->>Main: validate inputs & show save dialog
    Main->>Archiver: create ZIP stream (filter .git, node_modules)
    activate Archiver
    Archiver->>FS: stream archive to file
    deactivate Archiver
    Main->>IPC: return { success, filepath } or { cancelled } / error
    deactivate Main

    IPC->>UI: resolve result
    activate UI
    UI->>User: show toast (success/error) and close modal
    deactivate UI
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • helloanoop
  • lohit-bruno
  • bijin-bruno

Poem

🎁 Cards align, three paths to send,
Zip, YAML, Postman — pick a friend.
Electron hums, archiver sings,
Toasts applaud the exported things,
A tidy flow that ships the end. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: the ShareCollection component is significantly enhanced with multi-format export UI, state management, and styling improvements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/components/ShareCollection/index.js`:
- Around line 70-92: The modal is closed unconditionally after exports even when
the user cancels (e.g., canceling the ZIP save dialog), so change the export
handlers to signal success and only call onClose when an export actually
completed: update handleExportZip (and similarly
handleExportYaml/handleExportPostman if they can be canceled) to return a
boolean (true on successful save, false on cancel), then in handleProceed await
the handler result for each case and only call onClose() when the returned value
is true; keep existing isExporting state handling and error catch as-is.
🧹 Nitpick comments (4)
packages/bruno-electron/src/ipc/collection.js (1)

1862-1920: New ZIP export handler looks solid overall.

A few observations:

  1. Potential unhandled rejection: If archive.finalize() throws synchronously before the Promise settles, the error may not be caught by archive.on('error'). Consider wrapping finalize() in a try-catch.

  2. Blocking I/O: addDirectoryToArchive uses synchronous fs.readdirSync. For very large collections this could block the main process. Consider async iteration if performance becomes an issue.

  3. Dialog cancellation handling: The handler correctly returns { success: false, canceled: true } on cancel, which aligns with the renderer's expectations.

🛡️ Suggested defensive improvement for finalize
         addDirectoryToArchive(collectionPath, '');
-        archive.finalize();
+        archive.finalize().catch(reject);
       });
packages/bruno-app/src/components/ShareCollection/StyledWrapper.js (1)

83-91: Hardcoded color should use theme prop.

Line 89 uses color: white directly. Per coding standards, styled components should use the theme prop for colors to maintain theming consistency.

♻️ Suggested fix
       .recommended-badge {
         padding: 0.125rem 0.5rem;
         font-size: 0.6875rem;
         font-weight: 600;
         border-radius: 0.25rem;
         background-color: ${(props) => props.theme.colors.text.warning};
-        color: white;
+        color: ${(props) => props.theme.colors.text.onWarning || '#ffffff'};
       }

Based on learnings: "Use styled component's theme prop to manage CSS colors, not CSS variables, when in the context of a styled component."

packages/bruno-app/src/components/ShareCollection/index.js (2)

48-58: Consider handling the canceled case explicitly.

When the user cancels the save dialog, result.canceled is true but the code doesn't provide feedback. The modal will close silently without any indication.

♻️ Suggested improvement
   const handleExportZip = async () => {
     try {
       const { ipcRenderer } = window;
       const result = await ipcRenderer.invoke('renderer:export-collection-zip', collection.pathname, collection.name);
       if (result.success) {
         toast.success('Collection exported successfully');
+        return true;
+      } else if (result.canceled) {
+        return false;
       }
     } catch (error) {
       toast.error('Failed to export collection: ' + error.message);
+      throw error;
     }
   };

182-188: Inline styles with hardcoded colors bypass theming.

The warning alert uses inline style attributes with hardcoded RGBA and hex values. This won't adapt if themes change and is inconsistent with the styled-component approach used elsewhere.

Consider moving these styles to StyledWrapper and using theme colors.

♻️ Example approach
-          <div className="flex items-center mt-4 p-3 rounded" style={{ backgroundColor: 'rgba(251, 191, 36, 0.1)' }}>
-            <IconAlertTriangle size={16} className="mr-2 flex-shrink-0" style={{ color: '#f59e0b' }} />
-            <span className="text-sm" style={{ color: '#f59e0b' }}>
+          <div className="warning-alert flex items-center mt-4 p-3 rounded">
+            <IconAlertTriangle size={16} className="mr-2 flex-shrink-0 warning-icon" />
+            <span className="text-sm warning-text">

Then in StyledWrapper add:

.warning-alert {
  background-color: ${(props) => rgba(props.theme.colors.text.warning, 0.1)};
}
.warning-icon, .warning-text {
  color: ${(props) => props.theme.colors.text.warning};
}

Based on learnings: "Use styled component's theme prop to manage CSS colors."

Comment on lines +70 to 92
const handleProceed = async () => {
if (isCollectionLoading || isExporting) return;

setIsExporting(true);
try {
switch (selectedFormat) {
case EXPORT_FORMATS.ZIP:
await handleExportZip();
break;
case EXPORT_FORMATS.YAML:
handleExportYaml();
break;
case EXPORT_FORMATS.POSTMAN:
handleExportPostman();
break;
}
onClose();
} catch (error) {
console.error('Export error:', error);
} finally {
setIsExporting(false);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Modal closes even when export is canceled.

If the user cancels the ZIP save dialog, handleExportZip completes without throwing, so onClose() is still called at line 86. This might be unexpected UX—users typically expect the modal to remain open if they cancel.

♻️ Suggested fix to keep modal open on cancel
   const handleProceed = async () => {
     if (isCollectionLoading || isExporting) return;

     setIsExporting(true);
     try {
+      let shouldClose = true;
       switch (selectedFormat) {
         case EXPORT_FORMATS.ZIP:
-          await handleExportZip();
+          shouldClose = await handleExportZip();
           break;
         case EXPORT_FORMATS.YAML:
           handleExportYaml();
           break;
         case EXPORT_FORMATS.POSTMAN:
           handleExportPostman();
           break;
       }
-      onClose();
+      if (shouldClose !== false) {
+        onClose();
+      }
     } catch (error) {
       console.error('Export error:', error);
     } finally {
       setIsExporting(false);
     }
   };
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/components/ShareCollection/index.js` around lines 70 -
92, The modal is closed unconditionally after exports even when the user cancels
(e.g., canceling the ZIP save dialog), so change the export handlers to signal
success and only call onClose when an export actually completed: update
handleExportZip (and similarly handleExportYaml/handleExportPostman if they can
be canceled) to return a boolean (true on successful save, false on cancel),
then in handleProceed await the handler result for each case and only call
onClose() when the returned value is true; keep existing isExporting state
handling and error catch as-is.

bijin-bruno
bijin-bruno previously approved these changes Feb 2, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 1976-1985: The Promise that wraps the ZIP creation uses output
(fs.createWriteStream) and archive but only listens for output 'close' and
archive 'error'; add an output.on('error', ...) handler that rejects the Promise
(matching archive's behavior) so disk/permission errors don't cause unhandled
rejections or a hung export; update the same Promise closure where output and
archive are defined (referencing the variables output and archive) to attach and
propagate the error to the Promise rejection and perform any necessary cleanup.
- Around line 1963-1967: The code assumes collectionName is a string when
building defaultFileName and calls sanitizeName(collectionName), which will
throw if collectionName is undefined; update the logic around defaultFileName
(the variable and its use before dialog.showSaveDialog) to provide a safe
fallback (e.g. use a default string or coerce to String) so sanitizeName always
receives a string—refer to sanitizeName, collectionName, and defaultFileName
when making the change.
- Around line 1990-2003: The addDirectoryToArchive function currently archives
regular entries without checking for symlinks, which can cause files outside the
collection to be included; update addDirectoryToArchive to skip any symbolic
links: before recursing or calling archive.file, detect symlinks (use the
Dirent.isSymbolicLink() from entries or call
fs.lstatSync(fullPath).isSymbolicLink() for certainty) and continue/skip if
true, preserving the existing ignoredDirectories logic and using the same
fullPath and entryArchivePath variables so no symlink targets are followed or
archived.
- Around line 1990-2008: The synchronous recursive traversal in
addDirectoryToArchive blocks the main thread; replace it by using archiver's
glob support or an async traversal: remove the addDirectoryToArchive
implementation and call archive.glob('**/*', { cwd: collectionPath, ignore:
['node_modules/**', '.git/**', ...ignoredDirectories.map(d => d + '/**')] }) so
archiver handles files and skips ignoredDirectories, then call
archive.finalize(); alternatively, if you prefer async traversal, rewrite
addDirectoryToArchive to use fs.promises.readdir with withFileTypes and await
recursion, calling archive.file for files—ensure you reference
addDirectoryToArchive, archive, collectionPath, and ignoredDirectories when
making the change.

Comment on lines +1963 to +1967
const defaultFileName = `${sanitizeName(collectionName)}.zip`;
const { filePath, canceled } = await dialog.showSaveDialog(mainWindow, {
title: 'Export Collection as ZIP',
defaultPath: defaultFileName,
filters: [{ name: 'Zip Files', extensions: ['zip'] }]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, find and read the relevant section of the file
head -1970 packages/bruno-electron/src/ipc/collection.js | tail -50

Repository: usebruno/bruno

Length of output: 1679


🏁 Script executed:

# Get more context around the function to see the full export logic
head -2000 packages/bruno-electron/src/ipc/collection.js | tail -100

Repository: usebruno/bruno

Length of output: 3480


🏁 Script executed:

# Find the sanitizeName function definition
rg -A 10 "const sanitizeName|function sanitizeName" packages/bruno-electron/src/ipc/collection.js

Repository: usebruno/bruno

Length of output: 40


🏁 Script executed:

# Also check if sanitizeName is imported from elsewhere
head -50 packages/bruno-electron/src/ipc/collection.js | grep -i "sanitizeName\|import\|require"

Repository: usebruno/bruno

Length of output: 717


🏁 Script executed:

# Get the full imports section to see where sanitizeName comes from
head -30 packages/bruno-electron/src/ipc/collection.js

Repository: usebruno/bruno

Length of output: 1021


🏁 Script executed:

# Search for sanitizeName function definition or where it's imported from
rg "sanitizeName" packages/bruno-electron/src/ipc/collection.js -B 2 -A 2 | head -40

Repository: usebruno/bruno

Length of output: 1794


🏁 Script executed:

# Find the sanitizeName function in the codebase
fd -t f "\.js$" packages/bruno-electron/src | xargs rg -l "export.*sanitizeName|function sanitizeName" 2>/dev/null | head -5

Repository: usebruno/bruno

Length of output: 40


🏁 Script executed:

# Check bruno lang or utils packages
find . -name "*sanitize*" -o -name "*name*" | grep -E "\.(js|ts)$" | head -10

Repository: usebruno/bruno

Length of output: 357


🏁 Script executed:

# Try a different approach - search across the entire codebase for sanitizeName export
rg "export.*sanitizeName|export const sanitizeName|export function sanitizeName" --type js

Repository: usebruno/bruno

Length of output: 142


🏁 Script executed:

# Read the sanitizeName function implementation
cat packages/bruno-app/src/utils/common/regex.js

Repository: usebruno/bruno

Length of output: 2151


🏁 Script executed:

# Also check where sanitizeName is exported from in collection.js's imports
grep -n "sanitizeName" packages/bruno-electron/src/ipc/collection.js | head -5

Repository: usebruno/bruno

Length of output: 342


🏁 Script executed:

# Check the full import structure around the sanitizeName import in collection.js
sed -n '1,50p' packages/bruno-electron/src/ipc/collection.js | grep -A 20 "DEFAULT_GITIGNORE"

Repository: usebruno/bruno

Length of output: 422


🏁 Script executed:

# Check how sanitizeName is used elsewhere to see if there's a pattern of validation
rg -B 2 "sanitizeName\(" packages/bruno-electron/src/ipc/collection.js | head -40

Repository: usebruno/bruno

Length of output: 1741


🏁 Script executed:

# Verify the actual line number where the problematic code is
sed -n '1960,1970p' packages/bruno-electron/src/ipc/collection.js

Repository: usebruno/bruno

Length of output: 462


Add fallback for collectionName to prevent crash when undefined.

The sanitizeName function directly calls .replace() without null/undefined checks. Line 1963 assumes collectionName is always a string, but the handler only validates collectionPath. Pass a fallback to prevent a TypeError if collectionName is missing.

Proposed fix
-      const defaultFileName = `${sanitizeName(collectionName)}.zip`;
+      const defaultFileName = `${sanitizeName(collectionName || 'collection')}.zip`;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const defaultFileName = `${sanitizeName(collectionName)}.zip`;
const { filePath, canceled } = await dialog.showSaveDialog(mainWindow, {
title: 'Export Collection as ZIP',
defaultPath: defaultFileName,
filters: [{ name: 'Zip Files', extensions: ['zip'] }]
const defaultFileName = `${sanitizeName(collectionName || 'collection')}.zip`;
const { filePath, canceled } = await dialog.showSaveDialog(mainWindow, {
title: 'Export Collection as ZIP',
defaultPath: defaultFileName,
filters: [{ name: 'Zip Files', extensions: ['zip'] }]
🤖 Prompt for AI Agents
In `@packages/bruno-electron/src/ipc/collection.js` around lines 1963 - 1967, The
code assumes collectionName is a string when building defaultFileName and calls
sanitizeName(collectionName), which will throw if collectionName is undefined;
update the logic around defaultFileName (the variable and its use before
dialog.showSaveDialog) to provide a safe fallback (e.g. use a default string or
coerce to String) so sanitizeName always receives a string—refer to
sanitizeName, collectionName, and defaultFileName when making the change.

Comment on lines +1976 to +1985
await new Promise((resolve, reject) => {
const output = fs.createWriteStream(filePath);
const archive = archiver('zip', { zlib: { level: 9 } });

output.on('close', () => {
resolve();
});

archive.on('error', (err) => {
reject(err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n packages/bruno-electron/src/ipc/collection.js | sed -n '1970,1995p'

Repository: usebruno/bruno

Length of output: 1082


🏁 Script executed:

cat -n packages/bruno-electron/src/ipc/collection.js | sed -n '1976,2020p'

Repository: usebruno/bruno

Length of output: 1698


Add error handler to output stream to prevent unhandled rejections.

The output stream is missing an error listener. Failures like disk full or permission denied would emit unhandled errors and cause the promise to never settle, leaving the export hung.

🛠️ Proposed fix
         const output = fs.createWriteStream(filePath);
         const archive = archiver('zip', { zlib: { level: 9 } });

+        output.on('error', (err) => {
+          reject(err);
+        });
+
         output.on('close', () => {
           resolve();
         });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await new Promise((resolve, reject) => {
const output = fs.createWriteStream(filePath);
const archive = archiver('zip', { zlib: { level: 9 } });
output.on('close', () => {
resolve();
});
archive.on('error', (err) => {
reject(err);
await new Promise((resolve, reject) => {
const output = fs.createWriteStream(filePath);
const archive = archiver('zip', { zlib: { level: 9 } });
output.on('error', (err) => {
reject(err);
});
output.on('close', () => {
resolve();
});
archive.on('error', (err) => {
reject(err);
🤖 Prompt for AI Agents
In `@packages/bruno-electron/src/ipc/collection.js` around lines 1976 - 1985, The
Promise that wraps the ZIP creation uses output (fs.createWriteStream) and
archive but only listens for output 'close' and archive 'error'; add an
output.on('error', ...) handler that rejects the Promise (matching archive's
behavior) so disk/permission errors don't cause unhandled rejections or a hung
export; update the same Promise closure where output and archive are defined
(referencing the variables output and archive) to attach and propagate the error
to the Promise rejection and perform any necessary cleanup.

Comment on lines +1990 to +2003
const addDirectoryToArchive = (dirPath, archivePath) => {
const entries = fs.readdirSync(dirPath, { withFileTypes: true });

for (const entry of entries) {
const fullPath = path.join(dirPath, entry.name);
const entryArchivePath = archivePath ? path.join(archivePath, entry.name) : entry.name;

if (entry.isDirectory()) {
if (!ignoredDirectories.includes(entry.name)) {
addDirectoryToArchive(fullPath, entryArchivePath);
}
} else {
archive.file(fullPath, { name: entryArchivePath });
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and its size
wc -l packages/bruno-electron/src/ipc/collection.js

Repository: usebruno/bruno

Length of output: 108


🏁 Script executed:

# Read the relevant section (lines 1957-2015)
sed -n '1957,2015p' packages/bruno-electron/src/ipc/collection.js

Repository: usebruno/bruno

Length of output: 1940


🏁 Script executed:

# Also check around line 1990 for context
sed -n '1980,2010p' packages/bruno-electron/src/ipc/collection.js

Repository: usebruno/bruno

Length of output: 977


🏁 Script executed:

# Verify Node.js Dirent.isSymbolicLink() availability
node --version
cat <<'EOF' | node
const fs = require('fs');
const dirent = fs.readdirSync('.', { withFileTypes: true })[0];
console.log('isSymbolicLink available:', typeof dirent.isSymbolicLink === 'function');
EOF

Repository: usebruno/bruno

Length of output: 96


🏁 Script executed:

# Check if there's any symlink-related handling elsewhere in the file
rg -i "symlink|isSymbolic" packages/bruno-electron/src/ipc/collection.js

Repository: usebruno/bruno

Length of output: 40


Block symlinks to prevent exporting files outside the collection.

The addDirectoryToArchive function archives all non-directories without checking for symlinks. A symlink inside the collection can point to files outside it, allowing unintended data to be included in the export.

Proposed fix
            if (entry.isDirectory()) {
              if (!ignoredDirectories.includes(entry.name)) {
                addDirectoryToArchive(fullPath, entryArchivePath);
              }
+           } else if (entry.isSymbolicLink()) {
+             // Skip symlinks to prevent path traversal
+             continue;
            } else {
              archive.file(fullPath, { name: entryArchivePath });
            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const addDirectoryToArchive = (dirPath, archivePath) => {
const entries = fs.readdirSync(dirPath, { withFileTypes: true });
for (const entry of entries) {
const fullPath = path.join(dirPath, entry.name);
const entryArchivePath = archivePath ? path.join(archivePath, entry.name) : entry.name;
if (entry.isDirectory()) {
if (!ignoredDirectories.includes(entry.name)) {
addDirectoryToArchive(fullPath, entryArchivePath);
}
} else {
archive.file(fullPath, { name: entryArchivePath });
}
const addDirectoryToArchive = (dirPath, archivePath) => {
const entries = fs.readdirSync(dirPath, { withFileTypes: true });
for (const entry of entries) {
const fullPath = path.join(dirPath, entry.name);
const entryArchivePath = archivePath ? path.join(archivePath, entry.name) : entry.name;
if (entry.isDirectory()) {
if (!ignoredDirectories.includes(entry.name)) {
addDirectoryToArchive(fullPath, entryArchivePath);
}
} else if (entry.isSymbolicLink()) {
// Skip symlinks to prevent path traversal
continue;
} else {
archive.file(fullPath, { name: entryArchivePath });
}
🤖 Prompt for AI Agents
In `@packages/bruno-electron/src/ipc/collection.js` around lines 1990 - 2003, The
addDirectoryToArchive function currently archives regular entries without
checking for symlinks, which can cause files outside the collection to be
included; update addDirectoryToArchive to skip any symbolic links: before
recursing or calling archive.file, detect symlinks (use the
Dirent.isSymbolicLink() from entries or call
fs.lstatSync(fullPath).isSymbolicLink() for certainty) and continue/skip if
true, preserving the existing ignoredDirectories logic and using the same
fullPath and entryArchivePath variables so no symlink targets are followed or
archived.

Comment on lines +1990 to +2008
const addDirectoryToArchive = (dirPath, archivePath) => {
const entries = fs.readdirSync(dirPath, { withFileTypes: true });

for (const entry of entries) {
const fullPath = path.join(dirPath, entry.name);
const entryArchivePath = archivePath ? path.join(archivePath, entry.name) : entry.name;

if (entry.isDirectory()) {
if (!ignoredDirectories.includes(entry.name)) {
addDirectoryToArchive(fullPath, entryArchivePath);
}
} else {
archive.file(fullPath, { name: entryArchivePath });
}
}
};

addDirectoryToArchive(collectionPath, '');
archive.finalize();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n packages/bruno-electron/src/ipc/collection.js | head -2050 | tail -100

Repository: usebruno/bruno

Length of output: 3755


🏁 Script executed:

cat -n packages/bruno-electron/src/ipc/collection.js | sed -n '1950,2050p'

Repository: usebruno/bruno

Length of output: 3789


🏁 Script executed:

# Check what ipcMain handler this is within
rg -B 20 "addDirectoryToArchive" packages/bruno-electron/src/ipc/collection.js | head -50

Repository: usebruno/bruno

Length of output: 1261


🏁 Script executed:

# Check the archiver package capabilities
fd package.json | xargs grep -l archiver

Repository: usebruno/bruno

Length of output: 94


🏁 Script executed:

# Check archiver version and documentation
cat packages/bruno-electron/package.json | jq '.dependencies.archiver'

Repository: usebruno/bruno

Length of output: 66


🏁 Script executed:

# Look for archiver usage elsewhere in the codebase to see if directory() is used
rg "archive\.directory" packages/bruno-electron/

Repository: usebruno/bruno

Length of output: 40


🏁 Script executed:

# Check archiver documentation/types for directory method capabilities
fd -e d.ts -e ts | xargs rg -l "archiver" | head -5

Repository: usebruno/bruno

Length of output: 40


🌐 Web query:

archiver npm javascript directory method ignore patterns

💡 Result:

archiver’s archive.directory(dirpath, destpath, data) API does not expose ignore-pattern controls (the data argument is “entry data” like prefix, date, etc., not ignore rules). To ignore files/directories by pattern, use archive.glob() instead. [1]

Use glob() with ignore / skip

archive.glob(pattern, options, data) passes options through to readdir-glob, which supports:

  • ignore: exclude specific file/folder matches (but it may still traverse into matched folders) [2]
  • skip: exclude folders and don’t traverse them (prevents children from being included) [2]

Example:

archive.glob('**/*', {
  cwd: srcDir,
  dot: true, // include dotfiles
  ignore: ['**/*.map', '**/*.log', '**/.DS_Store'],
  skip: ['node_modules/**', '.git/**', 'dist/**']
});

Notes on patterns:

  • To exclude an entire directory tree, use someDir/** (globstar). [3]

Sources: Archiver API docs (directory vs glob) [1], readdir-glob options (ignore, skip) [2], glob ignore example guidance [3].


Use asynchronous directory traversal or archive.glob() with ignore patterns.

The recursive fs.readdirSync at line 1991 blocks the main process thread. Replace with archive.glob('**/*', { cwd: collectionPath, skip: ['node_modules/**', '.git/**'] }) to leverage archiver's native glob support with directory skipping, or use async traversal with fs.promises.readdir.

🤖 Prompt for AI Agents
In `@packages/bruno-electron/src/ipc/collection.js` around lines 1990 - 2008, The
synchronous recursive traversal in addDirectoryToArchive blocks the main thread;
replace it by using archiver's glob support or an async traversal: remove the
addDirectoryToArchive implementation and call archive.glob('**/*', { cwd:
collectionPath, ignore: ['node_modules/**', '.git/**',
...ignoredDirectories.map(d => d + '/**')] }) so archiver handles files and
skips ignoredDirectories, then call archive.finalize(); alternatively, if you
prefer async traversal, rewrite addDirectoryToArchive to use fs.promises.readdir
with withFileTypes and await recursion, calling archive.file for files—ensure
you reference addDirectoryToArchive, archive, collectionPath, and
ignoredDirectories when making the change.

@bijin-bruno bijin-bruno merged commit 5904c36 into usebruno:main Feb 2, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants