436e6a8824
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 20s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m11s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m16s
Security (MAJOR): - Add filepath.EvalSymlinks after Clean for system-prompt-file - Re-validate resolved path is still within workspace - Prevents symlink → /etc/shadow exfiltration via malicious repo Worst-wins: - Check BEFORE posting (not after) — no delete+repost dance - Identify sibling bots by <!-- review-bot: prefix in body - Only escalates for bot reviews, not human REQUEST_CHANGES - If sibling bot has REQUEST_CHANGES and we would APPROVE → post REQUEST_CHANGES instead Addresses security review finding #1 (MAJOR) and sonnet finding #1.
151 lines
3.9 KiB
Go
151 lines
3.9 KiB
Go
package main
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"gitea.weiker.me/rodin/review-bot/gitea"
|
|
)
|
|
|
|
func TestValidateReviewerName(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
input string
|
|
wantErr bool
|
|
}{
|
|
{"valid simple", "sonnet", false},
|
|
{"valid with dash", "code-review", false},
|
|
{"valid with underscore", "my_bot", false},
|
|
{"valid alphanumeric", "bot123", false},
|
|
{"valid uppercase", "MyBot", false},
|
|
{"empty is valid", "", false},
|
|
{"invalid html close", "foo-->", true},
|
|
{"invalid space", "my bot", true},
|
|
{"invalid dot", "my.bot", true},
|
|
{"invalid slash", "my/bot", true},
|
|
{"invalid angle", "bot<script>", true},
|
|
{"invalid colon", "bot:name", true},
|
|
}
|
|
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
err := validateReviewerName(tc.input)
|
|
if tc.wantErr && err == nil {
|
|
t.Errorf("expected error for %q, got nil", tc.input)
|
|
}
|
|
if !tc.wantErr && err != nil {
|
|
t.Errorf("expected no error for %q, got %v", tc.input, err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func makeReview(id int64, login, state string, stale bool, body string) gitea.Review {
|
|
r := gitea.Review{
|
|
ID: id,
|
|
Body: body,
|
|
State: state,
|
|
Stale: stale,
|
|
}
|
|
r.User.Login = login
|
|
return r
|
|
}
|
|
|
|
func TestShouldEscalate(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
reviews []gitea.Review
|
|
postedID int64
|
|
postedLogin string
|
|
ownSentinel string
|
|
want bool
|
|
}{
|
|
{
|
|
name: "no reviews",
|
|
reviews: nil,
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: false,
|
|
},
|
|
{
|
|
name: "sibling same user has REQUEST_CHANGES",
|
|
reviews: []gitea.Review{
|
|
makeReview(101, "bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:security -->"),
|
|
},
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: true,
|
|
},
|
|
{
|
|
name: "sibling different user has REQUEST_CHANGES (should NOT escalate)",
|
|
reviews: []gitea.Review{
|
|
makeReview(101, "other-bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:gpt -->"),
|
|
},
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: false,
|
|
},
|
|
{
|
|
name: "same user REQUEST_CHANGES but stale (should NOT escalate)",
|
|
reviews: []gitea.Review{
|
|
makeReview(101, "bot", "REQUEST_CHANGES", true, "old\n<!-- review-bot:security -->"),
|
|
},
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: false,
|
|
},
|
|
{
|
|
name: "same user same sentinel (own stale review, should NOT escalate)",
|
|
reviews: []gitea.Review{
|
|
makeReview(101, "bot", "REQUEST_CHANGES", false, "old\n<!-- review-bot:sonnet -->"),
|
|
},
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: false,
|
|
},
|
|
{
|
|
name: "same user APPROVED sibling (should NOT escalate)",
|
|
reviews: []gitea.Review{
|
|
makeReview(101, "bot", "APPROVED", false, "good\n<!-- review-bot:security -->"),
|
|
},
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: false,
|
|
},
|
|
{
|
|
name: "human REQUEST_CHANGES no sentinel (should NOT escalate)",
|
|
reviews: []gitea.Review{
|
|
makeReview(101, "bot", "REQUEST_CHANGES", false, "please fix this"),
|
|
},
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: false,
|
|
},
|
|
{
|
|
name: "skip own posted ID",
|
|
reviews: []gitea.Review{
|
|
makeReview(100, "bot", "REQUEST_CHANGES", false, "x\n<!-- review-bot:security -->"),
|
|
},
|
|
postedID: 100,
|
|
postedLogin: "bot",
|
|
ownSentinel: "<!-- review-bot:sonnet -->",
|
|
want: false,
|
|
},
|
|
}
|
|
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := shouldEscalate(tc.reviews, tc.postedID, tc.postedLogin, tc.ownSentinel)
|
|
if got != tc.want {
|
|
t.Errorf("shouldEscalate() = %v, want %v", got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|