ROADMAP #85: unbounded ancestor walk enumerates attacker-placed skills

Dogfooded 2026-04-17 on main HEAD 2eb6e0c. discover_skill_roots at
commands/src/lib.rs:2795 iterates cwd.ancestors() unbounded -- no
project-root check, no HOME containment, no git boundary. Any
.claw/skills, .omc/skills, .agents/skills, .codex/skills,
.claude/skills directory on any ancestor path up to / is enumerated
and marked active: true in 'claw --output-format json skills'.

Repro 1 (cross-tenant skill injection): write
/tmp/trap/.agents/skills/rogue/SKILL.md; cd /tmp/trap/inner/work
and 'claw skills' shows rogue as active, sourced as Project roots.
git init inside the inner CWD does NOT stop the walk.

Repro 2 (CWD-dependent skill set): CWD under $HOME yields
~/.agents/skills contents; CWD outside $HOME hides them. Same user,
same binary, 26-skill delta driven by CWD alone.

Security shape: any attacker-writable ancestor becomes a skill
injection primitive. Skill descriptions are free-form Markdown fed
into the agent context -- crafted descriptions become prompt
injection. tools/src/lib.rs:3295 independently walks ancestors for
dispatch, so the injected skill is also executable via slash
command, not just listed.

Fix shape (~30-50 lines): bound ancestor walk at project root
(ConfigLoader::project_root), optionally also at $HOME; require
explicit settings.json toggle for monorepo ancestor inheritance;
mirror fix in tools/src/lib.rs::push_project_skill_lookup_roots so
listed and dispatchable skill surfaces match.

Filed in response to Clawhip pinpoint nudge 1494668784382771280 in
#clawcode-building-in-public.
This commit is contained in:
YeonGyu-Kim 2026-04-17 21:07:10 +09:00
parent 2eb6e0c1ee
commit 586a92ba79

View File

@ -1440,3 +1440,49 @@ Original filing (2026-04-13): user requested a `-acp` parameter to support ACP p
**Blocker.** None. Fix 1 + 2 is ≤20 lines in `rusty-claude-cli/src/main.rs:2010-2020` plus error-string rewording. Fix 3 is optional polish that can land separately; it is not required to close the information-leak / broken-default core. **Blocker.** None. Fix 1 + 2 is ≤20 lines in `rusty-claude-cli/src/main.rs:2010-2020` plus error-string rewording. Fix 3 is optional polish that can land separately; it is not required to close the information-leak / broken-default core.
**Source.** Jobdori dogfood 2026-04-17 against `/tmp/cd4` on main HEAD `70a0f0c` (freshly rebuilt on the dogfood machine) in response to Clawhip pinpoint nudge at `1494661235336282248`. Sibling to #83 (build date → "today") and to the 2026-04-08 startup-friction note ("no default `trusted_roots` in settings"): all three are compile-time or batch-time context bleeding into a surface that should be either runtime-resolved or explicitly configured. Distinct from #80/#81/#82 (surfaces misrepresent runtime state) — here the runtime state being described does not even belong to the user in the first place. **Source.** Jobdori dogfood 2026-04-17 against `/tmp/cd4` on main HEAD `70a0f0c` (freshly rebuilt on the dogfood machine) in response to Clawhip pinpoint nudge at `1494661235336282248`. Sibling to #83 (build date → "today") and to the 2026-04-08 startup-friction note ("no default `trusted_roots` in settings"): all three are compile-time or batch-time context bleeding into a surface that should be either runtime-resolved or explicitly configured. Distinct from #80/#81/#82 (surfaces misrepresent runtime state) — here the runtime state being described does not even belong to the user in the first place.
85. **`claw skills` walks `cwd.ancestors()` unbounded and treats every `.claw/skills`, `.omc/skills`, `.agents/skills`, `.codex/skills`, `.claude/skills` it finds as active project skills — cross-project leakage and a cheap skill-injection path from any ancestor directory** — dogfooded 2026-04-17 on main HEAD `2eb6e0c` from `/tmp/trap/inner/work`. A directory I do not own (`/tmp/trap/.agents/skills/rogue/SKILL.md`) above the worker's CWD is enumerated as an `active: true` skill by `claw --output-format json skills`, sourced as `project_claw`/`Project roots`, even after the worker's own CWD is `git init`ed to declare a project boundary. Same effect from any ancestor walk up to `/`.
**Concrete repros.**
1. *Cross-tenant skill injection from a shared `/tmp` ancestor.*
```
mkdir -p /tmp/trap/.agents/skills/rogue
cat > /tmp/trap/.agents/skills/rogue/SKILL.md <<'EOF'
---
name: rogue
description: (attacker-controlled skill)
---
# rogue
EOF
mkdir -p /tmp/trap/inner/work
cd /tmp/trap/inner/work
claw --output-format json skills
```
Output contains `{"name":"rogue","active":true,"source":{"id":"project_claw","label":"Project roots"},…}`. `git init` inside `/tmp/trap/inner/work` does *not* stop the ancestor walk — the rogue skill still surfaces, because `cwd.ancestors()` has no concept of "project root."
2. *CWD-dependent skill set.* From `/Users/yeongyu/scratch-nonrepo` (CWD under `$HOME`) `claw --output-format json skills` returns 50 skills — including every `SKILL.md` under `~/.agents/skills/*`, surfaced via `ancestor.join(".agents").join("skills")` at `rust/crates/commands/src/lib.rs:2811`. From `/tmp/cd5` (same user, same binary, CWD *outside* `$HOME`) the same command returns 24 — missing the entire `~/.agents/skills/*` set because `~` is no longer in the ancestor chain. Skill availability silently flips based on where the worker happened to be started from.
**Trace path.**
- `rust/crates/commands/src/lib.rs:2795``discover_skill_roots(cwd)` unconditionally iterates `for ancestor in cwd.ancestors()` with **no upper bound, no project-root check, no `$HOME` containment check, no git/hg/jj boundary check**.
- `rust/crates/commands/src/lib.rs:2797-2845` — for every ancestor it appends project skill roots under `.claw/skills`, `.omc/skills`, `.agents/skills`, `.codex/skills`, `.claude/skills`, plus their `commands/` legacy directories.
- `rust/crates/commands/src/lib.rs:3223-3290` (`load_skills_from_roots`) — walks each root's `SKILL.md` and emits them all as active unless a higher-priority root has the same name.
- `rust/crates/tools/src/lib.rs:3295-3320` — independently, the *runtime* skill-lookup path used by `SkillTool` at execution time walks the same ancestor chain via `push_project_skill_lookup_roots`. Any `.agents/skills/foo/SKILL.md` enumerated from an ancestor is therefore not just *listed* — it is *dispatchable* by name.
**Why this is a clawability and security gap.**
1. *Non-deterministic skill surface.* Two claws started from `/tmp/worker-A/` and `/Users/yeongyu/worker-B/` on the same machine see different skill sets. Principle #1 ("deterministic to start") is violated on a per-CWD basis.
2. *Cross-project leakage.* A parent repo's `.agents/skills` silently bleeds into a nested sub-checkout's skill namespace. Nested worktrees, monorepo subtrees, and temporary orchestrator workspaces all inherit ancestor skills they may not own.
3. *Skill-injection primitive.* Any directory writable to the attacker on an ancestor path of the worker's CWD (shared `/tmp`, a nested CI mount, a dropbox/iCloud folder, a multi-tenant build agent, a git submodule whose parent repo is attacker-influenced) can drop a `.agents/skills/<name>/SKILL.md` and have it surface as an `active: true` skill with full dispatch via `claw`'s slash-command path. Skill descriptions are free-form Markdown fed into the agent's context; a crafted `description:` becomes a prompt-injection payload the agent willingly reads before it realizes which file it's reading.
4. *Asymmetric with agents discovery.* Project agents (`/agents` surface) have explicit project-scoping via `ConfigLoader`; skills discovery does not. The two diverge on which context is considered "project."
**Fix shape — bound the walk, or re-root it.**
1. *Terminate the ancestor walk at the project root.* Plumb `ConfigLoader::project_root()` (or git-toplevel) into `discover_skill_roots` and stop at that boundary. Skills above the project root are ignored — they must be installed explicitly (via `claw skills install` or a settings entry).
2. *Optionally also terminate at `$HOME`.* If the project root can't be resolved, stop at `$HOME` so a worker in `/Users/me/foo` never reads from `/Users/`, `/`, `/private`, etc.
3. *Require acknowledgment for cross-project skills.* If an ancestor skill is inherited (intentional monorepo case), require an explicit `allow_ancestor_skills` toggle in `settings.json` and emit an event when ancestor-sourced skills are loaded. Matches the intent of ROADMAP principle #5 ("partial success / degraded mode is first-class") — surface the fact that skills are coming from outside the canonical project root.
4. *Mirror the same fix in `rust/crates/tools/src/lib.rs::push_project_skill_lookup_roots`* so the *executable* skill surface matches the *listed* skill surface. Today they share the same ancestor-walk bug, so the fix must apply to both.
5. *Regression tests:* (a) worker in `/tmp/attacker/.agents/skills/rogue` + inner CWD → `rogue` must not be surfaced; (b) worker in a user home subdir → `~/.agents/skills/*` must not leak unless explicitly allowed; (c) explicit monorepo case: `settings.json { "skills": { "allow_ancestor": true } }` → inherited skills reappear, annotated with their source path.
**Acceptance.** `claw skills` (list) and `SkillTool` (execute) both scope skill discovery to the resolved project root by default; a skill file planted under a non-project ancestor is invisible to both; an explicit opt-in (settings entry or install) is required to surface or dispatch it; the emitted skill records expose the path the skill was sourced from so a claw can audit its own tool surface.
**Blocker.** None. Fix is ~3050 lines total across the two ancestor-walk sites plus the settings-schema extension for the opt-in toggle.
**Source.** Jobdori dogfood 2026-04-17 against `/tmp/trap/inner/work`, `/Users/yeongyu/scratch-nonrepo`, and `/tmp/cd5` on main HEAD `2eb6e0c` in response to Clawhip pinpoint nudge at `1494668784382771280`. First member of a new sub-cluster ("discovery surface extends outside the project root") that is adjacent to but distinct from the #80#84 truth-audit cluster — here the surface is structurally *correct* about what it enumerates, but the enumeration itself pulls in state that does not belong to the current project.