feat: Enhance agent skills API with validation and limits

Author: ctlaltlaltc

packages/global/core/agentSkills/api.d.ts

@@ -81,9 +81,15 @@ export type GetSkillDetailResponse = {
 };
 
 // ==================== Import Skill ====================
+/**
+ * ImportSkillBody — optional metadata fields for multipart/form-data upload.
+ * The actual archive file (ZIP / TAR / TAR.GZ) must be sent as the `file` field
+ * in a multipart/form-data request; it is not represented in this type.
+ */
 export type ImportSkillBody = {
   name?: string; // required for multi-skill packages; optional for single-skill (falls back to SKILL.md)
   description?: string; // optional, fallback to SKILL.md or ''
+  avatar?: string; // optional skill icon/avatar URL or base64
 };
 
 export type ImportSkillResponse = string; // single skillId

packages/global/core/agentSkills/type.d.ts

@@ -93,6 +93,7 @@ export type AgentSkillConfigType = {
     url: string;
     method: 'GET' | 'POST' | 'PUT' | 'DELETE';
     headers?: Record<string, string>;
+    /** Request timeout in seconds */
     timeout?: number;
   };
 

packages/service/core/agentSkills/archiveUtils.ts

@@ -15,15 +15,26 @@ export function getSupportedArchiveFormat(filename: string): ArchiveFormat | nul
 /**
  * Extract archive file (zip/tar/tar.gz) to a file map.
  * Path traversal entries are filtered out for security.
+ * Total uncompressed size is capped to prevent Zip Bomb OOM attacks.
  */
-export async function extractToFileMap(filePath: string): Promise<ArchiveFileMap> {
+export async function extractToFileMap(
+  filePath: string,
+  maxUncompressedBytes = 200 * 1024 * 1024
+): Promise<ArchiveFileMap> {
   const files = await decompress(filePath);
   const fileMap: ArchiveFileMap = {};
+  let totalSize = 0;
   for (const file of files) {
     if (file.type === 'directory') continue;
     const normalized = file.path.replace(/\\/g, '/').replace(/^\/+/, '');
     // Filter path traversal
     if (!normalized || normalized.includes('../')) continue;
+    totalSize += file.data.length;
+    if (totalSize > maxUncompressedBytes) {
+      throw new Error(
+        `Uncompressed archive exceeds maximum allowed size (${maxUncompressedBytes / 1024 / 1024}MB)`
+      );
+    }
     fileMap[normalized] = file.data;
   }
   return fileMap;

packages/service/core/agentSkills/controller.ts

@@ -157,9 +157,9 @@ export async function listSkills(
     query.teamId = teamId;
   }
 
-  // Category filter
+  // Category filter — use $in because category is an array field
   if (category) {
-    query.category = category;
+    query.category = { $in: [category] };
   }
 
   // Search key (text search on name and description)

packages/service/core/agentSkills/sandboxConfig.ts

@@ -6,6 +6,12 @@
 
 import type { SandboxImageConfigType } from '@fastgpt/global/core/agentSkills/type';
 
+/** Parse an integer from an env-var string, returning defaultValue when the result is NaN. */
+function safeParseInt(value: string | undefined, defaultValue: number): number {
+  const n = parseInt(value ?? '', 10);
+  return isNaN(n) ? defaultValue : n;
+}
+
 export type SandboxProviderConfig = {
   provider: string;
   baseUrl: string;
@@ -45,9 +51,9 @@ export function getSandboxProviderConfig(): SandboxProviderConfig {
  */
 export function getSandboxDefaults(): SandboxDefaults {
   return {
-    timeout: parseInt(process.env.SANDBOX_DEFAULT_TIMEOUT || '600', 10), // 10 minutes, Automatic termination timeout (server-side TTL)
-    cleanupInterval: parseInt(process.env.SANDBOX_CLEANUP_INTERVAL || '3600000', 10),
-    inactiveThreshold: parseInt(process.env.SANDBOX_INACTIVE_THRESHOLD || '7200', 10),
+    timeout: safeParseInt(process.env.SANDBOX_DEFAULT_TIMEOUT, 600), // 10 minutes, Automatic termination timeout (server-side TTL)
+    cleanupInterval: safeParseInt(process.env.SANDBOX_CLEANUP_INTERVAL, 3600000),
+    inactiveThreshold: safeParseInt(process.env.SANDBOX_INACTIVE_THRESHOLD, 7200),
     defaultImage: {
       repository: process.env.SANDBOX_DEFAULT_IMAGE || 'node',
       tag: process.env.SANDBOX_DEFAULT_IMAGE_TAG || '18-alpine'

packages/service/core/agentSkills/schema.ts

@@ -82,11 +82,11 @@ try {
   AgentSkillsSchema.index({ source: 1, teamId: 1, deleteTime: 1, createTime: -1 });
   // Category index
   AgentSkillsSchema.index({ category: 1 });
-  // Removed unique index on name to allow duplicate skill names
-  // AgentSkillSchema.index(
-  //   { name: 1, teamId: 1, deleteTime: 1 },
-  //   { unique: true, partialFilterExpression: { deleteTime: null } }
-  // );
+  // Unique constraint: same team cannot have two live skills with the same name
+  AgentSkillsSchema.index(
+    { name: 1, teamId: 1, deleteTime: 1 },
+    { unique: true, partialFilterExpression: { deleteTime: null } }
+  );
 } catch (error) {
   console.log('AgentSkill index error:', error);
 }

packages/service/core/agentSkills/storage.ts

@@ -93,7 +93,10 @@ export async function uploadSkillPackage(
 /**
  * Download skill package from MinIO/S3 storage
  */
-export async function downloadSkillPackage(params: DownloadSkillPackageParams): Promise<Buffer> {
+export async function downloadSkillPackage(
+  params: DownloadSkillPackageParams,
+  maxBytes = 200 * 1024 * 1024
+): Promise<Buffer> {
   const { storageInfo } = params;
 
   const bucket = new S3PrivateBucket();
@@ -106,10 +109,16 @@ export async function downloadSkillPackage(params: DownloadSkillPackageParams):
     throw new Error(`Failed to download skill package: ${storageInfo.key}`);
   }
 
-  // Convert stream to buffer
+  // Convert stream to buffer with size limit to prevent OOM
   const chunks: Buffer[] = [];
+  let totalSize = 0;
   for await (const chunk of response.body) {
-    chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
+    const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
+    totalSize += buf.length;
+    if (totalSize > maxBytes) {
+      throw new Error(`Skill package exceeds maximum allowed size (${maxBytes / 1024 / 1024}MB)`);
+    }
+    chunks.push(buf);
   }
 
   return Buffer.concat(chunks);

packages/service/core/agentSkills/versionController.ts

@@ -46,13 +46,17 @@ export async function createVersion(
 }
 
 /**
- * Get the next version number for a skill
+ * Get the next version number for a skill.
+ * Should be called inside a transaction session to avoid version number races.
  */
-export async function getNextVersionNumber(skillId: string): Promise<number> {
+export async function getNextVersionNumber(
+  skillId: string,
+  session?: ClientSession
+): Promise<number> {
   const lastVersion = await MongoAgentSkillsVersion.findOne(
     { skillId, isDeleted: false },
     { version: 1 },
-    { sort: { version: -1 } }
+    { sort: { version: -1 }, session }
   ).lean();
 
   return (lastVersion?.version ?? -1) + 1;
@@ -140,7 +144,8 @@ export async function setActiveVersion(
 }
 
 /**
- * Soft delete a version
+ * Soft delete a version.
+ * Active versions cannot be deleted — deactivate or switch to another version first.
  */
 export async function deleteVersion(
   skillId: string,
@@ -157,18 +162,24 @@ export async function deleteVersion(
     throw new Error(`Version ${version} not found for skill ${skillId}`);
   }
 
-  // If this is the active version, we should not allow deletion
-  // or we should deactivate it first
-  const updateData: Record<string, any> = {
-    isDeleted: true,
-    isActive: false
-  };
+  // Refuse to delete the currently active version to prevent data orphaning
+  if (versionDoc.isActive) {
+    throw new Error(
+      `Cannot delete active version ${version}. Switch to another version before deleting.`
+    );
+  }
 
-  await MongoAgentSkillsVersion.updateOne({ skillId, version }, { $set: updateData }, { session });
+  await MongoAgentSkillsVersion.updateOne(
+    { skillId, version },
+    { $set: { isDeleted: true } },
+    { session }
+  );
 }
 
 /**
- * Restore a deleted version
+ * Restore a deleted version.
+ * The restored version is set back to isDeleted=false but remains inactive (isActive=false).
+ * Call setActiveVersion explicitly if you want to make it the active version.
  */
 export async function restoreVersion(
   skillId: string,

projects/app/src/pages/api/core/app/agent/skills/create.ts

@@ -58,6 +58,17 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
       });
     }
 
+    // Validate category enum values
+    const validCategories = Object.values(AgentSkillCategoryEnum) as string[];
+    if (category.length > 0 && category.some((c) => !validCategories.includes(c))) {
+      return jsonRes(res, { code: 400, error: 'Invalid category value' });
+    }
+
+    // Validate config size (max 50 KB)
+    if (config && JSON.stringify(config).length > 50_000) {
+      return jsonRes(res, { code: 400, error: 'Config exceeds maximum allowed size (50KB)' });
+    }
+
     // Check if skill name already exists
     const nameExists = await checkSkillNameExists(name.trim(), teamId);
     if (nameExists) {
@@ -125,7 +136,11 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
     jsonRes<CreateSkillResponse>(res, {
       data: skillId
     });
-  } catch (err) {
+  } catch (err: any) {
+    // E11000: unique index violation (concurrent duplicate name creation)
+    if (err.code === 11000 || err.codeName === 'DuplicateKey') {
+      return jsonRes(res, { code: 409, error: 'Skill name already exists' });
+    }
     jsonRes(res, {
       code: 500,
       error: err

projects/app/src/pages/api/core/app/agent/skills/delete.ts

@@ -4,6 +4,7 @@ import { authUserPer } from '@fastgpt/service/support/permission/user/auth';
 import { mongoSessionRun } from '@fastgpt/service/common/mongo/sessionRun';
 import { deleteSkill, canModifySkill } from '@fastgpt/service/core/agentSkills/controller';
 import type { DeleteSkillQuery, DeleteSkillResponse } from '@fastgpt/global/core/agentSkills/api';
+import { isValidObjectId } from 'mongoose';
 
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
   try {
@@ -33,6 +34,10 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
       });
     }
 
+    if (!isValidObjectId(skillId)) {
+      return jsonRes(res, { code: 400, error: 'Invalid skill ID format' });
+    }
+
     // Check if user can delete this skill
     const canDelete = await canModifySkill(skillId, tmbId);
     if (!canDelete) {