13 KiB
TODO — opšti pregled koda (NTech)
Pregled urađen bez izmena koda (samo istraga). Fokus: dual-render forme, transakciona ispravnost repository sloja, handler validacija/CSRF/dozvole, neslaganja model↔baza↔šablon, JS logika (duple registracije, double-submit zaštita), i ostalo sumnjivo.
Legenda ozbiljnosti: 🔴 visok · 🟡 srednji · 🟢 nizak
1. Dual-submit / duple registracije event listenera
-
🔴
prodaja_forma.html— dva nezavisna 'submit' listenera na istoj formi, mogući DUPLI POST po jednom kliku. ISPRAVLJENO. Dodatdata-full-reloadatribut na<form>(web/templates/stranice/prodaja_forma.html:35) — generički AJAX submit-interceptor ubase.html:401sad tu formu preskače (ranireturn), pa jedini submit-put ostaje Alpine-ov@submit.prevent="posaljiProdaju($event)"(nativee.target.submit()). Potvrđeno statičkom analizom (browser extension nije bio dostupan u ovoj sesiji za live Network-tab proveru): pre izmene,f.hasAttribute('data-full-reload')je bilofalseza ovu formu pa bi generički handler nastavio nafetch()POST — sad jetruei handler odmah izlazi. Proverеноcurl-om da se atribut ispravno renderuje u serviranom HTML-u (/prodaja/nova).web/templates/stranice/prodaja_forma.html:35—<form method="POST" action="/prodaja/nova" @submit.prevent="posaljiProdaju($event)">nemadata-full-reloadatribut.web/templates/teme/podrazumevana/base.html:396-410— generički AJAX submit-interceptor se kači na SVAKUform[method="POST"]u dokumentu (osim multipart ilidata-full-reload), uključujući ovu formu, i pri submit-u odmah šalje sopstvenifetch()POST.web/static/js/ntech.js:400-410(posaljiProdaju) — Alpine-ov@submit.preventhandler ODVOJENO zakazujee.target.submit()(pravi native POST, pun page-reload) preko$nextTick. Pošto nijedan od handlera ne zovestopImmediatePropagation(), oba se izvršavaju na isti klik → dva POST zahteva ka/prodaja/nova(jedan prekofetch, jedan preko native submit-a). Ovo je najverovatniji PRAVI uzrok originalno prijavljenog bug-a "prodaja se upiše dva puta" — raniji nalaz (nedostajući:disablednapdv_stopa[], već ispravljen) je stvaran, ali verovatno NIJE bio glavni uzrok dupliranja REDOVA u bazi. Napomena: upravo dodatiidempotency_key(migracija 106,ProdajaRepo.Kreiraj) verovatno already prikriva VIDLJIV simptom (drugi zahtev se sad prepoznaje i vraća isti nalog) — ali dupli HTTP zahtev i dalje postoji (nepotrebno opterećenje, mogući flash pogrešnog tosta/greške ako prvi fetch stigne sa nepočišćenim praznim stavkama pre nego što Alpine stigne da ih filtrira). Postojeća konvencija za ovakav slučaj već postoji u repo-u:data-full-reload(v.nabavka_detalji.html:197,pdv_kir.html:29,kpo.html:24itd.) — forma bi trebalo da dobije taj atribut da se isključi iz generičkog handlera, ili se custom@submit.preventmehanizam treba ukloniti u korist generičkog. Preporuka: potvrditi u browseru (Network tab) da li se zaista šalju 2 zahteva pri klik na "Naplati", pa dodatidata-full-reloadna<form>(ili ukloniti dupli mehanizam). -
🟢 Ostale forme (
nabavka_forma.html,servis_forma.html, itd.) ne koriste@submitAlpine direktivu (potvrđeno:grep -rln "@submit" web/templates/vraća samoprodaja_forma.html) — generički AJAX handler ubase.htmlje jedini submit-put za njih, nema konflikta. Bez akcije.
2. Dual-render forme (desktop tabela + mobilna kartica) — :disabled="isMobile" obrazac
-
✅
prodaja_forma.html:131,223—pdv_stopa[]bez:disabled— VEĆ ISPRAVLJENO u prethodnom zadatku (dodato:disabled="isMobile"/:disabled="!isMobile"analogno susednim poljima). -
🟢
nabavka_forma.html(jedina druga stranica saisMobiledual-render obrascem,stavkeblok) — svi[]inputi (artikal_id[],kolicina[],cena_po_komadu[],marza[],prodajna[]) imaju simetričan:disabled="isMobile"/:disabled="!isMobile"par (redovi 135-161 desktop, 208-233 mobilni). Nema iste greške. Zavisni troškovi (trosak_naziv[],trosak_iznos[], redovi 274-278) NISU dual-render (samo jedanx-forblok, bez posebne mobilne kopije) — nema rizika duplog slanja. -
🟢 Nijedna druga
stranice/*.htmlstranica ne koristiisMobiledual-render Alpine obrazac (servis, klijenti, artikli/magacin liste koriste responsive CSS/HTMX pristup, ne duplo renderovanjex-forsa:disabledprekidačem) — obrazac bug-a je specifičan za prodaju/nabavku, oba pokrivena.
3. Repository sloj — transakciona ispravnost i race conditions
-
Svih 20
BeginTx(...)poziva uinternal/db/sqlite/*.goimadefer tx.Rollback()odmah posle provere greške (provereno u:artikal.go,rezervni_kodovi.go,servisni_potrazivani_delovi.go,nabavka.go,prodaja.go,nivelacija.go,servisni_radovi.go,servisni_delovi.go,servis.go) — nema propusta, bez akcije. -
🟡
ServisRepo.Kreirajnema idempotency zaštitu — ISPRAVLJENO. Primenjen isti obrazac kao za prodaju (migracija 106): nova migracijamigrations/107_servis_idempotency_key.sql(kolonaidempotency_key+ parcijalniUNIQUEindeks naservisni_nalozi),model.ServisniNalog.IdempotencyKeypolje,ServisRepo.Kreiraj(internal/db/sqlite/servis.go) proverava postojeći ključ pre insert-a i vraća postojeći ID ako je nalog već kreiran istim ključem,parseFormuNaloga(internal/handler/servis.go) čitaidempotency_keyiz forme,servis_forma.htmldobija skriveno polje + JS UUID generator (samo za novi nalog, ne za izmenu). Dodatinternal/db/sqlite/servis_idempotency_test.go(2 testa: sa ključem vraća isti ID, bez ključa prave se dva odvojena naloga kao ranije) — oba prolaze. -
🟢
NabavkaRepo.Kreiraj(internal/db/sqlite/nabavka.go:178) nema interni sekvencijalni broj (koristibroj_racunadobavljača, korisnički unet) — nema race-a oko generisanja broja, ali isti opšti rizik "dva odvojena POST-a = dve nabavke" postoji arhitekturno kao i svuda gde nema idempotency ključa. Niži prioritet jer nema poznatu žalbu/simptom. PREOSTALO — nije automatski rađeno. Isti obrazac (idempotency_key kolona + parcijalni UNIQUE indeks) bi se mogao primeniti i ovde radi pune konzistentnosti sa prodajom/servisom — javljeno koordinatoru na odluku, nije prioritet jer nema poznat simptom. -
🟢 Parametrizacija SQL upita — nasumičan pregled
fmt.Sprintf/string-concat obrazaca uinternal/db/sqlite/*.go(magacin.go:112, artikal.go:167,191, klijent.go, trosak.go, usluga.go) — svi slučajevi concat-uju samo hardkodovane identifikatore kolona/tabela (const liste, fiksni literali iz poziva) ili grade?placeholder nizove; korisnički unos ide isključivo krozargs. Nema SQL injection rizika u pregledanim mestima.
4. Handler sloj — validacija, CSRF, autorizacija
-
🔴 IDOR / broken access control na podsetnicima (ličnim podsetnicima). ISPRAVLJENO. Dodata
korisnikSmeDaMenjaPodsetnik(k, p)provera (internal/handler/podsetnici.go) — dozvoljava izmenu/završavanje/brisanje ako je korisnik admin/superadmin (middleware.JeAdmin) ili jep.KorisnikIDtačno njegov ID; u suprotnom403 Forbidden. Primenjeno u sve tri funkcije:SacuvajIzmenePodsetnika(sad prvo učitava postojeći podsetnik pre izmene),OznaciPodsetnikiObrisiPodsetnik(sad prvo učitava podsetnik pre brisanja, ranije je brisao direktno po ID-u bez čitanja). Dodatinternal/handler/podsetnici_test.gosa 6 test-slučajeva (radnik/tuđi radnik/admin × svoj/tuđi/nedodeljen podsetnik) — svi prolaze.internal/handler/podsetnici.go:SacuvajIzmenePodsetnika(red 166-195) — učitavaidiz URL-a, pozivaPodsetnikRepo.IzmeniBEZ provere dapodsetnik.KorisnikIDpripada ulogovanom korisniku (ili je izmenilac admin/superadmin).OznaciPodsetnik(red 198-218) — isto, menja status završenosti bilo kog podsetnika po ID-u bez provere vlasništva.ObrisiPodsetnik(red 221-234) — isto, briše bilo koji podsetnik po ID-u bez provere vlasništva. Lista (filter.KorisnikID = &k.ID, red 51) i kreiranje ISPRAVNO vezuju podsetnik za korisnika (osim kad admin/superadmin eksplicitno dodeli drugom korisniku, red 260-269), ali IZMENA/ZAVRŠI/BRISANJE nemaju tu proveru — bilo koji ulogovan korisnik (uključujući najniže privilegovanu ulogu) može menjati/brisati/označavati kao završene tuđe (pa i admin/superadmin) podsetnike prostim pogađanjem/iteracijom numeričkog ID-a u POST ruti. Preporuka: u sve tri funkcije dohvatiti postojeći podsetnik, proveritipostojeci.KorisnikID == &k.ID(ilik.Ulogaje admin/superadmin), vratiti 403/redirect sa greškom ako ne odgovara — isti obrazac kao provera vlasništva koja bi trebalo da postoji za bilo koji "moj" resurs.
-
🟢 CSRF pokrivenost rutera (
cmd/ntech/main.go) — sve mutating rute (POST/PUT/DELETE) su unutar jednog od dvar.Groupbloka koji uključujuntechmw.CsrfMiddleware(redovi 255-263 javne, 285-515 zaštićene), OSIM/status/{token}/prihvati|odbij|odluka-odabrano(redovi 270-281) — ovo je namerno i dokumentovano (komentar red 265-268): autentikacija je jednokratni tajni token u URL-u, capability model, ne sesijski kolačić, pa klasičan CSRF model napada ne važi. Logika je zdrava, bez akcije, samo zabeleženo radi potpunosti pregleda. -
🟢 Raw Go greške korisniku — pregled
http.Error(w, err.Error(), ...)/ sličnih obrazaca uinternal/handler/*.go: nije pronađen nijedan slučaj curenja sirove Go greške korisniku; svuda se koriste fiksne srpske poruke uzslog.Errorza internu dijagnostiku. Bez akcije. -
🟢 Sistematski
_ = .../x, _ = ...obrasci uinternal/handler/*.go(npr.podsetnici.go:91,154,287—korisnici, _ = h.KorisniciRepo.Lista(...)za popunu dropdown-a;prijava.go— best-effort audit logovanje neuspešnih pokušaja prijave) — namerno "best-effort" ponašanje (log/dropdown ne sme blokirati glavni tok), prihvatljivo. Jedini blagi rizik: akoKorisniciRepo.Listapadne, dropdown za dodelu podsetnika drugom korisniku će tiho biti prazan bez indikacije greške korisniku — kozmetički, nizak prioritet. PREOSTALO — nije automatski rađeno. Kozmetička izmena (npr.slog.Warnpri grešci) bi se mogla dodati — javljeno koordinatoru na odluku, vrlo nizak prioritet.
5. Neslaganja model ↔ baza ↔ šablon
- 🟢 Spot-provera
model.ProdajniNalog↔SELECTuProdajaRepo.DohvatiID(internal/db/sqlite/prodaja.go:121-147) — polja se poklapaju 1:1 (uklj. i novoIdempotencyKey, namerno izostavljeno iz SELECT-a jer se ne prikazuje nigde — ne predstavlja bug). Nije pronađeno neslaganje na ovom mestu. Dublja/iscrpna provera SVIH modul-tabela-šablon trojki nije urađena u ovom prolazu zbog obima repoa (~40+ tabela) — vredi ponoviti ciljano po modulu ako se posumnja na konkretno polje.
6. Ostalo
- 🟢 Nema zaostalih
TODO/FIXME/XXX/HACKkomentara uinternal/,web/,cmd/(pretraga bez pogotka) — kod je već "čist" po tom kriterijumu. - 🟢 Data-refresh re-izvršavanje
<script>blokova (base.html:437-441) — ugrađeni inline skriptovi unutardata-refreshregiona (npr.servis_detalji.htmlnaplata-dijalog skripta oko reda 100-234) su ispravno vezani za NOVI DOM čvor pri svakomreplaceWith, pa ne dolazi do akumulacije listenera na istom, trajnom elementu — obrazac je ispravan, bez akcije.
Sažetak
Status posle ispravke: sva 🔴 visok i 🟡 srednji rizik nalaza su ISPRAVLJENA (3/3). Svi 🟢 nizak rizik nalazi su ili potvrde "bez akcije" (kod je već ispravan) ili su prijavljeni koordinatoru kao preostali, po dogovoru bez automatske izmene.
| Kategorija | Nalaza | 🔴 Visok | 🟡 Srednji | 🟢 Nizak |
|---|---|---|---|---|
| 1. Dual-submit / listeneri | 2 | 1 ✅ | 0 | 1 |
| 2. Dual-render forme | 3 | 0 | 0 | 3 (1 ✅ ispravljen ranije) |
| 3. Repository / transakcije | 4 | 0 | 1 ✅ | 3 (1 preostalo) |
| 4. Handler / CSRF / dozvole | 4 | 1 ✅ | 0 | 3 (1 preostalo) |
| 5. Model↔baza↔šablon | 1 | 0 | 0 | 1 |
| 6. Ostalo | 2 | 0 | 0 | 2 |
| Ukupno | 16 | 2/2 ✅ | 1/1 ✅ | 13 (2 preostala, niska prioriteta) |
Ispravljeni nalazi (visok/srednji rizik):
- ✅ Dupli POST na
/prodaja/nova— dodatdata-full-reloadna formu, isključuje je iz generičkog AJAX interceptora ubase.html; jedini submit-put ostaje Alpine@submit.prevent. Commit:db1b551. - ✅ IDOR na podsetnicima — dodata
korisnikSmeDaMenjaPodsetnikprovera vlasništva uSacuvajIzmenePodsetnika,OznaciPodsetnik,ObrisiPodsetnik; 403 ako korisnik nije vlasnik niti admin/superadmin. Commit:b9e7043. - ✅
ServisRepo.Kreirajbez idempotency zaštite — dodat isti obrazac kao za prodaju (migracija 107,idempotency_keykolona + parcijalni UNIQUE indeks, provera uKreiraj, skriveno polje uservis_forma.html). Commit:3998119.
Preostalo (nizak rizik, nije automatski rađeno — čeka odluku):
NabavkaRepo.Kreirajnema isti idempotency obrazac (nema poznat simptom, niži prioritet).- Silent-fail dropdown u
podsetnici.go(korisnici, _ = ...) — kozmetička izmena, vrlo nizak prioritet.