OpenJobDescription / openjd-sessions-for-python

Provides a Python library that can be used to build a runtime that is able to run Jobs in a Session as defined by Open Job Description.
https://github.com/OpenJobDescription/openjd-specifications/wiki
Apache License 2.0
10 stars 12 forks source link

fix: upper case all env vars on Windows #161

Closed edwards-aws closed 2 months ago

edwards-aws commented 2 months ago

Fixes: https://github.com/OpenJobDescription/openjd-sessions-for-python/issues/157

What was the problem/requirement? (What/Why)

We've observed on Windows systems that even though environment variables are case insensitive, the win32 API will allow duplicate environment variables with different casing. This causes issues as openjd reads in user environment variables via Python, which uppercases all environment variables it reads in on Windows. If there already existed an environment variable in the system that wasn't uppercased, say Path, and openjd reads in a variable it needs to update, say PATH. The win32 API will accept both. This leads to undefined behaviour when Windows accesses that environment variable

What was the solution? (How)

Upper case all environment variables that come into openjd on Windows, whether it's through the win32 API or openjd.

What is the impact of this change?

Paths added by printing openjd_env: key=value should override environment variables as expected, regardless of casing.

How was this change tested?

I've added unit tests that ensure that the environment variables are all converted to upper case as expected and that variables are overwritten as expected regardless of casing.

Was this change documented?

N/A, but I've also added some dev documentation to make running the impersonation tests on Windows easier.

Is this a breaking change?

N/A

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

edwards-aws commented 2 months ago

Thanks for pointing that out Stephen! I've updated the PR text!

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud