cli: Handle PermissionDenied when reading /proc/1/ns/ipc#2162
cli: Handle PermissionDenied when reading /proc/1/ns/ipc#2162jmarrero wants to merge 1 commit intobootc-dev:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the join_host_ipc_namespace function to gracefully handle PermissionDenied errors when reading /proc/1/ns/ipc, which is common in restricted container environments. The review feedback identifies a critical compilation error in the error handling logic where the returned Result type does not match the function signature. Additionally, it was noted that the new debug log message will be ineffective because tracing is not yet initialized when this function is executed.
| let ns_pid1 = match std::fs::read_link("/proc/1/ns/ipc") { | ||
| Ok(v) => v, | ||
| Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { | ||
| tracing::debug!("Skipping IPC namespace join: /proc/1/ns/ipc not accessible: {e}"); |
There was a problem hiding this comment.
Note that tracing is not yet initialized when this function is called during global_init in the bootc CLI. As a result, this debug message will not be visible in the logs even if the log level is set to debug. To fix this, the tracing initialization in crates/cli/src/main.rs would need to be moved before the call to global_init.
There was a problem hiding this comment.
That's a very insightful comment.
jmarrero
force-pushed
the
fix-ipc-ns-permission-denied
branch
from
80384b4 to
b225a6a
Compare
| /// and do the rest of the work there. | ||
| fn run() -> Result<()> { | ||
| // Initialize tracing early so that global_init() can use tracing macros | ||
| bootc_utils::initialize_tracing(); |
There was a problem hiding this comment.
I want to ensure that we call into as little code as possible before global_init().
So my preferred fix here is to drop the tracing usage from global_init instead.
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Marking as requested changes per above
In restricted build environments such as Tekton/Buildah containers, /proc/1/ns/ipc can be masked even when the process has CAP_SYS_ADMIN. The read_link() call fails with EACCES, which causes bootc to exit with a fatal error. Handle PermissionDenied by silently skipping the IPC namespace join, consistent with the existing CAP_SYS_ADMIN gate. Also drop tracing::debug! from join_host_ipc_namespace() since tracing is not yet initialized when global_init() runs. Fixes: bootc-dev@d250000 Assisted-by: OpenCode (Claude Opus 4.6) Signed-off-by: Joseph Marrero Corchado <jmarrero@redhat.com>
jmarrero
force-pushed
the
fix-ipc-ns-permission-denied
branch
from
b225a6a to
c921172
Compare
2 participants
In restricted build environments such as Tekton/Buildah containers, /proc/1/ns/ipc can be masked even when the process has CAP_SYS_ADMIN. The read_link() call fails with EACCES, which causes bootc to exit with a fatal error.
Handle PermissionDenied by skipping the IPC namespace join, consistent with the existing CAP_SYS_ADMIN gate.
Fixes: d250000
Assisted-by: OpenCode (Claude Opus 4.6)