From 24ead979b4426867bacab190644c6219d17317bb Mon Sep 17 00:00:00 2001 From: Joah Gerstenberg Date: Tue, 4 Aug 2026 14:53:49 -0500 Subject: [PATCH 1/2] feat: link agent skills into environments on activation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Environments can now declare agent skills — SKILL.md directories used by AI coding agents — in bin/hermit.hcl, and Hermit provides them to the project the same way it provides toolchains: skill-repo "https://github.com/org/repo.git" { path = "skills" skills = ["my-skill"] # ref = "" # optional security pin } On activation (both the activate command and 'hermit env --activate') each repository is resolved to a commit — the pinned SHA, or the remote HEAD re-checked at most every 15 minutes — and each declared skill is materialised as an immutable, content-addressed snapshot under the Hermit state directory, then symlinked into .agents/skills/ and .claude/skills/. Skill content never enters the project's version control: only the hermit.hcl declaration is committed, and Hermit warns when the link directories are not gitignored. Reconciliation is self-healing and conservative: links for undeclared skills are removed via a per-env ownership ledger, repo-committed skill directories and foreign symlinks are never touched, and an unreachable remote degrades to the last good snapshot instead of blocking activation or tearing down working links. Buzz-Message: buzz://message?channel=573ff355-6c19-422d-b425-112853d0ec7e&id=39bb545df8409daa9df623a016f92a1da98bac803d0e370417a65235c208f327 Amp-Thread-ID: https://ampcode.com/threads/T-019fce44-560f-770d-bacb-fba12dd9ae31 Co-authored-by: Amp --- agentskills/agentskills.go | 134 +++++++++++++++ agentskills/agentskills_test.go | 284 ++++++++++++++++++++++++++++++++ agentskills/git.go | 277 +++++++++++++++++++++++++++++++ agentskills/link.go | 176 ++++++++++++++++++++ app/activate_cmd.go | 4 + app/env_cmd.go | 4 + docs/docs/usage/config.md | 41 +++++ env.go | 11 ++ 8 files changed, 931 insertions(+) create mode 100644 agentskills/agentskills.go create mode 100644 agentskills/agentskills_test.go create mode 100644 agentskills/git.go create mode 100644 agentskills/link.go diff --git a/agentskills/agentskills.go b/agentskills/agentskills.go new file mode 100644 index 000000000..7bfd9d4d1 --- /dev/null +++ b/agentskills/agentskills.go @@ -0,0 +1,134 @@ +// Package agentskills materialises agent skills declared in a Hermit +// environment's configuration and links them into the environment's +// .agents/skills and .claude/skills directories on activation. +// +// Skills are resolved to immutable, content-addressed snapshots under the +// Hermit state directory. The environment only ever contains symlinks into +// those snapshots; skill content is never written into the project itself. +package agentskills + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "path/filepath" + "regexp" + "strings" + "time" + + "github.com/cashapp/hermit/errors" + "github.com/cashapp/hermit/ui" +) + +// SkillRepo configures a git repository providing agent skills. +type SkillRepo struct { + URL string `hcl:"url,label" help:"Git repository URL providing agent skills."` + Path string `hcl:"path,optional" help:"Subdirectory within the repository containing the skill directories."` + Skills []string `hcl:"skills" help:"Names of the skill directories to link into the environment."` + Ref string `hcl:"ref,optional" help:"Full commit SHA to pin to. When omitted the remote HEAD is used, re-checked at most every 15 minutes."` +} + +const ( + // stateSubdir is the directory under the Hermit state dir holding all + // agent skill state: + // + // snapshots/@/ immutable skill content + // refs/.json freshness stamps per repository URL + // ledgers/.json per-environment link ledgers + // tmp/ transient checkouts + // .lock global snapshot lock + stateSubdir = "agent-skills" + + // freshnessWindow is how long a resolved remote HEAD is trusted before + // it is re-checked on activation. + freshnessWindow = 15 * time.Minute + + // resolveTimeout bounds remote git operations so offline activation + // never blocks the shell for long. + resolveTimeout = 10 * time.Second +) + +var ( + skillNameRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,63}$`) + commitSHARe = regexp.MustCompile(`^[0-9a-f]{40}$`) + + // linkDirs are the environment-relative directories skills are linked + // into. Both are ecosystem conventions for agent skill discovery. + linkDirs = []string{ + filepath.Join(".agents", "skills"), + filepath.Join(".claude", "skills"), + } +) + +// Validate checks the skill repository declarations for configuration errors. +func Validate(repos []SkillRepo) error { + seen := map[string]string{} + for _, repo := range repos { + if repo.URL == "" { + return errors.Errorf("skill-repo: repository URL is required") + } + if strings.HasPrefix(repo.URL, "-") { + return errors.Errorf("skill-repo %q: invalid repository URL", repo.URL) + } + if len(repo.Skills) == 0 { + return errors.Errorf("skill-repo %q: at least one skill name is required", repo.URL) + } + if repo.Ref != "" && !commitSHARe.MatchString(repo.Ref) { + return errors.Errorf("skill-repo %q: ref must be a full 40-character commit SHA, got %q", repo.URL, repo.Ref) + } + if repo.Path != "" { + clean := filepath.Clean(repo.Path) + if filepath.IsAbs(clean) || clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + return errors.Errorf("skill-repo %q: path must be relative to the repository root, got %q", repo.URL, repo.Path) + } + } + for _, name := range repo.Skills { + if !skillNameRe.MatchString(name) { + return errors.Errorf("skill-repo %q: invalid skill name %q (must match %s)", repo.URL, name, skillNameRe) + } + if prev, ok := seen[name]; ok { + return errors.Errorf("skill %q is declared by both %q and %q", name, prev, repo.URL) + } + seen[name] = repo.URL + } + } + return nil +} + +// Sync ensures every declared skill has an immutable snapshot under the state +// directory and that the environment's skill directories contain exactly the +// declared links. Fetch failures degrade to the last good snapshot with a +// warning; only configuration errors are fatal. +func Sync(l *ui.UI, stateDir, envRoot string, repos []SkillRepo) error { + if err := Validate(repos); err != nil { + return errors.WithStack(err) + } + root := filepath.Join(stateDir, stateSubdir) + desired := map[string]string{} // skill name -> snapshot dir + failed := map[string]bool{} // skills whose repo could not be synced + for _, repo := range repos { + snapshots, err := ensureSnapshots(l, root, repo) + if err != nil { + l.Warnf("skills: %s: %s", repo.URL, err) + // Leave any existing links for this repository's skills alone + // rather than tearing down a previously working set. + for _, name := range repo.Skills { + failed[name] = true + } + continue + } + for name, dir := range snapshots { + desired[name] = dir + } + } + return errors.WithStack(reconcileLinks(l, root, envRoot, desired, failed)) +} + +func hashKey(s string) string { + sum := sha256.Sum256([]byte(s)) + return hex.EncodeToString(sum[:])[:16] +} + +func snapshotDirName(name, sha string) string { + return fmt.Sprintf("%s@%s", name, sha[:12]) +} diff --git a/agentskills/agentskills_test.go b/agentskills/agentskills_test.go new file mode 100644 index 000000000..f3b32d338 --- /dev/null +++ b/agentskills/agentskills_test.go @@ -0,0 +1,284 @@ +package agentskills + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/alecthomas/assert/v2" + + "github.com/cashapp/hermit/ui" +) + +func git(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com", + "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com", + ) + out, err := cmd.CombinedOutput() + assert.NoError(t, err, "git %v: %s", args, out) + return string(out) +} + +// makeSkillRepo creates a local git repository containing skill directories +// under skills/, returning its path and HEAD commit. +func makeSkillRepo(t *testing.T, skills ...string) (repoDir, head string) { + t.Helper() + repoDir = t.TempDir() + git(t, repoDir, "init", "-q", "-b", "main", ".") + for _, name := range skills { + dir := filepath.Join(repoDir, "skills", name) + assert.NoError(t, os.MkdirAll(dir, 0700)) + assert.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("# "+name+"\n"), 0600)) + } + git(t, repoDir, "add", ".") + git(t, repoDir, "commit", "-q", "-m", "skills") + return repoDir, resolveTestHead(t, repoDir) +} + +func resolveTestHead(t *testing.T, repoDir string) string { + t.Helper() + out := git(t, repoDir, "rev-parse", "HEAD") + return out[:40] +} + +func TestValidate(t *testing.T) { + assert.NoError(t, Validate(nil)) + assert.NoError(t, Validate([]SkillRepo{{URL: "https://example.com/x.git", Skills: []string{"a-1", "b"}}})) + + assert.Error(t, Validate([]SkillRepo{{URL: "", Skills: []string{"a"}}})) + assert.Error(t, Validate([]SkillRepo{{URL: "-upload-pack=x", Skills: []string{"a"}}})) + assert.Error(t, Validate([]SkillRepo{{URL: "https://example.com/x.git"}})) + assert.Error(t, Validate([]SkillRepo{{URL: "https://example.com/x.git", Skills: []string{"Bad_Name"}}})) + assert.Error(t, Validate([]SkillRepo{{URL: "https://example.com/x.git", Skills: []string{"a"}, Ref: "abc123"}})) + assert.Error(t, Validate([]SkillRepo{{URL: "https://example.com/x.git", Skills: []string{"a"}, Path: "../escape"}})) + assert.Error(t, Validate([]SkillRepo{ + {URL: "https://example.com/x.git", Skills: []string{"a"}}, + {URL: "https://example.com/y.git", Skills: []string{"a"}}, + })) +} + +func TestSyncLinksSkills(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, head := makeSkillRepo(t, "alpha", "beta") + stateDir := t.TempDir() + envRoot := t.TempDir() + + repos := []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha", "beta"}}} + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + + for _, dir := range []string{".agents/skills", ".claude/skills"} { + for _, name := range []string{"alpha", "beta"} { + link := filepath.Join(envRoot, dir, name) + fi, err := os.Lstat(link) + assert.NoError(t, err) + assert.NotZero(t, fi.Mode()&os.ModeSymlink) + target, err := os.Readlink(link) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(stateDir, "agent-skills", "snapshots", name+"@"+head[:12]), target) + _, err = os.Stat(filepath.Join(link, "SKILL.md")) + assert.NoError(t, err) + } + } +} + +func TestSyncRemovesUndeclaredLinks(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, _ := makeSkillRepo(t, "alpha", "beta") + stateDir := t.TempDir() + envRoot := t.TempDir() + + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha", "beta"}}})) + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}})) + + _, err := os.Lstat(filepath.Join(envRoot, ".agents", "skills", "alpha")) + assert.NoError(t, err) + _, err = os.Lstat(filepath.Join(envRoot, ".agents", "skills", "beta")) + assert.Error(t, err) + _, err = os.Lstat(filepath.Join(envRoot, ".claude", "skills", "beta")) + assert.Error(t, err) +} + +func TestSyncLeavesRepoCommittedSkillsAlone(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, _ := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + + committed := filepath.Join(envRoot, ".agents", "skills", "alpha") + assert.NoError(t, os.MkdirAll(committed, 0700)) + assert.NoError(t, os.WriteFile(filepath.Join(committed, "SKILL.md"), []byte("local"), 0600)) + + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}})) + + fi, err := os.Lstat(committed) + assert.NoError(t, err) + assert.True(t, fi.IsDir()) + data, err := os.ReadFile(filepath.Join(committed, "SKILL.md")) + assert.NoError(t, err) + assert.Equal(t, "local", string(data)) +} + +func TestSyncLeavesForeignSymlinksAlone(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, _ := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + + other := t.TempDir() + link := filepath.Join(envRoot, ".agents", "skills", "alpha") + assert.NoError(t, os.MkdirAll(filepath.Dir(link), 0700)) + assert.NoError(t, os.Symlink(other, link)) + + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}})) + + target, err := os.Readlink(link) + assert.NoError(t, err) + assert.Equal(t, other, target) +} + +func TestSyncPinnedRef(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, head := makeSkillRepo(t, "alpha") + + // Advance the repo past the pin. + assert.NoError(t, os.WriteFile(filepath.Join(repoDir, "skills", "alpha", "extra.md"), []byte("new"), 0600)) + git(t, repoDir, "add", ".") + git(t, repoDir, "commit", "-q", "-m", "update") + + stateDir := t.TempDir() + envRoot := t.TempDir() + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}, Ref: head}})) + + link := filepath.Join(envRoot, ".agents", "skills", "alpha") + target, err := os.Readlink(link) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(stateDir, "agent-skills", "snapshots", "alpha@"+head[:12]), target) + _, err = os.Stat(filepath.Join(link, "extra.md")) + assert.Error(t, err) +} + +func TestSyncUpdatesToNewHead(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, oldHead := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + repos := []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}} + + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + + assert.NoError(t, os.WriteFile(filepath.Join(repoDir, "skills", "alpha", "extra.md"), []byte("new"), 0600)) + git(t, repoDir, "add", ".") + git(t, repoDir, "commit", "-q", "-m", "update") + newHead := resolveTestHead(t, repoDir) + assert.NotEqual(t, oldHead, newHead) + + // Expire the freshness stamp so the new HEAD is picked up. + stampPath := filepath.Join(stateDir, "agent-skills", "refs", hashKey(repoDir)+".json") + writeStamp(stampPath, &refStamp{URL: repoDir, SHA: oldHead, CheckedAt: time.Now().Add(-time.Hour)}) + + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + + target, err := os.Readlink(filepath.Join(envRoot, ".agents", "skills", "alpha")) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(stateDir, "agent-skills", "snapshots", "alpha@"+newHead[:12]), target) + _, err = os.Stat(filepath.Join(envRoot, ".agents", "skills", "alpha", "extra.md")) + assert.NoError(t, err) +} + +func TestSyncFreshnessWindowSkipsRemoteCheck(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, head := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + repos := []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}} + + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + + // Advance the repo; within the freshness window Sync must keep the + // stamped revision. + assert.NoError(t, os.WriteFile(filepath.Join(repoDir, "skills", "alpha", "extra.md"), []byte("new"), 0600)) + git(t, repoDir, "add", ".") + git(t, repoDir, "commit", "-q", "-m", "update") + + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + target, err := os.Readlink(filepath.Join(envRoot, ".agents", "skills", "alpha")) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(stateDir, "agent-skills", "snapshots", "alpha@"+head[:12]), target) +} + +func TestSyncOfflineFallsBackToLastGood(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, head := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + repos := []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}} + + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + + // Simulate the remote disappearing with an expired stamp: resolution + // fails but the last good snapshot keeps working. + assert.NoError(t, os.RemoveAll(filepath.Join(repoDir, ".git"))) + stampPath := filepath.Join(stateDir, "agent-skills", "refs", hashKey(repoDir)+".json") + writeStamp(stampPath, &refStamp{URL: repoDir, SHA: head, CheckedAt: time.Now().Add(-time.Hour)}) + + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + target, err := os.Readlink(filepath.Join(envRoot, ".agents", "skills", "alpha")) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(stateDir, "agent-skills", "snapshots", "alpha@"+head[:12]), target) +} + +func TestSyncEmptyConfigCleansUp(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, _ := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}})) + assert.NoError(t, Sync(l, stateDir, envRoot, nil)) + + _, err := os.Lstat(filepath.Join(envRoot, ".agents", "skills", "alpha")) + assert.Error(t, err) + _, err = os.Lstat(filepath.Join(envRoot, ".claude", "skills", "alpha")) + assert.Error(t, err) +} + +func TestSyncMissingSkillWarnsButLinksRest(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, _ := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + + // "ghost" is declared but does not exist in the repository: the repo + // fails with a warning and contributes nothing, without failing Sync. + repos := []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha", "ghost"}}} + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + + _, err := os.Lstat(filepath.Join(envRoot, ".agents", "skills", "alpha")) + assert.Error(t, err) + _, err = os.Lstat(filepath.Join(envRoot, ".agents", "skills", "ghost")) + assert.Error(t, err) +} + +func TestSyncFailedRepoPreservesExistingLinks(t *testing.T) { + l, _ := ui.NewForTesting() + repoDir, _ := makeSkillRepo(t, "alpha") + stateDir := t.TempDir() + envRoot := t.TempDir() + + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha"}}})) + + // Declaring a skill the repository does not contain makes the whole + // repository fail to sync; the previously linked skill must survive. + assert.NoError(t, Sync(l, stateDir, envRoot, []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"alpha", "ghost"}}})) + + _, err := os.Lstat(filepath.Join(envRoot, ".agents", "skills", "alpha")) + assert.NoError(t, err) + _, err = os.Lstat(filepath.Join(envRoot, ".claude", "skills", "alpha")) + assert.NoError(t, err) +} diff --git a/agentskills/git.go b/agentskills/git.go new file mode 100644 index 000000000..3285df43d --- /dev/null +++ b/agentskills/git.go @@ -0,0 +1,277 @@ +package agentskills + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/otiai10/copy" + + "github.com/cashapp/hermit/errors" + "github.com/cashapp/hermit/ui" + "github.com/cashapp/hermit/util/flock" +) + +// refStamp records the last successfully resolved remote HEAD for a +// repository URL, so activation only pays for a network round trip when the +// freshness window has lapsed, and offline activation can fall back to the +// last good resolution. +type refStamp struct { + URL string `json:"url"` + SHA string `json:"sha"` + CheckedAt time.Time `json:"checked_at"` +} + +// ensureSnapshots resolves the repository to a commit and materialises a +// snapshot for each declared skill, returning skill name -> snapshot dir. +func ensureSnapshots(l *ui.UI, root string, repo SkillRepo) (map[string]string, error) { + sha := repo.Ref + if sha == "" { + var err error + sha, err = resolveHead(l, root, repo.URL) + if err != nil { + return nil, err + } + } + + snapshotsDir := filepath.Join(root, "snapshots") + result := map[string]string{} + var missing []string + for _, name := range repo.Skills { + dir := filepath.Join(snapshotsDir, snapshotDirName(name, sha)) + result[name] = dir + if _, err := os.Stat(dir); err != nil { + missing = append(missing, name) + } + } + if len(missing) == 0 { + return result, nil + } + + release, err := lockState(root) + if err != nil { + return nil, err + } + defer release() //nolint:errcheck + + // Re-check under the lock: another activation may have materialised the + // snapshots while we waited. + missing = missing[:0] + for _, name := range repo.Skills { + if _, err := os.Stat(result[name]); err != nil { + missing = append(missing, name) + } + } + if len(missing) == 0 { + return result, nil + } + + task := l.Task("skills") + defer task.Done() + checkout, cleanup, err := fetchCommit(task, root, repo.URL, sha) + if err != nil { + return nil, err + } + defer cleanup() + + for _, name := range missing { + src := filepath.Join(checkout, filepath.FromSlash(repo.Path), name) + if fi, err := os.Stat(src); err != nil || !fi.IsDir() { + return nil, errors.Errorf("skill %q not found at %s in %s@%s", name, filepath.Join(repo.Path, name), repo.URL, sha[:12]) + } + if _, err := os.Stat(filepath.Join(src, "SKILL.md")); err != nil { + return nil, errors.Errorf("skill %q in %s@%s has no SKILL.md", name, repo.URL, sha[:12]) + } + if err := snapshot(src, snapshotsDir, snapshotDirName(name, sha)); err != nil { + return nil, errors.Wrapf(err, "snapshotting skill %q", name) + } + task.Infof("Installed agent skill %s@%s", name, sha[:12]) + } + return result, nil +} + +// resolveHead returns the commit SHA of the remote HEAD, consulting the +// freshness stamp first and falling back to it when the remote is +// unreachable. +func resolveHead(l *ui.UI, root, url string) (string, error) { + stampPath := filepath.Join(root, "refs", hashKey(url)+".json") + stamp, _ := readStamp(stampPath) + if stamp != nil && time.Since(stamp.CheckedAt) < freshnessWindow { + return stamp.SHA, nil + } + + sha, err := lsRemoteHead(url) + if err != nil { + if stamp != nil { + l.Warnf("skills: could not check %s for updates, using last known revision %s: %s", url, stamp.SHA[:12], err) + return stamp.SHA, nil + } + return "", errors.Wrapf(err, "could not resolve HEAD (offline and no previous snapshot?)") + } + writeStamp(stampPath, &refStamp{URL: url, SHA: sha, CheckedAt: time.Now()}) + return sha, nil +} + +func lsRemoteHead(url string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), resolveTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, "git", "ls-remote", "--", url, "HEAD") + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + out, err := cmd.Output() + if err != nil { + return "", errors.Wrapf(err, "git ls-remote %s", url) + } + fields := strings.Fields(string(out)) + if len(fields) < 1 || !commitSHARe.MatchString(fields[0]) { + return "", errors.Errorf("unexpected ls-remote output from %s: %q", url, string(out)) + } + return fields[0], nil +} + +// fetchCommit produces a working checkout of the given commit in a transient +// directory. It first attempts a direct shallow fetch of the commit (works on +// GitHub and any server with allow-any-sha1-in-want) and falls back to a +// shallow clone of the default branch. +func fetchCommit(task *ui.Task, root, url, sha string) (dir string, cleanup func(), err error) { + tmpRoot := filepath.Join(root, "tmp") + if err := os.MkdirAll(tmpRoot, 0700); err != nil { + return "", nil, errors.WithStack(err) + } + tmp, err := os.MkdirTemp(tmpRoot, "checkout-*") + if err != nil { + return "", nil, errors.WithStack(err) + } + cleanup = func() { _ = os.RemoveAll(tmp) } + + err = runGit(task, tmp, "init", "--quiet", ".") + if err == nil { + err = runGit(task, tmp, "fetch", "--quiet", "--depth=1", "--", url, sha) + if err == nil { + err = runGit(task, tmp, "checkout", "--quiet", "--detach", "FETCH_HEAD") + } + } + if err == nil { + return tmp, cleanup, nil + } + + // Fallback for servers that refuse fetching arbitrary SHAs. + _ = os.RemoveAll(tmp) + if err := os.MkdirAll(tmp, 0700); err != nil { + return "", nil, errors.WithStack(err) + } + if err := runGit(task, tmpRoot, "clone", "--quiet", "--depth=1", "--", url, tmp); err != nil { + cleanup() + return "", nil, errors.Wrapf(err, "fetching %s", url) + } + out, err := gitOutput(tmp, "rev-parse", "HEAD") + if err != nil { + cleanup() + return "", nil, err + } + if head := strings.TrimSpace(out); head != sha { + cleanup() + return "", nil, errors.Errorf("%s: default branch moved to %s while resolving %s and the server does not support fetching commits directly", url, head[:12], sha[:12]) + } + return tmp, cleanup, nil +} + +func runGit(task *ui.Task, dir string, args ...string) error { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + out, err := cmd.CombinedOutput() + if err != nil { + return errors.Wrapf(err, "git %s: %s", strings.Join(args, " "), strings.TrimSpace(string(out))) + } + _, _ = task.Write(out) + return nil +} + +func gitOutput(dir string, args ...string) (string, error) { + cmd := exec.Command("git", args...) //nolint:noctx + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + return "", errors.Wrapf(err, "git %s", strings.Join(args, " ")) + } + return string(out), nil +} + +// snapshot copies the skill tree into the snapshots directory via a temporary +// directory and atomic rename, so a partially written snapshot is never +// observable at its final path. +func snapshot(src, snapshotsDir, name string) error { + if err := os.MkdirAll(snapshotsDir, 0700); err != nil { + return errors.WithStack(err) + } + tmp, err := os.MkdirTemp(snapshotsDir, ".tmp-"+name+"-*") + if err != nil { + return errors.WithStack(err) + } + defer os.RemoveAll(tmp) + // Skip symlinks: a skill snapshot must be self-contained and a + // repository symlink could point anywhere. + err = copy.Copy(src, tmp, copy.Options{ + OnSymlink: func(string) copy.SymlinkAction { return copy.Skip }, + }) + if err != nil { + return errors.WithStack(err) + } + dest := filepath.Join(snapshotsDir, name) + if err := os.Rename(tmp, dest); err != nil { + if os.IsExist(err) || errors.Is(err, os.ErrExist) { + return nil // Lost a benign race with another process. + } + return errors.WithStack(err) + } + return nil +} + +func lockState(root string) (release func() error, err error) { + if err := os.MkdirAll(root, 0700); err != nil { + return nil, errors.WithStack(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + release, err = flock.Acquire(ctx, filepath.Join(root, ".lock"), "materialising agent skills") + cancel() + if err != nil { + return nil, errors.WithStack(err) + } + return release, nil +} + +func readStamp(path string) (*refStamp, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, errors.WithStack(err) + } + stamp := &refStamp{} + if err := json.Unmarshal(data, stamp); err != nil { + return nil, errors.WithStack(err) + } + if !commitSHARe.MatchString(stamp.SHA) { + return nil, errors.Errorf("corrupt ref stamp %s", path) + } + return stamp, nil +} + +func writeStamp(path string, stamp *refStamp) { + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + return + } + data, err := json.Marshal(stamp) + if err != nil { + return + } + tmp := path + ".tmp" + if err := os.WriteFile(tmp, data, 0600); err != nil { + return + } + _ = os.Rename(tmp, path) +} diff --git a/agentskills/link.go b/agentskills/link.go new file mode 100644 index 000000000..ad6835bdd --- /dev/null +++ b/agentskills/link.go @@ -0,0 +1,176 @@ +package agentskills + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + + "github.com/cashapp/hermit/errors" + "github.com/cashapp/hermit/ui" +) + +// ledger records the symlinks this machine created for an environment, so +// reconciliation can safely remove links for skills that are no longer +// declared without ever touching nodes it does not own. +type ledger struct { + EnvRoot string `json:"env_root"` + Links []ledgerEntry `json:"links"` +} + +type ledgerEntry struct { + // Link is the symlink path relative to the environment root. + Link string `json:"link"` + // Target is the absolute snapshot path the link pointed at. + Target string `json:"target"` +} + +// reconcileLinks makes the environment's skill directories match the desired +// skill set: creating missing links, retargeting stale ones we own, and +// removing links for undeclared skills. Pre-existing nodes not created by us +// (e.g. a repo-committed skill directory) always win and are left untouched. +// Skills in failed are still declared but could not be synced; their existing +// links are preserved as-is. +func reconcileLinks(l *ui.UI, root, envRoot string, desired map[string]string, failed map[string]bool) error { + ledgerPath := filepath.Join(root, "ledgers", hashKey(envRoot)+".json") + previous := readLedger(ledgerPath) + snapshotsDir := filepath.Join(root, "snapshots") + + owned := map[string]string{} + for _, entry := range previous.Links { + owned[entry.Link] = entry.Target + } + + next := &ledger{EnvRoot: envRoot} + created := false + for _, dir := range linkDirs { + for name, target := range desired { + rel := filepath.Join(dir, name) + link := filepath.Join(envRoot, rel) + madeNew, err := ensureLink(l, link, rel, target, snapshotsDir, owned[rel]) + if err != nil { + return err + } + created = created || madeNew + next.Links = append(next.Links, ledgerEntry{Link: rel, Target: target}) + } + } + + // Remove links we created for skills that are no longer declared. + for _, entry := range previous.Links { + name := filepath.Base(entry.Link) + if _, still := desired[name]; still { + continue + } + if failed[name] { + // Still declared, just unsyncable right now: keep both the link + // and its ledger record. + next.Links = append(next.Links, entry) + continue + } + link := filepath.Join(envRoot, entry.Link) + fi, err := os.Lstat(link) + if err != nil || fi.Mode()&os.ModeSymlink == 0 { + continue + } + dest, err := os.Readlink(link) + if err != nil || (dest != entry.Target && !within(dest, snapshotsDir)) { + continue + } + if err := os.Remove(link); err != nil { + l.Warnf("skills: could not remove stale link %s: %s", link, err) + } + } + + if len(next.Links) == 0 && len(previous.Links) == 0 { + return nil + } + sort.Slice(next.Links, func(i, j int) bool { return next.Links[i].Link < next.Links[j].Link }) + writeLedger(ledgerPath, next) + if created { + warnIfNotIgnored(l, envRoot) + } + return nil +} + +// ensureLink points link at target, creating it atomically. It only replaces +// an existing node when it is a symlink we own: one recorded in the ledger or +// pointing into the snapshots directory. +func ensureLink(l *ui.UI, link, rel, target, snapshotsDir, ownedTarget string) (created bool, err error) { + fi, lerr := os.Lstat(link) + switch { + case lerr == nil && fi.Mode()&os.ModeSymlink != 0: + dest, err := os.Readlink(link) + if err == nil && dest == target { + return false, nil + } + if dest != ownedTarget && !within(dest, snapshotsDir) { + l.Warnf("skills: %s is a symlink not managed by Hermit, leaving it alone", rel) + return false, nil + } + case lerr == nil: + // A real file or directory: the repository provides this skill + // itself and it takes precedence. + l.Debugf("skills: %s exists in the repository, leaving it alone", rel) + return false, nil + } + + if err := os.MkdirAll(filepath.Dir(link), 0750); err != nil { + return false, errors.WithStack(err) + } + tmp := link + ".hermit-tmp" + _ = os.Remove(tmp) + if err := os.Symlink(target, tmp); err != nil { + return false, errors.WithStack(err) + } + if err := os.Rename(tmp, link); err != nil { + _ = os.Remove(tmp) + return false, errors.WithStack(err) + } + return lerr != nil, nil +} + +func within(path, dir string) bool { + rel, err := filepath.Rel(dir, path) + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + +// warnIfNotIgnored nudges the user to gitignore the skill link directories. +// The links contain machine-local absolute paths and must not be committed. +func warnIfNotIgnored(l *ui.UI, envRoot string) { + if _, err := os.Stat(filepath.Join(envRoot, ".git")); err != nil { + return + } + probe := filepath.Join(linkDirs[0], "probe") + cmd := exec.Command("git", "-C", envRoot, "check-ignore", "-q", "--", probe) //nolint:noctx + if err := cmd.Run(); err != nil { + l.Warnf("skills: add %q and %q to .gitignore — Hermit-managed skill links must not be committed", linkDirs[0]+string(filepath.Separator), linkDirs[1]+string(filepath.Separator)) + } +} + +func readLedger(path string) *ledger { + led := &ledger{} + data, err := os.ReadFile(path) + if err != nil { + return led + } + _ = json.Unmarshal(data, led) + return led +} + +func writeLedger(path string, led *ledger) { + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + return + } + data, err := json.MarshalIndent(led, "", " ") + if err != nil { + return + } + tmp := path + ".tmp" + if err := os.WriteFile(tmp, data, 0600); err != nil { + return + } + _ = os.Rename(tmp, path) +} diff --git a/app/activate_cmd.go b/app/activate_cmd.go index b7738607b..099faea2f 100644 --- a/app/activate_cmd.go +++ b/app/activate_cmd.go @@ -38,6 +38,10 @@ func (a *activateCmd) Run(l *ui.UI, cache *cache.Cache, sta *state.State, global if err := env.EnsureInstalled(l); err != nil { return errors.WithStack(err) } + if err := env.EnsureSkills(l); err != nil { + // Skills are advisory content; never block activation on them. + l.Warnf("skills: %s", err) + } messages, err := env.Trigger(l, manifest.EventEnvActivate) if err != nil { return errors.WithStack(err) diff --git a/app/env_cmd.go b/app/env_cmd.go index b2bc97d8d..8f7d2d955 100644 --- a/app/env_cmd.go +++ b/app/env_cmd.go @@ -65,6 +65,10 @@ func (e *envCmd) Run(l *ui.UI, env *hermit.Env) error { switch { case e.Activate: + if err := env.EnsureSkills(l); err != nil { + // Skills are advisory content; never block activation on them. + l.Warnf("skills: %s", err) + } environ := envars.Parse(os.Environ()).Apply(env.Root(), ops).Changed(true) return errors.WithStack(sh.ApplyEnvars(os.Stdout, environ)) diff --git a/docs/docs/usage/config.md b/docs/docs/usage/config.md index d45b568ec..64f3e64c9 100644 --- a/docs/docs/usage/config.md +++ b/docs/docs/usage/config.md @@ -39,6 +39,20 @@ github-token-auth { // A list of globs to match against GitHub repositories. match = ["ORG/REPO", "ORG/*"] } + +// Git repositories providing agent skills to link into the environment +// on activation. Skills are resolved to immutable snapshots under the +// Hermit state directory and symlinked into `.agents/skills/` and +// `.claude/skills/`. Add both directories to .gitignore. +skill-repo "https://github.com/ORG/REPO.git" { + // Subdirectory within the repository containing the skill directories. + path = "skills" + // Names of the skill directories to link into the environment. + skills = ["SKILL"] + // Optional full commit SHA to pin to. When omitted the remote HEAD is + // used, re-checked at most every 15 minutes. + // ref = "0123456789abcdef0123456789abcdef01234567" +} ``` ## Attributes @@ -51,6 +65,7 @@ github-token-auth { | `inherit-parent` | `bool?` | Whether this Hermit environment should inherit an environment from a parent directory. | | `github-token-auth` | `GitHubTokenAuthConfig?` | When to use GitHub token authentication. | | `idea` | `bool?` | Whether Hermit should automatically add the IntelliJ IDEA plugin. | +| `skill-repo` | `[SkillRepo]?` | Git repositories providing agent skills to link into the environment on activation. | ### GitHubTokenAuthConfig @@ -58,6 +73,32 @@ github-token-auth { |-----------|----------|-----------------------------------------------------------------------------------------------------------------------| | match | `[string]?` | One or more glob patterns. If any of these match the 'owner/repo' pair of a GitHub repository, the GitHub token from the current environment will be used to fetch their artifacts. | +### SkillRepo + +| Attribute | Type | Description | +|-----------|-------------|---------------------------------------------------------------------------------------------------------| +| `url` | `string` | Git repository URL providing agent skills (block label). | +| `path` | `string?` | Subdirectory within the repository containing the skill directories. | +| `skills` | `[string]` | Names of the skill directories to link into the environment. Each must contain a `SKILL.md`. | +| `ref` | `string?` | Full commit SHA to pin to. When omitted the remote HEAD is used, re-checked at most every 15 minutes. | + +## Agent Skills + +Environments can declare agent skills — `SKILL.md` directories used by AI +coding agents — that Hermit links into the project on activation, the same +way it provides toolchains. +On activation Hermit resolves each `skill-repo` to a commit, materialises an +immutable snapshot of each declared skill under the Hermit state directory, +and symlinks it into `.agents/skills/` and `.claude/skills/`. + +Skill content is never written into the project itself and must not be +committed: add `.agents/skills/` and `.claude/skills/` to `.gitignore`. +A skill directory committed directly to the repository always takes +precedence over a Hermit-managed one with the same name. + +When offline, activation falls back to the last good snapshot. Removing a +skill from the configuration removes its links on the next activation. + ## Per-environment Sources Hermit supports three different manifest sources: diff --git a/env.go b/env.go index f8317f952..5f01b9cd7 100644 --- a/env.go +++ b/env.go @@ -23,6 +23,7 @@ import ( "github.com/alecthomas/hcl" "github.com/kballard/go-shellquote" + "github.com/cashapp/hermit/agentskills" "github.com/cashapp/hermit/cache" "github.com/cashapp/hermit/envars" "github.com/cashapp/hermit/errors" @@ -98,6 +99,8 @@ type Config struct { AddIJPlugin bool `hcl:"idea,optional" default:"false" help:"Whether Hermit should automatically add the IntelliJ IDEA plugin."` GitHubTokenAuth GitHubTokenAuthConfig `hcl:"github-token-auth,block" help:"When to use GitHub token authentication."` + + SkillRepos []agentskills.SkillRepo `hcl:"skill-repo,block,optional" help:"Git repositories providing agent skills to link into the environment on activation."` } // GitHubTokenAuthConfig configures under what conditions @@ -471,6 +474,14 @@ func (e *Env) EnsureInstalled(l *ui.UI) error { return nil } +// EnsureSkills materialises the agent skills declared in the environment +// configuration and links them into the environment's skill directories. +func (e *Env) EnsureSkills(l *ui.UI) error { + // Called even with no skills declared so that links created by a + // previous configuration are cleaned up. + return errors.WithStack(agentskills.Sync(l, e.state.Root(), e.envDir, e.config.SkillRepos)) +} + // Trigger an event for all installed packages. func (e *Env) Trigger(l *ui.UI, event manifest.Event) (messages []string, err error) { pkgs, err := e.ListInstalled(l) From 6bc8ff5e006cfbd9f15c4eb42952eebc1ff664b5 Mon Sep 17 00:00:00 2001 From: Joah Gerstenberg Date: Tue, 4 Aug 2026 15:03:54 -0500 Subject: [PATCH 2/2] fix: reject symlinked skill paths that escape the checkout The skill source path is repository-controlled and was checked with os.Stat, which follows symlinks: a repository could commit its skills directory (or a skill entry) as a symlink to a local path outside the checkout, and the snapshot would materialise that local content. The OnSymlink skip policy only covers symlinks inside the copied tree, not a symlinked source path. Resolve the selected skill path and require it to remain within the checkout before snapshotting. Buzz-Message: buzz://message?channel=573ff355-6c19-422d-b425-112853d0ec7e&id=b5be8f34bb7e24316b8be6171fcb3089eeb8f9c8e0240cd7e0d5441cb503c8a7 Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-019fce27-e59e-727c-b6d3-cdeeba894a4e --- agentskills/agentskills_test.go | 30 ++++++++++++++++++++++++++++++ agentskills/git.go | 19 ++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/agentskills/agentskills_test.go b/agentskills/agentskills_test.go index f3b32d338..1dd120bf1 100644 --- a/agentskills/agentskills_test.go +++ b/agentskills/agentskills_test.go @@ -265,6 +265,36 @@ func TestSyncMissingSkillWarnsButLinksRest(t *testing.T) { assert.Error(t, err) } +func TestSyncRejectsSymlinkedSkillPathEscapingCheckout(t *testing.T) { + l, _ := ui.NewForTesting() + stateDir := t.TempDir() + envRoot := t.TempDir() + + // The victim directory exists locally, outside any checkout. + outside := t.TempDir() + assert.NoError(t, os.WriteFile(filepath.Join(outside, "SKILL.md"), []byte("secret"), 0600)) + + // The repository commits skills/evil as a symlink to that directory. + repoDir := t.TempDir() + git(t, repoDir, "init", "-q", "-b", "main", ".") + assert.NoError(t, os.MkdirAll(filepath.Join(repoDir, "skills"), 0700)) + assert.NoError(t, os.Symlink(outside, filepath.Join(repoDir, "skills", "evil"))) + git(t, repoDir, "add", ".") + git(t, repoDir, "commit", "-q", "-m", "evil") + head := resolveTestHead(t, repoDir) + + repos := []SkillRepo{{URL: repoDir, Path: "skills", Skills: []string{"evil"}}} + assert.NoError(t, Sync(l, stateDir, envRoot, repos)) + + // The outside content must be neither snapshotted nor linked. + _, err := os.Lstat(filepath.Join(stateDir, "agent-skills", "snapshots", "evil@"+head[:12])) + assert.Error(t, err) + _, err = os.Lstat(filepath.Join(envRoot, ".agents", "skills", "evil")) + assert.Error(t, err) + _, err = os.Lstat(filepath.Join(envRoot, ".claude", "skills", "evil")) + assert.Error(t, err) +} + func TestSyncFailedRepoPreservesExistingLinks(t *testing.T) { l, _ := ui.NewForTesting() repoDir, _ := makeSkillRepo(t, "alpha") diff --git a/agentskills/git.go b/agentskills/git.go index 3285df43d..c727d0cf8 100644 --- a/agentskills/git.go +++ b/agentskills/git.go @@ -78,8 +78,25 @@ func ensureSnapshots(l *ui.UI, root string, repo SkillRepo) (map[string]string, } defer cleanup() + // The checkout path itself may traverse symlinks (e.g. macOS /var), so + // resolve it once as the containment root for the skill paths below. + resolvedCheckout, err := filepath.EvalSymlinks(checkout) + if err != nil { + return nil, errors.WithStack(err) + } + for _, name := range missing { - src := filepath.Join(checkout, filepath.FromSlash(repo.Path), name) + // Resolve symlinks in the repository-controlled skill path and + // require the result to remain within the checkout: a committed + // symlink could otherwise select local content outside it for the + // snapshot. + src, err := filepath.EvalSymlinks(filepath.Join(checkout, filepath.FromSlash(repo.Path), name)) + if err != nil { + return nil, errors.Errorf("skill %q not found at %s in %s@%s", name, filepath.Join(repo.Path, name), repo.URL, sha[:12]) + } + if rel, err := filepath.Rel(resolvedCheckout, src); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return nil, errors.Errorf("skill %q at %s in %s@%s resolves outside the repository checkout", name, filepath.Join(repo.Path, name), repo.URL, sha[:12]) + } if fi, err := os.Stat(src); err != nil || !fi.IsDir() { return nil, errors.Errorf("skill %q not found at %s in %s@%s", name, filepath.Join(repo.Path, name), repo.URL, sha[:12]) }