Skip to content

Commit 8963981

Browse files
committed
fix(skills): realpath bare-name SKILL.md + docs polish
Bare-name resolution now contains symlinks to the winning base dir. Docs: .corbits clause, homedir paths, skill-dir footer, spawn skills[].
1 parent 06ad63a commit 8963981

6 files changed

Lines changed: 112 additions & 34 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ The primary director is **dispatch** (`buildChatRole` → `createDispatchSystemP
343343

344344
**Provider-conditional residuals.** Per-family additions layer on top of the shared block via the same `ModelFamilyPolicy` mechanism the directors use (`src/subagent/provider-family.ts`, `src/agent/model-family-policy.ts`) — additive lines, never prompt forks. **Grok** leaves get `buildGrokLeafAntiThrashNote` (gated by `shouldApplyGrokAntiThrash` / `applyGrokFinishBias`, withheld from orchestrators): a compact finish-bias reinforcement plus a one-line reminder to route file/web work through the dedicated tools rather than `run_shell`, motivated by observed tool-routing thrash on the same harness. **Kimi** intentionally has no residual yet — `detectModelFamily` already resolves the family so callers can branch on it, but the prompt seam is left unfilled pending eval characterization of Kimi's behavior, mirroring the provisional (permissive-default) policy in `model-family-policy.ts`.
345345

346-
`buildChatSystemPrompt` (TUI chat) assembles: base → core tool list → name-only skills listing → live `<env>` block → appended extensions. `buildSubAgentSystemPrompt` assembles the worker prompt without a catalog skills listing. Closed directors that declare `attachedSkills` (none of the shipped directors do today; never the primary) get those bodies injected once at spawn from **plugin skill dirs only** (`skillDirsFromEnabledPlugins`) — `resolveSkillBody` is called with `pluginDirsOnly`, so a project-local `.agents`/`.claude`/`.codex/skills` SKILL.md cannot become system-prompt constraints. A miss is noted in the attached section; the worker proceeds (no park). Optional skills stay names-only in the identity header; workers mount `skill_search` + `use_skill` on every family (including grok/kimi leaves), scoped to the union of `attachedSkills` and `optionalSkills`. A worker that declares neither field gets an empty allowlist (fail closed) — `skill_search` / `use_skill` match nothing until the package or plugin profile lists names. `coder` ships `optionalSkills: ["typescript"]`. Built-in catalog tools (including `skill_search`) are advertised on the primary wire and callable directly; MCP integrations are discovered via `tool_search` rather than being enumerated. Skills follow the same lazy principle on the primary: each discovered skill contributes only its name. The model calls `skill_search` for descriptions when choosing, then `use_skill` to load a body. The operator can also invoke the same skill as `/<skill-name>` (see Skills below). Skills are discovered (and deduped by name, first-wins) from enabled plugin dirs, then project `.corbits`/`.agents`/`.claude`/`.codex/skills`, then the same relative dirs under `homedir()`. A user-global path that equals the project path (cwd is home) is skipped. Corbits Code ships a bundled catalog via the first-party `corbits-skills` plugin (origin `repo`); project-local skills of the same name are shadowed by an enabled plugin skill. A trusted/enabled plugin (or local `.agents/agents/`) profile whose `id` matches a closed director replaces that director at `spawn_agent(agent=…)` (prompt, capabilities, inference, skill fields); `intent=` still resolves the closed package.
346+
`buildChatSystemPrompt` (TUI chat) assembles: base → core tool list → name-only skills listing → live `<env>` block → appended extensions. `buildSubAgentSystemPrompt` assembles the worker prompt without a catalog skills listing. Closed directors that declare `attachedSkills` (none of the shipped directors do today; never the primary) get those bodies injected once at spawn from **plugin skill dirs only** (`skillDirsFromEnabledPlugins`) — `resolveSkillBody` is called with `pluginDirsOnly`, so a project-local `.corbits`/`.agents`/`.claude`/`.codex/skills` SKILL.md cannot become system-prompt constraints. A miss is noted in the attached section; the worker proceeds (no park). Optional skills stay names-only in the identity header; workers mount `skill_search` + `use_skill` on every family (including grok/kimi leaves), scoped to the union of `attachedSkills` and `optionalSkills`. A worker that declares neither field gets an empty allowlist (fail closed) — `skill_search` / `use_skill` match nothing until the package or plugin profile lists names. `coder` ships `optionalSkills: ["typescript"]`. Built-in catalog tools (including `skill_search`) are advertised on the primary wire and callable directly; MCP integrations are discovered via `tool_search` rather than being enumerated. Skills follow the same lazy principle on the primary: each discovered skill contributes only its name. The model calls `skill_search` for descriptions when choosing, then `use_skill` to load a body. The operator can also invoke the same skill as `/<skill-name>` (see Skills below). Skills are discovered (and deduped by name, first-wins) from enabled plugin dirs, then project `.corbits`/`.agents`/`.claude`/`.codex/skills`, then the same relative dirs under `homedir()`. A user-global path that equals the project path (cwd is home) is skipped. Corbits Code ships a bundled catalog via the first-party `corbits-skills` plugin (origin `repo`); project-local skills of the same name are shadowed by an enabled plugin skill. A trusted/enabled plugin (or local `.agents/agents/`) profile whose `id` matches a closed director replaces that director at `spawn_agent(agent=…)` (prompt, capabilities, inference, skill fields); `intent=` still resolves the closed package.
347347

348348
**Overrides.** `loadSystemPromptOverrides` (`src/agent/context-extensions.ts`) resolves a project `SYSTEM.md` (repo root, then `.corbits/`) that **replaces** the static base block, and an `APPEND_SYSTEM.md` that is **appended** as an extension. These compose with `config.systemPromptExtensions` (profile config) and the auto-discovered `AGENTS.md`, all of which attach as appended sections after the base.
349349

@@ -486,7 +486,7 @@ There is no skill `type` field required for model invocation — a skill body is
486486

487487
`buildSkillsSection` lists discovered skill names in the system prompt (no descriptions). Details come from `skill_search`; the full instructions enter context in two ways:
488488

489-
1. **Model** — `skill_search` for descriptions, then `use_skill` (`src/agent/use-skill.ts`) with a skill name. The handler refuses names already attached at spawn or already loaded this session (short “already in context”; it does not dump the body again). Otherwise it calls `resolveSkillBody`, strips the frontmatter, and returns the body as the tool result.
489+
1. **Model** — `skill_search` for descriptions, then `use_skill` (`src/agent/use-skill.ts`) with a skill name. The handler refuses names already attached at spawn or already loaded this session (short “already in context”; it does not dump the body again). Otherwise it calls `resolveSkillBody`, strips the frontmatter, and returns the body as the tool result. The result appends a `Skill directory:` footer naming the winning skill dir plus a sibling-file hint (read sibling files with `read_file` relative to that dir).
490490
2. **Operator** — `/<skill-name>` from `loadSkillCommands` sends the same SKILL.md body (plus typed args) to the primary as a user turn. Skills with `user-invocable: false` are omitted from the slash registry and remain `use_skill` only. Dispatch then follows the recipe.
491491

492492
Which plugin skill directories are in scope is decided in `runner.ts` / `skillDirsFromEnabledPlugins`, which passes the enabled plugins' dirs to both `discoverSkills` (for the listing) and the `use_skill` tool (for resolution). Project-local `.corbits`/`.agents`/`.claude`/`.codex/skills` and the same dirs under `homedir()` are always searched (user paths that equal project paths are skipped). Slash-command registration is first-wins (built-ins, then plugins in discovery order), so a first-party `/implement` stays first-party if a marketplace plugin of the same slash name is also enabled.

‎docs/IMPLEMENTATION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ Twenty packages under `src/agent/directors/<id>/` register in `DIRECTOR_REGISTRY
168168
**One advertised posix set (CL-8400).** Registry engines stay posix-named (`read_file`, `write_file`, `edit_file`, `delete_file`, `run_shell`, `search_files`, `grep`). Advertise is a 1:1 projection onto wire names (`read`, `write`, `edit`, `delete`, `bash`, `glob`, `grep`) plus the unchanged control-plane. Incoming aliases (wire names, old posix ids, Codex `shell` → `run_shell`, `update_plan` → `manage_tasks`) canonicalize onto the engine id for dispatch and grants. `apply_patch` is neither advertised nor dispatched (not an alias of `edit`). `list_dir` stays mounted and unadvertised. Director `tools.allow` stays engine names. Codex does not dual-publish `shell`+`run_shell`. Hidden `shell` coerces Codex `command` (string or `["bash","-lc",script]` argv), `workdir`, and `timeout_ms` onto `run_shell`. Hidden `update_plan` maps `plan: [{step, status}]` onto `manage_tasks(action: "create")` (`pending`/`in_progress`/`completed` → `todo`/`doing`/`done`).
169169

170170
6. There is no static write-path declaration on packages or profiles (CL-6952 removed it — no shipped director ever set one). Instead, `agent-fleet.ts` tracks each running dispatch by cwd; a new mutating dispatch that lands on the same cwd as a live mutating peer (`pending_init`/`running`, and not a declared read-only `modelRole` of `explore`/`plan`/`review`/`test`) records at most one `concurrent-lane-overlap` entry per cwd wave in `intervention-log.ts` (class `conflict`). The wave flag clears when no live mutating writer remains for that cwd. Terminal-but-unsettled lanes (for example cancelled with `finishedAt` set while the run promise has not reached `finally`) are pruned from the map and do not warn. This is advisory only — it never blocks the spawn, since cwd overlap does not prove the two lanes touch the same files.
171-
7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Attached skills (none on the shipped directors today; never the primary) are injected into the worker system prompt at spawn from plugin skill dirs only (no project-local `.corbits`/`.agents`/`.claude`/`.codex` or user-global fallback). Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` on every family, scoped to the union of `attachedSkills` and `optionalSkills`. A worker that declares neither field gets an empty allowlist (fail closed). `coder` lists `typescript`. `use_skill` refuses names already attached or already loaded this session and does not return the body again. Primary mounts `use_skill` for its own skill list (same in-session refuse; no attached set). Plugin skill discovery also searches `<cwd>/.corbits/skills` and `homedir()/.corbits`/`.agents`/`.claude`/`.codex/skills`.
171+
7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Attached skills (none on the shipped directors today; never the primary) are injected into the worker system prompt at spawn from plugin skill dirs only (no project-local `.corbits`/`.agents`/`.claude`/`.codex` or user-global fallback). Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` on every family, scoped to the union of `attachedSkills` and `optionalSkills`. A worker that declares neither field gets an empty allowlist (fail closed). `coder` lists `typescript`. `use_skill` refuses names already attached or already loaded this session and does not return the body again. Primary mounts `use_skill` for its own skill list (same in-session refuse; no attached set). `spawn_agent` accepts a `skills[]` param unioned with the package allowlist for the worker's `skill_search` / `use_skill` scope. Plugin skill discovery also searches `<cwd>/.corbits/skills`, `homedir()/.corbits/skills`, `homedir()/.agents/skills`, `homedir()/.claude/skills`, and `homedir()/.codex/skills`.
172172

173173
Intent defaults: `intent=implement` → director `coder`; `explore` → `explorer`; `plan` → `planner`; `review` → `reviewer`; general → error. Spawn: dispatch full fleet; all other directors mount no fleet tools. Dispatch assigns one focused task per worker; fan-out width follows independent lanes (one lane per PR/path/ownership). Live `<env>` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt.
174174

‎docs/PLUGINS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ shape.
280280
plugin root (`pluginRoot`), with lexical containment plus a realpath check so
281281
a symlink under the root cannot escape. Absolute paths, bare `.` / `..`, and
282282
escapes outside the root are rejected (skill miss, not a crash).
283+
- `spawn_agent` accepts a `skills[]` param unioned with the package allowlist, scoping the worker's `skill_search` / `use_skill`.
283284
- An agent plugin is wired in only when `settings.plugins[id].enabled` is true
284285
(same gating as command plugins — no consent needed, since profiles are
285286
configuration data, not in-process code).

‎src/agent/profiles.test.ts‎

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, test } from "bun:test";
2-
import { mkdtemp, writeFile } from "node:fs/promises";
2+
import { mkdtemp, rm, writeFile } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55

@@ -9,38 +9,46 @@ import { defined } from "../../testkit/defined.js";
99
describe("loadAgentProfiles director overlay", () => {
1010
test("plugin extraProfiles with a closed director id replace the default", async () => {
1111
const dir = await mkdtemp(join(tmpdir(), "cl9917-profiles-"));
12-
const profiles = await loadAgentProfiles(dir, [
13-
{
14-
id: "designer",
15-
source: "plugin:p1",
16-
systemPromptRole: "Plugin designer prompt.",
17-
optionalSkills: ["better-ui"],
18-
},
19-
]);
20-
const designer = defined(
21-
profiles.find((p) => p.id === "designer"),
22-
"designer profile",
23-
);
24-
expect(designer.systemPromptRole).toBe("Plugin designer prompt.");
25-
expect(designer.source).toBe("plugin:p1");
26-
expect(designer.optionalSkills).toEqual(["better-ui"]);
12+
try {
13+
const profiles = await loadAgentProfiles(dir, [
14+
{
15+
id: "designer",
16+
source: "plugin:p1",
17+
systemPromptRole: "Plugin designer prompt.",
18+
optionalSkills: ["better-ui"],
19+
},
20+
]);
21+
const designer = defined(
22+
profiles.find((p) => p.id === "designer"),
23+
"designer profile",
24+
);
25+
expect(designer.systemPromptRole).toBe("Plugin designer prompt.");
26+
expect(designer.source).toBe("plugin:p1");
27+
expect(designer.optionalSkills).toEqual(["better-ui"]);
28+
} finally {
29+
await rm(dir, { recursive: true, force: true });
30+
}
2731
});
2832

2933
test("local file with a closed director id replaces the default", async () => {
3034
const dir = await mkdtemp(join(tmpdir(), "cl9917-profiles-local-"));
31-
await writeFile(
32-
join(dir, "designer.json"),
33-
JSON.stringify({
34-
id: "designer",
35-
systemPromptRole: "Local designer prompt.",
36-
}),
37-
);
38-
const profiles = await loadAgentProfiles(dir);
39-
const designer = defined(
40-
profiles.find((p) => p.id === "designer"),
41-
"designer profile",
42-
);
43-
expect(designer.systemPromptRole).toBe("Local designer prompt.");
44-
expect(designer.source).toBe("local");
35+
try {
36+
await writeFile(
37+
join(dir, "designer.json"),
38+
JSON.stringify({
39+
id: "designer",
40+
systemPromptRole: "Local designer prompt.",
41+
}),
42+
);
43+
const profiles = await loadAgentProfiles(dir);
44+
const designer = defined(
45+
profiles.find((p) => p.id === "designer"),
46+
"designer profile",
47+
);
48+
expect(designer.systemPromptRole).toBe("Local designer prompt.");
49+
expect(designer.source).toBe("local");
50+
} finally {
51+
await rm(dir, { recursive: true, force: true });
52+
}
4553
});
4654
});

‎src/extensions/skills.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,57 @@ describe("skill resolution", () => {
241241
});
242242
});
243243

244+
describe("bare-name symlink escape", () => {
245+
test("file-symlink SKILL.md pointing outside fails closed", async () => {
246+
const cwd = await mkdtemp(join(tmpdir(), "skill-bare-filelink-cwd-"));
247+
const outside = await mkdtemp(join(tmpdir(), "skill-bare-filelink-out-"));
248+
const home = await mkdtemp(join(tmpdir(), "skill-bare-filelink-home-"));
249+
try {
250+
await mkdir(join(cwd, ".agents", "skills", "evil"), { recursive: true });
251+
await writeFile(
252+
join(outside, "SKILL.md"),
253+
"escaped outside body\n",
254+
"utf8",
255+
);
256+
await symlink(
257+
join(outside, "SKILL.md"),
258+
join(cwd, ".agents", "skills", "evil", "SKILL.md"),
259+
);
260+
await withMockedHomedir(home, async () => {
261+
expect(await resolveSkillBody(cwd, "evil", [])).toBeUndefined();
262+
expect(await resolveSkillWithDir(cwd, "evil", [])).toBeUndefined();
263+
});
264+
} finally {
265+
await rm(cwd, { recursive: true, force: true });
266+
await rm(outside, { recursive: true, force: true });
267+
await rm(home, { recursive: true, force: true });
268+
}
269+
});
270+
271+
test("dir-symlink skill dir pointing outside fails closed", async () => {
272+
const cwd = await mkdtemp(join(tmpdir(), "skill-bare-dirlink-cwd-"));
273+
const outside = await mkdtemp(join(tmpdir(), "skill-bare-dirlink-out-"));
274+
const home = await mkdtemp(join(tmpdir(), "skill-bare-dirlink-home-"));
275+
try {
276+
await mkdir(join(cwd, ".agents", "skills"), { recursive: true });
277+
await writeFile(
278+
join(outside, "SKILL.md"),
279+
"escaped outside body\n",
280+
"utf8",
281+
);
282+
await symlink(outside, join(cwd, ".agents", "skills", "evildir"), "dir");
283+
await withMockedHomedir(home, async () => {
284+
expect(await resolveSkillBody(cwd, "evildir", [])).toBeUndefined();
285+
expect(await resolveSkillWithDir(cwd, "evildir", [])).toBeUndefined();
286+
});
287+
} finally {
288+
await rm(cwd, { recursive: true, force: true });
289+
await rm(outside, { recursive: true, force: true });
290+
await rm(home, { recursive: true, force: true });
291+
}
292+
});
293+
});
294+
244295
describe("path-like skill refs", () => {
245296
let pluginRoot: string;
246297

‎src/extensions/skills.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,25 @@ export async function resolveSkillWithDir(
200200
options?.pluginDirsOnly !== true,
201201
)) {
202202
const dir = join(base, name);
203-
const body = await bodyFromSkillPath(join(dir, "SKILL.md"));
203+
const skillMd = join(dir, "SKILL.md");
204+
// Symlink escape bar (mirrors resolvePathLikeSkill): realpath both sides
205+
// so a skill dir or SKILL.md symlinked outside the winning base fails
206+
// closed instead of returning outside content.
207+
let realSkill: string;
208+
let realBase: string;
209+
try {
210+
[realSkill, realBase] = await Promise.all([
211+
realpath(skillMd),
212+
realpath(base),
213+
]);
214+
} catch {
215+
// Missing base or skill → not resolvable here.
216+
continue;
217+
}
218+
if (!pathIsInsideOrEqual(realSkill, realBase)) continue;
219+
const body = await bodyFromSkillPath(realSkill);
220+
// Return the lexical dir (unchanged public semantics): containment of the
221+
// realpath'd SKILL.md in the realpath'd base already failed closed above.
204222
if (body !== undefined) return { body, dir };
205223
}
206224
return undefined;

0 commit comments

Comments
 (0)