Allow users to set a secrets.toml file in their home directory #6230
Conversation
lib/streamlit/runtime/secrets.py
Outdated
Check notice
Code scanning / CodeQL
Unused global variable
ff2351c to
2f9babd
Compare
There was a problem hiding this comment.
Looks good to me!
I'm agnostic on this, but I wonder if we actually want "a local secrets file replaces the global secrets file" as opposed to "a local secrets file overrides the global secrets"?
I ask because my initial assumption was that the feature would use a "replace" (not "override") strategy for handling multiple secrets file. (I think "override" is a reasonable strategy as long as it doesn't violate user expectation in a dangerous way.) Should we get an opinion from Harsh or another security-minded eng?
Ah, yeah possibly. I chose the current behavior to be consistent with how |
2f9babd to
8246f6d
Compare
I made a quick straw poll here, but my thought was override - I think it's the most flexible/powerful and also consistent with other common tools like git, vscode and streamlit itself. @sfc-gh-tsimons previously suggested surfacing to the developer if secrets are being drawn from multiple files. I think that would do a lot to mitigate the risk of accidentally leaking global secrets. Could we easily log to the developer if multiple files are found when the app launches (and/or when secret file changed, if we reload)? Something like:
|
Yep, this should be a pretty easy change -- I can update this PR to log the message to stdout if we detect both files |
8246f6d to
9107198
Compare
| if len([p for p in self._file_paths if os.path.exists(p)]) > 1: | ||
| _LOGGER.info( | ||
| f"Secrets found in multiple locations: {', '.join(self._file_paths)}. " | ||
| "When multiple secret.toml files exist, local secrets will take precedence over global secrets." | ||
| ) |
There was a problem hiding this comment.
@sfc-gh-jcarroll heads up that I tweaked the message wording slightly because the way this code is implemented now allows us to watch an arbitrary list of secrets.toml files.
I think it's quite unlikely that we'll ever want to watch more than 2, but on the off chance that we do it seems worth it to me to make the error message not refer to an exact number
|
Manual testing looked fine to me on both Windows and MacOS -- should be good to merge once some security process wraps up |
|
3 out of 3 votes for "Override" rather than "replace" in the case of multiple files from our resident power users ✅ |
* develop: Tweak no-else-return config options (streamlit#6343) Allow users to set a secrets.toml file in their home directory (streamlit#6230) Add support for number and boolean types in categorical columns (streamlit#6248)
* develop: StreamlitEndpoints.buildMediaURL (streamlit#6366) Set a default for RuntimeConfig.cache_storage_manager (streamlit#6361) FullScreenWrapper: add a type declaration for react context usage (streamlit#6364) Improve st.help (and st.write's usage of st.help!) (streamlit#5857) Fix regression with query_params (streamlit#6348) Have util.calc_md5 also take bytes (streamlit#6358) Improve deploy button (streamlit#6223) Return whether a secrets.toml file is successfully parsed (streamlit#6333) Add `.webp` to list of safe static file extensions (streamlit#6331) AppContext docstrings (streamlit#6353) fix: upgrade command-line-args from 5.0.2 to 5.2.1 (streamlit#6258) fix: upgrade flatbuffers from 1.11.0 to 1.12.0 (streamlit#6259) sendMessageToHost: no longer a global function (streamlit#6345) Tweak no-else-return config options (streamlit#6343) Allow users to set a secrets.toml file in their home directory (streamlit#6230) Add support for number and boolean types in categorical columns (streamlit#6248) Add support for period type in st.table and st.dataframe (streamlit#5429) Add ability to turn off anchors (streamlit#6158) Add sqlalchemy mypy types to test-requirements.txt (streamlit#6329)
📚 Context
As part of the upcoming
st.connectionfeature, we thought that it'd be useful to allowusers to set secrets via a global
~/.streamlit/secrets.tomlfile along with the per-projectsecrets.tomlfiles that are supported today.This works similarly to how
config.tomlfiles currently do: values set in the globalsecrets.tomlfile are overwritten by those set in per-project files.What kind of change does this PR introduce?
🧠 Description of Changes
🧪 Testing Done