code-review

ServiceNow Code Review Checklist

Use this checklist when reviewing ServiceNow server-side code (Business Rules, Script Includes, Scheduled Jobs, etc.).

1. ES5 Compliance (CRITICAL)

// CHECK FOR THESE ES6+ VIOLATIONS:
const x = 5;           // ❌ Use var
let items = [];        // ❌ Use var
() => {}               // ❌ Use function()
`template ${var}`      // ❌ Use 'string ' + var
for (x of arr)         // ❌ Use traditional for loop
{a, b} = obj           // ❌ Use obj.a, obj.b

Action: Flag ALL ES6+ syntax as CRITICAL errors.

2. Security Issues

2.1 SQL/GlideRecord Injection

// ❌ DANGEROUS - User input directly in query
gr.addEncodedQuery(userInput)
gr.addQuery("field", userInput) // OK if validated

// ✅ SAFE - Validate and sanitize
var safeInput = new GlideSysAttachment().cleanFileName(userInput)
gr.addQuery("field", safeInput)

2.2 Cross-Site Scripting (XSS)

// ❌ DANGEROUS - Unescaped output
gs.addInfoMessage(userInput)

// ✅ SAFE - Escape HTML
gs.addInfoMessage(GlideStringUtil.escapeHTML(userInput))

2.3 Access Control

// ❌ MISSING - No ACL check
var gr = new GlideRecord("sys_user")
gr.get(userProvidedSysId)

// ✅ SAFE - Check permissions
var gr = new GlideRecord("sys_user")
if (gr.get(userProvidedSysId) && gr.canRead()) {
  // Process record
}

2.4 Sensitive Data Exposure

// ❌ DANGEROUS - Logging sensitive data
gs.info("Password: " + password)
gs.info("API Key: " + apiKey)

// ✅ SAFE - Never log credentials
gs.info("Authentication attempt for user: " + username)

3. Performance Issues

3.1 Queries in Loops

// ❌ SLOW - N+1 query problem
for (var i = 0; i < userIds.length; i++) {
  var gr = new GlideRecord("sys_user")
  gr.get(userIds[i]) // Query for each user!
}

// ✅ FAST - Single query
var gr = new GlideRecord("sys_user")
gr.addQuery("sys_id", "IN", userIds.join(","))
gr.query()
while (gr.next()) {}

3.2 Missing setLimit()