Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

CORTX-33346: Codacy Code cleanup #2032

Merged
merged 1 commit into from
Aug 2, 2022
Merged

Conversation

rkothiya
Copy link
Contributor

@rkothiya rkothiya commented Aug 1, 2022

This patch fixes some of the codacy warnings.
warning fixed : "subprocess call with shell=True identified, security
issue.

Signed-off-by: Rinku Kothiya rinku.kothiya@seagate.com

Problem Statement

  • We are getting a critical codacy warning, "subprocess call with shell=True identified, security
    issue."

Design

  • We have set shell=False, however after doing that we are able to solve the critical error but we get another warning
    of low severity which needs to be manually verified and ignored. The reason of getting this new new warning is explained
    in the pr.

Coding

Checklist for Author

  • Coding conventions are followed and code is consistent

Testing

Checklist for Author

  • Unit and System Tests are added
  • Test Cases cover Happy Path, Non-Happy Path and Scalability
  • Testing was performed with RPM

Impact Analysis

Checklist for Author/Reviewer/GateKeeper

  • Interface change (if any) are documented
  • Side effects on other features (deployment/upgrade)
  • Dependencies on other component(s)

Review Checklist

Checklist for Author

  • JIRA number/GitHub Issue added to PR
  • PR is self reviewed
  • Jira and state/status is updated and JIRA is updated with PR link
  • Check if the description is clear and explained

Documentation

Checklist for Author

  • Changes done to WIKI / Confluence page / Quick Start Guide

This patch fixes some of the codacy warnings.
warning fixed : "subprocess call with shell=True identified, security
issue.

Signed-off-by: Rinku Kothiya <rinku.kothiya@seagate.com>
@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 1, 2022

retest this please

1 similar comment
@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 1, 2022

retest this please

@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 2, 2022

run sncr

@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 2, 2022

test sncr

@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 2, 2022

As mentioned in the ticket. There are 2 new codacy issues after fixing the 2 issues. The new warnings is, "subprocess call - check for execution of untrusted input". According to the below issue it seems that we need to manually ignore this if we think that the input is trusted :
PyCQA/bandit#333

Copy link
Contributor

@yeshpal-jain-seagate yeshpal-jain-seagate left a comment

Choose a reason for hiding this comment

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

codacy issue is marked as false positive.

Copy link
Contributor

@yatin-mahajan yatin-mahajan left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 2, 2022

rerun sncr

@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 2, 2022

Jenkins CI Result : Motr#1557

Motr Test Summary

Test ResultCountInfo
❌Failed2
📁

04motr-single-node/49motr-rpc-cancel
01motr-single-node/00userspace-tests

🏁Skipped31
📁

01motr-single-node/28sys-kvs
01motr-single-node/35m0singlenode
01motr-single-node/04initscripts
02motr-single-node/51kem
02motr-single-node/20rpc-session-cancel
02motr-single-node/10pver-assign
02motr-single-node/21fsync-single-node
02motr-single-node/13dgmode-io
02motr-single-node/14poolmach
02motr-single-node/11m0t1fs
02motr-single-node/26motr-user-kernel-tests
02motr-single-node/08spiel
03motr-single-node/06conf
03motr-single-node/36spare-reservation
04motr-single-node/34sns-repair-1n-1f
04motr-single-node/08spiel-sns-repair-quiesce
04motr-single-node/28sys-kvs-kernel
04motr-single-node/11m0t1fs-rconfc-fail
04motr-single-node/08spiel-sns-repair
04motr-single-node/19sns-repair-abort
04motr-single-node/22sns-repair-ios-fail
05motr-single-node/18sns-repair-quiesce
05motr-single-node/12fwait
05motr-single-node/16sns-repair-multi
05motr-single-node/07mount-fail
05motr-single-node/15sns-repair-single
05motr-single-node/23sns-abort-quiesce
05motr-single-node/17sns-repair-concurrent-io
05motr-single-node/07mount
05motr-single-node/07mount-multiple
05motr-single-node/12fsync

✔️Passed42
📁

01motr-single-node/43m0crate
01motr-single-node/05confgen
01motr-single-node/06hagen
01motr-single-node/52motr-singlenode-sanity
01motr-single-node/01net
01motr-single-node/01kernel-tests
01motr-single-node/03console
01motr-single-node/37protocol
01motr-single-node/02rpcping
02motr-single-node/07m0d-fatal
02motr-single-node/67fdmi-plugin-multi-filters
02motr-single-node/53clusterusage-alert
02motr-single-node/41motr-conf-update
03motr-single-node/61sns-repair-motr-1n-1f
03motr-single-node/72spiel-sns-motr-repair-quiesce
03motr-single-node/08spiel-multi-confd
03motr-single-node/69sns-repair-motr-quiesce
03motr-single-node/62sns-repair-motr-mf
03motr-single-node/70sns-failure-after-repair-quiesce
03motr-single-node/63sns-repair-motr-1k-1f
03motr-single-node/60sns-repair-motr-1f
03motr-single-node/66sns-repair-motr-abort-quiesce
03motr-single-node/24motr-dix-repair-lookup-insert-spiel
03motr-single-node/68sns-repair-motr-shutdown
03motr-single-node/64sns-repair-motr-ios-fail
03motr-single-node/71spiel-sns-motr-repair
03motr-single-node/24motr-dix-repair-lookup-insert-m0repair
03motr-single-node/04sss
03motr-single-node/65sns-repair-motr-abort
04motr-single-node/48motr-raid0-io
04motr-single-node/25m0kv
04motr-single-node/44motr-rm-lock-cc-io
04motr-single-node/45motr-rmw
05motr-single-node/23dix-repair-m0repair
05motr-single-node/43motr-sync-replication
05motr-single-node/42motr-utils
05motr-single-node/45motr-sns-repair-N-1
05motr-single-node/40motr-dgmode
05motr-single-node/23dix-repair-quiesce-m0repair
05motr-single-node/23spiel-dix-repair-quiesce
05motr-single-node/44motr-sns-repair
05motr-single-node/23spiel-dix-repair

Total75🔗

CppCheck Summary

   Cppcheck: No new warnings found 👍

@rkothiya
Copy link
Contributor Author

rkothiya commented Aug 2, 2022

Since the two newly added pre-merge jobs are still in its initial stage, we have not made them mandatory merge requirement.

@rkothiya rkothiya merged commit f5fee3d into Seagate:main Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants