Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some issues after CVE fixes #59664

Merged
merged 12 commits into from
Mar 5, 2021

Conversation

meaksh
Copy link
Contributor

@meaksh meaksh commented Mar 1, 2021

What does this PR do?

We've detected few issues after February CVE fixes:

  • The extra_filerefs parameter is discarded for the SSHClient
  • The cmd.run function crashes if passing tuple arg:
[root@suma-test-min-centos7 ~]# salt-call --local partition.mklabel /dev/vda3 msdos
[ERROR   ] An un-handled exception was caught by salt's global exception handler:
AttributeError: 'tuple' object has no attribute 'split'
Traceback (most recent call last):
  File "/usr/bin/salt-call", line 11, in <module>
    salt_call()
  File "/usr/lib/python2.7/site-packages/salt/scripts.py", line 445, in salt_call
    client.run()
  File "/usr/lib/python2.7/site-packages/salt/cli/call.py", line 57, in run
    caller.run()
  File "/usr/lib/python2.7/site-packages/salt/cli/caller.py", line 119, in run
    ret = self.call()
  File "/usr/lib/python2.7/site-packages/salt/cli/caller.py", line 218, in call
    ret['return'] = self.minion.executors[fname](self.opts, data, func, args, kwargs)
  File "/usr/lib/python2.7/site-packages/salt/executors/direct_call.py", line 12, in execute
    return func(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/salt/modules/parted_partition.py", line 450, in mklabel
    out = __salt__['cmd.run'](cmd, python_shell=False).splitlines()
  File "/usr/lib/python2.7/site-packages/salt/modules/cmdmod.py", line 1177, in run
    **kwargs)
  File "/usr/lib/python2.7/site-packages/salt/modules/cmdmod.py", line 387, in _run
    _log_cmd(cmd),
  File "/usr/lib/python2.7/site-packages/salt/modules/cmdmod.py", line 81, in _log_cmd
    return cmd.split()[0].strip()
AttributeError: 'tuple' object has no attribute 'split'

This PR fixes these issues.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@meaksh meaksh requested a review from a team as a code owner March 1, 2021 15:15
@meaksh meaksh requested review from xeacott and removed request for a team March 1, 2021 15:15
@meaksh
Copy link
Contributor Author

meaksh commented Mar 1, 2021

@dwoz here you have it 👍

@meaksh meaksh force-pushed the master-fix-some-issues-after-cve-fixes branch from ce6b06d to a412bdf Compare March 1, 2021 16:11
twangboy
twangboy previously approved these changes Mar 1, 2021
Copy link
Collaborator

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failures on windows were introduced in this PR and look valid.

@meaksh
Copy link
Contributor Author

meaksh commented Mar 2, 2021

Hey @s0undt3ch, the previously failing test for windows is a new test I've just introduced with this PR - it was not an existing one.

I've skipped it on Windows since it seems doesn't like calling echo like this. I don't have a Windows machine here to easily test this.

Do you want this test to be also implemented to run on Windows?

@s0undt3ch
Copy link
Collaborator

Hey @s0undt3ch, the previously failing test for windows is a new test I've just introduced with this PR - it was not an existing one

Yes, I explained myself incorrectly :)

@meaksh
Copy link
Contributor Author

meaksh commented Mar 3, 2021

@s0undt3ch I've fixed the test for Windows.

I see now some other failures for "windows2019" and "macosxcatalina" after rebasing with master, but I think they're not related to this PR.

@s0undt3ch s0undt3ch added this to the Aluminium milestone Mar 4, 2021
@s0undt3ch s0undt3ch added the Aluminium Release Post Mg and Pre Si label Mar 4, 2021
@s0undt3ch s0undt3ch requested a review from krionbsd March 5, 2021 11:49
@Ch3LL Ch3LL merged commit 2944e2d into saltstack:master Mar 5, 2021
@meaksh meaksh deleted the master-fix-some-issues-after-cve-fixes branch March 5, 2021 15:14
@sagetherage sagetherage added v3002.6 vulnerable version v3000.9 vulnerable version v3001.7 vulnerable version labels Mar 16, 2021
dwoz pushed a commit to dwoz/salt that referenced this pull request Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aluminium Release Post Mg and Pre Si v3000.9 vulnerable version v3001.7 vulnerable version v3002.6 vulnerable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants