Skip to content

Commit af8ad44

Browse files
fix(setup): pin kube context, keep secrets out of argv, validate reused keys
Review findings from #5911: - helm/kubectl now run against the validated context instead of the ambient one - helm values are piped on stdin rather than passed as --set arguments - ENCRYPTION_KEY/API_ENCRYPTION_KEY are checked for the 64-hex format the app requires, not just length, so an unusable key is replaced rather than kept - the managed Redis container's published port is read back instead of assumed
1 parent 090342c commit af8ad44

3 files changed

Lines changed: 101 additions & 28 deletions

File tree

scripts/setup/modes/k8s.ts

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,12 @@ const RELEASE = 'sim-dev'
1111
const NAMESPACE = 'sim-dev'
1212
const LOCAL_CONTEXT_PREFIXES = ['kind-', 'docker-desktop', 'minikube', 'orbstack']
1313

14-
function run(command: string, args: string[], failMessage: string): string {
15-
const result = spawnSync(command, args, { encoding: 'utf8' })
14+
/**
15+
* `input` is piped on stdin rather than passed as arguments — argv is readable
16+
* by any process on the machine, so secrets must never travel that way.
17+
*/
18+
function run(command: string, args: string[], failMessage: string, input?: string): string {
19+
const result = spawnSync(command, args, { encoding: 'utf8', input })
1620
if (result.status !== 0) {
1721
throw new Error(`${failMessage}: ${result.stderr.trim() || result.stdout.trim()}`)
1822
}
@@ -65,11 +69,12 @@ async function ensureLocalContext(detection: Detection): Promise<string> {
6569
return 'kind-sim'
6670
}
6771

68-
function existingReleaseSecrets(): Record<string, string> | null {
69-
const status = spawnSync('helm', ['status', RELEASE, '-n', NAMESPACE], { stdio: 'ignore' })
72+
function existingReleaseSecrets(context: string): Record<string, string> | null {
73+
const scope = ['--kube-context', context, '-n', NAMESPACE]
74+
const status = spawnSync('helm', ['status', RELEASE, ...scope], { stdio: 'ignore' })
7075
if (status.status !== 0) return null
7176
const values = JSON.parse(
72-
run('helm', ['get', 'values', RELEASE, '-n', NAMESPACE, '-o', 'json'], 'helm get values failed')
77+
run('helm', ['get', 'values', RELEASE, ...scope, '-o', 'json'], 'helm get values failed')
7378
) as { app?: { env?: Record<string, string> }; postgresql?: { auth?: { password?: string } } }
7479
const env = values.app?.env ?? {}
7580
const password = values.postgresql?.auth?.password
@@ -91,10 +96,27 @@ function existingReleaseSecrets(): Record<string, string> | null {
9196
}
9297
}
9398

99+
/**
100+
* Values document piped to helm on stdin instead of `--set`. `JSON.stringify`
101+
* quotes and escapes each value — JSON is a subset of YAML, so a secret
102+
* containing `#`, `:`, or a leading `*` can neither break the document nor be
103+
* reinterpreted as YAML syntax.
104+
*/
105+
function secretValues(secrets: Record<string, string>): string {
106+
const { POSTGRES_PASSWORD, ...appEnv } = secrets
107+
const env = Object.entries(appEnv)
108+
.map(([key, value]) => ` ${key}: ${JSON.stringify(value)}`)
109+
.join('\n')
110+
return `app:\n env:\n${env}\npostgresql:\n auth:\n password: ${JSON.stringify(POSTGRES_PASSWORD)}\n`
111+
}
112+
94113
export async function runK8sMode(detection: Detection): Promise<void> {
95-
await ensureLocalContext(detection)
114+
// Pin every subsequent call to the context we validated: the ambient context
115+
// can change between detection and deploy, which would send generated
116+
// credentials to an unintended cluster.
117+
const context = await ensureLocalContext(detection)
96118

97-
const reused = existingReleaseSecrets()
119+
const reused = existingReleaseSecrets(context)
98120
const secrets = reused ?? {
99121
BETTER_AUTH_SECRET: generateSecret(),
100122
ENCRYPTION_KEY: generateSecret(),
@@ -114,26 +136,21 @@ export async function runK8sMode(detection: Detection): Promise<void> {
114136
'--install',
115137
RELEASE,
116138
'./helm/sim',
139+
'--kube-context',
140+
context,
117141
'--namespace',
118142
NAMESPACE,
119143
'--create-namespace',
120144
'--values',
121145
'./helm/sim/examples/values-development.yaml',
122-
'--set',
123-
`app.env.BETTER_AUTH_SECRET=${secrets.BETTER_AUTH_SECRET}`,
124-
'--set',
125-
`app.env.ENCRYPTION_KEY=${secrets.ENCRYPTION_KEY}`,
126-
'--set',
127-
`app.env.INTERNAL_API_SECRET=${secrets.INTERNAL_API_SECRET}`,
128-
'--set',
129-
`app.env.CRON_SECRET=${secrets.CRON_SECRET}`,
130-
'--set',
131-
`postgresql.auth.password=${secrets.POSTGRES_PASSWORD}`,
146+
'--values',
147+
'-',
132148
'--wait',
133149
'--timeout',
134150
'15m',
135151
],
136-
'helm upgrade --install failed'
152+
'helm upgrade --install failed',
153+
secretValues(secrets)
137154
)
138155
} catch (error) {
139156
spin.stop(`${theme.error('✗')} helm install failed`)
@@ -147,7 +164,7 @@ export async function runK8sMode(detection: Detection): Promise<void> {
147164

148165
const testSpin = p.spinner()
149166
testSpin.start('Running helm test…')
150-
const test = spawnSync('helm', ['test', RELEASE, '-n', NAMESPACE], {
167+
const test = spawnSync('helm', ['test', RELEASE, '--kube-context', context, '-n', NAMESPACE], {
151168
encoding: 'utf8',
152169
cwd: ROOT,
153170
})
@@ -162,9 +179,9 @@ export async function runK8sMode(detection: Detection): Promise<void> {
162179

163180
p.note(
164181
[
165-
`kubectl -n ${NAMESPACE} port-forward svc/${RELEASE}-app 3000:3000`,
166-
`kubectl -n ${NAMESPACE} get pods`,
167-
`helm uninstall ${RELEASE} -n ${NAMESPACE} # tear down`,
182+
`kubectl --context ${context} -n ${NAMESPACE} port-forward svc/${RELEASE}-app 3000:3000`,
183+
`kubectl --context ${context} -n ${NAMESPACE} get pods`,
184+
`helm uninstall ${RELEASE} --kube-context ${context} -n ${NAMESPACE} # tear down`,
168185
].join('\n'),
169186
'Reach your cluster'
170187
)

scripts/setup/redis.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { spawnSync } from 'node:child_process'
12
import { docker } from './db.ts'
23
import { type Detection, REDIS_CONTAINER } from './detect.ts'
34
import { ensureDocker } from './docker.ts'
@@ -8,6 +9,18 @@ import { theme } from './theme.ts'
89

910
const LOCAL_URL = 'redis://localhost:6379'
1011

12+
/**
13+
* Host port the managed container actually publishes. `startManagedRedis` falls
14+
* back to 6380 when 6379 is taken, so assuming the default adopts the wrong
15+
* Redis — or fails the ping and then collides on the container name.
16+
*/
17+
function managedRedisUrl(): string | null {
18+
const result = spawnSync('docker', ['port', REDIS_CONTAINER, '6379/tcp'], { encoding: 'utf8' })
19+
if (result.status !== 0) return null
20+
const port = result.stdout.trim().split('\n')[0]?.split(':').pop()
21+
return port ? `redis://localhost:${port}` : null
22+
}
23+
1124
async function pingWithSpinner(url: string, label: string): Promise<boolean> {
1225
const spin = p.spinner()
1326
spin.start(label)
@@ -67,6 +80,17 @@ async function promptRedisUrl(existing?: string): Promise<string> {
6780
* consent), restart/start a wizard-managed container, or take a URL.
6881
*/
6982
export async function resolveRedis(detection: Detection, existing?: string): Promise<string> {
83+
// A configured REDIS_URL wins over the port scan: it may point at a non-default
84+
// port, or at a host that isn't this machine at all.
85+
if (
86+
existing &&
87+
existing !== LOCAL_URL &&
88+
(await pingWithSpinner(existing, `Pinging ${existing}…`))
89+
) {
90+
const keep = await p.confirm({ message: `Keep using ${existing}?`, initialValue: true })
91+
if (keep) return existing
92+
}
93+
7094
if (
7195
detection.redisPortOpen &&
7296
(await pingWithSpinner(LOCAL_URL, 'Redis found on :6379 — pinging…'))
@@ -78,11 +102,19 @@ export async function resolveRedis(detection: Detection, existing?: string): Pro
78102
if (adopt) return LOCAL_URL
79103
}
80104

81-
if (detection.redisContainer?.managed && detection.redisContainer.state === 'stopped') {
82-
docker(['start', REDIS_CONTAINER])
83-
if (await pingWithSpinner(LOCAL_URL, `Starting existing ${REDIS_CONTAINER} container…`)) {
84-
return LOCAL_URL
105+
if (detection.redisContainer?.managed) {
106+
if (detection.redisContainer.state === 'stopped') docker(['start', REDIS_CONTAINER])
107+
// Read the published port back rather than assuming the default.
108+
const managedUrl = managedRedisUrl()
109+
if (
110+
managedUrl &&
111+
(await pingWithSpinner(managedUrl, `Starting existing ${REDIS_CONTAINER} container…`))
112+
) {
113+
return managedUrl
85114
}
115+
p.log.warn(
116+
`${REDIS_CONTAINER} exists but is not answering — remove it with ${theme.command(`docker rm -f ${REDIS_CONTAINER}`)} before starting a new one.`
117+
)
86118
}
87119

88120
const dockerAvailable = await ensureDocker(false)

scripts/setup/steps.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,43 @@ import * as p from './prompter.ts'
55
import { link, theme } from './theme.ts'
66
import { FLAG_TWINS, hasMailProvider, LOGIN_PROVIDERS, SELF_HOST_UNLOCKS } from './twins.ts'
77

8+
/**
9+
* `ENCRYPTION_KEY` and `API_ENCRYPTION_KEY` are read as raw AES-256 material,
10+
* so the app requires exactly 64 hex characters and throws on anything else
11+
* (`lib/core/security/encryption.ts`, `lib/api-key/crypto.ts`). A merely-long
12+
* passphrase passes a length check here and then fails every encryption path at
13+
* runtime, so those two are validated on format rather than length.
14+
*/
15+
const HEX_KEY_PATTERN = /^[0-9a-f]{64}$/i
16+
const HEX_SECRET_KEYS = new Set(['ENCRYPTION_KEY', 'API_ENCRYPTION_KEY'])
17+
18+
function isUsableSecret(key: string, value: string): boolean {
19+
if (isPlaceholder(value)) return false
20+
return HEX_SECRET_KEYS.has(key) ? HEX_KEY_PATTERN.test(value) : value.length >= 32
21+
}
22+
823
/** Reuses existing valid secrets (never regenerates them) and generates the rest. */
924
export function collectSecrets(existing: EnvFile): Record<string, string> {
1025
const secrets: Record<string, string> = {}
1126
const generated: string[] = []
27+
const replaced: string[] = []
1228
for (const key of SECRET_KEYS) {
1329
const current = existing.vars.get(key)
14-
if (current && !isPlaceholder(current) && current.length >= 32) {
30+
if (current && isUsableSecret(key, current)) {
1531
secrets[key] = current
1632
} else {
1733
secrets[key] = generateSecret()
18-
generated.push(key)
34+
// A key the app would reject never successfully encrypted anything, so
35+
// replacing it cannot orphan existing ciphertext.
36+
if (current && !isPlaceholder(current)) replaced.push(key)
37+
else generated.push(key)
1938
}
2039
}
40+
if (replaced.length > 0) {
41+
p.log.warn(
42+
`Replaced ${replaced.join(', ')} — the existing value is not a 64-character hex key, which the app rejects at runtime.`
43+
)
44+
}
2145
if (generated.length > 0) {
2246
p.log.step(`Generated ${generated.join(', ')}`)
2347
}

0 commit comments

Comments
 (0)