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

Feature request: add option to hide output from cmd module state return data #30842

Closed
cmclaughlin opened this issue Feb 3, 2016 · 14 comments
Closed
Assignees
Labels
Feature new functionality including changes to functionality and code refactors, etc. fixed-pls-verify fix is linked, bug author to confirm fix Platform Relates to OS, containers, platform-based utilities like FS, system based apps State-Module ZRELEASED - 2018.3.0
Milestone

Comments

@cmclaughlin
Copy link

I found issue #19479... I seem to be having the same problem. This isn't my finest work, but here's the SLS:

echo "{{ create_admin_cmd }}" | {{ dir }}/manage-wrapper.sh shell && touch {{ dir }}/.admin:
  cmd.run:
    - creates: {{ dir }}/.admin
    - output_loglevel: quiet
    - quiet: True

It's logged as follows:

2016-02-03 00:16:41,469 [salt.state][INFO][23000] Completed state [echo "from django.contrib.auth.models import User; User.objects.create_superuser('admin', 'REMOVED', 'REMOVED')" | /mnt/cabot/manage-wrapper.sh shell && touch /mnt/cabot/.admin] at time 00:16:41.468493

The minions and master are running v2015.8.3. Please let me know if I can provide more info that would help.

@jfindlay jfindlay added Question The issue is more of a question rather than a bug or a feature request info-needed waiting for more info labels Feb 4, 2016
@jfindlay jfindlay added this to the Blocked milestone Feb 4, 2016
@jfindlay
Copy link
Contributor

jfindlay commented Feb 4, 2016

@cmclaughlin, thanks for the report. What happens when you run the state like this:

django-auth-models:
  cmd.run:
    - name: echo "{{ create_admin_cmd }}" | {{ dir }}/manage-wrapper.sh shell && touch {{ dir }}/.admin:
    - creates: {{ dir }}/.admin
    - output_loglevel: quiet
    - quiet: True

When you combine the name and id, salt will report the single value in the log to record that the state succeeded. If you want to hide the command run from the log, you will need to use a separate name and id. For further explanation, see the documentation on highstate data format.

@cmclaughlin
Copy link
Author

@jfindlay thanks for the suggestion. The password is still logged. Here's my change and resulting log in case I'm missing something...

Code:

django-auth-models:
  cmd.run:
    - name: echo "{{ create_admin_cmd }}" | {{ dir }}/manage-wrapper.sh shell && touch {{ dir }}/.admin
    - creates: {{ dir }}/.admin
    - output_loglevel: quiet
    - quiet: True

Logs:

2016-02-04 00:32:25,138 [salt.state                                                ][INFO    ][28047] Running state [echo "from django.contrib.auth.models import User; User.objects.create_superuser('admin', 'REMOVED', 'REMOVED')" | /mnt/cabot/manage-wrapper.sh shell && touch /mnt/cabot/.admin] at time 00:32:25.138359
2016-02-04 00:32:25,139 [salt.state                                                ][INFO    ][28047] Executing state cmd.run for echo "from django.contrib.auth.models import User; User.objects.create_superuser('admin', 'REMOVED', 'REMOVED')" | /mnt/cabot/manage-wrapper.sh shell && touch /mnt/cabot/.admin
2016-02-04 00:32:25,139 [salt.state                                                ][INFO    ][28047] /mnt/cabot/.admin exists
2016-02-04 00:32:25,139 [salt.state                                                ][INFO    ][28047] Completed state [echo "from django.contrib.auth.models import User; User.objects.create_superuser('admin', 'REMOVED', 'REMOVED')" | /mnt/cabot/manage-wrapper.sh shell && touch /mnt/cabot/.admin] at time 00:32:25.139211

@jfindlay jfindlay modified the milestones: Approved, Blocked Feb 4, 2016
@jfindlay jfindlay added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Regression The issue is a bug that breaks functionality known to work in previous releases. P2 Priority 2 Platform Relates to OS, containers, platform-based utilities like FS, system based apps Execution-Module State-Module Confirmed Salt engineer has confirmed bug/feature - often including a MCVE and removed info-needed waiting for more info Question The issue is more of a question rather than a bug or a feature request Execution-Module labels Feb 4, 2016
@jfindlay
Copy link
Contributor

jfindlay commented Feb 4, 2016

@cmclaughlin, thanks for the extra information. I can confirm that the INFO log is showing name data.

sls

jmoney-main ~ master # cat /srv/salt/test.sls
django-auth-models:
  cmd.run:
    - name: echo secret | wc
    - output_loglevel: quiet
    - quiet: True

command

jmoney-main ~ master # salt jmoney-main state.apply test
jmoney-main:
----------
          ID: django-auth-models
    Function: cmd.run
        Name: echo secret | wc
      Result: True
     Comment: Command "echo secret | wc" run
     Started: 17:48:25.963892
    Duration: 19.518 ms
     Changes:
              ----------
              pid:
                  16488
              retcode:
                  0
              stderr:
              stdout:
                        1       1       7

Summary for jmoney-main
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  19.518 ms

logs

# minion log
[INFO    ] User root Executing command state.apply with jid 20160203174825759783
[DEBUG   ] Command details {'tgt_type': 'glob', 'jid': '20160203174825759783', 'tgt': 'jmoney-main', 'ret': '', 'user': 'root', 'arg': ['test'], 'fun': 'state.apply'}
[INFO    ] Starting a new job with PID 16482
[DEBUG   ] LazyLoaded state.apply
[DEBUG   ] LazyLoaded saltutil.is_running
[DEBUG   ] LazyLoaded grains.get
[DEBUG   ] Initializing new AsyncZeroMQReqChannel for ('/etc/salt/pki/minion', 'jmoney-main', 'tcp://127.0.0.1:4506', 'aes')
[DEBUG   ] Initializing new SAuth for ('/etc/salt/pki/minion', 'jmoney-main', 'tcp://127.0.0.1:4506')
[DEBUG   ] Initializing new AsyncZeroMQReqChannel for ('/etc/salt/pki/minion', 'jmoney-main', 'tcp://127.0.0.1:4506', 'aes')
[DEBUG   ] Initializing new SAuth for ('/etc/salt/pki/minion', 'jmoney-main', 'tcp://127.0.0.1:4506')
[DEBUG   ] Loaded minion key: /etc/salt/pki/minion/minion.pem
[INFO    ] Loading fresh modules for state activity
[DEBUG   ] LazyLoaded jinja.render
[DEBUG   ] LazyLoaded yaml.render
[DEBUG   ] In saltenv 'base', looking at rel_path u'test.sls' to resolve u'salt://test.sls'
[DEBUG   ] In saltenv 'base', ** considering ** path u'/var/cache/salt/minion/files/base/test.sls' to resolve u'salt://test.sls'
[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache u'salt://test.sls'
[DEBUG   ] compile template: /var/cache/salt/minion/files/base/test.sls
[DEBUG   ] Jinja search path: ['/var/cache/salt/minion/files/base']
[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/test.sls' using 'jinja' renderer: 0.00494885444641
[DEBUG   ] Rendered data from file: /var/cache/salt/minion/files/base/test.sls:
django-auth-models:
  cmd.run:
    - name: echo secret | wc
    - output_loglevel: quiet
    - quiet: True

[DEBUG   ] LazyLoaded config.get
[DEBUG   ] Results of YAML rendering:
OrderedDict([('django-auth-models', OrderedDict([('cmd.run', [OrderedDict([('name', 'echo secret | wc')]), OrderedDict([('output_loglevel', 'quiet')]), OrderedDict([('quiet', True)])])]))])
[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/test.sls' using 'yaml' renderer: 0.00334811210632
[DEBUG   ] LazyLoaded cmd.run
[INFO    ] Running state [echo secret | wc] at time 17:48:25.963892
[INFO    ] Executing state cmd.run for echo secret | wc
[DEBUG   ] LazyLoaded cmd.run_all
[INFO    ] {'pid': 16488, 'retcode': 0, 'stderr': '', 'stdout': '      1       1       7'}
[INFO    ] Completed state [echo secret | wc] at time 17:48:25.983410
[DEBUG   ] File /var/cache/salt/minion/accumulator/140313430907920 does not exist, no need to cleanup.
[DEBUG   ] Minion return retry timer set to 9 seconds (randomized)
[INFO    ] Returning information for job: 20160203174825759783
[DEBUG   ] Initializing new AsyncZeroMQReqChannel for ('/etc/salt/pki/minion', 'jmoney-main', 'tcp://127.0.0.1:4506', 'aes')
[DEBUG   ] Initializing new SAuth for ('/etc/salt/pki/minion', 'jmoney-main', 'tcp://127.0.0.1:4506')

@cmarzullo
Copy link

Seeing the same issue. Trying to use ceph-authtool without exposing the key I'm using.

@thenoid
Copy link

thenoid commented Nov 18, 2016

As much as i too love logging all my super top secret info. Can we get this fixed please. Would be nice to have a 'sensitive' flag much like chef's that suppresses everything

https://imgflip.com/i/1ef4i5

@alias454
Copy link

Is there a way to completely disable logging on a per state basis? If not, what would be a good way to disable logging only for say cmd.run states on only certain nodes?

Regards,
Brandon

@azmng
Copy link

azmng commented Jun 28, 2017

'output_loglevel: quiet' still shows full cmd.run output in version 2016.11.5
Also tried 'quiet: True' with the same result: full cmd.run output shown.

Please provide some way to hide sensitive output from cmd states.
Thank you.

@verhulstm
Copy link

This bug still exists in version 2017.7.1

Do we have a work around for this issue? Or an ETA for the bug fix?

@verhulstm
Copy link

I have looked into writing a tmp file via file.append. this also logs the file changes.

@terminalmage
Copy link
Contributor

output_loglevel works exactly how it is intended to work, it suppresses the command being run, as well as the output from that command, from showing up in the minion log. There is definitely a case for suppressing the output from the changes dict however, when it is set to quiet.

@terminalmage terminalmage self-assigned this Dec 5, 2017
@terminalmage terminalmage added Feature new functionality including changes to functionality and code refactors, etc. and removed Regression The issue is a bug that breaks functionality known to work in previous releases. Bug broken, incorrect, or confusing behavior labels Dec 5, 2017
@terminalmage terminalmage removed Confirmed Salt engineer has confirmed bug/feature - often including a MCVE severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P2 Priority 2 labels Dec 5, 2017
@terminalmage terminalmage changed the title cmd.run state - quiet / output_loglevel quiet have no affect Feature request: add option to hide output from cmd module state return data Dec 5, 2017
@terminalmage terminalmage modified the milestones: Approved, oxygen Dec 5, 2017
@terminalmage
Copy link
Contributor

This has been implemented in #44840.

@terminalmage terminalmage added the fixed-pls-verify fix is linked, bug author to confirm fix label Dec 5, 2017
@Ch3LL Ch3LL closed this as completed Dec 8, 2017
@thetoolsmith
Copy link
Contributor

Is there a bug fix release for this? We have 2017.7.4 and still cannot stop the output of selected sensitive text. Tried all suggested options, hide_output: True, output_loglevel: quiet, quiet: True etc....
Nothing is preventing the minion command from being output in the master console.

@amoffat
Copy link

amoffat commented Jul 30, 2018

@thetoolsmith, and anybody else running into this. I've had success using cmd.run environment variables. Example:

    cmd.run:
        - name: echo -en "$password\n$password\n" | smbpasswd -s -a backups
        - env:
            - password: {{pillar['samba']['password']}}

output:

          ID: samba
    Function: cmd.run
        Name: echo -en "$password\n$password\n" | smbpasswd -s -a backups
      Result: True
     Comment: Command "echo -en "$password\n$password\n" | smbpasswd -s -a backups" run
     Started: 19:20:11.155744
    Duration: 36.669 ms
     Changes:   
              ----------
              pid:
                  5763
              retcode:
                  0
              stderr:
              stdout:

@arno01
Copy link
Contributor

arno01 commented Nov 3, 2020

Also works when the CLI tool doesn't read the password from stdin:

set new admin password:
  cmd.run:
    - name: grafana-cli admin reset-admin-password $PASSWORD
    - env:
      - PASSWORD: "{{ admin_password }}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. fixed-pls-verify fix is linked, bug author to confirm fix Platform Relates to OS, containers, platform-based utilities like FS, system based apps State-Module ZRELEASED - 2018.3.0
Projects
None yet
Development

No branches or pull requests