fix(cli): split outboundUrlGuard's DB helpers so setup-opencode packages cleanly (#7682) - #7760
Conversation
There was a problem hiding this comment.
Code Review
This pull request splits the database and feature-flag-backed helpers of the outbound URL guard from outboundUrlGuard.ts into a new module outboundUrlGuardPolicy.ts. This resolves an issue where the CLI command omniroute setup-opencode crashes on global npm installs due to unresolved @/ path aliases. Feedback on the changes highlights a logic bug in arePrivateProviderUrlsAllowed where a DB override of "false" is ignored if the environment variable is set to "true", and a potential test failure on Windows due to symlinkSync requiring admin privileges for 'dir' symlinks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function arePrivateProviderUrlsAllowed() { | ||
| // 1) DB override takes precedence — it represents an explicit user toggle in | ||
| // 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. | ||
| try { | ||
| const dbValue = resolveFeatureFlag(PRIVATE_PROVIDER_URLS_ENV); | ||
| if (isTrueValue(dbValue)) return true; | ||
| } catch { | ||
| // DB not initialized yet — fall through to env-only check. | ||
| } |
There was a problem hiding this comment.
There is a logic bug where an explicit DB override of "false" (e.g., when a user disables private provider URLs in the dashboard) will be ignored if the environment variable OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS is set to "true". This is because isTrueValue(dbValue) evaluates to false, causing the function to fall through to the environment variable check. To ensure the DB override takes precedence as intended, check if dbValue is defined and not empty, and return its evaluated truthiness directly.
| export function arePrivateProviderUrlsAllowed() { | |
| // 1) DB override takes precedence — it represents an explicit user toggle in | |
| // 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. | |
| try { | |
| const dbValue = resolveFeatureFlag(PRIVATE_PROVIDER_URLS_ENV); | |
| if (isTrueValue(dbValue)) return true; | |
| } catch { | |
| // DB not initialized yet — fall through to env-only check. | |
| } | |
| export function arePrivateProviderUrlsAllowed(): boolean { | |
| // 1) DB override takes precedence — it represents an explicit user toggle in | |
| // 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. | |
| try { | |
| const dbValue = resolveFeatureFlag(PRIVATE_PROVIDER_URLS_ENV); | |
| if (dbValue !== undefined && dbValue !== "") { | |
| return isTrueValue(dbValue); | |
| } | |
| } catch { | |
| // DB not initialized yet — fall through to env-only check. | |
| } |
| cpSync(join(REPO_ROOT, rel), join(stage, rel), { recursive: true }); | ||
| } | ||
| cpSync(join(REPO_ROOT, "package.json"), join(stage, "package.json")); | ||
| symlinkSync(join(REPO_ROOT, "node_modules"), join(stage, "node_modules"), "dir"); |
There was a problem hiding this comment.
On Windows, creating a directory symlink using symlinkSync with the 'dir' type requires administrator privileges or developer mode enabled, which can cause test failures in local or CI environments. Using a 'junction' link on Windows avoids this requirement and works out of the box.
const isWindows = process.platform === "win32";
symlinkSync(
join(REPO_ROOT, "node_modules"),
join(stage, "node_modules"),
isWindows ? "junction" : "dir"
);
Closes #7682
Root cause
omniroute setup-opencodecrashed withCannot find package '@/shared' imported from src/shared/network/outboundUrlGuard.tson any global npm install. A prior fix (#6162/#6163) converted the direct@/shared/...imports in the 4src/lib/cli-helper/*.tsfiles to relative paths, but missed thatsrc/shared/network/outboundUrlGuard.ts— transitively loaded byconfig-generator/opencode.ts— still had a top-levelimport { resolveFeatureFlag } from "@/shared/utils/featureFlags". ES module static imports resolve eagerly at load time even for bindings the CLI path never calls, and the published npm package doesn't shiptsconfig.json, sotsx's alias resolution (which iscwd-based, not importing-file-based) has nowhere to resolve@/*in a real global install.A naive "relative-ize the next import" fix doesn't work cleanly: it just shifts the failure one hop deeper (
@/shared/utils/featureFlags→@/lib/db/featureFlags→@/typesinsrc/lib/db/migrationRunner.ts), cascading through much ofsrc/lib/db/.Fix
Split
src/shared/network/outboundUrlGuard.tsinto two modules:src/shared/network/outboundUrlGuard.ts— the "pure" module (isPrivateHost,isCloudMetadataHost,OutboundUrlGuardError,parseOutboundUrl,parseAndValidatePublicUrl,parseAndValidateNonMetadataUrl). Zero@/-aliased imports — onlynode:net. This is what the CLI transitively loads.src/shared/network/outboundUrlGuardPolicy.ts(new) — the DB/feature-flag-backed helpers (arePrivateProviderUrlsAllowed,areLocalProviderUrlsAllowed,getProviderOutboundGuard,getProviderValidationGuard,parseAndValidateWebhookUrl), which keep the@/shared/utils/featureFlagsimport. This file is only ever loaded by Next.js/webpack-bundled server code, never the CLI, so@/always resolves fine there.Updated the ~16 call sites that imported the moved functions to import from
outboundUrlGuardPolicy.tsinstead (mechanical import-path rename, zero logic change — same functions, same bodies, just re-homed).Regression test (Hard Rule #18 — TDD)
tests/unit/cli-setup-opencode-nested-alias-7682.test.ts— stages a tsconfig-less copy ofbin/,src/lib/,src/shared/+package.json(mirroring a real global npm install), symlinksnode_modules, and spawns a fresh Node process that importsconfig-generator/opencode.tsviatsx/esm. Confirmed RED on unfixed code with the byte-identical reporter error:GREEN after the split.
Gates run (all green)
node --import tsx/esm --test tests/unit/cli-setup-opencode-nested-alias-7682.test.ts— RED → GREENoutbound-url-guard-feature-flag,provider-models-route-lan-guard,proxy-fallback-ssrf,webhook-metadata-guard-3269,webhook-private-optin-3269,webhook-ssrf-guard,cli-helper-tool-detector-paths-6162,provider-validation-ssrf-guard, plus the broader provider-validation/webhook/remote-image-fetch suitesnpm run typecheck:core— exit 0npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>— exit 0node scripts/check/check-file-size.mjs— OKnode scripts/check/check-complexity.mjs— OK (2058 violations vs baseline 2059)node scripts/check/check-cognitive-complexity.mjs— OK (890 vs baseline 890)node scripts/check/check-changelog-integrity.mjs— OKnpm run check:cycles— OK, no cyclesScope note
Only the confirmed root cause (
setup-opencodeCLI crash) is fixed here. The plan-file's secondary reported symptom (OpenCode Free missing from Combo Builder) is a separate, unconfirmed issue per the triage analysis and is not bundled into this fix.