diff --git a/code-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py b/code-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py index 6b5f71adfbd..727416a160b 100644 --- a/code-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py +++ b/code-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py @@ -8,6 +8,24 @@ import yaml +MAX_STRUCTURED_ITEMS_PER_SKILL = 64 +MAX_STRUCTURED_ID_LEN = 96 +MAX_STRUCTURED_PURPOSE_LEN = 256 +MAX_STRUCTURED_VALUE_LEN = 512 +MAX_COMMAND_ARGV_TOKENS = 32 +MAX_COMMAND_TOKEN_LEN = 160 +ALLOWED_TOP_LEVEL_KEYS = { + "name", + "description", + "metadata", + "policy", + "resources", + "commands", + "workflow_defaults", +} +ALLOWED_RESOURCE_KINDS = {"script", "reference", "template", "asset"} +ALLOWED_COMMAND_SOURCES = {"skill", "repo", "external"} + def validate_skill(skill_path): """Basic validation of a skill""" @@ -37,6 +55,10 @@ def validate_skill(skill_path): if "description" not in frontmatter: return False, "Missing required field: description" + unexpected_keys = set(frontmatter) - ALLOWED_TOP_LEVEL_KEYS + if unexpected_keys: + return False, f"Unexpected frontmatter key(s): {', '.join(sorted(unexpected_keys))}" + name = frontmatter.get("name", "") if not isinstance(name, str) or not name.strip(): return False, "Name must be a non-empty string" @@ -61,9 +83,189 @@ def validate_skill(skill_path): f"Description is too long ({len(description)} characters). Maximum is 1024 characters.", ) + structured_error = validate_structured_metadata(frontmatter, skill_path) + if structured_error: + return False, structured_error + return True, "Skill validation passed" +def validate_structured_metadata(frontmatter, skill_path): + resource_paths, resource_error = validate_resources(frontmatter.get("resources"), skill_path) + if resource_error: + return resource_error + + command_error = validate_commands(frontmatter.get("commands"), resource_paths, skill_path) + if command_error: + return command_error + + return validate_workflow_defaults(frontmatter.get("workflow_defaults")) + + +def validate_resources(resources, skill_path): + if resources is None: + return set(), None + if not isinstance(resources, list): + return set(), "resources must be a list" + if len(resources) > MAX_STRUCTURED_ITEMS_PER_SKILL: + return set(), f"resources must contain at most {MAX_STRUCTURED_ITEMS_PER_SKILL} entries" + + paths = set() + for index, resource in enumerate(resources): + field = f"resources[{index}]" + if not isinstance(resource, dict): + return paths, f"{field} must be a YAML dictionary" + unexpected = set(resource) - {"path", "kind", "description"} + if unexpected: + return paths, f"Unexpected key(s) in {field}: {', '.join(sorted(unexpected))}" + + raw_path = resource.get("path") + path_error = validate_relative_path(raw_path, f"{field}.path") + if path_error: + return paths, path_error + assert isinstance(raw_path, str) + raw_path = raw_path.strip() + if raw_path in paths: + return paths, f"{field}.path duplicates {raw_path}" + if not (skill_path / raw_path).exists(): + return paths, f"{field}.path points to missing {raw_path}" + paths.add(raw_path) + + if resource.get("kind") not in ALLOWED_RESOURCE_KINDS: + allowed = ", ".join(sorted(ALLOWED_RESOURCE_KINDS)) + return paths, f"{field}.kind must be one of: {allowed}" + desc_error = validate_optional_text( + resource.get("description"), + MAX_STRUCTURED_PURPOSE_LEN, + f"{field}.description", + ) + if desc_error: + return paths, desc_error + return paths, None + + +def validate_commands(commands, resource_paths, skill_path): + if commands is None: + return None + if not isinstance(commands, list): + return "commands must be a list" + if len(commands) > MAX_STRUCTURED_ITEMS_PER_SKILL: + return f"commands must contain at most {MAX_STRUCTURED_ITEMS_PER_SKILL} entries" + + names = set() + for index, command in enumerate(commands): + field = f"commands[{index}]" + if not isinstance(command, dict): + return f"{field} must be a YAML dictionary" + unexpected = set(command) - {"name", "source", "resource_path", "example_argv", "purpose"} + if unexpected: + return f"Unexpected key(s) in {field}: {', '.join(sorted(unexpected))}" + + name = command.get("name") + text_error = validate_required_text(name, MAX_STRUCTURED_ID_LEN, f"{field}.name") + if text_error: + return text_error + assert isinstance(name, str) + name = name.strip() + if not all(c.islower() or c.isdigit() or c == "-" for c in name): + return f"{field}.name must be lowercase with only letters, digits, and hyphens" + if name in names: + return f"{field}.name duplicates {name}" + names.add(name) + + source = command.get("source", "skill") + if source not in ALLOWED_COMMAND_SOURCES: + allowed = ", ".join(sorted(ALLOWED_COMMAND_SOURCES)) + return f"{field}.source must be one of: {allowed}" + + resource_path = command.get("resource_path") + if source == "skill": + path_error = validate_relative_path(resource_path, f"{field}.resource_path") + if path_error: + return path_error + assert isinstance(resource_path, str) + resource_path = resource_path.strip() + if resource_path not in resource_paths: + return f"{field}.resource_path must be listed in resources" + if not (skill_path / resource_path).is_file(): + return f"{field}.resource_path points to missing file {resource_path}" + elif resource_path is not None: + return f"{field}.resource_path is only allowed for source: skill" + + argv_error = validate_example_argv(command.get("example_argv"), f"{field}.example_argv") + if argv_error: + return argv_error + purpose_error = validate_required_text( + command.get("purpose"), + MAX_STRUCTURED_PURPOSE_LEN, + f"{field}.purpose", + ) + if purpose_error: + return purpose_error + return None + + +def validate_workflow_defaults(defaults): + if defaults is None: + return None + if not isinstance(defaults, list): + return "workflow_defaults must be a list" + if len(defaults) > MAX_STRUCTURED_ITEMS_PER_SKILL: + return f"workflow_defaults must contain at most {MAX_STRUCTURED_ITEMS_PER_SKILL} entries" + for index, default in enumerate(defaults): + field = f"workflow_defaults[{index}]" + if not isinstance(default, dict): + return f"{field} must be a YAML dictionary" + unexpected = set(default) - {"name", "value", "description"} + if unexpected: + return f"Unexpected key(s) in {field}: {', '.join(sorted(unexpected))}" + name_error = validate_required_text(default.get("name"), MAX_STRUCTURED_ID_LEN, f"{field}.name") + if name_error: + return name_error + value_error = validate_required_text(default.get("value"), MAX_STRUCTURED_VALUE_LEN, f"{field}.value") + if value_error: + return value_error + desc_error = validate_optional_text(default.get("description"), MAX_STRUCTURED_PURPOSE_LEN, f"{field}.description") + if desc_error: + return desc_error + return None + + +def validate_required_text(value, max_length, field): + if not isinstance(value, str) or not value.strip(): + return f"{field} must be a non-empty string" + if len(value.strip()) > max_length: + return f"{field} is too long ({len(value.strip())} characters). Maximum is {max_length} characters." + return None + + +def validate_optional_text(value, max_length, field): + if value is None: + return None + return validate_required_text(value, max_length, field) + + +def validate_example_argv(argv, field): + if not isinstance(argv, list) or not argv: + return f"{field} must be a non-empty list" + if len(argv) > MAX_COMMAND_ARGV_TOKENS: + return f"{field} must contain at most {MAX_COMMAND_ARGV_TOKENS} tokens" + for index, token in enumerate(argv): + token_error = validate_required_text(token, MAX_COMMAND_TOKEN_LEN, f"{field}[{index}]") + if token_error: + return token_error + return None + + +def validate_relative_path(value, field): + if not isinstance(value, str) or not value.strip(): + return f"{field} must be a non-empty string" + path = Path(value.strip()) + if path.is_absolute() or ".." in path.parts: + return f"{field} must be relative and must not contain '..'" + return None + + def main(): if len(sys.argv) < 2: print("Usage: python quick_validate.py ") diff --git a/code-rs/core/src/skills/loader.rs b/code-rs/core/src/skills/loader.rs index 792d1309ee4..5a3b48996cd 100644 --- a/code-rs/core/src/skills/loader.rs +++ b/code-rs/core/src/skills/loader.rs @@ -497,7 +497,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result, skill_dir: &Path, - scope: SkillScope, declared_resources: &[SkillResource], ) -> Result, SkillParseError> { if commands.len() > MAX_STRUCTURED_ITEMS_PER_SKILL { @@ -588,7 +587,6 @@ fn parse_skill_commands( let resource_path = parse_command_resource_path( &name, source, - scope, command.resource_path, skill_dir, declared_resources, @@ -614,7 +612,6 @@ fn parse_skill_commands( fn parse_command_resource_path( name: &str, source: SkillCommandSource, - scope: SkillScope, resource_path: Option, skill_dir: &Path, declared_resources: &[SkillResource], @@ -652,12 +649,6 @@ fn parse_command_resource_path( Ok(Some(resolved_resource_path)) } SkillCommandSource::Repo => { - if scope != SkillScope::Repo { - return Err(SkillParseError::InvalidField { - field: "commands.source", - reason: format!("command '{name}' uses source repo outside a repo skill"), - }); - } if resource_path.is_some() { return Err(SkillParseError::InvalidField { field: "commands.resource_path", @@ -1254,7 +1245,7 @@ workflow_defaults: } #[test] - fn repo_commands_require_repo_scope() { + fn repo_commands_can_be_declared_by_user_skills() { let skills_root = tempfile::tempdir().expect("tempdir"); let skill_dir = skills_root.path().join("bad"); fs::create_dir_all(&skill_dir).expect("create skill dir"); @@ -1280,15 +1271,15 @@ commands: scope: SkillScope::User, }]); - assert!(outcome.skills.is_empty()); - assert_eq!(outcome.errors.len(), 1); assert!( - outcome.errors[0] - .message - .contains("uses source repo outside a repo skill"), - "unexpected error: {}", - outcome.errors[0].message + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors ); + let skill = outcome.skills.first().expect("skill"); + assert_eq!(skill.commands.len(), 1); + assert_eq!(skill.commands[0].source, SkillCommandSource::Repo); + assert_eq!(skill.commands[0].resource_path, None); } #[test]