[security] fix(commands): keep bridge local-only by default#208
Merged
tjb-tech merged 1 commit intoHKUDS:mainfrom Apr 28, 2026
Merged
Conversation
11 tasks
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
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.
Summary
This PR hardens a residual remote-gateway slash-command boundary related to the already-public command hardening family addressed in #127.
The
/bridgecommand can spawn bridge-managed shell sessions through/bridge spawn CMD. It was still registered with the defaultremote_invocable=True, which means an accepted remote channel/gateway sender could reach the command path from chat unless the deployment separately restricted the sender at the channel layer.This patch makes
/bridgelocal-only by default and adds regression coverage for both the command metadata and gateway enforcement path.Security issues covered
/bridge spawnshell executionBefore this PR
SlashCommand.remote_invocabledefaults toTrue./bridgewas registered without overriding that default.remote_invocableis explicitly false./bridge spawn CMDforwards attacker-controlled command text to the bridge session manager.After this PR
/bridgeis registered withremote_invocable=False./bridgeis markedremote_admin_opt_in=True, preserving an explicit trusted-operator opt-in path consistent with the existing gateway model./bridge spawn ...messages are denied by the gateway before the command handler runs.Why this matters
Remote channel/gateway users are a different trust boundary than a local OpenHarness UI operator.
/bridge spawnis not a read-only helper: it can start arbitrary shell commands in the configured project working directory as the OpenHarness process user. If reachable from remote chat, it can expose local files, credentials, workspace state, and repository contents, and it can mutate the working tree or start long-running processes without going through the normal model/tool permission flow.How this differs from #127
#127 added the remote command boundary and fixed known sensitive commands such as
/permissionsand the/memory showtraversal path.This PR is a same-family residual fix, not a claim that the earlier patch was incomplete in scope:
remote_invocableandremote_admin_opt_inmodel./bridgeremained registered with the default remote-invocable behavior./bridge spawnreaches shell subprocess creation, not permission-mode mutation or memory-file reads.Both fixes are needed because the gateway can only block a sensitive command if that command is explicitly marked local-only.
Attack flow
Affected code
src/openharness/commands/registry.pytests/test_ohmo/test_gateway.pytests/test_commands/test_registry.pyRoot cause
/bridgewas not explicitly marked local-only even though one of its subcommands starts shell processes.CVSS assessment
/bridge spawnshell executionAV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:HRationale:
Safe reproduction steps
On a vulnerable build, with a remote channel/gateway sender accepted by configuration:
A local validation harness can also prove the same boundary by checking that
/bridgewas registered as remote-invocable and that/bridge spawn ...starts a shell subprocess. I re-ran that harness against both the vulnerable base and this branch:Expected vulnerable behavior
/bridgeis considered remotely invocable./bridge spawn ...reaches the command handler from the remote gateway path.Changes in this PR
/bridgewithremote_invocable=False./bridgewithremote_admin_opt_in=Trueso trusted deployments can still opt in explicitly using the existing gateway admin-command mechanism./bridgeis local-only by default and eligible for explicit remote admin opt-in./bridge spawn ...is denied before the handler runs.Files changed
src/openharness/commands/registry.py/bridgelocal-only by default and explicit-admin-opt-in capabletests/test_commands/test_registry.py/bridgetests/test_ohmo/test_gateway.py/bridge spawnMaintainer impact
/bridgeis preserved./bridgeif they intentionally want remote bridge administration.Fix rationale
The gateway already has a command-level remote execution boundary. The least surprising and lowest-risk fix is to apply that existing boundary to
/bridge, because/bridge spawnis a local control-plane operation with direct process-execution effects.Type of change
Test plan
Executed locally:
PYTHONPATH=src:. uv run pytest -o addopts='' tests/test_commands/test_registry.py::test_bridge_command_is_marked_local_only tests/test_commands/test_registry.py::test_bridge_command_supports_explicit_remote_admin_opt_in tests/test_ohmo/test_gateway.py::test_runtime_pool_blocks_bridge_spawn_from_remote_messages tests/test_ohmo/test_gateway.py::test_runtime_pool_blocks_local_only_commands_from_remote_messages tests/test_ohmo/test_gateway.py::test_runtime_pool_allows_opted_in_remote_admin_commands -qPYTHONPATH=src:. uv run pytest -o addopts='' tests/test_commands/test_registry.py tests/test_ohmo/test_gateway.py -qPYTHONPATH=src:. uv run python -m compileall -q src/openharness/commands/registry.py tests/test_commands/test_registry.py tests/test_ohmo/test_gateway.pygit diff --checkuv run ruff check src/openharness/commands/registry.py tests/test_commands/test_registry.py tests/test_ohmo/test_gateway.pyorigin/mainat380bab4: marker file was written withREMOTE_BRIDGE_EXEC/bridgedenied, no bridge sessions created, marker file absentNote:
uv run ruff format --checkreports pre-existing whole-file formatting churn for the touched legacy files, so this PR keeps the diff minimal and does not include unrelated formatting-only rewrites.Disclosure notes