Closed emadurandal closed 3 years ago
採用するしないは、KEINOS さんが判断だろうとは思いますが、気になるところを数点。
手元(Ubuntu)では動きは問題ないですが、 #25 をみると mac では動かないようなので、-e md5sum
や -e md5
などに which をつけた方が良いと思います。
また、bin/checkkeylength に実行権限をつけた方が良いと思います。
.github/run-lint.sh の LIST_SCRIPT_NO_EXT
にも追加したほうが良いと思います。
(現状、このスクリプトで PR 時に lint を掛けるようにしているため)
いかがでしょうか?
@yoshi389111 ご指摘ありがとうございます。 ご指摘の3点対応しました。
鍵の長さチェックは強度確認含め欲しいなぁと思っていました。
後述する気になる点が 1 つありますが、LGTM なので入れちゃいましょう!
気になる点というのが、すでに check
コマンドがあるのでコマンド名が気になります。
例えば check keylength KEINOS
とサブコマンド風にするか check --keylength KEINOS ~/.ssh/id_rsa
みたいにオプションが使えれば直感的な気がします。
むしろ、check
事項の 1 つとして標準で組み込んじゃってもいいかもしれません。
ただ、それ自体は check
スクリプト内で checkkeylength
を呼び出せればいいだけなので、それは後に考えましょう。今は Let's go to 久美子んじゃいましょう!
# bin/check
...
echo "$@" | grep keylength 2>/dev/null 1>/dev/null && {
"${PATH_DIR_BIN}/checkkeylength" "$2" || {
echo >&2 "公開鍵の鍵長の取得に失敗しました"
exit 1
}
exit 0
}
もしかすると、今後このコマンドはデフォルトで他のコマンドのチェックに使われることになるかもしれません。その場合は関数化されてコマンドとしては消えるかもしれません。それだけはご承知おきください。(消えるとしても v2.0 だと思いますが) m( )m
実行権限が付いていないように思いますが、それ以外は問題なさそうです(Ubuntuで確認)
実行権限はあとでもよさそうなきがするので
確かに実行権限付いてないですね。まぁ、マージ後につけましょう。
おお、マージありがとうございます。
もしかすると、今後このコマンドはデフォルトで他のコマンドのチェックに使われることになるかもしれません。その場合は関数化されてコマンドとしては消えるかもしれません。それだけはご承知おきください。(消えるとしても v2.0 だと思いますが) m( )m
あ、そこらへんはもう全然お好きにやっちゃってくださいー。
RSA公開鍵の鍵長を確認するためのコマンドを追加してみました。
ご検討よろしくおねがいします。