microsoft / microsoft/simplechat
Every admin settings save silently destroys stored configuration values
- Dominant language
- Python
- Stars
- 152
- Forks
- 116
- Avg merge
- 7h 7m
- Merged PRs (30d)
- 122
Description
**Severity: high. Silent data loss. Fix available.**
`application/single_app/route_frontend_admin_settings.py` reads fields from the
submitted admin settings form that have no corresponding input anywhere in
`application/single_app/templates/admin_settings.html`:
| Line | Setting |
|------|---------|
| 1892 | `enable_summarize_content_history_for_search` |
| 1893 | `enable_summarize_content_history_beyond_conversation_history_limit` |
| 1894 | `number_of_historical_messages_to_summarize` |
| 1932 | `office_docs_key` |
| 1935 | `video_files_key` |
| 1938 | `audio_files_key` |
Because the inputs do not exist, `form_data.get(...)` returns the default on
every save and each value is overwritten with an empty string or a default.
The consequence that makes this urgent: `office_docs_key` is read when generating
citation SAS tokens at `route_frontend_chats.py:1611` and `:1770`. An
administrator configures Enhanced Citations, later saves settings for a
completely unrelated reason, and Enhanced Citations silently stops working with
no error anywhere. Nothing in the UI indicates the key was cleared.
`enable_enhanced_citations_mount` and `enhanced_citations_mount` have the same
problem, so every save also forces the mount off and resets the path to
`/view_documents`. `video_files_authentication_type` and
`audio_files_authentication_type` are reset to `'key'` on every save, currently
inert only because that is also the stored default.
Note that `admin_settings.js` still wires `togglePassword('toggle_office_docs_key',
'office_docs_key')`, so these inputs existed at one point and were lost from the
template while the server kept reading them.
**Suggested fix.** Two halves are both needed. Make the reads non-destructive so
a key is written only when the field is genuinely present in the submitted form,
noting that an empty input is still present so deliberately clearing a secret
keeps working. Then add real inputs where a coherent home exists. Checkboxes need
more than a presence check, because an unchecked box and an absent input are
indistinguishable in form data; a hidden always-submitted section marker proving
the enclosing section was rendered solves that without making the setting
impossible to turn off.
---
Found while enabling features on a fork of `v0.250.001`, verified against commit `ff8059163e09ede433003b1ed2822061c41239fe`. Line numbers are against that baseline.
We have a working fix and a functional test for this on our fork. Happy to open a PR if that is useful, or to share the patch and let you take a different approach. No expectation either way.
Contributor guide
Assessment
This issue has not been assessed yet.