Skip to content

fix:sanitize item names before appending file extension on import#8743

Open
adwait-bruno wants to merge 1 commit into
usebruno:mainfrom
adwait-bruno:fix/filename-bru
Open

fix:sanitize item names before appending file extension on import#8743
adwait-bruno wants to merge 1 commit into
usebruno:mainfrom
adwait-bruno:fix/filename-bru

Conversation

@adwait-bruno

@adwait-bruno adwait-bruno commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description

REF:BRU-3250

Cause:Filenames were built by appending the extension and then running the results through sanitizeName(),which stripped trailing dots
Fix:Sanitize the name first and then append the extension

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

  • Bug Fixes
    • Improved collection import and export filename handling.
    • Request and environment names are now sanitized consistently while preserving their file extensions.
    • Removed trailing periods from generated filenames before adding .bru, .yml, or .js extensions.
    • Ensured exported YAML files consistently receive the correct .yml extension.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Collection export and import filename generation now sanitize base names separately from file extensions for request, JavaScript, and environment files. CLI tests cover trailing-period names in both BRU and YML output formats.

Changes

Filename normalization

Layer / File(s) Summary
CLI export filename normalization
packages/bruno-cli/src/utils/collection.js, packages/bruno-cli/tests/utils/collection/create-collection-from-bruno-object.spec.js
CLI exports sanitize request and environment names before appending .bru or .yml, with tests covering trailing-period names.
Electron import helper normalization
packages/bruno-electron/src/utils/collection-import.js
Default request, JavaScript, and environment filenames sanitize only the base name before appending their extensions.
Electron import wiring
packages/bruno-electron/src/ipc/collection.js
Import filename handling applies sanitization consistently and avoids sanitizing appended .js and environment extensions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: bijin-bruno

Poem

Names shed stray dots in flight,
Extensions stay crisp and bright.
BRU and YML files align,
Electron follows the same design.
Clean filenames cross the line.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: sanitizing imported item names before appending file extensions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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-electron/src/utils/collection-import.js`:
- Around line 41-44: Use one shared collision-resolution strategy across both
Electron import flows. In collection-import.js, uniquify sanitized request,
JavaScript, and environment filenames before constructing their paths; in
collection.js, reserve unique request, JavaScript, and environment paths before
asynchronous writes so Promise.all cannot select duplicate destinations.
Preserve the intended extensions and ensure each generated path is unique within
the import operation.
🪄 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 Plus

Run ID: 2a8a5c79-a416-41bb-bec3-16deaac69212

📥 Commits

Reviewing files that changed from the base of the PR and between ff8e988 and 49b5416.

📒 Files selected for processing (4)
  • packages/bruno-cli/src/utils/collection.js
  • packages/bruno-cli/tests/utils/collection/create-collection-from-bruno-object.spec.js
  • packages/bruno-electron/src/ipc/collection.js
  • packages/bruno-electron/src/utils/collection-import.js

Comment on lines +41 to 44
let sanitizedFilename = item.filename ? sanitizeName(item.filename) : `${sanitizeName(item.name)}.${format}`;
const content = await stringifyRequestViaWorker(item, { format });
const filePath = path.join(currentPath, sanitizedFilename);
safeWriteFileSync(filePath, content);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Use one collision-resolution strategy across both Electron import flows.

Sanitization intentionally causes distinct names to converge, but neither flow reserves or uniquifies the resulting paths. This can overwrite imported data or fail imports; packages/bruno-electron/src/ipc/collection.js is additionally nondeterministic because request and JavaScript writes use Promise.all.

  • packages/bruno-electron/src/utils/collection-import.js Lines 41-44: uniquify sanitized request filenames.
  • packages/bruno-electron/src/utils/collection-import.js Lines 64-67: uniquify sanitized JavaScript filenames.
  • packages/bruno-electron/src/utils/collection-import.js Lines 79-82: uniquify sanitized environment filenames.
  • packages/bruno-electron/src/ipc/collection.js Lines 1344-1354: reserve unique request paths before writing.
  • packages/bruno-electron/src/ipc/collection.js Lines 1374-1376: reserve unique JavaScript paths before writing.
  • packages/bruno-electron/src/ipc/collection.js Lines 1389-1391: reserve unique environment paths before writing.
📍 Affects 2 files
  • packages/bruno-electron/src/utils/collection-import.js#L41-L44 (this comment)
  • packages/bruno-electron/src/utils/collection-import.js#L64-L67
  • packages/bruno-electron/src/utils/collection-import.js#L79-L82
  • packages/bruno-electron/src/ipc/collection.js#L1344-L1354
  • packages/bruno-electron/src/ipc/collection.js#L1374-L1376
  • packages/bruno-electron/src/ipc/collection.js#L1389-L1391
🤖 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/utils/collection-import.js` around lines 41 - 44,
Use one shared collision-resolution strategy across both Electron import flows.
In collection-import.js, uniquify sanitized request, JavaScript, and environment
filenames before constructing their paths; in collection.js, reserve unique
request, JavaScript, and environment paths before asynchronous writes so
Promise.all cannot select duplicate destinations. Preserve the intended
extensions and ensure each generated path is unique within the import operation.

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.

1 participant