Skip to content

Commit 2e52618

Browse files
authored
Improve the forbidden env vars check (#542)
- Now reports all forbidden env vars found in one go, rather than only the first found. - Now checks for known problematic Poetry env vars too. - Uses a const slice instead of a fixed size array to avoid having to manually update the array every time entries are added/removed. - Uses a static str slice in the error to avoid an unnecessary string allocation. - Adds additional unit test coverage. GUS-W-21914465.
1 parent ee48e58 commit 2e52618

4 files changed

Lines changed: 64 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- The forbidden env vars check now reports all forbidden env vars found rather than only the first. ([#542](https://github.com/heroku/buildpacks-python/pull/542))
13+
- The forbidden env vars check now checks for known problematic Poetry env vars too. ([#542](https://github.com/heroku/buildpacks-python/pull/542))
14+
1015
## [6.2.0] - 2026-04-02
1116

1217
### Changed

src/checks.rs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,20 @@ use libcnb::Env;
77
// https://docs.python.org/3/using/cmdline.html#environment-variables
88
// https://pip.pypa.io/en/stable/cli/pip/#general-options
99
// https://pip.pypa.io/en/stable/cli/pip_install/#options
10-
// https://docs.astral.sh/uv/configuration/environment/
11-
const FORBIDDEN_ENV_VARS: [&str; 22] = [
10+
// https://python-poetry.org/docs/configuration/
11+
// https://docs.astral.sh/uv/reference/environment/
12+
const FORBIDDEN_ENV_VARS: &[&str] = &[
1213
"PIP_CACHE_DIR",
1314
"PIP_PREFIX",
1415
"PIP_PYTHON",
1516
"PIP_ROOT",
1617
"PIP_TARGET",
1718
"PIP_USER",
19+
"POETRY_CACHE_DIR",
20+
"POETRY_DATA_DIR",
21+
"POETRY_HOME",
22+
"POETRY_VIRTUALENVS_CREATE",
23+
"POETRY_VIRTUALENVS_USE_POETRY_PYTHON",
1824
"PYTHONHOME",
1925
"PYTHONINSPECT",
2026
"PYTHONNOUSERSITE",
@@ -34,11 +40,14 @@ const FORBIDDEN_ENV_VARS: [&str; 22] = [
3440
];
3541

3642
pub(crate) fn check_environment(env: &Env) -> Result<(), ChecksError> {
37-
if let Some(&name) = FORBIDDEN_ENV_VARS
43+
let forbidden_vars_found: Vec<&'static str> = FORBIDDEN_ENV_VARS
3844
.iter()
39-
.find(|&name| env.contains_key(name))
40-
{
41-
return Err(ChecksError::ForbiddenEnvVar(name.to_string()));
45+
.copied()
46+
.filter(|&name| env.contains_key(name))
47+
.collect();
48+
49+
if !forbidden_vars_found.is_empty() {
50+
return Err(ChecksError::ForbiddenEnvVars(forbidden_vars_found));
4251
}
4352

4453
Ok(())
@@ -47,7 +56,7 @@ pub(crate) fn check_environment(env: &Env) -> Result<(), ChecksError> {
4756
/// Errors due to one of the environment checks failing.
4857
#[derive(Debug, PartialEq)]
4958
pub(crate) enum ChecksError {
50-
ForbiddenEnvVar(String),
59+
ForbiddenEnvVars(Vec<&'static str>),
5160
}
5261

5362
#[cfg(test)]
@@ -56,9 +65,12 @@ mod tests {
5665

5766
#[test]
5867
fn check_environment_valid() {
68+
assert_eq!(check_environment(&Env::new()), Ok(()));
5969
let mut env = Env::new();
60-
env.insert("PYTHONPATH", "/example");
6170
env.insert("PIP_EXTRA_INDEX_URL", "https://example.tld/simple");
71+
env.insert("POETRY_FOO", "example");
72+
env.insert("PYTHONPATH", "/example");
73+
env.insert("UV_FOO", "example");
6274
assert_eq!(check_environment(&env), Ok(()));
6375
}
6476

@@ -68,7 +80,21 @@ mod tests {
6880
env.insert("PYTHONHOME", "/example");
6981
assert_eq!(
7082
check_environment(&env),
71-
Err(ChecksError::ForbiddenEnvVar("PYTHONHOME".to_string()))
83+
Err(ChecksError::ForbiddenEnvVars(vec!["PYTHONHOME"]))
84+
);
85+
env.insert("PIP_PYTHON", "/example");
86+
env.insert("POETRY_HOME", "/example");
87+
env.insert("VIRTUAL_ENV", "/example");
88+
env.insert("UV_PYTHON", "/example");
89+
assert_eq!(
90+
check_environment(&env),
91+
Err(ChecksError::ForbiddenEnvVars(vec![
92+
"PIP_PYTHON",
93+
"POETRY_HOME",
94+
"PYTHONHOME",
95+
"UV_PYTHON",
96+
"VIRTUAL_ENV",
97+
]))
7298
);
7399
}
74100
}

src/errors.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,21 @@ fn on_buildpack_error(error: BuildpackError) {
6262

6363
fn on_buildpack_checks_error(error: ChecksError) {
6464
match error {
65-
ChecksError::ForbiddenEnvVar(name) => log_error(
66-
"Unsafe environment variable found",
67-
formatdoc! {"
68-
The environment variable `{name}` is set, however, it can
69-
cause problems with the build so we don't allow using it.
65+
ChecksError::ForbiddenEnvVars(env_vars) => {
66+
let env_vars_list = env_vars.join("\n");
67+
log_error(
68+
"Unsafe environment variable(s) found",
69+
formatdoc! {"
70+
The following environment variable(s) can cause problems
71+
with the build so we don't allow using them:
7072
71-
You must unset that environment variable. If you didn't set it
72-
yourself, check that it wasn't set by an earlier buildpack.
73-
"},
74-
),
73+
{env_vars_list}
74+
75+
You must unset the above env var(s). If you didn't set them
76+
yourself, check if they were set by an earlier buildpack.
77+
"},
78+
);
79+
}
7580
}
7681
}
7782

tests/checks_test.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,25 @@ use libcnb_test::{PackResult, TestRunner, assert_contains};
44

55
#[test]
66
#[ignore = "integration test"]
7-
fn checks_reject_pythonhome_env_var() {
7+
fn checks_reject_forbidden_env_vars() {
88
let mut config = default_build_config("tests/fixtures/pyproject_toml_only");
99
config.env("PYTHONHOME", "/invalid");
10+
config.env("VIRTUAL_ENV", "/invalid");
1011
config.expected_pack_result(PackResult::Failure);
1112

1213
TestRunner::default().build(config, |context| {
1314
assert_contains!(
1415
context.pack_stdout,
1516
indoc! {"
16-
[Error: Unsafe environment variable found]
17-
The environment variable `PYTHONHOME` is set, however, it can
18-
cause problems with the build so we don't allow using it.
17+
[Error: Unsafe environment variable(s) found]
18+
The following environment variable(s) can cause problems
19+
with the build so we don't allow using them:
1920
20-
You must unset that environment variable. If you didn't set it
21-
yourself, check that it wasn't set by an earlier buildpack.
21+
PYTHONHOME
22+
VIRTUAL_ENV
23+
24+
You must unset the above env var(s). If you didn't set them
25+
yourself, check if they were set by an earlier buildpack.
2226
2327
ERROR: failed to build: exit status 1
2428
"}

0 commit comments

Comments
 (0)