ReviewOS

also looking at this

stacks/ts-cloud

feat(consolidation): report an attach's credential radius, and rename a server in place

#175
Merged glennmichael123 wants to merge feat/consolidation-credential-reach-and-rename into main
12 files +1138 -14

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/config.mdmodified+26-3
Changes to docs/config.md
@@ -149,9 +149,32 @@ own provider project cannot be attached at all, because its token cannot see the
149149owner's box. Co-hosting trades credential isolation for a shared box; that trade
150150is often worth making, but it should be a decision rather than a surprise.
151151
152`describeCredentialReach()` and `formatCredentialReach()` report what a given
153token actually reaches, separating the owner's boxes from the ones no one asked
154for, so the radius can be shown before an attach is approved.
152Every attach deploy states this radius before it acts on it, splitting the
153owner's boxes from the ones nobody asked for:
154
155```
156Attaching to 'statushq' shares one provider project, so this deploy's credential
157can write to all 4 server(s) it can see.
1583 of them belong to neither project:
159 bughq: bughq-production-app
160 stacks: stacks-production-app
161 not managed by ts-cloud: some-legacy-box
162A compromised CI run or a mistargeted teardown in this project now reaches those.
163Keep the app in its own provider project instead if that is not acceptable, which
164rules out attaching.
165```
166
167It is reported, never enforced the trade is frequently worth making, and a
168deploy that started failing on upgrade would teach operators to silence it
169rather than read it. When the reach is exactly the two projects being joined it
170is one quiet line, because a warning that fires every time is a warning nobody
171reads.
172
173The radius comes from the driver (`CloudDriver.listReachableResources()`), not
174from a global assumption about tokens: a provider whose credential can be
175scoped per-resource simply enumerates less, and the same report comes out
176correct without a special case. `describeCredentialReach()` and
177`formatCredentialReach()` are exported for building your own plan output.
155178
156179### It cannot install services on the owner's box
157180
packages/core/src/drivers/types.tsmodified+31-0
Changes to packages/core/src/drivers/types.ts
@@ -134,6 +134,37 @@ export interface CloudDriver {
134134
135135 /** Run a shell script on every target (SSM, SSH, etc.) */
136136 runRemoteDeploy(options: RunRemoteDeployOptions): Promise<RemoteDeployResult>
137
138 /**
139 * Enumerate every resource this driver's credential can see — and therefore,
140 * on providers without per-resource scoping, write to and delete.
141 *
142 * Exists so nothing above the driver has to assume "one all-powerful token"
143 * is the only credential shape. Attaching to another project's box works by
144 * LISTING the provider with the attaching project's own credential, so the
145 * radius is a property of that credential rather than of the config, and only
146 * the driver holding it can report it. A driver whose credential IS narrowly
147 * scoped simply enumerates less, and the same reporting comes out right
148 * without a special case.
149 *
150 * Optional: a driver that cannot enumerate omits it, and callers report no
151 * radius rather than a wrong one.
152 *
153 * @see https://github.com/stacksjs/ts-cloud/issues/169
154 */
155 listReachableResources?(): Promise<ReachableResource[]>
156}
157
158/**
159 * One resource a provider credential can reach, reduced to what attribution
160 * needs: a name to print and the labels that say who owns it.
161 *
162 * Deliberately structural and provider-agnostic — a Hetzner server satisfies it
163 * as-is, and another driver can satisfy it without importing anything.
164 */
165export interface ReachableResource {
166 name: string
167 labels?: Record<string, string>
137168}
138169
139170export interface DeploySiteReleaseOptions {
packages/ts-cloud/src/deploy/attach-credentials.tsmodified+4-8
Changes to packages/ts-cloud/src/deploy/attach-credentials.ts
@@ -1,3 +1,4 @@
1import type { ReachableResource } from '@ts-cloud/core'
12import { TS_CLOUD_LABEL_PREFIX } from '../drivers/hetzner/instance-sizes'
23
34/**
@@ -28,15 +29,10 @@ import { TS_CLOUD_LABEL_PREFIX } from '../drivers/hetzner/instance-sizes'
2829const PROJECT_LABEL = `${TS_CLOUD_LABEL_PREFIX}/project`
2930
3031/**
31 * The minimum a driver has to produce for a server to be attributed.
32 *
33 * Structural rather than a provider type so a Hetzner server satisfies it as-is
34 * and another driver can satisfy it without importing anything.
32 * The minimum a driver has to produce for a server to be attributed — the shape
33 * `CloudDriver.listReachableResources()` returns.
3534 */
36export interface ReachableServer {
37 name: string
38 labels?: Record<string, string>
39}
35export type ReachableServer = ReachableResource
4036
4137export interface CredentialReach {
4238 /** Every server the credential enumerated. */
packages/ts-cloud/src/drivers/hetzner/driver.tsmodified+12-1
Changes to packages/ts-cloud/src/drivers/hetzner/driver.ts
@@ -1,4 +1,4 @@
1import type { CloudDriver, ComputeProxyConfig, ComputeStackOutputs, ComputeTarget, FindComputeTargetsOptions, ProvisionComputeOptions, RemoteDeployResult, RunRemoteDeployOptions, SiteConfig, UploadReleaseOptions, UploadReleaseResult } from '@ts-cloud/core'
1import type { CloudDriver, ComputeProxyConfig, ComputeStackOutputs, ComputeTarget, FindComputeTargetsOptions, ProvisionComputeOptions, ReachableResource, RemoteDeployResult, RunRemoteDeployOptions, SiteConfig, UploadReleaseOptions, UploadReleaseResult } from '@ts-cloud/core'
22import type { RpxLbAppBox } from '../shared/rpx-gateway'
33import type { HetznerFirewall, HetznerFirewallRule, HetznerServer } from './client'
44import type { HetznerDriverState } from './state'
@@ -1239,6 +1239,17 @@ export class HetznerDriver implements CloudDriver {
12391239 return this.outputsFromState(state)
12401240 }
12411241
1242 /**
1243 * Every server this token can see. On Hetzner that is every server in the
1244 * provider project, because a Cloud API token is project-scoped with Read or
1245 * Read & Write and no per-resource scoping — so "can see" and "can delete"
1246 * are the same set, and the count is the honest blast radius.
1247 */
1248 async listReachableResources(): Promise<ReachableResource[]> {
1249 const servers = await this.client.listServers()
1250 return servers.map((server) => ({ name: server.name, labels: server.labels }))
1251 }
1252
12421253 async runRemoteDeploy(options: RunRemoteDeployOptions): Promise<RemoteDeployResult> {
12431254 if (options.targets.length === 0) {
12441255 return { success: false, instanceCount: 0, perInstance: [], error: 'No targets provided' }
packages/ts-cloud/src/drivers/shared/compute-deploy.tsmodified+52-0
Changes to packages/ts-cloud/src/drivers/shared/compute-deploy.ts
@@ -4,6 +4,7 @@ import { copyFileSync } from 'node:fs'
44import { tmpdir } from 'node:os'
55import { join } from 'node:path'
66import { hasManagementDashboardSite, resolveAppDatabase, resolveProjectStackName } from '@ts-cloud/core'
7import { describeCredentialReach, formatCredentialReach, unrelatedReachCount } from '../../deploy/attach-credentials'
78import { buildManagementDashboardArtifact, ensureManagementDashboard, managementDashboardSiteNames } from '../../deploy/management-dashboard'
89import { isPhpSite, resolveSiteKind, siteInstallBase } from '../../deploy/site-target'
910import { buildSiteServicesScript, siteHasServices } from './app-services'
@@ -494,6 +495,53 @@ async function reconcileManagementDashboardServices(
494495 return true
495496}
496497
498/**
499 * Attach mode: report what this deploy's credential can actually reach.
500 *
501 * Attaching resolves the owner's box by LISTING the provider with the ATTACHING
502 * project's own credential. That listing is the whole mechanism, and it has a
503 * consequence the config never states — the owner's box has to be visible to
504 * this credential, so both projects share one provider project, and on a
505 * provider without per-resource scoping that means write over every server in
506 * it. Three apps that each owned one box become three pipelines that each reach
507 * all three.
508 *
509 * Reported, never enforced. The trade is frequently worth making, and a deploy
510 * that started failing on upgrade would teach operators to silence it rather
511 * than to read it. The quiet case stays quiet: when the reach is exactly the two
512 * projects being joined, this is one info line, because a warning that fires
513 * every time is a warning nobody reads.
514 *
515 * @see https://github.com/stacksjs/ts-cloud/issues/169
516 */
517async function reportAttachCredentialReach(
518 driver: CloudDriver,
519 options: DeployAllSitesOptions,
520 logger: ComputeDeployLogger,
521): Promise<void> {
522 const { config } = options
523 const ownerSlug = config.cloud?.attachTo
524 // A driver that cannot enumerate reports no radius rather than a wrong one.
525 if (!ownerSlug || !driver.listReachableResources) return
526
527 let resources
528 try {
529 resources = await driver.listReachableResources()
530 } catch (error) {
531 // Never fail a deploy over the advisory. A token that cannot list is a
532 // problem the real work reports far better than this can.
533 logger.warn(`Could not determine this credential's reach: ${error instanceof Error ? error.message : String(error)}`)
534 return
535 }
536
537 const reach = describeCredentialReach(resources, { ownerSlug, selfSlug: config.project.slug })
538 const surprising = unrelatedReachCount(reach) > 0
539 for (const line of formatCredentialReach(reach, { ownerSlug, selfSlug: config.project.slug })) {
540 if (surprising) logger.warn(line)
541 else logger.info(line)
542 }
543}
544
497545/**
498546 * Attach mode preflight: does the owner's box actually provide the on-box
499547 * services this project declares?
@@ -713,6 +761,10 @@ export async function deployAllComputeSites(options: DeployAllSitesOptions): Pro
713761 // ran green and the box kept a hand-maintained fragment.
714762 if (deployable.length === 0) return reloadRpxGateway(options)
715763
764 // Attach mode (`cloud.attachTo`): state the credential radius this attach
765 // implies before it is acted on. Advisory — see the function.
766 await reportAttachCredentialReach(driver, options, logger)
767
716768 // Attach mode (`cloud.attachTo`): before anything is built ON the owner's
717769 // services, confirm the owner's box actually runs them.
718770 if (!(await preflightAttachedHostServices(driver, options, logger))) return false
packages/ts-cloud/test/drivers/compute-deploy.test.tsmodified+114-0
Changes to packages/ts-cloud/test/drivers/compute-deploy.test.ts
@@ -1100,3 +1100,117 @@ describe('deployAllComputeSites attach-mode service preflight', () => {
11001100 expect(commands).not.toContain('ts_cloud_probe')
11011101 })
11021102})
1103
1104/**
1105 * Attaching resolves the owner's box by LISTING the provider with the ATTACHING
1106 * project's credential, so the owner's box must be visible to it — which on a
1107 * provider without per-resource scoping means write over every server in the
1108 * project. Reported so it is a decision rather than a discovery.
1109 */
1110describe('deployAllComputeSites attach-mode credential reach', () => {
1111 function attachedConfig(): CloudConfig {
1112 return {
1113 project: { name: 'Log HQ', slug: 'loghq', region: 'fsn1' },
1114 environments: { production: { type: 'production' } },
1115 cloud: { provider: 'hetzner', attachTo: 'statushq' },
1116 sites: { web: { domain: 'loghq.example.com', port: 3000, root: '.output', start: 'bun run server.ts' } },
1117 infrastructure: { compute: { runtime: 'bun', proxy: { engine: 'rpx' } } },
1118 }
1119 }
1120
1121 async function deployWith(reachable: Array<{ name: string, labels?: Record<string, string> }> | Error) {
1122 const warnings: string[] = []
1123 const infos: string[] = []
1124 const driver = createMockDriver({
1125 name: 'hetzner',
1126 usesCloudFormation: false,
1127 listReachableResources: mock(async () => {
1128 if (reachable instanceof Error) throw reachable
1129 return reachable
1130 }),
1131 })
1132 const tempDir = mkdtempSync(join(tmpdir(), 'ts-cloud-reach-'))
1133 const tarball = join(tempDir, 'release.tar.gz')
1134 writeFileSync(tarball, 'fake tarball')
1135 process.env.TS_CLOUD_UI_DISABLE = '1'
1136 const ok = await deployAllComputeSites({
1137 config: attachedConfig(),
1138 environment: 'production',
1139 driver,
1140 sha: 'abc',
1141 runtime: 'bun',
1142 tarballForSite: () => tarball,
1143 logger: {
1144 info: (message: string) => infos.push(message),
1145 warn: (message: string) => warnings.push(message),
1146 error: () => {},
1147 step: () => {},
1148 success: () => {},
1149 },
1150 }).finally(() => {
1151 delete process.env.TS_CLOUD_UI_DISABLE
1152 rmSync(tempDir, { recursive: true, force: true })
1153 })
1154 return { ok, warnings: warnings.join('\n'), infos: infos.join('\n') }
1155 }
1156
1157 const label = (project: string) => ({ 'ts-cloud/project': project })
1158
1159 it('warns, naming the servers neither project owns', async () => {
1160 const { ok, warnings } = await deployWith([
1161 { name: 'statushq-production-app', labels: label('statushq') },
1162 { name: 'bughq-production-app', labels: label('bughq') },
1163 { name: 'stacks-production-app', labels: label('stacks') },
1164 { name: 'some-legacy-box' },
1165 ])
1166 expect(ok).toBe(true)
1167 expect(warnings).toContain('all 4 server(s)')
1168 expect(warnings).toContain('bughq: bughq-production-app')
1169 expect(warnings).toContain('stacks: stacks-production-app')
1170 expect(warnings).toContain('not managed by ts-cloud: some-legacy-box')
1171 })
1172
1173 /**
1174 * A warning that fires every time is a warning nobody reads: when the reach is
1175 * exactly the two projects being joined there is nothing to decide.
1176 */
1177 it('stays quiet when the reach is only the two projects being joined', async () => {
1178 const { ok, warnings, infos } = await deployWith([
1179 { name: 'statushq-production-app', labels: label('statushq') },
1180 { name: 'loghq-production-app', labels: label('loghq') },
1181 ])
1182 expect(ok).toBe(true)
1183 expect(warnings).toBe('')
1184 expect(infos).toContain('Nothing outside the two projects being joined is reachable with it.')
1185 })
1186
1187 it('never fails the deploy when the credential cannot enumerate', async () => {
1188 const { ok, warnings } = await deployWith(new Error('403 forbidden'))
1189 expect(ok).toBe(true)
1190 expect(warnings).toContain('403 forbidden')
1191 })
1192
1193 it('reports nothing for a driver that cannot enumerate at all', async () => {
1194 const driver = createMockDriver({ name: 'hetzner', usesCloudFormation: false })
1195 expect(driver.listReachableResources).toBeUndefined()
1196 const warnings: string[] = []
1197 const tempDir = mkdtempSync(join(tmpdir(), 'ts-cloud-reach-'))
1198 const tarball = join(tempDir, 'release.tar.gz')
1199 writeFileSync(tarball, 'fake tarball')
1200 process.env.TS_CLOUD_UI_DISABLE = '1'
1201 const ok = await deployAllComputeSites({
1202 config: attachedConfig(),
1203 environment: 'production',
1204 driver,
1205 sha: 'abc',
1206 runtime: 'bun',
1207 tarballForSite: () => tarball,
1208 logger: { info: () => {}, warn: (m: string) => warnings.push(m), error: () => {}, step: () => {}, success: () => {} },
1209 }).finally(() => {
1210 delete process.env.TS_CLOUD_UI_DISABLE
1211 rmSync(tempDir, { recursive: true, force: true })
1212 })
1213 expect(ok).toBe(true)
1214 expect(warnings.join('\n')).not.toContain('credential')
1215 })
1216})