feat: actionable internal-host allowlist error guidance for desktop/BYOK providers - #5339
feat: actionable internal-host allowlist error guidance for desktop/BYOK providers#5339lefarcen wants to merge 2 commits into
Conversation
…top/BYOK Implements #1244 (maps to #1059). - Central sentinel + isInternalIpBlocked helper in contracts - Daemon and provider-models now emit the stable sentinel - UI mappers (SettingsDialog renderTestMessage + provider models failure, EntryShell onboarding test/models) branch on exact sentinel (not generic forbidden) - New i18n key settings.testInternalIpBlocked with host echo + OD_ALLOWED_INTERNAL_HOSTS + restart + launch-env guidance - All 18 locales updated (English source + placeholders) - README paragraph now has stable #internally-hosted-model-endpoints anchor for docs linking - No change to enforcement, client validation, packaged env forwarder, or data paths Closes #1244 Generated-By: looper 0.0.0-dev (runner=worker, agent=opencode)
…径或快速修复按钮，而非泛泛的错误代码 Generated-By: looper 0.0.0-dev (runner=worker, agent=opencode)
lefarcen
added
size/XXL
|
🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready. |
lefarcen
left a comment
lefarcen
left a comment
There was a problem hiding this comment.
The SSRF-guidance direction is clear. Before pool review picks this up, could you reshape the description into the template fields we use here — Why, What users will see, Surface area, and Validation — so the next pass can skim it against the standard checklist quickly?
Visual regression reviewHead: 0 changed · 53 unchanged · 0 new without baseline · 0 failed Unchanged cases
Visual diff is advisory only and does not block merging. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@lefarcen thanks for tightening the SSRF guidance path here. I reviewed the daemon sentinel flow, contracts helper, settings/onboarding renderers, README anchor, and locale additions. I found two non-blocking fixes that would make the feature line up better with the stated behavior and keep the repo dependency surface clean.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| 'settings.testSuccessCli': '{agentName} replied in {ms} ms — \'{sample}\'', | ||
| 'settings.testAuthFailed': 'Authentication failed. Check your API key.', | ||
| 'settings.testForbidden': 'Access forbidden. Verify your account, region, or organization.', | ||
| 'settings.testInternalIpBlocked': 'Internal host blocked for security (SSRF guard). Host not in allowlist. Set OD_ALLOWED_INTERNAL_HOSTS=<host> in the launch environment, then restart the daemon/desktop app. Copy the snippet or see the docs.', |
There was a problem hiding this comment.
The call sites now compute and pass { host } into settings.testInternalIpBlocked, but this string renders a literal <host> instead of interpolating the actual rejected hostname. The translator only replaces {name} placeholders, so a user who entered http://10.0.0.5:4000 will see OD_ALLOWED_INTERNAL_HOSTS=<host> rather than the concrete host this PR says it should echo. That weakens the main remediation path because the allowlist is exact-host. Please change the source string to include {host} (for example Host {host} is not in the allowlist. Set OD_ALLOWED_INTERNAL_HOSTS={host} ...) and mirror that placeholder in the other locale entries.
| @@ -0,0 +1,3644 @@ | |||
| { | |||
There was a problem hiding this comment.
This PR introduces a root bun.lock, but the repository declares pnpm@10.33.2 in package.json, already tracks pnpm-lock.yaml, and the root workflow documentation calls out Corepack/pnpm as the package-manager baseline. Keeping a second generated lockfile that is unrelated to the provider allowlist change makes dependency state look ambiguous and can mislead future installs or reviews into thinking Bun is supported here. Please remove bun.lock from this PR unless the package-manager support change is intentional and documented as a separate repo-wide change.
2 participants
Implements #1244 (maps to internal report #1059).
When a desktop/BYOK user configures an internally-hosted provider base URL that the daemon's default-deny SSRF guard rejects, the returned sentinel "Internal IPs blocked" is now surfaced with concrete, executable guidance:
OD_ALLOWED_INTERNAL_HOSTSand its semantics (opt-in at launch time, exact host).settings.testInternalIpBlocked.Implementation:
isInternalIpBlockedpredicate added to contracts (narrow, stable classification on exact sentinel, notkind === 'forbidden').#internally-hosted-model-endpointsanchor.No new persisted config, no packaged env injection, no relaxation of the guard, no daemon data path changes.
Closes #1244