fix: let an account over its settings cap still replace what it has
Some checks are pending
build / container (push) Waiting to run
build / binary (push) Waiting to run
test / test (push) Waiting to run
test / lint-openapi (push) Waiting to run

Lowering the per-account limit below what an account already holds
froze every key it had, because a request that only replaced existing
keys was still measured against the new cap. There is no way to delete
a setting, so nothing could get the account back under the limit. The
cap is now only applied to requests that add a key.

Separately, a request body containing a second document or trailing
rubbish was accepted and acted on, because decoding stops at the end of
the first value. The rest of the body is now checked.
This commit is contained in:
Chmouel Boudjnah 2026-09-17 07:14:32 +02:00
parent e509652b3b
commit 86e47e6013
3 changed files with 34 additions and 2 deletions

View file

@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"sort"
"strings"
@ -49,7 +50,15 @@ func (s *Server) HandlePutSettings(w http.ResponseWriter, r *http.Request) {
} `json:"settings"`
}
max := s.Cfg.Ops.MaxBodyBytes
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, max)).Decode(&body); err != nil {
dec := json.NewDecoder(http.MaxBytesReader(w, r.Body, max))
if err := dec.Decode(&body); err != nil {
writeError(w, http.StatusBadRequest, "invalid JSON body")
return
}
// Decode stops at the end of the first value, so without this a
// second document, trailing rubbish, or padding past the body limit
// would be stored and answered 200 as if the request were clean.
if _, err := dec.Token(); err != io.EOF {
writeError(w, http.StatusBadRequest, "invalid JSON body")
return
}

View file

@ -389,3 +389,22 @@ func TestSettingsRejectsOversizedBatch(t *testing.T) {
t.Fatalf("want 400 for a batch larger than the account cap, got %d %v", code, body)
}
}
func TestSettingsRejectsTrailingContent(t *testing.T) {
f := newFolderFixture(t)
put := `{"settings":{"reader.font":{"value":"literata","updated_at":"2026-06-01T12:00:00Z"}}}{"settings":{}}`
code, body := putJSONReq(t, f.ts.URL+"/v1/me/settings", f.token, put)
if code != http.StatusBadRequest {
t.Fatalf("want 400 for a body with a second document, got %d %v", code, body)
}
// And nothing was stored on the way to refusing it.
code, body = getJSON(t, f.ts.URL+"/v1/me/settings", f.token)
if code != http.StatusOK {
t.Fatalf("get: %d %v", code, body)
}
if settings := body["settings"].(map[string]any); len(settings) != 0 {
t.Fatalf("a refused request stored something: %v", settings)
}
}

View file

@ -93,7 +93,11 @@ func checkSettingsQuota(ctx context.Context, tx *sql.Tx, userID string, settings
added++
}
}
if existing+added > maxPerAccount {
// Only a request that adds a key can be over the limit. An account
// already above it — because an operator lowered the cap — must
// still be able to replace what it has, or every key it holds is
// frozen with no delete route to get back under.
if added > 0 && existing+added > maxPerAccount {
return store.ErrQuotaExceeded
}
return nil