Harden the inspector, broaden tests, and reconcile docs (self-audit fixes)#2
Merged
Conversation
The inspector walked .git (and node_modules, .venv, __pycache__, .pytest_cache), so auditing a git working tree counted git internals toward directory limits, inflated file/size counts, and exposed .git/config to the secret scan. Skip these directories during traversal and report which were skipped via a new excluded_directories output field. Separately, the backtick branch of RESOURCE_REF_PATTERN captured everything up to the closing backtick, so a documented command like `scripts/tool.py --json` was extracted as a nonexistent path, producing a false missing_resource_reference (and marking the real script orphaned). Stop the capture at whitespace and keep only the first token defensively. Add regression tests covering a git working tree with a planted secret and 1200 loose objects, and a SKILL.md that documents its helper script with command flags. Self-tests: 27/27. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
Three frontmatter-robustness fixes so the inspector stops rejecting valid skills: - Decode text with utf-8-sig, so a SKILL.md with a leading UTF-8 BOM (common from Windows editors) parses instead of failing frontmatter_missing_or_invalid. - Recognize license, allowed-tools, metadata, and version as optional platform keys (info-level) instead of error-level unexpected keys; genuinely unknown keys still error. - Preserve '#'-prefixed and blank lines as literal content inside YAML block scalars (| and >), so a multiline description is not silently truncated and description_length is accurate. Add regression tests for a BOM SKILL.md, BOM+CRLF, optional platform keys, an unknown-key guard, and a block scalar with a '#' line and a blank line. Self-tests: 32/32. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
- Flat zips (SKILL.md at the archive root, no wrapping folder) now produce a warning-level zip_missing_top_level_skill_folder whose expected value is the skill name, instead of an error whose expected value was the random temp extraction dir name. - Gate the on-disk archive size before opening the zip. zipfile parses the entire central directory into memory at open() time, so a crafted archive with very many members could drive memory up before any per-member limit fired; the size cap now bounds that. package_zip_too_large moves entirely to this pre-open gate so folder and zip size checks never double-report. - Detect members that collide only by case (File.txt vs file.txt), which silently overwrite on case-insensitive filesystems, via zip_case_collision_member. - Add a heuristic, warning-level script_dangerous_command scan over bundled shell scripts (curl|bash installers, rm -rf of root/home, fork bombs, dd to /dev, mkfs). Shell-script-only to avoid false positives on docs and source. Add regression tests for all four. Self-tests: 36/36. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
- iter_findings now dedupes by object identity, so an OpenAI-metadata finding exposed under both platform_metadata_findings and its backward-compatible agent_metadata_findings alias (the same list object) is counted once in summary.error_count/warning_count/finding_count instead of twice. - inspector-output-schema.md now documents every emitted finding code: the previously-missing zip_read_error, directory_root_lstat_failed, directory_root_resolve_failed, directory_file_stat_failed, frontmatter_unavailable, package_zip_too_large, and package_folder_large, plus the codes added in this branch (zip_missing_top_level_skill_folder, zip_case_collision_member, script_dangerous_command). Also documents the excluded_directories, dangerous_command_findings, and dangerous_command_note output fields, the expected/actual/keys evidence fields, and the expanded optional-keys set. Emitted codes and the schema doc are now in exact agreement in both directions. Add a regression test asserting the aliased finding is counted once. Self-tests: 37/37. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
Grow the regression suite from 37 to 58 cases and strengthen the harness: - Assert finding severity (not just presence) via a new expected_severity, so an error->warning downgrade is caught even when the strict exit code is unchanged. Backfilled on the single-code cases. - Add a SkipCase path: the folder-symlink fixtures now SKIP (not abort the suite) on platforms where os.symlink is unprivileged. - Assert stderr content for the invalid-limit case so it cannot silently pass for the wrong reason. - Exercise the markdown renderer on a findings-rich fixture (not just the zero-finding footer). New coverage for previously-untested codes: openai_metadata_missing_interface / missing_display_name / unreadable, missing_resource_reference, the .env.* suspicious-filename variant, all remaining secret-content rules (private key, github, slack, google service account, jwt, api-key, password), bounded-read truncation, package_folder_large, corrupt zip, and several frontmatter/size limit codes. 48 of 63 emittable codes are now exercised; the rest require injected filesystem errors or are emission-covered. Fake secrets in fixtures are constructed by concatenation so this test file's own source contains no literal secret pattern (verified: skill-forge strict- passes its own inspector). Self-tests: 58/58. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
- The release workflow now runs the regression suite and strict-inspects the built skill-forge.zip before publishing, and refuses to release if strict inspection fails or a bundled shell script trips script_dangerous_command. Previously it only grepped the archive listing for two filenames. - The self-tests workflow runs on a Python version matrix (3.9 and latest stable) so a regression on the minimum supported interpreter is caught, not just on the newest. All scripts are verified to parse under 3.9. - Document the Python 3.9+ floor in README and use python3 in its inspector example. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
- Rewrite the frontmatter description (198 chars) to sentence-case, add the "Review"/"QA" trigger verbs and an explicit "diagnose triggering" hook for the skill's flagship capability, and capitalize the proper nouns. Passes the skill's own frontmatter heuristics. - Make the official-validator step decidable: name concrete candidates and a discovery procedure, and state the fallback when none is found, instead of an unlocatable "if available" condition tied to a Critical severity. - Use python3 in all command examples with a python fallback note; document the Python 3.9+ floor. - Add a non-negotiable safety-floor standard so secret/unsafe findings are always reported even when the user narrows scope. - Give the repair path a self-contained fallback (edit, re-inspect --strict, repackage) instead of pointing at an undefined external workflow. - Add a positive containment procedure for running bundled scripts, and wrong-input-type redirect guidance for non-skill inputs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
- Rewrite example-report.md to follow report-template.md's exact 11-section structure and its 6-column, 11-category pressure-test table (was 10 sections with a 3-column, 5-row table), so the executive-report path and the mandated template no longer disagree. - Add the reserved-name and 64-character name rules to platform-compatibility.md, which SKILL.md step 4 directs reviewers to consult. - Add a release gate for destructive/network-piping commands in bundled scripts (script_dangerous_command), and note the overlap between the release-gate and audit checklists so shared items are verified once. - De-duplicate policy prose: SKILL.md relays the inspector's secret_scan_note instead of restating it, and defers the 11/10 criteria to the rubric that owns them. - Disambiguate the near-duplicate audit-checklist bullets by moving the inspector-output check into Validation Evidence. - Use python3 in the README and schema-doc examples; add release-skill.yml and .gitattributes to the README tree and describe the release workflow. - Remove stale export-ignore entries (tests/, README.dev.md) and fix heading-case and double-blank-line formatting nits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1WmZkinHbJxST6dn5xeos
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
2 participants
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
Self-audited skill-forge with its own methodology, then fixed every finding. This addresses 2 High, 14 Medium, 12 Low, and 4 Nit issues across the inspector, self-tests, CI,
SKILL.md, and the reference docs. Each inspector change ships with a regression test.The headline problem was that the inspector failed good skills: it counted
.gitinternals toward strict limits, treated a documented command like`scripts/tool.py --json`as a missing resource, and rejected valid frontmatter (BOMs, optional keys). Those false-positives are gone, release CI now enforces the gate, and the self-test suite grew from 25 to 58 cases.What changed, by commit
.git/node_modules/etc. (addsexcluded_directories); stop backtick refs at whitespace so documented commands aren't falsemissing_resource_reference. (H1, H2)utf-8-sig; recognizelicense/allowed-tools/metadata/version; keep#/blank lines inside|/>scalars. (M2, M4, L1)script_dangerous_commandfor shell installers. (M3, M9, L3, M13)iter_findingsdedupes by identity so aliased findings count once; schema doc now documents every emitted code (verified bidirectionally). (L2, L12)3.9/latest matrix. (M10, L10)python3examples; non-negotiable safety-floor rule; self-contained repair/containment/redirect guidance. (M1, M5, M6, M12, L4, L5, L8, N3)example-report.mdto match the template; add reserved-name/64-char rules; script-safety gate; de-dup policy prose; README tree + release-workflow docs; remove stale.gitattributesentries; formatting nits. (M7, M8, M11, L6, L7, L13, N1, N2)scripts/generate_release_notes.pywired into the release workflow (export-ignored from the shipped zip).Verification
.gitexcluded (file_count84 → 20)🤖 Generated with Claude Code
Generated by Claude Code