From f015ae533a2807c3d78edb5b494b5842c98715e5 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Mon, 13 Jul 2026 20:17:23 +0530 Subject: [PATCH 1/2] feat(ci): add drift check for pinned dependencies Add a new drift check that compares hawk's own go.mod requirements for tracked pins against what each external/ submodule declares. This helps detect version mismatches that could cause silent build-time upgrades. Related: docs/compatibility.md explains the drift concern in detail. --- .github/workflows/compatibility-matrix.yml | 7 ++ Makefile | 5 +- cmd/compat-test/drift.go | 79 ++++++++++++++++++++++ cmd/compat-test/main.go | 15 ++++ docs/compatibility.md | 25 +++++++ go.mod | 2 +- 6 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 cmd/compat-test/drift.go diff --git a/.github/workflows/compatibility-matrix.yml b/.github/workflows/compatibility-matrix.yml index 4c8ab939..da4d596a 100644 --- a/.github/workflows/compatibility-matrix.yml +++ b/.github/workflows/compatibility-matrix.yml @@ -23,6 +23,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: ./.github/actions/checkout-eyrie + with: + ref: ${{ github.head_ref || github.ref_name }} + allow_branch_fallback: "true" - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: "1.26.5" @@ -31,3 +35,6 @@ jobs: run: make compat-check - name: Report 'next' matrix run: make compat-test + - name: Pin freshness vs external/ (advisory) + run: make compat-drift + continue-on-error: true diff --git a/Makefile b/Makefile index 6c277a7d..147f33bf 100644 --- a/Makefile +++ b/Makefile @@ -235,7 +235,7 @@ help: ## Show this help. # Validates compatibility-matrix.json and reports the resolved versions for # a chosen matrix entry. Wired into the compatibility-test workflow. # --------------------------------------------------------------------------- -.PHONY: compat-test compat-check +.PHONY: compat-test compat-check compat-drift compat-test: ## Validate testdata/compatibility-matrix.json and report the 'next' matrix. @go run ./cmd/compat-test -matrix=next -file=testdata/compatibility-matrix.json @@ -243,6 +243,9 @@ compat-test: ## Validate testdata/compatibility-matrix.json and report the 'next compat-check: ## Strict validation — non-zero exit if any component lacks a version. @go run ./cmd/compat-test -matrix=next -strict -file=testdata/compatibility-matrix.json +compat-drift: ## Advisory: report pin drift between hawk's go.mod and external/ submodules. Never fails. + @go run ./cmd/compat-test -check-external -file=testdata/compatibility-matrix.json + .PHONY: hooks sync-submodules sync-clone hooks: ## Install git hooks via lefthook (formatting, linting, conventional commits). @command -v lefthook >/dev/null 2>&1 || (echo "install: go install github.com/evilmartians/lefthook@latest" && exit 1) diff --git a/cmd/compat-test/drift.go b/cmd/compat-test/drift.go new file mode 100644 index 00000000..d080c87e --- /dev/null +++ b/cmd/compat-test/drift.go @@ -0,0 +1,79 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + + "golang.org/x/mod/modfile" +) + +// trackedPins are the shared leaf dependencies most likely to drift silently: +// a consumer (inspect, sight, ...) can pin an older version than what hawk's +// own go.mod requires, and Go's minimal version selection will silently pull +// in hawk's newer version at build time without the consumer's own CI ever +// having tested it. See docs/compatibility.md. +var trackedPins = []string{ + "github.com/GrayCodeAI/hawk-core-contracts", + "github.com/GrayCodeAI/hawk-mcpkit", +} + +// checkDrift compares hawk's own go.mod requirements for trackedPins against +// what each external/ submodule (a locally pinned clone of a hawk dependency) +// declares for the same modules in its own go.mod. It never fails — this is +// advisory, printed for humans/CI logs to notice, not a build gate. +func checkDrift(repoRoot string) error { + hawkRequires, err := readRequires(filepath.Join(repoRoot, "go.mod")) + if err != nil { + return fmt.Errorf("read hawk go.mod: %w", err) + } + + externalDir := filepath.Join(repoRoot, "external") + entries, err := os.ReadDir(externalDir) + if err != nil { + return fmt.Errorf("read external/: %w", err) + } + + fmt.Println("Pin freshness (advisory — see docs/compatibility.md):") + drifted := 0 + for _, e := range entries { + if !e.IsDir() { + continue + } + modPath := filepath.Join(externalDir, e.Name(), "go.mod") + consumerRequires, err := readRequires(modPath) + if err != nil { + continue // submodule not checked out / no go.mod — skip silently + } + for _, pin := range trackedPins { + hawkVer, hawkHas := hawkRequires[pin] + consumerVer, consumerHas := consumerRequires[pin] + if !hawkHas || !consumerHas || hawkVer == consumerVer { + continue + } + drifted++ + fmt.Printf(" %-22s requires %s@%s, hawk requires %s@%s\n", + e.Name(), pin, consumerVer, pin, hawkVer) + } + } + if drifted == 0 { + fmt.Println(" OK — no drift between hawk's pins and external/ consumers") + } + return nil +} + +func readRequires(path string) (map[string]string, error) { + raw, err := os.ReadFile(path) + if err != nil { + return nil, err + } + mf, err := modfile.Parse(path, raw, nil) + if err != nil { + return nil, err + } + out := make(map[string]string, len(mf.Require)) + for _, r := range mf.Require { + out[r.Mod.Path] = r.Mod.Version + } + return out, nil +} diff --git a/cmd/compat-test/main.go b/cmd/compat-test/main.go index 5a051266..f352e26a 100644 --- a/cmd/compat-test/main.go +++ b/cmd/compat-test/main.go @@ -17,6 +17,11 @@ // go run ./cmd/compat-test -matrix=stable -strict // # exit non-zero if any // # component lacks a version +// go run ./cmd/compat-test -check-external # advisory: compare hawk's own +// # go.mod pins for shared leaf +// # deps against what each +// # external/ submodule declares. +// # Always exits 0; see drift.go. package main import ( @@ -47,12 +52,22 @@ func main() { matrixName := flag.String("matrix", "next", "matrix entry to inspect (next, stable, ...)") strict := flag.Bool("strict", false, "exit non-zero if any component lacks a pinned version") path := flag.String("file", findMatrixFile(), "path to compatibility-matrix.json") + checkExternal := flag.Bool("check-external", false, "advisory: report pin drift against external/ submodules and exit (see drift.go)") flag.Parse() if *path == "" { die("compatibility-matrix.json not found in current dir or repo root") } + if *checkExternal { + repoRoot := filepath.Dir(filepath.Dir(*path)) // testdata/compatibility-matrix.json -> repo root + if err := checkDrift(repoRoot); err != nil { + // Advisory tool: report the problem but still exit 0. + fmt.Fprintf(os.Stderr, "compat-test: check-external: %v\n", err) + } + return + } + raw, err := os.ReadFile(*path) if err != nil { die("read %s: %v", *path, err) diff --git a/docs/compatibility.md b/docs/compatibility.md index 30a44675..b4ef930a 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -68,6 +68,31 @@ It runs on: - **Drift is visible.** A component that's never updated in `next` but is fine in `stable` shows up as a drift candidate in CI reports. +## Pin freshness (advisory) + +Separate from the matrix above: `hawk`'s own `go.mod` directly pins a couple +of shared leaf dependencies (currently `hawk-core-contracts`), and several +`external/` submodule consumers (`inspect`, `sight`, ...) pin the *same* +dependencies independently in their own `go.mod`. Go's minimal version +selection means whatever `hawk` pins wins in `hawk`'s own build — but if a +consumer's own pin is older, that consumer's CI has never actually tested the +version that ships. This is exactly the kind of drift that let a real bug +through in July 2026 (a stale goreleaser pin sat unnoticed until the first +tag push exercised it). + +`make compat-drift` (wired into this workflow as an advisory, non-blocking +step) reports any such mismatch by comparing `hawk/go.mod` against each +`external//go.mod`: + +```bash +make compat-drift +``` + +This **never fails the build** — it's a signal for humans to notice and +re-pin the consumer, not a gate. It depends on `external/` submodules being +checked out (the workflow does this via `checkout-eyrie`); running it without +that will just report nothing to check. + ## Validating the file The file is validated against [`testdata/compatibility-matrix.schema.json`](./testdata/compatibility-matrix.schema.json) diff --git a/go.mod b/go.mod index 629a190b..a6b1cc2a 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( go.opentelemetry.io/otel/sdk v1.44.0 go.opentelemetry.io/otel/sdk/metric v1.44.0 go.opentelemetry.io/otel/trace v1.44.0 + golang.org/x/mod v0.37.0 golang.org/x/sys v0.46.0 golang.org/x/term v0.44.0 golang.org/x/text v0.38.0 @@ -121,7 +122,6 @@ require ( github.com/zalando/go-keyring v0.2.8 // indirect go4.org v0.0.0-20260112195520-a5071408f32f // indirect golang.org/x/crypto v0.53.0 // indirect - golang.org/x/mod v0.37.0 // indirect golang.org/x/sync v0.21.0 // indirect golang.org/x/time v0.15.0 // indirect ) From 838226f33c028482d187738043522034a5f14be3 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Tue, 14 Jul 2026 12:25:23 +0530 Subject: [PATCH 2/2] fix(tool): validate paths in patch/structured-edit/smart-create tools PatchTool, StructuredEditTool, and SmartCreateTool wrote/deleted files from LLM-authored paths without calling validatePathAllowed, the sandbox guard already used by file_write.go and file_edit.go. This let an LLM escape the working directory / allowed-directories sandbox via these three tools even though the equivalent Write/Edit tools were guarded. Thread ctx through PatchParser.ApplyAll so each patch's path (including the Delete File directive, which shares the same Apply function) is validated before any filesystem mutation. Add the same guard check to StructuredEditTool.Execute and SmartCreateTool.Execute before their ReadFile/WriteFile/MkdirAll calls. Add a regression test asserting PatchTool.Execute rejects a Create File patch targeting a path outside the allowed directories and does not create the file. Co-Authored-By: Claude Sonnet 5 --- internal/tool/patch.go | 7 +++++-- internal/tool/patch_test.go | 30 +++++++++++++++++++++++++++++- internal/tool/smart_create.go | 3 +++ internal/tool/structured_edit.go | 3 +++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/internal/tool/patch.go b/internal/tool/patch.go index ca4466ee..08f7a4b1 100644 --- a/internal/tool/patch.go +++ b/internal/tool/patch.go @@ -243,9 +243,12 @@ func Apply(patch *FilePatch) error { } // ApplyAll applies all patches and returns the list of modified file paths. -func (p *PatchParser) ApplyAll() ([]string, error) { +func (p *PatchParser) ApplyAll(ctx context.Context) ([]string, error) { var modified []string for i := range p.patches { + if err := validatePathAllowed(ctx, p.patches[i].Path); err != nil { + return modified, err + } if err := Apply(&p.patches[i]); err != nil { return modified, fmt.Errorf("failed to apply patch for %s: %w", p.patches[i].Path, err) } @@ -440,7 +443,7 @@ func (PatchTool) Execute(ctx context.Context, input json.RawMessage) (string, er return "", fmt.Errorf("failed to parse patch: %w", err) } - modified, err := parser.ApplyAll() + modified, err := parser.ApplyAll(ctx) if err != nil { return "", err } diff --git a/internal/tool/patch_test.go b/internal/tool/patch_test.go index 3007e52d..7237816f 100644 --- a/internal/tool/patch_test.go +++ b/internal/tool/patch_test.go @@ -473,7 +473,7 @@ func TestApplyAll(t *testing.T) { t.Fatalf("parse error: %v", err) } - modified, err := parser.ApplyAll() + modified, err := parser.ApplyAll(context.Background()) if err != nil { t.Fatalf("ApplyAll error: %v", err) } @@ -567,6 +567,34 @@ func main() { } } +func TestPatchTool_Execute_BlocksPathOutsideAllowedDirectories(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + outsidePath := filepath.Join(outside, "escape.go") + + orig, _ := os.Getwd() + if err := os.Chdir(root); err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(orig) }() + + patchContent := "*** Begin Patch\n" + + "*** Create File: " + outsidePath + "\n" + + "+ package main\n" + + "*** End Patch" + + input, _ := json.Marshal(map[string]string{"patch": patchContent}) + + guarded := WithToolContext(context.Background(), &ToolContext{}) + tool := PatchTool{} + if _, err := tool.Execute(guarded, input); err == nil { + t.Fatal("expected error for path outside allowed directories") + } + if _, statErr := os.Stat(outsidePath); !os.IsNotExist(statErr) { + t.Fatal("expected file outside allowed directories to not be created") + } +} + func TestPatchTool_Interface(t *testing.T) { var _ Tool = PatchTool{} diff --git a/internal/tool/smart_create.go b/internal/tool/smart_create.go index 2fb25fdc..01d23c35 100644 --- a/internal/tool/smart_create.go +++ b/internal/tool/smart_create.go @@ -680,6 +680,9 @@ func (t *SmartCreateTool) Execute(ctx context.Context, input json.RawMessage) (s if params.Path == "" { return "", fmt.Errorf("path is required") } + if err := validatePathAllowed(ctx, params.Path); err != nil { + return "", err + } // Generate boilerplate. content := t.Creator.GenerateBoilerplate(params.Path) diff --git a/internal/tool/structured_edit.go b/internal/tool/structured_edit.go index 0ebbff18..7f2d40c6 100644 --- a/internal/tool/structured_edit.go +++ b/internal/tool/structured_edit.go @@ -79,6 +79,9 @@ func (s StructuredEditTool) Execute(ctx context.Context, input json.RawMessage) if len(p.Blocks) == 0 { return "", fmt.Errorf("at least one SEARCH/REPLACE block is required") } + if err := validatePathAllowed(ctx, p.Path); err != nil { + return "", err + } // Read the file. data, err := os.ReadFile(p.Path)