Conversation
Adds the JUMPSTARTER_FORCE_SYSTEM_CERTS env variable to still allow importing the system certificate store in MacOS Fixes-Issue: #362
WalkthroughThe changes introduce a new environment variable, Changes
Sequence Diagram(s)sequenceDiagram
participant OIDC as OIDC Module
participant OS as Operating System
participant TS as Truststore
OIDC->>OS: Determine OS type (e.g., "Darwin"?)
OS-->>OIDC: Return OS info
alt Non-Darwin or forced system certs enabled
OIDC->>TS: Call inject_into_ssl()
else MacOS without forced flag
OIDC-->>TS: Skip certificate injection
end
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
mangelajo
requested review from
NickCao and
kirkbrauer
and removed request for
NickCao
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/source/config/cli.md (1)
137-138: Fix capitalization of "macOS"The documentation for the new environment variable looks good, but there's a minor issue with the capitalization of "macOS" (Apple's preferred spelling).
- which is the behavior by default for all systems but MacOS (see [bug](https://github.com/jumpstarter-dev/jumpstarter/issues/362)) + which is the behavior by default for all systems but macOS (see [bug](https://github.com/jumpstarter-dev/jumpstarter/issues/362))🧰 Tools
🪛 LanguageTool
[grammar] ~138-~138: The operating system from Apple is written “macOS”.
Context: ...behavior by default for all systems but MacOS (see [bug](https://github.com/jumpstart...(MAC_OS)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/config/cli.md(1 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/source/config/cli.md
[grammar] ~138-~138: The operating system from Apple is written “macOS”.
Context: ...behavior by default for all systems but MacOS (see [bug](https://github.com/jumpstart...
(MAC_OS)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build
- GitHub Check: pytest-matrix (3.13)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-devspace .devfile/Containerfile.client)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-utils Dockerfile.utils)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter Dockerfile)
🔇 Additional comments (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (2)
2-2: LGTM - Added import for os moduleThe import for
osmodule is correctly added to support the operating system detection and environment variable access.
16-21:Details
✅ Verification successful
Proper workaround for macOS certificate issue
The conditional check properly prevents system certificate injection on macOS unless explicitly requested through the environment variable. This effectively addresses the issue #362 while maintaining backward compatibility for other platforms.
🏁 Script executed:
#!/bin/bash # Verify that the Darwin check is consistent with other platform checks in the codebase rg -i "darwin|macos|os\.uname\(\)\.sysname" --type pyLength of output: 349
Verified Darwin Check Successfully Consistent
The check in
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pycorrectly uses the conditionif os.uname().sysname != "Darwin" or os.environ.get("JUMPSTARTER_FORCE_SYSTEM_CERTS") == "1":to prevent the injection of system certificates on macOS unless explicitly forced via the environment variable. The search output confirms that this pattern is unique to this location and aligns with expected behavior to address issue #362 while preserving compatibility across other platforms.
2 participants
Adds the JUMPSTARTER_FORCE_SYSTEM_CERTS env variable to still allow importing the
system certificate store in MacOS
Fixes-Issue: #362
Summary by CodeRabbit
JUMPSTARTER_FORCE_SYSTEM_CERTS, that allows users to enforce the use of system CA certificates even on systems (like macOS) where this is not the default behavior.