diff --git a/file-service/main.go b/file-service/main.go index 00f020b..85d1dc2 100644 --- a/file-service/main.go +++ b/file-service/main.go @@ -7,11 +7,12 @@ import ( "io" "mime/multipart" "net/http" - "net/url" "os" "os/signal" "path/filepath" + "regexp" "slices" + "strconv" "syscall" "time" @@ -61,6 +62,7 @@ func newServer() (*server, error) { s3Client, err := minio.New(endpoint, &minio.Options{ Creds: credentials.NewStaticV4(accessKey, secretKey, ""), Secure: false, + Region: "us-east-1", BucketLookup: minio.BucketLookupPath, }) if err != nil { @@ -101,9 +103,14 @@ func newServer() (*server, error) { s.echo.GET("/api/v1/health", s.handleHealth) s.echo.HEAD("/api/v1/health", s.handleHealth) s.echo.POST("/api/v1/file/upload", s.handleUploadImage) + s.echo.GET("/file-uploads/:object", s.handleDownloadFile) + s.echo.HEAD("/file-uploads/:object", s.handleDownloadFile) return s, nil } +// objectNameRe matches UUID object keys written by processUploadedFile. +var objectNameRe = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.(png|pdf|webm|zip)$`) + type publicFile struct { FileName string `json:"fileName"` PublicURL string `json:"publicURL"` @@ -183,18 +190,45 @@ func (s *server) processUploadedFile(ctx context.Context, fh *multipart.FileHead return publicFile{}, fmt.Errorf("could not put object: %w", err) } - publicURL, err := s.s3Client.PresignedGetObject(ctx, BUCKET_NAME, objectName, time.Minute*10, url.Values{}) - if err != nil { - return publicFile{}, fmt.Errorf("could not generate public URL: %w", err) - } - return publicFile{ Extension: fileExtension, FileName: fh.Filename, - PublicURL: publicURL.EscapedPath() + "?" + publicURL.RawQuery, + // Served by this process (see handleDownloadFile) so browsers never + // present a SigV4 Host signed for the in-cluster rustfs:9000 endpoint. + PublicURL: "/file-uploads/" + objectName, }, nil } +func (s *server) handleDownloadFile(c *echo.Context) error { + objectName := c.Param("object") + if !objectNameRe.MatchString(objectName) { + return echo.NewHTTPError(http.StatusBadRequest, "invalid object name") + } + + obj, err := s.s3Client.GetObject(c.Request().Context(), BUCKET_NAME, objectName, minio.GetObjectOptions{}) + if err != nil { + return fmt.Errorf("could not get object: %w", err) + } + defer obj.Close() + + stat, err := obj.Stat() + if err != nil { + errResp := minio.ToErrorResponse(err) + if errResp.Code == "NoSuchKey" || errResp.StatusCode == http.StatusNotFound { + return echo.NewHTTPError(http.StatusNotFound, "not found") + } + return fmt.Errorf("could not stat object: %w", err) + } + + c.Response().Header().Set("Content-Type", stat.ContentType) + c.Response().Header().Set("Content-Length", strconv.FormatInt(stat.Size, 10)) + c.Response().Header().Set("Cache-Control", "public, max-age=3600") + if c.Request().Method == http.MethodHead { + return c.NoContent(http.StatusOK) + } + return c.Stream(http.StatusOK, stat.ContentType, obj) +} + func (s *server) handleHealth(c *echo.Context) error { return c.String(http.StatusOK, "OK") } diff --git a/file-service/main_test.go b/file-service/main_test.go index 9056e15..088662d 100644 --- a/file-service/main_test.go +++ b/file-service/main_test.go @@ -3,7 +3,6 @@ package main import ( "bytes" "encoding/json" - "io" "mime/multipart" "net/http" "net/http/httptest" @@ -88,22 +87,21 @@ func TestUploadPNGAgainstRustFS(t *testing.T) { if !strings.HasPrefix(files[0].PublicURL, "/file-uploads/") { t.Fatalf("public URL is not path-style: %s", files[0].PublicURL) } - - objectURL := "http://" + os.Getenv("S3_ENDPOINT") + files[0].PublicURL - resp, err := http.Get(objectURL) - if err != nil { - t.Fatalf("could not GET stored object: %v", err) + if strings.Contains(files[0].PublicURL, "?") { + t.Fatalf("public URL should not be presigned: %s", files[0].PublicURL) } - defer resp.Body.Close() - got, err := io.ReadAll(resp.Body) - if err != nil { - t.Fatal(err) + + getReq := httptest.NewRequest(http.MethodGet, files[0].PublicURL, nil) + getRec := httptest.NewRecorder() + s.echo.ServeHTTP(getRec, getReq) + if getRec.Code != http.StatusOK { + t.Fatalf("download status = %d, body = %s", getRec.Code, getRec.Body.String()) } - if resp.StatusCode != http.StatusOK { - t.Fatalf("GET object status = %d, body = %s", resp.StatusCode, got) + if ct := getRec.Header().Get("Content-Type"); ct != "image/png" { + t.Fatalf("content-type = %q", ct) } - if !bytes.Equal(got, png1x1) { - t.Fatalf("stored object did not round-trip, got %d bytes", len(got)) + if !bytes.Equal(getRec.Body.Bytes(), png1x1) { + t.Fatalf("downloaded object did not round-trip, got %d bytes", getRec.Body.Len()) } } @@ -138,3 +136,32 @@ func TestRejectsDisallowedMimeType(t *testing.T) { t.Fatalf("expected rejected upload, got %d", rec.Code) } } + +func TestObjectNameRe(t *testing.T) { + if !objectNameRe.MatchString("8cc1c45b-9f14-4d92-894d-4f4a62fc6691.png") { + t.Fatal("expected uuid png to match") + } + for _, name := range []string{"../etc/passwd", "foo.png", "8cc1c45b-9f14-4d92-894d-4f4a62fc6691.txt", "8cc1c45b-9f14-4d92-894d-4f4a62fc6691.png/../x"} { + if objectNameRe.MatchString(name) { + t.Fatalf("did not expect %q to match", name) + } + } +} + +func TestDownloadRejectsInvalidObjectName(t *testing.T) { + if os.Getenv("S3_ENDPOINT") == "" { + t.Skip("S3_ENDPOINT is not set") + } + + s, err := newServer() + if err != nil { + t.Fatalf("could not init server: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/file-uploads/not-a-uuid.png", nil) + rec := httptest.NewRecorder() + s.echo.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } +} diff --git a/frontend/Caddyfile b/frontend/Caddyfile index 9d28265..2a6c26f 100644 --- a/frontend/Caddyfile +++ b/frontend/Caddyfile @@ -11,7 +11,6 @@ header *.tff Cache-Control max-age=86400 log reverse_proxy /service/control/* control:8080 -reverse_proxy /file-uploads/* rustfs:9000 { - header_up Host rustfs:9000 +reverse_proxy /file-uploads/* file:8080 { method GET } diff --git a/k8/frontend-ingress.yaml b/k8/frontend-ingress.yaml index 2c82922..0defcff 100644 --- a/k8/frontend-ingress.yaml +++ b/k8/frontend-ingress.yaml @@ -12,6 +12,13 @@ spec: - host: try.playwright.tech http: paths: + - path: /file-uploads + pathType: Prefix + backend: + service: + name: file + port: + number: 8080 - path: / pathType: Prefix backend: