diff --git a/.gitea/actions/review/action.yml b/.gitea/actions/review/action.yml index 0fdccdc..912c7da 100644 --- a/.gitea/actions/review/action.yml +++ b/.gitea/actions/review/action.yml @@ -70,6 +70,10 @@ inputs: description: 'Delete previous review from same bot before posting. Accepts: true/1/yes or false/0/no (default true)' required: false default: 'true' + system-prompt-file: + description: 'Local file with additional system prompt instructions (e.g. security review focus)' + required: false + default: '' runs: using: 'composite' @@ -150,6 +154,7 @@ runs: LLM_TIMEOUT: ${{ inputs.timeout }} LLM_PROVIDER: ${{ inputs.llm-provider }} UPDATE_EXISTING: ${{ inputs.update-existing }} + SYSTEM_PROMPT_FILE: ${{ inputs.system-prompt-file }} run: | ARGS="" if [ "${{ inputs.dry-run }}" = "true" ]; then diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 4ada9a6..b429c56 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -32,6 +32,10 @@ jobs: - name: gpt token_secret: GPT_REVIEW_TOKEN model: gpt-4.1 + - name: security + token_secret: SONNET_REVIEW_TOKEN + model: gpt-5 + system_prompt_file: SECURITY_REVIEW.md steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 @@ -44,6 +48,7 @@ jobs: GITEA_REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }} + REVIEWER_NAME: ${{ matrix.name }} LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }} LLM_API_KEY: ${{ secrets.LLM_API_KEY }} LLM_MODEL: ${{ matrix.model }} @@ -51,4 +56,5 @@ jobs: PATTERNS_REPO: "rodin/go-patterns" PATTERNS_FILES: "README.md,patterns/" LLM_TIMEOUT: "600" + SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }} run: ./review-bot diff --git a/SECURITY_REVIEW.md b/SECURITY_REVIEW.md new file mode 100644 index 0000000..4b2566f --- /dev/null +++ b/SECURITY_REVIEW.md @@ -0,0 +1,18 @@ +You are performing a security-focused code review. Your primary concern is identifying vulnerabilities, not general code quality. + +Focus areas: +- **Injection attacks**: SQL injection, command injection, path traversal, template injection +- **Authentication/Authorization**: Missing auth checks, privilege escalation, IDOR +- **Secrets exposure**: Hardcoded credentials, API keys in code, tokens in logs +- **Input validation**: Untrusted input used without sanitization, unsafe deserialization +- **Cryptography**: Weak algorithms, predictable randomness, improper key management +- **Error handling**: Information leakage in error messages, stack traces exposed +- **Dependencies**: Known vulnerable patterns, unsafe use of external libraries +- **Race conditions**: TOCTOU bugs, unsynchronized shared state +- **Resource exhaustion**: Unbounded allocations, missing timeouts, denial-of-service vectors + +Rules for this review: +- Only report findings with actual security implications. Ignore style, naming, and general code quality. +- Severity mapping: MAJOR = exploitable vulnerability or data exposure. MINOR = defense-in-depth improvement or hardening opportunity. NIT = theoretical concern with low practical risk. +- If the code has no security-relevant changes, APPROVE with an empty findings list. +- Do not duplicate findings that a standard code review would catch (logic bugs, missing error checks) unless they have a security dimension. diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 098a2da..a99270e 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -30,6 +30,7 @@ func main() { llmAPIKey := flag.String("llm-api-key", envOrDefault("LLM_API_KEY", ""), "LLM API key") llmModel := flag.String("llm-model", envOrDefault("LLM_MODEL", ""), "LLM model name") conventionsFile := flag.String("conventions-file", envOrDefault("CONVENTIONS_FILE", ""), "Conventions file path in repo (e.g. CLAUDE.md)") + systemPromptFile := flag.String("system-prompt-file", envOrDefault("SYSTEM_PROMPT_FILE", ""), "Local file with additional system prompt instructions") patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)") patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo") dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting") @@ -150,9 +151,24 @@ func main() { log.Printf("Loaded patterns from %s (%d bytes)", *patternsRepo, len(patterns)) } + // Step 6b: Load additional system prompt if specified + additionalPrompt := "" + if *systemPromptFile != "" { + data, err := os.ReadFile(*systemPromptFile) + if err != nil { + log.Fatalf("Failed to read system prompt file %q: %v", *systemPromptFile, err) + } + additionalPrompt = string(data) + log.Printf("Loaded system prompt file: %s (%d bytes)", *systemPromptFile, len(additionalPrompt)) + } + // Step 7: Budget-aware prompt assembly + systemBase := review.BuildSystemBase() + if additionalPrompt != "" { + systemBase += "\n\n## Additional Review Instructions\n\n" + additionalPrompt + } sections := budget.Sections{ - SystemBase: review.BuildSystemBase(), + SystemBase: systemBase, Patterns: patterns, Conventions: conventions, FileContext: fileContext, @@ -203,14 +219,15 @@ func main() { } log.Printf("Review posted (id=%d, user=%s)", posted.ID, posted.User.Login) - // Delete stale reviews from this bot if update-existing is enabled - if *updateExisting && posted.User.Login != "" { + // Delete stale reviews from this bot using sentinel matching + sentinel := fmt.Sprintf("", *reviewerName) + if *updateExisting && *reviewerName != "" { reviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) if err != nil { log.Printf("Warning: could not list existing reviews: %v", err) } else { for _, r := range reviews { - if r.User.Login == posted.User.Login && r.ID != posted.ID { + if r.ID != posted.ID && strings.Contains(r.Body, sentinel) { if err := giteaClient.DeleteReview(ctx, owner, repoName, prNumber, r.ID); err != nil { log.Printf("Warning: could not delete old review %d: %v", r.ID, err) } else { diff --git a/gitea/client.go b/gitea/client.go index 75bd096..1b4a472 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -279,6 +279,7 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string // Review represents a pull request review from the Gitea API. type Review struct { ID int64 `json:"id"` + Body string `json:"body"` User struct { Login string `json:"login"` } `json:"user"` diff --git a/review/formatter.go b/review/formatter.go index 4048b58..f0af56d 100644 --- a/review/formatter.go +++ b/review/formatter.go @@ -30,6 +30,8 @@ func FormatMarkdown(result *ReviewResult, reviewerName string) string { if reviewerName != "" { sb.WriteString(fmt.Sprintf("\n---\n*Review by %s*\n", reviewerName)) + // Hidden sentinel for identifying this bot's reviews during cleanup + sb.WriteString(fmt.Sprintf("\n\n", reviewerName)) } return sb.String() diff --git a/review/formatter_test.go b/review/formatter_test.go index f15dc2c..dbc95db 100644 --- a/review/formatter_test.go +++ b/review/formatter_test.go @@ -116,3 +116,21 @@ func TestGiteaEvent(t *testing.T) { } } } + +func TestFormatMarkdown_Sentinel(t *testing.T) { + result := &ReviewResult{ + Verdict: "APPROVE", + Summary: "All good.", + Recommendation: "Merge it.", + } + output := FormatMarkdown(result, "security") + if !strings.Contains(output, "") { + t.Error("expected sentinel comment in output") + } + + // Empty reviewer name should NOT have sentinel + output2 := FormatMarkdown(result, "") + if strings.Contains(output2, "