diff --git a/CHANGELOG.md b/CHANGELOG.md index 901fb38..2dddfa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ### Security - Camera auto-output keeps the exclusive `mkstemp` inode (no unlink + `${tmpl}.jpg` sibling). - Reject I2C `bus` outside 0–255 at the tool JSON boundary. +- `camera_capture` fails closed when `workspace_only` is on but `workspace_path` is empty (same deny policy as `write_file`); previously an empty workspace root allowed arbitrary JPEG output paths. --- diff --git a/src/hardware/hardware_camera.c b/src/hardware/hardware_camera.c index 5fbf95d..0d2d60c 100644 --- a/src/hardware/hardware_camera.c +++ b/src/hardware/hardware_camera.c @@ -32,6 +32,9 @@ typedef enum camera_cli_kind { static int s_camera_ready; static char s_workspace[PATH_MAX]; +/** Non-zero when workspace_only is on (set_workspace with non-NULL). Empty + * s_workspace with enforcement fails closed — same policy as write_file. */ +static int s_workspace_enforced; static hardware_camera_spawn_fn s_test_spawn; static char *s_last_argv[HARDWARE_CAMERA_ARGV_MAX]; static char s_last_argv_storage[HARDWARE_CAMERA_ARGV_MAX][ARG_BUF_SZ]; @@ -109,8 +112,12 @@ static int path_inside_workspace(const char *path) char resolved[PATH_MAX]; char path_copy[PATH_MAX]; - if (s_workspace[0] == '\0' || !path || path[0] == '\0') + /* workspace_only off (set_workspace(NULL)): no containment. */ + if (!s_workspace_enforced) return 1; + /* Enforced but missing/empty root or empty path: deny (file.c parity). */ + if (s_workspace[0] == '\0' || !path || path[0] == '\0') + return 0; if (realpath(s_workspace, ws_resolved) == NULL) return 0; if (realpath(path, resolved) != NULL) @@ -417,7 +424,15 @@ int hardware_camera_init(void) void hardware_camera_set_workspace(const char *workspace) { - if (!workspace || workspace[0] == '\0') { + if (!workspace) { + /* workspace_only off: do not enforce containment. */ + s_workspace[0] = '\0'; + s_workspace_enforced = 0; + return; + } + /* workspace_only on: enforce. Empty string fails closed (cannot validate). */ + s_workspace_enforced = 1; + if (workspace[0] == '\0') { s_workspace[0] = '\0'; return; } @@ -436,6 +451,7 @@ void hardware_camera_shutdown(void) s_test_spawn = NULL; s_camera_ready = 0; s_workspace[0] = '\0'; + s_workspace_enforced = 0; s_spawn_timeout_ms = HARDWARE_CAMERA_SPAWN_TIMEOUT_MS; s_last_argv_count = 0; memset(s_last_argv, 0, sizeof(s_last_argv)); diff --git a/src/hardware/hardware_camera.h b/src/hardware/hardware_camera.h index 48bb014..a93e87f 100644 --- a/src/hardware/hardware_camera.h +++ b/src/hardware/hardware_camera.h @@ -51,14 +51,16 @@ int hardware_camera_capture(board_id_t board, const char *camera_type, /** * Bind the file-tool workspace root used for caller-supplied capture paths. - * NULL or empty disables the check (auto temp files and unit tests). + * NULL disables containment (workspace_only off). Non-NULL enables enforcement; + * an empty string fails closed (deny caller paths) — same as write_file when + * workspace_path is missing/empty under workspace_only. */ void hardware_camera_set_workspace(const char *workspace); /** * Return 1 if @p path may be used as a caller-supplied capture output. - * Auto temp (NULL/empty) is always allowed. When a workspace is bound, - * the path must resolve under that root (same policy as write_file). + * Auto temp (NULL/empty) is always allowed. When enforcement is on, the path + * must resolve under the bound root (same policy as write_file). */ int hardware_camera_output_allowed(const char *path); diff --git a/src/tools/hardware_tools.c b/src/tools/hardware_tools.c index 1667e15..9289be9 100644 --- a/src/tools/hardware_tools.c +++ b/src/tools/hardware_tools.c @@ -100,10 +100,14 @@ static const size_t HARDWARE_TOOL_COUNT = void tool_hardware_set_config(const config_t *cfg) { g_hw_cfg = cfg; - if (cfg && config_workspace_only(cfg)) - hardware_camera_set_workspace(config_workspace_path(cfg)); - else + if (cfg && config_workspace_only(cfg)) { + const char *ws = config_workspace_path(cfg); + + /* Pass "" when path is missing so camera fails closed like write_file. */ + hardware_camera_set_workspace(ws ? ws : ""); + } else { hardware_camera_set_workspace(NULL); + } } size_t tool_hardware_get_all(const tool_t **out, size_t max_count) diff --git a/tests/test_hardware_camera.c b/tests/test_hardware_camera.c index 3e27175..1c13b16 100644 --- a/tests/test_hardware_camera.c +++ b/tests/test_hardware_camera.c @@ -214,6 +214,36 @@ static int test_output_path_outside_workspace_rejected(void) return 0; } +/** + * workspace_only with empty workspace_path must fail closed (file.c parity). + * Previously path_inside_workspace treated empty s_workspace as "allow all". + */ +static int test_empty_workspace_enforced_denies_outside(void) +{ + char result[256]; + char err[128]; + const char *outside = "/tmp/shellclaw_cam_empty_ws_escape.jpg"; + + unlink(outside); + /* Non-NULL empty string: workspace_only on, path missing/empty. */ + hardware_camera_set_workspace(""); + ASSERT(hardware_camera_output_allowed(outside) == 0); + RUN(setup_mock()); + ASSERT(hardware_camera_capture(BOARD_JETSON_ORIN_NANO, "csi", "640x480", 75, 0, 0, + outside, result, sizeof(result), err, + sizeof(err)) == -1); + ASSERT(strstr(err, "workspace") != NULL); + ASSERT(s_spawn_called == 0); + ASSERT(access(outside, F_OK) != 0); + teardown(); + /* NULL: workspace_only off — containment disabled. */ + hardware_camera_init(); + hardware_camera_set_workspace(NULL); + ASSERT(hardware_camera_output_allowed(outside) == 1); + hardware_camera_shutdown(); + return 0; +} + static int test_output_path_inside_workspace_allowed(void) { char result[256]; @@ -605,6 +635,7 @@ int main(void) RUN(test_capture_argument_validation()); RUN(test_unsafe_output_path_rejected()); RUN(test_output_path_outside_workspace_rejected()); + RUN(test_empty_workspace_enforced_denies_outside()); RUN(test_output_path_inside_workspace_allowed()); RUN(test_output_path_traversal_rejected()); RUN(test_resolution_injection_rejected());