Skip to content

Commit e109625

Browse files
barckcodeclaude
andcommitted
feat: structured subagent config (#52) + file upload in chat (#63)
Issue #52 - Structured subagent configuration: - Add SubAgentInstructions field to Agent model, DTOs, and handlers - Update GenerateSubAgentContent and GenerateOpenCodeSubAgentContent to write Instructions as markdown body after YAML frontmatter - Add size validation: Description max 2KB, Instructions max 100KB Issue #63 - File upload support in chat endpoint: - Accept multipart/form-data with optional file attachments - Validate file size (10MB max), count (5 max), and MIME types - Sanitize filenames against path traversal and injection - Write files to leader container workspace via exec - Add FileRef to UserMessagePayload for NATS messaging - Backward compatible with JSON-only chat requests Security hardening: - Escape agent name with yamlQuoteIfNeeded in frontmatter generation - Add quotes and double-quotes to YAML trigger characters - Quote filePath in shell commands for defense in depth - Configure Fiber BodyLimit to 51MB for file uploads Tests: 16 new backend tests (workspace generation, size validation, filename sanitization, MIME validation) Bump version to 0.3.7 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0b27500 commit e109625

File tree

13 files changed

+670
-56
lines changed

13 files changed

+670
-56
lines changed

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
0.3.6
1+
0.3.7

internal/api/dto.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ type CreateAgentInput struct {
4848
Skills interface{} `json:"skills"`
4949
Permissions interface{} `json:"permissions"`
5050
Resources interface{} `json:"resources"`
51-
SubAgentDescription string `json:"sub_agent_description"`
52-
SubAgentModel string `json:"sub_agent_model"`
53-
SubAgentSkills interface{} `json:"sub_agent_skills"`
51+
SubAgentDescription string `json:"sub_agent_description"`
52+
SubAgentInstructions string `json:"sub_agent_instructions"`
53+
SubAgentModel string `json:"sub_agent_model"`
54+
SubAgentSkills interface{} `json:"sub_agent_skills"`
5455
}
5556

5657
// CreateAgentRequest is the payload for POST /api/teams/:id/agents.
@@ -64,9 +65,10 @@ type CreateAgentRequest struct {
6465
Skills interface{} `json:"skills"`
6566
Permissions interface{} `json:"permissions"`
6667
Resources interface{} `json:"resources"`
67-
SubAgentDescription string `json:"sub_agent_description"`
68-
SubAgentModel string `json:"sub_agent_model"`
69-
SubAgentSkills interface{} `json:"sub_agent_skills"`
68+
SubAgentDescription string `json:"sub_agent_description"`
69+
SubAgentInstructions string `json:"sub_agent_instructions"`
70+
SubAgentModel string `json:"sub_agent_model"`
71+
SubAgentSkills interface{} `json:"sub_agent_skills"`
7072
}
7173

7274
// UpdateAgentRequest is the payload for PUT /api/teams/:id/agents/:agentId.
@@ -80,9 +82,10 @@ type UpdateAgentRequest struct {
8082
Skills interface{} `json:"skills"`
8183
Permissions interface{} `json:"permissions"`
8284
Resources interface{} `json:"resources"`
83-
SubAgentDescription *string `json:"sub_agent_description"`
84-
SubAgentModel *string `json:"sub_agent_model"`
85-
SubAgentSkills interface{} `json:"sub_agent_skills"`
85+
SubAgentDescription *string `json:"sub_agent_description"`
86+
SubAgentInstructions *string `json:"sub_agent_instructions"`
87+
SubAgentModel *string `json:"sub_agent_model"`
88+
SubAgentSkills interface{} `json:"sub_agent_skills"`
8689
}
8790

8891
// ChatRequest is the payload for POST /api/teams/:id/chat.

internal/api/handlers_agents.go

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,13 @@ func (s *Server) CreateAgent(c *fiber.Ctx) error {
8989
return fiber.NewError(fiber.StatusBadRequest, "sub_agent_model must be one of: inherit, sonnet, opus, haiku")
9090
}
9191

92+
if len(req.SubAgentDescription) > maxDescriptionSize {
93+
return fiber.NewError(fiber.StatusBadRequest, fmt.Sprintf("sub_agent_description exceeds maximum size of %d bytes", maxDescriptionSize))
94+
}
95+
if len(req.SubAgentInstructions) > maxInstructionsSize {
96+
return fiber.NewError(fiber.StatusBadRequest, fmt.Sprintf("sub_agent_instructions exceeds maximum size of %d bytes", maxInstructionsSize))
97+
}
98+
9299
if req.SubAgentSkills != nil {
93100
if err := validateSubAgentSkills(req.SubAgentSkills); err != nil {
94101
return fiber.NewError(fiber.StatusBadRequest, err.Error())
@@ -123,9 +130,10 @@ func (s *Server) CreateAgent(c *fiber.Ctx) error {
123130
Skills: models.JSON(skills),
124131
Permissions: models.JSON(perms),
125132
Resources: models.JSON(resources),
126-
SubAgentDescription: req.SubAgentDescription,
127-
SubAgentModel: subAgentModel,
128-
SubAgentSkills: models.JSON(subAgentSkills),
133+
SubAgentDescription: req.SubAgentDescription,
134+
SubAgentInstructions: req.SubAgentInstructions,
135+
SubAgentModel: subAgentModel,
136+
SubAgentSkills: models.JSON(subAgentSkills),
129137
}
130138

131139
if err := s.db.Create(&agent).Error; err != nil {
@@ -197,8 +205,17 @@ func (s *Server) UpdateAgent(c *fiber.Ctx) error {
197205
updates["resources"] = models.JSON(raw)
198206
}
199207
if req.SubAgentDescription != nil {
208+
if len(*req.SubAgentDescription) > maxDescriptionSize {
209+
return fiber.NewError(fiber.StatusBadRequest, fmt.Sprintf("sub_agent_description exceeds maximum size of %d bytes", maxDescriptionSize))
210+
}
200211
updates["sub_agent_description"] = *req.SubAgentDescription
201212
}
213+
if req.SubAgentInstructions != nil {
214+
if len(*req.SubAgentInstructions) > maxInstructionsSize {
215+
return fiber.NewError(fiber.StatusBadRequest, fmt.Sprintf("sub_agent_instructions exceeds maximum size of %d bytes", maxInstructionsSize))
216+
}
217+
updates["sub_agent_instructions"] = *req.SubAgentInstructions
218+
}
202219
if req.SubAgentModel != nil {
203220
if *req.SubAgentModel != "" && !isValidSubAgentModel(*req.SubAgentModel) {
204221
return fiber.NewError(fiber.StatusBadRequest, "sub_agent_model must be one of: inherit, sonnet, opus, haiku")
@@ -348,6 +365,7 @@ func (s *Server) InstallAgentSkill(c *fiber.Ctx) error {
348365
subInfo := runtime.SubAgentInfo{
349366
Name: agent.Name,
350367
Description: agent.SubAgentDescription,
368+
Instructions: agent.SubAgentInstructions,
351369
Model: agent.SubAgentModel,
352370
Skills: json.RawMessage(updatedSkillsJSON),
353371
GlobalSkills: workerLeaderSkills,
@@ -360,7 +378,7 @@ func (s *Server) InstallAgentSkill(c *fiber.Ctx) error {
360378
filename := runtime.SubAgentFileName(agent.Name)
361379
agentsDir := agentsContainerDir(team.Provider)
362380
filePath := agentsDir + "/" + filename
363-
writeCmd := []string{"sh", "-c", fmt.Sprintf("printf '%%s' '%s' | base64 -d > %s", encoded, filePath)}
381+
writeCmd := []string{"sh", "-c", fmt.Sprintf("printf '%%s' '%s' | base64 -d > '%s'", encoded, filePath)}
364382

365383
if _, err := s.runtime.ExecInContainer(c.Context(), leader.ContainerID, writeCmd); err != nil {
366384
slog.Error("failed to update agent .md file in container", "agent", agent.Name, "error", err)
@@ -383,6 +401,7 @@ func (s *Server) InstallAgentSkill(c *fiber.Ctx) error {
383401
subInfo := runtime.SubAgentInfo{
384402
Name: w.Name,
385403
Description: w.SubAgentDescription,
404+
Instructions: w.SubAgentInstructions,
386405
Model: w.SubAgentModel,
387406
Skills: json.RawMessage(w.SubAgentSkills),
388407
GlobalSkills: globalSkills,
@@ -392,7 +411,7 @@ func (s *Server) InstallAgentSkill(c *fiber.Ctx) error {
392411
encoded := base64.StdEncoding.EncodeToString([]byte(content))
393412
filename := runtime.SubAgentFileName(w.Name)
394413
filePath := agentsDir + "/" + filename
395-
writeCmd := []string{"sh", "-c", fmt.Sprintf("printf '%%s' '%s' | base64 -d > %s", encoded, filePath)}
414+
writeCmd := []string{"sh", "-c", fmt.Sprintf("printf '%%s' '%s' | base64 -d > '%s'", encoded, filePath)}
396415

397416
if _, err := s.runtime.ExecInContainer(c.Context(), leader.ContainerID, writeCmd); err != nil {
398417
slog.Error("failed to update worker .md file after leader skill install", "worker", w.Name, "error", err)
@@ -411,6 +430,9 @@ func (s *Server) InstallAgentSkill(c *fiber.Ctx) error {
411430
// maxInstructionsSize is the maximum allowed size for agent instructions content (100KB).
412431
const maxInstructionsSize = 100 * 1024
413432

433+
// maxDescriptionSize is the maximum allowed size for sub-agent description (2KB).
434+
const maxDescriptionSize = 2 * 1024
435+
414436
// GetInstructions reads the instructions file from a running agent's container.
415437
func (s *Server) GetInstructions(c *fiber.Ctx) error {
416438
teamID := c.Params("id")

internal/api/handlers_chat.go

Lines changed: 169 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@ package api
22

33
import (
44
"context"
5+
"encoding/base64"
56
"encoding/json"
67
"fmt"
8+
"io"
79
"log/slog"
10+
"mime"
811
"os"
12+
"path/filepath"
13+
"regexp"
914
"strings"
1015
"time"
1116

@@ -17,7 +22,21 @@ import (
1722
"github.com/helmcode/agent-crew/internal/protocol"
1823
)
1924

25+
const (
26+
// maxFileSize is the maximum allowed size per uploaded file (10 MB).
27+
maxFileSize = 10 * 1024 * 1024
28+
// maxFileCount is the maximum number of files per chat message.
29+
maxFileCount = 5
30+
)
31+
32+
// unsafeFilenameChars matches characters that are not safe in filenames.
33+
var unsafeFilenameChars = regexp.MustCompile(`[^a-zA-Z0-9._-]`)
34+
35+
// allowedMIMEPrefixes lists the MIME type prefixes accepted for file uploads.
36+
var allowedMIMEPrefixes = []string{"text/", "image/", "application/pdf"}
37+
2038
// SendChat sends a user message to the team leader via NATS.
39+
// It supports both JSON (backward compat) and multipart/form-data with file uploads.
2140
func (s *Server) SendChat(c *fiber.Ctx) error {
2241
teamID := c.Params("id")
2342

@@ -30,17 +49,111 @@ func (s *Server) SendChat(c *fiber.Ctx) error {
3049
return fiber.NewError(fiber.StatusConflict, "team is not running")
3150
}
3251

33-
var req ChatRequest
34-
if err := c.BodyParser(&req); err != nil {
35-
return fiber.NewError(fiber.StatusBadRequest, "invalid request body")
36-
}
52+
var message string
53+
var fileRefs []protocol.FileRef
54+
55+
contentType := string(c.Request().Header.ContentType())
56+
mediaType, _, _ := mime.ParseMediaType(contentType)
57+
58+
if mediaType == "multipart/form-data" {
59+
// Parse multipart form.
60+
message = c.FormValue("message")
61+
if message == "" {
62+
return fiber.NewError(fiber.StatusBadRequest, "message is required")
63+
}
64+
65+
// Parse uploaded files.
66+
form, err := c.MultipartForm()
67+
if err != nil {
68+
return fiber.NewError(fiber.StatusBadRequest, "failed to parse multipart form")
69+
}
3770

38-
if req.Message == "" {
39-
return fiber.NewError(fiber.StatusBadRequest, "message is required")
71+
files := form.File["files"]
72+
if len(files) > maxFileCount {
73+
return fiber.NewError(fiber.StatusBadRequest, fmt.Sprintf("maximum %d files allowed", maxFileCount))
74+
}
75+
76+
if len(files) > 0 {
77+
// Find the leader container to write files into.
78+
var leader models.Agent
79+
if err := s.db.Where("team_id = ? AND role = ? AND container_status = ?",
80+
teamID, models.AgentRoleLeader, models.ContainerStatusRunning).First(&leader).Error; err != nil {
81+
return fiber.NewError(fiber.StatusConflict, "no running leader agent found for file upload")
82+
}
83+
84+
timestamp := time.Now().Unix()
85+
86+
for _, fh := range files {
87+
// Validate file size.
88+
if fh.Size > maxFileSize {
89+
return fiber.NewError(fiber.StatusBadRequest,
90+
fmt.Sprintf("file %q exceeds maximum size of %d bytes", fh.Filename, maxFileSize))
91+
}
92+
93+
// Validate MIME type.
94+
if !isAllowedMIME(fh.Header.Get("Content-Type")) {
95+
return fiber.NewError(fiber.StatusBadRequest,
96+
fmt.Sprintf("file %q has unsupported type %q; allowed: text/*, image/*, application/pdf",
97+
fh.Filename, fh.Header.Get("Content-Type")))
98+
}
99+
100+
// Sanitize filename.
101+
safeName := sanitizeFilename(fh.Filename)
102+
containerPath := fmt.Sprintf("/workspace/uploads/%d_%s", timestamp, safeName)
103+
104+
// Read file content.
105+
f, err := fh.Open()
106+
if err != nil {
107+
return fiber.NewError(fiber.StatusInternalServerError, "failed to read uploaded file")
108+
}
109+
data, err := io.ReadAll(io.LimitReader(f, maxFileSize+1))
110+
f.Close()
111+
if err != nil {
112+
return fiber.NewError(fiber.StatusInternalServerError, "failed to read uploaded file")
113+
}
114+
if int64(len(data)) > maxFileSize {
115+
return fiber.NewError(fiber.StatusBadRequest,
116+
fmt.Sprintf("file %q exceeds maximum size of %d bytes", fh.Filename, maxFileSize))
117+
}
118+
119+
// Write file to leader container using base64 + exec.
120+
encoded := base64.StdEncoding.EncodeToString(data)
121+
writeCmd := []string{"sh", "-c",
122+
fmt.Sprintf("mkdir -p /workspace/uploads && printf '%%s' '%s' | base64 -d > '%s'",
123+
encoded, containerPath)}
124+
if _, err := s.runtime.ExecInContainer(c.Context(), leader.ContainerID, writeCmd); err != nil {
125+
slog.Error("failed to write uploaded file to container",
126+
"file", safeName, "error", err)
127+
return fiber.NewError(fiber.StatusInternalServerError,
128+
fmt.Sprintf("failed to write file %q to container", fh.Filename))
129+
}
130+
131+
fileRefs = append(fileRefs, protocol.FileRef{
132+
Name: fh.Filename,
133+
Path: containerPath,
134+
Size: fh.Size,
135+
Type: fh.Header.Get("Content-Type"),
136+
})
137+
}
138+
}
139+
} else {
140+
// Fallback: JSON body (backward compatible).
141+
var req ChatRequest
142+
if err := c.BodyParser(&req); err != nil {
143+
return fiber.NewError(fiber.StatusBadRequest, "invalid request body")
144+
}
145+
if req.Message == "" {
146+
return fiber.NewError(fiber.StatusBadRequest, "message is required")
147+
}
148+
message = req.Message
40149
}
41150

42151
// Log to task log for persistence and Activity panel.
43-
content, _ := json.Marshal(map[string]string{"content": req.Message})
152+
logPayload := map[string]interface{}{"content": message}
153+
if len(fileRefs) > 0 {
154+
logPayload["files"] = fileRefs
155+
}
156+
content, _ := json.Marshal(logPayload)
44157
taskLog := models.TaskLog{
45158
ID: uuid.New().String(),
46159
TeamID: teamID,
@@ -53,7 +166,11 @@ func (s *Server) SendChat(c *fiber.Ctx) error {
53166

54167
// Publish to NATS leader channel so the agent actually receives the message.
55168
sanitizedName := SanitizeName(team.Name)
56-
if err := s.publishToTeamNATS(sanitizedName, req.Message); err != nil {
169+
payload := protocol.UserMessagePayload{
170+
Content: message,
171+
Files: fileRefs,
172+
}
173+
if err := s.publishToTeamNATS(sanitizedName, payload); err != nil {
57174
slog.Error("failed to publish chat to NATS", "team", team.Name, "error", err)
58175
return c.JSON(fiber.Map{
59176
"status": "queued",
@@ -72,7 +189,7 @@ func (s *Server) SendChat(c *fiber.Ctx) error {
72189
// to avoid managing per-team NATS connections in the API server.
73190
// It retries up to 3 times to handle cases where the NATS container was just
74191
// recreated (e.g. after port binding fix).
75-
func (s *Server) publishToTeamNATS(teamName, message string) error {
192+
func (s *Server) publishToTeamNATS(teamName string, payload protocol.UserMessagePayload) error {
76193
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
77194
defer cancel()
78195

@@ -124,9 +241,7 @@ func (s *Server) publishToTeamNATS(teamName, message string) error {
124241
defer nc.Close()
125242

126243
// Build the protocol message.
127-
msg, err := protocol.NewMessage("user", "leader", protocol.TypeUserMessage, protocol.UserMessagePayload{
128-
Content: message,
129-
})
244+
msg, err := protocol.NewMessage("user", "leader", protocol.TypeUserMessage, payload)
130245
if err != nil {
131246
return fmt.Errorf("building protocol message: %w", err)
132247
}
@@ -256,3 +371,45 @@ func splitCSV(s string) []string {
256371
}
257372
return result
258373
}
374+
375+
// sanitizeFilename strips path separators, null bytes, and special characters
376+
// from a filename to prevent path traversal and injection attacks.
377+
func sanitizeFilename(name string) string {
378+
// Extract base name to strip any directory components.
379+
name = filepath.Base(name)
380+
381+
// Remove null bytes.
382+
name = strings.ReplaceAll(name, "\x00", "")
383+
384+
// Replace unsafe characters with underscores.
385+
name = unsafeFilenameChars.ReplaceAllString(name, "_")
386+
387+
// Collapse consecutive underscores.
388+
for strings.Contains(name, "__") {
389+
name = strings.ReplaceAll(name, "__", "_")
390+
}
391+
392+
name = strings.Trim(name, "_.")
393+
394+
if name == "" || name == "." || name == ".." {
395+
name = "upload"
396+
}
397+
398+
// Truncate to 255 characters.
399+
if len(name) > 255 {
400+
name = name[:255]
401+
}
402+
403+
return name
404+
}
405+
406+
// isAllowedMIME checks whether a MIME type is in the allowed list.
407+
func isAllowedMIME(mimeType string) bool {
408+
mimeType = strings.ToLower(strings.TrimSpace(mimeType))
409+
for _, prefix := range allowedMIMEPrefixes {
410+
if strings.HasPrefix(mimeType, prefix) {
411+
return true
412+
}
413+
}
414+
return false
415+
}

0 commit comments

Comments
 (0)