Skip to content

feat(sandbox): add Rscript support to environment configuration #490

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.

Sign up for GitHub

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

Closed
zieen wants to merge 1 commit into from
Closed

feat(sandbox): add Rscript support to environment configuration #490

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions crates/openshell-sandbox/src/child_env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub(crate) fn proxy_env_vars(proxy_url: &str) -> [(&'static str, String); 9] {
("grpc_proxy", proxy_url.to_owned()),
// Node.js only honors HTTP(S)_PROXY for built-in fetch/http clients when
// proxy support is explicitly enabled at process startup.
// Rscript automatically respects lowercase http_proxy/https_proxy.
("NODE_USE_ENV_PROXY", "1".to_owned()),
Comment on lines 18 to 21
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Rscript note is placed immediately above NODE_USE_ENV_PROXY, which could imply it relates to that setting. Since it documents the lowercase http_proxy/https_proxy entries, consider moving this comment next to those env var entries (or splitting into separate comments for Node vs. lowercase proxies) to keep the rationale aligned with the relevant keys.

Copilot uses AI. Check for mistakes.
]
}
Expand All @@ -28,9 +29,13 @@ pub(crate) fn tls_env_vars(
let ca_cert_path = ca_cert_path.display().to_string();
let combined_bundle_path = combined_bundle_path.display().to_string();
[
// Node.js extra CA certificates for TLS verification
("NODE_EXTRA_CA_CERTS", ca_cert_path.clone()),
// OpenSSL-based tools (Python urllib, Ruby, R, etc.)
("SSL_CERT_FILE", combined_bundle_path.clone()),
// Python requests library
("REQUESTS_CA_BUNDLE", combined_bundle_path.clone()),
// libcurl-based tools (R httr, curl CLI, etc.)
("CURL_CA_BUNDLE", combined_bundle_path),
]
}
Expand Down Expand Up @@ -59,6 +64,9 @@ mod tests {
assert!(stdout.contains("NO_PROXY=127.0.0.1,localhost,::1"));
assert!(stdout.contains("NODE_USE_ENV_PROXY=1"));
assert!(stdout.contains("no_proxy=127.0.0.1,localhost,::1"));
// Verify lowercase proxy vars for Rscript/Python compatibility
assert!(stdout.contains("http_proxy=http://10.200.0.1:3128"));
assert!(stdout.contains("https_proxy=http://10.200.0.1:3128"));
}

#[test]
Expand All @@ -79,5 +87,7 @@ mod tests {

assert!(stdout.contains("NODE_EXTRA_CA_CERTS=/etc/openshell-tls/openshell-ca.pem"));
assert!(stdout.contains("SSL_CERT_FILE=/etc/openshell-tls/ca-bundle.pem"));
// Verify CURL_CA_BUNDLE for Rscript (httr package) and curl CLI
assert!(stdout.contains("CURL_CA_BUNDLE=/etc/openshell-tls/ca-bundle.pem"));
}
}
Loading