Use stable url for public file

This will allow the CDN infrastructure to cache it properly

Signed-off-by: Bryan Frimin <bryan@probo.com>

e2e/trust/trust_center_logo_test.go

@@ -15,6 +15,8 @@
 package trust_test
 
 import (
+	"io"
+	"net/http"
 	"strings"
 	"testing"
 
@@ -155,10 +157,64 @@ func TestTrustCenter_LogoFileDownloadURL(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, trustResult.CurrentTrustCenter.Logo)
 	assert.Equal(t, uploadResult.UpdateTrustCenterBrand.TrustCenter.Logo.ID, trustResult.CurrentTrustCenter.Logo.ID)
+
+	downloadURL := trustResult.CurrentTrustCenter.Logo.DownloadURL
 	assert.True(
 		t,
-		strings.Contains(trustResult.CurrentTrustCenter.Logo.DownloadURL, "/api/files/v1/public/"),
+		strings.Contains(downloadURL, "/api/files/v1/public/"),
 		"downloadUrl must route through the public files API, got %q",
-		trustResult.CurrentTrustCenter.Logo.DownloadURL,
+		downloadURL,
 	)
+
+	// The public endpoint streams the file bytes directly (no presigned
+	// redirect) with cache headers, so the stable URL is CDN/browser cacheable.
+	resp, err := http.Get(downloadURL)
+	require.NoError(t, err)
+	defer func() { _ = resp.Body.Close() }()
+
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	assert.Equal(t, pngContent, body, "public endpoint must stream the original file bytes")
+
+	cacheControl := resp.Header.Get("Cache-Control")
+	assert.Contains(t, cacheControl, "public")
+	assert.Contains(t, cacheControl, "max-age=31536000")
+	assert.Contains(t, cacheControl, "immutable")
+
+	etag := resp.Header.Get("ETag")
+	require.NotEmpty(t, etag, "public file response must carry an ETag for revalidation")
+
+	// Untrusted, unauthenticated content served from the app origin must be
+	// hardened against MIME sniffing and script execution (e.g. SVG uploads).
+	assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"))
+	assert.Contains(t, resp.Header.Get("Content-Security-Policy"), "sandbox")
+
+	// A matching If-None-Match must revalidate to 304 without transferring
+	// the body again.
+	revalidateReq, err := http.NewRequest(http.MethodGet, downloadURL, nil)
+	require.NoError(t, err)
+	revalidateReq.Header.Set("If-None-Match", etag)
+
+	revalidateResp, err := http.DefaultClient.Do(revalidateReq)
+	require.NoError(t, err)
+	defer func() { _ = revalidateResp.Body.Close() }()
+
+	assert.Equal(t, http.StatusNotModified, revalidateResp.StatusCode)
+
+	// A matching If-Modified-Since (using the returned Last-Modified) must also
+	// revalidate to 304.
+	lastModified := resp.Header.Get("Last-Modified")
+	require.NotEmpty(t, lastModified, "public file response must carry Last-Modified")
+
+	sinceReq, err := http.NewRequest(http.MethodGet, downloadURL, nil)
+	require.NoError(t, err)
+	sinceReq.Header.Set("If-Modified-Since", lastModified)
+
+	sinceResp, err := http.DefaultClient.Do(sinceReq)
+	require.NoError(t, err)
+	defer func() { _ = sinceResp.Body.Close() }()
+
+	assert.Equal(t, http.StatusNotModified, sinceResp.StatusCode)
 }

pkg/filemanager/s3.go

@@ -17,16 +17,39 @@ package filemanager
 import (
 	"context"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
+	smithyhttp "github.com/aws/smithy-go/transport/http"
 	"go.probo.inc/probo/pkg/coredata"
 )
 
+// FileObject carries a streamed S3 object body plus the metadata needed to set
+// HTTP response headers. NotModified is set when the caller's conditional
+// request matched the current object, in which case Body is nil.
+type FileObject struct {
+	Body          io.ReadCloser
+	ContentType   string
+	ContentLength int64
+	ETag          string
+	LastModified  time.Time
+	NotModified   bool
+}
+
+// FileConditions carries HTTP conditional-request values forwarded to S3 so it
+// can answer with 304 Not Modified without transferring the body. Zero values
+// are omitted.
+type FileConditions struct {
+	IfNoneMatch     string
+	IfModifiedSince time.Time
+}
+
 func (s *Service) GetFileBase64(
 	ctx context.Context,
 	file *coredata.File,
@@ -77,6 +100,53 @@ func (s *Service) GetFileBytes(
 	return data, nil
 }
 
+// OpenFile streams an object from S3 without buffering it in memory. Conditional
+// request values in conds are forwarded to S3; a 304 Not Modified response is
+// surfaced as a FileObject with NotModified set (and a nil Body). The caller
+// owns closing Body.
+func (s *Service) OpenFile(
+	ctx context.Context,
+	file *coredata.File,
+	conds FileConditions,
+) (*FileObject, error) {
+	input := &s3.GetObjectInput{
+		Bucket: new(file.BucketName),
+		Key:    new(file.FileKey),
+	}
+	if conds.IfNoneMatch != "" {
+		input.IfNoneMatch = &conds.IfNoneMatch
+	}
+	if !conds.IfModifiedSince.IsZero() {
+		input.IfModifiedSince = &conds.IfModifiedSince
+	}
+
+	result, err := s.s3Client.GetObject(ctx, input)
+	if err != nil {
+		if respErr, ok := errors.AsType[*smithyhttp.ResponseError](err); ok {
+			if respErr.HTTPStatusCode() == http.StatusNotModified {
+				return &FileObject{NotModified: true}, nil
+			}
+		}
+
+		return nil, fmt.Errorf("cannot get file from S3: %w", err)
+	}
+
+	obj := &FileObject{
+		Body:          result.Body,
+		ContentType:   file.MimeType,
+		ContentLength: file.FileSize,
+		LastModified:  file.UpdatedAt,
+	}
+	if result.ETag != nil {
+		obj.ETag = *result.ETag
+	}
+	if result.LastModified != nil {
+		obj.LastModified = *result.LastModified
+	}
+
+	return obj, nil
+}
+
 func (s *Service) PutFile(
 	ctx context.Context,
 	file *coredata.File,

pkg/filemanager/s3_test.go

@@ -0,0 +1,155 @@
+// Copyright (c) 2026 Probo Inc <hello@probo.com>.
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
+// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+package filemanager_test
+
+import (
+	"context"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/credentials"
+	awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.probo.inc/probo/pkg/coredata"
+	"go.probo.inc/probo/pkg/filemanager"
+)
+
+func newTestS3Service(t *testing.T, handler http.HandlerFunc) *filemanager.Service {
+	t.Helper()
+
+	srv := httptest.NewServer(handler)
+	t.Cleanup(srv.Close)
+
+	s3Client := awss3.NewFromConfig(
+		aws.Config{
+			Region:      "us-east-1",
+			Credentials: credentials.NewStaticCredentialsProvider("access-key", "secret-key", ""),
+		},
+		func(o *awss3.Options) {
+			o.BaseEndpoint = aws.String(srv.URL)
+			o.UsePathStyle = true
+		},
+	)
+
+	return filemanager.NewService(nil, nil, s3Client)
+}
+
+func TestOpenFile_StreamsBody(t *testing.T) {
+	t.Parallel()
+
+	const (
+		etag    = `"abc123"`
+		content = "hello world"
+	)
+
+	svc := newTestS3Service(
+		t,
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("ETag", etag)
+			w.Header().Set("Content-Type", "application/octet-stream")
+			_, _ = io.WriteString(w, content)
+		},
+	)
+
+	file := &coredata.File{
+		BucketName: "uploads",
+		FileKey:    "tenant/file",
+		MimeType:   "text/plain",
+		FileSize:   int64(len(content)),
+	}
+
+	obj, err := svc.OpenFile(context.Background(), file, filemanager.FileConditions{})
+	require.NoError(t, err)
+	require.NotNil(t, obj)
+	require.False(t, obj.NotModified)
+
+	defer func() { _ = obj.Body.Close() }()
+
+	assert.Equal(t, etag, obj.ETag)
+	assert.Equal(t, "text/plain", obj.ContentType)
+	assert.Equal(t, int64(len(content)), obj.ContentLength)
+
+	body, err := io.ReadAll(obj.Body)
+	require.NoError(t, err)
+	assert.Equal(t, content, string(body))
+}
+
+func TestOpenFile_NotModifiedByETag(t *testing.T) {
+	t.Parallel()
+
+	const etag = `"abc123"`
+
+	svc := newTestS3Service(
+		t,
+		func(w http.ResponseWriter, r *http.Request) {
+			if r.Header.Get("If-None-Match") == etag {
+				w.WriteHeader(http.StatusNotModified)
+				return
+			}
+
+			w.Header().Set("ETag", etag)
+			_, _ = io.WriteString(w, "content")
+		},
+	)
+
+	file := &coredata.File{
+		BucketName: "uploads",
+		FileKey:    "tenant/file",
+		MimeType:   "text/plain",
+	}
+
+	obj, err := svc.OpenFile(context.Background(), file, filemanager.FileConditions{IfNoneMatch: etag})
+	require.NoError(t, err)
+	require.NotNil(t, obj)
+	assert.True(t, obj.NotModified)
+	assert.Nil(t, obj.Body)
+}
+
+func TestOpenFile_NotModifiedByModifiedSince(t *testing.T) {
+	t.Parallel()
+
+	svc := newTestS3Service(
+		t,
+		func(w http.ResponseWriter, r *http.Request) {
+			if r.Header.Get("If-Modified-Since") != "" {
+				w.WriteHeader(http.StatusNotModified)
+				return
+			}
+
+			_, _ = io.WriteString(w, "content")
+		},
+	)
+
+	file := &coredata.File{
+		BucketName: "uploads",
+		FileKey:    "tenant/file",
+		MimeType:   "text/plain",
+	}
+
+	obj, err := svc.OpenFile(
+		context.Background(),
+		file,
+		filemanager.FileConditions{IfModifiedSince: time.Now()},
+	)
+	require.NoError(t, err)
+	require.NotNil(t, obj)
+	assert.True(t, obj.NotModified)
+	assert.Nil(t, obj.Body)
+}

pkg/server/api/files/v1/handler.go

@@ -17,7 +17,9 @@ package files_v1
 import (
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
+	"strconv"
 	"time"
 
 	"github.com/go-chi/chi/v5"
@@ -121,11 +123,18 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	presignedURL, err := h.fileSvc.GeneratePresignedURL(r.Context(), file, presignedURLExpiry)
+	conds := filemanager.FileConditions{IfNoneMatch: r.Header.Get("If-None-Match")}
+	if ifModifiedSince := r.Header.Get("If-Modified-Since"); ifModifiedSince != "" {
+		if t, parseErr := http.ParseTime(ifModifiedSince); parseErr == nil {
+			conds.IfModifiedSince = t
+		}
+	}
+
+	obj, err := h.fileSvc.OpenFile(r.Context(), file, conds)
 	if err != nil {
 		h.logger.ErrorCtx(
 			r.Context(),
-			"cannot get public file URL",
+			"cannot open public file",
 			log.Error(err),
 			log.String("file_id", fileIDStr),
 		)
@@ -134,7 +143,38 @@ func (h *Handler) handleGetPublicFile(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	http.Redirect(w, r, presignedURL, http.StatusTemporaryRedirect)
+	w.Header().Set("Cache-Control", "public, max-age=31536000, immutable")
+
+	if obj.ETag != "" {
+		w.Header().Set("ETag", obj.ETag)
+	}
+
+	if obj.NotModified {
+		w.WriteHeader(http.StatusNotModified)
+		return
+	}
+
+	defer func() { _ = obj.Body.Close() }()
+
+	w.Header().Set("Content-Type", file.MimeType)
+	w.Header().Set("Content-Length", strconv.FormatInt(file.FileSize, 10))
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; sandbox")
+
+	if !obj.LastModified.IsZero() {
+		w.Header().Set("Last-Modified", obj.LastModified.UTC().Format(http.TimeFormat))
+	}
+
+	if _, err := io.Copy(w, obj.Body); err != nil {
+		h.logger.ErrorCtx(
+			r.Context(),
+			"cannot stream public file",
+			log.Error(err),
+			log.String("file_id", fileIDStr),
+		)
+
+		return
+	}
 }
 
 func (h *Handler) handleGetFile(w http.ResponseWriter, r *http.Request) {