microsoft / microsoft/amplifier
tool-skills: default ~/.agents/skills scan rejects externally-symlinked skills (boundary check), warning on every session start
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 3.1k
- Forks
- 261
- Avg merge
- 3h 28m
- Merged PRs (30d)
- 13
Description
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/skillsand~/.agents/skills(paths under$HOMEthe 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_dirscould accept an opt-out (e.g.
skills_default_dirs: falseor 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.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with modules/tool-skills/amplifier_module_tool_skills/discovery.py, especially discover_skills, and amplifier_app_cli/runtime/config.py at _ensure_default_skills_dirs. Run the existing symlink tests, including test_symlink_outside_repo_root_is_blocked and test_non_git_dir_symlink_outside_skills_dir_is_blocked, then reproduce the external ~/.agents/skills link. Done means the agreed user-scope behavior is covered without weakening project-scope rejection, including nested-link pruning and bounded cycles.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, python
- Domain
- security, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100