ReviewOS

also looking at this

stacks/ts-cloud

fix(fleet): carry a site's TLS material when it moves

#179
Merged chrisbbreuer wants to merge feat/site-move-tls into main
4 files +328 -2

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

docs/cli.mdmodified+20-0
Changes to docs/cli.md
@@ -233,6 +233,26 @@ Like the tree snapshot, the dump is re-taken on a resume rather than skipped: on
233233from an earlier attempt predates whatever the source has committed since, and
234234shipping stale rows is worse than dumping twice.
235235
236#### TLS
237
238Certificates live in the gateway's cert directory (`/etc/rpx/certs` by default),
239which belongs to the box rather than to the site the same shape as an on-box
240database, and the same failure if left behind. The move carries the certificate
241and private key for the site's domain and every alias, **before** it routes the
242site and well before DNS: a hostname that resolves to a box holding no
243certificate for it is refused by every browser, which is a worse outcome than the
244site simply still being on the old box. Private keys are restored `0600`.
245
246Whether the target already has them is decided by checksum, not by whether a file
247exists an older certificate for the same hostname, expired or issued while the
248domain pointed somewhere else, is not the one being moved. A hostname the source
249has no certificate for is skipped rather than blocking: a site behind on-demand
250TLS may legitimately have none yet, and the target re-issues on first request.
251
252Renewal is not carried. The per-project renewal timer is written by the normal
253provisioning path, so the next `cloud deploy` against the target establishes it;
254until then the carried certificates stand on their own remaining validity.
255
236256The archive travels through the machine running the command rather than directly
237257between the boxes: a direct hop would need the target to hold a credential for
238258the source, which is the same credential-radius problem consolidation already
packages/ts-cloud/bin/commands/site.tsmodified+45-2
Changes to packages/ts-cloud/bin/commands/site.ts
@@ -14,10 +14,19 @@ import { siteInstallBase } from '../../src/deploy/site-target'
1414import { buildBackupScript } from '../../src/deploy/dashboard-database'
1515import { buildDatabaseSetupScript, isLocalDatabase } from '../../src/drivers/shared/db-provision'
1616import { buildBackupRestoreScript } from '../../src/drivers/shared/backups'
17import { buildRpxConfig, buildRpxFragmentRefreshScript } from '../../src/drivers/shared/rpx-gateway'
17import { buildRpxConfig, buildRpxFragmentRefreshScript, DEFAULT_RPX_CERTS_DIR } from '../../src/drivers/shared/rpx-gateway'
1818import { FleetStore, SystemFleetSshTransport } from '../../src/fleet'
1919import { applyPlan, formatPlan, resolvePlan } from '../../src/operations/plan'
20import { planSiteMove, siteMoveArchivePath } from '../../src/operations/site-move'
20import {
21 buildCertificatePackScript,
22 buildCertificateStateScript,
23 buildCertificateUnpackScript,
24 certificatesMatch,
25 parseCertificateState,
26 planSiteMove,
27 siteMoveArchivePath,
28 siteMoveCertArchivePath,
29} from '../../src/operations/site-move'
2130import { loadValidatedConfig, resolveDnsProviderConfig } from './shared'
2231
2332interface SiteAddOptions {
@@ -126,6 +135,10 @@ async function runSiteMove(siteName: string, options: SiteMoveCommandOptions): P
126135 const onBoxDatabase =
127136 appDatabase?.name && isLocalDatabase(appDatabase) ? { name: appDatabase.name } : undefined
128137 const dumpPath = `/tmp/ts-cloud-move-${slug}-${siteName}.sql.gz`
138 const certArchive = siteMoveCertArchivePath(slug, siteName)
139 const certsDir = proxy?.certsDir ?? DEFAULT_RPX_CERTS_DIR
140 // Every hostname this site is served on needs its own certificate.
141 const certDomains = [domain, ...(site.aliases ?? [])].filter((value): value is string => !!value)
129142 const engine = (appDatabase?.engine ?? 'mysql') as 'mysql' | 'mariadb' | 'postgres'
130143
131144 const effects: SiteMoveEffects = {
@@ -233,6 +246,36 @@ async function runSiteMove(siteName: string, options: SiteMoveCommandOptions): P
233246 },
234247 }
235248 : {}),
249 ...(proxy
250 ? {
251 certificates: {
252 inPlace: async () => {
253 const state = buildCertificateStateScript(certsDir, certDomains)
254 const [onSource, onTarget] = await Promise.all([
255 transport.exec(source, state),
256 transport.exec(target, state),
257 ])
258 return certificatesMatch(
259 parseCertificateState(onSource.stdout),
260 parseCertificateState(onTarget.stdout),
261 )
262 },
263 carry: async () => {
264 await execOn(transport, source, buildCertificatePackScript(certsDir, certDomains, certArchive))
265 const local = `${process.cwd()}/.ts-cloud-move-${slug}-${siteName}-certs.tar.gz`
266 // A site behind on-demand TLS may have no certificate yet; the
267 // pack script says so and exits clean rather than failing.
268 const staged = await transport.exec(source, `test -s ${certArchive} && echo staged || true`)
269 if (!staged.stdout.includes('staged')) return
270 await copyFile(source, `${source.sshUser}@${source.endpoint}:${certArchive}`, local)
271 await copyFile(target, local, `${target.sshUser}@${target.endpoint}:${certArchive}`)
272 await Bun.file(local).delete().catch(() => {})
273 await execOn(transport, target, buildCertificateUnpackScript(certsDir, certArchive))
274 await transport.exec(source, `rm -f ${certArchive}`)
275 },
276 },
277 }
278 : {}),
236279 cutoverDns: async () => {
237280 if (!domain) return []
238281 if (!dnsName) return [`No DNS provider configured — point ${domain} at ${target.endpoint} manually.`]
packages/ts-cloud/src/operations/site-move.test.tsmodified+129-0
Changes to packages/ts-cloud/src/operations/site-move.test.ts
@@ -2,6 +2,11 @@ import type { SiteMoveEffects } from './site-move'
22import { describe, expect, it } from 'bun:test'
33import { applyPlan, formatPlan, resolvePlan } from './plan'
44import {
5 buildCertificatePackScript,
6 buildCertificateStateScript,
7 buildCertificateUnpackScript,
8 certificatesMatch,
9 parseCertificateState,
510 buildDrainSourceScript,
611 buildHealthGateScript,
712 buildPauseWorkersScript,
@@ -383,3 +388,127 @@ describe('planSiteMove with an on-box database', () => {
383388 expect(state.sourceRunning).toBe(true)
384389 })
385390})
391
392/**
393 * Certificates live in the gateway's cert directory, which belongs to the box
394 * rather than the site — the same shape as an on-box database. With pinned
395 * production certs, a cutover to a box without them is refused by every browser.
396 */
397describe('planSiteMove with TLS material', () => {
398 function certWorld(sourceCerts: Record<string, string>, targetCerts: Record<string, string> = {}) {
399 const base = world()
400 const carried = { count: 0, target: { ...targetCerts } }
401 const effects = {
402 ...base.effects,
403 certificates: {
404 inPlace: async () =>
405 certificatesMatch(new Map(Object.entries(sourceCerts)), new Map(Object.entries(carried.target))),
406 carry: async () => { carried.count++; carried.target = { ...sourceCerts } },
407 },
408 }
409 return { state: base.state, carried, effects }
410 }
411
412 it('carries certificates before routing, and well before DNS', async () => {
413 const p = await planSiteMove(options, certWorld({ 'bughq.example.com': 'abc' }).effects)
414 const ids = p.steps.map(step => step.id)
415 expect(ids.indexOf('certificates')).toBeLessThan(ids.indexOf('gateway'))
416 expect(ids.indexOf('certificates')).toBeLessThan(ids.indexOf('dns'))
417 expect(ids.indexOf('certificates')).toBeGreaterThan(ids.indexOf('restore'))
418 })
419
420 it('carries them when the target has none', async () => {
421 const { carried, effects } = certWorld({ 'bughq.example.com': 'abc' })
422 const p = await planSiteMove(options, effects)
423 expect((await applyPlan(p, await resolvePlan(p))).success).toBe(true)
424 expect(carried.count).toBe(1)
425 })
426
427 /** An OLDER cert for the same hostname is not the one being moved. */
428 it('replaces a stale certificate the target already holds', async () => {
429 const { carried, effects } = certWorld({ 'bughq.example.com': 'new' }, { 'bughq.example.com': 'expired' })
430 const resolved = await resolvePlan(await planSiteMove(options, effects))
431 expect(resolved.find(item => item.step.id === 'certificates')?.state).toBe('pending')
432 const p = await planSiteMove(options, effects)
433 await applyPlan(p, await resolvePlan(p))
434 expect(carried.count).toBe(1)
435 expect(carried.target['bughq.example.com']).toBe('new')
436 })
437
438 it('skips the carry when the target already holds the same material', async () => {
439 const { carried, effects } = certWorld({ 'bughq.example.com': 'abc' }, { 'bughq.example.com': 'abc' })
440 const p = await planSiteMove(options, effects)
441 const outcome = await applyPlan(p, await resolvePlan(p))
442 expect(outcome.steps.find(step => step.id === 'certificates')?.state).toBe('skipped')
443 expect(carried.count).toBe(0)
444 })
445
446 /** On-demand TLS may legitimately have issued nothing yet. */
447 it('does not block on a hostname the source has no certificate for', async () => {
448 const { effects } = certWorld({ 'bughq.example.com': 'absent' })
449 const resolved = await resolvePlan(await planSiteMove(options, effects))
450 expect(resolved.find(item => item.step.id === 'certificates')?.state).toBe('satisfied')
451 })
452
453 it('adds no certificate step when TLS is terminated off the box', async () => {
454 const p = await planSiteMove(options, world().effects)
455 expect(p.steps.map(step => step.id)).not.toContain('certificates')
456 })
457
458 it('leaves DNS on the source when the carry fails', async () => {
459 const { state, effects } = certWorld({ 'bughq.example.com': 'abc' })
460 const p = await planSiteMove(options, {
461 ...effects,
462 certificates: { ...effects.certificates, carry: async () => { throw new Error('permission denied') } },
463 })
464 const outcome = await applyPlan(p, await resolvePlan(p))
465 expect(outcome.success).toBe(false)
466 expect(outcome.steps.at(-1)?.id).toBe('certificates')
467 expect(state.published).toBe('203.0.113.1')
468 expect(state.sourceRunning).toBe(true)
469 })
470})
471
472describe('certificate scripts', () => {
473 it('reports a checksum per hostname, or absent', () => {
474 const script = buildCertificateStateScript('/etc/rpx/certs', ['bughq.example.com'])
475 expect(script).toContain("'/etc/rpx/certs/bughq.example.com.crt'")
476 expect(script).toContain('sha256sum')
477 expect(script).toContain('cert:bughq.example.com:absent')
478 })
479
480 it('parses the reported state', () => {
481 const state = parseCertificateState('cert:a.example.com:abc123\ncert:b.example.com:absent\nnoise')
482 expect(state.get('a.example.com')).toBe('abc123')
483 expect(state.get('b.example.com')).toBe('absent')
484 expect(state.size).toBe(2)
485 })
486
487 it('compares only the hostnames the source actually has a cert for', () => {
488 const source = new Map([['a', 'x'], ['b', 'absent']])
489 expect(certificatesMatch(source, new Map([['a', 'x']]))).toBe(true)
490 expect(certificatesMatch(source, new Map([['a', 'y']]))).toBe(false)
491 expect(certificatesMatch(source, new Map())).toBe(false)
492 })
493
494 it('packs the key alongside the certificate for every hostname', () => {
495 const script = buildCertificatePackScript('/etc/rpx/certs', ['a.example.com', 'b.example.com'], '/tmp/c.tar.gz')
496 expect(script).toContain("'a.example.com.crt'")
497 expect(script).toContain("'a.example.com.key'")
498 expect(script).toContain("'b.example.com.key'")
499 })
500
501 /** A site whose certificate has not been issued yet is not an error. */
502 it('exits clean when there is nothing to pack', () => {
503 const script = buildCertificatePackScript('/etc/rpx/certs', ['a.example.com'], '/tmp/c.tar.gz')
504 expect(script).toContain('no certificates to carry')
505 expect(script).toContain('exit 0')
506 })
507
508 /** A world-readable private key on a shared box is silent until it is not. */
509 it('restores private keys 0600', () => {
510 const script = buildCertificateUnpackScript('/etc/rpx/certs', '/tmp/c.tar.gz')
511 expect(script).toContain("chmod 600 '/etc/rpx/certs'/*.key")
512 expect(script).toContain('tar xzf')
513 })
514})
packages/ts-cloud/src/operations/site-move.tsmodified+134-0
Changes to packages/ts-cloud/src/operations/site-move.ts
@@ -217,6 +217,97 @@ export function parseSourceDrained(output: string): boolean {
217217 return !output.split('\n').some(line => line.trim().startsWith('active:'))
218218}
219219
220/** Where a site's TLS material is staged while it crosses between boxes. */
221export function siteMoveCertArchivePath(slug: string, siteName: string): string {
222 return `/tmp/ts-cloud-move-${slug}-${siteName}-certs.tar.gz`
223}
224
225/**
226 * Report a checksum per hostname for the cert the box holds, or `absent`.
227 *
228 * A checksum rather than "does the file exist": the target may hold an OLDER
229 * certificate for the same hostname — an expired one from a previous tenancy,
230 * or a staging cert issued while the domain still pointed elsewhere — and
231 * treating that as "already carried" would hand the cutover a certificate
232 * browsers reject.
233 *
234 * Read-only, so a plan can use it without changing either box.
235 */
236export function buildCertificateStateScript(certsDir: string, domains: readonly string[]): string {
237 return [
238 'set -u',
239 ...domains.map(domain =>
240 `if [ -s ${sh(`${certsDir}/${domain}.crt`)} ]; then`
241 + ` echo "cert:${domain}:$(sha256sum ${sh(`${certsDir}/${domain}.crt`)} | cut -d' ' -f1)";`
242 + ` else echo "cert:${domain}:absent"; fi`,
243 ),
244 ].join('\n')
245}
246
247/** `hostname → checksum | 'absent'` from {@link buildCertificateStateScript}. */
248export function parseCertificateState(output: string): Map<string, string> {
249 const state = new Map<string, string>()
250 for (const line of output.split('\n')) {
251 const trimmed = line.trim()
252 if (!trimmed.startsWith('cert:')) continue
253 const [, domain, checksum] = trimmed.split(':')
254 if (domain && checksum) state.set(domain, checksum)
255 }
256 return state
257}
258
259/**
260 * Is the target's TLS material already what the source holds?
261 *
262 * Only hostnames the SOURCE has a certificate for are compared. A site running
263 * behind on-demand TLS may legitimately have none yet, and a move must not
264 * block on carrying something that does not exist.
265 */
266export function certificatesMatch(source: Map<string, string>, target: Map<string, string>): boolean {
267 for (const [domain, checksum] of source) {
268 if (checksum === 'absent') continue
269 if (target.get(domain) !== checksum) return false
270 }
271 return true
272}
273
274/** Pack a site's certificate and private key for each hostname it serves. */
275export function buildCertificatePackScript(
276 certsDir: string,
277 domains: readonly string[],
278 archive: string,
279): string {
280 const names = domains.flatMap(domain => [`${domain}.crt`, `${domain}.key`])
281 return [
282 'set -eu',
283 `rm -f ${sh(archive)}`,
284 `cd ${sh(certsDir)} 2>/dev/null || { echo "no certs dir at ${certsDir}" >&2; exit 0; }`,
285 // `|| true` on the listing: a hostname with no cert yet is not an error, it
286 // is a site whose certificate has not been issued.
287 `TS_CLOUD_CERTS="$(ls ${names.map(sh).join(' ')} 2>/dev/null | tr '\\n' ' ' || true)"`,
288 '[ -n "$TS_CLOUD_CERTS" ] || { echo "no certificates to carry"; exit 0; }',
289 `tar czf ${sh(archive)} $TS_CLOUD_CERTS`,
290 ].join('\n')
291}
292
293/**
294 * Unpack the certificates on the target.
295 *
296 * The private keys are re-chmodded to 0600 after extraction. `tar` restores the
297 * modes it recorded, so this is belt-and-braces — but a world-readable private
298 * key on a shared box is the kind of mistake that is silent until it is not.
299 */
300export function buildCertificateUnpackScript(certsDir: string, archive: string): string {
301 return [
302 'set -eu',
303 `[ -s ${sh(archive)} ] || { echo "no certificate archive staged"; exit 0; }`,
304 `mkdir -p ${sh(certsDir)}`,
305 `tar xzf ${sh(archive)} -C ${sh(certsDir)}`,
306 `chmod 600 ${sh(certsDir)}/*.key 2>/dev/null || true`,
307 `rm -f ${sh(archive)}`,
308 ].join('\n')
309}
310
220311/**
221312 * The side effects a move needs, injected so the operation is testable without
222313 * two live boxes, a provider, or a DNS account.
@@ -258,6 +349,35 @@ export interface SiteMoveEffects {
258349 * connects to the same endpoint the source did, and there is nothing to move.
259350 */
260351 database?: SiteMoveDatabaseEffects
352 /**
353 * How to carry the site's TLS material, when the project terminates TLS on the
354 * box itself.
355 *
356 * Certificates live in the gateway's cert directory, which belongs to the box
357 * rather than to the site — the same shape as an on-box database, and the same
358 * failure if it is left behind. With pinned production certificates the
359 * cutover lands on a box that has no certificate for the hostname and every
360 * browser refuses it. With on-demand TLS the target re-issues on first
361 * request, so it self-heals, but only after a visible gap during which the
362 * site is down.
363 *
364 * Absent when TLS is terminated somewhere else entirely — a CDN, a managed
365 * load balancer — where the box never holds the material to begin with.
366 */
367 certificates?: SiteMoveCertificateEffects
368}
369
370/** Carrying a site's TLS material between boxes. */
371export interface SiteMoveCertificateEffects {
372 /**
373 * Does the target already hold exactly what the source holds?
374 *
375 * Compared by checksum rather than existence: an older certificate for the
376 * same hostname on the target is not the one being moved.
377 */
378 inPlace: () => Promise<boolean>
379 /** Pack on the source, carry, unpack on the target. */
380 carry: () => Promise<void>
261381}
262382
263383/** Carrying an on-box engine database between boxes, in resumable pieces. */
@@ -426,6 +546,20 @@ export async function planSiteMove(options: SiteMoveOptions, effects: SiteMoveEf
426546 }
427547
428548 steps.push(
549 ...(effects.certificates
550 ? [
551 {
552 id: 'certificates',
553 title: `Carry the site's TLS material to ${to}`,
554 // Before the route and well before DNS: a hostname that resolves to
555 // a box with no certificate for it is refused by every browser, and
556 // that is a worse failure than the site simply still being on the
557 // old box.
558 satisfied: () => effects.certificates!.inPlace(),
559 apply: () => effects.certificates!.carry(),
560 } satisfies OperationStep,
561 ]
562 : []),
429563 {
430564 id: 'gateway',
431565 title: `Route the site on ${to}`,