GHSA-74P7-6H78-GW8P: Root-cause hardening for skillctl agent-mode trust boundaries

Vulnerability

GHSA-74P7-6H78-GW8P covers multiple trust-boundary failures in skillctl when processing repository-controlled skill metadata and filesystem content. The patch changelog in commit 28dfce3865c10bf2072081ee621d1adbcc2f19e1 identifies five distinct exploit classes: unvalidated source_sha flowing into git ls-tree -r -z <refspec> -- <path>, special files reaching fs::copy, unsafe add --dest handling in non-interactive agent mode, commit trailer forgery via unsanitized names and messages, and hardlink-based exfiltration during copy/push round-trips.

The common root issue is that attacker-controlled repository state was treated as trusted operational input. In particular, values from .skills.toml and skill directory contents influenced git invocation semantics, destructive overwrite targets, and outbound content selection without sufficient structural validation. Because skillctl is intended for AI developer agent workflows, these flaws are especially dangerous: a malicious repository can steer an automated tool into deleting arbitrary directories, hanging indefinitely on FIFOs, misclassifying git state, or exporting sensitive local files.

// project_config.rs
if !is_hex_sha(&self.source_sha) {
    return Err(AppError::Config(format!(
        "invalid source_sha for skill `{}` in .skills.toml: expected 40-64 hex characters, got `{}`",
        self.name, self.source_sha
    )));
}

The advisory context is corroborated by the official patch digest and the mirrored short commit reference at commit 28dfce3, both of which describe the same remediation set.

Patch

The patch in version 0.1.3 introduces targeted validation and fail-closed checks across the affected code paths.

• Argument injection: InstalledSkill::validate now validates name and constrains source_sha to 40-64 hexadecimal characters, preventing option-like values from being interpreted as git flags before the -- separator. This directly addresses the refspec injection path documented in the patch commit.
• Filesystem safety: copy_dir_all now rejects non-directory top-level sources, rejects descendant entries that are neither regular files nor directories, and adds Unix hardlink detection via MetadataExt::nlink(). The new logic explicitly blocks FIFOs, sockets, devices, symlinks, and hardlinked files.
• Agent-mode path safety: add --dest now rejects any .. path component and rejects absolute paths when ctx.interactive is false, narrowing the attack surface for non-interactive or JSON-driven agent execution.
• Output and commit safety: push now validates user-supplied commit messages with validate_message_safe, and the new sanitize module is wired into the binary via mod sanitize;. The changelog states that names and tags receive strict identifier validation, while descriptions and messages receive lenient but control-character-safe validation.
• Regression coverage: the patch adds tests for FIFO rejection, hardlink rejection, and non-directory source rejection, as described in the filesystem utility diff.

// add.rs
for component in dest.components() {
    if matches!(component, std::path::Component::ParentDir) {
        return Err(AppError::Config(format!(
            "invalid --dest `{}`: parent traversal (`..`) is not allowed",
            dest.display()
        ))
        .into());
    }
}
if dest.is_absolute() && !ctx.interactive {
    return Err(AppError::Config(format!(
        "invalid --dest `{}`: absolute paths are not allowed in non-interactive mode...",
        dest.display()
    ))
    .into());
}

Review

Pros

• The fixes are mapped to the actual vulnerable sinks rather than relying on superficial escaping. The source_sha remediation is strong because it constrains the value to a commit-hash shape before it can reach git argument parsing.
• The filesystem changes adopt a fail-closed model. Rejecting all non-regular files and directories is the correct default for a content-copy primitive exposed to untrusted repositories.
• The patch recognizes the distinct threat model of agent mode. Blocking absolute destinations only in non-interactive mode preserves some operator flexibility while reducing automated arbitrary-directory overwrite risk.
• The commit/message sanitization is grounded in the documented exploit mechanism: newline and control-character handling for commit trailers and JSON output. This is more robust than trying to sanitize only specific trailer strings.
• The added tests cover the newly introduced rejection classes and provide evidence that the maintainers validated the intended behavior, as referenced in the official patch.

Cons

• Windows hardlink detection is explicitly skipped with #[cfg(not(unix))], leaving a platform-specific gap for the hardlink exfiltration class. The patch acknowledges this limitation rather than solving it cross-platform.
• The review material references a new sanitize module, but the provided diff snippets do not include its implementation. The security posture for name/tag/description handling therefore depends on code not shown in the supplied excerpts.
• The add --dest mitigation is context-sensitive rather than absolute. Interactive mode still permits absolute paths, which is reasonable for usability but means the dangerous primitive still exists if an attacker can influence an operator's manual invocation.
• The changelog notes a remaining backlog of medium and low issues, including atomicity and concurrency concerns, so this patch should not be interpreted as a complete hardening pass for all repository-to-local trust transitions.

Verdict

Root-cause.

This patch addresses the disclosed vulnerabilities at the correct abstraction layer: it validates attacker-controlled metadata before command execution, constrains destination paths before destructive file operations, and narrows filesystem copying to safe object classes. For the documented exploit paths in the advisory and the patch commit, the remediation is substantive rather than cosmetic.

The main caveat is platform completeness: Unix hardlink exfiltration is blocked, while Windows remains deferred. That limitation does not negate the overall assessment, but engineering teams should treat cross-platform hardlink handling as follow-up work before claiming uniform protection across supported environments.