Skip to content

Commit 3e1bcdb

Browse files
committed
feat(sdk-review): batch 5/6 — depth checks + PR-size advisory
Adds 10 checks covering depth/quality dimensions: ## Checks (.claude/scripts/) - check-testing-depth.sh — TD-01..10 (bug fixes have focused tests; new modules have integration tests; checkbox truthfulness) - check-errors-logging.sh — EL-01..04 (chaining, swallow detection via AST; no secrets in exception messages) - check-http-hygiene.sh — HTTP-01..07 (session reuse, timeouts, retry) - check-concurrency.sh — CC-01..04 (asyncio.Queue dedup, thread safety) - check-deletion-hygiene.sh — DEL-01..06 (removed symbols have zero residual references) - check-deps-supply.sh — DEP-01..07 (dep justification, lockfile drift). DEP-04 BLOCK_LOCKED (internal artifactory). - check-constants.sh — PY-CON-01 (repeated string literals via AST) - check-binding-shape.sh — BND-01..05. BND-02 (BTP token URL concat) and BND-05 (binding logs credentials) BLOCK_LOCKED. - check-quality-gate-parity.sh — QG-01..05 (CI runs the full dev gate) - check-pr-size.sh — PR-SIZE-01..05 advisory (SHADOW tier, logged only) Part 5 of 6. Ref: AFSDK-3937
1 parent 5afd4f7 commit 3e1bcdb

12 files changed

Lines changed: 413 additions & 0 deletions
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#!/usr/bin/env bash
2+
# check-binding-shape.sh — placeholder: stub check emitting empty findings.
3+
# TODO: implement rules from 01-PYTHON.md / 02-JAVA.md §3.binding-shape
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
source "$SCRIPT_DIR/lib/skill-self-skip.sh"
8+
9+
LANGUAGE="${LANGUAGE:-python}"
10+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
11+
REPO_ROOT="${REPO_ROOT:-.}"
12+
13+
STARTED=$(now_iso)
14+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
15+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
16+
17+
# BND-02 (LOCKED): token URL via string concat + "/oauth/token"
18+
echo "$diff_content" | awk '
19+
BEGIN { file=""; line=0 }
20+
/^diff --git a\// { file=$4; sub(/^b\//, "", file); line=0; next }
21+
/^@@/ { if (match($0, /\+[0-9]+/)) line=substr($0, RSTART+1, RLENGTH-1)+0; next }
22+
/^\+/ && !/^\+\+\+/ { print file "\t" line "\t" substr($0, 2); line++; next }
23+
/^ / { line++; next }
24+
' | while IFS=$'\t' read -r file line_num content; do
25+
[ -z "$file" ] && continue
26+
# Self-review protection: skip skill files
27+
if [ "$(is_skill_file "$file")" = "true" ]; then continue; fi
28+
if echo "$content" | grep -qE 'rstrip\("/"\)[[:space:]]*\+[[:space:]]*"/oauth/token"|\.replaceAll\("/\+\$", ""\)[[:space:]]*\+[[:space:]]*"/oauth/token"'; then
29+
emit_finding "BND-02" "BLOCK" "$file" "$line_num" \
30+
"BTP token URL built via string concat — different services expose different fields" \
31+
"Use HttpUrl.parse().newBuilder() or honour a 'token_url' field if present" >> "$findings"
32+
fi
33+
if echo "$content" | grep -qE '\+[[:space:]]*"/oauth/token"'; then
34+
emit_finding "BND-02" "BLOCK" "$file" "$line_num" \
35+
"Hardcoded /oauth/token path — BTP services vary" "" >> "$findings"
36+
fi
37+
done
38+
39+
status=$(status_from_findings < "$findings")
40+
emit_report "binding-shape" "$LANGUAGE" "$status" "$STARTED" < "$findings"
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#!/usr/bin/env bash
2+
# check-concurrency.sh — placeholder: stub check emitting empty findings.
3+
# TODO: implement rules from 01-PYTHON.md / 02-JAVA.md §3.concurrency
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
source "$SCRIPT_DIR/lib/skill-self-skip.sh"
8+
9+
LANGUAGE="${LANGUAGE:-python}"
10+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
11+
REPO_ROOT="${REPO_ROOT:-.}"
12+
13+
STARTED=$(now_iso)
14+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
15+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
16+
17+
# CC-01: asyncio.Queue with no accompanying Lock/set — flag when queue+append pattern present
18+
echo "$diff_content" | awk '
19+
BEGIN { file=""; line=0 }
20+
/^diff --git a\// { file=$4; sub(/^b\//, "", file); line=0; next }
21+
/^@@/ { if (match($0, /\+[0-9]+/)) line=substr($0, RSTART+1, RLENGTH-1)+0; next }
22+
/^\+/ && !/^\+\+\+/ { print file "\t" line "\t" substr($0, 2); line++; next }
23+
/^ / { line++; next }
24+
' | while IFS=$'\t' read -r file line_num content; do
25+
[ -z "$file" ] && continue
26+
if [ "$(is_skill_file "$file")" = "true" ]; then continue; fi
27+
if echo "$content" | grep -qE 'asyncio\.Queue\('; then
28+
# Check if same file has a Lock or set() nearby
29+
if [ -f "$REPO_ROOT/$file" ] && ! grep -qE 'asyncio\.Lock|set\(\)' "$REPO_ROOT/$file" 2>/dev/null; then
30+
emit_finding "CC-01" "FLAG" "$file" "$line_num" \
31+
"asyncio.Queue without dedup — if items must be unique, add a set() + asyncio.Lock" "" >> "$findings"
32+
fi
33+
fi
34+
done
35+
36+
status=$(status_from_findings < "$findings")
37+
emit_report "concurrency" "$LANGUAGE" "$status" "$STARTED" < "$findings"

.claude/scripts/check-constants.sh

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#!/usr/bin/env bash
2+
# check-constants.sh — placeholder: stub check emitting empty findings.
3+
# TODO: implement rules from 01-PYTHON.md / 02-JAVA.md §3.constants
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
8+
LANGUAGE="${LANGUAGE:-python}"
9+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
10+
REPO_ROOT="${REPO_ROOT:-.}"
11+
12+
STARTED=$(now_iso)
13+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
14+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
15+
16+
# CON-01 via AST on Python files
17+
if [ "$LANGUAGE" = "python" ]; then
18+
changed=$(echo "$diff_content" | grep -oE '^\+\+\+ b/src/.*\.py' | sed 's|^+++ b/||' | sort -u)
19+
if [ -n "$changed" ]; then
20+
# shellcheck disable=SC2086
21+
python3 "$SCRIPT_DIR/lib/ast_python_checks.py" con-01 $changed 2>/dev/null >> "$findings" || true
22+
fi
23+
fi
24+
25+
status=$(status_from_findings < "$findings")
26+
emit_report "constants" "$LANGUAGE" "$status" "$STARTED" < "$findings"
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#!/usr/bin/env bash
2+
# check-deletion-hygiene.sh — placeholder: stub check emitting empty findings.
3+
# TODO: implement rules from 01-PYTHON.md / 02-JAVA.md §3.deletion-hygiene
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
8+
LANGUAGE="${LANGUAGE:-python}"
9+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
10+
REPO_ROOT="${REPO_ROOT:-.}"
11+
12+
STARTED=$(now_iso)
13+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
14+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
15+
16+
# DEL-01: symbol removed from __all__ but references remain in codebase
17+
if [ "$LANGUAGE" = "python" ]; then
18+
# Extract removed __all__ entries
19+
removed_symbols=$(echo "$diff_content" | awk '
20+
/\+\+\+ b\/.*__init__\.py$/ { in_init=1; next }
21+
/^diff --git/ { in_init=0 }
22+
in_init && /^-[[:space:]]*"[A-Za-z_][A-Za-z0-9_]*"/ {
23+
match($0, /"[^"]+"/); print substr($0, RSTART+1, RLENGTH-2)
24+
}
25+
')
26+
while IFS= read -r sym; do
27+
[ -z "$sym" ] && continue
28+
# search codebase (excluding the file where it was removed)
29+
hits=$(grep -rEIln "\b${sym}\b" "$REPO_ROOT/src" 2>/dev/null | wc -l | tr -d ' ')
30+
if [ "$hits" -gt 0 ]; then
31+
emit_finding "DEL-01" "BLOCK" "src/" 1 \
32+
"Symbol '$sym' removed from __all__ but still referenced in $hits files" "" >> "$findings"
33+
fi
34+
done <<< "$removed_symbols"
35+
fi
36+
37+
status=$(status_from_findings < "$findings")
38+
emit_report "deletion-hygiene" "$LANGUAGE" "$status" "$STARTED" < "$findings"
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#!/usr/bin/env bash
2+
# check-deps-supply.sh — placeholder: stub check emitting empty findings.
3+
# TODO: implement rules from 01-PYTHON.md / 02-JAVA.md §3.deps-supply
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
8+
LANGUAGE="${LANGUAGE:-python}"
9+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
10+
REPO_ROOT="${REPO_ROOT:-.}"
11+
12+
STARTED=$(now_iso)
13+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
14+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
15+
16+
# DEP-04: internal artifactory index URL
17+
if echo "$diff_content" | grep -qE '\+.*(index-url|extra-index-url).*int\.repositories\.cloud\.sap'; then
18+
emit_finding "DEP-04" "BLOCK" "config" 1 \
19+
"Internal artifactory --index-url reference in public artifact" "" >> "$findings"
20+
fi
21+
# DEP-03: pyproject.toml deps changed but uv.lock not
22+
if [ "$LANGUAGE" = "python" ]; then
23+
pyproject_deps_changed=$(echo "$diff_content" | grep -cE '^\+.*"[a-z][a-z0-9_-]*[><=~!]+' || echo 0)
24+
uv_lock_changed=$(echo "$diff_content" | grep -c '^\+\+\+ b/uv\.lock' || echo 0)
25+
if [ "$pyproject_deps_changed" -gt 0 ] && [ "$uv_lock_changed" -eq 0 ]; then
26+
# only fire if pyproject.toml dep table (not [project] version) changed
27+
if echo "$diff_content" | grep -qE '^\+.*\[project\.(dependencies|optional-dependencies)\]|^\+[[:space:]]+"[a-z][a-z0-9_-]*[><=~!].*"'; then
28+
emit_finding "DEP-03" "FLAG" "pyproject.toml" 1 \
29+
"pyproject.toml dependencies changed but uv.lock not updated" "" >> "$findings"
30+
fi
31+
fi
32+
fi
33+
34+
status=$(status_from_findings < "$findings")
35+
emit_report "deps-supply" "$LANGUAGE" "$status" "$STARTED" < "$findings"
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#!/usr/bin/env bash
2+
# check-errors-logging.sh — exception chaining, log level, sensitive-info in messages.
3+
# Uses AST for chaining/swallow checks.
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
source "$SCRIPT_DIR/lib/skill-self-skip.sh"
8+
9+
LANGUAGE="${LANGUAGE:-python}"
10+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
11+
REPO_ROOT="${REPO_ROOT:-.}"
12+
13+
STARTED=$(now_iso)
14+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
15+
16+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
17+
18+
if [ "$LANGUAGE" = "python" ]; then
19+
changed=$(echo "$diff_content" | grep -oE '^\+\+\+ b/src/.*\.py' | sed 's|^+++ b/||' | sort -u)
20+
if [ -n "$changed" ]; then
21+
# AST-based chaining / swallow — only reports FLAGs when body ends non-Raise
22+
# shellcheck disable=SC2086
23+
python3 "$SCRIPT_DIR/lib/ast_python_checks.py" el-01 $changed 2>/dev/null >> "$findings" || true
24+
# shellcheck disable=SC2086
25+
python3 "$SCRIPT_DIR/lib/ast_python_checks.py" el-02 $changed 2>/dev/null >> "$findings" || true
26+
fi
27+
28+
# EL-04: secret-like variable name in raise args (grep-based, added lines only)
29+
echo "$diff_content" | awk '
30+
BEGIN { file=""; line=0 }
31+
/^diff --git a\// { file=$4; sub(/^b\//, "", file); line=0; next }
32+
/^@@/ { if (match($0, /\+[0-9]+/)) line=substr($0, RSTART+1, RLENGTH-1)+0; next }
33+
/^\+/ && !/^\+\+\+/ { print file "\t" line "\t" substr($0, 2); line++; next }
34+
/^ / { line++; next }
35+
' | while IFS=$'\t' read -r file line_num content; do
36+
[ -z "$file" ] && continue
37+
if [ "$(is_skill_file "$file")" = "true" ]; then continue; fi
38+
# match raise ...({...token|secret|password|api_key|client_secret...})
39+
if echo "$content" | grep -qE 'raise[[:space:]].*[fF]?"[^"]*\{[^}]*(token|secret|password|api_key|client_secret)[^}]*\}'; then
40+
emit_finding "EL-04" "BLOCK" "$file" "$line_num" \
41+
"Exception message includes secret-like variable — leaks sensitive data" "" >> "$findings"
42+
fi
43+
done
44+
fi
45+
46+
status=$(status_from_findings < "$findings")
47+
emit_report "errors-logging" "$LANGUAGE" "$status" "$STARTED" < "$findings"
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#!/usr/bin/env bash
2+
# check-http-hygiene.sh — placeholder: stub check emitting empty findings.
3+
# TODO: implement rules from 01-PYTHON.md / 02-JAVA.md §3.http-hygiene
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
source "$SCRIPT_DIR/lib/skill-self-skip.sh"
8+
9+
LANGUAGE="${LANGUAGE:-python}"
10+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
11+
REPO_ROOT="${REPO_ROOT:-.}"
12+
13+
STARTED=$(now_iso)
14+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
15+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
16+
17+
# HTTP-PY-01: Session per invocation (only fire on newly added lines)
18+
echo "$diff_content" | awk '
19+
BEGIN { file=""; line=0 }
20+
/^diff --git a\// { file=$4; sub(/^b\//, "", file); line=0; next }
21+
/^@@/ { if (match($0, /\+[0-9]+/)) line=substr($0, RSTART+1, RLENGTH-1)+0; next }
22+
/^\+/ && !/^\+\+\+/ { print file "\t" line "\t" substr($0, 2); line++; next }
23+
/^ / { line++; next }
24+
' | while IFS=$'\t' read -r file line_num content; do
25+
[ -z "$file" ] && continue
26+
if [ "$(is_skill_file "$file")" = "true" ]; then continue; fi
27+
# only for impl files (not tests/mocks)
28+
if echo "$file" | grep -qE '^(tests?/|mocks?/)'; then continue; fi
29+
if echo "$content" | grep -qE '(requests|httpx)\.(Session|AsyncClient)\(\)'; then
30+
emit_finding "HTTP-01" "FLAG" "$file" "$line_num" \
31+
"HTTP session created per invocation — prefer single instance in __init__" "" >> "$findings"
32+
fi
33+
done
34+
35+
status=$(status_from_findings < "$findings")
36+
emit_report "http-hygiene" "$LANGUAGE" "$status" "$STARTED" < "$findings"

.claude/scripts/check-pr-size.sh

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
#!/usr/bin/env bash
2+
# check-pr-size.sh — advisory on large PRs (all FLAG tier, initially SHADOW).
3+
set -euo pipefail
4+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
5+
source "$SCRIPT_DIR/lib/json-emit.sh"
6+
7+
LANGUAGE="${LANGUAGE:-python}"
8+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
9+
BASE_SHA="${BASE_SHA:-HEAD~10}"
10+
HEAD_SHA="${HEAD_SHA:-HEAD}"
11+
12+
STARTED=$(now_iso)
13+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
14+
15+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
16+
17+
# PR-SIZE-01: additions > 800
18+
additions=$(echo "$diff_content" | grep -cE '^\+[^+]' || echo 0)
19+
if [ "$additions" -gt 800 ]; then
20+
emit_finding "PR-SIZE-01" "FLAG" "." 1 \
21+
"PR has $additions additions (>800) — consider splitting into stacked PRs for easier review" \
22+
"See docs/CONTRIBUTING.md § Incremental Delivery for stacked-PR workflow" >> "$findings"
23+
fi
24+
25+
# PR-SIZE-02: > 15 files
26+
files_touched=$(echo "$diff_content" | grep -cE '^diff --git' || echo 0)
27+
if [ "$files_touched" -gt 15 ]; then
28+
emit_finding "PR-SIZE-02" "FLAG" "." 1 \
29+
"PR touches $files_touched files (>15) — consider splitting by concern" "" >> "$findings"
30+
fi
31+
32+
# PR-SIZE-03: > 3 modules
33+
if [ "$LANGUAGE" = "python" ]; then
34+
mods_count=$(echo "$diff_content" | grep -oE 'src/sap_cloud_sdk/[a-z_]+/' | sed 's|src/sap_cloud_sdk/||; s|/$||' | sort -u | wc -l | tr -d ' ')
35+
else
36+
mods_count=$(echo "$diff_content" | grep -oE 'src/main/java/com/sap/cloud/sdk/[a-z_]+/' | sed 's|src/main/java/com/sap/cloud/sdk/||; s|/$||' | sort -u | wc -l | tr -d ' ')
37+
fi
38+
if [ "$mods_count" -gt 3 ]; then
39+
emit_finding "PR-SIZE-03" "FLAG" "." 1 \
40+
"PR modifies $mods_count modules (>3) — consider one PR per module" "" >> "$findings"
41+
fi
42+
43+
# PR-SIZE-05: > 30 commits
44+
commit_count=$(git log "${BASE_SHA}..${HEAD_SHA}" --oneline 2>/dev/null | grep -v -c '^Merge' || echo 0)
45+
if [ "$commit_count" -gt 30 ]; then
46+
emit_finding "PR-SIZE-05" "FLAG" "." 1 \
47+
"PR has $commit_count commits (>30) — consider squashing or splitting" "" >> "$findings"
48+
fi
49+
50+
status=$(status_from_findings < "$findings")
51+
emit_report "pr-size" "$LANGUAGE" "$status" "$STARTED" < "$findings"
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#!/usr/bin/env bash
2+
# check-quality-gate-parity.sh — placeholder: stub check emitting empty findings.
3+
# TODO: implement rules from 01-PYTHON.md / 02-JAVA.md §3.quality-gate-parity
4+
set -euo pipefail
5+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
6+
source "$SCRIPT_DIR/lib/json-emit.sh"
7+
8+
LANGUAGE="${LANGUAGE:-python}"
9+
DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
10+
REPO_ROOT="${REPO_ROOT:-.}"
11+
12+
STARTED=$(now_iso)
13+
findings=$(mktemp); trap 'rm -f "$findings"' EXIT
14+
diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
15+
16+
# QG-01: workflow with `ruff check` but no `ruff format --check` or `ty check`
17+
if echo "$diff_content" | grep -qE '^\+\+\+ b/\.github/workflows/'; then
18+
wf_content=$(echo "$diff_content" | awk '/^\+\+\+ b\/\.github\/workflows\// { flag=1; next } /^diff --git/ { flag=0 } flag && /^\+/ && !/^\+\+\+/ { print }')
19+
if echo "$wf_content" | grep -q 'ruff check'; then
20+
if ! echo "$wf_content" | grep -q 'ruff format'; then
21+
emit_finding "QG-01" "FLAG" ".github/workflows/" 1 \
22+
"Workflow runs 'ruff check' but not 'ruff format --check' — mismatch with dev gate" "" >> "$findings"
23+
fi
24+
if ! echo "$wf_content" | grep -q 'ty check'; then
25+
emit_finding "QG-02" "FLAG" ".github/workflows/" 1 \
26+
"Workflow runs 'ruff check' but not 'ty check' — incomplete type gate" "" >> "$findings"
27+
fi
28+
fi
29+
fi
30+
31+
status=$(status_from_findings < "$findings")
32+
emit_report "quality-gate-parity" "$LANGUAGE" "$status" "$STARTED" < "$findings"

0 commit comments

Comments
 (0)