feat(config): derive VOLCANO_WEB_URL from VOLCANO_API_URL by convention

[tkkhq]

Tracking

• Related: fix(auth): route browser login through Volcano Web's login page #72
• Related: https://konghq.atlassian.net/browse/VOL-472

Summary

• cfg.WebURL() only ever read VOLCANO_WEB_URL or the compiled default, so VOLCANO_API_URL and VOLCANO_WEB_URL had to be configured independently — the only Makefile support for deriving one from the other was a build-time-only loopback special case in make local
• add config.deriveWebURL: when VOLCANO_WEB_URL isn't explicitly set, derive it from the resolved VOLCANO_API_URL by convention — a loopback API host maps to the conventional local Web port 3000, an api. API host maps to the same host with that prefix stripped (api.volcano.dev -> volcano.dev, api.staging.volcano.dev -> staging.volcano.dev); falls back to the compiled default when neither convention matches
• extract config.IsLoopbackAPIURL (used by both the new derivation and internal/auth's existing local-mode device-client selection, replacing the duplicate copy that lived in internal/auth/auth.go)
• remove the now-redundant build-time loopback inference from the make local target since the same behavior now happens at runtime for any VOLCANO_API_URL, not just ones baked in via .env.local
• update CONTRIBUTING.md to describe VOLCANO_WEB_URL as optional at runtime, not just at build time

Verification

• go build ./...
• go vet ./...
• go test ./...
• go tool golangci-lint run ./...

[Copilot]

Pull request overview

Adds runtime derivation of Volcano Web URLs from configured API URLs.

Changes:

• Derives web origins for loopback and api. hosts.
• Centralizes loopback URL detection for config and authentication.
• Updates local-build behavior, tests, and contributor documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
Makefile	Removes build-time loopback inference.
internal/config/config.go	Adds web URL derivation and shared loopback detection.
internal/config/config_test.go	Tests derivation, precedence, and loopback detection.
internal/auth/auth.go	Uses the shared loopback helper.
CONTRIBUTING.md	Documents runtime URL derivation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tkkhq]

Fixed in ed475ee — WebURL() now checks compiledDefaultWebURL != defaultCompiledWebURL (i.e. an explicit compile-time override) before attempting derivation, so a custom compiled web URL always wins over the loopback/api. convention. Added TestWebURLExplicitCompiledDefaultWinsOverLoopbackDerivation covering exactly this scenario.

[tkkhq]

Warning

This review was generated for commit ecbff91915e9ac8d26c59616fa5cfc2b5edb3fdd. At publish preflight, the PR pointed to 3a75357ebcd2228b1f301748db7a84d2c0448893.
Inline findings were folded into this body because their original diff anchors may be stale.

Code Review — PR #74: feat(config): derive VOLCANO_WEB_URL from VOLCANO_API_URL by convention

Verification: No user-level pr_review_verify baselines are configured (action:list returned enabled=false, empty baselines/rejected), so the sandboxed baseline run was skipped by design; this did not delay or reduce the 5-pass batch. The orchestrator independently ran go build ./... (clean) and go test ./internal/config/... ./internal/auth/... (both packages pass, including the 5 new config tests) on the PR head to source-ground the correctness/contracts findings.

Overview

This PR lets the CLI derive VOLCANO_WEB_URL from VOLCANO_API_URL at runtime instead of requiring both to be configured independently. config.deriveWebURL maps a loopback API host to http://localhost:3000, and an api.-prefixed API host to the same host/port with that prefix stripped (api.volcano.dev -> volcano.dev), falling back to the compiled default web URL when neither convention matches. The loopback check is extracted into a newly exported config.IsLoopbackAPIURL, deduplicating the identical logic previously duplicated in internal/auth's isLocalAPIURL, which now just delegates to it. The now-redundant build-time-only loopback special case is removed from the local Makefile target, and CONTRIBUTING.md is updated to describe VOLCANO_WEB_URL as optional at runtime.

The change is small, well-scoped, and covered by 5 new table-driven tests in config_test.go exercising derivation, loopback detection, env-override precedence, and the no-convention fallback. Independent edge-case verification (IPv6 literals, missing scheme, ports, case sensitivity, api.-as-substring, userinfo-injection tricks) confirms deriveWebURL degrades safely to "" (triggering the compiled-default fallback) on any malformed or ambiguous input, and that the extracted IsLoopbackAPIURL is behavior-equivalent to the code it replaced.

Strengths

• Correct, DRY extraction: internal/auth's duplicate loopback-detection logic is replaced by a delegating call to the new exported config.IsLoopbackAPIURL, verified byte-for-byte behavior-equivalent at its only call site (resolveDeviceClientID).
• Solid new test coverage (config_test.go) for derivation by API-host convention, loopback detection (including IPv6), explicit-env-override precedence, and fallback to a custom compiled default.
• deriveWebURL fails safe: any URL it can't confidently map (missing scheme, non-matching host, userinfo tricks, non-loopback IPv6) returns "", so WebURL() cleanly falls through to the compiled default rather than producing a malformed origin.
• Matching documentation update in CONTRIBUTING.md describing the new runtime-optional behavior of VOLCANO_WEB_URL, and a clean Makefile removal that leaves no dangling references to the deleted build-time special case.

Findings — 1 total (0 inline, 1 summary-only)

Severity	Summary-only finding	Location
P2	[P2] Runtime derivation can shadow a custom compiled default web URL	internal/config/config.go:218-221 RIGHT

[P2] Runtime derivation can shadow a custom compiled default web URL

internal/config/config.go:218-221 RIGHT

Config.WebURL() now checks deriveWebURL(c.APIURL()) before falling back to compiledDefaultWebURL (internal/config/config.go:214-221). This changes behavior for the exact make local workflow this PR touches: if a developer's .env.local sets VOLCANO_API_URL=http://localhost:8000 and a non-conventional VOLCANO_WEB_URL (e.g. http://localhost:4000, matching a frontend dev server that isn't on port 3000), make local bakes both as compiled defaults via LDFLAGS. Previously, running the resulting binary with no env vars exported returned the compiled http://localhost:4000. Now, because the compiled API URL resolves as loopback, deriveWebURL unconditionally returns http://localhost:3000, silently overriding the developer's own compiled web URL with no warning. This is reachable via the only functional WebURL() consumer, the resolveWebTarget fallback in internal/auth/auth.go (used when the device-authorization response carries no verification URI). It only bites when the local web port differs from the documented 3000 convention, so it's not a general regression, but it is a real, PR-introduced interaction the new tests don't cover (the existing TestWebURLFallsBackToCompiledDefaultWhenNoConventionMatches test only covers a non-loopback, non-api. API host, not a loopback API host paired with a custom compiled web URL).

Correctness / Security / Performance

• Correctness: One non-blocking P2: runtime loopback derivation can shadow a custom compiled default web URL for the make local workflow; core precedence, delegation, and fallback logic are otherwise correct and test-verified.
• Security: No security issues found; derived origins are bounded to the API URL's own host family, fail safe to the compiled default on any malformed/ambiguous input, and are always dominated by higher-precedence sources (explicit VOLCANO_WEB_URL, device-flow verification URI) in any real attack scenario.
• Performance: No performance concerns; deriveWebURL runs at most once per interactive Signup/LoginWithBrowser call with negligible URL-parsing cost, and the Makefile cleanup leaves no dead/duplicate shell logic.

Verdict

Suggested verdict: approve

Build and tests pass, the core derivation/delegation logic is correct and well-tested, and the only surviving finding is a non-blocking P2 edge case affecting a narrow local-dev configuration rather than the general behavior.

Confidence: 0.85