Skip to content

Commit 49953a6

Browse files
authored
Security and quality hardening pass (#20)
* chore: commit wip changes on feature/wip-20260705 * Security and quality hardening pass - Fix parseRange to clamp negative values on malformed diff hunk headers, found via FuzzParseHunkHeader; commit crash reproducer as regression corpus - Add fuzz CI job for the diff parser targets - MCP server and taint analysis hardening fixes from cross-repo consistency review - Update AGENTS.md to the shared hawk-eco extension-authoring format
1 parent db6a46a commit 49953a6

8 files changed

Lines changed: 219 additions & 110 deletions

File tree

.github/workflows/ci.yml

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Canonical CI workflow for hawk-eco Go repos.
2-
# Source of truth: .shared-templates/workflows/go-ci.yml.tmpl
2+
# Source of truth: https://github.com/GrayCodeAI/hawk/blob/main/.shared-templates/workflows/go-ci.yml.tmpl
33
#
44
# Two deployment models:
55
#
@@ -180,6 +180,26 @@ jobs:
180180
run: |
181181
npx jscpd --min-lines 5 --min-tokens 50 --reporters console --blame . 2>&1 | head -50
182182
183+
# -------------------------------------------------------------------------
184+
# Fuzz the diff parser, which consumes untrusted/adversarial unified-diff
185+
# text from arbitrary git repos being reviewed.
186+
# -------------------------------------------------------------------------
187+
fuzz:
188+
name: fuzz (60s)
189+
runs-on: ubuntu-latest
190+
needs: [test]
191+
steps:
192+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
193+
- uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0
194+
with:
195+
go-version: ${{ env.GO_VERSION }}
196+
cache: true
197+
- name: Run fuzz targets
198+
run: |
199+
go test -fuzz=FuzzParseDiff -fuzztime=60s ./internal/diff
200+
go test -fuzz=FuzzParseHunkHeader -fuzztime=60s ./internal/diff
201+
go test -fuzz=FuzzParseUnifiedDiff -fuzztime=60s ./internal/diff
202+
183203
# -------------------------------------------------------------------------
184204
# Cross-platform build matrix — only for repos that produce a binary.
185205
# Repos that are pure libraries can keep this job (it'll just `go build ./...`)

AGENTS.md

Lines changed: 160 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,112 +1,168 @@
1-
# AGENTS.md — Sight
1+
---
2+
description: Extending hawk-eco — how to write AGENTS.md files, custom specialists, skills, hooks, MCP servers, and plugins.
3+
globs: "*.go, *.js, *.md, *.json, *.toml, *.yaml, *.yml"
4+
alwaysApply: false
5+
---
26

3-
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.
7+
# Extending hawk-eco
48

5-
## Design Principles
9+
hawk-eco is an open-source code intelligence platform. This document describes how to extend it with custom tools, skills, hooks, and integrations.
610

7-
- **Library only** — no CLI, no binary
8-
- **No LLM SDK dependency** — defines a Provider interface; consumers implement it
9-
- **No opinions** — consumers inject their own LLM client (e.g., via eyrie)
11+
## 1. Drop a project `AGENTS.md`
1012

11-
## Build & Test
13+
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)`).
14+
15+
Accepted file names, in priority order at each level:
16+
17+
| Path | Notes |
18+
| --- | --- |
19+
| `./AGENTS.md` | The classic spot — committed to your repo, shared with the team. |
20+
| `./ZERO.md` | Brand-specific alias. Same format, lower priority. |
21+
| `./.zero/AGENTS.md` | Project-local, hidden, gitignored. Personal notes that stay out of git. |
22+
23+
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.
24+
25+
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.
26+
27+
```markdown
28+
# Project conventions for <your project>
29+
30+
- Build with `make`, not `go build` directly.
31+
- Tests live next to the source file (`foo_test.go` next to `foo.go`).
32+
- Run `make lint` before opening a PR.
33+
- Never edit files under `third_party/` — those are vendored.
34+
```
35+
36+
Tips:
37+
38+
- Keep each file under ~8 KiB. hawk-eco caps the **total** across all matched files at 32 KiB; everything past the cap is dropped.
39+
- Re-state rules in the imperative voice: "Run `make lint`", not "you should consider running the linter".
40+
- Don't put secrets, model IDs, or environment-specific paths in `AGENTS.md`. Use config files for those.
41+
- 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.
42+
- 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.
43+
44+
### Personal guidelines, across every project
45+
46+
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.
47+
48+
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.
49+
50+
## 2. Custom specialists
51+
52+
Specialists are hawk-eco's sub-agents. Three scopes, in priority order:
53+
54+
| Scope | Path | Shared? |
55+
| --- | --- | --- |
56+
| Built-in | compiled into hawk-eco | yes |
57+
| User | `~/.config/hawk-eco/specialists/*.md` | no — your machine only |
58+
| Project | `./.zero/specialists/*.md` | yes — the repo team |
59+
60+
Project overrides user overrides built-in when names collide.
61+
62+
A specialist is a markdown manifest with frontmatter and a system prompt:
63+
64+
```markdown
65+
---
66+
description: Reviews API changes for breaking-change risk and missing tests.
67+
tools: read-only,plan
68+
---
69+
70+
You review API changes. For every changed hunk in `internal/api/` or any file
71+
that ends in `_api.go`:
72+
73+
1. Confirm the public signature is backward-compatible, or note the breaking
74+
change explicitly with the migration path.
75+
2. Confirm a corresponding test exists in `internal/api/*_test.go` and that
76+
the new behaviour is exercised.
77+
3. Flag any new exported symbol without a doc comment.
78+
79+
Reply with one JSON object per finding: `{"file", "line", "severity", "message", "fix"}`.
80+
```
81+
82+
CLI management:
1283

1384
```bash
14-
go test ./... # Run all tests
15-
go test -race ./... # Race detector
16-
go test -coverprofile=c.out ./... # Coverage
17-
go vet ./... # Static analysis
18-
gofumpt -w . # Format
85+
hawk-eco specialist list
86+
hawk-eco specialist show api-reviewer
87+
hawk-eco specialist create api-reviewer \
88+
--project \
89+
--description "Reviews API changes" \
90+
--tools read-only,plan \
91+
--prompt "$(cat api-reviewer.md)"
92+
hawk-eco specialist edit api-reviewer --project
93+
hawk-eco specialist delete api-reviewer --project
94+
hawk-eco specialist path # prints the resolved specialists directory
1995
```
2096

21-
## Architecture
22-
23-
- `diff_parser.go` — Parses unified diffs into structured hunks
24-
- `enricher.go` — Adds surrounding code context and git blame
25-
- `reviewer.go` — Runs multi-concern parallel reviews
26-
- `provider.go` — LLM provider interface (consumers implement this)
27-
- `finding.go` — Review findings with severity and suggestions
28-
- `taint_analysis.go` — Security taint tracking for vulnerability detection
29-
- `internal/output/` — SARIF and other output formatters
30-
31-
## Conventions
32-
33-
- Go 1.26+, pure Go, no CGO
34-
- Table-driven tests
35-
- Conventional Commits: `feat:`, `fix:`, `docs:`, `refactor:`, `test:`
36-
- No `Co-authored-by:` trailers (auto-stripped by githook)
37-
- `gofumpt` formatting enforced in CI
38-
- Import `hawk-core-contracts/types` for cross-repo types
39-
40-
## Common Pitfalls
41-
42-
- Do not add LLM client implementations — that's the consumer's job
43-
- The Provider interface is the boundary; keep it minimal
44-
- Taint analysis tests need careful setup — see existing test patterns
45-
46-
## Naming Conventions
47-
48-
- **Types are nouns, not abbreviations**: `Finding`, `InlineComment`, `Result`, `Stats` — not `Fnd` or `InlCmt`
49-
- **Option functions use `With` prefix**: `WithProvider()`, `WithMaxTokens()`, `WithParallel()` — never bare `Provider()`
50-
- **Preset options are bare vars**: `Quick`, `Thorough`, `SecurityFocus`, `CI` — exported `var Option` values
51-
- **Internal types mirror public ones**: public `Finding` maps to internal `review.Finding` via `toPublicFindings()`
52-
- **Severity is a type alias**: `type Severity = types.Severity` from `hawk-core-contracts/types` — never define your own
53-
- **Error sentinel naming**: `ErrNoProvider`, `ErrEmptyDiff`, `ErrContextCancelled` — always `Err` prefix, package-scoped
54-
- **Mock types in tests**: `mockProvider` (unexported), implements `Provider` with `response string` and `err error` fields
55-
56-
## API Patterns
57-
58-
- **Functional options pattern**: all configuration goes through `Option` interface with `optFunc` adapter:
59-
```go
60-
type Option interface { apply(*config) }
61-
type optFunc func(*config)
62-
func (f optFunc) apply(c *config) { f(c) }
63-
```
64-
- **One-shot + reusable**: `Review(ctx, diff, opts...)` creates a `Reviewer` internally; `NewReviewer(opts...)` for reuse
65-
- **Sentinel errors**: returned directly (e.g. `ErrNoProvider`), not wrapped — callers compare with `==`
66-
- **Result methods**: `Failed()` checks severity threshold, `MaxSeverity()` returns highest finding severity
67-
- **JSON tags on all public struct fields**: `json:"concern"`, `json:"severity"`, etc. — `omitempty` for optional fields
68-
- **Provider interface is minimal**: single `Chat(ctx, messages, opts) (*Response, error)` method — no streaming, no tools
69-
- **Concurrency**: `Reviewer` is safe for concurrent use; internal `sync.Mutex` protects shared state during parallel reviews
70-
71-
## Testing Patterns
72-
73-
- **External test package**: `package sight_test` — tests import `sight` as a consumer would
74-
- **Mock provider**: `mockProvider` struct with `response string`, `err error`, `calls int64` (mutex-protected counter)
75-
- **Mock findings helper**: `mockFindings()` returns a JSON string matching the expected LLM response format
76-
- **Test diff constant**: `testDiff` is a `const` unified diff used across multiple tests
77-
- **Error path tests**: `TestReview_NoProvider`, `TestReview_EmptyDiff`, `TestReview_ProviderError` — each error sentinel gets its own test
78-
- **Dedup test**: verify that identical findings from multiple concerns are collapsed to one
79-
- **No table-driven tests for Review**: the function has too many options; individual test functions per scenario are preferred
80-
- **Assertions use `t.Fatalf` for setup failures, `t.Errorf` for assertion failures** — never `t.Fatal` after assertions
81-
82-
## Refactoring Guidelines
83-
84-
- **Safe to refactor**: `dedup()`, `filterFiles()`, `matchesExclude()`, `countHunks()` — pure functions, well-tested
85-
- **Safe to refactor**: `toPublicFindings()`, `toPublicComments()` — mapping functions, add fields freely
86-
- **Do not touch**: `Provider` interface signature — breaking change for all consumers (hawk, eyrie integration)
87-
- **Do not touch**: `Finding`, `Result`, `Stats` struct field names/tags — used in JSON serialization by consumers
88-
- **Do not touch**: `Severity` type alias — it re-exports from `hawk-core-contracts/types`; changing it breaks cross-repo compatibility
89-
- **Safe to extend**: add new `Option` functions, new presets, new `StaticRule` entries, new taint source/sink patterns
90-
- **When adding concerns**: add to `defaultConfig().concerns` list and create corresponding `review.Concern` in `internal/review/`
91-
92-
## Key File Locations
93-
94-
| What | Where |
95-
|---|---|
96-
| Public API entry point | `sight.go` (types, `Review()`, error sentinels) |
97-
| Reviewer implementation | `reviewer.go` (orchestration, parallel concerns, reflection) |
98-
| Configuration & presets | `options.go` (`config` struct, `With*` functions, presets) |
99-
| LLM provider interface | `provider.go` (`Provider`, `Message`, `ChatOpts`, `Response`) |
100-
| Severity type alias | `severity.go` (re-exports from `hawk-core-contracts/types`) |
101-
| Static analysis rules | `static_rules.go` (`StaticRule`, `StaticAnalyzer`, 30+ rules) |
102-
| Taint analysis | `taint_analysis.go` (`TaintAnalyzer`, source/sink/sanitizer patterns) |
103-
| Diff parsing internals | `internal/diff/` |
104-
| Review concern building | `internal/review/` (concerns, prompts, response parsing) |
105-
| Inline comment mapping | `internal/comment/` |
106-
| Output formatters | `internal/output/` (SARIF, terminal) |
107-
| Git context enrichment | `internal/context/` |
108-
| Main test file | `sight_test.go` (mock provider, test diff, core scenarios) |
109-
| Taint analysis tests | `taint_analysis_test.go` |
110-
| Static rules tests | `static_rules_test.go` |
111-
| SARIF output tests | `sarif_test.go` |
112-
| Linter config | `.golangci.yml` (govet, ineffassign, nilerr, misspell) |
97+
## 3. Skills
98+
99+
Skills are markdown instruction files that extend agent capabilities. They can be:
100+
- Project-scoped: dropped in `./.zero/skills/` or `./skills/`
101+
- User-scoped: dropped in `~/.config/hawk-eco/skills/`
102+
103+
A skill manifest:
104+
105+
```markdown
106+
---
107+
description: How to review Go code for security issues
108+
globs: "*.go"
109+
alwaysApply: true
110+
---
111+
112+
When reviewing Go code for security:
113+
114+
1. Check for SQL injection patterns
115+
2. Verify error handling doesn't expose sensitive data
116+
3. Confirm secrets are not hardcoded
117+
4. Validate input sanitization
118+
```
119+
120+
## 4. Hooks
121+
122+
Hooks allow custom commands to run at specific lifecycle points:
123+
- `beforeReview` — runs before code review starts
124+
- `afterReview` — runs after code review completes
125+
- `sessionStart` — runs at session initialization
126+
- `sessionEnd` — runs at session teardown
127+
128+
```bash
129+
hawk-eco hook add beforeReview --command "lint-check"
130+
hawk-eco hook remove beforeReview
131+
hawk-eco hook list
132+
```
133+
134+
## 5. MCP integration
135+
136+
MCP (Model Context Protocol) servers can expose tools to hawk-eco:
137+
138+
```bash
139+
hawk-eco mcp add --name server --url http://localhost:8080
140+
hawk-eco mcp remove server
141+
hawk-eco mcp list
142+
```
143+
144+
## 6. Plugins
145+
146+
Plugins extend hawk-eco with custom tools and capabilities:
147+
148+
```bash
149+
hawk-eco plugin add --name my-plugin --path ./my-plugin
150+
hawk-eco plugin remove my-plugin
151+
hawk-eco plugin list
152+
```
153+
154+
## 7. Verification
155+
156+
hawk-eco includes a self-verification system to validate local changes before contributing:
157+
158+
```bash
159+
hawk-eco verify
160+
hawk-eco verify --fix
161+
```
162+
163+
## Development
164+
165+
```bash
166+
make lint
167+
hawk-eco verify
168+
```

Makefile

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Canonical hawk-eco Makefile for Go library repos.
2-
# Source of truth: .shared-templates/Makefile.library.tmpl at the eco root.
2+
# Source of truth: https://github.com/GrayCodeAI/hawk/blob/main/.shared-templates/Makefile.library.tmpl
33
# Placeholders rendered per repo: sight.
44

55
# ---------------------------------------------------------------------------
@@ -115,5 +115,7 @@ help: ## Show this help.
115115
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-15s\033[0m %s\n", $$1, $$2}'
116116

117117
.PHONY: hooks
118-
hooks:
119-
git config core.hooksPath .githooks
118+
hooks: ## Install git hooks via lefthook (format, lint, conventional commits, co-author strip).
119+
@command -v lefthook >/dev/null 2>&1 || (echo "install: go install github.com/evilmartians/lefthook@latest" && exit 1)
120+
git config --unset core.hooksPath 2>/dev/null || true
121+
lefthook install

internal/diff/diff.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,16 @@ func parseRange(s string) (int, int) {
196196
if len(parts) == 2 {
197197
count, _ = strconv.Atoi(parts[1])
198198
}
199+
// A malformed header (e.g. a stray "+-1" range after the "+"/"-" prefix is
200+
// already stripped) can parse as a negative integer even though Atoi itself
201+
// didn't error. Clamp to 0, consistent with parseHunkHeader's documented
202+
// "defaults to 0 on parse errors" contract for any other malformed input.
203+
if start < 0 {
204+
start = 0
205+
}
206+
if count < 0 {
207+
count = 0
208+
}
199209
return start, count
200210
}
201211

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
go test fuzz v1
2+
string("@@+-1")

lefthook.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Canonical lefthook config for hawk-eco Go repos.
2-
# Source of truth: .shared-templates/lefthook.yml.tmpl
2+
# Source of truth: https://github.com/GrayCodeAI/hawk/blob/main/.shared-templates/lefthook.yml.tmpl
33
#
44
# Install lefthook:
55
# brew install lefthook (macOS)
@@ -75,6 +75,9 @@ pre-commit:
7575
pre-push:
7676
commands:
7777

78+
boundaries:
79+
run: bash ./scripts/check-ecosystem-boundaries.sh
80+
7881
test:
7982
run: go test ./... -count=1 -timeout=60s
8083

mcp/server.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"strings"
7+
"time"
78

89
mcpkit "github.com/GrayCodeAI/hawk-mcpkit"
910
mcplib "github.com/mark3labs/mcp-go/mcp"
@@ -82,8 +83,11 @@ func (s *Server) handleTaint(ctx context.Context, req mcplib.CallToolRequest) (*
8283
}
8384
}
8485

86+
ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
87+
defer cancel()
88+
8589
analyzer := sight.NewSSATaintAnalyzer()
86-
findings, err := analyzer.AnalyzePackages(path, patterns...)
90+
findings, err := analyzer.AnalyzePackagesContext(ctx, path, patterns...)
8791
if err != nil {
8892
return mcplib.NewToolResultError(fmt.Sprintf("taint analysis failed: %v", err)), nil
8993
}

0 commit comments

Comments
 (0)