Skip to content

skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203

Open

Description

@easonLiangWorldedtech

Problem

SkillsManager.updateSkillModes / createSkill publish a SKILL.md by resolving the target, validating it against the configured skills roots, staging a temp sidecar, and renaming it into place. CodeRabbit's review of #1321 (Security Boundaries row) notes the remaining publish-time TOCTOU: between the realpath validation and the rename, the target or one of its parents can be replaced with a symlink, so a path-based validation cannot fully prove the published file stays inside the selected skills root.

The current unit mitigates the common cases: exclusive staging (wx), a repeated realpath + root-containment check immediately before the rename with the error propagating (no fallback), a per-directory realpath-keyed queue shared by all SkillsManager instances, and a bounded stale-sidecar sweep. Closing the race itself needs directory-handle-based (descriptor-relative) publication, not path strings.

Proposed direction

  • Open and pin the configured skills root and the target parent directory with no-follow semantics; retain the handles through validation and publication.
  • Validate identity through the pinned handles and create/rename the temporary file relative to the pinned parent (or copy to a validated descriptor and fsync the parent).
  • Fail closed on every realpath/handle failure, including the pre-rename re-resolution (already done for the path-based check in fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321).
  • Add regression tests that swap the target or an ancestor symlink between validation and rename and assert publication is refused and the staged sidecar is removed.

Context

Found in the CodeRabbit review of #1321 (SkillsManager.ts). The path-based mitigations shipped there are intentional: the remaining work is a platform-level primitive shared with #2148 (guarded write descriptor-relative publication). This issue tracks the SkillsManager unit applying that primitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions