René's URL Explorer Experiment

Title: fix(api): resolve the OAuth callback's app origin from configured APP_URL by glifocat · Pull Request #421 · onecli/onecli · GitHub

Open Graph Title: fix(api): resolve the OAuth callback's app origin from configured APP_URL by glifocat · Pull Request #421 · onecli/onecli

X Title: fix(api): resolve the OAuth callback's app origin from configured APP_URL by glifocat · Pull Request #421 · onecli/onecli

Description: I have read the CONTRIBUTING.md file. YES What kind of change does this PR introduce? Bug fix. Fixes #420. Maintainer note. @glifocat's diagnosis here was exactly right, and the fix proposed in the issue (#420) was the correct design — read the raw env so it is undefined when unset, and let the existing || apiOrigin fallback finally run. That is what this PR now implements. The version that had been pushed to the branch simplified it further, to const appOrigin = apiOrigin, and that turned out to break deployments that serve the API and the dashboard on different hosts (details below). I've reset the branch to main, re-applied the fix against current code, and extended it to the other places with the same defect. Credit for finding this is entirely @glifocat's — thanks for the unusually clear write-up, including pinning the exact line and commit. The bug packages/api/src/routes/apps.ts picked the callback's redirect target with: const appOrigin = APP_URL || apiOrigin; APP_URL there is the constant from lib/env.ts, which defaults to http://localhost:10254. It is never falsy, so || apiOrigin was unreachable dead code. Self-hosters who had not set APP_URL completed provider consent and were then sent to localhost. Also reported independently in #192. Why not "just use the request origin" Those are two different questions, and conflating them is what caused this in the first place: Who answered this request? → getRequestOrigin() Where does a browser go for a dashboard page? → not necessarily the same host A deployment can run this API package standalone — answering on an API host and declaring it via the selfUrl option — while the dashboard is served elsewhere. CLI and SDK clients talk to the API host, so a browser really can arrive at the callback there. But /app-connect/* is a Next.js page that only exists on the dashboard host, so the last redirect has to cross origins, and it can only do that from a configured APP_URL. apps-callback-origin.test.ts locks that in: with selfUrl on one host and APP_URL on another, the redirect must land on the dashboard host. It fails with https://api.example.com/app-connect/... if the preference is dropped — the literal dead URL a user would hit. The fix lib/app-origin.ts (new) — configuredAppUrl() returns the explicitly configured URL, or undefined when unset/empty/whitespace. That undefined is the whole point: it is what lets a call site say "…and otherwise use the request origin". Plus originFromHeaders(), the header-derivation logic lifted out of request-origin.ts. getAppOrigin(request) in lib/request-origin.ts — names the invariant so it is harder to break: configured APP_URL first, request origin only as the fallback. Applied at the callback and at the agent-facing "go set up your credentials" link (dashboardUrl gained an optional fallback origin). Settings → Instance now shows the origin actually in use, with an auto-detected note. Side benefit: that page was statically prerendered, so it had been displaying its build-time APP_URL; it is dynamic now. originFromHeaders() validates the host and scheme it derives. These origins reach Location headers and, on the OAuth fragment-bridge path, the inside of a . Deployment APP_URL request origin result single host set same unchanged split API / dashboard hosts set differs redirect stays on the dashboard host self-hosted, set set = it unchanged self-hosted, unset unset derived fixed Not included The docker-compose.yml APP_URL pin. Unpinning it is right in principle — deriving a public URL from a bind address is a category error — but scripts/install.sh auto-selects the docker0 bridge IP on bare-metal Linux and publishes only there, so 127.0.0.1:10254 is not listening. The gateway builds absolute links with no request to derive from (apps/gateway/src/gateway/response.rs), and unpinning alone would point those at a dead localhost. Those two have to change together; tracked separately. Verification @onecli/api: check-types, lint (--max-warnings 0), 715 tests passing. @onecli/web: check-types, lint. Prettier clean. Each regression test was confirmed to fail against the original bug before being kept.

Open Graph Description: I have read the CONTRIBUTING.md file. YES What kind of change does this PR introduce? Bug fix. Fixes #420. Maintainer note. @glifocat's diagnosis here was exactly right, and the fix proposed ...

X Description: I have read the CONTRIBUTING.md file. YES What kind of change does this PR introduce? Bug fix. Fixes #420. Maintainer note. @glifocat's diagnosis here was exactly right, and the fix propo...

Opengraph URL: https://github.com/onecli/onecli/pull/421

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width