Skip to content

Commit f335613

Browse files
authored
fix(cli): reject placeholder secret inherited from env or prompt in setup (#1018) (#1058)
Fixes #1018 ### Overview Addresses security finding **F-030 (weak cryptography / CWE-330)** where `nostream setup` could report success while retaining the default placeholder secret in `.env`. ### Problem When running setup via the CLI, the package entry point preloads `.env` into `process.env`. If `.env` contains the default placeholder (`change_me_to_something_long_and_random`), `process.env.SECRET` inherits that value. Previously: 1. `resolveSecret()` checked `if (process.env.SECRET?.trim())` and returned it without verifying whether it was the default placeholder. 2. `ensureEnvFile()` persisted this value back to `.env` without secondary validation, resulting in setup completing while retaining an insecure, predictable session signing secret. 3. The interactive secret prompt only validated that the input was non-empty, allowing the default placeholder to be entered manually. ### Changes - **Reject Inherited Placeholder**: Updated `resolveSecret()` in `src/cli/commands/setup.ts` to check `!needsSecretReplacement(inheritedSecret)` before accepting `process.env.SECRET`. - **Interactive Validation**: Extended prompt validation in `setupPrompts.text` to reject the default placeholder string in addition to empty input. - **Defense-in-Depth Persistence Guard**: Added a validation check in `ensureEnvFile()` before writing to disk; if the resolved secret is empty or matches the placeholder, a secure 64-byte random hex secret (`crypto.randomBytes(64)`) is generated. - **Process Memory Synchronization**: Ensured `process.env.SECRET` is updated with the final secret so downstream steps in the same process (such as `runStart`) use the new secret immediately. - **Unit Tests**: Added test coverage in `test/unit/cli/setup.spec.ts` covering: - Rejecting an inherited placeholder from `process.env.SECRET` and generating a fresh random secret. - Interactive prompt validation rejecting empty strings and placeholder values. - Final fallback replacement if interactive input somehow resolved to the placeholder. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Setup rejects the default placeholder when supplied through inherited environment variables or interactive input. Prompted secrets must be non-empty and cannot contain `#` or newlines. * If the resolved secret is missing or remains a placeholder, setup generates a secure secret and makes it available to the running process. * Quoted secrets—including values with escaped quotes or `#`—are parsed correctly. Valid secrets from the environment file are also made available to the running process. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents f7b9d1d + 550ceb8 commit f335613

3 files changed

Lines changed: 198 additions & 8 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"nostream": patch
3+
---
4+
5+
fix(cli): reject placeholder secret inherited from env or prompt in setup (#1018)
6+
7+
Setup now rejects the default placeholder secret (`change_me_to_something_long_and_random`) when read from inherited environment variables or interactive prompt input, and validates the resolved secret before persisting to `.env`. If a placeholder is detected, setup automatically generates a fresh cryptographically secure 64-byte random hex secret.

‎src/cli/commands/setup.ts‎

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,42 +29,102 @@ class SetupCancelledError extends Error {
2929
}
3030
}
3131

32+
const stripQuotes = (value: string): string => {
33+
const trimmed = value.trim()
34+
if ((trimmed.startsWith('"') && trimmed.endsWith('"')) || (trimmed.startsWith("'") && trimmed.endsWith("'"))) {
35+
return trimmed.slice(1, -1).trim()
36+
}
37+
return trimmed
38+
}
39+
40+
const parseEnvValue = (rawValue: string): string => {
41+
const trimmed = rawValue.trim()
42+
if (!trimmed) {
43+
return ''
44+
}
45+
46+
if (trimmed.startsWith('"')) {
47+
let closingIndex = -1
48+
for (let i = 1; i < trimmed.length; i += 1) {
49+
if (trimmed[i] === '"' && trimmed[i - 1] !== '\\') {
50+
closingIndex = i
51+
break
52+
}
53+
}
54+
if (closingIndex !== -1) {
55+
return trimmed.slice(1, closingIndex).replace(/\\"/g, '"')
56+
}
57+
return trimmed.slice(1)
58+
}
59+
60+
if (trimmed.startsWith("'")) {
61+
const closingIndex = trimmed.indexOf("'", 1)
62+
if (closingIndex !== -1) {
63+
return trimmed.slice(1, closingIndex)
64+
}
65+
return trimmed.slice(1)
66+
}
67+
68+
const [unquoted] = trimmed.split('#', 1)
69+
return unquoted.trim()
70+
}
71+
3272
const readEnvSecret = (content: string): string | undefined => {
3373
for (const line of content.split(/\r?\n/)) {
3474
const trimmed = line.trim()
3575
if (!trimmed || trimmed.startsWith('#') || !trimmed.startsWith('SECRET=')) {
3676
continue
3777
}
3878

39-
const [rawValue] = trimmed.slice('SECRET='.length).split('#', 1)
40-
return rawValue.trim()
79+
const raw = trimmed.slice('SECRET='.length)
80+
return parseEnvValue(raw)
4181
}
4282

4383
return undefined
4484
}
4585

4686
const needsSecretReplacement = (secret: string | undefined): boolean => {
47-
return !secret || secret === SECRET_PLACEHOLDER
87+
const unquoted = secret ? stripQuotes(secret) : undefined
88+
return !unquoted || unquoted === SECRET_PLACEHOLDER
4889
}
4990

5091
const resolveSecret = async (assumeYes: boolean): Promise<string> => {
51-
if (process.env.SECRET?.trim()) {
52-
return process.env.SECRET.trim()
92+
const inheritedSecret = process.env.SECRET?.trim()
93+
if (inheritedSecret && !needsSecretReplacement(inheritedSecret)) {
94+
return stripQuotes(inheritedSecret)
5395
}
5496

5597
if (!assumeYes && process.stdin.isTTY) {
5698
const value = await setupPrompts.text({
5799
message: 'SECRET env var value (hex recommended)',
58100
placeholder: 'openssl rand -hex 128',
59-
validate: (input) => (input.trim() ? undefined : 'SECRET is required'),
101+
validate: (input) => {
102+
const trimmed = stripQuotes(input)
103+
if (!trimmed) {
104+
return 'SECRET is required'
105+
}
106+
if (needsSecretReplacement(trimmed)) {
107+
return 'SECRET cannot be the default placeholder'
108+
}
109+
if (input.includes('#')) {
110+
return 'SECRET cannot contain "#"'
111+
}
112+
if (/[\r\n]/.test(input)) {
113+
return 'SECRET cannot contain newlines'
114+
}
115+
return undefined
116+
},
60117
})
61118

62119
if (setupPrompts.isCancel(value)) {
63120
setupPrompts.cancel('Setup cancelled')
64121
throw new SetupCancelledError()
65122
}
66123

67-
return value.trim()
124+
const trimmed = value.trim()
125+
if (!needsSecretReplacement(trimmed)) {
126+
return trimmed
127+
}
68128
}
69129

70130
return randomBytes(64).toString('hex')
@@ -119,7 +179,11 @@ const ensureEnvFile = async (assumeYes: boolean): Promise<boolean> => {
119179

120180
const current = fs.readFileSync(envPath, 'utf-8')
121181

122-
if (!needsSecretReplacement(readEnvSecret(current))) {
182+
const envFileSecret = readEnvSecret(current)
183+
if (!needsSecretReplacement(envFileSecret)) {
184+
if (needsSecretReplacement(process.env.SECRET) && envFileSecret) {
185+
process.env.SECRET = envFileSecret
186+
}
123187
return true
124188
}
125189

@@ -133,6 +197,11 @@ const ensureEnvFile = async (assumeYes: boolean): Promise<boolean> => {
133197
throw error
134198
}
135199

200+
if (needsSecretReplacement(secret)) {
201+
secret = randomBytes(64).toString('hex')
202+
}
203+
204+
process.env.SECRET = secret
136205
fs.writeFileSync(envPath, upsertSecret(current, secret), 'utf-8')
137206
return true
138207
}

‎test/unit/cli/setup.spec.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,118 @@ describe('runSetup', () => {
114114
expect(outroStub.notCalled).to.equal(true)
115115
expect(readEnv()).to.equal('SECRET=change_me_to_something_long_and_random\n')
116116
})
117+
118+
it('rejects an inherited placeholder secret from process.env.SECRET and generates a fresh secret', async () => {
119+
fs.writeFileSync(path.join(tempDir, '.env.example'), 'SECRET=change_me_to_something_long_and_random\n', 'utf-8')
120+
process.env.SECRET = 'change_me_to_something_long_and_random'
121+
122+
const code = await setupCommand.runSetup({ yes: true })
123+
124+
expect(code).to.equal(0)
125+
const generatedSecret = readEnv().match(/^SECRET=([a-f0-9]+)$/m)?.[1]
126+
expect(generatedSecret).to.not.equal(undefined)
127+
expect(generatedSecret).to.not.equal('change_me_to_something_long_and_random')
128+
expect(generatedSecret).to.have.length(128)
129+
expect(process.env.SECRET).to.equal(generatedSecret)
130+
})
131+
132+
it('validates secret input in interactive mode and rejects empty or placeholder values', async () => {
133+
process.stdin.isTTY = true
134+
fs.writeFileSync(path.join(tempDir, '.env.example'), 'SECRET=change_me_to_something_long_and_random\n', 'utf-8')
135+
136+
let validateFn: ((input: string) => string | undefined) | undefined
137+
sinon.stub(setupCommand.setupPrompts, 'text').callsFake((opts: any) => {
138+
validateFn = opts.validate
139+
return Promise.resolve('my-strong-interactive-secret')
140+
})
141+
sinon.stub(setupCommand.setupPrompts, 'confirm').resolves(false as any)
142+
143+
const code = await setupCommand.runSetup({ yes: false })
144+
145+
expect(code).to.equal(0)
146+
expect(validateFn).to.be.a('function')
147+
expect(validateFn!('')).to.equal('SECRET is required')
148+
expect(validateFn!(' ')).to.equal('SECRET is required')
149+
expect(validateFn!('change_me_to_something_long_and_random')).to.equal('SECRET cannot be the default placeholder')
150+
expect(validateFn!(' change_me_to_something_long_and_random ')).to.equal(
151+
'SECRET cannot be the default placeholder',
152+
)
153+
expect(validateFn!('my-strong-interactive-secret')).to.equal(undefined)
154+
expect(validateFn!('secret#comment')).to.equal('SECRET cannot contain "#"')
155+
expect(validateFn!('secret\nnewline')).to.equal('SECRET cannot contain newlines')
156+
157+
expect(readEnv()).to.include('SECRET=my-strong-interactive-secret')
158+
expect(process.env.SECRET).to.equal('my-strong-interactive-secret')
159+
})
160+
161+
it('replaces placeholder with generated secret if interactive input somehow resolves to placeholder', async () => {
162+
process.stdin.isTTY = true
163+
fs.writeFileSync(path.join(tempDir, '.env.example'), 'SECRET=change_me_to_something_long_and_random\n', 'utf-8')
164+
165+
sinon.stub(setupCommand.setupPrompts, 'text').resolves('change_me_to_something_long_and_random')
166+
sinon.stub(setupCommand.setupPrompts, 'confirm').resolves(false as any)
167+
168+
const code = await setupCommand.runSetup({ yes: false })
169+
170+
expect(code).to.equal(0)
171+
const generatedSecret = readEnv().match(/^SECRET=([a-f0-9]+)$/m)?.[1]
172+
expect(generatedSecret).to.not.equal(undefined)
173+
expect(generatedSecret).to.not.equal('change_me_to_something_long_and_random')
174+
expect(generatedSecret).to.have.length(128)
175+
expect(process.env.SECRET).to.equal(generatedSecret)
176+
})
177+
178+
it('replaces a quoted placeholder secret in .env', async () => {
179+
fs.writeFileSync(path.join(tempDir, '.env'), 'SECRET="change_me_to_something_long_and_random"\nFOO=bar\n', 'utf-8')
180+
181+
const code = await setupCommand.runSetup({ yes: true })
182+
183+
expect(code).to.equal(0)
184+
const generatedSecret = readEnv().match(/^SECRET=([a-f0-9]+)$/m)?.[1]
185+
expect(generatedSecret).to.not.equal(undefined)
186+
expect(generatedSecret).to.not.equal('change_me_to_something_long_and_random')
187+
expect(generatedSecret).to.have.length(128)
188+
expect(process.env.SECRET).to.equal(generatedSecret)
189+
})
190+
191+
it('syncs process.env.SECRET from an existing valid .env secret when process.env.SECRET held a placeholder', async () => {
192+
fs.writeFileSync(path.join(tempDir, '.env'), 'SECRET="persisted-real-secret"\n', 'utf-8')
193+
process.env.SECRET = 'change_me_to_something_long_and_random'
194+
195+
const code = await setupCommand.runSetup({ yes: true })
196+
197+
expect(code).to.equal(0)
198+
expect(process.env.SECRET).to.equal('persisted-real-secret')
199+
})
200+
201+
it('preserves full quoted secret containing # from .env and does not truncate at #', async () => {
202+
fs.writeFileSync(path.join(tempDir, '.env'), 'SECRET="abc#def" # trailing comment\n', 'utf-8')
203+
204+
const code = await setupCommand.runSetup({ yes: true })
205+
206+
expect(code).to.equal(0)
207+
expect(process.env.SECRET).to.equal('abc#def')
208+
expect(readEnv()).to.equal('SECRET="abc#def" # trailing comment\n')
209+
})
210+
211+
it('preserves explicitly supplied valid process.env.SECRET when .env contains a different valid secret', async () => {
212+
fs.writeFileSync(path.join(tempDir, '.env'), 'SECRET="file-secret"\n', 'utf-8')
213+
process.env.SECRET = 'operator-supplied-secret'
214+
215+
const code = await setupCommand.runSetup({ yes: true })
216+
217+
expect(code).to.equal(0)
218+
expect(process.env.SECRET).to.equal('operator-supplied-secret')
219+
expect(readEnv()).to.equal('SECRET="file-secret"\n')
220+
})
221+
222+
it('correctly handles escaped quotes inside double-quoted secrets', async () => {
223+
fs.writeFileSync(path.join(tempDir, '.env'), 'SECRET="sec\\"ret#with#hashes" # comment\n', 'utf-8')
224+
225+
const code = await setupCommand.runSetup({ yes: true })
226+
227+
expect(code).to.equal(0)
228+
expect(process.env.SECRET).to.equal('sec"ret#with#hashes')
229+
expect(readEnv()).to.equal('SECRET="sec\\"ret#with#hashes" # comment\n')
230+
})
117231
})

0 commit comments

Comments
 (0)