CORS関連のオプションを追加

[ts-klassen]

内容

force_cors_policyオプションでオリジン間リソース共有ポリシーを、app://.に限定します。
allow_originオプションで新たに許可するオリジンを指定します。

例
python run.py --force_cors_policy --allow_origin 'http://localhost:8080' 'http://example.com'

関連 Issue

ref #392

その他

デフォルトは今までどおりすべてのオリジンを許可します。
無関係なリクエストを弾くor 実行をキャンセルする機能は未実装です

[Hiroshiba]

PR作成ありがとうございます！
PR説明文やオプション説明文もわかりやすくて助かります！

すみません、仕様をちゃんと定めていませんでした 🙇

何も指定せず起動するとapp://のみが許可される形を考えていました！
理由は、サードパーティアプリがオプションを知らず起動する形を取っていても、セキュリティ的に問題ないようにしたいためです。
（リクエスト通さない形の方のデフォルト設定をその形にすれば良いのですが、CORS側も足並み揃えておくと綺麗かなーと。）

あと、localhostと127.0.0.1もデフォルト設定で許可しても良いのかなとちょっと思っています。（こちらのコメント参照）

もしよければPRの内容を変えていただくのをお願いしても良いでしょうか。
ご面倒でしたら一度マージ後にこちらでよしなに調整したいと思います･･･！

[ts-klassen]

拝承。修正します。

[ts-klassen]

修正しました。
今後corsポリシーのデフォルトを変更したり追加したりしやすいように、選択式にしました。
それと、複数指定の方法を誤解していたのでHELPを修正しました。

簡単なテストを行ったので、結果を載せます。

Bashコマンド

export origins=(app://. http://localhost https://localhost http://localhost:8080 https://localhost:8080 http://127.0.0.1 https://127.0.0.1 http://127.0.0.1:8080 https://127.0.0.1:8080);

for val in "${origins[@]}"; do echo ''; echo $val; echo -e "GET / HTTP/1.1\r\nHost: null\r\nOrigin: $val\r\n\r\n" | nc localhost 50021 | grep allow-origin >/dev/null && echo 'allowed' || echo 'disallowed'; done

デフォルトで許可される例

app://.
allowed

http://localhost
allowed

https://localhost
allowed

http://localhost:8080
allowed

https://localhost:8080
allowed

http://127.0.0.1
allowed

https://127.0.0.1
allowed

http://127.0.0.1:8080
allowed

https://127.0.0.1:8080
allowed

デフォルトで不許可になる例

app://
disallowed

null
disallowed

data://.
disallowed

sftp://localhost
disallowed

sftp://localhost:22
disallowed

http://localhost.example.com
disallowed

http://localhost.example.com:8080
disallowed

localhost
disallowed

localhost:8080
disallowed

sftp://127.0.0.1
disallowed

sftp://127.0.0.1:22
disallowed

http://127.0.0.1.example.com
disallowed

http://127.0.0.1.example.com:8080
disallowed

127.0.0.1
disallowed

127.0.0.1:8080
disallowed

許可しても問題ないが、仕様通り不許可になる例

http://www.localhost
disallowed

https://www.localhost
disallowed

http://www.localhost:8080
disallowed

https://www.localhost:8080
disallowed

http://www.127.0.0.1
disallowed

https://www.127.0.0.1
disallowed

http://www.127.0.0.1:8080
disallowed

https://www.127.0.0.1:8080
disallowed

http://127.10.20.30
disallowed

https://127.10.20.30
disallowed

http://127.10.20.30:8080
disallowed

https://127.10.20.30:8080
disallowed

[ts-klassen]

正規表現のバックスラッシュをエスケープし忘れていました。
前述の簡易的なBashコマンドでのテスト結果は同じです。

[Hiroshiba]

修正ありがとうございます！！たすかります！！

デフォルト設定でVOICEVOXからはアクセスでき、適当なサイトからfetchすると↓みたいなエラーが出ることを確認しました！

[Hiroshiba]

@ts-klassen
フォーマッターが通されていないことによりlintエラーが発生していそうです。
pysen run format lintを実行していただければ！

@masinc さん
よろしければレビューお願いできると嬉しいです！

[ts-klassen]

pysen run format lintを実行しました。
よろしくお願いします！

[Hiroshiba]

LGTM！！
プルリクエストありがとうございました！！

[Hiroshiba]

あ、1行の文字数が多いことが原因でテストエラーが出てますね。オプションのあたりかな。

/home/runner/work/voicevox_engine/voicevox_engine/run.py:915:153: B950 line too long (152 > 88 characters)

pythonはたしかこんな感じで複数行にできると思うので、良い感じに改行して頂ければ！

(
  "１行目"
  "２行目"
)

[ts-klassen]

多分直ったと思います。

放送見ました！
「app://.」は許可しますが、「app://」は不許可です。最後のピリオドの有無が違います。（なのであえてテストケースに入れました）

「http://www.localhost:8080/home/index.cgi?arg=123#abc 」にアクセスした場合、オリジンは
「http://www.localhost:8080 」までです。

ちなみに、ブラウザで「http://localhost:50021/docs 」からリクエストを出しても、同一オリジンなのでCORSポリシーに関係なくすべて通ります。（試しに「http://www.localhost:50021/docs 」とか）

[masinc]

@ts-klassen さん
検証中、引数が非常に使いやすい設計でした！

@Hiroshiba さん @ts-klassen さん
少し気になったのが、 --allow_origin 引数値に * を許容するかは考えどころですね。
エディタ側で、 --cors_policy_mode 引数値が all になるUIを用意するのであれば、気にする必要がないかもですが、そうではなくオリジンをテキスト入力するのみのUIの場合、ユーザーがサードパーティアプリの導入説明などで気軽に * が入力できてしまうのは少し恐いと思いました。

[ts-klassen]

@masinc さん

WOAH!! VERY NICE CATCH!!

その可能性は完全に見落としていました！
ご指摘ありがとうございます！！

確かhttps://*みたいな文字列は意味をなさないので、*だけ制限するか考える必要があると思います。

[github-actions]

Coverage Result

Resultを開く

Name	Stmts	Miss	Cover
voicevox_engine/init.py	1	0
voicevox_engine/acoustic_feature_extractor.py	75	0
voicevox_engine/dev/synthesis_engine/init.py	2	0
voicevox_engine/dev/synthesis_engine/mock.py	36	2
voicevox_engine/full_context_label.py	162	3
voicevox_engine/kana_parser.py	86	1
voicevox_engine/model.py	154	7
voicevox_engine/mora_list.py	4	0
voicevox_engine/part_of_speech_data.py	5	0
voicevox_engine/preset/Preset.py	12	0
voicevox_engine/preset/PresetLoader.py	34	1
voicevox_engine/preset/init.py	3	0
voicevox_engine/synthesis_engine/init.py	5	0
voicevox_engine/synthesis_engine/core_wrapper.py	206	166
voicevox_engine/synthesis_engine/make_synthesis_engines.py	57	49
voicevox_engine/synthesis_engine/synthesis_engine.py	133	12
voicevox_engine/synthesis_engine/synthesis_engine_base.py	67	9
voicevox_engine/user_dict.py	129	6
voicevox_engine/utility/init.py	3	0
voicevox_engine/utility/connect_base64_waves.py	37	0
voicevox_engine/utility/path_utility.py	26	6
TOTAL	1237	262

[Hiroshiba]

エディタ側で、 --cors_policy_mode 引数値が all になるUIを用意するのであれば、気にする必要がないかもですが、そうではなくオリジンをテキスト入力するのみのUIの場合、ユーザーがサードパーティアプリの導入説明などで気軽に * が入力できてしまうのは少し恐いと思いました。

エディタ側からエンジンのCORS周りの起動オプションを提供する予定はありませんでした。
（たしかに便利そうではあるのですが、エンジンを直接起動するショートカットを用意していただくとかになるかなーと）

ちょっとピンと来てないのでお聞きしたいのですが、仮に*を入力できてしまうのが怖い理由はなんでしょう 👀
--cors_policy_mode 引数値が all になるUI（＝allowed_origins=*）と変わらない気がした次第です！

[ts-klassen]

仕様として許容するなら何ら問題はないと思います。

懸念があるとするならば、利用者が「すべてを許可する」という意味だと知らずに*を指定する可能性があることです。

*だけ特殊な意味を持つので、その扱いは仕様として決めておいたほうが良いでしょう。

[masinc]

@Hiroshiba さん
私も @ts-klassen さんとほぼ同意見です。

エディタ側からエンジンのCORS周りの起動オプションを提供する予定はありませんでした。
（たしかに便利そうではあるのですが、エンジンを直接起動するショートカットを用意していただくとかになるかなーと）

気軽に設定すべきでないという意味でも良いと思います！

[Hiroshiba]

仕様として許容するなら何ら問題はないと思います。
懸念があるとするならば、利用者が「すべてを許可する」という意味だと知らずに*を指定する可能性があることです。

なるほどです、すごくよくわかりました！！
であれば、*をオプションに与えることは可能な仕様で良いかなと思いました。
その代わり、仮にUIを作る場合は*入力時にワーニングかエラーを出すとかしましょう！

それでもブログなどで*を勧められるということは割りとありえると感じました。
*入力時はエンジン側でワーニングを出す、という仕様もありかもしれません。
もし実装頂けるならお願いしたいです！

PS. warnings.warnが使えると思います https://itasuke.hatenadiary.org/entry/20141122/p1

[ts-klassen]

warnings.warnは開発者向けの警告らしく、ソースファイルと行数、該当するソースを印字するため、

voicevox_engine/run.py

Line 841 in f171643

"WARNING: invalid VV_OUTPUT_LOG_UTF8 environment variable value",

と同様、標準エラーに出すようにしました。

上と統一して英語にしましたが、和文のほうが良ければ直します。

[Hiroshiba]

エラーはとりあえず英文で良いと思います！

@masinc さん的にもOKそうであればマージしたいと思います！

[masinc]

LGTM!

[Hiroshiba]

マージします！！

一度ビルドしてみて、その成果物とともに公式ツイッターから案内し、デフォルト設定だとブラウザから通信ができなくなるのでご確認ください的なのを言ってみようかなと思います。