Extract CI validation into reusable scripts#1
Conversation
Move inline shell from validate-pr.yml into scripts/validate.sh and scripts/audit.sh so CI and local dev run the same code. Add Makefile for convenient local testing (make validate / make audit / make ci).
Add scripts/** and the workflow file itself to the paths filter so CI runs when validation logic is modified, not only when skillshare-hub.json changes.
Summary of ChangesHello @runkids, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the CI validation process by extracting existing inline shell logic into reusable scripts. This change significantly improves the consistency between CI and local development environments, allowing developers to run the exact same validation and auditing checks locally. A new Makefile simplifies the execution of these checks, enhancing the overall developer experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
- Add scripts/format.sh: sort skills by name, normalize to jq 2-space - Add format check to scripts/validate.sh (fails if unformatted) - Add make format target - Apply formatting to skillshare-hub.json
- Generate markdown report with source, status, and risk score - Auto-post results as PR comment (replaces previous comment on re-run) - Add permissions for pull-requests: write - Fix grep -P incompatibility for macOS/Linux portability
runkids
force-pushed
the
extract-ci-scripts
branch
from
98df02e to
959a08a
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extracts CI validation logic into reusable shell scripts, validate.sh and audit.sh, and adds a Makefile for local execution. However, it introduces GitHub Actions Command Injection vulnerabilities because untrusted data from the skillshare-hub.json file is printed to the console without proper sanitization. This could allow attackers to spoof CI results or manipulate workflow execution. It is recommended to validate all fields in skillshare-hub.json against strict regular expressions before use. Additionally, while the scripts are well-structured, there are suggestions to improve error message formatting in validate.sh and to use mktemp in audit.sh for robustness.
|
|
||
| if [[ "$source" == http* ]]; then | ||
| clone_url="$source" | ||
| else |
There was a problem hiding this comment.
Using a predictable temporary directory name can lead to race conditions if multiple instances of this script run concurrently. It's safer to use mktemp -d to create a unique directory. For even more robust cleanup on script interruption, consider using a trap at the script level to remove the temporary directory on exit.
| else | |
| clone_dir=$(mktemp -d "/tmp/audit-${safe_name}.XXXXXX") |
| # Check required fields | ||
| missing=$(jq -r '.skills[] | select(.name == "" or .name == null or .description == "" or .description == null or .source == "" or .source == null) | .name // "unnamed"' "$HUB_FILE") | ||
| if [ -n "$missing" ]; then | ||
| echo "ERROR: Skills missing required fields: $missing" |
There was a problem hiding this comment.
The missing variable, which contains skill names from skillshare-hub.json, is printed directly to the console. This creates a GitHub Actions Command Injection vulnerability if a skill name contains a newline followed by a workflow command. It is crucial to validate the format of skill names before using them in any output. Additionally, for readability, if multiple skills are missing, they should be listed on separate lines.
| # Check no duplicate names | ||
| dupes=$(jq -r '[.skills[].name] | group_by(.) | map(select(length > 1)) | flatten | .[]' "$HUB_FILE") | ||
| if [ -n "$dupes" ]; then | ||
| echo "ERROR: Duplicate skill names: $dupes" |
There was a problem hiding this comment.
The dupes variable, containing skill names from skillshare-hub.json, is printed directly to the console. This poses a GitHub Actions Command Injection vulnerability if a duplicate skill name includes a newline followed by a workflow command. It is critical to validate skill name formats before output. Additionally, for better readability, duplicate names should be listed on separate lines.
| echo "No new or changed skill sources to audit" | ||
| [ -n "$REPORT_FILE" ] && echo "No new or changed skill sources to audit." > "$REPORT_FILE" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
The script prints the contents of the new_sources variable directly to the console. new_sources is derived from the source field in skillshare-hub.json, which is untrusted input from a pull request. If a malicious source contains a newline followed by a GitHub Actions workflow command (e.g., ::error::), the command will be executed by the GitHub Actions runner. This can be used to spoof CI results or manipulate the workflow.
| if [ -n "$bad_names" ]; then | ||
| echo "ERROR: Invalid skill names (must be lowercase, hyphens): $bad_names" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
If there are multiple invalid skill names, they will be displayed on a single line, making the list difficult to parse. Displaying each invalid name on a new line would be more readable.
| if [ -n "$bad_names" ]; then | |
| echo "ERROR: Invalid skill names (must be lowercase, hyphens): $bad_names" | |
| exit 1 | |
| fi | |
| if [ -n "$bad_names" ]; then | |
| echo "ERROR: Invalid skill names (must be lowercase, hyphens):" | |
| echo "$bad_names" | |
| exit 1 | |
| fi |
runkids
force-pushed
the
extract-ci-scripts
branch
from
4d7ee1b to
87caefe
Compare
- Switch skillshare install from gh release to curl install script - Run skillshare init after install to create global config - Treat "config not found" as audit failure instead of warning skip - Add job summary step to display audit results in workflow page
runkids
force-pushed
the
extract-ci-scripts
branch
from
87caefe to
2517bb3
Compare
hashFiles() only works within $GITHUB_WORKSPACE, so /tmp/audit-report.md was never detected. Use step output has_report flag to conditionally run comment and summary steps.
Add collapsible details section showing full skillshare audit output (findings, severity, risk score) for each source.
Block only when aggregate Risk >= HIGH, not when individual findings have high severity. LOW/MEDIUM risk skills now pass.
runkids
force-pushed
the
extract-ci-scripts
branch
from
78a0e62 to
ff4543d
Compare
Skill Audit Results
Audit Details
|
1 participant
Summary
validate-pr.ymlintoscripts/validate.shandscripts/audit.shMakefilefor convenient local testing.gitignoreto exclude.claude/Local usage
Test plan
make validatepasses locallymake auditpasses locally (no diff against main)make ciruns both targets successfully