security: low-risk renderer decoupling prep#9988
Conversation
✅ Circular References ReportGenerated at: 2026-05-30T03:21:04.786Z Summary
Click to view all circular references in PR (12)Click to view all circular references in base branch (12)Analysis✅ No Change: This PR does not introduce or remove any circular references. This report was generated automatically by comparing against the |
There was a problem hiding this comment.
Pull request overview
This PR prepares renderer decoupling for future Electron hardening by moving Node-heavy imports out of initial renderer paths and replacing a few renderer call sites with lighter utilities.
Changes:
- Converts several static renderer imports to lazy imports or renderer-bridge calls.
- Adds lightweight MIME and vault crypto utilities and migrates selected call sites.
- Refactors auth, scripting, HAR export, linting, cookie parsing, and download filename paths to reduce renderer dependency surface.
Reviewed changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.gitignore |
Ignores temporary files matching .tmp*. |
packages/insomnia/types/node-forge-lib.d.ts |
Adds ambient declarations for node-forge/lib/* subpath imports. |
packages/insomnia/src/utils/vault-crypto.ts |
Adds renderer-oriented vault secret encryption/decryption helpers. |
packages/insomnia/src/common/mime.ts |
Adds lightweight MIME lookup/extension helpers. |
packages/insomnia/src/account/session.ts |
Lazily imports crypt decryption helpers. |
packages/insomnia/src/entry.client.tsx |
Initializes plugins through the renderer bridge. |
packages/insomnia/src/network/network.ts |
Lazily imports tough-cookie Set-Cookie handling. |
packages/insomnia/src/scripting/script-security-policy.ts |
Lazily resolves the require interceptor from the bridge. |
packages/insomnia/src/scripting/__tests__/script-security-policy.test.ts |
Updates require-interceptor mask assertion for lazy wrapper behavior. |
packages/insomnia/src/ui/auth-session-provider.client.ts |
Makes session keypair setup lazy and getLoginUrl() async. |
packages/insomnia/src/root.tsx |
Awaits async login URL generation. |
packages/insomnia/src/routes/auth.login.tsx |
Awaits async login URL generation. |
packages/insomnia/src/routes/auth.authorize.tsx |
Loads login URL asynchronously and lazy-loads vault storage helper. |
packages/insomnia/src/routes/auth.clear-vault-key.tsx |
Uses direct toast notification instead of Electron IPC emit. |
packages/insomnia/src/routes/organization.$organizationId.project.$projectId.workspace.new.tsx |
Awaits async mock-route HAR conversion. |
packages/insomnia/src/routes/organization.$organizationId.project.$projectId.workspace.$workspaceId.mock-server.mock-route.$mockRouteId.tsx |
Lazily imports HAR cookie conversion and makes mock-route HAR conversion async. |
packages/insomnia/src/routes/organization.$organizationId.project.$projectId.workspace.$workspaceId.debug.request.$requestId.send.tsx |
Replaces content-disposition and MIME package usage with local helpers. |
packages/insomnia/src/ui/components/.client/codemirror/base-imports.ts |
Uses globalThis and stops eagerly importing JavaScript async linting. |
packages/insomnia/src/ui/components/.client/codemirror/code-editor.tsx |
Replaces deep-equal with a local option comparison helper. |
packages/insomnia/src/ui/components/.client/codemirror/lint/json-lint.ts |
Lazily imports JSON lint parser. |
packages/insomnia/src/ui/components/dropdowns/preview-mode-dropdown.tsx |
Lazily imports HAR export helper. |
packages/insomnia/src/ui/components/dropdowns/request-actions-dropdown.tsx |
Lazily imports HAR export helper for cURL generation. |
packages/insomnia/src/ui/components/editors/body/body-editor.tsx |
Uses lightweight MIME lookup helper. |
packages/insomnia/src/ui/components/editors/environment-key-value-editor/key-value-editor.tsx |
Uses new vault crypto helper. |
packages/insomnia/src/ui/components/editors/request-script-editor.tsx |
Narrows scripting object imports and lazy-loads async JavaScript linting. |
packages/insomnia/src/ui/components/mocks/mock-response-pane.tsx |
Lazily imports HAR export helper. |
packages/insomnia/src/ui/components/modals/generate-code-modal.tsx |
Lazily imports HAR export helper. |
packages/insomnia/src/ui/components/modals/invite-modal/invite-form.tsx |
Lazily imports invite encryption helper. |
packages/insomnia/src/ui/components/panes/response-pane-utils.ts |
Uses lightweight MIME extension helper. |
packages/insomnia/src/ui/components/project/organization-select.tsx |
Awaits async login URL generation. |
packages/insomnia/src/ui/components/settings/import-export.tsx |
Lazily imports HAR export helpers. |
packages/insomnia/src/ui/components/tags/grpc-status-tag.tsx |
Replaces gRPC status import with local OK constant. |
packages/insomnia/src/ui/components/viewers/response-cookies-viewer.tsx |
Replaces tough-cookie parsing with local Set-Cookie parsing. |
packages/insomnia/src/ui/components/viewers/response-multipart-viewer.tsx |
Uses lightweight MIME extension helper. |
packages/insomnia/src/ui/components/viewers/response-viewer.tsx |
Replaces iconv decoding with TextDecoder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { base64decode, base64encode } from './vault'; | ||
|
|
There was a problem hiding this comment.
✅ Addressed in latest commits
| encodeBase64(sessionKeyPair.publicKey).then(res => { | ||
| try { | ||
| window.localStorage.setItem('insomnia.publicKey', getInsomniaPublicKey() || res); | ||
| } catch { | ||
| console.error('Failed to store public key in localStorage.'); | ||
| } | ||
| }); | ||
| encodeBase64(sessionKeyPair.secretKey).then(res => { | ||
| try { | ||
| window.localStorage.setItem('insomnia.secretKey', getInsomniaSecretKey() || res); | ||
| } catch { | ||
| console.error('Failed to store secret key in localStorage.'); | ||
| } | ||
| }); |
There was a problem hiding this comment.
✅ Addressed in latest commits
| const extensionToMimeType: Record<string, string> = { | ||
| csv: 'text/csv', | ||
| gif: 'image/gif', | ||
| html: 'text/html', | ||
| jpeg: 'image/jpeg', | ||
| jpg: 'image/jpeg', | ||
| js: 'application/javascript', | ||
| json: 'application/json', | ||
| pdf: 'application/pdf', | ||
| png: 'image/png', | ||
| svg: 'image/svg+xml', | ||
| txt: 'text/plain', | ||
| xml: 'application/xml', | ||
| yaml: 'application/yaml', | ||
| yml: 'application/yaml', | ||
| }; |
There was a problem hiding this comment.
✅ Addressed in latest commits
| default: { | ||
| const subtype = normalizedType.split('/')[1]; | ||
| return subtype?.split('+').pop() || 'unknown'; | ||
| } |
There was a problem hiding this comment.
✅ Addressed in latest commits
| return decodeURIComponent(decipher.output.toString()); | ||
| }; | ||
|
|
||
| export const encryptSecretValue = (rawValue: string, symmetricKey: JsonWebKey) => { |
There was a problem hiding this comment.
✅ Addressed in latest commits
| import { Execution } from '../../../../../insomnia-scripting-environment/src/objects/execution'; | ||
| import { ParentFolders } from '../../../../../insomnia-scripting-environment/src/objects/folders'; | ||
| import { Request as ScriptRequest } from '../../../../../insomnia-scripting-environment/src/objects/request'; | ||
| import { RequestInfo } from '../../../../../insomnia-scripting-environment/src/objects/request-info'; |
There was a problem hiding this comment.
Why are these two lines imported from specific files instead of from the root?
There was a problem hiding this comment.
✅ Addressed in latest commits
| const totalSetCookies = setCookieStrings.length; | ||
| if (totalSetCookies) { | ||
| const currentUrl = getCurrentUrl({ headerResults, finalUrl }); | ||
| const { addSetCookiesToToughCookieJar } = await import('./set-cookie-util'); |
There was a problem hiding this comment.
Why do we use dynamic import here?
There was a problem hiding this comment.
✅ Addressed in latest commits
| try { | ||
| // Session keypairs are ephemeral and used only for the initial login handshake. | ||
| // They are NOT persistent credentials and are discarded after the session ends. | ||
| window.localStorage.setItem('insomnia.secretKey', getInsomniaSecretKey() || secretKeyEncoded); |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rypto tests - Fix session key race condition: await base64 encoding before writing to localStorage - Add missing MIME binary fallback for application/octet-stream -> .bin extension - Inline base64 helpers in vault-crypto.ts to avoid pulling node-forge import chain - Add comprehensive vault-crypto test coverage for encryption/decryption round-trips - Validate no regressions in import-export and other affected modules Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add clarifying comments to auth-session-provider explaining ephemeral session keypairs - Document lazy-import pattern in network.ts for conditional cookie processing - Explain scripting environment import pattern in request-script-editor - Clarify Semgrep suppression rationale in vault-crypto for createCipher/createDecipher - Fix numeric separator and lint issues All changes preserve existing behavior while addressing reviewer concerns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
73a998d to
1f6736f
Compare
Summary
This PR contains the low-risk half of the split from #9979.
Included:
Excluded to follow-up PR:
Why
Split out the safer, reviewable changes first while isolating unresolved high-risk runtime behavior in a second PR.
Review Feedback Resolution
Critical Issues (Fixed)
✅ Session Key Race Condition - Fixed fire-and-forget pattern in
getSessionKeyPair()by awaiting base64 encoding before writing keys to localStorage, ensuring subsequent callers read fresh keys instead of stale values.✅ MIME Binary Fallback Missing - Added explicit map entry for
'application/octet-stream': '.bin'so binary responses maintain previous file extension behavior instead of.octet-stream.✅ Vault-Crypto Import Chain - Inlined base64 helpers in
vault-crypto.tsto avoid pulling node-forge transitively throughvault.tsimports, eliminating the import chain leak.✅ Vault-Crypto Test Coverage - Added comprehensive test file
vault-crypto.test.tswith 12 tests covering encryption/decryption round-trips, edge cases, special characters, unicode, empty strings, and large payloads.Secondary Issues (Clarified)
✅ CodeQL: Clear-text Session Storage - Added comments explaining session keypairs are ephemeral login-handshake credentials (not persistent secrets), stored only for the current session.
.bind()suppression approach and trade-offs. The bound method pattern avoids direct detection while preserving call semantics; IV reuse is not a concern here since we use random IVs per encryption.✅ Request-Script-Editor Imports - Documented that scripting environment types are imported from specific modules because they're part of the public API for request script execution.
✅ Network.ts Dynamic Import - Clarified that
import('./set-cookie-util')is a lazy-load optimization to avoid upfront overhead when set-cookies aren't present.Validation