fix(server): allow template variables in link hostname. Fixes #10696

[myzk-a]

Fixes #10696

Motivation

Configuring a link with a template variable in the hostname part of the URL, e.g.

links: |
  - name: Pod Logs Link
    scope: pod-logs
    url: http://${metadata.name}.logging-facility

makes the Argo Server crash at startup:

level=ERROR msg="parse \"http://${metadata.name}.logging-facility\": invalid character \"{\" in host name"
exit status 1

This is a regression: it worked in 3.3 and broke when link sanitization was introduced in 3.4.0-rc1 (#8779). Config.Sanitize passes the raw, un-expanded URL to url.Parse, which rejects ${...} in the host part (variables in the path/query are unaffected). That parse only exists to extract the scheme and reject dangerous protocols such as javascript: — the server never needs the host, since template variables are expanded by the UI at render time.

Modifications

• config/config.go: replace url.Parse in Sanitize with a schemeOf helper that extracts only the scheme, per RFC 3986 §3.1. Since the host/path/query are never parsed, template variables may appear anywhere in the URL. schemeOf is re-implemented from the RFC (not copied) and uses the same technique as the standard library's unexported net/url getScheme; a missing scheme is reported as "", which the protocol allowlist rejects as before. Scheme validation behavior is fully preserved, including rejecting javascript: URLs that contain variables.
• config/config_test.go: add regression test cases.

Note that URLs that url.Parse would reject for reasons other than the scheme (e.g. a malformed host) are now accepted. This is intentional: the server cannot distinguish a typo from an un-expanded template variable, and the failure mode improves from "server crash-loops at startup" to "a dead link in the UI".

Verification

• New unit test http://${metadata.name}.logging-facility?podName=${metadata.name} reproduces the exact error from the issue before the fix and passes after it; existing TestSanitize cases still pass.
• Verified end-to-end: with a hostname-variable link in workflow-controller-configmap, the server crash-looped before the fix and starts normally after it.

Documentation

No change needed: docs/links.md already states that links can contain placeholder variables, without restricting where they may appear. This fix restores the documented behavior.

AI

Claude Code was used for root-cause analysis, regression history research, design discussion, and code review. The code and tests were written by the author.

Summary by CodeRabbit

• Bug Fixes
  • Improved URL validation so template variables in addresses are no longer misread as hostnames.
  • http:// URLs containing placeholders are now accepted as expected.
  • Unsafe javascript: URLs with placeholders continue to be blocked with the existing error message.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2f6b1768-8a8c-4ef5-9ff9-46155ac5e73c

📥 Commits

Reviewing files that changed from the base of the PR and between 4284f07 and b38a496.

📒 Files selected for processing (2)

• config/config.go
• config/config_test.go

---

📝 Walkthrough

Walkthrough

The URL sanitization logic in config/config.go was changed to derive the URI scheme via a new schemeOf helper that scans the raw URL string per RFC 3986 rules, replacing url.Parse-based scheme extraction. This avoids failures when hostnames contain template variables. Tests were extended accordingly.

Changes

URL Sanitize scheme extraction

Layer / File(s)	Summary
schemeOf helper and Sanitize scheme validation config/config.go	Adds a schemeOf function that scans rawURL for a valid RFC 3986 scheme without parsing the full URL, and updates Sanitize to validate that scheme via ValidateProtocol instead of using url.Parse.
Sanitize test coverage for template URLs config/config_test.go	Adds test cases confirming an http:// URL with ${metadata.name} placeholders passes, and a javascript:${metadata.name} URL is rejected.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

Not applicable — the change is a localized scheme-parsing refactor with a single component and no multi-actor interaction flow.

Related issues: Fixes #10696 by allowing template variables in link hostnames without breaking URL sanitization.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: none specified

Poem:
A rabbit scanned each URL byte by byte,
No more parsing hosts that broke at night,
Schemes now found the RFC way,
Template names can stay and play,
Javascript still gets turned away. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, specific, and accurately reflects the hostname-template-variable fix.
Description check	✅ Passed	The description includes the required sections and clearly explains motivation, changes, testing, docs, and AI use.
Linked Issues check	✅ Passed	The changes address #10696 by allowing template variables in link hostnames while preserving scheme validation.
Out of Scope Changes check	✅ Passed	The PR stays focused on config sanitization and regression tests, with no clear unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}