tool-skills: default ~/.agents/skills scan rejects externally-symlinked skills (boundary check), warning on every session start

[Joi]

Affects: microsoft/amplifier-bundle-skills (modules/tool-skills) + microsoft/amplifier-app-cli (d45c033). Filed here because both have issues disabled.

Summary

amplifier-app-cli (since commit d45c033, amplifier_app_cli/runtime/config.py,
_ensure_default_skills_dirs) unconditionally appends ~/.agents/skills and .agents/skills
to tool-skills's config.skills. Its docstring gives the rationale: the cross-tool
location other agent CLIs install user- and project-scoped skills into (agentskills.io
convention). There is no opt-out: the paths are appended regardless of what config.skills
already contains.

modules/tool-skills/amplifier_module_tool_skills/discovery.py (discover_skills) walks each
configured directory with os.walk(followlinks=True) and rejects any entry whose resolved path
is outside a boundary. The boundary is the enclosing git repo root when the skills dir is inside
a repo, otherwise the skills dir itself.

~/.agents/skills is a user-home directory and is not inside a git repo. Some CLIs (observed:
the Codex environment installer) install skills there as symlinks to the skill's source
checkout. Every such symlink resolves outside ~/.agents/skills, so each is rejected and emits
a WARNING at session start.

So for users whose ~/.agents/skills is populated by symlinks, the new default discovers none
of them and cannot be silenced from settings. Symlinks whose targets stay inside the skills
dir, or inside the enclosing repo for project-scoped dirs, are still accepted (existing tests
cover both); the failure is specific to external-target symlinks under the non-repo user dir.

Reproduction

T=$(mktemp -d)
mkdir -p "$T/src/hello" ~/.agents/skills
printf -- '---\nname: hello-repro\ndescription: boundary repro skill, safe to delete\n---\nhi\n' > "$T/src/hello/SKILL.md"
ln -s "$T/src/hello" ~/.agents/skills/hello-repro
amplifier tool invoke load_skill list=true 2>&1 | grep -E "hello-repro|Skipping symlink"
# cleanup
rm ~/.agents/skills/hello-repro; rm -rf "$T"

Observed:

WARNING amplifier_module_tool_skills.discovery: Skipping symlink that escapes skill
directory boundary: /Users/<me>/.agents/skills/hello-repro (resolves to
/private/var/.../src/hello, outside /Users/<me>/.agents/skills)

hello-repro does not appear in the skill list.

Real-world instance: 13 skills symlinked into ~/.agents/skills by a Codex environment
installer, with targets under two git repos elsewhere in $HOME. All 13 warn and are
skipped on every Amplifier session start.

Versions: macOS 26.6.2 (arm64), Python 3.14; amplifier 2026.09.10-d45c033 (amplifier-core
1.6.1); amplifier-bundle-skills cache at commit f5b1bb1f5b0e5653f7a504a95bbd9ec0134d8e39
(modules/tool-skills).

Expected

Either (a) skills symlinked into the app-added user-scope dir ~/.agents/skills are
discovered, or (b) the app does not scan a directory whose typical contents it will reject,
and does not warn about them.

Relevant code

discovery.py:

base_resolved = skills_dir.resolve()
repo_root = _find_repo_root(skills_dir)
boundary = repo_root if repo_root is not None else base_resolved

for root, _dirs, files in os.walk(skills_dir, followlinks=True):
    root_resolved = Path(root).resolve()
    if not root_resolved.is_relative_to(boundary):
        logger.warning("Skipping symlink that escapes ... boundary: ...")
        continue

runtime/config.py:

default_paths = [
    ".amplifier/skills",
    ".agents/skills",
    "~/.amplifier/skills",
    "~/.agents/skills",
    str(packaged_skills_dir),
]
...
for path in default_paths:
    if path not in skills:
        skills.append(path)

Secondary observation on the same loop

continue does not stop os.walk from descending into a rejected directory: with top-down
traversal, recursion is pruned only by clearing the yielded _dirs list. So an escaping
symlink is skipped but its target tree is still enumerated (one warning per descendant
directory), and because followlinks=True does not track visited directories, a symlink
cycle in the target can loop. Adding _dirs.clear() (or _dirs[:] = []) before continue
would make the rejection also prune traversal. This is separate from the policy question
above but is in the same lines.

Discussion / possible fix

The boundary check exists to stop a project-scoped skills dir in a repo the user did not
author from pointing a symlink at, e.g., /etc. That threat model is right for
.agents/skills and .amplifier/skills inside a checkout. It does not fit a directory under
the user's own $HOME that the app itself adds and that the user (or a CLI acting for them)
populated by symlinking on purpose.

Suggested direction: scope-aware policy, relaxed only for the app-added user-scope
directories, keeping the project-scope negative cases exactly as tested today
(test_symlink_outside_repo_root_is_blocked, test_non_git_dir_symlink_outside_skills_dir_is_blocked).

• For ~/.amplifier/skills and ~/.agents/skills (paths under $HOME the app adds), treat a
  top-level symlink as an installed skill: the boundary for that skill's contents becomes the
  resolved target directory (so a skill's own nested links still cannot escape).
• Project-scope dirs keep the current repo-root / dir boundary unchanged.
• Test contract: external-target top-level symlink accepted from user-scope
  ~/.agents/skills; the identical layout rejected from project-scope .agents/skills;
  nested escaping link inside an accepted skill rejected and pruned; symlink cycle bounded.
• Independently: _ensure_default_skills_dirs could accept an opt-out (e.g.
  skills_default_dirs: false or an exclude list) so the scan set can be made explicit in
  settings.

I deliberately did not propose "trust any top-level symlink" or "derive the boundary from the
link target" for all dirs — both would let a checked-in .agents/skills/evil -> /outside
bypass the project-scope check.

Filing on the umbrella repo because amplifier-bundle-skills and amplifier-app-cli have
issues disabled. The boundary rule lives in amplifier-bundle-skills modules/tool-skills
(cache commit f5b1bb1); the app-cli side is that d45c033 adds a path whose typical contents
the module rejects. Happy to open the PR
against modules/tool-skills if the direction above is acceptable.