From 4dfd862f11da853e606be69ee919d4d533b20856 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 16 Mar 2026 10:05:14 -0500 Subject: [PATCH] Address Greptile company logo feedback --- docs/api/companies.md | 2 +- server/src/__tests__/assets.test.ts | 131 ++++++++++++++++------------ server/src/routes/assets.ts | 128 +++++++++++++++++++++++++-- ui/src/api/assets.ts | 12 ++- ui/src/pages/CompanySettings.tsx | 6 +- 5 files changed, 208 insertions(+), 71 deletions(-) diff --git a/docs/api/companies.md b/docs/api/companies.md index e48d0176..ca9fa2f1 100644 --- a/docs/api/companies.md +++ b/docs/api/companies.md @@ -48,7 +48,7 @@ PATCH /api/companies/{companyId} Upload an image for a company icon and store it as that company’s logo. ``` -POST /api/companies/{companyId}/assets/images +POST /api/companies/{companyId}/logo Content-Type: multipart/form-data ``` diff --git a/server/src/__tests__/assets.test.ts b/server/src/__tests__/assets.test.ts index 745b9a76..b6f740b7 100644 --- a/server/src/__tests__/assets.test.ts +++ b/server/src/__tests__/assets.test.ts @@ -92,7 +92,62 @@ describe("POST /api/companies/:companyId/assets/images", () => { const res = await request(app) .post("/api/companies/company-1/assets/images") - .field("namespace", "companies") + .field("namespace", "goals") + .attach("file", Buffer.from("png"), "logo.png"); + + expect(res.status).toBe(201); + expect(res.body.contentPath).toBe("/api/assets/asset-1/content"); + expect(createAssetMock).toHaveBeenCalledTimes(1); + expect(png.putFile).toHaveBeenCalledWith({ + companyId: "company-1", + namespace: "assets/goals", + originalFilename: "logo.png", + contentType: "image/png", + body: expect.any(Buffer), + }); + }); + + it("allows supported non-image attachments outside the company logo flow", async () => { + const text = createStorageService("text/plain"); + const app = createApp(text); + + createAssetMock.mockResolvedValue({ + ...createAsset(), + contentType: "text/plain", + originalFilename: "note.txt", + }); + + const res = await request(app) + .post("/api/companies/company-1/assets/images") + .field("namespace", "issues/drafts") + .attach("file", Buffer.from("hello"), { filename: "note.txt", contentType: "text/plain" }); + + expect(res.status).toBe(201); + expect(text.putFile).toHaveBeenCalledWith({ + companyId: "company-1", + namespace: "assets/issues/drafts", + originalFilename: "note.txt", + contentType: "text/plain", + body: expect.any(Buffer), + }); + }); +}); + +describe("POST /api/companies/:companyId/logo", () => { + afterEach(() => { + createAssetMock.mockReset(); + getAssetByIdMock.mockReset(); + logActivityMock.mockReset(); + }); + + it("accepts PNG logo uploads and returns an asset path", async () => { + const png = createStorageService("image/png"); + const app = createApp(png); + + createAssetMock.mockResolvedValue(createAsset()); + + const res = await request(app) + .post("/api/companies/company-1/logo") .attach("file", Buffer.from("png"), "logo.png"); expect(res.status).toBe(201); @@ -107,7 +162,7 @@ describe("POST /api/companies/:companyId/assets/images", () => { }); }); - it("sanitizes SVG image uploads before storing them", async () => { + it("sanitizes SVG logo uploads before storing them", async () => { const svg = createStorageService("image/svg+xml"); const app = createApp(svg); @@ -118,8 +173,7 @@ describe("POST /api/companies/:companyId/assets/images", () => { }); const res = await request(app) - .post("/api/companies/company-1/assets/images") - .field("namespace", "companies") + .post("/api/companies/company-1/logo") .attach( "file", Buffer.from( @@ -141,47 +195,38 @@ describe("POST /api/companies/:companyId/assets/images", () => { expect(body).not.toContain("https://evil.example/"); }); - it("rejects files larger than 100 KB", async () => { + it("allows a logo exactly 100 KB in size", async () => { + const png = createStorageService("image/png"); + const app = createApp(png); + createAssetMock.mockResolvedValue(createAsset()); + + const file = Buffer.alloc(100 * 1024, "a"); + const res = await request(app) + .post("/api/companies/company-1/logo") + .attach("file", file, "exact-limit.png"); + + expect(res.status).toBe(201); + }); + + it("rejects logo files larger than 100 KB", async () => { const app = createApp(createStorageService()); createAssetMock.mockResolvedValue(createAsset()); const file = Buffer.alloc(100 * 1024 + 1, "a"); const res = await request(app) - .post("/api/companies/company-1/assets/images") - .field("namespace", "companies") + .post("/api/companies/company-1/logo") .attach("file", file, "too-large.png"); expect(res.status).toBe(422); expect(res.body.error).toBe("Image exceeds 102400 bytes"); }); - it("allows larger non-logo images within the general asset limit", async () => { - const png = createStorageService("image/png"); - const app = createApp(png); - - createAssetMock.mockResolvedValue({ - ...createAsset(), - contentType: "image/png", - originalFilename: "goal.png", - }); - - const file = Buffer.alloc(150 * 1024, "a"); - const res = await request(app) - .post("/api/companies/company-1/assets/images") - .field("namespace", "goals") - .attach("file", file, "goal.png"); - - expect(res.status).toBe(201); - expect(createAssetMock).toHaveBeenCalledTimes(1); - }); - it("rejects unsupported image types", async () => { const app = createApp(createStorageService("text/plain")); createAssetMock.mockResolvedValue(createAsset()); const res = await request(app) - .post("/api/companies/company-1/assets/images") - .field("namespace", "companies") + .post("/api/companies/company-1/logo") .attach("file", Buffer.from("not an image"), "note.txt"); expect(res.status).toBe(422); @@ -189,38 +234,12 @@ describe("POST /api/companies/:companyId/assets/images", () => { expect(createAssetMock).not.toHaveBeenCalled(); }); - it("allows supported non-image attachments outside the company logo namespace", async () => { - const text = createStorageService("text/plain"); - const app = createApp(text); - - createAssetMock.mockResolvedValue({ - ...createAsset(), - contentType: "text/plain", - originalFilename: "note.txt", - }); - - const res = await request(app) - .post("/api/companies/company-1/assets/images") - .field("namespace", "issues/drafts") - .attach("file", Buffer.from("hello"), "note.txt"); - - expect(res.status).toBe(201); - expect(text.putFile).toHaveBeenCalledWith({ - companyId: "company-1", - namespace: "assets/issues/drafts", - originalFilename: "note.txt", - contentType: "text/plain", - body: expect.any(Buffer), - }); - }); - it("rejects SVG image uploads that cannot be sanitized", async () => { const app = createApp(createStorageService("image/svg+xml")); createAssetMock.mockResolvedValue(createAsset()); const res = await request(app) - .post("/api/companies/company-1/assets/images") - .field("namespace", "companies") + .post("/api/companies/company-1/logo") .attach("file", Buffer.from("not actually svg"), "logo.svg"); expect(res.status).toBe(422); diff --git a/server/src/routes/assets.ts b/server/src/routes/assets.ts index 0ec6ffb1..ce2793a5 100644 --- a/server/src/routes/assets.ts +++ b/server/src/routes/assets.ts @@ -10,6 +10,14 @@ import { isAllowedContentType, MAX_ATTACHMENT_BYTES } from "../attachment-types. import { assertCompanyAccess, getActorInfo } from "./authz.js"; const MAX_COMPANY_LOGO_BYTES = 100 * 1024; const SVG_CONTENT_TYPE = "image/svg+xml"; +const ALLOWED_COMPANY_LOGO_CONTENT_TYPES = new Set([ + "image/png", + "image/jpeg", + "image/jpg", + "image/webp", + "image/gif", + SVG_CONTENT_TYPE, +]); function sanitizeSvgBuffer(input: Buffer): Buffer | null { const raw = input.toString("utf8").trim(); @@ -78,12 +86,20 @@ function sanitizeSvgBuffer(input: Buffer): Buffer | null { export function assetRoutes(db: Db, storage: StorageService) { const router = Router(); const svc = assetService(db); - const upload = multer({ + const assetUpload = multer({ storage: multer.memoryStorage(), limits: { fileSize: MAX_ATTACHMENT_BYTES, files: 1 }, }); + const companyLogoUpload = multer({ + storage: multer.memoryStorage(), + limits: { fileSize: MAX_COMPANY_LOGO_BYTES + 1, files: 1 }, + }); - async function runSingleFileUpload(req: Request, res: Response) { + async function runSingleFileUpload( + upload: ReturnType, + req: Request, + res: Response, + ) { await new Promise((resolve, reject) => { upload.single("file")(req, res, (err: unknown) => { if (err) reject(err); @@ -97,7 +113,7 @@ export function assetRoutes(db: Db, storage: StorageService) { assertCompanyAccess(req, companyId); try { - await runSingleFileUpload(req, res); + await runSingleFileUpload(assetUpload, req, res); } catch (err) { if (err instanceof multer.MulterError) { if (err.code === "LIMIT_FILE_SIZE") { @@ -123,12 +139,7 @@ export function assetRoutes(db: Db, storage: StorageService) { } const namespaceSuffix = parsedMeta.data.namespace ?? "general"; - const isCompanyLogoNamespace = namespaceSuffix === "companies" || namespaceSuffix.startsWith("companies/"); const contentType = (file.mimetype || "").toLowerCase(); - if (isCompanyLogoNamespace && !contentType.startsWith("image/")) { - res.status(422).json({ error: `Unsupported image type: ${contentType || "unknown"}` }); - return; - } if (contentType !== SVG_CONTENT_TYPE && !isAllowedContentType(contentType)) { res.status(422).json({ error: `Unsupported file type: ${contentType || "unknown"}` }); return; @@ -146,7 +157,7 @@ export function assetRoutes(db: Db, storage: StorageService) { res.status(422).json({ error: "Image is empty" }); return; } - if (isCompanyLogoNamespace && fileBody.length > MAX_COMPANY_LOGO_BYTES) { + if (fileBody.length > MAX_COMPANY_LOGO_BYTES) { res.status(422).json({ error: `Image exceeds ${MAX_COMPANY_LOGO_BYTES} bytes` }); return; } @@ -204,6 +215,105 @@ export function assetRoutes(db: Db, storage: StorageService) { }); }); + router.post("/companies/:companyId/logo", async (req, res) => { + const companyId = req.params.companyId as string; + assertCompanyAccess(req, companyId); + + try { + await runSingleFileUpload(companyLogoUpload, req, res); + } catch (err) { + if (err instanceof multer.MulterError) { + if (err.code === "LIMIT_FILE_SIZE") { + res.status(422).json({ error: `Image exceeds ${MAX_COMPANY_LOGO_BYTES} bytes` }); + return; + } + res.status(400).json({ error: err.message }); + return; + } + throw err; + } + + const file = (req as Request & { file?: { mimetype: string; buffer: Buffer; originalname: string } }).file; + if (!file) { + res.status(400).json({ error: "Missing file field 'file'" }); + return; + } + + const contentType = (file.mimetype || "").toLowerCase(); + if (!ALLOWED_COMPANY_LOGO_CONTENT_TYPES.has(contentType)) { + res.status(422).json({ error: `Unsupported image type: ${contentType || "unknown"}` }); + return; + } + + let fileBody = file.buffer; + if (contentType === SVG_CONTENT_TYPE) { + const sanitized = sanitizeSvgBuffer(file.buffer); + if (!sanitized || sanitized.length <= 0) { + res.status(422).json({ error: "SVG could not be sanitized" }); + return; + } + fileBody = sanitized; + } + + if (fileBody.length <= 0) { + res.status(422).json({ error: "Image is empty" }); + return; + } + + const actor = getActorInfo(req); + const stored = await storage.putFile({ + companyId, + namespace: "assets/companies", + originalFilename: file.originalname || null, + contentType, + body: fileBody, + }); + + const asset = await svc.create(companyId, { + provider: stored.provider, + objectKey: stored.objectKey, + contentType: stored.contentType, + byteSize: stored.byteSize, + sha256: stored.sha256, + originalFilename: stored.originalFilename, + createdByAgentId: actor.agentId, + createdByUserId: actor.actorType === "user" ? actor.actorId : null, + }); + + await logActivity(db, { + companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + action: "asset.created", + entityType: "asset", + entityId: asset.id, + details: { + originalFilename: asset.originalFilename, + contentType: asset.contentType, + byteSize: asset.byteSize, + namespace: "assets/companies", + }, + }); + + res.status(201).json({ + assetId: asset.id, + companyId: asset.companyId, + provider: asset.provider, + objectKey: asset.objectKey, + contentType: asset.contentType, + byteSize: asset.byteSize, + sha256: asset.sha256, + originalFilename: asset.originalFilename, + createdByAgentId: asset.createdByAgentId, + createdByUserId: asset.createdByUserId, + createdAt: asset.createdAt, + updatedAt: asset.updatedAt, + contentPath: `/api/assets/${asset.id}/content`, + }); + }); + router.get("/assets/:assetId/content", async (req, res, next) => { const assetId = req.params.assetId as string; const asset = await svc.getById(assetId); diff --git a/ui/src/api/assets.ts b/ui/src/api/assets.ts index 8b3d056c..6fcf323f 100644 --- a/ui/src/api/assets.ts +++ b/ui/src/api/assets.ts @@ -11,11 +11,19 @@ export const assetsApi = { const safeFile = new File([buffer], file.name, { type: file.type }); const form = new FormData(); - form.append("file", safeFile); if (namespace && namespace.trim().length > 0) { form.append("namespace", namespace.trim()); } + form.append("file", safeFile); return api.postForm(`/companies/${companyId}/assets/images`, form); }, -}; + uploadCompanyLogo: async (companyId: string, file: File) => { + const buffer = await file.arrayBuffer(); + const safeFile = new File([buffer], file.name, { type: file.type }); + + const form = new FormData(); + form.append("file", safeFile); + return api.postForm(`/companies/${companyId}/logo`, form); + }, +}; diff --git a/ui/src/pages/CompanySettings.tsx b/ui/src/pages/CompanySettings.tsx index 3cd310ca..f805eebc 100644 --- a/ui/src/pages/CompanySettings.tsx +++ b/ui/src/pages/CompanySettings.tsx @@ -140,7 +140,7 @@ export function CompanySettings() { const logoUploadMutation = useMutation({ mutationFn: (file: File) => assetsApi - .uploadImage(selectedCompanyId!, file, "companies") + .uploadCompanyLogo(selectedCompanyId!, file) .then((asset) => companiesApi.update(selectedCompanyId!, { logoAssetId: asset.assetId })), onSuccess: (company) => { syncLogoState(company.logoUrl); @@ -160,8 +160,8 @@ export function CompanySettings() { const file = event.target.files?.[0] ?? null; event.currentTarget.value = ""; if (!file) return; - if (file.size >= 100 * 1024) { - setLogoUploadError("Logo image must be smaller than 100 KB."); + if (file.size > 100 * 1024) { + setLogoUploadError("Logo image must be 100 KB or smaller."); return; } setLogoUploadError(null);