Skip to content

[security] fix(ohmo): secure default remote channel allowlists#147

Merged
tjb-tech merged 1 commit intoHKUDS:mainfrom
Hinotoi-agent:fix/secure-default-channel-allowlist
Apr 15, 2026
Merged

[security] fix(ohmo): secure default remote channel allowlists#147
tjb-tech merged 1 commit intoHKUDS:mainfrom
Hinotoi-agent:fix/secure-default-channel-allowlist

Conversation

@Hinotoi-agent
Copy link
Copy Markdown
Contributor

@Hinotoi-agent Hinotoi-agent commented Apr 15, 2026

Summary

This PR hardens the remote-channel admission boundary for ohmo/OpenHarness by making channel allowlists secure by default instead of implicitly trusting every remote sender.

It addresses a verified gateway-adjacent variant of the trust-boundary family previously discussed in HKUDS/OpenHarness#127:

  • newly enabled remote channels no longer default to allow_from = ["*"]
  • the gateway/config wizard now requires an explicit operator choice for open access instead of silently allowing everyone
  • empty allowlists are now treated as a valid secure-default deny-all posture instead of a startup error
  • regression tests cover explicit allowlists and the deny-all secure default

Security issues covered

Issue Impact Severity
Insecure default remote channel allowlist Arbitrary remote senders can reach a host-backed agent if an operator enables a channel and keeps the default wildcard ACL High

Before this PR

  • enabling a remote channel inherited allow_from = ["*"] from the default channel config model
  • the interactive ohmo gateway wizard also defaulted channel admission to *, which silently opened access to every remote sender unless the operator noticed and changed it
  • ChannelManager treated an empty allowlist as fatal startup misconfiguration, which made a deny-all secure default impossible to keep enabled
  • this left a gateway-adjacent trust boundary weaker than intended even after the command-level hardening in #127

After this PR

  • new channel config defaults are deny-all until an operator explicitly adds allowed identities or intentionally chooses *
  • the ohmo CLI config wizard now presents explicit operator choice: specific identities, blank deny-all, or * for open access
  • empty allowlists are accepted as a valid secure-default configuration and produce a warning instead of aborting startup
  • regression tests lock in both explicit allowlists and blank deny-all behavior

Explicit operator choice

Secure default example:

{
  "enabled_channels": ["telegram"],
  "channel_configs": {
    "telegram": {
      "allow_from": [],
      "token": "..."
    }
  }
}

Explicitly open example:

{
  "enabled_channels": ["telegram"],
  "channel_configs": {
    "telegram": {
      "allow_from": ["*"],
      "token": "..."
    }
  }
}

When disabled/blank:

  • remote access is denied until the operator explicitly configures trusted identities

When explicitly enabled with *:

  • the operator restores intentionally open access and accepts that any reachable remote sender may interact with the bot

Why this matters

  • the broken boundary is the first gate into the remote gateway path: who is allowed to talk to the bot at all
  • if that gate defaults to *, any remote sender who can reach the configured channel can drive a host-backed agent session
  • because OpenHarness permits read-only tools automatically in default mode, unauthorized remote admission can compose into unauthorized file disclosure and other host-backed read access without any additional admin-only command bypass
  • this is a variant of the same broader remote gateway trust-boundary family as #127, but it lives one layer earlier at channel admission rather than slash-command privilege checks

How this differs from HKUDS/OpenHarness#127

This PR is related to #127, but it fixes a different issue and a different layer of the remote trust boundary.

1. Different boundary

  • #127 hardened what an already-admitted remote user could do after entering the gateway path
  • specifically, it focused on:
    • local-only administrative slash commands being reachable remotely
    • /memory show reading outside its intended directory
  • this PR instead hardens who gets admitted to the remote gateway path in the first place
  • in other words:
    • #127 = command/path enforcement after remote entry
    • this PR = remote sender admission before those later checks even matter

2. Different trigger condition

  • #127 required a remote user to reach the gateway slash-command surface and then invoke a dangerous command path
  • this PR is triggered earlier:
    • an operator enables a channel
    • the default config silently trusts every sender via allow_from = ["*"]
    • any reachable sender can then access the runtime
  • that means this issue exists even if the later command-specific hardening from #127 is present

3. Different vulnerable code

#127 changed:

  • ohmo/gateway/runtime.py
  • ohmo/gateway/models.py
  • ohmo/gateway/service.py
  • ohmo/workspace.py
  • src/openharness/commands/registry.py
  • related tests

This PR changes a different set of files tied to admission defaults and operator configuration:

  • src/openharness/config/schema.py
  • src/openharness/channels/impl/manager.py
  • ohmo/cli.py
  • tests/test_ohmo/test_cli.py

That file split reflects the boundary split:

  • #127 patched runtime command handling and one file-read command path
  • this PR patches config defaults, startup handling, and operator-facing setup flow

4. Different failure mode

  • #127 was about dangerous behavior remaining remotely reachable for users who had already crossed the gateway admission boundary
  • this PR is about the gateway admission boundary itself being insecure by default
  • the operator failure here is not “explicitly opted into remote admin”
  • the operator failure is “enabled a channel and inherited a wildcard ACL without making an explicit trust decision”

5. Why this is a variant instead of a duplicate

  • both issues belong to the same broader remote gateway trust-boundary family
  • both can lead to unauthorized host-backed behavior from remote chat users
  • but the exploit primitive is different:
    • #127: dangerous remote command/file-read paths after admission
    • this PR: insecure default admission policy before command/path checks
  • fixing #127 does not remove this earlier-layer wildcard-admission behavior
  • fixing this PR does not replace the command/path hardening from #127
  • both are needed for a complete secure-default story

Attack flow

operator enables a remote channel
    -> channel config defaults to allow_from = ["*"]
        -> arbitrary remote sender passes admission check
            -> unauthorized remote user reaches host-backed agent runtime
                -> read-only host-backed capabilities become reachable

Affected code

Issue Files
Insecure default remote channel allowlist src/openharness/config/schema.py, src/openharness/channels/impl/manager.py, ohmo/cli.py, tests/test_ohmo/test_cli.py

Root cause

Issue 1: insecure default remote channel allowlist

  • the compatibility channel config model defaulted allow_from to [*], which made open remote admission the implicit behavior for newly enabled channels
  • the ohmo configuration wizard reinforced the same wildcard default in operator prompts
  • the channel manager rejected an empty allowlist as fatal misconfiguration, so operators could not keep a deny-all secure default while enabling a channel for later explicit rollout
  • together, those choices made the remote gateway trust boundary permissive by default instead of requiring explicit operator intent

CVSS assessment

Issue CVSS v3.1 Vector
Insecure default remote channel allowlist 8.1 High CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:N

Rationale:

  • enabling a remote channel with the default wildcard admission policy can expose a host-backed agent to arbitrary remote senders without any further privileged foothold
  • the strongest practical impact is unauthorized disclosure through host-backed read surfaces that become reachable after admission succeeds
  • integrity impact is lower than the earlier #127 command-level issue because this PR does not rely on privileged admin-command acceptance; it fixes the admission boundary itself

Safe reproduction steps

1. Insecure default remote channel allowlist

  1. Use a vulnerable OpenHarness snapshot before this patch.
  2. Initialize or configure ohmo and enable a remote channel such as Telegram through the interactive wizard.
  3. Accept the default channel admission value without replacing the wildcard.
  4. Observe that the resulting channel config stores allow_from = ["*"].
  5. Send a message from an arbitrary reachable remote sender on that channel.
  6. Observe that the sender passes the channel admission check and reaches the host-backed runtime.

2. Deny-all secure default was previously impossible

  1. Use a vulnerable OpenHarness snapshot before this patch.
  2. Configure a channel with an empty allow_from list to intentionally deny all remote senders until explicit rollout.
  3. Start the gateway/channel manager.
  4. Observe that startup aborts because the empty allowlist is treated as invalid rather than as a secure default.

Expected vulnerable behavior

  • newly enabled remote channels should not silently trust every remote sender by default
  • operators should be able to keep a deny-all admission policy until they explicitly configure trusted identities
  • pre-patch behavior instead stored wildcard admission by default and rejected deny-all secure-default configurations

Changes in this PR

  • change BaseChannelConfig.allow_from to default to an empty list instead of [*]
  • keep wildcard access available only as an explicit operator choice, not an implicit default
  • update ChannelManager so an empty allowlist becomes a warning-backed secure default instead of a fatal startup error
  • update the ohmo CLI wizard prompt text and defaults so operators explicitly choose identities, blank deny-all, or *
  • extend the CLI summary output to call out enabled channels that remain deny-all until configured
  • add regression tests covering explicit allowlists and blank secure-default allowlists

Files changed

Category Files What changed
Channel config defaults src/openharness/config/schema.py switch remote channel admission from implicit wildcard to deny-all default
Channel startup behavior src/openharness/channels/impl/manager.py accept empty allowlists as a secure-default posture and warn instead of aborting
Operator UX ohmo/cli.py remove wildcard-default prompts, allow blank deny-all input, and print secure-default rollout hints
Regression coverage tests/test_ohmo/test_cli.py add/adjust tests for explicit allowlists and blank deny-all behavior

Maintainer impact

  • the patch is narrow and limited to remote channel admission defaults, CLI configuration UX, and tests
  • it does not change unrelated frontend behavior, model/runtime behavior, or the command-level hardening introduced in #127
  • existing deployments that explicitly set allow_from = ["*"] keep working as-is
  • new and reconfigured deployments now require explicit operator intent before opening the remote admission boundary
  • this makes the gateway trust boundary easier to reason about in future refactors because secure-default behavior lives in the default config and wizard flow, not just in documentation

Fix rationale

  • the right security boundary is the first remote admission check, not only later command-level protections
  • making wildcard remote access explicit is safer and more durable than relying on operators to notice and override a permissive default
  • accepting blank allowlists as valid configuration keeps secure rollout possible: operators can enable a channel, verify connectivity, and only later add trusted identities
  • the tests are sufficient for this change because they cover both the explicit trusted-identity path and the secure deny-all default that was previously impossible to preserve

Reference patterns from other software

These systems are not identical products, but they reflect the same secure-default principle: remote or untrusted principals should not inherit privileged access merely because a capability was enabled.

Type of change

  • Security fix
  • Tests
  • Documentation update
  • Refactor with no behavior change

Test plan

  • uv run --extra dev pytest tests/test_ohmo/test_cli.py -q
  • uv run --extra dev ruff check src/openharness/config/schema.py src/openharness/channels/impl/manager.py ohmo/cli.py tests/test_ohmo/test_cli.py
  • Full project test suite was not run for this targeted fix

Executed with:

  • cd /tmp/openharness && uv run --extra dev pytest tests/test_ohmo/test_cli.py -q
  • cd /tmp/openharness && uv run --extra dev ruff check src/openharness/config/schema.py src/openharness/channels/impl/manager.py ohmo/cli.py tests/test_ohmo/test_cli.py

Disclosure notes

  • this PR is intentionally bounded to a gateway-adjacent variant of the remote trust-boundary family already discussed publicly in HKUDS/OpenHarness#127
  • it does not claim a new command-level privilege-escalation primitive beyond what #127 already covered; it hardens the earlier channel-admission layer that can still expose the host-backed runtime under insecure defaults
  • no unrelated files were changed
  • the repository still does not publish a SECURITY.md policy, so public PR-based security hardening remains the most visible maintainer-facing path in this repo today

@Hinotoi-agent
Copy link
Copy Markdown
Contributor Author

Added a detailed distinction section to the PR body:

  • ## How this differs from HKUDS/OpenHarness#127

That section now explains, in maintainer terms, why this PR is a related variant rather than a duplicate:

  • different boundary
  • different trigger condition
  • different vulnerable code
  • different failure mode
  • why both patches are needed for a complete secure-default story

If useful, the shortest framing is:

  • #127 hardens dangerous behavior after a remote user has already entered the gateway path
  • this PR hardens the earlier admission boundary so operators do not silently trust every remote sender by default

@tjb-tech tjb-tech merged commit fab40c6 into HKUDS:main Apr 15, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants