Fix: OVH environment variables#4
Conversation
📝 WalkthroughWalkthroughThis PR updates OVH configuration variable naming to use NUXT-prefixed keys across example and Docker Compose configuration, switches the Nuxt runtime configuration from environment-variable-dependent to hardcoded empty defaults, and increments the package version to 1.1.2. ChangesOVH Configuration Consolidation and Runtime Hardcoding
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nuxt.config.ts`:
- Line 29: The runtime config key databaseUrl in nuxt.config.ts is inconsistent
with other env renames; update all environment references to use
NUXT_DATABASE_URL for consistency: change the env var names in .env.example and
docker-compose.yml from DATABASE_URL to NUXT_DATABASE_URL, and ensure
lib/prisma.ts (where it currently falls back to process.env.DATABASE_URL) reads
process.env.NUXT_DATABASE_URL (or checks it first) so the code and config
consistently use NUXT_DATABASE_URL instead of DATABASE_URL.
- Around line 29-32: The ovhAllowedNics value can be a CSV string when
overridden via env, causing incorrect whitelist checks; update the code that
reads/uses ovhAllowedNics (reference config.ovhAllowedNics and the whitelist
logic in server/api/auth/ovh-callback.get.ts) to normalize the value into a true
string[]: if it's a string, split on commas, trim entries and filter out
empties; if it's already an array, ensure all entries are strings (map
toString/trim). Use this normalized array for the .includes(userNic) check so
element membership is correct regardless of env deserialization format.
In `@package.json`:
- Line 3: The package bump to "1.1.2" was done while nuxt.config.ts now
hardcodes runtimeConfig defaults (databaseUrl, ovhAppKey, ovhAppSecret,
ovhAllowedNics) and removed references to old env vars (OVH_APPLICATION_KEY,
OVH_APPLICATION_SECRET, OVH_CONSUMER_KEY, OVH_SERVICE_NAME), causing a breaking
change; either (A) update package.json version to an appropriate MAJOR or MINOR
per semver for a breaking change, or (B) restore backwards compatibility by
updating nuxt.config.ts to read existing environment variables as fallbacks
(e.g., check process.env.OVH_APPLICATION_KEY etc. when setting runtimeConfig
ovhAppKey/ovhAppSecret/ovhAllowedNics and consumer/service names) and add
migration/deprecation notes to the changelog explaining the env var rename and
expected timeline so users aren’t surprised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20debe2e-3fa3-429f-99be-1b0da3d4bfb7
📒 Files selected for processing (4)
.env.exampledocker-compose.ymlnuxt.config.tspackage.json
| ovhAppKey: process.env.OVH_APP_KEY, | ||
| ovhAppSecret: process.env.OVH_APP_SECRET, | ||
| ovhAllowedNics: (process.env.NUXT_OVH_ALLOWED_NIC_HANDLES || '').split(','), | ||
| databaseUrl: '', |
There was a problem hiding this comment.
DATABASE_URL not renamed to NUXT_DATABASE_URL for consistency.
The databaseUrl runtime config field is set to an empty string, which means it expects the NUXT_DATABASE_URL environment variable to override it (following Nuxt's automatic override convention). However, .env.example (line 14) and docker-compose.yml (line 63) still use DATABASE_URL without the NUXT_ prefix.
While this may still work due to the fallback in lib/prisma.ts:5-7 (let url = process.env.DATABASE_URL), it's inconsistent with the rest of this PR which renamed all OVH variables from OVH_* to NUXT_OVH_*.
📝 Suggested fix for consistency
Option 1 (Recommended): Rename DATABASE_URL to NUXT_DATABASE_URL in .env.example and docker-compose.yml:
In .env.example:
-DATABASE_URL="postgresql://grhb_user:grhb_password@localhost:5432/grhb_db?schema=public"
+NUXT_DATABASE_URL="postgresql://grhb_user:grhb_password@localhost:5432/grhb_db?schema=public"In docker-compose.yml:
- DATABASE_URL: postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}`@db`:5432/${POSTGRES_DB}?schema=public
+ NUXT_DATABASE_URL: postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}`@db`:5432/${POSTGRES_DB}?schema=publicOption 2: Restore explicit process.env.DATABASE_URL reading if you want to keep the DATABASE_URL convention:
- databaseUrl: '',
+ databaseUrl: process.env.DATABASE_URL || '',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| databaseUrl: '', | |
| databaseUrl: process.env.DATABASE_URL || '', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nuxt.config.ts` at line 29, The runtime config key databaseUrl in
nuxt.config.ts is inconsistent with other env renames; update all environment
references to use NUXT_DATABASE_URL for consistency: change the env var names in
.env.example and docker-compose.yml from DATABASE_URL to NUXT_DATABASE_URL, and
ensure lib/prisma.ts (where it currently falls back to process.env.DATABASE_URL)
reads process.env.NUXT_DATABASE_URL (or checks it first) so the code and config
consistently use NUXT_DATABASE_URL instead of DATABASE_URL.
| databaseUrl: '', | ||
| ovhAppKey: '', | ||
| ovhAppSecret: '', | ||
| ovhAllowedNics: [], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how Nuxt parses comma-separated vs JSON array environment variables
# Search for how ovhAllowedNics is consumed in the codebase
rg -n -C3 'ovhAllowedNics' --type=ts
# Check if there's any custom parsing logic we might have missed
ast-grep --pattern 'ovhAllowedNics'Repository: baptiste-may/GRHB
Length of output: 885
Critical: ovhAllowedNics is treated as a string[] but can be deserialized into the wrong type from env overrides.
server/api/auth/ovh-callback.get.ts casts config.ovhAllowedNics to string[] and uses .includes(userNic) for the whitelist check. With nuxt.config.ts setting ovhAllowedNics: [] and relying on Nuxt env override/deserialization, a comma-separated env value (e.g. NUXT_OVH_ALLOWED_NICS="a,b") may not become an actual array—making the whitelist check behave incorrectly (substring match instead of element match, or other mismatches).
🔧 Proposed fix: Split CSV explicitly (or require JSON array format)
runtimeConfig: {
databaseUrl: '',
ovhAppKey: '',
ovhAppSecret: '',
- ovhAllowedNics: [],
+ ovhAllowedNics: (process.env.NUXT_OVH_ALLOWED_NICS || '').split(',').filter(Boolean),
},-NUXT_OVH_ALLOWED_NICS="handle1-ovh,handle2-ovh"
+NUXT_OVH_ALLOWED_NICS=["handle1-ovh","handle2-ovh"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| databaseUrl: '', | |
| ovhAppKey: '', | |
| ovhAppSecret: '', | |
| ovhAllowedNics: [], | |
| databaseUrl: '', | |
| ovhAppKey: '', | |
| ovhAppSecret: '', | |
| ovhAllowedNics: (process.env.NUXT_OVH_ALLOWED_NICS || '').split(',').filter(Boolean), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nuxt.config.ts` around lines 29 - 32, The ovhAllowedNics value can be a CSV
string when overridden via env, causing incorrect whitelist checks; update the
code that reads/uses ovhAllowedNics (reference config.ovhAllowedNics and the
whitelist logic in server/api/auth/ovh-callback.get.ts) to normalize the value
into a true string[]: if it's a string, split on commas, trim entries and filter
out empties; if it's already an array, ensure all entries are strings (map
toString/trim). Use this normalized array for the .includes(userNic) check so
element membership is correct regardless of env deserialization format.
| { | ||
| "name": "grhb", | ||
| "version": "1.1.1", | ||
| "version": "1.1.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for migration docs, changelog, and backwards compatibility for env var changes
# Check for changelog or migration docs
echo "=== Checking for CHANGELOG or migration documentation ==="
fd -i 'changelog|migration|upgrade' -e md -e txt
# Check nuxt.config.ts for backwards compatibility (e.g., fallback to old env var names)
echo -e "\n=== Checking nuxt.config.ts for environment variable handling ==="
rg -n -A5 -B5 'runtimeConfig' nuxt.config.ts
# Check if old env var names are still referenced anywhere (backwards compatibility)
echo -e "\n=== Checking for references to old (non-NUXT-prefixed) OVH env var names ==="
rg -n 'OVH_APPLICATION_KEY|OVH_APPLICATION_SECRET|OVH_CONSUMER_KEY|OVH_SERVICE_NAME' --type-add 'config:*.{ts,js,yml,yaml,env,example}' --type config
# Check for any deprecation notices or migration guides in comments
echo -e "\n=== Checking for deprecation notices in code comments ==="
rg -n -i 'deprecat|migrat|breaking' --type ts --type mdRepository: baptiste-may/GRHB
Length of output: 789
PATCH bump likely incorrect (breaking runtimeConfig + env var rename)
nuxt.config.tshardcodesruntimeConfigto empty defaults (databaseUrl,ovhAppKey,ovhAppSecret,ovhAllowedNics), so the app no longer reads OVH values from environment variables.- Old env var names (
OVH_APPLICATION_KEY,OVH_APPLICATION_SECRET,OVH_CONSUMER_KEY,OVH_SERVICE_NAME) are not referenced anywhere, indicating no backwards-compatible fallback. - No CHANGELOG/migration/deprecation notes found.
If this is intentionally breaking, 1.1.1 → 1.1.2 is the wrong semver bump; update to a MINOR/MAJOR that reflects the breaking change and/or add migration/deprecation support.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 3, The package bump to "1.1.2" was done while
nuxt.config.ts now hardcodes runtimeConfig defaults (databaseUrl, ovhAppKey,
ovhAppSecret, ovhAllowedNics) and removed references to old env vars
(OVH_APPLICATION_KEY, OVH_APPLICATION_SECRET, OVH_CONSUMER_KEY,
OVH_SERVICE_NAME), causing a breaking change; either (A) update package.json
version to an appropriate MAJOR or MINOR per semver for a breaking change, or
(B) restore backwards compatibility by updating nuxt.config.ts to read existing
environment variables as fallbacks (e.g., check process.env.OVH_APPLICATION_KEY
etc. when setting runtimeConfig ovhAppKey/ovhAppSecret/ovhAllowedNics and
consumer/service names) and add migration/deprecation notes to the changelog
explaining the env var rename and expected timeline so users aren’t surprised.
1 participant
Summary by CodeRabbit