← Back to PRs

#23795: Skills: enforce managed skills.lock integrity with allowUnlocked escape hatch

by bmendonca3 open 2026-02-22 18:05 View on GitHub →
agents size: M trusted-contributor
## Summary - add managed-skill integrity lock enforcement using `skills.lock` - require lock entries for default managed (ClawHub) skills and block drifted hashes by default - add `skills.allowUnlocked=true` escape hatch to permit unlocked/drifted managed skills and seed lock entries - add focused tests for missing lock entries, baseline creation, and drift detection ## Why This gives lockfile-style tamper detection for installed managed skills and fails closed by default in the primary managed-skills path. ## Tests - `pnpm vitest run src/agents/skills*.test.ts src/agents/skills/*.test.ts` - `pnpm vitest run src/config/config.skills-allowunlocked.test.ts src/agents/skills/integrity.test.ts` - `pnpm lint` <!-- greptile_comment --> <h3>Greptile Summary</h3> Adds lockfile-style tamper detection for managed skills via `skills.lock`. Managed skills from the default ClawHub directory are now fingerprinted (SHA256 over directory contents), and the fingerprint is compared against a stored `skills.lock` file. Skills missing a lock entry or with drifted hashes are blocked by default. A `skills.allowUnlocked` config escape hatch permits loading unlocked/drifted managed skills and seeds lock entries for newly discovered skills. - New `src/agents/skills/integrity.ts` implements fingerprint computation and lock file management - `shouldIncludeSkill` in `config.ts` gains integrity-based gating with per-skill `integrityFingerprint` override support - `workspace.ts` integrates integrity resolution into the skill loading pipeline, scoped to the default managed skills directory - Config schema, types, help text, and labels updated for the new `skills.allowUnlocked` boolean field - Tests cover missing lock detection, baseline creation, drift detection, and workspace-local skill isolation <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with one minor fingerprint determinism concern to address. - The implementation is well-structured, follows existing patterns (sync FS ops matching workspace.ts), has good test coverage for the core integrity paths, and correctly scopes enforcement to the default managed skills directory. The one concern is `localeCompare` in `computeSkillFingerprint` which could theoretically produce different sort orders across system locales, leading to non-deterministic fingerprints. This is unlikely to cause issues in practice (paths are ASCII) but should use a locale-independent comparison for correctness in a security-sensitive hashing function. - `src/agents/skills/integrity.ts` — the `localeCompare` path sorting in `computeSkillFingerprint` should use locale-independent comparison for deterministic fingerprints. <sub>Last reviewed commit: e39a7f4</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs