From aa3f51b4ed4cb93a5adb3fc937dcd34d93ec7eee Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Fri, 2 Oct 2026 09:33:07 +0200 Subject: [PATCH] Take the internal MinIO's connection from the catalog, not the container The internal mode read its endpoint and keys from MINIO_MCP_INTERNAL_* in the container's environment. They now arrive per session as pinned headers - x-minio-endpoint, x-minio-access-key, x-minio-secret-key and optionally x-minio-region - sent by the catalog entry that opens the session, next to x-minio-scope. That entry is where the platform's other credentials already live, and the one place the flow author's choice of storage is made. Each session builds and owns its client, closed with it, so there is no shared client to configure. The endpoint is held to MINIO_MCP_ALLOWED_ENDPOINTS like an external one: whoever holds the API key could otherwise send these headers and point the server at any address. The bucket prefix stays in the server's environment on purpose. It is the boundary the caller must not be able to set, and it still cannot be emptied. Co-Authored-By: Claude Sonnet 5.5 --- MCP-STACK.md | 2 +- mcp-stack.compose.yml | 11 +-- mcp-stack.env.example | 19 ++--- minio-mcp/README.md | 43 ++++++---- minio-mcp/src/connection-registry.js | 81 +++++++++++------- minio-mcp/src/index.js | 30 ++----- minio-mcp/test/connection-registry.test.js | 99 +++++++++++++--------- 7 files changed, 157 insertions(+), 128 deletions(-) diff --git a/MCP-STACK.md b/MCP-STACK.md index 2fd9f9d..cef35ad 100644 --- a/MCP-STACK.md +++ b/MCP-STACK.md @@ -6,7 +6,7 @@ This workspace contains four independent Streamable HTTP MCP servers: - `dev-server-mcp`: authenticated facade for a private, allowlisted development-server worker. One instance per execution, each in a throwaway copy of that execution's tree and on its own port. - `egress-proxy`: the development worker's only route to package registries; it allows their CONNECT requests and refuses everything else. - `browser-mcp`: Playwright automation restricted to exact allowed origins. -- `minio-mcp`: bounded list/read/stat/write access to object storage, with no delete tool. In internal mode (catalog entry `minio-mcp-internal`, header `x-minio-scope`) it uses the platform's MinIO from `MINIO_MCP_INTERNAL_*` and one bucket per execution, ``, created when the session opens; in external mode the model opens session-scoped connections to MinIO deployments itself. +- `minio-mcp`: bounded list/read/stat/write access to object storage, with no delete tool. In internal mode (catalog entry `minio-mcp-internal`, header `x-minio-scope`) it uses the platform's MinIO whose endpoint and keys the catalog entry sends as headers, and one bucket per execution, ``, created when the session opens; in external mode the model opens session-scoped connections to MinIO deployments itself. The MCP client orchestrates them; servers do not share MCP sessions or credentials. diff --git a/mcp-stack.compose.yml b/mcp-stack.compose.yml index 5daa08f..3d322ab 100644 --- a/mcp-stack.compose.yml +++ b/mcp-stack.compose.yml @@ -201,13 +201,10 @@ services: # Optional comma-separated origins. Empty permits any HTTP(S) MinIO origin; restrict this # when the set of deployments is known, because every allowed MCP client can open sessions. MINIO_MCP_ALLOWED_ENDPOINTS: ${MINIO_MCP_ALLOWED_ENDPOINTS:-} - # The platform's own MinIO, for sessions the catalog opens with x-minio-scope: one bucket - # per execution, , created on first use. Leave all three empty to offer only the external - # mode. The endpoint is reached over minio-egress, so it has to be routable from there. - MINIO_MCP_INTERNAL_ENDPOINT: ${MINIO_MCP_INTERNAL_ENDPOINT:-} - MINIO_MCP_INTERNAL_ACCESS_KEY: ${MINIO_MCP_INTERNAL_ACCESS_KEY:-} - MINIO_MCP_INTERNAL_SECRET_KEY: ${MINIO_MCP_INTERNAL_SECRET_KEY:-} - MINIO_MCP_INTERNAL_REGION: ${MINIO_MCP_INTERNAL_REGION:-} + # Internal mode: the workflow manager's catalog entry minio-mcp-internal sends the platform + # MinIO's endpoint and keys with x-minio-scope, and the bucket is + # created on first use. Only the prefix is this server's. The endpoint is reached over + # minio-egress and, when the allowlist above is set, has to be on it. MINIO_MCP_INTERNAL_BUCKET_PREFIX: ${MINIO_MCP_INTERNAL_BUCKET_PREFIX:-exec-} user: "${MCP_UID:-10001}:${MCP_GID:-10001}" read_only: true diff --git a/mcp-stack.env.example b/mcp-stack.env.example index af12e14..0b6d90f 100644 --- a/mcp-stack.env.example +++ b/mcp-stack.env.example @@ -15,19 +15,12 @@ MINIO_MCP_ALLOWED_ENDPOINTS= # Applies independently to reads and writes. Base64 overhead is handled by the HTTP body limit. MINIO_MCP_MAX_OBJECT_BYTES=1048576 -# The internal mode, chosen by the catalog entry minio-mcp-internal rather than by the model: the -# session arrives with x-minio-scope set to the execution's id, and this server uses the -# platform's MinIO below, creating the bucket on first use. These -# credentials never reach the model. They must be allowed to create buckets; leave all three empty -# to disable the mode. -MINIO_MCP_INTERNAL_ENDPOINT= -MINIO_MCP_INTERNAL_ACCESS_KEY= -MINIO_MCP_INTERNAL_SECRET_KEY= -# Defaults to MINIO_MCP_DEFAULT_REGION. -MINIO_MCP_INTERNAL_REGION= -# Every internal bucket starts with this, so the API key reaches only buckets this server made - -# never the storage nodes' or any other bucket on that MinIO. Cannot be empty; up to 27 characters -# of lowercase letters, digits and '-'. +# The internal mode is chosen by the catalog entry minio-mcp-internal rather than by the model, and +# its MinIO endpoint and keys are that entry's headers in the workflow manager's catalog - not +# variables here. The session arrives with x-minio-scope set to the execution's id, and this server +# creates the bucket on first use. Those keys must be allowed to create +# buckets, and the endpoint must be on MINIO_MCP_ALLOWED_ENDPOINTS when that is set. +# MINIO_MCP_INTERNAL_BUCKET_PREFIX=exec- # Use a dedicated project directory, never a home directory or filesystem root. diff --git a/minio-mcp/README.md b/minio-mcp/README.md index be00026..72f434f 100644 --- a/minio-mcp/README.md +++ b/minio-mcp/README.md @@ -4,9 +4,10 @@ Authenticated Streamable HTTP MCP server with two modes, chosen per MCP session - in the workflow manager, by which catalog entry the flow author picked - and never by the model: - **Internal** (`minio-mcp-internal`): the session is opened with an `x-minio-scope` header naming - the execution. The server uses the platform's own MinIO from its environment and creates the - bucket `` when the session opens, the way the coding agent creates an execution's workspace - directory. The model only lists, reads and writes keys; it sees no endpoint and no credential. + the execution, and headers for the platform MinIO's endpoint and keys, all set by the catalog + entry. The server creates the bucket `` when the session opens, the way the + coding agent creates an execution's workspace directory. The model only lists, reads and writes + keys; it sees no endpoint and no credential. - **External** (`minio-mcp`): no header. The model opens a MinIO session with endpoint, bucket and credentials, then uses the returned opaque ID for object operations, and may hold several. @@ -30,23 +31,35 @@ Optional variables: - `MINIO_MCP_REQUEST_MAX_BYTES`, `MINIO_MCP_MAX_SESSIONS`, `MINIO_MCP_SESSION_TTL_SECONDS`. - `MINIO_MCP_HOST` (default `0.0.0.0`), `MINIO_MCP_PORT` (default `3000`) and `MINIO_MCP_CORS_ORIGINS`. -Internal mode, all three or none (none disables the mode): +Internal mode: -- `MINIO_MCP_INTERNAL_ENDPOINT`, `MINIO_MCP_INTERNAL_ACCESS_KEY`, `MINIO_MCP_INTERNAL_SECRET_KEY`. -- `MINIO_MCP_INTERNAL_REGION` (defaults to `MINIO_MCP_DEFAULT_REGION`). - `MINIO_MCP_INTERNAL_BUCKET_PREFIX` (default `exec-`). Not optional: it cannot be set empty, and it is up to 27 lowercase letters, digits or `-`, leaving room for a 36-character execution id. -The bucket is always the prefix followed by the header's value, so a caller holding the API key -reaches only buckets this server created - never the storage nodes' or any other bucket on that -MinIO. Granting the internal credentials only `exec-*` buckets in a MinIO policy makes the same -boundary hold on the MinIO side too. +The connection is not configured here but sent per session, at `initialize`, by whoever opens it - +in the workflow manager, the `minio-mcp-internal` catalog entry, which is where the credentials +live: -The internal credentials must be allowed to create buckets. A session opened with `x-minio-scope` -while the mode is not configured, or with a value that does not make a valid bucket name behind the -prefix (lowercase letters, digits and `-`, at most 63 characters in all), is refused at -`initialize`. The header is fixed for the life of -the session: a request carrying a different value gets 400. +| Header | | +|---|---| +| `x-minio-scope` | the scope, normally `${{context.rootExecutionId}}`; required | +| `x-minio-endpoint` | S3 API origin, such as `http://minio:9000`; required | +| `x-minio-access-key`, `x-minio-secret-key` | required | +| `x-minio-region` | optional, defaults to `MINIO_MCP_DEFAULT_REGION` | + +All of them are fixed for the life of the session: a request carrying a different value gets 400. +They are never logged. The endpoint is held to `MINIO_MCP_ALLOWED_ENDPOINTS` like an external one, +since whoever holds the API key could otherwise send these headers to point the server anywhere. + +The bucket is always the prefix followed by the scope, so a caller holding the API key reaches only +buckets this server created - never the storage nodes' or any other bucket on that MinIO. Granting +the internal credentials only `exec-*` buckets in a MinIO policy makes the same boundary hold on +the MinIO side too. + +The internal credentials must be allowed to create buckets. A session missing a connection header, +or whose scope does not make a valid bucket name behind the prefix (lowercase letters, digits and +`-`, at most 63 characters in all), is refused at `initialize`, and so is one whose bucket cannot be +reached or created. In external mode, endpoint, bucket, access key and secret key are arguments to `minio_open_session`, not environment variables of the long-running server. The endpoint must be an HTTP(S) origin with no embedded diff --git a/minio-mcp/src/connection-registry.js b/minio-mcp/src/connection-registry.js index b311a38..79b1064 100644 --- a/minio-mcp/src/connection-registry.js +++ b/minio-mcp/src/connection-registry.js @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { MinioService, createS3Client, normalizeEndpoint } from "./minio-service.js"; +import { MinioService, normalizeEndpoint } from "./minio-service.js"; /** * The header that puts a session on the internal storage: its value names the scope - normally the @@ -8,10 +8,26 @@ import { MinioService, createS3Client, normalizeEndpoint } from "./minio-service */ export const INTERNAL_SCOPE_HEADER = "x-minio-scope"; +/** + * How the internal storage is reached, sent by the service from its own catalog entry alongside the + * scope. They live in the catalog, which only the service reads, so the MinIO credentials sit in + * one place and the model never sees them. + */ +export const INTERNAL_CONNECTION_HEADERS = Object.freeze({ + endpoint: "x-minio-endpoint", + accessKey: "x-minio-access-key", + secretKey: "x-minio-secret-key", + region: "x-minio-region" +}); + +/** Every header a session is pinned by, in the order the transport reads them. */ +export const PINNED_HEADERS = Object.freeze([INTERNAL_SCOPE_HEADER, ...Object.values(INTERNAL_CONNECTION_HEADERS)]); + /** * Every internal bucket starts with this, and the prefix is not optional: it is what keeps a caller * holding the API key inside the buckets this server made, rather than free to name any bucket on - * the platform's MinIO - the storage nodes' among them. + * the platform's MinIO - the storage nodes' among them. It is the server's, not the catalog's, for + * the same reason. */ export const DEFAULT_INTERNAL_BUCKET_PREFIX = "exec-"; @@ -67,11 +83,8 @@ export class MinioConnectionRegistry { maxObjectBytes = 1024 * 1024, defaultRegion = "us-east-1", allowedEndpoints = [], - // { endpoint, accessKey, secretKey, region, bucketPrefix } of the platform's own MinIO, or null - // when this server offers only sessions the model opens itself. - internal = null, - serviceFactory, - clientFactory = createS3Client + internalBucketPrefix = DEFAULT_INTERNAL_BUCKET_PREFIX, + serviceFactory } = {}) { if (!Number.isInteger(maxConnectionsPerSession) || maxConnectionsPerSession < 1) { throw new Error("maxConnectionsPerSession must be a positive integer"); @@ -80,46 +93,57 @@ export class MinioConnectionRegistry { this.maxObjectBytes = maxObjectBytes; this.defaultRegion = defaultRegion; this.allowedEndpoints = new Set(allowedEndpoints.map(normalizeEndpoint)); + this.internalBucketPrefix = validBucketPrefix(internalBucketPrefix); this.serviceFactory = serviceFactory ?? ((options) => new MinioService(options)); this.connections = new Map(); - this.internal = internal ? { - ...internal, - region: internal.region || defaultRegion, - bucketPrefix: validBucketPrefix(internal.bucketPrefix ?? DEFAULT_INTERNAL_BUCKET_PREFIX) - } : null; - // One client for every execution's bucket: the credentials are the same, only the bucket varies. - this.internalClient = this.internal ? clientFactory(this.internal) : null; this.internalSessions = new Map(); } /** - * Binds an MCP session to its execution's bucket on the internal MinIO - the server's prefix - * followed by the scope - creating the bucket when it does not exist yet. Runs at session open, so - * a session whose bucket cannot be had never opens. + * Binds an MCP session to its execution's bucket - the server's prefix followed by the scope - on + * the MinIO the session's headers name, creating the bucket when it does not exist yet. Runs at + * session open, so a session whose bucket cannot be had never opens. */ async openInternal(context) { const owner = mcpSessionId(context); const scope = internalScopeOf(context); if (scope === null) throw clientError(`${INTERNAL_SCOPE_HEADER} is required for the internal storage`); - if (!this.internal) throw clientError("The internal MinIO storage is not configured on this server"); - const bucket = this.internal.bucketPrefix + scope; + const bucket = this.internalBucketPrefix + scope; if (!SCOPE.test(scope) || !BUCKET_NAME.test(bucket)) { throw clientError(`${INTERNAL_SCOPE_HEADER} must be lowercase letters, digits or '-', and make a bucket name of at most 63 characters`); } if (this.internalSessions.has(owner)) return { bucket }; + const pinned = context.pinned ?? {}; + const missing = ["endpoint", "accessKey", "secretKey"] + .filter((name) => !pinned[INTERNAL_CONNECTION_HEADERS[name]]) + .map((name) => INTERNAL_CONNECTION_HEADERS[name]); + if (missing.length > 0) throw clientError(`The internal storage needs ${missing.join(", ")}`); + let endpoint; + try { endpoint = normalizeEndpoint(pinned[INTERNAL_CONNECTION_HEADERS.endpoint]); } + catch (error) { throw clientError(`${INTERNAL_CONNECTION_HEADERS.endpoint}: ${error.message}`); } + // The same allowlist as the external mode: whoever holds the API key could otherwise point this + // server at any address by sending the header themselves. + if (this.allowedEndpoints.size > 0 && !this.allowedEndpoints.has(endpoint)) { + throw clientError("This MinIO endpoint is not on the server allowlist"); + } + const service = this.serviceFactory({ - endpoint: this.internal.endpoint, + endpoint, bucket, - accessKey: this.internal.accessKey, - secretKey: this.internal.secretKey, - region: this.internal.region, - maxObjectBytes: this.maxObjectBytes, - client: this.internalClient + accessKey: pinned[INTERNAL_CONNECTION_HEADERS.accessKey], + secretKey: pinned[INTERNAL_CONNECTION_HEADERS.secretKey], + region: pinned[INTERNAL_CONNECTION_HEADERS.region] || this.defaultRegion, + maxObjectBytes: this.maxObjectBytes }); - const ensured = await service.ensureBucket(); - this.internalSessions.set(owner, service); - return ensured; + try { + const ensured = await service.ensureBucket(); + this.internalSessions.set(owner, service); + return ensured; + } catch (error) { + service.destroy(); + throw error; + } } async callInternal(method, args, context) { @@ -204,6 +228,5 @@ export class MinioConnectionRegistry { closeAll() { for (const id of [...this.connections.keys(), ...this.internalSessions.keys()]) this.closeMcpSession(id); - this.internalClient?.destroy?.(); } } diff --git a/minio-mcp/src/index.js b/minio-mcp/src/index.js index b353b31..e76febe 100644 --- a/minio-mcp/src/index.js +++ b/minio-mcp/src/index.js @@ -1,6 +1,6 @@ #!/usr/bin/env node import { startStreamableHttpServer } from "./streamable-http.js"; -import { DEFAULT_INTERNAL_BUCKET_PREFIX, INTERNAL_SCOPE_HEADER, MinioConnectionRegistry, internalScopeOf } from "./connection-registry.js"; +import { DEFAULT_INTERNAL_BUCKET_PREFIX, MinioConnectionRegistry, PINNED_HEADERS, internalScopeOf } from "./connection-registry.js"; import { INTERNAL_TOOL_DEFINITIONS, TOOL_DEFINITIONS } from "./tool-definitions.js"; function csv(value) { return (value ?? "").split(",").map((item) => item.trim()).filter(Boolean); } @@ -10,35 +10,15 @@ function integer(value, fallback, maximum = Number.MAX_SAFE_INTEGER) { if (!Number.isInteger(parsed) || parsed < 1 || parsed > maximum) throw new Error(`Invalid positive integer: ${value}`); return parsed; } -/** - * The platform's own MinIO, for sessions opened with an {@code x-minio-scope} header. Endpoint and - * keys together or not at all: a partial configuration would fail on first use, so it fails at - * start instead. The bucket prefix defaults rather than being optional - it cannot be emptied. - */ -function internalStorage() { - const values = { - endpoint: process.env.MINIO_MCP_INTERNAL_ENDPOINT || "", - accessKey: process.env.MINIO_MCP_INTERNAL_ACCESS_KEY || "", - secretKey: process.env.MINIO_MCP_INTERNAL_SECRET_KEY || "", - region: process.env.MINIO_MCP_INTERNAL_REGION || "", - bucketPrefix: process.env.MINIO_MCP_INTERNAL_BUCKET_PREFIX ?? DEFAULT_INTERNAL_BUCKET_PREFIX - }; - const required = ["endpoint", "accessKey", "secretKey"]; - const set = required.filter((name) => values[name]); - if (set.length === 0) return null; - if (set.length < required.length) { - throw new Error("MINIO_MCP_INTERNAL_ENDPOINT, MINIO_MCP_INTERNAL_ACCESS_KEY and MINIO_MCP_INTERNAL_SECRET_KEY must be set together"); - } - return values; -} - const maxObjectBytes = integer(process.env.MINIO_MCP_MAX_OBJECT_BYTES, 1024 * 1024, 32 * 1024 * 1024); const registry = new MinioConnectionRegistry({ maxConnectionsPerSession: integer(process.env.MINIO_MCP_MAX_CONNECTIONS_PER_SESSION, 8, 64), maxObjectBytes, defaultRegion: process.env.MINIO_MCP_DEFAULT_REGION || "us-east-1", allowedEndpoints: csv(process.env.MINIO_MCP_ALLOWED_ENDPOINTS), - internal: internalStorage() + // The internal storage's connection comes per session from the catalog; the prefix stays here. + // Read with ?? so an empty value reaches validation and stops the server, not falls back. + internalBucketPrefix: process.env.MINIO_MCP_INTERNAL_BUCKET_PREFIX ?? DEFAULT_INTERNAL_BUCKET_PREFIX }); // Same tool names in both modes; what differs is the connection the operation runs against. @@ -80,7 +60,7 @@ const listener = await startStreamableHttpServer({ serverVersion: "1.0.0", tools: (context) => internalScopeOf(context) !== null ? INTERNAL_TOOL_DEFINITIONS : TOOL_DEFINITIONS, callTool: auditedCall, - pinnedHeaders: [INTERNAL_SCOPE_HEADER], + pinnedHeaders: PINNED_HEADERS, // Only a session that asks for the internal storage has a bucket to prepare. onSessionInitialize: async (context) => { if (internalScopeOf(context) !== null) await registry.openInternal(context); diff --git a/minio-mcp/test/connection-registry.test.js b/minio-mcp/test/connection-registry.test.js index c9a964c..bc11d1e 100644 --- a/minio-mcp/test/connection-registry.test.js +++ b/minio-mcp/test/connection-registry.test.js @@ -103,58 +103,67 @@ test("destroys a client whose connection verification fails", async () => { assert.equal(registry.connections.size, 0); }); -const internalConfig = { - endpoint: "http://minio:9000", - accessKey: "platform-access", - secretKey: "platform-secret" +const connectionHeaders = { + "x-minio-endpoint": "http://minio:9000", + "x-minio-access-key": "platform-access", + "x-minio-secret-key": "platform-secret" }; -function internalFixture() { +function internalFixture(options = {}) { const services = []; - const sharedClient = { destroyed: false, destroy() { this.destroyed = true; } }; const registry = new MinioConnectionRegistry({ maxObjectBytes: 2048, defaultRegion: "eu-test-1", - internal: internalConfig, - clientFactory: () => sharedClient, - serviceFactory: (options) => { + ...options, + serviceFactory: (serviceOptions) => { const service = { - options, + options: serviceOptions, ensured: 0, destroyed: false, - async ensureBucket() { this.ensured += 1; return { bucket: options.bucket, created: true }; }, - async getObject(args) { return { bucket: options.bucket, ...args }; }, + async ensureBucket() { + this.ensured += 1; + if (options.failEnsure) throw new Error("MinIO connection failed"); + return { bucket: serviceOptions.bucket, created: true }; + }, + async getObject(args) { return { bucket: serviceOptions.bucket, ...args }; }, destroy() { this.destroyed = true; } }; services.push(service); return service; } }); - return { registry, services, sharedClient }; + return { registry, services }; } const executionId = "9f3d2c8c-16b1-4a89-baf8-9d45b2d67e10"; const executionBucket = `exec-${executionId}`; -function internalContext(sessionId, scope = executionId) { - return { sessionId, pinned: { "x-minio-scope": scope } }; +function internalContext(sessionId, scope = executionId, headers = connectionHeaders) { + return { sessionId, pinned: { "x-minio-scope": scope, ...headers } }; } -test("binds an internal session to its execution's bucket, behind the prefix, on the platform MinIO", async () => { - const { registry, services, sharedClient } = internalFixture(); +test("binds an internal session to its execution's bucket, behind the prefix, on the MinIO its headers name", async () => { + const { registry, services } = internalFixture(); const context = internalContext("mcp-a"); assert.deepEqual(await registry.openInternal(context), { bucket: executionBucket, created: true }); assert.equal(services[0].ensured, 1); assert.equal(services[0].options.endpoint, "http://minio:9000"); assert.equal(services[0].options.accessKey, "platform-access"); + assert.equal(services[0].options.secretKey, "platform-secret"); assert.equal(services[0].options.region, "eu-test-1"); - assert.equal(services[0].options.client, sharedClient); + assert.equal(services[0].options.client, undefined, "each session owns its client, closed with it"); assert.deepEqual( await registry.callInternal("getObject", { key: "result.txt" }, context), { bucket: executionBucket, key: "result.txt" } ); }); +test("takes the region from its header when the catalog sets one", async () => { + const { registry, services } = internalFixture(); + await registry.openInternal(internalContext("mcp-a", executionId, { ...connectionHeaders, "x-minio-region": "eu-south-1" })); + assert.equal(services[0].options.region, "eu-south-1"); +}); + test("prepares an internal session's bucket once", async () => { const { registry, services } = internalFixture(); const context = internalContext("mcp-a"); @@ -178,23 +187,41 @@ test("keeps every internal bucket behind the prefix, whatever the scope names", assert.equal(services[0].options.bucket, "exec-storage-nodes-bucket"); }); -test("uses a configured bucket prefix and refuses an empty or invalid one", () => { - const registry = new MinioConnectionRegistry({ - internal: { ...internalConfig, bucketPrefix: "hf-run-" }, - clientFactory: () => ({}) - }); - assert.equal(registry.internal.bucketPrefix, "hf-run-"); - for (const bucketPrefix of ["", "-x", "Exec-", "has.dot", "x".repeat(28)]) { - assert.throws( - () => new MinioConnectionRegistry({ internal: { ...internalConfig, bucketPrefix }, clientFactory: () => ({}) }), - /internal bucket prefix/ - ); +test("uses a configured bucket prefix and refuses an empty or invalid one", async () => { + const { registry, services } = internalFixture({ internalBucketPrefix: "hf-run-" }); + await registry.openInternal(internalContext("mcp-a")); + assert.equal(services[0].options.bucket, `hf-run-${executionId}`); + for (const internalBucketPrefix of ["", "-x", "Exec-", "has.dot", "x".repeat(28)]) { + assert.throws(() => new MinioConnectionRegistry({ internalBucketPrefix }), /internal bucket prefix/); } }); -test("refuses an internal session when no internal MinIO is configured", async () => { - const registry = new MinioConnectionRegistry({ serviceFactory: () => { throw new Error("service should not be created"); } }); - await assert.rejects(registry.openInternal(internalContext("mcp-a")), /not configured/); +test("refuses an internal session missing its connection headers", async () => { + const { registry, services } = internalFixture(); + const { "x-minio-secret-key": _secret, ...withoutSecret } = connectionHeaders; + await assert.rejects( + registry.openInternal(internalContext("mcp-a", executionId, withoutSecret)), + (error) => error.statusCode === 400 && /x-minio-secret-key/.test(error.message) + ); + await assert.rejects( + registry.openInternal(internalContext("mcp-a", executionId, { ...connectionHeaders, "x-minio-endpoint": "minio:9000" })), + (error) => error.statusCode === 400 && /x-minio-endpoint/.test(error.message) + ); + assert.equal(services.length, 0); +}); + +test("holds the internal endpoint to the server allowlist", async () => { + const { registry, services } = internalFixture({ allowedEndpoints: ["http://minio.platform:9000"] }); + await assert.rejects(registry.openInternal(internalContext("mcp-a")), /not on the server allowlist/); + await registry.openInternal(internalContext("mcp-b", executionId, { ...connectionHeaders, "x-minio-endpoint": "http://minio.platform:9000" })); + assert.equal(services.length, 1); +}); + +test("releases the client of an internal session whose bucket cannot be had", async () => { + const { registry, services } = internalFixture({ failEnsure: true }); + await assert.rejects(registry.openInternal(internalContext("mcp-a")), /connection failed/); + assert.equal(services[0].destroyed, true); + assert.equal(registry.internalSessions.size, 0); }); test("keeps internal sessions apart and offers them only object operations", async () => { @@ -204,14 +231,10 @@ test("keeps internal sessions apart and offers them only object operations", asy await assert.rejects(registry.callInternal("ensureBucket", {}, internalContext("mcp-a")), /Unsupported MinIO operation/); }); -test("releases an internal session without closing the shared client until shutdown", async () => { - const { registry, services, sharedClient } = internalFixture(); +test("closes an internal session's client with the session", async () => { + const { registry, services } = internalFixture(); await registry.openInternal(internalContext("mcp-a")); registry.closeMcpSession("mcp-a"); assert.equal(services[0].destroyed, true); - assert.equal(sharedClient.destroyed, false); assert.equal(registry.internalSessions.size, 0); - - registry.closeAll(); - assert.equal(sharedClient.destroyed, true); });