From c37f621fbeea4d390ac5cf104010082f2fd1cc39 Mon Sep 17 00:00:00 2001 From: MustafaYamin Date: Sun, 1 Nov 2026 20:34:41 +0500 Subject: [PATCH 1/2] fix(server): count sign-in attempts per client --- apps/server/src/app.ts | 15 +++++-------- apps/server/src/rate-limit.ts | 7 ++++-- tests/rate-limit.test.ts | 41 +++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index bff91dd2f..d6c325543 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -104,15 +104,12 @@ 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); + // Counted per client so one caller's failed guesses cannot lock out the owner. + const signInLimit = rateLimit(Boolean(config.trustProxy), { + maxRequests: 30, + message: "Too many sign-in attempts. Try again in a minute.", + }); + 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); diff --git a/apps/server/src/rate-limit.ts b/apps/server/src/rate-limit.ts index 95c1f5ff6..2ccf30abc 100644 --- a/apps/server/src/rate-limit.ts +++ b/apps/server/src/rate-limit.ts @@ -72,7 +72,10 @@ 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; + } = {}, ) { const store = createRateLimitStore(options); const getAddress = options.getAddress ?? connectionAddress; @@ -84,7 +87,7 @@ 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(); }; } diff --git a/tests/rate-limit.test.ts b/tests/rate-limit.test.ts index 30ed7871e..605ab4a34 100644 --- a/tests/rate-limit.test.ts +++ b/tests/rate-limit.test.ts @@ -111,6 +111,47 @@ test("unauthenticated requests cannot multiply buckets by spoofing X-Forwarded-F ); }); +test("one client's failed sign-ins do not block another client with the correct key", async () => { + const accessKey = "fictional-owner-access-key"; + 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, + }); + const signIn = (client: string, key: string) => + live.request("/api/session", { + method: "POST", + headers: { "Content-Type": "application/json", "X-Forwarded-For": client }, + body: JSON.stringify({ accessKey: key }), + }); + for (let i = 0; i < 30; i++) + assert.equal((await signIn("203.0.113.1", "wrong-guess")).status, 401); + const blocked = await signIn("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("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"); + } finally { + await liveDb.close(); + await rm(liveDirectory, { recursive: true, force: true }); + } +}); + function limitedApp(options: Parameters[1]) { const app = new Hono(); app.onError((error, c) => From cf084b8865111e4cbf0c5946e2264c4db6a3b705 Mon Sep 17 00:00:00 2001 From: MustafaYamin Date: Mon, 2 Nov 2026 22:02:38 +0500 Subject: [PATCH 2/2] fix(server): count only rejected sign-in guesses --- SECURITY.md | 2 ++ apps/server/src/app.ts | 3 +- apps/server/src/rate-limit.ts | 10 ++++++- tests/rate-limit.test.ts | 56 +++++++++++++++++++++++++---------- 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 8065c0393..b0ed3f3c0 100644 --- a/SECURITY.md +++ b/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). diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index d6c325543..41f3b8d75 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -104,10 +104,11 @@ export async function createApp( browserConfigured: Boolean(config.workerUrl && config.workerToken), }), ); - // Counted per client so one caller's failed guesses cannot lock out the owner. + // 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()); diff --git a/apps/server/src/rate-limit.ts b/apps/server/src/rate-limit.ts index 2ccf30abc..e4eb969b4 100644 --- a/apps/server/src/rate-limit.ts +++ b/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 @@ -75,6 +80,8 @@ export function rateLimit( 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); @@ -89,5 +96,6 @@ export function rateLimit( if (!store.take(key, Date.now())) 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()); }; } diff --git a/tests/rate-limit.test.ts b/tests/rate-limit.test.ts index 605ab4a34..1863cca96 100644 --- a/tests/rate-limit.test.ts +++ b/tests/rate-limit.test.ts @@ -111,8 +111,9 @@ test("unauthenticated requests cannot multiply buckets by spoofing X-Forwarded-F ); }); -test("one client's failed sign-ins do not block another client with the correct key", async () => { - const accessKey = "fictional-owner-access-key"; +const accessKey = "fictional-owner-access-key"; + +async function withLiveApp(run: (live: typeof app) => Promise) { const liveDirectory = await mkdtemp(join(tmpdir(), "openmuse-signin-")); const liveDb = await createStore(); try { @@ -125,20 +126,35 @@ test("one client's failed sign-ins do not block another client with the correct encryptionKey: Buffer.alloc(32, 1).toString("base64"), trustProxy: true, }); - const signIn = (client: string, key: string) => - live.request("/api/session", { - method: "POST", - headers: { "Content-Type": "application/json", "X-Forwarded-For": client }, - body: JSON.stringify({ accessKey: key }), - }); + 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("203.0.113.1", "wrong-guess")).status, 401); - const blocked = await signIn("203.0.113.1", "wrong-guess"); + 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("203.0.113.2", accessKey); + 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", { @@ -146,10 +162,20 @@ test("one client's failed sign-ins do not block another client with the correct 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"); - } finally { - await liveDb.close(); - await rm(liveDirectory, { recursive: true, force: true }); - } + }); +}); + +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[1]) {