diff --git a/CHANGELOG b/CHANGELOG index 2a0e6f9e..a114eddf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,15 @@ ### Breaking Changes +- **Content digests now have a versioned form, `sha256-tree-v2:`.** Every `checksum` in + `skills.lock` and `global-skills.lock`, every bundle release and member digest, and every new + bundle artifact is written in this form. Digests written by earlier releases (64 bare hex + characters) are still accepted when they match, with a one-time warning, and are replaced the + next time fastskill rewrites that record; `project install --lock` keeps working without a + manual edit. Earlier releases cannot verify the new form and report a checksum mismatch, so + upgrade every machine and CI job that shares a project. File names that are not valid UTF-8 are + now refused. See [ADR-0017](docs/adr/0017-versioned-content-digests.md). + - **`skill-project.toml` is now written with `schema_version = "2"`.** In schema 2 every git origin `url` is a plain clone URL: the branch or tag lives only in `ref`, the subdirectory only in `subdir`. Manifests that declare `"1"` — or declare nothing — are still read and are @@ -97,6 +106,13 @@ ### Fixed +- **Two different skill directories could share a content digest.** The digest hashed each + file's contents without their length, so the boundary between one file and the next was + ambiguous, and content that differed from what a Lock or bundle pinned could pass + verification. Each path and each file's contents are now length-prefixed. Legacy digests are + still accepted until their record is rewritten (see Breaking Changes); rebuild bundles with + `fastskill bundle build` to give them current-form digests. + - **`project install` reports every dependency that fails to resolve, not just the first.** A missing local path, a moved git subdirectory or a renamed skill used to stop the run at the first bad entry, so fixing a large manifest took one run per error. All root dependencies are now diff --git a/CONTEXT.md b/CONTEXT.md index d8ee8d33..60fa8af2 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -41,6 +41,14 @@ _Avoid_: project file, config. Used by `project install --lock` for reproducible installs. _Avoid_: lockfile (in prose), pin file. +**Content digest**: +The string that names the exact files of a skill directory: `sha256-tree-v2:` plus a SHA-256 +over every file's relative path and contents, each length-prefixed. A Lock pins it as `checksum`; +bundle records and artifacts pin it per member. A **legacy digest** (64 bare hex characters, +written before ADR-0017) is still accepted with a warning and is replaced when its record is +rewritten. See [ADR-0017](./docs/adr/0017-versioned-content-digests.md). +_Avoid_: hash, checksum (except as the Lock field name). + **Installed skill**: A skill physically present in the **skills directory** (`.claude/skills/` by default). The skills directory — not the Manifest — is the source of truth for what `skill list` reports. diff --git a/crates/fastskill-cli/src/commands/install.rs b/crates/fastskill-cli/src/commands/install.rs index 71324dd2..e49561e9 100644 --- a/crates/fastskill-cli/src/commands/install.rs +++ b/crates/fastskill-cli/src/commands/install.rs @@ -646,10 +646,11 @@ async fn build_install_targets( let unchanged = current.as_ref().is_some_and(|installed| { installed.version == target.resolved_version && target.checksum.as_ref().is_some_and(|checksum| { - fastskill_core::core::install::content_digest( + fastskill_core::core::content_digest::content_digest_matches( + checksum, &service.config().skill_storage_path.join(&target.id), ) - .is_ok_and(|actual| &actual == checksum) + .unwrap_or(false) }) }); let current_revision = current.map(|installed| installed.version); diff --git a/crates/fastskill-cli/src/commands/install/change.rs b/crates/fastskill-cli/src/commands/install/change.rs index 85cca7f5..a044d5df 100644 --- a/crates/fastskill-cli/src/commands/install/change.rs +++ b/crates/fastskill-cli/src/commands/install/change.rs @@ -295,8 +295,8 @@ fn candidate_matches_lock_and_install( .zip(source.canonicalize().ok()) .is_some_and(|(installed, source)| installed == source), _ => entry.resolved.checksum.as_ref().is_some_and(|expected| { - fastskill_core::core::install::content_digest(&installed) - .is_ok_and(|actual| &actual == expected) + fastskill_core::core::content_digest::content_digest_matches(expected, &installed) + .unwrap_or(false) }), } } diff --git a/crates/fastskill-cli/src/commands/install/global.rs b/crates/fastskill-cli/src/commands/install/global.rs index fd5026f0..4cc6503d 100644 --- a/crates/fastskill-cli/src/commands/install/global.rs +++ b/crates/fastskill-cli/src/commands/install/global.rs @@ -317,8 +317,8 @@ fn installed_matches(path: &Path, entry: &GlobalLockedSkillEntry) -> bool { .zip(source.canonicalize().ok()) .is_some_and(|(installed, source)| installed == source), _ => entry.resolved.checksum.as_ref().is_some_and(|expected| { - fastskill_core::core::install::content_digest(path) - .is_ok_and(|actual| &actual == expected) + fastskill_core::core::content_digest::content_digest_matches(expected, path) + .unwrap_or(false) }), } } diff --git a/crates/fastskill-cli/src/commands/install/plan.rs b/crates/fastskill-cli/src/commands/install/plan.rs index 669fb963..6de7c407 100644 --- a/crates/fastskill-cli/src/commands/install/plan.rs +++ b/crates/fastskill-cli/src/commands/install/plan.rs @@ -406,7 +406,10 @@ pub(crate) fn validate_declared_bundle_members( .filter(|member| member.id == planned.prepared.id()) { if !member.overridable - && planned.prepared.resolved().checksum.as_deref() != Some(member.digest.as_str()) + && !planned + .prepared + .checksum_matches(&member.digest) + .map_err(CliError::Service)? { return Err(CliError::Config(format!( "individual skill '{}' conflicts with member content from bundle '{}'", diff --git a/crates/fastskill-cli/src/commands/list.rs b/crates/fastskill-cli/src/commands/list.rs index a0bed0e4..d6274a30 100644 --- a/crates/fastskill-cli/src/commands/list.rs +++ b/crates/fastskill-cli/src/commands/list.rs @@ -13,12 +13,12 @@ use cli_framework::command::{FromArgValueMap, IntoCommandSpec}; use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality}; use cli_framework::spec::command_tree::CommandSpec; use cli_framework::spec::value::ArgValue; +use fastskill_core::core::content_digest::{all_match, recorded_digests_conflict}; use fastskill_core::core::lock::ProjectSkillsLock; use fastskill_core::core::manifest::SkillProjectToml; use fastskill_core::core::origin::Origin; use fastskill_core::core::ownership::ProjectOwnership; use fastskill_core::core::project::resolve_project_file; -use fastskill_core::core::project_removal::managed_tree_digest; use fastskill_core::core::service::FastSkillService; use fastskill_core::OutputFormat; use std::collections::{HashMap, HashSet}; @@ -435,32 +435,22 @@ pub async fn execute_list( } else if mutable { ReconciliationStatus::Ok } else if let Some(expected) = &locked.resolved.checksum { - match managed_tree_digest(&service.config().skill_storage_path.join(&id)) { - Ok(actual) if &actual == expected => ReconciliationStatus::Ok, - Ok(_) => ReconciliationStatus::ContentMismatch, - Err(_) => ReconciliationStatus::IntegrityError, - } + content_status(&[expected], &service.config().skill_storage_path.join(&id)) } else { ReconciliationStatus::InsufficientIntegrity } } else if selected && (!bundle_members.is_empty() || override_entry.is_some()) { - let expected = override_entry - .map(|entry| entry.digest.as_str()) - .or_else(|| bundle_members.first().map(|member| member.digest.as_str())); - let owners_agree = override_entry.is_some() - || bundle_members + let expected: Vec<&str> = match override_entry { + Some(entry) => vec![entry.digest.as_str()], + None => bundle_members .iter() - .all(|member| Some(member.digest.as_str()) == expected); - match (owners_agree, expected) { - (false, _) => ReconciliationStatus::OwnershipConflict, - (true, Some(expected)) => { - match managed_tree_digest(&service.config().skill_storage_path.join(&id)) { - Ok(actual) if actual == expected => ReconciliationStatus::Ok, - Ok(_) => ReconciliationStatus::ContentMismatch, - Err(_) => ReconciliationStatus::IntegrityError, - } - } - _ => ReconciliationStatus::InsufficientIntegrity, + .map(|member| member.digest.as_str()) + .collect(), + }; + if recorded_digests_conflict(expected.iter().copied()) { + ReconciliationStatus::OwnershipConflict + } else { + content_status(&expected, &service.config().skill_storage_path.join(&id)) } } else if let (Some(locked), Some(actual)) = (locked_entry, installed_map.get(&id)) { if actual.version != locked.resolved.version { @@ -468,11 +458,7 @@ pub async fn execute_list( } else if mutable { ReconciliationStatus::Ok } else if let Some(expected) = &locked.resolved.checksum { - match managed_tree_digest(&service.config().skill_storage_path.join(&id)) { - Ok(actual) if &actual == expected => ReconciliationStatus::Ok, - Ok(_) => ReconciliationStatus::ContentMismatch, - Err(_) => ReconciliationStatus::IntegrityError, - } + content_status(&[expected], &service.config().skill_storage_path.join(&id)) } else { ReconciliationStatus::InsufficientIntegrity } @@ -542,6 +528,15 @@ pub async fn execute_list( clippy::panic, clippy::await_holding_lock )] +/// Compare installed content with every recorded digest; a legacy digest is accepted. +fn content_status(expected: &[&str], installed: &std::path::Path) -> ReconciliationStatus { + match all_match(expected, installed) { + Ok(true) => ReconciliationStatus::Ok, + Ok(false) => ReconciliationStatus::ContentMismatch, + Err(_) => ReconciliationStatus::IntegrityError, + } +} + #[cfg(test)] #[path = "list/tests.rs"] mod tests; diff --git a/crates/fastskill-cli/src/commands/list/global.rs b/crates/fastskill-cli/src/commands/list/global.rs index a3a751ff..46b99ee8 100644 --- a/crates/fastskill-cli/src/commands/list/global.rs +++ b/crates/fastskill-cli/src/commands/list/global.rs @@ -1,7 +1,6 @@ -use super::{format_source_info, origin_location_label, ListArgs}; +use super::{content_status, format_source_info, origin_location_label, ListArgs}; use crate::error::{CliError, CliResult}; use fastskill_core::core::lock::{global_lock_path, GlobalSkillsLock}; -use fastskill_core::core::project_removal::managed_tree_digest; use fastskill_core::core::Origin; use fastskill_core::{FastSkillService, OutputFormat}; use std::collections::{BTreeSet, HashMap, HashSet}; @@ -193,11 +192,7 @@ fn reconcile( (Some(_), Some(_)) if mutable => ReconciliationStatus::Ok, (Some(locked), Some(_)) => match &locked.resolved.checksum { Some(expected) => { - match managed_tree_digest(&service.config().skill_storage_path.join(id)) { - Ok(actual) if &actual == expected => ReconciliationStatus::Ok, - Ok(_) => ReconciliationStatus::ContentMismatch, - Err(_) => ReconciliationStatus::IntegrityError, - } + content_status(&[expected], &service.config().skill_storage_path.join(id)) } None => ReconciliationStatus::InsufficientIntegrity, }, @@ -213,6 +208,7 @@ mod tests { use chrono::Utc; use fastskill_core::core::lock::GlobalLockedSkillEntry; use fastskill_core::core::origin::Resolved; + use fastskill_core::core::project_removal::managed_tree_digest; use fastskill_core::{ServiceConfig, SkillDefinition, SkillId}; use std::path::Path; use tempfile::TempDir; diff --git a/crates/fastskill-cli/src/commands/list/reconciliation_tests.rs b/crates/fastskill-cli/src/commands/list/reconciliation_tests.rs index 6aa15268..071f550f 100644 --- a/crates/fastskill-cli/src/commands/list/reconciliation_tests.rs +++ b/crates/fastskill-cli/src/commands/list/reconciliation_tests.rs @@ -2,6 +2,7 @@ use super::*; use fastskill_core::core::lock::{ ProjectLockedBundleEntry, ProjectLockedBundleMember, ProjectLockedPersonalOverride, }; +use fastskill_core::core::project_removal::managed_tree_digest; use serde_json::Value; struct ProjectFixture { diff --git a/crates/fastskill-cli/src/commands/list/tests.rs b/crates/fastskill-cli/src/commands/list/tests.rs index bc7275f0..92c48842 100644 --- a/crates/fastskill-cli/src/commands/list/tests.rs +++ b/crates/fastskill-cli/src/commands/list/tests.rs @@ -2,6 +2,7 @@ use super::*; use fastskill_core::core::lock::{ProjectLockedSkillEntry, ProjectSkillsLock}; use fastskill_core::core::origin::GitRef; use fastskill_core::core::origin::Resolved; +use fastskill_core::core::project_removal::managed_tree_digest; use fastskill_core::{FastSkillService, ServiceConfig}; use std::fs; use tempfile::TempDir; diff --git a/crates/fastskill-cli/src/commands/remove/global.rs b/crates/fastskill-cli/src/commands/remove/global.rs index c54b0090..e92d7c88 100644 --- a/crates/fastskill-cli/src/commands/remove/global.rs +++ b/crates/fastskill-cli/src/commands/remove/global.rs @@ -198,9 +198,9 @@ fn validate_unmodified( "global skill '{id}' has no locked content digest; restore it before removal" )) })?; - let actual = - fastskill_core::core::install::content_digest(&path).map_err(CliError::Service)?; - if &actual != expected { + if !fastskill_core::core::content_digest::content_digest_matches(expected, &path) + .map_err(CliError::Service)? + { return Err(CliError::Config(format!( "global skill '{id}' is locally modified; FastSkill will not delete it" ))); diff --git a/crates/fastskill-cli/src/commands/update/global.rs b/crates/fastskill-cli/src/commands/update/global.rs index 24319e96..1446d7d9 100644 --- a/crates/fastskill-cli/src/commands/update/global.rs +++ b/crates/fastskill-cli/src/commands/update/global.rs @@ -4,9 +4,9 @@ use crate::config::create_service_config; use crate::error::{CliError, CliResult}; use crate::utils::messages; use fastskill_core::core::contained_path::{remove_contained, ContainedPath}; +use fastskill_core::core::content_digest::content_digest_matches; use fastskill_core::core::install::PreparedSkill; use fastskill_core::core::lock::{global_lock_path, GlobalLockedSkillEntry, GlobalSkillsLock}; -use fastskill_core::core::project_removal::managed_tree_digest; use fastskill_core::core::resolution::{ prepare_resolution, prepare_resolution_preview, ResolutionRoot, }; @@ -142,8 +142,7 @@ pub(crate) fn validate_global_replacement_content( entry.id )) })?; - let actual = managed_tree_digest(&installed).map_err(CliError::Service)?; - if &actual != expected { + if !content_digest_matches(expected, &installed).map_err(CliError::Service)? { return Err(CliError::Config(format!( "installed global skill '{}' was modified; restore or remove local edits before replacement", entry.id @@ -525,9 +524,9 @@ pub(super) async fn execute_update_global( )); continue; }; - match managed_tree_digest(&installed) { - Ok(actual) if &actual == expected => {} - Ok(_) => { + match content_digest_matches(expected, &installed) { + Ok(true) => {} + Ok(false) => { failures.push(format!( "{}: installed content was modified; restore it before updating", entry.id diff --git a/crates/fastskill-cli/src/commands/update/global_tests.rs b/crates/fastskill-cli/src/commands/update/global_tests.rs index 8ea16db9..d39359be 100644 --- a/crates/fastskill-cli/src/commands/update/global_tests.rs +++ b/crates/fastskill-cli/src/commands/update/global_tests.rs @@ -1,4 +1,5 @@ use super::*; +use fastskill_core::core::project_removal::managed_tree_digest; struct EnvGuard { name: &'static str, diff --git a/crates/fastskill-core/src/core/bundle.rs b/crates/fastskill-core/src/core/bundle.rs index 067f7e01..714b9c9a 100644 --- a/crates/fastskill-core/src/core/bundle.rs +++ b/crates/fastskill-core/src/core/bundle.rs @@ -8,11 +8,12 @@ use crate::core::bundle_archive::{write_bundle_archive, BundleArchiveLock}; use crate::core::bundle_build::prepare_bundle_build_manifest; use crate::core::bundle_persistence::{ - apply_personal_override, digest_directory, parse_bundle_descriptor, prepare_members, - preview_personal_override, remove_skill_directory, replace_skill_directory, - save_bundle_declarations, BundleHistory, BundleTransaction, + apply_personal_override, parse_bundle_descriptor, prepare_members, preview_personal_override, + remove_skill_directory, replace_skill_directory, save_bundle_declarations, BundleHistory, + BundleTransaction, }; use crate::core::contained_path::ContainedPath; +use crate::core::content_digest::{content_digest, content_digest_matches, DigestForms}; use crate::core::lock::{ProjectLockedBundleEntry, ProjectLockedBundleMember, ProjectSkillsLock}; use crate::core::manifest::SkillProjectToml; use crate::core::ownership::ProjectOwnership; @@ -365,7 +366,7 @@ impl BundleService { if let Some(installed) = &existing { if installed.version == prepared.descriptor.version - && installed.digest != prepared.release_digest + && !prepared.release_digest.matches(&installed.digest) { return Err(ServiceError::Validation(format!( "Bundle release '{}@{}' is immutable: its contents differ from the installed digest", @@ -383,7 +384,7 @@ impl BundleService { installed.id, installed.version, installed.id ))); } - if installed.digest == prepared.release_digest { + if prepared.release_digest.matches(&installed.digest) { return Ok(BundleApplyResult { id: installed.id.clone(), version: installed.version.clone(), @@ -406,7 +407,7 @@ impl BundleService { ))); } if let Some(installed) = &existing { - if installed.digest == prepared.release_digest + if prepared.release_digest.matches(&installed.digest) && self.bundle_members_match(installed, &lock) { return Ok(BundleApplyResult { @@ -427,7 +428,7 @@ impl BundleService { } if let Some(installed) = &existing { if installed.version == prepared.descriptor.version - && installed.digest == prepared.release_digest + && prepared.release_digest.matches(&installed.digest) && self.bundle_members_match(installed, &lock) { return Ok(BundleApplyResult { @@ -466,7 +467,7 @@ impl BundleService { let release_key = format!("{}@{}", prepared.descriptor.id, prepared.descriptor.version); if let Some(known_digest) = history.releases.get(&release_key) { - if known_digest != &prepared.release_digest { + if !prepared.release_digest.matches(known_digest) { return Err(ServiceError::Validation(format!( "Bundle release '{}' is immutable: its contents differ from the previously known digest", release_key @@ -477,21 +478,11 @@ impl BundleService { self.preflight_apply(&prepared, existing.as_ref(), &manifest, &lock)?; let changes = self.plan_changes(&prepared, existing.as_ref(), &manifest, &lock)?; let artifact_relative = self.artifact_relative(&prepared.descriptor)?; - let next = ProjectLockedBundleEntry { - id: prepared.descriptor.id.clone(), - version: prepared.descriptor.version.clone(), - artifact: artifact_relative.clone(), - digest: prepared.release_digest.clone(), - members: prepared - .members - .values() - .map(|member| ProjectLockedBundleMember { - id: member.id.clone(), - digest: member.digest.clone(), - overridable: member.overridable, - }) - .collect(), + let recorded = match mode { + ApplyMode::RestoreLocked { expected } => Some(expected), + _ => None, }; + let next = prepared.lock_entry(artifact_relative.clone(), recorded); lock.bundles.retain(|bundle| bundle.id != next.id); lock.bundles.push(next); manifest.bundles.insert( @@ -503,7 +494,7 @@ impl BundleService { ); history .releases - .insert(release_key, prepared.release_digest.clone()); + .insert(release_key, prepared.release_digest.current.clone()); let changed_members: Vec<_> = changes .replacements @@ -595,7 +586,7 @@ impl BundleService { let needs_mutation = still_present .then(|| prepared.members.get(&member.id)) .flatten() - .is_none_or(|replacement| replacement.digest != member.digest) + .is_none_or(|replacement| !replacement.digest.matches(&member.digest)) || !still_present; if needs_mutation && !self.has_other_owner(lock, ¤t.id, &member.id) @@ -620,7 +611,10 @@ impl BundleService { } continue; } - if owners.iter().any(|owner| owner.digest != member.digest) { + if owners + .iter() + .any(|owner| !member.digest.matches(&owner.digest)) + { return Err(ServiceError::InvalidOperation(format!( "Skill '{}' has conflicting contents required by another installed bundle", member.id @@ -632,7 +626,7 @@ impl BundleService { .iter() .find(|entry| entry.id == member.id) .and_then(|entry| entry.resolved.checksum.as_deref()); - if selected_digest.is_some_and(|digest| digest != member.digest) { + if selected_digest.is_some_and(|digest| !member.digest.matches(digest)) { return Err(ServiceError::InvalidOperation(format!( "Skill '{}' has conflicting contents required by individual root(s): {}", member.id, @@ -642,8 +636,8 @@ impl BundleService { } let destination = self.skills_directory.join(&member.id); if destination.exists() { - let actual = digest_directory(&destination)?; - if !individual_roots.is_empty() && actual != member.digest { + let actual = DigestForms::of_directory(&destination)?; + if !individual_roots.is_empty() && actual.current != member.digest.current { return Err(ServiceError::InvalidOperation(format!( "Skill '{}' has conflicting contents required by individual root(s): {}", member.id, @@ -664,13 +658,13 @@ impl BundleService { }) }); if let Some(expected) = expected_existing { - if actual != expected { + if !actual.matches(expected) { return Err(ServiceError::InvalidOperation(format!( "Skill '{}' is locally modified; declare a permitted personal override or discard the edit before changing bundles", member.id ))); } - } else if actual != member.digest { + } else if actual.current != member.digest.current { return Err(ServiceError::InvalidOperation(format!( "Skill '{}' already exists with different contents", member.id @@ -696,9 +690,9 @@ impl BundleService { let destination = self.skills_directory.join(&member.id); let identical = destination .exists() - .then(|| digest_directory(&destination)) + .then(|| content_digest(&destination)) .transpose()? - .is_some_and(|digest| digest == member.digest); + .is_some_and(|digest| digest == member.digest.current); if !identical { changes.replacements.push(BundleReplacement { id: member.id.clone(), @@ -744,8 +738,7 @@ impl BundleService { if !destination.exists() { return Ok(()); } - let actual = digest_directory(&destination)?; - if actual == expected { + if content_digest_matches(expected, &destination)? { Ok(()) } else { Err(ServiceError::InvalidOperation(format!( @@ -805,8 +798,7 @@ impl BundleService { .map_or(member.digest.as_str(), |override_entry| { override_entry.digest.as_str() }); - digest_directory(&self.skills_directory.join(&member.id)) - .map(|digest| digest == effective_digest) + content_digest_matches(effective_digest, &self.skills_directory.join(&member.id)) .unwrap_or(false) }) } diff --git a/crates/fastskill-core/src/core/bundle_archive.rs b/crates/fastskill-core/src/core/bundle_archive.rs index a2372134..78332f40 100644 --- a/crates/fastskill-core/src/core/bundle_archive.rs +++ b/crates/fastskill-core/src/core/bundle_archive.rs @@ -1,5 +1,6 @@ use crate::core::bundle::{BundleArchiveLockMember, BundleDescriptor, PreparedMember}; use crate::core::bundle_persistence::digest_release; +use crate::core::content_digest::DigestForms; use crate::core::service::ServiceError; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; @@ -20,9 +21,18 @@ pub(super) struct BundleArchiveLock { } impl BundleArchiveLock { + /// The archive lock a new artifact carries: every digest in the current form. pub(super) fn from_members( descriptor: &BundleDescriptor, members: &BTreeMap, + ) -> Self { + Self::with_member_digests(descriptor, members, |forms| &forms.current) + } + + fn with_member_digests( + descriptor: &BundleDescriptor, + members: &BTreeMap, + form: fn(&DigestForms) -> &String, ) -> Self { let members: BTreeMap<_, _> = members .iter() @@ -30,7 +40,7 @@ impl BundleArchiveLock { ( id.clone(), BundleArchiveLockMember { - digest: member.digest.clone(), + digest: form(&member.digest).clone(), overridable: member.overridable, }, ) @@ -50,6 +60,22 @@ impl BundleArchiveLock { &self.release_digest } + /// The release digest of `members` in both forms: the current form fastskill records, + /// and the legacy form older releases recorded in Locks and bundle history. + pub(super) fn release_digests( + descriptor: &BundleDescriptor, + members: &BTreeMap, + ) -> DigestForms { + DigestForms { + current: Self::from_members(descriptor, members).release_digest, + legacy: Self::with_member_digests(descriptor, members, |forms| &forms.legacy) + .release_digest, + } + } + + /// Check the archive lock against the extracted members. An artifact built before + /// ADR-0017 declares every digest in the legacy form; it is accepted with a warning. An + /// archive lock that mixes forms matches neither and is refused. pub(super) fn verify( &self, descriptor: &BundleDescriptor, @@ -63,13 +89,25 @@ impl BundleArchiveLock { "Bundle skills.lock does not match bundle identity and version".to_string(), )); } - let expected = Self::from_members(descriptor, members); - if self.members != expected.members || self.release_digest != expected.release_digest { - return Err(ServiceError::Validation( - "Bundle contents do not match the digests in skills.lock".to_string(), - )); + if self.declares(&Self::from_members(descriptor, members)) { + return Ok(()); } - Ok(()) + let legacy = Self::with_member_digests(descriptor, members, |forms| &forms.legacy); + if self.declares(&legacy) { + crate::utils::warn_once( + "legacy-bundle-artifact", + "bundle artifact was built by an older fastskill and declares legacy content \ + digests; rebuild it with `fastskill bundle build` (ADR-0017)", + ); + return Ok(()); + } + Err(ServiceError::Validation( + "Bundle contents do not match the digests in skills.lock".to_string(), + )) + } + + fn declares(&self, expected: &Self) -> bool { + self.members == expected.members && self.release_digest == expected.release_digest } } @@ -159,6 +197,13 @@ mod tests { use crate::core::bundle::{BundleMemberPolicy, BUNDLE_FORMAT}; use std::os::unix::fs::PermissionsExt; + fn forms(current: &str) -> DigestForms { + DigestForms { + current: current.to_string(), + legacy: format!("legacy-{current}"), + } + } + #[test] fn bundle_archive_preserves_the_executable_bit_for_member_files() { let root = tempfile::tempdir().unwrap(); @@ -179,7 +224,7 @@ mod tests { PreparedMember { id: "demo".to_string(), source, - digest: "digest".to_string(), + digest: forms("digest"), overridable: false, }, )]); @@ -213,7 +258,7 @@ mod tests { PreparedMember { id: "demo".to_string(), source: std::path::PathBuf::from("demo"), - digest: "digest".to_string(), + digest: forms("digest"), overridable: false, }, )]); @@ -229,7 +274,7 @@ mod tests { )); let mut changed_members = members; - changed_members.get_mut("demo").unwrap().digest = "changed".to_string(); + changed_members.get_mut("demo").unwrap().digest = forms("changed"); assert!(matches!( lock.verify(&descriptor, &changed_members), Err(ServiceError::Validation(message)) @@ -255,7 +300,7 @@ mod tests { PreparedMember { id: "demo".to_string(), source, - digest: "digest".to_string(), + digest: forms("digest"), overridable: false, }, )]); @@ -270,4 +315,48 @@ mod tests { Err(ServiceError::Io(_)) )); } + + #[test] + fn bundle_lock_verification_accepts_an_all_legacy_lock_but_not_a_mixed_one() { + let descriptor = BundleDescriptor { + format_marker: BUNDLE_FORMAT.to_string(), + id: "team".to_string(), + version: "1.0.0".to_string(), + members: BTreeMap::from([ + ("one".to_string(), BundleMemberPolicy::default()), + ("two".to_string(), BundleMemberPolicy::default()), + ]), + }; + let member = |id: &str| PreparedMember { + id: id.to_string(), + source: std::path::PathBuf::from(id), + digest: forms(id), + overridable: false, + }; + let members = BTreeMap::from([ + ("one".to_string(), member("one")), + ("two".to_string(), member("two")), + ]); + let current = BundleArchiveLock::from_members(&descriptor, &members); + let legacy = + BundleArchiveLock::with_member_digests(&descriptor, &members, |forms| &forms.legacy); + assert_ne!(current.release_digest, legacy.release_digest); + legacy.verify(&descriptor, &members).unwrap(); + assert_eq!( + BundleArchiveLock::release_digests(&descriptor, &members), + DigestForms { + current: current.release_digest.clone(), + legacy: legacy.release_digest.clone(), + } + ); + + let mut mixed = legacy.clone(); + mixed.members.get_mut("one").unwrap().digest = "one".to_string(); + mixed.release_digest = digest_release(&descriptor, &mixed.members); + assert!(matches!( + mixed.verify(&descriptor, &members), + Err(ServiceError::Validation(message)) + if message.contains("do not match the digests") + )); + } } diff --git a/crates/fastskill-core/src/core/bundle_override.rs b/crates/fastskill-core/src/core/bundle_override.rs index 07083e67..4b8a5508 100644 --- a/crates/fastskill-core/src/core/bundle_override.rs +++ b/crates/fastskill-core/src/core/bundle_override.rs @@ -4,10 +4,12 @@ use crate::core::bundle::{ BundleManifestTables, BundleOverridePreview, BundleService, PreparedBundle, }; use crate::core::bundle_persistence::{ - digest_directory, replace_skill_directory, save_bundle_declarations, BundleOverrideDeclaration, - BundleTransaction, + replace_skill_directory, save_bundle_declarations, BundleOverrideDeclaration, BundleTransaction, }; use crate::core::contained_path::ContainedPath; +use crate::core::content_digest::{ + content_digest_matches, recorded_digests_conflict, upgraded_digest, +}; use crate::core::lock::{ProjectLockedSkillEntry, ProjectSkillsLock}; use crate::core::manifest::{DependenciesSection, DependencySpec, SkillProjectToml}; use crate::core::origin::{Origin, Resolved}; @@ -83,7 +85,7 @@ pub(crate) fn retain_final_personal_overrides( promotions.push(Promotion { id: override_entry.id.clone(), origin, - digest: override_entry.digest.clone(), + digest: upgraded_digest(&override_entry.digest, Path::new(&override_entry.origin))?, name, version, required_dependencies, @@ -263,7 +265,7 @@ pub(crate) fn reset_personal_override( } }; let packaged = match prepared.members.get(id) { - Some(packaged) if packaged.digest == packaged_digest => packaged, + Some(packaged) if packaged.digest.matches(&packaged_digest) => packaged, Some(_) => { state_guard.recovered()?; return Err(ServiceError::Config(format!( @@ -280,14 +282,15 @@ pub(crate) fn reset_personal_override( } }; if installed.as_path().exists() { - let installed_digest = match digest_directory(installed.as_path()) { - Ok(digest) => digest, + let unchanged = match content_digest_matches(&expected_override.digest, installed.as_path()) + { + Ok(unchanged) => unchanged, Err(error) => { state_guard.recovered()?; return Err(error); } }; - if installed_digest != expected_override.digest { + if !unchanged { state_guard.recovered()?; return Err(ServiceError::InvalidOperation(format!( "Skill '{id}' changed while the reset was prepared; retry" @@ -386,7 +389,7 @@ fn prepare_reset( }; let (artifact, packaged_digest) = agreed_owner_selection(service, &lock, id)?; let installed = service.skills_directory.join(id); - if installed.exists() && digest_directory(&installed)? != expected_override.digest { + if installed.exists() && !content_digest_matches(&expected_override.digest, &installed)? { return Err(ServiceError::InvalidOperation(format!( "Skill '{id}' is locally modified; FastSkill will not discard those edits while resetting the override" ))); @@ -398,7 +401,7 @@ fn prepare_reset( artifact.display() )) })?; - if packaged.digest != packaged_digest { + if !packaged.digest.matches(&packaged_digest) { return Err(ServiceError::Config(format!( "Cached bundle artifact '{}' does not match the locked contents for '{id}'", artifact.display() @@ -451,10 +454,7 @@ fn agreed_owner_selection( "Personal override '{id}' has no retained bundle owner to restore" ))); }; - if owners - .iter() - .any(|(_, member)| member.digest != first_member.digest) - { + if recorded_digests_conflict(owners.iter().map(|(_, member)| member.digest.as_str())) { let bundles = owners .iter() .map(|(bundle, _)| bundle.id.as_str()) diff --git a/crates/fastskill-core/src/core/bundle_persistence.rs b/crates/fastskill-core/src/core/bundle_persistence.rs index c404d9e6..4ec170e6 100644 --- a/crates/fastskill-core/src/core/bundle_persistence.rs +++ b/crates/fastskill-core/src/core/bundle_persistence.rs @@ -3,6 +3,10 @@ use crate::core::bundle::{ BundleMemberPolicy, BundleOverridePreview, BundleService, PreparedMember, BUNDLE_FORMAT, }; use crate::core::contained_path::ContainedPath; +use crate::core::content_digest::{ + all_match, content_digest, content_digest_matches, hash_field, recorded_digests_conflict, + DigestForms, +}; use crate::core::lock::{ProjectLockedPersonalOverride, ProjectSkillsLock}; use crate::core::manifest::DependencySpec; use crate::core::origin::Origin; @@ -14,7 +18,6 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use std::collections::{BTreeMap, VecDeque}; use std::fs; -use std::io::Read; use std::path::{Path, PathBuf}; use tempfile::TempDir; use walkdir::WalkDir; @@ -161,7 +164,7 @@ pub(crate) fn prepare_members( id.clone(), PreparedMember { id, - digest: digest_directory(&source)?, + digest: DigestForms::of_directory(&source)?, source, overridable: policy.overridable, }, @@ -234,47 +237,6 @@ pub(crate) fn digest_release( crate::utils::to_hex_lower(&hasher.finalize()) } -pub(crate) fn digest_directory(path: &Path) -> Result { - if !path.is_dir() { - return Err(ServiceError::Validation(format!( - "Expected skill directory at {}", - path.display() - ))); - } - let mut hasher = Sha256::new(); - for entry in WalkDir::new(path).sort_by_file_name() { - let entry = entry.map_err(|error| ServiceError::Io(io_error(error)))?; - if entry.file_type().is_symlink() { - return Err(ServiceError::Validation(format!( - "Symbolic links are not permitted in bundle members: {}", - entry.path().display() - ))); - } - if !entry.file_type().is_file() { - continue; - } - let relative = entry.path().strip_prefix(path).map_err(|error| { - ServiceError::Custom(format!("Failed to form skill digest path: {error}")) - })?; - hash_field(&mut hasher, &relative.to_string_lossy().replace('\\', "/")); - let mut file = fs::File::open(entry.path()).map_err(ServiceError::Io)?; - let mut buffer = [0u8; 8192]; - loop { - let read = file.read(&mut buffer).map_err(ServiceError::Io)?; - if read == 0 { - break; - } - hasher.update(&buffer[..read]); - } - } - Ok(crate::utils::to_hex_lower(&hasher.finalize())) -} - -fn hash_field(hasher: &mut Sha256, value: &str) { - hasher.update((value.len() as u64).to_be_bytes()); - hasher.update(value.as_bytes()); -} - #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub(crate) struct BundleHistory { #[serde(default)] @@ -301,7 +263,7 @@ pub(crate) fn apply_personal_override( ))); } let source = source.canonicalize().map_err(ServiceError::Io)?; - let digest = digest_directory(&source)?; + let digest = content_digest(&source)?; if preview.target_revision.as_deref() != Some(digest.as_str()) { return Err(ServiceError::InvalidOperation(format!( "Personal override source for '{id}' changed while the operation was prepared; retry" @@ -399,7 +361,7 @@ pub(crate) fn apply_personal_override( if take_override_test_change(7) { fs::remove_file(source.join("SKILL.md")).map_err(ServiceError::Io)?; } - let current_digest = match digest_directory(&source) { + let current_digest = match content_digest(&source) { Ok(current_digest) => current_digest, Err(error) => { state_guard.recovered()?; @@ -471,7 +433,7 @@ pub(crate) fn preview_personal_override( ))); } let source = source.canonicalize().map_err(ServiceError::Io)?; - let target = digest_directory(&source)?; + let target = DigestForms::of_directory(&source)?; let manifest_path = service.project_root.join("skill-project.toml"); let content = fs::read_to_string(&manifest_path).map_err(ServiceError::Io)?; let tables: BundleManifestTables = toml::from_str(&content).map_err(|error| { @@ -496,8 +458,7 @@ pub(crate) fn preview_personal_override( "Every bundle owning '{id}' must permit a personal override" ))); } - let owner_digest = &owners[0].digest; - if owners.iter().any(|member| member.digest != *owner_digest) { + if recorded_digests_conflict(owners.iter().map(|member| member.digest.as_str())) { return Err(ServiceError::InvalidOperation(format!( "Bundle owners of '{id}' disagree on packaged contents" ))); @@ -512,20 +473,22 @@ pub(crate) fn preview_personal_override( ))); } } - let current = existing_override - .map(|entry| entry.digest.clone()) - .unwrap_or_else(|| owner_digest.clone()); + let recorded = match existing_override { + Some(entry) => vec![entry.digest.as_str()], + None => owners.iter().map(|member| member.digest.as_str()).collect(), + }; + let current = recorded[0].to_string(); let installed = service.skills_directory.join(id); - if installed.exists() && digest_directory(&installed)? != current { + if installed.exists() && !all_match(&recorded, &installed)? { return Err(ServiceError::InvalidOperation(format!( "Skill '{id}' is locally modified; FastSkill will not discard those edits while setting an override" ))); } - let changed = current != target || existing_override.is_none(); + let changed = !target.matches(¤t) || existing_override.is_none(); Ok(BundleOverridePreview { id: id.to_string(), current_revision: Some(current), - target_revision: Some(target), + target_revision: Some(target.current), changed, changes: if changed { vec![ @@ -652,7 +615,7 @@ fn restore_personal_overrides_impl( source.display() ))); } - if digest_directory(&source)? != locked.digest { + if !content_digest_matches(&locked.digest, &source)? { return Err(ServiceError::InvalidOperation(format!( "Personal override '{id}' no longer matches its locked digest" ))); diff --git a/crates/fastskill-core/src/core/bundle_persistence_tests.rs b/crates/fastskill-core/src/core/bundle_persistence_tests.rs index aa05a83b..38d7f2fe 100644 --- a/crates/fastskill-core/src/core/bundle_persistence_tests.rs +++ b/crates/fastskill-core/src/core/bundle_persistence_tests.rs @@ -35,7 +35,7 @@ fn override_fixture() -> (TempDir, BundleService, PathBuf) { ) .unwrap(); fs::write(root.path().join("skill-project.toml"), "").unwrap(); - let packaged_digest = digest_directory(&packaged).unwrap(); + let packaged_digest = content_digest(&packaged).unwrap(); let mut lock = ProjectSkillsLock::new_empty(); lock.bundles.push(ProjectLockedBundleEntry { id: "team".to_string(), @@ -501,19 +501,19 @@ fn directory_helpers_stage_replacements_and_refuse_non_directories() { fn digest_and_copy_reject_links_inside_managed_content() { let root = TempDir::new().unwrap(); let source = root.path().join("source"); - assert!(digest_directory(&root.path().join("missing")).is_err()); + assert!(content_digest(&root.path().join("missing")).is_err()); fs::create_dir_all(&source).unwrap(); fs::write(source.join("SKILL.md"), "demo").unwrap(); std::os::unix::fs::symlink(source.join("SKILL.md"), source.join("linked")).unwrap(); assert!(matches!( - digest_directory(&source), + content_digest(&source), Err(ServiceError::Validation(_)) )); assert!(matches!( copy_directory(&source, &root.path().join("copy")), Err(ServiceError::Validation(_)) )); - assert!(digest_directory(&root.path().join("missing")).is_err()); + assert!(content_digest(&root.path().join("missing")).is_err()); } #[test] @@ -708,7 +708,7 @@ fn an_override_id_that_leaves_the_skills_directory_is_refused_before_restore() { lock.overrides.push(ProjectLockedPersonalOverride { id: "../victim".to_string(), origin, - digest: digest_directory(&source).unwrap(), + digest: content_digest(&source).unwrap(), }); lock.save_to_file(&root.path().join("skills.lock")).unwrap(); @@ -730,7 +730,7 @@ fn a_bundle_member_id_that_leaves_the_skills_directory_is_refused_before_removal let mut lock = ProjectSkillsLock::load_from_file(&root.path().join("skills.lock")).unwrap(); lock.bundles[0].members.push(ProjectLockedBundleMember { id: "../victim".to_string(), - digest: digest_directory(&victim).unwrap(), + digest: content_digest(&victim).unwrap(), overridable: false, }); lock.save_to_file(&root.path().join("skills.lock")).unwrap(); diff --git a/crates/fastskill-core/src/core/bundle_plan.rs b/crates/fastskill-core/src/core/bundle_plan.rs index ae48359e..cfcdd4be 100644 --- a/crates/fastskill-core/src/core/bundle_plan.rs +++ b/crates/fastskill-core/src/core/bundle_plan.rs @@ -23,7 +23,7 @@ pub(crate) fn preview_install( .find(|bundle| bundle.id == prepared.descriptor.id); if let Some(installed) = existing { if installed.version == prepared.descriptor.version - && installed.digest != prepared.release_digest + && !prepared.release_digest.matches(&installed.digest) { return Err(ServiceError::Validation(format!( "Bundle release '{}@{}' is immutable: its contents differ from the installed digest", @@ -36,7 +36,7 @@ pub(crate) fn preview_install( installed.id, installed.version, installed.id ))); } - if installed.digest == prepared.release_digest { + if prepared.release_digest.matches(&installed.digest) { return Ok(BundleInstallPreview { id: installed.id.clone(), current_revision: Some(installed.version.clone()), @@ -56,7 +56,7 @@ pub(crate) fn preview_install( .load_history()? .releases .get(&release_key) - .is_some_and(|known| known != &prepared.release_digest) + .is_some_and(|known| !prepared.release_digest.matches(known)) { return Err(ServiceError::Validation(format!( "Bundle release '{release_key}' is immutable: its contents differ from the previously known digest" @@ -129,8 +129,9 @@ pub(crate) fn preview_update( .iter() .find(|candidate| candidate.id == *member); let proposed = prepared.members.get(*member); - if locked.map(|candidate| candidate.digest.as_str()) - != proposed.map(|candidate| candidate.digest.as_str()) + if !locked + .zip(proposed) + .is_some_and(|(locked, proposed)| proposed.digest.matches(&locked.digest)) { changes.push(format!("replace {member}")); } else if locked.map(|candidate| candidate.overridable) @@ -225,7 +226,7 @@ pub(crate) fn validate_declared( .map(|member| DeclaredBundleMember { bundle_id: id.clone(), id: member.id.clone(), - digest: member.digest.clone(), + digest: member.digest.current.clone(), overridable: member.overridable, }), ); @@ -234,13 +235,13 @@ pub(crate) fn validate_declared( id: id.clone(), version: prepared.descriptor.version.clone(), artifact: dependency.artifact.clone(), - digest: prepared.release_digest.clone(), + digest: prepared.release_digest.current.clone(), members: prepared .members .values() .map(|member| ProjectLockedBundleMember { id: member.id.clone(), - digest: member.digest.clone(), + digest: member.digest.current.clone(), overridable: member.overridable, }) .collect(), diff --git a/crates/fastskill-core/src/core/bundle_tests.rs b/crates/fastskill-core/src/core/bundle_tests.rs index c39693d1..334780d2 100644 --- a/crates/fastskill-core/src/core/bundle_tests.rs +++ b/crates/fastskill-core/src/core/bundle_tests.rs @@ -260,3 +260,138 @@ fn artifact_and_installed_content_validation_report_invalid_versions_and_local_e .to_string() .contains("locally modified")); } + +fn author_single_member_bundle() -> (TempDir, PathBuf) { + let author = TempDir::new().unwrap(); + fs::write( + author.path().join("skill-project.toml"), + format!( + "[bundle]\nformat = \"{BUNDLE_FORMAT}\"\nid = \"team\"\nversion = \"1.0.0\"\n\n\ + [bundle.members.demo]\noverridable = false\n\n[dependencies]\ndemo = \"1.0.0\"\n" + ), + ) + .unwrap(); + let skill = author.path().join("skills/demo"); + fs::create_dir_all(&skill).unwrap(); + fs::write( + skill.join("SKILL.md"), + "---\nname: demo\nversion: 1.0.0\ndescription: demo\n---\ndemo\n", + ) + .unwrap(); + let artifact = BundleService::new(author.path(), author.path().join("skills")) + .build(author.path()) + .unwrap() + .artifact; + (author, artifact) +} + +/// Rewrite `artifact` as a release built before ADR-0017: every digest in the legacy form. +fn legacy_artifact(artifact: &Path, destination: &Path) -> PreparedBundle { + let prepared = PreparedBundle::load(artifact).unwrap(); + let extract = TempDir::new().unwrap(); + ZipHandler::new() + .unwrap() + .extract_to_dir(artifact, extract.path()) + .unwrap(); + let current = fs::read_to_string(extract.path().join("skills.lock")).unwrap(); + let member = &prepared.members["demo"]; + let legacy = current + .replace(&member.digest.current, &member.digest.legacy) + .replace( + &prepared.release_digest.current, + &prepared.release_digest.legacy, + ); + assert_ne!(legacy, current); + let legacy_lock: BundleArchiveLock = toml::from_str(&legacy).unwrap(); + let manifest = fs::read(extract.path().join("skill-project.toml")).unwrap(); + write_bundle_archive(destination, &manifest, &legacy_lock, &prepared.members).unwrap(); + prepared +} + +fn record_legacy_digests(root: &Path, prepared: &PreparedBundle) { + let lock_path = root.join("skills.lock"); + let mut lock = ProjectSkillsLock::load_from_file(&lock_path).unwrap(); + lock.bundles[0].digest = prepared.release_digest.legacy.clone(); + lock.bundles[0].members[0].digest = prepared.members["demo"].digest.legacy.clone(); + lock.save_to_file(&lock_path).unwrap(); + fs::write( + root.join(BUNDLE_HISTORY_FILE), + format!( + "[releases]\n\"team@1.0.0\" = {:?}\n", + prepared.release_digest.legacy + ), + ) + .unwrap(); +} + +#[test] +fn legacy_bundle_artifacts_and_records_keep_working_and_upgrade_on_rewrite() { + let (author, artifact) = author_single_member_bundle(); + let old_artifact = author.path().join("team-1.0.0-legacy.zip"); + let prepared = legacy_artifact(&artifact, &old_artifact); + let current_release = prepared.release_digest.current.clone(); + let current_member = prepared.members["demo"].digest.current.clone(); + assert!(current_member.starts_with(crate::core::content_digest::CONTENT_DIGEST_PREFIX)); + let (root, service) = fixture(); + let lock_path = root.path().join("skills.lock"); + + // An artifact built by an older release installs, and the Lock records the current form. + assert!(!service.install(&old_artifact).unwrap().unchanged); + let lock = ProjectSkillsLock::load_from_file(&lock_path).unwrap(); + assert_eq!(lock.bundles[0].digest, current_release); + assert_eq!(lock.bundles[0].members[0].digest, current_member); + + // A Lock and history written by an older release still identify the same release. + record_legacy_digests(root.path(), &prepared); + assert!(service.install(&artifact).unwrap().unchanged); + assert!(service.plan_install(&artifact).unwrap().changes.is_empty()); + let untouched = ProjectSkillsLock::load_from_file(&lock_path).unwrap(); + assert_eq!(untouched.bundles[0].digest, prepared.release_digest.legacy); + + // Restoring from the Lock never changes it, even to upgrade a legacy digest. + let legacy_bytes = fs::read(&lock_path).unwrap(); + fs::remove_dir_all(root.path().join("skills/demo")).unwrap(); + service.install_declared_locked().unwrap(); + assert!(root.path().join("skills/demo/SKILL.md").exists()); + assert_eq!(fs::read(&lock_path).unwrap(), legacy_bytes); + + // Any other rewrite of the records upgrades them. + fs::remove_dir_all(root.path().join("skills/demo")).unwrap(); + service.install_declared().unwrap(); + let upgraded = ProjectSkillsLock::load_from_file(&lock_path).unwrap(); + assert_eq!(upgraded.bundles[0].digest, current_release); + assert_eq!(upgraded.bundles[0].members[0].digest, current_member); + assert_eq!( + service.load_history().unwrap().releases["team@1.0.0"], + current_release + ); + + // Edit protection still honours a legacy record, and still catches an edit. + record_legacy_digests(root.path(), &prepared); + fs::write(root.path().join("skills/demo/extra.md"), "edit").unwrap(); + assert!(service + .remove("team") + .unwrap_err() + .to_string() + .contains("locally modified")); + fs::remove_file(root.path().join("skills/demo/extra.md")).unwrap(); + service.remove("team").unwrap(); + assert!(!root.path().join("skills/demo").exists()); +} + +#[test] +fn a_legacy_locked_release_that_names_other_contents_is_refused() { + let (_author, artifact) = author_single_member_bundle(); + let (root, service) = fixture(); + service.install(&artifact).unwrap(); + let lock_path = root.path().join("skills.lock"); + let mut lock = ProjectSkillsLock::load_from_file(&lock_path).unwrap(); + lock.bundles[0].members[0].digest = "0".repeat(64); + lock.save_to_file(&lock_path).unwrap(); + fs::remove_dir_all(root.path().join("skills/demo")).unwrap(); + let error = service.install_declared_locked().unwrap_err().to_string(); + assert!( + error.contains("does not match its cached artifact"), + "{error}" + ); +} diff --git a/crates/fastskill-core/src/core/bundle_types.rs b/crates/fastskill-core/src/core/bundle_types.rs index c45ef4f3..b6fac2a7 100644 --- a/crates/fastskill-core/src/core/bundle_types.rs +++ b/crates/fastskill-core/src/core/bundle_types.rs @@ -3,7 +3,8 @@ use crate::core::bundle_archive::BundleArchiveLock; use crate::core::bundle_persistence::{ parse_bundle_project, prepare_members, BundleOverrideDeclaration, }; -use crate::core::lock::ProjectLockedBundleEntry; +use crate::core::content_digest::DigestForms; +use crate::core::lock::{ProjectLockedBundleEntry, ProjectLockedBundleMember}; use crate::core::manifest::{DependencySpec, SkillProjectToml}; use crate::core::service::{ServiceError, SkillId}; use crate::storage::zip::ZipHandler; @@ -146,7 +147,7 @@ pub(crate) struct PreparedBundle { _temporary: TempDir, pub(crate) descriptor: BundleDescriptor, pub(crate) members: BTreeMap, - pub(crate) release_digest: String, + pub(crate) release_digest: DigestForms, } impl PreparedBundle { @@ -172,9 +173,9 @@ impl PreparedBundle { archive_lock.verify(&descriptor, &members)?; Ok(Self { _temporary: temporary, + release_digest: BundleArchiveLock::release_digests(&descriptor, &members), descriptor, members, - release_digest: archive_lock.release_digest().to_string(), }) } @@ -185,12 +186,13 @@ impl PreparedBundle { let members_match = expected.members.len() == self.members.len() && expected.members.iter().all(|locked| { self.members.get(&locked.id).is_some_and(|member| { - member.digest == locked.digest && member.overridable == locked.overridable + member.digest.matches(&locked.digest) + && member.overridable == locked.overridable }) }); if self.descriptor.id != expected.id || self.descriptor.version != expected.version - || self.release_digest != expected.digest + || !self.release_digest.matches(&expected.digest) || !members_match { return Err(ServiceError::Validation(format!( @@ -200,13 +202,47 @@ impl PreparedBundle { } Ok(()) } + + /// The Lock entry that records this release at `artifact`. A locked restore passes the + /// entry it verified as `recorded`, and its digests are kept as written, so restoring + /// from a Lock never changes it. Every other write records the current form. + pub(crate) fn lock_entry( + &self, + artifact: String, + recorded: Option<&ProjectLockedBundleEntry>, + ) -> ProjectLockedBundleEntry { + let recorded_member = |id: &str| { + recorded.and_then(|entry| entry.members.iter().find(|member| member.id == id)) + }; + ProjectLockedBundleEntry { + id: self.descriptor.id.clone(), + version: self.descriptor.version.clone(), + artifact, + digest: recorded.map_or_else( + || self.release_digest.current.clone(), + |entry| entry.digest.clone(), + ), + members: self + .members + .values() + .map(|member| ProjectLockedBundleMember { + id: member.id.clone(), + digest: recorded_member(&member.id).map_or_else( + || member.digest.current.clone(), + |locked| locked.digest.clone(), + ), + overridable: member.overridable, + }) + .collect(), + } + } } #[derive(Debug, Clone)] pub(crate) struct PreparedMember { pub(crate) id: String, pub(crate) source: PathBuf, - pub(crate) digest: String, + pub(crate) digest: DigestForms, pub(crate) overridable: bool, } diff --git a/crates/fastskill-core/src/core/content_digest.rs b/crates/fastskill-core/src/core/content_digest.rs new file mode 100644 index 00000000..b1a4bbf4 --- /dev/null +++ b/crates/fastskill-core/src/core/content_digest.rs @@ -0,0 +1,291 @@ +//! The content digest: one string that identifies the exact files of an installed skill +//! directory. Locks pin it as `checksum`, bundle records and bundle archives pin it per +//! member, and install, update and remove compare it with what is on disk before they touch +//! anything. See [ADR-0017](../../../../docs/adr/0017-versioned-content-digests.md). +//! +//! # Current form +//! +//! `sha256-tree-v2:<64 lowercase hex>` — SHA-256 over: +//! +//! 1. the domain tag [`V2_DOMAIN`], length-prefixed; +//! 2. for every regular file, in byte order of its path: the path relative to the directory +//! (UTF-8, components joined with `/`), length-prefixed, then the file's contents, +//! length-prefixed. +//! +//! Every length is a big-endian `u64`, so the byte stream decodes to exactly one tree. +//! Symbolic links are refused. File names that are not valid UTF-8 are refused, because +//! they have no portable spelling. +//! +//! # Legacy form +//! +//! Before ADR-0017 the digest was 64 bare hex characters, and it hashed file contents +//! without their length, so the bytes of one file could stand in for the next file's path +//! and contents. Legacy digests are still accepted when they match, with a warning, so that +//! Locks and bundles written by older releases keep installing. Every digest fastskill +//! writes is in the current form; a legacy value is replaced whenever its record is +//! rewritten. + +use crate::core::service::ServiceError; +use sha2::{Digest, Sha256}; +use std::fs; +use std::io::Read; +use std::path::{Component, Path}; +use walkdir::WalkDir; + +/// Prefix of every current-form content digest. +pub const CONTENT_DIGEST_PREFIX: &str = "sha256-tree-v2:"; + +/// Domain tag hashed first, so a current-form digest never equals a hash of anything else. +const V2_DOMAIN: &str = "fastskill content digest v2"; + +/// Compute the current-form content digest of the skill directory at `path`. +pub fn content_digest(path: &Path) -> Result { + ensure_directory(path)?; + let mut files = Vec::new(); + for entry in WalkDir::new(path) { + let entry = entry.map_err(|error| ServiceError::Io(std::io::Error::other(error)))?; + refuse_symlink(&entry)?; + if entry.file_type().is_file() { + files.push(( + portable_relative_path(path, entry.path())?, + entry.into_path(), + )); + } + } + files.sort_by(|left, right| left.0.cmp(&right.0)); + + let mut hasher = Sha256::new(); + hash_field(&mut hasher, V2_DOMAIN); + for (relative, file) in &files { + hash_field(&mut hasher, relative); + hash_file_framed(&mut hasher, file)?; + } + Ok(format!( + "{CONTENT_DIGEST_PREFIX}{}", + crate::utils::to_hex_lower(&hasher.finalize()) + )) +} + +/// Whether `expected` names the contents of the directory at `path`. +/// +/// A legacy digest is compared with the legacy algorithm and, when it matches, a warning +/// says the record is weaker than the current form. Any other value is compared with +/// [`content_digest`], so only a current-form digest can match. Content that cannot be read +/// is an error whatever the recorded form. +pub fn content_digest_matches(expected: &str, path: &Path) -> Result { + if !is_legacy_digest(expected) { + // An unrecognised record never matches, but unreadable content is still an error. + return Ok(content_digest(path)? == expected); + } + let matches = legacy_content_digest(path)? == expected; + if matches { + warn_legacy_digest_accepted(); + } + Ok(matches) +} + +/// The digest to record for content that matched `stored`: the current form of `path` +/// when `stored` is a matching legacy digest, and `stored` unchanged otherwise. Used where +/// a rewrite carries a recorded digest forward instead of recomputing it. +pub fn upgraded_digest(stored: &str, path: &Path) -> Result { + if is_legacy_digest(stored) && content_digest_matches(stored, path)? { + return content_digest(path); + } + Ok(stored.to_string()) +} + +/// Whether `value` has the legacy form: exactly 64 lowercase hex characters, no prefix. +pub fn is_legacy_digest(value: &str) -> bool { + value.len() == 64 + && value + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) +} + +/// Whether every recorded digest names the contents of `path`. +pub fn all_match(recorded: &[&str], path: &Path) -> Result { + for digest in recorded { + if !content_digest_matches(digest, path)? { + return Ok(false); + } + } + Ok(true) +} + +/// Whether recorded digests disagree about one skill's contents. Two digests of the same +/// form disagree when they differ. A current-form and a legacy digest cannot be compared +/// without the content, so they are not reported here; callers check each against the +/// content itself. +pub fn recorded_digests_conflict<'a>(digests: impl IntoIterator) -> bool { + let digests: Vec<&str> = digests.into_iter().collect(); + digests.iter().enumerate().any(|(index, left)| { + digests[index + 1..] + .iter() + .any(|right| left != right && is_legacy_digest(left) == is_legacy_digest(right)) + }) +} + +/// A directory's digest in both forms, for content that is compared with recorded digests +/// of either form many times, such as the members of a bundle artifact. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct DigestForms { + /// The form fastskill records. + pub(crate) current: String, + /// The legacy form, used only to accept records written by older releases. + pub(crate) legacy: String, +} + +impl DigestForms { + pub(crate) fn of_directory(path: &Path) -> Result { + Ok(Self { + current: content_digest(path)?, + legacy: legacy_content_digest(path)?, + }) + } + + /// Whether a recorded digest names this content, in either form. + pub(crate) fn matches(&self, recorded: &str) -> bool { + if recorded == self.current { + return true; + } + let matches = recorded == self.legacy; + if matches { + warn_legacy_digest_accepted(); + } + matches + } +} + +/// The digest algorithm before ADR-0017. Kept only to verify records older releases wrote; +/// never used to write a new record. +pub(crate) fn legacy_content_digest(path: &Path) -> Result { + ensure_directory(path)?; + let mut hasher = Sha256::new(); + for entry in WalkDir::new(path).sort_by_file_name() { + let entry = entry.map_err(|error| ServiceError::Io(std::io::Error::other(error)))?; + refuse_symlink(&entry)?; + if !entry.file_type().is_file() { + continue; + } + let relative = entry.path().strip_prefix(path).map_err(|error| { + ServiceError::Custom(format!("Failed to form skill digest path: {error}")) + })?; + hash_field(&mut hasher, relative.to_string_lossy().replace('\\', "/")); + let mut file = fs::File::open(entry.path()).map_err(ServiceError::Io)?; + let mut buffer = [0u8; 8192]; + loop { + let read = file.read(&mut buffer).map_err(ServiceError::Io)?; + if read == 0 { + break; + } + hasher.update(&buffer[..read]); + } + } + Ok(crate::utils::to_hex_lower(&hasher.finalize())) +} + +/// Hash `value` with its length, so that consecutive fields cannot run into each other. +pub(crate) fn hash_field(hasher: &mut Sha256, value: impl AsRef<[u8]>) { + let value = value.as_ref(); + hasher.update((value.len() as u64).to_be_bytes()); + hasher.update(value); +} + +fn hash_file_framed(hasher: &mut Sha256, path: &Path) -> Result<(), ServiceError> { + let file = fs::File::open(path).map_err(ServiceError::Io)?; + let length = file.metadata().map_err(ServiceError::Io)?.len(); + hash_framed(hasher, file, length, path) +} + +/// Hash `length`, then exactly `length` bytes from `reader`. A reader that yields more or +/// fewer bytes means the file changed after its length was taken. +fn hash_framed( + hasher: &mut Sha256, + mut reader: impl Read, + length: u64, + path: &Path, +) -> Result<(), ServiceError> { + hasher.update(length.to_be_bytes()); + let mut remaining = length; + let mut buffer = [0u8; 8192]; + loop { + let read = reader.read(&mut buffer).map_err(ServiceError::Io)?; + if read == 0 { + break; + } + remaining = remaining + .checked_sub(read as u64) + .ok_or_else(|| changed(path))?; + hasher.update(&buffer[..read]); + } + if remaining != 0 { + return Err(changed(path)); + } + Ok(()) +} + +fn changed(path: &Path) -> ServiceError { + ServiceError::Validation(format!( + "{} changed while its content digest was computed; retry", + path.display() + )) +} + +/// The path of `file` relative to `root`, as UTF-8 components joined with `/`. Joining +/// components, rather than rewriting separators in a string, keeps a Unix file name that +/// contains `\` distinct from a nested path. +fn portable_relative_path(root: &Path, file: &Path) -> Result { + let relative = file.strip_prefix(root).map_err(|error| { + ServiceError::Custom(format!("Failed to form skill digest path: {error}")) + })?; + let mut parts = Vec::new(); + for component in relative.components() { + let Component::Normal(name) = component else { + return Err(ServiceError::Custom(format!( + "Unexpected path component in {}", + file.display() + ))); + }; + parts.push(name.to_str().ok_or_else(|| { + ServiceError::Validation(format!( + "File name is not valid UTF-8, so it has no portable content digest: {}", + file.display() + )) + })?); + } + Ok(parts.join("/")) +} + +fn ensure_directory(path: &Path) -> Result<(), ServiceError> { + if path.is_dir() { + return Ok(()); + } + Err(ServiceError::Validation(format!( + "Expected skill directory at {}", + path.display() + ))) +} + +fn refuse_symlink(entry: &walkdir::DirEntry) -> Result<(), ServiceError> { + if entry.file_type().is_symlink() { + return Err(ServiceError::Validation(format!( + "Symbolic links are not permitted in bundle members: {}", + entry.path().display() + ))); + } + Ok(()) +} + +fn warn_legacy_digest_accepted() { + crate::utils::warn_once( + "legacy-content-digest", + "accepted a legacy content digest written by an older fastskill; legacy digests are \ + weaker than the current `sha256-tree-v2` form, and fastskill replaces each one the \ + next time it rewrites that record (ADR-0017)", + ); +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +#[path = "content_digest_tests.rs"] +mod tests; diff --git a/crates/fastskill-core/src/core/content_digest_tests.rs b/crates/fastskill-core/src/core/content_digest_tests.rs new file mode 100644 index 00000000..8a40ca9a --- /dev/null +++ b/crates/fastskill-core/src/core/content_digest_tests.rs @@ -0,0 +1,258 @@ +use super::*; +use tempfile::TempDir; + +fn skill(root: &Path, name: &str, files: &[(&str, &[u8])]) -> std::path::PathBuf { + let directory = root.join(name); + for (relative, contents) in files { + let path = directory.join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); + } + fs::create_dir_all(&directory).unwrap(); + directory +} + +/// A one-file tree whose bytes spell out a second file, as the legacy digest saw it. +fn boundary_pair(root: &Path) -> (std::path::PathBuf, std::path::PathBuf) { + let split = skill(root, "split", &[("a.bin", b"head"), ("z.sh", b"tail")]); + let mut joined = b"head".to_vec(); + joined.extend_from_slice(&(b"z.sh".len() as u64).to_be_bytes()); + joined.extend_from_slice(b"z.sh"); + joined.extend_from_slice(b"tail"); + let merged = skill(root, "merged", &[("a.bin", &joined)]); + (split, merged) +} + +#[test] +fn current_form_is_prefixed_and_stable() { + let root = TempDir::new().unwrap(); + let first = skill(root.path(), "first", &[("SKILL.md", b"x"), ("sub/b", b"y")]); + let second = skill( + root.path(), + "second", + &[("sub/b", b"y"), ("SKILL.md", b"x")], + ); + let digest = content_digest(&first).unwrap(); + let hex = digest.strip_prefix(CONTENT_DIGEST_PREFIX).unwrap(); + assert_eq!(hex.len(), 64); + assert!( + is_legacy_digest(hex), + "the hex part is 64 lowercase hex characters" + ); + assert!(!is_legacy_digest(&digest)); + assert_eq!(digest, content_digest(&second).unwrap()); + assert_ne!(digest, legacy_content_digest(&first).unwrap()); +} + +#[test] +fn legacy_form_collides_where_the_current_form_does_not() { + let root = TempDir::new().unwrap(); + let (split, merged) = boundary_pair(root.path()); + assert_eq!( + legacy_content_digest(&split).unwrap(), + legacy_content_digest(&merged).unwrap() + ); + assert_ne!( + content_digest(&split).unwrap(), + content_digest(&merged).unwrap() + ); +} + +#[test] +fn contents_moving_between_files_change_the_digest() { + let root = TempDir::new().unwrap(); + let left = skill(root.path(), "left", &[("a", b"xy"), ("b", b"z")]); + let right = skill(root.path(), "right", &[("a", b"x"), ("b", b"yz")]); + assert_ne!( + content_digest(&left).unwrap(), + content_digest(&right).unwrap() + ); +} + +#[test] +fn empty_directories_do_not_contribute() { + let root = TempDir::new().unwrap(); + let plain = skill(root.path(), "plain", &[("SKILL.md", b"x")]); + let with_empty = skill(root.path(), "with-empty", &[("SKILL.md", b"x")]); + fs::create_dir_all(with_empty.join("empty/nested")).unwrap(); + assert_eq!( + content_digest(&plain).unwrap(), + content_digest(&with_empty).unwrap() + ); +} + +#[test] +fn missing_directory_is_refused() { + let root = TempDir::new().unwrap(); + let missing = root.path().join("missing"); + assert!(content_digest(&missing).is_err()); + assert!(legacy_content_digest(&missing).is_err()); + assert!(content_digest_matches(&"0".repeat(64), &missing).is_err()); + assert!(content_digest_matches("unknown", &missing).is_err()); +} + +#[test] +fn framed_reader_must_yield_exactly_its_length() { + let path = Path::new("file"); + let mut hasher = Sha256::new(); + assert!(hash_framed(&mut hasher, &b"abc"[..], 3, path).is_ok()); + let longer = hash_framed(&mut hasher, &b"abc"[..], 2, path).unwrap_err(); + assert!(longer.to_string().contains("changed while")); + assert!(hash_framed(&mut hasher, &b"abc"[..], 4, path).is_err()); +} + +#[test] +fn matching_dispatches_on_the_recorded_form() { + let root = TempDir::new().unwrap(); + let (split, merged) = boundary_pair(root.path()); + let current = content_digest(&split).unwrap(); + let legacy = legacy_content_digest(&split).unwrap(); + + assert!(content_digest_matches(¤t, &split).unwrap()); + assert!(!content_digest_matches(¤t, &merged).unwrap()); + assert!(content_digest_matches(&legacy, &split).unwrap()); + // The legacy form cannot tell these trees apart; that is the residual risk it carries. + assert!(content_digest_matches(&legacy, &merged).unwrap()); + let other = skill(root.path(), "other", &[("SKILL.md", b"other")]); + assert!(!content_digest_matches(&legacy, &other).unwrap()); + + for unknown in [ + "", + "abc", + "sha256:0000", + &legacy.to_uppercase(), + &format!("sha256-tree-v3:{legacy}"), + ] { + assert!( + !content_digest_matches(unknown, &split).unwrap(), + "{unknown}" + ); + } +} + +#[test] +fn upgrade_replaces_only_a_matching_legacy_digest() { + let root = TempDir::new().unwrap(); + let first = skill(root.path(), "first", &[("SKILL.md", b"one")]); + let second = skill(root.path(), "second", &[("SKILL.md", b"two")]); + let current = content_digest(&first).unwrap(); + let legacy = legacy_content_digest(&first).unwrap(); + + assert_eq!(upgraded_digest(&legacy, &first).unwrap(), current); + assert_eq!(upgraded_digest(&legacy, &second).unwrap(), legacy); + assert_eq!(upgraded_digest(¤t, &second).unwrap(), current); +} + +#[test] +fn recorded_digests_conflict_only_within_one_form() { + let current_a = format!("{CONTENT_DIGEST_PREFIX}{}", "a".repeat(64)); + let current_b = format!("{CONTENT_DIGEST_PREFIX}{}", "b".repeat(64)); + let legacy_a = "a".repeat(64); + let legacy_b = "b".repeat(64); + assert!(!recorded_digests_conflict([])); + assert!(!recorded_digests_conflict([ + current_a.as_str(), + current_a.as_str() + ])); + assert!(recorded_digests_conflict([ + current_a.as_str(), + current_b.as_str() + ])); + assert!(recorded_digests_conflict([ + legacy_a.as_str(), + legacy_b.as_str() + ])); + assert!(!recorded_digests_conflict([ + current_a.as_str(), + legacy_b.as_str() + ])); + assert!(recorded_digests_conflict([ + legacy_a.as_str(), + current_a.as_str(), + legacy_b.as_str() + ])); +} + +#[test] +fn all_recorded_digests_must_name_the_content() { + let root = TempDir::new().unwrap(); + let first = skill(root.path(), "first", &[("SKILL.md", b"one")]); + let second = skill(root.path(), "second", &[("SKILL.md", b"two")]); + let current = content_digest(&first).unwrap(); + let legacy = legacy_content_digest(&first).unwrap(); + assert!(all_match(&[], &first).unwrap()); + assert!(all_match(&[¤t, &legacy], &first).unwrap()); + assert!(!all_match(&[¤t, &legacy], &second).unwrap()); +} + +#[test] +fn digest_forms_accept_either_form_of_their_own_content() { + let root = TempDir::new().unwrap(); + let first = skill(root.path(), "first", &[("SKILL.md", b"one")]); + let second = skill(root.path(), "second", &[("SKILL.md", b"two")]); + let forms = DigestForms::of_directory(&first).unwrap(); + assert_eq!(forms.current, content_digest(&first).unwrap()); + assert!(forms.matches(&forms.current)); + assert!(forms.matches(&forms.legacy)); + assert!(!forms.matches(&content_digest(&second).unwrap())); + assert!(!forms.matches(&legacy_content_digest(&second).unwrap())); +} + +#[cfg(unix)] +#[test] +fn backslash_in_a_file_name_is_not_a_directory_separator() { + let root = TempDir::new().unwrap(); + let flat = skill(root.path(), "flat", &[("a\\b", b"x")]); + let nested = skill(root.path(), "nested", &[("a/b", b"x")]); + assert_eq!( + legacy_content_digest(&flat).unwrap(), + legacy_content_digest(&nested).unwrap() + ); + assert_ne!( + content_digest(&flat).unwrap(), + content_digest(&nested).unwrap() + ); +} + +#[cfg(unix)] +#[test] +fn non_utf8_file_names_are_refused() { + use std::os::unix::ffi::OsStrExt; + let root = TempDir::new().unwrap(); + let directory = skill(root.path(), "skill", &[("SKILL.md", b"x")]); + let name = std::ffi::OsStr::from_bytes(b"bad-\xff"); + if fs::write(directory.join(name), b"x").is_err() { + return; // The filesystem itself refuses non-UTF-8 names. + } + let error = content_digest(&directory).unwrap_err(); + assert!(error.to_string().contains("not valid UTF-8")); + assert!(legacy_content_digest(&directory).is_ok()); +} + +#[cfg(unix)] +#[test] +fn symbolic_links_are_refused() { + let root = TempDir::new().unwrap(); + let directory = skill(root.path(), "skill", &[("SKILL.md", b"x")]); + std::os::unix::fs::symlink(directory.join("SKILL.md"), directory.join("link")).unwrap(); + assert!(content_digest(&directory).is_err()); + assert!(legacy_content_digest(&directory).is_err()); +} + +#[cfg(unix)] +#[test] +fn unreadable_subdirectories_are_reported() { + use std::os::unix::fs::PermissionsExt; + let root = TempDir::new().unwrap(); + let directory = skill(root.path(), "skill", &[("SKILL.md", b"x"), ("sub/f", b"y")]); + let sub = directory.join("sub"); + fs::set_permissions(&sub, fs::Permissions::from_mode(0o000)).unwrap(); + let readable = fs::read_dir(&sub).is_ok(); // true when running with elevated privileges + let current = content_digest(&directory); + let legacy = legacy_content_digest(&directory); + fs::set_permissions(&sub, fs::Permissions::from_mode(0o755)).unwrap(); + if !readable { + assert!(current.is_err()); + assert!(legacy.is_err()); + } +} diff --git a/crates/fastskill-core/src/core/install.rs b/crates/fastskill-core/src/core/install.rs index 446b36aa..49fe1a81 100644 --- a/crates/fastskill-core/src/core/install.rs +++ b/crates/fastskill-core/src/core/install.rs @@ -6,7 +6,6 @@ //! skills dir → upsert Manifest → write Lock → reindex-if-provider). `mode` only //! governs the id-conflict policy. `add`/`update` are one operation. -use crate::core::bundle_persistence::digest_directory; use crate::core::cache::{CacheIdentity, SkillCache, SourceIndex, SourceIndexEntry, ZipValidator}; use crate::core::lock::{project_lock_path, ProjectSkillsLock}; use crate::core::manifest::{ @@ -78,9 +77,11 @@ pub struct PreparedSkill { dependencies: Vec, } -/// Compute the canonical content digest used by Lock and bundle ownership. +/// Compute the canonical content digest used by Lock and bundle ownership, in the current +/// `sha256-tree-v2:` form. Compare a recorded digest with +/// [`crate::core::content_digest::content_digest_matches`], which also accepts the legacy form. pub fn content_digest(path: &Path) -> Result { - digest_directory(path) + crate::core::content_digest::content_digest(path) } impl FastSkillService { @@ -267,6 +268,7 @@ impl FastSkillService { &recorded_origin, expected, &fetched.resolved, + &fetched.skill_path, )?; } @@ -361,7 +363,7 @@ impl FastSkillService { resolved: Resolved { version, commit_hash: expected.commit_hash.clone(), - checksum: Some(digest_directory(&skill_path)?), + checksum: Some(content_digest(&skill_path)?), }, skill_path, }) @@ -488,7 +490,7 @@ impl FastSkillService { let frontmatter = read_skill_frontmatter(&skill_path).await?; let (_, version) = derive_skill_id_and_version(&skill_path, &frontmatter)?; - let checksum = digest_directory(&skill_path)?; + let checksum = content_digest(&skill_path)?; Ok(Fetched { temp_dir, skill_path, @@ -547,7 +549,7 @@ impl FastSkillService { let checksum = if editable { None } else { - Some(digest_directory(&skill_path)?) + Some(content_digest(&skill_path)?) }; Ok(Fetched { temp_dir, @@ -579,7 +581,7 @@ impl FastSkillService { let frontmatter = read_skill_frontmatter(&skill_path).await?; let (_, version) = derive_skill_id_and_version(&skill_path, &frontmatter)?; - let checksum = digest_directory(&skill_path)?; + let checksum = content_digest(&skill_path)?; Ok(Fetched { temp_dir, skill_path, @@ -662,7 +664,7 @@ impl FastSkillService { } } - let checksum = digest_directory(&skill_path)?; + let checksum = content_digest(&skill_path)?; Ok(Fetched { temp_dir, skill_path, diff --git a/crates/fastskill-core/src/core/install/prepared_apply.rs b/crates/fastskill-core/src/core/install/prepared_apply.rs index 3fc7abad..f1b8c1a1 100644 --- a/crates/fastskill-core/src/core/install/prepared_apply.rs +++ b/crates/fastskill-core/src/core/install/prepared_apply.rs @@ -13,6 +13,19 @@ impl PreparedSkill { pub fn dependencies(&self) -> &[crate::core::manifest::SkillEntry] { &self.dependencies } + + /// Whether a recorded content digest, in either form, names this candidate's contents. + /// A candidate without a content digest (an editable local origin) matches nothing. + pub fn checksum_matches(&self, expected: &str) -> Result { + match self.fetched.resolved.checksum.as_deref() { + None => Ok(false), + Some(actual) if actual == expected => Ok(true), + Some(_) => crate::core::content_digest::content_digest_matches( + expected, + &self.fetched.skill_path, + ), + } + } } impl FastSkillService { diff --git a/crates/fastskill-core/src/core/install/tests.rs b/crates/fastskill-core/src/core/install/tests.rs index 1d941188..37b96adf 100644 --- a/crates/fastskill-core/src/core/install/tests.rs +++ b/crates/fastskill-core/src/core/install/tests.rs @@ -765,6 +765,33 @@ async fn offline_preparation_restores_cached_repository_and_zip_artifacts() { restored.resolved().checksum.as_deref(), Some(checksum.as_str()) ); + // A Lock written before ADR-0017 pins the legacy form. It still verifies, and the + // prepared facts, which become the next Lock entry, carry the current form. + let legacy = crate::core::content_digest::legacy_content_digest(&cached).unwrap(); + let legacy_expected = Resolved { + checksum: Some(legacy.clone()), + ..expected.clone() + }; + let upgraded = service + .prepare_install_offline(repository_origin.clone(), "test-skill", &legacy_expected) + .await + .unwrap(); + assert_eq!( + upgraded.resolved().checksum.as_deref(), + Some(checksum.as_str()) + ); + assert!(upgraded.checksum_matches(&legacy).unwrap()); + assert!(upgraded.checksum_matches(&checksum).unwrap()); + assert!(!upgraded.checksum_matches(&"0".repeat(64)).unwrap()); + let tampered = Resolved { + checksum: Some("0".repeat(64)), + ..expected.clone() + }; + let error = service + .prepare_install_offline(repository_origin.clone(), "test-skill", &tampered) + .await + .unwrap_err(); + assert!(error.to_string().contains("locked checksum"), "{error}"); let fresh = service .prepare_add_offline(repository_origin, Some("test-skill")) .await @@ -1714,3 +1741,27 @@ fn plain_git_urls_pass_the_browser_url_guard() { assert!(reject_github_tree_url(url).is_ok(), "{url} must be allowed"); } } + +/// Two different trees must never share a content digest. A file's bytes used +/// to be hashed without their length, so the boundary between one file and +/// the next was not part of the digest. +#[test] +fn content_digest_distinguishes_file_boundaries() { + let root = TestTempDir::new().unwrap(); + let split = write_valid_skill(root.path(), "split"); + std::fs::write(split.join("a.bin"), b"head").unwrap(); + std::fs::write(split.join("z.sh"), b"tail").unwrap(); + + let merged = write_valid_skill(root.path(), "merged"); + let mut joined = b"head".to_vec(); + joined.extend_from_slice(&(b"z.sh".len() as u64).to_be_bytes()); + joined.extend_from_slice(b"z.sh"); + joined.extend_from_slice(b"tail"); + std::fs::write(merged.join("a.bin"), joined).unwrap(); + + assert_ne!( + content_digest(&split).unwrap(), + content_digest(&merged).unwrap(), + "a one-file tree and a two-file tree produced the same content digest" + ); +} diff --git a/crates/fastskill-core/src/core/install/verification.rs b/crates/fastskill-core/src/core/install/verification.rs index ba468e38..0f4f6f4f 100644 --- a/crates/fastskill-core/src/core/install/verification.rs +++ b/crates/fastskill-core/src/core/install/verification.rs @@ -1,6 +1,8 @@ use super::support::{git_ref_cache_key, resolve_registry_version, resolve_repo_name}; +use crate::core::content_digest::content_digest_matches; use crate::core::origin::{GitRef, Origin, Resolved}; use crate::core::service::{FastSkillService, ServiceError}; +use std::path::Path; pub(super) fn offline_resolution( service: &FastSkillService, @@ -59,11 +61,15 @@ pub(super) fn offline_resolution( } } +/// Check fetched facts against the Lock. `skill_path` holds the fetched contents; a Lock +/// written before ADR-0017 pins a legacy content digest, which is checked against those +/// contents. The Lock written afterwards records `actual`, so the pin is upgraded. pub(super) fn verify_resolved_facts( id: &str, origin: &Origin, expected: &Resolved, actual: &Resolved, + skill_path: &Path, ) -> Result<(), ServiceError> { if expected.version != actual.version { return Err(ServiceError::Validation(format!( @@ -82,7 +88,9 @@ pub(super) fn verify_resolved_facts( "locked skill '{id}' has no checksum; run an explicit update to establish verified contents" )) })?; - if actual.checksum.as_ref() != Some(expected_checksum) { + if actual.checksum.as_ref() != Some(expected_checksum) + && !content_digest_matches(expected_checksum, skill_path)? + { return Err(ServiceError::Validation(format!( "locked checksum for '{id}' does not match the fetched contents" ))); @@ -111,6 +119,10 @@ mod tests { use std::path::PathBuf; use tempfile::TempDir; + fn missing() -> &'static Path { + Path::new("/nonexistent/fastskill-verification") + } + fn resolved(version: &str, commit: Option<&str>, checksum: Option<&str>) -> Resolved { Resolved { version: version.to_string(), @@ -123,24 +135,31 @@ mod tests { fn immutable_facts_must_all_match() { let origin = Origin::ZipUrl { url: "u".into() }; let expected = resolved("1.0.0", None, Some("abc")); - assert!(verify_resolved_facts("x", &origin, &expected, &expected).is_ok()); + assert!(verify_resolved_facts("x", &origin, &expected, &expected, missing()).is_ok()); + assert!(verify_resolved_facts( + "x", + &origin, + &expected, + &resolved("2.0.0", None, Some("abc")), + missing() + ) + .is_err()); assert!(verify_resolved_facts( "x", &origin, &expected, - &resolved("2.0.0", None, Some("abc")) + &resolved("1.0.0", None, Some("bad")), + missing() ) .is_err()); assert!(verify_resolved_facts( "x", &origin, + &resolved("1.0.0", None, None), &expected, - &resolved("1.0.0", None, Some("bad")) + missing() ) .is_err()); - assert!( - verify_resolved_facts("x", &origin, &resolved("1.0.0", None, None), &expected).is_err() - ); } #[test] @@ -163,7 +182,8 @@ mod tests { "x", &git, &resolved("1.0.0", Some("a"), Some("c")), - &resolved("1.0.0", Some("b"), Some("c")) + &resolved("1.0.0", Some("b"), Some("c")), + missing() ) .is_err()); let editable = Origin::Local { @@ -174,7 +194,8 @@ mod tests { "x", &editable, &resolved("1.0.0", None, None), - &resolved("1.0.0", None, None) + &resolved("1.0.0", None, None), + missing() ) .is_ok()); } diff --git a/crates/fastskill-core/src/core/mod.rs b/crates/fastskill-core/src/core/mod.rs index b59d5763..facfbb69 100644 --- a/crates/fastskill-core/src/core/mod.rs +++ b/crates/fastskill-core/src/core/mod.rs @@ -13,6 +13,7 @@ pub(crate) mod bundle_types; pub mod cache; pub mod change_detection; pub mod contained_path; +pub mod content_digest; pub mod context_resolver; pub mod dependencies; pub mod embedding; diff --git a/crates/fastskill-core/src/core/project_apply.rs b/crates/fastskill-core/src/core/project_apply.rs index 00a29ffa..e5df924a 100644 --- a/crates/fastskill-core/src/core/project_apply.rs +++ b/crates/fastskill-core/src/core/project_apply.rs @@ -1,6 +1,7 @@ //! Transactional application of a verified project dependency plan. use crate::core::contained_path::{remove_contained, ContainedPath}; +use crate::core::content_digest::{all_match, content_digest_matches, recorded_digests_conflict}; use crate::core::install::PreparedSkill; use crate::core::lifecycle_transaction::LifecycleTransaction; use crate::core::lock::{ProjectLockedSkillEntry, ProjectSkillsLock}; @@ -590,7 +591,7 @@ fn validate_bundle_ownership( } else { member.digest.as_str() }; - if candidate.prepared.resolved().checksum.as_deref() != Some(expected) { + if !candidate.prepared.checksum_matches(expected)? { return Err(ServiceError::InvalidOperation(format!( "individual skill '{}' conflicts with installed bundle '{}'", member.id, bundle.id @@ -631,8 +632,7 @@ fn validate_installed_content( "personal override '{id}' is no longer permitted by every bundle owner" ))); } - let actual = crate::core::install::content_digest(&installed)?; - if actual != active_override.digest { + if !content_digest_matches(&active_override.digest, &installed)? { return Err(ServiceError::InvalidOperation(format!( "installed skill '{id}' was modified; restore or remove local edits before changing managed state" ))); @@ -656,19 +656,16 @@ fn validate_installed_content( .filter(|member| member.id == *id) .map(|member| member.digest.as_str()), ); - if expected.windows(2).any(|pair| pair[0] != pair[1]) { + if recorded_digests_conflict(expected.iter().copied()) { return Err(ServiceError::InvalidOperation(format!( "installed skill '{id}' has conflicting recorded ownership digests" ))); } - if let Some(expected) = expected.first() { - let actual = crate::core::install::content_digest(&installed)?; - if actual != **expected { - return Err(ServiceError::InvalidOperation(format!( - "installed skill '{}' was modified; restore or remove local edits before changing managed state", - id - ))); - } + if !expected.is_empty() && !all_match(&expected, &installed)? { + return Err(ServiceError::InvalidOperation(format!( + "installed skill '{}' was modified; restore or remove local edits before changing managed state", + id + ))); } } Ok(()) diff --git a/crates/fastskill-core/src/core/project_removal.rs b/crates/fastskill-core/src/core/project_removal.rs index d9ced48a..33287291 100644 --- a/crates/fastskill-core/src/core/project_removal.rs +++ b/crates/fastskill-core/src/core/project_removal.rs @@ -247,8 +247,7 @@ impl ProjectRemovalService { "Skill '{id}' has no locked content digest; restore it before removal so FastSkill can protect local edits" )) })?; - let actual = managed_tree_digest(&path)?; - if &actual != expected { + if !crate::core::content_digest::content_digest_matches(expected, &path)? { return Err(ServiceError::InvalidOperation(format!( "Skill '{id}' is locally modified; FastSkill will not delete it" ))); @@ -276,9 +275,11 @@ fn load_and_plan( Ok((manifest, lock, plan)) } -/// Canonical digest used for immutable installed-tree edit protection. +/// Canonical digest used for immutable installed-tree edit protection, in the current +/// `sha256-tree-v2:` form. Compare a recorded digest with +/// [`crate::core::content_digest::content_digest_matches`], which also accepts the legacy form. pub fn managed_tree_digest(path: &Path) -> Result { - crate::core::bundle_persistence::digest_directory(path) + crate::core::content_digest::content_digest(path) } #[cfg(test)] diff --git a/crates/fastskill-core/src/http/handlers/skills_update.rs b/crates/fastskill-core/src/http/handlers/skills_update.rs index 2df7cb9b..ff5b05bb 100644 --- a/crates/fastskill-core/src/http/handlers/skills_update.rs +++ b/crates/fastskill-core/src/http/handlers/skills_update.rs @@ -433,7 +433,8 @@ fn candidate_is_current( .zip(path.canonicalize().ok()) .is_some_and(|(installed, source)| installed == source), _ => entry.resolved.checksum.as_ref().is_some_and(|expected| { - crate::core::install::content_digest(&installed).is_ok_and(|actual| &actual == expected) + crate::core::content_digest::content_digest_matches(expected, &installed) + .unwrap_or(false) }), } } diff --git a/docs/adr/0016-machines-follow-a-signed-managed-state.md b/docs/adr/0016-machines-follow-a-signed-managed-state.md index 5a218234..c899a22d 100644 --- a/docs/adr/0016-machines-follow-a-signed-managed-state.md +++ b/docs/adr/0016-machines-follow-a-signed-managed-state.md @@ -106,7 +106,8 @@ It doesn't protect against: - a digest is in `blocked` and also in `skills` or `also_allowed` - two `skills` entries share an id - an id isn't a safe skill id (one path component, under ADR-0014's rules) - - a digest isn't in FastSkill's current content digest format. The original digest didn't + - a digest isn't in FastSkill's current content digest format + ([ADR-0017](0017-versioned-content-digests.md)). The original digest didn't length-frame file contents, so two different trees could share it. It is never accepted here. - an artifact location or `report_url` isn't `https://` (a file source may use local paths) diff --git a/docs/adr/0017-versioned-content-digests.md b/docs/adr/0017-versioned-content-digests.md new file mode 100644 index 00000000..6f8239f1 --- /dev/null +++ b/docs/adr/0017-versioned-content-digests.md @@ -0,0 +1,109 @@ +# Content digests are versioned and frame every field + +Status: accepted. Date: 2026-09-24. Implemented: 2026-09-24. + +Related: [ADR-0007](0007-self-contained-tracked-skill-bundles.md), +[ADR-0008](0008-bundle-ownership-and-local-changes.md), +[ADR-0009](0009-resolution-and-restoration-policy.md), +[ADR-0015](0015-manifest-composition-and-bundle-exports.md), and +[ADR-0016](0016-machines-follow-a-signed-managed-state.md). + +## Context + +A **content digest** is the one string fastskill uses to say "these exact files". Locks pin it +as a skill's `checksum`, bundle records pin it per member and per release, bundle artifacts carry +it in their embedded `skills.lock`, and install, update, restore and remove compare it with the +directory on disk before they change anything. Whether a locked restore installs the reviewed +content, and whether a bundle update overwrites local edits, both rest on it. + +Until now the digest was SHA-256 over, for each file in name order, the relative path with its +length and then the file's bytes *without* their length. The resulting byte stream does not +decode to a single directory tree: the end of one file's contents and the start of the next +file's path cannot be told apart. Two different skill directories could therefore share a +digest, and a directory that differed from the one a Lock pinned could pass verification. + +The digest had no version marker either, so the algorithm could not be changed without making +old and new values indistinguishable. + +## Decision + +### The current form + +A content digest is written as `sha256-tree-v2:` followed by 64 lowercase hex characters: SHA-256 +over + +1. the domain tag `fastskill content digest v2`, and then +2. for every regular file, in byte order of its path: the path relative to the skill directory + and then the file's contents. + +Every field is preceded by its length as a big-endian `u64`, so the stream decodes to exactly one +tree. In addition: + +- A path is its UTF-8 components joined with `/`. It is built from components, not by rewriting + separators, so a Unix file name containing `\` stays distinct from a nested path. +- A file name that is not valid UTF-8 is refused: it has no spelling that means the same thing on + every platform. +- Symbolic links are refused, as before. +- A file whose length changes while it is hashed is an error, not a digest of torn content. + +The digest covers file paths and contents only. It does not cover empty directories, file modes +(including the executable bit) or timestamps; see Out of scope. + +### Compatibility with digests written by older releases + +A **legacy digest** is 64 bare lowercase hex characters. fastskill never writes one, but it still +accepts one that matches, so existing Locks and bundles keep installing: + +- A recorded digest is checked with the algorithm its form names. A legacy value is checked with + the old algorithm; any other value must equal the current-form digest. Unreadable content is an + error whatever the form. +- Accepting a legacy value prints one warning per process saying the record is weaker and will be + replaced. +- Every record fastskill writes uses the current form. When fastskill rewrites a record that held + a legacy value, the rewrite stores the current form. +- Two recorded digests of the same form that differ are a conflict, as before. A legacy and a + current value cannot be compared with each other, so each is checked against the content. + +`project install --lock` never changes the Lock, so it verifies legacy values and keeps them. Where +digests are stored, and when a legacy value is replaced: + +| Record | Written by | Legacy value replaced when | +|---|---|---| +| `skills.lock` `[[skills]].checksum` | `skill add`, `skill update`, `project install` for a newly resolved root | That entry is re-resolved. `project install --lock` verifies against the Lock and does not rewrite existing entries. | +| `global-skills.lock` `checksum` | `skill add --global`, `skill update --global` | That entry is reinstalled or updated. | +| `skills.lock` `[[bundles]]` release and member digests | `bundle add`, `bundle update`, `project install` | The bundle is added, updated or restored by `project install`. An unchanged `bundle add` leaves the entry alone. | +| `skills.lock` `[[overrides]].digest` | `bundle override --from` | The override is recorded again, or removing its last owning bundle turns it into an individual skill. | +| `.fastskill/bundle-history.toml` | every recorded bundle release | That release is recorded again. | +| `skills.lock` inside a bundle artifact, and its copy under `.fastskill/bundles/` | `bundle build` | Never: an artifact is immutable. Rebuild it with `bundle build`. | + +A bundle artifact is accepted when its members and its release digest are all current or all +legacy. An artifact that mixes forms was not produced by any release and is refused. The release +digest algorithm is unchanged, but it hashes the member digests, so a rebuilt artifact has a new +release digest; fastskill treats both as the same release, because both name the same contents. + +The bundle archive format (`fastskill-bundle-lock-v1`) and the Lock format version are not +changed. Changing either would make older releases refuse to read files they could otherwise +report on, with a message suggesting that the Lock be deleted. + +## Tradeoffs + +- **Legacy values are still trusted until they are rewritten.** A tree crafted to collide with a + legacy digest passes where that digest is still recorded. Refusing legacy digests would close + this, but every committed Lock and every bundle built by an earlier release would stop + installing, including `project install --lock` in CI, with no way to verify the content they + pinned. The warning makes the weaker record visible, and a rewrite removes it. A later release + can refuse legacy digests once Locks have had time to be rewritten. +- **Older releases cannot verify current-form digests.** A Lock or artifact written by this + release reports a checksum mismatch on an older fastskill. Every machine and CI job that shares + a project must be upgraded together, as with manifest `schema_version = "2"`. +- **Non-UTF-8 file names are no longer installable.** They were rare, and their digest depended + on how the platform converted them. + +## Out of scope + +- **The executable bit.** Windows cannot represent it, and a Lock written on one platform must + verify on every other. Including it would need a platform-independent source for the mode + (for example, the archive or git tree), which fastskill does not have for every Origin. +- **Empty directories.** Neither ZIP artifacts nor git preserve them consistently, so they cannot + be part of what a digest promises. +- **Removing legacy acceptance.** That is a later decision, with its own migration notice. diff --git a/docs/requirements/two-scope-lock-model.md b/docs/requirements/two-scope-lock-model.md index f8d4c2d4..81f4ecf5 100644 --- a/docs/requirements/two-scope-lock-model.md +++ b/docs/requirements/two-scope-lock-model.md @@ -60,7 +60,7 @@ id = "my-skill" name = "My Skill" version = "1.2.0" source = { type = "source", name = "default", skill = "my-skill", version = "1.2.0" } -checksum = "sha256:abc123..." +checksum = "sha256-tree-v2:abc123..." dependencies = ["dependency-skill"] groups = ["dev"] editable = false @@ -79,7 +79,8 @@ parent_skill = "parent-skill-id" # Optional, only for transitive deps - `name`: Human-readable skill name - `version`: Installed version - `source`: Where the skill was fetched from (registry, git, local path, etc.) -- `checksum`: SHA-256 hash of skill content (when available) +- `checksum`: content digest of the skill directory (when available); see + [ADR-0017](../adr/0017-versioned-content-digests.md) - `dependencies`: List of skill IDs this skill depends on - `groups`: Optional groups this skill belongs to (e.g., "dev", "prod") - `editable`: Whether the skill was installed in editable mode @@ -100,7 +101,7 @@ id = "my-global-skill" name = "My Global Skill" version = "2.0.0" source = { type = "git", url = "https://github.com/org/repo.git", branch = "main" } -checksum = "sha256:def456..." +checksum = "sha256-tree-v2:def456..." installed_at = "2026-04-28T10:00:00Z" last_checked_at = "2026-04-28T12:00:00Z" last_updated_at = "2026-04-28T10:00:00Z" @@ -335,8 +336,11 @@ Project lock entries are **always sorted alphabetically by skill ID** before wri ### Checksum Verification -When available, skills include a SHA-256 checksum in the lock file. This verifies integrity during -`project install --lock` operations. +When available, skills include a content digest in the lock file. This verifies integrity during +`project install --lock` operations. The digest is `sha256-tree-v2:` plus a SHA-256 over every +file's relative path and contents, each length-prefixed. A 64-character digest without the prefix +was written by an earlier release; it is still verified, with a warning, until the entry is +rewritten. See [ADR-0017](../adr/0017-versioned-content-digests.md). Not all source types provide checksums (e.g., editable local paths), so `checksum` is optional. diff --git a/webdocs/cli-reference/bundle-command.mdx b/webdocs/cli-reference/bundle-command.mdx index 0af1393a..93796587 100644 --- a/webdocs/cli-reference/bundle-command.mdx +++ b/webdocs/cli-reference/bundle-command.mdx @@ -37,6 +37,8 @@ fastskill bundle build --output dist The result is `dist/notes-team-1.0.0.zip`. Its embedded identity, version, membership, and digests are authoritative, so renaming the download does not change the release. +An artifact built by an earlier release, whose digests lack the `sha256-tree-v2:` prefix, still +installs with a warning; rebuild it to record current-form digests. There is no `[bundle.members]` list to maintain. A source Manifest using the earlier `[bundle]` format remains buildable for migration, while all new bundles should use `[metadata]`. diff --git a/webdocs/skill-management/manifest-system.mdx b/webdocs/skill-management/manifest-system.mdx index 2914862f..5ae74cf1 100644 --- a/webdocs/skill-management/manifest-system.mdx +++ b/webdocs/skill-management/manifest-system.mdx @@ -95,6 +95,12 @@ roots excluded from that resolution; `--lock` cannot resolve missing coverage. dependency edges, groups, covered roots, bundle ownership, and overrides. It is generated by FastSkill; do not hand-write a partial lockfile. +A content digest names the exact files of a skill directory and is written as +`sha256-tree-v2:`. A lockfile from an earlier release may hold 64-character digests +without that prefix; they are still verified, with a warning, and are replaced when fastskill +next rewrites that entry. fastskill releases before this format report a checksum mismatch on +the new form, so upgrade every machine and CI job that shares the lockfile. + ```bash fastskill project install fastskill project install --lock