feat(core): read auth configuration from environment variables#108
Merged
halvaradop merged 4 commits intomasterfrom Mar 1, 2026
Merged
feat(core): read auth configuration from environment variables#108halvaradop merged 4 commits intomasterfrom
halvaradop merged 4 commits intomasterfrom
Conversation
Support configuration via: - AUTH_DEBUG - AUTH_LOG_LEVEL - SALT - SECRET - TRUSTED_ORIGINS - TRUSTED_PROXY_HEADERS
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (4)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. 📝 WalkthroughWalkthroughThis change introduces environment-aware proxy logging and standardized environment variable resolution. It adds public helper functions for reading environment variables (getEnv, getEnvBoolean, getEnvArray), updates the logger interface to accept boolean values, replaces internal logging with environment-driven proxy logging, and refactors configuration consumers to use the new environment helpers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
packages/core/src/jose.ts (1)
24-37: Consider updating error messages to reflect all checked patterns.The error messages reference specific env var names (
AURA_AUTH_SECRET,AURA_AUTH_SALT or AUTH_SALT), butgetEnvnow checks multiple patterns (e.g.,AURA_AUTH_SECRET,AURA_SECRET,AUTH_SECRET,SECRET). Consider updating the messages to list all valid options or use a generic phrasing like "No secret/salt found in environment variables."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/jose.ts` around lines 24 - 37, Update the thrown AuthInternalError messages in the jose initialization to reflect all env var patterns checked by getEnv: when secret is missing (the variable referenced as secret and set via getEnv) change the message to list all acceptable secret env names or use a generic "No secret found in environment variables" phrasing; do the same for the salt check (salt variable obtained via getEnv) so the AuthInternalError for JOSE_INITIALIZATION_FAILED accurately references all salt env options or uses a generic "No salt found in environment variables." Ensure you update the error strings passed to AuthInternalError where secret and salt are validated.packages/core/test/env.test.ts (1)
5-29: Consider expanding test coverage.The test suite only covers the
envproxy directly. Consider adding tests for the newgetEnv,getEnvBoolean, andgetEnvArrayhelpers introduced inenv.tsto validate the prioritized key resolution and parsing behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/env.test.ts` around lines 5 - 29, Add unit tests for the new helpers getEnv, getEnvBoolean, and getEnvArray to validate prioritized key resolution and parsing behavior: write tests that use vi.stubEnv to set fallback and primary keys and assert getEnv returns primary when present and fallback when primary missing (reference getEnv), tests for getEnvBoolean that verify common truthy/falsey string values ("true","false","1","0","yes","no") and default behavior when unset, and tests for getEnvArray that verify splitting on commas, trimming whitespace, handling empty items, and returning an empty array or default when unset (reference getEnvBoolean and getEnvArray to locate code paths); use afterEach vi.unstubAllEnvs like existing tests to isolate env changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/env.ts`:
- Around line 48-59: getEnvArray's signature and fallbacks are incorrect: change
the generic function getEnvArray to return string[] (not <T>), type the
defaultValue as string[] = [], and simplify the body to call getEnv(key) and if
falsy return defaultValue; otherwise split on /[,;\n]+/, trim and
filter(Boolean) and return that array (remove the redundant "?? defaultValue ??
[]" chain). Keep references to getEnvArray and getEnv when making the edits.
In `@packages/core/src/index.ts`:
- Line 33: The logger is created with createProxyLogger(authConfig) but the
logger helper currently reads DEBUG/LOG_LEVEL instead of the PR-intended
AUTH_DEBUG/AUTH_LOG_LEVEL; update the logger helper
(packages/core/src/logger.ts) to read AUTH_DEBUG and AUTH_LOG_LEVEL and fall
back to DEBUG/LOG_LEVEL only if the AUTH_* vars are absent, or change
createProxyLogger usage to pass the correct env keys from authConfig; ensure
functions createProxyLogger and any internal functions that read process.env use
AUTH_DEBUG and AUTH_LOG_LEVEL as the primary sources so
createProxyLogger(authConfig) aligns with the PR contract.
- Line 32: The expression setting useProxyHeaders incorrectly treats an explicit
false from getEnvBoolean("TRUSTED_PROXY_HEADERS") as unset, so env cannot
disable authConfig.trustedProxyHeaders; update the assignment for
useProxyHeaders to prefer the environment value when present (including false)
by checking for presence/definedness of getEnvBoolean("TRUSTED_PROXY_HEADERS")
rather than using ||, e.g. retrieve the env value once via
getEnvBoolean("TRUSTED_PROXY_HEADERS") and if it is !== undefined use it,
otherwise fall back to authConfig?.trustedProxyHeaders or false; locate the
logic around getEnvBoolean, TRUSTED_PROXY_HEADERS and
authConfig?.trustedProxyHeaders where useProxyHeaders is defined to make this
change.
In `@packages/core/src/logger.ts`:
- Around line 271-273: The logger currently sets procId using process.pid which
will throw in non-Node runtimes; change the procId assignment (the "procId"
property in the logger creation) to safely check for the existence of process
and process.pid before calling toString, e.g. use a conditional/ternary that
verifies typeof process !== "undefined" and process.pid is present and then
converts to string, otherwise use a stable fallback (null/undefined or empty
string) so the logger won't crash in Bun/Deno/Edge environments.
- Around line 303-308: The syslog message can contain the literal "undefined"
because createSyslogMessage destructures appName from SyslogOptions but
createLogEntry doesn't set it; fix this by providing a safe default for appName
(e.g., '-' or a configured fallback) when building the message in
createSyslogMessage (or ensure createLogEntry always supplies appName) so that
appName is never the string "undefined" in the returned value; update the
destructuring/handling in createSyslogMessage to use that default.
- Around line 334-344: createProxyLogger incorrectly passes a boolean false into
createLogger when debug is falsy and config.logger === false; update the return
so you only forward a non-boolean logger to createLogger. Specifically, in
createProxyLogger check the type of config.logger (e.g., typeof config?.logger
=== "object" || typeof config?.logger === "function") and pass that value to
createLogger, otherwise pass undefined (or omit the argument) so createLogger is
never called with a boolean; reference createProxyLogger and createLogger to
locate the change.
In `@packages/core/src/utils.ts`:
- Around line 158-159: The exported function getEnvironmentVariables is empty
and should not be a no-op public API; either implement its intended behavior or
remove the export (or at minimum mark it as intentionally unfinished). Update
getEnvironmentVariables to return the environment map or specific vars expected
by callers (implement logic to read process.env and filter/normalize keys) or
remove the export entirely; if you must keep a placeholder, add a clear TODO
comment and make it throw a descriptive Error like "getEnvironmentVariables not
implemented" so consumers fail fast instead of receiving undefined.
In `@packages/core/test/env.test.ts`:
- Around line 23-29: Rename the unclear test title "nose" in the test function
to a descriptive name that reflects its intent (for example, "last stubbed value
overwrites previous values"); update the call to test(...) surrounding the
assertions in env.test.ts so the first argument is the new descriptive string
while leaving the body (generateSecure, vi.stubEnv,
expect(env.AURA_AUTH_SECRET).toBe(...)) unchanged.
---
Nitpick comments:
In `@packages/core/src/jose.ts`:
- Around line 24-37: Update the thrown AuthInternalError messages in the jose
initialization to reflect all env var patterns checked by getEnv: when secret is
missing (the variable referenced as secret and set via getEnv) change the
message to list all acceptable secret env names or use a generic "No secret
found in environment variables" phrasing; do the same for the salt check (salt
variable obtained via getEnv) so the AuthInternalError for
JOSE_INITIALIZATION_FAILED accurately references all salt env options or uses a
generic "No salt found in environment variables." Ensure you update the error
strings passed to AuthInternalError where secret and salt are validated.
In `@packages/core/test/env.test.ts`:
- Around line 5-29: Add unit tests for the new helpers getEnv, getEnvBoolean,
and getEnvArray to validate prioritized key resolution and parsing behavior:
write tests that use vi.stubEnv to set fallback and primary keys and assert
getEnv returns primary when present and fallback when primary missing (reference
getEnv), tests for getEnvBoolean that verify common truthy/falsey string values
("true","false","1","0","yes","no") and default behavior when unset, and tests
for getEnvArray that verify splitting on commas, trimming whitespace, handling
empty items, and returning an empty array or default when unset (reference
getEnvBoolean and getEnvArray to locate code paths); use afterEach
vi.unstubAllEnvs like existing tests to isolate env changes.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/core/src/@types/index.tspackages/core/src/env.tspackages/core/src/index.tspackages/core/src/jose.tspackages/core/src/logger.tspackages/core/src/oauth/index.tspackages/core/src/utils.tspackages/core/test/env.test.tspackages/core/test/presets.tspackages/core/vitest.config.ts
💤 Files with no reviewable changes (1)
- packages/core/test/presets.ts
halvaradop
added
feature
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/src/content/docs/configuration/env.mdx`:
- Around line 31-35: The warning Callout text contains grammatical errors and
unclear phrasing; update the sentence to be concise and correct by stating that
environment variables override corresponding options passed to createAuth
(specifically secret, trustedOrigins, and logger). For example, rephrase to:
"Environment variables override the corresponding options passed to createAuth
(for example: secret, trustedOrigins, and logger), so setting these environment
variables will replace the values provided in createAuth." Ensure you reference
createAuth and the option names exactly as shown.
In `@packages/core/CHANGELOG.md`:
- Line 17: The changelog line for the timingSafeEqual function is malformed: add
the missing leading bullet and a verb to make it a proper entry (e.g., "- Add
timingSafeEqual function for constant-time string comparison across runtimes.
[`#99`]"). Update the entry referencing timingSafeEqual so it reads as a full
bullet-point sentence and preserves the PR link.
In `@packages/core/src/logger.ts`:
- Around line 333-343: The current createProxyLogger function ignores a provided
custom logger because the condition (debug || Boolean(config?.logger)) returns
true when config.logger exists and causes a default syslog logger to be created;
change the branch logic so the default syslog logger is only created when debug
is true or no custom logger is provided (e.g. use if (debug || !config?.logger)
{ return createLogger({ level: (level as LogLevel) ?? "debug", log:
createSyslogMessage }) } else return typeof config?.logger === "object" ?
createLogger(config.logger) : undefined), keeping references to
createProxyLogger, AuthConfig, createLogger, createSyslogMessage and
config?.logger to locate the code.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/src/content/docs/api-reference/core.mdxdocs/src/content/docs/configuration/env.mdxpackages/core/CHANGELOG.mdpackages/core/src/env.tspackages/core/src/index.tspackages/core/src/logger.tspackages/core/test/env.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/env.ts
- packages/core/test/env.test.ts
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
1 participant
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.
Description
This pull request introduces support for reading authentication configuration values from environment variables. This enhancement improves application security by reducing direct exposure of sensitive configuration values used when creating the Auth instance via the
createAuthfunction.The feature was initially motivated by loading
TRUSTED_ORIGINSfrom environment variables, but the scope was expanded to support all relevant configuration values. Aura Auth now automatically resolves environment variables following these naming patterns:AURA_AUTH_VALUEAURA_VALUEAUTH_VALUEVALUESupported Environment Variables
DEBUGLOG_LEVELSALTSECRETTRUSTED_ORIGINSTRUSTED_PROXY_HEADERSNote
Values provided via environment variables take precedence over values defined directly in the
createAuthconfiguration. Use this behavior carefully to avoid unintended overrides.Resources