Add proxy-logs workload endpoint

[peppescg]

Summary

Based on a proposal stacklok/toolhive-studio#934 from @danbarr showing proxy logs on UI ( that is particularly useful for remote mcp) I created this PR, in order to expose the endpoint and let the UI show the logs.

Add new API endpoint to retrieve proxy logs for MCP workloads.

Changes

• New endpoint: GET /api/v1beta/workloads/{name}/proxy-logs
• Functionality: Reads proxy log files from .../toolhive/logs/{workload-name}.log
• Response format: Plain text logs content
• Error handling: Returns 404 when log file doesn't exist

API Details

Endpoint

Responses

• 200 OK: Returns the proxy log file content as plain text
• 404 Not Found: When the proxy log file doesn't exist for the specified workload
• 500 Internal Server Error: For filesystem or other internal errors

Usage

This endpoint allows the UI to display proxy logs for individual MCP servers, providing visibility into proxy process activity and debugging information.

Implementation

• Added getProxyLogsForWorkload handler function
• Registered new route in the workloads router
• Includes proper path sanitization to prevent directory traversal
• Follows existing API patterns and error handling conventions
• Updated OpenAPI/Swagger documentation

Testing

The endpoint can be tested with:

curl -X GET "http://localhost:8080/api/v1beta/workloads/{workload-name}/proxy-logs" \
  -H "accept: text/plain"

[codecov]

Codecov Report

❌ Patch coverage is 0% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.41%. Comparing base (0091210) to head (18eb8b1).

Files with missing lines	Patch %	Lines
pkg/api/v1/workloads.go	0.00%	28 Missing ⚠️
pkg/workloads/manager.go	0.00%	17 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2208      +/-   ##
==========================================
- Coverage   53.54%   53.41%   -0.13%     
==========================================
  Files         228      228              
  Lines       29313    29358      +45     
==========================================
- Hits        15696    15683      -13     
- Misses      12489    12552      +63     
+ Partials     1128     1123       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[amirejaz]

@claude, could you review the PR with a security hat on?

[claude]

Claude finished @amirejaz's task —— View job

---

Security Review

I've completed a security-focused review of the proxy-logs endpoint implementation. Here are my findings:

---

Critical Security Issue

🔴 Missing Workload Name Validation

The getProxyLogsForWorkload function does not validate the workload name parameter before using it to construct a file path. This is a critical path traversal vulnerability.

Location: pkg/api/v1/workloads.go:562-597

The Problem:

func (s *WorkloadRoutes) getProxyLogsForWorkload(w http.ResponseWriter, r *http.Request) {
    name := chi.URLParam(r, "name")  // ❌ No validation
    
    // Directly constructs path with unvalidated input
    logFilePath, err := xdg.DataFile(fmt.Sprintf("toolhive/logs/%s.log", name))

Why This is Critical:

1. The workload name comes directly from the URL path parameter without any validation
2. While filepath.Clean() is called (line 574), this happens after xdg.DataFile() has already constructed the path
3. An attacker can still read arbitrary .log files within the data directory by crafting malicious workload names
4. The PR description claims "proper path sanitization to prevent directory traversal," but this is misleading - the sanitization occurs too late

Attack Example:

# Could potentially read other log files
curl "http://localhost:8080/api/v1beta/workloads/../../../etc/passwd%00.log/proxy-logs"

# Or traverse to other workload logs
curl "http://localhost:8080/api/v1beta/workloads/../../other-workload/proxy-logs"

Comparison with Other Endpoints:
Every other endpoint that accepts a workload name validates it:

• getWorkload (line 144): Validates via GetWorkload() which checks ErrInvalidWorkloadName
• stopWorkload (line 192): Validates via ErrInvalidWorkloadName check
• getLogsForWorkload (line 533): Validates via GetLogs() which validates the workload exists

The Fix:
The endpoint must validate the workload name before using it. The codebase has a dedicated validation function:

func (s *WorkloadRoutes) getProxyLogsForWorkload(w http.ResponseWriter, r *http.Request) {
    name := chi.URLParam(r, "name")
    
    // ✅ Add validation BEFORE constructing path
    if err := wt.ValidateWorkloadName(name); err != nil {
        http.Error(w, "Invalid workload name: "+err.Error(), http.StatusBadRequest)
        return
    }
    
    // Now safe to proceed with validated name
    logFilePath, err := xdg.DataFile(fmt.Sprintf("toolhive/logs/%s.log", name))
    // ... rest of function
}

The ValidateWorkloadName function (pkg/workloads/types/validate.go:103) provides comprehensive protection against:

• Path traversal (..)
• Absolute paths
• Command injection
• Null bytes
• Invalid characters
• Excessive length

---

Additional Security Observations

🟡 Information Disclosure

Issue: Returns entire log file contents without size limits (line 583: os.ReadFile(cleanLogFilePath))

Risk:

• Large log files could cause memory exhaustion or DoS
• May expose sensitive information (API keys, tokens, internal paths) if logs aren't carefully sanitized
• No pagination or size limits

Recommendation:

• Implement file size limits before reading
• Consider streaming or pagination for large files
• Add rate limiting to prevent abuse
• Review what sensitive data might be logged

---

🟢 Authentication/Authorization

Good: The endpoint inherits authentication middleware from the router setup (pkg/api/server.go:152-156):

• All workload routes go through GetAuthenticationMiddleware()
• Supports OIDC/OAuth authentication when configured
• Uses anonymous auth by default for local access

Note: No explicit authorization checks per workload - any authenticated user can access any workload's logs. This may be acceptable depending on your threat model, but consider if workload-level access control is needed.

---

🟢 Error Handling

The error handling is generally good:

• Generic error messages don't leak internal paths
• Proper HTTP status codes (404, 500)
• Logs detailed errors server-side (lines 568, 585, 593)

---

Additional Recommendations

1. Add Unit Tests: The codecov report shows 0% coverage for this new code. Security-critical path handling should have comprehensive tests including:

   • Path traversal attempts
   • Invalid workload names
   • Non-existent files
   • Large files

2. Consider Workload Existence Check: Unlike other endpoints, this doesn't verify the workload exists before attempting to read logs. This could leak information about file system structure.

3. Add Content-Type Validation: Ensure the response is always treated as plain text to prevent any injection attacks if log content is displayed in a browser.

---

Summary

Must Fix Before Merge:

• ❌ Add workload name validation using ValidateWorkloadName() before constructing file paths

Should Consider:

• ⚠️ Add file size limits
• ⚠️ Add comprehensive tests
• ⚠️ Verify workload exists before reading logs
• ⚠️ Review logged content for sensitive data exposure

The core security issue is straightforward to fix by adding one validation call at the beginning of the function, following the pattern used by all other workload endpoints in this file.

---

Todo List:

• Read the implementation in pkg/api/v1/workloads.go
• Review OpenAPI specification changes
• Analyze security controls
• Provide security review feedback

---

Job run: https://github.com/stacklok/toolhive/actions/runs/${{ github.run_id }}
Branch: add-proxy-logs-endpoint

[eleftherias]

I updated the PR title to match out commit conventions