fix(server): count sign-in attempts per client

SECURITY.md

@@ -8,6 +8,8 @@ Use the repository's **Security → Report a vulnerability** form for private re
 
 OpenMuse currently supports one owner per deployment. Live mode uses a shared access key; it is not multi-tenant account authentication. Sample mode binds to loopback and contains fictional data. Use HTTPS and restricted network access for a remote live deployment.
 
+Rate limits, including failed sign-ins, are counted per client address. Set `TRUST_PROXY=true` only behind a reverse proxy that strips or overwrites `X-Forwarded-For`. The first entry is trusted, so an edge that appends to a caller-supplied value lets one caller appear as many clients. Without `TRUST_PROXY`, clients behind a proxy share one limit.
+
 The API holds provider credentials. Google tokens are encrypted at rest; short-lived signed URLs grant file and browser-console access. Protect `.env`, `.openmuse`, database backups, and browser profiles as private data. A signed URL is a credential until it expires.
 
 The browser worker must remain private and require its own random token. It runs persistent Chromium with application-enforced public-network checks. Playwright disables Chromium's internal sandbox by default; this is not a full desktop VM or a security boundary for hostile tenants. The browser Docker image reduces host access but does not establish kernel-enforced network isolation. See [worker boundaries](apps/worker/README.md).

apps/server/src/app.ts

@@ -104,15 +104,13 @@ export async function createApp(
       browserConfigured: Boolean(config.workerUrl && config.workerToken),
     }),
   );
-  let loginWindow = 0,
-    loginAttempts = 0;
-  app.post("/api/session", async (c) => {
-    if (Date.now() - loginWindow > 60000) {
-      loginWindow = Date.now();
-      loginAttempts = 0;
-    }
-    if (++loginAttempts > 30)
-      throw new AppError("Too many sign-in attempts. Try again in a minute.", 429);
+  // Rejected guesses are counted per client so one caller cannot lock out the owner.
+  const signInLimit = rateLimit(Boolean(config.trustProxy), {
+    maxRequests: 30,
+    message: "Too many sign-in attempts. Try again in a minute.",
+    counts: (status) => status === 401,
+  });
+  app.post("/api/session", signInLimit, async (c) => {
     const body = z.object({ accessKey: z.string().optional() }).parse(await c.req.json());
     const session = await auth.session(body.accessKey);
     await workspace.ensureSample("local-user", actions);

apps/server/src/rate-limit.ts

@@ -40,7 +40,12 @@ export function createRateLimitStore(options: RateLimitStoreOptions = {}) {
     return entry.count <= maxRequests;
   }
 
-  return { take, size: () => entries.size };
+  function release(key: string, now: number) {
+    const entry = entries.get(key);
+    if (entry && entry.expiresAt > now && entry.count > 0) entry.count -= 1;
+  }
+
+  return { take, release, size: () => entries.size };
 }
 
 // This limiter runs before authentication, so the key must never be derived from
@@ -72,7 +77,12 @@ function connectionAddress(c: Context): string | undefined {
 
 export function rateLimit(
   trustProxy: boolean,
-  options: RateLimitStoreOptions & { getAddress?: (c: Context) => string | undefined } = {},
+  options: RateLimitStoreOptions & {
+    getAddress?: (c: Context) => string | undefined;
+    message?: string;
+    /** Refund a request whose response status this rejects. Reserving first bounds parallel bursts. */
+    counts?: (status: number) => boolean;
+  } = {},
 ) {
   const store = createRateLimitStore(options);
   const getAddress = options.getAddress ?? connectionAddress;
@@ -84,7 +94,8 @@ export function rateLimit(
       connectionAddress: getAddress(c),
     });
     if (!store.take(key, Date.now()))
-      throw new AppError("Too many requests. Try again in a minute.", 429);
+      throw new AppError(options.message ?? "Too many requests. Try again in a minute.", 429);
     await next();
+    if (options.counts && !options.counts(c.res.status)) store.release(key, Date.now());
   };
 }

tests/rate-limit.test.ts

@@ -111,6 +111,73 @@ test("unauthenticated requests cannot multiply buckets by spoofing X-Forwarded-F
   );
 });
 
+const accessKey = "fictional-owner-access-key";
+
+async function withLiveApp(run: (live: typeof app) => Promise<void>) {
+  const liveDirectory = await mkdtemp(join(tmpdir(), "openmuse-signin-"));
+  const liveDb = await createStore();
+  try {
+    const { app: live } = await createApp(liveDb, {
+      ...config,
+      mode: "live",
+      agentBackend: "model",
+      dataDir: liveDirectory,
+      accessKey,
+      encryptionKey: Buffer.alloc(32, 1).toString("base64"),
+      trustProxy: true,
+    });
+    await run(live);
+  } finally {
+    await liveDb.close();
+    await rm(liveDirectory, { recursive: true, force: true });
+  }
+}
+
+function signIn(live: typeof app, client: string, key: string) {
+  return postSession(live, client, JSON.stringify({ accessKey: key }));
+}
+
+function postSession(live: typeof app, client: string, body: string) {
+  return live.request("/api/session", {
+    method: "POST",
+    headers: { "Content-Type": "application/json", "X-Forwarded-For": client },
+    body,
+  });
+}
+
+test("one client's failed sign-ins do not block another client with the correct key", async () => {
+  await withLiveApp(async (live) => {
+    for (let i = 0; i < 30; i++)
+      assert.equal((await signIn(live, "203.0.113.1", "wrong-guess")).status, 401);
+    const blocked = await signIn(live, "203.0.113.1", "wrong-guess");
+    assert.equal(blocked.status, 429);
+    assert.deepEqual(await blocked.json(), {
+      error: "Too many sign-in attempts. Try again in a minute.",
+    });
+    const owner = await signIn(live, "203.0.113.2", accessKey);
+    assert.equal(owner.status, 200, "another client's guesses must not lock out the owner");
+    const { token } = (await owner.json()) as { token: string };
+    const signOut = await live.request("/api/session", {
+      method: "DELETE",
+      headers: { Authorization: `Bearer ${token}`, "X-Forwarded-For": "203.0.113.1" },
+    });
+    assert.equal(signOut.status, 200, "the sign-in limit must not block signing out");
+  });
+});
+
+test("only rejected sign-in guesses use a client's attempts", async () => {
+  await withLiveApp(async (live) => {
+    const client = "203.0.113.3";
+    for (let i = 0; i < 31; i++) {
+      assert.equal((await signIn(live, client, accessKey)).status, 200);
+      assert.equal((await postSession(live, client, "{")).status, 400);
+    }
+    for (let i = 0; i < 30; i++)
+      assert.equal((await signIn(live, client, "wrong-guess")).status, 401);
+    assert.equal((await signIn(live, client, "wrong-guess")).status, 429);
+  });
+});
+
 function limitedApp(options: Parameters<typeof rateLimit>[1]) {
   const app = new Hono();
   app.onError((error, c) =>