fix(skills): set per-skill directory mode to 0755 through the pinned descriptor

packages/client/README.md

@@ -837,7 +837,8 @@ however its transport does.
 **Run `write_skills` as a different identity than the agent.** Reconcile as one user, run the
 agent as another. The reconcile sets modes explicitly rather than from your umask: skill files
 and the manifest at `0644` (via `fchmod` on the descriptor, so it cannot be redirected),
-per-skill `<root>/<key>/` directories at `0755`, and never the execute bit. Those modes only
+per-skill `<root>/<key>/` directories at `0755` (keeping a setgid bit inherited from a setgid
+root, so a shared group still propagates), and never the execute bit. Those modes only
 protect anything if the two identities differ.
 
 **What to verify, as the identity that will run the agent.** The SDK cannot check this for you

packages/client/src/launchdarkly_ai_server/safe_fs.py

@@ -30,6 +30,11 @@
 """Mode set explicitly on every written file: never from the umask, never
 executable."""
 
+_DIR_MODE = 0o755
+"""Permission bits set explicitly on every directory this module creates, because
+``mkdir``'s mode argument is masked by the umask: under ``0077`` a separate agent
+identity could not traverse the directory to read what is inside."""
+
 _SUPPORTS_FCHMOD = hasattr(os, "fchmod")
 """Whether the mode can be set on the descriptor. Probed because Windows only
 has ``os.fchmod`` from CPython 3.13, and this package supports 3.12."""
@@ -134,8 +139,10 @@ def open_or_create_directory(
     # Without the *at() family the mkdir below would raise on a dir_fd.
     if not SUPPORTS_DIR_FD:
         dir_fd = None
+    created = False
     try:
-        os.mkdir(_at(directory, dir_fd), 0o755, dir_fd=dir_fd)
+        os.mkdir(_at(directory, dir_fd), _DIR_MODE, dir_fd=dir_fd)
+        created = True
     except FileExistsError:
         # os.stat(follow_symlinks=False) is the spelling os.supports_dir_fd
         # advertises; it is equivalent to os.lstat.
@@ -147,7 +154,22 @@ def open_or_create_directory(
             raise ValueError("the directory is a symlink") from None
         if not stat.S_ISDIR(mode):
             raise ValueError("the path is not a directory") from None
-    return open_directory_nofollow(directory, dir_fd=dir_fd)
+    fd = open_directory_nofollow(directory, dir_fd=dir_fd)
+    # Only a directory this call created: an existing one keeps the mode its
+    # owner gave it. fchmod on the pinned descriptor, never chmod on the path,
+    # which a swap between mkdir and open could redirect. Without a descriptor
+    # (no *at() family, i.e. Windows) there are no POSIX modes to correct.
+    if created and fd is not None and _SUPPORTS_FCHMOD:
+        try:
+            # Keep the setgid bit Linux copies from a setgid parent: fchmod sets
+            # exactly the bits given, so a bare 0755 would clear it and files
+            # written inside would stop inheriting the shared group.
+            inherited = os.fstat(fd).st_mode & stat.S_ISGID
+            os.fchmod(fd, inherited | _DIR_MODE)
+        except BaseException:
+            os.close(fd)
+            raise
+    return fd
 
 
 @contextmanager

packages/client/tests/test_safe_fs.py

@@ -64,6 +64,20 @@ def test_create_makes_the_directory(self, tmp_path: Path) -> None:
             if fd is not None:
                 os.close(fd)
 
+    @pytest.mark.skipif(os.name == "nt", reason="POSIX modes")
+    def test_create_leaves_an_existing_directory_mode_alone(
+        self, tmp_path: Path
+    ) -> None:
+        """The explicit ``0755`` is for directories this call creates; one that
+        was already there keeps the mode its owner chose."""
+        target = tmp_path / "existing"
+        target.mkdir()
+        target.chmod(0o700)
+        fd = open_or_create_directory(target)
+        if fd is not None:
+            os.close(fd)
+        assert stat.S_IMODE(target.stat().st_mode) == 0o700
+
     def test_create_refuses_an_existing_symlink(self, tmp_path: Path) -> None:
         """``Path.mkdir(exist_ok=True)`` would accept this and reopen the hole.
 

packages/client/tests/test_skills_fs.py

@@ -1446,6 +1446,43 @@ async def test_written_file_is_0644_and_not_executable(self, root: Path) -> None
         assert not mode & stat.S_IXGRP
         assert not mode & stat.S_IXOTH
 
+    @pytest.mark.skipif(os.name == "nt", reason="POSIX modes")
+    async def test_created_skill_directory_is_0755_under_a_restrictive_umask(
+        self, root: Path
+    ) -> None:
+        """``mkdir``'s mode is masked by the umask, so without an explicit
+        ``fchmod`` a ``0077`` umask leaves ``<root>/<key>/`` at ``0700`` and a
+        separate agent identity cannot read the ``SKILL.md`` inside it."""
+        previous = os.umask(0o077)
+        try:
+            report = await write_skills([_skill("a")], root)
+        finally:
+            os.umask(previous)
+        assert report.ok is True, _error_messages(report)
+        assert stat.S_IMODE((root / "a").stat().st_mode) == 0o755
+        assert stat.S_IMODE((root / "a" / "SKILL.md").stat().st_mode) == 0o644
+
+    async def test_created_skill_directory_keeps_an_inherited_setgid_bit(
+        self, root: Path
+    ) -> None:
+        """Linux copies a setgid parent's bit onto a new directory so files
+        inside inherit the shared group. The explicit ``0755`` must not clear
+        it; ``fchmod`` sets exactly the bits it is given."""
+        os.chmod(root, 0o2755)
+        probe = root / "probe"
+        probe.mkdir()
+        inherits = bool(probe.stat().st_mode & stat.S_ISGID)
+        probe.rmdir()
+        if not inherits:
+            pytest.skip("new directories do not inherit setgid on this platform")
+        previous = os.umask(0o077)
+        try:
+            report = await write_skills([_skill("a")], root)
+        finally:
+            os.umask(previous)
+        assert report.ok is True, _error_messages(report)
+        assert stat.S_IMODE((root / "a").stat().st_mode) == 0o2755
+
     async def test_write_goes_through_a_single_atomic_rename(
         self, root: Path, monkeypatch: pytest.MonkeyPatch
     ) -> None: