fix(robustness): harden field/validation helpers; fix update-field persistence

Adversarial pass (50+ probes) surfaced 15 defects, all fixed test-first: gf_update_field saved a change before reporting it blocked (dependency guard now runs before the write, like deleteField); gf_add_field crashed on null properties/position and accepted empty/null/non-string field_type; gf_create_form fields:[null] threw an opaque TypeError instead of a clean error; null-input crashes in validateFieldConfig/detectFieldVariant/generateCompoundInputs/stripEntryMeta/DependencyTracker; stripEmpty/sanitize could stack-overflow on circular input; buildToolList emitted an undefined entry when gkReloadDef was omitted; validateFieldFilter silently stringified object values. Offline gate green (test:node 377, test:unit 100%, field-validation, publint, lint:docs); adversarial flags 13->3 (remaining 3 are non-bugs); live-verified on dev.test.

Claude-Session: https://claude.ai/code/session_01LJHfTpQknHFs1j5ayj7fpo

Author: zackkatz

CHANGELOG.md

@@ -12,6 +12,8 @@ This release lets the field tools work with custom and third-party field types,
 ### 🐛 Fixed
 - **`gf_add_field` now accepts custom and third-party field types.** Field types added by other plugins (Gravity Perks, Gravity Wiz, and similar add-ons) were previously rejected because they aren't in the built-in registry. They are now created normally, with a note when type-specific defaults or sub-inputs aren't available; pass `inputs`/`choices` explicitly for custom compound or choice fields.
 - **Unrecognized field types no longer leave an internal marker in saved forms** when creating or updating a form.
+- **`gf_update_field` no longer saves a change it then reports as blocked.** When a field had dependent conditional logic and `force` wasn't set, the update was written before the "use force to proceed" response, so the check protected nothing. The dependency check now runs before the write (matching `gf_delete_field`).
+- **Hardened tool-input handling.** Malformed or hostile input to the field, validation, and dependency helpers (empty/`null`/non-string field types, `null` properties or position, a `null` entry in a form's `fields`, non-scalar filter values, self-referential objects) now returns a clean error or is handled safely instead of failing opaquely.
 
 ## [2.4.0] - 2026-06-19
 

src/config/field-validation.js

@@ -43,8 +43,8 @@ export class FieldAwareValidator {
       } else {
         errors.push({
           index: i,
-          fieldId: field.id,
-          fieldType: field.type,
+          fieldId: field?.id,
+          fieldType: field?.type,
           error: validation.error
         });
       }

src/config/validation.js

@@ -104,6 +104,13 @@ export class BaseValidator {
       throw new Error('Field filter must have a value');
     }
 
+    // Reject non-array objects: String({}) becomes "[object Object]", a silent
+    // garbage filter value. Arrays stay valid for the multi-value operators
+    // handled below; a plain object never is.
+    if (typeof value === 'object' && !Array.isArray(value)) {
+      throw new Error('Field filter value must be a string, number, boolean, or array');
+    }
+
     // GF normalizes operators case-insensitively (IN/in/In all match), so accept
     // any case rather than rejecting valid GF operators.
     const validOperators = getEnumValues('fieldOperators');

src/field-definitions/field-registry.js

@@ -902,6 +902,9 @@ export function getStorageFormat(fieldType) {
  * Helper function to detect field variant
  */
 export function detectFieldVariant(field) {
+  if (!field || typeof field !== 'object') {
+    return 'default';
+  }
   const definition = fieldRegistry[field.type];
   if (!definition || !definition.variants) {
     return 'default';
@@ -927,6 +930,9 @@ export function detectFieldVariant(field) {
  * Validate field configuration
  */
 export function validateFieldConfig(field) {
+  if (!field || typeof field !== 'object') {
+    return { isValid: false, error: 'Field must be an object' };
+  }
   const definition = fieldRegistry[field.type];
 
   // Unknown / third-party types are tolerated, not rejected (Gravity Forms
@@ -1059,6 +1065,9 @@ export function getCompoundFieldInputs(fieldType) {
  * @returns {array|null} Array of input definitions or null if not a compound field.
  */
 export function generateCompoundInputs(field) {
+  if (!field || typeof field !== 'object') {
+    return null;
+  }
   const fieldDef = fieldRegistry[field.type];
 
   if (!fieldDef || !fieldDef.isCompound) {

src/field-operations/field-dependencies.js

@@ -18,6 +18,10 @@ export class DependencyTracker {
       dynamicPopulation: []
     };
 
+    if (!form || typeof form !== 'object') {
+      return dependencies;
+    }
+
     // 1. Check conditional logic dependencies in all fields
     this.scanConditionalLogic(form, fieldId, dependencies);
 
@@ -260,10 +264,13 @@ export class DependencyTracker {
    * Check if dependencies would break form functionality
    */
   hasBreakingDependencies(dependencies) {
+    if (!dependencies || typeof dependencies !== 'object') {
+      return false;
+    }
     return (
-      dependencies.conditionalLogic.length > 0 ||
-      dependencies.calculations.length > 0 ||
-      dependencies.mergeTags.length > 0
+      (dependencies.conditionalLogic?.length > 0) ||
+      (dependencies.calculations?.length > 0) ||
+      (dependencies.mergeTags?.length > 0)
     );
   }
 

src/field-operations/field-manager.js

@@ -27,6 +27,14 @@ export class FieldManager {
    * @returns {object} Field creation result with warnings
    */
   async addField(formId, fieldType, properties = {}, position = {}) {
+    if (typeof fieldType !== 'string' || fieldType.trim() === '') {
+      throw new Error('field_type is required and must be a non-empty string');
+    }
+    // Parameter defaults only cover `undefined`; coerce an explicit null so
+    // adversarial input can't crash createField or the position engine.
+    properties = properties || {};
+    position = position || {};
+
     // The registry is an ENHANCEMENT source, not a gate. Known types get
     // type-specific defaults and sub-inputs. Unknown types (third-party add-ons,
     // GravityKit, custom fields) are still created (Gravity Forms accepts them on
@@ -95,31 +103,46 @@ export class FieldManager {
   /**
    * Update existing field with dependency checking
    */
-  async updateField(formId, fieldId, updates = {}) {
+  async updateField(formId, fieldId, updates = {}, options = {}) {
+    const { force = false } = options;
+
     // Fetch form
     const { form } = await this.api.getForm({ id: formId });
-    
+
     // Find field
     const fieldIndex = form.fields?.findIndex(f => f.id == fieldId);
-    if (fieldIndex === -1) {
+    if (fieldIndex === undefined || fieldIndex === -1) {
       throw new Error(`Field ${fieldId} not found in form ${formId}`);
     }
-    
-    // Check dependencies
+
+    // Gate the write on dependencies BEFORE mutating, the way deleteField does.
+    // Saving first and reporting failure afterward persisted a "blocked" update
+    // and made the success:false response a lie.
     const dependencies = this.dependencyTracker?.scanFormDependencies(form, fieldId) || {};
-    
+    const hasBreakingDeps = dependencies.conditionalLogic?.length > 0;
+
+    if (hasBreakingDeps && !force) {
+      return {
+        success: false,
+        error: 'Field has dependencies that may be affected',
+        field_id: fieldId,
+        dependencies,
+        suggestion: 'Use force=true to update anyway'
+      };
+    }
+
     // Apply updates
     const originalField = { ...form.fields[fieldIndex] };
     form.fields[fieldIndex] = {
       ...originalField,
-      ...updates,
+      ...(updates || {}),
       id: originalField.id // Preserve ID
     };
     this.normalizeLayoutProperties(form.fields[fieldIndex], formId);
 
-    // Replace form via direct PUT (no re-fetch — we already have the full state)
+    // Replace form via direct PUT (no re-fetch; we already have the full state)
     const result = await this.api.replaceForm(formId, form);
-    
+
     return {
       success: true,
       field: result.form.fields[fieldIndex],
@@ -128,8 +151,7 @@ export class FieldManager {
         after: result.form.fields[fieldIndex]
       },
       warnings: {
-        dependencies: dependencies.conditionalLogic?.length > 0 ? 
-          ['Field has conditional logic dependencies'] : [],
+        dependencies: hasBreakingDeps ? ['Field has conditional logic dependencies'] : [],
         validationIssues: this.validator.getWarnings(result.form.fields[fieldIndex])
       }
     };

src/field-operations/index.js

@@ -72,26 +72,9 @@ export const fieldOperationHandlers = {
   async gf_update_field(params, { fieldManager }) {
     const { form_id, field_id, properties, force = false } = params;
 
-    const result = await fieldManager.updateField(
-      form_id,
-      field_id,
-      properties
-    );
-
-    // Check for breaking changes if not forced
-    if (!force && result.warnings?.dependencies?.length > 0) {
-      return {
-        success: false,
-        error: 'Field has dependencies that may be affected',
-        ...result,
-        suggestion: 'Use force=true to update anyway'
-      };
-    }
-
-    return {
-      success: true,
-      ...result
-    };
+    // updateField gates the write on dependencies and returns the final
+    // success/failure shape; it no longer saves before reporting a block.
+    return fieldManager.updateField(form_id, field_id, properties, { force });
   },
 
   /**

src/server-runtime.js

@@ -29,7 +29,7 @@ export function buildToolList({ gfReady, gfToolDefs = [], fieldOpTools = [], abi
     ...(gfReady ? [...gfToolDefs, ...fieldOpTools] : []),
     ...(abilityDefs ?? []),
     gkReloadDef,
-  ];
+  ].filter(Boolean);
 }
 
 /**

src/utils/compact.js

@@ -12,15 +12,19 @@
  * @param {*} obj - Value to compact
  * @returns {*} Compacted value
  */
-export function stripEmpty(obj) {
+export function stripEmpty(obj, seen = new WeakSet()) {
   if (Array.isArray(obj)) {
-    return obj.map(stripEmpty);
+    if (seen.has(obj)) return obj;
+    seen.add(obj);
+    return obj.map((v) => stripEmpty(v, seen));
   }
   if (obj !== null && typeof obj === 'object') {
+    if (seen.has(obj)) return obj;
+    seen.add(obj);
     const result = {};
     for (const [key, value] of Object.entries(obj)) {
       if (value === null || value === '') continue;
-      result[key] = stripEmpty(value);
+      result[key] = stripEmpty(value, seen);
     }
     return result;
   }
@@ -53,6 +57,9 @@ function isFieldKey(key) {
  * @returns {object} Entry with only core + field keys
  */
 export function stripEntryMeta(entry) {
+  if (!entry || typeof entry !== 'object') {
+    return {};
+  }
   const result = {};
   for (const [key, value] of Object.entries(entry)) {
     if (CORE_ENTRY_KEYS.has(key) || isFieldKey(key)) {

src/utils/sanitize.js

@@ -43,11 +43,15 @@ function mask(value) {
 /**
  * Sanitize an object for logging
  */
-export function sanitize(obj) {
+export function sanitize(obj, seen = new WeakSet()) {
   if (!obj || typeof obj !== 'object') return obj;
 
+  // Cut cycles so logging a self-referential object can never stack-overflow.
+  if (seen.has(obj)) return Array.isArray(obj) ? [] : {};
+  seen.add(obj);
+
   if (Array.isArray(obj)) {
-    return obj.map(sanitize);
+    return obj.map((v) => sanitize(v, seen));
   }
 
   const result = {};
@@ -58,7 +62,7 @@ export function sanitize(obj) {
     if (isSensitive) {
       result[key] = mask(value);
     } else if (typeof value === 'object' && value !== null) {
-      result[key] = sanitize(value);
+      result[key] = sanitize(value, seen);
     } else {
       result[key] = value;
     }