fix dashboard shared email provider display for self-hosted env config #1353
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1165,6 +1165,9 @@ export const renderedOrganizationConfigToProjectCrud = (renderedConfig: Complete | |||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| email_config: renderedConfig.emails.server.isShared ? { | ||||||||||||||||||||||||||||||||||||||||||||
| type: 'shared', | ||||||||||||||||||||||||||||||||||||||||||||
| host: getEnvVariable("STACK_EMAIL_HOST"), | ||||||||||||||||||||||||||||||||||||||||||||
| port: parseInt(getEnvVariable("STACK_EMAIL_PORT")), | ||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| sender_email: getEnvVariable("STACK_EMAIL_SENDER"), | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
1166
to
+1170
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check whether STACK_EMAIL_HOST/PORT/SENDER are set unconditionally in deployment/env configs.
rg -nP 'STACK_EMAIL_(HOST|PORT|SENDER)\b' -C2
# Verify getEnvVariable semantics
rg -nP --type=ts -C3 'export function getEnvVariable\b'Repository: stack-auth/stack-auth Length of output: 7314 🏁 Script executed: cat -n packages/stack-shared/src/utils/env.tsx | head -50Repository: stack-auth/stack-auth Length of output: 2214 🏁 Script executed: sed -n '49,65p' packages/stack-shared/src/utils/env.tsxRepository: stack-auth/stack-auth Length of output: 443 🏁 Script executed: # Check when isShared is set to true in email config
rg -n 'isShared' apps/backend/src/lib/config.tsx -B5 -A5 | head -60Repository: stack-auth/stack-auth Length of output: 2856 🏁 Script executed: # Look for where emails.server.isShared is actually set/determined
rg -n 'emails.server' apps/backend/src/lib/config.tsx -B3 -A3 | head -80Repository: stack-auth/stack-auth Length of output: 1745 🏁 Script executed: # Find where emails.server configuration is built/rendered
rg -n 'emails.*server' packages/stack-shared/src -A2 -B2 | grep -A5 -B5 'isShared'Repository: stack-auth/stack-auth Length of output: 765 🏁 Script executed: # Find where isShared is assigned/set for emails
rg -n 'isShared.*=' packages/stack-shared/src/config/schema.ts -B5 -A2Repository: stack-auth/stack-auth Length of output: 1685 🏁 Script executed: # Search for where isShared is computed in the config schema
rg -n 'isShared:' packages/stack-shared/src/config/schema.ts -B5 -A5Repository: stack-auth/stack-auth Length of output: 3169 🏁 Script executed: # Check if STACK_EMAIL_HOST/PORT/SENDER are set in any standard deployment configs
find . -name ".env*" -o -name "*.env" | head -20 | xargs grep -l "STACK_EMAIL" 2>/dev/null || echo "No .env files found with STACK_EMAIL"Repository: stack-auth/stack-auth Length of output: 165 🏁 Script executed: # Check contents of these env files for STACK_EMAIL variables
echo "=== apps/backend/.env ===" && cat apps/backend/.env | grep -i "STACK_EMAIL" || echo "Not found"
echo -e "\n=== apps/backend/.env.development ===" && cat apps/backend/.env.development | grep -i "STACK_EMAIL" || echo "Not found"
echo -e "\n=== docker/server/.env ===" && cat docker/server/.env | grep -i "STACK_EMAIL" || echo "Not found"Repository: stack-auth/stack-auth Length of output: 3318 Critical:
The downstream UI already handles 🛠️ Proposed fix email_config: renderedConfig.emails.server.isShared ? {
type: 'shared',
- host: getEnvVariable("STACK_EMAIL_HOST"),
- port: parseInt(getEnvVariable("STACK_EMAIL_PORT")),
- sender_email: getEnvVariable("STACK_EMAIL_SENDER"),
+ host: getEnvVariable("STACK_EMAIL_HOST", "") || undefined,
+ port: (() => {
+ const raw = getEnvVariable("STACK_EMAIL_PORT", "");
+ if (!raw) return undefined;
+ const parsed = parseInt(raw, 10);
+ return Number.isFinite(parsed) ? parsed : undefined;
+ })(),
+ sender_email: getEnvVariable("STACK_EMAIL_SENDER", "") || undefined,
} : renderedConfig.emails.server.provider === "managed" ? {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
1166
to
+1170
There was a problem hiding this comment.
Comment on lines
1166
to
+1170
There was a problem hiding this comment. This new shared-email serialization behavior isn’t covered by the in-source vitest tests in this file. Adding a small test for
Comment on lines
+1168
to
+1170
There was a problem hiding this comment.
Suggested change
Missing default values for |
||||||||||||||||||||||||||||||||||||||||||||
| } : renderedConfig.emails.server.provider === "managed" ? { | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
1166
to
1171
Contributor
There was a problem hiding this comment.
A safe approach is to fall back to email_config: renderedConfig.emails.server.isShared ? {
type: 'shared',
...(process.env.STACK_EMAIL_HOST ? { host: process.env.STACK_EMAIL_HOST } : {}),
...(process.env.STACK_EMAIL_PORT ? { port: parseInt(process.env.STACK_EMAIL_PORT) } : {}),
...(process.env.STACK_EMAIL_SENDER ? { sender_email: process.env.STACK_EMAIL_SENDER } : {}),
} : ...Note: Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/backend/src/lib/config.tsx
Line: 1166-1171
Comment:
**Unconditional throw breaks config reads when env vars are absent**
`getEnvVariable` throws `Missing environment variable: <name>` if the named variable is not set and no default is provided. Before this PR, the `isShared` branch returned only `{ type: 'shared' }` and never touched the env vars. After this PR every call to `renderedOrganizationConfigToProjectCrud` (which backs the `/api/v1/internal/projects` read endpoint) will throw for any project in shared-email mode that doesn't have all three of `STACK_EMAIL_HOST`, `STACK_EMAIL_PORT`, and `STACK_EMAIL_SENDER` defined. Self-hosted operators who haven't set these vars (or deliberately run without email) would find their entire project config endpoint broken after this change, even though no email-sending code was touched.
A safe approach is to fall back to `undefined`/`null` when the var is absent rather than throwing:
```typescript
email_config: renderedConfig.emails.server.isShared ? {
type: 'shared',
...(process.env.STACK_EMAIL_HOST ? { host: process.env.STACK_EMAIL_HOST } : {}),
...(process.env.STACK_EMAIL_PORT ? { port: parseInt(process.env.STACK_EMAIL_PORT) } : {}),
...(process.env.STACK_EMAIL_SENDER ? { sender_email: process.env.STACK_EMAIL_SENDER } : {}),
} : ...
```
Note: `getEnvVariable` cannot be used here with a fallback because an empty string is treated as "not set" by that utility; using `process.env` directly and checking truthiness is safer.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||
| type: 'standard', | ||||||||||||||||||||||||||||||||||||||||||||
| host: "smtp.resend.com", | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parseIntcan silently produceNaNIf
STACK_EMAIL_PORTis set but contains a non-integer value (e.g."465abc"or whitespace),parseInt(...)returnsNaN. ThisNaNvalue will be serialised and sent to the dashboard, where it may render as"NaN"in the port fields. Adding a validation guard or usingNumber()with aNumber.isIntegercheck would make the failure explicit rather than silent.Prompt To Fix With AI