From 1a704d3c7d95ccfd4bb3b5ea9513f23014d604e2 Mon Sep 17 00:00:00 2001
From: Ed Morley <501702+edmorley@users.noreply.github.com>
Date: Tue, 16 Apr 2024 15:40:27 +0000
Subject: [PATCH] Exclude `LD_LIBRARY_PATH` and `PYTHONHOME` when invoking
 subprocesses (#1565)

During the build, the buildpack makes the config vars set on the app
available to certain subprocesses (such as Django collectstatic) via the
`sub_env` utility function. This function filters out env vars that
might cause the subprocess to fail. (Note: This filtering only affects
app config vars, and not the env vars provided by buildpacks that run
prior to the Python buildpack.)

This change adds `LD_LIBRARY_PATH` and `PYTHONHOME` to the list of env
vars that are filtered out, to prevent errors when they are set to
invalid values.

In particular, very old versions of the buildpack used to set these env
vars as actual app config vars (via the `bin/release` script), to values
that no longer work:
https://github.com/heroku/heroku-buildpack-python/blob/27abdfe7d7ad104dabceb45641415251e965671c/bin/release#L11-L18

These broken app config vars have not typically caused problems since:
1. Only Python apps created in 2012 or earlier will have them (unless
   someone manually sets them on an app)
2. Static builds of Python don't rely upon `LD_LIBRARY_PATH`

However, as of Python 3.10 we switched to building in shared mode (see
#1320), and so apps with broken config vars will otherwise see errors
like the following once they upgrade Python versions:

```
python: error while loading shared libraries: libpython3.10.so.1.0: cannot open shared object file: No such file or directory
```

As seen in:
https://heroku.support/1365030

The `GIT_DIR` env var was removed from the filter list, since there is
no need to filter it out, since it's no longer set by the build system,
see #1120.

(The CNB isn't affected by this issue, and already has a test to confirm that.)

GUS-W-15519103.
---
 .rubocop.yml                           |  6 ++++++
 CHANGELOG.md                           |  1 +
 spec/hatchet/django_spec.rb            | 15 ++++++++++++++-
 spec/hatchet/profile_d_scripts_spec.rb |  2 +-
 vendor/buildpack-stdlib_v8.sh          |  2 +-
 5 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 47e95a35f..44a322aef 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -26,3 +26,9 @@ RSpec/Focus:
 
 RSpec/MultipleExpectations:
   Enabled: false
+
+Style/TrailingCommaInArrayLiteral:
+  EnforcedStyleForMultiline: consistent_comma
+
+Style/TrailingCommaInHashLiteral:
+  EnforcedStyleForMultiline: consistent_comma
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0ff40bbcc..a4908ec2a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Excluded `LD_LIBRARY_PATH` and `PYTHONHOME` app config vars when invoking subprocesses during the build. ([#1565](https://github.com/heroku/heroku-buildpack-python/pull/1565))
 
 ## [v248] - 2024-04-09
 
diff --git a/spec/hatchet/django_spec.rb b/spec/hatchet/django_spec.rb
index c128c0673..865d5b833 100644
--- a/spec/hatchet/django_spec.rb
+++ b/spec/hatchet/django_spec.rb
@@ -2,10 +2,23 @@
 
 require_relative '../spec_helper'
 
+# Tests that broken user-provided env vars don't take precedence over those set by this buildpack
+# and break running Python. This is particularly important when using shared builds of Python,
+# since they rely upon `LD_LIBRARY_PATH` being correct. This list of env vars is based on those
+# that used to be set to different values by `bin/release` in very old versions of the buildpack:
+# https://github.com/heroku/heroku-buildpack-python/blob/27abdfe7d7ad104dabceb45641415251e965671c/bin/release#L11-L18
+BROKEN_CONFIG_VARS = {
+  LD_LIBRARY_PATH: '/invalid-path',
+  LIBRARY_PATH: '/invalid-path',
+  PATH: '/invalid-path',
+  PYTHONHOME: '/invalid-path',
+  PYTHONPATH: '/invalid-path',
+}.freeze
+
 RSpec.describe 'Django support' do
   describe 'collectstatic' do
     context 'when building a Django project' do
-      let(:app) { Hatchet::Runner.new('python-getting-started') }
+      let(:app) { Hatchet::Runner.new('python-getting-started', config: BROKEN_CONFIG_VARS) }
 
       it 'runs collectstatic' do
         app.deploy do |app|
diff --git a/spec/hatchet/profile_d_scripts_spec.rb b/spec/hatchet/profile_d_scripts_spec.rb
index 38889a849..434d6ccad 100644
--- a/spec/hatchet/profile_d_scripts_spec.rb
+++ b/spec/hatchet/profile_d_scripts_spec.rb
@@ -46,7 +46,7 @@
         'PYTHONHOME=/this-should-be-overridden',
         'PYTHONPATH=/this-should-be-preserved',
         'PYTHONUNBUFFERED=this-should-be-overridden',
-        'WEB_CONCURRENCY=this-should-be-preserved'
+        'WEB_CONCURRENCY=this-should-be-preserved',
       ]
       app.run_multi(list_envs_cmd, heroku: { env: user_env_vars.join(';'), type: 'example-worker' }) do |output, _|
         expect(output).to eq(<<~OUTPUT)
diff --git a/vendor/buildpack-stdlib_v8.sh b/vendor/buildpack-stdlib_v8.sh
index afb6580ed..ca7db3a97 100755
--- a/vendor/buildpack-stdlib_v8.sh
+++ b/vendor/buildpack-stdlib_v8.sh
@@ -95,7 +95,7 @@ _env_blacklist() {
   if [ -n "$regex" ]; then
     regex="|$regex"
   fi
-  echo "^(PATH|GIT_DIR|CPATH|CPPATH|LD_PRELOAD|LIBRARY_PATH$regex)$"
+  echo "^(PATH|CPATH|CPPATH|LD_PRELOAD|LIBRARY_PATH|LD_LIBRARY_PATH|PYTHONHOME$regex)$"
 }
 
 # Usage: $ export-env ENV_DIR WHITELIST BLACKLIST