diff --git a/.gitignore b/.gitignore index bde3c5347d..25e535954a 100644 --- a/.gitignore +++ b/.gitignore @@ -29,3 +29,4 @@ assets/ CLAUDE.md PLAN-*.md .pi/ +.specs/ diff --git a/build.gradle b/build.gradle index 36c9222376..43f96073e3 100644 --- a/build.gradle +++ b/build.gradle @@ -237,7 +237,7 @@ spotless { lineEndings 'UNIX' format 'misc', { target '**/*.gradle', '**/*.md', '**/.gitignore' - targetExclude 'CONTRIBUTORS.md', 'src/main/scripts/container/README.md', 'build/**/*', 'out/**/*' + targetExclude 'CONTRIBUTORS.md', 'src/main/scripts/container/README.md', 'build/**/*', 'out/**/*', '.specs/**/*' // all-contributor bot adds non-indented code trimTrailingWhitespace() leadingSpacesToTabs(4) diff --git a/docs/modules/ROOT/pages/agent-skills.adoc b/docs/modules/ROOT/pages/agent-skills.adoc new file mode 100644 index 0000000000..bef1adcdac --- /dev/null +++ b/docs/modules/ROOT/pages/agent-skills.adoc @@ -0,0 +1,116 @@ += Agent Skills + +JBang ships a set of https://agentskills.io[Agent Skills] - instructions that teach AI coding agents (such as Claude Code, Codex, OpenCode and others) how to install and drive JBang. The skills are maintained in the https://github.com/jbangdev/agent-skills[jbangdev/agent-skills] repository. + +The `jbang skill` command installs these skills directly into the agents on your machine, with no extra tooling required. + +The skills follow the common https://agentskills.io[Agent Skills] layout, so you can also install them with a generic skills installer instead of JBang, for example: + +[source, bash] +---- +npx skills add jbangdev/agent-skills +---- + +== Where skills are installed + +[cols="1,2"] +|=== +|Agent |Skills directory + +|Claude Code +|`/.claude/skills//` + +|Codex / OpenCode / Pi / ... (shared convention) +|`/.agents/skills//` +|=== + +`` is your user home by default (per-account), or the current directory when you pass `--project` (per-repository). In the per-account default an agent is considered *detected* when its directory (for example `.claude`) already exists under ``; undetected agents are skipped. With `--project` both locations are always targeted (and created as needed), since you are explicitly opting the repository in. + +Use `--agent` to limit the command to specific target(s) - `claude` or `agents` (repeatable or comma-separated): + +[source, bash] +---- +jbang skill install --agent claude +jbang skill install --agent claude,agents +---- + +`--repo` points at a different source - a GitHub `owner/repo` (optionally with `@ref` for a branch, tag or commit) or a full tarball URL. It defaults to `jbangdev/agent-skills`: + +[source, bash] +---- +jbang skill install --repo myorg/my-skills +jbang skill install --repo myorg/my-skills@dev +jbang skill list --repo https://example.com/skills.tar.gz +---- + +NOTE: `--repo` is mainly intended for testing changes to a skills repository (for example a fork or a feature branch). It installs *all* skills found in that repository, which may not be what you want - for everyday use prefer the default repository and name the specific skills you want. + +== Installing skills + +[source, bash] +---- +jbang skill install # all skills, into detected agents in your home +jbang skill install jbang # a single skill +jbang skill install jbang jbang-scripting +jbang skill install --project # into the current repository instead +---- + +For each skill and detected agent, install does one of: + +* installs it if missing, +* leaves it untouched if already up to date, +* refuses to overwrite a skill that was *modified* locally or a directory that is *not* a JBang skill - pass `--force` to replace it: + +[source, bash] +---- +jbang skill install --force jbang +---- + +== Listing skills + +[source, bash] +---- +jbang skill list +jbang skill list --project +---- + +For each skill in the repository, lists every agent location that is checked, its status, and the exact path: + +---- +jbang + Claude Code installed (up to date) ~/.claude/skills/jbang + Agents (Codex/OpenCode/Pi/...) not installed ~/.agents/skills/jbang +---- + +Status is one of: + +[cols="1,3"] +|=== +|Status |Meaning + +|`installed (up to date)` +|Installed and identical to the current repository version + +|`installed (modified/outdated)` +|Installed but edited locally or from an older version (re-run `install --force` to refresh) + +|`not installed` +|Agent detected, but the skill is absent + +|`conflict (not a JBang skill)` +|A directory with that name exists but was not installed by JBang + +|`agent not detected` +|The agent's directory does not exist under the current base +|=== + +== Uninstalling skills + +[source, bash] +---- +jbang skill uninstall # remove all JBang-installed skills +jbang skill uninstall jbang # remove a specific skill +jbang skill uninstall --project # from the current repository +---- + +Every skill JBang installs carries a small `.jbang-skill` marker file. Uninstall uses that marker, so it works offline (no download) and only ever removes skills that JBang installed - a skill of the same name that you added by other means is left untouched. With no name given, all marker-bearing skills in the selected scope are removed. diff --git a/docs/modules/ROOT/partials/nav.adoc b/docs/modules/ROOT/partials/nav.adoc index 719983b6e1..55378d1471 100644 --- a/docs/modules/ROOT/partials/nav.adoc +++ b/docs/modules/ROOT/partials/nav.adoc @@ -26,6 +26,7 @@ ** xref:jbang:ROOT:publishing-app-catalogs.adoc[Publishing App Catalogs] ** xref:jbang:ROOT:alias_catalogs.adoc[Aliases & Catalogs] ** xref:jbang:ROOT:integration.adoc[Build Integration] +** xref:jbang:ROOT:agent-skills.adoc[Agent Skills] * Configuration & Management ** xref:jbang:ROOT:configuration.adoc[Configuration] diff --git a/src/main/java/dev/jbang/Main.java b/src/main/java/dev/jbang/Main.java index 8cacf26a46..cbcb8603e0 100644 --- a/src/main/java/dev/jbang/Main.java +++ b/src/main/java/dev/jbang/Main.java @@ -126,6 +126,7 @@ public static Set getSubcommandNames() { names.add("export"); names.add("config"); names.add("deps"); + names.add("skill"); subcommandNames = names; } return subcommandNames; diff --git a/src/main/java/dev/jbang/cli/JBang.java b/src/main/java/dev/jbang/cli/JBang.java index e8d32e879f..c2c2be4db3 100644 --- a/src/main/java/dev/jbang/cli/JBang.java +++ b/src/main/java/dev/jbang/cli/JBang.java @@ -21,7 +21,7 @@ Run.class, Build.class, Init.class, Edit.class, Deps.class, Cache.class, Export.class, Jdk.class, Config.class, Trust.class, Alias.class, Template.class, Catalog.class, App.class, - Completion.class, Info.class, Version.class, + Skill.class, Completion.class, Info.class, Version.class, Wrapper.class }, generateHelp = true, helpSectionProvider = ExternalCommandsProvider.class) public class JBang extends BaseCommand { diff --git a/src/main/java/dev/jbang/cli/Skill.java b/src/main/java/dev/jbang/cli/Skill.java new file mode 100644 index 0000000000..37d5a049df --- /dev/null +++ b/src/main/java/dev/jbang/cli/Skill.java @@ -0,0 +1,368 @@ +package dev.jbang.cli; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Stream; + +import org.aesh.command.CommandDefinition; +import org.aesh.command.option.Arguments; +import org.aesh.command.option.Option; +import org.aesh.command.option.OptionList; + +import dev.jbang.ExitException; +import dev.jbang.util.NetUtil; +import dev.jbang.util.UnpackUtil; +import dev.jbang.util.Util; + +@CommandDefinition(name = "skill", description = "Install JBang agent skills (AI coding agent instructions).", groupCommands = { + Skill.SkillInstall.class, Skill.SkillUninstall.class, + Skill.SkillList.class }, generateHelp = true, helpGroup = "Configuration", helpSectionProvider = SkillHelpProvider.class) +public class Skill extends BaseCommand { + + static final String DEFAULT_REPO = "jbangdev/agent-skills"; + static final String DEFAULT_REF = "main"; + + /** Provenance marker JBang writes into every skill it installs. */ + static final String MARKER = ".jbang-skill"; + + /** + * The distinct skill locations we manage. Codex/OpenCode/Pi share a single + * directory, so we expose two targets, not one per tool. Add per-tool entries + * here if/when an agent needs its own location or settings. + */ + enum Agent { + CLAUDE("claude", "Claude Code", ".claude"), + AGENTS("agents", "Agents (Codex/OpenCode/Pi/...)", ".agents"); + + final String token; + final String label; + final String dir; + + Agent(String token, String label, String dir) { + this.token = token; + this.label = label; + this.dir = dir; + } + + static Agent fromToken(String token) { + for (Agent a : values()) { + if (a.token.equalsIgnoreCase(token)) { + return a; + } + } + throw new ExitException(ExitException.EXIT_INVALID_INPUT, + "Unknown agent '" + token + "'. Available: claude, agents"); + } + } + + /** Status of one skill in one agent location. */ + enum Status { + CURRENT("installed (up to date)"), + MODIFIED("installed (modified/outdated)"), + CONFLICT("conflict (not a JBang skill)"), + MISSING("not installed"), + AGENT_NOT_DETECTED("agent not detected"); + + final String label; + + Status(String label) { + this.label = label; + } + } + + @Override + public Integer doCall() throws IOException { + return missingSubcommand(); + } + + /** + * Agent skill directories we install into, keyed by a human label. With + * {@code project} the repo-local dirs under the current directory are used, + * otherwise the per-account dirs under the user home. + */ + static Map agentSkillDirs(boolean project, List selected) { + Path base = project ? Paths.get("").toAbsolutePath() : Paths.get(System.getProperty("user.home")); + List agents = new ArrayList<>(); + if (selected == null || selected.isEmpty()) { + agents.addAll(Arrays.asList(Agent.values())); + } else { + for (String token : selected) { + Agent a = Agent.fromToken(token); + if (!agents.contains(a)) { + agents.add(a); + } + } + } + Map dirs = new LinkedHashMap<>(); + for (Agent a : agents) { + dirs.put(a.label, base.resolve(a.dir).resolve("skills")); + } + return dirs; + } + + /** Render a path for display, abbreviating the user home as {@code ~}. */ + static String display(Path path) { + String home = System.getProperty("user.home"); + String s = path.toString(); + if (home != null && !home.isEmpty() && s.startsWith(home)) { + return "~" + s.substring(home.length()); + } + return s; + } + + /** + * Resolve a {@code --repo} value to a source-tarball URL. A value containing + * {@code ://} is used verbatim; otherwise it is a GitHub {@code owner/repo} + * shorthand with an optional {@code @ref} (branch, tag or commit). + */ + static String repoTarballUrl(String repo) { + String spec = (repo == null || repo.isEmpty()) ? DEFAULT_REPO : repo; + if (spec.contains("://")) { + return spec; + } + String ref = DEFAULT_REF; + int at = spec.indexOf('@'); + if (at >= 0) { + ref = spec.substring(at + 1); + spec = spec.substring(0, at); + } + return "https://github.com/" + spec + "/archive/" + ref + ".tar.gz"; + } + + /** Download the skills repo tarball (cached) and return its local path. */ + static Path fetchTarball(String repo) throws IOException { + return NetUtil.downloadAndCacheFile(repoTarballUrl(repo)); + } + + /** Determine the status of skill {@code name} in the given agent skills dir. */ + static Status status(Path tarball, Path skillsDir, String name, boolean project) throws IOException { + // per-account installs target only agents already present; a project scope + // explicitly opts this repo in, so every agent is a valid target there. + if (!project && !Files.isDirectory(skillsDir.getParent())) { + return Status.AGENT_NOT_DETECTED; + } + Path target = resolveInside(skillsDir, name); + if (!Files.exists(target)) { + return Status.MISSING; + } + Path installedMd = target.resolve("SKILL.md"); + if (!Files.isRegularFile(installedMd)) { + return Status.CONFLICT; + } + byte[] repoMd = UnpackUtil.readEntry(tarball, true, Paths.get(name, "SKILL.md")); + if (repoMd != null && Arrays.equals(repoMd, Files.readAllBytes(installedMd))) { + return Status.CURRENT; + } + return Status.MODIFIED; + } + + static abstract class BaseSkillCommand extends BaseCommand { + @Arguments(paramLabel = "skills", description = "Skill name(s); defaults to all skills in the repo") + List names; + + @Option(name = "project", hasValue = false, description = "Target the current project/repository (./.claude, ./.agents) instead of your user home.") + boolean project; + + @OptionList(name = "agent", valueSeparator = ',', description = "Limit to agent(s): claude, agents (repeatable or comma-separated).") + List agentTokens; + } + + @CommandDefinition(name = "install", description = "Install JBang agent skill(s) into detected agents.", generateHelp = true) + public static class SkillInstall extends BaseSkillCommand { + + @Option(name = "force", hasValue = false, description = "Overwrite a skill that was modified or is not a JBang skill.") + boolean force; + + @Option(name = "repo", description = "Source of the skills: a GitHub owner/repo[@ref] or a full tarball URL (default jbangdev/agent-skills).") + String repo; + + @Override + public Integer doCall() throws IOException { + return installFromTarball(fetchTarball(repo), names, force, project, agentTokens, + repo == null || repo.isEmpty() ? DEFAULT_REPO : repo); + } + } + + static int installFromTarball(Path tarball, List names, boolean force, boolean project, + List agentTokens, String source) throws IOException { + Set available = UnpackUtil.listTopFolders(tarball, true); + List wanted = resolveNames(names, available); + Map agents = agentSkillDirs(project, agentTokens); + + boolean detectedAny = false; + boolean installedAny = false; + boolean needForce = false; + for (Map.Entry agent : agents.entrySet()) { + Path skillsDir = agent.getValue(); + // per-account: only touch agents already present; project: target all + if (!project && !Files.isDirectory(skillsDir.getParent())) { + Util.verboseMsg("Skipping " + agent.getKey() + " (agent not detected)"); + continue; + } + detectedAny = true; + for (String name : wanted) { + Path target = resolveInside(skillsDir, name); + Status st = status(tarball, skillsDir, name, project); + switch (st) { + case CURRENT: + Util.infoMsg("Up to date: '" + name + "' in " + agent.getKey()); + break; + case MISSING: + install(tarball, name, target, source); + Util.infoMsg("Installed '" + name + "' into " + agent.getKey() + " (" + display(target) + ")"); + installedAny = true; + break; + case MODIFIED: + case CONFLICT: + if (force) { + Util.deletePath(target, true); + install(tarball, name, target, source); + Util.infoMsg( + "Reinstalled '" + name + "' into " + agent.getKey() + " (" + display(target) + ")"); + installedAny = true; + } else { + Util.warnMsg(st.label + ": '" + name + "' in " + agent.getKey() + + " - use --force to overwrite (" + display(target) + ")"); + needForce = true; + } + break; + default: + break; + } + } + } + if (!detectedAny) { + Util.warnMsg("No supported agents detected. Launch an agent first, or use --project inside a repository."); + return ExitException.EXIT_GENERIC_ERROR; + } + if (!installedAny && needForce) { + return ExitException.EXIT_GENERIC_ERROR; + } + return ExitException.EXIT_OK; + } + + static void install(Path tarball, String name, Path target, String source) throws IOException { + Files.createDirectories(target); + UnpackUtil.unpack(tarball, target, true, Paths.get(name)); + Files.write(target.resolve(MARKER), ("repo=" + source + System.lineSeparator()).getBytes()); + } + + @CommandDefinition(name = "uninstall", description = "Remove JBang agent skill(s) from agents.", generateHelp = true) + public static class SkillUninstall extends BaseSkillCommand { + + @Override + public Integer doCall() { + return uninstall(names, project, agentTokens); + } + } + + static int uninstall(List names, boolean project, List agentTokens) { + boolean any = false; + for (Map.Entry agent : agentSkillDirs(project, agentTokens).entrySet()) { + Path skillsDir = agent.getValue(); + // no names -> every JBang-installed skill we find (offline, marker-based) + List wanted = (names == null || names.isEmpty()) ? installedSkills(skillsDir) : names; + for (String name : wanted) { + Path target = resolveInside(skillsDir, name); + if (!Files.isDirectory(target)) { + continue; + } + // safety: only remove dirs JBang installed, never a foreign dir + if (!Files.isRegularFile(target.resolve(MARKER))) { + Util.warnMsg("Skipping '" + name + "' in " + agent.getKey() + + " - not a JBang-installed skill (" + display(target) + ")"); + continue; + } + Util.deletePath(target, true); + Util.infoMsg("Removed '" + name + "' from " + agent.getKey() + " (" + display(target) + ")"); + any = true; + } + } + if (!any) { + Util.infoMsg("Nothing to remove."); + } + return ExitException.EXIT_OK; + } + + /** + * Names of JBang-installed skills (those carrying the marker) in a skills dir. + */ + static List installedSkills(Path skillsDir) { + List found = new ArrayList<>(); + if (!Files.isDirectory(skillsDir)) { + return found; + } + try (Stream entries = Files.list(skillsDir)) { + entries.filter(p -> Files.isRegularFile(p.resolve(MARKER))) + .forEach(p -> found.add(p.getFileName().toString())); + } catch (IOException e) { + Util.verboseMsg("Could not list " + skillsDir + ": " + e.getMessage()); + } + return found; + } + + @CommandDefinition(name = "list", description = "List available skills and their status in each agent.", generateHelp = true) + public static class SkillList extends BaseCommand { + + @Option(name = "project", hasValue = false, description = "Report on the current project/repository (./.claude, ./.agents) instead of your user home.") + boolean project; + + @OptionList(name = "agent", valueSeparator = ',', description = "Limit to agent(s): claude, agents (repeatable or comma-separated).") + List agentTokens; + + @Option(name = "repo", description = "Source of the skills: a GitHub owner/repo[@ref] or a full tarball URL (default jbangdev/agent-skills).") + String repo; + + @Override + public Integer doCall() throws IOException { + Path tarball = fetchTarball(repo); + Set available = UnpackUtil.listTopFolders(tarball, true); + Map agents = agentSkillDirs(project, agentTokens); + for (String name : new TreeSet<>(available)) { + Util.infoMsg(name); + for (Map.Entry agent : agents.entrySet()) { + Path target = resolveInside(agent.getValue(), name); + Status st = status(tarball, agent.getValue(), name, project); + Util.infoMsg(String.format(" %-34s %-28s %s", agent.getKey(), st.label, display(target))); + } + } + return ExitException.EXIT_OK; + } + } + + /** + * Resolves {@code name} against {@code base}, guaranteeing the result stays + * inside {@code base}. Protects against path traversal ("Zip Slip") when the + * name originates from an archive entry or user input. + */ + static Path resolveInside(Path base, String name) { + Path target = base.resolve(name).normalize(); + if (!target.startsWith(base.normalize())) { + throw new ExitException(ExitException.EXIT_INVALID_INPUT, + "Invalid skill name '" + name + "': escapes the skills directory"); + } + return target; + } + + private static List resolveNames(List requested, Set available) { + if (requested == null || requested.isEmpty()) { + return new ArrayList<>(available); + } + for (String name : requested) { + if (!available.contains(name)) { + throw new ExitException(ExitException.EXIT_INVALID_INPUT, + "Unknown skill '" + name + "'. Available: " + String.join(", ", available)); + } + } + return requested; + } +} diff --git a/src/main/java/dev/jbang/cli/SkillHelpProvider.java b/src/main/java/dev/jbang/cli/SkillHelpProvider.java new file mode 100644 index 0000000000..ac66ff4472 --- /dev/null +++ b/src/main/java/dev/jbang/cli/SkillHelpProvider.java @@ -0,0 +1,48 @@ +package dev.jbang.cli; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import org.aesh.command.HelpEntry; +import org.aesh.command.HelpSectionProvider; + +/** + * Extra explanatory help for the {@code jbang skill} command, describing what + * it does and pointing at alternative skill installers. + */ +public class SkillHelpProvider implements HelpSectionProvider { + + @Override + public String getHeader() { + return "\n" + + "Install JBang's agent skills - instructions that teach AI coding agents\n" + + "(Claude Code, Codex, OpenCode, Pi, ...) how to install and use JBang.\n" + + "\n" + + "Skills are fetched from https://github.com/jbangdev/agent-skills and copied\n" + + "into each detected agent's skills directory:\n" + + " ~/.claude/skills// (Claude Code)\n" + + " ~/.agents/skills// (Codex / OpenCode / Pi / ...)\n" + + "Use --project to install into ./.claude and ./.agents in the current repository.\n" + + "\n" + + "Installs are safe to repeat: an up-to-date skill is left alone, a locally\n" + + "modified one is only replaced with --force, and each skill JBang installs is\n" + + "marked so 'uninstall' removes only what JBang put there and never a foreign skill.\n" + + "\n" + + " jbang skill install install the skills into detected agents\n" + + " jbang skill list show, per agent, which skills are installed\n" + + " jbang skill uninstall remove the skills JBang installed\n"; + } + + @Override + public String getFooter() { + return "\n" + + "Alternatively, install the same skills with a generic skills installer, e.g.:\n" + + " npx skills add jbangdev/agent-skills\n"; + } + + @Override + public Map> getAdditionalSections() { + return Collections.emptyMap(); + } +} diff --git a/src/main/java/dev/jbang/util/UnpackUtil.java b/src/main/java/dev/jbang/util/UnpackUtil.java index 294f6735e6..8f0abc7a9c 100644 --- a/src/main/java/dev/jbang/util/UnpackUtil.java +++ b/src/main/java/dev/jbang/util/UnpackUtil.java @@ -1,5 +1,6 @@ package dev.jbang.util; +import java.io.ByteArrayOutputStream; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; @@ -84,7 +85,7 @@ public static void unzip(Path zip, Path outputDir, boolean stripRootFolder, Path entry = entry.subpath(selectFolder.getNameCount(), entry.getNameCount()); } entry = outputDir.resolve(entry).normalize(); - if (!entry.startsWith(outputDir)) { + if (!entry.startsWith(outputDir.normalize())) { throw new IOException("Entry is outside of the target dir: " + zipEntry.getName()); } try { @@ -93,7 +94,12 @@ public static void unzip(Path zip, Path outputDir, boolean stripRootFolder, Path } else if (zipEntry.isUnixSymlink()) { try (Scanner s = new Scanner(zipFile.getInputStream(zipEntry)).useDelimiter("\\A")) { String result = s.hasNext() ? s.next() : ""; - Files.createSymbolicLink(entry, Paths.get(result)); + Path linkTarget = Paths.get(result); + if (!symlinkStaysInside(outputDir, entry, linkTarget)) { + throw new IOException("Symbolic link escapes the target dir: " + zipEntry.getName() + + " -> " + result); + } + Files.createSymbolicLink(entry, linkTarget); } } else { if (checkValidParent(entry.getParent())) { @@ -108,6 +114,18 @@ public static void unzip(Path zip, Path outputDir, boolean stripRootFolder, Path } } + /** + * Verify that a symlink located at {@code linkPath} and pointing at + * {@code linkTarget} resolves to a location inside {@code outputDir}. Rejects + * absolute targets and relative targets that escape via {@code ..}, preventing + * a symlink from being used to write outside the extraction root ("Zip Slip"). + */ + private static boolean symlinkStaysInside(Path outputDir, Path linkPath, Path linkTarget) { + Path base = outputDir.normalize(); + Path resolved = linkPath.getParent().resolve(linkTarget).normalize(); + return resolved.startsWith(base); + } + private static boolean checkValidParent(Path path) throws FileAlreadyExistsException { while (path != null && !Files.exists(path)) { path = path.getParent(); @@ -134,6 +152,71 @@ public static void defaultZipEntryCopy(ZipFile zipFile, ZipArchiveEntry zipEntry } } + /** + * Reads a single entry from a .tar.gz archive, optionally after stripping the + * single root folder (as GitHub source tarballs have). Returns {@code null} if + * the entry is not found. + */ + public static byte[] readEntry(Path targz, boolean stripRootFolder, Path relative) throws IOException { + try (TarArchiveInputStream in = new TarArchiveInputStream( + new GzipCompressorInputStream(new FileInputStream(targz.toFile())))) { + TarArchiveEntry entry; + while ((entry = in.getNextEntry()) != null) { + Path p = Paths.get(entry.getName()).normalize(); + // Ignore entries that would escape the archive root ("Zip Slip") + if (p.isAbsolute() || p.startsWith("..")) { + continue; + } + if (stripRootFolder) { + if (p.getNameCount() == 1) { + continue; + } + p = p.subpath(1, p.getNameCount()); + } + if (p.equals(relative) && !entry.isDirectory()) { + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + byte[] buf = new byte[8192]; + int n; + while ((n = in.read(buf)) != -1) { + bos.write(buf, 0, n); + } + return bos.toByteArray(); + } + } + } + return null; + } + + /** + * Lists the top-level folder names inside a .tar.gz archive, optionally after + * stripping the single root folder (as GitHub source tarballs have). + */ + public static Set listTopFolders(Path targz, boolean stripRootFolder) throws IOException { + Set folders = new LinkedHashSet<>(); + try (TarArchiveInputStream in = new TarArchiveInputStream( + new GzipCompressorInputStream(new FileInputStream(targz.toFile())))) { + TarArchiveEntry entry; + while ((entry = in.getNextEntry()) != null) { + Path p = Paths.get(entry.getName()).normalize(); + // Ignore entries that would escape the archive root ("Zip Slip") + if (p.isAbsolute() || p.startsWith("..")) { + continue; + } + if (stripRootFolder) { + if (p.getNameCount() == 1) { + continue; + } + p = p.subpath(1, p.getNameCount()); + } + // only count paths with content below them, i.e. actual folders + if (p.getNameCount() >= 2) { + folders.add(p.getName(0).toString()); + } + } + } + return folders; + } + public static void untargz(Path targz, Path outputDir, boolean stripRootFolder, Path selectFolder) throws IOException { try (TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream( @@ -159,7 +242,7 @@ public static void untargz(Path targz, Path outputDir, boolean stripRootFolder, entry = outputDir.resolve(entry).normalize(); - if (!entry.startsWith(outputDir)) { + if (!entry.startsWith(outputDir.normalize())) { throw new IOException("Entry is outside of the target dir: " + targzEntry.getName()); } @@ -169,6 +252,11 @@ public static void untargz(Path targz, Path outputDir, boolean stripRootFolder, // Handle symbolic links Path linkTarget = Paths.get(targzEntry.getLinkName()); + if (!symlinkStaysInside(outputDir, entry, linkTarget)) { + throw new IOException("Symbolic link escapes the target dir: " + targzEntry.getName() + + " -> " + linkTarget); + } + // Ensure parent directory exists Files.createDirectories(entry.getParent()); diff --git a/src/test/java/dev/jbang/cli/TestSkill.java b/src/test/java/dev/jbang/cli/TestSkill.java new file mode 100644 index 0000000000..e43afa1732 --- /dev/null +++ b/src/test/java/dev/jbang/cli/TestSkill.java @@ -0,0 +1,236 @@ +package dev.jbang.cli; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Set; + +import org.apache.commons.compress.archivers.tar.TarArchiveEntry; +import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; +import org.apache.commons.compress.archivers.tar.TarConstants; +import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import dev.jbang.BaseTest; +import dev.jbang.ExitException; +import dev.jbang.util.UnpackUtil; + +public class TestSkill extends BaseTest { + + /** + * Build a GitHub-style source tarball: one root folder, two skill folders + a + * stray file. + */ + private Path fixtureTarball(Path dir) throws IOException { + Path tgz = dir.resolve("agent-skills.tar.gz"); + try (TarArchiveOutputStream out = new TarArchiveOutputStream( + new GzipCompressorOutputStream(Files.newOutputStream(tgz)))) { + addFile(out, "repo-main/README.md", "root readme"); + addFile(out, "repo-main/skilla/SKILL.md", "skill a"); + addFile(out, "repo-main/skillb/SKILL.md", "skill b"); + addFile(out, "repo-main/skillb/references/x.md", "ref x"); + } + return tgz; + } + + /** + * Build a malicious GitHub-style tarball whose entries try to escape the + * extraction root via ".." path traversal ("Zip Slip"). + */ + private Path maliciousTarball(Path dir) throws IOException { + Path tgz = dir.resolve("evil-skills.tar.gz"); + try (TarArchiveOutputStream out = new TarArchiveOutputStream( + new GzipCompressorOutputStream(Files.newOutputStream(tgz)))) { + out.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU); + addFile(out, "repo-main/good/SKILL.md", "good skill"); + // entry that normalizes to "../evil/SKILL.md" - escaping above the archive root + addFile(out, "repo-main/../../evil/SKILL.md", "pwned"); + } + return tgz; + } + + private void addFile(TarArchiveOutputStream out, String name, String content) throws IOException { + byte[] data = content.getBytes(StandardCharsets.UTF_8); + TarArchiveEntry entry = new TarArchiveEntry(name); + entry.setSize(data.length); + out.putArchiveEntry(entry); + out.write(data); + out.closeArchiveEntry(); + } + + private void addSymlink(TarArchiveOutputStream out, String name, String linkTarget) throws IOException { + TarArchiveEntry entry = new TarArchiveEntry(name, TarConstants.LF_SYMLINK); + entry.setLinkName(linkTarget); + out.putArchiveEntry(entry); + out.closeArchiveEntry(); + } + + @Test + void unpackRejectsSymlinkEscapingTarget(@TempDir Path dir) throws IOException { + Path tgz = dir.resolve("symlink-skills.tar.gz"); + try (TarArchiveOutputStream out = new TarArchiveOutputStream( + new GzipCompressorOutputStream(Files.newOutputStream(tgz)))) { + out.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU); + addFile(out, "repo-main/jbang/SKILL.md", "skill"); + // a symlink inside the skill pointing outside the extraction root + addSymlink(out, "repo-main/jbang/escape", "../../../../../../tmp/evil"); + } + Path target = dir.resolve("out"); + Files.createDirectories(target); + IOException ex = assertThrows(IOException.class, + () -> UnpackUtil.unpack(tgz, target, true, Path.of("jbang"))); + assertThat(ex.getMessage(), containsString("escapes the target dir")); + } + + /** + * Simulate a detected agent by creating its parent directory under the (temp) + * user home. + */ + private void detectAgents() throws IOException { + Path home = Path.of(System.getProperty("user.home")); + Files.createDirectories(home.resolve(".claude")); + Files.createDirectories(home.resolve(".agents")); + } + + @Test + void listTopFoldersIgnoresRootAndStrayFiles(@TempDir Path dir) throws IOException { + Set folders = UnpackUtil.listTopFolders(fixtureTarball(dir), true); + assertThat(folders, containsInAnyOrder("skilla", "skillb")); + } + + @Test + void listTopFoldersIgnoresTraversalEntries(@TempDir Path dir) throws IOException { + Set folders = UnpackUtil.listTopFolders(maliciousTarball(dir), true); + assertThat(folders, containsInAnyOrder("good")); + } + + @Test + void readEntryIgnoresTraversalEntries(@TempDir Path dir) throws IOException { + // a ".." escaping entry must never be readable + byte[] evil = UnpackUtil.readEntry(maliciousTarball(dir), true, Path.of("..", "evil", "SKILL.md")); + assertThat(evil, is(nullValue())); + } + + @Test + void resolveInsideRejectsTraversal(@TempDir Path dir) { + assertThrows(ExitException.class, () -> Skill.resolveInside(dir, "../evil")); + assertThrows(ExitException.class, () -> Skill.resolveInside(dir, "a/../../evil")); + // a plain name resolves fine + assertEquals(dir.resolve("ok").normalize(), Skill.resolveInside(dir, "ok")); + } + + @Test + void repoTarballUrlResolvesShorthandsAndUrls() { + assertEquals("https://github.com/jbangdev/agent-skills/archive/main.tar.gz", Skill.repoTarballUrl(null)); + assertEquals("https://github.com/acme/skills/archive/main.tar.gz", Skill.repoTarballUrl("acme/skills")); + assertEquals("https://github.com/acme/skills/archive/dev.tar.gz", Skill.repoTarballUrl("acme/skills@dev")); + assertEquals("https://example.com/my.tar.gz", Skill.repoTarballUrl("https://example.com/my.tar.gz")); + } + + @Test + void readEntryReturnsFileBytes(@TempDir Path dir) throws IOException { + byte[] md = UnpackUtil.readEntry(fixtureTarball(dir), true, Path.of("skillb", "SKILL.md")); + assertEquals("skill b", new String(md, StandardCharsets.UTF_8)); + } + + @Test + void installAndUninstallAllSkills(@TempDir Path dir) throws IOException { + detectAgents(); + Path tgz = fixtureTarball(dir); + Path home = Path.of(System.getProperty("user.home")); // BaseTest points this at a temp dir + + int rc = Skill.installFromTarball(tgz, null, false, false, null, "test/repo"); + assertEquals(0, rc); + + Path claude = home.resolve(".claude").resolve("skills"); + Path agents = home.resolve(".agents").resolve("skills"); + for (Path base : Arrays.asList(claude, agents)) { + assertThat(Files.isRegularFile(base.resolve("skilla").resolve("SKILL.md")), is(true)); + assertThat(Files.isRegularFile(base.resolve("skillb").resolve("SKILL.md")), is(true)); + assertThat(Files.isRegularFile(base.resolve("skillb").resolve("references").resolve("x.md")), is(true)); + } + + // status is CURRENT right after install + assertEquals(Skill.Status.CURRENT, Skill.status(tgz, claude, "skilla", false)); + + // marker was written by install + assertThat(Files.isRegularFile(claude.resolve("skilla").resolve(".jbang-skill")), is(true)); + + Skill.uninstall(Arrays.asList("skilla", "skillb"), false, null); + assertThat(Files.exists(claude.resolve("skilla")), is(false)); + assertThat(Files.exists(agents.resolve("skillb")), is(false)); + } + + @Test + void uninstallAllUsesMarkerAndSkipsForeign(@TempDir Path dir) throws IOException { + detectAgents(); + Path tgz = fixtureTarball(dir); + Path claude = Path.of(System.getProperty("user.home")).resolve(".claude").resolve("skills"); + + Skill.installFromTarball(tgz, null, false, false, null, "test/repo"); + // a foreign skill of a different name, not installed by JBang (no marker) + Path foreign = claude.resolve("foreign"); + Files.createDirectories(foreign); + Files.writeString(foreign.resolve("SKILL.md"), "someone else"); + + // uninstall with no names removes only marker-bearing dirs + Skill.uninstall(null, false, null); + assertThat(Files.exists(claude.resolve("skilla")), is(false)); + assertThat(Files.exists(claude.resolve("skillb")), is(false)); + assertThat(Files.isRegularFile(foreign.resolve("SKILL.md")), is(true)); + } + + @Test + void agentOptionLimitsToOneTarget(@TempDir Path dir) throws IOException { + detectAgents(); + Path tgz = fixtureTarball(dir); + Path home = Path.of(System.getProperty("user.home")); + + Skill.installFromTarball(tgz, null, false, false, Arrays.asList("claude"), "test/repo"); + assertThat(Files.isRegularFile(home.resolve(".claude") + .resolve("skills") + .resolve("skilla") + .resolve("SKILL.md")), is(true)); + // the agents target was not selected, so it stays empty + assertThat(Files.exists(home.resolve(".agents").resolve("skills").resolve("skilla")), is(false)); + } + + @Test + void installWithoutDetectedAgentsFails(@TempDir Path dir) throws IOException { + // no ~/.claude or ~/.agents in the temp home -> nothing detected + int rc = Skill.installFromTarball(fixtureTarball(dir), null, false, false, null, "test/repo"); + assertEquals(ExitException.EXIT_GENERIC_ERROR, rc); + } + + @Test + void modifiedSkillNeedsForce(@TempDir Path dir) throws IOException { + detectAgents(); + Path tgz = fixtureTarball(dir); + Path home = Path.of(System.getProperty("user.home")); + Path claude = home.resolve(".claude").resolve("skills"); + + Skill.installFromTarball(tgz, null, false, false, null, "test/repo"); + // user edits the installed skill + Files.writeString(claude.resolve("skilla").resolve("SKILL.md"), "edited"); + assertEquals(Skill.Status.MODIFIED, Skill.status(tgz, claude, "skilla", false)); + + // without --force the edit survives + Skill.installFromTarball(tgz, Arrays.asList("skilla"), false, false, null, "test/repo"); + assertEquals("edited", Files.readString(claude.resolve("skilla").resolve("SKILL.md"))); + + // with --force it is restored + Skill.installFromTarball(tgz, Arrays.asList("skilla"), true, false, null, "test/repo"); + assertEquals(Skill.Status.CURRENT, Skill.status(tgz, claude, "skilla", false)); + } +}