fix(core): Refuse default superadmin credentials in production - #4718
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request adds a security validation utility to detect and prevent usage of well-known default superadmin credentials at application bootstrap. The utility function 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
michaelbromley
left a comment
There was a problem hiding this comment.
Hi Martin, this is generally a good idea but a couple of changes needed:
- We should not disallow the
superadminidentifier. It’s totally fine to use that, as long as there’s a non-default, strong password. - This must target the
minorbranch. It is a breaking change in behaviour.
Vendure shipped with hardcoded default `superadmin` / `superadmin` credentials and no built-in guard against using them in production. Add a bootstrap-time check in `AdministratorService.ensureSuperAdminExists` that inspects `authOptions.superadminCredentials` against the well-known defaults from `@vendure/common`. The check: - throws (refusing to start) when `NODE_ENV === 'production'` and either the identifier or the password matches the default; - logs a `Logger.warn` with remediation hints in development / staging / any other non-test environment; - stays silent in `NODE_ENV === 'test'` to keep the test runner output clean (the e2e suite uses the defaults by design). The check function is extracted to `helpers/utils/check-superadmin-credentials.ts` and covered by unit tests. Existing 819 core unit tests still pass; no schema or public-API change. Reported privately via responsible disclosure (Linear OSS-490).
…sword Address review feedback: the well-known superadmin identifier is fine to use as long as the password is non-default, so the bootstrap guard now checks only the password against the default. Refusing to start in production on a default password is a behavioural change, hence targeted at the minor branch. Relates to OSS-490
e4eaad6 to
d56c118
Compare
|
|
|
all done |
Summary
Vendure ships with the well-known defaults
superadmin/superadminfor the initial admin user and has no built-in guard against running with them in production. This is a hard-coded-credentials issue (CWE-798): anyone with network access to a vanilla 3.6.x deployment can log in as superadmin if the operator did not overrideauthOptions.superadminCredentials.This PR adds a bootstrap-time check that:
NODE_ENV === 'production'and the password matches the default constant from@vendure/common. The defaultsuperadminidentifier is intentionally allowed — only a default password is treated as insecure.Logger.warnwith remediation hints in development / staging / any other non-test environment.NODE_ENV === 'test'so the existing e2e suite (which uses the defaults by design) keeps a clean output.The remediation hint points operators at wiring
superadminCredentialsfrom environment variables, which is the same pattern most starters already use.Scope
@vendure/commonare kept as-is — they are still exported because plugins and the e2e/testing layer rely on them.majorchange. This PR is the minimum-blast-radius patch that closes the critical concern.minor: refusing to start in production on a default password is a behavioural change, so it ships in a minor release rather than a patch.Changes
packages/core/src/service/helpers/utils/check-superadmin-credentials.tscontaining the pure check (testable, no DI).AdministratorService.ensureSuperAdminExistsnow calls the helper before creating / reconciling the superadmin user.Test plan
vitest run service/helpers/utils/check-superadmin-credentials.spec.ts— 7/7 pass@vendure/coreunit suite (vitest run) — 819/819 pass; no existing tests regressedLogger.warnline about the default password, server starts normallyNODE_ENV=productionand default password → expect bootstrap to throw[Vendure] Refusing to start: Default superadmin password is configured…NODE_ENV=productionand explicit override → silent, server starts normallyNotes
superadminidentifier with a strong, non-default password is fully supported (per maintainer review). The guard cannot detect weak-but-custom passwords — enforcing "non-default" is the enforceable boundary; "strong" remains the operator's responsibility.Need help on this PR? Tag
@codesmithwith what you need.