From 241903888445fc5b16a9c099969d8dfad5b1fade Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:48:27 +0100 Subject: [PATCH] fix(auth): rate-limit login per IP, 12-character passwords, loopback default - POST /api/auth/login: 10 attempts per minute per TCP peer IP (X-Forwarded-For ignored), then 429 with Retry-After. The block starts at 1 minute and doubles on each attempt while blocked and on each new block, capped at 15 minutes. A successful login clears the IP. No per-account lockout. In-memory, idle entries swept after 15 minutes, at most 10,000 IPs. - New passwords (init, change-password, /setup page) need 12 characters. Login does not check length, so existing 8-character passwords work. - The binary's -addr default is 127.0.0.1:8787. The Docker image passes -addr :8787 explicitly and is unchanged. - GET /api/health returns {"ok":true} without a valid token; with one it still returns service and version for the web UI top bar. --- CHANGELOG.md | 10 ++ cmd/artex/main.go | 2 +- server/auth.go | 23 +++- server/auth_test.go | 9 +- server/login_limit.go | 126 ++++++++++++++++++ server/login_limit_test.go | 210 ++++++++++++++++++++++++++++++ server/server.go | 12 +- web/src/app/(auth)/setup/page.tsx | 8 +- 8 files changed, 388 insertions(+), 12 deletions(-) create mode 100644 server/login_limit.go create mode 100644 server/login_limit_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c9d6bec..440c8c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] +### Changed + +- **The binary listens on `127.0.0.1:8787` by default**: the `-addr` default was `:8787`, every interface. Running `artex` or `./start.sh` without `-addr` now accepts connections from the local machine only; pass `-addr :8787` to listen on every interface. Docker is unchanged: the image starts the server with `-addr :8787` inside the container, and `docker-compose.yml` already publishes it on `127.0.0.1` by default (`ARTEX_BIND_ADDR`). +- **New passwords need at least 12 characters** (was 8), at first-run setup (`POST /api/auth/init` and the `/setup` page) and in `POST /api/auth/change-password`. Login does not check length, so a password set under the old minimum keeps working until it is changed. `reset-password.sh` does not check length. + ### Fixed - **The JWT signing key survives container rebuilds**: the server wrote `jwt.key` to the executable's directory, `/app` in the image, so the key lived in the container's writable layer and recreating the container (for example `docker compose up -d --build`) generated a new key and signed every user out. The key directory is now set by `ARTEX_JWT_KEY_DIR` (default: the executable's directory, unchanged for non-Docker installs). `docker-compose.yml` sets it to `/app/keys` and bind-mounts `./keys` there, outside `./data`, which the file manager browses. On start, when `/jwt.key` is missing, a key at `data/jwt.key`, then at the executable's directory, is copied there with mode 0600 and the old file is removed; otherwise a new key is generated. An existing key is never overwritten. Upgrading a Docker install signs every user out once, because the old key is in the container being replaced; to keep sessions, copy it out before recreating the container: @@ -13,6 +18,11 @@ All notable changes to this project are documented in this file. The format is b docker cp artex-artex-1:/app/jwt.key ./keys/jwt.key ``` +### Security + +- **Login rate limit**: `POST /api/auth/login` allows 10 attempts per minute per client IP. The next attempt gets 429 with a `Retry-After` header (seconds) and a JSON error, and the IP is blocked for 1 minute. Each attempt while blocked, and each later block, doubles the wait, up to 15 minutes. A successful login clears the IP's count. The IP is the TCP peer address; `X-Forwarded-For` and similar headers are ignored, because the client controls them. There is no per-account lockout, so failing logins cannot lock the admin out from other addresses. The state is in memory: a restart clears it, an IP idle for 15 minutes is dropped, and at most 10,000 IPs are tracked (the least recently seen is dropped first). Behind a reverse proxy every client has the proxy's address and shares one budget, so an attacker failing logins through the proxy also blocks the real users behind it. +- **`GET /api/health` hides the version from anonymous callers**: without a valid token it answers `{"ok":true}` only. With a token (the web UI sends one for the version in its top bar) the answer is unchanged: `ok`, `service` and `version`. + ## [0.5.0] - 2026-11-04 Ollama support, a setup token for the first-run claim, secret scrubbing for child processes, a Docker image that builds from a fresh clone, Go 1.26.10 and Node 22, and race and shellcheck runs in CI. Entries marked "direct commit" landed on `main` without a pull request. diff --git a/cmd/artex/main.go b/cmd/artex/main.go index 028db15..602c934 100644 --- a/cmd/artex/main.go +++ b/cmd/artex/main.go @@ -50,7 +50,7 @@ func main() { func run() int { var ( - addr = flag.String("addr", ":8787", "HTTP listen address") + addr = flag.String("addr", "127.0.0.1:8787", "HTTP listen address") dataDir = flag.String("data", filepath.Join(config.BaseDir(), "data"), "data directory for SQLite stores (default: data/ next to the executable)") proxy = flag.String("proxy", "127.0.0.1:8788", "traffic recording proxy address (empty to disable)") ) diff --git a/server/auth.go b/server/auth.go index 4242d22..4b646a9 100644 --- a/server/auth.go +++ b/server/auth.go @@ -10,6 +10,7 @@ import ( "net/http" "os" "path/filepath" + "strconv" "strings" "time" "unicode/utf8" @@ -29,7 +30,9 @@ const ( // The lower bound matches the setup page's frontend validation — validation that // lives only in the frontend is as good as none: hitting the API directly bypasses // it. The upper bound is bcrypt's hard limit: past 72 bytes GenerateFromPassword returns ErrPasswordTooLong; rejecting early beats a cryptic "password encryption failed". - minPasswordRunes = 8 + // Both apply to new passwords only (init, change-password); login never checks + // length, so passwords set under the old 8-character minimum still log in. + minPasswordRunes = 12 maxPasswordBytes = 72 ) @@ -372,6 +375,21 @@ func (s *Server) authLogin(w http.ResponseWriter, r *http.Request) { if pg == nil { return } + s.login(w, r, pg.GetSetting) +} + +// login is authLogin past the data-source check; getSetting is pg.GetSetting, +// a parameter so tests can run it without PostgreSQL. Every attempt counts +// against the caller's IP (s.loginLimit) and a successful one clears it. There +// is no per-account lockout and no password length check here. +func (s *Server) login(w http.ResponseWriter, r *http.Request, getSetting func(string) (string, bool, error)) { + ip := peerIP(r) + if wait := s.loginLimit.allow(ip); wait > 0 { + secs := int((wait + time.Second - 1) / time.Second) + w.Header().Set("Retry-After", strconv.Itoa(secs)) + writeErr(w, 429, fmt.Sprintf("Too many login attempts, try again in %d seconds", secs)) + return + } var req struct { Username string `json:"username"` Password string `json:"password"` @@ -384,7 +402,7 @@ func (s *Server) authLogin(w http.ResponseWriter, r *http.Request) { writeErr(w, 401, "Incorrect username or password") return } - hash, ok, err := pg.GetSetting(authPassKey) + hash, ok, err := getSetting(authPassKey) if err != nil { writeErr(w, 503, errDataSourceUnavailable) return @@ -397,6 +415,7 @@ func (s *Server) authLogin(w http.ResponseWriter, r *http.Request) { writeErr(w, 401, "Incorrect username or password") return } + s.loginLimit.reset(ip) tok, err := signJWT(s.jwtKey) if err != nil { writeErr(w, 500, "token generation failed") diff --git a/server/auth_test.go b/server/auth_test.go index 7067992..5975377 100644 --- a/server/auth_test.go +++ b/server/auth_test.go @@ -302,10 +302,11 @@ func TestValidatePassword(t *testing.T) { wantErr bool }{ {"empty", "", true}, - {"seven chars", "1234567", true}, - {"eight chars", "12345678", false}, - {"eight multibyte chars counted as runes not bytes", strings.Repeat("€", 8), false}, - {"three chars reach 9 bytes but only 3 runes", strings.Repeat("€", 3), true}, + {"eight chars, the old minimum", "12345678", true}, + {"eleven chars", "12345678901", true}, + {"twelve chars", "123456789012", false}, + {"twelve multibyte chars counted as runes not bytes", strings.Repeat("€", 12), false}, + {"four chars reach 12 bytes but only 4 runes", strings.Repeat("€", 4), true}, {"72 bytes", strings.Repeat("a", 72), false}, {"73 bytes exceeds the bcrypt cap", strings.Repeat("a", 73), true}, } { diff --git a/server/login_limit.go b/server/login_limit.go new file mode 100644 index 0000000..dc65fdc --- /dev/null +++ b/server/login_limit.go @@ -0,0 +1,126 @@ +package server + +import ( + "net" + "net/http" + "sync" + "time" +) + +// Login attempts are limited per client IP, never per account: a per-account +// lockout would let anyone lock the admin out by failing on purpose. +const ( + loginMaxAttempts = 10 // attempts allowed per IP per window + loginWindow = time.Minute // the counting window + loginBaseBlock = time.Minute // length of the first block + loginMaxBlock = 15 * time.Minute // cap on the block; an entry idle this long is dropped + loginSweepEvery = time.Minute // how often stale entries are swept + loginMaxEntries = 10000 // hard cap on tracked IPs +) + +type loginEntry struct { + attempts int + windowStart time.Time + blockedUntil time.Time + block time.Duration // current block length; doubles on every new block or blocked attempt + lastSeen time.Time +} + +// loginLimiter counts login attempts per IP. Past loginMaxAttempts in one +// loginWindow the IP is blocked for loginBaseBlock; every attempt while blocked, +// and every later block, doubles the block up to loginMaxBlock. State is in +// memory only and swept of idle entries, so a restart clears it. +type loginLimiter struct { + mu sync.Mutex + now func() time.Time + entries map[string]*loginEntry + lastSweep time.Time +} + +func newLoginLimiter(now func() time.Time) *loginLimiter { + return &loginLimiter{now: now, entries: map[string]*loginEntry{}} +} + +// allow records one attempt from ip. It returns 0 when the attempt may go on, +// otherwise how long ip must wait. +func (l *loginLimiter) allow(ip string) time.Duration { + l.mu.Lock() + defer l.mu.Unlock() + now := l.now() + l.sweep(now) + e := l.entries[ip] + if e == nil { + if len(l.entries) >= loginMaxEntries { + l.evictOldest() + } + e = &loginEntry{windowStart: now} + l.entries[ip] = e + } + e.lastSeen = now + if now.Before(e.blockedUntil) { + e.block = min(2*e.block, loginMaxBlock) + e.blockedUntil = now.Add(e.block) + return e.block + } + if now.Sub(e.windowStart) >= loginWindow { + e.attempts, e.windowStart = 0, now + } + if e.attempts >= loginMaxAttempts { + if e.block == 0 { + e.block = loginBaseBlock + } else { + e.block = min(2*e.block, loginMaxBlock) + } + e.blockedUntil = now.Add(e.block) + e.attempts, e.windowStart = 0, now + return e.block + } + e.attempts++ + return 0 +} + +// reset forgets ip, after a successful login. A blocked IP never reaches the +// password check, so this only runs outside a block. +func (l *loginLimiter) reset(ip string) { + l.mu.Lock() + defer l.mu.Unlock() + delete(l.entries, ip) +} + +// sweep drops entries idle for loginMaxBlock, at most once per loginSweepEvery. +// Such an entry is never blocked: a block ends at most loginMaxBlock after the +// attempt that set it. +func (l *loginLimiter) sweep(now time.Time) { + if now.Sub(l.lastSweep) < loginSweepEvery { + return + } + l.lastSweep = now + for ip, e := range l.entries { + if now.Sub(e.lastSeen) >= loginMaxBlock { + delete(l.entries, ip) + } + } +} + +// evictOldest drops the least recently seen entry to stay under loginMaxEntries. +func (l *loginLimiter) evictOldest() { + var oldest string + var at time.Time + for ip, e := range l.entries { + if oldest == "" || e.lastSeen.Before(at) { + oldest, at = ip, e.lastSeen + } + } + delete(l.entries, oldest) +} + +// peerIP is the host part of the TCP peer address. X-Forwarded-For and similar +// headers are ignored on purpose: the client sets them, so trusting them would +// give an attacker a fresh bucket per request. +func peerIP(r *http.Request) string { + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return r.RemoteAddr + } + return host +} diff --git a/server/login_limit_test.go b/server/login_limit_test.go new file mode 100644 index 0000000..6f5b285 --- /dev/null +++ b/server/login_limit_test.go @@ -0,0 +1,210 @@ +package server + +import ( + "encoding/json" + "fmt" + "net/http/httptest" + "strings" + "testing" + "time" + + "golang.org/x/crypto/bcrypt" +) + +type fakeClock struct{ t time.Time } + +func (c *fakeClock) now() time.Time { return c.t } +func (c *fakeClock) advance(d time.Duration) { c.t = c.t.Add(d) } + +func newFakeClock() *fakeClock { return &fakeClock{t: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)} } + +// oldPassword is 8 characters: valid under the old minimum, too short for a new password now. +const oldPassword = "abcd1234" + +// loginServer returns a Server whose login reads a stored hash of oldPassword +// from a stub instead of PostgreSQL. +func loginServer(t *testing.T, clk *fakeClock) (*Server, func(remote, password string, hdr map[string]string) *httptest.ResponseRecorder) { + t.Helper() + hash, err := bcrypt.GenerateFromPassword([]byte(oldPassword), bcrypt.MinCost) + if err != nil { + t.Fatal(err) + } + getSetting := func(key string) (string, bool, error) { + if key != authPassKey { + return "", false, nil + } + return string(hash), true, nil + } + s := &Server{jwtKey: []byte("0123456789abcdef0123456789abcdef"), loginLimit: newLoginLimiter(clk.now)} + post := func(remote, password string, hdr map[string]string) *httptest.ResponseRecorder { + body := fmt.Sprintf(`{"username":"ARTEX","password":%q}`, password) + r := httptest.NewRequest("POST", "/api/auth/login", strings.NewReader(body)) + r.RemoteAddr = remote + for k, v := range hdr { + r.Header.Set(k, v) + } + w := httptest.NewRecorder() + s.login(w, r, getSetting) + return w + } + return s, post +} + +func failN(t *testing.T, post func(string, string, map[string]string) *httptest.ResponseRecorder, remote string, n int) { + t.Helper() + for i := 0; i < n; i++ { + if w := post(remote, "wrong-password", nil); w.Code != 401 { + t.Fatalf("attempt %d from %s: status=%d want 401; body=%s", i+1, remote, w.Code, w.Body.String()) + } + } +} + +func TestLoginRateLimitReturns429WithRetryAfter(t *testing.T) { + clk := newFakeClock() + _, post := loginServer(t, clk) + const ip = "192.0.2.10:40000" + failN(t, post, ip, loginMaxAttempts) + + w := post(ip, oldPassword, nil) // even the right password is refused while blocked + if w.Code != 429 { + t.Fatalf("status=%d want 429; body=%s", w.Code, w.Body.String()) + } + if got := w.Header().Get("Retry-After"); got != "60" { + t.Fatalf("Retry-After=%q want 60", got) + } + var body struct { + Error string `json:"error"` + Token string `json:"token"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil || body.Error == "" || body.Token != "" { + t.Fatalf("want a JSON error body without a token, got %s (err=%v)", w.Body.String(), err) + } + + // Each attempt while blocked doubles the wait, capped at loginMaxBlock. + for _, want := range []string{"120", "240", "480", "900", "900"} { + if got := post(ip, "wrong-password", nil).Header().Get("Retry-After"); got != want { + t.Fatalf("Retry-After=%q want %s", got, want) + } + } + + // Once the block runs out the IP gets a fresh window. + clk.advance(loginMaxBlock) + if w := post(ip, oldPassword, nil); w.Code != 200 { + t.Fatalf("after the block: status=%d want 200; body=%s", w.Code, w.Body.String()) + } +} + +func TestLoginSuccessResetsCounter(t *testing.T) { + clk := newFakeClock() + _, post := loginServer(t, clk) + const ip = "192.0.2.11:40000" + failN(t, post, ip, loginMaxAttempts-1) + if w := post(ip, oldPassword, nil); w.Code != 200 { + t.Fatalf("status=%d want 200; body=%s", w.Code, w.Body.String()) + } + // The counter starts over: a full budget of failures before the next block. + failN(t, post, ip, loginMaxAttempts) + if w := post(ip, "wrong-password", nil); w.Code != 429 { + t.Fatalf("status=%d want 429", w.Code) + } +} + +func TestLoginRateLimitIsPerIP(t *testing.T) { + clk := newFakeClock() + _, post := loginServer(t, clk) + failN(t, post, "192.0.2.12:40000", loginMaxAttempts) + if w := post("192.0.2.12:40001", "wrong-password", nil); w.Code != 429 { + t.Fatalf("same IP, other port: status=%d want 429", w.Code) + } + if w := post("198.51.100.7:40000", "wrong-password", nil); w.Code != 401 { + t.Fatalf("other IP: status=%d want 401", w.Code) + } + if w := post("[2001:db8::1]:40000", oldPassword, nil); w.Code != 200 { + t.Fatalf("other IP: status=%d want 200", w.Code) + } +} + +func TestLoginRateLimitIgnoresForwardedFor(t *testing.T) { + clk := newFakeClock() + _, post := loginServer(t, clk) + const blocked = "192.0.2.13" + failN(t, post, blocked+":40000", loginMaxAttempts) + + // A blocked peer cannot escape by claiming another address. + for _, h := range []string{"X-Forwarded-For", "X-Real-IP"} { + if w := post(blocked+":40000", oldPassword, map[string]string{h: "203.0.113.9"}); w.Code != 429 { + t.Fatalf("%s from blocked peer: status=%d want 429", h, w.Code) + } + } + // And another peer naming the blocked address is not blocked. + if w := post("198.51.100.8:40000", oldPassword, map[string]string{"X-Forwarded-For": blocked}); w.Code != 200 { + t.Fatalf("X-Forwarded-For naming the blocked IP: status=%d want 200", w.Code) + } +} + +// The 12-character minimum applies to new passwords only. A hash stored under the +// old 8-character minimum must still log in. +func TestEightCharPasswordLogsInButIsRejectedAsNew(t *testing.T) { + if n := len([]rune(oldPassword)); n != 8 { + t.Fatalf("oldPassword has %d characters, want 8", n) + } + _, post := loginServer(t, newFakeClock()) + w := post("192.0.2.14:40000", oldPassword, nil) + if w.Code != 200 || !strings.Contains(w.Body.String(), `"token"`) { + t.Fatalf("login with an existing 8-character password: status=%d body=%s", w.Code, w.Body.String()) + } + // authInit and authChangePassword both refuse a new password through validatePassword. + if msg := validatePassword(oldPassword); msg == "" { + t.Fatal("validatePassword accepted an 8-character new password") + } +} + +func TestLoginLimiterSweepsIdleEntries(t *testing.T) { + clk := newFakeClock() + l := newLoginLimiter(clk.now) + l.allow("192.0.2.20") + clk.advance(loginMaxBlock - time.Second) + l.allow("192.0.2.21") // sweeps, but .20 is not idle long enough yet + if len(l.entries) != 2 { + t.Fatalf("entries=%d want 2 before either is idle for %v", len(l.entries), loginMaxBlock) + } + clk.advance(loginSweepEvery) + l.allow("192.0.2.22") // next sweep drops .20 + if _, ok := l.entries["192.0.2.20"]; ok || len(l.entries) != 2 { + t.Fatalf("idle entry not swept: %d entries", len(l.entries)) + } +} + +func TestLoginLimiterCapsEntries(t *testing.T) { + l := newLoginLimiter(newFakeClock().now) + for i := 0; i <= loginMaxEntries; i++ { + l.allow(fmt.Sprintf("10.%d.%d.%d", i>>16&255, i>>8&255, i&255)) + } + if len(l.entries) != loginMaxEntries { + t.Fatalf("entries=%d want the cap %d", len(l.entries), loginMaxEntries) + } +} + +func TestHealthHidesVersionWithoutToken(t *testing.T) { + key := []byte("0123456789abcdef0123456789abcdef") + s := &Server{jwtKey: key} + + w := httptest.NewRecorder() + s.health(w, httptest.NewRequest("GET", "/api/health", nil)) + if got := strings.TrimSpace(w.Body.String()); w.Code != 200 || got != `{"ok":true}` { + t.Fatalf("no token: status=%d body=%s want {\"ok\":true}", w.Code, got) + } + + tok, err := signJWT(key) + if err != nil { + t.Fatal(err) + } + r := httptest.NewRequest("GET", "/api/health", nil) + r.Header.Set("Authorization", "Bearer "+tok) + w = httptest.NewRecorder() + s.health(w, r) + var body map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil || body["version"] != BuildVersion || body["service"] != "artex" { + t.Fatalf("valid token: body=%s want service and version", w.Body.String()) + } +} diff --git a/server/server.go b/server/server.go index c74ecba..6f69ffb 100644 --- a/server/server.go +++ b/server/server.go @@ -51,6 +51,8 @@ type Server struct { setupMu sync.Mutex setupToken string // one-time token POST /api/auth/init requires; empty when none is active + loginLimit *loginLimiter // per-IP attempt limit on POST /api/auth/login + // concMu serializes concurrency-cap decisions (admission + reconcile) so a // scheduler tick and an HTTP settings change / task creation can't both count // free slots off the same snapshot and over-promote past the limit. @@ -155,7 +157,8 @@ func New(ctx context.Context, m *Manager, skillDir string, dataDir string, keyDi triggerActive: map[string]int{}, triggerCfg: map[string]triggerBehavior{}, profChatAgents: map[int64]*agent.ChatAgent{}, provByProfile: map[int64]*provEntry{}, llmHealth: newLLMHealthRegistry(m.pg), - taskAgents: map[string]*taskAgentBundle{}, archiveWake: make(chan struct{}, 1)} + taskAgents: map[string]*taskAgentBundle{}, archiveWake: make(chan struct{}, 1), + loginLimit: newLoginLimiter(time.Now)} for _, o := range opts { o(s) } @@ -964,7 +967,14 @@ func (s *Server) Handler() http.Handler { // --- handlers --- +// health answers without a token (requireAuth exempts it), but only a caller +// with a valid token gets the service name and version; the web UI's top bar +// reads the version, and sends its token. func (s *Server) health(w http.ResponseWriter, r *http.Request) { + if !verifyJWT(extractToken(r), s.jwtKey) { + writeJSON(w, 200, map[string]any{"ok": true}) + return + } writeJSON(w, 200, map[string]any{"ok": true, "service": "artex", "version": BuildVersion}) } diff --git a/web/src/app/(auth)/setup/page.tsx b/web/src/app/(auth)/setup/page.tsx index db57917..0df08a7 100644 --- a/web/src/app/(auth)/setup/page.tsx +++ b/web/src/app/(auth)/setup/page.tsx @@ -41,8 +41,8 @@ export default function SetupPage() { setError("The two passwords do not match"); return; } - if (password.length < 8) { - setError("Password must be at least 8 characters"); + if (password.length < 12) { + setError("Password must be at least 12 characters"); return; } setLoading(true); @@ -81,7 +81,7 @@ export default function SetupPage() {

{unavailable ? "The backend or database is temporarily unavailable. To avoid overwriting a password already set on this instance, setup is temporarily closed — restore the service and try again." - : "First time using ARTEX? Enter the setup token from the server log, then set a login password for the account (at least 8 characters)"} + : "First time using ARTEX? Enter the setup token from the server log, then set a login password for the account (at least 12 characters)"}

{unavailable ? ( @@ -111,7 +111,7 @@ export default function SetupPage() { type="password" value={password} onChange={(e) => setPassword(e.target.value)} - placeholder="At least 8 characters" + placeholder="At least 12 characters" autoComplete="new-password" />