Chore/relock#7606
Conversation
laurac8r
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds dependency upgrades and overrides, replaces rollup-plugin-terser imports, migrates faker image calls to Picsum, tweaks gRPC float generation, introduces an idempotent security patcher script with tests, and wires the patcher into setup/install sequencing. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant Setup as scripts/setup.js
participant Patcher as scripts/patch-security.js
participant FS as Repository FS
participant NPM as npm
Dev->>Setup: run setup()
Setup->>FS: remove node_modules
Setup->>Patcher: patchSecurityVulnerabilities(rootDir)
Patcher->>FS: read root package.json & workspace package.json files
Patcher->>FS: update root devDeps (force-install pins)
Patcher->>FS: update workspace package.json (replace devDeps), patch JS files (require(...) rewrites)
Patcher-->>Setup: return success
Setup->>NPM: npm install
NPM-->>Setup: install complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates dependency versions across the Bruno monorepo (“relock”), including a major @faker-js/faker upgrade and related small code adjustments to keep sample/mock generation working.
Changes:
- Bump
@faker-js/fakerto^10.4.0across multiple workspaces and update faker API usage where needed. - Upgrade build tooling/deps (notably
rollupto3.30.0, plus updates likemarkdown-it,ajv,electron-builder, AWS SDK). - Adjust mock image generation in
bruno-commonto use Picsum-based URLs.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bruno-tests/package.json | Updates testbench dependencies (e.g., body-parser, fast-xml-parser). |
| packages/bruno-tests/collection_oauth2/package.json | Bumps faker used by the OAuth2 test collection. |
| packages/bruno-tests/collection_level_oauth2/package.json | Bumps faker used by the level OAuth2 test collection. |
| packages/bruno-tests/collection/package.json | Bumps faker used by the main test collection. |
| packages/bruno-requests/src/grpc/grpcMessageGenerator.js | Updates faker float API option for sample gRPC message generation. |
| packages/bruno-requests/package.json | Bumps faker + Rollup version for requests package. |
| packages/bruno-query/package.json | Bumps Rollup version (and its local override) for query package. |
| packages/bruno-js/package.json | Updates ajv and Rollup version for bruno-js package. |
| packages/bruno-graphql-docs/package.json | Upgrades markdown-it and Rollup override for graphql-docs package. |
| packages/bruno-filestore/package.json | Bumps Rollup version for filestore package. |
| packages/bruno-electron/package.json | Updates AWS credential providers + electron-builder. |
| packages/bruno-converters/package.json | Bumps Rollup version for converters package. |
| packages/bruno-common/src/utils/faker-functions.ts | Replaces category-based LoremFlickr URLs with Picsum URLs for image mock functions. |
| packages/bruno-common/package.json | Bumps faker + Rollup version for common package. |
| packages/bruno-cli/package.json | Updates AWS credential providers for CLI package. |
| packages/bruno-app/package.json | Updates app dependencies (e.g., jsonlint, markdown-it, rsbuild plugins). |
| package.json | Updates root faker and adds/updates dependency overrides (rollup/ajv/qs). |
Comments suppressed due to low confidence (4)
packages/bruno-common/package.json:60
rollupwas bumped to 3.30.0 in devDependencies, but this package’soverrides.rollupstill pins 3.29.5. That override will typically force the older Rollup version and can negate the intended upgrade; update/remove the override so it aligns with the new Rollup version.
"rollup": "3.30.0",
"rollup-plugin-dts": "^5.0.0",
"rollup-plugin-peer-deps-external": "^2.2.4",
"rollup-plugin-terser": "^7.0.2",
"typescript": "^5.8.3"
},
"overrides": {
"rollup": "3.29.5"
}
packages/bruno-converters/package.json:48
rollupwas updated to 3.30.0, butoverrides.rollupstill pins 3.29.5. This will commonly force the old Rollup version and make builds inconsistent; update/remove the override to match 3.30.0.
"rollup": "3.30.0",
"rollup-plugin-dts": "^5.0.0",
"rollup-plugin-peer-deps-external": "^2.2.4",
"rollup-plugin-terser": "^7.0.2",
"typescript": "^4.8.4"
},
"overrides": {
"rollup": "3.29.5"
}
packages/bruno-filestore/package.json:45
rollupwas updated to 3.30.0, butoverrides.rollupstill pins 3.29.5. This override can force the old Rollup version and undermine the upgrade; update/remove the override so it matches 3.30.0.
"rollup": "3.30.0",
"rollup-plugin-dts": "^5.0.0",
"rollup-plugin-peer-deps-external": "^2.2.4",
"rollup-plugin-terser": "^7.0.2",
"typescript": "^4.8.4"
},
"overrides": {
"rollup": "3.29.5"
},
packages/bruno-requests/package.json:60
rollupwas bumped to 3.30.0, but the package-leveloverrides.rollupstill pins 3.29.5. This can force an older Rollup than devDependencies declare; update/remove the override to align with 3.30.0.
"rollup": "3.30.0",
"rollup-plugin-dts": "^5.0.0",
"rollup-plugin-peer-deps-external": "^2.2.4",
"rollup-plugin-terser": "^7.0.2",
"typescript": "^4.8.4"
},
"overrides": {
"rollup": "3.29.5"
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| randomAbstractImage: () => faker.image.urlPicsumPhotos(), | ||
| randomAnimalsImage: () => faker.image.urlPicsumPhotos(), | ||
| randomBusinessImage: () => faker.image.urlPicsumPhotos(), | ||
| randomCatsImage: () => faker.image.urlPicsumPhotos(), | ||
| randomCityImage: () => faker.image.urlPicsumPhotos(), | ||
| randomFoodImage: () => faker.image.urlPicsumPhotos(), | ||
| randomNightlifeImage: () => faker.image.urlPicsumPhotos(), | ||
| randomFashionImage: () => faker.image.urlPicsumPhotos(), | ||
| randomPeopleImage: () => faker.image.urlPicsumPhotos(), | ||
| randomNatureImage: () => faker.image.urlPicsumPhotos(), | ||
| randomSportsImage: () => faker.image.urlPicsumPhotos(), | ||
| randomTransportImage: () => faker.image.urlPicsumPhotos(), |
There was a problem hiding this comment.
These functions are still named as category-specific image generators (e.g., randomCatsImage, randomFoodImage), but they now all return the same generic urlPicsumPhotos() output. This makes the API misleading; either restore category-specific behavior (e.g., by constructing category URLs) or rename/adjust documentation so consumers don’t expect category-specific images.
| randomAbstractImage: () => faker.image.urlPicsumPhotos(), | |
| randomAnimalsImage: () => faker.image.urlPicsumPhotos(), | |
| randomBusinessImage: () => faker.image.urlPicsumPhotos(), | |
| randomCatsImage: () => faker.image.urlPicsumPhotos(), | |
| randomCityImage: () => faker.image.urlPicsumPhotos(), | |
| randomFoodImage: () => faker.image.urlPicsumPhotos(), | |
| randomNightlifeImage: () => faker.image.urlPicsumPhotos(), | |
| randomFashionImage: () => faker.image.urlPicsumPhotos(), | |
| randomPeopleImage: () => faker.image.urlPicsumPhotos(), | |
| randomNatureImage: () => faker.image.urlPicsumPhotos(), | |
| randomSportsImage: () => faker.image.urlPicsumPhotos(), | |
| randomTransportImage: () => faker.image.urlPicsumPhotos(), | |
| randomAbstractImage: () => faker.image.urlLoremFlickr({ category: 'abstract' }), | |
| randomAnimalsImage: () => faker.image.urlLoremFlickr({ category: 'animals' }), | |
| randomBusinessImage: () => faker.image.urlLoremFlickr({ category: 'business' }), | |
| randomCatsImage: () => faker.image.urlLoremFlickr({ category: 'cats' }), | |
| randomCityImage: () => faker.image.urlLoremFlickr({ category: 'city' }), | |
| randomFoodImage: () => faker.image.urlLoremFlickr({ category: 'food' }), | |
| randomNightlifeImage: () => faker.image.urlLoremFlickr({ category: 'nightlife' }), | |
| randomFashionImage: () => faker.image.urlLoremFlickr({ category: 'fashion' }), | |
| randomPeopleImage: () => faker.image.urlLoremFlickr({ category: 'people' }), | |
| randomNatureImage: () => faker.image.urlLoremFlickr({ category: 'nature' }), | |
| randomSportsImage: () => faker.image.urlLoremFlickr({ category: 'sports' }), | |
| randomTransportImage: () => faker.image.urlLoremFlickr({ category: 'transport' }), |
| "graphql": "^16.6.0", | ||
| "markdown-it": "^13.0.1", | ||
| "markdown-it": "^14.1.1", | ||
| "postcss": "8.4.47", | ||
| "react": "19.0.0", | ||
| "react-dom": "18.2.0", | ||
| "rollup":"3.29.5", | ||
| "rollup":"3.30.0", |
There was a problem hiding this comment.
markdown-it was upgraded to ^14.1.1, but peerDependencies.markdown-it still requires ^13.0.1. This creates an inconsistent contract for consumers and can lead to install warnings or incompatible runtime behavior; update the peer dependency range (or keep devDependency on v13) to reflect the supported major versions.
| "ajv": "^8.18.0", | ||
| "qs": "^6.15.0", |
There was a problem hiding this comment.
The new overrides entries for ajv and qs are ranges (^...). Overrides are typically used to force a specific version for reproducibility/security; using a range can allow unexpected future upgrades. Consider pinning exact versions here (consistent with the rollup override).
| "ajv": "^8.18.0", | |
| "qs": "^6.15.0", | |
| "ajv": "8.18.0", | |
| "qs": "6.15.0", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/package.json`:
- Line 52: The package.json shows a mismatch between devDependencies.rollup and
overrides.rollup (devDependencies.rollup is "3.30.0" while overrides.rollup is
"3.29.5"); update the overrides.rollup entry to "3.30.0" (or alternatively
revert devDependencies.rollup back to "3.29.5") so the override and
devDependency versions match and resolution is deterministic—edit the
overrides.rollup field to the chosen version to fix the inconsistency.
In `@packages/bruno-common/src/utils/faker-functions.ts`:
- Around line 46-57: The category-specific helpers (randomAbstractImage,
randomAnimalsImage, randomFoodImage, randomNightlifeImage, randomFashionImage,
randomPeopleImage, randomNatureImage, randomSportsImage, randomTransportImage,
randomBusinessImage, randomCatsImage, randomCityImage) all call the same
faker.image.urlPicsumPhotos and thus no longer match their names; update each
helper to use a category-aware image generator provided by your faker provider
(e.g., the provider's category-specific methods or URL parameters for category)
so the return value matches the helper name, or if the provider cannot produce
category images, rename these functions to a single generic helper (e.g.,
randomImage) and mark the old category-named helpers as deprecated wrappers that
forward to the generic one; ensure changes touch the functions listed above so
callers get correct semantics or clear deprecation.
In `@packages/bruno-converters/package.json`:
- Line 40: The package.json has mismatched rollup versions:
devDependencies.rollup is "3.30.0" while overrides.rollup stays "3.29.5", so the
override wins and makes the dev change a no-op; update the overrides entry
(overrides.rollup) to match the intended version "3.30.0" (or alternatively set
devDependencies.rollup to the override version if you prefer downgrading) so
both devDependencies.rollup and overrides.rollup are consistent across packages
(also check and update the same mismatch noted at the other locations
referenced).
In `@packages/bruno-electron/package.json`:
- Line 85: The electron-builder Windows signing config uses removed top-level
fields (win.sign and win.publisherName) which will break under v26; update the
package.json electron-builder config by moving any signing-related settings into
either win.signtoolOptions or win.azureSignOptions (or remove them if signing is
not required), e.g., replace win.sign and win.publisherName with the appropriate
nested structure under win.signtoolOptions or win.azureSignOptions and ensure
other Windows signing keys are renamed accordingly so the config parses under
electron-builder v26.
In `@packages/bruno-filestore/package.json`:
- Line 37: Update the inconsistent Rollup version so the override doesn't force
the older release: change the overrides entry for "rollup" (the overrides.rollup
key) to match devDependencies.rollup (3.30.0) or remove the overrides.rollup
entry entirely if it's no longer needed; ensure all occurrences (including the
entries referenced around lines 43-45) are updated so the project uses rollup
3.30.0 consistently.
In `@packages/bruno-graphql-docs/package.json`:
- Line 22: The package.json lists devDependencies.markdown-it as ^14.1.1 but
peerDependencies.markdown-it is still ^13.0.1; update
peerDependencies.markdown-it to ^14.1.1 (or broaden to a compatible range that
includes 14.1.1) so consumers are expected to provide the same major/minor
version you're developing against, ensuring the peerDependencies entry is
consistent with devDependencies.markdown-it.
In `@packages/bruno-requests/package.json`:
- Line 52: The package.json has a Rollup version mismatch between
devDependencies.rollup and overrides.rollup; pick one canonical version used
across the repo and make them consistent (e.g., set devDependencies.rollup to
"3.29.5" to match overrides, or update overrides.rollup to "3.30.0" if you
intend to move to 3.30.0) and apply the same change to the other occurrences
noted (the entries around the overrides block) so the package's
devDependencies.rollup and overrides.rollup are identical.
🪄 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: 0dc5fe52-1f35-4857-9766-db6ad0f7cb68
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
package.jsonpackages/bruno-app/package.jsonpackages/bruno-cli/package.jsonpackages/bruno-common/package.jsonpackages/bruno-common/src/utils/faker-functions.tspackages/bruno-converters/package.jsonpackages/bruno-electron/package.jsonpackages/bruno-filestore/package.jsonpackages/bruno-graphql-docs/package.jsonpackages/bruno-js/package.jsonpackages/bruno-query/package.jsonpackages/bruno-requests/package.jsonpackages/bruno-requests/src/grpc/grpcMessageGenerator.jspackages/bruno-tests/collection/package.jsonpackages/bruno-tests/collection_level_oauth2/package.jsonpackages/bruno-tests/collection_oauth2/package.jsonpackages/bruno-tests/package.json
| "devDependencies": { | ||
| "electron": "~37.6.1", | ||
| "electron-builder": "^24.13.3", | ||
| "electron-builder": "^26.8.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
electron-builder v26 breaking changes migration
💡 Result:
electron-builder v26 (released January 2025 as the major version after v25) introduced several significant changes, primarily around dependency collection, packaging, and platform-specific improvements. While not explicitly labeled as "breaking changes" in the official changelog, early v26.x releases (starting ~v26.0.4) had regressions that broke builds for some users, particularly around native module handling and node_modules collection. These were fixed in subsequent patches. Key changes in v26.0.0 (from GitHub PR #8808): Major: - Removed HFS+ DMG support on non-arm64 macOS (macOS 15.2 sunset). - Removed deprecated fields from winOptions/macOptions. - Migrated to official electron/asar packaging. - Migrated electronDist to a build Hook. - Other feats: pacman autoupdates, S3 forcePathStyle, pwsh detection for Azure, etc. Notable regressions (fixed in later patches): - v26.0.4: Builds broken for projects with native deps (e.g., Rust/C); fixed in v26.0.5+ via PR #8845 (Issue #8842). - v26.0.12: node-module-collector issues on Windows (assumes .cmd extensions, shell execution problems, Yarn npm fallback); fixed in v26.0.13+ and v26.0.17 (PR #9026, Issue #9020). - v26.3.1-v26.4.0: Bloated app.asar/node_modules size (e.g., 35->294 folders, 36MB->348MB); fixed in later releases like v26.7.0 via duplicate dependency resolution (PR #9548, Issue #9532). - v26.0.0: TypeScript type errors for consumers (skipLibCheck=false); fixed in v26.0.1 (Issue #8812). Migration steps: 1. Update to latest v26.x (as of 2026-03-29: v26.8.2). Run npm install electron-builder@latest --save-dev or equivalent. 2. For Yarn 3+: Ensure .yarnrc.yml has nodeLinker: "node-modules" (required for native deps). 3. Run electron-builder install-app-deps before building to handle native modules correctly. 4. Check/update config for removed deprecated options (e.g., winOptions signing -> win.signtoolOptions). 5. Test builds on all targets; monitor app size and native module inclusion (e.g., sharp). 6. If using workspaces or monorepos, verify node_modules collection (improved in v26). 7. Official docs unchanged: https://www.electron.build/configuration.html. No dedicated migration guide, but review your package.json "build" section. Most issues were patched quickly; v26 is now stable with better Yarn/pnpm/corepack support, smaller bundles, and improved collectors.
Citations:
- 1: chore(deploy): Release GA - v26.0.0 (electron-updater v6.4.0) electron-userland/electron-builder#8808
- 2: https://github.com/electron-userland/electron-builder/blob/master/packages/electron-builder/CHANGELOG.md
- 3: https://www.electron.build/electron-builder/index.html
- 4: https://www.electron.build/index.html
- 5: https://www.electron.build/configuration.html
- 6: https://github.com/electron-userland/electron-builder
- 7: builds broken since v26.0.4 electron-userland/electron-builder#8842
- 8: v26.0.12 breaks build in multiple ways electron-userland/electron-builder#9020
- 9: Installer size drastically increased between 26.0.10 and 26.4.0 electron-userland/electron-builder#9532
- 10: Broken types since v26 electron-userland/electron-builder#8812
🏁 Script executed:
find . -name "*electron-builder*" -type f | grep -E "\.(js|json|config)" | head -20Repository: usebruno/bruno
Length of output: 110
🏁 Script executed:
cat -n ./packages/bruno-electron/electron-builder-config.jsRepository: usebruno/bruno
Length of output: 3305
🌐 Web query:
electron-builder v26 removed winOptions sign deprecated
💡 Result:
In electron-builder v26.0.0 (released January 2025), deprecated signing configuration fields were removed from the top-level win options object as a breaking change. Previously available signing options like certificateFile, certificatePassword, certificateSubjectName, publisherName, signingHashAlgorithms, etc., which were directly under win (sometimes referred to as winOptions), have been removed. These options have been restructured: - Standard signtool.exe signing configuration is now nested under win.signtoolOptions. - Azure Trusted Signing configuration is under win.azureSignOptions. This cleanup was done to support separate Azure signing configuration without bloating the win object and to remove long-deprecated fields. The change was implemented in PR #8582 (merged Jan 13, 2025) and included in the v26.0.0 GA release (PR #8808). Migration example: "build": { "win": { "signtoolOptions": { "certificateSubjectName": "Your Subject Name", "publisherName": "Your Publisher Name", "signingHashAlgorithms": ["sha256"] // other signtool options } } } Official documentation confirms win.signtoolOptions as the current path for Windows signing configuration using signtool.exe. Configurations using the old direct win signing fields will fail in v26+ and must be updated to the new nested structure. Latest version as of 2026-03-29 is v26.8.2.
Citations:
- 1: chore(deploy): Release GA - v26.0.0 (electron-updater v6.4.0) electron-userland/electron-builder#8808
- 2: chore: remove deprecated fields from
winOptionsandmacOptionsto clean up configuration (BREAKING CHANGE) electron-userland/electron-builder#8582 - 3: https://www.electron.build/configuration/win
- 4: https://www.electron.build/app-builder-lib.Interface.WindowsSigntoolConfiguration.html
- 5: feat(win): implement Azure Trusted Signing electron-userland/electron-builder#8458
- 6: https://www.electron.build/win.html
- 7: https://www.electron.build/electron-builder.Interface.WindowsConfiguration.html
Review electron-builder v26 Windows signing config structure.
electron-builder v26 removed deprecated signing fields (sign, publisherName, certificateFile, etc.) from the top-level win object as a breaking change. These must now be nested under win.signtoolOptions or win.azureSignOptions. The config currently has win.sign: null and win.publisherName: 'Bruno Software Inc' which will not parse in v26. Update the Windows section to use the new structure, or verify if signing is required and configure accordingly. Other properties (mac.hardenedRuntime, mac.entitlements, nsis options) remain compatible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-electron/package.json` at line 85, The electron-builder
Windows signing config uses removed top-level fields (win.sign and
win.publisherName) which will break under v26; update the package.json
electron-builder config by moving any signing-related settings into either
win.signtoolOptions or win.azureSignOptions (or remove them if signing is not
required), e.g., replace win.sign and win.publisherName with the appropriate
nested structure under win.signtoolOptions or win.azureSignOptions and ensure
other Windows signing keys are renamed accordingly so the config parses under
electron-builder v26.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-query/rollup.config.js`:
- Line 5: The import string for the terser plugin uses double quotes; update the
require call for terser (the const terser = require("@rollup/plugin-terser");
statement) to use single quotes instead to match repo string conventions (e.g.
require('@rollup/plugin-terser')). Ensure only the quotation marks change and no
other code is modified.
🪄 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: 89bf30e5-cf52-4b8c-aee5-7b86f4fc01a3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
package.jsonpackages/bruno-cli/package.jsonpackages/bruno-common/rollup.config.jspackages/bruno-converters/package.jsonpackages/bruno-converters/rollup.config.jspackages/bruno-electron/package.jsonpackages/bruno-filestore/package.jsonpackages/bruno-filestore/rollup.config.jspackages/bruno-graphql-docs/package.jsonpackages/bruno-graphql-docs/rollup.config.jspackages/bruno-query/package.jsonpackages/bruno-query/rollup.config.jspackages/bruno-requests/package.jsonpackages/bruno-requests/rollup.config.js
✅ Files skipped from review due to trivial changes (3)
- packages/bruno-electron/package.json
- packages/bruno-graphql-docs/package.json
- packages/bruno-query/package.json
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/bruno-cli/package.json
- packages/bruno-filestore/package.json
- packages/bruno-converters/package.json
- packages/bruno-requests/package.json
- package.json
| const typescript = require("@rollup/plugin-typescript"); | ||
| const dts = require("rollup-plugin-dts"); | ||
| const { terser } = require("rollup-plugin-terser"); | ||
| const terser = require("@rollup/plugin-terser"); |
There was a problem hiding this comment.
Use single quotes for the import string.
Line 5 uses double quotes; this should be single quotes to match repo conventions.
Suggested fix
-const terser = require("@rollup/plugin-terser");
+const terser = require('@rollup/plugin-terser');As per coding guidelines: “Stick to single quotes for strings.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-query/rollup.config.js` at line 5, The import string for the
terser plugin uses double quotes; update the require call for terser (the const
terser = require("@rollup/plugin-terser"); statement) to use single quotes
instead to match repo string conventions (e.g.
require('@rollup/plugin-terser')). Ensure only the quotation marks change and no
other code is modified.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/setup.js (1)
90-94: Minor inconsistency: hardcoded emoji vsiconsobject.Line 91 uses a hardcoded
🔒emoji while the rest of the file uses theiconsobject pattern (lines 6-14). Consider adding asecurityorpatchicon to maintain consistency.♻️ Suggested fix for consistency
const icons = { clean: '🧹', delete: '🗑️', install: '📦', build: '🔨', success: '✅', error: '❌', - working: '⚡' + working: '⚡', + security: '🔒' };Then update the log statement:
// Patch known security vulnerabilities in package.json files before install - console.log(`\n🔒 Patching security vulnerabilities...`); + console.log(`\n${icons.security} Patching security vulnerabilities...`); patchSecurityVulnerabilities(); console.log(`${icons.success} Security patches applied`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/setup.js` around lines 90 - 94, Replace the hardcoded lock emoji in the setup logging with a value from the shared icons object to keep logging consistent; update the console.log that prints "🔒 Patching security vulnerabilities..." to use icons.security (or add icons.patch/icons.security to the icons object if it doesn't exist), and ensure subsequent success log uses icons.success as already done; modify the icons definition (where icons is declared) to include the new key and then change the call site that invokes patchSecurityVulnerabilities() to reference icons.security instead of the hardcoded emoji.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup.js`:
- Line 4: The import of patchSecurityVulnerabilities is unresolved: either add a
new module that exports function patchSecurityVulnerabilities or remove the
import and its invocation; to fix, create a scripts/patch-security.js that
exports an async function patchSecurityVulnerabilities (implementing the
intended vulnerability-patching logic or a no-op placeholder that logs and
returns) and ensure module.exports = { patchSecurityVulnerabilities }, or remove
the require('./patch-security') and the call to patchSecurityVulnerabilities()
from the setup script so there is no missing dependency.
---
Nitpick comments:
In `@scripts/setup.js`:
- Around line 90-94: Replace the hardcoded lock emoji in the setup logging with
a value from the shared icons object to keep logging consistent; update the
console.log that prints "🔒 Patching security vulnerabilities..." to use
icons.security (or add icons.patch/icons.security to the icons object if it
doesn't exist), and ensure subsequent success log uses icons.success as already
done; modify the icons definition (where icons is declared) to include the new
key and then change the call site that invokes patchSecurityVulnerabilities() to
reference icons.security instead of the hardcoded emoji.
🪄 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: ab6ae028-e9aa-4748-82a7-677756f77591
📒 Files selected for processing (1)
scripts/setup.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/patch-security.test.js (2)
32-34: Clean up temp directories inafterEach
afterEachis currently a no-op, so every test run leaves temp folders behind in OS temp storage.Suggested fix
+const createdTmpDirs = []; + function makeTmpDir() { - return fs.mkdtempSync(path.join(os.tmpdir(), 'bruno-patch-test-')); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'bruno-patch-test-')); + createdTmpDirs.push(dir); + return dir; } @@ afterEach(() => { - // cleanup handled by OS tmpdir + for (const dir of createdTmpDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } });Based on learnings: "Ensure tests are deterministic and reproducible. No randomness, timing dependencies, or environment-specific assumptions without explicit control."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/patch-security.test.js` around lines 32 - 34, The test suite's afterEach is a no-op and leaves temp folders behind; update scripts/patch-security.test.js to track any temp directories created during tests (e.g., push paths into a tempDirs array whenever tests call mkdtemp or create temp dirs) and implement afterEach(() => { tempDirs.forEach(d => fs.rmSync(d, { recursive: true, force: true })); tempDirs.length = 0; }); ensuring fs is imported and removal uses recursive+force to reliably clean up directories created by the tests (refer to the existing afterEach and any mkdtemp/temp dir helpers used in the file).
36-198: Add regression tests for workspace format edge casesPlease add cases for
workspaces: { packages: [...] }and glob entries (e.g.packages/*). These are high-value problematic paths for this script and currently untested.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 when appropriate."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/patch-security.test.js` around lines 36 - 198, Add two new tests to the existing patchSecurityVulnerabilities suite that exercise workspace-format edge cases: one where the root package.json uses the object form workspaces: { packages: ['packages/foo'] } and another where workspaces contains a glob like 'packages/*'; in each test create the matching package under packages/, write package.json and (when applicable) rollup.config.js with rollup-plugin-terser, invoke patchSecurityVulnerabilities(tmp), and assert the same outcomes as the existing tests (root overrides get security pins, package devDependencies replace rollup-plugin-terser with `@rollup/plugin-terser` and rollup.config.js require strings are updated, and `@aws-sdk/credential-providers` version is bumped where present) so these format variations are covered.scripts/patch-security.js (1)
45-54: Consider hardening workspace discovery for glob patterns and object format (not currently required)The current code works correctly with the repository's existing simple array format (
"workspaces": [...]). However, the function lacks support for technically valid npm workspace formats: glob patterns likepackages/*and object format ("workspaces": { "packages": [...] }). If workspace definitions ever transition to these formats, the function would silently skip or fail to discover packages.This is an optional improvement to prevent future issues if workspace configuration evolves, rather than a blocking bug for the current state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/patch-security.js` around lines 45 - 54, findWorkspacePackageJsons currently assumes root.workspaces is a simple array of literal paths; update it to handle both object format and glob patterns by: detect if readJson(root).workspaces is an object and pull the array from workspaces.packages (or fallback to the value), normalize to an array, then for each entry if it contains a glob pattern (e.g., '*' or '**') use a globbing library (glob or fast-glob) to expand matches and collect package.json files under matched directories (join each match with 'package.json' and fs.existsSync), otherwise treat it as a literal path as before; ensure you still return the discovered package.json paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/patch-security.js`:
- Around line 67-71: The loop currently unconditionally replaces any differing
override in rootPkg.overrides with ROOT_OVERRIDES values; instead, detect if an
existing override is stricter/newer and skip overwriting it. Update the logic in
the for (const [dep, ver] of Object.entries(ROOT_OVERRIDES)) loop to: if there
is no existing rootPkg.overrides[dep] then set it and mark rootChanged; if there
is an existing override compare semver (use the semver package, e.g.,
semver.lt/gt or semver.compare) and only assign rootPkg.overrides[dep] = ver and
set rootChanged when the ROOT_OVERRIDES ver is stricter/newer than the existing
one — otherwise leave the existing override untouched.
---
Nitpick comments:
In `@scripts/patch-security.js`:
- Around line 45-54: findWorkspacePackageJsons currently assumes root.workspaces
is a simple array of literal paths; update it to handle both object format and
glob patterns by: detect if readJson(root).workspaces is an object and pull the
array from workspaces.packages (or fallback to the value), normalize to an
array, then for each entry if it contains a glob pattern (e.g., '*' or '**') use
a globbing library (glob or fast-glob) to expand matches and collect
package.json files under matched directories (join each match with
'package.json' and fs.existsSync), otherwise treat it as a literal path as
before; ensure you still return the discovered package.json paths.
In `@scripts/patch-security.test.js`:
- Around line 32-34: The test suite's afterEach is a no-op and leaves temp
folders behind; update scripts/patch-security.test.js to track any temp
directories created during tests (e.g., push paths into a tempDirs array
whenever tests call mkdtemp or create temp dirs) and implement afterEach(() => {
tempDirs.forEach(d => fs.rmSync(d, { recursive: true, force: true }));
tempDirs.length = 0; }); ensuring fs is imported and removal uses
recursive+force to reliably clean up directories created by the tests (refer to
the existing afterEach and any mkdtemp/temp dir helpers used in the file).
- Around line 36-198: Add two new tests to the existing
patchSecurityVulnerabilities suite that exercise workspace-format edge cases:
one where the root package.json uses the object form workspaces: { packages:
['packages/foo'] } and another where workspaces contains a glob like
'packages/*'; in each test create the matching package under packages/, write
package.json and (when applicable) rollup.config.js with rollup-plugin-terser,
invoke patchSecurityVulnerabilities(tmp), and assert the same outcomes as the
existing tests (root overrides get security pins, package devDependencies
replace rollup-plugin-terser with `@rollup/plugin-terser` and rollup.config.js
require strings are updated, and `@aws-sdk/credential-providers` version is bumped
where present) so these format variations are covered.
🪄 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: e29ac0cc-b7b9-4d77-a460-1a9c2d95625e
📒 Files selected for processing (2)
scripts/patch-security.jsscripts/patch-security.test.js
| for (const [dep, ver] of Object.entries(ROOT_OVERRIDES)) { | ||
| if (rootPkg.overrides[dep] !== ver) { | ||
| rootPkg.overrides[dep] = ver; | ||
| rootChanged = true; | ||
| } |
There was a problem hiding this comment.
Don’t overwrite existing stricter overrides
Line 68 rewrites any existing override that differs from the table value. That can downgrade a manually-updated secure override to an older floor and create churn on future security bumps.
Suggested fix
for (const [dep, ver] of Object.entries(ROOT_OVERRIDES)) {
- if (rootPkg.overrides[dep] !== ver) {
+ if (rootPkg.overrides[dep] === undefined) {
rootPkg.overrides[dep] = ver;
rootChanged = true;
}
}📝 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.
| for (const [dep, ver] of Object.entries(ROOT_OVERRIDES)) { | |
| if (rootPkg.overrides[dep] !== ver) { | |
| rootPkg.overrides[dep] = ver; | |
| rootChanged = true; | |
| } | |
| for (const [dep, ver] of Object.entries(ROOT_OVERRIDES)) { | |
| if (rootPkg.overrides[dep] === undefined) { | |
| rootPkg.overrides[dep] = ver; | |
| rootChanged = true; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/patch-security.js` around lines 67 - 71, The loop currently
unconditionally replaces any differing override in rootPkg.overrides with
ROOT_OVERRIDES values; instead, detect if an existing override is stricter/newer
and skip overwriting it. Update the logic in the for (const [dep, ver] of
Object.entries(ROOT_OVERRIDES)) loop to: if there is no existing
rootPkg.overrides[dep] then set it and mark rootChanged; if there is an existing
override compare semver (use the semver package, e.g., semver.lt/gt or
semver.compare) and only assign rootPkg.overrides[dep] = ver and set rootChanged
when the ROOT_OVERRIDES ver is stricter/newer than the existing one — otherwise
leave the existing override untouched.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/patch-security.test.js (1)
7-29: Consider cleaning up temp directories after tests.The
makeTmpDir()function creates directories inos.tmpdir()but there's noafterEachorafterAllhook to remove them. While the OS eventually cleans temp directories, repeated test runs could accumulate orphaned folders.♻️ Optional cleanup pattern
+const createdDirs = []; + function makeTmpDir() { - return fs.mkdtempSync(path.join(os.tmpdir(), 'bruno-patch-test-')); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'bruno-patch-test-')); + createdDirs.push(dir); + return dir; } + +afterAll(() => { + for (const dir of createdDirs) { + fs.rmSync(dir, { recursive: true, force: true }); + } +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/patch-security.test.js` around lines 7 - 29, The tests create temp dirs via makeTmpDir() but never remove them; modify makeTmpDir to register each created path (e.g., push to a module-level array) and add an afterEach or afterAll hook in scripts/patch-security.test.js that iterates the array and removes each directory using fs.rmSync(dir, { recursive: true, force: true }) (or fs.rmdirSync for older Node), then clear the tracking array; reference makeTmpDir and the cleanup hook so temp directories are removed after tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/patch-security.test.js`:
- Around line 7-29: The tests create temp dirs via makeTmpDir() but never remove
them; modify makeTmpDir to register each created path (e.g., push to a
module-level array) and add an afterEach or afterAll hook in
scripts/patch-security.test.js that iterates the array and removes each
directory using fs.rmSync(dir, { recursive: true, force: true }) (or
fs.rmdirSync for older Node), then clear the tracking array; reference
makeTmpDir and the cleanup hook so temp directories are removed after tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d911f628-1f70-4749-b7f1-1ccc2ff24195
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonscripts/patch-security.jsscripts/patch-security.test.jsscripts/setup.js
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/setup.js
- package.json
- scripts/patch-security.js
# Conflicts: # package-lock.json
Description
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
Chores
Improvements
Tests