Escalate Cognito auth-disabled log to error in production - #214
Closed
chnnick wants to merge 1 commit into
Closed
Conversation
Running with auth off is a normal local workflow but almost always a missing-secrets bug in a deployed app, so CognitoModule now logs the disabled message at error level when NODE_ENV=production and keeps the warning otherwise. That check only means something if NODE_ENV is actually set, and nothing in this repo sets it: nx builds node targets with webpack mode 'none' specifically so process.env.NODE_ENV is not inlined, and there are no deployment configs here. So this also introduces the variable itself -- NODE_ENV=development in example.env, plus a README section on setting it in the deploy environment rather than a baked-in .env. Adds cognito.module.spec.ts, the module's first test: enabled, disabled in production, disabled in development, disabled with NODE_ENV unset (must stay a warning, since that is the default for every fresh clone), and partial Cognito config in production. Split out of #168 so that branch stays runnable with no env var to track. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
premature push by claude oops Testing out feature I wrote in the README for requiring you to explicitly be in a dev environment to have a missing cognito env variable disable auth and only warn you. If in production, and by default, it should just fail outright should there be any missing cognito variables. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently the cognito module implementation throws warnings when there are missing cognito env variables, but still allows the app to run with auth off. This is a potential feature addition for throwing loud errors rather than warnings based on not only the existing variables, but if you are in a prod or dev environment. It was written in the README, but this just builds it out for us.
Stacked on #168. Base is
140-Cognito— it will retarget tomainautomatically once #168 merges.Split out of #168 so that branch stays runnable with nothing new to track. This is the piece that asks downstream projects to manage an environment variable, and it is deliberately separate.
What this does
CognitoModulelogs the "auth disabled" startup message at error level whenNODE_ENV=production, and keeps the warn level otherwise. Running with auth off is a normal local workflow; in a deployed app it is almost always missing secrets.Why it also touches
example.envand the READMEThe check is inert on its own — nothing in this repo sets
NODE_ENV:@nx/webpackbuilds node targets withmode: 'none'specifically soprocess.env.NODE_ENVis not inlined, so the compiled backend reads it from the runtime environmentci-cd.ymlis entirely commented out, and the build workflows don't set it)So this introduces the variable:
NODE_ENV=developmentinexample.env, and a README section saying to set it in the deploy environment (ECS task definition, EB config, systemd unit) rather than a.envbaked into the image.Tests
Adds
cognito.module.spec.ts— the module had no spec at all. Covers auth enabled, disabled in production, disabled in development, disabled withNODE_ENVunset, and partial Cognito config in production.The unset case matters most: it is the default for every fresh clone, and it must stay a warning rather than an error. Jest sets
NODE_ENV='test', so the spec deletes it explicitly.Verified
Built the backend and booted it with the Cognito vars blank:
node dist/apps/backend/main.js→WARN [CognitoModule] Cognito auth disabled...NODE_ENV=production node dist/apps/backend/main.js→ERROR [CognitoModule] Cognito auth disabled...nx test backendpasses (61 tests, 7 suites);nx lint backendclean.Not included
CI workflows don't get
NODE_ENV— they only build and test, so setting it there would make CI diverge from local runs for no benefit.The
[!WARNING]block in the README still documents the stronger option (throwon boot instead of logging) as the next step for a project that wants a misconfigured deploy to fail rather than run open.🤖 Generated with Claude Code