Update cap-std-ext to 5.1.2, use new CmdFds API#2143
Update cap-std-ext to 5.1.2, use new CmdFds API#2143cgwalters merged 1 commit intobootc-dev:mainfrom
Conversation
This is a safer API, but this is specifically prep for using varlink. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
There was a problem hiding this comment.
Code Review
This pull request updates the cap-std-ext dependency to version 5.1.2 and refactors file descriptor management across several modules to use the new CmdFds API. The changes in podstorage.rs, skopeo.rs, and write.rs replace direct calls to take_fd_n on Command with the use of a CmdFds collector. A potential issue was identified in podstorage.rs where using a raw file descriptor for the registry authentication file could lead to collisions with other fixed file descriptors like STORAGE_RUN_FD.
| let target_fd = fd.as_fd().as_raw_fd(); | ||
| cmd.take_fd_n(fd, target_fd); | ||
| fds.take_fd_n(fd, target_fd); | ||
| cmd.env("REGISTRY_AUTH_FILE", format!("/proc/self/fd/{target_fd}")); |
There was a problem hiding this comment.
Using the parent process's raw file descriptor as the target FD in the child is fragile and risks collisions with other FDs being passed, such as STORAGE_RUN_FD (which is fixed at 3). If target_fd happens to be 3, CmdFds will detect a conflict. It is safer to use a fixed, distinct FD number for the registry authentication file.
| let target_fd = fd.as_fd().as_raw_fd(); | |
| cmd.take_fd_n(fd, target_fd); | |
| fds.take_fd_n(fd, target_fd); | |
| cmd.env("REGISTRY_AUTH_FILE", format!("/proc/self/fd/{target_fd}")); | |
| let target_fd = 4; // Use a fixed FD to avoid potential collisions with STORAGE_RUN_FD (3) | |
| fds.take_fd_n(fd, target_fd); | |
| cmd.env("REGISTRY_AUTH_FILE", format!("/proc/self/fd/{target_fd}")); |
This is a safer API, but this is specifically prep for using varlink.
Assisted-by: OpenCode (Claude Opus 4)