fix(upload): harden upload flow — sanitize filenames, refresh CF cookies, deduplicate handlers
- Sanitize Content-Disposition filenames to prevent header injection (strip control chars, quotes, semicolons) - Add CloudFront cookie refresh middleware so cookies are re-issued when expired - Log errors in groupAttachments instead of silently swallowing them - Move useFileUpload hook to shared/hooks/ per project architecture conventions - Add uploadWithToast helper to deduplicate try/catch/toast pattern across 3 components - Refactor ApiClient.uploadFile to reuse auth headers, 401 handling, and error parsing - Allow empty MIME types client-side (let server sniff and decide) - Constrain Image extension max-width in rich-text-editor to prevent layout overflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f5353c6691
commit
9e23fb76fc
11 changed files with 120 additions and 86 deletions
|
|
@ -9,7 +9,7 @@ import { Card, CardContent } from "@/components/ui/card";
|
|||
import { toast } from "sonner";
|
||||
import { useAuthStore } from "@/features/auth";
|
||||
import { api } from "@/shared/api";
|
||||
import { useFileUpload } from "@/hooks/use-file-upload";
|
||||
import { useFileUpload } from "@/shared/hooks/use-file-upload";
|
||||
|
||||
export function AccountTab() {
|
||||
const user = useAuthStore((s) => s.user);
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ import { Markdown } from "tiptap-markdown";
|
|||
import { Extension } from "@tiptap/core";
|
||||
import { Plugin, PluginKey } from "@tiptap/pm/state";
|
||||
import { cn } from "@/lib/utils";
|
||||
import type { UploadResult } from "@/hooks/use-file-upload";
|
||||
import type { UploadResult } from "@/shared/hooks/use-file-upload";
|
||||
import { createMentionSuggestion } from "./mention-suggestion";
|
||||
import "./rich-text-editor.css";
|
||||
|
||||
|
|
@ -263,7 +263,11 @@ const RichTextEditor = forwardRef<RichTextEditorRef, RichTextEditorProps>(
|
|||
LinkExtension,
|
||||
Typography,
|
||||
MentionExtension,
|
||||
Image.configure({ inline: false, allowBase64: false }),
|
||||
Image.configure({
|
||||
inline: false,
|
||||
allowBase64: false,
|
||||
HTMLAttributes: { style: "max-width: 100%; height: auto;" },
|
||||
}),
|
||||
Markdown.configure({
|
||||
html: false,
|
||||
transformPastedText: true,
|
||||
|
|
|
|||
|
|
@ -4,8 +4,7 @@ import { useRef, useState } from "react";
|
|||
import { ArrowUp, Paperclip } from "lucide-react";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { RichTextEditor, type RichTextEditorRef } from "@/components/common/rich-text-editor";
|
||||
import { useFileUpload } from "@/hooks/use-file-upload";
|
||||
import { toast } from "sonner";
|
||||
import { useFileUpload } from "@/shared/hooks/use-file-upload";
|
||||
|
||||
interface CommentInputProps {
|
||||
issueId: string;
|
||||
|
|
@ -17,17 +16,9 @@ function CommentInput({ issueId, onSubmit }: CommentInputProps) {
|
|||
const fileInputRef = useRef<HTMLInputElement>(null);
|
||||
const [isEmpty, setIsEmpty] = useState(true);
|
||||
const [submitting, setSubmitting] = useState(false);
|
||||
const { upload, uploading } = useFileUpload();
|
||||
const { uploadWithToast, uploading } = useFileUpload();
|
||||
|
||||
const handleUpload = async (file: File) => {
|
||||
try {
|
||||
const result = await upload(file, { issueId });
|
||||
return result;
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : "Upload failed");
|
||||
return null;
|
||||
}
|
||||
};
|
||||
const handleUpload = (file: File) => uploadWithToast(file, { issueId });
|
||||
|
||||
const handleFileSelect = async (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
const file = e.target.files?.[0];
|
||||
|
|
|
|||
|
|
@ -69,7 +69,7 @@ import { useIssueTimeline } from "@/features/issues/hooks/use-issue-timeline";
|
|||
import { useIssueReactions } from "@/features/issues/hooks/use-issue-reactions";
|
||||
import { useIssueSubscribers } from "@/features/issues/hooks/use-issue-subscribers";
|
||||
import { ReactionBar } from "@/components/common/reaction-bar";
|
||||
import { useFileUpload } from "@/hooks/use-file-upload";
|
||||
import { useFileUpload } from "@/shared/hooks/use-file-upload";
|
||||
import { timeAgo } from "@/shared/utils";
|
||||
|
||||
function shortDate(date: string | null): string {
|
||||
|
|
@ -180,7 +180,7 @@ export function IssueDetail({ issueId, onDelete, defaultSidebarOpen = true, layo
|
|||
const prevIssue = currentIndex > 0 ? allIssues[currentIndex - 1] : null;
|
||||
const nextIssue = currentIndex < allIssues.length - 1 ? allIssues[currentIndex + 1] : null;
|
||||
const { getActorName, getActorInitials } = useActorName();
|
||||
const { upload: uploadFile } = useFileUpload();
|
||||
const { uploadWithToast } = useFileUpload();
|
||||
const { defaultLayout, onLayoutChanged } = useDefaultLayout({
|
||||
id: layoutId,
|
||||
});
|
||||
|
|
@ -252,15 +252,8 @@ export function IssueDetail({ issueId, onDelete, defaultSidebarOpen = true, layo
|
|||
);
|
||||
|
||||
const handleDescriptionUpload = useCallback(
|
||||
async (file: File) => {
|
||||
try {
|
||||
return await uploadFile(file, { issueId: id });
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : "Upload failed");
|
||||
return null;
|
||||
}
|
||||
},
|
||||
[uploadFile, id],
|
||||
(file: File) => uploadWithToast(file, { issueId: id }),
|
||||
[uploadWithToast, id],
|
||||
);
|
||||
|
||||
const handleDelete = async () => {
|
||||
|
|
|
|||
|
|
@ -5,8 +5,7 @@ import { ArrowUp, Paperclip } from "lucide-react";
|
|||
import { Button } from "@/components/ui/button";
|
||||
import { RichTextEditor, type RichTextEditorRef } from "@/components/common/rich-text-editor";
|
||||
import { ActorAvatar } from "@/components/common/actor-avatar";
|
||||
import { useFileUpload } from "@/hooks/use-file-upload";
|
||||
import { toast } from "sonner";
|
||||
import { useFileUpload } from "@/shared/hooks/use-file-upload";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Types
|
||||
|
|
@ -37,17 +36,9 @@ function ReplyInput({
|
|||
const fileInputRef = useRef<HTMLInputElement>(null);
|
||||
const [isEmpty, setIsEmpty] = useState(true);
|
||||
const [submitting, setSubmitting] = useState(false);
|
||||
const { upload, uploading } = useFileUpload();
|
||||
const { uploadWithToast, uploading } = useFileUpload();
|
||||
|
||||
const handleUpload = async (file: File) => {
|
||||
try {
|
||||
const result = await upload(file, { issueId });
|
||||
return result;
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : "Upload failed");
|
||||
return null;
|
||||
}
|
||||
};
|
||||
const handleUpload = (file: File) => uploadWithToast(file, { issueId });
|
||||
|
||||
const handleFileSelect = async (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
const file = e.target.files?.[0];
|
||||
|
|
|
|||
|
|
@ -63,6 +63,35 @@ export class ApiClient {
|
|||
this.workspaceId = id;
|
||||
}
|
||||
|
||||
private authHeaders(): Record<string, string> {
|
||||
const headers: Record<string, string> = {};
|
||||
if (this.token) headers["Authorization"] = `Bearer ${this.token}`;
|
||||
if (this.workspaceId) headers["X-Workspace-ID"] = this.workspaceId;
|
||||
return headers;
|
||||
}
|
||||
|
||||
private handleUnauthorized() {
|
||||
if (typeof window !== "undefined") {
|
||||
localStorage.removeItem("multica_token");
|
||||
localStorage.removeItem("multica_workspace_id");
|
||||
this.token = null;
|
||||
this.workspaceId = null;
|
||||
if (window.location.pathname !== "/login") {
|
||||
window.location.href = "/login";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private async parseErrorMessage(res: Response, fallback: string): Promise<string> {
|
||||
try {
|
||||
const data = await res.json() as { error?: string };
|
||||
if (typeof data.error === "string" && data.error) return data.error;
|
||||
} catch {
|
||||
// Ignore non-JSON error bodies.
|
||||
}
|
||||
return fallback;
|
||||
}
|
||||
|
||||
private async fetch<T>(path: string, init?: RequestInit): Promise<T> {
|
||||
const rid = crypto.randomUUID().slice(0, 8);
|
||||
const start = Date.now();
|
||||
|
|
@ -71,14 +100,9 @@ export class ApiClient {
|
|||
const headers: Record<string, string> = {
|
||||
"Content-Type": "application/json",
|
||||
"X-Request-ID": rid,
|
||||
...this.authHeaders(),
|
||||
...((init?.headers as Record<string, string>) ?? {}),
|
||||
};
|
||||
if (this.token) {
|
||||
headers["Authorization"] = `Bearer ${this.token}`;
|
||||
}
|
||||
if (this.workspaceId) {
|
||||
headers["X-Workspace-ID"] = this.workspaceId;
|
||||
}
|
||||
|
||||
this.logger.info(`→ ${method} ${path}`, { rid });
|
||||
|
||||
|
|
@ -88,25 +112,8 @@ export class ApiClient {
|
|||
});
|
||||
|
||||
if (!res.ok) {
|
||||
if (res.status === 401 && typeof window !== "undefined") {
|
||||
localStorage.removeItem("multica_token");
|
||||
localStorage.removeItem("multica_workspace_id");
|
||||
this.token = null;
|
||||
this.workspaceId = null;
|
||||
if (window.location.pathname !== "/login") {
|
||||
window.location.href = "/login";
|
||||
}
|
||||
}
|
||||
|
||||
let message = `API error: ${res.status} ${res.statusText}`;
|
||||
try {
|
||||
const data = await res.json() as { error?: string };
|
||||
if (typeof data.error === "string" && data.error) {
|
||||
message = data.error;
|
||||
}
|
||||
} catch {
|
||||
// Ignore non-JSON error bodies.
|
||||
}
|
||||
if (res.status === 401) this.handleUnauthorized();
|
||||
const message = await this.parseErrorMessage(res, `API error: ${res.status} ${res.statusText}`);
|
||||
this.logger.error(`← ${res.status} ${path}`, { rid, duration: `${Date.now() - start}ms`, error: message });
|
||||
throw new Error(message);
|
||||
}
|
||||
|
|
@ -528,37 +535,24 @@ export class ApiClient {
|
|||
if (opts?.issueId) formData.append("issue_id", opts.issueId);
|
||||
if (opts?.commentId) formData.append("comment_id", opts.commentId);
|
||||
|
||||
const headers: Record<string, string> = {};
|
||||
if (this.token) headers["Authorization"] = `Bearer ${this.token}`;
|
||||
if (this.workspaceId) headers["X-Workspace-ID"] = this.workspaceId;
|
||||
const rid = crypto.randomUUID().slice(0, 8);
|
||||
const start = Date.now();
|
||||
this.logger.info("→ POST /api/upload-file", { rid });
|
||||
|
||||
const res = await fetch(`${this.baseUrl}/api/upload-file`, {
|
||||
method: "POST",
|
||||
headers,
|
||||
headers: this.authHeaders(),
|
||||
body: formData,
|
||||
});
|
||||
|
||||
if (!res.ok) {
|
||||
if (res.status === 401 && typeof window !== "undefined") {
|
||||
localStorage.removeItem("multica_token");
|
||||
localStorage.removeItem("multica_workspace_id");
|
||||
this.token = null;
|
||||
this.workspaceId = null;
|
||||
if (window.location.pathname !== "/login") {
|
||||
window.location.href = "/login";
|
||||
}
|
||||
}
|
||||
|
||||
let message = `Upload failed: ${res.status}`;
|
||||
try {
|
||||
const data = (await res.json()) as { error?: string };
|
||||
if (typeof data.error === "string" && data.error) message = data.error;
|
||||
} catch {
|
||||
// Ignore non-JSON error bodies.
|
||||
}
|
||||
if (res.status === 401) this.handleUnauthorized();
|
||||
const message = await this.parseErrorMessage(res, `Upload failed: ${res.status}`);
|
||||
this.logger.error(`← ${res.status} /api/upload-file`, { rid, duration: `${Date.now() - start}ms`, error: message });
|
||||
throw new Error(message);
|
||||
}
|
||||
|
||||
this.logger.info(`← ${res.status} /api/upload-file`, { rid, duration: `${Date.now() - start}ms` });
|
||||
return res.json() as Promise<Attachment>;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
"use client";
|
||||
|
||||
import { useState, useCallback } from "react";
|
||||
import { toast } from "sonner";
|
||||
import { api } from "@/shared/api";
|
||||
import type { Attachment } from "@/shared/types";
|
||||
|
||||
|
|
@ -24,6 +25,8 @@ const ALLOWED_TYPES = new Set([
|
|||
]);
|
||||
|
||||
function isAllowedType(type: string): boolean {
|
||||
// Empty MIME type (browser couldn't determine) — let the server sniff and decide.
|
||||
if (!type) return true;
|
||||
const mediaType = type.split(";")[0] ?? "";
|
||||
return ALLOWED_TYPES.has(mediaType.trim().toLowerCase());
|
||||
}
|
||||
|
|
@ -64,5 +67,17 @@ export function useFileUpload() {
|
|||
[],
|
||||
);
|
||||
|
||||
return { upload, uploading };
|
||||
const uploadWithToast = useCallback(
|
||||
async (file: File, ctx?: UploadContext): Promise<UploadResult | null> => {
|
||||
try {
|
||||
return await upload(file, ctx);
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : "Upload failed");
|
||||
return null;
|
||||
}
|
||||
},
|
||||
[upload],
|
||||
);
|
||||
|
||||
return { upload, uploadWithToast, uploading };
|
||||
}
|
||||
|
|
@ -110,6 +110,7 @@ func NewRouter(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus) chi.Route
|
|||
// Protected API routes
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(middleware.Auth(queries))
|
||||
r.Use(middleware.RefreshCloudFrontCookies(cfSigner))
|
||||
|
||||
// --- User-scoped routes (no workspace context required) ---
|
||||
r.Get("/api/me", h.GetMe)
|
||||
|
|
|
|||
|
|
@ -96,6 +96,7 @@ func (h *Handler) groupAttachments(r *http.Request, commentIDs []pgtype.UUID) ma
|
|||
}
|
||||
attachments, err := h.Queries.ListAttachmentsByCommentIDs(r.Context(), commentIDs)
|
||||
if err != nil {
|
||||
slog.Error("failed to load attachments for comments", "error", err)
|
||||
return nil
|
||||
}
|
||||
grouped := make(map[string][]AttachmentResponse, len(commentIDs))
|
||||
|
|
|
|||
28
server/internal/middleware/cloudfront.go
Normal file
28
server/internal/middleware/cloudfront.go
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
package middleware
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
"github.com/multica-ai/multica/server/internal/auth"
|
||||
)
|
||||
|
||||
// RefreshCloudFrontCookies is middleware that refreshes CloudFront signed cookies
|
||||
// on authenticated requests when the cookie is missing (expired or first request
|
||||
// after login). This prevents 403s from the CDN when cookies expire before the
|
||||
// user's session does.
|
||||
func RefreshCloudFrontCookies(signer *auth.CloudFrontSigner) func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
if signer == nil {
|
||||
return next
|
||||
}
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if _, err := r.Cookie("CloudFront-Policy"); err != nil {
|
||||
for _, cookie := range signer.SignedCookies(time.Now().Add(72 * time.Hour)) {
|
||||
http.SetCookie(w, cookie)
|
||||
}
|
||||
}
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -68,13 +68,29 @@ func NewS3StorageFromEnv() *S3Storage {
|
|||
}
|
||||
}
|
||||
|
||||
// sanitizeFilename removes characters that could cause header injection in Content-Disposition.
|
||||
func sanitizeFilename(name string) string {
|
||||
var b strings.Builder
|
||||
b.Grow(len(name))
|
||||
for _, r := range name {
|
||||
// Strip control chars, newlines, null bytes, quotes, semicolons, backslashes
|
||||
if r < 0x20 || r == 0x7f || r == '"' || r == ';' || r == '\\' || r == '\x00' {
|
||||
b.WriteRune('_')
|
||||
} else {
|
||||
b.WriteRune(r)
|
||||
}
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
func (s *S3Storage) Upload(ctx context.Context, key string, data []byte, contentType string, filename string) (string, error) {
|
||||
safe := sanitizeFilename(filename)
|
||||
_, err := s.client.PutObject(ctx, &s3.PutObjectInput{
|
||||
Bucket: aws.String(s.bucket),
|
||||
Key: aws.String(key),
|
||||
Body: bytes.NewReader(data),
|
||||
ContentType: aws.String(contentType),
|
||||
ContentDisposition: aws.String(fmt.Sprintf(`inline; filename="%s"`, strings.ReplaceAll(filename, `"`, "'"))),
|
||||
ContentDisposition: aws.String(fmt.Sprintf(`inline; filename="%s"`, safe)),
|
||||
CacheControl: aws.String("max-age=432000,public"),
|
||||
StorageClass: types.StorageClassIntelligentTiering,
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue