fix: quote values containing hash (#) in .env file serialization#7380
Open
chirag-bruno wants to merge 2 commits intousebruno:mainfrom
Open
fix: quote values containing hash (#) in .env file serialization#7380chirag-bruno wants to merge 2 commits intousebruno:mainfrom
chirag-bruno wants to merge 2 commits 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
WalkthroughReplaces ad-hoc .env serialization with a new shared Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Renderer (App)
participant IPC as Electron IPC
participant Util as `@usebruno/common.jsonToDotenv`
participant FS as Filesystem
UI->>IPC: save-dotenv-variables(variables)
IPC->>Util: jsonToDotenv(variables)
Util-->>IPC: dotenvContent
IPC->>FS: writeFile(path, dotenvContent)
FS-->>IPC: writeResult
IPC-->>UI: result / error
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
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/bruno-common/src/utils/jsonToDotenv.ts (2)
28-35: Consider quoting values with leading/trailing whitespace.Values with leading or trailing whitespace (e.g.,
" password ") will serialize asKEY= passwordand round-trip back as"password"(stripped by dotenv parser). This is an edge case but could cause data loss.♻️ Proposed fix
const value = v.value || ''; // If value contains special characters, wrap in quotes - if (value.includes('\n') || value.includes('\r') || value.includes('"') || value.includes('\'') || value.includes('\\') || value.includes('#')) { + const needsQuoting = value.includes('\n') || value.includes('\r') || value.includes('"') || value.includes('\'') || value.includes('\\') || value.includes('#') || value !== value.trim(); + if (needsQuoting) {🤖 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 28 - 35, The serializer in jsonToDotenv.ts uses the local variable value (from v.value) and currently only quotes when it contains special chars; extend the quoting condition to also quote when value has leading or trailing whitespace (e.g., detect with value.startsWith(' ') || value.endsWith(' ') or a regex /^\s|\s$/) so such values are wrapped in quotes and escaped the same way as other quoted values; update the conditional that builds escapedValue (the block referencing v.name and escapedValue) to trigger for this new whitespace case so round-tripping via dotenv preserves leading/trailing spaces.
16-16: Minor documentation inaccuracy.Single quotes don't actually get escaped in the implementation (line 32) — they just trigger double-quoting of the value. Consider rewording to "single quotes ('): must be enclosed in double quotes" for accuracy.
📝 Suggested doc fix
- * - single quotes ('): need escaping + * - single quotes ('): must be enclosed in double quotes🤖 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` at line 16, Update the documentation string in the jsonToDotenv implementation to accurately describe handling of single quotes: change the phrase "single quotes ('): need escaping" to "single quotes ('): must be enclosed in double quotes" (or similar) so it reflects that the code double-quotes values containing single quotes rather than escaping them; locate the comment near the jsonToDotenv function/implementation and adjust the wording accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/bruno-common/src/utils/jsonToDotenv.ts`:
- Around line 28-35: The serializer in jsonToDotenv.ts uses the local variable
value (from v.value) and currently only quotes when it contains special chars;
extend the quoting condition to also quote when value has leading or trailing
whitespace (e.g., detect with value.startsWith(' ') || value.endsWith(' ') or a
regex /^\s|\s$/) so such values are wrapped in quotes and escaped the same way
as other quoted values; update the conditional that builds escapedValue (the
block referencing v.name and escapedValue) to trigger for this new whitespace
case so round-tripping via dotenv preserves leading/trailing spaces.
- Line 16: Update the documentation string in the jsonToDotenv implementation to
accurately describe handling of single quotes: change the phrase "single quotes
('): need escaping" to "single quotes ('): must be enclosed in double quotes"
(or similar) so it reflects that the code double-quotes values containing single
quotes rather than escaping them; locate the comment near the jsonToDotenv
function/implementation and adjust the wording accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d5d4f6e-c2dc-4ee9-a4d6-015663f587cc
📒 Files selected for processing (2)
packages/bruno-common/src/utils/jsonToDotenv.spec.tspackages/bruno-common/src/utils/jsonToDotenv.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-common/src/utils/jsonToDotenv.spec.ts
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