From bb45fb8ede0d065e90f8fa426185b55057c9a3c7 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Thu, 17 Sep 2026 07:02:35 +0700 Subject: [PATCH] fix(providers): let a dashboard OFF for private provider URLs beat the env opt-in (#13323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed on the tip: `src/shared/network/outboundUrlGuardPolicy.ts` only checked `isTrueValue(dbValue)`, so an explicit dashboard OFF fell through to the env opt-in instead of overriding it — the operator turning something off in the UI had no effect. Probe on your head: 7/7 pass. Thanks, @datrixlab — an explicit OFF in the UI losing to an env var is the kind of thing that erodes trust in the whole settings surface. **Batch validation** — boarded with the other 10 PRs of your batch into one worktree cut from `release/v3.8.51`; every PR verified as an ancestor of the combined HEAD before validating. - Focused tests across all 11 PRs: **104/104 pass** on the combined tree. - Gates on the combined tree: `check-changelog-integrity` PASS, `check-complexity` PASS, `check-cognitive-complexity` PASS, `typecheck:core` PASS, `check:open-sse-typecheck` PASS. - `check-file-size` is red, but reproduces with byte-identical line counts on the pure `release/v3.8.51` tip (`open-sse/handlers/imageGeneration.ts` 3304, `open-sse/services/combo/roundRobinCombo.ts` 1221, `open-sse/utils/stream.ts` 3115). Inherited base-red, nothing added by this batch — it is also why this PR's "Fast Quality Gates" check was red. --- .../13323-private-provider-urls-db-off.md | 1 + src/shared/network/outboundUrlGuardPolicy.ts | 16 ++++-- .../outbound-url-guard-feature-flag.test.ts | 54 +++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 changelog.d/fixes/13323-private-provider-urls-db-off.md diff --git a/changelog.d/fixes/13323-private-provider-urls-db-off.md b/changelog.d/fixes/13323-private-provider-urls-db-off.md new file mode 100644 index 0000000000..1f2f99668b --- /dev/null +++ b/changelog.d/fixes/13323-private-provider-urls-db-off.md @@ -0,0 +1 @@ +- **fix(providers):** Switching "Allow Private Provider URLs" off in the dashboard now takes effect when `OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS=true` is set in the environment ([#13323](https://github.com/diegosouzapw/OmniRoute/pull/13323)) diff --git a/src/shared/network/outboundUrlGuardPolicy.ts b/src/shared/network/outboundUrlGuardPolicy.ts index a991647288..322d1bce35 100644 --- a/src/shared/network/outboundUrlGuardPolicy.ts +++ b/src/shared/network/outboundUrlGuardPolicy.ts @@ -1,3 +1,4 @@ +import { getFeatureFlagOverride } from "@/lib/db/featureFlags"; import { resolveFeatureFlag } from "@/shared/utils/featureFlags"; import { OutboundUrlGuardError, @@ -32,16 +33,21 @@ export function arePrivateProviderUrlsAllowed() { // the dashboard ("Allow Private Provider URLs"). This is critical for the // Electron build (#2575) where the server is spawned with the env value // captured at boot, so subsequent UI toggles only land in the DB and the - // env-first ordering would otherwise mask them. + // env-first ordering would otherwise mask them. That holds for a toggle OFF + // too: an override of "false" must not be re-enabled by the env opt-in below. + let dbValue: string | undefined; try { - const dbValue = resolveFeatureFlag(PRIVATE_PROVIDER_URLS_ENV); - if (isTrueValue(dbValue)) return true; + dbValue = getFeatureFlagOverride(PRIVATE_PROVIDER_URLS_ENV); } catch { // DB not initialized yet — fall through to env-only check. } - // 2) Explicit env opt-in (for headless/Docker users who set it before boot). - if (isTrueValue(process.env[PRIVATE_PROVIDER_URLS_ENV])) return true; + if (dbValue !== undefined && dbValue !== "") { + if (isTrueValue(dbValue)) return true; + } else if (isTrueValue(process.env[PRIVATE_PROVIDER_URLS_ENV])) { + // 2) Explicit env opt-in (for headless/Docker users who set it before boot). + return true; + } // 3) Legacy escape hatch — disabling the outbound guard implies allowing // private URLs. diff --git a/tests/unit/outbound-url-guard-feature-flag.test.ts b/tests/unit/outbound-url-guard-feature-flag.test.ts index a58dacb68a..161dba12bc 100644 --- a/tests/unit/outbound-url-guard-feature-flag.test.ts +++ b/tests/unit/outbound-url-guard-feature-flag.test.ts @@ -78,3 +78,57 @@ test("arePrivateProviderUrlsAllowed default (no env, no DB) returns false", asyn }); }); }); + +test("arePrivateProviderUrlsAllowed honors DB override = 'false' even when env is 'true'", async () => { + await withEnv("true", async () => { + await withDbOverride("false", async () => { + const { arePrivateProviderUrlsAllowed, getProviderValidationGuard } = + await import("../../src/shared/network/outboundUrlGuardPolicy.ts"); + assert.equal( + arePrivateProviderUrlsAllowed(), + false, + "a dashboard OFF must not be re-enabled by the env opt-in" + ); + assert.equal(getProviderValidationGuard(), "block-metadata"); + }); + }); +}); + +test("DB override = 'false' leaves the legacy OUTBOUND_SSRF_GUARD_ENABLED=false hatch as it was", async () => { + const prev = process.env.OUTBOUND_SSRF_GUARD_ENABLED; + process.env.OUTBOUND_SSRF_GUARD_ENABLED = "false"; + try { + await withEnv(undefined, async () => { + await withDbOverride("false", async () => { + const { arePrivateProviderUrlsAllowed } = + await import("../../src/shared/network/outboundUrlGuardPolicy.ts"); + assert.equal(arePrivateProviderUrlsAllowed(), true); + }); + }); + } finally { + if (prev === undefined) delete process.env.OUTBOUND_SSRF_GUARD_ENABLED; + else process.env.OUTBOUND_SSRF_GUARD_ENABLED = prev; + } +}); + +test("a DB override = 'false' for local provider URLs still restores public-only", async () => { + const LOCAL_KEY = "OMNIROUTE_ALLOW_LOCAL_PROVIDER_URLS"; + const prev = process.env[LOCAL_KEY]; + delete process.env[LOCAL_KEY]; + const { setFeatureFlagOverride, removeFeatureFlagOverride } = + await import("../../src/lib/db/featureFlags.ts"); + setFeatureFlagOverride(LOCAL_KEY, "false"); + try { + await withEnv(undefined, async () => { + await withDbOverride(undefined, async () => { + const { getProviderValidationGuard } = + await import("../../src/shared/network/outboundUrlGuardPolicy.ts"); + assert.equal(getProviderValidationGuard(), "public-only"); + }); + }); + } finally { + removeFeatureFlagOverride(LOCAL_KEY); + if (prev === undefined) delete process.env[LOCAL_KEY]; + else process.env[LOCAL_KEY] = prev; + } +});