Skip to content

Hide secrets from logging output - #104

Merged
dmgav merged 1 commit into
mainfrom
fix-secret-settings
Nov 9, 2026
Merged

dmgav merged 1 commit into
mainfrom
fix-secret-settings

Conversation

@thopkins32

Copy link
Copy Markdown
Contributor

The startup log currently dumps the full Settings object, which can include single_user_api_key and secret_keys. Redact those fields so they do not appear in logs.

Assisted-by: oh-my-pi:claude-opus-5.5
@thopkins32
thopkins32 requested review from dmgav and a balanced review from Copilot November 7, 2026 02:18
@thopkins32
thopkins32 removed the request for review from Copilot November 7, 2026 02:21
@dmgav

dmgav commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Since we are preparing to extend logging capabilities, we may need more robust solution that prevents accidental printing. For example, the secrets can be represented as pydantic.SecretStr type and URI can be represented as sqlalchemy.engine.url.URL:

In [2]: from pydantic import SecretStr

In [3]: a = SecretStr("Hello")

In [4]: print(a)
**********

In [5]: a
Out[5]: SecretStr('**********')

In [6]: a.get_secret_value()
Out[6]: 'Hello'


In [13]: from sqlalchemy.engine import make_url

In [14]: uri = "postgresql://user:hunter2@host:5432/db"

In [15]: url = make_url(uri) 

In [16]: url
Out[16]: postgresql://user:***@host:5432/db

In [17]: url.render_as_string(hide_password=False)
Out[17]: 'postgresql://user:hunter2@host:5432/db'

Tiled is currently using SecretStr to store access token https://github.com/bluesky/tiled/blob/49fe22daae308237470d739b2f87622319f4e81c/tiled/server/schemas.py#L432, the other secrets are stored as strings, so they potentially can leak into logs. @genematx

@thopkins32

Copy link
Copy Markdown
Contributor Author

Can we merge this in the interim? @dmgav

@dmgav
dmgav merged commit 51d7141 into main Nov 9, 2026
19 checks passed
@thopkins32
thopkins32 deleted the fix-secret-settings branch November 9, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants