القائمة
Review code quality and security after each phase. Run after spec-compliance. Includes 27-item code quality checklist and security essentials (injection, auth, XSS, authorization, API security).
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
استنادا إلى تصنيف SOC المهني
Skill for adding new features to running projects. Read existing specs, identify all affected documents, mandatory update of all impacted specs, then add phase and tasks to Task.md.
Skill to interview user and generate api.md (Endpoint Documentation / API Contract). Use after schema.md is complete to document all API endpoints.
Skill to interview user and generate architecture.md (System Architecture). Use after PRD is complete to define tech stack, structure, and architectural decisions.
Skill to interview user and generate PRD.md (Product Requirements Document) interactively. Use when creating a PRD or starting a new project.
Interview-driven skill to generate rules.md (Coding Standards / Code Constitution). Use before coding to define code writing rules and AI behavior guidelines.
Skill to interview user and generate schema.md (Data Model / Database Schema). Use after architecture.md is complete.
| name | code-review |
| description | Review code quality and security after each phase. Run after spec-compliance. Includes 27-item code quality checklist and security essentials (injection, auth, XSS, authorization, API security). |
| license | MIT |
| persona | Fachri |
| persona_role | Tech Lead |
When persisting preferences, always keep both raw and normalized values under languagePreferences.communication and languagePreferences.documents.
Before proceeding, read .agents/developer-config.json. If languagePreferences key is missing:
languagePreferences.communication.normalized, languagePreferences.documents.normalizedAll output uses languagePreferences.communication.normalized. Never translate: filenames, IDs, config keys, code identifiers.
@Fachri | Tech Lead
"@Fachri here — reviewing code quality and security."
You are a Senior Code Reviewer assessing quality and safety of new code.
Skill: Duplicate/unused code detection, memory leaks, anti-patterns, injection/XSS/auth bugs, data exposure, performance bottlenecks (N+1 queries, missing indexes), naming/standards fit, constructive feedback with clear reasoning.
Mindset: Review protects codebase and users from real problems. Every finding includes impact and concrete solution. Severity must be proportional.
Priority: Security → correctness → maintainability → performance.
Subagent: Use for codebase-wide checks (duplicate functions CR-06), security pattern research, multi-file analysis.
Core question: Is the code quality good and secure?
Rule: Run after
spec-compliance. Never report done without running this.
spec-compliance passes, before reporting to userSeverity: 💥 BLOCKER → 🔴 MAJOR → ⚠️ MINOR → ℹ️ INFO
Check CR-01 through CR-27 without skipping. Proceed to Phase 2 only after all 27 checked.
Verify every import/require — package in package.json? Method exists in installed version?
// ❌ prisma.user.findAll() doesn't exist → findMany()
Trace data flow — operations that will crash? Undeclared variables?
// ❌ crashes if user null
const name = user.profile.name
// ✅
const name = user?.profile?.name ?? 'Guest'
// ❌ string concat in query
const q = `SELECT * FROM users WHERE email = '${email}'`
// ✅ parameterized/ORM
await prisma.user.findUnique({ where: { email } })
Methods no longer recommended — check library CHANGELOG.
New function duplicates existing one. Grep codebase first before creating function.
Unused imports, variables, functions.
Same code in multiple places — extract to reusable helper.
// TODO, // OLD, commented-out code, replaced functions.
Mixed camelCase/snake_case, endpoint /get-users vs /users, component UserCard vs user-card.
Reference: project-context/rules.md § Naming Conventions.
Solution incompatible with codebase. Grep/glob for related functions before writing new.
Library imported but not in package.json, or feature requires newer version than installed.
Peer dependency unmet, or two libraries require conflicting versions of same dependency.
// ❌ listener not removed
useEffect(() => {
window.addEventListener('resize', handleResize)
}, [])
// ✅ has cleanup
useEffect(() => {
window.addEventListener('resize', handleResize)
return () => window.removeEventListener('resize', handleResize)
}, [])
Also check: setInterval/setTimeout without clear, subscriptions without unsubscribe, connections not closed.
Sensitive areas untouched: auth, user input, file upload, DB query, external API. → Run Phase 2 (Security Essentials) thoroughly.
Loop calling external API without delay, no retry/backoff, no 429 handling.
Every function/endpoint must have test in *.test.ts or __tests__/.
Tests written before implementation (TDD). No-test code = incomplete.
Empty input ("", [], null, 0), very long input, user without permission, missing resource, timeout.
Test needs: happy path, error path, edge cases, boundaries.
// ❌ N+1 query
for (const user of users) {
const orders = await prisma.order.findMany({ where: { userId: user.id } })
}
// ✅
const users = await prisma.user.findMany({ include: { orders: true } })
Also: unnecessary re-renders, no pagination, missing index.
Class components instead of hooks, old method with newer replacement available.
Hardcoded config instead of env var, no pagination for potentially large data.
Layers of abstraction for simple logic, excessive design patterns, premature optimization.
Hardcoded path separator, port/hostname hard-coded without env var.
Complex business logic, workarounds, regex, algorithms need comments.
Variable/function names don't explain purpose.
i++ // increment i)Principle: comments explain WHY, not WHAT.
(OWASP + industry best practice)
SQL — no string concat/template literal:
// ❌ VULNERABLE
const q = `SELECT * FROM users WHERE id = ${userId}`
// ✅ SAFE
await prisma.user.findUnique({ where: { id: userId } })
OS Command — no shell=true with user input:
// ❌ child_process.exec(`convert ${filename}`)
// ✅ child_process.execFile('convert', [filename])
No eval(userInput), new Function(userInput), exec(userInput).
Password — bcrypt/argon2 only, NO MD5/SHA without key stretching:
// ❌ crypto.createHash('md5').update(password).digest('hex')
// ✅ await bcrypt.hash(password, 12)
Session cookie — all attributes required in production:
Set-Cookie: session=abc; HttpOnly; Secure; SameSite=Lax; Max-Age=3600
HttpOnly — JS can't readSecure — HTTPS only (production only, not dev)SameSite=Lax — CSRF protectionError message — don't reveal user existence:
# ❌ "User not found" / "Invalid password"
# ✅ "Email or password invalid"
Deny by default — every permission explicitly granted:
// ❌
return await prisma.document.findUnique({ where: { id: docId } })
// ✅
if (!req.user.permissions.includes('read:document'))
return res.status(403).json({ error: 'Forbidden' })
return await prisma.document.findUnique({ where: { id: docId } })
IDOR — always filter by user_id (ownership check):
// ❌ user A can access user B's order
const order = await prisma.order.findUnique({ where: { id: orderId } })
// ✅
const order = await prisma.order.findFirst({
where: { id: orderId, userId: req.user.id }
})
if (!order) return res.status(404).json({ error: 'Not found' })
Mass assignment — allowlist fields only:
// ❌ is_admin can be injected via req.body
await prisma.user.update({ where: { id }, data: req.body })
// ✅ extract approved fields
const { name, email, bio } = req.body
await prisma.user.update({ where: { id }, data: { name, email, bio } })
All endpoints protected — no forgotten @require_auth / @require_admin.
// ❌ innerHTML with user data
element.innerHTML = userInput
// ✅ textContent
element.textContent = userInput
// ❌ React dangerouslySetInnerHTML
<div dangerouslySetInnerHTML={{ __html: userContent }} />
// ✅ sanitized
<div dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(userContent) }} />
Safe by default: React {variable}, Vue {{ variable }}, Django {{ variable }}.
Flag if: dangerouslySetInnerHTML, v-html="userInput", {{ variable|safe }}, {% autoescape off %}.
Rate limiting required on auth endpoints:
rateLimit({ windowMs: 60_000, max: 5 }) // login: 5/minute
rateLimit({ windowMs: 3_600_000, max: 3 }) // password reset: 3/hour
CORS — never * with credentials:
// ❌ cors({ origin: '*' })
// ✅ cors({ origin: 'https://app.example.com', credentials: true })
JWT — verify algorithm explicitly:
// ❌
jwt.verify(token, secret, { algorithms: ['none'] })
// ✅
jwt.verify(token, process.env.JWT_SECRET!, { algorithms: ['HS256'] })
Error response — hide internals:
// ❌ expose internal detail
res.status(500).json({ error: e.message, stack: e.stack })
// ✅
logger.error(e)
res.status(500).json({ error: 'Internal server error' })
Response filtering — no sensitive fields:
// ❌ passwordHash in response
res.json(user)
// ✅
const { passwordHash, ...safe } = user
res.json(safe)
Never log sensitive data:
// ❌ console.log(`Login: ${username}, password: ${password}`)
// ❌ console.log(req.headers) // Authorization header logged
// ✅
logger.info(`Login attempt: ${username}`)
No hardcoded secrets:
// ❌ const API_KEY = "sk-abc123..."
// ✅ const API_KEY = process.env.API_KEY
Next.js — NEXT_PUBLIC_* only for public data:
# ❌ NEXT_PUBLIC_SECRET_KEY=sk-abc123
# ✅ SECRET_KEY=sk-abc123
Fail-closed — DENY on error:
// ❌ fail-open: allows on error
} catch (e) { return true }
// ✅ fail-closed: denies on error
} catch (e) { return false }
Never swallow security exception:
// ❌ validation skipped
try { validateInput(x) } catch { }
// ✅ proper handling
try { validateInput(x) } catch (e) {
return res.status(400).json({ error: 'Invalid input' })
}
Async errors always caught:
// ❌ uncaught: crashes server
const data = await fetchData()
// ✅ caught
try { ... } catch (e) {
res.status(500).json({ error: 'Internal error' })
}
TypeScript ≠ runtime validation:
// ❌ cast without validation
const body = await req.json() as CreateUserBody
// ✅ runtime validation
const body = schema.parse(await req.json()) // Zod/Joi
Validate: request body, params, query, headers, cookies. Reject with 400 if invalid.
NEXT_PUBLIC_* only for public-safe datadynamic = 'force-static')public/ directoryMandatory before Phase 3. Review often runs once per phase — don't miss anything.
Only after self-review, create report.
## Code Review Report
**Task/Phase:** [name]
**Scope:** [files reviewed]
**Status:** [💥 BLOCKER | 🔴 MAJOR | ⚠️ MINOR | ✅ PASS]
### Summary
| Category | Count |
|----------|-------|
| 💥 Blocker | X |
| 🔴 Major | X |
| ⚠️ Minor | X |
| ℹ️ Info | X |
### 💥 BLOCKER
#### [CR-XX] [Problem Name]
- **File:** `path/to/file.ts:42`
- **Issue:** [concrete description]
- **Fix:** [correct code]
### 🔴 MAJOR
#### [CR-XX / SEC-XX] [Problem Name]
- **File:** `path/to/file.ts:100`
- **Issue:** [description]
- **Recommend:** [what to do]
### ⚠️ MINOR / ℹ️ INFO
- **[CR-XX]** `file.ts:50` — [brief]
### Checklist
| # | Item | Status |
|---|------|--------|
| CR-01 | Hallucination | ✅ |
| CR-02 | Runtime Errors | ✅ |
| ... | ... | ... |
| SEC-01 | Injection | ✅ |
| SEC-02 | Authentication | 🔴 |
| ... | ... | ... |
💥 BLOCKER → Fix now. Don't report done.
🔴 MAJOR → Fix before next phase.
⚠️ MINOR → Report to user, ask.
ℹ️ INFO → Backlog.
---