Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boolean environment variables are cast to numbers #21744

Closed
ubercj opened this issue Mar 7, 2024 · 3 comments · Fixed by #22349
Closed

Boolean environment variables are cast to numbers #21744

ubercj opened this issue Mar 7, 2024 · 3 comments · Fixed by #22349

Comments

@ubercj
Copy link

ubercj commented Mar 7, 2024

Describe the Bug

Starting in version 10.9.0, if you have an environment variable that is a boolean type, it gets cast to a number when Directus parses it. I noticed this when our self-hosted instance stopped sending cors headers after upgrading from v10.8.3 to v10.9.0

I identified the casting issue when logging the parsed env in a custom endpoint extension

Screenshot 2024-03-07 at 10 56 13 AM

The culprit appears to be the guess-type.ts utility

if (value === 'true' || value === 'false') {
		return 'boolean';
	}

	if (
		String(value).startsWith('0') === false &&
		isNaN(Number(value)) === false &&
		String(value).length > 0 &&
		Number(value) <= Number.MAX_SAFE_INTEGER
	) {
		return 'number';
	}

true does not strictly equal 'true' and then Number(true) is 1, so it passes the isNaN check

The workaround I'm using for now is to deliberately use the strings 'true' or 'false' for boolean env variables

To Reproduce

Use a boolean value for an environment variable

Directus Version

v10.9.0 and above

Hosting Strategy

Self-Hosted (Docker Image)

@br41nslug
Copy link
Member

br41nslug commented Mar 7, 2024

Environment variables are historically always strings VAR=true should definitely equal "true". If this does not hold true for your local environment make sure to always use quoted strings for best portability to other systems.

Instead of relying on the "guess-type" logic have you tried explicitly casting the environment variable? VAR="string:true" or VAR="number:1" or similar

https://docs.directus.io/self-hosted/config-options.html#environment-syntax-prefix

@ubercj
Copy link
Author

ubercj commented Mar 7, 2024

I also expected them to be treated as strings, this was a surprise to me. For additional context, we are using a .cjs file for our env file. Prior to v10.9, we had something like this, which worked as we expected.

module.exports = function (env) {
  CORS_ENABLED: true,
  CORS_ORIGIN: true,
}

I can see your case that this behavior is expected. From our point of view, we changed nothing in our code except for updating the Directus version and it caused cors errors that weren't there before. It took a very long time to identify that the issue was actually due to environment variables. My concern is that this is a "breaking" change that is devilishly tricky to debug, it manifests as other issues downstream

If you don't plan on supporting boolean values in these env files like you did before v10.9, you might at least consider adding a note to the documentation that says boolean values must be provided as strings

@rijkvanzanten
Copy link
Member

Ahh gotcha! The guess type util expects every value in the env to be a string, as that's the default for .env / process.env files. If you're already using "actual" types in the env config that'll get confused!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants