demo: code-review bait (Go)#1
Conversation
kramlipi
left a comment
kramlipi
left a comment
There was a problem hiding this comment.
Automated code review by kramlip code-agent.
Review notes from kramlip code-agent — grounded scanners + critic, not a rubber stamp.
| // BaitHandler has intentional issues for code-review demo. | ||
| func BaitHandler(w http.ResponseWriter, r *http.Request) { | ||
| var body []byte | ||
| n, _ := r.Body.Read(body) // ignore error; Read into nil slice |
There was a problem hiding this comment.
Must fix · logic · demokramlipi/review_bait.go:11 · Unsafe read into nil slice
What: Reading into a nil slice results in 0 bytes read. Why it matters: The buffer remains empty, and the error return is ignored, masking potential I/O failures. Do this: Initialize the slice with a fixed size or use io.ReadAll.
| fmt.Fprint(w, m["missing"]) | ||
|
|
||
| s := r.URL.Query().Get("x") | ||
| fmt.Sprintf(s) // result discarded; format string from user |
There was a problem hiding this comment.
Must fix · vulnerability · demokramlipi/review_bait.go:18 · Format string injection
What: Using user-controlled input as a format string in fmt.Sprintf. Why it matters: An attacker can provide malicious format specifiers (e.g., %s%s%s%s) to cause panics or leak memory. Do this: Use fmt.Print(s) or fmt.Printf("%s", s).
| fmt.Fprintf(w, "n=%d data=%s", n, string(body)) | ||
|
|
||
| var m map[string]string | ||
| fmt.Fprint(w, m["missing"]) |
There was a problem hiding this comment.
Should fix · design · demokramlipi/review_bait.go:15 · Fragile map access
What: Accessing a nil map is safe in Go (returns zero value), but relying on this behavior for business logic is brittle. Why it matters: It hides potential initialization bugs. Do this: Initialize the map or check for existence.
| ) | ||
|
|
||
| // BaitHandler has intentional issues for code-review demo. | ||
| func BaitHandler(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Worth a look · design · demokramlipi/review_bait.go:9 · No behavior risk: api_signature
What: No behavior risk: api_signature
Why it matters: The function signature follows standard http.HandlerFunc pattern.
Do this: Either fix it in this PR, or reply here explaining why the current behavior is intentional.
Evidence: func BaitHandler(w http.ResponseWriter, r *http.Request) {
| // BaitHandler has intentional issues for code-review demo. | ||
| func BaitHandler(w http.ResponseWriter, r *http.Request) { | ||
| var body []byte | ||
| n, _ := r.Body.Read(body) // ignore error; Read into nil slice |
There was a problem hiding this comment.
Worth a look · logic · demokramlipi/review_bait.go:11 · Possible ignored error
What: Error return may be ignored; check err before use.
Why it matters: (rule:unchecked_err_assign)
Do this: Either fix it in this PR, or reply here explaining why the current behavior is intentional.
Evidence: n, _ := r.Body.Read(body) // ignore error; Read into nil slice
Review notes5 note(s) — 2 must-fix · 1 should-fix · 2 worth a look · 1 from deterministic scanners Themes: logic (2), design (2), vulnerability (1) Start here:
Review notes from kramlip code-agent — grounded scanners + critic, not a rubber stamp. |
1 participant
Summary
Demo PR for kramlipi code-agent code-review — intentional issues in demokramlipi/review_bait.go.
Notes
Do not merge. Showcase inline review comments.
— kramlipi · cluevion@gmail.com · https://kramlipi.com
Made with Cursor