Production-safety gap found during PR #13 review
Reported in: PR #13, review comment by @coderabbitai (the valid sub-concern behind an otherwise-invalid "remove the test gate" suggestion)
File: apps/node-message-broker/src/app/modules/http/module.ts
Description
The HTTP module wires authorization conditionally on the Authup client resolving:
const authupResult = container.tryResolve(AuthupClientInjectionKey);
// ...
if (authupResult.success && !isTestEnvironment) {
const gateway = new AuthupPermissionGateway({ client: authupResult.data });
mountPermissionChecker(app, gateway);
}The !isTestEnvironment gate is correct — it only skips the permission-checker override in tests to keep them network-free, and in production the gateway mounts whenever the client resolves.
The real risk is upstream: the Authup client is only registered when config.authupURL is set. If authupURL is misconfigured/unset in production, mountAuthorizationMiddleware silently falls back to fake mode (createFakeTokenVerificationData) — every request gets a synthetic identity and passes ForceLoggedInMiddleware, and the permission gateway is never mounted. The broker then runs effectively unauthenticated, silently.
Suggested fix
Validate at startup that the auth stack is configured in non-test environments — e.g. require authupURL (and the related client credentials) when config.env !== TEST, failing fast with a clear error rather than degrading to fake mode. Could live in the config validator or as an explicit guard in the HTTP module.
Production-safety gap found during PR #13 review
Reported in: PR #13, review comment by @coderabbitai (the valid sub-concern behind an otherwise-invalid "remove the test gate" suggestion)
File:
apps/node-message-broker/src/app/modules/http/module.tsDescription
The HTTP module wires authorization conditionally on the Authup client resolving:
The
!isTestEnvironmentgate is correct — it only skips the permission-checker override in tests to keep them network-free, and in production the gateway mounts whenever the client resolves.The real risk is upstream: the Authup client is only registered when
config.authupURLis set. IfauthupURLis misconfigured/unset in production,mountAuthorizationMiddlewaresilently falls back to fake mode (createFakeTokenVerificationData) — every request gets a synthetic identity and passesForceLoggedInMiddleware, and the permission gateway is never mounted. The broker then runs effectively unauthenticated, silently.Suggested fix
Validate at startup that the auth stack is configured in non-test environments — e.g. require
authupURL(and the related client credentials) whenconfig.env !== TEST, failing fast with a clear error rather than degrading to fake mode. Could live in the config validator or as an explicit guard in the HTTP module.