fix(privacy): keep verbatim transcript text off INFO logs - #44
Conversation
Both the s2s and ambient bridges logged full conversation/utterance text at INFO, landing plaintext in journald. Move the verbatim text to DEBUG and gate verbosity behind an env (default INFO): - s2s: genesis_tool_service per-turn text -> logger.debug (count summary stays INFO); main.py basicConfig honors S2S_LOG_LEVEL. - ambient: pipeline utterance text -> DEBUG, keeping source/duration at INFO; server.py basicConfig honors AMBIENT_LOG_LEVEL. Mirrors MEETING/OMI_LOG_LEVEL. Level env is guarded (getattr fallback to INFO) so a bad value can't crash the bridge at startup. Verbatim text is recoverable via *_LOG_LEVEL=DEBUG; the persist-failure local fallback file already preserves transcripts, so nothing is lost. Tests pin text-absent-at-INFO + present-at-DEBUG + count-summary-logged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b18258705
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Mirrors MEETING_LOG_LEVEL / OMI_LOG_LEVEL in the sibling bridges. | ||
| logging.basicConfig( | ||
| level=logging.INFO, | ||
| level=getattr(logging, os.environ.get("S2S_LOG_LEVEL", "INFO").upper(), logging.INFO), |
There was a problem hiding this comment.
If either environment variable contains an invalid value that happens to match another uppercase attribute of logging—for example, S2S_LOG_LEVEL=BASIC_FORMAT—getattr returns that non-level value and logging.basicConfig() raises during import, preventing the bridge from starting instead of falling back to INFO as intended. The identical expression in bridges/ambient_bridge/server.py has the same failure mode; restrict the lookup to recognized level names or verify that the resolved value is an integer.
Useful? React with 👍 / 👎.
| # Mirrors MEETING_LOG_LEVEL / OMI_LOG_LEVEL in the sibling bridges. | ||
| logging.basicConfig( | ||
| level=logging.INFO, | ||
| level=getattr(logging, os.environ.get("S2S_LOG_LEVEL", "INFO").upper(), logging.INFO), |
There was a problem hiding this comment.
When the supported python -m app.main entry point runs with S2S_LOG_LEVEL=WARNING or a stricter level, logger is named __main__, but the existing logging.getLogger("__main__").setLevel(logging.INFO) immediately lowers that logger's threshold back to INFO. As a result, the numerous INFO messages in main.py continue reaching journald despite the new environment setting; remove that override or derive it from the configured level.
Useful? React with 👍 / 👎.
| # Mirrors MEETING_LOG_LEVEL / OMI_LOG_LEVEL in the sibling bridges. | ||
| logging.basicConfig( | ||
| level=logging.INFO, | ||
| level=getattr(logging, os.environ.get("S2S_LOG_LEVEL", "INFO").upper(), logging.INFO), |
There was a problem hiding this comment.
When the bridge is launched directly with configuration supplied through its built-in python-dotenv support, S2S_LOG_LEVEL exists only in .env, but this expression reads the process environment before dotenv.load_dotenv() runs at line 40. The bridge therefore remains at INFO and the advertised DEBUG transcript logging cannot be enabled this way, even though the other .env settings are read later during initialization; load dotenv before configuring logging.
Useful? React with 👍 / 👎.
- Load dotenv BEFORE basicConfig so S2S_LOG_LEVEL from .env is honored. - Validate the resolved level (isinstance int) in both s2s + ambient, so a bad value that resolves to a non-level attr (e.g. logging.BASIC_FORMAT) falls back to INFO instead of crashing the bridge at startup. - Drop the __main__->INFO override so S2S_LOG_LEVEL governs main's own logs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What
Both the s2s and ambient bridges logged full conversation/utterance text at INFO, landing plaintext in the systemd journal. This moves the verbatim text to DEBUG and gates verbosity behind an env (default INFO).
genesis_tool_serviceper-turn text →logger.debug(count summary stays INFO);main.pyhonorsS2S_LOG_LEVEL.pipelineutterance text → DEBUG, keeping source/duration at INFO;server.pyhonorsAMBIENT_LOG_LEVEL. Mirrors the existingMEETING_LOG_LEVEL/OMI_LOG_LEVEL.The level env is guarded (
getattrfallback to INFO) so a bad value can't crash the bridge at startup.Why
Captured voice content in journald plaintext is a privacy risk. Verbatim text stays recoverable on demand via
*_LOG_LEVEL=DEBUG, and the persist-failure local fallback file already preserves transcripts — so nothing is lost.Testing
pytest bridges/s2s_bridge/tests/test_genesis_tool_service.py— 15 passed. New tests pin: text absent at INFO + count summary still logged; text present at DEBUG. Verify-RED confirmed.pipeline.pychange has no unit test (sherpa_onnx unavailable locally) — symmetric to the tested s2s change; covered once the pytest CI job (separate PR) installs the deps.