The production boot guard for GITHUB_CLIENT_ID/GITHUB_CLIENT_SECRET is dead code — hardcoded `mock_*` fallbacks make both values permanently non-empty
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 3/5
- Tiempo estimado
- 1-2 días
- Aptitud para principiantes
- 55/100
- Tipo de issue
- Error
- Claridad
- Bien especificado
- Estado de actividad
- Activo
- Stack tecnológico
- node.js, typescript
- Área
- authentication, backend, security, testing
Línea de trabajo
Start with the config factory in src/config/configuration.ts and follow how its GitHub credentials reach collectConfigIssues() in src/config/validate-required-config.ts and the GithubStrategy constructor. Then inspect .env.example and the existing configuration validation specs. Done means missing or published placeholder credentials are rejected in production, the example file no longer supplies mock values, and tests cover the missing credentials; the issue also suggests checking local strategy registration behavior.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
Location
src/config/configuration.ts:88-89— the exported config factorysrc/config/validate-required-config.ts:91-100—collectConfigIssues()src/auth/strategies/github.strategy.ts:16-23—GithubStrategyconstructor.env.example:31-32
Problem
collectConfigIssues is meant to refuse a production boot when GitHub OAuth is not configured:
requireNonEmpty('GITHUB_CLIENT_ID', config.github.clientId, 'GitHub OAuth login cannot be completed');
requireNonEmpty('GITHUB_CLIENT_SECRET', config.github.clientSecret, 'GitHub OAuth login cannot be completed');
requireNonEmpty only reports an issue when the value is falsy or whitespace (validate-required-config.ts:48-59). But the config factory never produces a falsy value for either key:
clientId: process.env.GITHUB_CLIENT_ID || 'mock_client_id_12345',
clientSecret: process.env.GITHUB_CLIENT_SECRET || 'mock_secret_key_67890',
So with GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET completely unset in production, config.github.clientId is the non-empty string 'mock_client_id_12345', requireNonEmpty passes, and both checks can never fire. They are unreachable branches.
The same two literals are hardcoded a second time inside GithubStrategy's constructor (github.strategy.ts:17-18), with a comment that states the intent explicitly: "If the config system returns an empty string or undefined, it automatically uses static string fallbacks so Passport NEVER crashes." That second layer guarantees that even a fixed config factory would not surface the problem at construction time.
.env.example:31-32 ships the same two mock values as if they were real settings, so an operator who copies .env.example to .env in production inherits them without ever seeing a blank to fill in.
This is the same class of defect as closed issue #288 (ENCRYPTION_KEY exempt from production enforcement because of a published fallback), applied to the OAuth client credentials instead.
Why it matters
The whole point of assertRequiredConfig running before NestFactory.create (see the comment block at src/main.ts:12-25) is that a misconfigured production pod must refuse to start rather than come up looking healthy. For GitHub OAuth that guarantee does not exist:
- A production deploy that forgot
GITHUB_CLIENT_ID/GITHUB_CLIENT_SECRETboots cleanly, reports healthy onGET /, and passes every readiness check. - The only way anyone discovers it is a user clicking "Sign in with GitHub" and being bounced by GitHub's own error page, or — once the callback runs — landing on
/auth/callback?error=oauth_failedviaAuthController.githubCallback's catch block. Since GitHub OAuth is the only authentication mechanism, the entire platform is unusable while reporting itself healthy. - Worse, the fallbacks are guessable, published values.
'mock_client_id_12345'is a real OAuth client id from GitHub's perspective — just one nobody controls. If anyone ever registers a GitHub OAuth app whose client id happens to collide with that literal, a production deploy with the env vars missing would complete an OAuth handshake against an app controlled by a third party, who would then receive the authorization code and theredirect_urifor this platform's users.
Reproduction / failure path
- Build the image and run it with
NODE_ENV=production, a realDATABASE_URL, a realJWT_SECRET,GITHUB_WEBHOOK_SECRET,ESCROW_CONTRACT_IDandTREASURY_SECRETset — andGITHUB_CLIENT_ID/GITHUB_CLIENT_SECRETdeliberately unset. - Observed:
assertRequiredConfigthrows nothing, the app listens, and the startup banner prints normally. GET /api/auth/githubredirects the browser tohttps://github.com/login/oauth/authorize?client_id=mock_client_id_12345&..., where GitHub replies with "The redirect_uri MUST match the registered callback URL for this application" / an unknown-client error.- Expected: the process exits at boot with
Refusing to start in production with an incomplete configuration: - GITHUB_CLIENT_ID is unset ....
Suggested fix
Two changes, both needed:
- In
src/config/configuration.ts, stop manufacturing a value. Read the raw env var and let it be empty so the validator can see it:
clientId: process.env.GITHUB_CLIENT_ID ?? '',
clientSecret: process.env.GITHUB_CLIENT_SECRET ?? '',
oauthCallbackUrl: process.env.GITHUB_OAUTH_CALLBACK_URL ?? '',
collectConfigIssues already returns early outside production (validate-required-config.ts:46), so local development is unaffected by the stricter config — development simply gets an empty client id, which is the honest representation of "not configured".
- In
src/auth/strategies/github.strategy.ts, remove the|| 'mock_client_id_12345'/|| 'mock_secret_key_67890'fallbacks and let the strategy throw when the credentials are absent in production. If "Passport must never crash in dev" is a requirement, gate the strategy's registration onenv !== 'production'inAuthModuleinstead of substituting fake credentials silently.
Additionally, in src/config/validate-required-config.ts, add explicit "still the published placeholder" checks for both values — the same pattern already used for INSECURE_DEFAULT_JWT_SECRET and DEFAULT_LOCAL_DATABASE_URL — so a deploy that copied .env.example verbatim is also refused:
export const PLACEHOLDER_GITHUB_CLIENT_ID = 'mock_client_id_12345';
export const PLACEHOLDER_GITHUB_CLIENT_SECRET = 'mock_secret_key_67890';
Finally, replace the values in .env.example:31-32 with empty assignments (GITHUB_CLIENT_ID=) so the file reads as a form to fill in rather than as working defaults, and add a configuration.spec.ts / validate-required-config.spec.ts case asserting that a production config with both env vars unset produces two issues.
- Lenguaje dominante
- TypeScript
- Estrellas
- 2
- Forks
- 90
- Merge medio
- 31 min
- PR fusionados (30 d)
- 20
Preparar el entorno
- Incluye un Dockerfile o un archivo de Docker Compose
- Sin plantilla de pull request
- Sin guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de MergeFi/backend
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
Los mantenedores suelen responder en 1 día
-
bug security
Dificultad 2/5 1-3 horas Aptitud para principiantes 74/100
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
Los mantenedores suelen responder en 1 día
-
bug testing
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
Los mantenedores suelen responder en 1 día
-
bug security
Dificultad 2/5 1-3 horas Aptitud para principiantes 76/100
Los mantenedores suelen responder en 1 día
Todos los issues de MergeFi/backend
Issues similares
-
ログイン中の404画面にヘッダーとナビを出すAbiertoCodex使用可 コード
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
ryuki-moro/Nicomaru#218 ·
Los mantenedores suelen responder en 3 días
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
sveltejs/svelte-eslint-parser#1008 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
silverbucket/inbox-rs#351 ·
Los mantenedores suelen responder en 1 día
-
[Bug]: The H2 series' and the H3C's fan 3 is labelled "Chamber Fan", Bambu calls it the exhaust fanAbiertoarea:ui
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
Los mantenedores suelen responder en 1 día
-
bug front P2
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
invoicerr-app/invoicerr#964 ·
Los mantenedores suelen responder en 1 día