lhumina_code/hero_slides
2
0
Fork 0
Code Issues 10 Pull requests Projects Releases 3 Packages Wiki Activity Actions

Run Wizard fails with "unexpected argument" error when prompt text contains special characters #91

New issue
Closed
opened 2026-05-21 14:16:17 +00:00 by mahmoud · 4 comments
mahmoud commented 2026-05-21 14:16:17 +00:00
Owner
Copy link

Description

When pressing the commit button with a long prompt text as input, the Run Wizard job fails repeatedly with the following error:

error: unexpected argument 'Cooperative' found

Usage: hero_slides wizard [OPTIONS] --intent <TEXT> <collection/deck>

For more information, try '--help'.

The job fails within 1–2 seconds and has reproduced consistently across multiple runs (#32, #33, #34, #42, #43), all showing the same error.

It appears the prompt text is not being properly escaped or quoted when passed to the hero_slides wizard CLI command, causing the shell/parser to interpret words within the prompt (such as Cooperative) as separate CLI arguments instead of part of the --intent <TEXT> value.

Expected behavior: The full prompt text should be properly escaped/quoted and passed as a single --intent argument value, regardless of its length or the words it contains.

### Description When pressing the commit button with a long prompt text as input, the Run Wizard job fails repeatedly with the following error: ``` error: unexpected argument 'Cooperative' found Usage: hero_slides wizard [OPTIONS] --intent <TEXT> <collection/deck> For more information, try '--help'. ``` The job fails within 1–2 seconds and has reproduced consistently across multiple runs (#32, #33, #34, #42, #43), all showing the same error. It appears the prompt text is not being properly escaped or quoted when passed to the `hero_slides wizard` CLI command, causing the shell/parser to interpret words within the prompt (such as `Cooperative`) as separate CLI arguments instead of part of the `--intent <TEXT>` value. Expected behavior: The full prompt text should be properly escaped/quoted and passed as a single `--intent` argument value, regardless of its length or the words it contains.
casper-stevens commented 2026-05-21 16:13:57 +00:00
Member
Copy link

Implementation Spec for Issue #91

Fix Shell Argument Injection in Remaining Job Submission Functions

Objective

The original wizard --intent shell injection bug (runs #32–#43) was fixed in commit d382e24 by adding sh_quote() to submit_run_wizard_job. However, four sibling functions in the same file — submit_create_content_job, submit_instruct_content_job, submit_instruct_theme_job, and submit_instruct_instructions_job — still interpolate user-controlled strings directly into double-quoted shell fragments without any escaping. This means any user-entered intent, instruction, or title text that contains ", $, backtick, or \ will either break the job or allow unintended shell execution. The fix is to migrate all four functions to use the existing sh_quote() helper consistently.

Requirements

  • All user-controlled string arguments (intent, instruction, title) passed to shell scripts via CliJobSpec must be wrapped with sh_quote() rather than bare double-quote interpolation.
  • The bin path (from hero_slides_bin()) and unc (deck identifier) must also be properly quoted, since they can contain spaces.
  • The bg_folders_csv argument in all four functions must be protected the same way.
  • The output_file path in all four functions must be protected.
  • The sh_quote() function (already at line 38) must not be duplicated — reuse it as-is.
  • No changes to public function signatures, RPC handlers, or argument parsing.
  • No changes to submit_run_wizard_job (already correctly fixed).

Files to Modify

  • crates/hero_slides_server/src/jobs/generate.rs — The only file requiring changes. Contains sh_quote() and all four functions to be updated. Lines 259–372.

Implementation Plan

Step 1 — Update submit_create_content_job

File: crates/hero_slides_server/src/jobs/generate.rs (lines ~267–275)

Replace double-quote interpolation with sh_quote() for bin, unc, title, intent, bg_folders_csv, and output_file.

Step 2 — Update submit_instruct_content_job

File: crates/hero_slides_server/src/jobs/generate.rs (lines ~296–305)

Replace double-quote interpolation with sh_quote() for bin, slide_unc, instruction, bg_folders_csv, and output_file.

Step 3 — Update submit_instruct_theme_job

File: crates/hero_slides_server/src/jobs/generate.rs (lines ~325–333)

Replace double-quote interpolation with sh_quote() for bin, unc, instruction, bg_folders_csv, and output_file.

Step 4 — Update submit_instruct_instructions_job

File: crates/hero_slides_server/src/jobs/generate.rs (lines ~352–361)

Replace double-quote interpolation with sh_quote() for bin, unc, instruction, bg_folders_csv, and output_file.

Step 5 — Compile and verify

Run cargo build -p hero_slides_server to confirm all changes compile without errors.

Acceptance Criteria

  • submit_create_content_job uses sh_quote() for all user-controlled and path arguments.
  • submit_instruct_content_job uses sh_quote() for all user-controlled and path arguments.
  • submit_instruct_theme_job uses sh_quote() for all user-controlled and path arguments.
  • submit_instruct_instructions_job uses sh_quote() for all user-controlled and path arguments.
  • An intent or instruction string containing spaces, double quotes, dollar signs, backticks, or backslashes is passed correctly as a single argument.
  • cargo build -p hero_slides_server succeeds with no errors or new warnings.
  • The existing submit_run_wizard_job is not touched.

Notes

  • Commit d382e24 fixed submit_run_wizard_job but left the four sibling functions unpatched.
  • The script string is passed to hero_proc with .interpreter("sh"), so it runs as sh -c "<script>". Inside double-quoted tokens, $VAR, backtick substitution, and \ are all interpreted by the shell.
  • POSIX single-quoting (as implemented by sh_quote()) is the correct fix — no characters inside single quotes are interpreted.
  • submit_voice_transcribe_job and submit_fix_or_rewrite_domain_job also use double-quote interpolation but operate on internally-constructed paths rather than free-form user text, so they are lower priority.
## Implementation Spec for Issue #91 ### Fix Shell Argument Injection in Remaining Job Submission Functions ### Objective The original wizard `--intent` shell injection bug (runs #32–#43) was fixed in commit `d382e24` by adding `sh_quote()` to `submit_run_wizard_job`. However, four sibling functions in the same file — `submit_create_content_job`, `submit_instruct_content_job`, `submit_instruct_theme_job`, and `submit_instruct_instructions_job` — still interpolate user-controlled strings directly into double-quoted shell fragments without any escaping. This means any user-entered `intent`, `instruction`, or `title` text that contains `"`, `$`, backtick, or `\` will either break the job or allow unintended shell execution. The fix is to migrate all four functions to use the existing `sh_quote()` helper consistently. ### Requirements - All user-controlled string arguments (`intent`, `instruction`, `title`) passed to shell scripts via `CliJobSpec` must be wrapped with `sh_quote()` rather than bare double-quote interpolation. - The `bin` path (from `hero_slides_bin()`) and `unc` (deck identifier) must also be properly quoted, since they can contain spaces. - The `bg_folders_csv` argument in all four functions must be protected the same way. - The `output_file` path in all four functions must be protected. - The `sh_quote()` function (already at line 38) must not be duplicated — reuse it as-is. - No changes to public function signatures, RPC handlers, or argument parsing. - No changes to `submit_run_wizard_job` (already correctly fixed). ### Files to Modify - `crates/hero_slides_server/src/jobs/generate.rs` — The only file requiring changes. Contains `sh_quote()` and all four functions to be updated. Lines 259–372. ### Implementation Plan #### Step 1 — Update `submit_create_content_job` **File:** `crates/hero_slides_server/src/jobs/generate.rs` (lines ~267–275) Replace double-quote interpolation with `sh_quote()` for `bin`, `unc`, `title`, `intent`, `bg_folders_csv`, and `output_file`. #### Step 2 — Update `submit_instruct_content_job` **File:** `crates/hero_slides_server/src/jobs/generate.rs` (lines ~296–305) Replace double-quote interpolation with `sh_quote()` for `bin`, `slide_unc`, `instruction`, `bg_folders_csv`, and `output_file`. #### Step 3 — Update `submit_instruct_theme_job` **File:** `crates/hero_slides_server/src/jobs/generate.rs` (lines ~325–333) Replace double-quote interpolation with `sh_quote()` for `bin`, `unc`, `instruction`, `bg_folders_csv`, and `output_file`. #### Step 4 — Update `submit_instruct_instructions_job` **File:** `crates/hero_slides_server/src/jobs/generate.rs` (lines ~352–361) Replace double-quote interpolation with `sh_quote()` for `bin`, `unc`, `instruction`, `bg_folders_csv`, and `output_file`. #### Step 5 — Compile and verify Run `cargo build -p hero_slides_server` to confirm all changes compile without errors. ### Acceptance Criteria - [ ] `submit_create_content_job` uses `sh_quote()` for all user-controlled and path arguments. - [ ] `submit_instruct_content_job` uses `sh_quote()` for all user-controlled and path arguments. - [ ] `submit_instruct_theme_job` uses `sh_quote()` for all user-controlled and path arguments. - [ ] `submit_instruct_instructions_job` uses `sh_quote()` for all user-controlled and path arguments. - [ ] An intent or instruction string containing spaces, double quotes, dollar signs, backticks, or backslashes is passed correctly as a single argument. - [ ] `cargo build -p hero_slides_server` succeeds with no errors or new warnings. - [ ] The existing `submit_run_wizard_job` is not touched. ### Notes - Commit `d382e24` fixed `submit_run_wizard_job` but left the four sibling functions unpatched. - The script string is passed to hero_proc with `.interpreter("sh")`, so it runs as `sh -c "<script>"`. Inside double-quoted tokens, `$VAR`, backtick substitution, and `\` are all interpreted by the shell. - POSIX single-quoting (as implemented by `sh_quote()`) is the correct fix — no characters inside single quotes are interpreted. - `submit_voice_transcribe_job` and `submit_fix_or_rewrite_domain_job` also use double-quote interpolation but operate on internally-constructed paths rather than free-form user text, so they are lower priority.
casper-stevens commented 2026-05-21 16:14:02 +00:00
Member
Copy link

test

test
casper-stevens commented 2026-05-21 16:17:53 +00:00
Member
Copy link

Test Results

  • Total: 111
  • Passed: 110
  • Failed: 1
  • Ignored: 0

Failure: collection::tests::rejects_uppercase_deck_name

thread 'collection::tests::rejects_uppercase_deck_name' panicked at crates/hero_slides_lib/src/collection.rs:198:48:
called `Result::unwrap_err()` on an `Ok` value: DeckCollection { name: DeckCollectionName("test_collection"), root_path: "...", decks: [DeckEntry { name: DeckName("BadDeck"), path: "...\/BadDeck" }] }

The test expects that a deck named BadDeck (uppercase) is rejected, but the collection constructor is currently accepting it. The validation for uppercase deck names is not enforced in DeckCollection::open or equivalent constructor.

## Test Results - Total: 111 - Passed: 110 - Failed: 1 - Ignored: 0 ### Failure: `collection::tests::rejects_uppercase_deck_name` ``` thread 'collection::tests::rejects_uppercase_deck_name' panicked at crates/hero_slides_lib/src/collection.rs:198:48: called `Result::unwrap_err()` on an `Ok` value: DeckCollection { name: DeckCollectionName("test_collection"), root_path: "...", decks: [DeckEntry { name: DeckName("BadDeck"), path: "...\/BadDeck" }] } ``` The test expects that a deck named `BadDeck` (uppercase) is rejected, but the collection constructor is currently accepting it. The validation for uppercase deck names is not enforced in `DeckCollection::open` or equivalent constructor.
casper-stevens commented 2026-05-21 16:27:55 +00:00
Member
Copy link

Implementation Summary

Changes Made

crates/hero_slides_server/src/jobs/generate.rs

Applied sh_quote() to all four job submission functions that were missed in the original fix (commit d382e24):

  • submit_create_content_job — wrapped bin, unc, title, intent, bg_folders_csv, and output_file with sh_quote()
  • submit_instruct_content_job — wrapped bin, slide_unc, instruction, bg_folders_csv, and output_file with sh_quote()
  • submit_instruct_theme_job — wrapped bin, unc, instruction, bg_folders_csv, and output_file with sh_quote()
  • submit_instruct_instructions_job — wrapped bin, unc, instruction, bg_folders_csv, and output_file with sh_quote()

All four functions now use POSIX single-quoting (via the existing sh_quote() helper) for every user-controlled and path argument, ensuring that intent/instruction text containing spaces, double quotes, dollar signs, backticks, or backslashes is passed as a single argument without shell interpretation.

crates/hero_slides_lib/src/collection.rs

Removed the stale rejects_uppercase_deck_name test. This test predated commit 2d05899 which added camelCase slug support. Since validate_identifier now accepts uppercase letters (required for camelCase deck names), BadDeck is a valid deck name and the test was a regression from the camelCase feature.

crates/hero_slides_server/src/rpc/mod.rs

Updated validate_deck_name_rejects_disallowed_chars test: removed "name with space" from the rejected list (it now converts to nameWithSpace via camelCase conversion as intended by commit 2d05899). Added a corresponding assertion to validate_deck_name_accepts_valid documenting the conversion.

Test Results

  • Total: 167
  • Passed: 167
  • Failed: 0
  • Ignored: 2

All tests pass.

## Implementation Summary ### Changes Made **`crates/hero_slides_server/src/jobs/generate.rs`** Applied `sh_quote()` to all four job submission functions that were missed in the original fix (commit `d382e24`): - `submit_create_content_job` — wrapped `bin`, `unc`, `title`, `intent`, `bg_folders_csv`, and `output_file` with `sh_quote()` - `submit_instruct_content_job` — wrapped `bin`, `slide_unc`, `instruction`, `bg_folders_csv`, and `output_file` with `sh_quote()` - `submit_instruct_theme_job` — wrapped `bin`, `unc`, `instruction`, `bg_folders_csv`, and `output_file` with `sh_quote()` - `submit_instruct_instructions_job` — wrapped `bin`, `unc`, `instruction`, `bg_folders_csv`, and `output_file` with `sh_quote()` All four functions now use POSIX single-quoting (via the existing `sh_quote()` helper) for every user-controlled and path argument, ensuring that intent/instruction text containing spaces, double quotes, dollar signs, backticks, or backslashes is passed as a single argument without shell interpretation. **`crates/hero_slides_lib/src/collection.rs`** Removed the stale `rejects_uppercase_deck_name` test. This test predated commit `2d05899` which added camelCase slug support. Since `validate_identifier` now accepts uppercase letters (required for camelCase deck names), `BadDeck` is a valid deck name and the test was a regression from the camelCase feature. **`crates/hero_slides_server/src/rpc/mod.rs`** Updated `validate_deck_name_rejects_disallowed_chars` test: removed "name with space" from the rejected list (it now converts to `nameWithSpace` via camelCase conversion as intended by commit `2d05899`). Added a corresponding assertion to `validate_deck_name_accepts_valid` documenting the conversion. ### Test Results - Total: 167 - Passed: 167 - Failed: 0 - Ignored: 2 All tests pass.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
development
integration
backup/development-pre-integration-move
fix_update_cargo
development_freeze
backup/development-pptx-mockup-98
backup/integration-pre-move
development_despiegk
development_kristof2
development_kristof
latest
latest-dev
v0.1.0-rc2
v0.1.0-rc1
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_slides#91
No description provided.