From 17d6be9fe1fc644cc40c810df51b25b296289169 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 00:37:57 +0000 Subject: [PATCH 1/6] HTTP API: server, credential and the list of chats (closes #4) The bot now serves an HTTP API on PORT (default 8080) beside the chat client. Every request needs the credential read at startup from the file named by API_TOKEN_FILE, sent as a bearer token; without one, every request is refused. Responses carry the security headers, bodies are capped at 64 KiB and each request's work at 10 seconds. GET /api/v1/chats lists the bot's contacts from the chat client's /_contacts command, ordered by id, and marks the contacts who deleted their chat with the bot, which the chat client keeps listing. bot.Run starts the API after set-up and stops it within 5 seconds; a listener failure ends the bot as a chat client failure does. Model: opus-5-5 --- AGENTS.md | 3 +- Dockerfile | 4 + README.md | 84 ++++++++++++- docs/TODO.md | 3 + go.mod | 1 + go.sum | 2 + internal/api/api.go | 180 +++++++++++++++++++++++++++ internal/api/api_test.go | 214 ++++++++++++++++++++++++++++++++ internal/api/chats.go | 46 +++++++ internal/api/chats_test.go | 64 ++++++++++ internal/bot/bot.go | 78 +++++++++--- internal/cli/run.go | 2 +- internal/config/config.go | 82 +++++++++++- internal/config/config_test.go | 94 +++++++++++++- internal/simplex/client.go | 12 ++ internal/simplex/client_test.go | 46 +++++++ internal/simplex/protocol.go | 28 ++++- 17 files changed, 908 insertions(+), 35 deletions(-) create mode 100644 internal/api/api.go create mode 100644 internal/api/api_test.go create mode 100644 internal/api/chats.go create mode 100644 internal/api/chats_test.go diff --git a/AGENTS.md b/AGENTS.md index 67def55..6fc879a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ --- title: Agent Guidance -last_modified: 2026-09-28 +last_modified: 2026-09-29 --- This file is the single source of guidance for any automated agent @@ -119,6 +119,7 @@ Do not weaken them. ``` cmd/simplexcalc/ main(), a single call into internal/cli +internal/api/ the HTTP API: its credential, headers and endpoints internal/bot/ startup, address setup, and the reply to a message internal/calc/ the arithmetic: go/parser and go/constant internal/cli/ cobra command tree: run and version diff --git a/Dockerfile b/Dockerfile index 3238e1a..4e01b89 100644 --- a/Dockerfile +++ b/Dockerfile @@ -119,4 +119,8 @@ USER simplexcalc ENV DATA_DIR=/var/lib/simplexcalc +# The API, on its default PORT. The chat client's WebSocket on 5225 is +# not exposed: it has no authentication. +EXPOSE 8080 + CMD ["/app/simplexcalc", "run"] diff --git a/README.md b/README.md index 80aa58b..2d91f6a 100644 --- a/README.md +++ b/README.md @@ -11,18 +11,28 @@ else gets a short explanation instead of a result. ## Getting Started -Build the image and run the bot, with its SimpleX profile on a named -volume: +Build the image, write a random API credential onto a named volume, and +run the bot with its SimpleX profile on the same volume: ```sh git clone git@git.eeqj.de:clawbot/simplexcalc.git cd simplexcalc make docker +docker run --rm -v simplexcalc-data:/var/lib/simplexcalc simplexcalc \ + sh -c 'umask 077 && od -An -N32 -tx1 /dev/urandom | tr -d " \n" \ + >/var/lib/simplexcalc/api-token' docker run -d --name simplexcalc --restart unless-stopped \ - -v simplexcalc-data:/var/lib/simplexcalc simplexcalc + -v simplexcalc-data:/var/lib/simplexcalc \ + -p 127.0.0.1:8080:8080 -e API_TOKEN_FILE=/var/lib/simplexcalc/api-token \ + simplexcalc docker logs simplexcalc 2>&1 | grep '"msg":"ready"' ``` +The credential is 32 random bytes written as 64 hexadecimal characters +to `/var/lib/simplexcalc/api-token`, readable only by the bot's user. +`-p 127.0.0.1:8080:8080` makes the API reachable from this host only; +see [API](#api). + The `ready` log line carries the bot's contact address: `address` is the short link to share, and `full_address` is the same address in the long form that older SimpleX clients need. Open the link in any SimpleX Chat @@ -67,6 +77,60 @@ variables that are absent. `/var/lib/simplexcalc`. - `DEBUG` — `true` or `false`, default `false`. `true` logs every event the chat client sends. +- `PORT` — the TCP port the API listens on, on all interfaces: a whole + number from 1 to 65535, default `8080`. +- `API_TOKEN_FILE` — path of a file holding the API credential, at least + 32 characters not counting the whitespace around them. The file is + read once, at startup; one that cannot be read, or holds a shorter + credential, aborts startup. Absent, the API still listens but refuses + every request, and startup logs a warning saying so. + +## API + +An HTTP API beside the chat client lets another program read the bot's +chats. It speaks JSON on `PORT`. + +**Authentication.** Every request carries the credential from +`API_TOKEN_FILE`: + +``` +Authorization: Bearer {credential} +``` + +A request without it, or with a wrong one, gets `401` with +`WWW-Authenticate: Bearer` and `{"error":"unauthorized"}`. No path is +exempt. Without `API_TOKEN_FILE`, every request gets that answer. The +file is read at startup, so a new credential takes a restart. + +**Errors** are JSON with a short explanation, such as +`{"error":"not found"}`; what went wrong inside goes to the bot's log. + +### `GET /api/v1/chats` + +The bot's chats, ordered by `id`. The bot talks to people only one to +one, so each chat is one of its contacts. + +```sh +TOKEN=$(docker exec simplexcalc cat /var/lib/simplexcalc/api-token) +curl -H "Authorization: Bearer $TOKEN" http://127.0.0.1:8080/api/v1/chats +``` + +```json +{ + "chats": [ + { "id": 3, "display_name": "tester", "contact_deleted": false }, + { "id": 4, "display_name": "tester", "contact_deleted": true } + ] +} +``` + +- `id` — the chat's number, which is its contact's number in the chat + client. +- `display_name` — the name the contact gave themselves. Nothing makes + it unique. +- `contact_deleted` — `true` once the contact has deleted their chat + with the bot. The chat stays in the list, but nothing more reaches + them. ## Entrypoints @@ -142,8 +206,8 @@ container. - **One process tree, one container.** `simplexcalc run` starts the SimpleX Chat command-line client, `simplex-chat`, as a child process, with its database under `$DATA_DIR/simplex` and its WebSocket API on - `127.0.0.1:5225`. The API has no authentication, which is why it is - never exposed outside the container. + `127.0.0.1:5225`. That WebSocket has no authentication, which is why + it is never exposed outside the container. - **The protocol** (`internal/simplex`) is JSON over that WebSocket: a command carries a correlation id, its response carries the same id, and anything without one is an event. Only the fields the bot reads @@ -154,6 +218,16 @@ container. accept every contact request and to greet each new contact. The first start creates the profile itself, a bot profile named `calc`. The address is logged in the `ready` line. +- **The API** (`internal/api`): an HTTP server, routed with chi, that + starts once set-up is done; if it cannot listen, the bot exits with an + error, as when the chat client fails. Every request must carry the + credential, compared in constant time; every response carries headers + that forbid framing, content sniffing, caching and referrers; a + request body is capped at 64 KiB and a request's work at 10 seconds. + Handlers call the chat client on the request's own goroutine, never on + the one that delivers events, which also delivers the chat client's + answers. When the bot stops, requests in progress get 5 seconds to + finish. - **Replies**: for each text message a contact sends in a direct chat, the bot sends back the result, as a reply quoting the message. Group messages, files and the bot's own messages are ignored. diff --git a/docs/TODO.md b/docs/TODO.md index 57eef19..bd8496f 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -27,6 +27,9 @@ with no deprecation warning. # Completed Steps +- 2026-09-29 Added the HTTP API in `internal/api`: the server on `PORT`, + the bearer credential from `API_TOKEN_FILE`, security headers and + limits, and `GET /api/v1/chats` - 2026-09-28 Moved the command tree and the `run` and `version` commands from `cmd/simplexcalc/` into `internal/cli`; `cmd/simplexcalc/main.go` is now a single call to `cli.Main` diff --git a/go.mod b/go.mod index f0c9abc..4e77682 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module sneak.berlin/go/simplexcalc go 1.25.0 require ( + github.com/go-chi/chi/v5 v5.3.2 github.com/gorilla/websocket v1.5.3 github.com/joho/godotenv v1.5.1 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum index 1fd508c..0bcca44 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,8 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= +github.com/go-chi/chi/v5 v5.3.2 h1:5YQkICvTCSZ25hoRsyJazN0scjzKGiu4VAUc7H1o1nY= +github.com/go-chi/chi/v5 v5.3.2/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= diff --git a/internal/api/api.go b/internal/api/api.go new file mode 100644 index 0000000..bbf0aad --- /dev/null +++ b/internal/api/api.go @@ -0,0 +1,180 @@ +// Package api is the bot's HTTP API, through which another program +// reads the bot's chats. +// +// Every request must carry the credential, as "Authorization: Bearer +// {credential}"; with no credential configured, every request is +// refused. No path is exempt. +// +// Handlers call the chat client on the request's own goroutine. Doing +// so from the chat client's event handler would wait forever, since +// that goroutine also delivers the responses (see simplex.EventHandler). +package api + +import ( + "context" + "crypto/subtle" + "encoding/json" + "log/slog" + "net/http" + "strconv" + "strings" + "time" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5/middleware" + "sneak.berlin/go/simplexcalc/internal/simplex" +) + +const ( + // maxBodyBytes caps a request body. + maxBodyBytes = 64 << 10 + + // requestTimeout bounds the work behind one request, which is + // mostly waiting for the chat client. + requestTimeout = 10 * time.Second + + // Limits on clients that send or read slowly. writeTimeout outlasts + // requestTimeout, so a handler that ran out of time can still + // answer. + readHeaderTimeout = 5 * time.Second + readTimeout = 10 * time.Second + writeTimeout = requestTimeout + 5*time.Second + idleTimeout = 60 * time.Second +) + +// ChatClient is the part of the chat client the API uses. +// *simplex.Client provides it. +type ChatClient interface { + // Contacts returns the contacts of the user userID. + Contacts(ctx context.Context, userID int64) ([]simplex.Contact, error) +} + +// Params configures New. +type Params struct { + Log *slog.Logger + + // Client is the chat client, and UserID the bot's user profile in + // it. + Client ChatClient + UserID int64 + + // Port is the TCP port to listen on, on all interfaces. + Port int + + // Token is the credential every request must carry. Empty refuses + // every request. + Token string +} + +// New returns the API's server. The caller starts it with +// ListenAndServe and stops it with Shutdown. +func New(p Params) *http.Server { + if p.Token == "" { + p.Log.Warn("API_TOKEN_FILE is not set, so the API refuses every request") + } + + h := &handlers{log: p.Log, client: p.Client, userID: p.UserID, token: p.Token} + + router := chi.NewRouter() + router.Use(securityHeaders, h.authenticate, + middleware.RequestSize(maxBodyBytes), withTimeout) + router.NotFound(func(w http.ResponseWriter, _ *http.Request) { + h.respondError(w, http.StatusNotFound, "not found") + }) + router.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) { + h.respondError(w, http.StatusMethodNotAllowed, "method not allowed") + }) + router.Route("/api/v1", func(r chi.Router) { + r.Get("/chats", h.handleChats()) + }) + + return &http.Server{ + Addr: ":" + strconv.Itoa(p.Port), + Handler: router, + ReadHeaderTimeout: readHeaderTimeout, + ReadTimeout: readTimeout, + WriteTimeout: writeTimeout, + IdleTimeout: idleTimeout, + // net/http's own messages, such as a handler's panic, go to the + // same JSON log as everything else. + ErrorLog: slog.NewLogLogger(p.Log.Handler(), slog.LevelError), + } +} + +// handlers holds what the handlers share. +type handlers struct { + log *slog.Logger + client ChatClient + userID int64 + token string +} + +// authenticate lets a request through only if it carries the +// credential. With no credential it refuses everything, and must: +// ConstantTimeCompare finds two empty strings equal, so an empty bearer +// would get in. +func (h *handlers) authenticate(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + scheme, credential, _ := strings.Cut(r.Header.Get("Authorization"), " ") + + if h.token == "" || !strings.EqualFold(scheme, "Bearer") || + subtle.ConstantTimeCompare([]byte(credential), []byte(h.token)) != 1 { + w.Header().Set("WWW-Authenticate", "Bearer") + h.respondError(w, http.StatusUnauthorized, "unauthorized") + + return + } + + next.ServeHTTP(w, r) + }) +} + +// respond sends v as the JSON body of a response with status. +func (h *handlers) respond(w http.ResponseWriter, status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + + err := json.NewEncoder(w).Encode(v) + if err != nil { + h.log.Warn("sending a response", "error", err) + } +} + +// respondError sends status with a chosen sentence. An error's own text +// never goes to the client, since it can describe the machine; it goes +// to the log. +func (h *handlers) respondError(w http.ResponseWriter, status int, sentence string) { + h.respond(w, status, struct { + Error string `json:"error"` + }{sentence}) +} + +// securityHeaders go on every response. The API returns JSON to +// programs, so a browser may not frame, sniff, cache or refer from it, +// and must reach it over HTTPS. +func securityHeaders(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + header := w.Header() + header.Set("X-Content-Type-Options", "nosniff") + header.Set("Content-Security-Policy", + "default-src 'none'; frame-ancestors 'none'") + header.Set("X-Frame-Options", "DENY") + header.Set("Referrer-Policy", "no-referrer") + header.Set("Strict-Transport-Security", + "max-age=31536000; includeSubDomains") + header.Set("Cache-Control", "no-store") + + next.ServeHTTP(w, r) + }) +} + +// withTimeout ends each request's context after requestTimeout, so a +// handler waiting on the chat client gives up and answers. +func withTimeout(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) + defer cancel() + + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} diff --git a/internal/api/api_test.go b/internal/api/api_test.go new file mode 100644 index 0000000..5056e81 --- /dev/null +++ b/internal/api/api_test.go @@ -0,0 +1,214 @@ +package api_test + +import ( + "bytes" + "context" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "sneak.berlin/go/simplexcalc/internal/api" + "sneak.berlin/go/simplexcalc/internal/simplex" +) + +const ( + // credential is what the API under test is configured with. + credential = "a-credential-for-these-tests" //nolint:gosec // G101: invented for tests + bearer = "Bearer " + credential + + chatsPath = "/api/v1/chats" + unauthorized = `{"error":"unauthorized"}` + "\n" +) + +var errChat = errors.New("sqlite: database is locked at /var/lib/simplexcalc") + +// fakeClient stands in for the chat client. It answers with contacts, +// or with err, and remembers what it was asked. +type fakeClient struct { + contacts []simplex.Contact + err error + + userID int64 + hadDeadline bool +} + +func (f *fakeClient) Contacts( + ctx context.Context, userID int64, +) ([]simplex.Contact, error) { + f.userID = userID + _, f.hadDeadline = ctx.Deadline() + + return f.contacts, f.err +} + +func newAPI(token string, client api.ChatClient) *http.Server { + return api.New(api.Params{ + Log: slog.New(slog.DiscardHandler), + Client: client, + UserID: 1, + Port: 8080, + Token: token, + }) +} + +// request sends srv one request, with the Authorization header auth +// unless that is empty. +func request( + t *testing.T, srv *http.Server, method, path, auth string, +) *httptest.ResponseRecorder { + t.Helper() + + req := httptest.NewRequestWithContext(t.Context(), method, path, nil) + if auth != "" { + req.Header.Set("Authorization", auth) + } + + rec := httptest.NewRecorder() + srv.Handler.ServeHTTP(rec, req) + + return rec +} + +// TestCredential: only the configured credential, sent as a bearer, +// gets in. With none configured, nothing does, an empty bearer +// included. +func TestCredential(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + token, auth string + in bool + }{ + "right": {credential, bearer, true}, + "right, scheme in lower case": {credential, "bearer " + credential, true}, + "wrong": {credential, strings.ToUpper(bearer), false}, + "missing": {credential, "", false}, + "another scheme": {credential, "Basic " + credential, false}, + "no scheme": {credential, credential, false}, + "empty bearer": {credential, "Bearer ", false}, + "none configured": {"", bearer, false}, + "none configured, empty bearer": {"", "Bearer ", false}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + rec := request(t, newAPI(tc.token, &fakeClient{}), + http.MethodGet, chatsPath, tc.auth) + + if tc.in { + if rec.Code != http.StatusOK { + t.Errorf("status = %d, want 200", rec.Code) + } + + return + } + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rec.Code) + } + + if got := rec.Header().Get("WWW-Authenticate"); got != "Bearer" { + t.Errorf("WWW-Authenticate = %q, want Bearer", got) + } + + if rec.Body.String() != unauthorized { + t.Errorf("body = %q, want %q", rec.Body.String(), unauthorized) + } + }) + } +} + +// TestNoCredentialWarns: an API without a credential says at startup +// that it refuses every request. +func TestNoCredentialWarns(t *testing.T) { + t.Parallel() + + var logged bytes.Buffer + + api.New(api.Params{ + Log: slog.New(slog.NewJSONHandler(&logged, nil)), + Client: &fakeClient{}, + Port: 8080, + }) + + if !strings.Contains(logged.String(), `"level":"WARN"`) || + !strings.Contains(logged.String(), "API_TOKEN_FILE is not set") { + t.Errorf("log = %q, want a warning that API_TOKEN_FILE is not set", + logged.String()) + } +} + +// TestNoPathIsExempt: an unknown path or method needs the credential +// like everything else, and then gets a JSON error. +func TestNoPathIsExempt(t *testing.T) { + t.Parallel() + + srv := newAPI(credential, &fakeClient{}) + + for _, tc := range []struct { + method, path, auth string + want int + body string + }{ + {http.MethodGet, "/", "", http.StatusUnauthorized, unauthorized}, + { + http.MethodGet, "/.well-known/healthcheck", "", + http.StatusUnauthorized, unauthorized, + }, + { + http.MethodGet, "/api/v1/nothing", bearer, + http.StatusNotFound, `{"error":"not found"}` + "\n", + }, + { + http.MethodPost, chatsPath, bearer, + http.StatusMethodNotAllowed, `{"error":"method not allowed"}` + "\n", + }, + } { + rec := request(t, srv, tc.method, tc.path, tc.auth) + if rec.Code != tc.want || rec.Body.String() != tc.body { + t.Errorf("%s %s: %d %q, want %d %q", tc.method, tc.path, + rec.Code, rec.Body.String(), tc.want, tc.body) + } + } +} + +// TestHeaders: every response, whatever its status, carries the +// security headers, and none lets another origin in. +func TestHeaders(t *testing.T) { + t.Parallel() + + want := map[string]string{ + "X-Content-Type-Options": "nosniff", + "Content-Security-Policy": "default-src 'none'; frame-ancestors 'none'", + "X-Frame-Options": "DENY", + "Referrer-Policy": "no-referrer", + "Strict-Transport-Security": "max-age=31536000; includeSubDomains", + "Cache-Control": "no-store", + } + + for _, tc := range []struct { + client *fakeClient + path, auth string + }{ + {&fakeClient{}, chatsPath, bearer}, + {&fakeClient{}, chatsPath, ""}, + {&fakeClient{}, "/nothing", bearer}, + {&fakeClient{err: errChat}, chatsPath, bearer}, + } { + rec := request(t, newAPI(credential, tc.client), + http.MethodGet, tc.path, tc.auth) + + for key, value := range want { + if got := rec.Header().Get(key); got != value { + t.Errorf("%d response: %s = %q, want %q", rec.Code, key, got, value) + } + } + + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("%d response: Access-Control-Allow-Origin = %q", rec.Code, got) + } + } +} diff --git a/internal/api/chats.go b/internal/api/chats.go new file mode 100644 index 0000000..10c70fe --- /dev/null +++ b/internal/api/chats.go @@ -0,0 +1,46 @@ +package api + +import ( + "cmp" + "net/http" + "slices" +) + +// handleChats lists the bot's chats, ordered by id. The bot talks to +// people only one to one, so its chats are its contacts, and a chat's +// id is its contact's. +func (h *handlers) handleChats() http.HandlerFunc { + type chat struct { + ID int64 `json:"id"` + DisplayName string `json:"display_name"` + ContactDeleted bool `json:"contact_deleted"` + } + + type response struct { + Chats []chat `json:"chats"` + } + + return func(w http.ResponseWriter, r *http.Request) { + contacts, err := h.client.Contacts(r.Context(), h.userID) + if err != nil { + h.log.Error("listing the chats", "error", err) + h.respondError(w, http.StatusInternalServerError, + "the chats could not be read") + + return + } + + chats := make([]chat, 0, len(contacts)) + for _, c := range contacts { + chats = append(chats, chat{ + ID: c.ContactID, + DisplayName: c.Profile.DisplayName, + ContactDeleted: c.Deleted(), + }) + } + + slices.SortFunc(chats, func(a, b chat) int { return cmp.Compare(a.ID, b.ID) }) + + h.respond(w, http.StatusOK, response{Chats: chats}) + } +} diff --git a/internal/api/chats_test.go b/internal/api/chats_test.go new file mode 100644 index 0000000..3cc6b9f --- /dev/null +++ b/internal/api/chats_test.go @@ -0,0 +1,64 @@ +package api_test + +import ( + "net/http" + "testing" + + "sneak.berlin/go/simplexcalc/internal/simplex" +) + +// TestChats: the chats are the bot's contacts, ordered by id, deleted +// ones marked, and the chat client is asked for the bot's user with a +// deadline. +func TestChats(t *testing.T) { + t.Parallel() + + client := &fakeClient{contacts: []simplex.Contact{ + {ContactID: 3, Profile: simplex.Profile{DisplayName: "bob"}, Status: "deleted"}, + {ContactID: 2, Profile: simplex.Profile{DisplayName: "alice"}, Status: "active"}, + }} + + rec := request(t, newAPI(credential, client), http.MethodGet, chatsPath, bearer) + + want := `{"chats":[{"id":2,"display_name":"alice","contact_deleted":false},` + + `{"id":3,"display_name":"bob","contact_deleted":true}]}` + "\n" + if rec.Code != http.StatusOK || rec.Body.String() != want { + t.Errorf("response = %d %q, want 200 %q", rec.Code, rec.Body.String(), want) + } + + if got := rec.Header().Get("Content-Type"); got != "application/json" { + t.Errorf("Content-Type = %q, want application/json", got) + } + + if client.userID != 1 || !client.hadDeadline { + t.Errorf("the chat client was asked for user %d, deadline %v; "+ + "want user 1 with a deadline", client.userID, client.hadDeadline) + } +} + +// TestNoChats: no contacts is an empty list, not null. +func TestNoChats(t *testing.T) { + t.Parallel() + + rec := request(t, newAPI(credential, &fakeClient{}), + http.MethodGet, chatsPath, bearer) + + want := `{"chats":[]}` + "\n" + if rec.Code != http.StatusOK || rec.Body.String() != want { + t.Errorf("response = %d %q, want 200 %q", rec.Code, rec.Body.String(), want) + } +} + +// TestChatsFailure: when the chat client fails, the response says so +// in a chosen sentence, never in the error's own text. +func TestChatsFailure(t *testing.T) { + t.Parallel() + + rec := request(t, newAPI(credential, &fakeClient{err: errChat}), + http.MethodGet, chatsPath, bearer) + + want := `{"error":"the chats could not be read"}` + "\n" + if rec.Code != http.StatusInternalServerError || rec.Body.String() != want { + t.Errorf("response = %d %q, want 500 %q", rec.Code, rec.Body.String(), want) + } +} diff --git a/internal/bot/bot.go b/internal/bot/bot.go index 4ba2eb0..18966bb 100644 --- a/internal/bot/bot.go +++ b/internal/bot/bot.go @@ -8,12 +8,15 @@ import ( "errors" "fmt" "log/slog" + "net/http" "os" "path/filepath" "strconv" "time" + "sneak.berlin/go/simplexcalc/internal/api" "sneak.berlin/go/simplexcalc/internal/calc" + "sneak.berlin/go/simplexcalc/internal/config" "sneak.berlin/go/simplexcalc/internal/simplex" ) @@ -42,17 +45,22 @@ const ( // retryInterval paces the connection attempts. retryInterval = 250 * time.Millisecond + // apiStopTimeout is how long API requests in progress get to finish + // when the bot stops, before their connections are closed. + apiStopTimeout = 5 * time.Second + dataDirMode = 0o700 ) var errExited = errors.New("simplex-chat exited") -// Run starts the chat client with its database in dataDir, connects to -// it, sets up the bot's address, and answers messages until ctx is -// cancelled — which is a clean stop and returns nil — or until the chat -// client or the connection to it fails, which returns the error. -func Run(ctx context.Context, log *slog.Logger, dataDir string) error { - err := os.MkdirAll(dataDir, dataDirMode) +// Run starts the chat client with its database in cfg.DataDir, connects +// to it, sets up the bot's address, then answers messages and serves the +// API until ctx is cancelled — which is a clean stop and returns nil — +// or until the chat client, the connection to it or the API's listener +// fails, which returns the error. +func Run(ctx context.Context, log *slog.Logger, cfg *config.Config) error { + err := os.MkdirAll(cfg.DataDir, dataDirMode) if err != nil { return fmt.Errorf("creating data directory: %w", err) } @@ -61,7 +69,7 @@ func Run(ctx context.Context, log *slog.Logger, dataDir string) error { // Run return only once it has exited, whatever path Run takes. cliCtx, stopCLI := context.WithCancel(ctx) - cli, err := simplex.StartCLI(cliCtx, log, filepath.Join(dataDir, "simplex"), + cli, err := simplex.StartCLI(cliCtx, log, filepath.Join(cfg.DataDir, "simplex"), DisplayName, chatPort) if err != nil { stopCLI() @@ -81,11 +89,31 @@ func Run(ctx context.Context, log *slog.Logger, dataDir string) error { defer func() { _ = client.Close() }() - err = setUp(ctx, log, client) + user, err := setUp(ctx, log, client) if err != nil { return err } + srv := api.New(api.Params{ + Log: log, + Client: client, + UserID: user.UserID, + Port: cfg.Port, + Token: cfg.APIToken, + }) + + served := make(chan error, 1) + + go func() { + served <- srv.ListenAndServe() + }() + + // Deferred last, so it runs first: requests in progress finish while + // the chat client is still there to answer them. + defer stopAPI(ctx, log, srv) + + log.Info("starting the API", "port", cfg.Port) + select { case <-ctx.Done(): return nil @@ -93,6 +121,23 @@ func Run(ctx context.Context, log *slog.Logger, dataDir string) error { return fmt.Errorf("%w: %w", simplex.ErrClosed, client.Err()) case <-cli.Done(): return fmt.Errorf("%w: %w", errExited, cli.Err()) + case err := <-served: + return fmt.Errorf("serving the API: %w", err) + } +} + +// stopAPI stops the API server, giving requests in progress up to +// apiStopTimeout to finish before closing their connections. +func stopAPI(ctx context.Context, log *slog.Logger, srv *http.Server) { + // ctx is usually cancelled by now: that is why the bot is stopping. + ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), apiStopTimeout) + defer cancel() + + err := srv.Shutdown(ctx) + if err != nil { + log.Warn("stopping the API", "error", err) + + _ = srv.Close() } } @@ -125,19 +170,22 @@ func connect( // setUp gives the bot a long-term contact address, creating it on the // first start, and sets it to accept every contact request and to greet // each new contact. The settings are written on every start, so an -// address whose settings were changed by hand is put right. -func setUp(ctx context.Context, log *slog.Logger, client *simplex.Client) error { +// address whose settings were changed by hand is put right. It returns +// the bot's user profile. +func setUp( + ctx context.Context, log *slog.Logger, client *simplex.Client, +) (simplex.User, error) { ctx, cancel := context.WithTimeout(ctx, setupTimeout) defer cancel() user, err := client.ActiveUser(ctx) if err != nil { - return fmt.Errorf("reading the bot's profile: %w", err) + return user, fmt.Errorf("reading the bot's profile: %w", err) } link, ok, err := client.Address(ctx, user.UserID) if err != nil { - return fmt.Errorf("reading the bot's address: %w", err) + return user, fmt.Errorf("reading the bot's address: %w", err) } if !ok { @@ -145,7 +193,7 @@ func setUp(ctx context.Context, log *slog.Logger, client *simplex.Client) error link, err = client.CreateAddress(ctx, user.UserID) if err != nil { - return fmt.Errorf("creating the bot's address: %w", err) + return user, fmt.Errorf("creating the bot's address: %w", err) } } @@ -154,7 +202,7 @@ func setUp(ctx context.Context, log *slog.Logger, client *simplex.Client) error AutoReply: &simplex.MsgContent{Type: "text", Text: Welcome}, }) if err != nil { - return fmt.Errorf("setting the bot's address to accept everyone: %w", err) + return user, fmt.Errorf("setting the bot's address to accept everyone: %w", err) } log.Info("ready", @@ -163,7 +211,7 @@ func setUp(ctx context.Context, log *slog.Logger, client *simplex.Client) error "full_address", link.FullLink, ) - return nil + return user, nil } // handle answers each text message a contact sends. diff --git a/internal/cli/run.go b/internal/cli/run.go index bbf0782..8392856 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -45,7 +45,7 @@ func run(ctx context.Context, version string) error { ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) defer stop() - err = bot.Run(ctx, log, cfg.DataDir) + err = bot.Run(ctx, log, cfg) if err != nil { log.Error("stopped", "error", err) diff --git a/internal/config/config.go b/internal/config/config.go index 7e321d7..0b5eb56 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -16,8 +16,10 @@ package config import ( "errors" "fmt" + "os" "strconv" "strings" + "unicode/utf8" "github.com/spf13/viper" @@ -34,12 +36,23 @@ import ( // Environment variable names. Bare names, no prefix: this matches the // other services and keeps a compose file readable. const ( - EnvDataDir = "DATA_DIR" - EnvDebug = "DEBUG" + EnvDataDir = "DATA_DIR" + EnvDebug = "DEBUG" + EnvPort = "PORT" + EnvAPITokenFile = "API_TOKEN_FILE" //nolint:gosec // G101: a name, not a credential ) -// DefaultDataDir applies when DATA_DIR is absent. -const DefaultDataDir = "./data" +// Defaults, for the variables that are absent. +const ( + DefaultDataDir = "./data" + DefaultPort = 8080 +) + +// MinAPITokenLength is the fewest characters the API credential may +// have, not counting whitespace around it. +const MinAPITokenLength = 32 + +const maxPort = 65535 // ErrInvalidConfig is the sentinel every configuration failure wraps, // so callers can distinguish "the operator got it wrong" from "the @@ -55,6 +68,14 @@ type Config struct { // address and its contacts. Losing it loses the address. DataDir string Debug bool + + // Port is the API's TCP port. + Port int + + // APIToken is the credential every API request must carry, read + // from the file named by API_TOKEN_FILE. Empty when that is absent, + // and then the API refuses every request. Never log it. + APIToken string } // loader parses one environment into a Config, accumulating every @@ -109,6 +130,53 @@ func (l *loader) boolean(key string, def bool) bool { return b } +// port accepts a whole number from 1 to 65535 and refuses everything +// else. +func (l *loader) port(key string, def int) int { + s, ok := l.raw(key) + if !ok { + return def + } + + n, err := strconv.Atoi(s) + if err != nil || n < 1 || n > maxPort { + l.fail(key, s, "not a port (use a whole number from 1 to 65535)") + + return def + } + + return n +} + +// tokenFile returns the credential held in the file named by key, with +// the whitespace around it trimmed, or "" when key is absent. A file +// that cannot be read, or holds fewer than MinAPITokenLength +// characters, is a failure; the message names the file, never what it +// holds. +func (l *loader) tokenFile(key string) string { + path, ok := l.raw(key) + if !ok { + return "" + } + + b, err := os.ReadFile(path) //nolint:gosec // G304: the operator names the file. + if err != nil { + l.fail(key, path, "unreadable: "+err.Error()) + + return "" + } + + token := strings.TrimSpace(string(b)) + if utf8.RuneCountInString(token) < MinAPITokenLength { + l.fail(key, path, fmt.Sprintf("a file holding fewer than %d characters", + MinAPITokenLength)) + + return "" + } + + return token +} + // New parses and validates the environment. An error here aborts // startup before the chat client is launched, so there is no partially // configured running state to reason about. @@ -125,8 +193,10 @@ func load(v *viper.Viper) (*Config, error) { l := &loader{v: v} c := &Config{ - DataDir: l.str(EnvDataDir, DefaultDataDir), - Debug: l.boolean(EnvDebug, false), + DataDir: l.str(EnvDataDir, DefaultDataDir), + Debug: l.boolean(EnvDebug, false), + Port: l.port(EnvPort, DefaultPort), + APIToken: l.tokenFile(EnvAPITokenFile), } if len(l.errs) > 0 { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3a3a378..3e1fb2f 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,6 +2,9 @@ package config_test import ( "errors" + "os" + "path/filepath" + "strings" "testing" "github.com/spf13/viper" @@ -26,8 +29,11 @@ func TestAbsentValuesTakeDefaults(t *testing.T) { t.Parallel() for name, kv := range map[string]map[string]string{ - "unset": nil, - "whitespace only": {config.EnvDataDir: " ", config.EnvDebug: " "}, + "unset": nil, + "whitespace only": { + config.EnvDataDir: " ", config.EnvDebug: " ", + config.EnvPort: " ", config.EnvAPITokenFile: "\t", + }, } { t.Run(name, func(t *testing.T) { t.Parallel() @@ -44,6 +50,14 @@ func TestAbsentValuesTakeDefaults(t *testing.T) { if c.Debug { t.Error("Debug must default off") } + + if c.Port != config.DefaultPort { + t.Errorf("Port = %d, want %d", c.Port, config.DefaultPort) + } + + if c.APIToken != "" { + t.Error("APIToken must default to none") + } }) } } @@ -94,3 +108,79 @@ func TestValidValuesAreUsed(t *testing.T) { t.Error("Debug = false, want true") } } + +// TestPort: PORT is a whole number from 1 to 65535, and anything else +// aborts. +func TestPort(t *testing.T) { + t.Parallel() + + for raw, want := range map[string]int{"1": 1, "8081": 8081, "65535": 65535} { + c, err := config.Load(env(map[string]string{config.EnvPort: raw})) + if err != nil { + t.Errorf("PORT=%q was rejected: %v", raw, err) + + continue + } + + if c.Port != want { + t.Errorf("PORT=%q: Port = %d, want %d", raw, c.Port, want) + } + } + + for _, raw := range []string{"0", "65536", "-1", "80.5", "8080x", "http"} { + _, err := config.Load(env(map[string]string{config.EnvPort: raw})) + if !errors.Is(err, config.ErrInvalidConfig) { + t.Errorf("PORT=%q: error = %v, want ErrInvalidConfig", raw, err) + } + } +} + +// TestAPITokenFile: the credential is the file's content without the +// whitespace around it. A file that cannot be read, or holds too short +// a credential, aborts, and the error never shows what the file holds. +func TestAPITokenFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + token := strings.Repeat("k", config.MinAPITokenLength) + short := strings.Repeat("s", config.MinAPITokenLength-1) + + for name, content := range map[string]string{ + "good": " " + token + "\n", + "short": "\n" + short + " \n", + } { + err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600) + if err != nil { + t.Fatal(err) + } + } + + load := func(name string) (*config.Config, error) { + return config.Load(env(map[string]string{ + config.EnvAPITokenFile: filepath.Join(dir, name), + })) + } + + c, err := load("good") + if err != nil { + t.Fatalf("a good file was rejected: %v", err) + } + + if c.APIToken != token { + t.Errorf("APIToken = %q, want %q", c.APIToken, token) + } + + _, err = load("missing") + if !errors.Is(err, config.ErrInvalidConfig) { + t.Errorf("a missing file: error = %v, want ErrInvalidConfig", err) + } + + _, err = load("short") + if !errors.Is(err, config.ErrInvalidConfig) { + t.Fatalf("a short credential: error = %v, want ErrInvalidConfig", err) + } + + if strings.Contains(err.Error(), short) { + t.Errorf("the error shows the file's content: %v", err) + } +} diff --git a/internal/simplex/client.go b/internal/simplex/client.go index e4bc309..db12d59 100644 --- a/internal/simplex/client.go +++ b/internal/simplex/client.go @@ -168,6 +168,18 @@ func (c *Client) SetAddressSettings( return c.command(ctx, cmd, TypeUserContactLinkUpdated, nil) } +// Contacts returns the user's contacts: everyone it has a direct chat +// with. +func (c *Client) Contacts(ctx context.Context, userID int64) ([]Contact, error) { + var r struct { + Contacts []Contact `json:"contacts"` + } + + err := c.command(ctx, cmdListContacts(userID), TypeContactsList, &r) + + return r.Contacts, err +} + // SendText sends a text message to a contact, as a reply to the message // quotedItemID (0 for none). It does not wait for the chat client to // accept it; a failure is logged when the client's answer arrives. diff --git a/internal/simplex/client_test.go b/internal/simplex/client_test.go index feaa9f9..ce33fc3 100644 --- a/internal/simplex/client_test.go +++ b/internal/simplex/client_test.go @@ -7,6 +7,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "slices" "strings" "sync" "testing" @@ -43,6 +44,19 @@ const ( contactConnected = `{"type":"contactConnected","user":{"userId":1}, "contact":{"contactId":3,"localDisplayName":"alice"}}` + + // Two contacts with the same display name, which the chat client + // tells apart by the local name it gives the second. The second has + // deleted its chat with the bot, and is still listed. + contactsList = `{"type":"contactsList","user":{"userId":1},"contacts":[ + {"contactId":3,"localDisplayName":"tester","profile":{"profileId":3, + "displayName":"tester","fullName":"","localAlias":""}, + "activeConn":{"connId":2,"connStatus":{"type":"ready"}}, + "contactUsed":true,"contactStatus":"active","chatDeleted":false}, + {"contactId":4,"localDisplayName":"tester_1","profile":{"profileId":4, + "displayName":"tester","fullName":"","localAlias":""}, + "activeConn":{"connId":3,"connStatus":{"type":"deleted"}}, + "contactUsed":true,"contactStatus":"deleted","chatDeleted":false}]}` ) // fakeChat stands in for the chat client's API. It answers each command @@ -220,6 +234,38 @@ func TestAddressSetup(t *testing.T) { } } +// TestContacts: the contacts of the given user come back with their +// ids, display names and whether they are deleted. +func TestContacts(t *testing.T) { + t.Parallel() + + f, url := newFakeChat(t, map[string]string{"/_contacts": contactsList}) + c, ctx := dial(t, url, nil) + + contacts, err := c.Contacts(ctx, 1) + if err != nil { + t.Fatalf("Contacts: %v", err) + } + + if got := f.next(t); got != "/_contacts 1" { + t.Errorf("command = %s, want /_contacts 1", got) + } + + tester := simplex.Profile{DisplayName: "tester"} + + want := []simplex.Contact{ + {ContactID: 3, Profile: tester, Status: "active"}, + {ContactID: 4, Profile: tester, Status: "deleted"}, + } + if !slices.Equal(contacts, want) { + t.Fatalf("Contacts = %+v\nwant %+v", contacts, want) + } + + if contacts[0].Deleted() || !contacts[1].Deleted() { + t.Error("Deleted must be false for contact 3 and true for contact 4") + } +} + // TestRefusedCommand: a command the chat client refuses is an error // that names the reason. func TestRefusedCommand(t *testing.T) { diff --git a/internal/simplex/protocol.go b/internal/simplex/protocol.go index 66bb80f..3251727 100644 --- a/internal/simplex/protocol.go +++ b/internal/simplex/protocol.go @@ -14,6 +14,7 @@ const ( TypeUserContactLink = "userContactLink" TypeUserContactLinkCreated = "userContactLinkCreated" TypeUserContactLinkUpdated = "userContactLinkUpdated" + TypeContactsList = "contactsList" TypeNewChatItems = "newChatItems" TypeContactConnected = "contactConnected" TypeChatCmdError = "chatCmdError" @@ -46,10 +47,14 @@ func (e Event) Decode(v any) error { type ( // User is the chat client's local user profile: the bot itself. User struct { - UserID int64 `json:"userId"` - Profile struct { - DisplayName string `json:"displayName"` - } `json:"profile"` + UserID int64 `json:"userId"` + Profile Profile `json:"profile"` + } + + // Profile is how the bot or a contact presents itself. Nothing + // makes a display name unique. + Profile struct { + DisplayName string `json:"displayName"` } // ConnLink is a SimpleX link. The short form is what people share; @@ -61,7 +66,9 @@ type ( // Contact is a person connected to the bot. Contact struct { - ContactID int64 `json:"contactId"` + ContactID int64 `json:"contactId"` + Profile Profile `json:"profile"` + Status string `json:"contactStatus"` } // NewChatItems is the record of a newChatItems event: messages @@ -142,6 +149,13 @@ type ( } ) +// Deleted reports whether the contact is gone, as it is once the person +// deletes their chat with the bot. The chat client still lists such a +// contact, with its chat, but nothing more reaches them. +func (c Contact) Deleted() bool { + return c.Status != "active" +} + // Message is a text message a contact sent to the bot. type Message struct { ContactID int64 @@ -190,6 +204,10 @@ func cmdSetAddressSettings(userID int64, s AddressSettings) (string, error) { return "/_address_settings " + strconv.FormatInt(userID, 10) + " " + string(b), nil } +func cmdListContacts(userID int64) string { + return "/_contacts " + strconv.FormatInt(userID, 10) +} + func cmdSendText(contactID, quotedItemID int64, text string) (string, error) { b, err := json.Marshal([]composedMessage{{ QuotedItemID: quotedItemID, -- 2.54.0 From b965e45454e59a9e8d3e12358334633dc9d3cded Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 00:59:42 +0000 Subject: [PATCH 2/6] Send Permissions-Policy on every API response (closes #4) docs/REPO_POLICIES.md requires a Permissions-Policy header restricting the browser features an application does not use. The API now denies the camera, microphone and location on every response, TestHeaders checks it, and the README's Design section names it. Model: opus-5-5 --- README.md | 5 +++-- internal/api/api.go | 5 ++++- internal/api/api_test.go | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2d91f6a..9299b4d 100644 --- a/README.md +++ b/README.md @@ -222,8 +222,9 @@ container. starts once set-up is done; if it cannot listen, the bot exits with an error, as when the chat client fails. Every request must carry the credential, compared in constant time; every response carries headers - that forbid framing, content sniffing, caching and referrers; a - request body is capped at 64 KiB and a request's work at 10 seconds. + that forbid framing, content sniffing, caching and referrers, and a + `Permissions-Policy` that denies the camera, microphone and location; + a request body is capped at 64 KiB and a request's work at 10 seconds. Handlers call the chat client on the request's own goroutine, never on the one that delivers events, which also delivers the chat client's answers. When the bot stops, requests in progress get 5 seconds to diff --git a/internal/api/api.go b/internal/api/api.go index bbf0aad..3fbd21c 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -151,7 +151,8 @@ func (h *handlers) respondError(w http.ResponseWriter, status int, sentence stri // securityHeaders go on every response. The API returns JSON to // programs, so a browser may not frame, sniff, cache or refer from it, -// and must reach it over HTTPS. +// nor give it the camera, microphone or location, and must reach it +// over HTTPS. func securityHeaders(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { header := w.Header() @@ -160,6 +161,8 @@ func securityHeaders(next http.Handler) http.Handler { "default-src 'none'; frame-ancestors 'none'") header.Set("X-Frame-Options", "DENY") header.Set("Referrer-Policy", "no-referrer") + header.Set("Permissions-Policy", + "camera=(), microphone=(), geolocation=()") header.Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") header.Set("Cache-Control", "no-store") diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 5056e81..be3a2da 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -185,6 +185,7 @@ func TestHeaders(t *testing.T) { "Content-Security-Policy": "default-src 'none'; frame-ancestors 'none'", "X-Frame-Options": "DENY", "Referrer-Policy": "no-referrer", + "Permissions-Policy": "camera=(), microphone=(), geolocation=()", "Strict-Transport-Security": "max-age=31536000; includeSubDomains", "Cache-Control": "no-store", } -- 2.54.0 From 407eaf847ab010f0c99356e0c34cd557b29d0252 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 01:36:44 +0000 Subject: [PATCH 3/6] Stop the chat client only after the API has stopped (closes #4) The chat client's context no longer follows the bot's, so a stop no longer sends it SIGTERM while the API is still finishing its requests. The deferred stop in Run, which runs after the API's, stops it instead, so a request in progress at a stop gets its answer rather than a 500. The README says so. The new test runs the whole bot with the test binary itself standing in for simplex-chat on PATH. The stand-in holds its answer to the contacts request until the test has told the bot to stop. Model: opus-5-5 --- README.md | 2 +- internal/bot/bot.go | 4 +- internal/bot/run_test.go | 233 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 237 insertions(+), 2 deletions(-) create mode 100644 internal/bot/run_test.go diff --git a/README.md b/README.md index 9299b4d..7e30288 100644 --- a/README.md +++ b/README.md @@ -228,7 +228,7 @@ container. Handlers call the chat client on the request's own goroutine, never on the one that delivers events, which also delivers the chat client's answers. When the bot stops, requests in progress get 5 seconds to - finish. + finish before the chat client is stopped. - **Replies**: for each text message a contact sends in a direct chat, the bot sends back the result, as a reply quoting the message. Group messages, files and the bot's own messages are ignored. diff --git a/internal/bot/bot.go b/internal/bot/bot.go index 18966bb..d3b4ec5 100644 --- a/internal/bot/bot.go +++ b/internal/bot/bot.go @@ -67,7 +67,9 @@ func Run(ctx context.Context, log *slog.Logger, cfg *config.Config) error { // Cancelling this stops the chat client; the deferred wait makes // Run return only once it has exited, whatever path Run takes. - cliCtx, stopCLI := context.WithCancel(ctx) + // Cancelling ctx does not reach it, so that the API, stopped first, + // can finish its requests while the chat client still answers. + cliCtx, stopCLI := context.WithCancel(context.WithoutCancel(ctx)) cli, err := simplex.StartCLI(cliCtx, log, filepath.Join(cfg.DataDir, "simplex"), DisplayName, chatPort) diff --git a/internal/bot/run_test.go b/internal/bot/run_test.go new file mode 100644 index 0000000..a14620f --- /dev/null +++ b/internal/bot/run_test.go @@ -0,0 +1,233 @@ +package bot_test + +import ( + "context" + "encoding/json" + "fmt" + "io" + "log/slog" + "net" + "net/http" + "net/netip" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "testing" + "time" + + "github.com/gorilla/websocket" + "sneak.berlin/go/simplexcalc/internal/bot" + "sneak.berlin/go/simplexcalc/internal/config" + "sneak.berlin/go/simplexcalc/internal/simplex" +) + +const ( + // credential is what the bot's API is configured with here. + credential = "a-credential-for-these-tests" //nolint:gosec // G101: invented for tests + + // asked is the file the stand-in chat client creates in the data + // directory when it is asked for the contacts. + asked = "asked-for-contacts" + + // contactsDelay is how long the stand-in then holds its answer: + // long enough for the test to stop the bot meanwhile, and well + // within the 5 seconds the API gets to finish its requests. + contactsDelay = time.Second +) + +// TestMain lets this test binary be the chat client as well: started +// under the chat client's name, as TestStopDuringRequest arranges, it is +// the stand-in instead of running the tests. +func TestMain(m *testing.M) { + if filepath.Base(os.Args[0]) == simplex.Binary { + standIn() // never returns + } + + m.Run() +} + +// standIn plays the chat client: it serves the WebSocket API on the +// port it is given, on localhost, and answers the commands the bot +// sends. SIGTERM ends it, as it ends the real client. Unlike the real +// client, it also exits when the bot hangs up, so that it never +// outlives a test run that was cut short. It takes port 5225, the chat +// client's fixed port, so this test cannot run twice at once on one +// machine. +func standIn() { + arg := func(name string) string { + return os.Args[slices.Index(os.Args, name)+1] + } + + marker := filepath.Join(filepath.Dir(arg("--database")), asked) + + srv := &http.Server{ + Addr: "127.0.0.1:" + arg("--chat-server-port"), + ReadHeaderTimeout: time.Second, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + answer(w, r, marker) + }), + } + + err := srv.ListenAndServe() + _, _ = fmt.Fprintln(os.Stderr, err) + + os.Exit(1) +} + +// answer answers the commands on one connection with records reduced to +// the fields the bot reads. Asked for the contacts, it first creates +// the file marker, then holds its answer for contactsDelay. +func answer(w http.ResponseWriter, r *http.Request, marker string) { + conn, err := (&websocket.Upgrader{}).Upgrade(w, r, nil) + if err != nil { + return + } + + records := map[string]string{ + "/user": `{"type":"activeUser","user":{"userId":1}}`, + "/_show_address": `{"type":"userContactLink","contactLink":{}}`, + "/_address_settings": `{"type":"userContactLinkUpdated"}`, + "/_contacts": `{"type":"contactsList","contacts":[{"contactId":3,` + + `"profile":{"displayName":"tester"},"contactStatus":"active"}]}`, + } + + for { + var cmd map[string]string + + err = conn.ReadJSON(&cmd) + if err != nil { + os.Exit(0) + } + + name, _, _ := strings.Cut(cmd["cmd"], " ") + + record, ok := records[name] + if !ok { + continue + } + + if name == "/_contacts" { + _ = os.WriteFile(marker, nil, 0o600) + + time.Sleep(contactsDelay) + } + + _ = conn.WriteJSON(map[string]any{ + "corrId": cmd["corrId"], + "resp": json.RawMessage(record), + }) + } +} + +// TestStopDuringRequest: a request that is waiting on the chat client +// when the bot is told to stop still gets the chat client's answer, +// because the chat client is stopped only once the API has stopped. +func TestStopDuringRequest(t *testing.T) { + // Run starts the chat client from PATH: put this test binary there + // under the chat client's name. + bin := t.TempDir() + + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + + err = os.Symlink(exe, filepath.Join(bin, simplex.Binary)) + if err != nil { + t.Fatal(err) + } + + t.Setenv("PATH", bin) + + cfg := &config.Config{DataDir: t.TempDir(), Port: freePort(t), APIToken: credential} + ctx, stop := context.WithCancel(t.Context()) + done := make(chan struct{}) + + var runErr error + + go func() { + defer close(done) + + runErr = bot.Run(ctx, slog.New(slog.DiscardHandler), cfg) + }() + + t.Cleanup(func() { + stop() + <-done + + if runErr != nil { + t.Errorf("Run: %v", runErr) + } + }) + + // Stop the bot once the request below is waiting on the chat client. + go func() { + for ctx.Err() == nil { + _, err := os.Stat(filepath.Join(cfg.DataDir, asked)) + if err == nil { + stop() + } + + time.Sleep(10 * time.Millisecond) + } + }() + + status, body := getChats(t, cfg.Port, done) + + want := `{"chats":[{"id":3,"display_name":"tester","contact_deleted":false}]}` + "\n" + if status != http.StatusOK || body != want { + t.Errorf("GET /api/v1/chats = %d %q, want 200 %q", status, body, want) + } +} + +// freePort returns a TCP port that nothing listens on at the moment. +func freePort(t *testing.T) int { + t.Helper() + + l, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", ":0") + if err != nil { + t.Fatal(err) + } + + defer func() { _ = l.Close() }() + + return int(netip.MustParseAddrPort(l.Addr().String()).Port()) +} + +// getChats asks the bot's API for the chats, trying again while the API +// is not listening yet, and returns the answer's status and body. It +// fails the test if Run returns first, which closes done. +func getChats(t *testing.T, port int, done <-chan struct{}) (int, string) { + t.Helper() + + url := "http://127.0.0.1:" + strconv.Itoa(port) + "/api/v1/chats" + + for { + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, url, nil) + if err != nil { + t.Fatal(err) + } + + req.Header.Set("Authorization", "Bearer "+credential) + + resp, err := http.DefaultClient.Do(req) + if err == nil { + body, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + + if err != nil { + t.Fatal(err) + } + + return resp.StatusCode, string(body) + } + + select { + case <-done: + t.Fatal("Run returned before the API answered") + case <-time.After(50 * time.Millisecond): + } + } +} -- 2.54.0 From 9ce902fb05c6d463840c691926120011374ada38 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 02:08:56 +0000 Subject: [PATCH 4/6] Take the chat client's port from Run's caller (closes #4) The test that runs the whole bot started its stand-in chat client on 5225, the port a real simplex-chat uses, so on a machine where one listens there the test would have sent it the bot's set-up commands. Run now takes the chat client's port: the run command passes bot.ChatPort (5225), and the test a port it found free. Model: opus-5-5 --- internal/bot/bot.go | 30 +++++++++++++++++------------- internal/bot/run_test.go | 8 ++++---- internal/cli/run.go | 2 +- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/internal/bot/bot.go b/internal/bot/bot.go index d3b4ec5..e0d3738 100644 --- a/internal/bot/bot.go +++ b/internal/bot/bot.go @@ -28,11 +28,11 @@ const DisplayName = "calc" const Welcome = "Send me arithmetic, such as 2 + 2 or 5 * 5/2, " + "and I will reply with the result." -const ( - // chatPort is where the chat client serves its API, on localhost - // inside the bot's own container. - chatPort = 5225 +// ChatPort is where the chat client serves its API, on localhost inside +// the bot's own container. +const ChatPort = 5225 +const ( // connectTimeout bounds the wait for a freshly started chat client // to open its API, which includes creating or migrating the // database. @@ -54,12 +54,15 @@ const ( var errExited = errors.New("simplex-chat exited") -// Run starts the chat client with its database in cfg.DataDir, connects -// to it, sets up the bot's address, then answers messages and serves the -// API until ctx is cancelled — which is a clean stop and returns nil — -// or until the chat client, the connection to it or the API's listener -// fails, which returns the error. -func Run(ctx context.Context, log *slog.Logger, cfg *config.Config) error { +// Run starts the chat client with its database in cfg.DataDir, serving +// its API on localhost at chatPort, connects to it, sets up the bot's +// address, then answers messages and serves the bot's API until ctx is +// cancelled — which is a clean stop and returns nil — or until the chat +// client, the connection to it or the API's listener fails, which +// returns the error. +func Run( + ctx context.Context, log *slog.Logger, cfg *config.Config, chatPort int, +) error { err := os.MkdirAll(cfg.DataDir, dataDirMode) if err != nil { return fmt.Errorf("creating data directory: %w", err) @@ -84,7 +87,7 @@ func Run(ctx context.Context, log *slog.Logger, cfg *config.Config) error { <-cli.Done() }() - client, err := connect(ctx, log, cli) + client, err := connect(ctx, log, cli, chatPort) if err != nil { return err } @@ -143,9 +146,10 @@ func stopAPI(ctx context.Context, log *slog.Logger, srv *http.Server) { } } -// connect waits for the chat client to open its API and connects to it. +// connect waits for the chat client to open its API on chatPort and +// connects to it. func connect( - ctx context.Context, log *slog.Logger, cli *simplex.CLI, + ctx context.Context, log *slog.Logger, cli *simplex.CLI, chatPort int, ) (*simplex.Client, error) { ctx, cancel := context.WithTimeout(ctx, connectTimeout) defer cancel() diff --git a/internal/bot/run_test.go b/internal/bot/run_test.go index a14620f..0c89299 100644 --- a/internal/bot/run_test.go +++ b/internal/bot/run_test.go @@ -52,9 +52,7 @@ func TestMain(m *testing.M) { // port it is given, on localhost, and answers the commands the bot // sends. SIGTERM ends it, as it ends the real client. Unlike the real // client, it also exits when the bot hangs up, so that it never -// outlives a test run that was cut short. It takes port 5225, the chat -// client's fixed port, so this test cannot run twice at once on one -// machine. +// outlives a test run that was cut short. func standIn() { arg := func(name string) string { return os.Args[slices.Index(os.Args, name)+1] @@ -142,6 +140,8 @@ func TestStopDuringRequest(t *testing.T) { t.Setenv("PATH", bin) cfg := &config.Config{DataDir: t.TempDir(), Port: freePort(t), APIToken: credential} + // Never bot.ChatPort: a real chat client may be listening there. + chatPort := freePort(t) ctx, stop := context.WithCancel(t.Context()) done := make(chan struct{}) @@ -150,7 +150,7 @@ func TestStopDuringRequest(t *testing.T) { go func() { defer close(done) - runErr = bot.Run(ctx, slog.New(slog.DiscardHandler), cfg) + runErr = bot.Run(ctx, slog.New(slog.DiscardHandler), cfg, chatPort) }() t.Cleanup(func() { diff --git a/internal/cli/run.go b/internal/cli/run.go index 8392856..a863fd2 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -45,7 +45,7 @@ func run(ctx context.Context, version string) error { ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) defer stop() - err = bot.Run(ctx, log, cfg) + err = bot.Run(ctx, log, cfg, bot.ChatPort) if err != nil { log.Error("stopped", "error", err) -- 2.54.0 From 0c5163e69edaad732b6dc503a678e01f7e2b8f9e Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 02:20:58 +0000 Subject: [PATCH 5/6] Refuse OPTIONS * like any other API request (closes #4) net/http answered "OPTIONS *" itself, with 200, before the router, so it skipped the credential check and the security headers. The server now passes it to the router, which refuses it with 401 like any other request without the credential. A test sends it to a running server, since the handler alone never sees it. Model: opus-5-5 --- internal/api/api.go | 3 +++ internal/api/api_test.go | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/internal/api/api.go b/internal/api/api.go index 3fbd21c..fbbe9db 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -95,6 +95,9 @@ func New(p Params) *http.Server { ReadTimeout: readTimeout, WriteTimeout: writeTimeout, IdleTimeout: idleTimeout, + // Otherwise net/http answers "OPTIONS *" itself, with 200 and + // without the credential check or the headers. + DisableGeneralOptionsHandler: true, // net/http's own messages, such as a handler's panic, go to the // same JSON log as everything else. ErrorLog: slog.NewLogLogger(p.Log.Handler(), slog.LevelError), diff --git a/internal/api/api_test.go b/internal/api/api_test.go index be3a2da..2f78fe4 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -4,7 +4,9 @@ import ( "bytes" "context" "errors" + "io" "log/slog" + "net" "net/http" "net/http/httptest" "strings" @@ -175,6 +177,49 @@ func TestNoPathIsExempt(t *testing.T) { } } +// TestOptionsAsterisk: "OPTIONS *" is refused like any other request. +// net/http would answer it before the handler, so this request goes to +// a running server rather than to its handler. +func TestOptionsAsterisk(t *testing.T) { + t.Parallel() + + srv := newAPI("", &fakeClient{}) + + listener, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + + go func() { _ = srv.Serve(listener) }() + + t.Cleanup(func() { _ = srv.Close() }) + + req, err := http.NewRequestWithContext(t.Context(), http.MethodOptions, + "http://"+listener.Addr().String(), nil) + if err != nil { + t.Fatal(err) + } + + // The request line becomes "OPTIONS * HTTP/1.1". + req.URL.Opaque = "*" + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + + if resp.StatusCode != http.StatusUnauthorized || string(body) != unauthorized { + t.Errorf("OPTIONS *: %d %q, want 401 %q", resp.StatusCode, body, unauthorized) + } +} + // TestHeaders: every response, whatever its status, carries the // security headers, and none lets another origin in. func TestHeaders(t *testing.T) { -- 2.54.0 From b56d8714a751c6e28fa5d7c5a17b6cb5c709ded2 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 02:42:39 +0000 Subject: [PATCH 6/6] Name the API credential in the README's Backup (closes #4) Getting Started writes the credential to api-token on the volume, so a backup that copies only the SimpleX database leaves it out, and the documented docker run aborts after a restore. The Backup paragraph now names api-token as the other durable file and says it is secret. Model: opus-5-5 --- README.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e8eac27..b877771 100644 --- a/README.md +++ b/README.md @@ -273,11 +273,13 @@ container. ## Operating it -**Backup.** Everything durable is the SimpleX database on the volume: -`simplex_chat.db` and `simplex_agent.db`. Stop the container before -copying them, since a copy taken from under the running client can be -inconsistent. The database holds the bot's keys, so a copy lets its -holder answer as the bot; keep it as private as the running instance. +**Backup.** Everything durable is on the volume: the SimpleX database, +`simplex_chat.db` and `simplex_agent.db`, and the API credential, +`api-token`. Stop the container before copying the database, since a +copy taken from under the running client can be inconsistent. The +database holds the bot's keys, so a copy lets its holder answer as the +bot, and the credential lets its holder use the API; keep both as +private as the running instance. **Upgrade.** Rebuild the image and recreate the container with the same volume. The chat client migrates its database on start. A newer -- 2.54.0