Skip to content

Commit

Permalink
Merge pull request #3248 from boegel/fix_token_log_leak
Browse files Browse the repository at this point in the history
censor authorization part of headers before logging ReST API request
  • Loading branch information
migueldiascosta committed Mar 16, 2020
2 parents 75f0f59 + a029a91 commit 210743d
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 5 deletions.
9 changes: 9 additions & 0 deletions RELEASE_NOTES
Expand Up @@ -3,6 +3,15 @@ For more detailed information, please see the git log.

These release notes can also be consulted at https://easybuild.readthedocs.io/en/latest/Release_notes.html.

v4.1.2 (March 16th 2020)
------------------------

bugfix release

- fix gitdb dependency on Python 2.6 in test configuration (#3212)
- fix broken test for --review-pr by using different PR to test with (#3226)
- censor authorization part of headers before logging ReST API request (#3248)

v4.1.1 (January 16th 2020)
--------------------------

Expand Down
9 changes: 8 additions & 1 deletion easybuild/base/rest.py
Expand Up @@ -35,6 +35,7 @@
:author: Jens Timmerman
"""
import base64
import copy
import json
from functools import partial

Expand Down Expand Up @@ -162,7 +163,13 @@ def request(self, method, url, body, headers, content_type=None):
if self.auth_header is not None:
headers['Authorization'] = self.auth_header
headers['User-Agent'] = self.user_agent
fancylogger.getLogger().debug('cli request: %s, %s, %s, %s', method, url, body, headers)

# censor contents of 'Authorization' part of header, to avoid leaking tokens or passwords in logs
headers_censored = copy.deepcopy(headers)
headers_censored['Authorization'] = '<actual authorization header censored>'

fancylogger.getLogger().debug('cli request: %s, %s, %s, %s', method, url, body, headers_censored)

# TODO: in recent python: Context manager
conn = self.get_connection(method, url, body, headers)
status = conn.code
Expand Down
2 changes: 1 addition & 1 deletion easybuild/tools/version.py
Expand Up @@ -43,7 +43,7 @@
# recent setuptools versions will *TRANSFORM* something like 'X.Y.Zdev' into 'X.Y.Z.dev0', with a warning like
# UserWarning: Normalizing '2.4.0dev' to '2.4.0.dev0'
# This causes problems further up the dependency chain...
VERSION = LooseVersion('4.1.1')
VERSION = LooseVersion('4.1.2')
UNKNOWN = 'UNKNOWN'


Expand Down
4 changes: 4 additions & 0 deletions requirements.txt
Expand Up @@ -5,6 +5,10 @@ keyring==5.7.1; python_version < '2.7'
keyring<=9.1; python_version >= '2.7'
keyrings.alt; python_version >= '2.7'

# GitDB 4.0.1 no longer supports Python 2.6
gitdb==0.6.4; python_version < '2.7'
gitdb; python_version >= '2.7'

# GitPython 2.1.9 no longer supports Python 2.6
GitPython==2.1.8; python_version < '2.7'
GitPython; python_version >= '2.7'
Expand Down
39 changes: 36 additions & 3 deletions test/framework/options.py
Expand Up @@ -1233,6 +1233,39 @@ def test_from_pr(self):
print("Ignoring URLError '%s' in test_from_pr" % err)
shutil.rmtree(tmpdir)

def test_from_pr_token_log(self):
"""Check that --from-pr doesn't leak GitHub token in log."""
if self.github_token is None:
print("Skipping test_from_pr_token_log, no GitHub token available?")
return

fd, dummylogfn = tempfile.mkstemp(prefix='easybuild-dummy', suffix='.log')
os.close(fd)

args = [
# PR for foss/2018b, see https://github.com/easybuilders/easybuild-easyconfigs/pull/6424/files
'--from-pr=6424',
'--dry-run',
'--debug',
# an argument must be specified to --robot, since easybuild-easyconfigs may not be installed
'--robot=%s' % os.path.join(os.path.dirname(__file__), 'easyconfigs'),
'--github-user=%s' % GITHUB_TEST_ACCOUNT, # a GitHub token should be available for this user
]
try:
self.mock_stdout(True)
self.mock_stderr(True)
outtxt = self.eb_main(args, logfile=dummylogfn, raise_error=True)
stdout = self.get_stdout()
stderr = self.get_stderr()
self.mock_stdout(False)
self.mock_stderr(False)
self.assertFalse(self.github_token in outtxt)
self.assertFalse(self.github_token in stdout)
self.assertFalse(self.github_token in stderr)

except URLError as err:
print("Ignoring URLError '%s' in test_from_pr" % err)

def test_from_pr_listed_ecs(self):
"""Test --from-pr in combination with specifying easyconfigs on the command line."""
if self.github_token is None:
Expand Down Expand Up @@ -2710,17 +2743,17 @@ def test_review_pr(self):

self.mock_stdout(True)
self.mock_stderr(True)
# PR for CMake 3.12.1 easyconfig, see https://github.com/easybuilders/easybuild-easyconfigs/pull/6660
# PR for gzip 1.10 easyconfig, see https://github.com/easybuilders/easybuild-easyconfigs/pull/9921
args = [
'--color=never',
'--github-user=%s' % GITHUB_TEST_ACCOUNT,
'--review-pr=6660',
'--review-pr=9921',
]
self.eb_main(args, raise_error=True)
txt = self.get_stdout()
self.mock_stdout(False)
self.mock_stderr(False)
regex = re.compile(r"^Comparing CMake-3.12.1-\S* with CMake-3.12.1-")
regex = re.compile(r"^Comparing gzip-1.10-\S* with gzip-1.10-")
self.assertTrue(regex.search(txt), "Pattern '%s' not found in: %s" % (regex.pattern, txt))

def test_set_tmpdir(self):
Expand Down

0 comments on commit 210743d

Please sign in to comment.