The production boot guard for GITHUB_CLIENT_ID/GITHUB_CLIENT_SECRET is dead code — hardcoded `mock_*` fallbacks make both values permanently non-empty
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Idoneità per principianti
- 55/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- node.js, typescript
- Ambito
- authentication, backend, security, testing
Direzione di ricerca
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.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
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.
- Lingua principale
- TypeScript
- Stelle
- 2
- Fork
- 90
- Merge medio
- 31m
- PR unite (30g)
- 20
Preparare l'ambiente
- Include un Dockerfile o un file Docker Compose
- Nessun modello di pull request
- Nessuna guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di MergeFi/backend
-
bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
I maintainer di solito rispondono entro 1 giorno
-
bug security
Difficoltà 2/5 1-3 ore Idoneità per principianti 74/100
I maintainer di solito rispondono entro 1 giorno
-
bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 82/100
I maintainer di solito rispondono entro 1 giorno
-
bug testing
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
I maintainer di solito rispondono entro 1 giorno
-
bug security
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di MergeFi/backend
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
zapcooking/frontend#946 ·
I maintainer di solito rispondono entro 1 giorno
-
enhancement good first issue
Difficoltà 2/5 1-3 ore Idoneità per principianti 88/100
WiliamRosa/cosmos-semantic-platform#40 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 77/100
liberation-data/drivine#733 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 64/100
makecindy/cindy#10062 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
I maintainer di solito rispondono entro 1 giorno