Skip to content

feat: auto-detect harness and harden install/validation - #64

Merged
dkmnx merged 23 commits into
mainfrom
develop
Sep 13, 2026
Merged

dkmnx merged 23 commits into
mainfrom
develop

Conversation

@dkmnx

@dkmnx dkmnx commented Sep 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

Brings develop into main with harness auto-detection, Crush catalog env-var support, install/validation hardening, and a cmd/ digest into internal/secrets + internal/app.

Changes

  • [BREAKING] When no harness is configured, scan PATH (pi, claude, qwen, crush) and use the first installed CLI; if none are found, list supported harnesses and suggest kairo harness set
  • Crush receives catalog-specific API-key env vars (HF_TOKEN, GEMINI_API_KEY, etc.)
  • Move encrypted secrets store I/O to internal/secrets and env/launch resolution to internal/app
  • Fail-closed install checksums, SSRF/IP-encoding checks, atomic --reset-secrets, PowerShell wrapper/installer fixes, exit-code and checksum-compare fixes
  • Go 1.27 toolchain; golangci-lint v2.13.2
  • Consolidate Unreleased changelog to net user-facing final state vs v2.10.3

Test Plan

  • go test ./... (race) passes locally
  • golangci-lint run ./... clean
  • CI green on develop (lint, test, build across ubuntu/windows/macos)
  • Fresh install with only claude on PATH launches Claude when default_harness is unset
  • Machine with no harness CLI prints the install reminder

Notes

  • Breaking: implicit default is no longer claude (or a hard-coded pi). Pin with --harness or kairo harness set for a fixed choice
  • Squash or merge-commit both fine; no release tag is implied by this PR

dkmnx and others added 23 commits July 20, 2026 21:20
- cancel session context on stop in execution signal handling
install.sh and install.ps1 previously installed unverified binaries when the
checksums file could not be downloaded or lacked an entry for the artifact.
Both now abort on download failure, missing entry, and hash mismatch.

Also fixes install.ps1 silently skipping cosign verification: $versionNoPrefix
was scoped to Get-Checksum and read as $null in Install-Binary, producing a
malformed bundle URL that always failed to download.

Regenerates scripts/checksums.txt for the modified scripts.
ResetSecretsFiles deleted the old age.key and secrets.age before generating
the replacement key, so an interrupted or failed reset (e.g. disk full)
permanently destroyed stored API keys.

The new key is now generated to a temp path first, then swapped in with a
rename: old key -> .backup, new key -> age.key, then the backup and old
secrets are removed. Failure before the swap leaves the old key and secrets
intact; a failed install rename restores the old key.

mockCrypto.GenerateKey now delegates to the real implementation by default,
matching the rename contract, and resetDeps injects failure at GenerateKey.
Corrects the KAIRO_REQUIRE_COSIGN claim: strict mode only aborts on an
actual bundle verification failure, not on a missing cosign binary.
Documents that standalone install scripts now fail closed on missing or
mismatched checksums, and records the security fixes in the changelog.
ValidateURL accepted hosts that resolvers interpret as loopback/private IPs
via inet_aton rules (127.1, 2130706433, 0x7f000001, 0177.0.0.1) and any
hostname resolving to a private/link-local address (DNS rebinding), since
net.ParseIP only accepts canonical forms and no resolution was performed.

The host is now canonicalized (trailing dot stripped), legacy numeric forms
are parsed with the classic inet_aton grammar and checked against the
blocked CIDRs, and hostnames are resolved with every address checked.
DNS resolution is injectable so tests stay offline and deterministic.
setup: persist encrypted secrets before config.yaml so a failure cannot
leave a configured provider without its stored key; add a regression test
that injects an EncryptSecrets failure and asserts the config is untouched.

wrapper: correct PowerShell argument escaping. The previous escaper
backtick-prefixed $ " & ; | % and control chars inside single quotes,
which PowerShell treats as literal — the harness received corrupted values
(e.g. `$HOME). Escaping is now single-quote wrapping with quote doubling
only, paths are quoted the same way instead of Go %q, and a real
PowerShell round-trip test proves values arrive intact.

tests: prompt tests previously relied on fixed 50ms sleeps, which raced
slow prompt setup (DNS, race detector) and hung. They now wait for the
prompt to render and retry keypresses until the output advances. Also
restores the leaked verboseFlag package global in root_execute_test.go.
Model validation previously applied its charset only to the four catalog
providers with default models; anthropic/openai/google and friends accepted
anything, while custom providers were skipped entirely. Validation is now
uniform for every provider, and the charset accepts common identifier
punctuation (: / + @ parens) that real model IDs use (org/model,
deployment:version, @cf/meta/...). Whitespace-only names pass through so
callers keep their required-field rules.

Cross-provider env conflict detection skipped valueless entries, never
compared API-key env vars, and reported only the first conflict. It now
binds valueless entries (empty value), flags shared API-key variables
(which would overwrite each other when both providers run), and aggregates
all conflicting keys into one error. The catalog's colliding env-var
defaults (kimi vs minimax, zai vs deepseek) were removed from kimi and zai
so catalog data is self-consistent.
- Preserve the harness exit code (e.g. 130 for Ctrl-C) when a run fails
  instead of always exiting 1
- Compare download checksums in constant time (crypto/subtle), keeping the
  case-insensitive contract by normalizing case first
- Prune blockedHosts entries already covered by the blocked CIDRs
  (127.0.0.1, ::1, 0.0.0.0, 169.254.169.254); keep 'localhost' and '::'
  (IPv6 unspecified, not covered by ::1/128)
- Align docs provider ordering with providerPriority (deepseek before kimi)
- Remove a test that asserted Go stdlib strings.Contains behavior
- PowerShell installer stops only kairo.exe processes running the target
  binary during self-update; instances from other install paths are left
  alone (previously all kairo.exe processes were force-killed)
- Regenerate install script checksums
When neither the --harness flag nor default_harness is configured, the
effective harness falls back to pi instead of claude (harness.Resolve,
resolveHarness warning, and 'harness get' message updated accordingly).
Docs examples and notes updated to reflect the new default.
Align go.mod, CI workflows, and docs with the current Go 1.27 toolchain so local builds, tests, and GitHub Actions all use the same language version.
runPiProvider inlined catalog/EnvKey/conventional env-var resolution and multi-key injection in one loop. Pull that into injectPiAPIKeys and apiKeyEnvVarName so the multi-provider behavior is unit-testable, and document in architecture why Pi receives every configured provider key (in-session provider switching) instead of only the launch provider's.
LoadSecrets, SaveSecrets, and ResetSecretsFiles lived in cmd/ but only needed a crypto.Service and paths. Move them to internal/secrets as Load/Save/Reset so the store is testable without CLIContext, and keep returning paths on decrypt failure so --reset-secrets can still rotate a corrupt store.
BuildProviderEnv, Pi multi-key injection, and API-key env-var resolution lived in cmd/ but only needed a crypto.Service, config dir, and provider config — not cobra or CLIContext. Move them to internal/app so env construction is unit-testable without the CLI shell, and drop the cmd mergeEnvVars alias now that app calls envutil.Merge directly.
OrchestrateExecution mixed config loading, argument parsing, provider lookup, and user-facing error text in one cobra Run. Move pure resolution (ResolveExecution, SplitArgs, ProviderFromArgs) into internal/app with typed errors so it is testable without cobra, leave printing in cmd, and carry RootCtx on ExecutionConfig so harness exec no longer reaches back into CLIContext.
The architecture tree still described secrets load/save as living in cmd and had no internal/app entry after the cmd digest. Update the tree and key-API table, fix AGENTS.md to point at providerPriority instead of the computed providerOrder, and fail TestDocsCoverPublicSymbols when an internal package is missing from docs/architecture so the next package cannot land undocumented.
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 6 to 7.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@v6...v7)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [filippo.io/age](https://github.com/FiloSottile/age) from 1.3.1 to 1.3.2.
- [Release notes](https://github.com/FiloSottile/age/releases)
- [Commits](FiloSottile/age@v1.3.1...v1.3.2)

---
updated-dependencies:
- dependency-name: filippo.io/age
  dependency-version: 1.3.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Crush received the conventional PROVIDER_API_KEY name from harness.Dispatch, so providers whose tools expect catalog names (HF_TOKEN for huggingface, GEMINI_API_KEY for google) never saw the key. Resolve the wrapper EnvVarName via app.APIKeyEnvVarName for Crush only; secrets.age stays keyed by the conventional form. Document the two naming layers at the app seam.
golangci-lint v2.12.2 panics in staticcheck/buildir on Go 1.27 (unexpected *ast.KeyValueExpr in package poll). v2.13.2 includes honnef.co/go/tools 0.8.1, which supports Go 1.27; local golangci-lint run reports 0 issues.
The Unreleased section had mixed in CI pins, internal package moves, and test-only fixes. Drop those and record the harness session-context cancel fix so the section matches what users of the CLI actually see since v2.10.3.
Fresh installs always launched pi even when the user only had claude or qwen on PATH. When neither --harness nor default_harness is set, scan PATH in order (pi, claude, qwen, crush) and use the first hit; if none are installed, list the supported harnesses and suggest kairo harness set instead of failing later on a missing binary.
Entries still described an intermediate default-to-pi state that never shipped. Relative to v2.10.3 the default was claude; collapse that path into a single breaking note about PATH auto-detect and keep only net user-facing fixes.
@dkmnx dkmnx added bug Something isn't working enhancement New feature or request labels Sep 13, 2026
@dkmnx
dkmnx merged commit f9de287 into main Sep 13, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant