PR #64166: fix: copy SKILL.md as hard file instead of symlink to fix security check failure

• Repository: openclaw/openclaw
• Author: sliverp
• State: closed | merged: True
• Link: https://github.com/openclaw/openclaw/pull/64166

Description (problem / solution / changelog)

Fixes #64138

Summary

• Problem: stage-bundled-plugin-runtime.mjs creates symlinks for most files in dist-runtime/, including SKILL.md. At runtime, resolveContainedSkillPath in workspace.ts calls realpathSync() which resolves the symlink to the dist/ directory, failing the path containment security check (assertNoPathAliasEscape).
• Why it matters: All 23 bundled plugin skills (wecom 15, feishu 4, qqbot 3, tavily 1) were silently skipped at load time, making them completely unavailable to users.
• What changed: Added relativePath.endsWith("/SKILL.md") to the shouldCopyRuntimeFile whitelist in scripts/stage-bundled-plugin-runtime.mjs, so SKILL.md files are hard-copied instead of symlinked — matching the existing pattern for package.json, openclaw.plugin.json, and other plugin config files.
• What did NOT change (scope boundary): Other symlinked files (e.g. references/ subdirectories under skills) are not addressed here. They may have a similar issue in non-sandbox mode but are lower priority and tracked separately.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes #64138
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: shouldCopyRuntimeFile only whitelisted package.json and *plugin.json config files for hard copy. SKILL.md was not included, so it was symlinked like all other files. The runtime security check (realpathSync + path containment assertion) correctly rejects paths that resolve outside dist-runtime/, but this meant all plugin skills were silently dropped.
• Missing detection / guardrail: No test or build-time validation exists to verify that skill files are actually loadable after staging. The skip is logged at debug level only, making it easy to miss.
• Contributing context: The symlink optimization in the staging script is intentional for saving disk space, but the security boundary check was added later without updating the copy whitelist for skill-critical files.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: A build validation test that runs find dist-runtime -name SKILL.md -type l and asserts zero symlinks.
• Scenario the test should lock in: After stage-bundled-plugin-runtime, all SKILL.md files in dist-runtime/ must be regular files, not symlinks.
• Why this is the smallest reliable guardrail: It catches the exact failure mode (symlink → realpathSync escape) at build time before any runtime behavior is affected.
• If no new test is added, why not: This is a one-line whitelist addition with straightforward behavior. The fix was verified manually (see Evidence below). A CI check can be added as a follow-up.

User-visible / Behavior Changes

• Before: pnpm openclaw skills list showed 8/73 skills ready. All 23 bundled plugin skills were silently skipped.
• After: pnpm openclaw skills list shows 31/73 skills ready. All 23 bundled plugin skills (wecom, feishu, qqbot, tavily) load correctly.

Diagram (if applicable)

Before:
dist-runtime/extensions/wecom/skills/*/SKILL.md (symlink → ../../dist/extensions/...)
  → realpathSync() resolves to dist/extensions/...
  → assertNoPathAliasEscape fails (outside dist-runtime/)
  → skill skipped

After:
dist-runtime/extensions/wecom/skills/*/SKILL.md (regular file, hard copy)
  → realpathSync() resolves within dist-runtime/
  → security check passes
  → skill loaded successfully

Security Impact (required)

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No
• Command/tool execution surface changed? No — skills were always designed to be loaded; this fix restores intended behavior.
• Data access scope changed? No

Repro + Verification

Environment

• OS: Linux (dev container)
• Runtime/container: Node.js via pnpm
• Model/provider: N/A (build script issue)
• Integration/channel: wecom, feishu, qqbot, tavily plugins

Steps

1. Build openclaw with pnpm build
2. Check dist-runtime/ for SKILL.md files: find dist-runtime -name SKILL.md -exec file {} \;
3. Run pnpm openclaw skills list

Expected

• All SKILL.md files are regular files
• Plugin skills (wecom, feishu, qqbot, tavily) appear as "ready"

Actual (before fix)

• All SKILL.md files were symlinks
• 23 plugin skills skipped, only 8/73 ready

Evidence

• Before fix: find dist-runtime -name SKILL.md — all 29 files were symlinks; skills list showed 8/73 ready; debug log showed 23 skills skipped with "Skipping skill — resolved path escapes plugin directory"
• After fix: find dist-runtime -name SKILL.md -exec file {} \; — all 29 files reported as "regular file"; skills list showed 31/73 ready (8 base + 23 plugin skills restored)
• Tavily log confirmation: Configured tavily plugin separately to confirm its single skill was also affected and recovered after fix.

Human Verification (required)

• Verified scenarios: Full rebuild → find confirmed no SKILL.md symlinks remain → skills list confirmed 31/73 ready → individual plugin skills (wecom, feishu, qqbot, tavily) all present
• Edge cases checked: Tavily (single-skill plugin) verified separately to confirm fix applies universally, not just to multi-skill plugins
• What I did not verify: references/ subdirectory symlinks (tracked as separate follow-up); gateway hot-reload behavior (requires process restart)

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

• Backward compatible? Yes
• Config/env changes? No
• Migration needed? No

Risks and Mitigations

• Risk: references/ and other auxiliary files under skill directories remain as symlinks, which may fail path containment checks in non-sandbox mode.
  • Mitigation: This is pre-existing behavior unrelated to this change. Tracked for separate follow-up. Sandbox mode (the common case) resolves symlinks via fsp.cp and is unaffected.

## Changed files

- `scripts/stage-bundled-plugin-runtime.mjs` (modified, +2/-1)


---

# PR #64176: fix(skills): copy staged SKILL.md into dist-runtime

- Repository: openclaw/openclaw
- Author: LiuHuaize
- State: closed | merged: False
- Link: https://github.com/openclaw/openclaw/pull/64176

## Description (problem / solution / changelog)

## Summary

- Problem: `scripts/stage-bundled-plugin-runtime.mjs` staged plugin `SKILL.md` files as symlinks inside `dist-runtime`, so `realpath()` resolved them back into `dist/`.
- Why it matters: plugin skills from enabled extensions were skipped by the existing containment check in `src/agents/skills/workspace.ts` and never loaded at runtime.
- What changed: `SKILL.md` is now treated as a copy-only runtime artifact, and the staging test suite now covers both the direct copy regression and the existing Windows symlink-fallback path.
- What did NOT change (scope boundary): this PR does not relax or modify runtime containment/security logic.

## Change Type (select all)

- [x] Bug fix
- [ ] Feature
- [ ] Refactor required for the fix
- [ ] Docs
- [ ] Security hardening
- [ ] Chore/infra

## Scope (select all touched areas)

- [ ] Gateway / orchestration
- [x] Skills / tool execution
- [ ] Auth / tokens
- [ ] Memory / storage
- [ ] Integrations
- [ ] API / contracts
- [ ] UI / DX
- [x] CI/CD / infra

## Linked Issue/PR

- Closes #64138
- Related #64138
- [x] This PR fixes a bug or regression

## Root Cause (if applicable)

- Root cause: the build overlay created `dist-runtime/extensions/*/skills/*/SKILL.md` as symlinks, but the runtime loader validates skill paths after `realpath()`. That resolved the skill file into `dist/`, so the existing `isPathInside(rootRealPath, candidateRealPath)` check correctly rejected it as outside `dist-runtime`.
- Missing detection / guardrail: the staging tests covered Windows symlink fallback, but not the steady-state requirement that `SKILL.md` must remain physically inside `dist-runtime` for the runtime containment check.
- Contributing context (if known): the issue reproduces for any enabled bundled plugin that declares `skills`.

## Regression Test Plan (if applicable)

- Coverage level that should have caught this:
  - [x] Seam / integration test
  - [ ] Unit test
  - [ ] End-to-end test
  - [ ] Existing coverage already sufficient
- Target test or file: `test/scripts/stage-bundled-plugin-runtime.test.ts`
- Scenario the test should lock in: staging copies `SKILL.md` into `dist-runtime` while leaving unrelated skill assets symlinked.
- Why this is the smallest reliable guardrail: it exercises the exact build overlay behavior that feeds the runtime loader without widening scope into runtime security code.
- Existing test that already covers this (if any): the Windows fallback test in the same file still covers copy-on-symlink-failure for non-skill assets.
- If no new test is added, why not: N/A

## User-visible / Behavior Changes

Plugin skills from enabled bundled extensions now load again after build/runtime staging because their `SKILL.md` files stay within `dist-runtime`.

## Diagram (if applicable)

```text
Before:
[build stage] -> [dist-runtime/.../SKILL.md symlink] -> [realpath -> dist/.../SKILL.md] -> [containment check rejects skill]

After:
[build stage] -> [dist-runtime/.../SKILL.md copied file] -> [realpath stays under dist-runtime] -> [skill loads]

Security Impact (required)

• New permissions/capabilities? (Yes/No) No
• Secrets/tokens handling changed? (Yes/No) No
• New/changed network calls? (Yes/No) No
• Command/tool execution surface changed? (Yes/No) No
• Data access scope changed? (Yes/No) No
• If any Yes, explain risk + mitigation:

Repro + Verification

Environment

• OS: macOS
• Runtime/container: local Node 22 + pnpm workspace
• Model/provider: N/A
• Integration/channel (if any): bundled plugin skills staging
• Relevant config (redacted): N/A

Steps

1. Create a staged plugin skill under dist/extensions/<plugin>/skills/<skill>/SKILL.md.
2. Run stageBundledPluginRuntime.
3. Inspect the corresponding file under dist-runtime/extensions/<plugin>/skills/<skill>/SKILL.md.

Expected

• SKILL.md is copied into dist-runtime, so realpath() remains under the runtime root.

Actual

• Verified in the new regression test; non-skill assets still symlink as before.

Evidence

Attach at least one:

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Human Verification (required)

• Verified scenarios: pnpm test test/scripts/stage-bundled-plugin-runtime.test.ts; pnpm build
• Edge cases checked: Windows-style symlink failure still falls back to copying regular skill assets.
• What you did not verify: I did not broaden this PR into the unrelated pnpm check failure currently present on origin/main under extensions/discord/src/components*.ts.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

• Backward compatible? (Yes/No) Yes
• Config/env changes? (Yes/No) No
• Migration needed? (Yes/No) No
• If yes, exact upgrade steps:

Risks and Mitigations

• Risk: copying SKILL.md slightly increases dist-runtime duplication for plugin skills.
  • Mitigation: scope stays minimal to a small markdown file per skill; all other non-skill assets keep the previous symlink behavior.

Changed files

• scripts/stage-bundled-plugin-runtime.mjs (modified, +3/-0)
• test/scripts/stage-bundled-plugin-runtime.test.ts (modified, +42/-5)