ci: add marketplace + skill structure validation workflow#8
Merged
Conversation
Adds a GitHub-hosted (ubuntu-latest, public-repo-safe) check that runs a stdlib-only Python validator on every PR/push to main. It guards the exact failure classes this repo has hit: - marketplace.json is valid JSON with the expected shape - every plugin `source` dir exists; every `skills[]` entry resolves to a dir with a SKILL.md (catches a renamed source path with a stale entry) - every SKILL.md has frontmatter with name + description - every committed .claude/skills/* symlink resolves (catches dev symlinks left dangling after a plugin rename — cf. the aws-cdk -> aws-iac rename) - no orphan skills unregistered in any plugin The workflow uses no untrusted input and runs with contents:read only. Once green on main, the `validate-marketplace` check is set as a required status check on the branch protection rule.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR adds a valuable CI validation workflow for marketplace structure. The implementation is well-structured and follows security best practices for GitHub Actions. However, 4 critical issues must be addressed before merge:
Critical Issues
- Missing error handling in
frontmatter()- file read operations can crash the script - Missing directory existence check in
check_all_skill_frontmatter()- will crash ifplugins/doesn't exist - Path traversal vulnerability in
check_dev_symlinks()- malicious symlinks can expose filesystem paths outside the repository (CWE-22) - KeyError risk in
main()- unsafe dictionary access can crash when JSON is malformed
All issues have commit-able fix suggestions. Once resolved, this workflow will provide robust validation for the repository structure.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Address Amazon Q review findings on the validator: - frontmatter(): wrap read_text in try/except OSError, report + treat as empty rather than crashing on an unreadable SKILL.md - check_all_skill_frontmatter(): guard against a missing plugins/ dir before rglob() instead of raising FileNotFoundError - check_dev_symlinks(): flag symlinks that resolve outside the repo (supply-chain hygiene) before the existence check - main(): use isinstance(plugins, list) for the summary count so a malformed (non-list) plugins value can't raise
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a lightweight CI check (GitHub-hosted
ubuntu-latest, stdlib-only Python — no deps, nopackage.jsonneeded) that validates the marketplace + skill structure on every PR/push tomain. Once green, thevalidate-marketplacecheck will be added as a required status check on themainbranch protection rule.What it validates
Each is a hard failure — chosen to catch the failure classes this repo has actually hit:
marketplace.jsonis valid JSON with the expected shape; no duplicate plugin namessourcedir existsskills[]entry resolves to a dir containingSKILL.md(catches a renamed source path that forgot a skill entry — cf. theaws-cdk→aws-iacrename)SKILL.mdhas frontmatter with non-emptyname+description.claude/skills/*symlink resolves to an existing dir (catches dev symlinks left dangling after a rename — the exact issue fixed in fix(.claude): repoint dev skill symlinks after aws-iac rename #7)plugins/*/skills/*/SKILL.mdis registered in some plugin)Security
No untrusted input is used anywhere in the workflow; the only
run:step executes a fixed script with nogithub.event.*interpolation. Runs withpermissions: contents: read. Public-repo-safe (ubuntu-latest, no self-hosted runner).Test Plan
main(5 plugins, 6 skills).claude/skills/*symlink (verified locally)skills[]entry pointing at a missing dir (verified locally)validate-marketplacecheck runs green on this PR