Skip to content

Commit

Permalink
Exclude LD_LIBRARY_PATH and PYTHONHOME when invoking subprocesses
Browse files Browse the repository at this point in the history
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 existed for a while, but have not
typically caused problems since 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 when 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.
  • Loading branch information
edmorley committed Apr 16, 2024
1 parent 4e335e1 commit f180263
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,9 @@ RSpec/Focus:

RSpec/MultipleExpectations:
Enabled: false

Style/TrailingCommaInArrayLiteral:
EnforcedStyleForMultiline: consistent_comma

Style/TrailingCommaInHashLiteral:
EnforcedStyleForMultiline: consistent_comma
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 14 additions & 1 deletion spec/hatchet/django_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
2 changes: 1 addition & 1 deletion spec/hatchet/profile_d_scripts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion vendor/buildpack-stdlib_v8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f180263

Please sign in to comment.