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

[210] [HIGH] Numbers that start with '0' should always be encapsulated in quotation marks - False positive in latest release 0.9.0 #299

Closed
ggiesen opened this issue Jan 13, 2023 · 7 comments
Assignees
Labels
Type: Bug Something isn't working

Comments

@ggiesen
Copy link

ggiesen commented Jan 13, 2023

Describe the bug
I have the following state:

file_/etc/cron.d/certbot:
  file.managed:
    - name: /etc/cron.d/certbot
    - source: salt://files/systems/crontab.j2
    - user: root
    - group: root
    - mode: '0644'
    - template: jinja
    - context:
        mailto: user@example.com
        cron_tz: UTC
        random_delay: 60
        entries:
          - datetimespec: '0 */6 * * *'
            user: root
            command: 'certbot renew -n && cp -fp /etc/letsencrypt/live/{{ salt['grains.get']('fqdn') }}/fullchain.pem /etc/pki/tls/certs/{{ salt['grains.get']('fqdn') }}.pem && cat /etc/letsencrypt/live/{{ salt['grains.get']('fqdn') }}/privkey.pem >> /etc/pki/tls/certs/{{ salt['grains.get']('fqdn') }}.pem && chmod 0640 /etc/pki/tls/certs/{{ salt['grains.get']('fqdn') }}.pem && systemctl reload nginx; systemctl reload haproxy; systemctl restart duoauthproxy'
    - require:
        - file_exists_/etc/letsencrypt/renewal/{{ salt['grains.get']('fqdn') }}.conf

that was previously passing linting, but is now complaining about:

chmod 0640 even though the whole string is encapsulated in quotes:

[210] [HIGH] Numbers that start with '0' should always be encapsulated in quotation marks

To Reproduce
Steps to reproduce the behavior:

  1. Create state above in SLS file
    2.Run salt-lint to lint it

Expected behavior
File to pass linting

Desktop (please complete the following information):

  • OS: AlmaLinux 8.7
  • Version 0.9.0
@ggiesen ggiesen added the Type: Bug Something isn't working label Jan 13, 2023
@ggiesen ggiesen changed the title [210] [HIGH] Numbers that start with '0' should always be encapsulated in quotation marks - False positive in latest release [210] [HIGH] Numbers that start with '0' should always be encapsulated in quotation marks - False positive in latest release 0.9.0 Jan 13, 2023
@ggiesen
Copy link
Author

ggiesen commented Jan 13, 2023

Looks like it was this commit that caused the issue:

cc6e617

@jcjones
Copy link

jcjones commented Jan 13, 2023

I'm also seeing this in a yaml heredoc, as:

[210] [HIGH] Numbers that start with '0' should always be encapsulated in quotation marks
./salt/prod/storage/config.sls:36
        [0:0:0:1]    mediumx QUANTUM  UHDL             0096  /dev/sch0  /dev/sg4

[210] [HIGH] Numbers that start with '0' should always be encapsulated in quotation marks
./salt/prod/storage/config.sls:38
        [0:0:1:1]    mediumx QUANTUM  UHDL             0096  /dev/sch1  /dev/sg6
fake-scsi:
  file.managed:
    - name: /usr/local/scsi.txt
    - user: root
    - group: root
    - mode: '0644'
    - contents: |
        [0:0:0:0]    tape    IBM      ULTRIUM-HH8      MA71  /dev/st0   /dev/sg3
        [0:0:0:1]    mediumx QUANTUM  UHDL             0096  /dev/sch0  /dev/sg4
        [0:0:1:0]    tape    IBM      ULTRIUM-HH6      KAJ9  /dev/st1   /dev/sg5

@jbouter jbouter self-assigned this Jan 15, 2023
@jbouter
Copy link
Member

jbouter commented Jan 15, 2023

Hi all 👋🏻 !

Thanks for the reports. We are indeed experiencing this ourselves as well. I will discuss with @roaldnefs next week and, by the looks of things, most likely remove the rule and draft a new release.

We apologise for the inconvenience caused by this.

jbouter added a commit that referenced this issue Jan 16, 2023
As reported in #299

Signed-off-by: Jeffrey Bouter <jeffrey.bouter@warpnet.nl>
@jbouter
Copy link
Member

jbouter commented Jan 16, 2023

Hi @nicholasmhughes - I'd like to inform you that your change has caused a lot of false-positives, and we will therefore be reverting it to resolve that issue. I'm aware that your change was a fix for another false-positive, which we will need to address in a future release.

jbouter added a commit that referenced this issue Jan 16, 2023
As reported in #299

Signed-off-by: Jeffrey Bouter <jeffrey.bouter@warpnet.nl>
@nicholasmhughes
Copy link
Contributor

Make sure all of these are captured in the test cases. Tests ran clean with my change.

@jbouter
Copy link
Member

jbouter commented Jan 16, 2023

@nicholasmhughes Indeed, for the next test we will add the common false-positives. :-)

@jbouter
Copy link
Member

jbouter commented Jan 16, 2023

This issue has been resolved in release v0.9.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants