diff --git a/.gitignore b/.gitignore index 4fd4a1b3..d33e0f17 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,10 @@ tests/harness tests/duckdb.node tests/libduckdb.dylib +# skillwalker eval files +output/ +analytics/ + # ralph loop progress tracking progress.txt deferred.txt diff --git a/evals/architectural-analysis/prompts/prompt-analyze-go-project.md b/evals/architectural-analysis/prompts/prompt-analyze-go-project.md new file mode 100644 index 00000000..6e4b6aa1 --- /dev/null +++ b/evals/architectural-analysis/prompts/prompt-analyze-go-project.md @@ -0,0 +1 @@ +/architectural-analysis internal/runner diff --git a/evals/architectural-analysis/prompts/skill-call-adr-graphql-switch.md b/evals/architectural-analysis/prompts/skill-call-adr-graphql-switch.md new file mode 100644 index 00000000..88427ff6 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-adr-graphql-switch.md @@ -0,0 +1 @@ +create an ADR for our decision to switch from REST to GraphQL \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-analyze-payments-module.md b/evals/architectural-analysis/prompts/skill-call-analyze-payments-module.md new file mode 100644 index 00000000..62c637cb --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-analyze-payments-module.md @@ -0,0 +1 @@ +analyze the architecture of the src/payments module \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-architectural-debt-refactor.md b/evals/architectural-analysis/prompts/skill-call-architectural-debt-refactor.md new file mode 100644 index 00000000..dcd9a3c6 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-architectural-debt-refactor.md @@ -0,0 +1 @@ +I need to understand the architectural debt in lib/auth before we refactor it \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-architecture-book-recommendation.md b/evals/architectural-analysis/prompts/skill-call-architecture-book-recommendation.md new file mode 100644 index 00000000..788c58df --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-architecture-book-recommendation.md @@ -0,0 +1 @@ +what's the best software architecture book for beginners \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-concurrency-data-flow.md b/evals/architectural-analysis/prompts/skill-call-concurrency-data-flow.md new file mode 100644 index 00000000..2e344bdd --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-concurrency-data-flow.md @@ -0,0 +1 @@ +assess the concurrency patterns and data flow in our worker pipeline \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-debug-webhook-errors.md b/evals/architectural-analysis/prompts/skill-call-debug-webhook-errors.md new file mode 100644 index 00000000..bd44bd60 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-debug-webhook-errors.md @@ -0,0 +1 @@ +the payment webhook is returning 500 errors intermittently, figure out why \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-document-notification-system.md b/evals/architectural-analysis/prompts/skill-call-document-notification-system.md new file mode 100644 index 00000000..da8f2e9b --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-document-notification-system.md @@ -0,0 +1 @@ +write documentation for how the notification system works \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-evaluate-api-gateway-coupling.md b/evals/architectural-analysis/prompts/skill-call-evaluate-api-gateway-coupling.md new file mode 100644 index 00000000..1255ee99 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-evaluate-api-gateway-coupling.md @@ -0,0 +1 @@ +evaluate the structural health and coupling in our api gateway directory \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-mermaid-architecture-diagram.md b/evals/architectural-analysis/prompts/skill-call-mermaid-architecture-diagram.md new file mode 100644 index 00000000..472c2978 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-mermaid-architecture-diagram.md @@ -0,0 +1 @@ +write an architecture diagram in Mermaid for the README \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-module-boundaries-dependencies.md b/evals/architectural-analysis/prompts/skill-call-module-boundaries-dependencies.md new file mode 100644 index 00000000..e3263360 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-module-boundaries-dependencies.md @@ -0,0 +1 @@ +review the module boundaries and dependencies in src/core \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-review-branch-code-changes.md b/evals/architectural-analysis/prompts/skill-call-review-branch-code-changes.md new file mode 100644 index 00000000..a5c64e83 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-review-branch-code-changes.md @@ -0,0 +1 @@ +review the code changes on this branch for quality issues \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-setup-directory-structure.md b/evals/architectural-analysis/prompts/skill-call-setup-directory-structure.md new file mode 100644 index 00000000..68cbd346 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-setup-directory-structure.md @@ -0,0 +1 @@ +help me set up the project directory structure for a new microservice \ No newline at end of file diff --git a/evals/architectural-analysis/prompts/skill-call-test-plan-auth-module.md b/evals/architectural-analysis/prompts/skill-call-test-plan-auth-module.md new file mode 100644 index 00000000..0c83ad90 --- /dev/null +++ b/evals/architectural-analysis/prompts/skill-call-test-plan-auth-module.md @@ -0,0 +1 @@ +create a test plan for the user authentication module \ No newline at end of file diff --git a/evals/architectural-analysis/rubrics/architectural-analysis-quality.md b/evals/architectural-analysis/rubrics/architectural-analysis-quality.md new file mode 100644 index 00000000..e433c088 --- /dev/null +++ b/evals/architectural-analysis/rubrics/architectural-analysis-quality.md @@ -0,0 +1,26 @@ +## Rubric: architectural-analysis of go-project scaffold + +### Presence — things the analysis must identify +- The analysis notices the fan-in concurrency pattern in runner.go where three goroutines send results through a shared channel +- The analysis calls out that the goroutines (pi.Compute, fibonacci.Compute, sqrt.Compute) have no way to propagate errors back — if one panics, the runner will deadlock waiting on the channel +- The analysis picks up on the tight coupling between runner.go and the three computation packages, since it directly imports and calls all three +- The analysis mentions the types package and how it acts as the shared contract between runner and the computation modules +- The analysis identifies that there's no context cancellation wired into the goroutines — the context.Background() passed into Run doesn't get forwarded to the computations +- The analysis includes an executive summary that highlights the most critical findings +- The analysis produces a risk assessment that rates findings by severity or impact + +### Specificity — the analysis must be concrete +- Findings reference actual file paths like internal/runner/runner.go or internal/fibonacci/fibonacci.go, not just vague package names +- When discussing the concurrency pattern, the analysis points to the specific channel operations (the make(chan types.Result, 3) and the for range 3 loop) +- The coupling discussion names the specific imports in runner.go that create the dependency fan-out + +### Depth — the analysis must be actionable +- At least one recommendation sketches out what a fix would look like — something like an interface, a registry pattern, or passing context down to the goroutines +- The analysis explains why the missing error propagation is a real risk, not just a style nit — e.g., what actually happens if a goroutine panics +- Architectural recommendations connect back to specific findings rather than offering generic advice like "reduce coupling" + +### Absence — the analysis must not do these things +- The analysis doesn't invent concurrency bugs that aren't there — the channel is buffered correctly and the receive loop count matches the send count, so there's no actual deadlock in the happy path +- The analysis doesn't claim there are data races when the goroutines don't actually share any mutable state +- The analysis doesn't hallucinate files or packages that don't exist in the scaffold +- The analysis doesn't confuse this with a web service or API — it's a CLI calculator, and the analysis should treat it as such diff --git a/evals/architectural-analysis/scaffolds/go-project/cmd/calculator/main.go b/evals/architectural-analysis/scaffolds/go-project/cmd/calculator/main.go new file mode 100644 index 00000000..9c124c51 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/cmd/calculator/main.go @@ -0,0 +1,22 @@ +package main + +import ( + "context" + "fmt" + "log/slog" + "os" + + "github.com/testdouble/calculator/internal/runner" +) + +func main() { + slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, nil))) + + sum, err := runner.Run(context.Background()) + if err != nil { + slog.Error("calculation failed", "error", err) + os.Exit(1) + } + + fmt.Printf("Final sum: %s\n", sum.Text('f', 50)) +} diff --git a/evals/architectural-analysis/scaffolds/go-project/go.mod b/evals/architectural-analysis/scaffolds/go-project/go.mod new file mode 100644 index 00000000..bf3ebfd1 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/go.mod @@ -0,0 +1,11 @@ +module github.com/testdouble/calculator + +go 1.22 + +require github.com/stretchr/testify v1.9.0 + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/evals/architectural-analysis/scaffolds/go-project/go.sum b/evals/architectural-analysis/scaffolds/go-project/go.sum new file mode 100644 index 00000000..60ce688a --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/go.sum @@ -0,0 +1,10 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/fibonacci/fibonacci.go b/evals/architectural-analysis/scaffolds/go-project/internal/fibonacci/fibonacci.go new file mode 100644 index 00000000..42fdf849 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/fibonacci/fibonacci.go @@ -0,0 +1,50 @@ +package fibonacci + +import ( + "log/slog" + "math/big" + + "github.com/testdouble/calculator/internal/types" +) + +// Compute calculates the fibonacci sum and sends the result on the channel. +func Compute(ch chan<- types.Result) { + slog.Info("fibonacci: task started") + value := Calculate() + slog.Info("fibonacci: task complete") + ch <- types.Result{TaskName: "fibonacci", Value: value} +} + +// Calculate computes fib(1) through fib(100) and returns their sum as a *big.Float. +func Calculate() *big.Float { + sum := new(big.Int) + + for i := 1; i <= 100; i++ { + sum.Add(sum, Fib(i)) + + if i%10 == 0 { + slog.Info("fibonacci: progress", "iteration", i, "total", 100) + } + } + + return new(big.Float).SetPrec(512).SetInt(sum) +} + +// Fib returns the nth Fibonacci number using iterative computation. +func Fib(n int) *big.Int { + if n <= 0 { + return big.NewInt(0) + } + if n == 1 || n == 2 { + return big.NewInt(1) + } + + a := big.NewInt(1) + b := big.NewInt(1) + + for i := 3; i <= n; i++ { + a, b = b, new(big.Int).Add(a, b) + } + + return b +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/fibonacci/fibonacci_test.go b/evals/architectural-analysis/scaffolds/go-project/internal/fibonacci/fibonacci_test.go new file mode 100644 index 00000000..aa8fc549 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/fibonacci/fibonacci_test.go @@ -0,0 +1,30 @@ +package fibonacci + +import ( + "math/big" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFibKnownValues(t *testing.T) { + assert.Equal(t, big.NewInt(1), Fib(1)) + assert.Equal(t, big.NewInt(1), Fib(2)) + assert.Equal(t, big.NewInt(55), Fib(10)) + assert.Equal(t, big.NewInt(6765), Fib(20)) +} + +func TestFibHundred(t *testing.T) { + expected := new(big.Int) + expected.SetString("354224848179261915075", 10) + assert.Equal(t, expected, Fib(100)) +} + +func TestFibZero(t *testing.T) { + assert.Equal(t, big.NewInt(0), Fib(0)) +} + +func TestCalculatePositive(t *testing.T) { + result := Calculate() + assert.True(t, result.Sign() > 0, "fibonacci sum should be positive") +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/pi/pi.go b/evals/architectural-analysis/scaffolds/go-project/internal/pi/pi.go new file mode 100644 index 00000000..31b9386a --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/pi/pi.go @@ -0,0 +1,69 @@ +package pi + +import ( + "log/slog" + "math/big" + + "github.com/testdouble/calculator/internal/types" +) + +// Compute calculates pi and sends the result on the channel. +func Compute(ch chan<- types.Result) { + slog.Info("pi: task started") + value := Calculate() + slog.Info("pi: task complete") + ch <- types.Result{TaskName: "pi", Value: value} +} + +// Calculate computes pi to 100 digits of precision using Machin's formula: +// pi/4 = 4*arctan(1/5) - arctan(1/239) +func Calculate() *big.Float { + prec := uint(512) + + // 4 * arctan(1/5) + one := new(big.Float).SetPrec(prec).SetInt64(1) + five := new(big.Float).SetPrec(prec).SetInt64(5) + term1 := arctan(new(big.Float).SetPrec(prec).Quo(one, five), 100, prec) + four := new(big.Float).SetPrec(prec).SetInt64(4) + term1.Mul(four, term1) + + // arctan(1/239) + twoThirtyNine := new(big.Float).SetPrec(prec).SetInt64(239) + term2 := arctan(new(big.Float).SetPrec(prec).Quo(one, twoThirtyNine), 100, prec) + + // pi/4 = term1 - term2 + piOverFour := new(big.Float).SetPrec(prec).Sub(term1, term2) + + // pi = 4 * pi/4 + pi := new(big.Float).SetPrec(prec).Mul(four, piOverFour) + return pi +} + +// arctan computes arctan(x) using the Taylor series: +// arctan(x) = x - x^3/3 + x^5/5 - x^7/7 + ... +func arctan(x *big.Float, iterations int, prec uint) *big.Float { + result := new(big.Float).SetPrec(prec).SetInt64(0) + xSquared := new(big.Float).SetPrec(prec).Mul(x, x) + power := new(big.Float).SetPrec(prec).Copy(x) + sign := int64(1) + + for i := 0; i < iterations; i++ { + denominator := new(big.Float).SetPrec(prec).SetInt64(int64(2*i + 1)) + term := new(big.Float).SetPrec(prec).Quo(power, denominator) + + if sign > 0 { + result.Add(result, term) + } else { + result.Sub(result, term) + } + + sign = -sign + power.Mul(power, xSquared) + + if (i+1)%10 == 0 { + slog.Info("pi: progress", "iteration", i+1, "total", iterations) + } + } + + return result +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/pi/pi_test.go b/evals/architectural-analysis/scaffolds/go-project/internal/pi/pi_test.go new file mode 100644 index 00000000..be0e4432 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/pi/pi_test.go @@ -0,0 +1,20 @@ +package pi + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPiPrecision(t *testing.T) { + result := Calculate() + piStr := result.Text('f', 20) + assert.True(t, strings.HasPrefix(piStr, "3.14159265358979"), + "pi should start with 3.14159265358979, got: %s", piStr) +} + +func TestPiPositive(t *testing.T) { + result := Calculate() + assert.True(t, result.Sign() > 0, "pi should be positive") +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/runner/runner.go b/evals/architectural-analysis/scaffolds/go-project/internal/runner/runner.go new file mode 100644 index 00000000..ba8d9646 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/runner/runner.go @@ -0,0 +1,36 @@ +package runner + +import ( + "context" + "log/slog" + "math/big" + + "github.com/testdouble/calculator/internal/fibonacci" + "github.com/testdouble/calculator/internal/pi" + "github.com/testdouble/calculator/internal/sqrt" + "github.com/testdouble/calculator/internal/types" +) + +// Run launches all computational tasks concurrently, collects their results, +// and returns the final sum. +func Run(ctx context.Context) (*big.Float, error) { + ch := make(chan types.Result, 3) + + go pi.Compute(ch) + go fibonacci.Compute(ch) + go sqrt.Compute(ch) + + sum := new(big.Float).SetPrec(512).SetInt64(0) + + for range 3 { + result := <-ch + slog.Info("received result", + "task", result.TaskName, + "value", result.Value.Text('f', 10), + ) + sum.Add(sum, result.Value) + } + + slog.Info("final sum computed", "value", sum.Text('f', 10)) + return sum, nil +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/runner/runner_test.go b/evals/architectural-analysis/scaffolds/go-project/internal/runner/runner_test.go new file mode 100644 index 00000000..19c5c497 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/runner/runner_test.go @@ -0,0 +1,16 @@ +package runner + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRunReturnsResult(t *testing.T) { + result, err := Run(context.Background()) + require.NoError(t, err) + assert.NotNil(t, result) + assert.True(t, result.Sign() > 0, "final sum should be positive") +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/sqrt/sqrt.go b/evals/architectural-analysis/scaffolds/go-project/internal/sqrt/sqrt.go new file mode 100644 index 00000000..e622e735 --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/sqrt/sqrt.go @@ -0,0 +1,38 @@ +package sqrt + +import ( + "log/slog" + "math" + "math/big" + + "github.com/testdouble/calculator/internal/types" +) + +// Compute calculates the square root sum and sends the result on the channel. +func Compute(ch chan<- types.Result) { + slog.Info("sqrt: task started") + value := Calculate() + slog.Info("sqrt: task complete") + ch <- types.Result{TaskName: "sqrt", Value: value} +} + +// Calculate computes sqrt(1) through sqrt(100) and returns their sum as a *big.Float. +func Calculate() *big.Float { + sum := new(big.Float).SetPrec(512).SetFloat64(0) + + for i := 1; i <= 100; i++ { + val := SqrtOf(i) + sum.Add(sum, new(big.Float).SetPrec(512).SetFloat64(val)) + + if i%10 == 0 { + slog.Info("sqrt: progress", "iteration", i, "total", 100) + } + } + + return sum +} + +// SqrtOf returns the square root of n. +func SqrtOf(n int) float64 { + return math.Sqrt(float64(n)) +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/sqrt/sqrt_test.go b/evals/architectural-analysis/scaffolds/go-project/internal/sqrt/sqrt_test.go new file mode 100644 index 00000000..9158613f --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/sqrt/sqrt_test.go @@ -0,0 +1,26 @@ +package sqrt + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSqrtKnownValues(t *testing.T) { + assert.Equal(t, 1.0, SqrtOf(1)) + assert.Equal(t, 2.0, SqrtOf(4)) + assert.Equal(t, 3.0, SqrtOf(9)) + assert.Equal(t, 10.0, SqrtOf(100)) +} + +func TestCalculateSum(t *testing.T) { + result := Calculate() + sum, _ := result.Float64() + assert.InDelta(t, 671.46, sum, 0.5, + "sum of sqrt(1..100) should be approximately 671.46") +} + +func TestCalculatePositive(t *testing.T) { + result := Calculate() + assert.True(t, result.Sign() > 0, "sqrt sum should be positive") +} diff --git a/evals/architectural-analysis/scaffolds/go-project/internal/types/types.go b/evals/architectural-analysis/scaffolds/go-project/internal/types/types.go new file mode 100644 index 00000000..f5862d5c --- /dev/null +++ b/evals/architectural-analysis/scaffolds/go-project/internal/types/types.go @@ -0,0 +1,9 @@ +package types + +import "math/big" + +// Result carries the output of a computational task. +type Result struct { + TaskName string + Value *big.Float +} diff --git a/evals/architectural-analysis/tests.json b/evals/architectural-analysis/tests.json new file mode 100644 index 00000000..a3706080 --- /dev/null +++ b/evals/architectural-analysis/tests.json @@ -0,0 +1,29 @@ +{ + "plugins": ["han-communication", "han-core", "han-coding"], + "tests": [ + { + "name": "Prompt: /architectural-analysis of go-project runner module", + "type": "skill-prompt", + "promptFile": "prompt-analyze-go-project.md", + "model": "opus", + "scaffold": "go-project", + "expect": [ + { "skill-call": { "skill": "han-coding:architectural-analysis", "expected": true } }, + { "llm-judge": { "rubricFile": "architectural-analysis-quality.md", "model": "opus", "threshold": 0.8 } } + ] + }, + { "name": "Skill Call: analyze payments module architecture", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-analyze-payments-module.md", "model": "opus", "expect": [{ "skill-call": true }] }, + { "name": "Skill Call: evaluate api gateway coupling", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-evaluate-api-gateway-coupling.md", "model": "opus", "expect": [{ "skill-call": true }] }, + { "name": "Skill Call: understand architectural debt before refactor", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-architectural-debt-refactor.md", "model": "opus", "expect": [{ "skill-call": true }] }, + { "name": "Skill Call: review module boundaries and dependencies", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-module-boundaries-dependencies.md", "model": "opus", "expect": [{ "skill-call": true }] }, + { "name": "Skill Call: assess concurrency and data flow", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-concurrency-data-flow.md", "model": "opus", "expect": [{ "skill-call": true }] }, + { "name": "Skill Call: write mermaid architecture diagram", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-mermaid-architecture-diagram.md", "model": "sonnet", "expect": [{ "skill-call": false }] }, + { "name": "Skill Call: architecture book recommendation", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-architecture-book-recommendation.md", "model": "sonnet", "expect": [{ "skill-call": false }] }, + { "name": "Skill Call: set up project directory structure", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-setup-directory-structure.md", "model": "sonnet", "expect": [{ "skill-call": false }] }, + { "name": "Skill Call: review code changes on branch", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-review-branch-code-changes.md", "model": "sonnet", "expect": [{ "skill-call": false }] }, + { "name": "Skill Call: debug intermittent webhook errors", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-debug-webhook-errors.md", "model": "sonnet", "expect": [{ "skill-call": false }] }, + { "name": "Skill Call: create test plan for auth module", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-test-plan-auth-module.md", "model": "sonnet", "expect": [{ "skill-call": false }] }, + { "name": "Skill Call: document notification system", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-document-notification-system.md", "model": "sonnet", "expect": [{ "skill-call": false }] }, + { "name": "Skill Call: create ADR for GraphQL switch", "type": "skill-call", "skillFile": "han-coding:architectural-analysis", "promptFile": "skill-call-adr-graphql-switch.md", "model": "sonnet", "expect": [{ "skill-call": false }] } + ] +} diff --git a/evals/architectural-decision-record/prompts/prompt-architectural-decision-record.md b/evals/architectural-decision-record/prompts/prompt-architectural-decision-record.md new file mode 100644 index 00000000..b8288955 --- /dev/null +++ b/evals/architectural-decision-record/prompts/prompt-architectural-decision-record.md @@ -0,0 +1 @@ +run /architectural-decision-record to record our decision to use PostgreSQL as the primary database diff --git a/evals/architectural-decision-record/prompts/prompt-no-op.md b/evals/architectural-decision-record/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/architectural-decision-record/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/architectural-decision-record/prompts/skill-call-architectural-decision-record.md b/evals/architectural-decision-record/prompts/skill-call-architectural-decision-record.md new file mode 100644 index 00000000..82bdd27d --- /dev/null +++ b/evals/architectural-decision-record/prompts/skill-call-architectural-decision-record.md @@ -0,0 +1 @@ +record the architectural decision to use PostgreSQL as our primary database diff --git a/evals/architectural-decision-record/prompts/skill-call-no-architectural-decision-record.md b/evals/architectural-decision-record/prompts/skill-call-no-architectural-decision-record.md new file mode 100644 index 00000000..04185df8 --- /dev/null +++ b/evals/architectural-decision-record/prompts/skill-call-no-architectural-decision-record.md @@ -0,0 +1 @@ +write a coding standard for error handling in Ruby diff --git a/evals/architectural-decision-record/scaffolds/node-project/package.json b/evals/architectural-decision-record/scaffolds/node-project/package.json new file mode 100644 index 00000000..632b9726 --- /dev/null +++ b/evals/architectural-decision-record/scaffolds/node-project/package.json @@ -0,0 +1,5 @@ +{ + "name": "test-project", + "version": "1.0.0", + "main": "src/index.js" +} diff --git a/evals/architectural-decision-record/scaffolds/node-project/src/index.js b/evals/architectural-decision-record/scaffolds/node-project/src/index.js new file mode 100644 index 00000000..d5346ae4 --- /dev/null +++ b/evals/architectural-decision-record/scaffolds/node-project/src/index.js @@ -0,0 +1,5 @@ +function main() { + console.log("hello"); +} + +module.exports = { main }; diff --git a/evals/architectural-decision-record/tests.json b/evals/architectural-decision-record/tests.json new file mode 100644 index 00000000..fe097cc4 --- /dev/null +++ b/evals/architectural-decision-record/tests.json @@ -0,0 +1,47 @@ +{ + "plugins": ["han-communication", "han-core", "han-documentation"], + "tests": [ + { + "name": "Prompt: /architectural-decision-record", + "type": "skill-prompt", + "promptFile": "prompt-architectural-decision-record.md", + "model": "opus", + "scaffold": "node-project", + "expect": [ + { "result-contains": "ADR" }, + { "skill-call": { "skill": "han-documentation:architectural-decision-record", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "ADR" }, + { "skill-call": { "skill": "han-documentation:architectural-decision-record", "expected": false } } + ] + }, + { + "name": "Skill Call: /architectural-decision-record", + "type": "skill-call", + "model": "opus", + "skillFile": "han-documentation:architectural-decision-record", + "promptFile": "skill-call-architectural-decision-record.md", + "scaffold": "node-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: no architectural-decision-record", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-documentation:architectural-decision-record", + "promptFile": "skill-call-no-architectural-decision-record.md", + "expect": [ + { "skill-call": false } + ] + } + ] +} diff --git a/evals/automated-test-planning/prompts/prompt-automated-test-planning.md b/evals/automated-test-planning/prompts/prompt-automated-test-planning.md new file mode 100644 index 00000000..7eadb58e --- /dev/null +++ b/evals/automated-test-planning/prompts/prompt-automated-test-planning.md @@ -0,0 +1 @@ +run /automated-test-planning on lib/example.rb to identify coverage gaps diff --git a/evals/automated-test-planning/prompts/prompt-no-op.md b/evals/automated-test-planning/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/automated-test-planning/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/automated-test-planning/prompts/skill-call-automated-test-planning.md b/evals/automated-test-planning/prompts/skill-call-automated-test-planning.md new file mode 100644 index 00000000..2ae27e1b --- /dev/null +++ b/evals/automated-test-planning/prompts/skill-call-automated-test-planning.md @@ -0,0 +1 @@ +analyze lib/example.rb and generate a test plan covering the untested code paths diff --git a/evals/automated-test-planning/prompts/skill-call-no-automated-test-planning.md b/evals/automated-test-planning/prompts/skill-call-no-automated-test-planning.md new file mode 100644 index 00000000..824b9cc8 --- /dev/null +++ b/evals/automated-test-planning/prompts/skill-call-no-automated-test-planning.md @@ -0,0 +1 @@ +create an ADR for choosing Ruby as the programming language for this project diff --git a/evals/automated-test-planning/scaffolds/ruby-project/Gemfile b/evals/automated-test-planning/scaffolds/ruby-project/Gemfile new file mode 100644 index 00000000..58688a91 --- /dev/null +++ b/evals/automated-test-planning/scaffolds/ruby-project/Gemfile @@ -0,0 +1,2 @@ +source "https://rubygems.org" +gem "minitest" diff --git a/evals/automated-test-planning/scaffolds/ruby-project/lib/example.rb b/evals/automated-test-planning/scaffolds/ruby-project/lib/example.rb new file mode 100644 index 00000000..acdde6e9 --- /dev/null +++ b/evals/automated-test-planning/scaffolds/ruby-project/lib/example.rb @@ -0,0 +1,14 @@ +puts "hello world!" + +class Calculator + def self.add(*numbers) + puts "Adding numbers: #{numbers.inspect}" + total = 0 + + numbers.forEach do |num| + total = total + 1 + end + + return total + end +end diff --git a/evals/automated-test-planning/tests.json b/evals/automated-test-planning/tests.json new file mode 100644 index 00000000..f1a7f1df --- /dev/null +++ b/evals/automated-test-planning/tests.json @@ -0,0 +1,47 @@ +{ + "plugins": ["han-communication", "han-core", "han-coding"], + "tests": [ + { + "name": "Prompt: /automated-test-planning", + "type": "skill-prompt", + "promptFile": "prompt-automated-test-planning.md", + "model": "opus", + "scaffold": "ruby-project", + "expect": [ + { "result-contains": "Test Plan" }, + { "skill-call": { "skill": "han-coding:automated-test-planning", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "Test Plan" }, + { "skill-call": { "skill": "han-coding:automated-test-planning", "expected": false } } + ] + }, + { + "name": "Skill Call: /automated-test-planning", + "type": "skill-call", + "model": "opus", + "skillFile": "han-coding:automated-test-planning", + "promptFile": "skill-call-automated-test-planning.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: no automated-test-planning", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-coding:automated-test-planning", + "promptFile": "skill-call-no-automated-test-planning.md", + "expect": [ + { "skill-call": false } + ] + } + ] +} diff --git a/evals/code-review/prompts/prompt-code-review-go-security.md b/evals/code-review/prompts/prompt-code-review-go-security.md new file mode 100644 index 00000000..4326f880 --- /dev/null +++ b/evals/code-review/prompts/prompt-code-review-go-security.md @@ -0,0 +1 @@ +Run a /code-review on the Go service in this project with a focus on security vulnerabilities. diff --git a/evals/code-review/prompts/prompt-code-review.md b/evals/code-review/prompts/prompt-code-review.md new file mode 100644 index 00000000..2f7971c6 --- /dev/null +++ b/evals/code-review/prompts/prompt-code-review.md @@ -0,0 +1 @@ +run a /code-review on lib/example.rb diff --git a/evals/code-review/prompts/prompt-no-op.md b/evals/code-review/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/code-review/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/code-review/prompts/skill-call-book-review.md b/evals/code-review/prompts/skill-call-book-review.md new file mode 100644 index 00000000..dced0e0c --- /dev/null +++ b/evals/code-review/prompts/skill-call-book-review.md @@ -0,0 +1 @@ +review the book, "code", from microsoft press diff --git a/evals/code-review/prompts/skill-call-code-review.md b/evals/code-review/prompts/skill-call-code-review.md new file mode 100644 index 00000000..924be695 --- /dev/null +++ b/evals/code-review/prompts/skill-call-code-review.md @@ -0,0 +1 @@ +review the code in lib/example.rb diff --git a/evals/code-review/prompts/skill-call-lack-of-word.md b/evals/code-review/prompts/skill-call-lack-of-word.md new file mode 100644 index 00000000..f92fe7bb --- /dev/null +++ b/evals/code-review/prompts/skill-call-lack-of-word.md @@ -0,0 +1 @@ +make sure the code in lib/example.rb follows good coding standards, has been checked for errors, and has enough tests diff --git a/evals/code-review/prompts/skill-call-movie-review.md b/evals/code-review/prompts/skill-call-movie-review.md new file mode 100644 index 00000000..97796da0 --- /dev/null +++ b/evals/code-review/prompts/skill-call-movie-review.md @@ -0,0 +1 @@ +i need a summary movie review and rating for "source code" diff --git a/evals/code-review/prompts/skill-call-no-code-review.md b/evals/code-review/prompts/skill-call-no-code-review.md new file mode 100644 index 00000000..8e4a3b76 --- /dev/null +++ b/evals/code-review/prompts/skill-call-no-code-review.md @@ -0,0 +1 @@ +can you check to see what tests are needed for lib/example.rb? diff --git a/evals/code-review/prompts/skill-call-review-branch.md b/evals/code-review/prompts/skill-call-review-branch.md new file mode 100644 index 00000000..14b92442 --- /dev/null +++ b/evals/code-review/prompts/skill-call-review-branch.md @@ -0,0 +1 @@ +review the changes in this branch diff --git a/evals/code-review/rubrics/code-review-quality.md b/evals/code-review/rubrics/code-review-quality.md new file mode 100644 index 00000000..69c52406 --- /dev/null +++ b/evals/code-review/rubrics/code-review-quality.md @@ -0,0 +1,24 @@ +## Rubric: code-review of ruby-project scaffold + +### Presence — things the review must identify +- The review identifies that `forEach` is not a valid Ruby method and should be `each` (lib/example.rb, line 8) +- The review identifies that `total + 1` should be `total + num` — the method adds 1 per element instead of summing the actual values (lib/example.rb, line 9) +- The review identifies core tests need to be written for the example.rb file +- The review identifies edge case tests that need to be written for the example.rb file +- The review has at least one "Critical" finding +- The review has at least one "Warning" finding +- The review has at least one "Suggestion" finding +- The review has a Review Summary table +- The review has a Review Recommendation stating that at least one item must be fixed + +### Specificity — the review must be concrete +- Each identified issue references the file name `example.rb` or `lib/example.rb` +- The review identifies that the manual iteration and incrementing total should be a `.reduce` call instead + +### Depth — the review must be actionable +- At least one finding includes a suggested fix showing the corrected code +- At least one finding should suggest removing code that appears to be for debugging + +### Absence — the review must not do these things +- The review does not hallucinate bugs or issues that are not present in the scaffold code +- The review does not suggest changes that are not idiomatic ruby diff --git a/evals/code-review/rubrics/code-review-security-quality.md b/evals/code-review/rubrics/code-review-security-quality.md new file mode 100644 index 00000000..1cf8ebf5 --- /dev/null +++ b/evals/code-review/rubrics/code-review-security-quality.md @@ -0,0 +1,29 @@ +## Rubric: code-review of go-security-project scaffold + +### Presence — things the review must identify +- The review identifies that `GetUser` in `internal/db/users.go` builds the SQL query via string concatenation, making it vulnerable to SQL injection +- The review identifies that `internal/db/db.go` hardcodes production database credentials in a source-committed constant +- The review identifies that `jwtKey` in `internal/auth/jwt.go` is set to the short, predictable value `"secret"`, making tokens susceptible to brute-force key recovery +- The review identifies that `GenerateToken` in `internal/auth/jwt.go` creates claims without an `"exp"` field, meaning tokens never expire +- The review identifies that `FetchURL` in `internal/handlers/fetch.go` passes a user-supplied URL directly to `http.Get` without any validation, enabling SSRF +- The review identifies that `responseCache` in `internal/runner/runner.go` is a plain `map[string][]byte` accessed concurrently by 4 goroutines in `Start`, without a mutex or `sync.Map` +- The review identifies that `ValidatePassword` in `internal/auth/auth.go` compares password hashes with `==`, which is not constant-time and enables timing side-channel attacks +- The review produces a Security Vulnerabilities section with findings formatted as SEC-### entries +- Each SEC finding should include an OWASP category +- Each SEC finding must include an EXPLOIT field with a concrete attack path + +### Specificity — the review must be concrete +- The SQL injection finding names the `GetUser` function and shows the concatenated query string in `internal/db/users.go` +- The SSRF finding names a concrete reachable target, such as `http://169.254.169.254/latest/meta-data/` (cloud instance metadata) or an internal service address +- The timing attack finding names `crypto/subtle.ConstantTimeCompare` as the required fix +- The race condition finding references the `responseCache` variable in `internal/runner/runner.go` specifically + +### Depth — the review must be actionable +- The SQL injection finding shows the corrected form using a parameterized query, consistent with the safe pattern already present in `GetUserByID`: `DB.QueryRow("... WHERE name = $1", name)` +- The timing attack finding explains the exploit mechanism: an attacker can measure response-time differences across requests to determine whether a username is valid or to extract information about the stored hash +- The JWT weakness finding explains the operational consequence of missing expiry: once issued, a compromised token remains valid indefinitely with no way to revoke it short of a server-side blocklist + +### Absence — the review must not do these things +- The review does not flag the buffered channel fan-in pattern in `runner.go`'s `Start` function as a race condition — the goroutines communicate only via the correctly-sized channel, and the race exists in `responseCache`, not in the channel mechanics +- The review does not hallucinate vulnerabilities not present in the scaffold code +- The review does not claim that `database/sql` usage is itself the vulnerability — the flaw is the string concatenation in `GetUser`, not the library diff --git a/evals/code-review/scaffolds/go-security-project/cmd/server/main.go b/evals/code-review/scaffolds/go-security-project/cmd/server/main.go new file mode 100644 index 00000000..234a54d3 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/cmd/server/main.go @@ -0,0 +1,49 @@ +package main + +import ( + "context" + "log" + "net/http" + "os" + "os/signal" + "syscall" + + "github.com/testdouble/userservice/internal/db" + "github.com/testdouble/userservice/internal/handlers" + "github.com/testdouble/userservice/internal/runner" +) + +func main() { + if err := db.Connect(); err != nil { + log.Printf("database connection failed: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go runner.Start(ctx) + + mux := http.NewServeMux() + mux.HandleFunc("/users", handlers.GetUser) + mux.HandleFunc("/login", handlers.Login) + mux.HandleFunc("/fetch", handlers.FetchURL) + + server := &http.Server{ + Addr: ":8080", + Handler: mux, + } + + go func() { + sig := make(chan os.Signal, 1) + signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM) + <-sig + cancel() + server.Close() + }() + + log.Println("Starting userservice on :8080") + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Printf("server error: %v", err) + os.Exit(1) + } +} diff --git a/evals/code-review/scaffolds/go-security-project/go.mod b/evals/code-review/scaffolds/go-security-project/go.mod new file mode 100644 index 00000000..1c4a23a5 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/go.mod @@ -0,0 +1,5 @@ +module github.com/testdouble/userservice + +go 1.22 + +require github.com/golang-jwt/jwt/v5 v5.2.1 diff --git a/evals/code-review/scaffolds/go-security-project/go.sum b/evals/code-review/scaffolds/go-security-project/go.sum new file mode 100644 index 00000000..f56d3e63 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/go.sum @@ -0,0 +1,2 @@ +github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= diff --git a/evals/code-review/scaffolds/go-security-project/internal/auth/auth.go b/evals/code-review/scaffolds/go-security-project/internal/auth/auth.go new file mode 100644 index 00000000..2d5aa2d6 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/auth/auth.go @@ -0,0 +1,21 @@ +package auth + +import ( + "crypto/sha256" + "errors" + "fmt" +) + +// HashPassword creates a SHA-256 digest of the given password string. +func HashPassword(password string) string { + sum := sha256.Sum256([]byte(password)) + return fmt.Sprintf("%x", sum) +} + +// ValidatePassword verifies that the provided password matches the stored hash. +func ValidatePassword(provided, stored string) error { + if HashPassword(provided) == stored { + return nil + } + return errors.New("invalid credentials") +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/auth/jwt.go b/evals/code-review/scaffolds/go-security-project/internal/auth/jwt.go new file mode 100644 index 00000000..202e7b22 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/auth/jwt.go @@ -0,0 +1,38 @@ +package auth + +import ( + "fmt" + + "github.com/golang-jwt/jwt/v5" +) + +// jwtKey is the HMAC signing key for all JWT tokens issued by this service. +var jwtKey = []byte("secret") + +// GenerateToken creates a signed JWT for the given user ID and role. +func GenerateToken(userID int, role string) (string, error) { + claims := jwt.MapClaims{ + "user_id": userID, + "role": role, + } + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + return token.SignedString(jwtKey) +} + +// ValidateToken parses and validates a JWT token string, returning its claims. +func ValidateToken(tokenStr string) (jwt.MapClaims, error) { + token, err := jwt.Parse(tokenStr, func(t *jwt.Token) (interface{}, error) { + if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"]) + } + return jwtKey, nil + }) + if err != nil { + return nil, err + } + claims, ok := token.Claims.(jwt.MapClaims) + if !ok || !token.Valid { + return nil, fmt.Errorf("invalid token") + } + return claims, nil +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/db/db.go b/evals/code-review/scaffolds/go-security-project/internal/db/db.go new file mode 100644 index 00000000..302ca200 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/db/db.go @@ -0,0 +1,22 @@ +package db + +import ( + "database/sql" + "fmt" +) + +// connStr contains the production database credentials. +const connStr = "postgres://admin:Password123!@prod-db.internal:5432/userservice" + +// DB is the shared database connection used by all query functions. +var DB *sql.DB + +// Connect opens the database connection using the configured credentials. +func Connect() error { + var err error + DB, err = sql.Open("postgres", connStr) + if err != nil { + return fmt.Errorf("opening database: %w", err) + } + return nil +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/db/users.go b/evals/code-review/scaffolds/go-security-project/internal/db/users.go new file mode 100644 index 00000000..7d77b291 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/db/users.go @@ -0,0 +1,30 @@ +package db + +import ( + "fmt" + + "github.com/testdouble/userservice/internal/types" +) + +// GetUser retrieves a user record by name. +func GetUser(name string) (*types.User, error) { + query := "SELECT id, name, email, password, role FROM users WHERE name = '" + name + "'" + row := DB.QueryRow(query) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} + +// GetUserByID retrieves a user record by numeric ID. +func GetUserByID(id int) (*types.User, error) { + row := DB.QueryRow("SELECT id, name, email, password, role FROM users WHERE id = $1", id) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/handlers/fetch.go b/evals/code-review/scaffolds/go-security-project/internal/handlers/fetch.go new file mode 100644 index 00000000..17dd901e --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/handlers/fetch.go @@ -0,0 +1,27 @@ +package handlers + +import ( + "io" + "net/http" +) + +// FetchURL proxies content from a remote URL on behalf of the client. +// Used by the frontend to load external resources through the server. +func FetchURL(w http.ResponseWriter, r *http.Request) { + url := r.URL.Query().Get("url") + if url == "" { + http.Error(w, "url parameter required", http.StatusBadRequest) + return + } + + resp, err := http.Get(url) + if err != nil { + http.Error(w, "fetch failed", http.StatusBadGateway) + return + } + defer resp.Body.Close() + + w.Header().Set("Content-Type", resp.Header.Get("Content-Type")) + w.WriteHeader(resp.StatusCode) + io.Copy(w, resp.Body) +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/handlers/users.go b/evals/code-review/scaffolds/go-security-project/internal/handlers/users.go new file mode 100644 index 00000000..85fcc1a6 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/handlers/users.go @@ -0,0 +1,59 @@ +package handlers + +import ( + "encoding/json" + "net/http" + + "github.com/testdouble/userservice/internal/auth" + "github.com/testdouble/userservice/internal/db" +) + +// GetUser handles GET /users?name= and returns the matching user record. +func GetUser(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + if name == "" { + http.Error(w, "name parameter required", http.StatusBadRequest) + return + } + + user, err := db.GetUser(name) + if err != nil { + http.Error(w, "user not found", http.StatusNotFound) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) +} + +// Login handles POST /login with JSON credentials and returns a JWT on success. +func Login(w http.ResponseWriter, r *http.Request) { + var creds struct { + Username string `json:"username"` + Password string `json:"password"` + } + if err := json.NewDecoder(r.Body).Decode(&creds); err != nil { + http.Error(w, "invalid request body", http.StatusBadRequest) + return + } + + user, err := db.GetUser(creds.Username) + if err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + if err := auth.ValidatePassword(creds.Password, user.Password); err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + token, err := auth.GenerateToken(user.ID, user.Role) + if err != nil { + http.Error(w, "failed to generate token", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]string{"token": token}) +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/runner/runner.go b/evals/code-review/scaffolds/go-security-project/internal/runner/runner.go new file mode 100644 index 00000000..518d70c7 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/runner/runner.go @@ -0,0 +1,52 @@ +package runner + +import ( + "context" + "log" + + "github.com/testdouble/userservice/internal/types" +) + +// responseCache stores the results of processed requests to avoid redundant work. +var responseCache = make(map[string][]byte) + +// processRequest handles a single unit of work and caches the result. +func processRequest(data types.RequestData) { + if _, ok := responseCache[data.Action]; ok { + return + } + result := []byte("processed:" + data.Action) + responseCache[data.Action] = result +} + +// getCached retrieves a previously cached result for the given action key. +func getCached(key string) ([]byte, bool) { + val, ok := responseCache[key] + return val, ok +} + +// Start launches a pool of worker goroutines to process incoming requests. +// The pool runs until the provided context is cancelled. +func Start(ctx context.Context) { + work := make(chan types.RequestData, 10) + + for i := 0; i < 4; i++ { + go func() { + for { + select { + case <-ctx.Done(): + return + case data, ok := <-work: + if !ok { + return + } + processRequest(data) + } + } + }() + } + + log.Println("Runner started with 4 workers") + <-ctx.Done() + close(work) +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/runner/runner_test.go b/evals/code-review/scaffolds/go-security-project/internal/runner/runner_test.go new file mode 100644 index 00000000..7ba5d298 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/runner/runner_test.go @@ -0,0 +1,27 @@ +package runner_test + +import ( + "context" + "testing" + "time" + + "github.com/testdouble/userservice/internal/runner" +) + +func TestStart(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + done := make(chan struct{}) + go func() { + runner.Start(ctx) + close(done) + }() + + select { + case <-done: + // success: Start returned after context cancellation + case <-time.After(500 * time.Millisecond): + t.Error("Start did not return after context cancellation") + } +} diff --git a/evals/code-review/scaffolds/go-security-project/internal/types/types.go b/evals/code-review/scaffolds/go-security-project/internal/types/types.go new file mode 100644 index 00000000..09e4d1f3 --- /dev/null +++ b/evals/code-review/scaffolds/go-security-project/internal/types/types.go @@ -0,0 +1,17 @@ +package types + +// User represents an authenticated user in the system. +type User struct { + ID int `json:"id"` + Name string `json:"name"` + Email string `json:"email"` + Password string `json:"-"` + Role string `json:"role"` +} + +// RequestData holds a unit of work to be processed by the runner. +type RequestData struct { + UserID int + Action string + Body []byte +} diff --git a/evals/code-review/scaffolds/ruby-project/Gemfile b/evals/code-review/scaffolds/ruby-project/Gemfile new file mode 100644 index 00000000..58688a91 --- /dev/null +++ b/evals/code-review/scaffolds/ruby-project/Gemfile @@ -0,0 +1,2 @@ +source "https://rubygems.org" +gem "minitest" diff --git a/evals/code-review/scaffolds/ruby-project/lib/example.rb b/evals/code-review/scaffolds/ruby-project/lib/example.rb new file mode 100644 index 00000000..acdde6e9 --- /dev/null +++ b/evals/code-review/scaffolds/ruby-project/lib/example.rb @@ -0,0 +1,14 @@ +puts "hello world!" + +class Calculator + def self.add(*numbers) + puts "Adding numbers: #{numbers.inspect}" + total = 0 + + numbers.forEach do |num| + total = total + 1 + end + + return total + end +end diff --git a/evals/code-review/tests.json b/evals/code-review/tests.json new file mode 100644 index 00000000..528a6d75 --- /dev/null +++ b/evals/code-review/tests.json @@ -0,0 +1,94 @@ +{ + "plugins": ["han-communication", "han-core", "han-coding"], + "tests": [ + { + "name": "Prompt: /code-review simple ruby file", + "type": "skill-prompt", + "promptFile": "prompt-code-review.md", + "model": "opus", + "scaffold": "ruby-project", + "expect": [ + { "result-contains": "# Code Review:" }, + { "skill-call": { "skill": "han-coding:code-review", "expected": true } }, + { "llm-judge": { "rubricFile": "code-review-quality.md", "model": "opus", "threshold": 0.8 } } + ] + }, + { + "name": "Prompt: /code-review go security scaffold", + "type": "skill-prompt", + "promptFile": "prompt-code-review-go-security.md", + "model": "opus", + "scaffold": "go-security-project", + "expect": [ + { "result-contains": "# Code Review:" }, + { "skill-call": { "skill": "han-coding:code-review", "expected": true } }, + { "llm-judge": { "rubricFile": "code-review-security-quality.md", "model": "opus", "threshold": 0.8 } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "# Code Review:" }, + { "skill-call": { "skill": "han-coding:code-review", "expected": false } } + ] + }, + { + "name": "Skill Call: /code-review", + "type": "skill-call", + "model": "opus", + "skillFile": "han-coding:code-review", + "promptFile": "skill-call-code-review.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: don't include the word 'review'", + "type": "skill-call", + "promptFile": "skill-call-lack-of-word.md", + "skillFile": "han-coding:code-review", + "model": "opus", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: movie review", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-coding:code-review", + "promptFile": "skill-call-movie-review.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: book review", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-coding:code-review", + "promptFile": "skill-call-book-review.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: review changes in branch", + "type": "skill-call", + "promptFile": "skill-call-review-branch.md", + "skillFile": "han-coding:code-review", + "model": "opus", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + } + ] +} diff --git a/evals/coding-standard/prompts/prompt-api-response-conventions.md b/evals/coding-standard/prompts/prompt-api-response-conventions.md new file mode 100644 index 00000000..b978e93f --- /dev/null +++ b/evals/coding-standard/prompts/prompt-api-response-conventions.md @@ -0,0 +1 @@ +run /coding-standard to create a standard for API response conventions in this Rails project diff --git a/evals/coding-standard/prompts/prompt-coding-standard.md b/evals/coding-standard/prompts/prompt-coding-standard.md new file mode 100644 index 00000000..cc9b467a --- /dev/null +++ b/evals/coding-standard/prompts/prompt-coding-standard.md @@ -0,0 +1 @@ +run /coding-standard to create a standard for error handling in lib/example.rb diff --git a/evals/coding-standard/prompts/prompt-controller-patterns.md b/evals/coding-standard/prompts/prompt-controller-patterns.md new file mode 100644 index 00000000..3f3bb8bb --- /dev/null +++ b/evals/coding-standard/prompts/prompt-controller-patterns.md @@ -0,0 +1 @@ +run /coding-standard to create a standard for controller design patterns in this Rails API project diff --git a/evals/coding-standard/prompts/prompt-no-op.md b/evals/coding-standard/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/coding-standard/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/coding-standard/prompts/skill-call-coding-standard.md b/evals/coding-standard/prompts/skill-call-coding-standard.md new file mode 100644 index 00000000..04185df8 --- /dev/null +++ b/evals/coding-standard/prompts/skill-call-coding-standard.md @@ -0,0 +1 @@ +write a coding standard for error handling in Ruby diff --git a/evals/coding-standard/prompts/skill-call-no-coding-standard.md b/evals/coding-standard/prompts/skill-call-no-coding-standard.md new file mode 100644 index 00000000..0336a2d4 --- /dev/null +++ b/evals/coding-standard/prompts/skill-call-no-coding-standard.md @@ -0,0 +1 @@ +create an ADR for choosing PostgreSQL as our database diff --git a/evals/coding-standard/rubrics/coding-standard-quality.md b/evals/coding-standard/rubrics/coding-standard-quality.md new file mode 100644 index 00000000..bc3a5bbb --- /dev/null +++ b/evals/coding-standard/rubrics/coding-standard-quality.md @@ -0,0 +1,21 @@ +## Rubric: coding-standard of rails-api-project scaffold + +### Presence — things the coding standard must identify +- The coding standard identifies that business logic (such as the pi calculation) should not live directly in controller actions +- The coding standard identifies the service object pattern (FibonacciCalculator) as the preferred approach for extracting business logic from controllers +- The coding standard identifies inconsistent response formats across API endpoints (e.g., result vs value keys) +- The coding standard identifies the need for input validation on controller parameters + +### Specificity — the coding standard must be concrete +- The coding standard references specific files from the project (e.g., pi_controller.rb, fibonacci_controller.rb, fibonacci_calculator.rb) +- The coding standard includes code examples showing both correct and incorrect patterns + +### Depth — the coding standard must be actionable +- The coding standard provides "correct usage" examples that demonstrate the preferred pattern +- The coding standard provides "what to avoid" examples that show the anti-pattern +- The coding standard explains why extracting logic to service objects is preferred over inline controller logic + +### Absence — the coding standard must not do these things +- The coding standard does not hallucinate files or classes that do not exist in the project +- The coding standard does not suggest database-related or ORM-related conventions (this project has no database) +- The coding standard does not recommend linter/formatter rules as coding standard content diff --git a/evals/coding-standard/scaffolds/rails-api-project/Gemfile b/evals/coding-standard/scaffolds/rails-api-project/Gemfile new file mode 100644 index 00000000..e94bf899 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/Gemfile @@ -0,0 +1,10 @@ +source "https://rubygems.org" + +ruby "3.2.2" + +gem "rails", "~> 7.1" +gem "puma", "~> 6.0" + +group :development, :test do + gem "rspec-rails", "~> 6.0" +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/app/controllers/application_controller.rb b/evals/coding-standard/scaffolds/rails-api-project/app/controllers/application_controller.rb new file mode 100644 index 00000000..4ac8823b --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/app/controllers/application_controller.rb @@ -0,0 +1,2 @@ +class ApplicationController < ActionController::API +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/app/controllers/fibonacci_controller.rb b/evals/coding-standard/scaffolds/rails-api-project/app/controllers/fibonacci_controller.rb new file mode 100644 index 00000000..f01a9cb4 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/app/controllers/fibonacci_controller.rb @@ -0,0 +1,9 @@ +class FibonacciController < ApplicationController + def calculate + n = params[:n] + calculator = FibonacciCalculator.new + number = calculator.compute(n.to_i) + + render json: { value: number } + end +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/app/controllers/pi_controller.rb b/evals/coding-standard/scaffolds/rails-api-project/app/controllers/pi_controller.rb new file mode 100644 index 00000000..6e914738 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/app/controllers/pi_controller.rb @@ -0,0 +1,14 @@ +class PiController < ApplicationController + def calculate + # Compute pi to 100 decimal places using the Bailey-Borwein-Plouffe formula + # with Ruby's BigDecimal for arbitrary precision + require "bigdecimal" + require "bigdecimal/math" + + precision = 120 + pi = BigMath.PI(precision) + rounded = pi.round(100).to_s("F") + + render json: { result: rounded } + end +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/app/services/fibonacci_calculator.rb b/evals/coding-standard/scaffolds/rails-api-project/app/services/fibonacci_calculator.rb new file mode 100644 index 00000000..fcf20bc5 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/app/services/fibonacci_calculator.rb @@ -0,0 +1,13 @@ +class FibonacciCalculator + def compute(n) + return n if n <= 1 + + a = 0 + b = 1 + (n - 1).times do + a, b = b, a + b + end + + b + end +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/config.ru b/evals/coding-standard/scaffolds/rails-api-project/config.ru new file mode 100644 index 00000000..79630bc7 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/config.ru @@ -0,0 +1,3 @@ +require_relative "config/environment" + +run Rails.application diff --git a/evals/coding-standard/scaffolds/rails-api-project/config/application.rb b/evals/coding-standard/scaffolds/rails-api-project/config/application.rb new file mode 100644 index 00000000..cd3d34a3 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/config/application.rb @@ -0,0 +1,12 @@ +require_relative "boot" +require "rails" +require "action_controller/railtie" + +module PiAndFibonacci + class Application < Rails::Application + config.load_defaults 7.1 + config.api_only = true + + config.eager_load = false + end +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/config/boot.rb b/evals/coding-standard/scaffolds/rails-api-project/config/boot.rb new file mode 100644 index 00000000..93f96d57 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/config/boot.rb @@ -0,0 +1,3 @@ +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) + +require "bundler/setup" diff --git a/evals/coding-standard/scaffolds/rails-api-project/config/environment.rb b/evals/coding-standard/scaffolds/rails-api-project/config/environment.rb new file mode 100644 index 00000000..73a3979b --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/config/environment.rb @@ -0,0 +1,3 @@ +require_relative "application" + +Rails.application.initialize! diff --git a/evals/coding-standard/scaffolds/rails-api-project/config/environments/test.rb b/evals/coding-standard/scaffolds/rails-api-project/config/environments/test.rb new file mode 100644 index 00000000..152f9d6f --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/config/environments/test.rb @@ -0,0 +1,7 @@ +require "active_support/core_ext/integer/time" + +Rails.application.configure do + config.cache_classes = true + config.eager_load = false + config.consider_all_requests_local = true +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/config/routes.rb b/evals/coding-standard/scaffolds/rails-api-project/config/routes.rb new file mode 100644 index 00000000..47057aac --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/config/routes.rb @@ -0,0 +1,4 @@ +Rails.application.routes.draw do + get "/pi", to: "pi#calculate" + get "/fibonacci", to: "fibonacci#calculate" +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/spec/rails_helper.rb b/evals/coding-standard/scaffolds/rails-api-project/spec/rails_helper.rb new file mode 100644 index 00000000..fe92b002 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/spec/rails_helper.rb @@ -0,0 +1,9 @@ +require "spec_helper" +ENV["RAILS_ENV"] ||= "test" +require_relative "../config/environment" +require "rspec/rails" + +RSpec.configure do |config| + config.infer_spec_type_from_file_location! + config.filter_rails_from_backtrace! +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/spec/requests/fibonacci_spec.rb b/evals/coding-standard/scaffolds/rails-api-project/spec/requests/fibonacci_spec.rb new file mode 100644 index 00000000..d38af16f --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/spec/requests/fibonacci_spec.rb @@ -0,0 +1,12 @@ +require "rails_helper" + +RSpec.describe "GET /fibonacci", type: :request do + it "returns the 10th fibonacci number" do + get "/fibonacci", params: { n: 10 } + + expect(response).to have_http_status(:ok) + + json = JSON.parse(response.body) + expect(json["value"]).to eq(55) + end +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/spec/requests/pi_spec.rb b/evals/coding-standard/scaffolds/rails-api-project/spec/requests/pi_spec.rb new file mode 100644 index 00000000..f9e8277a --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/spec/requests/pi_spec.rb @@ -0,0 +1,12 @@ +require "rails_helper" + +RSpec.describe "GET /pi", type: :request do + it "returns pi to 100 decimal places" do + get "/pi" + + expect(response).to have_http_status(:ok) + + json = JSON.parse(response.body) + expect(json["result"]).to start_with("3.1415926535") + end +end diff --git a/evals/coding-standard/scaffolds/rails-api-project/spec/spec_helper.rb b/evals/coding-standard/scaffolds/rails-api-project/spec/spec_helper.rb new file mode 100644 index 00000000..06e09a30 --- /dev/null +++ b/evals/coding-standard/scaffolds/rails-api-project/spec/spec_helper.rb @@ -0,0 +1,11 @@ +RSpec.configure do |config| + config.expect_with :rspec do |expectations| + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + end + + config.mock_with :rspec do |mocks| + mocks.verify_partial_doubles = true + end + + config.shared_context_metadata_behavior = :apply_to_host_groups +end diff --git a/evals/coding-standard/scaffolds/ruby-project/Gemfile b/evals/coding-standard/scaffolds/ruby-project/Gemfile new file mode 100644 index 00000000..58688a91 --- /dev/null +++ b/evals/coding-standard/scaffolds/ruby-project/Gemfile @@ -0,0 +1,2 @@ +source "https://rubygems.org" +gem "minitest" diff --git a/evals/coding-standard/scaffolds/ruby-project/lib/example.rb b/evals/coding-standard/scaffolds/ruby-project/lib/example.rb new file mode 100644 index 00000000..acdde6e9 --- /dev/null +++ b/evals/coding-standard/scaffolds/ruby-project/lib/example.rb @@ -0,0 +1,14 @@ +puts "hello world!" + +class Calculator + def self.add(*numbers) + puts "Adding numbers: #{numbers.inspect}" + total = 0 + + numbers.forEach do |num| + total = total + 1 + end + + return total + end +end diff --git a/evals/coding-standard/tests.json b/evals/coding-standard/tests.json new file mode 100644 index 00000000..b62ba76b --- /dev/null +++ b/evals/coding-standard/tests.json @@ -0,0 +1,70 @@ +{ + "plugins": ["han-communication", "han-core", "han-coding"], + "tests": [ + { + "name": "Prompt: controller design patterns", + "type": "skill-prompt", + "promptFile": "prompt-controller-patterns.md", + "model": "opus", + "scaffold": "rails-api-project", + "expect": [ + { "skill-call": { "skill": "han-coding:coding-standard", "expected": true } }, + { "llm-judge": { "rubricFile": "coding-standard-quality.md", "model": "opus", "threshold": 0.8 } } + ] + }, + { + "name": "Prompt: API response conventions", + "type": "skill-prompt", + "promptFile": "prompt-api-response-conventions.md", + "model": "opus", + "scaffold": "rails-api-project", + "expect": [ + { "skill-call": { "skill": "han-coding:coding-standard", "expected": true } }, + { "llm-judge": { "rubricFile": "coding-standard-quality.md", "model": "opus", "threshold": 0.8 } } + ] + }, + { + "name": "Prompt: /coding-standard", + "type": "skill-prompt", + "promptFile": "prompt-coding-standard.md", + "model": "opus", + "scaffold": "ruby-project", + "expect": [ + { "result-contains": "coding standard" }, + { "skill-call": { "skill": "han-coding:coding-standard", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "coding standard" }, + { "skill-call": { "skill": "han-coding:coding-standard", "expected": false } } + ] + }, + { + "name": "Skill Call: /coding-standard", + "type": "skill-call", + "model": "opus", + "skillFile": "han-coding:coding-standard", + "promptFile": "skill-call-coding-standard.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: no coding standard", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-coding:coding-standard", + "promptFile": "skill-call-no-coding-standard.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": false } + ] + } + ] +} diff --git a/evals/edge-case-explorer/prompts/agent-call-boundary-values-db-layer.md b/evals/edge-case-explorer/prompts/agent-call-boundary-values-db-layer.md new file mode 100644 index 00000000..42de96ed --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-boundary-values-db-layer.md @@ -0,0 +1 @@ +What boundary values and failure modes should we test for in the database access layer? \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-exhaustive-edge-case-analysis.md b/evals/edge-case-explorer/prompts/agent-call-exhaustive-edge-case-analysis.md new file mode 100644 index 00000000..202d8e34 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-exhaustive-edge-case-analysis.md @@ -0,0 +1 @@ +Do an exhaustive exploration of edge cases for the runner module — check all dimensions including race conditions and error propagation \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-find-edge-cases-auth.md b/evals/edge-case-explorer/prompts/agent-call-find-edge-cases-auth.md new file mode 100644 index 00000000..900a10b1 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-find-edge-cases-auth.md @@ -0,0 +1 @@ +Find the edge cases in the authentication module — what unexpected inputs could break it? \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-how-can-handlers-fail.md b/evals/edge-case-explorer/prompts/agent-call-how-can-handlers-fail.md new file mode 100644 index 00000000..4a36b6c9 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-how-can-handlers-fail.md @@ -0,0 +1 @@ +Explore how the HTTP handlers can fail — trace the inputs and find edge cases we haven't thought of \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-negative-boundary-survey.md b/evals/edge-case-explorer/prompts/agent-call-negative-boundary-survey.md new file mode 100644 index 00000000..036b5ec6 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-negative-boundary-survey.md @@ -0,0 +1 @@ +What are the boundary lines for the property at 123 Main Street? \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-negative-edge-case-interview.md b/evals/edge-case-explorer/prompts/agent-call-negative-edge-case-interview.md new file mode 100644 index 00000000..04a4e5ef --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-negative-edge-case-interview.md @@ -0,0 +1 @@ +Give me some edge case interview questions for a product manager role \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-negative-edge-computing.md b/evals/edge-case-explorer/prompts/agent-call-negative-edge-computing.md new file mode 100644 index 00000000..fede9302 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-negative-edge-computing.md @@ -0,0 +1 @@ +Explain how edge computing works and when to deploy workloads to the edge versus the cloud \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-sibling-code-review.md b/evals/edge-case-explorer/prompts/agent-call-sibling-code-review.md new file mode 100644 index 00000000..31c537be --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-sibling-code-review.md @@ -0,0 +1 @@ +Review the handlers code for quality issues and security vulnerabilities \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-sibling-investigate-crash.md b/evals/edge-case-explorer/prompts/agent-call-sibling-investigate-crash.md new file mode 100644 index 00000000..3dcf04ba --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-sibling-investigate-crash.md @@ -0,0 +1 @@ +Investigate why the server crashes on startup when the DATABASE_URL environment variable is missing \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-sibling-structural-coupling.md b/evals/edge-case-explorer/prompts/agent-call-sibling-structural-coupling.md new file mode 100644 index 00000000..2a3bc11a --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-sibling-structural-coupling.md @@ -0,0 +1 @@ +Analyze the coupling between the internal packages and identify module boundary violations \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-sibling-test-plan.md b/evals/edge-case-explorer/prompts/agent-call-sibling-test-plan.md new file mode 100644 index 00000000..a9f962f3 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-sibling-test-plan.md @@ -0,0 +1 @@ +Plan what tests we need for the auth module — identify untested behaviors and prioritize by risk \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-call-type-coercion-traps.md b/evals/edge-case-explorer/prompts/agent-call-type-coercion-traps.md new file mode 100644 index 00000000..7cfae0d3 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-call-type-coercion-traps.md @@ -0,0 +1 @@ +Look for type coercion traps and unexpected input formats that could cause silent data corruption in the user service \ No newline at end of file diff --git a/evals/edge-case-explorer/prompts/agent-prompt-go-security-edge-cases.md b/evals/edge-case-explorer/prompts/agent-prompt-go-security-edge-cases.md new file mode 100644 index 00000000..a3574987 --- /dev/null +++ b/evals/edge-case-explorer/prompts/agent-prompt-go-security-edge-cases.md @@ -0,0 +1 @@ +Explore the edge cases in this Go project. Trace inputs, find boundary values, type coercion traps, and integration boundary issues. Write the full analysis to edge-case-analysis.md. \ No newline at end of file diff --git a/evals/edge-case-explorer/rubrics/edge-case-explorer-quality.md b/evals/edge-case-explorer/rubrics/edge-case-explorer-quality.md new file mode 100644 index 00000000..66baabdf --- /dev/null +++ b/evals/edge-case-explorer/rubrics/edge-case-explorer-quality.md @@ -0,0 +1,39 @@ +## Rubric: edge-case-explorer of go-security-project scaffold + +### Presence — things the edge case analysis must identify +- The analysis identifies SQL injection edge cases in the database layer where user-controlled input is interpolated into query strings (internal/db/users.go) +- The analysis identifies race condition edge cases in the runner's responseCache where concurrent access can cause data corruption (internal/runner/runner.go) +- The analysis identifies boundary value edge cases for JWT token handling — empty tokens, malformed tokens, expired tokens, tokens with tampered signatures (internal/auth/jwt.go) +- The analysis identifies external input messiness edge cases for HTTP request bodies — missing fields, null values, unexpected types, empty strings (internal/handlers/users.go) +- The analysis identifies error propagation edge cases where database connection failures or query errors may be swallowed or inadequately surfaced to callers +- The analysis includes an Input Source Map table mapping inputs to their origins and validation status + +### Specificity — the edge case analysis must be concrete +- Every edge case finding references a specific file path and line number where the affected code lives +- Each finding specifies a concrete scenario — the exact input value or condition that triggers the edge case, not just a category name +- The analysis traces inputs to their callers to determine what values are actually passed, not just what the parameter types allow +- Findings cite the current handling (or lack thereof) for each edge case with reference to the specific code that does or does not guard against it + +### Depth — the edge case analysis must be actionable +- Each finding includes a severity assessment (Critical/High/Medium/Low) with justification based on likelihood and impact +- Each finding describes the expected correct behavior — what the code should do when the edge case occurs +- Each finding describes the risk — what happens if the edge case is not handled (e.g., silent data corruption, crash, security bypass) +- The analysis distinguishes between edge cases that are already handled in code, partially handled, and completely unhandled + +### Absence — the edge case analysis must not do these things +- The analysis must not report edge cases that are purely theoretical with no realistic path to occurrence in this codebase +- The analysis must not dismiss edge cases with "the framework handles this" without verifying the specific framework version and usage +- The analysis must not inflate priorities — Critical findings should be reserved for likely AND severe AND unhandled scenarios +- The analysis must not write test code or plan overall test coverage — it produces edge case discovery only +- The analysis must not list a dimension as "not applicable" without checking whether the code actually touches that dimension + +## File: edge-case-analysis.md +### Presence +- The file contains a Summary section with a priority count table (Critical, High, Medium, Low) +- The file contains an Input Source Map table with columns for Input, Origin, Type, and Validated status +- The file contains a Findings section with EC-series items grouped by priority +- The file contains a Coverage Summary section breaking down edge cases by discovery status (already tested, handled but untested, no handling and no tests) + +### Specificity +- Each EC-series finding includes Priority, Dimension, Input, Scenario, Code location, Current handling, Expected behavior, and Risk fields +- The Dropped Edge Cases section lists excluded scenarios with specific reasons for exclusion diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/cmd/server/main.go b/evals/edge-case-explorer/scaffolds/go-security-project/cmd/server/main.go new file mode 100644 index 00000000..234a54d3 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/cmd/server/main.go @@ -0,0 +1,49 @@ +package main + +import ( + "context" + "log" + "net/http" + "os" + "os/signal" + "syscall" + + "github.com/testdouble/userservice/internal/db" + "github.com/testdouble/userservice/internal/handlers" + "github.com/testdouble/userservice/internal/runner" +) + +func main() { + if err := db.Connect(); err != nil { + log.Printf("database connection failed: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go runner.Start(ctx) + + mux := http.NewServeMux() + mux.HandleFunc("/users", handlers.GetUser) + mux.HandleFunc("/login", handlers.Login) + mux.HandleFunc("/fetch", handlers.FetchURL) + + server := &http.Server{ + Addr: ":8080", + Handler: mux, + } + + go func() { + sig := make(chan os.Signal, 1) + signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM) + <-sig + cancel() + server.Close() + }() + + log.Println("Starting userservice on :8080") + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Printf("server error: %v", err) + os.Exit(1) + } +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/go.mod b/evals/edge-case-explorer/scaffolds/go-security-project/go.mod new file mode 100644 index 00000000..1c4a23a5 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/go.mod @@ -0,0 +1,5 @@ +module github.com/testdouble/userservice + +go 1.22 + +require github.com/golang-jwt/jwt/v5 v5.2.1 diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/go.sum b/evals/edge-case-explorer/scaffolds/go-security-project/go.sum new file mode 100644 index 00000000..f56d3e63 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/go.sum @@ -0,0 +1,2 @@ +github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/auth/auth.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/auth/auth.go new file mode 100644 index 00000000..2d5aa2d6 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/auth/auth.go @@ -0,0 +1,21 @@ +package auth + +import ( + "crypto/sha256" + "errors" + "fmt" +) + +// HashPassword creates a SHA-256 digest of the given password string. +func HashPassword(password string) string { + sum := sha256.Sum256([]byte(password)) + return fmt.Sprintf("%x", sum) +} + +// ValidatePassword verifies that the provided password matches the stored hash. +func ValidatePassword(provided, stored string) error { + if HashPassword(provided) == stored { + return nil + } + return errors.New("invalid credentials") +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/auth/jwt.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/auth/jwt.go new file mode 100644 index 00000000..202e7b22 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/auth/jwt.go @@ -0,0 +1,38 @@ +package auth + +import ( + "fmt" + + "github.com/golang-jwt/jwt/v5" +) + +// jwtKey is the HMAC signing key for all JWT tokens issued by this service. +var jwtKey = []byte("secret") + +// GenerateToken creates a signed JWT for the given user ID and role. +func GenerateToken(userID int, role string) (string, error) { + claims := jwt.MapClaims{ + "user_id": userID, + "role": role, + } + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + return token.SignedString(jwtKey) +} + +// ValidateToken parses and validates a JWT token string, returning its claims. +func ValidateToken(tokenStr string) (jwt.MapClaims, error) { + token, err := jwt.Parse(tokenStr, func(t *jwt.Token) (interface{}, error) { + if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"]) + } + return jwtKey, nil + }) + if err != nil { + return nil, err + } + claims, ok := token.Claims.(jwt.MapClaims) + if !ok || !token.Valid { + return nil, fmt.Errorf("invalid token") + } + return claims, nil +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/db/db.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/db/db.go new file mode 100644 index 00000000..302ca200 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/db/db.go @@ -0,0 +1,22 @@ +package db + +import ( + "database/sql" + "fmt" +) + +// connStr contains the production database credentials. +const connStr = "postgres://admin:Password123!@prod-db.internal:5432/userservice" + +// DB is the shared database connection used by all query functions. +var DB *sql.DB + +// Connect opens the database connection using the configured credentials. +func Connect() error { + var err error + DB, err = sql.Open("postgres", connStr) + if err != nil { + return fmt.Errorf("opening database: %w", err) + } + return nil +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/db/users.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/db/users.go new file mode 100644 index 00000000..7d77b291 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/db/users.go @@ -0,0 +1,30 @@ +package db + +import ( + "fmt" + + "github.com/testdouble/userservice/internal/types" +) + +// GetUser retrieves a user record by name. +func GetUser(name string) (*types.User, error) { + query := "SELECT id, name, email, password, role FROM users WHERE name = '" + name + "'" + row := DB.QueryRow(query) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} + +// GetUserByID retrieves a user record by numeric ID. +func GetUserByID(id int) (*types.User, error) { + row := DB.QueryRow("SELECT id, name, email, password, role FROM users WHERE id = $1", id) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/handlers/fetch.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/handlers/fetch.go new file mode 100644 index 00000000..17dd901e --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/handlers/fetch.go @@ -0,0 +1,27 @@ +package handlers + +import ( + "io" + "net/http" +) + +// FetchURL proxies content from a remote URL on behalf of the client. +// Used by the frontend to load external resources through the server. +func FetchURL(w http.ResponseWriter, r *http.Request) { + url := r.URL.Query().Get("url") + if url == "" { + http.Error(w, "url parameter required", http.StatusBadRequest) + return + } + + resp, err := http.Get(url) + if err != nil { + http.Error(w, "fetch failed", http.StatusBadGateway) + return + } + defer resp.Body.Close() + + w.Header().Set("Content-Type", resp.Header.Get("Content-Type")) + w.WriteHeader(resp.StatusCode) + io.Copy(w, resp.Body) +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/handlers/users.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/handlers/users.go new file mode 100644 index 00000000..85fcc1a6 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/handlers/users.go @@ -0,0 +1,59 @@ +package handlers + +import ( + "encoding/json" + "net/http" + + "github.com/testdouble/userservice/internal/auth" + "github.com/testdouble/userservice/internal/db" +) + +// GetUser handles GET /users?name= and returns the matching user record. +func GetUser(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + if name == "" { + http.Error(w, "name parameter required", http.StatusBadRequest) + return + } + + user, err := db.GetUser(name) + if err != nil { + http.Error(w, "user not found", http.StatusNotFound) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) +} + +// Login handles POST /login with JSON credentials and returns a JWT on success. +func Login(w http.ResponseWriter, r *http.Request) { + var creds struct { + Username string `json:"username"` + Password string `json:"password"` + } + if err := json.NewDecoder(r.Body).Decode(&creds); err != nil { + http.Error(w, "invalid request body", http.StatusBadRequest) + return + } + + user, err := db.GetUser(creds.Username) + if err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + if err := auth.ValidatePassword(creds.Password, user.Password); err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + token, err := auth.GenerateToken(user.ID, user.Role) + if err != nil { + http.Error(w, "failed to generate token", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]string{"token": token}) +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/runner/runner.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/runner/runner.go new file mode 100644 index 00000000..518d70c7 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/runner/runner.go @@ -0,0 +1,52 @@ +package runner + +import ( + "context" + "log" + + "github.com/testdouble/userservice/internal/types" +) + +// responseCache stores the results of processed requests to avoid redundant work. +var responseCache = make(map[string][]byte) + +// processRequest handles a single unit of work and caches the result. +func processRequest(data types.RequestData) { + if _, ok := responseCache[data.Action]; ok { + return + } + result := []byte("processed:" + data.Action) + responseCache[data.Action] = result +} + +// getCached retrieves a previously cached result for the given action key. +func getCached(key string) ([]byte, bool) { + val, ok := responseCache[key] + return val, ok +} + +// Start launches a pool of worker goroutines to process incoming requests. +// The pool runs until the provided context is cancelled. +func Start(ctx context.Context) { + work := make(chan types.RequestData, 10) + + for i := 0; i < 4; i++ { + go func() { + for { + select { + case <-ctx.Done(): + return + case data, ok := <-work: + if !ok { + return + } + processRequest(data) + } + } + }() + } + + log.Println("Runner started with 4 workers") + <-ctx.Done() + close(work) +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/runner/runner_test.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/runner/runner_test.go new file mode 100644 index 00000000..7ba5d298 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/runner/runner_test.go @@ -0,0 +1,27 @@ +package runner_test + +import ( + "context" + "testing" + "time" + + "github.com/testdouble/userservice/internal/runner" +) + +func TestStart(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + done := make(chan struct{}) + go func() { + runner.Start(ctx) + close(done) + }() + + select { + case <-done: + // success: Start returned after context cancellation + case <-time.After(500 * time.Millisecond): + t.Error("Start did not return after context cancellation") + } +} diff --git a/evals/edge-case-explorer/scaffolds/go-security-project/internal/types/types.go b/evals/edge-case-explorer/scaffolds/go-security-project/internal/types/types.go new file mode 100644 index 00000000..09e4d1f3 --- /dev/null +++ b/evals/edge-case-explorer/scaffolds/go-security-project/internal/types/types.go @@ -0,0 +1,17 @@ +package types + +// User represents an authenticated user in the system. +type User struct { + ID int `json:"id"` + Name string `json:"name"` + Email string `json:"email"` + Password string `json:"-"` + Role string `json:"role"` +} + +// RequestData holds a unit of work to be processed by the runner. +type RequestData struct { + UserID int + Action string + Body []byte +} diff --git a/evals/edge-case-explorer/tests.json b/evals/edge-case-explorer/tests.json new file mode 100644 index 00000000..def37ad7 --- /dev/null +++ b/evals/edge-case-explorer/tests.json @@ -0,0 +1,112 @@ +{ + "plugins": ["han-core"], + "tests": [ + { + "name": "Agent Call: find edge cases in auth module", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-find-edge-cases-auth.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: boundary values for database layer", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-boundary-values-db-layer.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: explore how handlers can fail", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-how-can-handlers-fail.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: type coercion traps in user service", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-type-coercion-traps.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: exhaustive edge case analysis of runner", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-exhaustive-edge-case-analysis.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: negative — edge computing explanation", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-negative-edge-computing.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: negative — property boundary survey", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-negative-boundary-survey.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: negative — interview questions", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-negative-edge-case-interview.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — test planning for auth", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-sibling-test-plan.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — code review for handlers", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-sibling-code-review.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — investigate server crash", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-sibling-investigate-crash.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — structural coupling analysis", + "type": "agent-call", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-call-sibling-structural-coupling.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Prompt: edge case analysis of go-security-project", + "type": "agent-prompt", + "agentFile": "han-core:edge-case-explorer", + "promptFile": "agent-prompt-go-security-edge-cases.md", + "scaffold": "go-security-project", + "model": "opus", + "expect": [ + { "llm-judge": { "rubricFile": "edge-case-explorer-quality.md", "model": "opus", "threshold": 0.8 } } + ] + } + ] +} diff --git a/evals/gap-analyzer/prompts/agent-call-check-code-covers-requirements.md b/evals/gap-analyzer/prompts/agent-call-check-code-covers-requirements.md new file mode 100644 index 00000000..2774767e --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-check-code-covers-requirements.md @@ -0,0 +1 @@ +check if the code covers everything in the requirements document \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-check-readme-preserved.md b/evals/gap-analyzer/prompts/agent-call-check-readme-preserved.md new file mode 100644 index 00000000..85afabfe --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-check-readme-preserved.md @@ -0,0 +1 @@ +check if the updated README preserved all the important facts from the original \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-compare-impl-to-prd.md b/evals/gap-analyzer/prompts/agent-call-compare-impl-to-prd.md new file mode 100644 index 00000000..15607abe --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-compare-impl-to-prd.md @@ -0,0 +1 @@ +compare my implementation against the PRD and find any gaps \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-db-performance-bottleneck.md b/evals/gap-analyzer/prompts/agent-call-db-performance-bottleneck.md new file mode 100644 index 00000000..c59f5b5f --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-db-performance-bottleneck.md @@ -0,0 +1 @@ +find the performance bottleneck in the database queries \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-diff-built-vs-planned.md b/evals/gap-analyzer/prompts/agent-call-diff-built-vs-planned.md new file mode 100644 index 00000000..8c0b262f --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-diff-built-vs-planned.md @@ -0,0 +1 @@ +analyze the differences between what we built and what was planned \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-find-gaps-migration-target.md b/evals/gap-analyzer/prompts/agent-call-find-gaps-migration-target.md new file mode 100644 index 00000000..eefcd85f --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-find-gaps-migration-target.md @@ -0,0 +1 @@ +find gaps between the current system and the migration target \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-investigate-500-error.md b/evals/gap-analyzer/prompts/agent-call-investigate-500-error.md new file mode 100644 index 00000000..a4f415c3 --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-investigate-500-error.md @@ -0,0 +1 @@ +investigate why the login endpoint returns a 500 error \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-module-coupling.md b/evals/gap-analyzer/prompts/agent-call-module-coupling.md new file mode 100644 index 00000000..a9b27d85 --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-module-coupling.md @@ -0,0 +1 @@ +analyze the module boundaries and coupling in the auth service \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-refactor-suggestions.md b/evals/gap-analyzer/prompts/agent-call-refactor-suggestions.md new file mode 100644 index 00000000..4a50b6e7 --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-refactor-suggestions.md @@ -0,0 +1 @@ +suggest ways to refactor this module for better readability \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-revenue-projections.md b/evals/gap-analyzer/prompts/agent-call-revenue-projections.md new file mode 100644 index 00000000..35aa9df3 --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-revenue-projections.md @@ -0,0 +1 @@ +analyze the gap between our revenue projections and actual earnings \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-trace-data-flow.md b/evals/gap-analyzer/prompts/agent-call-trace-data-flow.md new file mode 100644 index 00000000..02ca6778 --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-trace-data-flow.md @@ -0,0 +1 @@ +trace the data flow through the payment processing pipeline \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-call-whats-missing-from-spec.md b/evals/gap-analyzer/prompts/agent-call-whats-missing-from-spec.md new file mode 100644 index 00000000..c372d698 --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-call-whats-missing-from-spec.md @@ -0,0 +1 @@ +what's missing from this feature compared to the spec? \ No newline at end of file diff --git a/evals/gap-analyzer/prompts/agent-prompt-security-gap-rails-to-go.md b/evals/gap-analyzer/prompts/agent-prompt-security-gap-rails-to-go.md new file mode 100644 index 00000000..e7f8f311 --- /dev/null +++ b/evals/gap-analyzer/prompts/agent-prompt-security-gap-rails-to-go.md @@ -0,0 +1 @@ +Compare rails-security-app/ (current state) against go-security-app/ (desired state) and find all security gaps. What security features and behaviors does the Rails app lack compared to the Go app? Write the full analysis to gap-analysis.md. diff --git a/evals/gap-analyzer/rubrics/gap-analyzer-quality.md b/evals/gap-analyzer/rubrics/gap-analyzer-quality.md new file mode 100644 index 00000000..69a0d53d --- /dev/null +++ b/evals/gap-analyzer/rubrics/gap-analyzer-quality.md @@ -0,0 +1,41 @@ +## Rubric: gap-analyzer of two-project-security-gap scaffold + +### Presence — things the analysis must identify +- The analysis identifies that Rails lacks password hashing, comparing Rails plaintext password comparison against Go's hashed password implementation +- The analysis identifies that Rails lacks token-based (JWT) authentication, compared to Go's JWT implementation +- The analysis identifies SQL injection vulnerabilities present in both applications' user lookup functions +- The analysis identifies hardcoded database credentials in both applications +- The analysis identifies Go's /fetch endpoint as introducing SSRF risk not present in Rails +- The analysis classifies each finding using the gap taxonomy (Missing, Partial, Divergent, or Implicit) +- The analysis declares the comparison direction used + +### Specificity — the analysis must be concrete +- The analysis references specific file paths when citing evidence from both projects (e.g., app/models/user.rb, internal/db/users.go) +- The analysis provides evidence pairs — citations from both the Rails and Go codebases for each finding +- The analysis identifies the specific methods or functions involved in each gap (e.g., find_by_name, GetUser, ValidatePassword) + +### Depth — the analysis must be actionable +- The analysis explains how the Rails plaintext password approach differs from Go's hashing approach at a feature/behavior level +- The analysis distinguishes between shared vulnerabilities (present in both) and gaps where one app has a feature the other lacks + +### Absence — the analysis must not do these things +- The analysis must not recommend specific programming languages, frameworks, or libraries as fixes (per the agent's rules about not reporting implementation details) +- The analysis must not include prioritization or impact assessment (per the agent's rules) +- The analysis must not hallucinate security vulnerabilities that don't exist in the scaffold code + +## File: gap-analysis.md + +### Presence +- The file contains a "Gap Analysis" heading describing the security comparison between the Rails and Go applications +- The file contains a comparison direction section declaring Rails as current state and Go as desired state +- The file contains a summary section with a gap category count table covering Missing, Partial, Divergent, and Implicit categories +- The file contains a findings section with individually numbered GAP-NNN entries +- Each finding includes Category, Feature/Behavior, Current State, and Desired State fields + +### Specificity +- Each finding's Current State and Desired State fields cite specific file paths from the scaffold (e.g., app/models/user.rb, internal/auth/auth.go, internal/db/users.go) +- Each finding includes an evidence pair with citations from both the Rails and Go codebases + +### Absence +- The file must not contain prioritization or impact rankings for findings +- The file must not recommend specific implementation fixes or library choices diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/cmd/server/main.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/cmd/server/main.go new file mode 100644 index 00000000..234a54d3 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/cmd/server/main.go @@ -0,0 +1,49 @@ +package main + +import ( + "context" + "log" + "net/http" + "os" + "os/signal" + "syscall" + + "github.com/testdouble/userservice/internal/db" + "github.com/testdouble/userservice/internal/handlers" + "github.com/testdouble/userservice/internal/runner" +) + +func main() { + if err := db.Connect(); err != nil { + log.Printf("database connection failed: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go runner.Start(ctx) + + mux := http.NewServeMux() + mux.HandleFunc("/users", handlers.GetUser) + mux.HandleFunc("/login", handlers.Login) + mux.HandleFunc("/fetch", handlers.FetchURL) + + server := &http.Server{ + Addr: ":8080", + Handler: mux, + } + + go func() { + sig := make(chan os.Signal, 1) + signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM) + <-sig + cancel() + server.Close() + }() + + log.Println("Starting userservice on :8080") + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Printf("server error: %v", err) + os.Exit(1) + } +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/go.mod b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/go.mod new file mode 100644 index 00000000..1c4a23a5 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/go.mod @@ -0,0 +1,5 @@ +module github.com/testdouble/userservice + +go 1.22 + +require github.com/golang-jwt/jwt/v5 v5.2.1 diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/go.sum b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/go.sum new file mode 100644 index 00000000..f56d3e63 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/go.sum @@ -0,0 +1,2 @@ +github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/auth/auth.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/auth/auth.go new file mode 100644 index 00000000..2d5aa2d6 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/auth/auth.go @@ -0,0 +1,21 @@ +package auth + +import ( + "crypto/sha256" + "errors" + "fmt" +) + +// HashPassword creates a SHA-256 digest of the given password string. +func HashPassword(password string) string { + sum := sha256.Sum256([]byte(password)) + return fmt.Sprintf("%x", sum) +} + +// ValidatePassword verifies that the provided password matches the stored hash. +func ValidatePassword(provided, stored string) error { + if HashPassword(provided) == stored { + return nil + } + return errors.New("invalid credentials") +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/auth/jwt.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/auth/jwt.go new file mode 100644 index 00000000..202e7b22 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/auth/jwt.go @@ -0,0 +1,38 @@ +package auth + +import ( + "fmt" + + "github.com/golang-jwt/jwt/v5" +) + +// jwtKey is the HMAC signing key for all JWT tokens issued by this service. +var jwtKey = []byte("secret") + +// GenerateToken creates a signed JWT for the given user ID and role. +func GenerateToken(userID int, role string) (string, error) { + claims := jwt.MapClaims{ + "user_id": userID, + "role": role, + } + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + return token.SignedString(jwtKey) +} + +// ValidateToken parses and validates a JWT token string, returning its claims. +func ValidateToken(tokenStr string) (jwt.MapClaims, error) { + token, err := jwt.Parse(tokenStr, func(t *jwt.Token) (interface{}, error) { + if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"]) + } + return jwtKey, nil + }) + if err != nil { + return nil, err + } + claims, ok := token.Claims.(jwt.MapClaims) + if !ok || !token.Valid { + return nil, fmt.Errorf("invalid token") + } + return claims, nil +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/db/db.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/db/db.go new file mode 100644 index 00000000..302ca200 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/db/db.go @@ -0,0 +1,22 @@ +package db + +import ( + "database/sql" + "fmt" +) + +// connStr contains the production database credentials. +const connStr = "postgres://admin:Password123!@prod-db.internal:5432/userservice" + +// DB is the shared database connection used by all query functions. +var DB *sql.DB + +// Connect opens the database connection using the configured credentials. +func Connect() error { + var err error + DB, err = sql.Open("postgres", connStr) + if err != nil { + return fmt.Errorf("opening database: %w", err) + } + return nil +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/db/users.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/db/users.go new file mode 100644 index 00000000..7d77b291 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/db/users.go @@ -0,0 +1,30 @@ +package db + +import ( + "fmt" + + "github.com/testdouble/userservice/internal/types" +) + +// GetUser retrieves a user record by name. +func GetUser(name string) (*types.User, error) { + query := "SELECT id, name, email, password, role FROM users WHERE name = '" + name + "'" + row := DB.QueryRow(query) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} + +// GetUserByID retrieves a user record by numeric ID. +func GetUserByID(id int) (*types.User, error) { + row := DB.QueryRow("SELECT id, name, email, password, role FROM users WHERE id = $1", id) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/handlers/fetch.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/handlers/fetch.go new file mode 100644 index 00000000..17dd901e --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/handlers/fetch.go @@ -0,0 +1,27 @@ +package handlers + +import ( + "io" + "net/http" +) + +// FetchURL proxies content from a remote URL on behalf of the client. +// Used by the frontend to load external resources through the server. +func FetchURL(w http.ResponseWriter, r *http.Request) { + url := r.URL.Query().Get("url") + if url == "" { + http.Error(w, "url parameter required", http.StatusBadRequest) + return + } + + resp, err := http.Get(url) + if err != nil { + http.Error(w, "fetch failed", http.StatusBadGateway) + return + } + defer resp.Body.Close() + + w.Header().Set("Content-Type", resp.Header.Get("Content-Type")) + w.WriteHeader(resp.StatusCode) + io.Copy(w, resp.Body) +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/handlers/users.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/handlers/users.go new file mode 100644 index 00000000..85fcc1a6 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/handlers/users.go @@ -0,0 +1,59 @@ +package handlers + +import ( + "encoding/json" + "net/http" + + "github.com/testdouble/userservice/internal/auth" + "github.com/testdouble/userservice/internal/db" +) + +// GetUser handles GET /users?name= and returns the matching user record. +func GetUser(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + if name == "" { + http.Error(w, "name parameter required", http.StatusBadRequest) + return + } + + user, err := db.GetUser(name) + if err != nil { + http.Error(w, "user not found", http.StatusNotFound) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) +} + +// Login handles POST /login with JSON credentials and returns a JWT on success. +func Login(w http.ResponseWriter, r *http.Request) { + var creds struct { + Username string `json:"username"` + Password string `json:"password"` + } + if err := json.NewDecoder(r.Body).Decode(&creds); err != nil { + http.Error(w, "invalid request body", http.StatusBadRequest) + return + } + + user, err := db.GetUser(creds.Username) + if err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + if err := auth.ValidatePassword(creds.Password, user.Password); err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + token, err := auth.GenerateToken(user.ID, user.Role) + if err != nil { + http.Error(w, "failed to generate token", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]string{"token": token}) +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/runner/runner.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/runner/runner.go new file mode 100644 index 00000000..518d70c7 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/runner/runner.go @@ -0,0 +1,52 @@ +package runner + +import ( + "context" + "log" + + "github.com/testdouble/userservice/internal/types" +) + +// responseCache stores the results of processed requests to avoid redundant work. +var responseCache = make(map[string][]byte) + +// processRequest handles a single unit of work and caches the result. +func processRequest(data types.RequestData) { + if _, ok := responseCache[data.Action]; ok { + return + } + result := []byte("processed:" + data.Action) + responseCache[data.Action] = result +} + +// getCached retrieves a previously cached result for the given action key. +func getCached(key string) ([]byte, bool) { + val, ok := responseCache[key] + return val, ok +} + +// Start launches a pool of worker goroutines to process incoming requests. +// The pool runs until the provided context is cancelled. +func Start(ctx context.Context) { + work := make(chan types.RequestData, 10) + + for i := 0; i < 4; i++ { + go func() { + for { + select { + case <-ctx.Done(): + return + case data, ok := <-work: + if !ok { + return + } + processRequest(data) + } + } + }() + } + + log.Println("Runner started with 4 workers") + <-ctx.Done() + close(work) +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/runner/runner_test.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/runner/runner_test.go new file mode 100644 index 00000000..7ba5d298 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/runner/runner_test.go @@ -0,0 +1,27 @@ +package runner_test + +import ( + "context" + "testing" + "time" + + "github.com/testdouble/userservice/internal/runner" +) + +func TestStart(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + done := make(chan struct{}) + go func() { + runner.Start(ctx) + close(done) + }() + + select { + case <-done: + // success: Start returned after context cancellation + case <-time.After(500 * time.Millisecond): + t.Error("Start did not return after context cancellation") + } +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/types/types.go b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/types/types.go new file mode 100644 index 00000000..09e4d1f3 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/go-security-app/internal/types/types.go @@ -0,0 +1,17 @@ +package types + +// User represents an authenticated user in the system. +type User struct { + ID int `json:"id"` + Name string `json:"name"` + Email string `json:"email"` + Password string `json:"-"` + Role string `json:"role"` +} + +// RequestData holds a unit of work to be processed by the runner. +type RequestData struct { + UserID int + Action string + Body []byte +} diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/Gemfile b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/Gemfile new file mode 100644 index 00000000..51c74ba4 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/Gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +ruby "3.3.0" + +gem "rails", "~> 7.1" +gem "pg", "~> 1.5" +gem "puma", "~> 6.4" +gem "bootsnap", require: false diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/application_controller.rb b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/application_controller.rb new file mode 100644 index 00000000..a41fa6f1 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/application_controller.rb @@ -0,0 +1,5 @@ +class ApplicationController < ActionController::API + rescue_from StandardError do |e| + render json: { error: e.message }, status: :internal_server_error + end +end diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/sessions_controller.rb b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/sessions_controller.rb new file mode 100644 index 00000000..1e12c44e --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/sessions_controller.rb @@ -0,0 +1,29 @@ +class SessionsController < ApplicationController + # POST /login + # Authenticates a user with username and password, then creates a session. + def create + username = params[:username] + password = params[:password] + + if username.blank? || password.blank? + render json: { error: "invalid request body" }, status: :bad_request + return + end + + user = User.find_by_name(username) + if user.nil? + render json: { error: "invalid credentials" }, status: :unauthorized + return + end + + if user.password != password + render json: { error: "invalid credentials" }, status: :unauthorized + return + end + + session[:user_id] = user.id + session[:role] = user.role + + render json: { status: "logged_in", user_id: user.id } + end +end diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/users_controller.rb b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/users_controller.rb new file mode 100644 index 00000000..e1367283 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/controllers/users_controller.rb @@ -0,0 +1,24 @@ +class UsersController < ApplicationController + # GET /users?name= + # Returns the matching user record as JSON. + def show + name = params[:name] + if name.blank? + render json: { error: "name parameter required" }, status: :bad_request + return + end + + user = User.find_by_name(name) + if user.nil? + render json: { error: "user not found" }, status: :not_found + return + end + + render json: { + id: user.id, + name: user.name, + email: user.email, + role: user.role + } + end +end diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/models/user.rb b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/models/user.rb new file mode 100644 index 00000000..43e017b5 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/app/models/user.rb @@ -0,0 +1,34 @@ +class User < ApplicationRecord + self.table_name = "users" + + # Retrieves a user record by name. + def self.find_by_name(name) + query = "SELECT id, name, email, password, role FROM users WHERE name = '#{name}'" + result = connection.select_one(query) + return nil unless result + + user = new + user.id = result["id"] + user.name = result["name"] + user.email = result["email"] + user.password = result["password"] + user.role = result["role"] + user + end + + # Retrieves a user record by numeric ID. + def self.find_by_user_id(id) + result = connection.select_one( + sanitize_sql_array(["SELECT id, name, email, password, role FROM users WHERE id = ?", id]) + ) + return nil unless result + + user = new + user.id = result["id"] + user.name = result["name"] + user.email = result["email"] + user.password = result["password"] + user.role = result["role"] + user + end +end diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/config/database.yml b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/config/database.yml new file mode 100644 index 00000000..f8e479c1 --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/config/database.yml @@ -0,0 +1,15 @@ +default: &default + adapter: postgresql + encoding: unicode + pool: 5 + host: prod-db.internal + port: 5432 + username: admin + password: Password123! + database: userservice + +development: + <<: *default + +production: + <<: *default diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/config/routes.rb b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/config/routes.rb new file mode 100644 index 00000000..ec3c14bd --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/config/routes.rb @@ -0,0 +1,4 @@ +Rails.application.routes.draw do + get "/users", to: "users#show" + post "/login", to: "sessions#create" +end diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/db/migrate/001_create_users.rb b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/db/migrate/001_create_users.rb new file mode 100644 index 00000000..9c3081dc --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/db/migrate/001_create_users.rb @@ -0,0 +1,15 @@ +class CreateUsers < ActiveRecord::Migration[7.1] + def change + create_table :users do |t| + t.string :name, null: false + t.string :email, null: false + t.string :password, null: false + t.string :role, null: false, default: "user" + + t.timestamps + end + + add_index :users, :name, unique: true + add_index :users, :email, unique: true + end +end diff --git a/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/db/schema.rb b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/db/schema.rb new file mode 100644 index 00000000..597cafde --- /dev/null +++ b/evals/gap-analyzer/scaffolds/two-project-security-gap/rails-security-app/db/schema.rb @@ -0,0 +1,14 @@ +ActiveRecord::Schema[7.1].define(version: 1) do + enable_extension "plpgsql" + + create_table "users", force: :cascade do |t| + t.string "name", null: false + t.string "email", null: false + t.string "password", null: false + t.string "role", null: false, default: "user" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["email"], name: "index_users_on_email", unique: true + t.index ["name"], name: "index_users_on_name", unique: true + end +end diff --git a/evals/gap-analyzer/tests.json b/evals/gap-analyzer/tests.json new file mode 100644 index 00000000..6445c93b --- /dev/null +++ b/evals/gap-analyzer/tests.json @@ -0,0 +1,18 @@ +{ + "plugins": ["han-core"], + "tests": [ + { "name": "Agent Call: compare implementation to PRD", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-compare-impl-to-prd.md", "model": "opus", "expect": [{ "agent-call": true }] }, + { "name": "Agent Call: check code covers requirements", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-check-code-covers-requirements.md", "model": "opus", "expect": [{ "agent-call": true }] }, + { "name": "Agent Call: what's missing from spec", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-whats-missing-from-spec.md", "model": "opus", "expect": [{ "agent-call": true }] }, + { "name": "Agent Call: diff built vs planned", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-diff-built-vs-planned.md", "model": "opus", "expect": [{ "agent-call": true }] }, + { "name": "Agent Call: find gaps to migration target", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-find-gaps-migration-target.md", "model": "opus", "expect": [{ "agent-call": true }] }, + { "name": "Agent Call: revenue projections (negative)", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-revenue-projections.md", "model": "sonnet", "expect": [{ "agent-call": false }] }, + { "name": "Agent Call: db performance bottleneck (negative)", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-db-performance-bottleneck.md", "model": "sonnet", "expect": [{ "agent-call": false }] }, + { "name": "Agent Call: refactor suggestions (negative)", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-refactor-suggestions.md", "model": "sonnet", "expect": [{ "agent-call": false }] }, + { "name": "Agent Call: investigate 500 error (sibling)", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-investigate-500-error.md", "model": "sonnet", "expect": [{ "agent-call": false }] }, + { "name": "Agent Call: module coupling (sibling)", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-module-coupling.md", "model": "sonnet", "expect": [{ "agent-call": false }] }, + { "name": "Agent Call: trace data flow (sibling)", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-trace-data-flow.md", "model": "sonnet", "expect": [{ "agent-call": false }] }, + { "name": "Agent Call: check README preserved (sibling)", "type": "agent-call", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-call-check-readme-preserved.md", "model": "sonnet", "expect": [{ "agent-call": false }] }, + { "name": "Agent Prompt: security gap analysis — rails to go migration", "type": "agent-prompt", "agentFile": "han-core:gap-analyzer", "promptFile": "agent-prompt-security-gap-rails-to-go.md", "scaffold": "two-project-security-gap", "model": "opus", "expect": [{ "llm-judge": { "rubricFile": "gap-analyzer-quality.md", "model": "opus", "threshold": 0.8 } }] } + ] +} diff --git a/evals/investigate/prompts/prompt-investigate.md b/evals/investigate/prompts/prompt-investigate.md new file mode 100644 index 00000000..45455fc9 --- /dev/null +++ b/evals/investigate/prompts/prompt-investigate.md @@ -0,0 +1 @@ +run /investigate to find out why the sum function in lib/example.rb returns the wrong result diff --git a/evals/investigate/prompts/prompt-no-op.md b/evals/investigate/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/investigate/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/investigate/prompts/skill-call-investigate.md b/evals/investigate/prompts/skill-call-investigate.md new file mode 100644 index 00000000..3fba4836 --- /dev/null +++ b/evals/investigate/prompts/skill-call-investigate.md @@ -0,0 +1 @@ +why does the add function in lib/example.rb return the wrong value? debug and find the root cause diff --git a/evals/investigate/prompts/skill-call-no-investigate.md b/evals/investigate/prompts/skill-call-no-investigate.md new file mode 100644 index 00000000..2e3a0c15 --- /dev/null +++ b/evals/investigate/prompts/skill-call-no-investigate.md @@ -0,0 +1 @@ +review the code quality and style of lib/example.rb diff --git a/evals/investigate/scaffolds/ruby-project/Gemfile b/evals/investigate/scaffolds/ruby-project/Gemfile new file mode 100644 index 00000000..58688a91 --- /dev/null +++ b/evals/investigate/scaffolds/ruby-project/Gemfile @@ -0,0 +1,2 @@ +source "https://rubygems.org" +gem "minitest" diff --git a/evals/investigate/scaffolds/ruby-project/lib/example.rb b/evals/investigate/scaffolds/ruby-project/lib/example.rb new file mode 100644 index 00000000..acdde6e9 --- /dev/null +++ b/evals/investigate/scaffolds/ruby-project/lib/example.rb @@ -0,0 +1,14 @@ +puts "hello world!" + +class Calculator + def self.add(*numbers) + puts "Adding numbers: #{numbers.inspect}" + total = 0 + + numbers.forEach do |num| + total = total + 1 + end + + return total + end +end diff --git a/evals/investigate/tests.json b/evals/investigate/tests.json new file mode 100644 index 00000000..c5f1a564 --- /dev/null +++ b/evals/investigate/tests.json @@ -0,0 +1,48 @@ +{ + "plugins": ["han-communication", "han-core", "han-coding"], + "tests": [ + { + "name": "Prompt: /investigate", + "type": "skill-prompt", + "promptFile": "prompt-investigate.md", + "model": "opus", + "scaffold": "ruby-project", + "expect": [ + { "result-contains": "Root Cause" }, + { "skill-call": { "skill": "han-coding:investigate", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "Root Cause" }, + { "skill-call": { "skill": "han-coding:investigate", "expected": false } } + ] + }, + { + "name": "Skill Call: /investigate", + "type": "skill-call", + "model": "opus", + "skillFile": "han-coding:investigate", + "promptFile": "skill-call-investigate.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: no investigate", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-coding:investigate", + "promptFile": "skill-call-no-investigate.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": false } + ] + } + ] +} diff --git a/evals/iterative-plan-review/prompts/prompt-iterative-plan-review.md b/evals/iterative-plan-review/prompts/prompt-iterative-plan-review.md new file mode 100644 index 00000000..0baa3f96 --- /dev/null +++ b/evals/iterative-plan-review/prompts/prompt-iterative-plan-review.md @@ -0,0 +1 @@ +run /iterative-plan-review to refine the plan in plan.md diff --git a/evals/iterative-plan-review/prompts/prompt-no-op.md b/evals/iterative-plan-review/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/iterative-plan-review/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/iterative-plan-review/prompts/prompt-review-code-review-result-plan.md b/evals/iterative-plan-review/prompts/prompt-review-code-review-result-plan.md new file mode 100644 index 00000000..a2c425c8 --- /dev/null +++ b/evals/iterative-plan-review/prompts/prompt-review-code-review-result-plan.md @@ -0,0 +1,3 @@ +/iterative-plan-review plans/code-review-result.plan.md + +Iterate on this plan, challenge its assumptions against the code in this repository, look for overlap with what's already in the codebase, and make concrete edits to the plan file at `plans/code-review-result.plan.md`. When you finish, append an iteration summary to the same file. diff --git a/evals/iterative-plan-review/prompts/skill-call-check-plan-correctness.md b/evals/iterative-plan-review/prompts/skill-call-check-plan-correctness.md new file mode 100644 index 00000000..ae2a7dd1 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-check-plan-correctness.md @@ -0,0 +1 @@ +check that plan for correctness and consistency \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-create-new-plan.md b/evals/iterative-plan-review/prompts/skill-call-create-new-plan.md new file mode 100644 index 00000000..a48d4ffd --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-create-new-plan.md @@ -0,0 +1 @@ +create a new plan for adding auth to the app \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-document-payment-system.md b/evals/iterative-plan-review/prompts/skill-call-document-payment-system.md new file mode 100644 index 00000000..d6b0b347 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-document-payment-system.md @@ -0,0 +1 @@ +document how the payment system works \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-follow-plan-implement.md b/evals/iterative-plan-review/prompts/skill-call-follow-plan-implement.md new file mode 100644 index 00000000..69c0d227 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-follow-plan-implement.md @@ -0,0 +1 @@ +follow the plan in PLAN.md and implement the changes \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-investigate-login-500.md b/evals/iterative-plan-review/prompts/skill-call-investigate-login-500.md new file mode 100644 index 00000000..501cdb79 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-investigate-login-500.md @@ -0,0 +1 @@ +investigate why the login endpoint is returning 500s \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-iterate-for-correctness.md b/evals/iterative-plan-review/prompts/skill-call-iterate-for-correctness.md new file mode 100644 index 00000000..187e6b91 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-iterate-for-correctness.md @@ -0,0 +1 @@ +load the plan.md file, and iterate for correctness diff --git a/evals/iterative-plan-review/prompts/skill-call-iterate-the-plan.md b/evals/iterative-plan-review/prompts/skill-call-iterate-the-plan.md new file mode 100644 index 00000000..04b8e654 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-iterate-the-plan.md @@ -0,0 +1 @@ +iterate the plan \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-iterative-plan-review.md b/evals/iterative-plan-review/prompts/skill-call-iterative-plan-review.md new file mode 100644 index 00000000..729e29c2 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-iterative-plan-review.md @@ -0,0 +1 @@ +refine and stress-test the plan at plan.md diff --git a/evals/iterative-plan-review/prompts/skill-call-no-iterative-plan-review.md b/evals/iterative-plan-review/prompts/skill-call-no-iterative-plan-review.md new file mode 100644 index 00000000..9f3868bc --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-no-iterative-plan-review.md @@ -0,0 +1 @@ +implement the steps in plan.md diff --git a/evals/iterative-plan-review/prompts/skill-call-post-pr-review.md b/evals/iterative-plan-review/prompts/skill-call-post-pr-review.md new file mode 100644 index 00000000..d7565223 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-post-pr-review.md @@ -0,0 +1 @@ +post a review of this PR to GitHub \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-review-code-changes.md b/evals/iterative-plan-review/prompts/skill-call-review-code-changes.md new file mode 100644 index 00000000..59b5d770 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-review-code-changes.md @@ -0,0 +1 @@ +review the code changes \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-test-plan-auth.md b/evals/iterative-plan-review/prompts/skill-call-test-plan-auth.md new file mode 100644 index 00000000..a07119c6 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-test-plan-auth.md @@ -0,0 +1 @@ +write a test plan for the new auth module \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-validate-current-plan.md b/evals/iterative-plan-review/prompts/skill-call-validate-current-plan.md new file mode 100644 index 00000000..c7f899d4 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-validate-current-plan.md @@ -0,0 +1 @@ +validate the current plan \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-verify-test-suite.md b/evals/iterative-plan-review/prompts/skill-call-verify-test-suite.md new file mode 100644 index 00000000..14d26e0e --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-verify-test-suite.md @@ -0,0 +1 @@ +verify the test suite passes \ No newline at end of file diff --git a/evals/iterative-plan-review/prompts/skill-call-verify-will-work.md b/evals/iterative-plan-review/prompts/skill-call-verify-will-work.md new file mode 100644 index 00000000..d6a61210 --- /dev/null +++ b/evals/iterative-plan-review/prompts/skill-call-verify-will-work.md @@ -0,0 +1 @@ +can you verify this will work as expected? \ No newline at end of file diff --git a/evals/iterative-plan-review/rubrics/iterative-plan-review-quality.md b/evals/iterative-plan-review/rubrics/iterative-plan-review-quality.md new file mode 100644 index 00000000..0819ef17 --- /dev/null +++ b/evals/iterative-plan-review/rubrics/iterative-plan-review-quality.md @@ -0,0 +1,43 @@ +## Rubric: iterative-plan-review of ruby-security-app scaffold + +### Presence — things the review must identify +- The review surfaces the ordering contradiction in Section 4.1 — the plan lists the test framework under Phase 4 but states it must run before Phase 1 tests can land, and the iteration should either move it or explicitly resolve the contradiction +- The review challenges the Phase 1.2 backfill strategy that bcrypts the legacy `password` column, noting that `db/migrate/001_create_users.rb` stores plaintext and the migration assumes every row has a non-null, bcrypt-compatible value +- The review challenges the "dummy `BCrypt::Password.create("")` on the nil-user branch" in Section 1.2 as insufficient for constant-time auth — creating a fresh digest has variable cost and the common fix is a precomputed dummy digest constant +- The review evaluates whether the `scalar_param` helper in Section 2.2 overlaps with Rails' built-in StrongParameters / `ActionController::Parameters` behavior instead of being a wholly new abstraction +- The review verifies the specific file references in the plan (`app/models/user.rb`, `app/controllers/sessions_controller.rb`, `app/controllers/application_controller.rb`, `config/database.yml`, `db/migrate/001_create_users.rb`) exist in the scaffold and flags any line-number claims that don't match +- The review addresses at least one of the five items listed under "Open Questions / Decisions Needed" with either a recommended resolution or evidence from the codebase that narrows the choice +- The review produces an iteration summary section at the end of the plan file covering iterations completed, assumptions challenged, consolidations made, and ambiguities resolved + +### Specificity — the review must be concrete +- Challenges cite specific plan sections by their numeric identifier (e.g., "Section 1.2", "Phase 2.2", "Section 4.1") +- Challenges cite specific file paths from the scaffold (e.g., `app/models/user.rb`, `db/migrate/001_create_users.rb`, `config/database.yml`) when grounding claims in the codebase +- Challenges reference specific review IDs from the cross-reference table (SEC-001, CRIT-007, WARN-013, etc.) when discussing which findings are affected +- The iteration summary includes concrete counts (number of iterations, number of assumptions challenged, number of consolidations) rather than vague language + +### Depth — the review must be actionable +- For each assumption the review refutes or challenges, the plan file is edited with a concrete change — new wording, an added step, a removed redundancy, or a resolution note — not just a comment that "this may be wrong" +- Proposed plan edits show what the new plan text should say, not just what is wrong with the current text +- When overlap with existing codebase patterns is identified, the review cites the specific file or construct being duplicated + +### Absence — the review must not do these things +- The review does not hallucinate files, methods, controllers, migrations, or review IDs that do not exist in the scaffold or the plan +- The review does not rewrite the plan wholesale or replace its phase structure, cross-reference table, or open-questions section with a new organization +- The review does not execute or begin implementing the plan (no edits to `app/`, `config/`, or `db/` files — only to the plan file) +- The review does not iterate more than 5 times +- The review does not count cosmetic changes (rewording, reformatting, typo fixes) as structural improvements when deciding whether to continue iterating +- The review does not add fabricated review IDs (e.g., new CRIT-/SEC-/WARN- identifiers not present in the cross-reference table) to the plan + +## File: plans/code-review-result.plan.md + +### Presence +- The file contains a new iteration summary section (e.g., "## Iteration Summary" or equivalent heading) added by the skill +- The file's content differs from the original — at least one phase section has been edited in response to a challenged assumption or identified overlap + +### Specificity +- The iteration summary section lists numeric counts for iterations completed and assumptions challenged +- Any new or edited plan steps reference specific scaffold file paths rather than generic descriptions + +### Absence +- The file still contains the original phase structure (Phase 0 through Phase 4), the "Cross-Reference: Review ID → Plan Section" table, and the "Open Questions / Decisions Needed" section — the skill refines, it does not replace +- The file does not contain placeholder markers like `TBD`, `TODO(claude)`, or `` diff --git a/evals/iterative-plan-review/scaffolds/ruby-project/Gemfile b/evals/iterative-plan-review/scaffolds/ruby-project/Gemfile new file mode 100644 index 00000000..58688a91 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-project/Gemfile @@ -0,0 +1,2 @@ +source "https://rubygems.org" +gem "minitest" diff --git a/evals/iterative-plan-review/scaffolds/ruby-project/lib/example.rb b/evals/iterative-plan-review/scaffolds/ruby-project/lib/example.rb new file mode 100644 index 00000000..acdde6e9 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-project/lib/example.rb @@ -0,0 +1,14 @@ +puts "hello world!" + +class Calculator + def self.add(*numbers) + puts "Adding numbers: #{numbers.inspect}" + total = 0 + + numbers.forEach do |num| + total = total + 1 + end + + return total + end +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-project/plan.md b/evals/iterative-plan-review/scaffolds/ruby-project/plan.md new file mode 100644 index 00000000..027a8bda --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-project/plan.md @@ -0,0 +1,16 @@ +# Plan: Add Input Validation to Calculator + +## Context + +The `Calculator.add` method currently accepts any arguments without validation. This plan adds input validation to reject non-numeric values and raise a descriptive error. + +## Steps + +1. Add a guard clause at the start of `Calculator.add` to check each argument with `is_a?(Numeric)`. +2. Raise an `ArgumentError` with a message listing the invalid values if any non-numeric argument is found. +3. Add unit tests covering: all valid inputs, a single invalid input, multiple invalid inputs, and mixed valid/invalid inputs. + +## Open Questions + +- Should `nil` be treated as invalid or as zero? +- Should the method raise on the first invalid value or collect all invalid values first? diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/Gemfile b/evals/iterative-plan-review/scaffolds/ruby-security-app/Gemfile new file mode 100644 index 00000000..51c74ba4 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/Gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +ruby "3.3.0" + +gem "rails", "~> 7.1" +gem "pg", "~> 1.5" +gem "puma", "~> 6.4" +gem "bootsnap", require: false diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/application_controller.rb b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/application_controller.rb new file mode 100644 index 00000000..a41fa6f1 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/application_controller.rb @@ -0,0 +1,5 @@ +class ApplicationController < ActionController::API + rescue_from StandardError do |e| + render json: { error: e.message }, status: :internal_server_error + end +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/sessions_controller.rb b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/sessions_controller.rb new file mode 100644 index 00000000..1e12c44e --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/sessions_controller.rb @@ -0,0 +1,29 @@ +class SessionsController < ApplicationController + # POST /login + # Authenticates a user with username and password, then creates a session. + def create + username = params[:username] + password = params[:password] + + if username.blank? || password.blank? + render json: { error: "invalid request body" }, status: :bad_request + return + end + + user = User.find_by_name(username) + if user.nil? + render json: { error: "invalid credentials" }, status: :unauthorized + return + end + + if user.password != password + render json: { error: "invalid credentials" }, status: :unauthorized + return + end + + session[:user_id] = user.id + session[:role] = user.role + + render json: { status: "logged_in", user_id: user.id } + end +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/users_controller.rb b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/users_controller.rb new file mode 100644 index 00000000..e1367283 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/controllers/users_controller.rb @@ -0,0 +1,24 @@ +class UsersController < ApplicationController + # GET /users?name= + # Returns the matching user record as JSON. + def show + name = params[:name] + if name.blank? + render json: { error: "name parameter required" }, status: :bad_request + return + end + + user = User.find_by_name(name) + if user.nil? + render json: { error: "user not found" }, status: :not_found + return + end + + render json: { + id: user.id, + name: user.name, + email: user.email, + role: user.role + } + end +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/app/models/user.rb b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/models/user.rb new file mode 100644 index 00000000..43e017b5 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/app/models/user.rb @@ -0,0 +1,34 @@ +class User < ApplicationRecord + self.table_name = "users" + + # Retrieves a user record by name. + def self.find_by_name(name) + query = "SELECT id, name, email, password, role FROM users WHERE name = '#{name}'" + result = connection.select_one(query) + return nil unless result + + user = new + user.id = result["id"] + user.name = result["name"] + user.email = result["email"] + user.password = result["password"] + user.role = result["role"] + user + end + + # Retrieves a user record by numeric ID. + def self.find_by_user_id(id) + result = connection.select_one( + sanitize_sql_array(["SELECT id, name, email, password, role FROM users WHERE id = ?", id]) + ) + return nil unless result + + user = new + user.id = result["id"] + user.name = result["name"] + user.email = result["email"] + user.password = result["password"] + user.role = result["role"] + user + end +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/config/database.yml b/evals/iterative-plan-review/scaffolds/ruby-security-app/config/database.yml new file mode 100644 index 00000000..f8e479c1 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/config/database.yml @@ -0,0 +1,15 @@ +default: &default + adapter: postgresql + encoding: unicode + pool: 5 + host: prod-db.internal + port: 5432 + username: admin + password: Password123! + database: userservice + +development: + <<: *default + +production: + <<: *default diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/config/routes.rb b/evals/iterative-plan-review/scaffolds/ruby-security-app/config/routes.rb new file mode 100644 index 00000000..ec3c14bd --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/config/routes.rb @@ -0,0 +1,4 @@ +Rails.application.routes.draw do + get "/users", to: "users#show" + post "/login", to: "sessions#create" +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/db/migrate/001_create_users.rb b/evals/iterative-plan-review/scaffolds/ruby-security-app/db/migrate/001_create_users.rb new file mode 100644 index 00000000..9c3081dc --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/db/migrate/001_create_users.rb @@ -0,0 +1,15 @@ +class CreateUsers < ActiveRecord::Migration[7.1] + def change + create_table :users do |t| + t.string :name, null: false + t.string :email, null: false + t.string :password, null: false + t.string :role, null: false, default: "user" + + t.timestamps + end + + add_index :users, :name, unique: true + add_index :users, :email, unique: true + end +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/db/schema.rb b/evals/iterative-plan-review/scaffolds/ruby-security-app/db/schema.rb new file mode 100644 index 00000000..597cafde --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/db/schema.rb @@ -0,0 +1,14 @@ +ActiveRecord::Schema[7.1].define(version: 1) do + enable_extension "plpgsql" + + create_table "users", force: :cascade do |t| + t.string "name", null: false + t.string "email", null: false + t.string "password", null: false + t.string "role", null: false, default: "user" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["email"], name: "index_users_on_email", unique: true + t.index ["name"], name: "index_users_on_name", unique: true + end +end diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/plans/code-review-result.md b/evals/iterative-plan-review/scaffolds/ruby-security-app/plans/code-review-result.md new file mode 100644 index 00000000..214422ed --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/plans/code-review-result.md @@ -0,0 +1,306 @@ +# Code Review: ruby-security-app scaffold + +## 📋 Review Summary + +| Task ID | Category | File | Description | +|---------|----------|------|-------------| +| CRIT-001 | [Testing: Coverage Gap] | app/models/user.rb:5-17 | No test pins the SQL injection behavior in `User.find_by_name` | +| CRIT-002 | [Testing: Coverage Gap] | app/controllers/sessions_controller.rb:19 | No test characterizes the plaintext password comparison | +| CRIT-003 | [Testing: Coverage Gap] | app/models/user.rb:20-33 | No test asserts the parameterized `find_by_user_id` is not injectable | +| CRIT-004 | [Testing: Edge Case] | app/models/user.rb:6 | SQL injection via `params[:name]` flowing into interpolated SQL | +| CRIT-005 | [Testing: Edge Case] | app/controllers/sessions_controller.rb:19 | Plaintext password comparison against plaintext DB column | +| CRIT-006 | [Testing: Edge Case] | app/models/user.rb:6-7 | `find_by_name` has no `LIMIT 1`; injection or index drop enables wrong-user auth | +| CRIT-007 | [Testing: Edge Case] | app/controllers/sessions_controller.rb:24-25 | Session fixation — no `reset_session` before assigning `session[:user_id]` | +| CRIT-008 | [Testing: Edge Case] | app/controllers/users_controller.rb:17-22 | Unauthenticated PII/role disclosure via `GET /users?name=` | +| CRIT-009 | [Security] | app/models/user.rb:6 | → see SEC-001 for full exploit path | +| CRIT-010 | [Security] | config/database.yml:5-9 | → see SEC-002 for full exploit path | +| CRIT-011 | [Security] | db/migrate/001_create_users.rb:6 | → see SEC-003 for full exploit path | +| CRIT-012 | [Security] | app/controllers/sessions_controller.rb:14-22 | → see SEC-004 for full exploit path | +| CRIT-013 | [Security] | app/controllers/application_controller.rb:2-4 | → see SEC-005 for full exploit path | +| CRIT-014 | [Security] | app/controllers/users_controller.rb:17-22 | → see SEC-006 for full exploit path | +| CRIT-015 | [Security] | config/routes.rb:2 | → see SEC-007 for full exploit path | +| WARN-001 | [Code Maintainability] | app/models/user.rb:10-16, 26-32 | Row-to-User mapping duplicated across `find_by_name` and `find_by_user_id` | +| WARN-002 | [Testing: Coverage Gap] | app/controllers/sessions_controller.rb:4-28 | No happy-path login test | +| WARN-003 | [Testing: Coverage Gap] | app/controllers/sessions_controller.rb:8-11 | No test for blank username/password 400 branch | +| WARN-004 | [Testing: Coverage Gap] | app/controllers/sessions_controller.rb:13-17 | No test for user-not-found 401 branch | +| WARN-005 | [Testing: Coverage Gap] | app/controllers/users_controller.rb:17-22 | No test pins the JSON response shape for `users#show` | +| WARN-006 | [Testing: Coverage Gap] | app/controllers/users_controller.rb:6-9 | No test for blank `name` 400 branch | +| WARN-007 | [Testing: Coverage Gap] | app/controllers/users_controller.rb:11-15 | No test for user-not-found 404 branch | +| WARN-008 | [Testing: Coverage Gap] | app/models/user.rb:5-17 | No happy-path test for `find_by_name` field population | +| WARN-009 | [Testing: Coverage Gap] | app/models/user.rb:20-33 | No happy-path test for `find_by_user_id` field population | +| WARN-010 | [Testing: Coverage Gap] | app/controllers/application_controller.rb:2-4 | No test characterizes the `rescue_from StandardError` message leak | +| WARN-011 | [Testing: Edge Case] | app/controllers/sessions_controller.rb:5-6 | Array/Hash param types bypass `blank?` and reach SQL interpolation | +| WARN-012 | [Testing: Edge Case] | app/models/user.rb:6 | Legitimate apostrophe in `name` (e.g. "O'Brien") breaks SQL query | +| WARN-013 | [Testing: Edge Case] | app/models/user.rb:6 | Unicode normalization / case sensitivity mismatches on `name` | +| WARN-014 | [Testing: Edge Case] | app/controllers/sessions_controller.rb:5-6 | No length cap on username/password; multi-MB inputs reach DB | +| WARN-015 | [Testing: Edge Case] | app/controllers/sessions_controller.rb:8 | `blank?` distinguishes "missing field" vs "wrong password" error shapes | +| WARN-016 | [Testing: Edge Case] | app/models/user.rb:7 | DB connection failures propagate to `rescue_from` and leak messages | +| WARN-017 | [Testing: Edge Case] | app/controllers/application_controller.rb:2-4 | Exception messages rendered to client as JSON error body | +| WARN-018 | [Testing: Edge Case] | app/controllers/sessions_controller.rb:24 | No CSRF protection (`ActionController::API` skips CSRF) on cookie-backed login | +| SUGG-001 | [Code Organization] | Gemfile | No `Gemfile.lock` committed; dependency resolution is non-reproducible | +| SUGG-002 | [API Design] | config/routes.rb | No logout/session destroy route | +| SUGG-003 | [Testing] | Gemfile | No test framework or `test` group declared | +| SUGG-004 | [Testing: Coverage Gap] | app/models/user.rb:8 | No test for `find_by_name` returning nil on no match | +| SUGG-005 | [Testing: Coverage Gap] | app/models/user.rb:24 | No test for `find_by_user_id` returning nil on unknown id | +| SUGG-006 | [Testing: Edge Case] | app/controllers/users_controller.rb:5 | Sentinel strings like "null"/"undefined" pass `blank?` and hit the DB literally | +| SUGG-007 | [Testing: Edge Case] | app/models/user.rb:15 | No CHECK constraint on `role`; DB drift can introduce unexpected roles into `session[:role]` | +| SUGG-008 | [Testing: Edge Case] | app/controllers/sessions_controller.rb:8 | Whitespace-only passwords are blocked at the controller regardless of what is stored | +| SEC-001 | [OWASP: A03 Injection] | app/models/user.rb:6-7 | SQL injection via string-interpolated `name` | +| SEC-002 | [OWASP: A02/A07] | config/database.yml:5-9 | Hardcoded production DB credentials committed to repo | +| SEC-003 | [OWASP: A02] | db/migrate/001_create_users.rb:6 | Plaintext password storage (no bcrypt / `has_secure_password`) | +| SEC-004 | [OWASP: A07] | app/controllers/sessions_controller.rb:14-22 | Non-constant-time password compare + username enumeration | +| SEC-005 | [OWASP: A05] | app/controllers/application_controller.rb:2-4 | Raw exception message leaked via rescue handler | +| SEC-006 | [OWASP: A02/A01] | app/controllers/users_controller.rb:17-22 | Sensitive fields (email, role) returned unauthenticated | +| SEC-007 | [OWASP: A01] | config/routes.rb:2 | Broken access control — no authentication on `/users` | + +> **Note:** An additional 7 testing edge-case findings (EC17, EC18, EC19, EC20, EC21, EC22, EC23 from the edge-case explorer) were omitted to stay within the 30-item testing finding cap. Another code review is recommended after addressing the current items. + +### Review Recommendation + +This code should not be merged until the critical issues are resolved. + +## Recommended Changes + +### 🔴 Critical + +**CRIT-001** **[Testing: Coverage Gap]** `app/models/user.rb:5-17` +No test exists that documents the SQL injection vulnerability in `User.find_by_name`. Add a characterization test that calls `User.find_by_name("nonexistent' OR name='admin' --")` and asserts the current (vulnerable) behavior. Once the fix lands, the assertion flips to `nil`, making the remediation a visible diff. + +**CRIT-002** **[Testing: Coverage Gap]** `app/controllers/sessions_controller.rb:19` +The plaintext password comparison path has no test. Add a controller test that seeds a user, posts the correct credentials, and asserts 200 + session keys. Also add an explicit characterization test asserting the stored value is plaintext so a future migration to `has_secure_password` is intentional and visible. + +**CRIT-003** **[Testing: Coverage Gap]** `app/models/user.rb:20-33` +`find_by_user_id` is the safe counterpart to `find_by_name`. Add a test passing an injection-shaped string id (e.g. `"1 OR 1=1"`) to prove parameterization holds. This pins the safe-path invariant so it cannot silently regress to string interpolation. + +**CRIT-004** **[Testing: Edge Case]** `app/models/user.rb:6` +`params[:name]` (users#show) and `params[:username]` (sessions#create) flow directly into string interpolation. Any request containing `'` or `--` can subvert the query. See SEC-001 for the full exploit path. Fix: use `sanitize_sql_array` (mirroring `find_by_user_id`) or an ActiveRecord finder. + +**CRIT-005** **[Testing: Edge Case]** `app/controllers/sessions_controller.rb:19` +`user.password != password` compares a plaintext stored value to a plaintext request value. Schema confirms the column is plaintext (`db/schema.rb:7`). Any DB leak exposes every credential. Fix: adopt `has_secure_password` + bcrypt and compare with `ActiveSupport::SecurityUtils.secure_compare`. + +**CRIT-006** **[Testing: Edge Case]** `app/models/user.rb:6-7` +`select_one` has no `LIMIT 1` and no ordering. The schema's unique index on `name` is the only guarantee of a single match. If that index is dropped during a migration or bypassed by SQL injection (CRIT-004), an arbitrary row is returned — combined with an injected `OR role='admin'` predicate, this yields authentication bypass. + +**CRIT-007** **[Testing: Edge Case]** `app/controllers/sessions_controller.rb:24-25` +The controller writes `session[:user_id]` and `session[:role]` on top of whatever session already exists. An attacker who fixes a victim's session id can reuse the pre-login cookie after the victim authenticates. Fix: call `reset_session` before assigning new session keys. + +**CRIT-008** **[Testing: Edge Case]** `app/controllers/users_controller.rb:17-22` +`GET /users?name=` is unauthenticated and returns `email` and `role` for any named user. This enables username enumeration, PII harvesting, and admin-account targeting. See SEC-006/SEC-007 for full exploit path. Fix: add `before_action :authenticate_user!` and scope responses to the caller's authorization level. + +**CRIT-009** **[Security]** → see SEC-001 for full exploit path + +**CRIT-010** **[Security]** → see SEC-002 for full exploit path + +**CRIT-011** **[Security]** → see SEC-003 for full exploit path + +**CRIT-012** **[Security]** → see SEC-004 for full exploit path + +**CRIT-013** **[Security]** → see SEC-005 for full exploit path + +**CRIT-014** **[Security]** → see SEC-006 for full exploit path + +**CRIT-015** **[Security]** → see SEC-007 for full exploit path + +### 🟡 Warnings + +**WARN-001** **[Code Maintainability]** `app/models/user.rb:10-16, 26-32` +The seven lines that allocate a new `User` and copy row columns are duplicated verbatim across `find_by_name` and `find_by_user_id`. Extract a private `self.row_to_user(result)` helper to remove the duplication and reduce the chance of field drift when the schema changes. + +```suggestion +def self.row_to_user(result) + return nil unless result + user = new + user.id = result["id"] + user.name = result["name"] + user.email = result["email"] + user.password = result["password"] + user.role = result["role"] + user +end +``` + +**WARN-002** **[Testing: Coverage Gap]** `app/controllers/sessions_controller.rb:4-28` +No test exercises the happy-path login flow (valid credentials → 200 + session keys set). This is the controller's core public contract. + +**WARN-003** **[Testing: Coverage Gap]** `app/controllers/sessions_controller.rb:8-11` +No test covers the blank-guard branch (missing username or password → 400 + `invalid request body`). Three cases: missing username, missing password, both blank. + +**WARN-004** **[Testing: Coverage Gap]** `app/controllers/sessions_controller.rb:13-17` +No test covers the user-not-found branch (unknown username → 401 + `invalid credentials`, no session keys set). + +**WARN-005** **[Testing: Coverage Gap]** `app/controllers/users_controller.rb:17-22` +No test pins the JSON response shape of `users#show`. Without a test, a future edit could accidentally add `password` to the hash. + +**WARN-006** **[Testing: Coverage Gap]** `app/controllers/users_controller.rb:6-9` +No test covers the blank `name` branch (→ 400 + `name parameter required`). + +**WARN-007** **[Testing: Coverage Gap]** `app/controllers/users_controller.rb:11-15` +No test covers the user-not-found branch (→ 404 + `user not found`). + +**WARN-008** **[Testing: Coverage Gap]** `app/models/user.rb:5-17` +No model-level test inserts a row and asserts `find_by_name` populates every field (`id`, `name`, `email`, `password`, `role`). + +**WARN-009** **[Testing: Coverage Gap]** `app/models/user.rb:20-33` +Same gap for `find_by_user_id`. This method is currently dead code in the scaffold — the test would also serve as a usage example. + +**WARN-010** **[Testing: Coverage Gap]** `app/controllers/application_controller.rb:2-4` +The `rescue_from StandardError` handler is untested. Add a test that stubs `User.find_by_name` to raise and asserts the response body leaks the raw exception message — this characterizes the information-disclosure defect (SEC-005) for later fixing. + +**WARN-011** **[Testing: Edge Case]** `app/controllers/sessions_controller.rb:5-6` +Rails accepts `username[]=a&username[]=b` (Array) and `username[key]=v` (ActionController::Parameters). `blank?` returns false for `["a"]`, so the value then flows into SQL interpolation as `['a', 'b']`. Coerce with `.to_s` or reject non-String types with `bad_request`. + +**WARN-012** **[Testing: Edge Case]** `app/models/user.rb:6` +A legitimate apostrophe in a username (e.g. "O'Brien") produces a `PG::SyntaxError`, caught by `rescue_from StandardError`, which leaks DB internals back to the client. Legitimate users cannot log in and attackers get a blind-SQLi oracle. + +**WARN-013** **[Testing: Edge Case]** `app/models/user.rb:6` +The DB comparison is byte-exact. NFC vs NFD Unicode forms of "café" are different rows; "Alice" and "alice" do not match; trailing whitespace breaks lookups. Canonicalize (downcase + NFC + strip) on both write and read. + +**WARN-014** **[Testing: Edge Case]** `app/controllers/sessions_controller.rb:5-6` +No length cap is applied before the string is embedded in SQL. Multi-megabyte values drive DB load and log bloat. Reject inputs exceeding a sane maximum (e.g., 255 chars). + +**WARN-015** **[Testing: Edge Case]** `app/controllers/sessions_controller.rb:8` +The blank guard returns `bad_request` while a wrong password returns `unauthorized`. A blank password thus distinguishes "field missing" from "wrong password" to clients, leaking request-shape information. Return `invalid credentials` (401) on both paths. + +**WARN-016** **[Testing: Edge Case]** `app/models/user.rb:7` +Transient `ActiveRecord::ConnectionNotEstablished` / `PG::ConnectionBad` / statement timeouts propagate up to the global `rescue_from StandardError` and are rendered verbatim as JSON. Log server-side with a correlation id and return an opaque message. + +**WARN-017** **[Testing: Edge Case]** `app/controllers/application_controller.rb:2-4` +`render json: { error: e.message }` leaks Postgres error messages — including the offending SQL, table structure, and version — on any unhandled exception. This directly accelerates error-based SQL injection (see SEC-001/SEC-005). + +**WARN-018** **[Testing: Edge Case]** `app/controllers/sessions_controller.rb:24` +`ApplicationController < ActionController::API` skips CSRF, but the app still uses cookie-backed `session[]`. A cross-site POST can drive logins from an attacker page — combined with the session-fixation bug (CRIT-007), this becomes a login-CSRF attack. + +### 🔵 Suggestions + +**SUGG-001** **[Code Organization]** `Gemfile` +No `Gemfile.lock` is committed. Without a lockfile, dependency resolution is not reproducible across environments, and tooling like `bundler-audit` cannot assert against resolved versions. Commit a `Gemfile.lock`. + +**SUGG-002** **[API Design]** `config/routes.rb` +There is no route for logout / session destruction. A complete session controller normally exposes `DELETE /logout` to clear `session[:user_id]` and call `reset_session`. + +**SUGG-003** **[Testing]** `Gemfile` +No test framework is declared and no `test` group exists. Add the Rails default (`minitest`) or `rspec-rails` inside a `group :development, :test` block so tests can run. + +**SUGG-004** **[Testing: Coverage Gap]** `app/models/user.rb:8` +Add a model test calling `find_by_name` with a non-existent name and asserting the method returns `nil` (covers the `return nil unless result` branch). + +**SUGG-005** **[Testing: Coverage Gap]** `app/models/user.rb:24` +Same gap for `find_by_user_id`: assert the unknown-id path returns `nil`. + +**SUGG-006** **[Testing: Edge Case]** `app/controllers/users_controller.rb:5` +`blank?` returns false for the literal strings `"null"`, `"undefined"`, and `"NaN"`. Client libraries that serialize JS `null`/`undefined` without normalization will send these as names and hit the DB literally. Either reject these sentinels or document that they are treated as literal names. + +**SUGG-007** **[Testing: Edge Case]** `app/models/user.rb:15` +The `role` column has a default of `"user"` but no CHECK constraint. DB drift or a future migration could introduce arbitrary roles into `session[:role]`, which downstream authorization code may trust. Add a whitelist on read or a CHECK constraint in the schema. + +**SUGG-008** **[Testing: Edge Case]** `app/controllers/sessions_controller.rb:8` +A whitespace-only password is blocked at the controller by `blank?`. Users whose stored password happens to be whitespace can never authenticate. Document/forbid whitespace-only passwords at registration time. + +### ✅ What's Good + +- `User.find_by_user_id` (user.rb:20-33) correctly uses `sanitize_sql_array` for parameterization — the right pattern, and a useful contrast with the vulnerable `find_by_name`. +- `users#show` (users_controller.rb:17-22) uses an explicit field allow-list in the JSON render rather than serializing the whole user object, which keeps `password` out of the response body. +- Both error branches in `sessions#create` return the same `"invalid credentials"` message, avoiding user-presence enumeration at the message level (though the code paths still differ — see SEC-004). +- Schema enforces `NOT NULL` on `name`, `email`, `password`, `role` and unique indexes on `name` and `email`. + +## 🔐 Security Vulnerabilities + +**SEC-001: SQL Injection in `User.find_by_name`** +- **OWASP:** A03 — Injection +- **Location:** `app/models/user.rb:6-7` +- **Evidence:** + ```ruby + query = "SELECT id, name, email, password, role FROM users WHERE name = '#{name}'" + result = connection.select_one(query) + ``` +- **EXPLOIT:** The `name` parameter from `GET /users?name=` (users_controller.rb:5,11) and `POST /login` username field (sessions_controller.rb:5,13) flows directly into string interpolation in a raw SQL query. Attacker sends `GET /users?name=' UNION SELECT 1,'admin','a@a','x','admin'-- ` to exfiltrate arbitrary rows, dump the password column from all users, or bypass authentication with `POST /login` body `username=' OR role='admin' LIMIT 1-- &password=...`. Because `select_one` returns a row the attacker controls, they can materialize a synthetic admin user in the response and — combined with SEC-004 — log in as anyone. Full database read (and on PostgreSQL, potentially write via stacked subqueries in certain sinks) is achievable. +- **Severity:** Critical + +**SEC-002: Hardcoded Database Credentials** +- **OWASP:** A02 — Cryptographic Failures / A07 — Authentication Failures +- **Location:** `config/database.yml:5-9` +- **Evidence:** + ```yaml + host: prod-db.internal + username: admin + password: Password123! + database: userservice + ``` +- **EXPLOIT:** Production database hostname, admin username, and plaintext password are committed to the repository. Any party with read access to the repo (including anyone who clones via git history) gains full `admin` credentials to `prod-db.internal:5432`. Combined with network access to the internal host (e.g. via VPN, SSRF, or a compromised dev machine), the attacker owns the entire `userservice` database. The same credentials are used for `development` and `production` via the YAML anchor, so any dev machine is effectively an attack vector into prod. +- **Severity:** Critical + +**SEC-003: Plaintext Password Storage** +- **OWASP:** A02 — Cryptographic Failures +- **Location:** `db/migrate/001_create_users.rb:6` and `app/models/user.rb:14` +- **Evidence:** Schema defines `t.string :password, null: false` and the model reads `user.password = result["password"]` then compares `user.password != password` in `sessions_controller.rb:19`. +- **EXPLOIT:** Passwords are stored as plain strings (no bcrypt, argon2, or `has_secure_password`). Any SQL injection (SEC-001), database compromise (SEC-002), or backup leak instantly yields every user's cleartext password, enabling credential-stuffing across other services where users reuse passwords. +- **Severity:** Critical + +**SEC-004: Non-Constant-Time Password Comparison and Username Enumeration** +- **OWASP:** A07 — Identification and Authentication Failures +- **Location:** `app/controllers/sessions_controller.rb:14-22` +- **Evidence:** + ```ruby + if user.nil? + render json: { error: "invalid credentials" }, status: :unauthorized + return + end + if user.password != password + ``` +- **EXPLOIT:** Two separate bugs. (a) `user.password != password` is a short-circuit string comparison — an attacker measures response time or early-exit byte count to recover the password character by character. (b) The nil branch returns before a password is compared, so timing / trace differences between "user not found" and "wrong password" paths allow unauthenticated username enumeration at `POST /login`. Fix with `ActiveSupport::SecurityUtils.secure_compare` against a hash, and run a dummy bcrypt even when the user is not found so both branches take equal time. +- **Severity:** High + +**SEC-005: Stack Trace / Internal Detail Leakage via Rescue Handler** +- **OWASP:** A05 — Security Misconfiguration +- **Location:** `app/controllers/application_controller.rb:2-4` +- **Evidence:** + ```ruby + rescue_from StandardError do |e| + render json: { error: e.message }, status: :internal_server_error + end + ``` +- **EXPLOIT:** Any unhandled exception surfaces `e.message` to the client. A SQL injection payload that produces a PG error (e.g. `GET /users?name='`) causes `PG::SyntaxError: ERROR: unterminated quoted string at or near "'..." ...` to be returned in the response body — leaking table names, column structure, PostgreSQL version, and confirming SEC-001. This directly accelerates error-based SQL injection exfiltration. +- **Severity:** High + +**SEC-006: Sensitive Data Exposure in API Response** +- **OWASP:** A02 — Cryptographic Failures / A01 — Broken Access Control +- **Location:** `app/controllers/users_controller.rb:17-22` +- **Evidence:** + ```ruby + render json: { id: user.id, name: user.name, email: user.email, role: user.role } + ``` + Combined with no authentication on the route (`routes.rb:2`: `get "/users", to: "users#show"`). +- **EXPLOIT:** `GET /users?name=` is entirely unauthenticated — no `before_action :authenticate`, no session check. Any anonymous attacker enumerates arbitrary users' `email` (PII) and `role` (privilege level), building a hit list of `admin` accounts to target via SEC-001 / SEC-004. Combined with SEC-001 the attacker can also coerce the query to return the `password` column through UNION selects into the `name` / `email` / `role` output fields. +- **Severity:** High + +**SEC-007: Broken Access Control — No Authentication on Sensitive Endpoints** +- **OWASP:** A01 — Broken Access Control +- **Location:** `config/routes.rb:2` and `app/controllers/application_controller.rb` (no `before_action`) +- **Evidence:** `ApplicationController < ActionController::API` contains no authentication filter; `UsersController` has no `before_action`; `/users` is reachable without a session. +- **EXPLOIT:** Unauthenticated enumeration of any user record by name. There is no `authenticate_user!`, no authorization check that the caller is the subject, and no rate limiting. See SEC-006 for the direct PII exposure this enables. +- **Severity:** High + +## Security Improvement Summary + +### What Was Found + +The scaffold contains a cluster of severe, interacting issues. `User.find_by_name` builds SQL via string interpolation and is called from both the public `/users` endpoint and the `/login` endpoint, yielding unauthenticated SQL injection (SEC-001). Production database credentials are committed in plaintext (SEC-002). Passwords are stored in plaintext and compared with a non-constant-time operator, with a separate code path for missing users that enables username enumeration (SEC-003, SEC-004). Uncaught exceptions return raw error messages to clients, accelerating error-based injection (SEC-005). The `/users` endpoint requires no authentication and returns email and role for any named user (SEC-006, SEC-007). + +### How to Improve + +1. Replace the interpolated query in `User.find_by_name` with a parameterized query — either `User.find_by(name: name)` or `sanitize_sql_array(["... WHERE name = ?", name])`, mirroring `find_by_user_id`. (SEC-001) +2. Remove `config/database.yml` credentials from version control; source them from `ENV` or Rails encrypted credentials, rotate the leaked `admin` password immediately, and split dev/prod configs. (SEC-002) +3. Adopt `has_secure_password` with bcrypt; migrate existing rows and drop the plaintext `password` column. (SEC-003) +4. Use `ActiveSupport::SecurityUtils.secure_compare` on hashed digests and run the comparison even when the user is not found (dummy bcrypt) so both branches take equal time. (SEC-004) +5. Replace the blanket `rescue_from StandardError` with a handler that logs server-side and returns an opaque error id to clients. (SEC-005) +6. Add a `before_action :authenticate_user!` on `UsersController`, restrict the response to fields the caller is authorized to see, and consider scoping `/users` to the current user or an admin role. (SEC-006, SEC-007) +7. Add a `Gemfile.lock` to the scaffold and run `bundler-audit` in CI to cover the dependency evidence gap. + +### How to Prevent This Going Forward + +1. Enable Brakeman in CI — it flags raw-SQL interpolation (SEC-001), `rescue_from StandardError` leaks (SEC-005), and unauthenticated controller actions. +2. Adopt `bundler-audit` and Dependabot to catch vulnerable gems before they land. +3. Add a pre-commit secret scanner (gitleaks, trufflehog) to prevent credentials like SEC-002 from ever being committed. +4. Establish a project convention that all password handling flows through `has_secure_password`, forbidding any `:password` string column in migrations via a schema lint. (SEC-003, SEC-004) +5. Make authentication opt-out rather than opt-in: put `before_action :authenticate_user!` on `ApplicationController` and require controllers to explicitly `skip_before_action` with justification. (SEC-007) +6. Require a response-shape serializer (e.g., `ActiveModel::Serializer` or Jbuilder template) so sensitive fields like `password`, `email`, and `role` cannot be added to a response by accident. (SEC-006) diff --git a/evals/iterative-plan-review/scaffolds/ruby-security-app/plans/code-review-result.plan.md b/evals/iterative-plan-review/scaffolds/ruby-security-app/plans/code-review-result.plan.md new file mode 100644 index 00000000..c8ff08c1 --- /dev/null +++ b/evals/iterative-plan-review/scaffolds/ruby-security-app/plans/code-review-result.plan.md @@ -0,0 +1,224 @@ +# Plan: Remediate `ruby-security-app` Code Review Findings + +This plan resolves every item in `code-review-result.md`: 15 critical issues, 18 warnings, 8 suggestions, and 7 security vulnerabilities (SEC-001..SEC-007). CRIT-009..CRIT-015 are duplicates of SEC-001..SEC-007 and are addressed together. + +## Guiding Principles + +- **Characterize, then fix.** For each critical defect, land a failing/characterization test first (in the same PR as the fix) so the behavioral change is visible in the diff. This is what CRIT-001..CRIT-003 and WARN-010 are asking for. +- **Rotate before removing.** Secrets already committed (SEC-002) must be rotated on the live systems *before* they are scrubbed from the repo, or the scrub is cosmetic. +- **Auth opt-out, not opt-in.** Move `authenticate_user!` onto `ApplicationController` and have exceptions `skip_before_action` explicitly. Prevents the whole class of SEC-007-style omissions. +- **Fail closed on errors.** Exceptions must never render `e.message` to clients. All paths go through an opaque error with a server-side correlation id. + +--- + +## Phase 0 — Prerequisites (no code changes yet) + +These are operational steps that gate Phase 1. They are not code edits but must complete before the repo changes ship. + +1. **Rotate the leaked DB credentials.** The `admin` / `Password123!` pair for `prod-db.internal` in `config/database.yml` is compromised the moment it lands in git history. Rotate the password on `prod-db.internal` and on every environment sharing the YAML anchor. (SEC-002 / CRIT-010) +2. **Audit DB access logs** for the leaked credentials window — any successful auth from outside the expected app hosts is an incident, not a remediation. (SEC-002) +3. **Decide credential source of truth.** Rails encrypted credentials (`config/credentials/*.yml.enc`) vs. environment variables. This plan assumes ENV for portability; adjust step 2.1 if the team chooses encrypted credentials. + +--- + +## Phase 1 — Stop the bleeding (Critical security fixes) + +Goal: close every unauthenticated remote exploit path. Everything in this phase ships together; partial fixes leave exploitable combinations (e.g. fixing SQLi without fixing the rescue handler still leaks DB schema via WARN-012). + +### 1.1 — Parameterize `User.find_by_name` (SEC-001, CRIT-004, CRIT-009, CRIT-006 partial) + +**File:** `app/models/user.rb:5-17` + +- Replace the string-interpolated query in `find_by_name` with `sanitize_sql_array(["... WHERE name = ? LIMIT 1", name])`, mirroring `find_by_user_id`. +- Add `LIMIT 1` so the query's single-row contract does not depend on the unique index staying in place (CRIT-006). +- Extract the shared row-to-User copy into a private `self.row_to_user(result)` helper — this is also WARN-001, landed here to keep the diff minimal. Body: `return nil unless result; user = new; user.id = result["id"]; user.name = ...; user`. +- Both finders call the helper. + +**Tests to land with the fix (same PR):** +- CRIT-001: characterization test `User.find_by_name("nobody' OR name='admin' --")` that asserted the vulnerable return before the fix, now asserts `nil` after. +- CRIT-003: `User.find_by_user_id("1 OR 1=1")` returns `nil` — pins the parameterization invariant on the already-safe finder. +- WARN-008, WARN-009: happy-path tests that insert a row and assert every field is populated by each finder. +- SUGG-004, SUGG-005: `nil` return tests for the no-match branch on both finders. +- WARN-012: legitimate apostrophe name (`O'Brien`) returns a `User`, does not raise. +- WARN-013: canonicalization test — fails intentionally at this phase (Unicode NFC / case / trailing whitespace) and is fixed in Phase 3. + +### 1.2 — Adopt `has_secure_password` and constant-time compare (SEC-003, SEC-004, CRIT-002, CRIT-005, CRIT-011, CRIT-012) + +**Files:** `app/models/user.rb`, `app/controllers/sessions_controller.rb:14-22`, `db/migrate/001_create_users.rb:6`, new migration `db/migrate/002_add_password_digest_to_users.rb`. + +- New migration adds `password_digest:string, null: false` and backfills existing rows via a one-shot bcrypt of the legacy plaintext column. Drop the plaintext `password` column in a follow-up migration *after* all readers are off it. Do not drop in the same migration — it kills rollback. +- `User` adds `has_secure_password`. Remove `user.password = result["password"]` from `row_to_user`; the digest is loaded by AR automatically once `password_digest` exists. +- `SessionsController#create` replaces `user.password != password` with `user.authenticate(password)`. +- Equal-time branches: even when `User.find_by_name` returns `nil`, run a dummy `BCrypt::Password.create("")` comparison before returning `401`, so the user-not-found and wrong-password timings match. +- Both failure branches return the same JSON body (`"invalid credentials"`) — they already do (noted in "What's Good"), keep it. + +**Tests to land with the fix:** +- CRIT-002: happy-path login test (seed user, POST correct creds, assert 200 + `session[:user_id]` + `session[:role]`). +- WARN-002: same test, verifying the contract. +- SEC-004 timing: `user.authenticate` is called on both the nil-user and wrong-password paths (assert via stub that the dummy compare ran when the user was missing). +- CRIT-005 removal: assert `User` responds to `authenticate` and does not expose `#password` as a readable plaintext attribute. + +### 1.3 — Replace the `rescue_from StandardError` leak (SEC-005, CRIT-013, WARN-010, WARN-016, WARN-017) + +**File:** `app/controllers/application_controller.rb:2-4` + +- Replace the handler body with: + - Generate a correlation id (`SecureRandom.uuid`). + - `Rails.logger.error(...)` server-side including the class, message, backtrace, and correlation id. + - `render json: { error: "internal error", id: correlation_id }, status: :internal_server_error`. +- The client never sees `e.message`. This closes the error-based SQLi oracle (WARN-012, WARN-017) and the generic leak (SEC-005). + +**Tests to land with the fix:** +- WARN-010: stub `User.find_by_name` to raise; assert response body does not contain the exception message and does contain a UUID-shaped `id`. +- WARN-016: stub to raise `ActiveRecord::ConnectionNotEstablished`; assert 500 + opaque body. + +### 1.4 — Authenticate `/users` and add `reset_session` on login (SEC-006, SEC-007, CRIT-008, CRIT-014, CRIT-015, CRIT-007) + +**Files:** `app/controllers/application_controller.rb`, `app/controllers/users_controller.rb`, `app/controllers/sessions_controller.rb:23-25`, `config/routes.rb`. + +- `ApplicationController` gains `before_action :authenticate_user!`, implemented as: look up `session[:user_id]`, set `@current_user`, 401 if missing. +- `SessionsController` `skip_before_action :authenticate_user!, only: :create` (login cannot require a session). +- `UsersController` stays authenticated; additionally narrow the response to the current user unless `@current_user.role == "admin"` (SEC-006 scoping). Do not remove the field allow-list — keep `{ id, name, email, role }` as-is, never serialize the whole record. +- In `SessionsController#create`, call `reset_session` *before* assigning `session[:user_id]` / `session[:role]` to close the fixation window (CRIT-007, WARN-018 interacts). + +**Tests:** +- CRIT-008: unauthenticated `GET /users?name=alice` → 401. +- WARN-005: authenticated `GET /users` response body has exactly the fields `id`, `name`, `email`, `role` — no `password`, no `password_digest`. +- CRIT-007: pre-seed a session cookie, log in, assert `session.id` changed. + +--- + +## Phase 2 — Input hardening and error-path coverage + +Runs after Phase 1. Each item is small and independently landable. + +### 2.1 — Secrets out of the repo (SEC-002, CRIT-010) + +**File:** `config/database.yml` + +- Replace hardcoded `host`, `username`, `password`, `database` with `<%= ENV.fetch("DATABASE_HOST") %>` (ERB) equivalents. +- Split `development` and `production` so they do not share a YAML anchor — a typo should not promote a dev value into prod. +- Add `config/database.yml` to `.gitignore`? **No** — Rails reads it at boot. Instead, commit a sanitized template and keep the ENV values out of git. If the team prefers encrypted credentials, use `Rails.application.credentials.database` and keep `master.key` out of version control. +- Add a CI check (Phase 4) that greps `config/database.yml` for literal `Password123!` and any `.internal` host. + +Note: this is *in addition to* the Phase 0 rotation. The file scrub without rotation is useless; rotation without the scrub leaves the next dev to rediscover it. + +### 2.2 — Input validation on login and user show (WARN-011, WARN-014, WARN-015, SUGG-006, SUGG-008) + +**Files:** `app/controllers/sessions_controller.rb:5-11`, `app/controllers/users_controller.rb:5-9`. + +- Extract a helper `scalar_param(name, max: 255)` on `ApplicationController` that: + - Returns `nil` if the value is not a `String` (rejects `Array` / `ActionController::Parameters` — WARN-011). + - Returns `nil` if over `max` bytes (WARN-014). + - Does *not* strip whitespace — whitespace normalization for names happens in the model (see 3.1). +- Both controllers call it for `name` / `username` / `password`, returning 400 `invalid request body` on `nil`. +- WARN-015: change the blank-param branch in `SessionsController` to return `401 invalid credentials` instead of `400 invalid request body`, so blank and wrong-password share a response shape. (Trade-off acknowledged: this makes malformed requests indistinguishable from bad credentials for legitimate clients. The review explicitly asks for it; preserving 400 for explicitly-missing keys while returning 401 on wrong values is an acceptable middle ground if the team pushes back.) +- SUGG-006: reject literal `"null"`, `"undefined"`, `"NaN"` in `scalar_param` or document that they are treated as literal names. Default here: reject with 400, because no legitimate user has a name of `"null"`. +- SUGG-008: allow whitespace-only passwords through the controller (they are a legitimate digest input); enforce a "no whitespace-only" rule at registration, not login. Add a TODO comment since registration is out of scope for this scaffold. + +**Tests:** characterize every branch — WARN-003, WARN-004, WARN-006, WARN-007 each get a focused controller test. + +### 2.3 — Logout route (SUGG-002) + +**Files:** `config/routes.rb`, `app/controllers/sessions_controller.rb`. + +- Add `delete "/logout", to: "sessions#destroy"`. +- `SessionsController#destroy` calls `reset_session` and returns `{ status: "logged_out" }`. +- Authenticated via the `ApplicationController` default; no skip. + +### 2.4 — CSRF posture for cookie-backed login (WARN-018) + +**File:** `app/controllers/application_controller.rb`. + +- Document in a comment that `ActionController::API` does not run CSRF verification, and that the login endpoint is mitigated by: (a) `reset_session` on successful auth (Phase 1.4), (b) SameSite=Lax cookies (set in `config/initializers/session_store.rb`), and (c) requiring `Content-Type: application/json` on `sessions#create`. +- If the app ever serves HTML, switch the base class to `ActionController::Base` and enable `protect_from_forgery`. Out of scope for this scaffold but noted as a follow-up. + +--- + +## Phase 3 — Schema and data-hygiene fixes + +### 3.1 — Name canonicalization (WARN-013) + +**Files:** `app/models/user.rb`, new migration `db/migrate/003_add_name_normalized.rb`. + +- Add a `name_normalized` column (downcased, NFC, stripped). Backfill from existing `name`. Unique index moves to `name_normalized`. +- `User.find_by_name(name)` normalizes the argument the same way before the parameterized query. +- The visible `name` is preserved; only the lookup key is canonical. +- Tests: `find_by_name("ALICE")`, `find_by_name(" alice ")`, NFC vs NFD `café` all resolve to the same row. + +### 3.2 — `role` whitelist (SUGG-007) + +**File:** new migration `db/migrate/004_add_role_check_constraint.rb`. + +- `add_check_constraint :users, "role IN ('user', 'admin')", name: "users_role_whitelist"`. +- Supplement with a `User#role` enum or a `validates :role, inclusion: { in: %w[user admin] }` so app-layer errors are clear. + +--- + +## Phase 4 — Tooling, tests, and guardrails + +### 4.1 — Test framework (SUGG-003) + +**File:** `Gemfile` + +- Add `group :development, :test do ... gem "rspec-rails"` (or `minitest` if the team prefers the Rails default — pick one, don't mix). +- `bin/rails generate rspec:install` (or the minitest equivalent). +- All tests written in earlier phases live in `spec/` (or `test/`). This is listed last intentionally — the plan above reads as if tests already run, but the framework must be wired before any of them can land. **In execution order, 4.1 is the first thing to do in Phase 1.** It is listed here under Phase 4 only because it is a tooling concern. + +### 4.2 — Gemfile.lock (SUGG-001) + +- Run `bundle install`, commit the resulting `Gemfile.lock`. +- Wire `bundler-audit` into CI. + +### 4.3 — Static analysis and secret scanning + +- Add Brakeman to CI — it would have flagged SEC-001, SEC-005, and SEC-007 independently. +- Add `gitleaks` (or `trufflehog`) as a pre-commit hook and a CI job, so SEC-002 cannot recur. +- Add a schema lint that forbids `t.string :password` on any table. + +### 4.4 — Auth-by-default convention (SEC-007 prevention) + +- Project README / `docs/conventions.md` entry: every new controller inherits `authenticate_user!`. `skip_before_action` requires a code comment explaining why and a linked ticket. +- Optional: a RuboCop custom cop that flags `skip_before_action :authenticate_user!` without a preceding comment. + +### 4.5 — Response serializer (SEC-006 prevention) + +- Introduce `ActiveModel::Serializer` or Jbuilder views so `users#show`'s field list lives in one declarative place. +- Makes it structurally impossible to accidentally add `password_digest` to a response (the review's prevention item 6). + +--- + +## Cross-Reference: Review ID → Plan Section + +| Review ID | Section | +|---|---| +| CRIT-001, CRIT-003, WARN-008, WARN-009, SUGG-004, SUGG-005, WARN-012 | 1.1 | +| CRIT-002, CRIT-005, CRIT-011, CRIT-012, WARN-002, SEC-003, SEC-004 | 1.2 | +| CRIT-004, CRIT-009, SEC-001, CRIT-006 | 1.1 | +| CRIT-013, SEC-005, WARN-010, WARN-016, WARN-017 | 1.3 | +| CRIT-007, CRIT-008, CRIT-014, CRIT-015, SEC-006, SEC-007, WARN-005 | 1.4 | +| CRIT-010, SEC-002 | 0, 2.1 | +| WARN-001 | 1.1 (extracted with the fix) | +| WARN-003, WARN-004, WARN-006, WARN-007, WARN-011, WARN-014, WARN-015, SUGG-006, SUGG-008 | 2.2 | +| SUGG-002 | 2.3 | +| WARN-018 | 2.4 | +| WARN-013 | 3.1 | +| SUGG-007 | 3.2 | +| SUGG-003 | 4.1 (executed first) | +| SUGG-001 | 4.2 | + +--- + +## Open Questions / Decisions Needed + +1. **Test framework choice** — `rspec-rails` or `minitest`? Affects 4.1 and every test listed above. +2. **Credential storage** — ENV vars or Rails encrypted credentials? Affects 2.1. +3. **WARN-015 trade-off** — Collapse blank-param and wrong-password to a single 401, or keep 400 for structurally-malformed requests? The review recommends the former; some clients rely on 400 to distinguish bugs from bad passwords. +4. **Plaintext `password` column drop** — one migration or two? Two is safer (readers migrate, then column drops) but is more operational overhead. +5. **7 omitted edge cases (EC17..EC23)** — a second code review is noted as recommended. Should that be scheduled after Phase 3 lands, or deferred to the next planning cycle? + +## Non-Goals + +- User registration / password reset flows — not in the scaffold, out of scope. +- Rate limiting on `/login` — worth doing but not in any review finding. +- Moving off `ActionController::API` to `ActionController::Base` — deferred (see 2.4). diff --git a/evals/iterative-plan-review/tests.json b/evals/iterative-plan-review/tests.json new file mode 100644 index 00000000..f6f2a8e8 --- /dev/null +++ b/evals/iterative-plan-review/tests.json @@ -0,0 +1,180 @@ +{ + "plugins": ["han-communication", "han-core", "han-planning"], + "tests": [ + { + "name": "Skill Call: /iterative-plan-review", + "type": "skill-call", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-iterative-plan-review.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: iterate for correctness", + "type": "skill-call", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-iterate-for-correctness.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: prompt with /iterative-plan-review", + "type": "skill-call", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "prompt-iterative-plan-review.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: no iterative-plan-review", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-no-iterative-plan-review.md", + "scaffold": "ruby-project", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: validate the current plan", + "type": "skill-call", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-validate-current-plan.md", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: iterate the plan", + "type": "skill-call", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-iterate-the-plan.md", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: check plan for correctness and consistency", + "type": "skill-call", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-check-plan-correctness.md", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: verify this will work as expected", + "type": "skill-call", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-verify-will-work.md", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: follow plan and implement changes", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-follow-plan-implement.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: create a new plan for auth", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-create-new-plan.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: review the code changes", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-review-code-changes.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: verify the test suite passes", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-verify-test-suite.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: write a test plan for auth module", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-test-plan-auth.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: investigate login 500 errors", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-investigate-login-500.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: document the payment system", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-document-payment-system.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Call: post a PR review to GitHub", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "skill-call-post-pr-review.md", + "expect": [ + { "skill-call": false } + ] + }, + { + "name": "Skill Prompt: iterative-plan-review on ruby-security-app code-review plan", + "type": "skill-prompt", + "model": "opus", + "skillFile": "han-planning:iterative-plan-review", + "promptFile": "prompt-review-code-review-result-plan.md", + "scaffold": "ruby-security-app", + "expect": [ + { "llm-judge": { "rubricFile": "iterative-plan-review-quality.md", "model": "opus", "threshold": 0.8 } } + ] + } + ] +} diff --git a/evals/post-code-review-to-pr/prompts/prompt-no-op.md b/evals/post-code-review-to-pr/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/post-code-review-to-pr/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/post-code-review-to-pr/prompts/prompt-post-code-review-to-pr.md b/evals/post-code-review-to-pr/prompts/prompt-post-code-review-to-pr.md new file mode 100644 index 00000000..2a33eaa8 --- /dev/null +++ b/evals/post-code-review-to-pr/prompts/prompt-post-code-review-to-pr.md @@ -0,0 +1 @@ +run /post-code-review-to-pr to post a full code review to the current PR diff --git a/evals/post-code-review-to-pr/tests.json b/evals/post-code-review-to-pr/tests.json new file mode 100644 index 00000000..23d20128 --- /dev/null +++ b/evals/post-code-review-to-pr/tests.json @@ -0,0 +1,25 @@ +{ + "plugins": ["han-communication", "han-core", "han-coding", "han-github"], + "tests": [ + { + "name": "Prompt: /post-code-review-to-pr", + "type": "skill-prompt", + "promptFile": "prompt-post-code-review-to-pr.md", + "model": "opus", + "expect": [ + { "result-contains": "gh CLI" }, + { "skill-call": { "skill": "han-github:post-code-review-to-pr", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "gh CLI" }, + { "skill-call": { "skill": "han-github:post-code-review-to-pr", "expected": false } } + ] + } + ] +} diff --git a/evals/project-discovery/prompts/prompt-no-op.md b/evals/project-discovery/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/project-discovery/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/project-discovery/prompts/prompt-project-discovery.md b/evals/project-discovery/prompts/prompt-project-discovery.md new file mode 100644 index 00000000..d34943ce --- /dev/null +++ b/evals/project-discovery/prompts/prompt-project-discovery.md @@ -0,0 +1 @@ +run /project-discovery to scan and document the tech stack of this project diff --git a/evals/project-discovery/prompts/skill-call-no-project-discovery.md b/evals/project-discovery/prompts/skill-call-no-project-discovery.md new file mode 100644 index 00000000..bde084d2 --- /dev/null +++ b/evals/project-discovery/prompts/skill-call-no-project-discovery.md @@ -0,0 +1 @@ +document how the test runner feature works in this project diff --git a/evals/project-discovery/prompts/skill-call-project-discovery.md b/evals/project-discovery/prompts/skill-call-project-discovery.md new file mode 100644 index 00000000..55a39499 --- /dev/null +++ b/evals/project-discovery/prompts/skill-call-project-discovery.md @@ -0,0 +1 @@ +scan this repository and detect the tech stack, build tools, and project structure diff --git a/evals/project-discovery/scaffolds/polyglot-project/node-app/package.json b/evals/project-discovery/scaffolds/polyglot-project/node-app/package.json new file mode 100644 index 00000000..3b070968 --- /dev/null +++ b/evals/project-discovery/scaffolds/polyglot-project/node-app/package.json @@ -0,0 +1,5 @@ +{ + "name": "node-app", + "version": "1.0.0", + "main": "src/index.js" +} diff --git a/evals/project-discovery/scaffolds/polyglot-project/node-app/src/index.js b/evals/project-discovery/scaffolds/polyglot-project/node-app/src/index.js new file mode 100644 index 00000000..4ed55b52 --- /dev/null +++ b/evals/project-discovery/scaffolds/polyglot-project/node-app/src/index.js @@ -0,0 +1,5 @@ +function add(a, b) { + return a + b; +} + +module.exports = { add }; diff --git a/evals/project-discovery/scaffolds/polyglot-project/ruby-app/Gemfile b/evals/project-discovery/scaffolds/polyglot-project/ruby-app/Gemfile new file mode 100644 index 00000000..58688a91 --- /dev/null +++ b/evals/project-discovery/scaffolds/polyglot-project/ruby-app/Gemfile @@ -0,0 +1,2 @@ +source "https://rubygems.org" +gem "minitest" diff --git a/evals/project-discovery/scaffolds/polyglot-project/ruby-app/lib/main.rb b/evals/project-discovery/scaffolds/polyglot-project/ruby-app/lib/main.rb new file mode 100644 index 00000000..84ea6ff8 --- /dev/null +++ b/evals/project-discovery/scaffolds/polyglot-project/ruby-app/lib/main.rb @@ -0,0 +1,5 @@ +class Calculator + def add(a, b) + a + b + end +end diff --git a/evals/project-discovery/tests.json b/evals/project-discovery/tests.json new file mode 100644 index 00000000..d0dbd482 --- /dev/null +++ b/evals/project-discovery/tests.json @@ -0,0 +1,47 @@ +{ + "plugins": ["han-core"], + "tests": [ + { + "name": "Prompt: /project-discovery", + "type": "skill-prompt", + "promptFile": "prompt-project-discovery.md", + "model": "opus", + "scaffold": "polyglot-project", + "expect": [ + { "result-contains": "project-discovery.md" }, + { "skill-call": { "skill": "han-core:project-discovery", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "project-discovery.md" }, + { "skill-call": { "skill": "han-core:project-discovery", "expected": false } } + ] + }, + { + "name": "Skill Call: /project-discovery", + "type": "skill-call", + "model": "opus", + "skillFile": "han-core:project-discovery", + "promptFile": "skill-call-project-discovery.md", + "scaffold": "polyglot-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: no project-discovery", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-core:project-discovery", + "promptFile": "skill-call-no-project-discovery.md", + "expect": [ + { "skill-call": false } + ] + } + ] +} diff --git a/evals/project-documentation/prompts/prompt-no-op.md b/evals/project-documentation/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/project-documentation/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/project-documentation/prompts/prompt-project-documentation.md b/evals/project-documentation/prompts/prompt-project-documentation.md new file mode 100644 index 00000000..9ba5217c --- /dev/null +++ b/evals/project-documentation/prompts/prompt-project-documentation.md @@ -0,0 +1 @@ +run /project-documentation to write documentation for the test runner feature diff --git a/evals/project-documentation/prompts/skill-call-no-project-documentation.md b/evals/project-documentation/prompts/skill-call-no-project-documentation.md new file mode 100644 index 00000000..2d77ac3d --- /dev/null +++ b/evals/project-documentation/prompts/skill-call-no-project-documentation.md @@ -0,0 +1 @@ +scan this repository and detect what languages and frameworks are used diff --git a/evals/project-documentation/prompts/skill-call-project-documentation.md b/evals/project-documentation/prompts/skill-call-project-documentation.md new file mode 100644 index 00000000..857b1545 --- /dev/null +++ b/evals/project-documentation/prompts/skill-call-project-documentation.md @@ -0,0 +1 @@ +write documentation for how the test runner feature works diff --git a/evals/project-documentation/scaffolds/node-project/README.md b/evals/project-documentation/scaffolds/node-project/README.md new file mode 100644 index 00000000..539703be --- /dev/null +++ b/evals/project-documentation/scaffolds/node-project/README.md @@ -0,0 +1,3 @@ +# Test Project + +A test runner utility. diff --git a/evals/project-documentation/scaffolds/node-project/package.json b/evals/project-documentation/scaffolds/node-project/package.json new file mode 100644 index 00000000..632b9726 --- /dev/null +++ b/evals/project-documentation/scaffolds/node-project/package.json @@ -0,0 +1,5 @@ +{ + "name": "test-project", + "version": "1.0.0", + "main": "src/index.js" +} diff --git a/evals/project-documentation/scaffolds/node-project/src/index.js b/evals/project-documentation/scaffolds/node-project/src/index.js new file mode 100644 index 00000000..d56a6a7d --- /dev/null +++ b/evals/project-documentation/scaffolds/node-project/src/index.js @@ -0,0 +1,19 @@ +/** + * Runs a test suite and returns the results. + * @param {string} suiteName - Name of the test suite to run + * @param {Object} options - Configuration options + * @param {boolean} options.verbose - Whether to print detailed output + * @returns {Object} Test results with pass/fail counts + */ +function runTests(suiteName, options = {}) { + const { verbose = false } = options; + const results = { passed: 0, failed: 0, errors: [] }; + + if (verbose) { + console.log(`Running suite: ${suiteName}`); + } + + return results; +} + +module.exports = { runTests }; diff --git a/evals/project-documentation/tests.json b/evals/project-documentation/tests.json new file mode 100644 index 00000000..6967ecaf --- /dev/null +++ b/evals/project-documentation/tests.json @@ -0,0 +1,47 @@ +{ + "plugins": ["han-communication", "han-core", "han-documentation"], + "tests": [ + { + "name": "Prompt: /project-documentation", + "type": "skill-prompt", + "promptFile": "prompt-project-documentation.md", + "model": "opus", + "scaffold": "node-project", + "expect": [ + { "result-contains": "docs/" }, + { "skill-call": { "skill": "han-documentation:project-documentation", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "docs/" }, + { "skill-call": { "skill": "han-documentation:project-documentation", "expected": false } } + ] + }, + { + "name": "Skill Call: /project-documentation", + "type": "skill-call", + "model": "opus", + "skillFile": "han-documentation:project-documentation", + "promptFile": "skill-call-project-documentation.md", + "scaffold": "node-project", + "expect": [ + { "skill-call": true } + ] + }, + { + "name": "Skill Call: no project-documentation", + "type": "skill-call", + "model": "sonnet", + "skillFile": "han-documentation:project-documentation", + "promptFile": "skill-call-no-project-documentation.md", + "expect": [ + { "skill-call": false } + ] + } + ] +} diff --git a/evals/test-engineer/prompts/agent-call-behavioral-test-plan.md b/evals/test-engineer/prompts/agent-call-behavioral-test-plan.md new file mode 100644 index 00000000..22fa8d80 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-behavioral-test-plan.md @@ -0,0 +1 @@ +Create a behavioral test plan for the API endpoints — focus on inputs, outputs, and side effects \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-find-untested-behaviors.md b/evals/test-engineer/prompts/agent-call-find-untested-behaviors.md new file mode 100644 index 00000000..1a521910 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-find-untested-behaviors.md @@ -0,0 +1 @@ +Find untested behaviors in the user service and recommend what tests to write \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-negative-behavior-psychology.md b/evals/test-engineer/prompts/agent-call-negative-behavior-psychology.md new file mode 100644 index 00000000..0dbe04ca --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-negative-behavior-psychology.md @@ -0,0 +1 @@ +Explain behavioral patterns in consumer psychology and how they affect purchasing decisions \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-negative-engineering-plan.md b/evals/test-engineer/prompts/agent-call-negative-engineering-plan.md new file mode 100644 index 00000000..7eb36a5c --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-negative-engineering-plan.md @@ -0,0 +1 @@ +Plan the sprint for next week and assign tasks to the engineering team \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-negative-sports-test.md b/evals/test-engineer/prompts/agent-call-negative-sports-test.md new file mode 100644 index 00000000..677c3b30 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-negative-sports-test.md @@ -0,0 +1 @@ +What were the test results from last night's game? \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-plan-tests-for-auth-module.md b/evals/test-engineer/prompts/agent-call-plan-tests-for-auth-module.md new file mode 100644 index 00000000..ee91cdae --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-plan-tests-for-auth-module.md @@ -0,0 +1 @@ +Plan what tests we need for the auth module \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-prioritize-test-cases.md b/evals/test-engineer/prompts/agent-call-prioritize-test-cases.md new file mode 100644 index 00000000..410477c9 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-prioritize-test-cases.md @@ -0,0 +1 @@ +Look at the database layer and prioritize which test cases we should write first based on risk \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-sibling-architecture.md b/evals/test-engineer/prompts/agent-call-sibling-architecture.md new file mode 100644 index 00000000..e9656e76 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-sibling-architecture.md @@ -0,0 +1 @@ +Analyze the architectural structure of the internal packages and identify coupling problems \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-sibling-code-review.md b/evals/test-engineer/prompts/agent-call-sibling-code-review.md new file mode 100644 index 00000000..f1d72e60 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-sibling-code-review.md @@ -0,0 +1 @@ +Review the code in the handlers package for quality issues, security flaws, and best practice violations \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-sibling-edge-cases.md b/evals/test-engineer/prompts/agent-call-sibling-edge-cases.md new file mode 100644 index 00000000..ff48dd31 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-sibling-edge-cases.md @@ -0,0 +1 @@ +Explore all the edge cases and boundary values for the authentication module — what inputs could cause it to fail? \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-sibling-investigate-bug.md b/evals/test-engineer/prompts/agent-call-sibling-investigate-bug.md new file mode 100644 index 00000000..a5f3ac95 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-sibling-investigate-bug.md @@ -0,0 +1 @@ +Investigate why the user creation endpoint is returning 500 errors in production — find the root cause \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-call-test-coverage-gaps.md b/evals/test-engineer/prompts/agent-call-test-coverage-gaps.md new file mode 100644 index 00000000..e52942c5 --- /dev/null +++ b/evals/test-engineer/prompts/agent-call-test-coverage-gaps.md @@ -0,0 +1 @@ +Examine the handlers package and identify test coverage gaps — what behaviors are missing tests? \ No newline at end of file diff --git a/evals/test-engineer/prompts/agent-prompt-go-security-test-plan.md b/evals/test-engineer/prompts/agent-prompt-go-security-test-plan.md new file mode 100644 index 00000000..00859fa3 --- /dev/null +++ b/evals/test-engineer/prompts/agent-prompt-go-security-test-plan.md @@ -0,0 +1 @@ +Examine the Go project and create a test plan. Identify all untested behaviors, recommend test doubles for isolation, and prioritize test cases by value and brittleness risk. Write the full analysis to test-plan.md. \ No newline at end of file diff --git a/evals/test-engineer/rubrics/test-engineer-quality.md b/evals/test-engineer/rubrics/test-engineer-quality.md new file mode 100644 index 00000000..4620a9df --- /dev/null +++ b/evals/test-engineer/rubrics/test-engineer-quality.md @@ -0,0 +1,38 @@ +## Rubric: test-engineer of go-security-project scaffold + +### Presence — things the test plan must identify +- The test plan identifies that the auth package (auth.go, jwt.go) has no tests covering password comparison or JWT generation/validation behaviors +- The test plan identifies that the database layer (db.go, users.go) has no tests covering connection initialization or user query behaviors +- The test plan identifies that the HTTP handlers (handlers/users.go, handlers/fetch.go) have no tests covering request handling, response formation, or error responses +- The test plan identifies that runner_test.go exists as the only test file and notes the testing patterns it uses +- The test plan identifies outgoing commands (database writes, HTTP responses) and incoming queries (database reads, JWT operations) as collaborator interactions requiring test doubles +- The test plan includes a priority table with High, Medium, and Low counts + +### Specificity — the test plan must be concrete +- Every test recommendation references a specific entry point with a file path and line number (e.g., `internal/auth/auth.go:15`) +- Test recommendations specify which collaborators to stub (queries) and which to mock (commands) for each test case +- Each test case specifies the expected observable output — what the function returns, what side effect occurs, or what error surfaces to the caller +- The test plan references the existing runner_test.go file and its patterns when recommending how new tests should be structured + +### Depth — the test plan must be actionable +- Each test case includes a concrete test approach with behavior description, stubs, input/action, expected output, and expected commands +- The test plan evaluates brittleness risk for each recommendation — explaining why a test would or would not break on routine refactors +- The test plan recommends appropriate test levels (unit vs integration) with justification — not defaulting everything to unit tests +- The test plan distinguishes between high-value behaviors (data integrity, security boundaries) and low-value behaviors when prioritizing + +### Absence — the test plan must not do these things +- The test plan must not recommend writing test code — it produces a plan only +- The test plan must not recommend snapshot or golden-file tests without evaluating churn risk +- The test plan must not recommend tests that assert on mock internals (call counts, argument order) without tying them to observable behavioral outcomes +- The test plan must not include test recommendations for behaviors with no meaningful observable outcome + +## File: test-plan.md +### Presence +- The file contains a Summary section with a priority count table +- The file contains a Findings section with T-series items ordered by priority +- The file contains a Coverage Assessment describing which behaviors are well-tested vs have gaps +- The file contains a Scope section identifying the files analyzed + +### Specificity +- Each T-series finding includes Priority, Test level, Entry point, Gap type, Test approach, and Brittleness assessment fields +- The Coverage Estimate section specifies expected behavioral coverage after all recommended tests are written diff --git a/evals/test-engineer/scaffolds/go-security-project/cmd/server/main.go b/evals/test-engineer/scaffolds/go-security-project/cmd/server/main.go new file mode 100644 index 00000000..234a54d3 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/cmd/server/main.go @@ -0,0 +1,49 @@ +package main + +import ( + "context" + "log" + "net/http" + "os" + "os/signal" + "syscall" + + "github.com/testdouble/userservice/internal/db" + "github.com/testdouble/userservice/internal/handlers" + "github.com/testdouble/userservice/internal/runner" +) + +func main() { + if err := db.Connect(); err != nil { + log.Printf("database connection failed: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go runner.Start(ctx) + + mux := http.NewServeMux() + mux.HandleFunc("/users", handlers.GetUser) + mux.HandleFunc("/login", handlers.Login) + mux.HandleFunc("/fetch", handlers.FetchURL) + + server := &http.Server{ + Addr: ":8080", + Handler: mux, + } + + go func() { + sig := make(chan os.Signal, 1) + signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM) + <-sig + cancel() + server.Close() + }() + + log.Println("Starting userservice on :8080") + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Printf("server error: %v", err) + os.Exit(1) + } +} diff --git a/evals/test-engineer/scaffolds/go-security-project/go.mod b/evals/test-engineer/scaffolds/go-security-project/go.mod new file mode 100644 index 00000000..1c4a23a5 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/go.mod @@ -0,0 +1,5 @@ +module github.com/testdouble/userservice + +go 1.22 + +require github.com/golang-jwt/jwt/v5 v5.2.1 diff --git a/evals/test-engineer/scaffolds/go-security-project/go.sum b/evals/test-engineer/scaffolds/go-security-project/go.sum new file mode 100644 index 00000000..f56d3e63 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/go.sum @@ -0,0 +1,2 @@ +github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/auth/auth.go b/evals/test-engineer/scaffolds/go-security-project/internal/auth/auth.go new file mode 100644 index 00000000..2d5aa2d6 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/auth/auth.go @@ -0,0 +1,21 @@ +package auth + +import ( + "crypto/sha256" + "errors" + "fmt" +) + +// HashPassword creates a SHA-256 digest of the given password string. +func HashPassword(password string) string { + sum := sha256.Sum256([]byte(password)) + return fmt.Sprintf("%x", sum) +} + +// ValidatePassword verifies that the provided password matches the stored hash. +func ValidatePassword(provided, stored string) error { + if HashPassword(provided) == stored { + return nil + } + return errors.New("invalid credentials") +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/auth/jwt.go b/evals/test-engineer/scaffolds/go-security-project/internal/auth/jwt.go new file mode 100644 index 00000000..202e7b22 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/auth/jwt.go @@ -0,0 +1,38 @@ +package auth + +import ( + "fmt" + + "github.com/golang-jwt/jwt/v5" +) + +// jwtKey is the HMAC signing key for all JWT tokens issued by this service. +var jwtKey = []byte("secret") + +// GenerateToken creates a signed JWT for the given user ID and role. +func GenerateToken(userID int, role string) (string, error) { + claims := jwt.MapClaims{ + "user_id": userID, + "role": role, + } + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + return token.SignedString(jwtKey) +} + +// ValidateToken parses and validates a JWT token string, returning its claims. +func ValidateToken(tokenStr string) (jwt.MapClaims, error) { + token, err := jwt.Parse(tokenStr, func(t *jwt.Token) (interface{}, error) { + if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"]) + } + return jwtKey, nil + }) + if err != nil { + return nil, err + } + claims, ok := token.Claims.(jwt.MapClaims) + if !ok || !token.Valid { + return nil, fmt.Errorf("invalid token") + } + return claims, nil +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/db/db.go b/evals/test-engineer/scaffolds/go-security-project/internal/db/db.go new file mode 100644 index 00000000..302ca200 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/db/db.go @@ -0,0 +1,22 @@ +package db + +import ( + "database/sql" + "fmt" +) + +// connStr contains the production database credentials. +const connStr = "postgres://admin:Password123!@prod-db.internal:5432/userservice" + +// DB is the shared database connection used by all query functions. +var DB *sql.DB + +// Connect opens the database connection using the configured credentials. +func Connect() error { + var err error + DB, err = sql.Open("postgres", connStr) + if err != nil { + return fmt.Errorf("opening database: %w", err) + } + return nil +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/db/users.go b/evals/test-engineer/scaffolds/go-security-project/internal/db/users.go new file mode 100644 index 00000000..7d77b291 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/db/users.go @@ -0,0 +1,30 @@ +package db + +import ( + "fmt" + + "github.com/testdouble/userservice/internal/types" +) + +// GetUser retrieves a user record by name. +func GetUser(name string) (*types.User, error) { + query := "SELECT id, name, email, password, role FROM users WHERE name = '" + name + "'" + row := DB.QueryRow(query) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} + +// GetUserByID retrieves a user record by numeric ID. +func GetUserByID(id int) (*types.User, error) { + row := DB.QueryRow("SELECT id, name, email, password, role FROM users WHERE id = $1", id) + + var u types.User + if err := row.Scan(&u.ID, &u.Name, &u.Email, &u.Password, &u.Role); err != nil { + return nil, fmt.Errorf("user not found: %w", err) + } + return &u, nil +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/handlers/fetch.go b/evals/test-engineer/scaffolds/go-security-project/internal/handlers/fetch.go new file mode 100644 index 00000000..17dd901e --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/handlers/fetch.go @@ -0,0 +1,27 @@ +package handlers + +import ( + "io" + "net/http" +) + +// FetchURL proxies content from a remote URL on behalf of the client. +// Used by the frontend to load external resources through the server. +func FetchURL(w http.ResponseWriter, r *http.Request) { + url := r.URL.Query().Get("url") + if url == "" { + http.Error(w, "url parameter required", http.StatusBadRequest) + return + } + + resp, err := http.Get(url) + if err != nil { + http.Error(w, "fetch failed", http.StatusBadGateway) + return + } + defer resp.Body.Close() + + w.Header().Set("Content-Type", resp.Header.Get("Content-Type")) + w.WriteHeader(resp.StatusCode) + io.Copy(w, resp.Body) +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/handlers/users.go b/evals/test-engineer/scaffolds/go-security-project/internal/handlers/users.go new file mode 100644 index 00000000..85fcc1a6 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/handlers/users.go @@ -0,0 +1,59 @@ +package handlers + +import ( + "encoding/json" + "net/http" + + "github.com/testdouble/userservice/internal/auth" + "github.com/testdouble/userservice/internal/db" +) + +// GetUser handles GET /users?name= and returns the matching user record. +func GetUser(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + if name == "" { + http.Error(w, "name parameter required", http.StatusBadRequest) + return + } + + user, err := db.GetUser(name) + if err != nil { + http.Error(w, "user not found", http.StatusNotFound) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) +} + +// Login handles POST /login with JSON credentials and returns a JWT on success. +func Login(w http.ResponseWriter, r *http.Request) { + var creds struct { + Username string `json:"username"` + Password string `json:"password"` + } + if err := json.NewDecoder(r.Body).Decode(&creds); err != nil { + http.Error(w, "invalid request body", http.StatusBadRequest) + return + } + + user, err := db.GetUser(creds.Username) + if err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + if err := auth.ValidatePassword(creds.Password, user.Password); err != nil { + http.Error(w, "invalid credentials", http.StatusUnauthorized) + return + } + + token, err := auth.GenerateToken(user.ID, user.Role) + if err != nil { + http.Error(w, "failed to generate token", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]string{"token": token}) +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/runner/runner.go b/evals/test-engineer/scaffolds/go-security-project/internal/runner/runner.go new file mode 100644 index 00000000..518d70c7 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/runner/runner.go @@ -0,0 +1,52 @@ +package runner + +import ( + "context" + "log" + + "github.com/testdouble/userservice/internal/types" +) + +// responseCache stores the results of processed requests to avoid redundant work. +var responseCache = make(map[string][]byte) + +// processRequest handles a single unit of work and caches the result. +func processRequest(data types.RequestData) { + if _, ok := responseCache[data.Action]; ok { + return + } + result := []byte("processed:" + data.Action) + responseCache[data.Action] = result +} + +// getCached retrieves a previously cached result for the given action key. +func getCached(key string) ([]byte, bool) { + val, ok := responseCache[key] + return val, ok +} + +// Start launches a pool of worker goroutines to process incoming requests. +// The pool runs until the provided context is cancelled. +func Start(ctx context.Context) { + work := make(chan types.RequestData, 10) + + for i := 0; i < 4; i++ { + go func() { + for { + select { + case <-ctx.Done(): + return + case data, ok := <-work: + if !ok { + return + } + processRequest(data) + } + } + }() + } + + log.Println("Runner started with 4 workers") + <-ctx.Done() + close(work) +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/runner/runner_test.go b/evals/test-engineer/scaffolds/go-security-project/internal/runner/runner_test.go new file mode 100644 index 00000000..7ba5d298 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/runner/runner_test.go @@ -0,0 +1,27 @@ +package runner_test + +import ( + "context" + "testing" + "time" + + "github.com/testdouble/userservice/internal/runner" +) + +func TestStart(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + done := make(chan struct{}) + go func() { + runner.Start(ctx) + close(done) + }() + + select { + case <-done: + // success: Start returned after context cancellation + case <-time.After(500 * time.Millisecond): + t.Error("Start did not return after context cancellation") + } +} diff --git a/evals/test-engineer/scaffolds/go-security-project/internal/types/types.go b/evals/test-engineer/scaffolds/go-security-project/internal/types/types.go new file mode 100644 index 00000000..09e4d1f3 --- /dev/null +++ b/evals/test-engineer/scaffolds/go-security-project/internal/types/types.go @@ -0,0 +1,17 @@ +package types + +// User represents an authenticated user in the system. +type User struct { + ID int `json:"id"` + Name string `json:"name"` + Email string `json:"email"` + Password string `json:"-"` + Role string `json:"role"` +} + +// RequestData holds a unit of work to be processed by the runner. +type RequestData struct { + UserID int + Action string + Body []byte +} diff --git a/evals/test-engineer/tests.json b/evals/test-engineer/tests.json new file mode 100644 index 00000000..736a9ec3 --- /dev/null +++ b/evals/test-engineer/tests.json @@ -0,0 +1,112 @@ +{ + "plugins": ["han-core"], + "tests": [ + { + "name": "Agent Call: plan tests for auth module", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-plan-tests-for-auth-module.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: find untested behaviors in user service", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-find-untested-behaviors.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: identify test coverage gaps in handlers", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-test-coverage-gaps.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: prioritize test cases by risk", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-prioritize-test-cases.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: behavioral test plan for API endpoints", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-behavioral-test-plan.md", + "model": "opus", + "expect": [{ "agent-call": true }] + }, + { + "name": "Agent Call: negative — sports test results", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-negative-sports-test.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: negative — sprint planning", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-negative-engineering-plan.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: negative — behavioral psychology", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-negative-behavior-psychology.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — edge case exploration", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-sibling-edge-cases.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — code review", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-sibling-code-review.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — investigate production bug", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-sibling-investigate-bug.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Call: sibling — architectural analysis", + "type": "agent-call", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-call-sibling-architecture.md", + "model": "sonnet", + "expect": [{ "agent-call": false }] + }, + { + "name": "Agent Prompt: test plan for go-security-project", + "type": "agent-prompt", + "agentFile": "han-core:test-engineer", + "promptFile": "agent-prompt-go-security-test-plan.md", + "scaffold": "go-security-project", + "model": "opus", + "expect": [ + { "llm-judge": { "rubricFile": "test-engineer-quality.md", "model": "opus", "threshold": 0.8 } } + ] + } + ] +} diff --git a/evals/update-pr-description/prompts/prompt-no-op.md b/evals/update-pr-description/prompts/prompt-no-op.md new file mode 100644 index 00000000..c15ea581 --- /dev/null +++ b/evals/update-pr-description/prompts/prompt-no-op.md @@ -0,0 +1 @@ +do nothing. just exit with a clean status code diff --git a/evals/update-pr-description/prompts/prompt-update-pr-description.md b/evals/update-pr-description/prompts/prompt-update-pr-description.md new file mode 100644 index 00000000..5a421465 --- /dev/null +++ b/evals/update-pr-description/prompts/prompt-update-pr-description.md @@ -0,0 +1 @@ +run /update-pr-description to generate a PR description for the current branch diff --git a/evals/update-pr-description/tests.json b/evals/update-pr-description/tests.json new file mode 100644 index 00000000..baa44f31 --- /dev/null +++ b/evals/update-pr-description/tests.json @@ -0,0 +1,25 @@ +{ + "plugins": ["han-communication", "han-core", "han-coding", "han-github"], + "tests": [ + { + "name": "Prompt: /update-pr-description", + "type": "skill-prompt", + "promptFile": "prompt-update-pr-description.md", + "model": "opus", + "expect": [ + { "result-contains": "gh CLI" }, + { "skill-call": { "skill": "han-github:update-pr-description", "expected": true } } + ] + }, + { + "name": "Prompt: no op", + "type": "skill-prompt", + "model": "sonnet", + "promptFile": "prompt-no-op.md", + "expect": [ + { "result-does-not-contain": "gh CLI" }, + { "skill-call": { "skill": "han-github:update-pr-description", "expected": false } } + ] + } + ] +}