fix(security): harden rate limits with extension-owned Redis

cli/commands/skills/handler.test.ts

@@ -9,7 +9,7 @@ describe("Skills Command", () => {
   ): Promise<{ code: number; stdout: string; stderr: string }> {
     const cliPath = new URL("../../main.ts", import.meta.url).pathname;
     const result = await new Deno.Command(Deno.execPath(), {
-      args: ["run", "-A", cliPath, "skills", "info", ...args, "--json"],
+      args: ["run", "--quiet", "-A", cliPath, "skills", "info", ...args, "--json"],
       env: { VERYFRONT_NO_UPDATE_CHECK: "1", NO_COLOR: "1" },
       stdin: "null",
       stdout: "piped",

docs/api-reference/veryfront/agent.md

@@ -729,7 +729,7 @@ Input delivered to a hosted agent-service detached execution callback.
 | `createRuntimeAgentSystemMessages` | Create runtime agent system messages. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/runtime/agent-definition.ts#L244) |
 | `createRuntimeLoadSkillTool` | Create runtime load skill tool. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/runtime/load-skill-tool.ts#L1858) |
 | `createRuntimeProjectFilesClient` | Create runtime project files client. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/runtime/project-files-client.ts#L312) |
-| `createRuntimeProjectSkillLoader` | Create runtime project skill loader. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/runtime/project-skill-loader.ts#L706) |
+| `createRuntimeProjectSkillLoader` | Create runtime project skill loader. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/runtime/project-skill-loader.ts#L685) |
 | `createRuntimePromptBlock` | Create runtime prompt block. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/runtime/prompt-block.ts#L9) |
 | `createStreamedStepState` | State for create streamed step. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/streaming/fork-runtime-step-state.ts#L43) |
 | `createToolExecutionDataEventBridgeStream` | Create tool execution data event bridge stream. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/streaming/tool-execution-data-event-bridge.ts#L41) |

docs/api-reference/veryfront/middleware.md

@@ -110,11 +110,19 @@ Options accepted by rate limit.
 
 | Property | Type | Description | Source |
 |----------|------|-------------|--------|
-| `maxRequests?` | `number` | Max requests per window | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L73) |
-| `windowMs?` | `number` | Time window (ms) | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L74) |
-| `store?` | `RateLimitStore` | Storage backend | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L75) |
-| `keyGenerator?` | <code>(req: Request) =&gt; string</code> | Function to derive rate limit key from request | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L76) |
-| `trustProxy?` | `boolean` | Trust proxy-set forwarding headers (X-Forwarded-For) for keying. Defaults to false so forwarded headers are ignored and cannot be used to evade limits. Enable only when a trusted proxy that appends the real client IP sits in front of this middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L83) |
+| `maxRequests?` | `number` | Max requests per window | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L132) |
+| `windowMs?` | `number` | Time window (ms) | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L133) |
+| `store?` | `RateLimitStore` | Storage backend | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L134) |
+| `keyGenerator?` | <code>(req: Request) =&gt; string</code> | Function to derive rate limit key from request | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L135) |
+| `trustProxy?` | `boolean` | Trust proxy-set forwarding headers (X-Forwarded-For) for keying. Defaults to false so forwarded headers are ignored and cannot be used to evade limits. Enable only when a trusted proxy that appends the real client IP sits in front of this middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L142) |
+
+### `MemoryRateLimitStoreOptions`
+
+Options accepted by the in-memory rate limit store.
+
+| Property | Type | Description | Source |
+|----------|------|-------------|--------|
+| `maxEntries?` | `number` | Maximum number of active identities retained by the store. Size this above the expected concurrent identities in one rate-limit window. New identities fail closed when all entries are active; existing identities remain tracked until their windows expire. Capacity exhaustion emits a `store-capacity` log stage with the configured `maxEntries`. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L137) |
 
 ### `LoggerOptions`
 
@@ -142,40 +150,41 @@ Options accepted by timeout.
 
 | Name | Description | Source |
 |------|-------------|--------|
-| `authRateLimit` | Pre-configured rate limiter for authentication endpoints (5 req/15min). | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L140) |
+| `authRateLimit` | Pre-configured rate limiter for authentication endpoints (5 req/15min). | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L289) |
 | `cors` | Create CORS middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/cors/middleware.ts#L10) |
 | `devLogger` | Create development request logging middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/logger.ts#L244) |
 | `getTimeoutFromEnv` | Gets timeout from environment variable REQUEST_TIMEOUT_MS | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/timeout.ts#L94) |
 | `logger` | Create request logging middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/logger.ts#L191) |
 | `prodLogger` | Create production request logging middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/logger.ts#L249) |
-| `rateLimit` | Create rate-limit middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L106) |
+| `rateLimit` | Create rate-limit middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L205) |
 | `timeout` | Creates a middleware that enforces request timeouts. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/timeout.ts#L52) |
 | `timeoutFromEnv` | Creates a timeout middleware with configuration from environment | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/timeout.ts#L102) |
 
 ### Classes
 
 | Name | Description | Source |
 |------|-------------|--------|
-| `MemoryRateLimitStore` | Implement memory rate limit store. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L25) |
+| `MemoryRateLimitStore` | Implement memory rate limit store. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L37) |
 | `MiddlewareContext` | Context for middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/context.ts#L5) |
 | `MiddlewarePipeline` | Implement middleware pipeline. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/pipeline/pipeline.ts#L9) |
-| `RedisRateLimitStore` | Implement redis rate limit store. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/redis-rate-limit.ts#L27) |
+| `RedisRateLimitStore` | Create a Redis rate limit store. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/redis-rate-limit.ts#L55) |
 
 ### Types
 
 | Name | Description | Source |
 |------|-------------|--------|
-| `AuthRateLimitOptions` | Options accepted by the authentication rate-limit preset. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L87) |
+| `AuthRateLimitOptions` | Options accepted by the authentication rate-limit preset. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L146) |
 | `Context` | Context for context. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/types.ts#L8) |
 | `CorsOptions` | Options accepted by cors. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/types.ts#L26) |
 | `ExecutionContext` | Context for execution. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/types.ts#L2) |
 | `LogFormat` | Public API contract for log format. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/logger.ts#L14) |
 | `LoggerOptions` | Options accepted by logger. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/logger.ts#L17) |
+| `MemoryRateLimitStoreOptions` | Options accepted by the in-memory rate limit store. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L122) |
 | `MiddlewareFactory` | Public API contract for middleware factory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/types.ts#L32) |
 | `MiddlewareHandler` | Handler for middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/types.ts#L26) |
 | `MiddlewarePipelineOptions` | Options accepted by middleware pipeline. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/pipeline/types.ts#L2) |
 | `Next` | Public API contract for next. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/core/types.ts#L23) |
-| `RateLimitOptions` | Options accepted by rate limit. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L72) |
+| `RateLimitOptions` | Options accepted by rate limit. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/rate-limit.ts#L131) |
 | `RateLimitStore` | Public API contract for rate limit store. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/types.ts#L32) |
-| `RedisRateLimitOptions` | Options accepted by redis rate limit. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/redis-rate-limit.ts#L21) |
+| `RedisRateLimitOptions` | Options accepted by Redis rate limit. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/security/redis-rate-limit.ts#L45) |
 | `TimeoutOptions` | Options accepted by timeout. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/middleware/builtin/timeout.ts#L17) |

extensions/ext-redis/src/index.ts

@@ -54,7 +54,7 @@ export default extRedis;
 export { RedisMemory } from "./agent-memory.ts";
 export { createRedisCacheAdministration } from "./cache-administration.ts";
 export { RedisCacheBackend } from "./cache-backend.ts";
-export { RedisRateLimitStore } from "./rate-limit-store.ts";
+export { type RedisRateLimitOptions, RedisRateLimitStore } from "./rate-limit-store.ts";
 export { RedisCacheStore } from "./render-cache-store.ts";
 export { startProxyRoutingInvalidationBus } from "./routing-invalidation-bus.ts";
 export { createRedisRuntimeProvider } from "./redis-runtime-provider.ts";

extensions/ext-redis/src/rate-limit-store.test.ts

@@ -1,7 +1,8 @@
 import "#veryfront/schemas/_test-setup.ts";
 import { assertEquals, assertRejects, assertThrows } from "#veryfront/testing/assert.ts";
 import { describe, it } from "#veryfront/testing/bdd.ts";
-import { type RedisRateLimitOptions, RedisRateLimitStore } from "./rate-limit-store.ts";
+import { ClientClosedError } from "redis";
+import { type RedisRateLimitOptions, RedisRateLimitStore } from "./index.ts";
 
 async function outcomeWithin(
   promise: Promise<unknown>,
@@ -339,6 +340,22 @@ describe("middleware/builtin/security/redis-rate-limit", () => {
         assertEquals((rateStore as any).client, null);
       });
 
+      it("should treat already-closed clients as destroyed", async () => {
+        const { rateStore, mockClient } = createStoreWithMock();
+        let disconnectAttempts = 0;
+        mockClient.disconnect = () => {
+          disconnectAttempts++;
+          return Promise.reject(new ClientClosedError());
+        };
+
+        await rateStore.destroy();
+        await rateStore.destroy();
+
+        assertEquals(disconnectAttempts, 1);
+        // deno-lint-ignore no-explicit-any
+        assertEquals((rateStore as any).client, null);
+      });
+
       it("should retain a failed disconnect so shutdown can retry it", async () => {
         const { rateStore, mockClient } = createStoreWithMock();
         let disconnectAttempts = 0;
@@ -360,6 +377,24 @@ describe("middleware/builtin/security/redis-rate-limit", () => {
       });
     });
 
+    describe("reset", () => {
+      it("should reject invalid keys before connecting", async () => {
+        const rateStore = new RedisRateLimitStore();
+        const mockClient = createMockRedisClient();
+
+        // deno-lint-ignore no-explicit-any
+        (rateStore as any).loadClientFactory = () => Promise.resolve(() => mockClient);
+
+        await assertRejects(
+          () => rateStore.reset("x".repeat(1025)),
+          RangeError,
+          "1024",
+        );
+
+        assertEquals(mockClient._connectCalls, 0);
+      });
+    });
+
     describe("ensureClient", () => {
       it("should reuse existing client", async () => {
         const { rateStore, mockClient } = createStoreWithMock();
@@ -564,6 +599,60 @@ describe("middleware/builtin/security/redis-rate-limit", () => {
         const outcome = await outcomeWithin(incrementPromise, 50);
         assertEquals(outcome, "rejected");
       });
+
+      it("should attach pending rejection handling before destroy cancels it", async () => {
+        const rateStore = new RedisRateLimitStore();
+        const mockClient = createMockRedisClient();
+        let connectStarted = false;
+        mockClient.connect = () => {
+          connectStarted = true;
+          return new Promise<void>(() => {});
+        };
+
+        // deno-lint-ignore no-explicit-any
+        (rateStore as any).loadClientFactory = () => Promise.resolve(() => mockClient);
+
+        const incrementPromise = rateStore.increment("pending", 1000);
+        for (let attempt = 0; attempt < 10 && !connectStarted; attempt++) {
+          await Promise.resolve();
+        }
+
+        // deno-lint-ignore no-explicit-any
+        const pending = (rateStore as any).clientPromise as Promise<unknown> | null;
+        let cancelObserved = false;
+        let catchAttachedBeforeCancel = false;
+        if (pending) {
+          const originalCatch = pending.catch.bind(pending);
+          Object.defineProperty(pending, "catch", {
+            configurable: true,
+            value: (...args: Parameters<Promise<unknown>["catch"]>) => {
+              if (!cancelObserved) catchAttachedBeforeCancel = true;
+              return originalCatch(...args);
+            },
+          });
+        }
+        // deno-lint-ignore no-explicit-any
+        const originalCancel = (rateStore as any).cancelPendingConnection as
+          | (() => void)
+          | null;
+        // deno-lint-ignore no-explicit-any
+        (rateStore as any).cancelPendingConnection = () => {
+          cancelObserved = true;
+          originalCancel?.();
+        };
+
+        await rateStore.destroy();
+
+        if (!pending) throw new Error("Expected pending connection promise");
+        assertEquals(catchAttachedBeforeCancel, true);
+        const pendingOutcome = await outcomeWithin(pending, 50);
+        await assertRejects(
+          () => incrementPromise,
+          Error,
+          "superseded",
+        );
+        assertEquals(pendingOutcome, "rejected");
+      });
     });
   });
 });

extensions/ext-redis/src/rate-limit-store.ts

@@ -1,6 +1,6 @@
 import { createError, toError } from "veryfront/errors";
 import { serverLogger } from "veryfront/utils/logger";
-import { createClient } from "redis";
+import { ClientClosedError, createClient } from "redis";
 import {
   MAX_TIMER_DELAY_MS,
   type RateLimitEntry,
@@ -157,6 +157,10 @@ export class RedisRateLimitStore implements RateLimitStore {
     try {
       await this.disconnectClient(client);
     } catch (error) {
+      if (isAlreadyClosedClientError(error)) {
+        this.markDisconnected(client);
+        return;
+      }
       logger.warn("client disconnect failed", {
         errorName: error instanceof Error ? error.name : typeof error,
       });
@@ -180,24 +184,38 @@ export class RedisRateLimitStore implements RateLimitStore {
     try {
       Promise.resolve(client.disconnect()).then(
         () => {
-          this.disconnectPromises.delete(client);
-          this.pendingDisconnectClients.delete(client);
-          this.disconnectedClients.add(client);
+          this.markDisconnected(client);
           resolveDisconnect();
         },
         (error) => {
           this.disconnectPromises.delete(client);
+          if (isAlreadyClosedClientError(error)) {
+            this.markDisconnected(client);
+            resolveDisconnect();
+            return;
+          }
           rejectDisconnect(error);
         },
       );
     } catch (error) {
       this.disconnectPromises.delete(client);
-      rejectDisconnect(error);
+      if (isAlreadyClosedClientError(error)) {
+        this.markDisconnected(client);
+        resolveDisconnect();
+      } else {
+        rejectDisconnect(error);
+      }
     }
 
     return pending;
   }
 
+  private markDisconnected(client: RedisClient): void {
+    this.disconnectPromises.delete(client);
+    this.pendingDisconnectClients.delete(client);
+    this.disconnectedClients.add(client);
+  }
+
   private async withTimeout<T>(
     operation: Promise<T>,
     timeoutMs: number,
@@ -318,11 +336,12 @@ export class RedisRateLimitStore implements RateLimitStore {
   }
 
   async reset(key: string): Promise<void> {
+    const normalizedKey = requireRateLimitKey(key);
     const client = await this.ensureClient();
     const generation = this.clientGeneration;
     try {
       await this.withTimeout(
-        client.del(this.storageKey(requireRateLimitKey(key))),
+        client.del(this.storageKey(normalizedKey)),
         this.operationTimeoutMs,
         "reset",
       );
@@ -338,6 +357,7 @@ export class RedisRateLimitStore implements RateLimitStore {
     const client = this.client;
     const connectingClient = this.connectingClient;
     const pending = this.clientPromise;
+    pending?.catch(() => {});
     const cancelPendingConnection = this.cancelPendingConnection;
     const clientsToDisconnect = new Set(this.pendingDisconnectClients);
     if (client) clientsToDisconnect.add(client);
@@ -359,8 +379,6 @@ export class RedisRateLimitStore implements RateLimitStore {
         })
       ),
     );
-    pending?.catch(() => {});
-
     if (disconnectFailed) throw disconnectError;
   }
 }
@@ -391,6 +409,10 @@ function isTimeoutError(error: unknown): boolean {
   return error instanceof Error && error.name === "TimeoutError";
 }
 
+function isAlreadyClosedClientError(error: unknown): boolean {
+  return error instanceof ClientClosedError;
+}
+
 function parseIncrementResult(result: unknown): [number, number] {
   if (!Array.isArray(result) || result.length < 2) {
     throw toError(