itk-dev · cableman · Jun 11, 2026
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Dedicated security-review step in `itkdev-review-php` and
+  `itkdev-review-javascript`, mirroring the one `itkdev-review-python` already
+  had: ten language-specific categories, a per-finding format (severity,
+  location, vulnerability type, description, proof of concept, remediation),
+  and a closing prioritized remediation plan. Skill descriptions updated to
+  advertise it.
+- Drupal deep checks in `itkdev-review-php` (`\Drupal::` static calls in
+  services, access checks on routes/entities, render-array escaping and `t()`
+  placeholders, database API placeholders, deprecated Drupal 10/11 APIs,
+  business logic in hooks), with a soft reference to the `itkdev-drupal` skill
+  for deeper checks. Framework list reordered to Symfony, Drupal, Laravel,
+  plain PHP to match how ITK Dev actually works.
+- Python, YAML, and Shell rows in the `itkdev-review-comments` comment-syntax
+  table.
+- Security parity in `itkdev-review-python` itself: XSS (`mark_safe`, Jinja2
+  autoescape), CSRF (`@csrf_exempt`), template injection (SSTI), and
+  weak-randomness/crypto checks in the Critical checklist; HTTP-timeout and
+  `assert`-for-validation deep checks; quick-reference rows for weak tokens,
+  SSTI, and missing timeouts.
+
+### Changed
+
+- The `itkdev-code-review` agent now defines the explicit severity mapping from
+  the review skills' four tiers to the report's three (Critical → Critical,
+  High → Warning, Medium/Low → Suggestion), and only flags `error_log` /
+  `console.warn` as debug code when they are clearly leftover debugging rather
+  than deliberate logging.
+- Tone Guidelines, severity headers, and the output-format template are now
+  word-for-word identical across the three language review skills, so future
+  drift shows up in diffs.
+
 ### Changed
 
 - **BREAKING:** The single `itkdev-skills` plugin has been split into three

diff --git a/plugins/itkdev-code-quality-and-review/.claude-plugin/plugin.json b/plugins/itkdev-code-quality-and-review/.claude-plugin/plugin.json
@@ -1,7 +1,7 @@
 {
   "name": "itkdev-code-quality-and-review",
   "description": "ITK Dev code quality and review: code review agent and per-language review skills (PHP, Python, JavaScript), comment review, and standards validation",
-  "version": "0.1.1",
+  "version": "0.2.0",
   "author": {
     "name": "ITK Dev",
     "email": "itkdev@mkb.aarhus.dk"

diff --git a/plugins/itkdev-code-quality-and-review/agents/itkdev-code-review.md b/plugins/itkdev-code-quality-and-review/agents/itkdev-code-review.md
@@ -90,9 +90,11 @@ Apply Drupal-specific checks only when a Drupal project is detected.
 ### General Checks (all projects)
 
 **Debug code** — Flag any leftover debugging statements:
-- PHP: `var_dump`, `print_r`, `dd()`, `dump()`, `dpm()`, `dsm()`, `kint()`, `error_log`
-- JavaScript/TypeScript: `console.log`, `console.debug`, `console.warn`, `debugger`
+- PHP: `var_dump`, `print_r`, `dd()`, `dump()`, `dpm()`, `dsm()`, `kint()`
+- JavaScript/TypeScript: `console.log`, `console.debug`, `debugger`
 - Twig: `{{ dump() }}`, `{{ kint() }}`
+- `error_log` (PHP) and `console.warn` (JS) only when they are clearly leftover debugging (dumps
+  of local variables, "here" markers) — not deliberate logging
 
 **Hardcoded secrets** — Flag potential secrets or credentials:
 - Hardcoded passwords, API keys, tokens
@@ -121,8 +123,9 @@ the matching `itkdev-review-<lang>` skill. Apply only the skills whose languages
 | `.js`, `.jsx`, `.ts`, `.tsx`, `.mjs`, `.cjs` | `itkdev-review-javascript` |
 
 Each skill provides the prioritized Critical/High/Medium/Low checklist, language-specific deep
-checks, and a security pass for that language — map its findings into this report's Critical /
-Warning / Suggestion severities.
+checks, and a security pass for that language. Map its findings into this report's severities as:
+skill **Critical → Critical**, **High → Warning**, **Medium / Low → Suggestion** (include Low
+findings only when they are genuinely worth the author's time).
 
 **Twig:**
 - Unescaped output (`|raw` filter) without justification

diff --git a/plugins/itkdev-code-quality-and-review/skills/itkdev-review-comments/SKILL.md b/plugins/itkdev-code-quality-and-review/skills/itkdev-review-comments/SKILL.md
@@ -33,8 +33,11 @@ Use the idiomatic comment syntax for each language:
 |------------|-----------------|----------------------|
 | PHP        | `//`            | `/** */`             |
 | JavaScript | `//`            | `/** */` (JSDoc)     |
+| Python     | `#`             | `"""docstring"""`    |
 | Twig       | `{# #}`         | `{# #}` (multiline) |
 | CSS / SCSS | `/* */`         | `/* */`              |
+| YAML       | `#`             | `#` (no block form)  |
+| Shell      | `#`             | `#` (no block form)  |
 
 For languages not listed, follow the language's standard convention.
 

diff --git a/plugins/itkdev-code-quality-and-review/skills/itkdev-review-javascript/SKILL.md b/plugins/itkdev-code-quality-and-review/skills/itkdev-review-javascript/SKILL.md
@@ -1,7 +1,7 @@
 ---
 name: itkdev-review-javascript
 user-invocable: true
-description: JavaScript/TypeScript code review for ITK Dev projects (browser, Node.js, frontend frameworks). Use when reviewing, auditing, critiquing, or giving feedback on JS/TS code — including snippets, files, or pull requests. Covers security (XSS, prototype pollution, eval, injection), correctness, async/promise error handling, type safety (any usage), and style (var vs const/let). Triggers for "review my JS/TS", "what's wrong with this", "security review", or when JavaScript/TypeScript code is pasted for feedback.
+description: JavaScript/TypeScript code review for ITK Dev projects (browser, Node.js, frontend frameworks). Use when reviewing, auditing, critiquing, or giving feedback on JS/TS code — including snippets, files, or pull requests. Covers a dedicated security review (XSS, prototype pollution, eval, injection, secrets in bundles, SSRF, supply chain, misconfiguration) plus correctness, async/promise error handling, type safety (any usage), and style (var vs const/let). Triggers for "review my JS/TS", "security review", "what's wrong with this", or when JavaScript/TypeScript code is pasted for feedback.
 ---
 
 # JavaScript / TypeScript Code Review
@@ -91,7 +91,42 @@ Work through each category. Skip irrelevant ones and say so briefly.
 - **`innerHTML` and friends**: justified and sanitized?
 - **Dependency risks**: known-vulnerable packages; flag any imports worth checking.
 
-### Step 4 — Format the output
+### Step 4 — Dedicated security review
+
+When the user asks for a **security review** (or a full audit), act as a senior application
+security engineer and cover at minimum these categories:
+
+1. **XSS sinks** — `innerHTML`, `dangerouslySetInnerHTML`, `document.write`, unescaped
+   server-rendered output.
+2. **Prototype pollution** — untrusted objects merged into `__proto__` / `constructor` / shared
+   objects.
+3. **Dynamic code execution** — `eval()`, `new Function()`, string arguments to
+   `setTimeout`/`setInterval`.
+4. **Injection** — shell commands, SQL, or templates built from user input (Node.js).
+5. **Secrets & credentials** — keys/tokens hardcoded or shipped in client bundles, insecure
+   env-variable usage.
+6. **Origin handling** — `postMessage` without origin checks, permissive CORS, unsafe
+   `window.open` targets.
+7. **SSRF (Node.js)** — user-controlled URLs passed to `fetch`/`http` without allowlisting.
+8. **Dependency & supply-chain risks** — known-vulnerable packages, typosquats, install scripts
+   worth checking.
+9. **Cryptography** — `Math.random()` for tokens/IDs, homegrown crypto instead of Web Crypto /
+   `node:crypto`.
+10. **Security misconfigurations** — missing CSP, cookies without `HttpOnly`/`Secure`/`SameSite`,
+    debug endpoints exposed.
+
+For each security finding, provide:
+
+- **Severity**: Critical / High / Medium / Low / Informational
+- **Location**: file and line number(s)
+- **Vulnerability type**: e.g. SQLi, XSS, SSRF, hardcoded secret
+- **Description**: what the issue is and why it's dangerous
+- **Proof of concept**: a brief example of how it could be exploited
+- **Remediation**: a specific code fix or mitigation
+
+After the findings, provide a **prioritized remediation plan** — what to fix first and why.
+
+### Step 5 — Format the output
 
 ```
 ## Code Review: [filename or description]
@@ -115,11 +150,14 @@ One paragraph: what the code does, overall impression, most important theme.
 [2–5 genuine positives]
 ```
 
+For a security-focused review, use the per-finding shape from Step 4 and end with the prioritized
+remediation plan.
+
 ## Tone Guidelines
 
 - **Be direct, not harsh.** Explain the fix, don't just condemn the code.
 - **Always explain why**, not just what to change.
-- **Distinguish opinion from fact.** "This *could* be memoized" vs "This *is* an XSS sink."
+- **Distinguish opinion from fact.** "This *could* be extracted" vs "This *is* vulnerable."
 - **Prioritize ruthlessly.** Group issues that share a root cause.
 - **Acknowledge good work.** "What's Working Well" is not optional filler.
 

diff --git a/plugins/itkdev-code-quality-and-review/skills/itkdev-review-php/SKILL.md b/plugins/itkdev-code-quality-and-review/skills/itkdev-review-php/SKILL.md
@@ -1,22 +1,22 @@
 ---
 name: itkdev-review-php
 user-invocable: true
-description: PHP code review for ITK Dev projects (Laravel, Symfony, plain PHP). Use when reviewing, auditing, critiquing, or giving feedback on PHP code — including snippets, files, or pull requests. Covers security (SQL injection, XSS, mass assignment, file inclusion, command injection), correctness, performance (N+1 queries), type safety, and PSR-12 style. Triggers for "review my PHP", "what's wrong with this", "security review", or when PHP code is pasted for feedback.
+description: PHP code review for ITK Dev projects (Symfony, Drupal, Laravel, plain PHP). Use when reviewing, auditing, critiquing, or giving feedback on PHP code — including snippets, files, or pull requests. Covers a dedicated security review (SQL injection, XSS, mass assignment, file inclusion, command injection, weak crypto, insecure deserialization, misconfiguration) plus correctness, performance (N+1 queries), type safety, and PSR-12 style. Triggers for "review my PHP", "security review", "what's wrong with this", or when PHP code is pasted for feedback.
 ---
 
 # PHP Code Review
 
-Produce structured, prioritized code reviews for PHP codebases (Laravel, Symfony, plain PHP).
-Reviews should be developer-friendly: specific, actionable, and ranked so the author knows what
-to fix first.
+Produce structured, prioritized code reviews for PHP codebases (Symfony, Drupal, Laravel, plain
+PHP). Reviews should be developer-friendly: specific, actionable, and ranked so the author knows
+what to fix first.
 
 ## Review Process
 
 ### Step 1 — Gather context
 
 Before diving in, identify:
 
-- **Framework**: Laravel, Symfony, or raw PHP.
+- **Framework**: Symfony, Drupal, Laravel, or raw PHP.
 - **Purpose**: Web endpoint? CLI tool? Library? Queue worker?
 - **Scope**: Quick feedback, security focus, full audit, or pre-merge PR review?
 - **Standards**: Apply PSR-12 and any project conventions the user mentions; otherwise use
@@ -87,19 +87,57 @@ Work through each category. Skip irrelevant ones and say so briefly.
 - **`eval()`**: almost always a red flag.
 - **`include`/`require` with variables**: potential file inclusion vulnerability.
 - **`isset()` vs `empty()` vs `??`**: correct null-handling idiom used?
+- **Symfony specifics**:
+  - Services autowired correctly?
+  - Doctrine entities: fetch type appropriate (LAZY vs EAGER)?
+  - Forms: CSRF protection enabled?
+- **Drupal specifics**:
+  - `\Drupal::` static calls inside services? Inject dependencies instead.
+  - Access checks on routes and entity operations (`_permission` requirements, `$entity->access()`)?
+  - Output escaped in render arrays? `t()` placeholders (`@`, `%`, `:`) used instead of string
+    concatenation?
+  - Database API with placeholders — no string-concatenated SQL?
+  - APIs deprecated in Drupal 10/11?
+  - Business logic in hooks (prefer services)?
+  - For deeper Drupal checks, use the `itkdev-drupal` skill (from the
+    `itkdev-scaffolding-and-templates` plugin) when it is installed.
 - **Laravel specifics**:
   - Mass assignment protection (`$fillable` / `$guarded` on models).
   - Policies and Gates used for authorization?
   - Jobs/Queues idempotent? Handle retries?
   - Eloquent N+1: using `with()` for relationships?
   - Config via `config()` in app code (not `env()` outside config files).
-- **Symfony specifics**:
-  - Services autowired correctly?
-  - Doctrine entities: fetch type appropriate (LAZY vs EAGER)?
-  - Forms: CSRF protection enabled?
 - **PSR compliance**: PSR-4 autoloading, PSR-12 code style.
 
-### Step 4 — Format the output
+### Step 4 — Dedicated security review
+
+When the user asks for a **security review** (or a full audit), act as a senior application
+security engineer and cover at minimum these categories:
+
+1. **Injection** — SQL, command, LDAP, template injection.
+2. **Authentication & authorization** — broken auth, missing access controls, insecure sessions.
+3. **Secrets & credentials** — hardcoded keys/passwords/tokens, insecure env-variable usage.
+4. **Cryptography** — weak algorithms, hardcoded salts/IVs, improper key management.
+5. **Input validation** — missing sanitization, type juggling, path traversal, file inclusion.
+6. **Dependency risks** — known-vulnerable Composer packages (flag any worth checking).
+7. **Insecure deserialization** — `unserialize()` on untrusted input, Phar deserialization.
+8. **Error handling & logging** — sensitive data in logs, verbose errors exposed to users.
+9. **Race conditions & TOCTOU** — filesystem or shared-state issues.
+10. **Security misconfigurations** — `display_errors` in production, debug mode on, permissive
+    CORS, weak TLS settings.
+
+For each security finding, provide:
+
+- **Severity**: Critical / High / Medium / Low / Informational
+- **Location**: file and line number(s)
+- **Vulnerability type**: e.g. SQLi, XSS, SSRF, hardcoded secret
+- **Description**: what the issue is and why it's dangerous
+- **Proof of concept**: a brief example of how it could be exploited
+- **Remediation**: a specific code fix or mitigation
+
+After the findings, provide a **prioritized remediation plan** — what to fix first and why.
+
+### Step 5 — Format the output
 
 ```
 ## Code Review: [filename or description]
@@ -123,13 +161,14 @@ One paragraph: what the code does, overall impression, most important theme.
 [2–5 genuine positives]
 ```
 
+For a security-focused review, use the per-finding shape from Step 4 and end with the prioritized
+remediation plan.
+
 ## Tone Guidelines
 
-- **Be direct, not harsh.** "This query is vulnerable to SQL injection — use a prepared statement
-  instead", not "This is terrible code."
+- **Be direct, not harsh.** Explain the fix, don't just condemn the code.
 - **Always explain why**, not just what to change.
-- **Distinguish opinion from fact.** "This *could* be extracted into a helper" vs "This query *is*
-  vulnerable."
+- **Distinguish opinion from fact.** "This *could* be extracted" vs "This *is* vulnerable."
 - **Prioritize ruthlessly.** Group issues that share a root cause.
 - **Acknowledge good work.** "What's Working Well" is not optional filler.
 

diff --git a/plugins/itkdev-code-quality-and-review/skills/itkdev-review-python/SKILL.md b/plugins/itkdev-code-quality-and-review/skills/itkdev-review-python/SKILL.md
@@ -1,7 +1,7 @@
 ---
 name: itkdev-review-python
 user-invocable: true
-description: Python code review for ITK Dev projects (Django, Flask, FastAPI, scripts, data pipelines). Use when reviewing, auditing, critiquing, or giving feedback on Python code — including snippets, files, or pull requests. Covers a dedicated security review (injection, broken auth, secrets, weak crypto, insecure deserialization, SSRF, path traversal, misconfiguration) plus correctness, performance, type safety, and PEP 8 style. Triggers for "review my Python", "security review", "what's wrong with this", or when Python code is pasted for feedback.
+description: Python code review for ITK Dev projects (Django, Flask, FastAPI, scripts, data pipelines). Use when reviewing, auditing, critiquing, or giving feedback on Python code — including snippets, files, or pull requests. Covers a dedicated security review (injection, XSS, CSRF, broken auth, secrets, weak crypto, insecure deserialization, SSRF, path traversal, misconfiguration) plus correctness, performance (N+1 queries), type safety, and PEP 8 style. Triggers for "review my Python", "security review", "what's wrong with this", or when Python code is pasted for feedback.
 ---
 
 # Python Code Review
@@ -42,6 +42,13 @@ Work through each category. Skip irrelevant ones and say so briefly.
 - [ ] Sensitive data exposure: hardcoded API keys, passwords, tokens; secrets in logs or errors.
 - [ ] Path traversal: user-controlled file paths without sanitization.
 - [ ] SSRF: user-controlled URLs passed to `requests`/`urllib` without allowlisting.
+- [ ] XSS: `mark_safe()` / `|safe` (Django), autoescape disabled or `Markup()` (Jinja2), HTML
+      built by hand from user input.
+- [ ] CSRF: `@csrf_exempt` without justification (Django); state-changing Flask routes without
+      CSRF protection.
+- [ ] Template injection (SSTI): `render_template_string()` / `jinja2.Template()` on user input.
+- [ ] Weak crypto/randomness: `random` for tokens or secrets (use `secrets`); `md5`/`sha1` for
+      passwords (use framework hashers / `bcrypt` / `scrypt`).
 
 **Correctness**
 
@@ -86,9 +93,11 @@ Work through each category. Skip irrelevant ones and say so briefly.
 - **`global` / `nonlocal` overuse**: sign of poor state design.
 - **`__all__`**: defined if the module is a public API?
 - **Context managers**: `with` used for files, locks, DB sessions, HTTP clients?
+- **HTTP timeouts**: `requests`/`httpx` calls without `timeout=` hang forever.
+- **`assert` for validation**: stripped under `python -O` — use explicit checks that raise.
 - **Django specifics**: raw SQL via `raw()` / `cursor.execute()` vs ORM; `select_related()` /
   `prefetch_related()` for N+1; `@login_required` / permissions on views; no business logic in
-  signals.
+  signals; `mark_safe` / `|safe` and `@csrf_exempt` usages justified?
 - **Flask/FastAPI specifics**: input validation (Pydantic, WTForms, marshmallow); thin route
   handlers (business logic in services); proper HTTP status codes.
 - **Data pipelines / scripts**: large files streamed/chunked rather than loaded fully into memory;
@@ -167,3 +176,6 @@ remediation plan.
 | Subprocess injection | `subprocess.run(f"ls {user_input}", shell=True)` | `subprocess.run(["ls", user_input])` |
 | Insecure deserialization | `pickle.loads(user_data)` | Use JSON, or validate the source |
 | Unsafe YAML | `yaml.load(data)` | `yaml.safe_load(data)` |
+| Weak token | `random.random()` | `secrets.token_urlsafe()` |
+| SSTI | `render_template_string(user_input)` | Render a fixed template, pass data as variables |
+| Missing timeout | `requests.get(url)` | `requests.get(url, timeout=10)` |