From 96b7e6da9e332a873e31af4bc1b460c47bee3858 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Fri, 11 Sep 2026 13:08:15 +0530 Subject: [PATCH 1/2] refactor!: remove legacy migrations and shared/types guards - Remove the `hawk credentials migrate` subcommand, its man page entry, config/gateway `MigrateEnvFileCredentials` wrappers, and the "plaintext env" developer-path check with its helper and test. - Remove the one-time model/provider settings migration into Eyrie (`migrateStoredModelProvider`, `SetActiveSelection`) and its call in `LoadSettings`; keep the `Settings.Model`/`Settings.Provider` fields. - Remove the startup `MigrateProviderSecrets` pass: both cmd/root.go call sites, `logMigrateProviderSecretsError` and its tests, `MigrateEngineProviderSecrets`, and the method on the gateway `CatalogMaintenance` interface, implementation, and test stub. The provider.json security check stays with a manual-fix hint. - Remove the obsolete `shared/types` guards: the import-check script, the `contracts-guard` make target, lefthook and CI steps, and the related testaudit tests; drop `shared/types` from the ecosystem boundary pattern, its template, and the architecture docs. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 2 - .../check-ecosystem-boundaries.sh.tmpl | 4 +- AGENTS.md | 2 +- CHANGELOG.md | 6 ++ Makefile | 9 +-- README.md | 3 +- cmd/credentials.go | 23 ------- cmd/manpage.go | 1 - cmd/migrate_secrets_test.go | 65 ------------------- cmd/root.go | 33 ---------- docs/COMPETITIVE.md | 2 +- docs/DEVELOPER-PATH.md | 3 +- docs/SECURITY-DEVELOPER.md | 23 +++---- docs/architecture.md | 5 +- .../ecosystem-implementation-plan.md | 1 - ...hawk-architecture-v1-definition-of-done.md | 5 +- .../architecture/hawk-product-architecture.md | 9 +-- internal/config/config_test.go | 38 +---------- internal/config/developer_path.go | 29 +-------- internal/config/developer_path_test.go | 8 --- internal/config/eyrie_engine.go | 12 ---- internal/config/eyrie_selection.go | 60 ----------------- internal/config/migrate_provider_secrets.go | 6 -- internal/config/milestone_verify_test.go | 38 ----------- internal/config/settings.go | 5 +- internal/provider/gateway/engine_client.go | 5 -- internal/provider/gateway/gateway.go | 4 -- .../provider/gateway/provider_stub_test.go | 2 - internal/testaudit/audit_test.go | 25 ------- internal/testaudit/docs_audit_test.go | 25 ------- internal/testaudit/package_boundaries_test.go | 2 +- lefthook.yml | 3 - scripts/check-ecosystem-boundaries.sh | 4 +- scripts/check-shared-types-imports.sh | 21 ------ 34 files changed, 35 insertions(+), 448 deletions(-) delete mode 100644 cmd/migrate_secrets_test.go delete mode 100644 internal/config/migrate_provider_secrets.go delete mode 100644 scripts/check-shared-types-imports.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8f09b1f9..e84e350a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,8 +71,6 @@ jobs: echo "$out" | head -20 exit 1 fi - - name: shared types import guard - run: bash ./scripts/check-shared-types-imports.sh - name: ecosystem boundary guard run: bash ./scripts/check-ecosystem-boundaries.sh - name: internal layer boundary guard diff --git a/.shared-templates/scripts/check-ecosystem-boundaries.sh.tmpl b/.shared-templates/scripts/check-ecosystem-boundaries.sh.tmpl index 1d5211bf9..0fe3c5f63 100644 --- a/.shared-templates/scripts/check-ecosystem-boundaries.sh.tmpl +++ b/.shared-templates/scripts/check-ecosystem-boundaries.sh.tmpl @@ -16,7 +16,7 @@ cd "$ROOT_DIR" # Engines are peers: they may depend on falcon, # but never on hawk/internal/* or another engine. # ============================================================================= -FORBIDDEN_HAWK='github\.com/GrayCodeAI/hawk/(internal/|shared/types)' +FORBIDDEN_HAWK='github\.com/GrayCodeAI/hawk/internal/' FORBIDDEN_ENGINES='github\.com/GrayCodeAI/(harrier|shrike|swift|kestrel|merlin)(/|")' # ^ list every OTHER engine here — never include yourself. @@ -34,7 +34,7 @@ if [[ -n "${violations}" ]]; then echo "forbidden Hawk imports found:" echo "${violations}" echo - echo "support repos must use local contracts, not hawk/internal or removed hawk/shared/types" + echo "support repos must use local contracts, not hawk/internal" exit_code=1 fi diff --git a/AGENTS.md b/AGENTS.md index f29b339ca..1ba0998d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -187,7 +187,7 @@ hawk verify ### Architecture note: cross-repo contracts -Legacy `hawk/shared/types` has been removed. Cross-repo severity and finding contracts now live in hawk's `internal/contracts` (vendored from the removed `github.com/GrayCodeAI/eagle` module) — extensions and support repos must vendor the needed DTOs instead of Hawk internals until a published contracts module exists. +Cross-repo severity and finding contracts now live in hawk's `internal/contracts` (vendored from the removed `github.com/GrayCodeAI/eagle` module) — extensions and support repos must vendor the needed DTOs instead of Hawk internals until a published contracts module exists. ### Architecture note: provider ownership diff --git a/CHANGELOG.md b/CHANGELOG.md index b3ec7ef8b..b4b84d3c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Makefile lint pin matches CI**: `make lint`/`lint-fix`/`setup` install `golangci-lint@v2.1.0` (was `@latest`), the same version CI enforces. - **Docs truth and housekeeping**: SECURITY.md/CONTRIBUTING.md now describe the actual Go toolchain (golangci-lint, go vet, govulncheck) instead of the polyglot template's ruff/mypy/pip-audit/pnpm-lock language, CONTRIBUTING documents `make setup`/`boundaries`/`test-10x`/`smoke`, and the planning docs (`SPEC_DRIVEN_PLAN.md`, `SPEC_DRIVEN_PHASE2_PLAN.md`, `internal/engine/REFACTOR_PLAN.md`) moved to `docs/plans/`. +### Removed +- **BREAKING — `hawk credentials migrate` removed**: the subcommand, its man page entry, and the `MigrateEnvFileCredentials` wrappers are gone, and `hawk path` no longer reports legacy `~/.hawk/env` / `~/.hawk/.env` files. Save keys through `/config`. +- **BREAKING — settings model/provider migration removed**: `LoadSettings` no longer moves `model`/`provider` from `settings.json` into Eyrie's `provider.json`, and `SetActiveSelection` is deleted. Stale `model`/`provider` values left in `settings.json` are no longer cleared; select the model in `/config`. +- **BREAKING — startup provider-secrets migration removed**: print/REPL/watch/TUI startup no longer calls `MigrateProviderSecrets`, and the method is dropped from the gateway `CatalogMaintenance` interface. `hawk path` still fails when `provider.json` holds secrets; remove those fields manually. +- **BREAKING — `shared/types` guards removed**: `scripts/check-shared-types-imports.sh`, the `contracts-guard` make target, its lefthook and CI steps, and the matching testaudit checks are deleted because the package no longer exists. The ecosystem boundary guards still block `hawk/internal` imports. + ## [0.2.0] — 2026-07-13 ### Changed diff --git a/Makefile b/Makefile index 68e298d19..b3426bd27 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,7 @@ GORELEASER := $(GOBIN_DIR)/goreleaser # --------------------------------------------------------------------------- # Phony declarations (alphabetical). # --------------------------------------------------------------------------- -.PHONY: all bench boundaries build check-replace ci clean contracts-guard ecosystem-guard eyrie-client-guard eyrie-engine-guard manifest-guard peer-guard internal-layers-guard package-boundaries-guard release-parity cover cover-new fmt help install lint lint-fix \ +.PHONY: all bench boundaries build check-replace ci clean ecosystem-guard eyrie-client-guard eyrie-engine-guard manifest-guard peer-guard internal-layers-guard package-boundaries-guard release-parity cover cover-new fmt help install lint lint-fix \ release security setup smoke path sync test test-10x test-live test-new test-race tidy version vet api-docs api-validate workspace check-replace: ## Fail if go.mod has local replace directives (run before tagging) @@ -112,10 +112,7 @@ fmt: ## Format source files (gofumpt + goimports). vet: ## Run go vet. go vet ./... -contracts-guard: ## Fail on any legacy imports of removed hawk/shared/types. - bash ./scripts/check-shared-types-imports.sh - -ecosystem-guard: ## Fail if external ecosystem repos import hawk/internal or removed hawk/shared/types. +ecosystem-guard: ## Fail if external ecosystem repos import hawk/internal. bash ./scripts/check-ecosystem-boundaries.sh eyrie-client-guard: ## Fail on any production eyrie/client import. @@ -133,7 +130,7 @@ internal-layers-guard: ## Enforce one-way dependencies across stable Hawk intern package-boundaries-guard: ## Enforce AST/package-graph boundaries with file/line diagnostics. bash ./scripts/check-package-boundaries.sh -boundaries: manifest-guard check-replace contracts-guard ecosystem-guard eyrie-client-guard eyrie-engine-guard peer-guard internal-layers-guard package-boundaries-guard ## Alias for all boundary guards (matches `make boundaries` in engine repos). +boundaries: manifest-guard check-replace ecosystem-guard eyrie-client-guard eyrie-engine-guard peer-guard internal-layers-guard package-boundaries-guard ## Alias for all boundary guards (matches `make boundaries` in engine repos). release-parity: ## Verify every go.mod ecosystem version resolves to a reachable remote commit. bash ./scripts/check-module-release-parity.sh diff --git a/README.md b/README.md index e913769c7..c1279318f 100644 --- a/README.md +++ b/README.md @@ -471,8 +471,7 @@ Local development uses: Cross-repo contracts now live in `internal/contracts` (vendored from the removed `github.com/GrayCodeAI/eagle` module) so support repos do not depend -on Hawk internals. The old `hawk/shared/types` path has been removed; -external consumers should vendor the needed DTOs from `internal/contracts` +on Hawk internals. External consumers should vendor the needed DTOs from `internal/contracts` until a published contracts module exists. Current contract packages (`internal/contracts/`): diff --git a/cmd/credentials.go b/cmd/credentials.go index b97c6e359..80e560019 100644 --- a/cmd/credentials.go +++ b/cmd/credentials.go @@ -47,30 +47,7 @@ var credentialsRemoveCmd = &cobra.Command{ }, } -var credentialsMigrateCmd = &cobra.Command{ - Use: "migrate", - Short: "Import plaintext credential files into the OS secret store", - RunE: func(cmd *cobra.Command, args []string) error { - ctx := context.Background() - storage := hawkconfig.CredentialStorageStatus(ctx) - if !storage.Writable { - return fmt.Errorf("cannot migrate: %s", storage.Detail) - } - n, err := hawkconfig.MigrateEnvFileCredentials(ctx) - if err != nil { - return err - } - if n == 0 { - cmd.Println(auditTint("No plaintext credential files found (already using secure storage).", textMuted)) - } else { - cmd.Printf("%s\n", auditTint(fmt.Sprintf("Migrated %d key(s) to %s and removed plaintext credential files.", n, hawkconfig.CredentialStoreName()), doneGreen)) - } - return nil - }, -} - func init() { credentialsCmd.AddCommand(credentialsStatusCmd) - credentialsCmd.AddCommand(credentialsMigrateCmd) credentialsCmd.AddCommand(credentialsRemoveCmd) } diff --git a/cmd/manpage.go b/cmd/manpage.go index 2c65961d7..bc32d7fcb 100644 --- a/cmd/manpage.go +++ b/cmd/manpage.go @@ -117,7 +117,6 @@ func GenerateManPage() string { b.WriteString("Use \\fBhawk\\fR and \\fB/config\\fR to save keys; hawk does not read API keys from .env files.\n") b.WriteString(".TP\n\\fBhawk credentials status\\fR\nShow secure storage status\n") b.WriteString(".TP\n\\fBhawk credentials remove \\fR\nRemove a stored API key from the OS secret store\n") - b.WriteString(".TP\n\\fBhawk credentials migrate\\fR\nImport legacy plaintext credential files into the OS store\n") // Environment b.WriteString(".SH ENVIRONMENT\n") diff --git a/cmd/migrate_secrets_test.go b/cmd/migrate_secrets_test.go deleted file mode 100644 index 4e1a50ccc..000000000 --- a/cmd/migrate_secrets_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package cmd - -import ( - "bytes" - "errors" - "strings" - "testing" - - "github.com/GrayCodeAI/hawk/internal/observability/logger" -) - -func TestLogMigrateProviderSecretsError_Nil_NoOutput(t *testing.T) { - var buf bytes.Buffer - l := logger.New(&buf, logger.Debug) - - logMigrateProviderSecretsError(l, nil) - - if buf.Len() != 0 { - t.Errorf("expected no output for nil error, got: %q", buf.String()) - } -} - -func TestLogMigrateProviderSecretsError_LogsWarn(t *testing.T) { - var buf bytes.Buffer - l := logger.New(&buf, logger.Debug) - - logMigrateProviderSecretsError(l, errors.New("read provider.json: permission denied")) - - out := buf.String() - if !strings.Contains(out, "WARN") { - t.Errorf("expected WARN level, got: %q", out) - } - if !strings.Contains(out, "provider secret migration failed") { - t.Errorf("expected message about migration failure, got: %q", out) - } - if !strings.Contains(out, "permission denied") { - t.Errorf("expected error message in log, got: %q", out) - } -} - -func TestLogMigrateProviderSecretsError_IncludesRemediationHint(t *testing.T) { - var buf bytes.Buffer - l := logger.New(&buf, logger.Debug) - - logMigrateProviderSecretsError(l, errors.New("boom")) - - out := buf.String() - if !strings.Contains(out, "hawk /config") { - t.Errorf("expected remediation hint mentioning `hawk /config`, got: %q", out) - } - if !strings.Contains(out, "keychain") { - t.Errorf("expected remediation hint mentioning keychain, got: %q", out) - } -} - -func TestLogMigrateProviderSecretsError_RespectsLogLevel(t *testing.T) { - var buf bytes.Buffer - l := logger.New(&buf, logger.Error) // WARN < ERROR is filtered - - logMigrateProviderSecretsError(l, errors.New("boom")) - - if buf.Len() != 0 { - t.Errorf("WARN should be filtered at Error level, got: %q", buf.String()) - } -} diff --git a/cmd/root.go b/cmd/root.go index 83fae56e5..28e75e939 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -12,7 +12,6 @@ import ( hawkconfig "github.com/GrayCodeAI/hawk/internal/config" "github.com/GrayCodeAI/hawk/internal/engine" - "github.com/GrayCodeAI/hawk/internal/observability/logger" "github.com/GrayCodeAI/hawk/internal/onboarding" "github.com/GrayCodeAI/hawk/internal/plugin" "github.com/GrayCodeAI/hawk/internal/session" @@ -147,11 +146,6 @@ Run hawk and use /config to set up your first provider.`, registeredProviderCoun } if printMode || promptFlag != "" || inputFormat == "stream-json" || replFlag || watchFlag { - // Credential migration is deferred until a path that actually - // uses credentials: `hawk path`, `hawk version`, auto-skill and - // other cold commands no longer construct the eyrie engine - // (M17 — was ~1.8s on every root command). - logMigrateProviderSecretsError(logger.Default(), hawkconfig.MigrateProviderSecrets()) if promptFlag == "" && !replFlag && !watchFlag { stdinPrompt, err := readPromptFromStdin(inputFormat) if err != nil { @@ -205,9 +199,6 @@ Run hawk and use /config to set up your first provider.`, registeredProviderCoun return err } - // TUI path uses credentials — run the one-time hygiene pass here. - logMigrateProviderSecretsError(logger.Default(), hawkconfig.MigrateProviderSecrets()) - // Folder trust check — block starting CLI in an untrusted directory if tr := engine.ProjectTrust(""); tr.Blocked { return fmt.Errorf("cannot start CLI: folder not trusted (%s)\nProject-scoped hooks, MCP servers, and custom specialists are blocked.\nRun 'hawk trust add' to trust this folder before starting hawk", tr.Path) @@ -977,27 +968,3 @@ func resumeRecoveredSession(ctx context.Context, sessionID string) error { } return recoverRunChat() } - -// logMigrateProviderSecretsError surfaces a non-nil error from -// hawkconfig.MigrateProviderSecrets via the structured logger. -// -// MigrateProviderSecrets is a one-time hygiene pass that strips API keys -// from the on-disk provider.json (a known-bad location — see AGENTS.md). -// If it fails, the keys remain in the file and the user must be told so -// they can run hawk /config to move them to the OS keychain. Previously -// the error was silently discarded (cmd/root.go:114), so a failure left -// the user with secrets in plaintext and no indication that anything was -// wrong. -// -// We log and continue rather than failing startup: the migration is -// best-effort, and a missing or unreadable provider.json is not -// fatal — the rest of the app can still function. -func logMigrateProviderSecretsError(l *logger.Logger, err error) { - if err == nil { - return - } - l.Warn( - "provider secret migration failed; API keys may remain in provider.json. Run `hawk /config` to move them to the OS keychain.", - map[string]interface{}{"err": err.Error()}, - ) -} diff --git a/docs/COMPETITIVE.md b/docs/COMPETITIVE.md index baca16501..711cdd8f6 100644 --- a/docs/COMPETITIVE.md +++ b/docs/COMPETITIVE.md @@ -68,7 +68,7 @@ External star counts below are approximate web-search snapshots (2026-09-08), no 5. **Sandbox.** Docker-only fail-closed is strictest default alongside Codex net-off and Gemini gVisor. Tradeoff is onboarding friction without Docker. Must not add host-exec fallback (violates `docs/SECURITY-DEVELOPER.md:71-73`); fix with preflight/path/doctor messaging + image pull/build guidance. Gap-01. 6. **Memory/context.** AST repomap + Harrier graph + compaction segments + relevance-prune + conversation-arc + 80% tool-result clearing exceeds most. Missing: Hermes-style auto-skill learning loop (we have curator archive + harness — surface it). 7. **Multi-agent.** `mission` worktrees + family messenger + path reservations + budgets + portable `mission-graph.json` + `graph export` (hashes only) is unique verifiable-execution story. Surface it; no new runtime needed. -8. **MCP/skills/plugins.** MCP stdio/HTTP/SSE/WS + LSP + skills search/install/audit + curator matches Goose/Gemini/Codex. Contracts live in `internal/contracts` (no `shared/types`) — extensions vendor DTOs. Correct; don't regress. +8. **MCP/skills/plugins.** MCP stdio/HTTP/SSE/WS + LSP + skills search/install/audit + curator matches Goose/Gemini/Codex. Contracts live in `internal/contracts` — extensions vendor DTOs. Correct; don't regress. 9. **Media/computer-use.** Tools exist (`Browser/Screenshot/CodeMatch/SearchX/AppVerify/GenerateMedia/ComputerUse`) but media/computer/STT backends are nil-by-default seams. README notes router ships `ImageClient`/`AudioClient`; host wiring is the gap. Gap-05. Kitty graphics (image display) still missing despite terminal detection. Gap-03. 10. **Ops/determinism.** Daemon `:4590` health/ready/chat-SSE + cron + `exec --fanout N` + replay cache + circuit breaker + smart routing + harness eval is ahead of Pi minimalism and Aider single-agent. Keep; add published eval numbers (Gap-04). diff --git a/docs/DEVELOPER-PATH.md b/docs/DEVELOPER-PATH.md index 7e04ca941..70a2e9ba0 100644 --- a/docs/DEVELOPER-PATH.md +++ b/docs/DEVELOPER-PATH.md @@ -76,14 +76,13 @@ hawk models refresh `hawk path` treats these as important security conditions: - Eyrie's resolved `provider.json` must not contain secret fields -- legacy `~/.hawk/env` or `~/.hawk/.env` files should be migrated away - sensitive files like provider config and SSH paths should be blocked from agent reads Eyrie resolves provider state from `EYRIE_CONFIG_DIR` first, then the platform user-config directory. Hawk protects that resolved path even when it is customized or symlinked. -If Hawk detects old plaintext secrets, run Hawk once and complete `/config`, or remove the secret fields manually after backing up the file. +If Hawk detects secret fields in `provider.json`, back up the file, remove those fields manually, and save your keys again through `/config`. Read the full credential and isolation model in [SECURITY-DEVELOPER.md](./SECURITY-DEVELOPER.md). diff --git a/docs/SECURITY-DEVELOPER.md b/docs/SECURITY-DEVELOPER.md index a437e2f80..94350fd88 100644 --- a/docs/SECURITY-DEVELOPER.md +++ b/docs/SECURITY-DEVELOPER.md @@ -17,12 +17,8 @@ This document describes how hawk and eyrie handle API keys and agent isolation f |-------|------|--------| | `/config` paste flow → `eyrie/engine.Engine.SaveCredential` | `Engine.ResolveCredential` (secret store only) | `/config key remove` or `hawk credentials remove` | -On startup, Hawk asks the Eyrie engine facade to migrate legacy -`~/.hawk/env` / `~/.hawk/.env` values into the secret store and delete those -files. It also imports recognized historical secret fields from -`provider.json` before atomically rewriting that file with metadata only. A -secret-store or state-write failure aborts the rewrite and rolls back newly -imported values. +Hawk does not import keys from legacy `~/.hawk/env` / `~/.hawk/.env` files or +from secret fields in `provider.json`. Save every key through `/config`. Check status: `hawk credentials status`, `hawk path`, or `hawk preflight`. @@ -84,15 +80,14 @@ required for users, and neither provisioning path enables host execution. `~/.ssh/*`, etc. - **Bash**: `printenv`, `env`, reading hawk env paths, echoing `*_API_KEY` variables. -## Migration +## Secrets left on disk -- **Legacy env files**: startup migration imports `~/.hawk/env` and - `~/.hawk/.env` into the OS secret store, then deletes the plaintext files. -- **provider.json secrets**: Eyrie transactionally imports recognized top-level - and deployment credentials, atomically writes sanitized metadata, and uses a - temporary `provider.json.pre-secret-migrate.bak` only during the transaction. -- **All subsequent writes**: the Eyrie engine applies the same sanitization and - atomic-write path, so migrated secret fields cannot be reintroduced. +- **provider.json secrets**: `hawk path` fails its `provider.json` security + check when the file still holds secret fields. Hawk does not remove them + automatically: back up the file, delete the secret fields, and save the keys + again through `/config`. +- **Provider state writes**: the Eyrie engine sanitizes provider state and + writes it atomically, so secret fields are never written back to disk. ## Provider state path diff --git a/docs/architecture.md b/docs/architecture.md index c3d1fe39a..6d0f49c4b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -55,9 +55,6 @@ hawk/ └── (ecosystem siblings live at ../ in the graycode-eco workspace; see docs/architecture/ecosystem-design.md) ``` -Legacy note: `hawk/shared/types` has been removed. Shared cross-repo severity -and finding contracts now live in `eagle/types`. - --- ## globe Daemon HTTP API (:4590) @@ -130,4 +127,4 @@ Tool Call → 0, paths -} diff --git a/internal/config/developer_path_test.go b/internal/config/developer_path_test.go index a84c48c6d..ead270030 100644 --- a/internal/config/developer_path_test.go +++ b/internal/config/developer_path_test.go @@ -58,14 +58,6 @@ func TestProviderJSONHasSecretsOnDisk_None(t *testing.T) { } } -func TestPlaintextCredentialFilesPresent_None(t *testing.T) { - isolateMilestoneTest(t) - found, paths := plaintextCredentialFilesPresent() - if found || len(paths) > 0 { - t.Fatalf("expected no plaintext files, got %v", paths) - } -} - func TestPathStatusGlyph(t *testing.T) { if pathStatusGlyph(PathPass) != icons.CheckBold() { t.Fatal("pass glyph") diff --git a/internal/config/eyrie_engine.go b/internal/config/eyrie_engine.go index c480e9387..f5e7fd623 100644 --- a/internal/config/eyrie_engine.go +++ b/internal/config/eyrie_engine.go @@ -56,10 +56,6 @@ func CredentialStorageStatus(ctx context.Context) gateway.CredentialStorageRepor return gateway.CredentialStorage(ctx) } -func MigrateEnvFileCredentials(ctx context.Context) (int, error) { - return gateway.MigrateEnvFileCredentials(ctx) -} - // EnginePreflightReport runs preflight against the default gateway. func EnginePreflightReport(ctx context.Context) EnginePreflight { return gateway.PreflightWithProviders(ctx, nil, EnginePreflightOptions{}) @@ -124,14 +120,6 @@ func ProviderStateSecurityStatus() gateway.ProviderStateSecurity { return gw.ProviderStateSecurityStatus() } -func MigrateEngineProviderSecrets() error { - gw, err := newEyrieEngine() - if err != nil { - return err - } - return gw.MigrateProviderSecrets() -} - func EngineDeploymentSummary(ctx context.Context, model string) (gateway.DeploymentSummary, error) { gw, err := newEyrieEngine() if err != nil { diff --git a/internal/config/eyrie_selection.go b/internal/config/eyrie_selection.go index 3cf28054c..b707a4b68 100644 --- a/internal/config/eyrie_selection.go +++ b/internal/config/eyrie_selection.go @@ -2,7 +2,6 @@ package config import ( "context" - "strings" "github.com/GrayCodeAI/hawk/internal/provider/gateway" ) @@ -83,65 +82,6 @@ func SetActiveProvider(ctx context.Context, provider string) error { return engine.SetActiveProvider(ctx, provider) } -// SetActiveSelection validates and persists a provider/model pair atomically. -// Use this for migrations and other flows where persisting only one half would -// leave provider state inconsistent. -func SetActiveSelection(ctx context.Context, provider, modelID string) error { - if ctx == nil { - ctx = context.Background() - } - engine, err := newEyrieEngine() - if err != nil { - return err - } - return engine.SetSelection(ctx, provider, modelID) -} - -// migrateStoredModelProvider moves model/provider from ~/.hawk/settings.json into eyrie once. -func migrateStoredModelProvider(s *Settings) { - if s == nil { - return - } - ctx := context.Background() - oldModel := strings.TrimSpace(s.Model) - oldProvider := strings.TrimSpace(s.Provider) - activeModel := strings.TrimSpace(ActiveModel(ctx)) - activeProvider := strings.TrimSpace(ActiveProvider(ctx)) - changed := false - - // Existing Eyrie state is authoritative. Otherwise migrate a stored pair - // in one validated write so a rejected model cannot strand only the - // provider in the destination or silently erase the user's source value. - if activeModel != "" { - if oldModel != "" { - s.Model = "" - changed = true - } - if oldProvider != "" { - s.Provider = "" - changed = true - } - } else if oldModel != "" { - provider := activeProvider - if provider == "" { - provider = oldProvider - } - if err := SetActiveSelection(ctx, provider, oldModel); err == nil { - s.Model = "" - s.Provider = "" - changed = true - } - } else if oldProvider != "" { - if activeProvider != "" || SetActiveProvider(ctx, oldProvider) == nil { - s.Provider = "" - changed = true - } - } - if changed { - _ = SaveGlobal(*s) - } -} - func stripHostModelSelection(s Settings) Settings { s.Model = "" s.Provider = "" diff --git a/internal/config/migrate_provider_secrets.go b/internal/config/migrate_provider_secrets.go deleted file mode 100644 index 88d1108b1..000000000 --- a/internal/config/migrate_provider_secrets.go +++ /dev/null @@ -1,6 +0,0 @@ -package config - -// MigrateProviderSecrets strips api keys from on-disk provider.json (one-time hygiene). -func MigrateProviderSecrets() error { - return MigrateEngineProviderSecrets() -} diff --git a/internal/config/milestone_verify_test.go b/internal/config/milestone_verify_test.go index 7fbb1767d..f3606fc02 100644 --- a/internal/config/milestone_verify_test.go +++ b/internal/config/milestone_verify_test.go @@ -45,44 +45,6 @@ func TestVerify_ProviderJSONOnDiskHasNoSecrets(t *testing.T) { assertProviderJSONFileHasNoSecrets(t, path) } -func TestVerify_MigrateProviderSecretsStripsDisk(t *testing.T) { - hawkDir := isolateMilestoneTest(t) - store := &credentials.MapStore{} - credentials.SetDefaultStore(store) - t.Cleanup(func() { credentials.SetDefaultStore(nil) }) - path := filepath.Join(hawkDir, "provider.json") - secret := "sk-ant-migrate-verify-key-1234567890" - raw := `{ - "version": "1", - "config_version": 2, - "openai_api_key": "` + secret + `-top-level", - "deployments": { - "anthropic-direct": { - "api_key": "` + secret + `" - } - } -}` - if err := os.WriteFile(path, []byte(raw), 0o600); err != nil { - t.Fatal(err) - } - if err := MigrateProviderSecrets(); err != nil { - t.Fatal(err) - } - for _, envKey := range []string{"OPENAI_API_KEY", "ANTHROPIC_API_KEY"} { - if value, err := store.Get(context.Background(), credentials.AccountForEnv(envKey)); err != nil || !strings.Contains(value, secret) { - t.Fatalf("legacy %s was not imported before sanitizing: value=%q err=%v", envKey, value, err) - } - } - assertProviderJSONFileHasNoSecrets(t, path) - data, err := os.ReadFile(path) - if err != nil { - t.Fatal(err) - } - if strings.Contains(string(data), secret) { - t.Fatal("provider.json still contains a legacy or deployment API key after migrate") - } -} - func TestVerify_PersistAPIKeyDoesNotWriteProviderJSON(t *testing.T) { hawkDir := isolateMilestoneTest(t) credentials.SetDefaultStore(emptyCredentialStore{}) diff --git a/internal/config/settings.go b/internal/config/settings.go index d9ab5dcc1..d4725167f 100644 --- a/internal/config/settings.go +++ b/internal/config/settings.go @@ -32,8 +32,8 @@ type Settings struct { // PolicySchemaVersion versions permission/autonomy/sandbox fields. Zero is // the legacy format and is migrated to CurrentPolicySchemaVersion on load. PolicySchemaVersion int `json:"policy_schema_version,omitempty"` - // Model and Provider are retained only for one-time migration into eyrie provider.json. - // Hawk does not persist model/provider here; use SetActiveModel / SetActiveProvider. + // Model and Provider carry host-level selection only. Hawk does not persist + // model/provider here; Eyrie owns the stored selection (SetActiveModel / SetActiveProvider). Model string `json:"model,omitempty"` Provider string `json:"provider,omitempty"` Theme string `json:"theme,omitempty"` @@ -238,7 +238,6 @@ func LoadSettings() Settings { // authority stays in the user profile or explicit runtime overrides. s = MergeSettings(s, projectSafeSettings(*project)) } - migrateStoredModelProvider(&s) if s.PolicySchemaVersion == 0 { s.PolicySchemaVersion = CurrentPolicySchemaVersion } diff --git a/internal/provider/gateway/engine_client.go b/internal/provider/gateway/engine_client.go index 881e7578c..709c543a5 100644 --- a/internal/provider/gateway/engine_client.go +++ b/internal/provider/gateway/engine_client.go @@ -106,7 +106,6 @@ type CatalogMaintenance interface { DefaultProviderFilter(ctx context.Context) string PreflightWithOptions(ctx context.Context, opts eyrieengine.PreflightOptions) eyrieengine.PreflightReport ProviderStateSecurityStatus() eyrieengine.ProviderStateSecurity - MigrateProviderSecrets() error } // engineProvider is the production Provider: a thin wrapper over Eyrie's @@ -303,10 +302,6 @@ func (p *engineProvider) ProviderStateSecurityStatus() eyrieengine.ProviderState return p.eng.ProviderStateSecurityStatus() } -func (p *engineProvider) MigrateProviderSecrets() error { - return p.eng.MigrateProviderSecrets() -} - func (p *engineProvider) SupportsNativeCompaction(ctx context.Context, provider, model string) bool { return p.eng.SupportsNativeCompaction(ctx, provider, model) } diff --git a/internal/provider/gateway/gateway.go b/internal/provider/gateway/gateway.go index b2ff72d0c..6e3665a3b 100644 --- a/internal/provider/gateway/gateway.go +++ b/internal/provider/gateway/gateway.go @@ -399,10 +399,6 @@ func CredentialStorage(ctx context.Context) CredentialStorageReport { return eyrieengine.CredentialStorage(ctx) } -func MigrateEnvFileCredentials(ctx context.Context) (int, error) { - return eyrieengine.MigrateEnvFileCredentials(ctx) -} - func CredentialGuidance(providerID, secret string) string { return eyrieengine.CredentialGuidance(providerID, secret) } diff --git a/internal/provider/gateway/provider_stub_test.go b/internal/provider/gateway/provider_stub_test.go index f822e483a..70970de50 100644 --- a/internal/provider/gateway/provider_stub_test.go +++ b/internal/provider/gateway/provider_stub_test.go @@ -136,8 +136,6 @@ func (s *stubProvider) ClearSelection(context.Context) error { ret func (s *stubProvider) ProviderStateSecurityStatus() eyrieengine.ProviderStateSecurity { return eyrieengine.ProviderStateSecurity{} } -func (s *stubProvider) MigrateProviderSecrets() error { return nil } -func (s *stubProvider) MigrateProviderSecretsContext(context.Context) error { return nil } func (s *stubProvider) SupportsNativeCompaction(context.Context, string, string) bool { return false } func (s *stubProvider) CompactNative(context.Context, eyrieengine.NativeCompactionRequest) (string, error) { diff --git a/internal/testaudit/audit_test.go b/internal/testaudit/audit_test.go index 77e812e61..801d357a0 100644 --- a/internal/testaudit/audit_test.go +++ b/internal/testaudit/audit_test.go @@ -256,31 +256,6 @@ func TestNoLazyProviderConstructionInHawk(t *testing.T) { } } -// TestNoDirectSharedTypesImports verifies Hawk does not reintroduce the removed -// legacy shared/types import path into production code. -func TestNoDirectSharedTypesImports(t *testing.T) { - root := repoRoot(t) - paths := []string{ - filepath.Join(root, "internal"), - filepath.Join(root, "cmd"), - } - - for _, dir := range paths { - files := parseGoFiles(t, dir) - for _, pf := range files { - rel := relPath(root, pf.Path) - for _, imp := range pf.File.Imports { - path := strings.Trim(imp.Path.Value, `"`) - if path != "github.com/GrayCodeAI/hawk/shared/types" { - continue - } - pos := pf.FSet.Position(imp.Pos()) - t.Fatalf("forbidden direct hawk/shared/types import at %s:%d; the path has been removed, use internal/contracts instead", rel, pos.Line) - } - } - } -} - // TestAllExportedTypesHaveDocComments verifies that all exported type // declarations in non-test .go files have doc comments. func TestAllExportedTypesHaveDocComments(t *testing.T) { diff --git a/internal/testaudit/docs_audit_test.go b/internal/testaudit/docs_audit_test.go index 7a9b0da0a..16cac14b8 100644 --- a/internal/testaudit/docs_audit_test.go +++ b/internal/testaudit/docs_audit_test.go @@ -72,28 +72,3 @@ func TestArchitectureDocsMentionCurrentReviewVerifyContracts(t *testing.T) { } } } - -func TestArchitectureDocsDescribeSharedTypesAsRemoved(t *testing.T) { - root := repoRoot(t) - - files := []string{ - "README.md", - "AGENTS.md", - "docs/architecture.md", - } - - for _, rel := range files { - path := filepath.Join(root, rel) - data, err := os.ReadFile(path) - if err != nil { - t.Fatalf("read %s: %v", rel, err) - } - content := strings.ToLower(string(data)) - if !strings.Contains(content, "shared/types") { - t.Fatalf("expected %s to mention shared/types", rel) - } - if !strings.Contains(content, "removed") { - t.Fatalf("expected %s to describe shared/types as removed", rel) - } - } -} diff --git a/internal/testaudit/package_boundaries_test.go b/internal/testaudit/package_boundaries_test.go index 75dc7499f..8415331b4 100644 --- a/internal/testaudit/package_boundaries_test.go +++ b/internal/testaudit/package_boundaries_test.go @@ -98,7 +98,7 @@ func checkSupportRepositoryBoundaries(t *testing.T, root string) { for _, owner := range supportEngines { for _, repoRoot := range repositoryRoots(root, owner) { for _, imp := range productionImports(t, root, repoRoot) { - if strings.HasPrefix(imp.path, hawkModule+"/internal/") || imp.path == hawkModule+"/shared/types" { + if strings.HasPrefix(imp.path, hawkModule+"/internal/") { violations = append(violations, formatImportViolation(root, imp, "support engines must not import Hawk internals")) continue } diff --git a/lefthook.yml b/lefthook.yml index 6d134c79d..816cbfb9c 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -112,9 +112,6 @@ pre-push: fi govulncheck ./... - boundary-shared-types: - run: bash scripts/check-shared-types-imports.sh - boundary-eyrie-client: run: bash scripts/check-eyrie-client-imports.sh diff --git a/scripts/check-ecosystem-boundaries.sh b/scripts/check-ecosystem-boundaries.sh index 51ed04c73..4f848189c 100755 --- a/scripts/check-ecosystem-boundaries.sh +++ b/scripts/check-ecosystem-boundaries.sh @@ -4,7 +4,7 @@ set -euo pipefail ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" cd "$ROOT_DIR" -pattern='github\.com/GrayCodeAI/hawk/(internal/|shared/types)' +pattern='github\.com/GrayCodeAI/hawk/internal/' violations="" while IFS= read -r repo; do @@ -24,7 +24,7 @@ if [[ -n "${violations}" ]]; then echo "forbidden Hawk imports found in sibling ecosystem repos:" echo "${violations}" echo - echo "support repos must use their own contracts, not hawk/internal or removed hawk/shared/types" + echo "support repos must use their own contracts, not hawk/internal" exit 1 fi diff --git a/scripts/check-shared-types-imports.sh b/scripts/check-shared-types-imports.sh deleted file mode 100644 index 69d161022..000000000 --- a/scripts/check-shared-types-imports.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -cd "$ROOT_DIR" - -violations="$( - git grep -n 'github\.com/GrayCodeAI/hawk/shared/types' -- '*.go' \ - ':(exclude)shared/types/**' \ - ':(exclude)internal/testaudit/audit_test.go' || true -)" - -if [[ -n "${violations}" ]]; then - echo "forbidden imports of removed github.com/GrayCodeAI/hawk/shared/types found:" - echo "${violations}" - echo - echo "hawk/shared/types has been removed; use internal/contracts instead" - exit 1 -fi - -echo "legacy shared/types import guard passed" From 5e7924b485eaf7739ab35aed326f161692062766 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Fri, 11 Sep 2026 13:57:24 +0530 Subject: [PATCH 2/2] fix(config): ignore stale model/provider in settings.json on load With the one-time migration gone, a model/provider left in an old ~/.hawk/settings.json would act as a host override over Eyrie's stored selection. LoadGlobalSettings now drops them (SaveGlobal already did); --settings overrides still apply. Also gofumpt the gateway test stub. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 +- docs/SECURITY-DEVELOPER.md | 2 +- internal/config/config_test.go | 5 ++++- internal/config/settings.go | 8 ++++++-- internal/provider/gateway/provider_stub_test.go | 1 + 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4b84d3c1..4c25e3101 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed - **BREAKING — `hawk credentials migrate` removed**: the subcommand, its man page entry, and the `MigrateEnvFileCredentials` wrappers are gone, and `hawk path` no longer reports legacy `~/.hawk/env` / `~/.hawk/.env` files. Save keys through `/config`. -- **BREAKING — settings model/provider migration removed**: `LoadSettings` no longer moves `model`/`provider` from `settings.json` into Eyrie's `provider.json`, and `SetActiveSelection` is deleted. Stale `model`/`provider` values left in `settings.json` are no longer cleared; select the model in `/config`. +- **BREAKING — settings model/provider migration removed**: `LoadSettings` no longer moves `model`/`provider` from `settings.json` into Eyrie's `provider.json`, and `SetActiveSelection` is deleted. Stale `model`/`provider` values left in `settings.json` are now ignored on load (Eyrie's selection wins; `--settings` overrides still apply); select the model in `/config`. - **BREAKING — startup provider-secrets migration removed**: print/REPL/watch/TUI startup no longer calls `MigrateProviderSecrets`, and the method is dropped from the gateway `CatalogMaintenance` interface. `hawk path` still fails when `provider.json` holds secrets; remove those fields manually. - **BREAKING — `shared/types` guards removed**: `scripts/check-shared-types-imports.sh`, the `contracts-guard` make target, its lefthook and CI steps, and the matching testaudit checks are deleted because the package no longer exists. The ecosystem boundary guards still block `hawk/internal` imports. diff --git a/docs/SECURITY-DEVELOPER.md b/docs/SECURITY-DEVELOPER.md index 94350fd88..39366f831 100644 --- a/docs/SECURITY-DEVELOPER.md +++ b/docs/SECURITY-DEVELOPER.md @@ -37,7 +37,7 @@ Eyrie engine discover/apply (credentials from store, not JSON body) SetupUI JSON (display_name + canonical_id per model) | v -User picks model -> settings.json (canonical id only) +User picks model -> Eyrie provider.json (canonical id only) ``` Remove a stored key: `/config key remove` (interactive picker). diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c1c91ba1b..d53d0fa27 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -174,11 +174,14 @@ func TestLoadSettingsUsesUserConfigOnly(t *testing.T) { if err := os.MkdirAll(configDir, 0o755); err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(configDir, "settings.json"), []byte(`{"allowedTools":["Read"]}`), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(configDir, "settings.json"), []byte(`{"model":"stale-model","provider":"openai","allowedTools":["Read"]}`), 0o644); err != nil { t.Fatal(err) } settings := LoadSettings() + if settings.Model != "" || settings.Provider != "" { + t.Fatalf("stale settings.json model/provider must be ignored, got model=%q provider=%q", settings.Model, settings.Provider) + } if len(settings.AllowedTools) != 1 || settings.AllowedTools[0] != "Read" { t.Fatalf("expected global allowedTools, got %v", settings.AllowedTools) } diff --git a/internal/config/settings.go b/internal/config/settings.go index d4725167f..e9c449e82 100644 --- a/internal/config/settings.go +++ b/internal/config/settings.go @@ -32,8 +32,9 @@ type Settings struct { // PolicySchemaVersion versions permission/autonomy/sandbox fields. Zero is // the legacy format and is migrated to CurrentPolicySchemaVersion on load. PolicySchemaVersion int `json:"policy_schema_version,omitempty"` - // Model and Provider carry host-level selection only. Hawk does not persist - // model/provider here; Eyrie owns the stored selection (SetActiveModel / SetActiveProvider). + // Model and Provider carry runtime host overrides only (e.g. --settings). + // Eyrie owns the stored selection (SetActiveModel / SetActiveProvider); + // values found in settings.json are dropped on load and on save. Model string `json:"model,omitempty"` Provider string `json:"provider,omitempty"` Theme string `json:"theme,omitempty"` @@ -222,6 +223,9 @@ func LoadGlobalSettings() Settings { slog.Warn("failed to parse settings", "path", path, "error", err) } } + // Eyrie owns the stored model/provider selection. Stale values left in + // settings.json must not act as a host override. + s = stripHostModelSelection(s) if s.PolicySchemaVersion == 0 { s.PolicySchemaVersion = CurrentPolicySchemaVersion } diff --git a/internal/provider/gateway/provider_stub_test.go b/internal/provider/gateway/provider_stub_test.go index 70970de50..fbcee4fca 100644 --- a/internal/provider/gateway/provider_stub_test.go +++ b/internal/provider/gateway/provider_stub_test.go @@ -136,6 +136,7 @@ func (s *stubProvider) ClearSelection(context.Context) error { ret func (s *stubProvider) ProviderStateSecurityStatus() eyrieengine.ProviderStateSecurity { return eyrieengine.ProviderStateSecurity{} } + func (s *stubProvider) SupportsNativeCompaction(context.Context, string, string) bool { return false } func (s *stubProvider) CompactNative(context.Context, eyrieengine.NativeCompactionRequest) (string, error) {