fix(server): accept Office issue attachments (#8562)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents and board users can attach files to issues so context and deliverables stay with the task > - Some clients upload Microsoft Office files with generic binary MIME types such as `application/octet-stream` > - Current `master` now accepts arbitrary issue attachment MIME types, so the upload should keep working for unknown binary files > - Office files still benefit from being stored with a specific Office MIME type when the filename makes that inference safe > - Shared attachment allow-list defaults should also include common Office MIME types for routes that still use that allow-list > - This pull request keeps the current arbitrary-MIME issue upload behavior and only narrows generic binary uploads to Office MIME types for known Office filename extensions ## Linked Issues or Issue Description Fixes #8243 Duplicate search performed before implementation: - No matching open or closed PR found for `8243`, `Office document`, `attachment MIME`, or `openxmlformats`. ## What Changed - Added common Office MIME types to the default shared attachment allow-list. - Added upload content-type normalization that maps generic binary uploads to a specific Office MIME type only for known Office filename extensions. - Added an optional helper-level allow-list gate so callers that still validate against an effective allow-list can keep generic binary uploads generic when the inferred Office MIME type is not allowed. - Reused the shared generic attachment content-type list for response handling. - Preserved current `master` behavior for issue uploads that use unknown or arbitrary MIME types. - Added regression coverage for default Office allow-list matching, filename inference, optional allow-list fallback, official Office MIME uploads, inferred generic Office uploads, and preservation of unknown generic binary uploads. ## Verification - `env CI=true corepack pnpm install --frozen-lockfile --force` - `env CI=true corepack pnpm --filter @paperclipai/server exec vitest run src/__tests__/attachment-types.test.ts src/__tests__/issue-attachment-routes.test.ts` - `env CI=true corepack pnpm --filter @paperclipai/plugin-sdk ensure-build-deps` - `env CI=true corepack pnpm --filter @paperclipai/server exec tsc --noEmit` - `git diff --check origin/master...HEAD` GitHub CI, security checks, and Greptile pass on rebased head `acc364cfbe3440a59db6570bb907818046649eb4`. ## Risks Low risk. The issue attachment route continues to accept arbitrary MIME types as current `master` does; this change only stores a more specific Office MIME type for generic binary uploads when the filename has a known Office extension. Unknown generic binary uploads remain generic. For callers that use an allow-list before storing uploads, `normalizeUploadAttachmentContentType` supports an optional gate so inference can be limited to MIME types that are already allowed. No docs change included because this is a default upload compatibility fix covered by server tests. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. This is a narrow bug fix, not roadmap-level core feature work. `ROADMAP.md` was checked. ## Model Used OpenAI Codex using GPT-5, tool-enabled coding agent. Context window details are not exposed in this environment. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Sami Rusani <sr@samirusani>
This commit is contained in:
Sam
GitHub
parent
3e1dc90bf2
commit
b565603a86
|
|
@ -2,9 +2,11 @@ import { describe, it, expect } from "vitest";
|
|||
import {
|
||||
DEFAULT_ALLOWED_TYPES,
|
||||
INLINE_ATTACHMENT_TYPES,
|
||||
inferOfficeAttachmentContentTypeFromFilename,
|
||||
isInlineAttachmentContentType,
|
||||
matchesContentType,
|
||||
normalizeContentType,
|
||||
normalizeUploadAttachmentContentType,
|
||||
parseAllowedTypes,
|
||||
} from "../attachment-types.js";
|
||||
|
||||
|
|
@ -97,6 +99,19 @@ describe("matchesContentType", () => {
|
|||
expect(matchesContentType("text/plain", patterns)).toBe(true);
|
||||
expect(matchesContentType("application/zip", patterns)).toBe(true);
|
||||
});
|
||||
|
||||
it("allows common Office document types by default", () => {
|
||||
for (const contentType of [
|
||||
"application/msword",
|
||||
"application/vnd.ms-excel",
|
||||
"application/vnd.ms-powerpoint",
|
||||
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
|
||||
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
|
||||
"application/vnd.openxmlformats-officedocument.presentationml.presentation",
|
||||
]) {
|
||||
expect(matchesContentType(contentType, [...DEFAULT_ALLOWED_TYPES])).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeContentType", () => {
|
||||
|
|
@ -110,6 +125,67 @@ describe("normalizeContentType", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("inferOfficeAttachmentContentTypeFromFilename", () => {
|
||||
it("infers common Office content types from filenames", () => {
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename("notes.docx")).toBe(
|
||||
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
|
||||
);
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename("raw-data.xlsx")).toBe(
|
||||
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
|
||||
);
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename("deck.pptx")).toBe(
|
||||
"application/vnd.openxmlformats-officedocument.presentationml.presentation",
|
||||
);
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename("legacy.doc")).toBe("application/msword");
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename("legacy.xls")).toBe("application/vnd.ms-excel");
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename("legacy.ppt")).toBe("application/vnd.ms-powerpoint");
|
||||
});
|
||||
|
||||
it("does not infer unknown extensions", () => {
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename("payload.bin")).toBeNull();
|
||||
expect(inferOfficeAttachmentContentTypeFromFilename(undefined)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeUploadAttachmentContentType", () => {
|
||||
it("keeps explicit content types unchanged", () => {
|
||||
expect(
|
||||
normalizeUploadAttachmentContentType({
|
||||
contentType: "application/pdf",
|
||||
originalFilename: "raw-data.xlsx",
|
||||
}),
|
||||
).toBe("application/pdf");
|
||||
});
|
||||
|
||||
it("infers Office content type for generic binary uploads", () => {
|
||||
expect(
|
||||
normalizeUploadAttachmentContentType({
|
||||
contentType: "application/octet-stream",
|
||||
originalFilename: "raw-data.xlsx",
|
||||
}),
|
||||
).toBe("application/vnd.openxmlformats-officedocument.spreadsheetml.sheet");
|
||||
});
|
||||
|
||||
it("keeps generic binary uploads generic when the inferred Office type is not allowed", () => {
|
||||
expect(
|
||||
normalizeUploadAttachmentContentType({
|
||||
contentType: "application/octet-stream",
|
||||
originalFilename: "raw-data.xlsx",
|
||||
isAllowedContentType: (contentType) => contentType === "application/octet-stream",
|
||||
}),
|
||||
).toBe("application/octet-stream");
|
||||
});
|
||||
|
||||
it("keeps generic binary uploads generic for unknown filenames", () => {
|
||||
expect(
|
||||
normalizeUploadAttachmentContentType({
|
||||
contentType: "application/octet-stream",
|
||||
originalFilename: "payload.bin",
|
||||
}),
|
||||
).toBe("application/octet-stream");
|
||||
});
|
||||
});
|
||||
|
||||
describe("isInlineAttachmentContentType", () => {
|
||||
it("allows the configured inline-safe types", () => {
|
||||
for (const contentType of ["image/png", "image/svg+xml", "application/pdf", "text/plain", "video/mp4"]) {
|
||||
|
|
|
|||
|
|
@ -350,6 +350,93 @@ describe("issue attachment routes", () => {
|
|||
expect(res.body.contentType).toBe("application/x-msdownload");
|
||||
});
|
||||
|
||||
it("accepts Office uploads with official MIME types for issue attachments", async () => {
|
||||
const contentType = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet";
|
||||
const storage = createStorageService();
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
companyId: "company-1",
|
||||
identifier: "PAP-1",
|
||||
});
|
||||
mockIssueService.createAttachment.mockResolvedValue(makeAttachment(contentType, "raw-data.xlsx"));
|
||||
|
||||
const app = await createApp(storage);
|
||||
const res = await request(app)
|
||||
.post("/api/companies/company-1/issues/11111111-1111-4111-8111-111111111111/attachments")
|
||||
.attach("file", Buffer.from("xlsx"), { filename: "raw-data.xlsx", contentType });
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(storage.__calls.putFile).toMatchObject({
|
||||
contentType,
|
||||
originalFilename: "raw-data.xlsx",
|
||||
});
|
||||
expect(mockIssueService.createAttachment).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
contentType,
|
||||
originalFilename: "raw-data.xlsx",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("infers Office MIME types for generic binary issue attachment uploads", async () => {
|
||||
const contentType = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet";
|
||||
const storage = createStorageService();
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
companyId: "company-1",
|
||||
identifier: "PAP-1",
|
||||
});
|
||||
mockIssueService.createAttachment.mockResolvedValue(makeAttachment(contentType, "raw-data.xlsx"));
|
||||
|
||||
const app = await createApp(storage);
|
||||
const res = await request(app)
|
||||
.post("/api/companies/company-1/issues/11111111-1111-4111-8111-111111111111/attachments")
|
||||
.attach("file", Buffer.from("xlsx"), {
|
||||
filename: "raw-data.xlsx",
|
||||
contentType: "application/octet-stream",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(storage.__calls.putFile).toMatchObject({
|
||||
contentType,
|
||||
originalFilename: "raw-data.xlsx",
|
||||
});
|
||||
expect(mockIssueService.createAttachment).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
contentType,
|
||||
originalFilename: "raw-data.xlsx",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves generic binary uploads when the filename is not a known Office document", async () => {
|
||||
const storage = createStorageService();
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
companyId: "company-1",
|
||||
identifier: "PAP-1",
|
||||
});
|
||||
mockIssueService.createAttachment.mockResolvedValue(makeAttachment("application/octet-stream", "payload.bin"));
|
||||
|
||||
const app = await createApp(storage);
|
||||
const res = await request(app)
|
||||
.post("/api/companies/company-1/issues/11111111-1111-4111-8111-111111111111/attachments")
|
||||
.attach("file", Buffer.from("bin"), { filename: "payload.bin", contentType: "application/octet-stream" });
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(storage.__calls.putFile).toMatchObject({
|
||||
contentType: "application/octet-stream",
|
||||
originalFilename: "payload.bin",
|
||||
});
|
||||
expect(mockIssueService.createAttachment).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
contentType: "application/octet-stream",
|
||||
originalFilename: "payload.bin",
|
||||
}),
|
||||
);
|
||||
expect(res.body.contentType).toBe("application/octet-stream");
|
||||
});
|
||||
|
||||
it("enforces the process-level issue attachment limit even when the company limit allows more", async () => {
|
||||
const storage = createStorageService();
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
|
|
|
|||
|
|
@ -32,6 +32,12 @@ export const DEFAULT_ALLOWED_TYPES: readonly string[] = [
|
|||
"application/json",
|
||||
"text/csv",
|
||||
"text/html",
|
||||
"application/msword",
|
||||
"application/vnd.ms-excel",
|
||||
"application/vnd.ms-powerpoint",
|
||||
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
|
||||
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
|
||||
"application/vnd.openxmlformats-officedocument.presentationml.presentation",
|
||||
"video/mp4",
|
||||
"video/webm",
|
||||
"video/quicktime",
|
||||
|
|
@ -40,6 +46,11 @@ export const DEFAULT_ALLOWED_TYPES: readonly string[] = [
|
|||
|
||||
export const DEFAULT_ATTACHMENT_CONTENT_TYPE = "application/octet-stream";
|
||||
export const SVG_CONTENT_TYPE = "image/svg+xml";
|
||||
export const GENERIC_ATTACHMENT_CONTENT_TYPES: readonly string[] = [
|
||||
"application/octet-stream",
|
||||
"binary/octet-stream",
|
||||
"application/x-binary",
|
||||
];
|
||||
export const INLINE_ATTACHMENT_TYPES: readonly string[] = [
|
||||
"image/*",
|
||||
"application/pdf",
|
||||
|
|
@ -88,6 +99,38 @@ export function normalizeContentType(contentType: string | null | undefined): st
|
|||
return normalized || DEFAULT_ATTACHMENT_CONTENT_TYPE;
|
||||
}
|
||||
|
||||
export function inferOfficeAttachmentContentTypeFromFilename(
|
||||
filename: string | null | undefined,
|
||||
): string | null {
|
||||
const lower = (filename ?? "").trim().toLowerCase();
|
||||
if (lower.endsWith(".docx")) {
|
||||
return "application/vnd.openxmlformats-officedocument.wordprocessingml.document";
|
||||
}
|
||||
if (lower.endsWith(".xlsx")) {
|
||||
return "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet";
|
||||
}
|
||||
if (lower.endsWith(".pptx")) {
|
||||
return "application/vnd.openxmlformats-officedocument.presentationml.presentation";
|
||||
}
|
||||
if (lower.endsWith(".doc")) return "application/msword";
|
||||
if (lower.endsWith(".xls")) return "application/vnd.ms-excel";
|
||||
if (lower.endsWith(".ppt")) return "application/vnd.ms-powerpoint";
|
||||
return null;
|
||||
}
|
||||
|
||||
export function normalizeUploadAttachmentContentType(input: {
|
||||
contentType: string | null | undefined;
|
||||
originalFilename?: string | null;
|
||||
isAllowedContentType?: (contentType: string) => boolean;
|
||||
}): string {
|
||||
const normalized = normalizeContentType(input.contentType);
|
||||
if (!GENERIC_ATTACHMENT_CONTENT_TYPES.includes(normalized)) return normalized;
|
||||
const inferred = inferOfficeAttachmentContentTypeFromFilename(input.originalFilename);
|
||||
if (!inferred) return normalized;
|
||||
if (input.isAllowedContentType && !input.isAllowedContentType(inferred)) return normalized;
|
||||
return inferred;
|
||||
}
|
||||
|
||||
export function isInlineAttachmentContentType(contentType: string): boolean {
|
||||
return matchesContentType(contentType, [...INLINE_ATTACHMENT_TYPES]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -137,9 +137,11 @@ import {
|
|||
} from "./workspace-command-authz.js";
|
||||
import { shouldWakeAssigneeOnCheckout } from "./issues-checkout-wakeup.js";
|
||||
import {
|
||||
GENERIC_ATTACHMENT_CONTENT_TYPES,
|
||||
isInlineAttachmentContentType,
|
||||
normalizeIssueAttachmentMaxBytes,
|
||||
normalizeContentType,
|
||||
normalizeUploadAttachmentContentType,
|
||||
SVG_CONTENT_TYPE,
|
||||
} from "../attachment-types.js";
|
||||
import { queueIssueAssignmentWakeup } from "../services/issue-assignment-wakeup.js";
|
||||
|
|
@ -351,11 +353,7 @@ function buildAttachmentContentPath(attachmentId: string): string {
|
|||
return `/api/attachments/${attachmentId}/content`;
|
||||
}
|
||||
|
||||
const GENERIC_ATTACHMENT_CONTENT_TYPES = new Set([
|
||||
"application/octet-stream",
|
||||
"binary/octet-stream",
|
||||
"application/x-binary",
|
||||
]);
|
||||
const GENERIC_RESPONSE_ATTACHMENT_CONTENT_TYPES = new Set(GENERIC_ATTACHMENT_CONTENT_TYPES);
|
||||
|
||||
function inferVideoContentTypeFromFilename(filename: string | null | undefined): string | null {
|
||||
const lower = (filename ?? "").toLowerCase();
|
||||
|
|
@ -371,7 +369,7 @@ function resolveAttachmentResponseContentType(input: {
|
|||
originalFilename?: string | null;
|
||||
}) {
|
||||
const storedContentType = normalizeContentType(input.storedContentType || input.objectContentType);
|
||||
if (!GENERIC_ATTACHMENT_CONTENT_TYPES.has(storedContentType)) return storedContentType;
|
||||
if (!GENERIC_RESPONSE_ATTACHMENT_CONTENT_TYPES.has(storedContentType)) return storedContentType;
|
||||
return inferVideoContentTypeFromFilename(input.originalFilename) ?? storedContentType;
|
||||
}
|
||||
|
||||
|
|
@ -10443,7 +10441,10 @@ export function issueRoutes(
|
|||
res.status(400).json({ error: "Missing file field 'file'" });
|
||||
return;
|
||||
}
|
||||
const contentType = normalizeContentType(file.mimetype);
|
||||
const contentType = normalizeUploadAttachmentContentType({
|
||||
contentType: file.mimetype,
|
||||
originalFilename: file.originalname,
|
||||
});
|
||||
if (file.buffer.length <= 0) {
|
||||
res.status(422).json({ error: "Attachment is empty" });
|
||||
return;
|
||||
|
|
|
|||
Loading…
Reference in New Issue