Improve code-security rules with safer examples and updated guidance

Author: Vibhuti Patel

skills/code-security.zip

@@ -24,14 +24,13 @@ function getUserData(token) {
 }
 ```
 
-**Correct (JavaScript jsonwebtoken - verify before decode):**
+**Correct (JavaScript jsonwebtoken - use verify which returns decoded payload):**
 
 ```javascript
 const jwt = require('jsonwebtoken');
 
 function getUserData(token, secretKey) {
-  jwt.verify(token, secretKey);
-  const decoded = jwt.decode(token, true);
+  const decoded = jwt.verify(token, secretKey);
   if (decoded.isAdmin) {
     return getAdminData();
   }

skills/code-security/rules/authentication-jwt.md

@@ -17,15 +17,20 @@ def unsafe(request):
     eval(code)
 ```
 
-**Correct (Python - static eval with hardcoded strings):**
+**Correct (Python - avoid eval entirely, use safe alternatives):**
 
 ```python
-eval("x = 1; x = x + 2")
+import ast
 
-blah = "import requests; r = requests.get('https://example.com')"
-eval(blah)
+def safe_parse(user_expr):
+    # ast.literal_eval only allows literals (strings, numbers, tuples, lists, dicts, booleans, None)
+    return ast.literal_eval(user_expr)
+
+# For math expressions, use a purpose-built parser instead of eval
 ```
 
+> **Note:** Avoid `eval()`/`exec()` entirely. Even with hardcoded strings, it normalizes a dangerous pattern. Use `ast.literal_eval()` for parsing data literals, or purpose-built parsers for expressions.
+
 **Incorrect (JavaScript - eval with dynamic content):**
 
 ```javascript
@@ -38,15 +43,20 @@ function evalSomething(something) {
 }
 ```
 
-**Correct (JavaScript - static eval strings):**
+**Correct (JavaScript - avoid eval, use safe alternatives):**
 
 ```javascript
-eval('var x = "static strings are okay";');
+// Instead of eval for JSON parsing:
+const data = JSON.parse(jsonString);
+
+// Instead of eval for dynamic property access:
+const value = obj[propertyName];
 
-const constVar = "function staticStrings() { return 'static strings are okay';}";
-eval(constVar);
+// Instead of eval for math: use a sandboxed expression parser
 ```
 
+> **Note:** There is almost never a legitimate reason to use `eval()`. Use `JSON.parse()`, computed property access, or a sandboxed parser. Avoid `new Function()` as well — it executes arbitrary code just like `eval()`.
+
 **Incorrect (Java - ScriptEngine injection):**
 
 ```java
@@ -94,34 +104,35 @@ a = %q{def hello() "Hello there!" end}
 Thing.module_eval(a)
 ```
 
-**Incorrect (PHP - dangerous exec functions with user input):**
+**Incorrect (PHP - code injection via eval/assert):**
 
 ```php
-exec($user_input);
-passthru($user_input);
-$output = shell_exec($user_input);
-$output = system($user_input, $retval);
+$code = $_GET['code'];
+eval($code);
 
-$username = $_COOKIE['username'];
-exec("wto -n \"$username\" -g", $ret);
+$input = $_POST['input'];
+assert($input);  // assert() evaluates strings as code in PHP < 8.0
 ```
 
-**Correct (PHP - static commands with escapeshellarg):**
+**Correct (PHP - avoid eval, use structured alternatives):**
 
 ```php
-exec('whoami');
+// Instead of eval for dynamic config, use a data format:
+$config = json_decode(file_get_contents('config.json'), true);
 
-$fullpath = $_POST['fullpath'];
-$filesize = trim(shell_exec('stat -c %s ' . escapeshellarg($fullpath)));
+// Instead of eval for templates, use a template engine (Twig, Blade)
 ```
 
+> **Note:** `exec()`/`shell_exec()`/`system()` are OS command execution — see the command-injection rule for those. This rule covers code evaluation via `eval()`, `assert()`, `preg_replace` with `/e`, and similar.
+
 ## Key Prevention Patterns
 
-1. **Never pass user input to eval/exec functions** - Treat all user input as untrusted
-2. **Use hardcoded strings** - Static strings in eval/exec calls are safe
-3. **Validate and sanitize** - If dynamic code execution is unavoidable, validate against a strict whitelist
-4. **Use parameterized alternatives** - Many languages offer safer alternatives to eval
-5. **Escape shell arguments** - Use escapeshellarg() in PHP or equivalent functions
+1. **Avoid eval/exec entirely** - Use safer alternatives (`JSON.parse`, `ast.literal_eval`, template engines, computed property access)
+2. **Never pass user input to code evaluation functions** - Treat all user input as untrusted
+3. **If dynamic code execution is unavoidable** - Validate against a strict allowlist and sandbox the execution
+4. **Use parameterized alternatives** - Most languages offer structured APIs that eliminate the need for eval
+
+> For OS command execution (`exec`, `shell_exec`, `system`) and shell-escaping (`escapeshellarg`), see the **command-injection** rule.
 
 ## References
 

skills/code-security/rules/code-injection.md

@@ -58,6 +58,14 @@ finally:
     break  # Suppresses the exception!
 ```
 
+**CORRECT** - Let the exception propagate; use finally only for cleanup:
+```python
+try:
+    raise ValueError()
+finally:
+    cleanup()  # Cleanup runs, exception still propagates
+```
+
 ### Raising Non-Exceptions
 
 **INCORRECT**:
@@ -106,7 +114,9 @@ return `value is ${x}`
 
 ### Loop Pointer Export
 
-Loop variables are shared across iterations.
+> **Note:** Go 1.22+ scopes loop variables per-iteration, fixing this issue. The pattern below applies to Go < 1.22.
+
+Loop variables are shared across iterations (Go < 1.22).
 
 **INCORRECT**:
 ```go
@@ -135,7 +145,15 @@ bigValue, _ := strconv.Atoi("2147483648")
 value := int16(bigValue)  // Overflow!
 ```
 
-**CORRECT**: Use `strconv.ParseInt` with correct bit size.
+**CORRECT**:
+```go
+parsed, err := strconv.ParseInt("2147483648", 10, 32)
+if err != nil {
+    // handles out-of-range and invalid syntax
+    log.Fatal(err)
+}
+value := int32(parsed)
+```
 
 ---
 
@@ -180,7 +198,12 @@ int i = atoi(buf);
 
 **CORRECT**:
 ```c
-long l = strtol(buf, NULL, 10);
+char *endptr;
+errno = 0;
+long l = strtol(buf, &endptr, 10);
+if (errno != 0 || endptr == buf || *endptr != '\0') {
+    // handle conversion error
+}
 ```
 
 ---

skills/code-security/rules/correctness.md

@@ -42,30 +42,64 @@ def my_view(request):
 
 #### Missing CSRF Middleware
 
-**Incorrect (Express app without csurf middleware):**
+> **⚠ Deprecation Notice:** The `csurf` npm package is **deprecated** and should not be used in new projects. Use a maintained alternative such as `csrf-csrf` (Double-Submit Cookie pattern) or `csrf-sync` (Synchronizer Token pattern).
+
+**Incorrect (Express app without CSRF protection):**
 ```javascript
-var express = require('express')
-var bodyParser = require('body-parser')
+const express = require('express')
+const bodyParser = require('body-parser')
 
-var app = express()
+const app = express()
 
 app.post('/process', bodyParser.urlencoded({ extended: false }), function(req, res) {
     res.send('data is being processed')
 })
 ```
 
-**Correct (include csurf middleware):**
+**Correct — Option A: `csrf-csrf` (Double-Submit Cookie pattern):**
+```javascript
+const express = require('express')
+const cookieParser = require('cookie-parser')
+const { doubleCsrf } = require('csrf-csrf')
+
+const { doubleCsrfProtection, generateToken } = doubleCsrf({
+  getSecret: () => process.env.CSRF_SECRET,
+  cookieName: '__Host-psifi.x-csrf-token',
+  cookieOptions: { sameSite: 'strict', secure: true },
+})
+
+const app = express()
+app.use(cookieParser())
+app.use(doubleCsrfProtection)
+
+// Generate a token for forms/SPA clients
+app.get('/csrf-token', (req, res) => {
+  res.json({ token: generateToken(req, res) })
+})
+```
+
+**Correct — Option B: `csrf-sync` (Synchronizer Token pattern):**
 ```javascript
-var csrf = require('csurf')
-var express = require('express')
+const express = require('express')
+const { csrfSync } = require('csrf-sync')
 
-var app = express()
-app.use(csrf({ cookie: true }))
+const { csrfSynchronisedProtection, generateToken } = csrfSync()
+
+const app = express()
+app.use(csrfSynchronisedProtection)
 ```
 
+**Additional defenses (complement token-based CSRF protection):**
+- Set `SameSite=Strict` or `SameSite=Lax` on session cookies.
+- Validate `Sec-Fetch-Site` / `Origin` headers (Fetch Metadata) to reject cross-origin requests at the edge.
+
 **References:**
-- [csurf npm package](https://www.npmjs.com/package/csurf)
+- [csrf-csrf (Double-Submit Cookie)](https://www.npmjs.com/package/csrf-csrf)
+- [csrf-sync (Synchronizer Token)](https://www.npmjs.com/package/csrf-sync)
+- [csurf — deprecated](https://www.npmjs.com/package/csurf) *(do not use in new projects)*
 - [OWASP CSRF Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html)
+- [OWASP Fetch Metadata / Resource Isolation Policy](https://web.dev/articles/fetch-metadata)
+- [MDN SameSite Cookies](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value)
 
 ---
 

skills/code-security/rules/csrf.md

@@ -16,7 +16,7 @@ The last user in the container should not be 'root'. If an attacker gains contro
 **Incorrect:**
 
 ```dockerfile
-FROM busybox
+FROM debian:bookworm
 RUN apt-get update && apt-get install -y some-package
 USER appuser
 USER root
@@ -25,7 +25,7 @@ USER root
 **Correct:**
 
 ```dockerfile
-FROM busybox
+FROM debian:bookworm
 USER root
 RUN apt-get update && apt-get install -y some-package
 USER appuser
@@ -102,15 +102,17 @@ services:
       - /var/run/docker.sock:/var/run/docker.sock
 ```
 
-**Correct:**
+**Correct (use a named volume instead of host mounts):**
 
 ```yaml
 version: "3.9"
 services:
   worker:
     image: my-worker-image:1.0
     volumes:
-      - /tmp/data:/tmp/data
+      - worker-data:/app/data
+volumes:
+  worker-data:
 ```
 
 ### Arbitrary Container Run (Python Docker SDK)

skills/code-security/rules/docker.md

@@ -68,16 +68,22 @@ function hashPassword(pwtext) {
 }
 ```
 
-**Correct (SHA256 hashing):**
+**Correct (bcrypt for password hashing):**
 
 ```javascript
-const crypto = require("crypto");
+const bcrypt = require("bcrypt");
 
-function hashPassword(pwtext) {
-    return crypto.createHash("sha256").update(pwtext).digest("hex");
+async function hashPassword(pwtext) {
+    return bcrypt.hash(pwtext, 12);
+}
+
+async function verifyPassword(pwtext, hash) {
+    return bcrypt.compare(pwtext, hash);
 }
 ```
 
+> **Note:** SHA-256/SHA-512 are fine for data integrity but too fast for password hashing. Use bcrypt, scrypt, or Argon2 for passwords.
+
 ---
 
 ### Java
@@ -94,21 +100,23 @@ byte[] hash = md5.digest();
 MessageDigest sha1 = MessageDigest.getInstance("SHA-1");
 ```
 
-**Correct (SHA-512 hashing):**
+**Correct (BCrypt for password hashing):**
 
 ```java
-import java.security.MessageDigest;
+import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
 
-MessageDigest sha512 = MessageDigest.getInstance("SHA-512");
-sha512.update(password.getBytes());
-byte[] hash = sha512.digest();
+BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
+String hash = encoder.encode(password);
+boolean matches = encoder.matches(password, hash);
 ```
 
+> **Note:** `MessageDigest` (SHA-256/SHA-512) is appropriate for data integrity checks but not for password storage. Use BCrypt, scrypt, or Argon2 for passwords.
+
 **Incorrect (DES cipher):**
 
 ```java
 Cipher c = Cipher.getInstance("DES/ECB/PKCS5Padding");
-c.init(Cipher.ENCRYPT_MODE, k, iv);
+c.init(Cipher.ENCRYPT_MODE, k);
 ```
 
 **Correct (AES with GCM):**