Skip to content

Commit e8922ee

Browse files
fix(s3): trim whitespace in bucket name + creds so uploads can't break on a stray space (#1338)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 80bbc3d commit e8922ee

3 files changed

Lines changed: 54 additions & 5 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { describe, it, expect, beforeAll, afterAll } from "vitest";
2+
3+
// Regression guard: a trailing space in S3_BUCKET_NAME (or the keys) once made
4+
// every presigned PUT fail with 400 InvalidBucketName. These values must be
5+
// trimmed before they reach the bucket name / SigV4 credential.
6+
describe("getPresignedUrl whitespace hardening", () => {
7+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
8+
let getPresignedUrl: any;
9+
10+
beforeAll(async () => {
11+
// Whitespace-padded credentials, set before import so the s3 client (built
12+
// at module load) sees them.
13+
process.env.ACCESS_KEY = "AKIATESTKEY1234567890 ";
14+
process.env.SECRET_KEY = "secretsecretsecretsecretsecretsecret1234\n";
15+
({ getPresignedUrl } = await import("@/server/common/getPresignedUrl"));
16+
});
17+
18+
afterAll(() => {
19+
delete process.env.ACCESS_KEY;
20+
delete process.env.SECRET_KEY;
21+
delete process.env.S3_BUCKET_NAME;
22+
});
23+
24+
it("trims a trailing space in S3_BUCKET_NAME so the PUT targets the real bucket", async () => {
25+
process.env.S3_BUCKET_NAME = "codu.uploads "; // <- the prod footgun
26+
const url = await getPresignedUrl("image/png", 123, {
27+
kind: "uploads",
28+
userId: "u1",
29+
});
30+
expect(url).toContain("/codu.uploads/");
31+
expect(url).not.toContain("codu.uploads%20");
32+
expect(url).not.toContain("codu.uploads ");
33+
});
34+
35+
it("trims whitespace in the access key used to sign the URL", async () => {
36+
process.env.S3_BUCKET_NAME = "codu.uploads";
37+
const url = await getPresignedUrl("image/png", 123, {
38+
kind: "uploads",
39+
userId: "u1",
40+
});
41+
const cred = new URL(url).searchParams.get("X-Amz-Credential") ?? "";
42+
expect(cred.startsWith("AKIATESTKEY1234567890/")).toBe(true);
43+
expect(cred).not.toContain("%20");
44+
});
45+
});

server/common/getPresignedUrl.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,14 @@ export const getPresignedUrl = async (
4242
const Key = getKey(config, extension);
4343

4444
const putCommand = new PutObjectCommand({
45-
Bucket: process.env.S3_BUCKET_NAME,
45+
// Trim: a stray space in S3_BUCKET_NAME (invisible in host UIs) makes S3
46+
// reject every PUT with 400 InvalidBucketName.
47+
Bucket: process.env.S3_BUCKET_NAME?.trim(),
4648
Key,
4749
ContentType: `image/${fileType}`,
4850
ContentLength: fileSize,
4951
});
5052

51-
// @FIX TS ERROR
5253
const putUrl = await getSignedUrl(s3Client, putCommand, {
5354
expiresIn: 3600,
5455
});

utils/s3helpers.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
import { S3Client } from "@aws-sdk/client-s3";
22

3-
const hasAccessKeys = process.env.ACCESS_KEY && process.env.SECRET_KEY;
3+
// Trim: stray whitespace in the key env vars otherwise breaks SigV4 signing.
4+
const accessKeyId = process.env.ACCESS_KEY?.trim();
5+
const secretAccessKey = process.env.SECRET_KEY?.trim();
6+
const hasAccessKeys = accessKeyId && secretAccessKey;
47

58
export const s3Client = new S3Client({
69
region: "eu-west-1",
710
...(hasAccessKeys
811
? {
912
credentials: {
10-
accessKeyId: process.env.ACCESS_KEY || "",
11-
secretAccessKey: process.env.SECRET_KEY || "",
13+
accessKeyId,
14+
secretAccessKey,
1215
},
1316
}
1417
: {}),

0 commit comments

Comments
 (0)