Skip to content

Harden HTTP endpoints against cross-site requests and XSS - #170

Merged
wimaha merged 4 commits into
mainfrom
security/hardening
Oct 18, 2026
Merged

wimaha merged 4 commits into
mainfrom
security/hardening

Conversation

@wimaha

@wimaha wimaha commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

Why

The proxy has no authentication and listens on all interfaces, so it trusts the LAN. The practical risk is the browser: any web page opened by someone in the home network can send requests to http://<proxy>:8080.

  • <img src="http://proxy:8080/remove_keys?role=owner"> deleted the key. The car then has to be re-paired with the NFC card.
  • With an Owner key, a plain form POST to /api/1/vehicles/<VIN>/command/door_unlock unlocks the car. A form POST needs no CORS preflight, and the VIN is visible through the windshield.
  • Templates used text/template, so messages carrying user input (for example the VIN from send_key) were rendered unescaped. This also prepares for the richer dashboard requested in Feature Request: Enhanced Web Dashboard with Live Vehicle Status #168.

What

  • Wrap the router in Go 1.25's http.CrossOriginProtection. Browser POSTs from other sites (Sec-Fetch-Site: cross-site, or an Origin that doesn't match Host) are rejected with 403. evcc, curl and Home Assistant send neither header and pass unchanged.
  • /gen_keys and /remove_keys accept POST only. The dashboard forms are updated.
  • Dashboard and log viewer render with html/template.
  • commonDefer logs a failed response write instead of calling logging.Fatal, so a client that disconnects can no longer stop the process.
  • MessageStack is safe for concurrent handlers.

Breaking change

GET /gen_keys and GET /remove_keys now return 405. They were never documented and are only used by the dashboard forms, which now send POST. POST /remove_keys?role=… still works for scripts.

Release notes

⚠️ Breaking: /gen_keys and /remove_keys accept POST only. GET now returns 405. Old bookmarks or scripts that call them with GET stop working.

Security: State-changing requests a browser sends from another website are now rejected (403), so a visited page can no longer change your keys or send commands to the car. evcc, curl and Home Assistant are not affected.

If you get 403 in the browser: Set the new trustedOrigins env var (e.g. trustedOrigins=http://nas.local:8080). You need it if you open the dashboard through a reverse proxy over plain HTTP that rewrites Host, or if a custom web page or browser tool calls the API from another origin. Over HTTPS through a reverse proxy nothing needs to be set.

Reverse proxies

Browsers send Sec-Fetch-Site only to secure origins (HTTPS, localhost). Over HTTPS through a reverse proxy the browser reports same-origin, and Host is not checked. Over plain HTTP, CrossOriginProtection falls back to comparing Origin with Host. A proxy that rewrites Host (nginx default) then gets 403 on dashboard buttons. trustedOrigins=http://nas.local:8080 exempts that origin (AddTrustedOrigin). Entries are normalized first (lowercase, no trailing slash, no default port), because browsers send the Origin in that form and it is compared verbatim. Invalid entries are logged and skipped. Documented in docs/environment_variables.md and the README troubleshooting section.

Not covered (follow-ups)

  • Attackers inside the LAN and DNS rebinding. Both look same-origin to the browser, so only optional authentication would help.
  • Races around BleControlInstance and BLE connections that bypass the command queue (separate PR).

Testing

  • go vet ./..., go test -race ./...
  • New tests: trusted origin behind a Host-rewriting proxy (403 without, 303 with), cross-site rejection (Origin and Sec-Fetch-Site), header-less clients still reach the car (simulated door_unlock), GET on key routes → 405, escaped dashboard messages.
  • Manual: a local binary checked with curl (evcc-style 503, cross-site 403, GET remove_keys 405), plus key generation clicked in a real browser.

🤖 Generated with Claude Code

wimaha and others added 2 commits October 18, 2026 22:15
Any web page opened in the home network could send requests to the proxy:
a plain <img src=".../remove_keys?role=owner"> deleted the key, and with an
Owner key a simple form POST could unlock the car.

- Wrap the router in http.CrossOriginProtection. Cross-site browser POSTs
  are rejected; evcc, curl and Home Assistant send neither Origin nor
  Sec-Fetch-Site and pass unchanged.
- /gen_keys and /remove_keys accept POST only (dashboard forms updated).
- Render dashboard and log viewer with html/template so messages carrying
  user input (e.g. the VIN) are escaped.
- Log instead of exiting when writing a response fails.
- Make MessageStack safe for concurrent handlers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ted car

Cross-site POSTs to door_unlock (Origin or Sec-Fetch-Site) are rejected
before reaching the car, while clients without those headers still unlock
it. GET /gen_keys and /remove_keys are refused, and form input from
send_key renders escaped on the dashboard. tesla_test.go follows the new
SetupRoutes(fs.FS, fs.FS) http.Handler signature.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

🟡 Merge after the to-dos

The change is sound and evcc/Home Assistant/curl calls are not affected. Two small things remain: a possible 403 behind a reverse proxy, and a breaking change that should be in the release notes.

Impact on existing users: Anyone who calls GET /gen_keys or GET /remove_keys by script or bookmark now gets 405. Browser users behind a reverse proxy (nginx, HA ingress) may get 403 on dashboard buttons.

To-do for the author

  • Put the GET→POST change for /gen_keys and /remove_keys in the release notes and docs.
  • Check the dashboard behind a reverse proxy that changes the Host header. If it fails, document it or add trusted origins (AddTrustedOrigin).

Test before merging: Update a real install, open the dashboard in a browser, generate and remove a key, and send a command from evcc or curl.

Technical details
  • internal/api/routes/routes.go:31: CrossOriginProtection compares Origin with Host. A proxy that rewrites Host makes browser POSTs from the dashboard return 403. Non-browser clients (no Origin/Sec-Fetch-Site) are unaffected. I did not test this with a real proxy.
  • routes.go:27-28: Methods("POST") makes old GET calls return 405. The PR says they were undocumented. I found no other uses in the repo.
  • html.go:69,117: r.FormValue also reads the query string, so POST /remove_keys?role=owner still works for scripts. That is fine.
  • tesla.go:45: Logging instead of Fatal on a failed write is a good fix. No change in API shape.
  • html/template switch: templates use plain values in HTML and in the onclick JS string. I saw no case that would break. Not run in a browser by me.
  • go.mod is 1.25.5 and the Dockerfile uses golang:1.25.5, so http.CrossOriginProtection is available. No platform or dependency changes.

Browsers send Sec-Fetch-Site only to secure origins. Over plain HTTP the
cross-origin check falls back to comparing Origin with Host, so a reverse
proxy that rewrites Host (nginx default) made dashboard buttons return 403.
The new trustedOrigins env var (comma-separated scheme://host[:port])
exempts such origins; invalid entries are logged and skipped. Over HTTPS
nothing needs to be set.

README troubleshooting now explains the 403 and that /gen_keys and
/remove_keys accept POST only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wimaha wimaha added the claude-review Add to request a new Claude review (removed automatically) label Oct 18, 2026
@github-actions

Copy link
Copy Markdown

🟡 Merge after the to-dos

Good hardening. evcc, curl and Home Assistant send no Origin/Sec-Fetch-Site, so the command API keeps working. The only break is GET on the two key routes, and that needs a clear release note.

Impact on existing users: GET /gen_keys and GET /remove_keys now return 405. Old bookmarks or scripts stop working. Browser-based clients on another origin get 403 on POST.

To-do for the author

  • Put the "Breaking: POST only" line at the top of the release notes. Users update with :latest and may not read them.
  • Add one line to the docs that custom web pages or browser tools calling the API from another origin now get 403, and that trustedOrigins fixes it.

Test before merging: On a Pi, open the dashboard directly and through your reverse proxy (HTTP), then click Generate and Remove. Also check that evcc still controls the car.

Technical details
  • internal/api/routes/routes.go: Methods("POST") on key routes gives 405 for GET. This is intended. Handlers use r.FormValue, which also reads the query string, so POST /remove_keys?role=… still works.
  • http.CrossOriginProtection needs Go 1.25. go.mod (1.25.5) and the Dockerfile (golang:1.25.5) already match. No change to platforms or dependencies.
  • Behaviour change for browser clients: a request with an Origin that does not match Host on plain HTTP gets 403, also on command routes. Header-less clients are fine.
  • commonDefer now logs instead of calling Fatal. Good fix.
  • Not verified: I did not run the tests or a real BLE setup. I could not check how trustedOrigins handles a trailing slash or default ports.

@github-actions github-actions Bot removed the claude-review Add to request a new Claude review (removed automatically) label Oct 18, 2026
…rigins

The Origin header is compared verbatim, so "http://nas.local:80" never
matched and "http://nas.local/" was rejected as having a path. Entries are
now lowercased and stripped of a trailing slash and default ports before
they are trusted.

The docs now also say that custom web pages or browser tools calling the
API from another origin get 403 unless listed in trustedOrigins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wimaha
wimaha merged commit 4aadf45 into main Oct 18, 2026
6 checks passed
@wimaha
wimaha deleted the security/hardening branch October 18, 2026 20:40
@github-actions github-actions Bot mentioned this pull request Oct 19, 2026
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.

1 participant