Fix audit findings: TUI system messages + history-on-switch, update_name uniqueness, REST version guard, DRY constants - #54
Merged
Conversation
…update_name uniqueness, REST guards, DRY constants Python-only fixes for a batch of verified audit findings. - [PY-E HIGH] TUI dropped every daemon-authored [system] message: widen message.ParticipantType to include "system" so Sender validates daemon-published payloads; _handle_message now routes them to add_system_message instead of swallowing them at parse time. Audited every consumer of message.ParticipantType — two test helpers feed registry.join (participant.ParticipantType, claude/human only) and now import the type from participant. Removed the dead is_broadcast clause (is_for already covers broadcasts) and the now-reachable branch's pyright-ignore. - [PY-E MED] TUI never fetched history on channel switch: _resubscribe_conversation now fetches REST history once per conversation (guarded by _fetched_history), independent of the MQTT connection. - [PY-B MED] update_name had no uniqueness check: tool_comms_update_name now refuses a rename onto a name held by a different participant (clear error); case-only self-rename still allowed. - [PY-A LOW] _api_artifacts_get crashed to 500 on non-numeric ?version: extracted build_artifact_get_route with a 400 guard mirroring /api/messages. - [PY-A LOW] web command opened http://0.0.0.0:PORT under bind-all host: now uses _web_ui_urls()[0] (localhost), matching doctor/banner behaviour. - [DRY MED] key regex ^[0-9a-f]{8}$ reimplemented at ~7 sites: all routed to the canonical participant.KEY_PATTERN. - [DRY MED] system sender key "00000000" hardcoded across files: promoted to message.SYSTEM_SENDER_KEY and single-sourced in notifier/mcp_tools/mcp_server/cli. Tests added: end-to-end system-message rendering, history-fetch-once-per-conv, update_name collision (+ case-insensitive + self-rename), artifact ?version 400, web bind-all->localhost, and DRY single-source guards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Python-only fixes for a batch of verified audit findings. Web files untouched (concurrent agents own them); deferred-security items (broker auth, read-side authz/membership gating, /api/invite auth model) intentionally out of scope.
Findings fixed
[PY-E HIGH] TUI silently dropped every
[system]message.Sender.typeexcluded"system", soMessage.from_mqtt_payloadraisedValidationErrorand_handle_messageswallowed every daemon notification. Widenedmessage.ParticipantTypeto include"system"(the registry'sparticipant.ParticipantTypestaysclaude/human— system is not joinable) and routed system payloads toadd_system_message. Audited every consumer ofmessage.ParticipantType: two test helpers that feedregistry.joinnow import the type fromparticipant. Also dropped the deadis_broadcastclause (is_foralready covers broadcasts).[PY-E MED] TUI never fetched server history on channel switch.
_resubscribe_conversationnow fetches REST history once per conversation (guarded by_fetched_history), independent of the MQTT connection.[PY-B MED]
update_namehad no uniqueness check.tool_comms_update_namenow refuses a rename onto a name held by a different participant (clear error); case-only self-rename still allowed.[PY-A LOW]
_api_artifacts_get500 on?version=abc. Extractedbuild_artifact_get_route(TestClient-coverable, mirrors the otherbuild_*_routefactories) with a 400 guard matching/api/messages.[PY-A LOW]
webopenedhttp://0.0.0.0:PORTunder bind-all host. Now uses_web_ui_urls()[0](localhost), matchingdoctor/banner behaviour.[DRY MED] key regex
^[0-9a-f]{8}$reimplemented at ~7 sites. All routed to canonicalparticipant.KEY_PATTERN.[DRY MED] system sender key
"00000000"hardcoded across files. Promoted tomessage.SYSTEM_SENDER_KEY; single-sourced in notifier/mcp_tools/mcp_server/cli.Tests
Added: end-to-end system-message rendering, history-fetch-once-per-conv, update_name collision (+ case-insensitive + self-rename allowed), artifact
?version400/404/ok, web bind-all->localhost, and DRY single-source guards.Gates
pytest: 1548 passed, 1 skipped (was 1525/1 — +23 tests, no new skips)🤖 Generated with Claude Code