From 85e8f26ba917bd48b1ad94d93dc18d62c79b2118 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 10:04:16 +0800 Subject: [PATCH 01/15] Defer contributed tool permission checks until key authorization passes --- .../run/halo/mcpserver/McpToolRegistry.java | 2 +- .../halo/mcpserver/McpToolRegistryTest.java | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/main/java/run/halo/mcpserver/McpToolRegistry.java b/src/main/java/run/halo/mcpserver/McpToolRegistry.java index e6620ae..f35629d 100644 --- a/src/main/java/run/halo/mcpserver/McpToolRegistry.java +++ b/src/main/java/run/halo/mcpserver/McpToolRegistry.java @@ -55,7 +55,7 @@ Mono> executeIfContributed( .findFirst()) .flatMap(tool -> tool .map(value -> gateway(() -> authorization.require(name) - .then(execute(value.definition(), arguments))) + .then(Mono.defer(() -> execute(value.definition(), arguments)))) .map(Optional::of)) .orElseGet(() -> Mono.just(Optional.empty()))); } diff --git a/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java b/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java index c9c62af..ba7c543 100644 --- a/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java +++ b/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -170,6 +171,34 @@ void rejectsToolWhenKeyOrProviderPermissionDenies() { .verifyComplete(); } + @Test + void defersTheProviderPermissionCallbackUntilTheKeyAllowlistPasses() { + var permissionChecks = new AtomicInteger(); + var tool = McpToolDefinition.builder() + .name("demo/secret") + .inputSchema(objectSchema(Map.of(), List.of())) + .permission(invocation -> { + permissionChecks.incrementAndGet(); + return Mono.just(true); + }) + .handler(invocation -> Mono.just(McpToolResult.success(Map.of("ok", true)))) + .build(); + providerTools(provider, "demo", tool); + + StepVerifier.create(registry.executeIfContributed("demo/secret", Map.of()) + .contextWrite(context("demo/other"))) + .assertNext(result -> assertThat(result.orElseThrow().structuredContent().toString()) + .contains("FORBIDDEN")) + .verifyComplete(); + assertThat(permissionChecks).hasValue(0); + + StepVerifier.create(registry.executeIfContributed("demo/secret", Map.of()) + .contextWrite(context("demo/secret"))) + .assertNext(result -> assertThat(result.orElseThrow().isError()).isFalse()) + .verifyComplete(); + assertThat(permissionChecks).hasValue(1); + } + @Test void resolvesProviderListForEachRequest() { var current = new java.util.concurrent.atomic.AtomicReference(tool("demo/one")); From 6f21b6d6b99bafb757a41b8d12357278bc10b4a4 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 10:04:16 +0800 Subject: [PATCH 02/15] Rate limit MCP requests by canonical client address --- .../run/halo/mcpserver/McpIpAllowlist.java | 24 ++++++++++-- .../halo/mcpserver/McpRequestRateLimiter.java | 6 +-- .../mcpserver/McpRequestRateLimiterTest.java | 37 +++++++++++++++++++ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpIpAllowlist.java b/src/main/java/run/halo/mcpserver/McpIpAllowlist.java index 9f43198..1b06a36 100644 --- a/src/main/java/run/halo/mcpserver/McpIpAllowlist.java +++ b/src/main/java/run/halo/mcpserver/McpIpAllowlist.java @@ -37,13 +37,12 @@ static boolean allows(Set ranges, InetSocketAddress remoteAddress) { if (ranges == null || ranges.isEmpty()) { return true; } - if (remoteAddress == null) { + var resolved = resolveNumericAddress(remoteAddress); + if (resolved.isEmpty()) { return false; } try { - var address = remoteAddress.getAddress() == null - ? parseNumericAddress(remoteAddress.getHostString()) - : remoteAddress.getAddress(); + var address = resolved.get(); var matchers = ranges.stream() .map(McpIpAllowlist::compile) .toList(); @@ -58,6 +57,23 @@ static boolean allows(Set ranges, InetSocketAddress remoteAddress) { return false; } + /** + * Resolves the numeric client address for both IP authorization and rate limiting, whether + * the socket address carried a resolved InetAddress or only a numeric host string. + */ + static java.util.Optional resolveNumericAddress(InetSocketAddress remoteAddress) { + if (remoteAddress == null) { + return java.util.Optional.empty(); + } + try { + return java.util.Optional.of(remoteAddress.getAddress() == null + ? parseNumericAddress(remoteAddress.getHostString()) + : remoteAddress.getAddress()); + } catch (IllegalArgumentException error) { + return java.util.Optional.empty(); + } + } + private static CompiledRange compile(String range) { var matcher = InetAddressMatchers.fromIpAddress(range); var slashIndex = range.indexOf('/'); diff --git a/src/main/java/run/halo/mcpserver/McpRequestRateLimiter.java b/src/main/java/run/halo/mcpserver/McpRequestRateLimiter.java index 225bc95..c388ced 100644 --- a/src/main/java/run/halo/mcpserver/McpRequestRateLimiter.java +++ b/src/main/java/run/halo/mcpserver/McpRequestRateLimiter.java @@ -31,9 +31,9 @@ class McpRequestRateLimiter { } boolean allowRequest(InetSocketAddress remoteAddress) { - var source = remoteAddress == null || remoteAddress.getAddress() == null - ? "unknown" - : remoteAddress.getAddress().getHostAddress(); + var source = McpIpAllowlist.resolveNumericAddress(remoteAddress) + .map(java.net.InetAddress::getHostAddress) + .orElse("unknown"); return allow(currentWindow().requestCounts, source, REQUESTS_PER_MINUTE); } diff --git a/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java b/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java index 6cc3340..492db06 100644 --- a/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java @@ -37,4 +37,41 @@ void isolatesToolLimitsByKeyAndToolAndCanBeCleared() { limiter.clear(); assertThat(limiter.allowTool("key-one", "demo/one")).isTrue(); } + + @Test + void givesForwardedClientsIndependentBuckets() { + var limiter = new McpRequestRateLimiter(() -> 0L); + var first = InetSocketAddress.createUnresolved("203.0.113.10", 443); + var second = InetSocketAddress.createUnresolved("203.0.113.11", 443); + + for (var i = 0; i < McpRequestRateLimiter.REQUESTS_PER_MINUTE; i++) { + assertThat(limiter.allowRequest(first)).isTrue(); + } + assertThat(limiter.allowRequest(first)).isFalse(); + assertThat(limiter.allowRequest(second)).isTrue(); + } + + @Test + void canonicalizesResolvedAndUnresolvedFormsOfOneAddress() { + var limiter = new McpRequestRateLimiter(() -> 0L); + var resolved = new InetSocketAddress("203.0.113.10", 443); + var unresolved = InetSocketAddress.createUnresolved("203.0.113.10", 443); + + for (var i = 0; i < McpRequestRateLimiter.REQUESTS_PER_MINUTE; i++) { + assertThat(limiter.allowRequest(resolved)).isTrue(); + } + assertThat(limiter.allowRequest(unresolved)).isFalse(); + } + + @Test + void constrainsUnresolvableClientsToOneSharedBucket() { + var limiter = new McpRequestRateLimiter(() -> 0L); + + for (var i = 0; i < McpRequestRateLimiter.REQUESTS_PER_MINUTE; i++) { + assertThat(limiter.allowRequest(null)).isTrue(); + } + assertThat(limiter.allowRequest(InetSocketAddress.createUnresolved("not-an-ip", 443))) + .isFalse(); + assertThat(limiter.allowRequest(new InetSocketAddress("203.0.113.10", 443))).isTrue(); + } } From f38446a4b358c900bdd3d8c8bba8c5008c345bd3 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 10:04:16 +0800 Subject: [PATCH 03/15] Revalidate MCP access keys after password verification --- .../halo/mcpserver/McpAccessKeyService.java | 24 +++-- .../mcpserver/McpAccessKeyServiceTest.java | 89 +++++++++++++++++++ 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpAccessKeyService.java b/src/main/java/run/halo/mcpserver/McpAccessKeyService.java index 9d90c59..b94e435 100644 --- a/src/main/java/run/halo/mcpserver/McpAccessKeyService.java +++ b/src/main/java/run/halo/mcpserver/McpAccessKeyService.java @@ -127,14 +127,26 @@ Mono authenticate( .filter(Boolean::booleanValue) .filter(ignored -> McpIpAllowlist.allows( accessKey.getSpec().getAllowedIpRanges(), remoteAddress)) - .flatMap(ignored -> touch(accessKey).thenReturn(new McpKeyAuthenticationToken( + .flatMap(ignored -> revalidate(accessKey)) + .flatMap(fresh -> touch(fresh).thenReturn(new McpKeyAuthenticationToken( parsed.id(), - accessKey.getSpec().getDisplayName(), - accessKey.getSpec().getKeyPrefix(), - accessKey.getSpec().getOwnerName(), - accessKey.getSpec().getAllowedTools() == null + fresh.getSpec().getDisplayName(), + fresh.getSpec().getKeyPrefix(), + fresh.getSpec().getOwnerName(), + fresh.getSpec().getAllowedTools() == null ? Set.of() - : accessKey.getSpec().getAllowedTools())))); + : fresh.getSpec().getAllowedTools())))); + } + + /** + * Re-fetches the key after asynchronous password verification so a rotation, disablement, + * scope change, or deletion committed meanwhile invalidates this authentication. Status-only + * writes such as last-used updates do not affect the comparison. + */ + private Mono revalidate(McpAccessKey snapshot) { + return client.fetch(McpAccessKey.class, snapshot.getMetadata().getName()) + .filter(this::active) + .filter(fresh -> fresh.getSpec().equals(snapshot.getSpec())); } private Mono get(String id) { diff --git a/src/test/java/run/halo/mcpserver/McpAccessKeyServiceTest.java b/src/test/java/run/halo/mcpserver/McpAccessKeyServiceTest.java index db2dd17..fc2a749 100644 --- a/src/test/java/run/halo/mcpserver/McpAccessKeyServiceTest.java +++ b/src/test/java/run/halo/mcpserver/McpAccessKeyServiceTest.java @@ -130,4 +130,93 @@ void authenticatesOnlyFromAnAllowedIpRange() { .isNotNull(); assertThat(created.accessKey().getStatus().getLastUsedAt()).isNotNull(); } + + @Test + void authenticatesWhenTheKeyIsUnchangedDuringVerification() { + var created = service.create( + "Automation", "admin", Set.of("halo_search_content"), Set.of(), null) + .block(); + var id = created.accessKey().getMetadata().getName(); + when(client.fetch(McpAccessKey.class, id)) + .thenReturn(Mono.just(created.accessKey())) + .thenReturn(Mono.just(copyOf(created.accessKey()))); + + assertThat(service.authenticate(created.token(), null).block()).isNotNull(); + } + + @Test + void rejectsAuthenticationWhenRotationCommitsDuringVerification() { + var created = service.create( + "Automation", "admin", Set.of("halo_search_content"), Set.of(), null) + .block(); + var id = created.accessKey().getMetadata().getName(); + var rotated = copyOf(created.accessKey()); + rotated.getSpec().setKeyHash("rotated-hash"); + when(client.fetch(McpAccessKey.class, id)) + .thenReturn(Mono.just(created.accessKey())) + .thenReturn(Mono.just(rotated)); + + assertThat(service.authenticate(created.token(), null).block()).isNull(); + } + + @Test + void rejectsAuthenticationWhenDisablementOrScopeChangeCommitsDuringVerification() { + var created = service.create( + "Automation", "admin", Set.of("halo_search_content"), Set.of(), null) + .block(); + var id = created.accessKey().getMetadata().getName(); + + var disabled = copyOf(created.accessKey()); + disabled.getSpec().setEnabled(false); + when(client.fetch(McpAccessKey.class, id)) + .thenReturn(Mono.just(created.accessKey())) + .thenReturn(Mono.just(disabled)); + assertThat(service.authenticate(created.token(), null).block()).isNull(); + + var narrowed = copyOf(created.accessKey()); + narrowed.getSpec().setAllowedTools(Set.of()); + when(client.fetch(McpAccessKey.class, id)) + .thenReturn(Mono.just(created.accessKey())) + .thenReturn(Mono.just(narrowed)); + assertThat(service.authenticate(created.token(), null).block()).isNull(); + } + + @Test + void rejectsAuthenticationWhenDeletionCommitsDuringVerification() { + var created = service.create("Automation", "admin", Set.of(), Set.of(), null).block(); + var id = created.accessKey().getMetadata().getName(); + + when(client.fetch(McpAccessKey.class, id)) + .thenReturn(Mono.just(created.accessKey())) + .thenReturn(Mono.empty()); + assertThat(service.authenticate(created.token(), null).block()).isNull(); + + var deleting = copyOf(created.accessKey()); + deleting.getMetadata().setDeletionTimestamp(Instant.now()); + when(client.fetch(McpAccessKey.class, id)) + .thenReturn(Mono.just(created.accessKey())) + .thenReturn(Mono.just(deleting)); + assertThat(service.authenticate(created.token(), null).block()).isNull(); + } + + private static McpAccessKey copyOf(McpAccessKey accessKey) { + var copy = new McpAccessKey(); + var metadata = new run.halo.app.extension.Metadata(); + metadata.setName(accessKey.getMetadata().getName()); + metadata.setVersion(accessKey.getMetadata().getVersion()); + copy.setMetadata(metadata); + var source = accessKey.getSpec(); + var spec = new McpAccessKey.Spec(); + spec.setDisplayName(source.getDisplayName()); + spec.setKeyHash(source.getKeyHash()); + spec.setKeyPrefix(source.getKeyPrefix()); + spec.setOwnerName(source.getOwnerName()); + spec.setEnabled(source.isEnabled()); + spec.setExpiresAt(source.getExpiresAt()); + spec.setAllowedTools(new java.util.LinkedHashSet<>(source.getAllowedTools())); + spec.setAllowedIpRanges(new java.util.LinkedHashSet<>(source.getAllowedIpRanges())); + copy.setSpec(spec); + copy.setStatus(accessKey.getStatus()); + return copy; + } } From b54349ab9e0d7fa8b4e6a2018eb231db2f2fec9e Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 10:04:35 +0800 Subject: [PATCH 04/15] Enforce MCP request timeout and in-flight concurrency limits --- .../run/halo/mcpserver/HaloMcpServer.java | 2 - .../halo/mcpserver/McpInFlightLimiter.java | 60 +++++++++++++++++++ .../mcpserver/McpKeyAuthenticationFilter.java | 31 ++++++++++ .../mcpserver/McpInFlightLimiterTest.java | 58 ++++++++++++++++++ .../McpKeyAuthenticationFilterTest.java | 52 +++++++++++++++- 5 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 src/main/java/run/halo/mcpserver/McpInFlightLimiter.java create mode 100644 src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java diff --git a/src/main/java/run/halo/mcpserver/HaloMcpServer.java b/src/main/java/run/halo/mcpserver/HaloMcpServer.java index af276c6..62cdc2d 100644 --- a/src/main/java/run/halo/mcpserver/HaloMcpServer.java +++ b/src/main/java/run/halo/mcpserver/HaloMcpServer.java @@ -6,7 +6,6 @@ import io.modelcontextprotocol.server.McpStatelessAsyncServer; import io.modelcontextprotocol.server.transport.DefaultServerTransportSecurityValidator; import io.modelcontextprotocol.spec.McpSchema; -import java.time.Duration; import org.springframework.ai.mcp.server.webflux.transport.WebFluxStatelessServerTransport; import org.springframework.stereotype.Component; import org.springframework.web.reactive.function.server.RouterFunction; @@ -53,7 +52,6 @@ class HaloMcpServer { .capabilities(McpSchema.ServerCapabilities.builder() .tools(false) .build()) - .requestTimeout(Duration.ofSeconds(30)) .tools(builtInTools.specifications()) .build(); } diff --git a/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java new file mode 100644 index 0000000..b1df5f9 --- /dev/null +++ b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java @@ -0,0 +1,60 @@ +package run.halo.mcpserver; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import org.springframework.stereotype.Component; + +/** Process-local bounds on concurrent authenticated MCP request work. */ +@Component +class McpInFlightLimiter { + + static final int GLOBAL_LIMIT = 100; + static final int PER_KEY_LIMIT = 16; + private static final int MAX_TRACKED_KEYS = 10_000; + + private final AtomicInteger globalCount = new AtomicInteger(); + private final ConcurrentHashMap keyCounts = new ConcurrentHashMap<>(); + + /** + * Reserves one global and per-key slot for the key, or returns null when either budget is + * exhausted. The caller must close the permit exactly once when the request terminates. + */ + Permit tryAcquire(String keyId) { + if (keyCounts.size() >= MAX_TRACKED_KEYS && !keyCounts.containsKey(keyId)) { + return null; + } + if (globalCount.incrementAndGet() > GLOBAL_LIMIT) { + globalCount.decrementAndGet(); + return null; + } + var count = keyCounts.computeIfAbsent(keyId, ignored -> new AtomicInteger()); + if (count.incrementAndGet() > PER_KEY_LIMIT) { + count.decrementAndGet(); + globalCount.decrementAndGet(); + return null; + } + return new Permit(keyId); + } + + final class Permit implements AutoCloseable { + + private final String keyId; + private final AtomicBoolean released = new AtomicBoolean(); + + private Permit(String keyId) { + this.keyId = keyId; + } + + @Override + public void close() { + if (released.compareAndSet(false, true)) { + globalCount.decrementAndGet(); + var count = keyCounts.get(keyId); + if (count != null) { + count.decrementAndGet(); + } + } + } + } +} diff --git a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java index 7803e18..2d96602 100644 --- a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java +++ b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java @@ -20,21 +20,36 @@ class McpKeyAuthenticationFilter implements BeforeSecurityWebFilter { static final String MCP_PATH = "/mcp"; + static final java.time.Duration DEFAULT_REQUEST_TIMEOUT = java.time.Duration.ofSeconds(30); private static final String BEARER_SCHEME = "Bearer "; private final McpAccessKeyService accessKeyService; private final McpRequestRateLimiter rateLimiter; + private final McpInFlightLimiter inFlightLimiter; private final WebHandler mcpHandler; private final java.util.Set protocolVersions; + private final java.time.Duration requestTimeout; McpKeyAuthenticationFilter( McpAccessKeyService accessKeyService, McpRequestRateLimiter rateLimiter, + McpInFlightLimiter inFlightLimiter, HaloMcpServer mcpServer) { + this(accessKeyService, rateLimiter, inFlightLimiter, mcpServer, DEFAULT_REQUEST_TIMEOUT); + } + + McpKeyAuthenticationFilter( + McpAccessKeyService accessKeyService, + McpRequestRateLimiter rateLimiter, + McpInFlightLimiter inFlightLimiter, + HaloMcpServer mcpServer, + java.time.Duration requestTimeout) { this.accessKeyService = accessKeyService; this.rateLimiter = rateLimiter; + this.inFlightLimiter = inFlightLimiter; this.mcpHandler = RouterFunctions.toWebHandler(mcpServer.routerFunction()); this.protocolVersions = java.util.Set.copyOf(mcpServer.protocolVersions()); + this.requestTimeout = requestTimeout; } @Override @@ -56,10 +71,18 @@ public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { if (!hasSupportedProtocolVersion(exchange)) { return badRequest(exchange).thenReturn(true); } + var permit = inFlightLimiter.tryAcquire(authentication.keyId()); + if (permit == null) { + return tooManyRequests(exchange).thenReturn(true); + } var request = exchange.getRequest().mutate() .headers(headers -> headers.remove(AUTHORIZATION)) .build(); return mcpHandler.handle(exchange.mutate().request(request).build()) + .timeout(requestTimeout) + .onErrorResume(java.util.concurrent.TimeoutException.class, + error -> serviceUnavailable(exchange)) + .doFinally(ignored -> permit.close()) .contextWrite(org.springframework.security.core.context.ReactiveSecurityContextHolder .withAuthentication(authentication)) .thenReturn(true); @@ -104,4 +127,12 @@ private static Mono tooManyRequests(ServerWebExchange exchange) { RETRY_AFTER, String.valueOf(McpRequestRateLimiter.RETRY_AFTER_SECONDS)); return exchange.getResponse().setComplete(); } + + private static Mono serviceUnavailable(ServerWebExchange exchange) { + if (exchange.getResponse().isCommitted()) { + return Mono.empty(); + } + exchange.getResponse().setStatusCode(HttpStatus.SERVICE_UNAVAILABLE); + return exchange.getResponse().setComplete(); + } } diff --git a/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java new file mode 100644 index 0000000..9c104db --- /dev/null +++ b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java @@ -0,0 +1,58 @@ +package run.halo.mcpserver; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import org.junit.jupiter.api.Test; + +class McpInFlightLimiterTest { + + @Test + void enforcesThePerKeyLimitIndependently() { + var limiter = new McpInFlightLimiter(); + var permits = new ArrayList(); + + for (var i = 0; i < McpInFlightLimiter.PER_KEY_LIMIT; i++) { + permits.add(limiter.tryAcquire("key-one")); + } + assertThat(limiter.tryAcquire("key-one")).isNull(); + assertThat(limiter.tryAcquire("key-two")).isNotNull(); + + permits.getFirst().close(); + assertThat(limiter.tryAcquire("key-one")).isNotNull(); + } + + @Test + void enforcesTheGlobalLimitAcrossKeys() { + var limiter = new McpInFlightLimiter(); + var permits = new ArrayList(); + + var key = 0; + while (permits.size() < McpInFlightLimiter.GLOBAL_LIMIT) { + var permit = limiter.tryAcquire("key-" + key++); + assertThat(permit).isNotNull(); + permits.add(permit); + } + assertThat(limiter.tryAcquire("key-extra")).isNull(); + + permits.forEach(McpInFlightLimiter.Permit::close); + for (var i = 0; i < McpInFlightLimiter.GLOBAL_LIMIT; i++) { + assertThat(limiter.tryAcquire("key-reused-" + i)).isNotNull(); + } + } + + @Test + void ignoresDoubleRelease() { + var limiter = new McpInFlightLimiter(); + var permit = limiter.tryAcquire("key-one"); + assertThat(permit).isNotNull(); + + permit.close(); + permit.close(); + + for (var i = 0; i < McpInFlightLimiter.PER_KEY_LIMIT; i++) { + assertThat(limiter.tryAcquire("key-one")).isNotNull(); + } + assertThat(limiter.tryAcquire("key-one")).isNull(); + } +} diff --git a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java index 321c618..bd8bc47 100644 --- a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java +++ b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java @@ -56,7 +56,8 @@ void setUp() { .build()); when(mcpServer.protocolVersions()).thenReturn(java.util.List.of("2025-11-25")); rateLimiter = new McpRequestRateLimiter(); - filter = new McpKeyAuthenticationFilter(accessKeyService, rateLimiter, mcpServer); + filter = new McpKeyAuthenticationFilter( + accessKeyService, rateLimiter, new McpInFlightLimiter(), mcpServer); } @Test @@ -187,4 +188,53 @@ void rateLimitsRequestsBeforeAccessKeyLookup() { .isEqualTo("60"); verify(accessKeyService, never()).authenticate(rawToken, null); } + + @Test + void timesOutAStalledMcpRequestAndReleasesThePermit() { + org.springframework.web.reactive.function.server.HandlerFunction stalled = + request -> Mono.never(); + when(mcpServer.routerFunction()).thenReturn( + RouterFunctions.route().POST("/mcp", stalled).build()); + var inFlightLimiter = new McpInFlightLimiter(); + var fastFilter = new McpKeyAuthenticationFilter( + accessKeyService, new McpRequestRateLimiter(), inFlightLimiter, mcpServer, + java.time.Duration.ofMillis(50)); + var keyId = "00000000-0000-0000-0000-000000000000"; + var rawToken = "hmcp_" + keyId + "_secret"; + when(accessKeyService.authenticate(rawToken, null)) + .thenReturn(Mono.just(new McpKeyAuthenticationToken( + keyId, "Automation", "hmcp_00000000", "admin", Set.of()))); + var exchange = MockServerWebExchange.from(MockServerHttpRequest.post(McpKeyAuthenticationFilter.MCP_PATH) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken)); + + fastFilter.filter(exchange, ignored -> Mono.error(new AssertionError("Request must not continue"))) + .block(); + + assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + for (var i = 0; i < McpInFlightLimiter.PER_KEY_LIMIT; i++) { + assertThat(inFlightLimiter.tryAcquire(keyId)).isNotNull(); + } + } + + @Test + void rejectsRequestsWhenTheInFlightBudgetIsExhausted() { + var inFlightLimiter = new McpInFlightLimiter(); + var keyId = "00000000-0000-0000-0000-000000000000"; + for (var i = 0; i < McpInFlightLimiter.PER_KEY_LIMIT; i++) { + assertThat(inFlightLimiter.tryAcquire(keyId)).isNotNull(); + } + var rawToken = "hmcp_" + keyId + "_secret"; + when(accessKeyService.authenticate(rawToken, null)) + .thenReturn(Mono.just(new McpKeyAuthenticationToken( + keyId, "Automation", "hmcp_00000000", "admin", Set.of()))); + var exchange = MockServerWebExchange.from(MockServerHttpRequest.post(McpKeyAuthenticationFilter.MCP_PATH) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken)); + + new McpKeyAuthenticationFilter(accessKeyService, rateLimiter, inFlightLimiter, mcpServer) + .filter(exchange, ignored -> Mono.error(new AssertionError("Request must not continue"))) + .block(); + + assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.TOO_MANY_REQUESTS); + assertThat(handledPath.get()).isNull(); + } } From 594eff92436394f3aa7e59b8bd23f63a2333e68c Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 10:04:35 +0800 Subject: [PATCH 05/15] Budget in-flight attachment upload bytes --- README.md | 6 ++ .../run/halo/mcpserver/McpAuthorization.java | 14 ++++ .../halo/mcpserver/tools/AttachmentTools.java | 42 ++++++++--- .../tools/AttachmentUploadLimiter.java | 75 +++++++++++++++++++ .../mcpserver/tools/AttachmentToolsTest.java | 75 +++++++++++++++++-- .../tools/AttachmentUploadLimiterTest.java | 60 +++++++++++++++ .../mcpserver/tools/BuiltInToolsTest.java | 6 +- 7 files changed, 258 insertions(+), 20 deletions(-) create mode 100644 src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java create mode 100644 src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java diff --git a/README.md b/README.md index 153bfbf..47b85d9 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,12 @@ includes successful requests. Tool calls are additionally limited to 120 per minute for each access-key and tool pair. Limits are process-local and therefore apply independently to each Halo replica. +Authenticated request handling is cancelled after a 30-second deadline, and +concurrent authenticated requests are capped at 100 globally and 16 per access +key; requests beyond the cap receive `429 Too Many Requests`. Attachment uploads +reserve their decoded size against in-flight byte budgets of 64 MiB globally and +32 MiB per key before decoding, and excess uploads fail with `RATE_LIMITED`. + Use a dedicated, least-privilege key. Do not put keys in URLs, configuration files committed to source control, shell history, or logs. diff --git a/src/main/java/run/halo/mcpserver/McpAuthorization.java b/src/main/java/run/halo/mcpserver/McpAuthorization.java index bf4ca23..d9aa45b 100644 --- a/src/main/java/run/halo/mcpserver/McpAuthorization.java +++ b/src/main/java/run/halo/mcpserver/McpAuthorization.java @@ -32,6 +32,20 @@ public Mono authorize(String toolName, Supplier> action) { })); } + /** Runs an action with the current key's ID while keeping the authentication token internal. */ + public Mono withKeyId(java.util.function.Function> action) { + return authentication().map(McpKeyAuthenticationToken::keyId).flatMap(keyId -> { + try { + var result = action.apply(keyId); + return result == null + ? Mono.error(new McpToolException("INTERNAL", "The tool returned no result")) + : result; + } catch (Throwable error) { + return Mono.error(error); + } + }); + } + Mono authentication() { return ReactiveSecurityContextHolder.getContext() .map(SecurityContext::getAuthentication) diff --git a/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java b/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java index eca71cd..4041cfe 100644 --- a/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java +++ b/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java @@ -33,14 +33,17 @@ class AttachmentTools extends ToolSupport implements ToolGroup { private final ReactiveExtensionClient client; private final AttachmentService attachmentService; + private final AttachmentUploadLimiter uploadLimiter; AttachmentTools( ReactiveExtensionClient client, AttachmentService attachmentService, + AttachmentUploadLimiter uploadLimiter, McpAuthorization authorization) { super(authorization); this.client = client; this.attachmentService = attachmentService; + this.uploadLimiter = uploadLimiter; } @Override @@ -74,6 +77,29 @@ Mono upload(Map arguments) { if (encoded.length() > 12_000_000) { throw new McpToolException("INVALID_ARGUMENT", "contentBase64 exceeds the 8 MiB limit"); } + var mediaType = mediaType(arguments.get("mediaType")); + return authorization.withKeyId(keyId -> { + var reservation = uploadLimiter.tryAcquire(keyId, estimatedDecodedBytes(encoded)); + if (reservation == null) { + return Mono.error(new McpToolException( + "RATE_LIMITED", "Too many concurrent attachment uploads; please retry shortly")); + } + return Mono.fromCallable(() -> decode(encoded)) + .flatMap(bytes -> { + var buffer = DefaultDataBufferFactory.sharedInstance.wrap(bytes); + return attachmentService + .upload(policyName, groupName, filename, Flux.just(buffer), mediaType) + .switchIfEmpty(Mono.error(new McpToolException( + "ATTACHMENT_UNAVAILABLE", "Halo did not create the attachment"))) + .map(attachment -> payload( + ContentPayloads.attachment(attachment), + "Uploaded attachment " + filename)); + }) + .doFinally(ignored -> reservation.close()); + }); + } + + private static byte[] decode(String encoded) { final byte[] bytes; try { bytes = Base64.getDecoder().decode(encoded); @@ -84,17 +110,11 @@ Mono upload(Map arguments) { throw new McpToolException( "INVALID_ARGUMENT", "Attachment content must be between 1 byte and 8 MiB"); } - var buffer = DefaultDataBufferFactory.sharedInstance.wrap(bytes); - return attachmentService - .upload( - policyName, - groupName, - filename, - Flux.just(buffer), - mediaType(arguments.get("mediaType"))) - .switchIfEmpty(Mono.error(new McpToolException( - "ATTACHMENT_UNAVAILABLE", "Halo did not create the attachment"))) - .map(attachment -> payload(ContentPayloads.attachment(attachment), "Uploaded attachment " + filename)); + return bytes; + } + + private static long estimatedDecodedBytes(String encoded) { + return encoded.length() / 4L * 3L + 3L; } Mono delete(Map arguments) { diff --git a/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java new file mode 100644 index 0000000..d88d5ef --- /dev/null +++ b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java @@ -0,0 +1,75 @@ +package run.halo.mcpserver.tools; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import org.springframework.stereotype.Component; + +/** Process-local in-flight byte budgets that bound concurrent attachment uploads. */ +@Component +class AttachmentUploadLimiter { + + static final long GLOBAL_BUDGET_BYTES = 64L * 1024 * 1024; + static final long PER_KEY_BUDGET_BYTES = 32L * 1024 * 1024; + private static final int MAX_TRACKED_KEYS = 10_000; + + private final AtomicLong globalBytes = new AtomicLong(); + private final ConcurrentHashMap keyBytes = new ConcurrentHashMap<>(); + + /** + * Reserves decoded bytes against the global and per-key budgets, or returns null when either + * is exhausted. The caller must close the reservation exactly once when the upload terminates. + */ + Reservation tryAcquire(String keyId, long bytes) { + if (bytes > GLOBAL_BUDGET_BYTES) { + return null; + } + if (keyBytes.size() >= MAX_TRACKED_KEYS && !keyBytes.containsKey(keyId)) { + return null; + } + if (!reserve(globalBytes, GLOBAL_BUDGET_BYTES, bytes)) { + return null; + } + var usage = keyBytes.computeIfAbsent(keyId, ignored -> new AtomicLong()); + if (!reserve(usage, PER_KEY_BUDGET_BYTES, bytes)) { + globalBytes.addAndGet(-bytes); + return null; + } + return new Reservation(keyId, bytes); + } + + private static boolean reserve(AtomicLong usage, long limit, long bytes) { + while (true) { + var current = usage.get(); + if (current + bytes > limit) { + return false; + } + if (usage.compareAndSet(current, current + bytes)) { + return true; + } + } + } + + final class Reservation implements AutoCloseable { + + private final String keyId; + private final long bytes; + private final AtomicBoolean released = new AtomicBoolean(); + + private Reservation(String keyId, long bytes) { + this.keyId = keyId; + this.bytes = bytes; + } + + @Override + public void close() { + if (released.compareAndSet(false, true)) { + globalBytes.addAndGet(-bytes); + var usage = keyBytes.get(keyId); + if (usage != null) { + usage.addAndGet(-bytes); + } + } + } + } +} diff --git a/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java b/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java index ee72f11..0412554 100644 --- a/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java +++ b/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java @@ -1,11 +1,13 @@ package run.halo.mcpserver.tools; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -33,14 +35,67 @@ class AttachmentToolsTest { @Test void validatesAttachmentBase64BeforeUpload() { - var tools = new AttachmentTools(client, attachmentService, authorization); + var tools = new AttachmentTools( + client, attachmentService, new AttachmentUploadLimiter(), authorization); + stubKeyId("key-one"); - assertThatThrownBy(() -> tools.upload(Map.of( + StepVerifier.create(tools.upload(Map.of( "filename", "a.txt", "policyName", "local", "contentBase64", "not-base64"))) - .isInstanceOf(McpToolException.class) - .hasMessageContaining("valid Base64"); + .expectErrorSatisfies(error -> assertThat(error) + .isInstanceOf(McpToolException.class) + .hasMessageContaining("valid Base64")) + .verify(); + verify(attachmentService, never()).upload(any(), any(), any(), any(reactor.core.publisher.Flux.class), any(org.springframework.http.MediaType.class)); + } + + @Test + void rejectsUploadsWhenTheInFlightByteBudgetIsExhausted() { + var uploadLimiter = new AttachmentUploadLimiter(); + var reservation = uploadLimiter.tryAcquire( + "key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES); + var tools = new AttachmentTools(client, attachmentService, uploadLimiter, authorization); + stubKeyId("key-one"); + var arguments = Map.of( + "filename", "a.txt", + "policyName", "local", + "contentBase64", Base64.getEncoder().encodeToString("hello".getBytes(StandardCharsets.UTF_8))); + + StepVerifier.create(tools.upload(arguments)) + .expectErrorSatisfies(error -> assertThat(error) + .isInstanceOf(McpToolException.class) + .hasMessageContaining("concurrent attachment uploads")) + .verify(); + verify(attachmentService, never()).upload(any(), any(), any(), any(reactor.core.publisher.Flux.class), any(org.springframework.http.MediaType.class)); + + reservation.close(); + var attachment = new Attachment(); + attachment.setMetadata(ToolSupport.metadata("a.txt")); + when(attachmentService.upload(any(), any(), any(), any(reactor.core.publisher.Flux.class), any(org.springframework.http.MediaType.class))) + .thenReturn(Mono.just(attachment)); + StepVerifier.create(tools.upload(arguments)) + .assertNext(payload -> assertThat(payload.summary()).contains("a.txt")) + .verifyComplete(); + } + + @Test + void releasesTheByteBudgetWhenTheUploadIsCancelled() { + var uploadLimiter = new AttachmentUploadLimiter(); + var tools = new AttachmentTools(client, attachmentService, uploadLimiter, authorization); + stubKeyId("key-one"); + when(attachmentService.upload(any(), any(), any(), any(reactor.core.publisher.Flux.class), any(org.springframework.http.MediaType.class))).thenReturn(Mono.never()); + + StepVerifier.create(tools.upload(Map.of( + "filename", "a.txt", + "policyName", "local", + "contentBase64", + Base64.getEncoder().encodeToString("hello".getBytes(StandardCharsets.UTF_8))))) + .thenCancel() + .verify(); + + assertThat(uploadLimiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) + .isNotNull(); } @Test @@ -50,7 +105,8 @@ void deletionUsesExtensionLifecycleSoReconcilerCleansStorage() { attachment.getMetadata().setVersion(1L); when(client.fetch(Attachment.class, "attachment-one")).thenReturn(Mono.just(attachment)); when(client.delete(attachment)).thenReturn(Mono.just(attachment)); - var tools = new AttachmentTools(client, attachmentService, authorization); + var tools = new AttachmentTools( + client, attachmentService, new AttachmentUploadLimiter(), authorization); StepVerifier.create(tools.delete(Map.of("name", "attachment-one"))) .assertNext(payload -> assertThat(payload.summary()).contains("attachment-one")) @@ -59,4 +115,11 @@ void deletionUsesExtensionLifecycleSoReconcilerCleansStorage() { verify(client).delete(attachment); verify(attachmentService, never()).delete(attachment); } + + private void stubKeyId(String keyId) { + when(authorization.withKeyId(any())).thenAnswer(invocation -> { + java.util.function.Function> action = invocation.getArgument(0); + return action.apply(keyId); + }); + } } diff --git a/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java new file mode 100644 index 0000000..578a322 --- /dev/null +++ b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java @@ -0,0 +1,60 @@ +package run.halo.mcpserver.tools; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import org.junit.jupiter.api.Test; + +class AttachmentUploadLimiterTest { + + private static final long EIGHT_MIB = 8L * 1024 * 1024; + + @Test + void enforcesThePerKeyBudgetIndependently() { + var limiter = new AttachmentUploadLimiter(); + var reservations = new ArrayList(); + + for (var i = 0; i < AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES / EIGHT_MIB; i++) { + reservations.add(limiter.tryAcquire("key-one", EIGHT_MIB)); + } + assertThat(limiter.tryAcquire("key-one", EIGHT_MIB)).isNull(); + assertThat(limiter.tryAcquire("key-two", EIGHT_MIB)).isNotNull(); + + reservations.getFirst().close(); + assertThat(limiter.tryAcquire("key-one", EIGHT_MIB)).isNotNull(); + } + + @Test + void enforcesTheGlobalBudgetAcrossKeys() { + var limiter = new AttachmentUploadLimiter(); + + assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) + .isNotNull(); + assertThat(limiter.tryAcquire("key-two", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) + .isNotNull(); + assertThat(limiter.tryAcquire("key-three", EIGHT_MIB)).isNull(); + } + + @Test + void rejectsASingleReservationLargerThanTheGlobalBudget() { + var limiter = new AttachmentUploadLimiter(); + + assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.GLOBAL_BUDGET_BYTES + 1)) + .isNull(); + assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) + .isNotNull(); + } + + @Test + void ignoresDoubleRelease() { + var limiter = new AttachmentUploadLimiter(); + var reservation = limiter.tryAcquire("key-one", EIGHT_MIB); + assertThat(reservation).isNotNull(); + + reservation.close(); + reservation.close(); + + assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) + .isNotNull(); + } +} diff --git a/src/test/java/run/halo/mcpserver/tools/BuiltInToolsTest.java b/src/test/java/run/halo/mcpserver/tools/BuiltInToolsTest.java index 2e602ab..8e91257 100644 --- a/src/test/java/run/halo/mcpserver/tools/BuiltInToolsTest.java +++ b/src/test/java/run/halo/mcpserver/tools/BuiltInToolsTest.java @@ -27,7 +27,7 @@ void organizesToolsByHaloDomainAndUsesChineseConsoleDescriptions() { new CategoryTools(client, authorization), new TagTools(client, authorization), new CommentTools(client, authorization), - new AttachmentTools(client, mock(AttachmentService.class), authorization)); + new AttachmentTools(client, mock(AttachmentService.class), new AttachmentUploadLimiter(), authorization)); var names = tools.names().toList(); assertThat(tools.tools()).hasSize(29); @@ -70,7 +70,7 @@ void marksOnlyRecycleAndDeleteToolsAsDestructive() { new CategoryTools(client, authorization), new TagTools(client, authorization), new CommentTools(client, authorization), - new AttachmentTools(client, mock(AttachmentService.class), authorization)); + new AttachmentTools(client, mock(AttachmentService.class), new AttachmentUploadLimiter(), authorization)); var destructive = tools.tools().stream() .filter(tool -> Boolean.TRUE.equals(tool.protocolTool().annotations().destructiveHint())) @@ -98,7 +98,7 @@ void publishesDetailedOutputObjectSchemas() { new CategoryTools(client, authorization), new TagTools(client, authorization), new CommentTools(client, authorization), - new AttachmentTools(client, mock(AttachmentService.class), authorization)); + new AttachmentTools(client, mock(AttachmentService.class), new AttachmentUploadLimiter(), authorization)); var schemaValidator = new DefaultJsonSchemaValidator(); assertThat(tools.tools()).allSatisfy(tool -> { From 0c5a1106df18694ddb2aafaa61f3f7e4b802c7b3 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 10:04:35 +0800 Subject: [PATCH 06/15] Keep MCP access tokens out of client connection guides --- ui/src/components/AccessKeySecretModal.vue | 2 +- ui/src/components/McpConnectionGuide.vue | 14 ++-- ui/src/utils/__tests__/mcp-config.test.ts | 48 +++++++++++-- ui/src/utils/mcp-config.ts | 81 ++++++++++------------ 4 files changed, 88 insertions(+), 57 deletions(-) diff --git a/ui/src/components/AccessKeySecretModal.vue b/ui/src/components/AccessKeySecretModal.vue index 709dfab..3cefcf0 100644 --- a/ui/src/components/AccessKeySecretModal.vue +++ b/ui/src/components/AccessKeySecretModal.vue @@ -32,7 +32,7 @@ const { copy, copied } = useClipboard({
接入方式
- +
diff --git a/ui/src/utils/__tests__/mcp-config.test.ts b/ui/src/utils/__tests__/mcp-config.test.ts index ac6c4e0..4ac4d1f 100644 --- a/ui/src/utils/__tests__/mcp-config.test.ts +++ b/ui/src/utils/__tests__/mcp-config.test.ts @@ -3,10 +3,50 @@ import { describe, expect, it } from 'vitest' import { mcpClientGuides } from '../mcp-config' describe('mcpClientGuides', () => { - it('writes the Codex token directly to a static authorization header', () => { - const codex = mcpClientGuides('hmcp_secret').find((guide) => guide.id === 'codex') + it('embeds no bearer material in any guide content or install URL', () => { + for (const guide of mcpClientGuides()) { + expect(guide.content).not.toContain('hmcp_') + expect(guide.content).not.toMatch(/Bearer (?!\$)/) + const installPayload = decodeURIComponent(guide.installUrl ?? '') + expect(installPayload).not.toContain('hmcp_') + expect(installPayload).not.toMatch(/Bearer (?!\$)/) + } + }) + + it('references the token from the environment for CLI and file-based clients', () => { + const guides = mcpClientGuides() + + const claudeCode = guides.find((guide) => guide.id === 'claude-code') + expect(claudeCode?.content).toContain("--header 'Authorization: Bearer ${HALO_MCP_TOKEN}'") + + const codex = guides.find((guide) => guide.id === 'codex') + expect(codex?.content).toContain('bearer_token_env_var = "HALO_MCP_TOKEN"') + expect(codex?.content).not.toContain('http_headers') + + const cursor = guides.find((guide) => guide.id === 'cursor') + expect(cursor?.content).toContain('Bearer ${env:HALO_MCP_TOKEN}') + }) + + it('keeps only endpoint metadata and an env reference in the Cursor install URL', () => { + const cursor = mcpClientGuides().find((guide) => guide.id === 'cursor') + + const config = JSON.parse(atob(cursor!.installUrl!.split('config=')[1]!)) + expect(config.type).toBe('http') + expect(config.url).toMatch(/\/mcp$/) + expect(config.headers.Authorization).toBe('Bearer ${env:HALO_MCP_TOKEN}') + }) + + it('lets VS Code prompt for the token through secure input storage', () => { + const vscode = mcpClientGuides().find((guide) => guide.id === 'vscode') + + expect(vscode?.content).toContain('${input:halo-mcp-token}') + expect(vscode?.content).toContain('"password": true') - expect(codex?.content).toContain('http_headers = { Authorization = "Bearer hmcp_secret" }') - expect(codex?.content).not.toContain('bearer_token_env_var') + const payload = JSON.parse( + decodeURIComponent(vscode!.installUrl!.slice('vscode:mcp/install?'.length)), + ) + expect(payload.headers.Authorization).toBe('Bearer ${input:halo-mcp-token}') + expect(payload.inputs).toHaveLength(1) + expect(payload.inputs[0]).toMatchObject({ type: 'promptString', password: true }) }) }) diff --git a/ui/src/utils/mcp-config.ts b/ui/src/utils/mcp-config.ts index 5347802..d0a6888 100644 --- a/ui/src/utils/mcp-config.ts +++ b/ui/src/utils/mcp-config.ts @@ -3,25 +3,8 @@ export function mcpEndpoint() { } const SERVER_NAME = 'halo' -const TOKEN_PLACEHOLDER = '$HALO_MCP_TOKEN' - -export function mcpHttpConfig(token = TOKEN_PLACEHOLDER) { - return JSON.stringify( - { - mcpServers: { - [SERVER_NAME]: { - type: 'http', - url: mcpEndpoint(), - headers: { - Authorization: `Bearer ${token}`, - }, - }, - }, - }, - null, - 2, - ) -} +const TOKEN_ENV_VAR = 'HALO_MCP_TOKEN' +const VSCODE_INPUT_ID = 'halo-mcp-token' export type McpClientId = 'claude-code' | 'codex' | 'cursor' | 'vscode' @@ -32,20 +15,39 @@ export interface McpClientGuide { installUrl?: string } -export function mcpClientGuides(token?: string): McpClientGuide[] { +/** + * Guides never receive the plaintext token: CLI and file-based clients read it from the + * HALO_MCP_TOKEN environment variable at runtime, and VS Code prompts for it through its + * secure input storage on first start. + */ +export function mcpClientGuides(): McpClientGuide[] { const endpoint = mcpEndpoint() - const bearer = `Bearer ${token ?? TOKEN_PLACEHOLDER}` - const serverConfig = { + + const cursorServer = { type: 'http', url: endpoint, - headers: { Authorization: bearer }, + headers: { Authorization: `Bearer \${env:${TOKEN_ENV_VAR}}` }, } - const guides: McpClientGuide[] = [ + const vscodeInputs = [ + { + type: 'promptString', + id: VSCODE_INPUT_ID, + description: 'Halo MCP 密钥', + password: true, + }, + ] + const vscodeServer = { + type: 'http', + url: endpoint, + headers: { Authorization: `Bearer \${input:${VSCODE_INPUT_ID}}` }, + } + + return [ { id: 'claude-code', label: 'Claude Code', - content: `claude mcp add --transport http ${SERVER_NAME} ${endpoint} --header "Authorization: ${bearer}"`, + content: `claude mcp add --transport http ${SERVER_NAME} ${endpoint} --header 'Authorization: Bearer \${${TOKEN_ENV_VAR}}'`, }, { id: 'codex', @@ -53,32 +55,25 @@ export function mcpClientGuides(token?: string): McpClientGuide[] { content: `# ~/.codex/config.toml [mcp_servers.${SERVER_NAME}] url = "${endpoint}" -http_headers = { Authorization = "${bearer}" }`, +bearer_token_env_var = "${TOKEN_ENV_VAR}"`, }, { id: 'cursor', label: 'Cursor', - content: mcpHttpConfig(token), + content: JSON.stringify({ mcpServers: { [SERVER_NAME]: cursorServer } }, null, 2), + installUrl: `cursor://anysphere.cursor-deeplink/mcp/install?name=${SERVER_NAME}&config=${window.btoa(JSON.stringify(cursorServer))}`, }, { id: 'vscode', label: 'VS Code', - content: JSON.stringify({ servers: { [SERVER_NAME]: serverConfig } }, null, 2), + content: JSON.stringify( + { inputs: vscodeInputs, servers: { [SERVER_NAME]: vscodeServer } }, + null, + 2, + ), + installUrl: `vscode:mcp/install?${encodeURIComponent( + JSON.stringify({ name: SERVER_NAME, ...vscodeServer, inputs: vscodeInputs }), + )}`, }, ] - - if (token) { - for (const guide of guides) { - if (guide.id === 'cursor') { - const config = window.btoa(JSON.stringify(serverConfig)) - guide.installUrl = `cursor://anysphere.cursor-deeplink/mcp/install?name=${SERVER_NAME}&config=${config}` - } - if (guide.id === 'vscode') { - const config = encodeURIComponent(JSON.stringify({ name: SERVER_NAME, ...serverConfig })) - guide.installUrl = `vscode:mcp/install?${config}` - } - } - } - - return guides } From d4eaafdd03d775cb2e3b69d73bdfea564eed58e5 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 11:10:11 +0800 Subject: [PATCH 07/15] Parse MCP client addresses without DNS resolution resolveNumericAddress fed unresolved host strings into InetAddress.getByName, which falls back to a blocking DNS lookup on the event loop for values that slip past Spring's numeric heuristic (e.g. hex-digit + colon prefixes from a client-controlled Forwarded header). Parse strict numeric literals with Guava's InetAddresses.forString instead so non-literals resolve to empty without ever touching DNS. --- .../java/run/halo/mcpserver/McpIpAllowlist.java | 13 ++++++------- .../java/run/halo/mcpserver/McpIpAllowlistTest.java | 8 ++++++++ .../halo/mcpserver/McpRequestRateLimiterTest.java | 11 +++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpIpAllowlist.java b/src/main/java/run/halo/mcpserver/McpIpAllowlist.java index 1b06a36..1f9c5d4 100644 --- a/src/main/java/run/halo/mcpserver/McpIpAllowlist.java +++ b/src/main/java/run/halo/mcpserver/McpIpAllowlist.java @@ -2,9 +2,9 @@ import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.UnknownHostException; import java.util.LinkedHashSet; import java.util.Set; +import com.google.common.net.InetAddresses; import org.springframework.security.util.matcher.InetAddressMatcher; import org.springframework.security.util.matcher.InetAddressMatchers; import org.springframework.util.StringUtils; @@ -85,16 +85,15 @@ private static CompiledRange compile(String range) { return new CompiledRange(addressLength, matcher); } + /** + * Parses a strict numeric IP literal without ever consulting DNS; anything else is rejected + * with an IllegalArgumentException. + */ private static InetAddress parseNumericAddress(String address) { var value = address.startsWith("[") && address.endsWith("]") ? address.substring(1, address.length() - 1) : address; - InetAddressMatchers.fromIpAddress(value); - try { - return InetAddress.getByName(value); - } catch (UnknownHostException error) { - throw new IllegalArgumentException(error); - } + return InetAddresses.forString(value); } private static void validateMask(String mask, int maxBits) { diff --git a/src/test/java/run/halo/mcpserver/McpIpAllowlistTest.java b/src/test/java/run/halo/mcpserver/McpIpAllowlistTest.java index 78d9006..1f73dea 100644 --- a/src/test/java/run/halo/mcpserver/McpIpAllowlistTest.java +++ b/src/test/java/run/halo/mcpserver/McpIpAllowlistTest.java @@ -102,6 +102,14 @@ void allowsUnknownAddressesWhenNotConfigured() { assertThat(McpIpAllowlist.allows(Set.of(), null)).isTrue(); } + @Test + void rejectsHostnamesDisguisedWithHexPrefixesAndColons() { + var disguised = InetSocketAddress.createUnresolved("1:a.attacker.example", 443); + assertThat(McpIpAllowlist.resolveNumericAddress(disguised)).isEmpty(); + assertThat(McpIpAllowlist.allows(Set.of("0.0.0.0/0"), disguised)).isFalse(); + assertThat(McpIpAllowlist.allows(Set.of(), disguised)).isTrue(); + } + @Test void failsClosedWhenStoredConfigurationIsInvalid() { var ranges = new LinkedHashSet<>(java.util.List.of("203.0.113.10", "invalid")); diff --git a/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java b/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java index 492db06..601982e 100644 --- a/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/McpRequestRateLimiterTest.java @@ -74,4 +74,15 @@ void constrainsUnresolvableClientsToOneSharedBucket() { .isFalse(); assertThat(limiter.allowRequest(new InetSocketAddress("203.0.113.10", 443))).isTrue(); } + + @Test + void bucketsHostnamesAsUnknownWithoutResolvingThem() { + var limiter = new McpRequestRateLimiter(() -> 0L); + var disguised = InetSocketAddress.createUnresolved("1:a.attacker.example", 443); + + for (var i = 0; i < McpRequestRateLimiter.REQUESTS_PER_MINUTE; i++) { + assertThat(limiter.allowRequest(disguised)).isTrue(); + } + assertThat(limiter.allowRequest(null)).isFalse(); + } } From 3646e19290e145fb64d9e1955e0f2e0f4c044750 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 11:10:43 +0800 Subject: [PATCH 08/15] Reserve exact decoded bytes for attachment uploads The length/4*3+3 estimate overstates every upload by up to 3 bytes, so four concurrent 8 MiB uploads summed past the 32 MiB per-key budget and the fourth was rejected even though the exact content fits. Derive the exact decoded length from the Base64 padding instead, clamped at zero for malformed input, and cover the boundary with a test admitting four exact 8 MiB uploads while rejecting a fifth. --- .../halo/mcpserver/tools/AttachmentTools.java | 17 ++++++++-- .../mcpserver/tools/AttachmentToolsTest.java | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java b/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java index 4041cfe..df443f9 100644 --- a/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java +++ b/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java @@ -79,7 +79,7 @@ Mono upload(Map arguments) { } var mediaType = mediaType(arguments.get("mediaType")); return authorization.withKeyId(keyId -> { - var reservation = uploadLimiter.tryAcquire(keyId, estimatedDecodedBytes(encoded)); + var reservation = uploadLimiter.tryAcquire(keyId, decodedLength(encoded)); if (reservation == null) { return Mono.error(new McpToolException( "RATE_LIMITED", "Too many concurrent attachment uploads; please retry shortly")); @@ -113,8 +113,19 @@ private static byte[] decode(String encoded) { return bytes; } - private static long estimatedDecodedBytes(String encoded) { - return encoded.length() / 4L * 3L + 3L; + /** + * Returns the exact length valid Base64 decodes to. Invalid input may misestimate but never + * below zero; the content is still validated when it is actually decoded. + */ + private static long decodedLength(String encoded) { + var length = encoded.length(); + var padding = encoded.endsWith("==") ? 2 : encoded.endsWith("=") ? 1 : 0; + var decoded = switch (length % 4) { + case 2 -> length / 4L * 3L + 1L; + case 3 -> length / 4L * 3L + 2L; + default -> length / 4L * 3L - padding; + }; + return Math.max(decoded, 0L); } Mono delete(Map arguments) { diff --git a/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java b/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java index 0412554..8f705c3 100644 --- a/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java +++ b/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java @@ -98,6 +98,39 @@ void releasesTheByteBudgetWhenTheUploadIsCancelled() { .isNotNull(); } + @Test + void admitsConcurrentUploadsUpToTheExactPerKeyByteBudget() { + var uploadLimiter = new AttachmentUploadLimiter(); + var tools = new AttachmentTools(client, attachmentService, uploadLimiter, authorization); + stubKeyId("key-one"); + when(attachmentService.upload(any(), any(), any(), any(reactor.core.publisher.Flux.class), any(org.springframework.http.MediaType.class))) + .thenReturn(Mono.never()); + var arguments = Map.of( + "filename", "a.bin", + "policyName", "local", + "contentBase64", Base64.getEncoder().encodeToString(new byte[8 * 1024 * 1024])); + + var errors = new java.util.concurrent.CopyOnWriteArrayList(); + var uploads = new java.util.ArrayList(); + for (var i = 0; i < 4; i++) { + uploads.add(tools.upload(arguments).subscribe(payload -> {}, errors::add)); + } + + // Four times 8 MiB is exactly the per-key budget: all four must be admitted. + assertThat(errors).isEmpty(); + assertThat(uploads).allSatisfy(upload -> assertThat(upload.isDisposed()).isFalse()); + + StepVerifier.create(tools.upload(arguments)) + .expectErrorSatisfies(error -> assertThat(error) + .isInstanceOf(McpToolException.class) + .hasMessageContaining("concurrent attachment uploads")) + .verify(); + + uploads.forEach(reactor.core.Disposable::dispose); + assertThat(uploadLimiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) + .isNotNull(); + } + @Test void deletionUsesExtensionLifecycleSoReconcilerCleansStorage() { var attachment = new Attachment(); From 1d9030b6730cde76a21af4e210afc7a725037516 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 11:10:58 +0800 Subject: [PATCH 09/15] Forget limiter keys whose counts return to zero Both limiters kept their per-key map entries forever, so after 10,000 distinct keys every new key was permanently rejected until restart. Remove the entry when a release brings a key's count back to zero so long-lived processes recover from key churn. --- .../java/run/halo/mcpserver/McpInFlightLimiter.java | 10 +++++----- .../mcpserver/tools/AttachmentUploadLimiter.java | 10 +++++----- .../run/halo/mcpserver/McpInFlightLimiterTest.java | 13 +++++++++++++ .../tools/AttachmentUploadLimiterTest.java | 13 +++++++++++++ 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java index b1df5f9..ad3c7c5 100644 --- a/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java +++ b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java @@ -11,7 +11,7 @@ class McpInFlightLimiter { static final int GLOBAL_LIMIT = 100; static final int PER_KEY_LIMIT = 16; - private static final int MAX_TRACKED_KEYS = 10_000; + static final int MAX_TRACKED_KEYS = 10_000; private final AtomicInteger globalCount = new AtomicInteger(); private final ConcurrentHashMap keyCounts = new ConcurrentHashMap<>(); @@ -50,10 +50,10 @@ private Permit(String keyId) { public void close() { if (released.compareAndSet(false, true)) { globalCount.decrementAndGet(); - var count = keyCounts.get(keyId); - if (count != null) { - count.decrementAndGet(); - } + // Drop the entry once the key holds no permits so churn through many keys + // cannot permanently exhaust MAX_TRACKED_KEYS. + keyCounts.computeIfPresent( + keyId, (key, count) -> count.decrementAndGet() <= 0 ? null : count); } } } diff --git a/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java index d88d5ef..8187c46 100644 --- a/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java +++ b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java @@ -11,7 +11,7 @@ class AttachmentUploadLimiter { static final long GLOBAL_BUDGET_BYTES = 64L * 1024 * 1024; static final long PER_KEY_BUDGET_BYTES = 32L * 1024 * 1024; - private static final int MAX_TRACKED_KEYS = 10_000; + static final int MAX_TRACKED_KEYS = 10_000; private final AtomicLong globalBytes = new AtomicLong(); private final ConcurrentHashMap keyBytes = new ConcurrentHashMap<>(); @@ -65,10 +65,10 @@ private Reservation(String keyId, long bytes) { public void close() { if (released.compareAndSet(false, true)) { globalBytes.addAndGet(-bytes); - var usage = keyBytes.get(keyId); - if (usage != null) { - usage.addAndGet(-bytes); - } + // Drop the entry once the key holds no reservation so churn through many keys + // cannot permanently exhaust MAX_TRACKED_KEYS. + keyBytes.computeIfPresent( + keyId, (key, usage) -> usage.addAndGet(-bytes) <= 0 ? null : usage); } } } diff --git a/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java index 9c104db..01558ef 100644 --- a/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java @@ -41,6 +41,19 @@ void enforcesTheGlobalLimitAcrossKeys() { } } + @Test + void forgetsKeysWhosePermitsHaveAllBeenReleased() { + var limiter = new McpInFlightLimiter(); + + for (var i = 0; i < McpInFlightLimiter.MAX_TRACKED_KEYS + 1; i++) { + var permit = limiter.tryAcquire("key-" + i); + assertThat(permit).isNotNull(); + permit.close(); + } + + assertThat(limiter.tryAcquire("key-fresh")).isNotNull(); + } + @Test void ignoresDoubleRelease() { var limiter = new McpInFlightLimiter(); diff --git a/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java index 578a322..b9b2fb3 100644 --- a/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java @@ -45,6 +45,19 @@ void rejectsASingleReservationLargerThanTheGlobalBudget() { .isNotNull(); } + @Test + void forgetsKeysWhoseReservationsHaveAllBeenReleased() { + var limiter = new AttachmentUploadLimiter(); + + for (var i = 0; i < AttachmentUploadLimiter.MAX_TRACKED_KEYS + 1; i++) { + var reservation = limiter.tryAcquire("key-" + i, 1); + assertThat(reservation).isNotNull(); + reservation.close(); + } + + assertThat(limiter.tryAcquire("key-fresh", EIGHT_MIB)).isNotNull(); + } + @Test void ignoresDoubleRelease() { var limiter = new AttachmentUploadLimiter(); From 330952a6b8127f1167b8aea368311d70cb45dea5 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 11:11:21 +0800 Subject: [PATCH 10/15] Release in-flight permits when handler assembly fails mcpHandler.handle was invoked synchronously while assembling the inner chain, so a synchronous throw would escape before doFinally attached and leak the in-flight permit until the key's budget was exhausted. Defer the invocation so assembly failures flow through the error path. Also document that access-key revalidation relies on the extension client returning freshly deserialized instances. --- .../halo/mcpserver/McpAccessKeyService.java | 4 ++- .../mcpserver/McpKeyAuthenticationFilter.java | 4 ++- .../McpKeyAuthenticationFilterTest.java | 30 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpAccessKeyService.java b/src/main/java/run/halo/mcpserver/McpAccessKeyService.java index b94e435..cd87446 100644 --- a/src/main/java/run/halo/mcpserver/McpAccessKeyService.java +++ b/src/main/java/run/halo/mcpserver/McpAccessKeyService.java @@ -141,7 +141,9 @@ Mono authenticate( /** * Re-fetches the key after asynchronous password verification so a rotation, disablement, * scope change, or deletion committed meanwhile invalidates this authentication. Status-only - * writes such as last-used updates do not affect the comparison. + * writes such as last-used updates do not affect the comparison. This relies on the extension + * client returning a freshly deserialized instance per fetch; a shared mutable instance would + * make the spec comparison trivially equal and silently defeat this check. */ private Mono revalidate(McpAccessKey snapshot) { return client.fetch(McpAccessKey.class, snapshot.getMetadata().getName()) diff --git a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java index 2d96602..7cce07c 100644 --- a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java +++ b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java @@ -78,7 +78,9 @@ public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { var request = exchange.getRequest().mutate() .headers(headers -> headers.remove(AUTHORIZATION)) .build(); - return mcpHandler.handle(exchange.mutate().request(request).build()) + // Defer so a synchronously throwing handler assembly still flows through + // the error path and releases the permit. + return Mono.defer(() -> mcpHandler.handle(exchange.mutate().request(request).build())) .timeout(requestTimeout) .onErrorResume(java.util.concurrent.TimeoutException.class, error -> serviceUnavailable(exchange)) diff --git a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java index bd8bc47..9955776 100644 --- a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java +++ b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java @@ -1,6 +1,7 @@ package run.halo.mcpserver; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -237,4 +238,33 @@ void rejectsRequestsWhenTheInFlightBudgetIsExhausted() { assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.TOO_MANY_REQUESTS); assertThat(handledPath.get()).isNull(); } + + @Test + void releasesThePermitWhenTheHandlerAssemblyFailsSynchronously() { + org.springframework.web.reactive.function.server.RouterFunction broken = + request -> { + throw new IllegalStateException("boom"); + }; + when(mcpServer.routerFunction()).thenReturn(broken); + var inFlightLimiter = new McpInFlightLimiter(); + var brokenFilter = new McpKeyAuthenticationFilter( + accessKeyService, new McpRequestRateLimiter(), inFlightLimiter, mcpServer); + var keyId = "00000000-0000-0000-0000-000000000000"; + var rawToken = "hmcp_" + keyId + "_secret"; + when(accessKeyService.authenticate(rawToken, null)) + .thenReturn(Mono.just(new McpKeyAuthenticationToken( + keyId, "Automation", "hmcp_00000000", "admin", Set.of()))); + var exchange = MockServerWebExchange.from(MockServerHttpRequest.post(McpKeyAuthenticationFilter.MCP_PATH) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken)); + + assertThatThrownBy(() -> brokenFilter + .filter(exchange, ignored -> Mono.error(new AssertionError("Request must not continue"))) + .block()) + .isInstanceOf(IllegalStateException.class) + .hasMessage("boom"); + + for (var i = 0; i < McpInFlightLimiter.PER_KEY_LIMIT; i++) { + assertThat(inFlightLimiter.tryAcquire(keyId)).isNotNull(); + } + } } From 32edef5470dde03e45d5ae8e54bbc5692d362b41 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 11:45:22 +0800 Subject: [PATCH 11/15] Serialize limiter accounting within each key's map bin The zero-count eviction in 1d9030b raced with acquisition: computeIfAbsent handed out the counter before incrementing it, so a concurrent release could evict it in between, leaving new permits on an orphaned counter that bypasses the per-key limit and miscounts the next generation on release. Run create-or-increment and the limit check inside the key's compute bin, and have release decrement its own captured counter, evicting only when it is still the mapped, zero-valued one. Adds concurrency churn tests that assert the per-key ceiling and full budget drain under parallel load. --- .../halo/mcpserver/McpInFlightLimiter.java | 35 +++++++++--- .../tools/AttachmentUploadLimiter.java | 34 ++++++++--- .../mcpserver/McpInFlightLimiterTest.java | 57 +++++++++++++++++++ .../tools/AttachmentUploadLimiterTest.java | 52 +++++++++++++++++ 4 files changed, 161 insertions(+), 17 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java index ad3c7c5..13460e4 100644 --- a/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java +++ b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java @@ -3,6 +3,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import org.springframework.stereotype.Component; /** Process-local bounds on concurrent authenticated MCP request work. */ @@ -28,32 +29,48 @@ Permit tryAcquire(String keyId) { globalCount.decrementAndGet(); return null; } - var count = keyCounts.computeIfAbsent(keyId, ignored -> new AtomicInteger()); - if (count.incrementAndGet() > PER_KEY_LIMIT) { - count.decrementAndGet(); + // Create-or-increment and the limit check run inside the key's map bin so a permit can + // never land on a counter that a concurrent release has already unmapped. + var acquired = new AtomicReference(); + keyCounts.compute(keyId, (key, existing) -> { + var count = existing == null ? new AtomicInteger() : existing; + if (count.incrementAndGet() > PER_KEY_LIMIT) { + count.decrementAndGet(); + return existing; + } + acquired.set(count); + return count; + }); + var count = acquired.get(); + if (count == null) { globalCount.decrementAndGet(); return null; } - return new Permit(keyId); + return new Permit(keyId, count); } final class Permit implements AutoCloseable { private final String keyId; + private final AtomicInteger count; private final AtomicBoolean released = new AtomicBoolean(); - private Permit(String keyId) { + private Permit(String keyId, AtomicInteger count) { this.keyId = keyId; + this.count = count; } @Override public void close() { if (released.compareAndSet(false, true)) { globalCount.decrementAndGet(); - // Drop the entry once the key holds no permits so churn through many keys - // cannot permanently exhaust MAX_TRACKED_KEYS. - keyCounts.computeIfPresent( - keyId, (key, count) -> count.decrementAndGet() <= 0 ? null : count); + if (count.decrementAndGet() <= 0) { + // Drop the entry once the key holds no permits so churn through many keys + // cannot permanently exhaust MAX_TRACKED_KEYS. The identity and zero + // rechecks keep a racing acquisition from losing its counter. + keyCounts.computeIfPresent(keyId, + (key, mapped) -> mapped == count && mapped.get() <= 0 ? null : mapped); + } } } } diff --git a/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java index 8187c46..fd06320 100644 --- a/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java +++ b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java @@ -3,6 +3,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import org.springframework.stereotype.Component; /** Process-local in-flight byte budgets that bound concurrent attachment uploads. */ @@ -30,12 +31,24 @@ Reservation tryAcquire(String keyId, long bytes) { if (!reserve(globalBytes, GLOBAL_BUDGET_BYTES, bytes)) { return null; } - var usage = keyBytes.computeIfAbsent(keyId, ignored -> new AtomicLong()); - if (!reserve(usage, PER_KEY_BUDGET_BYTES, bytes)) { + // Create-or-add and the budget check run inside the key's map bin so a reservation can + // never land on a counter that a concurrent release has already unmapped. + var acquired = new AtomicReference(); + keyBytes.compute(keyId, (key, existing) -> { + var usage = existing == null ? new AtomicLong() : existing; + if (usage.addAndGet(bytes) > PER_KEY_BUDGET_BYTES) { + usage.addAndGet(-bytes); + return existing; + } + acquired.set(usage); + return usage; + }); + var usage = acquired.get(); + if (usage == null) { globalBytes.addAndGet(-bytes); return null; } - return new Reservation(keyId, bytes); + return new Reservation(keyId, usage, bytes); } private static boolean reserve(AtomicLong usage, long limit, long bytes) { @@ -53,11 +66,13 @@ private static boolean reserve(AtomicLong usage, long limit, long bytes) { final class Reservation implements AutoCloseable { private final String keyId; + private final AtomicLong usage; private final long bytes; private final AtomicBoolean released = new AtomicBoolean(); - private Reservation(String keyId, long bytes) { + private Reservation(String keyId, AtomicLong usage, long bytes) { this.keyId = keyId; + this.usage = usage; this.bytes = bytes; } @@ -65,10 +80,13 @@ private Reservation(String keyId, long bytes) { public void close() { if (released.compareAndSet(false, true)) { globalBytes.addAndGet(-bytes); - // Drop the entry once the key holds no reservation so churn through many keys - // cannot permanently exhaust MAX_TRACKED_KEYS. - keyBytes.computeIfPresent( - keyId, (key, usage) -> usage.addAndGet(-bytes) <= 0 ? null : usage); + if (usage.addAndGet(-bytes) <= 0) { + // Drop the entry once the key holds no reservation so churn through many keys + // cannot permanently exhaust MAX_TRACKED_KEYS. The identity and zero + // rechecks keep a racing acquisition from losing its counter. + keyBytes.computeIfPresent(keyId, + (key, mapped) -> mapped == usage && mapped.get() <= 0 ? null : mapped); + } } } } diff --git a/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java index 01558ef..311de59 100644 --- a/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java @@ -3,6 +3,9 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.ArrayList; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; class McpInFlightLimiterTest { @@ -54,6 +57,60 @@ void forgetsKeysWhosePermitsHaveAllBeenReleased() { assertThat(limiter.tryAcquire("key-fresh")).isNotNull(); } + @Test + void keepsPerKeyAccountingConsistentUnderConcurrentChurn() throws InterruptedException { + var limiter = new McpInFlightLimiter(); + // The test counters bracket acquire and close so they always overestimate the true + // in-flight count; exceeding the limit here therefore proves a real breach. + var inFlightPerKey = new ConcurrentHashMap(); + var maxPerKey = new ConcurrentHashMap(); + var errors = new CopyOnWriteArrayList(); + var threads = new ArrayList(); + for (var t = 0; t < 8; t++) { + var thread = new Thread(() -> { + try { + for (var i = 0; i < 2_000; i++) { + var key = "key-" + i % 4; + var permit = limiter.tryAcquire(key); + if (permit == null) { + continue; + } + var inFlight = inFlightPerKey + .computeIfAbsent(key, ignored -> new AtomicInteger()) + .incrementAndGet(); + maxPerKey.computeIfAbsent(key, ignored -> new AtomicInteger()) + .accumulateAndGet(inFlight, Math::max); + inFlightPerKey.get(key).decrementAndGet(); + permit.close(); + } + } catch (Throwable error) { + errors.add(error); + } + }); + threads.add(thread); + thread.start(); + } + for (var thread : threads) { + thread.join(); + } + + assertThat(errors).isEmpty(); + assertThat(maxPerKey.values()) + .allSatisfy(max -> assertThat(max.get()) + .isLessThanOrEqualTo(McpInFlightLimiter.PER_KEY_LIMIT)); + // Both budgets must drain back to zero once every permit is closed. + for (var i = 0; i < 4; i++) { + var permits = new ArrayList(); + for (var j = 0; j < McpInFlightLimiter.PER_KEY_LIMIT; j++) { + var permit = limiter.tryAcquire("key-" + i); + assertThat(permit).isNotNull(); + permits.add(permit); + } + assertThat(limiter.tryAcquire("key-" + i)).isNull(); + permits.forEach(McpInFlightLimiter.Permit::close); + } + } + @Test void ignoresDoubleRelease() { var limiter = new McpInFlightLimiter(); diff --git a/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java index b9b2fb3..ba5ad5e 100644 --- a/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java @@ -3,6 +3,9 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.ArrayList; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicLong; import org.junit.jupiter.api.Test; class AttachmentUploadLimiterTest { @@ -58,6 +61,55 @@ void forgetsKeysWhoseReservationsHaveAllBeenReleased() { assertThat(limiter.tryAcquire("key-fresh", EIGHT_MIB)).isNotNull(); } + @Test + void keepsPerKeyAccountingConsistentUnderConcurrentChurn() throws InterruptedException { + var limiter = new AttachmentUploadLimiter(); + // The test counters bracket acquire and close so they always overestimate the true + // reserved bytes; exceeding the budget here therefore proves a real breach. + var inFlightPerKey = new ConcurrentHashMap(); + var maxPerKey = new ConcurrentHashMap(); + var errors = new CopyOnWriteArrayList(); + var threads = new ArrayList(); + for (var t = 0; t < 8; t++) { + var thread = new Thread(() -> { + try { + for (var i = 0; i < 2_000; i++) { + var key = "key-" + i % 4; + var reservation = limiter.tryAcquire(key, EIGHT_MIB); + if (reservation == null) { + continue; + } + var usage = inFlightPerKey + .computeIfAbsent(key, ignored -> new AtomicLong()); + var inFlight = usage.addAndGet(EIGHT_MIB); + maxPerKey.computeIfAbsent(key, ignored -> new AtomicLong()) + .accumulateAndGet(inFlight, Math::max); + usage.addAndGet(-EIGHT_MIB); + reservation.close(); + } + } catch (Throwable error) { + errors.add(error); + } + }); + threads.add(thread); + thread.start(); + } + for (var thread : threads) { + thread.join(); + } + + assertThat(errors).isEmpty(); + assertThat(maxPerKey.values()) + .allSatisfy(max -> assertThat(max.get()) + .isLessThanOrEqualTo(AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)); + // Both budgets must drain back to zero once every reservation is closed. + for (var i = 0; i < 4; i++) { + var drained = limiter.tryAcquire("key-" + i, AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES); + assertThat(drained).isNotNull(); + drained.close(); + } + } + @Test void ignoresDoubleRelease() { var limiter = new AttachmentUploadLimiter(); From 45b681c0496969e7a625d6b18ae597b3808657b6 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 11:45:39 +0800 Subject: [PATCH 12/15] Bound MCP authentication by the request deadline The 30s deadline only wrapped the MCP handler, so a stalled extension store lookup or password verification could park a request indefinitely before a permit was ever acquired. Move the timeout and its 503 mapping to the outer chain so the deadline covers authentication and handling alike; permits are still released through the inner doFinally when the deadline cancels in-flight handler work. --- .../mcpserver/McpKeyAuthenticationFilter.java | 8 +++++--- .../McpKeyAuthenticationFilterTest.java | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java index 7cce07c..536688b 100644 --- a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java +++ b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java @@ -81,15 +81,17 @@ public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { // Defer so a synchronously throwing handler assembly still flows through // the error path and releases the permit. return Mono.defer(() -> mcpHandler.handle(exchange.mutate().request(request).build())) - .timeout(requestTimeout) - .onErrorResume(java.util.concurrent.TimeoutException.class, - error -> serviceUnavailable(exchange)) .doFinally(ignored -> permit.close()) .contextWrite(org.springframework.security.core.context.ReactiveSecurityContextHolder .withAuthentication(authentication)) .thenReturn(true); }) .defaultIfEmpty(false) + // The deadline covers the whole authenticate-and-handle chain so stalled + // credential lookups cannot park requests either. + .timeout(requestTimeout) + .onErrorResume(java.util.concurrent.TimeoutException.class, + error -> serviceUnavailable(exchange).thenReturn(true)) .flatMap(handled -> handled ? Mono.empty() : unauthorized(exchange)); } diff --git a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java index 9955776..bcc63f4 100644 --- a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java +++ b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java @@ -217,6 +217,22 @@ accessKeyService, new McpRequestRateLimiter(), inFlightLimiter, mcpServer, } } + @Test + void timesOutAStalledAuthentication() { + var rawToken = "hmcp_00000000-0000-0000-0000-000000000000_secret"; + when(accessKeyService.authenticate(rawToken, null)).thenReturn(Mono.never()); + var fastFilter = new McpKeyAuthenticationFilter( + accessKeyService, new McpRequestRateLimiter(), new McpInFlightLimiter(), mcpServer, + java.time.Duration.ofMillis(50)); + var exchange = MockServerWebExchange.from(MockServerHttpRequest.post(McpKeyAuthenticationFilter.MCP_PATH) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken)); + + fastFilter.filter(exchange, ignored -> Mono.error(new AssertionError("Request must not continue"))) + .block(); + + assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + } + @Test void rejectsRequestsWhenTheInFlightBudgetIsExhausted() { var inFlightLimiter = new McpInFlightLimiter(); From fcccb3c6b2932151df7728616fa4a1335d7f890d Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 12:40:16 +0800 Subject: [PATCH 13/15] Simplify MCP in-flight limit accounting --- .../halo/mcpserver/KeyedInFlightLimiter.java | 72 +++++++++++ .../run/halo/mcpserver/McpAuthorization.java | 14 +-- .../halo/mcpserver/McpInFlightLimiter.java | 62 +--------- .../halo/mcpserver/tools/AttachmentTools.java | 12 +- .../tools/AttachmentUploadLimiter.java | 83 ++----------- .../mcpserver/KeyedInFlightLimiterTest.java | 94 ++++++++++++++ .../mcpserver/McpInFlightLimiterTest.java | 115 ++---------------- .../mcpserver/tools/AttachmentToolsTest.java | 5 +- .../tools/AttachmentUploadLimiterTest.java | 104 ++-------------- 9 files changed, 199 insertions(+), 362 deletions(-) create mode 100644 src/main/java/run/halo/mcpserver/KeyedInFlightLimiter.java create mode 100644 src/test/java/run/halo/mcpserver/KeyedInFlightLimiterTest.java diff --git a/src/main/java/run/halo/mcpserver/KeyedInFlightLimiter.java b/src/main/java/run/halo/mcpserver/KeyedInFlightLimiter.java new file mode 100644 index 0000000..68ef6f0 --- /dev/null +++ b/src/main/java/run/halo/mcpserver/KeyedInFlightLimiter.java @@ -0,0 +1,72 @@ +package run.halo.mcpserver; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +/** Process-local global and per-key in-flight budgets. */ +public final class KeyedInFlightLimiter { + + private final int perKeyLimit; + private final int maxTrackedKeys; + private final Semaphore globalPermits; + private final ConcurrentHashMap keyPermits = new ConcurrentHashMap<>(); + + public KeyedInFlightLimiter(int globalLimit, int perKeyLimit, int maxTrackedKeys) { + this.perKeyLimit = perKeyLimit; + this.maxTrackedKeys = maxTrackedKeys; + this.globalPermits = new Semaphore(globalLimit); + } + + public Lease tryAcquire(String keyId, int permits) { + if (permits < 0 || keyPermits.size() >= maxTrackedKeys && !keyPermits.containsKey(keyId)) { + return null; + } + if (!globalPermits.tryAcquire(permits)) { + return null; + } + var acquired = new AtomicReference(); + keyPermits.compute(keyId, (key, existing) -> { + var semaphore = existing == null ? new Semaphore(perKeyLimit) : existing; + if (!semaphore.tryAcquire(permits)) { + return existing; + } + acquired.set(semaphore); + return semaphore; + }); + var semaphore = acquired.get(); + if (semaphore == null) { + globalPermits.release(permits); + return null; + } + return new Lease(keyId, semaphore, permits); + } + + public final class Lease implements AutoCloseable { + + private final String keyId; + private final Semaphore semaphore; + private final int permits; + private final AtomicBoolean released = new AtomicBoolean(); + + private Lease(String keyId, Semaphore semaphore, int permits) { + this.keyId = keyId; + this.semaphore = semaphore; + this.permits = permits; + } + + @Override + public void close() { + if (released.compareAndSet(false, true)) { + globalPermits.release(permits); + keyPermits.compute(keyId, (key, mapped) -> { + semaphore.release(permits); + return mapped == semaphore && semaphore.availablePermits() == perKeyLimit + ? null + : mapped; + }); + } + } + } +} diff --git a/src/main/java/run/halo/mcpserver/McpAuthorization.java b/src/main/java/run/halo/mcpserver/McpAuthorization.java index d9aa45b..e6e374b 100644 --- a/src/main/java/run/halo/mcpserver/McpAuthorization.java +++ b/src/main/java/run/halo/mcpserver/McpAuthorization.java @@ -32,18 +32,8 @@ public Mono authorize(String toolName, Supplier> action) { })); } - /** Runs an action with the current key's ID while keeping the authentication token internal. */ - public Mono withKeyId(java.util.function.Function> action) { - return authentication().map(McpKeyAuthenticationToken::keyId).flatMap(keyId -> { - try { - var result = action.apply(keyId); - return result == null - ? Mono.error(new McpToolException("INTERNAL", "The tool returned no result")) - : result; - } catch (Throwable error) { - return Mono.error(error); - } - }); + public Mono keyId() { + return authentication().map(McpKeyAuthenticationToken::keyId); } Mono authentication() { diff --git a/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java index 13460e4..c544075 100644 --- a/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java +++ b/src/main/java/run/halo/mcpserver/McpInFlightLimiter.java @@ -1,9 +1,5 @@ package run.halo.mcpserver; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import org.springframework.stereotype.Component; /** Process-local bounds on concurrent authenticated MCP request work. */ @@ -14,64 +10,14 @@ class McpInFlightLimiter { static final int PER_KEY_LIMIT = 16; static final int MAX_TRACKED_KEYS = 10_000; - private final AtomicInteger globalCount = new AtomicInteger(); - private final ConcurrentHashMap keyCounts = new ConcurrentHashMap<>(); + private final KeyedInFlightLimiter limiter = + new KeyedInFlightLimiter(GLOBAL_LIMIT, PER_KEY_LIMIT, MAX_TRACKED_KEYS); /** * Reserves one global and per-key slot for the key, or returns null when either budget is * exhausted. The caller must close the permit exactly once when the request terminates. */ - Permit tryAcquire(String keyId) { - if (keyCounts.size() >= MAX_TRACKED_KEYS && !keyCounts.containsKey(keyId)) { - return null; - } - if (globalCount.incrementAndGet() > GLOBAL_LIMIT) { - globalCount.decrementAndGet(); - return null; - } - // Create-or-increment and the limit check run inside the key's map bin so a permit can - // never land on a counter that a concurrent release has already unmapped. - var acquired = new AtomicReference(); - keyCounts.compute(keyId, (key, existing) -> { - var count = existing == null ? new AtomicInteger() : existing; - if (count.incrementAndGet() > PER_KEY_LIMIT) { - count.decrementAndGet(); - return existing; - } - acquired.set(count); - return count; - }); - var count = acquired.get(); - if (count == null) { - globalCount.decrementAndGet(); - return null; - } - return new Permit(keyId, count); - } - - final class Permit implements AutoCloseable { - - private final String keyId; - private final AtomicInteger count; - private final AtomicBoolean released = new AtomicBoolean(); - - private Permit(String keyId, AtomicInteger count) { - this.keyId = keyId; - this.count = count; - } - - @Override - public void close() { - if (released.compareAndSet(false, true)) { - globalCount.decrementAndGet(); - if (count.decrementAndGet() <= 0) { - // Drop the entry once the key holds no permits so churn through many keys - // cannot permanently exhaust MAX_TRACKED_KEYS. The identity and zero - // rechecks keep a racing acquisition from losing its counter. - keyCounts.computeIfPresent(keyId, - (key, mapped) -> mapped == count && mapped.get() <= 0 ? null : mapped); - } - } - } + KeyedInFlightLimiter.Lease tryAcquire(String keyId) { + return limiter.tryAcquire(keyId, 1); } } diff --git a/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java b/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java index df443f9..ea6d6aa 100644 --- a/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java +++ b/src/main/java/run/halo/mcpserver/tools/AttachmentTools.java @@ -78,7 +78,7 @@ Mono upload(Map arguments) { throw new McpToolException("INVALID_ARGUMENT", "contentBase64 exceeds the 8 MiB limit"); } var mediaType = mediaType(arguments.get("mediaType")); - return authorization.withKeyId(keyId -> { + return authorization.keyId().flatMap(keyId -> { var reservation = uploadLimiter.tryAcquire(keyId, decodedLength(encoded)); if (reservation == null) { return Mono.error(new McpToolException( @@ -117,15 +117,9 @@ private static byte[] decode(String encoded) { * Returns the exact length valid Base64 decodes to. Invalid input may misestimate but never * below zero; the content is still validated when it is actually decoded. */ - private static long decodedLength(String encoded) { - var length = encoded.length(); + private static int decodedLength(String encoded) { var padding = encoded.endsWith("==") ? 2 : encoded.endsWith("=") ? 1 : 0; - var decoded = switch (length % 4) { - case 2 -> length / 4L * 3L + 1L; - case 3 -> length / 4L * 3L + 2L; - default -> length / 4L * 3L - padding; - }; - return Math.max(decoded, 0L); + return Math.max(encoded.length() * 3 / 4 - padding, 0); } Mono delete(Map arguments) { diff --git a/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java index fd06320..014da31 100644 --- a/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java +++ b/src/main/java/run/halo/mcpserver/tools/AttachmentUploadLimiter.java @@ -1,93 +1,24 @@ package run.halo.mcpserver.tools; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.atomic.AtomicReference; import org.springframework.stereotype.Component; +import run.halo.mcpserver.KeyedInFlightLimiter; /** Process-local in-flight byte budgets that bound concurrent attachment uploads. */ @Component class AttachmentUploadLimiter { - static final long GLOBAL_BUDGET_BYTES = 64L * 1024 * 1024; - static final long PER_KEY_BUDGET_BYTES = 32L * 1024 * 1024; + static final int GLOBAL_BUDGET_BYTES = 64 * 1024 * 1024; + static final int PER_KEY_BUDGET_BYTES = 32 * 1024 * 1024; static final int MAX_TRACKED_KEYS = 10_000; - private final AtomicLong globalBytes = new AtomicLong(); - private final ConcurrentHashMap keyBytes = new ConcurrentHashMap<>(); + private final KeyedInFlightLimiter limiter = + new KeyedInFlightLimiter(GLOBAL_BUDGET_BYTES, PER_KEY_BUDGET_BYTES, MAX_TRACKED_KEYS); /** * Reserves decoded bytes against the global and per-key budgets, or returns null when either * is exhausted. The caller must close the reservation exactly once when the upload terminates. */ - Reservation tryAcquire(String keyId, long bytes) { - if (bytes > GLOBAL_BUDGET_BYTES) { - return null; - } - if (keyBytes.size() >= MAX_TRACKED_KEYS && !keyBytes.containsKey(keyId)) { - return null; - } - if (!reserve(globalBytes, GLOBAL_BUDGET_BYTES, bytes)) { - return null; - } - // Create-or-add and the budget check run inside the key's map bin so a reservation can - // never land on a counter that a concurrent release has already unmapped. - var acquired = new AtomicReference(); - keyBytes.compute(keyId, (key, existing) -> { - var usage = existing == null ? new AtomicLong() : existing; - if (usage.addAndGet(bytes) > PER_KEY_BUDGET_BYTES) { - usage.addAndGet(-bytes); - return existing; - } - acquired.set(usage); - return usage; - }); - var usage = acquired.get(); - if (usage == null) { - globalBytes.addAndGet(-bytes); - return null; - } - return new Reservation(keyId, usage, bytes); - } - - private static boolean reserve(AtomicLong usage, long limit, long bytes) { - while (true) { - var current = usage.get(); - if (current + bytes > limit) { - return false; - } - if (usage.compareAndSet(current, current + bytes)) { - return true; - } - } - } - - final class Reservation implements AutoCloseable { - - private final String keyId; - private final AtomicLong usage; - private final long bytes; - private final AtomicBoolean released = new AtomicBoolean(); - - private Reservation(String keyId, AtomicLong usage, long bytes) { - this.keyId = keyId; - this.usage = usage; - this.bytes = bytes; - } - - @Override - public void close() { - if (released.compareAndSet(false, true)) { - globalBytes.addAndGet(-bytes); - if (usage.addAndGet(-bytes) <= 0) { - // Drop the entry once the key holds no reservation so churn through many keys - // cannot permanently exhaust MAX_TRACKED_KEYS. The identity and zero - // rechecks keep a racing acquisition from losing its counter. - keyBytes.computeIfPresent(keyId, - (key, mapped) -> mapped == usage && mapped.get() <= 0 ? null : mapped); - } - } - } + KeyedInFlightLimiter.Lease tryAcquire(String keyId, int bytes) { + return limiter.tryAcquire(keyId, bytes); } } diff --git a/src/test/java/run/halo/mcpserver/KeyedInFlightLimiterTest.java b/src/test/java/run/halo/mcpserver/KeyedInFlightLimiterTest.java new file mode 100644 index 0000000..4a5963d --- /dev/null +++ b/src/test/java/run/halo/mcpserver/KeyedInFlightLimiterTest.java @@ -0,0 +1,94 @@ +package run.halo.mcpserver; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class KeyedInFlightLimiterTest { + + private static final int GLOBAL_LIMIT = 8; + private static final int PER_KEY_LIMIT = 2; + private static final int MAX_TRACKED_KEYS = 4; + + @Test + void enforcesAndReleasesBudgets() { + var limiter = new KeyedInFlightLimiter(GLOBAL_LIMIT, PER_KEY_LIMIT, MAX_TRACKED_KEYS); + var first = limiter.tryAcquire("key-one", 1); + var second = limiter.tryAcquire("key-one", 1); + + assertThat(first).isNotNull(); + assertThat(second).isNotNull(); + assertThat(limiter.tryAcquire("key-one", 1)).isNull(); + first.close(); + first.close(); + assertThat(limiter.tryAcquire("key-one", 1)).isNotNull(); + assertThat(limiter.tryAcquire("key-one", 1)).isNull(); + + var globalLimiter = new KeyedInFlightLimiter(GLOBAL_LIMIT, GLOBAL_LIMIT, GLOBAL_LIMIT + 1); + var global = new ArrayList(); + for (var i = 0; i < GLOBAL_LIMIT; i++) { + global.add(globalLimiter.tryAcquire("global-" + i, 1)); + } + assertThat(global).doesNotContainNull(); + assertThat(globalLimiter.tryAcquire("global-extra", 1)).isNull(); + } + + @Test + void forgetsReleasedKeys() { + var limiter = new KeyedInFlightLimiter(GLOBAL_LIMIT, PER_KEY_LIMIT, MAX_TRACKED_KEYS); + + for (var i = 0; i < MAX_TRACKED_KEYS + 1; i++) { + var lease = limiter.tryAcquire("key-" + i, 1); + assertThat(lease).isNotNull(); + lease.close(); + } + } + + @Test + void keepsPerKeyAccountingConsistentUnderConcurrentChurn() throws InterruptedException { + var limiter = new KeyedInFlightLimiter(GLOBAL_LIMIT, PER_KEY_LIMIT, MAX_TRACKED_KEYS); + var inFlight = new ConcurrentHashMap(); + var maximum = new ConcurrentHashMap(); + var errors = new CopyOnWriteArrayList(); + var threads = new ArrayList(); + for (var t = 0; t < 8; t++) { + var thread = new Thread(() -> { + try { + for (var i = 0; i < 2_000; i++) { + var key = "key-" + i % 4; + var lease = limiter.tryAcquire(key, 1); + if (lease == null) { + continue; + } + var count = inFlight.computeIfAbsent(key, ignored -> new AtomicInteger()); + var current = count.incrementAndGet(); + maximum.computeIfAbsent(key, ignored -> new AtomicInteger()) + .accumulateAndGet(current, Math::max); + count.decrementAndGet(); + lease.close(); + } + } catch (Throwable error) { + errors.add(error); + } + }); + threads.add(thread); + thread.start(); + } + for (var thread : threads) { + thread.join(); + } + + assertThat(errors).isEmpty(); + assertThat(maximum.values()) + .allSatisfy(max -> assertThat(max.get()).isLessThanOrEqualTo(PER_KEY_LIMIT)); + for (var i = 0; i < 4; i++) { + var lease = limiter.tryAcquire("key-" + i, PER_KEY_LIMIT); + assertThat(lease).isNotNull(); + lease.close(); + } + } +} diff --git a/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java index 311de59..e225bc4 100644 --- a/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/McpInFlightLimiterTest.java @@ -3,126 +3,27 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.ArrayList; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; class McpInFlightLimiterTest { @Test - void enforcesThePerKeyLimitIndependently() { + void appliesRequestLimits() { var limiter = new McpInFlightLimiter(); - var permits = new ArrayList(); + var permits = new ArrayList(); for (var i = 0; i < McpInFlightLimiter.PER_KEY_LIMIT; i++) { permits.add(limiter.tryAcquire("key-one")); } assertThat(limiter.tryAcquire("key-one")).isNull(); - assertThat(limiter.tryAcquire("key-two")).isNotNull(); + var otherKey = limiter.tryAcquire("key-two"); + assertThat(otherKey).isNotNull(); - permits.getFirst().close(); - assertThat(limiter.tryAcquire("key-one")).isNotNull(); - } - - @Test - void enforcesTheGlobalLimitAcrossKeys() { - var limiter = new McpInFlightLimiter(); - var permits = new ArrayList(); - - var key = 0; - while (permits.size() < McpInFlightLimiter.GLOBAL_LIMIT) { - var permit = limiter.tryAcquire("key-" + key++); - assertThat(permit).isNotNull(); - permits.add(permit); - } - assertThat(limiter.tryAcquire("key-extra")).isNull(); - - permits.forEach(McpInFlightLimiter.Permit::close); + permits.forEach(KeyedInFlightLimiter.Lease::close); + otherKey.close(); for (var i = 0; i < McpInFlightLimiter.GLOBAL_LIMIT; i++) { - assertThat(limiter.tryAcquire("key-reused-" + i)).isNotNull(); + assertThat(limiter.tryAcquire("key-" + i)).isNotNull(); } - } - - @Test - void forgetsKeysWhosePermitsHaveAllBeenReleased() { - var limiter = new McpInFlightLimiter(); - - for (var i = 0; i < McpInFlightLimiter.MAX_TRACKED_KEYS + 1; i++) { - var permit = limiter.tryAcquire("key-" + i); - assertThat(permit).isNotNull(); - permit.close(); - } - - assertThat(limiter.tryAcquire("key-fresh")).isNotNull(); - } - - @Test - void keepsPerKeyAccountingConsistentUnderConcurrentChurn() throws InterruptedException { - var limiter = new McpInFlightLimiter(); - // The test counters bracket acquire and close so they always overestimate the true - // in-flight count; exceeding the limit here therefore proves a real breach. - var inFlightPerKey = new ConcurrentHashMap(); - var maxPerKey = new ConcurrentHashMap(); - var errors = new CopyOnWriteArrayList(); - var threads = new ArrayList(); - for (var t = 0; t < 8; t++) { - var thread = new Thread(() -> { - try { - for (var i = 0; i < 2_000; i++) { - var key = "key-" + i % 4; - var permit = limiter.tryAcquire(key); - if (permit == null) { - continue; - } - var inFlight = inFlightPerKey - .computeIfAbsent(key, ignored -> new AtomicInteger()) - .incrementAndGet(); - maxPerKey.computeIfAbsent(key, ignored -> new AtomicInteger()) - .accumulateAndGet(inFlight, Math::max); - inFlightPerKey.get(key).decrementAndGet(); - permit.close(); - } - } catch (Throwable error) { - errors.add(error); - } - }); - threads.add(thread); - thread.start(); - } - for (var thread : threads) { - thread.join(); - } - - assertThat(errors).isEmpty(); - assertThat(maxPerKey.values()) - .allSatisfy(max -> assertThat(max.get()) - .isLessThanOrEqualTo(McpInFlightLimiter.PER_KEY_LIMIT)); - // Both budgets must drain back to zero once every permit is closed. - for (var i = 0; i < 4; i++) { - var permits = new ArrayList(); - for (var j = 0; j < McpInFlightLimiter.PER_KEY_LIMIT; j++) { - var permit = limiter.tryAcquire("key-" + i); - assertThat(permit).isNotNull(); - permits.add(permit); - } - assertThat(limiter.tryAcquire("key-" + i)).isNull(); - permits.forEach(McpInFlightLimiter.Permit::close); - } - } - - @Test - void ignoresDoubleRelease() { - var limiter = new McpInFlightLimiter(); - var permit = limiter.tryAcquire("key-one"); - assertThat(permit).isNotNull(); - - permit.close(); - permit.close(); - - for (var i = 0; i < McpInFlightLimiter.PER_KEY_LIMIT; i++) { - assertThat(limiter.tryAcquire("key-one")).isNotNull(); - } - assertThat(limiter.tryAcquire("key-one")).isNull(); + assertThat(limiter.tryAcquire("key-extra")).isNull(); } } diff --git a/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java b/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java index 8f705c3..18b55a6 100644 --- a/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java +++ b/src/test/java/run/halo/mcpserver/tools/AttachmentToolsTest.java @@ -150,9 +150,6 @@ void deletionUsesExtensionLifecycleSoReconcilerCleansStorage() { } private void stubKeyId(String keyId) { - when(authorization.withKeyId(any())).thenAnswer(invocation -> { - java.util.function.Function> action = invocation.getArgument(0); - return action.apply(keyId); - }); + when(authorization.keyId()).thenReturn(Mono.just(keyId)); } } diff --git a/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java index ba5ad5e..2c75ce1 100644 --- a/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java +++ b/src/test/java/run/halo/mcpserver/tools/AttachmentUploadLimiterTest.java @@ -3,123 +3,35 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.ArrayList; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.atomic.AtomicLong; import org.junit.jupiter.api.Test; +import run.halo.mcpserver.KeyedInFlightLimiter; class AttachmentUploadLimiterTest { - private static final long EIGHT_MIB = 8L * 1024 * 1024; + private static final int EIGHT_MIB = 8 * 1024 * 1024; @Test - void enforcesThePerKeyBudgetIndependently() { + void appliesUploadByteBudgets() { var limiter = new AttachmentUploadLimiter(); - var reservations = new ArrayList(); + var reservations = new ArrayList(); for (var i = 0; i < AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES / EIGHT_MIB; i++) { reservations.add(limiter.tryAcquire("key-one", EIGHT_MIB)); } assertThat(limiter.tryAcquire("key-one", EIGHT_MIB)).isNull(); - assertThat(limiter.tryAcquire("key-two", EIGHT_MIB)).isNotNull(); - - reservations.getFirst().close(); - assertThat(limiter.tryAcquire("key-one", EIGHT_MIB)).isNotNull(); - } - - @Test - void enforcesTheGlobalBudgetAcrossKeys() { - var limiter = new AttachmentUploadLimiter(); - - assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) - .isNotNull(); assertThat(limiter.tryAcquire("key-two", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) .isNotNull(); assertThat(limiter.tryAcquire("key-three", EIGHT_MIB)).isNull(); + + reservations.getFirst().close(); + assertThat(limiter.tryAcquire("key-one", EIGHT_MIB)).isNotNull(); } @Test - void rejectsASingleReservationLargerThanTheGlobalBudget() { + void rejectsReservationsLargerThanTheGlobalBudget() { var limiter = new AttachmentUploadLimiter(); assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.GLOBAL_BUDGET_BYTES + 1)) .isNull(); - assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) - .isNotNull(); - } - - @Test - void forgetsKeysWhoseReservationsHaveAllBeenReleased() { - var limiter = new AttachmentUploadLimiter(); - - for (var i = 0; i < AttachmentUploadLimiter.MAX_TRACKED_KEYS + 1; i++) { - var reservation = limiter.tryAcquire("key-" + i, 1); - assertThat(reservation).isNotNull(); - reservation.close(); - } - - assertThat(limiter.tryAcquire("key-fresh", EIGHT_MIB)).isNotNull(); - } - - @Test - void keepsPerKeyAccountingConsistentUnderConcurrentChurn() throws InterruptedException { - var limiter = new AttachmentUploadLimiter(); - // The test counters bracket acquire and close so they always overestimate the true - // reserved bytes; exceeding the budget here therefore proves a real breach. - var inFlightPerKey = new ConcurrentHashMap(); - var maxPerKey = new ConcurrentHashMap(); - var errors = new CopyOnWriteArrayList(); - var threads = new ArrayList(); - for (var t = 0; t < 8; t++) { - var thread = new Thread(() -> { - try { - for (var i = 0; i < 2_000; i++) { - var key = "key-" + i % 4; - var reservation = limiter.tryAcquire(key, EIGHT_MIB); - if (reservation == null) { - continue; - } - var usage = inFlightPerKey - .computeIfAbsent(key, ignored -> new AtomicLong()); - var inFlight = usage.addAndGet(EIGHT_MIB); - maxPerKey.computeIfAbsent(key, ignored -> new AtomicLong()) - .accumulateAndGet(inFlight, Math::max); - usage.addAndGet(-EIGHT_MIB); - reservation.close(); - } - } catch (Throwable error) { - errors.add(error); - } - }); - threads.add(thread); - thread.start(); - } - for (var thread : threads) { - thread.join(); - } - - assertThat(errors).isEmpty(); - assertThat(maxPerKey.values()) - .allSatisfy(max -> assertThat(max.get()) - .isLessThanOrEqualTo(AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)); - // Both budgets must drain back to zero once every reservation is closed. - for (var i = 0; i < 4; i++) { - var drained = limiter.tryAcquire("key-" + i, AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES); - assertThat(drained).isNotNull(); - drained.close(); - } - } - - @Test - void ignoresDoubleRelease() { - var limiter = new AttachmentUploadLimiter(); - var reservation = limiter.tryAcquire("key-one", EIGHT_MIB); - assertThat(reservation).isNotNull(); - - reservation.close(); - reservation.close(); - - assertThat(limiter.tryAcquire("key-one", AttachmentUploadLimiter.PER_KEY_BUDGET_BYTES)) - .isNotNull(); } } From d0160c9a932002b5dc540bd1fb0f0aafef4d4296 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 15:44:46 +0800 Subject: [PATCH 14/15] Fix MCP authentication filter bean creation --- .../mcpserver/McpKeyAuthenticationFilter.java | 2 ++ .../McpKeyAuthenticationFilterTest.java | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java index 536688b..e68f5dc 100644 --- a/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java +++ b/src/main/java/run/halo/mcpserver/McpKeyAuthenticationFilter.java @@ -4,6 +4,7 @@ import static org.springframework.http.HttpHeaders.RETRY_AFTER; import static org.springframework.http.HttpHeaders.WWW_AUTHENTICATE; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatus; @@ -30,6 +31,7 @@ class McpKeyAuthenticationFilter implements BeforeSecurityWebFilter { private final java.util.Set protocolVersions; private final java.time.Duration requestTimeout; + @Autowired McpKeyAuthenticationFilter( McpAccessKeyService accessKeyService, McpRequestRateLimiter rateLimiter, diff --git a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java index bcc63f4..8005d93 100644 --- a/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java +++ b/src/test/java/run/halo/mcpserver/McpKeyAuthenticationFilterTest.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; @@ -61,6 +62,21 @@ void setUp() { accessKeyService, rateLimiter, new McpInFlightLimiter(), mcpServer); } + @Test + void springCreatesTheFilterUsingTheProductionConstructor() { + try (var context = new AnnotationConfigApplicationContext()) { + context.registerBean(McpAccessKeyService.class, () -> accessKeyService); + context.registerBean(McpRequestRateLimiter.class, () -> rateLimiter); + context.registerBean(McpInFlightLimiter.class, McpInFlightLimiter::new); + context.registerBean(HaloMcpServer.class, () -> mcpServer); + context.register(McpKeyAuthenticationFilter.class); + + context.refresh(); + + assertThat(context.getBean(McpKeyAuthenticationFilter.class)).isNotNull(); + } + } + @Test void authenticatesAndStripsTheMcpBearerTokenBeforeTheHaloJwtFilter() { var rawToken = "hmcp_00000000-0000-0000-0000-000000000000_secret"; From 8dfd3e6a2aef9eac4d55f931bb1fffd2d8db5de8 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 25 Aug 2026 15:48:37 +0800 Subject: [PATCH 15/15] Clarify access key warning text Updated the access key secret modal warning to explicitly remind users not to disclose or share the key anywhere, reducing accidental leakage risk. The change keeps the existing guidance about single-time visibility and rotation. --- ui/src/components/AccessKeySecretModal.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/components/AccessKeySecretModal.vue b/ui/src/components/AccessKeySecretModal.vue index 3cefcf0..c2f7843 100644 --- a/ui/src/components/AccessKeySecretModal.vue +++ b/ui/src/components/AccessKeySecretModal.vue @@ -26,7 +26,7 @@ const { copy, copied } = useClipboard({