Skip to content

fix(home-assistant): respect OOMOL_CONNECT_ALLOW_PRIVATE_NETWORK - #220

Merged
l1shen merged 1 commit into
oomol-lab:mainfrom
reopenpilot:worktree-ha-private-network
Jul 29, 2026
Merged

fix(home-assistant): respect OOMOL_CONNECT_ALLOW_PRIVATE_NETWORK#220
l1shen merged 1 commit into
oomol-lab:mainfrom
reopenpilot:worktree-ha-private-network

Conversation

@reopenpilot

Copy link
Copy Markdown
Contributor

Summary

  • Home Assistant connections to a LAN instance fail with request URL must not target private or reserved IP addresses, even when the deployment sets
    OOMOL_CONNECT_ALLOW_PRIVATE_NETWORK.
  • src/providers/home_assistant/executors.ts never passed allowPrivateNetwork to defineProviderExecutors, so the flag had no effect for this
    provider.
  • Adds the one-line opt-in, matching the 26 other providers that already wire it.

Problem

defineProviderExecutors builds a private-network-aware egress fetch only when a provider opts in; otherwise it falls back to the public-only providerFetch. Home Assistant took that fallback, so the flag could never reach a 192.168.x.x or 10.x.x.x instance — the self-hosted case it exists for. homeassistant.local:8123, the placeholder shown in the connection form, was unreachable for the same reason.

Default behavior is unchanged: with the flag unset, private targets stay blocked. Loopback, link-local, and cloud-metadata targets stay blocked either way.

Scope

Only the executor needed wiring. Home Assistant has no assertPublicHttpUrl call site to thread the flag into resolveHomeAssistantBaseUrl and validateHomeAssistantCredential both route through the provider-local normalizeBaseUrl — and no proxy.registry.ts entry. Migrating normalizeBaseUrl onto the shared assertPublicHttpUrl is a separate change, not attempted here.

Tests

Added to src/providers/provider-runtime.test.ts rather than a provider-local file, per AGENTS.md: "Keep open-source-only shared-infrastructure tests beside the shared module rather than inside a provider directory." Three cases cover the shared opt-in path: default blocks a private target, an opted-in executor reaches it once the flag is enabled, and loopback stays blocked even then.

Verification

npm run fix-check (clean) and npm test — 57 files, 566 tests passing.

Home Assistant instances on a LAN (e.g. 10.0.0.5) were rejected by the SSRF
egress guard even when the deployment set OOMOL_CONNECT_ALLOW_PRIVATE_NETWORK,
because the provider's executor definition never passed allowPrivateNetwork to
defineProviderExecutors. Without that opt-in the runtime falls back to the
shared public-only providerFetch, so the deployment flag had no effect for
Home Assistant.

Pass allowPrivateNetwork: isPrivateNetworkAccessAllowed, matching the 26 other
providers that already wire it.

Default behavior is unchanged: with the flag unset, private targets stay
blocked. Loopback, link-local, and cloud-metadata targets remain blocked in
both states.

Cover the opt-in beside the shared module in provider-runtime.test.ts, per
AGENTS.md ("Keep open-source-only shared-infrastructure tests beside the
shared module rather than inside a provider directory").
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Summary by CodeRabbit

  • New Features
    • Home Assistant connections can access private network targets when private network access is enabled.
  • Bug Fixes
    • Added safeguards to block private network requests for integrations that do not explicitly support them.
    • Loopback destinations remain blocked even when private network access is enabled.

Walkthrough

Home Assistant executors now set allowPrivateNetwork to isPrivateNetworkAccessAllowed. Provider runtime tests cover blocking private targets for non-opted-in executors, allowing private IP targets for opted-in executors when enabled, and continuing to block loopback destinations.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required type(scope): subject format and accurately describes the Home Assistant private-network fix.
Description check ✅ Passed The description is directly related to the Home Assistant executor change and the added runtime tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/providers/provider-runtime.test.ts (1)

142-150: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Define one named executor test-context interface.

  • src/providers/provider-runtime.test.ts#L142-L150: replace the inline generic with the shared interface.
  • src/providers/provider-runtime.test.ts#L162-L171: reuse that interface.
  • src/providers/provider-runtime.test.ts#L181-L190: reuse that interface.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/providers/provider-runtime.test.ts` around lines 142 - 150, Define one
named executor test-context interface for the fetcher context, then replace the
inline generic at src/providers/provider-runtime.test.ts lines 142-150 and reuse
that interface at lines 162-171 and 181-190 when calling
defineProviderExecutors.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/providers/provider-runtime.test.ts`:
- Around line 142-150: Define one named executor test-context interface for the
fetcher context, then replace the inline generic at
src/providers/provider-runtime.test.ts lines 142-150 and reuse that interface at
lines 162-171 and 181-190 when calling defineProviderExecutors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7441883b-4799-4784-bec9-eee73d775548

📥 Commits

Reviewing files that changed from the base of the PR and between fe7dd3c and 2b6f540.

📒 Files selected for processing (2)
  • src/providers/home_assistant/executors.ts
  • src/providers/provider-runtime.test.ts

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