SonarCube warning

Szkriptek: Python, Perl, Bash, ...

Sziasztok,

 

Egyszeru os.system('echo "xxxx--------------------------------"') sorra Warningot dob a SonarCube.

ezt irj:

Make sure that executing this OS command is safe here.
Using shell interpreter when executing OS commands is security-sensitive python:S4721

Hogy lehetne ezt orvosolni?

 

Koszonet elore a segitsegert.

Ardi

  • 1374 megtekintés

( NevemTeve | 2023. 04. 20., cs – 09:28 )

Valószínűleg reflexszerűen minden os.system-re ezt mondja. # NOSONAR a sor végére.

( hory v | 2023. 04. 21., p – 11:14 )

Szerkesztve: 2023. 04. 21., p – 11:21

Ne hasznalj ilyen vackokat. Ha nem segit, csak ketsegeket ebreszt es hulyesegeket beszel, akkor az egy akadaly, nem segitseg.

Nekem meg nem volt olyan error/warning, ami nem hulyeseg lett volna, vagy nem lett volna felette egy FIXME. Nem is hasznalok ilyeneket sonar* szoftvereket.

Ugyanez igaz a jetbrains tippjeire is, bar azok egy nagysagrenddel korrektebbek, de 90% false positive.

Pontosítanék: akkor használjon ilyet, ha tudja értelmezni a figyelmeztetéseket, _és_ utánajár annak, hogy az miért van. A múlt ködébe vesző emlékem ezzel kapcsolatban egy ssh-t megvalósító java-s motyó esete: két dolgot a sonar szerint rossz sorrendben rakott a kódba a programozó, ezért az egyik verzióváltás során azt is javították. És nem is működött utána az ssh-s része a dolognak. Forrásra ránézve látszott a különbség - az eredeti komponensre a neten rákeresve a doksijában igen előkelő helyen szerepelt, hogy bár a sonar azt mondja hogy nem jó, de ez csak úgy működik, ahogy itt le vagyon írva...
Következő verzióban visszacserélték, és a sonar-nak "megmagyarázták" :-) hogy jó'van az úgy :-D

(Elméletileg igaza volt a sonar-nak, gyakorlatilag viszont az, amit a Java-ban by design követni kellett volna, pont nem működött...)
 

Nekem meg nem volt olyan error/warning, ami nem hulyeseg lett volna, vagy nem lett volna felette egy FIXME. [...] Ugyanez igaz a jetbrains tippjeire is, bar azok egy nagysagrenddel korrektebbek, de 90% false positive.

Ha ez a két mondat nálad igaz, akkor nagyon rossz kódminőséged... helyén kell kezelni a statikus kódellenőrzést és az abból kieső problémákat, de az esetek nagy részében igaza van.

https://iotguru.cloud

Ha neked ujat tud mondani egy statikus kodellenorzes, akkor max. medior szinten vagy.

Meg nem talalkoztam senior/expert szintu dev-vel, aki hasznosnak talalta volna ezeket.

Azt szoktak mondani, hogy segit, hogy 'fogja a junior kezet'. De a gyakorlatban ez azt jelenti, hogy allandoan a kapott szoveget probalja ertelmezni, es akkor nekem kell elmagyaraznom, hogy miert faszsag, amit kidob a static analysis.

Tehat valamilyen szinten igazad van, de senior szint es afolott sajnos nagyon irritaloak es idorabloak ezek a tool-ok.

 

update: illetve van egy borzaszto feature-e a static code analysis-nek: raijeszt szegeny kezdo dev-re. Ugye van a szokasos 'unused variable' meg hasonlo hasznavehetetlen warning-ja, ez meg OK, bar szegeny junior dev ilyenkor jol nekiall kiirtani az adatmodellt, amihez epp irjuk a kodot :P, de a durvabb, amikor ilyen okossagokat nyog be, hogy 'reassigned variable', vagy hasonlo, error prioritassal. Megprobal okoskodni, kitalalni, hogy de az az algoritmus ott hibas. Barmilyen primitiv, 3 if es 1 while ciklus jellegu kod blokkba bele tud kotni random modon, mondvacsinalt indokkal, szegeny kezdo dev meg hisz neki, es aztan rohangal hozzam, hogy mi a baj (semmi). Na ezert talalom irritalonak oket.

De ami az igazan gaz, hogy szegeny junior dev-et rossz iranyba tereli. Ahelyett, hogy programozni tanitana, tippeket adna, raijeszt hibas/mondvacsinalt dolgokkal. Nem egy dev-et lattam mar iranyt teveszteni a 80-as evek pedantic OO java vilagaba a static analysis tool-ok 'tanacsai' miatt. Rigolyakat tanit ugyanis, dinamikus, funkcionalis programozas helyett.

Ha neked ujat tud mondani egy statikus kodellenorzes, akkor max. medior szinten vagy.

Szerintem nem az a célja, hogy újat mutasson. Még juniornak sem feltétlenül. Arra való, hogy életszerű körülmények között segítse a munkádat. Tudom, itt a hupon mindenki a szakmájának a csúcsa, de én még a saját régi kódomat sem tudom 100%-ban fejben tartani minden nyűgjével, nemhogy másét. Ez nem egy oktatóeszköz, bár néha lehet belőle tanulni.

Ahogy fentebb írtam, szerintem teljesen jó az a workflow, hogy a warningokat explicit megjelölöd. Az nem baj, hogy valami warningot dob (ezt egyébként neked, a seniornak kell megtanítani a kezdőknek!), de nekem például nagyon sokat segít, ha ránézek egy régebbi kódomra (vagy máséra), és látom, hogy a kérdőjeles rész szándékosan van ott.

Az meg szimpla edukációs probléma, hogy valaki megijed egy tooltól. :)

Meg nem talalkoztam senior/expert szintu dev-vel, aki hasznosnak talalta volna ezeket.

Szerintem meg pont ez a rossz hozzáállás. Igenis sok problémát tud megtalálni, bármekkora tapasztalata is van az embernek. Aki ezzel nem ért egyet, az küldje rá a Sonar-t a kódjára, és majd meglátja, hogy mennyi issuet talál. És igen, a találatok 90+ százaléka fals pozitív lesz, de ennek az egésznek az a lényege, hogy a munkafolyamat része a sonar check (vagy ugye bármilyen másik SCA/SAST megoldás), és amikor jelez, akkor egyből el kell dönteni, hogy javítani kell, vagy lehet ignorálni. És ha csak 1-1 bug-ot hamarabb meg lehet találni vele, vagy segít a kódminőséget jobbá tenni, akkor már megéri. Amúgy általában egy fejlelsztőcsapat eltérő képességű emberekből áll és igenis segít, hogy nem csak a code review-n múlik, hogy milyen minőségű kód kerül a kódbázisba, ráadásul tudjuk, hogy a code review minősége is tud igencsak hullámzó lenni (finoman szólva).

 

update: illetve van egy borzaszto feature-e a static code analysis-nek: raijeszt szegeny kezdo dev-re. Ugye van a szokasos 'unused variable' meg hasonlo hasznavehetetlen warning-ja, ez meg OK, bar szegeny junior dev ilyenkor jol nekiall kiirtani az adatmodellt, amihez epp irjuk a kodot :P, de a durvabb, amikor ilyen okossagokat nyog be, hogy 'reassigned variable', vagy hasonlo, error prioritassal. Megprobal okoskodni, kitalalni, hogy de az az algoritmus ott hibas. Barmilyen primitiv, 3 if es 1 while ciklus jellegu kod blokkba bele tud kotni random modon, mondvacsinalt indokkal, szegeny kezdo dev meg hisz neki, es aztan rohangal hozzam, hogy mi a baj (semmi). Na ezert talalom irritalonak oket.

Ez megint félreértése az egésznek. Egyrészt igaza van, nem merge-ölünk félkész kódot a mainre, teljesen jogos, hogy unused variable-re sikít. Elég nagy baj, ha két különböző task között az interface egy variable... A SQ-nak (illetve bármilyen más static code analyzernek is) pont az a lényege, hogy nem engedjünk rossz minőségű kódot merge-ölni, a mainen alapvetően mindig olyan kódnak kellene lennie, amit release-elni lehet, amibe nyilván nem fér bele a félkész kódból adódó warningok. Azt, hogy neked mi számít jó minőségű kódnak, azt neked kell beállítani, nincs one-size-fits-all solution, a quality gate-eket úgy kell beállítanod, hogy csak azokat a szabályokat használja, amik a te projekted számára hasznosak (pl. lehetnek coding style eltérések, vagy olyan szabályok, amivel nem értesz egyet).

 De ami az igazan gaz, hogy szegeny junior dev-et rossz iranyba tereli. Ahelyett, hogy programozni tanitana, tippeket adna, raijeszt hibas/mondvacsinalt dolgokkal. Nem egy dev-et lattam mar iranyt teveszteni a 80-as evek pedantic OO java vilagaba a static analysis tool-ok 'tanacsai' miatt. Rigolyakat tanit ugyanis, dinamikus, funkcionalis programozas helyett.

A SonarQube-ot is a helyén kell kezelni, nem ebből kell megtanulni programozni, de segít rámutatni azokra a kritikus pontokra, amik hibákat rejthetnek. A juniorok terelése meg a senior fejlesztők dolga lenne egyrészt a saját példájukon, másrészt meg code-review-k/design review-k során. 

 

Szerintem meg pont ez a rossz hozzáállás. Igenis sok problémát tud megtalálni, bármekkora tapasztalata is van az embernek. Aki ezzel nem ért egyet, az küldje rá a Sonar-t a kódjára, és majd meglátja, hogy mennyi issuet talál. És igen, a találatok 90+ százaléka fals pozitív lesz, de ennek az egésznek az a lényege, hogy a munkafolyamat része a sonar check (vagy ugye bármilyen másik SCA/SAST megoldás), és amikor jelez, akkor egyből el kell dönteni, hogy javítani kell, vagy lehet ignorálni. És ha csak 1-1 bug-ot hamarabb meg lehet találni vele, vagy segít a kódminőséget jobbá tenni, akkor már megéri. Amúgy általában egy fejlelsztőcsapat eltérő képességű emberekből áll és igenis segít, hogy nem csak a code review-n múlik, hogy milyen minőségű kód kerül a kódbázisba, ráadásul tudjuk, hogy a code review minősége is tud igencsak hullámzó lenni (finoman szólva).

 

Tudod, hany kodot lattam mar, amiben tobbtucatnyi sonar hiba volt, es gyonyoruszep commit, ment is a merge, es hany olyat, amelyikben nem jelzett hibat, de egy okadek fos hanyas volt? Pont ez a baj, ha valaminek 90% a fals pozitiv aranya, es semmi olyat nem ellenoriz, ami tenylegesen a kodminoseg jelzoje lenne, akkor az olyan szinten megbizhatatlan, hogy kar is idot tolteni vele.

Az egyetlen megbizhato (es gyors/egyszeru) code review az, amikor leultok ketten es vegigmentek a changeset-en.

 

 

Ez megint félreértése az egésznek. Egyrészt igaza van, nem merge-ölünk félkész kódot a mainre, teljesen jogos, hogy unused variable-re sikít. Elég nagy baj, ha két különböző task között az interface egy variable... A SQ-nak (illetve bármilyen más static code analyzernek is) pont az a lényege, hogy nem engedjünk rossz minőségű kódot merge-ölni, a mainen alapvetően mindig olyan kódnak kellene lennie, amit release-elni lehet, amibe nyilván nem fér bele a félkész kódból adódó warningok. Azt, hogy neked mi számít jó minőségű kódnak, azt neked kell beállítani, nincs one-size-fits-all solution, a quality gate-eket úgy kell beállítanod, hogy csak azokat a szabályokat használja, amik a te projekted számára hasznosak (pl. lehetnek coding style eltérések, vagy olyan szabályok, amivel nem értesz egyet).

Nem egy unused variable -tol lesz rossz a kod. Hanem pl. amikor jon az OO-agyu user, es elkezdi mixelni az adatmodellt a business logic-kal, teleszorja a kodot getter/setter-rel total feleslegesen, amikor nem veszi eszre, hogy tobb szalon fer hozza egy nem thread-safe adathoz (vagy forditva). A legdurvabb eddig az volt, amikor egy .clone() metodust ugy oldott meg egy dev, hogy serializalta json-be, es visszaszerializalta utana. Persze tokeletes kod sonar szerint...

De attol is falnak lehet rohanni, amikor jon a felrekonfiguralt IDE-jevel, es a changelog fele import auto-rearrange.

"Az egyetlen megbizhato (es gyors/egyszeru) code review az, amikor leultok ketten es vegigmentek a changeset-en." - melyiken mentek ketten vegig? A frontenden, a backenden, a middleware-en? Es kivel a 15+ fejleszto kozul? Vagy felvesztek majd 3 emberenekent egy "senior peer review representative"-ot?

Ezt most te halalosan komolyan mondtad? Mert viccnek eros lenne :D

Amugy a fejlesztes soran mar maga a merge is csak akkor tortenhet meg ha a peer-ek rabolitanak. A peer-ek meg lehetnek emberek, robotok, plusz a build. Altalaban van egy szavazata mindnekinek merge meg majd mondjuk 3+ pontttol lesz. Lehet hogy az egyik egy kodanalizis vegeredmeny. 

Most tulajdonkeppen leirtad hogy nem is tudod mi az a sonarcube :D

Nem csak sima statikus kodelemzo, hanem vulnerability, code coverage es meg sok mas dolgora jo. A masik, hogy buildrol buildre kapsz statisztikat. Ami nagyon jo egy csapat kodminoseg javulasanak vagy romlasanak figyelesere es mondjuk egy team leader mind a ket dolgora tud megfeleloen reagalni. Valamint amanagement fele is szep statisztikakat lehet gyartani amit el is varnak a jobb helyeken.

De ne menjunk abba bele, hogy hogyan is mukodik egy fejlesztoceg, vagy hogyan kellene mukodnie.

Ha te azt hiszed hogy a sonarcube es trsai csak annyira jok, hogy megijesszek a junior deveket akkor az sok mindent elmond :D

Az elmúlt napok beszélgetéséből nekem az jön le, hogy itt nagyon sok ember nem dolgozott még fejlesztőcégnél, akár úgy is, hogy közben évtizedes tapasztalattal rendelkeznek. Ezt nagyon szomorúnak tartom,

mert sosem volt lehetőségük megtanulni azt, hogy hogyan kell csoportban dolgozni, ott milyen problémákkal találkozik az ember, és azokat hogyan lehet megoldani.

"Kedvencem amikor 25 eve programozo emberek meg csak cvnt-t/git-et sem hasznalnak, mert az is csak az lmbqt puhagyerekeknek van" - A dátum-idő nevű könyvtárakba bemásolás elég mindenre, nem? :-D Ennél csak az a szebb, amikor szállító kiad egy 1.2.3-as verziót aaa.1.2.3.tar.gz) , majd amikor jelezzük a hibát, kiadja az 1.2.3 verziót, aaa.1.2.3.tar.gz néven, de módosított tartalommal... Vagy a másik nagyon okos kiadja az az 1.2.5-öt, utána az 1.2.6-ot, majd az 1.2.61-et, hogy aztán következzen az 1.2.7... Azóta (egyik se mostanság volt...) lehet, hogy fejlődtek, de ezek azért "megmaradó" emlékek...

Kedvencem amikor 25 eve programozo emberek meg csak cvnt-t/git-et sem hasznalnak, mert az is csak az lmbqt puhagyerekeknek van :D

a legjobb, amikor csak random könyvtárakba elmásolják, és pár év elteltével tűnik csak fel nekik, hogy az egyébként apró kis tool-nak a forrása 1 giga emiatt

illetve amikor fogalmuk sincs, hogy mikor, hova és mi lett telepítve, aztán csak pörögnek a hátukon, hogy jaj-jaj, olyan verzióban kellene hibát javítani, ami nincs is. (egészen nagy cégeknél is láttam már ilyet)

buildrol buildre kapsz statisztikat. Ami nagyon jo egy csapat kodminoseg javulasanak vagy romlasanak figyelesere

Pont erre utalok, hogy NEM. Nem kapsz statisztikat. Mert a 'hibak' 90%-a fals pozitiv vagy flat out hulyeseg. Pont ezert nem alkalmas semmire, es ez alapjan nem lehet megitelni sem a 'fejlodest', sem egy commit-ot.

( asch v | 2023. 04. 21., p – 11:56 )

Milyen nyelv, milyen Sonar szabályok, mi a cél vele? Ha magad kedvéért Sonarozol, akkor simán NOSONAR-ozhatod ami szerinted jó. Ha valaminek meg kell felelni, akkor annak illene megfelelni.

Az os.system-mel általánosságban az a baj, hogy ilyenen keresztül lehet sérülékenységet injektálni, ha usertől jövő paraméterei vannak a hívásnak. Lásd: eszképelés https://xkcd.com/327/ .  Másik probléma lehet, hogy parancsértelmező-függő lehet, hogy mi lesz a parancs jelentése.

A konkrét példára: vilmos.nagy-gyal értek egyet, miért hívsz erre os-t mikor az stdoutra írsz a végén. Ha valóban os-t kell hívni, akkor arra a legtöbb nyelvben van olyan API, amivel tisztességes módon össze lehet rakni egy processz indítást. Azt kell használni.

( golgota | 2023. 04. 21., p – 12:23 )

"os.system('echo ..." ez most komoly? de tenyleg? mert viccnek durva lenne :D

( sedder | 2023. 05. 03., sze – 09:30 )

A Sonar ha jol emlekszem barmilyen hasonlo hivasra panaszkodni fog, mert nem szereti a shell utasitasokat.

A os.system helyett egy kicsit szofisztikaltabb megoldas a Popen lenne, aminel listaban adod meg az argumentumokat (kihagyhato az idezojelezes), es megmondhatod, hogyan legyen kezelve a status kod es a standard in/out/err.

Arra nem emlekszem, hogy shell ertelmezi-e a popen hivasokat.

A Sonar erre is jelezni fog, ugyhogy NOSONAR kell arra a hivasra. Ajanlott utana megmagyarazni egy kommentben, hogy miert igy oldottad meg a problemat, es megbizonyosodtal rola, hogy nincs mas megoldas ra.

De termeszetesen mindezt csak a legvegso esetben, ha tenyleg nem tudod megoldani kulso program nelkul.

Lambda calculus puts the fun into functional programming

( NevemTeve | 2023. 05. 03., sze – 14:13 )

Nagyon off, de pont most van egy hibám, ahol a gcc és a clang egymással versengve nem vettek észre egy inicializálatlan változót.

valgrind segített volna -- ha nem lenne Oracle-kliens a történetben: az sajnos annyi 'Conditional jump or move depends on uninitialised value(s)'-ot generál, hogy eltűnik közte a saját hiba.

Szerk: kezdem azt hinni, hogy ma jött el a suppress-file napja:

    {
       oracle_c
       Memcheck:Cond
       obj:/opt/instantclient*/lib*
    }
    {
       oracle_v4
       Memcheck:Value4
       obj:/opt/instantclient*/lib*
    }
    {
       oracle_v8
       Memcheck:Value8
       obj:/opt/instantclient*/lib*
    }

( yetii | 2023. 05. 04., cs – 15:02 )

Valaki mondja már el, miért ír mindenki a topic-ban SonarCube-ot, amikor az eszköz neve SonarQube? A címben is hibásan van...