Add remote skill scope/product_surface/enabled params and cleanup#11801
Add remote skill scope/product_surface/enabled params and cleanup#11801
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65f275f2e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
codex-rs/core/src/skills/remote.rs
Outdated
| let output_dir = config.codex_home.join("skills"); | ||
| tokio::fs::create_dir_all(&output_dir) | ||
| .await | ||
| .context("Failed to create downloaded skills directory")?; | ||
|
|
||
| let allowed_files = hazelnut.files.keys().cloned().collect::<HashSet<String>>(); | ||
| let zip_bytes = body.to_vec(); | ||
| let output_dir_clone = output_dir.clone(); | ||
| let prefix_candidates = vec![hazelnut.name.clone(), hazelnut.id.clone()]; | ||
| let prefix_candidates = vec![hazelnut_id.to_string()]; | ||
| tokio::task::spawn_blocking(move || { | ||
| extract_zip_to_dir( | ||
| zip_bytes, | ||
| &output_dir_clone, | ||
| &allowed_files, | ||
| &prefix_candidates, | ||
| ) | ||
| extract_zip_to_dir(zip_bytes, &output_dir_clone, &prefix_candidates) |
There was a problem hiding this comment.
Extract remote skills into an isolated target directory
export_remote_skill now writes directly into codex_home/skills and then strips the hazelnut_id prefix during normalization. If the archive is rooted by that id (common for exports), files are flattened into the shared skills root, so one export can overwrite files from previous skills and the returned path is no longer skill-specific.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
that's the plan. remote skill should already packed in to a folder.
| let file_path = safe_join(output_dir, &normalized)?; | ||
| if let Some(parent) = file_path.parent() { | ||
| std::fs::create_dir_all(parent) |
There was a problem hiding this comment.
Restrict extracted files to expected archive entries
The extraction loop now writes every normalized zip entry without validating it against an expected file list. Because extraction target is the shared skills tree, a crafted remote archive can write arbitrary filenames under that tree (e.g. other skill dirs or hidden dirs), causing integrity/security issues.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
it is ok for now.
65f275f to
e19a514
Compare
skills/remote/list: params=hazelnutScope, productSurface, enabled; returns=data: { id, name, description }[]
skills/remote/export: params=hazelnutId; returns={ id, path }