Skip to content

Add Caddy custom-domains deploy destination - #95

Open
jimvandervoort wants to merge 1 commit into
mainfrom
caddy
Open

Add Caddy custom-domains deploy destination#95
jimvandervoort wants to merge 1 commit into
mainfrom
caddy

Conversation

@jimvandervoort

@jimvandervoort jimvandervoort commented Sep 10, 2026

Copy link
Copy Markdown

Summary

Closes #96

Adds a caddy deploy destination for the new Caddy-based custom-domains setup (see simpleanalytics/infrastructure#231, ansible/vars/apps/custom-domains.yml).

  • Uploads the same set of files as the old external nginx server (custom destination): latest/events/latest.dev, SRI v<n>/ files, light, proxy and auto-events, plus source maps.
  • Rewrites the nginx SSI directives to Caddy template placeholders on upload, because Caddy's templates directive substitutes the request hostname ({{.Req.Host}}) and the proxy-product hostname/path query parameters at serve time.
  • The caddy-ssi transform fails the deploy if a build contains stray {{ delimiters or an SSI directive without a known replacement, so a bad build cannot turn into per-request 500s.
  • Uploads go via a temporary file so the transformed bytes are what lands on esapp05, and post-upload verification compares against the transformed content.
  • pnpm deploy --caddy-only deploys exclusively to the new setup, leaving the CDN and the old external server untouched during the migration.

Follow-up

  • The "update https://docs.simpleanalytics.com/sri" reminder now only shows when the old custom destination is selected, since Caddy needs no nginx config change for a new SRI version. The docs update still applies to Caddy-only SRI deploys; worth splitting the reminder once the old server is gone.

Test plan

  • pnpm typecheck
  • pnpm test:deploy (17 pass, 5 new)
  • Dry run pnpm deploy --caddy-only and compare the preview against esapp05
  • Real --caddy-only deploy, then run verify-custom-domains-parity.sh / verify-proxy-parity.sh from elastic-infra

🤖 Generated with Claude Code

https://claude.ai/code/session_01ApznmYeffWY6YojKXhEJDW

Security implications

Has security impact - described as: This PR is labeled change: needs review, but Claude did not provide specific security implications. Reviewers should confirm the actual impact on security, privacy, customer/user data, system stability, or critical functionality.

Testing

Not run by Claude.

Checklist

  • Linked to an issue
  • Tested
  • Asked for a review

Add a "caddy" destination alongside "custom" for the new Caddy-based
custom-domains setup (elastic-infra ansible/vars/apps/custom-domains.yml).
It uploads the same files as the old external nginx server, but rewrites
the nginx SSI directives to Caddy template placeholders on upload, since
Caddy's templates directive injects the request hostname and the
proxy-product query parameters at serve time.

The caddy-ssi transform fails the deploy if a build contains stray Go
template delimiters or an SSI directive without a known replacement, so
a bad build cannot turn into per-request 500s. Uploads go via a temporary
file so the transformed bytes are what lands on the server, and the
post-upload verification compares against the transformed content.

A --caddy-only flag deploys exclusively to the new setup, leaving the CDN
and the old external server untouched during the migration.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ApznmYeffWY6YojKXhEJDW
Comment thread deploy.mts
Comment on lines +89 to +90
['<!--# echo var="proxy_hostname" default="" -->', '{{.Req.URL.Query.Get "hostname"}}'],
['<!--# echo var="proxy_path" default="/simple" -->', '{{.Req.URL.Query.Get "path"}}'],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caddy's templates uses text/template (no escaping) and Query.Get percent-decodes, whereas nginx's $arg_* does not — so these placeholders sit unescaped inside the JS string literal }(window,{},"…") in proxy.js, and ?hostname=%22%2balert(document.domain)%2b%22 renders as }(window,{},""+alert(document.domain)+""), executing attacker script on the customer's proxy domain (verified against Go's text/template). Wrapping both in Go's built-in js escaper keeps the decoding while making the value safe; the caddy-ssi unit test expectations need the same update.

Suggested change
['<!--# echo var="proxy_hostname" default="" -->', '{{.Req.URL.Query.Get "hostname"}}'],
['<!--# echo var="proxy_path" default="/simple" -->', '{{.Req.URL.Query.Get "path"}}'],
['', '{{js (.Req.URL.Query.Get "hostname")}}'],
['', '{{js (.Req.URL.Query.Get "path")}}'],

@github-actions github-actions Bot added the change: needs review Changes affecting security, data protection, or system stability. label Sep 10, 2026
@github-actions

Copy link
Copy Markdown

SDLC label: change: needs review because it changes how the customer-facing scripts are transformed and served on the production custom-domains infrastructure.

@github-actions

Copy link
Copy Markdown
Claude review checkpoint

Reviewed commit b7b511be9de0d3221671009c53f7461026a6e2df. This is used to keep later automated reviews focused on changes Claude has not checked yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change: needs review Changes affecting security, data protection, or system stability.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proxy-product hostname and path values are injected unescaped into the served tracking script

1 participant