Skip to content

Add OpenAI hosted skill package target#62

Merged
dd3ok merged 2 commits into
mainfrom
add-openai-hosted-package-target
May 27, 2026
Merged

Add OpenAI hosted skill package target#62
dd3ok merged 2 commits into
mainfrom
add-openai-hosted-package-target

Conversation

@dd3ok

@dd3ok dd3ok commented May 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • add scripts/package-skill.py --target raw-local|openai-hosted while preserving the existing raw-local default archive
  • generate OpenAI hosted archives under dist/openai/lucid.zip with a single lucid/ top-level folder
  • validate both archive shapes, default output paths, unknown targets, and hosted forbidden-path checks
  • document the raw local archive vs OpenAI hosted upload archive difference

Closes #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.py
  • python3 scripts/validate-skill.py
  • python3 scripts/validate-no-dangerous-io.py
  • python3 scripts/validate-evals.py
  • python3 scripts/validate-docs.py
  • python3 scripts/validate-package-skill.py
  • python3 lucid.py verify --root . --strict
  • git diff --check

Subagent review

  • local code review: PASS, no blockers
  • independent package shape verification: PASS, hosted zip has only lucid/ top-level and no root SKILL.md

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread scripts/validate-package-skill.py Outdated
Comment on lines +54 to +57
if package_root is not None:
prefix = f"{package_root}/"
if name.startswith(prefix):
relative_name = name[len(prefix) :]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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}")

Comment on lines +150 to +153
expect_failure(
"forbidden path under openai-hosted package root",
lambda: validate_member("lucid/docs/extra.md", package_root="lucid"),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a test case to verify that validate_member correctly rejects paths that are outside the specified package_root.

Suggested change
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"),
)

@dd3ok dd3ok merged commit 1438dc7 into main May 27, 2026
1 check passed
@dd3ok dd3ok deleted the add-openai-hosted-package-target branch May 27, 2026 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add OpenAI hosted skill package target

1 participant