feat(mosaic): add secure skill registration CLI #826

2026-07-17T23:02:29Z

jason.woltje commented

2026-07-17 23:02:29 +00:00

Summary

add mosaic skill register|unregister|list for Claude Code
auto-reconcile canonical skills on wizard finalize and framework update
reject traversal and preserve foreign files/directories/symlinks
harden legacy install linkers to preserve foreign runtime symlinks

Verification

package tests: 69 files / 1325 tests green before final installer hardening
pre-push typecheck, lint, and format gates green
final focused installer-link test was added red-first; shellcheck stopped on a pre-existing SC2115 warning before that focused rerun

Closes #824.

## Summary - add `mosaic skill register|unregister|list` for Claude Code - auto-reconcile canonical skills on wizard finalize and framework update - reject traversal and preserve foreign files/directories/symlinks - harden legacy install linkers to preserve foreign runtime symlinks ## Verification - package tests: 69 files / 1325 tests green before final installer hardening - pre-push typecheck, lint, and format gates green - final focused installer-link test was added red-first; shellcheck stopped on a pre-existing SC2115 warning before that focused rerun Closes #824.

jason.woltje added 3 commits 2026-07-17 23:02:29 +00:00

test(#824 ): define secure skill bridge behavior 1cc45b7deb

feat(#824 ): manage Claude skill registrations f751fc9e1f

feat(#824 ): protect installer skill links

ci/woodpecker/pr/ci Pipeline was successful

Details

e074c7dc69

jason.woltje commented

2026-07-17 23:24:12 +00:00

Independent RoR — MS-lead reviewer lane ms-rev-826 (terra), author lane ms-824 (sol). Author ≠ reviewer PROCESS/LANE-attested (single Gitea account).

Review of Record — VERDICT: REQUEST CHANGES

Reviewed tree (TRUE PROSPECTIVE MERGE): 2fe75a43162bfe6f25c9f8621613d26667bb7dab
Merge-base: 3f77229e · Target main: d3bf5289 · PR head e074c7dc merged onto current main (branch was cut pre-#804; reviewed the materialized merge, not the raw head).

Independent exact-head RoR (author ≠ reviewer; terra lane). Full package suite on the merged tree: 1328/1328 green, lint/typecheck/format PASS. #804×#824 install.sh integration: PASS (no #824 path passes an argument the #804 parser rejects; reseed invocation is no-arg). Name-validation, generic auto-sync, temp-dir test hygiene: PASS.

Blockers (must fix before merge)

MAJOR-1 — foreign-link deletion during auto-sync. packages/mosaic/framework/tools/_scripts/mosaic-sync-skills:245-254 removes a link whose resolved target is anywhere under $MOSAIC_HOME, rather than only links demonstrably Mosaic-owned (target under $MOSAIC_HOME/skills/* or the expected source). A foreign ~/.claude/skills link pointing at $MOSAIC_HOME/foreign-non-skill-target is rm -f'd. The unreadable/unresolvable-link removal at 245-249 also deletes without proving ownership. Focused coverage at skill.spec.ts:245-266 masks this (each foreign link is given a name that also exists in the canonical skills dir, so pruning is skipped). Fix: gate deletion on demonstrable Mosaic ownership; add a genuine foreign-name regression test.

MAJOR-2 — unregister accepts a misdirected Mosaic-root link. packages/mosaic/src/commands/skill.ts:230-233 unlinks any target inside mosaicSkillsDir without requiring the expected source. mosaic skill unregister requested deletes ~/.claude/skills/requested -> ~/.config/mosaic/skills/other. The CLI already labels this misdirected at skill.ts:307-312 yet unregister still deletes it. Fix: require targetPath === resolve(directChild(mosaicSkillsDir, name)) (preserving the intended dangling-link case) before unlinking; add a regression test.

Non-blocking

MINOR-1 — ≥85% new-code coverage is unverified: vitest.config.ts has no coverage provider/threshold and @vitest/coverage-v8 is not installed.
NIT-1 — trailing-dot names (safe.) are accepted; reject for Windows filename-normalization portability.

Remediation is red-first (failing test proving the invariant, then fix). No commits/pushes/mutations were performed during review. Full RoR retained by the MS lead.

**Independent RoR — MS-lead reviewer lane `ms-rev-826` (terra), author lane `ms-824` (sol). Author ≠ reviewer PROCESS/LANE-attested (single Gitea account).** ## Review of Record — VERDICT: REQUEST CHANGES **Reviewed tree (TRUE PROSPECTIVE MERGE):** `2fe75a43162bfe6f25c9f8621613d26667bb7dab` **Merge-base:** `3f77229e` · **Target main:** `d3bf5289` · PR head `e074c7dc` merged onto current main (branch was cut pre-#804; reviewed the materialized merge, not the raw head). Independent exact-head RoR (author ≠ reviewer; terra lane). Full package suite on the merged tree: **1328/1328 green**, lint/typecheck/format PASS. #804×#824 `install.sh` integration: **PASS** (no #824 path passes an argument the #804 parser rejects; reseed invocation is no-arg). Name-validation, generic auto-sync, temp-dir test hygiene: **PASS**. ### Blockers (must fix before merge) **MAJOR-1 — foreign-link deletion during auto-sync.** `packages/mosaic/framework/tools/_scripts/mosaic-sync-skills:245-254` removes a link whose resolved target is anywhere under `$MOSAIC_HOME`, rather than only links demonstrably Mosaic-owned (target under `$MOSAIC_HOME/skills/*` or the expected source). A foreign `~/.claude/skills` link pointing at `$MOSAIC_HOME/foreign-non-skill-target` is `rm -f`'d. The unreadable/unresolvable-link removal at `245-249` also deletes without proving ownership. Focused coverage at `skill.spec.ts:245-266` masks this (each foreign link is given a name that also exists in the canonical skills dir, so pruning is skipped). Fix: gate deletion on demonstrable Mosaic ownership; add a genuine foreign-name regression test. **MAJOR-2 — unregister accepts a misdirected Mosaic-root link.** `packages/mosaic/src/commands/skill.ts:230-233` unlinks any target inside `mosaicSkillsDir` without requiring the expected source. `mosaic skill unregister requested` deletes `~/.claude/skills/requested -> ~/.config/mosaic/skills/other`. The CLI already labels this `misdirected` at `skill.ts:307-312` yet unregister still deletes it. Fix: require `targetPath === resolve(directChild(mosaicSkillsDir, name))` (preserving the intended dangling-link case) before unlinking; add a regression test. ### Non-blocking - **MINOR-1** — ≥85% new-code coverage is unverified: `vitest.config.ts` has no coverage provider/threshold and `@vitest/coverage-v8` is not installed. - **NIT-1** — trailing-dot names (`safe.`) are accepted; reject for Windows filename-normalization portability. Remediation is red-first (failing test proving the invariant, then fix). No commits/pushes/mutations were performed during review. Full RoR retained by the MS lead.

jason.woltje added 2 commits 2026-07-17 23:35:11 +00:00

test(#824 ): cover skill bridge ownership regressions d014a7830a

fix(#824 ): enforce exact skill link ownership

ci/woodpecker/pr/ci Pipeline was successful

Details

2db67ed572

jason.woltje commented

2026-07-17 23:44:56 +00:00

Independent RE-RoR — MS-lead reviewer lane ms-rev-826 (terra), author lane ms-824 (sol). Author ≠ reviewer PROCESS/LANE-attested (single Gitea account). Zero evidence transfer — prior RoR (head e074c7dc) VOID; verified fresh at this head.

Review of Record — VERDICT: APPROVE

Remediation head: 2db67ed5726bc56ad21fe2da9aaa09eb5159761a
Reviewed TRUE PROSPECTIVE MERGE tree: 11b96982206fe386684d29c675edbd90ab3e19b0 (head merged onto current main d3bf5289; merge-base 3f77229e; clean auto-merge, zero conflicts)

Both prior MAJOR blockers are FIXED on this exact merged tree, each proven by a fresh hermetic reproduction AND an independent red-then-green replay:

MAJOR-1 (foreign-link deletion) — FIXED. mosaic-sync-skills:245-251 now canonicalizes lexically (readlink -m), preserves any link whose ownership is unproven, and deletes only a retired link whose target is under $MOSAIC_HOME/skills/* (not merely under $MOSAIC_HOME). Fresh repro: a live foreign link foreign-tool -> $MOSAIC_HOME/foreign-non-skill-target and an unresolvable foreign link both SURVIVE sync while canonical skills link. Regression skill.spec.ts:312-334 uses genuine foreign names; independent RED replay on e074c7dc failed the foreign-name test; current focused run 43/43 green.

MAJOR-2 (misdirected unregister) — FIXED. skill.ts:231-242 now requires targetPath === resolve(directChild(mosaicSkillsDir, name)) before unlinking; a requested -> skills/other link is refused (reported misdirected) while the exact-name dangling link remains removable. Fresh CLI repro confirms both; regression skill.spec.ts:264-272; independent RED replay on e074c7dc failed this exact test; current 43/43 green.

Regression guard — PASS. Full package suite on merged tree: 70/70 files, 1334/1334 tests green. #804×#824 install.sh integration PASS (no #824 path passes an arg #804 rejects; reseed no-arg; install.sh byte-identical to main). Name-validation, generic auto-sync (no hardcoded allowlist), temp-dir test hygiene all PASS. typecheck 42/42, lint 23/23, format PASS.

Prior non-blockers — resolved. Coverage: @vitest/coverage-v8 added + 85% thresholds configured; skill.ts = 91.05% stmts/lines, 86.27% branches, 95.23% funcs (all ≥85%). Trailing-dot names now rejected (skill.ts:91).

No surviving MAJOR/MINOR findings. No commits/pushes/mutations performed during review. Cleared for coordinated-lane merge on the base-drift-at-merge-instant gate (main verified d3bf5289 at merge). Full RoR retained at reviews/pr826-824-merged-RoR.md.

**Independent RE-RoR — MS-lead reviewer lane `ms-rev-826` (terra), author lane `ms-824` (sol). Author ≠ reviewer PROCESS/LANE-attested (single Gitea account). Zero evidence transfer — prior RoR (head e074c7dc) VOID; verified fresh at this head.** ## Review of Record — VERDICT: APPROVE **Remediation head:** `2db67ed5726bc56ad21fe2da9aaa09eb5159761a` **Reviewed TRUE PROSPECTIVE MERGE tree:** `11b96982206fe386684d29c675edbd90ab3e19b0` (head merged onto current main `d3bf5289`; merge-base `3f77229e`; clean auto-merge, zero conflicts) Both prior MAJOR blockers are FIXED on this exact merged tree, each proven by a fresh hermetic reproduction AND an independent red-then-green replay: **MAJOR-1 (foreign-link deletion) — FIXED.** `mosaic-sync-skills:245-251` now canonicalizes lexically (`readlink -m`), preserves any link whose ownership is unproven, and deletes only a retired link whose target is under `$MOSAIC_HOME/skills/*` (not merely under `$MOSAIC_HOME`). Fresh repro: a live foreign link `foreign-tool -> $MOSAIC_HOME/foreign-non-skill-target` and an unresolvable foreign link both SURVIVE sync while canonical skills link. Regression `skill.spec.ts:312-334` uses genuine foreign names; independent RED replay on `e074c7dc` failed the foreign-name test; current focused run 43/43 green. **MAJOR-2 (misdirected unregister) — FIXED.** `skill.ts:231-242` now requires `targetPath === resolve(directChild(mosaicSkillsDir, name))` before unlinking; a `requested -> skills/other` link is refused (reported `misdirected`) while the exact-name dangling link remains removable. Fresh CLI repro confirms both; regression `skill.spec.ts:264-272`; independent RED replay on `e074c7dc` failed this exact test; current 43/43 green. **Regression guard — PASS.** Full package suite on merged tree: **70/70 files, 1334/1334 tests green**. #804×#824 install.sh integration PASS (no #824 path passes an arg #804 rejects; reseed no-arg; install.sh byte-identical to main). Name-validation, generic auto-sync (no hardcoded allowlist), temp-dir test hygiene all PASS. typecheck 42/42, lint 23/23, format PASS. **Prior non-blockers — resolved.** Coverage: `@vitest/coverage-v8` added + 85% thresholds configured; `skill.ts` = 91.05% stmts/lines, 86.27% branches, 95.23% funcs (all ≥85%). Trailing-dot names now rejected (`skill.ts:91`). No surviving MAJOR/MINOR findings. No commits/pushes/mutations performed during review. Cleared for coordinated-lane merge on the base-drift-at-merge-instant gate (main verified `d3bf5289` at merge). Full RoR retained at reviews/pr826-824-merged-RoR.md.

jason.woltje merged commit d801d6c4c8 into main

2026-07-17 23:45:36 +00:00

jason.woltje referenced this issue from a commit

2026-07-17 23:45:37 +00:00

feat(mosaic): add secure skill registration CLI (#826)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#826