Skip to content

Commit eecfb34

Browse files
fix: match path parts with fullmatch, and describe the allowlist accurately
A folder name ending in a newline matched the path pattern, because $ also matches before a trailing newline. The docs no longer claim that reading the extension allowlist from the base branch stops a pull request from widening it: the pull request runs its own copy of the check either way.
1 parent 5e7b634 commit eecfb34

4 files changed

Lines changed: 8 additions & 4 deletions

File tree

‎.claude/CLAUDE.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ The `metadata` block with `author` and `version` fields is required. Initial ver
8484
- Every skill is checked on every PR, because skills on `main` are published as one set. Errors from any skill fail; warnings print only for skills the PR touches.
8585
- Skills are read from Git objects (`git ls-tree -r -z` plus `git cat-file --batch`), never the working tree, so modes are exact and a local run checks only committed changes.
8686
- Rule data lives in `.github/scripts/skill-rules/` and the check fails closed (exit 2) when a file there is malformed: `agent-types.json`, `vocabulary.json` (dimension values; unknown values only warn), `published-files.json` (root names left out of the published set) and `conformance-cases.json`.
87-
- The extension allowlist is not duplicated: it is parsed from the `!*.<ext>` lines of `skills/.gitignore`, read from the merge base so a PR can't widen it for itself.
87+
- The extension allowlist is not duplicated: it is parsed from the `!*.<ext>` lines of `skills/.gitignore`, read from the merge base, so a new extension takes effect only once its `.gitignore` change has merged.
88+
- Like the repository's other checks, a PR runs its own copy of the script, rules and tests, so it could edit them to pass. Maintainer review of `.github/` changes (CODEOWNERS) and the post-merge run on `main` are the controls; don't describe any in-script measure as stopping a determined PR.
8889
- Secrets in test fixtures are built at runtime (`"AKIA" + ...`) or stored as base64 in the conformance cases, so no file in the repository contains a literal key or invisible character.
8990
- `conformance-cases.json` is a language-neutral contract: every tool that validates or publishes these skills must give each case's `expect` result. The script runs it before checking any skill and exits 2 on a mismatch. A rule change needs a matching case change in the same PR.
9091
- Exit codes: 0 pass (warnings allowed), 1 a rule broken, 2 could not run. Never report a 2 as the PR's fault.

‎.github/scripts/skill_checks.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
ZIP_LIMIT_BYTES = 1_048_576 - 65_536
6161
ZIP_WARNING_RATIO = 0.9
6262
MAX_PATH_LENGTH = 512
63-
SEGMENT_PATTERN = re.compile(r"^[A-Za-z0-9._-]+$")
63+
SEGMENT_PATTERN = re.compile(r"[A-Za-z0-9._-]+")
6464
WINDOWS_RESERVED_NAMES = frozenset(
6565
{"CON", "PRN", "AUX", "NUL"} | {f"COM{i}" for i in range(1, 10)} | {f"LPT{i}" for i in range(1, 10)}
6666
)
@@ -664,7 +664,7 @@ def _path_problem(path: str) -> str | None:
664664
return f'"{path}" has an empty, "." or ".." part'
665665
if segment.startswith("."):
666666
return f'"{path}" is a hidden file or folder ("{segment}"), which would be published; remove it'
667-
if not SEGMENT_PATTERN.match(segment):
667+
if not SEGMENT_PATTERN.fullmatch(segment):
668668
return f'"{path}" may use only letters, digits, ".", "_" and "-" in each part of its path'
669669
if segment.endswith("."):
670670
return f'"{path}" has a part that ends with a period, which Windows can\'t extract'

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ The frontmatter must be plain YAML that every parser reads the same way. Anchors
6767
Published files. Everything in the skill folder is published and installed into customers' Agent Spaces except `evals/`, `.skilleval.yaml`, `.skilleval.yml` and `CHANGELOG.md` at the skill root ([`published-files.json`](.github/scripts/skill-rules/published-files.json)). For the published files:
6868

6969
- At most 100 files, and their zip at most 983,040 bytes (1 MiB less a margin, because zip sizes vary by tool). The check warns past 90%.
70-
- Each file's extension must be one that `skills/.gitignore` allows. The check reads that list from the base branch, so a pull request can't widen it for its own files.
70+
- Each file's extension must be one that `skills/.gitignore` allows. The check reads that list from the base branch, so a new extension takes effect only after the `skills/.gitignore` change that adds it has merged.
7171
- Paths use only letters, digits, `.`, `_` and `-`, are at most 512 characters, have no hidden parts (such as `.DS_Store`), no Windows-reserved names (such as `CON`) and no part ending in a period, and don't differ from another path only in case.
7272
- No file starts with `#!`.
7373

‎tests/test_skill_checks.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,9 @@ def test_path_rules(self):
370370
with self.subTest(path=path):
371371
self.assertOneError(with_files(path), fragment)
372372

373+
def test_folder_name_with_trailing_newline_is_rejected(self):
374+
self.assertOneError(with_files("references\n/a.md"), "letters, digits")
375+
373376
def test_case_collision(self):
374377
self.assertOneError(with_files("references/Guide.md", "references/guide.md"), "differ only in case")
375378

0 commit comments

Comments
 (0)