abap-code-review

name	abap-code-review
description	Pre-release ABAP code security and quality peer review across 12 dimensions. Auto-runs abaplint + ATC. Produces structured GO/NO-GO report with cumulative tracking between pipeline stages. Covers ZROUTER HTTP/OData/RFC/BDC patterns. Use before transporting ABAP code to QA or PRD.
trigger	["review ABAP","code review","peer review","pre-release check","transport review","assess code quality","audit security","release gate","ABAP review","GO NO-GO","ATC check","revisar codigo","quality gate","melhore o peer-review"]

ABAP Code Review v3.0 — 12-Dimension Pre-Release Gate

Structured peer review with cumulative tracking. Pipeline stages:

• Stage 3 (Proposal review): Architecture + design against 12 dims -> REVIEW_1.md
• Stage 7 (Code review): Implemented code vs Stage 3 findings -> REVIEW_2.md

Prerequisites (auto-run)

# Step 0 — Auto lint (mandatory, runs first)
npm run abap:lint 2>&1 | tee reports/lint.txt

# Step 0b — ATC check (if ADT available)
npm run abap:review 2>&1 | tee reports/atc.txt

12-Dimension Scoring

Each dimension scored 0-100. Weighted average determines GO/NO-GO.

Core (original 9)

#	Key	Weight	Description	Error Threshold
1	SEC	2.0	SQL injection, dynamic code, credential exposure, XSS in HTTP handler	CRITICAL=0
2	AUTH	1.5	Missing AUTHORITY-CHECK, S_DEVELOP gaps, ICF auth bypass	HIGH=0
3	DATA	1.5	No BAPIRET2 check, missing ENQUEUE, COMMIT without WAIT, LUW integrity	HIGH=0
4	PERF	1.0	SELECT in loop, SELECT *, missing WHERE, nested LOOP, HTTP timeout missing	MEDIUM<=2
5	STD	1.0	Deprecated statements, hardcoding, naming, Clean ABAP violations	MEDIUM<=5
6	INTERFACE	1.0	Hardcoded RFC dest, missing HTTP timeout, CORS misconfig, missing Content-Type	HIGH=0
7	CHANGE	1.5	Objects not in transport, shared INCLUDEs, cross-module impact	HIGH=0
8	COMP	1.0	PII exposure, missing change docs, GDPR, NW 7.40 compatibility	HIGH=0
9	FUNC	1.0	Logic errors, edge cases, requirement gaps, BAPI param validation	MEDIUM<=3

ZROUTER v5 (new 3)

#	Key	Weight	Description	Error Threshold
10	HTTP	1.5	IF_HTTP_EXTENSION security, CORS config, CSRF protection, path traversal, JSON injection, Content-Type validation	HIGH=0
11	BDC	1.0	BDC_OKCODE must be explicit, BDC_CURSOR set, MODE N validation, SHDB recording integrity, CALL TRANSACTION error handling	MEDIUM<=2
12	RFC	1.5	RFC destination not hardcoded, PARAMETER-TABLE security, dynamic CALL FUNCTION allowlist, bgRFC error handling, timeout config	HIGH=0

Auto-Check Rules (applied per file)

SEC — Security Patterns

# Mandatory checks - every file
grep -n "CONCATENATE.*INTO.*sql"          # SQL injection via dynamic SQL
grep -n "GENERATE SUBROUTINE"              # Dynamic code generation
grep -n "INSERT REPORT"                    # Program injection
grep -n 'PASSWORD.*=.*'\''[^$]'            # Hardcoded password
grep -n "CALL FUNCTION.*DESTINATION"       # Hardcoded RFC dest
grep -n "cl_http_client=>create_by_url"    # HTTP client without timeout

HTTP — REST/HTTP Security (ZROUTER v5)

grep -n "set_header_field.*Origin"         # CORS config present
grep -n "get_cdata("                       # Must validate Content-Type
grep -n "get_form_field("                  # Must escape/validate input
grep -n "CS.*'/sap/"                       # Path traversal check
grep -n "escape_url\|escape_html"          # Output escaping present

BDC — Batch Input Security (YFG_SBDC)

grep -n "BDC_OKCODE"                       # Must be explicit per screen
grep -n "BDC_CURSOR"                       # Must be set per screen
grep -n "MODE.*'N'"                        # Silent mode must handle errors
grep -n "MESSAGES INTO"                    # Must capture messages

RFC — Dynamic RFC Security

grep -n "CALL FUNCTION.*lv_\|("             # Dynamic FM call needs allowlist
grep -n "PARAMETER-TABLE"                  # Must validate before call
grep -n "DESTINATION.*lv_\|("              # Dynamic destination needs validation

Scoring Algorithm

score = 100 - (critical * 25) - (high * 10) - (medium * 3) - (low * 1)
weighted = sum(score[i] * weight[i]) / sum(weight[i])

GO:         all CRITICAL=0, all HIGH=0, weighted >= 70
CONDITIONAL: no CRITICAL, some HIGH=0, weighted >= 50
NO-GO:      any CRITICAL > 0 OR weighted < 50

Output Format

REVIEW_1.md (Stage 3 — Proposal)

# Peer Review 1 — Architecture Assessment
**Date**: {date} | **Stage**: 3/8 | **Objects**: {count}

## Dimension Scores
| # | Dim | Score | Weight | Findings C/H/M/L | Status |
|---|-----|-------|--------|------------------|--------|
| 1 | SEC | 100 | 2.0 | 0/0/0/0 | PASS |
| 2 | AUTH | 85 | 1.5 | 0/1/0/0 | FAIL |
| ... | ... | ... | ... | ... | ... |

## Weighted Score: {score}/100 → {GO|CONDITIONAL|NO-GO}

## Findings
### AUTH-1 [HIGH] — Missing AUTHORITY-CHECK in dispatch_action()
**File**: zcl_zrouter_http.clas.abap:287
**Problem**: No authorization check before dispatching action.
**Fix**: Add `AUTHORITY-CHECK OBJECT 'ZROUTER' ID 'ACTIVITY' FIELD lv_activity.`
**Stage 7 tracking**: Must be resolved before transport.

### HTTP-1 [HIGH] — CORS wildcard without validation
**File**: zcl_zrouter_http.clas.abap:102
**Problem**: Access-Control-Allow-Origin: * without request validation.
**Fix**: Validate Origin header against allowlist before setting wildcard.
**Stage 7 tracking**: If prod, restrict to known domains.

## Cumulative — Findings requiring Stage 7 verification
| ID | Severity | Object | Line | Status |
|----|----------|--------|------|--------|
| AUTH-1 | HIGH | ZCL_ZROUTER_HTTP | 287 | → Stage 7 |
| HTTP-1 | HIGH | ZCL_ZROUTER_HTTP | 102 | → Stage 7 |

REVIEW_2.md (Stage 7 — Code Review)

# Peer Review 2 — Code Verification
**Date**: {date} | **Stage**: 7/8 | **Transport**: {TR}

## Stage 3 Resolution
| ID | Was | Resolution | Status |
|----|-----|------------|--------|
| AUTH-1 | HIGH | AUTHORITY-CHECK added line 291 | RESOLVED |
| HTTP-1 | HIGH | Origin allowlist implemented line 104 | RESOLVED |

## New Findings (post-implementation)
...same format as Stage 3...

## Transport Gate Decision
**All Stage 3 findings resolved**: YES
**New CRITICAL/HIGH**: 0
**Weighted Score**: 92/100
**Decision**: GO — Ready for transport

Pipeline Integration

Stage 3 Execution

# In sap_router.py pipeline stage 3
def stage3_peer_review(proposal_files):
    results = []
    for file in proposal_files:
        lint = run_abaplint(file)
        review = agent("sap-code-reviewer",
            prompt=f"Review {file} architecture against 12 dimensions. Output per SKILL.md format.",
            schema=REVIEW_SCHEMA)
        results.append(review)
    return synthesize_review_1(results)

Stage 7 Execution

def stage7_peer_review(implemented_files, review_1):
    for finding in review_1.findings:
        verify_resolution(finding, implemented_files)
    new_findings = agent("sap-code-reviewer",
        prompt=f"Review implemented code in {implemented_files}. "
                f"Reference Stage 3 findings: {review_1.findings}. "
                f"Output per SKILL.md format with resolution tracking.",
        schema=REVIEW_SCHEMA)
    return synthesize_review_2(review_1, new_findings)

Verification

# Run full review pipeline
npm run abap:review

# Check lint report
test -f reports/lint.txt && echo "OK: lint report" || echo "FAIL: no lint"

# Check zero CRITICAL
! grep -q "severity.*critical" reports/lint.txt && echo "OK: 0 critical" || echo "FAIL: critical"

# Verify review decision
grep -E "^(GO|NO-GO|CONDITIONAL)" reports/REVIEW_2.md && echo "OK: decision" || echo "FAIL: no decision"

# Check cumulative tracking
grep -c "RESOLVED" reports/REVIEW_2.md | xargs -I{} test {} -gt 0 && echo "OK: Stage 3 resolved" || echo "WARN: no Stage 3 tracking"

# Auto-fix common issues
npm run abap:lint:fix   # Fix keyword casing, obsolete statements

Self-Learning

After each review, record outcomes:

npm run learn:route -- --action CODE_REVIEW --success true --score 92
npm run learn:mcp -- --mcp abaplint --latency 2.3 --success true