garrytan/gstack · critical · Error

commitSkill: destination "${dest}" escapes tier root.

Error message

commitSkill: destination "${dest}" escapes tier root.

What it means

Thrown by commitSkill when isPathWithin(dest, realTierRoot) returned false after realpath-resolving the tier root. The comment marks this as 'should be impossible after validateSkillName' — validateSkillName already rejects slashes, dots, and uppercase, so a name that escapes the tier root via lexical tricks cannot get here. This is pure defense in depth against a future regression in the name validator or a weird tier root path (e.g. containing a NUL or trailing component that path.join collapses unexpectedly).

Source

Thrown at browse/src/browser-skill-write.ts:151

  } catch (err: any) {
    throw new Error(`commitSkill: staged dir "${opts.stagedDir}" not accessible: ${err.code ?? err.message}`);
  }
  if (stagedStat.isSymbolicLink()) {
    throw new Error(`commitSkill: staged dir "${opts.stagedDir}" is a symlink — refusing to commit.`);
  }
  if (!stagedStat.isDirectory()) {
    throw new Error(`commitSkill: staged path "${opts.stagedDir}" is not a directory.`);
  }

  // Ensure the tier root exists, then resolve its real path so the final
  // destination check defends against tierRoot itself being a symlink.
  fs.mkdirSync(tierRoot, { recursive: true, mode: 0o755 });
  const realTierRoot = fs.realpathSync(tierRoot);

  const dest = path.join(realTierRoot, opts.name);
  if (!isPathWithin(dest, realTierRoot)) {
    // Should be impossible after validateSkillName, but defense in depth.
    throw new Error(`commitSkill: destination "${dest}" escapes tier root.`);
  }

  // Refuse to clobber. Both regular dirs and symlinks count.
  let destExists = false;
  try {
    fs.lstatSync(dest);
    destExists = true;
  } catch (err: any) {
    if (err.code !== 'ENOENT') throw err;
  }
  if (destExists) {
    throw new Error(
      `commitSkill: a skill named "${opts.name}" already exists at ${dest}. ` +
      `Pick a different name or remove the existing skill first ` +
      `($B skill rm ${opts.name}${opts.tier === 'global' ? ' --global' : ''}).`,
    );
  }

View on GitHub (pinned to 94993f7401)

Solutions

  1. File a bug — this should be impossible given validateSkillName.
  2. Verify the name matches /^[a-z][a-z0-9]*(-[a-z0-9]+)*$/ manually.
  3. Check isPathWithin logic if the tier root contains unusual characters.
  4. Ensure opts.name was not mutated between validateSkillName and the dest check.
Defensive patterns

Strategy: try-catch

Validate before calling

import * as path from 'path';
import { isPathWithin } from './platform';

function assertDestWithinTier(dest: string, tierRoot: string): void {
  if (!isPathWithin(dest, tierRoot)) {
    throw new Error(`Destination "${dest}" escapes tier root "${tierRoot}" — defense-in-depth trip.`);
  }
}

Try / catch

try {
  commitSkill(opts);
} catch (err: any) {
  if (/escapes tier root/.test(err.message)) {
    // Should be impossible — report as a security incident.
    console.error('SECURITY: validateSkillName regression suspected', { name: opts.name });
    throw err;
  }
}

Prevention

When it happens

Trigger: A bug in validateSkillName lets a name containing a slash or '..' through; the tier root realpath resolves to something unexpected (e.g. a bind-mount boundary) that makes isPathWithin reject a normal join; a hand-edited name bypassed validateSkillName via a direct commitSkill call with opts.tier set and no prior validation.

Common situations: Effectively unreachable in normal use. If observed, suspect a regression in SKILL_NAME_PATTERN, an isPathWithin bug, or a caller that mutated opts.name after validateSkillName ran.

Related errors


AI-assisted analysis of garrytan/gstack@94993f7401 (2026-08-12). Data as JSON: /api/errors/a5de396300448871. Report an issue: GitHub.