-
Notifications
You must be signed in to change notification settings - Fork 468
bug: resolve_from() silently treats non-existent paths as image references #751
Description
Agent Diagnostic
Loaded principal-engineer-reviewer skill to evaluate an external contributor's bug report from discussion #731. The reviewer confirmed the bug by tracing the code path in crates/openshell-cli/src/run.rs.
Findings:
resolve_from()exists atrun.rs:2479and follows this resolution order:path.is_file()— checks if it's an existing file with "Dockerfile" in the name (line 2483)path.is_dir()— checks if it's an existing directory containing a Dockerfile (line 2506)value.contains('/') || value.contains(':') || value.contains('.')— falls through to treating the value as an image reference (line 2526)- Otherwise, treat as a community sandbox name (line 2531)
- When a non-existent filesystem path is passed (e.g.
--from /home/user/project/Dockerfile), steps 1 and 2 skip because neitheris_file()noris_dir()returns true, and step 3 matches because the path contains/. - The path is then passed as-is to Kubernetes as a container image reference, which fails with an opaque
InvalidImageNameerror. resolve_from()currently has zero unit tests.
Description
Actual behavior: When sandbox create --from /path/to/Dockerfile is given and the path doesn't exist, resolve_from() silently falls through to treating the path as a container image reference. Kubernetes then rejects it with InvalidImageName, giving the user no indication that the real problem is a missing file.
Expected behavior: If the value looks like a filesystem path (starts with /, ./, or ~/) but the path doesn't exist on disk, resolve_from() should return a clear error such as "Path does not exist: /path/to/Dockerfile".
Reproduction Steps
- Run
openshell sandbox create --from /nonexistent/path/Dockerfile - Observe that the CLI does not report a missing file
- Kubernetes fails with
InvalidImageNamefor/nonexistent/path/Dockerfile
Fix Approach
Add a path-existence check between step 2 (directory check) and step 3 (image-reference fallback) in resolve_from(), approximately line 2524:
- If the value starts with
/,./, or~/and the path does not exist, return an early error with a clear message. - OCI image references never start with
/or./, so this heuristic has no false-positive risk for valid image refs. - Add unit tests for
resolve_from().
Environment
- Discovered on Intel Mac + Colima + Docker-wrapped openshell CLI
- Applicable to all platforms
Logs
InvalidImageName: /home/user/project/Dockerfile