lhumina_code/hero_slides

Fork 0

fix(rpc): harden API against destructive ops, standardize param names, validate inputs #36

Merged

salmaelsoly merged 2 commits from development_rpc_api_hardening into development

2026-04-29 11:14:23 +00:00

salmaelsoly commented

2026-04-29 09:31:30 +00:00

Member

Closes #35.

Summary

Hardens the JSON-RPC API surface against destructive operations on arbitrary paths, standardizes parameter naming across deck/slide methods, and replaces silent input mangling with explicit validation.

Changes

Safety

deck_create requires existing parent and refuses pre-existing target (no more silent multi-level mkdir).
deck_delete requires .slides marker file before removal.
deck_duplicate refuses pre-existing target.

Validation

handle_deck_create/rename/duplicate use a strict validate_deck_name helper (ASCII alphanumeric + _ -; rejects spaces/punctuation/leading . or -). No more silent name mangling.
handle_slide_insert parses at with as_i64 + range check; requires at >= 1. Negative values return a real range error instead of "Missing 'at' parameter".
handle_slide_save_content rejects empty content.

Parameter standardization (with deprecation alias)

All path-taking deck.* RPC methods accept deck_path (preferred) and the legacy path (deprecated, logs a warning).
deck.scan accepts root_path (preferred, semantically correct) and path (legacy).
openrpc.json advertises only the new names; the regenerated client uses them. Downstream caller crates/hero_slides/src/main.rs updated.
dashboard.js migrated to new names (15 call sites).

Tests

6 new lib integration tests in crates/hero_slides_lib/tests/deck_safety_test.rs covering create/delete/duplicate preconditions.
8 new in-module unit tests in crates/hero_slides_server/src/rpc.rs covering validate_deck_name and param_string helpers.
One pre-existing test (test_scan_decks) updated to create the parent dir before deck_create (the test relied on the old auto-mkdir behavior).
cargo build --workspace: clean.
cargo test -p hero_slides_lib -p hero_slides_server: 84/84 passing.

Smoke test (post-deploy)

Service restarted on this branch:

deck.scan {root_path: "/tmp"}        -> works
deck.scan {path: "/tmp"}             -> works (deprecation logged)
deck.delete {deck_path: "/tmp/.X11-unix"}
                                     -> "Invalid input: not a deck (missing .slides marker): /tmp/.X11-unix"

Test plan

Manual dashboard regression: list decks, scan a directory, create a deck, rename, duplicate, get theme, save theme, delete.
Verify deprecation warnings appear in hero_proc logs hero_slides_server when a stale client uses path.
Confirm deck.create with a name containing spaces or / is rejected (not silently rewritten).

Follow-ups (not in this PR)

deck.readBackground also takes path and was not in scope; should be normalized to deck_path.
bg.* methods use path for a deck path; out of scope.
Response payloads currently include both path and deck_path for transition. Removing the legacy path field is a separate breaking change for a future release.
After one release, remove the path/legacy aliases on RPC inputs.

Closes #35. ## Summary Hardens the JSON-RPC API surface against destructive operations on arbitrary paths, standardizes parameter naming across deck/slide methods, and replaces silent input mangling with explicit validation. ## Changes ### Safety - `deck_create` requires existing parent and refuses pre-existing target (no more silent multi-level mkdir). - `deck_delete` requires `.slides` marker file before removal. - `deck_duplicate` refuses pre-existing target. ### Validation - `handle_deck_create`/`rename`/`duplicate` use a strict `validate_deck_name` helper (ASCII alphanumeric + `_` `-`; rejects spaces/punctuation/leading `.` or `-`). No more silent name mangling. - `handle_slide_insert` parses `at` with `as_i64` + range check; requires `at >= 1`. Negative values return a real range error instead of "Missing 'at' parameter". - `handle_slide_save_content` rejects empty content. ### Parameter standardization (with deprecation alias) - All path-taking `deck.*` RPC methods accept `deck_path` (preferred) and the legacy `path` (deprecated, logs a warning). - `deck.scan` accepts `root_path` (preferred, semantically correct) and `path` (legacy). - `openrpc.json` advertises only the new names; the regenerated client uses them. Downstream caller `crates/hero_slides/src/main.rs` updated. - `dashboard.js` migrated to new names (15 call sites). ## Tests - 6 new lib integration tests in `crates/hero_slides_lib/tests/deck_safety_test.rs` covering create/delete/duplicate preconditions. - 8 new in-module unit tests in `crates/hero_slides_server/src/rpc.rs` covering `validate_deck_name` and `param_string` helpers. - One pre-existing test (`test_scan_decks`) updated to create the parent dir before `deck_create` (the test relied on the old auto-mkdir behavior). - `cargo build --workspace`: clean. - `cargo test -p hero_slides_lib -p hero_slides_server`: 84/84 passing. ## Smoke test (post-deploy) Service restarted on this branch: ``` deck.scan {root_path: "/tmp"} -> works deck.scan {path: "/tmp"} -> works (deprecation logged) deck.delete {deck_path: "/tmp/.X11-unix"} -> "Invalid input: not a deck (missing .slides marker): /tmp/.X11-unix" ``` ## Test plan - [x] Manual dashboard regression: list decks, scan a directory, create a deck, rename, duplicate, get theme, save theme, delete. - [x] Verify deprecation warnings appear in `hero_proc logs hero_slides_server` when a stale client uses `path`. - [x] Confirm `deck.create` with a name containing spaces or `/` is rejected (not silently rewritten). ## Follow-ups (not in this PR) - `deck.readBackground` also takes `path` and was not in scope; should be normalized to `deck_path`. - `bg.*` methods use `path` for a deck path; out of scope. - Response payloads currently include both `path` and `deck_path` for transition. Removing the legacy `path` field is a separate breaking change for a future release. - After one release, remove the `path`/legacy aliases on RPC inputs.

salmaelsoly added 1 commit

2026-04-29 09:31:30 +00:00

fix(rpc): harden API against destructive ops, standardize param names, validate inputs

Test / test (push) Failing after 1m31s

Details

Test / test (pull_request) Failing after 2m3s

Details

aa73572706

- deck_create requires existing parent and refuses pre-existing target
- deck_delete requires .slides marker file before removal
- deck_duplicate refuses pre-existing target
- handle_deck_create/rename/duplicate: strict name validation via validate_deck_name
- handle_slide_insert: as_i64 + range check, requires at >= 1
- handle_slide_save_content: rejects empty content
- All path-taking deck.* RPC methods accept deck_path (preferred) and path (legacy, deprecation log)
- deck.scan accepts root_path (preferred) and path (legacy)
- openrpc.json updated with new names and descriptions; generated client regenerated
- dashboard.js migrated to new parameter names (15 call sites)
- api-reference.md gains deprecation notice
- 6 lib integration tests + 8 server unit tests for helpers; existing tests green

#35

salmaelsoly added 1 commit

2026-04-29 09:38:23 +00:00

fix(tests): pre-create parent dir; fmt rpc.rs test asserts

Test / test (push) Has been cancelled

Details

Test / test (pull_request) Has been cancelled

Details

64213e7965

- crates/hero_slides_rhai/tests/integration_test.rs: test_deck_scan_finds_created_decks
  now creates `sub/` before calling deck_create, since the new precondition
  rejects non-existent parents. Caught by CI.
- cargo fmt --check fix: split long lines in param_string assertions.

salmaelsoly force-pushed development_rpc_api_hardening from 64213e7965

Test / test (push) Has been cancelled

Details

Test / test (pull_request) Has been cancelled

Details

to fd2c6c286f

Test / test (pull_request) Successful in 1m15s

Details

Test / test (push) Successful in 1m35s

Details

2026-04-29 09:38:52 +00:00

Compare