[security] fix(commands): keep diff local-only by default#253
Merged
tjb-tech merged 2 commits intoMay 16, 2026
Conversation
Contributor
Author
|
CI note for this run:
That failing test is outside this PR's touched files and appears timing/flakiness-related rather than related to the Additional local validation after seeing the failure: PYTHONPATH=src:. uv run pytest -o addopts='' tests/test_tools/test_integration_flows.py::test_agent_send_message_flow_restarts_completed_agent -qLocal result: |
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 the remote slash-command boundary for the repository diff command.
/diff fullcan include the full uncommitted workspace patch, so it should remain a local OpenHarness UI action unless an operator explicitly opts it into the remote-admin command allowlist./diffas local-only by default withremote_invocable=False.remote_admin_opt_in=True./diff fullis denied and does not return a marker value from an uncommitted Git diff.Security issues covered
/diff fullworkspace disclosureBefore this PR
/diffinherited the default remote-invocable slash-command behavior./diff fullcould execute the command against the active workspace.git diff HEAD, which may include.envedits, local configuration changes, unpublished patches, prompt/context files, or proprietary source changes./diff full.After this PR
/diffis local-only by default./diff fullreturns the existing local-only denial message.Why this matters
Remote chat channels are a different trust boundary from the local OpenHarness UI. Even when a sender is allowed to chat with the gateway, returning full local repository diffs can expose sensitive or unpublished workspace contents that the sender should not automatically receive.
How this differs from related issue/PR
This is the same remote slash-command trust-boundary family as prior hardening work, but it covers a distinct residual command:
/bridgelocal-only because it could spawn shell bridge sessions./taskslocal-only because it can spawn background shell tasks./diff full, which is a read-side workspace disclosure path throughgit diff HEAD.The fix is intentionally separate because the vulnerable command, handler, and impact are different: this PR prevents returning local workspace patches, not shell execution or configuration mutation.
Attack flow
Affected code
/diff fullworkspace disclosuresrc/openharness/commands/registry.py,ohmo/gateway/runtime.pyRoot cause
Remote
/diff fullworkspace disclosure:SlashCommand.remote_invocabledefaults toTrue./diffwas registered without overriding that default._diff_handler()returns fullgit diff HEADoutput for/diff full, but that local workspace inspection command was reachable from remote gateway messages.CVSS assessment
/diff fullworkspace disclosureCVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:NRationale: the issue requires access as an allowed remote channel/gateway sender, has low attack complexity, and primarily affects confidentiality of local workspace contents returned in Git diffs. Severity can be higher in deployments with broad remote channel admission or secret-heavy workspaces.
Safe reproduction steps
Start a vulnerable build where
/diffis registered with the defaultremote_invocable=Truebehavior.In the active workspace, create a Git repo with one committed file and then modify it without committing:
Send a remote gateway message with
/diff full.Observe that the final response contains the marker value from the uncommitted diff.
Expected vulnerable behavior
On vulnerable builds, the gateway returns a patch containing the uncommitted marker, for example:
After this PR, the remote gateway path returns:
Changes in this PR
/diffwithremote_invocable=False./diffwithremote_admin_opt_in=Trueso trusted operators can explicitly allow it using the existing remote-admin mechanism if they accept that exposure./diffcommand metadata./diff full, and asserts the marker is not leaked.Files changed
src/openharness/commands/registry.py/difflocal-only by default with explicit remote-admin opt-in supporttests/test_commands/test_registry.py/diffremote invocation metadatatests/test_ohmo/test_gateway.py/diff fulldenial and verifies a marker diff is not returnedMaintainer impact
/diffbehavior remains unchanged./diffthrough the existing administrative command allowlist.Fix rationale
/diff fullreads local workspace state and can return sensitive code or configuration changes. Keeping that command local-only by default matches the existing approach used for other local control-plane commands while preserving an explicit operator-controlled escape hatch for trusted deployments.Type of change
Test plan
/diffregistry and gateway regression tests.Executed with:
Local results:
3 passed114 passed, 7 warningsdatetime.utcnow()deprecation warnings in gateway tests.Token usage
Disclosure notes
/diff fullworkspace disclosure./diffworkspace disclosure path specifically.