Enhance SRI script robustness and refine Restaurant docs#265
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbitリリースノート
Walkthroughこのプルリクエストは、Markdownドキュメントの説明文を簡潔に更新し、シェルスクリプトの信頼性を強化しています。calc_sri_fix.shに厳密なシェルオプションとエラーハンドリングを追加し、HTTPダウンロードの失敗に対する耐性を向上させました。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@calc_sri_fix.sh`:
- Around line 7-25: The temporary file created in this function (temp_file via
mktemp) can leak if a command like the openssl pipeline fails under set -e; add
a trap at the start of the function to ensure rm -f "$temp_file" runs on
function exit (use trap 'rm -f "$temp_file"' RETURN for Bash 4+ or trap 'rm -f
"$temp_file"' EXIT at script top-level for wider compatibility), then remove the
final explicit rm -f "$temp_file" or keep it as a no-op; apply the change around
the block that downloads with curl and computes the hash (the temp_file variable
and the openssl dgst ... | openssl base64 -A pipeline).
- Line 3: The strict shell mode (set -euo pipefail) can leave the temp file
behind if openssl in the pipeline fails; inside the calculate_sri function, add
a cleanup trap so the temp file is always removed on function exit (e.g., trap
"rm -f \"$temp_file\"" RETURN placed at the top of calculate_sri), ensure
temp_file is defined before the trap is set, and keep the existing rm -f
"$temp_file" at the end for normal-path cleanup.
| @@ -1,9 +1,28 @@ | |||
| #!/bin/bash | |||
|
|
|||
| set -euo pipefail | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat calc_sri_fix.shRepository: myoshi2891/Algorithm-DataStructures-Math-SQL
Length of output: 948
set -euo pipefail の追加により、厳格なシェルモードが有効になりました。
-e オプションによるエラー発生時の即座の終了は、スクリプトの堅牢性を向上させます。Line 28 の calculate_sri が失敗した場合、スクリプト全体が終了し Line 29 の呼び出しが実行されないのは、エラー時の適切なフェイルファーストの動作です。
ただし、重要な問題として、Line 23 のパイプライン内で openssl コマンドが失敗した場合、set -e により即座にスクリプトが終了するため、Line 25 の rm -f "$temp_file" が実行されず、一時ファイルがクリーンアップされないままになります。 この問題を解決するには、関数内で以下のようなトラップを設定してください：
trap "rm -f \"$temp_file\"" RETURNこのトラップを関数の先頭に追加することで、予期しない終了時にも一時ファイルが確実にクリーンアップされます。
🤖 Prompt for AI Agents
In `@calc_sri_fix.sh` at line 3, The strict shell mode (set -euo pipefail) can
leave the temp file behind if openssl in the pipeline fails; inside the
calculate_sri function, add a cleanup trap so the temp file is always removed on
function exit (e.g., trap "rm -f \"$temp_file\"" RETURN placed at the top of
calculate_sri), ensure temp_file is defined before the trap is set, and keep the
existing rm -f "$temp_file" at the end for normal-path cleanup.
Comment on lines
+7
to
+25
| temp_file=$(mktemp) | ||
|
|
||
| # curl options: -f (fail on HTTP error), -S (show error), -s (silent equivalent), -L (follow redirects) | ||
| if ! curl -fS -sL "$url" -o "$temp_file"; then | ||
| echo "Error downloading $url" >&2 | ||
| rm -f "$temp_file" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Check for empty response | ||
| if [ ! -s "$temp_file" ]; then | ||
| echo "Error: Empty response from $url" >&2 | ||
| rm -f "$temp_file" | ||
| return 1 | ||
| fi | ||
|
|
||
| hash=$(openssl dgst -sha384 -binary < "$temp_file" | openssl base64 -A) | ||
| echo "$url sha384-$hash" | ||
| rm -f "$temp_file" |
Contributor
There was a problem hiding this comment.
一時ファイルのクリーンアップに trap の使用を推奨します。
Line 23 の openssl コマンドが失敗した場合、set -e によりスクリプトが即座に終了し、Line 25 の rm -f "$temp_file" が実行されずに一時ファイルがリークします。関数冒頭で trap を設定することで、異常終了時も確実にクリーンアップできます。
🛡️ trap を使った修正案
calculate_sri() {
url="$1"
temp_file=$(mktemp)
+ trap 'rm -f "$temp_file"' RETURN
# curl options: -f (fail on HTTP error), -S (show error), -s (silent equivalent), -L (follow redirects)
if ! curl -fS -sL "$url" -o "$temp_file"; then
echo "Error downloading $url" >&2
- rm -f "$temp_file"
return 1
fi
# Check for empty response
if [ ! -s "$temp_file" ]; then
echo "Error: Empty response from $url" >&2
- rm -f "$temp_file"
return 1
fi
hash=$(openssl dgst -sha384 -binary < "$temp_file" | openssl base64 -A)
echo "$url sha384-$hash"
- rm -f "$temp_file"
}注意:
trap ... RETURNは Bash 4.0+ で利用可能です。もし互換性が必要な場合は、trap ... EXITをスクリプトのトップレベルで設定するか、各パスでの手動rmを維持しつつopenssl行をifで囲む方法も有効です。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| temp_file=$(mktemp) | |
| # curl options: -f (fail on HTTP error), -S (show error), -s (silent equivalent), -L (follow redirects) | |
| if ! curl -fS -sL "$url" -o "$temp_file"; then | |
| echo "Error downloading $url" >&2 | |
| rm -f "$temp_file" | |
| return 1 | |
| fi | |
| # Check for empty response | |
| if [ ! -s "$temp_file" ]; then | |
| echo "Error: Empty response from $url" >&2 | |
| rm -f "$temp_file" | |
| return 1 | |
| fi | |
| hash=$(openssl dgst -sha384 -binary < "$temp_file" | openssl base64 -A) | |
| echo "$url sha384-$hash" | |
| rm -f "$temp_file" | |
| temp_file=$(mktemp) | |
| trap 'rm -f "$temp_file"' RETURN | |
| # curl options: -f (fail on HTTP error), -S (show error), -s (silent equivalent), -L (follow redirects) | |
| if ! curl -fS -sL "$url" -o "$temp_file"; then | |
| echo "Error downloading $url" >&2 | |
| return 1 | |
| fi | |
| # Check for empty response | |
| if [ ! -s "$temp_file" ]; then | |
| echo "Error: Empty response from $url" >&2 | |
| return 1 | |
| fi | |
| hash=$(openssl dgst -sha384 -binary < "$temp_file" | openssl base64 -A) | |
| echo "$url sha384-$hash" |
🤖 Prompt for AI Agents
In `@calc_sri_fix.sh` around lines 7 - 25, The temporary file created in this
function (temp_file via mktemp) can leak if a command like the openssl pipeline
fails under set -e; add a trap at the start of the function to ensure rm -f
"$temp_file" runs on function exit (use trap 'rm -f "$temp_file"' RETURN for
Bash 4+ or trap 'rm -f "$temp_file"' EXIT at script top-level for wider
compatibility), then remove the final explicit rm -f "$temp_file" or keep it as
a no-op; apply the change around the block that downloads with curl and computes
the hash (the temp_file variable and the openssl dgst ... | openssl base64 -A
pipeline).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.