Skip to content

Commit 26e9166

Browse files
authored
fix(mofa-slides): codex P3 follow-ups from #62 (#63)
Three MINOR issues codex flagged on the #62 review: 1. plugin_list_styles `skill` arg had no path-traversal guard. An input like `slides/../_unpublished/mofa-video` escaped the intended catalog and could list TOML files under `_unpublished/`. Now restricted to a single ASCII slug (letters, digits, '-', '_'); leading dots and any path separator are rejected. 2. `mofa_list_styles` output (both the JSON summary and the human text) leaked absolute `styles_dir` paths into the LLM context. Same for the missing-style bail in `plugin_slides`. Both stripped — the absolute path is operator detail, not LLM context. 3. SKILL.md "Output Paths" said the workspace contract picks up the deck via the plugin's `files_to_send`, but mofa_slides does not emit a files_to_send envelope — the host derives it from the `out` arg. Reworded to match actual behavior.
1 parent a322c37 commit 26e9166

2 files changed

Lines changed: 22 additions & 15 deletions

File tree

mofa-cli/src/main.rs

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -546,14 +546,13 @@ fn plugin_slides(
546546
} else {
547547
let available = style::list_style_names(&builtin_dir);
548548
let list = if available.is_empty() {
549-
format!("(none found in {})", builtin_dir.display())
549+
"(no styles installed on this deployment)".to_string()
550550
} else {
551551
available.join(", ")
552552
};
553553
eyre::bail!(
554-
"style '{style_name}' not found under {}. Available: {list}. \
555-
Call the `mofa_list_styles` tool to inspect variants and descriptions.",
556-
builtin_dir.display()
554+
"style '{style_name}' not found. Available: {list}. \
555+
Call the `mofa_list_styles` tool to inspect variants and descriptions."
557556
);
558557
};
559558
let loaded_style = style::load_style(&style_file)?;
@@ -929,6 +928,22 @@ fn plugin_list_styles(
929928
.get("skill")
930929
.and_then(|v| v.as_str())
931930
.unwrap_or("slides");
931+
// Reject path separators / parent-dir escapes / leading dots. Without
932+
// this, `skill=slides/../_unpublished/mofa-video` would resolve to
933+
// `mofa-_unpublished/mofa-video/styles` and let callers list TOML
934+
// files outside the intended catalog. Treat the input as a single
935+
// ASCII slug.
936+
if skill.is_empty()
937+
|| skill.starts_with('.')
938+
|| skill
939+
.chars()
940+
.any(|c| !(c.is_ascii_alphanumeric() || c == '-' || c == '_'))
941+
{
942+
eyre::bail!(
943+
"invalid skill name: '{skill}'. Must be a single ASCII slug \
944+
(letters, digits, '-', '_'), no path separators or leading dots."
945+
);
946+
}
932947
let styles_dir = find_styles_dir(mofa_root, skill);
933948
let names = style::list_style_names(&styles_dir);
934949

@@ -986,22 +1001,14 @@ fn plugin_list_styles(
9861001

9871002
let summary = serde_json::json!({
9881003
"skill": skill,
989-
"styles_dir": styles_dir.to_string_lossy(),
9901004
"count": names.len(),
9911005
"styles": styles_json,
9921006
});
9931007

9941008
let human = if names.is_empty() {
995-
format!(
996-
"No styles found under {} (skill={skill})",
997-
styles_dir.display()
998-
)
1009+
format!("No {skill} styles available on this deployment")
9991010
} else {
1000-
let mut lines = vec![format!(
1001-
"{} {skill} styles available (from {}):",
1002-
names.len(),
1003-
styles_dir.display()
1004-
)];
1011+
let mut lines = vec![format!("{} {skill} styles available:", names.len())];
10051012
for s in &summary["styles"].as_array().cloned().unwrap_or_default() {
10061013
let name = s.get("name").and_then(|v| v.as_str()).unwrap_or("?");
10071014
let display = s.get("display_name").and_then(|v| v.as_str()).unwrap_or("");

mofa-slides/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ CLI: `mofa slides` | Styles: `mofa-slides/styles/*.toml` | Config: `mofa/config.
2222
"slide_dir": "slides/<slug>/output/imgs"
2323
```
2424

25-
The deck ends up at `<workspace>/skill-output/slides/<slug>/output/deck.pptx`, the workspace contract picks it up via the plugin's `files_to_send`, and the deck is auto-delivered.
25+
The deck ends up at `<workspace>/skill-output/slides/<slug>/output/deck.pptx`. The host detects the `.pptx` output from the LLM-supplied `out` arg and the workspace contract surfaces it to the user automatically; no extra delivery step is needed from you.
2626

2727
**Standalone (outside Octos)** — use a unique per-request subdirectory under the current working directory:
2828

0 commit comments

Comments
 (0)