Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions components/backend/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
)

require (
github.com/bmatcuk/doublestar/v4 v4.9.1 // indirect
github.com/bytedance/sonic v1.13.3 // indirect
github.com/bytedance/sonic/loader v0.2.4 // indirect
github.com/cloudwego/base64x v0.1.5 // indirect
Expand Down
2 changes: 2 additions & 0 deletions components/backend/go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/bmatcuk/doublestar/v4 v4.9.1 h1:X8jg9rRZmJd4yRy7ZeNDRnM+T3ZfHv15JiBJ/avrEXE=
github.com/bmatcuk/doublestar/v4 v4.9.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/bytedance/sonic v1.13.3 h1:MS8gmaH16Gtirygw7jV91pDCN33NyMrPbN7qiYhEsF0=
github.com/bytedance/sonic v1.13.3/go.mod h1:o68xyaF9u2gvVBuGHPlUVCy+ZfmNNO5ETf1+KgkJhz4=
github.com/bytedance/sonic/loader v0.1.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU=
Expand Down
126 changes: 122 additions & 4 deletions components/backend/handlers/content.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,29 @@ import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
"log"
"net/http"
"os"
"os/exec"
"path/filepath"
"sort"
"strings"
"time"

"ambient-code-backend/git"

"github.com/bmatcuk/doublestar/v4"
"github.com/gin-gonic/gin"
)

// StateBaseDir is the base directory for content storage
// Set by main during initialization
var StateBaseDir string

// MaxResultFileSize is the maximum size for result files to prevent memory issues
const MaxResultFileSize = 10 * 1024 * 1024 // 10MB

// Git operation functions - set by main package during initialization
// These are set to the actual implementations from git package
var (
Expand Down Expand Up @@ -601,10 +607,11 @@ func parseFrontmatter(filePath string) map[string]string {

// AmbientConfig represents the ambient.json configuration
type AmbientConfig struct {
Name string `json:"name"`
Description string `json:"description"`
SystemPrompt string `json:"systemPrompt"`
ArtifactsDir string `json:"artifactsDir"`
Name string `json:"name"`
Description string `json:"description"`
SystemPrompt string `json:"systemPrompt"`
ArtifactsDir string `json:"artifactsDir"`
Results map[string]string `json:"results,omitempty"` // displayName -> glob pattern
}

// parseAmbientConfig reads and parses ambient.json from workflow directory
Expand Down Expand Up @@ -640,6 +647,117 @@ func parseAmbientConfig(workflowDir string) *AmbientConfig {
return &config
}

// ResultFile represents a workflow result file
type ResultFile struct {
DisplayName string `json:"displayName"`
Path string `json:"path"` // Relative path from workspace
Exists bool `json:"exists"`
Content string `json:"content,omitempty"`
Error string `json:"error,omitempty"`
}

// ContentWorkflowResults handles GET /content/workflow-results?session=
func ContentWorkflowResults(c *gin.Context) {
sessionName := c.Query("session")
if sessionName == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "missing session parameter"})
return
}

workflowDir := findActiveWorkflowDir(sessionName)
if workflowDir == "" {
c.JSON(http.StatusOK, gin.H{"results": []ResultFile{}})
return
}

ambientConfig := parseAmbientConfig(workflowDir)
if len(ambientConfig.Results) == 0 {
c.JSON(http.StatusOK, gin.H{"results": []ResultFile{}})
return
}

workspaceBase := filepath.Join(StateBaseDir, "sessions", sessionName, "workspace")
results := []ResultFile{}

// Sort keys to ensure consistent order (maps are unordered in Go)
displayNames := make([]string, 0, len(ambientConfig.Results))
for displayName := range ambientConfig.Results {
displayNames = append(displayNames, displayName)
}
sort.Strings(displayNames)

for _, displayName := range displayNames {
pattern := ambientConfig.Results[displayName]
matches := findMatchingFiles(workspaceBase, pattern)

if len(matches) == 0 {
results = append(results, ResultFile{
DisplayName: displayName,
Path: pattern,
Exists: false,
})
} else {
// Sort matches for consistent order
sort.Strings(matches)

for _, matchedPath := range matches {
relPath, _ := filepath.Rel(workspaceBase, matchedPath)

result := ResultFile{
DisplayName: displayName,
Path: relPath,
Exists: true,
}

// Check file size before reading
fileInfo, statErr := os.Stat(matchedPath)
if statErr != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical: Error handling issue

The filepath.Rel error is silently ignored with relPath, _ := filepath.Rel(...). This could lead to incorrect paths if the workspace base and matched path are on different volumes or have issues.

Recommendation:

relPath, err := filepath.Rel(workspaceBase, matchedPath)
if err != nil {
    result.Error = fmt.Sprintf("Failed to compute relative path: %v", err)
    results = append(results, result)
    continue
}

result.Error = fmt.Sprintf("Failed to stat file: %v", statErr)
results = append(results, result)
continue
}

if fileInfo.Size() > MaxResultFileSize {
result.Error = fmt.Sprintf("File too large (%d bytes, max %d)", fileInfo.Size(), MaxResultFileSize)
results = append(results, result)
continue
}

// Read file content
content, readErr := os.ReadFile(matchedPath)
if readErr != nil {
result.Error = fmt.Sprintf("Failed to read: %v", readErr)
} else {
result.Content = string(content)
}

results = append(results, result)
}
}
}

c.JSON(http.StatusOK, gin.H{"results": results})
}

// findMatchingFiles finds files matching a glob pattern with ** support for recursive matching
func findMatchingFiles(baseDir, pattern string) []string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Major: Security & Resource Management

The doublestar.Glob implementation has potential issues:

  1. Directory Traversal Risk: No validation that matched paths stay within baseDir
  2. Resource Exhaustion: No limit on number of matches (e.g., **/* could match thousands of files)
  3. Error Logging Only: Errors are logged but empty slice is returned - caller cannot distinguish between "no matches" and "glob failed"

Recommendations:

func findMatchingFiles(baseDir, pattern string) ([]string, error) {
    // Limit glob matches to prevent resource exhaustion
    const maxMatches = 100
    
    fsys := os.DirFS(baseDir)
    matches, err := doublestar.Glob(fsys, pattern)
    if err != nil {
        return nil, fmt.Errorf("glob pattern error: %w", err)
    }
    
    if len(matches) > maxMatches {
        return nil, fmt.Errorf("too many matches (%d), maximum is %d", len(matches), maxMatches)
    }
    
    // Validate paths stay within baseDir
    var absolutePaths []string
    for _, match := range matches {
        absPath := filepath.Join(baseDir, match)
        cleanPath := filepath.Clean(absPath)
        
        // Ensure path is under baseDir (prevent traversal)
        if !strings.HasPrefix(cleanPath, filepath.Clean(baseDir)) {
            log.Printf("SECURITY: path traversal attempt blocked: %s", match)
            continue
        }
        
        absolutePaths = append(absolutePaths, cleanPath)
    }
    
    return absolutePaths, nil
}

Then update caller to handle errors properly.

// Use doublestar for glob matching with ** support
fsys := os.DirFS(baseDir)
matches, err := doublestar.Glob(fsys, pattern)
if err != nil {
log.Printf("findMatchingFiles: glob error for pattern %q: %v", pattern, err)
return []string{}
}

// Convert relative paths to absolute paths
var absolutePaths []string
for _, match := range matches {
absolutePaths = append(absolutePaths, filepath.Join(baseDir, match))
}

return absolutePaths
}

// findActiveWorkflowDir finds the active workflow directory for a session
func findActiveWorkflowDir(sessionName string) string {
// Workflows are stored at {StateBaseDir}/sessions/{session-name}/workspace/workflows/{workflow-name}
Expand Down
58 changes: 58 additions & 0 deletions components/backend/handlers/sessions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,64 @@ func GetWorkflowMetadata(c *gin.Context) {
c.Data(resp.StatusCode, "application/json", b)
}

// GetWorkflowResults retrieves workflow result files from the active workflow
// GET /api/projects/:projectName/agentic-sessions/:sessionName/workflow/results
func GetWorkflowResults(c *gin.Context) {
project := c.GetString("project")
if project == "" {
project = c.Param("projectName")
}
sessionName := c.Param("sessionName")

if project == "" {
log.Printf("GetWorkflowResults: project is empty, session=%s", sessionName)
c.JSON(http.StatusBadRequest, gin.H{"error": "Project namespace required"})
return
}

// Get authorization token
token := c.GetHeader("Authorization")
if strings.TrimSpace(token) == "" {
token = c.GetHeader("X-Forwarded-Access-Token")
}

// Try temp service first (for completed sessions), then regular service
serviceName := fmt.Sprintf("temp-content-%s", sessionName)
reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s != nil {
if _, err := reqK8s.CoreV1().Services(project).Get(c.Request.Context(), serviceName, v1.GetOptions{}); err != nil {
// Temp service doesn't exist, use regular service
serviceName = fmt.Sprintf("ambient-content-%s", sessionName)
}
} else {
serviceName = fmt.Sprintf("ambient-content-%s", sessionName)
}

// Build URL to content service
endpoint := fmt.Sprintf("http://%s.%s.svc:8080", serviceName, project)
u := fmt.Sprintf("%s/content/workflow-results?session=%s", endpoint, sessionName)

log.Printf("GetWorkflowResults: project=%s session=%s endpoint=%s", project, sessionName, endpoint)

// Create and send request to content pod
req, _ := http.NewRequestWithContext(c.Request.Context(), http.MethodGet, u, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Major: Error ignored

http.NewRequestWithContext can return an error (though rarely), but it's being silently ignored with req, _ := ....

Fix:

req, err := http.NewRequestWithContext(c.Request.Context(), http.MethodGet, u, nil)
if err != nil {
    log.Printf("GetWorkflowResults: failed to create request: %v", err)
    c.JSON(http.StatusInternalServerError, gin.H{"error": "Internal error"})
    return
}

if strings.TrimSpace(token) != "" {
req.Header.Set("Authorization", token)
}
client := &http.Client{Timeout: 4 * time.Second}
resp, err := client.Do(req)
if err != nil {
log.Printf("GetWorkflowResults: content service request failed: %v", err)
// Return empty results on error
c.JSON(http.StatusOK, gin.H{"results": []interface{}{}})
return
}
defer resp.Body.Close()

b, _ := io.ReadAll(resp.Body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Minor: Ignored error

io.ReadAll error is ignored. While the response body has already been checked, it's better practice to handle this:

b, err := io.ReadAll(resp.Body)
if err != nil {
    log.Printf("GetWorkflowResults: failed to read response: %v", err)
    c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to read results"})
    return
}
c.Data(resp.StatusCode, "application/json", b)

c.Data(resp.StatusCode, "application/json", b)
}

// fetchGitHubFileContent fetches a file from GitHub via API
// token is optional - works for public repos without authentication (but has rate limits)
func fetchGitHubFileContent(ctx context.Context, owner, repo, ref, path, token string) ([]byte, error) {
Expand Down
2 changes: 2 additions & 0 deletions components/backend/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func registerContentRoutes(r *gin.Engine) {
r.POST("/content/git-configure-remote", handlers.ContentGitConfigureRemote)
r.POST("/content/git-sync", handlers.ContentGitSync)
r.GET("/content/workflow-metadata", handlers.ContentWorkflowMetadata)
r.GET("/content/workflow-results", handlers.ContentWorkflowResults)
r.GET("/content/git-merge-status", handlers.ContentGitMergeStatus)
r.POST("/content/git-pull", handlers.ContentGitPull)
r.POST("/content/git-push", handlers.ContentGitPushToBranch)
Expand Down Expand Up @@ -74,6 +75,7 @@ func registerRoutes(r *gin.Engine) {
projectGroup.DELETE("/agentic-sessions/:sessionName/content-pod", handlers.DeleteContentPod)
projectGroup.POST("/agentic-sessions/:sessionName/workflow", handlers.SelectWorkflow)
projectGroup.GET("/agentic-sessions/:sessionName/workflow/metadata", handlers.GetWorkflowMetadata)
projectGroup.GET("/agentic-sessions/:sessionName/workflow/results", handlers.GetWorkflowResults)
projectGroup.POST("/agentic-sessions/:sessionName/repos", handlers.AddRepo)
projectGroup.DELETE("/agentic-sessions/:sessionName/repos/:repoName", handlers.RemoveRepo)

Expand Down
48 changes: 48 additions & 0 deletions components/frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions components/frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"devDependencies": {
"@eslint/eslintrc": "^3",
"@tailwindcss/postcss": "^4",
"@tailwindcss/typography": "^0.5.19",
"@types/node": "^20",
"@types/react": "^19",
"@types/react-dom": "^19",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { BACKEND_URL } from '@/lib/config';
import { buildForwardHeadersAsync } from '@/lib/auth';

export async function GET(
request: Request,
{ params }: { params: Promise<{ name: string; sessionName: string }> },
) {
const { name, sessionName } = await params;
const headers = await buildForwardHeadersAsync(request);
const resp = await fetch(
`${BACKEND_URL}/projects/${encodeURIComponent(name)}/agentic-sessions/${encodeURIComponent(sessionName)}/workflow/results`,
{ headers }
);
const data = await resp.text();
return new Response(data, { status: resp.status, headers: { 'Content-Type': 'application/json' } });
}

Loading
Loading