diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56569da..159e08e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,5 +1,5 @@ # Canonical CI workflow for hawk-eco Go repos. -# Source of truth: .shared-templates/workflows/go-ci.yml.tmpl +# Source of truth: https://github.com/GrayCodeAI/hawk/blob/main/.shared-templates/workflows/go-ci.yml.tmpl # # Two deployment models: # @@ -180,6 +180,26 @@ jobs: run: | npx jscpd --min-lines 5 --min-tokens 50 --reporters console --blame . 2>&1 | head -50 + # ------------------------------------------------------------------------- + # Fuzz the diff parser, which consumes untrusted/adversarial unified-diff + # text from arbitrary git repos being reviewed. + # ------------------------------------------------------------------------- + fuzz: + name: fuzz (60s) + runs-on: ubuntu-latest + needs: [test] + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 + with: + go-version: ${{ env.GO_VERSION }} + cache: true + - name: Run fuzz targets + run: | + go test -fuzz=FuzzParseDiff -fuzztime=60s ./internal/diff + go test -fuzz=FuzzParseHunkHeader -fuzztime=60s ./internal/diff + go test -fuzz=FuzzParseUnifiedDiff -fuzztime=60s ./internal/diff + # ------------------------------------------------------------------------- # Cross-platform build matrix — only for repos that produce a binary. # Repos that are pure libraries can keep this job (it'll just `go build ./...`) diff --git a/AGENTS.md b/AGENTS.md index b5107ba..4976a45 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,112 +1,168 @@ -# AGENTS.md — Sight +--- +description: Extending hawk-eco — how to write AGENTS.md files, custom specialists, skills, hooks, MCP servers, and plugins. +globs: "*.go, *.js, *.md, *.json, *.toml, *.yaml, *.yml" +alwaysApply: false +--- -AI-powered code review library for diffs. Parses unified diffs, enriches with code context and git history, runs parallel multi-concern reviews through an LLM provider. +# Extending hawk-eco -## Design Principles +hawk-eco is an open-source code intelligence platform. This document describes how to extend it with custom tools, skills, hooks, and integrations. -- **Library only** — no CLI, no binary -- **No LLM SDK dependency** — defines a Provider interface; consumers implement it -- **No opinions** — consumers inject their own LLM client (e.g., via eyrie) +## 1. Drop a project `AGENTS.md` -## Build & Test +When hawk-eco starts in a directory, it looks for project-level instructions and injects them into the system prompt. The lookup walks from your current working directory **up to the nearest git root** and reads the first matching file at each level — general rules at the repo root, more specific rules in sub-trees. Files are labeled with their directory in the prompt (e.g. `## Project guidelines (services/api/AGENTS.md)`). + +Accepted file names, in priority order at each level: + +| Path | Notes | +| --- | --- | +| `./AGENTS.md` | The classic spot — committed to your repo, shared with the team. | +| `./ZERO.md` | Brand-specific alias. Same format, lower priority. | +| `./.zero/AGENTS.md` | Project-local, hidden, gitignored. Personal notes that stay out of git. | + +Matching is **case-insensitive** on the basename, so `AGENTS.md`, `Agents.md`, and `agents.md` resolve to the same file on Windows and macOS. The git-tracked filename in this repo is `AGENTS.md` — keep that on case-sensitive filesystems (Linux, the WSL filesystem, or a CI runner) to match what the loader looks for. + +Both files use the same format. YAML frontmatter is optional; the markdown body is loaded as instructions for the agent. hawk-eco reads the file once at session start, so changes take effect on the next launch — not mid-session. + +```markdown +# Project conventions for + +- Build with `make`, not `go build` directly. +- Tests live next to the source file (`foo_test.go` next to `foo.go`). +- Run `make lint` before opening a PR. +- Never edit files under `third_party/` — those are vendored. +``` + +Tips: + +- Keep each file under ~8 KiB. hawk-eco caps the **total** across all matched files at 32 KiB; everything past the cap is dropped. +- Re-state rules in the imperative voice: "Run `make lint`", not "you should consider running the linter". +- Don't put secrets, model IDs, or environment-specific paths in `AGENTS.md`. Use config files for those. +- In a monorepo, drop a narrower `AGENTS.md` in each sub-tree (e.g. `services/api/AGENTS.md`). hawk-eco picks those up automatically when you launch from inside the sub-tree. +- A YAML frontmatter block (`---\n...\n---`) at the top is preserved verbatim in the injected prompt but is not parsed for `globs:` or `alwaysApply:` scoping today — keep the body self-contained. + +### Personal guidelines, across every project + +For preferences that follow *you*, not a specific repo (tone, tooling habits, workflow), drop a `ZERO.md` in your user config directory: `~/.config/hawk-eco/ZERO.md` on Linux/macOS, `%AppData%\Roaming\hawk-eco\ZERO.md` on Windows — the same directory as config files and your personal specialists. Same format and 8 KiB cap as the project files above, and the same case-insensitive basename match. + +This file is injected as its own `## User guidelines` section, before the project's `AGENTS.md`/`ZERO.md`, and is labeled as personal preference in the prompt: project guidelines are the later, more specific instruction and take precedence over it when the two conflict. + +## 2. Custom specialists + +Specialists are hawk-eco's sub-agents. Three scopes, in priority order: + +| Scope | Path | Shared? | +| --- | --- | --- | +| Built-in | compiled into hawk-eco | yes | +| User | `~/.config/hawk-eco/specialists/*.md` | no — your machine only | +| Project | `./.zero/specialists/*.md` | yes — the repo team | + +Project overrides user overrides built-in when names collide. + +A specialist is a markdown manifest with frontmatter and a system prompt: + +```markdown +--- +description: Reviews API changes for breaking-change risk and missing tests. +tools: read-only,plan +--- + +You review API changes. For every changed hunk in `internal/api/` or any file +that ends in `_api.go`: + +1. Confirm the public signature is backward-compatible, or note the breaking + change explicitly with the migration path. +2. Confirm a corresponding test exists in `internal/api/*_test.go` and that + the new behaviour is exercised. +3. Flag any new exported symbol without a doc comment. + +Reply with one JSON object per finding: `{"file", "line", "severity", "message", "fix"}`. +``` + +CLI management: ```bash -go test ./... # Run all tests -go test -race ./... # Race detector -go test -coverprofile=c.out ./... # Coverage -go vet ./... # Static analysis -gofumpt -w . # Format +hawk-eco specialist list +hawk-eco specialist show api-reviewer +hawk-eco specialist create api-reviewer \ + --project \ + --description "Reviews API changes" \ + --tools read-only,plan \ + --prompt "$(cat api-reviewer.md)" +hawk-eco specialist edit api-reviewer --project +hawk-eco specialist delete api-reviewer --project +hawk-eco specialist path # prints the resolved specialists directory ``` -## Architecture - -- `diff_parser.go` — Parses unified diffs into structured hunks -- `enricher.go` — Adds surrounding code context and git blame -- `reviewer.go` — Runs multi-concern parallel reviews -- `provider.go` — LLM provider interface (consumers implement this) -- `finding.go` — Review findings with severity and suggestions -- `taint_analysis.go` — Security taint tracking for vulnerability detection -- `internal/output/` — SARIF and other output formatters - -## Conventions - -- Go 1.26+, pure Go, no CGO -- Table-driven tests -- Conventional Commits: `feat:`, `fix:`, `docs:`, `refactor:`, `test:` -- No `Co-authored-by:` trailers (auto-stripped by githook) -- `gofumpt` formatting enforced in CI -- Import `hawk-core-contracts/types` for cross-repo types - -## Common Pitfalls - -- Do not add LLM client implementations — that's the consumer's job -- The Provider interface is the boundary; keep it minimal -- Taint analysis tests need careful setup — see existing test patterns - -## Naming Conventions - -- **Types are nouns, not abbreviations**: `Finding`, `InlineComment`, `Result`, `Stats` — not `Fnd` or `InlCmt` -- **Option functions use `With` prefix**: `WithProvider()`, `WithMaxTokens()`, `WithParallel()` — never bare `Provider()` -- **Preset options are bare vars**: `Quick`, `Thorough`, `SecurityFocus`, `CI` — exported `var Option` values -- **Internal types mirror public ones**: public `Finding` maps to internal `review.Finding` via `toPublicFindings()` -- **Severity is a type alias**: `type Severity = types.Severity` from `hawk-core-contracts/types` — never define your own -- **Error sentinel naming**: `ErrNoProvider`, `ErrEmptyDiff`, `ErrContextCancelled` — always `Err` prefix, package-scoped -- **Mock types in tests**: `mockProvider` (unexported), implements `Provider` with `response string` and `err error` fields - -## API Patterns - -- **Functional options pattern**: all configuration goes through `Option` interface with `optFunc` adapter: - ```go - type Option interface { apply(*config) } - type optFunc func(*config) - func (f optFunc) apply(c *config) { f(c) } - ``` -- **One-shot + reusable**: `Review(ctx, diff, opts...)` creates a `Reviewer` internally; `NewReviewer(opts...)` for reuse -- **Sentinel errors**: returned directly (e.g. `ErrNoProvider`), not wrapped — callers compare with `==` -- **Result methods**: `Failed()` checks severity threshold, `MaxSeverity()` returns highest finding severity -- **JSON tags on all public struct fields**: `json:"concern"`, `json:"severity"`, etc. — `omitempty` for optional fields -- **Provider interface is minimal**: single `Chat(ctx, messages, opts) (*Response, error)` method — no streaming, no tools -- **Concurrency**: `Reviewer` is safe for concurrent use; internal `sync.Mutex` protects shared state during parallel reviews - -## Testing Patterns - -- **External test package**: `package sight_test` — tests import `sight` as a consumer would -- **Mock provider**: `mockProvider` struct with `response string`, `err error`, `calls int64` (mutex-protected counter) -- **Mock findings helper**: `mockFindings()` returns a JSON string matching the expected LLM response format -- **Test diff constant**: `testDiff` is a `const` unified diff used across multiple tests -- **Error path tests**: `TestReview_NoProvider`, `TestReview_EmptyDiff`, `TestReview_ProviderError` — each error sentinel gets its own test -- **Dedup test**: verify that identical findings from multiple concerns are collapsed to one -- **No table-driven tests for Review**: the function has too many options; individual test functions per scenario are preferred -- **Assertions use `t.Fatalf` for setup failures, `t.Errorf` for assertion failures** — never `t.Fatal` after assertions - -## Refactoring Guidelines - -- **Safe to refactor**: `dedup()`, `filterFiles()`, `matchesExclude()`, `countHunks()` — pure functions, well-tested -- **Safe to refactor**: `toPublicFindings()`, `toPublicComments()` — mapping functions, add fields freely -- **Do not touch**: `Provider` interface signature — breaking change for all consumers (hawk, eyrie integration) -- **Do not touch**: `Finding`, `Result`, `Stats` struct field names/tags — used in JSON serialization by consumers -- **Do not touch**: `Severity` type alias — it re-exports from `hawk-core-contracts/types`; changing it breaks cross-repo compatibility -- **Safe to extend**: add new `Option` functions, new presets, new `StaticRule` entries, new taint source/sink patterns -- **When adding concerns**: add to `defaultConfig().concerns` list and create corresponding `review.Concern` in `internal/review/` - -## Key File Locations - -| What | Where | -|---|---| -| Public API entry point | `sight.go` (types, `Review()`, error sentinels) | -| Reviewer implementation | `reviewer.go` (orchestration, parallel concerns, reflection) | -| Configuration & presets | `options.go` (`config` struct, `With*` functions, presets) | -| LLM provider interface | `provider.go` (`Provider`, `Message`, `ChatOpts`, `Response`) | -| Severity type alias | `severity.go` (re-exports from `hawk-core-contracts/types`) | -| Static analysis rules | `static_rules.go` (`StaticRule`, `StaticAnalyzer`, 30+ rules) | -| Taint analysis | `taint_analysis.go` (`TaintAnalyzer`, source/sink/sanitizer patterns) | -| Diff parsing internals | `internal/diff/` | -| Review concern building | `internal/review/` (concerns, prompts, response parsing) | -| Inline comment mapping | `internal/comment/` | -| Output formatters | `internal/output/` (SARIF, terminal) | -| Git context enrichment | `internal/context/` | -| Main test file | `sight_test.go` (mock provider, test diff, core scenarios) | -| Taint analysis tests | `taint_analysis_test.go` | -| Static rules tests | `static_rules_test.go` | -| SARIF output tests | `sarif_test.go` | -| Linter config | `.golangci.yml` (govet, ineffassign, nilerr, misspell) | +## 3. Skills + +Skills are markdown instruction files that extend agent capabilities. They can be: +- Project-scoped: dropped in `./.zero/skills/` or `./skills/` +- User-scoped: dropped in `~/.config/hawk-eco/skills/` + +A skill manifest: + +```markdown +--- +description: How to review Go code for security issues +globs: "*.go" +alwaysApply: true +--- + +When reviewing Go code for security: + +1. Check for SQL injection patterns +2. Verify error handling doesn't expose sensitive data +3. Confirm secrets are not hardcoded +4. Validate input sanitization +``` + +## 4. Hooks + +Hooks allow custom commands to run at specific lifecycle points: +- `beforeReview` — runs before code review starts +- `afterReview` — runs after code review completes +- `sessionStart` — runs at session initialization +- `sessionEnd` — runs at session teardown + +```bash +hawk-eco hook add beforeReview --command "lint-check" +hawk-eco hook remove beforeReview +hawk-eco hook list +``` + +## 5. MCP integration + +MCP (Model Context Protocol) servers can expose tools to hawk-eco: + +```bash +hawk-eco mcp add --name server --url http://localhost:8080 +hawk-eco mcp remove server +hawk-eco mcp list +``` + +## 6. Plugins + +Plugins extend hawk-eco with custom tools and capabilities: + +```bash +hawk-eco plugin add --name my-plugin --path ./my-plugin +hawk-eco plugin remove my-plugin +hawk-eco plugin list +``` + +## 7. Verification + +hawk-eco includes a self-verification system to validate local changes before contributing: + +```bash +hawk-eco verify +hawk-eco verify --fix +``` + +## Development + +```bash +make lint +hawk-eco verify +``` diff --git a/Makefile b/Makefile index b2e4beb..95d19ff 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ # Canonical hawk-eco Makefile for Go library repos. -# Source of truth: .shared-templates/Makefile.library.tmpl at the eco root. +# Source of truth: https://github.com/GrayCodeAI/hawk/blob/main/.shared-templates/Makefile.library.tmpl # Placeholders rendered per repo: sight. # --------------------------------------------------------------------------- @@ -115,5 +115,7 @@ help: ## Show this help. @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-15s\033[0m %s\n", $$1, $$2}' .PHONY: hooks -hooks: - git config core.hooksPath .githooks +hooks: ## Install git hooks via lefthook (format, lint, conventional commits, co-author strip). + @command -v lefthook >/dev/null 2>&1 || (echo "install: go install github.com/evilmartians/lefthook@latest" && exit 1) + git config --unset core.hooksPath 2>/dev/null || true + lefthook install diff --git a/internal/diff/diff.go b/internal/diff/diff.go index c435594..e3f5a72 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -196,6 +196,16 @@ func parseRange(s string) (int, int) { if len(parts) == 2 { count, _ = strconv.Atoi(parts[1]) } + // A malformed header (e.g. a stray "+-1" range after the "+"/"-" prefix is + // already stripped) can parse as a negative integer even though Atoi itself + // didn't error. Clamp to 0, consistent with parseHunkHeader's documented + // "defaults to 0 on parse errors" contract for any other malformed input. + if start < 0 { + start = 0 + } + if count < 0 { + count = 0 + } return start, count } diff --git a/internal/diff/testdata/fuzz/FuzzParseHunkHeader/8efaeae90f5ff47c b/internal/diff/testdata/fuzz/FuzzParseHunkHeader/8efaeae90f5ff47c new file mode 100644 index 0000000..5b13f4b --- /dev/null +++ b/internal/diff/testdata/fuzz/FuzzParseHunkHeader/8efaeae90f5ff47c @@ -0,0 +1,2 @@ +go test fuzz v1 +string("@@+-1") diff --git a/lefthook.yml b/lefthook.yml index 93ead66..66773fa 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -1,5 +1,5 @@ # Canonical lefthook config for hawk-eco Go repos. -# Source of truth: .shared-templates/lefthook.yml.tmpl +# Source of truth: https://github.com/GrayCodeAI/hawk/blob/main/.shared-templates/lefthook.yml.tmpl # # Install lefthook: # brew install lefthook (macOS) @@ -75,6 +75,9 @@ pre-commit: pre-push: commands: + boundaries: + run: bash ./scripts/check-ecosystem-boundaries.sh + test: run: go test ./... -count=1 -timeout=60s diff --git a/mcp/server.go b/mcp/server.go index 73f87a1..f6696ad 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "time" mcpkit "github.com/GrayCodeAI/hawk-mcpkit" mcplib "github.com/mark3labs/mcp-go/mcp" @@ -82,8 +83,11 @@ func (s *Server) handleTaint(ctx context.Context, req mcplib.CallToolRequest) (* } } + ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) + defer cancel() + analyzer := sight.NewSSATaintAnalyzer() - findings, err := analyzer.AnalyzePackages(path, patterns...) + findings, err := analyzer.AnalyzePackagesContext(ctx, path, patterns...) if err != nil { return mcplib.NewToolResultError(fmt.Sprintf("taint analysis failed: %v", err)), nil } diff --git a/ssa_taint.go b/ssa_taint.go index 106adcd..8a5a85d 100644 --- a/ssa_taint.go +++ b/ssa_taint.go @@ -1,6 +1,7 @@ package sight import ( + "context" "fmt" "go/token" "go/types" @@ -37,11 +38,22 @@ func NewSSATaintAnalyzer() *SSATaintAnalyzer { // "./...", "./internal/handlers") rooted at dir, builds SSA, and reports // cross-function taint flows as Findings. A non-nil error is returned only for // load failures; per-package type errors are tolerated where possible. +// +// The load has no deadline; callers analyzing untrusted or unbounded input +// (e.g. an MCP tool argument) should use AnalyzePackagesContext instead. func (a *SSATaintAnalyzer) AnalyzePackages(dir string, patterns ...string) ([]Finding, error) { + return a.AnalyzePackagesContext(context.Background(), dir, patterns...) +} + +// AnalyzePackagesContext is AnalyzePackages with a caller-supplied context, +// so the package load (which shells out to `go list` and can run arbitrarily +// long against a large or pathological module) can be bounded or cancelled. +func (a *SSATaintAnalyzer) AnalyzePackagesContext(ctx context.Context, dir string, patterns ...string) ([]Finding, error) { if len(patterns) == 0 { patterns = []string{"./..."} } cfg := &packages.Config{ + Context: ctx, Mode: packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles | packages.NeedImports | packages.NeedDeps | packages.NeedTypes | packages.NeedSyntax | packages.NeedTypesInfo,