fix(slice-05): bundle-skills-router target-side path containment (guard B1 repair on 9de91ad4) (OPR.0.3.2.5)

Repairs guard BLOCKING-CONCERN B1 on qitem-20260518215234-f84fff45 /
9de91ad4: routeSkills had path-containment on SOURCE (sourceAbs vs
bundleRoot) but missed it on TARGET (targetAbs vs targetSkillsDir).
Declared path skills/../outside/SKILL.md passes source-containment
(resolves under bundleRoot since the leading skills segment is
consumed before ../) but after the leading skills/ strip becomes
../outside/SKILL.md which resolves OUTSIDE targetSkillsDir — file
would land in the wrong location on the operator host.

Third trust-boundary regression in this slice (same class as Item 2
raw-extract B1 repaired at 6401fe4b and Item 3 rig_spec containment
B1 repaired at a660b475). Pattern fully repeats: when handling
untrusted path input, contain BOTH source AND target against their
respective bounding directories. Banked lesson
feedback_pre_existing_trust_boundary_reuse_canonical_helper applies
verbatim — I only applied half of it.

Fix applied:
- src/domain/bundle-skills-router.ts: added targetRootResolved =
  nodePath.resolve(input.targetSkillsDir) at the top of routeSkills.
  After computing targetAbs (from declaredTrimmed under
  targetSkillsDir), check that targetAbs equals targetRootResolved OR
  starts with targetRootResolved + nodePath.sep. If neither holds,
  reject with status=unsafe and detail mentioning target-library
  escape. Source-containment check unchanged.
- test/bundle-skills-router.test.ts: 1 new test R7-B1 — declared
  skills/../outside/SKILL.md, source exists under bundleRoot via the
  resolved path (so source-containment passes), but target after
  prefix-strip escapes target dir. Asserts status=unsafe + detail
  contains "escapes target skills library" + NO writes occurred
  outside target. Also added node:path import for the test.

Verified:
- 8/8 skills-router suite green post-fix (was 7/7 baseline; +1 new
  B1-repair test)
- Discriminator confirmed: removing the new target-containment if-
  block causes 1/8 to fail (R7-B1 specifically); restoring returns
  8/8. The 7 prior tests stay green either way because none of them
  exercise the target-escape scenario.
- scope-and-versioning + audit-every-layer unchanged

Pre-commit + handoff-claim disciplines both applied: git diff
--cached --stat verified scope BEFORE commit; discriminator-flip ran
BEFORE writing the "tests reject target-escape" claim.

Diff: 2 files, +29 / -2 lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

packages/daemon/src/domain/bundle-skills-router.ts

@@ -68,13 +68,15 @@ export interface RouteSkillsResult {
 export function routeSkills(input: RouteSkillsInput, fs: SkillsRouterFsOps): RouteSkillsResult {
   const records: RoutedSkillRecord[] = [];
   const bundleRootResolved = nodePath.resolve(input.bundleRoot);
+  const targetRootResolved = nodePath.resolve(input.targetSkillsDir);
   fs.mkdirp(input.targetSkillsDir);
 
   for (const declared of input.declaredSkills) {
     const sourceAbs = nodePath.resolve(input.bundleRoot, declared);
-    // Defense-in-depth path-containment (mirrors bundle-source-resolver pattern;
-    // the manifest validator already rejects unsafe paths via isRelativeSafePath
-    // but we re-check here in case the input bypassed validation upstream).
+    // Defense-in-depth path-containment on SOURCE (mirrors bundle-source-
+    // resolver pattern; the manifest validator already rejects unsafe paths
+    // via isRelativeSafePath but we re-check here in case the input bypassed
+    // validation upstream).
     if (sourceAbs !== bundleRootResolved && !sourceAbs.startsWith(bundleRootResolved + nodePath.sep)) {
       records.push({
         declaredPath: declared,
@@ -98,6 +100,23 @@ export function routeSkills(input: RouteSkillsInput, fs: SkillsRouterFsOps): Rou
     // root of the operator's skill tree.
     const declaredTrimmed = declared.startsWith("skills/") ? declared.slice("skills/".length) : declared;
     const targetAbs = nodePath.resolve(input.targetSkillsDir, declaredTrimmed);
+    // Defense-in-depth path-containment on TARGET (B1 repair on
+    // qitem-20260518215234-f84fff45). The leading "skills/" strip can promote
+    // a relative segment that looks safe under bundleRoot (e.g.
+    // "skills/../outside/SKILL.md" passes source-containment because the bundle
+    // tree may contain "outside/SKILL.md", but stripping yields
+    // "../outside/SKILL.md" which would escape targetSkillsDir). Reject if the
+    // target resolves outside the target library. Banked lesson:
+    // feedback_pre_existing_trust_boundary_reuse_canonical_helper — contain
+    // BOTH source and target when handling untrusted path input.
+    if (targetAbs !== targetRootResolved && !targetAbs.startsWith(targetRootResolved + nodePath.sep)) {
+      records.push({
+        declaredPath: declared,
+        status: "unsafe",
+        detail: `skill target path for '${declared}' escapes target skills library; rejected`,
+      });
+      continue;
+    }
     fs.mkdirp(nodePath.dirname(targetAbs));
     const content = fs.readFile(sourceAbs);
     fs.writeFile(targetAbs, content);

packages/daemon/test/bundle-skills-router.test.ts

@@ -1,4 +1,5 @@
 import { describe, it, expect } from "vitest";
+import nodePath from "node:path";
 import { routeSkills, type SkillsRouterFsOps, type RouteSkillsInput } from "../src/domain/bundle-skills-router.js";
 
 // Item 6 / slice-05 Checkpoint 7.2: bundle-skills-router pure-function tests.
@@ -116,7 +117,34 @@ describe("routeSkills", () => {
     expect(result.records[2]!.status).toBe("unsafe");
   });
 
-  // R7: non-"skills/" prefixed declared path is honored as-is (no leading strip)
+  // R7-B1: target-side path containment (B1 repair on
+  // qitem-20260518215234-f84fff45). Declared path "skills/../outside/SKILL.md"
+  // passes SOURCE containment (resolves under bundleRoot since the leading
+  // "skills/" segment is consumed before ../) but after the leading "skills/"
+  // strip becomes "../outside/SKILL.md" which would escape targetSkillsDir.
+  // Must be rejected.
+  it("declared path that would escape target skills dir after prefix strip is rejected", () => {
+    // Source file exists under bundleRoot (passes source containment) but the
+    // resolved target after strip escapes target dir.
+    const fs = mockFs({
+      // Source is reachable from bundleRoot via "skills/../outside/SKILL.md"
+      // which resolves to "<bundleRoot>/outside/SKILL.md".
+      [`${BUNDLE_ROOT}/outside/SKILL.md`]: "would-escape-target",
+    });
+    const result = routeSkills(
+      makeInput({ declaredSkills: ["skills/../outside/SKILL.md"] }),
+      fs,
+    );
+    expect(result.routedCount).toBe(0);
+    expect(result.rejectedCount).toBe(1);
+    expect(result.records[0]!.status).toBe("unsafe");
+    expect(result.records[0]!.detail).toContain("escapes target skills library");
+    // Crucially: no write happened outside target.
+    expect(fs._written.has(`${TARGET}/../outside/SKILL.md`)).toBe(false);
+    expect(fs._written.has(nodePath.resolve(`${TARGET}/../outside/SKILL.md`))).toBe(false);
+  });
+
+  // R8: non-"skills/" prefixed declared path is honored as-is (no leading strip)
   it("declared path without leading skills/ prefix routes verbatim", () => {
     const fs = mockFs({
       [`${BUNDLE_ROOT}/custom/path/X.md`]: "x",