fix: quote values containing hash (#) in .env file serialization#7380
Open
chirag-bruno wants to merge 1 commit intousebruno:mainfrom
Open
fix: quote values containing hash (#) in .env file serialization#7380chirag-bruno wants to merge 1 commit intousebruno:mainfrom
chirag-bruno wants to merge 1 commit intousebruno:mainfrom
Conversation
Values containing # characters were being truncated when saved to .env files because the dotenv parser interprets # as a comment character. This fix adds a shared jsonToDotenv utility in bruno-common that properly quotes values containing special characters (#, \n, ", ', \) to ensure they are preserved through serialization and parsing. - Add jsonToDotenv utility with comprehensive test coverage - Update bruno-electron and bruno-app to use shared utility - Remove duplicate serialization logic from multiple locations Fixes usebruno#7375 Fixes usebruno#7327
chirag-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
WalkthroughThe PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/bruno-common/src/utils/jsonToDotenv.spec.ts (1)
50-83: Add a CRLF (\r\n) serialization test case.The suite covers
\n, but not Windows-style CRLF input, which is the key cross-platform edge here.Suggested test addition
describe('special character handling', () => { + test('it should quote values containing CRLF newlines and escape them', () => { + const variables = [{ name: 'MULTILINE_CRLF', value: 'line1\r\nline2' }]; + const output = jsonToDotenv(variables); + expect(output).toBe('MULTILINE_CRLF="line1\\r\\nline2"'); + }); + test('it should quote values containing hash (#)', () => { const variables = [ { name: 'PASSWORD', value: 'ABC#DEF' },Based on learnings "Cover both the 'happy path' and the realistically problematic paths. Validate expected success behaviour, but also validate error handling, edge cases, and degraded-mode behaviour".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-common/src/utils/jsonToDotenv.spec.ts` around lines 50 - 83, Add a test to the special character handling suite that passes a variable with a CRLF line break to jsonToDotenv and asserts it is serialized with escaped CR and LF sequences; e.g. create variables = [{ name: 'MULTILINE_CRLF', value: 'line1\r\nline2' }], call jsonToDotenv(variables) and expect the output toBe('MULTILINE_CRLF="line1\\r\\nline2"') so CRLF inputs are correctly escaped to "\\r\\n".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-common/src/utils/jsonToDotenv.ts`:
- Around line 29-32: The serializer currently escapes backslashes, double
quotes, and newlines but leaves carriage returns raw, which breaks Windows CRLF
handling; update the escaping in the jsonToDotenv serialization (where `value`
is transformed into `escapedValue` and used to build `${v.name}="..."`) to also
replace `\r` (e.g., add a replace for `\r` that yields `\\r`) and perform it
after escaping backslashes but before escaping `\n` so CRLF (`\r\n`) becomes
`\\r\\n`, ensuring consistent cross-platform line ending serialization.
---
Nitpick comments:
In `@packages/bruno-common/src/utils/jsonToDotenv.spec.ts`:
- Around line 50-83: Add a test to the special character handling suite that
passes a variable with a CRLF line break to jsonToDotenv and asserts it is
serialized with escaped CR and LF sequences; e.g. create variables = [{ name:
'MULTILINE_CRLF', value: 'line1\r\nline2' }], call jsonToDotenv(variables) and
expect the output toBe('MULTILINE_CRLF="line1\\r\\nline2"') so CRLF inputs are
correctly escaped to "\\r\\n".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f67422b-099e-4c1c-b3dc-6b709e56c5bc
📒 Files selected for processing (6)
packages/bruno-app/src/components/Environments/DotEnvFileEditor/utils.jspackages/bruno-common/src/utils/index.tspackages/bruno-common/src/utils/jsonToDotenv.spec.tspackages/bruno-common/src/utils/jsonToDotenv.tspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/global-environments.js
Comment on lines
+29
to
+32
| if (value.includes('\n') || value.includes('"') || value.includes('\'') || value.includes('\\') || value.includes('#')) { | ||
| // Escape backslashes first, then double quotes, then newlines | ||
| const escapedValue = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); | ||
| return `${v.name}="${escapedValue}"`; |
Contributor
There was a problem hiding this comment.
Handle CRLF safely in serialization.
At Line 31, \n is escaped but \r is left raw. Values containing Windows line endings (\r\n) can be serialized inconsistently across platforms.
Proposed fix
- if (value.includes('\n') || value.includes('"') || value.includes('\'') || value.includes('\\') || value.includes('#')) {
+ if (value.includes('\n') || value.includes('\r') || value.includes('"') || value.includes('\'') || value.includes('\\') || value.includes('#')) {
// Escape backslashes first, then double quotes, then newlines
- const escapedValue = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n');
+ const escapedValue = value
+ .replace(/\\/g, '\\\\')
+ .replace(/"/g, '\\"')
+ .replace(/\r/g, '\\r')
+ .replace(/\n/g, '\\n');
return `${v.name}="${escapedValue}"`;
}As per coding guidelines "Line endings should be handled consistently (be aware of CRLF vs LF issues)".
📝 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
| if (value.includes('\n') || value.includes('"') || value.includes('\'') || value.includes('\\') || value.includes('#')) { | |
| // Escape backslashes first, then double quotes, then newlines | |
| const escapedValue = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); | |
| return `${v.name}="${escapedValue}"`; | |
| if (value.includes('\n') || value.includes('\r') || value.includes('"') || value.includes('\'') || value.includes('\\') || value.includes('#')) { | |
| // Escape backslashes first, then double quotes, then newlines | |
| const escapedValue = value | |
| .replace(/\\/g, '\\\\') | |
| .replace(/"/g, '\\"') | |
| .replace(/\r/g, '\\r') | |
| .replace(/\n/g, '\\n'); | |
| return `${v.name}="${escapedValue}"`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-common/src/utils/jsonToDotenv.ts` around lines 29 - 32, The
serializer currently escapes backslashes, double quotes, and newlines but leaves
carriage returns raw, which breaks Windows CRLF handling; update the escaping in
the jsonToDotenv serialization (where `value` is transformed into `escapedValue`
and used to build `${v.name}="..."`) to also replace `\r` (e.g., add a replace
for `\r` that yields `\\r`) and perform it after escaping backslashes but before
escaping `\n` so CRLF (`\r\n`) becomes `\\r\\n`, ensuring consistent
cross-platform line ending serialization.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Values containing # characters were being truncated when saved to .env files because the dotenv parser interprets # as a comment character.
This fix adds a shared jsonToDotenv utility in bruno-common that properly quotes values containing special characters (#, \n, ", ', ) to ensure they are preserved through serialization and parsing.
Fixes #7375 and #7327
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
Refactor
Tests