-
Notifications
You must be signed in to change notification settings - Fork 310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: appプロトコルでfetch可能なディレクトリを制限する #2153
feat: appプロトコルでfetch可能なディレクトリを制限する #2153
Conversation
src/backend/electron/main.ts
Outdated
const isUnsafe = | ||
path.isAbsolute(relativePath) || | ||
relativePath.startsWith(`..${path.sep}`) || | ||
relativePath === ".." || | ||
relativePath === ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
voicevox/src/backend/electron/main.ts
Lines 652 to 656 in 4ea08d1
return !( | |
path.isAbsolute(relativePath) || | |
relativePath.startsWith(`..${path.sep}`) || | |
relativePath === ".." | |
); |
ここと共通化できそうな気がしたのですが
relativePath === ""
をどうするかという問題と「あるパスがあるパスの親ディレクトリであるか」を判定する関数を作ると考慮漏れやミスが起きそうでとりあえずしていません。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ですね! 役割が違ってくる思うので、共通化は避けるべきだと思います!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほぼLGTMです!!
助かります!!
あ、コメントでコードの参考にしたURL載せても良いかも。
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!!
内容
Electronのベストプラクティスに基づきセキュリティを強化します。
18. Avoid usage of the
file://
protocol and prefer usage of custom protocolsref: https://www.electronjs.org/ja/docs/latest/api/protocol#protocolhandlescheme-handler
その他
app://
プロトコルのファイルパスを制限して意図しないファイルの読み込みを防ぎます。