feature: move S3Service off the minio client to files-sdk

package.json

@@ -34,6 +34,10 @@
     "postinstall": "patch-package"
   },
   "dependencies": {
+    "@aws-sdk/client-s3": "^3.1125.0",
+    "@aws-sdk/lib-storage": "^3.1125.0",
+    "@aws-sdk/s3-presigned-post": "^3.1125.0",
+    "@aws-sdk/s3-request-presigner": "^3.1125.0",
     "@bull-board/api": "^6.3.0",
     "@bull-board/express": "^6.3.0",
     "@bull-board/nestjs": "^6.3.0",
@@ -58,10 +62,10 @@
     "connect-redis": "^7.1.0",
     "discord.js": "^14.14.1",
     "express-session": "^1.18.0",
+    "files-sdk": "^2.3.0",
     "globaloffensive": "^3.0.2",
     "ioredis": "^5.6.1",
     "jsonwebtoken": "^9.0.2",
-    "minio": "^8.0.1",
     "minisearch": "^7.1.0",
     "oauth": "^0.10.0",
     "openpgp": "^6.1.1",

src/configs/s3.ts

@@ -8,8 +8,16 @@ export default (): {
     secret: process.env.S3_SECRET,
     bucket: process.env.S3_BUCKET,
     db_backup_bucket: process.env.S3_DB_BACKUP_BUCKET || "5stack-db-backups",
-    endpoint: process.env.S3_ENDPOINT || "minio",
+    endpoint: process.env.S3_ENDPOINT || "rustfs",
+    // SigV4 puts the region in the credential scope, so a remote bucket whose
+    // region this does not match rejects every signature. The in-cluster store
+    // accepts any region, which is why this can have a default at all.
+    region: process.env.S3_REGION || "us-east-1",
     useSSL: process.env.S3_USE_SSL === "true" ? true : false,
+    // AWS dropped path-style addressing for buckets created after Sept 2020,
+    // so a real AWS endpoint needs this off. Every other S3 implementation
+    // 5stack targets accepts path style, and the in-cluster store requires it.
+    forcePathStyle: process.env.S3_FORCE_PATH_STYLE !== "false",
     port: process.env.S3_PORT || "9000",
   },
 });

src/configs/types/S3Config.ts

@@ -4,6 +4,8 @@ export type S3Config = {
   bucket: string;
   db_backup_bucket: string;
   endpoint: string;
+  region: string;
   useSSL: boolean;
+  forcePathStyle: boolean;
   port: string;
 };

src/s3-scan/s3-scan.service.ts

@@ -1,5 +1,4 @@
 import { Injectable, Logger } from "@nestjs/common";
-import { ObjectInfo } from "minio/dist/main/internal/type";
 import {
   e_notification_types_enum,
   e_player_roles_enum,
@@ -105,8 +104,7 @@ export class S3ScanService {
       const sizeByKey = new Map<string, number>();
 
       const stream = this.s3.listStream();
-      for await (const entry of stream) {
-        const obj = entry as ObjectInfo;
+      for await (const obj of stream) {
         const key = obj.name;
         if (!key) {
           continue;

src/s3/s3.service.spec.ts

@@ -0,0 +1,136 @@
+import { S3Service } from "./s3.service";
+
+const DEMOS_DOMAIN = "demos.example.com";
+
+const build = (
+  endpoint: string,
+  port: string,
+  useSSL: boolean,
+  overrides: { region?: string; forcePathStyle?: boolean } = {},
+) => {
+  const logger = { log: jest.fn(), warn: jest.fn(), error: jest.fn() };
+
+  const config = {
+    get: () => ({
+      key: "access-key",
+      secret: "secret-key",
+      bucket: "5stack",
+      db_backup_bucket: "5stack-db-backups",
+      endpoint,
+      region: "us-east-1",
+      useSSL,
+      forcePathStyle: true,
+      port,
+      ...overrides,
+    }),
+  };
+
+  return new S3Service(logger as never, config as never);
+};
+
+const hostOf = (url: string) => new URL(url).host;
+
+// Exercised through getPresignedPartUrl because it shares endpointFor() and
+// isInternalStore with every other signing path while staying on the AWS SDK.
+// The files-sdk paths cannot be reached from here: it is ESM-only, and a real
+// dynamic import inside jest's VM needs --experimental-vm-modules.
+describe("S3Service presigned url routing", () => {
+  const originalDomain = process.env.DEMOS_DOMAIN;
+
+  beforeAll(() => {
+    process.env.DEMOS_DOMAIN = DEMOS_DOMAIN;
+  });
+
+  afterAll(() => {
+    // Assigning undefined would store the string "undefined" and leak that
+    // into every later test in this worker.
+    if (originalDomain === undefined) {
+      delete process.env.DEMOS_DOMAIN;
+      return;
+    }
+
+    process.env.DEMOS_DOMAIN = originalDomain;
+  });
+
+  // A URL signed against either in-cluster name is unreachable from the
+  // browser doing the upload, so both have to route to the demos domain.
+  describe.each(["rustfs", "minio"])("with S3_ENDPOINT=%s", (endpoint) => {
+    it("signs multipart part uploads against the public demos domain", async () => {
+      const url = await build(endpoint, "9000", false).getPresignedPartUrl(
+        "key",
+        "upload-id",
+        1,
+        60,
+      );
+
+      expect(hostOf(url)).toBe(DEMOS_DOMAIN);
+    });
+
+    it("carries the upload id and part number into the signature", async () => {
+      const url = await build(endpoint, "9000", false).getPresignedPartUrl(
+        "demos/match/big.dem",
+        "upload-id",
+        7,
+        60,
+      );
+
+      const params = new URL(url).searchParams;
+
+      expect(params.get("uploadId")).toBe("upload-id");
+      expect(params.get("partNumber")).toBe("7");
+    });
+  });
+
+  describe("with a remote bucket", () => {
+    const endpoint = "s3.us-east-005.backblazeb2.com";
+
+    it("signs against the remote host, not the demos domain", async () => {
+      const url = await build(endpoint, "443", true).getPresignedPartUrl(
+        "key",
+        "upload-id",
+        1,
+        60,
+      );
+
+      expect(hostOf(url)).toBe(endpoint);
+    });
+
+    // A region that does not match the endpoint's puts the wrong scope in the
+    // signature, and the store rejects every signed request.
+    it("signs with the configured region", async () => {
+      const url = await build(endpoint, "443", true, {
+        region: "us-east-005",
+      }).getPresignedPartUrl("key", "upload-id", 1, 60);
+
+      const credential = new URL(url).searchParams.get(
+        "X-Amz-Credential",
+      ) as string;
+
+      expect(credential).toContain("/us-east-005/s3/aws4_request");
+    });
+
+    it("addresses the bucket virtual-host style when path style is off", async () => {
+      const url = await build(endpoint, "443", true, {
+        forcePathStyle: false,
+      }).getPresignedPartUrl("key", "upload-id", 1, 60);
+
+      expect(hostOf(url)).toBe(`5stack.${endpoint}`);
+    });
+  });
+
+  // Path style is forced for the in-cluster store regardless of config: it is
+  // reached by Service name, and no DNS answers a bucket subdomain of it.
+  describe.each(["rustfs", "minio"])(
+    "with S3_ENDPOINT=%s and path style disabled",
+    (endpoint) => {
+      it("still addresses the bucket path style", async () => {
+        const url = await build(endpoint, "9000", false, {
+          forcePathStyle: false,
+        }).getPresignedPartUrl("key", "upload-id", 1, 60);
+
+        expect(hostOf(url)).toBe(DEMOS_DOMAIN);
+        expect(new URL(url).pathname).toBe("/5stack/key");
+      });
+    },
+  );
+});