Skip to content

Commit

Permalink
Use ATOM_SUPPRESS_ENV_PATCHING Environment Variable
Browse files Browse the repository at this point in the history 
- Stop using shell whitelist
  • Loading branch information
@joefitzgerald
joefitzgerald committed Sep 8, 2016
1 parent c570e14 commit 1027060
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 57 deletions.
2 changes: 2 additions & 0 deletions atom.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ else
BETA_VERSION=
fi

export ATOM_SUPPRESS_ENV_PATCHING=true

while getopts ":wtfvh-:" opt; do
case "$opt" in
-)
Expand Down
74 changes: 41 additions & 33 deletions spec/update-process-env-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ describe('updateProcessEnv(launchEnv)', function () {
NODE_PATH: '/the/node/path',
ATOM_HOME: '/the/atom/home'
}

const initialProcessEnv = process.env

updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2'})
updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', TERM: 'xterm-something', KEY1: 'value1', KEY2: 'value2'})
expect(process.env).toEqual({
ATOM_SUPPRESS_ENV_PATCHING: 'true',
PWD: '/the/dir',
TERM: 'xterm-something',
KEY1: 'value1',
Expand All @@ -58,34 +60,62 @@ describe('updateProcessEnv(launchEnv)', function () {
ATOM_HOME: '/the/atom/home'
}

updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something'})
updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir'})
expect(process.env).toEqual({
PWD: '/the/dir',
TERM: 'xterm-something',
ATOM_SUPPRESS_ENV_PATCHING: 'true',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
ATOM_HOME: '/the/atom/home'
})

updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')})
updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', ATOM_HOME: path.join(newAtomHomePath, 'non-existent')})
expect(process.env).toEqual({
ATOM_SUPPRESS_ENV_PATCHING: 'true',
PWD: '/the/dir',
TERM: 'xterm-something',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
ATOM_HOME: '/the/atom/home'
})


updateProcessEnv({PWD: '/the/dir', TERM: 'xterm-something', ATOM_HOME: newAtomHomePath})
updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir', ATOM_HOME: newAtomHomePath})
expect(process.env).toEqual({
ATOM_SUPPRESS_ENV_PATCHING: 'true',
PWD: '/the/dir',
TERM: 'xterm-something',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
ATOM_HOME: newAtomHomePath
})
})

it('allows ATOM_SUPPRESS_ENV_PATCHING to be preserved if set', function () {
newAtomHomePath = temp.mkdirSync('atom-home')

process.env = {
WILL_BE_DELETED: 'hi',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
ATOM_HOME: '/the/atom/home'
}

updateProcessEnv({ATOM_SUPPRESS_ENV_PATCHING: 'true', PWD: '/the/dir'})
expect(process.env).toEqual({
PWD: '/the/dir',
ATOM_SUPPRESS_ENV_PATCHING: 'true',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
ATOM_HOME: '/the/atom/home'
})

updateProcessEnv({PWD: '/the/dir'})
expect(process.env).toEqual({
ATOM_SUPPRESS_ENV_PATCHING: 'true',
PWD: '/the/dir',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
ATOM_HOME: '/the/atom/home'
})
})
})

describe('when the launch environment does not come from a shell', function () {
Expand Down Expand Up @@ -119,7 +149,6 @@ describe('updateProcessEnv(launchEnv)', function () {
it('updates process.env to match the environment in the user\'s login shell', function () {
process.platform = 'linux'
process.env.SHELL = '/my/custom/bash'
delete process.env.TERM

spyOn(child_process, 'spawnSync').andReturn({
stdout: dedent`
Expand All @@ -142,7 +171,7 @@ describe('updateProcessEnv(launchEnv)', function () {
})
})

describe('not on osx', function () {
describe('on windows', function () {
it('does not update process.env', function () {
process.platform = 'win32'
spyOn(child_process, 'spawnSync')
Expand All @@ -158,36 +187,15 @@ describe('updateProcessEnv(launchEnv)', function () {
it('indicates when the environment should be fetched from the shell', function () {
process.platform = 'darwin'
expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true)
process.platform = 'linux'
expect(shouldGetEnvFromShell({SHELL: '/bin/sh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/sh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/bin/bash'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/bash'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/bin/zsh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/zsh'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/bin/fish'})).toBe(true)
expect(shouldGetEnvFromShell({SHELL: '/usr/local/bin/fish'})).toBe(true)
})

it('returns false when the shell should not be patched', function () {
process.platform = 'darwin'
expect(shouldGetEnvFromShell({SHELL: '/bin/unsupported'})).toBe(false)
expect(shouldGetEnvFromShell({SHELL: '/bin/shh'})).toBe(false)
expect(shouldGetEnvFromShell({SHELL: '/bin/tcsh'})).toBe(false)
expect(shouldGetEnvFromShell({SHELL: '/usr/csh'})).toBe(false)

expect(shouldGetEnvFromShell({ATOM_SUPPRESS_ENV_PATCHING: 'true', SHELL: '/bin/sh'})).toBe(false)
process.platform = 'linux'
expect(shouldGetEnvFromShell({SHELL: '/bin/unsupported'})).toBe(false)
expect(shouldGetEnvFromShell({SHELL: '/bin/shh'})).toBe(false)
expect(shouldGetEnvFromShell({SHELL: '/bin/tcsh'})).toBe(false)
expect(shouldGetEnvFromShell({SHELL: '/usr/csh'})).toBe(false)
expect(shouldGetEnvFromShell({ATOM_SUPPRESS_ENV_PATCHING: 'true', SHELL: '/bin/sh'})).toBe(false)
})

it('returns false when the shell is undefined or empty', function () {
Expand Down
35 changes: 11 additions & 24 deletions src/update-process-env.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,8 @@ import {spawnSync} from 'child_process'
const ENVIRONMENT_VARIABLES_TO_PRESERVE = new Set([
'NODE_ENV',
'NODE_PATH',
'ATOM_HOME'
])

const SHELLS_KNOWN_TO_WORK = new Set([
'/sh',
'/bash',
'/zsh',
'/fish'
'ATOM_HOME',
'ATOM_SUPPRESS_ENV_PATCHING'
])

const PLATFORMS_KNOWN_TO_WORK = new Set([
Expand All @@ -23,10 +17,9 @@ const PLATFORMS_KNOWN_TO_WORK = new Set([

function updateProcessEnv (launchEnv) {
let envToAssign

if (launchEnv && shouldGetEnvFromShell(launchEnv)) {
envToAssign = getEnvFromShell(launchEnv)
} else if (launchEnv && launchEnv.PWD) { // Launched from shell
} else if (launchEnv && launchEnv.PWD) {
envToAssign = launchEnv
}

Expand All @@ -40,6 +33,10 @@ function updateProcessEnv (launchEnv) {
for (let key in envToAssign) {
if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key)) {
process.env[key] = envToAssign[key]
} else {
if (!process.env[key] && envToAssign[key]) {
process.env[key] = envToAssign[key]
}
}
}

Expand All @@ -50,29 +47,19 @@ function updateProcessEnv (launchEnv) {
}

function shouldGetEnvFromShell (env) {
if (!PLATFORMS_KNOWN_TO_WORK.has(process.platform)) { // Untested platforms
if (!PLATFORMS_KNOWN_TO_WORK.has(process.platform)) {
return false
}

if (!env || !env.SHELL || env.SHELL.trim() === '') { // Nothing to launch
if (!env || !env.SHELL || env.SHELL.trim() === '') {
return false
}

if (process.platform === 'linux' && env.TERM) { // Launched from shell
if (env.ATOM_SUPPRESS_ENV_PATCHING || process.env.ATOM_SUPPRESS_ENV_PATCHING) {
return false
}

if (process.platform === 'darwin' && env.PWD) { // Launched from shell
return false
}

for (let s of SHELLS_KNOWN_TO_WORK) {
if (env.SHELL.endsWith(s)) {
return true
}
}

return false
return true
}

function getEnvFromShell (env) {
Expand Down

0 comments on commit 1027060

Please sign in to comment.