Skip to content

Preview TLS: opt-in wildcard mode (#276) - #277

Merged
simpros merged 2 commits into
mainfrom
gh-276-preview-tls-opt-in-wildcard-mode
Oct 4, 2026
Merged

simpros merged 2 commits into
mainfrom
gh-276-preview-tls-opt-in-wildcard-mode

Conversation

@simpros

@simpros simpros commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Closes #276

Why the change

Per-preview ACME issuance spends the CA's 50-certificates-per-domain weekly budget until no new preview gets HTTPS, so an opt-in gateway mode declares one wildcard per preview suffix and every preview host becomes a certificate-store hit with zero new orders per deploy.

Special things to note

  • Default is fully off: with SPROUT_TRAEFIK_WILDCARD_TLS unset, emitted labels are byte-identical to today and no existing deployment changes behaviour.
  • Flag on requires SPROUT_TRAEFIK_CERTRESOLVER (boot fails naming it) and that resolver must be DNS-01 — a wildcard can never validate over HTTP-01, and boot logs a warning naming the docs section because the challenge type is outside sprout's view.
  • Deliberately out of scope per the issue: no automation of the operator's DNS-01 resolver or certificate bootstrap, and no change to the default per-host issuance path.

Change outline

New gateway knob flowing into the existing TLS policy seam:

type TraefikTls = {
  entrypoints: string;
  certResolver?: string;
+ wildcard?: boolean;  // SPROUT_TRAEFIK_WILDCARD_TLS, default false
}

Label emission per router (suffix = hostname minus first label):

  traefik.http.routers.<name>.tls=true
  traefik.http.routers.<name>.entrypoints=<entrypoints>
  traefik.http.routers.<name>.tls.certresolver=<resolver>
+ traefik.http.routers.<name>.tls.domains[0].main=*.a.previews.example.com
+ traefik.http.routers.<name>.tls.domains[0].sans=a.previews.example.com

Fail-fast control flow:

boot
  wildcard on + certresolver unset -> throw naming SPROUT_TRAEFIK_CERTRESOLVER
  wildcard on + entrypoints unset  -> throw naming SPROUT_TRAEFIK_ENTRYPOINTS
deploy gate
  wildcard on + hostname with <3 labels -> 422 invalid_wildcard_hostname
materialize
  wildcard on -> wildcardDomains(hostname) throws on short host (defense in depth)

Shallow file tree with responsibilities:

apps/server/src/app-deployment/labels.ts      # wildcardSuffix/domains + router emission
apps/server/src/config.ts                     # flag parse, boot validation, summary, warning
apps/server/src/index.ts                      # boot warning wire-up
apps/server/src/http/deploy.ts                # 422 gate for short app/service hostnames
docker-compose.yml / .env.example / compose.env.example  # knob passthrough + docs
docs/operator-deploy.md                       # opt-in mode, DNS-01 prerequisite, one-label rule
deploy/traefik/README.md                      # mode noted as bootstrap alternative

@simpros

simpros commented Oct 4, 2026

Copy link
Copy Markdown
Owner Author

🧊 Thermo-nuclear review (round 1) — REQUESTCHANGES · 6782ebfb

  • Findings
    1. traefikLabelKeys no longer honors its contract (structural)
    1. Dead branch and unexpressed invariant in parseTraefikTls
    1. Third copy of the boolean-token env parser (duplication)
  • Minor (fold in, no separate action needed)
  • Residual risks
  • Unverifiable from here
Full report (click to expand)

VERDICT: REQUEST CHANGES

Findings

1. traefikLabelKeys no longer honors its contract (structural)

apps/server/src/app-deployment/labels.ts:143-166

The docstring promises "key names never depend on the rule value or the loadbalancer port, so dummy values suffice," and the function samples traefikLabels with a magic hostname. That was true before this PR. It is now false: when tls.wildcard === true, enumerating reserved keys invokes wildcardDomains(hostname) → wildcardSuffix, so key presence depends on the placeholder having ≥3 labels. Two consequences:

  • The reserved-key set is now silently coupled to a fake hostname satisfying real business validation. Change the wildcard minimum-label rule and traefikLabelKeys starts throwing for wildcard policies instead of returning keys, which surfaces as a 500 in the collision path rather than a clean gate.
  • resolveLabelCollisions (which runs after the hostname gate) is the only production caller, but gatewayLabelKeys ignores the actual routing hostname (workload-labels.ts:79-93), so the "reserved set == emission" invariant is no longer a pure function of the policy.

The two tls.domains[0].main/.sans key names are static strings — they do not depend on the hostname value at all. Remedy: derive them structurally from routerName/tls.wildcard (or have traefikLabels accept a precomputed {main, sans} / a wildcard: boolean), so key enumeration never round-trips through wildcardSuffix and the placeholder is no longer load-bearing. This deletes the hidden throw path rather than papering over it.

2. Dead branch and unexpressed invariant in parseTraefikTls

apps/server/src/config.ts:500-502

Line 495 throws when wildcard && certResolver === "", so by line 500 wildcard is necessarily false. The { entrypoints, wildcard: true } arm of the ternary is unreachable. Simplify to if (certResolver === "") return { entrypoints };.

The underlying invariant is real — wildcard implies a cert resolver — but the TraefikTls type models them independently (certResolver?, wildcard?), which is exactly why this arm looked plausible. If you want it enforced, make the type say so; otherwise at least delete the dead arm so the reader is not misled about a resolveless wildcard being representable.

3. Third copy of the boolean-token env parser (duplication)

apps/server/src/config.ts:466-481

parseTraefikWildcardTls is a near-copy of parseTelemetryFlag (270-286) — same token lists ["on","1","true","yes"]/["off","0","false","no"], same wrapper shape — and the same pattern appears again in parseMailSecure (387-401). Extract one parseBooleanEnv(key, { trueTokens, falseTokens, defaultValue }) and route all three through it. The new variable then costs one declaration and the error-message wording stays uniform across gateway knobs.

Minor (fold in, no separate action needed)

  • apps/server/src/app-deployment/labels.ts:35 — labels.length < 3 || suffix.split(".").length < 2: the second disjunct is logically implied by the first (a hostname of ≥3 labels always yields a suffix of ≥2), so it reads like a separate case that can never fire. Keep labels.length < 3.
  • apps/server/src/http/deploy.ts:545-557 — wildcardSuffix always throws an Error, so the non-Error fallback string is unreachable and duplicates the message in labels.ts:36-38. Since wildcardSuffix's throw is the single source of that error text, drop the fallback.

I do not consider file-size or modularity a problem here: config.ts 835→885, deploy.ts 762→803, labels.ts 212 — no threshold crossing, and the new gate follows the established {ok,error,detail} resolver pattern rather than tangling the deploy flow. The blockers are the lost traefikLabelKeys contract and the dead/unexpressed wildcard–resolver invariant.

Residual risks

  1. HTTP-01 resolver pointed at wildcard mode — boot only warns; routers then request a wildcard the resolver can never validate, so TLS issuance silently fails. Probe: confirm the boot warning SPROUT_TRAEFIK_CERTRESOLVER must be a DNS-01 resolver and watch Traefik ACME logs for a challenge-type failure on *.suffix.
  2. Short hostnames now rejected when the flag is on — localhost/two-label hosts that deployed before become a 422, breaking local or non-prod flows that turn the flag on. Probe: POST /v1/deploy with localhost → expect 422 invalid_wildcard_hostname.
  3. Certificate dedupe depends on an identical domain set — hosts whose suffixes differ in depth under the zone (pr-1.a.previews… vs pr-2.previews…) order separate certs, quietly spending the budget the feature exists to protect. Probe: note ACME order count while deploying previews at two different suffix depths; expect the second to be a store hit.
  4. Reserved-key expansion rejects existing adopter labels — manifests that already set tls.domains[0].main/.sans now collide only in wildcard mode. Probe: deploy with such a label → expect 422 reserved_preview_label.
  5. Coolify paste-file does not forward the flag — deploy/coolify/gateway.compose.yml hardcodes the Traefik env block and omits SPROUT_TRAEFIK_WILDCARD_TLS, so operators on that path get no option until they edit it. Probe: inspect gateway container env / grep the compose file.

Unverifiable from here

No bun/tsc in this checkout, so the test suites and typecheck were not executed; behavior was verified by reading only, and the real-Traefik + DNS-01 acceptance (zero orders per deploy, store reuse) cannot be exercised from a checkout.


Automated agent review (opencode) · worktree /home/sim/.herdr/worktrees/sprout/gh-276-preview-tls-opt-in-wildcard-mode · subsequent rounds will reply in this thread.

- traefikLabelKeys derives wildcard domain keys structurally instead of
  validating a placeholder hostname (drops the hidden throw path)
- TraefikTls models wildcard-implies-certResolver; dead arm removed
- single parseBooleanEnv for the gateway boolean knobs
- drop redundant suffix check and unreachable non-Error fallback
@simpros

simpros commented Oct 4, 2026

Copy link
Copy Markdown
Owner Author

🧊 Thermo-nuclear review (round 2) — APPROVE · e794ebe0

  • Findings status
  • New findings
  • Residual risks
  • Unverifiable from here
Full report (click to expand)

VERDICT: APPROVE

Findings status

  1. traefikLabelKeys no longer honors its contract — addressed. Current tree strips the wildcard flag before sampling traefikLabels and appends the two domain keys structurally: apps/server/src/app-deployment/labels.ts:174-188 (tls: input.tls?.wildcard === true ? { entrypoints, certResolver } : input.tls … keys.push(...tlsDomainKeys(input.routerName))). The static names live in one place (tlsDomainKeys, labels.ts:54-60) used by both emission (labels.ts:113) and enumeration, so the reserved set is again a pure function of the policy and no longer round-trips through wildcardSuffix. The docstring at labels.ts:155-161 now matches the code.
  2. Dead branch and unexpressed invariant in parseTraefikTls — addressed, and better than asked. apps/server/src/config.ts:505 is now return { entrypoints }; (unreachable wildcard: true arm deleted), and the invariant is encoded in the type: labels.ts:4-14 makes wildcard: true require certResolver: string while the off member is wildcard?: false. formatTraefikTlsSummary (config.ts:852-859) drops its dead undefined-resolver wildcard branch accordingly.
  3. Third copy of the boolean-token env parser — addressed. parseBooleanEnv at config.ts:274-295 is the single implementation; parseTelemetryFlag (297-308), parseMailSecure (406-413), parseTraefikWildcardTls (479-486) and parseDashboardEnabled (530-537) all route through it with per-key tokens/error text. Token sets and error strings are preserved per caller (mail still rejects on/off; telemetry still defaults to "on").

Minor 1: addressed — labels.ts:40 is now just if (labels.length < 3), dropping the logically implied suffix.split(".").length < 2 disjunct.

Minor 2: addressed in intent — the unreachable non-Error fallback is gone; apps/server/src/http/deploy.ts:552 is now detail: (err as Error).message (see the non-blocking note below).

New findings

No structural findings. The delta is confined to the three files the prior report touched and each fix deletes the flagged complexity (unreachable arm, duplicated parser, hostname-coupled key enumeration) rather than wrapping it. No file crosses the 1k threshold (config.ts ~880, deploy.ts ~800, labels.ts 236), and no feature logic leaked into a shared path.

Non-blocking note (no change required): deploy.ts:552 asserts (err as Error).message. Given wildcardSuffix is the only throw site and always throws Error, and the prior review asked to remove the unreachable branch, this is the minimal honest encoding; a catch payload is unknown under strict TS. If ever touched, the canonical alternative already exists in the file at config.ts:255 (err instanceof Error ? err.message : String(err)).

Residual risks

  1. Blank-default semantics changed — parseBooleanEnv returns false when both raw and defaultRaw are blank (config.ts:285-287), whereas the old parseTelemetryFlag threw on a blank default. Probe: add a caller with defaultRaw: "" and confirm boot with a blank env silently disables the knob rather than erroring.
  2. Cast can surface undefined detail — (err as Error).message yields detail: undefined if a future refactor throws a non-Error from wildcardSuffix. Probe: temporarily throw "boom" and POST a short hostname; assert detail is a string.
  3. Compile-time-only resolver invariant — a cast/JSON-built { entrypoints, wildcard: true } bypasses the union, and configSummary then renders https (certresolver=undefined, wildcard) at config.ts:859. Probe: cast that literal to TraefikTls, call configSummary, inspect the line.
  4. Reserved set now hostname-independent for wildcard — wildcard domain keys are reserved for every wildcard request, even one whose hostname the gate later rejects; ordering decides which 422 wins. Probe: POST a short hostname plus a colliding tls.domains label and read whether invalid_wildcard_hostname or reserved_preview_label comes back.
  5. Typecheck is green, tests unrun — tsc --noEmit -p apps/server/tsconfig.json exits 0 in this checkout (union narrowing and all test literals compile). Probe: run bun test apps/server/src/{config,app-deployment/labels,app-deployment/workload-labels,http/deploy-services-request}.test.ts in an environment with bun.

Unverifiable from here

bun is not installed, so bun test could not be executed; the real-Traefik + DNS-01 acceptance (zero ACME orders per deploy, cert-store reuse) cannot be exercised from a checkout.

@simpros
simpros merged commit 56f0168 into main Oct 4, 2026
2 checks passed
@simpros
simpros deleted the gh-276-preview-tls-opt-in-wildcard-mode branch October 4, 2026 21:42
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