From b9b341e389c25418c23c788e333afb040e68ca62 Mon Sep 17 00:00:00 2001 From: Jefry Date: Sat, 17 Oct 2026 14:25:39 +0200 Subject: [PATCH] Fix saving an existing private key (500 on every save after the first) set_private_record used store.add, which only inserts: for a key that already has a row it raises EntryAlreadyExists, so admins could create a private key (e.g. hordes_join) once but every later save in the admin Private tab failed with a 500. Use store.overwrite, which inserts or replaces. Adds a regression test that saves the same key twice. Co-Authored-By: Claude Opus 5.5 --- .../src/apiserver/data/features/private_kv.py | 10 ++++-- backend/tests/test_private_kv.py | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 backend/tests/test_private_kv.py diff --git a/backend/src/apiserver/data/features/private_kv.py b/backend/src/apiserver/data/features/private_kv.py index 904c6d6..27a7a81 100644 --- a/backend/src/apiserver/data/features/private_kv.py +++ b/backend/src/apiserver/data/features/private_kv.py @@ -20,8 +20,14 @@ def get_private_record(store: Storage, key: str) -> bytes | None: def set_private_record(store: Storage, key: str, record_bytes: bytes) -> None: - """Store the encoded ``{role, value}`` record under ``key``.""" - store.add(PRIVATE_TABLE, key, record_bytes, expires_at=0) + """Store the encoded ``{role, value}`` record under ``key``. + + Creates the key, or replaces its current record: the admin Private tab + re-saves the whole JSON document every time, so replacing is the normal + case. (``store.add`` only inserts and raised ``EntryAlreadyExists`` for an + existing key, so every save after the first one failed with a 500.) + """ + store.overwrite(PRIVATE_TABLE, key, record_bytes, expires_at=0) def list_private_keys(store: Storage) -> list[str]: diff --git a/backend/tests/test_private_kv.py b/backend/tests/test_private_kv.py new file mode 100644 index 0000000..47121ac --- /dev/null +++ b/backend/tests/test_private_kv.py @@ -0,0 +1,35 @@ +"""Regression test: saving an existing private key must replace it.""" + +from test_spec import ( + ADMIN_COOKIE_HEADERS_BY_CREDS, + get_admin_cookie_headers, + post_json, +) + + +def test_admin_can_save_an_existing_private_key_again( + command, backend_url, auth_client +) -> None: + ADMIN_COOKIE_HEADERS_BY_CREDS.clear() + admin = get_admin_cookie_headers(command, auth_client) + + first = post_json( + backend_url, + "/admin/private_kv/set/", + {"key": "hordes_join", "value": {"Koers": "a"}, "role": "member"}, + admin, + ) + assert first.status_code == 200, first.text + + # Used to fail with a 500 (EntryAlreadyExists from store.add) + second = post_json( + backend_url, + "/admin/private_kv/set/", + {"key": "hordes_join", "value": {"Koers": "b"}}, + admin, + ) + assert second.status_code == 200, second.text + assert second.json()["required_role"] == "member" + + read = post_json(backend_url, "/members/private/", {"key": "hordes_join"}, admin) + assert read.json() == {"value": {"Koers": "b"}}