Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (164)
📒 Files selected for processing (31)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new Bash utility that downloads and normalizes OpenAPI specs from multiple SAMS server URLs and stores them by sanitized server folder and API version; also updates Changes
Sequence Diagram(s)sequenceDiagram
participant Script as "download-api-specs.sh"
participant SAMS as "SAMS Server (swagger.json)"
participant FS as "Local Filesystem (api-specs/)"
Script->>SAMS: HTTP GET /api/v2/swagger.json
SAMS-->>Script: swagger.json (JSON)
Script->>Script: validate content\nextract info.version\nnormalize servers\nsanitize URL -> folder
Script->>FS: mkdir -p api-specs/<sanitized-url>
Script->>FS: write <version>.json
alt empty/invalid response
Script->>Script: log error (German message)\ncontinue
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (3 warnings, 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)
📝 Coding Plan
Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@download-api-specs.sh`:
- Around line 28-32: The api_version value taken from the remote JSON (variable
api_version) is used directly to build filename/server_folder which allows
unsafe characters or path traversal; sanitize api_version before using it by
normalizing/restricting characters (e.g. replace slashes and backslashes with
'-', strip any ".." segments, and restrict to a safe whitelist like
[A-Za-z0-9._-], with a fallback value if empty) and then use the sanitized value
when constructing filename and server_folder so content is never written outside
the intended directory.
- Line 1: Add a Bash shebang as the very first line so the script runs under
Bash (it uses arrays and [[) — insert e.g. "#!/usr/bin/env bash" above the
existing "base_urls=(" line to ensure Bash is used instead of /bin/sh; no other
code changes required.
- Around line 21-28: The download/parsing path currently uses curl -s and jq -r
which can mask HTTP errors, hang without timeouts, and produce the literal
"null"; update the curl invocation that populates the variable content to use
fail-on-error and timeouts (e.g., enable -f plus --connect-timeout and
--max-time) and change the jq extraction for api_version to use jq -er
'.info.version // empty' so jq exits non-zero and emits nothing for missing/null
versions; after these changes, ensure the existing empty-check on content and
the handling that continues the loop when api_version is empty still apply
(variables: content, api_version).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54fcc140-f9ed-44d8-9949-98b1bff889d9
📒 Files selected for processing (16)
api-specs/flvb.sams-server.de/2.1.jsonapi-specs/hessen-volley.de/2.1.jsonapi-specs/nwvv.sams-server.de/2.1.jsonapi-specs/vvb.sams-server.de/2.1.jsonapi-specs/vvsa.sams-server.de/2.1.jsonapi-specs/wvv.sams-server.de/2.1.jsonapi-specs/www.dvv-ligen.de/2.1.jsonapi-specs/www.shvv.de/2.1.jsonapi-specs/www.ssvb.org/2.1.jsonapi-specs/www.tv-v.de/2.1.jsonapi-specs/www.vlw-online.de/2.1.jsonapi-specs/www.volley-saar.de/2.1.jsonapi-specs/www.volleyball-baden.de/2.1.jsonapi-specs/www.volleyball-bundesliga.de/2.1.jsonapi-specs/www.vvrp.de/2.1.jsondownload-api-specs.sh
| @@ -0,0 +1,36 @@ | |||
| base_urls=( | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the file to check shebang, syntax, and content
cat -n download-api-specs.shRepository: renao/sams-integration
Length of output: 1324
🏁 Script executed:
# Run the proposed verification check from the review comment
#!/bin/bash
set -euo pipefail
# Parse checks only (read-only): bash should accept, sh should fail for bash-only syntax.
bash -n download-api-specs.sh
if sh -n download-api-specs.sh 2>&1; then
echo "RESULT: Unexpected - POSIX sh parsed script successfully (may not use bash-only syntax)"
else
echo "RESULT: Expected - POSIX sh parse failed (confirms bash-only syntax)"
fiRepository: renao/sams-integration
Length of output: 189
Add Bash shebang (script uses Bash-only syntax and will fail under /bin/sh).
This script uses Bash arrays and [[ conditional syntax but has no shebang, making execution shell ambiguous. It fails to parse under POSIX sh with a "Syntax error: '(' unexpected".
Proposed fix
+#!/usr/bin/env bash
+set -euo pipefail
+
base_urls=(📝 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
| base_urls=( | |
| #!/usr/bin/env bash | |
| set -euo pipefail | |
| base_urls=( |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@download-api-specs.sh` at line 1, Add a Bash shebang as the very first line
so the script runs under Bash (it uses arrays and [[) — insert e.g.
"#!/usr/bin/env bash" above the existing "base_urls=(" line to ensure Bash is
used instead of /bin/sh; no other code changes required.
download-api-specs.sh
Outdated
Comment on lines
+21
to
+28
| content=$(curl -s "$url/api/v2/swagger.json") | ||
|
|
||
| if [[ -z "$content" ]]; then | ||
| echo "Fehler: Kein Content von $url geladen!" | ||
| continue | ||
| fi | ||
|
|
||
| api_version=$(echo "$content" | jq -r '.info.version') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "download-api-specs.sh" | head -20Repository: renao/sams-integration
Length of output: 89
🏁 Script executed:
cat -n ./download-api-specs.shRepository: renao/sams-integration
Length of output: 1324
🏁 Script executed:
# Test 1: Verify jq -r behavior on missing field
echo '{"info": {}}' | jq -r '.info.version'
echo "Exit code: $?"
# Test 2: Verify jq -r behavior with missing top-level field
echo '{}' | jq -r '.info.version'
echo "Exit code: $?"
# Test 3: Verify proposed fix with jq -er
echo '{"info": {}}' | jq -er '.info.version // empty'
echo "Exit code: $?"
# Test 4: Verify proposed fix with valid data
echo '{"info": {"version": "1.0"}}' | jq -er '.info.version // empty'
echo "Exit code: $?"Repository: renao/sams-integration
Length of output: 131
🏁 Script executed:
# Test curl -s behavior with HTTP error response
# We'll simulate this by checking what curl does
# First, let's verify curl exit codes with -s flag
echo "Testing curl -s behavior (using httpbin.org for testing):"
curl -s -w "\nHTTP Status: %{http_code}\n" https://httpbin.org/status/500 2>&1 | head -20
echo "---"
curl -s -w "\nHTTP Status: %{http_code}\n" https://httpbin.org/status/404 2>&1 | head -20Repository: renao/sams-integration
Length of output: 163
Harden download/parsing path for HTTP and JSON failures.
curl -s hides HTTP errors without the -f flag, allowing non-2xx responses (e.g., HTML error pages) to pass the empty check; no timeout is set, risking indefinite hangs; jq -r outputs the literal string "null" when fields are missing, creating files like null.json with invalid content.
The proposed fix correctly addresses these issues:
curl -fmakes curl exit non-zero on HTTP errors--connect-timeout 10 --max-time 30prevent hangsjq -er '.info.version // empty'exits non-zero and outputs nothing when the field is missing or null, enabling proper error handling in bash
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@download-api-specs.sh` around lines 21 - 28, The download/parsing path
currently uses curl -s and jq -r which can mask HTTP errors, hang without
timeouts, and produce the literal "null"; update the curl invocation that
populates the variable content to use fail-on-error and timeouts (e.g., enable
-f plus --connect-timeout and --max-time) and change the jq extraction for
api_version to use jq -er '.info.version // empty' so jq exits non-zero and
emits nothing for missing/null versions; after these changes, ensure the
existing empty-check on content and the handling that continues the loop when
api_version is empty still apply (variables: content, api_version).
download-api-specs.sh
Outdated
Comment on lines
+28
to
+32
| api_version=$(echo "$content" | jq -r '.info.version') | ||
| server_folder="api-specs/$(echo $url | sed 's/https\?:\/\///g' | sed 's/\//-/g')" | ||
| filename="$server_folder/${api_version}.json" | ||
| mkdir -p "$server_folder" | ||
| echo "$content" > "$filename" |
There was a problem hiding this comment.
Sanitize remote api_version before using it in a filename.
api_version comes from remote JSON and is used directly in the path. Unsafe characters (including / or ..) can lead to unintended writes.
Proposed fix
api_version=$(echo "$content" | jq -r '.info.version')
server_folder="api-specs/$(echo $url | sed 's/https\?:\/\///g' | sed 's/\//-/g')"
- filename="$server_folder/${api_version}.json"
+ safe_version=$(printf '%s' "$api_version" | sed -E 's/[^A-Za-z0-9._-]+/_/g')
+ if [[ -z "$safe_version" ]]; then
+ echo "Error: Unsafe API version '$api_version' from $url" >&2
+ continue
+ fi
+ filename="$server_folder/${safe_version}.json"
mkdir -p "$server_folder"
echo "$content" > "$filename"📝 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
| api_version=$(echo "$content" | jq -r '.info.version') | |
| server_folder="api-specs/$(echo $url | sed 's/https\?:\/\///g' | sed 's/\//-/g')" | |
| filename="$server_folder/${api_version}.json" | |
| mkdir -p "$server_folder" | |
| echo "$content" > "$filename" | |
| api_version=$(echo "$content" | jq -r '.info.version') | |
| server_folder="api-specs/$(echo $url | sed 's/https\?:\/\///g' | sed 's/\//-/g')" | |
| safe_version=$(printf '%s' "$api_version" | sed -E 's/[^A-Za-z0-9._-]+/_/g') | |
| if [[ -z "$safe_version" ]]; then | |
| echo "Error: Unsafe API version '$api_version' from $url" >&2 | |
| continue | |
| fi | |
| filename="$server_folder/${safe_version}.json" | |
| mkdir -p "$server_folder" | |
| echo "$content" > "$filename" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@download-api-specs.sh` around lines 28 - 32, The api_version value taken from
the remote JSON (variable api_version) is used directly to build
filename/server_folder which allows unsafe characters or path traversal;
sanitize api_version before using it by normalizing/restricting characters (e.g.
replace slashes and backslashes with '-', strip any ".." segments, and restrict
to a safe whitelist like [A-Za-z0-9._-], with a fallback value if empty) and
then use the sanitized value when constructing filename and server_folder so
content is never written outside the intended directory.
|
✅ Created PR with unit tests: #63 |
* Replace server.url with absolute url (for rest client generation) * Save json files formatted (for better diffs in the future) * Refactor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
download-api-specs.sh (3)
56-56:⚠️ Potential issue | 🟠 MajorSanitize remote
api_versionbefore building the filename.Line [56] uses server-provided
api_versiondirectly in the path. Unsafe characters (including slashes) can write outside the intended location.Proposed fix
- filename="$server_folder/${api_version}.json" + safe_version=$(printf '%s' "$api_version" | sed -E 's/[^A-Za-z0-9._-]+/_/g') + if [[ -z "$safe_version" ]]; then + echo "Error: Unsafe API version '$api_version' from $url" >&2 + continue + fi + filename="$server_folder/${safe_version}.json"#!/usr/bin/env bash set -euo pipefail sanitize() { printf '%s' "$1" | sed -E 's/[^A-Za-z0-9._-]+/_/g' } for v in "v2.1.0" "../x" "2026/03" " "; do echo "$v -> $(sanitize "$v")" done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@download-api-specs.sh` at line 56, Sanitize the server-provided api_version before using it to build filename (currently: filename="$server_folder/${api_version}.json") to prevent directory traversal or unsafe characters; add a helper (e.g., sanitize function) that replaces any character not in [A-Za-z0-9._-] with an underscore and use the sanitized result (e.g., sanitized_version) when composing filename and any file paths that reference api_version, ensuring server_folder and filename concatenation only uses the sanitized token.
30-33:⚠️ Potential issue | 🟠 MajorHarden HTTP fetch and version extraction failure handling.
Line [44] uses
curl -s(no fail-fast/timeouts), and Line [32] usesjq -r, which can return literalnull. This can silently generate invalid outputs.Proposed fix
get_api_version() { local json_content="$1" - echo "$json_content" | jq -r '.info.version' + echo "$json_content" | jq -er '.info.version // empty' } @@ - content=$(curl -s "$api_base_url/swagger.json") + if ! content=$(curl -fsS --connect-timeout 10 --max-time 30 "$api_base_url/swagger.json"); then + echo "Error: Failed to download swagger.json from $url" >&2 + continue + fi @@ api_version=$(get_api_version "$content") + if [[ -z "$api_version" ]]; then + echo "Error: Missing info.version in spec from $url" >&2 + continue + fi#!/usr/bin/env bash set -euo pipefail # jq behavior check: -r vs -er for missing value echo '{}' | jq -r '.info.version'; echo " <- jq -r output" if echo '{}' | jq -er '.info.version // empty' >/dev/null; then echo "Unexpected: jq -er succeeded for missing version" else echo "Expected: jq -er fails for missing version" fi # curl behavior check for HTTP errors code_no_fail=$(curl -s -o /dev/null -w '%{http_code}' https://httpbin.org/status/404 || true) echo "curl -s status: $code_no_fail" if curl -fsS https://httpbin.org/status/404 >/dev/null 2>&1; then echo "Unexpected: curl -f succeeded on 404" else echo "Expected: curl -f fails on 404" fiAlso applies to: 44-49, 53-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@download-api-specs.sh` around lines 30 - 33, The get_api_version function can return the literal "null" silently; change it to use jq's strict-fail mode (e.g., jq -er with a safe default like '.info.version // empty') and make the function return a non-empty value or fail so callers can detect the error; likewise harden the HTTP fetch that uses curl by adding fail-fast and error output flags (use -fS), a sensible timeout (e.g., --max-time), and check exit status / HTTP code so failures are handled instead of producing empty/invalid JSON for get_api_version to parse.
1-1:⚠️ Potential issue | 🔴 CriticalFix the shebang syntax so Bash is actually used.
Line [1] uses
# !/bin/bash, which is treated as a comment, not a shebang. This can execute under/bin/shand fail on Bash array syntax.Proposed fix
-# !/bin/bash +#!/usr/bin/env bash +set -euo pipefail#!/usr/bin/env bash set -euo pipefail # Verify interpreter parsing behavior bash -n download-api-specs.sh if sh -n download-api-specs.sh; then echo "Unexpected: script parses in sh" else echo "Expected: script requires bash syntax" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@download-api-specs.sh` at line 1, The leading shebang is malformed ("# !/bin/bash") so the script may be run under /bin/sh; replace the first line with a proper Bash shebang (for example "#!/usr/bin/env bash" or "#!/bin/bash") and add strict mode (e.g., "set -euo pipefail") near the top; after updating, validate the script with "bash -n download-api-specs.sh" (and optionally verify it fails under sh with "sh -n download-api-specs.sh") to ensure it indeed requires Bash-only syntax.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@download-api-specs.sh`:
- Line 56: Sanitize the server-provided api_version before using it to build
filename (currently: filename="$server_folder/${api_version}.json") to prevent
directory traversal or unsafe characters; add a helper (e.g., sanitize function)
that replaces any character not in [A-Za-z0-9._-] with an underscore and use the
sanitized result (e.g., sanitized_version) when composing filename and any file
paths that reference api_version, ensuring server_folder and filename
concatenation only uses the sanitized token.
- Around line 30-33: The get_api_version function can return the literal "null"
silently; change it to use jq's strict-fail mode (e.g., jq -er with a safe
default like '.info.version // empty') and make the function return a non-empty
value or fail so callers can detect the error; likewise harden the HTTP fetch
that uses curl by adding fail-fast and error output flags (use -fS), a sensible
timeout (e.g., --max-time), and check exit status / HTTP code so failures are
handled instead of producing empty/invalid JSON for get_api_version to parse.
- Line 1: The leading shebang is malformed ("# !/bin/bash") so the script may be
run under /bin/sh; replace the first line with a proper Bash shebang (for
example "#!/usr/bin/env bash" or "#!/bin/bash") and add strict mode (e.g., "set
-euo pipefail") near the top; after updating, validate the script with "bash -n
download-api-specs.sh" (and optionally verify it fails under sh with "sh -n
download-api-specs.sh") to ensure it indeed requires Bash-only syntax.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd3afadb-79f4-4e85-869f-546668f48a24
📒 Files selected for processing (16)
api-specs/flvb.sams-server.de/2.1.jsonapi-specs/hessen-volley.de/2.1.jsonapi-specs/nwvv.sams-server.de/2.1.jsonapi-specs/vvb.sams-server.de/2.1.jsonapi-specs/vvsa.sams-server.de/2.1.jsonapi-specs/wvv.sams-server.de/2.1.jsonapi-specs/www.dvv-ligen.de/2.1.jsonapi-specs/www.shvv.de/2.1.jsonapi-specs/www.ssvb.org/2.1.jsonapi-specs/www.tv-v.de/2.1.jsonapi-specs/www.vlw-online.de/2.1.jsonapi-specs/www.volley-saar.de/2.1.jsonapi-specs/www.volleyball-baden.de/2.1.jsonapi-specs/www.volleyball-bundesliga.de/2.1.jsonapi-specs/www.vvrp.de/2.1.jsondownload-api-specs.sh
|
✅ Created PR with unit tests: #64 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * @type {String} | ||
| * @default https://wvv.sams-server.de/api/v2 | ||
| */ | ||
| this.basePath = basePath.replace(/\/+$/, ''); |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
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
2 participants
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.
Closes #55
Summary by CodeRabbit
New Features
Documentation