feat: リモートコンテンツからのセッション権限リクエストを拒否するハンドラを追加 #2744
Conversation
|
🚀 プレビュー用ページを作成しました 🚀 更新時点でのコミットハッシュ： |
src/backend/electron/main.ts
Outdated
| const parsedUrl = new URL(webContents.getURL()); | ||
| const parsedRequestingUrl = new URL(requestingUrl); | ||
| let isAppUrl: boolean; | ||
| if (import.meta.env.VITE_DEV_SERVER_URL != undefined) { |
There was a problem hiding this comment.
import.meta.env.DEVのほうが適していると思います。
一応これはDead Code Eliminationで消えると思いますが、まぁ一応...
| if (import.meta.env.VITE_DEV_SERVER_URL != undefined) { | |
| if (import.meta.env.DEV) { |
There was a problem hiding this comment.
型定義上import.meta.env.VITE_DEV_SERVER_URL != undefinedは消せないのですよね。
そうなると冗長な気がします。
There was a problem hiding this comment.
ん～、となるとassertNotNull(import.meta.env.VITE_DEV_SERVER_URL)を使ってもいいかも？DEVかつVITE_DEV_SERVER_URL == undefinedはunreachableなのでそれを防げますし。
There was a problem hiding this comment.
たぶん要するにこういうことですよね：
| if (import.meta.env.VITE_DEV_SERVER_URL != undefined) { | |
| if (import.meta.env.DEV != undefined) { | |
| assertNonNullable(import.meta.env.VITE_DEV_SERVER_URL) |
assertNonNullableはこれ↓
https://github.com/VOICEVOX/voicevox/blob/d2a9ebf67267b1ce79a6840fe0519193a16dcc04/src/type/utility.ts#L14C17-L14C34
There was a problem hiding this comment.
Pull Request Overview
This PR implements security best practices by adding a handler to manage session permission requests from remote content in the Electron application. The handler follows Electron's security recommendations to prevent unauthorized access to system permissions.
- Adds a permission request handler that filters requests based on origin/protocol
- Implements different logic for development (VITE_DEV_SERVER_URL origin) vs production (app: protocol) environments
- Rejects all permission requests from untrusted remote content by default
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/backend/electron/main.ts
Outdated
| // リモートコンテンツからのセッション権限リクエストを全て拒否 | ||
| void app.whenReady().then(() => { | ||
| session.defaultSession.setPermissionRequestHandler( | ||
| (webContents, permission, callback, { requestingUrl }) => { | ||
| const parsedUrl = new URL(webContents.getURL()); | ||
| const parsedRequestingUrl = new URL(requestingUrl); | ||
| let isAppUrl: boolean; | ||
| if (import.meta.env.VITE_DEV_SERVER_URL != undefined) { | ||
| const { origin } = new URL(import.meta.env.VITE_DEV_SERVER_URL); | ||
| isAppUrl = | ||
| parsedUrl.origin === origin && parsedRequestingUrl.origin === origin; | ||
| } else { | ||
| isAppUrl = | ||
| parsedUrl.protocol === "app:" && | ||
| parsedRequestingUrl.protocol === "app:"; | ||
| } | ||
| return callback(isAppUrl); | ||
| }, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The URL constructor can throw an error if webContents.getURL() or requestingUrl contain invalid URLs. This should be wrapped in a try-catch block to prevent the application from crashing, and invalid URLs should be treated as untrusted (return false).
There was a problem hiding this comment.
これはあり得ないような?
起こるとしたらそれはElectronのバグしかないような気がします。
(パースできないURLにElectronがアクセスしているかリクエスト元のURLが偽装されているということになる)
|
ping @sevenc-nanashi |
Update src/backend/electron/main.ts Co-authored-by: Copilot <[email protected]>
|
マージします。 |
3 participants
内容
セキュリティのベストプラクティスに基づきリモートコンテンツからのセッション権限リクエストのハンドリングを行います。
5. リモートコンテンツからのセッション権限リクエストのハンドリング
その他
とりあえず開発時はオリジンが
VITE_DEV_SERVER_URLのオリジンと一致している場合、リリース時はappプロトコルの場合は無条件にリクエストを許可し、それ以外は全て拒否しています。もっと安全にするなら権限をホワイトリスト形式で許可するべきかもしれません。