feat(integrity): SHA-256 skill integrity verification (#90 phase 1)#122
Open
rohitg00 wants to merge 1 commit into
Open
feat(integrity): SHA-256 skill integrity verification (#90 phase 1)#122rohitg00 wants to merge 1 commit into
rohitg00 wants to merge 1 commit into
Conversation
Ships supply-chain content integrity for the SkillKit ecosystem (phase 1 of issue #90): - New @skillkit/core integrity module computes a deterministic SHA-256 digest over every file in a skill (excluding .skillkit.json metadata, VCS artifacts, editor noise) and emits a Subresource Integrity string (sha256-<base64>). 50 MB safety cap. - New `skillkit verify <path>` CLI with --expected, --against-lock, --files, --format sri|hex, --json. - `skillkit publish` now writes an `integrity` field per skill into .well-known/skills/index.json. - `skillkit install` and `skillkit update` record the full SRI digest into ~/.skillkit/lock.json (graceful fallback to legacy short checksum if integrity computation fails). - Docs: fumadocs security.mdx retitled "Security & Integrity" with a full verify section; commands.mdx + index.mdx updated; website Commands.tsx + Features.tsx surface the new feature and comparison matrix row. - Root CHANGELOG.md added. Phase 1 (content hashing) is the response to the ClawHavoc Q1 2026 supply-chain campaign and OWASP Agentic Skills Top 10 guidance. Phase 2 (ed25519 signing + publisher key registry) is tracked separately. Tests: 11 unit tests for the integrity module, 5 CLI E2E tests for `skillkit verify`. All 25 task suites green; typecheck clean.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces deterministic SHA-256 Subresource Integrity (SRI) verification for skills, including a new ChangesSupply-Chain Integrity Verification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove stray trailing content.
Line 25 appears to contain an orphan
25, which looks like an accidental artifact in the markdown file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` at line 25, Remove the stray orphan token "25" found in the changelog content (it appears as accidental trailing content) by deleting that character so the CHANGELOG.md contains only valid markdown entries; ensure no other extraneous characters remain and validate the file renders correctly after removing the orphan "25".
🧹 Nitpick comments (3)
packages/core/src/integrity/integrity.ts (2)
97-97: ⚡ Quick winConsider using
basenamefor clearer filename extraction.The expression
file.split(sep).pop()!works correctly but usingbasename(file)from the already-importednode:pathmodule would be more idiomatic and eliminate the non-null assertion.♻️ Proposed refactor
Update the import on line 3:
-import { join, relative, sep, posix } from 'node:path'; +import { join, relative, sep, posix, basename } from 'node:path';Then update line 97:
- const rel = stats.isFile() ? toPosix(file.split(sep).pop()!) : toPosix(relative(root, file)); + const rel = stats.isFile() ? toPosix(basename(file)) : toPosix(relative(root, file));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/integrity/integrity.ts` at line 97, The filename extraction in the integrity computation uses file.split(sep).pop()! which is brittle and uses a non-null assertion; replace that with the path.basename utility: import basename from node:path (or add basename to the existing path import) and change the expression in the const rel assignment (the line assigning rel in integrity.ts) to use basename(file) wrapped in toPosix, removing the non-null assertion. This updates the import list to include basename and makes the filename extraction idiomatic and safe.
45-45: ⚡ Quick winConsider locale-independent sorting for full cross-platform determinism.
Using
localeCompare()without a locale argument may produce different sort orders for filenames with non-ASCII characters across different system locales. While ASCII filenames (the common case) sort consistently, specifying a fixed locale ensures identical integrity digests across all environments.♻️ Proposed change for deterministic sorting
- for (const entry of readdirSync(dir, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) { + for (const entry of readdirSync(dir, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name, 'en-US'))) {Or use plain comparison for full independence:
- for (const entry of readdirSync(dir, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) { + for (const entry of readdirSync(dir, { withFileTypes: true }).sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/integrity/integrity.ts` at line 45, The directory-entry sort uses a.name.localeCompare(b.name) which depends on system locale; change the comparator in the sort call (the anonymous comparator passed to readdirSync(...).sort(...)) to a deterministic, locale-independent comparison — either use localeCompare with an explicit fixed locale (e.g., localeCompare(b.name, 'en') or similar) or replace with a plain bytewise comparison like (a.name > b.name) ? 1 : (a.name < b.name) ? -1 : 0 so filename ordering (and resulting integrity digests) is identical across environments.packages/cli/src/__tests__/verify.test.ts (1)
77-81: ⚡ Quick winAssert JSON
reasonfor invalid expected format.This case only checks exit code. Since
reasoncodes are part of the CLI contract, assertinvalid-expected-formathere too.Proposed test tightening
it('rejects garbage --expected', () => { writeFileSync(join(dir, 'SKILL.md'), 'x'); - const r = run(['verify', dir, '--expected', 'garbage']); + const r = run(['verify', dir, '--expected', 'garbage', '--json']); expect(r.code).toBe(1); + const payload = JSON.parse(r.stdout); + expect(payload.ok).toBe(false); + expect(payload.reason).toBe('invalid-expected-format'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/__tests__/verify.test.ts` around lines 77 - 81, The test 'rejects garbage --expected' only checks exit code; update it to also parse the CLI JSON output and assert that the error reason equals "invalid-expected-format". After running run(['verify', dir, '--expected', 'garbage']) and checking r.code === 1, parse r.stdout or r.stderr (whichever the CLI emits JSON to) into an object and assert obj.reason === 'invalid-expected-format' so the test verifies the CLI contract; reference the existing test name and the variable r used to capture the run result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 22: Update the placeholder date in the changelog header "## [1.24.0] -
2026-04-XX": replace "2026-04-XX" with the concrete release date (e.g.,
"2026-04-15") or remove the date portion entirely until the final release so the
entry for version 1.24.0 does not contain an ambiguous "XX" placeholder.
In `@docs/skillkit/components/Commands.tsx`:
- Around line 103-104: In the Commands.tsx examples for the verify command (the
array entries in the Commands component), include the required positional path
argument: change the cmd strings to include a <path> placeholder so they read
like "verify <path> --expected sha256-..." and "verify <path> --against-lock"
(update the two objects with cmd properties accordingly).
In `@packages/cli/src/commands/verify.ts`:
- Around line 93-123: The code currently calls verifySkillIntegrity(target,
expected) which recomputes digests; instead reuse the previously computed
integrity object (computed) from computeSkillIntegrity(target) to determine
validity and metadata so we don't hit disk twice. Change the call site in the
verify flow to either (A) call a new/overloaded verifySkillIntegrity(expected,
computed) that accepts the precomputed result, or (B) replace the call with a
simple comparison using parsed.digest vs computed.digest/SRI to produce the same
result shape (valid, algorithm, computed, reason), and then use that result for
the JSON/console output (ensure you reference parsed, computed, and result
fields consistently).
- Around line 72-92: If the user passed --expected but it is empty/whitespace,
and when parseIntegrity(expected) fails, we must treat that as a user-provided
invalid expected and emit structured JSON when this.json is true. Update the
logic around expectedFromArgs/expected to: detect when this.expected !==
undefined but expectedFromArgs === '' and handle it like an invalid expected
(return 1); and when parsed is falsy, replace the plain error(...) call with
conditional JSON output (this.context.stdout.write(JSON.stringify({ ok: false,
reason: 'invalid-expected-format', target }, null, 2) + '\n')) when this.json is
true, otherwise call error(...) as before; use the existing symbols
expectedFromArgs, expectedFromLock, expected, parseIntegrity, this.json, error,
warn and target to locate and modify the checks.
---
Outside diff comments:
In `@CHANGELOG.md`:
- Line 25: Remove the stray orphan token "25" found in the changelog content (it
appears as accidental trailing content) by deleting that character so the
CHANGELOG.md contains only valid markdown entries; ensure no other extraneous
characters remain and validate the file renders correctly after removing the
orphan "25".
---
Nitpick comments:
In `@packages/cli/src/__tests__/verify.test.ts`:
- Around line 77-81: The test 'rejects garbage --expected' only checks exit
code; update it to also parse the CLI JSON output and assert that the error
reason equals "invalid-expected-format". After running run(['verify', dir,
'--expected', 'garbage']) and checking r.code === 1, parse r.stdout or r.stderr
(whichever the CLI emits JSON to) into an object and assert obj.reason ===
'invalid-expected-format' so the test verifies the CLI contract; reference the
existing test name and the variable r used to capture the run result.
In `@packages/core/src/integrity/integrity.ts`:
- Line 97: The filename extraction in the integrity computation uses
file.split(sep).pop()! which is brittle and uses a non-null assertion; replace
that with the path.basename utility: import basename from node:path (or add
basename to the existing path import) and change the expression in the const rel
assignment (the line assigning rel in integrity.ts) to use basename(file)
wrapped in toPosix, removing the non-null assertion. This updates the import
list to include basename and makes the filename extraction idiomatic and safe.
- Line 45: The directory-entry sort uses a.name.localeCompare(b.name) which
depends on system locale; change the comparator in the sort call (the anonymous
comparator passed to readdirSync(...).sort(...)) to a deterministic,
locale-independent comparison — either use localeCompare with an explicit fixed
locale (e.g., localeCompare(b.name, 'en') or similar) or replace with a plain
bytewise comparison like (a.name > b.name) ? 1 : (a.name < b.name) ? -1 : 0 so
filename ordering (and resulting integrity digests) is identical across
environments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa5b3d0a-b922-4f2b-8202-92b176e89637
📒 Files selected for processing (20)
CHANGELOG.mdREADME.mdapps/skillkit/src/cli.tsdocs/fumadocs/content/docs/commands.mdxdocs/fumadocs/content/docs/index.mdxdocs/fumadocs/content/docs/security.mdxdocs/skillkit/components/Commands.tsxdocs/skillkit/components/Features.tsxpackages/cli/src/__tests__/commands.test.tspackages/cli/src/__tests__/verify.test.tspackages/cli/src/commands/index.tspackages/cli/src/commands/install.tspackages/cli/src/commands/publish.tspackages/cli/src/commands/update.tspackages/cli/src/commands/verify.tspackages/core/src/index.tspackages/core/src/integrity/__tests__/integrity.test.tspackages/core/src/integrity/index.tspackages/core/src/integrity/integrity.tspackages/core/src/providers/wellknown.ts
| - 11 unit tests for the integrity module (`packages/core/src/integrity/__tests__/integrity.test.ts`). | ||
| - 5 CLI end-to-end tests for `skillkit verify` (`packages/cli/src/__tests__/verify.test.ts`). | ||
|
|
||
| ## [1.24.0] - 2026-04-XX |
There was a problem hiding this comment.
Replace placeholder release date before merge.
Line 22 still has 2026-04-XX. For a versioned changelog entry, this should be a concrete date (or removed until release) to avoid ambiguous release metadata.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` at line 22, Update the placeholder date in the changelog header
"## [1.24.0] - 2026-04-XX": replace "2026-04-XX" with the concrete release date
(e.g., "2026-04-15") or remove the date portion entirely until the final release
so the entry for version 1.24.0 does not contain an ambiguous "XX" placeholder.
Comment on lines
+103
to
+104
| { cmd: 'verify --expected sha256-...', desc: 'Match publisher digest' }, | ||
| { cmd: 'verify --against-lock', desc: 'Verify vs lockfile' }, |
There was a problem hiding this comment.
Include the required <path> in verify examples.
These two entries are missing the required positional path argument, so the displayed commands are not executable as shown.
💡 Suggested fix
- { cmd: 'verify --expected sha256-...', desc: 'Match publisher digest' },
- { cmd: 'verify --against-lock', desc: 'Verify vs lockfile' },
+ { cmd: 'verify <path> --expected sha256-...', desc: 'Match publisher digest' },
+ { cmd: 'verify <path> --against-lock', desc: 'Verify vs lockfile' },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { cmd: 'verify --expected sha256-...', desc: 'Match publisher digest' }, | |
| { cmd: 'verify --against-lock', desc: 'Verify vs lockfile' }, | |
| { cmd: 'verify <path> --expected sha256-...', desc: 'Match publisher digest' }, | |
| { cmd: 'verify <path> --against-lock', desc: 'Verify vs lockfile' }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/skillkit/components/Commands.tsx` around lines 103 - 104, In the
Commands.tsx examples for the verify command (the array entries in the Commands
component), include the required positional path argument: change the cmd
strings to include a <path> placeholder so they read like "verify <path>
--expected sha256-..." and "verify <path> --against-lock" (update the two
objects with cmd properties accordingly).
Comment on lines
+72
to
+92
| const expectedFromArgs = this.expected?.trim(); | ||
| const expectedFromLock = this.againstLock ? findLockChecksum(target) : null; | ||
| const expected = expectedFromArgs ?? expectedFromLock ?? null; | ||
|
|
||
| if (this.againstLock && !expectedFromLock) { | ||
| if (this.json) { | ||
| this.context.stdout.write( | ||
| JSON.stringify({ ok: false, reason: 'no-lock-entry', target }, null, 2) + '\n', | ||
| ); | ||
| } else { | ||
| warn(`No lock entry found for ${target}`); | ||
| } | ||
| return 1; | ||
| } | ||
|
|
||
| if (expected) { | ||
| const parsed = parseIntegrity(expected); | ||
| if (!parsed) { | ||
| error(`Invalid --expected value (must be sha256-<base64> or 64-char hex)`); | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
Honor --expected intent and return structured JSON reasons.
If --expected is provided but empty/whitespace, the command currently falls back to compute-only success. Also, invalid expected format emits plain text even with --json, which breaks CI reason-code handling.
💡 Suggested fix
- const expectedFromArgs = this.expected?.trim();
+ const expectedFromArgsRaw = this.expected;
+ const expectedFromArgs = expectedFromArgsRaw?.trim();
+
+ if (expectedFromArgsRaw !== undefined && !expectedFromArgs) {
+ if (this.json) {
+ this.context.stdout.write(
+ JSON.stringify({ ok: false, reason: 'missing-expected', target }, null, 2) + '\n',
+ );
+ } else {
+ error(`Missing --expected value`);
+ }
+ return 1;
+ }
@@
if (expected) {
const parsed = parseIntegrity(expected);
if (!parsed) {
- error(`Invalid --expected value (must be sha256-<base64> or 64-char hex)`);
+ if (this.json) {
+ this.context.stdout.write(
+ JSON.stringify({ ok: false, reason: 'invalid-expected-format', target }, null, 2) + '\n',
+ );
+ } else {
+ error(`Invalid --expected value (must be sha256-<base64> or 64-char hex)`);
+ }
return 1;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/verify.ts` around lines 72 - 92, If the user passed
--expected but it is empty/whitespace, and when parseIntegrity(expected) fails,
we must treat that as a user-provided invalid expected and emit structured JSON
when this.json is true. Update the logic around expectedFromArgs/expected to:
detect when this.expected !== undefined but expectedFromArgs === '' and handle
it like an invalid expected (return 1); and when parsed is falsy, replace the
plain error(...) call with conditional JSON output
(this.context.stdout.write(JSON.stringify({ ok: false, reason:
'invalid-expected-format', target }, null, 2) + '\n')) when this.json is true,
otherwise call error(...) as before; use the existing symbols expectedFromArgs,
expectedFromLock, expected, parseIntegrity, this.json, error, warn and target to
locate and modify the checks.
Comment on lines
+93
to
+123
| const result = verifySkillIntegrity(target, expected); | ||
| if (this.json) { | ||
| this.context.stdout.write( | ||
| JSON.stringify( | ||
| { | ||
| ok: result.valid, | ||
| algorithm: result.algorithm, | ||
| expected: parsed.digest, | ||
| computed: result.computed, | ||
| sri: computed.sri, | ||
| files: computed.files.length, | ||
| totalBytes: computed.totalBytes, | ||
| reason: result.reason, | ||
| }, | ||
| null, | ||
| 2, | ||
| ) + '\n', | ||
| ); | ||
| return result.valid ? 0 : 1; | ||
| } | ||
| if (result.valid) { | ||
| success(`Integrity verified (${computed.files.length} files, ${computed.totalBytes} bytes)`); | ||
| this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`); | ||
| this.context.stdout.write(` ${colors.dim('computed')} ${result.computed}\n`); | ||
| return 0; | ||
| } | ||
| error('Integrity mismatch — skill content has changed since signing'); | ||
| this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`); | ||
| this.context.stdout.write(` ${colors.dim('computed')} ${result.computed}\n`); | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
Avoid recomputing integrity during verification.
verifySkillIntegrity(target, expected) recomputes the digest after computeSkillIntegrity(target) already ran. This adds redundant disk I/O and can make reported fields inconsistent if files change between reads.
💡 Suggested fix
- const result = verifySkillIntegrity(target, expected);
+ const isValid = parsed.digest === computed.digest;
+ const reason = isValid ? undefined : 'mismatch';
if (this.json) {
this.context.stdout.write(
JSON.stringify(
{
- ok: result.valid,
- algorithm: result.algorithm,
+ ok: isValid,
+ algorithm: computed.algorithm,
expected: parsed.digest,
- computed: result.computed,
+ computed: computed.digest,
sri: computed.sri,
files: computed.files.length,
totalBytes: computed.totalBytes,
- reason: result.reason,
+ reason,
},
null,
2,
) + '\n',
);
- return result.valid ? 0 : 1;
+ return isValid ? 0 : 1;
}
- if (result.valid) {
+ if (isValid) {
success(`Integrity verified (${computed.files.length} files, ${computed.totalBytes} bytes)`);
this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`);
- this.context.stdout.write(` ${colors.dim('computed')} ${result.computed}\n`);
+ this.context.stdout.write(` ${colors.dim('computed')} ${computed.digest}\n`);
return 0;
}
error('Integrity mismatch — skill content has changed since signing');
this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`);
- this.context.stdout.write(` ${colors.dim('computed')} ${result.computed}\n`);
+ this.context.stdout.write(` ${colors.dim('computed')} ${computed.digest}\n`);
return 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = verifySkillIntegrity(target, expected); | |
| if (this.json) { | |
| this.context.stdout.write( | |
| JSON.stringify( | |
| { | |
| ok: result.valid, | |
| algorithm: result.algorithm, | |
| expected: parsed.digest, | |
| computed: result.computed, | |
| sri: computed.sri, | |
| files: computed.files.length, | |
| totalBytes: computed.totalBytes, | |
| reason: result.reason, | |
| }, | |
| null, | |
| 2, | |
| ) + '\n', | |
| ); | |
| return result.valid ? 0 : 1; | |
| } | |
| if (result.valid) { | |
| success(`Integrity verified (${computed.files.length} files, ${computed.totalBytes} bytes)`); | |
| this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`); | |
| this.context.stdout.write(` ${colors.dim('computed')} ${result.computed}\n`); | |
| return 0; | |
| } | |
| error('Integrity mismatch — skill content has changed since signing'); | |
| this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`); | |
| this.context.stdout.write(` ${colors.dim('computed')} ${result.computed}\n`); | |
| return 1; | |
| } | |
| const isValid = parsed.digest === computed.digest; | |
| const reason = isValid ? undefined : 'mismatch'; | |
| if (this.json) { | |
| this.context.stdout.write( | |
| JSON.stringify( | |
| { | |
| ok: isValid, | |
| algorithm: computed.algorithm, | |
| expected: parsed.digest, | |
| computed: computed.digest, | |
| sri: computed.sri, | |
| files: computed.files.length, | |
| totalBytes: computed.totalBytes, | |
| reason, | |
| }, | |
| null, | |
| 2, | |
| ) + '\n', | |
| ); | |
| return isValid ? 0 : 1; | |
| } | |
| if (isValid) { | |
| success(`Integrity verified (${computed.files.length} files, ${computed.totalBytes} bytes)`); | |
| this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`); | |
| this.context.stdout.write(` ${colors.dim('computed')} ${computed.digest}\n`); | |
| return 0; | |
| } | |
| error('Integrity mismatch — skill content has changed since signing'); | |
| this.context.stdout.write(` ${colors.dim('expected')} ${parsed.digest}\n`); | |
| this.context.stdout.write(` ${colors.dim('computed')} ${computed.digest}\n`); | |
| return 1; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/verify.ts` around lines 93 - 123, The code
currently calls verifySkillIntegrity(target, expected) which recomputes digests;
instead reuse the previously computed integrity object (computed) from
computeSkillIntegrity(target) to determine validity and metadata so we don't hit
disk twice. Change the call site in the verify flow to either (A) call a
new/overloaded verifySkillIntegrity(expected, computed) that accepts the
precomputed result, or (B) replace the call with a simple comparison using
parsed.digest vs computed.digest/SRI to produce the same result shape (valid,
algorithm, computed, reason), and then use that result for the JSON/console
output (ensure you reference parsed, computed, and result fields consistently).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ships phase 1 of #90 — supply-chain content integrity for SkillKit. Direct response to the Q1 2026 ClawHavoc campaign (341 malicious skills flooding ClawHub in 3 days) and OWASP Agentic Skills Top 10 guidance.
The scanner asks "is this skill dangerous?". The new
verifycommand asks "is this skill what the publisher actually shipped?".What ships
@skillkit/coreintegrity module — deterministic recursive SHA-256 over every file in a skill (paths + content), emitted as a Subresource Integrity stringsha256-<base64>. Skips.skillkit.json, VCS,.DS_Store,node_modules,dist,.turbo. 50 MB safety cap.skillkit verify <path>command with--expected,--against-lock,--files,--format sri|hex,--json. Exit codes 0/1, structuredreasoncodes (mismatch,invalid-expected-format,missing-expected,no-lock-entry) for CI pipelines.skillkit publishwrites anintegrityfield per skill into.well-known/skills/index.json. Downstream consumers can now detect tampering between publish and install.skillkit installandskillkit updaterecord the full SRI digest into~/.skillkit/lock.json(with graceful fallback to legacy short checksum if integrity computation fails).docs/fumadocs/content/docs/security.mdxretitled "Security & Integrity" with a full verify section (digest algorithm, publish/install/lock integration, CI example, phase-2 roadmap pointer)docs/fumadocs/content/docs/commands.mdx+index.mdxupdateddocs/skillkit/components/Commands.tsxadds verify entries to the Security categorydocs/skillkit/components/Features.tsxadds a "Content Integrity" feature card + a new comparison-matrix rowCHANGELOG.mdadded.Why now
ed25519 + content_hash(publisher signing)Phase 2 (ed25519 publisher key registry + signed publishes) is tracked in #90 and follows once this lands.
Test plan
packages/core/src/integrity/__tests__/integrity.test.ts(stable digest, drift detection, file-add detection, SRI/hex parsing, max-bytes guard)packages/cli/src/__tests__/verify.test.ts(SRI emission,--json, match, mismatch, garbage--expected)pnpm test)pnpm typecheckcleanpnpm buildgreenverify,verify --files,verify --json,verify --expected ...(match + mismatch),publish --dry-runconfirmsintegrityfield inindex.json,translate+scanstill workBackwards compatibility
LockFileschema unchanged —checksumfield type stillstring. Existing lockfile entries with the legacy 16-char short hash still load;skillkit verify --against-lockwill reportinvalid-expected-formatfor them, andskillkit updateupgrades them to SRI on next refresh.computeSkillChecksum(short SKILL.md-only hash) is untouched and still drivesskillkit update's fast change-detection compare. The newcomputeSkillIntegrityis additive.WellKnownSkillinterface gains an optionalintegrity?: stringfield — existing consumers that ignore it continue to work.Closes phase 1 of #90.
Summary by CodeRabbit
New Features
skillkit verifycommand to compute and verify skill integrity using SHA-256 hashingDocumentation
Tests