Skip to content

fix(skills): accept owner-prefixed clawhub slugs (#71767)#71857

Draft
MkDev11 wants to merge 15 commits intoopenclaw:mainfrom
MkDev11:fix/issue-71767-owner-prefixed-skill-slug
Draft

fix(skills): accept owner-prefixed clawhub slugs (#71767)#71857
MkDev11 wants to merge 15 commits intoopenclaw:mainfrom
MkDev11:fix/issue-71767-owner-prefixed-skill-slug

Conversation

@MkDev11
Copy link
Copy Markdown
Contributor

@MkDev11 MkDev11 commented Apr 26, 2026

Summary

  • Problem: openclaw skills install spclaudehome/skill-vetter failed with Invalid skill slug: spclaudehome/skill-vetter even though owner-prefixed ClawHub references use that form.
  • Why it matters: Users who copy-paste or reuse owner-prefixed ClawHub references hit a hard error and have no signal that they should drop the owner prefix. The accepted reference shape and the CLI disagreed.
  • What changed: New local parser parseClawHubSkillSlug(raw, mode) accepts an optional <owner>/<name> prefix, validates both segments under the existing ASCII guard, and returns the canonical bare slug for the registry call, install dir, and lockfile key. normalizeTrackedSlug / validateRequestedSlug are thin wrappers; call sites unchanged.
  • What did NOT change (scope boundary): No changes to src/infra/clawhub.ts registry I/O, src/cli/skills-cli.ts, the gateway skills.install / skills.update RPCs, the plugin install path, or src/infra/install-safe-path.ts. Path-traversal protection is unchanged (safeDirName still neutralizes / and \ at the FS layer). Homograph rejection from fix: tighten skill slug validation to ASCII-only #53206 is preserved per-segment.

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

Root Cause (if applicable)

  • Root cause: normalizeTrackedSlug in src/agents/skills-clawhub.ts:70 rejected any input containing / as a path-traversal guard. Earlier issue analysis found owner-prefixed ClawHub install references such as openclaw skills install spclaudehome/skill-vetter. The CLI did not understand that form, so copy-paste installs failed before any registry request.
  • Missing detection / guardrail: No test in src/agents/skills-clawhub.test.ts exercised slugs with a / in them. PR fix: tighten skill slug validation to ASCII-only #53206's "backward compatible? Yes — slugs that were valid before remain valid" claim was technically true for single-segment slugs, but missed that the website was already shipping owner-prefixed install commands.
  • Contributing context (if known): The original slug.includes("/") rejection landed in the very first commit of this file (91b2800241) as a path-traversal guard. PR fix(clawhub): sanitize archive temp filenames #56779 later added safeDirName at the archive layer (src/infra/install-safe-path.ts:28), making the validator's /-rejection redundant for safety. The validator never got the memo.

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: src/agents/skills-clawhub.test.ts
  • Scenario the test should lock in: installSkillFromClawHub({ slug: "spclaudehome/skill-vetter" }) returns { ok: true } and the downstream fetchClawHubSkillDetail / downloadClawHubSkillArchive calls receive slug: "skill-vetter". updateSkillsFromClawHub against a workspace that tracks the bare skill-vetter resolves the owner-prefixed input to the same lockfile entry. All malformed forms (a/b/c, /x, x/, a\\b, a/../b, non-ASCII per segment, hyphen edges) still return { ok: false, error: containing "Invalid skill slug" } without touching the network.
  • Why this is the smallest reliable guardrail: All slug entry points (CLI install, CLI update, gateway skills.install/skills.update) converge through installSkillFromClawHub / updateSkillsFromClawHub and the two normalizers. Asserting at that boundary covers every caller without integration tests.
  • Existing test that already covers this (if any): None. The existing 9 cases at skills-clawhub.test.ts:224-329 only exercised single-segment slugs.
  • If no new test is added, why not: 12 new test cases added (one acceptance + one mixed-case + nine rejection + one cross-form lockfile lookup).

User-visible / Behavior Changes

  • openclaw skills install <owner>/<slug> and openclaw skills update <owner>/<slug> are now accepted and behave identically to the bare-slug form — the owner prefix is informational and stripped before any further processing.
  • The lockfile and the skill install directory are still keyed by the bare slug, so installing via either form (and updating from either form) targets the same entry. Inputs without a / are unchanged.

Diagram (if applicable)

Before:
"openclaw skills install spclaudehome/skill-vetter"
  -> validateRequestedSlug
  -> normalizeTrackedSlug rejects on slug.includes("/")
  -> Error: "Invalid skill slug: spclaudehome/skill-vetter"

After:
"openclaw skills install spclaudehome/skill-vetter"
  -> validateRequestedSlug
  -> parseClawHubSkillSlug(raw, "requested")
       split "/" -> ["spclaudehome", "skill-vetter"]
       each segment passes ASCII + VALID_SLUG_PATTERN
  -> "skill-vetter"
  -> fetchClawHubSkillDetail({ slug: "skill-vetter" })  // /api/v1/skills/skill-vetter -> 200
  -> install at <workspace>/skills/skill-vetter, lock.skills["skill-vetter"]

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No (the bare slug is what the registry already expected; the owner prefix is stripped before any request)
  • Command/tool execution surface changed? No
  • Data access scope changed? No
  • If any Yes, explain risk + mitigation: N/A. Homograph protection from fix: tighten skill slug validation to ASCII-only #53206 is preserved by applying NON_ASCII_PATTERN and VALID_SLUG_PATTERN to both segments in requested mode; path-traversal protection is preserved by rejecting \\, .., empty segments, and >1 separator before the registry/filesystem layers.

Repro + Verification

Environment

  • OS: Linux 6.8 (Ubuntu 22.04 in the issue; reproduced locally on Debian-derived host)
  • Runtime/container: Node 22.22.1, pnpm 10.33.0
  • Model/provider: N/A
  • Integration/channel (if any): N/A
  • Relevant config (redacted): default — no ClawHub credentials needed to reproduce or verify

Steps

  1. pnpm install
  2. Reproduce on main (pre-fix): pnpm test src/agents/skills-clawhub.test.ts -- --testNamePattern="owner-prefixed" → fails (3 critical assertions).
  3. Apply this branch and re-run: pnpm test src/agents/skills-clawhub.test.ts → 27/27 pass (15 baseline + 12 new).
  4. Smart gate: pnpm check:changed → conflict-markers ✓, typecheck core ✓, typecheck core tests ✓, lint core ✓, all guards ✓, 4542 tests ✓.

Expected

  • Owner-prefixed slug installs end-to-end and the registry call uses the bare slug.
  • Bare slug behavior is unchanged.
  • Malformed forms (multiple slashes, empty segments, backslash, traversal, non-ASCII per segment, hyphen edges) still error out before any network call.

Actual

  • Matches expected on all three rows.

Evidence

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

Regression proof — running the new tests against unfixed source produced exactly the failures predicted by the analysis:

FAIL  src/agents/skills-clawhub.test.ts > owner-prefixed slugs match ClawHub references > strips the owner prefix and installs the bare slug
  AssertionError: expected "vi.fn()" to be called with arguments: [ { slug: 'skill-vetter', baseUrl: undefined } ]
  Number of calls: 0

FAIL  > accepts mixed-case owner-prefixed slugs
  AssertionError: expected "vi.fn()" to be called with arguments: [ { slug: 'Skill-Vetter', ...

FAIL  > updates a bare-slug install when invoked with the owner-prefixed form
  Error: Invalid skill slug: spclaudehome/skill-vetter
   ❯ normalizeTrackedSlug src/agents/skills-clawhub.ts:73:11
   ❯ resolveRequestedUpdateSlug src/agents/skills-clawhub.ts:91:23
   ❯ updateSkillsFromClawHub src/agents/skills-clawhub.ts:420:15

Tests  3 failed | 24 passed (27)

After the fix on the same test file: Tests 27 passed (27).

Historical context from earlier issue analysis identified the owner-prefixed web snippet shape:

function Bq(e, t, n) {                         // e = ownerHandle, t = ownerId, n = slug
  let r = e?.trim();
  return r ? `${r}/${n}` : t ? `${String(t)}/${n}` : n;
}
function Hq(e, t, n) {
  return `openclaw skills install ${Bq(e, t, n)}`;
}

Registry confirmation: GET /api/v1/skills/skill-vetter → 200 with slug: "skill-vetter", owner.handle: "spclaudehome". GET /api/v1/skills/spclaudehome%2Fskill-vetter → 404. Bare slug is canonical at the API; owner prefix is a UI affordance.

Human Verification (required)

  • Verified scenarios:
    • Install with owner-prefix: installSkillFromClawHub({ slug: "spclaudehome/skill-vetter" }) calls fetchClawHubSkillDetail({ slug: "skill-vetter" }) and returns ok: true with slug: "skill-vetter".
    • Update cross-form: workspace tracks skill-vetter; updateSkillsFromClawHub({ slug: "spclaudehome/skill-vetter" }) resolves to that lockfile entry and uses the saved registry URL.
    • Mixed-case owner-prefix preserved end-to-end.
    • Each rejection edge case fails fast with the existing Invalid skill slug: <raw> error, no network call.
    • Legacy non-ASCII tracked-slug update path (existing test at skills-clawhub.test.ts:160) still passes — tracked mode keeps lenient ASCII handling.
  • Edge cases checked: empty input, whitespace-only input, .. traversal, backslash, /skill, skill/, //, a/b/c, leading hyphen on either segment, trailing hyphen on either segment, Cyrillic homograph in either segment, mixed case, single-segment regression.
  • What I did not verify: gateway RPC path against a live gateway instance (the gateway forwards slug unchanged to installSkillFromClawHub, so the boundary fix covers it). I did not exercise a real network round-trip against clawhub.ai; the API contract was confirmed via direct curl to /api/v1/skills/skill-vetter only.

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 — slugs that were valid before remain valid. Newly-accepted owner-prefixed forms canonicalize to the same bare slug used by existing single-segment installs, so install dirs and lockfile keys never change.
  • Config/env changes? No
  • Migration needed? No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: A user who previously installed react (bare) and runs update someowner/react might be confused if they think someowner/react is a different skill.
    • Mitigation: Slugs are globally unique on ClawHub (registry serves a single skill per slug regardless of owner; verified at /api/v1/skills/<slug>). The owner prefix is purely informational, matching the website's own treatment.
  • Risk: Future ClawHub schema change adds owner-namespaced slugs (so owner/name and name would refer to different skills).
    • Mitigation: Localized to one helper. If the schema changes, the parser is the single place to update; lockfile/install-dir keying lives in the same file.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Apr 26, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 26, 2026

Greptile Summary

This PR fixes openclaw skills install <owner>/<slug> failing with Invalid skill slug by introducing parseClawHubSkillSlug(raw, mode), which accepts the owner/name form clawhub.ai renders in its install snippets, validates both segments, and returns the canonical bare slug. The refactor is backward-compatible, all prior single-segment slugs remain valid, and the 12 new test cases thoroughly cover both the acceptance path and the full set of rejection edge cases.

Confidence Score: 4/5

Safe to merge — the change is narrowly scoped to slug parsing, security invariants (path-traversal guard, homograph rejection) are preserved per-segment, and the install/lockfile layers are untouched.

No P0 or P1 issues found. The parser correctly strips the owner prefix before any registry or filesystem call, both validation modes are tested end-to-end, and the fallback in resolveRequestedUpdateSlug handles all update-path combinations correctly.

No files require special attention.

Reviews (1): Last reviewed commit: "fix(skills): accept owner-prefixed clawh..." | Re-trigger Greptile

@MkDev11 MkDev11 force-pushed the fix/issue-71767-owner-prefixed-skill-slug branch 2 times, most recently from 19e299b to 1251fd1 Compare April 26, 2026 09:00
@MkDev11 MkDev11 force-pushed the fix/issue-71767-owner-prefixed-skill-slug branch 2 times, most recently from 3e2c20e to 5c5fa26 Compare April 26, 2026 12:30
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 26, 2026

Codex review: needs maintainer review before merge.

Summary
The PR accepts optional owner/slug ClawHub skill references, canonicalizes them to the bare slug for install/update, adds focused regression tests, and adds a changelog entry.

Reproducibility: yes. Current main reproduces from source: openclaw skills install spclaudehome/skill-vetter passes the raw slug into validateRequestedSlug, which calls normalizeTrackedSlug and rejects / before any registry call.

Next step before merge
The remaining action is normal PR handling: the branch is draft with failing exact-head CI, and I did not find a narrow source defect for an automated repair PR.

Security
Cleared: The diff only changes local slug parsing, unit tests, and changelog text, with no new dependency, workflow, secret, package, lifecycle-script, or artifact-execution surface.

Review details

Best possible solution:

Keep the parser and regression tests, keep the changelog entry under current Unreleased Fixes after rebase, and land once the draft state and exact-head checks are resolved.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main reproduces from source: openclaw skills install spclaudehome/skill-vetter passes the raw slug into validateRequestedSlug, which calls normalizeTrackedSlug and rejects / before any registry call.

Is this the best way to solve the issue?

Yes. The PR fixes the shared install/update boundary instead of duplicating CLI or RPC parsing, validates both owner and slug segments, and canonicalizes before registry/filesystem use.

Acceptance criteria:

  • pnpm test src/agents/skills-clawhub.test.ts
  • pnpm check:changed in Testbox before maintainer handoff if required

What I checked:

  • Current main rejects slash-containing requested slugs: normalizeTrackedSlug trims input and rejects any slug containing /; validateRequestedSlug calls it before install or update reaches ClawHub. (src/agents/skills-clawhub.ts:69, 5e1529c48bf1)
  • CLI forwards raw install/update slug to shared helpers: openclaw skills install passes the user argument as slug to installSkillFromClawHub, and update passes the optional slug to updateSkillsFromClawHub, so the shared helper is the right boundary. (src/cli/skills-cli.ts:150, 5e1529c48bf1)
  • PR head implements canonical owner-prefix parsing: The PR-head helper validates requested-mode owner and skill segments, rejects traversal/backslash/empty/multiple separators, and returns the last segment as the canonical slug before registry/filesystem work. (src/agents/skills-clawhub.ts:70, b5c34e504414)
  • PR head adds focused regression coverage: The tests cover owner-prefixed install, mixed-case install, malformed slash/backslash/traversal/non-ASCII/hyphen rejection, invalid update prefixes, and updating an existing bare-slug install via owner-prefixed input. (src/agents/skills-clawhub.test.ts:331, b5c34e504414)
  • Changelog placement rechecked on PR head: The added Skills/ClawHub bullet is at PR-head line 599 under the ## Unreleased / ### Fixes section, so the earlier changelog-placement finding is stale. (CHANGELOG.md:599, b5c34e504414)
  • Exact-head CI is not merge-ready: GitHub check-runs for the exact PR head include failures in checks-node-core, check-additional, and checks-node-agentic-cli, while the PR is still draft. (b5c34e504414)

Likely related people:

  • steipete: History shows Peter Steinberger introduced the native ClawHub install flow, split tracked ClawHub update flows, and has recent maintenance touches on the same skills code path. (role: introduced behavior and recent maintainer; confidence: high; commits: 91b2800241c1, 38137b0cf87b, 84c85734a813; files: src/agents/skills-clawhub.ts, src/agents/skills-clawhub.test.ts, src/cli/skills-cli.ts)
  • drobison00: The related ASCII-only slug hardening changed the same ClawHub skill validation and test surface that this PR extends to owner-prefixed inputs. (role: validation hardening contributor; confidence: medium; commits: 40071ea23ec2, a339d706c1c2, 003752b9b315; files: src/agents/skills-clawhub.ts, src/agents/skills-clawhub.test.ts)
  • soimy: Related PR fix(clawhub): sanitize archive temp filenames #56779 added safeDirName coverage for slash-containing ClawHub archive identifiers in the adjacent download/install path referenced by this PR. (role: adjacent ClawHub path safety contributor; confidence: medium; commits: eee8e9679e9e; files: src/infra/clawhub.ts, src/infra/install-safe-path.ts)

Remaining risk / open question:

  • The PR is draft and exact-head CI currently has failing check runs, so merge readiness still needs author or maintainer follow-up.
  • The branch is behind current main, so the final changelog placement and changed-gate proof should be rechecked after rebase or merge-queue update.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 5e1529c48bf1.

@MkDev11 MkDev11 force-pushed the fix/issue-71767-owner-prefixed-skill-slug branch 7 times, most recently from 9cfd629 to 655bb27 Compare April 27, 2026 04:48
@openclaw-barnacle openclaw-barnacle Bot added the cli CLI command changes label Apr 27, 2026
@MkDev11 MkDev11 force-pushed the fix/issue-71767-owner-prefixed-skill-slug branch 2 times, most recently from 47b3cd6 to e52e602 Compare April 27, 2026 05:34
@openclaw-barnacle openclaw-barnacle Bot removed the cli CLI command changes label Apr 27, 2026
@MkDev11 MkDev11 force-pushed the fix/issue-71767-owner-prefixed-skill-slug branch from e52e602 to 4e26893 Compare April 27, 2026 07:16
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: msteams Channel integration: msteams labels Apr 27, 2026
MkDev11 added 2 commits April 27, 2026 09:26
The install snippet rendered on clawhub.ai is openclaw skills install <owner>/<slug>,
but the CLI rejected anything with a slash. Parse the owner prefix off and use the
bare slug for the registry call, install dir, and lockfile; preserve the existing
ASCII guard on each segment so PR openclaw#53206's homograph protection still applies.
@MkDev11 MkDev11 force-pushed the fix/issue-71767-owner-prefixed-skill-slug branch from 33f034d to b786f18 Compare April 27, 2026 07:28
@openclaw-barnacle openclaw-barnacle Bot removed docs Improvements or additions to documentation channel: msteams Channel integration: msteams labels Apr 27, 2026
@MkDev11
Copy link
Copy Markdown
Contributor Author

MkDev11 commented Apr 27, 2026

the ci test failures are not related to my pr

…-prefixed-skill-slug

# Conflicts:
#	CHANGELOG.md
@openclaw-barnacle openclaw-barnacle Bot added channel: slack Channel integration: slack app: web-ui App: web-ui gateway Gateway runtime commands Command implementations channel: feishu Channel integration: feishu extensions: deepinfra labels Apr 28, 2026
…er-prefixed-skill-slug

# Conflicts:
#	extensions/deepinfra/index.test.ts
@openclaw-barnacle openclaw-barnacle Bot removed channel: slack Channel integration: slack app: web-ui App: web-ui gateway Gateway runtime commands Command implementations channel: feishu Channel integration: feishu labels Apr 28, 2026
@MkDev11
Copy link
Copy Markdown
Contributor Author

MkDev11 commented May 1, 2026

@greptileai pls review again

@MkDev11
Copy link
Copy Markdown
Contributor Author

MkDev11 commented May 1, 2026

@greptile-apps

@openclaw-barnacle openclaw-barnacle Bot added channel: line Channel integration: line commands Command implementations labels May 1, 2026
@openclaw-barnacle openclaw-barnacle Bot removed channel: line Channel integration: line commands Command implementations labels May 1, 2026
@MkDev11 MkDev11 marked this pull request as draft May 1, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Invalid skill slug: spclaudehome/skill-vetter

1 participant