Skip to content

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

Merged
XieX merged 2 commits into
xie/agent-skills-feature-ac9ac7from
xie/skills-dir-mode-fchmod
Oct 9, 2026
Merged

XieX merged 2 commits into
xie/agent-skills-feature-ac9ac7from
xie/skills-dir-mode-fchmod

Conversation

@XieX

@XieX XieX commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Same finding as the Bugbot comment on launchdarkly/python-ai-sdk#87 (comment 4232816271). The gap is identical in this SDK.

What was wrong: openOrCreateDirectory in safe-fs.ts created <root>/<key>/ with mkdir(directory, 0o755), but the process umask still masks that mode, and nothing set it afterwards. Under umask 0077 the directory came out 0700, so an agent running as a different identity couldn't read SKILL.md, even though writeSkills reported success. The README's privilege-separation section promises per-skill directories at 0755 no matter what the umask is.

Fix: after a successful create, the code sets the mode to 0755 with FileHandle.chmod (that is, fchmod) on the handle openDirectoryNoFollow already returns, the same way FILE_MODE is applied to files. It never chmods the path, because a swap between the mkdir and the open could redirect a path-based chmod.

  • The mode is only changed when this call created the directory. An existing <root>/<key>/ keeps its mode.
  • The managed root's mode is still up to the user, as the README says. It's created by a separate mkdir(root) in skills-fs.ts and isn't touched here.
  • A setgid bit the new directory inherited is kept. On Linux, a directory created under a setgid parent gets the parent's S_ISGID bit, and fchmod uses exactly the bits passed, so a bare 0755 cleared it. Files written inside would then get the reconcile identity's primary group instead of the shared group. The mode is now (mode & S_ISGID) | 0755, with the current mode read through handle.stat() on the same descriptor. macOS doesn't propagate the bit, so nothing changes there.
  • The symlink and not-a-directory refusals are unchanged. If the chmod fails, the handle is closed before the error is rethrown.

Tests (skipped on win32):

  • creates per-skill directories as 0755 even under a restrictive umask runs writeSkills under process.umask(0o077) and asserts the directory is 0755 and SKILL.md is 0644. It fails without the fix.
  • keeps a setgid bit the new skill directory inherited makes the root 2755 and asserts the new skill directory is 2755. It's skipped unless a probe directory under that root actually inherits the bit, so it runs on Linux and skips on macOS. On Linux (Docker, node:22) it fails without the setgid change. fs.constants has no S_ISGID, so safe-fs.ts defines it locally.
  • leaves the mode of an existing directory alone checks that an existing 0700 directory stays 0700.

Full suite: 1394 passed on macOS (the setgid test skips) and 1405 passed on Linux. biome and tsc are clean.

The Python fix is in a separate PR, linked below. The spec lines are in launchdarkly/ai-sdks-monorepo#53 (merged) and its setgid follow-up.

🤖 Generated with Claude Code

…descriptor

mkdir's mode argument is masked by the process umask, so under umask 0077
<root>/<key>/ ended up 0700 and a separate agent identity could not read
SKILL.md, though the reconcile reported success. The README promises 0755
regardless of the umask.

After a successful create, chmod the directory through the descriptor the
code already pins, never its path. A directory that already existed keeps
its mode, and the managed root is left to the user.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@XieX

XieX commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

…mode

Linux copies a setgid parent's bit onto a new directory so files created
inside inherit the shared group. fchmod sets exactly the bits it is given,
so the bare 0755 cleared it. Carry S_ISGID over from the same handle's
stat.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@XieX

XieX commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Setgid follow-up to the spec: https://github.com/launchdarkly/ai-sdks-monorepo/pull/54 (the first spec PR, #53, is merged).

@XieX
XieX merged commit 960ecc3 into xie/agent-skills-feature-ac9ac7 Oct 9, 2026
10 checks passed
@XieX
XieX deleted the xie/skills-dir-mode-fchmod branch October 9, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants