Conversation
…de integration in web preferences
📝 WalkthroughWalkthroughThis PR enhances Electron application security by disabling Node.js integration and enabling context isolation across all BrowserWindow instances, and making renderer-exposed APIs immutable via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/preload/index.ts (1)
33-107:⚠️ Potential issue | 🟡 MinorUpdate renderer test mocks for the frozen bridge objects.
The production hardening is good, but existing tests will fail due to property mutations on frozen objects:
test/renderer/components/ChatInputBox.test.ts:170usesObject.assign(((window as any).api ??= {}), {...})which attempts to mutate the frozenapiobjecttest/renderer/stores/uiSettingsStore.test.ts:182useswindow.deepchat.invoke = invokewhich attempts to mutate the frozen bridge objectThese patterns will throw
TypeErroronce the preload installs the frozen objects. Update these tests to either assign fresh globals before imports or usevi.stubGlobal/Object.definePropertyinstead of mutating the exposed objects.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/preload/index.ts` around lines 33 - 107, Tests fail because preload now freezes bridge objects (api and deepchat) and tests attempt to mutate them (e.g., Object.assign on window.api and assigning window.deepchat.invoke). Update those tests to avoid mutating frozen globals by creating fresh globals before importing the modules or stubbing them via the test runner: replace patterns like Object.assign(((window as any).api ??= {}), {...}) and direct assignment to window.deepchat.invoke with vi.stubGlobal('api', {...}) / vi.stubGlobal('deepchat', {...}) or use Object.defineProperty to define non-writable properties, ensuring you reference the exposed symbols api and deepchat (and specific test files ChatInputBox.test.ts and uiSettingsStore.test.ts) when making the replacements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/preload/index.ts`:
- Around line 33-107: Tests fail because preload now freezes bridge objects (api
and deepchat) and tests attempt to mutate them (e.g., Object.assign on
window.api and assigning window.deepchat.invoke). Update those tests to avoid
mutating frozen globals by creating fresh globals before importing the modules
or stubbing them via the test runner: replace patterns like
Object.assign(((window as any).api ??= {}), {...}) and direct assignment to
window.deepchat.invoke with vi.stubGlobal('api', {...}) /
vi.stubGlobal('deepchat', {...}) or use Object.defineProperty to define
non-writable properties, ensuring you reference the exposed symbols api and
deepchat (and specific test files ChatInputBox.test.ts and
uiSettingsStore.test.ts) when making the replacements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83220f07-af66-49ef-ac07-184872466a53
📒 Files selected for processing (3)
src/main/presenter/lifecyclePresenter/SplashWindowManager.tssrc/main/presenter/windowPresenter/index.tssrc/preload/index.ts
…de integration in web preferences
Summary by CodeRabbit