feat(skills): add opt-in AST diagnostics#30918
Closed
Tranquil-Flow wants to merge 1 commit into
Closed
Conversation
Add opt-in AST diagnostics for skill review without making Skills Guard stricter by default. - Add hermes skills inspect --ast-deep to scan fetched skill bundles before installation - Add hermes skills audit --deep to scan already-installed hub skills - Keep AST analysis in tools/skills_ast_audit.py, separate from tools/skills_guard.py - Label output as diagnostic hints, not security verdicts - Cover dynamic import/access patterns: importlib, __import__(computed), getattr(computed), and __dict__[computed] This follows the maintainer guidance from closed PR NousResearch#7436: useful AST-level analysis belongs in an opt-in diagnostic path, not in Skills Guard's default heuristic scan.
19 tasks
1 task
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
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
This follows up on the maintainer guidance from closed PR #7436 by moving the AST-based skill checks out of Skills Guard and into an explicitly opt-in diagnostic path.
hermes skills inspect <source> --ast-deepfor pre-install bundle reviewhermes skills audit [name] --deepfor already-installed hub skillstools/skills_ast_audit.pytools/skills_guard.pyuntouched; this does not change install/scan verdictsRationale
PR #7436 was closed because the AST logic was wired into Skills Guard as an automatic security gate. This version preserves the useful review signal while matching the suggested opt-in/diagnostic shape:
hermes skills inspect --ast-deepor similar.Test plan
python -m pytest tests/tools/test_skills_ast_audit.py tests/tools/test_skills_guard.py -q --timeout=30python -m ruff check tools/skills_ast_audit.py tests/tools/test_skills_ast_audit.py hermes_cli/skills_hub.py hermes_cli/main.py hermes_cli/commands.pypython -m hermes_cli.main skills inspect --helppython -m hermes_cli.main skills audit --helpNotes
tools/skills_guard.pyhas no diff in this PR.