fix(projects): nullify empty strings on permissions update payload (#673)

* fix(projects): nullify empty strings on permissions update payload

LFXV2-1724

Upstream LFX_V2_SERVICE rejects empty strings on validated fields in
the project settings PUT body. The prior workaround only omitted the
avatar key when empty; other fields (email, name, optional user refs)
echoed empty strings back to the upstream and could fail validation.

Introduces a generic, non-mutating nullifyEmptyStrings helper in
@lfx-one/shared/utils (new object.utils.ts) and applies it to the full
settings payload before the ETag PUT in ProjectService.

Signed-off-by: Asitha de Silva <asithade@gmail.com>

* fix(review): address PR #673 review feedback

Address review comments from copilot-pull-request-reviewer, coderabbitai:

- packages/shared/src/utils/object.utils.ts: introduce NullifyEmptyStrings<T>
  mapped return type so callers see accurate nullability for nested string
  fields, removing the unsound `T` return when T is `string` (per
  copilot-pull-request-reviewer). The mapped type also flows through to the
  ProjectSettings call site, resolving the runtime/declared type divergence
  flagged on project.service.ts.
- packages/shared/src/utils/object.utils.ts: guard the object branch with a
  plain-prototype check so Date, Map, Set, URL, RegExp, and class instances
  are returned unchanged instead of being rebuilt into bare records (per
  copilot-pull-request-reviewer and coderabbitai).

Resolves 4 review threads.

Signed-off-by: Asitha de Silva <asithade@gmail.com>

* refactor(shared): move NullifyEmptyStrings type to interfaces folder

Per repo convention, type aliases live alongside interfaces in
@lfx-one/shared/interfaces, not in utils files. Also flatten the
mapped type into single-conditional aliases so there are no chained
(nested) ternaries.

- packages/shared/src/interfaces/object.interface.ts: new home for
  NullifyEmptyStrings<T>, split into NullifyStringBranch /
  PreservedBranch / ArrayBranch / ObjectBranch helpers, each with a
  single conditional.
- packages/shared/src/utils/object.utils.ts: imports the type from
  interfaces and keeps only the runtime helper.

Signed-off-by: Asitha de Silva <asithade@gmail.com>

* fix(review): address PR #673 review feedback

Address review comment from copilot[bot]:

- packages/shared/src/utils/object.utils.ts: use Object.defineProperty
  when rebuilding plain objects to prevent prototype pollution via
  __proto__ keys (per copilot[bot])

Resolves 1 review thread.

Signed-off-by: Asitha de Silva <asithade@gmail.com>

* fix(review): address PR #673 review feedback

Address review comments from copilot[bot]:

- packages/shared/src/interfaces/object.interface.ts: add function branch
  to NullifyEmptyStrings so callable types are preserved instead of being
  mapped into their key records (per copilot[bot])
- packages/shared/src/utils/object.utils.ts: rename `trimmed` to
  `nullIfBlank` since the value is the original string or null, not the
  trimmed string (per copilot[bot])

Resolves 2 review threads.

Signed-off-by: Asitha de Silva <asithade@gmail.com>

---------

Signed-off-by: Asitha de Silva <asithade@gmail.com>
Signed-off-by: David Deal <ddeal@linuxfoundation.org>
Co-authored-by: David Deal <ddeal@linuxfoundation.org>

Author: asithade

apps/lfx-one/src/server/services/project.service.ts

@@ -102,7 +102,7 @@ import {
   UploadProjectDocumentRequest,
   WebActivitiesSummaryResponse,
 } from '@lfx-one/shared/interfaces';
-import { computeIsFoundation } from '@lfx-one/shared/utils';
+import { computeIsFoundation, nullifyEmptyStrings } from '@lfx-one/shared/utils';
 import { Request } from 'express';
 import FormData from 'form-data';
 
@@ -453,31 +453,8 @@ export class ProjectService {
     }
     // For 'remove' operation, user is already removed from both arrays above
 
-    // Step 3: Clean up empty avatar fields before sending to API
-    // The API validation doesn't accept empty strings for avatar URLs
-    updatedSettings.writers = updatedSettings.writers.map((user) => {
-      const cleanUser: any = {
-        name: user.name,
-        email: user.email,
-        username: user.username,
-      };
-      if (user.avatar && user.avatar.trim() !== '') {
-        cleanUser.avatar = user.avatar;
-      }
-      return cleanUser;
-    });
-
-    updatedSettings.auditors = updatedSettings.auditors.map((user) => {
-      const cleanUser: any = {
-        name: user.name,
-        email: user.email,
-        username: user.username,
-      };
-      if (user.avatar && user.avatar.trim() !== '') {
-        cleanUser.avatar = user.avatar;
-      }
-      return cleanUser;
-    });
+    // Upstream rejects empty strings on validated fields; send null instead so the key is preserved.
+    const sanitizedSettings = nullifyEmptyStrings(updatedSettings);
 
     // Step 4: Update settings with ETag
     const startTime = logger.startOperation(req, `${operation}_user_project_permissions`, {
@@ -491,7 +468,7 @@ export class ProjectService {
       'LFX_V2_SERVICE',
       `/projects/${uid}/settings`,
       etag,
-      updatedSettings,
+      sanitizedSettings,
       `${operation}_user_project_permissions`
     );
 

packages/shared/src/interfaces/index.ts

@@ -148,5 +148,8 @@ export * from './stat-card.interface';
 // Changelog interfaces
 export * from './changelog.interface';
 
+// Object utility types
+export * from './object.interface';
+
 // Marketing Impact interfaces
 export * from './marketing-impact.interface';

packages/shared/src/interfaces/object.interface.ts

@@ -0,0 +1,23 @@
+// Copyright The Linux Foundation and each contributor to LFX.
+// SPDX-License-Identifier: MIT
+
+/**
+ * Built-in object types that should be passed through `NullifyEmptyStrings`
+ * unchanged. Without this exclusion, the mapped-object branch would rebuild
+ * them into bare records and lose their prototype.
+ */
+type PreservedObjectTypes = Date | Map<unknown, unknown> | Set<unknown> | URL | RegExp;
+
+/**
+ * Maps `T` so every nested `string` becomes `string | null`. Arrays map
+ * elementwise, plain objects map each property, and {@link PreservedObjectTypes}
+ * and functions are returned as-is. Implemented as a chain of single-conditional
+ * aliases to avoid nested ternary expressions.
+ */
+export type NullifyEmptyStrings<T> = NullifyStringBranch<T>;
+
+type NullifyStringBranch<T> = T extends string ? string | null : NullifyFunctionBranch<T>;
+type NullifyFunctionBranch<T> = T extends (...args: any[]) => any ? T : NullifyPreservedBranch<T>;
+type NullifyPreservedBranch<T> = T extends PreservedObjectTypes ? T : NullifyArrayBranch<T>;
+type NullifyArrayBranch<T> = T extends readonly (infer U)[] ? NullifyEmptyStrings<U>[] : NullifyObjectBranch<T>;
+type NullifyObjectBranch<T> = T extends object ? { [K in keyof T]: NullifyEmptyStrings<T[K]> } : T;

packages/shared/src/utils/index.ts

@@ -15,6 +15,7 @@ export * from './survey.utils';
 export * from './vote.utils';
 export * from './committee.utils';
 export * from './number.utils';
+export * from './object.utils';
 export * from './platform.utils';
 export * from './project.utils';
 export * from './badge.utils';

packages/shared/src/utils/object.utils.ts

@@ -0,0 +1,44 @@
+// Copyright The Linux Foundation and each contributor to LFX.
+// SPDX-License-Identifier: MIT
+
+import { NullifyEmptyStrings } from '../interfaces/object.interface';
+
+/**
+ * Recursively replaces empty / whitespace-only strings with `null` throughout a
+ * value. Useful when sending payloads to APIs that reject `""` on validated
+ * fields but accept `null`. Non-string primitives, arrays, and plain objects are
+ * walked; non-plain objects (Date, Map, Set, class instances, etc.) are returned
+ * unchanged so their prototype is preserved. The original value is not mutated.
+ *
+ * @param value - The value to sanitize (object, array, or primitive)
+ * @returns A new value with empty strings replaced by `null`
+ */
+export function nullifyEmptyStrings<T>(value: T): NullifyEmptyStrings<T> {
+  if (value === null || value === undefined) return value as NullifyEmptyStrings<T>;
+  if (typeof value === 'string') {
+    const nullIfBlank = value.trim() === '' ? null : value;
+    return nullIfBlank as NullifyEmptyStrings<T>;
+  }
+  if (Array.isArray(value)) {
+    return value.map((item) => nullifyEmptyStrings(item)) as NullifyEmptyStrings<T>;
+  }
+  if (typeof value === 'object') {
+    const proto = Object.getPrototypeOf(value);
+    if (proto !== Object.prototype && proto !== null) {
+      return value as NullifyEmptyStrings<T>;
+    }
+    const result: Record<string, unknown> = {};
+    for (const [key, val] of Object.entries(value as Record<string, unknown>)) {
+      // Use defineProperty to bypass setters (e.g. __proto__) and prevent prototype pollution
+      // when keys come from untrusted sources like JSON.parse.
+      Object.defineProperty(result, key, {
+        value: nullifyEmptyStrings(val),
+        writable: true,
+        enumerable: true,
+        configurable: true,
+      });
+    }
+    return result as NullifyEmptyStrings<T>;
+  }
+  return value as NullifyEmptyStrings<T>;
+}