Commit Diff


commit - 565182c8b8094abb4bd19d037ae6e2201a181704
commit + 7f51b2b2cda1fe76b2e54f2c9b9d4903c9684981
blob - 1cdba1fc1bc799035aa80854b43c8ac30d8d7bf1
blob + 714fa37d674719aaf58c13fda0d952dc4c7bf7a5
--- regress/skills/skills_test.c
+++ regress/skills/skills_test.c
@@ -21,9 +21,11 @@
  */
 
 #include <sys/stat.h>
+#include <sys/wait.h>
 
 #include <err.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -513,6 +515,67 @@ test_no_home(void)
 	}
 }
 
+/*
+ * The UI unveils ~/.fugu/skills before loading so a symlink planted under it
+ * cannot make a SKILL.md resolve to an arbitrary file.  skills_unveil()
+ * confines the whole process irreversibly, so exercise it in a child: /real
+ * must load, but evil's SKILL.md (a symlink to a file OUTSIDE the skills dir)
+ * must be refused by unveil, so /evil never loads.
+ */
+static void
+test_skills_unveil_confinement(void)
+{
+	char	tmpl[] = "/tmp/fugu-skills-uv.XXXXXX";
+	char	*home, secret[1024], path[1100], cmd[1200];
+	pid_t	pid;
+	int	status, sfd;
+
+	if ((home = mkdtemp(tmpl)) == NULL)
+		err(1, "mkdtemp");
+	(void)snprintf(path, sizeof(path), "%s/.fugu", home);
+	mkdir(path, 0755);
+	(void)snprintf(path, sizeof(path), "%s/.fugu/skills", home);
+	mkdir(path, 0755);
+
+	drop_skill(home, "real", "real body\n");
+
+	/* A file outside the skills dir, and an evil skill symlinked to it. */
+	(void)snprintf(secret, sizeof(secret), "%s/secret.txt", home);
+	if ((sfd = open(secret, O_CREAT | O_WRONLY, 0600)) != -1) {
+		(void)write(sfd, "SECRET-LEAK\n", 12);
+		close(sfd);
+	}
+	(void)snprintf(path, sizeof(path), "%s/.fugu/skills/evil", home);
+	mkdir(path, 0755);
+	(void)snprintf(path, sizeof(path), "%s/.fugu/skills/evil/SKILL.md", home);
+	if (symlink(secret, path) == -1)
+		err(1, "symlink");
+
+	setenv("HOME", home, 1);
+
+	if ((pid = fork()) == -1)
+		err(1, "fork");
+	if (pid == 0) {
+		struct skills	sk;
+		const char	*tr;
+		int		ok;
+
+		memset(&sk, 0, sizeof(sk));
+		skills_unveil();
+		skills_load(&sk);
+		ok = skills_match(&sk, "/real", &tr) != NULL &&
+		    skills_match(&sk, "/evil", &tr) == NULL;
+		_exit(ok ? 0 : 1);
+	}
+	if (waitpid(pid, &status, 0) == -1)
+		err(1, "waitpid");
+	CHECK(WIFEXITED(status) && WEXITSTATUS(status) == 0,
+	    "skills_unveil confines the load: out-of-tree symlink not followed");
+
+	(void)snprintf(cmd, sizeof(cmd), "rm -rf %s", home);
+	(void)system(cmd);
+}
+
 int
 main(void)
 {
@@ -552,6 +615,7 @@ main(void)
 	test_disabled_skill_not_invocable(home);
 	test_skill_with_internal_nul(home);
 	test_no_home();
+	test_skills_unveil_confinement();
 
 	/* Cleanup (best-effort). */
 	{
blob - 35ee38a1b98b93491e4e2d2b1993086ff5f70af6
blob + e202d8f0e8caac18fd20c2d6a73f9d5e9491ee40
--- src/ui/ui.c
+++ src/ui/ui.c
@@ -457,10 +457,15 @@ main(int argc, char *argv[])
 	    getenv("TERM") != NULL;
 
 	/*
-	 * Load skills (~/.fugu/skills/) once, before either front-end pledges
-	 * or unveils.  Slurping bodies into memory now means the runtime needs
-	 * neither rpath nor a peek at the skills directory.
+	 * Load skills (~/.fugu/skills/) once, before either front-end pledges.
+	 * unveil(2) the skills dir first so the load is pinned to it: a symlink
+	 * planted under ~/.fugu/skills cannot make a SKILL.md resolve to an
+	 * arbitrary file and slurp its bytes into a model-visible skill body
+	 * (unveil checks the resolved target).  The skills dir then joins the
+	 * front-end's locked unveil set, read-only -- benign: it is the user's
+	 * own dir and no runtime code reads it.
 	 */
+	skills_unveil();
 	skills_load(&sk);
 
 	if (ip_compose(&ip, M_READY, EP_PARENT, EP_UI, NULL, 0) == -1 ||