Skip to content

feat: CORS allowlist for cross-origin HTTP clients#82

Merged
SolAstrius merged 3 commits into
TeaGuild:mainfrom
jonpepler:telemachus/cors-allowlist
May 11, 2026
Merged

feat: CORS allowlist for cross-origin HTTP clients#82
SolAstrius merged 3 commits into
TeaGuild:mainfrom
jonpepler:telemachus/cors-allowlist

Conversation

@jonpepler
Copy link
Copy Markdown
Contributor

@jonpepler jonpepler commented May 11, 2026

⚠️ I've used Claude to make this change and draft most of the description below. Built, tested locally in KSP, and happy with the result, so sharing here.

Summary

Adds a configurable CORS allowlist so JavaScript on origins other than the API host can read Telemachus responses. Today Telemachus sends no CORS headers at all, so anything hosted off-origin (a separate machine on the LAN, a deployed static dashboard, a dev server on a different port) is blocked by the browser even when the user explicitly wants it to work.

Behaviour

  • New ALLOWED_ORIGINS setting in PluginConfiguration (Telemachus's PluginData/Telemachus/config.xml).
  • Empty list (the default) preserves the historical behaviour of never sending CORS headers — existing setups, including the bundled same-origin web UI, are unaffected.
  • When configured:
    • Echoes the request's Origin in Access-Control-Allow-Origin if it matches the allowlist, with Vary: Origin. Echoing rather than emitting * keeps the same permissiveness for configured clients but blocks credentialed requests from arbitrary origins.
    • Responds 204 to OPTIONS preflight with standard Access-Control-Allow-Methods / Access-Control-Allow-Headers / Access-Control-Max-Age.
    • Non-allowlisted requests get no CORS header, so browsers block-by-default.

Config example

<string name="ALLOWED_ORIGINS">http://&lt;client-host&gt;:&lt;port&gt;,https://&lt;deployed-dashboard&gt;</string>

Comma-separated scheme://host[:port] entries — match exactly what the browser sends in the Origin header. Trailing slashes and empty entries are stripped.

Validation

Tested locally on KSP 1.12.x with ALLOWED_ORIGINS set in config.xml:

  • Allowed CORS origins: <list> appears in KSP.log on init when the config is set.
  • curl -H "Origin: <allowlisted>" -i http://127.0.0.1:8085/telemachus/datalink?a=v.altitude returns Access-Control-Allow-Origin: <allowlisted> + Vary: Origin.
  • Same request from a non-allowlisted Origin returns no CORS header, and a browser fetch from that origin is blocked as expected.
  • OPTIONS preflight with an allowlisted Origin returns 204 with the expected Access-Control-* headers.
  • With ALLOWED_ORIGINS unset (the default), responses include no CORS headers at all — the bundled same-origin web UI continues to load normally.

jonpepler added 2 commits May 11, 2026 13:12
Configurable via a new ALLOWED_ORIGINS setting in
PluginConfiguration. Empty list (the default) preserves the
historical behaviour of never sending CORS headers, so existing
setups are unaffected.

When configured, the server:
  - echoes the request's Origin in Access-Control-Allow-Origin
    if it matches the allowlist (with Vary: Origin), rather than
    emitting a wildcard
  - responds 204 to OPTIONS preflight with standard
    methods/headers
  - leaves non-allowlisted requests untouched, so browsers
    block-by-default

Lets dashboards and SPAs hosted on a different origin (e.g.
GitHub Pages, a local Vite dev server) read action responses
without needing a same-origin proxy.
@jonpepler jonpepler marked this pull request as ready for review May 11, 2026 16:53
- Match origins case-insensitively (RFC 6454 §6.1) via HashSet<string>
  with StringComparer.OrdinalIgnoreCase. Browsers normalise to lowercase
  before sending, but admin entries shouldn't have to.
- Always send Vary: Origin when CORS is configured, including on
  non-matching responses. Otherwise a shared cache can serve a no-CORS
  response to a subsequent cross-origin request that would have matched.
- Reject Origin values containing CR or LF before echoing into the
  Access-Control-Allow-Origin header. WebSocketSharp's header setter
  already validates, but defence in depth is cheap and avoids relying
  on library behaviour.
- OPTIONS requests no longer fall through to the data pipeline when
  CORS is configured (a non-matching preflight previously serialised
  an empty JSON body); unconfigured-CORS behaviour is unchanged.
@SolAstrius
Copy link
Copy Markdown
Member

LGTM on the feature shape. Pushed a follow-up commit (4e839a7) with three small hardening tweaks while it was in front of me — flagging here for the record so the diff doesn't surprise anyone:

  1. Case-insensitive origin matching. Switched AllowedOrigins from List<string> to HashSet<string> with StringComparer.OrdinalIgnoreCase. RFC 6454 §6.1 says scheme + host are case-insensitive; browsers do normalise to lowercase before sending so this is mostly an admin-ergonomics fix, but silent non-match is a nasty failure mode if someone capitalises an entry in config.xml.

  2. Vary: Origin on every response when CORS is configured, not just matched ones. A shared cache between the dashboard and Telemachus could otherwise serve a no-CORS response to a subsequent cross-origin request that would have matched. Unlikely on a LAN, but the fix is one line.

  3. Reject CR/LF in Origin before echoing. WebSocketSharp's WebHeaderCollection.Set already validates header values, so a crafted Origin: foo\r\nX-Injected: bar would throw rather than inject — but relying on a library's internal validation for security is brittle. Defence in depth, basically free.

Also tightened the OPTIONS handling so a non-matching preflight 204s without serialising an empty JSON body (only when CORS is configured — unconfigured-CORS OPTIONS behaviour is unchanged from before this PR).

Merging.

@SolAstrius SolAstrius merged commit 1b71db0 into TeaGuild:main May 11, 2026
2 checks passed
@jonpepler jonpepler deleted the telemachus/cors-allowlist branch May 12, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants