code-review

ServiceNow 代码审查

概述

此技能涵盖 ServiceNow 脚本的系统化代码审查，以识别并修复以下问题：

• 安全漏洞（注入、XSS、权限提升、数据泄露）
• 性能反模式（不必要的查询、N+1 问题、缺少查询限制）
• 平台最佳实践违规（API 误用、作用域问题、已弃用的方法）
• 可维护性问题（命名、文档、复杂度、死代码）
• 并发和事务安全问题
• 客户端性能和用户体验问题

适用场景： 在将新脚本部署到生产环境之前、代码审查过程中、排查性能问题时，或审计现有脚本的安全合规性时。

前提条件

• 角色： admin、security_admin，或具有脚本表读取权限的开发者
• 访问权限： sys_script、sys_script_include、sys_ui_script、sys_script_client 表
• 知识： ServiceNow 脚本 API、OWASP 安全原则、JavaScript 最佳实践
• 相关技能： 完成 development/code-assist，以了解代码生成背景

流程

步骤 1：检索待审查脚本

查询特定表的业务规则：

MCP 方式：

Tool: SN-Query-Table
Parameters:
  table_name: sys_script
  query: collection=incident^active=true
  fields: sys_id,name,collection,when,script,filter_condition,order
  limit: 50

REST 替代方式：

curl -u "$SN_USER:$SN_PASS" \
  "$SN_INSTANCE/api/now/table/sys_script?sysparm_query=collection=incident^active=true&sysparm_fields=sys_id,name,collection,when,script,filter_condition,order&sysparm_limit=50" \
  -H "Accept: application/json"

查询脚本包含项：

MCP 方式：

Tool: SN-Query-Table
Parameters:
  table_name: sys_script_include
  query: active=true^sys_updated_on>=javascript:gs.daysAgo(30)
  fields: sys_id,name,script,client_callable,access,api_name
  limit: 50

查询客户端脚本：

MCP 方式：

Tool: SN-Query-Table
Parameters:
  table_name: sys_script_client
  query: table=incident^active=true
  fields: sys_id,name,table,type,fieldname,script,ui_type
  limit: 50

步骤 2：安全审查清单

根据以下关键安全模式审查每个脚本：

2a. SQL/GlideRecord 注入：

VULNERABILITY: Unsanitized user input in queries
BAD:
  gr.addEncodedQuery(current.variables.user_input);
  gr.addQuery('name', request.getParameter('name'));

GOOD:
  // Validate and sanitize input
  var sanitized = GlideStringUtil.escapeHTML(input);
  // Use parameterized queries
  gr.addQuery('name', sanitizedName);
  // Validate against allowlist
  var validFields = ['name', 'number', 'state'];
  if (validFields.indexOf(fieldName) === -1) return;

2b. 跨站脚本攻击（XSS）：

VULNERABILITY: Unescaped output in UI scripts or HTML fields
BAD:
  element.innerHTML = current.short_description;
  gs.getMessage(userInput);

GOOD:
  element.textContent = current.short_description;
  var escaped = GlideStringUtil.escapeHTML(current.short_description);

2c. 权限提升：

VULNERABILITY: Missing role checks or using setWorkflow(false) inappropriately
BAD:
  current.setWorkflow(false); // Bypasses business rules and ACLs
  current.autoSysFields(false); // Hides audit trail

GOOD:
  // Only bypass when absolutely necessary and documented
  if (gs.hasRole('admin')) {
    current.setWorkflow(false); // REASON: Bulk import requires bypass
  }

2d. 敏感数据泄露：

VULNERABILITY: Logging or returning sensitive data
BAD:
  gs.log('User password: ' + password);
  gs.log('SSN: ' + current.social_security_number);

GOOD:
  gs.log('Authentication attempted for user: ' + username);
  // Never log PII, credentials, or tokens

步骤 3：性能审查清单

3a. 无界 GlideRecord 查询：

ANTI-PATTERN: Query without limit
BAD:
  var gr = new GlideRecord('incident');
  gr.addQuery('active', true);
  gr.query(); // Could return millions of records

GOOD:
  var gr = new GlideRecord('incident');
  gr.addQuery('active', true);
  gr.setLimit(1000); // Always set a reasonable limit
  gr.query();

3b. N+1 查询模式：

ANTI-PATTERN: Query inside a loop
BAD:
  var incidents = new GlideRecord('incident');
  incidents.query();
  while (incidents.next()) {
    var user = new GlideRecord('sys_user');
    user.get(incidents.caller_id); // Query per iteration!
    // process...
  }

GOOD:
  // Use dot-walking or join queries
  var incidents = new GlideRecord('incident');
  incidents.addQuery('active', true);
  incidents.query();
  while (incidents.next()) {
    var callerName = incidents.caller_id.getDisplayValue(); // Dot-walk
    // process...
  }

  // Or use GlideAggregate for counts
  var ga = new GlideAggregate('incident');
  ga.addQuery('active', true);
  ga.groupBy('assignment_group');
  ga.addAggregate('COUNT');
  ga.query();

3c. 客户端脚本中的同步 GlideRecord：

ANTI-PATTERN: Synchronous server calls from client
BAD:
  var gr = new GlideRecord('sys_user');
  gr.addQuery('sys_id', userId);
  gr.query(); // Synchronous - blocks browser!
  if (gr.next()) { ... }

GOOD:
  // Use GlideAjax for async server calls
  var ga = new GlideAjax('MyScriptInclude');
  ga.addParam('sysparm_name', 'getUserInfo');
  ga.addParam('sysparm_user_id', userId);
  ga.getXMLAnswer(function(answer) {
    // Process response asynchronously
  });

3d. 不必要的字段检索：

ANTI-PATTERN: Selecting all fields when only a few are needed
BAD:
  var gr = new GlideRecord('incident');
  gr.query(); // Returns ALL fields

GOOD:
  var gr = new GlideRecord('incident');
  gr.addQuery('active', true);
  gr.chooseWindow(0, 100);
  gr.setFields('sys_id,number,short_description,state');
  gr.query();

步骤 4：平台最佳实践审查

4a. 使用已弃用的 API：

DEPRECATED: Avoid these methods
BAD:
  current.getDisplayValue(); // Without field name - ambiguous
  gs.log(); // Use gs.info/warn/error instead
  Packages.java.lang.*; // Direct Java calls - unsupported

GOOD:
  current.getDisplayValue('short_description');
  gs.info('Message: {0}', current.number);

4b. 作用域违规：

ANTI-PATTERN: Accessing global scope from scoped app
BAD:
  // In scoped app, directly calling global
  var result = new IncidentUtils(); // Global script include

GOOD:
  // Use scoped API or cross-scope access
  var si = new global.IncidentUtils(); // Explicit scope prefix
  // Or use sn_ws for REST-based cross-scope communication

4c. 业务规则排序：

BEST PRACTICE: Proper ordering
- 100: Data defaults and initialization
- 200: Validation rules
- 300: Data transformation
- 400: Cross-table updates (after rules)
- 500+: Notifications and integrations (after rules)

4d. GlideRecord 的 getValue 与直接访问：

ANTI-PATTERN: Direct field access for comparisons
BAD:
  if (current.state == 6) { ... } // Returns GlideElement, not value

GOOD:
  if (current.getValue('state') == '6') { ... }
  // Or use typed comparison
  if (current.state.changesTo(6)) { ... }

步骤 5：客户端脚本审查

5a. DOM 操作：

ANTI-PATTERN: Direct DOM manipulation
BAD:
  document.getElementById('incident.short_description').style.color = 'red';
  $('incident.state').value = '6';

GOOD:
  g_form.setMandatory('short_description', true);
  g_form.addDecoration('short_description', 'color_red');
  g_form.setValue('state', '6');

5b. 缺少 isLoading 检查：

ANTI-PATTERN: onChange without isLoading guard
BAD:
  function onChange(control, oldValue, newValue, isLoading) {
    // Fires on every form load, causing unnecessary processing
    callServer(newValue);
  }

GOOD:
  function onChange(control, oldValue, newValue, isLoading) {
    if (isLoading || newValue === '') {
      return;
    }
    callServer(newValue);
  }

步骤 6：生成审查报告

将审查结果整理成结构化报告：

Code Review Report
================================================
Script: [Script Name]
Table: [Table Name]
Type: [Business Rule / Client Script / Script Include]
Reviewer: [AI-Assisted Review]
Date: [Current Date]

CRITICAL ISSUES:
  [C1] Security - SQL Injection in line X
  [C2] Security - XSS vulnerability in line Y

HIGH ISSUES:
  [H1] Performance - N+1 query pattern in line Z
  [H2] Performance - Unbounded GlideRecord query

MEDIUM ISSUES:
  [M1] Best Practice - Deprecated API usage
  [M2] Best Practice - Missing error handling

LOW ISSUES:
  [L1] Maintainability - Missing JSDoc comments
  [L2] Maintainability - Variable naming inconsistency

RECOMMENDATIONS:
  1. [Specific fix for C1]
  2. [Specific fix for C2]
  3. [Specific fix for H1]

步骤 7：应用修复

更新脚本并应用修复：

MCP 方式：

Tool: SN-Update-Record
Parameters:
  table_name: sys_script
  sys_id: [script_sys_id]
  data:
    script: |
      // [Updated script with fixes applied]

REST 替代方式：

curl -u "$SN_USER:$SN_PASS" \
  "$SN_INSTANCE/api/now/table/sys_script/[script_sys_id]" \
  -H "Content-Type: application/json" \
  -X PATCH \
  -d '{"script": "// Updated script content"}'

工具使用

操作	MCP 工具	REST 端点
查询业务规则	SN-Query-Table	GET /api/now/table/sys_script
查询客户端脚本	SN-Query-Table	GET /api/now/table/sys_script_client
查询脚本包含项	SN-Query-Table	GET /api/now/table/sys_script_include
查询 UI 脚本	SN-Query-Table	GET /api/now/table/sys_ui_script
获取单个脚本	SN-Get-Record	GET /api/now/table/{table}/{sys_id}
更新已修复的脚本	SN-Update-Record	PATCH /api/now/table/{table}/{sys_id}

最佳实践

• 分层审查： 先审查安全性，再审查性能、最佳实践和可维护性
• 自动化重复检查： 编写扫描脚本，检查常见反模式
• 一并检查所有脚本类型： 业务规则、客户端脚本和脚本包含项通常会相互影响
• 在非生产环境测试修复： 切勿直接在生产环境应用代码修复
• 记录例外情况： 如果有意采用已知反模式，请添加注释说明原因
• 同时审查 ACL： 确保 ACL 与业务规则中的安全检查相辅相成
• 检查死代码： 非活动脚本在升级期间仍会影响实例性能
• 验证错误处理： 每次调用 GlideRecord.get() 都应检查结果是否为 null
• 审查更新集： 部署前检查脚本是否已纳入适当的更新集

故障排查

无法访问脚本表

症状： 查询 sys_script 时提示权限被拒绝 原因： 缺少管理员或脚本相关角色 解决方案： 验证角色分配：

Tool: SN-Query-Table
Parameters:
  table_name: sys_user_has_role
  query: user=[user_sys_id]^role.name=admin
  fields: sys_id,role,user

脚本过大，无法检索

症状： API 响应中的脚本字段被截断 原因： API 字段大小限制 解决方案： 直接获取特定记录：

Tool: SN-Get-Record
Parameters:
  table_name: sys_script
  sys_id: [script_sys_id]
  fields: script

查找近期修改的所有脚本

症状： 需要检查所有近期更改 解决方案：

Tool: SN-Query-Table
Parameters:
  table_name: sys_script
  query: sys_updated_on>=javascript:gs.daysAgo(7)^active=true
  fields: sys_id,name,collection,when,sys_updated_by,sys_updated_on
  limit: 100

识别重复或冲突的规则

症状： 同一表上的多个规则导致行为异常 解决方案：

Tool: SN-Query-Table
Parameters:
  table_name: sys_script
  query: collection=incident^active=true^when=before
  fields: sys_id,name,order,filter_condition,action_insert,action_update
  limit: 100

示例

示例 1：对 Incident 业务规则进行全面安全审计

# Step 1: Get all active business rules for incident table
Tool: SN-Query-Table
Parameters:
  table_name: sys_script
  query: collection=incident^active=true
  fields: sys_id,name,when,script,order
  limit: 100

# Step 2: For each script, check for security patterns
# Look for: addEncodedQuery with user input, setWorkflow(false),
# gs.log with sensitive data, eval() usage, Packages.* calls

# Step 3: Get client scripts for incident
Tool: SN-Query-Table
Parameters:
  table_name: sys_script_client
  query: table=incident^active=true
  fields: sys_id,name,type,script
  limit: 50

# Step 4: Check for synchronous GlideRecord in client scripts
# Look for: GlideRecord().query() without callback, direct DOM access

示例 2：评审 Script Includes 的性能

# Get all active script includes with recent updates
Tool: SN-Query-Table
Parameters:
  table_name: sys_script_include
  query: active=true^sys_updated_on>=javascript:gs.daysAgo(90)
  fields: sys_id,name,script,api_name
  limit: 100

# For each script include, check for:
# - GlideRecord queries without setLimit()
# - Nested loops with queries (N+1)
# - Missing null checks after .get()
# - Large string concatenation in loops

示例 3：评审最近部署的代码

# Get scripts from the latest update set
Tool: SN-Query-Table
Parameters:
  table_name: sys_update_xml
  query: update_set=[update_set_sys_id]^nameLIKEsys_script
  fields: sys_id,name,action,target_name
  limit: 50

# Then retrieve each script for review
Tool: SN-Get-Record
Parameters:
  table_name: sys_script
  sys_id: [script_sys_id]
  fields: name,script,collection,when

相关技能

• development/code-assist - AI 辅助代码生成
• security/acl-management - 访问控制列表管理
• admin/update-set-management - 部署的更新集处理
• security/audit-compliance - 安全加固指南