Skip to content

fix(host-service): accept unknown mediaType on attachment upload #4439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
May 12, 2026
+30 −15
Merged

fix(host-service): accept unknown mediaType on attachment upload #4439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions packages/host-service/src/trpc/router/attachments/attachments.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,30 @@ describe("attachmentsRouter.upload", () => {
}
});

it("rejects unrecognized media type", async () => {
it("falls back to application/octet-stream for unrecognized media type", async () => {
const caller = createCaller();
await expect(
caller.upload({
data: { kind: "base64", data: PNG_BASE64 },
mediaType: "application/x-totally-fake",
}),
).rejects.toThrow(/unrecognized media type/i);
const result = await caller.upload({
data: { kind: "base64", data: PNG_BASE64 },
mediaType: "application/x-totally-fake",
originalFilename: "racetrack.bmad",
});
expect(result.mediaType).toBe("application/octet-stream");
const filePath = getAttachmentFilePath(
result.attachmentId,
"application/octet-stream",
);
expect(filePath).toMatch(/\.bin$/);
expect(existsSync(filePath)).toBe(true);
});

it("falls back to application/octet-stream for empty media type", async () => {
const caller = createCaller();
const result = await caller.upload({
data: { kind: "base64", data: PNG_BASE64 },
mediaType: "",
originalFilename: "racetrack.bmad",
});
expect(result.mediaType).toBe("application/octet-stream");
});
Comment on lines +103 to 111
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Empty-mediaType test doesn't verify disk write or metadata

The unrecognized-mediaType test (line 87) fully verifies the upload by checking existsSync(filePath) and that the path ends in .bin. This test only checks result.mediaType, leaving it possible that a regression in writeAttachment (e.g. an exception thrown after the fallback is selected) would go undetected here. Adding an existsSync assertion and a check that metadata.json contains originalFilename: "racetrack.bmad" would give symmetrical coverage to the two new fallback paths.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/attachments/attachments.test.ts
Line: 103-111

Comment:
**Empty-mediaType test doesn't verify disk write or metadata**

The unrecognized-mediaType test (line 87) fully verifies the upload by checking `existsSync(filePath)` and that the path ends in `.bin`. This test only checks `result.mediaType`, leaving it possible that a regression in `writeAttachment` (e.g. an exception thrown after the fallback is selected) would go undetected here. Adding an `existsSync` assertion and a check that `metadata.json` contains `originalFilename: "racetrack.bmad"` would give symmetrical coverage to the two new fallback paths.

How can I resolve this? If you propose a fix, please make it concise.


it("accepts a single decoded byte", async () => {
Expand Down
15 changes: 7 additions & 8 deletions packages/host-service/src/trpc/router/attachments/attachments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ const uploadInputSchema = z.object({
kind: z.literal("base64"),
data: z.string().min(1),
}),
mediaType: z.string().min(1),
mediaType: z.string(),
originalFilename: z.string().optional(),
});

const FALLBACK_MEDIA_TYPE = "application/octet-stream";

/**
* Cheap size estimate from a base64 string without allocating the
* decoded buffer. Used to reject oversized uploads before Buffer.from
Expand All @@ -36,12 +38,9 @@ export const attachmentsRouter = router({
* renderer never sees the on-disk path.
*/
upload: protectedProcedure.input(uploadInputSchema).mutation(({ input }) => {
if (!mimeTypes.extension(input.mediaType)) {
throw new TRPCError({
code: "BAD_REQUEST",
message: `Unrecognized media type: ${input.mediaType}`,
});
}
const mediaType = mimeTypes.extension(input.mediaType)
? input.mediaType
: FALLBACK_MEDIA_TYPE;

// Reject before allocating the decoded buffer so a 1GB base64
// payload doesn't spike host memory only to be rejected at the end.
Expand All @@ -65,7 +64,7 @@ export const attachmentsRouter = router({

const metadata: AttachmentMetadata = {
attachmentId: randomUUID(),
mediaType: input.mediaType,
mediaType,
originalFilename: input.originalFilename,
sizeBytes: bytes.length,
createdAt: Date.now(),
Expand Down
Loading