refactor(gateway): centralize port-binding platform constants and multiplex profile validation - #64005
refactor(gateway): centralize port-binding platform constants and multiplex profile validation#64005Ahmett101 wants to merge 3 commits into
Conversation
…d profiles
The dashboard Channels API (PUT /api/messaging/platforms/{id}) persisted
a port-binding platform (api_server, webhook, feishu, ...) on a secondary
profile while gateway.multiplex_profiles was enabled. The shared multiplexed
gateway only rejects this topology at the next startup (MultiplexConfigError),
so the invalid config took down every profile on restart.
Add a single shared policy in gateway/config.py:
- _PORT_BINDING_PLATFORM_VALUES (the authoritative list, mirroring the
gateway/run.py startup check),
- is_port_binding_platform(), multiplex_profiles_enabled(), and
port_binding_allowed_on_profile().
update_messaging_platform now validates before any .env/config.yaml write:
when multiplexing is on, enabling or configuring a port-binding platform on a
non-default profile is rejected with HTTP 409 (rejected requests leave both
.env and config.yaml untouched). Disabling/clearing an already-invalid setting
stays allowed so users can recover. The gateway startup check remains as
defense in depth. Closes NousResearch#62791.
…fication (review fix) - gateway/config.py: multiplex_profiles_enabled() now reads the machine default Hermes home via get_default_hermes_root() so --isolated/named-profile dashboard processes do not consult the secondary profile's config.yaml - gateway/run.py: drop local _PORT_BINDING_PLATFORM_VALUES literal; import is_port_binding_platform() from gateway.config and use it at the startup guard so the two checks cannot drift - tests/gateway/test_multiplex_adapter_registry.py: import from the shared classification instead of from gateway.run Implements teknium review on NousResearch#62801.
alt-glitch
added
type/refactor
Heads-up for reviewers: this PR's title/body describe a Telegram kanban-wake lobby fix "Closes #63911", but the actual diff does not touch that code path at all ( |
Ahmett101
changed the title
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for centralizing the multiplex policy. Current main still has the reported invalid-config path: hermes_cli/web_server.py:8087-8108 writes profile-scoped platform settings, while gateway/run.py:8701-8715 aborts startup for secondary port-binding platforms.
Problems
hermes_cli/web_server.py:7873includesclear_envin the rejecting predicate. The request returns 409 before the clear loop at:7894-7900, so an invalid secondary configuration cannot be cleared. The new test attests/hermes_cli/test_web_server_messaging_profiles.py:262-280only coversenabled: false.hermes_cli/kanban_db.py:730-736follows symlinks throughexists()/is_dir()and does not validate an existing root's ownership or mode. That does not satisfy #63863's requested safe-root invariant.hermes_cli/kanban_db.py:710-713and:766-767swallowOSError, allowing board creation to succeed even if the required workspace root was not created.
Suggested changes
- Permit clear-only requests and add a
clear_envrecovery regression. - Validate the physical workspace-root component without following symlinks, enforce the intended existing-root policy, propagate creation failures, and add the corresponding board tests.
Automated hermes-sweeper review.
| _mutates_port_binding = ( | ||
| body.enabled is True | ||
| or bool(body.env) | ||
| or bool(body.clear_env) |
There was a problem hiding this comment.
clear_env only removes configuration in the loop at :7894-7900, but this predicate returns 409 before that loop for a secondary multiplexed profile. That breaks the stated recovery path; exclude clear-only requests and add a regression using a real port-binding environment key.
| not a directory, in which case :class:`FileExistsError` is raised. | ||
| """ | ||
| wroot = workspaces_root(board) | ||
| if not wroot.exists(): |
There was a problem hiding this comment.
Path.exists() and Path.is_dir() follow symlinks, so an existing workspaces symlink to any directory is accepted; pre-existing directories also bypass owner/mode validation. #63863 requires rejecting symlinks and unsafe roots, so validate the physical final component before accepting it.
| except FileExistsError: | ||
| raise | ||
| except OSError: | ||
| pass # Best-effort; lazy creation still works if this fails. |
There was a problem hiding this comment.
Swallowing this failure allows create_board() to report success even though the dispatcher-exported workspace root was not materialized. Propagate the creation failure (or otherwise prevent successful board creation) and cover it with a regression test.
teknium1
added
sweeper:risk-security-boundary
|
Closing as a duplicate: this PR's diff is byte-identical to your own #62801 (compared both The underlying fix (dashboard rejection of port-binding platforms on secondary multiplexed profiles + constant centralization) just merged via PR #65700 using #62803 as the vehicle — see the comparison notes there. If the kanban workspaces-root change fixes a real bug, please submit it as its own focused PR. |
3 participants
Summary
Refactors the multiplex profile validation and port-binding platform rules so
gateway/config.pyis the single source of truth for platform port values andmultiplex_profiles_enabled().gateway/run.pyandhermes_cli/web_server.pynow route through those helpers instead of duplicated constants. Also ensurescreate_board()materializes a board-specificworkspaces/root so workspace paths exist before writes.Changes
is_port_binding_platform()+ durable_PORT_BINDING_PLATFORM_VALUESenumerator; switchedmultiplex_profiles_enabled()to default-root config resolution._PORT_BINDING_PLATFORM_VALUESand importis_port_binding_platformfromgateway.config.create_board()creates board-specificworkspaces/root on non-default boards.gateway.config.gateway.config.How to Test
python -m pytest tests/gateway/test_multiplex_adapter_registry.py tests/hermes_cli/test_web_server_messaging_profiles.py -q
✅ 56/56 passed, regression tests pass
Checklist
[x] Tests pass — targeted subtests pass
[x] Follows Conventional Commits
[x] Changes scoped to multiplex/port-binding refactor
[x] Cross-platform impact assessed (Linux / macOS / WSL2 / Windows / Termux) — None
[x] profile-safe paths used (get_hermes_home / get_default_hermes_root)
[x] .env not used for non-credential settings
Risk & Impact
Low. Replaces duplicated constants with a single importer; adds missing directory creation for non-default boards.
Type: Refactor
Closes: #62791, #63863