From 40af11521f9a621b929ca703605c95dbd84478a5 Mon Sep 17 00:00:00 2001 From: NiallJoeMaher Date: Wed, 17 Jun 2026 15:30:53 +0100 Subject: [PATCH 1/2] fix(s3): trim whitespace in bucket name + creds so uploads can't break on a stray space A trailing space in the prod S3_BUCKET_NAME env var made every presigned PUT target the invalid bucket "codu.uploads " -> S3 returned 400 InvalidBucketName, taking uploads down entirely. The whitespace is invisible in the Vercel UI, and S3_BUCKET_NAME/ACCESS_KEY/SECRET_KEY aren't in config/env.js validation, so the bad value failed silently at runtime instead of at boot. Trim these values at the point of use (getPresignedUrl bucket name, s3 client credentials) so stray whitespace on any of them can't break signing or the PUT. Adds a regression test that fails without the trim (URL came out as .../codu.uploads%20/...) and passes with it. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/common/getPresignedUrl.test.ts | 39 +++++++++++++++++++++++++++ server/common/getPresignedUrl.ts | 4 ++- utils/s3helpers.ts | 9 ++++--- 3 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 server/common/getPresignedUrl.test.ts diff --git a/server/common/getPresignedUrl.test.ts b/server/common/getPresignedUrl.test.ts new file mode 100644 index 000000000..e92531694 --- /dev/null +++ b/server/common/getPresignedUrl.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect, beforeAll } from "vitest"; + +// Regression guard: a trailing space in S3_BUCKET_NAME (or the keys) once made +// every presigned PUT fail with 400 InvalidBucketName. These values must be +// trimmed before they reach the bucket name / SigV4 credential. +describe("getPresignedUrl whitespace hardening", () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let getPresignedUrl: any; + + beforeAll(async () => { + // Whitespace-padded credentials, set before import so the s3 client (built + // at module load) sees them. + process.env.ACCESS_KEY = "AKIATESTKEY1234567890 "; + process.env.SECRET_KEY = "secretsecretsecretsecretsecretsecret1234\n"; + ({ getPresignedUrl } = await import("@/server/common/getPresignedUrl")); + }); + + it("trims a trailing space in S3_BUCKET_NAME so the PUT targets the real bucket", async () => { + process.env.S3_BUCKET_NAME = "codu.uploads "; // <- the prod footgun + const url = await getPresignedUrl("image/png", 123, { + kind: "uploads", + userId: "u1", + }); + expect(url).toContain("/codu.uploads/"); + expect(url).not.toContain("codu.uploads%20"); + expect(url).not.toContain("codu.uploads "); + }); + + it("trims whitespace in the access key used to sign the URL", async () => { + process.env.S3_BUCKET_NAME = "codu.uploads"; + const url = await getPresignedUrl("image/png", 123, { + kind: "uploads", + userId: "u1", + }); + const cred = new URL(url).searchParams.get("X-Amz-Credential") ?? ""; + expect(cred.startsWith("AKIATESTKEY1234567890/")).toBe(true); + expect(cred).not.toContain("%20"); + }); +}); diff --git a/server/common/getPresignedUrl.ts b/server/common/getPresignedUrl.ts index a20eee233..15535559a 100644 --- a/server/common/getPresignedUrl.ts +++ b/server/common/getPresignedUrl.ts @@ -42,7 +42,9 @@ export const getPresignedUrl = async ( const Key = getKey(config, extension); const putCommand = new PutObjectCommand({ - Bucket: process.env.S3_BUCKET_NAME, + // Trim: a stray space in S3_BUCKET_NAME (invisible in host UIs) makes S3 + // reject every PUT with 400 InvalidBucketName. + Bucket: process.env.S3_BUCKET_NAME?.trim(), Key, ContentType: `image/${fileType}`, ContentLength: fileSize, diff --git a/utils/s3helpers.ts b/utils/s3helpers.ts index 00d4382ca..e70b1717d 100644 --- a/utils/s3helpers.ts +++ b/utils/s3helpers.ts @@ -1,14 +1,17 @@ import { S3Client } from "@aws-sdk/client-s3"; -const hasAccessKeys = process.env.ACCESS_KEY && process.env.SECRET_KEY; +// Trim: stray whitespace in the key env vars otherwise breaks SigV4 signing. +const accessKeyId = process.env.ACCESS_KEY?.trim(); +const secretAccessKey = process.env.SECRET_KEY?.trim(); +const hasAccessKeys = accessKeyId && secretAccessKey; export const s3Client = new S3Client({ region: "eu-west-1", ...(hasAccessKeys ? { credentials: { - accessKeyId: process.env.ACCESS_KEY || "", - secretAccessKey: process.env.SECRET_KEY || "", + accessKeyId, + secretAccessKey, }, } : {}), From 510b821ddb8d8c77d0932a39bcbec5e66887d969 Mon Sep 17 00:00:00 2001 From: NiallJoeMaher Date: Wed, 17 Jun 2026 15:35:00 +0100 Subject: [PATCH 2/2] chore(review): self-contained test cleanup + drop stale @FIX comment - restore process.env in afterAll so the suite doesn't lean on neighbours - remove the misleading `// @FIX TS ERROR` comment (no actual error) Co-Authored-By: Claude Opus 4.8 (1M context) --- server/common/getPresignedUrl.test.ts | 8 +++++++- server/common/getPresignedUrl.ts | 1 - 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/server/common/getPresignedUrl.test.ts b/server/common/getPresignedUrl.test.ts index e92531694..8260ece53 100644 --- a/server/common/getPresignedUrl.test.ts +++ b/server/common/getPresignedUrl.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeAll } from "vitest"; +import { describe, it, expect, beforeAll, afterAll } from "vitest"; // Regression guard: a trailing space in S3_BUCKET_NAME (or the keys) once made // every presigned PUT fail with 400 InvalidBucketName. These values must be @@ -15,6 +15,12 @@ describe("getPresignedUrl whitespace hardening", () => { ({ getPresignedUrl } = await import("@/server/common/getPresignedUrl")); }); + afterAll(() => { + delete process.env.ACCESS_KEY; + delete process.env.SECRET_KEY; + delete process.env.S3_BUCKET_NAME; + }); + it("trims a trailing space in S3_BUCKET_NAME so the PUT targets the real bucket", async () => { process.env.S3_BUCKET_NAME = "codu.uploads "; // <- the prod footgun const url = await getPresignedUrl("image/png", 123, { diff --git a/server/common/getPresignedUrl.ts b/server/common/getPresignedUrl.ts index 15535559a..099a4e674 100644 --- a/server/common/getPresignedUrl.ts +++ b/server/common/getPresignedUrl.ts @@ -50,7 +50,6 @@ export const getPresignedUrl = async ( ContentLength: fileSize, }); - // @FIX TS ERROR const putUrl = await getSignedUrl(s3Client, putCommand, { expiresIn: 3600, });