Description

The new skillPrelude exception in evaluateShellAllowlist() treats cat <...>/SKILL.md (and a marker printf) as implicitly approved when they precede a trusted/allowlisted skill wrapper via &&.

However, isSkillPreludeReadSegment() only checks that the path string resolves to something that looks like .../skills/<id>/SKILL.md and does not verify that the resolved file is actually inside the expected skills directory after resolving symlinks.

As a result, if an attacker can create/modify files in the workspace (including creating symlinks), they can point skills/<id>/SKILL.md at an arbitrary target (e.g. /etc/passwd, $HOME/.ssh/id_rsa, cloud credentials) and have it read and printed via an otherwise-approved command chain:

• input: shell command containing cat <workspace>/skills/<id>/SKILL.md && ... && <allowlisted wrapper>
• bypass: allowlist evaluation marks the cat segment as skillPrelude even if cat is not allowlisted/safeBins
• sink: cat reads the symlink target and outputs it to stdout (potentially captured/logged/transmitted by the agent)

Vulnerable code:

return isSkillMarkdownPreludePath(resolveSkillPreludePath(rawPath, cwd));

This only enforces a pathname pattern; it does not enforce filesystem containment.

Recommendation

Treat skill prelude reads as safe only when the target file is verified to be within the skills directory after resolving symlinks.

Recommended hardening:

1. Compute an expected skills root (e.g. path.resolve(cwd, "skills") or an explicit configured skills directory).
2. Resolve the candidate path and then resolve symlinks using fs.realpathSync.native() (or async equivalent).
3. Ensure the real path is under the real skills root (prefix check with path separators).
4. Optionally require the basename to be exactly SKILL.md and disallow skills/SKILL.md (no id) if not intended.

Example:

import fs from "node:fs";

function isUnderDir(child: string, parent: string): boolean {
  const rel = path.relative(parent, child);
  return !!rel && !rel.startsWith(".." + path.sep) && rel !== "..";
}

function isSafeSkillPreludePath(rawPath: string, cwd?: string): boolean {
  const candidate = resolveSkillPreludePath(rawPath, cwd);
  const skillsRoot = path.resolve(cwd?.trim() || process.cwd(), "skills");
  const realCandidate = fs.realpathSync.native(candidate);
  const realSkillsRoot = fs.realpathSync.native(skillsRoot);
  return (
    realCandidate.toLowerCase().endsWith(path.sep + "skill.md") &&
    isUnderDir(realCandidate, realSkillsRoot)
  );
}

If symlink support is required, consider disallowing symlinks for SKILL.md altogether (e.g., via lstat + isSymbolicLink) to prevent exfiltration.

Description

evaluateShellAllowlist() introduces a skillPrelude exception that allows previously-non-allowlisted cat <...>/SKILL.md and printf '\n---CMD---\n' segments to run without prompting when the command chain later reaches a “trusted skill wrapper”.

However, resolveTrustedSkillExecutionIds() treats any allowlisted executable whose (normalized) basename ends with -wrapper as a trusted skill wrapper by deriving a skillId from the basename.

Impact:

• If an allowlist entry matches an arbitrary binary named *-wrapper (including a user-controlled path, a symlink, or a broad allowlist pattern), the chain will be considered to “reach a trusted skill wrapper”.
• This can allow cat of .../skills/<derived-skillId>/SKILL.md (and the printf marker) to execute without approval even though cat/printf are not allowlisted.
• Because the check is based on path string shape only, an attacker who can influence the filesystem could place skills/<id>/SKILL.md as a symlink to other readable files, turning the prelude into an unintended file-read/exfiltration primitive.

Vulnerable logic:

• Trust is granted based solely on normalizeExecutableToken(...).endsWith('-wrapper'), not on a verified wrapper identity/path/trust plan.

Recommendation

Do not infer “trusted skill wrapper” status from the executable basename.

Safer options:

1. Only treat wrappers as trusted when they are satisfied by the skills trust mechanism (i.e., satisfiedBy === 'skills'), and remove the allowlist *-wrapper suffix inference.

2. If allowlisted wrappers must be supported, require stronger verification, for example:

• Resolve the executable to a real path and verify it is within an expected, immutable skill-wrapper directory, and
• Verify the wrapper corresponds to a known skill id (from a registry), and
• Avoid symlink-based confusion by checking realpath and optionally file ownership/permissions.

Example tightening (conceptual):

if (satisfiedBy === 'skills') {
  ​// existing behavior
} else {
  ​// do NOT treat generic allowlist hits as trusted wrappers
}

Additionally, consider hardening the prelude file rule:

• Verify the target SKILL.md is a regular file under an expected skills root (using realpath), rejecting symlinks/out-of-root paths.