🛡️ fix(prompts): address PR danny-avila#11882 review — ObjectId casting, null error return, unit tests

- getOwnedPromptGroupIds: validate and cast author to ObjectId
  (consistent with rest of Prompt.js, prevents silent empty-list on
  invalid input)
- getPromptGroup catch: return null instead of { message } object so
  callers (canAccessPromptGroupResource) don't treat errors as found
- Add format.spec.ts with 8 tests covering MY_PROMPTS, SHARED_PROMPTS,
  ALL filtering logic including deduplication and legacy fallback paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: NalinNair

api/models/Prompt.js

@@ -376,7 +376,14 @@ async function getListPromptGroupsByAccess({
  */
 const getOwnedPromptGroupIds = async (author) => {
   try {
-    const groups = await PromptGroup.find({ author }, { _id: 1 }).lean();
+    if (!author || !ObjectId.isValid(author)) {
+      logger.warn('getOwnedPromptGroupIds called with invalid author', { author });
+      return [];
+    }
+    const groups = await PromptGroup.find(
+      { author: new ObjectId(author) },
+      { _id: 1 },
+    ).lean();
     return groups.map((g) => g._id);
   } catch (error) {
     logger.error('Error getting owned prompt group IDs', error);
@@ -567,7 +574,7 @@ module.exports = {
       return group;
     } catch (error) {
       logger.error('Error getting prompt group', error);
-      return { message: 'Error getting prompt group' };
+      return null;
     }
   },
   /**

packages/api/src/prompts/format.spec.ts

@@ -0,0 +1,125 @@
+import { Types } from 'mongoose';
+import { filterAccessibleIdsBySharedLogic } from './format';
+
+const id = () => new Types.ObjectId();
+
+describe('filterAccessibleIdsBySharedLogic', () => {
+  const ownedA = id();
+  const ownedB = id();
+  const sharedC = id();
+  const sharedD = id();
+  const publicE = id();
+  const publicF = id();
+
+  // accessible = everything the ACL resolver says this user can see
+  const accessibleIds = [ownedA, ownedB, sharedC, sharedD];
+  const ownedPromptGroupIds = [ownedA, ownedB];
+  const publicPromptGroupIds = [publicE, publicF];
+
+  function toStrings(ids: Types.ObjectId[]) {
+    return ids.map((i) => i.toString()).sort();
+  }
+
+  describe('MY_PROMPTS (searchShared=false)', () => {
+    it('returns only owned IDs when ownedPromptGroupIds provided', async () => {
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds,
+        searchShared: false,
+        searchSharedOnly: false,
+        publicPromptGroupIds,
+        ownedPromptGroupIds,
+      });
+      expect(toStrings(result)).toEqual(toStrings([ownedA, ownedB]));
+    });
+
+    it('legacy fallback: excludes public IDs when ownedPromptGroupIds omitted', async () => {
+      // accessible includes a public ID for this test
+      const accessible = [ownedA, ownedB, publicE];
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds: accessible,
+        searchShared: false,
+        searchSharedOnly: false,
+        publicPromptGroupIds,
+      });
+      // Should exclude publicE, keep ownedA and ownedB
+      expect(toStrings(result)).toEqual(toStrings([ownedA, ownedB]));
+    });
+  });
+
+  describe('SHARED_PROMPTS (searchSharedOnly=true)', () => {
+    it('returns accessible + public minus owned when ownedPromptGroupIds provided', async () => {
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds,
+        searchShared: true,
+        searchSharedOnly: true,
+        publicPromptGroupIds,
+        ownedPromptGroupIds,
+      });
+      // Should include sharedC, sharedD, publicE, publicF (not ownedA, ownedB)
+      expect(toStrings(result)).toEqual(toStrings([sharedC, sharedD, publicE, publicF]));
+    });
+
+    it('deduplicates when an ID appears in both accessible and public', async () => {
+      const overlap = id();
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds: [ownedA, overlap],
+        searchShared: true,
+        searchSharedOnly: true,
+        publicPromptGroupIds: [overlap, publicE],
+        ownedPromptGroupIds: [ownedA],
+      });
+      // overlap should appear once, not twice
+      expect(toStrings(result)).toEqual(toStrings([overlap, publicE]));
+    });
+
+    it('legacy fallback: returns intersection of public and accessible when ownedPromptGroupIds omitted', async () => {
+      // publicE is also accessible
+      const accessible = [ownedA, publicE];
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds: accessible,
+        searchShared: true,
+        searchSharedOnly: true,
+        publicPromptGroupIds: [publicE, publicF],
+      });
+      // Only publicE is in both accessible and public
+      expect(toStrings(result)).toEqual(toStrings([publicE]));
+    });
+
+    it('legacy fallback: returns empty when no public IDs', async () => {
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds,
+        searchShared: true,
+        searchSharedOnly: true,
+        publicPromptGroupIds: [],
+      });
+      expect(result).toEqual([]);
+    });
+  });
+
+  describe('ALL (searchShared=true, searchSharedOnly=false)', () => {
+    it('returns union of accessible + public, deduplicated', async () => {
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds,
+        searchShared: true,
+        searchSharedOnly: false,
+        publicPromptGroupIds,
+        ownedPromptGroupIds,
+      });
+      expect(toStrings(result)).toEqual(
+        toStrings([ownedA, ownedB, sharedC, sharedD, publicE, publicF]),
+      );
+    });
+
+    it('deduplicates overlapping IDs', async () => {
+      const overlap = id();
+      const result = await filterAccessibleIdsBySharedLogic({
+        accessibleIds: [ownedA, overlap],
+        searchShared: true,
+        searchSharedOnly: false,
+        publicPromptGroupIds: [overlap, publicE],
+        ownedPromptGroupIds,
+      });
+      expect(toStrings(result)).toEqual(toStrings([ownedA, overlap, publicE]));
+    });
+  });
+});