Add OpenAI hosted skill package target#62
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for packaging and validating OpenAI-hosted skill archives, wrapping skill files under a single top-level directory. The feedback highlights a potential validation bypass where files outside the specified package root could go undetected, and suggests enforcing strict package root boundaries along with corresponding test coverage.
| if package_root is not None: | ||
| prefix = f"{package_root}/" | ||
| if name.startswith(prefix): | ||
| relative_name = name[len(prefix) :] |
There was a problem hiding this comment.
When package_root is specified, validate_member should strictly enforce that the member path is actually within that package root (or is the package root itself). Currently, if a path is completely outside the package root (e.g., outside/file.md), it bypasses the prefix removal and is validated as-is, which could allow files outside the package root to silently pass validation if they don't match any forbidden prefixes.
| if package_root is not None: | |
| prefix = f"{package_root}/" | |
| if name.startswith(prefix): | |
| relative_name = name[len(prefix) :] | |
| if package_root is not None: | |
| prefix = f"{package_root}/" | |
| if name.startswith(prefix): | |
| relative_name = name[len(prefix) :] | |
| elif name != package_root: | |
| fail(f"archive contains path outside package root: {name}") |
| expect_failure( | ||
| "forbidden path under openai-hosted package root", | ||
| lambda: validate_member("lucid/docs/extra.md", package_root="lucid"), | ||
| ) |
There was a problem hiding this comment.
Add a test case to verify that validate_member correctly rejects paths that are outside the specified package_root.
| expect_failure( | |
| "forbidden path under openai-hosted package root", | |
| lambda: validate_member("lucid/docs/extra.md", package_root="lucid"), | |
| ) | |
| expect_failure( | |
| "forbidden path under openai-hosted package root", | |
| lambda: validate_member("lucid/docs/extra.md", package_root="lucid"), | |
| ) | |
| expect_failure( | |
| "path outside openai-hosted package root", | |
| lambda: validate_member("outside/file.md", package_root="lucid"), | |
| ) |
Summary
scripts/package-skill.py --target raw-local|openai-hostedwhile preserving the existing raw-local default archivedist/openai/lucid.zipwith a singlelucid/top-level folderCloses #61
Verification
python3 -m py_compile lucid.py skills/lucid/scripts/lucid.py scripts/validate-skill.py scripts/validate-evals.py scripts/validate-no-dangerous-io.py scripts/validate-docs.py scripts/validate-package-skill.py scripts/package-skill.pypython3 scripts/validate-skill.pypython3 scripts/validate-no-dangerous-io.pypython3 scripts/validate-evals.pypython3 scripts/validate-docs.pypython3 scripts/validate-package-skill.pypython3 lucid.py verify --root . --strictgit diff --checkSubagent review
lucid/top-level and no rootSKILL.md