Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 11 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
LISTEN_ADDR=:8090
REDIS_URL=redis://localhost:6379

# Required. Shared with the CRM, which sends it as X-Bot-Runtime-Secret on /events.
# An empty value would authenticate any caller that omits the header.
BOT_RUNTIME_SECRET=

AI_CALL_TIMEOUT_SECONDS=30

# Optional. Extra hosts allowed to serve incoming media, comma-separated, no scheme
# or port (e.g. "minio.internal,cdn.example.com"). By default only the host of the
# event's postback_url is allowed. Set this when blobs are served elsewhere
# (ActiveStorage redirect mode, CDN); media on an unlisted host is skipped and
# logged as pipeline.ai.attachment.blocked_url, and the text reply still goes out.
MEDIA_HOST_ALLOWLIST=
41 changes: 41 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: CI

# Nothing ran the Go suite on a PR before this: test/e2e sat non-compiling from
# EVO-558 to EVO-2180 without a single red check. Redis is a service container
# because the repository tests need a real one.

on:
pull_request:
push:
branches: [develop, main]

jobs:
test:
runs-on: ubuntu-latest
services:
redis:
image: redis:7-alpine
ports: ['6379:6379']
options: >-
--health-cmd "redis-cli ping"
--health-interval 5s
--health-timeout 3s
--health-retries 10
env:
REDIS_TEST_URL: redis://localhost:6379
steps:
- uses: actions/checkout@v4

- uses: actions/setup-go@v5
with:
go-version-file: go.mod
cache: true

- name: Build
run: go build ./...

- name: Vet
run: go vet ./...

- name: Test
run: go test ./...
7 changes: 6 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ func Load() (*Config, error) {
if err != nil {
return nil, err
}
botRuntimeSecret := os.Getenv("BOT_RUNTIME_SECRET")
// Required: SecretMiddleware compares the header against this, so an empty value
// authenticates every caller that omits it.
botRuntimeSecret, err := mustGetEnv("BOT_RUNTIME_SECRET")
if err != nil {
return nil, err
}
aiCallTimeout, err := getEnvIntOrDefault("AI_CALL_TIMEOUT_SECONDS", 30)
if err != nil {
return nil, err
Expand Down
3 changes: 3 additions & 0 deletions pkg/ai/model/a2a.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ type A2ARequest struct {
Message string // aggregated buffer content (FR-15)
Metadata map[string]any // CRM metadata passed through to processor (tools context)
Attachments []Attachment // EVO-2180: incoming media to forward as A2A file parts
// PostbackURL anchors the media host allowlist (see allowedMediaHosts); it is
// not used for the A2A call itself.
PostbackURL string
}

// Attachment is an incoming media item (image/audio/…) the adapter downloads and
Expand Down
101 changes: 94 additions & 7 deletions pkg/ai/service/ai_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"mime"
"net/http"
neturl "net/url"
"os"
"path"
"strings"
"time"
Expand Down Expand Up @@ -49,6 +50,11 @@ const (
attachmentsTotalTimeFactor = 3
)

// mediaHostAllowlistEnv names extra hosts authorized to serve incoming media,
// comma-separated, for deployments serving blobs off the CRM host (ActiveStorage
// redirect mode, CDN). The default anchor is the event's own postback host.
const mediaHostAllowlistEnv = "MEDIA_HOST_ALLOWLIST"

// maxBackoff caps the exponential backoff between retries so a large
// AI_CALL_RETRY_BASE_MS or retry count cannot balloon the wait.
const maxBackoff = 5 * time.Second
Expand Down Expand Up @@ -333,12 +339,26 @@ func (a *aiAdapter) buildFileParts(ctx context.Context, req *model.A2ARequest) [
budgetCtx, cancelBudget := context.WithTimeout(ctx, attachmentsTotalTimeFactor*perDownload)
defer cancelBudget()

// Built once per turn: the client closes over the authorized hosts.
hosts := allowedMediaHosts(req.PostbackURL)
client := a.mediaClient(hosts)

parts := make([]model.JSONRPCPart, 0, len(req.Attachments))
remaining := maxAttachmentsTotalBytes
for i, att := range req.Attachments {
if att.URL == "" {
continue
}
if err := checkMediaURL(att.URL, hosts); err != nil {
slog.Warn("pipeline.ai.attachment.blocked_url",
"contact_id", req.ContactID,
"conversation_id", req.ConversationID,
"file_type", att.FileType,
"error", err,
"hint", "set "+mediaHostAllowlistEnv+" when blobs are served off the CRM host",
)
continue
}
if budgetCtx.Err() != nil {
slog.Warn("pipeline.ai.attachment.budget_exhausted",
"contact_id", req.ContactID,
Expand All @@ -360,12 +380,15 @@ func (a *aiAdapter) buildFileParts(ctx context.Context, req *model.A2ARequest) [
)
break
}
data, respContentType, err := a.downloadAttachment(budgetCtx, att.URL, perDownload, limit)
data, respContentType, err := downloadAttachment(budgetCtx, client, att.URL, perDownload, limit)
if err != nil {
// Status separates the common 404-from-an-expired-signed-link case from
// an unreachable host; they logged identically before.
slog.Warn("pipeline.ai.attachment.download_failed",
"contact_id", req.ContactID,
"conversation_id", req.ConversationID,
"file_type", att.FileType,
"status", statusOf(err),
"error", err,
)
continue
Expand Down Expand Up @@ -412,24 +435,88 @@ func (a *aiAdapter) attachmentTimeout() time.Duration {
return d
}

// downloadAttachment GETs the URL with the adapter's client and the given timeout,
// reading at most limit bytes. It returns the body and the response Content-Type so
// the caller can decide what the bytes actually are.
func (a *aiAdapter) downloadAttachment(ctx context.Context, url string, timeout time.Duration, limit int) ([]byte, string, error) {
// allowedMediaHosts returns the hostnames authorized to serve this event's media.
// An empty set authorizes nothing: failing closed beats fetching a URL the caller
// chose.
func allowedMediaHosts(postbackURL string) map[string]struct{} {
hosts := make(map[string]struct{}, 2)
if u, err := neturl.Parse(postbackURL); err == nil {
if h := strings.ToLower(u.Hostname()); h != "" {
hosts[h] = struct{}{}
}
}
for _, h := range strings.Split(os.Getenv(mediaHostAllowlistEnv), ",") {
if h = strings.ToLower(strings.TrimSpace(h)); h != "" {
hosts[h] = struct{}{}
}
}
return hosts
}

// checkMediaURL reports why a media URL must not be fetched, or nil when it may be.
func checkMediaURL(rawURL string, hosts map[string]struct{}) error {
u, err := neturl.Parse(rawURL)
if err != nil {
return fmt.Errorf("unparseable media url: %w", err)
}
if scheme := strings.ToLower(u.Scheme); scheme != "http" && scheme != "https" {
return fmt.Errorf("scheme %q is not allowed for media", u.Scheme)
}
host := strings.ToLower(u.Hostname())
if host == "" {
return errors.New("media url has no host")
}
if _, ok := hosts[host]; !ok {
return fmt.Errorf("host %q is not authorized to serve media for this event", host)
}
return nil
}

// mediaClient shares the adapter's transport but re-runs checkMediaURL on every
// redirect hop, so an authorized host cannot 302 the download onto an internal one.
func (a *aiAdapter) mediaClient(hosts map[string]struct{}) *http.Client {
return &http.Client{
Transport: a.client.Transport,
CheckRedirect: func(req *http.Request, via []*http.Request) error {
if len(via) >= 10 {
return errors.New("stopped after 10 redirects")
}
return checkMediaURL(req.URL.String(), hosts)
},
}
}
Comment on lines +477 to +487

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): mediaClient drops http.Client-level settings from a.client; consider reusing or cloning the existing client.

This creates a new http.Client that only reuses a.client.Transport and adds CheckRedirect, dropping any other configuration on a.client (e.g., CookieJar, Timeout, etc.) for media downloads.

Consider either:

  • Basing mediaClient on a.client and overriding only CheckRedirect, or
  • Clearly documenting that mediaClient is intentionally configured differently.

This avoids surprises if a.client is later updated (e.g., global Timeout or Jar) with the expectation that media downloads share those settings.

Suggested change
func (a *aiAdapter) mediaClient(hosts map[string]struct{}) *http.Client {
return &http.Client{
Transport: a.client.Transport,
CheckRedirect: func(req *http.Request, via []*http.Request) error {
if len(via) >= 10 {
return errors.New("stopped after 10 redirects")
}
return checkMediaURL(req.URL.String(), hosts)
},
}
}
func (a *aiAdapter) mediaClient(hosts map[string]struct{}) *http.Client {
// Clone the base client so media downloads inherit all configuration
// (timeouts, cookie jar, etc.) but enforce our own redirect policy.
clientCopy := *a.client
clientCopy.CheckRedirect = func(req *http.Request, via []*http.Request) error {
if len(via) >= 10 {
return errors.New("stopped after 10 redirects")
}
return checkMediaURL(req.URL.String(), hosts)
}
return &clientCopy
}


// httpStatusError carries the status of a non-200 media response.
type httpStatusError struct{ status int }

func (e *httpStatusError) Error() string { return fmt.Sprintf("unexpected status %d", e.status) }

// statusOf returns the HTTP status of a download error, or 0 if there was no response.
func statusOf(err error) int {
var se *httpStatusError
if errors.As(err, &se) {
return se.status
}
return 0
}

// downloadAttachment GETs the URL with the given client and timeout, reading at most
// limit bytes. It returns the body and the response Content-Type.
func downloadAttachment(ctx context.Context, client *http.Client, url string, timeout time.Duration, limit int) ([]byte, string, error) {
dlCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()

httpReq, err := http.NewRequestWithContext(dlCtx, http.MethodGet, url, nil)
if err != nil {
return nil, "", fmt.Errorf("new_request: %w", err)
}
resp, err := a.client.Do(httpReq)
resp, err := client.Do(httpReq)
if err != nil {
return nil, "", fmt.Errorf("do: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return nil, "", fmt.Errorf("unexpected status %d", resp.StatusCode)
return nil, "", &httpStatusError{status: resp.StatusCode}
}
// +1 so an exactly-at-cap read is distinguishable from an oversize one.
data, err := io.ReadAll(io.LimitReader(resp.Body, int64(limit)+1))
Expand Down
12 changes: 7 additions & 5 deletions pkg/ai/service/ai_adapter_media_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func TestCall_ForwardsAttachmentAsFilePart(t *testing.T) {
adapter := aiService.NewAIAdapter(30, 0, 1)
_, err := adapter.Call(context.Background(), &aiModel.A2ARequest{
OutgoingURL: proc.URL + "/api/v1/a2a/agent-1",
PostbackURL: proc.URL,
ContactID: 1,
ConversationID: 2,
ApiKey: "k",
Expand Down Expand Up @@ -98,6 +99,7 @@ func TestCall_AttachmentDownloadFailure_SendsTextOnly(t *testing.T) {
adapter := aiService.NewAIAdapter(30, 0, 1)
_, err := adapter.Call(context.Background(), &aiModel.A2ARequest{
OutgoingURL: proc.URL + "/api/v1/a2a/agent-1",
PostbackURL: proc.URL,
ContactID: 1,
ConversationID: 2,
ApiKey: "k",
Expand Down Expand Up @@ -159,7 +161,7 @@ func TestCall_TotalAttachmentBudget_DropsExcessAndStillSends(t *testing.T) {

adapter := aiService.NewAIAdapter(30, 0, 1)
if _, err := adapter.Call(context.Background(), &aiModel.A2ARequest{
OutgoingURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "album", Attachments: atts,
OutgoingURL: proc.URL, PostbackURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "album", Attachments: atts,
}); err != nil {
t.Fatalf("a media budget overflow must not error the call: %v", err)
}
Expand Down Expand Up @@ -195,7 +197,7 @@ func TestCall_AttachmentTimeBudget_IsBounded(t *testing.T) {
adapter := aiService.NewAIAdapter(1, 0, 1)
start := time.Now()
if _, err := adapter.Call(context.Background(), &aiModel.A2ARequest{
OutgoingURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi", Attachments: atts,
OutgoingURL: proc.URL, PostbackURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi", Attachments: atts,
}); err != nil {
t.Fatalf("unreachable media must not error the call: %v", err)
}
Expand All @@ -221,7 +223,7 @@ func TestCall_HTMLResponse_IsNotForwardedAsMedia(t *testing.T) {

adapter := aiService.NewAIAdapter(30, 0, 1)
if _, err := adapter.Call(context.Background(), &aiModel.A2ARequest{
OutgoingURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi",
OutgoingURL: proc.URL, PostbackURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi",
Attachments: []aiModel.Attachment{{URL: htmlSrv.URL + "/photo.jpg", ContentType: "image/jpeg", FileType: "image"}},
}); err != nil {
t.Fatalf("Call: %v", err)
Expand Down Expand Up @@ -260,7 +262,7 @@ func TestCall_MimeTypeResolution(t *testing.T) {

adapter := aiService.NewAIAdapter(30, 0, 1)
if _, err := adapter.Call(context.Background(), &aiModel.A2ARequest{
OutgoingURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi",
OutgoingURL: proc.URL, PostbackURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi",
Attachments: []aiModel.Attachment{{URL: srv.URL + tc.urlPath, ContentType: tc.declared, FileType: "image"}},
}); err != nil {
t.Fatalf("Call: %v", err)
Expand Down Expand Up @@ -294,7 +296,7 @@ func TestCall_OversizeAttachment_SendsTextOnly(t *testing.T) {

adapter := aiService.NewAIAdapter(30, 0, 1)
if _, err := adapter.Call(context.Background(), &aiModel.A2ARequest{
OutgoingURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi",
OutgoingURL: proc.URL, PostbackURL: proc.URL, ContactID: 1, ConversationID: 2, Message: "hi",
Attachments: []aiModel.Attachment{{URL: srv.URL + "/big.png", ContentType: "image/png", FileType: "image"}},
}); err != nil {
t.Fatalf("an oversize attachment must not error the call: %v", err)
Expand Down
Loading
Loading