codescan: merge 139-alert remediation waves into main#195
Conversation
KooshaPari
force-pushed
the
codescan-merge-139
branch
from
6e5ce26 to
d2f99ca
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 15
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if existing, exists := c.Get("API_RESPONSE"); exists { | ||
| if existingBytes, ok := existing.([]byte); ok && len(existingBytes) > 0 { | ||
| combined := make([]byte, 0, len(existingBytes)+len(data)+1) | ||
| combined := make([]byte, 0, len(existingBytes)) |
There was a problem hiding this comment.
Buffer pre-allocation capacity reduced causing unnecessary reallocations
Medium Severity
The combined slice capacity was reduced from len(existingBytes)+len(data)+1 to just len(existingBytes). Since the function immediately appends existingBytes (filling the capacity exactly), then a potential newline, then data, this guarantees at least one unnecessary reallocation on every call. For streaming responses with many chunks, this causes repeated avoidable memory allocations in a hot path.
| // Default to Sonnet 4 | ||
| log.Debugf("kiro: unknown Sonnet model '%s', mapping to claude-sonnet-4", model) | ||
| return "claude-sonnet-4" | ||
| } |
There was a problem hiding this comment.
Opus model check nested inside Sonnet block unreachable
High Severity
The closing } and default return for the sonnet if block were removed during merge, causing the Opus variant check on line 1671 to be syntactically nested inside the Sonnet if block. Models containing "opus" but not "sonnet" (e.g. claude-opus-4) now skip the entire block and incorrectly fall through to claude-sonnet-4.5. The default claude-sonnet-4 mapping for generic Sonnet models was also lost.
| return "", fmt.Errorf("postgres store: path %q outside managed directory", candidate) | ||
| } | ||
| return resolved, nil | ||
| } |
There was a problem hiding this comment.
Unused resolveManagedAuthPath duplicates ensureManagedAuthPath
Low Severity
The new resolveManagedAuthPath method on PostgresStore is never called anywhere in the codebase. All path resolution goes through ensureManagedAuthPath instead. These two functions implement nearly identical logic (resolve path, check it's within authDir), making resolveManagedAuthPath dead code that likely resulted from merging overlapping lanes.
| } | ||
| sum := sha256.Sum256([]byte(trimmed)) | ||
| return hex.EncodeToString(sum[:8]) | ||
| } |
There was a problem hiding this comment.
Unused fingerprint functions never wired into log statements
Low Severity
antigravityModelFingerprint and kiroModelFingerprint are new functions that hash model names for safe logging, but neither is called from any production code. The log statements that previously included raw model names were instead changed to omit model names entirely, making these fingerprint functions dead code from a superseded approach.
Additional Locations (1)
codescan: merge 139-alert remediation waves into main
1 participant
This PR consolidates the lane-based code-scanning remediation work into main.\n\nIncluded waves:\n- Batch 1: 6 lanes (30 issues)\n- Batch 2: 6 lanes (60 issues)\n- Batch 3: 6 lanes (49 issues)\n\nTotal consolidated: 139 code-scanning issues.\n\nValidation run on merge branch:\n- go test ./pkg/llmproxy/thinking -count=1\n- go test ./pkg/llmproxy/store -count=1\n- go test ./pkg/llmproxy/auth/codex -count=1\n\nNotes:\n- Consolidation resolved overlapping lane edits and preserved the latest lane intent in conflict areas.
Note
Medium Risk
Broad, cross-cutting security hardening touches request routing, persistence paths, and hashing/logging behavior; functional risk is moderate due to stricter validation potentially rejecting previously-accepted inputs.
Overview
Hardens multiple auth/config surfaces against injection, traversal, and data leakage. Management endpoints now validate outbound URLs (blocking localhost/private IPs), restrict Copilot quota enrichment to trusted
httpshosts, and lock down callback forwarder targets to loopback/localhost with normalized callback paths.Token/auth file storage is tightened across management APIs and
FileTokenStore/GitTokenStore/ObjectTokenStore/PostgresStoreto prevent path traversal/escaping; shared helpers (misc.ResolveSafeFilePath*, store path guards) and extensive new tests cover these cases. OAuth success pages (Claude/Codex) now validate/escape platform URLs, WebSocket errors are JSON-escaped, request logging sanitizes request headers + request IDs, and various executor/thinking logs redact sensitive identifiers or validate inputs (e.g., Kiro region, Antigravity base URL), alongside safer hashing/signature generation updates.Written by Cursor Bugbot for commit d2f99ca. This will update automatically on new commits. Configure here.