fix: security and reliability — path traversal protection, dummy vertex token, bash errors, marshal errors

Author: Patel230

bedrock.go

@@ -77,7 +77,10 @@ func (p *BedrockProvider) Complete(ctx context.Context, messages []Message, opts
 		body["temperature"] = p.config.Temperature
 	}
 
-	jsonBody, _ := json.Marshal(body)
+	jsonBody, err := json.Marshal(body)
+	if err != nil {
+		return "", fmt.Errorf("marshal request: %w", err)
+	}
 
 	req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(jsonBody))
 	if err != nil {
@@ -95,7 +98,10 @@ func (p *BedrockProvider) Complete(ctx context.Context, messages []Message, opts
 	}
 	defer resp.Body.Close()
 
-	respBody, _ := io.ReadAll(resp.Body)
+	respBody, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", fmt.Errorf("read response: %w", err)
+	}
 
 	if resp.StatusCode != http.StatusOK {
 		return "", fmt.Errorf("Bedrock error (%d): %s", resp.StatusCode, string(respBody))

openairesponses.go

@@ -205,7 +205,10 @@ func (p *OpenAIResponsesProvider) Stream(ctx context.Context, config StreamConfi
 		Store:       false,
 	}
 
-	jsonBody, _ := json.Marshal(body)
+	jsonBody, err := json.Marshal(body)
+	if err != nil {
+		return Message{}, fmt.Errorf("marshal request: %w", err)
+	}
 
 	req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(jsonBody))
 	if err != nil {
@@ -223,7 +226,7 @@ func (p *OpenAIResponsesProvider) Stream(ctx context.Context, config StreamConfi
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		respBody, _ := io.ReadAll(resp.Body)
+		respBody, _ := io.ReadAll(resp.Body) // best-effort for error message
 		return Message{}, fmt.Errorf("OpenAI Responses API error (%d): %s", resp.StatusCode, string(respBody))
 	}
 

tools.go

@@ -50,6 +50,24 @@ func isCommandDangerous(cmd string) bool {
 	return false
 }
 
+// safeJoin joins repoPath with the user-supplied relative path and ensures the
+// result stays within repoPath, preventing path traversal attacks.
+func safeJoin(repoPath, rel string) (string, error) {
+	joined := filepath.Join(repoPath, rel)
+	absRepo, err := filepath.Abs(repoPath)
+	if err != nil {
+		return "", fmt.Errorf("invalid repo path: %w", err)
+	}
+	absJoined, err := filepath.Abs(joined)
+	if err != nil {
+		return "", fmt.Errorf("invalid path: %w", err)
+	}
+	if !strings.HasPrefix(absJoined, absRepo+string(filepath.Separator)) && absJoined != absRepo {
+		return "", fmt.Errorf("path %q is outside the repository", rel)
+	}
+	return absJoined, nil
+}
+
 // DefaultTools returns all built-in tools available to the agent.
 func DefaultTools(repoPath string) []Tool {
 	return []Tool{
@@ -86,8 +104,8 @@ func BashTool(repoPath string) Tool {
 			var out bytes.Buffer
 			c.Stdout = &out
 			c.Stderr = &out
-			_ = c.Run()
-			return out.String(), nil
+			err := c.Run()
+			return out.String(), err
 		},
 	}
 }
@@ -97,7 +115,10 @@ func ReadFileTool(repoPath string) Tool {
 		Name:        "read_file",
 		Description: "Read a file from the repo.\nArgs: {\"path\": \"internal/agent/agent.go\"}",
 		Execute: func(ctx context.Context, args map[string]string) (string, error) {
-			path := filepath.Join(repoPath, args["path"])
+			path, err := safeJoin(repoPath, args["path"])
+			if err != nil {
+				return "", err
+			}
 			data, err := os.ReadFile(path)
 			if err != nil {
 				return "", fmt.Errorf("read %s: %w", args["path"], err)
@@ -112,7 +133,10 @@ func WriteFileTool(repoPath string) Tool {
 		Name:        "write_file",
 		Description: "Write or overwrite a file in the repo.\nArgs: {\"path\": \"internal/agent/agent.go\", \"content\": \"...\"}",
 		Execute: func(ctx context.Context, args map[string]string) (string, error) {
-			path := filepath.Join(repoPath, args["path"])
+			path, err := safeJoin(repoPath, args["path"])
+			if err != nil {
+				return "", err
+			}
 			if isPathProtected(path) {
 				return "", fmt.Errorf("write to %s is protected", args["path"])
 			}
@@ -132,7 +156,10 @@ func EditFileTool(repoPath string) Tool {
 		Name:        "edit_file",
 		Description: "Edit a file by replacing oldString with newString.\nArgs: {\"path\": \"file.go\", \"oldString\": \"old\", \"newString\": \"new\"}",
 		Execute: func(ctx context.Context, args map[string]string) (string, error) {
-			path := filepath.Join(repoPath, args["path"])
+			path, err := safeJoin(repoPath, args["path"])
+			if err != nil {
+				return "", err
+			}
 			if isPathProtected(path) {
 				return "", fmt.Errorf("edit %s is protected", args["path"])
 			}
@@ -194,7 +221,11 @@ func SearchTool(repoPath string) Tool {
 			}
 			path := repoPath
 			if args["path"] != "" {
-				path = filepath.Join(repoPath, args["path"])
+				var err error
+				path, err = safeJoin(repoPath, args["path"])
+				if err != nil {
+					return "", err
+				}
 			}
 
 			c := exec.CommandContext(ctx, "grep", "-r", "-n", pattern, path)

vertex.go

@@ -54,7 +54,7 @@ func (p *VertexProvider) getAccessToken(ctx context.Context) (string, error) {
 		if err := json.Unmarshal(data, &creds); err != nil {
 			return "", err
 		}
-		return "dummy_token_from_service_account", nil
+		return "", fmt.Errorf("service account credentials file found but JWT signing is not implemented; set GOOGLE_ACCESS_TOKEN instead")
 	}
 
 	tokenSrc := os.Getenv("GOOGLE_ACCESS_TOKEN")